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

Sun Feb 6 06:07:02 PST 2022

dmgreen added a comment.

Sorry for the delay - I hadn't realised this was something I should look at.  I can make the change blocking you here. Some of the other results look larger, but I'm not sure I understand how the patch is changing things enough to know why. The Thumb2 tests that change are not super important in themselves (other than they don't crash!). They are not expected to come up commonly in practice, but it might show something unexpected is happening. Either with this patch or the way Arm tries to lower buildvectors/shuffles.

================
Comment at: llvm/lib/Analysis/VectorUtils.cpp:529
+        if (FirstReg < 0)
+          FirstReg = SrcRegIdx;
+        RegMasks[SrcRegIdx].assign(SzDest, UndefMaskElem);
----------------
This FirstReg doesn't appear to be used anywhere.

================
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);
----------------
ABataev wrote:
> craig.topper wrote:
> > Does ARM already have code that just does the right thing for these types?
> Actually, need advice here. Without this change, there is an infinite loop for ARM tests for `v2f64` type. VECTOR_SHUFFLE is assigned Expand action, so it is expanded, then it is transformed back to vector (`setOperationAction(ISD::SCALAR_TO_VECTOR, MVT::v2f64, Legal);`), then again expanded, etc. I assume that it is legal for `v2f64` type if `ISD::SCALAR_TO_VECTOR` is legal. But better to have some input here from ARM folks.
The change looks like it will help, especially for i64 types that can now go via ARMISD::BUILD_VECTOR. We should make this change separately though. I'll see if I can do that today.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115653