[gcc r13-8366] Fix internal error on non-byte-aligned reference in GIMPLE DSE

[gcc r13-8366] Fix internal error on non-byte-aligned reference in GIMPLE DSE

[Eric Botcazou]

https://gcc.gnu.org/g:5cc9ae567c627e84d32ba4ae13bab9bbeada33c0

commit r13-8366-g5cc9ae567c627e84d32ba4ae13bab9bbeada33c0
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Tue Feb 27 18:01:00 2024 +0100

    Fix internal error on non-byte-aligned reference in GIMPLE DSE
    
    This is a regression present on the mainline, 13 and 12 branches.  For the
    attached Ada case, it's a tree checking failure on the mainline at -O:
    
    +===========================GNAT BUG DETECTED==============================+
    | 14.0.1 20240226 (experimental) [master r14-9171-g4972f97a265]  GCC error:|
    | tree check: expected tree that contains 'decl common' structure,         |
    | have 'component_ref' in tree_could_trap_p, at tree-eh.cc:2733        |
    | Error detected around /home/eric/cvs/gcc/gcc/testsuite/gnat.dg/opt104.adb:
    
    Time is a 10-byte record and Packed_Rec.T is placed at bit-offset 65 because
    of the packing. so tree-ssa-dse.cc:setup_live_bytes_from_ref has computed a
    const_size of 88 from ref->offset of 65 and ref->max_size of 80.
    
    Then in tree-ssa-dse.cc:compute_trims:
    
    411       int last_live = bitmap_last_set_bit (live);
    (gdb) next
    412       if (ref->size.is_constant (&const_size))
    (gdb)
    414           int last_orig = (const_size / BITS_PER_UNIT) - 1;
    (gdb)
    418           *trim_tail = last_orig - last_live;
    
    (gdb) call debug_bitmap (live)
    n_bits = 256, set = {0 1 2 3 4 5 6 7 8 9 10 }
    (gdb) p last_live
    $33 = 10
    (gdb) p const_size
    $34 = 80
    (gdb) p last_orig
    $35 = 9
    (gdb) p *trim_tail
    $36 = -1
    
    In other words, compute_trims is overlooking the alignment adjustments that
    setup_live_bytes_from_ref applied earlier.  Moveover it reads:
    
      /* We use sbitmaps biased such that ref->offset is bit zero and the bitmap
         extends through ref->size.  So we know that in the original bitmap
         bits 0..ref->size were true.  We don't actually need the bitmap, just
         the REF to compute the trims.  */
    
    but setup_live_bytes_from_ref used ref->max_size instead of ref->size.
    
    It appears that all the callers of compute_trims assume that ref->offset is
    byte aligned and that the trimmed bytes are relative to ref->size, so the
    patch simply adds an early return if either condition is not fulfilled.
    
    gcc/
            * tree-ssa-dse.cc (compute_trims): Fix description.  Return early
            if either ref->offset is not byte aligned or ref->size is not known
            to be equal to ref->max_size.
            (maybe_trim_complex_store): Fix description.
            (maybe_trim_constructor_store): Likewise.
            (maybe_trim_partially_dead_store): Likewise.
    
    gcc/testsuite/
            * gnat.dg/opt104.ads, gnat.dg/opt104.adb: New test.

Diff:
---
 gcc/testsuite/gnat.dg/opt104.adb | 22 +++++++++++++++++
 gcc/testsuite/gnat.dg/opt104.ads | 40 +++++++++++++++++++++++++++++++
 gcc/tree-ssa-dse.cc              | 51 +++++++++++++++++++++-------------------
 3 files changed, 89 insertions(+), 24 deletions(-)

diff --git a/gcc/testsuite/gnat.dg/opt104.adb b/gcc/testsuite/gnat.dg/opt104.adb
new file mode 100644
index 00000000000..1548c3ef17a
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/opt104.adb
@@ -0,0 +1,22 @@
+-- { dg-do compile }
+-- { dg-options "-O -gnatws" }
+
+package body Opt104 is
+
+   procedure Proc (R : Rec) is
+      Data : Packed_Rec;
+
+   begin
+      case R.D is
+         when True =>
+            for I in 1 .. R.Len loop
+               exit;
+            end loop;
+
+         when False =>
+            null;
+      end case;
+   end;
+
+end Opt104;
+
diff --git a/gcc/testsuite/gnat.dg/opt104.ads b/gcc/testsuite/gnat.dg/opt104.ads
new file mode 100644
index 00000000000..a177ed08169
--- /dev/null
+++ b/gcc/testsuite/gnat.dg/opt104.ads
@@ -0,0 +1,40 @@
+package Opt104 is
+
+   type Time is record
+      S  : Integer;
+      B1 : Boolean;
+      B2 : Boolean;
+      B3 : Boolean;
+      B4 : Boolean;
+      B5 : Boolean;
+      B6 : Boolean;
+   end record;
+
+   Zero_Time : constant Time :=
+     (S  => 0,
+      B1 => False,
+      B2 => False,
+      B3 => False,
+      B4 => False,
+      B5 => False,
+      B6 => False);
+
+   type Root is tagged null record;
+
+   type Packed_Rec is record
+      R : Root;
+      B : Boolean;
+      T : Time := Zero_Time;
+   end record;
+   pragma Pack (Packed_Rec);
+
+   type Rec (D : Boolean) is record
+      case D is
+         when True  => Len : Integer;
+         when False => null;
+      end case;
+   end record;
+
+   procedure Proc (R : Rec);
+
+end Opt104;
diff --git a/gcc/tree-ssa-dse.cc b/gcc/tree-ssa-dse.cc
index eabe8ba4522..e28eb88068a 100644
--- a/gcc/tree-ssa-dse.cc
+++ b/gcc/tree-ssa-dse.cc
@@ -388,11 +388,11 @@ setup_live_bytes_from_ref (ao_ref *ref, sbitmap live_bytes)
   return false;
 }
 
