This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 4/4, MIPS] Add *and<mode>3_ext pattern


Adam Nemet <anemet@caviumnetworks.com> writes:
> This is the patch the actually adds the new pattern to generate ext when the
> same operation with "and"  would require us to synthesize the constant first.
>
> Between all these different "and" combine patterns
> (*and<mode>3, *clear_upper32_dext, *and<mode>3_ext) I let the order in the .md
> file decide the precedence and added to tests to validate this.  This is
> obviously more efficient than enforcing mutual exclusivity on these patterns
> but it's also more error-prone (hence the tests).
>
> If we want to enforce exclusivity I can change the predicate to (untested):
>
> ;; Match the constant low-order bitmask operand in AND that can be implemented
> ;; with <d>ext.  Give preference to *and<mode>3 and *clear_upper32_dext by not
> ;; matching their operand.
> (define_predicate "and_ext_low_bitmask_operand"
>   (and (match_code "const_int")
>        (match_test "low_bitmask_len (VOIDmode, INTVAL (op)) > 0")
>        (match_test "INTVAL (op) != 0xffffffff")
>        (not (match_operand 0 "const_uns_arith_operand"))))

I think this reduces to:

  (and (const_int "const_int")
       (match_test "IN_RANGE (low_bitmask_len (mode, INTVAL (op)), 17, 31)"))

(and I think we should be using "mode" here.)

However, how about:

- adding ISA_HAS_EXT_DEXT to this predicate

- adding a new "and_operand" predicate:

    (ior (match_operand "uns_arith_operand")
         (match_operand "and_ext_low_bitmask_operand"))

- using and_operand instead of uns_arith_operand for the "and" patterns.

- adding a new constraint (Y something, maybe "YL" for "low"?)
  that maps to and_ext_low_bitmask_operand

- adding this as an alternative to "*and<mode>3"

?

The point being, it's generally better to have a single insn with
several alternatives than several insns with fewer alternatives.

(Yes, it could be argued that we should do the same for
*clear_upper32, but it's not as easy to do that cleanly with
the extra memory alternative.  Here it's different, because
the new alternative is very much an and-like pattern.)

Richard


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]