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] Fix crash in PEI with empty entry block with Zcmp #72117

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

nemanjai
Copy link
Member

We check the opcode of the first instruction in the block where the prologue is inserted without checking if the iterator points to any instructions. When the basic block is empty, that causes a crash. One way the prologue block can be empty is when it starts with a call to
__builtin_readcyclecounter on RV32 since that produces a loop.

We check the opcode of the first instruction in the block
where the prologue is inserted without checking if the
iterator points to any instructions. When the basic block
is empty, that causes a crash. One way the prologue block
can be empty is when it starts with a call to
__builtin_readcyclecounter on RV32 since that produces a loop.
@nemanjai nemanjai requested review from asb and topperc November 13, 2023 14:11
@nemanjai nemanjai self-assigned this Nov 13, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 13, 2023

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

Author: Nemanja Ivanovic (nemanjai)

Changes

We check the opcode of the first instruction in the block where the prologue is inserted without checking if the iterator points to any instructions. When the basic block is empty, that causes a crash. One way the prologue block can be empty is when it starts with a call to
__builtin_readcyclecounter on RV32 since that produces a loop.


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

2 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVFrameLowering.cpp (+2-1)
  • (added) llvm/test/CodeGen/RISCV/pei-crash.ll (+28)
diff --git a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
index d873da7d684cfe2..1709ac28d3c5687 100644
--- a/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
@@ -526,7 +526,8 @@ void RISCVFrameLowering::emitPrologue(MachineFunction &MF,
     RealStackSize = FirstSPAdjustAmount;
   }
 
-  if (RVFI->isPushable(MF) && FirstFrameSetup->getOpcode() == RISCV::CM_PUSH) {
+  if (RVFI->isPushable(MF) && FirstFrameSetup != MBB.end() &&
+      FirstFrameSetup->getOpcode() == RISCV::CM_PUSH) {
     // Use available stack adjustment in push instruction to allocate additional
     // stack space.
     uint64_t Spimm = std::min(StackSize, (uint64_t)48);
diff --git a/llvm/test/CodeGen/RISCV/pei-crash.ll b/llvm/test/CodeGen/RISCV/pei-crash.ll
new file mode 100644
index 000000000000000..7778f9580cf69df
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/pei-crash.ll
@@ -0,0 +1,28 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2
+; RUN: llc -O0 --frame-pointer=none -mtriple=riscv32 -mattr=+zcmp \
+; RUN:   -verify-machineinstrs < %s | FileCheck %s
+define dso_local i64 @a() #0 {
+; CHECK-LABEL: a:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    addi sp, sp, -16
+; CHECK-NEXT:  .LBB0_1: # %entry
+; CHECK-NEXT:    # =>This Inner Loop Header: Depth=1
+; CHECK-NEXT:    rdcycleh a0
+; CHECK-NEXT:    sw a0, 8(sp) # 4-byte Folded Spill
+; CHECK-NEXT:    rdcycle a1
+; CHECK-NEXT:    sw a1, 12(sp) # 4-byte Folded Spill
+; CHECK-NEXT:    rdcycleh a1
+; CHECK-NEXT:    bne a0, a1, .LBB0_1
+; CHECK-NEXT:  # %bb.2: # %entry
+; CHECK-NEXT:    lw a1, 8(sp) # 4-byte Folded Reload
+; CHECK-NEXT:    lw a0, 12(sp) # 4-byte Folded Reload
+; CHECK-NEXT:    addi sp, sp, 16
+; CHECK-NEXT:    ret
+entry:
+  %0 = call i64 @llvm.readcyclecounter()
+  ret i64 %0
+}
+
+declare i64 @llvm.readcyclecounter() #1
+
+attributes #0 = { noinline nounwind optnone }

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nemanjai nemanjai merged commit 2276071 into llvm:main Nov 17, 2023
4 checks passed
sr-tream pushed a commit to sr-tream/llvm-project that referenced this pull request Nov 20, 2023
We check the opcode of the first instruction in the block where the
prologue is inserted without checking if the iterator points to any
instructions. When the basic block is empty, that causes a crash. One
way the prologue block can be empty is when it starts with a call to
__builtin_readcyclecounter on RV32 since that produces a loop.

Co-authored-by: Nemanja Ivanovic <nemanja@synopsys.com>
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
We check the opcode of the first instruction in the block where the
prologue is inserted without checking if the iterator points to any
instructions. When the basic block is empty, that causes a crash. One
way the prologue block can be empty is when it starts with a call to
__builtin_readcyclecounter on RV32 since that produces a loop.

Co-authored-by: Nemanja Ivanovic <nemanja@synopsys.com>
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.

None yet

3 participants