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

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:7876
+  } else
+    Index =
+        DAG.getNode(ISD::MUL, dl, IdxVT, Index,
----------------
sdesmalen wrote:
> CarolineConcatto wrote:
> > Can you do something like this:
> > 
> > ```
> > 
> > Index = DAG.getVScale(DL, IdxVT, APInt(IdxVT.getSizeInBits(), Index.getConstantOperandVal(0)));
> > ```
> > ?
> > Insead of multiply the index by a scalable vector of size 1?
> I'm not sure if you can always know for sure that Index is a constant value so that's why I used an explicit multiply with vscale.
To me the rational for not using `getConstantOperandAPInt` is at odds to the rational for not clamping the index.  None of what you say is wrong but there's nothing to say this function has to be used in conjunction with `ISD::INSERT_SUBVECTOR`, but the function's description does say:
```
If \p Idx plus the size of \p SubVecVT is out of bounds the returned pointer is unspecified, but the value returned will be such that the entire subvector would be within the vector bounds.
```
So either this function is only ever used in combination with `EXTRACT_SUBVECTOR/INSERT_SUBVECTOR`, in which case we can assume `Index` to be a constant (perhaps even changing the prototype to force this?), or this is a generic helper function and thus `Index` can be anything and must be clamped to honour the function's description.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111633