Re: [PATCH 2/2] or1k: Improve If-Conversion by delaying cbranch splits

[Stafford Horne <shorne@gmail.com>]

On Thu, Jun 19, 2025 at 02:14:26PM +0100, Stafford Horne wrote:
> When working on PR120587 I found that the ce1 pass was not able to
> properly optimize branches on OpenRISC.  This is because of the early
> splitting of "compare" and "branch" instructions during the expand pass.
> 
> Convert the cbranch* instructions from define_expand to
> define_insn_and_split.  This dalays the instruction split until after
> the ce1 pass is done giving ce1 the best opportunity to perform the
> optimizations on the original form of cbranch<mode>4 instructions.
> 
> gcc/ChangeLog:
> 
> 	* config/or1k/or1k.cc (or1k_noce_conversion_profitable_p): New
> 	function.
> 	(or1k_is_cmov_insn): New function.
> 	(TARGET_NOCE_CONVERSION_PROFITABLE_P): Define macro.
> 	* config/or1k/or1k.md (cbranchsi4): Convert to insn_and_split.
> 	(cbranch<mode>4): Convert to insn_and_split.
> 
> Signed-off-by: Stafford Horne <shorne@gmail.com>
> ---
>  gcc/config/or1k/or1k.cc | 54 +++++++++++++++++++++++++++++++++++++++++
>  gcc/config/or1k/or1k.md | 32 ++++++++++++++++++++++--
>  2 files changed, 84 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/config/or1k/or1k.cc b/gcc/config/or1k/or1k.cc
> index f1c92c6bf6c..36c16a51d44 100644
> --- a/gcc/config/or1k/or1k.cc
> +++ b/gcc/config/or1k/or1k.cc
> @@ -1654,6 +1654,60 @@ or1k_rtx_costs (rtx x, machine_mode mode, int outer_code, int /* opno */,
>  #undef TARGET_RTX_COSTS
>  #define TARGET_RTX_COSTS or1k_rtx_costs
>  
> +static bool
> +or1k_is_cmov_insn (rtx_insn *seq)
> +{
> +  rtx_insn *curr_insn = seq;
> +  rtx set = NULL_RTX;
> +
> +  /* The pattern may start with a simple set with register operands.  Skip
> +     through any of those.  */
> +  while (curr_insn)
> +    {
> +      set = single_set (curr_insn);
> +      if (!set
> +	  || !REG_P (SET_DEST (set)))
> +	return false;
> +
> +      /* If it's not a simple reg or immediate break.  */
> +      if (REG_P (SET_SRC (set)) || CONST_INT_P (SET_SRC (set)))
> +        curr_insn = NEXT_INSN (curr_insn);
> +      else
> +	break;

Indentation issue.

> +    }
> +
> +  /* The next instruction should be a compare.  OpenRISC has many operators used
> +     for comparison so skip and confirm the next is IF_THEN_ELSE.  */
> +  curr_insn = NEXT_INSN (curr_insn);

Need to check curr_insn before calling NEXT_INSN () to avoid failures.

> +  if (!curr_insn)
> +    return false;
> +
> +  /* And the last instruction should be an IF_THEN_ELSE.  */
> +  set = single_set (curr_insn);
> +  if (!set
> +      || !REG_P (SET_DEST (set))
> +      || GET_CODE (SET_SRC (set)) != IF_THEN_ELSE)
> +    return false;
> +
> +  return !NEXT_INSN (curr_insn);
> +}
> +
> +/* Implement TARGET_NOCE_CONVERSION_PROFITABLE_P.  We detect if the conversion
> +   resulted in a l.cmov instruction and if so we consider it more profitable than
> +   branch instructions.  */
> +
> +static bool
> +or1k_noce_conversion_profitable_p (rtx_insn *seq,
> +				    struct noce_if_info *if_info)
> +{
> +  if (TARGET_CMOV)
> +    return or1k_is_cmov_insn (seq);
> +
> +  return default_noce_conversion_profitable_p (seq, if_info);
> +}
> +
> +#undef TARGET_NOCE_CONVERSION_PROFITABLE_P
> +#define TARGET_NOCE_CONVERSION_PROFITABLE_P or1k_noce_conversion_profitable_p
>  
>  /* A subroutine of the atomic operation splitters.  Jump to LABEL if
>     COND is true.  Mark the jump as unlikely to be taken.  */
> diff --git a/gcc/config/or1k/or1k.md b/gcc/config/or1k/or1k.md
> index a30cc18892d..bf7125375ac 100644
> --- a/gcc/config/or1k/or1k.md
> +++ b/gcc/config/or1k/or1k.md
> @@ -609,7 +609,7 @@
>  ;; Branch instructions
>  ;; -------------------------------------------------------------------------
>  
> -(define_expand "cbranchsi4"
> +(define_insn_and_split "cbranchsi4"
>    [(set (pc)
>  	(if_then_else
>  	  (match_operator 0 "comparison_operator"
> @@ -618,13 +618,27 @@
>  	  (label_ref (match_operand 3 "" ""))
>  	  (pc)))]
>    ""
> +  "#"
> +  "&& 1"
> +  [(const_int 0)]
>  {
> +  rtx label;
> +
> +  /* Generate *scc */
>    or1k_expand_compare (operands);
> +  /* Generate *cbranch */
> +  label = gen_rtx_LABEL_REF (VOIDmode, operands[3]);
> +  emit_jump_insn (gen_rtx_SET (pc_rtx,
> +			       gen_rtx_IF_THEN_ELSE (VOIDmode,
> +						     operands[0],
> +						     label,
> +						     pc_rtx)));
> +  DONE;
>  })
>  
>  ;; Support FP branching
>  
> -(define_expand "cbranch<F:mode>4"
> +(define_insn_and_split "cbranch<F:mode>4"
>    [(set (pc)
>  	(if_then_else
>  	  (match_operator 0 "fp_comparison_operator"
> @@ -633,8 +647,22 @@
>  	  (label_ref (match_operand 3 "" ""))
>  	  (pc)))]
>    "TARGET_HARD_FLOAT"
> +  "#"
> +  "&& 1"
> +  [(const_int 0)]
>  {
> +  rtx label;
> +
> +  /* Generate *scc */
>    or1k_expand_compare (operands);
> +  /* Generate *cbranch */
> +  label = gen_rtx_LABEL_REF (VOIDmode, operands[3]);
> +  emit_jump_insn (gen_rtx_SET (pc_rtx,
> +			       gen_rtx_IF_THEN_ELSE (VOIDmode,
> +						     operands[0],
> +						     label,
> +						     pc_rtx)));
> +  DONE;
>  })
>  
>  (define_insn "*cbranch"
> -- 
> 2.49.0
>