[PATCH] D78297: [MachineSink] Bail out if BreakPHIEdge is not consistent across iterations

[Kai Luo via Phabricator via llvm-commits]

lkail created this revision.
lkail added reviewers: qcolombet, jsji, nemanjai, PowerPC, alex-t.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

Suppose we have CFG

                  +-----------------+
                  |%bb.0:           |
                  |%r0, %r1 = OP ...|
           +------+-------------+---+
           |                    |
           |                    |
           |                    |
           |                    |
  +--------v-------+            |
  |%bb.1:          |            |
  |%r2 = OP ...    |            |
  +----------------+---+        |
                       |        |
                       |        |
                       |        |
                       |        |
                  +----v--------v----+
                  |%bb.2:            |
                  |%r3 = PHI %r0, %r2|
                  |USE %r1           |
                  +------------------+

When testing if `%bb.2` is a proper sink point for `%r0`, `BreakPHIEdge` is set `true`. Then `%r1` is also tested against `%bb.2`, however `BreakPHIEdge` is set `false`. This leads to sinking `%r0, %r1 = OP ...` after `%r3 = PHI %r0, %r2` in `%bb.2`, which is not right. This patch checks consistency of `BreakPHIEdge` between adjacent iteration and bail out if it's inconsistent.

Fixes https://bugs.llvm.org/show_bug.cgi?id=45557.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78297

Files:
  llvm/lib/CodeGen/MachineSink.cpp
  llvm/test/CodeGen/PowerPC/pr45557.mir


Index: llvm/test/CodeGen/PowerPC/pr45557.mir
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/pr45557.mir
@@ -0,0 +1,32 @@
+# RUN: llc -mtriple=powerpc -verify-machineinstrs -simplify-mir \
+# RUN:   --run-pass=machine-sink %s -o - | FileCheck %s
+
+--- |
+  @__const = external dso_local unnamed_addr constant [2 x i32], align 8
+  define void @c() {
+  entry:
+    unreachable
+  }
+...
+---
+name:            c
+alignment:       4
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    %3:gprc_and_gprc_nor0 = LIS target-flags(ppc-ha) @__const
+    %0:gprc, %4:gprc_and_gprc_nor0 = LWZU target-flags(ppc-lo) @__const, %3 :: (load 4 from `i64* bitcast ([2 x i32]* @__const to i64*)`, align 8)
+    %5:crbitrc = IMPLICIT_DEF
+    BC killed %5, %bb.2
+    B %bb.1
+
+  bb.1:
+    %6:gprc = LI 0
+
+  bb.2:
+    %2:gprc = PHI %0, %bb.0, %6, %bb.1
+    %1:gprc = LWZ 4, killed %4 :: (dereferenceable load 4 from `i64* bitcast ([2 x i32]* @__const to i64*)` + 4, align 8)
+    BLR implicit $lr, implicit $rm
+...
+# CHECK-LABEL: bb.2:
+# CHECK-NOT:     LWZU
Index: llvm/lib/CodeGen/MachineSink.cpp
===================================================================
--- llvm/lib/CodeGen/MachineSink.cpp
+++ llvm/lib/CodeGen/MachineSink.cpp
@@ -695,9 +695,10 @@
       if (SuccToSinkTo) {
         // If a previous operand picked a block to sink to, then this operand
         // must be sinkable to the same block.
-        bool LocalUse = false;
-        if (!AllUsesDominatedByBlock(Reg, SuccToSinkTo, MBB,
-                                     BreakPHIEdge, LocalUse))
+        bool LocalUse = false, PrevBreakPHIEdge = BreakPHIEdge;
+        if (!AllUsesDominatedByBlock(Reg, SuccToSinkTo, MBB, BreakPHIEdge,
+                                     LocalUse) ||
+            BreakPHIEdge != PrevBreakPHIEdge)
           return nullptr;
 
         continue;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78297.258044.patch
Type: text/x-patch
Size: 1924 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200416/37c77b8a/attachment-0001.bin>