[Localizer] Don't trick to be smart for the insertion point
authorQuentin Colombet <qcolombet@apple.com>
Tue, 30 May 2017 20:53:06 +0000 (20:53 +0000)
committerQuentin Colombet <qcolombet@apple.com>
Tue, 30 May 2017 20:53:06 +0000 (20:53 +0000)
There is no guarantee that the first use of a constant that is traversed
is actually the first in the related basic block. Thus, if we use that
as the insertion point we may end up with definitions that don't
dominate there use.

llvm-svn: 304244

llvm/lib/CodeGen/GlobalISel/Localizer.cpp
llvm/test/CodeGen/AArch64/GlobalISel/localizer.mir

index c2a568e..c5d0999 100644 (file)
@@ -98,12 +98,10 @@ bool Localizer::runOnMachineFunction(MachineFunction &MF) {
           // Create the localized instruction.
           MachineInstr *LocalizedMI = MF.CloneMachineInstr(&MI);
           LocalizedInstrs.insert(LocalizedMI);
-          // Move it at the right place.
-          MachineInstr &MIUse = *MOUse.getParent();
-          if (MIUse.getParent() == InsertMBB)
-            InsertMBB->insert(MIUse, LocalizedMI);
-          else
-            InsertMBB->insert(InsertMBB->getFirstNonPHI(), LocalizedMI);
+          // Don't try to be smart for the insertion point.
+          // There is no guarantee that the first seen use is the first
+          // use in the block.
+          InsertMBB->insert(InsertMBB->getFirstNonPHI(), LocalizedMI);
 
           // Set a new register for the definition.
           unsigned NewReg =
index 8fbb204..5bf8dac 100644 (file)
@@ -12,6 +12,7 @@
   define void @non_local_phi_use_followed_by_use() { ret void }
   define void @non_local_phi_use_followed_by_use_fi() { ret void }
   define void @float_non_local_phi_use_followed_by_use_fi() { ret void }
+  define void @non_local_phi() { ret void }
 ...
 
 ---
@@ -310,3 +311,51 @@ body:             |
     %3(s32) = PHI %0(s32), %bb.1
     %2(s32) = G_FADD %3, %0
 ...
+
+---
+# Make sure we don't insert a constant before PHIs.
+# This used to happen for loops of one basic block.
+# CHECK-LABEL: name: non_local_phi
+name:            non_local_phi
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+
+# CHECK:      registers:
+# Existing registers should be left untouched
+# CHECK:  - { id: 0, class: fpr }
+#CHECK-NEXT:  - { id: 1, class: fpr }
+#CHECK-NEXT:  - { id: 2, class: fpr }
+#CHECK-NEXT:  - { id: 3, class: fpr }
+# The newly created reg should be on the same regbank/regclass as its origin.
+#CHECK-NEXT:  - { id: 4, class: fpr }
+
+registers:
+  - { id: 0, class: fpr }
+  - { id: 1, class: fpr }
+  - { id: 2, class: fpr }
+  - { id: 3, class: fpr }
+
+# CHECK:  body:
+# CHECK:    %0(s32) = G_FCONSTANT float 1.0
+# CHECK-NEXT: %1(s32) = G_FADD %0, %0
+
+# CHECK: bb.1:
+# CHECK: %3(s32) = PHI %1(s32), %bb.0, %4(s32), %bb.1
+# CHECK: %4(s32) = G_FCONSTANT float 1.0
+
+# CHECK-NEXT: %2(s32) = G_FADD %3, %1
+body:             |
+  bb.0:
+    successors: %bb.1
+
+    %0(s32) = G_FCONSTANT float 1.0
+    %1(s32) = G_FADD %0, %0
+
+  bb.1:
+    successors: %bb.1
+
+    %3(s32) = PHI %1(s32), %bb.0, %0(s32), %bb.1
+    %2(s32) = G_FADD %3, %1
+    G_BR %bb.1
+...