[RISC-V] Fix jump stubs and emit direct calls PC-relative

src/coreclr/jit/codegenriscv64.cpp

@@ -5540,20 +5540,21 @@ void CodeGen::genCallInstruction(GenTreeCall* call)
 
         if (target->isContainedIntOrIImmed())
         {
+            // A direct call: auipc + jalr with a relocation. No register is needed for the target, and ireg is
+            // left unset because the emitter chooses the auipc and jalr registers itself.
             params.callType = EC_FUNC_TOKEN;
-            params.ireg     = internalRegisters.GetSingle(call);
             params.addr     = (void*)target->AsIntCon()->IconValue();
         }
         else
         {
             params.callType = EC_INDIR_R;
             params.ireg     = target->GetRegNum();
-        }
 
-        // We have already generated code for gtControlExpr evaluating it into a register.
-        // We just need to emit "call reg" in this case.
-        //
-        assert(genIsValidIntReg(params.ireg));
+            // We have already generated code for gtControlExpr evaluating it into a register.
+            // We just need to emit "call reg" in this case.
+            //
+            assert(genIsValidIntReg(params.ireg));
+        }
 
         genEmitCallWithCurrentGC(params);
     }

src/coreclr/jit/emitriscv64.cpp

@@ -1901,7 +1901,7 @@ void emitter::emitIns_Call(const EmitCallParams& params)
     /* Sanity check the arguments depending on callType */
 
     assert(params.callType < EC_COUNT);
-    assert(isGeneralRegister(params.ireg));
+    assert((params.callType == EC_FUNC_TOKEN) || isGeneralRegister(params.ireg));
     assert(params.callType < EC_INDIR_R || params.addr == nullptr);
 
     // RISCV64 never uses these
@@ -1931,19 +1931,6 @@ void emitter::emitIns_Call(const EmitCallParams& params)
     }
 #endif
 
-    ssize_t jalrOffset = 0;
-    if (params.callType == EC_FUNC_TOKEN && !IsAddressInRange(params.addr))
-    {
-        // Load upper bits of the absolute call address into a register:
-        // li   ireg, addr_upper
-        // jalr zero/ra, ireg, addr_lo12 (emitted below)
-        assert(params.addr != nullptr);
-        ssize_t imm = (ssize_t)params.addr;
-        jalrOffset  = (imm << (64 - 12)) >> (64 - 12); // low 12-bits, sign-extended
-        imm -= jalrOffset;
-        emitLoadImmediate<true>(EA_PTRSIZE, params.ireg, imm); // upper bits
-    }
-
     /*
         We need to allocate the appropriate instruction descriptor based
         on whether this is a direct/indirect call, and whether we need to
@@ -2009,18 +1996,18 @@ void emitter::emitIns_Call(const EmitCallParams& params)
     id->idInsOpt(INS_OPTS_C);
 
     /* Record the address: method, indirection, or funcptr */
-    if ((params.callType == EC_INDIR_R) || (params.callType == EC_FUNC_TOKEN && !IsAddressInRange(params.addr)))
+    if (params.callType == EC_INDIR_R)
     {
         /* This is an indirect call (either a virtual call or func ptr call) */
 
-        //      jalr zero/ra, ireg, offset
+        //      jalr zero/ra, ireg, 0
 
         id->idSetIsCallRegPtr();
 
         regNumber reg_jalr = params.isJump ? REG_R0 : REG_RA;
         id->idReg4(reg_jalr);
         id->idReg3(params.ireg); // NOTE: for EC_INDIR_R, using idReg3.
-        id->idSmallCns(jalrOffset);
+        id->idSmallCns(0);
         id->idCodeSize(4);
     }
     else
@@ -2029,10 +2016,12 @@ void emitter::emitIns_Call(const EmitCallParams& params)
 
         //      auipc t2/ra, offset-hi20
         //      jalr zero/ra, t2/ra, offset-lo12
+        //
+        // Always PC-relative, with a relocation for the VM to resolve: when the target is out of reach
+        // of the auipc + jalr pair the VM inserts a jump stub, as it does for arm64 and amd64 calls.
 
         assert(params.callType == EC_FUNC_TOKEN);
         assert(params.addr != nullptr);
