Skip to content

Commit 2f70705

Browse files
author
dylanmckay
committed
[AVR] Fix a backend bug that left extraneous operands after expansion
This patch fixes a bug in the AVR FRMIDX expansion logic. The expansion would leave a leftover operand from the original FRMIDX, but now attached to a MOVWRdRr instruction. The MOVWRdRr instruction did not expect this operand and so LLVM rejected the machine instruction. This would trigger an assertion: Assertion failed: ((isImpReg || Op.isRegMask() || MCID->isVariadic() || OpNo < MCID->getNumOperands() || isMetaDataOp) && "Trying to add an operand to a machine instr that is already done!"), function addOperand, file llvm/lib/CodeGen/MachineInstr.cpp Tim fixed this so that now the FRMIDX is expanded correctly into a well-formed MOVWRdRr. Patch by Tim Neumann git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@346117 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 718f055 commit 2f70705

File tree

2 files changed

+49
-0
lines changed

2 files changed

+49
-0
lines changed

lib/Target/AVR/AVRRegisterInfo.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ void AVRRegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator II,
152152
if (MI.getOpcode() == AVR::FRMIDX) {
153153
MI.setDesc(TII.get(AVR::MOVWRdRr));
154154
MI.getOperand(FIOperandNum).ChangeToRegister(AVR::R29R28, false);
155+
MI.RemoveOperand(2);
155156

156157
assert(Offset > 0 && "Invalid offset");
157158

test/CodeGen/AVR/rust-avr-bug-112.ll

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
; RUN: llc < %s -march=avr | FileCheck %s
2+
3+
; The avr-rust bug can be found here:
4+
; https://github.com/avr-rust/rust/issues/112
5+
;
6+
; In this test, the codegen stage generates a FRMIDX
7+
; instruction. Later in the pipeline, the frame index
8+
; gets expanded into a 16-bit MOVWRdRr instruction.
9+
;
10+
; There was a bug in the FRMIDX->MOVWRdRr expansion logic
11+
; that could leave the MOVW instruction with an extraneous
12+
; operand, left over from the original FRMIDX.
13+
;
14+
; This would trigger an assertion:
15+
;
16+
; Assertion failed: ((isImpReg || Op.isRegMask() || MCID->isVariadic() ||
17+
; OpNo < MCID->getNumOperands() || isMetaDataOp) &&
18+
; "Trying to add an operand to a machine instr that is already done!"),
19+
; function addOperand, file llvm/lib/CodeGen/MachineInstr.cpp
20+
;
21+
; The logic has since been fixed.
22+
23+
; CHECK-LABEL: "core::str::slice_error_fail"
24+
define void @"core::str::slice_error_fail"(i16 %arg) personality i32 (...) addrspace(1)* @rust_eh_personality {
25+
start:
26+
%char_range = alloca { i16, i16 }, align 1
27+
br i1 undef, label %"<core::option::Option<T>>::unwrap.exit.thread", label %bb11.i.i
28+
29+
"<core::option::Option<T>>::unwrap.exit.thread":
30+
br label %"core::char::methods::<impl char>::len_utf8.exit"
31+
32+
bb11.i.i:
33+
%tmp = bitcast { i16, i16 }* %char_range to i8*
34+
%tmp1 = icmp ult i32 undef, 65536
35+
%..i = select i1 %tmp1, i16 3, i16 4
36+
br label %"core::char::methods::<impl char>::len_utf8.exit"
37+
38+
"core::char::methods::<impl char>::len_utf8.exit":
39+
%tmp2 = phi i8* [ %tmp, %bb11.i.i ], [ undef, %"<core::option::Option<T>>::unwrap.exit.thread" ]
40+
%_0.0.i12 = phi i16 [ %..i, %bb11.i.i ], [ 1, %"<core::option::Option<T>>::unwrap.exit.thread" ]
41+
%tmp3 = add i16 %_0.0.i12, %arg
42+
store i16 %tmp3, i16* undef, align 1
43+
store i8* %tmp2, i8** undef, align 1
44+
unreachable
45+
}
46+
47+
declare i32 @rust_eh_personality(...) addrspace(1)
48+

0 commit comments

Comments
 (0)