[PATCH] D151616: [Transforms][Reassociate] "Reassociate expressions" pass optimizations not always profitable

[Paul Osmialowski via Phabricator via llvm-commits]

pawosm01 created this revision.
pawosm01 added a reviewer: qcolombet.
Herald added a project: All.
pawosm01 requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Consider the following piece of code:

  void innermost_loop(int i, double d1, double d2, double delta, int n, double cells[n])
  {
    int j;
    const double d1d = d1 * delta;
    const double d2d = d2 * delta;
  
    for (j = 0; j <= i; j++)
      cells[j] = d1d * cells[j + 1] + d2d * cells[j];
  }

When compiling at `-Ofast` level, after the "Reassociate expressions"
pass, this code is transformed into an equivalent of:

  int j;
  
  for (j = 0; j <= i; j++)
    cells[j] = (d1 * cells[j + 1] + d2 * cells[j]) * delta;

Effectively, the computation of those loop invariants isn't done
before the loop anymore, we have one extra multiplication on each
loop iteration instead. Sadly, this results in a significant
performance hit.

This patch makes the OptimizeAdd() function of the "Reassociate
expressions" pass aware of modifying operations in a loop and
bails out when some of the operands are pulled from the outside
of the loop.

See also: https://github.com/llvm/llvm-project/issues/62736


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151616

Files:
  llvm/test/Transforms/Reassociate/reassociate-not-from-the-outside-of-the-loop.ll


Index: llvm/test/Transforms/Reassociate/reassociate-not-from-the-outside-of-the-loop.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/Reassociate/reassociate-not-from-the-outside-of-the-loop.ll
@@ -0,0 +1,44 @@
+; RUN: opt -passes=reassociate -S < %s | FileCheck %s
+
+; This test is to ensure that no computations are pulled into a loop
+; by the Reassociate pass. Doing so can result in the loop invariants not being
+; computed before the loop anymore. In case of this test, it would add an extra
+; multiplication into the loop.
+
+; FIXME: the checks below need to be inverted to confirm the change to the
+; Reassociate pass.
+
+define void @innermost_loop(i32 %i, double %d1, double %d2, double %delta, ptr %cells) {
+; CHECK-LABEL: @innermost_loop(
+entry:
+; CHECK-LABEL: entry:
+  %mul = fmul fast double %d1, %delta
+  %mul1 = fmul fast double %d2, %delta
+; CHECK-NOT: %{{.*}} = fmul {{.*}} %delta
+  br label %for.cond
+
+for.cond:
+  %j.0 = phi i32 [ 0, %entry ], [ %add, %for.body ]
+  %cmp.not = icmp sgt i32 %j.0, %i
+  br i1 %cmp.not, label %for.end, label %for.body
+
+for.body:
+; CHECK-LABEL: for.body:
+  %add = add nuw nsw i32 %j.0, 1
+  %idxprom = zext i32 %add to i64
+  %arrayidx = getelementptr inbounds double, ptr %cells, i64 %idxprom
+  %0 = load double, ptr %arrayidx
+  %mul2 = fmul fast double %mul, %0
+  %idxprom3 = zext i32 %j.0 to i64
+  %arrayidx4 = getelementptr inbounds double, ptr %cells, i64 %idxprom3
+  %1 = load double, ptr %arrayidx4
+  %mul5 = fmul fast double %mul1, %1
+  %add6 = fadd fast double %mul2, %mul5
+; CHECK: %reass{{.*}} = fadd
+; CHECK-NEXT: %reass{{.*}} = fmul {{.*}} %delta
+  store double %add6, ptr %arrayidx4
+  br label %for.cond
+
+for.end:
+  ret void
+}


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D151616.526279.patch
Type: text/x-patch
Size: 1787 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230527/fb00a6c6/attachment.bin>