Patchwork PATCH RFC: Clean up TREE_THIS_NOTRAP, copy it to MEM_REF

login
register
mail settings
Submitter Ian Taylor
Date Oct. 8, 2010, 11:54 p.m.
Message ID <mcrr5g0z1em.fsf@google.com>
Download mbox | patch
Permalink /patch/67306/
State New
Headers show

Comments

Ian Taylor - Oct. 8, 2010, 11:54 p.m.
This patch does two things:

1) It adds a check that TREE_THIS_NOTRAP is only used on nodes for which
   it is defined.  It extends that list to include MEM_REF and
   TARGET_MEM_REF.  It fixes two cases in emit-rtl.c which used
   TREE_THIS_NOTRAP inappropriately, on nodes types which do not use the
   flag.

2) When the gimplifier creates a MEM_REF for an INDIRECT_REF, it copies
   over the TREE_THIS_NOTRAP flag.  This is the more important part of
   the change, as it improves the generated code when using
   -fnon-call-exceptions.

I tried to also add checking for TREE_NOTHROW, which is another use of
the same flag in the tree_base structure, but that was not quite so easy
because the C++ frontend uses TREE_NOTHROW on the C++ specific node type
AGGR_INIT_EXPR.  I would have to change at least some of the uses in the
C++ frontend to use CP_TREE_NOTHROW or some such thing.  I have not
bothered to try that.

This patch is entirely in the middle-end, so I do not need approval.
However, I would like to know if there are any comments on this before I
commit it.

Ian


2010-10-08  Ian Lance Taylor  <iant@google.com>

	* tree.h (TREE_THIS_NOTRAP): Use TREE_CHECK5.
	* emit-rtl.c (set_mem_attributes_minus_bitpos): Only check
	TREE_THIS_NOTRAP when appropriate.
	(get_spill_slot_decl): Don't set TREE_THIS_NOTRAP on the new
	VAR_DECL.
	* gimplify.c (gimplify_expr): Copy NOTRAP from INDIRECT_REF to
	newly created MEM_REF.
Richard Guenther - Oct. 9, 2010, 10:52 a.m.
On Sat, Oct 9, 2010 at 1:54 AM, Ian Lance Taylor <iant@google.com> wrote:
> This patch does two things:
>
> 1) It adds a check that TREE_THIS_NOTRAP is only used on nodes for which
>   it is defined.  It extends that list to include MEM_REF and
>   TARGET_MEM_REF.  It fixes two cases in emit-rtl.c which used
>   TREE_THIS_NOTRAP inappropriately, on nodes types which do not use the
>   flag.
>
> 2) When the gimplifier creates a MEM_REF for an INDIRECT_REF, it copies
>   over the TREE_THIS_NOTRAP flag.  This is the more important part of
>   the change, as it improves the generated code when using
>   -fnon-call-exceptions.
>
> I tried to also add checking for TREE_NOTHROW, which is another use of
> the same flag in the tree_base structure, but that was not quite so easy
> because the C++ frontend uses TREE_NOTHROW on the C++ specific node type
> AGGR_INIT_EXPR.  I would have to change at least some of the uses in the
> C++ frontend to use CP_TREE_NOTHROW or some such thing.  I have not
> bothered to try that.
>
> This patch is entirely in the middle-end, so I do not need approval.
> However, I would like to know if there are any comments on this before I
> commit it.

Looks good.  I think TREE_THIS_NOTRAP is relatively new (and no FE
generated it sofar).

Richard.

> Ian
>
>
> 2010-10-08  Ian Lance Taylor  <iant@google.com>
>
>        * tree.h (TREE_THIS_NOTRAP): Use TREE_CHECK5.
>        * emit-rtl.c (set_mem_attributes_minus_bitpos): Only check
>        TREE_THIS_NOTRAP when appropriate.
>        (get_spill_slot_decl): Don't set TREE_THIS_NOTRAP on the new
>        VAR_DECL.
>        * gimplify.c (gimplify_expr): Copy NOTRAP from INDIRECT_REF to
>        newly created MEM_REF.
>
>
>
Ian Taylor - Oct. 11, 2010, 2:54 p.m.
Richard Guenther <richard.guenther@gmail.com> writes:

> Looks good.  I think TREE_THIS_NOTRAP is relatively new (and no FE
> generated it sofar).

Java does.

Thanks for looking at the patch.

Ian

Patch

