[JITLink] Fix the incorrect relocation behavior for R_RISCV_BRANCH and extractBits function

[StephenFan]

Summary

In D116573, the relocation behavior of R_RISCV_BRANCH didn't consider that branch instruction like 'bge' has a branch target address which is given as a PC-relative offset, sign-extend and multiplied by 2.
Although the target address is a 12-bits number, acctually its range is [-4096, 4094].

The previous function extractBits was incorrect due to it would extract one more bit each time.

This patch fix both of these problems.

[fourdim]

Hi, could you please update the code comments on R_RISCV_BRANCH in llvm/include/llvm/ExecutionEngine/JITLink/riscv.h
at the same time?

[StephenFan]

fix comments in riscv.h

[fourdim]

LGTM

[lhames]

Patch should be clang-format'd to eliminate the lint warnings, but otherwise LGTM. Thanks Stephen and Siwei!

[StephenFan]

clang-format and rebase

llvm/include/llvm/ExecutionEngine/JITLink/riscv.h

enum EdgeKind_riscv : Edge::Kind {
 
   /// A plain 64-bit pointer value relocation
   ///
   /// Fixup expression:
   ///   Fixup <- Target + Addend : uint32
   ///
   R_RISCV_64,
 
-  /// Low 12 bits of PC-relative branch pointer value relocation
+  /// PC-relative branch pointer value relocation
   ///
   /// Fixup expression:
-  ///   Fixup <- (Target - Fixup + Addend) & 0xFFF
+  ///   Fixup <- (Target - Fixup + Addend)
   ///
   R_RISCV_BRANCH,
 
   /// High 20 bits of 32-bit pointer value relocation
   ///
   /// Fixup expression
   ///   Fixup <- (Target + Addend + 0x800) >> 12
   R_RISCV_HI20,

llvm/lib/ExecutionEngine/JITLink/ELF_riscv.cpp

if (It->getKind() == R_RISCV_PCREL_HI20)
       return *It;
   }
 
   return make_error<JITLinkError>(
       "No HI20 PCREL relocation type be found for LO12 PCREL relocation type");
 }
 
 static uint32_t extractBits(uint32_t Num, unsigned Low, unsigned Size) {
-  return (Num & (((1ULL << (Size + 1)) - 1) << Low)) >> Low;
+  return (Num & (((1ULL << Size) - 1) << Low)) >> Low;
 }
 
 inline Error checkAlignment(llvm::orc::ExecutorAddr loc, uint64_t v, int n,
                             const Edge &E) {
   if (v & (n - 1))
     return make_error<JITLinkError>("0x" + llvm::utohexstr(loc.getValue()) +
                                     " improper alignment for relocation " +
                                     formatv("{0:d}", E.getKind()) + ": 0x" +
case R_RISCV_64: {
       break;
     }
     case R_RISCV_BRANCH: {
       int64_t Value = E.getTarget().getAddress() + E.getAddend() - FixupAddress;
       Error AlignmentIssue = checkAlignment(FixupAddress, Value, 2, E);
       if (AlignmentIssue) {
         return AlignmentIssue;
       }
-      int64_t Lo = Value & 0xFFF;
-      uint32_t Imm31_25 = extractBits(Lo, 5, 6) << 25 | extractBits(Lo, 12, 1)
-                                                            << 31;
-      uint32_t Imm11_7 = extractBits(Lo, 1, 4) << 8 | extractBits(Lo, 11, 1)
-                                                          << 7;
+      uint32_t Imm31_25 =
+          extractBits(Value, 5, 6) << 25 | extractBits(Value, 12, 1) << 31;
+      uint32_t Imm11_7 =
+          extractBits(Value, 1, 4) << 8 | extractBits(Value, 11, 1) << 7;
       uint32_t RawInstr = *(little32_t *)FixupPtr;
       *(little32_t *)FixupPtr = (RawInstr & 0x1FFF07F) | Imm31_25 | Imm11_7;
       break;
     }
     case R_RISCV_HI20: {
       int64_t Value = (E.getTarget().getAddress() + E.getAddend()).getValue();
       int64_t Hi = Value + 0x800;
       if (LLVM_UNLIKELY(!isInRangeForImmS32(Hi)))

llvm/test/ExecutionEngine/JITLink/RISCV/ELF_branch.s

 # RUN: rm -rf %t && mkdir -p %t
 # RUN: llvm-mc -triple=riscv64 -filetype=obj \
 # RUN:     -o %t/elf_riscv64_branch.o %s
 # RUN: llvm-mc -triple=riscv32 -filetype=obj \
 # RUN:     -o %t/elf_riscv32_branch.o %s
 # RUN: llvm-jitlink -noexec \
-# RUN:     -slab-allocate 100Kb -slab-address 0xfff00000 -slab-page-size 4096 \
-# RUN:     -abs external_func=0xfe \
+# RUN:     -slab-allocate 100Kb -slab-address 0xfff00ff4 -slab-page-size 4096 \
+# RUN:     -abs external_func_positive_offset=0xfff00ffc -abs external_func_negative_offset=0xfff00000\
 # RUN:     -check %s %t/elf_riscv64_branch.o
 # RUN: llvm-jitlink -noexec \
-# RUN:     -slab-allocate 100Kb -slab-address 0xfff00000 -slab-page-size 4096 \
-# RUN:     -abs external_func=0xfe \
+# RUN:     -slab-allocate 100Kb -slab-address 0xfff00ff4 -slab-page-size 4096 \
+# RUN:     -abs external_func_positive_offset=0xfff00ffc -abs external_func_negative_offset=0xfff00000 \
 # RUN:     -check %s %t/elf_riscv32_branch.o
 #
 
         .text
-        .file   "testcase.c"
-
 # Empty main entry point.
         .globl  main
         .p2align  1
         .type   main,@function
 main:
         ret
 
         .size   main, .-main
 
 # Test R_RISCV_BRANCH
 
-# jitlink-check: decode_operand(test_branch, 2)[11:0] = (external_func - test_branch)[11:0]
+# jitlink-check: decode_operand(test_branch, 2)[12:0] = (external_func_positive_offset - test_branch)[12:0]
+# jitlink-check: decode_operand(test_branch+4, 2)[12:0] = (external_func_negative_offset - test_branch - 4)[12:0]
   .globl  test_branch
   .p2align  1
   .type  test_branch,@function
 test_branch:
-  bge a0, a1, external_func
+  bge  a0, a1, external_func_positive_offset
+  bge  a0, a1, external_func_negative_offset
 
   .size test_branch, .-test_branch