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
