Xi Ruoyao <xry...@xry111.site> writes:
> The tranform would be unsafe if !TRULY_NOOP_TRUNCATION because on these
> machines the hardware may look at bits outside of the given mode.
>
> gcc/ChangeLog:
>
>       PR rtl-optimization/120050
>       * ext-dce.cc (ext_dce_try_optimize_insn): Only transform the
>       insn if TRULY_NOOP_TRUNCATION.

Yeah, it's not obvious to me what the right fix is here.  When I
responded to the PR, I was really only responding to the comment about
using mode_rep_extended.  Now that I've read the rest of the PR trail,
I agree with you that the /u (unsigned promotion) in insn 440 looks
really suspicious.  I don't see how MIPS can escape using a truncate for
that instruction.  So it does seem that the code is potentially wrong
coming into ext-dce.

But then the question is whether ext-dce could still misbehave when
the incoming code is correct.  Certainly it mustn't remove/invalidate
SUBREG_PROMOTED_VAR_P for signed promotions from SI to DI, since MIPS
relies on those (for correctness) to avoid truncations.

But then the question is where the TRULY_NOOP_TRUNCATION test should go.
The subreg in ext_dce_try_optimize_insn isn't itself problematic, since it's
creating an extension via a paradoxical subreg.  So I wonder if the check
should instead be in the analysis phase, aimed at identifying insn 440.

Specifically, if we see an rhs (subreg/s[/u]:M1 (...:M2)) (/s for
SUBREG_PROMOTED_VAR_P) and TRULY_NOOP_TRUNCATION says that the
truncation is not a noop, then all bits of M2 are significant/live.
We cannot reduce the live mask to that of M1.

I'm also not sure off-hand what the consequences of the TRUNCATE handling
in safe_for_live_propagation would be for !TRULY_NOOP_TRUNCATION.
At first it seemed wrong, but now I think it's proably ok.  I suppose
the truncation never goes away, and the whole point of MIPS truncations
is that they make no assumption about the upper bits of the incoming DI
register, so there's also no rule against changing those bits.

Thanks,
Richard

> ---
>
> Bootstrapped on mips64el-linux-gnuabi64.  Ok for trunk and gcc-15?
>
>  gcc/ext-dce.cc | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/gcc/ext-dce.cc b/gcc/ext-dce.cc
> index a0343950141..4be4f8a007b 100644
> --- a/gcc/ext-dce.cc
> +++ b/gcc/ext-dce.cc
> @@ -35,6 +35,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "print-rtl.h"
>  #include "dbgcnt.h"
>  #include "diagnostic-core.h"
> +#include "target.h"
>  
>  /* These should probably move into a C++ class.  */
>  static vec<bitmap_head> livein;
> @@ -415,6 +416,13 @@ ext_dce_try_optimize_insn (rtx_insn *insn, rtx set)
>        return;
>      }
>  
> +  if (!TRULY_NOOP_TRUNCATION_MODES_P (GET_MODE (inner), GET_MODE (src)))
> +    {
> +      if (dump_file)
> +     fprintf (dump_file, "The transformation is unsafe on this machine.\n");
> +      return;
> +    }
> +
>    new_pattern = simplify_gen_subreg (GET_MODE (src), inner,
>                                    GET_MODE (inner), 0);
>    /* simplify_gen_subreg may fail in which case NEW_PATTERN will be NULL.

Reply via email to