[PATCH] D87976: Support the division-by-constant strength reduction for more integer types

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:4954
   EVT SVT = VT.getScalarType();
-  EVT ShVT = getShiftAmountTy(VT, DAG.getDataLayout());
+  EVT ShVT = getShiftAmountTy(VT, DAG.getDataLayout(), IsAfterLegalization);
   EVT ShSVT = ShVT.getScalarType();
----------------
IsAfterLegalization refers to LegalOperations not LegalTypes.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:4959
   // Check to see if we can do this.
-  // FIXME: We should be more aggressive here.
-  if (!isTypeLegal(VT))
+  if (IsAfterLegalization && !isTypeLegal(VT))
     return SDValue();
----------------
I believe IsAfterLegalization refers to LegalOperations rather than LegalTypes.

But if we get here after type legalization then the VT must be Legal or it wouldn't have been seen by DAGCombiner to call this so we might just be able to remove this whole check.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:5029
+  } else {
+    if (IsAfterLegalization)
+      return SDValue();
----------------
This needs to be a check for LegalTypes not LegalOperations.


================
Comment at: llvm/test/CodeGen/X86/vshli-simplify-demanded-bits.ll:11
 ; CHECK:       # %bb.0: # %bb
-; CHECK-NEXT:    movdqa {{.*#+}} xmm0 = [2863311531,2863311531,2863311531,2863311531]
-; CHECK-NEXT:    pshufd {{.*#+}} xmm3 = xmm1[1,1,3,3]
-; CHECK-NEXT:    pmuludq %xmm0, %xmm1
-; CHECK-NEXT:    pshufd {{.*#+}} xmm4 = xmm1[1,3,2,3]
-; CHECK-NEXT:    pmuludq %xmm0, %xmm3
-; CHECK-NEXT:    pshufd {{.*#+}} xmm1 = xmm3[1,3,2,3]
-; CHECK-NEXT:    punpckldq {{.*#+}} xmm4 = xmm4[0],xmm1[0],xmm4[1],xmm1[1]
-; CHECK-NEXT:    pshufd {{.*#+}} xmm1 = xmm2[1,1,3,3]
-; CHECK-NEXT:    pmuludq %xmm0, %xmm2
-; CHECK-NEXT:    pshufd {{.*#+}} xmm2 = xmm2[1,3,2,3]
-; CHECK-NEXT:    pmuludq %xmm0, %xmm1
-; CHECK-NEXT:    pshufd {{.*#+}} xmm0 = xmm1[1,3,2,3]
-; CHECK-NEXT:    punpckldq {{.*#+}} xmm2 = xmm2[0],xmm0[0],xmm2[1],xmm0[1]
+; CHECK-NEXT:    pshufd {{.*#+}} xmm3 = xmm1[2,3,3,3]
+; CHECK-NEXT:    movdqa {{.*#+}} xmm4 = [2863311531,2863311531]
----------------
Why did the amount of code increase here? This is a legal type so why was it affected?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87976