Re: [Qemu-devel] [RFC PATCH v3 02/46] target/i386: Push rex_w into DisasContext

[Jan Bobek]

On 8/15/19 6:19 AM, Aleksandar Markovic wrote:
> 
> 15.08.2019. 11.55, "Richard Henderson" <address@hidden 
> <mailto:address@hidden>> је написао/ла:
>>
>> On 8/15/19 8:30 AM, Aleksandar Markovic wrote:
>> >
>> > 15.08.2019. 04.13, "Jan Bobek" <address@hidden <mailto:address@hidden>
>> > <mailto:address@hidden <mailto:address@hidden>>> је написао/ла:
>> >>
>> >> From: Richard Henderson <address@hidden <mailto:address@hidden> 
>> >> <mailto: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> 
>> >> <mailto: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.

FWIW, personally I don't find it confusing. I think even just the
first couple of lines of the patch make it quite clear what's
going on. Just my 2 cents.

-Jan

signature.asc
Description: OpenPGP digital signature