RE: [PATCH] MIPS: Fix generation of DIV.G and MOD.G for Loongson targets.

["Maciej W. Rozycki" <macro at imgtec dot com>]

On Thu, 12 Jan 2017, Toma Tabacu wrote:

> > > Unfortunately, this interferes with the generation of DIV.G and MOD.G
> > > (the <u>div<mode>3 and <u>mod<mode>3 patterns) for Loongson targets,
> > which
> > > causes test failures.
> > 
> >  What test failures?  Details please.
> > 
> 
> It's
> gcc.target/mips/loongson-muldiv-1.c
> gcc.target/mips/loongson-muldiv-2.c
> gcc.target/mips/loongson3a-muldiv-1.c
> gcc.target/mips/loongson3a-muldiv-2.c
> on O2, O3, and Os.
> 
> They are also checking for the Loongson-specific multiply instruction,
> but there are no failures for that.

 So these tests have e.g.:

NOMIPS16 st sdiv (st x, st y) { return x / y + x % y; }

and as such it looks to me like this code does legitimately use a single 
DIV instruction rather than a DIV.G and MOD.G pair, at least at -O2/-O3, 
as I'd expect two divisions to take twice as much computing time as one 
does, and the avoidance of the extra accumulator access overhead needed 
with DIV does not compensate for it.  For -Os actual code generated will 
have to be checked; I suspect DIV.G/MOD.G ought to be used rather than 
DIV/MFLO/MFHI as it's two instructions vs three.

 So as a first step I'd split these tests into individual cases covering 
signed/unsigned function pairs each of which doing a single operation 
only, i.e. smul/umul, sdiv/udiv, smod/umod, which will then be expected to 
always use the extra Loongson instructions.  This ought to provide the 
coverage originally intended (study the discussion around the submission 
of the patch that introduced these tests to double-check).

 As a further step a test case for sdivmod/udivmod will then be needed, 
to cover the use of DIV vs DIV.G/MOD.G as required for speed vs space 
optimisation.

 Likewise for GSMULT/GSDIV/GSMOD, etc. (hmm, why are the signed 
MULT.G/GSMULT instruction variants never used?).

> > > This solution might be excessive, however, as it effectively forbids the
> > > generation of the old DIV instruction for Loongson targets, which actually do
> > > support it.
> > 
> >  What's the purpose of this change other than "fixing test failures"?
> > Can you please demonstrate a technical justification of this change?  Has
> > there been a code quality regression which this patch addresses for
> > example?  What about source code which did emit `divmod<mode>4' and
> > `udivmod<mode>4' patterns on Loongson targets before r241660?
> > 
> >  Given that the DIV.G, MOD.G and accumulator DIV instructions (and their
> > unsigned counterparts) are all available the compiler should have freedom
> > to choose whichever hardware operation is the most suitable for the
> > calculations required according to code generation options selected and
> > artificially disabling some hardware instructions does not appear to be a
> > move in that direction to me.
> 
> I'll be honest here: I don't know when the compiler should generate the
> Loongson-specific division and modulo instructions, I don't have access to
> Loongson hardware, and I wasn't even specifically trying to fix Loongson-related
> issues.
> 
> I admit that the patch was submitted in haste, and I now realize that my
> proposal was unfounded and that I don't have the means to find a satisfactory
> solution. Too much wishful thinking on my part.
> 
> However, there is a legitimate underlying issue here and I felt it had to be
> brought up, but this should have been a bug report, not a patch submission.

 This doesn't look to me like a bug even, just a test suite regression 
which has been uncovered by the change recently made, so I think it would 
be enough if you just posted the relevant piece of `gcc.log', so that it 
is immediately known to the reader what the symptoms are.

 Thanks for reporting!

  Maciej