Skip to content

Commit ef84452

Browse files
authored
[DAGCombiner] Be more careful about looking through extends and truncates in mergeTruncStores. (#91375)
Previously we recursively looked through extends and truncates on both SourceValue and WideVal. SourceValue is the largest source found for each of the stores we are combining. WideVal is the source for the current store. Previously we could incorrectly look through a (zext (trunc X)) pair and incorrectly believe X to be a good source. I think we could also look through a zext on one store and a sext on another store and arbitrarily pick one of the extends as the final source. With this patch we only look through one level of extend or truncate. And we don't look through extends/truncs on both SourceValue and WideVal at the same time. This may lose some optimization cases, but keeps everything we had tests for. Fixes #90936.
1 parent 85ef6b7 commit ef84452

File tree

2 files changed

+18
-11
lines changed

2 files changed

+18
-11
lines changed

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8728,15 +8728,16 @@ static std::optional<bool> isBigEndian(const ArrayRef<int64_t> ByteOffsets,
87288728
return BigEndian;
87298729
}
87308730

8731+
// Look through one layer of truncate or extend.
87318732
static SDValue stripTruncAndExt(SDValue Value) {
87328733
switch (Value.getOpcode()) {
87338734
case ISD::TRUNCATE:
87348735
case ISD::ZERO_EXTEND:
87358736
case ISD::SIGN_EXTEND:
87368737
case ISD::ANY_EXTEND:
8737-
return stripTruncAndExt(Value.getOperand(0));
8738+
return Value.getOperand(0);
87388739
}
8739-
return Value;
8740+
return SDValue();
87408741
}
87418742

87428743
/// Match a pattern where a wide type scalar value is stored by several narrow
@@ -8849,16 +8850,18 @@ SDValue DAGCombiner::mergeTruncStores(StoreSDNode *N) {
88498850
}
88508851

88518852
// Stores must share the same source value with different offsets.
8852-
// Truncate and extends should be stripped to get the single source value.
88538853
if (!SourceValue)
88548854
SourceValue = WideVal;
8855-
else if (stripTruncAndExt(SourceValue) != stripTruncAndExt(WideVal))
8856-
return SDValue();
8857-
else if (SourceValue.getValueType() != WideVT) {
8858-
if (WideVal.getValueType() == WideVT ||
8859-
WideVal.getScalarValueSizeInBits() >
8860-
SourceValue.getScalarValueSizeInBits())
8855+
else if (SourceValue != WideVal) {
8856+
// Truncate and extends can be stripped to see if the values are related.
8857+
if (stripTruncAndExt(SourceValue) != WideVal &&
8858+
stripTruncAndExt(WideVal) != SourceValue)
8859+
return SDValue();
8860+
8861+
if (WideVal.getScalarValueSizeInBits() >
8862+
SourceValue.getScalarValueSizeInBits())
88618863
SourceValue = WideVal;
8864+
88628865
// Give up if the source value type is smaller than the store size.
88638866
if (SourceValue.getScalarValueSizeInBits() < WideVT.getScalarSizeInBits())
88648867
return SDValue();

llvm/test/CodeGen/AArch64/pr90936.ll

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,12 @@ bb:
2222
define void @g(i32 %arg, ptr %arg1) {
2323
; CHECK-LABEL: g:
2424
; CHECK: // %bb.0: // %bb
25-
; CHECK-NEXT: and w8, w0, #0xff
26-
; CHECK-NEXT: str w8, [x1]
25+
; CHECK-NEXT: lsr w8, w0, #8
26+
; CHECK-NEXT: lsr w9, w0, #16
27+
; CHECK-NEXT: strb w0, [x1]
28+
; CHECK-NEXT: strb wzr, [x1, #3]
29+
; CHECK-NEXT: strb w8, [x1, #1]
30+
; CHECK-NEXT: strb w9, [x1, #2]
2731
; CHECK-NEXT: ret
2832
bb:
2933
%i = trunc i32 %arg to i8

0 commit comments

Comments
 (0)