https://gcc.gnu.org/g:7d3d58554cfdac348e62b4c340cf56499c8d3003

commit r13-9891-g7d3d58554cfdac348e62b4c340cf56499c8d3003
Author: Martin Jambor <[email protected]>
Date:   Thu Jul 17 18:28:33 2025 +0200

    tree-sra: Fix grp_covered flag computation when totally scalarizing 
(PR117423)
    
    Testcase of PR 117423 shows a flaw in the fancy way we do "total
    scalarization" in SRA now.  We use the types encountered in the
    function body and not in type declaration (allowing us to totally
    scalarize when only one union field is ever used, since we effectively
    "skip" the union then) and can accommodate pre-existing accesses that
    happen to fall into padding.
    
    In this case, we skipped the union (bypassing the
    totally_scalarizable_type_p check) and the access falling into the
    "padding" is an aggregate and so not a candidate for SRA but actually
    containing data.  Arguably total scalarization should just bail out
    when it encounters this situation (but I decided not to depend on this
    mainly because we'd need to detect all cases when we eventually cannot
    scalarize, such as when a scalar access has children accesses) but the
    actual bug is that the detection if all data in an aggregate is indeed
    covered by replacements just assumes that is always the case if total
    scalarization triggers which however may not be the case in cases like
    this - and perhaps more.
    
    This patch fixes the bug by just assuming that all padding is taken
    care of when total scalarization triggered, not that every access was
    actually scalarized.
    
    gcc/ChangeLog:
    
    2025-07-17  Martin Jambor  <[email protected]>
    
            PR tree-optimization/117423
            * tree-sra.cc (analyze_access_subtree): Fix computation of 
grp_covered
            flag.
    
    gcc/testsuite/ChangeLog:
    
    2025-07-17  Martin Jambor  <[email protected]>
    
            PR tree-optimization/117423
            * gcc.dg/tree-ssa/pr117423.c: New test.
    
    (cherry picked from commit 7efca50a9a18e69a4921140fc5a0325e3f519b1a)

Diff:
---
 gcc/testsuite/gcc.dg/tree-ssa/pr117423.c | 49 ++++++++++++++++++++++++++++++++
 gcc/tree-sra.cc                          |  9 ++++--
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr117423.c 
b/gcc/testsuite/gcc.dg/tree-ssa/pr117423.c
new file mode 100644
index 000000000000..a5d3b29886f8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr117423.c
@@ -0,0 +1,49 @@
+/* { dg-do run } */
+/* { dg-options "-O1" } */
+
+struct s4 {
+    int _0;
+};
+struct s1 {
+    unsigned char _4;
+    long _1;
+};
+struct s2 {
+    union {
+        struct s3 {
+            unsigned char _1;
+            struct s4 _0;
+        } var_0;
+        struct s1 var_1;
+    } DATA;
+};
+int f1(int arg0) { return arg0 > 12345; }
+__attribute__((noinline))
+struct s4 f2(int arg0) {
+    struct s4 rv = {arg0};
+    return rv;
+}
+struct s2 f3(int arg0) {
+    struct s2 rv;
+    struct s1 var6 = {0};
+    struct s4 var7;
+    if (f1(arg0)) {
+        rv.DATA.var_1 = var6;
+        return rv;
+    } else {
+        rv.DATA.var_0._1 = 2;
+        var7 = f2(arg0);
+        rv.DATA.var_0._0 = var7;
+        return rv;
+    }
+}
+int main() {
+  if (f3(12345).DATA.var_0._0._0 == 12345)
+    ;
+  else
+    __builtin_abort();
+  if (f3(12344).DATA.var_0._0._0 == 12344)
+    ;
+  else
+    __builtin_abort();
+}
diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc
index c3c0a70338d2..6533e340c3e7 100644
--- a/gcc/tree-sra.cc
+++ b/gcc/tree-sra.cc
@@ -2687,7 +2687,10 @@ analyze_access_subtree (struct access *root, struct 
access *parent,
 
   for (child = root->first_child; child; child = child->next_sibling)
     {
-      hole |= covered_to < child->offset;
+      if (totally)
+       covered_to = child->offset;
+      else
+       hole |= covered_to < child->offset;
       sth_created |= analyze_access_subtree (child, root,
                                             allow_replacements && !scalar,
                                             totally);
@@ -2697,6 +2700,8 @@ analyze_access_subtree (struct access *root, struct 
access *parent,
        covered_to += child->size;
       else
        hole = true;
+      if (totally && !hole)
+       covered_to = limit;
     }
 
   if (allow_replacements && scalar && !root->first_child
@@ -2765,7 +2770,7 @@ analyze_access_subtree (struct access *root, struct 
access *parent,
        root->grp_total_scalarization = 0;
     }
 
-  if (!hole || totally)
+  if (!hole)
     root->grp_covered = 1;
   else if (root->grp_write || comes_initialized_p (root->base))
     root->grp_unscalarized_data = 1; /* not covered and written to */

Reply via email to