[PATCH] D36772: Unmerge GEPs to reduce register pressure on IndirectBr edges.

[Hiroshi Yamauchi via Phabricator via llvm-commits]

yamauchi added a comment.

Sorry for a delay.

In https://reviews.llvm.org/D36772#848262, @hfinkel wrote:

> > With this patch, the Python interpreter loop runs faster by ~5%.
>
> On what platform?


x86-64 Haswell.

> Did you try always doing this (instead of just doing it over indirect branches)? You're possibly increasing the critical path by doing the computation this way, and if you have a processor with a good branch predictor maybe this shows up? But if you told me that it generally always helps, I'd not be too surprised either.

Good points.

No, I didn't try always doing this. My thoughts follow:

As you point out, I think there may be a tradeoff between potential spills and a potentially longer critical path, and it's not 100% clear which way is better *in general* because that would depend on how the CPU works and it's not easy to tell whether this would actually save spills at this stage, though for the indirectbr in the python interpreter, it's a clear win due to fewer spills (on x86-64).

The benefits of restricting this to relatively rare indirectbr are that (a) it might limit the impact of a potentially longer critical path, if any, and (b) the impact on the compile time should be minimal because it's the first check we do there and the common case doesn't need to go through the subsequent more elaborate checks.

Maybe should query the target and do this only if the number of registers is low (or just x86(-64))?

I wish I could formulate this in a better way. I haven't found a better way so far. If you see a better way, please let me know.

> You should be careful not to create constants that aren't cheap to represent if you start with ones that are. Specifically, `UGEPIIdx->getValue() - GEPIIdx->getValue()` might be large (if one of those values is negative). TTI has getIntImmCost, and if both UIdx and Idx are cheap, but (Uidx - Idx) is expensive, we probably don't want to do this.

Agreed. Will work on this.


https://reviews.llvm.org/D36772