On Fri, Aug 6, 2021 at 12:59 PM Hongtao Liu <crazy...@gmail.com> wrote: > > On Fri, Aug 6, 2021 at 11:44 AM Andrew Pinski via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > On Thu, Aug 5, 2021 at 8:33 PM liuhongt via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > > > > > > Hi: > > > --- > > > OK, I think sth is amiss here upthread. insv/extv do look like they > > > are designed > > > to work on integer modes (but docs do not say anything about this here). > > > In fact the caller of extract_bit_field_using_extv is named > > > extract_integral_bit_field. Of course nothing seems to check what kind of > > > modes we're dealing with, but we're for example happily doing > > > expand_shift in 'mode'. In the extract_integral_bit_field call 'mode' is > > > some integer mode and op0 is HFmode? From the above I get it's > > > the other way around? In that case we should wrap the > > > call to extract_integral_bit_field, extracting in an integer mode with the > > > same size as 'mode' and then converting the result as (subreg:HF (reg:HI > > > ...)). > > > > This seems related to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93235 . CC jakub. > > I wonder why the fix for that did not help here. > > > aarch64 didn't hit gcc_assert with my testcase, and I debugged it to > figure out why. > > in gimple level, both x86 and aarch64 is the same with > _3 = BIT_FIELD_REF <a_2(D), 16, 0>; > > and they all goes into > extract_bit_field_using_extv > > The difference is aarch64 has ext_mode as DImode, but x86 has ext_mode > as SImode. > with ext_mode as DImode and target as (reg:HF 94), aarch64 doesn't hit > gcc_assert in > gen_lowpart (ext_mode, target) > > since validate_subreg allow (subreg:DI (reg:HF)), but disallow > (subreg:SI (reg:HF)). > > /* ??? This should not be here. Temporarily continue to allow word_mode > subregs of anything. The most common offender is (subreg:SI (reg:DF)). > Generally, backends are doing something sketchy but it'll take time to > fix them all. */ > if (omode == word_mode) > ; > > ext_mode is assigned from extv->field mode which is initialized in > get_best_reg_extraction_insn. > get_best_reg_extraction_insn will finally call > get_optab_extraction_insn and find > aarch64 doesn't have CODE_FOR_extzvsi but x86 has. > > That's why aarch64 has ext_mode as DImode and x86 SImode. > > > Thanks, > > Andrew Pinski > > > > > --- > > > This is a separate patch as a follow up of upper comments. > > > > > > gcc/ChangeLog: > > > > > > * expmed.c (extract_bit_field_1): Wrap the call to > > > extract_integral_bit_field, extracting in an integer mode with > > > the same size as 'tmode' and then converting the result > > > as (subreg:tmode (reg:imode)). > > > > > > gcc/testsuite/ChangeLog: > > > * gcc.target/i386/float16-5.c: New test. > > > --- > > > gcc/expmed.c | 19 +++++++++++++++++++ > > > gcc/testsuite/gcc.target/i386/float16-5.c | 12 ++++++++++++ > > > 2 files changed, 31 insertions(+) > > > create mode 100644 gcc/testsuite/gcc.target/i386/float16-5.c > > > > > > diff --git a/gcc/expmed.c b/gcc/expmed.c > > > index 3143f38e057..72790693ef0 100644 > > > --- a/gcc/expmed.c > > > +++ b/gcc/expmed.c > > > @@ -1850,6 +1850,25 @@ extract_bit_field_1 (rtx str_rtx, poly_uint64 > > > bitsize, poly_uint64 bitnum, > > > op0_mode = opt_scalar_int_mode (); > > > } > > > > > > + /* Make sure we are playing with integral modes. Pun with subregs > > > + if we aren't. When tmode is HFmode, op0 is SImode, there will be ICE > > > + in extract_integral_bit_field. */ > > > + if (int_mode_for_mode (tmode).exists (&imode) > > > + && imode != tmode > > > + && imode != GET_MODE (op0)) > > > + { > > > + rtx ret = extract_integral_bit_field (op0, op0_mode, > > > + bitsize.to_constant (), > > > + bitnum.to_constant (), > > > unsignedp, > > > + NULL, imode, imode, > > > + reverse, fallback_p); > > > + gcc_assert (ret); > > > + > > > + if (!REG_P (ret)) > > > + ret = force_reg (imode, ret); > > > + return gen_lowpart_SUBREG (tmode, ret); > > > + } > > > + > > > /* It's possible we'll need to handle other cases here for > > > polynomial bitnum and bitsize. */ > > > > > > diff --git a/gcc/testsuite/gcc.target/i386/float16-5.c > > > b/gcc/testsuite/gcc.target/i386/float16-5.c > > > new file mode 100644 > > > index 00000000000..ebc0af1490b > > > --- /dev/null > > > +++ b/gcc/testsuite/gcc.target/i386/float16-5.c > > > @@ -0,0 +1,12 @@ > > > +/* { dg-do compile } */ > > > +/* { dg-options "-msse2 -O2" } */ > > > +_Float16 > > > +foo (int a) > > > +{ > > > + union { > > > + int a; > > > + _Float16 b; > > > + }c; > > > + c.a = a; > > > + return c.b; > > > +} > > > -- > > > 2.27.0 > > > > > > > -- > BR, > Hongtao
-- BR, Hongtao
