Page MenuHomePhabricator

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

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



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.


getVectorElementCount? (Same in other places.)


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.Mar 11 2020, 1:09 PM

Please write out the type ElementCOunt.


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.Mar 12 2020, 11:50 AM
huihuiz marked 3 inline comments as done.

Addressed review comments.


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.Mar 12 2020, 12:26 PM
This revision was automatically updated to reflect the committed changes.