[PATCH] Remove static marker for range in alloca pass.

[PATCH] Remove static marker for range in alloca pass.

[Aldy Hernandez]

The m_ranges[] field in int_range<N> are trees, so they live in GC
space.  Since invalid_range is static, it must be marked with GTY
magic.  However, calculating invalid_range is not particularly slow,
or on a critical path, so we can just put it in local scope and
recalculate every time.

Tested on x86-64 Linux.

Since this is more of a GC thing than a range thing, I'd like a nod from
a global reviewer.

OK?

gcc/ChangeLog:

	PR tree-optimization/102560
	* gimple-ssa-warn-alloca.c (alloca_call_type): Remove static
	marker for invalid_range.

gcc/testsuite/ChangeLog:

	* g++.dg/Walloca2.C: New test.
---
 gcc/gimple-ssa-warn-alloca.c    | 7 +++----
 gcc/testsuite/g++.dg/Walloca2.C | 6 ++++++
 2 files changed, 9 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/Walloca2.C

diff --git a/gcc/gimple-ssa-warn-alloca.c b/gcc/gimple-ssa-warn-alloca.c
index 4fc7125d378..d59cea8d4ec 100644
--- a/gcc/gimple-ssa-warn-alloca.c
+++ b/gcc/gimple-ssa-warn-alloca.c
@@ -221,10 +221,9 @@ alloca_call_type (gimple *stmt, bool is_vla)
       && !r.varying_p ())
     {
       // The invalid bits are anything outside of [0, MAX_SIZE].
-      static int_range<2> invalid_range (build_int_cst (size_type_node, 0),
-					 build_int_cst (size_type_node,
-							max_size),
-					 VR_ANTI_RANGE);
+      int_range<2> invalid_range (build_int_cst (size_type_node, 0),
+				  build_int_cst (size_type_node, max_size),
+				  VR_ANTI_RANGE);
 
       r.intersect (invalid_range);
       if (r.undefined_p ())
diff --git a/gcc/testsuite/g++.dg/Walloca2.C b/gcc/testsuite/g++.dg/Walloca2.C
new file mode 100644
index 00000000000..b6992d08bf3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/Walloca2.C
@@ -0,0 +1,6 @@
+// { dg-do compile }
+// { dg-options "-Walloca-larger-than=4207115063 -Wvla-larger-than=1233877270 -O2 --param ggc-min-heapsize=0 --param ggc-min-expand=0 -w" }
+// { dg-require-effective-target alloca }
+
+int a;
+char *b = static_cast<char *>(__builtin_alloca (a));
-- 
2.31.1

Re: [PATCH] Remove static marker for range in alloca pass.

[Richard Biener]

On Mon, Oct 4, 2021 at 8:55 AM Aldy Hernandez via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> The m_ranges[] field in int_range<N> are trees, so they live in GC
> space.  Since invalid_range is static, it must be marked with GTY
> magic.  However, calculating invalid_range is not particularly slow,
> or on a critical path, so we can just put it in local scope and
> recalculate every time.
>
> Tested on x86-64 Linux.
>
> Since this is more of a GC thing than a range thing, I'd like a nod from
> a global reviewer.
>
> OK?

OK, but can we somehow make int_range<>::intersect work
with non-tree as well?  That is, can we somehow make this
operation w/o constructing temporary trees?

Thanks,
Richard.

> gcc/ChangeLog:
>
>         PR tree-optimization/102560
>         * gimple-ssa-warn-alloca.c (alloca_call_type): Remove static
>         marker for invalid_range.
>
> gcc/testsuite/ChangeLog:
>
>         * g++.dg/Walloca2.C: New test.
> ---
>  gcc/gimple-ssa-warn-alloca.c    | 7 +++----
>  gcc/testsuite/g++.dg/Walloca2.C | 6 ++++++
>  2 files changed, 9 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/Walloca2.C
>
> diff --git a/gcc/gimple-ssa-warn-alloca.c b/gcc/gimple-ssa-warn-alloca.c
> index 4fc7125d378..d59cea8d4ec 100644
> --- a/gcc/gimple-ssa-warn-alloca.c
> +++ b/gcc/gimple-ssa-warn-alloca.c
> @@ -221,10 +221,9 @@ alloca_call_type (gimple *stmt, bool is_vla)
>        && !r.varying_p ())
>      {
>        // The invalid bits are anything outside of [0, MAX_SIZE].
> -      static int_range<2> invalid_range (build_int_cst (size_type_node, 0),
> -                                        build_int_cst (size_type_node,
> -                                                       max_size),
> -                                        VR_ANTI_RANGE);
> +      int_range<2> invalid_range (build_int_cst (size_type_node, 0),
> +                                 build_int_cst (size_type_node, max_size),
> +                                 VR_ANTI_RANGE);
>
>        r.intersect (invalid_range);
>        if (r.undefined_p ())
> diff --git a/gcc/testsuite/g++.dg/Walloca2.C b/gcc/testsuite/g++.dg/Walloca2.C
> new file mode 100644
> index 00000000000..b6992d08bf3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/Walloca2.C
> @@ -0,0 +1,6 @@
> +// { dg-do compile }
> +// { dg-options "-Walloca-larger-than=4207115063 -Wvla-larger-than=1233877270 -O2 --param ggc-min-heapsize=0 --param ggc-min-expand=0 -w" }
> +// { dg-require-effective-target alloca }
> +
> +int a;
> +char *b = static_cast<char *>(__builtin_alloca (a));
> --
> 2.31.1
>

Re: [PATCH] Remove static marker for range in alloca pass.

[Andrew MacLeod]

On 10/4/21 4:15 AM, Richard Biener via Gcc-patches wrote:
> On Mon, Oct 4, 2021 at 8:55 AM Aldy Hernandez via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>> The m_ranges[] field in int_range<N> are trees, so they live in GC
>> space.  Since invalid_range is static, it must be marked with GTY
>> magic.  However, calculating invalid_range is not particularly slow,
>> or on a critical path, so we can just put it in local scope and
>> recalculate every time.
>>
>> Tested on x86-64 Linux.
>>
>> Since this is more of a GC thing than a range thing, I'd like a nod from
>> a global reviewer.
>>
>> OK?
> OK, but can we somehow make int_range<>::intersect work
> with non-tree as well?  That is, can we somehow make this
> operation w/o constructing temporary trees?
>
> Thanks,
> Richard.

Yeah, I'll shortly provide an intersect which takes 2 wide_ints, along 
with some other performance improvements.

  It can be more efficient since the general case has to build into a 
temporary range, and if its single pair, we can go directly into the 
original range, and possible even avoid ever creating a new tree.

Andrew