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:
FAIL: gcc.c-torture/compile/pr90275.c -O3 -g (internal compiler error)
during RTL pass: cse_local
/gcc/testsuite/gcc.c-torture/compile/pr90275.c: In function 'e':
/gcc/testsuite/gcc.c-torture/compile/pr90275.c:25:1: internal compiler
error: in insert_regs, at cse.c:1128
0x15725bd insert_regs
/gcc/cse.c:1128
0x1579731 cse_insn
/gcc/cse.c:5957
0x157aff6 cse_extended_basic_block
/gcc/cse.c:6615
0x157aff6 cse_main
/gcc/cse.c:6794
0x157bc0d rest_of_handle_cse_after_global_opts
/gcc/cse.c:7766
0x157bc0d execute
/gcc/cse.c:7817
Please submit a full bug report,
Is the patch supposed to fix all the ICEs on ARM?
I see this with cross-compilers, it seems OK on native builds? (I
can't find error reports about this on gcc-testresults)
Christophe
> THanks,
> jeff
> >
>
