[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
--- Comment #21 from jakub at gcc dot gnu dot org 2009-01-28 10:40 --- Subject: Bug 38934 Author: jakub Date: Wed Jan 28 10:40:06 2009 New Revision: 143723 URL: http://gcc.gnu.org/viewcvs?root=gccview=revrev=143723 Log: PR middle-end/38934 * tree-vrp.c (extract_range_from_assert): For LE_EXPR and LT_EXPR set to varying whenever max has TREE_OVERFLOW set, similarly for GE_EXPR and GT_EXPR and TREE_OVERFLOW min. * gcc.dg/pr38934.c: New test. Added: trunk/gcc/testsuite/gcc.dg/pr38934.c Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/tree-vrp.c -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
--- Comment #9 from jakub at gcc dot gnu dot org 2009-01-27 10:25 ---
So, what should we do for 4.4 then?
--- tree-vrp.c.jj2009-01-02 09:32:55.0 +0100
+++ tree-vrp.c2009-01-27 11:16:57.0 +0100
@@ -1620,7 +1620,8 @@ extract_range_from_assert (value_range_t
all should be optimized away above us. */
if ((cond_code == LT_EXPR
compare_values (max, min) == 0)
- || is_overflow_infinity (max))
+ || is_overflow_infinity (max)
+ || TREE_OVERFLOW (max))
set_value_range_to_varying (vr_p);
else
{
@@ -1655,7 +1656,8 @@ extract_range_from_assert (value_range_t
all should be optimized away above us. */
if ((cond_code == GT_EXPR
compare_values (min, max) == 0)
- || is_overflow_infinity (min))
+ || is_overflow_infinity (min)
+ || TREE_OVERFLOW (min))
set_value_range_to_varying (vr_p);
else
{
fixes this for me (and, is_overflow_infinity check could be actually dropped).
There is a problem with the testcase also - while the
integer constant is so large that it is unsigned
warning is emitted always,
integer overflow in expression
is emitted only for 32-bit HWI gcc, for x86_64 -m32 as well as -m64 only the
first warning is emitted.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
--- Comment #10 from rguenther at suse dot de 2009-01-27 10:29 ---
Subject: Re: [4.3/4.4 Regression] ICE in set_value_range,
at tree-vrp.c:398
On Tue, 27 Jan 2009, jakub at gcc dot gnu dot org wrote:
--- Comment #9 from jakub at gcc dot gnu dot org 2009-01-27 10:25 ---
So, what should we do for 4.4 then?
--- tree-vrp.c.jj2009-01-02 09:32:55.0 +0100
+++ tree-vrp.c2009-01-27 11:16:57.0 +0100
@@ -1620,7 +1620,8 @@ extract_range_from_assert (value_range_t
all should be optimized away above us. */
if ((cond_code == LT_EXPR
compare_values (max, min) == 0)
- || is_overflow_infinity (max))
+ || is_overflow_infinity (max)
+ || TREE_OVERFLOW (max))
set_value_range_to_varying (vr_p);
else
{
@@ -1655,7 +1656,8 @@ extract_range_from_assert (value_range_t
all should be optimized away above us. */
if ((cond_code == GT_EXPR
compare_values (min, max) == 0)
- || is_overflow_infinity (min))
+ || is_overflow_infinity (min)
+ || TREE_OVERFLOW (min))
set_value_range_to_varying (vr_p);
else
{
That looks reasonable to me.
Richard.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
--- Comment #11 from bonzini at gnu dot org 2009-01-27 10:33 --- integer overflow in expression is emitted only for 32-bit HWI gcc, for x86_64 -m32 as well as -m64 only the first warning is emitted. If the warning should be there in 64-bit HWI gcc too, that's a separate bug. But if (as I think it should be) it should not be there, the front-end bug is the root cause of this PR and fixing it will remove the need for the VRP fix. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
--- Comment #12 from jakub at gcc dot gnu dot org 2009-01-27 11:29 --- The difference is in interpret_integer: 579 if (itk == itk_none) 580/* cpplib has already issued a warning for overflow. */ 581type = ((flags CPP_N_UNSIGNED) 582? widest_unsigned_literal_type_node 583: widest_integer_literal_type_node); 584 else 585type = integer_types[itk]; in both cases the overflow warning has been issued, but widest_integer_literal_type_node differs between 32-bit HWI and 64-bit HWI - in the former case it is signed DImode, in the latter case signed TImode. Negating TImode 0x8000 doesn't overflow, but negating DImode 0x8000 does and thus the warning in 32-bit HWI case. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
--- Comment #13 from jakub at gcc dot gnu dot org 2009-01-27 11:59 --- I wonder if the libcpp warning is correct in this case for -std=c99/-std=gnu99, saying that the constant is too large that it is unsigned doesn't match the C99 wording, which says that the constant may have a signed extended type, not unsigned. For 32-bit HWI, unfortunately I don't see how we could have a larger type than 2 * HOST_BITS_PER_WIDE_INT, as there would be no way to represet TYPE_MIN_VALUE and TYPE_MAX_VALUE. -- jakub at gcc dot gnu dot org changed: What|Removed |Added CC||jsm28 at gcc dot gnu dot org http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
--- Comment #14 from bonzini at gnu dot org 2009-01-27 12:32 --- Yes, for the record those were my thoughts too when I prepared the --std=c99 testcase. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
--- Comment #15 from joseph at codesourcery dot com 2009-01-27 13:33 --- Subject: Re: [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398 On Tue, 27 Jan 2009, jakub at gcc dot gnu dot org wrote: I wonder if the libcpp warning is correct in this case for -std=c99/-std=gnu99, saying that the constant is too large that it is unsigned doesn't match the C99 wording, which says that the constant may have a signed extended type, not I'm not clear what the issue here is supposed to be. Anything that involves a type wider than intmax_t in a program should yield a pedwarn, not just a warning, for C99. That was part of the intended effect of my patch http://gcc.gnu.org/ml/gcc-patches/2000-09/msg00816.html. The part for mode attributes was lost with http://gcc.gnu.org/ml/gcc-patches/2001-09/msg00887.html - since the relevant code is now in c-common.c again, it could be restored. I don't know offhand when the check for integer constants was lost, although I don't see it in c-lex.c. Unfortunately my original patch didn't have testcases for these diagnostics, probably because making them target-independent is hard and we didn't have the effective-target infrastructure back then. unsigned. For 32-bit HWI, unfortunately I don't see how we could have a larger type than 2 * HOST_BITS_PER_WIDE_INT, as there would be no way to represet TYPE_MIN_VALUE and TYPE_MAX_VALUE. It's very clear to me by now that HOST_WIDE_INT should only depend on the target (probably only on the target architecture), not the host; that's the only way to ensure consistency between hosts for the same target. And so all the cases that really mean something related to the target should use some other name. (There may be cases where the host not the target really is relevant, though there may also be better host types such as size_t to use in some of those cases. Unfortunately there are nearly 5000 uses of HOST_WIDE_INT in the compiler, so a huge task for anyone cleaning this up.) -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
--- Comment #16 from bonzini at gnu dot org 2009-01-27 13:44 --- It's very clear to me by now that HOST_WIDE_INT should only depend on the target (probably only on the target architecture), not the host; that's the only way to ensure consistency between hosts for the same target. It wouldn't actually be very hard to do that. I'm almost sure that it would just work. But the (sub)problem here is that the same target architecture (32-bit i386) has different HOST_WIDE_INT size depending on whether GCC is a 32-bit single-arch or a 32-/64-bit bi-arch compiler. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
--- Comment #17 from joseph at codesourcery dot com 2009-01-27 14:55 --- Subject: Re: [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398 On Tue, 27 Jan 2009, bonzini at gnu dot org wrote: It's very clear to me by now that HOST_WIDE_INT should only depend on the target (probably only on the target architecture), not the host; that's the only way to ensure consistency between hosts for the same target. It wouldn't actually be very hard to do that. I'm almost sure that it would just work. But the (sub)problem here is that the same target architecture (32-bit i386) has different HOST_WIDE_INT size depending on whether GCC is a 32-bit single-arch or a 32-/64-bit bi-arch compiler. My claim is that we should stop doing this: it should always be 64-bit for x86 and ARM, rather than depending on whether the target is biarch (x86) or whether it is EABI (ARM), just as it is for other targets that may or may not be biarch (MIPS, Power, SPARC). -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
--- Comment #18 from jakub at gcc dot gnu dot org 2009-01-27 15:14 --- The problem on ix86 is I believe that on such a register starved host, using long long in many places will mean very noticeable performance degradation. In any case, I'd prefer to change tree-vrp.c at this point (am going to bootstrap/regtest it), with the testcase with -w in dg-options, and handle the rest separately, as P2. The rest is certainly not a recent regression (3.4.0 behaves the same way between i386 cc1 and x86_64 cc1 -m32). -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
--- Comment #19 from bonzini at gnu dot org 2009-01-27 15:21 --- The problem on ix86 is I believe that on such a register starved host, using long long in many places will mean very noticeable performance degradation. Probably noticeable; however, Debian and Apple are already distributing bi-arch compilers, and something has been done to improve the situation (HOST_WIDEST_FAST_INT, the -fsplit-wide-types pass, ...). -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
-- jakub at gcc dot gnu dot org changed: What|Removed |Added AssignedTo|unassigned at gcc dot gnu |jakub at gcc dot gnu dot org |dot org | Status|NEW |ASSIGNED Last reconfirmed|2009-01-22 21:30:48 |2009-01-27 22:37:53 date|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
--- Comment #20 from rob1weld at aol dot com 2009-01-28 06:23 ---
(In reply to comment #17)
Subject: Re: [4.3/4.4 Regression] ICE in set_value_range,
On Tue, 27 Jan 2009, bonzini at gnu dot org wrote:
It's very clear to me by now that HOST_WIDE_INT should only depend on the
target (probably only on the target architecture), not the host; that's
the only way to ensure consistency between hosts for the same target.
Yes.
But the (sub)problem here is that the same target architecture (32-bit i386)
has different HOST_WIDE_INT size depending on whether GCC is a 32-bit
single-arch or a 32-/64-bit bi-arch compiler.
My claim is that we should stop doing this: it should always be 64-bit for
x86 and ARM, rather than depending on whether the target is biarch (x86)
or whether it is EABI (ARM), just as it is for other targets that may or
may not be biarch (MIPS, Power, SPARC).
(In reply to comment #18)
The problem on ix86 is I believe that on such a register starved host, using
long long in many places will mean very noticeable performance degradation.
Whether the value ought to be 64 or 32 bit depends on the boot mode of the
Target and which size code runs fastest. Register usage may be considered
but memory usage is usually not considered since ram (or HD space) is cheap.
In the event that the Target has an abnormally small amount of memory
(Picochip?) or is truly short of registers (386) then you ought to go 32
bit even if 64 bit were quicker due to this being a bigger win in the end.
In architectures where you could be in either boat a compiler switch
would be a great feature. A Pragma with the ability to set mode on
a function by function basis would be great for writing Drivers.
If I had a Desktop computer with a MIPS or Arm uP I would likely want
64 bits but if I was trying to cross-compile for my Router (MIPS Linux
2.6 with 64 M) or my Cell Phone (Arm Symbian with 96 M) then I would
likely want 32 bit as the memory available is scarce.
In Fedora 10 this seems to be well handled by the OS's header's:
/usr/include/inttypes.h
#if __WORDSIZE == 64
/* We have to define the `uintmax_t' type using `ldiv_t'. */
typedef struct
{
long int quot; /* Quotient. */
long int rem; /* Remainder. */
} imaxdiv_t;
#else
/* We have to define the `uintmax_t' type using `lldiv_t'. */
typedef struct
{
long long int quot; /* Quotient. */
long long int rem; /* Remainder. */
} imaxdiv_t;
#endif
Make it fast, make it practical, make it work,
Rob
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
--- Comment #4 from bonzini at gnu dot org 2009-01-26 15:56 --- This only happens with 32bit HWI. Does this mean it is a front-end bug that TREE_OVERFLOW is set? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
--- Comment #5 from rguenther at suse dot de 2009-01-26 15:57 --- Subject: Re: [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398 On Mon, 26 Jan 2009, bonzini at gnu dot org wrote: --- Comment #4 from bonzini at gnu dot org 2009-01-26 15:56 --- This only happens with 32bit HWI. Does this mean it is a front-end bug that TREE_OVERFLOW is set? Yes. In general TREE_OVERFLOW leaking into the IL is always a bug. Richard. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
--- Comment #6 from jakub at gcc dot gnu dot org 2009-01-26 16:02 --- That brings us back to http://gcc.gnu.org/ml/gcc-patches/2008-11/msg00532.html If the gimplifier shouldn't drop TREE_OVERFLOW bits from the IL, then it is valid to have TREE_OVERFLOWs set and tree-vrp.c needs to deal with it (use VARYING range or something similar). -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
--- Comment #7 from rguenther at suse dot de 2009-01-26 16:07 --- Subject: Re: [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398 On Mon, 26 Jan 2009, jakub at gcc dot gnu dot org wrote: --- Comment #6 from jakub at gcc dot gnu dot org 2009-01-26 16:02 --- That brings us back to http://gcc.gnu.org/ml/gcc-patches/2008-11/msg00532.html If the gimplifier shouldn't drop TREE_OVERFLOW bits from the IL, then it is valid to have TREE_OVERFLOWs set and tree-vrp.c needs to deal with it (use VARYING range or something similar). Not exactly. Your patch was correct in principle but exposed latent bugs elsewhere. Difficult ones, because fold is shared between FEs and the middle-end and fold and the FEs _do_ care about TREE_OVERFLOW. ... So indeed, working around these issues where they pop up is the way to go in the near future. (this doesn't mean a TREE_OVERFLOW in the IL is valid ...) Richard. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
--- Comment #8 from bonzini at gnu dot org 2009-01-26 16:48 --- I disagree with this: working around these issues where they pop up is the way to go in the near future. I think we need to be a bit more ambitious, and that does not mean separating overflow/non-overflow tree codes. For example in Eric's testcase, the way to go would have been not to reassociate if there was a TREE_OVERFLOW on resulting constant operands. It might even be as easy as if (con0 TREE_OVERFLOW (con0)) return NULL_TREE; (search for con0 = in fold-const.c to get the context). For 4.5, I suggest applying Jakub's patch very early and fixing bugs that result. If it turns out it's a huge Pandora's box, we can always re-revert the patch. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
[Bug middle-end/38934] [4.3/4.4 Regression] ICE in set_value_range, at tree-vrp.c:398
-- rguenth at gcc dot gnu dot org changed: What|Removed |Added Priority|P3 |P1 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=38934
