Address feedback

Co-authored-by: Tomek Sowiński <tomeksowi@gmail.com>

src/coreclr/nativeaot/Runtime/MiscHelpers.cpp

@@ -375,35 +375,40 @@ FCIMPL1(uint8_t *, RhGetCodeTarget, uint8_t * pCodeOrg)
 #elif TARGET_RISCV64
     uint32_t * pCode = (uint32_t *)pCodeOrg;
     // is this "addi a0, a0, 8"?
-    if (pCode[0] == 0x000200b3)  // Encoding for `addi a0, a0, 8` in 32-bit RISC-V
+    if (pCode[0] == 0x00850513)  // Encoding for `addi a0, a0, 8` in 32-bit instruction format
     {
         // unboxing sequence
         unboxingStub = true;
         pCode++;
     }
     // is this an indirect jump?
     // lui t0, imm; jalr t0, t0, imm12
-    if ((pCode[0] & 0x000ff000) == 0x00020000 &&
-        (pCode[1] & 0x000ff000) == 0x000000a0 &&
-        (pCode[2] & 0x0000000f) == 0x00000000)
+    if ((pCode[0] & 0x7f) == 0x17 &&                 // auipc
+        (pCode[1] & 0x707f) == 0x3003 &&             // ld with funct3=011
+        (pCode[2] & 0x707f) == 0x0067)               // jr (jalr with x0 as rd and funct3=000)
     {
-        // normal import stub - dist to IAT cell is relative to (PC & ~0xfff)
-        // lui: imm = SignExtend(imm20:Zeros(12), 64);
-        int64_t distToIatCell = ((((int64_t)pCode[0] & ~0xfff) << 12) >> 32);
-        // jalr: offset = SignExtend(imm12, 64);
-        distToIatCell += (int64_t)(pCode[1] & 0xfff);
+        // Compute the distance to the IAT cell
+        int64_t distToIatCell = (((int64_t)pCode[0] & 0xfffff000) >> 12);  // Extract imm20 from auipc
+        distToIatCell += ((int64_t)(pCode[1] & 0xfff));                    // Add imm12 from ld
+
         uint8_t ** pIatCell = (uint8_t **)(((int64_t)pCode & ~0xfff) + distToIatCell);
         return *pIatCell;
     }
-    // is this an unboxing stub followed by a relative jump?
-    // jal ra, imm
-    else if (unboxingStub && (pCode[0] & 0xffe00000) == 0x00000000 &&
-             (pCode[1] & 0x0000ffff) == 0x00000000)
+
+    // Is this an unboxing stub followed by a relative jump?
+    // auipc t0, imm20; jalr ra, imm12(t0)
+    else if (unboxingStub &&
+            (pCode[0] & 0x7f) == 0x17 &&                 // auipc opcode
+            (pCode[1] & 0x707f) == 0x0067)              // jalr opcode with funct3=000
     {
-        // relative jump - dist is relative to the instruction
-        // offset = SignExtend(imm20:Zeros(12), 64);
-        int64_t distToTarget = ((int64_t)(pCode[1] & 0xffff) << 12) >> 12;
-        return (uint8_t *)pCode + distToTarget;
+        // Extract imm20 from auipc
+        int64_t distToTarget = (((int64_t)pCode[0] & 0xfffff000) >> 12);  // Extract imm20 (bits 31:12)
+
+        // Extract imm12 from jalr
+        distToTarget += ((int64_t)pCode[1] & 0xfff);  // Extract imm12 (bits 31:20)
+
+        // Calculate the final target address relative to PC
+        return (uint8_t *)((int64_t)pCode + distToTarget);
     }
 
 #else

src/coreclr/nativeaot/Runtime/PalRedhawk.h

