* [PATCH] VECT: Fix ICE of variable stride on strieded load/store with SELECT_VL loop control. @ 2023-07-06 6:16 juzhe.zhong 2023-07-06 6:43 ` Richard Biener 0 siblings, 1 reply; 3+ messages in thread From: juzhe.zhong @ 2023-07-06 6:16 UTC (permalink / raw) To: gcc-patches; +Cc: richard.sandiford, rguenther, Ju-Zhe Zhong From: Ju-Zhe Zhong <juzhe.zhong@rivai.ai> Hi, Richi. Sorry for making mistake on LEN_MASK_GATHER_LOAD/LEN_MASK_SCATTER_STORE with SELECT_VL loop control. Consider this following case: #define TEST_LOOP(DATA_TYPE, BITS) \ void __attribute__ ((noinline, noclone)) \ f_##DATA_TYPE##_##BITS (DATA_TYPE *restrict dest, DATA_TYPE *restrict src, \ INDEX##BITS stride, INDEX##BITS n) \ { \ for (INDEX##BITS i = 0; i < n; ++i) \ dest[i] += src[i * stride]; \ } When "stride" is a constant, current flow works fine. However, when "stride" is a variable. It causes an ICE: # vectp_src.67_85 = PHI <vectp_src.67_86(6), src_21(D)(12)> ... _96 = .SELECT_VL (ivtmp_94, 4); ... ivtmp_78 = ((sizetype) _39 * (sizetype) _96) * 4; vect__11.69_87 = .LEN_MASK_GATHER_LOAD (vectp_src.67_85, _84, 4, { 0, 0, 0, 0 }, { -1, -1, -1, -1 }, _96, 0); ... vectp_src.67_86 = vectp_src.67_85 + ivtmp_78; Becase the IR: ivtmp_78 = ((sizetype) _39 * (sizetype) _96) * 4; Instead, I split the IR into: step_stride = _39 step = step_stride * 4 ivtmp_78 = step * _96 I don't think this patch's code is elegant enough, could you help me refine these codes? Thanks. gcc/ChangeLog: * tree-vect-stmts.cc (vect_get_strided_load_store_ops): Fix ICE. --- gcc/tree-vect-stmts.cc | 38 +++++++++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc index c10a4be60eb..12d1b0f1ac0 100644 --- a/gcc/tree-vect-stmts.cc +++ b/gcc/tree-vect-stmts.cc @@ -3172,12 +3172,40 @@ vect_get_strided_load_store_ops (stmt_vec_info stmt_info, vectp_a.9_26 = vectp_a.9_7 + ivtmp_8; */ tree loop_len = vect_get_loop_len (loop_vinfo, gsi, loop_lens, 1, vectype, 0, 0); - tree tmp - = fold_build2 (MULT_EXPR, sizetype, - fold_convert (sizetype, unshare_expr (DR_STEP (dr))), - loop_len); + tree tmp; + gassign *assign; + + if (TREE_CODE (DR_STEP (dr)) == INTEGER_CST) + tmp = fold_build2 (MULT_EXPR, sizetype, + fold_convert (sizetype, unshare_expr (DR_STEP (dr))), + loop_len); + else + { + /* If DR_STEP = (unsigned int) _37 * 4; + Extract _37 and 4, explicit MULT_EXPR. */ + + /* 1. step_stride = (unsigned int) _37. */ + tree step_stride = make_ssa_name (create_tmp_var (sizetype)); + assign = gimple_build_assign ( + step_stride, TREE_OPERAND (TREE_OPERAND (DR_STEP (dr), 0), 0)); + gsi_insert_before (gsi, assign, GSI_SAME_STMT); + + /* 2. step = step_stride * 4. */ + tree step_align = TREE_OPERAND (TREE_OPERAND (DR_STEP (dr), 0), 1); + tree step = make_ssa_name (create_tmp_var (sizetype)); + assign + = gimple_build_assign (step, fold_build2 (MULT_EXPR, sizetype, + step_stride, step_align)); + gsi_insert_before (gsi, assign, GSI_SAME_STMT); + + /* 3. tmp = step * loop_len. */ + tmp = make_ssa_name (create_tmp_var (sizetype)); + assign = gimple_build_assign (tmp, fold_build2 (MULT_EXPR, sizetype, + step, loop_len)); + gsi_insert_before (gsi, assign, GSI_SAME_STMT); + } tree bump = make_temp_ssa_name (sizetype, NULL, "ivtmp"); - gassign *assign = gimple_build_assign (bump, tmp); + assign = gimple_build_assign (bump, tmp); gsi_insert_before (gsi, assign, GSI_SAME_STMT); *dataref_bump = bump; } -- 2.36.3 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] VECT: Fix ICE of variable stride on strieded load/store with SELECT_VL loop control. 2023-07-06 6:16 [PATCH] VECT: Fix ICE of variable stride on strieded load/store with SELECT_VL loop control juzhe.zhong @ 2023-07-06 6:43 ` Richard Biener 2023-07-06 6:52 ` juzhe.zhong 0 siblings, 1 reply; 3+ messages in thread From: Richard Biener @ 2023-07-06 6:43 UTC (permalink / raw) To: Ju-Zhe Zhong; +Cc: gcc-patches, richard.sandiford On Thu, 6 Jul 2023, juzhe.zhong@rivai.ai wrote: > From: Ju-Zhe Zhong <juzhe.zhong@rivai.ai> > > Hi, Richi. > > Sorry for making mistake on LEN_MASK_GATHER_LOAD/LEN_MASK_SCATTER_STORE > with SELECT_VL loop control. > > Consider this following case: > #define TEST_LOOP(DATA_TYPE, BITS) \ > void __attribute__ ((noinline, noclone)) \ > f_##DATA_TYPE##_##BITS (DATA_TYPE *restrict dest, DATA_TYPE *restrict src, \ > INDEX##BITS stride, INDEX##BITS n) \ > { \ > for (INDEX##BITS i = 0; i < n; ++i) \ > dest[i] += src[i * stride]; \ > } > > When "stride" is a constant, current flow works fine. > However, when "stride" is a variable. It causes an ICE: > # vectp_src.67_85 = PHI <vectp_src.67_86(6), src_21(D)(12)> > ... > _96 = .SELECT_VL (ivtmp_94, 4); > ... > ivtmp_78 = ((sizetype) _39 * (sizetype) _96) * 4; > vect__11.69_87 = .LEN_MASK_GATHER_LOAD (vectp_src.67_85, _84, 4, { 0, 0, 0, 0 }, { -1, -1, -1, -1 }, _96, 0); > ... > vectp_src.67_86 = vectp_src.67_85 + ivtmp_78; > > Becase the IR: ivtmp_78 = ((sizetype) _39 * (sizetype) _96) * 4; > > Instead, I split the IR into: > > step_stride = _39 > step = step_stride * 4 > ivtmp_78 = step * _96 > > I don't think this patch's code is elegant enough, could you help me refine these codes? > > Thanks. > > gcc/ChangeLog: > > * tree-vect-stmts.cc (vect_get_strided_load_store_ops): Fix ICE. > > --- > gcc/tree-vect-stmts.cc | 38 +++++++++++++++++++++++++++++++++----- > 1 file changed, 33 insertions(+), 5 deletions(-) > > diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc > index c10a4be60eb..12d1b0f1ac0 100644 > --- a/gcc/tree-vect-stmts.cc > +++ b/gcc/tree-vect-stmts.cc > @@ -3172,12 +3172,40 @@ vect_get_strided_load_store_ops (stmt_vec_info stmt_info, > vectp_a.9_26 = vectp_a.9_7 + ivtmp_8; */ > tree loop_len > = vect_get_loop_len (loop_vinfo, gsi, loop_lens, 1, vectype, 0, 0); > - tree tmp > - = fold_build2 (MULT_EXPR, sizetype, > - fold_convert (sizetype, unshare_expr (DR_STEP (dr))), > - loop_len); > + tree tmp; > + gassign *assign; > + > + if (TREE_CODE (DR_STEP (dr)) == INTEGER_CST) > + tmp = fold_build2 (MULT_EXPR, sizetype, > + fold_convert (sizetype, unshare_expr (DR_STEP (dr))), > + loop_len); > + else > + { > + /* If DR_STEP = (unsigned int) _37 * 4; > + Extract _37 and 4, explicit MULT_EXPR. */ > + > + /* 1. step_stride = (unsigned int) _37. */ > + tree step_stride = make_ssa_name (create_tmp_var (sizetype)); > + assign = gimple_build_assign ( > + step_stride, TREE_OPERAND (TREE_OPERAND (DR_STEP (dr), 0), 0)); > + gsi_insert_before (gsi, assign, GSI_SAME_STMT); > + > + /* 2. step = step_stride * 4. */ > + tree step_align = TREE_OPERAND (TREE_OPERAND (DR_STEP (dr), 0), 1); > + tree step = make_ssa_name (create_tmp_var (sizetype)); > + assign > + = gimple_build_assign (step, fold_build2 (MULT_EXPR, sizetype, > + step_stride, step_align)); > + gsi_insert_before (gsi, assign, GSI_SAME_STMT); > + > + /* 3. tmp = step * loop_len. */ > + tmp = make_ssa_name (create_tmp_var (sizetype)); > + assign = gimple_build_assign (tmp, fold_build2 (MULT_EXPR, sizetype, > + step, loop_len)); > + gsi_insert_before (gsi, assign, GSI_SAME_STMT); > + } > tree bump = make_temp_ssa_name (sizetype, NULL, "ivtmp"); > - gassign *assign = gimple_build_assign (bump, tmp); instead of tree bump = make_temp_ssa_name (sizetype, NULL, "ivtmp"); gassign *assign = gimple_build_assign (bump, tmp); you can simply do tree bump = force_gimple_operand_gsi (gsi, tmp, true, NULL_TREE, true, GSI_SAME_STMT); That's all that is needed. Richard. > + assign = gimple_build_assign (bump, tmp); > gsi_insert_before (gsi, assign, GSI_SAME_STMT); > *dataref_bump = bump; > } > -- 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] 3+ messages in thread
* Re: Re: [PATCH] VECT: Fix ICE of variable stride on strieded load/store with SELECT_VL loop control. 2023-07-06 6:43 ` Richard Biener @ 2023-07-06 6:52 ` juzhe.zhong 0 siblings, 0 replies; 3+ messages in thread From: juzhe.zhong @ 2023-07-06 6:52 UTC (permalink / raw) To: rguenther; +Cc: gcc-patches, richard.sandiford [-- Attachment #1: Type: text/plain, Size: 4916 bytes --] Thank you so much. I have sent V2: https://gcc.gnu.org/pipermail/gcc-patches/2023-July/623734.html which is working fine for both stride = constant and variable. Could you take a look at it? Thanks. juzhe.zhong@rivai.ai From: Richard Biener Date: 2023-07-06 14:43 To: Ju-Zhe Zhong CC: gcc-patches; richard.sandiford Subject: Re: [PATCH] VECT: Fix ICE of variable stride on strieded load/store with SELECT_VL loop control. On Thu, 6 Jul 2023, juzhe.zhong@rivai.ai wrote: > From: Ju-Zhe Zhong <juzhe.zhong@rivai.ai> > > Hi, Richi. > > Sorry for making mistake on LEN_MASK_GATHER_LOAD/LEN_MASK_SCATTER_STORE > with SELECT_VL loop control. > > Consider this following case: > #define TEST_LOOP(DATA_TYPE, BITS) \ > void __attribute__ ((noinline, noclone)) \ > f_##DATA_TYPE##_##BITS (DATA_TYPE *restrict dest, DATA_TYPE *restrict src, \ > INDEX##BITS stride, INDEX##BITS n) \ > { \ > for (INDEX##BITS i = 0; i < n; ++i) \ > dest[i] += src[i * stride]; \ > } > > When "stride" is a constant, current flow works fine. > However, when "stride" is a variable. It causes an ICE: > # vectp_src.67_85 = PHI <vectp_src.67_86(6), src_21(D)(12)> > ... > _96 = .SELECT_VL (ivtmp_94, 4); > ... > ivtmp_78 = ((sizetype) _39 * (sizetype) _96) * 4; > vect__11.69_87 = .LEN_MASK_GATHER_LOAD (vectp_src.67_85, _84, 4, { 0, 0, 0, 0 }, { -1, -1, -1, -1 }, _96, 0); > ... > vectp_src.67_86 = vectp_src.67_85 + ivtmp_78; > > Becase the IR: ivtmp_78 = ((sizetype) _39 * (sizetype) _96) * 4; > > Instead, I split the IR into: > > step_stride = _39 > step = step_stride * 4 > ivtmp_78 = step * _96 > > I don't think this patch's code is elegant enough, could you help me refine these codes? > > Thanks. > > gcc/ChangeLog: > > * tree-vect-stmts.cc (vect_get_strided_load_store_ops): Fix ICE. > > --- > gcc/tree-vect-stmts.cc | 38 +++++++++++++++++++++++++++++++++----- > 1 file changed, 33 insertions(+), 5 deletions(-) > > diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc > index c10a4be60eb..12d1b0f1ac0 100644 > --- a/gcc/tree-vect-stmts.cc > +++ b/gcc/tree-vect-stmts.cc > @@ -3172,12 +3172,40 @@ vect_get_strided_load_store_ops (stmt_vec_info stmt_info, > vectp_a.9_26 = vectp_a.9_7 + ivtmp_8; */ > tree loop_len > = vect_get_loop_len (loop_vinfo, gsi, loop_lens, 1, vectype, 0, 0); > - tree tmp > - = fold_build2 (MULT_EXPR, sizetype, > - fold_convert (sizetype, unshare_expr (DR_STEP (dr))), > - loop_len); > + tree tmp; > + gassign *assign; > + > + if (TREE_CODE (DR_STEP (dr)) == INTEGER_CST) > + tmp = fold_build2 (MULT_EXPR, sizetype, > + fold_convert (sizetype, unshare_expr (DR_STEP (dr))), > + loop_len); > + else > + { > + /* If DR_STEP = (unsigned int) _37 * 4; > + Extract _37 and 4, explicit MULT_EXPR. */ > + > + /* 1. step_stride = (unsigned int) _37. */ > + tree step_stride = make_ssa_name (create_tmp_var (sizetype)); > + assign = gimple_build_assign ( > + step_stride, TREE_OPERAND (TREE_OPERAND (DR_STEP (dr), 0), 0)); > + gsi_insert_before (gsi, assign, GSI_SAME_STMT); > + > + /* 2. step = step_stride * 4. */ > + tree step_align = TREE_OPERAND (TREE_OPERAND (DR_STEP (dr), 0), 1); > + tree step = make_ssa_name (create_tmp_var (sizetype)); > + assign > + = gimple_build_assign (step, fold_build2 (MULT_EXPR, sizetype, > + step_stride, step_align)); > + gsi_insert_before (gsi, assign, GSI_SAME_STMT); > + > + /* 3. tmp = step * loop_len. */ > + tmp = make_ssa_name (create_tmp_var (sizetype)); > + assign = gimple_build_assign (tmp, fold_build2 (MULT_EXPR, sizetype, > + step, loop_len)); > + gsi_insert_before (gsi, assign, GSI_SAME_STMT); > + } > tree bump = make_temp_ssa_name (sizetype, NULL, "ivtmp"); > - gassign *assign = gimple_build_assign (bump, tmp); instead of tree bump = make_temp_ssa_name (sizetype, NULL, "ivtmp"); gassign *assign = gimple_build_assign (bump, tmp); you can simply do tree bump = force_gimple_operand_gsi (gsi, tmp, true, NULL_TREE, true, GSI_SAME_STMT); That's all that is needed. Richard. > + assign = gimple_build_assign (bump, tmp); > gsi_insert_before (gsi, assign, GSI_SAME_STMT); > *dataref_bump = bump; > } > -- 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] 3+ messages in thread
end of thread, other threads:[~2023-07-06 6:52 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-07-06 6:16 [PATCH] VECT: Fix ICE of variable stride on strieded load/store with SELECT_VL loop control juzhe.zhong 2023-07-06 6:43 ` Richard Biener 2023-07-06 6:52 ` 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).