Patchwork [v3,08/14] tcg-arm: Make use of conditional availability of opcodes for divide

login
register
mail settings
Submitter Richard Henderson
Date July 3, 2013, 9:29 p.m.
Message ID <1372886968-17497-9-git-send-email-rth@twiddle.net>
Download mbox | patch
Permalink /patch/256759/
State New
Headers show

Comments

Richard Henderson - July 3, 2013, 9:29 p.m.
We can now detect and use divide instructions at runtime, rather than
having to restrict their availability to compile-time.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/arm/tcg-target.c | 16 ++++++++++++++--
 tcg/arm/tcg-target.h | 14 ++++++++------
 2 files changed, 22 insertions(+), 8 deletions(-)
Peter Maydell - July 4, 2013, 11:02 a.m.
On 3 July 2013 22:29, Richard Henderson <rth@twiddle.net> wrote:
> We can now detect and use divide instructions at runtime, rather than
> having to restrict their availability to compile-time.
>
> Signed-off-by: Richard Henderson <rth@twiddle.net>
> ---
>  tcg/arm/tcg-target.c | 16 ++++++++++++++--
>  tcg/arm/tcg-target.h | 14 ++++++++------
>  2 files changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c
> index 8321f80..2c46ceb 100644
> --- a/tcg/arm/tcg-target.c
> +++ b/tcg/arm/tcg-target.c
> @@ -67,6 +67,13 @@ static const int use_armv7_instructions = 0;
>  #endif
>  #undef USE_ARMV7_INSTRUCTIONS
>
> +#ifndef use_idiv_instructions
> +bool use_idiv_instructions;
> +#endif
> +#ifdef CONFIG_GETAUXVAL
> +# include <sys/auxv.h>
> +#endif

My ARM system doesn't have a sys/auxv.h, which renders most of this patch
a bit moot (and certainly untestable :-)). Do newer glibc have this?

