From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa3.mentor.iphmx.com (esa3.mentor.iphmx.com [68.232.137.180]) by sourceware.org (Postfix) with ESMTPS id 868D93860758 for ; Fri, 30 Jun 2023 11:46:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 868D93860758 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com X-IronPort-AV: E=Sophos;i="6.01,170,1684828800"; d="scan'208,223";a="10380792" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa3.mentor.iphmx.com with ESMTP; 30 Jun 2023 03:46:16 -0800 IronPort-SDR: nKrW6S1JQTlejcmFe+eNltcTYb+bDL7tw8rUgrsmNkLWTwmh1KNCoBjpQiYRSexksdOHa3/0jf 24OM8R6DbWXHcMG9410K4rNtk+rc/xy8ge8Id4/3kJiaDPyy+6G5Mw8PlimBi91uykTJr7P/0Y 8sMgDnPUaz2LvJ4X6PklJeugzdeGISZ8aUALsIF0vo/TQwvOVHwqyK24kgB1Ag+j+PghSFYYPm UKx9GvsCkYxRpAuJfMXRt+IUMU3zvozSeJXn0P0u87sQQQGTU/76J1TKya+ihbbv4CwzDZ9KKn fAg= From: Thomas Schwinge To: Richard Biener , , , Bernhard Reutner-Fischer , "Jakub Jelinek" CC: Kito Cheng , , , , Subject: Adjust LTO mode tables for "Machine_Mode: Extend machine_mode from 8 to 16 bits" (was: [PATCH] Machine_Mode: Extend machine_mode from 8 to 16 bits) In-Reply-To: References: <20230512050016.476110-1-pan2.li@intel.com> <2CEAD79B-D664-41B4-A337-5E77ECFB2F9D@gmail.com> User-Agent: Notmuch/0.29.3+94~g74c3f1b (https://notmuchmail.org) Emacs/28.2 (x86_64-pc-linux-gnu) Date: Fri, 30 Jun 2023 13:46:07 +0200 Message-ID: <87o7kxuq9s.fsf@euler.schwinge.homeip.net> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-15.mgc.mentorg.com (139.181.222.15) To svr-ies-mbx-10.mgc.mentorg.com (139.181.222.10) X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,SPF_HELO_PASS,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: --=-=-= Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi! On 2023-05-13T16:44:41+0800, Kito Cheng via Gcc-patches wrote: > Tried this patch and I ran into some issues, some variables are using > unsigned char to hold machine mode and will have problems when the > number of modes is larger than 255... > > And here is the fix: > --- a/gcc/genmodes.cc > +++ b/gcc/genmodes.cc > @@ -1141,10 +1141,10 @@ inline __attribute__((__always_inline__))\n\ > #else\n\ > extern __inline__ __attribute__((__always_inline__, __gnu_inline__))\n\ > #endif\n\ > -unsigned char\n\ > +unsigned short\n\ > mode_inner_inline (machine_mode mode)\n\ > {\n\ > - extern const unsigned char mode_inner[NUM_MACHINE_MODES];\n\ > + extern const unsigned short mode_inner[NUM_MACHINE_MODES];\n\ > gcc_assert (mode >=3D 0 && mode < NUM_MACHINE_MODES);\n\ > switch (mode)\n\ > {"); > @@ -1529,7 +1529,7 @@ emit_mode_wider (void) > int c; > struct mode_data *m; > > - print_decl ("unsigned char", "mode_next", "NUM_MACHINE_MODES"); > + print_decl ("unsigned short", "mode_next", "NUM_MACHINE_MODES"); Etc. Instead of 's%char%short', shouldn't we really be using 'enum machine_mode' here? (I understand such a change may require some further surgery, but wouldn't it be the correct thing to do?) And, in context of working on "Streamer: Fix out of range memory access of machine mode", I found another one, see attached '[WIP] Adjust LTO mode tables for "Machine_Mode: Extend machine_mode from 8= to 16 bits"' (..., which applies on top of the former.) There, in fact, I did change to 'enum machine_mode' instead of 's%char%short' -- correct? Any comments on the 'GTY' issues: (1) 'const' build error, (2) '[build-gcc]/gcc/gtype-desc.cc' changes, and (3) is 'GTY((atomic))' actually the right thing to use, here? In particular, the 'lto_mode_identity_table' changes would seem necessary to keep standard LTO ('-flto') functional for large 'machine_mode' size? Bernhard: Fancy writing a Coccinelle check whether there are any more places where we put what originally was a 'machine_mode' type into a 'char' (or, into a non-'machine_mode' generally)? ;-) Hey, just a Friday afternoon idea! Gr=C3=BC=C3=9Fe Thomas ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstra=C3=9Fe 201= , 80634 M=C3=BCnchen; Gesellschaft mit beschr=C3=A4nkter Haftung; Gesch=C3= =A4ftsf=C3=BChrer: Thomas Heurung, Frank Th=C3=BCrauf; Sitz der Gesellschaf= t: M=C3=BCnchen; Registergericht M=C3=BCnchen, HRB 106955 --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename="0001-WIP-Adjust-LTO-mode-tables-for-Machine_Mode-Extend-m.patch" >From 0fd8f65bb87b11ef8ae366a797aec572d67b284f Mon Sep 17 00:00:00 2001 From: Thomas Schwinge Date: Fri, 30 Jun 2023 13:23:55 +0200 Subject: [PATCH] [WIP] Adjust LTO mode tables for "Machine_Mode: Extend machine_mode from 8 to 16 bits" --- gcc/lto-streamer-in.cc | 2 +- gcc/lto-streamer.h | 56 +++++++++++++++++++++++++++++++++++++++++- gcc/lto/lto-common.cc | 10 ++++---- gcc/lto/lto-common.h | 2 +- gcc/tree-streamer.h | 2 +- 5 files changed, 63 insertions(+), 9 deletions(-) diff --git a/gcc/lto-streamer-in.cc b/gcc/lto-streamer-in.cc index 1876e1967ec..bbd44504ff8 100644 --- a/gcc/lto-streamer-in.cc +++ b/gcc/lto-streamer-in.cc @@ -1997,7 +1997,7 @@ lto_input_mode_table (struct lto_file_decl_data *file_data) bitpack_d bp = streamer_read_bitpack (&ib); unsigned mode_bits = bp_unpack_value (&bp, 5); - unsigned char *table = ggc_cleared_vec_alloc (1 << mode_bits); + machine_mode *table = ggc_cleared_vec_alloc (1 << mode_bits); file_data->mode_table = table; file_data->mode_bits = mode_bits; diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h index 0556b34c837..4d83741e4c6 100644 --- a/gcc/lto-streamer.h +++ b/gcc/lto-streamer.h @@ -596,7 +596,61 @@ struct GTY(()) lto_file_decl_data hash_map * GTY((skip)) resolution_map; /* Mode translation table. */ - const unsigned char *mode_table; + /*TODO const +With 'const', we get: + + gtype-desc.cc: In function 'void gt_pch_nx_lto_file_decl_data(void*)': + gtype-desc.cc:6531:34: error: invalid conversion from 'const void*' to 'void*' [-fpermissive] + gt_pch_note_object ((*x).mode_table, x, gt_pch_p_18lto_file_decl_data); + ^ + In file included from [...]/source-gcc/gcc/hash-table.h:247:0, + from [...]/source-gcc/gcc/coretypes.h:486, + from gtype-desc.cc:23: + [...]/source-gcc/gcc/ggc.h:47:12: note: initializing argument 1 of 'int gt_pch_note_object(void*, void*, gt_note_pointers, size_t)' + extern int gt_pch_note_object (void *, void *, gt_note_pointers, + ^ + make[2]: *** [Makefile:1180: gtype-desc.o] Error 1 + */ + machine_mode * GTY((atomic)) mode_table; + /* +This (without 'const') changes '[build-gcc]/gcc/gtype-desc.cc' as follows: + + @@ -2566,7 +2566,9 @@ gt_ggc_mx_lto_file_decl_data (void *x_p) + gt_ggc_m_17lto_in_decl_state ((*x).global_decl_state); + gt_ggc_m_29hash_table_decl_state_hasher_ ((*x).function_decl_states); + gt_ggc_m_18lto_file_decl_data ((*x).next); + - gt_ggc_m_S ((*x).mode_table); + + if ((*x).mode_table != NULL) { + + ggc_mark ((*x).mode_table); + + } + } + } + + @@ -6525,7 +6527,9 @@ gt_pch_nx_lto_file_decl_data (void *x_p) + gt_pch_n_17lto_in_decl_state ((*x).global_decl_state); + gt_pch_n_29hash_table_decl_state_hasher_ ((*x).function_decl_states); + gt_pch_n_18lto_file_decl_data ((*x).next); + - gt_pch_n_S ((*x).mode_table); + + if ((*x).mode_table != NULL) { + + gt_pch_note_object ((*x).mode_table, x, gt_pch_p_18lto_file_decl_data); + + } + } + } + + @@ -10929,8 +10933,10 @@ gt_pch_p_18lto_file_decl_data (ATTRIBUTE + op (&((*x).function_decl_states), NULL, cookie); + if ((void *)(x) == this_obj) + op (&((*x).next), NULL, cookie); + - if ((void *)(x) == this_obj) + - op (&((*x).mode_table), NULL, cookie); + + if ((*x).mode_table != NULL) { + + if ((void *)(x) == this_obj) + + op (&((*x).mode_table), NULL, cookie); + + } + } + +Given that the '[...]_S' routines are for strings (due to previous 'char *', I suppose), that's probably correct? + */ /* Read LTO section. */ lto_section lto_section_header; diff --git a/gcc/lto/lto-common.cc b/gcc/lto/lto-common.cc index 973ab791712..f483f42997e 100644 --- a/gcc/lto/lto-common.cc +++ b/gcc/lto/lto-common.cc @@ -64,7 +64,7 @@ static bool type_streaming_finished = false; GTY(()) tree first_personality_decl; -GTY(()) const unsigned char *lto_mode_identity_table; +GTY(()) const machine_mode *lto_mode_identity_table; /* Returns a hash code for P. */ @@ -2274,7 +2274,7 @@ lto_file_finalize (struct lto_file_decl_data *file_data, lto_file *file, #ifdef ACCEL_COMPILER lto_input_mode_table (file_data); #else - file_data->mode_table = lto_mode_identity_table; + file_data->mode_table = /*TODO const*/ (machine_mode *) lto_mode_identity_table; file_data->mode_bits = ceil_log2 (MAX_MACHINE_MODE); #endif @@ -3116,10 +3116,10 @@ lto_fe_init (void) bitmap_obstack_initialize (NULL); gimple_register_cfg_hooks (); #ifndef ACCEL_COMPILER - unsigned char *table - = ggc_vec_alloc (MAX_MACHINE_MODE); + machine_mode *table + = ggc_vec_alloc (MAX_MACHINE_MODE); for (int m = 0; m < MAX_MACHINE_MODE; m++) - table[m] = m; + table[m] = (machine_mode) m; lto_mode_identity_table = table; #endif } diff --git a/gcc/lto/lto-common.h b/gcc/lto/lto-common.h index 24b2445673b..2b868085139 100644 --- a/gcc/lto/lto-common.h +++ b/gcc/lto/lto-common.h @@ -26,7 +26,7 @@ void print_lto_report_1 (void); extern tree lto_eh_personality_decl; extern GTY(()) vec *tree_with_vars; -extern const unsigned char *lto_mode_identity_table; +extern const machine_mode *lto_mode_identity_table; extern tree first_personality_decl; #endif diff --git a/gcc/tree-streamer.h b/gcc/tree-streamer.h index ff49d1ba637..1e346b775e6 100644 --- a/gcc/tree-streamer.h +++ b/gcc/tree-streamer.h @@ -118,7 +118,7 @@ bp_unpack_machine_mode (struct bitpack_d *bp) lto_input_block *ib = (class lto_input_block *) bp->stream; int last = 1 << ib->file_data->mode_bits; unsigned ix = bp_unpack_enum (bp, machine_mode, last); - return (machine_mode) ib->file_data->mode_table[ix]; + return ib->file_data->mode_table[ix]; } #endif /* GCC_TREE_STREAMER_H */ -- 2.34.1 --=-=-=--