Re: [Qemu-devel] [PATCH v5 2/2] target/mips: Optimize ILVEV.<B|H|W|D> MSA instructions

[Aleksandar Markovic]

> From: Richard Henderson <address@hidden>
> Subject: Re: [Qemu-devel] [PATCH v5 2/2] target/mips: Optimize 
> ILVEV.<B|H|W|D> MSA > instructions
> 
> On 4/3/19 6:25 AM, Aleksandar Markovic wrote:
> >
> > On Apr 2, 2019 5:20 PM, "Mateja Marjanovic" <address@hidden
> > <mailto:address@hidden>> wrote:
> >>
> >> From: Mateja Marjanovic <address@hidden
> > <mailto:address@hidden>>
> >>
> >> Optimize set of MSA instructions ILVEV, using directly
> >
> > Use full instruction names, with the only exception of possible Bachus-Naur
> > forms... again.
> >
> >> tcg registers and performing logic on them instead of
> >> using helpers.
> >>
> >> In the following table, the first column is the performance
> >> before this patch. The second represents the performance,
> >> after converting from helpers to tcg, but without using
> >> tcg_gen_deposit function. The third one is the solution
> >> which is implemented in this patch.
> >>
> >>  instr    ||   before    || no-deposit ||  with-deposit
> >> ========================================================
> >>  ilvev.b  ||  126.92 ms  ||  24.52 ms  ||  24.43 ms
> >>  ilvev.h  ||   93.67 ms  ||  23.92 ms  ||  23.86 ms
> >>  ilvev.w  ||  117.86 ms  ||  23.83 ms  ||  22.17 ms
> >>  ilvev.d  ||   45.49 ms  ||  19.74 ms  ||  19.71 ms
> >>
> >> The solution with deposit is suggested by Richard Henderson.
> >>
> >> Signed-off-by: Mateja Marjanovic <address@hidden
> > <mailto:address@hidden>>
> >> ---
> >
> > The byte and halfword cases of this patch most likely produce highly
> > unoptimized code for cases:
> >
> > wd == wt == ws
> > wd == wt != ws
> > wd != ws == wt
> > wd == ws != wt
> >
> > Please take these cases into account.
> 
> I beg to differ.  We want to minimize the amount of special cases.
> 
> If you multiply the different cases like this you also multiply the 
> maintenance
> overhead.  You force future maintainers to wonder if the cases are truly
> distinct or if they are mere optimization.
> 

I find your objection hard to understand.

The subject and the goal of the patch is obviously optimization. If there is
a danger of unclarity in the resulting source code, this is easily alleviated
by, for example, inserting informative comments, as is routinely done in
many other areas of QEMU or any software product.

Sincerely,
Aleksandar

> The only special cases that I advocate that you add are driven by standard
> macros that the assembler generates -- e.g. register move (via or), register
> negate (via nor), etc.
> 
> 
> r~