This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA:] Fix PR 40086 - reorg.c again and again


> From: Eric Botcazou <ebotcazou@adacore.com>
> Date: Mon, 18 May 2009 18:55:07 +0200

> > In the trail for *this* PR 40086, I listed a couple of
> > alternative solutions, but I went for the same fix as in PR
> > optimization/15296.  In the patch below, I changed just one
> > next_active_insn to next_real insn, the one exposed by the
> > regression in the PR.  What I'd like to do is to change *all*
> > calls to next_active_insn into next_real insn in reorg.c and
> > resource.c (not completely correct; what I'd *really* like to do
> > is to rewrite reorg.c, alas...)
> 
> I wonder whether this is not a common case though and, consequently, whether 
> this won't pessimize too much.  Suppose the old USE comes from an insn moved 
> to the delay slot of the jump in a previous pass; computing the redundancy of 
> the next active insn while disregarding the USE in-betwenn is correct since 
> redundant_insn will take into account the insns in the delay slot, hence the 
> USE effectively.

That's in a sentence the bug of the current code: it won't be
correct for any other jump to the same target label or to a
subsequent insn, because for that register, the life info after
the move isn't considered, as the bb regs-live info isn't
updated after a reorg_pass_number round, yet used as if it were
so.  Hm, it sounds like a bb rtl dump right before the invalid
transformation is required for proper visualization...

>  So, if the outcome is positive, it is correct to delete the 
> next active insn and then try with the following one, the only requirement 
> being to preserve the old USE.
> 
> Did you take a look at the impact on gcc.c-torture/compile at -O2 for example?

No, as safe correctness was the issue I prioritized re-using the
previous solution.  I thought it couldn't happen too often, or
this bug would have been noticed before.  What indications of
non-pessimization do you require?

> > Ok for trunk and all open branches?
> 
> Things are different on the trunk since I changed back the DF problem to LIVE 
> so I'm not sure we need to patch reorg.c everywhere.

I'm pretty sure it's needed everywhere.  I thought I made the
point that PR15296 is virtually the same!  What effects would
the DF change have?  IIUC better live information (less live
registers) only aggravates the problem, it does not cause it.

brgds, H-P


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]