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

[Toma Tabacu]

> From: Matthew Fortune
> 
> Apart from those changes this looks OK to me.
> 
> Matthew

Thanks.
Committed as r244570.

Regards,
Toma

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

[Matthew Fortune]

Toma Tabacu  writes:
> Matthew Fortune writes:
> >
> > Sounds good. I'd prefer to get the testsuite clean first then improve the
> > code quality as a later step since it is not a regression and we are
> > a few days off stage 4.
> >
> > In terms of the patch then the ISA_HAS_DIV3 macro is not currently used so
> > I suggest that instead it is renamed to ISA_AVOID_DIV_HILO and then use
> > that macro in the definition of ISA_HAS_DIV and ISA_HAS_DDIV to turn
> > off the DIV/DDIV instructions.
> >
> > The ISA_HAS_DIV3 should have been cleaned up when R6 was added as it is
> > ambiguous and could refer to multiple variants of 3-reg operand DIV now
> > rather than just Loongson's.
> >
> > Thanks,
> > Matthew
> 
> I believe the patch below fits the description.
> I've also added a (too?) succinct explanation for the ISA_AVOID_DIV_HILO 
> macro.
> 
> Tested with mips-mti-elf.
> 
> Regards,
> Toma
> 
> gcc/ChangeLog:
> 
>   * config/mips/mips.h: Add macro to prevent generation of regular
>   (D)DIV(U) instructions for Loongson.

The changelog needs to be more detailed about what changed and can be
less detailed about why:

* config/mips/mips.h (ISA_HAS_DIV3): Remove unused macro.
(ISA_AVOID_DIV_HILO): New macro.
(ISA_HAS_DIV): Use new ISA_AVOID_DIV_HILO macro.
(ISA_HAS_DDIV): Likewise.

> diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
> index f91b43d..e21e7d8 100644
> --- a/gcc/config/mips/mips.h
> +++ b/gcc/config/mips/mips.h
> @@ -967,19 +967,24 @@ struct mips_cpu_info {
>  /* ISA supports instructions DMUL, DMULU, DMUH, DMUHU.  */
>  #define ISA_HAS_R6DMUL   (TARGET_64BIT && mips_isa_rev >= 6)
> 
> +/* For Loongson, it is preferable to use the Loongson-specific division and
> +   modulo instructions instead of the regular (D)DIV(U) instruction, because
> +   the former are faster and also have the effect of reducing code size.  */

Might want to put 'also can have the effect' given they don't yet.

> +#define ISA_AVOID_DIV_HILO   ((TARGET_LOONGSON_2EF   \
> +   || TARGET_LOONGSON_3A)\
> +  && !TARGET_MIPS16)
> +
>  /* ISA supports instructions DDIV and DDIVU. */
>  #define ISA_HAS_DDIV (TARGET_64BIT   \
>&& !TARGET_MIPS5900\
> +  && !ISA_AVOID_DIV_HILO \
>&& mips_isa_rev <= 5)
> 
>  /* ISA supports instructions DIV and DIVU.
> This is always true, but the macro is needed for ISA_HAS_DIV
> in mips.md.  */
> -#define ISA_HAS_DIV  (mips_isa_rev <= 5)
> -
> -#define ISA_HAS_DIV3 ((TARGET_LOONGSON_2EF   \
> -   || TARGET_LOONGSON_3A)\
> -  && !TARGET_MIPS16)
> +#define ISA_HAS_DIV  (!ISA_AVOID_DIV_HILO\
> +  && mips_isa_rev <= 5)
> 
>  /* ISA supports instructions DIV, DIVU, MOD and MODU.  */
>  #define ISA_HAS_R6DIV(mips_isa_rev >= 6)

Apart from those changes this looks OK to me.

Matthew

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

[Toma Tabacu]

