Re: [patch] Fix PR middle-end/66633
After looking into this some more, because the PR got reopened, there were two issues: 1) __builtin_adjust_trampoline call needing a frame or chain (as can be seen on the new testcases) that wasn't added to parallel/task/target clauses 2) for !optimize, there is code to add those, when frame or chain is needed for calls, but the problem was that as !optimize didn't clear DECL_STATIC_CHAIN initially it wasn't discovered. Both issues fixed in the following patch, which made the earlier change unnecessary. Thanks for looking into this! -- Eric Botcazou
Re: [patch] Fix PR middle-end/66633
On Mon, Jun 29, 2015 at 11:23:24AM +0200, Eric Botcazou wrote:
Don't you need to handle convert_nonlocal_omp_clauses similarly (need_chain
in that case)?
At least looking at your r211308 commit, for !optimize you force not just
the frame, but also chain.
You're very likely right, although I didn't manage to write a Fortran
testcase
with my limited knowledge of the language (I get a Bad statement code ICE).
In fact the conditions should also mimic those of the aforementioned commit.
I initially didn't realize it, because I didn't quite grasp how the GOMP
stuff
was interacting with the nesting stuff, but I guess that the code ought not
to
do anything if there aren't pre-existing nested functions in the sources.
After looking into this some more, because the PR got reopened, there were
two issues: 1) __builtin_adjust_trampoline call needing a frame or chain (as
can be seen on the new testcases) that wasn't added to parallel/task/target
clauses 2) for !optimize, there is code to add those, when frame or chain
is needed for calls, but the problem was that as !optimize didn't clear
DECL_STATIC_CHAIN initially it wasn't discovered.
Both issues fixed in the following patch, which made the earlier change
unnecessary.
Bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.
Should go to 5 branch eventually.
2015-07-09 Jakub Jelinek ja...@redhat.com
PR middle-end/66633
* tree-nested.c (get_static_chain): Or in a flag into
info-static_chain_added.
(get_frame_field, get_nonlocal_debug_decl): Likewise.
(convert_nonlocal_omp_clauses, convert_local_omp_clauses): Revert
2015-07-01 changes.
(convert_tramp_reference_stmt): If a frame_decl or chain_decl
is needed newly inside of GIMPLE_OMP_{PARALLEL,TASK,TARGET} body,
add it to clauses.
* gcc.dg/gomp/pr66633-1.c: New test.
* gcc.dg/gomp/pr66633-2.c: New test.
* gcc.dg/gomp/pr66633-3.c: New test.
* gcc.dg/gomp/pr66633-4.c: New test.
--- gcc/tree-nested.c.jj2015-07-08 19:50:09.0 +0200
+++ gcc/tree-nested.c 2015-07-09 19:33:51.116923514 +0200
@@ -767,10 +767,12 @@ get_static_chain (struct nesting_info *i
if (info-context == target_context)
{
x = build_addr (info-frame_decl, target_context);
+ info-static_chain_added |= 1;
}
else
{
x = get_chain_decl (info);
+ info-static_chain_added |= 2;
for (i = info-outer; i-context != target_context; i = i-outer)
{
@@ -802,10 +804,12 @@ get_frame_field (struct nesting_info *in
/* Make sure frame_decl gets created. */
(void) get_frame_type (info);
x = info-frame_decl;
+ info-static_chain_added |= 1;
}
else
{
x = get_chain_decl (info);
+ info-static_chain_added |= 2;
for (i = info-outer; i-context != target_context; i = i-outer)
{
@@ -851,10 +855,12 @@ get_nonlocal_debug_decl (struct nesting_
(void) get_frame_type (info);
x = info-frame_decl;
i = info;
+ info-static_chain_added |= 1;
}
else
{
x = get_chain_decl (info);
+ info-static_chain_added |= 2;
for (i = info-outer; i-context != target_context; i = i-outer)
{
field = get_chain_field (i);
@@ -1061,9 +1067,7 @@ static bool
convert_nonlocal_omp_clauses (tree *pclauses, struct walk_stmt_info *wi)
{
struct nesting_info *const info = (struct nesting_info *) wi-info;
- /* If not optimizing, we will force the creation of the CHAIN object in
- convert_all_function_calls, so we need to take it into account here. */
- bool need_chain = info-outer !optimize, need_stmts = false;
+ bool need_chain = false, need_stmts = false;
tree clause, decl;
int dummy;
bitmap new_suppress;
@@ -1691,9 +1695,7 @@ static bool
convert_local_omp_clauses (tree *pclauses, struct walk_stmt_info *wi)
{
struct nesting_info *const info = (struct nesting_info *) wi-info;
- /* If not optimizing, we will force the creation of the FRAME object in
- convert_all_function_calls, so we need to take it into account here. */
- bool need_frame = info-inner !optimize, need_stmts = false;
+ bool need_frame = false, need_stmts = false;
tree clause, decl;
int dummy;
bitmap new_suppress;
@@ -2292,17 +2294,55 @@ convert_tramp_reference_stmt (gimple_stm
case GIMPLE_OMP_PARALLEL:
case GIMPLE_OMP_TASK:
{
- tree save_local_var_chain;
+ tree save_local_var_chain = info-new_local_var_chain;
walk_gimple_op (stmt, convert_tramp_reference_op, wi);
- save_local_var_chain = info-new_local_var_chain;
info-new_local_var_chain = NULL;
+ char save_static_chain_added = info-static_chain_added;
+ info-static_chain_added = 0;
walk_body (convert_tramp_reference_stmt, convert_tramp_reference_op,
info, gimple_omp_body_ptr (stmt));
if
Re: [patch] Fix PR middle-end/66633
On Wed, Jul 01, 2015 at 09:52:04AM +0200, Eric Botcazou wrote: Jakub, 2015-06-29 Eric Botcazou ebotca...@adacore.com PR middle-end/66633 * tree-nested.c (convert_nonlocal_omp_clauses): Initialize need_chain to true if the function is nested and if not optimizing. (convert_local_omp_clauses): Initialize need_frame to true if the function contains nested functions and if not optimizing. Any opinion about it? Do you want me to backport it onto the 5 branch now? LGTM, and I'd apply it to 5 too. Jakub
Re: [patch] Fix PR middle-end/66633
Jakub, 2015-06-29 Eric Botcazou ebotca...@adacore.com PR middle-end/66633 * tree-nested.c (convert_nonlocal_omp_clauses): Initialize need_chain to true if the function is nested and if not optimizing. (convert_local_omp_clauses): Initialize need_frame to true if the function contains nested functions and if not optimizing. Any opinion about it? Do you want me to backport it onto the 5 branch now? -- Eric Botcazou
Re: [patch] Fix PR middle-end/66633
Don't you need to handle convert_nonlocal_omp_clauses similarly (need_chain
in that case)?
At least looking at your r211308 commit, for !optimize you force not just
the frame, but also chain.
You're very likely right, although I didn't manage to write a Fortran testcase
with my limited knowledge of the language (I get a Bad statement code ICE).
In fact the conditions should also mimic those of the aforementioned commit.
I initially didn't realize it, because I didn't quite grasp how the GOMP stuff
was interacting with the nesting stuff, but I guess that the code ought not to
do anything if there aren't pre-existing nested functions in the sources.
Tested on x86_64-suse-linux, OK for the mainline and 5 branch?
2015-06-29 Eric Botcazou ebotca...@adacore.com
PR middle-end/66633
* tree-nested.c (convert_nonlocal_omp_clauses): Initialize need_chain
to true if the function is nested and if not optimizing.
(convert_local_omp_clauses): Initialize need_frame to true if the
function contains nested functions and if not optimizing.
2015-06-29 Eric Botcazou ebotca...@adacore.com
* gfortran.dg/gomp/pr66633.f90: New test.
--
Eric BotcazouIndex: tree-nested.c
===
--- tree-nested.c (revision 225111)
+++ tree-nested.c (working copy)
@@ -1069,7 +1069,9 @@ static bool
convert_nonlocal_omp_clauses (tree *pclauses, struct walk_stmt_info *wi)
{
struct nesting_info *const info = (struct nesting_info *) wi-info;
- bool need_chain = false, need_stmts = false;
+ /* If not optimizing, we will force the creation of the CHAIN object in
+ convert_all_function_calls, so we need to take it into account here. */
+ bool need_chain = info-outer !optimize, need_stmts = false;
tree clause, decl;
int dummy;
bitmap new_suppress;
@@ -1697,7 +1699,9 @@ static bool
convert_local_omp_clauses (tree *pclauses, struct walk_stmt_info *wi)
{
struct nesting_info *const info = (struct nesting_info *) wi-info;
- bool need_frame = false, need_stmts = false;
+ /* If not optimizing, we will force the creation of the FRAME object in
+ convert_all_function_calls, so we need to take it into account here. */
+ bool need_frame = info-inner !optimize, need_stmts = false;
tree clause, decl;
int dummy;
bitmap new_suppress;! PR middle-end/66633
! Testcase by Andrew Benson abenso...@gmail.com
! { dg-do compile }
! { dg-options -O0 -fopenmp }
module spls
contains
function spl()
!$omp parallel
write (0,*) igrt(fli)
!$omp end parallel
contains
double precision function fli()
end function fli
end function spl
end module spls
Re: [patch] Fix PR middle-end/66633
On Fri, Jun 26, 2015 at 12:38:48PM +0200, Eric Botcazou wrote:
this is a regression present on the mainline and 5 branch. For the attached
Fortran testcase, the GIMPLE verifier stops the compiler on an error mark
inserted by omp-low.c:omp_copy_decl for a FRAME variable created during the
nested function lowering pass because it is not marked as shared since it is
created too late (it is created at the end of the pass when optimization is
disabled after the change for PR debug/53927).
Fixed by teaching convert_local_omp_clauses about this case, tested on x86_64-
suse-linux, OK for the mainline and 5 branch?
Don't you need to handle convert_nonlocal_omp_clauses similarly (need_chain
in that case)?
At least looking at your r211308 commit, for !optimize you force not just
the frame, but also chain.
2015-06-26 Eric Botcazou ebotca...@adacore.com
* gfortran.dg/pr66633.f90: New test.
The test should go into gfortran.dg/gomp/, without the
! { dg-require-effective-target fopenmp }
line.
Thanks for working on this.
Jakub
[patch] Fix PR middle-end/66633
Hi,
this is a regression present on the mainline and 5 branch. For the attached
Fortran testcase, the GIMPLE verifier stops the compiler on an error mark
inserted by omp-low.c:omp_copy_decl for a FRAME variable created during the
nested function lowering pass because it is not marked as shared since it is
created too late (it is created at the end of the pass when optimization is
disabled after the change for PR debug/53927).
Fixed by teaching convert_local_omp_clauses about this case, tested on x86_64-
suse-linux, OK for the mainline and 5 branch?
2015-06-26 Eric Botcazou ebotca...@adacore.com
PR middle-end/66633
* tree-nested.c (convert_local_omp_clauses): Initialize need_frame to
true if not optimizing.
2015-06-26 Eric Botcazou ebotca...@adacore.com
* gfortran.dg/pr66633.f90: New test.
--
Eric BotcazouIndex: tree-nested.c
===
--- tree-nested.c (revision 224993)
+++ tree-nested.c (working copy)
@@ -1697,7 +1697,9 @@ static bool
convert_local_omp_clauses (tree *pclauses, struct walk_stmt_info *wi)
{
struct nesting_info *const info = (struct nesting_info *) wi-info;
- bool need_frame = false, need_stmts = false;
+ /* If not optimizing, we will force the creation of the FRAME object in
+ convert_all_function_calls, so we need to take it into account here. */
+ bool need_frame = !optimize, need_stmts = false;
tree clause, decl;
int dummy;
bitmap new_suppress;
! PR middle-end/66633
! Testcase by Andrew Benson abenso...@gmail.com
! { dg-do compile }
! { dg-options -fopenmp }
! { dg-require-effective-target fopenmp }
module spls
contains
function spl()
!$omp parallel
write (0,*) igrt(fli)
!$omp end parallel
contains
double precision function fli()
end function fli
end function spl
end module spls
