Chisel decoder #2836

sequencer · 2021-05-12T14:31:51Z

This PR use new chisel API to replace original RocketChip Decoder.
Chisel Decoder depends on the explicit width, so I adds explicit width for most usages. This will be a breaking change to RC.
Related issue:

Type of change: new feature

Impact: no functional change

Development Phase: implementation

Release Notes
Switch to Chisel Decoder

linux-foundation-easycla · 2021-05-12T14:31:56Z

The committers are authorized under a signed CLA.

✅ Jiuyang Liu (2ce2750, 6bf257d)

sequencer · 2022-01-18T11:47:57Z

@jackkoenig Ask to merge this.

jackkoenig · 2022-02-01T19:08:30Z

I bumped to 3.5-SNAPSHOT to show that this is still failing (at least for me locally)

sequencer · 2022-02-01T21:56:04Z

This is really weird:

  BitPat(0000001??????????000?????0110011) -> BitPat(1000001101001???100?????00000110000000)
  BitPat(0000001??????????001?????0110011) -> BitPat(1000001101001???110?????00000110000000)
  BitPat(0000001??????????011?????0110011) -> BitPat(1000001101001???1110?????00000110000000)
  BitPat(0000001??????????010?????0110011) -> BitPat(1000001101001???1100?????00000110000000)
  BitPat(0000001??????????100?????0110011) -> BitPat(1000001101001???11000?????00000110000000)
  BitPat(0000001??????????101?????0110011) -> BitPat(1000001101001???11010?????00000110000000)
  BitPat(0000001??????????110?????0110011) -> BitPat(1000001101001???11100?????00000110000000)
  BitPat(0000001??????????111?????0110011) -> BitPat(1000001101001???11110?????00000110000000)
  BitPat(00000????????????010?????0101111) -> BitPat(1000001100001???101100000000010000010)
  BitPat(00100????????????010?????0101111) -> BitPat(1000001100001???101100100000010000010)
  BitPat(00001????????????010?????0101111) -> BitPat(1000001100001???10110000000010000010)
  BitPat(01100????????????010?????0101111) -> BitPat(1000001100001???101101100000010000010)
  BitPat(01000????????????010?????0101111) -> BitPat(1000001100001???101101000000010000010)
  BitPat(10000????????????010?????0101111) -> BitPat(1000001100001???101110000000010000010)
  BitPat(11000????????????010?????0101111) -> BitPat(1000001100001???101111000000010000010)
  BitPat(10100????????????010?????0101111) -> BitPat(1000001100001???101110100000010000010)
  BitPat(11100????????????010?????0101111) -> BitPat(1000001100001???101111100000010000010)
  BitPat(00010??00000?????010?????0101111) -> BitPat(1000001100001???10111000000010000010)
  BitPat(00011????????????010?????0101111) -> BitPat(1000001100001???10111100000010000010)
  BitPat(0010000??????????000?????1010011) -> BitPat(110000000????????????0?????11010000000000)
  BitPat(0010000??????????010?????1010011) -> BitPat(110000000????????????0?????11010000000000)
  BitPat(0010000??????????001?????1010011) -> BitPat(110000000????????????0?????11010000000000)
  BitPat(0010100??????????000?????1010011) -> BitPat(110000000????????????0?????11010000000000)
  BitPat(0010100??????????001?????1010011) -> BitPat(110000000????????????0?????11010000000000)
  BitPat(0000000??????????????????1010011) -> BitPat(110000000????????????0?????11010000000000)
  BitPat(0000100??????????????????1010011) -> BitPat(110000000????????????0?????11010000000000)
  BitPat(0001000??????????????????1010011) -> BitPat(110000000????????????0?????11010000000000)
  BitPat(?????00??????????????????1000011) -> BitPat(110000000????????????0?????11110000000000)
  BitPat(?????00??????????????????1000111) -> BitPat(110000000????????????0?????11110000000000)
  BitPat(?????00??????????????????1001111) -> BitPat(110000000????????????0?????11110000000000)
  BitPat(?????00??????????????????1001011) -> BitPat(110000000????????????0?????11110000000000)
  BitPat(111000000000?????001?????1010011) -> BitPat(110000000????????????0?????10000010000000)
  BitPat(111000000000?????000?????1010011) -> BitPat(110000000????????????0?????10000010000000)
  BitPat(110000000000?????????????1010011) -> BitPat(110000000????????????0?????10000010000000)
  BitPat(110000000001?????????????1010011) -> BitPat(110000000????????????0?????10000010000000)
  BitPat(1010000??????????010?????1010011) -> BitPat(110000000????????????0?????11000010000000)
  BitPat(1010000??????????001?????1010011) -> BitPat(110000000????????????0?????11000010000000)
  BitPat(1010000??????????000?????1010011) -> BitPat(110000000????????????0?????11000010000000)
  BitPat(111100000000?????000?????1010011) -> BitPat(110000010??01????????0?????00010000000000)
  BitPat(110100000000?????????????1010011) -> BitPat(110000010??01????????0?????00010000000000)
  BitPat(110100000001?????????????1010011) -> BitPat(110000010??01????????0?????00010000000000)
  BitPat(?????????????????010?????0000111) -> BitPat(1100000101101100101000010000000000)
  BitPat(?????????????????010?????0100111) -> BitPat(1100000101101000101101000000000000)
  BitPat(0001100??????????????????1010011) -> BitPat(110000000????????????0?????11010000000000)
  BitPat(010110000000?????????????1010011) -> BitPat(110000000????????????0?????11010000000000)
  BitPat(0000000??????????001?????0010011) -> BitPat(1000000101101100110?????00000010000000)
  BitPat(0000000??????????101?????0010011) -> BitPat(100000010110110011010?????00000010000000)
  BitPat(0100000??????????101?????0010011) -> BitPat(1000000101101100110110?????00000010000000)
  BitPat(0001001??????????000000001110011) -> BitPat(1000001100001???1011010000000001000000)
  BitPat(00010000001000000000000001110011) -> BitPat(1000000?0????????????0?????00000001000000)
  BitPat(01111011001000000000000001110011) -> BitPat(1000000?0????????????0?????00000001000000)
  BitPat(?????????????????001?????0001111) -> BitPat(100000000????????????0?????00000000001100)
  BitPat(?????????????????001?????1100011) -> BitPat(1001001101001001?110?????00000000000000)
  BitPat(?????????????????000?????1100011) -> BitPat(1001001101001001?100?????00000000000000)
  BitPat(?????????????????100?????1100011) -> BitPat(1001001101001001?11000?????00000000000000)
  BitPat(?????????????????110?????1100011) -> BitPat(1001001101001001?11100?????00000000000000)
  BitPat(?????????????????101?????1100011) -> BitPat(1001001101001001?11010?????00000000000000)
  BitPat(?????????????????111?????1100011) -> BitPat(1001001101001001?11110?????00000000000000)
  BitPat(?????????????????????????1101111) -> BitPat(1000100000110011100?????00000010000000)
  BitPat(?????????????????000?????1100111) -> BitPat(1000010101101100100?????00000010000000)
  BitPat(?????????????????????????0010111) -> BitPat(1000000001110010100?????00000010000000)
  BitPat(?????????????????000?????0000011) -> BitPat(1000000101101100101000000010000000)
  BitPat(?????????????????001?????0000011) -> BitPat(1000000101101100101000000010000000)
  BitPat(?????????????????010?????0000011) -> BitPat(1000000101101100101000000010000000)
  BitPat(?????????????????100?????0000011) -> BitPat(1000000101101100101000000010000000)
  BitPat(?????????????????101?????0000011) -> BitPat(1000000101101100101000000010000000)
  BitPat(?????????????????000?????0100011) -> BitPat(1000001101101000101100000000000000)
  BitPat(?????????????????001?????0100011) -> BitPat(1000001101101000101100000000000000)
  BitPat(?????????????????010?????0100011) -> BitPat(1000001101101000101100000000000000)
  BitPat(?????????????????????????0110111) -> BitPat(1000000001100010100?????00000010000000)
  BitPat(?????????????????000?????0010011) -> BitPat(1000000101101100100?????00000010000000)
  BitPat(?????????????????010?????0010011) -> BitPat(1000000101101100111000?????00000010000000)
  BitPat(?????????????????011?????0010011) -> BitPat(1000000101101100111100?????00000010000000)
  BitPat(?????????????????111?????0010011) -> BitPat(100000010110110011110?????00000010000000)
  BitPat(?????????????????110?????0010011) -> BitPat(100000010110110011100?????00000010000000)
  BitPat(?????????????????100?????0010011) -> BitPat(100000010110110011000?????00000010000000)
  BitPat(0000000??????????000?????0110011) -> BitPat(1000001101001???100?????00000010000000)
  BitPat(0100000??????????000?????0110011) -> BitPat(1000001101001???110100?????00000010000000)
  BitPat(0000000??????????010?????0110011) -> BitPat(1000001101001???111000?????00000010000000)
  BitPat(0000000??????????011?????0110011) -> BitPat(1000001101001???111100?????00000010000000)
  BitPat(0000000??????????111?????0110011) -> BitPat(1000001101001???11110?????00000010000000)
  BitPat(0000000??????????110?????0110011) -> BitPat(1000001101001???11100?????00000010000000)
  BitPat(0000000??????????100?????0110011) -> BitPat(1000001101001???11000?????00000010000000)
  BitPat(0000000??????????001?????0110011) -> BitPat(1000001101001???110?????00000010000000)
  BitPat(0000000??????????101?????0110011) -> BitPat(1000001101001???11010?????00000010000000)
  BitPat(0100000??????????101?????0110011) -> BitPat(1000001101001???110110?????00000010000000)
  BitPat(?????????????????000?????0001111) -> BitPat(100000000????????????0?????00000000000100)
  BitPat(00000000000000000000000001110011) -> BitPat(1000000?0????????????0?????00000001000000)
  BitPat(00000000000100000000000001110011) -> BitPat(1000000?0????????????0?????00000001000000)
  BitPat(00110000001000000000000001110011) -> BitPat(1000000?0????????????0?????00000001000000)
  BitPat(00010000010100000000000001110011) -> BitPat(1000000?0????????????0?????00000001000000)
  BitPat(00110000010100000000000001110011) -> BitPat(1000000?0????????????0?????00000001000000)
  BitPat(?????????????????001?????1110011) -> BitPat(1000000100001???100?????00000011010000)
  BitPat(?????????????????010?????1110011) -> BitPat(1000000100001???100?????00000011100000)
  BitPat(?????????????????011?????1110011) -> BitPat(1000000100001???100?????00000011110000)
  BitPat(?????????????????101?????1110011) -> BitPat(1000000001100101100?????00000011010000)
  BitPat(?????????????????110?????1110011) -> BitPat(1000000001100101100?????00000011100000)
  BitPat(?????????????????111?????1110011) -> BitPat(1000000001100101100?????00000011110000)

