Re: [PATCH] ipa: Create LOAD references when necessary during inlining (PR 103171)

[Martin Jambor <mjambor@suse.cz>]

Hello,

I would like to ping this patch, please.

Thanks,

Martin

On Fri, Jan 28 2022, Martin Jambor wrote:
> Hi,
>
> in r12-2523-g13586172d0b70c ipa-prop tracking of jump functions during
> inlining got the ability to remove ADDR references when inlining
> discovered that they were not necessary or turn them into LOAD
> references when we know that what was a function call argument passed
> by reference will end up as a load (one or more).
>
> Unfortunately, the code only creates the LOAD references when
> replacing removed ADDR references and PR 103171 showed that with some
> ordering of inlining, we need to add the LOAD reference before we know
> we can remove the ADDR one - or the reference will be lost, leading to
> link errors or even ICEs.
>
> Specifically in testcase gcc.dg/lto/pr103171_1.c added in this patch,
> if foo() is inlined to entry(), we need to create the LOAD reference
> so that when later bar() is inlined into foo() and we discover that
> the paameter is unused, we can remove the ADDR reference and still
> keep the varaible around for the load.
>
> Bootstrapped, LTO bootstrapped and tested on x86_64-linux.  OK for
> trunk?
>
> Thanks,
>
> Martin
>
>
>
> gcc/ChangeLog:
>
> 2022-01-28  Martin Jambor  <mjambor@suse.cz>
>
> 	PR ipa/103171
> 	* ipa-prop.cc (propagate_controlled_uses): Add a LOAD reference
> 	always when an ADDR_EXPR constant is known to reach a load because
> 	of inlining, not just when removing an ADDR reference.
>
> gcc/testsuite/ChangeLog:
>
> 2022-01-28  Martin Jambor  <mjambor@suse.cz>
>
> 	PR ipa/103171
> 	* gcc.dg/ipa/remref-6.c: Adjust dump scan string.
> 	* gcc.dg/ipa/remref-7.c: New test.
> 	* gcc.dg/lto/pr103171_0.c: New test.
> 	* gcc.dg/lto/pr103171_1.c: Likewise.
> ---
>  gcc/ipa-prop.cc                       | 30 ++++++++++++-----------
>  gcc/testsuite/gcc.dg/ipa/remref-6.c   |  2 +-
>  gcc/testsuite/gcc.dg/ipa/remref-7.c   | 33 +++++++++++++++++++++++++
>  gcc/testsuite/gcc.dg/lto/pr103171_0.c | 11 +++++++++
>  gcc/testsuite/gcc.dg/lto/pr103171_1.c | 35 +++++++++++++++++++++++++++
>  5 files changed, 96 insertions(+), 15 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/ipa/remref-7.c
>  create mode 100644 gcc/testsuite/gcc.dg/lto/pr103171_0.c
>  create mode 100644 gcc/testsuite/gcc.dg/lto/pr103171_1.c
>
> diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc
> index e55fe2776f2..72aa3e2f60d 100644
> --- a/gcc/ipa-prop.cc
> +++ b/gcc/ipa-prop.cc
> @@ -4181,6 +4181,20 @@ propagate_controlled_uses (struct cgraph_edge *cs)
>  	  int d = ipa_get_controlled_uses (old_root_info, i);
>  	  int c = rdesc->refcount;
>  	  rdesc->refcount = combine_controlled_uses_counters (c, d);
> +	  if (rdesc->refcount != IPA_UNDESCRIBED_USE
> +	      && ipa_get_param_load_dereferenced (old_root_info, i))
> +	    {
> +	      tree cst = ipa_get_jf_constant (jf);
> +	      gcc_checking_assert (TREE_CODE (cst) == ADDR_EXPR
> +				   && (TREE_CODE (TREE_OPERAND (cst, 0))
> +				       == VAR_DECL));
> +	      symtab_node *n = symtab_node::get (TREE_OPERAND (cst, 0));
> +	      new_root->create_reference (n, IPA_REF_LOAD, NULL);
> +	      if (dump_file)
> +		fprintf (dump_file, "ipa-prop: Address IPA constant will reach "
> +			 "a load so adding LOAD reference from %s to %s.\n",
> +			 new_root->dump_name (), n->dump_name ());
> +	    }
>  	  if (rdesc->refcount == 0)
>  	    {
>  	      tree cst = ipa_get_jf_constant (jf);
> @@ -4193,20 +4207,8 @@ propagate_controlled_uses (struct cgraph_edge *cs)
>  	      symtab_node *n = symtab_node::get (TREE_OPERAND (cst, 0));
>  	      if (n)
>  		{
> -		  struct cgraph_node *clone;
> -		  bool removed = remove_described_reference (n, rdesc);
> -		  /* The reference might have been removed by IPA-CP.  */
> -		  if (removed
> -		      && ipa_get_param_load_dereferenced (old_root_info, i))
> -		    {
> -		      new_root->create_reference (n, IPA_REF_LOAD, NULL);
> -		      if (dump_file)
> -			fprintf (dump_file, "ipa-prop: ...replaced it with "
> -				 "LOAD one from %s to %s.\n",
> -				 new_root->dump_name (), n->dump_name ());
> -		    }
> -
> -		  clone = cs->caller;
> +		  remove_described_reference (n, rdesc);
> +		  cgraph_node *clone = cs->caller;
>  		  while (clone->inlined_to
>  			 && clone->ipcp_clone
>  			 && clone != rdesc->cs->caller)
> diff --git a/gcc/testsuite/gcc.dg/ipa/remref-6.c b/gcc/testsuite/gcc.dg/ipa/remref-6.c
> index 7deae3114a4..f31f4c14319 100644
> --- a/gcc/testsuite/gcc.dg/ipa/remref-6.c
> +++ b/gcc/testsuite/gcc.dg/ipa/remref-6.c
> @@ -20,5 +20,5 @@ void entry()
>  }
>  
>  /* { dg-final { scan-ipa-dump "Removed a reference"  "inline" } }  */
> -/* { dg-final { scan-ipa-dump "replaced it with LOAD"  "inline"  } } */
> +/* { dg-final { scan-ipa-dump "adding LOAD reference"  "inline"  } } */
>  /* { dg-final { scan-tree-dump-not "builtin_exp"  "optimized"  } } */
> diff --git a/gcc/testsuite/gcc.dg/ipa/remref-7.c b/gcc/testsuite/gcc.dg/ipa/remref-7.c
> new file mode 100644
> index 00000000000..b2c26ab7fd5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ipa/remref-7.c
> @@ -0,0 +1,33 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-early-inlining -fno-ipa-sra -fdump-ipa-inline"  }  */
> +
> +int rglobal = 0;
> +int g;
> +
> +int c;
> +double *array;
> +
> +/* unused parameter */
> +static void bar(int *p)
> +{
> +  int i;
> +  for (i = 0; i < c; i++)
> +    {
> +      /* something big so that it is inlined second. */
> +      array[i] = __builtin_exp(array[i]+1)*2;
> +    }
> +}
> +
> +void foo(int *p) {
> +  g = *p;
> +  bar(p);
> +}
> +
> +void entry()
> +{
> +  foo(&rglobal);
> +}
> +
> +/* { dg-final { scan-ipa-dump "Removed a reference"  "inline" } }  */
> +/* { dg-final { scan-ipa-dump "adding LOAD reference"  "inline"  } } */
> +
> diff --git a/gcc/testsuite/gcc.dg/lto/pr103171_0.c b/gcc/testsuite/gcc.dg/lto/pr103171_0.c
> new file mode 100644
> index 00000000000..5dc17d6c6c8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/lto/pr103171_0.c
> @@ -0,0 +1,11 @@
> +/* { dg-lto-do link } */
> +/* { dg-lto-options { { -O2 -flto -flto-partition=1to1 -fno-early-inlining -fno-ipa-sra -w } } } */
> +
> +extern void __attribute__((noinline)) entry(void);
> +
> +int
> +main (int argc, char **argv)
> +{
> +  entry();
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.dg/lto/pr103171_1.c b/gcc/testsuite/gcc.dg/lto/pr103171_1.c
> new file mode 100644
> index 00000000000..39aed25daf7
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/lto/pr103171_1.c
> @@ -0,0 +1,35 @@
> +int rglobal = 0;
> +
> +volatile int g;
> +volatile int c;
> +volatile double *array;
> +
> +/* unused parameter */
> +static void
> +bar(int *p)
> +{
> +  int i;
> +  for (i = 0; i < c; i++)
> +    {
> +      /* something big so that it is inlined second. */
> +      array[i] = (array[i+1]+array[i]+1)*2;
> +    }
> +}
> +
> +void foo(int *p) {
> +  g = *p;
> +  bar(p);
> +}
> +
> +void __attribute__((noinline))
> +entry(void)
> +{
> +  foo(&rglobal);
> +}
> +
> +void __attribute__((used))
> +blah(int *p)
> +{
> +  bar(p);
> +}
> +
> -- 
> 2.34.1