From: Tamar Christina <tamar.christina@arm.com>
To: gcc-patches@gcc.gnu.org
Cc: nd@arm.com, rguenther@suse.de, jeffreyalaw@gmail.com
Subject: [PATCH]middle-end fix floating out of constants in conditionals
Date: Fri, 23 Sep 2022 10:21:20 +0100 [thread overview]
Message-ID: <patch-15680-tamar@arm.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 4268 bytes --]
Hi All,
The following testcase:
int zoo1 (int a, int b, int c, int d)
{
return (a > b ? c : d) & 1;
}
gets de-optimized by the front-end since somewhere around GCC 4.x due to a fix
that was added to fold_binary_op_with_conditional_arg.
The folding is supposed to succeed only if we have folded at least one of the
branches, however the check doesn't tests that all of the values are
non-constant. So if one of the operators are a constant it accepts the folding.
This ends up folding
return (a > b ? c : d) & 1;
into
return (a > b ? c & 1 : d & 1);
and thus performing the AND twice.
change changes it to reject the folding if one of the arguments are a constant
and if the operations being performed are the same.
Secondly it adds a new match.pd rule to now also fold the opposite direction, so
it now also folds:
return (a > b ? c & 1 : d & 1);
into
return (a > b ? c : d) & 1;
Bootstrapped Regtested on aarch64-none-linux-gnu, x86_64-pc-linux-gnu
and <on-goin> issues.
Ok for master?
Thanks,
Tamar
gcc/ChangeLog:
* fold-const.cc (fold_binary_op_with_conditional_arg): Add relaxation.
* match.pd: Add ternary constant fold rule.
* tree-cfg.cc (verify_gimple_assign_ternary): RHS1 of a COND_EXPR isn't
a value but an expression itself.
gcc/testsuite/ChangeLog:
* gcc.target/aarch64/if-compare_3.c: New test.
--- inline copy of patch --
diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
index 4f4ec81c8d4b6937ade3141a14c695b67c874c35..0ee083f290d12104969f1b335dc33917c97b4808 100644
--- a/gcc/fold-const.cc
+++ b/gcc/fold-const.cc
@@ -7212,7 +7212,9 @@ fold_binary_op_with_conditional_arg (location_t loc,
}
/* Check that we have simplified at least one of the branches. */
- if (!TREE_CONSTANT (arg) && !TREE_CONSTANT (lhs) && !TREE_CONSTANT (rhs))
+ if ((!TREE_CONSTANT (arg) && !TREE_CONSTANT (lhs) && !TREE_CONSTANT (rhs))
+ || (TREE_CONSTANT (arg) && TREE_CODE (lhs) == TREE_CODE (rhs)
+ && !TREE_CONSTANT (lhs)))
return NULL_TREE;
return fold_build3_loc (loc, cond_code, type, test, lhs, rhs);
diff --git a/gcc/match.pd b/gcc/match.pd
index b225d36dc758f1581502c8d03761544bfd499c01..b61ed70e69b881a49177f10f20c1f92712bb8665 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4318,6 +4318,16 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(op @3 (vec_cond:s @0 @1 @2))
(vec_cond @0 (op! @3 @1) (op! @3 @2))))
+/* Float out binary operations from branches if they can't be folded.
+ Fold (a ? (b op c) : (d op c)) --> (op (a ? b : d) c). */
+(for op (plus mult min max bit_and bit_ior bit_xor minus lshift rshift rdiv
+ trunc_div ceil_div floor_div round_div trunc_mod ceil_mod floor_mod
+ round_mod)
+ (simplify
+ (cond @0 (op @1 @2) (op @3 @2))
+ (if (!FLOAT_TYPE_P (type) || !(HONOR_NANS (@1) && flag_trapping_math))
+ (op (cond @0 @1 @3) @2))))
+
#if GIMPLE
(match (nop_atomic_bit_test_and_p @0 @1 @4)
(bit_and (convert?@4 (ATOMIC_FETCH_OR_XOR_N @2 INTEGER_CST@0 @3))
diff --git a/gcc/testsuite/gcc.target/aarch64/if-compare_3.c b/gcc/testsuite/gcc.target/aarch64/if-compare_3.c
new file mode 100644
index 0000000000000000000000000000000000000000..1d97da5c0d6454175881c219927471a567a6f0c7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/if-compare_3.c
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-O3 -std=c99" } */
+/* { dg-final { check-function-bodies "**" "" "" { target { le } } } } */
+
+/*
+**zoo:
+** cmp w0, w1
+** csel w0, w3, w2, le
+** ret
+*/
+int zoo (int a, int b, int c, int d)
+{
+ return a > b ? c : d;
+}
+
+/*
+**zoo1:
+** cmp w0, w1
+** csel w0, w3, w2, le
+** and w0, w0, 1
+** ret
+*/
+int zoo1 (int a, int b, int c, int d)
+{
+ return (a > b ? c : d) & 1;
+}
+
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index b19710392940cf469de52d006603ae1e3deb6b76..aaf1b29da5c598add25dad2c38b828eaa89c49ce 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -4244,7 +4244,9 @@ verify_gimple_assign_ternary (gassign *stmt)
return true;
}
- if (!is_gimple_val (rhs1)
+ /* In a COND_EXPR the rhs1 is the condition and thus isn't
+ a gimple value by definition. */
+ if ((!is_gimple_val (rhs1) && rhs_code != COND_EXPR)
|| !is_gimple_val (rhs2)
|| !is_gimple_val (rhs3))
{
--
[-- Attachment #2: rb15680.patch --]
[-- Type: text/plain, Size: 2881 bytes --]
diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
index 4f4ec81c8d4b6937ade3141a14c695b67c874c35..0ee083f290d12104969f1b335dc33917c97b4808 100644
--- a/gcc/fold-const.cc
+++ b/gcc/fold-const.cc
@@ -7212,7 +7212,9 @@ fold_binary_op_with_conditional_arg (location_t loc,
}
/* Check that we have simplified at least one of the branches. */
- if (!TREE_CONSTANT (arg) && !TREE_CONSTANT (lhs) && !TREE_CONSTANT (rhs))
+ if ((!TREE_CONSTANT (arg) && !TREE_CONSTANT (lhs) && !TREE_CONSTANT (rhs))
+ || (TREE_CONSTANT (arg) && TREE_CODE (lhs) == TREE_CODE (rhs)
+ && !TREE_CONSTANT (lhs)))
return NULL_TREE;
return fold_build3_loc (loc, cond_code, type, test, lhs, rhs);
diff --git a/gcc/match.pd b/gcc/match.pd
index b225d36dc758f1581502c8d03761544bfd499c01..b61ed70e69b881a49177f10f20c1f92712bb8665 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -4318,6 +4318,16 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(op @3 (vec_cond:s @0 @1 @2))
(vec_cond @0 (op! @3 @1) (op! @3 @2))))
+/* Float out binary operations from branches if they can't be folded.
+ Fold (a ? (b op c) : (d op c)) --> (op (a ? b : d) c). */
+(for op (plus mult min max bit_and bit_ior bit_xor minus lshift rshift rdiv
+ trunc_div ceil_div floor_div round_div trunc_mod ceil_mod floor_mod
+ round_mod)
+ (simplify
+ (cond @0 (op @1 @2) (op @3 @2))
+ (if (!FLOAT_TYPE_P (type) || !(HONOR_NANS (@1) && flag_trapping_math))
+ (op (cond @0 @1 @3) @2))))
+
#if GIMPLE
(match (nop_atomic_bit_test_and_p @0 @1 @4)
(bit_and (convert?@4 (ATOMIC_FETCH_OR_XOR_N @2 INTEGER_CST@0 @3))
diff --git a/gcc/testsuite/gcc.target/aarch64/if-compare_3.c b/gcc/testsuite/gcc.target/aarch64/if-compare_3.c
new file mode 100644
index 0000000000000000000000000000000000000000..1d97da5c0d6454175881c219927471a567a6f0c7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/if-compare_3.c
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-O3 -std=c99" } */
+/* { dg-final { check-function-bodies "**" "" "" { target { le } } } } */
+
+/*
+**zoo:
+** cmp w0, w1
+** csel w0, w3, w2, le
+** ret
+*/
+int zoo (int a, int b, int c, int d)
+{
+ return a > b ? c : d;
+}
+
+/*
+**zoo1:
+** cmp w0, w1
+** csel w0, w3, w2, le
+** and w0, w0, 1
+** ret
+*/
+int zoo1 (int a, int b, int c, int d)
+{
+ return (a > b ? c : d) & 1;
+}
+
diff --git a/gcc/tree-cfg.cc b/gcc/tree-cfg.cc
index b19710392940cf469de52d006603ae1e3deb6b76..aaf1b29da5c598add25dad2c38b828eaa89c49ce 100644
--- a/gcc/tree-cfg.cc
+++ b/gcc/tree-cfg.cc
@@ -4244,7 +4244,9 @@ verify_gimple_assign_ternary (gassign *stmt)
return true;
}
- if (!is_gimple_val (rhs1)
+ /* In a COND_EXPR the rhs1 is the condition and thus isn't
+ a gimple value by definition. */
+ if ((!is_gimple_val (rhs1) && rhs_code != COND_EXPR)
|| !is_gimple_val (rhs2)
|| !is_gimple_val (rhs3))
{
next reply other threads:[~2022-09-23 9:21 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-23 9:21 Tamar Christina [this message]
2022-09-24 20:44 ` Jeff Law
2022-09-26 10:28 ` Richard Biener
2022-09-26 11:08 ` Eric Botcazou
2022-09-26 11:12 ` Eric Botcazou
2022-10-17 9:17 ` Tamar Christina
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=patch-15680-tamar@arm.com \
--to=tamar.christina@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jeffreyalaw@gmail.com \
--cc=nd@arm.com \
--cc=rguenther@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).