From 4c5a5c8db6a9bcfafbb8798e9129e125d1f98155 Mon Sep 17 00:00:00 2001
From: Dylan McKay <me@dylanmckay.io>
Date: Mon, 5 Nov 2018 05:49:04 +0000
Subject: [PATCH] [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

llvm-svn: 346117
---
 llvm/lib/Target/AVR/AVRRegisterInfo.cpp   |  1 +
 llvm/test/CodeGen/AVR/rust-avr-bug-112.ll | 48 +++++++++++++++++++++++
 2 files changed, 49 insertions(+)
 create mode 100644 llvm/test/CodeGen/AVR/rust-avr-bug-112.ll

diff --git a/llvm/lib/Target/AVR/AVRRegisterInfo.cpp b/llvm/lib/Target/AVR/AVRRegisterInfo.cpp
index d171a620760e..808a85e459c1 100644
--- a/llvm/lib/Target/AVR/AVRRegisterInfo.cpp
+++ b/llvm/lib/Target/AVR/AVRRegisterInfo.cpp
@@ -152,6 +152,7 @@ void AVRRegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator II,
   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");
 
diff --git a/llvm/test/CodeGen/AVR/rust-avr-bug-112.ll b/llvm/test/CodeGen/AVR/rust-avr-bug-112.ll
new file mode 100644
index 000000000000..7cf14330cdc1
--- /dev/null
+++ b/llvm/test/CodeGen/AVR/rust-avr-bug-112.ll
@@ -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)
+