Re: Propagate value ranges of return values
On Tue, Nov 21, 2023 at 6:07 AM Jan Hubicka wrote:
>
> > After this patch in addition to the problem already reported about
> > vlda1.c and return-value-range-1.c, we have noticed these regressions
> > on aarch64:
> > Running gcc:gcc.target/aarch64/aarch64.exp ...
> > FAIL: gcc.target/aarch64/movk.c scan-assembler movk\tx[0-9]+, 0x4667, lsl 16
> > FAIL: gcc.target/aarch64/movk.c scan-assembler movk\tx[0-9]+, 0x7a3d, lsl 32
> >
> > Running gcc:gcc.target/aarch64/simd/simd.exp ...
> > FAIL: gcc.target/aarch64/simd/vmulxd_f64_2.c scan-assembler-times
> > fmul[ \t]+[dD][0-9]+, ?[dD][0-9]+, ?[dD][0-9]+\n 1
> > FAIL: gcc.target/aarch64/simd/vmulxd_f64_2.c scan-assembler-times
> > fmulx[ \t]+[dD][0-9]+, ?[dD][0-9]+, ?[dD][0-9]+\n 4
> > FAIL: gcc.target/aarch64/simd/vmulxs_f32_2.c scan-assembler-times
> > fmul[ \t]+[sS][0-9]+, ?[sS][0-9]+, ?[sS][0-9]+\n 1
> > FAIL: gcc.target/aarch64/simd/vmulxs_f32_2.c scan-assembler-times
> > fmulx[ \t]+[sS][0-9]+, ?[sS][0-9]+, ?[sS][0-9]+\n 4
>
> Sorry for that - I guess we will see some on various targets.
> This is quite common issue - the testcase is having
> dummy_number_generator function returning constant and prevents
> inlining to avoid constant being visible to compiler. This no longer
> works, since we get it from the return value range. This should fix it.
>
> return-value_range-1.c should be fixed now and I do not have vlda1.c in
> my tree. I will check.
This is the other change that needs to happen I think:
diff --git a/gcc/testsuite/gcc.target/aarch64/simd/vmulx.x
b/gcc/testsuite/gcc.target/aarch64/simd/vmulx.x
index 8968a64a95c..869e7485646 100644
--- a/gcc/testsuite/gcc.target/aarch64/simd/vmulx.x
+++ b/gcc/testsuite/gcc.target/aarch64/simd/vmulx.x
@@ -33,13 +33,13 @@
while (0)\
/* Functions used to return values that won't be optimised away. */
-float32_t __attribute__ ((noinline))
+float32_t __attribute__ ((noipa))
foo32 ()
{
return 1.0;
}
-float64_t __attribute__ ((noinline))
+float64_t __attribute__ ((noipa))
foo64 ()
{
return 1.0;
Thanks,
Andrew Pinski
>
> diff --git a/gcc/testsuite/gcc.target/aarch64/movk.c
> b/gcc/testsuite/gcc.target/aarch64/movk.c
> index e6e4e3a8961..6b1f3f8ecf5 100644
> --- a/gcc/testsuite/gcc.target/aarch64/movk.c
> +++ b/gcc/testsuite/gcc.target/aarch64/movk.c
> @@ -1,8 +1,9 @@
> /* { dg-do run } */
> -/* { dg-options "-O2 --save-temps -fno-inline" } */
> +/* { dg-options "-O2 --save-temps" } */
>
> extern void abort (void);
>
> +__attribute__ ((noipa))
> long long int
> dummy_number_generator ()
> {
>
> >
> > We have already sent you a notification for the regression on arm, but
> > it includes on vla-1.c and return-value-range-1.c.
> > The notification email contains a pointer to the page where we record
> > all the configurations that regress because of this patch:
> >
> > https://linaro.atlassian.net/browse/GNU-1025
> >
> > Can you have a look?
> >
> > Thanks,
> >
> > Christophe
> >
> >
> >
> >
> > > diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
> > > index e41e5ad3ae7..71dacf23ce1 100644
> > > --- a/gcc/cgraph.cc
> > > +++ b/gcc/cgraph.cc
> > > @@ -2629,6 +2629,54 @@ cgraph_node::set_malloc_flag (bool malloc_p)
> > >return changed;
> > > }
> > >
> > > +/* Worker to set malloc flag. */
> > > +static void
> > > +add_detected_attribute_1 (cgraph_node *node, const char *attr, bool
> > > *changed)
> > > +{
> > > + if (!lookup_attribute (attr, DECL_ATTRIBUTES (node->decl)))
> > > +{
> > > + DECL_ATTRIBUTES (node->decl) = tree_cons (get_identifier (attr),
> > > +NULL_TREE, DECL_ATTRIBUTES
> > > (node->decl));
> > > + *changed = true;
> > > +}
> > > +
> > > + ipa_ref *ref;
> > > + FOR_EACH_ALIAS (node, ref)
> > > +{
> > > + cgraph_node *alias = dyn_cast (ref->referring);
> > > + if (alias->get_availability () > AVAIL_INTERPOSABLE)
> > > + add_detected_attribute_1 (alias, attr, changed);
> > > +}
> > > +
> > > + for (cgraph_edge *e = node->callers; e; e = e->next_caller)
> > > +if (e->caller->thunk
> > > + && (e->caller->get_availability () > AVAIL_INTERPOSABLE))
> > > + add_detected_attribute_1 (e->caller, attr, changed);
> > > +}
> > > +
> > > +/* Set DECL_IS_MALLOC on NODE's decl and on NODE's aliases if any. */
> > > +
> > > +bool
> > > +cgraph_node::add_detected_attribute (const char *attr)
> > > +{
> > > + bool changed = false;
> > > +
> > > + if (get_availability () > AVAIL_INTERPOSABLE)
> > > +add_detected_attribute_1 (this, attr, );
> > > + else
> > > +{
> > > + ipa_ref *ref;
> > > +
> > > + FOR_EACH_ALIAS (this, ref)
> > > + {
> > > + cgraph_node *alias = dyn_cast (ref->referring);
> > > + if (alias->get_availability () > AVAIL_INTERPOSABLE)
> > > + add_detected_attribute_1 (alias, attr, );
> > > + }
> > > +}
> > > + return changed;
> > > +}
> > > +
> > > /*
Adjust 'libgomp.c/declare-variant-{3,4}-[...]' for inter-procedural value range propagation (was: Propagate value ranges of return values)
Hi!
On 2023-11-19T16:05:42+0100, Jan Hubicka wrote:
> this is updated version which also adds testuiste compensation
> I lost earlier while maintaining the patch in my testing tree.
> There are quite few testcases that use constant return values to hide
> something from optimizer.
One more: commit a53da3a213ee00866d132c228a0e89bd2f61d65c
"Adjust 'libgomp.c/declare-variant-{3,4}-[...]' for inter-procedural value
range propagation"
pushed to master branch, see attached. (Those regressions are only
visible in GCC offloading configurations.) (And actually, all those test
cases have other issues; will install further patches later on.)
Jakub, Tobias, please let me know if it's not expected that *all* the
"variant" functions have to be tagged '__attribute__ ((noipa))' (as I've
done); just tagging the "dispatcher" function 'f' isn't sufficient.
Grüße
Thomas
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
>From a53da3a213ee00866d132c228a0e89bd2f61d65c Mon Sep 17 00:00:00 2001
From: Thomas Schwinge
Date: Tue, 21 Nov 2023 22:42:49 +0100
Subject: [PATCH] Adjust 'libgomp.c/declare-variant-{3,4}-[...]' for
inter-procedural value range propagation
..., that is, commit 53ba8d669550d3a1f809048428b97ca607f95cf5
"inter-procedural value range propagation", after which we see:
[-PASS:-]{+FAIL:+} libgomp.c/declare-variant-3-sm30.c scan-nvptx-none-offload-tree-dump optimized "= f30 \\(\\);"
Etc. That's due to:
@@ -144,13 +144,11 @@
__attribute__((omp target entrypoint, noclone))
void main._omp_fn.0 (const struct .omp_data_t.3 & restrict .omp_data_i)
{
- int _3;
int * _5;
[local count: 1073741824]:
- _3 = f30 ();
_5 = *.omp_data_i_4(D).v;
- *_5 = _3;
+ *_5 = 30;
return;
It's nice to see this optimization work here, too, but it does interfere with
how we're currently testing OpenMP 'declare variant'.
libgomp/
* testsuite/libgomp.c/declare-variant-3.h (f30, f35, f53, f70)
(f75, f80, f): Add '__attribute__ ((noipa))'.
* testsuite/libgomp.c/declare-variant-4.h (gfx803, gfx900, gfx906)
(gfx908, gfx90a, f): Likewise.
---
libgomp/testsuite/libgomp.c/declare-variant-3.h | 8
libgomp/testsuite/libgomp.c/declare-variant-4.h | 7 +++
2 files changed, 15 insertions(+)
diff --git a/libgomp/testsuite/libgomp.c/declare-variant-3.h b/libgomp/testsuite/libgomp.c/declare-variant-3.h
index 772fc20a519..646e15e5311 100644
--- a/libgomp/testsuite/libgomp.c/declare-variant-3.h
+++ b/libgomp/testsuite/libgomp.c/declare-variant-3.h
@@ -1,34 +1,41 @@
#pragma omp declare target
+
+__attribute__ ((noipa))
int
f30 (void)
{
return 30;
}
+__attribute__ ((noipa))
int
f35 (void)
{
return 35;
}
+__attribute__ ((noipa))
int
f53 (void)
{
return 53;
}
+__attribute__ ((noipa))
int
f70 (void)
{
return 70;
}
+__attribute__ ((noipa))
int
f75 (void)
{
return 75;
}
+__attribute__ ((noipa))
int
f80 (void)
{
@@ -41,6 +48,7 @@ f80 (void)
#pragma omp declare variant (f70) match (device={isa("sm_70")})
#pragma omp declare variant (f75) match (device={isa("sm_75")})
#pragma omp declare variant (f80) match (device={isa("sm_80")})
+__attribute__ ((noipa))
int
f (void)
{
diff --git a/libgomp/testsuite/libgomp.c/declare-variant-4.h b/libgomp/testsuite/libgomp.c/declare-variant-4.h
index 2d7c1ef1a5a..47517b75ee7 100644
--- a/libgomp/testsuite/libgomp.c/declare-variant-4.h
+++ b/libgomp/testsuite/libgomp.c/declare-variant-4.h
@@ -1,28 +1,34 @@
#pragma omp declare target
+
+__attribute__ ((noipa))
int
gfx803 (void)
{
return 0x803;
}
+__attribute__ ((noipa))
int
gfx900 (void)
{
return 0x900;
}
+__attribute__ ((noipa))
int
gfx906 (void)
{
return 0x906;
}
+__attribute__ ((noipa))
int
gfx908 (void)
{
return 0x908;
}
+__attribute__ ((noipa))
int
gfx90a (void)
{
@@ -38,6 +44,7 @@ gfx90a (void)
#pragma omp declare variant(gfx906) match(device = {isa("gfx906")})
#pragma omp declare variant(gfx908) match(device = {isa("gfx908")})
#pragma omp declare variant(gfx90a) match(device = {isa("gfx90a")})
+__attribute__ ((noipa))
int
f (void)
{
--
2.34.1
Re: Fix 'gcc.dg/tree-ssa/return-value-range-1.c' for 'char' defaulting to 'unsigned' (was: Propagate value ranges of return values)
Hi!
On 2023-11-22T11:51:02+0100, Christophe Lyon wrote:
> On Tue, 21 Nov 2023 at 22:24, Thomas Schwinge wrote:
>> On 2023-11-19T16:05:42+0100, Jan Hubicka wrote:
>> > --- /dev/null
>> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c
>>
>> Pushed to master branch commit a0240662b22312ffb3e3fefb85f258ab0e7010f4
>> "Fix 'gcc.dg/tree-ssa/return-value-range-1.c' for 'char' defaulting to
>> 'unsigned'", see attached. On powerpc64le-linux-gnu ('char' defaulting
>> to 'unsigned') I still saw:
>>
>> /tmp/ccd1xwD7.o: In function `test':
>> return-value-range-1.c:(.text+0x50): undefined reference to `link_error'
>>
> We do see the same error in our CI (Thomas, normally you have received
> a notification because your patch turned ERROR in FAIL)
Yes, I have; and I even tried to log in there, to point to my commit
mentioned above, which is meant to address this issue -- please let me
know if you're still seeing the FAIL after that commit.
> Thomas, you said in another email that adding -O2 avoids the linker
> error with missing link_error(), but I don't see how that would be
> possible?
That's the gist of Honza's "Propagate value ranges of return values"
optimization, per my understanding: from 'int a(signed char c)' doing
'return c;' figure out that 'a(d) > 200)' is always false (due to
'-128 <= c <= 127)'.
> (and hence I expect the error you quoted above to happen)
>
> So should we use dg-compile instead of dg-link? Not sure what the
> original intention was?
No, the idea really is to prove that the 'link_error ()' call is
unreachable.
Grüße
Thomas
>> > @@ -0,0 +1,22 @@
>> > +/* { dg-do ling } */
>> > +/* { dg-options "-O1 -dump-tree-evrp-details" } */
>> > +__attribute__ ((__noinline__))
>> > +int a(char c)
>> > +{
>> > + return c;
>> > +}
>> > +void link_error ();
>> > +
>> > +void
>> > +test(int d)
>> > +{
>> > + if (a(d) > 200)
>> > + link_error ();
>> > +}
>> > +int
>> > +main(int argc, char **argv)
>> > +{
>> > + test(argc);
>> > + return 0;
>> > +}
>> > +/* { dg-final { scan-tree-dump-times "Recording return range" 2 "evrp"} }
>> > */
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
Re: Fix 'gcc.dg/tree-ssa/return-value-range-1.c' for 'char' defaulting to 'unsigned' (was: Propagate value ranges of return values)
Hi!
On Tue, 21 Nov 2023 at 22:24, Thomas Schwinge wrote:
>
> Hi!
>
> On 2023-11-19T16:05:42+0100, Jan Hubicka wrote:
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c
>
> Pushed to master branch commit a0240662b22312ffb3e3fefb85f258ab0e7010f4
> "Fix 'gcc.dg/tree-ssa/return-value-range-1.c' for 'char' defaulting to
> 'unsigned'", see attached. On powerpc64le-linux-gnu ('char' defaulting
> to 'unsigned') I still saw:
>
> /tmp/ccd1xwD7.o: In function `test':
> return-value-range-1.c:(.text+0x50): undefined reference to `link_error'
>
We do see the same error in our CI (Thomas, normally you have received
a notification because your patch turned ERROR in FAIL)
Thomas, you said in another email that adding -O2 avoids the linker
error with missing link_error(), but I don't see how that would be
possible?
(and hence I expect the error you quoted above to happen)
So should we use dg-compile instead of dg-link? Not sure what the
original intention was?
Thanks,
Christophe
>
> Grüße
> Thomas
>
>
> > @@ -0,0 +1,22 @@
> > +/* { dg-do ling } */
> > +/* { dg-options "-O1 -dump-tree-evrp-details" } */
> > +__attribute__ ((__noinline__))
> > +int a(char c)
> > +{
> > + return c;
> > +}
> > +void link_error ();
> > +
> > +void
> > +test(int d)
> > +{
> > + if (a(d) > 200)
> > + link_error ();
> > +}
> > +int
> > +main(int argc, char **argv)
> > +{
> > + test(argc);
> > + return 0;
> > +}
> > +/* { dg-final { scan-tree-dump-times "Recording return range" 2 "evrp"} }
> > */
>
>
> -
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
> München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
> Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
> München, HRB 106955
Re: Propagate value ranges of return values
Hi Honza!
On 2023-11-21T15:06:54+0100, Jan Hubicka wrote:
>> After this patch in addition to the problem already reported about
>> vlda1.c and return-value-range-1.c, [...]
> return-value_range-1.c should be fixed now and I do not have vlda1.c in
> my tree. I will check.
Typo, I suppose; probably 'gcc.dg/vla-1.c' is meant here:
PASS: gcc.dg/vla-1.c (test for excess errors)
[-PASS:-]{+FAIL:+} gcc.dg/vla-1.c scan-tree-dump-times optimized " s=> i" 2
..., and can you please also check if the following also relates to your
commit? I've not looked into any details, but noticed that you did
adjusted 'gcc.dg/tree-prof/time-profiler-1.c', and
'gcc.dg/tree-prof/time-profiler-2.c', but not
'gcc.dg/tree-prof/time-profiler-3.c':
@@ -142800,9 +142802,9 @@ PASS: gcc.dg/tree-prof/time-profiler-3.c
compilation, -fprofile-generate -D_PRO
PASS: gcc.dg/tree-prof/time-profiler-3.c compilation, -fprofile-use
-D_PROFILE_USE
PASS: gcc.dg/tree-prof/time-profiler-3.c execution,-fprofile-generate
-D_PROFILE_GENERATE
PASS: gcc.dg/tree-prof/time-profiler-3.c execution,-fprofile-use
-D_PROFILE_USE
[-PASS:-]{+FAIL:+} gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times
profile "Read tp_first_run: 0" 1
PASS: gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times profile "Read
tp_first_run: 1" 1
[-PASS:-]{+FAIL:+} gcc.dg/tree-prof/time-profiler-3.c scan-ipa-dump-times
profile "Read tp_first_run: 2" 1
>> > --- a/gcc/testsuite/gcc.dg/tree-prof/time-profiler-1.c
>> > +++ b/gcc/testsuite/gcc.dg/tree-prof/time-profiler-1.c
>> > @@ -1,4 +1,4 @@
>> > -/* { dg-options "-O2 -fdump-ipa-profile" } */
>> > +/* { dg-options "-O2 -fdump-ipa-profile -fno-ipa-vrp" } */
>> > --- a/gcc/testsuite/gcc.dg/tree-prof/time-profiler-2.c
>> > +++ b/gcc/testsuite/gcc.dg/tree-prof/time-profiler-2.c
>> > @@ -1,4 +1,4 @@
>> > -/* { dg-options "-O2 -fdump-ipa-profile" } */
>> > +/* { dg-options "-O2 -fdump-ipa-profile -fno-ipa-vrp" } */
Grüße
Thomas
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
Fix 'gcc.dg/tree-ssa/return-value-range-1.c' for 'char' defaulting to 'unsigned' (was: Propagate value ranges of return values)
Hi!
On 2023-11-19T16:05:42+0100, Jan Hubicka wrote:
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c
Pushed to master branch commit a0240662b22312ffb3e3fefb85f258ab0e7010f4
"Fix 'gcc.dg/tree-ssa/return-value-range-1.c' for 'char' defaulting to
'unsigned'", see attached. On powerpc64le-linux-gnu ('char' defaulting
to 'unsigned') I still saw:
/tmp/ccd1xwD7.o: In function `test':
return-value-range-1.c:(.text+0x50): undefined reference to `link_error'
Grüße
Thomas
> @@ -0,0 +1,22 @@
> +/* { dg-do ling } */
> +/* { dg-options "-O1 -dump-tree-evrp-details" } */
> +__attribute__ ((__noinline__))
> +int a(char c)
> +{
> + return c;
> +}
> +void link_error ();
> +
> +void
> +test(int d)
> +{
> + if (a(d) > 200)
> + link_error ();
> +}
> +int
> +main(int argc, char **argv)
> +{
> + test(argc);
> + return 0;
> +}
> +/* { dg-final { scan-tree-dump-times "Recording return range" 2 "evrp"} } */
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
>From a0240662b22312ffb3e3fefb85f258ab0e7010f4 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge
Date: Tue, 21 Nov 2023 22:07:13 +0100
Subject: [PATCH] Fix 'gcc.dg/tree-ssa/return-value-range-1.c' for 'char'
defaulting to 'unsigned'
... added in recent commit 53ba8d669550d3a1f809048428b97ca607f95cf5
"inter-procedural value range propagation", fixed in
commit 878a860cae78146d98d7a21612f0bcec0930a9c2
"Fix 'gcc.dg/tree-ssa/return-value-range-1.c'".
gcc/testsuite/
* gcc.dg/tree-ssa/return-value-range-1.c: Fix.
---
gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c b/gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c
index 74f1a5080bb..97294482c05 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c
@@ -1,7 +1,7 @@
/* { dg-do link } */
/* { dg-options "-O2 -fdump-tree-evrp-details" } */
__attribute__ ((__noinline__))
-int a(char c)
+int a(signed char c)
{
return c;
}
--
2.34.1
Re: Propagate value ranges of return values
> After this patch in addition to the problem already reported about
> vlda1.c and return-value-range-1.c, we have noticed these regressions
> on aarch64:
> Running gcc:gcc.target/aarch64/aarch64.exp ...
> FAIL: gcc.target/aarch64/movk.c scan-assembler movk\tx[0-9]+, 0x4667, lsl 16
> FAIL: gcc.target/aarch64/movk.c scan-assembler movk\tx[0-9]+, 0x7a3d, lsl 32
>
> Running gcc:gcc.target/aarch64/simd/simd.exp ...
> FAIL: gcc.target/aarch64/simd/vmulxd_f64_2.c scan-assembler-times
> fmul[ \t]+[dD][0-9]+, ?[dD][0-9]+, ?[dD][0-9]+\n 1
> FAIL: gcc.target/aarch64/simd/vmulxd_f64_2.c scan-assembler-times
> fmulx[ \t]+[dD][0-9]+, ?[dD][0-9]+, ?[dD][0-9]+\n 4
> FAIL: gcc.target/aarch64/simd/vmulxs_f32_2.c scan-assembler-times
> fmul[ \t]+[sS][0-9]+, ?[sS][0-9]+, ?[sS][0-9]+\n 1
> FAIL: gcc.target/aarch64/simd/vmulxs_f32_2.c scan-assembler-times
> fmulx[ \t]+[sS][0-9]+, ?[sS][0-9]+, ?[sS][0-9]+\n 4
Sorry for that - I guess we will see some on various targets.
This is quite common issue - the testcase is having
dummy_number_generator function returning constant and prevents
inlining to avoid constant being visible to compiler. This no longer
works, since we get it from the return value range. This should fix it.
return-value_range-1.c should be fixed now and I do not have vlda1.c in
my tree. I will check.
diff --git a/gcc/testsuite/gcc.target/aarch64/movk.c
b/gcc/testsuite/gcc.target/aarch64/movk.c
index e6e4e3a8961..6b1f3f8ecf5 100644
--- a/gcc/testsuite/gcc.target/aarch64/movk.c
+++ b/gcc/testsuite/gcc.target/aarch64/movk.c
@@ -1,8 +1,9 @@
/* { dg-do run } */
-/* { dg-options "-O2 --save-temps -fno-inline" } */
+/* { dg-options "-O2 --save-temps" } */
extern void abort (void);
+__attribute__ ((noipa))
long long int
dummy_number_generator ()
{
>
> We have already sent you a notification for the regression on arm, but
> it includes on vla-1.c and return-value-range-1.c.
> The notification email contains a pointer to the page where we record
> all the configurations that regress because of this patch:
>
> https://linaro.atlassian.net/browse/GNU-1025
>
> Can you have a look?
>
> Thanks,
>
> Christophe
>
>
>
>
> > diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
> > index e41e5ad3ae7..71dacf23ce1 100644
> > --- a/gcc/cgraph.cc
> > +++ b/gcc/cgraph.cc
> > @@ -2629,6 +2629,54 @@ cgraph_node::set_malloc_flag (bool malloc_p)
> >return changed;
> > }
> >
> > +/* Worker to set malloc flag. */
> > +static void
> > +add_detected_attribute_1 (cgraph_node *node, const char *attr, bool
> > *changed)
> > +{
> > + if (!lookup_attribute (attr, DECL_ATTRIBUTES (node->decl)))
> > +{
> > + DECL_ATTRIBUTES (node->decl) = tree_cons (get_identifier (attr),
> > +NULL_TREE, DECL_ATTRIBUTES
> > (node->decl));
> > + *changed = true;
> > +}
> > +
> > + ipa_ref *ref;
> > + FOR_EACH_ALIAS (node, ref)
> > +{
> > + cgraph_node *alias = dyn_cast (ref->referring);
> > + if (alias->get_availability () > AVAIL_INTERPOSABLE)
> > + add_detected_attribute_1 (alias, attr, changed);
> > +}
> > +
> > + for (cgraph_edge *e = node->callers; e; e = e->next_caller)
> > +if (e->caller->thunk
> > + && (e->caller->get_availability () > AVAIL_INTERPOSABLE))
> > + add_detected_attribute_1 (e->caller, attr, changed);
> > +}
> > +
> > +/* Set DECL_IS_MALLOC on NODE's decl and on NODE's aliases if any. */
> > +
> > +bool
> > +cgraph_node::add_detected_attribute (const char *attr)
> > +{
> > + bool changed = false;
> > +
> > + if (get_availability () > AVAIL_INTERPOSABLE)
> > +add_detected_attribute_1 (this, attr, );
> > + else
> > +{
> > + ipa_ref *ref;
> > +
> > + FOR_EACH_ALIAS (this, ref)
> > + {
> > + cgraph_node *alias = dyn_cast (ref->referring);
> > + if (alias->get_availability () > AVAIL_INTERPOSABLE)
> > + add_detected_attribute_1 (alias, attr, );
> > + }
> > +}
> > + return changed;
> > +}
> > +
> > /* Worker to set noreturng flag. */
> > static void
> > set_noreturn_flag_1 (cgraph_node *node, bool noreturn_p, bool *changed)
> > diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> > index cedaaac3a45..cfdd9f693a8 100644
> > --- a/gcc/cgraph.h
> > +++ b/gcc/cgraph.h
> > @@ -1190,6 +1190,10 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node :
> > public symtab_node
> >
> >bool set_pure_flag (bool pure, bool looping);
> >
> > + /* Add attribute ATTR to cgraph_node's decl and on aliases of the node
> > + if any. */
> > + bool add_detected_attribute (const char *attr);
> > +
> >/* Call callback on function and aliases associated to the function.
> > When INCLUDE_OVERWRITABLE is false, overwritable aliases and thunks
> > are
> > skipped. */
> > diff --git a/gcc/common.opt b/gcc/common.opt
> > index d21db5d4a20..c6599c7147b 100644
> > --- a/gcc/common.opt
> > +++ b/gcc/common.opt
> > @@ -781,6 +781,10 @@ Wsuggest-attribute=malloc
> > Common
Re: Propagate value ranges of return values
Hi!
On Sun, 19 Nov 2023 at 16:05, Jan Hubicka wrote:
>
> Hi,
> this is updated version which also adds testuiste compensation
> I lost earlier while maintaining the patch in my testing tree.
> There are quite few testcases that use constant return values to hide
> something from optimizer.
>
> Bootstrapped/regtested x86_64-linux.
> gcc/ChangeLog:
>
> * cgraph.cc (add_detected_attribute_1): New function.
> (cgraph_node::add_detected_attribute): Likewise.
> * cgraph.h (cgraph_node::add_detected_attribute): Declare.
> * common.opt: Add -Wsuggest-attribute=returns_nonnull.
> * doc/invoke.texi: Document new flag.
> * gimple-range-fold.cc (fold_using_range::range_of_call):
> Use known reutrn value ranges.
> * ipa-prop.cc (struct ipa_return_value_summary): New type.
> (class ipa_return_value_sum_t): New type.
> (ipa_return_value_sum): New summary.
> (ipa_record_return_value_range): New function.
> (ipa_return_value_range): New function.
> * ipa-prop.h (ipa_return_value_range): Declare.
> (ipa_record_return_value_range): Declare.
> * ipa-pure-const.cc (warn_function_returns_nonnull): New funcion.
> * ipa-utils.h (warn_function_returns_nonnull): Declare.
> * symbol-summary.h: Fix comment.
> * tree-vrp.cc (execute_ranger_vrp): Record return values.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/ipa/devirt-2.C: Add noipa attribute to prevent ipa-vrp.
> * g++.dg/ipa/devirt-7.C: Disable ipa-vrp.
> * g++.dg/ipa/ipa-icf-2.C: Disable ipa-vrp.
> * g++.dg/ipa/ipa-icf-3.C: Disable ipa-vrp.
> * g++.dg/ipa/ivinline-1.C: Disable ipa-vrp.
> * g++.dg/ipa/ivinline-3.C: Disable ipa-vrp.
> * g++.dg/ipa/ivinline-5.C: Disable ipa-vrp.
> * g++.dg/ipa/ivinline-8.C: Disable ipa-vrp.
> * g++.dg/ipa/nothrow-1.C: Disable ipa-vrp.
> * g++.dg/ipa/pure-const-1.C: Disable ipa-vrp.
> * g++.dg/ipa/pure-const-2.C: Disable ipa-vrp.
> * g++.dg/lto/inline-crossmodule-1_0.C: Disable ipa-vrp.
> * gcc.c-torture/compile/pr106433.c: Add noipa attribute to prevent
> ipa-vrp.
> * gcc.c-torture/execute/frame-address.c: Likewise.
> * gcc.dg/ipa/fopt-info-inline-1.c: Disable ipa-vrp.
> * gcc.dg/ipa/ipa-icf-25.c: Disable ipa-vrp.
> * gcc.dg/ipa/ipa-icf-38.c: Disable ipa-vrp.
> * gcc.dg/ipa/pure-const-1.c: Disable ipa-vrp.
> * gcc.dg/ipa/remref-0.c: Add noipa attribute to prevent ipa-vrp.
> * gcc.dg/tree-prof/time-profiler-1.c: Disable ipa-vrp.
> * gcc.dg/tree-prof/time-profiler-2.c: Disable ipa-vrp.
> * gcc.dg/tree-ssa/pr110269.c: Disable ipa-vrp.
> * gcc.dg/tree-ssa/pr20701.c: Disable ipa-vrp.
> * gcc.dg/tree-ssa/vrp05.c: Disable ipa-vrp.
> * gcc.dg/tree-ssa/return-value-range-1.c: New test.
>
After this patch in addition to the problem already reported about
vlda1.c and return-value-range-1.c, we have noticed these regressions
on aarch64:
Running gcc:gcc.target/aarch64/aarch64.exp ...
FAIL: gcc.target/aarch64/movk.c scan-assembler movk\tx[0-9]+, 0x4667, lsl 16
FAIL: gcc.target/aarch64/movk.c scan-assembler movk\tx[0-9]+, 0x7a3d, lsl 32
Running gcc:gcc.target/aarch64/simd/simd.exp ...
FAIL: gcc.target/aarch64/simd/vmulxd_f64_2.c scan-assembler-times
fmul[ \t]+[dD][0-9]+, ?[dD][0-9]+, ?[dD][0-9]+\n 1
FAIL: gcc.target/aarch64/simd/vmulxd_f64_2.c scan-assembler-times
fmulx[ \t]+[dD][0-9]+, ?[dD][0-9]+, ?[dD][0-9]+\n 4
FAIL: gcc.target/aarch64/simd/vmulxs_f32_2.c scan-assembler-times
fmul[ \t]+[sS][0-9]+, ?[sS][0-9]+, ?[sS][0-9]+\n 1
FAIL: gcc.target/aarch64/simd/vmulxs_f32_2.c scan-assembler-times
fmulx[ \t]+[sS][0-9]+, ?[sS][0-9]+, ?[sS][0-9]+\n 4
We have already sent you a notification for the regression on arm, but
it includes on vla-1.c and return-value-range-1.c.
The notification email contains a pointer to the page where we record
all the configurations that regress because of this patch:
https://linaro.atlassian.net/browse/GNU-1025
Can you have a look?
Thanks,
Christophe
> diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
> index e41e5ad3ae7..71dacf23ce1 100644
> --- a/gcc/cgraph.cc
> +++ b/gcc/cgraph.cc
> @@ -2629,6 +2629,54 @@ cgraph_node::set_malloc_flag (bool malloc_p)
>return changed;
> }
>
> +/* Worker to set malloc flag. */
> +static void
> +add_detected_attribute_1 (cgraph_node *node, const char *attr, bool *changed)
> +{
> + if (!lookup_attribute (attr, DECL_ATTRIBUTES (node->decl)))
> +{
> + DECL_ATTRIBUTES (node->decl) = tree_cons (get_identifier (attr),
> +NULL_TREE, DECL_ATTRIBUTES
> (node->decl));
> + *changed = true;
> +}
> +
> + ipa_ref *ref;
> + FOR_EACH_ALIAS (node, ref)
> +{
> + cgraph_node *alias = dyn_cast (ref->referring);
> + if (alias->get_availability () > AVAIL_INTERPOSABLE)
> + add_detected_attribute_1
Re: Fix 'gcc.dg/tree-ssa/return-value-range-1.c' (was: Propagate value ranges of return values)
> Hi!
>
> On 2023-11-19T16:05:42+0100, Jan Hubicka wrote:
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c
> > @@ -0,0 +1,22 @@
> > +/* { dg-do ling } */
>
> ERROR: gcc.dg/tree-ssa/return-value-range-1.c: 1: syntax error for "
> dg-do 1 ling "
>
> With that fixed into 'dg-do link', and...
>
> > +/* { dg-options "-O1 -dump-tree-evrp-details" } */
>
> ... that one fixed into '-fdump-tree-evrp-details', I then get:
>
> FAIL: gcc.dg/tree-ssa/return-value-range-1.c (test for excess errors)
> UNRESOLVED: gcc.dg/tree-ssa/return-value-range-1.c scan-tree-dump-times
> evrp "Recording return range" 2
>
> /tmp/ccTEuffl.o: In function `test':
> return-value-range-1.c:(.text+0x24): undefined reference to `link_error'
>
> This disappears when switching from '-O1' to '-O2'. OK to push the
> attached "Fix 'gcc.dg/tree-ssa/return-value-range-1.c'"? (..., or did
> you intend something else, here?)
Ah sorry for that - I looked for FAIl and missed the error. Yes, the
change is OK. Indeed -fipa-vrp is enabled only at -O2. (I think basic
non-dataflow VRP could be doable and effective even at -O1, but we don't
do that)
Honza
>
>
> Grüße
> Thomas
>
>
> > +__attribute__ ((__noinline__))
> > +int a(char c)
> > +{
> > + return c;
> > +}
> > +void link_error ();
> > +
> > +void
> > +test(int d)
> > +{
> > + if (a(d) > 200)
> > + link_error ();
> > +}
> > +int
> > +main(int argc, char **argv)
> > +{
> > + test(argc);
> > + return 0;
> > +}
> > +/* { dg-final { scan-tree-dump-times "Recording return range" 2 "evrp"} }
> > */
>
>
> -
> Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
> München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
> Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
> München, HRB 106955
> From f3a47339a9df9726da7e3c1daeadc216e1d5b365 Mon Sep 17 00:00:00 2001
> From: Thomas Schwinge
> Date: Tue, 21 Nov 2023 11:51:42 +0100
> Subject: [PATCH] Fix 'gcc.dg/tree-ssa/return-value-range-1.c'
>
> ... added in recent commit 53ba8d669550d3a1f809048428b97ca607f95cf5
> "inter-procedural value range propagation".
>
> gcc/testsuite/
> * gcc.dg/tree-ssa/return-value-range-1.c: Fix.
> ---
> gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c
> b/gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c
> index 4db52233c5d..74f1a5080bb 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c
> @@ -1,5 +1,5 @@
> -/* { dg-do ling } */
> -/* { dg-options "-O1 -dump-tree-evrp-details" } */
> +/* { dg-do link } */
> +/* { dg-options "-O2 -fdump-tree-evrp-details" } */
> __attribute__ ((__noinline__))
> int a(char c)
> {
> --
> 2.34.1
>
Fix 'gcc.dg/tree-ssa/return-value-range-1.c' (was: Propagate value ranges of return values)
Hi!
On 2023-11-19T16:05:42+0100, Jan Hubicka wrote:
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c
> @@ -0,0 +1,22 @@
> +/* { dg-do ling } */
ERROR: gcc.dg/tree-ssa/return-value-range-1.c: 1: syntax error for " dg-do
1 ling "
With that fixed into 'dg-do link', and...
> +/* { dg-options "-O1 -dump-tree-evrp-details" } */
... that one fixed into '-fdump-tree-evrp-details', I then get:
FAIL: gcc.dg/tree-ssa/return-value-range-1.c (test for excess errors)
UNRESOLVED: gcc.dg/tree-ssa/return-value-range-1.c scan-tree-dump-times
evrp "Recording return range" 2
/tmp/ccTEuffl.o: In function `test':
return-value-range-1.c:(.text+0x24): undefined reference to `link_error'
This disappears when switching from '-O1' to '-O2'. OK to push the
attached "Fix 'gcc.dg/tree-ssa/return-value-range-1.c'"? (..., or did
you intend something else, here?)
Grüße
Thomas
> +__attribute__ ((__noinline__))
> +int a(char c)
> +{
> + return c;
> +}
> +void link_error ();
> +
> +void
> +test(int d)
> +{
> + if (a(d) > 200)
> + link_error ();
> +}
> +int
> +main(int argc, char **argv)
> +{
> + test(argc);
> + return 0;
> +}
> +/* { dg-final { scan-tree-dump-times "Recording return range" 2 "evrp"} } */
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
>From f3a47339a9df9726da7e3c1daeadc216e1d5b365 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge
Date: Tue, 21 Nov 2023 11:51:42 +0100
Subject: [PATCH] Fix 'gcc.dg/tree-ssa/return-value-range-1.c'
... added in recent commit 53ba8d669550d3a1f809048428b97ca607f95cf5
"inter-procedural value range propagation".
gcc/testsuite/
* gcc.dg/tree-ssa/return-value-range-1.c: Fix.
---
gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c b/gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c
index 4db52233c5d..74f1a5080bb 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/return-value-range-1.c
@@ -1,5 +1,5 @@
-/* { dg-do ling } */
-/* { dg-options "-O1 -dump-tree-evrp-details" } */
+/* { dg-do link } */
+/* { dg-options "-O2 -fdump-tree-evrp-details" } */
__attribute__ ((__noinline__))
int a(char c)
{
--
2.34.1
Re: Propagate value ranges of return values
Hi,
thanks for working on this.
On Sun, Nov 19 2023, Jan Hubicka wrote:
> Hi,
> this is updated version which also adds testuiste compensation
> I lost earlier while maintaining the patch in my testing tree.
> There are quite few testcases that use constant return values to hide
> something from optimizer.
>
> Bootstrapped/regtested x86_64-linux.
> gcc/ChangeLog:
>
> * cgraph.cc (add_detected_attribute_1): New function.
> (cgraph_node::add_detected_attribute): Likewise.
> * cgraph.h (cgraph_node::add_detected_attribute): Declare.
> * common.opt: Add -Wsuggest-attribute=returns_nonnull.
> * doc/invoke.texi: Document new flag.
> * gimple-range-fold.cc (fold_using_range::range_of_call):
> Use known reutrn value ranges.
> * ipa-prop.cc (struct ipa_return_value_summary): New type.
> (class ipa_return_value_sum_t): New type.
> (ipa_return_value_sum): New summary.
> (ipa_record_return_value_range): New function.
> (ipa_return_value_range): New function.
> * ipa-prop.h (ipa_return_value_range): Declare.
> (ipa_record_return_value_range): Declare.
> * ipa-pure-const.cc (warn_function_returns_nonnull): New funcion.
> * ipa-utils.h (warn_function_returns_nonnull): Declare.
> * symbol-summary.h: Fix comment.
> * tree-vrp.cc (execute_ranger_vrp): Record return values.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/ipa/devirt-2.C: Add noipa attribute to prevent ipa-vrp.
> * g++.dg/ipa/devirt-7.C: Disable ipa-vrp.
> * g++.dg/ipa/ipa-icf-2.C: Disable ipa-vrp.
> * g++.dg/ipa/ipa-icf-3.C: Disable ipa-vrp.
> * g++.dg/ipa/ivinline-1.C: Disable ipa-vrp.
> * g++.dg/ipa/ivinline-3.C: Disable ipa-vrp.
> * g++.dg/ipa/ivinline-5.C: Disable ipa-vrp.
> * g++.dg/ipa/ivinline-8.C: Disable ipa-vrp.
> * g++.dg/ipa/nothrow-1.C: Disable ipa-vrp.
> * g++.dg/ipa/pure-const-1.C: Disable ipa-vrp.
> * g++.dg/ipa/pure-const-2.C: Disable ipa-vrp.
> * g++.dg/lto/inline-crossmodule-1_0.C: Disable ipa-vrp.
> * gcc.c-torture/compile/pr106433.c: Add noipa attribute to prevent
> ipa-vrp.
> * gcc.c-torture/execute/frame-address.c: Likewise.
> * gcc.dg/ipa/fopt-info-inline-1.c: Disable ipa-vrp.
> * gcc.dg/ipa/ipa-icf-25.c: Disable ipa-vrp.
> * gcc.dg/ipa/ipa-icf-38.c: Disable ipa-vrp.
> * gcc.dg/ipa/pure-const-1.c: Disable ipa-vrp.
> * gcc.dg/ipa/remref-0.c: Add noipa attribute to prevent ipa-vrp.
> * gcc.dg/tree-prof/time-profiler-1.c: Disable ipa-vrp.
> * gcc.dg/tree-prof/time-profiler-2.c: Disable ipa-vrp.
> * gcc.dg/tree-ssa/pr110269.c: Disable ipa-vrp.
> * gcc.dg/tree-ssa/pr20701.c: Disable ipa-vrp.
> * gcc.dg/tree-ssa/vrp05.c: Disable ipa-vrp.
> * gcc.dg/tree-ssa/return-value-range-1.c: New test.
>
> diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
> index e41e5ad3ae7..71dacf23ce1 100644
> --- a/gcc/cgraph.cc
> +++ b/gcc/cgraph.cc
> @@ -2629,6 +2629,54 @@ cgraph_node::set_malloc_flag (bool malloc_p)
>return changed;
> }
>
> +/* Worker to set malloc flag. */
I think the comment must be stale, and the name of the function also, it
does not add anything, does it?
> +static void
> +add_detected_attribute_1 (cgraph_node *node, const char *attr, bool *changed)
> +{
> + if (!lookup_attribute (attr, DECL_ATTRIBUTES (node->decl)))
> +{
> + DECL_ATTRIBUTES (node->decl) = tree_cons (get_identifier (attr),
> + NULL_TREE, DECL_ATTRIBUTES
> (node->decl));
> + *changed = true;
> +}
> +
> + ipa_ref *ref;
> + FOR_EACH_ALIAS (node, ref)
> +{
> + cgraph_node *alias = dyn_cast (ref->referring);
> + if (alias->get_availability () > AVAIL_INTERPOSABLE)
> + add_detected_attribute_1 (alias, attr, changed);
> +}
> +
> + for (cgraph_edge *e = node->callers; e; e = e->next_caller)
> +if (e->caller->thunk
> + && (e->caller->get_availability () > AVAIL_INTERPOSABLE))
> + add_detected_attribute_1 (e->caller, attr, changed);
> +}
> +
> +/* Set DECL_IS_MALLOC on NODE's decl and on NODE's aliases if any. */
Likewise.
> +
> +bool
> +cgraph_node::add_detected_attribute (const char *attr)
> +{
> + bool changed = false;
> +
> + if (get_availability () > AVAIL_INTERPOSABLE)
> +add_detected_attribute_1 (this, attr, );
> + else
> +{
> + ipa_ref *ref;
> +
> + FOR_EACH_ALIAS (this, ref)
> + {
> + cgraph_node *alias = dyn_cast (ref->referring);
> + if (alias->get_availability () > AVAIL_INTERPOSABLE)
> + add_detected_attribute_1 (alias, attr, );
> + }
> +}
> + return changed;
> +}
> +
> /* Worker to set noreturng flag. */
> static void
> set_noreturn_flag_1 (cgraph_node *node, bool noreturn_p, bool *changed)
[...]
> diff --git a/gcc/gimple-range-fold.cc b/gcc/gimple-range-fold.cc
> index 6e9530c3d7f..998b7608d78 100644
> --- a/gcc/gimple-range-fold.cc
> +++
Re: Propagate value ranges of return values
> > On 11/18/23 20:21, Jan Hubicka wrote: > > Hi, > > this patch implements very basic propaation of return value ranges from VRP > > pass. This helps std::vector's push_back since we work out value range of > > allocated block. This propagates only within single translation unit. I > > hoped > > we will also do the propagation at WPA stage, but that needs more work on > > ipa-cp side. > > > > I also added code auto-detecting return_nonnull and corresponding > > -Wsuggest-attribute > > > > Variant of this patch bootstrapped/regtested x86_64-linux, testing with > > this version is running. I plan to commit the patch at Monday provided > > there are no issues. > > I see no obvious issues with the ranger/vrp changes... > > My only comment is that execute_ranger_vrp is called 3 times... EVRP, VRP1 > and VRP2.. perhaps more someday. As long as that is OK with the call to > warn_function_returns_nonnull(). That should be OK. Add the nonnull attribute and also local_pure_onst behaves same way. Honza > > Andrew > >
Re: Propagate value ranges of return values
On 11/18/23 20:21, Jan Hubicka wrote: Hi, this patch implements very basic propaation of return value ranges from VRP pass. This helps std::vector's push_back since we work out value range of allocated block. This propagates only within single translation unit. I hoped we will also do the propagation at WPA stage, but that needs more work on ipa-cp side. I also added code auto-detecting return_nonnull and corresponding -Wsuggest-attribute Variant of this patch bootstrapped/regtested x86_64-linux, testing with this version is running. I plan to commit the patch at Monday provided there are no issues. I see no obvious issues with the ranger/vrp changes... My only comment is that execute_ranger_vrp is called 3 times... EVRP, VRP1 and VRP2.. perhaps more someday. As long as that is OK with the call to warn_function_returns_nonnull(). Andrew
Re: Propagate value ranges of return values
Hi,
this is updated version which also adds testuiste compensation
I lost earlier while maintaining the patch in my testing tree.
There are quite few testcases that use constant return values to hide
something from optimizer.
Bootstrapped/regtested x86_64-linux.
gcc/ChangeLog:
* cgraph.cc (add_detected_attribute_1): New function.
(cgraph_node::add_detected_attribute): Likewise.
* cgraph.h (cgraph_node::add_detected_attribute): Declare.
* common.opt: Add -Wsuggest-attribute=returns_nonnull.
* doc/invoke.texi: Document new flag.
* gimple-range-fold.cc (fold_using_range::range_of_call):
Use known reutrn value ranges.
* ipa-prop.cc (struct ipa_return_value_summary): New type.
(class ipa_return_value_sum_t): New type.
(ipa_return_value_sum): New summary.
(ipa_record_return_value_range): New function.
(ipa_return_value_range): New function.
* ipa-prop.h (ipa_return_value_range): Declare.
(ipa_record_return_value_range): Declare.
* ipa-pure-const.cc (warn_function_returns_nonnull): New funcion.
* ipa-utils.h (warn_function_returns_nonnull): Declare.
* symbol-summary.h: Fix comment.
* tree-vrp.cc (execute_ranger_vrp): Record return values.
gcc/testsuite/ChangeLog:
* g++.dg/ipa/devirt-2.C: Add noipa attribute to prevent ipa-vrp.
* g++.dg/ipa/devirt-7.C: Disable ipa-vrp.
* g++.dg/ipa/ipa-icf-2.C: Disable ipa-vrp.
* g++.dg/ipa/ipa-icf-3.C: Disable ipa-vrp.
* g++.dg/ipa/ivinline-1.C: Disable ipa-vrp.
* g++.dg/ipa/ivinline-3.C: Disable ipa-vrp.
* g++.dg/ipa/ivinline-5.C: Disable ipa-vrp.
* g++.dg/ipa/ivinline-8.C: Disable ipa-vrp.
* g++.dg/ipa/nothrow-1.C: Disable ipa-vrp.
* g++.dg/ipa/pure-const-1.C: Disable ipa-vrp.
* g++.dg/ipa/pure-const-2.C: Disable ipa-vrp.
* g++.dg/lto/inline-crossmodule-1_0.C: Disable ipa-vrp.
* gcc.c-torture/compile/pr106433.c: Add noipa attribute to prevent
ipa-vrp.
* gcc.c-torture/execute/frame-address.c: Likewise.
* gcc.dg/ipa/fopt-info-inline-1.c: Disable ipa-vrp.
* gcc.dg/ipa/ipa-icf-25.c: Disable ipa-vrp.
* gcc.dg/ipa/ipa-icf-38.c: Disable ipa-vrp.
* gcc.dg/ipa/pure-const-1.c: Disable ipa-vrp.
* gcc.dg/ipa/remref-0.c: Add noipa attribute to prevent ipa-vrp.
* gcc.dg/tree-prof/time-profiler-1.c: Disable ipa-vrp.
* gcc.dg/tree-prof/time-profiler-2.c: Disable ipa-vrp.
* gcc.dg/tree-ssa/pr110269.c: Disable ipa-vrp.
* gcc.dg/tree-ssa/pr20701.c: Disable ipa-vrp.
* gcc.dg/tree-ssa/vrp05.c: Disable ipa-vrp.
* gcc.dg/tree-ssa/return-value-range-1.c: New test.
diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
index e41e5ad3ae7..71dacf23ce1 100644
--- a/gcc/cgraph.cc
+++ b/gcc/cgraph.cc
@@ -2629,6 +2629,54 @@ cgraph_node::set_malloc_flag (bool malloc_p)
return changed;
}
+/* Worker to set malloc flag. */
+static void
+add_detected_attribute_1 (cgraph_node *node, const char *attr, bool *changed)
+{
+ if (!lookup_attribute (attr, DECL_ATTRIBUTES (node->decl)))
+{
+ DECL_ATTRIBUTES (node->decl) = tree_cons (get_identifier (attr),
+NULL_TREE, DECL_ATTRIBUTES
(node->decl));
+ *changed = true;
+}
+
+ ipa_ref *ref;
+ FOR_EACH_ALIAS (node, ref)
+{
+ cgraph_node *alias = dyn_cast (ref->referring);
+ if (alias->get_availability () > AVAIL_INTERPOSABLE)
+ add_detected_attribute_1 (alias, attr, changed);
+}
+
+ for (cgraph_edge *e = node->callers; e; e = e->next_caller)
+if (e->caller->thunk
+ && (e->caller->get_availability () > AVAIL_INTERPOSABLE))
+ add_detected_attribute_1 (e->caller, attr, changed);
+}
+
+/* Set DECL_IS_MALLOC on NODE's decl and on NODE's aliases if any. */
+
+bool
+cgraph_node::add_detected_attribute (const char *attr)
+{
+ bool changed = false;
+
+ if (get_availability () > AVAIL_INTERPOSABLE)
+add_detected_attribute_1 (this, attr, );
+ else
+{
+ ipa_ref *ref;
+
+ FOR_EACH_ALIAS (this, ref)
+ {
+ cgraph_node *alias = dyn_cast (ref->referring);
+ if (alias->get_availability () > AVAIL_INTERPOSABLE)
+ add_detected_attribute_1 (alias, attr, );
+ }
+}
+ return changed;
+}
+
/* Worker to set noreturng flag. */
static void
set_noreturn_flag_1 (cgraph_node *node, bool noreturn_p, bool *changed)
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index cedaaac3a45..cfdd9f693a8 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -1190,6 +1190,10 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node :
public symtab_node
bool set_pure_flag (bool pure, bool looping);
+ /* Add attribute ATTR to cgraph_node's decl and on aliases of the node
+ if any. */
+ bool add_detected_attribute (const char *attr);
+
/* Call callback on function and aliases associated to the function.
When
Re: Propagate value ranges of return values
> > +Wsuggest-attribute=returns_nonnull > > - or _? > > (If changing it, needs adjustment in rest of patch too.) I was thinking of this and I am not sure what is better. Sure _ in command line option looks odd, but this is an identifier and it is returns_nonnull and not returns-nonnull. I am not sure we have earlier situation like that. I can live with both variants and would be happy to hear opinions on this. > > > +Common Var(warn_suggest_attribute_malloc) Warning > > +Warn about functions which might be candidates for __attribute__((malloc)). > > + > > Typo: s/malloc/nonnull/? Thanks! Honza
Re: Propagate value ranges of return values
Jan Hubicka writes:
> Hi,
> this patch implements very basic propaation of return value ranges from VRP
> pass. This helps std::vector's push_back since we work out value range of
> allocated block. This propagates only within single translation unit. I
> hoped
> we will also do the propagation at WPA stage, but that needs more work on
> ipa-cp side.
>
> I also added code auto-detecting return_nonnull and corresponding
> -Wsuggest-attribute
>
> Variant of this patch bootstrapped/regtested x86_64-linux, testing with
> this version is running. I plan to commit the patch at Monday provided
> there are no issues.
>
> gcc/ChangeLog:
>
> * cgraph.cc (add_detected_attribute_1): New function.
> (cgraph_node::add_detected_attribute): New member function.
> * cgraph.h (struct cgraph_node): Declare it.
> * common.opt: Add Wsuggest-attribute=returns_nonnull.
> * doc/invoke.texi: Document +Wsuggest-attribute=returns_nonnull.
> * gimple-range-fold.cc: Include ipa-prop and dependencies.
> (fold_using_range::range_of_call): Look for return value range.
> * ipa-prop.cc (struct ipa_return_value_summary): New structure.
> (class ipa_return_value_sum_t): New summary.
> (ipa_record_return_value_range): New function.
> (ipa_return_value_range): New function.
> * ipa-prop.h (ipa_return_value_range): Declare.
> (ipa_record_return_value_range): Declare.
> * ipa-pure-const.cc (warn_function_returns_nonnull): New function.
> * ipa-utils.h (warn_function_returns_nonnull): Declare.
> * symbol-summary.h: Fix comment typo.
> * tree-vrp.cc (execute_ranger_vrp): Record return values.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/tree-ssa/return-value-range-1.c: New test.
>
> diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
> index e41e5ad3ae7..71dacf23ce1 100644
> --- a/gcc/cgraph.cc
> +++ b/gcc/cgraph.cc
> @@ -2629,6 +2629,54 @@ cgraph_node::set_malloc_flag (bool malloc_p)
>return changed;
> }
>
> +/* Worker to set malloc flag. */
> +static void
> +add_detected_attribute_1 (cgraph_node *node, const char *attr, bool *changed)
> +{
> + if (!lookup_attribute (attr, DECL_ATTRIBUTES (node->decl)))
> +{
> + DECL_ATTRIBUTES (node->decl) = tree_cons (get_identifier (attr),
> + NULL_TREE, DECL_ATTRIBUTES
> (node->decl));
> + *changed = true;
> +}
> +
> + ipa_ref *ref;
> + FOR_EACH_ALIAS (node, ref)
> +{
> + cgraph_node *alias = dyn_cast (ref->referring);
> + if (alias->get_availability () > AVAIL_INTERPOSABLE)
> + add_detected_attribute_1 (alias, attr, changed);
> +}
> +
> + for (cgraph_edge *e = node->callers; e; e = e->next_caller)
> +if (e->caller->thunk
> + && (e->caller->get_availability () > AVAIL_INTERPOSABLE))
> + add_detected_attribute_1 (e->caller, attr, changed);
> +}
> +
> +/* Set DECL_IS_MALLOC on NODE's decl and on NODE's aliases if any. */
> +
> +bool
> +cgraph_node::add_detected_attribute (const char *attr)
> +{
> + bool changed = false;
> +
> + if (get_availability () > AVAIL_INTERPOSABLE)
> +add_detected_attribute_1 (this, attr, );
> + else
> +{
> + ipa_ref *ref;
> +
> + FOR_EACH_ALIAS (this, ref)
> + {
> + cgraph_node *alias = dyn_cast (ref->referring);
> + if (alias->get_availability () > AVAIL_INTERPOSABLE)
> + add_detected_attribute_1 (alias, attr, );
> + }
> +}
> + return changed;
> +}
> +
> /* Worker to set noreturng flag. */
> static void
> set_noreturn_flag_1 (cgraph_node *node, bool noreturn_p, bool *changed)
> diff --git a/gcc/cgraph.h b/gcc/cgraph.h
> index cedaaac3a45..cfdd9f693a8 100644
> --- a/gcc/cgraph.h
> +++ b/gcc/cgraph.h
> @@ -1190,6 +1190,10 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node :
> public symtab_node
>
>bool set_pure_flag (bool pure, bool looping);
>
> + /* Add attribute ATTR to cgraph_node's decl and on aliases of the node
> + if any. */
> + bool add_detected_attribute (const char *attr);
> +
>/* Call callback on function and aliases associated to the function.
> When INCLUDE_OVERWRITABLE is false, overwritable aliases and thunks are
> skipped. */
> diff --git a/gcc/common.opt b/gcc/common.opt
> index d21db5d4a20..0be4f02677c 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -781,6 +781,10 @@ Wsuggest-attribute=malloc
> Common Var(warn_suggest_attribute_malloc) Warning
> Warn about functions which might be candidates for __attribute__((malloc)).
>
> +Wsuggest-attribute=returns_nonnull
- or _?
(If changing it, needs adjustment in rest of patch too.)
> +Common Var(warn_suggest_attribute_malloc) Warning
> +Warn about functions which might be candidates for __attribute__((malloc)).
> +
Typo: s/malloc/nonnull/?
Propagate value ranges of return values
Hi,
this patch implements very basic propaation of return value ranges from VRP
pass. This helps std::vector's push_back since we work out value range of
allocated block. This propagates only within single translation unit. I hoped
we will also do the propagation at WPA stage, but that needs more work on
ipa-cp side.
I also added code auto-detecting return_nonnull and corresponding
-Wsuggest-attribute
Variant of this patch bootstrapped/regtested x86_64-linux, testing with
this version is running. I plan to commit the patch at Monday provided
there are no issues.
gcc/ChangeLog:
* cgraph.cc (add_detected_attribute_1): New function.
(cgraph_node::add_detected_attribute): New member function.
* cgraph.h (struct cgraph_node): Declare it.
* common.opt: Add Wsuggest-attribute=returns_nonnull.
* doc/invoke.texi: Document +Wsuggest-attribute=returns_nonnull.
* gimple-range-fold.cc: Include ipa-prop and dependencies.
(fold_using_range::range_of_call): Look for return value range.
* ipa-prop.cc (struct ipa_return_value_summary): New structure.
(class ipa_return_value_sum_t): New summary.
(ipa_record_return_value_range): New function.
(ipa_return_value_range): New function.
* ipa-prop.h (ipa_return_value_range): Declare.
(ipa_record_return_value_range): Declare.
* ipa-pure-const.cc (warn_function_returns_nonnull): New function.
* ipa-utils.h (warn_function_returns_nonnull): Declare.
* symbol-summary.h: Fix comment typo.
* tree-vrp.cc (execute_ranger_vrp): Record return values.
gcc/testsuite/ChangeLog:
* gcc.dg/tree-ssa/return-value-range-1.c: New test.
diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc
index e41e5ad3ae7..71dacf23ce1 100644
--- a/gcc/cgraph.cc
+++ b/gcc/cgraph.cc
@@ -2629,6 +2629,54 @@ cgraph_node::set_malloc_flag (bool malloc_p)
return changed;
}
+/* Worker to set malloc flag. */
+static void
+add_detected_attribute_1 (cgraph_node *node, const char *attr, bool *changed)
+{
+ if (!lookup_attribute (attr, DECL_ATTRIBUTES (node->decl)))
+{
+ DECL_ATTRIBUTES (node->decl) = tree_cons (get_identifier (attr),
+NULL_TREE, DECL_ATTRIBUTES
(node->decl));
+ *changed = true;
+}
+
+ ipa_ref *ref;
+ FOR_EACH_ALIAS (node, ref)
+{
+ cgraph_node *alias = dyn_cast (ref->referring);
+ if (alias->get_availability () > AVAIL_INTERPOSABLE)
+ add_detected_attribute_1 (alias, attr, changed);
+}
+
+ for (cgraph_edge *e = node->callers; e; e = e->next_caller)
+if (e->caller->thunk
+ && (e->caller->get_availability () > AVAIL_INTERPOSABLE))
+ add_detected_attribute_1 (e->caller, attr, changed);
+}
+
+/* Set DECL_IS_MALLOC on NODE's decl and on NODE's aliases if any. */
+
+bool
+cgraph_node::add_detected_attribute (const char *attr)
+{
+ bool changed = false;
+
+ if (get_availability () > AVAIL_INTERPOSABLE)
+add_detected_attribute_1 (this, attr, );
+ else
+{
+ ipa_ref *ref;
+
+ FOR_EACH_ALIAS (this, ref)
+ {
+ cgraph_node *alias = dyn_cast (ref->referring);
+ if (alias->get_availability () > AVAIL_INTERPOSABLE)
+ add_detected_attribute_1 (alias, attr, );
+ }
+}
+ return changed;
+}
+
/* Worker to set noreturng flag. */
static void
set_noreturn_flag_1 (cgraph_node *node, bool noreturn_p, bool *changed)
diff --git a/gcc/cgraph.h b/gcc/cgraph.h
index cedaaac3a45..cfdd9f693a8 100644
--- a/gcc/cgraph.h
+++ b/gcc/cgraph.h
@@ -1190,6 +1190,10 @@ struct GTY((tag ("SYMTAB_FUNCTION"))) cgraph_node :
public symtab_node
bool set_pure_flag (bool pure, bool looping);
+ /* Add attribute ATTR to cgraph_node's decl and on aliases of the node
+ if any. */
+ bool add_detected_attribute (const char *attr);
+
/* Call callback on function and aliases associated to the function.
When INCLUDE_OVERWRITABLE is false, overwritable aliases and thunks are
skipped. */
diff --git a/gcc/common.opt b/gcc/common.opt
index d21db5d4a20..0be4f02677c 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -781,6 +781,10 @@ Wsuggest-attribute=malloc
Common Var(warn_suggest_attribute_malloc) Warning
Warn about functions which might be candidates for __attribute__((malloc)).
+Wsuggest-attribute=returns_nonnull
+Common Var(warn_suggest_attribute_malloc) Warning
+Warn about functions which might be candidates for __attribute__((malloc)).
+
Wsuggest-final-types
Common Var(warn_suggest_final_types) Warning
Warn about C++ polymorphic types where adding final keyword would improve code
quality.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 557d613a1e6..b9e98843613 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -8092,7 +8092,7 @@ if the array is referenced as a flexible array member.
@opindex Wsuggest-attribute=
@opindex Wno-suggest-attribute=
-@item
