[Bug tree-optimization/113727] [14 Regression] csmith: differences from nothing to -O1 since r14-4612-g6decda1a35be57

["rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113727

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jamborm at gcc dot gnu.org

--- Comment #21 from Richard Biener <rguenth at gcc dot gnu.org> ---
Ah, so the mistake happens in 135.sra which does

   <bb 2> [local count: 178992760]:
-  as.f3 = 5;
+  as$f3_6 = 5;

   <bb 3> [local count: 894749064]:
   # y_24 = PHI <y_14(5), 0(2)>
   # as_27 = PHI <as_12(5), 169(2)>
+  # as$f3_8 = PHI <as$f3_4(5), as$f3_6(2)>
   _1 = as_27 & 31;
   if (_1 != 0)
     goto <bb 5>; [50.00%]
@@ -26,12 +39,12 @@
     goto <bb 4>; [50.00%]

   <bb 4> [local count: 447374532]:
-  cstore_19 = MEM <struct f> [(void *)&as].f3;
+  cstore_19 = as$f3_8;

   <bb 5> [local count: 894749064]:
   # as_12 = PHI <as_27(4), 66(3)>
   # cstore_20 = PHI <cstore_19(4), 154(3)>
-  MEM <struct f> [(void *)&as].f3 = cstore_20;
+  as$f3_4 = cstore_20;
   y_14 = y_24 + 1;
   if (y_14 <= 4)
     goto <bb 3>; [80.00%]
@@ -41,8 +54,12 @@
   <bb 6> [local count: 178992760]:
   # as_28 = PHI <as_12(5)>
   BIT_FIELD_REF <as, 8, 0> = as_28;
+  as$f3_22 = as.f3;
+  as.f3 = as$f3_22;
   aq1 = as;


note how we elide as.f3 but in BB6 fail to process the BIT_FIELD_REF
but then re-materialize as.f3 as if 'as' were fully stored to by
the BIT_FIELD_REF.

The BIT_FIELD_REF should have triggered re-materialization before it.

Upon handling BIT_FIELD_REF <as, 8, 0> = as_28; we create the re-load
of as.f3, but as said we fail to re-materialize 'as' before it from the
replacement.

For the following aggregate copy we run into

      if (access_has_children_p (lacc)
          && access_has_children_p (racc)
          /* When an access represents an unscalarizable region, it usually
             represents accesses with variable offset and thus must not be used
             to generate new memory accesses.  */
          && !lacc->grp_unscalarizable_region
          && !racc->grp_unscalarizable_region)
        {
          struct subreplacement_assignment_data sad;

          sad.left_offset = lacc->offset;
          sad.assignment_lhs = lhs;
          sad.assignment_rhs = rhs; 
          sad.top_racc = racc;
          sad.old_gsi = *gsi;
          sad.new_gsi = gsi;
          sad.loc = gimple_location (stmt);
          sad.refreshed = SRA_UDH_NONE;

          if (lacc->grp_read && !lacc->grp_covered)
            handle_unscalarized_data_in_subtree (&sad);

which I think is a similar situation in that the BIT_FIELD_REF on the LHS
overlaps with replacements and is a RMW operation.  I think SRA simply
assumes that any non-aggregate copy will hever partially invalidate
replacements?

I'm not sure how BIT_FIELD_REF was handled (and worked) before my change,
we record the whole variable as access for the BIT_FIELD_REF write
(but with ->grp_partial_lhs set).  But we do not look at grp_partial_lhs
when analyzing for overlaps.

The following fixes this, but a "better" change would be to record the
proper extent, including the BIT_FIELD_REF, even for LHS?  Before my RHS
handling change we likely always produced a replacement for the BIT_FIELD_REF
base and kept the BIT_FIELD_REFs around, correct?

diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc
index f8e71ec48b9..848bb8b89e0 100644
--- a/gcc/tree-sra.cc
+++ b/gcc/tree-sra.cc
@@ -2269,6 +2269,11 @@ sort_and_splice_var_accesses (tree var)
           && TREE_CODE (access->expr) == COMPONENT_REF
           && DECL_BIT_FIELD (TREE_OPERAND (access->expr, 1)));

+      /* When there is a partial LHS involved we have no way to see what it
+        accesses, so if it's not the only access we have to fail.  */
+      if (access->grp_partial_lhs && access_count != 1)
+       return NULL;
+
       if (first || access->offset >= high)
        {
          first = false;