[PATCH] D70852: [InstCombine] Guard maxnum/minnum conversions with a TTI query

[Eric Christopher via Phabricator via llvm-commits]

echristo added a comment.

In D70852#1763946 <https://reviews.llvm.org/D70852#1763946>, @uweigand wrote:

> But in those cases like funnel shifts, common code (specifically SelectionDAGBuilder::visitIntrinsicCall) will handle the expansion if the target doesn't have an instruction.
>
> So if we do need to do that for minnum/maxnum, that would probably be the better place.
>
> However, this is still a bit different, because minnum/maxnum can also be the result of an explicit call to fmin/fmax in the source, and it would seem surprising to have that translated into a select sequence as well.  If we originally had an fmin/fmax call, which was translated into minnum/maxnum that we then couldn't further optimize, the current expansion (back to the fmin/fmax call) seems correct to me.
>
> It's only the case where we originally had a conditional that was translated into minnum/maxnum that expansion into fmin/fmax is incorrect (because those routines are only available when linking with -lm which the user might not do since there's no actual call to a libm routine in the original source).
>
> The problem really is IMO that there's no way to tell these two cases apart at instruction selection time.


FWIW I agree with Ulrich here, either we can lower this in the middle end or we depend upon either libc/compiler-rt/libgcc/etc. Depending upon libm is historically different here and would require some additional discussion.


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

https://reviews.llvm.org/D70852