[llvm] 5f057ea - [DAGCombiner] reassociationCanBreakAddressingModePattern should check uses of the outer add.

[Craig Topper via llvm-commits]

Author: Craig Topper
Date: 2022-05-02T16:38:53-07:00
New Revision: 5f057eaa0ddf76118a0164b345dff7b382b55234

URL: https://github.com/llvm/llvm-project/commit/5f057eaa0ddf76118a0164b345dff7b382b55234
DIFF: https://github.com/llvm/llvm-project/commit/5f057eaa0ddf76118a0164b345dff7b382b55234.diff

LOG: [DAGCombiner] reassociationCanBreakAddressingModePattern should check uses of the outer add.

When looking for memory uses,
reassociationCanBreakAddressingModePattern should check uses of
the outer ADD rather than the inner ADD. We want to know if the
two ops we're reassociating are used by a load/store.

In practice, the existing check usually works because CodeGenPrepare
will make one of the load/stores have an offset of 0 relative to
split GEP. That will make the inner add have a memory use.

To test this, I've manually split the GEPs so there is no 0 offset
store.

This issue was recently discussed in the original review D60294.

Reviewed By: luismarques

Differential Revision: https://reviews.llvm.org/D124644

Added: 
    

Modified: 
    llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
    llvm/test/CodeGen/RISCV/split-offsets.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index c4b3d373b49b8..e483c3a1fdf86 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -519,7 +519,9 @@ namespace {
 
     SDValue XformToShuffleWithZero(SDNode *N);
     bool reassociationCanBreakAddressingModePattern(unsigned Opc,
-                                                    const SDLoc &DL, SDValue N0,
+                                                    const SDLoc &DL,
+                                                    SDNode *N,
+                                                    SDValue N0,
                                                     SDValue N1);
     SDValue reassociateOpsCommutative(unsigned Opc, const SDLoc &DL, SDValue N0,
                                       SDValue N1);
@@ -996,6 +998,7 @@ static bool canSplitIdx(LoadSDNode *LD) {
 
 bool DAGCombiner::reassociationCanBreakAddressingModePattern(unsigned Opc,
                                                              const SDLoc &DL,
+                                                             SDNode *N,
                                                              SDValue N0,
                                                              SDValue N1) {
   // Currently this only tries to ensure we don't undo the GEP splits done by
@@ -1025,7 +1028,7 @@ bool DAGCombiner::reassociationCanBreakAddressingModePattern(unsigned Opc,
     return false;
   const int64_t CombinedValue = CombinedValueIntVal.getSExtValue();
 
-  for (SDNode *Node : N0->uses()) {
+  for (SDNode *Node : N->uses()) {
     auto LoadStore = dyn_cast<MemSDNode>(Node);
     if (LoadStore) {
       // Is x[offset2] already not a legal addressing mode? If so then
@@ -2447,7 +2450,7 @@ SDValue DAGCombiner::visitADDLike(SDNode *N) {
     return NewSel;
 
   // reassociate add
-  if (!reassociationCanBreakAddressingModePattern(ISD::ADD, DL, N0, N1)) {
+  if (!reassociationCanBreakAddressingModePattern(ISD::ADD, DL, N, N0, N1)) {
     if (SDValue RADD = reassociateOps(ISD::ADD, DL, N0, N1, N->getFlags()))
       return RADD;
 

diff  --git a/llvm/test/CodeGen/RISCV/split-offsets.ll b/llvm/test/CodeGen/RISCV/split-offsets.ll
index d08157b824c16..35671d8c94818 100644
--- a/llvm/test/CodeGen/RISCV/split-offsets.ll
+++ b/llvm/test/CodeGen/RISCV/split-offsets.ll
@@ -118,3 +118,38 @@ while_end:
   ret void
 }
 
+; GEPs have been manually split so the base GEP does not get used by any memory
+; instructions. Make sure we use a small offset in each of the stores.
+define void @test3([65536 x i32]* %t) {
+; RV32I-LABEL: test3:
+; RV32I:       # %bb.0: # %entry
+; RV32I-NEXT:    lui a1, 20
+; RV32I-NEXT:    addi a1, a1, -1920
+; RV32I-NEXT:    add a0, a0, a1
+; RV32I-NEXT:    li a1, 2
+; RV32I-NEXT:    sw a1, 4(a0)
+; RV32I-NEXT:    li a1, 3
+; RV32I-NEXT:    sw a1, 8(a0)
+; RV32I-NEXT:    ret
+;
+; RV64I-LABEL: test3:
+; RV64I:       # %bb.0: # %entry
+; RV64I-NEXT:    lui a1, 20
+; RV64I-NEXT:    addiw a1, a1, -1920
+; RV64I-NEXT:    add a0, a0, a1
+; RV64I-NEXT:    li a1, 2
+; RV64I-NEXT:    sw a1, 4(a0)
+; RV64I-NEXT:    li a1, 3
+; RV64I-NEXT:    sw a1, 8(a0)
+; RV64I-NEXT:    ret
+entry:
+  %0 = bitcast [65536 x i32]* %t to i8*
+  %splitgep = getelementptr i8, i8* %0, i64 80000
+  %1 = getelementptr i8, i8* %splitgep, i64 4
+  %2 = bitcast i8* %1 to i32*
+  %3 = getelementptr i8, i8* %splitgep, i64 8
+  %4 = bitcast i8* %3 to i32*
+  store i32 2, i32* %2, align 4
+  store i32 3, i32* %4, align 4
+  ret void
+}