On Mon, Nov 25, 2019 at 09:40:36PM +0000, Richard Sandiford wrote:
> Segher Boessenkool <seg...@kernel.crashing.org> writes:
> > - i386 goes into an infinite loop compiling, or at least an hour or so...
> > Erm I forgot too record what it was compiling. I did attach a GDB... It
> > is something from lra_create_live_ranges.
>
> Hmm.
This one is actually worrying me -- it's not obviously a simple problem,
or a target problem, or a pre-existing problem.
> Ah, this'll be while m68k was still a cc0 target. Yeah, I should probably
> just skip the whole pass for cc0.
Yes, tree of last friday or saturday or so.
And yup if you don't handle cc0 yet, yes you want to skip it completely :-)
> > - sh (that's sh4-linux):
> >
> > /home/segher/src/kernel/net/ipv4/af_inet.c: In function
> > 'snmp_get_cpu_field':
> > /home/segher/src/kernel/net/ipv4/af_inet.c:1638:1: error: unable to find a
> > register to spill in class 'R0_REGS'
> > 1638 | }
> > | ^
> > /home/segher/src/kernel/net/ipv4/af_inet.c:1638:1: error: this is the insn:
> > (insn 18 17 19 2 (set (reg:SI 0 r0)
> > (mem:SI (plus:SI (reg:SI 4 r4 [178])
> > (reg:SI 6 r6 [171])) [17 *_3+0 S4 A32]))
> > "/home/segher/src/kernel/net/ipv4/af_inet.c":1638:1 188 {movsi_i}
> > (expr_list:REG_DEAD (reg:SI 4 r4 [178])
> > (expr_list:REG_DEAD (reg:SI 6 r6 [171])
> > (nil))))
> > /home/segher/src/kernel/net/ipv4/af_inet.c:1638: confused by earlier
> > errors, bailing out
>
> Would have to look more at this one. Seems odd that it can't allocate
> R0 when it's already the destination and when R0 can't be live before
> the insn. But there again, this is reload, so my enthuasiasm for looking
> is a bit limited :-)
It wants to use r0 in some other insn, so it needs to spill it here, but
cannot. This is what class_likely_spilled is for.
> > Looking at just binary size, which is a good stand-in for how many insns
> > it combined:
> >
> > R2
> > arm64 99.709%
> > ia64 99.651%
> > s390 99.734%
> > sparc 99.877%
> > sparc64 100.011%
> >
> > (These are those that are not between 99.9% and 100.0%).
> >
> > So only sparc64 regressed, and just a tiny bit (I can look at what that
> > is, if there is interest). But 32-bit sparc improved, and s390, arm64,
> > and ia64 got actual benefit.
> >
> > Again this is just code size, not analysing the actually changed code.
>
> OK. Certainly not an earth-shattering improvement then, but not
> entirely worthless either.
I usually takes 0.2% as "definitely useful" for combine improvements, so
there are a few targets that have that. There can be improvements that
are important for a target even if they do not improve code size much,
of course, and it can identify weaknesses in the backend code, so you
always need to look at what really changes.
> I see combine also tests cannot_copy_insn_p. I'm not sure whether that's
> appropriate for the new pass or not. Arguably it's not copying the
> instruction, it's just moving it to be in parallel with something else.
> (But then that's largely true of the combine case too.)
combine tests this only for the cases where it *does* have to copy an
insn: when the dest if i0, i1, or i2 doesn't die, it is added as another
arm to the (parallel) result.
Segher
