[PATCH][AArch64] Add support for fused compare and branch

Richard Sandiford richard.sandiford@arm.com
Fri Nov 29 16:48:00 GMT 2019


Wilco Dijkstra <Wilco.Dijkstra@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;



More information about the Gcc-patches mailing list