[PATCH] D37164: [ARM] Fix bug in ARMLoadStoreOptimizer when kill flags are missing.

[Geoff Berry via Phabricator via llvm-commits]

gberry created this revision.
Herald added subscribers: kristof.beyls, javed.absar, aemerson.

ARMLoadStoreOpt::FixInvalidRegPairOp() was only checking if one of the
load destination registers to be split overlapped with the base register
if the base register was marked as killed.  Since kill flags may not
always be present, this can lead to incorrect code.

This bug was exposed by my MachineCopyPropagation change https://reviews.llvm.org/D30751 breaking
the sanitizer-x86_64-linux-android buildbot.


https://reviews.llvm.org/D37164

Files:
  lib/Target/ARM/ARMLoadStoreOptimizer.cpp
  test/CodeGen/ARM/load_store_opt_kill.mir


Index: test/CodeGen/ARM/load_store_opt_kill.mir
===================================================================
--- /dev/null
+++ test/CodeGen/ARM/load_store_opt_kill.mir
@@ -0,0 +1,12 @@
+# RUN: llc -mtriple=armv7-none-eabi -verify-machineinstrs -run-pass arm-ldst-opt %s -o - | FileCheck %s
+---
+# CHECK-LABEL: name: f
+name:            f
+# Make sure the load into %r0 doesn't clobber the base register before the second load uses it.
+# CHECK: %r3 = LDRi12 %r0, 12, 14, _
+# CHECK-NEXT: %r0 = LDRi12 %r0, 8, 14, _
+body:             |
+  bb.0:
+    liveins: %r0, %r3
+    %r0, %r3 = LDRD %r0, %noreg, 8, 14, %noreg
+...
Index: lib/Target/ARM/ARMLoadStoreOptimizer.cpp
===================================================================
--- lib/Target/ARM/ARMLoadStoreOptimizer.cpp
+++ lib/Target/ARM/ARMLoadStoreOptimizer.cpp
@@ -1655,11 +1655,9 @@
       ? (isT2 ? (OffImm+4 < 0 ? ARM::t2LDRi8 : ARM::t2LDRi12) : ARM::LDRi12)
       : (isT2 ? (OffImm+4 < 0 ? ARM::t2STRi8 : ARM::t2STRi12) : ARM::STRi12);
     DebugLoc dl = MBBI->getDebugLoc();
-    // If this is a load and base register is killed, it may have been
-    // re-defed by the load, make sure the first load does not clobber it.
-    if (isLd &&
-        (BaseKill || OffKill) &&
-        (TRI->regsOverlap(EvenReg, BaseReg))) {
+    // If this is a load, make sure the first load does not clobber the base
+    // register before the second load reads it.
+    if (isLd && (TRI->regsOverlap(EvenReg, BaseReg))) {
       assert(!TRI->regsOverlap(OddReg, BaseReg));
       InsertLDR_STR(MBB, MBBI, OffImm+4, isLd, dl, NewOpc2,
                     OddReg, OddDeadKill, false,


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D37164.112741.patch
Type: text/x-patch
Size: 1649 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170825/4a722daa/attachment.bin>