CODE STRUCTURE: Refine codes in Vectorizer
Checks
Commit Message
From: Ju-Zhe Zhong <juzhe.zhong@rivai.ai>
Hi, Richard and Richi.
I plan to refine the codes that I recently support for RVV auto-vectorization.
This patch is inspired last review comments from Richard:
https://patchwork.sourceware.org/project/gcc/patch/20230712042124.111818-1-juzhe.zhong@rivai.ai/
Richard said he prefer the the code structure as follows:
Please instead switch the if condition so that the structure is:
if (...)
vect_record_loop_mask (...)
else if (...)
vect_record_loop_len (...)
else
can't use partial vectors
This is his last comments.
So, I come back to refine this piece of codes.
Does it look reasonable ?
This next refine patch is change all names of "LEN_MASK" into "MASK_LEN" but should come after this
patch.
gcc/ChangeLog:
* tree-vect-stmts.cc (check_load_store_for_partial_vectors): Refine code structure.
---
gcc/tree-vect-stmts.cc | 38 +++++++++++++++++---------------------
1 file changed, 17 insertions(+), 21 deletions(-)
Comments
Just finish Bootstrap and regression on X86.
Ok for trunk ?
juzhe.zhong@rivai.ai
From: juzhe.zhong
Date: 2023-07-20 16:06
To: gcc-patches
CC: richard.sandiford; rguenther; Ju-Zhe Zhong
Subject: [PATCH] CODE STRUCTURE: Refine codes in Vectorizer
From: Ju-Zhe Zhong <juzhe.zhong@rivai.ai>
Hi, Richard and Richi.
I plan to refine the codes that I recently support for RVV auto-vectorization.
This patch is inspired last review comments from Richard:
https://patchwork.sourceware.org/project/gcc/patch/20230712042124.111818-1-juzhe.zhong@rivai.ai/
Richard said he prefer the the code structure as follows:
Please instead switch the if condition so that the structure is:
if (...)
vect_record_loop_mask (...)
else if (...)
vect_record_loop_len (...)
else
can't use partial vectors
This is his last comments.
So, I come back to refine this piece of codes.
Does it look reasonable ?
This next refine patch is change all names of "LEN_MASK" into "MASK_LEN" but should come after this
patch.
gcc/ChangeLog:
* tree-vect-stmts.cc (check_load_store_for_partial_vectors): Refine code structure.
---
gcc/tree-vect-stmts.cc | 38 +++++++++++++++++---------------------
1 file changed, 17 insertions(+), 21 deletions(-)
diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
index cb86d544313..b86e159ae4c 100644
--- a/gcc/tree-vect-stmts.cc
+++ b/gcc/tree-vect-stmts.cc
@@ -1605,6 +1605,7 @@ check_load_store_for_partial_vectors (loop_vec_info loop_vinfo, tree vectype,
nvectors = vect_get_num_copies (loop_vinfo, vectype);
vec_loop_masks *masks = &LOOP_VINFO_MASKS (loop_vinfo);
+ vec_loop_lens *lens = &LOOP_VINFO_LENS (loop_vinfo);
machine_mode vecmode = TYPE_MODE (vectype);
bool is_load = (vls_type == VLS_LOAD);
if (memory_access_type == VMAT_LOAD_STORE_LANES)
@@ -1631,33 +1632,29 @@ check_load_store_for_partial_vectors (loop_vec_info loop_vinfo, tree vectype,
internal_fn ifn = (is_load
? IFN_MASK_GATHER_LOAD
: IFN_MASK_SCATTER_STORE);
- if (!internal_gather_scatter_fn_supported_p (ifn, vectype,
- gs_info->memory_type,
- gs_info->offset_vectype,
- gs_info->scale))
- {
- ifn = (is_load
- ? IFN_LEN_MASK_GATHER_LOAD
- : IFN_LEN_MASK_SCATTER_STORE);
- if (internal_gather_scatter_fn_supported_p (ifn, vectype,
- gs_info->memory_type,
- gs_info->offset_vectype,
- gs_info->scale))
- {
- vec_loop_lens *lens = &LOOP_VINFO_LENS (loop_vinfo);
- vect_record_loop_len (loop_vinfo, lens, nvectors, vectype, 1);
- return;
- }
+ internal_fn len_ifn = (is_load
+ ? IFN_LEN_MASK_GATHER_LOAD
+ : IFN_LEN_MASK_SCATTER_STORE);
+ if (internal_gather_scatter_fn_supported_p (ifn, vectype,
+ gs_info->memory_type,
+ gs_info->offset_vectype,
+ gs_info->scale))
+ vect_record_loop_mask (loop_vinfo, masks, nvectors, vectype,
+ scalar_mask);
+ else if (internal_gather_scatter_fn_supported_p (len_ifn, vectype,
+ gs_info->memory_type,
+ gs_info->offset_vectype,
+ gs_info->scale))
+ vect_record_loop_len (loop_vinfo, lens, nvectors, vectype, 1);
+ else
+ {
if (dump_enabled_p ())
dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
"can't operate on partial vectors because"
" the target doesn't have an appropriate"
" gather load or scatter store instruction.\n");
LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P (loop_vinfo) = false;
- return;
}
- vect_record_loop_mask (loop_vinfo, masks, nvectors, vectype,
- scalar_mask);
return;
}
@@ -1703,7 +1700,6 @@ check_load_store_for_partial_vectors (loop_vec_info loop_vinfo, tree vectype,
if (get_len_load_store_mode (vecmode, is_load).exists (&vmode))
{
nvectors = group_memory_nvectors (group_size * vf, nunits);
- vec_loop_lens *lens = &LOOP_VINFO_LENS (loop_vinfo);
unsigned factor = (vecmode == vmode) ? 1 : GET_MODE_UNIT_SIZE (vecmode);
vect_record_loop_len (loop_vinfo, lens, nvectors, vectype, factor);
using_partial_vectors_p = true;
--
2.36.3
On Thu, 20 Jul 2023, juzhe.zhong@rivai.ai wrote:
> Just finish Bootstrap and regression on X86.
>
> Ok for trunk ?
OK. Not an issue currently but I think LEN_MASK should be
checked before MASK.
Richard.
>
> juzhe.zhong@rivai.ai
>
> From: juzhe.zhong
> Date: 2023-07-20 16:06
> To: gcc-patches
> CC: richard.sandiford; rguenther; Ju-Zhe Zhong
> Subject: [PATCH] CODE STRUCTURE: Refine codes in Vectorizer
> From: Ju-Zhe Zhong <juzhe.zhong@rivai.ai>
>
> Hi, Richard and Richi.
>
> I plan to refine the codes that I recently support for RVV auto-vectorization.
> This patch is inspired last review comments from Richard:
> https://patchwork.sourceware.org/project/gcc/patch/20230712042124.111818-1-juzhe.zhong@rivai.ai/
>
> Richard said he prefer the the code structure as follows:
>
> Please instead switch the if condition so that the structure is:
>
> if (...)
> vect_record_loop_mask (...)
> else if (...)
> vect_record_loop_len (...)
> else
> can't use partial vectors
>
> This is his last comments.
>
> So, I come back to refine this piece of codes.
>
> Does it look reasonable ?
>
> This next refine patch is change all names of "LEN_MASK" into "MASK_LEN" but should come after this
> patch.
>
> gcc/ChangeLog:
>
> * tree-vect-stmts.cc (check_load_store_for_partial_vectors): Refine code structure.
>
> ---
> gcc/tree-vect-stmts.cc | 38 +++++++++++++++++---------------------
> 1 file changed, 17 insertions(+), 21 deletions(-)
>
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index cb86d544313..b86e159ae4c 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -1605,6 +1605,7 @@ check_load_store_for_partial_vectors (loop_vec_info loop_vinfo, tree vectype,
> nvectors = vect_get_num_copies (loop_vinfo, vectype);
> vec_loop_masks *masks = &LOOP_VINFO_MASKS (loop_vinfo);
> + vec_loop_lens *lens = &LOOP_VINFO_LENS (loop_vinfo);
> machine_mode vecmode = TYPE_MODE (vectype);
> bool is_load = (vls_type == VLS_LOAD);
> if (memory_access_type == VMAT_LOAD_STORE_LANES)
> @@ -1631,33 +1632,29 @@ check_load_store_for_partial_vectors (loop_vec_info loop_vinfo, tree vectype,
> internal_fn ifn = (is_load
> ? IFN_MASK_GATHER_LOAD
> : IFN_MASK_SCATTER_STORE);
> - if (!internal_gather_scatter_fn_supported_p (ifn, vectype,
> - gs_info->memory_type,
> - gs_info->offset_vectype,
> - gs_info->scale))
> - {
> - ifn = (is_load
> - ? IFN_LEN_MASK_GATHER_LOAD
> - : IFN_LEN_MASK_SCATTER_STORE);
> - if (internal_gather_scatter_fn_supported_p (ifn, vectype,
> - gs_info->memory_type,
> - gs_info->offset_vectype,
> - gs_info->scale))
> - {
> - vec_loop_lens *lens = &LOOP_VINFO_LENS (loop_vinfo);
> - vect_record_loop_len (loop_vinfo, lens, nvectors, vectype, 1);
> - return;
> - }
> + internal_fn len_ifn = (is_load
> + ? IFN_LEN_MASK_GATHER_LOAD
> + : IFN_LEN_MASK_SCATTER_STORE);
> + if (internal_gather_scatter_fn_supported_p (ifn, vectype,
> + gs_info->memory_type,
> + gs_info->offset_vectype,
> + gs_info->scale))
> + vect_record_loop_mask (loop_vinfo, masks, nvectors, vectype,
> + scalar_mask);
> + else if (internal_gather_scatter_fn_supported_p (len_ifn, vectype,
> + gs_info->memory_type,
> + gs_info->offset_vectype,
> + gs_info->scale))
> + vect_record_loop_len (loop_vinfo, lens, nvectors, vectype, 1);
> + else
> + {
> if (dump_enabled_p ())
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> "can't operate on partial vectors because"
> " the target doesn't have an appropriate"
> " gather load or scatter store instruction.\n");
> LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P (loop_vinfo) = false;
> - return;
> }
> - vect_record_loop_mask (loop_vinfo, masks, nvectors, vectype,
> - scalar_mask);
> return;
> }
> @@ -1703,7 +1700,6 @@ check_load_store_for_partial_vectors (loop_vec_info loop_vinfo, tree vectype,
> if (get_len_load_store_mode (vecmode, is_load).exists (&vmode))
> {
> nvectors = group_memory_nvectors (group_size * vf, nunits);
> - vec_loop_lens *lens = &LOOP_VINFO_LENS (loop_vinfo);
> unsigned factor = (vecmode == vmode) ? 1 : GET_MODE_UNIT_SIZE (vecmode);
> vect_record_loop_len (loop_vinfo, lens, nvectors, vectype, factor);
> using_partial_vectors_p = true;
>
Committed, thanks Richard.
Pan
-----Original Message-----
From: Gcc-patches <gcc-patches-bounces+pan2.li=intel.com@gcc.gnu.org> On Behalf Of Richard Biener via Gcc-patches
Sent: Thursday, July 20, 2023 8:54 PM
To: juzhe.zhong@rivai.ai
Cc: gcc-patches <gcc-patches@gcc.gnu.org>; richard.sandiford <richard.sandiford@arm.com>
Subject: Re: [PATCH] CODE STRUCTURE: Refine codes in Vectorizer
On Thu, 20 Jul 2023, juzhe.zhong@rivai.ai wrote:
> Just finish Bootstrap and regression on X86.
>
> Ok for trunk ?
OK. Not an issue currently but I think LEN_MASK should be
checked before MASK.
Richard.
>
> juzhe.zhong@rivai.ai
>
> From: juzhe.zhong
> Date: 2023-07-20 16:06
> To: gcc-patches
> CC: richard.sandiford; rguenther; Ju-Zhe Zhong
> Subject: [PATCH] CODE STRUCTURE: Refine codes in Vectorizer
> From: Ju-Zhe Zhong <juzhe.zhong@rivai.ai>
>
> Hi, Richard and Richi.
>
> I plan to refine the codes that I recently support for RVV auto-vectorization.
> This patch is inspired last review comments from Richard:
> https://patchwork.sourceware.org/project/gcc/patch/20230712042124.111818-1-juzhe.zhong@rivai.ai/
>
> Richard said he prefer the the code structure as follows:
>
> Please instead switch the if condition so that the structure is:
>
> if (...)
> vect_record_loop_mask (...)
> else if (...)
> vect_record_loop_len (...)
> else
> can't use partial vectors
>
> This is his last comments.
>
> So, I come back to refine this piece of codes.
>
> Does it look reasonable ?
>
> This next refine patch is change all names of "LEN_MASK" into "MASK_LEN" but should come after this
> patch.
>
> gcc/ChangeLog:
>
> * tree-vect-stmts.cc (check_load_store_for_partial_vectors): Refine code structure.
>
> ---
> gcc/tree-vect-stmts.cc | 38 +++++++++++++++++---------------------
> 1 file changed, 17 insertions(+), 21 deletions(-)
>
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index cb86d544313..b86e159ae4c 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -1605,6 +1605,7 @@ check_load_store_for_partial_vectors (loop_vec_info loop_vinfo, tree vectype,
> nvectors = vect_get_num_copies (loop_vinfo, vectype);
> vec_loop_masks *masks = &LOOP_VINFO_MASKS (loop_vinfo);
> + vec_loop_lens *lens = &LOOP_VINFO_LENS (loop_vinfo);
> machine_mode vecmode = TYPE_MODE (vectype);
> bool is_load = (vls_type == VLS_LOAD);
> if (memory_access_type == VMAT_LOAD_STORE_LANES)
> @@ -1631,33 +1632,29 @@ check_load_store_for_partial_vectors (loop_vec_info loop_vinfo, tree vectype,
> internal_fn ifn = (is_load
> ? IFN_MASK_GATHER_LOAD
> : IFN_MASK_SCATTER_STORE);
> - if (!internal_gather_scatter_fn_supported_p (ifn, vectype,
> - gs_info->memory_type,
> - gs_info->offset_vectype,
> - gs_info->scale))
> - {
> - ifn = (is_load
> - ? IFN_LEN_MASK_GATHER_LOAD
> - : IFN_LEN_MASK_SCATTER_STORE);
> - if (internal_gather_scatter_fn_supported_p (ifn, vectype,
> - gs_info->memory_type,
> - gs_info->offset_vectype,
> - gs_info->scale))
> - {
> - vec_loop_lens *lens = &LOOP_VINFO_LENS (loop_vinfo);
> - vect_record_loop_len (loop_vinfo, lens, nvectors, vectype, 1);
> - return;
> - }
> + internal_fn len_ifn = (is_load
> + ? IFN_LEN_MASK_GATHER_LOAD
> + : IFN_LEN_MASK_SCATTER_STORE);
> + if (internal_gather_scatter_fn_supported_p (ifn, vectype,
> + gs_info->memory_type,
> + gs_info->offset_vectype,
> + gs_info->scale))
> + vect_record_loop_mask (loop_vinfo, masks, nvectors, vectype,
> + scalar_mask);
> + else if (internal_gather_scatter_fn_supported_p (len_ifn, vectype,
> + gs_info->memory_type,
> + gs_info->offset_vectype,
> + gs_info->scale))
> + vect_record_loop_len (loop_vinfo, lens, nvectors, vectype, 1);
> + else
> + {
> if (dump_enabled_p ())
> dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
> "can't operate on partial vectors because"
> " the target doesn't have an appropriate"
> " gather load or scatter store instruction.\n");
> LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P (loop_vinfo) = false;
> - return;
> }
> - vect_record_loop_mask (loop_vinfo, masks, nvectors, vectype,
> - scalar_mask);
> return;
> }
> @@ -1703,7 +1700,6 @@ check_load_store_for_partial_vectors (loop_vec_info loop_vinfo, tree vectype,
> if (get_len_load_store_mode (vecmode, is_load).exists (&vmode))
> {
> nvectors = group_memory_nvectors (group_size * vf, nunits);
> - vec_loop_lens *lens = &LOOP_VINFO_LENS (loop_vinfo);
> unsigned factor = (vecmode == vmode) ? 1 : GET_MODE_UNIT_SIZE (vecmode);
> vect_record_loop_len (loop_vinfo, lens, nvectors, vectype, factor);
> using_partial_vectors_p = true;
>
@@ -1605,6 +1605,7 @@ check_load_store_for_partial_vectors (loop_vec_info loop_vinfo, tree vectype,
nvectors = vect_get_num_copies (loop_vinfo, vectype);
vec_loop_masks *masks = &LOOP_VINFO_MASKS (loop_vinfo);
+ vec_loop_lens *lens = &LOOP_VINFO_LENS (loop_vinfo);
machine_mode vecmode = TYPE_MODE (vectype);
bool is_load = (vls_type == VLS_LOAD);
if (memory_access_type == VMAT_LOAD_STORE_LANES)
@@ -1631,33 +1632,29 @@ check_load_store_for_partial_vectors (loop_vec_info loop_vinfo, tree vectype,
internal_fn ifn = (is_load
? IFN_MASK_GATHER_LOAD
: IFN_MASK_SCATTER_STORE);
- if (!internal_gather_scatter_fn_supported_p (ifn, vectype,
- gs_info->memory_type,
- gs_info->offset_vectype,
- gs_info->scale))
- {
- ifn = (is_load
- ? IFN_LEN_MASK_GATHER_LOAD
- : IFN_LEN_MASK_SCATTER_STORE);
- if (internal_gather_scatter_fn_supported_p (ifn, vectype,
- gs_info->memory_type,
- gs_info->offset_vectype,
- gs_info->scale))
- {
- vec_loop_lens *lens = &LOOP_VINFO_LENS (loop_vinfo);
- vect_record_loop_len (loop_vinfo, lens, nvectors, vectype, 1);
- return;
- }
+ internal_fn len_ifn = (is_load
+ ? IFN_LEN_MASK_GATHER_LOAD
+ : IFN_LEN_MASK_SCATTER_STORE);
+ if (internal_gather_scatter_fn_supported_p (ifn, vectype,
+ gs_info->memory_type,
+ gs_info->offset_vectype,
+ gs_info->scale))
+ vect_record_loop_mask (loop_vinfo, masks, nvectors, vectype,
+ scalar_mask);
+ else if (internal_gather_scatter_fn_supported_p (len_ifn, vectype,
+ gs_info->memory_type,
+ gs_info->offset_vectype,
+ gs_info->scale))
+ vect_record_loop_len (loop_vinfo, lens, nvectors, vectype, 1);
+ else
+ {
if (dump_enabled_p ())
dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
"can't operate on partial vectors because"
" the target doesn't have an appropriate"
" gather load or scatter store instruction.\n");
LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P (loop_vinfo) = false;
- return;
}
- vect_record_loop_mask (loop_vinfo, masks, nvectors, vectype,
- scalar_mask);
return;
}
@@ -1703,7 +1700,6 @@ check_load_store_for_partial_vectors (loop_vec_info loop_vinfo, tree vectype,
if (get_len_load_store_mode (vecmode, is_load).exists (&vmode))
{
nvectors = group_memory_nvectors (group_size * vf, nunits);
- vec_loop_lens *lens = &LOOP_VINFO_LENS (loop_vinfo);
unsigned factor = (vecmode == vmode) ? 1 : GET_MODE_UNIT_SIZE (vecmode);
vect_record_loop_len (loop_vinfo, lens, nvectors, vectype, factor);
using_partial_vectors_p = true;