This is an archive of the discontinued LLVM Phabricator instance.

[IR] Change vector.splice intrinsic to reject out-of-bounds indices
ClosedPublic

Authored by david-arm on Dec 17 2021, 4:47 AM.

Details

Summary

I've changed the definition of the experimental.vector.splice
instrinsic to reject indices that are known to be or possibly
out-of-bounds. In practice, this means changing the definition so that
the index is now only valid in the range [-VL, VL-1] where VL is the
known minimum vector length. We use the vscale_range attribute to
take the minimum vscale value into account so that we can permit
more indices when the attribute is present.

The splice intrinsic is currently only ever generated by the vectoriser,
which will never attempt to splice vectors with out-of-bounds values.
Changing the definition also makes things simpler for codegen since we
can always assume that the index is valid.

This patch was created in response to review comments on D115863

Diff Detail

Event Timeline

david-arm created this revision.Dec 17 2021, 4:47 AM
david-arm requested review of this revision.Dec 17 2021, 4:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 17 2021, 4:47 AM
sdesmalen added inline comments.Dec 20 2021, 7:10 AM
llvm/docs/LangRef.rst
17223–17228

this isn't strictly correct, for a <vscale x 4 x i32> and the IR being compiled with vscale_range(2, N) (for N>=2) then imm = -5 should still be a valid immediate.

It would be better to phrase this in terms of vscale_range directly.

llvm/lib/IR/Verifier.cpp
5366–5367

the number of elements in the vector for the vscale_range being compiled for.

david-arm updated this revision to Diff 397282.Jan 4 2022, 6:39 AM
  • Addressed review comments
david-arm marked 2 inline comments as done.Jan 4 2022, 6:40 AM
david-arm added inline comments.
llvm/docs/LangRef.rst
17223–17228

So I can only really refer to vscale_range for scalable vectors, since this intrinsic also works for fixed length vectors too. I've tried to be more specific for the scalable case.

llvm/lib/IR/Verifier.cpp
5366–5367

Same problem as above!

sdesmalen added inline comments.Jan 7 2022, 7:53 AM
llvm/docs/LangRef.rst
17224–17227

I don't think this is entirely right. What I think you want to say is:

The first two operands are vectors with the same type. The start index is imm modulo the runtime number of elements in the source vector.
For a fixed-width vector <N x eltty>, imm is a signed integer constant in the range -N <= imm < N.
For a scalable vector <vscale x N x eltty>, imm is a signed integer constant in the range -X <= imm < X where X=vscale_range_min * N.

david-arm updated this revision to Diff 398555.Jan 10 2022, 3:09 AM
david-arm marked 2 inline comments as done.
  • Updated LangRef documentation.
david-arm marked an inline comment as done.Jan 10 2022, 3:10 AM
sdesmalen accepted this revision.Jan 10 2022, 9:21 AM

LGTM!

llvm/lib/IR/Verifier.cpp
5368–5369

nit: the minimum number of elements is determined from vscale_range.

This revision is now accepted and ready to land.Jan 10 2022, 9:21 AM