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