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