From 7c3fe23f5d78d2d4bc04592778211ded3a24c1dc Mon Sep 17 00:00:00 2001
From: Florian Hahn <florian_hahn@apple.com>
Date: Wed, 22 Jan 2020 09:16:40 -0800
Subject: [PATCH] [AArch64] Don't rename registers with pseudo defs in Ld/St
 opt.

If the root def of for renaming is a noop-pseudo instruction like kill,
we would end up without a correct def for the renamed register, causing
miscompiles.

This patch conservatively bails out on any pseudo instruction.

This fixes https://bugs.chromium.org/p/chromium/issues/detail?id=1037912#c70
---
 .../AArch64/AArch64LoadStoreOptimizer.cpp     | 13 ++++++++
 .../CodeGen/AArch64/stp-opt-with-renaming.mir | 33 +++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp b/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
index 3156bb44696..bc91d628f0b 100644
--- a/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
+++ b/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp
@@ -1325,6 +1325,19 @@ canRenameUpToDef(MachineInstr &FirstMI, LiveRegUnits &UsedInBetween,
 
     // For defs, check if we can rename the first def of RegToRename.
     if (FoundDef) {
+      // For some pseudo instructions, we might not generate code in the end
+      // (e.g. KILL) and we would end up without a correct def for the rename
+      // register.
+      // TODO: This might be overly conservative and we could handle those cases
+      // in multiple ways:
+      //       1. Insert an extra copy, to materialize the def.
+      //       2. Skip pseudo-defs until we find an non-pseudo def.
+      if (MI.isPseudo()) {
+        LLVM_DEBUG(dbgs() << "  Cannot rename pseudo instruction " << MI
+                          << "\n");
+        return false;
+      }
+
       for (auto &MOP : MI.operands()) {
         if (!MOP.isReg() || !MOP.isDef() || MOP.isDebug() || !MOP.getReg() ||
             !TRI->regsOverlap(MOP.getReg(), RegToRename))
diff --git a/test/CodeGen/AArch64/stp-opt-with-renaming.mir b/test/CodeGen/AArch64/stp-opt-with-renaming.mir
index 018827772da..b57e32338f0 100644
--- a/test/CodeGen/AArch64/stp-opt-with-renaming.mir
+++ b/test/CodeGen/AArch64/stp-opt-with-renaming.mir
@@ -469,3 +469,36 @@ body:             |
     RET undef $lr
 
 ...
+# Make sure we do not rename if pseudo-defs. Noop pseudo instructions like KILL
+# may lead to a missing definition of the rename register.
+#
+# CHECK-LABEL: name: test14_pseudo
+# CHECK: bb.0:
+# CHECK-NEXT:    liveins: $w8, $fp, $w25
+# CHECK:         renamable $w8 = KILL killed renamable $w8, implicit-def $x8
+# CHECK-NEXT:    STURXi killed renamable $x8, $fp, -40 :: (store 8)
+# CHECK-NEXT:    $w8 = ORRWrs $wzr, killed $w25, 0, implicit-def $x8
+# CHECK-NEXT:    STURXi killed renamable $x8, $fp, -32 :: (store 8)
+# CHECK-NEXT:    RET undef $lr
+#
+name:            test14_pseudo
+alignment:       4
+tracksRegLiveness: true
+liveins:
+  - { reg: '$x0' }
+  - { reg: '$x1' }
+  - { reg: '$x8' }
+frameInfo:
+  maxAlignment:    1
+  maxCallFrameSize: 0
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+    liveins: $w8, $fp, $w25
+
+    renamable $w8 = KILL killed renamable $w8, implicit-def $x8
+    STURXi killed renamable $x8, $fp, -40 :: (store 8)
+    $w8 = ORRWrs $wzr, killed $w25, 0, implicit-def $x8
+    STURXi killed renamable $x8, $fp, -32 :: (store 8)
+    RET undef $lr
+...