[Calyx Emitter] Avoid emitting redundant `write_en = 0` to satisfy `write_together` for reads

[Abhilekhgautam]

Calyx specifies that, for a seq_memory_d1 if the port write_en is driven then write_data must also be driven through the write_together spec.

However, when the port content_en is 1 and port write_en is 0, it implies a read, so write_data need not be driven.

Since write_together is a safety mechanism can we just tell the emitter to not drive write_en when it is 0 and content_en is 1.

For example:

Instead of emitting this (which papercut pass complains about):

 group bb0_1 {
      std_slice_10.in = for_2_induction_var_reg.out;
      arg_mem_0.addr0 = std_slice_10.out;
      arg_mem_0.content_en = 1'b1;
      arg_mem_0.write_en = 1'd0; <- unnecessary
      bb0_1[done] = arg_mem_0.done;
 }

we emit:

 group bb0_1 {
      std_slice_10.in = for_2_induction_var_reg.out;
      arg_mem_0.addr0 = std_slice_10.out;
      arg_mem_0.content_en = 1'b1;
      bb0_1[done] = arg_mem_0.done;
 }

Since, by default the value will be set to 0, these two will be equivalent and will satisfy the write_together spec as well.

[Abhilekhgautam]

@cgyurgyik any thoughts on this?

[cgyurgyik]

How do we know the value is set to 0 when not driven? Is this specified somewhere? Sorry, I haven't looked at HDL code in a while.

[Abhilekhgautam]

This discussion mentions:

Calyx, like every language, has some problems with undefined values. To summarize the status quo:

• The compiler inserts a bunch of "if not otherwise defined, then the value is zero" assignments into the RTL.
• The interpreter uses zero pervasively for things that are not assigned.

However, they also mention:

After nailing down some of these questions, we probably have some hacking to do:

• Remove those "default to zero" assignments in the compiler.
• Update the interpreter. The environment would need to make all values "optional," i.e., they can either be a bitstring or a distinct "undefined" value. Then it would need to follow the propagation rules above. And finally, it would want to catch errors according the policies mentioned above.

Not sure, if it is implemented or not.

[cgyurgyik]

@rachitnigam or @sampsyo are likely again more up-to-date on these things

[rachitnigam]

This seems fine to me! Let's just make sure to test against the mainline compiler to make sure it doesn't break existing code.