[PATCH][ARM] Handle UNSPEC_VOLATILE in rtx costs and don't recurse inside the unspec

[Ramana Radhakrishnan]

On Mon, Apr 20, 2015 at 5:28 PM, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
> Hi all,
>
> A pet project of mine is to get to the point where backend rtx costs
> functions won't have
> to handle rtxes that don't match down to any patterns/expanders we have. Or
> at least limit such cases.
> A case dealt with in this patch is QImode PLUS. We don't actually generate
> or handle these anywhere in
> the arm backend *except* in sync.md where, for example,
> atomic_<sync_optab><mode> matches:
> (set (match_operand:QHSD 0 "mem_noofs_operand" "+Ua")
>     (unspec_volatile:QHSD
>       [(syncop:QHSD (match_dup 0)
>          (match_operand:QHSD 1 "<atomic_op_operand>" "<atomic_op_str>"))
>        (match_operand:SI 2 "const_int_operand")]        ;; model
>       VUNSPEC_ATOMIC_OP))
>
> Here QHSD can contain QImode and HImode while syncop can be PLUS.
> Now immediately during splitting in arm_split_atomic_op we convert that
> QImode PLUS into an SImode one, so we never actually generate any kind of
> QImode add operations
> (how would we? we don't have define_insns for such things) but the RTL
> optimisers will get a hold
> of the UNSPEC_VOLATILE in the meantime and ask for it's cost (for example,
> cse when building libatomic).
> Currently we don't handle UNSPEC_VOLATILE (VUNSPEC_ATOMIC_OP) so the arm rtx
> costs function just recurses
> into the QImode PLUS that I'd like to avoid.
> This patch stops that by passing the VUNSPEC_ATOMIC_OP into arm_unspec_cost
> and handling it there
> (very straightforwardly just returning COSTS_N_INSNS (2); there's no
> indication that we want to do anything
> smarter here) and stopping the recursion.
>
> This is a small step in the direction of not having to care about obviously
> useless rtxes in the backend.
> The astute reader might notice that in sync.md we also have the pattern
> atomic_fetch_<sync_optab><mode>
> which expands to/matches this:
> (set (match_operand:QHSD 0 "s_register_operand" "=&r")
>     (match_operand:QHSD 1 "mem_noofs_operand" "+Ua"))
>    (set (match_dup 1)
>     (unspec_volatile:QHSD
>       [(syncop:QHSD (match_dup 1)
>          (match_operand:QHSD 2 "<atomic_op_operand>" "<atomic_op_str>"))
>        (match_operand:SI 3 "const_int_operand")]        ;; model
>       VUNSPEC_ATOMIC_OP))
>
>
> Here the QImode PLUS is in a PARALLEL together with the UNSPEC, so it might
> have rtx costs called on it
> as well. This will always be a (plus (reg) (mem)) rtx, which is unlike any
> other normal rtx we generate
> in the arm backend. I'll try to get a patch to handle that case, but I'm
> still thinking on how to best
> do that.
>
> Tested arm-none-eabi, I didn't see any codegen differences in some compiled
> codebases.
>
> Ok for trunk?

OK

Ramana


>
> P.S. I know that expmed creates all kinds of irregular rtxes and asks for
> their costs. I'm hoping to clean that
> up at some point...
>
> 2015-04-20  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     * config/arm/arm.c (arm_new_rtx_costs): Handle UNSPEC_VOLATILE.
>     (arm_unspec_cost): Allos UNSPEC_VOLATILE.  Do not recurse inside
>     unknown unspecs.