[PATCH] D111035: [AArch64] Fix incorrect removal of COPYs in AArch64RedundantCopyElimination

[Yuichiro Utsumi via Phabricator via llvm-commits]

yutsumi created this revision.
yutsumi added reviewers: t.p.northover, tmatheson, paulwalker-arm, gberry, mcrosier.
Herald added subscribers: hiraditya, kristof.beyls.
yutsumi requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Don't remove COPYs with implicit defs, unless we know the implicitly
defined register is also zero.

Removing the following mov instruction generated from a COPY
is invalid if the upper 32 bits of x1 is non-zero and x1 will be used.
So, we should avoid this removal.

  cbnzw b2
  b1:
  mov x1, wzr // set all 64 bits of x1 zero
  // code assuming upper half of x1 is non-zero

The same fix https://reviews.llvm.org/D29850 as this was abandoned
probably because the above instruction seemed not to be generated.
But the generation of it is depending on the passes executed
before, e.g., register allocator.
So, this assumption should not be made and we should
avoid removing the above COPYs as well as MOVi32imms.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111035

Files:
  llvm/lib/Target/AArch64/AArch64RedundantCopyElimination.cpp
  llvm/test/CodeGen/AArch64/machine-copy-remove.mir


Index: llvm/test/CodeGen/AArch64/machine-copy-remove.mir
===================================================================
--- llvm/test/CodeGen/AArch64/machine-copy-remove.mir
+++ llvm/test/CodeGen/AArch64/machine-copy-remove.mir
@@ -608,3 +608,57 @@
 
   bb.2:
     RET_ReallyLR
+...
+# Check that we don't remove a COPY that is implicitly defining the upper
+# 32 bits of an X reg if we don't know the upper 32 bits are zero.
+# CHECK-LABEL: name: test24
+# CHECK: COPY $wzr
+name:            test24
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $x0, $x1, $w0
+
+    $x0 = COPY $x1
+    CBNZW $w0, %bb.2
+
+  bb.1:
+    $w0 = COPY $wzr, implicit-def $x0
+    B %bb.3
+
+  bb.2:
+    liveins: $x1
+
+    $x0 = LDRXui $x1, 0
+
+  bb.3:
+    liveins: $x0
+
+    RET_ReallyLR implicit $x0
+...
+# Check that we do remove a COPY that is implicitly defining the upper
+# 32 bits of an X reg if we know the upper 32 bits are zero.
+# CHECK-LABEL: name: test25
+# CHECK-NOT: COPY $wzr
+name:            test25
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    liveins: $x0, $x1, $w0
+
+    $x0 = COPY $x1
+    CBNZX $x0, %bb.2
+
+  bb.1:
+    $w0 = COPY $wzr, implicit-def $x0
+    B %bb.3
+
+  bb.2:
+    liveins: $x1
+
+    $x0 = LDRXui $x1, 0
+
+  bb.3:
+    liveins: $x0
+
+    RET_ReallyLR implicit $x0
Index: llvm/lib/Target/AArch64/AArch64RedundantCopyElimination.cpp
===================================================================
--- llvm/lib/Target/AArch64/AArch64RedundantCopyElimination.cpp
+++ llvm/lib/Target/AArch64/AArch64RedundantCopyElimination.cpp
@@ -400,21 +400,21 @@
             if (KnownReg.Imm != SrcImm)
               continue;
 
-            // Don't remove a move immediate that implicitly defines the upper
-            // bits when only the lower 32 bits are known.
-            MCPhysReg CmpReg = KnownReg.Reg;
-            if (any_of(MI->implicit_operands(), [CmpReg](MachineOperand &O) {
-                  return !O.isDead() && O.isReg() && O.isDef() &&
-                         O.getReg() != CmpReg;
-                }))
-              continue;
-
             // Don't remove a move immediate that implicitly defines the upper
             // bits as different.
             if (TRI->isSuperRegister(DefReg, KnownReg.Reg) && KnownReg.Imm < 0)
               continue;
           }
 
+          // Don't remove a move immediate or a copy that implicitly defines the
+          // upper bits when only the lower 32 bits are known.
+          MCPhysReg CmpReg = KnownReg.Reg;
+          if (any_of(MI->implicit_operands(), [CmpReg](MachineOperand &O) {
+                return !O.isDead() && O.isReg() && O.isDef() &&
+                       O.getReg() != CmpReg;
+              }))
+            continue;
+
           if (IsCopy)
             LLVM_DEBUG(dbgs() << "Remove redundant Copy : " << *MI);
           else


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D111035.376802.patch
Type: text/x-patch
Size: 2886 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211004/c585dc2f/attachment.bin>