[llvm] r346117 - [AVR] Fix a backend bug that left extraneous operands after expansion

[Dylan McKay via llvm-commits]

Author: dylanmckay
Date: Sun Nov  4 21:49:04 2018
New Revision: 346117

URL: http://llvm.org/viewvc/llvm-project?rev=346117&view=rev
Log:
[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

Added:
    llvm/trunk/test/CodeGen/AVR/rust-avr-bug-112.ll
Modified:
    llvm/trunk/lib/Target/AVR/AVRRegisterInfo.cpp

Modified: llvm/trunk/lib/Target/AVR/AVRRegisterInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AVR/AVRRegisterInfo.cpp?rev=346117&r1=346116&r2=346117&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AVR/AVRRegisterInfo.cpp (original)
+++ llvm/trunk/lib/Target/AVR/AVRRegisterInfo.cpp Sun Nov  4 21:49:04 2018
@@ -152,6 +152,7 @@ void AVRRegisterInfo::eliminateFrameInde
   if (MI.getOpcode() == AVR::FRMIDX) {
     MI.setDesc(TII.get(AVR::MOVWRdRr));
     MI.getOperand(FIOperandNum).ChangeToRegister(AVR::R29R28, false);
+    MI.RemoveOperand(2);
 
     assert(Offset > 0 && "Invalid offset");
 

Added: llvm/trunk/test/CodeGen/AVR/rust-avr-bug-112.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AVR/rust-avr-bug-112.ll?rev=346117&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/AVR/rust-avr-bug-112.ll (added)
+++ llvm/trunk/test/CodeGen/AVR/rust-avr-bug-112.ll Sun Nov  4 21:49:04 2018
@@ -0,0 +1,48 @@
+; RUN: llc < %s -march=avr | FileCheck %s
+
+; The avr-rust bug can be found here:
+; https://github.com/avr-rust/rust/issues/112
+;
+; In this test, the codegen stage generates a FRMIDX
+; instruction. Later in the pipeline, the frame index
+; gets expanded into a 16-bit MOVWRdRr instruction.
+;
+; There was a bug in the FRMIDX->MOVWRdRr expansion logic
+; that could leave the MOVW instruction with an extraneous
+; operand, left over from the original FRMIDX.
+;
+; 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
+;
+; The logic has since been fixed.
+
+; CHECK-LABEL: "core::str::slice_error_fail"
+define void @"core::str::slice_error_fail"(i16 %arg) personality i32 (...) addrspace(1)* @rust_eh_personality {
+start:
+  %char_range = alloca { i16, i16 }, align 1
+  br i1 undef, label %"<core::option::Option<T>>::unwrap.exit.thread", label %bb11.i.i
+
+"<core::option::Option<T>>::unwrap.exit.thread":
+  br label %"core::char::methods::<impl char>::len_utf8.exit"
+
+bb11.i.i:
+  %tmp = bitcast { i16, i16 }* %char_range to i8*
+  %tmp1 = icmp ult i32 undef, 65536
+  %..i = select i1 %tmp1, i16 3, i16 4
+  br label %"core::char::methods::<impl char>::len_utf8.exit"
+
+"core::char::methods::<impl char>::len_utf8.exit":
+  %tmp2 = phi i8* [ %tmp, %bb11.i.i ], [ undef, %"<core::option::Option<T>>::unwrap.exit.thread" ]
+  %_0.0.i12 = phi i16 [ %..i, %bb11.i.i ], [ 1, %"<core::option::Option<T>>::unwrap.exit.thread" ]
+  %tmp3 = add i16 %_0.0.i12, %arg
+  store i16 %tmp3, i16* undef, align 1
+  store i8* %tmp2, i8** undef, align 1
+  unreachable
+}
+
+declare i32 @rust_eh_personality(...) addrspace(1)
+