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.

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

[GlobalISel] Combine (X == 0) & (Y == 0) -> (X | Y) == 0 #71949

Merged

Conversation

dfszabo
Copy link
Contributor

@dfszabo dfszabo commented Nov 10, 2023

Also combine (X != 0) | (Y != 0) -> (X | Y) != 0

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 10, 2023

@llvm/pr-subscribers-llvm-globalisel

@llvm/pr-subscribers-backend-aarch64

Author: Dávid Ferenc Szabó (dfszabo)

Changes

Also combine (X != 0) | (Y != 0) -> (X | Y) != 0


Patch is 30.18 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/71949.diff

3 Files Affected:

  • (modified) llvm/include/llvm/Target/GlobalISel/Combine.td (+32-1)
  • (added) llvm/test/CodeGen/AArch64/GlobalISel/combine-2-icmps-of-0-and-or.mir (+764)
  • (modified) llvm/test/CodeGen/AArch64/cmp-chains.ll (+13-36)
diff --git a/llvm/include/llvm/Target/GlobalISel/Combine.td b/llvm/include/llvm/Target/GlobalISel/Combine.td
index 7e0691e1ee95048..551fa812cee21ed 100644
--- a/llvm/include/llvm/Target/GlobalISel/Combine.td
+++ b/llvm/include/llvm/Target/GlobalISel/Combine.td
@@ -903,6 +903,37 @@ def redundant_binop_in_equality : GICombineRule<
          [{ return Helper.matchRedundantBinOpInEquality(*${root}, ${info}); }]),
   (apply [{ Helper.applyBuildFn(*${root}, ${info}); }])>;
 
