[llvm] [SelectionDAG] Scalarize binary ops of splats before legal types (PR #100749)

Luke Lau via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 7 01:54:04 PDT 2024


================
@@ -26993,11 +26993,20 @@ static SDValue scalarizeBinOpOfSplats(SDNode *N, SelectionDAG &DAG,
   // TODO: use DAG.isSplatValue instead?
   bool IsBothSplatVector = N0.getOpcode() == ISD::SPLAT_VECTOR &&
                            N1.getOpcode() == ISD::SPLAT_VECTOR;
+
+  // If binop is legal or custom on EltVT, scalarize should be profitable. The
+  // check is the same as isOperationLegalOrCustom without isTypeLegal. We
+  // can do this only before LegalTypes, because it may generate illegal `op
+  // EltVT` from legal `op VT (splat EltVT)`, where EltVT is not legal type but
+  // the result type of splat is legal.
+  auto EltAction = TLI.getOperationAction(Opcode, EltVT);
   if (!Src0 || !Src1 || Index0 != Index1 ||
       Src0.getValueType().getVectorElementType() != EltVT ||
       Src1.getValueType().getVectorElementType() != EltVT ||
       !(IsBothSplatVector || TLI.isExtractVecEltCheap(VT, Index0)) ||
-      !TLI.isOperationLegalOrCustom(Opcode, EltVT))
+      (LegalTypes && !TLI.isTypeLegal(EltVT)) ||
+      !(EltAction == TargetLoweringBase::Legal ||
+        EltAction == TargetLoweringBase::Custom))
----------------
lukel97 wrote:

Sorry for the delay here: I took another look at this and I think this might actually be working by coincidence.

I'm not sure it makes much sense to check if an operation is legal for an illegal type. I think this just happens to work because by default all op actions are initiated to legal. 

I think what we need to do is if we're before legal types, we check if the operation is legal on the legalized type:

```suggestion
      // If before type legalization, allow scalar types that will eventually be
      // made legal.
      !TLI.isOperationLegalOrCustom(
          Opcode, LegalTypes
                      ? EltVT
                      : TLI.getTypeToTransformTo(*DAG.getContext(), EltVT)))
```

I tried this out locally and this actually fixed that `vmul_xx_nxv8i64` test case.

https://github.com/llvm/llvm-project/pull/100749


More information about the llvm-commits mailing list