Skip to content

GlobalISel: neg (and x, 1) --> SIGN_EXTEND_INREG x, 1 #131367

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 3 commits into from
Mar 31, 2025
Merged

GlobalISel: neg (and x, 1) --> SIGN_EXTEND_INREG x, 1 #131367

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d9a2554
GlobalISel: neg (and x, 1) --> SIGN_EXTEND_INREG x, 1
MatzeB Mar 14, 2025
4ddc23d
address review comments
MatzeB Mar 17, 2025
f6aa409
address review comment
MatzeB Mar 31, 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
13 changes: 12 additions & 1 deletion llvm/include/llvm/Target/GlobalISel/Combine.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,17 @@ def shl_ashr_to_sext_inreg : GICombineRule<
(apply [{ Helper.applyAshShlToSextInreg(*${root}, ${info});}])
>;

// Fold sub 0, (and x, 1) -> sext_inreg x, 1
def neg_and_one_to_sext_inreg : GICombineRule<
(defs root:$dst),
(match (G_AND $and, $x, 1),
(G_SUB $dst, 0, $and),
[{ return MRI.hasOneNonDBGUse(${and}.getReg()) &&
Helper.isLegalOrBeforeLegalizer(
{TargetOpcode::G_SEXT_INREG, {MRI.getType(${x}.getReg())}}); }]),
(apply (G_SEXT_INREG $dst, $x, 1))
>;

// Fold and(and(x, C1), C2) -> C1&C2 ? and(x, C1&C2) : 0
def overlapping_and: GICombineRule <
(defs root:$root, build_fn_matchinfo:$info),
Expand Down Expand Up @@ -2013,7 +2024,7 @@ def all_combines : GICombineGroup<[integer_reassoc_combines, trivial_combines,
undef_combines, identity_combines, phi_combines,
simplify_add_to_sub, hoist_logic_op_with_same_opcode_hands, shifts_too_big,
reassocs, ptr_add_immed_chain, cmp_combines,
shl_ashr_to_sext_inreg, sext_inreg_of_load,
shl_ashr_to_sext_inreg, neg_and_one_to_sext_inreg, sext_inreg_of_load,
width_reduction_combines, select_combines,
known_bits_simplifications, trunc_shift,
not_cmp_fold, opt_brcond_by_inverting_cond,
Expand Down
61 changes: 61 additions & 0 deletions llvm/test/CodeGen/AArch64/GlobalISel/combine-neg-and-one-to-sext-inreg.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
# RUN: llc -o - -mtriple aarch64-- -run-pass=aarch64-prelegalizer-combiner %s | FileCheck %s
---
name: test_combine_neg_and_one_to_sext_inreg_s32
body: |
bb.1:
liveins: $w0
; CHECK-LABEL: name: test_combine_neg_and_one_to_sext_inreg_s32
; CHECK: liveins: $w0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: %x:_(s32) = COPY $w0
; CHECK-NEXT: %res:_(s32) = G_SEXT_INREG %x, 1
; CHECK-NEXT: $w0 = COPY %res(s32)
%x:_(s32) = COPY $w0
%one:_(s32) = G_CONSTANT i32 1
%zero:_(s32) = G_CONSTANT i32 0
%and:_(s32) = G_AND %x:_, %one:_
%res:_(s32) = G_SUB %zero:_, %and:_
$w0 = COPY %res:_(s32)
...
---
name: test_combine_neg_and_one_to_sext_inreg_s64
body: |
bb.1:
liveins: $x0
; CHECK-LABEL: name: test_combine_neg_and_one_to_sext_inreg_s64
; CHECK: liveins: $x0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: %x:_(s64) = COPY $x0
; CHECK-NEXT: %res:_(s64) = G_SEXT_INREG %x, 1
; CHECK-NEXT: $x0 = COPY %res(s64)
%x:_(s64) = COPY $x0
%one:_(s64) = G_CONSTANT i64 1
%zero:_(s64) = G_CONSTANT i64 0
%and:_(s64) = G_AND %x:_, %one:_
%res:_(s64) = G_SUB %zero:_, %and:_
$x0 = COPY %res:_(s64)
...
---
name: test_combine_neg_and_one_to_sext_inreg_multiuse_should_not_transform
body: |
bb.1:
liveins: $x0
; CHECK-LABEL: name: test_combine_neg_and_one_to_sext_inreg_multiuse_should_not_transform
; CHECK: liveins: $x0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: %x:_(s64) = COPY $x0
; CHECK-NEXT: %one:_(s64) = G_CONSTANT i64 1
; CHECK-NEXT: %zero:_(s64) = G_CONSTANT i64 0
; CHECK-NEXT: %and:_(s64) = G_AND %x, %one
; CHECK-NEXT: %res:_(s64) = G_SUB %zero, %and
; CHECK-NEXT: $x0 = COPY %res(s64)
; CHECK-NEXT: $x1 = COPY %and(s64)
%x:_(s64) = COPY $x0
%one:_(s64) = G_CONSTANT i64 1
%zero:_(s64) = G_CONSTANT i64 0
%and:_(s64) = G_AND %x:_, %one:_
%res:_(s64) = G_SUB %zero:_, %and:_
$x0 = COPY %res:_(s64)
$x1 = COPY %and:_(s64)
...
22 changes: 22 additions & 0 deletions llvm/test/CodeGen/AMDGPU/GlobalISel/combine-neg-and-one-to-sext-inreg.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
# RUN: llc -o - -mtriple=amdgcn -run-pass=amdgpu-prelegalizer-combiner %s | FileCheck %s
---
name: test_combine_neg_and_one_to_sext_inreg_v2i32
Copy link
Contributor

@Pierre-vh Pierre-vh Mar 18, 2025

Choose a reason for hiding this comment

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

Why are the tests in separate file?
Can you put them all in one file of either target ?

Copy link
Contributor Author

@MatzeB MatzeB Mar 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It's a generic combine rule not tied to a target. Given that people wanted to see scalar and vectorized versions I wondered whether people maybe also wanted to see two targets especially given that for aarch64 the combine isn't really helpful in the vectorized version[1]

Anyway moved the test to AArch64 now.

[1] The combine helps scalar AArch64 because G_SEXT_INREG can be implement with an sbfx instruction; for the vectorized case a G_SEXT_INREG will just be broken to two instructions (SHL + SHRS) which isn't really helpful (and hopefully doesn't hurt compared to the original AND + SUB instructions). Unfortunately as far as I can tell, the GlobalISel legalization interface however does not provide enough information for the target to differentiate between those two cases.

body: |
bb.1:
liveins: $vgpr0_vgpr1
; CHECK-LABEL: name: test_combine_neg_and_one_to_sext_inreg_v2i32
; CHECK: liveins: $vgpr0_vgpr1
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: %x:_(<2 x s32>) = COPY $vgpr0_vgpr1
; CHECK-NEXT: %res:_(<2 x s32>) = G_SEXT_INREG %x, 1
; CHECK-NEXT: $vgpr0_vgpr1 = COPY %res(<2 x s32>)
%x:_(<2 x s32>) = COPY $vgpr0_vgpr1
%one:_(s32) = G_CONSTANT i32 1
%ones:_(<2 x s32>) = G_BUILD_VECTOR %one, %one
%zero:_(s32) = G_CONSTANT i32 0
%zeros:_(<2 x s32>) = G_BUILD_VECTOR %zero, %zero
%and:_(<2 x s32>) = G_AND %x:_, %ones:_
%res:_(<2 x s32>) = G_SUB %zeros:_, %and:_
$vgpr0_vgpr1 = COPY %res:_(<2 x s32>)
...