The Rocket Decoder do have different output width...
However that should be impossible...

src/main/scala/rocket/ALU.scala

When using new API, io.rw.addr << 20 would not match any entry in decode_table. e.g. When io.rw.addr[11:0] == "h102", after left shift 20 it would become "h10200000", which wont match SRET ("h10200073") as defined in Instructions.scala. Note, SRET is not defined with DontCare. Magically, old API can work with this case, which should not. In this fix the least significant 20 bits of decode_table keys are dropped to match addr.

linux-foundation-easycla · 2022-02-21T04:40:59Z

The committers are authorized under a signed CLA.

✅ Jack Koenig (238b61e)
✅ Jiuyang Liu (4c3d432, 8402324, e5e070f, bed0d93, 81355ac)
✅ Zenithal (ae002f5)

sequencer · 2022-02-21T04:51:01Z

ae002f5 fixed a legacy bug from original QMC decoder in RocketChip. Thanks debugging by @ZenithalHourlyRate

sequencer · 2022-02-21T07:08:02Z

@jackkoenig Ask for merge.

sequencer · 2022-02-22T20:09:19Z

It works with 3.5.1 now. After review, I'll rebase it.

src/main/scala/rocket/Consts.scala

src/main/scala/tilelink/Bundles.scala

jackkoenig