+// Transform: (X == 0 & Y == 0) -> (X | Y) == 0
+def double_icmp_zero_and_combine: GICombineRule<
+  (defs root:$root),
+  (match (G_ICMP $d1, $p, $s1, 0),
+         (G_ICMP $d2, $p, $s2, 0),
+         (G_AND $root, $d1, $d2),
+         [{ return ${p}.getPredicate() == CmpInst::ICMP_EQ &&
+                       !MRI.getType(${s1}.getReg()).isPointer() &&
+                       (MRI.getType(${s1}.getReg()) ==
+                           MRI.getType(${s2}.getReg())); }]),
+  (apply (G_OR i32:$ordst, $s1, $s2),
+         (G_ICMP $root, $p, $ordst, 0))
+>;
+
+// Transform: (X != 0 | Y != 0) -> (X | Y) != 0
+def double_icmp_zero_or_combine: GICombineRule<
+  (defs root:$root),
+  (match (G_ICMP $d1, $p, $s1, 0),
+         (G_ICMP $d2, $p, $s2, 0),
+         (G_OR $root, $d1, $d2),
+         [{ return ${p}.getPredicate() == CmpInst::ICMP_NE &&
+                       !MRI.getType(${s1}.getReg()).isPointer() &&
+                       (MRI.getType(${s1}.getReg()) ==
+                           MRI.getType(${s2}.getReg())); }]),
+  (apply (G_OR i32:$ordst, $s1, $s2),
+         (G_ICMP $root, $p, $ordst, 0))
+>;
+
+def double_icmp_zero_and_or_combine : GICombineGroup<[double_icmp_zero_and_combine,
+                                                      double_icmp_zero_or_combine]>;
+
 def and_or_disjoint_mask : GICombineRule<
   (defs root:$root, build_fn_matchinfo:$info),
   (match (wip_match_opcode G_AND):$root,
@@ -1265,7 +1296,7 @@ def all_combines : GICombineGroup<[trivial_combines, insert_vec_elt_combines,
     intdiv_combines, mulh_combines, redundant_neg_operands,
     and_or_disjoint_mask, fma_combines, fold_binop_into_select,
     sub_add_reg, select_to_minmax, redundant_binop_in_equality,
-    fsub_to_fneg, commute_constant_to_rhs]>;
+    double_icmp_zero_and_or_combine, fsub_to_fneg, commute_constant_to_rhs]>;
 
 // A combine group used to for prelegalizer combiners at -O0. The combines in
 // this group have been selected based on experiments to balance code size and
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/combine-2-icmps-of-0-and-or.mir b/llvm/test/CodeGen/AArch64/GlobalISel/combine-2-icmps-of-0-and-or.mir
new file mode 100644
index 000000000000000..a285ccf0c88fea4
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/combine-2-icmps-of-0-and-or.mir
@@ -0,0 +1,764 @@
+# 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:            valid_and_eq_0_eq_0_s32
+tracksRegLiveness: true
+legalized: true
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: valid_and_eq_0_eq_0_s32
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %x:_(s32) = COPY $w0
+    ; CHECK-NEXT: %y:_(s32) = COPY $w1
+    ; CHECK-NEXT: %zero:_(s32) = G_CONSTANT i32 0
+    ; CHECK-NEXT: [[OR:%[0-9]+]]:_(s32) = G_OR %x, %y
+    ; CHECK-NEXT: %and:_(s1) = G_ICMP intpred(eq), [[OR]](s32), %zero
+    ; CHECK-NEXT: %zext:_(s32) = G_ZEXT %and(s1)
+    ; CHECK-NEXT: $w0 = COPY %zext(s32)
+    ; CHECK-NEXT: RET_ReallyLR implicit $w0
+    %x:_(s32) = COPY $w0
+    %y:_(s32) = COPY $w1
+    %zero:_(s32) = G_CONSTANT i32 0
+    %cmp1:_(s1) = G_ICMP intpred(eq), %x:_(s32), %zero:_
+    %cmp2:_(s1) = G_ICMP intpred(eq), %y:_(s32), %zero:_
+    %and:_(s1) = G_AND %cmp1, %cmp2
+    %zext:_(s32) = G_ZEXT %and:_(s1)
+    $w0 = COPY %zext
+    RET_ReallyLR implicit $w0
+
+...
+---
+name:            invalid_and_eq_1_eq_0_s32
+tracksRegLiveness: true
+legalized: true
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: invalid_and_eq_1_eq_0_s32
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %x:_(s32) = COPY $w0
+    ; CHECK-NEXT: %y:_(s32) = COPY $w1
+    ; CHECK-NEXT: %zero:_(s32) = G_CONSTANT i32 0
+    ; CHECK-NEXT: %one:_(s32) = G_CONSTANT i32 1
+    ; CHECK-NEXT: %cmp1:_(s1) = G_ICMP intpred(eq), %x(s32), %one
+    ; CHECK-NEXT: %cmp2:_(s1) = G_ICMP intpred(eq), %y(s32), %zero
+    ; CHECK-NEXT: %and:_(s1) = G_AND %cmp1, %cmp2
+    ; CHECK-NEXT: %zext:_(s32) = G_ZEXT %and(s1)
+    ; CHECK-NEXT: $w0 = COPY %zext(s32)
+    ; CHECK-NEXT: RET_ReallyLR implicit $w0
+    %x:_(s32) = COPY $w0
+    %y:_(s32) = COPY $w1
+    %zero:_(s32) = G_CONSTANT i32 0
+    %one:_(s32) = G_CONSTANT i32 1
+    %cmp1:_(s1) = G_ICMP intpred(eq), %x:_(s32), %one:_
+    %cmp2:_(s1) = G_ICMP intpred(eq), %y:_(s32), %zero:_
+    %and:_(s1) = G_AND %cmp1, %cmp2
+    %zext:_(s32) = G_ZEXT %and:_(s1)
+    $w0 = COPY %zext
+    RET_ReallyLR implicit $w0
+
+...
+---
+name:            invalid_and_eq_0_eq_1_s32
+tracksRegLiveness: true
+legalized: true
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: invalid_and_eq_0_eq_1_s32
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %x:_(s32) = COPY $w0
+    ; CHECK-NEXT: %y:_(s32) = COPY $w1
+    ; CHECK-NEXT: %zero:_(s32) = G_CONSTANT i32 0
+    ; CHECK-NEXT: %one:_(s32) = G_CONSTANT i32 1
+    ; CHECK-NEXT: %cmp1:_(s1) = G_ICMP intpred(eq), %x(s32), %zero
+    ; CHECK-NEXT: %cmp2:_(s1) = G_ICMP intpred(eq), %y(s32), %one
+    ; CHECK-NEXT: %and:_(s1) = G_AND %cmp1, %cmp2
+    ; CHECK-NEXT: %zext:_(s32) = G_ZEXT %and(s1)
+    ; CHECK-NEXT: $w0 = COPY %zext(s32)
+    ; CHECK-NEXT: RET_ReallyLR implicit $w0
+    %x:_(s32) = COPY $w0
+    %y:_(s32) = COPY $w1
+    %zero:_(s32) = G_CONSTANT i32 0
+    %one:_(s32) = G_CONSTANT i32 1
+    %cmp1:_(s1) = G_ICMP intpred(eq), %x:_(s32), %zero:_
+    %cmp2:_(s1) = G_ICMP intpred(eq), %y:_(s32), %one:_
+    %and:_(s1) = G_AND %cmp1, %cmp2
+    %zext:_(s32) = G_ZEXT %and:_(s1)
+    $w0 = COPY %zext
+    RET_ReallyLR implicit $w0
+
+...
+---
+name:            invalid_and_ne_0_eq_0_s32
+tracksRegLiveness: true
+legalized: true
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: invalid_and_ne_0_eq_0_s32
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %x:_(s32) = COPY $w0
+    ; CHECK-NEXT: %y:_(s32) = COPY $w1
+    ; CHECK-NEXT: %zero:_(s32) = G_CONSTANT i32 0
+    ; CHECK-NEXT: %cmp1:_(s1) = G_ICMP intpred(ne), %x(s32), %zero
+    ; CHECK-NEXT: %cmp2:_(s1) = G_ICMP intpred(eq), %y(s32), %zero
+    ; CHECK-NEXT: %and:_(s1) = G_AND %cmp1, %cmp2
+    ; CHECK-NEXT: %zext:_(s32) = G_ZEXT %and(s1)
+    ; CHECK-NEXT: $w0 = COPY %zext(s32)
+    ; CHECK-NEXT: RET_ReallyLR implicit $w0
+    %x:_(s32) = COPY $w0
+    %y:_(s32) = COPY $w1
+    %zero:_(s32) = G_CONSTANT i32 0
+    %cmp1:_(s1) = G_ICMP intpred(ne), %x:_(s32), %zero:_
+    %cmp2:_(s1) = G_ICMP intpred(eq), %y:_(s32), %zero:_
+    %and:_(s1) = G_AND %cmp1, %cmp2
+    %zext:_(s32) = G_ZEXT %and:_(s1)
+    $w0 = COPY %zext
+    RET_ReallyLR implicit $w0
+
+...
+---
+name:            invalid_and_eq_0_ne_0_s32
+tracksRegLiveness: true
+legalized: true
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: invalid_and_eq_0_ne_0_s32
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %x:_(s32) = COPY $w0
+    ; CHECK-NEXT: %y:_(s32) = COPY $w1
+    ; CHECK-NEXT: %zero:_(s32) = G_CONSTANT i32 0
+    ; CHECK-NEXT: %cmp1:_(s1) = G_ICMP intpred(eq), %x(s32), %zero
+    ; CHECK-NEXT: %cmp2:_(s1) = G_ICMP intpred(ne), %y(s32), %zero
+    ; CHECK-NEXT: %and:_(s1) = G_AND %cmp1, %cmp2
+    ; CHECK-NEXT: %zext:_(s32) = G_ZEXT %and(s1)
+    ; CHECK-NEXT: $w0 = COPY %zext(s32)
+    ; CHECK-NEXT: RET_ReallyLR implicit $w0
+    %x:_(s32) = COPY $w0
+    %y:_(s32) = COPY $w1
+    %zero:_(s32) = G_CONSTANT i32 0
+    %cmp1:_(s1) = G_ICMP intpred(eq), %x:_(s32), %zero:_
+    %cmp2:_(s1) = G_ICMP intpred(ne), %y:_(s32), %zero:_
+    %and:_(s1) = G_AND %cmp1, %cmp2
+    %zext:_(s32) = G_ZEXT %and:_(s1)
+    $w0 = COPY %zext
+    RET_ReallyLR implicit $w0
+
+...
+---
+name:            invalid_and_ne_0_ne_0_s32
+tracksRegLiveness: true
+legalized: true
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: invalid_and_ne_0_ne_0_s32
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %x:_(s32) = COPY $w0
+    ; CHECK-NEXT: %y:_(s32) = COPY $w1
+    ; CHECK-NEXT: %zero:_(s32) = G_CONSTANT i32 0
+    ; CHECK-NEXT: %cmp1:_(s1) = G_ICMP intpred(ne), %x(s32), %zero
+    ; CHECK-NEXT: %cmp2:_(s1) = G_ICMP intpred(ne), %y(s32), %zero
+    ; CHECK-NEXT: %and:_(s1) = G_AND %cmp1, %cmp2
+    ; CHECK-NEXT: %zext:_(s32) = G_ZEXT %and(s1)
+    ; CHECK-NEXT: $w0 = COPY %zext(s32)
+    ; CHECK-NEXT: RET_ReallyLR implicit $w0
+    %x:_(s32) = COPY $w0
+    %y:_(s32) = COPY $w1
+    %zero:_(s32) = G_CONSTANT i32 0
+    %cmp1:_(s1) = G_ICMP intpred(ne), %x:_(s32), %zero:_
+    %cmp2:_(s1) = G_ICMP intpred(ne), %y:_(s32), %zero:_
+    %and:_(s1) = G_AND %cmp1, %cmp2
+    %zext:_(s32) = G_ZEXT %and:_(s1)
+    $w0 = COPY %zext
+    RET_ReallyLR implicit $w0
+
+...
+---
+name:            valid_or_ne_0_ne_0_s32
+tracksRegLiveness: true
+legalized: true
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: valid_or_ne_0_ne_0_s32
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %x:_(s32) = COPY $w0
+    ; CHECK-NEXT: %y:_(s32) = COPY $w1
+    ; CHECK-NEXT: %zero:_(s32) = G_CONSTANT i32 0
+    ; CHECK-NEXT: [[OR:%[0-9]+]]:_(s32) = G_OR %x, %y
+    ; CHECK-NEXT: %or:_(s1) = G_ICMP intpred(ne), [[OR]](s32), %zero
+    ; CHECK-NEXT: %zext:_(s32) = G_ZEXT %or(s1)
+    ; CHECK-NEXT: $w0 = COPY %zext(s32)
+    ; CHECK-NEXT: RET_ReallyLR implicit $w0
+    %x:_(s32) = COPY $w0
+    %y:_(s32) = COPY $w1
+    %zero:_(s32) = G_CONSTANT i32 0
+    %cmp1:_(s1) = G_ICMP intpred(ne), %x:_(s32), %zero:_
+    %cmp2:_(s1) = G_ICMP intpred(ne), %y:_(s32), %zero:_
+    %or:_(s1) = G_OR %cmp1, %cmp2
+    %zext:_(s32) = G_ZEXT %or:_(s1)
+    $w0 = COPY %zext
+    RET_ReallyLR implicit $w0
+
+...
+---
+name:            invalid_or_ne_1_ne_0_s32
+tracksRegLiveness: true
+legalized: true
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: invalid_or_ne_1_ne_0_s32
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %x:_(s32) = COPY $w0
+    ; CHECK-NEXT: %y:_(s32) = COPY $w1
+    ; CHECK-NEXT: %zero:_(s32) = G_CONSTANT i32 0
+    ; CHECK-NEXT: %one:_(s32) = G_CONSTANT i32 1
+    ; CHECK-NEXT: %cmp1:_(s1) = G_ICMP intpred(ne), %x(s32), %one
+    ; CHECK-NEXT: %cmp2:_(s1) = G_ICMP intpred(ne), %y(s32), %zero
+    ; CHECK-NEXT: %or:_(s1) = G_OR %cmp1, %cmp2
+    ; CHECK-NEXT: %zext:_(s32) = G_ZEXT %or(s1)
+    ; CHECK-NEXT: $w0 = COPY %zext(s32)
+    ; CHECK-NEXT: RET_ReallyLR implicit $w0
+    %x:_(s32) = COPY $w0
+    %y:_(s32) = COPY $w1
+    %zero:_(s32) = G_CONSTANT i32 0
+    %one:_(s32) = G_CONSTANT i32 1
+    %cmp1:_(s1) = G_ICMP intpred(ne), %x:_(s32), %one:_
+    %cmp2:_(s1) = G_ICMP intpred(ne), %y:_(s32), %zero:_
+    %or:_(s1) = G_OR %cmp1, %cmp2
+    %zext:_(s32) = G_ZEXT %or:_(s1)
+    $w0 = COPY %zext
+    RET_ReallyLR implicit $w0
+
+...
+---
+name:            invalid_or_ne_0_ne_1_s32
+tracksRegLiveness: true
+legalized: true
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: invalid_or_ne_0_ne_1_s32
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %x:_(s32) = COPY $w0
+    ; CHECK-NEXT: %y:_(s32) = COPY $w1
+    ; CHECK-NEXT: %zero:_(s32) = G_CONSTANT i32 0
+    ; CHECK-NEXT: %one:_(s32) = G_CONSTANT i32 1
+    ; CHECK-NEXT: %cmp1:_(s1) = G_ICMP intpred(ne), %x(s32), %zero
+    ; CHECK-NEXT: %cmp2:_(s1) = G_ICMP intpred(ne), %y(s32), %one
+    ; CHECK-NEXT: %or:_(s1) = G_OR %cmp1, %cmp2
+    ; CHECK-NEXT: %zext:_(s32) = G_ZEXT %or(s1)
+    ; CHECK-NEXT: $w0 = COPY %zext(s32)
+    ; CHECK-NEXT: RET_ReallyLR implicit $w0
+    %x:_(s32) = COPY $w0
+    %y:_(s32) = COPY $w1
+    %zero:_(s32) = G_CONSTANT i32 0
+    %one:_(s32) = G_CONSTANT i32 1
+    %cmp1:_(s1) = G_ICMP intpred(ne), %x:_(s32), %zero:_
+    %cmp2:_(s1) = G_ICMP intpred(ne), %y:_(s32), %one:_
+    %or:_(s1) = G_OR %cmp1, %cmp2
+    %zext:_(s32) = G_ZEXT %or:_(s1)
+    $w0 = COPY %zext
+    RET_ReallyLR implicit $w0
+
+...
+---
+name:            invalid_or_eq_0_ne_0_s32
+tracksRegLiveness: true
+legalized: true
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: invalid_or_eq_0_ne_0_s32
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %x:_(s32) = COPY $w0
+    ; CHECK-NEXT: %y:_(s32) = COPY $w1
+    ; CHECK-NEXT: %zero:_(s32) = G_CONSTANT i32 0
+    ; CHECK-NEXT: %cmp1:_(s1) = G_ICMP intpred(eq), %x(s32), %zero
+    ; CHECK-NEXT: %cmp2:_(s1) = G_ICMP intpred(ne), %y(s32), %zero
+    ; CHECK-NEXT: %or:_(s1) = G_OR %cmp1, %cmp2
+    ; CHECK-NEXT: %zext:_(s32) = G_ZEXT %or(s1)
+    ; CHECK-NEXT: $w0 = COPY %zext(s32)
+    ; CHECK-NEXT: RET_ReallyLR implicit $w0
+    %x:_(s32) = COPY $w0
+    %y:_(s32) = COPY $w1
+    %zero:_(s32) = G_CONSTANT i32 0
+    %cmp1:_(s1) = G_ICMP intpred(eq), %x:_(s32), %zero:_
+    %cmp2:_(s1) = G_ICMP intpred(ne), %y:_(s32), %zero:_
+    %or:_(s1) = G_OR %cmp1, %cmp2
+    %zext:_(s32) = G_ZEXT %or:_(s1)
+    $w0 = COPY %zext
+    RET_ReallyLR implicit $w0
+
+...
+---
+name:            invalid_or_ne_0_eq_0_s32
+tracksRegLiveness: true
+legalized: true
+body:             |
+  bb.0:
+    liveins: $w0, $w1
+
+    ; CHECK-LABEL: name: invalid_or_ne_0_eq_0_s32
+    ; CHECK: liveins: $w0, $w1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %x:_(s32) = COPY $w0
+    ; CHECK-NEXT: %y:_(s32) = COPY $w1
+    ; CHECK-NEXT: %zero:_(s32) = G_CONSTANT i32 0
+    ; CHECK-NEXT: %cmp1:_(s1) = G_ICMP intpred(ne), %x(s32), %zero
+    ; CHECK-NEXT: %cmp2:_(s1) = G_ICMP intpred(eq), %y(s32), %zero
+    ; CHECK-NEXT: %or:_(s1) = G_OR %cmp1, %cmp2
+    ; CHECK-NEXT: %zext:_(s32) = G_ZEXT %or(s1)
+    ; CHECK-NEXT: $w0 = COPY %zext(s32)
+    ; CHECK-NEXT: RET_ReallyLR implicit $w0
+    %x:_(s32) = COPY $w0
+    %y:_(s32) = COPY $w1
+    %zero:_(s32) = G_CONSTANT i32 0
+    %cmp1:_(s1) = G_ICMP intpred(ne), %x:_(s32), %zero:_
+    %cmp2:_(s1) = G_ICMP intpred(eq), %y:_(s32), %zero:_
+    %or:_(s1) = G_OR %cmp1, %cmp2
+    %zext:_(s32) = G_ZEXT %or:_(s1)
+    $w0 = COPY %zext
+    RET_ReallyLR implicit $w0
+
+...
+
+---
+name:            valid_and_eq_0_eq_0_s64
+tracksRegLiveness: true
+legalized: true
+body:             |
+  bb.0:
+    liveins: $x0, $x1
+
+    ; CHECK-LABEL: name: valid_and_eq_0_eq_0_s64
+    ; CHECK: liveins: $x0, $x1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %x:_(s64) = COPY $x0
+    ; CHECK-NEXT: %y:_(s64) = COPY $x1
+    ; CHECK-NEXT: %zero:_(s64) = G_CONSTANT i64 0
+    ; CHECK-NEXT: [[OR:%[0-9]+]]:_(s64) = G_OR %x, %y
+    ; CHECK-NEXT: %and:_(s1) = G_ICMP intpred(eq), [[OR]](s64), %zero
+    ; CHECK-NEXT: %zext:_(s64) = G_ZEXT %and(s1)
+    ; CHECK-NEXT: $x0 = COPY %zext(s64)
+    ; CHECK-NEXT: RET_ReallyLR implicit $x0
+    %x:_(s64) = COPY $x0
+    %y:_(s64) = COPY $x1
+    %zero:_(s64) = G_CONSTANT i64 0
+    %cmp1:_(s1) = G_ICMP intpred(eq), %x:_(s64), %zero:_
+    %cmp2:_(s1) = G_ICMP intpred(eq), %y:_(s64), %zero:_
+    %and:_(s1) = G_AND %cmp1, %cmp2
+    %zext:_(s64) = G_ZEXT %and:_(s1)
+    $x0 = COPY %zext
+    RET_ReallyLR implicit $x0
+
+...
+---
+name:            invalid_and_eq_1_eq_0_s64
+tracksRegLiveness: true
+legalized: true
+body:             |
+  bb.0:
+    liveins: $x0, $x1
+
+    ; CHECK-LABEL: name: invalid_and_eq_1_eq_0_s64
+    ; CHECK: liveins: $x0, $x1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %x:_(s64) = COPY $x0
+    ; CHECK-NEXT: %y:_(s64) = COPY $x1
+    ; CHECK-NEXT: %zero:_(s64) = G_CONSTANT i64 0
+    ; CHECK-NEXT: %one:_(s64) = G_CONSTANT i64 1
+    ; CHECK-NEXT: %cmp1:_(s1) = G_ICMP intpred(eq), %x(s64), %one
+    ; CHECK-NEXT: %cmp2:_(s1) = G_ICMP intpred(eq), %y(s64), %zero
+    ; CHECK-NEXT: %and:_(s1) = G_AND %cmp1, %cmp2
+    ; CHECK-NEXT: %zext:_(s64) = G_ZEXT %and(s1)
+    ; CHECK-NEXT: $x0 = COPY %zext(s64)
+    ; CHECK-NEXT: RET_ReallyLR implicit $x0
+    %x:_(s64) = COPY $x0
+    %y:_(s64) = COPY $x1
+    %zero:_(s64) = G_CONSTANT i64 0
+    %one:_(s64) = G_CONSTANT i64 1
+    %cmp1:_(s1) = G_ICMP intpred(eq), %x:_(s64), %one:_
+    %cmp2:_(s1) = G_ICMP intpred(eq), %y:_(s64), %zero:_
+    %and:_(s1) = G_AND %cmp1, %cmp2
+    %zext:_(s64) = G_ZEXT %and:_(s1)
+    $x0 = COPY %zext
+    RET_ReallyLR implicit $x0
+
+...
+---
+name:            invalid_and_eq_0_eq_1_s64
+tracksRegLiveness: true
+legalized: true
+body:             |
+  bb.0:
+    liveins: $x0, $x1
+
+    ; CHECK-LABEL: name: invalid_and_eq_0_eq_1_s64
+    ; CHECK: liveins: $x0, $x1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %x:_(s64) = COPY $x0
+    ; CHECK-NEXT: %y:_(s64) = COPY $x1
+    ; CHECK-NEXT: %zero:_(s64) = G_CONSTANT i64 0
+    ; CHECK-NEXT: %one:_(s64) = G_CONSTANT i64 1
+    ; CHECK-NEXT: %cmp1:_(s1) = G_ICMP intpred(eq), %x(s64), %zero
+    ; CHECK-NEXT: %cmp2:_(s1) = G_ICMP intpred(eq), %y(s64), %one
+    ; CHECK-NEXT: %and:_(s1) = G_AND %cmp1, %cmp2
+    ; CHECK-NEXT: %zext:_(s64) = G_ZEXT %and(s1)
+    ; CHECK-NEXT: $x0 = COPY %zext(s64)
+    ; CHECK-NEXT: RET_ReallyLR implicit $x0
+    %x:_(s64) = COPY $x0
+    %y:_(s64) = COPY $x1
+    %zero:_(s64) = G_CONSTANT i64 0
+    %one:_(s64) = G_CONSTANT i64 1
+    %cmp1:_(s1) = G_ICMP intpred(eq), %x:_(s64), %zero:_
+    %cmp2:_(s1) = G_ICMP intpred(eq), %y:_(s64), %one:_
+    %and:_(s1) = G_AND %cmp1, %cmp2
+    %zext:_(s64) = G_ZEXT %and:_(s1)
+    $x0 = COPY %zext
+    RET_ReallyLR implicit $x0
+
+...
+---
+name:            invalid_and_ne_0_eq_0_s64
+tracksRegLiveness: true
+legalized: true
+body:             |
+  bb.0:
+    liveins: $x0, $x1
+
+    ; CHECK-LABEL: name: invalid_and_ne_0_eq_0_s64
+    ; CHECK: liveins: $x0, $x1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %x:_(s64) = COPY $x0
+    ; CHECK-NEXT: %y:_(s64) = COPY $x1
+    ; CHECK-NEXT: %zero:_(s64) = G_CONSTANT i64 0
+    ; CHECK-NEXT: %cmp1:_(s1) = G_ICMP intpred(ne), %x(s64), %zero
+    ; CHECK-NEXT: %cmp2:_(s1) = G_ICMP intpred(eq), %y(s64), %zero
+    ; CHECK-NEXT: %and:_(s1) = G_AND %cmp1, %cmp2
+    ; CHECK-NEXT: %zext:_(s64) = G_ZEXT %and(s1)
+    ; CHECK-NEXT: $x0 = COPY %zext(s64)
+    ; CHECK-NEXT: RET_ReallyLR implicit $x0
+    %x:_(s64) = COPY $x0
+    %y:_(s64) = COPY $x1
+    %zero:_(s64) = G_CONSTANT i64 0
+    %cmp1:_(s1) = G_ICMP intpred(ne), %x:_(s64), %zero:_
+    %cmp2:_(s1) = G_ICMP intpred(eq), %y:_(s64), %zero:_
+    %and:_(s1) = G_AND %cmp1, %cmp2
+    %zext:_(s64) = G_ZEXT %and:_(s1)
+    $x0 = COPY %zext
+    RET_ReallyLR implicit $x0
+
+...
+---
+name:            invalid_and_eq_0_ne_0_s64
+tracksRegLiveness: true
+legalized: true
+body:             |
+  bb.0:
+    liveins: $x0, $x1
+
+    ; CHECK-LABEL: name: invalid_and_eq_0_ne_0_s64
+    ; CHECK: liveins: $x0, $x1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %x:_(s64) = COPY $x0
+    ; CHECK-NEXT: %y:_(s64) = COPY $x1
+    ; CHECK-NEXT: %zero:_(s64) = G_CONSTANT i64 0
+    ; CHECK-NEXT: %cmp1:_(s1) = G_ICMP intpred(eq), %x(s64), %zero
+    ; CHECK-NEXT: %cmp2:_(s1) = G_ICMP intpred(ne), %y(s64), %zero
+    ; CHECK-NEXT: %and:_(s1) = G_AND %cmp1, %cmp2
+    ; CHECK-NEXT: %zext:_(s64) = G_ZEXT %and(s1)
+    ; CHECK-NEXT: $x0 = COPY %zext(s64)
+    ; CHECK-NEXT: RET_ReallyLR implicit $x0
+    %x:_(s64) = COPY $x0
+    %y:_(s64) = COPY $x1
+    %zero:_(s64) = G_CONSTANT i64 0
+    %cmp1:_(s1) = G_ICMP intpred(eq), %x:_(s64), %zero:_
+    %cmp2:_(s1) = G_ICMP intpred(ne), %y:_(s64), %zero:_
+    %and:_(s1) = G_AND %cmp1, %cmp2
+    %zext:_(s64) = G_ZEXT %and:_(s1)
+    $x0 = COPY %zext
+    RET_ReallyLR implicit $x0
+
+...
+---
+name:            invalid_and_ne_0_ne_0_s64
+tracksRegLiveness: true
+legalized: true
+body:             |
+  bb.0:
+    liveins: $x0, $x1
+
+    ; CHECK-LABEL: name: invalid_and_ne_0_ne_0_s64
+    ; CHECK: liveins: $x0, $x1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: %x:_(s64) = COPY $x0
+    ; CHECK-NEXT: %y:_(s64) = COPY $x1
+    ; CHECK-NEXT: %zero:_(s64) = ...
[truncated]

