Bug 26004 - [4.1 Regression] gcc errors on valid code [SVO]
|
Bug#:
26004
|
Product: gcc
|
Version: 4.2.0
|
|
Host:
|
Target:
|
Build:
|
|
Status: RESOLVED
|
Severity: normal
|
Priority: P2
|
|
Resolution: FIXED
|
Assigned To: jason@gcc.gnu.org
|
Reported By: fnf@specifix.com
|
|
Component: middle-end
|
Target Milestone: 4.1.1
|
|
Summary: [4.1 Regression] gcc errors on valid code [SVO]
|
|
Keywords: rejects-valid
|
|
Opened: 2006-01-28 13:06
|
|
Description:
|
Last confirmed: 2006-03-09 17:22
|
Opened: 2006-01-28 13:06
|
Gcc complains about legal code. Test case stripped down from a gdb
testsuite test case.
Environment:
System: Linux fishfood.ninemoons.com 2.6.14-1.1656_FC4 #1 Thu Jan 5 22:13:22
EST 2006 i686 i686 i386 GNU/Linux
Architecture: i686
host: i686-pc-linux-gnu
build: i686-pc-linux-gnu
target: i686-pc-linux-gnu
configured with: /src/latest/trunk/src/gcc/configure -v
--prefix=/opt/local/latest/trunk --enable-languages=c,c++
--cache-file=.././config.cache --srcdir=/src/latest/trunk/src/gcc
How-To-Repeat:
$ cat /tmp/bug.c
struct s_1 { short s[1]; } z_1;
struct s_1
add_struct_1 (struct s_1 s)
{
return s;
}
struct s_1
wack_struct_1 (void)
{
register struct s_1 u = z_1;
u = add_struct_1 (u);
return u;
}
$ gcc -c /tmp/bug.c
/tmp/bug.c: In function ‘wack_struct_1’:
/tmp/bug.c:13: error: address of register variable ‘u’ requested
Confirmed, a regression from at least 4.0.2.
Strange - both on mainline and the 4.1 branch, I can reproduce this (albeit
with
a more sensible variable name of "u") for i686-pc-linux-gnu native, but not for
a cross to sh-elf. Yet the failing mark_addressable call is done for i686 from
the gimplifier.
This is caused by the following code in gimplifier:
3297 if (use_target)
3298 {
3299 CALL_EXPR_RETURN_SLOT_OPT (*from_p) = 1;
3300 lang_hooks.mark_addressable (*to_p);
3301 }
Should we really be calling lang_hooks.mark_addressable, here?
(In reply to comment #3)
> This is caused by the following code in gimplifier:
> 3297 if (use_target)
> 3298 {
> 3299 CALL_EXPR_RETURN_SLOT_OPT (*from_p) = 1;
> 3300 lang_hooks.mark_addressable (*to_p);
> 3301 }
>
>
> Should we really be calling lang_hooks.mark_addressable, here?
>
I think the problem is actually not testing properly if the value is a
register variable first.
A few lines before in gimplify.c:gimplify_modify_expr_rhs, we have:
else if (is_gimple_reg_type (TREE_TYPE (*to_p)))
/* Also don't force regs into memory. */
use_target = false;
However, is_gimple_reg_type merely looks if the type is an aggregate.
The test that c-typeck.c:c_mark_addressable uses tests C_DECL_REGISTER .
It appears we actually don't have any way to query from the
frontend-indenpendent code if we can mark something as
safely.
> It appears we actually don't have any way to query from the
> frontend-indenpendent code if we can mark something as
> safely.
^ addressable
(In reply to comment #2)
> Strange - both on mainline and the 4.1 branch, I can reproduce this (albeit
> with
> a more sensible variable name of "u") for i686-pc-linux-gnu native, but not for
> a cross to sh-elf. Yet the failing mark_addressable call is done for i686 from
> the gimplifier.
FWIW, the optimization that malfunctions applies to calls of functions that
return their value in memory. Because the s_1 type is not returned in memory
on sh-elf, the testcase doesn't trigger there. For the test to trigger, s_1
has to be an aggregate (to satisfy the gimple test that this is not a register)
which is returned in memory (so that the optimization is relevant), and the
return value must be assigne to a variable that is declared as register.
This is a design bug. The middle-end should not be calling mark_addressable
hooks in the front ends. The fix may be to factor the front end
mark_addressable functions into a part that just does the appropriate
marking/recursion for language-dependent tree codes, and a separate part that
enforces semantic constraints. The former function would be the one available
to the middle end; the latter (which would presumably call the former) would be
used by the front end itself.
This should be a relatively straightforward fix.
Created an attachment (id=10857) [edit]
infrastructure patch - defective
(In reply to comment #7)
> This should be a relatively straightforward fix.
I had a go at this, but ran into a problem. When an optimizer requests
a variable to be made addressable, I think we need to keep track of this, so
that we don't complain henceforth when the address is taken again, where
addressing the variable once causes further instances of addressing it.
I thought I could reset DECL_REGISTER for this purpose, and accept addressing
if DECL_REGISTER is cleared on entry in c_mark_addressable.
(The C front end uses C_DECL_REGISTER to keep track of a variable being
declared
register, and DECL_REGISTER more or less means that we intent do use this as a
register.)
However, this causes a regression on gcc.dg/reg-vol-struct-1.c line 16 - the
DECL_REGISTER flag has already been reset for 'a' on account of it being
volatile.
So it appears that we need another flag to keep track of when a register has
been made addressable on behalf of the optimizers.
That patch looks wrong. There has to be a better way, maybe just rejecting
return slot optimization instead.
I will look into fixing this bug later today when I get home.
This issue will not be resolved in GCC 4.1.0; retargeted at GCC 4.1.1.
*** Bug 26591 has been marked as a duplicate of this bug. ***
This is fixed on the mainline how I don't know.
Janis could you figure out which patch fixed this on the mainline if it was
ever broken on the mainline.
The reduced testcase in this bug is fixed but the full testcase in the gdb
testsuite.
New reduced testcase:
struct s_3 { short s[3]; } z_3, s_3;
struct s_3 add_struct_3 (struct s_3 s){}
wack_struct_3 (void)
{
int i; register struct s_3 u = z_3;
u = add_struct_3 (u);
}
Subject: Bug 26004
Author: jason
Date: Fri Mar 10 23:42:48 2006
New Revision: 111952
URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=111952
Log:
PR c/26004
* gimplify.c (gimplify_modify_expr_rhs): Don't do return slot opt if
the target was declared 'register'.
Added:
branches/gcc-4_1-branch/gcc/testsuite/gcc.dg/pr26004.c
- copied unchanged from r111947, trunk/gcc/testsuite/gcc.dg/pr26004.c
Modified:
branches/gcc-4_1-branch/gcc/ChangeLog
branches/gcc-4_1-branch/gcc/gimplify.c
*** Bug 27932 has been marked as a duplicate of this bug. ***