On Fri, Feb 7, 2020 at 8:58 AM Jakub Jelinek <ja...@redhat.com> wrote: > > Hi! > > The following testcase ICEs. The generated split_insns starts > with recog_data.insn = NULL and then tries to put various operands into > recog_data.operand array and checks various splitter conditions. > The problem is that some atom related tuning splitters indirectly call > extract_insn_cached on the insn they are used in. This can change > recog_data.operand, but most likely it will just keep it as is, but > sets recog_data.insn to the current instruction. If that splitter doesn't > match, we continue trying some other split conditions and modify > recog_data.operand array again. If even that doesn't find any usable > splitter, we punt, but at that point recog_data.insn says that recog_data > is valid for that particular instruction, even when recog_data.operand array > can be anything. > The safest thing would be to copy whole recog_data to a temporary object > before doing the calls that can call extract_insn_cached and restore it > afterwards, but it would be also very costly, recog_data has 1280 bytes. > So, this patch just makes sure to clear recog_data.insn if it has changed > during the extract_insn_cached call, which means if we extract_insn_cached > later, we'll extract it properly, while if we call it say from some other > context than splitter conditions, the insn is already cached, we don't reset > the cache. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2020-02-07 Jakub Jelinek <ja...@redhat.com> > > PR target/93611 > * config/i386/i386.c (ix86_lea_outperforms): Make sure to clear > recog_data.insn if distance_non_agu_define changed it. > > * gcc.target/i386/pr93611.c: New test.
LGTM. Thanks, Uros. > --- gcc/config/i386/i386.c.jj 2020-01-28 08:45:56.781090684 +0100 > +++ gcc/config/i386/i386.c 2020-02-06 16:29:35.548663197 +0100 > @@ -14459,9 +14459,18 @@ ix86_lea_outperforms (rtx_insn *insn, un > return true; > } > > + rtx_insn *rinsn = recog_data.insn; > + > dist_define = distance_non_agu_define (regno1, regno2, insn); > dist_use = distance_agu_use (regno0, insn); > > + /* distance_non_agu_define can call extract_insn_cached. If this function > + is called from define_split conditions, that can break insn splitting, > + because split_insns works by clearing recog_data.insn and then modifying > + recog_data.operand array and match the various split conditions. */ > + if (recog_data.insn != rinsn) > + recog_data.insn = NULL; > + > if (dist_define < 0 || dist_define >= LEA_MAX_STALL) > { > /* If there is no non AGU operand definition, no AGU > --- gcc/testsuite/gcc.target/i386/pr93611.c.jj 2020-02-06 12:24:28.005976435 > +0100 > +++ gcc/testsuite/gcc.target/i386/pr93611.c 2020-02-06 12:24:17.685131826 > +0100 > @@ -0,0 +1,5 @@ > +/* PR target/93611 */ > +/* { dg-do compile } */ > +/* { dg-options "-fira-algorithm=priority -O3 -mtune=bonnell" } */ > + > +#include "../../gcc.dg/vect/pr58508.c" > > Jakub >