Richard Henderson <richard.hender...@linaro.org> writes: > On 8/8/25 21:18, Richard Sandiford wrote: >>> +(define_insn "*aarch64_cb<code><mode>" >>> + [(set (pc) (if_then_else >>> + (INT_CMP >>> + (match_operand:GPI 0 "register_operand" "r") >>> + (match_operand:GPI 1 >>> + "<cmpbr_imm_predicate>" "r<cmpbr_imm_constraint>")) >> >> An alternative to adding a new code attribute would be to embed >> the immediate constraint in the predicate name, to ensure that >> the two don't get out of sync. We've done that for things like >> aarch64_sve_cmp_<sve_imm_con>_operand. It does make the predicate >> name slightly less mnemonic though... > > Good idea. I was uncomfortable having to replicate the range info twice, > much less the > third time into the predicate. > > The other idea I had was a match_operator which also examines XEXP (op, 1). > But that > means we lose the macro expansion of the strings in the body. %m/%M doesn't > quite work > for CMPBR, because it emits cs/cc instead of hs/lo. I suspect that all > assemblers would > accept either for existing uses (B.cond, CCMP, CSEL, CSET), but I didn't want > to try.
Yeah, it was because of the cs/cc vs. hs/lo thing that we went for the current approach. As far as the range check goes, I'm not sure whether a match_operator that checks XEXP (x, 1) would be functionally any different from the aarch64_cb_rhs test in the current C++ condition. I suppose a disadvantage of both is that, because the operand predicate isn't doing the range checking itself, the usual create_*_operand/expand_insn sequence won't enforce the correct range. The version in the patch avoids that. Richard
