[PATCH] D125755: [AggressiveInstcombine] Conditionally fold saturated fptosi to llvm.fptosi.sat

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

There's a continual struggle between pushing towards canonical IR, vs, pushing towards things we think are cheap on some specific target.  Normally, the way we've resolved that is by distinguishing "early" vs. "late" optimizations: we try to push towards a canonical form early on to make the code easier to analyze, then we start doing optimizations like vectorization etc. using target-specific heuristics.  AggressiveInstCombine doesn't really have anything to do with "early" vs. "late"; if we want something that runs just before vectorization, we should probably add a dedicated pass.

If vectorization is involved, we have to worry about cost difference between vector and scalar versions.  For example, for vectors, we might want to use floating-point min, but for scalars we prefer integer min.  Not sure if this is actually true for any target, but worth considering.  If we need to deal with situations like that, we can't cleanly query the cost model, so we should prefer some unified approach.  For example, attach some range metadata to fptosi.sat, or add some sort of "combiner" to VPlan.

That said, what targets are we worried about here?  I guess soft-float targets?  For targets with native floats, it's hard for me to imagine "nnan fptosi.sat" is significantly more expensive than "fptosi+min+max". It looks like isel currently doesn't take advantage of nnan, but it probably should.


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

https://reviews.llvm.org/D125755