On 07/09/12 12:45, Christophe Lyon wrote:
> On 6 September 2012 18:42, Richard Earnshaw <rearn...@arm.com> wrote:
>> On 06/09/12 17:07, Christophe Lyon wrote:
>>>
>>> But why are the thumb1_XXX patterns still necessary?
>>> I tried removing them, but compiling the testcase with -march=armv6
>>> -mthumb makes the compiler fail (internal compiler error:
>>> output_operand: invalid %-code)
>>>
>>
>> They probably aren't necessary. It should be possible to combine these
>> patterns into
>>
>> (define_insn "*arm_revsh"
>> [(set (match_operand:SI 0 "s_register_operand" "=l,r")
>> (sign_extend:SI (bswap:HI (match_operand:HI 1
>> "s_register_operand" "l,r"))))]
>> "arm_arch6"
>> "revsh%?\t%0, %1"
>> [(set_attr "predicable" "yes")
>> (set_attr "arch" "t,32")
>> (set_attr "length" "2,4")]
>>
>>
>
> Thanks for showing me the right "arch" value.
> The problem with this pattern if I delete the *thumb1_revsh one, is
> that %? is not accepted as a valid punctuation indicator for an
> operand when in thumb1 mode.
>
> In particular, arm_print_operand_punct_valid_p would return true in
> thumb1 if the punctuation character was '_'.
>
> However, I failed to find a '%_' operand in the ARM description: is
> arm_print_operand_punct_valid_p still accurate?
>
> Or can I replace '_' by '?' in this function?
>
Ah, sigh! I'd forgotten about the cond-exec issue. That makes things
a little awkward, since we also have to deal with the fact that thumb1
does not support predication. The solution, unfortunately, is thus a
bit more involved.
What we need are two patterns (although currently it looks like we've got two,
in reality the predication means there are three), which need to read:
(define_insn "*arm_revsh"
[(set (match_operand:SI 0 "s_register_operand" "=l,l,r")
(sign_extend:SI (bswap:HI (match_operand:HI 1
"s_register_operand" "l,l,r"))))]
"arm_arch6"
"@
revsh\t%0, %1
revsh%?\t%0, %1
revsh%?\t%0, %1"
[(set_attr "arch" "t1,t2,32")
(set_attr "length" "2,2,4")]
(define_insn "*arm_revsh_cond"
[(cond_exec (match_operator 2 "arm_comparison_operator"
[(match_operand 3 "cc_register" "") (const_int 0)])
(set (match_operand:SI 0 "s_register_operand" "=l,r")
(sign_extend:SI (bswap:HI (match_operand:HI 1
"s_register_operand" "l,r")))))]
"TARGET32_BIT && arm_arch6"
"revsh%?\t%0, %1"
[(set_attr "arch" "t2,*")
(set_attr "length" "2,4")])
Note that this removes the "predicable" attribute as we now handle this
manually rather than with the auto-generation.
Sorry, this has turned out to be more complex than I originally realised.
R.
