diff mbox series

Set TREE_THIS_NOTRAP throughout tree-nested.c

Message ID 3162661.bVZMVEFtQX@arcturus.home
State New
Headers show
Series Set TREE_THIS_NOTRAP throughout tree-nested.c | expand

Commit Message

Eric Botcazou July 24, 2019, 9:14 a.m. UTC 
Hi,

stack memory is considered non-trapping by the compiler once the frame has 
been established so TREE_THIS_NOTRAP can be set on the dereferences built 
during the unnesting pass.

Tested on x86_64-suse-linux, OK for the mainline?


2019-07-24  Eric Botcazou  <ebotcazou@adacore.com>

	* tree-nested.c (build_simple_mem_ref_notrap): New function.
	(get_static_chain): Call it instead of (build_simple_mem_ref.
	(get_frame_field): Likewise.
	(get_nonlocal_debug_decl): Likewise.
	(convert_nonlocal_reference_op): Likewise.

Comments

Richard Biener July 24, 2019, 1:29 p.m. UTC | #1 
On Wed, Jul 24, 2019 at 11:14 AM Eric Botcazou <ebotcazou@adacore.com> wrote:
>
> Hi,
>
> stack memory is considered non-trapping by the compiler once the frame has
> been established so TREE_THIS_NOTRAP can be set on the dereferences built
> during the unnesting pass.
>
> Tested on x86_64-suse-linux, OK for the mainline?

OK.

Thanks,
Richard.

>
> 2019-07-24  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * tree-nested.c (build_simple_mem_ref_notrap): New function.
>         (get_static_chain): Call it instead of (build_simple_mem_ref.
>         (get_frame_field): Likewise.
>         (get_nonlocal_debug_decl): Likewise.
>         (convert_nonlocal_reference_op): Likewise.
>
> --
> Eric Botcazou
Martin Liška Aug. 2, 2019, 6:20 a.m. UTC | #2 
On 7/24/19 11:14 AM, Eric Botcazou wrote:
> Hi,
> 
> stack memory is considered non-trapping by the compiler once the frame has 
> been established so TREE_THIS_NOTRAP can be set on the dereferences built 
> during the unnesting pass.
> 
> Tested on x86_64-suse-linux, OK for the mainline?
> 
> 
> 2019-07-24  Eric Botcazou  <ebotcazou@adacore.com>
> 
> 	* tree-nested.c (build_simple_mem_ref_notrap): New function.
> 	(get_static_chain): Call it instead of (build_simple_mem_ref.
> 	(get_frame_field): Likewise.
> 	(get_nonlocal_debug_decl): Likewise.
> 	(convert_nonlocal_reference_op): Likewise.
> 

Note that the revision caused size increase of 548.exchange2_r SPEC 2017 benchmark:
https://lnt.opensuse.org/db_default/v4/SPEC/graph?plot.0=35.398.4

Martin
Eric Botcazou Aug. 2, 2019, 7:35 a.m. UTC | #3 
> Note that the revision caused size increase of 548.exchange2_r SPEC 2017
> benchmark:
> https://lnt.opensuse.org/db_default/v4/SPEC/graph?plot.0=35.398.4

Are you sure?  This should only change anything for nested functions.
Martin Liška Aug. 2, 2019, 10:06 a.m. UTC | #4 
On 8/2/19 9:35 AM, Eric Botcazou wrote:
>> Note that the revision caused size increase of 548.exchange2_r SPEC 2017
>> benchmark:
>> https://lnt.opensuse.org/db_default/v4/SPEC/graph?plot.0=35.398.4
> 
> Are you sure?  This should only change anything for nested functions.
> 

Yes, I've just verified that. Do you have access to the SPEC benchmark?

First big change is in before/exchange2.fppized.f90.130t.pre

I'm attaching diff.
Martin
Eric Botcazou Aug. 2, 2019, 10:15 a.m. UTC | #5 
> Yes, I've just verified that. Do you have access to the SPEC benchmark?

Nope.

> First big change is in before/exchange2.fppized.f90.130t.pre

Is that at -Os?  If not, then this isn't necessarily a problem.  It looks like 
PRE is able to PRE-ify more stores, very likely because they no longer trap.
Martin Liška Aug. 2, 2019, 10:18 a.m. UTC | #6 
On 8/2/19 12:15 PM, Eric Botcazou wrote:
>> Yes, I've just verified that. Do you have access to the SPEC benchmark?
> 
> Nope.
> 
>> First big change is in before/exchange2.fppized.f90.130t.pre
> 
> Is that at -Os?  If not, then this isn't necessarily a problem.  It looks like 
> PRE is able to PRE-ify more stores, very likely because they no longer trap.
> 

It's -Ofast.

Martin
diff mbox series

Patch

commit cbe1e80c5af525403608dc00ef5e92c5a9f85ee1
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Wed Jul 17 18:11:32 2019 +0200

    Part of work for S716-019 (compiler crash on nested task type at -O2).

diff --git a/gcc/tree-nested.c b/gcc/tree-nested.c
index 60dfc548b5a..74c70681d40 100644
--- a/gcc/tree-nested.c
+++ b/gcc/tree-nested.c
@@ -169,6 +169,16 @@  create_tmp_var_for (struct nesting_info *info, tree type, const char *prefix)
   return tmp_var;
 }
 
+/* Like build_simple_mem_ref, but set TREE_THIS_NOTRAP on the result.  */
+
+static tree
+build_simple_mem_ref_notrap (tree ptr)
+{
+  tree t = build_simple_mem_ref (ptr);
+  TREE_THIS_NOTRAP (t) = 1;
+  return t;
+}
+
 /* Take the address of EXP to be used within function CONTEXT.
    Mark it for addressability as necessary.  */
 
@@ -877,7 +887,7 @@  get_static_chain (struct nesting_info *info, tree target_context,
 	{
 	  tree field = get_chain_field (i);
 
-	  x = build_simple_mem_ref (x);
+	  x = build_simple_mem_ref_notrap (x);
 	  x = build3 (COMPONENT_REF, TREE_TYPE (field), x, field, NULL_TREE);
 	  x = init_tmp_var (info, x, gsi);
 	}
@@ -914,12 +924,12 @@  get_frame_field (struct nesting_info *info, tree target_context,
 	{
 	  tree field = get_chain_field (i);
 
-	  x = build_simple_mem_ref (x);
+	  x = build_simple_mem_ref_notrap (x);
 	  x = build3 (COMPONENT_REF, TREE_TYPE (field), x, field, NULL_TREE);
 	  x = init_tmp_var (info, x, gsi);
 	}
 
-      x = build_simple_mem_ref (x);
+      x = build_simple_mem_ref_notrap (x);
     }
 
   x = build3 (COMPONENT_REF, TREE_TYPE (field), x, field, NULL_TREE);
@@ -963,16 +973,16 @@  get_nonlocal_debug_decl (struct nesting_info *info, tree decl)
       for (i = info->outer; i->context != target_context; i = i->outer)
 	{
 	  field = get_chain_field (i);
-	  x = build_simple_mem_ref (x);
+	  x = build_simple_mem_ref_notrap (x);
 	  x = build3 (COMPONENT_REF, TREE_TYPE (field), x, field, NULL_TREE);
 	}
-      x = build_simple_mem_ref (x);
+      x = build_simple_mem_ref_notrap (x);
     }
 
   field = lookup_field_for_decl (i, decl, INSERT);
   x = build3 (COMPONENT_REF, TREE_TYPE (field), x, field, NULL_TREE);
   if (use_pointer_in_frame (decl))
-    x = build_simple_mem_ref (x);
+    x = build_simple_mem_ref_notrap (x);
 
   /* ??? We should be remapping types as well, surely.  */
   new_decl = build_decl (DECL_SOURCE_LOCATION (decl),
@@ -1060,7 +1070,7 @@  convert_nonlocal_reference_op (tree *tp, int *walk_subtrees, void *data)
 	    if (use_pointer_in_frame (t))
 	      {
 		x = init_tmp_var (info, x, &wi->gsi);
-		x = build_simple_mem_ref (x);
+		x = build_simple_mem_ref_notrap (x);
 	      }
 	  }