Wilco Dijkstra <wilco.dijks...@arm.com> 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;