Fixing translation for sext/zext with sliced value

[jsn1993]

This PR is to handle the special case that sign-extending a single sliced variable is incorrectly translated, by incorporating the slice value which previously the code wasn't aware of.

Now this is working: [pymtl] sext ( a[31:32], 20 ) --> [verilog] { { 20-1 {a[31]} }, a[31:31] }.

Previously the slice [x:y] is a part of sig_name, "a[x:y]", and when you sext it, every variable has two layers of indices (such as a[x:y][z]). Also the sext amount is wrong because the length of a sliced value A[x:y] is usually shorter than A itself. For example. the previous translation of sext( a[31:32], 20 ) will look like { { 20-32 { a[(32)-1:31][31] } }, a[(32)-1:31][31:0] } which is absolutely wrong.

I fixed this by basically extract one layer deeper. The code first checks if the first arg of a sext/zext call in the AST is a Subscript, which means that the argument has a name followed by a [a:b] slice. Then I extracted slice_lower and slice_upper and reformat the string. If it's not Subscript, the code executes as before.

I also plan to add some test cases for zext/sext.

[cbatten]

Great! Did you add unit tests for this?

[jsn1993]

I mentioned that adding tests is my plan. I wasn't able to find a place to add the test at the beginning. I think originally there are some test files that include this kind of tests, but there are just too many different files ...

After talking to Khalid I found that this is actually too hacky? He explained to me that the multidimensional bit selection also happened for nested bitstructs and that looks the same as the problem I'm facing.

Basically we need a way to squash nested slice selections into one selection -- a[0:32][16:32][0:8] --> a[16:24] . I think I might have to enhance the ast visit function for Subscript. And after doing that I don't need to directly hack on zext/sext function.

[cbatten]

I am excited about exploring a more source-to-source translation route where we directly translate bitstructs into systemverilog structs. We could also have a little "PyMTL SystemVerilog lib" with say a function called sext that matches what the PyMTL sext does ... I think this might be more robust ...

[hawajkm]

I am not sure if going the SystemVerilog is necessary for this. Is SystemVerilog considered to be the de facto standard for RTL?

In reality, according to my experience, every tool supports a subset of SystemVerilog and some times even of Verilog itself. To be more robust and portable, we might want to do the structure management in PyMTL to avoid niche subsets of SystemVerilog/Verilog. In-essence, I do not see anything that prevent us from doing this. Technically, we can flat any structure we have and then perform a single dimension slicing. This could be resolved as a recursive call that keeps resetting the boundaries for the subscript until we reach the final dimension and we set the correct lower/upper limit.

[cbatten]

SV is gaining acceptance. The problem is there are a ton of corner cases and such once we start mixing interfaces, bit structs, port bundles, nested port bundles, nested bit structs.

[hawajkm]

I agree with the corner cases. But, I think this is easily manageable by avoiding being sucked-in by Python flexibility.

We can create a hierarchy that can manage the cases and support the outlined hierarchy only.

We can have a port bundle that can contain only ports. Ports can be either of type Bits or type BitStruct. BitStruct can have either Bits children or BitStruct children; and BitStruct can be recursive to eventually get to Bits.

+ PortBundle
|- Port
|--- Bits
|--- Bits
|      .
|      .
|
|- Port
|--- BitStruct
|------ Bits
|------ BitStruct
|--------- Bits
|---------  .
|---------  .
|--- Bits
|
|- Port
|--- Bits
|--- Bits

I don't think there is a reason why the above generic example cannot be flattened and the boundaries of each member, no matter how deeply nested, can be determined at synthesis time.

EDIT: Maybe nested PortBundle is fine, too ... :o)

[dmlockhart]

A few recommendations:

we need a way to squash nested slice selections into one selection -- a[0:32][16:32][0:8] --> a[16:24]

I strongly recommend the above approach. I think an issue with the current implementation of PyMTL translation is that I didn't do enough "lowering". That is, Verilog generation should be done from a very simplistic intermediate representation (IR/AST). AST-to-Verilog translation should essentially be an obvious one-to-one mapping, and if it's not then it hasn't been lowered enough. One of the lessons my compilers coworkers have repeatedly hammered into my head :).

I am excited about exploring a more source-to-source translation route where we directly translate bitstructs into systemverilog structs.

I do think it would be valuable to have this, but I would recommend it as a second backend. SV is gaining acceptance but there still very inconsistent support for SystemVerilog constructs, so it would be really valuable to have an escape hatch to pure Verilog. We still find bugs in the "industry standard" parser that many of the major tool vendors use.

It would actually be really cool if you could do a "mixed" mode translation: if only one particular module is having problems translating, annotating it to be Verilog translation (maybe with a SystemVerilog wrapper) and then translating everything else into SystemVerilog.

Also note that switching to SystemVerilog won't really save you from the need to do good lowering... it just means the types of nodes you have in your IR are different.

[cbatten]

RE: IR -- I have actually given this quite a bit of thought. I really like the IR idea as opposed to a source-to-source approach because hopefully it would make translation more robust -- i.e., a much wider range of Python syntax would be translatable.

One problem is really about control flow. If you lower the AST into a simple SSA form with basic blocks it will be hard to map it into Verilog because Verilog does not have gotos .. plus if you lower from Python to an IR and then from the IR to verilog you will likely loose the readability of the Verilog. I actually spent some time trying to figure out how we could use the Numba IR as an IR for PyMTL concurrent blocks but the big stumbling block was like I said above with control flow and loops.

Something we are still thinking about though.

And I definitely share your concern with a system verilog backend -- if we do the IR approach then systemverilog is less necessary. If we do a source-to-source translation it is nice if the input and output source languages are as close as possible which is why I wanted to map bitstructs to SV structs.

[dmlockhart]

Yeah, unfortunately there is no such thing as a general-purpose IR [1]. I think this is pretty widely known in the compiler community, but I never picked up on it until recently.

Your points are correct, and the solution is to not try to map into an IR for software languages and instead create an IR suited specialized for representing hardware/targeting Verilog. This is basically what the FIRTTL work from Berkeley is attempting to do, but I would suggest rolling your own at this point until FIRTTL is fleshed out a bit more.

[1] Even LLVM, which is considered a fairly "general" compiler framework, only works well for C-like languages. Languages that deviate from a C-like model either use a different IR or leverage a higher-level representation that is acts as a bridge to LLVM. Christian Lattner, creator of LLVM, eventually adopted this approach for Swift and gave a good presentation that discussions why it was needed: https://news.ycombinator.com/item?id=10485405