Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RISCV] Set RegState for the stack-clash prologue registers #119451

Closed
wants to merge 1 commit into from

Conversation

rzinsly
Copy link
Contributor

@rzinsly rzinsly commented Dec 10, 2024

Set both prologue scratch registers as InternalRead for stack-clash protection to fix the tests with LLVM_ENABLE_EXPENSIVE_CHECKS.

See buildbot failure: https://lab.llvm.org/buildbot/#/builders/16/builds/10433

Set both prologue scratch registers as InternalRead for stack-clash
protection to fix the tests with LLVM_ENABLE_EXPENSIVE_CHECKS.

See buildbot failure: https://lab.llvm.org/buildbot/#/builders/16/builds/10433
@llvmbot
Copy link
Member

llvmbot commented Dec 10, 2024

@llvm/pr-subscribers-backend-risc-v

Author: Raphael Moreira Zinsly (rzinsly)

Changes

Set both prologue scratch registers as InternalRead for stack-clash protection to fix the tests with LLVM_ENABLE_EXPENSIVE_CHECKS.

See buildbot failure: https://lab.llvm.org/buildbot/#/builders/16/builds/10433


Full diff: https://github.com/llvm/llvm-project/pull/119451.diff

1 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVFrameLowering.cpp (+2-2)
diff --git a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
index 655de0b4e7eb5d..e7a477a6165606 100644
--- a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
@@ -2043,7 +2043,7 @@ static void emitStackProbeInline(MachineFunction &MF, MachineBasicBlock &MBB,
   //   SUB SP, SP, ProbeSize
   BuildMI(*LoopTestMBB, LoopTestMBB->end(), DL, TII->get(RISCV::SUB), SPReg)
       .addReg(SPReg)
-      .addReg(ScratchReg)
+      .addReg(ScratchReg, RegState::InternalRead)
       .setMIFlags(Flags);
 
   //   s[d|w] zero, 0(sp)
@@ -2057,7 +2057,7 @@ static void emitStackProbeInline(MachineFunction &MF, MachineBasicBlock &MBB,
   //   BNE SP, TargetReg, LoopTest
   BuildMI(*LoopTestMBB, LoopTestMBB->end(), DL, TII->get(RISCV::BNE))
       .addReg(SPReg)
-      .addReg(TargetReg)
+      .addReg(TargetReg, RegState::InternalRead)
       .addMBB(LoopTestMBB)
       .setMIFlags(Flags);
 

@topperc
Copy link
Collaborator

topperc commented Dec 10, 2024

This is no the correct fix. This is

diff --git a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
index 655de0b4e7eb..1028149bf513 100644
--- a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
@@ -14,6 +14,7 @@
 #include "RISCVMachineFunctionInfo.h"
 #include "RISCVSubtarget.h"
 #include "llvm/BinaryFormat/Dwarf.h"
+#include "llvm/CodeGen/LivePhysRegs.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
@@ -2066,6 +2067,8 @@ static void emitStackProbeInline(MachineFunction &MF, MachineBasicBlock &MBB,
   LoopTestMBB->addSuccessor(ExitMBB);
   LoopTestMBB->addSuccessor(LoopTestMBB);
   MBB.addSuccessor(LoopTestMBB);
+  // Update liveins.
+  fullyRecomputeLiveIns({ExitMBB, LoopTestMBB});
 }

@rzinsly
Copy link
Contributor Author

rzinsly commented Dec 10, 2024

@topperc Ok, thanks for working on this, I'm closing this PR.

@rzinsly rzinsly closed this Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants