Page MenuHomePhabricator

[SVE] Allow scalable vector splats to be gep indices
AbandonedPublic

Authored by ctetreau on Jun 10 2020, 4:06 PM.

Details

Summary

getSplatValue(): Make getSplatValue work for scalable vector splats
of the form returned by ConstantVector::getSplat().

getUniqueInteger(): The value returned by getSplatValue() is the same
value that is returned by getAggregateElement(0). No need to have the
second call, and this allows this function to work for scalable splats.

getIndexType() and getCastOpcode(): These can call getElementCount() and
VectorType::get(Type *, VectorType *)

Added test for gep using scalable vector splat

Diff Detail

Event Timeline

ctetreau created this revision.Jun 10 2020, 4:06 PM
Herald added a project: Restricted Project. · View Herald Transcript
efriedma requested changes to this revision.Jun 10 2020, 4:24 PM

I'll assume you didn't mean to post this for review

This revision now requires changes to proceed.Jun 10 2020, 4:24 PM
ctetreau updated this revision to Diff 271475.Jun 17 2020, 1:58 PM

fixed issues with previous implementation, used new functionality to enable scalable vector indices to GEP, and added test

ctetreau retitled this revision from [SVE] Make getSplatValue work for some scalable vector splats to [SVE] Allow scalable vector splats to be gep indices.Jun 17 2020, 2:33 PM
ctetreau edited the summary of this revision. (Show Details)

Posted https://reviews.llvm.org/D82061 to try to make the GEP-related bits more sane.

llvm/lib/IR/Constants.cpp
1567

Constant::getUniqueInteger() is a terrible API. I'd prefer to fix the callers so it doesn't need to handle scalable vectors.

ctetreau updated this revision to Diff 272845.Jun 23 2020, 3:19 PM
ctetreau edited the summary of this revision. (Show Details)

Split getSplatVal stuff into its own commit, rebase

@efriedma I think so long as the language reference states that gep into a struct accepts any vector splat index, then this patch should be accepted. (assuming it has no technical issues) It's a trivial change, that is probably an improvement regardless of if we consider the use case to be valid.

ctetreau marked an inline comment as done.Jun 23 2020, 3:28 PM
ctetreau added inline comments.
llvm/lib/IR/Constants.cpp
1567

That may be true, but my version is better than what existed previously.

ctetreau abandoned this revision.Jul 20 2020, 3:06 PM