On Sun, Aug 19, 2012 at 10:13 AM, Richard Sandiford
<rdsandif...@googlemail.com> wrote:
> Andrew Pinski <andrew.pin...@caviumnetworks.com> writes:
>> Right now we only produce ins when a zero_extract is used on the
>> right hand side. We can do better by adding some patterns which
>> combine for the ins instruction. This patch adds those patterns and a
>> testcase which shows a simple example where the code is improved.
>
> Sorry for the delay in reviewing this. Had you thought about trying to
> teach combine.c about this instead? It doesn't look like any of the
> patterns are really providing more information about the underlying
> instruction.
combine.c has some code to do this already if one of the src register
is the same as the dest register; that is what make_field_assignment
does. Quickly looking at the code, the problem I doing it in
make_field_assignment is there is no way to return that you need a
copy of the value first unless I am missing something obvious. Now I
agree we should be optimize this in combine rather than these manual
patterns.
Thanks,
Andrew Pinski
>
> At the moment the patch has things like:
>
>> +(define_insn "*insv<mode>_internal1"
>> + [(set (match_operand:GPR 0 "register_operand" "=d")
>> + (ior:GPR (and:GPR (match_operand:GPR 1 "register_operand" "0")
>> + (match_operand:GPR 2 "const_int_operand" "i"))
>> + (and:GPR (match_operand:GPR 3 "register_operand" "d")
>> + (match_operand:GPR 4 "const_int_operand" "i"))))]
>> + "ISA_HAS_EXT_INS && mips_bottom_bitmask_p (INTVAL (operands[4]))
>> + && INTVAL(operands[2]) == ~INTVAL(operands[4])"
>> +{
>> + int len, pos;
>> + pos = mips_bitmask (INTVAL (operands[4]), &len, <MODE>mode);
>> + operands[2] = GEN_INT (pos);
>> + operands[4] = GEN_INT (len);
>> + return "<d>ins\t%0,%3,%2,%4";
>> +}
>> + [(set_attr "type" "arith")
>> + (set_attr "mode" "<MODE>")])
>
> but AFAIK there's nothing to guarantee that the bottom bitmask
> will be operand 2 rather than operand 4, so if we really do need
> do this via patterns, I think we'd need both orderrs.
>
> But if we do it this way, I assume every backend that provides
> an insv pattern will need to cut-&-paste these patterns too.
>
> Richard
>
