Hi Steven,

> >  This would allow us to, for example, disable the predicable variant
> of an
> > insn
> > based on a non-constant variable.
> 
> Is there a reason why you can't use attribute "enabled" for this?

The problem stems from the fact that the "predicable" attribute cannot have
a dynamic value.

Consider a pattern from the arm backend (config/arm/sync.md):

(define_insn "atomic_loaddi_1"
  [(set (match_operand:DI 0 "s_register_operand" "=r")
        (unspec:DI [(match_operand:DI 1 "mem_noofs_operand" "Ua")]
                   UNSPEC_LL))]
  "TARGET_HAVE_LDREXD && ARM_DOUBLEWORD_ALIGN"
  "ldrexd%?\t%0, %H0, %C1"
  [(set_attr "predicable" "yes")])

Now, say I want to disable the cond_exec variant of this when
TARGET_RESTRICT_CE is set.
If I wanted to do it with the "enabled" attribute I would have to do
something like

(define_insn "atomic_loaddi_1"
  [(set (match_operand:DI 0 "s_register_operand" "=r,r")
        (unspec:DI [(match_operand:DI 1 "mem_noofs_operand" "Ua,Ua")]
                   UNSPEC_LL))]
  "TARGET_HAVE_LDREXD && ARM_DOUBLEWORD_ALIGN"
  "ldrexd%?\t%0, %H0, %C1"
  [(set_attr "predicable" "yes,no")
   (set_attr_alternative "enabled" [(if_then_else (match_test
"TARGET_RESTRICT_CE")
                                                  (const_string "no")
                                                  (const_string "yes"))
                                    (if_then_else (match_test
"TARGET_RESTRICT_CE")
                                                  (const_string "yes")
                                                  (const_string "no")) ])])

A slightly cleaner approach would be to define an extra attribute:

(define_attr "control_attr" "yes,no" (const_string "yes"))

(define_attr "enabled" "no,yes"
  (cond [(and (eq_attr "control_attr" "no")
              (match_test "TARGET_RESTRICT_CE"))
         (const_string "no")]
         (const_string "yes"))

(define_insn "atomic_loaddi_1"
  [(set (match_operand:DI 0 "s_register_operand" "=r,r")
        (unspec:DI [(match_operand:DI 1 "mem_noofs_operand" "Ua,Ua")]
                   UNSPEC_LL))]
  "TARGET_HAVE_LDREXD && ARM_DOUBLEWORD_ALIGN"
  "ldrexd%?\t%0, %H0, %C1"
  [(set_attr "predicable" "yes,no")
   (set_attr "control_attr" "no,yes")])

This helps, but still requires me to duplicate the constraint string, which
for more complicated patterns
(for example, the arm_addsi3 pattern in arm.md has 14 alternatives) would
create a big mess.

With my proposed scheme we would have the setup:

(define_attr "predicated" "yes,no" (const_string "no"))

(define_attr "control_attr" "yes,no" (const_string "yes"))


(define_attr "enabled" "no,yes"
  (cond [(and (eq_attr "control_attr" "no")
              (and (eq_attr "predicated" "yes")
                   (match_test "TARGET_RESTRICT_CE")))
         (const_string "no")]
         (const_string "yes"))

(define_cond_exec
  [(match_operator 0 "arm_comparison_operator"
    [(match_operand 1 "cc_register" "")
     (const_int 0)])]
  ""
  ""
[(set_attr "predicated" "yes")])

and the above pattern would now be:

(define_insn "atomic_loaddi_1"
  [(set (match_operand:DI 0 "s_register_operand" "=r")
        (unspec:DI [(match_operand:DI 1 "mem_noofs_operand" "Ua")]
                   UNSPEC_LL))]
  "TARGET_HAVE_LDREXD && ARM_DOUBLEWORD_ALIGN"
  "ldrexd%?\t%0, %H0, %C1"
  [(set_attr "predicable" "yes")
   (set_attr "control_var" "no")]) // <<<<<


We only add one attribute to the pattern.
Any other approach I may have missed?

Thanks,
Kyrill




Reply via email to