Closed suehtamacv closed 7 years ago
Hi
I did a quick review. In file goil/templates/build/build_py.goilTemplate you added code if target is riscv.
1)
+cSourceList.append(projfile.ProjectFile("% !PROJECT %/tpl_vectors.c")) +cSourceList.append(projfile.ProjectFile("% !PROJECT %/tpl_primary_irq.S"))
should be set using the OIL attribute GENERATED_FILE. Check the other targets in the templates/.../config hierarchy
2)
+% +if TARGET == "riscv/pulpino" then +% +PULPINO_PATH = os.environ.get('PULPINO_PATH','MUST_SET_PULPINO_PATH_ENV_VARIABLE') +% +end if +%
Maybe this kind of thing can use an additional OIL attribute like NEEDS_ENV in the templates/.../config hierarchy to list the environment variables needed. In build.py.goilTemplate, a test could be generated for each of these attribute.
PULPINO_PATH
variable is necessary to correctly compile the code, as it contains Modelsim simulation files and tools to program the FPGA board. I refer to $PULPINO_PATH
in my build script. There is always the need to actually export this variable, as precised in my README, but no need to make the Python systems aware of its existence. I dropped these lines.Believe it was the single root Trampoline file that I touched.
Another idea to solve the second point would be to include PULPino's repo as a submodule to Trampoline. The submodule would be cloned only after a user request with git submodule update
(thus would not pollute this repository) and at least Trampoline would be self-contained. With PULPino in a known path, the user would be no longer be expected to indicate PULPino's location with $PULPINO_PATH
...
Which solution do you think that works better with Trampoline ? Even with submodules Trampoline would still expect the user to "clone" PULPino and to run setup.csh
, so I am not sure the gain is relevant enough to justify the change.
Regards.
Hi !
In fact we may need some way to generate checks in the build.py with goil (and maybe a goil option to force them to a value).
For the meantime, you can replicate the generation of the build.py file in goil/templates/build/riscv{/pulpino,}/ directory and do whatever you want in this file. Here's a patch that does this :
0002-pulpino-check-Add-PULPINO_PATH-environnment-variable.patch.txt
Plus, you can add other architecture-dependant oil relative checking in the goil/templates/check/riscv{/pulpino,} directory. Here's a patch that returns an error if trying to use Memory or Timing protection :
0001-pulpino-check-Warn-if-trying-to-use-Memory-Timing-pr.patch.txt
Hello,
The patches were applied !
Regards.
Hello,
I merged the pull-request. Thank you very much for your contribution !
Solves issue #55.