Wilco Dijkstra <[email protected]> writes:
> Hi,
>
> Add support for fused compare with branch. Rename the existing
> AARCH64_FUSE_CMP_BRANCH to ALU_BRANCH, and AARCH64_FUSE_ALU_BRANCH
> to ALU_CBZ to make it clear what is being fused.
This night have been easier to review as three patches:
(1) rename ALU_BRANCH to ALU_CBZ
(2) rename CMP_BRANCH to ALU_BRANCH
(3) add back CMP_BRANCH with more accurate semantics
But what uses CMP_BRANCH after the patch? It looked like you renamed
all existing uses and didn't add any new ones.
> @@ -20363,7 +20363,14 @@ aarch_macro_fusion_pair_p (rtx_insn *prev, rtx_insn
> *curr)
> }
> }
>
> + /* Fuse compare (CMP/CMN/TST/BICS) and conditional branch. */
> if (aarch64_fusion_enabled_p (AARCH64_FUSE_CMP_BRANCH)
> + && prev_set && curr_set && any_condjump_p (curr)
> + && reg_referenced_p (SET_DEST (curr_set), PATTERN (curr)))
^^^^^^^^
Looks like this should be prev_set. But this condition will trigger
for any prev_insn that is only being kept around for its effect on the
flags, not just CMP/CMN/TST/BICS. If it's only supposed to be those
four insns then I think we should test for them explicitly.
Thanks,
Richard
> + return true;
> +
> + /* Fuse flag-setting ALU instructions and conditional branch. */
> + if (aarch64_fusion_enabled_p (AARCH64_FUSE_ALU_BRANCH)
> && any_condjump_p (curr))
> {
> unsigned int condreg1, condreg2;
