[Qemu-devel] [PATCH] tcg/mips: fix invalid op definition errors

[Qemu-devel] [PATCH] tcg/mips: fix invalid op definition errors

[James Hogan]

tcg/mips/tcg-target.h defines various operations conditionally depending
upon the isa revision, however these operations are included in
mips_op_defs[] unconditionally resulting in the following runtime errors
if CONFIG_DEBUG_TCG is defined:

Invalid op definition for movcond_i32
Invalid op definition for rotl_i32
Invalid op definition for rotr_i32
Invalid op definition for deposit_i32
Invalid op definition for bswap16_i32
Invalid op definition for bswap32_i32
tcg/tcg.c:1196: tcg fatal error

Fix with ifdefs like the i386 backend does for movcond_i32.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Richard Henderson <rth@twiddle.net>
---
 tcg/mips/tcg-target.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
index 373c364..793532e 100644
--- a/tcg/mips/tcg-target.c
+++ b/tcg/mips/tcg-target.c
@@ -1617,19 +1617,29 @@ static const TCGTargetOpDef mips_op_defs[] = {
     { INDEX_op_shl_i32, { "r", "rZ", "ri" } },
     { INDEX_op_shr_i32, { "r", "rZ", "ri" } },
     { INDEX_op_sar_i32, { "r", "rZ", "ri" } },
+#if TCG_TARGET_HAS_rot_i32
     { INDEX_op_rotr_i32, { "r", "rZ", "ri" } },
     { INDEX_op_rotl_i32, { "r", "rZ", "ri" } },
+#endif
 
+#if TCG_TARGET_HAS_bswap16_i32
     { INDEX_op_bswap16_i32, { "r", "r" } },
+#endif
+#if TCG_TARGET_HAS_bswap32_i32
     { INDEX_op_bswap32_i32, { "r", "r" } },
+#endif
 
     { INDEX_op_ext8s_i32, { "r", "rZ" } },
     { INDEX_op_ext16s_i32, { "r", "rZ" } },
 
+#if TCG_TARGET_HAS_deposit_i32
     { INDEX_op_deposit_i32, { "r", "0", "rZ" } },
+#endif
 
     { INDEX_op_brcond_i32, { "rZ", "rZ" } },
+#if TCG_TARGET_HAS_movcond_i32
     { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "0" } },
+#endif
     { INDEX_op_setcond_i32, { "r", "rZ", "rZ" } },
     { INDEX_op_setcond2_i32, { "r", "rZ", "rZ", "rZ", "rZ" } },
 
-- 
1.8.1.2

Re: [Qemu-devel] [PATCH] tcg/mips: fix invalid op definition errors

[Richard Henderson]

On 08/08/2013 04:40 AM, James Hogan wrote:
> tcg/mips/tcg-target.h defines various operations conditionally depending
> upon the isa revision, however these operations are included in
> mips_op_defs[] unconditionally resulting in the following runtime errors
> if CONFIG_DEBUG_TCG is defined:
> 
> Invalid op definition for movcond_i32
> Invalid op definition for rotl_i32
> Invalid op definition for rotr_i32
> Invalid op definition for deposit_i32
> Invalid op definition for bswap16_i32
> Invalid op definition for bswap32_i32
> tcg/tcg.c:1196: tcg fatal error
> 
> Fix with ifdefs like the i386 backend does for movcond_i32.
> 
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Richard Henderson <rth@twiddle.net>

Reviewed-by: Richard Henderson <rth@twiddle.net>

Perfect for 1.6.

For 1.7 it would be really nice if you could figure out some way to make
these runtime tests, instead of ifdefs.  I'd have said getauxval(3), but
the mips kernel doesn't seem to define any identifying bits.  Perhaps
that's the first thing that ought to get fixed...


r~

Re: [Qemu-devel] [PATCH] tcg/mips: fix invalid op definition errors

[James Hogan]

