On Fri, 2020-03-13 at 09:09 +0100, Christophe Lyon wrote:
> Hi,
> 
> 
> On Thu, 12 Mar 2020 at 23:12, Jeff Law via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> > On Thu, 2020-03-12 at 13:23 -0500, Segher Boessenkool wrote:
> > > On Thu, Mar 12, 2020 at 12:03:08PM -0600, Jeff Law wrote:
> > > > On Sat, 2020-02-08 at 10:41 -0600, Segher Boessenkool wrote:
> > > > > I don't think each stanza of code should use it's own "noop-ness", no.
> > > > Richard's patch is actually better than mine in that regard as it 
> > > > handles
> > > > mem
> > > > and
> > > > reg nop moves in an indentical way.  I don't think refactoring the cse.c
> > > > code
> > > > is
> > > > advisable now though -- but I do want to fix the multiply-reported ICE 
> > > > on
> > > > ARM
> > > > and
> > > > Richard's cse changes are the cleanest way to do that that I can see.
> > > 
> > > It looks pretty simple, yeah...  All of CSE is hopelessly fragile, but
> > > this patch does not make things worse.
> > > 
> > > > > I don't know if this patch makes matters worse or not.  It doesn't 
> > > > > seem
> > > > > suitable for stage 4 though.  And Richard said the cse.c part breaks
> > > > > rs6000, if that is true, yes I do object ;-)
> > > > The rs6000 port breakage is trivial to fix.  In fact, I did so and ran 
> > > > it
> > > > through
> > > > my tester, which includes ppc64le and ppc64 a slew of *-elf targets x86
> > > > native
> > > > and more.
> > > 
> > > I don't see anything rs6000 below?  Is it just this generic code?
> > > 
> > > > @@ -5324,9 +5324,11 @@ cse_insn (rtx_insn *insn)
> > > >         }
> > > > 
> > > >       /* Similarly, lots of targets don't allow no-op
> > > > -        (set (mem x) (mem x)) moves.  */
> > > > +        (set (mem x) (mem x)) moves.  Even (set (reg x) (reg x))
> > > > +        might be impossible for certain registers (like CC
> > > > registers).  */
> > > >       else if (n_sets == 1
> > > > -              && MEM_P (trial)
> > > > +              && ! CALL_P (insn)
> > > > +              && (MEM_P (trial) || REG_P (trial))
> > > >                && MEM_P (dest)
> > > >                && rtx_equal_p (trial, dest)
> > > >                && !side_effects_p (dest)
> > > 
> > > This adds the !CALL_P (no space btw) condition, why is that?
> > > 
> > > (Is that CCmode reg-reg move comment about rs6000?  Huh, we *do* have
> > > patterns for that, or *should* have at least!)
> > I fixed the extraneous whitespace and committed the change.
> > 
> 
> The new test fails on ARM:
THanks.  I see what's happening, though I'm not sure *how* it happened.  Anyway,
doing some testing on the fix now.

jeff
> 

Reply via email to