Page MenuHomePhabricator

[SelectionDAG] Remove duplicate "is scaled" information from gather/scatter SDNodes.
ClosedPublic

Authored by paulwalker-arm on Apr 8 2022, 5:09 AM.

Details

Summary

During early early gather/scatter enablement two different
approaches were taken to represent scaled indices:

  • A Scale operand whereby byte_offsets = Index * Scale
  • An IndexType whereby byte_offsets = Index * sizeof(MemVT.ElementType)

Having multiple representations is bad as shown by this patch which
fixes instances where the two are out of sync. The dedicated scale
operand is more flexible and pervasive so this patch removes the
UNSCALED values from IndexType. This means all indices are scaled
but the scale can be one, hence unscaled. SDNodes now use the scale
operand to answer the "isScaledIndex" question.

I toyed with the idea of keeping the UNSCALED enums and helper
functions but because they will have no uses and force SDNodes to
validate the set of supported values I figured it's best to remove
them. We can re-add them if there's a real need. For similar
reasons I've kept the IndexType enum when a bool could be used as I
think being explicitly looks better.

Depends On D123347

Diff Detail

Event Timeline

paulwalker-arm created this revision.Apr 8 2022, 5:09 AM
paulwalker-arm requested review of this revision.Apr 8 2022, 5:09 AM
paulwalker-arm added inline comments.Apr 30 2022, 4:25 PM
llvm/include/llvm/CodeGen/ISDOpcodes.h
1370–1371

With scaling removed, there's a decision to be made regarding whether to keep the IndexType enum or just represent the signed information as a bool. So I figured I'd maintain these now seemingly redundant helper functions until the best option is decided.

david-arm added inline comments.May 9 2022, 4:06 AM
llvm/include/llvm/CodeGen/ISDOpcodes.h
1370–1371

Maybe you can kill off these functions getSignedIndexType and getUnsignedIndexType at least?

1371

nit: I think this should be equivalent?

Rebased, fixed typo and removed now mostly redundant getSignedIndexType and getUnsignedIndexType helpers.

This revision is now accepted and ready to land.May 16 2022, 6:20 AM
This revision was landed with ongoing or failed builds.May 16 2022, 1:01 PM
This revision was automatically updated to reflect the committed changes.