[PATCH] D65614: [Reassociate] Stop linearizing all associative expression trees w/o profitability

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D65614#1611421 <https://reviews.llvm.org/D65614#1611421>, @reames wrote:

> In D65614#1611255 <https://reviews.llvm.org/D65614#1611255>, @lebedev.ri wrote:
>
> > I'm seeing many regressions in diffs.
>
>
> Can you point to a specific example of what you mean?  I haven't studied every single one closely, but on a skim, I am not seeing regressions, so I'm curious what you mean?


Hm? There's clearly a lot of cases where instruction count increases.

>> Honestly i'd say this is the wrong fix/direction,
>>  i don't recall any such thing as register pressure
>>  for middle-end IR, there are no registers here, only value.
> 
> Right, but for the same reason, why should we be perturbing the input without reason?  We don't have any information about profitability to justify the transform.

This is a canonicalization pass. The same reasoning could be applied to InstCombine.



================
Comment at: test/Transforms/Reassociate/basictest.ll:39-40
 ; CHECK-NEXT:    [[T2:%.*]] = add i32 [[T1]], [[C]]
+; CHECK-NEXT:    [[T3:%.*]] = add i32 [[A]], [[C]]
+; CHECK-NEXT:    [[T4:%.*]] = add i32 [[B]], [[T3]]
 ; CHECK-NEXT:    store i32 [[T2]], i32* @e, align 4
----------------
 


================
Comment at: test/Transforms/Reassociate/basictest.ll:66-67
 ; CHECK-NEXT:    [[T2:%.*]] = add i32 [[T1]], [[C]]
+; CHECK-NEXT:    [[T3:%.*]] = add i32 [[A]], [[C]]
+; CHECK-NEXT:    [[T4:%.*]] = add i32 [[B]], [[T3]]
 ; CHECK-NEXT:    store i32 [[T2]], i32* @e, align 4
----------------
 


================
Comment at: test/Transforms/Reassociate/basictest.ll:93-94
 ; CHECK-NEXT:    [[T2:%.*]] = add i32 [[T1]], [[C]]
+; CHECK-NEXT:    [[T3:%.*]] = add i32 [[A]], [[C]]
+; CHECK-NEXT:    [[T4:%.*]] = add i32 [[B]], [[T3]]
 ; CHECK-NEXT:    store i32 [[T2]], i32* @e, align 4
----------------
 


================
Comment at: test/Transforms/Reassociate/basictest.ll:115-123
+; CHECK-NEXT:    [[TMP_0:%.*]] = load i32, i32* @a, align 4
+; CHECK-NEXT:    [[TMP_1:%.*]] = load i32, i32* @b, align 4
+; CHECK-NEXT:    [[TMP_2:%.*]] = add i32 [[TMP_0]], [[TMP_1]]
+; CHECK-NEXT:    [[TMP_4:%.*]] = load i32, i32* @c, align 4
+; CHECK-NEXT:    [[TMP_5:%.*]] = add i32 [[TMP_2]], [[TMP_4]]
+; CHECK-NEXT:    [[TMP_8:%.*]] = add i32 [[TMP_0]], [[TMP_4]]
+; CHECK-NEXT:    [[TMP_11:%.*]] = add i32 [[TMP_1]], [[TMP_8]]
----------------
!


================
Comment at: test/Transforms/Reassociate/basictest.ll:301
+; CHECK-NEXT:    [[C:%.*]] = mul i32 [[A]], [[X4:%.*]]
+; CHECK-NEXT:    [[D:%.*]] = mul i32 [[B]], [[X4]]
 ; CHECK-NEXT:    [[E:%.*]] = xor i32 [[C]], [[D]]
----------------
 


================
Comment at: test/Transforms/Reassociate/fast-basictest.ll:89-90
 ; CHECK-NEXT:    [[T2:%.*]] = fadd fast float [[T1]], [[C]]
+; CHECK-NEXT:    [[T3:%.*]] = fadd fast float [[A]], [[C]]
+; CHECK-NEXT:    [[T4:%.*]] = fadd fast float [[B]], [[T3]]
 ; CHECK-NEXT:    store float [[T2]], float* @fe, align 4
----------------
 


================
Comment at: test/Transforms/Reassociate/fast-basictest.ll:116-117
 ; CHECK-NEXT:    [[T2:%.*]] = fadd fast float [[T1]], [[C]]
+; CHECK-NEXT:    [[T3:%.*]] = fadd fast float [[A]], [[C]]
+; CHECK-NEXT:    [[T4:%.*]] = fadd fast float [[B]], [[T3]]
 ; CHECK-NEXT:    store float [[T2]], float* @fe, align 4
----------------
 


================
Comment at: test/Transforms/Reassociate/fast-basictest.ll:143-144
 ; CHECK-NEXT:    [[T2:%.*]] = fadd fast float [[T1]], [[C]]
+; CHECK-NEXT:    [[T3:%.*]] = fadd fast float [[A]], [[C]]
+; CHECK-NEXT:    [[T4:%.*]] = fadd fast float [[B]], [[T3]]
 ; CHECK-NEXT:    store float [[T2]], float* @fe, align 4
----------------
 


================
Comment at: test/Transforms/Reassociate/no-op.ll:31-35
 ; The initial add doesn't change so should not lose the nsw flag.
 ; CHECK-LABEL: @test2(
-; CHECK-NEXT:    [[A0:%.*]] = add nsw i32 [[B:%.*]], [[A:%.*]]
-; CHECK-NEXT:    [[A1:%.*]] = add i32 [[A0]], [[C:%.*]]
-; CHECK-NEXT:    [[A2:%.*]] = add i32 [[A1]], [[D:%.*]]
+; CHECK-NEXT:    [[A0:%.*]] = add nsw i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[A1:%.*]] = add nsw i32 [[A0]], [[D:%.*]]
+; CHECK-NEXT:    [[A2:%.*]] = add nsw i32 [[C:%.*]], [[A1]]
----------------
Hm, interesting.
I guess this is the first improvement i'm seeing.


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

https://reviews.llvm.org/D65614