[llvm] r328689 - [RegisterCoalescing] Don't move COPY if it would interfere with another value

[Mikael Holmen via llvm-commits]

Author: uabelho
Date: Tue Mar 27 23:01:30 2018
New Revision: 328689

URL: http://llvm.org/viewvc/llvm-project?rev=328689&view=rev
Log:
[RegisterCoalescing] Don't move COPY if it would interfere with another value

Summary:
RegisterCoalescer::removePartialRedundancy tries to hoist B = A from
BB0/BB2 to BB1:

  BB1:
       ...
  BB0/BB2:  ----
       B = A;   |
       ...      |
       A = B;   |
         |-------
         |

It does so if a number of conditions are fulfilled. However, it failed
to check if B was used by any of the terminators in BB1. Since we must
insert B = A before the terminators (since it's not a terminator itself),
this means that we could erroneously insert a new definition of B before a
use of it.

Reviewers: wmi, qcolombet

Reviewed By: wmi

Subscribers: MatzeB, llvm-commits, sdardis

Differential Revision: https://reviews.llvm.org/D44918

Added:
    llvm/trunk/test/CodeGen/Mips/coalesce-partial-redundant-reguse-terminator.mir
Modified:
    llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp

Modified: llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp?rev=328689&r1=328688&r2=328689&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp (original)
+++ llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp Tue Mar 27 23:01:30 2018
@@ -989,13 +989,24 @@ bool RegisterCoalescer::removePartialRed
   if (CopyLeftBB && CopyLeftBB->succ_size() > 1)
     return false;
 
-  // Now ok to move copy.
+  // Now (almost sure it's) ok to move copy.
   if (CopyLeftBB) {
+    // Position in CopyLeftBB where we should insert new copy.
+    auto InsPos = CopyLeftBB->getFirstTerminator();
+
+    // Make sure that B isn't referenced in the terminators (if any) at the end
+    // of the predecessor since we're about to insert a new definition of B
+    // before them.
+    if (InsPos != CopyLeftBB->end()) {
+      SlotIndex InsPosIdx = LIS->getInstructionIndex(*InsPos).getRegSlot(true);
+      if (IntB.overlaps(InsPosIdx, LIS->getMBBEndIdx(CopyLeftBB)))
+        return false;
+    }
+
     DEBUG(dbgs() << "\tremovePartialRedundancy: Move the copy to "
                  << printMBBReference(*CopyLeftBB) << '\t' << CopyMI);
 
     // Insert new copy to CopyLeftBB.
-    auto InsPos = CopyLeftBB->getFirstTerminator();
     MachineInstr *NewCopyMI = BuildMI(*CopyLeftBB, InsPos, CopyMI.getDebugLoc(),
                                       TII->get(TargetOpcode::COPY), IntB.reg)
                                   .addReg(IntA.reg);

Added: llvm/trunk/test/CodeGen/Mips/coalesce-partial-redundant-reguse-terminator.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Mips/coalesce-partial-redundant-reguse-terminator.mir?rev=328689&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/Mips/coalesce-partial-redundant-reguse-terminator.mir (added)
+++ llvm/trunk/test/CodeGen/Mips/coalesce-partial-redundant-reguse-terminator.mir Tue Mar 27 23:01:30 2018
@@ -0,0 +1,41 @@
+# RUN: llc -march=mips64 -o - %s -run-pass=simple-register-coalescing | FileCheck %s
+
+---
+name:            f
+tracksRegLiveness: true
+body:             |
+ bb.0:
+    successors: %bb.1
+
+    %21:gpr32 = ADDiu $zero, 0
+    %22:gpr32 = COPY %21
+    %22:gpr32 = ADDiu %22, 1
+    J %bb.1, implicit-def dead $at
+
+ bb.1:
+    successors: %bb.2
+
+    BEQ %22, $zero, %bb.2, implicit-def $at
+
+ bb.2:
+    successors: %bb.2, %bb.3
+
+    %22:gpr32 = COPY %21
+    %21:gpr32 = COPY %22
+    BEQ undef %0:gpr32, $zero, %bb.2, implicit-def $at
+
+ bb.3:
+
+...
+
+# We should not hoist the
+#
+#  %22:gpr32 = COPY %21
+#
+# into bb.1 since %22 is used in the BEQ.
+
+# CHECK-LABEL: bb.1:
+# CHECK-NOT:      COPY
+# CHECK:          BEQ
+
+# CHECK-LABEL: bb.2: