[llvm-bugs] [Bug 48821] New: Perf regressions with enable-new-pm due to SpeculateAroundPHIs splitting backedges

[via llvm-bugs]

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

            Bug ID: 48821
           Summary: Perf regressions with enable-new-pm due to
                    SpeculateAroundPHIs splitting backedges
           Product: libraries
           Version: trunk
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P
         Component: Scalar Optimizations
          Assignee: unassignedbugs at nondot.org
          Reporter: bjorn.a.pettersson at ericsson.com
                CC: llvm-bugs at lists.llvm.org

Downstream we have seen performance regressions when using -enable-new-pm.
Basically the effect is that our backend passes fail to detect that hardware
loops can be use.

We've tracked it down to the SpecualteAroundPHIs pass modifying loops by doing
splitting on the back edge of the loop. And I wonder if that is expected (and
if there are any examples where it would be profitable to do that).

Consider a test case like this:

define void @hello(i16 %n) {
entry:
  %cmp1 = icmp slt i16 %n, 8
  br i1 %cmp1, label %for.body, label %for.end

for.body:
  %i.02 = phi i16 [ %add, %for.body ], [ 0, %entry ]
  %add = add nuw nsw i16 %i.02, 1
  %cmp = icmp slt i16 %add, %n
  br i1 %cmp, label %for.body, label %for.end

for.end:
  ret void
}


when compiled with "opt -passes=spec-phis -S" we get

define void @hello(i16 %n) {
entry:
  %cmp1 = icmp slt i16 %n, 8
  br i1 %cmp1, label %entry.for.body_crit_edge, label %for.end

entry.for.body_crit_edge:
  %add.1 = add nuw nsw i16 0, 1
  br label %for.body

for.body:
  %add.phi = phi i16 [ %add.0, %for.body.for.body_crit_edge ], [ %add.1,
%entry.for.body_crit_edge ]
  %cmp = icmp slt i16 %add.phi, %n
  br i1 %cmp, label %for.body.for.body_crit_edge, label %for.end

for.body.for.body_crit_edge:
  %add.0 = add nuw nsw i16 %add.phi, 1
  br label %for.body

for.end:
  ret void
}


Is this a good transformation, or should SpecualateAroundPHIs avoid splitting
backedges?

Note that something like this might do the trick (not sure if it is an ideal
solution though):

diff --git a/llvm/lib/Transforms/Scalar/SpeculateAroundPHIs.cpp
b/llvm/lib/Transforms/Scalar/SpeculateAroundPHIs.cpp
index 9b18c945d950..41377f0572d5 100644
--- a/llvm/lib/Transforms/Scalar/SpeculateAroundPHIs.cpp
+++ b/llvm/lib/Transforms/Scalar/SpeculateAroundPHIs.cpp
@@ -792,6 +792,13 @@ static bool tryToSpeculatePHIs(SmallVectorImpl<PHINode *>
&PNs,
     return false;
   }

+  for (auto &Pred : PredSet) {
+    if (DT.dominates(PNs[0]->getParent(), Pred)) {
+      LLVM_DEBUG(dbgs() << "  Involves a back-edge\n");
+      return false;
+    }
+  }
+


I don't know if any in-tree targets suffers the same way from
SpeculateAroundPHIs. Maybe people could do some benchmarks with the above patch
for their targets to see if there is any impacts.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20210120/8e2f1454/attachment.html>