* [patch] Do not generate useless integral conversions
@ 2014-06-24 10:55 Eric Botcazou
2014-06-24 14:32 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Eric Botcazou @ 2014-06-24 10:55 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 711 bytes --]
Hi,
https://gcc.gnu.org/ml/gcc-patches/2012-03/msg00491.html changed the old
signed_type_for/unsigned_type_for functions and made them always return an
integer type, whereas they would previously leave integral types unchanged.
I don't see any justification for the latter and this has the annoying effect
of generating useless integral conversions in convert.c, for example between
boolean types and integer types of the same precision.
The attached patch restores the old behavior for them. Bootstrapped/regtested
on x86_64-suse-linux, OK for the mainline?
2014-06-24 Eric Botcazou <ebotcazou@adacore.com>
* tree.c (signed_or_unsigned_type_for): Treat integral types equally.
--
Eric Botcazou
[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 1728 bytes --]
Index: tree.c
===================================================================
--- tree.c (revision 211927)
+++ tree.c (working copy)
@@ -10684,14 +10684,14 @@ int_cst_value (const_tree x)
return val;
}
-/* If TYPE is an integral or pointer type, return an integer type with
+/* If TYPE is an integral or pointer type, return an integral type with
the same precision which is unsigned iff UNSIGNEDP is true, or itself
- if TYPE is already an integer type of signedness UNSIGNEDP. */
+ if TYPE is already an integral type of signedness UNSIGNEDP. */
tree
signed_or_unsigned_type_for (int unsignedp, tree type)
{
- if (TREE_CODE (type) == INTEGER_TYPE && TYPE_UNSIGNED (type) == unsignedp)
+ if (INTEGRAL_TYPE_P (type) && TYPE_UNSIGNED (type) == unsignedp)
return type;
if (TREE_CODE (type) == VECTOR_TYPE)
@@ -10713,9 +10713,9 @@ signed_or_unsigned_type_for (int unsigne
return build_nonstandard_integer_type (TYPE_PRECISION (type), unsignedp);
}
-/* If TYPE is an integral or pointer type, return an integer type with
+/* If TYPE is an integral or pointer type, return an integral type with
the same precision which is unsigned, or itself if TYPE is already an
- unsigned integer type. */
+ unsigned integral type. */
tree
unsigned_type_for (tree type)
@@ -10723,9 +10723,9 @@ unsigned_type_for (tree type)
return signed_or_unsigned_type_for (1, type);
}
-/* If TYPE is an integral or pointer type, return an integer type with
+/* If TYPE is an integral or pointer type, return an integral type with
the same precision which is signed, or itself if TYPE is already a
- signed integer type. */
+ signed integral type. */
tree
signed_type_for (tree type)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [patch] Do not generate useless integral conversions
2014-06-24 10:55 [patch] Do not generate useless integral conversions Eric Botcazou
@ 2014-06-24 14:32 ` Richard Biener
2014-06-24 21:18 ` Eric Botcazou
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2014-06-24 14:32 UTC (permalink / raw)
To: Eric Botcazou; +Cc: GCC Patches
On Tue, Jun 24, 2014 at 12:54 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> https://gcc.gnu.org/ml/gcc-patches/2012-03/msg00491.html changed the old
> signed_type_for/unsigned_type_for functions and made them always return an
> integer type, whereas they would previously leave integral types unchanged.
> I don't see any justification for the latter and this has the annoying effect
> of generating useless integral conversions in convert.c, for example between
> boolean types and integer types of the same precision.
>
> The attached patch restores the old behavior for them. Bootstrapped/regtested
> on x86_64-suse-linux, OK for the mainline?
I think that was on purpose to avoid arithmetics in enum types. As those
conversions are useless and thus stripped later is it really important
to retain enum and boolean kind here?
Richard.
> 2014-06-24 Eric Botcazou <ebotcazou@adacore.com>
>
> * tree.c (signed_or_unsigned_type_for): Treat integral types equally.
>
>
> --
> Eric Botcazou
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] Do not generate useless integral conversions
2014-06-24 14:32 ` Richard Biener
@ 2014-06-24 21:18 ` Eric Botcazou
2014-06-25 8:21 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Eric Botcazou @ 2014-06-24 21:18 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
> I think that was on purpose to avoid arithmetics in enum types. As those
> conversions are useless and thus stripped later is it really important
> to retain enum and boolean kind here?
The problem is that convert.c is called by front-ends and the patch also
removed the callback into them that made it possible to have some control.
So, yes, it's pretty annoying to see totally bogus conversion nodes being
introduced into your ASTs behind your back...
--
Eric Botcazou
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] Do not generate useless integral conversions
2014-06-24 21:18 ` Eric Botcazou
@ 2014-06-25 8:21 ` Richard Biener
2014-10-02 10:42 ` Eric Botcazou
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2014-06-25 8:21 UTC (permalink / raw)
To: Eric Botcazou; +Cc: GCC Patches
On Tue, Jun 24, 2014 at 11:16 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> I think that was on purpose to avoid arithmetics in enum types. As those
>> conversions are useless and thus stripped later is it really important
>> to retain enum and boolean kind here?
>
> The problem is that convert.c is called by front-ends and the patch also
> removed the callback into them that made it possible to have some control.
> So, yes, it's pretty annoying to see totally bogus conversion nodes being
> introduced into your ASTs behind your back...
Ok, but the (previous and proposed) behavior is odd as for
unsigned_type_for (boolean_type_node) you'd get back
a BOOLEAN_TYPE but for unsigned_type_for (signed_type_for
(boolean_type_node)) you get back an INTEGER_TYPE.
That is, because we use build_nonstandard_integer_type now
the result will always be an INTEGER_TYPE (unless you happen
to pass in a type with the correct signedness with your patch).
I don't like re-introducing that inconsistency.
Maybe instead make convert.c do if (!TYPE_UNSIGNED) unsigned_type_for ()
instead? I notice that all callers of [un]signed_type_for are in
"premature" optimizations convert.c performs (that better should be done
in fold-const.c).
Thanks,
Richard.
> --
> Eric Botcazou
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] Do not generate useless integral conversions
2014-06-25 8:21 ` Richard Biener
@ 2014-10-02 10:42 ` Eric Botcazou
2014-10-02 18:33 ` Jeff Law
0 siblings, 1 reply; 6+ messages in thread
From: Eric Botcazou @ 2014-10-02 10:42 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 758 bytes --]
[This is an old discussion about useless integral conversions introduced
behind the back of the front-end by the routines in convert.c]
> I don't like re-introducing that inconsistency.
OK.
> Maybe instead make convert.c do if (!TYPE_UNSIGNED) unsigned_type_for ()
> instead? I notice that all callers of [un]signed_type_for are in
> "premature" optimizations convert.c performs (that better should be done
> in fold-const.c).
Yes, that works for me too, patch attached, it makes sure convert_to_integer
only fiddles with the type when strictly necessary. Tested on x86-64/Linux.
2014-10-02 Eric Botcazou <ebotcazou@adacore.com>
* convert.c (convert_to_integer): Do not introduce useless conversions
between integral types.
--
Eric Botcazou
[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 1958 bytes --]
Index: convert.c
===================================================================
--- convert.c (revision 215656)
+++ convert.c (working copy)
@@ -746,8 +746,9 @@ convert_to_integer (tree type, tree expr
/* Can't do arithmetic in enumeral types
so use an integer type that will hold the values. */
if (TREE_CODE (typex) == ENUMERAL_TYPE)
- typex = lang_hooks.types.type_for_size
- (TYPE_PRECISION (typex), TYPE_UNSIGNED (typex));
+ typex
+ = lang_hooks.types.type_for_size (TYPE_PRECISION (typex),
+ TYPE_UNSIGNED (typex));
/* But now perhaps TYPEX is as wide as INPREC.
In that case, do nothing special here.
@@ -788,9 +789,15 @@ convert_to_integer (tree type, tree expr
&& (ex_form == PLUS_EXPR
|| ex_form == MINUS_EXPR
|| ex_form == MULT_EXPR)))
- typex = unsigned_type_for (typex);
+ {
+ if (!TYPE_UNSIGNED (typex))
+ typex = unsigned_type_for (typex);
+ }
else
- typex = signed_type_for (typex);
+ {
+ if (TYPE_UNSIGNED (typex))
+ typex = signed_type_for (typex);
+ }
return convert (type,
fold_build2 (ex_form, typex,
convert (typex, arg0),
@@ -805,7 +812,19 @@ convert_to_integer (tree type, tree expr
/* This is not correct for ABS_EXPR,
since we must test the sign before truncation. */
{
- tree typex = unsigned_type_for (type);
+ /* Do the arithmetic in type TYPEX,
+ then convert result to TYPE. */
+ tree typex = type;
+
+ /* Can't do arithmetic in enumeral types
+ so use an integer type that will hold the values. */
+ if (TREE_CODE (typex) == ENUMERAL_TYPE)
+ typex
+ = lang_hooks.types.type_for_size (TYPE_PRECISION (typex),
+ TYPE_UNSIGNED (typex));
+
+ if (!TYPE_UNSIGNED (typex))
+ typex = unsigned_type_for (typex);
return convert (type,
fold_build1 (ex_form, typex,
convert (typex,
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [patch] Do not generate useless integral conversions
2014-10-02 10:42 ` Eric Botcazou
@ 2014-10-02 18:33 ` Jeff Law
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Law @ 2014-10-02 18:33 UTC (permalink / raw)
To: Eric Botcazou, Richard Biener; +Cc: gcc-patches
On 10/02/14 04:40, Eric Botcazou wrote:
> [This is an old discussion about useless integral conversions introduced
> behind the back of the front-end by the routines in convert.c]
>
>> I don't like re-introducing that inconsistency.
>
> OK.
>
>> Maybe instead make convert.c do if (!TYPE_UNSIGNED) unsigned_type_for ()
>> instead? I notice that all callers of [un]signed_type_for are in
>> "premature" optimizations convert.c performs (that better should be done
>> in fold-const.c).
>
> Yes, that works for me too, patch attached, it makes sure convert_to_integer
> only fiddles with the type when strictly necessary. Tested on x86-64/Linux.
>
>
> 2014-10-02 Eric Botcazou <ebotcazou@adacore.com>
>
> * convert.c (convert_to_integer): Do not introduce useless conversions
> between integral types.
OK.
Jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-10-02 18:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-24 10:55 [patch] Do not generate useless integral conversions Eric Botcazou
2014-06-24 14:32 ` Richard Biener
2014-06-24 21:18 ` Eric Botcazou
2014-06-25 8:21 ` Richard Biener
2014-10-02 10:42 ` Eric Botcazou
2014-10-02 18:33 ` Jeff Law
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).