[v3] tcg: backend constraints cleanup

[PATCH v3 04/24] tcg/i386: Tidy register constraint definitions

Posted by Richard Henderson 3 years, 9 months ago

Create symbolic constants for all low-byte-addressable
and second-byte-addressable registers.  Create a symbol
for the registers that need reserving for softmmu.

There is no functional change for 's', as this letter is
only used for i386.  The BYTEL name is correct for the
action we wish from the constraint.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tcg/i386/tcg-target.c.inc | 40 +++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/tcg/i386/tcg-target.c.inc b/tcg/i386/tcg-target.c.inc
index 540debdf34..4feb7e2aa1 100644
--- a/tcg/i386/tcg-target.c.inc
+++ b/tcg/i386/tcg-target.c.inc
@@ -132,6 +132,22 @@ static const int tcg_target_call_oarg_regs[] = {
 # define TCG_REG_L1 TCG_REG_EDX
 #endif
 
+#define ALL_BYTEH_REGS         0x0000000fu
+#if TCG_TARGET_REG_BITS == 64
+# define ALL_GENERAL_REGS      0x0000ffffu
+# define ALL_VECTOR_REGS       0xffff0000u
+# define ALL_BYTEL_REGS        ALL_GENERAL_REGS
+#else
+# define ALL_GENERAL_REGS      0x000000ffu
+# define ALL_VECTOR_REGS       0x00ff0000u
+# define ALL_BYTEL_REGS        ALL_BYTEH_REGS
+#endif
+#ifdef CONFIG_SOFTMMU
+# define SOFTMMU_RESERVE_REGS  ((1 << TCG_REG_L0) | (1 << TCG_REG_L1))
+#else
+# define SOFTMMU_RESERVE_REGS  0
+#endif
+
 /* The host compiler should supply <cpuid.h> to enable runtime features
    detection, as we're not going to go so far as our own inline assembly.
    If not available, default values will be assumed.  */
@@ -193,14 +209,6 @@ static bool patch_reloc(tcg_insn_unit *code_ptr, int type,
     return true;
 }
 
-#if TCG_TARGET_REG_BITS == 64
-#define ALL_GENERAL_REGS   0x0000ffffu
-#define ALL_VECTOR_REGS    0xffff0000u
-#else
-#define ALL_GENERAL_REGS   0x000000ffu
-#define ALL_VECTOR_REGS    0x00ff0000u
-#endif
-
 /* parse target specific constraints */
 static const char *target_parse_constraint(TCGArgConstraint *ct,
                                            const char *ct_str, TCGType type)
@@ -226,11 +234,11 @@ static const char *target_parse_constraint(TCGArgConstraint *ct,
         break;
     case 'q':
         /* A register that can be used as a byte operand.  */
-        ct->regs = TCG_TARGET_REG_BITS == 64 ? 0xffff : 0xf;
+        ct->regs |= ALL_BYTEL_REGS;
         break;
     case 'Q':
         /* A register with an addressable second byte (e.g. %ah).  */
-        ct->regs = 0xf;
+        ct->regs |= ALL_BYTEH_REGS;
         break;
     case 'r':
         /* A general register.  */
@@ -247,19 +255,11 @@ static const char *target_parse_constraint(TCGArgConstraint *ct,
 
     case 'L':
         /* qemu_ld/st data+address constraint */
-        ct->regs = TCG_TARGET_REG_BITS == 64 ? 0xffff : 0xff;
-#ifdef CONFIG_SOFTMMU
-        tcg_regset_reset_reg(ct->regs, TCG_REG_L0);
-        tcg_regset_reset_reg(ct->regs, TCG_REG_L1);
-#endif
+        ct->regs |= ALL_GENERAL_REGS & ~SOFTMMU_RESERVE_REGS;
         break;
     case 's':
         /* qemu_st8_i32 data constraint */
-        ct->regs = 0xf;
-#ifdef CONFIG_SOFTMMU
-        tcg_regset_reset_reg(ct->regs, TCG_REG_L0);
-        tcg_regset_reset_reg(ct->regs, TCG_REG_L1);
-#endif
+        ct->regs |= ALL_BYTEL_REGS & ~SOFTMMU_RESERVE_REGS;
         break;
 
     case 'e':
-- 
2.25.1

Re: [PATCH v3 04/24] tcg/i386: Tidy register constraint definitions

Posted by Peter Maydell 3 years, 9 months ago

On Fri, 29 Jan 2021 at 20:14, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Create symbolic constants for all low-byte-addressable
> and second-byte-addressable registers.  Create a symbol
> for the registers that need reserving for softmmu.
>
> There is no functional change for 's', as this letter is
> only used for i386.  The BYTEL name is correct for the
> action we wish from the constraint.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tcg/i386/tcg-target.c.inc | 40 +++++++++++++++++++--------------------
>  1 file changed, 20 insertions(+), 20 deletions(-)
>
> @@ -226,11 +234,11 @@ static const char *target_parse_constraint(TCGArgConstraint *ct,
>          break;
>      case 'q':
>          /* A register that can be used as a byte operand.  */
> -        ct->regs = TCG_TARGET_REG_BITS == 64 ? 0xffff : 0xf;
> +        ct->regs |= ALL_BYTEL_REGS;
>          break;
>      case 'Q':
>          /* A register with an addressable second byte (e.g. %ah).  */
> -        ct->regs = 0xf;
> +        ct->regs |= ALL_BYTEH_REGS;
>          break;
>      case 'r':
>          /* A general register.  */
> @@ -247,19 +255,11 @@ static const char *target_parse_constraint(TCGArgConstraint *ct,
>
>      case 'L':
>          /* qemu_ld/st data+address constraint */
> -        ct->regs = TCG_TARGET_REG_BITS == 64 ? 0xffff : 0xff;
> -#ifdef CONFIG_SOFTMMU
> -        tcg_regset_reset_reg(ct->regs, TCG_REG_L0);
> -        tcg_regset_reset_reg(ct->regs, TCG_REG_L1);
> -#endif
> +        ct->regs |= ALL_GENERAL_REGS & ~SOFTMMU_RESERVE_REGS;
>          break;
>      case 's':
>          /* qemu_st8_i32 data constraint */
> -        ct->regs = 0xf;
> -#ifdef CONFIG_SOFTMMU
> -        tcg_regset_reset_reg(ct->regs, TCG_REG_L0);
> -        tcg_regset_reset_reg(ct->regs, TCG_REG_L1);
> -#endif
> +        ct->regs |= ALL_BYTEL_REGS & ~SOFTMMU_RESERVE_REGS;
>          break;

Should these cases really be ORing in these expressions
rather than just using '=' the way the old code was?

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH v3 04/24] tcg/i386: Tidy register constraint definitions

Posted by Richard Henderson 3 years, 9 months ago

On 1/29/21 1:20 PM, Peter Maydell wrote:
> On Fri, 29 Jan 2021 at 20:14, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Create symbolic constants for all low-byte-addressable
>> and second-byte-addressable registers.  Create a symbol
>> for the registers that need reserving for softmmu.
>>
>> There is no functional change for 's', as this letter is
>> only used for i386.  The BYTEL name is correct for the
>> action we wish from the constraint.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  tcg/i386/tcg-target.c.inc | 40 +++++++++++++++++++--------------------
>>  1 file changed, 20 insertions(+), 20 deletions(-)
>>
>> @@ -226,11 +234,11 @@ static const char *target_parse_constraint(TCGArgConstraint *ct,
>>          break;
>>      case 'q':
>>          /* A register that can be used as a byte operand.  */
>> -        ct->regs = TCG_TARGET_REG_BITS == 64 ? 0xffff : 0xf;
>> +        ct->regs |= ALL_BYTEL_REGS;
>>          break;
>>      case 'Q':
>>          /* A register with an addressable second byte (e.g. %ah).  */
>> -        ct->regs = 0xf;
>> +        ct->regs |= ALL_BYTEH_REGS;
>>          break;
>>      case 'r':
>>          /* A general register.  */
>> @@ -247,19 +255,11 @@ static const char *target_parse_constraint(TCGArgConstraint *ct,
>>
>>      case 'L':
>>          /* qemu_ld/st data+address constraint */
>> -        ct->regs = TCG_TARGET_REG_BITS == 64 ? 0xffff : 0xff;
>> -#ifdef CONFIG_SOFTMMU
>> -        tcg_regset_reset_reg(ct->regs, TCG_REG_L0);
>> -        tcg_regset_reset_reg(ct->regs, TCG_REG_L1);
>> -#endif
>> +        ct->regs |= ALL_GENERAL_REGS & ~SOFTMMU_RESERVE_REGS;
>>          break;
>>      case 's':
>>          /* qemu_st8_i32 data constraint */
>> -        ct->regs = 0xf;
>> -#ifdef CONFIG_SOFTMMU
>> -        tcg_regset_reset_reg(ct->regs, TCG_REG_L0);
>> -        tcg_regset_reset_reg(ct->regs, TCG_REG_L1);
>> -#endif
>> +        ct->regs |= ALL_BYTEL_REGS & ~SOFTMMU_RESERVE_REGS;
>>          break;
> 
> Should these cases really be ORing in these expressions
> rather than just using '=' the way the old code was?
> 
> Otherwise
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

All of the cases should always have been ORd.
In theory, one can combine register constraints,
just like one can combine constant constraints.
Not that it would really make sense for this
specific case.

r~