| Message ID | 20200722091641.8834-60-frank.chang@sifive.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv: support vector extension v0.9 | expand |
On 7/22/20 2:16 AM, frank.chang@sifive.com wrote: > +DEF_HELPER_6(vfslide1up_vf_h, void, ptr, ptr, i64, ptr, env, i32) > +DEF_HELPER_6(vfslide1up_vf_w, void, ptr, ptr, i64, ptr, env, i32) > +DEF_HELPER_6(vfslide1up_vf_d, void, ptr, ptr, i64, ptr, env, i32) > +DEF_HELPER_6(vfslide1down_vf_h, void, ptr, ptr, i64, ptr, env, i32) > +DEF_HELPER_6(vfslide1down_vf_w, void, ptr, ptr, i64, ptr, env, i32) > +DEF_HELPER_6(vfslide1down_vf_d, void, ptr, ptr, i64, ptr, env, i32) You should not need new helpers. The only difference between vfslide1up and vslide1up is the source and the nanboxing. Which you can do in the translator before using the existing helpers. r~
On Sat, Aug 1, 2020 at 12:05 AM Richard Henderson < richard.henderson@linaro.org> wrote: > On 7/22/20 2:16 AM, frank.chang@sifive.com wrote: > > +DEF_HELPER_6(vfslide1up_vf_h, void, ptr, ptr, i64, ptr, env, i32) > > +DEF_HELPER_6(vfslide1up_vf_w, void, ptr, ptr, i64, ptr, env, i32) > > +DEF_HELPER_6(vfslide1up_vf_d, void, ptr, ptr, i64, ptr, env, i32) > > +DEF_HELPER_6(vfslide1down_vf_h, void, ptr, ptr, i64, ptr, env, i32) > > +DEF_HELPER_6(vfslide1down_vf_w, void, ptr, ptr, i64, ptr, env, i32) > > +DEF_HELPER_6(vfslide1down_vf_d, void, ptr, ptr, i64, ptr, env, i32) > > You should not need new helpers. > > The only difference between vfslide1up and vslide1up is the source and the > nanboxing. Which you can do in the translator before using the existing > helpers. > > > r~ > I agree that the implementation of *vslide1up*'s helper function can be reused by *vfslide1up*. However, I've encountered an issue where the helper function called in *opfvf_trans()* used by *vfslide1up* takes *TCGv_i64* type as *s1* parameter. This conflicts with *vslide1up*'s helper function called in *opivx_trans(),* where *s1* parameter is the type of *TCGv* rather than *TCGv_i64*. i.e. *DEF_HELPER_6(vslide1up_vx_w, void, ptr, ptr, tl, ptr, env, i32)* vs. *DEF_HELPER_6(vfslide1up_vf_h, void, ptr, ptr, i64, ptr, env, i32)* As *opfvf_trans()* and *opivx_trans()* are shared among other instructions, I wouldn't like to make prototype changes to these functions. So far what I can come up with is to extract the logic in *vslide1up*'s helper function to an individual static function and make *vslide1up* and *vfslide1up*'s helper functions to call this static function. So that the're no redundant logic to be redefined. However, this still require to declare *vfslide1up'*s helper function explicitly as its function prototype is different with *vslide1up*. Any suggestions to this issue? Thanks Frank Chang
On 8/3/20 3:35 AM, Frank Chang wrote: > i.e. > /DEF_HELPER_6(vslide1up_vx_w, void, ptr, ptr, *tl*, ptr, env, i32)/ vs. > /DEF_HELPER_6(vfslide1up_vf_h, void, ptr, ptr, *i64*, ptr, env, i32)/ > > As /opfvf_trans()/ and /opivx_trans()/ are shared among other instructions, > I wouldn't like to make prototype changes to these functions. > > So far what I can come up with is to extract the logic in /vslide1up/'s > helper function to an individual static function and make /vslide1up/ > and /vfslide1up/'s helper functions to call this static function. > > So that the're no redundant logic to be redefined. > However, this still require to declare /vfslide1up'/s/ /helper function explicitly > as its function prototype is different with /vslide1up/. Well, of course the alternative is to adjust the prototypes to be the same, avoiding TCGv and tl. That would require an adjustment in the translator though, which would require a different form of macro instead of GEN_OPIVX_TRANS. It could be cleanest to do what you have done; hard to tell. r~
diff --git a/target/riscv/helper.h b/target/riscv/helper.h index b8a436d3aa..c6738336e8 100644 --- a/target/riscv/helper.h +++ b/target/riscv/helper.h @@ -1114,6 +1114,13 @@ DEF_HELPER_6(vslide1down_vx_h, void, ptr, ptr, tl, ptr, env, i32) DEF_HELPER_6(vslide1down_vx_w, void, ptr, ptr, tl, ptr, env, i32) DEF_HELPER_6(vslide1down_vx_d, void, ptr, ptr, tl, ptr, env, i32) +DEF_HELPER_6(vfslide1up_vf_h, void, ptr, ptr, i64, ptr, env, i32) +DEF_HELPER_6(vfslide1up_vf_w, void, ptr, ptr, i64, ptr, env, i32) +DEF_HELPER_6(vfslide1up_vf_d, void, ptr, ptr, i64, ptr, env, i32) +DEF_HELPER_6(vfslide1down_vf_h, void, ptr, ptr, i64, ptr, env, i32) +DEF_HELPER_6(vfslide1down_vf_w, void, ptr, ptr, i64, ptr, env, i32) +DEF_HELPER_6(vfslide1down_vf_d, void, ptr, ptr, i64, ptr, env, i32) + DEF_HELPER_6(vrgather_vv_b, void, ptr, ptr, ptr, ptr, env, i32) DEF_HELPER_6(vrgather_vv_h, void, ptr, ptr, ptr, ptr, env, i32) DEF_HELPER_6(vrgather_vv_w, void, ptr, ptr, ptr, ptr, env, i32) diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode index acd65cb3a7..ed34ccd0e3 100644 --- a/target/riscv/insn32.decode +++ b/target/riscv/insn32.decode @@ -530,6 +530,8 @@ vfsgnjn_vv 001001 . ..... ..... 001 ..... 1010111 @r_vm vfsgnjn_vf 001001 . ..... ..... 101 ..... 1010111 @r_vm vfsgnjx_vv 001010 . ..... ..... 001 ..... 1010111 @r_vm vfsgnjx_vf 001010 . ..... ..... 101 ..... 1010111 @r_vm +vfslide1up_vf 001110 . ..... ..... 101 ..... 1010111 @r_vm +vfslide1down_vf 001111 . ..... ..... 101 ..... 1010111 @r_vm vmfeq_vv 011000 . ..... ..... 001 ..... 1010111 @r_vm vmfeq_vf 011000 . ..... ..... 101 ..... 1010111 @r_vm vmfne_vv 011100 . ..... ..... 001 ..... 1010111 @r_vm diff --git a/target/riscv/insn_trans/trans_rvv.inc.c b/target/riscv/insn_trans/trans_rvv.inc.c index 5004707b87..d16db7f61a 100644 --- a/target/riscv/insn_trans/trans_rvv.inc.c +++ b/target/riscv/insn_trans/trans_rvv.inc.c @@ -3514,6 +3514,10 @@ GEN_OPIVX_TRANS(vslidedown_vx, slidedown_check) GEN_OPIVX_TRANS(vslide1down_vx, slidedown_check) GEN_OPIVI_TRANS(vslidedown_vi, IMM_ZX, vslidedown_vx, slidedown_check) +/* Vector Floating-Point Slide Instructions */ +GEN_OPFVF_TRANS(vfslide1up_vf, slideup_check) +GEN_OPFVF_TRANS(vfslide1down_vf, slidedown_check) + /* Vector Register Gather Instruction */ static bool vrgather_vv_check(DisasContext *s, arg_rmrr *a) { diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c index b47ca6c406..4036be4425 100644 --- a/target/riscv/vector_helper.c +++ b/target/riscv/vector_helper.c @@ -5023,6 +5023,57 @@ GEN_VEXT_VSLIDE1DOWN_VX(vslide1down_vx_h, uint16_t, H2) GEN_VEXT_VSLIDE1DOWN_VX(vslide1down_vx_w, uint32_t, H4) GEN_VEXT_VSLIDE1DOWN_VX(vslide1down_vx_d, uint64_t, H8) +/* Vector Floating-Point Slide Instructions */ +#define GEN_VEXT_VFSLIDE1UP_VF(NAME, ETYPE, H) \ +void HELPER(NAME)(void *vd, void *v0, uint64_t s1, void *vs2, \ + CPURISCVState *env, uint32_t desc) \ +{ \ + uint32_t vm = vext_vm(desc); \ + uint32_t vl = env->vl; \ + uint32_t i; \ + \ + for (i = 0; i < vl; i++) { \ + if (!vm && !vext_elem_mask(v0, i)) { \ + continue; \ + } \ + if (i == 0) { \ + *((ETYPE *)vd + H(i)) = s1; \ + } else { \ + *((ETYPE *)vd + H(i)) = *((ETYPE *)vs2 + H(i - 1)); \ + } \ + } \ +} + +/* vfslide1up.vf vd, vs2, rs1, vm # vd[0]=f[rs1], vd[i+1] = vs2[i] */ +GEN_VEXT_VFSLIDE1UP_VF(vfslide1up_vf_h, uint16_t, H1) +GEN_VEXT_VFSLIDE1UP_VF(vfslide1up_vf_w, uint32_t, H1) +GEN_VEXT_VFSLIDE1UP_VF(vfslide1up_vf_d, uint64_t, H1) + +#define GEN_VEXT_VFSLIDE1DOWN_VF(NAME, ETYPE, H) \ +void HELPER(NAME)(void *vd, void *v0, uint64_t s1, void *vs2, \ + CPURISCVState *env, uint32_t desc) \ +{ \ + uint32_t vm = vext_vm(desc); \ + uint32_t vl = env->vl; \ + uint32_t i; \ + \ + for (i = 0; i < vl; i++) { \ + if (!vm && !vext_elem_mask(v0, i)) { \ + continue; \ + } \ + if (i == vl - 1) { \ + *((ETYPE *)vd + H(i)) = s1; \ + } else { \ + *((ETYPE *)vd + H(i)) = *((ETYPE *)vs2 + H(i + 1)); \ + } \ + } \ +} + +/* vfslide1down.vf vd, vs2, rs1, vm # vd[i] = vs2[i+1], vd[vl-1]=f[rs1] */ +GEN_VEXT_VFSLIDE1DOWN_VF(vfslide1down_vf_h, uint16_t, H1) +GEN_VEXT_VFSLIDE1DOWN_VF(vfslide1down_vf_w, uint32_t, H1) +GEN_VEXT_VFSLIDE1DOWN_VF(vfslide1down_vf_d, uint64_t, H1) + /* Vector Register Gather Instruction */ #define GEN_VEXT_VRGATHER_VV(NAME, ETYPE, H, CLEAR_FN) \ void HELPER(NAME)(void *vd, void *v0, void *vs1, void *vs2, \