| Message ID | 20230816145547.477974-2-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tcg/i386: Improvements to deposit | expand |
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
On 8/17/23 08:44, Peter Maydell wrote: > 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? The if/else chain ends in g_assert_not_reached(). > 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. Correct. r~
diff --git a/tcg/i386/tcg-target-con-set.h b/tcg/i386/tcg-target-con-set.h index 5ea3a292f0..3949d49538 100644 --- a/tcg/i386/tcg-target-con-set.h +++ b/tcg/i386/tcg-target-con-set.h @@ -33,7 +33,7 @@ C_O1_I1(r, q) C_O1_I1(r, r) C_O1_I1(x, r) C_O1_I1(x, x) -C_O1_I2(Q, 0, Q) +C_O1_I2(q, 0, q) C_O1_I2(q, r, re) C_O1_I2(r, 0, ci) C_O1_I2(r, 0, r) diff --git a/tcg/i386/tcg-target-con-str.h b/tcg/i386/tcg-target-con-str.h index 24e6bcb80d..95a30e58cd 100644 --- a/tcg/i386/tcg-target-con-str.h +++ b/tcg/i386/tcg-target-con-str.h @@ -19,7 +19,6 @@ REGS('D', 1u << TCG_REG_EDI) REGS('r', ALL_GENERAL_REGS) REGS('x', ALL_VECTOR_REGS) REGS('q', ALL_BYTEL_REGS) /* regs that can be used as a byte operand */ -REGS('Q', ALL_BYTEH_REGS) /* regs with a second byte (e.g. %ah) */ REGS('L', ALL_GENERAL_REGS & ~SOFTMMU_RESERVE_REGS) /* qemu_ld/st */ REGS('s', ALL_BYTEL_REGS & ~SOFTMMU_RESERVE_REGS) /* qemu_st8_i32 data */ diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h index 2a2e3fffa8..30cce01ca4 100644 --- a/tcg/i386/tcg-target.h +++ b/tcg/i386/tcg-target.h @@ -227,8 +227,8 @@ typedef enum { #define TCG_TARGET_HAS_cmpsel_vec -1 #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 /* Check for the possibility of high-byte extraction and, for 64-bit, diff --git a/tcg/i386/tcg-target.c.inc b/tcg/i386/tcg-target.c.inc index a6b2eae995..ba40dd0f4d 100644 --- a/tcg/i386/tcg-target.c.inc +++ b/tcg/i386/tcg-target.c.inc @@ -144,7 +144,6 @@ static TCGReg tcg_target_call_oarg_reg(TCGCallReturnKind kind, int slot) # 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 @@ -152,7 +151,7 @@ static TCGReg tcg_target_call_oarg_reg(TCGCallReturnKind kind, int slot) #else # define ALL_GENERAL_REGS 0x000000ffu # define ALL_VECTOR_REGS 0x00ff0000u -# define ALL_BYTEL_REGS ALL_BYTEH_REGS +# define ALL_BYTEL_REGS 0x0000000fu #endif #ifdef CONFIG_SOFTMMU # define SOFTMMU_RESERVE_REGS ((1 << TCG_REG_L0) | (1 << TCG_REG_L1)) @@ -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) { /* load bits 8..15 */ tcg_out_modrm(s, OPC_MOVB_EvGv, a2, a0 + 4); } else if (args[3] == 0 && args[4] == 16) { @@ -3312,7 +3311,7 @@ static TCGConstraintSetIndex tcg_target_op_def(TCGOpcode op) case INDEX_op_deposit_i32: case INDEX_op_deposit_i64: - return C_O1_I2(Q, 0, Q); + return C_O1_I2(q, 0, q); case INDEX_op_setcond_i32: case INDEX_op_setcond_i64:
It is more useful to allow low-part deposits into all registers than to restrict allocation for high-byte deposits. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- tcg/i386/tcg-target-con-set.h | 2 +- tcg/i386/tcg-target-con-str.h | 1 - tcg/i386/tcg-target.h | 4 ++-- tcg/i386/tcg-target.c.inc | 7 +++---- 4 files changed, 6 insertions(+), 8 deletions(-)