Karl Meakin <karl.mea...@arm.com> writes:
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index be5a97294dd..1d4ae73a963 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -944,16 +944,50 @@ static const char *
> svpattern_token (enum aarch64_svpattern pattern)
> {
> switch (pattern)
> {
> #define CASE(UPPER, LOWER, VALUE) case AARCH64_SV_##UPPER: return #LOWER;
> AARCH64_FOR_SVPATTERN (CASE)
> #undef CASE
> case AARCH64_NUM_SVPATTERNS:
> break;
> }
> gcc_unreachable ();
> }
>
> +/* Return true if rhs is an operand suitable for a CB<cc> (immediate)
> + * instruction. */
Formatting nit, sorry, but: GCC style is not to have leading "*"s
on subsequent lines.
> [...]
> @@ -747,13 +770,65 @@ (define_expand "cbranch<mode>4"
> (define_expand "cbranchcc4"
> [(set (pc) (if_then_else (match_operator 0 "aarch64_comparison_operator"
> [(match_operand 1 "cc_register")
> (match_operand 2 "const0_operand")])
> (label_ref (match_operand 3))
> (pc)))]
> ""
> ""
> )
>
> +;; Emit a `CB<cond> (register)` or `CB<cond> (immediate)` instruction.
> +(define_insn "aarch64_cb<GPI:mode>"
> + [(set (pc) (if_then_else (match_operator 0 "aarch64_comparison_operator"
> + [(match_operand:GPI 1 "register_operand" "r")
> + (match_operand:GPI 2 "aarch64_cb_operand" "ri")])
We can't use "i" here. For operands like this one that are subject
to register allocation and reloading, the constraints must only accept
things that the predicates also accept.
Here, "i" accepts all "legitimate constants", which for AArch64 includes
all CONST_INTs. But aarch64_cb_operand is much more restricted than that.
The reason for this rule is that the register allocator only needs to look
at constraints, not predicates, when handling an existing instruction.
For example, say we have a comparison against 10000 in a loop. We would
need to force the 10000 into a register, since it is out of range of
both the compare-and-branch instructions and the compare instructions.
The optimisers might then decide to hoist the register out of the loop.
But if register pressure in the loop is high, the register allocator
might decide not to allocate the register and instead replace all uses
with 10000. It would then try to make 10000 match the constraints.
With "i", 10000 will match as-is, and so we'd end up with an invalid
instruction. This might lead to an ICE later, or it might go undetected
(by GCC) and produce invalid assembly.
The constraints should therefore have the same immediate range as the
predicates. In the earlier review, I suggested that we do this by
replacing the match_operator with a code iterator, and then use
a code attribute to select the right constraint for each code.
For example, "le" would map to a constraint with the range of [-1, 62],
"lt" would map to a constraint with the range [0, 63], and "ge" would
map to a constraint with the range [1, 64].
> + (label_ref (match_operand 3))
> + (pc)))]
> + "TARGET_CMPBR"
> + "cb%m0\\t%<w>1, %<w>2, %l3";
> + [(set_attr "type" "branch")
> + (set (attr "length")
> + (if_then_else (and (ge (minus (match_dup 3) (pc))
> + (const_int BRANCH_LEN_N_1Kib))
> + (lt (minus (match_dup 3) (pc))
> + (const_int BRANCH_LEN_P_1Kib)))
> + (const_int 4)
> + (const_int 8)))
> + (set (attr "far_branch")
> + (if_then_else (and (ge (minus (match_dup 3) (pc))
> + (const_int BRANCH_LEN_N_1Kib))
> + (lt (minus (match_dup 3) (pc))
> + (const_int BRANCH_LEN_P_1Kib)))
> + (const_string "no")
> + (const_string "yes")))]
> [...]
> --- a/gcc/config/aarch64/iterators.md
> +++ b/gcc/config/aarch64/iterators.md
> @@ -2879,6 +2879,11 @@ (define_code_attr cmp_op [(lt "lt")
> (geu "hs")
> (gtu "hi")])
>
> +(define_mode_attr cmpbr_suffix [(QI "b")
> + (HI "h")
> + (SI "")
> + (DI "")])
> +
Very minor, sorry, but we might as well drop the SI and DI entries,
since they're not used.
Thanks,
Richard
