This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Remove 64bit->128bit vector insert lowering
ClosedPublic

Authored by dmgreen on Feb 22 2023, 3:25 AM.

Details

Summary

The AArch64 backend, during lowering, will convert an 64bit vector insert to a 128bit vector:

vector_insert %dreg, %v, %idx
=>
%qreg = insert_subvector undef, %dreg, 0
%ins = vector_insert %qreg, %v, %idx
EXTRACT_SUBREG %ins, dsub

This creates a bit of mess in the DAG, and the EXTRACT_SUBREG being a machine nodes makes it difficult to simplify. This patch removes that, treating the 64bit vector insert as legal and handling them with extra tablegen patterns.

The end result is a simpler DAG that is easier to write tablegen patterns for. Unfortunately one of the tests here does get larger, because that simplification now allows it to optimize away sign_extend_inreg from a vector insert due to the bits not being demanded. In that case it now generates both SMOV and UMOV though, requiring more total instructions. This is unfortunate but seems like an unrelated issue.

Diff Detail

Event Timeline

dmgreen created this revision.Feb 22 2023, 3:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 22 2023, 3:25 AM
dmgreen requested review of this revision.Feb 22 2023, 3:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 22 2023, 3:25 AM

The idea makes sense I think, but just to put things into context, do you already have a case or patch where we can see the benefit of this?

The idea makes sense I think, but just to put things into context, do you already have a case or patch where we can see the benefit of this?

The insert-load-into-zero from D144086 is helped by this. I wrote them in a slightly odd order due to trying to work through the regressions in this change. Otherwise some of the patterns in that patch would need to look for insert(extractsubreg(zerovec), load, 0), which isn't impossible but it's cleaner without all the extracts.

Speaking of which I've put up D144850 to help with the regressions in combine_srem_sdiv.

SjoerdMeijer accepted this revision.Feb 27 2023, 5:14 AM

Ok, cheers, and this looks very reasonable to me.

This revision is now accepted and ready to land.Feb 27 2023, 5:14 AM
This revision was landed with ongoing or failed builds.Mar 1 2023, 1:40 AM
This revision was automatically updated to reflect the committed changes.