[PATCH] D66287: GlobalISel: add combiner for indexed loads and stores

Amara Emerson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 23 10:45:43 PDT 2019


aemerson added a comment.

In D66287#1642767 <https://reviews.llvm.org/D66287#1642767>, @t.p.northover wrote:

> > My one concern is with adding the machine dominator analysis the the pipeline at -O0. Is there a significant compile time cost?
>
> Doesn't look like it. I did a few runs of the test-suite, and it came out at maybe 1% difference but the numbers were pretty noisy. I'd be inclined to leave it, though it would certainly be possible to only use MachineDominator at higher levels.


Is that running the entire combine or just the MDT? If the latter, it's still non-negligible, but if there are code size savings to be had then its ok for me.

> 
> 
>> It's ok if we can pay for it by reducing the amount of code to analyze but we should run the numbers to check.
> 
> I doubt this is the kind of optimization that would pay for itself in that way.




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

https://reviews.llvm.org/D66287





More information about the llvm-commits mailing list