[RISC-V] Remove unused stack probing methods #112347

sirntar · 2025-02-10T09:20:00Z

This PR removes unneeded stack probing methods on riscv64. It was a leftover from la64, which was a legacy from arm64... On rv64 we are doing that a little bit different.

About genAllocLclFrame:
On riscv64 It was used to probe stack or let's say it was supposed to be used for that. In fact it was yet another legacy code from arm64 that was never used.

Part of #84834, cc @dotnet/samsung

src/coreclr/jit/codegenriscv64.cpp

src/coreclr/jit/emitriscv64.cpp

src/coreclr/jit/codegenriscv64.cpp

risc-vv · 2025-02-10T12:34:59Z

RISC-V Release-CLR-QEMU: 9456 / 9535 (99.17%)

=======================
      passed: 9456
      failed: 63
     skipped: 106
      killed: 16
------------------------
  TOTAL libs: 9641
 TOTAL tests: 9641
   REAL time: 1h 14min 47s 812ms
=======================

Release-CLR-QEMU.md, Release-CLR-QEMU.xml, testclr_output.tar.gz

RISC-V Release-FX-QEMU: 0 / 259 (0.00%)

=======================
      passed: 0
      failed: 0
     skipped: 0
      killed: 259
------------------------
  TOTAL libs: 259
 TOTAL tests: 259
   REAL time: 44s 19ms
=======================

Release-FX-QEMU.md, Release-FX-QEMU.xml, testfx_output.tar.gz

Build information and commands

GIT: e520193422652e14766e6c754dc254a94645dda7
CI: 237f25e8d63b12225c6b9d141d0bd2f89ff136ba
REPO: dotnet/runtime
BRANCH: main
CONFIG: Release
LIB_CONFIG: Release

RISC-V Release-CLR-QEMU: 9463 / 9535 (99.24%)

=======================
      passed: 9463
      failed: 57
     skipped: 106
      killed: 15
------------------------
  TOTAL libs: 9641
 TOTAL tests: 9641
   REAL time: 58min 33s 487ms
=======================

Release-CLR-QEMU.md, Release-CLR-QEMU.xml, testclr_output.tar.gz

RISC-V Release-FX-QEMU: 0 / 259 (0.00%)

=======================
      passed: 0
      failed: 0
     skipped: 0
      killed: 259
------------------------
  TOTAL libs: 259
 TOTAL tests: 259
   REAL time: 44s 19ms
=======================

Release-FX-QEMU.md, Release-FX-QEMU.xml, testfx_output.tar.gz

Build information and commands

GIT: e520193422652e14766e6c754dc254a94645dda7
CI: 237f25e8d63b12225c6b9d141d0bd2f89ff136ba
REPO: dotnet/runtime
BRANCH: main
CONFIG: Release
LIB_CONFIG: Release

risc-vv · 2025-02-10T14:21:00Z

RISC-V Release-CLR-QEMU: 9463 / 9535 (99.24%)

=======================
      passed: 9463
      failed: 57
     skipped: 106
      killed: 15
------------------------
  TOTAL libs: 9641
 TOTAL tests: 9641
   REAL time: 58min 33s 487ms
=======================

Release-CLR-QEMU.md, Release-CLR-QEMU.xml, testclr_output.tar.gz

RISC-V Release-FX-QEMU: 0 / 259 (0.00%)

=======================
      passed: 0
      failed: 0
     skipped: 0
      killed: 259
------------------------
  TOTAL libs: 259
 TOTAL tests: 259
   REAL time: 42s 980ms
=======================

Release-FX-QEMU.md, Release-FX-QEMU.xml, testfx_output.tar.gz

Build information and commands

GIT: 3a43c852005863bb2b2f1d742eb7192a701e4791
CI: 237f25e8d63b12225c6b9d141d0bd2f89ff136ba
REPO: dotnet/runtime
BRANCH: main
CONFIG: Release
LIB_CONFIG: Release

RISC-V Release-CLR-QEMU: 9463 / 9535 (99.24%)

=======================
      passed: 9463
      failed: 57
     skipped: 106
      killed: 15
------------------------
  TOTAL libs: 9641
 TOTAL tests: 9641
   REAL time: 58min 32s 265ms
=======================

Release-CLR-QEMU.md, Release-CLR-QEMU.xml, testclr_output.tar.gz

RISC-V Release-FX-QEMU: 0 / 259 (0.00%)

=======================
      passed: 0
      failed: 0
     skipped: 0
      killed: 259
------------------------
  TOTAL libs: 259
 TOTAL tests: 259
   REAL time: 42s 980ms
=======================

Release-FX-QEMU.md, Release-FX-QEMU.xml, testfx_output.tar.gz

Build information and commands

GIT: 3a43c852005863bb2b2f1d742eb7192a701e4791
CI: 237f25e8d63b12225c6b9d141d0bd2f89ff136ba
REPO: dotnet/runtime
BRANCH: main
CONFIG: Release
LIB_CONFIG: Release

