[llvm] da9ed3d - [AArch64] Avoid adding duplicate implicit operands when expanding pseudo insts.

[Andrew Wei via llvm-commits]

Author: Andrew Wei
Date: 2021-09-07T17:11:58+08:00
New Revision: da9ed3dc719bbe0dbe8d4d8b7e6e2061532392fe

URL: https://github.com/llvm/llvm-project/commit/da9ed3dc719bbe0dbe8d4d8b7e6e2061532392fe
DIFF: https://github.com/llvm/llvm-project/commit/da9ed3dc719bbe0dbe8d4d8b7e6e2061532392fe.diff

LOG: [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

Added: 
    llvm/test/CodeGen/AArch64/expand-subs-pseudo.mir

Modified: 
    llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp b/llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp
index b2eee2845ba93..1c05d6235e02f 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 0000000000000..b09e3b2665280
--- /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
+
+...