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

[Paul Osmialowski via Phabricator via llvm-commits]

pawosm01 added inline comments.


================
Comment at: llvm/test/Transforms/Reassociate/reassociate-not-from-the-outside-of-the-loop.ll:1
 ; RUN: opt -passes=reassociate -S < %s | FileCheck %s
 
----------------
nikic wrote:
> Should use update_test_checks.py.
It will make it look like this:


```
--- a/llvm/test/Transforms/Reassociate/reassociate-not-from-the-outside-of-the-loop.ll
+++ b/llvm/test/Transforms/Reassociate/reassociate-not-from-the-outside-of-the-loop.ll
@@ -1,3 +1,4 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
 ; RUN: opt -passes=reassociate -S < %s | FileCheck %s

 ; This test is to ensure that no computations are pulled into a loop
@@ -9,12 +10,35 @@
 ; Reassociate pass.

 define void @innermost_loop(i32 %i, double %d1, double %d2, double %delta, ptr %cells) {
-; CHECK-LABEL: @innermost_loop(
+; CHECK-LABEL: define void @innermost_loop
+; CHECK-SAME: (i32 [[I:%.*]], double [[D1:%.*]], double [[D2:%.*]], double [[DELTA:%.*]], ptr [[CELLS:%.*]]) {
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[MUL:%.*]] = fmul fast double [[DELTA]], [[D1]]
+; CHECK-NEXT:    [[MUL1:%.*]] = fmul fast double [[DELTA]], [[D2]]
+; CHECK-NEXT:    br label [[FOR_COND:%.*]]
+; CHECK:       for.cond:
+; CHECK-NEXT:    [[J_0:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ [[ADD:%.*]], [[FOR_BODY:%.*]] ]
+; CHECK-NEXT:    [[CMP_NOT:%.*]] = icmp sgt i32 [[J_0]], [[I]]
+; CHECK-NEXT:    br i1 [[CMP_NOT]], label [[FOR_END:%.*]], label [[FOR_BODY]]
+; CHECK:       for.body:
+; CHECK-NEXT:    [[ADD]] = add nuw nsw i32 [[J_0]], 1
+; CHECK-NEXT:    [[IDXPROM:%.*]] = zext i32 [[ADD]] to i64
+; CHECK-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds double, ptr [[CELLS]], i64 [[IDXPROM]]
+; CHECK-NEXT:    [[TMP0:%.*]] = load double, ptr [[ARRAYIDX]], align 8
+; CHECK-NEXT:    [[MUL2:%.*]] = fmul fast double [[MUL]], [[TMP0]]
+; CHECK-NEXT:    [[IDXPROM3:%.*]] = zext i32 [[J_0]] to i64
+; CHECK-NEXT:    [[ARRAYIDX4:%.*]] = getelementptr inbounds double, ptr [[CELLS]], i64 [[IDXPROM3]]
+; CHECK-NEXT:    [[TMP1:%.*]] = load double, ptr [[ARRAYIDX4]], align 8
+; CHECK-NEXT:    [[MUL5:%.*]] = fmul fast double [[MUL1]], [[TMP1]]
+; CHECK-NEXT:    [[ADD6:%.*]] = fadd fast double [[MUL5]], [[MUL2]]
+; CHECK-NEXT:    store double [[ADD6]], ptr [[ARRAYIDX4]], align 8
+; CHECK-NEXT:    br label [[FOR_COND]]
+; CHECK:       for.end:
+; CHECK-NEXT:    ret void
+;
```
Is that what is expected here? It doesn't seem focusing on the matter of this patch, and it doesn't seem futureproof: many things not related to this patch can change that can affect this test case in this form...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151616/new/

https://reviews.llvm.org/D151616