This is an archive of the discontinued LLVM Phabricator instance.

[ISD::IndexType] Helper functions for common queries.
ClosedPublic

Authored by paulwalker-arm on Apr 7 2022, 5:28 PM.

Details

Summary

Add helper functions to query the signed and scaled properties
of ISD::IndexType along with functions to change them.

Remove setIndexType from MaskedGatherSDNode because it only has
one usage and typically should only be changed alongside its
index operand.

Minimise the direct use of the enum values to lay the groundwork
for more refactoring.

Depends On D123326

Diff Detail

Event Timeline

paulwalker-arm created this revision.Apr 7 2022, 5:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 7 2022, 5:28 PM
paulwalker-arm requested review of this revision.Apr 7 2022, 5:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 7 2022, 5:28 PM
david-arm added inline comments.May 3 2022, 1:33 AM
llvm/include/llvm/CodeGen/ISDOpcodes.h
1377

For symmetry is it worth having a getScaledIndexType too that returns SIGNED_SCALED or UNSIGNED_SCALED?

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
10481

Removing this looks like an actual function change, not just refactoring. It looks sensible, but it's not obvious from the commit message that you're also changing the behaviour. After your patch we won't change the index type unless we manage to refine the index.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
4714–4715

Given you already have the IndexType as a variable, can you just call the ISD function helper functions instead? For example,

bool IsScaled = ISD::isIndexTypeScaled(IndexType);
bool IsSigned = ISD::isIndexTypeSigned(IndexType);
4813–4814

Same comment as before about using the ISD helper functions directly.

paulwalker-arm added inline comments.May 3 2022, 3:40 AM
llvm/include/llvm/CodeGen/ISDOpcodes.h
1377

D123381 removes the scaling property so it didn't seem worth adding an unused function only to later remove it.

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
10481

The new else block should maintain the original behaviour? The only difference it that is doesn't overwrite MSG's IndexType with the same value it already has.

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
4714–4715

D123381 removes the scaling property and so querying MGT directly becomes the only way to know if the index is scaled.

david-arm accepted this revision.May 3 2022, 3:46 AM

LGTM!

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
10481

Oh I see what you mean.

This revision is now accepted and ready to land.May 3 2022, 3:46 AM