[Bug c++/21087] [4.0 Regression] ICE in do_nonmember_using_decl
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-23 02:47 --- Subject: Re: [PR c++/21087] don't keep builtin anticipated decl, override it with actual declaration On Apr 21, 2005, Mark Mitchell <[EMAIL PROTECTED]> wrote: > Alexandre Oliva wrote: >> When push_overloaded_decl() was passed a new declaration that matches >> a builtin decl, it would verify that the declarations matched and, if >> so, leave the existing (built-in) declaration alone. >> The intended behavior is to merge the built-in declaration with the >> new declaration, into the location of the built-in declaration. >> The problem is that duplicate_decl() doesn't perform such merging >> when >> the new declaration is a template decl, and then we end up with an >> overload involving the template decl and the anticipated built-in >> decl. However, overloads involving anticipated decls are something we >> try to avoid, and actually check for elsewhere. >> This patch fixes the code such that, if the existing decl is >> anticipated and the two decls weren't merged, we discard the built-in >> and use the new decl by itself. >> Bootstrapped and regtested on amd64-linux-gnu. Ok to install? > OK. Ok for 4.0 branch as well? The same patch applies cleanly there, and it's just completed bootstrap and regtesting on amd64-linux-gnu in the branch as well. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21087
[Bug c++/21087] [4.0/4.1 Regression] ICE in do_nonmember_using_decl
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-19
21:45 ---
Subject: [PR c++/21087] don't keep builtin anticipated decl, override it with
actual declaration
When push_overloaded_decl() was passed a new declaration that matches
a builtin decl, it would verify that the declarations matched and, if
so, leave the existing (built-in) declaration alone.
The intended behavior is to merge the built-in declaration with the
new declaration, into the location of the built-in declaration.
The problem is that duplicate_decl() doesn't perform such merging when
the new declaration is a template decl, and then we end up with an
overload involving the template decl and the anticipated built-in
decl. However, overloads involving anticipated decls are something we
try to avoid, and actually check for elsewhere.
This patch fixes the code such that, if the existing decl is
anticipated and the two decls weren't merged, we discard the built-in
and use the new decl by itself.
Bootstrapped and regtested on amd64-linux-gnu. Ok to install?
Index: gcc/cp/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/21087
* name-lookup.c (push_overloaded_decl): Do not overload with
non-duplicate anticipated built-in.
Index: gcc/cp/name-lookup.c
===
RCS file: /cvs/gcc/gcc/gcc/cp/name-lookup.c,v
retrieving revision 1.113
diff -u -p -r1.113 name-lookup.c
--- gcc/cp/name-lookup.c 14 Mar 2005 14:51:14 - 1.113
+++ gcc/cp/name-lookup.c 19 Apr 2005 19:20:11 -
@@ -1883,6 +1883,13 @@ push_overloaded_decl (tree decl, int fla
if (duplicate_decls (decl, fn) == fn)
POP_TIMEVAR_AND_RETURN (TV_NAME_LOOKUP, fn);
}
+
+ /* We don't overload implicit built-ins. duplicate_decls()
+may fail to merge the decls if the new decl is e.g. a
+template function. */
+ if (TREE_CODE (old) == FUNCTION_DECL
+ && DECL_ANTICIPATED (old))
+ old = NULL;
}
else if (old == error_mark_node)
/* Ignore the undefined symbol marker. */
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/21087
* g++.dg/lookup/builtin2.C: New test.
Index: gcc/testsuite/g++.dg/lookup/builtin2.C
===
RCS file: gcc/testsuite/g++.dg/lookup/builtin2.C
diff -N gcc/testsuite/g++.dg/lookup/builtin2.C
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/g++.dg/lookup/builtin2.C 19 Apr 2005 19:20:27 -
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+
+/* PR c++/21087 */
+
+/* We used to overload the template function with the built-in
+ declaration, instead of replacing it as we should, and then barf at
+ the using decl because of a test that none of the overload set
+ members were anticipated built-ins. */
+
+extern "C" signed int toupper(signed int __c) throw();
+namespace std
+{
+ template< typename a > a toupper(a,int){}
+ using ::toupper;
+}
+
+int f () {
+ std::toupper((signed int)'a');
+}
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21087
[Bug target/16888] [3.3/3.4/4.0/4.1 Regression] ICE: in print_reg, at config/i386/i386.c:7254
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-19
17:08 ---
Subject: [PR target/16888] clear reg names of unavailable regs
We used to crash at print_operand time, because the register asm
variable named a REX register, not available in 32-bit mode.
This patch arranges for us to clear the names of registers not
available for the given command-line options or defaults, which gets
us an error message at the point of the register var declaration.
An alternative would be to introduce a new target hook to validate
register names, but this didn't sound worth the effort.
Bootstrapped and regtested on amd64-linux-gnu. Ok to install?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR target/16888
* config/i386/i386.h (CONDITIONAL_REGISTER_USAGE): Clear reg names
for unavailable registers.
Index: gcc/config/i386/i386.h
===
RCS file: /cvs/gcc/gcc/gcc/config/i386/i386.h,v
retrieving revision 1.428
diff -u -p -r1.428 i386.h
--- gcc/config/i386/i386.h 14 Apr 2005 23:42:45 - 1.428
+++ gcc/config/i386/i386.h 19 Apr 2005 06:03:00 -
@@ -1042,14 +1042,14 @@ do {
\
int i; \
for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)\
if (TEST_HARD_REG_BIT (reg_class_contents[(int)MMX_REGS], i))
\
- fixed_regs[i] = call_used_regs[i] = 1; \
+ fixed_regs[i] = call_used_regs[i] = 1, reg_names[i] = ""; \
}
\
if (! TARGET_SSE) \
{
\
int i; \
for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)\
if (TEST_HARD_REG_BIT (reg_class_contents[(int)SSE_REGS], i))
\
- fixed_regs[i] = call_used_regs[i] = 1; \
+ fixed_regs[i] = call_used_regs[i] = 1, reg_names[i] = ""; \
}
\
if (! TARGET_80387 && ! TARGET_FLOAT_RETURNS_IN_80387) \
{
\
@@ -1058,7 +1058,15 @@ do {
\
COPY_HARD_REG_SET (x, reg_class_contents[(int)FLOAT_REGS]);\
for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)\
if (TEST_HARD_REG_BIT (x, i))\
- fixed_regs[i] = call_used_regs[i] = 1; \
+ fixed_regs[i] = call_used_regs[i] = 1, reg_names[i] = ""; \
+ }
\
+if (! TARGET_64BIT)
\
+ {
\
+ int i; \
+ for (i = FIRST_REX_INT_REG; i <= LAST_REX_INT_REG; i++) \
+ reg_names[i] = "";\
+ for (i = FIRST_REX_SSE_REG; i <= LAST_REX_SSE_REG; i++) \
+ reg_names[i] = "";\
}
\
} while (0)
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR target/16888
* gcc.target/i386/asm-1.c: New test.
Index: gcc/testsuite/gcc.target/i386/asm-1.c
===
RCS file: gcc/testsuite/gcc.target/i386/asm-1.c
diff -N gcc/testsuite/gcc.target/i386/asm-1.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/gcc.target/i386/asm-1.c 19 Apr 2005 06:03:15 -
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-m32" } */
+
+register unsigned int EAX asm ("r14"); /* { dg-error "register name" } */
+
+void foo ()
+{
+ EAX = 0;
+}
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16888
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-17 06:24 --- Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable types On Apr 17, 2005, Mark Mitchell <[EMAIL PROTECTED]> wrote: > Alexandre Oliva wrote: >> Mark, did you give up on COMPOUND_LITERAL_EXPRs in C++ for 4.0? The >> C++ portion of the patch at http://gcc.gnu.org/PR20103 is still >> awaiting review even for mainline :-( > Our messages crossed. Ironically, I was just making a pass over the > open 4.0 regressions, and realized that I'd failed to review this > patch. It's an ICE-on-invalid on a GNU C++ extension, so I've now > pushed it back to 4.0.1. I will be sure to review it before then. > Sorry, No worries. I didn't think it was all that important, but thought I'd point it out just in case you'd forgotten about it. How about reviewing it for mainline ASAP, so that we can be even more confident when it goes into 4.0.1? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20103
[Bug c++/17805] too liberal operator lookup
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-17 04:00 --- Subject: Re: [PR c++/17805] limit operator overload candidates for enum operands On Apr 2, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote: > On Mar 18, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote: >> On Mar 1, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote: >>> On Feb 10, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote: We're a bit too lenient in creating the candidate list for overload resolution for expressions that use user-defined operator functions. This patch arranges for us to reject functions that don't get an exact match for at least one of the enum-typed arguments, if none of the arguments have class types. Regression-tested on x86_64-linux-gnu. Ok to install? Index: gcc/cp/ChangeLog from Alexandre Oliva <[EMAIL PROTECTED]> PR c++/17805 * call.c (build_new_op): Filter out operator functions that don't satisfy enum-conversion match requirements. >>> Ping? >>> http://gcc.gnu.org/ml/gcc-patches/2005-02/msg00453.html >> Ping? > Ping? Ping? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17805
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-17 03:59 --- Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable types Mark, did you give up on COMPOUND_LITERAL_EXPRs in C++ for 4.0? The C++ portion of the patch at http://gcc.gnu.org/PR20103 is still awaiting review even for mainline :-( -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20103
[Bug target/20126] [3.3/3.4/4.0 Regression] Inlined memcmp makes one argument null on entry
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-17 02:37 --- Subject: Re: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail On Apr 16, 2005, Roger Sayle <[EMAIL PROTECTED]> wrote: > Does this clear things up? Do you agree? Yup, for both questions. Thanks for the clarification. It wasn't clear to me that the assignments played any useful role, as soon as I found out the givs could be assumed to hold the correct value. It all makes sense to me now. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20126
[Bug middle-end/20739] [4.0 regression] ICE in gimplify_addr_expr
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-16 21:58 --- Subject: Re: [PR middle-end/20739] lvalue cond-expr gimplification may crash on cv-qual diffs On Apr 15, 2005, Jeffrey A Law <[EMAIL PROTECTED]> wrote: > On Thu, 2005-04-14 at 14:02 -0300, Alexandre Oliva wrote: >> On Apr 4, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote: >> >> > If the operands of a cond-expr used as an lvalue differ in cv >> > qualification, the front-end adds nop_exprs to add cv qualifiers that >> > the gimplifier drops when simplifying &(T const)*v. The `&' was added >> > by gimplify_cond_expr. >> >> > The problem is that gimplify_addr_expr gimplifies its operand in such >> > a way that the nop_expr is dropped, and nothing puts it back in, so >> > when we test that, in the indirect_ref case in gimplify_addr_expr, the >> > types are compatible, the test fails because of the missing >> > cv-qualifier in the pointed-to type. This patch fixes this. >> >> > Ok to install if bootstrap and regtest on amd64-linux-gnu pass? >> >> Bootstrap and regtest pased on amd64-linux-gnu, at least for mainline. >> I'm retesting on the branch, since I'm not entirely sure I actually >> tested it there. > Approved for mainline. Mark has final call on the branch. Thanks, Roger had already approved it for mainline, but not yet for the branch. Mark? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20739
[Bug target/20126] [3.3/3.4/4.0 Regression] Inlined memcmp makes one argument null on entry
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-16
21:58 ---
Subject: Re: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail
On Apr 15, 2005, Roger Sayle <[EMAIL PROTECTED]> wrote:
> I agree with your proposed game plan of keeping the hard failure in
> place temporarily, to discover whether there are any other "fallback"
> strategies that would be useful. Ultimately though, I don't think we
> should close PR20126 until a "soft failure" is implemented on mainline,
> like we've (Jakub has) done on the gcc-4_0-branch (such as the
> mainline code proposed in comment #30).
But see, the problem with the soft failure mode is that, if it is ever
legitimate to leave the giv alone and not make sure we set whatever
register appears in it to the right value, then can't we always do it,
removing all of the (thus useless) workarounds?
And if there's any case in which it is not legitimate to do so, then
the soft failure mode would be a disservice to the user, that would
silently get a miscompiled program. We should probably at least warn
in this case.
Anyhow, here's a patch that was tested with bootstrap and regtest on
amd64-linux-gnu on mainline, that brings in the soft failure mode from
the 4.0 branch to mainline, without removing the potentially-useless
workarounds.
Index: gcc/loop.c
===
RCS file: /cvs/gcc/gcc/gcc/loop.c,v
retrieving revision 1.526
diff -u -p -r1.526 loop.c
--- gcc/loop.c 10 Apr 2005 04:00:45 - 1.526
+++ gcc/loop.c 16 Apr 2005 21:37:45 -
@@ -5494,11 +5494,23 @@ loop_givs_rescan (struct loop *loop, str
rtx reg, seq;
start_sequence ();
reg = force_reg (v->mode, *v->location);
- seq = get_insns ();
- end_sequence ();
- loop_insn_emit_before (loop, 0, v->insn, seq);
- if (!validate_change_maybe_volatile (v->insn, v->location, reg))
- gcc_unreachable ();
+ if (validate_change_maybe_volatile (v->insn, v->location, reg))
+ {
+ seq = get_insns ();
+ end_sequence ();
+ loop_insn_emit_before (loop, 0, v->insn, seq);
+ }
+ else
+ {
+ end_sequence ();
+ if (loop_dump_stream)
+ fprintf (loop_dump_stream,
+"unable to reduce iv in insn %d\n",
+INSN_UID (v->insn));
+ bl->all_reduced = 0;
+ v->ignore = 1;
+ continue;
+ }
}
}
else if (v->replaceable)
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20126
[Bug target/20126] [3.3/3.4/4.0 Regression] Inlined memcmp makes one argument null on entry
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-16
21:48 ---
Subject: Re: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail
On Apr 15, 2005, Roger Sayle <[EMAIL PROTECTED]> wrote:
> Sure. Your patch in comment #28 of bugzilla PR20126 is OK for mainline
> to resolve Josh's bootstrap failure. Sounds like you've already done
> the necessary testing, and I'll trust you on a suitable ChangeLog entry.
Thanks, here's what I've just checked in.
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR target/20126
* loop.c (loop_givs_rescan): Handle non-replaceable (plus (reg)
(const)).
Index: gcc/loop.c
===
RCS file: /cvs/gcc/gcc/gcc/loop.c,v
retrieving revision 1.526
diff -u -p -r1.526 loop.c
--- gcc/loop.c 10 Apr 2005 04:00:45 - 1.526
+++ gcc/loop.c 16 Apr 2005 21:40:02 -
@@ -5488,6 +5488,15 @@ loop_givs_rescan (struct loop *loop, str
loop_insn_emit_before (loop, 0, v->insn,
gen_move_insn (*v->location,
v->new_reg));
+ else if (GET_CODE (*v->location) == PLUS
+ && REG_P (XEXP (*v->location, 0))
+ && CONSTANT_P (XEXP (*v->location, 1)))
+ loop_insn_emit_before (loop, 0, v->insn,
+ gen_move_insn (XEXP (*v->location, 0),
+ gen_rtx_MINUS
+ (GET_MODE (*v->location),
+ v->new_reg,
+ XEXP (*v->location, 1;
else
{
/* If it wasn't a reg, create a pseudo and use that. */
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20126
[Bug tree-optimization/21029] [4.1 Regression] vrp miscompiles Ada front-end, drops loop exit test in well-defined wrap-around circumstances
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-15
05:56 ---
Subject: [PR tree-optimization/21029, RFC] harmful chrec type conversions
I started out by handling the specific case that the Ada front-end
triggers, reduced to function f() in the attached testcase. Later on,
as I understood the failure more, I figured I'd exercise some other
cases, and painted myself into a corner in which each combination of
widening-or-narrowing signedness-changing-or-not conversion that
chrec_convert() might be asked to handle would be harmed by simply
converting CHREC_LEFT and CHREC_RIGHT to the requested type, and the
strategy we already used for widening conversions gave us correct
results.
Are there good reasons to try to perform the conversions in place and
try to work around the loss of information elsewhere, or is this
approach reasonable?
This is not a final patch; if the idea is considered sound, I'd simply
remove the if and the then-dead remaining code in chrec_convert.
Meanwhile, I'm bootstrapping this on amd64-linux-gnu.
Comments?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR tree-optimization/21029
* tree-chrec.c (chrec_convert): Handle same-precision integral
types that differ in signedness like widening conversions.
Index: gcc/tree-chrec.c
===
RCS file: /cvs/gcc/gcc/gcc/tree-chrec.c,v
retrieving revision 2.14
diff -u -p -r2.14 tree-chrec.c
--- gcc/tree-chrec.c 5 Apr 2005 07:06:23 - 2.14
+++ gcc/tree-chrec.c 15 Apr 2005 05:40:27 -
@@ -1033,7 +1033,16 @@ chrec_convert (tree type,
if (ct == type)
return chrec;
- if (TYPE_PRECISION (ct) < TYPE_PRECISION (type))
+ if (1 /* Even truncations need to carry information about
+ well-defined overflows in narrowing conversions that might
+ have invoked undefined behavior should the operations be
+ performed directly in the narrow type. */
+ || TYPE_PRECISION (ct) < TYPE_PRECISION (type)
+ /* Sign changes may involve well-defined overflows; avoid
+silently dropping the signedness change.
+??? Should we limit ourselves to same-precision types here? */
+ || (INTEGRAL_TYPE_P (ct) && INTEGRAL_TYPE_P (type)
+ && TYPE_UNSIGNED (ct) != TYPE_UNSIGNED (type)))
return count_ev_in_wider_type (type, chrec);
switch (TREE_CODE (chrec))
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR tree-optimization/21029
* gcc.dg/tree-ssa/pr21029.c: New.
Index: gcc/testsuite/gcc.dg/tree-ssa/pr21029.c
===
RCS file: gcc/testsuite/gcc.dg/tree-ssa/pr21029.c
diff -N gcc/testsuite/gcc.dg/tree-ssa/pr21029.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/gcc.dg/tree-ssa/pr21029.c 15 Apr 2005 05:40:41 -
@@ -0,0 +1,176 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+/* PR tree-optimization/21029
+
+ f() used to get optimized to an infinite loop by tree-vrp, because
+ j is assumed to be non-negative. Even though the conversion from
+ unsigned to signed has unspecified results if the expression value
+ is not representable in the signed type, the compiler itself (e.g.,
+ the Ada front end) depends on wrap-around behavior. */
+
+unsigned int g(void) {
+ return (unsigned int)g;
+}
+
+unsigned int f(void) {
+ unsigned int k = g ();
+
+ unsigned char i = 123;
+ signed char j;
+
+ do
+if ((j = (signed char) i) < 0)
+ break;
+else
+ i++;
+ while (1);
+
+ /* This is here just so that the compiler introduces ASSERT_EXPR so
+ as to run the vrp pass. Yuck. */
+ if (i <= k)
+k = i - 2;
+ else
+k = k + 3;
+
+ return k;
+}
+
+/* Now let's torture it a bit further. Narrowing conversions need
+ similar treatment. */
+
+unsigned int f1 (void) {
+ unsigned int k = g ();
+
+ unsigned short i = 123;
+ signed char j;
+
+ do
+if ((j = (signed char) i) < 0)
+ break;
+else
+ i++;
+ while (1);
+
+ /* This is here just so that the compiler introduces ASSERT_EXPR so
+ as to run the vrp pass. Yuck. */
+ if (i <= k)
+k = i - 2;
+ else
+k = k + 3;
+
+ return k;
+}
+
+/* And so do widening conversions. */
+
+unsigned int f2 (void) {
+ unsigned int k = g ();
+
+ unsigned char i = 123;
+ signed short j;
+
+ do
+if ((j = (signed short) (signed char) i) < 0)
+ break;
+else
+ i++;
+ while (1);
+
+ /* This is here just so that the compiler introduces ASSERT_EXPR so
+ as to run the vrp pass. Yuck. */
+ if (i <= k)
+k = i - 2;
+ else
+k = k + 3;
+
+ return k;
+}
+
+/* Check same-sign truncations with an increment that turns into
+ decrements. */
+
+unsigned int f3 (void) {
+ unsigned int k = g ();
+
+ signed short i = 5;
+ signed char j;
+
+ do
+if ((j = (signed char) i) < 0)
+ break;
+else
+ i += 255;
+ while (
[Bug target/20126] [3.3/3.4/4.0 Regression] Inlined memcmp makes one argument null on entry
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-15 03:31 --- Subject: Re: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail On Apr 12, 2005, Roger Sayle <[EMAIL PROTECTED]> wrote: > I still like your fallbacks, that by trying harder we perform better > optimization, The more I think about this, the more I have the impression that perhaps the fallbacks are not necessary. My gut feeling (that has been wrong before, so take that with a grain of salt) is that, if we mark the giv as ignored, its assignment within the loop won't be removed (if they ever are, I'm not entirely sure), so if we introduce a new assignment, we're wasting cpu cycles either in the compiler, if it has to remove the redundant set, or at run-time, if it can't figure that out. My feeling stems from the fact that we've never had (AFAIK) a situation in which the failure to make the change caused a miscompilation, except in the case of a biv whose final assignment was hoisted before the loop. I can't quite figure out why this would ever make sense; it appears to me that computing its final value after the loop is always profitable is always better because it would reduce register pressure, but I may be way off here. In the two other cases that were exposed by attempts to report failures in the substitution (namely, the Ada bootstrap error that Jakub reported against an earlier version of the patch, and the arm-elf build failure that Josh reported), the value was there, and, since the code actually refrained from checking the result of validate_change, perhaps it really didn't matter whether the substitution worked. At least until we started swimming bivs to before the loop... So I'm wondering if taking out all of the workarounds and going back to something like what is now in the 4.0 branch, except for the use of validate_change_maybe_volatile, wouldn't get us exactly what we want. Unfortunately, cases in which the substitution fails are quite rare, and the loop code isn't exactly trivial, so it's hard to decide whether we've just been lucky, or the code was already mostly correct. Anyhow, in the meantime, could I check in the patch to fix Josh's asm-elf build failure? I haven't been able to bootstrap the tree lately because of PR21029, but I know it would bootstrap successfully on amd64-linux-gnu (it would never hit the point that I changed :-), and I know it fixes the arm-elf problem. It would be nice to keep the hard failure in place for a bit longer, such that we stood a better chance of finding other situations that might require work arounds. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20126
[Bug middle-end/20739] [4.0/4.1 regression] ICE in gimplify_addr_expr
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-15 02:32 --- Subject: Re: [PR middle-end/20739] lvalue cond-expr gimplification may crash on cv-qual diffs On Apr 14, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote: > Bootstrap and regtest pased on amd64-linux-gnu, at least for mainline. > I'm retesting on the branch, since I'm not entirely sure I actually > tested it there. Bootstrap and test completed on amd64-linux-gnu. > Index: gcc/ChangeLog > from Alexandre Oliva <[EMAIL PROTECTED]> > PR middle-end/20739 > * gimplify.c (gimplify_addr_expr): Compensate for removal of > e.g. cv-qualification conversions. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20739
[Bug target/20126] [3.3/3.4/4.0 Regression] Inlined memcmp makes one argument null on entry
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-14 17:21 --- Subject: Re: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail On Apr 12, 2005, Roger Sayle <[EMAIL PROTECTED]> wrote: > ! v->ignore = 1; What's the point of the statement above? If we actually know the giv reg has the right value, why not use it for other purposes as well? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20126
[Bug middle-end/20739] [4.0/4.1 regression] ICE in gimplify_addr_expr
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-14
17:03 ---
Subject: Re: [PR middle-end/20739] lvalue cond-expr gimplification may crash on
cv-qual diffs
On Apr 4, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote:
> If the operands of a cond-expr used as an lvalue differ in cv
> qualification, the front-end adds nop_exprs to add cv qualifiers that
> the gimplifier drops when simplifying &(T const)*v. The `&' was added
> by gimplify_cond_expr.
> The problem is that gimplify_addr_expr gimplifies its operand in such
> a way that the nop_expr is dropped, and nothing puts it back in, so
> when we test that, in the indirect_ref case in gimplify_addr_expr, the
> types are compatible, the test fails because of the missing
> cv-qualifier in the pointed-to type. This patch fixes this.
> Ok to install if bootstrap and regtest on amd64-linux-gnu pass?
Bootstrap and regtest pased on amd64-linux-gnu, at least for mainline.
I'm retesting on the branch, since I'm not entirely sure I actually
tested it there.
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR middle-end/20739
* gimplify.c (gimplify_addr_expr): Compensate for removal of
e.g. cv-qualification conversions.
Index: gcc/gimplify.c
===
RCS file: /cvs/gcc/gcc/gcc/gimplify.c,v
retrieving revision 2.122
diff -u -p -r2.122 gimplify.c
--- gcc/gimplify.c 1 Apr 2005 03:42:41 - 2.122
+++ gcc/gimplify.c 4 Apr 2005 11:28:04 -
@@ -3229,6 +3232,9 @@ gimplify_addr_expr (tree *expr_p, tree *
builtins like __builtin_va_end). */
/* Caution: the silent array decomposition semantics we allow for
ADDR_EXPR means we can't always discard the pair. */
+ /* Gimplification of the ADDR_EXPR operand may drop
+cv-qualification conversions, so make sure we add them if
+needed. */
{
tree op00 = TREE_OPERAND (op0, 0);
tree t_expr = TREE_TYPE (expr);
@@ -3238,9 +3244,9 @@ gimplify_addr_expr (tree *expr_p, tree *
{
#ifdef ENABLE_CHECKING
tree t_op0 = TREE_TYPE (op0);
- gcc_assert (TREE_CODE (t_op0) == ARRAY_TYPE
- && POINTER_TYPE_P (t_expr)
- && cpt_same_type (TREE_TYPE (t_op0),
+ gcc_assert (POINTER_TYPE_P (t_expr)
+ && cpt_same_type (TREE_CODE (t_op0) == ARRAY_TYPE
+ ? TREE_TYPE (t_op0) : t_op0,
TREE_TYPE (t_expr))
&& POINTER_TYPE_P (t_op00)
&& cpt_same_type (t_op0, TREE_TYPE (t_op00)));
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR middle-end/20739
* gcc.dg/tree-ssa/pr20739.c: New test.
Index: gcc/testsuite/gcc.dg/tree-ssa/pr20739.c
===
RCS file: gcc/testsuite/gcc.dg/tree-ssa/pr20739.c
diff -N gcc/testsuite/gcc.dg/tree-ssa/pr20739.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/gcc.dg/tree-ssa/pr20739.c 4 Apr 2005 11:28:20 -
@@ -0,0 +1,24 @@
+/* PR middle-end/20739 */
+
+/* dg-do compile */
+/* dg-options "-O" */
+
+/* We used to fail to compile this because gimplification dropped the
+ conversion that added the const qualifier to the sub-expression
+ involving baz, and then immediately noticed and reported its
+ absence. */
+
+typedef struct
+{
+char chars[5];
+}
+baz_t;
+
+extern baz_t * baz;
+
+extern void foo (baz_t);
+int
+bar (const baz_t * ls)
+{
+foo (ls == 0 ? *(&baz[0]) : *ls);
+}
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20739
[Bug target/20126] [3.3/3.4/4.0 Regression] Inlined memcmp makes one argument null on entry
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-12 08:19 --- Subject: Re: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail On Apr 12, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote: > They are optimized away, but if I can figure out what the > conditions are in which we don't need to introduce assignments to the > giv, we could simply avoid all the messy new code, that assumes we > absolutely must replace the expression with something else. It appears that the only case in which we could do away with adding the assignment is if giv->same is biv, and we mark the bl with all_reduced=0. It appears to me that, in any other case, the assignment we're relying on might have been dropped or hoisted. I'd love to be wrong, though. If I am, we could get your problem, as well as the original bug report, fixed by simply reverting my patch and setting bl->all_reduced=0 if validate_change failed. This actually works for the testcases I've tried, but I'm not convinced it works in the general case. Does any expert in rtl loop care to chime in? Thanks, -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20126
[Bug target/20126] [3.3/3.4/4.0 Regression] Inlined memcmp makes one argument null on entry
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-12
06:58 ---
Subject: Re: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail
On Apr 12, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote:
> On Apr 11, 2005, Josh Conner <[EMAIL PROTECTED]> wrote:
>> I'm now getting a ICE building the mainline (please ignore the
>> "3.4.3" in the directory structure, it's a holdover from the build
>> scripts I'm using) that appears to be related to this patch.
> That sounds possible. The patch might indeed have the effect of
> turning previously-silent miscompilations into ICEs. I didn't expect
> we'd get any, but you may be proving me wrong. Would you please send
> me the preprocessed testcase in private, or just confirm that you're
> using newlib as the C library? I'll then try to duplicate the problem
> on one of the platforms at my disposal.
I went ahead and tried to build an arm-elf toolchain out of uberbaum,
and got the problem.
The problem was an stmia instruction in a loop, whose base address
pseudo loop wanted to replace with another pseudo plus a constant.
Since there's a requirement that all of the base addresses match, when
we started replacing the address expressions of any of the
destinations other than the first, that didn't have an offset, the
insn failed to match, and none of the work arounds I introduced for
such a replacement failure succeeded.
The only relatively simple way to overcome this error I could think of
was to introduce yet another work around, to enable us to handle not
only REGs, but also PLUS involving a REG and a constant (very likely
a literal immediate, but I didn't introduce such a requirement).
This has enabled the testcase to compile successfully and correctly,
although we generate a total of 4 assignments to the pseudo in
response to the 4 givs detected involving it in the 4-register stmia
instruction. Not a big deal, since they end up being optimized away.
The bad news is that this is a regression. The code before my patch
would still work: for some reason I still don't understand, the
assignment of the base pseudo isn't removed, so with my new work
around patch below, we end up introducing unnecessary assignments to
the giv. They are optimized away, but if I can figure out what the
conditions are in which we don't need to introduce assignments to the
giv, we could simply avoid all the messy new code, that assumes we
absolutely must replace the expression with something else.
Anyhow, here's the patch with the workaround for the problem you ran
into. I'll submit it formally when it completes testing.
Index: gcc/loop.c
===
RCS file: /cvs/uberbaum/gcc/loop.c,v
retrieving revision 1.526
diff -u -p -r1.526 loop.c
--- gcc/loop.c 10 Apr 2005 04:00:45 - 1.526
+++ gcc/loop.c 12 Apr 2005 06:35:26 -
@@ -5488,6 +5488,15 @@ loop_givs_rescan (struct loop *loop, str
loop_insn_emit_before (loop, 0, v->insn,
gen_move_insn (*v->location,
v->new_reg));
+ else if (GET_CODE (*v->location) == PLUS
+ && REG_P (XEXP (*v->location, 0))
+ && CONSTANT_P (XEXP (*v->location, 1)))
+ loop_insn_emit_before (loop, 0, v->insn,
+ gen_move_insn (XEXP (*v->location, 0),
+ gen_rtx_MINUS
+ (GET_MODE (*v->location),
+ v->new_reg,
+ XEXP (*v->location, 1;
else
{
/* If it wasn't a reg, create a pseudo and use that. */
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20126
[Bug target/20126] [3.3/3.4/4.0 Regression] Inlined memcmp makes one argument null on entry
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-12 03:35 --- Subject: Re: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail On Apr 11, 2005, Josh Conner <[EMAIL PROTECTED]> wrote: > I'm now getting a ICE building the mainline (please ignore the > "3.4.3" in the directory structure, it's a holdover from the build > scripts I'm using) that appears to be related to this patch. That sounds possible. The patch might indeed have the effect of turning previously-silent miscompilations into ICEs. I didn't expect we'd get any, but you may be proving me wrong. Would you please send me the preprocessed testcase in private, or just confirm that you're using newlib as the C library? I'll then try to duplicate the problem on one of the platforms at my disposal. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20126
[Bug target/20126] [3.3/3.4/4.0 Regression] Inlined memcmp makes one argument null on entry
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-11 03:51 --- Subject: Re: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail On Apr 10, 2005, Mark Mitchell <[EMAIL PROTECTED]> wrote: > Thanks for alerting me to this one; it does look relatively > serious. I've added it to: > http://gcc.gnu.org/wiki/Last-Minute%20Requests%20for%204.0.0 > and will consider whether it merits a second release-candidate. FWIW, I've bootstrapped and regtested it on amd64-linux-gnu on 4.0 branch as well. The same patch applies cleanly. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20126
[Bug target/20126] [3.3/3.4/4.0/4.1 Regression] Inlined memcmp makes one argument null on entry
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-10
02:43 ---
Subject: Re: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail
On Apr 8, 2005, Roger Sayle <[EMAIL PROTECTED]> wrote:
> ++ /* If there isn't a volatile MEM, there's nothing we can do. */
> ++ || !for_each_rtx (&object, volatile_mem_p, 0)
This actually caused crashes. We don't want to scan the entire insn
(it might contain NULLs), only the insn pattern. Here's the patch
that bootstrapped and passed regression testing on amd64-linux-gnu.
Ok?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR target/20126
* loop.c (loop_givs_rescan): If replacement of DEST_ADDR failed,
set the original address pseudo to the correct value before the
original insn, if possible, and leave the insn alone, otherwise
create a new pseudo, set it and replace it in the insn.
* recog.c (validate_change_maybe_volatile): New.
* recog.h (validate_change_maybe_volatile): Declare.
Index: gcc/loop.c
===
RCS file: /cvs/gcc/gcc/gcc/loop.c,v
retrieving revision 1.525
diff -u -p -r1.525 loop.c
--- gcc/loop.c 2 Apr 2005 16:53:38 - 1.525
+++ gcc/loop.c 10 Apr 2005 00:13:56 -
@@ -5476,9 +5476,31 @@ loop_givs_rescan (struct loop *loop, str
mark_reg_pointer (v->new_reg, 0);
if (v->giv_type == DEST_ADDR)
- /* Store reduced reg as the address in the memref where we found
- this giv. */
- validate_change (v->insn, v->location, v->new_reg, 0);
+ {
+ /* Store reduced reg as the address in the memref where we found
+this giv. */
+ if (validate_change_maybe_volatile (v->insn, v->location,
+ v->new_reg))
+ /* Yay, it worked! */;
+ /* Not replaceable; emit an insn to set the original
+giv reg from the reduced giv. */
+ else if (REG_P (*v->location))
+ loop_insn_emit_before (loop, 0, v->insn,
+ gen_move_insn (*v->location,
+ v->new_reg));
+ else
+ {
+ /* If it wasn't a reg, create a pseudo and use that. */
+ rtx reg, seq;
+ start_sequence ();
+ reg = force_reg (v->mode, *v->location);
+ seq = get_insns ();
+ end_sequence ();
+ loop_insn_emit_before (loop, 0, v->insn, seq);
+ if (!validate_change_maybe_volatile (v->insn, v->location, reg))
+ gcc_unreachable ();
+ }
+ }
else if (v->replaceable)
{
reg_map[REGNO (v->dest_reg)] = v->new_reg;
Index: gcc/recog.c
===
RCS file: /cvs/gcc/gcc/gcc/recog.c,v
retrieving revision 1.221
diff -u -p -r1.221 recog.c
--- gcc/recog.c 7 Mar 2005 13:52:08 - 1.221
+++ gcc/recog.c 10 Apr 2005 00:13:57 -
@@ -235,6 +235,46 @@ validate_change (rtx object, rtx *loc, r
return apply_change_group ();
}
+
+/* Function to be passed to for_each_rtx to test whether a piece of
+ RTL contains any mem/v. */
+static int
+volatile_mem_p (rtx *x, void *data ATTRIBUTE_UNUSED)
+{
+ return (MEM_P (*x) && MEM_VOLATILE_P (*x));
+}
+
+/* Same as validate_change, but doesn't support groups, and it accepts
+ volatile mems if they're already present in the original insn. */
+
+int
+validate_change_maybe_volatile (rtx object, rtx *loc, rtx new)
+{
+ int result;
+
+ if (validate_change (object, loc, new, 0))
+return 1;
+
+ if (volatile_ok
+ /* If there isn't a volatile MEM, there's nothing we can do. */
+ || !for_each_rtx (&PATTERN (object), volatile_mem_p, 0)
+ /* Make sure we're not adding or removing volatile MEMs. */
+ || for_each_rtx (loc, volatile_mem_p, 0)
+ || for_each_rtx (&new, volatile_mem_p, 0)
+ || !insn_invalid_p (object))
+return 0;
+
+ volatile_ok = 1;
+
+ gcc_assert (!insn_invalid_p (object));
+
+ result = validate_change (object, loc, new, 0);
+
+ volatile_ok = 0;
+
+ return result;
+}
+
/* This subroutine of apply_change_group verifies whether the changes to INSN
were valid; i.e. whether INSN can still be recognized. */
Index: gcc/recog.h
===
RCS file: /cvs/gcc/gcc/gcc/recog.h,v
retrieving revision 1.55
diff -u -p -r1.55 recog.h
--- gcc/recog.h 7 Mar 2005 13:52:09 - 1.55
+++ gcc/recog.h 10 Apr 2005 00:13:57 -
@@ -74,6 +74,7 @@ extern void init_recog_no_volatile (void
extern int check_asm_operands (rtx);
extern int asm_operand_ok (rtx, const char *);
extern int validate_change (rtx, rtx *, rtx, int);
+extern int validate_change_maybe_volatile (rtx, rtx *, rtx);
extern int insn_invalid_p (rtx);
extern void confirm_change_group (void);
extern int apply_chang
[Bug target/20126] [3.3/3.4/4.0/4.1 Regression] Inlined memcmp makes one argument null on entry
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-08 20:51 --- Subject: Re: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail On Apr 8, 2005, Roger Sayle <[EMAIL PROTECTED]> wrote: > Ahh, I now see the misunderstanding; you changed/fixed the other > "safe" gcc_assert statement, and missed the important one that I was > worried about. Sorry for the confusion. Yep. Doh! > Secondly: > + if (volatile_ok > + /* Make sure we're not adding or removing volatile MEMs. */ > + || for_each_rtx (loc, volatile_mem_p, 0) > + || for_each_rtx (&new, volatile_mem_p, 0) > + || ! insn_invalid_p (object)) > +return 0; > The suggestion wasn't just to reorder the existing for_each_rtx to > move these tests earlier, it was to confirm that the original "whole" > instruction had a volatile memory reference in it, i.e. that this is > a problematic case, before doing any more work. Aaah. Clearly, I wasn't thinking right when I made the change. I'll test your suggested change along with the gcc_assert fix. > Sorry again for the inconvenience, No worries. I shouldn't have rushed to making the changes and starting a bootstrap before going to bed on a night when I was so tired :-/ I'll post the patch when testing is complete. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20126
[Bug target/20126] [3.3/3.4/4.0/4.1 Regression] Inlined memcmp makes one argument null on entry
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-08
16:34 ---
Subject: Re: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail
On Mar 11, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote:
> On Mar 10, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote:
>> + ??? Should this should search new for new volatile MEMs and reject
>> + them? */
> Here's a stricter version that does test for this.
Roger suggested some changes in the patch. I've finally completed
bootstrap and test with and without the patch on amd64-linux-gnu, and
posted the results to the test-results list. No regressions. Ok to
install?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR target/20126
* loop.c (loop_givs_rescan): If replacement of DEST_ADDR failed,
set the original address pseudo to the correct value before the
original insn, if possible, and leave the insn alone, otherwise
create a new pseudo, set it and replace it in the insn.
* recog.c (validate_change_maybe_volatile): New.
* recog.h (validate_change_maybe_volatile): Declare.
Index: gcc/loop.c
===
RCS file: /cvs/gcc/gcc/gcc/loop.c,v
retrieving revision 1.525
diff -u -p -r1.525 loop.c
--- gcc/loop.c 2 Apr 2005 16:53:38 - 1.525
+++ gcc/loop.c 5 Apr 2005 19:22:41 -
@@ -5476,9 +5476,31 @@ loop_givs_rescan (struct loop *loop, str
mark_reg_pointer (v->new_reg, 0);
if (v->giv_type == DEST_ADDR)
- /* Store reduced reg as the address in the memref where we found
- this giv. */
- validate_change (v->insn, v->location, v->new_reg, 0);
+ {
+ /* Store reduced reg as the address in the memref where we found
+this giv. */
+ if (validate_change_maybe_volatile (v->insn, v->location,
+ v->new_reg))
+ /* Yay, it worked! */;
+ /* Not replaceable; emit an insn to set the original
+giv reg from the reduced giv. */
+ else if (REG_P (*v->location))
+ loop_insn_emit_before (loop, 0, v->insn,
+ gen_move_insn (*v->location,
+ v->new_reg));
+ else
+ {
+ /* If it wasn't a reg, create a pseudo and use that. */
+ rtx reg, seq;
+ start_sequence ();
+ reg = force_reg (v->mode, *v->location);
+ seq = get_insns ();
+ end_sequence ();
+ loop_insn_emit_before (loop, 0, v->insn, seq);
+ gcc_assert (validate_change_maybe_volatile (v->insn, v->location,
+ reg));
+ }
+ }
else if (v->replaceable)
{
reg_map[REGNO (v->dest_reg)] = v->new_reg;
Index: gcc/recog.c
===
RCS file: /cvs/gcc/gcc/gcc/recog.c,v
retrieving revision 1.221
diff -u -p -r1.221 recog.c
--- gcc/recog.c 7 Mar 2005 13:52:08 - 1.221
+++ gcc/recog.c 5 Apr 2005 19:22:43 -
@@ -235,6 +235,45 @@ validate_change (rtx object, rtx *loc, r
return apply_change_group ();
}
+
+/* Function to be passed to for_each_rtx to test whether a piece of
+ RTL contains any mem/v. */
+static int
+volatile_mem_p (rtx *x, void *data ATTRIBUTE_UNUSED)
+{
+ return (MEM_P (*x) && MEM_VOLATILE_P (*x));
+}
+
+/* Same as validate_change, but doesn't support groups, and it accepts
+ volatile mems if they're already present in the original insn. */
+
+int
+validate_change_maybe_volatile (rtx object, rtx *loc, rtx new)
+{
+ int result;
+
+ if (validate_change (object, loc, new, 0))
+return 1;
+
+ if (volatile_ok
+ /* Make sure we're not adding or removing volatile MEMs. */
+ || for_each_rtx (loc, volatile_mem_p, 0)
+ || for_each_rtx (&new, volatile_mem_p, 0)
+ || ! insn_invalid_p (object))
+return 0;
+
+ volatile_ok = 1;
+
+ if (insn_invalid_p (object))
+gcc_unreachable ();
+
+ result = validate_change (object, loc, new, 0);
+
+ volatile_ok = 0;
+
+ return result;
+}
+
/* This subroutine of apply_change_group verifies whether the changes to INSN
were valid; i.e. whether INSN can still be recognized. */
Index: gcc/recog.h
===
RCS file: /cvs/gcc/gcc/gcc/recog.h,v
retrieving revision 1.55
diff -u -p -r1.55 recog.h
--- gcc/recog.h 7 Mar 2005 13:52:09 - 1.55
+++ gcc/recog.h 5 Apr 2005 19:22:43 -
@@ -74,6 +74,7 @@ extern void init_recog_no_volatile (void
extern int check_asm_operands (rtx);
extern int asm_operand_ok (rtx, const char *);
extern int validate_change (rtx, rtx *, rtx, int);
+extern int validate_change_maybe_volatile (rtx, rtx *, rtx);
extern int insn_invalid_p (rtx);
extern void confirm_change_group (void);
extern int apply_change_g
[Bug c++/19199] [3.3/3.4/4.0 Regression] Wrong warning about returning a reference to a temporary
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-04 20:18 --- Subject: Re: [Committed] PR c++/19199: Preserve COND_EXPR lvalueness in fold On Apr 4, 2005, Roger Sayle <[EMAIL PROTECTED]> wrote: > My apologies yet again for not being more explicit about all of the > things that were wrong (and or I was unhappy with) with your proposed > solution. I'd hoped that I was clear in the comments in the bugzilla > thread, and that you'd appreciate the issues it addressed. I didn't. In fact, I only saw your responses in bugzilla early last week, since bugzilla e-mail I get goes to a folder that I don't read very often because of all the traffic in gcc-prs. > [1] The use of pedantic_lvalues to identify the non-C front-ends, > adversely affects code generation in the Java, Fortran and Ada > front-ends. Indeed. I'd misunderstood what pedantic_lvalues stood for, and thought it could be used to distinguish C++ from other languages, not C from other languages. My mistake. That's why it was never clear to me that you disliked its use, and why. > The use of COND_EXPRs as lvalues is unique to the > C++ front-end I don't know about that. Among all the languages supported by GCC, I'm only familiar with C, C++ and Java. > [3] Your patch is too invasive. Compared to the four-line counter > proposal that disables just the problematic class of transformations, > your much larger patch inherently contains a much larger risk. For > example, there is absolutely no need to modify the source code on the > "A >= 0 ? A : -A -> abs (A)" paths as these transformations could > never interfere with the lvalueness of an expression. Granted. OTOH, the patch faulted in trying to be overly consistent in potentially-broken code, as opposed to keeping potentially-broken code broken. > Additionally, once one of the longer term solutions proposed by Mark > or me is implemented, all of these workarounds will have to be undone/ > reverted. By only affecting a single clause, we avoid the potential > for leaving historic code bit-rotting in the tree. If you're convinced that's the only clause that triggers the problem, great. I wasn't. > [4] Despite your counter claims you approach *does* inhibit the ability > of the the tree-ssa optimizers to synthesize MIN_EXPR and MAX_EXPR > nodes. Indeed. I still had the behavior from an earlier version of the patch, in which cond_expr returned different values from maybe_lvalue_p depending on whether we were in gimple form or not. Sorry about that. > [5] For the immediate term, I don't think its worth worrying about > converting non-lvalues into lvalues, No worries here, all of my effort was toward preserving lvalueness. > And although, not serious enough to warrant a [6], it should be pointed > out that several of your recent patches have introduced regressions. Oh, like the patch in which I did check in a testcase after bootstrapping it on amd64-linux-gnu, although I didn't realize it still failed on i686-pc-linux-gnu, a host on which I don't bootstrap very often any more because my 32-bit hardware has become too slow? > Indeed, you've not yet reported that your patch has been sucessfully > bootstraped or regression tested on any target triple. Did I have to, before checking the patch in? I didn't think so. > Indeed, your > approach of posting a patch before completing the prerequisite testing > sprecified/stressed in contribute.html, on more than one occassion > recently resulted in the patch having to be rewritten/tweaked. And the need for rewriting/tweaking is exactly the reason why I choose to post patches early. Quite often, patches don't go in as they're posted, since maintainers often request additional minor tweaks. At about 24 hours per test cycle, and about as much for patch review, I don't think posting a patch proposal early would hurt anyone. In general, I'm asking for feedback on the approach, rather than on the exact patch spelling. If people find the patch to be good as is, great. Otherwise, sometimes I can still tweak the patch and get it into the next build&test cycle. And, more importantly, I like to describe the problem and the solution while it's still fresh in my mind. If I wait 1 or 2 days to post it, I won't be as precise in my description, and if questions arise, I may fail to answer correctly. > Indeed, as witnessed in "comment #17", I've already approved an > earlier version your patch once, only to later discover you were > wasting my time. If you find that reviewing patch proposals is wasting your time, I'm sorry. In general, I run a `make all; make check-gcc or make check-g++' on a previously-built tree before starting a full bootstrap and posting the patch, but this is not enough to catch all errors. In fact, a full bootstrap and test cycle isn't enough either, since we don't all use the same hosts. So, patches are going to be posted that need revision. Since most of my patches have minor revisions reque
[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-04 15:02 --- Subject: Re: [Committed] PR c++/19199: Preserve COND_EXPR lvalueness in fold On Apr 4, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote: > + result. We may still return other expressions that would be > + incorrect, but those are going to be rvalues, and the caller is > + supposed to discard them. */ Uhh... This sentence is no longer true. I'm removing it from my local copy of the patch. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199
[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-04
13:50 ---
Subject: Re: [Committed] PR c++/19199: Preserve COND_EXPR lvalueness in fold
On Apr 4, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote:
> On Apr 4, 2005, Roger Sayle <[EMAIL PROTECTED]> wrote:
>> long-term solutions have been proposed for g++ 4.1, the patch below is
>> the best "4.0 timeframe" work-around that doesn't adversely affect
>> performance for GCC's non C++ front-ends,
> I don't understand your claim. My patch used pedantic_lvalues, which
> is set in such a way by the C and C++ front-ends that makes it even
> faster than the string comparison you're using.
>> and even retains the ability to synthesize MIN_EXPR and MAX_EXPR for
>> C++ during the tree-ssa passes.
> This was never removed in my latest patch.
BTW, here's a patch for mainline that removes the bogus comment the
patch you checked in introduced, and adjusts the lvalueness tests such
that they're performed early, and used at any point we need to make a
decision on whether the transformation is valid. At the point you
perform the test, we may have already stripped some nop_exprs, which
might make rvalues look like lvalues, removing some optimization
possibilities.
Would you like me to add the langhook to this, or do you oppose the
entire approach for some reason you still haven't explained?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/19199
* fold-const.c (maybe_lvalue_p): Remove bogus comment introduced
in previous patch.
(fold_cond_expr_with_comparison): Determine whether lvalueness is
needed upfront, and proceed accordingly in each case.
Index: gcc/fold-const.c
===
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.554
diff -u -p -r1.554 fold-const.c
--- gcc/fold-const.c 4 Apr 2005 08:50:25 - 1.554
+++ gcc/fold-const.c 4 Apr 2005 13:42:43 -
@@ -2005,7 +2005,6 @@ fold_convert (tree type, tree arg)
/* Return false if expr can be assumed not to be an value, true
otherwise. */
-/* Return an expr equal to X but certainly not valid as an lvalue. */
static bool
maybe_lvalue_p (tree x)
@@ -4173,7 +4172,15 @@ fold_cond_expr_with_comparison (tree typ
tree arg00 = TREE_OPERAND (arg0, 0);
tree arg01 = TREE_OPERAND (arg0, 1);
tree arg1_type = TREE_TYPE (arg1);
- tree tem;
+ tree tem = NULL;
+ /* If the COND_EXPR can possibly be an lvalue, we don't want to
+ perform transformations that return a simplified result that will
+ be recognized as lvalue, but that will not match the expected
+ result. We may still return other expressions that would be
+ incorrect, but those are going to be rvalues, and the caller is
+ supposed to discard them. */
+ bool lvalue = !pedantic_lvalues
+&& maybe_lvalue_p (arg1) && maybe_lvalue_p (arg2);
STRIP_NOPS (arg1);
STRIP_NOPS (arg2);
@@ -4215,10 +4222,12 @@ fold_cond_expr_with_comparison (tree typ
case EQ_EXPR:
case UNEQ_EXPR:
tem = fold_convert (arg1_type, arg1);
- return pedantic_non_lvalue (fold_convert (type, negate_expr (tem)));
+ tem = pedantic_non_lvalue (fold_convert (type, negate_expr (tem)));
+ break;
case NE_EXPR:
case LTGT_EXPR:
- return pedantic_non_lvalue (fold_convert (type, arg1));
+ tem = pedantic_non_lvalue (fold_convert (type, arg1));
+ break;
case UNGE_EXPR:
case UNGT_EXPR:
if (flag_trapping_math)
@@ -4230,7 +4239,8 @@ fold_cond_expr_with_comparison (tree typ
arg1 = fold_convert (lang_hooks.types.signed_type
(TREE_TYPE (arg1)), arg1);
tem = fold_build1 (ABS_EXPR, TREE_TYPE (arg1), arg1);
- return pedantic_non_lvalue (fold_convert (type, tem));
+ tem = pedantic_non_lvalue (fold_convert (type, tem));
+ break;
case UNLE_EXPR:
case UNLT_EXPR:
if (flag_trapping_math)
@@ -4241,12 +4251,18 @@ fold_cond_expr_with_comparison (tree typ
arg1 = fold_convert (lang_hooks.types.signed_type
(TREE_TYPE (arg1)), arg1);
tem = fold_build1 (ABS_EXPR, TREE_TYPE (arg1), arg1);
- return negate_expr (fold_convert (type, tem));
+ tem = negate_expr (fold_convert (type, tem));
+ break;
default:
gcc_assert (TREE_CODE_CLASS (comp_code) == tcc_comparison);
break;
}
+ if (tem && (! lvalue || maybe_lvalue_p (tem)))
+return tem;
+ else
+tem = NULL;
+
/* A != 0 ? A : 0 is simply A, unless A is -0. Likewise
A == 0 ? A : 0 is always 0 unless A is -0. Note that
both transformations are correct when A is NaN: A != 0
@@ -4255,11 +4271,16 @@ fold_cond_expr_with_comparison (tree typ
if (integer_zerop (arg01) && integer_zerop (arg2))
{
if (comp_code == NE_EXPR)
- return pedantic_non_lvalue (fold_convert (typ
[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-04
13:32 ---
Subject: Re: [Committed] PR c++/19199: Preserve COND_EXPR lvalueness in fold
On Apr 4, 2005, Roger Sayle <[EMAIL PROTECTED]> wrote:
> (fold_cond_expr_with_comparison): Preserve lvalue-ness for the
> C++ front-end prior to lowering into gimple form.
> * expr2.C: Fixed.
Err... Why did you choose to drop the portion of the patch below,
that would have avoided the ugliness of comparing langhooks.name, but
still retained it in the ChangeLog entry?
Index: fold-const.c
===
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.554
diff -u -p -d -u -p -d -u -p -r1.554 fold-const.c
--- fold-const.c4 Apr 2005 08:50:25 - 1.554
+++ fold-const.c4 Apr 2005 13:25:47 -
@@ -4173,7 +4173,15 @@ fold_cond_expr_with_comparison (tree typ
tree arg00 = TREE_OPERAND (arg0, 0);
tree arg01 = TREE_OPERAND (arg0, 1);
tree arg1_type = TREE_TYPE (arg1);
- tree tem;
+ tree tem = NULL;
+ /* If the COND_EXPR can possibly be an lvalue, we don't want to
+ perform transformations that return a simplified result that will
+ be recognized as lvalue, but that will not match the expected
+ result. We may still return other expressions that would be
+ incorrect, but those are going to be rvalues, and the caller is
+ supposed to discard them. */
+ bool lvalue = !pedantic_lvalues
+&& maybe_lvalue_p (arg1) && maybe_lvalue_p (arg2);
STRIP_NOPS (arg1);
STRIP_NOPS (arg2);
@@ -4215,10 +4223,12 @@ fold_cond_expr_with_comparison (tree typ
case EQ_EXPR:
case UNEQ_EXPR:
tem = fold_convert (arg1_type, arg1);
- return pedantic_non_lvalue (fold_convert (type, negate_expr (tem)));
+ tem = pedantic_non_lvalue (fold_convert (type, negate_expr (tem)));
+ break;
case NE_EXPR:
case LTGT_EXPR:
- return pedantic_non_lvalue (fold_convert (type, arg1));
+ tem = pedantic_non_lvalue (fold_convert (type, arg1));
+ break;
case UNGE_EXPR:
case UNGT_EXPR:
if (flag_trapping_math)
@@ -4230,7 +4240,8 @@ fold_cond_expr_with_comparison (tree typ
arg1 = fold_convert (lang_hooks.types.signed_type
(TREE_TYPE (arg1)), arg1);
tem = fold_build1 (ABS_EXPR, TREE_TYPE (arg1), arg1);
- return pedantic_non_lvalue (fold_convert (type, tem));
+ tem = pedantic_non_lvalue (fold_convert (type, tem));
+ break;
case UNLE_EXPR:
case UNLT_EXPR:
if (flag_trapping_math)
@@ -4241,12 +4252,18 @@ fold_cond_expr_with_comparison (tree typ
arg1 = fold_convert (lang_hooks.types.signed_type
(TREE_TYPE (arg1)), arg1);
tem = fold_build1 (ABS_EXPR, TREE_TYPE (arg1), arg1);
- return negate_expr (fold_convert (type, tem));
+ tem = negate_expr (fold_convert (type, tem));
+ break;
default:
gcc_assert (TREE_CODE_CLASS (comp_code) == tcc_comparison);
break;
}
+ if (tem && (! lvalue || maybe_lvalue_p (tem)))
+return tem;
+ else
+tem = NULL;
+
/* A != 0 ? A : 0 is simply A, unless A is -0. Likewise
A == 0 ? A : 0 is always 0 unless A is -0. Note that
both transformations are correct when A is NaN: A != 0
@@ -4255,11 +4272,16 @@ fold_cond_expr_with_comparison (tree typ
if (integer_zerop (arg01) && integer_zerop (arg2))
{
if (comp_code == NE_EXPR)
- return pedantic_non_lvalue (fold_convert (type, arg1));
+ tem = pedantic_non_lvalue (fold_convert (type, arg1));
else if (comp_code == EQ_EXPR)
- return fold_convert (type, integer_zero_node);
+ tem = fold_convert (type, integer_zero_node);
}
+ if (tem && (! lvalue || maybe_lvalue_p (tem)))
+return tem;
+ else
+tem = NULL;
+
/* Try some transformations of A op B ? A : B.
A == B? A : Bsame as B
@@ -4309,9 +4331,15 @@ fold_cond_expr_with_comparison (tree typ
switch (comp_code)
{
case EQ_EXPR:
- return pedantic_non_lvalue (fold_convert (type, arg2));
+ if (lvalue)
+ break;
+ tem = pedantic_non_lvalue (fold_convert (type, arg2));
+ break;
case NE_EXPR:
- return pedantic_non_lvalue (fold_convert (type, arg1));
+ if (lvalue)
+ break;
+ tem = pedantic_non_lvalue (fold_convert (type, arg1));
+ break;
case LE_EXPR:
case LT_EXPR:
case UNLE_EXPR:
@@ -4327,7 +4355,7 @@ fold_cond_expr_with_comparison (tree typ
tem = (comp_code == LE_EXPR || comp_code == UNLE_EXPR)
? fold_build2 (MIN_EXPR, comp_type, comp_op0, comp_op1)
: fold_build2 (MIN_EXPR, comp_type, comp_op1, comp_op0);
- return pedantic_non_lvalue (fold_convert (type, tem));
+ tem =
[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-02 17:29 --- Subject: Re: [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary On Mar 9, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote: > PR c++/19199 > * fold-const.c (non_lvalue): Split tests into... > (maybe_lvalue_p): New function. > (fold_cond_expr_with_comparison): Preserve lvalue-ness. Ping? http://gcc.gnu.org/PR19199 I have further local changes to adjust for other changes that went in, that I'll post upon request, or when the patch goes in. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-02 17:27 --- Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable types On Mar 18, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote: > Index: gcc/ChangeLog > from Alexandre Oliva <[EMAIL PROTECTED]> > PR c++/20103 > * gimplify.c (gimplify_decl_expr): Add temp variable to binding > before gimplifying its initializer. > * c-gimplify.c (gimplify_compound_literal_expr): After replacing > the expr with a decl, we're all done. > Index: gcc/cp/ChangeLog > from Alexandre Oliva <[EMAIL PROTECTED]> > PR c++/20103 > * cp-tree.h (build_compound_literal): Declare. > * semantics.c (finish_compound_literal): Move most of the code... > * tree.c (build_compound_literal): ... here. New function. > (lvalue_p_1): Handle COMPOUND_LITERAL_EXPR. > (stabilize_init): Likewise. > * pt.c (tsubst_copy_and_build): Likewise. > * call.c (build_over_call): Likewise. > * class.c (fixed_type_or_null): Likewise. > * cp-gimplify.c (cp_gimplify_init_expr): Likewise. > (cp_gimplify_compound_literal_expr): New. > (cp_gimplify_expr): Use it. > * cvt.c (force_rvalue, ocp_convert): Handle COMPOUND_LITERAL_EXPR. > * typeck.c (build_x_unary_op): Likewise. > (cxx_mark_addressable): Likewise. > (maybe_warn_about_returning_address_of_local): Likewise. Ping? http://gcc.gnu.org/PR20103 -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20103
[Bug target/20126] [3.3/3.4/4.0/4.1 Regression] Inlined memcmp makes one argument null on entry
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-02 17:22 --- Subject: Re: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail On Mar 11, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote: > On Mar 10, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote: >> + ??? Should this should search new for new volatile MEMs and reject >> + them? */ > Here's a stricter version that does test for this. > Index: gcc/ChangeLog > from Alexandre Oliva <[EMAIL PROTECTED]> > PR target/20126 > * loop.c (loop_givs_rescan): If replacement of DEST_ADDR failed, > set the original address pseudo to the correct value before the > original insn, if possible, and leave the insn alone, otherwise > create a new pseudo, set it and replace it in the insn. > * recog.c (validate_change_maybe_volatile): New. > * recog.h (validate_change_maybe_volatile): Declare. Ping? http://gcc.gnu.org/PR20126 -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20126
[Bug tree-optimization/20640] [4.0 Regression] ICE on NULL PHI_ARG_DEF
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-02
16:59 ---
Subject: Re: [PR tree-optimization/20640] add phi args to dests of
dce-redirected edges
On Mar 31, 2005, Jeffrey A Law <[EMAIL PROTECTED]> wrote:
> On Thu, 2005-03-31 at 05:26 -0300, Alexandre Oliva wrote:
>> On Mar 31, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote:
> [ ... ]
>> Anyway, how does this patch look?
>> Index: gcc/ChangeLog
> from Alexandre Oliva <[EMAIL PROTECTED]>
> PR tree-optimization/20640
> * tree-ssa-dce.c (remove_dead_stmt): Don't redirect edge to
> post-dominator if it has phi nodes.
> (eliminate_unnecessary_stmts): Remove dead phis in all blocks
> before dead statements.
> BTW, if you want to go forward with this version, it looks OK to me
> assuming it passes bootstrapping and regression testing.
It does, so I'm checking it in with a minor change: it doesn't make
sense to request the edge to be redirected to itself, since it's a
no-op, so I moved the call into the `else' branch of the new
conditional.
Thanks,
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR tree-optimization/20640
* tree-ssa-dce.c (remove_dead_stmt): Don't redirect edge to
post-dominator if it has phi nodes.
(eliminate_unnecessary_stmts): Remove dead phis in all blocks
before dead statements.
Index: gcc/tree-ssa-dce.c
===
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-dce.c,v
retrieving revision 2.37
diff -u -p -r2.37 tree-ssa-dce.c
--- gcc/tree-ssa-dce.c 12 Mar 2005 20:53:18 - 2.37
+++ gcc/tree-ssa-dce.c 1 Apr 2005 20:30:24 -
@@ -637,7 +637,10 @@ eliminate_unnecessary_stmts (void)
{
/* Remove dead PHI nodes. */
remove_dead_phis (bb);
+}
+ FOR_EACH_BB (bb)
+{
/* Remove dead statements. */
for (i = bsi_start (bb); ! bsi_end_p (i) ; )
{
@@ -724,6 +727,7 @@ remove_dead_stmt (block_stmt_iterator *i
if (is_ctrl_stmt (t))
{
basic_block post_dom_bb;
+
/* The post dominance info has to be up-to-date. */
gcc_assert (dom_computed[CDI_POST_DOMINATORS] == DOM_OK);
/* Get the immediate post dominator of bb. */
@@ -737,9 +741,20 @@ remove_dead_stmt (block_stmt_iterator *i
return;
}
- /* Redirect the first edge out of BB to reach POST_DOM_BB. */
- redirect_edge_and_branch (EDGE_SUCC (bb, 0), post_dom_bb);
- PENDING_STMT (EDGE_SUCC (bb, 0)) = NULL;
+ /* If the post dominator block has PHI nodes, we might be unable
+to compute the right PHI args for them. Since the control
+statement is unnecessary, all edges can be regarded as
+equivalent, but we have to get rid of the condition, since it
+might reference a variable that was determined to be
+unnecessary and thus removed. */
+ if (phi_nodes (post_dom_bb))
+ post_dom_bb = EDGE_SUCC (bb, 0)->dest;
+ else
+ {
+ /* Redirect the first edge out of BB to reach POST_DOM_BB. */
+ redirect_edge_and_branch (EDGE_SUCC (bb, 0), post_dom_bb);
+ PENDING_STMT (EDGE_SUCC (bb, 0)) = NULL;
+ }
EDGE_SUCC (bb, 0)->probability = REG_BR_PROB_BASE;
EDGE_SUCC (bb, 0)->count = bb->count;
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR tree-optimization/20640
* gcc.dg/torture/tree-loop-1.c: New.
Index: gcc/testsuite/gcc.dg/torture/tree-loop-1.c
===
RCS file: gcc/testsuite/gcc.dg/torture/tree-loop-1.c
diff -N gcc/testsuite/gcc.dg/torture/tree-loop-1.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/gcc.dg/torture/tree-loop-1.c 30 Mar 2005 05:28:22 -
@@ -0,0 +1,21 @@
+/* PR tree-optimization/20640 */
+
+/* After unrolling the loop, we'd turn some conditional branches into
+ unconditional ones, but branch redirection would fail to compute
+ the PHI args for the PHI nodes in the replacement edge
+ destination, so they'd remain NULL causing crashes later on. */
+
+/* { dg-do compile } */
+
+static int a = 0;
+extern int foo (void);
+extern int *bar (void) __attribute__ ((__const__));
+
+void
+test (int x)
+{
+ int b = 10;
+ while (foo () == -1 && *bar () == 4 && b > 0)
+--b;
+ a = x;
+}
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20640
[Bug middle-end/20491] [4.0/4.1 Regression] internal compiler error: in subreg_regno_offset, at rtlanal.c:3042
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-02
16:57 ---
Subject: Re: [PR middle-end/20491] combine generates bad subregs
On Mar 31, 2005, Richard Henderson <[EMAIL PROTECTED]> wrote:
> On Wed, Mar 30, 2005 at 04:27:50PM -0300, Alexandre Oliva wrote:
>> - else
>> + else if (REG_P (y))
>> {
>> /* Simplify_subreg can't handle some REG cases, but we have to. */
>> unsigned int regno = subreg_regno (x);
> The next line is
> gcc_assert (REG_P (y));
> you should remove that. Ok with that change.
Thanks, here's what I'm checking in.
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR middle-end/20491
* final.c (alter_subreg): Don't call subreg_regno for a non-REG.
Index: gcc/final.c
===
RCS file: /cvs/gcc/gcc/gcc/final.c,v
retrieving revision 1.349
diff -u -p -r1.349 final.c
--- gcc/final.c 1 Apr 2005 15:27:58 - 1.349
+++ gcc/final.c 1 Apr 2005 20:19:02 -
@@ -2547,11 +2547,10 @@ alter_subreg (rtx *xp)
if (new != 0)
*xp = new;
- else
+ else if (REG_P (y))
{
/* Simplify_subreg can't handle some REG cases, but we have to. */
unsigned int regno = subreg_regno (x);
- gcc_assert (REG_P (y));
*xp = gen_rtx_REG_offset (y, GET_MODE (x), regno, SUBREG_BYTE (x));
}
}
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20491
[Bug debug/19345] [4.0/4.1 Regression] Segmentation fault with VLA and inlining and dwarf2
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-04-01 00:37 --- Subject: Re: [PR debug/19345] remap TYPE_STUB_DECL during inlining On Mar 31, 2005, Daniel Berlin <[EMAIL PROTECTED]> wrote: > Did i check it in, or someone else? You did, along with the patch mentioned in the ChangeLog. > If i did it, it was definitely by accident. Thanks for the confirmation. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19345
[Bug debug/19345] [4.0/4.1 Regression] Segmentation fault with VLA and inlining and dwarf2
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-31
20:47 ---
Subject: [PR debug/19345] remap TYPE_STUB_DECL during inlining
TYPE_STUB_DECL was NULL in the testcase given in the bug report
because tree inlining failed to remap TYPE_STUB_DECL. This patch
reverts the patch that hides the problem, that AFAICT was checked in
by accident, and installs a proper (IMHO :-) fix. Bootstrapping on
amd64-linux-gnu. Ok to install if it passes regtesting?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR debug/19345
* dwarf2out.c (add_abstract_origin_attribute): Revert accidental?
check in from 2005-03-03 for debug/20253.
* tree-inline.c (remap_type): Remap TYPE_STUB_DECL.
(remap_decl): Insert type decl in map earlier.
Index: gcc/dwarf2out.c
===
RCS file: /cvs/gcc/gcc/gcc/dwarf2out.c,v
retrieving revision 1.576
diff -u -p -r1.576 dwarf2out.c
--- gcc/dwarf2out.c 30 Mar 2005 23:08:17 - 1.576
+++ gcc/dwarf2out.c 31 Mar 2005 20:43:20 -
@@ -10465,11 +10465,7 @@ add_abstract_origin_attribute (dw_die_re
if (TYPE_P (fn))
fn = TYPE_STUB_DECL (fn);
- /* TYPE_STUB_DECL may have given us a NULL, which decl_function_context
-won't like. */
- if (fn)
- fn = decl_function_context (fn);
-
+ fn = decl_function_context (fn);
if (fn)
dwarf2out_abstract_function (fn);
}
Index: gcc/tree-inline.c
===
RCS file: /cvs/gcc/gcc/gcc/tree-inline.c,v
retrieving revision 1.175
diff -u -p -r1.175 tree-inline.c
--- gcc/tree-inline.c 30 Mar 2005 21:34:27 - 1.175
+++ gcc/tree-inline.c 31 Mar 2005 20:43:22 -
@@ -172,6 +172,11 @@ remap_decl (tree decl, inline_data *id)
/* Make a copy of the variable or label. */
tree t = copy_decl_for_inlining (decl, fn, VARRAY_TREE (id->fns, 0));
+ /* Remember it, so that if we encounter this local entity again
+we can reuse this copy. Do this early becuase remap_type may
+need this decl for TYPE_STUB_DECL. */
+ insert_decl_map (id, decl, t);
+
/* Remap types, if necessary. */
TREE_TYPE (t) = remap_type (TREE_TYPE (t), id);
if (TREE_CODE (t) == TYPE_DECL)
@@ -214,9 +219,6 @@ remap_decl (tree decl, inline_data *id)
}
#endif
- /* Remember it, so that if we encounter this local entity
-again we can reuse this copy. */
- insert_decl_map (id, decl, t);
return t;
}
@@ -285,6 +287,9 @@ remap_type (tree type, inline_data *id)
TYPE_NEXT_VARIANT (new) = NULL;
}
+ if (TYPE_STUB_DECL (type))
+TYPE_STUB_DECL (new) = remap_decl (TYPE_STUB_DECL (type), id);
+
/* Lazily create pointer and reference types. */
TYPE_POINTER_TO (new) = NULL;
TYPE_REFERENCE_TO (new) = NULL;
Index: gcc/testsuite/ChangeLog
from Daniel Berlin <[EMAIL PROTECTED]>
* gcc.dg/pr19345.c: New test.
Index: gcc/testsuite/gcc.dg/pr19345.c
===
RCS file: gcc/testsuite/gcc.dg/pr19345.c
diff -N gcc/testsuite/gcc.dg/pr19345.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/gcc.dg/pr19345.c 31 Mar 2005 20:43:37 -
@@ -0,0 +1,12 @@
+/* We shouldn't crash trying to produce the inlined structure type die debug
info. */
+/* { dg-do compile } */
+/* { dg-options "-O1 -g" } */
+inline void bar(char a[], unsigned int l)
+{
+ asm volatile ("" :: "m" ( *(struct {char x[l]; } *)a));
+}
+
+void foo(void)
+{
+ bar (0, 0);
+}
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19345
[Bug tree-optimization/20640] [4.0 Regression] ICE on NULL PHI_ARG_DEF
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-31
08:41 ---
Subject: Re: [PR tree-optimization/20640] add phi args to dests of
dce-redirected edges
On Mar 30, 2005, Jeffrey A Law <[EMAIL PROTECTED]> wrote:
> - PENDING_STMT (EDGE_SUCC (bb, 0)) = NULL;
> + flush_pending_stmts (EDGE_SUCC (bb, 0));
> I'm having trouble seeing how this can be correct.
After looking at what flush_pending_stmts() actually does, I must
confess I'm disappointed. I expected it to be far smarter than it
actually is.
Here's a newer version of the patch that survived bootstrap on
x86_64-linux-gnu, with default BOOT_CFLAGS in mainline, and with
BOOT_CFLAGS='-O3 -g -funroll-all-loops' in the 4.0 branch. I found
that the 4.0 branch would fail to bootstrap even with default
BOOT_CFLAGS if I added code to flush_pending_stmts() to verify that
the PHI args actually matched the corresponding PHI nodes.
My concern is that, if the code in this patch fails and we reach the
hopefully-unreachable point, we won't be able to obtain a PHI arg very
easily. I have a gut feeling that we'll always get a PHI arg from one
of the previous successors of the src of the redirected edge, but I
can't quite prove it. What do you think?
> This code is triggered rarely, I would expect it to be even rarer still
> for POST_DOM_BB to have PHI nodes. You could probably just ignore dead
> control statements where the post dominator has PHI nodes and I doubt
> it would ever be noticed.
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR tree-optimization/20640
* tree-ssa-dce.c (remove_dead_stmt): Add phi args to phi nodes
affected by an edge redirection.
Index: gcc/tree-ssa-dce.c
===
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-dce.c,v
retrieving revision 2.37
diff -u -p -r2.37 tree-ssa-dce.c
--- gcc/tree-ssa-dce.c 12 Mar 2005 20:53:18 - 2.37
+++ gcc/tree-ssa-dce.c 31 Mar 2005 06:39:48 -
@@ -724,6 +724,10 @@ remove_dead_stmt (block_stmt_iterator *i
if (is_ctrl_stmt (t))
{
basic_block post_dom_bb;
+ edge e;
+ tree phi;
+ tree pending_stmts;
+
/* The post dominance info has to be up-to-date. */
gcc_assert (dom_computed[CDI_POST_DOMINATORS] == DOM_OK);
/* Get the immediate post dominator of bb. */
@@ -739,7 +743,13 @@ remove_dead_stmt (block_stmt_iterator *i
/* Redirect the first edge out of BB to reach POST_DOM_BB. */
redirect_edge_and_branch (EDGE_SUCC (bb, 0), post_dom_bb);
+
+ e = EDGE_SUCC (bb, 0);
+
+ pending_stmts = PENDING_STMT (e);
+
PENDING_STMT (EDGE_SUCC (bb, 0)) = NULL;
+
EDGE_SUCC (bb, 0)->probability = REG_BR_PROB_BASE;
EDGE_SUCC (bb, 0)->count = bb->count;
@@ -755,6 +765,76 @@ remove_dead_stmt (block_stmt_iterator *i
else
EDGE_SUCC (bb, 0)->flags &= ~EDGE_FALLTHRU;
+ /* Now adjust the PHI args for the new edge in the new dest. */
+ for (phi = phi_nodes (e->dest);
+ phi;
+ phi = PHI_CHAIN (phi))
+ {
+ tree arg = NULL;
+ tree *pendp = &pending_stmts;
+ tree phi1;
+ edge e1;
+ edge_iterator ei;
+
+ /* If it's already set for a variable, we're done! */
+ if (PHI_ARG_DEF (phi, e->dest_idx))
+ continue;
+
+ /* Try to locate a value in the list of PHI args collected
+while removing the old edge. */
+ while (*pendp)
+ {
+ if (SSA_NAME_VAR (PHI_RESULT (phi))
+ == SSA_NAME_VAR (TREE_PURPOSE (*pendp)))
+ {
+ arg = TREE_VALUE (*pendp);
+ *pendp = TREE_CHAIN (*pendp);
+ break;
+ }
+ pendp = &TREE_CHAIN (*pendp);
+ }
+
+ if (arg)
+ {
+ add_phi_arg (phi, arg, e);
+ continue;
+ }
+
+ /* As a last resort, we can try to find ssa args in the
+other outgoing edges of the src block. */
+ FOR_EACH_EDGE (e1, ei, bb->succs)
+ {
+ if (e1 == e)
+ continue;
+
+ for (phi1 = phi_nodes (e1->dest); phi1;
+ phi1 = PHI_CHAIN (phi1))
+ {
+ if (SSA_NAME_VAR (PHI_RESULT (phi1))
+ == SSA_NAME_VAR (PHI_RESULT (phi)))
+ {
+ arg = PHI_ARG_DEF (phi1, e1->dest_idx);
+ break;
+ }
+ }
+
+ if (arg)
+ break;
+ }
+
+ if (arg)
+ {
+ add_phi_arg (phi, arg, e);
+ continue;
+ }
+
+ /* There's a slight possibility that we won't have been able
+to find a PHI arg in any of the previously-existing
+outgoing edges. Should this ever happen, we'd have to
+scan
[Bug tree-optimization/20640] [4.0 Regression] ICE on NULL PHI_ARG_DEF
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-31
08:28 ---
Subject: Re: [PR tree-optimization/20640] add phi args to dests of
dce-redirected edges
On Mar 31, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote:
> I have a gut feeling that we'll always get a PHI arg from one of the
> previous successors of the src of the redirected edge, but I can't
> quite prove it. What do you think?
A few seconds after posting the patch, ssa-dce-3.c failed, showing my
gut feeling was wrong. Oh well...
On Mar 30, 2005, Jeffrey A Law <[EMAIL PROTECTED]> wrote:
>> This code is triggered rarely, I would expect it to be even rarer still
>> for POST_DOM_BB to have PHI nodes. You could probably just ignore dead
>> control statements where the post dominator has PHI nodes and I doubt
>> it would ever be noticed.
It is noticed if we take the same path as the no-post_dom_bb,
infinite-loop case, because then the instruction that remains may
still reference variables that were deleted.
This patch, however, arranges for us to turn the edge into a
fall-through edge to its original destination should the immediate
post dominator be found to have PHI nodes.
I've also tweaked the code so as to remove all dead phi nodes before
removing all other statements, thereby improving the odds of
redirecting a dead control stmt to the post dominator.
Interestingly, the resulting code was no different for some cases I
inspected (the testcase included in the patch and ssa-dce-3.c).
That's because the edge dest bb ends up becoming a simple forwarding
block that is later removed.
As for infinite loops, I'm wondering if we should somehow try to
remove the condition from the loop. If the conditional branch is
found to be dead, it's quite possible that the set of that variable is
dead as well, and so we'd be keeping a reference to a deleted
variable. I couldn't actually exercise such an error, and I'm not
convinced it's possible, but I thought I'd bring this up. Thoughts?
Anyway, how does this patch look?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR tree-optimization/20640
* tree-ssa-dce.c (remove_dead_stmt): Don't redirect edge to
post-dominator if it has phi nodes.
(eliminate_unnecessary_stmts): Remove dead phis in all blocks
before dead statements.
Index: gcc/tree-ssa-dce.c
===
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-dce.c,v
retrieving revision 2.37
diff -u -p -r2.37 tree-ssa-dce.c
--- gcc/tree-ssa-dce.c 12 Mar 2005 20:53:18 - 2.37
+++ gcc/tree-ssa-dce.c 31 Mar 2005 07:56:42 -
@@ -637,7 +637,10 @@ eliminate_unnecessary_stmts (void)
{
/* Remove dead PHI nodes. */
remove_dead_phis (bb);
+}
+ FOR_EACH_BB (bb)
+{
/* Remove dead statements. */
for (i = bsi_start (bb); ! bsi_end_p (i) ; )
{
@@ -724,6 +727,7 @@ remove_dead_stmt (block_stmt_iterator *i
if (is_ctrl_stmt (t))
{
basic_block post_dom_bb;
+
/* The post dominance info has to be up-to-date. */
gcc_assert (dom_computed[CDI_POST_DOMINATORS] == DOM_OK);
/* Get the immediate post dominator of bb. */
@@ -737,6 +741,15 @@ remove_dead_stmt (block_stmt_iterator *i
return;
}
+ /* If the post dominator block has PHI nodes, we might be unable
+to compute the right PHI args for them. Since the control
+statement is unnecessary, all edges can be regarded as
+equivalent, but we have to get rid of the condition, since it
+might reference a variable that was determined to be
+unnecessary and thus removed. */
+ if (phi_nodes (post_dom_bb))
+ post_dom_bb = EDGE_SUCC (bb, 0)->dest;
+
/* Redirect the first edge out of BB to reach POST_DOM_BB. */
redirect_edge_and_branch (EDGE_SUCC (bb, 0), post_dom_bb);
PENDING_STMT (EDGE_SUCC (bb, 0)) = NULL;
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR tree-optimization/20640
* gcc.dg/torture/tree-loop-1.c: New.
Index: gcc/testsuite/gcc.dg/torture/tree-loop-1.c
===
RCS file: gcc/testsuite/gcc.dg/torture/tree-loop-1.c
diff -N gcc/testsuite/gcc.dg/torture/tree-loop-1.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/gcc.dg/torture/tree-loop-1.c 30 Mar 2005 05:28:22 -
@@ -0,0 +1,21 @@
+/* PR tree-optimization/20640 */
+
+/* After unrolling the loop, we'd turn some conditional branches into
+ unconditional ones, but branch redirection would fail to compute
+ the PHI args for the PHI nodes in the replacement edge
+ destination, so they'd remain NULL causing crashes later on. */
+
+/* { dg-do compile } */
+
+static int a = 0;
+extern int foo (void);
+extern int *bar (void) __attribute__ ((__const__));
+
+void
+test (int x)
+{
+ int b = 10;
+ while (foo () == -1 && *bar () == 4 && b > 0)
+
[Bug middle-end/20491] [4.0/4.1 Regression] internal compiler error: in subreg_regno_offset, at rtlanal.c:3042
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-30
19:28 ---
Subject: Re: [PR middle-end/20491] combine generates bad subregs
On Mar 29, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote:
> On Mar 28, 2005, Richard Henderson <[EMAIL PROTECTED]> wrote:
>> On Thu, Mar 24, 2005 at 07:45:44AM -0300, Alexandre Oliva wrote:
>>> * combine.c (subst): Make sure we don't create invalid subregs.
>> Ok.
> As it turned out, the bug seems to have been fixed by some other
> patch, because I no longer get the error in mainline or in the 4.0
> branch.
Or perhaps it never actually failed on x86_64-linux-gnu, which is
where I was trying to trigger it again. Not even with -m32. A build
on i686-pc-linux-gnu was enough to trigger it. Yay me! :-)
So, the patch posted in my previous e-mail still stands, but if you,
like me, find that it's working too hard to avoid such subregs, here's
an alternative patch that also fixes the problem, at least as long as
the X-constrained asm operand is not actually used in the asm output
pattern.
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR middle-end/20491
* final.c (alter_subreg): Don't call subreg_regno for a non-REG.
Index: gcc/final.c
===
RCS file: /cvs/gcc/gcc/gcc/final.c,v
retrieving revision 1.344.12.1
diff -u -p -r1.344.12.1 final.c
--- gcc/final.c 22 Mar 2005 13:39:12 - 1.344.12.1
+++ gcc/final.c 30 Mar 2005 19:21:02 -
@@ -1,6 +1,7 @@
/* Convert RTL to assembler code and output it, for GNU compiler.
Copyright (C) 1987, 1988, 1989, 1992, 1993, 1994, 1995, 1996, 1997,
- 1998, 1999, 2000, 2001, 2002, 2003, 2004 Free Software Foundation, Inc.
+ 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005
+ Free Software Foundation, Inc.
This file is part of GCC.
@@ -2638,7 +2639,7 @@ alter_subreg (rtx *xp)
if (new != 0)
*xp = new;
- else
+ else if (REG_P (y))
{
/* Simplify_subreg can't handle some REG cases, but we have to. */
unsigned int regno = subreg_regno (x);
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20491
[Bug fortran/20460] Nasty extensions that should always warn
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-30
05:56 ---
Subject: [PR tree-optimization/20460] add phi args to dests of dce-redirected
edges
When remove_dead_stmt() redirects a control stmt, the edge redirection
reserves space for the phi arg for the new incoming edge in all phi
nodes, but, instead of filling them in with information obtained from
the edge redirection, we simply discard this information. This leaves
NULL in the phi args, which may cause crashes later on.
This patch fixes the problem by filling in the phi args using the
PENDING_STMT list created during edge redirection. This appears to be
the intended use for this information, and it is used similarly in
e.g. loop unrolling.
Bootstrapping mainline and 4.0 branch on amd64-linux-gnu, and mainline
on i686-pc-linux-gnu. Ok to install if bootstrap and regtesting pass?
The patch below is for the 4.0 branch, but it applies cleanly and
correctly in mainline as well, since it's just a few lines off.
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR tree-optimization/20460
* tree-ssa-dce.c (remove_dead_stmt): Add phi args to phi nodes
affected by an edge redirection.
Index: gcc/tree-ssa-dce.c
===
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-dce.c,v
retrieving revision 2.32
diff -u -p -r2.32 tree-ssa-dce.c
--- gcc/tree-ssa-dce.c 17 Feb 2005 16:19:44 - 2.32
+++ gcc/tree-ssa-dce.c 30 Mar 2005 05:28:09 -
@@ -810,7 +810,7 @@ remove_dead_stmt (block_stmt_iterator *i
/* Redirect the first edge out of BB to reach POST_DOM_BB. */
redirect_edge_and_branch (EDGE_SUCC (bb, 0), post_dom_bb);
- PENDING_STMT (EDGE_SUCC (bb, 0)) = NULL;
+ flush_pending_stmts (EDGE_SUCC (bb, 0));
EDGE_SUCC (bb, 0)->probability = REG_BR_PROB_BASE;
EDGE_SUCC (bb, 0)->count = bb->count;
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR tree-optimization/20640
* gcc.dg/torture/tree-loop-1.c: New.
Index: gcc/testsuite/gcc.dg/torture/tree-loop-1.c
===
RCS file: gcc/testsuite/gcc.dg/torture/tree-loop-1.c
diff -N gcc/testsuite/gcc.dg/torture/tree-loop-1.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/gcc.dg/torture/tree-loop-1.c 30 Mar 2005 05:28:22 -
@@ -0,0 +1,21 @@
+/* PR tree-optimization/20640 */
+
+/* After unrolling the loop, we'd turn some conditional branches into
+ unconditional ones, but branch redirection would fail to compute
+ the PHI args for the PHI nodes in the replacement edge
+ destination, so they'd remain NULL causing crashes later on. */
+
+/* { dg-do compile } */
+
+static int a = 0;
+extern int foo (void);
+extern int *bar (void) __attribute__ ((__const__));
+
+void
+test (int x)
+{
+ int b = 10;
+ while (foo () == -1 && *bar () == 4 && b > 0)
+--b;
+ a = x;
+}
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20460
[Bug middle-end/20491] [4.0/4.1 Regression] internal compiler error: in subreg_regno_offset, at rtlanal.c:3042
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-29
21:48 ---
Subject: Re: [PR middle-end/20491] combine generates bad subregs
On Mar 28, 2005, Richard Henderson <[EMAIL PROTECTED]> wrote:
> On Thu, Mar 24, 2005 at 07:45:44AM -0300, Alexandre Oliva wrote:
>> * combine.c (subst): Make sure we don't create invalid subregs.
> Ok.
As it turned out, the bug seems to have been fixed by some other
patch, because I no longer get the error in mainline or in the 4.0
branch. Since my patch actually introduced a regression on
i686-pc-linux-gnu: gcc.dg/i386-rotate-1.c. Because of the ruled-out
combines, we prevented the complete transformation from taking place.
I actually wrote another, more complex patch, that fixed it, by using
the same machinery used by later portions of combine to push the
invalid subreg into the shift operands, i.e., turn (subreg (ashift
(reg) (const_int))) into (ashift (subreg (reg)) (const_int)), but then
I decided I was working too hard and figured I might be better off
fixing the problem elsewhere. To my surprise, after reverting the
patch I had, the problem no longer occurred. I tried some CVS
searching, but couldn't locate the patch that fixed the problem. In
fact, I couldn't even find a relatively-recent tree that triggered the
bug, although I'm pretty sure the tree in which I triggered the bug
was no older than some date early last week. Oh well...
I'm attaching the patch I had, in case you still think we should avoid
creating such invalid subregs, followed by the testcase patch I
installed in the 4.0 branch in mainline.
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR middle-end/20491
* combine.c (subst): Make sure we don't create invalid subregs.
Index: gcc/combine.c
===
RCS file: /cvs/gcc/gcc/gcc/combine.c,v
retrieving revision 1.484
diff -u -p -r1.484 combine.c
--- gcc/combine.c 22 Mar 2005 03:48:44 - 1.484
+++ combine.c 25 Mar 2005 21:21:40 -
@@ -3689,15 +3689,36 @@ subst (rtx x, rtx from, rtx to, int in_d
if (GET_CODE (new) == CLOBBER && XEXP (new, 0) == const0_rtx)
return new;
+ /* If we changed the reg of a subreg, make sure it's
+still valid. For anything but a REG, require the
+SUBREG to be simplified out. */
+
if (GET_CODE (x) == SUBREG
- && (GET_CODE (new) == CONST_INT
- || GET_CODE (new) == CONST_DOUBLE))
+ && ! rtx_equal_p (new, SUBREG_REG (x)))
{
enum machine_mode mode = GET_MODE (x);
+ rtx orig = x;
+
+ x = simplify_gen_subreg (mode, new, op0_mode,
+ SUBREG_BYTE (orig));
+
+ /* This will propagate the subreg into the operand,
+if appropriate. */
+ if (GET_CODE (x) == SUBREG
+ && GET_CODE (SUBREG_REG (x)) != REG)
+ x = force_to_mode (x, mode, GET_MODE_MASK (mode),
+ NULL_RTX, 0);
+
+ /* Now make sure we didn't create a subreg of
+something that is not a reg. */
+ if (GET_CODE (x) == SUBREG
+ && GET_CODE (SUBREG_REG (x)) != REG)
+ x = simplify_subreg (mode, SUBREG_REG (x),
+GET_MODE (SUBREG_REG (x)),
+SUBREG_BYTE (x));
+
+ op0_mode = VOIDmode;
- x = simplify_subreg (GET_MODE (x), new,
- GET_MODE (SUBREG_REG (x)),
- SUBREG_BYTE (x));
if (! x)
x = gen_rtx_CLOBBER (mode, const0_rtx);
}
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR middle-end/20491
* gcc.dg/torture/asm-subreg-1.c: New test.
Index: gcc/testsuite/gcc.dg/torture/asm-subreg-1.c
===
RCS file: gcc/testsuite/gcc.dg/torture/asm-subreg-1.c
diff -N gcc/testsuite/gcc.dg/torture/asm-subreg-1.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/gcc.dg/torture/asm-subreg-1.c 29 Mar 2005 21:31:55 -
@@ -0,0 +1,14 @@
+/* PR middle-end/20491 */
+
+/* { dg-do compile } */
+
+/* Combine used to introduce invalid subregs for the asm input, and
+ we'd crash later on, when removing all subregs. */
+
+volatile unsigned short _const_32 [4] = {1,2,3,4};
+void
+evas_common_convert_yuv_420p_601_rgba()
+{
+ __asm__ __volatile__ ("" : : "X" (*_const_32));
+}
+
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bug
[Bug middle-end/20491] [4.0/4.1 Regression] internal compiler error: in subreg_regno_offset, at rtlanal.c:3042
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-25 13:41 --- Subject: Re: [PR middle-end/20491] combine generates bad subregs On Mar 24, 2005, [EMAIL PROTECTED] (Richard Kenner) wrote: > Combine doesn't ensure the subregs it generates are valid. In most > cases, insn recog will reject the invalid subregs, or reload will > some how make them fit, but if the constraint of the insn or the asm > operand is "X", this won't work, so I think we're better off ensuring > we don't ever introduce subregs of non-REGs. > Aside from calling recog combine doesn't check that *anything* it generates > is valid. Why should subregs be different? Because, as I said, generating an invalid subreg at that point, without any opportunity to fix it up later, will cause us to crash when the time comes to remove all subregs. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20491
[Bug middle-end/20491] [4.0/4.1 Regression] internal compiler error: in subreg_regno_offset, at rtlanal.c:3042
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-24
10:46 ---
Subject: Re: [PR middle-end/20491] combine generates bad subregs
On Mar 24, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote:
> Combine doesn't ensure the subregs it generates are valid. In most
> cases, insn recog will reject the invalid subregs, or reload will
> somehow make them fit, but if the constraint of the insn or the asm
> operand is "X", this won't work, so I think we're better off ensuring
> we don't ever introduce subregs of non-REGs.
> Does this look like a reasonable change to make?
Ugh. It failed building libgcc for stage 1, after simplifying
(subreg:QI (subreg:SI (reg:HI))) to (subreg:QI (reg:HI)), leaving
op0_mode set to SImode, causing an error in combine_simplify_rtx(),
when it called simplify_subreg with the wrong mode.
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR middle-end/20491
* combine.c (subst): Make sure we don't create invalid subregs.
Index: gcc/combine.c
===
RCS file: /cvs/gcc/gcc/gcc/combine.c,v
retrieving revision 1.484
diff -u -p -r1.484 combine.c
--- gcc/combine.c 22 Mar 2005 03:48:44 - 1.484
+++ gcc/combine.c 24 Mar 2005 10:01:17 -
@@ -3689,15 +3689,24 @@ subst (rtx x, rtx from, rtx to, int in_d
if (GET_CODE (new) == CLOBBER && XEXP (new, 0) == const0_rtx)
return new;
- if (GET_CODE (x) == SUBREG
- && (GET_CODE (new) == CONST_INT
- || GET_CODE (new) == CONST_DOUBLE))
+ /* If we changed the reg of a subreg, make sure it's
+still valid. For anything but a REG, require the
+SUBREG to be simplified out. */
+
+ if (GET_CODE (x) == SUBREG && new != SUBREG_REG (x))
{
enum machine_mode mode = GET_MODE (x);
+ enum machine_mode submode = op0_mode;
+
+ if (GET_CODE (new) == REG)
+ x = simplify_gen_subreg (mode, new, submode,
+SUBREG_BYTE (x));
+ else
+ x = simplify_subreg (mode, new, submode,
+SUBREG_BYTE (x));
+
+ op0_mode = VOIDmode;
- x = simplify_subreg (GET_MODE (x), new,
- GET_MODE (SUBREG_REG (x)),
- SUBREG_BYTE (x));
if (! x)
x = gen_rtx_CLOBBER (mode, const0_rtx);
}
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR middle-end/20491
* gcc.dg/asm-subreg-1.c: New.
Index: gcc/testsuite/gcc.dg/asm-subreg-1.c
===
RCS file: gcc/testsuite/gcc.dg/asm-subreg-1.c
diff -N gcc/testsuite/gcc.dg/asm-subreg-1.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/gcc.dg/asm-subreg-1.c 24 Mar 2005 09:10:14 -
@@ -0,0 +1,14 @@
+/* PR middle-end/20491 */
+
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+/* Combine used to introduce invalid subregs for the asm input. */
+
+volatile unsigned short _const_32 [4] = {1,2,3,4};
+void
+evas_common_convert_yuv_420p_601_rgba()
+{
+ __asm__ __volatile__ ("" : : "X" (*_const_32));
+}
+
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20491
[Bug middle-end/20491] [4.0/4.1 Regression] internal compiler error: in subreg_regno_offset, at rtlanal.c:3042
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-24
09:25 ---
Subject: [PR middle-end/20491] combine generates bad subregs
Combine doesn't ensure the subregs it generates are valid. In most
cases, insn recog will reject the invalid subregs, or reload will
somehow make them fit, but if the constraint of the insn or the asm
operand is "X", this won't work, so I think we're better off ensuring
we don't ever introduce subregs of non-REGs.
Does this look like a reasonable change to make? If so, ok to install
if bootstrap and regtest on amd64-linux-gnu and i686-pc-linux-gnu
succeed?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR middle-end/20491
* combine.c (subst): Make sure we don't create invalid subregs.
Index: gcc/combine.c
===
RCS file: /cvs/gcc/gcc/gcc/combine.c,v
retrieving revision 1.484
diff -u -p -r1.484 combine.c
--- gcc/combine.c 22 Mar 2005 03:48:44 - 1.484
+++ gcc/combine.c 24 Mar 2005 09:10:00 -
@@ -3689,15 +3689,22 @@ subst (rtx x, rtx from, rtx to, int in_d
if (GET_CODE (new) == CLOBBER && XEXP (new, 0) == const0_rtx)
return new;
- if (GET_CODE (x) == SUBREG
- && (GET_CODE (new) == CONST_INT
- || GET_CODE (new) == CONST_DOUBLE))
+ /* If we changed the reg of a subreg, make sure it's
+still valid. For anything but a REG, require the
+SUBREG to be simplified out. */
+
+ if (GET_CODE (x) == SUBREG && new != SUBREG_REG (x))
{
enum machine_mode mode = GET_MODE (x);
+ enum machine_mode submode = GET_MODE (SUBREG_REG (x));
+
+ if (GET_CODE (new) == REG)
+ x = simplify_gen_subreg (mode, new, submode,
+SUBREG_BYTE (x));
+ else
+ x = simplify_subreg (mode, new, submode,
+SUBREG_BYTE (x));
- x = simplify_subreg (GET_MODE (x), new,
- GET_MODE (SUBREG_REG (x)),
- SUBREG_BYTE (x));
if (! x)
x = gen_rtx_CLOBBER (mode, const0_rtx);
}
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR middle-end/20491
* gcc.dg/asm-subreg-1.c: New.
Index: gcc/testsuite/gcc.dg/asm-subreg-1.c
===
RCS file: gcc/testsuite/gcc.dg/asm-subreg-1.c
diff -N gcc/testsuite/gcc.dg/asm-subreg-1.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/gcc.dg/asm-subreg-1.c 24 Mar 2005 09:10:14 -
@@ -0,0 +1,14 @@
+/* PR middle-end/20491 */
+
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+/* Combine used to introduce invalid subregs for the asm input. */
+
+volatile unsigned short _const_32 [4] = {1,2,3,4};
+void
+evas_common_convert_yuv_420p_601_rgba()
+{
+ __asm__ __volatile__ ("" : : "X" (*_const_32));
+}
+
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20491
[Bug rtl-optimization/20532] [4.0/4.1 Regression] Bad code for DImode left shifts by 31 and then 1
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-23
02:41 ---
Subject: [PR rtl-optimization/20532] plus(ashift,ashift) -> mult may overflow
In the sample testcase, if HOST_WIDE_INT is 32-bits wide, we ended up
trying to simplify the two shifts into a single multiply. The shift
by one was already represented as an add-to-itself. In combine, we
turned the shifts by 31 substed into both operands of the plus into
multiply by ((HOST_WIDE_INT)1 << 31), and then added the two
coefficients, resulting zero. Oops.
This patch arranges for us to represent coefficients as CONST_DOUBLEs
when needed, avoiding overflows in all cases, since the two
coefficients added are in the range
[HOST_WIDE_INT_MIN,2*HOST_WIDE_INT_MAX+1].
In order to generate the optimal code that we generate on a 64-bit
HOST_WIDE_INT host, I had to also get multiply simplification to apply
to CONST_DOUBLE exact log2 constants, such as those produced after the
patch.
Bootstrapping on amd64-linux-gnu. Ok to install if it passes
regtesting?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR rtl-optimization/20532
* simplify-rtx.c (simplify_binary_operation_1): Protect from
overflow when adding coefficients for PLUS or MINUS.
(simplify_binary_operation_1): Handle CONST_DOUBLE exact power of
two as multiplier.
Index: gcc/simplify-rtx.c
===
RCS file: /cvs/gcc/gcc/gcc/simplify-rtx.c,v
retrieving revision 1.234
diff -u -p -r1.234 simplify-rtx.c
--- gcc/simplify-rtx.c 21 Mar 2005 14:30:51 - 1.234
+++ gcc/simplify-rtx.c 23 Mar 2005 02:30:15 -
@@ -1257,44 +1257,67 @@ simplify_binary_operation_1 (enum rtx_co
if (! FLOAT_MODE_P (mode))
{
- HOST_WIDE_INT coeff0 = 1, coeff1 = 1;
+ HOST_WIDE_INT coeff0h = 0, coeff1h = 0;
+ unsigned HOST_WIDE_INT coeff0l = 1, coeff1l = 1;
rtx lhs = op0, rhs = op1;
if (GET_CODE (lhs) == NEG)
- coeff0 = -1, lhs = XEXP (lhs, 0);
+ {
+ coeff0l = -1;
+ coeff0h = -1;
+ lhs = XEXP (lhs, 0);
+ }
else if (GET_CODE (lhs) == MULT
&& GET_CODE (XEXP (lhs, 1)) == CONST_INT)
- coeff0 = INTVAL (XEXP (lhs, 1)), lhs = XEXP (lhs, 0);
+ {
+ coeff0l = INTVAL (XEXP (lhs, 1));
+ coeff0h = INTVAL (XEXP (lhs, 1)) < 0 ? -1 : 0;
+ lhs = XEXP (lhs, 0);
+ }
else if (GET_CODE (lhs) == ASHIFT
&& GET_CODE (XEXP (lhs, 1)) == CONST_INT
&& INTVAL (XEXP (lhs, 1)) >= 0
&& INTVAL (XEXP (lhs, 1)) < HOST_BITS_PER_WIDE_INT)
{
- coeff0 = ((HOST_WIDE_INT) 1) << INTVAL (XEXP (lhs, 1));
+ coeff0l = ((HOST_WIDE_INT) 1) << INTVAL (XEXP (lhs, 1));
+ coeff0h = 0;
lhs = XEXP (lhs, 0);
}
if (GET_CODE (rhs) == NEG)
- coeff1 = -1, rhs = XEXP (rhs, 0);
+ {
+ coeff1l = -1;
+ coeff1h = -1;
+ rhs = XEXP (rhs, 0);
+ }
else if (GET_CODE (rhs) == MULT
&& GET_CODE (XEXP (rhs, 1)) == CONST_INT)
{
- coeff1 = INTVAL (XEXP (rhs, 1)), rhs = XEXP (rhs, 0);
+ coeff1l = INTVAL (XEXP (rhs, 1));
+ coeff1h = INTVAL (XEXP (rhs, 1)) < 0 ? -1 : 0;
+ rhs = XEXP (rhs, 0);
}
else if (GET_CODE (rhs) == ASHIFT
&& GET_CODE (XEXP (rhs, 1)) == CONST_INT
&& INTVAL (XEXP (rhs, 1)) >= 0
&& INTVAL (XEXP (rhs, 1)) < HOST_BITS_PER_WIDE_INT)
{
- coeff1 = ((HOST_WIDE_INT) 1) << INTVAL (XEXP (rhs, 1));
+ coeff1l = ((HOST_WIDE_INT) 1) << INTVAL (XEXP (rhs, 1));
+ coeff1h = 0;
rhs = XEXP (rhs, 0);
}
if (rtx_equal_p (lhs, rhs))
{
rtx orig = gen_rtx_PLUS (mode, op0, op1);
- tem = simplify_gen_binary (MULT, mode, lhs,
-GEN_INT (coeff0 + coeff1));
+ rtx coeff;
+ unsigned HOST_WIDE_INT l;
+ HOST_WIDE_INT h;
+
+ add_double (coeff0l, coeff0h, coeff1l, coeff1h, &l, &h);
+ coeff = immed_double_const (l, h, mode);
+
+ tem = simplify_gen_binary (MULT, mode, lhs, coeff);
return rtx_cost (tem, SET) <= rtx_cost (orig, SET)
? tem : 0;
}
@@ -1405,46 +1428,67 @@ simplify_binary_operation_1 (enum rtx_co
if (! FLOAT_MODE_P (mode))
{
- HOST_WIDE_INT coeff0 = 1, coeff1 = 1;
+ HOST_WIDE_INT coeff0h = 0, negcoeff1h = -1;
+ unsigned HOST_WIDE_INT coeff0l = 1, negcoeff1l = -1;
rtx lhs = op0, rhs = op1;
if (GET_CODE (lhs) == NEG)
- coeff0 = -1,
[Bug rtl-optimization/20290] [4.0/4.1 Regression] Miscompilation on ppc/arm with -Os
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-20
18:34 ---
Subject: [PR rtl-optimization/20290] loop body doesn't run in every iteration
if exit test is the loop entry point
Tree loop optimizations transform the second loop in main() in the
attached testcase in a loop that enters through the exit test. We end
up miscomputing the final value of the original biv because we assume
the increment is executed in every iteration, but since the iteration
count is computed based on the number of times the exit test runs, the
multiplier we use is off by one.
This patch detects the situation of entering the loop through an
unconditional jump to the exit test, which I believe is a relatively
rare idiom that should probably be avoided by tree loop opts as well,
and makes sure the biv increment is marked as not executed in every
iteration. This unfortunately means the biv can't be eliminated. It
could if we kept track of two distinct properties: whether the
increment is conditional (not_every_iteration), and whether the
increment is skipped only in the last iteration (not_last_iteration).
I haven't gone as far as implementing this, since I understand the new
loop optimization infrastructure already handles this case correctly.
Bootstrapped and regtested on x86_64-linux-gnu, verified to fix the
testcase on ppc-linux by visual inspection of the assembly output. Ok
to install?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR rtl-optimization/20290
* loop.c (for_each_insn_in_loop): Don't assume the loop body runs
in every iteration if the entry point is the exit test.
Index: gcc/loop.c
===
RCS file: /cvs/gcc/gcc/gcc/loop.c,v
retrieving revision 1.522
diff -u -p -r1.522 loop.c
--- gcc/loop.c 17 Jan 2005 08:46:15 - 1.522
+++ gcc/loop.c 20 Mar 2005 06:36:43 -
@@ -4655,12 +4655,18 @@ for_each_insn_in_loop (struct loop *loop
int not_every_iteration = 0;
int maybe_multiple = 0;
int past_loop_latch = 0;
+ bool exit_test_is_entry = false;
rtx p;
- /* If loop_scan_start points to the loop exit test, we have to be wary of
- subversive use of gotos inside expression statements. */
+ /* If loop_scan_start points to the loop exit test, the loop body
+ cannot be counted on running on every iteration, and we have to
+ be wary of subversive use of gotos inside expression
+ statements. */
if (prev_nonnote_insn (loop->scan_start) != prev_nonnote_insn (loop->start))
-maybe_multiple = back_branch_in_range_p (loop, loop->scan_start);
+{
+ exit_test_is_entry = true;
+ maybe_multiple = back_branch_in_range_p (loop, loop->scan_start);
+}
/* Scan through loop and update NOT_EVERY_ITERATION and MAYBE_MULTIPLE. */
for (p = next_insn_in_loop (loop, loop->scan_start);
@@ -4718,10 +4724,12 @@ for_each_insn_in_loop (struct loop *loop
beginning, don't set not_every_iteration for that.
This can be any kind of jump, since we want to know if insns
will be executed if the loop is executed. */
- && !(JUMP_LABEL (p) == loop->top
- && ((NEXT_INSN (NEXT_INSN (p)) == loop->end
- && any_uncondjump_p (p))
- || (NEXT_INSN (p) == loop->end && any_condjump_p (p)
+ && (exit_test_is_entry
+ || !(JUMP_LABEL (p) == loop->top
+ && ((NEXT_INSN (NEXT_INSN (p)) == loop->end
+ && any_uncondjump_p (p))
+ || (NEXT_INSN (p) == loop->end
+ && any_condjump_p (p))
{
rtx label = 0;
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR rtl-optimization/20290
* gcc.c-torture/execute/loop-ivopts-2.c: New.
Index: gcc/testsuite/gcc.c-torture/execute/loop-ivopts-2.c
===
RCS file: gcc/testsuite/gcc.c-torture/execute/loop-ivopts-2.c
diff -N gcc/testsuite/gcc.c-torture/execute/loop-ivopts-2.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/gcc.c-torture/execute/loop-ivopts-2.c 20 Mar 2005 06:36:58
-
@@ -0,0 +1,50 @@
+/* PR rtl-optimization/20290 */
+
+/* We used to mis-optimize the second loop in main on at least ppc and
+ arm, because tree loop would change the loop to something like:
+
+ ivtmp.65 = &l[i];
+ ivtmp.16 = 113;
+ goto ();
+
+:;
+ *(ivtmp.65 + 4294967292B) = 9;
+ i = i + 1;
+
+:;
+ ivtmp.16 = ivtmp.16 - 1;
+ ivtmp.65 = ivtmp.65 + 4B;
+ if (ivtmp.16 != 0) goto ;
+
+ We used to consider the increment of i as executed in every
+ iteration, so we'd miscompute the final value. */
+
+extern void abort (void);
+
+void
+check (unsigned int *l)
+{
+ int i;
+ for (i = 0; i < 288; i++)
+if (l[i] != 7 + (i < 256 || i >= 280) + (i >= 144 && i < 256))
+ abort ();
+}
+
+int
+main (void)
+{
+ int i;
+ unsign
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-18 10:14 --- Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable types On Mar 18, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote: > On Mar 17, 2005, Richard Henderson <[EMAIL PROTECTED]> wrote: >>> +cp_gimplify_compound_literal_expr (tree *expr_p, tree *pre_p, tree *post_p) >>> + return gimplify_expr (expr_p, pre_p, post_p, is_gimple_val, >>> fb_rvalue); >> You don't need to recurse here. Just return GS_OK. > Neat! FWIW, removing the recursion enabled me to remove the post_p argument as well, but I only realized that after posting the patch. I'm not reposting it right now for this trivial reason, but consider the patch with this additional change. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20103
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-18
05:38 ---
Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable
types
On Mar 17, 2005, Richard Henderson <[EMAIL PROTECTED]> wrote:
> On Thu, Mar 17, 2005 at 05:11:08AM -0300, Alexandre Oliva wrote:
>> * gimplify.c (gimplify_decl_expr): Add temp variable to binding
>> before gimplifying its initializer.
> Ok.
>> +cp_gimplify_compound_literal_expr (tree *expr_p, tree *pre_p, tree *post_p)
>> + return gimplify_expr (expr_p, pre_p, post_p, is_gimple_val,
>> fb_rvalue);
> You don't need to recurse here. Just return GS_OK.
Neat!
>> + /* If no cleanups are needed, we can do things in a simpler way. */
>> + gimplify_and_add (decl_s, pre_p);
>> + *expr_p = decl;
>> + return GS_OK;
> If you've simplified to a decl, you can return GS_ALL_DONE.
Cool! I've now made the same change to the function in c-gimplify.c
that handles compound literal exprs, from which I'd copied this code.
>> +case COMPOUND_LITERAL_EXPR:
>> + cp_gimplify_compound_literal_expr (expr_p, pre_p, post_p);
>> + ret = GS_OK;
> Should be "ret = cp_gimplify_compound_literal_expr (...)".
Doh. One shouldn't change the return type of a function from void to
something else without checking existing calls, even ones introduced
in the same patch :-/
>> +// { dg-final { scan-assembler "_ZN1sD1Ev.*_ZN1sD1Ev.*_ZN1sD1Ev" } }
>> +
>> +// Make sure we issue 3 dtor calls: one for the t::x, one for the s
>> +// temporary in normal execution flow and one for the same s temporary
>> +// in the EH cleanup should the dtor of t::x throw.
> Wouldn't it be a lot safer to make this a run test instead?
Indeed. Done.
Here's the latest revision of the patch, yet to be bootstrapped and
tested on x86_64-linux-gnu. Hopefully my box won't hang overnight
again :-( It's giving me a lot of grief, but I still haven't figured
out what's wrong with it. Could be the FCdevel kernel built with gcc4
exposing bugs in usb2 or firewire disk drivers, or the internal ide HD
that a few weeks ago looked like it was going to die but then
apparently changed its mind :-(
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/20103
* gimplify.c (gimplify_decl_expr): Add temp variable to binding
before gimplifying its initializer.
* c-gimplify.c (gimplify_compound_literal_expr): After replacing
the expr with a decl, we're all done.
Index: gcc/gimplify.c
===
RCS file: /cvs/gcc/gcc/gcc/gimplify.c,v
retrieving revision 2.118
diff -u -p -r2.118 gimplify.c
--- gcc/gimplify.c 14 Mar 2005 20:01:40 - 2.118
+++ gcc/gimplify.c 18 Mar 2005 05:30:03 -
@@ -990,6 +990,15 @@ gimplify_decl_expr (tree *stmt_p)
{
tree init = DECL_INITIAL (decl);
+ /* This decl isn't mentioned in the enclosing block, so add it
+to the list of temps. FIXME it seems a bit of a kludge to
+say that anonymous artificial vars aren't pushed, but
+everything else is. c_gimplify_compound_literal_expr may
+have already added this tmp var, so don't do it again in this
+case. */
+ if (DECL_ARTIFICIAL (decl) && DECL_NAME (decl) == NULL_TREE)
+ gimple_add_tmp_var (decl);
+
if (!TREE_CONSTANT (DECL_SIZE (decl)))
{
/* This is a variable-sized decl. Simplify its size and mark it
@@ -1043,12 +1052,6 @@ gimplify_decl_expr (tree *stmt_p)
as they may contain a label address. */
walk_tree (&init, force_labels_r, NULL, NULL);
}
-
- /* This decl isn't mentioned in the enclosing block, so add it to the
-list of temps. FIXME it seems a bit of a kludge to say that
-anonymous artificial vars aren't pushed, but everything else is. */
- if (DECL_ARTIFICIAL (decl) && DECL_NAME (decl) == NULL_TREE)
- gimple_add_tmp_var (decl);
}
return GS_ALL_DONE;
Index: gcc/c-gimplify.c
===
RCS file: /cvs/gcc/gcc/gcc/c-gimplify.c,v
retrieving revision 2.26
diff -u -p -r2.26 c-gimplify.c
--- gcc/c-gimplify.c 27 Jan 2005 18:22:19 - 2.26
+++ gcc/c-gimplify.c 18 Mar 2005 05:29:53 -
@@ -487,7 +487,7 @@ gimplify_compound_literal_expr (tree *ex
gimplify_and_add (decl_s, pre_p);
*expr_p = decl;
- return GS_OK;
+ return GS_ALL_DONE;
}
/* Do C-specific gimplification. Args are as for gimplify_expr. */
Index: gcc/cp/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/20103
* cp-tree.h (build_compound_literal): Declare.
* semantics.c (finish_compound_literal): Move most of the code...
* tree.c (build_compound_literal): ... here. New function.
(lvalue_p_1): Handle COMPOUND_LITERAL_EXPR.
(stabilize_init): Likewise.
* pt.c (tsubst_copy_and_build): Likewise.
* call.c (build_over_call):
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-17
10:36 ---
Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable
types
On Mar 11, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote:
> gimplify_and_add calls gimplify_stmt for the stmt list, that calls
> calls gimplify_expr, that calls gimplify_decl_expr, that calls
> gimple_add_tmp_var again.
I don't see any benefit in calling gimple_add_tmp_var *after*
gimplifying the initializer, so I moved the call before that, which
enabled me to get the C++-specific compound-literal-expr gimplifier to
not call gimple_add_tmp_var at all.
As for handling compound-literal-expr cleanups, this patch does so by
using target_expr gimplification code, replacing the
compound-literal-expr with a target_expr when the
compound-literal-expr is found to need a cleanup, and immediately
gimplifying that. This avoids duplicating the code from a number of
static functions in gimplify.c, but I'm open to other approaches, such
as exporting gimple_push_cleanup from gimplify.c.
Tested on x86_64-linux-gnu. Ok to install?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/20103
* gimplify.c (gimplify_decl_expr): Add temp variable to binding
before gimplifying its initializer.
Index: gcc/gimplify.c
===
RCS file: /cvs/gcc/gcc/gcc/gimplify.c,v
retrieving revision 2.118
diff -u -p -r2.118 gimplify.c
--- gcc/gimplify.c 14 Mar 2005 20:01:40 - 2.118
+++ gcc/gimplify.c 17 Mar 2005 07:56:22 -
@@ -990,6 +990,15 @@ gimplify_decl_expr (tree *stmt_p)
{
tree init = DECL_INITIAL (decl);
+ /* This decl isn't mentioned in the enclosing block, so add it
+to the list of temps. FIXME it seems a bit of a kludge to
+say that anonymous artificial vars aren't pushed, but
+everything else is. c_gimplify_compound_literal_expr may
+have already added this tmp var, so don't do it again in this
+case. */
+ if (DECL_ARTIFICIAL (decl) && DECL_NAME (decl) == NULL_TREE)
+ gimple_add_tmp_var (decl);
+
if (!TREE_CONSTANT (DECL_SIZE (decl)))
{
/* This is a variable-sized decl. Simplify its size and mark it
@@ -1043,12 +1052,6 @@ gimplify_decl_expr (tree *stmt_p)
as they may contain a label address. */
walk_tree (&init, force_labels_r, NULL, NULL);
}
-
- /* This decl isn't mentioned in the enclosing block, so add it to the
-list of temps. FIXME it seems a bit of a kludge to say that
-anonymous artificial vars aren't pushed, but everything else is. */
- if (DECL_ARTIFICIAL (decl) && DECL_NAME (decl) == NULL_TREE)
- gimple_add_tmp_var (decl);
}
return GS_ALL_DONE;
Index: gcc/cp/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/20103
* cp-tree.h (build_compound_literal): Declare.
* semantics.c (finish_compound_literal): Move most of the code...
* tree.c (build_compound_literal): ... here. New function.
(lvalue_p_1): Handle COMPOUND_LITERAL_EXPR.
(stabilize_init): Likewise.
* pt.c (tsubst_copy_and_build): Likewise.
* call.c (build_over_call): Likewise.
* class.c (fixed_type_or_null): Likewise.
* cp-gimplify.c (cp_gimplify_init_expr): Likewise.
(cp_gimplify_compound_literal_expr): New.
(cp_gimplify_expr): Use it.
* cvt.c (force_rvalue, ocp_convert): Handle COMPOUND_LITERAL_EXPR.
* typeck.c (build_x_unary_op): Likewise.
(cxx_mark_addressable): Likewise.
(maybe_warn_about_returning_address_of_local): Likewise.
Index: gcc/cp/cp-tree.h
===
RCS file: /cvs/gcc/gcc/gcc/cp/cp-tree.h,v
retrieving revision 1.1110
diff -u -p -r1.1110 cp-tree.h
--- gcc/cp/cp-tree.h 14 Mar 2005 14:33:17 - 1.1110
+++ gcc/cp/cp-tree.h 17 Mar 2005 07:56:31 -
@@ -4219,6 +4219,7 @@ extern tree build_min_nt (enum tree_co
extern tree build_min_non_dep (enum tree_code, tree, ...);
extern tree build_cplus_new(tree, tree);
extern tree get_target_expr(tree);
+extern tree build_compound_literal (tree, tree);
extern tree build_cplus_array_type (tree, tree);
extern tree hash_tree_cons (tree, tree, tree);
extern tree hash_tree_chain(tree, tree);
Index: gcc/cp/semantics.c
===
RCS file: /cvs/gcc/gcc/gcc/cp/semantics.c,v
retrieving revision 1.464
diff -u -p -r1.464 semantics.c
--- gcc/cp/semantics.c 14 Mar 2005 14:33:34 - 1.464
+++ gcc/cp/semantics.c 17 Mar 2005 07:56:33 -
@@ -1980,23 +1980,16 @@ finish_compound_literal (tree type, tree
compound_literal = build_constructor (NULL_TREE, initializer_list);
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-11 19:29 --- Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable types On Mar 11, 2005, Richard Henderson <[EMAIL PROTECTED]> wrote: > On Tue, Mar 08, 2005 at 05:42:57PM -0300, Alexandre Oliva wrote: >> The change to the gimplify.c is needed to avoid having >> gimple_add_tmp_var twice for the variable, once while expanding the >> declaration/initialization, once while expanding the clean-ups. > Can you say more about how this comes to be? It doesn't make > sense to me... Just a matter of misinterpreting observed behavior :-( Here's the correct sequence of events without the change: gimplify_compound_literal_expr calls gimple_add_tmp_var, and then gimplify_and_add for the statement list containing the decl of the temporary variable gimplify_and_add calls gimplify_stmt for the stmt list, that calls calls gimplify_expr, that calls gimplify_decl_expr, that calls gimple_add_tmp_var again. I should now try to figure out why it is that in C it doesn't get called twice, in but in C++ it does. I remember I tried to remove the first call from gimplify_compound_literal_expr, but that didn't work because the presence of the initializer would cause gimplify_decl_expr to build a modify_expr referencing the decl, and this fails if the decl isn't, erhm, bound yet. Perhaps I should move the call to gimple_add_tmp_var in gimplify_decl_expr before the init stmt; this even makes sense to me. Incidentally, we don't actually generate cleanups for these temps, unlike the target_expr case, so we fail to call dtors for them. Ugh. Fixing this will take some more work. Patch withdrawn for the time being. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20103
[Bug target/20126] [3.3/3.4/4.0/4.1 Regression] Inlined memcmp makes one argument null on entry
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-11
14:29 ---
Subject: Re: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail
On Mar 10, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote:
> + ??? Should this should search new for new volatile MEMs and reject
> + them? */
Here's a stricter version that does test for this.
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR target/20126
* loop.c (loop_givs_rescan): If replacement of DEST_ADDR failed,
set the original address pseudo to the correct value before the
original insn, if possible, and leave the insn alone, otherwise
create a new pseudo, set it and replace it in the insn.
* recog.c (validate_change_maybe_volatile): New.
* recog.h (validate_change_maybe_volatile): Declare.
Index: gcc/loop.c
===
RCS file: /cvs/gcc/gcc/gcc/loop.c,v
retrieving revision 1.522
diff -u -p -r1.522 loop.c
--- gcc/loop.c 17 Jan 2005 08:46:15 - 1.522
+++ gcc/loop.c 11 Mar 2005 14:17:20 -
@@ -5470,9 +5470,31 @@ loop_givs_rescan (struct loop *loop, str
mark_reg_pointer (v->new_reg, 0);
if (v->giv_type == DEST_ADDR)
- /* Store reduced reg as the address in the memref where we found
- this giv. */
- validate_change (v->insn, v->location, v->new_reg, 0);
+ {
+ /* Store reduced reg as the address in the memref where we found
+this giv. */
+ if (validate_change_maybe_volatile (v->insn, v->location,
+ v->new_reg))
+ /* Yay, it worked! */;
+ /* Not replaceable; emit an insn to set the original
+giv reg from the reduced giv. */
+ else if (REG_P (*v->location))
+ loop_insn_emit_before (loop, 0, v->insn,
+ gen_move_insn (*v->location,
+ v->new_reg));
+ else
+ {
+ /* If it wasn't a reg, create a pseudo and use that. */
+ rtx reg, seq;
+ start_sequence ();
+ reg = force_reg (v->mode, *v->location);
+ seq = get_insns ();
+ end_sequence ();
+ loop_insn_emit_before (loop, 0, v->insn, seq);
+ gcc_assert (validate_change_maybe_volatile (v->insn, v->location,
+ reg));
+ }
+ }
else if (v->replaceable)
{
reg_map[REGNO (v->dest_reg)] = v->new_reg;
Index: gcc/recog.c
===
RCS file: /cvs/gcc/gcc/gcc/recog.c,v
retrieving revision 1.221
diff -u -p -r1.221 recog.c
--- gcc/recog.c 7 Mar 2005 13:52:08 - 1.221
+++ gcc/recog.c 11 Mar 2005 14:17:22 -
@@ -235,6 +235,45 @@ validate_change (rtx object, rtx *loc, r
return apply_change_group ();
}
+
+/* Function to be passed to for_each_rtx to test whether a piece of
+ RTL contains any mem/v. */
+static int
+volatile_mem_p (rtx *x, void *data ATTRIBUTE_UNUSED)
+{
+ return (MEM_P (*x) && MEM_VOLATILE_P (*x));
+}
+
+/* Same as validate_change, but doesn't support groups, and it accepts
+ volatile mems if they're already present in the original insn. */
+
+int
+validate_change_maybe_volatile (rtx object, rtx *loc, rtx new)
+{
+ int result;
+
+ if (validate_change (object, loc, new, 0))
+return 1;
+
+ if (volatile_ok || ! insn_invalid_p (object))
+return 0;
+
+ /* Make sure we're not adding or removing volatile MEMs. */
+ if (for_each_rtx (loc, volatile_mem_p, 0)
+ || for_each_rtx (&new, volatile_mem_p, 0))
+return 0;
+
+ volatile_ok = 1;
+
+ gcc_assert (! insn_invalid_p (object));
+
+ result = validate_change (object, loc, new, 0);
+
+ volatile_ok = 0;
+
+ return result;
+}
+
/* This subroutine of apply_change_group verifies whether the changes to INSN
were valid; i.e. whether INSN can still be recognized. */
Index: gcc/recog.h
===
RCS file: /cvs/gcc/gcc/gcc/recog.h,v
retrieving revision 1.55
diff -u -p -r1.55 recog.h
--- gcc/recog.h 7 Mar 2005 13:52:09 - 1.55
+++ gcc/recog.h 11 Mar 2005 14:17:22 -
@@ -74,6 +74,7 @@ extern void init_recog_no_volatile (void
extern int check_asm_operands (rtx);
extern int asm_operand_ok (rtx, const char *);
extern int validate_change (rtx, rtx *, rtx, int);
+extern int validate_change_maybe_volatile (rtx, rtx *, rtx);
extern int insn_invalid_p (rtx);
extern void confirm_change_group (void);
extern int apply_change_group (void);
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
* gcc.dg/pr20126.c: New.
Index: gcc/testsuite/gcc.dg/pr20126.c
===
RCS file: gcc/testsuite/gcc.dg/pr20126.c
diff -N gcc/testsui
[Bug rtl-optimization/18628] [4.0/4.1 regression] miscompilation of switch statement in loop
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-10
20:38 ---
Subject: Re: [PR middle-end/18628] do not fold to label load from tablejump to
reg
On Mar 10, 2005, Richard Henderson <[EMAIL PROTECTED]> wrote:
> On Wed, Mar 09, 2005 at 07:26:37AM -0300, Alexandre Oliva wrote:
>> +/* If it's not a REG, the REG_EQUAL note is inappropriate. */
>> +if (REG_P (SET_DEST (set)))
>> + set_unique_reg_note (insn, REG_EQUAL, label);
> I don't think this is a good idea at all. This is just
> asking for reload to recreate a reference to the deleted label.
Here's a patch with that bit removed, along with the change in
cse_init that it required. Ok?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR middle-end/18628
* cse.c (fold_rtx_mem): Don't fold a load from a jumptable into a
register.
Index: gcc/cse.c
===
RCS file: /cvs/gcc/gcc/gcc/cse.c,v
retrieving revision 1.349
diff -u -p -r1.349 cse.c
--- gcc/cse.c 8 Mar 2005 13:56:56 - 1.349
+++ gcc/cse.c 10 Mar 2005 20:36:36 -
@@ -3515,8 +3515,30 @@ fold_rtx_mem (rtx x, rtx insn)
if (offset >= 0
&& (offset / GET_MODE_SIZE (GET_MODE (table))
< XVECLEN (table, 0)))
- return XVECEXP (table, 0,
- offset / GET_MODE_SIZE (GET_MODE (table)));
+ {
+ rtx label = XVECEXP
+ (table, 0, offset / GET_MODE_SIZE (GET_MODE (table)));
+ rtx set;
+
+ /* If we have an insn that loads the label from the
+ jumptable into a reg, we don't want to set the reg
+ to the label, because this may cause a reference to
+ the label to remain after the label is removed in
+ some very obscure cases (PR middle-end/18628). */
+ if (!insn)
+ return label;
+
+ set = single_set (insn);
+
+ if (! set || SET_SRC (set) != x)
+ return x;
+
+ /* If it's a jump, it's safe to reference the label. */
+ if (SET_DEST (set) == pc_rtx)
+ return label;
+
+ return x;
+ }
}
if (table_insn && JUMP_P (table_insn)
&& GET_CODE (PATTERN (table_insn)) == ADDR_DIFF_VEC)
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
* gcc.dg/pr18628.c: New.
Index: gcc/testsuite/gcc.dg/pr18628.c
===
RCS file: gcc/testsuite/gcc.dg/pr18628.c
diff -N gcc/testsuite/gcc.dg/pr18628.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/gcc.dg/pr18628.c 10 Mar 2005 20:36:52 -
@@ -0,0 +1,31 @@
+/* { dg-do link } */
+/* { dg-options "-O2" } */
+
+/* PR middle-end/18628 exposed a problem in which cse folded a load
+ from a jump table into the label that was the target of the branch.
+ Unfortunately, the indirect jump was moved to a different basic
+ block, and the LABEL_REF copied to the register wasn't enough to
+ keep the cfg from optimizing the otherwise-unused label away. So
+ we ended up with a dangling reference to the label. */
+
+int i;
+
+int main()
+{
+ for (;;)
+ {
+switch (i)
+{
+ case 0:
+ case 1:
+return 1;
+
+ case 2:
+ case 3:
+return 0;
+
+ case 5:
+--i;
+}
+ }
+}
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=18628
[Bug target/20126] [3.3/3.4/4.0/4.1 Regression] Inlined memcmp makes one argument null on entry
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-10
11:44 ---
Subject: Re: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail
On Mar 9, 2005, Jakub Jelinek <[EMAIL PROTECTED]> wrote:
> On Wed, Mar 09, 2005 at 01:02:08AM -0300, Alexandre Oliva wrote:
>> On Mar 8, 2005, Jakub Jelinek <[EMAIL PROTECTED]> wrote:
>>
>> > Unfortunately, it seems to break ada bootstrap on at least x86-64 and i386:
>>
>> Odd... It surely completed bootstrap for me with default build flags.
> Your code hits just once on ali.adb
Rats, I didn't realize I needed --enable-languages=all,ada to pick
that up. Fixed now.
> Note that recog_memoized (v->insn) is -1 even without the change,
> not sure what would fix that up.
Problem is that the insn has a volatile memory access, and loop runs
with volatile_ok = 0.
I'm not entirely sure why that is; presumably to avoid introducing or
removing volatile memory accesses. I can see how this prevents
introducing them, but it appears to me that it still can remove them.
Anyhow, I've come up with a solution for this. This patch introduces
a new function that is like validate_change, but if validate_change
fails and the original insn fails to be recognized with !volatile_ok
but passes with volatile_ok, then try the change and recog with
volatile_ok.
> But it certainly shows that *v->location doesn't have to be
> a simple pseudo, so gen_move_insn might not work.
Indeed. I've introduced a new pseudo to hold the computed value now,
for the case in which *v->location isn't a reg.
Passed bootstrap and regtest on x86_64-linux-gnu on mainline, as well
as bootstrap on gcc-4_0-rhl-branch, both with Ada enabled. Ok to
install?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR target/20126
* loop.c (loop_givs_rescan): If replacement of DEST_ADDR failed,
set the original address pseudo to the correct value before the
original insn, if possible, and leave the insn alone, otherwise
create a new pseudo, set it and replace it in the insn.
* recog.c (validate_change_maybe_volatile): New.
* recog.h (validate_change_maybe_volatile): Declare.
Index: gcc/loop.c
===
RCS file: /cvs/gcc/gcc/gcc/loop.c,v
retrieving revision 1.522
diff -u -p -r1.522 loop.c
--- gcc/loop.c 17 Jan 2005 08:46:15 - 1.522
+++ gcc/loop.c 10 Mar 2005 11:23:59 -
@@ -5470,9 +5470,31 @@ loop_givs_rescan (struct loop *loop, str
mark_reg_pointer (v->new_reg, 0);
if (v->giv_type == DEST_ADDR)
- /* Store reduced reg as the address in the memref where we found
- this giv. */
- validate_change (v->insn, v->location, v->new_reg, 0);
+ {
+ /* Store reduced reg as the address in the memref where we found
+this giv. */
+ if (validate_change_maybe_volatile (v->insn, v->location,
+ v->new_reg))
+ /* Yay, it worked! */;
+ /* Not replaceable; emit an insn to set the original
+giv reg from the reduced giv. */
+ else if (REG_P (*v->location))
+ loop_insn_emit_before (loop, 0, v->insn,
+ gen_move_insn (*v->location,
+ v->new_reg));
+ else
+ {
+ /* If it wasn't a reg, create a pseudo and use that. */
+ rtx reg, seq;
+ start_sequence ();
+ reg = force_reg (v->mode, *v->location);
+ seq = get_insns ();
+ end_sequence ();
+ loop_insn_emit_before (loop, 0, v->insn, seq);
+ gcc_assert (validate_change_maybe_volatile (v->insn, v->location,
+ reg));
+ }
+ }
else if (v->replaceable)
{
reg_map[REGNO (v->dest_reg)] = v->new_reg;
Index: gcc/recog.c
===
RCS file: /cvs/gcc/gcc/gcc/recog.c,v
retrieving revision 1.221
diff -u -p -r1.221 recog.c
--- gcc/recog.c 7 Mar 2005 13:52:08 - 1.221
+++ gcc/recog.c 10 Mar 2005 11:24:01 -
@@ -235,6 +235,34 @@ validate_change (rtx object, rtx *loc, r
return apply_change_group ();
}
+/* Same as validate_change, but doesn't support groups, and it accepts
+ volatile mems if they're already present in the original insn.
+
+ ??? Should this should search new for new volatile MEMs and reject
+ them? */
+
+int
+validate_change_maybe_volatile (rtx object, rtx *loc, rtx new)
+{
+ int result;
+
+ if (validate_change (object, loc, new, 0))
+return 1;
+
+ if (volatile_ok || ! insn_invalid_p (object))
+return 0;
+
+ volatile_ok = 1;
+
+ gcc_assert (! insn_invalid_p (object));
+
+ result = validate_change (object, loc, new, 0);
+
+ volatile_ok = 0;
+
+ return result;
+}
+
/* This subroutine of apply_cha
[Bug middle-end/18628] [4.0/4.1 regression] miscompilation of switch statement in loop
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-09
10:30 ---
Subject: [PR middle-end/18628] do not fold to label load from tablejump to reg
This patch is meant to implement suggestion #3 proposed to fix the bug
by Roger Sayle and selected by RTH in bugzilla. So far, I've only
verified that it fixes the testcase included in the patch.
The thought of adding the REG_EQUAL note was to help other passes that
might want to turn the indirect jump into a direct jump. I'm not sure
this may actually happen.
Bootstrap and regtesting starting shortly. Ok to install if they
pass?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR middle-end/18628
* cse.c (fold_rtx_mem): Instead of returning the label extracted
from a tablejump, add it as an REG_EQUAL note, if the insn loaded
from the table to a register.
(cse_insn): Don't use it as src_eqv.
Index: gcc/cse.c
===
RCS file: /cvs/gcc/gcc/gcc/cse.c,v
retrieving revision 1.349
diff -u -p -r1.349 cse.c
--- gcc/cse.c 8 Mar 2005 13:56:56 - 1.349
+++ gcc/cse.c 9 Mar 2005 10:19:13 -
@@ -3515,8 +3515,36 @@ fold_rtx_mem (rtx x, rtx insn)
if (offset >= 0
&& (offset / GET_MODE_SIZE (GET_MODE (table))
< XVECLEN (table, 0)))
- return XVECEXP (table, 0,
- offset / GET_MODE_SIZE (GET_MODE (table)));
+ {
+ rtx label = XVECEXP
+ (table, 0, offset / GET_MODE_SIZE (GET_MODE (table)));
+ rtx set;
+
+ /* If we have an insn that loads the label from the
+ jumptable into a reg, we don't want to set the reg
+ to the label, because this may cause a reference to
+ the label to remain after the label is removed in
+ some very obscure cases (PR middle-end/18628). So
+ we just set a REG_EQUAL note for this case, and
+ return the original MEM. */
+ if (!insn)
+ return label;
+
+ set = single_set (insn);
+
+ if (! set || SET_SRC (set) != x)
+ return x;
+
+ /* If it's a jump, it's safe to reference the label. */
+ if (SET_DEST (set) == pc_rtx)
+ return label;
+
+ /* If it's not a REG, the REG_EQUAL note is inappropriate. */
+ if (REG_P (SET_DEST (set)))
+ set_unique_reg_note (insn, REG_EQUAL, label);
+
+ return x;
+ }
}
if (table_insn && JUMP_P (table_insn)
&& GET_CODE (PATTERN (table_insn)) == ADDR_DIFF_VEC)
@@ -4861,6 +4889,13 @@ cse_insn (rtx insn, rtx libcall_insn)
{
src_eqv = fold_rtx (canon_reg (XEXP (tem, 0), NULL_RTX), insn);
XEXP (tem, 0) = src_eqv;
+
+ /* We don't want to use the labels in REG_EQUAL notes that
+fold_rtx may have added in an earlier pass. If it's
+something as simple as a LABEL_REF and we didn't set to it
+directly, there was a reason not to do so. */
+ if (GET_CODE (src_eqv) == LABEL_REF)
+ src_eqv = NULL;
}
/* Canonicalize sources and addresses of destinations.
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
* gcc.dg/pr18628.c: New.
Index: gcc/testsuite/gcc.dg/pr18628.c
===
RCS file: gcc/testsuite/gcc.dg/pr18628.c
diff -N gcc/testsuite/gcc.dg/pr18628.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/gcc.dg/pr18628.c 9 Mar 2005 10:19:27 -
@@ -0,0 +1,31 @@
+/* { dg-do link } */
+/* { dg-options "-O2" } */
+
+/* PR middle-end/18628 exposed a problem in which cse folded a load
+ from a jump table into the label that was the target of the branch.
+ Unfortunately, the indirect jump was moved to a different basic
+ block, and the LABEL_REF copied to the register wasn't enough to
+ keep the cfg from optimizing the otherwise-unused label away. So
+ we ended up with a dangling reference to the label. */
+
+int i;
+
+int main()
+{
+ for (;;)
+ {
+switch (i)
+{
+ case 0:
+ case 1:
+return 1;
+
+ case 2:
+ case 3:
+return 0;
+
+ case 5:
+--i;
+}
+ }
+}
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=18628
[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-09
04:11 ---
Subject: Re: [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a
reference to a temporary
On Mar 8, 2005, Roger Sayle <[EMAIL PROTECTED]> wrote:
> On 8 Mar 2005, Alexandre Oliva wrote:
>>
>> * fold-const.c (non_lvalue): Split tests into...
>> (maybe_lvalue_p): New function.
>> (fold_ternary): Use it to avoid turning a COND_EXPR lvalue into
>> a MIN_EXPR rvalue.
> This version is Ok for mainline, and currently open release branches.
Unfortunately, the problem in g++.oliva/expr2.C resurfaced, because,
as it turns out, the transformation (I != J ? I : J) => I yields an
lvalue as required, but not the *correct* lvalue in all cases. I
tried what you'd suggested on IRC (testing whether the thing is an
lvalue after the fact), but that obviously failed in this case as
well.
So I figured a better approach would be to perform lvalue tests to
fold_cond_expr_with_comparison(), where we can avoid specific
inappropriate transformations while still trying other alternatives.
While at that, I figured we could use pedantic_lvalues to avoid
refraining from applying optimizations just because it looks like the
cond-expr might be an lvalue, even though the language requires it not
to be.
This is currently bootstrapping on x86_64-linux-gnu. Ok to install if
bootstrap completes and regtesting passes?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/19199
* fold-const.c (non_lvalue): Split tests into...
(maybe_lvalue_p): New function.
(fold_cond_expr_with_comparison): Preserve lvalue-ness.
Index: gcc/fold-const.c
===
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.535
diff -u -p -r1.535 fold-const.c
--- gcc/fold-const.c 7 Mar 2005 21:24:21 - 1.535
+++ gcc/fold-const.c 9 Mar 2005 03:59:18 -
@@ -2005,16 +2005,12 @@ fold_convert (tree type, tree arg)
}
}
-/* Return an expr equal to X but certainly not valid as an lvalue. */
+/* Return false if expr can be assumed to not be an lvalue, true
+ otherwise. */
-tree
-non_lvalue (tree x)
+static bool
+maybe_lvalue_p (tree x)
{
- /* While we are in GIMPLE, NON_LVALUE_EXPR doesn't mean anything to
- us. */
- if (in_gimple_form)
-return x;
-
/* We only need to wrap lvalue tree codes. */
switch (TREE_CODE (x))
{
@@ -2054,8 +2050,24 @@ non_lvalue (tree x)
/* Assume the worst for front-end tree codes. */
if ((int)TREE_CODE (x) >= NUM_TREE_CODES)
break;
-return x;
+return false;
}
+
+ return true;
+}
+
+/* Return an expr equal to X but certainly not valid as an lvalue. */
+
+tree
+non_lvalue (tree x)
+{
+ /* While we are in GIMPLE, NON_LVALUE_EXPR doesn't mean anything to
+ us. */
+ if (in_gimple_form)
+return x;
+
+ if (! maybe_lvalue_p (x))
+return x;
return build1 (NON_LVALUE_EXPR, TREE_TYPE (x), x);
}
@@ -4162,7 +4174,15 @@ fold_cond_expr_with_comparison (tree typ
tree arg00 = TREE_OPERAND (arg0, 0);
tree arg01 = TREE_OPERAND (arg0, 1);
tree arg1_type = TREE_TYPE (arg1);
- tree tem;
+ tree tem = NULL;
+ /* If the COND_EXPR can possibly be an lvalue, we don't want to
+ perform transformations that return a simplified result that will
+ be recognized as lvalue, but that will not match the expected
+ result. We may still return other expressions that would be
+ incorrect, but those are going to be rvalues, and the caller is
+ supposed to discard them. */
+ bool lvalue = !pedantic_lvalues
+&& maybe_lvalue_p (arg1) && maybe_lvalue_p (arg2);
STRIP_NOPS (arg1);
STRIP_NOPS (arg2);
@@ -4196,10 +4216,12 @@ fold_cond_expr_with_comparison (tree typ
case EQ_EXPR:
case UNEQ_EXPR:
tem = fold_convert (arg1_type, arg1);
- return pedantic_non_lvalue (fold_convert (type, negate_expr (tem)));
+ tem = pedantic_non_lvalue (fold_convert (type, negate_expr (tem)));
+ break;
case NE_EXPR:
case LTGT_EXPR:
- return pedantic_non_lvalue (fold_convert (type, arg1));
+ tem = pedantic_non_lvalue (fold_convert (type, arg1));
+ break;
case UNGE_EXPR:
case UNGT_EXPR:
if (flag_trapping_math)
@@ -4211,7 +4233,8 @@ fold_cond_expr_with_comparison (tree typ
arg1 = fold_convert (lang_hooks.types.signed_type
(TREE_TYPE (arg1)), arg1);
tem = fold (build1 (ABS_EXPR, TREE_TYPE (arg1), arg1));
- return pedantic_non_lvalue (fold_convert (type, tem));
+ tem = pedantic_non_lvalue (fold_convert (type, tem));
+ break;
case UNLE_EXPR:
case UNLT_EXPR:
if (flag_trapping_math)
@@ -4222,12 +4245,18 @@ fold_cond_expr_with_comparison (tree typ
arg1 = fold_convert (lang_hooks.types.signed_type
(TREE_TYPE (arg1)), arg1);
tem = fold (bu
[Bug target/20126] [3.3/3.4/4.0/4.1 Regression] Inlined memcmp makes one argument null on entry
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-09 04:02 --- Subject: Re: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail On Mar 8, 2005, Jakub Jelinek <[EMAIL PROTECTED]> wrote: > Unfortunately, it seems to break ada bootstrap on at least x86-64 and i386: Odd... It surely completed bootstrap for me with default build flags. Hmm... FORTIFY_SOURCE? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20126
[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-08
23:23 ---
Subject: Re: [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a
reference to a temporary
On Mar 7, 2005, Roger Sayle <[EMAIL PROTECTED]> wrote:
> For example, I believe that Alex's proposed solution to PR c++/19199
> isn't an appropriate fix. It's perfectly reasonable for fold to convert
> a C++ COND_EXPR into a MIN_EXPR or MAX_EXPR, as according to the C++
> front-end all three of these tree nodes are valid lvalues. Hence it's
> not this transformation in fold that's in error.
Bugzilla was kept out of the long discussion that ensued, so I'll try
to summarize. The problem is that the conversion is turning a
COND_EXPR such as:
((int)a < (int)b ? a : b)
into
(__typeof(a)) ((int)a Simply disabling the COND_EXPR -> MIN_EXPR/MAX_EXPR transformation is
> also likely to be a serious performance penalty, especially on targets
> that support efficient sequences for "min" and "max".
This was not what I intended to do with my patch, FWIW.
Unfortunately, I goofed in the added call to operand_equal_p, limiting
too much the situations in which the optimization could be applied.
The patch fixes this problem, and updates the patch such that it
applies cleanly again.
As for other languages whose COND_EXPRs can't be lvalues, we can
probably arrange quite easily for them to ensure at least one of their
result operands is not an lvalue, so as to enable the transformation
again.
Comments? Ok to install?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
* fold-const.c (non_lvalue): Split tests into...
(maybe_lvalue_p): New function.
(fold_ternary): Use it to avoid turning a COND_EXPR lvalue into
a MIN_EXPR rvalue.
Index: gcc/fold-const.c
===
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.535
diff -u -p -r1.535 fold-const.c
--- gcc/fold-const.c 7 Mar 2005 21:24:21 - 1.535
+++ gcc/fold-const.c 8 Mar 2005 22:07:52 -
@@ -2005,16 +2005,12 @@ fold_convert (tree type, tree arg)
}
}
-/* Return an expr equal to X but certainly not valid as an lvalue. */
+/* Return false if expr can be assumed to not be an lvalue, true
+ otherwise. */
-tree
-non_lvalue (tree x)
+static bool
+maybe_lvalue_p (tree x)
{
- /* While we are in GIMPLE, NON_LVALUE_EXPR doesn't mean anything to
- us. */
- if (in_gimple_form)
-return x;
-
/* We only need to wrap lvalue tree codes. */
switch (TREE_CODE (x))
{
@@ -2054,8 +2050,24 @@ non_lvalue (tree x)
/* Assume the worst for front-end tree codes. */
if ((int)TREE_CODE (x) >= NUM_TREE_CODES)
break;
-return x;
+return false;
}
+
+ return true;
+}
+
+/* Return an expr equal to X but certainly not valid as an lvalue. */
+
+tree
+non_lvalue (tree x)
+{
+ /* While we are in GIMPLE, NON_LVALUE_EXPR doesn't mean anything to
+ us. */
+ if (in_gimple_form)
+return x;
+
+ if (! maybe_lvalue_p (x))
+return x;
return build1 (NON_LVALUE_EXPR, TREE_TYPE (x), x);
}
@@ -9734,10 +9746,16 @@ fold_ternary (tree expr)
of B and C. Signed zeros prevent all of these transformations,
for reasons given above each one.
+We don't want to use operand_equal_for_comparison_p here,
+because it might turn an lvalue COND_EXPR into an rvalue one,
+see PR c++/19199.
+
Also try swapping the arguments and inverting the conditional. */
if (COMPARISON_CLASS_P (arg0)
- && operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
-arg1, TREE_OPERAND (arg0, 1))
+ && ((maybe_lvalue_p (op1) && maybe_lvalue_p (op2))
+ ? operand_equal_p (TREE_OPERAND (arg0, 0), op1, 0)
+ : operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
+ arg1, TREE_OPERAND (arg0, 1)))
&& !HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (arg1
{
tem = fold_cond_expr_with_comparison (type, arg0, op1, op2);
@@ -9746,9 +9764,10 @@ fold_ternary (tree expr)
}
if (COMPARISON_CLASS_P (arg0)
- && operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
-op2,
-TREE_OPERAND (arg0, 1))
+ && ((maybe_lvalue_p (op1) && maybe_lvalue_p (op2))
+ ? operand_equal_p (TREE_OPERAND (arg0, 0), op2, 0)
+ : operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
+ op2, TREE_OPERAND (arg0, 1)))
&& !HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (op2
{
tem = invert_truthvalue (arg0);
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/19199
* g++.dg/expr/lval2.C: New.
Index: gcc/testsuite/g++.dg/expr/
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-08
21:55 ---
Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable
types
On Mar 8, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote:
> Okie dokie, how about this?
> The change to the gimplify.c is needed to avoid having
> gimple_add_tmp_var twice for the variable, once while expanding the
> declaration/initialization, once while expanding the clean-ups.
> Ok to install? Passed check-g++ (except for the already-broken
> eh/cleanup1.C) on x86_64-linux-gnu; just started bootstrap and full
> reg-testing.
Err... *This* was the one that passed check-g++. The one I posted in
a hurry earlier was very incomplete, and contained fragments of other
patches. Sorry about that.
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/20103
* gimplify.c (gimplify_decl_expr): Don't add temp variable if it
was already seen in a bind expr.
Index: gcc/gimplify.c
===
RCS file: /cvs/gcc/gcc/gcc/gimplify.c,v
retrieving revision 2.115
diff -u -p -r2.115 gimplify.c
--- gcc/gimplify.c 8 Mar 2005 13:56:57 - 2.115
+++ gcc/gimplify.c 8 Mar 2005 21:48:41 -
@@ -1047,7 +1047,8 @@ gimplify_decl_expr (tree *stmt_p)
/* This decl isn't mentioned in the enclosing block, so add it to the
list of temps. FIXME it seems a bit of a kludge to say that
anonymous artificial vars aren't pushed, but everything else is. */
- if (DECL_ARTIFICIAL (decl) && DECL_NAME (decl) == NULL_TREE)
+ if (DECL_ARTIFICIAL (decl) && DECL_NAME (decl) == NULL_TREE
+ && !DECL_SEEN_IN_BIND_EXPR_P (decl))
gimple_add_tmp_var (decl);
}
Index: gcc/cp/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/20103
* cp-tree.h (build_compound_literal): Declare.
* semantics.c (finish_compound_literal): Move most of the code...
* tree.c (build_compound_literal): ... here. New function.
(lvalue_p_1): Handle COMPOUND_LITERAL_EXPR.
(stabilize_init): Likewise.
* pt.c (tsubst_copy_and_build): Likewise.
* call.c (build_over_call): Likewise.
* class.c (fixed_type_or_null): Likewise.
* cp-gimplify.c (cp_gimplify_init_expr): Likewise.
* cvt.c (force_rvalue, ocp_convert): Likewise.
* typeck.c (build_x_unary_op): Likewise.
(cxx_mark_addressable): Likewise.
(maybe_warn_about_returning_address_of_local): Likewise.
Index: gcc/cp/cp-tree.h
===
RCS file: /cvs/gcc/gcc/gcc/cp/cp-tree.h,v
retrieving revision 1.1107
diff -u -p -r1.1107 cp-tree.h
--- gcc/cp/cp-tree.h 1 Mar 2005 09:57:38 - 1.1107
+++ gcc/cp/cp-tree.h 8 Mar 2005 21:48:50 -
@@ -4218,6 +4218,7 @@ extern tree build_min_nt (enum tree_co
extern tree build_min_non_dep (enum tree_code, tree, ...);
extern tree build_cplus_new(tree, tree);
extern tree get_target_expr(tree);
+extern tree build_compound_literal (tree, tree);
extern tree build_cplus_array_type (tree, tree);
extern tree hash_tree_cons (tree, tree, tree);
extern tree hash_tree_chain(tree, tree);
Index: gcc/cp/semantics.c
===
RCS file: /cvs/gcc/gcc/gcc/cp/semantics.c,v
retrieving revision 1.463
diff -u -p -r1.463 semantics.c
--- gcc/cp/semantics.c 23 Feb 2005 05:30:48 - 1.463
+++ gcc/cp/semantics.c 8 Mar 2005 21:48:52 -
@@ -1980,23 +1980,16 @@ finish_compound_literal (tree type, tree
compound_literal = build_constructor (NULL_TREE, initializer_list);
/* Mark it as a compound-literal. */
TREE_HAS_CONSTRUCTOR (compound_literal) = 1;
+
if (processing_template_decl)
+/* This causes template substitution to run digest_init on the
+ CONSTRUCTOR. */
TREE_TYPE (compound_literal) = type;
else
-{
- /* Check the initialization. */
- compound_literal = digest_init (type, compound_literal, NULL);
- /* If the TYPE was an array type with an unknown bound, then we can
-figure out the dimension now. For example, something like:
-
- `(int []) { 2, 3 }'
-
-implies that the array has two elements. */
- if (TREE_CODE (type) == ARRAY_TYPE && !COMPLETE_TYPE_P (type))
- complete_array_type (type, compound_literal, 1);
-}
+/* Check the initialization. */
+compound_literal = digest_init (type, compound_literal, NULL);
- return compound_literal;
+ return build_compound_literal (type, compound_literal);
}
/* Return the declaration for the function-name variable indicated by
Index: gcc/cp/tree.c
===
RCS file: /cvs/gcc/gcc/gcc/cp/tree.c,v
retrieving revision 1.427
dif
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-08
20:44 ---
Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable
types
On Mar 8, 2005, Mark Mitchell <[EMAIL PROTECTED]> wrote:
> No, because there would be no TARGET_EXPR. In a template, you would
> just have a COMPOUND_LITERAL_EXPR, with no TARGET_EXPR, because we
> want a syntactic representation of the input program.
> Yes, and introducing TARGET_EXPRs into templates *is a bug* because in
> templates we want a syntactic representation. The closest thing we
> have to a syntactic representation of a compound literal is a
> CONSTRUCTOR; it's certainly not a TARGET_EXPR wrapped around a
> CONSTRUCTOR. It may be just fine to use CONSTRUCTOR, instead of
> introducing COMPOUND_LITERAL_EXPR, but TARGET_EXPRs should not be
> appearing here at all.
> Unfortunately, you've also caused me to think about this long enough
> to realize that having the TARGET_EXPR here is wrong in the first
> place, as per above.
Okie dokie, how about this?
The change to the gimplify.c is needed to avoid having
gimple_add_tmp_var twice for the variable, once while expanding the
declaration/initialization, once while expanding the clean-ups.
Ok to install? Passed check-g++ (except for the already-broken
eh/cleanup1.C) on x86_64-linux-gnu; just started bootstrap and full
reg-testing.
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/20103
* gimplify.c (gimplify_decl_expr): Don't add temp variable if it
was already seen in a bind expr.
Index: gcc/gimplify.c
===
RCS file: /cvs/gcc/gcc/gcc/gimplify.c,v
retrieving revision 2.115
diff -u -p -r2.115 gimplify.c
--- gcc/gimplify.c 8 Mar 2005 13:56:57 - 2.115
+++ gcc/gimplify.c 8 Mar 2005 20:33:03 -
@@ -1047,7 +1047,8 @@ gimplify_decl_expr (tree *stmt_p)
/* This decl isn't mentioned in the enclosing block, so add it to the
list of temps. FIXME it seems a bit of a kludge to say that
anonymous artificial vars aren't pushed, but everything else is. */
- if (DECL_ARTIFICIAL (decl) && DECL_NAME (decl) == NULL_TREE)
+ if (DECL_ARTIFICIAL (decl) && DECL_NAME (decl) == NULL_TREE
+ && !DECL_SEEN_IN_BIND_EXPR_P (decl))
gimple_add_tmp_var (decl);
}
@@ -2123,7 +2124,8 @@ gimple_boolify (tree expr)
*EXPR_P should be stored. */
static enum gimplify_status
-gimplify_cond_expr (tree *expr_p, tree *pre_p, tree *post_p, tree target)
+gimplify_cond_expr (tree *expr_p, tree *pre_p, tree *post_p, tree target,
+ fallback_t fallback)
{
tree expr = *expr_p;
tree tmp, tmp2, type;
@@ -2137,18 +2139,40 @@ gimplify_cond_expr (tree *expr_p, tree *
the arms. */
else if (! VOID_TYPE_P (type))
{
+ tree result;
+
if (target)
{
ret = gimplify_expr (&target, pre_p, post_p,
is_gimple_min_lval, fb_lvalue);
if (ret != GS_ERROR)
ret = GS_OK;
- tmp = target;
+ result = tmp = target;
tmp2 = unshare_expr (target);
}
+ else if ((fallback & fb_lvalue) == 0)
+ {
+ result = tmp2 = tmp = create_tmp_var (TREE_TYPE (expr), "iftmp");
+ ret = GS_ALL_DONE;
+ }
else
{
- tmp2 = tmp = create_tmp_var (TREE_TYPE (expr), "iftmp");
+ tree type = build_pointer_type (TREE_TYPE (expr));
+
+ if (TREE_TYPE (TREE_OPERAND (expr, 1)) != void_type_node)
+ TREE_OPERAND (expr, 1) =
+ build_fold_addr_expr (TREE_OPERAND (expr, 1));
+
+ if (TREE_TYPE (TREE_OPERAND (expr, 2)) != void_type_node)
+ TREE_OPERAND (expr, 2) =
+ build_fold_addr_expr (TREE_OPERAND (expr, 2));
+
+ tmp2 = tmp = create_tmp_var (type, "iftmp");
+
+ expr = build (COND_EXPR, void_type_node, TREE_OPERAND (expr, 0),
+ TREE_OPERAND (expr, 1), TREE_OPERAND (expr, 2));
+
+ result = build_fold_indirect_ref (tmp);
ret = GS_ALL_DONE;
}
@@ -2169,7 +2193,7 @@ gimplify_cond_expr (tree *expr_p, tree *
/* Move the COND_EXPR to the prequeue. */
gimplify_and_add (expr, pre_p);
- *expr_p = tmp;
+ *expr_p = result;
return ret;
}
@@ -2907,7 +2931,8 @@ gimplify_modify_expr_rhs (tree *expr_p,
if (!is_gimple_reg_type (TREE_TYPE (*from_p)))
{
*expr_p = *from_p;
- return gimplify_cond_expr (expr_p, pre_p, post_p, *to_p);
+ return gimplify_cond_expr (expr_p, pre_p, post_p, *to_p,
+ fb_rvalue);
}
else
ret = GS_UNHANDLED;
@@ -3721,7 +3746,8 @@ gimplify_expr (tree *expr_p, tree *pre_p
break;
case COND_EXPR:
- ret = gimplify_cond_expr (expr_p, pre_p, post_p, NULL_TREE);
+ ret = gimplify
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-08 07:24 --- Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable types On Mar 7, 2005, Mark Mitchell <[EMAIL PROTECTED]> wrote: > (a) we should never use "==" to compare types, because there's no > guarantee that the compiler will continue to use "==" types in places > where it does at present; The guarantee is the code in get_target_expr. That's also the only case in which type equality preserving matters, and even then, only when INITIAL is a CONSTRUCTOR. For all other cases, if preserving such equality mattered, we'd have had to handle those cases before at the point I'm handling them now. > Think about it this way: == has no semantic meaning in C++ Indeed, it's a front-end implementation detail. It should be fine to take advantage of it in the front-end. It's not about the language. The TARGET_EXPR itself is just working around a constraint imposed by the gimplifier. > So, using == means that you're doing something with types that doesn't > have semantics grounded in the language, which doesn't make sense, > except in places that are trying to get debugging information correct, > etc. So think of it this way: if we adopted COMPOUND_LITERAL_EXPR like you're inclined to do, we'd have to do the very same kind of type propagation after resolving the complete type of the initializer. This is no different from what I'm proposing to do here. And there's more: since there's no compiler-enforced equality AFAICT between the type of the COMPOUND_LITERAL_EXPR and that of the VAR_DECL enclosed in it, we'd have o make the propagation conditional in just the same way. > (b) you're applying a non-uniform transformation depending on > non-local knowledge. What I mean by "non-uniform" is that the > assignment to the type of the TARGET_EXPR is conditional. Conditional as in, if a condition held before, make sure it holds after. It really doesn't sound non-uniform to me. Note that we never needed this condition to hold before; TARGET_EXPRs just were not handled at all: we never emitted TARGET_EXPRs whose types were arrays of unknown bounds, to be inferred from the initializer length, before this change. > If it were an unconditional assignment, I'd be less nervous; Can't you just think of it as if we had, instead of TARGET_EXPR, a TARGET_EXPR_WITH_INITIALIZER_TYPE and TARGET_EXPR_WITH_DIFFERENT_TYPE, and the statement was unconditional for the former, and not present for the latter? This is a precise description of the properties at hand. Now what if, instead of using up two separate tree nodes, we use a single tree node for them, and use a comparison between the type of the initializer and the type of the target_expr to tell which case we have? Hey!, but this is *exactly* what the patch does! > then, you'd be asserting that the type of the TARGET_EXPR should > always be the type of its initializer, which might make sense. Further investigation has shown that TARGET_EXPR's SLOTs always have the same type as the enclosing TARGET_EXPR. Whether SLOTs and INITIALs always have the same type is not as obvious, but it appears to hold as well. How about making the assignments unconditional, then, with an assertion that the equality holds? > What I mean by "non-local" is that your transformation only works > because of some far-off logic that determins how TARGET_EXPRs are > created. No, because of some local preservation of a property. Not preserving it when it was present does break code. Introducing it when it wasn't there before might break code. So the right thing to do is, obviously, to test for the property, and preserve it if it was present. There isn't any non-local property here: the decision is entirely local, and there's no hidden dependency on anything elsewhere. It's as simple as: if both entities pointed to the same thing, make sure they still do afterwards. How can this possibly not be a reasonable thing to do? > It doesn't depend on any documented property of TARGET_EXPRs that is > enforced throughout the compiler. It's precisely because the property is not documented that it's tested locally. Perhaps the property is guaranteed by the current implementation, I can't quite tell for sure. But testing for the property and propagating it into the substed template feels like the very right thing to do for some property that isn't necessarily guaranteed. > Consider the comments you should write for your code: How about: /* If the type of the initializer was used to create the original TARGET_EXPR, make sure we adjust the type of the tsubsted TARGET_EXPR, should the type of the initializer change in unpredictable ways during tsubsting (e.g., the range of an array is inferred from a CONSTRUCTOR length). */ See? No need to change any other piece of code anywhere else. It's really that simple. > I'm sorry you're frustruated, but I don't think that your approach is
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-07 21:58 --- Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable types On Mar 7, 2005, Mark Mitchell <[EMAIL PROTECTED]> wrote: >>> The games that you want to play with type-equality are just too fragile. >> I still don't see why. > First, you're using "==". As I've told you, that's incredibly > fragile. Not for the purpose I've been trying to describe. The condition I want to maintain is that, if the TARGET_EXPR had the type of its INITIAL before template substitution, then it must have the type of its INITIAL after template substitution, because whatever transformations the INITIAL type might undergo won't be applied to the TARGET_EXPR. It's that simple, so it should be quite obvious that `==' is what I'm looking for. It's not just same_type_p; if the types of the TARGET_EXPR was not taken from the INITIAL, then I don't care how it evolves. > You're depending on a very non-local property that in the > case that you're interested in, the types will always be ==. No, I'm only guaranteeing that, if this property held before template substitution, it still holds afterwards. If it didn't hold before, it still won't hold afterwards. It's much simpler than what you describe. > But, minor changes elsewhere might make them same_type_p, but not > ==, in some cases. If that's fine for those cases, it will remain so after template substitution. Sounds *exactly* like what I want. > Second, you're applying a non-uniform manipulation on the types of the > TARGET_EXPR, based on a non-local property about how TARGET_EXPRs are > created, without actually checking that the condition you're > interested in (incomplete array types) applies. Huh?!? No, I'm not. Read the code again. It goes like this: if the decl type and the initializer type were the same before, make them the same after if the target_expr type and the decl type were the same before, make them the same after and that's it. It's that simple. Nothing non-uniform about it. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20103
[Bug target/20126] [3.3/3.4/4.0/4.1 Regression] Inlined memcmp makes one argument null on entry
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-07
21:57 ---
Subject: [PR target/20126, RFC] loop DEST_ADDR biv replacement may fail
loop attempts to eliminate a biv represented by a pseudo in favor of a
giv represented by (plus (reg) (const_int -1)). Unfortunately, the
biv pseudo appears in an insn that doesn't accept anything but a
register, so validate_change fails and the error goes unnoticed.
The patch below fixes the problem and passed bootstrap on
x86_64-linux-gnu (testing still pending), but I'm not very comfortable
with the use of location for the assignment. Is this a safe change?
Any loop experts around willing to take a second look on this? Thanks
in advance,
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR target/20126
* loop.c (loop_givs_rescan): If replacement of DEST_ADDR failed,
set the original address pseudo to the correct value before the
original insn and leave the insn alone.
Index: gcc/loop.c
===
RCS file: /cvs/gcc/gcc/gcc/loop.c,v
retrieving revision 1.522
diff -u -p -r1.522 loop.c
--- gcc/loop.c 17 Jan 2005 08:46:15 - 1.522
+++ gcc/loop.c 7 Mar 2005 21:37:43 -
@@ -5470,9 +5470,16 @@ loop_givs_rescan (struct loop *loop, str
mark_reg_pointer (v->new_reg, 0);
if (v->giv_type == DEST_ADDR)
- /* Store reduced reg as the address in the memref where we found
- this giv. */
- validate_change (v->insn, v->location, v->new_reg, 0);
+ {
+ /* Store reduced reg as the address in the memref where we found
+this giv. */
+ if (! validate_change (v->insn, v->location, v->new_reg, 0))
+ /* Not replaceable; emit an insn to set the original giv reg from
+ the reduced giv. */
+ v->insn = loop_insn_emit_before (loop, 0, v->insn,
+gen_move_insn (*v->location,
+ v->new_reg));
+ }
else if (v->replaceable)
{
reg_map[REGNO (v->dest_reg)] = v->new_reg;
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20126
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-07 17:05 --- Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable types On Mar 7, 2005, Mark Mitchell <[EMAIL PROTECTED]> wrote: > Are you sure that we can use TARGET_EXPR as a type-conversion node? Actually, no. I was led to believe so because there is a function that creates a TARGET_EXPR given an initializer and a type, in addition to the one that takes the type from the initializer. > I would think in that case that the INITIAL for the TARGET_EXPR > would be a call to the derived class constructor. I was thinking references, actually, so there wouldn't be a constructor involved. I.e., I was trying to preserve the earlier behavior of TARGET_EXPRs (i.e., mostly do nothing with them), while adjusting the behavior only as much as needed for this new use. >> We'd have to create a new tree node type for compound literals to >> be able to call finish_compound_literal at that point. > Then we really should do that. Eek. What for? All we need to do is adjust its type. A new tree node scattered all over the place feels like way too much overhead for this. > The games that you want to play with type-equality are just too fragile. I still don't see why. All I am doing is ensuring the equality is maintained if it existed. If they weren't equal in the first place, I just don't change anything. As a result, in the case in which I know the equality is important and present, it will be preserved. In other cases, we get the behavior we had before. Why are you so uncomfortable with it? Is it just the general thought that `pointer equality between type tree nodes is bad´, before actually looking into the problem it's trying to address, or other reasons pertaining to this particular issue? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20103
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-07 14:44 --- Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable types On Mar 7, 2005, Mark Mitchell <[EMAIL PROTECTED]> wrote: > Alexandre Oliva wrote: >>> This doesn't look quite right. First, we're trying to get rid of >>> tsubst_copy; we should not add new calls. You should do the RECURs >>> here, and then build up the new node. >> I'd have to use build3 (TARGET_EXPR...) or introduce a new call to >> create a target_expr with given slot, initial and cleanup, because >> AFAICT there isn't any that takes a cleanup. > Yes, you should use build3. Ok. >> They don't, and they can't without this piece of code. When we tsubst >> INITIAL, an incomplete array type (T[]), that had been used as the >> type of the slot and the target_expr itself in >> finish_compound_literal(), called while processing a template, >> digest_init() (or something else; I no longer remember exactly) >> completes the array type with the number of entries in the INITIAL >> CONSTRUCTOR. > Then you should tsubst the INITIAL first, and unconditionally copy the > type to the TARGET_EXPR when you use build3. But what if the TARGET_EXPR had been created for different purposes, and did have a different type than that of the initializer? Say, a Base& being bound to a Derived TARGET_EXPR? That's why I'm performing the tests the way I am. > Or, even better, call > the same functions in semantics.c that the parser would call if not in > a template. In other words, call finish_compound_literal again, from > pt.c. That's the overall design: we try to reuse the same semantic > routines again at template instantiation time. Yeah, I know we'd like to do that, but we can't. At that point we have no clue what that TARGET_EXPR or the CONSTRUCTOR in its initial came from. We'd have to create a new tree node type for compound literals to be able to call finish_compound_literal at that point. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20103
[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-07 03:28 --- Subject: Re: [PR c++/19199] don't turn cond_expr lvalue into min_expr rvalue (continued from PR c++/20280) On Mar 5, 2005, Mark Mitchell <[EMAIL PROTECTED]> wrote: > Roger has objected to this change in the past. As I noted in the > audit trail for 19199, there's stuff in build_modify_expr to handle > MIN_EXPR/MAX_EXPR as lvalues Problem is, with the transformation performed by fold, it's no longer an lvalue, and forcing it back into an lvalue just because it looks like it should be one could break other programs. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-07
03:26 ---
Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable
types
On Mar 6, 2005, Mark Mitchell <[EMAIL PROTECTED]> wrote:
> Alexandre Oliva wrote:
>> +case TARGET_EXPR:
>> + {
>> +tree r = tsubst_copy (t, args, complain, in_decl);
>> +
>> +TREE_TYPE (r) = RECUR (TREE_TYPE (t));
>> +TARGET_EXPR_SLOT (r) = RECUR (TARGET_EXPR_SLOT (t));
>> +TARGET_EXPR_INITIAL (r) = RECUR (TARGET_EXPR_INITIAL (t));
>> +TARGET_EXPR_CLEANUP (r) = RECUR (TARGET_EXPR_CLEANUP (t));
>> +
>> +if (TREE_TYPE (TARGET_EXPR_SLOT (t))
>> +== TREE_TYPE (TARGET_EXPR_INITIAL (t)))
>> + TREE_TYPE (TARGET_EXPR_SLOT (r)) =
>> +TREE_TYPE (TARGET_EXPR_INITIAL (r));
>> +
>> +if (TREE_TYPE (t) == TREE_TYPE (TARGET_EXPR_SLOT (t)))
>> + TREE_TYPE (r) = TREE_TYPE (TARGET_EXPR_SLOT (r));
>> +
>> +return r;
> This doesn't look quite right. First, we're trying to get rid of
> tsubst_copy; we should not add new calls. You should do the RECURs
> here, and then build up the new node.
I'd have to use build3 (TARGET_EXPR...) or introduce a new call to
create a target_expr with given slot, initial and cleanup, because
AFAICT there isn't any that takes a cleanup.
> And, the manipulations of TREE_TYPE don't make sense: (a) using "=="
> to compare types is almost always wrong,
It's right in the very case I care about, see below.
> and (b) the RECURs should already maintain the invariant you're
> trying to maintain.
They don't, and they can't without this piece of code. When we tsubst
INITIAL, an incomplete array type (T[]), that had been used as the
type of the slot and the target_expr itself in
finish_compound_literal(), called while processing a template,
digest_init() (or something else; I no longer remember exactly)
completes the array type with the number of entries in the INITIAL
CONSTRUCTOR. So what I'm doing here is propagating the completed type
to the SLOT and the TARGET_EXPR itself, otherwise their types would
remain incomplete, and errors would ensue. This ensures that we get
the same result as that of finish_compound_literal() when not
compiling a template.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20103
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-06
07:29 ---
Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable
types
On Mar 5, 2005, Mark Mitchell <[EMAIL PROTECTED]> wrote:
> Alexandre Oliva wrote:
>> Testing now. I was a bit surprised that the casts to (const B&)
>> weren't reported as faulty, but didn't check the standard on it.
> I'd have to check the rules -- but I'm sure it's not a problem with
> your patch. Either its correct, or an already-present bug.
>> Ok
>> to install if testing passes?
> Yes.
Unfortunately, g++.dg/template/ctor1.C and g++.dg/template/complit1.C
regressed. As it turned out, complit1.C was in error, and had to be
adjusted to match g++.dg/ext/complit1.C, since the only significant
difference between them was that one of them had the compound-literal
within a ctor of a template class.
ctor1.C, however, exposed a failure to handle TARGET_EXPRs while
instantiating templates. So I extended the tsubst machinery to do it,
and now ctor1.C passes again.
I also noticed that the patch fixes g++.old-deja/g++.oliva/expr2.C,
yay!
Here's the latest version of the patch, that so far passed make -C gcc
all check-g++. Full bootstrap and regtest on x86_64-linux-gnu still
pending. Ok to install if it passes?
Index: gcc/cp/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/20103
* semantics.c (finish_compound_literal): Wrap it in a
target_expr.
* pt.c (tsubst_copy_and_build): Handle TARGET_EXPR.
Index: gcc/cp/semantics.c
===
RCS file: /cvs/gcc/gcc/gcc/cp/semantics.c,v
retrieving revision 1.463
diff -u -p -r1.463 semantics.c
--- gcc/cp/semantics.c 23 Feb 2005 05:30:48 - 1.463
+++ gcc/cp/semantics.c 6 Mar 2005 07:12:11 -
@@ -1996,7 +1996,9 @@ finish_compound_literal (tree type, tree
complete_array_type (type, compound_literal, 1);
}
- return compound_literal;
+ /* A compound-literal is an lvalue in C, but is it going to be
+ in ISO C++? Assume it's an rvalue for now. */
+ return get_target_expr (compound_literal);
}
/* Return the declaration for the function-name variable indicated by
Index: gcc/cp/pt.c
===
RCS file: /cvs/gcc/gcc/gcc/cp/pt.c,v
retrieving revision 1.978
diff -u -p -r1.978 pt.c
--- gcc/cp/pt.c 21 Feb 2005 23:12:26 - 1.978
+++ gcc/cp/pt.c 6 Mar 2005 07:12:19 -
@@ -8744,6 +8744,26 @@ tsubst_copy_and_build (tree t,
return build_throw
(RECUR (TREE_OPERAND (t, 0)));
+case TARGET_EXPR:
+ {
+ tree r = tsubst_copy (t, args, complain, in_decl);
+
+ TREE_TYPE (r) = RECUR (TREE_TYPE (t));
+ TARGET_EXPR_SLOT (r) = RECUR (TARGET_EXPR_SLOT (t));
+ TARGET_EXPR_INITIAL (r) = RECUR (TARGET_EXPR_INITIAL (t));
+ TARGET_EXPR_CLEANUP (r) = RECUR (TARGET_EXPR_CLEANUP (t));
+
+ if (TREE_TYPE (TARGET_EXPR_SLOT (t))
+ == TREE_TYPE (TARGET_EXPR_INITIAL (t)))
+ TREE_TYPE (TARGET_EXPR_SLOT (r)) =
+ TREE_TYPE (TARGET_EXPR_INITIAL (r));
+
+ if (TREE_TYPE (t) == TREE_TYPE (TARGET_EXPR_SLOT (t)))
+ TREE_TYPE (r) = TREE_TYPE (TARGET_EXPR_SLOT (r));
+
+ return r;
+ }
+
case CONSTRUCTOR:
{
tree r;
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/20103
* g++.dg/tree-ssa/pr20103.C: New.
* g++.dg/template/complit1.C: Expect error like ext/complit1.C.
Index: gcc/testsuite/g++.dg/tree-ssa/pr20103.C
===
RCS file: gcc/testsuite/g++.dg/tree-ssa/pr20103.C
diff -N gcc/testsuite/g++.dg/tree-ssa/pr20103.C
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/g++.dg/tree-ssa/pr20103.C 6 Mar 2005 07:12:35 -
@@ -0,0 +1,59 @@
+// PR c++/20103
+
+// { dg-do compile }
+
+// { dg-options "" }
+
+// Gimplification used to fail for (B){x}, because create_tmp_var
+// required a non-addressable type, and we didn't wrap the constructor
+// in a target_expr, ensuring it's moved to a separate decl.
+
+// Whether it is an lvalue, like in C, or an rvalue, is up to the ISO
+// C++ Commitete to decide in the second version of the C++ Standard.
+// We're going with rvalues for now.
+
+struct A
+{
+A(const A&);
+};
+
+struct B
+{
+A a;
+};
+
+void foo(B);
+void bar(B&); // { dg-error "in passing argument" }
+void bac(const B&);
+void bap(const B*);
+
+void baz(A &x)
+{
+foo ((B){x});
+bar ((B){x}); // { dg-error "non-const reference" }
+bac ((B){x});
+bap (&(B){x}); // { dg-error "address of temporary" }
+
+foo ((const B&)(B){x});
+bar ((B&)(B){x}); // { dg-error "invalid cast" }
+bac ((const B&)(B){x});
+bap (&(const B&)(B){x});
+}
+
+template
+void baz(T &x)
+{
+foo ((U){x});
+bar ((U){x}); // { dg-error "non-const reference" }
+bac ((U){x});
+bap
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-05
14:03 ---
Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable
types
On Mar 5, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote:
> On Mar 4, 2005, Mark Mitchell <[EMAIL PROTECTED]> wrote:
>>> +foo ((B){x});
>> I don't think (B){x} should be an lvalue, C99 notwithstanding. B(3)
>> is not be an lavalue; I don't see why "(B){x}" should be.
> Works for me. We can always extend it later, should the ISO C++
> committee make a decision different from ours.
> Patch will follow hopefully later today.
For small values of later :-)
Testing now. I was a bit surprised that the casts to (const B&)
weren't reported as faulty, but didn't check the standard on it. Ok
to install if testing passes?
Index: gcc/cp/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/20103
* semantics.c (finish_compound_literal): Wrap it in a
target_expr.
Index: gcc/cp/semantics.c
===
RCS file: /cvs/gcc/gcc/gcc/cp/semantics.c,v
retrieving revision 1.463
diff -u -p -r1.463 semantics.c
--- gcc/cp/semantics.c 23 Feb 2005 05:30:48 - 1.463
+++ gcc/cp/semantics.c 5 Mar 2005 13:56:17 -
@@ -1996,7 +1996,9 @@ finish_compound_literal (tree type, tree
complete_array_type (type, compound_literal, 1);
}
- return compound_literal;
+ /* A compound-literal is an lvalue in C, but is it going to be in
+ ISO C++? Assume it's an rvalue for now. */
+ return get_target_expr (compound_literal);
}
/* Return the declaration for the function-name variable indicated by
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
* g++.dg/tree-ssa/pr20103.C: New.
Index: gcc/testsuite/g++.dg/tree-ssa/pr20103.C
===
RCS file: gcc/testsuite/g++.dg/tree-ssa/pr20103.C
diff -N gcc/testsuite/g++.dg/tree-ssa/pr20103.C
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/g++.dg/tree-ssa/pr20103.C 5 Mar 2005 13:56:31 -
@@ -0,0 +1,41 @@
+// PR c++/20103
+
+// { dg-do compile }
+
+// { dg-options "" }
+
+// Gimplification used to fail for (B){x}, because create_tmp_var
+// required a non-addressable type, and we didn't wrap the constructor
+// in a target_expr, ensuring it's moved to a separate decl.
+
+// Whether it is an lvalue, like in C, or an rvalue, is up to the ISO
+// C++ Commitete to decide in the second version of the C++ Standard.
+// We're going with rvalues for now.
+
+struct A
+{
+A(const A&);
+};
+
+struct B
+{
+A a;
+};
+
+void foo(B);
+void bar(B&); // { dg-error "in passing argument" }
+void bac(const B&);
+void bap(const B*);
+
+void baz(A &x)
+{
+foo ((B){x});
+bar ((B){x}); // { dg-error "non-const reference" }
+bac ((B){x});
+bap (&(B){x}); // { dg-error "address of temporary" }
+
+foo ((const B&)(B){x});
+bar ((B&)(B){x}); // { dg-error "invalid cast" }
+bac ((const B&)(B){x});
+bap (&(const B&)(B){x});
+}
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20103
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-05
13:36 ---
Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable
types
On Mar 4, 2005, Mark Mitchell <[EMAIL PROTECTED]> wrote:
>> +foo ((B){x});
> I don't think (B){x} should be an lvalue, C99 notwithstanding. B(3)
> is not be an lavalue; I don't see why "(B){x}" should be.
Works for me. We can always extend it later, should the ISO C++
committee make a decision different from ours.
Patch will follow hopefully later today.
> Has there been any discussion of this in the ISO committee? Or prior
> are in other compilers? Including previous versions of G++?
Not that I know.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20103
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-04
23:22 ---
Subject: Re: [PR c++/20103] failure to gimplify constructors for addressable
types
On Mar 3, 2005, Andrew Pinski <[EMAIL PROTECTED]> wrote:
> I think this is the wrong approach. The front-end and not
> the gimplifier should be creating these temporaries, I mentioned
> this already in the bug.
How about this?
I tried with the TARGET_EXPR by itself, but it failed to be recognized
as an lvalue, so I introduced the compound expr.
Testing on x86_64-linux-gnu. Ok to install if it passes?
Index: gcc/cp/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/20103
* semantics.c (finish_compound_literal): Ensure the result is an
lvalue, by creating a compound-expr with a target-expr and its
decl.
Index: gcc/cp/semantics.c
===
RCS file: /cvs/gcc/gcc/gcc/cp/semantics.c,v
retrieving revision 1.463
diff -u -p -r1.463 semantics.c
--- gcc/cp/semantics.c 23 Feb 2005 05:30:48 - 1.463
+++ gcc/cp/semantics.c 4 Mar 2005 23:17:50 -
@@ -1996,7 +1996,11 @@ finish_compound_literal (tree type, tree
complete_array_type (type, compound_literal, 1);
}
- return compound_literal;
+ /* A compound-literal is an lvalue in C, so make it so in C++ as
+ well. */
+ compound_literal = get_target_expr (compound_literal);
+ return build2 (COMPOUND_EXPR, TREE_TYPE (compound_literal),
+compound_literal, TARGET_EXPR_SLOT (compound_literal));
}
/* Return the declaration for the function-name variable indicated by
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
* g++.dg/tree-ssa/pr20103.C: New.
Index: gcc/testsuite/g++.dg/tree-ssa/pr20103.C
===
RCS file: gcc/testsuite/g++.dg/tree-ssa/pr20103.C
diff -N gcc/testsuite/g++.dg/tree-ssa/pr20103.C
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/g++.dg/tree-ssa/pr20103.C 4 Mar 2005 23:18:05 -
@@ -0,0 +1,34 @@
+// PR c++/20103
+
+// { dg-do compile }
+
+// { dg-options "" }
+
+// Gimplification used to fail for (B){x}, because create_tmp_var
+// required a non-addressable type, and we didn't wrap the constructor
+// in a target_expr, ensuring it's taken as an lvalue.
+
+struct A
+{
+A(const A&);
+};
+
+struct B
+{
+A a;
+};
+
+void foo(B);
+void bar(B&);
+void bap(B*);
+
+void baz(A &x)
+{
+foo ((B){x});
+bar ((B){x});
+bap (&(B){x});
+
+foo ((const B&)(B){x});
+bar ((B&)(B){x});
+bap (&(B&)(B){x});
+}
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20103
[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-04
19:23 ---
Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs
On Mar 4, 2005, Mark Mitchell <[EMAIL PROTECTED]> wrote:
> Your reading is logical, but it depends on exactly what "lvalue for a
> bit-field" means. (Note that it does not say "lvalue *is* a
> bit-field"; it says "lvalue *for* a bit-field".)
Good point. Here's an all-new patch, with the comment updated to
reflect our discussion.
Still testing on x86_64-linux-gnu, ok to install if it succeeds?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/20280
* gimplify.c (gimplify_cond_expr): Add fallback argument. Use a
temporary variable of pointer type if an lvalues is required.
(gimplify_modify_expr_rhs): Request an rvalue from it.
(gimplify_expr): Pass fallback on.
Index: gcc/gimplify.c
===
RCS file: /cvs/gcc/gcc/gcc/gimplify.c,v
retrieving revision 2.113
diff -u -p -r2.113 gimplify.c
--- gcc/gimplify.c 18 Feb 2005 19:35:37 - 2.113
+++ gcc/gimplify.c 4 Mar 2005 19:18:49 -
@@ -2123,7 +2123,8 @@ gimple_boolify (tree expr)
*EXPR_P should be stored. */
static enum gimplify_status
-gimplify_cond_expr (tree *expr_p, tree *pre_p, tree *post_p, tree target)
+gimplify_cond_expr (tree *expr_p, tree *pre_p, tree *post_p, tree target,
+ fallback_t fallback)
{
tree expr = *expr_p;
tree tmp, tmp2, type;
@@ -2137,18 +2138,40 @@ gimplify_cond_expr (tree *expr_p, tree *
the arms. */
else if (! VOID_TYPE_P (type))
{
+ tree result;
+
if (target)
{
ret = gimplify_expr (&target, pre_p, post_p,
is_gimple_min_lval, fb_lvalue);
if (ret != GS_ERROR)
ret = GS_OK;
- tmp = target;
+ result = tmp = target;
tmp2 = unshare_expr (target);
}
+ else if ((fallback & fb_lvalue) == 0)
+ {
+ result = tmp2 = tmp = create_tmp_var (TREE_TYPE (expr), "iftmp");
+ ret = GS_ALL_DONE;
+ }
else
{
- tmp2 = tmp = create_tmp_var (TREE_TYPE (expr), "iftmp");
+ tree type = build_pointer_type (TREE_TYPE (expr));
+
+ if (TREE_TYPE (TREE_OPERAND (expr, 1)) != void_type_node)
+ TREE_OPERAND (expr, 1) =
+ build_fold_addr_expr (TREE_OPERAND (expr, 1));
+
+ if (TREE_TYPE (TREE_OPERAND (expr, 2)) != void_type_node)
+ TREE_OPERAND (expr, 2) =
+ build_fold_addr_expr (TREE_OPERAND (expr, 2));
+
+ tmp2 = tmp = create_tmp_var (type, "iftmp");
+
+ expr = build (COND_EXPR, void_type_node, TREE_OPERAND (expr, 0),
+ TREE_OPERAND (expr, 1), TREE_OPERAND (expr, 2));
+
+ result = build_fold_indirect_ref (tmp);
ret = GS_ALL_DONE;
}
@@ -2169,7 +2192,7 @@ gimplify_cond_expr (tree *expr_p, tree *
/* Move the COND_EXPR to the prequeue. */
gimplify_and_add (expr, pre_p);
- *expr_p = tmp;
+ *expr_p = result;
return ret;
}
@@ -2907,7 +2930,8 @@ gimplify_modify_expr_rhs (tree *expr_p,
if (!is_gimple_reg_type (TREE_TYPE (*from_p)))
{
*expr_p = *from_p;
- return gimplify_cond_expr (expr_p, pre_p, post_p, *to_p);
+ return gimplify_cond_expr (expr_p, pre_p, post_p, *to_p,
+ fb_rvalue);
}
else
ret = GS_UNHANDLED;
@@ -3721,7 +3745,8 @@ gimplify_expr (tree *expr_p, tree *pre_p
break;
case COND_EXPR:
- ret = gimplify_cond_expr (expr_p, pre_p, post_p, NULL_TREE);
+ ret = gimplify_cond_expr (expr_p, pre_p, post_p, NULL_TREE,
+ fallback);
break;
case CALL_EXPR:
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/20280
* g++.dg/tree-ssa/pr20280.C: New.
Index: gcc/testsuite/g++.dg/tree-ssa/pr20280.C
===
RCS file: gcc/testsuite/g++.dg/tree-ssa/pr20280.C
diff -N gcc/testsuite/g++.dg/tree-ssa/pr20280.C
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/g++.dg/tree-ssa/pr20280.C 4 Mar 2005 19:19:03 -
@@ -0,0 +1,63 @@
+// PR c++/20280
+
+// { dg-do compile }
+
+// Gimplification of the COND_EXPR used to fail because it had an
+// addressable type, and create_tmp_var rejected that.
+
+struct A
+{
+~A();
+};
+
+struct B : A {};
+
+A& foo();
+
+void bar(bool b)
+{
+(B&) (b ? foo() : foo());
+}
+
+// Make sure bit-fields and addressable types don't cause crashes.
+// These were not in the original bug report.
+
+// Added by Alexandre Oliva <[EMAIL PROTECTED]>
+
+// Copyright 2005 Free Software Foundation
+
+struct X
+{
+ long i : 32, j, k : 32;
+};
+
+void g(long&);
+void h(c
[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-04
19:08 ---
Subject: [PR c++/19199] don't turn cond_expr lvalue into min_expr rvalue
(continued from PR c++/20280)
On Mar 4, 2005, Mark Mitchell <[EMAIL PROTECTED]> wrote:
> Actually, looking at this more closely, I think that something is
> forgetting to call rationalize_conditional_expr, which is normally
> called from unary_complex_lvalue. When the conditional expression is
> used in the lvalue context, something should be calling that.
> Normally, that happens because something calls build_unary_op
> (ADDR_EXPR, ...) on the COND_EXPR.
> It may be that I changed something to call build_addr (instead of
> build_unary_op) in a case where that's not safe. Can you confirm or
> deny that hypothesis?
I'm not sure you're still talking about PR 20280, or about PR 19199,
that is marked as a blocker because it appears to be the same problem,
but AFAICT really isn't.
Here's a patch that fixes PR c++/19199, by avoiding the transformation
from:
> > >
>
into:
> > >
since the latter is clearly not an lvalue, and
rationalize_conditional_expr doesn't manage to turn it back into one
(I don't think it should). I realize we might miss some optimization
opportunities because of this change in C, because the optimization
would be valid there, but I suppose we could arrange for cond_expr
operands in C to be forced into rvalues.
Still testing on x86_64-linux-gnu. Ok to install if it passes?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/19199
* fold-const.c (non_lvalue): Split tests into...
(maybe_lvalue_p): New function.
(fold_ternary): Use it to avoid turning a COND_EXPR lvalue into
an MIN_EXPR rvalue.
Index: gcc/fold-const.c
===
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.523
diff -u -p -r1.523 fold-const.c
--- gcc/fold-const.c 4 Mar 2005 06:24:09 - 1.523
+++ gcc/fold-const.c 4 Mar 2005 19:04:26 -
@@ -2004,16 +2004,12 @@ fold_convert (tree type, tree arg)
}
}
-/* Return an expr equal to X but certainly not valid as an lvalue. */
+/* Return false if expr can be assumed to not be an lvalue, true
+ otherwise. */
-tree
-non_lvalue (tree x)
+static bool
+maybe_lvalue_p (tree x)
{
- /* While we are in GIMPLE, NON_LVALUE_EXPR doesn't mean anything to
- us. */
- if (in_gimple_form)
-return x;
-
/* We only need to wrap lvalue tree codes. */
switch (TREE_CODE (x))
{
@@ -2053,8 +2049,24 @@ non_lvalue (tree x)
/* Assume the worst for front-end tree codes. */
if ((int)TREE_CODE (x) >= NUM_TREE_CODES)
break;
-return x;
+return false;
}
+
+ return true;
+}
+
+/* Return an expr equal to X but certainly not valid as an lvalue. */
+
+tree
+non_lvalue (tree x)
+{
+ /* While we are in GIMPLE, NON_LVALUE_EXPR doesn't mean anything to
+ us. */
+ if (in_gimple_form)
+return x;
+
+ if (! maybe_lvalue_p (x))
+return x;
return build1 (NON_LVALUE_EXPR, TREE_TYPE (x), x);
}
@@ -7095,10 +7107,17 @@ fold_ternary (tree expr)
of B and C. Signed zeros prevent all of these transformations,
for reasons given above each one.
+We don't want to use operand_equal_for_comparison_p here,
+because it might turn an lvalue COND_EXPR into an rvalue one,
+see PR c++/19199.
+
Also try swapping the arguments and inverting the conditional. */
if (COMPARISON_CLASS_P (arg0)
- && operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
-arg1, TREE_OPERAND (arg0, 1))
+ && ((maybe_lvalue_p (arg1)
+ && maybe_lvalue_p (TREE_OPERAND (t, 2)))
+ ? operand_equal_p (TREE_OPERAND (arg0, 0), arg1, OEP_ONLY_CONST)
+ : operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
+ arg1, TREE_OPERAND (arg0, 1)))
&& !HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (arg1
{
tem = fold_cond_expr_with_comparison (type, arg0,
@@ -7109,9 +7128,13 @@ fold_ternary (tree expr)
}
if (COMPARISON_CLASS_P (arg0)
- && operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
-TREE_OPERAND (t, 2),
-TREE_OPERAND (arg0, 1))
+ && ((maybe_lvalue_p (arg1)
+ && maybe_lvalue_p (TREE_OPERAND (t, 2)))
+ ? operand_equal_p (TREE_OPERAND (arg0, 0),
+TREE_OPERAND (t, 2), OEP_ONLY_CONST)
+ : operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
+ TREE_OPERAND (t, 2),
+ TREE_OPERAND (arg0, 1)))
&& !HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (TREE_OPERAND (t,
[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-04 06:59 --- Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs On Mar 4, 2005, Alexandre Oliva <[EMAIL PROTECTED]> wrote: >> we should be doing the same for all types (well except for >> bitfields) and not just "addressable" types, > Agreed. That's relatively easy to fix. Rats. Not that easy. A number of regressions showed up with the `improved' patch :-( It has to do with the uses of build_address, that marks variables and fields as addressable and used, so we end up having to emit them, instead of optimizing them out as intended. It seems like we may indeed need something more elaborate at gimplification time, instead of modifying the up-front representation. I'll keep digging. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280
[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-04
06:34 ---
Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs
On Mar 3, 2005, Andrew Pinski <[EMAIL PROTECTED]> wrote:
> On Mar 3, 2005, at 2:50 AM, Alexandre Oliva wrote:
>> I'm bootstrapping this on x86_64-linux-gnu, along with the patch for
>> PR c++/20103; it's also passed C++ regression testing. Ok to install
>> if bootstrap and all-languages regression testing passes?
> I think this is the wrong approach,
Err... But AFAICT this is exactly the approach RTH suggested to cope
with the issue, except for the removal of the unnecessary artificial
decl before gimplification.
> we should be doing the same for all types (well except for
> bitfields) and not just "addressable" types,
Agreed. That's relatively easy to fix.
Improved patch follows. Ok to install?
Index: gcc/cp/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/19199
PR c++/20280
* call.c (build_conditional_expr): Hoist indirect_ref out of
cond_expr if the result is a non-bitfield lvalue.
Index: gcc/cp/call.c
===
RCS file: /cvs/gcc/gcc/gcc/cp/call.c,v
retrieving revision 1.531
diff -u -p -r1.531 call.c
--- gcc/cp/call.c 24 Feb 2005 21:55:10 - 1.531
+++ gcc/cp/call.c 4 Mar 2005 06:32:44 -
@@ -3111,6 +3111,9 @@ build_conditional_expr (tree arg1, tree
tree result = NULL_TREE;
tree result_type = NULL_TREE;
bool lvalue_p = true;
+ bool indirect_p = false;
+ cp_lvalue_kind arg2_lvalue_kind;
+ cp_lvalue_kind arg3_lvalue_kind;
struct z_candidate *candidates = 0;
struct z_candidate *cand;
void *p;
@@ -3288,11 +3291,26 @@ build_conditional_expr (tree arg1, tree
If the second and third operands are lvalues and have the same
type, the result is of that type and is an lvalue. */
- if (real_lvalue_p (arg2)
- && real_lvalue_p (arg3)
+ if ((arg2_lvalue_kind = real_lvalue_p (arg2))
+ && (arg3_lvalue_kind = real_lvalue_p (arg3))
&& same_type_p (arg2_type, arg3_type))
{
- result_type = arg2_type;
+ if ((arg2_lvalue_kind & clk_bitfield) == clk_none
+ && (arg3_lvalue_kind & clk_bitfield) == clk_none)
+ {
+ indirect_p = true;
+ result_type = build_pointer_type (arg2_type);
+ if (TREE_CODE (arg2) == INDIRECT_REF)
+ arg2 = TREE_OPERAND (arg2, 0);
+ else
+ arg2 = fold_if_not_in_template (build_address (arg2));
+ if (TREE_CODE (arg3) == INDIRECT_REF)
+ arg3 = TREE_OPERAND (arg3, 0);
+ else
+ arg3 = fold_if_not_in_template (build_address (arg3));
+ }
+ else
+ result_type = arg2_type;
goto valid_operands;
}
@@ -3458,6 +3476,14 @@ build_conditional_expr (tree arg1, tree
/* We can't use result_type below, as fold might have returned a
throw_expr. */
+ if (indirect_p)
+{
+ if (TREE_CODE (TREE_TYPE (result)) == POINTER_TYPE)
+ result = build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (result)), result);
+ else
+ gcc_assert (TREE_CODE (result) == THROW_EXPR);
+}
+
/* Expand both sides into the same slot, hopefully the target of the
?: expression. We used to check for TARGET_EXPRs here, but now we
sometimes wrap them in NOP_EXPRs so the test would fail. */
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/19199
PR c++/20280
* g++.dg/tree-ssa/pr20280.C: New.
Index: gcc/testsuite/g++.dg/tree-ssa/pr20280.C
===
RCS file: gcc/testsuite/g++.dg/tree-ssa/pr20280.C
diff -N gcc/testsuite/g++.dg/tree-ssa/pr20280.C
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/g++.dg/tree-ssa/pr20280.C 4 Mar 2005 06:32:58 -
@@ -0,0 +1,80 @@
+// PR c++/20280
+
+// { dg-do compile }
+
+// Gimplification of the COND_EXPR used to fail because it had an
+// addressable type, and create_tmp_var rejected that.
+
+struct A
+{
+~A();
+};
+
+struct B : A {};
+
+A& foo();
+
+void bar(bool b)
+{
+(B&) (b ? foo() : foo());
+}
+
+// Make sure bit-fields and addressable types don't cause crashes.
+// These were not in the original bug report.
+
+// Added by Alexandre Oliva <[EMAIL PROTECTED]>
+
+// Copyright 2005 Free Software Foundation
+
+struct X
+{
+ long i : 32, j, k : 32;
+};
+
+void g(long&);
+void h(const long&);
+
+void f(X &x, bool b)
+{
+ (b ? x.i : x.j) = 1;
+ (b ? x.j : x.k) = 2;
+ (b ? x.i : x.k) = 3;
+
+ (void)(b ? x.i : x.j);
+ (void)(b ? x.i : x.k);
+ (void)(b ? x.j : x.k);
+
+ g (b ? x.i : x.j); // { dg-error "cannot bind bitfield" }
+ g (b ? x.i : x.k); // { dg-error "cannot bind bitfield" }
+ g (b ? x.j : x.k); // { dg-error "cannot bind bitfield" }
+
+ // Hmm... I don't think these should be accepted. The conditional
+ // expressions are lvalues for sure, an
[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-04 06:01 --- Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs On Mar 3, 2005, Mark Mitchell <[EMAIL PROTECTED]> wrote: > Alexandre Oliva wrote: > \ >> I went ahead and verified that I didn't break bit-field lvalues in >> conditional expressions (my first attempt did), but I was surprised to >> find out that the calls to h() pass. I understand why they do (we >> create a temporary and bind to that), but I'm not sure this is correct >> behavior. Opinions? >> + // Hmm... I don't think these should be accepted. The conditional >> + // expressions are lvalues for sure, and 8.5.3/5 exempts lvalues >> + // that are bit-fields, but not lvalues that are conditional >> + // expressions involving bit-fields. >> + h (b ? x.i : x.j); >> + h (b ? x.i : x.k); >> + h (b ? x.j : x.k); > That's legal because "h" takes a "const &", which permits the compiler > to create a temporary. Yeah, it permits, but only in certain circumstances that AFAICT aren't met. This expression AFAICT is an lvalue that isn't a bit-field, so it has to bind directly, per the first bullet in 8.5.3/5. Since it meets the conditions of this first bullet, it doesn't get to use the `otherwise' portion of that paragraph, that creates a temporary. Or am I misreading anything? > And, I think these kinds of transformations (if necessary) should be > done in a langhook during gimplification, not at COND_EXPR-creation > time. We really want the C++ front-end's data structures to be an > accurate mirror of the input program for as long as possible. Err... But in what sense does my patch change that? See, what I'm doing is hoisting the indirect_refs that are inserted by stabilize_reference out of the cond_expr. They weren't in the original code. There's no dereferencing going on unless the whole expression undergoes lvalue-to-rvalue decay, so I'd argue that the transformation I'm proposing actually matches even more accurately the meaning of the original source code. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280
[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-03
07:51 ---
Subject: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs
When passing an lvalue cond_expr to a function taking a reference that
binds directly to either operand of ?:, we'd fail gimplification when
the type of the expressions was addressable, because, once again,
create_tmp_var would reject addressable types.
The solution I came up with was to hoist the indirect_ref out of the
cond_expr at the time it was created, such that we create a tmp_var
with a reference to the result of the cond_expr, and when we take the
address of the cond_expr to pass its result by reference, it cancels
out with the indirect_ref, so it works beautifully.
I went ahead and verified that I didn't break bit-field lvalues in
conditional expressions (my first attempt did), but I was surprised to
find out that the calls to h() pass. I understand why they do (we
create a temporary and bind to that), but I'm not sure this is correct
behavior. Opinions?
I'm bootstrapping this on x86_64-linux-gnu, along with the patch for
PR c++/20103; it's also passed C++ regression testing. Ok to install
if bootstrap and all-languages regression testing passes?
Index: gcc/cp/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/20280
* call.c (build_conditional_expr): Hoist indirect_ref out of
cond_expr if the result is an addressable lvalue.
Index: gcc/cp/call.c
===
RCS file: /cvs/gcc/gcc/gcc/cp/call.c,v
retrieving revision 1.531
diff -u -p -r1.531 call.c
--- gcc/cp/call.c 24 Feb 2005 21:55:10 - 1.531
+++ gcc/cp/call.c 3 Mar 2005 07:42:24 -
@@ -3111,6 +3111,7 @@ build_conditional_expr (tree arg1, tree
tree result = NULL_TREE;
tree result_type = NULL_TREE;
bool lvalue_p = true;
+ bool indirect_p = false;
struct z_candidate *candidates = 0;
struct z_candidate *cand;
void *p;
@@ -3292,7 +3293,15 @@ build_conditional_expr (tree arg1, tree
&& real_lvalue_p (arg3)
&& same_type_p (arg2_type, arg3_type))
{
- result_type = arg2_type;
+ if (TREE_ADDRESSABLE (arg2_type) && TREE_ADDRESSABLE (arg3_type))
+ {
+ indirect_p = true;
+ result_type = build_pointer_type (arg2_type);
+ arg2 = fold_if_not_in_template (build_address (arg2));
+ arg3 = fold_if_not_in_template (build_address (arg3));
+ }
+ else
+ result_type = arg2_type;
goto valid_operands;
}
@@ -3458,6 +3467,14 @@ build_conditional_expr (tree arg1, tree
/* We can't use result_type below, as fold might have returned a
throw_expr. */
+ if (indirect_p)
+{
+ if (TREE_CODE (TREE_TYPE (result)) == POINTER_TYPE)
+ result = build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (result)), result);
+ else
+ gcc_assert (TREE_CODE (result) == THROW_EXPR);
+}
+
/* Expand both sides into the same slot, hopefully the target of the
?: expression. We used to check for TARGET_EXPRs here, but now we
sometimes wrap them in NOP_EXPRs so the test would fail. */
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
* g++.dg/tree-ssa/pr20103.C: New.
Index: gcc/testsuite/g++.dg/tree-ssa/pr20280.C
===
RCS file: gcc/testsuite/g++.dg/tree-ssa/pr20280.C
diff -N gcc/testsuite/g++.dg/tree-ssa/pr20280.C
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/g++.dg/tree-ssa/pr20280.C 3 Mar 2005 07:42:38 -
@@ -0,0 +1,62 @@
+// PR c++/20280
+
+// { dg-do compile }
+
+// Gimplification of the COND_EXPR used to fail because it had an
+// addressable type, and create_tmp_var rejected that.
+
+struct A
+{
+~A();
+};
+
+struct B : A {};
+
+A& foo();
+
+void bar(bool b)
+{
+(B&) (b ? foo() : foo());
+}
+
+// Make sure bit-fields and addressable types don't cause crashes.
+// These were not in the original bug report.
+
+// Added by Alexandre Oliva <[EMAIL PROTECTED]>
+
+// Copyright 2005 Free Software Foundation
+
+struct X
+{
+ long i : 32, j, k : 32;
+};
+
+void g(long&);
+void h(const long&);
+
+void f(X &x, bool b)
+{
+ (b ? x.i : x.j) = 1;
+ (b ? x.j : x.k) = 2;
+ (b ? x.i : x.k) = 3;
+
+ (void)(b ? x.i : x.j);
+ (void)(b ? x.i : x.k);
+ (void)(b ? x.j : x.k);
+
+ g (b ? x.i : x.j); // { dg-error "cannot bind bitfield" }
+ g (b ? x.i : x.k); // { dg-error "cannot bind bitfield" }
+ g (b ? x.j : x.k); // { dg-error "cannot bind bitfield" }
+
+ // Hmm... I don't think these should be accepted. The conditional
+ // expressions are lvalues for sure, and 8.5.3/5 exempts lvalues
+ // that are bit-fields, but not lvalues that are conditional
+ // expressions involving bit-fields.
+ h (b ? x.i : x.j);
+ h (b ? x.i : x.k);
+ h (b ? x.j : x.k);
+
+ (long &)(b ? x.i : x.j); // { dg-error "address of bit-field" }
+ (long &)(b ? x.i : x.k); // { dg
[Bug c++/20103] [4.0/4.1 regression] ICE in create_tmp_var with C99 style struct initializer
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-03
07:42 ---
Subject: [PR c++/20103] failure to gimplify constructors for addressable types
In the reduced testcase from the bug report, included in the patch
file below, we fail to gimplify the CONSTRUCTOR created for the
compound-literal expression. When we attempt to create_tmp_var to
hold the B-typed value, it fails because B is addressable.
This patch introduces an alternate entry point for create_tmp_var that
accepts addressable types, and uses that in the relevant caller when
the value is a CONSTRUCTOR.
Bootstrapping on x86_64-linux-gnu, after successful C++ regression
testing on make all. Ok to install if full bootstrap and regression
testing completes?
Index: gcc/gimplify.c
===
RCS file: /cvs/gcc/gcc/gcc/gimplify.c,v
retrieving revision 2.113
diff -u -p -r2.113 gimplify.c
--- gcc/gimplify.c 18 Feb 2005 19:35:37 - 2.113
+++ gcc/gimplify.c 3 Mar 2005 07:40:02 -
@@ -356,15 +356,13 @@ create_tmp_var_raw (tree type, const cha
only from gimplification or optimization, at which point the creation of
certain types are bugs. */
-tree
-create_tmp_var (tree type, const char *prefix)
+static tree
+create_tmp_var_maybe_addressable (tree type, const char *prefix)
{
tree tmp_var;
- /* We don't allow types that are addressable (meaning we can't make copies),
- incomplete, or of variable size. */
- gcc_assert (!TREE_ADDRESSABLE (type)
- && COMPLETE_TYPE_P (type)
+ /* We don't allow types that are incomplete, or of variable size. */
+ gcc_assert (COMPLETE_TYPE_P (type)
&& TREE_CODE (TYPE_SIZE_UNIT (type)) == INTEGER_CST);
tmp_var = create_tmp_var_raw (type, prefix);
@@ -372,6 +370,18 @@ create_tmp_var (tree type, const char *p
return tmp_var;
}
+
+/* Like create_tmp_var_maybe_addressable, but make sure the given type
+ is NOT addressable. */
+
+tree
+create_tmp_var (tree type, const char *prefix)
+{
+ gcc_assert (!TREE_ADDRESSABLE (type));
+
+ return create_tmp_var_maybe_addressable (type, prefix);
+}
+
/* Given a tree, try to return a useful variable name that we can use
to prefix a temporary that is being assigned the value of the tree.
I.E. given = &A, return A. */
@@ -404,6 +414,9 @@ get_name (tree t)
static inline tree
create_tmp_from_val (tree val)
{
+ if (TREE_CODE (val) == CONSTRUCTOR)
+return create_tmp_var_maybe_addressable (TREE_TYPE (val), get_name (val));
+
return create_tmp_var (TREE_TYPE (val), get_name (val));
}
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
* g++.dg/tree-ssa/pr20103.C: New.
Index: gcc/testsuite/g++.dg/tree-ssa/pr20103.C
===
RCS file: gcc/testsuite/g++.dg/tree-ssa/pr20103.C
diff -N gcc/testsuite/g++.dg/tree-ssa/pr20103.C
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/g++.dg/tree-ssa/pr20103.C 3 Mar 2005 07:40:16 -
@@ -0,0 +1,25 @@
+// PR c++/20103
+
+// { dg-do compile }
+
+// { dg-options "" }
+
+// Gimplification used to fail for (B){x}, because create_tmp_var
+// required a non-addressable type.
+
+struct A
+{
+A(const A&);
+};
+
+struct B
+{
+A a;
+};
+
+void foo(B);
+
+void bar(A &x)
+{
+foo((B){x});
+}
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20103
[Bug libgcj/20160] [4.0/4.1 Regression] link errors building libgcj tests
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-01
22:27 ---
Subject: [PR libgcj/20160] rename archive members with duplicate basenames
The archives created for libjava are broken, in that some of the
object files that should go into it are missing. That's because AR
is supposed to drop dirname components of pathnames in archive
members. Libtool was already careful enough to ensure the all archive
members made to the convenience library, by using ar cq if creating
archives piecewise, but it isn't as careful when extracting the
archive members to create other archives with them, so we end up
dropping all but the last-added overlapping-basename object from the
second-generation archive.
This problem is already fixed in libtool mainline and 1.5 branches,
using some clever tricks at extract time, that I'm not entirely
comfortable with, and not quite willing to back-port. Until we
actually upgrade to a newer libtool, I'd rather go with this change
that is IMHO safer, but unfortunately introduces some additional
overhead in archive creation time. Oh well...
I'm checking this in mainline and 4.0 branch. Tested on
arm-elf (thanks Richard!) and x86_64-linux-gnu.
Index: ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR libgcj/20160
* ltmain.sh: Avoid creating archives with components that have
duplicate basenames.
* libjava/configure: Rebuilt.
Index: ltmain.sh
===
RCS file: /cvs/gcc/gcc/ltmain.sh,v
retrieving revision 1.24
diff -u -p -r1.24 ltmain.sh
--- ltmain.sh 8 Sep 2004 15:43:46 - 1.24
+++ ltmain.sh 1 Mar 2005 22:16:48 -
@@ -4307,6 +4307,63 @@ fi\
#fi
# done
+ # POSIX demands no paths to be encoded in archives. We have
+ # to avoid creating archives with duplicate basenames if we
+ # might have to extract them afterwards, e.g., when creating a
+ # static archive out of a convenience library, or when linking
+ # the entirety of a libtool archive into another (currently
+ # not supported by libtool).
+if (for obj in $oldobjs
+ do
+ $echo "X$obj" | $Xsed -e 's%^.*/%%'
+ done | sort | sort -uc >/dev/null 2>&1); then
+ :
+ else
+ $echo "copying selected object files to avoid basename conflicts..."
+
+ if test -z "$gentop"; then
+ gentop="$output_objdir/${outputname}x"
+
+ $show "${rm}r $gentop"
+ $run ${rm}r "$gentop"
+ $show "$mkdir $gentop"
+ $run $mkdir "$gentop"
+ status=$?
+ if test $status -ne 0 && test ! -d "$gentop"; then
+ exit $status
+ fi
+ generated="$generated $gentop"
+ fi
+
+ save_oldobjs=$oldobjs
+ oldobjs=
+ counter=1
+ for obj in $save_oldobjs
+ do
+ objbase=`$echo "X$obj" | $Xsed -e 's%^.*/%%'`
+ case " $oldobjs " in
+ " ") oldobjs=$obj ;;
+ *[\ /]"$objbase "*)
+ while :; do
+ # Make sure we don't pick an alternate name that also
+ # overlaps.
+ newobj=lt$counter-$objbase
+ counter=`expr $counter + 1`
+ case " $oldobjs " in
+ *[\ /]"$newobj "*) ;;
+ *) if test ! -f "$gentop/$newobj"; then break; fi ;;
+ esac
+ done
+ $show "ln $obj $gentop/$newobj || cp $obj $gentop/$newobj"
+ $run ln "$obj" "$gentop/$newobj" ||
+ $run cp "$obj" "$gentop/$newobj"
+ oldobjs="$oldobjs $gentop/$newobj"
+ ;;
+ *) oldobjs="$oldobjs $obj" ;;
+ esac
+ done
+ fi
+
eval cmds=\"$old_archive_cmds\"
if len=`expr "X$cmds" : ".*"` &&
@@ -4320,20 +4377,7 @@ fi\
objlist=
concat_cmds=
save_oldobjs=$oldobjs
- # GNU ar 2.10+ was changed to match POSIX; thus no paths are
- # encoded into archives. This makes 'ar r' malfunction in
- # this piecewise linking case whenever conflicting object
- # names appear in distinct ar calls; check, warn and compensate.
- if (for obj in $save_oldobjs
- do
- $echo "X$obj" | $Xsed -e 's%^.*/%%'
- done | sort | sort -uc >/dev/null 2>&1); then
- :
- else
- $echo "$modename: warning: object name conflicts; overriding
AR_FLAGS to 'cq'" 1>&2
- $echo "$modename: warning: to ensure that POSIX-compatible ar will
work" 1>&2
- AR_FLAGS=cq
- fi
+
for obj in $save_oldobjs
do
oldobjs="$objlist $obj"
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_
[Bug tree-optimization/19786] [4.0 Regression] Aliasing optimisation bug
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-02-21
19:30 ---
Subject: [PR tree-optimization/19786] fix alias grouping lossage
The problem here was that we added type tags to other tag's may-alias
lists without adding them to the corresponding bitmaps. Later on,
when group_aliases performed an union of the bitmaps and discarded the
lists, we lost information about the aliases, which enabled LIM to
hoist a pointer access out of a loop because it appeared to be
invariant, since the VDEF supposed to modify it was missing.
Thanks to Jakub for having isolated the source of the problem, and
Diego for discussing tree-ssa alias analysis with me for a few hours
today.
Here's the patch I'm testing. Ok to install if it bootstraps and
regtests successfully?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR tree-optimization/19786
* tree-ssa-alias.c (compute_flow_insensitive_aliasing): Add one
tag to another's may-alias bitmap when adding to the other's list.
Index: gcc/tree-ssa-alias.c
===
RCS file: /cvs/gcc/gcc/gcc/tree-ssa-alias.c,v
retrieving revision 2.69
diff -u -p -r2.69 tree-ssa-alias.c
--- gcc/tree-ssa-alias.c 17 Feb 2005 16:19:42 - 2.69
+++ gcc/tree-ssa-alias.c 21 Feb 2005 19:15:40 -
@@ -1116,6 +1116,7 @@ compute_flow_insensitive_aliasing (struc
/* Since TAG2 does not have any aliases of its own, add
TAG2 itself to the alias set of TAG1. */
add_may_alias (tag1, tag2);
+ SET_BIT (may_aliases1, var_ann (tag2)->uid);
}
}
}
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR tree-optimization/19786
* g++.dg/tree-ssa/pr19786.C: New.
Index: gcc/testsuite/g++.dg/tree-ssa/pr19786.C
===
RCS file: gcc/testsuite/g++.dg/tree-ssa/pr19786.C
diff -N gcc/testsuite/g++.dg/tree-ssa/pr19786.C
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/g++.dg/tree-ssa/pr19786.C 21 Feb 2005 19:15:54 -
@@ -0,0 +1,48 @@
+// { dg-do run }
+/* { dg-options "-O2" } */
+
+// We used to get alias grouping wrong on this one, hoisting accesses
+// to the vector's end out of the loop.
+
+#include
+#include
+
+struct A
+{
+ double unused; // If I remove it => it works.
+ std::vector v;
+
+ A() : v(1) {}
+};
+
+inline // If not inline => it works.
+A g()
+{
+ A r;
+ r.v.resize(2);
+ r.v[0] = 1;
+
+ while (!r.v.empty() && r.v.back() == 0)
+r.v.pop_back();
+
+ return r;
+}
+
+A f(const A &a)
+{
+ if (a.v.empty()) return a;
+ if (a.v.empty()) return a;
+
+ // A z = g(); return z; // If I return like this => it works.
+ return g();
+}
+
+int main()
+{
+ A a;
+ A b;
+ A r = f(a);
+ assert(r.v.size() != 0);
+
+ return 0;
+}
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19786
[Bug c++/20008] [4.0 Regression] internal compiler error: in expand_case, at stmt.c:2397
--- Additional Comments From aoliva at gcc dot gnu dot org 2005-02-17
16:17 ---
Subject: [PR c++/20008, middle-end] handle switch with all cases out-of-range
Sure enough, the testcase relied on undefined behavior, but that's no
reason for us to ICE at compile time. I suppose it might be nice to
get the tree cfg cleanup code to detect that it can discard cases that
are out of range, but I'm not all that familiar with the cfg cleanup
code, so I figured I'd try this first.
Ok to install if regression testing passes on x86_64-linux-gnu?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/20008
* stmt.c (expand_case): Don't assume cleanup_tree_cfg will remove
cases that are out-of-range for the index type.
Index: gcc/stmt.c
===
RCS file: /cvs/gcc/gcc/gcc/stmt.c,v
retrieving revision 1.412
diff -u -p -r1.412 stmt.c
--- gcc/stmt.c 13 Dec 2004 16:03:38 - 1.412
+++ gcc/stmt.c 17 Feb 2005 16:12:31 -
@@ -1,6 +1,7 @@
/* Expands front end tree to back end RTL for GCC
Copyright (C) 1987, 1988, 1989, 1992, 1993, 1994, 1995, 1996, 1997,
- 1998, 1999, 2000, 2001, 2002, 2003, 2004 Free Software Foundation, Inc.
+ 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005
+ Free Software Foundation, Inc.
This file is part of GCC.
@@ -2393,8 +2394,14 @@ expand_case (tree exp)
BITMAP_XFREE (label_bitmap);
/* cleanup_tree_cfg removes all SWITCH_EXPR with a single
-destination, such as one with a default case only. */
- gcc_assert (count != 0);
+destination, such as one with a default case only. However,
+it doesn't remove cases that are out of range for the switch
+type, so we may still get a zero here. */
+ if (count == 0)
+ {
+ emit_jump (default_label);
+ return;
+ }
/* Compute span of values. */
range = fold (build2 (MINUS_EXPR, index_type, maxval, minval));
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/20008
* g++.dg/opt/switch3.C: New.
Index: gcc/testsuite/g++.dg/opt/switch3.C
===
RCS file: gcc/testsuite/g++.dg/opt/switch3.C
diff -N gcc/testsuite/g++.dg/opt/switch3.C
--- /dev/null 1 Jan 1970 00:00:00 -
+++ gcc/testsuite/g++.dg/opt/switch3.C 17 Feb 2005 16:12:45 -
@@ -0,0 +1,30 @@
+// { dg-do compile }
+
+// PR c++/20008
+
+// We failed to compile this because CFG cleanup left the switch
+// statement intact, whereas expand_case expected at least one
+// in-range case to remain.
+
+typedef enum _SECStatus {
+ SECWouldBlock = -2,
+ SECFailure = -1,
+ SECSuccess = 0
+} SECStatus;
+
+typedef enum {
+ SEC_ERROR_BAD_SIGNATURE = (-0x2000) + 10
+} SECErrorCodes;
+
+void g(void);
+void f(SECStatus status)
+{
+ switch( status )
+{
+case SEC_ERROR_BAD_SIGNATURE :
+ // This case can be optimized away in C++ (but apparently not in
+ // C), because the enum type is defined with a narrow range.
+ g();
+ break ;
+}
+}
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20008
[Bug c++/19143] [4.0 regression] ICE on invalid template parameter
--- Additional Comments From aoliva at redhat dot com 2004-12-29 08:04
---
Subject: Re: New: [4.0 regression] ICE on invalid template parameter
On Dec 23, 2004, "reichelt at gcc dot gnu dot org" <[EMAIL PROTECTED]> wrote:
> The following invalid code snippet triggers an ICE on mainline:
> ==
> template struct A;
> template<> struct A {};
> ==
> bug.cc:2: error: missing '>' to terminate the template argument list
> bug.cc:2: error: template argument 1 is invalid
> bug.cc:2: error: missing '>' to terminate the template argument list
> bug.cc:2: error: template argument 1 is invalid
> bug.cc:2: error: 'A' is not a template
> bug.cc:2: error: missing '>' to terminate the template argument list
> bug.cc:2: internal compiler error: Segmentation fault
> Please submit a full bug report, [etc.]
> Alexandre, this was causes by your path for PR18757:
> http://gcc.gnu.org/ml/gcc-cvs/2004-12/msg00382.html
> Could you please have a look?
This patch fixes it, but I'm not entirely sure this is the best
location for this test. Tested on amd64-linux-gnu, no failures. Ok
to install?
Index: gcc/cp/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
* pt.c (redeclare_class_template): Don't crash when parms
parsing failed.
Index: gcc/cp/pt.c
===
RCS file: /cvs/gcc/gcc/gcc/cp/pt.c,v
retrieving revision 1.962
diff -u -p -r1.962 pt.c
--- gcc/cp/pt.c 23 Dec 2004 19:54:08 - 1.962
+++ gcc/cp/pt.c 29 Dec 2004 07:32:11 -
@@ -3234,6 +3234,11 @@ redeclare_class_template (tree type, tre
type. */
return;
+ /* If the template parameters could not be parsed, assume we already
+ issued an error and bail out without crashing. */
+ if (! parms)
+return;
+
parms = INNERMOST_TEMPLATE_PARMS (parms);
tmpl_parms = DECL_INNERMOST_TEMPLATE_PARMS (tmpl);
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19143
[Bug bootstrap/14905] 'make install' fails on grepjar.1, not included in tarball
--- Additional Comments From aoliva at redhat dot com 2004-04-12 17:23 --- Subject: Re: 'make install' fails on grepjar.1, not included in tarball [v2] On Apr 12, 2004, Kelley Cook <[EMAIL PROTECTED]> wrote: > Now also tested under GCC 3.4 with and without > --enable-generated-files-in-srcdir. Please be sure to test with and without it both with and without the generated files already in srcdir. > * configure.ac: Parse --enable-generated-files-in-srcdir. > * Makefile.am: Copy man and info files to srcdir if requested. > * configure: Regenerate. > * Makefile.in Regenerate. Other than the VPATH issue with Solaris make, that we probably don't have to worry about, it looks ok to me. BTW, why are you marking the configure.ac changes as GCC LOCAL, but not the Makefile.am ones? -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=14905
