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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

I like the idea of limiting this pass. It annoys me that -reassociate can pseudo-randomly change the order of associative+commutative operands, and those transforms don't play by instcombine's operand complexity canonicalization rules (see getComplexity() in InstCombine). In other words, this pass can (hopefully only benignly) fight with instcombine.

But I agree with Roman's analysis of the test diffs - we need to preserve the cases where reassociation/factorization eliminates instructions. InstCombine doesn't handle that generally, and we don't want to shift that burden to InstCombine because that pass is already expensive.


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

https://reviews.llvm.org/D65614