Fix wrong SRA with VIEW_CONVERT_EXPR and reverse SSO
authorEric Botcazou <ebotcazou@adacore.com>
Fri, 13 May 2022 09:15:08 +0000 (11:15 +0200)
committerEric Botcazou <ebotcazou@adacore.com>
Fri, 13 May 2022 09:19:01 +0000 (11:19 +0200)
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.

gcc/testsuite/gnat.dg/sso17.adb [new file with mode: 0644]
gcc/tree-sra.cc

diff --git a/gcc/testsuite/gnat.dg/sso17.adb b/gcc/testsuite/gnat.dg/sso17.adb
new file mode 100644 (file)
index 0000000..ed57580
--- /dev/null
@@ -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;
index a86f8c0..081c51b 100644 (file)
@@ -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;
        }
     }