On Mon, Sep 01, 2014 at 10:39:10AM -0600, Jeff Law wrote:
> On 09/01/14 05:38, Segher Boessenkool wrote:
> >On Mon, Sep 01, 2014 at 11:36:07AM +0800, Bin.Cheng wrote:
> >>>In the testcase (and comment in the proposed patch), why is combine
> >>>combining four insns at all? That means it rejected combining just the
> >>>first three. Why did it do that?
> >>It is explicitly reject by below code in can_combine_p.
> >>
> >> if (GET_CODE (PATTERN (i3)) == PARALLEL)
> >> for (i = XVECLEN (PATTERN (i3), 0) - 1; i >= 0; i--)
> >> if (GET_CODE (XVECEXP (PATTERN (i3), 0, i)) == CLOBBER)
> >> {
> >> /* Don't substitute for a register intended as a clobberable
> >> operand. */
> >> rtx reg = XEXP (XVECEXP (PATTERN (i3), 0, i), 0);
> >> if (rtx_equal_p (reg, dest))
> >> return 0;
> >>
> >>Since insn i2 in the list of i0/i1/i2 as below contains parallel
> >>clobber of dest_of_insn76/use_of_insn77.
> >> 32: r84:SI=0
> >> 76: flags:CC=cmp(r84:SI,0x1)
> >> REG_DEAD r84:SI
> >> 77: {r84:SI=-ltu(flags:CC,0);clobber flags:CC;}
> >> REG_DEAD flags:CC
> >> REG_UNUSED flags:CC
> >
> >Archaeology suggests this check is because the clobber might be an
> >earlyclobber. Which seems silly: how can it be a valid insn at all
> >in that case? It seems to me the check can just be removed. That
> >will hide your issue, maybe even solve it (but I doubt it).
> Silly for other reasons, namely that earlyclobber doesn't come into play
> until after combine (register allocation and later).
The last change to this code was by Ulrich (cc:ed); in that thread (June
2004, mostly not threaded in the mail archive, broken MUAs :-( ) it was
said that any clobber should be considered an earlyclobber (an RTL insn
can expand to multiple machine instructions, for example). But I don't
see how that can matter for "dest" here (the dest of "insn", that's 76
in the example), only for "src".
The version of "flags" set in 76 obviously dies in 77 (it clobbers the
reg after all), but there is no way it could clobber it before it uses
it, that just makes no sense. And in the combined insn that version of
flags does not exist at all.
> >Another question is why is r84 set twice in the first place?
> Various transformations can set that kind of situation up.
Sure -- but also lazy expanders can reuse a register instead of doing
gen_reg_rtx. Which is why I asked :-)
> One could
> argue that a local pass ssa-like-ize things like this would be good
> independent of this BZ. The web code would probably help here, but
> seems awful heavyweight for an opportunity like this.
Not worth the effort I'd say.
Segher
