Skip to content

Commit 7601a21

Browse files
committed
[RISCV] Only return DestSourcePair from isCopyInstrImpl for registers
ADDI often has a frameindex in operand 1, but consumers of this interface, such as MachineSink, tend to call getReg() on the Destination and Source operands, leading to the following crash when building FreeBSD after this implementation was added in 8cf6778: ``` clang: llvm/include/llvm/CodeGen/MachineOperand.h:359: llvm::Register llvm::MachineOperand::getReg() const: Assertion `isReg() && "This is not a register operand!"' failed. PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script. Stack dump: #0 0x00007f4286f9b4d0 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) llvm/lib/Support/Unix/Signals.inc:563:0 #1 0x00007f4286f9b587 PrintStackTraceSignalHandler(void*) llvm/lib/Support/Unix/Signals.inc:630:0 #2 0x00007f4286f9926b llvm::sys::RunSignalHandlers() llvm/lib/Support/Signals.cpp:71:0 #3 0x00007f4286f9ae52 SignalHandler(int) llvm/lib/Support/Unix/Signals.inc:405:0 #4 0x00007f428646ffd0 (/lib/x86_64-linux-gnu/libc.so.6+0x3efd0) #5 0x00007f428646ff47 raise /build/glibc-2ORdQG/glibc-2.27/signal/../sysdeps/unix/sysv/linux/raise.c:51:0 #6 0x00007f42864718b1 abort /build/glibc-2ORdQG/glibc-2.27/stdlib/abort.c:81:0 #7 0x00007f428646142a __assert_fail_base /build/glibc-2ORdQG/glibc-2.27/assert/assert.c:89:0 #8 0x00007f42864614a2 (/lib/x86_64-linux-gnu/libc.so.6+0x304a2) #9 0x00007f428d4078e2 llvm::MachineOperand::getReg() const llvm/include/llvm/CodeGen/MachineOperand.h:359:0 #10 0x00007f428d8260e7 attemptDebugCopyProp(llvm::MachineInstr&, llvm::MachineInstr&) llvm/lib/CodeGen/MachineSink.cpp:862:0 #11 0x00007f428d826442 performSink(llvm::MachineInstr&, llvm::MachineBasicBlock&, llvm::MachineInstrBundleIterator<llvm::MachineInstr, false>, llvm::SmallVectorImpl<llvm::MachineInstr*>&) llvm/lib/CodeGen/MachineSink.cpp:918:0 #12 0x00007f428d826e27 (anonymous namespace)::MachineSinking::SinkInstruction(llvm::MachineInstr&, bool&, std::map<llvm::MachineBasicBlock*, llvm::SmallVector<llvm::MachineBasicBlock*, 4u>, std::less<llvm::MachineBasicBlock*>, std::allocator<std::pair<llvm::MachineBasicBlock* const, llvm::SmallVector<llvm::MachineBasicBlock*, 4u> > > >&) llvm/lib/CodeGen/MachineSink.cpp:1073:0 #13 0x00007f428d824a2c (anonymous namespace)::MachineSinking::ProcessBlock(llvm::MachineBasicBlock&) llvm/lib/CodeGen/MachineSink.cpp:410:0 #14 0x00007f428d824513 (anonymous namespace)::MachineSinking::runOnMachineFunction(llvm::MachineFunction&) llvm/lib/CodeGen/MachineSink.cpp:340:0 ``` Thus, check that operand 1 is also a register in the condition. Reviewed By: arichardson, luismarques Differential Revision: https://reviews.llvm.org/D89090
1 parent 66009a1 commit 7601a21

File tree

2 files changed

+64
-1
lines changed

2 files changed

+64
-1
lines changed

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -538,7 +538,9 @@ RISCVInstrInfo::isCopyInstrImpl(const MachineInstr &MI) const {
538538
default:
539539
break;
540540
case RISCV::ADDI:
541-
if (MI.getOperand(2).isImm() && MI.getOperand(2).getImm() == 0)
541+
// Operand 1 can be a frameindex but callers expect registers
542+
if (MI.getOperand(1).isReg() && MI.getOperand(2).isImm() &&
543+
MI.getOperand(2).getImm() == 0)
542544
return DestSourcePair{MI.getOperand(0), MI.getOperand(1)};
543545
break;
544546
case RISCV::FSGNJ_D:
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
2+
# RUN: llc -march=riscv64 -debugify-and-strip-all-safe -run-pass machine-sink %s -o - 2>&1 | FileCheck %s
3+
4+
--- |
5+
define void @sink_addi_fi(i32 %0) !dbg !5 {
6+
bb.0:
7+
%1 = alloca i32, align 4
8+
call void @llvm.dbg.value(metadata i32* %1, metadata !1, metadata !DIExpression()), !dbg !3
9+
%2 = icmp eq i32 %0, 0
10+
br i1 %2, label %bb.2, label %bb.1
11+
bb.1:
12+
store volatile i32 0, i32* %1, align 4
13+
br label %bb.2
14+
bb.2:
15+
ret void
16+
}
17+
18+
declare void @llvm.dbg.value(metadata, metadata, metadata)
19+
20+
!llvm.dbg.cu = !{!0}
21+
22+
!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !4)
23+
!1 = !DILocalVariable(name: "var", scope: !5, file: !4, line: 2, type: !2)
24+
!2 = !DIBasicType(name: "unsigned int", size: 32, encoding: DW_ATE_unsigned)
25+
!3 = !DILocation(line: 0, scope: !5)
26+
!4 = !DIFile(filename: "test.c", directory: "")
27+
!5 = distinct !DISubprogram(name: "sink_addi_fi", scope: !4, file: !4, line: 1, type: !6, scopeLine: 1, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !8)
28+
!6 = !DISubroutineType(types: !7)
29+
!7 = !{null, !2}
30+
!8 = !{}
31+
32+
...
33+
---
34+
name: sink_addi_fi
35+
liveins:
36+
- { reg: '$x10', virtual-reg: '%0' }
37+
stack:
38+
- { id: 0, offset: 0, size: 4, alignment: 4 }
39+
body: |
40+
; CHECK-LABEL: name: sink_addi_fi
41+
; CHECK: bb.0:
42+
; CHECK: successors: %bb.2(0x40000000), %bb.1(0x40000000)
43+
; CHECK: [[COPY:%[0-9]+]]:gpr = COPY $x10
44+
; CHECK: BEQ killed [[COPY]], $x0, %bb.2
45+
; CHECK: bb.1:
46+
; CHECK: successors: %bb.2(0x80000000)
47+
; CHECK: [[ADDI:%[0-9]+]]:gpr = ADDI %stack.0, 0
48+
; CHECK: SW $x0, killed [[ADDI]], 0 :: (volatile store 4 into %stack.0)
49+
; CHECK: bb.2:
50+
; CHECK: PseudoRET
51+
bb.0:
52+
liveins: $x10
53+
%0:gpr = COPY $x10
54+
DBG_VALUE %stack.0, $noreg, !1, !DIExpression(), debug-location !3
55+
%1:gpr = ADDI %stack.0, 0
56+
DBG_VALUE %1, $noreg, !1, !DIExpression(DW_OP_plus_uconst, 0, DW_OP_stack_value), debug-location !3
57+
BEQ killed %0:gpr, $x0, %bb.2
58+
bb.1:
59+
SW $x0, killed %1:gpr, 0 :: (volatile store 4 into %stack.0, align 4)
60+
bb.2:
61+
PseudoRET

0 commit comments

Comments
 (0)