[PATCH] D111633: [SelectionDAG] Fix getVectorSubVecPointer for scalable subvectors.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7873
            "Sub-vector must be a fixed vector with matching element type");
     Index = clampDynamicVectorIndex(DAG, Index, VecVT, dl,
                                     SubVecVT.getVectorNumElements());
----------------
david-arm wrote:
> sdesmalen wrote:
> > david-arm wrote:
> > > Hi @sdesmalen, it's just a thought, but while you're in this area is it also worth clamping the index for scalable vectors too? The comment above is incorrect, because we do explicitly clamp the index in other places for scalable vectors.
> > I don't think any clamping is required, because when both the subvector and the vector being inserted into are scalable, we know at compiletime whether the vector index will exceed the size of the input vector.
> Oh ok - I wonder why we do this for fixed-length vectors? I was sort of expecting the problem to be the same for both inserting fixed into fixed and inserting scalable into scalable?
> 
> I was specifically worried about what we did in practice for this case:
> 
> call <vscale x 8 x half> @llvm.experimental.vector.insert.nxv8f16.nxv2f16(<vscale x 8 x half> %vec, <vscale x 2 x half> %in, i64 10)
> 
> because if vscale=1 then we're inserting beyond the end of the vector.
> 
This exists for inserting/extracting fixed from scalable, where we don't know at compile-time if the fixed offset exceeds the scalable vector.

The example you give here:

  call <vscale x 8 x half> @llvm.experimental.vector.insert.nxv8f16.nxv2f16(<vscale x 8 x half> %vec, <vscale x 2 x half> %in, i64 10)

is always out of bounds, because `vscale*10 > vscale*(8-2)` for any vscale.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111633/new/

https://reviews.llvm.org/D111633