On Fri, Aug 6, 2021 at 5:32 AM liuhongt <hongtao....@intel.com> 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 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)
check !INTEGRAL_MODE_P (tmode) before, that should be slightly
cheaper. Then imode should always be != tmode. Maybe
even GET_MDOE_CLASS (tmode) != MODE_INT since I'm not sure
how it behaves for composite modes.
Of course the least surprises would happen when we restrict this
to FLOAT_MODE_P (tmode).
Richard - any preferences?
> + && 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
>
