On Fri, Oct 5, 2012 at 8:43 PM, Andrew Pinski
<andrew.pin...@caviumnetworks.com> wrote:
> 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.
I now have a patch which implements this in combine which allows the
backend not need to change. I generate a SEQUENCE which then
try_combine splits like we do for PARALLEL but keeping it in the
correct order and allowing for the case where we are combing two
instructions into two instructions.
I hope to be able to post it later on Saturday.
Thanks,
Andrew Pinski
>
> 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
>>
