[X86][SSE] Disable shouldFoldConstantShiftPairToMask for btver1/btver2 targets (PR40758)

[RKSimon]

Summary

As detailed on PR40758, Bobcat/Jaguar can perform vector immediate shifts on the same pipes as vector ANDs with the same latency - so it doesn't make sense to replace a shl+lshr with a shift+and pair as it requires an additional mask (with the extra constant pool, loading and register pressure costs).

[spatel]

lib/Target/X86/X86.td

427–430 ↗

(On Diff #196446)

Is there a possibility that we would use this for scalar transforms too? If not, better to make this explicitly about vectors:
"FeatureFastVectorShifts" ?

[RKSimon]

lib/Target/X86/X86.td

427–430 ↗

(On Diff #196446)

Yes AMD targets at least should benefit from the scalar case as well - I'll investigate.

[spatel]

LGTM

llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

if (ConstantSDNode *N0C1 = isConstOrConstSplat(N0.getOperand(1))) {
                          DAG.getConstant(C1 - C2, DL, N1.getValueType()));
     }
   }
 
   // fold (shl (srl x, c1), c2) -> (and (shl x, (sub c2, c1), MASK) or
   //                               (and (srl x, (sub c1, c2), MASK)
   // Only fold this if the inner shift has no other uses -- if it does, folding
   // this will increase the total number of instructions.
+  // TODO - drop hasOneUse requirement if c1 == c2?
+  // TODO - support non-uniform vector shift amounts.
   if (N1C && N0.getOpcode() == ISD::SRL && N0.hasOneUse() &&
       TLI.shouldFoldConstantShiftPairToMask(N, Level)) {
     if (ConstantSDNode *N0C1 = isConstOrConstSplat(N0.getOperand(1))) {
       if (N0C1->getAPIntValue().ult(OpSizeInBits)) {
         uint64_t c1 = N0C1->getZExtValue();
         uint64_t c2 = N1C->getZExtValue();
         APInt Mask = APInt::getHighBitsSet(OpSizeInBits, OpSizeInBits - c1);
         SDValue Shift;
if (auto N001C = isConstOrConstSplat(N0.getOperand(0).getOperand(1))) {
                                        N0.getOperand(0).getOperand(0),
                                        DAG.getConstant(c1 + c2, DL,
                                                        ShiftCountVT)));
       }
     }
   }
 
   // fold (srl (shl x, c), c) -> (and x, cst2)
+  // TODO - (srl (shl x, c1), c2).
   if (N0.getOpcode() == ISD::SHL && N0.getOperand(1) == N1 &&
       isConstantOrConstantVector(N1, /* NoOpaques */ true)) {
     SDLoc DL(N);
     SDValue Mask =
         DAG.getNode(ISD::SRL, DL, VT, DAG.getAllOnesConstant(DL, VT), N1);
     AddToWorklist(Mask.getNode());
     return DAG.getNode(ISD::AND, DL, VT, N0.getOperand(0), Mask);
   }

llvm/trunk/lib/Target/X86/X86.td

 // instructions if a CPU implements horizontal operations (introduced with
 // SSE3) with better latency/throughput than the alternative sequence.
 def FeatureFastHorizontalOps
     : SubtargetFeature<
         "fast-hops", "HasFastHorizontalOps", "true",
         "Prefer horizontal vector math instructions (haddp, phsub, etc.) over "
         "normal vector instructions with shuffles", [FeatureSSE3]>;
 
+def FeatureFastVectorShiftMasks
+    : SubtargetFeature<
+        "fast-vector-shift-masks", "HasFastVectorShiftMasks", "true",
+        "Prefer a left/right vector logical shift pair over a shift+and pair">;
+
 // Merge branches using three-way conditional code.
 def FeatureMergeToThreeWayBranch : SubtargetFeature<"merge-to-threeway-branch",
                                         "ThreewayBranchProfitable", "true",
                                         "Merge branches to a three-way "
                                         "conditional branch">;
 
 // Bonnell
 def ProcIntelAtom : SubtargetFeature<"", "X86ProcFamily", "IntelAtom", "">;
