Message ID | 563C8A38.10209@t-online.de |
---|---|
State | New |
Headers | show |
On 06/11/15 11:08, Bernd Schmidt wrote: > This one is a fix for something that could currently only affect c6x, but I have code that exposes it on i386. > > When optionally gathering operand info in regrename, we can overflow the array in certain situations. This can occur when we have a situation where a value is constructed in multiple small registers and then accessed as a larger one (CDImode in the testcase I have). In that case we enter the "superset" path, which fails the involved chains, but the smaller pieces still all get seen by record_operand_use, and there may be more of them than MAX_REGS_PER_ADDRESS. > > The following fixes it. Bootstrapped and tested with -frename-registers enabled at -O1 on x86_64-linux. Ok? > > > Bernd This sounds like it will fix http://gcc.gnu.org/PR66785 ... Ramana
On 11/06/2015 12:17 PM, Ramana Radhakrishnan wrote: > On 06/11/15 11:08, Bernd Schmidt wrote: >> This one is a fix for something that could currently only affect c6x, but I have code that exposes it on i386. >> >> When optionally gathering operand info in regrename, we can overflow the array in certain situations. This can occur when we have a situation where a value is constructed in multiple small registers and then accessed as a larger one (CDImode in the testcase I have). In that case we enter the "superset" path, which fails the involved chains, but the smaller pieces still all get seen by record_operand_use, and there may be more of them than MAX_REGS_PER_ADDRESS. >> >> The following fixes it. Bootstrapped and tested with -frename-registers enabled at -O1 on x86_64-linux. Ok? >> >> >> Bernd > > This sounds like it will fix http://gcc.gnu.org/PR66785 ... Ah, I didn't realize something else was using this functionality: gcc/config/aarch64/cortex-a57-fma-steering.c 1025: regrename_init (true); Yeah, the description of that bug makes it sound like the same issue. Bernd
On 06/11/15 11:31, Bernd Schmidt wrote: > On 11/06/2015 12:17 PM, Ramana Radhakrishnan wrote: >> On 06/11/15 11:08, Bernd Schmidt wrote: >>> This one is a fix for something that could currently only affect c6x, but I have code that exposes it on i386. >>> >>> When optionally gathering operand info in regrename, we can overflow the array in certain situations. This can occur when we have a situation where a value is constructed in multiple small registers and then accessed as a larger one (CDImode in the testcase I have). In that case we enter the "superset" path, which fails the involved chains, but the smaller pieces still all get seen by record_operand_use, and there may be more of them than MAX_REGS_PER_ADDRESS. >>> >>> The following fixes it. Bootstrapped and tested with -frename-registers enabled at -O1 on x86_64-linux. Ok? >>> >>> >>> Bernd >> >> This sounds like it will fix http://gcc.gnu.org/PR66785 ... > > Ah, I didn't realize something else was using this functionality: > > gcc/config/aarch64/cortex-a57-fma-steering.c > 1025: regrename_init (true); > > Yeah, the description of that bug makes it sound like the same issue. Yeah looks like the ICE goes away with a quick spin - I've not done any deeper analysis but that looks like a fix. I'll take the opportunity to point out gcc11{3-6} if you need an aarch64 machine on the compile farm if you wanted access to one. regards Ramana > > > Bernd
On 11/06/2015 04:08 AM, Bernd Schmidt wrote: > This one is a fix for something that could currently only affect c6x, > but I have code that exposes it on i386. > > When optionally gathering operand info in regrename, we can overflow the > array in certain situations. This can occur when we have a situation > where a value is constructed in multiple small registers and then > accessed as a larger one (CDImode in the testcase I have). In that case > we enter the "superset" path, which fails the involved chains, but the > smaller pieces still all get seen by record_operand_use, and there may > be more of them than MAX_REGS_PER_ADDRESS. > > The following fixes it. Bootstrapped and tested with -frename-registers > enabled at -O1 on x86_64-linux. Ok? > > > Bernd > > rr-fail-op.diff > > > * regrename.c (record_operand_use): Keep track of failed operands > and stop appending if we see any. > * regrename.h (struct operand_rr_info): Add a failed field and shrink > n_chains to short. OK. jeff
* regrename.c (record_operand_use): Keep track of failed operands and stop appending if we see any. * regrename.h (struct operand_rr_info): Add a failed field and shrink n_chains to short. Index: gcc/regrename.c =================================================================== --- gcc/regrename.c (revision 229049) +++ gcc/regrename.c (working copy) @@ -204,8 +204,13 @@ mark_conflict (struct du_head *chains, u static void record_operand_use (struct du_head *head, struct du_chain *this_du) { - if (cur_operand == NULL) + if (cur_operand == NULL || cur_operand->failed) return; + if (head->cannot_rename) + { + cur_operand->failed = true; + return; + } gcc_assert (cur_operand->n_chains < MAX_REGS_PER_ADDRESS); cur_operand->heads[cur_operand->n_chains] = head; cur_operand->chains[cur_operand->n_chains++] = this_du; Index: gcc/regrename.h =================================================================== --- gcc/regrename.h (revision 229049) +++ gcc/regrename.h (working copy) @@ -68,7 +71,8 @@ struct du_chain struct operand_rr_info { /* The number of chains recorded for this operand. */ - int n_chains; + short n_chains; + bool failed; /* Holds either the chain for the operand itself, or for the registers in a memory operand. */ struct du_chain *chains[MAX_REGS_PER_ADDRESS];