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

[Kyrill Tkachov <kyrylo.tkachov@arm.com>]

Ping.
https://gcc.gnu.org/ml/gcc-patches/2015-04/msg01047.html

Thanks,
Kyrill

On 20/04/15 17:28, Kyrill Tkachov 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?
>
> 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.