src/coreclr/jit/emitriscv64.cpp

risc-vv · 2025-02-11T08:46:48Z

RISC-V Release-CLR-QEMU: 9463 / 9535 (99.24%)

=======================
      passed: 9463
      failed: 57
     skipped: 106
      killed: 15
------------------------
  TOTAL libs: 9641
 TOTAL tests: 9641
   REAL time: 58min 32s 265ms
=======================

Release-CLR-QEMU.md, Release-CLR-QEMU.xml, testclr_output.tar.gz

RISC-V Release-FX-QEMU: 0 / 259 (0.00%)

=======================
      passed: 0
      failed: 0
     skipped: 0
      killed: 259
------------------------
  TOTAL libs: 259
 TOTAL tests: 259
   REAL time: 43s 18ms
=======================

Release-FX-QEMU.md, Release-FX-QEMU.xml, testfx_output.tar.gz

Build information and commands

GIT: 9de229bc31eeea98a62158cc0957dd13501fab05
CI: 237f25e8d63b12225c6b9d141d0bd2f89ff136ba
REPO: dotnet/runtime
BRANCH: main
CONFIG: Release
LIB_CONFIG: Release

risc-vv · 2025-02-12T08:36:37Z

f8034ce is being scheduled for building and testing

GIT: f8034ce118aa9ed887e1fe55bd2ee29d3262641c
REPO: dotnet/runtime
BRANCH: main

Release-FX-tests FAILED

buildinfo.json
coreFX tests failed for unknown reason. Check buildinfo.json or gocd for more details.

risc-vv · 2025-02-13T07:40:58Z

RISC-V Release-CLR-QEMU: 9458 / 9534 (99.20%)

=======================
      passed: 9458
      failed: 59
     skipped: 106
      killed: 17
------------------------
  TOTAL libs: 9640
 TOTAL tests: 9640
   REAL time: 2h 45min 16s 299ms
=======================

Release-CLR-QEMU.md, Release-CLR-QEMU.xml, testclr_output.tar.gz

Build information and commands

GIT: 32efb371fe96e1673f5ea3cd309e0c44a0ac0a97
CI: e5cdc6f6d9bf8eeaf0f6ef0b9c3436ded5aaa6fe
REPO: dotnet/runtime
BRANCH: main
CONFIG: Release
LIB_CONFIG: Release

RISC-V Release-FX-QEMU: 580657 / 612697 (94.77%)

=======================
      passed: 580657
      failed: 303
     skipped: 1452
      killed: 31737
------------------------
  TOTAL libs: 258
 TOTAL tests: 614149
   REAL time: 2h 23min 7s 554ms
=======================

Release-FX-QEMU.md, Release-FX-QEMU.xml, testfx_output.tar.gz

Build information and commands

GIT: 32efb371fe96e1673f5ea3cd309e0c44a0ac0a97
CI: e5cdc6f6d9bf8eeaf0f6ef0b9c3436ded5aaa6fe
REPO: dotnet/runtime
BRANCH: main
CONFIG: Release
LIB_CONFIG: Release

RISC-V Release-CLR-VF2: 9458 / 9534 (99.20%)

=======================
      passed: 9458
      failed: 59
     skipped: 106
      killed: 17
------------------------
  TOTAL libs: 9640
 TOTAL tests: 9640
   REAL time: 2h 2min 37s 244ms
=======================

Release-CLR-VF2.md, Release-CLR-VF2.xml, testclr_output.tar.gz

Build information and commands

GIT: 32efb371fe96e1673f5ea3cd309e0c44a0ac0a97
CI: e5cdc6f6d9bf8eeaf0f6ef0b9c3436ded5aaa6fe
REPO: dotnet/runtime
BRANCH: main
CONFIG: Release
LIB_CONFIG: Release

RISC-V Release-FX-VF2: 389221 / 423609 (91.88%)

=======================
      passed: 389221
      failed: 140
     skipped: 1479
      killed: 34248
------------------------
  TOTAL libs: 258
 TOTAL tests: 425088
   REAL time: 2h 45min 56s 994ms
=======================

Release-FX-VF2.md, Release-FX-VF2.xml, testfx_output.tar.gz

Build information and commands

GIT: 32efb371fe96e1673f5ea3cd309e0c44a0ac0a97
CI: e5cdc6f6d9bf8eeaf0f6ef0b9c3436ded5aaa6fe
REPO: dotnet/runtime
BRANCH: main
CONFIG: Release
LIB_CONFIG: Release

tomeksowi · 2025-02-17T07:13:41Z

src/coreclr/jit/codegenriscv64.cpp

@@ -1750,9 +1746,6 @@ void CodeGen::genLclHeap(GenTree* tree)

        // genDefineTempLabel(loop);

-        // tickle the page - Read from the updated SP - this triggers a page fault when on the guard page
-        emit->emitIns_R_R_I(INS_lw, EA_4BYTE, REG_R0, REG_SPBASE, 0);


