[PATCH] D145698: [IR][Legalizations] Widen illegal deinterleave and interleave vectors

[Matt Devereau via Phabricator via llvm-commits]

MattDevereau added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:5023-5051
+  if (WidenVT.isScalableVector()) {
+    // Break down the inputs such that we can concatenate individual parts
+    // into one wider vector, e.g.
+    //
+    // nxv6i32 = concat(nxv3i32 t0, nxv3i32 t1)
+    // =>
+    // nxv8i32 = concat(nxv1i32 (extract.. t0),
----------------
I'm not sure if i'm missing something but if i remove this whole block and run check-all tests I don't see anything failing. Is this missing a test case?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:5755
+SDValue DAGTypeLegalizer::WidenVecRes_VECTOR_DEINTERLEAVE(SDNode *N) {
+  SDLoc dl(N);
+  EVT InVT = N->getValueType(0);
----------------
DL?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:11592
   EVT InVT = getValue(I.getOperand(0)).getValueType();
+  EVT WideVT = InVT;
   SDValue InVec0 = getValue(I.getOperand(0));
----------------
This can be defined next to where it's used. Is PackedVT or LegalTY a better name for this? If InVT is `nxv3i32` then `WideVT = TLI.getTypeToTransformTo(Ctx, InVT);` will return `nxv4i32` which I found quite surprising


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:11624
+  // Return to the original vector size, before widening.
+  if (InVT.getVectorElementCount() != WideVT.getVectorElementCount()) {
+    OutVT = TLI.getValueType(DAG.getDataLayout(), I.getType());
----------------
The way i'm reading this if statement is "before we had to widen InVT to WideTV". If we do something like:
```
bool InVTMustWiden = (TLI.getTypeAction(Ctx, InVT) == TargetLowering::TypeWidenVector);
if (InVTMustWiden) {
  WideVT = TLI.getTypeToTransformTo(Ctx, InVT);
  ...
...
...
if (InVTMustWiden) {
  OutVT = TLI.getValueType(DAG.getDataLayout(), I.getType());
...

```
Do you think that makes it easier to understand?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145698