Re: [PATCH, PR 40464] Intra-SRA workaround so that PR 40464 is not hit

[Richard Guenther <rguenther at suse dot de>]

On Fri, 4 Sep 2009, Martin Jambor wrote:

> Hi,
> 
> On Fri, Sep 04, 2009 at 06:02:19PM +0200, Richard Guenther wrote:
> > On Fri, 4 Sep 2009, Martin Jambor wrote:
> > > On Fri, Aug 07, 2009 at 10:48:58AM +0200, Richard Guenther wrote:
> > > > On Fri, 7 Aug 2009, Martin Jambor wrote:
> > > > 
> > > > > On Thu, Aug 06, 2009 at 02:22:26PM -0700, Richard Henderson wrote:
> > > > > > On 08/06/2009 10:54 AM, Martin Jambor wrote:
> > > > > > >tree-complex.c does not do this by itself but through a call to
> > > > > > >force_gimple_opernad_gsi on "REALPART_EXPR<b._M_value>" which leads
> > > > > > >to invocation of gimplify_var_or_parm_decl on decl "b", which, being
> > > > > > >DECL_HAS_VALUE_EXPR_P, is translated by the means of DECL_VALUE_EXPR
> > > > > > >into the variable that is already dead.  More details are in the bug.
> > > > > > 
> > > > > > From the comment in front of DECL_VALUE_EXPR:
> > > > > >   ... once this field has been set, the decl itself may not
> > > > > >   legitimately appear in the function.
> > > > > > 
> > > > > > So the bug is, in fact, that it appears as an argument to the
> > > > > > REALPART_EXPR at all.  Where does the thing come from (or fail
> > > > > > to go away from) in the first place?
> > > > > > 
> > > > > 
> > > > > The second instruction of the first BB in function multiply of
> > > > > g++.dg/torture/pr34099.C is "b.3 = b;" since the first gimple dump.
> > > > > The b on the RHS has a DECL_VALUE_EXPR.  That is where it comes from.
> > > > > 
> > > > > Thanks, I am curious where the bug will eventually be.
> > > > 
> > > > Btw, instrumenting GCC like the following doesnt' show anything on
> > > > x86_64.
> > > 
> > > Thanks for the patch, it's really useful and I'd like to see it
> > > committed at some point in the future.  It has caused over two hundred
> > > failures on hppa in the c test suite alone.  I have opened PR 41250
> > > about this.
> > > 
> > > The statements with DECL_VALUE_EXPR are introduced by
> > > gimplify_parameters() in function.c, specifically by the call to:
> > > gimplify_assign (local, parm, &stmts);
> > > 
> > > Clearly, either this statement (introduced with the tuples merge) or
> > > the comment are wrong.  My favorite hypothesis at the moment is that
> > > the statement should simply not be there.  
> > > 
> > > I'm now running the testsuite with the verification patch and the
> > > statement chucked out.  So far it seems to proceed nicely but it is
> > > slow (and unreliable, the testsuite often just hangs for me).
> > > 
> 
> Well, this apparently does not work, there are some run-time failures.
> 
> > > I'll probably try to find another callee-copy architecture for more
> > > thorough testing and will let you know how things work out.
> > 
> > Oh indeed.  We use DECL_VALUE_EXPR so the gimplifier replaces all
> > param uses with the callee copy target.  But obviously for the
> > copy stmt itself the original decl is still referenced and thus
> > a decl with DECL_VALUE_EXPR remains.
> > 
> > We need to clear DECL_VALUE_EXPR for all parms after gimplifying
> > the function body.  Thus, in gimplify.c where the single call
> > to that function is after gimplifying the body clear them.
> > 
> 
> Won't that mess up debug info?  (Even though... SRA cherrypicking
> individual fields from such a parameter probably will also mess it up
> anyway).

Probably yes.  It's an unfortunate area wrt gimplification.  Maybe
we can introduce a flag in the gimplification context that guards the
DECL_VALUE_EXPR replacement and only set that in the places where it
matters (initial gimplification and nested function lowering).

Richard.