Skip to content

Commit 27e71e5

Browse files
committed
[PowerPC] Have BitPermutationSelector return SDValue, not SDNode
If BPS optimizes away a permutation (rotate all bits left by 0), the result might be from an SDNode with more than one result, such as a load pre-inc node. The node replacement assumed result number 0. Change it to use an SDValue with the correct result number. Fixes #133507
1 parent f7c6c7c commit 27e71e5

File tree

3 files changed

+52
-15
lines changed

3 files changed

+52
-15
lines changed

llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2256,7 +2256,7 @@ class BitPermutationSelector {
22562256
}
22572257

22582258
// Instruction selection for the 32-bit case.
2259-
SDNode *Select32(SDNode *N, bool LateMask, unsigned *InstCnt) {
2259+
SDValue Select32(SDNode *N, bool LateMask, unsigned *InstCnt) {
22602260
SDLoc dl(N);
22612261
SDValue Res;
22622262

@@ -2337,7 +2337,7 @@ class BitPermutationSelector {
23372337
ANDIVal, ANDISVal), 0);
23382338
}
23392339

2340-
return Res.getNode();
2340+
return Res;
23412341
}
23422342

23432343
unsigned SelectRotMask64Count(unsigned RLAmt, bool Repl32,
@@ -2642,7 +2642,7 @@ class BitPermutationSelector {
26422642
}
26432643

26442644
// Instruction selection for the 64-bit case.
2645-
SDNode *Select64(SDNode *N, bool LateMask, unsigned *InstCnt) {
2645+
SDValue Select64(SDNode *N, bool LateMask, unsigned *InstCnt) {
26462646
SDLoc dl(N);
26472647
SDValue Res;
26482648

@@ -2782,14 +2782,14 @@ class BitPermutationSelector {
27822782
}
27832783
}
27842784

2785-
return Res.getNode();
2785+
return Res;
27862786
}
27872787

2788-
SDNode *Select(SDNode *N, bool LateMask, unsigned *InstCnt = nullptr) {
2788+
SDValue Select(SDNode *N, bool LateMask, unsigned *InstCnt = nullptr) {
27892789
// Fill in BitGroups.
27902790
collectBitGroups(LateMask);
27912791
if (BitGroups.empty())
2792-
return nullptr;
2792+
return SDValue();
27932793

27942794
// For 64-bit values, figure out when we can use 32-bit instructions.
27952795
if (Bits.size() == 64)
@@ -2805,7 +2805,7 @@ class BitPermutationSelector {
28052805
return Select64(N, LateMask, InstCnt);
28062806
}
28072807

2808-
return nullptr;
2808+
return SDValue();
28092809
}
28102810

28112811
void eraseMatchingBitGroups(function_ref<bool(const BitGroup &)> F) {
@@ -2831,12 +2831,12 @@ class BitPermutationSelector {
28312831
// Here we try to match complex bit permutations into a set of
28322832
// rotate-and-shift/shift/and/or instructions, using a set of heuristics
28332833
// known to produce optimal code for common cases (like i32 byte swapping).
2834-
SDNode *Select(SDNode *N) {
2834+
SDValue Select(SDNode *N) {
28352835
Memoizer.clear();
28362836
auto Result =
28372837
getValueBits(SDValue(N, 0), N->getValueType(0).getSizeInBits());
28382838
if (!Result.first)
2839-
return nullptr;
2839+
return SDValue();
28402840
Bits = std::move(*Result.second);
28412841

28422842
LLVM_DEBUG(dbgs() << "Considering bit-permutation-based instruction"
@@ -2859,21 +2859,21 @@ class BitPermutationSelector {
28592859

28602860
unsigned InstCnt = 0, InstCntLateMask = 0;
28612861
LLVM_DEBUG(dbgs() << "\tEarly masking:\n");
2862-
SDNode *RN = Select(N, false, &InstCnt);
2862+
SDValue RV = Select(N, false, &InstCnt);
28632863
LLVM_DEBUG(dbgs() << "\t\tisel would use " << InstCnt << " instructions\n");
28642864

28652865
LLVM_DEBUG(dbgs() << "\tLate masking:\n");
2866-
SDNode *RNLM = Select(N, true, &InstCntLateMask);
2866+
SDValue RVLM = Select(N, true, &InstCntLateMask);
28672867
LLVM_DEBUG(dbgs() << "\t\tisel would use " << InstCntLateMask
28682868
<< " instructions\n");
28692869

28702870
if (InstCnt <= InstCntLateMask) {
28712871
LLVM_DEBUG(dbgs() << "\tUsing early-masking for isel\n");
2872-
return RN;
2872+
return RV;
28732873
}
28742874

28752875
LLVM_DEBUG(dbgs() << "\tUsing late-masking for isel\n");
2876-
return RNLM;
2876+
return RVLM;
28772877
}
28782878
};
28792879

@@ -4104,8 +4104,9 @@ bool PPCDAGToDAGISel::tryBitPermutation(SDNode *N) {
41044104
case ISD::AND:
41054105
case ISD::OR: {
41064106
BitPermutationSelector BPS(CurDAG);
4107-
if (SDNode *New = BPS.Select(N)) {
4108-
ReplaceNode(N, New);
4107+
if (SDValue New = BPS.Select(N)) {
4108+
CurDAG->ReplaceAllUsesWith(N, &New);
4109+
CurDAG->RemoveDeadNode(N);
41094110
return true;
41104111
}
41114112
return false;

llvm/test/CodeGen/PowerPC/lwzu-i48.ll

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
; RUN: llc -mtriple=powerpc-unknown-openbsd < %s | FileCheck %s
2+
3+
; BitPermutationSelector in PPCISelDAGToDAG.cpp was taking the wrong
4+
; result of a load <pre-inc> after optimizing away a permutation.
5+
; Here, the big end of i48 %3 was %1 but should be %0.
6+
7+
define i32 @hop(ptr %out, ptr %in) {
8+
entry:
9+
%0 = getelementptr i8, ptr %in, i32 28
10+
%1 = load i32, ptr %0, align 4
11+
%2 = ptrtoint ptr %0 to i48
12+
%3 = shl i48 %2, 16
13+
store i48 %3, ptr %out, align 4
14+
ret i32 %1
15+
}
16+
; The stw should store POINTER, not VALUE.
17+
; CHECK: lwzu [[VALUE:[0-9]+]], 28([[POINTER:[0-9]+]])
18+
; CHECK: stw [[POINTER]], 0({{[0-9]+}})

llvm/test/CodeGen/PowerPC/lwzu-i96.ll

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
; RUN: llc -mtriple=powerpc64-unknown-openbsd < %s | FileCheck %s
2+
3+
; BitPermutationSelector in PPCISelDAGToDAG.cpp was taking the wrong
4+
; result of a load <pre-inc> after optimizing away a permutation.
5+
; Here, the big end of i96 %3 was %1 but should be %0.
6+
7+
define i32 @hop(ptr %out, ptr %in) {
8+
entry:
9+
%0 = getelementptr i8, ptr %in, i32 28
10+
%1 = load i32, ptr %0, align 4
11+
%2 = ptrtoint ptr %0 to i96
12+
%3 = shl i96 %2, 32
13+
store i96 %3, ptr %out, align 8
14+
ret i32 %1
15+
}
16+
; The stw should store POINTER, not VALUE.
17+
; CHECK: lwzu [[VALUE:[0-9]+]], 28([[POINTER:[0-9]+]])
18+
; CHECK: std [[POINTER]], 0({{[0-9]+}})

0 commit comments

Comments
 (0)