* [Bug tree-optimization/101617] a ? -1 : 1 -> (-(type)a) | 1
2021-07-25 5:47 [Bug tree-optimization/101617] New: a ? -1 : 1 -> (-(type)a) | 1 pinskia at gcc dot gnu.org
@ 2021-07-25 5:47 ` pinskia at gcc dot gnu.org
2021-07-25 19:25 ` pinskia at gcc dot gnu.org
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-07-25 5:47 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101617
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Last reconfirmed| |2021-07-25
Status|UNCONFIRMED |ASSIGNED
Ever confirmed|0 |1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug tree-optimization/101617] a ? -1 : 1 -> (-(type)a) | 1
2021-07-25 5:47 [Bug tree-optimization/101617] New: a ? -1 : 1 -> (-(type)a) | 1 pinskia at gcc dot gnu.org
2021-07-25 5:47 ` [Bug tree-optimization/101617] " pinskia at gcc dot gnu.org
@ 2021-07-25 19:25 ` pinskia at gcc dot gnu.org
2021-07-25 20:47 ` [Bug rtl-optimization/101617] " pinskia at gcc dot gnu.org
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-07-25 19:25 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101617
--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
So it turns out you can make this generic and don't need to handle 1 specially
diff --git a/gcc/match.pd b/gcc/match.pd
index beb8d27535e..2af987278af 100644
--- a/gcc/match.pd
+++ b/gcc/match.pd
@@ -3805,14 +3805,23 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
(simplify
(cond @0 INTEGER_CST@1 INTEGER_CST@2)
(switch
+ /* a ? CST : -1 -> -(!a) | CST. */
+ (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@2))
+ (with {
+ tree booltrue = constant_boolean_node (true, boolean_type_node);
+ }
+ (bit_ior (negate (convert (bit_xor (convert:boolean_type_node @0) {
booltrue; } ))) @2)))
+ /* a ? -1 : CST -> -(a) | CST. */
+ (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@1))
+ (with {
+ tree booltrue = constant_boolean_node (true, boolean_type_node);
+ }
+ (bit_ior (negate (convert (convert:boolean_type_node @0))) @2)))
(if (integer_zerop (@2))
(switch
/* a ? 1 : 0 -> a if 0 and 1 are integral types. */
(if (integer_onep (@1))
(convert (convert:boolean_type_node @0)))
- /* a ? -1 : 0 -> -a. */
- (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@1))
- (negate (convert (convert:boolean_type_node @0))))
/* a ? powerof2cst : 0 -> a << (log2(powerof2cst)) */
(if (INTEGRAL_TYPE_P (type) && integer_pow2p (@1))
(with {
@@ -3827,9 +3836,6 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
/* a ? 0 : 1 -> !a. */
(if (integer_onep (@2))
(convert (bit_xor (convert:boolean_type_node @0) { booltrue; } )))
- /* a ? -1 : 0 -> -(!a). */
- (if (INTEGRAL_TYPE_P (type) && integer_all_onesp (@2))
- (negate (convert (bit_xor (convert:boolean_type_node @0) { booltrue; }
))))
/* a ? powerof2cst : 0 -> (!a) << (log2(powerof2cst)) */
(if (INTEGRAL_TYPE_P (type) && integer_pow2p (@2))
(with {
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug rtl-optimization/101617] a ? -1 : 1 -> (-(type)a) | 1
2021-07-25 5:47 [Bug tree-optimization/101617] New: a ? -1 : 1 -> (-(type)a) | 1 pinskia at gcc dot gnu.org
2021-07-25 5:47 ` [Bug tree-optimization/101617] " pinskia at gcc dot gnu.org
2021-07-25 19:25 ` pinskia at gcc dot gnu.org
@ 2021-07-25 20:47 ` pinskia at gcc dot gnu.org
2021-07-25 22:03 ` pinskia at gcc dot gnu.org
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-07-25 20:47 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101617
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Component|tree-optimization |rtl-optimization
--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I decided that this should really go on the RTL level ....
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug rtl-optimization/101617] a ? -1 : 1 -> (-(type)a) | 1
2021-07-25 5:47 [Bug tree-optimization/101617] New: a ? -1 : 1 -> (-(type)a) | 1 pinskia at gcc dot gnu.org
` (2 preceding siblings ...)
2021-07-25 20:47 ` [Bug rtl-optimization/101617] " pinskia at gcc dot gnu.org
@ 2021-07-25 22:03 ` pinskia at gcc dot gnu.org
2021-07-25 22:04 ` pinskia at gcc dot gnu.org
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-07-25 22:03 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101617
--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I have the ifcvt.c patch which adds this.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug rtl-optimization/101617] a ? -1 : 1 -> (-(type)a) | 1
2021-07-25 5:47 [Bug tree-optimization/101617] New: a ? -1 : 1 -> (-(type)a) | 1 pinskia at gcc dot gnu.org
` (3 preceding siblings ...)
2021-07-25 22:03 ` pinskia at gcc dot gnu.org
@ 2021-07-25 22:04 ` pinskia at gcc dot gnu.org
2021-07-25 22:35 ` pinskia at gcc dot gnu.org
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-07-25 22:04 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101617
--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 51203
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51203&action=edit
ifcvt patch
Patch which go into testing.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug rtl-optimization/101617] a ? -1 : 1 -> (-(type)a) | 1
2021-07-25 5:47 [Bug tree-optimization/101617] New: a ? -1 : 1 -> (-(type)a) | 1 pinskia at gcc dot gnu.org
` (4 preceding siblings ...)
2021-07-25 22:04 ` pinskia at gcc dot gnu.org
@ 2021-07-25 22:35 ` pinskia at gcc dot gnu.org
2021-07-26 0:21 ` pinskia at gcc dot gnu.org
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-07-25 22:35 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101617
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #51203|0 |1
is obsolete| |
--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Comment on attachment 51203
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51203
ifcvt patch
This patch is wrong if STORE_FLAG_VALUE == -1.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug rtl-optimization/101617] a ? -1 : 1 -> (-(type)a) | 1
2021-07-25 5:47 [Bug tree-optimization/101617] New: a ? -1 : 1 -> (-(type)a) | 1 pinskia at gcc dot gnu.org
` (5 preceding siblings ...)
2021-07-25 22:35 ` pinskia at gcc dot gnu.org
@ 2021-07-26 0:21 ` pinskia at gcc dot gnu.org
2021-07-26 0:28 ` pinskia at gcc dot gnu.org
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-07-26 0:21 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101617
--- Comment #6 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Thinking about this some more, there is a canonicalization issue. We need to
decide if we want to canonicalization to just a ? -1 : 1; or expand it out.
a ? 1 : 0 makes sense to do (cast) a; So does "a ? 0 : 1".
Does the current a ? -1 : 0 make sense or just add that to ifcvt.
I am going to take a few days to think of this and such.
There are other issues that deal with this. Even having a cmov existing makes
it harder to decide. Even though for an example -(a == 0) can be optimized
nicely on x86, it might not be nicely on other targets.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug rtl-optimization/101617] a ? -1 : 1 -> (-(type)a) | 1
2021-07-25 5:47 [Bug tree-optimization/101617] New: a ? -1 : 1 -> (-(type)a) | 1 pinskia at gcc dot gnu.org
` (6 preceding siblings ...)
2021-07-26 0:21 ` pinskia at gcc dot gnu.org
@ 2021-07-26 0:28 ` pinskia at gcc dot gnu.org
2021-07-27 10:52 ` rguenth at gcc dot gnu.org
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-07-26 0:28 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101617
--- Comment #7 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
A few more canonicalization issues that need to be thought of:
"a >>u (bitsize-1)" and "a <s 0"
"a >>s (bitsize-1)" and "-(a <s 0)"
(In reply to Andrew Pinski from comment #6)
> Thinking about this some more, there is a canonicalization issue. We need to
> decide if we want to canonicalization to just a ? -1 : 1; or expand it out.
> a ? 1 : 0 makes sense to do (cast) a; So does "a ? 0 : 1".
>
> Does the current a ? -1 : 0 make sense or just add that to ifcvt.
PR101339 is related to that canonicalization really.
There are others.
Even things like:
(a == 0) + 2
Should that be:
a == 0 ? 3 : 2
On the gimple level
and then do the correct thing on the RTL level?
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug rtl-optimization/101617] a ? -1 : 1 -> (-(type)a) | 1
2021-07-25 5:47 [Bug tree-optimization/101617] New: a ? -1 : 1 -> (-(type)a) | 1 pinskia at gcc dot gnu.org
` (7 preceding siblings ...)
2021-07-26 0:28 ` pinskia at gcc dot gnu.org
@ 2021-07-27 10:52 ` rguenth at gcc dot gnu.org
2022-05-30 20:28 ` cvs-commit at gcc dot gnu.org
2022-06-04 9:14 ` roger at nextmovesoftware dot com
10 siblings, 0 replies; 12+ messages in thread
From: rguenth at gcc dot gnu.org @ 2021-07-27 10:52 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101617
--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
We generally want less stmts on GIMPLE, (-(type)a) | 1 is more than
a ? -1 : 1 which means it should be a RTL expansion time optimization.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug rtl-optimization/101617] a ? -1 : 1 -> (-(type)a) | 1
2021-07-25 5:47 [Bug tree-optimization/101617] New: a ? -1 : 1 -> (-(type)a) | 1 pinskia at gcc dot gnu.org
` (8 preceding siblings ...)
2021-07-27 10:52 ` rguenth at gcc dot gnu.org
@ 2022-05-30 20:28 ` cvs-commit at gcc dot gnu.org
2022-06-04 9:14 ` roger at nextmovesoftware dot com
10 siblings, 0 replies; 12+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-05-30 20:28 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101617
--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Roger Sayle <sayle@gcc.gnu.org>:
https://gcc.gnu.org/g:f1652e3343b1ec47035370801d9b9aca1f8b613f
commit r13-857-gf1652e3343b1ec47035370801d9b9aca1f8b613f
Author: Roger Sayle <roger@nextmovesoftware.com>
Date: Mon May 30 21:26:37 2022 +0100
PR rtl-optimization/101617: Use neg/sbb in ix86_expand_int_movcc.
This patch resolves PR rtl-optimization/101617 where we should generate
the exact same code for (X ? -1 : 1) as we do for ((X ? -1 : 0) | 1).
The cause of the current difference on x86_64 is actually in
ix86_expand_int_movcc that doesn't know that negl;sbbl can be used
to create a -1/0 result depending on whether the input is zero/nonzero.
So for Andrew Pinski's test case:
int f1(int i)
{
return i ? -1 : 1;
}
GCC currently generates:
f1: cmpl $1, %edi
sbbl %eax, %eax // x ? 0 : -1
andl $2, %eax // x ? 0 : 2
subl $1, %eax // x ? -1 : 1
ret
but with the attached patch, now generates:
f1: negl %edi
sbbl %eax, %eax // x ? -1 : 0
orl $1, %eax // x ? -1 : 1
ret
To implement this I needed to add two expanders to i386.md to generate
the required instructions (in both SImode and DImode) matching the
pre-existing define_insns of the same name.
2022-05-30 Roger Sayle <roger@nextmovesoftware.com>
gcc/ChangeLog
PR rtl-optimization/101617
* config/i386/i386-expand.cc (ix86_expand_int_movcc): Add a
special case (indicated by negate_cc_compare_p) to generate a
-1/0 mask using neg;sbb.
* config/i386/i386.md (x86_neg<mode>_ccc): New define_expand
to generate an *x86_neg<mode>_ccc instruction.
(x86_mov<mode>cc_0_m1_neg): Likewise, a new define_expand to
generate a *x86_mov<mode>cc_0_m1_neg instruction.
gcc/testsuite/ChangeLog
PR rtl-optimization/101617
* gcc.target/i386/pr101617.c: New test case.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Bug rtl-optimization/101617] a ? -1 : 1 -> (-(type)a) | 1
2021-07-25 5:47 [Bug tree-optimization/101617] New: a ? -1 : 1 -> (-(type)a) | 1 pinskia at gcc dot gnu.org
` (9 preceding siblings ...)
2022-05-30 20:28 ` cvs-commit at gcc dot gnu.org
@ 2022-06-04 9:14 ` roger at nextmovesoftware dot com
10 siblings, 0 replies; 12+ messages in thread
From: roger at nextmovesoftware dot com @ 2022-06-04 9:14 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101617
Roger Sayle <roger at nextmovesoftware dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Target Milestone|--- |13.0
CC| |roger at nextmovesoftware dot com
Resolution|--- |FIXED
--- Comment #10 from Roger Sayle <roger at nextmovesoftware dot com> ---
This should now be fixed (for x86) on mainline.
^ permalink raw reply [flat|nested] 12+ messages in thread