@dfszabo
Copy link
Contributor Author

dfszabo commented Nov 10, 2023

Uploading this version for debug purposes. The original one is here: #69017.

Notes for now I used i32 as type for ordst, but it should be the type of the G_ICMP's source.

Findings:
With cpp code:

CSEInfo::Found Instr %2:_(s32) = G_CONSTANT i32 0
Creating: G_OR

CSEInfo::Recording new MI G_OR
Creating: G_OR

CSEInfo::Recording new MI G_OR
Creating: G_ICMP

Creating: G_ICMP

Erasing: %5:_(s1) = G_OR %3:_, %4:_

927: GIR_Done
Created: %8:_(s32) = G_OR %0:_, %1:_
Created: %5:_(s1) = G_ICMP intpred(ne), %8:_(s32), %2:_

Try combining %5:_(s1) = G_ICMP intpred(ne), %8:_(s32), %2:_

With this version:

1035: TempRegs[0] = GIR_MakeTempReg(0)
Creating: G_OR

CSEInfo::Recording new MI G_OR
1038: GIR_BuildMI(OutMIs[0], 57)
1042: GIR_AddTempRegister(OutMIs[0], TempRegisters[0], 0)
1046: GIR_Copy(OutMIs[0], MIs[1], 2)
1050: GIR_Copy(OutMIs[0], MIs[2], 2)
1052: GIR_EraseFromParent(MIs[0])
Erasing: %5:_(s1) = G_OR %3:_, %4:_

