[GlobalIsel] Combine G_ADD and G_SUB #92879

tschuett · 2024-05-21T08:03:59Z

No description provided.

llvmbot · 2024-05-21T08:04:32Z

@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-backend-aarch64

Author: Thorsten Schütt (tschuett)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/92879.diff

2 Files Affected:

(modified) llvm/include/llvm/Target/GlobalISel/Combine.td (+74-1)
(added) llvm/test/CodeGen/AArch64/GlobalISel/combine-integer.mir (+199)

diff --git a/llvm/include/llvm/Target/GlobalISel/Combine.td b/llvm/include/llvm/Target/GlobalISel/Combine.td
index 5d4b5a2479f6a..d9edca6f2bab5 100644
--- a/llvm/include/llvm/Target/GlobalISel/Combine.td
+++ b/llvm/include/llvm/Target/GlobalISel/Combine.td
@@ -1634,6 +1634,78 @@ extract_vector_element_shuffle_vector,
 insert_vector_element_extract_vector_element
 ]>;
 
+
+// fold ((0-A) + B) -> B-A
+def ZeroMinusAPlusB : GICombineRule<
+   (defs root:$root),
+   (match (G_SUB $sub, 0, $A),
+          (G_ADD $root, $sub, $B)),
+   (apply (G_SUB $root, $B, $A))>;
+
+// fold (A + (0-B)) -> A-B
+def APlusZeroMinusB : GICombineRule<
+   (defs root:$root),
+   (match (G_SUB $sub, 0, $B),
+          (G_ADD $root, $A, $sub)),
+   (apply (G_SUB $root, $A, $B))>;
+
+ // fold (A+(B-A)) -> B
+ def APlusBMinusB : GICombineRule<
+   (defs root:$root),
+   (match (G_SUB $sub, $B, $A),
+          (G_ADD $root, $A, $sub)),
+   (apply (GIReplaceReg $root, $B))>;
+
+// fold ((B-A)+A) -> B
+ def BMinusAPlusA : GICombineRule<
+   (defs root:$root),
+   (match (G_SUB $sub, $B, $A),
+          (G_ADD $root, $sub, $A)),
+   (apply (GIReplaceReg $root, $B))>;
+
+// fold ((A-B)+(C-A)) -> (C-B)
+def AMinusBPlusCMinusA : GICombineRule<
+   (defs root:$root),
+   (match (G_SUB $sub1, $A, $B),
+          (G_SUB $sub2, $C, $A),
+          (G_ADD $root, $sub1, $sub2)),
+   (apply (G_SUB $root, $C, $B))>;
+
+// fold ((A-B)+(B-C)) -> (A-C)
+def AMinusBPlusBMinusC : GICombineRule<
+   (defs root:$root),
+   (match (G_SUB $sub1, $A, $B),
+          (G_SUB $sub2, $B, $C),
+          (G_ADD $root, $sub1, $sub2)),
+   (apply (G_SUB $root, $A, $C))>;
+
+// fold (A+(B-(A+C))) to (B-C)
+def APlusBMinusAplusC : GICombineRule<
+   (defs root:$root),
+   (match (G_ADD $add1, $A, $C),
+          (G_SUB $sub1, $B, $add1),
+          (G_ADD $root, $A, $sub1)),
+   (apply (G_SUB $root, $B, $C))>;
+
+// fold (A+(B-(C+A))) to (B-C)
+def APlusBMinusCPlusA : GICombineRule<
+   (defs root:$root),
+   (match (G_ADD $add1, $C, $A),
+          (G_SUB $sub1, $B, $add1),
+          (G_ADD $root, $A, $sub1)),
+   (apply (G_SUB $root, $B, $C))>;
+
+def integer_reasso_combines: GICombineGroup<[
+ZeroMinusAPlusB,
+APlusZeroMinusB,
+APlusBMinusB,
+BMinusAPlusA,
+AMinusBPlusCMinusA,
+AMinusBPlusBMinusC,
+APlusBMinusAplusC,
+APlusBMinusCPlusA
+]>;
+
 // FIXME: These should use the custom predicate feature once it lands.
 def undef_combines : GICombineGroup<[undef_to_fp_zero, undef_to_int_zero,
                                      undef_to_negative_one,
@@ -1691,7 +1763,8 @@ def fma_combines : GICombineGroup<[combine_fadd_fmul_to_fmad_or_fma,
 def constant_fold_binops : GICombineGroup<[constant_fold_binop,
                                            constant_fold_fp_binop]>;
 
-def all_combines : GICombineGroup<[trivial_combines, vector_ops_combines,
+def all_combines : GICombineGroup<[integer_reasso_combines, trivial_combines,
+    vector_ops_combines,
     insert_vec_elt_combines, extract_vec_elt_combines, combines_for_extload,
     combine_extracted_vector_load,
     undef_combines, identity_combines, phi_combines,
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/combine-integer.mir b/llvm/test/CodeGen/AArch64/GlobalISel/combine-integer.mir
new file mode 100644
index 0000000000000..981e286f434fa
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/combine-integer.mir
@@ -0,0 +1,199 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple aarch64 -run-pass=aarch64-prelegalizer-combiner -verify-machineinstrs %s -o - | FileCheck %s
+# REQUIRES: asserts
+
+
+---
+name:   ZeroMinusAPlusB
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: ZeroMinusAPlusB
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %a:_(s32) = COPY $w0
+    ; CHECK-NEXT: %b:_(s32) = COPY $w0
+    ; CHECK-NEXT: %add:_(s32) = G_SUB %b, %a
+    ; CHECK-NEXT: $w0 = COPY %add(s32)
+    ; CHECK-NEXT: RET_ReallyLR implicit $w0
+    %x:_(s32) = COPY $w0
+    %a:_(s32) = COPY $w0
+    %b:_(s32) = COPY $w0
+    %zero:_(s32) = G_CONSTANT i32 0
+    %sub:_(s32) = G_SUB %zero, %a
+    %add:_(s32) = G_ADD %sub, %b
+    $w0 = COPY %add
+    RET_ReallyLR implicit $w0
+
+...
+---
+name:   APlusZeroMiunusB
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: APlusZeroMiunusB
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %a:_(s64) = COPY $x1
+    ; CHECK-NEXT: %b:_(s64) = COPY $x2
+    ; CHECK-NEXT: %add:_(s64) = G_SUB %a, %b
+    ; CHECK-NEXT: $x0 = COPY %add(s64)
+    ; CHECK-NEXT: RET_ReallyLR implicit $x0
+    %x:_(s64) = COPY $x0
+    %a:_(s64) = COPY $x1
+    %b:_(s64) = COPY $x2
+    %zero:_(s64) = G_CONSTANT i64 0
+    %sub:_(s64) = G_SUB %zero, %b
+    %add:_(s64) = G_ADD %a, %sub
+    $x0 = COPY %add
+    RET_ReallyLR implicit $x0
+
+...
+---
+name:   APlusBMinusB
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: APlusBMinusB
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %b:_(s64) = COPY $x1
+    ; CHECK-NEXT: $x0 = COPY %b(s64)
+    ; CHECK-NEXT: RET_ReallyLR implicit $x0
+    %a:_(s64) = COPY $x0
+    %b:_(s64) = COPY $x1
+    %zero:_(s64) = G_CONSTANT i64 0
+    %sub:_(s64) = G_SUB %b, %a
+    %add:_(s64) = G_ADD %a, %sub
+    $x0 = COPY %add
+    RET_ReallyLR implicit $x0
+
+...
+---
+name:   BMinusAPlusA
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: BMinusAPlusA
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %b:_(s64) = COPY $x1
+    ; CHECK-NEXT: $x0 = COPY %b(s64)
+    ; CHECK-NEXT: RET_ReallyLR implicit $x0
+    %a:_(s64) = COPY $x0
+    %b:_(s64) = COPY $x1
+    %zero:_(s64) = G_CONSTANT i64 0
+    %sub:_(s64) = G_SUB %b, %a
+    %add:_(s64) = G_ADD %sub, %a
+    $x0 = COPY %add
+    RET_ReallyLR implicit $x0
+
+...
+---
+name:   AMinusBPlusCMinusA
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: AMinusBPlusCMinusA
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %b:_(s64) = COPY $x1
+    ; CHECK-NEXT: %c:_(s64) = COPY $x2
+    ; CHECK-NEXT: %add:_(s64) = G_SUB %c, %b
+    ; CHECK-NEXT: $x0 = COPY %add(s64)
+    ; CHECK-NEXT: RET_ReallyLR implicit $x0
+    %a:_(s64) = COPY $x0
+    %b:_(s64) = COPY $x1
+    %c:_(s64) = COPY $x2
+    %zero:_(s64) = G_CONSTANT i64 0
+    %sub2:_(s64) = G_SUB %c, %a
+    %sub1:_(s64) = G_SUB %a, %b
+    %add:_(s64) = G_ADD %sub1, %sub2
+    $x0 = COPY %add
+    RET_ReallyLR implicit $x0
+
+...
+---
+name:   AMinusBPlusBMinusC
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: AMinusBPlusBMinusC
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %a:_(s64) = COPY $x0
+    ; CHECK-NEXT: %c:_(s64) = COPY $x2
+    ; CHECK-NEXT: %add:_(s64) = G_SUB %a, %c
+    ; CHECK-NEXT: $x0 = COPY %add(s64)
+    ; CHECK-NEXT: RET_ReallyLR implicit $x0
+    %a:_(s64) = COPY $x0
+    %b:_(s64) = COPY $x1
+    %c:_(s64) = COPY $x2
+    %zero:_(s64) = G_CONSTANT i64 0
+    %sub2:_(s64) = G_SUB %b, %c
+    %sub1:_(s64) = G_SUB %a, %b
+    %add:_(s64) = G_ADD %sub1, %sub2
+    $x0 = COPY %add
+    RET_ReallyLR implicit $x0
+
+
+...
+---
+name:   APlusBMinusAplusC
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: APlusBMinusAplusC
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %b:_(s64) = COPY $x1
+    ; CHECK-NEXT: %c:_(s64) = COPY $x2
+    ; CHECK-NEXT: %add:_(s64) = G_SUB %b, %c
+    ; CHECK-NEXT: $x0 = COPY %add(s64)
+    ; CHECK-NEXT: RET_ReallyLR implicit $x0
+    %a:_(s64) = COPY $x0
+    %b:_(s64) = COPY $x1
+    %c:_(s64) = COPY $x2
+    %zero:_(s64) = G_CONSTANT i64 0
+    %add1:_(s64) = G_ADD %a, %c
+    %sub1:_(s64) = G_SUB %b, %add1
+    %add:_(s64) = G_ADD %a, %sub1
+    $x0 = COPY %add
+    RET_ReallyLR implicit $x0
+
+...
+---
+name:   APlusBMinusCPlusA
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: APlusBMinusCPlusA
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %b:_(s64) = COPY $x1
+    ; CHECK-NEXT: %c:_(s64) = COPY $x2
+    ; CHECK-NEXT: %add:_(s64) = G_SUB %b, %c
+    ; CHECK-NEXT: $x0 = COPY %add(s64)
+    ; CHECK-NEXT: RET_ReallyLR implicit $x0
+    %a:_(s64) = COPY $x0
+    %b:_(s64) = COPY $x1
+    %c:_(s64) = COPY $x2
+    %zero:_(s64) = G_CONSTANT i64 0
+    %add1:_(s64) = G_ADD %c, %a
+    %sub1:_(s64) = G_SUB %b, %add1
+    %add:_(s64) = G_ADD %a, %sub1
+    $x0 = COPY %add
+    RET_ReallyLR implicit $x0
+
+
+
+
+

arsenm

All of these probably need a hasOneUse check?

llvm/include/llvm/Target/GlobalISel/Combine.td

arsenm · 2024-05-21T09:23:16Z

llvm/test/CodeGen/AArch64/GlobalISel/combine-integer.mir

+    $w0 = COPY %add
+    RET_ReallyLR implicit $w0
+
+...


Test the multi-use intermediate cases

tschuett · 2024-05-21T09:31:44Z

There are no hasOneUse checks in the dag. We would need to jump into C++ for the checks.

jayfoad · 2024-05-21T09:46:54Z

Combine integers

Please make the subject line more specific.

tschuett · 2024-05-22T05:41:13Z

They are from visitADDLike. There are no one-use checks. Fix the DAG or cheap and fast combines.

arsenm · 2024-05-22T12:19:58Z

There are no hasOneUse checks in the dag. We would need to jump into C++ for the checks.

We should have that? Arguably hasOneUse should be the default behavior for these patterns, you should need to do something to declare another use is OK

jayfoad · 2024-05-22T13:23:56Z

// fold ((0-A) + B) -> B-A

For cases like this it's not clear that a hasOneUse check is needed. If you do the transform when there are multiple uses you'll end up with the same number of operations but less latency, so it seems like a win. Before:

  sub x, 0, a
  add y, x, b // dependent on result of sub
  // both x and y are used after this

After:

  sub x, 0, a
  sub y, b, a // not dependent on first sub
  // both x and y are used after this

There might be register pressure effects but they are pretty hard to reason about at the GMIR level.

tschuett · 2024-05-22T15:21:52Z

From visitADDLike:

// fold ((0-A) + B) -> B-A
if (sd_match(N0, m_Neg(m_Value(A))))
    return DAG.getNode(ISD::SUB, DL, VT, N1, A);

llvm/include/llvm/Target/GlobalISel/Combine.td

llvm/test/CodeGen/AArch64/GlobalISel/combine-integer.mir

llvmbot added backend:AArch64 llvm:globalisel labels May 21, 2024

arsenm reviewed May 21, 2024

View reviewed changes

llvm/include/llvm/Target/GlobalISel/Combine.td Outdated Show resolved Hide resolved

arsenm reviewed May 21, 2024

View reviewed changes

tschuett changed the title ~~[GlobalIsel] Combine integers~~ [GlobalIsel] Combine G_ADD and G_SUB May 21, 2024

jayfoad requested a review from nikic May 22, 2024 13:24

arsenm reviewed May 22, 2024

View reviewed changes

llvm/include/llvm/Target/GlobalISel/Combine.td Outdated Show resolved Hide resolved

llvm/test/CodeGen/AArch64/GlobalISel/combine-integer.mir Outdated Show resolved Hide resolved

llvm/test/CodeGen/AArch64/GlobalISel/combine-integer.mir Outdated Show resolved Hide resolved

Thorsten Schütt added 4 commits May 23, 2024 07:54

[GlobalIsel] Combine integers

289b048

address review comments

287c2ee

vectors

cbf85de

indent

c8b920c

tschuett force-pushed the gisel-integer0 branch from 189e2ca to c8b920c Compare May 23, 2024 05:55

arsenm approved these changes May 23, 2024

View reviewed changes

arsenm reviewed May 23, 2024

View reviewed changes

llvm/test/CodeGen/AArch64/GlobalISel/combine-integer.mir Outdated Show resolved Hide resolved

white space

4826a0c

tschuett merged commit 9c60010 into llvm:main May 23, 2024
3 checks passed

tschuett deleted the gisel-integer0 branch May 23, 2024 14:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[GlobalIsel] Combine G_ADD and G_SUB #92879

[GlobalIsel] Combine G_ADD and G_SUB #92879

Uh oh!

tschuett commented May 21, 2024

Uh oh!

llvmbot commented May 21, 2024 •

edited

Loading

Uh oh!

arsenm left a comment

Uh oh!

Uh oh!

arsenm May 21, 2024

Uh oh!

tschuett commented May 21, 2024

Uh oh!

jayfoad commented May 21, 2024

Uh oh!

tschuett commented May 22, 2024

Uh oh!

arsenm commented May 22, 2024

Uh oh!

jayfoad commented May 22, 2024

Uh oh!

tschuett commented May 22, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

[GlobalIsel] Combine G_ADD and G_SUB #92879

[GlobalIsel] Combine G_ADD and G_SUB #92879

Uh oh!

Conversation

tschuett commented May 21, 2024

Uh oh!

llvmbot commented May 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arsenm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arsenm May 21, 2024

Choose a reason for hiding this comment

Uh oh!

tschuett commented May 21, 2024

Uh oh!

jayfoad commented May 21, 2024

Uh oh!

tschuett commented May 22, 2024

Uh oh!

arsenm commented May 22, 2024

Uh oh!

jayfoad commented May 22, 2024

Uh oh!

tschuett commented May 22, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

llvmbot commented May 21, 2024 •

edited

Loading