From da9ed3dc719bbe0dbe8d4d8b7e6e2061532392fe Mon Sep 17 00:00:00 2001
From: Andrew Wei <weiwei64@huawei.com>
Date: Tue, 7 Sep 2021 17:05:39 +0800
Subject: [PATCH] [AArch64] Avoid adding duplicate implicit operands when
 expanding pseudo insts.

When expanding pseudo insts, in order to create a new machine instr, we use BuildMI,
which will add implicit operands by default. And transferImpOps will also copy implicit
operands from old ones. Finally, duplicate implicit operands are added to the same inst.
Sometimes this can cause correctness issues. Like below inst,
    renamable $w18 = nsw SUBSWrr renamable $w30, renamable $w14, implicit-def dead $nzcv
After expanding, it will become
    $w18 = SUBSWrs renamable $w13, renamable $w14, 0, implicit-def $nzcv, implicit-def dead $nzcv
A redundant implicit-def $nzcv is added, but the dead flag is missing.

Reviewed By: dmgreen

Differential Revision: https://reviews.llvm.org/D109069
---
 .../AArch64/AArch64ExpandPseudoInsts.cpp      | 16 ++++++++------
 .../CodeGen/AArch64/expand-subs-pseudo.mir    | 21 +++++++++++++++++++
 2 files changed, 31 insertions(+), 6 deletions(-)
 create mode 100644 llvm/test/CodeGen/AArch64/expand-subs-pseudo.mir

diff --git a/llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp b/llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp
index b2eee2845ba9..1c05d6235e02 100644
--- a/llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp
@@ -937,12 +937,16 @@ bool AArch64ExpandPseudo::expandMI(MachineBasicBlock &MBB,
     case AArch64::ORRWrr:      Opcode = AArch64::ORRWrs; break;
     case AArch64::ORRXrr:      Opcode = AArch64::ORRXrs; break;
     }
-    MachineInstrBuilder MIB1 =
-        BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(Opcode),
-                MI.getOperand(0).getReg())
-            .add(MI.getOperand(1))
-            .add(MI.getOperand(2))
-            .addImm(AArch64_AM::getShifterImm(AArch64_AM::LSL, 0));
+    MachineFunction &MF = *MBB.getParent();
+    // Try to create new inst without implicit operands added.
+    MachineInstr *NewMI = MF.CreateMachineInstr(
+        TII->get(Opcode), MI.getDebugLoc(), /*NoImplicit=*/true);
+    MBB.insert(MBBI, NewMI);
+    MachineInstrBuilder MIB1(MF, NewMI);
+    MIB1.addReg(MI.getOperand(0).getReg(), RegState::Define)
+        .add(MI.getOperand(1))
+        .add(MI.getOperand(2))
+        .addImm(AArch64_AM::getShifterImm(AArch64_AM::LSL, 0));
     transferImpOps(MI, MIB1, MIB1);
     MI.eraseFromParent();
     return true;
diff --git a/llvm/test/CodeGen/AArch64/expand-subs-pseudo.mir b/llvm/test/CodeGen/AArch64/expand-subs-pseudo.mir
new file mode 100644
index 000000000000..b09e3b266528
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/expand-subs-pseudo.mir
@@ -0,0 +1,21 @@
+# RUN: llc -run-pass=aarch64-expand-pseudo -mtriple=aarch64-unknown-linux-gnu -o - %s | FileCheck %s
+
+---
+# CHECK-LABEL:  name: test
+# CHECK-LABEL:  bb.0:
+# CHECK:          $w5 = SUBSWrs renamable $w3, renamable $w2, 0, implicit-def dead $nzcv
+# CHECK-NEXT:     $w6 = SUBSWrs renamable $w5, renamable $w3, 0, implicit-def $nzcv
+# CHECK-NEXT:     RET undef $lr
+#
+name:            test
+alignment:       4
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $w5, $w6, $x2, $x3
+
+    renamable $w5 = nsw SUBSWrr renamable $w3, renamable $w2, implicit-def dead $nzcv
+    renamable $w6 = nsw SUBSWrr renamable $w5, renamable $w3, implicit-def $nzcv
+    RET_ReallyLR
+
+...