This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SVE] Add optimization to remove redundant ptest instructions
ClosedPublic

Authored by bsmith on Dec 15 2020, 3:57 AM.

Details

Summary

This patch adds a peepholing optimization that removes extra ptest instructions where they are not required, for example:

  • ptest of true and another existing condition
  • ptest of the same condition twice

Co-Authored-by: Graham Hunter <graham.hunter@arm.com>
Co-Authored-by: Paul Walker <paul.walker@arm.com>

Diff Detail

Event Timeline

bsmith created this revision.Dec 15 2020, 3:57 AM
bsmith requested review of this revision.Dec 15 2020, 3:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 15 2020, 3:57 AM

Other than some missing tests this looks reasonable to me.

llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
1341–1365

I cannot see any unit tests for this block of functionality.

1394–1395

What about:

If we pass all the checks, it's safe to remove the PTEST and either use the flags as they are prior to PTEST. Sometimes this requires the tested PTEST operand to be replaced with an equivalent instruction that also sets the flags.

paulwalker-arm added inline comments.Dec 17 2020, 4:07 AM
llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
1394–1395

Updated to remove rouge "either".

What about:

If we pass all the checks, it's safe to remove the PTEST and use the flags as they are prior to PTEST. Sometimes this requires the tested PTEST operand to be replaced with an equivalent instruction that also sets the flags.

bsmith updated this revision to Diff 312745.Dec 18 2020, 3:31 AM
  • Add missing tests for BRK* cases
  • Improve misleading comment
paulwalker-arm accepted this revision.Dec 18 2020, 5:42 AM
paulwalker-arm added inline comments.
llvm/test/CodeGen/AArch64/sve-ptest-removal-brk.ll
37

For the _neg tests it would be nice to add a comment to outline what behaviour the test is protecting against.

This revision is now accepted and ready to land.Dec 18 2020, 5:42 AM
bsmith updated this revision to Diff 314366.Jan 4 2021, 5:13 AM
  • Add comments to brk testcase to note what is being tested
This revision was landed with ongoing or failed builds.Jan 5 2021, 7:30 AM
This revision was automatically updated to reflect the committed changes.