Speaking with rocket-chip folks, it would be better to not need the explicit widths, let's get 3.5.2 released and bump to that which will make this work better.

This involves ensuring all BitPats that correspond to a particular portion of the decoded result are of the same width. BitPats with no "don't cares" can be zero padded to the common width. BitPats with "don't cares" that are too small are an error.

jackkoenig · 2022-05-16T22:24:03Z

@sequencer I've pushed some substantial changes that remove the need for any explicit widths. Now DecodeLogic is safe to differing widths from the BitPats (and will zero pad them so long as they have no don't cares). Since I've now contributed substantially to this, I'm going to dismiss my review and request review from @aswaterman

I am now a contributor to this PR

michael-etzkorn · 2022-05-16T22:28:38Z

Ignore my review / deleted comments, the build.sbt commit changes aren't relevant to the overall PR

jackkoenig · 2022-05-16T22:35:21Z

For posterity who may hit the requirement about a BitPat having don't cares and needing padding, see #2949 for an example of the general approach.

aswaterman

Ship it!

sequencer · 2022-05-17T06:40:02Z

opps, I doubt this may blames to CSR addresses decoding.

jackkoenig · 2022-05-17T16:19:14Z

opps, I doubt this may blames to CSR addresses decoding.

@sequencer I've restored one of your changes to the CSR in f2cbe12, do you think this is the issue?

