chisel decoder. by sequencer · Pull Request #2 · carlosedp/chiselv

sequencer · 2021-11-24T01:32:39Z

switch to decoder, recode InstructionType uop to OneHot, improve dispatch timing by removing useless Mux.
(just a prototype, not guarantee the correctness ;p)

sequencer · 2021-11-24T01:35:18Z

src/main/scala/Constants.scala

+  val INST_I = Value((1 << 0).U)
+  val INST_S = Value((1 << 1).U)
+  val INST_B = Value((1 << 2).U)
+  val INST_U = Value((1 << 3).U)
+  val INST_J = Value((1 << 4).U)
+  val INST_Z = Value((1 << 5).U)
+  val INST_R = Value((1 << 6).U)
+  val IN_ERR = Value((1 << 7).U)


From the RTL designer's review, your usage in decoder module needs a OH encoding which saves the extract comparing logic: (not (or (xor a b)))

This is great... with chipsalliance/chisel#2261 it will be even better :)

src/main/scala/Decoder.scala

sequencer · 2021-11-24T01:39:31Z

src/main/scala/Decoder.scala

+      0.U, // padding for InstructionType
+      0.U // padding for InstructionType


Actually these padding are useless, in your future design, my suggestion is splitting instruction error out of this type and remove INST_R here.

src/main/scala/Decoder.scala

carlosedp · 2021-11-25T00:07:01Z

I tried it out but got the error:

[info] - should Decode an ADD instruction (type R) *** FAILED ***
[info]   java.util.NoSuchElementException: None.get
[info]   at ... ()
[info]   at chiselv.Decoder.$anonfun$signals$2(Decoder.scala:46)
[info]   at chisel3.internal.prefix$.apply(prefix.scala:49)
[info]   at chiselv.Decoder.$anonfun$signals$1(Decoder.scala:108)
[info]   at chisel3.internal.plugin.package$.autoNameRecursively(package.scala:33)
[info]   at chiselv.Decoder.<init>(Decoder.scala:38)
[info]   at chiselv.DecoderSpec.$anonfun$new$2(DecoderSpec.scala:15)
[info]   at ... ()
[info]   at ... (Stack trace trimmed to user code only. Rerun with --full-stacktrace to see the full stack trace)
[info]   ...

Any idea what cou be wrong?

sequencer · 2021-11-30T15:19:49Z

I tried it out but got the error:

[info] - should Decode an ADD instruction (type R) *** FAILED ***
[info]   java.util.NoSuchElementException: None.get
[info]   at ... ()
[info]   at chiselv.Decoder.$anonfun$signals$2(Decoder.scala:46)
[info]   at chisel3.internal.prefix$.apply(prefix.scala:49)
[info]   at chiselv.Decoder.$anonfun$signals$1(Decoder.scala:108)
[info]   at chisel3.internal.plugin.package$.autoNameRecursively(package.scala:33)
[info]   at chiselv.Decoder.<init>(Decoder.scala:38)
[info]   at chiselv.DecoderSpec.$anonfun$new$2(DecoderSpec.scala:15)
[info]   at ... ()
[info]   at ... (Stack trace trimmed to user code only. Rerun with --full-stacktrace to see the full stack trace)
[info]   ...

Any idea what cou be wrong?

This is a strange bug:
chipsalliance/chisel#2268
I will try to fix this in Chisel

carlosedp · 2021-12-28T19:34:32Z

Hey @sequencer ... just a reminder to catch-up on this whenever we can :)
Would be awesome then to have it as a recipe in the Chisel cookbook :)

sequencer · 2022-01-03T17:10:57Z

Sorry for the delay again!
I did some fix ups, but seems there is a bug in your decode table which was figured by Decoder.

carlosedp · 2022-01-03T19:47:26Z

Hey Jiuyang, I've rebased and fixed my decode pattern but now I'm getting:

[info] - should Decode an ADD instruction (type R) *** FAILED ***
[info]   chisel3.internal.ChiselException: Connection between sink (Decoder.io.DecoderPort.inst: IO[Instruction]) and source (Decoder.signals.inst: Wire[UInt<6>]) failed @: Sink (Instruction) and Source (UInt<6>) have different types.
[info]   at ... ()
[info]   at chiselv.Decoder.<init>(Decoder.scala:127)
[info]   at chiselv.DecoderSpec.$anonfun$new$2(DecoderSpec.scala:15)
[info]   at ... ()
[info]   at ... (Stack trace trimmed to user code only. Rerun with --full-stacktrace to see the full stack trace)
[info]   ...

