diff mbox series

Plug loophole in string store merging

Message ID 2102818.3SrO4E3YVF@fomalhaut
State New
Headers show
Series Plug loophole in string store merging | expand

Commit Message

Eric Botcazou Nov. 19, 2020, 3:52 p.m. UTC 
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  <ebotcazou@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  <ebotcazou@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.

Comments

Jeff Law Nov. 19, 2020, 5:30 p.m. UTC | #1 
On 11/19/20 8:52 AM, Eric Botcazou wrote:
> 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  <ebotcazou@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  <ebotcazou@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.
OK
jeff
diff mbox series

Patch

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