[PATCH] D139812: [LoopFusion] sink second loop PHIs

[Joshua Cao via Phabricator via llvm-commits]

caojoshua created this revision.
Herald added a subscriber: hiraditya.
Herald added a project: All.
caojoshua added reviewers: Narutoworld, aeubanks.
caojoshua added a reviewer: uabelho.
caojoshua published this revision for review.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Fixes https://github.com/llvm/llvm-project/issues/59023

PHI nodes that are in the second loop only have the first loop as its
predecessor. These PHI nodes should be sunk to the end of the fused
loop. If the second loop uses the PHI, then the loops cannot be fused.

I don't think this should happen in typical compilation workflows.
The PHI will be in a dedicated exit block of the first loop following
LCSSA transformations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139812

Files:
  llvm/lib/Transforms/Scalar/LoopFuse.cpp
  llvm/test/Transforms/LoopFusion/lcssa.ll


Index: llvm/test/Transforms/LoopFusion/lcssa.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/LoopFusion/lcssa.ll
@@ -0,0 +1,79 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -passes=loop-fusion < %s 2>&1 | FileCheck %s
+
+; Tests when the second loop preheader has a phi coming from the first loop
+; exit. If the phi is not used in the second loop, the loops can be fused and
+; the phi is sunk to the fused loop exit. If the phi is used in the second loop,
+; the loops cannot be fused.
+
+define void @test1() {
+; CHECK-LABEL: @test1(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[VECTOR_PH:%.*]]
+; CHECK:       vector.ph:
+; CHECK-NEXT:    br label [[MIDDLE_BLOCK:%.*]]
+; CHECK:       middle.block:
+; CHECK-NEXT:    br i1 true, label [[FOR_END41:%.*]], label [[VECTOR_PH]]
+; CHECK:       for.end41:
+; CHECK-NEXT:    [[DOTLCSSA92:%.*]] = phi i16 [ 1, [[MIDDLE_BLOCK]] ]
+; CHECK-NEXT:    ret void
+;
+entry:
+  br label %vector.ph
+
+vector.ph:                                        ; preds = %middle.block, %entry
+  br label %middle.block
+
+middle.block:                                     ; preds = %vector.ph
+  br i1 true, label %for.cond17.preheader, label %vector.ph
+
+for.cond17.preheader:                             ; preds = %middle.block
+  %.lcssa92 = phi i16 [ 1, %middle.block ]
+  br label %vector.ph61
+
+vector.ph61:                                      ; preds = %middle.block59, %for.cond17.preheader
+  br i1 true, label %for.end41, label %vector.ph61
+
+for.end41:                                        ; preds = %middle.block59
+  ret void
+}
+
+define void @test2() {
+; CHECK-LABEL: @test2(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[VECTOR_PH:%.*]]
+; CHECK:       vector.ph:
+; CHECK-NEXT:    br label [[MIDDLE_BLOCK:%.*]]
+; CHECK:       middle.block:
+; CHECK-NEXT:    br i1 true, label [[FOR_COND17_PREHEADER:%.*]], label [[VECTOR_PH]]
+; CHECK:       for.cond17.preheader:
+; CHECK-NEXT:    [[DOTLCSSA92:%.*]] = phi i16 [ 1, [[MIDDLE_BLOCK]] ]
+; CHECK-NEXT:    br label [[VECTOR_PH61:%.*]]
+; CHECK:       vector.ph61:
+; CHECK-NEXT:    call void @a(i16 [[DOTLCSSA92]])
+; CHECK-NEXT:    br i1 true, label [[FOR_END41:%.*]], label [[VECTOR_PH61]]
+; CHECK:       for.end41:
+; CHECK-NEXT:    ret void
+;
+entry:
+  br label %vector.ph
+
+vector.ph:                                        ; preds = %middle.block, %entry
+  br label %middle.block
+
+middle.block:                                     ; preds = %vector.ph
+  br i1 true, label %for.cond17.preheader, label %vector.ph
+
+for.cond17.preheader:                             ; preds = %middle.block
+  %.lcssa92 = phi i16 [ 1, %middle.block ]
+  br label %vector.ph61
+
+vector.ph61:                                      ; preds = %middle.block59, %for.cond17.preheader
+  call void @a(i16 %.lcssa92)
+  br i1 true, label %for.end41, label %vector.ph61
+
+for.end41:                                        ; preds = %middle.block59
+  ret void
+}
+
+declare void @a(i16);
Index: llvm/lib/Transforms/Scalar/LoopFuse.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/LoopFuse.cpp
+++ llvm/lib/Transforms/Scalar/LoopFuse.cpp
@@ -1064,6 +1064,11 @@
       }
     }
 
+    // PHIs in FC1's header only have FC0 blocks as predecessors. PHIs
+    // cannot be hoisted and should be sunk to the exit of the fused loop.
+    if (isa<PHINode>(I))
+      return false;
+
     // If this isn't a memory inst, hoisting is safe
     if (!I.mayReadOrWriteMemory())
       return true;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D139812.482008.patch
Type: text/x-patch
Size: 3651 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221212/7fa5cf6e/attachment.bin>