Message ID | 1369855851-21400-1-git-send-email-rth@twiddle.net |
---|---|
State | New |
Headers | show |
Ping. On 05/29/2013 12:30 PM, Richard Henderson wrote: > The code reorganization in commit 4a6fd938 broke handling of PREFIX_ADR. > While fixing this, tidy and comment the code so that it's more obvious > what's going on in setting both aflag and dflag. > > The TARGET_X86_64 ifdef can be eliminated because CODE64 expands to the > constant zero when TARGET_X86_64 is undefined. > > Cc: Paolo Bonzini <pbonzini@redhat.com> > Reported-by: Laszlo Ersek <lersek@redhat.com> > Signed-off-by: Richard Henderson <rth@twiddle.net> > --- > target-i386/translate.c | 30 +++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/target-i386/translate.c b/target-i386/translate.c > index 0aeccdb..14b0298 100644 > --- a/target-i386/translate.c > +++ b/target-i386/translate.c > @@ -4677,8 +4677,6 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, > } > s->pc = pc_start; > prefixes = 0; > - aflag = s->code32; > - dflag = s->code32; > s->override = -1; > rex_w = -1; > rex_r = 0; > @@ -4801,23 +4799,25 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, > } > > /* Post-process prefixes. */ > - if (prefixes & PREFIX_DATA) { > - dflag ^= 1; > - } > - if (prefixes & PREFIX_ADR) { > - aflag ^= 1; > - } > -#ifdef TARGET_X86_64 > if (CODE64(s)) { > - if (rex_w == 1) { > - /* 0x66 is ignored if rex.w is set */ > - dflag = 2; > + /* In 64-bit mode, the default data size is 32-bit. Select 64-bit > + data with rex_w, and 16-bit data with 0x66; rex_w takes precedence > + over 0x66 if both are present. */ > + dflag = (rex_w > 0 ? 2 : prefixes & PREFIX_DATA ? 0 : 1); > + /* In 64-bit mode, 0x67 selects 32-bit addressing. */ > + aflag = (prefixes & PREFIX_ADR ? 1 : 2); > + } else { > + /* In 16/32-bit mode, 0x66 selects the opposite data size. */ > + dflag = s->code32; > + if (prefixes & PREFIX_DATA) { > + dflag ^= 1; > } > - if (!(prefixes & PREFIX_ADR)) { > - aflag = 2; > + /* In 16/32-bit mode, 0x67 selects the opposite addressing. */ > + aflag = s->code32; > + if (prefixes & PREFIX_ADR) { > + aflag ^= 1; > } > } > -#endif > > s->prefix = prefixes; > s->aflag = aflag; >
Il 31/05/2013 17:03, Richard Henderson ha scritto: > Ping. > > On 05/29/2013 12:30 PM, Richard Henderson wrote: >> The code reorganization in commit 4a6fd938 broke handling of PREFIX_ADR. >> While fixing this, tidy and comment the code so that it's more obvious >> what's going on in setting both aflag and dflag. >> >> The TARGET_X86_64 ifdef can be eliminated because CODE64 expands to the >> constant zero when TARGET_X86_64 is undefined. >> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Reported-by: Laszlo Ersek <lersek@redhat.com> >> Signed-off-by: Richard Henderson <rth@twiddle.net> >> --- >> target-i386/translate.c | 30 +++++++++++++++--------------- >> 1 file changed, 15 insertions(+), 15 deletions(-) >> >> diff --git a/target-i386/translate.c b/target-i386/translate.c >> index 0aeccdb..14b0298 100644 >> --- a/target-i386/translate.c >> +++ b/target-i386/translate.c >> @@ -4677,8 +4677,6 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, >> } >> s->pc = pc_start; >> prefixes = 0; >> - aflag = s->code32; >> - dflag = s->code32; >> s->override = -1; >> rex_w = -1; >> rex_r = 0; >> @@ -4801,23 +4799,25 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, >> } >> >> /* Post-process prefixes. */ >> - if (prefixes & PREFIX_DATA) { >> - dflag ^= 1; >> - } >> - if (prefixes & PREFIX_ADR) { >> - aflag ^= 1; >> - } >> -#ifdef TARGET_X86_64 >> if (CODE64(s)) { >> - if (rex_w == 1) { >> - /* 0x66 is ignored if rex.w is set */ >> - dflag = 2; >> + /* In 64-bit mode, the default data size is 32-bit. Select 64-bit >> + data with rex_w, and 16-bit data with 0x66; rex_w takes precedence >> + over 0x66 if both are present. */ >> + dflag = (rex_w > 0 ? 2 : prefixes & PREFIX_DATA ? 0 : 1); >> + /* In 64-bit mode, 0x67 selects 32-bit addressing. */ >> + aflag = (prefixes & PREFIX_ADR ? 1 : 2); >> + } else { >> + /* In 16/32-bit mode, 0x66 selects the opposite data size. */ >> + dflag = s->code32; >> + if (prefixes & PREFIX_DATA) { >> + dflag ^= 1; >> } >> - if (!(prefixes & PREFIX_ADR)) { >> - aflag = 2; >> + /* In 16/32-bit mode, 0x67 selects the opposite addressing. */ >> + aflag = s->code32; >> + if (prefixes & PREFIX_ADR) { >> + aflag ^= 1; >> } Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Cc: qemu-stable@nongnu.org >> } >> -#endif >> >> s->prefix = prefixes; >> s->aflag = aflag; >> >
On 05/29/13 21:30, Richard Henderson wrote: > The code reorganization in commit 4a6fd938 broke handling of PREFIX_ADR. > While fixing this, tidy and comment the code so that it's more obvious > what's going on in setting both aflag and dflag. > > The TARGET_X86_64 ifdef can be eliminated because CODE64 expands to the > constant zero when TARGET_X86_64 is undefined. > > Cc: Paolo Bonzini <pbonzini@redhat.com> > Reported-by: Laszlo Ersek <lersek@redhat.com> > Signed-off-by: Richard Henderson <rth@twiddle.net> > --- > target-i386/translate.c | 30 +++++++++++++++--------------- > 1 file changed, 15 insertions(+), 15 deletions(-) > > diff --git a/target-i386/translate.c b/target-i386/translate.c > index 0aeccdb..14b0298 100644 > --- a/target-i386/translate.c > +++ b/target-i386/translate.c > @@ -4677,8 +4677,6 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, > } > s->pc = pc_start; > prefixes = 0; > - aflag = s->code32; > - dflag = s->code32; > s->override = -1; > rex_w = -1; > rex_r = 0; > @@ -4801,23 +4799,25 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, > } > > /* Post-process prefixes. */ > - if (prefixes & PREFIX_DATA) { > - dflag ^= 1; > - } > - if (prefixes & PREFIX_ADR) { > - aflag ^= 1; > - } > -#ifdef TARGET_X86_64 > if (CODE64(s)) { > - if (rex_w == 1) { > - /* 0x66 is ignored if rex.w is set */ > - dflag = 2; > + /* In 64-bit mode, the default data size is 32-bit. Select 64-bit > + data with rex_w, and 16-bit data with 0x66; rex_w takes precedence > + over 0x66 if both are present. */ > + dflag = (rex_w > 0 ? 2 : prefixes & PREFIX_DATA ? 0 : 1); > + /* In 64-bit mode, 0x67 selects 32-bit addressing. */ > + aflag = (prefixes & PREFIX_ADR ? 1 : 2); > + } else { > + /* In 16/32-bit mode, 0x66 selects the opposite data size. */ > + dflag = s->code32; > + if (prefixes & PREFIX_DATA) { > + dflag ^= 1; > } > - if (!(prefixes & PREFIX_ADR)) { > - aflag = 2; > + /* In 16/32-bit mode, 0x67 selects the opposite addressing. */ > + aflag = s->code32; > + if (prefixes & PREFIX_ADR) { > + aflag ^= 1; > } > } > -#endif > > s->prefix = prefixes; > s->aflag = aflag; > Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Applied. Thanks. Regards, Anthony Liguori
diff --git a/target-i386/translate.c b/target-i386/translate.c index 0aeccdb..14b0298 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -4677,8 +4677,6 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, } s->pc = pc_start; prefixes = 0; - aflag = s->code32; - dflag = s->code32; s->override = -1; rex_w = -1; rex_r = 0; @@ -4801,23 +4799,25 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, } /* Post-process prefixes. */ - if (prefixes & PREFIX_DATA) { - dflag ^= 1; - } - if (prefixes & PREFIX_ADR) { - aflag ^= 1; - } -#ifdef TARGET_X86_64 if (CODE64(s)) { - if (rex_w == 1) { - /* 0x66 is ignored if rex.w is set */ - dflag = 2; + /* In 64-bit mode, the default data size is 32-bit. Select 64-bit + data with rex_w, and 16-bit data with 0x66; rex_w takes precedence + over 0x66 if both are present. */ + dflag = (rex_w > 0 ? 2 : prefixes & PREFIX_DATA ? 0 : 1); + /* In 64-bit mode, 0x67 selects 32-bit addressing. */ + aflag = (prefixes & PREFIX_ADR ? 1 : 2); + } else { + /* In 16/32-bit mode, 0x66 selects the opposite data size. */ + dflag = s->code32; + if (prefixes & PREFIX_DATA) { + dflag ^= 1; } - if (!(prefixes & PREFIX_ADR)) { - aflag = 2; + /* In 16/32-bit mode, 0x67 selects the opposite addressing. */ + aflag = s->code32; + if (prefixes & PREFIX_ADR) { + aflag ^= 1; } } -#endif s->prefix = prefixes; s->aflag = aflag;
The code reorganization in commit 4a6fd938 broke handling of PREFIX_ADR. While fixing this, tidy and comment the code so that it's more obvious what's going on in setting both aflag and dflag. The TARGET_X86_64 ifdef can be eliminated because CODE64 expands to the constant zero when TARGET_X86_64 is undefined. Cc: Paolo Bonzini <pbonzini@redhat.com> Reported-by: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Richard Henderson <rth@twiddle.net> --- target-i386/translate.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-)