[RISC-V] Fix invalid operand register in the emitted addition/subtraction code

[Bajtazar]

Fixes invalid operand register in the emitted addition/subtraction code when both of the operand registers are same. Slightly improves quality of the generated code. Also introduces sext.w preudoinstruction to replace double-shift sign extension snippets

Examples of old and new code:

; Old long overflow check
     mv      a0, s1
     add     s1, s1, s1
     srli    ra, a0, 63
     srli    a1, s1, 63 ; s1 should be a0 which was the cause of the bug
     xor     ra, ra, a1
     bnez    ra, label_1
     bnez    a1, label_2
     bge     s1, a0, label_1
label_3:
     j       overflow
label_2:
     blt     a0, s1, label_3 
label_1:
     ; valid code

; New long overflow check
     mv      a0, s1
     add     s1, s1, s1
     slt     a1, s1, a0
     slti    a0, a0, 0
     bne     a1, a0, overflow
     ; valid code

; Old int overflow check
     mv      a0, s1
     addw    s1, s1, s1
     srli    ra, a0, 31
     srli    a1, s1, 31 ; same problem with s1 instead of a0
     xor     ra, ra, a1
     andi    ra, ra, 1
     andi    a1, a1, 1
     bnez    ra, label_1
     bnez    a1, label_2
     bge     s1, a0, label_1
label_3:
     j       overflow
label_2:
     blt     a0, s1, label_3 
label_1:
    ; valid code

; New int overflow check
     mv      a0, s1
     addw    s1, s1, s1
     add     a1, a0, a0
     bne     s1, a1, overflow
    ; valid code

Part of #84834, cc @dotnet/samsung

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

[tomeksowi]

; Old long overflow check
     mv      a0, s1
     add     s1, s1, s1
     srli    ra, a0, 63
     srli    a1, s1, 63 ; s1 should be a0 which was the cause of the bug
     xor     ra, ra, a1

If s1 would be a0 in this snippet, both ra and a1 would have the same value after right shifts, so the xor below would always return 0, no?

[Bajtazar]

; Old long overflow check
     mv      a0, s1
     add     s1, s1, s1
     srli    ra, a0, 63
     srli    a1, s1, 63 ; s1 should be a0 which was the cause of the bug
     xor     ra, ra, a1

If s1 would be a0 in this snippet, both ra and a1 would have the same value after right shifts, so the xor below would always return 0, no?

Yes, this second shift was supposed to calculate whether the original second operand was negative but since the logic responsible for preserving the original's operands value wasn't prepared for the case where both of the operand registers were same and thus allowing the bug to happen. After fixing it it also implied that ra would always be equal to zero rendering it and its branch useless in this case, so I've decided to reshape the emitter a little bit

[clamp03]

Have you discussed in your team to use pseudo code actively in JIT?
Actually, I want to remove all pseudo codes like mov in JITC because some pseudo codes (contains multiple normal instructions) cannot be used in JIT.

[tomeksowi]

IMO simple "renaming" pseudos like sext.w, bnez, mov, etc, are fine. We're already using them, they convey the intention a bit more clearly, and they make the assembly look closer to print-outs from native compilers.

I agree about multi-instruction pseudos, though.

[clamp03]

Okay. Actually, we used few pesudos in few code locations, however AFAIK it is not printed properly in dump. I just let those instruction because I hope removing later. If you want to use pseudos. Could you add such instructions like 'mov' and 'nop' in emitDispInsName too?
Thank you.

[Bajtazar]

I can add pseudos to the disassm but I would prefer to do it in other PR. Would you mind?

[clamp03]

Which tests can you fix by this PR?

[clamp03]

For subtraction, is it correct comment?
I think if B = A - C is right for sub, this comment should be update or if A = B - C is right for sub, then // if B > A then overflow in line 5037 should be update.

[Bajtazar]

IMO it is Ok, I've used the property of the overflows in RISCV64 that operations ignore overflow and wraps around 2^64. Due to this if A = B - C overflows then B = A + C does too. I've shuffled registers at the beginning according to the type of the operation so that at the end I just checks whether there is an addition overflow

[clamp03]

I was confused. Thank you for your explanation.

[Bajtazar]

Which tests can you fix by this PR?

It fixes JIT/jit64/rtchecks/overflow/overflow04_add/overflow04_add.sh and during testing it also seems to fix JIT/opt/virtualstubdispatch/bigvtbl/bigvtbl_cs_d/bigvtbl_cs_d.sh

[clamp03]

@jakobbotsch Could you review this PR? Thank you.

[risc-vv]

RISC-V testing failed on init-build

GIT: e464442