[middle-end PATCH take #2] Only call targetm.truly_noop_truncation for truncations.

[middle-end PATCH take #2] Only call targetm.truly_noop_truncation for truncations.

[Roger Sayle]

[-- Attachment #1: Type: text/plain, Size: 5379 bytes --]


Very many thanks (and a Happy New Year) to the pre-commit
patch testing folks at linaro.org.   Their testing has revealed that
although my patch is clean on x86_64, it triggers some problems
on aarch64 and arm.  The issue (with the previous version of my
patch) is that these platforms require a paradoxical subreg to be
generated by the middle-end, where we were previously checking
for truly_noop_truncation.

This has been fixed (in revision 2) below.  Where previously I had:

@@ -66,7 +66,9 @@ gen_lowpart_general (machine_mode mode, rtx x)
       scalar_int_mode xmode;
       if (is_a <scalar_int_mode> (GET_MODE (x), &xmode)
          && GET_MODE_SIZE (xmode) <= UNITS_PER_WORD
-         && TRULY_NOOP_TRUNCATION_MODES_P (mode, xmode)
+         && (known_lt (GET_MODE_SIZE (mode), GET_MODE_SIZE (xmode))
+             ? TRULY_NOOP_TRUNCATION_MODES_P (mode, xmode)
+             : known_eq (GET_MODE_SIZE (mode), GET_MODE_SIZE (xmode)))
          && !reload_completed)
        return gen_lowpart_general (mode, force_reg (xmode, x));

the correct change is:

       scalar_int_mode xmode;
       if (is_a <scalar_int_mode> (GET_MODE (x), &xmode)
          && GET_MODE_SIZE (xmode) <= UNITS_PER_WORD
-         && TRULY_NOOP_TRUNCATION_MODES_P (mode, xmode)
+         && (known_ge (GET_MODE_SIZE (mode), GET_MODE_SIZE (xmode))
+             || TRULY_NOOP_TRUNCATION_MODES_P (mode, xmode))
          && !reload_completed)
        return gen_lowpart_general (mode, force_reg (xmode, x));

i.e. we only call TRULY_NOOP_TRUNCATION_MODES_P when we
know we have a truncation, but the behaviour of non-truncations
is preserved (no longer depends upon unspecified behaviour) and
gen_lowpart_general is called to create the paradoxical SUBREG.


This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
and make -k check, both with and without --target_board=unix{-m32}
with no new failures.  Ok for mainline?

Hopefully this revision tests cleanly on the linaro.org CI pipeline.

2023-12-31  Roger Sayle  <roger@nextmovesoftware.com>

gcc/ChangeLog
        * combine.cc (make_extraction): Confirm that OUTPREC is less than
        INPREC before calling TRULY_NOOP_TRUNCATION_MODES_P.
        * expmed.cc (store_bit_field_using_insv): Likewise.
        (extract_bit_field_using_extv): Likewise.
        (extract_bit_field_as_subreg): Likewise.
        * optabs-query.cc (get_best_extraction_insn): Likewise.
        * optabs.cc (expand_parity): Likewise.
        * rtlhooks.cc (gen_lowpart_general): Likewise.
        * simplify-rtx.cc (simplify_truncation): Disallow truncations
        to the same precision.
        (simplify_unary_operation_1) <case TRUNCATE>: Move optimization
        of truncations to the same mode earlier.


> -----Original Message-----
> From: Roger Sayle <roger@nextmovesoftware.com>
> Sent: 28 December 2023 15:35
> To: 'gcc-patches@gcc.gnu.org' <gcc-patches@gcc.gnu.org>
> Cc: 'Jeff Law' <jeffreyalaw@gmail.com>
> Subject: [middle-end PATCH] Only call targetm.truly_noop_truncation for
> truncations.
> 
> 
> The truly_noop_truncation target hook is documented, in target.def, as
"true if it
> is safe to convert a value of inprec bits to one of outprec bits (where
outprec is
> smaller than inprec) by merely operating on it as if it had only outprec
bits", i.e.
> the middle-end can use a SUBREG instead of a TRUNCATE.
> 
> What's perhaps potentially a little ambiguous in the above description is
whether
> it is the caller or the callee that's responsible for ensuring or checking
whether
> "outprec < inprec".  The name TRULY_NOOP_TRUNCATION_P, like
> SUBREG_PROMOTED_P, may be prone to being understood as a predicate that
> confirms that something is a no-op truncation or a promoted subreg, when
in fact
> the caller must first confirm this is a truncation/subreg and only then
call the
> "classification" macro.
> 
> Alas making the following minor tweak (for testing) to the i386 backend:
> 
> static bool
> ix86_truly_noop_truncation (poly_uint64 outprec, poly_uint64 inprec) {
>   gcc_assert (outprec < inprec);
>   return true;
> }
> 
> #undef TARGET_TRULY_NOOP_TRUNCATION
> #define TARGET_TRULY_NOOP_TRUNCATION ix86_truly_noop_truncation
> 
> reveals that there are numerous callers in middle-end that rely on the
default
> behaviour of silently returning true for any (invalid) input.
> These are fixed below.
> 
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and
> make -k check, both with and without --target_board=unix{-m32} with no new
> failures.  Ok for mainline?
> 
> 
> 2023-12-28  Roger Sayle  <roger@nextmovesoftware.com>
> 
> gcc/ChangeLog
>         * combine.cc (make_extraction): Confirm that OUTPREC is less than
>         INPREC before calling TRULY_NOOP_TRUNCATION_MODES_P.
>         * expmed.cc (store_bit_field_using_insv): Likewise.
>         (extract_bit_field_using_extv): Likewise.
>         (extract_bit_field_as_subreg): Likewise.
>         * optabs-query.cc (get_best_extraction_insn): Likewise.
>         * optabs.cc (expand_parity): Likewise.
>         * rtlhooks.cc (gen_lowpart_general): Likewise.
>         * simplify-rtx.cc (simplify_truncation): Disallow truncations
>         to the same precision.
>         (simplify_unary_operation_1) <case TRUNCATE>: Move optimization
>         of truncations to the same mode earlier.
> 
> 
> Thanks in advance,
> Roger
> --


[-- Attachment #2: patchnt3.txt --]
[-- Type: text/plain, Size: 5665 bytes --]

diff --git a/gcc/combine.cc b/gcc/combine.cc
index f2c64a9..5aa2f57 100644
--- a/gcc/combine.cc
+++ b/gcc/combine.cc
@@ -7613,7 +7613,8 @@ make_extraction (machine_mode mode, rtx inner, HOST_WIDE_INT pos,
 	   && (pos == 0 || REG_P (inner))
 	   && (inner_mode == tmode
 	       || !REG_P (inner)
-	       || TRULY_NOOP_TRUNCATION_MODES_P (tmode, inner_mode)
+	       || (known_lt (GET_MODE_SIZE (tmode), GET_MODE_SIZE (inner_mode))
+		   && TRULY_NOOP_TRUNCATION_MODES_P (tmode, inner_mode))
 	       || reg_truncated_to_mode (tmode, inner))
 	   && (! in_dest
 	       || (REG_P (inner)
@@ -7856,6 +7857,8 @@ make_extraction (machine_mode mode, rtx inner, HOST_WIDE_INT pos,
       /* On the LHS, don't create paradoxical subregs implicitely truncating
 	 the register unless TARGET_TRULY_NOOP_TRUNCATION.  */
       if (in_dest
+	  && known_lt (GET_MODE_SIZE (GET_MODE (inner)),
+		       GET_MODE_SIZE (wanted_inner_mode))
 	  && !TRULY_NOOP_TRUNCATION_MODES_P (GET_MODE (inner),
 					     wanted_inner_mode))
 	return NULL_RTX;
diff --git a/gcc/expmed.cc b/gcc/expmed.cc
index 05331dd..6398bf9 100644
--- a/gcc/expmed.cc
+++ b/gcc/expmed.cc
@@ -651,6 +651,7 @@ store_bit_field_using_insv (const extraction_insn *insv, rtx op0,
      X) 0)) is (reg:N X).  */
   if (GET_CODE (xop0) == SUBREG
       && REG_P (SUBREG_REG (xop0))
+      && paradoxical_subreg_p (xop0)
       && !TRULY_NOOP_TRUNCATION_MODES_P (GET_MODE (SUBREG_REG (xop0)),
 					 op_mode))
     {
@@ -1585,7 +1586,11 @@ extract_bit_field_using_extv (const extraction_insn *extv, rtx op0,
 	 mode.  Instead, create a temporary and use convert_move to set
 	 the target.  */
       if (REG_P (target)
-	  && TRULY_NOOP_TRUNCATION_MODES_P (GET_MODE (target), ext_mode)
+	  && (known_lt (GET_MODE_SIZE (GET_MODE (target)),
+			GET_MODE_SIZE (ext_mode))
+	      ? TRULY_NOOP_TRUNCATION_MODES_P (GET_MODE (target), ext_mode)
+	      : known_eq (GET_MODE_SIZE (GET_MODE (target)),
+			  GET_MODE_SIZE (ext_mode)))
 	  && (temp = gen_lowpart_if_possible (ext_mode, target)))
 	{
 	  target = temp;
@@ -1626,7 +1631,9 @@ extract_bit_field_as_subreg (machine_mode mode, rtx op0,
   if (multiple_p (bitnum, BITS_PER_UNIT, &bytenum)
       && known_eq (bitsize, GET_MODE_BITSIZE (mode))
       && lowpart_bit_field_p (bitnum, bitsize, op0_mode)
-      && TRULY_NOOP_TRUNCATION_MODES_P (mode, op0_mode))
+      && (known_lt (GET_MODE_SIZE (mode), GET_MODE_SIZE (op0_mode))
+	  ? TRULY_NOOP_TRUNCATION_MODES_P (mode, op0_mode)
+	  : known_eq (GET_MODE_SIZE (mode), GET_MODE_SIZE (op0_mode))))
     return simplify_gen_subreg (mode, op0, op0_mode, bytenum);
   return NULL_RTX;
 }
diff --git a/gcc/optabs-query.cc b/gcc/optabs-query.cc
index 947ccef..f33253f 100644
--- a/gcc/optabs-query.cc
+++ b/gcc/optabs-query.cc
@@ -213,7 +213,7 @@ get_best_extraction_insn (extraction_insn *insn,
 	  FOR_EACH_MODE_FROM (mode_iter, mode)
 	    {
 	      mode = mode_iter.require ();
-	      if (maybe_gt (GET_MODE_SIZE (mode), GET_MODE_SIZE (field_mode))
+	      if (maybe_ge (GET_MODE_SIZE (mode), GET_MODE_SIZE (field_mode))
 		  || TRULY_NOOP_TRUNCATION_MODES_P (insn->field_mode,
 						    field_mode))
 		break;
diff --git a/gcc/optabs.cc b/gcc/optabs.cc
index 6a34276..fad0d59 100644
--- a/gcc/optabs.cc
+++ b/gcc/optabs.cc
@@ -2954,7 +2954,11 @@ expand_parity (scalar_int_mode mode, rtx op0, rtx target)
 	  if (temp)
 	    {
 	      if (mclass != MODE_INT
-		  || !TRULY_NOOP_TRUNCATION_MODES_P (mode, wider_mode))
+		  || (known_lt (GET_MODE_SIZE (mode),
+				GET_MODE_SIZE (wider_mode))
+		      ? !TRULY_NOOP_TRUNCATION_MODES_P (mode, wider_mode)
+		      : maybe_ne (GET_MODE_SIZE (mode),
+				  GET_MODE_SIZE (wider_mode))))
 		return convert_to_mode (mode, temp, 0);
 	      else
 		return gen_lowpart (mode, temp);
diff --git a/gcc/rtlhooks.cc b/gcc/rtlhooks.cc
index 989d3c9..c3313fd 100644
--- a/gcc/rtlhooks.cc
+++ b/gcc/rtlhooks.cc
@@ -66,7 +66,8 @@ gen_lowpart_general (machine_mode mode, rtx x)
       scalar_int_mode xmode;
       if (is_a <scalar_int_mode> (GET_MODE (x), &xmode)
 	  && GET_MODE_SIZE (xmode) <= UNITS_PER_WORD
-	  && TRULY_NOOP_TRUNCATION_MODES_P (mode, xmode)
+	  && (known_ge (GET_MODE_SIZE (mode), GET_MODE_SIZE (xmode))
+	      || TRULY_NOOP_TRUNCATION_MODES_P (mode, xmode))
 	  && !reload_completed)
 	return gen_lowpart_general (mode, force_reg (xmode, x));
 
diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
index f3745d8..27518f5 100644
--- a/gcc/simplify-rtx.cc
+++ b/gcc/simplify-rtx.cc
@@ -617,7 +617,7 @@ simplify_context::simplify_truncation (machine_mode mode, rtx op,
   unsigned int op_precision = GET_MODE_UNIT_PRECISION (op_mode);
   scalar_int_mode int_mode, int_op_mode, subreg_mode;
 
-  gcc_assert (precision <= op_precision);
+  gcc_assert (precision < op_precision);
 
   /* Optimize truncations of zero and sign extended values.  */
   if (GET_CODE (op) == ZERO_EXTEND
@@ -1207,6 +1207,10 @@ simplify_context::simplify_unary_operation_1 (rtx_code code, machine_mode mode,
       break;
 
     case TRUNCATE:
+      /* Check for useless truncation.  */
+      if (GET_MODE (op) == mode)
+	return op;
+
       /* Don't optimize (lshiftrt (mult ...)) as it would interfere
 	 with the umulXi3_highpart patterns.  */
       if (GET_CODE (op) == LSHIFTRT
@@ -1271,9 +1275,6 @@ simplify_context::simplify_unary_operation_1 (rtx_code code, machine_mode mode,
 	    return temp;
 	}
 
-      /* Check for useless truncation.  */
-      if (GET_MODE (op) == mode)
-	return op;
       break;
 
     case FLOAT_TRUNCATE:

Re: [middle-end PATCH take #2] Only call targetm.truly_noop_truncation for truncations.

[Richard Sandiford]

"Roger Sayle" <roger@nextmovesoftware.com> writes:
> Very many thanks (and a Happy New Year) to the pre-commit
> patch testing folks at linaro.org.   Their testing has revealed that
> although my patch is clean on x86_64, it triggers some problems
> on aarch64 and arm.  The issue (with the previous version of my
> patch) is that these platforms require a paradoxical subreg to be
> generated by the middle-end, where we were previously checking
> for truly_noop_truncation.
>
> This has been fixed (in revision 2) below.  Where previously I had:
>
> @@ -66,7 +66,9 @@ gen_lowpart_general (machine_mode mode, rtx x)
>        scalar_int_mode xmode;
>        if (is_a <scalar_int_mode> (GET_MODE (x), &xmode)
>           && GET_MODE_SIZE (xmode) <= UNITS_PER_WORD
> -         && TRULY_NOOP_TRUNCATION_MODES_P (mode, xmode)
> +         && (known_lt (GET_MODE_SIZE (mode), GET_MODE_SIZE (xmode))
> +             ? TRULY_NOOP_TRUNCATION_MODES_P (mode, xmode)
> +             : known_eq (GET_MODE_SIZE (mode), GET_MODE_SIZE (xmode)))
>           && !reload_completed)
>         return gen_lowpart_general (mode, force_reg (xmode, x));
>
> the correct change is:
>
>        scalar_int_mode xmode;
>        if (is_a <scalar_int_mode> (GET_MODE (x), &xmode)
>           && GET_MODE_SIZE (xmode) <= UNITS_PER_WORD
> -         && TRULY_NOOP_TRUNCATION_MODES_P (mode, xmode)
> +         && (known_ge (GET_MODE_SIZE (mode), GET_MODE_SIZE (xmode))
> +             || TRULY_NOOP_TRUNCATION_MODES_P (mode, xmode))
>           && !reload_completed)
>         return gen_lowpart_general (mode, force_reg (xmode, x));
>
> i.e. we only call TRULY_NOOP_TRUNCATION_MODES_P when we
> know we have a truncation, but the behaviour of non-truncations
> is preserved (no longer depends upon unspecified behaviour) and
> gen_lowpart_general is called to create the paradoxical SUBREG.
>
>
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
> and make -k check, both with and without --target_board=unix{-m32}
> with no new failures.  Ok for mainline?
>
> Hopefully this revision tests cleanly on the linaro.org CI pipeline.
>
> 2023-12-31  Roger Sayle  <roger@nextmovesoftware.com>
>
> gcc/ChangeLog
>         * combine.cc (make_extraction): Confirm that OUTPREC is less than
>         INPREC before calling TRULY_NOOP_TRUNCATION_MODES_P.
>         * expmed.cc (store_bit_field_using_insv): Likewise.
>         (extract_bit_field_using_extv): Likewise.
>         (extract_bit_field_as_subreg): Likewise.
>         * optabs-query.cc (get_best_extraction_insn): Likewise.
>         * optabs.cc (expand_parity): Likewise.
>         * rtlhooks.cc (gen_lowpart_general): Likewise.
>         * simplify-rtx.cc (simplify_truncation): Disallow truncations
>         to the same precision.
>         (simplify_unary_operation_1) <case TRUNCATE>: Move optimization
>         of truncations to the same mode earlier.
>
>
>> -----Original Message-----
>> From: Roger Sayle <roger@nextmovesoftware.com>
>> Sent: 28 December 2023 15:35
>> To: 'gcc-patches@gcc.gnu.org' <gcc-patches@gcc.gnu.org>
>> Cc: 'Jeff Law' <jeffreyalaw@gmail.com>
>> Subject: [middle-end PATCH] Only call targetm.truly_noop_truncation for
>> truncations.
>> 
>> 
>> The truly_noop_truncation target hook is documented, in target.def, as
> "true if it
>> is safe to convert a value of inprec bits to one of outprec bits (where
> outprec is
>> smaller than inprec) by merely operating on it as if it had only outprec
> bits", i.e.
>> the middle-end can use a SUBREG instead of a TRUNCATE.
>> 
>> What's perhaps potentially a little ambiguous in the above description is
> whether
>> it is the caller or the callee that's responsible for ensuring or checking
> whether
>> "outprec < inprec".  The name TRULY_NOOP_TRUNCATION_P, like
>> SUBREG_PROMOTED_P, may be prone to being understood as a predicate that
>> confirms that something is a no-op truncation or a promoted subreg, when
> in fact
>> the caller must first confirm this is a truncation/subreg and only then
> call the
>> "classification" macro.
>> 
>> Alas making the following minor tweak (for testing) to the i386 backend:
>> 
>> static bool
>> ix86_truly_noop_truncation (poly_uint64 outprec, poly_uint64 inprec) {
>>   gcc_assert (outprec < inprec);
>>   return true;
>> }
>> 
>> #undef TARGET_TRULY_NOOP_TRUNCATION
>> #define TARGET_TRULY_NOOP_TRUNCATION ix86_truly_noop_truncation
>> 
>> reveals that there are numerous callers in middle-end that rely on the
> default
>> behaviour of silently returning true for any (invalid) input.
>> These are fixed below.
>> 
>> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap and
>> make -k check, both with and without --target_board=unix{-m32} with no new
>> failures.  Ok for mainline?
>> 
>> 
>> 2023-12-28  Roger Sayle  <roger@nextmovesoftware.com>
>> 
>> gcc/ChangeLog
>>         * combine.cc (make_extraction): Confirm that OUTPREC is less than
>>         INPREC before calling TRULY_NOOP_TRUNCATION_MODES_P.
>>         * expmed.cc (store_bit_field_using_insv): Likewise.
>>         (extract_bit_field_using_extv): Likewise.
>>         (extract_bit_field_as_subreg): Likewise.
>>         * optabs-query.cc (get_best_extraction_insn): Likewise.
>>         * optabs.cc (expand_parity): Likewise.
>>         * rtlhooks.cc (gen_lowpart_general): Likewise.
>>         * simplify-rtx.cc (simplify_truncation): Disallow truncations
>>         to the same precision.
>>         (simplify_unary_operation_1) <case TRUNCATE>: Move optimization
>>         of truncations to the same mode earlier.
>> 
>> 
>> Thanks in advance,
>> Roger
>> --
>
> diff --git a/gcc/combine.cc b/gcc/combine.cc
> index f2c64a9..5aa2f57 100644
> --- a/gcc/combine.cc
> +++ b/gcc/combine.cc
> @@ -7613,7 +7613,8 @@ make_extraction (machine_mode mode, rtx inner, HOST_WIDE_INT pos,
>  	   && (pos == 0 || REG_P (inner))
>  	   && (inner_mode == tmode
>  	       || !REG_P (inner)
> -	       || TRULY_NOOP_TRUNCATION_MODES_P (tmode, inner_mode)
> +	       || (known_lt (GET_MODE_SIZE (tmode), GET_MODE_SIZE (inner_mode))
> +		   && TRULY_NOOP_TRUNCATION_MODES_P (tmode, inner_mode))

It's not obvious that this has the effect of preserving the original
condition.  Previously TRULY_NOOP_TRUNCATION_MODES_P would return
true for anything that wasn't a truncation, so I think it was closer to:

	   && (inner_mode == tmode
	       || !REG_P (inner)
	       || maybe_ge (GET_MODE_SIZE (tmode), GET_MODE_SIZE (inner_mode))
	       || TRULY_NOOP_TRUNCATION_MODES_P (tmode, inner_mode)

In other words...

>  	       || reg_truncated_to_mode (tmode, inner))

...this should also only be tested for actual truncations.

>  	   && (! in_dest
>  	       || (REG_P (inner)
> @@ -7856,6 +7857,8 @@ make_extraction (machine_mode mode, rtx inner, HOST_WIDE_INT pos,
>        /* On the LHS, don't create paradoxical subregs implicitely truncating
>  	 the register unless TARGET_TRULY_NOOP_TRUNCATION.  */
>        if (in_dest
> +	  && known_lt (GET_MODE_SIZE (GET_MODE (inner)),
> +		       GET_MODE_SIZE (wanted_inner_mode))
>  	  && !TRULY_NOOP_TRUNCATION_MODES_P (GET_MODE (inner),
>  					     wanted_inner_mode))
>  	return NULL_RTX;
> diff --git a/gcc/expmed.cc b/gcc/expmed.cc
> index 05331dd..6398bf9 100644
> --- a/gcc/expmed.cc
> +++ b/gcc/expmed.cc
> @@ -651,6 +651,7 @@ store_bit_field_using_insv (const extraction_insn *insv, rtx op0,
>       X) 0)) is (reg:N X).  */
>    if (GET_CODE (xop0) == SUBREG
>        && REG_P (SUBREG_REG (xop0))
> +      && paradoxical_subreg_p (xop0)
>        && !TRULY_NOOP_TRUNCATION_MODES_P (GET_MODE (SUBREG_REG (xop0)),
>  					 op_mode))
>      {
> @@ -1585,7 +1586,11 @@ extract_bit_field_using_extv (const extraction_insn *extv, rtx op0,
>  	 mode.  Instead, create a temporary and use convert_move to set
>  	 the target.  */
>        if (REG_P (target)
> -	  && TRULY_NOOP_TRUNCATION_MODES_P (GET_MODE (target), ext_mode)
> +	  && (known_lt (GET_MODE_SIZE (GET_MODE (target)),
> +			GET_MODE_SIZE (ext_mode))
> +	      ? TRULY_NOOP_TRUNCATION_MODES_P (GET_MODE (target), ext_mode)
> +	      : known_eq (GET_MODE_SIZE (GET_MODE (target)),
> +			  GET_MODE_SIZE (ext_mode)))
>  	  && (temp = gen_lowpart_if_possible (ext_mode, target)))
>  	{
>  	  target = temp;
> @@ -1626,7 +1631,9 @@ extract_bit_field_as_subreg (machine_mode mode, rtx op0,
>    if (multiple_p (bitnum, BITS_PER_UNIT, &bytenum)
>        && known_eq (bitsize, GET_MODE_BITSIZE (mode))
>        && lowpart_bit_field_p (bitnum, bitsize, op0_mode)
> -      && TRULY_NOOP_TRUNCATION_MODES_P (mode, op0_mode))
> +      && (known_lt (GET_MODE_SIZE (mode), GET_MODE_SIZE (op0_mode))
> +	  ? TRULY_NOOP_TRUNCATION_MODES_P (mode, op0_mode)
> +	  : known_eq (GET_MODE_SIZE (mode), GET_MODE_SIZE (op0_mode))))
>      return simplify_gen_subreg (mode, op0, op0_mode, bytenum);
>    return NULL_RTX;
>  }

I think these two should also use the known_ge formulation in the
covering letter.  Same for expand_parity.

We're then left with all the sites testing for essentially the same
thing, which is: can I change directly from mode A to mode B using
subregs?  It might be worth having a wrapper for that, rather than
requiring the known_ge in so many places.

Thanks,
Richard

> diff --git a/gcc/optabs-query.cc b/gcc/optabs-query.cc
> index 947ccef..f33253f 100644
> --- a/gcc/optabs-query.cc
> +++ b/gcc/optabs-query.cc
> @@ -213,7 +213,7 @@ get_best_extraction_insn (extraction_insn *insn,
>  	  FOR_EACH_MODE_FROM (mode_iter, mode)
>  	    {
>  	      mode = mode_iter.require ();
> -	      if (maybe_gt (GET_MODE_SIZE (mode), GET_MODE_SIZE (field_mode))
> +	      if (maybe_ge (GET_MODE_SIZE (mode), GET_MODE_SIZE (field_mode))
>  		  || TRULY_NOOP_TRUNCATION_MODES_P (insn->field_mode,
>  						    field_mode))
>  		break;
> diff --git a/gcc/optabs.cc b/gcc/optabs.cc
> index 6a34276..fad0d59 100644
> --- a/gcc/optabs.cc
> +++ b/gcc/optabs.cc
> @@ -2954,7 +2954,11 @@ expand_parity (scalar_int_mode mode, rtx op0, rtx target)
>  	  if (temp)
>  	    {
>  	      if (mclass != MODE_INT
> -		  || !TRULY_NOOP_TRUNCATION_MODES_P (mode, wider_mode))
> +		  || (known_lt (GET_MODE_SIZE (mode),
> +				GET_MODE_SIZE (wider_mode))
> +		      ? !TRULY_NOOP_TRUNCATION_MODES_P (mode, wider_mode)
> +		      : maybe_ne (GET_MODE_SIZE (mode),
> +				  GET_MODE_SIZE (wider_mode))))
>  		return convert_to_mode (mode, temp, 0);
>  	      else
>  		return gen_lowpart (mode, temp);
> diff --git a/gcc/rtlhooks.cc b/gcc/rtlhooks.cc
> index 989d3c9..c3313fd 100644
> --- a/gcc/rtlhooks.cc
> +++ b/gcc/rtlhooks.cc
> @@ -66,7 +66,8 @@ gen_lowpart_general (machine_mode mode, rtx x)
>        scalar_int_mode xmode;
>        if (is_a <scalar_int_mode> (GET_MODE (x), &xmode)
>  	  && GET_MODE_SIZE (xmode) <= UNITS_PER_WORD
> -	  && TRULY_NOOP_TRUNCATION_MODES_P (mode, xmode)
> +	  && (known_ge (GET_MODE_SIZE (mode), GET_MODE_SIZE (xmode))
> +	      || TRULY_NOOP_TRUNCATION_MODES_P (mode, xmode))
>  	  && !reload_completed)
>  	return gen_lowpart_general (mode, force_reg (xmode, x));
>  
> diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
> index f3745d8..27518f5 100644
> --- a/gcc/simplify-rtx.cc
> +++ b/gcc/simplify-rtx.cc
> @@ -617,7 +617,7 @@ simplify_context::simplify_truncation (machine_mode mode, rtx op,
>    unsigned int op_precision = GET_MODE_UNIT_PRECISION (op_mode);
>    scalar_int_mode int_mode, int_op_mode, subreg_mode;
>  
> -  gcc_assert (precision <= op_precision);
> +  gcc_assert (precision < op_precision);
>  
>    /* Optimize truncations of zero and sign extended values.  */
>    if (GET_CODE (op) == ZERO_EXTEND
> @@ -1207,6 +1207,10 @@ simplify_context::simplify_unary_operation_1 (rtx_code code, machine_mode mode,
>        break;
>  
>      case TRUNCATE:
> +      /* Check for useless truncation.  */
> +      if (GET_MODE (op) == mode)
> +	return op;
> +
>        /* Don't optimize (lshiftrt (mult ...)) as it would interfere
>  	 with the umulXi3_highpart patterns.  */
>        if (GET_CODE (op) == LSHIFTRT
> @@ -1271,9 +1275,6 @@ simplify_context::simplify_unary_operation_1 (rtx_code code, machine_mode mode,
>  	    return temp;
>  	}
>  
> -      /* Check for useless truncation.  */
> -      if (GET_MODE (op) == mode)
> -	return op;
>        break;
>  
>      case FLOAT_TRUNCATE:

Re: [middle-end PATCH take #2] Only call targetm.truly_noop_truncation for truncations.

[Jeff Law]

On 12/31/23 09:23, Roger Sayle wrote:
> 
> Very many thanks (and a Happy New Year) to the pre-commit
> patch testing folks at linaro.org.   Their testing has revealed that
> although my patch is clean on x86_64, it triggers some problems
> on aarch64 and arm.  The issue (with the previous version of my
> patch) is that these platforms require a paradoxical subreg to be
> generated by the middle-end, where we were previously checking
> for truly_noop_truncation.
> 
> This has been fixed (in revision 2) below.  Where previously I had:
> 
> @@ -66,7 +66,9 @@ gen_lowpart_general (machine_mode mode, rtx x)
>         scalar_int_mode xmode;
>         if (is_a <scalar_int_mode> (GET_MODE (x), &xmode)
>            && GET_MODE_SIZE (xmode) <= UNITS_PER_WORD
> -         && TRULY_NOOP_TRUNCATION_MODES_P (mode, xmode)
> +         && (known_lt (GET_MODE_SIZE (mode), GET_MODE_SIZE (xmode))
> +             ? TRULY_NOOP_TRUNCATION_MODES_P (mode, xmode)
> +             : known_eq (GET_MODE_SIZE (mode), GET_MODE_SIZE (xmode)))
>            && !reload_completed)
>          return gen_lowpart_general (mode, force_reg (xmode, x));
> 
> the correct change is:
> 
>         scalar_int_mode xmode;
>         if (is_a <scalar_int_mode> (GET_MODE (x), &xmode)
>            && GET_MODE_SIZE (xmode) <= UNITS_PER_WORD
> -         && TRULY_NOOP_TRUNCATION_MODES_P (mode, xmode)
> +         && (known_ge (GET_MODE_SIZE (mode), GET_MODE_SIZE (xmode))
> +             || TRULY_NOOP_TRUNCATION_MODES_P (mode, xmode))
>            && !reload_completed)
>          return gen_lowpart_general (mode, force_reg (xmode, x));
> 
> i.e. we only call TRULY_NOOP_TRUNCATION_MODES_P when we
> know we have a truncation, but the behaviour of non-truncations
> is preserved (no longer depends upon unspecified behaviour) and
> gen_lowpart_general is called to create the paradoxical SUBREG.
> 
> 
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
> and make -k check, both with and without --target_board=unix{-m32}
> with no new failures.  Ok for mainline?
> 
> Hopefully this revision tests cleanly on the linaro.org CI pipeline.
> 
> 2023-12-31  Roger Sayle  <roger@nextmovesoftware.com>
> 
> gcc/ChangeLog
>          * combine.cc (make_extraction): Confirm that OUTPREC is less than
>          INPREC before calling TRULY_NOOP_TRUNCATION_MODES_P.
>          * expmed.cc (store_bit_field_using_insv): Likewise.
>          (extract_bit_field_using_extv): Likewise.
>          (extract_bit_field_as_subreg): Likewise.
>          * optabs-query.cc (get_best_extraction_insn): Likewise.
>          * optabs.cc (expand_parity): Likewise.
>          * rtlhooks.cc (gen_lowpart_general): Likewise.
>          * simplify-rtx.cc (simplify_truncation): Disallow truncations
>          to the same precision.
>          (simplify_unary_operation_1) <case TRUNCATE>: Move optimization
>          of truncations to the same mode earlier.
So just an FYI.  This should probably wait for gcc-15.  But it is worth 
noting that this causes tbz_2.c to fail on aarch64.  Basically with this 
patch we use the "x" registers whereas before we used a "w" register. 
Meaning we're doing a 64bit comparison now rather than a 32bit 
comparison.  THe test expects a 32bit comparison.

jeff