Stam Markianos-Wright <stam.markianos-wri...@arm.com> writes:
> Hi all,
>
> This fixes:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93300
>
> Genmodes.c was generating the "wider_mode" chain as follows:
>
> HF -> BF -> SF - > DF -> TF -> VOID
>
> This caused issues in some rare cases where conversion between modes
> was needed, such as the above PR93300 where BFmode was being picked up
> as a valid mode for:
>
> optabs.c:prepare_float_lib_cmp
>
> which then led to the ICE at expr.c:convert_mode_scalar.
Can you go into more details about why this chain was a problem?
Naively, it's the one I'd have expected: HF should certainly have
priority over BF, but BF coming before SF doesn't seem unusual in itself.
I'm not saying the patch is wrong. It just wasn't clear why it was
right either.
Thanks,
Richard
>
> This patch adds a new FLOAT_MODE_UNRANKED macro which uses the existing
> "order"
> attribute of mode_data to place BFmode as:
>
> HF -> SF - > DF -> TF -> BF -> VOID
>
> This fixes the existing ICE seen by PR93300 (hence providing this with no
> explicit test) and causes no further regressions.
> Reg-tested on arm-none-eabi, aarch64-none-elf and bootstrapped on a
> Cortex-A15.
>
> Ok for trunk?
>
> Cheers,
> Stam
>
> gcc/ChangeLog:
>
> 2020-01-28 Stam Markianos-Wright <stam.markianos-wri...@arm.com>
>
> * config/aarch64/aarch64-modes.def: Update BFmode to use
> FLOAT_MODE_UNRANKED.
> * config/arm/arm-modes.def: Update BFmode to use FLOAT_MODE_UNRANKED.
> * genmodes.c (FLOAT_MODE_UNRANKED): New macro.
> (make_float_mode): Add ORDER parameter.
>
> The whole diff for reference:
>
> diff --git a/gcc/config/aarch64/aarch64-modes.def
> b/gcc/config/aarch64/aarch64-modes.def
> index 1eeb8d88452..0b36da942b4 100644
> --- a/gcc/config/aarch64/aarch64-modes.def
> +++ b/gcc/config/aarch64/aarch64-modes.def
> @@ -69,10 +69,10 @@ VECTOR_MODES (FLOAT, 16); /* V4SF V2DF. */
> VECTOR_MODE (FLOAT, DF, 1); /* V1DF. */
> VECTOR_MODE (FLOAT, HF, 2); /* V2HF. */
>
> -/* Bfloat16 modes. */
> -FLOAT_MODE (BF, 2, 0);
> +/* Bfloat16 modes. Using 1 as the ORDER argument ensures that this is
> + placed after normal floating point modes in the GET_MODES_WIDER chain. */
> +FLOAT_MODE_UNRANKED (BF, 2, 0, 1);
> ADJUST_FLOAT_FORMAT (BF, &arm_bfloat_half_format);
> -
> VECTOR_MODE (FLOAT, BF, 4); /* V4BF. */
> VECTOR_MODE (FLOAT, BF, 8); /* V8BF. */
>
> diff --git a/gcc/config/arm/arm-modes.def b/gcc/config/arm/arm-modes.def
> index ea92ef35723..86551be8e3b 100644
> --- a/gcc/config/arm/arm-modes.def
> +++ b/gcc/config/arm/arm-modes.def
> @@ -78,7 +78,9 @@ VECTOR_MODES (FLOAT, 8); /* V4HF V2SF */
> VECTOR_MODES (FLOAT, 16); /* V8HF V4SF V2DF */
> VECTOR_MODE (FLOAT, HF, 2); /* V2HF */
>
> -FLOAT_MODE (BF, 2, 0);
> +/* Bfloat16 modes. Using 1 as the ORDER argument ensures that this is
> + placed after normal floating point modes in the GET_MODES_WIDER chain. */
> +FLOAT_MODE_UNRANKED (BF, 2, 0, 1);
> ADJUST_FLOAT_FORMAT (BF, &arm_bfloat_half_format);
> VECTOR_MODE (FLOAT, BF, 4); /* V4BF. */
> VECTOR_MODE (FLOAT, BF, 8); /* V8BF. */
> diff --git a/gcc/genmodes.c b/gcc/genmodes.c
> index bd78310ea24..c4e3dd1150d 100644
> --- a/gcc/genmodes.c
> +++ b/gcc/genmodes.c
> @@ -617,20 +617,23 @@ make_fixed_point_mode (enum mode_class cl,
> m->fbit = fbit;
> }
>
> -#define FLOAT_MODE(N, Y, F) FRACTIONAL_FLOAT_MODE (N, -1U, Y, F)
> -#define FRACTIONAL_FLOAT_MODE(N, B, Y, F) \
> - make_float_mode (#N, B, Y, #F, __FILE__, __LINE__)
> +#define FLOAT_MODE_UNRANKED(N, Y, F, ORDER) \
> + FRACTIONAL_FLOAT_MODE (N, -1U, Y, F, ORDER)
> +#define FLOAT_MODE(N, Y, F) FRACTIONAL_FLOAT_MODE (N, -1U, Y, F,
> 0)
> +#define FRACTIONAL_FLOAT_MODE(N, B, Y, F, ORDER) \
> + make_float_mode (#N, B, Y, #F, ORDER, __FILE__, __LINE__)
>
> static void
> make_float_mode (const char *name,
> unsigned int precision, unsigned int bytesize,
> - const char *format,
> + const char *format, unsigned int order,
> const char *file, unsigned int line)
> {
> struct mode_data *m = new_mode (MODE_FLOAT, name, file, line);
> m->bytesize = bytesize;
> m->precision = precision;
> m->format = format;
> + m->order = order;
> }
>
> #define DECIMAL_FLOAT_MODE(N, Y, F) \