Erasing: %5:_(s1) = G_OR %3:_, %4:_

PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /home/david/work/llvm-community/build/bin/llc -mtriple=aarch64-- -global-isel -debug
1.	Running pass 'Function Pass Manager' on module '<stdin>'.
2.	Running pass 'AArch64PreLegalizerCombiner' on function '@true_or2'
 #0 0x00007f7d339477b2 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /home/david/work/llvm-community/llvm-project/llvm/lib/Support/Unix/Signals.inc:723:22
 #1 0x00007f7d33947bc3 PrintStackTraceSignalHandler(void*) /home/david/work/llvm-community/llvm-project/llvm/lib/Support/Unix/Signals.inc:798:1
 #2 0x00007f7d33945146 llvm::sys::RunSignalHandlers() /home/david/work/llvm-community/llvm-project/llvm/lib/Support/Signals.cpp:105:20
 #3 0x00007f7d3394706c SignalHandler(int) /home/david/work/llvm-community/llvm-project/llvm/lib/Support/Unix/Signals.inc:413:1
 #4 0x00007f7d3312b0c0 (/lib/x86_64-linux-gnu/libc.so.6+0x430c0)
 #5 0x00007f7d3ad7d65a llvm::MachineBasicBlock::getParent() /home/david/work/llvm-community/llvm-project/llvm/include/llvm/CodeGen/MachineBasicBlock.h:275:41
 #6 0x00007f7d3adcb1cc llvm::BuildMI(llvm::MachineBasicBlock&, llvm::MachineInstrBundleIterator<llvm::MachineInstr, false>, llvm::MIMetadata const&, llvm::MCInstrDesc const&) /home/david/work/llvm-community/llvm-project/llvm/include/llvm/CodeGen/MachineInstrBuilder.h:435:38
 #7 0x00007f7d3adcb3fe llvm::BuildMI(llvm::MachineBasicBlock&, llvm::MachineInstr&, llvm::MIMetadata const&, llvm::MCInstrDesc const&) /home/david/work/llvm-community/llvm-project/llvm/include/llvm/CodeGen/MachineInstrBuilder.h:459:1
 #8 0x00007f7d3adcb448 llvm::BuildMI(llvm::MachineBasicBlock&, llvm::MachineInstr*, llvm::MIMetadata const&, llvm::MCInstrDesc const&) /home/david/work/llvm-community/llvm-project/llvm/include/llvm/CodeGen/MachineInstrBuilder.h:465:1
 #9 0x00007f7d3ae2a39d bool llvm::GIMatchTableExecutor::executeMatchTable<(anonymous namespace)::AArch64PreLegalizerCombinerImpl const, llvm::Bitset<0u>, std::optional<llvm::SmallVector<std::function<void (llvm::MachineInstrBuilder&)>, 4u>> ((anonymous namespace)::AArch64PreLegalizerCombinerImpl::*)(llvm::MachineOperand&) const, void ((anonymous namespace)::AArch64PreLegalizerCombinerImpl::*)(llvm::MachineInstrBuilder&, llvm::MachineInstr const&, int) const>((anonymous namespace)::AArch64PreLegalizerCombinerImpl const&, llvm::SmallVector<llvm::MachineInstrBuilder, 4u>&, llvm::GIMatchTableExecutor::MatcherState&, llvm::GIMatchTableExecutor::ExecInfoTy<llvm::Bitset<0u>, std::optional<llvm::SmallVector<std::function<void (llvm::MachineInstrBuilder&)>, 4u>> ((anonymous namespace)::AArch64PreLegalizerCombinerImpl::*)(llvm::MachineOperand&) const, void ((anonymous namespace)::AArch64PreLegalizerCombinerImpl::*)(llvm::MachineInstrBuilder&, llvm::MachineInstr const&, int) const> const&, long const*, llvm::TargetInstrInfo const&, llvm::MachineRegisterInfo&, llvm::TargetRegisterInfo const&, llvm::RegisterBankInfo const&, llvm::Bitset<0u> const&, llvm::CodeGenCoverage*, llvm::GISelChangeObserver*) const /home/david/work/llvm-community/llvm-project/llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutorImpl.h:917:34
