This is an archive of the discontinued LLVM Phabricator instance.

[SVE][CodeGen] Fix TypeSize/ElementCount related warnings in sve-split-load.ll
ClosedPublic

Authored by david-arm on Aug 27 2020, 4:41 AM.

Details

Summary

I have fixed up a number of warnings resulting from TypeSize -> uint64_t
casts and calling getVectorNumElements() on scalable vector types. I
think most of the changes are fairly trivial except for those in
DAGTypeLegalizer::SplitVecRes_MLOAD - I've tried to ensure we create
the MachineMemoryOperands in a sensible way for scalable vectors.

I have added a CHECK line to the following test:

CodeGen/AArch64/sve-split-load.ll

that ensures no new warnings are added.

Diff Detail

Event Timeline

david-arm created this revision.Aug 27 2020, 4:41 AM
Herald added a project: Restricted Project. · View Herald Transcript
david-arm requested review of this revision.Aug 27 2020, 4:41 AM
efriedma added inline comments.Aug 27 2020, 2:26 PM
llvm/include/llvm/Support/TypeSize.h
249

This name is possibly misleading; the runtime size might not be a power of two even if isPowerOf2() returns true.

In the one place you're using it, the type can't be a vector type, anyway.

llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
1663

I thought we added some form of pointer increment that took care of the MachinePointerInfo? Or am I misremembering?

Semantically, this looks correct.

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
9646

Should we add a comparison operator to ElementCount, like the one we have for TypeSize?

david-arm updated this revision to Diff 288558.Aug 28 2020, 2:17 AM
  • Rebased on top of my ElementCount refactor patch.
  • Removed isPowerOf2() member function from ElementCount.
david-arm marked 3 inline comments as done.Aug 28 2020, 2:22 AM
david-arm added inline comments.
llvm/include/llvm/Support/TypeSize.h
249

Good point and I've removed this function so it's calculated explicitly in the code as before

llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
1663

We do have IncrementPointer, but it sadly doesn't do the same thing. The code above increments the pointer using IncrementMemoryAddress, which attempts to deal with compressed memory based upon the mask.

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
9646

Based on discussions in the parent patch (https://reviews.llvm.org/D86065) I think the consensus is to avoid the use of operators for comparisons. Perhaps we can revisit this at some point when we have some kind of agreed set of interfaces?

efriedma accepted this revision.Aug 31 2020, 3:03 PM

LGTM with one minor comment.

llvm/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp
767

isPowerOf2_64() so you don't have implicit truncation.

This revision is now accepted and ready to land.Aug 31 2020, 3:03 PM