public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-8373] Fix wrong SRA with VIEW_CONVERT_EXPR and reverse SSO
@ 2022-05-13  9:30 Eric Botcazou
  0 siblings, 0 replies; only message in thread
From: Eric Botcazou @ 2022-05-13  9:30 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:f8598704c0bc75a98e0b4a66ebd36f72bfa75334

commit r12-8373-gf8598704c0bc75a98e0b4a66ebd36f72bfa75334
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Fri May 13 11:15:08 2022 +0200

    Fix wrong SRA with VIEW_CONVERT_EXPR and reverse SSO
    
    Most cases of VIEW_CONVERT_EXPRs involving reverse scalar storage order are
    disqualified for SRA because they are storage_order_barrier_p, but you can
    still have a VIEW_CONVERT_EXPR to a regular composite type being applied to
    a component of a record type with reverse scalar storage order.
    
    In this case the bypass for !useless_type_conversion_p in sra_modify_assign,
    albeit already heavily guarded, triggers and may generate wrong code, so the
    patch makes sure that it does only when the SSO is the same on both side.
    
    gcc/
            * tree-sra.cc (sra_modify_assign): Check that scalar storage order
            is the same on the LHS and RHS before rewriting one with the model
            of the other.
    gcc/testsuite/
            * gnat.dg/sso17.adb: New test.

Diff:
---
 gcc/testsuite/gnat.dg/sso17.adb | 34 +++++++++++++++++++++++++++++
 gcc/tree-sra.cc                 | 47 ++++++++++++++++++++---------------------
 2 files changed, 57 insertions(+), 24 deletions(-)

diff --git a/gcc/testsuite/gnat.dg/sso17.adb b/gcc/testsuite/gnat.dg/sso17.adb
new file mode 100644
index 00000000000..ed57580f0c5
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/sso17.adb
@@ -0,0 +1,34 @@
+--  { dg-do run }
+--  { dg-options "-gnatws -O" }
+
+with System;
+
+procedure SSO17 is
+
+  type My_Float is new Float range 0.0 .. 359.99;
+
+  type Rec is record
+    Az : My_Float;
+    El : My_Float;
+  end record;
+  for Rec'Bit_Order use System.High_Order_First;
+  for Rec'Scalar_Storage_Order use System.High_Order_First;
+
+  R : Rec;
+
+  procedure Is_True (B : Boolean);
+  pragma No_Inline (Is_True);
+
+  procedure Is_True (B : Boolean) is
+  begin
+    if not B then
+      raise Program_Error;
+    end if;
+  end;
+
+begin
+  R := (Az => 1.1, El => 2.2);
+  Is_True (R.Az'Valid);
+  R := (Az => 3.3, El => 4.4);
+  Is_True (R.Az'Valid);
+end;
diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc
index a86f8c01346..081c51b58a4 100644
--- a/gcc/tree-sra.cc
+++ b/gcc/tree-sra.cc
@@ -4270,32 +4270,31 @@ sra_modify_assign (gimple *stmt, gimple_stmt_iterator *gsi)
       sra_stats.exprs++;
     }
 
-  if (modify_this_stmt)
-    {
-      if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
+  if (modify_this_stmt
+      && !useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
+    {
+      /* If we can avoid creating a VIEW_CONVERT_EXPR, then do so.
+	 ??? This should move to fold_stmt which we simply should
+	 call after building a VIEW_CONVERT_EXPR here.  */
+      if (AGGREGATE_TYPE_P (TREE_TYPE (lhs))
+	  && TYPE_REVERSE_STORAGE_ORDER (TREE_TYPE (lhs)) == racc->reverse
+	  && !contains_bitfld_component_ref_p (lhs))
 	{
-	  /* If we can avoid creating a VIEW_CONVERT_EXPR do so.
-	     ???  This should move to fold_stmt which we simply should
-	     call after building a VIEW_CONVERT_EXPR here.  */
-	  if (AGGREGATE_TYPE_P (TREE_TYPE (lhs))
-	      && !contains_bitfld_component_ref_p (lhs))
-	    {
-	      lhs = build_ref_for_model (loc, lhs, 0, racc, gsi, false);
-	      gimple_assign_set_lhs (stmt, lhs);
-	    }
-	  else if (lacc
-		   && AGGREGATE_TYPE_P (TREE_TYPE (rhs))
-		   && !contains_vce_or_bfcref_p (rhs))
-	    rhs = build_ref_for_model (loc, rhs, 0, lacc, gsi, false);
+	  lhs = build_ref_for_model (loc, lhs, 0, racc, gsi, false);
+	  gimple_assign_set_lhs (stmt, lhs);
+	}
+      else if (lacc
+	       && AGGREGATE_TYPE_P (TREE_TYPE (rhs))
+	       && TYPE_REVERSE_STORAGE_ORDER (TREE_TYPE (rhs)) == lacc->reverse
+	       && !contains_vce_or_bfcref_p (rhs))
+	rhs = build_ref_for_model (loc, rhs, 0, lacc, gsi, false);
 
-	  if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
-	    {
-	      rhs = fold_build1_loc (loc, VIEW_CONVERT_EXPR, TREE_TYPE (lhs),
-				     rhs);
-	      if (is_gimple_reg_type (TREE_TYPE (lhs))
-		  && TREE_CODE (lhs) != SSA_NAME)
-		force_gimple_rhs = true;
-	    }
+      if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
+	{
+	  rhs = fold_build1_loc (loc, VIEW_CONVERT_EXPR, TREE_TYPE (lhs), rhs);
+	  if (is_gimple_reg_type (TREE_TYPE (lhs))
+	      && TREE_CODE (lhs) != SSA_NAME)
+	    force_gimple_rhs = true;
 	}
     }


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-05-13  9:30 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-13  9:30 [gcc r12-8373] Fix wrong SRA with VIEW_CONVERT_EXPR and reverse SSO Eric Botcazou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).