Closed silabs-PaulZ closed 4 years ago
FPGA synthesis `define may be used to increase the default count to the maximum allowed
I have had complaints about this feature in the current design. Personally I also think that we should not reconfigure modules just because the core is mapped onto FPGA. If FPGA users want a different configuration they should simply explicitly use the top level parameters to configure the core as they want (and not rely on a non-obvious side effect of ifdef FPGA)
What will the external (top level) interface look like?
Based on some feedback, I propose we remove the external event counters and also make the number of MPHCounters parameterizable with no FPGA `define.
I will modify the original ticket description comment. Even though the comment history is kept, the changes are listed below for convenience.
I agree that the external perf counters should be memory mapped, thus readable and writable with load and stores and not mapped to the CSR space. Rather than removing the LD_EX, ST_EX we should move them to NUM_MHPMCOUNTERS ? Is that correct?
Best Davide
Yes, these 'could' be part of the HPM Counters (mhpmcounter[]). There is a corresponding HPM Event register (mphmevent[]) that selects which events will increment each counter (similar to PCER, but per counter).
These external events were brought in through an external port that was removed.
I would prefer that we do not bring in external events.
But, if needed, it would be fairly easy of course. My concern then would be that the external events are user selectable, meaning they could hook up anything, not just a LD_EX (presumably LoaD EXternal). We would add the port and extend the event register with names such as EXT0, EXT1, EXT2, EXT3. If needed, can we create a separate issue/pull request?
Agreed to not implement EXT0, EXT1, EXT2, EXT3 at the moment. External events can be added on a future core if users request it.
We are closing this issue as it either has been fixed with the stated (merged) pull request, because it is no longer valid, or because a follow up question for further clarification or feedback remained unanswered for at least 7 days. Please do not hesitate to re-open this issue if you do not agree that the issue has been handled satisfactorily or if you have further questions.
The following change was approved by the OpenHW Core Task Group:
Replace existing performance counter implementation with the Hardware Performance Monitor specified in the privileged architecture.
Access to an existing performance counter will be removed:
Specific feature details that were not fully discussed are now mentioned in this issue :