#10 0x00007f7d3ae226eb (anonymous namespace)::AArch64PreLegalizerCombinerImpl::tryCombineAllImpl(llvm::MachineInstr&) const /home/david/work/llvm-community/build/lib/Target/AArch64/AArch64GenPreLegalizeGICombiner.inc:1699:24
#11 0x00007f7d3ae23a13 (anonymous namespace)::AArch64PreLegalizerCombinerImpl::tryCombineAll(llvm::MachineInstr&) const /home/david/work/llvm-community/llvm-project/llvm/lib/Target/AArch64/GISel/AArch64PreLegalizerCombiner.cpp:392:3
#12 0x00007f7d32bdcbd8 llvm::Combiner::combineMachineInstrs() /home/david/work/llvm-community/llvm-project/llvm/lib/CodeGen/GlobalISel/Combiner.cpp:165:15
#13 0x00007f7d3ae23fea (anonymous namespace)::AArch64PreLegalizerCombiner::runOnMachineFunction(llvm::MachineFunction&) /home/david/work/llvm-community/llvm-project/llvm/lib/Target/AArch64/GISel/AArch64PreLegalizerCombiner.cpp:490:35
#14 0x00007f7d38244366 llvm::MachineFunctionPass::runOnFunction(llvm::Function&) /home/david/work/llvm-community/llvm-project/llvm/lib/CodeGen/MachineFunctionPass.cpp:93:33
#15 0x00007f7d342033c7 llvm::FPPassManager::runOnFunction(llvm::Function&) /home/david/work/llvm-community/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1435:20
#16 0x00007f7d34203695 llvm::FPPassManager::runOnModule(llvm::Module&) /home/david/work/llvm-community/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1481:13
#17 0x00007f7d34203af4 (anonymous namespace)::MPPassManager::runOnModule(llvm::Module&) /home/david/work/llvm-community/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1550:20
#18 0x00007f7d341fe98c llvm::legacy::PassManagerImpl::run(llvm::Module&) /home/david/work/llvm-community/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:535:13
#19 0x00007f7d34204391 llvm::legacy::PassManager::run(llvm::Module&) /home/david/work/llvm-community/llvm-project/llvm/lib/IR/LegacyPassManager.cpp:1678:1
#20 0x000055b2213a40ea compileModule(char**, llvm::LLVMContext&) /home/david/work/llvm-community/llvm-project/llvm/tools/llc/llc.cpp:757:66
#21 0x000055b2213a1a59 main /home/david/work/llvm-community/llvm-project/llvm/tools/llc/llc.cpp:416:35
#22 0x00007f7d3310c0b3 __libc_start_main /build/glibc-sMfBJT/glibc-2.31/csu/../csu/libc-start.c:342:3
#23 0x000055b2213a078e _start (/home/david/work/llvm-community/build/bin/llc+0x3278e)

