Plug loophole in string store merging
authorEric Botcazou <ebotcazou@adacore.com>
Fri, 20 Nov 2020 11:21:12 +0000 (12:21 +0100)
committerEric Botcazou <ebotcazou@adacore.com>
Fri, 20 Nov 2020 11:24:08 +0000 (12:24 +0100)
There is a loophole in new string store merging support added recently:
it does not check that the stores are consecutive, which is obviously
required if you want to concatenate them...  Simple fix attached, the
nice thing being that it can fall back to the regular processing if
any hole is detected in the series of stores, thanks to the handling
of STRING_CST by native_encode_expr.

gcc/ChangeLog:
* gimple-ssa-store-merging.c (struct merged_store_group): Add
new 'consecutive' field.
(merged_store_group): Set it to true.
(do_merge): Set it to false if the store is not consecutive and
set string_concatenation to false in this case.
(merge_into): Call do_merge on entry.
(merge_overlapping): Likewise.

gcc/testsuite/ChangeLog:
* gnat.dg/opt90a.adb: New test.
* gnat.dg/opt90b.adb: Likewise.
* gnat.dg/opt90c.adb: Likewise.
* gnat.dg/opt90d.adb: Likewise.
* gnat.dg/opt90e.adb: Likewise.
* gnat.dg/opt90a_pkg.ads: New helper.
* gnat.dg/opt90b_pkg.ads: Likewise.
* gnat.dg/opt90c_pkg.ads: Likewise.
* gnat.dg/opt90d_pkg.ads: Likewise.
* gnat.dg/opt90e_pkg.ads: Likewise.

gcc/gimple-ssa-store-merging.c
gcc/testsuite/gnat.dg/opt90a.adb [new file with mode: 0644]
gcc/testsuite/gnat.dg/opt90a_pkg.ads [new file with mode: 0644]
gcc/testsuite/gnat.dg/opt90b.adb [new file with mode: 0644]
gcc/testsuite/gnat.dg/opt90b_pkg.ads [new file with mode: 0644]
gcc/testsuite/gnat.dg/opt90c.adb [new file with mode: 0644]
gcc/testsuite/gnat.dg/opt90c_pkg.ads [new file with mode: 0644]
gcc/testsuite/gnat.dg/opt90d.adb [new file with mode: 0644]
gcc/testsuite/gnat.dg/opt90d_pkg.ads [new file with mode: 0644]
gcc/testsuite/gnat.dg/opt90e.adb [new file with mode: 0644]
gcc/testsuite/gnat.dg/opt90e_pkg.ads [new file with mode: 0644]

index 6089faf7ac87baca96b14d0d8ed248f41a74a264..17a4250d77f4e54ceaaeefd80c1e64e022fdd22c 100644 (file)
@@ -1450,6 +1450,7 @@ public:
   bool bit_insertion;
   bool string_concatenation;
   bool only_constants;
+  bool consecutive;
   unsigned int first_nonmergeable_order;
   int lp_nr;
 
@@ -1822,6 +1823,7 @@ merged_store_group::merged_store_group (store_immediate_info *info)
   bit_insertion = info->rhs_code == BIT_INSERT_EXPR;
   string_concatenation = info->rhs_code == STRING_CST;
   only_constants = info->rhs_code == INTEGER_CST;
+  consecutive = true;
   first_nonmergeable_order = ~0U;
   lp_nr = info->lp_nr;
   unsigned HOST_WIDE_INT align_bitpos = 0;
@@ -1957,6 +1959,9 @@ merged_store_group::do_merge (store_immediate_info *info)
       first_stmt = stmt;
     }
 
+  if (info->bitpos != start + width)
+    consecutive = false;
+
   /* We need to use extraction if there is any bit-field.  */
   if (info->rhs_code == BIT_INSERT_EXPR)
     {
@@ -1964,13 +1969,17 @@ merged_store_group::do_merge (store_immediate_info *info)
       gcc_assert (!string_concatenation);
     }
 
-  /* We need to use concatenation if there is any string.  */
+  /* We want to use concatenation if there is any string.  */
   if (info->rhs_code == STRING_CST)
     {
       string_concatenation = true;
       gcc_assert (!bit_insertion);
     }
 
