[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
 
----------------
pawosm01 wrote:
> 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...
...and where those CHECK-NOT's has gone?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151616