Missing variants of umul/smul/ushl/sshl/ushr/sshr returning if it overflows

[bjorn3]

[71]

Does anyone plan on taking care of this any time soon? I'd like to get my hands dirty in the codegen, but it definitely will take me some time to familiarize myself with everything first.

[bjorn3]

I don't plan to do this myself.

[sunfishcode]

@6a Sounds like you can take this one :-).

We'll need both smul and umul, and sshl and ushl, since the overflow is different depending on the signedness interpretation. (Cranelift's convention is that 'i' means integer where the signedness doesn't matter, and 's' and 'u' are for when a particular signedness is needed.)

iadd_cout will be a good example to follow for adding instructions to the IR. When it gets down to actually implementing these in machine code there will be some differences, but we can figure out what's needed when we get there.

[froydnj]

I looked into this and umul_cout looks reasonable to add. I'm not sure why I would need to add things to semantics.py, but I'm going to assume that those bits are necessary (even though they're implemented for iadd_cout and friends, but not for isub_bout and similar...)?

But what I'm not sure is where to add the appropriate bits to the backend. I had hoped that x86 would already have cargo-cultable examples for iadd_cout and similar, but it looks like those aren't even implemented. If I'm reading things right, I think legalization takes care of translating iadd_cout down to more primitive bits of IR. I'm assuming this issue was for implementing the instructions in the IR with appropriate legalizations, but adding "efficient" variants that use CPU flags would be a separate (and possibly larger?) effort. Am I missing something?

[bjorn3]

Thinking about it again, we can use umulhi/smulhi not being equal to zero as overflow indicator for imul. Also if the shift amount for a shift is equal to or bigger than the integer bitsize, that would be an overflow.

A dedicated umul_cout instruction would probably be faster though.

[jameysharp]

Can we close this now that #5784 is merged, or do folks still want overflow-checking variants for shifts?

[cfallin]

It might be useful to leave open until we do; @T0b1-iOS if you're interested in pushing this further, I'd be happy to review overflow-checking shifts as well! (Of course this is only if you want to: we can just as well leave this for someone else to pick up later!)

[jameysharp]

After I wrote that question, I got thinking that it's not clear to me there's as much value in dedicated CLIF instructions for overflow-checked shifts. With addition and multiplication, there's often hardware support for detecting overflow as a side effect of the computation. But with shifts I don't think we can generate code that's any better than you'd get from explicitly writing something like icmp uge shift_amt, 64 for the overflow output of an I64 shift, right?