* [GCC][BUG][Aarch64][ARM] (PR93300) Fix ICE due to BFmode placement in GET_MODES_WIDER chain.
@ 2020-01-29 12:39 Stam Markianos-Wright
2020-01-29 12:58 ` Richard Sandiford
0 siblings, 1 reply; 10+ messages in thread
From: Stam Markianos-Wright @ 2020-01-29 12:39 UTC (permalink / raw)
To: gcc-patches
Cc: Richard Earnshaw, richard.sandiford, nickc, ramana.radhakrishnan,
Kyrylo Tkachov, marcus.shawcroft
[-- Attachment #1: Type: text/plain, Size: 3837 bytes --]
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.
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-wright@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) \
[-- Attachment #2: GET_MODE_WIDER.patch --]
[-- Type: text/x-patch, Size: 2635 bytes --]
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) \
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GCC][BUG][Aarch64][ARM] (PR93300) Fix ICE due to BFmode placement in GET_MODES_WIDER chain.
2020-01-29 12:39 [GCC][BUG][Aarch64][ARM] (PR93300) Fix ICE due to BFmode placement in GET_MODES_WIDER chain Stam Markianos-Wright
@ 2020-01-29 12:58 ` Richard Sandiford
2020-01-29 18:01 ` Stam Markianos-Wright
0 siblings, 1 reply; 10+ messages in thread
From: Richard Sandiford @ 2020-01-29 12:58 UTC (permalink / raw)
To: Stam Markianos-Wright
Cc: gcc-patches, Richard Earnshaw, nickc, ramana.radhakrishnan,
Kyrylo Tkachov, marcus.shawcroft
Stam Markianos-Wright <stam.markianos-wright@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-wright@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) \
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GCC][BUG][Aarch64][ARM] (PR93300) Fix ICE due to BFmode placement in GET_MODES_WIDER chain.
2020-01-29 12:58 ` Richard Sandiford
@ 2020-01-29 18:01 ` Stam Markianos-Wright
2020-01-30 11:49 ` Richard Sandiford
0 siblings, 1 reply; 10+ messages in thread
From: Stam Markianos-Wright @ 2020-01-29 18:01 UTC (permalink / raw)
To: gcc-patches, Richard Earnshaw, nickc, ramana.radhakrishnan,
Kyrylo Tkachov, marcus.shawcroft, richard.sandiford
On 1/29/20 12:42 PM, Richard Sandiford wrote:
> Stam Markianos-Wright <stam.markianos-wright@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.
Hi Richard,
>
> 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,
Is that because functionally it looks like genmodes puts things in reverse
alphabetical order if all else is equal? (If I'm reading the comment about
MODE_RANDOM, MODE_CC correctly)
> 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.
>
Yes, I see what you mean. I'll go through my thought process here:
In investigating the ICE PR93300 I found that the diversion from pre-bf16
behaviour was specifically at `optabs.c:prepare_float_lib_cmp`, where a
`FOR_EACH_MODE_FROM (mode, orig_mode)` is used to then go off and generate
library calls for conversions.
This was then being caught further down by the gcc_assert at expr.c:325 where
GET_MODE_PRECISION (from_mode) was equal to GET_MODE_PRECISION (to_mode) because
it was trying to emit a HF->BF conversion libcall as `bl __extendhfbf2` (which
is what happened if i removed the gcc_assert at expr.c:325)
With BFmode being a target-defined mode, I didn't want to add something like `if
(mode != BFmode)` to specifically exclude BFmode from being selected for this.
(and there's nothing different between HFmode and BFmode here to allow me to
make this distinction?)
Also I couldn't find anywhere where the target back-end is not consulted for a
"is this supported: yes/no" between the `FOR_EACH_MODE_FROM` loop and the
libcall being created later on as __extendhfbf2.
Finally, because we really don't want __bf16 to be in the same "conversion rank"
as standard float modes for things like automatic promotion, this seemed like a
reasonable solution to that problem :)
Let me know of your thoughts!
Cheers,
Stam
> 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-wright@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) \
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GCC][BUG][Aarch64][ARM] (PR93300) Fix ICE due to BFmode placement in GET_MODES_WIDER chain.
2020-01-29 18:01 ` Stam Markianos-Wright
@ 2020-01-30 11:49 ` Richard Sandiford
2020-01-30 17:54 ` Stam Markianos-Wright
0 siblings, 1 reply; 10+ messages in thread
From: Richard Sandiford @ 2020-01-30 11:49 UTC (permalink / raw)
To: Stam Markianos-Wright
Cc: gcc-patches, Richard Earnshaw, nickc, ramana.radhakrishnan,
Kyrylo Tkachov, marcus.shawcroft
Stam Markianos-Wright <stam.markianos-wright@arm.com> writes:
> On 1/29/20 12:42 PM, Richard Sandiford wrote:
>> Stam Markianos-Wright <stam.markianos-wright@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.
>
> Hi Richard,
>
>>
>> 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,
>
> Is that because functionally it looks like genmodes puts things in reverse
> alphabetical order if all else is equal? (If I'm reading the comment about
> MODE_RANDOM, MODE_CC correctly)
>
>> 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.
>>
> Yes, I see what you mean. I'll go through my thought process here:
>
> In investigating the ICE PR93300 I found that the diversion from pre-bf16
> behaviour was specifically at `optabs.c:prepare_float_lib_cmp`, where a
> `FOR_EACH_MODE_FROM (mode, orig_mode)` is used to then go off and generate
> library calls for conversions.
>
> This was then being caught further down by the gcc_assert at expr.c:325 where
> GET_MODE_PRECISION (from_mode) was equal to GET_MODE_PRECISION (to_mode) because
> it was trying to emit a HF->BF conversion libcall as `bl __extendhfbf2` (which
> is what happened if i removed the gcc_assert at expr.c:325)
>
> With BFmode being a target-defined mode, I didn't want to add something like `if
> (mode != BFmode)` to specifically exclude BFmode from being selected for this.
> (and there's nothing different between HFmode and BFmode here to allow me to
> make this distinction?)
>
> Also I couldn't find anywhere where the target back-end is not consulted for a
> "is this supported: yes/no" between the `FOR_EACH_MODE_FROM` loop and the
> libcall being created later on as __extendhfbf2.
Yeah, prepare_float_lib_cmp just checks for libfuncs rather than
calling target hooks directly. The libfuncs themselves are under
the control of the target though.
By default we assume all float modes have associated libfuncs.
It's then up to the target to remove functions that don't exist
(or redirect them to other functions). So I think we need to remove
BFmode libfuncs in arm_init_libfuncs in the same way as we currently
do for HFmode.
I guess we should also nullify the conversion libfuncs for BFmode,
not just the arithmetic and comparison ones.
Thanks,
Richard
> Finally, because we really don't want __bf16 to be in the same "conversion rank"
> as standard float modes for things like automatic promotion, this seemed like a
> reasonable solution to that problem :)
>
> Let me know of your thoughts!
>
> Cheers,
> Stam
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GCC][BUG][Aarch64][ARM] (PR93300) Fix ICE due to BFmode placement in GET_MODES_WIDER chain.
2020-01-30 11:49 ` Richard Sandiford
@ 2020-01-30 17:54 ` Stam Markianos-Wright
2020-01-31 14:13 ` Richard Sandiford
0 siblings, 1 reply; 10+ messages in thread
From: Stam Markianos-Wright @ 2020-01-30 17:54 UTC (permalink / raw)
To: gcc-patches, Richard Earnshaw, nickc, ramana.radhakrishnan,
Kyrylo Tkachov, marcus.shawcroft, richard.sandiford
[-- Attachment #1: Type: text/plain, Size: 3686 bytes --]
On 1/30/20 10:01 AM, Richard Sandiford wrote:
> Stam Markianos-Wright <stam.markianos-wright@arm.com> writes:
>> On 1/29/20 12:42 PM, Richard Sandiford wrote:
>>> Stam Markianos-Wright <stam.markianos-wright@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.
>>
>> Hi Richard,
>>
>>>
>>> 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,
>>
>> Is that because functionally it looks like genmodes puts things in reverse
>> alphabetical order if all else is equal? (If I'm reading the comment about
>> MODE_RANDOM, MODE_CC correctly)
>>
>>> 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.
>>>
>> Yes, I see what you mean. I'll go through my thought process here:
>>
>> In investigating the ICE PR93300 I found that the diversion from pre-bf16
>> behaviour was specifically at `optabs.c:prepare_float_lib_cmp`, where a
>> `FOR_EACH_MODE_FROM (mode, orig_mode)` is used to then go off and generate
>> library calls for conversions.
>>
>> This was then being caught further down by the gcc_assert at expr.c:325 where
>> GET_MODE_PRECISION (from_mode) was equal to GET_MODE_PRECISION (to_mode) because
>> it was trying to emit a HF->BF conversion libcall as `bl __extendhfbf2` (which
>> is what happened if i removed the gcc_assert at expr.c:325)
>>
>> With BFmode being a target-defined mode, I didn't want to add something like `if
>> (mode != BFmode)` to specifically exclude BFmode from being selected for this.
>> (and there's nothing different between HFmode and BFmode here to allow me to
>> make this distinction?)
>>
>> Also I couldn't find anywhere where the target back-end is not consulted for a
>> "is this supported: yes/no" between the `FOR_EACH_MODE_FROM` loop and the
>> libcall being created later on as __extendhfbf2.
>
> Yeah, prepare_float_lib_cmp just checks for libfuncs rather than
> calling target hooks directly. The libfuncs themselves are under
> the control of the target though.
>
> By default we assume all float modes have associated libfuncs.
> It's then up to the target to remove functions that don't exist
> (or redirect them to other functions). So I think we need to remove
> BFmode libfuncs in arm_init_libfuncs in the same way as we currently
> do for HFmode.
>
> I guess we should also nullify the conversion libfuncs for BFmode,
> not just the arithmetic and comparison ones.
Ahhh now this works, thank you for the suggestion!
I was aware of arm_init_libfuncs, but I had not realised that returning NULL
would have the desired effect for us, in this case. So I have essentially rolled
back the whole previous version of the patch and done this in the new diff.
It seems to have fixed the ICE and I am currently in the process of regression
testing!
Thank you!
Stam
>
> Thanks,
> Richard
>
>> Finally, because we really don't want __bf16 to be in the same "conversion rank"
>> as standard float modes for things like automatic promotion, this seemed like a
>> reasonable solution to that problem :)
>>
>> Let me know of your thoughts!
>>
>> Cheers,
>> Stam
[-- Attachment #2: BF_libfuncs --]
[-- Type: text/plain, Size: 1373 bytes --]
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index c47fc232f39..18055d4a75e 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -2643,6 +2643,30 @@ arm_init_libfuncs (void)
default:
break;
}
+
+ /* For all possible libcalls in BFmode, return NULL. */
+ /* Conversions. */
+ set_conv_libfunc (trunc_optab, BFmode, HFmode, (NULL));
+ set_conv_libfunc (sext_optab, HFmode, BFmode, (NULL));
+ set_conv_libfunc (trunc_optab, BFmode, SFmode, (NULL));
+ set_conv_libfunc (sext_optab, SFmode, BFmode, (NULL));
+ set_conv_libfunc (trunc_optab, BFmode, DFmode, (NULL));
+ set_conv_libfunc (sext_optab, DFmode, BFmode, (NULL));
+
+ /* Arithmetic. */
+ set_optab_libfunc (add_optab, BFmode, NULL);
+ set_optab_libfunc (sdiv_optab, BFmode, NULL);
+ set_optab_libfunc (smul_optab, BFmode, NULL);
+ set_optab_libfunc (neg_optab, BFmode, NULL);
+ set_optab_libfunc (sub_optab, BFmode, NULL);
+
+ /* Comparisons. */
+ set_optab_libfunc (eq_optab, BFmode, NULL);
+ set_optab_libfunc (ne_optab, BFmode, NULL);
+ set_optab_libfunc (lt_optab, BFmode, NULL);
+ set_optab_libfunc (le_optab, BFmode, NULL);
+ set_optab_libfunc (ge_optab, BFmode, NULL);
+ set_optab_libfunc (gt_optab, BFmode, NULL);
+ set_optab_libfunc (unord_optab, BFmode, NULL);
/* Use names prefixed with __gnu_ for fixed-point helper functions. */
{
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GCC][BUG][Aarch64][ARM] (PR93300) Fix ICE due to BFmode placement in GET_MODES_WIDER chain.
2020-01-30 17:54 ` Stam Markianos-Wright
@ 2020-01-31 14:13 ` Richard Sandiford
2020-02-04 11:30 ` Stam Markianos-Wright
0 siblings, 1 reply; 10+ messages in thread
From: Richard Sandiford @ 2020-01-31 14:13 UTC (permalink / raw)
To: Stam Markianos-Wright
Cc: gcc-patches, Richard Earnshaw, nickc, ramana.radhakrishnan,
Kyrylo Tkachov, marcus.shawcroft
Stam Markianos-Wright <stam.markianos-wright@arm.com> writes:
> On 1/30/20 10:01 AM, Richard Sandiford wrote:
>> Stam Markianos-Wright <stam.markianos-wright@arm.com> writes:
>>> On 1/29/20 12:42 PM, Richard Sandiford wrote:
>>>> Stam Markianos-Wright <stam.markianos-wright@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.
>>>
>>> Hi Richard,
>>>
>>>>
>>>> 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,
>>>
>>> Is that because functionally it looks like genmodes puts things in reverse
>>> alphabetical order if all else is equal? (If I'm reading the comment about
>>> MODE_RANDOM, MODE_CC correctly)
>>>
>>>> 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.
>>>>
>>> Yes, I see what you mean. I'll go through my thought process here:
>>>
>>> In investigating the ICE PR93300 I found that the diversion from pre-bf16
>>> behaviour was specifically at `optabs.c:prepare_float_lib_cmp`, where a
>>> `FOR_EACH_MODE_FROM (mode, orig_mode)` is used to then go off and generate
>>> library calls for conversions.
>>>
>>> This was then being caught further down by the gcc_assert at expr.c:325 where
>>> GET_MODE_PRECISION (from_mode) was equal to GET_MODE_PRECISION (to_mode) because
>>> it was trying to emit a HF->BF conversion libcall as `bl __extendhfbf2` (which
>>> is what happened if i removed the gcc_assert at expr.c:325)
>>>
>>> With BFmode being a target-defined mode, I didn't want to add something like `if
>>> (mode != BFmode)` to specifically exclude BFmode from being selected for this.
>>> (and there's nothing different between HFmode and BFmode here to allow me to
>>> make this distinction?)
>>>
>>> Also I couldn't find anywhere where the target back-end is not consulted for a
>>> "is this supported: yes/no" between the `FOR_EACH_MODE_FROM` loop and the
>>> libcall being created later on as __extendhfbf2.
>>
>> Yeah, prepare_float_lib_cmp just checks for libfuncs rather than
>> calling target hooks directly. The libfuncs themselves are under
>> the control of the target though.
>>
>> By default we assume all float modes have associated libfuncs.
>> It's then up to the target to remove functions that don't exist
>> (or redirect them to other functions). So I think we need to remove
>> BFmode libfuncs in arm_init_libfuncs in the same way as we currently
>> do for HFmode.
>>
>> I guess we should also nullify the conversion libfuncs for BFmode,
>> not just the arithmetic and comparison ones.
>
> Ahhh now this works, thank you for the suggestion!
>
> I was aware of arm_init_libfuncs, but I had not realised that returning NULL
> would have the desired effect for us, in this case. So I have essentially rolled
> back the whole previous version of the patch and done this in the new diff.
> It seems to have fixed the ICE and I am currently in the process of regression
> testing!
LGTM behaviourally, just a couple of requests about how it's written:
>
> Thank you!
> Stam
>
>>
>> Thanks,
>> Richard
>>
>>> Finally, because we really don't want __bf16 to be in the same "conversion rank"
>>> as standard float modes for things like automatic promotion, this seemed like a
>>> reasonable solution to that problem :)
>>>
>>> Let me know of your thoughts!
>>>
>>> Cheers,
>>> Stam
>
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index c47fc232f39..18055d4a75e 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -2643,6 +2643,30 @@ arm_init_libfuncs (void)
> default:
> break;
> }
> +
> + /* For all possible libcalls in BFmode, return NULL. */
> + /* Conversions. */
> + set_conv_libfunc (trunc_optab, BFmode, HFmode, (NULL));
> + set_conv_libfunc (sext_optab, HFmode, BFmode, (NULL));
> + set_conv_libfunc (trunc_optab, BFmode, SFmode, (NULL));
> + set_conv_libfunc (sext_optab, SFmode, BFmode, (NULL));
> + set_conv_libfunc (trunc_optab, BFmode, DFmode, (NULL));
> + set_conv_libfunc (sext_optab, DFmode, BFmode, (NULL));
It might be slightly safer to do:
FOR_EACH_MODE_IN_CLASS (mode_iter, MODE_FLOAT)
to iterate over all float modes on the non-BF side.
> + /* Arithmetic. */
> + set_optab_libfunc (add_optab, BFmode, NULL);
> + set_optab_libfunc (sdiv_optab, BFmode, NULL);
> + set_optab_libfunc (smul_optab, BFmode, NULL);
> + set_optab_libfunc (neg_optab, BFmode, NULL);
> + set_optab_libfunc (sub_optab, BFmode, NULL);
> +
> + /* Comparisons. */
> + set_optab_libfunc (eq_optab, BFmode, NULL);
> + set_optab_libfunc (ne_optab, BFmode, NULL);
> + set_optab_libfunc (lt_optab, BFmode, NULL);
> + set_optab_libfunc (le_optab, BFmode, NULL);
> + set_optab_libfunc (ge_optab, BFmode, NULL);
> + set_optab_libfunc (gt_optab, BFmode, NULL);
> + set_optab_libfunc (unord_optab, BFmode, NULL);
Could you split this part out into a subroutine and reuse it for the
existing HFmode code too?
Thanks,
Richard
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GCC][BUG][Aarch64][ARM] (PR93300) Fix ICE due to BFmode placement in GET_MODES_WIDER chain.
2020-01-31 14:13 ` Richard Sandiford
@ 2020-02-04 11:30 ` Stam Markianos-Wright
2020-02-04 12:02 ` Richard Sandiford
0 siblings, 1 reply; 10+ messages in thread
From: Stam Markianos-Wright @ 2020-02-04 11:30 UTC (permalink / raw)
To: gcc-patches, Richard Earnshaw, nickc, ramana.radhakrishnan,
Kyrylo Tkachov, marcus.shawcroft, richard.sandiford
[-- Attachment #1: Type: text/plain, Size: 6124 bytes --]
On 1/31/20 1:45 PM, Richard Sandiford wrote:
> Stam Markianos-Wright <stam.markianos-wright@arm.com> writes:
>> On 1/30/20 10:01 AM, Richard Sandiford wrote:
>>> Stam Markianos-Wright <stam.markianos-wright@arm.com> writes:
>>>> On 1/29/20 12:42 PM, Richard Sandiford wrote:
>>>>> Stam Markianos-Wright <stam.markianos-wright@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.
>>>>
>>>> Hi Richard,
>>>>
>>>>>
>>>>> 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,
>>>>
>>>> Is that because functionally it looks like genmodes puts things in reverse
>>>> alphabetical order if all else is equal? (If I'm reading the comment about
>>>> MODE_RANDOM, MODE_CC correctly)
>>>>
>>>>> 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.
>>>>>
>>>> Yes, I see what you mean. I'll go through my thought process here:
>>>>
>>>> In investigating the ICE PR93300 I found that the diversion from pre-bf16
>>>> behaviour was specifically at `optabs.c:prepare_float_lib_cmp`, where a
>>>> `FOR_EACH_MODE_FROM (mode, orig_mode)` is used to then go off and generate
>>>> library calls for conversions.
>>>>
>>>> This was then being caught further down by the gcc_assert at expr.c:325 where
>>>> GET_MODE_PRECISION (from_mode) was equal to GET_MODE_PRECISION (to_mode) because
>>>> it was trying to emit a HF->BF conversion libcall as `bl __extendhfbf2` (which
>>>> is what happened if i removed the gcc_assert at expr.c:325)
>>>>
>>>> With BFmode being a target-defined mode, I didn't want to add something like `if
>>>> (mode != BFmode)` to specifically exclude BFmode from being selected for this.
>>>> (and there's nothing different between HFmode and BFmode here to allow me to
>>>> make this distinction?)
>>>>
>>>> Also I couldn't find anywhere where the target back-end is not consulted for a
>>>> "is this supported: yes/no" between the `FOR_EACH_MODE_FROM` loop and the
>>>> libcall being created later on as __extendhfbf2.
>>>
>>> Yeah, prepare_float_lib_cmp just checks for libfuncs rather than
>>> calling target hooks directly. The libfuncs themselves are under
>>> the control of the target though.
>>>
>>> By default we assume all float modes have associated libfuncs.
>>> It's then up to the target to remove functions that don't exist
>>> (or redirect them to other functions). So I think we need to remove
>>> BFmode libfuncs in arm_init_libfuncs in the same way as we currently
>>> do for HFmode.
>>>
>>> I guess we should also nullify the conversion libfuncs for BFmode,
>>> not just the arithmetic and comparison ones.
>>
>> Ahhh now this works, thank you for the suggestion!
>>
>> I was aware of arm_init_libfuncs, but I had not realised that returning NULL
>> would have the desired effect for us, in this case. So I have essentially rolled
>> back the whole previous version of the patch and done this in the new diff.
>> It seems to have fixed the ICE and I am currently in the process of regression
>> testing!
>
> LGTM behaviourally, just a couple of requests about how it's written:
>
>>
>> Thank you!
>> Stam
>>
>>>
>>> Thanks,
>>> Richard
>>>
>>>> Finally, because we really don't want __bf16 to be in the same "conversion rank"
>>>> as standard float modes for things like automatic promotion, this seemed like a
>>>> reasonable solution to that problem :)
>>>>
>>>> Let me know of your thoughts!
>>>>
>>>> Cheers,
>>>> Stam
>>
>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>> index c47fc232f39..18055d4a75e 100644
>> --- a/gcc/config/arm/arm.c
>> +++ b/gcc/config/arm/arm.c
>> @@ -2643,6 +2643,30 @@ arm_init_libfuncs (void)
>> default:
>> break;
>> }
>> +
>> + /* For all possible libcalls in BFmode, return NULL. */
>> + /* Conversions. */
>> + set_conv_libfunc (trunc_optab, BFmode, HFmode, (NULL));
>> + set_conv_libfunc (sext_optab, HFmode, BFmode, (NULL));
>> + set_conv_libfunc (trunc_optab, BFmode, SFmode, (NULL));
>> + set_conv_libfunc (sext_optab, SFmode, BFmode, (NULL));
>> + set_conv_libfunc (trunc_optab, BFmode, DFmode, (NULL));
>> + set_conv_libfunc (sext_optab, DFmode, BFmode, (NULL));
>
> It might be slightly safer to do:
>
> FOR_EACH_MODE_IN_CLASS (mode_iter, MODE_FLOAT)
>
> to iterate over all float modes on the non-BF side.
Done :)
>
>> + /* Arithmetic. */
>> + set_optab_libfunc (add_optab, BFmode, NULL);
>> + set_optab_libfunc (sdiv_optab, BFmode, NULL);
>> + set_optab_libfunc (smul_optab, BFmode, NULL);
>> + set_optab_libfunc (neg_optab, BFmode, NULL);
>> + set_optab_libfunc (sub_optab, BFmode, NULL);
>> +
>> + /* Comparisons. */
>> + set_optab_libfunc (eq_optab, BFmode, NULL);
>> + set_optab_libfunc (ne_optab, BFmode, NULL);
>> + set_optab_libfunc (lt_optab, BFmode, NULL);
>> + set_optab_libfunc (le_optab, BFmode, NULL);
>> + set_optab_libfunc (ge_optab, BFmode, NULL);
>> + set_optab_libfunc (gt_optab, BFmode, NULL);
>> + set_optab_libfunc (unord_optab, BFmode, NULL);
>
> Could you split this part out into a subroutine and reuse it for the
> existing HFmode code too?
Done
Let me know if you spot any other issues or nits of course!
Cheers,
Stam
New changelog:
gcc/ChangeLog:
2020-02-04 Stam Markianos-Wright <stam.markianos-wright@arm.com>
* config/arm/arm.c (arm_block_arith_comp_libfuncs_for_mode): New.
(arm_init_libfuncs): Add BFmode support to block spurious BF libfuncs.
Use arm_block_arith_comp_libfuncs_for_mode for HFmode.
>
> Thanks,
> Richard
>
[-- Attachment #2: BFmode_libfuncs.patch --]
[-- Type: text/x-patch, Size: 2746 bytes --]
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index c47fc232f39..80425f77f9d 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -2491,10 +2491,35 @@ arm_set_fixed_conv_libfunc (convert_optab optable, machine_mode to,
static GTY(()) rtx speculation_barrier_libfunc;
+/* Return NULL to block arithmetic and comparison libfuncs in
+ a specific machine_mode. */
+
+static void
+arm_block_arith_comp_libfuncs_for_mode (machine_mode mode)
+{
+ /* Arithmetic. */
+ set_optab_libfunc (add_optab, mode, NULL);
+ set_optab_libfunc (sdiv_optab, mode, NULL);
+ set_optab_libfunc (smul_optab, mode, NULL);
+ set_optab_libfunc (neg_optab, mode, NULL);
+ set_optab_libfunc (sub_optab, mode, NULL);
+
+ /* Comparisons. */
+ set_optab_libfunc (eq_optab, mode, NULL);
+ set_optab_libfunc (ne_optab, mode, NULL);
+ set_optab_libfunc (lt_optab, mode, NULL);
+ set_optab_libfunc (le_optab, mode, NULL);
+ set_optab_libfunc (ge_optab, mode, NULL);
+ set_optab_libfunc (gt_optab, mode, NULL);
+ set_optab_libfunc (unord_optab, mode, NULL);
+}
+
/* Set up library functions unique to ARM. */
static void
arm_init_libfuncs (void)
{
+ machine_mode mode_iter;
+
/* For Linux, we have access to kernel support for atomic operations. */
if (arm_abi == ARM_ABI_AAPCS_LINUX)
init_sync_libfuncs (MAX_SYNC_LIBFUNC_SIZE);
@@ -2623,27 +2648,22 @@ arm_init_libfuncs (void)
? "__gnu_d2h_ieee"
: "__gnu_d2h_alternative"));
- /* Arithmetic. */
- set_optab_libfunc (add_optab, HFmode, NULL);
- set_optab_libfunc (sdiv_optab, HFmode, NULL);
- set_optab_libfunc (smul_optab, HFmode, NULL);
- set_optab_libfunc (neg_optab, HFmode, NULL);
- set_optab_libfunc (sub_optab, HFmode, NULL);
+ arm_block_arith_comp_libfuncs_for_mode (HFmode);
- /* Comparisons. */
- set_optab_libfunc (eq_optab, HFmode, NULL);
- set_optab_libfunc (ne_optab, HFmode, NULL);
- set_optab_libfunc (lt_optab, HFmode, NULL);
- set_optab_libfunc (le_optab, HFmode, NULL);
- set_optab_libfunc (ge_optab, HFmode, NULL);
- set_optab_libfunc (gt_optab, HFmode, NULL);
- set_optab_libfunc (unord_optab, HFmode, NULL);
break;
default:
break;
}
+ /* For all possible libcalls in BFmode, return NULL. */
+ FOR_EACH_MODE_IN_CLASS (mode_iter, MODE_FLOAT)
+ {
+ set_conv_libfunc (trunc_optab, BFmode, mode_iter, (NULL));
+ set_conv_libfunc (sext_optab, mode_iter, BFmode, (NULL));
+ }
+ arm_block_arith_comp_libfuncs_for_mode (BFmode);
+
/* Use names prefixed with __gnu_ for fixed-point helper functions. */
{
const arm_fixed_mode_set fixed_arith_modes[] =
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GCC][BUG][Aarch64][ARM] (PR93300) Fix ICE due to BFmode placement in GET_MODES_WIDER chain.
2020-02-04 11:30 ` Stam Markianos-Wright
@ 2020-02-04 12:02 ` Richard Sandiford
2020-02-04 16:33 ` Stam Markianos-Wright
0 siblings, 1 reply; 10+ messages in thread
From: Richard Sandiford @ 2020-02-04 12:02 UTC (permalink / raw)
To: Stam Markianos-Wright
Cc: gcc-patches, Richard Earnshaw, nickc, ramana.radhakrishnan,
Kyrylo Tkachov, marcus.shawcroft
Stam Markianos-Wright <stam.markianos-wright@arm.com> writes:
> On 1/31/20 1:45 PM, Richard Sandiford wrote:
>> Stam Markianos-Wright <stam.markianos-wright@arm.com> writes:
>>> On 1/30/20 10:01 AM, Richard Sandiford wrote:
>>>> Stam Markianos-Wright <stam.markianos-wright@arm.com> writes:
>>>>> On 1/29/20 12:42 PM, Richard Sandiford wrote:
>>>>>> Stam Markianos-Wright <stam.markianos-wright@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.
>>>>>
>>>>> Hi Richard,
>>>>>
>>>>>>
>>>>>> 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,
>>>>>
>>>>> Is that because functionally it looks like genmodes puts things in reverse
>>>>> alphabetical order if all else is equal? (If I'm reading the comment about
>>>>> MODE_RANDOM, MODE_CC correctly)
>>>>>
>>>>>> 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.
>>>>>>
>>>>> Yes, I see what you mean. I'll go through my thought process here:
>>>>>
>>>>> In investigating the ICE PR93300 I found that the diversion from pre-bf16
>>>>> behaviour was specifically at `optabs.c:prepare_float_lib_cmp`, where a
>>>>> `FOR_EACH_MODE_FROM (mode, orig_mode)` is used to then go off and generate
>>>>> library calls for conversions.
>>>>>
>>>>> This was then being caught further down by the gcc_assert at expr.c:325 where
>>>>> GET_MODE_PRECISION (from_mode) was equal to GET_MODE_PRECISION (to_mode) because
>>>>> it was trying to emit a HF->BF conversion libcall as `bl __extendhfbf2` (which
>>>>> is what happened if i removed the gcc_assert at expr.c:325)
>>>>>
>>>>> With BFmode being a target-defined mode, I didn't want to add something like `if
>>>>> (mode != BFmode)` to specifically exclude BFmode from being selected for this.
>>>>> (and there's nothing different between HFmode and BFmode here to allow me to
>>>>> make this distinction?)
>>>>>
>>>>> Also I couldn't find anywhere where the target back-end is not consulted for a
>>>>> "is this supported: yes/no" between the `FOR_EACH_MODE_FROM` loop and the
>>>>> libcall being created later on as __extendhfbf2.
>>>>
>>>> Yeah, prepare_float_lib_cmp just checks for libfuncs rather than
>>>> calling target hooks directly. The libfuncs themselves are under
>>>> the control of the target though.
>>>>
>>>> By default we assume all float modes have associated libfuncs.
>>>> It's then up to the target to remove functions that don't exist
>>>> (or redirect them to other functions). So I think we need to remove
>>>> BFmode libfuncs in arm_init_libfuncs in the same way as we currently
>>>> do for HFmode.
>>>>
>>>> I guess we should also nullify the conversion libfuncs for BFmode,
>>>> not just the arithmetic and comparison ones.
>>>
>>> Ahhh now this works, thank you for the suggestion!
>>>
>>> I was aware of arm_init_libfuncs, but I had not realised that returning NULL
>>> would have the desired effect for us, in this case. So I have essentially rolled
>>> back the whole previous version of the patch and done this in the new diff.
>>> It seems to have fixed the ICE and I am currently in the process of regression
>>> testing!
>>
>> LGTM behaviourally, just a couple of requests about how it's written:
>>
>>>
>>> Thank you!
>>> Stam
>>>
>>>>
>>>> Thanks,
>>>> Richard
>>>>
>>>>> Finally, because we really don't want __bf16 to be in the same "conversion rank"
>>>>> as standard float modes for things like automatic promotion, this seemed like a
>>>>> reasonable solution to that problem :)
>>>>>
>>>>> Let me know of your thoughts!
>>>>>
>>>>> Cheers,
>>>>> Stam
>>>
>>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>>> index c47fc232f39..18055d4a75e 100644
>>> --- a/gcc/config/arm/arm.c
>>> +++ b/gcc/config/arm/arm.c
>>> @@ -2643,6 +2643,30 @@ arm_init_libfuncs (void)
>>> default:
>>> break;
>>> }
>>> +
>>> + /* For all possible libcalls in BFmode, return NULL. */
>>> + /* Conversions. */
>>> + set_conv_libfunc (trunc_optab, BFmode, HFmode, (NULL));
>>> + set_conv_libfunc (sext_optab, HFmode, BFmode, (NULL));
>>> + set_conv_libfunc (trunc_optab, BFmode, SFmode, (NULL));
>>> + set_conv_libfunc (sext_optab, SFmode, BFmode, (NULL));
>>> + set_conv_libfunc (trunc_optab, BFmode, DFmode, (NULL));
>>> + set_conv_libfunc (sext_optab, DFmode, BFmode, (NULL));
>>
>> It might be slightly safer to do:
>>
>> FOR_EACH_MODE_IN_CLASS (mode_iter, MODE_FLOAT)
>>
>> to iterate over all float modes on the non-BF side.
>
> Done :)
>>
>>> + /* Arithmetic. */
>>> + set_optab_libfunc (add_optab, BFmode, NULL);
>>> + set_optab_libfunc (sdiv_optab, BFmode, NULL);
>>> + set_optab_libfunc (smul_optab, BFmode, NULL);
>>> + set_optab_libfunc (neg_optab, BFmode, NULL);
>>> + set_optab_libfunc (sub_optab, BFmode, NULL);
>>> +
>>> + /* Comparisons. */
>>> + set_optab_libfunc (eq_optab, BFmode, NULL);
>>> + set_optab_libfunc (ne_optab, BFmode, NULL);
>>> + set_optab_libfunc (lt_optab, BFmode, NULL);
>>> + set_optab_libfunc (le_optab, BFmode, NULL);
>>> + set_optab_libfunc (ge_optab, BFmode, NULL);
>>> + set_optab_libfunc (gt_optab, BFmode, NULL);
>>> + set_optab_libfunc (unord_optab, BFmode, NULL);
>>
>> Could you split this part out into a subroutine and reuse it for the
>> existing HFmode code too?
>
> Done
>
> Let me know if you spot any other issues or nits of course!
>
> Cheers,
> Stam
>
> New changelog:
>
> gcc/ChangeLog:
>
> 2020-02-04 Stam Markianos-Wright <stam.markianos-wright@arm.com>
>
> * config/arm/arm.c (arm_block_arith_comp_libfuncs_for_mode): New.
> (arm_init_libfuncs): Add BFmode support to block spurious BF libfuncs.
> Use arm_block_arith_comp_libfuncs_for_mode for HFmode.
>
>
>>
>> Thanks,
>> Richard
>>
>
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index c47fc232f39..80425f77f9d 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -2491,10 +2491,35 @@ arm_set_fixed_conv_libfunc (convert_optab optable, machine_mode to,
>
> static GTY(()) rtx speculation_barrier_libfunc;
>
> +/* Return NULL to block arithmetic and comparison libfuncs in
> + a specific machine_mode. */
"Return" to me sounds like this is talking about the return value
of the function. Maybe something like:
/* Record that we have no arithmetic or comparison libfuncs for
machinde mode MODE. */
> +
> +static void
> +arm_block_arith_comp_libfuncs_for_mode (machine_mode mode)
> +{
> + /* Arithmetic. */
> + set_optab_libfunc (add_optab, mode, NULL);
> + set_optab_libfunc (sdiv_optab, mode, NULL);
> + set_optab_libfunc (smul_optab, mode, NULL);
> + set_optab_libfunc (neg_optab, mode, NULL);
> + set_optab_libfunc (sub_optab, mode, NULL);
> +
> + /* Comparisons. */
> + set_optab_libfunc (eq_optab, mode, NULL);
> + set_optab_libfunc (ne_optab, mode, NULL);
> + set_optab_libfunc (lt_optab, mode, NULL);
> + set_optab_libfunc (le_optab, mode, NULL);
> + set_optab_libfunc (ge_optab, mode, NULL);
> + set_optab_libfunc (gt_optab, mode, NULL);
> + set_optab_libfunc (unord_optab, mode, NULL);
Too much indentation, should just be two spaces.
> +}
> +
> /* Set up library functions unique to ARM. */
> static void
> arm_init_libfuncs (void)
> {
> + machine_mode mode_iter;
> +
> /* For Linux, we have access to kernel support for atomic operations. */
> if (arm_abi == ARM_ABI_AAPCS_LINUX)
> init_sync_libfuncs (MAX_SYNC_LIBFUNC_SIZE);
> @@ -2623,27 +2648,22 @@ arm_init_libfuncs (void)
> ? "__gnu_d2h_ieee"
> : "__gnu_d2h_alternative"));
>
> - /* Arithmetic. */
> - set_optab_libfunc (add_optab, HFmode, NULL);
> - set_optab_libfunc (sdiv_optab, HFmode, NULL);
> - set_optab_libfunc (smul_optab, HFmode, NULL);
> - set_optab_libfunc (neg_optab, HFmode, NULL);
> - set_optab_libfunc (sub_optab, HFmode, NULL);
> + arm_block_arith_comp_libfuncs_for_mode (HFmode);
>
> - /* Comparisons. */
> - set_optab_libfunc (eq_optab, HFmode, NULL);
> - set_optab_libfunc (ne_optab, HFmode, NULL);
> - set_optab_libfunc (lt_optab, HFmode, NULL);
> - set_optab_libfunc (le_optab, HFmode, NULL);
> - set_optab_libfunc (ge_optab, HFmode, NULL);
> - set_optab_libfunc (gt_optab, HFmode, NULL);
> - set_optab_libfunc (unord_optab, HFmode, NULL);
> break;
Nit: no need for a blank line before "break;".
>
> default:
> break;
> }
>
> + /* For all possible libcalls in BFmode, return NULL. */
> + FOR_EACH_MODE_IN_CLASS (mode_iter, MODE_FLOAT)
> + {
> + set_conv_libfunc (trunc_optab, BFmode, mode_iter, (NULL));
> + set_conv_libfunc (sext_optab, mode_iter, BFmode, (NULL));
It would probably be safer to use both argument orders for each optab,
since BFmode isn't necessarily the "narrowest" mode.
Nit: no need for brackets around NULL.
Thanks,
Richard
> + }
> + arm_block_arith_comp_libfuncs_for_mode (BFmode);
> +
> /* Use names prefixed with __gnu_ for fixed-point helper functions. */
> {
> const arm_fixed_mode_set fixed_arith_modes[] =
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GCC][BUG][Aarch64][ARM] (PR93300) Fix ICE due to BFmode placement in GET_MODES_WIDER chain.
2020-02-04 12:02 ` Richard Sandiford
@ 2020-02-04 16:33 ` Stam Markianos-Wright
2020-02-05 16:37 ` Richard Sandiford
0 siblings, 1 reply; 10+ messages in thread
From: Stam Markianos-Wright @ 2020-02-04 16:33 UTC (permalink / raw)
To: gcc-patches, Richard Earnshaw, nickc, ramana.radhakrishnan,
Kyrylo Tkachov, marcus.shawcroft, richard.sandiford
[-- Attachment #1: Type: text/plain, Size: 10265 bytes --]
On 2/4/20 12:02 PM, Richard Sandiford wrote:
> Stam Markianos-Wright <stam.markianos-wright@arm.com> writes:
>> On 1/31/20 1:45 PM, Richard Sandiford wrote:
>>> Stam Markianos-Wright <stam.markianos-wright@arm.com> writes:
>>>> On 1/30/20 10:01 AM, Richard Sandiford wrote:
>>>>> Stam Markianos-Wright <stam.markianos-wright@arm.com> writes:
>>>>>> On 1/29/20 12:42 PM, Richard Sandiford wrote:
>>>>>>> Stam Markianos-Wright <stam.markianos-wright@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.
>>>>>>
>>>>>> Hi Richard,
>>>>>>
>>>>>>>
>>>>>>> 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,
>>>>>>
>>>>>> Is that because functionally it looks like genmodes puts things in reverse
>>>>>> alphabetical order if all else is equal? (If I'm reading the comment about
>>>>>> MODE_RANDOM, MODE_CC correctly)
>>>>>>
>>>>>>> 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.
>>>>>>>
>>>>>> Yes, I see what you mean. I'll go through my thought process here:
>>>>>>
>>>>>> In investigating the ICE PR93300 I found that the diversion from pre-bf16
>>>>>> behaviour was specifically at `optabs.c:prepare_float_lib_cmp`, where a
>>>>>> `FOR_EACH_MODE_FROM (mode, orig_mode)` is used to then go off and generate
>>>>>> library calls for conversions.
>>>>>>
>>>>>> This was then being caught further down by the gcc_assert at expr.c:325 where
>>>>>> GET_MODE_PRECISION (from_mode) was equal to GET_MODE_PRECISION (to_mode) because
>>>>>> it was trying to emit a HF->BF conversion libcall as `bl __extendhfbf2` (which
>>>>>> is what happened if i removed the gcc_assert at expr.c:325)
>>>>>>
>>>>>> With BFmode being a target-defined mode, I didn't want to add something like `if
>>>>>> (mode != BFmode)` to specifically exclude BFmode from being selected for this.
>>>>>> (and there's nothing different between HFmode and BFmode here to allow me to
>>>>>> make this distinction?)
>>>>>>
>>>>>> Also I couldn't find anywhere where the target back-end is not consulted for a
>>>>>> "is this supported: yes/no" between the `FOR_EACH_MODE_FROM` loop and the
>>>>>> libcall being created later on as __extendhfbf2.
>>>>>
>>>>> Yeah, prepare_float_lib_cmp just checks for libfuncs rather than
>>>>> calling target hooks directly. The libfuncs themselves are under
>>>>> the control of the target though.
>>>>>
>>>>> By default we assume all float modes have associated libfuncs.
>>>>> It's then up to the target to remove functions that don't exist
>>>>> (or redirect them to other functions). So I think we need to remove
>>>>> BFmode libfuncs in arm_init_libfuncs in the same way as we currently
>>>>> do for HFmode.
>>>>>
>>>>> I guess we should also nullify the conversion libfuncs for BFmode,
>>>>> not just the arithmetic and comparison ones.
>>>>
>>>> Ahhh now this works, thank you for the suggestion!
>>>>
>>>> I was aware of arm_init_libfuncs, but I had not realised that returning NULL
>>>> would have the desired effect for us, in this case. So I have essentially rolled
>>>> back the whole previous version of the patch and done this in the new diff.
>>>> It seems to have fixed the ICE and I am currently in the process of regression
>>>> testing!
>>>
>>> LGTM behaviourally, just a couple of requests about how it's written:
>>>
>>>>
>>>> Thank you!
>>>> Stam
>>>>
>>>>>
>>>>> Thanks,
>>>>> Richard
>>>>>
>>>>>> Finally, because we really don't want __bf16 to be in the same "conversion rank"
>>>>>> as standard float modes for things like automatic promotion, this seemed like a
>>>>>> reasonable solution to that problem :)
>>>>>>
>>>>>> Let me know of your thoughts!
>>>>>>
>>>>>> Cheers,
>>>>>> Stam
>>>>
>>>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>>>> index c47fc232f39..18055d4a75e 100644
>>>> --- a/gcc/config/arm/arm.c
>>>> +++ b/gcc/config/arm/arm.c
>>>> @@ -2643,6 +2643,30 @@ arm_init_libfuncs (void)
>>>> default:
>>>> break;
>>>> }
>>>> +
>>>> + /* For all possible libcalls in BFmode, return NULL. */
>>>> + /* Conversions. */
>>>> + set_conv_libfunc (trunc_optab, BFmode, HFmode, (NULL));
>>>> + set_conv_libfunc (sext_optab, HFmode, BFmode, (NULL));
>>>> + set_conv_libfunc (trunc_optab, BFmode, SFmode, (NULL));
>>>> + set_conv_libfunc (sext_optab, SFmode, BFmode, (NULL));
>>>> + set_conv_libfunc (trunc_optab, BFmode, DFmode, (NULL));
>>>> + set_conv_libfunc (sext_optab, DFmode, BFmode, (NULL));
>>>
>>> It might be slightly safer to do:
>>>
>>> FOR_EACH_MODE_IN_CLASS (mode_iter, MODE_FLOAT)
>>>
>>> to iterate over all float modes on the non-BF side.
>>
>> Done :)
>>>
>>>> + /* Arithmetic. */
>>>> + set_optab_libfunc (add_optab, BFmode, NULL);
>>>> + set_optab_libfunc (sdiv_optab, BFmode, NULL);
>>>> + set_optab_libfunc (smul_optab, BFmode, NULL);
>>>> + set_optab_libfunc (neg_optab, BFmode, NULL);
>>>> + set_optab_libfunc (sub_optab, BFmode, NULL);
>>>> +
>>>> + /* Comparisons. */
>>>> + set_optab_libfunc (eq_optab, BFmode, NULL);
>>>> + set_optab_libfunc (ne_optab, BFmode, NULL);
>>>> + set_optab_libfunc (lt_optab, BFmode, NULL);
>>>> + set_optab_libfunc (le_optab, BFmode, NULL);
>>>> + set_optab_libfunc (ge_optab, BFmode, NULL);
>>>> + set_optab_libfunc (gt_optab, BFmode, NULL);
>>>> + set_optab_libfunc (unord_optab, BFmode, NULL);
>>>
>>> Could you split this part out into a subroutine and reuse it for the
>>> existing HFmode code too?
>>
>> Done
>>
>> Let me know if you spot any other issues or nits of course!
>>
>> Cheers,
>> Stam
>>
>> New changelog:
>>
>> gcc/ChangeLog:
>>
>> 2020-02-04 Stam Markianos-Wright <stam.markianos-wright@arm.com>
>>
>> * config/arm/arm.c (arm_block_arith_comp_libfuncs_for_mode): New.
>> (arm_init_libfuncs): Add BFmode support to block spurious BF libfuncs.
>> Use arm_block_arith_comp_libfuncs_for_mode for HFmode.
>>
>>
>>>
>>> Thanks,
>>> Richard
>>>
>>
>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>> index c47fc232f39..80425f77f9d 100644
>> --- a/gcc/config/arm/arm.c
>> +++ b/gcc/config/arm/arm.c
>> @@ -2491,10 +2491,35 @@ arm_set_fixed_conv_libfunc (convert_optab optable, machine_mode to,
>>
>> static GTY(()) rtx speculation_barrier_libfunc;
>>
>> +/* Return NULL to block arithmetic and comparison libfuncs in
>> + a specific machine_mode. */
>
> "Return" to me sounds like this is talking about the return value
> of the function. Maybe something like:
>
> /* Record that we have no arithmetic or comparison libfuncs for
> machinde mode MODE. */
Done
>
>> +
>> +static void
>> +arm_block_arith_comp_libfuncs_for_mode (machine_mode mode)
>> +{
>> + /* Arithmetic. */
>> + set_optab_libfunc (add_optab, mode, NULL);
>> + set_optab_libfunc (sdiv_optab, mode, NULL);
>> + set_optab_libfunc (smul_optab, mode, NULL);
>> + set_optab_libfunc (neg_optab, mode, NULL);
>> + set_optab_libfunc (sub_optab, mode, NULL);
>> +
>> + /* Comparisons. */
>> + set_optab_libfunc (eq_optab, mode, NULL);
>> + set_optab_libfunc (ne_optab, mode, NULL);
>> + set_optab_libfunc (lt_optab, mode, NULL);
>> + set_optab_libfunc (le_optab, mode, NULL);
>> + set_optab_libfunc (ge_optab, mode, NULL);
>> + set_optab_libfunc (gt_optab, mode, NULL);
>> + set_optab_libfunc (unord_optab, mode, NULL);
>
> Too much indentation, should just be two spaces.
Done, sorry I didn't see that!
>
>> +}
>> +
>> /* Set up library functions unique to ARM. */
>> static void
>> arm_init_libfuncs (void)
>> {
>> + machine_mode mode_iter;
>> +
>> /* For Linux, we have access to kernel support for atomic operations. */
>> if (arm_abi == ARM_ABI_AAPCS_LINUX)
>> init_sync_libfuncs (MAX_SYNC_LIBFUNC_SIZE);
>> @@ -2623,27 +2648,22 @@ arm_init_libfuncs (void)
>> ? "__gnu_d2h_ieee"
>> : "__gnu_d2h_alternative"));
>>
>> - /* Arithmetic. */
>> - set_optab_libfunc (add_optab, HFmode, NULL);
>> - set_optab_libfunc (sdiv_optab, HFmode, NULL);
>> - set_optab_libfunc (smul_optab, HFmode, NULL);
>> - set_optab_libfunc (neg_optab, HFmode, NULL);
>> - set_optab_libfunc (sub_optab, HFmode, NULL);
>> + arm_block_arith_comp_libfuncs_for_mode (HFmode);
>>
>> - /* Comparisons. */
>> - set_optab_libfunc (eq_optab, HFmode, NULL);
>> - set_optab_libfunc (ne_optab, HFmode, NULL);
>> - set_optab_libfunc (lt_optab, HFmode, NULL);
>> - set_optab_libfunc (le_optab, HFmode, NULL);
>> - set_optab_libfunc (ge_optab, HFmode, NULL);
>> - set_optab_libfunc (gt_optab, HFmode, NULL);
>> - set_optab_libfunc (unord_optab, HFmode, NULL);
>> break;
>
> Nit: no need for a blank line before "break;".
Done
>
>>
>> default:
>> break;
>> }
>>
>> + /* For all possible libcalls in BFmode, return NULL. */
>> + FOR_EACH_MODE_IN_CLASS (mode_iter, MODE_FLOAT)
>> + {
>> + set_conv_libfunc (trunc_optab, BFmode, mode_iter, (NULL));
>> + set_conv_libfunc (sext_optab, mode_iter, BFmode, (NULL));
>
> It would probably be safer to use both argument orders for each optab,
> since BFmode isn't necessarily the "narrowest" mode.
>
> Nit: no need for brackets around NULL.
Done
Let me know if this is good to go now :)
Thank you so much for the help getting this finalised!
Cheers,
Stam
>
> Thanks,
> Richard
>
>> + }
>> + arm_block_arith_comp_libfuncs_for_mode (BFmode);
>> +
>> /* Use names prefixed with __gnu_ for fixed-point helper functions. */
>> {
>> const arm_fixed_mode_set fixed_arith_modes[] =
[-- Attachment #2: BFmode_libfuncs-3.patch --]
[-- Type: text/plain, Size: 2809 bytes --]
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index c47fc232f39..ad1c1fa8e3b 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -2491,10 +2491,35 @@ arm_set_fixed_conv_libfunc (convert_optab optable, machine_mode to,
static GTY(()) rtx speculation_barrier_libfunc;
+/* Record that we have no arithmetic or comparison libfuncs for
+ machine mode MODE. */
+
+static void
+arm_block_arith_comp_libfuncs_for_mode (machine_mode mode)
+{
+ /* Arithmetic. */
+ set_optab_libfunc (add_optab, mode, NULL);
+ set_optab_libfunc (sdiv_optab, mode, NULL);
+ set_optab_libfunc (smul_optab, mode, NULL);
+ set_optab_libfunc (neg_optab, mode, NULL);
+ set_optab_libfunc (sub_optab, mode, NULL);
+
+ /* Comparisons. */
+ set_optab_libfunc (eq_optab, mode, NULL);
+ set_optab_libfunc (ne_optab, mode, NULL);
+ set_optab_libfunc (lt_optab, mode, NULL);
+ set_optab_libfunc (le_optab, mode, NULL);
+ set_optab_libfunc (ge_optab, mode, NULL);
+ set_optab_libfunc (gt_optab, mode, NULL);
+ set_optab_libfunc (unord_optab, mode, NULL);
+}
+
/* Set up library functions unique to ARM. */
static void
arm_init_libfuncs (void)
{
+ machine_mode mode_iter;
+
/* For Linux, we have access to kernel support for atomic operations. */
if (arm_abi == ARM_ABI_AAPCS_LINUX)
init_sync_libfuncs (MAX_SYNC_LIBFUNC_SIZE);
@@ -2623,27 +2648,23 @@ arm_init_libfuncs (void)
? "__gnu_d2h_ieee"
: "__gnu_d2h_alternative"));
- /* Arithmetic. */
- set_optab_libfunc (add_optab, HFmode, NULL);
- set_optab_libfunc (sdiv_optab, HFmode, NULL);
- set_optab_libfunc (smul_optab, HFmode, NULL);
- set_optab_libfunc (neg_optab, HFmode, NULL);
- set_optab_libfunc (sub_optab, HFmode, NULL);
-
- /* Comparisons. */
- set_optab_libfunc (eq_optab, HFmode, NULL);
- set_optab_libfunc (ne_optab, HFmode, NULL);
- set_optab_libfunc (lt_optab, HFmode, NULL);
- set_optab_libfunc (le_optab, HFmode, NULL);
- set_optab_libfunc (ge_optab, HFmode, NULL);
- set_optab_libfunc (gt_optab, HFmode, NULL);
- set_optab_libfunc (unord_optab, HFmode, NULL);
+ arm_block_arith_comp_libfuncs_for_mode (HFmode);
break;
default:
break;
}
+ /* For all possible libcalls in BFmode, record NULL. */
+ FOR_EACH_MODE_IN_CLASS (mode_iter, MODE_FLOAT)
+ {
+ set_conv_libfunc (trunc_optab, BFmode, mode_iter, NULL);
+ set_conv_libfunc (trunc_optab, mode_iter, BFmode, NULL);
+ set_conv_libfunc (sext_optab, mode_iter, BFmode, NULL);
+ set_conv_libfunc (sext_optab, BFmode, mode_iter, NULL);
+ }
+ arm_block_arith_comp_libfuncs_for_mode (BFmode);
+
/* Use names prefixed with __gnu_ for fixed-point helper functions. */
{
const arm_fixed_mode_set fixed_arith_modes[] =
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [GCC][BUG][Aarch64][ARM] (PR93300) Fix ICE due to BFmode placement in GET_MODES_WIDER chain.
2020-02-04 16:33 ` Stam Markianos-Wright
@ 2020-02-05 16:37 ` Richard Sandiford
0 siblings, 0 replies; 10+ messages in thread
From: Richard Sandiford @ 2020-02-05 16:37 UTC (permalink / raw)
To: Stam Markianos-Wright
Cc: gcc-patches, Richard Earnshaw, nickc, ramana.radhakrishnan,
Kyrylo Tkachov, marcus.shawcroft
Stam Markianos-Wright <stam.markianos-wright@arm.com> writes:
> On 2/4/20 12:02 PM, Richard Sandiford wrote:
>> Stam Markianos-Wright <stam.markianos-wright@arm.com> writes:
>>> On 1/31/20 1:45 PM, Richard Sandiford wrote:
>>>> Stam Markianos-Wright <stam.markianos-wright@arm.com> writes:
>>>>> On 1/30/20 10:01 AM, Richard Sandiford wrote:
>>>>>> Stam Markianos-Wright <stam.markianos-wright@arm.com> writes:
>>>>>>> On 1/29/20 12:42 PM, Richard Sandiford wrote:
>>>>>>>> Stam Markianos-Wright <stam.markianos-wright@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.
>>>>>>>
>>>>>>> Hi Richard,
>>>>>>>
>>>>>>>>
>>>>>>>> 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,
>>>>>>>
>>>>>>> Is that because functionally it looks like genmodes puts things in reverse
>>>>>>> alphabetical order if all else is equal? (If I'm reading the comment about
>>>>>>> MODE_RANDOM, MODE_CC correctly)
>>>>>>>
>>>>>>>> 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.
>>>>>>>>
>>>>>>> Yes, I see what you mean. I'll go through my thought process here:
>>>>>>>
>>>>>>> In investigating the ICE PR93300 I found that the diversion from pre-bf16
>>>>>>> behaviour was specifically at `optabs.c:prepare_float_lib_cmp`, where a
>>>>>>> `FOR_EACH_MODE_FROM (mode, orig_mode)` is used to then go off and generate
>>>>>>> library calls for conversions.
>>>>>>>
>>>>>>> This was then being caught further down by the gcc_assert at expr.c:325 where
>>>>>>> GET_MODE_PRECISION (from_mode) was equal to GET_MODE_PRECISION (to_mode) because
>>>>>>> it was trying to emit a HF->BF conversion libcall as `bl __extendhfbf2` (which
>>>>>>> is what happened if i removed the gcc_assert at expr.c:325)
>>>>>>>
>>>>>>> With BFmode being a target-defined mode, I didn't want to add something like `if
>>>>>>> (mode != BFmode)` to specifically exclude BFmode from being selected for this.
>>>>>>> (and there's nothing different between HFmode and BFmode here to allow me to
>>>>>>> make this distinction?)
>>>>>>>
>>>>>>> Also I couldn't find anywhere where the target back-end is not consulted for a
>>>>>>> "is this supported: yes/no" between the `FOR_EACH_MODE_FROM` loop and the
>>>>>>> libcall being created later on as __extendhfbf2.
>>>>>>
>>>>>> Yeah, prepare_float_lib_cmp just checks for libfuncs rather than
>>>>>> calling target hooks directly. The libfuncs themselves are under
>>>>>> the control of the target though.
>>>>>>
>>>>>> By default we assume all float modes have associated libfuncs.
>>>>>> It's then up to the target to remove functions that don't exist
>>>>>> (or redirect them to other functions). So I think we need to remove
>>>>>> BFmode libfuncs in arm_init_libfuncs in the same way as we currently
>>>>>> do for HFmode.
>>>>>>
>>>>>> I guess we should also nullify the conversion libfuncs for BFmode,
>>>>>> not just the arithmetic and comparison ones.
>>>>>
>>>>> Ahhh now this works, thank you for the suggestion!
>>>>>
>>>>> I was aware of arm_init_libfuncs, but I had not realised that returning NULL
>>>>> would have the desired effect for us, in this case. So I have essentially rolled
>>>>> back the whole previous version of the patch and done this in the new diff.
>>>>> It seems to have fixed the ICE and I am currently in the process of regression
>>>>> testing!
>>>>
>>>> LGTM behaviourally, just a couple of requests about how it's written:
>>>>
>>>>>
>>>>> Thank you!
>>>>> Stam
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Richard
>>>>>>
>>>>>>> Finally, because we really don't want __bf16 to be in the same "conversion rank"
>>>>>>> as standard float modes for things like automatic promotion, this seemed like a
>>>>>>> reasonable solution to that problem :)
>>>>>>>
>>>>>>> Let me know of your thoughts!
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Stam
>>>>>
>>>>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>>>>> index c47fc232f39..18055d4a75e 100644
>>>>> --- a/gcc/config/arm/arm.c
>>>>> +++ b/gcc/config/arm/arm.c
>>>>> @@ -2643,6 +2643,30 @@ arm_init_libfuncs (void)
>>>>> default:
>>>>> break;
>>>>> }
>>>>> +
>>>>> + /* For all possible libcalls in BFmode, return NULL. */
>>>>> + /* Conversions. */
>>>>> + set_conv_libfunc (trunc_optab, BFmode, HFmode, (NULL));
>>>>> + set_conv_libfunc (sext_optab, HFmode, BFmode, (NULL));
>>>>> + set_conv_libfunc (trunc_optab, BFmode, SFmode, (NULL));
>>>>> + set_conv_libfunc (sext_optab, SFmode, BFmode, (NULL));
>>>>> + set_conv_libfunc (trunc_optab, BFmode, DFmode, (NULL));
>>>>> + set_conv_libfunc (sext_optab, DFmode, BFmode, (NULL));
>>>>
>>>> It might be slightly safer to do:
>>>>
>>>> FOR_EACH_MODE_IN_CLASS (mode_iter, MODE_FLOAT)
>>>>
>>>> to iterate over all float modes on the non-BF side.
>>>
>>> Done :)
>>>>
>>>>> + /* Arithmetic. */
>>>>> + set_optab_libfunc (add_optab, BFmode, NULL);
>>>>> + set_optab_libfunc (sdiv_optab, BFmode, NULL);
>>>>> + set_optab_libfunc (smul_optab, BFmode, NULL);
>>>>> + set_optab_libfunc (neg_optab, BFmode, NULL);
>>>>> + set_optab_libfunc (sub_optab, BFmode, NULL);
>>>>> +
>>>>> + /* Comparisons. */
>>>>> + set_optab_libfunc (eq_optab, BFmode, NULL);
>>>>> + set_optab_libfunc (ne_optab, BFmode, NULL);
>>>>> + set_optab_libfunc (lt_optab, BFmode, NULL);
>>>>> + set_optab_libfunc (le_optab, BFmode, NULL);
>>>>> + set_optab_libfunc (ge_optab, BFmode, NULL);
>>>>> + set_optab_libfunc (gt_optab, BFmode, NULL);
>>>>> + set_optab_libfunc (unord_optab, BFmode, NULL);
>>>>
>>>> Could you split this part out into a subroutine and reuse it for the
>>>> existing HFmode code too?
>>>
>>> Done
>>>
>>> Let me know if you spot any other issues or nits of course!
>>>
>>> Cheers,
>>> Stam
>>>
>>> New changelog:
>>>
>>> gcc/ChangeLog:
>>>
>>> 2020-02-04 Stam Markianos-Wright <stam.markianos-wright@arm.com>
>>>
>>> * config/arm/arm.c (arm_block_arith_comp_libfuncs_for_mode): New.
>>> (arm_init_libfuncs): Add BFmode support to block spurious BF libfuncs.
>>> Use arm_block_arith_comp_libfuncs_for_mode for HFmode.
>>>
>>>
>>>>
>>>> Thanks,
>>>> Richard
>>>>
>>>
>>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
>>> index c47fc232f39..80425f77f9d 100644
>>> --- a/gcc/config/arm/arm.c
>>> +++ b/gcc/config/arm/arm.c
>>> @@ -2491,10 +2491,35 @@ arm_set_fixed_conv_libfunc (convert_optab optable, machine_mode to,
>>>
>>> static GTY(()) rtx speculation_barrier_libfunc;
>>>
>>> +/* Return NULL to block arithmetic and comparison libfuncs in
>>> + a specific machine_mode. */
>>
>> "Return" to me sounds like this is talking about the return value
>> of the function. Maybe something like:
>>
>> /* Record that we have no arithmetic or comparison libfuncs for
>> machinde mode MODE. */
> Done
>>
>>> +
>>> +static void
>>> +arm_block_arith_comp_libfuncs_for_mode (machine_mode mode)
>>> +{
>>> + /* Arithmetic. */
>>> + set_optab_libfunc (add_optab, mode, NULL);
>>> + set_optab_libfunc (sdiv_optab, mode, NULL);
>>> + set_optab_libfunc (smul_optab, mode, NULL);
>>> + set_optab_libfunc (neg_optab, mode, NULL);
>>> + set_optab_libfunc (sub_optab, mode, NULL);
>>> +
>>> + /* Comparisons. */
>>> + set_optab_libfunc (eq_optab, mode, NULL);
>>> + set_optab_libfunc (ne_optab, mode, NULL);
>>> + set_optab_libfunc (lt_optab, mode, NULL);
>>> + set_optab_libfunc (le_optab, mode, NULL);
>>> + set_optab_libfunc (ge_optab, mode, NULL);
>>> + set_optab_libfunc (gt_optab, mode, NULL);
>>> + set_optab_libfunc (unord_optab, mode, NULL);
>>
>> Too much indentation, should just be two spaces.
> Done, sorry I didn't see that!
>>
>>> +}
>>> +
>>> /* Set up library functions unique to ARM. */
>>> static void
>>> arm_init_libfuncs (void)
>>> {
>>> + machine_mode mode_iter;
>>> +
>>> /* For Linux, we have access to kernel support for atomic operations. */
>>> if (arm_abi == ARM_ABI_AAPCS_LINUX)
>>> init_sync_libfuncs (MAX_SYNC_LIBFUNC_SIZE);
>>> @@ -2623,27 +2648,22 @@ arm_init_libfuncs (void)
>>> ? "__gnu_d2h_ieee"
>>> : "__gnu_d2h_alternative"));
>>>
>>> - /* Arithmetic. */
>>> - set_optab_libfunc (add_optab, HFmode, NULL);
>>> - set_optab_libfunc (sdiv_optab, HFmode, NULL);
>>> - set_optab_libfunc (smul_optab, HFmode, NULL);
>>> - set_optab_libfunc (neg_optab, HFmode, NULL);
>>> - set_optab_libfunc (sub_optab, HFmode, NULL);
>>> + arm_block_arith_comp_libfuncs_for_mode (HFmode);
>>>
>>> - /* Comparisons. */
>>> - set_optab_libfunc (eq_optab, HFmode, NULL);
>>> - set_optab_libfunc (ne_optab, HFmode, NULL);
>>> - set_optab_libfunc (lt_optab, HFmode, NULL);
>>> - set_optab_libfunc (le_optab, HFmode, NULL);
>>> - set_optab_libfunc (ge_optab, HFmode, NULL);
>>> - set_optab_libfunc (gt_optab, HFmode, NULL);
>>> - set_optab_libfunc (unord_optab, HFmode, NULL);
>>> break;
>>
>> Nit: no need for a blank line before "break;".
> Done
>>
>>>
>>> default:
>>> break;
>>> }
>>>
>>> + /* For all possible libcalls in BFmode, return NULL. */
>>> + FOR_EACH_MODE_IN_CLASS (mode_iter, MODE_FLOAT)
>>> + {
>>> + set_conv_libfunc (trunc_optab, BFmode, mode_iter, (NULL));
>>> + set_conv_libfunc (sext_optab, mode_iter, BFmode, (NULL));
>>
>> It would probably be safer to use both argument orders for each optab,
>> since BFmode isn't necessarily the "narrowest" mode.
>>
>> Nit: no need for brackets around NULL.
> Done
>
> Let me know if this is good to go now :)
Yeah, this is OK, thanks.
Richard
>
> Thank you so much for the help getting this finalised!
>
> Cheers,
> Stam
>>
>> Thanks,
>> Richard
>>
>>> + }
>>> + arm_block_arith_comp_libfuncs_for_mode (BFmode);
>>> +
>>> /* Use names prefixed with __gnu_ for fixed-point helper functions. */
>>> {
>>> const arm_fixed_mode_set fixed_arith_modes[] =
>
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index c47fc232f39..ad1c1fa8e3b 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -2491,10 +2491,35 @@ arm_set_fixed_conv_libfunc (convert_optab optable, machine_mode to,
>
> static GTY(()) rtx speculation_barrier_libfunc;
>
> +/* Record that we have no arithmetic or comparison libfuncs for
> + machine mode MODE. */
> +
> +static void
> +arm_block_arith_comp_libfuncs_for_mode (machine_mode mode)
> +{
> + /* Arithmetic. */
> + set_optab_libfunc (add_optab, mode, NULL);
> + set_optab_libfunc (sdiv_optab, mode, NULL);
> + set_optab_libfunc (smul_optab, mode, NULL);
> + set_optab_libfunc (neg_optab, mode, NULL);
> + set_optab_libfunc (sub_optab, mode, NULL);
> +
> + /* Comparisons. */
> + set_optab_libfunc (eq_optab, mode, NULL);
> + set_optab_libfunc (ne_optab, mode, NULL);
> + set_optab_libfunc (lt_optab, mode, NULL);
> + set_optab_libfunc (le_optab, mode, NULL);
> + set_optab_libfunc (ge_optab, mode, NULL);
> + set_optab_libfunc (gt_optab, mode, NULL);
> + set_optab_libfunc (unord_optab, mode, NULL);
> +}
> +
> /* Set up library functions unique to ARM. */
> static void
> arm_init_libfuncs (void)
> {
> + machine_mode mode_iter;
> +
> /* For Linux, we have access to kernel support for atomic operations. */
> if (arm_abi == ARM_ABI_AAPCS_LINUX)
> init_sync_libfuncs (MAX_SYNC_LIBFUNC_SIZE);
> @@ -2623,27 +2648,23 @@ arm_init_libfuncs (void)
> ? "__gnu_d2h_ieee"
> : "__gnu_d2h_alternative"));
>
> - /* Arithmetic. */
> - set_optab_libfunc (add_optab, HFmode, NULL);
> - set_optab_libfunc (sdiv_optab, HFmode, NULL);
> - set_optab_libfunc (smul_optab, HFmode, NULL);
> - set_optab_libfunc (neg_optab, HFmode, NULL);
> - set_optab_libfunc (sub_optab, HFmode, NULL);
> -
> - /* Comparisons. */
> - set_optab_libfunc (eq_optab, HFmode, NULL);
> - set_optab_libfunc (ne_optab, HFmode, NULL);
> - set_optab_libfunc (lt_optab, HFmode, NULL);
> - set_optab_libfunc (le_optab, HFmode, NULL);
> - set_optab_libfunc (ge_optab, HFmode, NULL);
> - set_optab_libfunc (gt_optab, HFmode, NULL);
> - set_optab_libfunc (unord_optab, HFmode, NULL);
> + arm_block_arith_comp_libfuncs_for_mode (HFmode);
> break;
>
> default:
> break;
> }
>
> + /* For all possible libcalls in BFmode, record NULL. */
> + FOR_EACH_MODE_IN_CLASS (mode_iter, MODE_FLOAT)
> + {
> + set_conv_libfunc (trunc_optab, BFmode, mode_iter, NULL);
> + set_conv_libfunc (trunc_optab, mode_iter, BFmode, NULL);
> + set_conv_libfunc (sext_optab, mode_iter, BFmode, NULL);
> + set_conv_libfunc (sext_optab, BFmode, mode_iter, NULL);
> + }
> + arm_block_arith_comp_libfuncs_for_mode (BFmode);
> +
> /* Use names prefixed with __gnu_ for fixed-point helper functions. */
> {
> const arm_fixed_mode_set fixed_arith_modes[] =
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-02-05 16:37 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-29 12:39 [GCC][BUG][Aarch64][ARM] (PR93300) Fix ICE due to BFmode placement in GET_MODES_WIDER chain Stam Markianos-Wright
2020-01-29 12:58 ` Richard Sandiford
2020-01-29 18:01 ` Stam Markianos-Wright
2020-01-30 11:49 ` Richard Sandiford
2020-01-30 17:54 ` Stam Markianos-Wright
2020-01-31 14:13 ` Richard Sandiford
2020-02-04 11:30 ` Stam Markianos-Wright
2020-02-04 12:02 ` Richard Sandiford
2020-02-04 16:33 ` Stam Markianos-Wright
2020-02-05 16:37 ` Richard Sandiford
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).