I expect the Instruction type in the output but currently it's a UInt.

Also I'm seeing lots of debug messages looking for "espresso" in my path.

sequencer · 2022-01-03T19:51:26Z

I expect the Instruction type in the output but currently it's a UInt.

Just cast it(I know it's dirty, we should have a better name in the future)

Also I'm seeing lots of debug messages looking for "espresso" in my path.

It's available from https://github.com/chipsalliance/espresso

…atch timing by removing useless Mux.

carlosedp · 2022-01-03T22:02:26Z

Thanks Jiuyang! It worked perfectly :D

I've cast like: io.DecoderPort.inst := signals.inst.asTypeOf(new Instruction.Type)

But I'm getting:

[warn] Decoder.scala:127: Casting non-literal UInt to chiselv.Instruction. You can use chiselv.Instruction.safe to cast without this warning. in class chiselv.Decoder

What's the right way to do without the warning?

Other than that... I think we need some nicer way to avoid all casts and etc... :) It's a kinda complex and hard to remember pattern :)

carlosedp · 2022-01-03T22:42:21Z

This is pretty neat, as an example I synthesized for ECP5 FPGA:

Without decoder (switch/is and MuxLookup)

Info: Device utilisation:
Info: 	       TRELLIS_SLICE:  3939/41820     9%
Info: 	          TRELLIS_IO:    13/  365     3%
Info: Max frequency for clock '$glbnet$SOC_clock': 16.02 MHz (PASS at 9.00 MHz)

With decoder/mux1H:

Info: 	       TRELLIS_SLICE:  3684/41820     8%
Info: 	          TRELLIS_IO:    13/  365     3%
Info: Max frequency for clock '$glbnet$SOC_clock': 19.43 MHz (PASS at 9.00 MHz)

Saved about 300 LEs and gained about 3.5Mhz!

sequencer · 2022-01-04T05:25:33Z

Would you mind benchmark QMC vs espresso. I'm also curious to this.
I thought PPA of QMC should be better, while slower than espresso.
But for this case, I think it will as fast as espresso.

carlosedp · 2022-01-04T13:48:17Z

You mean by having espresso or not right?
In terms of execution time for my core, the difference is not noticeable... all runs around 32-38 seconds... there is a difference tho in synthesis... without using the espresso compiler I get:

Info: Device utilisation:
Info: 	       TRELLIS_SLICE:  3762/41820     8%
Info: 	          TRELLIS_IO:    13/  365     3%

Info: Max frequency for clock '$glbnet$SOC_clock': 20.34 MHz (PASS at 9.00 MHz)

A bit more LEs but faster clock.

sequencer commented Nov 24, 2021

View reviewed changes

sequencer mentioned this pull request Nov 24, 2021

Remove deprecation note from ListLookup chipsalliance/chisel#2258

Closed

14 tasks

sequencer force-pushed the dec branch from 38f45ca to 5be2cdb Compare January 3, 2022 17:11

sequencer and others added 5 commits January 3, 2022 18:30

switch to decoder, recode InstructionType uop to OneHot, improve disp…

0281c8f

…atch timing by removing useless Mux.

bump

2d77a83

Update libraries, improve buildfiles

5aa9a8e

bump

3f7d84e

Rebase and fix decode pattern

58e9569

Fix type with cast and some test cases

9789ee4

carlosedp force-pushed the dec branch from 2e94563 to 9789ee4 Compare January 3, 2022 22:17

Improve formatting

753378b

carlosedp mentioned this pull request Jan 4, 2022

Deprecate ListLookup. chipsalliance/chisel#2259

Open

14 tasks

Improve API using some unmerged PRs

d676295

		0.U, // padding for InstructionType
		0.U // padding for InstructionType

Uh oh!

Conversation

sequencer commented Nov 24, 2021

Uh oh!

sequencer Nov 24, 2021

Choose a reason for hiding this comment

Uh oh!

carlosedp Jan 4, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sequencer Nov 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

carlosedp commented Nov 25, 2021

Uh oh!

sequencer commented Nov 30, 2021

Uh oh!

carlosedp commented Dec 28, 2021

Uh oh!

sequencer commented Jan 3, 2022

Uh oh!

carlosedp commented Jan 3, 2022

Uh oh!

sequencer commented Jan 3, 2022

Uh oh!

carlosedp commented Jan 3, 2022

Uh oh!

carlosedp commented Jan 3, 2022

Uh oh!

sequencer commented Jan 4, 2022

Uh oh!

carlosedp commented Jan 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sequencer Nov 24, 2021 •

edited

Loading