diff mbox

[LTO,ARM] Fix vector TYPE_MODE in streaming-out

Message ID 56727A0F.9050103@st.com
State New
Headers show

Commit Message

Christian Bruel Dec. 17, 2015, 9:02 a.m. UTC 
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
diff mbox

Patch

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.  */