[PATCH] D115653: [DAG]Introduce llvm::processShuffleMasks and use it for shuffles in DAG Type Legalizer.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:2042
   SDValue Inputs[4];
-  SDLoc dl(N);
+  SDLoc Dl(N);
   GetSplitVector(N->getOperand(0), Inputs[0], Inputs[1]);
----------------
If you change the case here. Use `DL`. As far as I know SelectionDAG uses `DL` or `dl`.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:2055
     // Build a shuffle mask for the output, discovering on the fly which
     // input vectors to use as shuffle operands (recorded in InputUsed).
     unsigned FirstMaskIdx = High * NewElts;
----------------
InputUsed no longer exists


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:2063
+        /*NumOfUsedRegs=*/1,
+        [&Output, this, NewVT]() { Output = DAG.getUNDEF(NewVT); },
+        [&Output, this, NewVT, &Dl, &Inputs](ArrayRef<int> Mask, unsigned Idx) {
----------------
Is `this` only used for DAG?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:2070
+         &InitManyRegs](ArrayRef<int> Mask, unsigned Idx1, unsigned Idx2) {
+          if (InitManyRegs) {
+            Output = Inputs[Idx1];
----------------
Can this use `SDValue::operator bool()` to see if it has already been assigned instead of using a separate flag.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:394
     setOperationAction(ISD::BUILD_VECTOR, VT, Custom);
+    setOperationAction(ISD::VECTOR_SHUFFLE, VT, Custom);
     setOperationAction(ISD::VSELECT, VT, Legal);
----------------
Does ARM already have code that just does the right thing for these types?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115653