[PATCH] D129612: [Reassociate] Cleanup minor missed optimizations

Warren Ristow via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 13 17:20:19 PDT 2022


wristow updated this revision to Diff 444461.
wristow added a comment.

Addressed review comments from @spatel , except for adding an integer test.

If I take a similar integer test:

  ; (-X)*Y + Z -> Z-X*Y
  define i32 @int_test7(i32 %X, i32 %Y, i32 %Z) {
    %A = sub i32 0, %X
    %B = mul i32 %A, %Y
    %C = add i32 %B, %Z
    ret i32 %C
  }

the change does clean the results up //somewhat//, but not as completely as the floating-point version.  Specifically, //without// the change, the above test is reassociated to:

  define i32 @int_test7(i32 %X, i32 %Y, i32 %Z) {
    %1 = sub i32 0, 0
    %A = mul i32 %Y, %X
    %B = mul i32 %A, -1
    %C = add i32 %B, %Z
    ret i32 %C
  }

So it has the unused instruction `%1 = sub i32 0, 0` remaining.

And //with// the change, that instruction is deleted:

  define i32 @int_test7(i32 %X, i32 %Y, i32 %Z) {
    %A = mul i32 %Y, %X
    %B = mul i32 %A, -1
    %C = add i32 %B, %Z
    ret i32 %C
  }

The equivalent FP version transforms the computation of `%B` into a multiply of positive 1 (which is then optimized away), and `%C` is then computed with a subtraction, rather than an addition.  So the FP version has only two arithmetic instructions (multiply and subtract), whereas the integer version ends up as above (multiply, multiply (by negative 1), addition).

Looking into this difference, I see it's because the routine `ReassociatePass::OptimizeInst` has:

  // Canonicalize negative constants out of expressions.
  if (Instruction *Res = canonicalizeNegFPConstants(I))
    I = Res;

which nicely handles this multiply by -1.0 for the floating-point case, but there isn't an equivalent `canonicalizeNegIntConstants` concept.


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

https://reviews.llvm.org/D129612

Files:
  llvm/lib/Transforms/Scalar/Reassociate.cpp
  llvm/test/Transforms/Reassociate/fast-ReassociateVector.ll
  llvm/test/Transforms/Reassociate/fast-basictest.ll
  llvm/test/Transforms/Reassociate/pr42349.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D129612.444461.patch
Type: text/x-patch
Size: 11138 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220714/e793a438/attachment.bin>


More information about the llvm-commits mailing list