search

openhwgroup / core-v-mcu

This is the CORE-V MCU project, hosting CORE-V's embedded-class cores.
https://docs.openhwgroup.org/projects/core-v-mcu
Other
166 stars 50 forks source link

Centralize compiler macros in a top-level package #188

Open MikeOpenHWGroup opened 2 years ago

MikeOpenHWGroup commented 2 years ago

There is at least one instance of a compiler macro in CORE-V-MCU that is defined and used in disjointed parts of the RTL. The macro in question is USE_FPU. It is used in fc_subsystem.sv to set the FPU parameter for the CV32E40P core. The macro is defined in pulp_soc_defines.sv. This is a dangerous situation because the value of USE_FPU passed to the core is dependent on compile order:

At a minimum, the above situation must be resolved. It would also be a great idea to review all macros in the MCU and ensure that either the macros are defined in a top-level package or define in the module in which they are used.

@gmartin102, I am assigning this to you as you are the default RTL engineer on this project. Feel free to delegate.

gmartin102 commented 2 years ago

@MikeOpenHWGroup - I thought the `include "pulp_soc_defines.sv" on line 10 or so inthe fc_subsystem would read the definitions in before compilation. maybe the file should be an svh suffix to properly denote it as a header file? Our (My) intent was to avoid the compile order issue and eliminate the confusion by having the parameter ripple up a down the hierarchy as an instantiated parameter.

MikeOpenHWGroup commented 2 years ago

the `include "pulp_soc_defines.sv" on line 10 or so inthe fc_subsystem would read the definitions in before compilation.

Yeah it "works", but its dangerous to make the fc_subsystem dependent on the pulp_soc_defines.

maybe the file should be an svh suffix to properly denote it as a header file?

I like that one better. If we are going to have defines anywhere they should be in a header file (svh) at the top of the hierarchy.

Better yet, just get rid of it. the fc_subsystem already has a USE_FPU parameter so that should be passed to the core.

MikeOpenHWGroup commented 2 years ago

Much, but not all, of this work has already been completed. Completion of this task will not gate V1.0.0