[gcc(refs/users/giulianob/heads/autopar_rebase2)] Optimize assignment to volatile aggregate variable

Giuliano Belinassi giulianob@gcc.gnu.org
Mon Aug 17 23:45:34 GMT 2020


https://gcc.gnu.org/g:42252ab35b97ba71d234808985d551958b12fb58

commit 42252ab35b97ba71d234808985d551958b12fb58
Author: Eric Botcazou <ebotcazou@gcc.gnu.org>
Date:   Mon Jun 15 19:56:00 2020 +0200

    Optimize assignment to volatile aggregate variable
    
    gimplify_modify_expr_rhs has an optimization whereby the assignment to
    an aggregate variable from a read-only object with a DECL_INITIAL is
    optimized into the direct assignment of the DECL_INITIAL, provided that
    no temporary is created in the process.
    
    The optimization is blocked if the read-only object is volatile, which
    is OK as per the semantics of volatile, but also if the target variable
    is volatile, on the grounds that the modified assignment might end up
    being done on a per field basis, which is also OK.  But this latter
    restriction is enforced a priori and there are cases where the modified
    assignment would be OK, for example if there is only one field or the
    DECL_INITIAL is equivalent to the empty CONSTRUCTOR, i.e. all zeros.
    
    So, in the latter case, the patch changes gimplify_modify_expr_rhs to ask
    gimplify_init_constructor whether the assignment would be done as a block,
    which is easy because gimplify_init_constructor knows that it must create
    a temporary if the LHS is volatile and this would not be the case, so it's
    just a matter of completing the NOTIFY_TEMP_CREATION mechanism.
    
    gcc/ChangeLog
            * gimplify.c (gimplify_init_constructor) <AGGREGATE_TYPE>: Declare
            new ENSURE_SINGLE_ACCESS constant and move variables down.  If it is
            true and all elements are zero, then always clear.  Return GS_ERROR
            if a temporary would be created for it and NOTIFY_TEMP_CREATION set.
            (gimplify_modify_expr_rhs) <VAR_DECL>: If the target is volatile but
            the type is aggregate non-addressable, ask gimplify_init_constructor
            whether it can generate a single access to the target.
    
    gcc/testsuite/ChangeLog
            * gnat.dg/aggr30.ads, gnat.dg/aggr30.adb: New test.

Diff:
---
 gcc/gimplify.c                   | 53 +++++++++++++++++++++++++++-------------
 gcc/testsuite/gnat.dg/aggr30.adb | 20 +++++++++++++++
 gcc/testsuite/gnat.dg/aggr30.ads | 25 +++++++++++++++++++
 3 files changed, 81 insertions(+), 17 deletions(-)

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 416fb609b94..9851edfc4db 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -4865,10 +4865,6 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
     case QUAL_UNION_TYPE:
     case ARRAY_TYPE:
       {
-	struct gimplify_init_ctor_preeval_data preeval_data;
-	HOST_WIDE_INT num_ctor_elements, num_nonzero_elements;
-	HOST_WIDE_INT num_unique_nonzero_elements;
-	bool cleared, complete_p, valid_const_initializer;
 	/* Use readonly data for initializers of this or smaller size
 	   regardless of the num_nonzero_elements / num_unique_nonzero_elements
 	   ratio.  */
@@ -4876,6 +4872,17 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	/* If num_nonzero_elements / num_unique_nonzero_elements ratio
 	   is smaller than this, use readonly data.  */
 	const int unique_nonzero_ratio = 8;
+	/* True if a single access of the object must be ensured.  This is the
+	   case if the target is volatile, the type is non-addressable and more
+	   than one field need to be assigned.  */
+	const bool ensure_single_access
+	  = TREE_THIS_VOLATILE (object)
+	    && !TREE_ADDRESSABLE (type)
+	    && vec_safe_length (elts) > 1;
+	struct gimplify_init_ctor_preeval_data preeval_data;
+	HOST_WIDE_INT num_ctor_elements, num_nonzero_elements;
+	HOST_WIDE_INT num_unique_nonzero_elements;
+	bool cleared, complete_p, valid_const_initializer;
 
 	/* Aggregate types must lower constructors to initialization of
 	   individual elements.  The exception is that a CONSTRUCTOR node
@@ -4914,6 +4921,7 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	  {
 	    if (notify_temp_creation)
 	      return GS_ERROR;
+
 	    DECL_INITIAL (object) = ctor;
 	    TREE_STATIC (object) = 1;
 	    if (!DECL_NAME (object))
@@ -4961,6 +4969,10 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	  /* If there are "lots" of zeros, it's more efficient to clear
 	     the memory and then set the nonzero elements.  */
 	  cleared = true;
+	else if (ensure_single_access && num_nonzero_elements == 0)
+	  /* If a single access to the target must be ensured and all elements
+	     are zero, then it's optimal to clear whatever their number.  */
+	  cleared = true;
 	else
 	  cleared = false;
 
@@ -5029,13 +5041,14 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	      }
 	  }
 