@@ -583,13 +583,11 @@ typedef struct DECLSPEC_ALIGN(16) _CONTEXT {
     uint64_t T5;
     uint64_t T6;
     uint64_t Pc;
-    uint64_t T61;
-    uint64_t Pc1;
 
     //
     // Floating Point Registers
     //
-    uint64_t F[4*32];
+    uint64_t F[32];
     uint32_t Fcsr;
 
     void SetIp(uintptr_t ip) { Pc = ip; }

src/coreclr/nativeaot/Runtime/StackFrameIterator.cpp

@@ -1462,7 +1462,7 @@ struct UniversalTransitionStackFrame
 private:
     uintptr_t m_pushedFP;                  // ChildSP+000     CallerSP-0F0 (0x08 bytes)    (fp)
     uintptr_t m_pushedRA;                  // ChildSP+008     CallerSP-0E8 (0x08 bytes)    (ra)
-    Fp128   m_fpArgRegs[8];                // ChildSP+010     CallerSP-0E0 (0x80 bytes)    (fa0-fa7)
+    Fp64      m_fpArgRegs[8];              // ChildSP+010     CallerSP-0E0 (0x80 bytes)    (fa0-fa7)
     uintptr_t m_returnBlock[4];            // ChildSP+090     CallerSP-060 (0x20 bytes)
     uintptr_t m_intArgRegs[8];             // ChildSP+0B0     CallerSP-040 (0x40 bytes)    (a0-a7)
     uintptr_t m_stackPassedArgs[1];        // ChildSP+0F0     CallerSP+000 (unknown size)
@@ -1484,7 +1484,7 @@ struct UniversalTransitionStackFrame
 private:
     uintptr_t m_pushedRA;                  // ChildSP+000     CallerSP-0F0 (0x08 bytes)    (ra)
     uintptr_t m_pushedFP;                  // ChildSP+008     CallerSP-0E8 (0x08 bytes)    (fp)
-    Fp128   m_fpArgRegs[8];                // ChildSP+010     CallerSP-0E0 (0x80 bytes)    (fa0-fa7)
+    Fp128     m_fpArgRegs[8];              // ChildSP+010     CallerSP-0E0 (0x80 bytes)    (fa0-fa7)
     uintptr_t m_returnBlock[4];            // ChildSP+090     CallerSP-060 (0x20 bytes)
     uintptr_t m_intArgRegs[8];             // ChildSP+0B0     CallerSP-040 (0x40 bytes)    (a0-a7)
     uintptr_t m_stackPassedArgs[1];        // ChildSP+0F0     CallerSP+000 (unknown size)

src/coreclr/nativeaot/Runtime/ThunksMapping.cpp

@@ -22,8 +22,10 @@
 #define THUNK_SIZE  20
 #elif TARGET_ARM64
 #define THUNK_SIZE  16
-#elif TARGET_LOONGARCH64 || TARGET_RISCV64
+#elif TARGET_LOONGARCH64
 #define THUNK_SIZE  16
+#elif TARGET_RISCV64
+#define THUNK_SIZE  12
 #else
 #define THUNK_SIZE  (2 * OS_PAGE_SIZE) // This will cause RhpGetNumThunksPerBlock to return 0
 #endif
@@ -257,10 +259,9 @@ EXTERN_C void* QCALLTYPE RhAllocateThunksMapping()
 
 #elif defined(TARGET_RISCV64)
 
-            // lui      t0, %hi(delta)         // Load upper immediate with address high bits
-            // addi     t0, t0, %lo(delta)     // Add lower immediate
-            // ld       t1, 0(t0)              // Load data from address in t0
-            // jalr     x0, t1, 0              // Jump and link register (set PC to t1)
+            // auipc  t0, %hi(delta)      // Load upper immediate with address high bits
+            // ld     t1, %lo(delta)(t0)  // Load data from address in (t0 + lower immediate)
+            // jr     t1                  // Jump and don't link register
 
             int delta = (int)(pCurrentDataAddress - pCurrentThunkAddress);
             *((uint32_t*)pCurrentThunkAddress) = 0x0002A013 | (((delta & 0x3FFFFC) >> 2) << 12);  // lui + addi

src/coreclr/nativeaot/Runtime/riscv64/AllocFast.S

@@ -115,7 +115,7 @@ LOCAL_LABEL(NewOutOfMemory):
 
         // Compute overall allocation size (align(base size + (element size * elements), 8)).
         li    a3, STRING_COMPONENT_SIZE                 // Load STRING_COMPONENT_SIZE into a3
-        mul   a2, a1, a3                                // a2 = a1 * STRING_COMPONENT_SIZE
+        slli  a2, a1, 1                                 // a2 = a1 * STRING_COMPONENT_SIZE, where STRING_COMPONENT_SIZE == 2
         addi  a2, a2, STRING_BASE_SIZE + 7              // a2 = a2 + STRING_BASE_SIZE + 7
         andi  a2, a2, ~0x7                              // Clear the bits[2:0] of a2 (align to 8 bytes)
 

src/coreclr/nativeaot/Runtime/riscv64/WriteBarriers.S

@@ -65,8 +65,7 @@
 
         // Someone went and updated the real heap. We need to invalidate INVALIDGCVALUE the shadow location since we cannot
         // guarantee whose shadow update won.
-        lui  t3, (INVALIDGCVALUE >> 12) & 0xFFFFF
-        ori  t3, t3, INVALIDGCVALUE & 0xFFF
+        li   t3, INVALIDGCVALUE
         sd   t3, 0(\destReg)
 
 0:

src/coreclr/nativeaot/Runtime/unix/UnixNativeCodeManager.cpp

@@ -1227,7 +1227,7 @@ int UnixNativeCodeManager::TrailingEpilogueInstructionsCount(MethodInfo * pMetho
 // BEQ, BNE, JAL, etc.
 // 1100 0000 0000 0000 0000 0000 0000 0000
 #define BEGS_BITS 0x00000063
-#define BEGS_MASK 0x7F000000
+#define BEGS_MASK 0x0000007F
 
     UnixNativeMethodInfo * pNativeMethodInfo = (UnixNativeMethodInfo *)pMethodInfo;
     ASSERT(pNativeMethodInfo != NULL);
@@ -1245,20 +1245,20 @@ int UnixNativeCodeManager::TrailingEpilogueInstructionsCount(MethodInfo * pMetho
     {
         uint32_t instr = *pInstr;
 
-        // check for Branches, Jumps, System calls.
-        // If we see such instruction before seeing registers restored, we are not in an epilog.
-        // Note: this includes RET, branches, jumps, system calls, etc...
+        // Check for branches, jumps, or system calls.
+        // If we see such instructions before registers are restored, we are not in an epilogue.
+        // Note: this includes RET, branches, jumps, and system calls.
         if ((instr & BEGS_MASK) == BEGS_BITS)
         {
             // not in an epilogue
             break;
         }
 
-        // check for restoring registers with LD or LUI
-        int rd = (instr >> 7) & 0x1F;
-        if (rd == 2 || rd == 1)  // example register numbers for FP or RA
+        // Check for restoring registers (FP or RA) with `ld`
+        int rd = (instr >> 7) & 0x1F;  // Extract the destination register
+        if (rd == 8 || rd == 1)  // Check for FP (x8) or RA (x1)
         {
-            if ((instr & LD_MASK) == LD_BITS || (instr & LUI_MASK) == LUI_BITS)
+            if ((instr & LD_MASK) == LD_BITS)  // Match `ld` instruction
             {
                 return -1;
             }

src/coreclr/nativeaot/Runtime/unix/UnwindHelpers.cpp

@@ -1134,10 +1134,7 @@ inline bool Registers_REGDISPLAY::validRegister(int num) const {
 
 bool Registers_REGDISPLAY::validVectorRegister(int num) const
 {
-    // Vector registers are mapped to floating-point registers F24 to F31
-    if (num >= UNW_RISCV_F24 && num <= UNW_RISCV_F31)
-        return true;
-
+    // Vector registers currently unsupported
     return false;
 }
 
@@ -1370,34 +1367,12 @@ void Registers_REGDISPLAY::setRegister(int regNum, uint64_t value, uint64_t loca
 
 libunwind::v128 Registers_REGDISPLAY::getVectorRegister(int num) const
 {
-    num -= UNW_RISCV_F24; // Adjust the base to 0
-
-    if (num < 0 || num >= sizeof(F) / sizeof(uint64_t))
-    {
-        PORTABILITY_ASSERT("unsupported riscv64 vector register");
-    }
-
-    libunwind::v128 result;
-
-    // Assuming F array stores 64-bit parts of the vector data
-    result.vec[0] = 0;
-    result.vec[1] = 0;
-    result.vec[2] = F[num] >> 32;
-    result.vec[3] = F[num] & 0xFFFFFFFF;
-
-    return result;
+    PORTABILITY_ASSERT("Vector registers currently unsupported on RISC-V");
 }
 
 void Registers_REGDISPLAY::setVectorRegister(int num, libunwind::v128 value)
 {
-    num -= UNW_RISCV_F24; // Adjust the base to 0
-
-    if (num < 0 || num >= sizeof(F) / sizeof(uint64_t))
-    {
-        PORTABILITY_ASSERT("unsupported riscv64 vector register");
-    }
-
-    F[num] = (uint64_t)value.vec[2] << 32 | (uint64_t)value.vec[3];
+    PORTABILITY_ASSERT("Vector registers currently unsupported on RISC-V");
 }
 
 #endif // TARGET_RISCV64

src/coreclr/nativeaot/Runtime/unix/unixasmmacrosriscv64.inc

@@ -61,20 +61,23 @@ C_FUNC(\Name):
 .macro PROLOG_STACK_ALLOC Size
     // If Size is larger than 2047, split it into multiple instructions
     .if (\Size > 2047) || (\Size < -2048)
-        // Load the upper 20 bits into a temporary register (e.g., t0)
-        lui t0, %hi(\Size)
-        // Add the lower 12 bits to the temporary register
-        addi t0, t0, %lo(\Size)
-        // Subtract the value from the stack pointer
-        sub sp, sp, t0
+        li t0, -\Size
+        add sp, sp, t0
     .else
         addi sp, sp, -\Size
     .endif
 .endm
 
 .macro EPILOG_STACK_FREE Size
-    addi  sp, sp, \Size
-    .cfi_adjust_cfa_offset -\Size
+    // If Size is larger than 2047 or smaller than -2048, split into multiple instructions
+    .if (\Size > 2047) || (\Size < -2048)
+        li t0, \Size           // Load the large Size value into a temporary register
+        add sp, sp, t0         // Use the add instruction for full 64-bit addition
+        .cfi_adjust_cfa_offset -\Size
+    .else
+        addi sp, sp, \Size     // Handle small immediate directly with addi
+        .cfi_adjust_cfa_offset -\Size
+    .endif
 .endm
 
 .macro EPILOG_STACK_RESTORE