Re: [Qemu-devel] [PATCH v4 5/8] target/mips: R5900 DMULT[U], DDIV[U], LL, SC, LLD and SCD are user only

[Philippe Mathieu-Daudé]

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)
>> {
>> #ifndef CONFIG_USER_ONLY
>>     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
>