Page MenuHomePhabricator

[SVE][CodeGen] Remove performMaskedGatherScatterCombine
ClosedPublic

Authored by kmclaughlin on Jan 12 2021, 9:52 AM.

Details

Summary

The AArch64 DAG combine added by D90945 & D91433 extends the index
of a scalable masked gather or scatter to i32 if necessary.

This patch removes the combine and instead adds shouldExtendGSIndex, which
is used by visitMaskedGather/Scatter in SelectionDAGBuilder to query whether
the index should be extended before calling getMaskedGather/Scatter.

Diff Detail

Event Timeline

kmclaughlin created this revision.Jan 12 2021, 9:52 AM
kmclaughlin requested review of this revision.Jan 12 2021, 9:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 12 2021, 9:52 AM
david-arm added inline comments.Jan 13 2021, 2:31 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
4347

Do we want to switch between zero, sign or any extend depending upon what the target wants? I noticed in the original perform...Combine code we chose SIGN or ZERO extend based on the isIndexSigned() result - not sure if we still need something similar here or not.

kmclaughlin added inline comments.Jan 28 2021, 5:38 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
4347

Hi @david-arm, I chose sign extension here because the index type at this point is set to the default of SIGNED_UNSCALED. Though if the index here is a zext node, getNode() will return an ISD::ZERO_EXTEND instead, so I think the correct extension will still be applied.

david-arm accepted this revision.Jan 28 2021, 5:45 AM

LGTM!

llvm/include/llvm/CodeGen/TargetLowering.h
1321

nit: Perhaps make these comments /// instead for documentation support?

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
4347

OK, thanks a lot for looking into this!

This revision is now accepted and ready to land.Jan 28 2021, 5:45 AM
This revision was automatically updated to reflect the committed changes.
kmclaughlin marked an inline comment as done.