Re: [PATCH, MIPS] Generate DMUL in widening multiplications

[Richard Sandiford <rdsandiford at googlemail dot com>]

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.

  - CODE_FOR_*s
  - using the new function in the define_insns too, and
  - passing the code directly, without the ==

Richard