[PATCH 0/2] OpenRISC fixes for PR120587

[PATCH 0/2] OpenRISC fixes for PR120587

[Stafford Horne]

This is a small series to fix If-Conversion on OpenRISC after the build broken
with recent subreg changes.

Stafford Horne (2):
  or1k: Implement *extendbisi* to fix ICE in convert_mode_scalar
    [PR120587]
  or1k: Improve If-Conversion by delaying cbranch splits

 gcc/config/or1k/or1k.cc       | 54 +++++++++++++++++++++++++++++++++
 gcc/config/or1k/or1k.md       | 57 +++++++++++++++++++++++++++++++++--
 gcc/config/or1k/predicates.md |  4 +++
 3 files changed, 113 insertions(+), 2 deletions(-)

-- 
2.49.0

[PATCH 1/2] or1k: Implement *extendbisi* to fix ICE in convert_mode_scalar [PR120587]

[Stafford Horne]

After commit 2dcc6dbd8a0 ("emit-rtl: Use simplify_subreg_regno to
validate hardware subregs [PR119966]") the OpenRISC port is broken
again.

Add extend* iinstruction patterns for the SR_F pseudo registers to avoid
having to use the subreg conversions which no longer work.

gcc/ChangeLog:

	PR target/120587
	* config/or1k/or1k.md (zero_extendbisi2_sr_f): New expand.
	(extendbisi2_sr_f): New expand.
	* config/or1k/predicates.md (sr_f_reg_operand): New predicate.

Signed-off-by: Stafford Horne <shorne@gmail.com>
---
 gcc/config/or1k/or1k.md       | 25 +++++++++++++++++++++++++
 gcc/config/or1k/predicates.md |  4 ++++
 2 files changed, 29 insertions(+)

diff --git a/gcc/config/or1k/or1k.md b/gcc/config/or1k/or1k.md
index 627e40084b3..a30cc18892d 100644
--- a/gcc/config/or1k/or1k.md
+++ b/gcc/config/or1k/or1k.md
@@ -515,6 +515,31 @@
 	(ne:SI (reg:BI SR_F_REGNUM) (const_int 0)))]
   "")
 
+;; Allowing "extending" the BImode SR_F to a general register
+;; avoids 'convert_mode_scalar' from trying to do subregging
+;; which we don't have support for.
+;; We require signed and unsigned extend instructions because
+;; signed comparisons require signed extention, but for SR_F
+;; it doesn't matter.
+
+(define_expand "zero_extendbisi2_sr_f"
+  [(set (match_operand:SI 0 "register_operand" "")
+	(zero_extend:SI (match_operand:BI 1 "sr_f_reg_operand" "")))]
+  ""
+{
+  emit_insn(gen_sne_sr_f (operands[0]));
+  DONE;
+})
+
+(define_expand "extendbisi2_sr_f"
+  [(set (match_operand:SI 0 "register_operand" "")
+	(sign_extend:SI (match_operand:BI 1 "sr_f_reg_operand" "")))]
+  ""
+{
+  emit_insn(gen_sne_sr_f (operands[0]));
+  DONE;
+})
+
 (define_insn_and_split "*scc"
   [(set (match_operand:SI 0 "register_operand" "=r")
 	(match_operator:SI 1 "equality_comparison_operator"
diff --git a/gcc/config/or1k/predicates.md b/gcc/config/or1k/predicates.md
index 144f4d7b577..7ccfd09985d 100644
--- a/gcc/config/or1k/predicates.md
+++ b/gcc/config/or1k/predicates.md
@@ -60,6 +60,10 @@
     (and (match_operand 0 "register_operand")
 	 (match_test "TARGET_ROR"))))
 
+(define_predicate "sr_f_reg_operand"
+  (and (match_operand 0 "register_operand")
+       (match_test "REGNO (op) == SR_F_REGNUM")))
+
 (define_predicate "call_insn_operand"
   (ior (and (match_code "symbol_ref")
 	    (match_test "!TARGET_CMODEL_LARGE"))
-- 
2.49.0

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

[Stafford Horne]

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;
+    }
+
+  /* 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);
+  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

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

[Stafford Horne]

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
>