Index: tree.h
===================================================================
--- tree.h	(revision 165193)
+++ tree.h	(working copy)
@@ -601,7 +601,7 @@  struct GTY(()) tree_common {
            all types
 
        TREE_THIS_NOTRAP in
-          INDIRECT_REF, ARRAY_REF, ARRAY_RANGE_REF
+          INDIRECT_REF, MEM_REF, TARGET_MEM_REF, ARRAY_REF, ARRAY_RANGE_REF
 
    deprecated_flag:
 
@@ -1255,7 +1255,9 @@  extern void omp_clause_range_check_faile
    (or slice of the array) always belongs to the range of the array.
    I.e. that the access will not trap, provided that the access to
    the base to the array will not trap.  */
-#define TREE_THIS_NOTRAP(NODE) ((NODE)->base.nothrow_flag)
+#define TREE_THIS_NOTRAP(NODE) \
+  (TREE_CHECK5 (NODE, INDIRECT_REF, MEM_REF, TARGET_MEM_REF, ARRAY_REF,	\
+		ARRAY_RANGE_REF)->base.nothrow_flag)
 
 /* In a VAR_DECL, PARM_DECL or FIELD_DECL, or any kind of ..._REF node,
    nonzero means it may not be the lhs of an assignment.
@@ -1296,8 +1298,10 @@  extern void omp_clause_range_check_faile
    In a BLOCK, this means that the block contains variables that are used.  */
 #define TREE_USED(NODE) ((NODE)->base.used_flag)
 
-/* In a FUNCTION_DECL, nonzero means a call to the function cannot throw
-   an exception.  In a CALL_EXPR, nonzero means the call cannot throw.  */
+/* In a FUNCTION_DECL, nonzero means a call to the function cannot
+   throw an exception.  In a CALL_EXPR, nonzero means the call cannot
+   throw.  We can't easily check the node type here as the C++
+   frontend also uses this flag (for AGGR_INIT_EXPR).  */
 #define TREE_NOTHROW(NODE) ((NODE)->base.nothrow_flag)
 
 /* In a CALL_EXPR, means that it's safe to use the target of the call
Index: gimplify.c
===================================================================
--- gimplify.c	(revision 165193)
+++ gimplify.c	(working copy)
@@ -6798,6 +6798,7 @@  gimplify_expr (tree *expr_p, gimple_seq 
 	case INDIRECT_REF:
 	  {
 	    bool volatilep = TREE_THIS_VOLATILE (*expr_p);
+	    bool notrap = TREE_THIS_NOTRAP (*expr_p);
 	    tree saved_ptr_type = TREE_TYPE (TREE_OPERAND (*expr_p, 0));
 
 	    *expr_p = fold_indirect_ref_loc (input_location, *expr_p);
@@ -6818,6 +6819,7 @@  gimplify_expr (tree *expr_p, gimple_seq 
 				       TREE_OPERAND (*expr_p, 0),
 				       build_int_cst (saved_ptr_type, 0));
 	    TREE_THIS_VOLATILE (*expr_p) = volatilep;
+	    TREE_THIS_NOTRAP (*expr_p) = notrap;
 	    ret = GS_OK;
 	    break;
 	  }
Index: emit-rtl.c
===================================================================
--- emit-rtl.c	(revision 165193)
+++ emit-rtl.c	(working copy)
@@ -1660,7 +1660,11 @@  set_mem_attributes_minus_bitpos (rtx ref
 	  else
 	    MEM_NOTRAP_P (ref) = 1;
 	}
-      else
+      else if (TREE_CODE (base) == INDIRECT_REF
+	       || TREE_CODE (base) == MEM_REF
+	       || TREE_CODE (base) == TARGET_MEM_REF
+	       || TREE_CODE (base) == ARRAY_REF
+	       || TREE_CODE (base) == ARRAY_RANGE_REF)
 	MEM_NOTRAP_P (ref) = TREE_THIS_NOTRAP (base);
 
       base = get_base_address (base);
@@ -2236,7 +2240,6 @@  get_spill_slot_decl (bool force_build_p)
   DECL_ARTIFICIAL (d) = 1;
   DECL_IGNORED_P (d) = 1;
   TREE_USED (d) = 1;
-  TREE_THIS_NOTRAP (d) = 1;
   spill_slot_decl = d;
 
   rd = gen_rtx_MEM (BLKmode, frame_pointer_rtx);