Page MenuHomePhabricator

[llvm][AArch64] Allow TB(N)Z to drop signext for sign bit tests.
ClosedPublic

Authored by fpetrogalli on Nov 2 2020, 6:02 AM.

Details

Summary

For example if the sign extension is only used in for TBZ, and the value is used elsewhere with a zero extension, this can eliminate a sign extension.

Diff Detail

Event Timeline

fpetrogalli created this revision.Nov 2 2020, 6:02 AM
fpetrogalli requested review of this revision.Nov 2 2020, 6:02 AM

Do we need to handle the same pattern for non IN_REG variations?

Thank you for the review @resistor. I have implememted the
optimization also looking through a sign_extend node. Tests to cover
this have been added, but only for extending i32 into i64 because
illegal values like i8 are always sign extended inreg.

Nit: I think the commit message could be improved such that the benefit is clear? Something referencing the fact you're modifying the behaviour of TBZ.

Suggestion: "Allow TB(N)Z to drop signext for sign bit tests"

With the body explaining why/circumstances this helps: "For example if the sign extension is only used in for TBZ, and the value is used elsewhere with a zero extension, this can eliminate a sign extension".

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
5810

sp. returns

fpetrogalli retitled this revision from [llvm][AArch64] Check the sign bit of a sign extended value before sign extension. to [llvm][AArch64] Allow TB(N)Z to drop signext for sign bit tests..Nov 3 2020, 5:34 AM
fpetrogalli edited the summary of this revision. (Show Details)

Fix typo in comment.

fpetrogalli marked an inline comment as done.Nov 3 2020, 5:58 AM
samparker accepted this revision.Nov 4 2020, 12:29 AM

Thanks, LGTM

This revision is now accepted and ready to land.Nov 4 2020, 12:29 AM
This revision was landed with ongoing or failed builds.Nov 9 2020, 10:27 AM
This revision was automatically updated to reflect the committed changes.