Just to make sure it's not doing anything shady, what's preventing the sp from jumping over the guard page now?

risc-vv · 2025-02-19T15:26:09Z

8709e9d is being scheduled for building and testing

GIT: 8709e9d68c439aa1d3048768dcb2d3b372cb1702
REPO: dotnet/runtime
BRANCH: main

sirntar · 2025-02-19T15:43:15Z

@tomeksowi

Just to make sure it's not doing anything shady, what's preventing the sp from jumping over the guard page now?

That was a pretty good question - I thought this mechanism was somewhere else, but it turned out that it wasn't. So instead of removing stack probing I simplified it. I also removed genStackProbe and I disabled genAllocLclFrame, since both methods aren't used on riscv64.

I don't want to create more mess in this PR, so I will close it and open a new one.

tomeksowi · 2025-02-19T16:12:29Z

src/coreclr/jit/codegenriscv64.cpp

@@ -1719,23 +1717,20 @@ void CodeGen::genLclHeap(GenTree* tree)
        //       addi     regCnt, REG_R0, 0
        //
        //  Skip:
-        //       lui      regTmp, eeGetPageSize()>>12
+        //       lui      regPageSize, eeGetPageSize()>>12
+        //       addi     regTmp, SP, 0


Is there a reason not to decrement sp directly?

sirntar added 4 commits February 6, 2025 10:19

remove redundant code

b10c38d

StackOverflow test

ef0892c

remove StackOverflow (for rv64) test

971be9b

remove commented code

b98abe6

dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 10, 2025

dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Feb 10, 2025

apply format patch

e520193

clamp03 assigned sirntar Feb 10, 2025

clamp03 added the arch-riscv Related to the RISC-V architecture label Feb 10, 2025

tomeksowi reviewed Feb 10, 2025

View reviewed changes

src/coreclr/jit/codegenriscv64.cpp Outdated Show resolved Hide resolved

src/coreclr/jit/emitriscv64.cpp Outdated Show resolved Hide resolved

src/coreclr/jit/codegenriscv64.cpp Outdated Show resolved Hide resolved

reverse RH region mark and null check

3a43c85

tomeksowi reviewed Feb 10, 2025

View reviewed changes

src/coreclr/jit/emitriscv64.cpp Outdated Show resolved Hide resolved

This was referenced Feb 10, 2025

slow macOS - "##[error]The job running on agent Azure Pipelines 9 ran longer than the maximum time of 60 minutes." dotnet/dnceng#1883

Open

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

revert emitJumpKindInstructions

9de229b

build-analysis bot mentioned this pull request Feb 11, 2025

System.Numerics.Tensors.Tests.ConvertTests.ConvertChecked failing with System.OverflowException #112286

Closed

Merge branch 'main' into remove-stack-probe

f8034ce

Merge branch 'main' into remove-stack-probe

32efb37

tomeksowi approved these changes Feb 14, 2025

View reviewed changes

tomeksowi reviewed Feb 17, 2025

View reviewed changes

sirntar added 2 commits February 19, 2025 14:40

simplify stack probing

8b0271e

Merge branch 'main' into remove-stack-probe

8709e9d

sirntar closed this Feb 19, 2025

tomeksowi reviewed Feb 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RISC-V] Remove unused stack probing methods #112347

[RISC-V] Remove unused stack probing methods #112347

sirntar commented Feb 10, 2025

risc-vv commented Feb 10, 2025 •

edited

Loading

risc-vv commented Feb 10, 2025 •

edited

Loading

risc-vv commented Feb 11, 2025 •

edited

Loading

risc-vv commented Feb 12, 2025 •

edited

Loading

risc-vv commented Feb 13, 2025 •

edited

Loading

tomeksowi Feb 17, 2025

risc-vv commented Feb 19, 2025

sirntar commented Feb 19, 2025

tomeksowi Feb 19, 2025

[RISC-V] Remove unused stack probing methods #112347

[RISC-V] Remove unused stack probing methods #112347

Conversation

sirntar commented Feb 10, 2025

risc-vv commented Feb 10, 2025 • edited Loading

risc-vv commented Feb 10, 2025 • edited Loading

risc-vv commented Feb 11, 2025 • edited Loading

risc-vv commented Feb 12, 2025 • edited Loading

risc-vv commented Feb 13, 2025 • edited Loading

tomeksowi Feb 17, 2025

Choose a reason for hiding this comment

risc-vv commented Feb 19, 2025

sirntar commented Feb 19, 2025

tomeksowi Feb 19, 2025

Choose a reason for hiding this comment

risc-vv commented Feb 10, 2025 •

edited

Loading

risc-vv commented Feb 10, 2025 •

edited

Loading

risc-vv commented Feb 11, 2025 •

edited

Loading

risc-vv commented Feb 12, 2025 •

edited

Loading

risc-vv commented Feb 13, 2025 •

edited

Loading