[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 5/8] target/mips: R5900 DMULT[U], DDIV[U], LL

From: Philippe Mathieu-Daudé
Subject: Re: [Qemu-devel] [PATCH v4 5/8] target/mips: R5900 DMULT[U], DDIV[U], LL, SC, LLD and SCD are user only
Date: Wed, 19 Sep 2018 11:47:37 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0

On 9/18/18 8:26 PM, Maciej W. Rozycki wrote:
> Hi Fredrik,
>> I agree, that is important too. I will post an updated v5 soon. Another
>> alternative change is to define check_insn_opc_user_only as
>> static inline void check_insn_opc_user_only(DisasContext *ctx, int flags)
>> {
>>     check_insn_opc_removed(ctx, flags);
>> #endif
>> }
>> by referring to check_insn_opc_removed (instead of copying its definition).
>> Would you consider this an improvement for v5 too?
>  Yes, it does look like an improvement to me, reducing code duplication.  
> Thanks for looking into it further.

Yes, also check_insn_opc_removed() is a good place to add
debugging/tracing events.

Fredrik with this change and the swaps, feel free to keep my previous
Reviewed-by tag.

>>> here (and swapping the two former calls ought to be fixed separately; I 
>>> haven't checked if there are more cases like this, but if so, then they 
>>> would best be amended with a single change).
>> I'll defer other ordering and indentation fixes since I'm not sure whether
>> such changes would be accepted.
>  Sure, no need for you to rush doing that.  In the absence of someone 
> willing to do such clean-ups voluntarily I would consider it a 
> maintainer's duty really.
>   Maciej

reply via email to

[Prev in Thread] Current Thread [Next in Thread]