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
Related show

Commit Message

Eric Botcazou July 24, 2019, 9:14 a.m.
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.

-- 
Eric Botcazou

Comments

Richard Biener July 24, 2019, 1:29 p.m. | #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. | #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. | #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.

-- 
Eric Botcazou
Martin Liška Aug. 2, 2019, 10:06 a.m. | #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. | #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.

-- 
Eric Botcazou
Martin Liška Aug. 2, 2019, 10:18 a.m. | #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

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);
 	      }
 	  }