[llvm] r304244 - [Localizer] Don't trick to be smart for the insertion point

[Quentin Colombet via llvm-commits]

Author: qcolombet
Date: Tue May 30 15:53:06 2017
New Revision: 304244

URL: http://llvm.org/viewvc/llvm-project?rev=304244&view=rev
Log:
[Localizer] Don't trick to be smart for the insertion point

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.

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

Modified: llvm/trunk/lib/CodeGen/GlobalISel/Localizer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/Localizer.cpp?rev=304244&r1=304243&r2=304244&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GlobalISel/Localizer.cpp (original)
+++ llvm/trunk/lib/CodeGen/GlobalISel/Localizer.cpp Tue May 30 15:53:06 2017
@@ -98,12 +98,10 @@ bool Localizer::runOnMachineFunction(Mac
           // 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 =

Modified: llvm/trunk/test/CodeGen/AArch64/GlobalISel/localizer.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/GlobalISel/localizer.mir?rev=304244&r1=304243&r2=304244&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/GlobalISel/localizer.mir (original)
+++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/localizer.mir Tue May 30 15:53:06 2017
@@ -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
+...