> +
>  #ifndef NDEBUG
>  static const char * const tcg_target_reg_names[TCG_TARGET_NB_REGS] = {
>      "%r0",
> @@ -2029,16 +2036,21 @@ static const TCGTargetOpDef arm_op_defs[] = {
>
>      { INDEX_op_deposit_i32, { "r", "0", "rZ" } },
>
> -#if TCG_TARGET_HAS_div_i32
>      { INDEX_op_div_i32, { "r", "r", "r" } },
>      { INDEX_op_divu_i32, { "r", "r", "r" } },
> -#endif
>
>      { -1 },
>  };
>
>  static void tcg_target_init(TCGContext *s)
>  {
> +#if defined(CONFIG_GETAUXVAL) && !defined(use_idiv_instructions)
> +    {
> +        unsigned long hwcap = getauxval(AT_HWCAP);
> +        use_idiv_instructions = hwcap & (HWCAP_ARM_IDIVA | HWCAP_ARM_IDIVT);

Doesn't this mean we'll try to use the ARM division
insns even if the CPU only supports the Thumb encodings?
I think you should only be testing for whether HWCAP_ARM_IDIVA
is set.

> +    }
> +#endif

thanks
-- PMM
Richard Henderson - July 4, 2013, 5:33 p.m.
On 07/04/2013 04:02 AM, Peter Maydell wrote:
> 
> My ARM system doesn't have a sys/auxv.h, which renders most of this patch
> a bit moot (and certainly untestable :-)). Do newer glibc have this?

glibc 2.16 has this.

I've also got another pending patch set that implements getauxval inside
qemu if it's not present in the system library.  Which means that we can
eliminate some of the ifdefery seen here.

>> +        unsigned long hwcap = getauxval(AT_HWCAP);
>> +        use_idiv_instructions = hwcap & (HWCAP_ARM_IDIVA | HWCAP_ARM_IDIVT);
> 
> Doesn't this mean we'll try to use the ARM division
> insns even if the CPU only supports the Thumb encodings?
> I think you should only be testing for whether HWCAP_ARM_IDIVA
> is set.

I suppose.  Though later kernels have actually merged these bits:

uapi/asm/hwcap.h:27:#define HWCAP_IDIV	(HWCAP_IDIVA | HWCAP_IDIVT)


r~
Peter Maydell - July 4, 2013, 5:39 p.m.
On 4 July 2013 18:33, Richard Henderson <rth@twiddle.net> wrote:
> On 07/04/2013 04:02 AM, Peter Maydell wrote:
>>> +        unsigned long hwcap = getauxval(AT_HWCAP);
>>> +        use_idiv_instructions = hwcap & (HWCAP_ARM_IDIVA | HWCAP_ARM_IDIVT);
>>
>> Doesn't this mean we'll try to use the ARM division
>> insns even if the CPU only supports the Thumb encodings?
>> I think you should only be testing for whether HWCAP_ARM_IDIVA
>> is set.
>
> I suppose.  Though later kernels have actually merged these bits:
>
> uapi/asm/hwcap.h:27:#define HWCAP_IDIV  (HWCAP_IDIVA | HWCAP_IDIVT)

Yes, but the two bits are still separate values:

 #define HWCAP_IDIVA     (1 << 17)
 #define HWCAP_IDIVT     (1 << 18)

Presumably the HWCAP_IDIV is if you want to check that you
have full division support via
  (hwcap & HWCAP_IDIV) == HWCAP_IDIV

We could do that if you prefer; it would be effectively
the same as just testing the IDIVA bit though.

-- PMM

Patch

diff --git a/tcg/arm/tcg-target.c b/tcg/arm/tcg-target.c
index 8321f80..2c46ceb 100644
--- a/tcg/arm/tcg-target.c
+++ b/tcg/arm/tcg-target.c
@@ -67,6 +67,13 @@  static const int use_armv7_instructions = 0;
 #endif
 #undef USE_ARMV7_INSTRUCTIONS
 
+#ifndef use_idiv_instructions
+bool use_idiv_instructions;
+#endif
+#ifdef CONFIG_GETAUXVAL
+# include <sys/auxv.h>
+#endif
+
 #ifndef NDEBUG
 static const char * const tcg_target_reg_names[TCG_TARGET_NB_REGS] = {
     "%r0",
@@ -2029,16 +2036,21 @@  static const TCGTargetOpDef arm_op_defs[] = {
 
     { INDEX_op_deposit_i32, { "r", "0", "rZ" } },
 
-#if TCG_TARGET_HAS_div_i32
     { INDEX_op_div_i32, { "r", "r", "r" } },
     { INDEX_op_divu_i32, { "r", "r", "r" } },
-#endif
 
     { -1 },
 };
 
 static void tcg_target_init(TCGContext *s)
 {
+#if defined(CONFIG_GETAUXVAL) && !defined(use_idiv_instructions)
+    {
+        unsigned long hwcap = getauxval(AT_HWCAP);
+        use_idiv_instructions = hwcap & (HWCAP_ARM_IDIVA | HWCAP_ARM_IDIVT);
+    }
+#endif
+
     tcg_regset_set32(tcg_target_available_regs[TCG_TYPE_I32], 0, 0xffff);
     tcg_regset_set32(tcg_target_call_clobber_regs, 0,
                      (1 << TCG_REG_R0) |
diff --git a/tcg/arm/tcg-target.h b/tcg/arm/tcg-target.h
index 263ea03..5cd9d6a 100644
--- a/tcg/arm/tcg-target.h
+++ b/tcg/arm/tcg-target.h
@@ -49,6 +49,13 @@  typedef enum {
 
 #define TCG_TARGET_NB_REGS 16
 
+#ifdef __ARM_ARCH_EXT_IDIV__
+#define use_idiv_instructions  1
+#else
+extern bool use_idiv_instructions;
+#endif
+
+
 /* used for function call generation */
 #define TCG_REG_CALL_STACK		TCG_REG_R13
 #define TCG_TARGET_STACK_ALIGN		8
@@ -73,12 +80,7 @@  typedef enum {
 #define TCG_TARGET_HAS_deposit_i32      1
 #define TCG_TARGET_HAS_movcond_i32      1
 #define TCG_TARGET_HAS_muls2_i32        1
-
-#ifdef __ARM_ARCH_EXT_IDIV__
-#define TCG_TARGET_HAS_div_i32          1
-#else
-#define TCG_TARGET_HAS_div_i32          0
-#endif
+#define TCG_TARGET_HAS_div_i32          use_idiv_instructions
 #define TCG_TARGET_HAS_rem_i32          0
 
 extern bool tcg_target_deposit_valid(int ofs, int len);