search

chipsalliance / chisel

Chisel: A Modern Hardware Design Language
https://www.chisel-lang.org/
Apache License 2.0
3.98k stars 597 forks source link

Subfield assignment error message unhelpful #2251

Open michael-etzkorn opened 2 years ago

michael-etzkorn commented 2 years ago

Type of issue: other enhancement

Impact: no functional change

Development Phase: request

Other information

What is the current behavior?

Currently, attempting to assign a subfield like so: 

cmdq_wrdata(127,0) := rxIO.rx_wrc_rd_data

results in a pretty confusing exception chisel3.internal.ChiselException: Cannot reassign to read-only UInt<128>.

What is the expected behavior?

Something that explains that subfield assignment isn't allowed and suggests using vectors bundles (even though I'm of the belief Chisel should support something so natural to hardware engineers as bit assignment).

Please tell us about your environment: version: 3.4.1 What is the use case for changing the behavior? Give better guidance on subfield assignment. A majority of people starting from SystemVerilog who look at https://github.com/freechipsproject/chisel-cheatsheet are going to try extracting the bitfield and then assigning to that bitfield. While I believe this should be legal in Chisel, if it's not going to be, it should at least be clear to the user as to what's going on.

Since https://github.com/chipsalliance/chisel3/issues/878 is closed and fwiw: I'm planning to use blackboxes / inline verilog to get around the issue of not being able to directly do subfield assignment.

sequencer commented 2 years ago

Maybe you wanna take a look at the dataview api by @jackkoenig. IIRC, it should resolve this issue elegantly.

michael-etzkorn commented 2 years ago

Maybe you wanna take a look at the dataview api by @jackkoenig. IIRC, it should resolve this issue elegantly.

I'm taking a deeper look into Dataview. I'm not quite sure on what the Dataview solution would look like (something with viewing Uints as BigInts?).

However, there is a simpler, albeit unnatural, workaround for Reg. Build the field with the necessary subfield replaced on the right-hand side. 

val cmdq_wrdata  = Reg(UInt(256.W))
cmdq_wrdata  := cmdq_wrdata(255, 128) ## rxIO.rx_wrc_rd_data

This will not work for wire types however due to combinational loops and perhaps DataView and viewing UInt as BigInt can provide a solution in that case.