[v3,4/4] ifcvt: Remove obsolete code for subreg handling in noce_convert_multiple_sets

Message ID 20230830101400.1539313-5-manolis.tsamis@vrull.eu
State Unresolved
Headers
Series ifcvt: Allow if conversion of arithmetic in basic blocks with multiple sets |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Manolis Tsamis Aug. 30, 2023, 10:14 a.m. UTC
  This code used to handle register replacement issues with SUBREG before
simplify_replace_rtx was introduced. This should not be needed anymore as
new_val has the correct mode and that should be preserved by
simplify_replace_rtx.

gcc/ChangeLog:

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

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

(no changes since v1)

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

Comments

Manolis Tsamis Nov. 13, 2023, 12:43 p.m. UTC | #1
Yes, my finding back then was that this is leftover code from the
initial implementation, nothing to do with the rest of the changes.
I will first re-evaluate this and test it separately from the other
series. If all is good I'll let you know so we can proceed.

Manolis

On Sat, Nov 11, 2023 at 12:03 AM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 8/30/23 04:14, Manolis Tsamis wrote:
> > This code used to handle register replacement issues with SUBREG before
> > simplify_replace_rtx was introduced. This should not be needed anymore as
> > new_val has the correct mode and that should be preserved by
> > simplify_replace_rtx.
> >
> > gcc/ChangeLog:
> >
> >       * ifcvt.cc (noce_convert_multiple_sets_1): Remove old code.
> So is it the case that this code is supposed to no longer be needed as a
> result of your kit or it is unnecessary independent of patches 1..3?  If
> the latter then it's OK for the trunk now.
>
> Jeff
  

Patch

diff --git a/gcc/ifcvt.cc b/gcc/ifcvt.cc
index ecc0cbabef9..3b4b873612c 100644
--- a/gcc/ifcvt.cc
+++ b/gcc/ifcvt.cc
@@ -3449,44 +3449,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);
-	}
-
       /* We have identified swap-style idioms before.  A normal
 	 set will need to be a cmov while the first instruction of a swap-style
 	 idiom can be a regular move.  This helps with costing.  */