On 4/9/24 06:43, Paolo Bonzini wrote:
diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index 05a1912f8a3..88653c4f824 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -105,6 +105,12 @@ typedef uint64_t TCGRegSet;
/* Turn some undef macros into true macros. */
#define TCG_TARGET_HAS_add2_i32 1
#define TCG_TARGET_HAS_sub2_i32 1
+/* Define parameterized _tl macros. */
+#define TCG_TARGET_deposit_tl_valid TCG_TARGET_deposit_i32_valid
+#define TCG_TARGET_extract_tl_valid TCG_TARGET_extract_i32_valid
+#else
+#define TCG_TARGET_deposit_tl_valid TCG_TARGET_deposit_i64_valid
+#define TCG_TARGET_extract_tl_valid TCG_TARGET_extract_i64_valid
#endif
So far we have been localizing these to emit.c.inc.
In general I'm not sure how I feel about them. It would be cleaner not to expose tcg
backend details at all, but there are several points where we can legitimately produce
better code knowing what the backend has, without having to have a strong optimizer.
+static void decode_group2(DisasContext *s, CPUX86State *env, X86OpEntry
*entry, uint8_t *b)
+{
+ static const X86GenFunc group2_gen[8] = {
+ gen_ROL, gen_ROR, gen_RCL, gen_RCR, gen_SHL, gen_SHR, gen_SHL, gen_SAR,
+ };
I think you need to keep a comment for /6 (currently OP_SHL1 /* undocumented */; I presume
this to be the original SAL opcode, before some prehistoric documentation changed it to
share /4 with SHL).
+static const X86OpEntry opcodes_grp3[16] = {
+ /* 0xf6 */
+ [0x00] = X86_OP_ENTRYrr(AND, E,b, I,b),
+ [0x02] = X86_OP_ENTRY1(NOT, E,b, lock),
+ [0x03] = X86_OP_ENTRY1(NEG, E,b, lock),
+ [0x04] = X86_OP_ENTRYrr(MUL, E,b, 0,b, zextT0),
+ [0x05] = X86_OP_ENTRYrr(IMUL,E,b, 0,b, sextT0),
+ [0x06] = X86_OP_ENTRYr(DIV, E,b),
+ [0x07] = X86_OP_ENTRYr(IDIV, E,b),
+
+ /* 0xf7 */
+ [0x08] = X86_OP_ENTRYrr(AND, E,v, I,z),
+ [0x0a] = X86_OP_ENTRY1(NOT, E,v, lock),
+ [0x0b] = X86_OP_ENTRY1(NEG, E,v, lock),
+ [0x0c] = X86_OP_ENTRYrr(MUL, E,v, 0,v, zextT0),
+ [0x0d] = X86_OP_ENTRYrr(IMUL,E,v, 0,v, sextT0),
+ [0x0e] = X86_OP_ENTRYr(DIV, E,v),
+ [0x0f] = X86_OP_ENTRYr(IDIV, E,v),
+};
+
+static void decode_group3(DisasContext *s, CPUX86State *env, X86OpEntry
*entry, uint8_t *b)
+{
+ int w = (*b & 1);
+ int reg = (get_modrm(s, env) >> 3) & 7;
+
+ *entry = opcodes_grp3[(w << 3) | reg];
+}
+
+static const X86OpEntry opcodes_grp4[16] = {
+ /* 0xfe */
+ [0x00] = X86_OP_ENTRY1(INC, E,b, lock),
+ [0x01] = X86_OP_ENTRY1(DEC, E,b, lock),
+
+ /* 0xff */
+ [0x08] = X86_OP_ENTRY1(INC, E,v, lock),
+ [0x09] = X86_OP_ENTRY1(DEC, E,v, lock),
+ [0x0a] = X86_OP_ENTRY3(CALL_m, None, None, E,f64, None, None, zextT0),
+ [0x0b] = X86_OP_ENTRYr(CALLF_m, M,p),
+ [0x0c] = X86_OP_ENTRY3(JMP_m, None, None, E,f64, None, None, zextT0),
+ [0x0d] = X86_OP_ENTRYr(JMPF_m, M,p),
+ [0x0e] = X86_OP_ENTRYr(PUSH, E,f64),
+};
+
+static void decode_group4(DisasContext *s, CPUX86State *env, X86OpEntry
*entry, uint8_t *b)
+{
+ int w = (*b & 1);
+ int reg = (get_modrm(s, env) >> 3) & 7;
+
+ *entry = opcodes_grp4[(w << 3) | reg];
+}
Did these tables need to be outside their functions?
Though this works, 0xff is named grp5.
+ [0xF1] = X86_OP_ENTRY0(INT1, svm(ICEBP)),
+ [0xF4] = X86_OP_ENTRY0(HLT, chk(cpl0)),
+ [0xF5] = X86_OP_ENTRY0(CMC),
+ [0xF6] = X86_OP_GROUP1(group3, E,b),
+ [0xF7] = X86_OP_GROUP1(group3, E,v),
Not adding spacers as you were above?
+static void gen_RCL(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode)
+{
+ bool have_1bit_cin, can_be_zero;
+ TCGv count;
+ TCGLabel *zero_label = NULL;
+ MemOp ot = gen_shift_count(s, decode, &can_be_zero, &count);
+ TCGv low = tcg_temp_new();
+ TCGv high = tcg_temp_new();
+ TCGv low_count = tcg_temp_new();
+
+ if (!count) {
+ return;
+ }
+
Delay all temp allocation until after the early return.
+static void gen_rot_carry(X86DecodedInsn *decode, TCGv result, TCGv count, int
bit)
+{
+ TCGv temp = count ? tcg_temp_new() : decode->cc_dst;
+
+ tcg_gen_setcondi_tl(TCG_COND_TSTNE, temp, result, 1ULL << bit);
tcg_gen_extract_tl.
It might be easier to read with only one if:
if (count == NULL) {
extract
} else {
temp = new
extract
movcond
}
r~
