[PATCH] Come up with debug counter for store-merging.

[PATCH] Come up with debug counter for store-merging.

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 501 bytes --]

Hi.

After I spent quite some time with PR91758, I would like
to see a debug counter in store merging for the next time.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2019-09-18  Martin Liska  <mliska@suse.cz>

	* dbgcnt.def (store_merging): New counter.
	* gimple-ssa-store-merging.c (imm_store_chain_info::output_merged_stores):
	Use it in store merging.
---
 gcc/dbgcnt.def                 | 1 +
 gcc/gimple-ssa-store-merging.c | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)



[-- Attachment #2: 0001-Come-up-with-debug-counter-for-store-merging.patch --]
[-- Type: text/x-patch, Size: 1050 bytes --]

diff --git a/gcc/dbgcnt.def b/gcc/dbgcnt.def
index 230072f7bb5..ef981aa6967 100644
--- a/gcc/dbgcnt.def
+++ b/gcc/dbgcnt.def
@@ -196,3 +196,4 @@ DEBUG_COUNTER (vect_loop)
 DEBUG_COUNTER (vect_slp)
 DEBUG_COUNTER (dom_unreachable_edges)
 DEBUG_COUNTER (match)
+DEBUG_COUNTER (store_merging)
diff --git a/gcc/gimple-ssa-store-merging.c b/gcc/gimple-ssa-store-merging.c
index 0bf64b314d6..5abaa7d18d8 100644
--- a/gcc/gimple-ssa-store-merging.c
+++ b/gcc/gimple-ssa-store-merging.c
@@ -166,6 +166,7 @@
 #include "rtl.h"
 #include "expr.h"	/* For get_bit_range.  */
 #include "optabs-tree.h"
+#include "dbgcnt.h"
 #include "selftest.h"
 
 /* The maximum size (in bits) of the stores this pass should generate.  */
@@ -4195,7 +4196,8 @@ imm_store_chain_info::output_merged_stores ()
   bool ret = false;
   FOR_EACH_VEC_ELT (m_merged_store_groups, i, merged_store)
     {
-      if (output_merged_store (merged_store))
+      if (dbg_cnt (store_merging)
+	  && output_merged_store (merged_store))
 	{
 	  unsigned int j;
 	  store_immediate_info *store;

Re: [PATCH] Come up with debug counter for store-merging.

[Richard Biener]

On Wed, Sep 18, 2019 at 9:22 AM Martin Liška <mliska@suse.cz> wrote:
>
> Hi.
>
> After I spent quite some time with PR91758, I would like
> to see a debug counter in store merging for the next time.
>
> Ready to be installed?
OK.

Richard.

> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> 2019-09-18  Martin Liska  <mliska@suse.cz>
>
>         * dbgcnt.def (store_merging): New counter.
>         * gimple-ssa-store-merging.c (imm_store_chain_info::output_merged_stores):
>         Use it in store merging.
> ---
>  gcc/dbgcnt.def                 | 1 +
>  gcc/gimple-ssa-store-merging.c | 4 +++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
>

Re: [PATCH] Come up with debug counter for store-merging.

[Bernhard Reutner-Fischer]

On Wed, 18 Sep 2019 11:01:59 +0200
Richard Biener <richard.guenther@gmail.com> wrote:

> On Wed, Sep 18, 2019 at 9:22 AM Martin Liška <mliska@suse.cz> wrote:
> >
> > Hi.
> >
> > After I spent quite some time with PR91758, I would like
> > to see a debug counter in store merging for the next time.
> >
> > Ready to be installed?  
> OK.

> @@ -4195,7 +4196,8 @@ imm_store_chain_info::output_merged_stores ()
>    bool ret = false;
>    FOR_EACH_VEC_ELT (m_merged_store_groups, i, merged_store)
>      {
> -      if (output_merged_store (merged_store))
> +      if (dbg_cnt (store_merging)
> +	  && output_merged_store (merged_store))

Doesn't this over-count actual merged stores? You really count the number of attempted store merges, not the actual successful stores merged. As i would expect to count the latter, i would have expected ..

>  	{
>  	  unsigned int j;
>  	  store_immediate_info *store;

dbg_cnt (store_merging) here, i.e. after the guard that determines if
the merge is benefical, no?

thanks,

Re: [PATCH] Come up with debug counter for store-merging.

[Martin Liška]

On 9/19/19 4:57 PM, Bernhard Reutner-Fischer wrote:
> On Wed, 18 Sep 2019 11:01:59 +0200
> Richard Biener <richard.guenther@gmail.com> wrote:
> 
>> On Wed, Sep 18, 2019 at 9:22 AM Martin Liška <mliska@suse.cz> wrote:
>>>
>>> Hi.
>>>
>>> After I spent quite some time with PR91758, I would like
>>> to see a debug counter in store merging for the next time.
>>>
>>> Ready to be installed?  
>> OK.
> 
>> @@ -4195,7 +4196,8 @@ imm_store_chain_info::output_merged_stores ()
>>    bool ret = false;
>>    FOR_EACH_VEC_ELT (m_merged_store_groups, i, merged_store)
>>      {
>> -      if (output_merged_store (merged_store))
>> +      if (dbg_cnt (store_merging)
>> +	  && output_merged_store (merged_store))
> 
> Doesn't this over-count actual merged stores? You really count the number of attempted store merges, not the actual successful stores merged. As i would expect to count the latter, i would have expected ..
> 
>>  	{
>>  	  unsigned int j;
>>  	  store_immediate_info *store;
> 
> dbg_cnt (store_merging) here, i.e. after the guard that determines if
> the merge is benefical, no?

Hi.

You are right. It's more about attempts rather than number of transformations.
However, the function imm_store_chain_info::output_merged_store emits GIMPLE statements
and I was unable to find a single place where to put the debug counter.

Martin

> 
> thanks,
>