-/* Compute the number of elements that we can trim from the head and
-   tail of ORIG resulting in a bitmap that is a superset of LIVE.
+/* Compute the number of stored bytes that we can trim from the head and
+   tail of REF.  LIVE is the bitmap of stores to REF that are still live.
 
-   Store the number of elements trimmed from the head and tail in
-   TRIM_HEAD and TRIM_TAIL.
+   Store the number of bytes trimmed from the head and tail in TRIM_HEAD
+   and TRIM_TAIL respectively.
 
    STMT is the statement being trimmed and is used for debugging dump
    output only.  */
@@ -401,10 +401,17 @@ static void
 compute_trims (ao_ref *ref, sbitmap live, int *trim_head, int *trim_tail,
 	       gimple *stmt)
 {
-  /* We use sbitmaps biased such that ref->offset is bit zero and the bitmap
-     extends through ref->size.  So we know that in the original bitmap
-     bits 0..ref->size were true.  We don't actually need the bitmap, just
-     the REF to compute the trims.  */
+  *trim_head = 0;
+  *trim_tail = 0;
+
+  /* We use bitmaps biased such that ref->offset is contained in bit zero and
+     the bitmap extends through ref->max_size, so we know that in the original
+     bitmap bits 0 .. ref->max_size were true.  But we need to check that this
+     covers the bytes of REF exactly.  */
+  const unsigned int align = known_alignment (ref->offset);
+  if ((align > 0 && align < BITS_PER_UNIT)
+      || !known_eq (ref->size, ref->max_size))
+    return;
 
   /* Now identify how much, if any of the tail we can chop off.  */
   HOST_WIDE_INT const_size;
@@ -429,8 +436,6 @@ compute_trims (ao_ref *ref, sbitmap live, int *trim_head, int *trim_tail,
 			       last_orig) <= 0)
 	*trim_tail = 0;
     }
-  else
-    *trim_tail = 0;
 
   /* Identify how much, if any of the head we can chop off.  */
   int first_orig = 0;
@@ -488,8 +493,7 @@ compute_trims (ao_ref *ref, sbitmap live, int *trim_head, int *trim_tail,
 	}
     }
 
-  if ((*trim_head || *trim_tail)
-      && dump_file && (dump_flags & TDF_DETAILS))
+  if ((*trim_head || *trim_tail) && dump_file && (dump_flags & TDF_DETAILS))
     {
       fprintf (dump_file, "  Trimming statement (head = %d, tail = %d): ",
 	       *trim_head, *trim_tail);
@@ -498,12 +502,12 @@ compute_trims (ao_ref *ref, sbitmap live, int *trim_head, int *trim_tail,
     }
 }
 
-/* STMT initializes an object from COMPLEX_CST where one or more of the
-   bytes written may be dead stores.  REF is a representation of the
-   memory written.  LIVE is the bitmap of stores that are actually live.
+/* STMT initializes an object from COMPLEX_CST where one or more of the bytes
+   written may be dead stores.  REF is a representation of the memory written.
+   LIVE is the bitmap of stores to REF that are still live.
 
-   Attempt to rewrite STMT so that only the real or imaginary part of
-   the object is actually stored.  */
+   Attempt to rewrite STMT so that only the real or the imaginary part of the
+   object is actually stored.  */
 
 static void
 maybe_trim_complex_store (ao_ref *ref, sbitmap live, gimple *stmt)
@@ -539,11 +543,10 @@ maybe_trim_complex_store (ao_ref *ref, sbitmap live, gimple *stmt)
 }
 
 /* STMT initializes an object using a CONSTRUCTOR where one or more of the
-   bytes written are dead stores.  ORIG is the bitmap of bytes stored by
-   STMT.  LIVE is the bitmap of stores that are actually live.
+   bytes written are dead stores.  REF is a representation of the memory
+   written.  LIVE is the bitmap of stores to REF that are still live.
 
-   Attempt to rewrite STMT so that only the real or imaginary part of
-   the object is actually stored.
+   Attempt to rewrite STMT so that it writes fewer memory locations.
 
    The most common case for getting here is a CONSTRUCTOR with no elements
    being used to zero initialize an object.  We do not try to handle other
@@ -765,9 +768,9 @@ maybe_trim_memstar_call (ao_ref *ref, sbitmap live, gimple *stmt)
     }
 }
 
-/* STMT is a memory write where one or more bytes written are dead
-   stores.  ORIG is the bitmap of bytes stored by STMT.  LIVE is the
-   bitmap of stores that are actually live.
+/* STMT is a memory write where one or more bytes written are dead stores.
+   REF is a representation of the memory written.  LIVE is the bitmap of
+   stores to REF that are still live.
 
    Attempt to rewrite STMT so that it writes fewer memory locations.  Right
    now we only support trimming at the start or end of the memory region.