Minres / CoreDSL

Xtext project to parse CoreDSL files
Apache License 2.0
14 stars 2 forks source link

Intrinsic sign_extend / zero_extend / truncate functions #64

Open AtomCrafty opened 1 year ago

AtomCrafty commented 1 year ago

I'm currently looking through the RISCV reference files and I'm already spotting potential for improvement. The very first instruction I looked at (LUI) does a sign extension by first casting imm to a signed type and then to an unsigned type of the correct width. According to the casting rules, there are actually three casts, as one happens implicitly: (unsigned<XLEN>)(signed<XLEN>)(signed<20>)imm.

It works, but in my opinion it's not very intuitive since a cast doesn't clearly communicate which kind of extension occurs. Luckily I built a very flexible system for intrinsic functions, so we can provide a sign_extend(width, expr) function to make the intent more explicit.

What do you think, is this a direction worth exploring?

AtomCrafty commented 1 year ago

To exand on this further: These intrinsics could have additional validation logic checking that the extended type is actually larger than the source type. If XLEN were smaller than the width of imm, the existing code would just silently fail, while the intrinsic function could instead notify the implementor that it can't sign or zero extend to a smaller type than it started with. The exact opposite goes for a possible truncate function that would raise an error if the truncated length is larger than the input value.

AtomCrafty commented 1 year ago

Alright, quick update: I'm currently rewriting the RV32I ISA to give you a demonstration of what the language would look like with all of my new proposals applied. I must say that these functions are invaluable for the readability of the code. So here's a proper proposal instead of the rough idea above.

5 new functions:

All of the width parameters must be constant expressions. The extend functions will report an error if the requested width is smaller than that of the input value, the truncate functions if it is larger.

The only annoyance is the constantly repeating XLEN parameter (which is also present with the old casts). As a remedy I would suggest a [[default_truncate_width]] attribute that can be applied to the XLEN parameter declaration. If that is the case, the first parameter to truncate can be omitted and the annotated parameter will be used instead.

eyck commented 1 year ago

I'm strictly against the introduction of intrinsics for several reasons:

jopperm commented 1 year ago

Thoughts:

jopperm commented 1 year ago

PS: The C-style cast syntax is not great. Personally, I'm annoyed that we often need multiple layers of parentheses.

((T<n>) (expr))

Maybe we should've gone for

T<n>(expr)

instead.

AtomCrafty commented 1 year ago

Are implicit casts still allowed in your proposal?

Yes, none of the existing semantics would be changed, there would just be additional functions to make the operations more explicit and provide additional static validation. These would be entirely optional, so as an implementor you could choose to simply not use them. Ideally, these constructs would be treated as syntactic sugar and eliminated in the fontend, so backends would just see a regular cast (or rather, an even more generic bit swizzle).

AtomCrafty commented 1 year ago

Also, wouldn't you have to prohibit extensions of bit vectors, in favor of concatenation with a constant?

Zero extension is a type-agnostic operation, so as described in the proposal, it can work on bit vectors. Sign extension on the other hand only makes sense on signed values and hence treats its operand as such. Whether we force the users to make this explicit by casting the operand to (signed), or whether we accept any bit vector and cast it internally is something we'd still need to decide on. The proposal as it stands only accepts signed values for sign_extend.