On 1/8/26 00:44, Richard Henderson wrote:
On 1/8/26 02:14, Paolo Bonzini wrote:
Unlike the older code in translate.c, mod=11b *is* filtered out earlier
by decode_modrm.
Signed-off-by: Paolo Bonzini <[email protected]>
---
target/i386/tcg/decode-new.c.inc | 7 -------
1 file changed, 7 deletions(-)
diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/
decode-new.c.inc
index 243df7e3735..7b595607fa7 100644
--- a/target/i386/tcg/decode-new.c.inc
+++ b/target/i386/tcg/decode-new.c.inc
@@ -2024,12 +2024,6 @@ static AddressParts
decode_modrm_address(CPUX86State *env, DisasContext *s,
rm = modrm & 7;
base = rm | REX_B(s);
- if (mod == 3) {
- /* Normally filtered out earlier, but including this path
- simplifies multi-byte nop, as well as bndcl, bndcu,
bndcn. */
- goto done;
- }
-
I can see that this is true, but one has to dig around to see that it's so.
There's enough prep code duplicated between decode_modrm and
decode_modrm_address that I think it would be worthwhile to merge the
two functions and simplify.
Is there really so much? I guess you could write the combined function like so:
int modrm = get_modrm(s, env);
int mod = (s->modrm >> 6) & 3;
int rm = s->modrm & 7;
if (mod == 3) {
op->n = (rm | REX_B(s)) & reg_nb_mask(s, op->unit);
return;
}
int def_seg = R_DS;
int base = rm | REX_B(s);
int index = -1;
int scale = 0;
targe_long disp = 0;
bool is_vsib = decode->e.vex_class == 12;
switch(s->aflag) {
...
}
op->has_ea = true;
op->n = -1;
decode->mem = (AddressParts){ def_seg, base, index, scale, disp };
but it does not seem so much better... I guess I could do the modrm
split just once:
diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/decode-new.c.inc
index 41012659104..23cae451eb7 100644
--- a/target/i386/tcg/decode-new.c.inc
+++ b/target/i386/tcg/decode-new.c.inc
@@ -2247,21 +2247,19 @@
/* Decompose an address. */
static AddressParts decode_modrm_address(CPUX86State *env, DisasContext *s,
- int modrm, bool is_vsib)
+ int mod, int rm, bool is_vsib)
{
- int def_seg, base, index, scale, mod, rm;
+ int def_seg, base, index, scale;
target_long disp;
bool havesib;
+ assert(mod == 3);
def_seg = R_DS;
+ base = rm | REX_B(s);
index = -1;
scale = 0;
disp = 0;
- mod = (modrm >> 6) & 3;
- rm = modrm & 7;
- base = rm | REX_B(s);
-
switch (s->aflag) {
case MO_64:
case MO_32:
@@ -2379,16 +2376,18 @@
static int decode_modrm(DisasContext *s, CPUX86State *env,
X86DecodedInsn *decode, X86DecodedOp *op)
{
- int modrm = s->modrm;
- if ((modrm >> 6) == 3) {
- op->n = (modrm & 7);
+ int modrm = get_modrm(s, env);
+ int mod = (s->modrm >> 6) & 3;
+ int rm = s->modrm & 7;
+ if (mod == 3) {
+ op->n = rm;
if (op->unit != X86_OP_MMX) {
op->n |= REX_B(s);
}
} else {
op->has_ea = true;
op->n = -1;
- decode->mem = decode_modrm_address(env, s, get_modrm(s, env),
+ decode->mem = decode_modrm_address(env, s, mod, rm,
decode->e.vex_class == 12);
}
return modrm;
Also, not do things like
switch (mod) {
...
default:
case 2:
where the default isn't reachable.
I agree and was curious to when it dated... and it's March 2003 :) (commit 4b74fe1f001,
"many fixes", which brought in the very first implementation of 16-bit
addressing modes):
+ default:
+ case 7:
+ gen_op_movl_A0_reg[R_EBX]();
+ break;
Paolo
