Page MenuHomePhabricator

[ARM] Expand VMOVRRD simplification pattern
ClosedPublic

Authored by dmgreen on Apr 10 2021, 4:36 AM.

Details

Summary

This expands the VMOVRRD(extract(..(build_vector(a, b, c, d)))) pattern, to also handle insert_vectors. Providing we can find the correct insert, this helps further simplify patterns by removing the redundant VMOVRRD.

Diff Detail

Event Timeline

dmgreen created this revision.Apr 10 2021, 4:36 AM
dmgreen requested review of this revision.Apr 10 2021, 4:36 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 10 2021, 4:36 AM
dmgreen added inline comments.Apr 10 2021, 4:38 AM
llvm/test/CodeGen/ARM/dagcombine-anyexttozeroext.ll
47

This is a bit unfortunate, but if you run the code through opt it is already simplified to a single zext, which still becomes a vmovl; vmovl pair

SjoerdMeijer added inline comments.Apr 20 2021, 5:43 AM
llvm/test/CodeGen/ARM/dagcombine-anyexttozeroext.ll
47

Just for my understanding, what does that mean? Is this something we have to live with, or can we fix this regression?

dmgreen added inline comments.Apr 20 2021, 10:20 AM
llvm/test/CodeGen/ARM/dagcombine-anyexttozeroext.ll
47

I don't think it's a real regressions. The mid-end of llvm will have turned it into other code already, which we already optimize to the pair of vmovl's. The test has a lot of inserts and extract's which are unlikely to come up in reality.

Last question: do we have BE tests for this?

llvm/test/CodeGen/ARM/dagcombine-anyexttozeroext.ll
47

Ok, thanks for clarifying.

dmgreen updated this revision to Diff 339764.Apr 22 2021, 1:06 PM

I hadn't forgotten to _test_ BE, but yeah none of these tests were BE. I've added one.

SjoerdMeijer accepted this revision.Apr 23 2021, 12:57 AM

Thanks, LGTM

This revision is now accepted and ready to land.Apr 23 2021, 12:57 AM
This revision was landed with ongoing or failed builds.Apr 26 2021, 4:28 AM
This revision was automatically updated to reflect the committed changes.