Skip to content

Commit 321d929

Browse files
nemanjaitstellar
authored andcommitted
[DAGCombine] Fix splitting indexed loads in ForwardStoreValueToDirectLoad()
In DAGCombiner::visitLOAD() we perform some checks before breaking up an indexed load. However, we don't do the same checking in ForwardStoreValueToDirectLoad() which can lead to failures later during combining (see: https://bugs.llvm.org/show_bug.cgi?id=45301). This patch just adds the same checks to this function as well. Fixes: https://bugs.llvm.org/show_bug.cgi?id=45301 Differential revision: https://reviews.llvm.org/D76778 (cherry picked from commit 4821411)
1 parent edbe962 commit 321d929

File tree

2 files changed

+73
-10
lines changed

2 files changed

+73
-10
lines changed

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -886,6 +886,13 @@ static bool isAnyConstantBuildVector(SDValue V, bool NoOpaques = false) {
886886
ISD::isBuildVectorOfConstantFPSDNodes(V.getNode());
887887
}
888888

889+
// Determine if this an indexed load with an opaque target constant index.
890+
static bool canSplitIdx(LoadSDNode *LD) {
891+
return MaySplitLoadIndex &&
892+
(LD->getOperand(2).getOpcode() != ISD::TargetConstant ||
893+
!cast<ConstantSDNode>(LD->getOperand(2))->isOpaque());
894+
}
895+
889896
bool DAGCombiner::reassociationCanBreakAddressingModePattern(unsigned Opc,
890897
const SDLoc &DL,
891898
SDValue N0,
@@ -14222,11 +14229,11 @@ SDValue DAGCombiner::ForwardStoreValueToDirectLoad(LoadSDNode *LD) {
1422214229

1422314230
auto ReplaceLd = [&](LoadSDNode *LD, SDValue Val, SDValue Chain) -> SDValue {
1422414231
if (LD->isIndexed()) {
14225-
bool IsSub = (LD->getAddressingMode() == ISD::PRE_DEC ||
14226-
LD->getAddressingMode() == ISD::POST_DEC);
14227-
unsigned Opc = IsSub ? ISD::SUB : ISD::ADD;
14228-
SDValue Idx = DAG.getNode(Opc, SDLoc(LD), LD->getOperand(1).getValueType(),
14229-
LD->getOperand(1), LD->getOperand(2));
14232+
// Cannot handle opaque target constants and we must respect the user's
14233+
// request not to split indexes from loads.
14234+
if (!canSplitIdx(LD))
14235+
return SDValue();
14236+
SDValue Idx = SplitIndexingFromLoad(LD);
1423014237
SDValue Ops[] = {Val, Idx, Chain};
1423114238
return CombineTo(LD, Ops, 3);
1423214239
}
@@ -14322,14 +14329,12 @@ SDValue DAGCombiner::visitLOAD(SDNode *N) {
1432214329
// the indexing into an add/sub directly (that TargetConstant may not be
1432314330
// valid for a different type of node, and we cannot convert an opaque
1432414331
// target constant into a regular constant).
14325-
bool HasOTCInc = LD->getOperand(2).getOpcode() == ISD::TargetConstant &&
14326-
cast<ConstantSDNode>(LD->getOperand(2))->isOpaque();
14332+
bool CanSplitIdx = canSplitIdx(LD);
1432714333

14328-
if (!N->hasAnyUseOfValue(0) &&
14329-
((MaySplitLoadIndex && !HasOTCInc) || !N->hasAnyUseOfValue(1))) {
14334+
if (!N->hasAnyUseOfValue(0) && (CanSplitIdx || !N->hasAnyUseOfValue(1))) {
1433014335
SDValue Undef = DAG.getUNDEF(N->getValueType(0));
1433114336
SDValue Index;
14332-
if (N->hasAnyUseOfValue(1) && MaySplitLoadIndex && !HasOTCInc) {
14337+
if (N->hasAnyUseOfValue(1) && CanSplitIdx) {
1433314338
Index = SplitIndexingFromLoad(LD);
1433414339
// Try to fold the base pointer arithmetic into subsequent loads and
1433514340
// stores.

llvm/test/CodeGen/PowerPC/pr45301.ll

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
2+
; RUN: llc -mtriple=powerpc64-- -verify-machineinstrs \
3+
; RUN: -ppc-asm-full-reg-names < %s | FileCheck %s
4+
%struct.e.0.1.2.3.12.29 = type { [10 x i32] }
5+
6+
define dso_local void @g(%struct.e.0.1.2.3.12.29* %agg.result) local_unnamed_addr #0 {
7+
; CHECK-LABEL: g:
8+
; CHECK: # %bb.0: # %entry
9+
; CHECK-NEXT: mflr r0
10+
; CHECK-NEXT: std r0, 16(r1)
11+
; CHECK-NEXT: stdu r1, -112(r1)
12+
; CHECK-NEXT: bl i
13+
; CHECK-NEXT: nop
14+
; CHECK-NEXT: addis r4, r2, g@toc@ha
15+
; CHECK-NEXT: addi r4, r4, g@toc@l
16+
; CHECK-NEXT: ld r5, 0(r4)
17+
; CHECK-NEXT: std r5, 0(r3)
18+
; CHECK-NEXT: ld r5, 16(r4)
19+
; CHECK-NEXT: std r5, 16(r3)
20+
; CHECK-NEXT: ld r6, 8(r4)
21+
; CHECK-NEXT: std r6, 8(r3)
22+
; CHECK-NEXT: ld r6, 24(r4)
23+
; CHECK-NEXT: std r6, 24(r3)
24+
; CHECK-NEXT: lwz r6, 0(r3)
25+
; CHECK-NEXT: ld r4, 32(r4)
26+
; CHECK-NEXT: std r4, 32(r3)
27+
; CHECK-NEXT: li r4, 20
28+
; CHECK-NEXT: stwbrx r6, 0, r3
29+
; CHECK-NEXT: stwbrx r5, r3, r4
30+
; CHECK-NEXT: addi r1, r1, 112
31+
; CHECK-NEXT: ld r0, 16(r1)
32+
; CHECK-NEXT: mtlr r0
33+
; CHECK-NEXT: blr
34+
entry:
35+
%call = tail call signext i32 bitcast (i32 (...)* @i to i32 ()*)()
36+
%conv = sext i32 %call to i64
37+
%0 = inttoptr i64 %conv to i8*
38+
tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* nonnull align 4 dereferenceable(40) %0, i8* nonnull align 4 dereferenceable(40) bitcast (void (%struct.e.0.1.2.3.12.29*)* @g to i8*), i64 40, i1 false)
39+
%1 = inttoptr i64 %conv to i32*
40+
%2 = load i32, i32* %1, align 4
41+
%rev.i = tail call i32 @llvm.bswap.i32(i32 %2)
42+
store i32 %rev.i, i32* %1, align 4
43+
%incdec.ptr.i.4 = getelementptr inbounds i32, i32* %1, i64 5
44+
%3 = load i32, i32* %incdec.ptr.i.4, align 4
45+
%rev.i.5 = tail call i32 @llvm.bswap.i32(i32 %3)
46+
store i32 %rev.i.5, i32* %incdec.ptr.i.4, align 4
47+
ret void
48+
}
49+
50+
declare i32 @i(...) local_unnamed_addr
51+
52+
; Function Attrs: argmemonly nounwind willreturn
53+
declare void @llvm.memcpy.p0i8.p0i8.i64(i8* noalias nocapture writeonly, i8* noalias nocapture readonly, i64, i1 immarg) #1
54+
55+
; Function Attrs: nounwind readnone speculatable willreturn
56+
declare i32 @llvm.bswap.i32(i32)
57+
58+
attributes #0 = { nounwind }

0 commit comments

Comments
 (0)