[RISC-V][JIT] Fix branch

[clamp03]

• Fix errors in branches. An example what I found is Console.WriteLine("{0}", 0);.
• Part of CoreCLR RISC-V architecture port #84834

[ghost]

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

Issue Details

---

null

Author:	clamp03
Assignees:	-
Labels:	area-CodeGen-coreclr, community-contribution
Milestone:	-

[alpencolt]

@clamp03 I don't understand, jal is J-type instruction and has signed 20-bit immediate, branches are B-type and have signed 12-bit immediate, why have you changed to 21 and 13?

[clamp03]

@clamp03 I don't understand, jal is J-type instruction and has signed 20-bit immediate, branches are B-type and have signed 12-bit immediate, why have you changed to 21 and 13?

JAL has signed 20-bit offset size (offset[20:1]) and branches have signed 12-bits offset size (offset[12:1]) in instruction encoding.
However, those instructions do not have 0th bit. So actual immediate values is checked with signed 21-bits and signed 13-bits (0th bit ignored).

https://five-embeddev.com/riscv-isa-manual/latest/rv32.html#programmers-model-for-base-integer-isa
Unconditional Jumps
The jump and link (JAL) instruction uses the J-type format, where the J-immediate encodes a signed offset in multiples of 2 bytes. The offset is sign-extended and added to the address of the jump instruction to form the jump target address. Jumps can therefore target a ±1 MiB range. JAL stores the address of the instruction following the jump (pc+4) into register rd. The standard software calling convention uses x1 as the return address register and x5 as an alternate link register.

https://stackoverflow.com/questions/59150608/offset-address-for-jal-and-jalr-instrctions-in-risc-v

[alpencolt]

Hm, interesting, so formulas there isn't correct:
https://jemu.oscc.cc/JAL
https://msyksphinz-self.github.io/riscv-isadoc/html/rvi.html#jal

[clamp03]

@jakobbotsch Could you please review this patch? I fixed some errors related to branches.
Recently, I worked for RISC-V for months.Can I have an authorization to put arch-riscv label to PR or ISSUE?

[clamp03]

Hm, interesting, so formulas there isn't correct: https://jemu.oscc.cc/JAL https://msyksphinz-self.github.io/riscv-isadoc/html/rvi.html#jal

https://jemu.oscc.cc/JAL is wrong.
However, I think https://msyksphinz-self.github.io/riscv-isadoc/html/rvi.html#jal is right. In the encoding, you can find 0th bit is omitted. The offset value is made with [20:1] values in encoding like below. Bits of offset value is 21 (encoded 20 bits and not assigned 0th bit). I mean offset itself in encoding is 20 bits, however offset value range is 21 bits (0th is 0).

https://github.com/dotnet/runtime/blob/b066a16f3af015be3f457430aeddfd7f24514c0a/src/coreclr/jit/emitriscv64.cpp#L3171C15-L3172

[alpencolt]

In the encoding, you can find 0th bit is omitted.

Yep, it's copied from specification.

[jakobbotsch]

@jakobbotsch Could you please review this patch? I fixed some errors related to branches. Recently, I worked for RISC-V for months.Can I have an authorization to put arch-riscv label to PR or ISSUE?

Thanks @clamp03. Since you have been contributing for many years now (on the ARM32 port too) we decided to grant you triaging rights so that you set the RISC-V work related area/architecture/OS labels.

[clamp03]

@jakobbotsch Could you please review this patch? I fixed some errors related to branches. Recently, I worked for RISC-V for months.Can I have an authorization to put arch-riscv label to PR or ISSUE?

Thanks @clamp03. Since you have been contributing for many years now (on the ARM32 port too) we decided to grant you triaging rights so that you set the RISC-V work related area/architecture/OS labels.

Thank you so much!

[alpencolt]

I fixed the dest reg in jalr and checked the generated codes on ./JIT/jit64/opt/cse/HugeField2/HugeField2.sh test (It is a single test that converts jal to jalr).

I've checked it on debug build, but it still hang.
But any way this PR could be merged since it fix other tests which print messages infinetly to console:

JIT/jit64/opt/cse/staticFieldExprUnchecked1_d_loop_try/staticFieldExprUnchecked1_d_loop_try.sh
JIT/jit64/opt/cse/staticFieldExprUnchecked1_r_loop/staticFieldExprUnchecked1_r_loop.sh
JIT/jit64/opt/cse/staticFieldExprUnchecked1_r_loop_try/staticFieldExprUnchecked1_r_loop_try.sh

[alpencolt]

I fixed the dest reg in jalr and checked the generated codes on ./JIT/jit64/opt/cse/HugeField2/HugeField2.sh test (It is a single test that converts jal to jalr).

I've checked it on debug build, but it still hang.
But any way this PR could be merged since it fix other tests which print messages infinetly to console:

JIT/jit64/opt/cse/staticFieldExprUnchecked1_d_loop_try/staticFieldExprUnchecked1_d_loop_try.sh
JIT/jit64/opt/cse/staticFieldExprUnchecked1_r_loop/staticFieldExprUnchecked1_r_loop.sh
JIT/jit64/opt/cse/staticFieldExprUnchecked1_r_loop_try/staticFieldExprUnchecked1_r_loop_try.sh

[clamp03]

@alpencolt Thank you for the test. I tested HugeField2.sh on CHECKED build. It may have another issue in DEBUG build. We can handle HugeField2.sh's hang in the next PR.
Thank you.

[broudy3]

I've checked it on debug build, but it still hang.

@alpencolt please try the test JIT/jit64/opt/cse/HugeField2/HugeField2.sh with my small patch #86870. For me the test is then passing in DEBUG build.

[alpencolt]

I've checked it on debug build, but it still hang.

@alpencolt please try the test JIT/jit64/opt/cse/HugeField2/HugeField2.sh with my small patch #86870. For me the test is then passing in DEBUG build.

Hm, it still hangs for me

[broudy3]

Hm, it still hangs for me

Are you sure it hangs? Or does it take long time to run? On my test setup this test takes almost 13 minutes to run.

[alpencolt]

Ups, sorry, it's really long test. It's pass with #86482. Haven't checked without it but with #86870, but thank you for information.

[clamp03]

@jakobbotsch Could you review and merge? Thank you.