* [PATCH][LTO,ARM] Fix vector TYPE_MODE in streaming-out
@ 2015-12-15 15:10 Christian Bruel
2015-12-15 16:14 ` Bernd Schmidt
0 siblings, 1 reply; 4+ messages in thread
From: Christian Bruel @ 2015-12-15 15:10 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1540 bytes --]
Hello,
This patch fixes a few ICEs that I'm having while enabling the ARM/NEON
builtins for LTO (in a parallel development line, so the testcase is
cannot work in isolation but should illustrate the problem)
For instance, the global declarations in the following code compiled
with -mfpu=neon
----------------------------------
__simd64_int8_t a;
__simd128_int16_t e;
int
main()
{
e = __builtin_neon_vaddlsv8qi (a, a);
return 0;
}
----------------------------------
in "normal" mode, the TYPE_MODE for vector_type __simd64_int8_t is set
to V8QImode by arm_vector_mode_supported_p during the builtins type
initializations, thanks to TARGET_NEON set bu the global flag.
Now, in LTO mode the streamer writes the information for this
vector_type as a scalar DImode, causing ICEs during arm_expand_builtin.
The root cause of this is that the streamer-out uses TYPE_MODE in a
context where the target_flags are not known return false for TARGET_NEON.
The streamer-in then will then read the wrong mode that propagates to
the back-end.
Using the value computed by the middle end (when the current target was
known) in type_common.mode instead of calling vector_type_mode during
the LTO streaming fixes this issue.
Does that seem all-right to skip vector_type_mode dynamic machinery in
the lto streamer since the streamer should not change modes used by the
middle-end (I assume) ?
comments ?
many thanks
Christian
(for ref: this is a follow-up of
https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01094.html)
[-- Attachment #2: lto-attrvec.patch --]
[-- Type: text/x-patch, Size: 558 bytes --]
Index: tree-streamer-out.c
===================================================================
--- tree-streamer-out.c (revision 231644)
+++ tree-streamer-out.c (working copy)
@@ -308,7 +308,7 @@ pack_ts_function_decl_value_fields (stru
static void
pack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
{
- bp_pack_machine_mode (bp, TYPE_MODE (expr));
+ bp_pack_machine_mode (bp, expr->type_common.mode);
bp_pack_value (bp, TYPE_STRING_FLAG (expr), 1);
/* TYPE_NO_FORCE_BLK is private to stor-layout and need
no streaming. */
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][LTO,ARM] Fix vector TYPE_MODE in streaming-out
2015-12-15 15:10 [PATCH][LTO,ARM] Fix vector TYPE_MODE in streaming-out Christian Bruel
@ 2015-12-15 16:14 ` Bernd Schmidt
2015-12-16 9:48 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Bernd Schmidt @ 2015-12-15 16:14 UTC (permalink / raw)
To: Christian Bruel, gcc-patches
On 12/15/2015 04:09 PM, Christian Bruel wrote:
> in "normal" mode, the TYPE_MODE for vector_type __simd64_int8_t is set
> to V8QImode by arm_vector_mode_supported_p during the builtins type
> initializations, thanks to TARGET_NEON set bu the global flag.
>
> Now, in LTO mode the streamer writes the information for this
> vector_type as a scalar DImode, causing ICEs during arm_expand_builtin.
> The root cause of this is that the streamer-out uses TYPE_MODE in a
> context where the target_flags are not known return false for TARGET_NEON.
>
> The streamer-in then will then read the wrong mode that propagates to
> the back-end.
> static void
> pack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
> {
> - bp_pack_machine_mode (bp, TYPE_MODE (expr));
> + bp_pack_machine_mode (bp, expr->type_common.mode);
This looks sensible given that tree-streamer-in uses SET_TYPE_MODE,
which just writes expr->type_common.mode.
Make a new macro TYPE_MODE_RAW for this and I think the patch is ok
(although there's precedent for direct access in vector_type_mode, but I
think that's just bad).
Bernd
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][LTO,ARM] Fix vector TYPE_MODE in streaming-out
2015-12-15 16:14 ` Bernd Schmidt
@ 2015-12-16 9:48 ` Richard Biener
2015-12-17 9:02 ` Christian Bruel
0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2015-12-16 9:48 UTC (permalink / raw)
To: Bernd Schmidt; +Cc: Christian Bruel, GCC Patches
On Tue, Dec 15, 2015 at 5:14 PM, Bernd Schmidt <bschmidt@redhat.com> wrote:
> On 12/15/2015 04:09 PM, Christian Bruel wrote:
>>
>> in "normal" mode, the TYPE_MODE for vector_type __simd64_int8_t is set
>> to V8QImode by arm_vector_mode_supported_p during the builtins type
>> initializations, thanks to TARGET_NEON set bu the global flag.
>>
>> Now, in LTO mode the streamer writes the information for this
>> vector_type as a scalar DImode, causing ICEs during arm_expand_builtin.
>> The root cause of this is that the streamer-out uses TYPE_MODE in a
>> context where the target_flags are not known return false for TARGET_NEON.
>>
>> The streamer-in then will then read the wrong mode that propagates to
>> the back-end.
>
>
>> static void
>> pack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
>> {
>> - bp_pack_machine_mode (bp, TYPE_MODE (expr));
>> + bp_pack_machine_mode (bp, expr->type_common.mode);
>
>
> This looks sensible given that tree-streamer-in uses SET_TYPE_MODE, which
> just writes expr->type_common.mode.
>
> Make a new macro TYPE_MODE_RAW for this and I think the patch is ok
> (although there's precedent for direct access in vector_type_mode, but I
> think that's just bad).
Yeah, though it's well-hidden ;)
I think the patch is ok if you add a comment why we're not using TYPE_MODE here
and if the patch passes the x86_64 vectorizer testsuite with -m32 -march=i586
with no regressions (I do expect some FAILs with -march=i586 but the
patch shouldn't
regress anything).
Thanks,
Richard.
>
> Bernd
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][LTO,ARM] Fix vector TYPE_MODE in streaming-out
2015-12-16 9:48 ` Richard Biener
@ 2015-12-17 9:02 ` Christian Bruel
0 siblings, 0 replies; 4+ messages in thread
From: Christian Bruel @ 2015-12-17 9:02 UTC (permalink / raw)
To: Richard Biener, Bernd Schmidt; +Cc: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 1778 bytes --]
On 12/16/2015 10:48 AM, Richard Biener wrote:
> On Tue, Dec 15, 2015 at 5:14 PM, Bernd Schmidt <bschmidt@redhat.com> wrote:
>> On 12/15/2015 04:09 PM, Christian Bruel wrote:
>>>
>>> in "normal" mode, the TYPE_MODE for vector_type __simd64_int8_t is set
>>> to V8QImode by arm_vector_mode_supported_p during the builtins type
>>> initializations, thanks to TARGET_NEON set bu the global flag.
>>>
>>> Now, in LTO mode the streamer writes the information for this
>>> vector_type as a scalar DImode, causing ICEs during arm_expand_builtin.
>>> The root cause of this is that the streamer-out uses TYPE_MODE in a
>>> context where the target_flags are not known return false for TARGET_NEON.
>>>
>>> The streamer-in then will then read the wrong mode that propagates to
>>> the back-end.
>>
>>
>>> static void
>>> pack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
>>> {
>>> - bp_pack_machine_mode (bp, TYPE_MODE (expr));
>>> + bp_pack_machine_mode (bp, expr->type_common.mode);
>>
>>
>> This looks sensible given that tree-streamer-in uses SET_TYPE_MODE, which
>> just writes expr->type_common.mode.
>>
>> Make a new macro TYPE_MODE_RAW for this and I think the patch is ok
>> (although there's precedent for direct access in vector_type_mode, but I
>> think that's just bad).
>
> Yeah, though it's well-hidden ;)
>
> I think the patch is ok if you add a comment why we're not using TYPE_MODE here
> and if the patch passes the x86_64 vectorizer testsuite with -m32 -march=i586
> with no regressions (I do expect some FAILs with -march=i586 but the
> patch shouldn't
> regress anything).
Thanks,
no regressions for unix\{-m32/-march=i586,\}, arm and aarch64.
I'm about to commit the attached patch.
Christian
>
> Thanks,
> Richard.
>
>
>>
>> Bernd
[-- Attachment #2: type-lto.patch --]
[-- Type: text/x-patch, Size: 1466 bytes --]
2015-12-16 Christian Bruel <christian.bruel@st.com>
* tree.h (TYPE_MODE_RAW): New macro.
* tree-streamer-out.c (pack_ts_type_common_value_fields):
Replace TYPE_MODE by TYPE_MODE_RAW.
Index: tree.h
===================================================================
--- tree.h (revision 231744)
+++ tree.h (working copy)
@@ -1793,6 +1793,7 @@ extern void protected_set_expr_location
#define TYPE_MAIN_VARIANT(NODE) (TYPE_CHECK (NODE)->type_common.main_variant)
#define TYPE_CONTEXT(NODE) (TYPE_CHECK (NODE)->type_common.context)
+#define TYPE_MODE_RAW(NODE) (TYPE_CHECK (NODE)->type_common.mode)
#define TYPE_MODE(NODE) \
(VECTOR_TYPE_P (TYPE_CHECK (NODE)) \
? vector_type_mode (NODE) : (NODE)->type_common.mode)
Index: tree-streamer-out.c
===================================================================
--- tree-streamer-out.c (revision 231744)
+++ tree-streamer-out.c (working copy)
@@ -308,7 +308,10 @@ pack_ts_function_decl_value_fields (stru
static void
pack_ts_type_common_value_fields (struct bitpack_d *bp, tree expr)
{
- bp_pack_machine_mode (bp, TYPE_MODE (expr));
+ /* for VECTOR_TYPE, TYPE_MODE reevaluates the mode using target_flags
+ not necessary valid in a global context.
+ Use the raw value previously set by layout_type. */
+ bp_pack_machine_mode (bp, TYPE_MODE_RAW (expr));
bp_pack_value (bp, TYPE_STRING_FLAG (expr), 1);
/* TYPE_NO_FORCE_BLK is private to stor-layout and need
no streaming. */
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-12-17 9:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-15 15:10 [PATCH][LTO,ARM] Fix vector TYPE_MODE in streaming-out Christian Bruel
2015-12-15 16:14 ` Bernd Schmidt
2015-12-16 9:48 ` Richard Biener
2015-12-17 9:02 ` Christian Bruel
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).