[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH v3 02/46] target/i386: Push rex_w into Disas
From: |
Aleksandar Markovic |
Subject: |
Re: [Qemu-devel] [RFC PATCH v3 02/46] target/i386: Push rex_w into DisasContext |
Date: |
Thu, 15 Aug 2019 12:19:42 +0200 |
15.08.2019. 11.55, "Richard Henderson" <address@hidden> је
написао/ла:
>
> On 8/15/19 8:30 AM, Aleksandar Markovic wrote:
> >
> > 15.08.2019. 04.13, "Jan Bobek" <address@hidden
> > <mailto:address@hidden>> је написао/ла:
> >>
> >> From: Richard Henderson <address@hidden <mailto:address@hidden>>
> >>
> >> Treat this the same as we already do for other rex bits.
> >>
> >> Signed-off-by: Richard Henderson <address@hidden <mailto:
address@hidden>>
> >> ---
> >> target/i386/translate.c | 19 +++++++++++--------
> >> 1 file changed, 11 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/target/i386/translate.c b/target/i386/translate.c
> >> index d74dbfd585..c0866c2797 100644
> >> --- a/target/i386/translate.c
> >> +++ b/target/i386/translate.c
> >> @@ -44,11 +44,13 @@
> >> #define REX_X(s) ((s)->rex_x)
> >> #define REX_B(s) ((s)->rex_b)
> >> #define REX_R(s) ((s)->rex_r)
> >> +#define REX_W(s) ((s)->rex_w)
> >> #else
> >> #define CODE64(s) 0
> >> #define REX_X(s) 0
> >> #define REX_B(s) 0
> >> #define REX_R(s) 0
> >> +#define REX_W(s) -1
> >
> > The commit message says "treat rex_w the same as other rex bits". Why
is then
> > REX_W() treated differently here?
>
> "Treated the same" in terms of being referenced by a macro instead of a
local
> variable. As for the -1, if you look at the rest of the patch you can
clearly
> see it preserves existing behaviour.
>
That is exactly what I dislike about your commit messages: they often
introduce ambiguity, without any real need, and with really bad
consequences to the reader. Is adding "in terms of being referenced by a
macro instead of a local
variable" to the commit message that hard?
When writing commit messages, you need to try to put yourself in the shoes
of the reader.
Aleksandar
> >> @@ -4503,6 +4504,7 @@ static target_ulong disas_insn(DisasContext *s,
> > CPUState *cpu)
> >> s->rex_x = 0;
> >> s->rex_b = 0;
> >> s->rex_r = 0;
> >> + s->rex_w = -1;
> >> s->x86_64_hregs = false;
> >> #endif
> >> s->rip_offset = 0; /* for relative ip address */
> >> @@ -4514,7 +4516,6 @@ static target_ulong disas_insn(DisasContext *s,
> > CPUState *cpu)
> >> }
> >>
> >> prefixes = 0;
> >> - rex_w = -1;
>
>
> r~
[Qemu-devel] [RFC PATCH v3 03/46] target/i386: reduce scope of variable aflag, Jan Bobek, 2019/08/14
[Qemu-devel] [RFC PATCH v3 01/46] target/i386: Push rex_r into DisasContext, Jan Bobek, 2019/08/14
[Qemu-devel] [RFC PATCH v3 08/46] target/i386: make variable b1 const, Jan Bobek, 2019/08/14
[Qemu-devel] [RFC PATCH v3 09/46] target/i386: make variable is_xmm const, Jan Bobek, 2019/08/14
[Qemu-devel] [RFC PATCH v3 10/46] target/i386: add vector register file alignment constraints, Jan Bobek, 2019/08/14
[Qemu-devel] [RFC PATCH v3 05/46] target/i386: use prefix from DisasContext, Jan Bobek, 2019/08/14
[Qemu-devel] [RFC PATCH v3 04/46] target/i386: use dflag from DisasContext, Jan Bobek, 2019/08/14
[Qemu-devel] [RFC PATCH v3 11/46] target/i386: introduce gen_(ld, st)d_env_A0, Jan Bobek, 2019/08/14