On 08/08/13 17:10, Richard Henderson wrote:
> On 08/08/2013 04:40 AM, James Hogan wrote:
>> tcg/mips/tcg-target.h defines various operations conditionally depending
>> upon the isa revision, however these operations are included in
>> mips_op_defs[] unconditionally resulting in the following runtime errors
>> if CONFIG_DEBUG_TCG is defined:
>>
>> Invalid op definition for movcond_i32
>> Invalid op definition for rotl_i32
>> Invalid op definition for rotr_i32
>> Invalid op definition for deposit_i32
>> Invalid op definition for bswap16_i32
>> Invalid op definition for bswap32_i32
>> tcg/tcg.c:1196: tcg fatal error
>>
>> Fix with ifdefs like the i386 backend does for movcond_i32.
>>
>> Signed-off-by: James Hogan <james.hogan@imgtec.com>
>> Cc: Aurelien Jarno <aurelien@aurel32.net>
>> Cc: Richard Henderson <rth@twiddle.net>
> 
> Reviewed-by: Richard Henderson <rth@twiddle.net>

Thanks,

> Perfect for 1.6.
> 
> For 1.7 it would be really nice if you could figure out some way to make
> these runtime tests, instead of ifdefs.  I'd have said getauxval(3), but
> the mips kernel doesn't seem to define any identifying bits.  Perhaps
> that's the first thing that ought to get fixed...

Yes, the auxvec sounds ideal for this, and AT_HWCAP is already used for
cpuid on x86. There were some patches a while ago for exposing the C0
configX registers through sysfs, but auxvec sounds cleaner IMO.

Cheers
James

Re: [Qemu-devel] [PATCH] tcg/mips: fix invalid op definition errors

[Aurelien Jarno]

On Thu, Aug 08, 2013 at 03:40:23PM +0100, James Hogan wrote:
> tcg/mips/tcg-target.h defines various operations conditionally depending
> upon the isa revision, however these operations are included in
> mips_op_defs[] unconditionally resulting in the following runtime errors
> if CONFIG_DEBUG_TCG is defined:
> 
> Invalid op definition for movcond_i32
> Invalid op definition for rotl_i32
> Invalid op definition for rotr_i32
> Invalid op definition for deposit_i32
> Invalid op definition for bswap16_i32
> Invalid op definition for bswap32_i32
> tcg/tcg.c:1196: tcg fatal error
> 
> Fix with ifdefs like the i386 backend does for movcond_i32.
> 
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Aurelien Jarno <aurelien@aurel32.net>
> Cc: Richard Henderson <rth@twiddle.net>
> ---
>  tcg/mips/tcg-target.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/tcg/mips/tcg-target.c b/tcg/mips/tcg-target.c
> index 373c364..793532e 100644
> --- a/tcg/mips/tcg-target.c
> +++ b/tcg/mips/tcg-target.c
> @@ -1617,19 +1617,29 @@ static const TCGTargetOpDef mips_op_defs[] = {
>      { INDEX_op_shl_i32, { "r", "rZ", "ri" } },
>      { INDEX_op_shr_i32, { "r", "rZ", "ri" } },
>      { INDEX_op_sar_i32, { "r", "rZ", "ri" } },
> +#if TCG_TARGET_HAS_rot_i32
>      { INDEX_op_rotr_i32, { "r", "rZ", "ri" } },
>      { INDEX_op_rotl_i32, { "r", "rZ", "ri" } },
> +#endif
>  
> +#if TCG_TARGET_HAS_bswap16_i32
>      { INDEX_op_bswap16_i32, { "r", "r" } },
> +#endif
> +#if TCG_TARGET_HAS_bswap32_i32
>      { INDEX_op_bswap32_i32, { "r", "r" } },
> +#endif
>  
>      { INDEX_op_ext8s_i32, { "r", "rZ" } },
>      { INDEX_op_ext16s_i32, { "r", "rZ" } },
>  
> +#if TCG_TARGET_HAS_deposit_i32
>      { INDEX_op_deposit_i32, { "r", "0", "rZ" } },
> +#endif
>  
>      { INDEX_op_brcond_i32, { "rZ", "rZ" } },
> +#if TCG_TARGET_HAS_movcond_i32
>      { INDEX_op_movcond_i32, { "r", "rZ", "rZ", "rZ", "0" } },
> +#endif
>      { INDEX_op_setcond_i32, { "r", "rZ", "rZ" } },
>      { INDEX_op_setcond2_i32, { "r", "rZ", "rZ", "rZ", "rZ" } },
>  

Thanks, applied.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net