Skip to content

Conversation

@jerryz123
Copy link
Contributor

An alternative approach to #3355, core implementations can set a custom traceCustom field to pull out extra information out of the TraceBundle.

@bgottschall does this look OK for you?

Related issue:

Type of change: other enhancement

Impact: API addition (no impact on existing code)

Development Phase: implementation

Release Notes

@jerryz123
Copy link
Contributor Author

@ZenithalHourlyRate @sequencer the CI machines do not provide logs, and the mill flow on dev fails for me with

[5/140] mill.scalalib.ZincWorkerModule.worker 
1 targets failed
mill.scalalib.ZincWorkerModule.worker java.lang.ClassNotFoundException: mill.scalalib.worker.ZincWorkerImpl
    java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:476)
    mill.api.ClassLoader$$anon$1.findClass(ClassLoader.scala:47)
    java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:589)
    java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
    mill.scalalib.ZincWorkerModule.$anonfun$worker$3(ZincWorkerModule.scala:58)
make: *** [/nscratch/jerryz/chipyard-proj/rocket-chip/Makefrag:47: /nscratch/jerryz/c

@bgottschall
Copy link

@bgottschall does this look OK for you?

Yes, that looks good!

@sequencer
Copy link
Member

Interesting, I'll debug it tomorrow.

@sequencer
Copy link
Member

I have two concerns, both these are not problem in the near future, and won't block this PR:

  • Data cannot be serialized, although this is a common issue in most of Chisel designs, but I think we should eventually get rid of this in the user API of rocket-chip. user can use json to config their IP, rather than CDE.
  • Trace has two design purpose: one for riscv-trace-spec, one for firesim-like debug. I personally think the later one is a part of verification and should be implemented with XMR.

@jerryz123
Copy link
Contributor Author

  • Data cannot be serialized, although this is a common issue in most of Chisel designs, but I think we should eventually get rid of this in the user API of rocket-chip. user can use json to config their IP, rather than CDE.

traceCustom doesn't have to be a user API, it is a source API, to allow other implementations of Tile to provide more information through that port. If a user wants to add more signals to that, then they have to edit source code to drive those signals from within the core anyways.

  • Trace has two design purpose: one for riscv-trace-spec, one for firesim-like debug. I personally think the later one is a part of verification and should be implemented with XMR.

Unfortunately firesim does not support XMR yet

@jerryz123 jerryz123 requested a review from sequencer May 11, 2023 17:52
@jerryz123
Copy link
Contributor Author

@Mergifyio backport master

@mergify
Copy link
Contributor

mergify bot commented May 12, 2023

backport master

✅ Backports have been created

Details

@jerryz123 jerryz123 merged commit a9aa444 into dev May 12, 2023
@jerryz123 jerryz123 deleted the trace branch May 12, 2023 05:03
jerryz123 added a commit that referenced this pull request May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants