[PATCH] D119851: [InstCombine] push constant operand up/inside in sequence of min/max intrinsics

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked 3 inline comments as done.
spatel added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/minmax-intrinsics.ll:2268
   %m1 = call i8 @llvm.smax.i8(i8 %x, i8 42)
   %m2 = call i8 @llvm.smax.i8(i8 %y, i8 %m1)
   %m3 = call i8 @llvm.smax.i8(i8 %m2, i8 126)
----------------
nikic wrote:
> What would happen if you swapped the operands on this smax? Unless I'm missing something, in the case of non-constant arguments, we're going to make a pretty arbitrary reassociation choice, which will depend on non-canonicalized argument order to produce a good result.
Nice catch! I'll add a test.

We don't do the more elaborate complexity-based canonicalization for intrinsics that we do for commutative instructions, so the operand order on the middle max is not fixed. 

We could pick another matching intrinsic if it exists, but there's no complete solution within the scope of instcombine AFAIK.


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

https://reviews.llvm.org/D119851