| Message ID | 20190629130017.2973-5-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tcg/ppc: Add vector opcodes | expand |
On Saturday, June 29, 2019, Richard Henderson <richard.henderson@linaro.org> wrote: > Introduce all of the flags required to enable tcg backend vector support, > and a runtime flag to indicate the host supports Altivec instructions. > > If two flags have different purpose and usage, it is better that they have different names. (perhaps one of them should have the suffix “_runtime“) Also, I am not sure if Altiveec can be reffered as isa, it is a part/extension of an isa, so “isa” seems superfluous here. checkpatch warning should also be honored. > For now, do not actually set have_isa_altivec to true, because we have not > yet added all of the code to actually generate all of the required insns. > However, we must define these flags in order to disable ifndefs that create > stub versions of the functions added here. > > The change to tcg_out_movi works around a buglet in tcg.c wherein if we > do not define tcg_out_dupi_vec we get a declared but not defined Werror, > but if we only declare it we get a defined but not used Werror. We need > to this change to tcg_out_movi eventually anyway, so it's no biggie. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com> > --- > tcg/ppc/tcg-target.h | 25 ++++++++++++++++ > tcg/ppc/tcg-target.opc.h | 5 ++++ > tcg/ppc/tcg-target.inc.c | 65 ++++++++++++++++++++++++++++++++++++++-- > 3 files changed, 92 insertions(+), 3 deletions(-) > create mode 100644 tcg/ppc/tcg-target.opc.h > > diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h > index 690fa744e1..f6283f468b 100644 > --- a/tcg/ppc/tcg-target.h > +++ b/tcg/ppc/tcg-target.h > @@ -58,6 +58,7 @@ typedef enum { > TCG_AREG0 = TCG_REG_R27 > } TCGReg; > > +extern bool have_isa_altivec; > extern bool have_isa_2_06; > extern bool have_isa_3_00; > > @@ -135,6 +136,30 @@ extern bool have_isa_3_00; > #define TCG_TARGET_HAS_mulsh_i64 1 > #endif > > +/* > + * While technically Altivec could support V64, it has no 64-bit store > + * instruction and substituting two 32-bit stores makes the generated > + * code quite large. > + */ > +#define TCG_TARGET_HAS_v64 0 > +#define TCG_TARGET_HAS_v128 have_isa_altivec > +#define TCG_TARGET_HAS_v256 0 > + > +#define TCG_TARGET_HAS_andc_vec 0 > +#define TCG_TARGET_HAS_orc_vec 0 > +#define TCG_TARGET_HAS_not_vec 0 > +#define TCG_TARGET_HAS_neg_vec 0 > +#define TCG_TARGET_HAS_abs_vec 0 > +#define TCG_TARGET_HAS_shi_vec 0 > +#define TCG_TARGET_HAS_shs_vec 0 > +#define TCG_TARGET_HAS_shv_vec 0 > +#define TCG_TARGET_HAS_cmp_vec 0 > +#define TCG_TARGET_HAS_mul_vec 0 > +#define TCG_TARGET_HAS_sat_vec 0 > +#define TCG_TARGET_HAS_minmax_vec 0 > +#define TCG_TARGET_HAS_bitsel_vec 0 > +#define TCG_TARGET_HAS_cmpsel_vec 0 > + > void flush_icache_range(uintptr_t start, uintptr_t stop); > void tb_target_set_jmp_target(uintptr_t, uintptr_t, uintptr_t); > > diff --git a/tcg/ppc/tcg-target.opc.h b/tcg/ppc/tcg-target.opc.h > new file mode 100644 > index 0000000000..fa680dd6a0 > --- /dev/null > +++ b/tcg/ppc/tcg-target.opc.h > @@ -0,0 +1,5 @@ > +/* > + * Target-specific opcodes for host vector expansion. These will be > + * emitted by tcg_expand_vec_op. For those familiar with GCC internals, > + * consider these to be UNSPEC with names. > + */ > diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c > index cfbd7ff12c..b938e9aac5 100644 > --- a/tcg/ppc/tcg-target.inc.c > +++ b/tcg/ppc/tcg-target.inc.c > @@ -64,6 +64,7 @@ > > static tcg_insn_unit *tb_ret_addr; > > +bool have_isa_altivec; > bool have_isa_2_06; > bool have_isa_3_00; > > @@ -717,10 +718,31 @@ static void tcg_out_movi_int(TCGContext *s, TCGType > type, TCGReg ret, > } > } > > -static inline void tcg_out_movi(TCGContext *s, TCGType type, TCGReg ret, > - tcg_target_long arg) > +static void tcg_out_dupi_vec(TCGContext *s, TCGType type, TCGReg ret, > + tcg_target_long val) > { > - tcg_out_movi_int(s, type, ret, arg, false); > + g_assert_not_reached(); > +} > + > +static void tcg_out_movi(TCGContext *s, TCGType type, TCGReg ret, > + tcg_target_long arg) > +{ > + switch (type) { > + case TCG_TYPE_I32: > + case TCG_TYPE_I64: > + tcg_debug_assert(ret < TCG_REG_V0); > + tcg_out_movi_int(s, type, ret, arg, false); > + break; > + > + case TCG_TYPE_V64: > + case TCG_TYPE_V128: > + tcg_debug_assert(ret >= TCG_REG_V0); > + tcg_out_dupi_vec(s, type, ret, arg); > + break; > + > + default: > + g_assert_not_reached(); > + } > } > > static bool mask_operand(uint32_t c, int *mb, int *me) > @@ -2605,6 +2627,36 @@ static void tcg_out_op(TCGContext *s, TCGOpcode > opc, const TCGArg *args, > } > } > > +int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type, unsigned vece) > +{ > + g_assert_not_reached(); > +} > + > +static bool tcg_out_dup_vec(TCGContext *s, TCGType type, unsigned vece, > + TCGReg dst, TCGReg src) > +{ > + g_assert_not_reached(); > +} > + > +static bool tcg_out_dupm_vec(TCGContext *s, TCGType type, unsigned vece, > + TCGReg out, TCGReg base, intptr_t offset) > +{ > + g_assert_not_reached(); > +} > + > +static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc, > + unsigned vecl, unsigned vece, > + const TCGArg *args, const int *const_args) > +{ > + g_assert_not_reached(); > +} > + > +void tcg_expand_vec_op(TCGOpcode opc, TCGType type, unsigned vece, > + TCGArg a0, ...) > +{ > + g_assert_not_reached(); > +} > + > static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode op) > { > static const TCGTargetOpDef r = { .args_ct_str = { "r" } }; > @@ -2787,6 +2839,9 @@ static void tcg_target_init(TCGContext *s) > unsigned long hwcap = qemu_getauxval(AT_HWCAP); > unsigned long hwcap2 = qemu_getauxval(AT_HWCAP2); > > + if (hwcap & /* PPC_FEATURE_HAS_ALTIVEC -- NOT YET */ 0) { > + have_isa_altivec = true; > + } > if (hwcap & PPC_FEATURE_ARCH_2_06) { > have_isa_2_06 = true; > } > @@ -2798,6 +2853,10 @@ static void tcg_target_init(TCGContext *s) > > tcg_target_available_regs[TCG_TYPE_I32] = 0xffffffff; > tcg_target_available_regs[TCG_TYPE_I64] = 0xffffffff; > + if (have_isa_altivec) { > + tcg_target_available_regs[TCG_TYPE_V64] = 0xffffffff00000000ull; > + tcg_target_available_regs[TCG_TYPE_V128] = 0xffffffff00000000ull; > + } > > tcg_target_call_clobber_regs = 0; > tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_R0); > -- > 2.17.1 > > >
On 6/30/19 11:46 AM, Aleksandar Markovic wrote: > > > On Saturday, June 29, 2019, Richard Henderson <richard.henderson@linaro.org > <mailto:richard.henderson@linaro.org>> wrote: > > Introduce all of the flags required to enable tcg backend vector support, > and a runtime flag to indicate the host supports Altivec instructions. > > > If two flags have different purpose and usage, it is better that they > have different names. (perhaps one of them should have the suffix “_runtime“) Huh? They do have different names. Very different names. > Also, I am not sure if Altiveec can be reffered as isa, it is a part/extension > of an isa, so “isa” seems superfluous here. It also matches the other existing names, so I'll leave it as is. > checkpatch warning should also be honored. It's bogus. > WARNING: Block comments use a leading /* on a separate line > #155: FILE: tcg/ppc/tcg-target.inc.c:2842: > + if (hwcap & /* PPC_FEATURE_HAS_ALTIVEC -- NOT YET */ 0) { It's not a block comment; the whole thing is on one line. I have no idea why it doesn't notice. In any case, this goes away in patch 13. r~
On Jun 30, 2019 12:48 PM, "Richard Henderson" <richard.henderson@linaro.org> wrote: > > On 6/30/19 11:46 AM, Aleksandar Markovic wrote: > > > > > > On Saturday, June 29, 2019, Richard Henderson < richard.henderson@linaro.org > > <mailto:richard.henderson@linaro.org>> wrote: > > > > Introduce all of the flags required to enable tcg backend vector support, > > and a runtime flag to indicate the host supports Altivec instructions. > > > > > > If two flags have different purpose and usage, it is better that they > > have different names. (perhaps one of them should have the suffix “_runtime“) > > Huh? They do have different names. Very different names. > They do. If you leave the same name, you would make any search for that name during future debugging/development more difficult. > > Also, I am not sure if Altiveec can be reffered as isa, it is a part/extension > > of an isa, so “isa” seems superfluous here. > > It also matches the other existing names, so I'll leave it as is. > If something is wrong in the old code, it does not mean one should continue the same practice. > > checkpatch warning should also be honored. > > It's bogus. I don't think it is bogus. The comment should be converted to a regular one-line ot perhaps multiline comment before if-statement. Althoug it may be correct in the sense of C-syntax, noone expects coment to be inlined into if-condition, and it makes the code feel obfuscated, rather than clear. > > WARNING: Block comments use a leading /* on a separate line > > #155: FILE: tcg/ppc/tcg-target.inc.c:2842: > > + if (hwcap & /* PPC_FEATURE_HAS_ALTIVEC -- NOT YET */ 0) { > > It's not a block comment; the whole thing is on one line. > I have no idea why it doesn't notice. > > In any case, this goes away in patch 13. > > > r~
diff --git a/tcg/ppc/tcg-target.h b/tcg/ppc/tcg-target.h index 690fa744e1..f6283f468b 100644 --- a/tcg/ppc/tcg-target.h +++ b/tcg/ppc/tcg-target.h @@ -58,6 +58,7 @@ typedef enum { TCG_AREG0 = TCG_REG_R27 } TCGReg; +extern bool have_isa_altivec; extern bool have_isa_2_06; extern bool have_isa_3_00; @@ -135,6 +136,30 @@ extern bool have_isa_3_00; #define TCG_TARGET_HAS_mulsh_i64 1 #endif +/* + * While technically Altivec could support V64, it has no 64-bit store + * instruction and substituting two 32-bit stores makes the generated + * code quite large. + */ +#define TCG_TARGET_HAS_v64 0 +#define TCG_TARGET_HAS_v128 have_isa_altivec +#define TCG_TARGET_HAS_v256 0 + +#define TCG_TARGET_HAS_andc_vec 0 +#define TCG_TARGET_HAS_orc_vec 0 +#define TCG_TARGET_HAS_not_vec 0 +#define TCG_TARGET_HAS_neg_vec 0 +#define TCG_TARGET_HAS_abs_vec 0 +#define TCG_TARGET_HAS_shi_vec 0 +#define TCG_TARGET_HAS_shs_vec 0 +#define TCG_TARGET_HAS_shv_vec 0 +#define TCG_TARGET_HAS_cmp_vec 0 +#define TCG_TARGET_HAS_mul_vec 0 +#define TCG_TARGET_HAS_sat_vec 0 +#define TCG_TARGET_HAS_minmax_vec 0 +#define TCG_TARGET_HAS_bitsel_vec 0 +#define TCG_TARGET_HAS_cmpsel_vec 0 + void flush_icache_range(uintptr_t start, uintptr_t stop); void tb_target_set_jmp_target(uintptr_t, uintptr_t, uintptr_t); diff --git a/tcg/ppc/tcg-target.opc.h b/tcg/ppc/tcg-target.opc.h new file mode 100644 index 0000000000..fa680dd6a0 --- /dev/null +++ b/tcg/ppc/tcg-target.opc.h @@ -0,0 +1,5 @@ +/* + * Target-specific opcodes for host vector expansion. These will be + * emitted by tcg_expand_vec_op. For those familiar with GCC internals, + * consider these to be UNSPEC with names. + */ diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c index cfbd7ff12c..b938e9aac5 100644 --- a/tcg/ppc/tcg-target.inc.c +++ b/tcg/ppc/tcg-target.inc.c @@ -64,6 +64,7 @@ static tcg_insn_unit *tb_ret_addr; +bool have_isa_altivec; bool have_isa_2_06; bool have_isa_3_00; @@ -717,10 +718,31 @@ static void tcg_out_movi_int(TCGContext *s, TCGType type, TCGReg ret, } } -static inline void tcg_out_movi(TCGContext *s, TCGType type, TCGReg ret, - tcg_target_long arg) +static void tcg_out_dupi_vec(TCGContext *s, TCGType type, TCGReg ret, + tcg_target_long val) { - tcg_out_movi_int(s, type, ret, arg, false); + g_assert_not_reached(); +} + +static void tcg_out_movi(TCGContext *s, TCGType type, TCGReg ret, + tcg_target_long arg) +{ + switch (type) { + case TCG_TYPE_I32: + case TCG_TYPE_I64: + tcg_debug_assert(ret < TCG_REG_V0); + tcg_out_movi_int(s, type, ret, arg, false); + break; + + case TCG_TYPE_V64: + case TCG_TYPE_V128: + tcg_debug_assert(ret >= TCG_REG_V0); + tcg_out_dupi_vec(s, type, ret, arg); + break; + + default: + g_assert_not_reached(); + } } static bool mask_operand(uint32_t c, int *mb, int *me) @@ -2605,6 +2627,36 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args, } } +int tcg_can_emit_vec_op(TCGOpcode opc, TCGType type, unsigned vece) +{ + g_assert_not_reached(); +} + +static bool tcg_out_dup_vec(TCGContext *s, TCGType type, unsigned vece, + TCGReg dst, TCGReg src) +{ + g_assert_not_reached(); +} + +static bool tcg_out_dupm_vec(TCGContext *s, TCGType type, unsigned vece, + TCGReg out, TCGReg base, intptr_t offset) +{ + g_assert_not_reached(); +} + +static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc, + unsigned vecl, unsigned vece, + const TCGArg *args, const int *const_args) +{ + g_assert_not_reached(); +} + +void tcg_expand_vec_op(TCGOpcode opc, TCGType type, unsigned vece, + TCGArg a0, ...) +{ + g_assert_not_reached(); +} + static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode op) { static const TCGTargetOpDef r = { .args_ct_str = { "r" } }; @@ -2787,6 +2839,9 @@ static void tcg_target_init(TCGContext *s) unsigned long hwcap = qemu_getauxval(AT_HWCAP); unsigned long hwcap2 = qemu_getauxval(AT_HWCAP2); + if (hwcap & /* PPC_FEATURE_HAS_ALTIVEC -- NOT YET */ 0) { + have_isa_altivec = true; + } if (hwcap & PPC_FEATURE_ARCH_2_06) { have_isa_2_06 = true; } @@ -2798,6 +2853,10 @@ static void tcg_target_init(TCGContext *s) tcg_target_available_regs[TCG_TYPE_I32] = 0xffffffff; tcg_target_available_regs[TCG_TYPE_I64] = 0xffffffff; + if (have_isa_altivec) { + tcg_target_available_regs[TCG_TYPE_V64] = 0xffffffff00000000ull; + tcg_target_available_regs[TCG_TYPE_V128] = 0xffffffff00000000ull; + } tcg_target_call_clobber_regs = 0; tcg_regset_set_reg(tcg_target_call_clobber_regs, TCG_REG_R0);