This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, MIPS] Generate DMUL in widening multiplications
Richard Sandiford writes:
> Adam Nemet <anemet@caviumnetworks.com> writes:
> > Richard Sandiford writes:
> >>
> >> - CODE_FOR_*s
> >> - using the new function in the define_insns too, and
> >> - passing the code directly, without the ==
> >
> > I had to limit the information in my email somewhere so I didn't mention one
> > additional problem with the combination of CODE_FOR and using
> > mips_mulsidi3_gen_fn in the condition of the define_insns. I.e.:
> >
> > (define_insn "<u>mulsidi3_64bit"
> > [(set (match_operand:DI 0 "register_operand" "=d")
> > (mult:DI (any_extend:DI (match_operand:SI 1 "register_operand" "d"))
> > (any_extend:DI (match_operand:SI 2 "register_operand" "d"))))
> > (clobber (match_scratch:TI 3 "=x"))
> > (clobber (match_scratch:DI 4 "=d"))]
> > "mips_mulsidi3_icode (<CODE>) == CODE_FOR_<u>mulsidi3_64bit"
> > "#"
> >
> > This fails to compile because gencondmd.c does not currently include
> > insn-codes.h. In fact, very few modules include insn-codes.h. If we want the
> > above construct we would have to change genconditions.c to also include
> > insn-codes.h into gencondmd.c. Let me know if you still want to take this
> > route or use function pointers instead.
>
> I think it'd be cleaner, but that sort of target-independent change isn't
> suitable at this stage. OK, let's go with the function pointers then.
Sorry for misleading you, I could have sworn that I had tried this but that
does not work either. While gencondmd.c includes tm.h which in turn includes
insn-flags.h which is where the gen_ functions are declared, insn-flags.h is
only included for non-GENERATOR_FILE compilations. IOW, we can't use gen_
functions in conditions either:
[(set (match_operand:DI 0 "register_operand" "=x")
(mult:DI (any_extend:DI (match_operand:SI 1 "register_operand" "d"))
(any_extend:DI (match_operand:SI 2 "register_operand" "d"))))]
- "!TARGET_64BIT && !TARGET_FIX_R4000 && !ISA_HAS_DSPR2"
+ "mips_mulsidi3_gen_fn (<CODE>) == gen_<u>mulsidi3_32bit"
"mult<u>\t%1,%2"
[(set_attr "type" "imul")
(set_attr "mode" "SI")])
:(. So I don't think we can do better ATM than the current patch at:
http://gcc.gnu.org/ml/gcc-patches/2009-10/msg01755.html
with the additional improvements of passing the rtx code to
mips_mulsidi3_gen_fn and also using the DSPr2 generators there. Do you agree?
I am going to retest this the same way as before. OK for mainline if it
passes?
Adam
* config/mips/mips-protos.h (mulsidi3_gen_fn): New typedef.
(mips_mulsidi3_gen_fn): Declare new function.
* config/mips/mips.c (mips_mulsidi3_gen_fn): New function.
* config/mips/mips.md (<u>mulsidi3): Change condition to use
mips_mulsidi3_gen_fn. Use mips_mulsidi3_gen_fn to generate the
insn.
(<u>mulsidi3_64bit): Don't match for ISA_HAS_DMUL3.
(mulsidi3_64bit_dmul): New define_insn.
testsuite/
* gcc.target/mips/mult-1.c: Forbid octeon.
* gcc.target/mips/octeon-dmul-3.c: New test.
Index: gcc/config/mips/mips-protos.h
===================================================================
--- gcc.orig/config/mips/mips-protos.h 2009-11-12 10:52:00.000000000 -0800
+++ gcc/config/mips/mips-protos.h 2009-11-12 10:53:27.000000000 -0800
@@ -345,4 +345,7 @@ extern bool mips_epilogue_uses (unsigned
extern void mips_final_prescan_insn (rtx, rtx *, int);
extern int mips_trampoline_code_size (void);
+typedef rtx (*mulsidi3_gen_fn) (rtx, rtx, rtx);
+extern mulsidi3_gen_fn mips_mulsidi3_gen_fn (enum rtx_code);
+
#endif /* ! GCC_MIPS_PROTOS_H */
Index: gcc/config/mips/mips.c
===================================================================
--- gcc.orig/config/mips/mips.c 2009-11-12 10:52:00.000000000 -0800
+++ gcc/config/mips/mips.c 2009-11-12 10:56:45.000000000 -0800
@@ -15926,6 +15926,39 @@ mips_final_postscan_insn (FILE *file ATT
if (mips_need_noat_wrapper_p (insn, opvec, noperands))
mips_pop_asm_switch (&mips_noat);
}
+
+/* Return the function that is used to expand the <u>mulsidi3 pattern.
+ EXT_CODE is the code of the extension used. Return NULL if widening
+ multiplication shouldn't be used. */
+
+mulsidi3_gen_fn
+mips_mulsidi3_gen_fn (enum rtx_code ext_code)
+{
+ bool signed_;
+
+ signed_ = ext_code == SIGN_EXTEND;
+ if (TARGET_64BIT)
+ {
+ /* Don't use widening multiplication with MULT when we have DMUL. Even
+ with the extension of its input operands DMUL is faster. Note that
+ the extension is not needed for signed multiplication. In order to
+ ensure that we always remove the redundant sign-extension in this
+ case we still expand mulsidi3 for DMUL. */
+ if (ISA_HAS_DMUL3)
+ return signed_ ? gen_mulsidi3_64bit_dmul : NULL;
+ if (TARGET_FIX_R4000)
+ return NULL;
+ return signed_ ? gen_mulsidi3_64bit : gen_umulsidi3_64bit;
+ }
+ else
+ {
+ if (TARGET_FIX_R4000)
+ return signed_ ? gen_mulsidi3_32bit_r4000 : gen_umulsidi3_32bit_r4000;
+ if (ISA_HAS_DSPR2)
+ return signed_ ? gen_mips_mult : gen_mips_multu;
+ return signed_ ? gen_mulsidi3_32bit : gen_umulsidi3_32bit;
+ }
+}
/* Return the size in bytes of the trampoline code, padded to
TRAMPOLINE_ALIGNMENT bits. The static chain pointer and target
Index: gcc/config/mips/mips.md
===================================================================
--- gcc.orig/config/mips/mips.md 2009-11-12 10:52:00.000000000 -0800
+++ gcc/config/mips/mips.md 2009-11-12 10:54:08.000000000 -0800
@@ -1847,15 +1847,10 @@ (define_expand "<u>mulsidi3"
[(set (match_operand:DI 0 "register_operand")
(mult:DI (any_extend:DI (match_operand:SI 1 "register_operand"))
(any_extend:DI (match_operand:SI 2 "register_operand"))))]
- "!TARGET_64BIT || !TARGET_FIX_R4000"
+ "mips_mulsidi3_gen_fn (<CODE>) != NULL"
{
- if (TARGET_64BIT)
- emit_insn (gen_<u>mulsidi3_64bit (operands[0], operands[1], operands[2]));
- else if (TARGET_FIX_R4000)
- emit_insn (gen_<u>mulsidi3_32bit_r4000 (operands[0], operands[1],
- operands[2]));
- else
- emit_insn (gen_<u>mulsidi3_32bit (operands[0], operands[1], operands[2]));
+ mulsidi3_gen_fn fn = mips_mulsidi3_gen_fn (<CODE>);
+ emit_insn (fn (operands[0], operands[1], operands[2]));
DONE;
})
@@ -1885,7 +1880,7 @@ (define_insn "<u>mulsidi3_64bit"
(any_extend:DI (match_operand:SI 2 "register_operand" "d"))))
(clobber (match_scratch:TI 3 "=x"))
(clobber (match_scratch:DI 4 "=d"))]
- "TARGET_64BIT && !TARGET_FIX_R4000"
+ "TARGET_64BIT && !TARGET_FIX_R4000 && !ISA_HAS_DMUL3"
"#"
[(set_attr "type" "imul")
(set_attr "mode" "SI")
@@ -1961,6 +1956,17 @@ (define_insn "<u>mulsidi3_64bit_hilo"
[(set_attr "type" "imul")
(set_attr "mode" "SI")])
+;; See comment before the ISA_HAS_DMUL3 case in mips_mulsidi3_gen_fn.
+(define_insn "mulsidi3_64bit_dmul"
+ [(set (match_operand:DI 0 "register_operand" "=d")
+ (mult:DI (sign_extend:DI (match_operand:SI 1 "register_operand" "d"))
+ (sign_extend:DI (match_operand:SI 2 "register_operand" "d"))))
+ (clobber (match_scratch:DI 3 "=l"))]
+ "TARGET_64BIT && ISA_HAS_DMUL3"
+ "dmul\t%0,%1,%2"
+ [(set_attr "type" "imul3")
+ (set_attr "mode" "DI")])
+
;; Widening multiply with negation.
(define_insn "*muls<u>_di"
[(set (match_operand:DI 0 "register_operand" "=x")
Index: gcc/testsuite/gcc.target/mips/mult-1.c
===================================================================
--- gcc.orig/testsuite/gcc.target/mips/mult-1.c 2009-11-12 10:52:00.000000000 -0800
+++ gcc/testsuite/gcc.target/mips/mult-1.c 2009-11-12 10:52:29.000000000 -0800
@@ -1,6 +1,6 @@
/* For SI->DI widening multiplication we should use DINS to combine the two
- halves. */
-/* { dg-options "-O -mgp64 isa_rev>=2" } */
+ halves. For Octeon use DMUL with explicit widening. */
+/* { dg-options "-O -mgp64 isa_rev>=2 forbid_cpu=octeon" } */
/* { dg-final { scan-assembler "\tdins\t" } } */
/* { dg-final { scan-assembler-not "\tdsll\t" } } */
/* { dg-final { scan-assembler-not "\tdsrl\t" } } */
Index: gcc/testsuite/gcc.target/mips/octeon-dmul-3.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ gcc/testsuite/gcc.target/mips/octeon-dmul-3.c 2009-11-12 10:52:29.000000000 -0800
@@ -0,0 +1,19 @@
+/* Use DMUL for widening multiplication too. */
+/* { dg-options "-O -march=octeon -mgp64" } */
+/* { dg-final { scan-assembler-times "\tdmul\t" 2 } } */
+/* { dg-final { scan-assembler-not "\td?mult\t" } } */
+/* { dg-final { scan-assembler-times "\tdext\t" 2 } } */
+
+NOMIPS16 long long
+f (int i, int j)
+{
+ i++;
+ return (long long) i * j;
+}
+
+NOMIPS16 unsigned long long
+g (unsigned int i, unsigned int j)
+{
+ i++;
+ return (unsigned long long) i * j;
+}