Re: [PATCH 1/3] tcg/i386: Drop BYTEH deposits for 64-bit

[Peter Maydell]

On Wed, 16 Aug 2023 at 16:01, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> It is more useful to allow low-part deposits into all registers
> than to restrict allocation for high-byte deposits.

>  #define TCG_TARGET_deposit_i32_valid(ofs, len) \
> -    (((ofs) == 0 && (len) == 8) || ((ofs) == 8 && (len) == 8) || \
> -     ((ofs) == 0 && (len) == 16))
> +    (((ofs) == 0 && ((len) == 8 || (len) == 16)) || \
> +     (TCG_TARGET_REG_BITS == 32 && (ofs) == 8 && (len) == 8))
>  #define TCG_TARGET_deposit_i64_valid    TCG_TARGET_deposit_i32_valid


> @@ -2752,7 +2751,7 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode 
> opc,
>          if (args[3] == 0 && args[4] == 8) {
>              /* load bits 0..7 */
>              tcg_out_modrm(s, OPC_MOVB_EvGv | P_REXB_R | P_REXB_RM, a2, a0);
> -        } else if (args[3] == 8 && args[4] == 8) {
> +        } else if (TCG_TARGET_REG_BITS == 32 && args[3] == 8 && args[4] == 
> 8) {

Should we assert(TCG_TARGET_REG_BITS == 32) rather than making it part of the
condition? If I understand the change to the deposit_i32_valid macro above, we
should never get here with 8, 8 if TCG_TARGET_REG_BITS is 64.

>              /* load bits 8..15 */
>              tcg_out_modrm(s, OPC_MOVB_EvGv, a2, a0 + 4);
>          } else if (args[3] == 0 && args[4] == 16) {

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

thanks
-- PMM