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
- From: Adam Nemet <anemet at caviumnetworks dot com>
- To: Richard Sandiford <rdsandiford at googlemail dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: 9 Nov 2009 10:55:56 -0800
- Subject: Re: [PATCH, MIPS] Generate DMUL in widening multiplications
- References: <19177.56862.899575.240299@ropi.home> <87eiof8fsz.fsf@firetop.home>
Richard Sandiford writes:
> Adam Nemet <anemet@caviumnetworks.com> writes:
> > * We could alternatively use enum insn_code (CODE_FOR_, etc.) rather than
> > function pointers for the gen functions.
> >
> > * We could use mips_mulsidi3_gen_fn in the condition part of the underlying
> > define_insns as well. E.g.:
> >
> > (define_insn "<u>mulsidi3_64bit"
> > [...]
> > - "TARGET_64BIT && !TARGET_FIX_R4000 && !ISA_HAS_DMUL3"
> > + "mips_mulsidi3_gen_fn (<CODE> == SIGN_EXTEND) == gen_<u>mulsidi64_bit"
> >
> > The idea being that we always want to match the insn that we expanded.
> > This not only looks a little funky but it's not actually true in case of
> > the DSPr2 widening mult patterns. So after pondering for a while I stayed
> > with the existing practice.
>
> I don't think it's that bad, especially if you just pass the code,
> not the result of the ==. I'd rather do that than duplicate the insn
> conditions (which could easily get out of sync otherwise).
>
> Making the choice between the DSPr2 and non-DSPr2 patterns explicit
> would be a good thing.
>
> Also, the priority of the insns is handled more naturally with that
> approach (fewer "&& !"s) and we're guaranteed to have mutually-
> exclusive insn conditions.
>
> Your alternative suggestion of using CODE_FOR_* would be neater for
> these comparisons, but I suppose it sacrifices a bit of type-safety in
> the define_expand code. OTOH, the CODE_FOR_* enum leads you to whatever
> info you need about an insn, whereas you can't really learn anything else
> from a gen_* function. Hmm.
>
> I suppose there's no one good solution here, but I'm afraid I do prefer
> both the alternatives you suggest. I.e.
No problem. I was really on the fence about these :).
>
> - 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.
Adam