From: Thomas Schwinge <thomas@codesourcery.com>
To: "Li, Pan2" <pan2.li@intel.com>,
Richard Biener <richard.guenther@gmail.com>,
Jakub Jelinek <jakub@redhat.com>
Cc: <gcc-patches@gcc.gnu.org>, <richard.sandiford@arm.com>,
<kito.cheng@gmail.com>, Jeff Law <jeffreyalaw@gmail.com>,
<juzhe.zhong@rivai.ai>,
"Wang, Yanzhang" <yanzhang.wang@intel.com>
Subject: RE: Machine Mode ICE in RISC-V when LTO
Date: Thu, 10 Aug 2023 15:23:37 +0200 [thread overview]
Message-ID: <878rajuj3a.fsf@dem-tschwing-1.ger.mentorg.com> (raw)
In-Reply-To: <MW5PR11MB5908E5743F472D48ACF60039A913A@MW5PR11MB5908.namprd11.prod.outlook.com>
Hi!
On 2023-08-10T12:25:36+0000, "Li, Pan2" <pan2.li@intel.com> wrote:
> Thanks Richard for comment, let me try to promote the table to unsigned short.
I have WIP work for this issue -- which I'd already raised a month ago:
<https://inbox.sourceware.org/87o7kxuq9s.fsf@euler.schwinge.homeip.net>:
On 2023-06-30T13:46:07+0200, Thomas Schwinge <thomas@codesourcery.com> wrote:
> In particular, the 'lto_mode_identity_table' changes would seem necessary
> to keep standard LTO ('-flto') functional for large 'machine_mode' size?
... which is exactly the problem you've now run into?
However, a simple:
-GTY(()) const unsigned char *lto_mode_identity_table;
+GTY(()) const unsigned short *lto_mode_identity_table;
..., or:
-GTY(()) const unsigned char *lto_mode_identity_table;
+GTY(()) const machine_mode *lto_mode_identity_table;
... is not sufficient: that runs into GTY issues, as the current
'unsigned char *lto_mode_identity_table' is (mis-)classified by
'gengtype' as a C string. This happens to work for this case, but still
isn't right, and only works for 'char *' but not 'short *' etc. I have
WIP work to tighten that. ..., which got me into other GTY issues, and
so on... ;-) (Richard already ACKed and I pushed some of the
prerequisite changes, but there's more to come.) I'm still planning on
resolving all that mess, but I'm tight on time right now.
However, I have a different proposal, which should address your current
issue: simply, get rid of the 'lto_mode_identity_table', which is just
that: a 1-to-1 mapping of array index to value. Instead, in
'gcc/lto/lto-common.cc:lto_file_finalize', for '!ACCEL_COMPILER', set
'file_data->mode_table = NULL', and in the users (only
'gcc/tree-streamer.h:bp_unpack_machine_mode'?), replace (untested):
-return (machine_mode) ib->file_data->mode_table[ix];
+return ib->file_data->mode_table ? ib->file_data->mode_table[ix] : ix;
Jakub, as the original author of 'lto_mode_identity_table' (see
commit db847fa8f2cca6139188b8dfa0a7064319b19193 (Subversion r221005)), is
there any reason not to do it this way?
Grüße
Thomas
> -----Original Message-----
> From: Richard Biener <richard.guenther@gmail.com>
> Sent: Thursday, August 10, 2023 7:08 PM
> To: Li, Pan2 <pan2.li@intel.com>
> Cc: richard.sandiford@arm.com; Thomas Schwinge <thomas@codesourcery.com>; jakub@redhat.com; kito.cheng@gmail.com; Jeff Law <jeffreyalaw@gmail.com>; juzhe.zhong@rivai.ai; Wang, Yanzhang <yanzhang.wang@intel.com>
> Subject: Re: Machine Mode ICE in RISC-V when LTO
>
> On Thu, Aug 10, 2023 at 10:19 AM Li, Pan2 <pan2.li@intel.com> wrote:
>>
>> Hi all,
>>
>>
>>
>> Recently I found there is still some issues for the machine mode with LTO part by fixing one
>>
>> ICE (only when compile with LTO) in RISC-V backend in , aka below case.
>>
>>
>>
>> >> ../__RISC-V_INSTALL___/bin/riscv64-unknown-elf-g++ -O2 -flto gcc/testsuite/g++.dg/torture/vshuf-v4df.C -o test.elf
>>
>> during RTL pass: expand
>>
>> gcc/testsuite/g++.dg/torture/vshuf-main.inc: In function 'main':
>>
>> gcc/testsuite/g++.dg/torture/vshuf-main.inc:15:9: internal compiler error: in as_a, at machmode.h:381
>>
>> 15 | V r = __builtin_shuffle(in1[i], mask1[i]);
>>
>> | ^
>>
>> 0x7e5b8e scalar_int_mode as_a<scalar_int_mode>(machine_mode)
>>
>> ../.././gcc/gcc/machmode.h:381
>>
>> 0x7eabdb scalar_mode as_a<scalar_mode>(machine_mode)
>>
>> ../.././gcc/gcc/expr.cc:332
>>
>> 0x7eabdb convert_mode_scalar
>>
>> ../.././gcc/gcc/expr.cc:325
>>
>> 0xb8485b store_expr(tree_node*, rtx_def*, int, bool, bool)
>>
>> ../.././gcc/gcc/expr.cc:6413
>>
>> 0xb8a556 store_field
>>
>> ../.././gcc/gcc/expr.cc:7648
>>
>> 0xb88f27 store_constructor(tree_node*, rtx_def*, int, poly_int<2u, long>, bool)
>>
>> ../.././gcc/gcc/expr.cc:7588
>>
>> 0xb8b8b8 expand_constructor
>>
>> ../.././gcc/gcc/expr.cc:8931
>>
>> 0xb76bc7 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)
>>
>> ../.././gcc/gcc/expr.cc:11170
>>
>> 0xb77ef7 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)
>>
>> ../.././gcc/gcc/expr.cc:10809
>>
>> 0xb83a80 store_expr(tree_node*, rtx_def*, int, bool, bool)
>>
>> ../.././gcc/gcc/expr.cc:6325
>>
>> 0xb851d9 expand_assignment(tree_node*, tree_node*, bool)
>>
>> ../.././gcc/gcc/expr.cc:6043
>>
>> 0xa48717 expand_gimple_stmt_1
>>
>> ../.././gcc/gcc/cfgexpand.cc:3946
>>
>> 0xa48717 expand_gimple_stmt
>>
>> ../.././gcc/gcc/cfgexpand.cc:4044
>>
>> 0xa4d030 expand_gimple_basic_block
>>
>> ../.././gcc/gcc/cfgexpand.cc:6096
>>
>> 0xa4efd6 execute
>>
>> ../.././gcc/gcc/cfgexpand.cc:6831
>>
>>
>>
>> I double checked the reason that comes from we add even more machine modes in the RISC-V backend,
>>
>> and then did some investigation for the root cause. It should be related to the mode_table, as well as the
>>
>> bp_unpack_machine_mode.
>>
>>
>>
>> In lto_fe_init:
>>
>> unsigned char *table
>>
>> = ggc_vec_alloc<unsigned char> (MAX_MACHINE_MODE);
>>
>>
>>
>> for (int m = 0; m < MAX_MACHINE_MODE; m++)
>>
>> table[m] = m; <== May overflow here given MAX_MACHINE_MODE > 256 and table[m] is unsigned char.
>>
>>
>>
>> in bp_unpack_machine_mode:
>>
>> unsigned ix = bp_unpack_enum (bp, machine_mode, last);
>>
>> return (machine_mode) ib->file_data->mode_table[ix]; <== May return truncated mode here.
>>
>>
>>
>> To validate this idea, I tried below hack code for double checking and then there is no ICE anymore, which indicates
>>
>> the problem here as I bet. However, the lto is quite complicated and I am not sure how to fix it in the right way.
>>
>>
>>
>> + = ggc_vec_alloc<unsigned char> (MAX_MACHINE_MODE * 2);
>>
>> …
>>
>> + ((unsigned short *)table)[m] = m;
>>
>> …
>>
>> + return (machine_mode) ((unsigned short *)ib->file_data->mode_table)[ix];
>>
>>
>>
>> Besides, I also tried to change the mode_table from char * to short * but got one weird error when building as below.
>>
>>
>>
>> gcc/lto-streamer.h:599: field `(*x).mode_table' is pointer to unimplemented type
>
> We still have some places using an array of char for the mode table.
> The above is assigned
> to lto_mode_identity_table which ends up in
> lto_file_decl_data::mode_table. I think those
> need to be all promoted to unsigned short.
>
> Richard.
>
>>
>>
>> Pan
>>
>>
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
next prev parent reply other threads:[~2023-08-10 13:23 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-12 5:00 [PATCH] Machine_Mode: Extend machine_mode from 8 to 16 bits pan2.li
2023-05-12 6:49 ` Richard Biener
2023-05-12 19:14 ` Bernhard Reutner-Fischer
2023-05-13 8:44 ` Kito Cheng
2023-05-13 12:26 ` Li, Pan2
2023-06-30 11:46 ` 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) Thomas Schwinge
2023-06-30 12:45 ` Kito Cheng
2023-06-30 16:11 ` Thomas Schwinge
2023-06-30 16:37 ` Jakub Jelinek
2023-07-04 15:45 ` Thomas Schwinge
[not found] ` <MW5PR11MB590876BB7E52B78E837C95C9A913A@MW5PR11MB5908.namprd11.prod.outlook.com>
[not found] ` <CAFiYyc0Ajoi__g1YJhdrh-Z3DsOyU7+1iG6SEmZMDzAShX-L6g@mail.gmail.com>
[not found] ` <MW5PR11MB5908E5743F472D48ACF60039A913A@MW5PR11MB5908.namprd11.prod.outlook.com>
2023-08-10 13:23 ` Thomas Schwinge [this message]
2023-08-10 14:14 ` Machine Mode ICE in RISC-V when LTO Li, Pan2
2023-08-11 6:40 ` Li, Pan2
2023-09-15 13:33 ` Robin Dapp
2023-09-18 14:46 ` LTO: Get rid of 'lto_mode_identity_table' (was: Machine Mode ICE in RISC-V when LTO) Thomas Schwinge
2023-09-18 14:53 ` Richard Biener
2023-05-12 8:24 ` [PATCH] Machine_Mode: Extend machine_mode from 8 to 16 bits Richard Sandiford
2023-05-12 11:16 ` Li, Pan2
2023-05-12 11:31 ` Richard Sandiford
2023-05-12 11:48 ` Li, Pan2
2023-05-16 15:35 ` pan2.li
2023-05-18 8:57 ` Richard Sandiford
2023-05-18 9:17 ` Li, Pan2
[not found] <Message-Id: <20230512050016.476110-1-pan2.li@intel.com>
2023-05-12 15:38 ` [PATCH v2] " pan2.li
2023-05-13 13:13 ` [PATCH v3] " pan2.li
2023-05-16 1:12 ` Li, Pan2
2023-05-16 7:29 ` Richard Sandiford
2023-05-16 7:55 ` Li, Pan2
2023-05-16 8:03 ` Xi Ruoyao
2023-05-16 8:05 ` Li, Pan2
2023-05-16 9:09 ` Richard Sandiford
2023-05-16 12:17 ` Li, Pan2
2023-05-16 15:39 ` Li, Pan2
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=878rajuj3a.fsf@dem-tschwing-1.ger.mentorg.com \
--to=thomas@codesourcery.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=jeffreyalaw@gmail.com \
--cc=juzhe.zhong@rivai.ai \
--cc=kito.cheng@gmail.com \
--cc=pan2.li@intel.com \
--cc=richard.guenther@gmail.com \
--cc=richard.sandiford@arm.com \
--cc=yanzhang.wang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).