sequencer · 2022-05-17T17:47:55Z

Yes, I have been thinking, I thought it used to be a bug, but I can't understand, why it worked before...

sequencer · 2022-05-17T17:55:16Z

I just greped code usage from public dependencies(UCB/SiFive/Xiangshan): I think only CSR need to be tweaked.
But I think this need more carefully debugging to figure the reason, in case of any subtle bugs.

in light of possible bug with CSR decoding

sequencer · 2022-05-17T18:03:59Z

In chipsalliance/chisel#2387, I pad 0 to LHS, since I think it is intuitive.
In

rocket-chip/src/main/scala/rocket/Decode.scala

Line 14 in 1f05ea8

    
           cache.getOrElseUpdate(t, (if (t.mask == 0) addr else addr & Bits(BigInt(2).pow(addrWidth)-(t.mask+1), addrWidth)) === Bits(t.value, addrWidth))

It seems to pad RHS?

aswaterman

This is fine with me. I was relying on QM doing the optimal thing (there should be no dependence on input bits 19..0 in an optimized circuit).

Partially reverts #2836, but in such a way that it works with the new QMC.

sequencer force-pushed the chisel_decoder branch from e5befad to 6bf257d Compare January 18, 2022 11:46

sequencer requested a review from jackkoenig January 18, 2022 11:47

jackkoenig mentioned this pull request Jan 19, 2022

[DO NOT MERGE] NBDCache Broken with Chisel 3.5.0 Bump #2932

Closed

sequencer force-pushed the chisel_decoder branch from 9d22607 to 2376492 Compare January 26, 2022 02:35

sequencer mentioned this pull request Jan 26, 2022

Fix Decoder bug for constant 0 and DC chipsalliance/chisel#2363

Merged

14 tasks

sequencer force-pushed the chisel_decoder branch from 0d18cec to 921b8fd Compare February 1, 2022 21:58

sequencer commented Feb 1, 2022

View reviewed changes

src/main/scala/rocket/ALU.scala Outdated Show resolved Hide resolved

sequencer force-pushed the chisel_decoder branch from a9f595e to 24baee5 Compare February 1, 2022 23:29

sequencer force-pushed the chisel_decoder branch from bf87454 to b48ddfe Compare February 13, 2022 02:54

jackkoenig and others added 6 commits February 17, 2022 17:09

bump to Chisel 3.5-SNAPSHOT

238b61e

remove original Decoder API.

4c3d432

switch to new chisel API.

8402324

use new UInt API for some constants and add explicit width to ALU ucode.

e5e070f

add explicit CSR address for Decoder.

bed0d93

add explicit width to TLMessage

81355ac

sequencer force-pushed the chisel_decoder branch from b48ddfe to 81355ac Compare February 17, 2022 09:10

jackkoenig reviewed Feb 25, 2022

View reviewed changes

src/main/scala/rocket/Consts.scala Outdated Show resolved Hide resolved

jackkoenig reviewed Feb 25, 2022

View reviewed changes

src/main/scala/tilelink/Bundles.scala Outdated Show resolved Hide resolved

