Page MenuHomePhabricator

[AArch64][SVE] Handle more cases in findMoreOptimalIndexType.
ClosedPublic

Authored by sdesmalen on Feb 14 2022, 8:17 AM.

Details

Summary

This patch addresses @paulwalker-arm's comment on D117900 to
only update/write the by-ref operands iff the function returns
true. It also handles a few more cases where a series of added
offsets can be folded into the base pointer, rather than just looking
at a single offset.

Diff Detail

Event Timeline

sdesmalen created this revision.Feb 14 2022, 8:17 AM
sdesmalen requested review of this revision.Feb 14 2022, 8:17 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 14 2022, 8:17 AM
llvm/test/CodeGen/AArch64/sve-gather-scatter-addr-opts.ll
302

If I switch this the fold does not happen. I know this was not supported before, but should we try to support now?

sdesmalen added inline comments.Feb 22 2022, 1:55 AM
llvm/test/CodeGen/AArch64/sve-gather-scatter-addr-opts.ll
302

Good spot, but I don't think we want to fix that in this patch. This requires changes to SelectionDAG::canonicalizeCommutativeBinop, which was left as follow-up work from D118459. This is better done separately from this patch.

paulwalker-arm added inline comments.Feb 24 2022, 5:12 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
16490

Is Index guaranteed to be a vector of i64s? For the general case I believe it can be any type so you'll either need an early exit if the scenario is unlikely or some kind of extension if it's something you care about, otherwise code like this will assert.

16510–16511

I guess it doesn't matter arithmetically? but is there a reason to switch from the original (Offset << Shift) * Scale to (Offset * Scale) << Shift?

sdesmalen updated this revision to Diff 411108.Feb 24 2022, 6:41 AM
sdesmalen marked an inline comment as done.

Added condition to findMoreOptimalIndexType that index is a vector of
i64's and swapped order of shl/mul since scale is guaranteed to be constant.

paulwalker-arm accepted this revision.Feb 24 2022, 7:46 AM
paulwalker-arm added inline comments.
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
16507

Perhaps splat(Shift) to keep the before and after consistent.

16559

getZExtValue? Shift amounts are always unsigned.

This revision is now accepted and ready to land.Feb 24 2022, 7:46 AM
sdesmalen marked 4 inline comments as done.Feb 28 2022, 4:16 AM
This revision was landed with ongoing or failed builds.Feb 28 2022, 4:16 AM
This revision was automatically updated to reflect the committed changes.