Skip to content
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

Jump to bottom

[DAGCombiner] Remove a hasOneUse check in visitAND #115142

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+64 −2
Open

[DAGCombiner] Remove a hasOneUse check in visitAND #115142

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f6753b
Add tests
david-arm Nov 6, 2024
204701b
[DAGCombiner] Remove a hasOneUse check in visitAND
david-arm Nov 6, 2024
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
3 changes: 1 addition & 2 deletions llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7095,8 +7095,7 @@ SDValue DAGCombiner::visitAND(SDNode *N) {
// fold (and (masked_load) (splat_vec (x, ...))) to zext_masked_load
auto *MLoad = dyn_cast<MaskedLoadSDNode>(N0);
ConstantSDNode *Splat = isConstOrConstSplat(N1, true, true);
if (MLoad && MLoad->getExtensionType() == ISD::EXTLOAD && Splat &&
N1.hasOneUse()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should it be checking N0.hasOneUse()? (i.e the load)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I add that check it causes regression in CodeGen/AArch64/sve-load-compare-store.ll and CodeGen/Thumb2/mve-masked-load.ll

Copy link
Contributor Author

Choose a reason for hiding this comment

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

An extra instruction in multi_user_zext (mve-masked-load.ll) and two extra in sve_load_compare_store (sve-load-compare-store.ll)

Copy link
Collaborator

@paulwalker-arm paulwalker-arm Nov 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

You can see from the code the transformation has explicit handling for this (i.e. no new use count check is necessary). The original load is an any-extend and so the transformation is simply promoting this to a zero-extend which can be used by all uses.

if (MLoad && MLoad->getExtensionType() == ISD::EXTLOAD && Splat) {
EVT LoadVT = MLoad->getMemoryVT();
EVT ExtVT = VT;
if (TLI.isLoadExtLegal(ISD::ZEXTLOAD, ExtVT, LoadVT)) {
Expand Down
63 changes: 63 additions & 0 deletions llvm/test/CodeGen/AArch64/sve-hadd.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1341,3 +1341,66 @@ entry:
%avg = ashr <vscale x 2 x i64> %add, splat (i64 1)
ret <vscale x 2 x i64> %avg
}

define void @zext_mload_avgflooru(ptr %p1, ptr %p2, <vscale x 8 x i1> %mask) {
; SVE-LABEL: zext_mload_avgflooru:
; SVE: // %bb.0:
; SVE-NEXT: ld1b { z0.h }, p0/z, [x0]
; SVE-NEXT: ld1b { z1.h }, p0/z, [x1]
; SVE-NEXT: eor z2.d, z0.d, z1.d
; SVE-NEXT: and z0.d, z0.d, z1.d
; SVE-NEXT: lsr z1.h, z2.h, #1
; SVE-NEXT: add z0.h, z0.h, z1.h
; SVE-NEXT: st1h { z0.h }, p0, [x0]
; SVE-NEXT: ret
;
; SVE2-LABEL: zext_mload_avgflooru:
; SVE2: // %bb.0:
; SVE2-NEXT: ld1b { z0.h }, p0/z, [x0]
; SVE2-NEXT: ld1b { z1.h }, p0/z, [x1]
; SVE2-NEXT: ptrue p1.h
; SVE2-NEXT: uhadd z0.h, p1/m, z0.h, z1.h
; SVE2-NEXT: st1h { z0.h }, p0, [x0]
; SVE2-NEXT: ret
%ld1 = call <vscale x 8 x i8> @llvm.masked.load(ptr %p1, i32 16, <vscale x 8 x i1> %mask, <vscale x 8 x i8> zeroinitializer)
%ld2 = call <vscale x 8 x i8> @llvm.masked.load(ptr %p2, i32 16, <vscale x 8 x i1> %mask, <vscale x 8 x i8> zeroinitializer)
%and = and <vscale x 8 x i8> %ld1, %ld2
%xor = xor <vscale x 8 x i8> %ld1, %ld2
%shift = lshr <vscale x 8 x i8> %xor, splat(i8 1)
%avg = add <vscale x 8 x i8> %and, %shift
%avgext = zext <vscale x 8 x i8> %avg to <vscale x 8 x i16>
call void @llvm.masked.store.nxv8i16(<vscale x 8 x i16> %avgext, ptr %p1, i32 16, <vscale x 8 x i1> %mask)
ret void
}

define void @zext_mload_avgceilu(ptr %p1, ptr %p2, <vscale x 8 x i1> %mask) {
; SVE-LABEL: zext_mload_avgceilu:
; SVE: // %bb.0:
; SVE-NEXT: ld1b { z0.h }, p0/z, [x0]
; SVE-NEXT: ld1b { z1.h }, p0/z, [x1]
; SVE-NEXT: eor z2.d, z0.d, z1.d
; SVE-NEXT: orr z0.d, z0.d, z1.d
; SVE-NEXT: lsr z1.h, z2.h, #1
; SVE-NEXT: sub z0.h, z0.h, z1.h
; SVE-NEXT: st1b { z0.h }, p0, [x0]
; SVE-NEXT: ret
;
; SVE2-LABEL: zext_mload_avgceilu:
; SVE2: // %bb.0:
; SVE2-NEXT: ld1b { z0.h }, p0/z, [x0]
; SVE2-NEXT: ld1b { z1.h }, p0/z, [x1]
; SVE2-NEXT: ptrue p1.h
; SVE2-NEXT: urhadd z0.h, p1/m, z0.h, z1.h
; SVE2-NEXT: st1b { z0.h }, p0, [x0]
; SVE2-NEXT: ret
%ld1 = call <vscale x 8 x i8> @llvm.masked.load(ptr %p1, i32 16, <vscale x 8 x i1> %mask, <vscale x 8 x i8> zeroinitializer)
%ld2 = call <vscale x 8 x i8> @llvm.masked.load(ptr %p2, i32 16, <vscale x 8 x i1> %mask, <vscale x 8 x i8> zeroinitializer)
%zext1 = zext <vscale x 8 x i8> %ld1 to <vscale x 8 x i16>
%zext2 = zext <vscale x 8 x i8> %ld2 to <vscale x 8 x i16>
%add1 = add nuw nsw <vscale x 8 x i16> %zext1, splat(i16 1)
%add2 = add nuw nsw <vscale x 8 x i16> %add1, %zext2
%shift = lshr <vscale x 8 x i16> %add2, splat(i16 1)
%trunc = trunc <vscale x 8 x i16> %shift to <vscale x 8 x i8>
call void @llvm.masked.store.nxv8i8(<vscale x 8 x i8> %trunc, ptr %p1, i32 16, <vscale x 8 x i1> %mask)
ret void
}
Loading