Seems like the issue is that after creating the G_OR the old one will be deleted already, but it is required to create the G_ICMP as well, since the parent info is came from it for example. In BuildMI the *Stete.MIs[0] will be the already deleted G_OR.

    case GIR_BuildMI: {
      uint64_t NewInsnID = MatchTable[CurrentIdx++];
      int64_t Opcode = MatchTable[CurrentIdx++];
      if (NewInsnID >= OutMIs.size())
        OutMIs.resize(NewInsnID + 1);

      OutMIs[NewInsnID] = BuildMI(*State.MIs[0]->getParent(), State.MIs[0],
                                  MIMetadata(*State.MIs[0]), TII.get(Opcode));
      DEBUG_WITH_TYPE(TgtExecutor::getName(),
                      dbgs() << CurrentIdx << ": GIR_BuildMI(OutMIs["
                             << NewInsnID << "], " << Opcode << ")\n");
      break;
    }

At least this is how I understand it so far.

@arsenm
Copy link
Contributor

arsenm commented Jan 18, 2024

Was this crash fixed by 8e8c954?

@dfszabo dfszabo force-pushed the main-global-isel-two-icmp-zero-or-and-combine-new branch from fc3f891 to 6328134 Compare January 18, 2024 18:41
@dfszabo
Copy link
Contributor Author

