* [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).