On September 10, 2021 3:27:09 PM GMT+02:00, Hongtao Liu <crazy...@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....@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. > >---------------- >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)
I see. Of course that leads to a suggestion to allow the subreg based on modes_tieable_p, but then others will know why that's the wrong thing to do? Richard. >> >> > + && 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 >> > > > >