* [Bug middle-end/8081] ICE with variably sized types and nested functions
[not found] <bug-8081-4@http.gcc.gnu.org/bugzilla/>
@ 2012-01-11 15:56 ` rguenth at gcc dot gnu.org
2012-01-11 15:58 ` [Bug c/8081] ICE with variably sized types returned from " rguenth at gcc dot gnu.org
` (8 subsequent siblings)
9 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2012-01-11 15:56 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=8081
--- Comment #22 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-01-11 15:53:51 UTC ---
*** Bug 21374 has been marked as a duplicate of this bug. ***
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug c/8081] ICE with variably sized types returned from nested functions
[not found] <bug-8081-4@http.gcc.gnu.org/bugzilla/>
2012-01-11 15:56 ` [Bug middle-end/8081] ICE with variably sized types and nested functions rguenth at gcc dot gnu.org
@ 2012-01-11 15:58 ` rguenth at gcc dot gnu.org
2012-01-12 10:23 ` rguenth at gcc dot gnu.org
` (7 subsequent siblings)
9 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2012-01-11 15:58 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=8081
Richard Guenther <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords|ice-on-valid-code |accepts-invalid,
| |ice-on-invalid-code
Component|middle-end |c
Summary|ICE with variably sized |ICE with variably sized
|types and nested functions |types returned from nested
| |functions
Known to fail| |4.7.0
--- Comment #23 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-01-11 15:57:03 UTC ---
In PR21374 it was decided to reject the testcase which makes this a C frontend
issue. Testcase that still ICEs on the trunk even when optimizing:
int
main (int argc, char **argv)
{
int size = 10;
typedef struct
{
char val[size];
}
block;
block b;
block __attribute__((noinline))
retframe_block ()
{
return *(block *) &b;
}
b=retframe_block ();
return 0;
}
as alternative to rejecting this case we can lower returning variable-size
types during un-nesting to explicit passing of a return slot and using
memcpy, making the nested function return nothing.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug c/8081] ICE with variably sized types returned from nested functions
[not found] <bug-8081-4@http.gcc.gnu.org/bugzilla/>
2012-01-11 15:56 ` [Bug middle-end/8081] ICE with variably sized types and nested functions rguenth at gcc dot gnu.org
2012-01-11 15:58 ` [Bug c/8081] ICE with variably sized types returned from " rguenth at gcc dot gnu.org
@ 2012-01-12 10:23 ` rguenth at gcc dot gnu.org
2012-01-13 8:55 ` ebotcazou at gcc dot gnu.org
` (6 subsequent siblings)
9 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2012-01-12 10:23 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=8081
--- Comment #24 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-01-12 10:23:05 UTC ---
Created attachment 26306
--> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26306
A patch for using by-reference passing
(In reply to comment #23)
> as alternative to rejecting this case we can lower returning variable-size
> types during un-nesting to explicit passing of a return slot and using
> memcpy, making the nested function return nothing.
It's of course not that easy as we gimplify before un-nesting. The frontend
would be responsible to arrange things that way, similar to how we pass
a return slot in the C++ frontend (DECL_BY_REFERENCE on the DECL_RESULT
variable). Like for the attached patch. Passes
extern void abort (void);
int
main (int argc, char **argv)
{
int size = 10;
typedef struct
{
char val[size];
}
block;
block a, b;
block __attribute__((noinline))
retframe_block ()
{
return *(block *) &b;
}
b.val[0] = -1;
b.val[1] = -2;
a=retframe_block ();
if (a.val[0] != -1
|| a.val[1] != -2)
abort ();
return 0;
}
I'm not sure if one can construct a testcase where using return-slot
optimization causes wrong-code generation. Alternatively checking
DECL_BY_REFERENCE on the callees DECL_RESULT instead of applying it to
all VLA types could work (though not for indirect calls).
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug c/8081] ICE with variably sized types returned from nested functions
[not found] <bug-8081-4@http.gcc.gnu.org/bugzilla/>
` (2 preceding siblings ...)
2012-01-12 10:23 ` rguenth at gcc dot gnu.org
@ 2012-01-13 8:55 ` ebotcazou at gcc dot gnu.org
2012-01-13 9:28 ` rguenther at suse dot de
` (5 subsequent siblings)
9 siblings, 0 replies; 10+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2012-01-13 8:55 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=8081
Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |ebotcazou at gcc dot
| |gnu.org
--- Comment #25 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-01-13 08:36:56 UTC ---
> It's of course not that easy as we gimplify before un-nesting. The frontend
> would be responsible to arrange things that way, similar to how we pass
> a return slot in the C++ frontend (DECL_BY_REFERENCE on the DECL_RESULT
> variable). Like for the attached patch. Passes
>
> extern void abort (void);
> int
> main (int argc, char **argv)
> {
> int size = 10;
> typedef struct
> {
> char val[size];
> }
> block;
> block a, b;
> block __attribute__((noinline))
> retframe_block ()
> {
> return *(block *) &b;
> }
> b.val[0] = -1;
> b.val[1] = -2;
> a=retframe_block ();
> if (a.val[0] != -1
> || a.val[1] != -2)
> abort ();
> return 0;
> }
>
> I'm not sure if one can construct a testcase where using return-slot
> optimization causes wrong-code generation. Alternatively checking
> DECL_BY_REFERENCE on the callees DECL_RESULT instead of applying it to
> all VLA types could work (though not for indirect calls).
You should ask specialists. :-) In Ada, we do this routinely and the strategy
used is that of the "forced RSO": we generate INIT_EXPR instead of MODIFY_EXPR
and we create an explicit temporary if we detect potential overlap.
Btw, I don't understand why you're mixing DECL_BY_REFERENCE and RSO here, just
Index: gimplify.c
===================================================================
--- gimplify.c (revision 183104)
+++ gimplify.c (working copy)
@@ -4417,6 +4417,9 @@ gimplify_modify_expr_rhs (tree *expr_p,
/* It's OK to use the target directly if it's being
initialized. */
use_target = true;
+ else if (variably_modified_type_p (TREE_TYPE (*to_p), NULL_TREE))
+ /* Always use the target for variable-sized types. */
+ use_target = true;
else if (TREE_CODE (*to_p) != SSA_NAME
&& (!is_gimple_variable (*to_p)
|| needs_to_live_in_memory (*to_p)))
works for me on the testcase.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug c/8081] ICE with variably sized types returned from nested functions
[not found] <bug-8081-4@http.gcc.gnu.org/bugzilla/>
` (3 preceding siblings ...)
2012-01-13 8:55 ` ebotcazou at gcc dot gnu.org
@ 2012-01-13 9:28 ` rguenther at suse dot de
2012-01-13 10:19 ` rguenth at gcc dot gnu.org
` (4 subsequent siblings)
9 siblings, 0 replies; 10+ messages in thread
From: rguenther at suse dot de @ 2012-01-13 9:28 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=8081
--- Comment #26 from rguenther at suse dot de <rguenther at suse dot de> 2012-01-13 09:08:30 UTC ---
On Fri, 13 Jan 2012, ebotcazou at gcc dot gnu.org wrote:
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=8081
>
> Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:
>
> What |Removed |Added
> ----------------------------------------------------------------------------
> CC| |ebotcazou at gcc dot
> | |gnu.org
>
> --- Comment #25 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-01-13 08:36:56 UTC ---
> > It's of course not that easy as we gimplify before un-nesting. The frontend
> > would be responsible to arrange things that way, similar to how we pass
> > a return slot in the C++ frontend (DECL_BY_REFERENCE on the DECL_RESULT
> > variable). Like for the attached patch. Passes
> >
> > extern void abort (void);
> > int
> > main (int argc, char **argv)
> > {
> > int size = 10;
> > typedef struct
> > {
> > char val[size];
> > }
> > block;
> > block a, b;
> > block __attribute__((noinline))
> > retframe_block ()
> > {
> > return *(block *) &b;
> > }
> > b.val[0] = -1;
> > b.val[1] = -2;
> > a=retframe_block ();
> > if (a.val[0] != -1
> > || a.val[1] != -2)
> > abort ();
> > return 0;
> > }
> >
> > I'm not sure if one can construct a testcase where using return-slot
> > optimization causes wrong-code generation. Alternatively checking
> > DECL_BY_REFERENCE on the callees DECL_RESULT instead of applying it to
> > all VLA types could work (though not for indirect calls).
>
> You should ask specialists. :-) In Ada, we do this routinely and the strategy
> used is that of the "forced RSO": we generate INIT_EXPR instead of MODIFY_EXPR
> and we create an explicit temporary if we detect potential overlap.
>
> Btw, I don't understand why you're mixing DECL_BY_REFERENCE and RSO here, just
>
> Index: gimplify.c
> ===================================================================
> --- gimplify.c (revision 183104)
> +++ gimplify.c (working copy)
> @@ -4417,6 +4417,9 @@ gimplify_modify_expr_rhs (tree *expr_p,
> /* It's OK to use the target directly if it's being
> initialized. */
> use_target = true;
> + else if (variably_modified_type_p (TREE_TYPE (*to_p), NULL_TREE))
> + /* Always use the target for variable-sized types. */
> + use_target = true;
> else if (TREE_CODE (*to_p) != SSA_NAME
> && (!is_gimple_variable (*to_p)
> || needs_to_live_in_memory (*to_p)))
>
> works for me on the testcase.
Ah, ok. So I suppose the Frontend could force RSO here as well by
just setting CALL_EXPR_RETURN_SLOT_OPT on the CALL_EXPR. Not sure
which approach is better.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug c/8081] ICE with variably sized types returned from nested functions
[not found] <bug-8081-4@http.gcc.gnu.org/bugzilla/>
` (4 preceding siblings ...)
2012-01-13 9:28 ` rguenther at suse dot de
@ 2012-01-13 10:19 ` rguenth at gcc dot gnu.org
2012-01-13 11:37 ` ebotcazou at gcc dot gnu.org
` (3 subsequent siblings)
9 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2012-01-13 10:19 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=8081
--- Comment #27 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-01-13 10:11:43 UTC ---
(In reply to comment #26)
> On Fri, 13 Jan 2012, ebotcazou at gcc dot gnu.org wrote:
>
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=8081
> >
> > Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:
> >
> > What |Removed |Added
> > ----------------------------------------------------------------------------
> > CC| |ebotcazou at gcc dot
> > | |gnu.org
> >
> > --- Comment #25 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-01-13 08:36:56 UTC ---
> > > It's of course not that easy as we gimplify before un-nesting. The frontend
> > > would be responsible to arrange things that way, similar to how we pass
> > > a return slot in the C++ frontend (DECL_BY_REFERENCE on the DECL_RESULT
> > > variable). Like for the attached patch. Passes
> > >
> > > extern void abort (void);
> > > int
> > > main (int argc, char **argv)
> > > {
> > > int size = 10;
> > > typedef struct
> > > {
> > > char val[size];
> > > }
> > > block;
> > > block a, b;
> > > block __attribute__((noinline))
> > > retframe_block ()
> > > {
> > > return *(block *) &b;
> > > }
> > > b.val[0] = -1;
> > > b.val[1] = -2;
> > > a=retframe_block ();
> > > if (a.val[0] != -1
> > > || a.val[1] != -2)
> > > abort ();
> > > return 0;
> > > }
> > >
> > > I'm not sure if one can construct a testcase where using return-slot
> > > optimization causes wrong-code generation. Alternatively checking
> > > DECL_BY_REFERENCE on the callees DECL_RESULT instead of applying it to
> > > all VLA types could work (though not for indirect calls).
> >
> > You should ask specialists. :-) In Ada, we do this routinely and the strategy
> > used is that of the "forced RSO": we generate INIT_EXPR instead of MODIFY_EXPR
> > and we create an explicit temporary if we detect potential overlap.
> >
> > Btw, I don't understand why you're mixing DECL_BY_REFERENCE and RSO here, just
> >
> > Index: gimplify.c
> > ===================================================================
> > --- gimplify.c (revision 183104)
> > +++ gimplify.c (working copy)
> > @@ -4417,6 +4417,9 @@ gimplify_modify_expr_rhs (tree *expr_p,
> > /* It's OK to use the target directly if it's being
> > initialized. */
> > use_target = true;
> > + else if (variably_modified_type_p (TREE_TYPE (*to_p), NULL_TREE))
> > + /* Always use the target for variable-sized types. */
> > + use_target = true;
> > else if (TREE_CODE (*to_p) != SSA_NAME
> > && (!is_gimple_variable (*to_p)
> > || needs_to_live_in_memory (*to_p)))
> >
> > works for me on the testcase.
>
> Ah, ok. So I suppose the Frontend could force RSO here as well by
> just setting CALL_EXPR_RETURN_SLOT_OPT on the CALL_EXPR. Not sure
> which approach is better.
OTOH as GIMPLE cannot deal with VLAs on the LHS of a CALL when not applying RSO
the above is correct anyway.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug c/8081] ICE with variably sized types returned from nested functions
[not found] <bug-8081-4@http.gcc.gnu.org/bugzilla/>
` (5 preceding siblings ...)
2012-01-13 10:19 ` rguenth at gcc dot gnu.org
@ 2012-01-13 11:37 ` ebotcazou at gcc dot gnu.org
2012-01-13 11:54 ` rguenth at gcc dot gnu.org
` (2 subsequent siblings)
9 siblings, 0 replies; 10+ messages in thread
From: ebotcazou at gcc dot gnu.org @ 2012-01-13 11:37 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=8081
--- Comment #28 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-01-13 11:21:34 UTC ---
> OTOH as GIMPLE cannot deal with VLAs on the LHS of a CALL when not applying RSO
> the above is correct anyway.
Right, gimplify_return_expr already has a similar provision:
else if (aggregate_value_p (result_decl, TREE_TYPE (current_function_decl)))
{
if (TREE_CODE (DECL_SIZE (result_decl)) != INTEGER_CST)
{
if (!TYPE_SIZES_GIMPLIFIED (TREE_TYPE (result_decl)))
gimplify_type_sizes (TREE_TYPE (result_decl), pre_p);
/* Note that we don't use gimplify_vla_decl because the RESULT_DECL
should be effectively allocated by the caller, i.e. all calls to
this function must be subject to the Return Slot Optimization. */
gimplify_one_sizepos (&DECL_SIZE (result_decl), pre_p);
gimplify_one_sizepos (&DECL_SIZE_UNIT (result_decl), pre_p);
}
result = result_decl;
}
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug c/8081] ICE with variably sized types returned from nested functions
[not found] <bug-8081-4@http.gcc.gnu.org/bugzilla/>
` (6 preceding siblings ...)
2012-01-13 11:37 ` ebotcazou at gcc dot gnu.org
@ 2012-01-13 11:54 ` rguenth at gcc dot gnu.org
2012-01-13 12:08 ` rguenth at gcc dot gnu.org
2012-01-13 12:10 ` rguenth at gcc dot gnu.org
9 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2012-01-13 11:54 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=8081
Richard Guenther <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
AssignedTo|unassigned at gcc dot |rguenth at gcc dot gnu.org
|gnu.org |
--- Comment #29 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-01-13 11:50:10 UTC ---
Mine.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug c/8081] ICE with variably sized types returned from nested functions
[not found] <bug-8081-4@http.gcc.gnu.org/bugzilla/>
` (7 preceding siblings ...)
2012-01-13 11:54 ` rguenth at gcc dot gnu.org
@ 2012-01-13 12:08 ` rguenth at gcc dot gnu.org
2012-01-13 12:10 ` rguenth at gcc dot gnu.org
9 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2012-01-13 12:08 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=8081
--- Comment #30 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-01-13 12:05:32 UTC ---
Author: rguenth
Date: Fri Jan 13 12:05:27 2012
New Revision: 183153
URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=183153
Log:
2012-01-13 Richard Guenther <rguenther@suse.de>
PR middle-end/8081
* gimplify.c (gimplify_modify_expr_rhs): For calls with a
variable-sized result always use RSO.
* gcc.dg/torture/pr8081.c: New testcase.
Added:
trunk/gcc/testsuite/gcc.dg/torture/pr8081.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/gimplify.c
trunk/gcc/testsuite/ChangeLog
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug c/8081] ICE with variably sized types returned from nested functions
[not found] <bug-8081-4@http.gcc.gnu.org/bugzilla/>
` (8 preceding siblings ...)
2012-01-13 12:08 ` rguenth at gcc dot gnu.org
@ 2012-01-13 12:10 ` rguenth at gcc dot gnu.org
9 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2012-01-13 12:10 UTC (permalink / raw)
To: gcc-bugs
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=8081
Richard Guenther <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords|accepts-invalid, |
|ice-on-invalid-code |
Status|ASSIGNED |RESOLVED
Known to work| |4.7.0
Resolution| |FIXED
Known to fail|4.7.0 |4.6.0
--- Comment #31 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-01-13 12:06:36 UTC ---
Fixed for GCC 4.7.
^ permalink raw reply [flat|nested] 10+ messages in thread