+  /* But we cannot use it if we don't have consecutive stores.  */
+  if (!consecutive)
+    string_concatenation = false;
+
   if (info->rhs_code != INTEGER_CST)
     only_constants = false;
 }
@@ -1982,12 +1991,13 @@ merged_store_group::do_merge (store_immediate_info *info)
 void
 merged_store_group::merge_into (store_immediate_info *info)
 {
+  do_merge (info);
+
   /* Make sure we're inserting in the position we think we're inserting.  */
   gcc_assert (info->bitpos >= start + width
              && info->bitregion_start <= bitregion_end);
 
   width = info->bitpos + info->bitsize - start;
-  do_merge (info);
 }
 
 /* Merge a store described by INFO into this merged store.
@@ -1997,11 +2007,11 @@ merged_store_group::merge_into (store_immediate_info *info)
 void
 merged_store_group::merge_overlapping (store_immediate_info *info)
 {
+  do_merge (info);
+
   /* If the store extends the size of the group, extend the width.  */
   if (info->bitpos + info->bitsize > start + width)
     width = info->bitpos + info->bitsize - start;
-
-  do_merge (info);
 }
 
 /* Go through all the recorded stores in this group in program order and
diff --git a/gcc/testsuite/gnat.dg/opt90a.adb b/gcc/testsuite/gnat.dg/opt90a.adb
new file mode 100644 (file)
index 0000000..7de6289
--- /dev/null
@@ -0,0 +1,16 @@
+-- { dg-do run }
+-- { dg-options "-O2" }
+
+with Ada.Calendar; use Ada.Calendar;
+with Opt90a_Pkg; use Opt90a_Pkg;
+
+procedure Opt90a is
+  B : constant Integer := Year (Clock);
+  V : Data;
+
+begin
+  V := (R => (A => 0, B => B, C => "000000000000"));
+  if V.R.B /= B then
+    raise Program_Error;
+  end if;
+end;
diff --git a/gcc/testsuite/gnat.dg/opt90a_pkg.ads b/gcc/testsuite/gnat.dg/opt90a_pkg.ads
new file mode 100644 (file)
index 0000000..10a527b
--- /dev/null
@@ -0,0 +1,15 @@
+package Opt90a_Pkg is
+
+  type Rec is record
+    A : Short_Short_Integer;
+    B : Integer;
+    C : String (1 .. 12);
+  end record;
+  pragma Pack (Rec);
+  for Rec'Alignment use 1;
+
+  type Data is tagged record
+    R : Rec;
+  end record;
+
+end Opt90a_Pkg;
diff --git a/gcc/testsuite/gnat.dg/opt90b.adb b/gcc/testsuite/gnat.dg/opt90b.adb
new file mode 100644 (file)
index 0000000..6da58bb
--- /dev/null
@@ -0,0 +1,16 @@
+-- { dg-do run }
+-- { dg-options "-O2" }
+
+with Ada.Calendar; use Ada.Calendar;
+with Opt90b_Pkg; use Opt90b_Pkg;
+
+procedure Opt90b is
+  B : constant Integer := Year (Clock);
+  V : Data;
+
+begin
+  V := (R => (A => 0, B => B, C => 0, D => "000000000000"));
+  if V.R.B /= B then
+    raise Program_Error;
+  end if;
+end;
diff --git a/gcc/testsuite/gnat.dg/opt90b_pkg.ads b/gcc/testsuite/gnat.dg/opt90b_pkg.ads
new file mode 100644 (file)
index 0000000..f0b233c
--- /dev/null
@@ -0,0 +1,16 @@
+package Opt90b_Pkg is
+
+  type Rec is record
+    A : Short_Short_Integer;
+    B : Integer;
+    C : Short_Integer;
+    D : String (1 .. 12);
+  end record;
+  pragma Pack (Rec);
+  for Rec'Alignment use 1;
+
+  type Data is tagged record
+    R : Rec;
+  end record;
+
+end Opt90b_Pkg;
diff --git a/gcc/testsuite/gnat.dg/opt90c.adb b/gcc/testsuite/gnat.dg/opt90c.adb
new file mode 100644 (file)
index 0000000..b4f4c27
--- /dev/null
@@ -0,0 +1,16 @@
+-- { dg-do run }
+-- { dg-options "-O2" }
+
+with Ada.Calendar; use Ada.Calendar;
+with Opt90c_Pkg; use Opt90c_Pkg;
+
+procedure Opt90c is
+  B : constant Integer := Year (Clock);
+  V : Data;
+
+begin
+  V := (R => (A => 0, B => B, C => 0, D => "000000000000"));
+  if V.R.B /= B then
+    raise Program_Error;
+  end if;
+end;
diff --git a/gcc/testsuite/gnat.dg/opt90c_pkg.ads b/gcc/testsuite/gnat.dg/opt90c_pkg.ads
new file mode 100644 (file)
index 0000000..e772340
--- /dev/null
@@ -0,0 +1,16 @@
+package Opt90c_Pkg is
+
+  type Rec is record
+    D : String (1 .. 12);
+    B : Integer;
+    A : Short_Short_Integer;
+    C : Short_Integer;
+  end record;
+  pragma Pack (Rec);
+  for Rec'Alignment use 1;
+
+  type Data is tagged record
+    R : Rec;
+  end record;
+
+end Opt90c_Pkg;
diff --git a/gcc/testsuite/gnat.dg/opt90d.adb b/gcc/testsuite/gnat.dg/opt90d.adb
new file mode 100644 (file)
index 0000000..32ecb68
--- /dev/null
@@ -0,0 +1,16 @@
+-- { dg-do run }
+-- { dg-options "-O2" }
+
+with Ada.Calendar; use Ada.Calendar;
+with Opt90d_Pkg; use Opt90d_Pkg;
+
+procedure Opt90d is
+  B : constant Integer := Year (Clock);
+  V : Data;
+
+begin
+  V := (R => (A => 0, B => B, C => 0, D => "000000000000"));
+  if V.R.B /= B then
+    raise Program_Error;
+  end if;
+end;
diff --git a/gcc/testsuite/gnat.dg/opt90d_pkg.ads b/gcc/testsuite/gnat.dg/opt90d_pkg.ads
new file mode 100644 (file)
index 0000000..a68b224
--- /dev/null
@@ -0,0 +1,16 @@
+package Opt90d_Pkg is
+
+  type Rec is record
+    D : String (1 .. 12);
+    C : Short_Integer;
+    A : Short_Short_Integer;
+    B : Integer;
+  end record;
+  pragma Pack (Rec);
+  for Rec'Alignment use 1;
+
+  type Data is tagged record
+    R : Rec;
+  end record;
+
+end Opt90d_Pkg;
diff --git a/gcc/testsuite/gnat.dg/opt90e.adb b/gcc/testsuite/gnat.dg/opt90e.adb
new file mode 100644 (file)
index 0000000..6d62774
--- /dev/null
@@ -0,0 +1,16 @@
+-- { dg-do run }
+-- { dg-options "-O2" }
+
+with Ada.Calendar; use Ada.Calendar;
+with Opt90e_Pkg; use Opt90e_Pkg;
+
+procedure Opt90e is
+  B : constant Integer := Year (Clock);
+  V : Data;
+
+begin
+  V := (R => (A => 0, B => B, C => 0, D => "000000000000"));
+  if V.R.B /= B then
+    raise Program_Error;
+  end if;
+end;
diff --git a/gcc/testsuite/gnat.dg/opt90e_pkg.ads b/gcc/testsuite/gnat.dg/opt90e_pkg.ads
new file mode 100644 (file)
index 0000000..fba16d7
--- /dev/null
@@ -0,0 +1,16 @@
+package Opt90e_Pkg is
+
+  type Rec is record
+    D : String (1 .. 12);
+    A : Short_Short_Integer;
+    B : Integer;
+    C : Short_Integer;
+  end record;
+  pragma Pack (Rec);
+  for Rec'Alignment use 1;
+
+  type Data is tagged record
+    R : Rec;
+  end record;
+
+end Opt90e_Pkg;