> Maciej Rozycki writes:
> >  This ought to be handled then, likely by adding Loongson-specific RTL
> > insns matching the `divmod4' and `udivmod4' expanders.  It
> > may be as simple as say (conceptually, untested):
> >
> > (define_insn "divmod4_loongson"
> >   [(set (match_operand:GPR 0 "register_operand" "=d")
> > (any_div:GPR (match_operand:GPR 1 "register_operand" "d")
> >  (match_operand:GPR 2 "register_operand" "d")))
> >(set (match_operand:GPR 3 "register_operand" "=d")
> > (any_mod:GPR (match_dup 1)
> >  (match_dup 2)))]
> >   "TARGET_LOONGSON_2EF"
> > {
> >   return mips_output_division
> > ("div.g\t%0,%1,%2\;mod.g\t%3,%1,%2",
> operands);
> > }
> >   [(set_attr "type" "idiv")
> >(set_attr "mode" "")])
> >
> > although any final fix will have to take an instruction count adjustment
> > into account too, as `mips_idiv_insns' won't as it stands handle the new
> > case.

Thanks for the tip Maciej!
I will tackle that issue in a separate patch.

Matthew Fortune writes:
> 
> Sounds good. I'd prefer to get the testsuite clean first then improve the
> code quality as a later step since it is not a regression and we are
> a few days off stage 4.
> 
> In terms of the patch then the ISA_HAS_DIV3 macro is not currently used so
> I suggest that instead it is renamed to ISA_AVOID_DIV_HILO and then use
> that macro in the definition of ISA_HAS_DIV and ISA_HAS_DDIV to turn
> off the DIV/DDIV instructions.
> 
> The ISA_HAS_DIV3 should have been cleaned up when R6 was added as it is
> ambiguous and could refer to multiple variants of 3-reg operand DIV now
> rather than just Loongson's.
> 
> Thanks,
> Matthew

I believe the patch below fits the description.
I've also added a (too?) succinct explanation for the ISA_AVOID_DIV_HILO macro.

Tested with mips-mti-elf.

Regards,
Toma

gcc/ChangeLog:

* config/mips/mips.h: Add macro to prevent generation of regular
(D)DIV(U) instructions for Loongson.

diff --git a/gcc/config/mips/mips.h b/gcc/config/mips/mips.h
index f91b43d..e21e7d8 100644
--- a/gcc/config/mips/mips.h
+++ b/gcc/config/mips/mips.h
@@ -967,19 +967,24 @@ struct mips_cpu_info {
 /* ISA supports instructions DMUL, DMULU, DMUH, DMUHU.  */
 #define ISA_HAS_R6DMUL (TARGET_64BIT && mips_isa_rev >= 6)
 
+/* For Loongson, it is preferable to use the Loongson-specific division and
+   modulo instructions instead of the regular (D)DIV(U) instruction, because
+   the former are faster and also have the effect of reducing code size.  */
+#define ISA_AVOID_DIV_HILO ((TARGET_LOONGSON_2EF   \
+ || TARGET_LOONGSON_3A)\
+&& !TARGET_MIPS16)
+
 /* ISA supports instructions DDIV and DDIVU. */
 #define ISA_HAS_DDIV   (TARGET_64BIT   \
 && !TARGET_MIPS5900\
+&& !ISA_AVOID_DIV_HILO \
 && mips_isa_rev <= 5)
 
 /* ISA supports instructions DIV and DIVU.
This is always true, but the macro is needed for ISA_HAS_DIV
in mips.md.  */
-#define ISA_HAS_DIV(mips_isa_rev <= 5)
-
-#define ISA_HAS_DIV3   ((TARGET_LOONGSON_2EF   \
- || TARGET_LOONGSON_3A)\
-&& !TARGET_MIPS16)
+#define ISA_HAS_DIV(!ISA_AVOID_DIV_HILO\
+&& mips_isa_rev <= 5)
 
 /* ISA supports instructions DIV, DIVU, MOD and MODU.  */
 #define ISA_HAS_R6DIV  (mips_isa_rev >= 6)

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

[Matthew Fortune]

Maciej Rozycki  writes:
> On Mon, 16 Jan 2017, Toma Tabacu wrote:
> 
> > After searching through the archives, I have found an interesting bit
> > of information about DIV.G/MOD.G in the original submission thread:
> >
> > > > Ruan Beihong 23 July 2008:
> > > >
> > > > I've seen the Loongson 2F manual carefully. The (d)div(u) is
> > > > internally splited into one (d)div(u).g and one (d)mod(u).g. So I
> > > > said before was wrong. The truth is that, (d)div(u).g and
> > > > (d)mod(u).g are always faster than (d)div(u), at least the time
> > > > spend on mflo/mfhi is saved.
> > > >
> > > > James Ruan
> > >
> > > Richard Sandiford 24 July 2008:
> > >
> > > OK, great.  In that case, it should simply be a case of disabling
> > > the divmod-related insns for Loongson, in addition to your patch.
> > > (Probably stating the obvious there, sorry.)
> > >
> > > Richard
> >
> > Here's the link for part 1 of the submission thread (has the quotes
> from above):
> > https://gcc.gnu.org/ml/gcc-patches/2008-07/msg01529.html
> > and here's part 2:
> > https://gcc.gnu.org/ml/gcc-patches/2008-11/msg00273.html
> 
>  Thanks for digging this out!
> 
> > If DIV.G/MOD.G are faster, according to Ruan Beihong, and also smaller
> > than DIV (or the same size [1]), as pointed out by Maciej, then I am
> > led to the same conclusion as Richard Sandiford: that only DIV.G/MOD.G
> > should be generated for Loongson.
> >
> > I think it would still be a good idea to add a test for separated
> > DIV.G/MOD.G, though.
> 
>  Possibly, though the combined tests need to stay then, to make sure
> generic DIV/DIVU is not ever produced.

I'm happy to just stick with the original tests as they effectively test
both scenarios just at different optimisation levels. i.e. the new divmod
expansion only kicks in at -O2 I believe.

> > What are your thoughts on this ?
> > Have I misunderstood something in the context of the submission thread
> ?
> >
> > Regards,
> > Toma
> >
> > [1] I've noticed that GCC generates the same TEQ instruction twice if
> > both DIV.G and MOD.G are needed, which makes the sequence just as big
> > as DIV + TEQ + MFHI + MFLO; this seems unnecessary to me.
> 
>  This ought to be handled then, likely by adding Loongson-specific RTL
> insns matching the `divmod4' and `udivmod4' expanders.  It
> may be as simple as say (conceptually, untested):
> 
> (define_insn "divmod4_loongson"
>   [(set (match_operand:GPR 0 "register_operand" "=d")
>   (any_div:GPR (match_operand:GPR 1 "register_operand" "d")
>(match_operand:GPR 2 "register_operand" "d")))
>(set (match_operand:GPR 3 "register_operand" "=d")
>   (any_mod:GPR (match_dup 1)
>(match_dup 2)))]
>   "TARGET_LOONGSON_2EF"
> {
>   return mips_output_division
> ("div.g\t%0,%1,%2\;mod.g\t%3,%1,%2", operands);
> }
>   [(set_attr "type" "idiv")
>(set_attr "mode" "")])
> 
> although any final fix will have to take an instruction count adjustment
> into account too, as `mips_idiv_insns' won't as it stands handle the new
> case.

Sounds good. I'd prefer to get the testsuite clean first then improve the
code quality as a later step since it is not a regression and we are
a few days off stage 4.

In terms of the patch then the ISA_HAS_DIV3 macro is not currently used so
I suggest that instead it is renamed to ISA_AVOID_DIV_HILO and then use
that macro in the definition of ISA_HAS_DIV and ISA_HAS_DDIV to turn
off the DIV/DDIV instructions.

The ISA_HAS_DIV3 should have been cleaned up when R6 was added as it is
ambiguous and could refer to multiple variants of 3-reg operand DIV now
rather than just Loongson's.

Thanks,
Matthew

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

[Maciej W. Rozycki]

On Mon, 16 Jan 2017, Toma Tabacu wrote:

> After searching through the archives, I have found an interesting bit of
> information about DIV.G/MOD.G in the original submission thread:
> 
> > > Ruan Beihong 23 July 2008:
> > > 
> > > I've seen the Loongson 2F manual carefully. The (d)div(u) is 
> > > internally splited into one (d)div(u).g and one (d)mod(u).g. So I said 
> > > before was wrong. The truth is that, (d)div(u).g and (d)mod(u).g are 
> > > always faster than (d)div(u), at least the time spend on mflo/mfhi is 
> > > saved. 
> > > 
> > > James Ruan 
> > 
> > Richard Sandiford 24 July 2008:
> > 
> > OK, great.  In that case, it should simply be a case of disabling
> > the divmod-related insns for Loongson, in addition to your patch.
> > (Probably stating the obvious there, sorry.)
> > 
> > Richard
> 
> Here's the link for part 1 of the submission thread (has the quotes from 
> above):
> https://gcc.gnu.org/ml/gcc-patches/2008-07/msg01529.html
> and here's part 2:
> https://gcc.gnu.org/ml/gcc-patches/2008-11/msg00273.html

 Thanks for digging this out!

> If DIV.G/MOD.G are faster, according to Ruan Beihong, and also smaller than 
> DIV
> (or the same size [1]), as pointed out by Maciej, then I am led to the same
> conclusion as Richard Sandiford: that only DIV.G/MOD.G should be generated for
> Loongson.
> 
> I think it would still be a good idea to add a test for separated DIV.G/MOD.G,
> though.

 Possibly, though the combined tests need to stay then, to make sure 
generic DIV/DIVU is not ever produced.

> What are your thoughts on this ?
> Have I misunderstood something in the context of the submission thread ?
> 
> Regards,
> Toma
> 
> [1] I've noticed that GCC generates the same TEQ instruction twice if both
> DIV.G and MOD.G are needed, which makes the sequence just as big as
> DIV + TEQ + MFHI + MFLO; this seems unnecessary to me.

 This ought to be handled then, likely by adding Loongson-specific RTL 
insns matching the `divmod4' and `udivmod4' expanders.  It may 
be as simple as say (conceptually, untested):