jackkoenig previously requested changes Mar 1, 2022

View reviewed changes

jackkoenig added 2 commits May 16, 2022 15:19

Make Decode logic width-safe

85a1183

This involves ensuring all BitPats that correspond to a particular portion of the decoded result are of the same width. BitPats with no "don't cares" can be zero padded to the common width. BitPats with "don't cares" that are too small are an error.

Undo unnecessary width additions

c7419af

Undo changes to build.sbt

a71de1c

jackkoenig requested a review from aswaterman May 16, 2022 22:24

chipsalliance deleted a comment from jackkoenig May 16, 2022

aswaterman previously approved these changes May 17, 2022

View reviewed changes

Restore tweak to CSRFile Decode logic

f2cbe12

Merge branch 'master' into chisel_decoder

e165ad8

aswaterman approved these changes May 17, 2022

View reviewed changes

jackkoenig enabled auto-merge (squash) May 17, 2022 19:20

jackkoenig merged commit e8b8efd into master May 17, 2022

jackkoenig deleted the chisel_decoder branch May 17, 2022 19:27

aswaterman added a commit that referenced this pull request Jun 4, 2022

Fix decoding of CSR addresses when usingHypervisor

24a0e6e

Partially reverts #2836, but in such a way that it works with the new QMC.

aswaterman mentioned this pull request Jun 4, 2022

Fix decoding of CSR addresses when usingHypervisor #2994

Merged

aswaterman added a commit that referenced this pull request Jun 4, 2022

Fix decoding of CSR addresses when usingHypervisor (#2994)

8a4f400

Partially reverts #2836, but in such a way that it works with the new QMC.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chisel decoder #2836

Chisel decoder #2836

sequencer commented May 12, 2021 •

edited

Loading

linux-foundation-easycla bot commented May 12, 2021 •

edited

Loading

sequencer commented Jan 18, 2022

jackkoenig commented Feb 1, 2022

sequencer commented Feb 1, 2022 •

edited

Loading

linux-foundation-easycla bot commented Feb 21, 2022 •

edited

Loading

sequencer commented Feb 21, 2022

sequencer commented Feb 21, 2022

sequencer commented Feb 22, 2022

jackkoenig left a comment

jackkoenig commented May 16, 2022 •

edited

Loading

michael-etzkorn commented May 16, 2022 •

edited

Loading

jackkoenig commented May 16, 2022

aswaterman left a comment

sequencer commented May 17, 2022

jackkoenig commented May 17, 2022

sequencer commented May 17, 2022 •

edited

Loading

sequencer commented May 17, 2022

sequencer commented May 17, 2022

aswaterman left a comment

Chisel decoder #2836

Chisel decoder #2836

Conversation

sequencer commented May 12, 2021 • edited Loading

linux-foundation-easycla bot commented May 12, 2021 • edited Loading

sequencer commented Jan 18, 2022

jackkoenig commented Feb 1, 2022

sequencer commented Feb 1, 2022 • edited Loading

linux-foundation-easycla bot commented Feb 21, 2022 • edited Loading

sequencer commented Feb 21, 2022

sequencer commented Feb 21, 2022

sequencer commented Feb 22, 2022

jackkoenig left a comment

Choose a reason for hiding this comment

jackkoenig commented May 16, 2022 • edited Loading

michael-etzkorn commented May 16, 2022 • edited Loading

jackkoenig commented May 16, 2022

aswaterman left a comment

Choose a reason for hiding this comment

sequencer commented May 17, 2022

jackkoenig commented May 17, 2022

sequencer commented May 17, 2022 • edited Loading

sequencer commented May 17, 2022

sequencer commented May 17, 2022

aswaterman left a comment

Choose a reason for hiding this comment

sequencer commented May 12, 2021 •

edited

Loading

linux-foundation-easycla bot commented May 12, 2021 •

edited

Loading

sequencer commented Feb 1, 2022 •

edited

Loading

linux-foundation-easycla bot commented Feb 21, 2022 •

edited

Loading

jackkoenig commented May 16, 2022 •

edited

Loading

michael-etzkorn commented May 16, 2022 •

edited

Loading

sequencer commented May 17, 2022 •

edited

Loading