Skip to content

DAG: Preserve range metadata when load is narrowed #128144

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 26, 2025
Merged

DAG: Preserve range metadata when load is narrowed #128144

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8a5fdc1
Preserve range information when load is narrowed
LU-JOHN Feb 21, 2025
c9b533d
Reduce 64-bit shl to 32-bit based on range metadata
LU-JOHN Feb 21, 2025
3a9f8de
Add negative test when bounds can't be truncated to i32
LU-JOHN Feb 21, 2025
d9b8042
Update testcase comments
LU-JOHN Feb 21, 2025
32bd7e5
Simplify bitwidth checking logic with ConstantRange utility function
LU-JOHN Feb 21, 2025
c933174
Make one new range to cover all sub-ranges
LU-JOHN Feb 22, 2025
179869c
Cleanly implement using ConstantRange utility functions
LU-JOHN Feb 23, 2025
a95676c
Use C++ comments, don
LU-JOHN Feb 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 41 additions & 6 deletions llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14903,12 +14903,47 @@ SDValue DAGCombiner::reduceLoadWidth(SDNode *N) {
AddToWorklist(NewPtr.getNode());

SDValue Load;
if (ExtType == ISD::NON_EXTLOAD)
Load = DAG.getLoad(VT, DL, LN0->getChain(), NewPtr,
LN0->getPointerInfo().getWithOffset(PtrOff),
LN0->getOriginalAlign(),
LN0->getMemOperand()->getFlags(), LN0->getAAInfo());
else
if (ExtType == ISD::NON_EXTLOAD) {
const MDNode *OldRanges = LN0->getRanges();
const MDNode *NewRanges = nullptr;
/* If LSBs are loaded and all bounds in the OldRanges metadata fit in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The core logic ideally would go in a utility function on ConstantRanges, not directly inline here. We will eventually want to share the same logic with globalisel

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used ConstantRange::getMinSignedBits() to implement core logic.

the narrower size, preserve the range information by translating
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need new negative tests if it's not in range?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added negative test where bounds cannot fit in 32-bits.

to the the new narrower type, NewTy */
if (ShAmt == 0 && OldRanges) {
Type *NewTy = VT.getTypeForEVT(*DAG.getContext());
const unsigned NumOperands = OldRanges->getNumOperands();
unsigned NumRanges = NumOperands / 2;
const unsigned NewWidth = NewTy->getIntegerBitWidth();
bool InRange = true;
SmallVector<Metadata *, 4> Bounds;
Bounds.reserve(NumOperands);

for (unsigned i = 0; i < NumRanges; ++i) {
const APInt &LowValue =
mdconst::extract<ConstantInt>(OldRanges->getOperand(2 * i))
->getValue();
const APInt &HighValue =
mdconst::extract<ConstantInt>(OldRanges->getOperand(2 * i + 1))
->getValue();
ConstantRange CurRange(LowValue, HighValue);

if (CurRange.getMinSignedBits() > NewWidth) {
InRange = false;
break;
}
Bounds.push_back(ConstantAsMetadata::get(
ConstantInt::get(NewTy, LowValue.trunc(NewWidth))));
Bounds.push_back(ConstantAsMetadata::get(
ConstantInt::get(NewTy, HighValue.trunc(NewWidth))));
}
if (InRange)
NewRanges = MDNode::get(*DAG.getContext(), Bounds);
}
Load = DAG.getLoad(
VT, DL, LN0->getChain(), NewPtr,
LN0->getPointerInfo().getWithOffset(PtrOff), LN0->getOriginalAlign(),
LN0->getMemOperand()->getFlags(), LN0->getAAInfo(), NewRanges);
} else
Load = DAG.getExtLoad(ExtType, DL, VT, LN0->getChain(), NewPtr,
LN0->getPointerInfo().getWithOffset(PtrOff), ExtVT,
LN0->getOriginalAlign(),
Expand Down
64 changes: 56 additions & 8 deletions llvm/test/CodeGen/AMDGPU/shl64_reduce.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,66 @@
; Test range with metadata
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

; FIXME: This case should be reduced, but SelectionDAG::computeKnownBits() cannot
; determine the minimum from metadata in this case. Match current results
; for now.

define i64 @shl_metadata(i64 %arg0, ptr %arg1.ptr) {
; CHECK-LABEL: shl_metadata:
; CHECK: ; %bb.0:
; CHECK-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; CHECK-NEXT: flat_load_dword v1, v[2:3]
; CHECK-NEXT: s_waitcnt vmcnt(0) lgkmcnt(0)
; CHECK-NEXT: v_lshlrev_b32_e32 v1, v1, v0
; CHECK-NEXT: v_mov_b32_e32 v0, 0
; CHECK-NEXT: s_setpc_b64 s[30:31]
%shift.amt = load i64, ptr %arg1.ptr, !range !0, !noundef !{}
%shl = shl i64 %arg0, %shift.amt
ret i64 %shl
}

define i64 @shl_metadata_two_ranges(i64 %arg0, ptr %arg1.ptr) {
; CHECK-LABEL: shl_metadata_two_ranges:
; CHECK: ; %bb.0:
; CHECK-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; CHECK-NEXT: flat_load_dword v1, v[2:3]
; CHECK-NEXT: s_waitcnt vmcnt(0) lgkmcnt(0)
; CHECK-NEXT: v_lshlrev_b32_e32 v1, v1, v0
; CHECK-NEXT: v_mov_b32_e32 v0, 0
; CHECK-NEXT: s_setpc_b64 s[30:31]
%shift.amt = load i64, ptr %arg1.ptr, !range !1, !noundef !{}
%shl = shl i64 %arg0, %shift.amt
ret i64 %shl
}

; Known minimum is too low. Reduction must not be done.
define i64 @shl_metadata_out_of_range(i64 %arg0, ptr %arg1.ptr) {
; CHECK-LABEL: shl_metadata_out_of_range:
; CHECK: ; %bb.0:
; CHECK-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; CHECK-NEXT: flat_load_dword v2, v[2:3]
; CHECK-NEXT: s_waitcnt vmcnt(0) lgkmcnt(0)
; CHECK-NEXT: v_lshlrev_b64 v[0:1], v2, v[0:1]
; CHECK-NEXT: s_setpc_b64 s[30:31]
%shift.amt = load i64, ptr %arg1.ptr, !range !2, !noundef !{}
%shl = shl i64 %arg0, %shift.amt
ret i64 %shl
}

; Bounds cannot be truncated to i32 when load is narrowed to i32.
; Reduction must not be done.
; Bounds were chosen so that if bounds were truncated to i32 the
; known minimum would be 32 and the shl would be erroneously reduced.
define i64 @shl_metadata_cant_be_narrowed_to_i32(i64 %arg0, ptr %arg1.ptr) {
; CHECK-LABEL: shl_metadata_cant_be_narrowed_to_i32:
; CHECK: ; %bb.0:
; CHECK-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; CHECK-NEXT: flat_load_dword v2, v[2:3]
; CHECK-NEXT: s_waitcnt vmcnt(0) lgkmcnt(0)
; CHECK-NEXT: v_lshlrev_b64 v[0:1], v2, v[0:1]
; CHECK-NEXT: s_setpc_b64 s[30:31]
%shift.amt = load i64, ptr %arg1.ptr, !range !0
%shift.amt = load i64, ptr %arg1.ptr, !range !3, !noundef !{}
%shl = shl i64 %arg0, %shift.amt
ret i64 %shl
}

; FIXME: This case should be reduced
define <2 x i64> @shl_v2_metadata(<2 x i64> %arg0, ptr %arg1.ptr) {
; CHECK-LABEL: shl_v2_metadata:
; CHECK: ; %bb.0:
Expand All @@ -39,11 +82,12 @@ define <2 x i64> @shl_v2_metadata(<2 x i64> %arg0, ptr %arg1.ptr) {
; CHECK-NEXT: v_lshlrev_b64 v[0:1], v4, v[0:1]
; CHECK-NEXT: v_lshlrev_b64 v[2:3], v6, v[2:3]
; CHECK-NEXT: s_setpc_b64 s[30:31]
%shift.amt = load <2 x i64>, ptr %arg1.ptr, !range !0
%shift.amt = load <2 x i64>, ptr %arg1.ptr, !range !0, !noundef !{}
%shl = shl <2 x i64> %arg0, %shift.amt
ret <2 x i64> %shl
}

; FIXME: This case should be reduced
define <3 x i64> @shl_v3_metadata(<3 x i64> %arg0, ptr %arg1.ptr) {
; CHECK-LABEL: shl_v3_metadata:
; CHECK: ; %bb.0:
Expand All @@ -55,11 +99,12 @@ define <3 x i64> @shl_v3_metadata(<3 x i64> %arg0, ptr %arg1.ptr) {
; CHECK-NEXT: v_lshlrev_b64 v[0:1], v8, v[0:1]
; CHECK-NEXT: v_lshlrev_b64 v[2:3], v10, v[2:3]
; CHECK-NEXT: s_setpc_b64 s[30:31]
%shift.amt = load <3 x i64>, ptr %arg1.ptr, !range !0
%shift.amt = load <3 x i64>, ptr %arg1.ptr, !range !0, !noundef !{}
%shl = shl <3 x i64> %arg0, %shift.amt
ret <3 x i64> %shl
}

; FIXME: This case should be reduced
define <4 x i64> @shl_v4_metadata(<4 x i64> %arg0, ptr %arg1.ptr) {
; CHECK-LABEL: shl_v4_metadata:
; CHECK: ; %bb.0:
Expand All @@ -74,12 +119,15 @@ define <4 x i64> @shl_v4_metadata(<4 x i64> %arg0, ptr %arg1.ptr) {
; CHECK-NEXT: v_lshlrev_b64 v[4:5], v13, v[4:5]
; CHECK-NEXT: v_lshlrev_b64 v[6:7], v15, v[6:7]
; CHECK-NEXT: s_setpc_b64 s[30:31]
%shift.amt = load <4 x i64>, ptr %arg1.ptr, !range !0
%shift.amt = load <4 x i64>, ptr %arg1.ptr, !range !0, !noundef !{}
%shl = shl <4 x i64> %arg0, %shift.amt
ret <4 x i64> %shl
}

!0 = !{i64 32, i64 64}
!1 = !{i64 32, i64 38, i64 42, i64 48}
!2 = !{i64 31, i64 38, i64 42, i64 48}
!3 = !{i64 32, i64 38, i64 2147483680, i64 2147483681}

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
; Test range with an "or X, 16"
Expand Down