dfszabo commented Jan 18, 2024

Was this crash fixed by 8e8c954?

Yes! Nice, I rebased the PR and did some minor change to make it work.

@dfszabo dfszabo force-pushed the main-global-isel-two-icmp-zero-or-and-combine-new branch from 6328134 to 1eadb1f Compare January 29, 2024 19:24
Comment on lines 954 to 964
[{ return ${p}.getPredicate() == CmpInst::ICMP_EQ &&
!MRI.getType(${s1}.getReg()).isPointer() &&
(MRI.getType(${s1}.getReg()) ==
MRI.getType(${s2}.getReg())); }]),
Copy link
Contributor

Choose a reason for hiding this comment

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

@Pierre-vh would be nice if we could match the literal G_ICMP predicate inline

Comment on lines +956 to +964
(MRI.getType(${s1}.getReg()) ==
MRI.getType(${s2}.getReg())); }]),
Copy link
Contributor

Choose a reason for hiding this comment

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

You should be able to match this with GITypeOf<"$s1"> instead of using the manual predicate.

@Pierre-vh is there a nicer way to filter out pointers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got this error when using it
error: GISpecialType is not supported in 'match' patterns

Maybe its only usable in the apply part atm.

$x0 = COPY %zext
RET_ReallyLR implicit $x0

...
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add some vector tests? In particular, compares involving vectors of pointers? I believe they are broken as-is

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed they were so added getScalarType as well.

Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

Need some vector and vector of pointer tests

@dfszabo dfszabo force-pushed the main-global-isel-two-icmp-zero-or-and-combine-new branch 4 times, most recently from 0bea4c8 to 94f15e6 Compare February 27, 2024 08:58
Also combine (X != 0) | (Y != 0) -> (X | Y) != 0
@dfszabo dfszabo force-pushed the main-global-isel-two-icmp-zero-or-and-combine-new branch from 94f15e6 to e762ae9 Compare February 27, 2024 11:13
@dfszabo
Copy link
Contributor Author

dfszabo commented Feb 27, 2024

Very recently got merged this #82733, which will insert G_FREEZE before the G_OR (https://godbolt.org/z/dcr83rfM6) causing now the above patterns not to work on cases like in cmp-chains.ll, losing those opt opportunities. I am not sure if G_FREEZE should be incorporated into the pattern or not.

@arsenm
Copy link
Contributor

arsenm commented Feb 27, 2024

Very recently got merged this #82733, which will insert G_FREEZE before the G_OR (https://godbolt.org/z/dcr83rfM6) causing now the above patterns not to work on cases like in cmp-chains.ll, losing those opt opportunities. I am not sure if G_FREEZE should be incorporated into the pattern or not.

You can try the equivalent transform in alive2 on the IR to see if it's valid

@dfszabo
Copy link
Contributor Author

dfszabo commented Feb 28, 2024

Very recently got merged this #82733, which will insert G_FREEZE before the G_OR (https://godbolt.org/z/dcr83rfM6) causing now the above patterns not to work on cases like in cmp-chains.ll, losing those opt opportunities. I am not sure if G_FREEZE should be incorporated into the pattern or not.

You can try the equivalent transform in alive2 on the IR to see if it's valid

Nice tool. Seems like these are valid: https://alive2.llvm.org/ce/z/3AHtx3, https://alive2.llvm.org/ce/z/W8uR9j. With these changes the gains coming back and even improving the regression caused by the G_FREEZE introduction. But I think this might need a separate PR.

diff --git a/llvm/include/llvm/Target/GlobalISel/Combine.td b/llvm/include/llvm/Target/GlobalISel/Combine.td
index 18db7a819540..66d3c14547c2 100644
--- a/llvm/include/llvm/Target/GlobalISel/Combine.td
+++ b/llvm/include/llvm/Target/GlobalISel/Combine.td
@@ -952,6 +952,21 @@ def redundant_binop_in_equality : GICombineRule<
          [{ return Helper.matchRedundantBinOpInEquality(*${root}, ${info}); }]),
   (apply [{ Helper.applyBuildFn(*${root}, ${info}); }])>;
 
+def remove_freeze_of_constants: GICombineRule <
+  (defs root:$dst),
+  (match (G_FREEZE $dst, $src),
+         [{ return !!isConstantOrConstantSplatVector(*MRI.getVRegDef(${src}.getReg()), MRI); }]),
+  (apply (GIReplaceReg $dst, $src))>;
+
+def canonicalize_icmp_freeze: GICombineRule<
+  (defs root:$root),
+  (match (G_ICMP $dst, $p, $src1, $src2),
+         (G_FREEZE $root, $dst)),
+  (apply (G_FREEZE $new_src1, $src1),
+         (G_FREEZE $new_src2, $src2),
+         (G_ICMP $root, $p, $new_src1, $new_src2))
+>;
+
 // Transform: (X == 0 & Y == 0) -> (X | Y) == 0
 def double_icmp_zero_and_combine: GICombineRule<
   (defs root:$root),
@@ -980,6 +995,8 @@ def double_icmp_zero_or_combine: GICombineRule<
          (G_ICMP $root, $p, $ordst, 0))
 >;
 
+def freeze_combines : GICombineGroup<[remove_freeze_of_constants, canonicalize_icmp_freeze]>;
+
 def double_icmp_zero_and_or_combine : GICombineGroup<[double_icmp_zero_and_combine,
                                                       double_icmp_zero_or_combine]>;
 