(define_insn "divmod4_loongson"
  [(set (match_operand:GPR 0 "register_operand" "=d")
(any_div:GPR (match_operand:GPR 1 "register_operand" "d")
 (match_operand:GPR 2 "register_operand" "d")))
   (set (match_operand:GPR 3 "register_operand" "=d")
(any_mod:GPR (match_dup 1)
 (match_dup 2)))]
  "TARGET_LOONGSON_2EF"
{
  return mips_output_division
("div.g\t%0,%1,%2\;mod.g\t%3,%1,%2", operands);
}
  [(set_attr "type" "idiv")
   (set_attr "mode" "")])

although any final fix will have to take an instruction count adjustment 
into account too, as `mips_idiv_insns' won't as it stands handle the new 
case.

  Maciej

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

[Toma Tabacu]

After searching through the archives, I have found an interesting bit of
information about DIV.G/MOD.G in the original submission thread:

> > Ruan Beihong 23 July 2008:
> > 
> > I've seen the Loongson 2F manual carefully. The (d)div(u) is 
> > internally splited into one (d)div(u).g and one (d)mod(u).g. So I said 
> > before was wrong. The truth is that, (d)div(u).g and (d)mod(u).g are 
> > always faster than (d)div(u), at least the time spend on mflo/mfhi is 
> > saved. 
> > 
> > James Ruan 
> 
> Richard Sandiford 24 July 2008:
> 
> OK, great.  In that case, it should simply be a case of disabling
> the divmod-related insns for Loongson, in addition to your patch.
> (Probably stating the obvious there, sorry.)
> 
> Richard

Here's the link for part 1 of the submission thread (has the quotes from above):
https://gcc.gnu.org/ml/gcc-patches/2008-07/msg01529.html
and here's part 2:
https://gcc.gnu.org/ml/gcc-patches/2008-11/msg00273.html

If DIV.G/MOD.G are faster, according to Ruan Beihong, and also smaller than DIV
(or the same size [1]), as pointed out by Maciej, then I am led to the same
conclusion as Richard Sandiford: that only DIV.G/MOD.G should be generated for
Loongson.

I think it would still be a good idea to add a test for separated DIV.G/MOD.G,
though.

What are your thoughts on this ?
Have I misunderstood something in the context of the submission thread ?

Regards,
Toma

[1] I've noticed that GCC generates the same TEQ instruction twice if both
DIV.G and MOD.G are needed, which makes the sequence just as big as
DIV + TEQ + MFHI + MFLO; this seems unnecessary to me.

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

[Matthew Fortune]

Maciej Rozycki  writes:
> On Thu, 12 Jan 2017, Toma Tabacu wrote:
> 
> > > > Unfortunately, this interferes with the generation of DIV.G and
> > > > MOD.G (the div3 and mod3 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).

This sounds like a reasonable fix. The theme of Toma's changes has been to
improve testsuite stability rather than code gen so breaking these tests
up to cover the original goal looks OK.

>  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.

I see no need to improve code quality currently as it would be new work.
Based on the description I expect -Os will be using DIV and MFLO/MFHI.
Adding a test to record that certain optimisation levels happen to now
get the base arch DIV instruction used could be risky for stability but
I have no objection unless the new test somehow fails when pitted against
the plethora of configurations we have to test.

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

Somewhat weird but so is the presence of signed and unsigned mul in R6.
I believe that microarchitecture optimisation is possible where results
could be memoised waiting for the high part multiply to be issued and
then reused.

Since there shouldn't be any compiler change needed to resolve this then
I see no need to test specifically against loongson. Just verify the
code generated looks reasonable as per the original tests.

Thanks,
Matthew

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

[Maciej W. Rozycki]

On Thu, 12 Jan 2017, Toma Tabacu wrote:

> > > Unfortunately, this interferes with the generation of DIV.G and MOD.G
> > > (the div3 and mod3 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 `divmod4' and
> > `udivmod4' 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

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

[Toma Tabacu]

> Maciej Rozycki writes:
> 
> >
> > Unfortunately, this interferes with the generation of DIV.G and MOD.G
> > (the div3 and mod3 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.

> 
> > 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 `divmod4' and
> `udivmod4' 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.
> 
>   Maciej

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.

Anyway, thank you for the feedback.

Regards,
Toma

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

[Maciej W. Rozycki]

On Mon, 9 Jan 2017, Toma Tabacu wrote:

> The expand_DIVMOD function, introduced in r241660, will pick the divmod4
> (or the udivmod4) pattern when it checks for the presence of hardware
> div/mod instructions, which results in the generation of the old DIV
> instruction.
> 
> Unfortunately, this interferes with the generation of DIV.G and MOD.G
> (the div3 and mod3 patterns) for Loongson targets, which
> causes test failures.

 What test failures?  Details please.

> This patch prevents the selection of divmod4 and udivmod4 when
> targeting Loongson by adding !ISA_HAS_DIV3 to the match condition.
> ISA_HAS_DIV3 checks for the presence of the 3-operand Loongson-specific DIV.G
> and MOD.G instructions.
> 
> Tested with mips-mti-elf.

 And Loongson hardware presumably, right?

> 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 `divmod4' and 
`udivmod4' 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.

  Maciej