This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][LTO,ARM] Fix vector TYPE_MODE in streaming-out
- From: Christian Bruel <christian dot bruel at st dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>, Bernd Schmidt <bschmidt at redhat dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 17 Dec 2015 10:02:07 +0100
- Subject: Re: [PATCH][LTO,ARM] Fix vector TYPE_MODE in streaming-out
- Authentication-results: sourceware.org; auth=none
- References: <56702D44 dot 1010103 at st dot com> <56703C54 dot 5090309 at redhat dot com> <CAFiYyc3LAHC0+LXhZgrc2Ge=uJCid5wt-m9bnjck5PoHyUKu1A at mail dot gmail dot com>
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
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. */