public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug middle-end/25186] (short)(((int)short_var) <<1) should be folded so that the shift is done in the short type
[not found] <bug-25186-4@http.gcc.gnu.org/bugzilla/>
@ 2021-09-02 2:03 ` gabravier at gmail dot com
2021-09-02 7:15 ` rguenth at gcc dot gnu.org
2023-04-22 8:06 ` roger at nextmovesoftware dot com
2 siblings, 0 replies; 9+ messages in thread
From: gabravier at gmail dot com @ 2021-09-02 2:03 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25186
Gabriel Ravier <gabravier at gmail dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |gabravier at gmail dot com
--- Comment #8 from Gabriel Ravier <gabravier at gmail dot com> ---
So, is there still an optimization problem on that kind of basic thing ? Or
have some things in GCC have changed since this time to make this irrelevant ?
Looking at these same examples at .optimized even in latest trunk still shows
the same conversions to int... 🤔
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug middle-end/25186] (short)(((int)short_var) <<1) should be folded so that the shift is done in the short type
[not found] <bug-25186-4@http.gcc.gnu.org/bugzilla/>
2021-09-02 2:03 ` [Bug middle-end/25186] (short)(((int)short_var) <<1) should be folded so that the shift is done in the short type gabravier at gmail dot com
@ 2021-09-02 7:15 ` rguenth at gcc dot gnu.org
2023-04-22 8:06 ` roger at nextmovesoftware dot com
2 siblings, 0 replies; 9+ messages in thread
From: rguenth at gcc dot gnu.org @ 2021-09-02 7:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25186
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords| |easyhack
Last reconfirmed|2021-08-21 00:00:00 |2021-9-2
--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
The optimization is still not done, the shift could be unconditionally made a
logical shift but on the smaller type.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug middle-end/25186] (short)(((int)short_var) <<1) should be folded so that the shift is done in the short type
[not found] <bug-25186-4@http.gcc.gnu.org/bugzilla/>
2021-09-02 2:03 ` [Bug middle-end/25186] (short)(((int)short_var) <<1) should be folded so that the shift is done in the short type gabravier at gmail dot com
2021-09-02 7:15 ` rguenth at gcc dot gnu.org
@ 2023-04-22 8:06 ` roger at nextmovesoftware dot com
2 siblings, 0 replies; 9+ messages in thread
From: roger at nextmovesoftware dot com @ 2023-04-22 8:06 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=25186
Roger Sayle <roger at nextmovesoftware dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|--- |13.0
Status|NEW |RESOLVED
Resolution|--- |FIXED
CC| |roger at nextmovesoftware dot com
Known to work| |13.0
--- Comment #10 from Roger Sayle <roger at nextmovesoftware dot com> ---
This is now fixed (at the tree level) on mainline (and was fixed in GCC 13).
For those wondering, LSHIFT_EXPR is well defined (in this case) in GIMPLE,
hence the "optimized" dump currently looks like:
void f ()
{
short int * a.0_1;
short int _2;
short int _3;
a.0_1 = a;
_2 = *a.0_1;
_3 = _2 << 1;
*a.0_1 = _3;
return;
}
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug middle-end/25186] (short)(((int)short_var) <<1) should be folded so that the shift is done in the short type
2005-11-30 18:21 [Bug middle-end/25186] New: (short)(((int)short_var) <<1) should be done in short pinskia at gcc dot gnu dot org
` (4 preceding siblings ...)
2005-12-01 14:17 ` rguenth at gcc dot gnu dot org
@ 2006-04-05 13:49 ` rguenth at gcc dot gnu dot org
5 siblings, 0 replies; 9+ messages in thread
From: rguenth at gcc dot gnu dot org @ 2006-04-05 13:49 UTC (permalink / raw)
To: gcc-bugs
------- Comment #7 from rguenth at gcc dot gnu dot org 2006-04-05 13:49 -------
I'm no longer working on this.
--
rguenth at gcc dot gnu dot org changed:
What |Removed |Added
----------------------------------------------------------------------------
AssignedTo|rguenth at gcc dot gnu dot |unassigned at gcc dot gnu
|org |dot org
Status|ASSIGNED |NEW
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25186
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug middle-end/25186] (short)(((int)short_var) <<1) should be folded so that the shift is done in the short type
2005-11-30 18:21 [Bug middle-end/25186] New: (short)(((int)short_var) <<1) should be done in short pinskia at gcc dot gnu dot org
` (3 preceding siblings ...)
2005-12-01 12:46 ` rguenth at gcc dot gnu dot org
@ 2005-12-01 14:17 ` rguenth at gcc dot gnu dot org
2006-04-05 13:49 ` rguenth at gcc dot gnu dot org
5 siblings, 0 replies; 9+ messages in thread
From: rguenth at gcc dot gnu dot org @ 2005-12-01 14:17 UTC (permalink / raw)
To: gcc-bugs
------- Comment #6 from rguenth at gcc dot gnu dot org 2005-12-01 14:17 -------
I have two fixes. One really safe and one ok via IRC communication.
--
rguenth at gcc dot gnu dot org changed:
What |Removed |Added
----------------------------------------------------------------------------
AssignedTo|unassigned at gcc dot gnu |rguenth at gcc dot gnu dot
|dot org |org
Status|NEW |ASSIGNED
Last reconfirmed|2005-12-01 10:53:06 |2005-12-01 14:17:56
date| |
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25186
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug middle-end/25186] (short)(((int)short_var) <<1) should be folded so that the shift is done in the short type
2005-11-30 18:21 [Bug middle-end/25186] New: (short)(((int)short_var) <<1) should be done in short pinskia at gcc dot gnu dot org
` (2 preceding siblings ...)
2005-12-01 12:28 ` rguenth at gcc dot gnu dot org
@ 2005-12-01 12:46 ` rguenth at gcc dot gnu dot org
2005-12-01 14:17 ` rguenth at gcc dot gnu dot org
2006-04-05 13:49 ` rguenth at gcc dot gnu dot org
5 siblings, 0 replies; 9+ messages in thread
From: rguenth at gcc dot gnu dot org @ 2005-12-01 12:46 UTC (permalink / raw)
To: gcc-bugs
------- Comment #5 from rguenth at gcc dot gnu dot org 2005-12-01 12:46 -------
I.e. 6.3.1.3/3 says that conversion from type T to signed type is
implementation defined if the value doesn't fit. And we define it to reducing
it modulo 2^N.
So, for !flag_wrapv
(short)((int)0x8000 << 1) == 0
but
0x8000 << 1 is undefined.
So this transformation is only ok for flag_wrapv.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25186
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug middle-end/25186] (short)(((int)short_var) <<1) should be folded so that the shift is done in the short type
2005-11-30 18:21 [Bug middle-end/25186] New: (short)(((int)short_var) <<1) should be done in short pinskia at gcc dot gnu dot org
2005-12-01 10:53 ` [Bug middle-end/25186] (short)(((int)short_var) <<1) should be folded so that the shift is done in the short type rguenth at gcc dot gnu dot org
2005-12-01 11:01 ` rguenth at gcc dot gnu dot org
@ 2005-12-01 12:28 ` rguenth at gcc dot gnu dot org
2005-12-01 12:46 ` rguenth at gcc dot gnu dot org
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: rguenth at gcc dot gnu dot org @ 2005-12-01 12:28 UTC (permalink / raw)
To: gcc-bugs
------- Comment #4 from rguenth at gcc dot gnu dot org 2005-12-01 12:28 -------
convert_to_integer contains
case LSHIFT_EXPR:
/* We can pass truncation down through left shifting
when the shift count is a nonnegative constant and
the target type is unsigned. */
if (TREE_CODE (TREE_OPERAND (expr, 1)) == INTEGER_CST
&& tree_int_cst_sgn (TREE_OPERAND (expr, 1)) >= 0
&& TYPE_UNSIGNED (type)
&& TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST)
which for our case then (should) falls through to
...
/* Don't do unsigned arithmetic where signed was wanted,
or vice versa.
Exception: if both of the original operands were
unsigned then we can safely do the work as unsigned.
Exception: shift operations take their type solely
from the first argument.
Exception: the LSHIFT_EXPR case above requires that
we perform this operation unsigned lest we produce
signed-overflow undefinedness.
And we may need to do it as unsigned
if we truncate to the original size. */
if (TYPE_UNSIGNED (TREE_TYPE (expr))
|| (TYPE_UNSIGNED (TREE_TYPE (arg0))
&& (TYPE_UNSIGNED (TREE_TYPE (arg1))
|| ex_form == LSHIFT_EXPR
|| ex_form == RSHIFT_EXPR
|| ex_form == LROTATE_EXPR
|| ex_form == RROTATE_EXPR))
|| ex_form == LSHIFT_EXPR)
typex = lang_hooks.types.unsigned_type (typex);
else
typex = lang_hooks.types.signed_type (typex);
now, this path seems to handle LSHIFT_EXPR of signed types, so the exeption
above does not need to apply. Further, I don't understand the reasoning
why we need to do the shift unsigned - we invoke undefined behavior for
signed overflow, but the original code, (short) int << n invoked undefined
behavior in truncating the int to short in case the value doesn't fit. Which
of course still happens for the unsigned case.
So, what would break with
Index: convert.c
===================================================================
*** convert.c (revision 107813)
--- convert.c (working copy)
*************** convert_to_integer (tree type, tree expr
*** 512,518 ****
the target type is unsigned. */
if (TREE_CODE (TREE_OPERAND (expr, 1)) == INTEGER_CST
&& tree_int_cst_sgn (TREE_OPERAND (expr, 1)) >= 0
- && TYPE_UNSIGNED (type)
&& TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST)
{
/* If shift count is less than the width of the truncated type,
--- 490,495 ----
*************** convert_to_integer (tree type, tree expr
*** 607,614 ****
|| ex_form == LSHIFT_EXPR
|| ex_form == RSHIFT_EXPR
|| ex_form == LROTATE_EXPR
! || ex_form == RROTATE_EXPR))
! || ex_form == LSHIFT_EXPR)
typex = lang_hooks.types.unsigned_type (typex);
else
typex = lang_hooks.types.signed_type (typex);
--- 584,590 ----
|| ex_form == LSHIFT_EXPR
|| ex_form == RSHIFT_EXPR
|| ex_form == LROTATE_EXPR
! || ex_form == RROTATE_EXPR)))
typex = lang_hooks.types.unsigned_type (typex);
else
typex = lang_hooks.types.signed_type (typex);
?
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25186
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug middle-end/25186] (short)(((int)short_var) <<1) should be folded so that the shift is done in the short type
2005-11-30 18:21 [Bug middle-end/25186] New: (short)(((int)short_var) <<1) should be done in short pinskia at gcc dot gnu dot org
2005-12-01 10:53 ` [Bug middle-end/25186] (short)(((int)short_var) <<1) should be folded so that the shift is done in the short type rguenth at gcc dot gnu dot org
@ 2005-12-01 11:01 ` rguenth at gcc dot gnu dot org
2005-12-01 12:28 ` rguenth at gcc dot gnu dot org
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: rguenth at gcc dot gnu dot org @ 2005-12-01 11:01 UTC (permalink / raw)
To: gcc-bugs
------- Comment #3 from rguenth at gcc dot gnu dot org 2005-12-01 11:01 -------
Doh. The C frontend _does_ the promotion (in the unsigned case):
(intD.0) *aD.1296 << 1
just convert.c:convert_to_integer "folds" it to a shift on unsigned short
again.
This transformation should be moved to fold instead.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25186
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug middle-end/25186] (short)(((int)short_var) <<1) should be folded so that the shift is done in the short type
2005-11-30 18:21 [Bug middle-end/25186] New: (short)(((int)short_var) <<1) should be done in short pinskia at gcc dot gnu dot org
@ 2005-12-01 10:53 ` rguenth at gcc dot gnu dot org
2005-12-01 11:01 ` rguenth at gcc dot gnu dot org
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: rguenth at gcc dot gnu dot org @ 2005-12-01 10:53 UTC (permalink / raw)
To: gcc-bugs
------- Comment #2 from rguenth at gcc dot gnu dot org 2005-12-01 10:53 -------
Confirmed. The first testcase is really just
short *a;
void f(void)
{
*a = *a << 1;
}
interestingly, the C frontend does not do integer promotion of
unsigned short *a;
void f(voif)
{
*a = *a << 1;
}
where *a should be promoted to int as of 6.3.1.8 and 6.5.7/3, which
says "Integer promotions are performed on each of the operands". Now
the question is how to read this, but either the C frontend does
unnecessary promution for the signed case or it misses it for the unsigned
case.
--
rguenth at gcc dot gnu dot org changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Ever Confirmed|0 |1
Last reconfirmed|0000-00-00 00:00:00 |2005-12-01 10:53:06
date| |
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25186
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-04-22 8:06 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <bug-25186-4@http.gcc.gnu.org/bugzilla/>
2021-09-02 2:03 ` [Bug middle-end/25186] (short)(((int)short_var) <<1) should be folded so that the shift is done in the short type gabravier at gmail dot com
2021-09-02 7:15 ` rguenth at gcc dot gnu.org
2023-04-22 8:06 ` roger at nextmovesoftware dot com
2005-11-30 18:21 [Bug middle-end/25186] New: (short)(((int)short_var) <<1) should be done in short pinskia at gcc dot gnu dot org
2005-12-01 10:53 ` [Bug middle-end/25186] (short)(((int)short_var) <<1) should be folded so that the shift is done in the short type rguenth at gcc dot gnu dot org
2005-12-01 11:01 ` rguenth at gcc dot gnu dot org
2005-12-01 12:28 ` rguenth at gcc dot gnu dot org
2005-12-01 12:46 ` rguenth at gcc dot gnu dot org
2005-12-01 14:17 ` rguenth at gcc dot gnu dot org
2006-04-05 13:49 ` rguenth at gcc dot gnu dot org
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).