* [PATCH] Fix a bug that propagation in recursive function uses wrong aggregate lattice (PR ipa/93084)
@ 2020-01-03 15:50 Feng Xue OS
2020-01-07 13:18 ` Martin Jambor
0 siblings, 1 reply; 5+ messages in thread
From: Feng Xue OS @ 2020-01-03 15:50 UTC (permalink / raw)
To: gcc-patches, Jan Hubicka, mjambor
[-- Attachment #1: Type: text/plain, Size: 529 bytes --]
When checking a self-recursively generated value for aggregate jump
function, wrong aggregate lattice was used, which will cause infinite
constant propagation. This patch is composed to fix this issue.
2020-01-03 Feng Xue <fxue@os.amperecomputing.com>
PR ipa/93084
* ipa-cp.c (self_recursively_generated_p): Find matched aggregate
lattice for a value to check.
(propagate_vals_across_arith_jfunc): Add an assertion to ensure
finite propagation in self-recursive scc.
Feng
[-- Attachment #2: 0001-Fix-pr93084.patch --]
[-- Type: application/octet-stream, Size: 2545 bytes --]
From e04cd6f51e2745337c57b6a2fabe0c56d8be4aa1 Mon Sep 17 00:00:00 2001
From: Feng Xue <fxue@os.amperecomputing.com>
Date: Fri, 3 Jan 2020 22:00:55 +0800
Subject: [PATCH] Fix pr93084
---
gcc/ipa-cp.c | 21 +++++++++++--
gcc/testsuite/gcc.dg/ipa/ipa-clone-3.c | 42 ++++++++++++++++++++++++++
2 files changed, 61 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-clone-3.c
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 4381b35a809..11631986293 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -1917,10 +1917,25 @@ self_recursively_generated_p (ipcp_value<tree> *val)
class ipcp_param_lattices *plats = ipa_get_parm_lattices (info,
src->index);
- ipcp_lattice<tree> *src_lat = src->offset == -1 ? &plats->itself
- : plats->aggs;
+ ipcp_lattice<tree> *src_lat;
ipcp_value<tree> *src_val;
+ if (src->offset == -1)
+ src_lat = &plats->itself;
+ else
+ {
+ struct ipcp_agg_lattice *src_aglat;
+
+ for (src_aglat = plats->aggs; src_aglat; src_aglat = src_aglat->next)
+ if (src_aglat->offset == src->offset)
+ break;
+
+ if (!src_aglat)
+ return false;
+
+ src_lat = src_aglat;
+ }
+
for (src_val = src_lat->values; src_val; src_val = src_val->next)
if (src_val == val)
break;
@@ -2017,6 +2032,8 @@ propagate_vals_across_arith_jfunc (cgraph_edge *cs,
val_seeds.safe_push (src_val);
}
+ gcc_assert ((int) val_seeds.length () <= param_ipa_cp_value_list_size);
+
/* Recursively generate lattice values with a limited count. */
FOR_EACH_VEC_ELT (val_seeds, i, src_val)
{
diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-clone-3.c b/gcc/testsuite/gcc.dg/ipa/ipa-clone-3.c
new file mode 100644
index 00000000000..18d29bdd0b3
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/ipa-clone-3.c
@@ -0,0 +1,42 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -fdump-ipa-cp-details -fno-early-inlining --param ipa-cp-max-recursive-depth=8 --param ipa-cp-eval-threshold=1" } */
+
+struct V {
+ int f0;
+ int f1;
+};
+
+int data[100];
+
+int fn ();
+
+int recur_fn (struct V * __restrict v)
+{
+ int i = v->f0;
+ int j = v->f1;
+ struct V t;
+
+ if (j > 100)
+ {
+ fn ();
+ return 1;
+ }
+
+ data[i] = i;
+
+ t.f0 = i - 2;
+ t.f1 = j + 1;
+
+ recur_fn (&t);
+
+ return i * j;
+}
+
+int main ()
+{
+ struct V v = {1, 3};
+
+ return recur_fn (&v);
+}
+
+/* { dg-final { scan-ipa-dump-times "Creating a specialized node of recur_fn/\[0-9\]*\\." 8 "cp" } } */
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix a bug that propagation in recursive function uses wrong aggregate lattice (PR ipa/93084)
2020-01-03 15:50 [PATCH] Fix a bug that propagation in recursive function uses wrong aggregate lattice (PR ipa/93084) Feng Xue OS
@ 2020-01-07 13:18 ` Martin Jambor
2020-01-09 8:18 ` Christophe Lyon
2020-01-14 20:50 ` Jeff Law
0 siblings, 2 replies; 5+ messages in thread
From: Martin Jambor @ 2020-01-07 13:18 UTC (permalink / raw)
To: Feng Xue OS, gcc-patches, Jan Hubicka
Hi,
On Fri, Jan 03 2020, Feng Xue OS wrote:
> When checking a self-recursively generated value for aggregate jump
> function, wrong aggregate lattice was used, which will cause infinite
> constant propagation. This patch is composed to fix this issue.
>
> 2020-01-03 Feng Xue <fxue@os.amperecomputing.com>
>
> PR ipa/93084
> * ipa-cp.c (self_recursively_generated_p): Find matched aggregate
> lattice for a value to check.
> (propagate_vals_across_arith_jfunc): Add an assertion to ensure
> finite propagation in self-recursive scc.
as far as I am concerned, the patch looks good.
Thanks,
Martin
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix a bug that propagation in recursive function uses wrong aggregate lattice (PR ipa/93084)
2020-01-07 13:18 ` Martin Jambor
@ 2020-01-09 8:18 ` Christophe Lyon
2020-01-10 13:23 ` Martin Jambor
2020-01-14 20:50 ` Jeff Law
1 sibling, 1 reply; 5+ messages in thread
From: Christophe Lyon @ 2020-01-09 8:18 UTC (permalink / raw)
To: Martin Jambor; +Cc: Feng Xue OS, gcc-patches, Jan Hubicka
On Tue, 7 Jan 2020 at 14:18, Martin Jambor <mjambor@suse.cz> wrote:
>
> Hi,
>
> On Fri, Jan 03 2020, Feng Xue OS wrote:
> > When checking a self-recursively generated value for aggregate jump
> > function, wrong aggregate lattice was used, which will cause infinite
> > constant propagation. This patch is composed to fix this issue.
> >
> > 2020-01-03 Feng Xue <fxue@os.amperecomputing.com>
> >
> > PR ipa/93084
> > * ipa-cp.c (self_recursively_generated_p): Find matched aggregate
> > lattice for a value to check.
> > (propagate_vals_across_arith_jfunc): Add an assertion to ensure
> > finite propagation in self-recursive scc.
>
> as far as I am concerned, the patch looks good.
Hi,
The new test introduced by this patch (gcc.dg/ipa/ipa-clone-3.c) fails
on arm, powerpc and mips according to gcc-testresults.
FAIL: gcc.dg/ipa/ipa-clone-3.c scan-ipa-dump-times cp "Creating a
specialized node of recur_fn/[0-9]*\\." 8
Can you have a look?
Thanks,
Christophe
>
> Thanks,
>
> Martin
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix a bug that propagation in recursive function uses wrong aggregate lattice (PR ipa/93084)
2020-01-09 8:18 ` Christophe Lyon
@ 2020-01-10 13:23 ` Martin Jambor
0 siblings, 0 replies; 5+ messages in thread
From: Martin Jambor @ 2020-01-10 13:23 UTC (permalink / raw)
To: Christophe Lyon; +Cc: Feng Xue OS, gcc-patches, Jan Hubicka
Hi,
On Thu, Jan 09 2020, Christophe Lyon wrote:
> On Tue, 7 Jan 2020 at 14:18, Martin Jambor <mjambor@suse.cz> wrote:
>>
>> Hi,
>>
>> On Fri, Jan 03 2020, Feng Xue OS wrote:
>> > When checking a self-recursively generated value for aggregate jump
>> > function, wrong aggregate lattice was used, which will cause infinite
>> > constant propagation. This patch is composed to fix this issue.
>> >
>> > 2020-01-03 Feng Xue <fxue@os.amperecomputing.com>
>> >
>> > PR ipa/93084
>> > * ipa-cp.c (self_recursively_generated_p): Find matched aggregate
>> > lattice for a value to check.
>> > (propagate_vals_across_arith_jfunc): Add an assertion to ensure
>> > finite propagation in self-recursive scc.
>>
>> as far as I am concerned, the patch looks good.
>
> Hi,
> The new test introduced by this patch (gcc.dg/ipa/ipa-clone-3.c) fails
> on arm, powerpc and mips according to gcc-testresults.
> FAIL: gcc.dg/ipa/ipa-clone-3.c scan-ipa-dump-times cp "Creating a
> specialized node of recur_fn/[0-9]*\\." 8
>
> Can you have a look?
>
thank you, next time please also consider filing a bug with the exact
architecture triplets/quadruplets where it fails. I started with a look
at aarch64-suse-linux and then at ppc64le-redhat-linux but apparently
those were not the arms and powerpcs I was looking for.
After building a cross-compiler for armv8l-unknown-linux-gnueabihf I can
see the failure, aggregate jump functions do not get built for:
main ()
{
struct V v;
<bb 2> [local count: 1073741824]:
v = *.LC0;
_4 = recur_fn (&v);
return _4;
}
which is how the release_ssa dump looks like for this target, even
though it might actually be simpler than for the supported and more
common form:
main ()
{
struct V v;
<bb 2> [local count: 1073741824]:
v.f0 = 1;
v.f1 = 3;
_5 = recur_fn (&v);
return _5;
}
The different form stems from the fact that can_move_by_pieces returns
false in gimplify_init_constructor. I'll put this on my TODO list.
Meanwhile, I' about to commit this which should fix the testcase for now
after lightly testing it with make -k check-gcc RUNTESTFLAGS="ipa.exp".
Thanks,
Martin
2020-01-10 Martin Jambor <mjambor@suse.cz>
* gcc.dg/ipa/ipa-clone-3.c: Replace struct initializer with
piecemeal initialization.
---
gcc/testsuite/gcc.dg/ipa/ipa-clone-3.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-clone-3.c b/gcc/testsuite/gcc.dg/ipa/ipa-clone-3.c
index 18d29bdd0b3..a73cb8b63fc 100644
--- a/gcc/testsuite/gcc.dg/ipa/ipa-clone-3.c
+++ b/gcc/testsuite/gcc.dg/ipa/ipa-clone-3.c
@@ -34,8 +34,10 @@ int recur_fn (struct V * __restrict v)
int main ()
{
- struct V v = {1, 3};
+ struct V v;
+ v.f0 = 1;
+ v.f1 = 3;
return recur_fn (&v);
}
--
2.24.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix a bug that propagation in recursive function uses wrong aggregate lattice (PR ipa/93084)
2020-01-07 13:18 ` Martin Jambor
2020-01-09 8:18 ` Christophe Lyon
@ 2020-01-14 20:50 ` Jeff Law
1 sibling, 0 replies; 5+ messages in thread
From: Jeff Law @ 2020-01-14 20:50 UTC (permalink / raw)
To: Martin Jambor, Feng Xue OS, gcc-patches, Jan Hubicka
On Tue, 2020-01-07 at 14:18 +0100, Martin Jambor wrote:
> Hi,
>
> On Fri, Jan 03 2020, Feng Xue OS wrote:
> > When checking a self-recursively generated value for aggregate jump
> > function, wrong aggregate lattice was used, which will cause infinite
> > constant propagation. This patch is composed to fix this issue.
> >
> > 2020-01-03 Feng Xue <fxue@os.amperecomputing.com>
> >
> > PR ipa/93084
> > * ipa-cp.c (self_recursively_generated_p): Find matched aggregate
> > lattice for a value to check.
> > (propagate_vals_across_arith_jfunc): Add an assertion to ensure
> > finite propagation in self-recursive scc.
>
> as far as I am concerned, the patch looks good.
In that case, OK for the trunk :-)
jeff
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-01-14 20:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-03 15:50 [PATCH] Fix a bug that propagation in recursive function uses wrong aggregate lattice (PR ipa/93084) Feng Xue OS
2020-01-07 13:18 ` Martin Jambor
2020-01-09 8:18 ` Christophe Lyon
2020-01-10 13:23 ` Martin Jambor
2020-01-14 20:50 ` Jeff Law
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).