list<SubtargetFeature> BtVer1InheritableFeatures = [FeatureX87,
                                                       FeatureNOPL,
                                                       Feature64Bit,
                                                       FeatureCMPXCHG16B,
                                                       FeaturePRFCHW,
                                                       FeatureLZCNT,
                                                       FeaturePOPCNT,
                                                       FeatureSlowSHLD,
                                                       FeatureLAHFSAHF,
-                                                      FeatureFast15ByteNOP];
+                                                      FeatureFast15ByteNOP,
+                                                      FeatureFastVectorShiftMasks];
   list<SubtargetFeature> BtVer1Features = BtVer1InheritableFeatures;
 
   // Jaguar
   list<SubtargetFeature> BtVer2AdditionalFeatures = [FeatureAVX,
                                                      FeatureAES,
                                                      FeaturePCLMUL,
                                                      FeatureBMI,
                                                      FeatureF16C,

llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

bool X86TargetLowering::hasAndNot(SDValue Y) const {
   if (VT == MVT::v4i32)
     return true;
 
   return Subtarget.hasSSE2();
 }
 
 bool X86TargetLowering::shouldFoldConstantShiftPairToMask(
     const SDNode *N, CombineLevel Level) const {
-  // TODO - some targets prefer immediate vector shifts to shift+mask.
+  assert((N->getOpcode() == ISD::SHL &&
+          N->getOperand(0).getOpcode() == ISD::SRL) ||
+         (N->getOpcode() == ISD::SRL &&
+          N->getOperand(0).getOpcode() == ISD::SHL) &&
+             "Expected shift-shift mask");
+
+  if (Subtarget.hasFastVectorShiftMasks() && N->getValueType(0).isVector()) {
+    // Only fold if the shift values are equal - so it folds to AND.
+    // TODO - we should fold if either is non-uniform but we don't do the
+    // fold for non-splats yet.
+    return N->getOperand(1) == N->getOperand(0).getOperand(1);
+  }
   return TargetLoweringBase::shouldFoldConstantShiftPairToMask(N, Level);
 }
 
 bool X86TargetLowering::shouldFoldMaskToVariableShiftPair(SDValue Y) const {
   EVT VT = Y.getValueType();
 
   // For vectors, we don't have a preference, but we probably want a mask.
   if (VT.isVector())

llvm/trunk/lib/Target/X86/X86Subtarget.h

protected:
   bool HasPCONFIG = false;
 
   /// Processor has a single uop BEXTR implementation.
   bool HasFastBEXTR = false;
 
   /// Try harder to combine to horizontal vector ops if they are fast.
   bool HasFastHorizontalOps = false;
 
+  /// Prefer a left/right vector logical shifts pair over a shift+and pair.
+  bool HasFastVectorShiftMasks = false;
+
   /// Use a retpoline thunk rather than indirect calls to block speculative
   /// execution.
   bool UseRetpolineIndirectCalls = false;
 
   /// Use a retpoline thunk or remove any indirect branch to block speculative
   /// execution.
   bool UseRetpolineIndirectBranches = false;
 
public:
   }
   bool hasFastGather() const { return HasFastGather; }
   bool hasFastScalarFSQRT() const { return HasFastScalarFSQRT; }
   bool hasFastVectorFSQRT() const { return HasFastVectorFSQRT; }
   bool hasFastLZCNT() const { return HasFastLZCNT; }
   bool hasFastSHLDRotate() const { return HasFastSHLDRotate; }
   bool hasFastBEXTR() const { return HasFastBEXTR; }
   bool hasFastHorizontalOps() const { return HasFastHorizontalOps; }
+  bool hasFastVectorShiftMasks() const { return HasFastVectorShiftMasks; }
   bool hasMacroFusion() const { return HasMacroFusion; }
   bool hasBranchFusion() const { return HasBranchFusion; }
   bool hasERMSB() const { return HasERMSB; }
   bool hasSlowDivide32() const { return HasSlowDivide32; }
   bool hasSlowDivide64() const { return HasSlowDivide64; }
   bool padShortFunctions() const { return PadShortFunctions; }
   bool slowTwoMemOps() const { return SlowTwoMemOps; }
   bool LEAusesAG() const { return LEAUsesAG; }

llvm/trunk/test/CodeGen/X86/sse2-vector-shifts.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc < %s -mtriple=x86_64-pc-linux -mattr=+sse2 | FileCheck %s
+; RUN: llc < %s -mtriple=x86_64-pc-linux -mattr=+sse2 | FileCheck %s --check-prefixes=CHECK,MASK
+; RUN: llc < %s -mtriple=x86_64-pc-linux -mattr=+sse2,+fast-vector-shift-masks | FileCheck %s --check-prefixes=CHECK,SHIFT
+; RUN: llc < %s -mtriple=x86_64-pc-linux -mcpu=btver1 | FileCheck %s --check-prefixes=CHECK,SHIFT
 
 ; SSE2 Logical Shift Left
 
 define <8 x i16> @test_sllw_1(<8 x i16> %InVec) {
 ; CHECK-LABEL: test_sllw_1:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    retq
 entry:

 ; CHECK-NEXT:    andps {{.*}}(%rip), %xmm0
 ; CHECK-NEXT:    retq
   %shl0 = ashr <4 x i32> %x, <i32 4, i32 4, i32 4, i32 4>
   %shl1 = shl <4 x i32> %shl0, <i32 4, i32 4, i32 4, i32 4>
   ret <4 x i32> %shl1
 }
 
 define <4 x i32> @shl_srl_v4i32(<4 x i32> %x) nounwind {
-; CHECK-LABEL: shl_srl_v4i32:
-; CHECK:       # %bb.0:
-; CHECK-NEXT:    pslld $3, %xmm0
-; CHECK-NEXT:    pand {{.*}}(%rip), %xmm0
-; CHECK-NEXT:    retq
+; MASK-LABEL: shl_srl_v4i32:
+; MASK:       # %bb.0:
+; MASK-NEXT:    pslld $3, %xmm0
+; MASK-NEXT:    pand {{.*}}(%rip), %xmm0
+; MASK-NEXT:    retq
+;
+; SHIFT-LABEL: shl_srl_v4i32:
+; SHIFT:       # %bb.0:
+; SHIFT-NEXT:    psrld $2, %xmm0
+; SHIFT-NEXT:    pslld $5, %xmm0
+; SHIFT-NEXT:    retq
   %shl0 = lshr <4 x i32> %x, <i32 2, i32 2, i32 2, i32 2>
   %shl1 = shl <4 x i32> %shl0, <i32 5, i32 5, i32 5, i32 5>
   ret <4 x i32> %shl1
 }
 
 define <4 x i32> @shl_zext_srl_v4i32(<4 x i16> %x) nounwind {
 ; CHECK-LABEL: shl_zext_srl_v4i32:
 ; CHECK:       # %bb.0: