public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jan Hubicka <hubicka@ucw.cz>
To: Richard Biener <rguenther@suse.de>
Cc: gcc-patches@gcc.gnu.org, Jan Hubicka <hubicka@ucw.cz>
Subject: Re: [PATCH] Fix PR60911
Date: Thu, 24 Apr 2014 14:37:00 -0000	[thread overview]
Message-ID: <20140424143533.GA21662@kam.mff.cuni.cz> (raw)
In-Reply-To: <alpine.LSU.2.11.1404241303150.18709@zhemvz.fhfr.qr>

> 
> Simple IPA passes are supposed to see function bodies with IPA transforms 
> applied - this is what the code in execute_one_pass tries to ensure.
> But that doesn't work anymore with on-demand function-body loading.
> The following addresses this in the least intrusive way - inlining
> do_per_function (apply_ipa_transforms) and adjusting it accordingly.
> 
> This IMHO is definitely the solution for the 4.9 branch (and for
> the time being on trunk).
> 
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> 
> Ok for trunk and branch?

I think it is fine for both 4.9 and mainline. I will try to make better version
for mainline as explained in PR hortly.

Can you, please, double check that it won't load all bodies prior late optimization
by default? 
Looking at gate of pass_omp_simd_clone, perhaps it actually kills late loading
of bodies and perhaps we need to mark in cgraph node whether the given node
needs clonning and page the gate return false if partition has no such unit?
bool
pass_omp_simd_clone::gate (function *)
{
  return ((flag_openmp || flag_openmp_simd
           || flag_cilkplus
           || (in_lto_p && !flag_wpa))
          && (targetm.simd_clone.compute_vecsize_and_simdlen != NULL));
}

I did not see there the in_lto_p previously.

Honza
> 
> Thanks,
> Richard.
> 
> 2014-04-24  Richard Biener  <rguenther@suse.de>
> 
> 	PR ipa/60911
> 	* passes.c (apply_ipa_transforms): Inline into only caller ...
> 	(execute_one_pass): ... here.  Properly bring in function
> 	bodies for nodes we want to apply IPA transforms to.
> 
> 	* gcc.dg/lto/pr60911_0.c: New testcase.
> 
> Index: gcc/passes.c
> ===================================================================
> --- gcc/passes.c	(revision 209742)
> +++ gcc/passes.c	(working copy)
> @@ -2109,20 +2109,6 @@ execute_all_ipa_transforms (void)
>      }
>  }
>  
> -/* Callback for do_per_function to apply all IPA transforms.  */
> -
> -static void
> -apply_ipa_transforms (void *data)
> -{
> -  struct cgraph_node *node = cgraph_get_node (current_function_decl);
> -  if (!node->global.inlined_to && node->ipa_transforms_to_apply.exists ())
> -    {
> -      *(bool *)data = true;
> -      execute_all_ipa_transforms ();
> -      rebuild_cgraph_edges ();
> -    }
> -}
> -
>  /* Check if PASS is explicitly disabled or enabled and return
>     the gate status.  FUNC is the function to be processed, and
>     GATE_STATUS is the gate status determined by pass manager by
> @@ -2194,8 +2180,26 @@ execute_one_pass (opt_pass *pass)
>       Apply all trnasforms first.  */
>    if (pass->type == SIMPLE_IPA_PASS)
>      {
> +      struct cgraph_node *node;
>        bool applied = false;
> -      do_per_function (apply_ipa_transforms, (void *)&applied);
> +      FOR_EACH_DEFINED_FUNCTION (node)
> +	if (node->analyzed
> +	    && cgraph_function_with_gimple_body_p (node)
> +	    && (!node->clone_of || node->decl != node->clone_of->decl))
> +	  {
> +	    if (!node->global.inlined_to
> +		&& node->ipa_transforms_to_apply.exists ())
> +	      {
> +		cgraph_get_body (node);
> +		push_cfun (DECL_STRUCT_FUNCTION (node->decl));
> +		execute_all_ipa_transforms ();
> +		rebuild_cgraph_edges ();
> +		free_dominance_info (CDI_DOMINATORS);
> +		free_dominance_info (CDI_POST_DOMINATORS);
> +		pop_cfun ();
> +		applied = true;
> +	      }
> +	  }
>        if (applied)
>          symtab_remove_unreachable_nodes (true, dump_file);
>        /* Restore current_pass.  */
> Index: gcc/testsuite/gcc.dg/lto/pr60911_0.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/lto/pr60911_0.c	(revision 0)
> +++ gcc/testsuite/gcc.dg/lto/pr60911_0.c	(working copy)
> @@ -0,0 +1,21 @@
> +// { dg-lto-do run }
> +// { dg-lto-options { { -O2 -flto -fipa-pta } } }
> +
> +int __attribute__ ((__noinline__)) f (unsigned *p, int *x)
> +{
> +  int y = *p++ & 0xfff;
> +  *x++ = y;
> +  *x = *p;
> +  return y;
> +}
> +
> +int
> +main ()
> +{
> +  unsigned u[2] = { 0x3aad, 0x5ad1 };
> +  int x[2] = { 17689, 23456 };
> +
> +  if (f (u, x) != 0xaad || x[0] != 0xaad || x[1] != 0x5ad1)
> +    __builtin_abort ();
> +  return 0;
> +}

  reply	other threads:[~2014-04-24 14:35 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-24 11:42 Richard Biener
2014-04-24 14:37 ` Jan Hubicka [this message]
2014-04-25  7:43   ` Richard Biener
2014-04-25 17:20     ` Jan Hubicka
2014-04-28  8:20       ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140424143533.GA21662@kam.mff.cuni.cz \
    --to=hubicka@ucw.cz \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=rguenther@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).