Re: [PATCH 2/2] validate_subreg before call gen_lowpart to avoid ICE.

[Hongtao Liu <crazylht@gmail.com>]

On Fri, Sep 10, 2021 at 9:27 PM Hongtao Liu <crazylht@gmail.com> wrote:
>
> On Fri, Sep 10, 2021 at 9:16 PM Richard Biener via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > On Fri, Sep 10, 2021 at 2:58 PM liuhongt <hongtao.liu@intel.com> wrote:
> > >
> > > gcc/ChangeLog:
> > >
> > >         * expmed.c (extract_bit_field_using_extv): validate_subreg
> > >         before call gen_lowpart.
> > > ---
> > >  gcc/expmed.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/gcc/expmed.c b/gcc/expmed.c
> > > index 3143f38e057..10d62d857a8 100644
> > > --- a/gcc/expmed.c
> > > +++ b/gcc/expmed.c
> > > @@ -1571,12 +1571,16 @@ extract_bit_field_using_extv (const extraction_insn *extv, rtx op0,
> > >
> > >    if (GET_MODE (target) != ext_mode)
> > >      {
> > > +      machine_mode tmode = GET_MODE (target);
> > >        /* Don't use LHS paradoxical subreg if explicit truncation is needed
> > >          between the mode of the extraction (word_mode) and the target
> > >          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))
> >
> > ^^^
> >
> > I wonder if herein lies the problem in that the HFmode "truncation" from SImode
> > is considered noop?  Note the underlying target hook only looks at the mode
> > precision and thus receives 16 and 32, and thus maybe that
> > TRULY_NOOP_TRUNCATION_MODES_P query only makes sense for
> > integer modes?  Though the documentation of the hook only talks about
> > "conversion" of "values" ...
> >
> > So maybe a targetm.modes_tieable_p (GET_MODE (target), extmode) check
> > is missing?
>
> According to document, it should be true for
> targetm.modes_tieable_p(HFmode, SImode) since HFmode can be allocated
> to gpr.
I was wrong it needs *any* r, so targetm.modes_tieable_p do return false here.

If TARGET_HARD_REGNO_MODE_OK (r, mode1) and TARGET_HARD_REGNO_MODE_OK (r,
mode2) are always the same for any r, then TARGET_MODES_TIEABLE_P (mode1,
mode2) should be true. If they differ for any r, you should define this hook to
return false unless some other mechanism ensures the accessibility of
the value in a
narrower mode.
You should define this hook to return true in as many cases as
possible since doing so
will allow GCC to perform better register allocation. The default
definition returns
true unconditionally.

>
> ----------------
> This hook returns true if a value of mode mode1 is accessible in mode
> mode2 without
> copying
> -------------------
>
> and also here gen_lowpart (SImode, HFmode, target) is called and hit
> gcc_assert, not (subreg:HF (reg:SI) 0)
>
> >
> > > +         && TRULY_NOOP_TRUNCATION_MODES_P (tmode, ext_mode)
> > > +         && validate_subreg (ext_mode, tmode,
> > > +                             target,
> > > +                             subreg_lowpart_offset (ext_mode, tmode)))
> > >         {
> > >           target = gen_lowpart (ext_mode, target);
> > >           if (partial_subreg_p (GET_MODE (spec_target), ext_mode))
> > > --
> > > 2.27.0
> > >
>
>
>
> --
> BR,
> Hongtao



-- 
BR,
Hongtao