Hi,

there is a loophole in new string store merging support I 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.

Tested on x86-64/Linux, OK for the mainline?


2020-11-19  Eric Botcazou  <ebotca...@adacore.com>

        * 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.


2020-11-19  Eric Botcazou  <ebotca...@adacore.com>

        * 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.

-- 
Eric Botcazou
diff --git a/gcc/gimple-ssa-store-merging.c b/gcc/gimple-ssa-store-merging.c
index 6089faf7ac8..17a4250d77f 100644
--- a/gcc/gimple-ssa-store-merging.c
+++ b/gcc/gimple-ssa-store-merging.c
@@ -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
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;
-- { 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;
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;
-- { 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;
-- { 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;
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;
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;
-- { 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;
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;
-- { 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;

Reply via email to