* [PATCH] ipa: Create LOAD references when necessary during inlining (PR 103171)
@ 2022-01-28 17:34 Martin Jambor
2022-02-14 18:33 ` Martin Jambor
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Martin Jambor @ 2022-01-28 17:34 UTC (permalink / raw)
To: GCC Patches; +Cc: Jan Hubicka
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ipa: Create LOAD references when necessary during inlining (PR 103171)
2022-01-28 17:34 [PATCH] ipa: Create LOAD references when necessary during inlining (PR 103171) Martin Jambor
@ 2022-02-14 18:33 ` Martin Jambor
2022-03-23 16:33 ` Martin Jambor
2022-03-31 12:38 ` Jan Hubicka
2 siblings, 0 replies; 4+ messages in thread
From: Martin Jambor @ 2022-02-14 18:33 UTC (permalink / raw)
To: GCC Patches; +Cc: Jan Hubicka
Hello and ping, 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ipa: Create LOAD references when necessary during inlining (PR 103171)
2022-01-28 17:34 [PATCH] ipa: Create LOAD references when necessary during inlining (PR 103171) Martin Jambor
2022-02-14 18:33 ` Martin Jambor
@ 2022-03-23 16:33 ` Martin Jambor
2022-03-31 12:38 ` Jan Hubicka
2 siblings, 0 replies; 4+ messages in thread
From: Martin Jambor @ 2022-03-23 16:33 UTC (permalink / raw)
To: GCC Patches; +Cc: Jan Hubicka, Jan Hubicka
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ipa: Create LOAD references when necessary during inlining (PR 103171)
2022-01-28 17:34 [PATCH] ipa: Create LOAD references when necessary during inlining (PR 103171) Martin Jambor
2022-02-14 18:33 ` Martin Jambor
2022-03-23 16:33 ` Martin Jambor
@ 2022-03-31 12:38 ` Jan Hubicka
2 siblings, 0 replies; 4+ messages in thread
From: Jan Hubicka @ 2022-03-31 12:38 UTC (permalink / raw)
To: Martin Jambor; +Cc: GCC Patches
> 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?
OK,
Thanks!
Honza
>
> 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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-03-31 12:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-28 17:34 [PATCH] ipa: Create LOAD references when necessary during inlining (PR 103171) Martin Jambor
2022-02-14 18:33 ` Martin Jambor
2022-03-23 16:33 ` Martin Jambor
2022-03-31 12:38 ` Jan Hubicka
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).