| Message ID | 1546601.hgDYK4VNce@polaris |
|---|---|
| State | New |
| Headers | show |
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
> 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...
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
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)