@@ -1374,7 +1391,7 @@ def all_combines : GICombineGroup<[trivial_combines, insert_vec_elt_combines,
     and_or_disjoint_mask, fma_combines, fold_binop_into_select,
     sub_add_reg, select_to_minmax, redundant_binop_in_equality,
     fsub_to_fneg, commute_constant_to_rhs, match_ands, match_ors, 
-    combine_concat_vector, double_icmp_zero_and_or_combine]>;
+    combine_concat_vector, double_icmp_zero_and_or_combine, freeze_combines]>;
 
 // A combine group used to for prelegalizer combiners at -O0. The combines in
 // this group have been selected based on experiments to balance code size and
diff --git a/llvm/test/CodeGen/AArch64/cmp-chains.ll b/llvm/test/CodeGen/AArch64/cmp-chains.ll
index 1d9f39e51859..4eaa25608d16 100644
--- a/llvm/test/CodeGen/AArch64/cmp-chains.ll
+++ b/llvm/test/CodeGen/AArch64/cmp-chains.ll
@@ -109,8 +109,7 @@ define i32 @cmp_or2(i32 %0, i32 %1, i32 %2, i32 %3) {
 ; GISEL-NEXT:    cset w8, lo
 ; GISEL-NEXT:    cmp w2, w3
 ; GISEL-NEXT:    cset w9, ne
-; GISEL-NEXT:    orr w8, w8, w9
-; GISEL-NEXT:    and w0, w8, #0x1
+; GISEL-NEXT:    orr w0, w8, w9
 ; GISEL-NEXT:    ret
   %5 = icmp ult i32 %0, %1
   %6 = icmp ne i32 %2, %3
@@ -138,8 +137,7 @@ define i32 @cmp_or3(i32 %0, i32 %1, i32 %2, i32 %3, i32 %4, i32 %5) {
 ; GISEL-NEXT:    cmp w4, w5
 ; GISEL-NEXT:    orr w8, w8, w9
 ; GISEL-NEXT:    cset w9, ne
-; GISEL-NEXT:    orr w8, w8, w9
-; GISEL-NEXT:    and w0, w8, #0x1
+; GISEL-NEXT:    orr w0, w8, w9
 ; GISEL-NEXT:    ret
   %7 = icmp ult i32 %0, %1
   %8 = icmp ugt i32 %2, %3
@@ -173,8 +171,7 @@ define i32 @cmp_or4(i32 %0, i32 %1, i32 %2, i32 %3, i32 %4, i32 %5, i32 %6, i32
 ; GISEL-NEXT:    orr w8, w8, w9
 ; GISEL-NEXT:    cset w11, eq
 ; GISEL-NEXT:    orr w9, w10, w11
-; GISEL-NEXT:    orr w8, w8, w9
-; GISEL-NEXT:    and w0, w8, #0x1
+; GISEL-NEXT:    orr w0, w8, w9
 ; GISEL-NEXT:    ret
   %9 = icmp ult i32 %0, %1
   %10 = icmp ugt i32 %2, %3
@@ -189,22 +186,12 @@ define i32 @cmp_or4(i32 %0, i32 %1, i32 %2, i32 %3, i32 %4, i32 %5, i32 %6, i32
 
 ; (x0 != 0) || (x1 != 0)
 define i32 @true_or2(i32 %0, i32 %1) {
-; SDISEL-LABEL: true_or2:
-; SDISEL:       // %bb.0:
-; SDISEL-NEXT:    orr w8, w0, w1
-; SDISEL-NEXT:    cmp w8, #0
-; SDISEL-NEXT:    cset w0, ne
-; SDISEL-NEXT:    ret
-;
-; GISEL-LABEL: true_or2:
-; GISEL:       // %bb.0:
-; GISEL-NEXT:    cmp w0, #0
-; GISEL-NEXT:    cset w8, ne
-; GISEL-NEXT:    cmp w1, #0
-; GISEL-NEXT:    cset w9, ne
-; GISEL-NEXT:    orr w8, w8, w9
-; GISEL-NEXT:    and w0, w8, #0x1
-; GISEL-NEXT:    ret
+; CHECK-LABEL: true_or2:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    orr w8, w0, w1
+; CHECK-NEXT:    cmp w8, #0
+; CHECK-NEXT:    cset w0, ne
+; CHECK-NEXT:    ret
   %3 = icmp ne i32 %0, 0
   %4 = icmp ne i32 %1, 0
   %5 = select i1 %3, i1 true, i1 %4
@@ -214,26 +201,13 @@ define i32 @true_or2(i32 %0, i32 %1) {
 
 ; (x0 != 0) || (x1 != 0) || (x2 != 0)
 define i32 @true_or3(i32 %0, i32 %1, i32 %2) {
-; SDISEL-LABEL: true_or3:
-; SDISEL:       // %bb.0:
-; SDISEL-NEXT:    orr w8, w0, w1
-; SDISEL-NEXT:    orr w8, w8, w2
-; SDISEL-NEXT:    cmp w8, #0
-; SDISEL-NEXT:    cset w0, ne
-; SDISEL-NEXT:    ret
-;
-; GISEL-LABEL: true_or3:
-; GISEL:       // %bb.0:
-; GISEL-NEXT:    cmp w0, #0
-; GISEL-NEXT:    cset w8, ne
-; GISEL-NEXT:    cmp w1, #0
-; GISEL-NEXT:    cset w9, ne
-; GISEL-NEXT:    cmp w2, #0
-; GISEL-NEXT:    orr w8, w8, w9
-; GISEL-NEXT:    cset w9, ne
-; GISEL-NEXT:    orr w8, w8, w9
-; GISEL-NEXT:    and w0, w8, #0x1
-; GISEL-NEXT:    ret
+; CHECK-LABEL: true_or3:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    orr w8, w0, w1
+; CHECK-NEXT:    orr w8, w8, w2
+; CHECK-NEXT:    cmp w8, #0
+; CHECK-NEXT:    cset w0, ne
+; CHECK-NEXT:    ret
   %4 = icmp ne i32 %0, 0
   %5 = icmp ne i32 %1, 0
   %6 = select i1 %4, i1 true, i1 %5
@@ -242,5 +216,3 @@ define i32 @true_or3(i32 %0, i32 %1, i32 %2) {
   %9 = zext i1 %8 to i32
   ret i32 %9
 }

@arsenm
Copy link
Contributor

arsenm commented Feb 29, 2024

Nice tool. Seems like these are valid: https://alive2.llvm.org/ce/z/3AHtx3, https://alive2.llvm.org/ce/z/W8uR9j. With these changes the gains coming back and even improving the regression caused by the G_FREEZE introduction. But I think this might need a separate PR.

Yes, this should be done separately

@arsenm arsenm merged commit 71c06bb into llvm:main Feb 29, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants