This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Enable type promotion for AArch64
ClosedPublic

Authored by dmgreen on Sep 22 2021, 6:06 AM.

Details

Summary

This enables the type promotion pass for AArch64, which acts as a CodeGenPrepare pass to promote illegal integers to legal ones, especially useful for removing extends that would otherwise require cross-basic-block analysis.

(I have enabled this generally, for both ISel and GlobalISel. In some quick experiments it appeared to help GlobalISel remove extra extends in places too, but that might just be missing optimizations that are better left for later. I can disable it if that sounds better).

In my experiments, this can improvement performance in some cases, and codesize was a small improvement. SPEC was a very small improvement, within the noise. Some of the test cases show extends being moved out of loops, often when the extend would be part of a cmp operand, but that should reduce the latency of the instruction in the loop on many cpus. The signed-truncation-check tests are increasing as they are no longer matching specific DAG combines. I will work on improving those.

We also hope to add some additional improvements to the pass in the near future, to capture more cases of promoting extends through phis that have come up in a few places lately.

Diff Detail

Event Timeline

dmgreen created this revision.Sep 22 2021, 6:06 AM
dmgreen requested review of this revision.Sep 22 2021, 6:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 22 2021, 6:06 AM

I didn't think about the extra latency of a uxt cmp, and I wouldn't have thought it useful with GISel either - so this is a pleasant surprise. Do you reckon it's worth sorting out the sign extend dag patterns before enabling? Or do you think that the change is really gonna be in the noise so much that no-one will notice?

I didn't think about the extra latency of a uxt cmp, and I wouldn't have thought it useful with GISel either - so this is a pleasant surprise. Do you reckon it's worth sorting out the sign extend dag patterns before enabling? Or do you think that the change is really gonna be in the noise so much that no-one will notice?

I think they are fairly specific cases, so either way would hopefully be OK. They are deliberate tests for the code in TargetLowering::optimizeSetCCOfSignedTruncationCheck, which is not firing in the new form (and I don't see it coming up in other benchmarks). I'll look into trying to fix it when I get some time, but you know, have other patches that are likely more important to fix.

samparker accepted this revision.Sep 23 2021, 2:12 AM

Okay, fair enough.

This revision is now accepted and ready to land.Sep 23 2021, 2:12 AM
Matt added a subscriber: Matt.Sep 27 2021, 1:29 PM

Thanks. I took a look at the signed-truncation-check cases - they are a bit of a pain due to some other AArch64 folds that change the setcc X, 65280 into setcc (srl X, 8), 255 fairly early on, breaking up the pattern that would be recognized.

Andre has another patch that will be dependent on this being on by default. I'll commit this now, as my benchmarks suggest this to be an improvement and the cases that are getting worse I believe should be uncommon. Hopefully at some point I will find some time to make the worse cases better again in a more general way.

This revision was landed with ongoing or failed builds.Sep 29 2021, 7:13 AM
This revision was automatically updated to reflect the committed changes.