Page MenuHomePhabricator

[SVE] Update API ConstantVector::getSplat() to use ElementCount.
ClosedPublic

Authored by huihuiz on Feb 10 2020, 11:11 PM.

Details

Summary

Support ConstantInt::get() and Constant::getAllOnesValue() for scalable
vector type, this requires ConstantVector::getSplat() to take in 'ElementCount',
instead of 'unsigned' number of element count.

This change is needed for D73753.

Diff Detail

Event Timeline

huihuiz created this revision.Feb 10 2020, 11:11 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 10 2020, 11:11 PM

This is sort of a large number of functional changes, but it's probably okay to skip test coverage for all the simple getNumElements->getElementCount changes; it's obvious it can't break anything.

llvm/include/llvm/Analysis/Utils/Local.h
67

getVectorElementCount? (Same in other places.)

llvm/lib/IR/Constants.cpp
2129

I'd like to see test coverage for the GEP changes here, since the logic is relatively intricate.

huihuiz updated this revision to Diff 244546.Feb 13 2020, 4:17 PM
huihuiz marked 2 inline comments as done.

Thanks Eli for the review!

Add test cases to check for size query changes in ConstantExpr::getGetElementPtr().

efriedma added inline comments.Wed, Mar 11, 1:09 PM
clang/lib/CodeGen/CGBuiltin.cpp
4497

Please write out the type ElementCOunt.

llvm/lib/CodeGen/CodeGenPrepare.cpp
6542

Please write out the type ElementCount.

This is unfortunately turning the explicit assertion if the type is scalable into a later verifier failure in the case where it isn't a splat. Please either fix it properly, or change it so the non-splat codepath still asserts.

huihuiz updated this revision to Diff 250001.Thu, Mar 12, 11:50 AM
huihuiz marked 3 inline comments as done.

Addressed review comments.

llvm/lib/CodeGen/CodeGenPrepare.cpp
6542

Good catch! Thanks Eli!

Going with assert for non-splat codepath for scalable vector.

We should implement like:

UndefValue *UndefVal = UndefValue::get(getTransitionType());
Type *I32Ty = Type::getInt32Ty(getTransitionType()->getContext());
return ConstantExpr::getInsertElement(UndefVal, Val, ConstantInt::get(I32Ty, ExtractIdx));

But current target lowering will reject scalable vector earlier while checking isTypeLegal(EVT VT).

I am adding a test to check this. So we get assert once target lowering is ready. Then I can bring in this implementation and check for its correctness.

define void @simpleOneInstructionPromotion(<vscale x 2 x i32>* %addr1, i32* %dest) {
  %in1 = load <vscale x 2 x i32>, <vscale x 2 x i32>* %addr1, align 8
  %extract = extractelement <vscale x 2 x i32> %in1, i32 1
  %out = or i32 %extract, 1
  store i32 %out, i32* %dest, align 4
  ret void
}
This revision is now accepted and ready to land.Thu, Mar 12, 12:26 PM
This revision was automatically updated to reflect the committed changes.