[llvm][RISCV] Don't merge pseudo selects with stack adjustment instr in between

[enoskova-sc]

When we have sequence of select pseudo instruction with stack adjustment instr in between, we shouldn't apply the optimization, proposed here (https://reviews.llvm.org/D59355) for them, because in that case on Finalize ISel function won't be marked as adjustsStack.

[llvmbot]

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

Author: Elizaveta Noskova (enoskova-sc)

Changes

When we have sequence of select pseudo instruction with stack adjustment instr in between, we shouldn't apply the optimization, proposed here (https://reviews.llvm.org/D59355) for them, because in that case on Finalize ISel function won't be marked as adjustsStack.

---

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

2 Files Affected:

• (modified) llvm/lib/Target/RISCV/RISCVISelLowering.cpp (+6-2)
• (added) llvm/test/CodeGen/RISCV/select-pseudo-merge-with-stack-adj.ll (+20)

diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 8070a512ab078..7f7bf1243939a 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -22190,6 +22190,7 @@ static MachineBasicBlock *emitSelectPseudo(MachineInstr &MI,
   // - They are debug instructions. Otherwise,
   // - They do not have side-effects, do not access memory and their inputs do
   //   not depend on the results of the select pseudo-instructions.
+  // - They don't adjust stack.
   // The TrueV/FalseV operands of the selects cannot depend on the result of
   // previous selects in the sequence.
   // These conditions could be further relaxed. See the X86 target for a
@@ -22218,6 +22219,8 @@ static MachineBasicBlock *emitSelectPseudo(MachineInstr &MI,
   SelectDests.insert(MI.getOperand(0).getReg());
 
   MachineInstr *LastSelectPseudo = &MI;
+  const RISCVInstrInfo &TII = *Subtarget.getInstrInfo();
+
   for (auto E = BB->end(), SequenceMBBI = MachineBasicBlock::iterator(MI);
        SequenceMBBI != E; ++SequenceMBBI) {
     if (SequenceMBBI->isDebugInstr())
@@ -22237,7 +22240,9 @@ static MachineBasicBlock *emitSelectPseudo(MachineInstr &MI,
     }
     if (SequenceMBBI->hasUnmodeledSideEffects() ||
         SequenceMBBI->mayLoadOrStore() ||
-        SequenceMBBI->usesCustomInsertionHook())
+        SequenceMBBI->usesCustomInsertionHook() ||
+        TII.isFrameInstr(*SequenceMBBI) ||
+        SequenceMBBI->isStackAligningInlineAsm())
       break;
     if (llvm::any_of(SequenceMBBI->operands(), [&](MachineOperand &MO) {
           return MO.isReg() && MO.isUse() && SelectDests.count(MO.getReg());
@@ -22245,7 +22250,6 @@ static MachineBasicBlock *emitSelectPseudo(MachineInstr &MI,
       break;
   }
 
-  const RISCVInstrInfo &TII = *Subtarget.getInstrInfo();
   const BasicBlock *LLVM_BB = BB->getBasicBlock();
   DebugLoc DL = MI.getDebugLoc();
   MachineFunction::iterator I = ++BB->getIterator();
diff --git a/llvm/test/CodeGen/RISCV/select-pseudo-merge-with-stack-adj.ll b/llvm/test/CodeGen/RISCV/select-pseudo-merge-with-stack-adj.ll
new file mode 100644
index 0000000000000..9644bc167b9f9
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/select-pseudo-merge-with-stack-adj.ll
@@ -0,0 +1,20 @@
+; RUN: llc -mtriple riscv32 %s
+
+define i32 @test(i1 %arg_1, i32 %arg_2) {
+entry:
+  %sel_1 = select i1 %arg_1, i32 %arg_2, i32 1
+  %div = udiv i32 %arg_2, 7
+  %cond_1 = icmp ugt i32 %div, %sel_1
+  %sel_2 = select i1 %arg_1, i32 %div, i32 3
+  %sel = select i1 %arg_1, i32 %sel_1, i32 %sel_2
+  br label %body
+
+body:
+  %res = phi i32 [ %sel, %entry ], [ %add_loop, %body ]
+  %add_loop = add i32 4, %res
+  %cond_2 = icmp ugt i32 %add_loop, 3
+  br i1 %cond_2, label %body, label %exit
+
+exit:
+  ret i32 %add_loop
+}

[topperc]

Add -verify-machineinstrs so this would catch the issue on release builds and not just asserts builds.

[lenary]

Can we also add FileCheck + update_llc_test_checks.py lines?

[enoskova-sc]

added checks and -verify-machineinstrs

[wangpc-pp]

No CHECKs for this file?

[topperc]

LGTM