[PATCH v2] ifcvt: Remove obsolete code for subreg handling in noce_convert_multiple_sets

[PATCH v2] ifcvt: Remove obsolete code for subreg handling in noce_convert_multiple_sets

[Manolis Tsamis]

This code used to handle SUBREG for register replacement when ifcvt was doing
the replacements manually. This special handling is not needed anymore
because simplify_replace_rtx is used for the replacements and it properly
handles these cases.

gcc/ChangeLog:

	* ifcvt.cc (noce_convert_multiple_sets_1): Remove old code.

Signed-off-by: Manolis Tsamis <manolis.tsamis@vrull.eu>
---

Changes in v2:
        - Made standalone patch.

 gcc/ifcvt.cc | 38 --------------------------------------
 1 file changed, 38 deletions(-)

diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
index a0af553b9ff..be42609a848 100644
--- a/gcc/ifcvt.cc
+++ b/gcc/ifcvt.cc
@@ -3492,44 +3492,6 @@ noce_convert_multiple_sets_1 (struct noce_if_info *if_info,
       if (if_info->then_else_reversed)
 	std::swap (old_val, new_val);
 
-
-      /* We allow simple lowpart register subreg SET sources in
-	 bb_ok_for_noce_convert_multiple_sets.  Be careful when processing
-	 sequences like:
-	 (set (reg:SI r1) (reg:SI r2))
-	 (set (reg:HI r3) (subreg:HI (r1)))
-	 For the second insn new_val or old_val (r1 in this example) will be
-	 taken from the temporaries and have the wider mode which will not
-	 match with the mode of the other source of the conditional move, so
-	 we'll end up trying to emit r4:HI = cond ? (r1:SI) : (r3:HI).
-	 Wrap the two cmove operands into subregs if appropriate to prevent
-	 that.  */
-
-      if (!CONSTANT_P (new_val)
-	  && GET_MODE (new_val) != GET_MODE (temp))
-	{
-	  machine_mode src_mode = GET_MODE (new_val);
-	  machine_mode dst_mode = GET_MODE (temp);
-	  if (!partial_subreg_p (dst_mode, src_mode))
-	    {
-	      end_sequence ();
-	      return false;
-	    }
-	  new_val = lowpart_subreg (dst_mode, new_val, src_mode);
-	}
-      if (!CONSTANT_P (old_val)
-	  && GET_MODE (old_val) != GET_MODE (temp))
-	{
-	  machine_mode src_mode = GET_MODE (old_val);
-	  machine_mode dst_mode = GET_MODE (temp);
-	  if (!partial_subreg_p (dst_mode, src_mode))
-	    {
-	      end_sequence ();
-	      return false;
-	    }
-	  old_val = lowpart_subreg (dst_mode, old_val, src_mode);
-	}
-
       /* Try emitting a conditional move passing the backend the
 	 canonicalized comparison.  The backend is then able to
 	 recognize expressions like
-- 
2.34.1

Re: [PATCH v2] ifcvt: Remove obsolete code for subreg handling in noce_convert_multiple_sets

[Manolis Tsamis]

Bootstrapped and tested on x86 and aarch64. This only assumes that the
mode of what simplify_replace_rtx returns is the same with its first
argument.

Thanks,
Manolis

On Tue, Nov 21, 2023 at 8:04 PM Manolis Tsamis <manolis.tsamis@vrull.eu> wrote:
>
> This code used to handle SUBREG for register replacement when ifcvt was doing
> the replacements manually. This special handling is not needed anymore
> because simplify_replace_rtx is used for the replacements and it properly
> handles these cases.
>
> gcc/ChangeLog:
>
>         * ifcvt.cc (noce_convert_multiple_sets_1): Remove old code.
>
> Signed-off-by: Manolis Tsamis <manolis.tsamis@vrull.eu>
> ---
>
> Changes in v2:
>         - Made standalone patch.
>
>  gcc/ifcvt.cc | 38 --------------------------------------
>  1 file changed, 38 deletions(-)
>
> diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
> index a0af553b9ff..be42609a848 100644
> --- a/gcc/ifcvt.cc
> +++ b/gcc/ifcvt.cc
> @@ -3492,44 +3492,6 @@ noce_convert_multiple_sets_1 (struct noce_if_info *if_info,
>        if (if_info->then_else_reversed)
>         std::swap (old_val, new_val);
>
> -
> -      /* We allow simple lowpart register subreg SET sources in
> -        bb_ok_for_noce_convert_multiple_sets.  Be careful when processing
> -        sequences like:
> -        (set (reg:SI r1) (reg:SI r2))
> -        (set (reg:HI r3) (subreg:HI (r1)))
> -        For the second insn new_val or old_val (r1 in this example) will be
> -        taken from the temporaries and have the wider mode which will not
> -        match with the mode of the other source of the conditional move, so
> -        we'll end up trying to emit r4:HI = cond ? (r1:SI) : (r3:HI).
> -        Wrap the two cmove operands into subregs if appropriate to prevent
> -        that.  */
> -
> -      if (!CONSTANT_P (new_val)
> -         && GET_MODE (new_val) != GET_MODE (temp))
> -       {
> -         machine_mode src_mode = GET_MODE (new_val);
> -         machine_mode dst_mode = GET_MODE (temp);
> -         if (!partial_subreg_p (dst_mode, src_mode))
> -           {
> -             end_sequence ();
> -             return false;
> -           }
> -         new_val = lowpart_subreg (dst_mode, new_val, src_mode);
> -       }
> -      if (!CONSTANT_P (old_val)
> -         && GET_MODE (old_val) != GET_MODE (temp))
> -       {
> -         machine_mode src_mode = GET_MODE (old_val);
> -         machine_mode dst_mode = GET_MODE (temp);
> -         if (!partial_subreg_p (dst_mode, src_mode))
> -           {
> -             end_sequence ();
> -             return false;
> -           }
> -         old_val = lowpart_subreg (dst_mode, old_val, src_mode);
> -       }
> -
>        /* Try emitting a conditional move passing the backend the
>          canonicalized comparison.  The backend is then able to
>          recognize expressions like
> --
> 2.34.1
>

Re: [PATCH v2] ifcvt: Remove obsolete code for subreg handling in noce_convert_multiple_sets

[Jeff Law]

On 11/21/23 11:04, Manolis Tsamis wrote:
> This code used to handle SUBREG for register replacement when ifcvt was doing
> the replacements manually. This special handling is not needed anymore
> because simplify_replace_rtx is used for the replacements and it properly
> handles these cases.
> 
> gcc/ChangeLog:
> 
> 	* ifcvt.cc (noce_convert_multiple_sets_1): Remove old code.
OK.
jeff

Re: [PATCH v2] ifcvt: Remove obsolete code for subreg handling in noce_convert_multiple_sets

[Philipp Tomsich]

Applied to master, thanks!
Philipp,


On Thu, 23 Nov 2023 at 04:48, Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 11/21/23 11:04, Manolis Tsamis wrote:
> > This code used to handle SUBREG for register replacement when ifcvt was doing
> > the replacements manually. This special handling is not needed anymore
> > because simplify_replace_rtx is used for the replacements and it properly
> > handles these cases.
> >
> > gcc/ChangeLog:
> >
> >       * ifcvt.cc (noce_convert_multiple_sets_1): Remove old code.
> OK.
> jeff