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

[Richard Henderson <richard.henderson@linaro.org>]

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~