[DAGCombiner] Add a command line option to guard ReduceLoadOpStoreWidth

[Carrot]

Summary

As discussed in the thread http://lists.llvm.org/pipermail/llvm-dev/2020-May/141838.html, some bit field access width can be reduced by ReduceLoadOpStoreWidth, some can't. If two accesses are very close, and the first access width is reduced, the second is not. Then the wide load of second access will be stalled for long time.

This patch guards the function ReduceLoadOpStoreWidth with a new command line option combiner-reduce-load-op-store-width, so it gives user a chance to disable ReduceLoadOpStoreWidth.

[efriedma]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

15418

There are two separate transforms here: ShrinkLoadReplaceStoreWithStore, which actually erases the load, and the isNarrowingProfitable transform, which just shrinks the store. The performance implications might be different, so I think we want separate flags.

(We don't really want to encourage users to use these flags in production, but I think it's okay to add them to evaluate the performance impact.)

llvm/test/CodeGen/X86/clear-bitfield.ll

10

Probably we want to see what happens with i16 specifically, since there's some weirdness involving the encodings.

Please generate checks with update_llc_test_checks.py.

[void]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

15418

Would using this flag in isNarrowingProfitable (or conditionalizing the execution of same with the flag) better?

(We don't really want to encourage users to use these flags in production, but I think it's okay to add them to evaluate the performance impact.)

Agreed. I think it's better to have a better heuristic to determine if narrowing is profitable.

[Carrot]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

15418

isNarrowingProfitable is also used to reduce load and other alu width, these don't stall following load.

I added a new flag to guard ShrinkLoadReplaceStoreWithStore.

[efriedma]

LGTM

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

static cl::opt<unsigned> TokenFactorInlineLimit(
     "combiner-tokenfactor-inline-limit", cl::Hidden, cl::init(2048),
     cl::desc("Limit the number of operands to inline for Token Factors"));
 
 static cl::opt<unsigned> StoreMergeDependenceLimit(
     "combiner-store-merge-dependence-limit", cl::Hidden, cl::init(10),
     cl::desc("Limit the number of times for the same StoreNode and RootNode "
              "to bail out in store merging dependence check"));
 
+static cl::opt<bool> EnableReduceLoadOpStoreWidth(
+    "combiner-reduce-load-op-store-width", cl::Hidden, cl::init(true),
+    cl::desc("DAG cominber enable reducing the width of load/op/store "
+             "sequence"));
+
+static cl::opt<bool> EnableShrinkLoadReplaceStoreWithStore(
+    "combiner-shrink-load-replace-store-with-store", cl::Hidden, cl::init(true),
+    cl::desc("DAG cominber enable load/<replace bytes>/store with "
+             "a narrower store"));
+
 namespace {
 
   class DAGCombiner {
     SelectionDAG &DAG;
     const TargetLowering &TLI;
     const SelectionDAGTargetInfo *STI;
     CombineLevel Level;
     CodeGenOpt::Level OptLevel;

 }
 
 /// Look for sequence of load / op / store where op is one of 'or', 'xor', and
 /// 'and' of immediates. If 'op' is only touching some of the loaded bits, try
 /// narrowing the load and store if it would end up being a win for performance
 /// or code size.
 SDValue DAGCombiner::ReduceLoadOpStoreWidth(SDNode *N) {
   StoreSDNode *ST  = cast<StoreSDNode>(N);
   if (!ST->isSimple())
     return SDValue();
 
   SDValue Chain = ST->getChain();
   SDValue Value = ST->getValue();
   SDValue Ptr   = ST->getBasePtr();
   EVT VT = Value.getValueType();
 
   if (ST->isTruncatingStore() || VT.isVector() || !Value.hasOneUse())
     return SDValue();
 
   unsigned Opc = Value.getOpcode();
 
   // If this is "store (or X, Y), P" and X is "(and (load P), cst)", where cst
   // is a byte mask indicating a consecutive number of bytes, check to see if
   // Y is known to provide just those bytes.  If so, we try to replace the
   // load + replace + store sequence with a single (narrower) store, which makes
   // the load dead.
-  if (Opc == ISD::OR) {
+  if (Opc == ISD::OR && EnableShrinkLoadReplaceStoreWithStore) {
     std::pair<unsigned, unsigned> MaskedLoad;
     MaskedLoad = CheckForMaskedLoad(Value.getOperand(0), Ptr, Chain);
     if (MaskedLoad.first)
       if (SDValue NewST = ShrinkLoadReplaceStoreWithStore(MaskedLoad,
                                                   Value.getOperand(1), ST,this))
         return NewST;
 
     // Or is commutative, so try swapping X and Y.
     MaskedLoad = CheckForMaskedLoad(Value.getOperand(1), Ptr, Chain);
     if (MaskedLoad.first)
       if (SDValue NewST = ShrinkLoadReplaceStoreWithStore(MaskedLoad,
                                                   Value.getOperand(0), ST,this))
         return NewST;
   }
 
+  if (!EnableReduceLoadOpStoreWidth)
+    return SDValue();
+
   if ((Opc != ISD::OR && Opc != ISD::XOR && Opc != ISD::AND) ||
       Value.getOperand(1).getOpcode() != ISD::Constant)
     return SDValue();
 
   SDValue N0 = Value.getOperand(0);
   if (ISD::isNormalLoad(N0.getNode()) && N0.hasOneUse() &&
       Chain == SDValue(N0.getNode(), 1)) {
     LoadSDNode *LD = cast<LoadSDNode>(N0);

llvm/test/CodeGen/X86/clear-bitfield.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-- -combiner-reduce-load-op-store-width=false | FileCheck %s
+
+%struct.bit_fields = type { i32 }
+
+define void @clear_b1(%struct.bit_fields* %ptr) {
+; CHECK-LABEL: clear_b1:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    andl $-2, (%rdi)
+; CHECK-NEXT:    retq
+entry:
+  %0 = bitcast %struct.bit_fields* %ptr to i32*
+  %bf.load = load i32, i32* %0
+  %bf.clear = and i32 %bf.load, -2
+  store i32 %bf.clear, i32* %0
+  ret void
+}
+
+define void @clear16(%struct.bit_fields* %ptr) {
+; CHECK-LABEL: clear16:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    andw $-2, (%rdi)
+; CHECK-NEXT:    retq
+entry:
+  %0 = bitcast %struct.bit_fields* %ptr to i16*
+  %bf.load = load i16, i16* %0
+  %bf.clear = and i16 %bf.load, -2
+  store i16 %bf.clear, i16* %0
+  ret void
+}

llvm/test/CodeGen/X86/disable-shrink-store.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-- -combiner-shrink-load-replace-store-with-store=false | FileCheck %s
+
+define void @shrink(i16* %ptr) {
+; CHECK-LABEL: shrink:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    movzbl (%rdi), %eax
+; CHECK-NEXT:    orl $25600, %eax # imm = 0x6400
+; CHECK-NEXT:    movw %ax, (%rdi)
+; CHECK-NEXT:    retq
+entry:
+  %val = load i16, i16* %ptr
+  %masked_val = and i16 %val, 255
+  %replaced_val = or i16 %masked_val, 25600
+  store i16 %replaced_val, i16* %ptr
+  ret void
+}
+