-	/* If the target is volatile, we have non-zero elements and more than
-	   one field to assign, initialize the target from a temporary.  */
-	if (TREE_THIS_VOLATILE (object)
-	    && !TREE_ADDRESSABLE (type)
-	    && (num_nonzero_elements > 0 || !cleared)
-	    && vec_safe_length (elts) > 1)
+	/* If a single access to the target must be ensured and there are
+	   nonzero elements or the zero elements are not assigned en masse,
+	   initialize the target from a temporary.  */
+	if (ensure_single_access && (num_nonzero_elements > 0 || !cleared))
 	  {
+	    if (notify_temp_creation)
+	      return GS_ERROR;
+
 	    tree temp = create_tmp_var (TYPE_MAIN_VARIANT (type));
 	    TREE_OPERAND (*expr_p, 0) = temp;
 	    *expr_p = build2 (COMPOUND_EXPR, TREE_TYPE (*expr_p),
@@ -5243,14 +5256,20 @@ gimplify_modify_expr_rhs (tree *expr_p, tree *from_p, tree *to_p,
 	{
 	case VAR_DECL:
 	  /* If we're assigning from a read-only variable initialized with
-	     a constructor, do the direct assignment from the constructor,
-	     but only if neither source nor target are volatile since this
-	     latter assignment might end up being done on a per-field basis.  */
-	  if (DECL_INITIAL (*from_p)
-	      && TREE_READONLY (*from_p)
+	     a constructor and not volatile, do the direct assignment from
+	     the constructor, but only if the target is not volatile either
+	     since this latter assignment might end up being done on a per
+	     field basis.  However, if the target is volatile and the type
+	     is aggregate and non-addressable, gimplify_init_constructor
+	     knows that it needs to ensure a single access to the target
+	     and it will return GS_OK only in this case.  */
+	  if (TREE_READONLY (*from_p)
+	      && DECL_INITIAL (*from_p)
+	      && TREE_CODE (DECL_INITIAL (*from_p)) == CONSTRUCTOR
 	      && !TREE_THIS_VOLATILE (*from_p)
-	      && !TREE_THIS_VOLATILE (*to_p)
-	      && TREE_CODE (DECL_INITIAL (*from_p)) == CONSTRUCTOR)
+	      && (!TREE_THIS_VOLATILE (*to_p)
+		  || (AGGREGATE_TYPE_P (TREE_TYPE (*to_p))
+		      && !TREE_ADDRESSABLE (TREE_TYPE (*to_p)))))
 	    {
 	      tree old_from = *from_p;
 	      enum gimplify_status subret;
diff --git a/gcc/testsuite/gnat.dg/aggr30.adb b/gcc/testsuite/gnat.dg/aggr30.adb
new file mode 100644
index 00000000000..a69c8b651ea
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/aggr30.adb
@@ -0,0 +1,20 @@
+-- { dg-do compile }
+-- { dg-options "-fdump-tree-gimple" }
+
+package body Aggr30 is
+
+   Null_Constant : constant Rec := (Data => (others => 0),
+                                    Padding => (others => 0));
+   procedure Init is
+   begin
+      Instance := Null_Constant;
+   end;
+
+   procedure Init_Volatile is
+   begin
+      Instance_Volatile := Null_Constant;
+   end;
+
+end Aggr30;
+
+-- { dg-final { scan-tree-dump-times "= {}" 2 "gimple"} }
diff --git a/gcc/testsuite/gnat.dg/aggr30.ads b/gcc/testsuite/gnat.dg/aggr30.ads
new file mode 100644
index 00000000000..998403487e7
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/aggr30.ads
@@ -0,0 +1,25 @@
+with Interfaces;
+
+package Aggr30 is
+
+   type Data_Type is array (1 .. 4) of Interfaces.Unsigned_8;
+
+   type Padding_Type is array (5 .. 4096) of Interfaces.Unsigned_8;
+
+   type Rec is record
+      Data    : Data_Type;
+      Padding : Padding_Type;
+   end record;
+
+   procedure Init;
+
+   procedure Init_Volatile;
+
+private
+
+   Instance : Rec;
+
+   Instance_Volatile : Rec;
+   pragma Volatile (Instance_Volatile);
+
+end Aggr30;


More information about the Gcc-cvs mailing list