* [PATCH] DSE: Add LEN_MASK_STORE analysis into DSE
@ 2023-06-23 14:48 juzhe.zhong
2023-06-23 14:57 ` Jeff Law
2023-06-26 6:28 ` Richard Biener
0 siblings, 2 replies; 5+ messages in thread
From: juzhe.zhong @ 2023-06-23 14:48 UTC (permalink / raw)
To: gcc-patches; +Cc: rguenther, richard.sandiford, Ju-Zhe Zhong
From: Ju-Zhe Zhong <juzhe.zhong@rivai.ai>
gcc/ChangeLog:
* tree-ssa-dse.cc (initialize_ao_ref_for_dse): Add LEN_MASK_STORE.
(dse_optimize_stmt): Ditto.
---
gcc/tree-ssa-dse.cc | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/gcc/tree-ssa-dse.cc b/gcc/tree-ssa-dse.cc
index 3c7a2e9992d..01b0951f1a9 100644
--- a/gcc/tree-ssa-dse.cc
+++ b/gcc/tree-ssa-dse.cc
@@ -174,6 +174,23 @@ initialize_ao_ref_for_dse (gimple *stmt, ao_ref *write, bool may_def_ok = false)
return true;
}
break;
+ case IFN_LEN_MASK_STORE: {
+ /* We cannot initialize a must-def ao_ref (in all cases) but we
+ can provide a may-def variant. */
+ if (may_def_ok)
+ {
+ tree len_size
+ = int_const_binop (MINUS_EXPR, gimple_call_arg (stmt, 2),
+ gimple_call_arg (stmt, 5));
+ tree mask_size
+ = TYPE_SIZE_UNIT (TREE_TYPE (gimple_call_arg (stmt, 4)));
+ tree size = int_const_binop (MAX_EXPR, len_size, mask_size);
+ ao_ref_init_from_ptr_and_size (write, gimple_call_arg (stmt, 0),
+ size);
+ return true;
+ }
+ break;
+ }
default:;
}
}
@@ -1502,6 +1519,7 @@ dse_optimize_stmt (function *fun, gimple_stmt_iterator *gsi, sbitmap live_bytes)
{
case IFN_LEN_STORE:
case IFN_MASK_STORE:
+ case IFN_LEN_MASK_STORE:
{
enum dse_store_status store_status;
store_status = dse_classify_store (&ref, stmt, false, live_bytes);
--
2.36.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] DSE: Add LEN_MASK_STORE analysis into DSE
2023-06-23 14:48 [PATCH] DSE: Add LEN_MASK_STORE analysis into DSE juzhe.zhong
@ 2023-06-23 14:57 ` Jeff Law
2023-06-23 23:19 ` 钟居哲
2023-06-26 6:28 ` Richard Biener
1 sibling, 1 reply; 5+ messages in thread
From: Jeff Law @ 2023-06-23 14:57 UTC (permalink / raw)
To: juzhe.zhong, gcc-patches; +Cc: rguenther, richard.sandiford
On 6/23/23 08:48, juzhe.zhong@rivai.ai wrote:
> From: Ju-Zhe Zhong <juzhe.zhong@rivai.ai>
>
> gcc/ChangeLog:
>
> * tree-ssa-dse.cc (initialize_ao_ref_for_dse): Add LEN_MASK_STORE.
> (dse_optimize_stmt): Ditto.
>
> ---
> gcc/tree-ssa-dse.cc | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/gcc/tree-ssa-dse.cc b/gcc/tree-ssa-dse.cc
> index 3c7a2e9992d..01b0951f1a9 100644
> --- a/gcc/tree-ssa-dse.cc
> +++ b/gcc/tree-ssa-dse.cc
> @@ -174,6 +174,23 @@ initialize_ao_ref_for_dse (gimple *stmt, ao_ref *write, bool may_def_ok = false)
> return true;
> }
> break;
> + case IFN_LEN_MASK_STORE: {
> + /* We cannot initialize a must-def ao_ref (in all cases) but we
> + can provide a may-def variant. */
> + if (may_def_ok)
> + {
> + tree len_size
> + = int_const_binop (MINUS_EXPR, gimple_call_arg (stmt, 2),
> + gimple_call_arg (stmt, 5));
> + tree mask_size
> + = TYPE_SIZE_UNIT (TREE_TYPE (gimple_call_arg (stmt, 4)));
> + tree size = int_const_binop (MAX_EXPR, len_size, mask_size);
> + ao_ref_init_from_ptr_and_size (write, gimple_call_arg (stmt, 0),
> + size);
So isn't len_size here the size in elements? If so, don't you need to
multiply len_size by the element size?
Jeff
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Re: [PATCH] DSE: Add LEN_MASK_STORE analysis into DSE
2023-06-23 14:57 ` Jeff Law
@ 2023-06-23 23:19 ` 钟居哲
0 siblings, 0 replies; 5+ messages in thread
From: 钟居哲 @ 2023-06-23 23:19 UTC (permalink / raw)
To: Jeff Law, gcc-patches; +Cc: rguenther, richard.sandiford
[-- Attachment #1: Type: text/plain, Size: 1552 bytes --]
Address comment.
juzhe.zhong@rivai.ai
From: Jeff Law
Date: 2023-06-23 22:57
To: juzhe.zhong; gcc-patches
CC: rguenther; richard.sandiford
Subject: Re: [PATCH] DSE: Add LEN_MASK_STORE analysis into DSE
On 6/23/23 08:48, juzhe.zhong@rivai.ai wrote:
> From: Ju-Zhe Zhong <juzhe.zhong@rivai.ai>
>
> gcc/ChangeLog:
>
> * tree-ssa-dse.cc (initialize_ao_ref_for_dse): Add LEN_MASK_STORE.
> (dse_optimize_stmt): Ditto.
>
> ---
> gcc/tree-ssa-dse.cc | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/gcc/tree-ssa-dse.cc b/gcc/tree-ssa-dse.cc
> index 3c7a2e9992d..01b0951f1a9 100644
> --- a/gcc/tree-ssa-dse.cc
> +++ b/gcc/tree-ssa-dse.cc
> @@ -174,6 +174,23 @@ initialize_ao_ref_for_dse (gimple *stmt, ao_ref *write, bool may_def_ok = false)
> return true;
> }
> break;
> + case IFN_LEN_MASK_STORE: {
> + /* We cannot initialize a must-def ao_ref (in all cases) but we
> + can provide a may-def variant. */
> + if (may_def_ok)
> + {
> + tree len_size
> + = int_const_binop (MINUS_EXPR, gimple_call_arg (stmt, 2),
> + gimple_call_arg (stmt, 5));
> + tree mask_size
> + = TYPE_SIZE_UNIT (TREE_TYPE (gimple_call_arg (stmt, 4)));
> + tree size = int_const_binop (MAX_EXPR, len_size, mask_size);
> + ao_ref_init_from_ptr_and_size (write, gimple_call_arg (stmt, 0),
> + size);
So isn't len_size here the size in elements? If so, don't you need to
multiply len_size by the element size?
Jeff
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] DSE: Add LEN_MASK_STORE analysis into DSE
2023-06-23 14:48 [PATCH] DSE: Add LEN_MASK_STORE analysis into DSE juzhe.zhong
2023-06-23 14:57 ` Jeff Law
@ 2023-06-26 6:28 ` Richard Biener
2023-06-26 6:39 ` juzhe.zhong
1 sibling, 1 reply; 5+ messages in thread
From: Richard Biener @ 2023-06-26 6:28 UTC (permalink / raw)
To: Ju-Zhe Zhong; +Cc: gcc-patches, richard.sandiford
On Fri, 23 Jun 2023, juzhe.zhong@rivai.ai wrote:
> From: Ju-Zhe Zhong <juzhe.zhong@rivai.ai>
>
> gcc/ChangeLog:
>
> * tree-ssa-dse.cc (initialize_ao_ref_for_dse): Add LEN_MASK_STORE.
> (dse_optimize_stmt): Ditto.
>
> ---
> gcc/tree-ssa-dse.cc | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/gcc/tree-ssa-dse.cc b/gcc/tree-ssa-dse.cc
> index 3c7a2e9992d..01b0951f1a9 100644
> --- a/gcc/tree-ssa-dse.cc
> +++ b/gcc/tree-ssa-dse.cc
> @@ -174,6 +174,23 @@ initialize_ao_ref_for_dse (gimple *stmt, ao_ref *write, bool may_def_ok = false)
> return true;
> }
> break;
> + case IFN_LEN_MASK_STORE: {
> + /* We cannot initialize a must-def ao_ref (in all cases) but we
> + can provide a may-def variant. */
> + if (may_def_ok)
> + {
> + tree len_size
> + = int_const_binop (MINUS_EXPR, gimple_call_arg (stmt, 2),
> + gimple_call_arg (stmt, 5));
please use the accessors for the mask/len operand number where available.
Also I think this shows the existing IFN_LEN_STORE support is bogus
since the LEN argument isn't guaranteed to be constant. Can you
fix this as well please? If the length isn't constant the code should
be the same as the IFN_MASK_STORE variant.
> + tree mask_size
> + = TYPE_SIZE_UNIT (TREE_TYPE (gimple_call_arg (stmt, 4)));
> + tree size = int_const_binop (MAX_EXPR, len_size, mask_size);
> + ao_ref_init_from_ptr_and_size (write, gimple_call_arg (stmt, 0),
> + size);
> + return true;
> + }
> + break;
> + }
> default:;
> }
> }
> @@ -1502,6 +1519,7 @@ dse_optimize_stmt (function *fun, gimple_stmt_iterator *gsi, sbitmap live_bytes)
> {
> case IFN_LEN_STORE:
> case IFN_MASK_STORE:
> + case IFN_LEN_MASK_STORE:
> {
> enum dse_store_status store_status;
> store_status = dse_classify_store (&ref, stmt, false, live_bytes);
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Re: [PATCH] DSE: Add LEN_MASK_STORE analysis into DSE
2023-06-26 6:28 ` Richard Biener
@ 2023-06-26 6:39 ` juzhe.zhong
0 siblings, 0 replies; 5+ messages in thread
From: juzhe.zhong @ 2023-06-26 6:39 UTC (permalink / raw)
To: rguenther; +Cc: gcc-patches, richard.sandiford
[-- Attachment #1: Type: text/plain, Size: 2881 bytes --]
Hi, Richi.
>> please use the accessors for the mask/len operand number where available.
>>Also I think this shows the existing IFN_LEN_STORE support is bogus
>>since the LEN argument isn't guaranteed to be constant. Can you
>>fix this as well please? If the length isn't constant the code should
>>be the same as the IFN_MASK_STORE variant.
I understand your idea. Forget about V2 patch (which I sent). I will send V3 patch to fix
both LEN_STORE and LEN_MASK_STORE.
Thank you so much for the review!
juzhe.zhong@rivai.ai
From: Richard Biener
Date: 2023-06-26 14:28
To: Ju-Zhe Zhong
CC: gcc-patches; richard.sandiford
Subject: Re: [PATCH] DSE: Add LEN_MASK_STORE analysis into DSE
On Fri, 23 Jun 2023, juzhe.zhong@rivai.ai wrote:
> From: Ju-Zhe Zhong <juzhe.zhong@rivai.ai>
>
> gcc/ChangeLog:
>
> * tree-ssa-dse.cc (initialize_ao_ref_for_dse): Add LEN_MASK_STORE.
> (dse_optimize_stmt): Ditto.
>
> ---
> gcc/tree-ssa-dse.cc | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/gcc/tree-ssa-dse.cc b/gcc/tree-ssa-dse.cc
> index 3c7a2e9992d..01b0951f1a9 100644
> --- a/gcc/tree-ssa-dse.cc
> +++ b/gcc/tree-ssa-dse.cc
> @@ -174,6 +174,23 @@ initialize_ao_ref_for_dse (gimple *stmt, ao_ref *write, bool may_def_ok = false)
> return true;
> }
> break;
> + case IFN_LEN_MASK_STORE: {
> + /* We cannot initialize a must-def ao_ref (in all cases) but we
> + can provide a may-def variant. */
> + if (may_def_ok)
> + {
> + tree len_size
> + = int_const_binop (MINUS_EXPR, gimple_call_arg (stmt, 2),
> + gimple_call_arg (stmt, 5));
please use the accessors for the mask/len operand number where available.
Also I think this shows the existing IFN_LEN_STORE support is bogus
since the LEN argument isn't guaranteed to be constant. Can you
fix this as well please? If the length isn't constant the code should
be the same as the IFN_MASK_STORE variant.
> + tree mask_size
> + = TYPE_SIZE_UNIT (TREE_TYPE (gimple_call_arg (stmt, 4)));
> + tree size = int_const_binop (MAX_EXPR, len_size, mask_size);
> + ao_ref_init_from_ptr_and_size (write, gimple_call_arg (stmt, 0),
> + size);
> + return true;
> + }
> + break;
> + }
> default:;
> }
> }
> @@ -1502,6 +1519,7 @@ dse_optimize_stmt (function *fun, gimple_stmt_iterator *gsi, sbitmap live_bytes)
> {
> case IFN_LEN_STORE:
> case IFN_MASK_STORE:
> + case IFN_LEN_MASK_STORE:
> {
> enum dse_store_status store_status;
> store_status = dse_classify_store (&ref, stmt, false, live_bytes);
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-26 6:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-23 14:48 [PATCH] DSE: Add LEN_MASK_STORE analysis into DSE juzhe.zhong
2023-06-23 14:57 ` Jeff Law
2023-06-23 23:19 ` 钟居哲
2023-06-26 6:28 ` Richard Biener
2023-06-26 6:39 ` juzhe.zhong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).