* [PATCH, Fortran, PR 57987] Do not call cgraph_finalize_function multiple times on finalizers
@ 2013-08-02 10:08 Martin Jambor
2013-08-02 11:02 ` Jan Hubicka
0 siblings, 1 reply; 4+ messages in thread
From: Martin Jambor @ 2013-08-02 10:08 UTC (permalink / raw)
To: GCC Patches, fortran; +Cc: Jan Hubicka
Hi,
when looking at another PR, I found out that inliner refused to even
consider __final_test2_T/0 because, according to the dump, "redefined
extern inline functions are not considered for inlining." When I
looked at why, I realized that the function is "finalized" (by
cgraph_finalize_function) twice. Once directly from front-end and
second time from un-nesting nested functions.
This patch makes sure that the Fortran front-end does not do that.
Some details about the patch development over the time is in bugzilla.
Bootstrapped and tested on x86_64-linux without any problems, OK for
trunk?
Thanks,
Martin
2013-08-01 Martin Jambor <mjambor@suse.cz>
* cgraphunit.c (cgraph_finalize_function): Assert that nested function
is not re-finalized. Rename second parameter to no_collect.
fortran/
* trans-decl.c (gfc_generate_function_code): Never call
cgraph_finalize_function on nested functions.
testsuite/
* gfortran.dg/pr57987.f90: New test.
Index: src/gcc/cgraphunit.c
===================================================================
--- src.orig/gcc/cgraphunit.c
+++ src/gcc/cgraphunit.c
@@ -405,17 +405,20 @@ referred_to_p (symtab_node node)
}
/* DECL has been parsed. Take it, queue it, compile it at the whim of the
- logic in effect. If NESTED is true, then our caller cannot stand to have
+ logic in effect. If NO_COLLECT is true, then our caller cannot stand to have
the garbage collector run at the moment. We would need to either create
a new GC context, or just not compile right now. */
void
-cgraph_finalize_function (tree decl, bool nested)
+cgraph_finalize_function (tree decl, bool no_collect)
{
struct cgraph_node *node = cgraph_get_create_node (decl);
if (node->symbol.definition)
{
+ /* Nested functions should only be defined once. */
+ gcc_assert (!DECL_CONTEXT (decl)
+ || TREE_CODE (DECL_CONTEXT (decl)) != FUNCTION_DECL);
cgraph_reset_node (node);
node->local.redefined_extern_inline = true;
}
@@ -454,7 +457,7 @@ cgraph_finalize_function (tree decl, boo
if (warn_unused_parameter)
do_warn_unused_parameter (decl);
- if (!nested)
+ if (!no_collect)
ggc_collect ();
if (cgraph_state == CGRAPH_STATE_CONSTRUCTION
Index: src/gcc/fortran/trans-decl.c
===================================================================
--- src.orig/gcc/fortran/trans-decl.c
+++ src/gcc/fortran/trans-decl.c
@@ -5640,14 +5640,16 @@ gfc_generate_function_code (gfc_namespac
}
current_function_decl = old_context;
- if (decl_function_context (fndecl) && gfc_option.coarray != GFC_FCOARRAY_LIB
- && has_coarray_vars)
- /* Register this function with cgraph just far enough to get it
- added to our parent's nested function list.
- If there are static coarrays in this function, the nested _caf_init
- function has already called cgraph_create_node, which also created
- the cgraph node for this function. */
- (void) cgraph_create_node (fndecl);
+ if (decl_function_context (fndecl))
+ {
+ /* Register this function with cgraph just far enough to get it
+ added to our parent's nested function list.
+ If there are static coarrays in this function, the nested _caf_init
+ function has already called cgraph_create_node, which also created
+ the cgraph node for this function. */
+ if (!has_coarray_vars || gfc_option.coarray != GFC_FCOARRAY_LIB)
+ (void) cgraph_create_node (fndecl);
+ }
else
cgraph_finalize_function (fndecl, true);
Index: src/gcc/testsuite/gfortran.dg/pr57987.f90
===================================================================
--- /dev/null
+++ src/gcc/testsuite/gfortran.dg/pr57987.f90
@@ -0,0 +1,24 @@
+! { dg-do compile }
+! { dg-options "-O3 -fno-ipa-cp -fdump-ipa-inline" }
+
+program test
+ call test2 ()
+contains
+ subroutine test2 ()
+ type t
+ integer, allocatable :: x
+ end type t
+
+ type t2
+ class(t), allocatable :: a
+ end type t2
+
+ type(t2) :: one, two
+
+ allocate (two%a)
+ one = two
+ end subroutine test2
+end program test
+
+! { dg-final { scan-ipa-dump-not "redefined extern inline functions are not considered for inlining" "inline" } }
+! { dg-final { cleanup-ipa-dump "inline" } }
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, Fortran, PR 57987] Do not call cgraph_finalize_function multiple times on finalizers
2013-08-02 10:08 [PATCH, Fortran, PR 57987] Do not call cgraph_finalize_function multiple times on finalizers Martin Jambor
@ 2013-08-02 11:02 ` Jan Hubicka
0 siblings, 0 replies; 4+ messages in thread
From: Jan Hubicka @ 2013-08-02 11:02 UTC (permalink / raw)
To: GCC Patches, fortran, Jan Hubicka
> Hi,
>
> when looking at another PR, I found out that inliner refused to even
> consider __final_test2_T/0 because, according to the dump, "redefined
> extern inline functions are not considered for inlining." When I
> looked at why, I realized that the function is "finalized" (by
> cgraph_finalize_function) twice. Once directly from front-end and
> second time from un-nesting nested functions.
>
> This patch makes sure that the Fortran front-end does not do that.
> Some details about the patch development over the time is in bugzilla.
> Bootstrapped and tested on x86_64-linux without any problems, OK for
> trunk?
OK (I hope that the fortran part is cgraph centric so I can approve it), thanks.
Thanks for noticing it - those missed inlines seems quite unforutnate.
Honza
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, Fortran, PR 57987] Do not call cgraph_finalize_function multiple times on finalizers
2013-08-03 11:12 Dominique Dhumieres
@ 2013-08-05 16:39 ` Martin Jambor
0 siblings, 0 replies; 4+ messages in thread
From: Martin Jambor @ 2013-08-05 16:39 UTC (permalink / raw)
To: Dominique Dhumieres; +Cc: fortran, gcc-patches
Hi,
On Sat, Aug 03, 2013 at 01:12:41PM +0200, Dominique Dhumieres wrote:
> Hi Martin,
>
> I have applied the patch on top of r201441 and I still get the warning for
> gcc/testsuite/gfortran.dg/class_48.f90 with -m32 -O(2|s):
>
> /opt/gcc/work/gcc/testsuite/gfortran.dg/class_48.f90: In function '__final_test2_T.2136.constprop.0':
> /opt/gcc/work/gcc/testsuite/gfortran.dg/class_48.f90:39:0: warning: iteration 2147483648 invokes undefined behavior [-Waggressive-loop-optimizations]
> class(t), allocatable :: a
> ^
> /opt/gcc/work/gcc/testsuite/gfortran.dg/class_48.f90:39:0: note: containing loop
>
> The test gcc/testsuite/gfortran.dg/pr57987.f90 passes.
Well, that is because this patch is a fix for PR 57987 (Fortran
finalizers considered extern-inline by middle-end) and not for PR
57904 (the incorrect warning), even though the testcase is the same.
As I pointed out, the warning happens in dead code (that is dead
thanks to IPA-CP but not removed yet when the warning is emitted). I
have not given much thought to fixing this problem and worked on other
things. IIRC I dug out of history that Jakub added the warning code
recently and so I CCed him. I suppose he will have a look at this
once he re-appears.
Thanks for testing,
Martin
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH, Fortran, PR 57987] Do not call cgraph_finalize_function multiple times on finalizers
@ 2013-08-03 11:12 Dominique Dhumieres
2013-08-05 16:39 ` Martin Jambor
0 siblings, 1 reply; 4+ messages in thread
From: Dominique Dhumieres @ 2013-08-03 11:12 UTC (permalink / raw)
To: fortran; +Cc: gcc-patches, mjambor
Hi Martin,
I have applied the patch on top of r201441 and I still get the warning for
gcc/testsuite/gfortran.dg/class_48.f90 with -m32 -O(2|s):
/opt/gcc/work/gcc/testsuite/gfortran.dg/class_48.f90: In function '__final_test2_T.2136.constprop.0':
/opt/gcc/work/gcc/testsuite/gfortran.dg/class_48.f90:39:0: warning: iteration 2147483648 invokes undefined behavior [-Waggressive-loop-optimizations]
class(t), allocatable :: a
^
/opt/gcc/work/gcc/testsuite/gfortran.dg/class_48.f90:39:0: note: containing loop
The test gcc/testsuite/gfortran.dg/pr57987.f90 passes.
Dominique
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-08-05 16:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-02 10:08 [PATCH, Fortran, PR 57987] Do not call cgraph_finalize_function multiple times on finalizers Martin Jambor
2013-08-02 11:02 ` Jan Hubicka
2013-08-03 11:12 Dominique Dhumieres
2013-08-05 16:39 ` Martin Jambor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).