tcg/mips: fix invalid op definition errors

Submitted by James Hogan on Aug. 8, 2013, 2:40 p.m.

Details

Message ID 1375972823-25333-1-git-send-email-james.hogan@imgtec.com
State New
Headers show

Commit Message

James Hogan Aug. 8, 2013, 2:40 p.m.
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(+)

Comments

Richard Henderson Aug. 8, 2013, 4:10 p.m.
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~
Aurelien Jarno Aug. 8, 2013, 9:13 p.m.
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.
James Hogan Aug. 13, 2013, 8:59 a.m.
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

Patch hide | download patch | download mbox

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" } },