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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a subscriber: lattner.
spatel added a comment.

In D125755#3521773 <https://reviews.llvm.org/D125755#3521773>, @RKSimon wrote:

> OK - speaking to @spatel offline there was a concern that AIC was becoming a bit of a dumping ground for combines that didn't really fit anywhere else.

AggressiveInstCombine was **intended** to be the dumping ground for regular InstCombine. :)

There was concern that InstCombine was taking too long per invocation and was getting added all over the default optimization pipelines just to cleanup mess from other passes. So AIC was a place that would only run once or twice in the pipeline where we could offload peepholes to ease compile-time cost. But that hasn't happened - we don't seem to be as collectively concerned about the compiler getting slower as much as when AIC was added. And everyone defaults to patching InstCombine because that guarantees that their fold will run and mesh with more folds. AIC is still only run at -O3 AFAIK.

> It might be that we need to consider a CostDrivenInstCombine pass or something instead? But then I'm not sure how much that will overlap with VectorCombine etc,

VectorCombine is definitely a more focused pass. This patch and D113291 <https://reviews.llvm.org/D113291> do not have vector requirements...but yes, the code starts to look similar once we put TTI cost comparisons into the mix.

In D113291 <https://reviews.llvm.org/D113291>, Craig made the point that we can predicate InstCombine and other pass transforms based on type legality from the DataLayout, so why not on opcode/intrinsic legality too?

So I'm not opposed, but I also don't have a good sense about potential downside of allowing target-specific IR transforms earlier in the optimization pipeline. Clearly, there's demand to do this because we get this kind of request fairly regularly.

@efriedma @lattner - any thoughts?


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

https://reviews.llvm.org/D125755