This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][PR65818][bootstrap,hppa] Return side-effect free result in gimplify_va_arg_internal
- From: Tom de Vries <Tom_deVries at mentor dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, John David Anglin <dave dot anglin at bell dot net>
- Date: Tue, 28 Apr 2015 22:29:42 +0200
- Subject: Re: [PATCH][PR65818][bootstrap,hppa] Return side-effect free result in gimplify_va_arg_internal
- Authentication-results: sourceware.org; auth=none
- References: <553B89FB dot 7000009 at mentor dot com> <CAFiYyc1AhuTe-pfBhoKUFLBF7KBGmR_CgQphcN=gT6aDwDzBsQ at mail dot gmail dot com> <553E3451 dot 5010300 at mentor dot com> <CAFiYyc0tOQ34-NeOASFAd1aXWj7xM0znwL+=BVLts-8t4NVtdw at mail dot gmail dot com> <553E500B dot 2060505 at mentor dot com> <CAFiYyc0AxbU3RTM5m0jAdWYFyVFvNajeNmO25XwKx4qtxDZX0w at mail dot gmail dot com>
On 28-04-15 12:34, Richard Biener wrote:
On Mon, Apr 27, 2015 at 5:04 PM, Tom de Vries <Tom_deVries@mentor.com> wrote:
On 27-04-15 15:40, Richard Biener wrote:
On Mon, Apr 27, 2015 at 3:06 PM, Tom de Vries <Tom_deVries@mentor.com>
wrote:
On 27-04-15 10:17, Richard Biener wrote:
This patch fixes that by gimplifying the address expression of the
mem-ref
returned by the target hook (borrowing code from gimplify_expr, case
MEM_REF).
Bootstrapped and reg-tested on x86_64.
Bootstrapped and reg-tested on hppa2.0w-hp-hpux11.11.
OK for trunk?
Hmm, that assert looks suspicious...
Can't you simply always do
gimplify_expr (expr, pre_p, post_p, is_gimple_lvalue, fb_lvalue);
It's a bit counter-intuitive for me, using is_gimple_lvalue for something
(the result of va_arg) we use as rvalue.
Yeah, choosing that was done because you could assert it's a MEM_REF
and tree-stdarg eventually builds a WITH_SIZE_EXPR around it.
It would possibly be easier to simply "inline" gimplify_va_arg_internal
at its use and only gimplify the assignment. Though in that case the
original code probably worked - just in the lhs == NULL case it didn't,
which hints at a better place for the fix - in expand_ifn_va_arg_1 do
if (lhs != NULL_TREE)
{
...
}
else
gimplify_expr (&expr, &pre, &post, is_gimple_val, fb_either);
So ... if you can re-try that one it's pre-approved.
Using that proposed patch, we run into the following problem.
Consider this testcase with ignored-result-va_arg:
...
#include <stdarg.h>
void
f2 (int i, ...)
{
va_list ap;
va_start (ap, i);
va_arg (ap, long);
va_end (ap);
}
...
after gimplify_va_arg_internal we have:
...
(gdb) call debug_generic_expr (expr)
*(long int * {ref-all}) addr.2
...
In a bit more detail:
...
(gdb) call debug_tree (expr)
<mem_ref 0x7ffff65ea1b8
type <integer_type 0x7ffff64a77e0 long int DI
size <integer_cst 0x7ffff64a3ca8 constant 64>
unit size <integer_cst 0x7ffff64a3cc0 constant 8>
align 64 symtab 0 alias set -1 canonical type 0x7ffff64a77e0
precision 64 min <integer_cst 0x7ffff64a3f30 -9223372036854775808> max
<integer_cst 0x7ffff64a3f48 9223372036854775807>
pointer_to_this <pointer_type 0x7ffff65e0498>>
arg 0 <nop_expr 0x7ffff65cbb60
type <pointer_type 0x7ffff65e0498 type <integer_type 0x7ffff64a77e0
long int>
public static unsigned DI size <integer_cst 0x7ffff64a3ca8 64>
unit size <integer_cst 0x7ffff64a3cc0 8>
align 64 symtab 0 alias set -1 canonical type 0x7ffff65e0498>
arg 0 <var_decl 0x7ffff65e32d0 addr.2 type <pointer_type
0x7ffff64c2150>
used unsigned ignored DI file stdarg-1.c line 4 col 1 size
<integer_cst 0x7ffff64a3ca8 64> unit size <integer_cst 0x7ffff64a3cc0 8>
align 64 context <function_decl 0x7ffff65c21b0 f2>>>
arg 1 <integer_cst 0x7ffff65e64e0 type <pointer_type 0x7ffff65e0498>
constant 0>>
...
During 'gimplify_expr (&expr, &pre, &post, is_gimple_val, fb_either)' we ICE
here:
...
8886 gcc_assert ((*gimple_test_f) (*expr_p));
...
At this point expr is:
...
(gdb) call debug_generic_expr (*expr_p)
*addr.2
...
In more detail:
...
(gdb) call debug_tree (*expr_p)
<mem_ref 0x7ffff65ea1e0
type <void_type 0x7ffff64c2000 void VOID
align 8 symtab 0 alias set -1 canonical type 0x7ffff64c2000
pointer_to_this <pointer_type 0x7ffff64c2150>>
arg 0 <var_decl 0x7ffff65e32d0 addr.2
type <pointer_type 0x7ffff64c2150 type <void_type 0x7ffff64c2000
void>
sizes-gimplified public unsigned DI
size <integer_cst 0x7ffff64a3ca8 constant 64>
unit size <integer_cst 0x7ffff64a3cc0 constant 8>
align 64 symtab 0 alias set -1 canonical type 0x7ffff64c2150
pointer_to_this <pointer_type 0x7ffff64c9bd0>>
used unsigned ignored DI file stdarg-1.c line 4 col 1 size
<integer_cst 0x7ffff64a3ca8 64> unit size <integer_cst 0x7ffff64a3cc0 8>
align 64 context <function_decl 0x7ffff65c21b0 f2>>
arg 1 <integer_cst 0x7ffff64c10c0 type <pointer_type 0x7ffff64c2150>
constant 0>>
...
The memref is now VOID type. And that's not a gimple_val:
...
(gdb) p is_gimple_val (*expr_p)
$1 = false
...
On which target? I can't seem to reproduce on x86_64 or i?86. I also
can't see how this
could happen.
Reproduced using attached patch on top of r222537, for target x86_64, with and
without -m32.
Thanks,
- Tom
Richard.
Attached patch uses is_gimple_lvalue, but in expand_ifn_va_arg_1.
I'll bootstrap and reg-test on x86_64 and commit, unless further comments of
course.
Thanks,
- Tom
Reproduce ICE in 'gimplify_expr:
gcc_assert ((*gimple_test_f) (*expr_p))'
diff --git a/gcc/testsuite/gcc.dg/reproduce.c b/gcc/testsuite/gcc.dg/reproduce.c
new file mode 100644
index 0000000..c0c8e0b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/reproduce.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+#include <stdarg.h>
+
+void
+f2 (int i, ...)
+{
+ va_list ap;
+ va_start (ap, i);
+ va_arg (ap, long);
+ va_end (ap);
+}
diff --git a/gcc/tree-stdarg.c b/gcc/tree-stdarg.c
index 1356374..829cbc1 100644
--- a/gcc/tree-stdarg.c
+++ b/gcc/tree-stdarg.c
@@ -1079,7 +1079,7 @@ expand_ifn_va_arg_1 (function *fun)
gimplify_assign (lhs, expr, &pre);
}
else
- gimplify_expr (&expr, &pre, &post, is_gimple_lvalue, fb_lvalue);
+ gimplify_expr (&expr, &pre, &post, is_gimple_val, fb_either);
pop_gimplify_context (NULL);
--
1.9.1