Converted Vec to UInt #2597
Converted Vec to UInt #2597
Conversation
| }.elsewhen (hartIsInResetSync.reduce(_ | _)) { | ||
| haveResetBitRegs := haveResetBitRegs | hartIsInResetSync.asUInt | ||
| }.elsewhen (io.innerCtrl.fire() && io.innerCtrl.bits.ackhavereset) { | ||
| haveResetBitRegs := haveResetBitRegs & (~(hamaskWrSel.asUInt)) |
There was a problem hiding this comment.
This logic is not the same as the original. Suppose hart 23 is always in reset (maybe it is waiting for hart 0 to enable it). The second clause will always take effect, preventing the third clause from handling ackhavereset for other harts.
There was a problem hiding this comment.
Hi,
Thank you for the inputs.
I wrongly assumed only 1 hart would be in 1 state at a time. I'm looking into correcting this error.
| when (hartIsInResetSync.reduce(_ | _)) { | ||
| haltedBitRegs := haltedBitRegs & ~(hartIsInResetSync.asUInt) | ||
| resumeReqRegs := resumeReqRegs & ~(hartIsInResetSync.asUInt) | ||
| }.elsewhen (hartHaltedWrEn) { |
There was a problem hiding this comment.
Same comment - this code is not functionally equivalent. You must allow for some harts to be in reset while others are not.
There was a problem hiding this comment.
This is much closer, but it still doesn't quite have the same behavior it used to. When there are multiple active assignments to one Reg, the last one takes effect and all others are ignored. In other words, you need to handle setting some bits of haltedBitRegs and haveResetBitRegs and clearing others in the same expression.
val a = RegInit(0.U(8.W))
a := a | 1.U
a := a | 2.U
results in a=2, not a=3. Only the second assignment occurs.
Also, please convert all the tab characters in the file to spaces.
| val hartHaltedIdIndex = UIntToOH(hartSelFuncs.hartIdToHartSel(hartHaltedId)) | ||
| val hartResumingIdIndex = UIntToOH(hartSelFuncs.hartIdToHartSel(hartResumingId)) | ||
| val hartselIndex = UIntToOH(io.innerCtrl.bits.hartsel) | ||
| when (hartHaltedWrEn) { | ||
| haltedBitRegs := haltedBitRegs | hartHaltedIdIndex | ||
| }.elsewhen (hartResumingWrEn) { | ||
| when (hartSelFuncs.hartIdToHartSel(hartResumingId) === component.U) { | ||
| haltedBitRegs(component) := false.B | ||
| } | ||
| haltedBitRegs := haltedBitRegs & ~(hartResumingIdIndex) | ||
| } |
There was a problem hiding this comment.
This will fail if one hart's hartIsInResetSync is 1 in the same cycle that another hart is doing hartHaltedWrEn. I suggest combining everything into one expression, like this:
when (hartHaltedWrEn) {
haltedBitRegs := (haltedBitRegs | hartHaltedIdIndex) & ~(hartIsInResetSync.asUInt)
}.elsewhen (hartResumingWrEn) {
haltedBitRegs := haltedBitRegs & ~(hartResumingIdIndex) & ~(hartIsInResetSync.asUInt)
}.otherwise {
haltedBitRegs := haltedBitRegs & ~(hartIsInResetSync.asUInt)
}
There was a problem hiding this comment.
I tried this out, but the asm-tests run indefinitely in this case.
There was a problem hiding this comment.
Correction.
It was when I tried applying the same transformation to resumeReqRegs that the asm-tests run indefintely.
For now pulling only haltedBitRegs
Co-authored-by: Ernie Edgar <43148441+ernie-sifive@users.noreply.github.com>
|
I'm running some additional tests, will merge assuming they come back green. |
|
Turns out my tests were flakey, thanks a lot @MathewTG! |
Converted Vec to UInt to resolve verilog elaboration issue - freechipsproject/chisel3 Issue #1456
*Enhancement to devices/debug/Debug.scala to resolve verilog elaboration issue.
Changes incorporated as mentioned in freechipsproject/chisel3 - Issue 1456(Not in chipalliance/rocket-chip)