-        assert(IsAddressInRange(params.addr));
 
         void* addr =
             (void*)(((size_t)params.addr) + (params.isJump ? 0 : 1)); // NOTE: low-bit0 is used for jalr ra/r0,rd,0

src/coreclr/jit/lower.cpp

@@ -2948,8 +2948,8 @@ GenTree* Lowering::LowerCall(GenTree* node)
         call->gtControlExpr = controlExpr;
 
 #ifdef TARGET_RISCV64
-        // If controlExpr is a constant, we should contain it inside the call so that we can move the lower 12-bits of
-        // the value to call instruction's (JALR) offset.
+        // If controlExpr is a constant, contain it inside the call so that codegen emits a direct call to it
+        // (auipc + jalr with a relocation) instead of materializing the address in a register.
         if (controlExpr->IsCnsIntOrI() && !controlExpr->AsIntCon()->ImmedValNeedsReloc(m_compiler) &&
             !call->IsFastTailCall())
         {

src/coreclr/jit/lsrariscv64.cpp

@@ -947,12 +947,6 @@ int LinearScan::BuildCall(GenTreeCall* call)
             }
             assert(ctrlExprCandidates != RBM_NONE);
         }
-
-        // In case ctrlExpr is a contained constant, we need a register to store the value.
-        if (ctrlExpr->isContainedIntOrIImmed())
-        {
-            buildInternalIntRegisterDefForNode(call);
-        }
     }
     else if (call->IsR2ROrVirtualStubRelativeIndir())
     {

src/coreclr/vm/jitinterface.cpp

@@ -12546,18 +12546,23 @@ void CEEJitInfo::recordRelocation(void *       location,
             // Target is out of 32-bit range
             // If we're fixing up a jump, try jumping through a stub
             PCODE jumpStubAddr = (PCODE)NULL;
-            if ((fRelocType == CorInfoReloc::RISCV64_CALL_PLT) && !m_fJumpStubOverflow)
+            if (fRelocType == CorInfoReloc::RISCV64_CALL_PLT)
             {
-                BYTE* loAddr = (BYTE*)location - (1ll << 31) - (1ll << 11);
-                BYTE* hiAddr = (BYTE*)location + (1ll << 31) - (1ll << 11) - 1;
+                // Once an overflow has been recorded the code is thrown away and recompiled, so do not allocate
+                // more jump stubs; keep counting them so that the retry reserves enough space.
+                if (!m_fJumpStubOverflow)
+                {
+                    BYTE* loAddr = (BYTE*)location - (1ll << 31) - (1ll << 11);
+                    BYTE* hiAddr = (BYTE*)location + (1ll << 31) - (1ll << 11) - 1;
 
-                // Check for the wrap around cases
-                if (loAddr > location)
-                    loAddr = (BYTE*)UINT64_MIN; // overflow
-                if (hiAddr < location)
-                    hiAddr = (BYTE*)UINT64_MAX; // overflow
+                    // Check for the wrap around cases
+                    if (loAddr > location)
+                        loAddr = (BYTE*)UINT64_MIN; // overflow
+                    if (hiAddr < location)
+                        hiAddr = (BYTE*)UINT64_MAX; // overflow
 
-                PCODE jumpStubAddr = ExecutionManager::jumpStub(m_pMethodBeingCompiled, (PCODE)target, loAddr, hiAddr, nullptr);
+                    jumpStubAddr = ExecutionManager::jumpStub(m_pMethodBeingCompiled, (PCODE)target, loAddr, hiAddr, nullptr);
+                }
 
                 // Keep track of conservative estimate of how much memory may be needed by jump stubs. We will use it
                 // to reserve extra memory on retry to increase chances that the retry succeeds.

src/coreclr/vm/riscv64/asmhelpers.S

@@ -682,7 +682,7 @@ NESTED_ENTRY \helper\()Naked, _TEXT, NoHandler
     // On entry:
     //   t0 = functionIDOrClientID
     //   t1 = profiledSp
-    //   t6 = throwable
+    //   (t6 is scratch: it is overwritten below before any read, so a jump stub may use it)
     //
     // On exit:
     //   Values of a0-a7, fa0-fa7, fp are preserved.

src/coreclr/vm/riscv64/cgencpu.h

@@ -287,14 +287,22 @@ inline void emitBackToBackJump(LPBYTE pBufferRX, LPBYTE pBufferRW, LPVOID target
     // We require 8-byte alignment so the LD instruction is aligned properly
     _ASSERTE(((UINT_PTR)pCode & 7) == 0);
 
-    // auipc ra, 0
-    // ld    ra, ra, 16
-    // jalr  x0, ra, 0
+    // The stub sits between a call and its target, so it may only use a register that
+    // neither side expects to survive the call. The link register is not one: a call
+    // arrives here with ra holding the return address, and a tail jump with ra holding
+    // the caller's return address. Of the temporaries, t0/t1 carry the profiler enter
+    // arguments, t2 the secret stub parameter and the dispatch token, t3/t4 the write
+    // barrier arguments and t5 the indirection cell. t6 (REG_OPT_RSVD in the JIT) carries
+    // nothing across a call, and the write barrier already declares it trashed.
+
+    // auipc t6, 0
+    // ld    t6, 16(t6)
+    // jalr  x0, t6, 0
     // nop    //padding.
 
-    pCode[0] = 0x00000097; // auipc ra, 0
-    pCode[1] = 0x0100b083; // ld    ra, 16(ra)
-    pCode[2] = 0x00008067; // jalr  x0, ra, 0
+    pCode[0] = 0x00000f97; // auipc t6, 0
+    pCode[1] = 0x010fbf83; // ld    t6, 16(t6)
+    pCode[2] = 0x000f8067; // jalr  x0, t6, 0
     pCode[3] = 0x00000013; // padding nop.
 
     // Ensure that the updated instructions get updated in the I-Cache

src/tests/baseservices/jumpstubs/ForceRelocsJumpStubs.cs

@@ -0,0 +1,58 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+// Runs four call shapes with DOTNET_ForceRelocs=1. On RISC-V, where W^X is off by default, Checked and Debug
+// runtimes then reserve executable memory at scattered addresses, which can put helper and cross-heap targets
+// out of auipc + jalr range, so that those calls go through jump stubs.
+
+using System;
+using System.Runtime.CompilerServices;
+using Xunit;
+
+public static class ForceRelocsJumpStubs
+{
+    private sealed class Holder
+    {
+        public object Field;
+    }
+
+    [Fact]
+    public static void TestEntryPoint()
+    {
+        var holder = new Holder();
+        Assert.Same(StoreNew(holder), holder.Field);
+        Assert.Equal("jumpstub", Concat("jump", "stub"));
+        Assert.Equal(42, CatchFromThrower());
+        Assert.Equal(7, Add(3, 4));
+    }
+
+    // Allocation and write-barrier helpers.
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static object StoreNew(Holder holder) => holder.Field = new object();
+
+    // A call into CoreLib.
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static string Concat(string a, string b) => string.Concat(a, b);
+
+    // The throw helper, and an unwind back through the caller.
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static int CatchFromThrower()
+    {
+        try
+        {
+            Thrower();
+            return -1;
+        }
+        catch (InvalidOperationException)
+        {
+            return 42;
+        }
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static void Thrower() => throw new InvalidOperationException();
+
+    // A plain managed call with a return value.
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static int Add(int a, int b) => a + b;
+}

src/tests/baseservices/jumpstubs/ForceRelocsJumpStubs.csproj

@@ -0,0 +1,17 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <!-- Needed for CLRTestEnvironmentVariable -->
+    <RequiresProcessIsolation>true</RequiresProcessIsolation>
+    <Optimize>true</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="ForceRelocsJumpStubs.cs" />
+
+    <!-- Read by Checked and Debug runtimes only. On RISC-V, where W^X is off by default, it also scatters
+         executable memory, which can put helper and cross-heap targets out of range so that those calls go
+         through jump stubs; elsewhere this is an ordinary ForceRelocs run. -->
+    <CLRTestEnvironmentVariable Include="DOTNET_ForceRelocs" Value="1" />
+    <!-- Optimized code from the start, so that the call shapes under test are the optimized ones. -->
+    <CLRTestEnvironmentVariable Include="DOTNET_TieredCompilation" Value="0" />
+  </ItemGroup>
+</Project>