Message ID | alpine.LSU.2.11.1510020935350.6516@zhemvz.fhfr.qr |
---|---|
State | New |
Headers | show |
On 10/02/2015 01:37 AM, Richard Biener wrote: > > The following patch doesn't pass bootstrap & regtest. It did at some > point though and its comment hints that fixing leaks after inlining > was too interesting a problem to solve ;) > > Thus patch is FYI. > > Richard. > > Index: tree-ssa.c > =================================================================== > --- tree-ssa.c (revision 228320) > +++ tree-ssa.c (working copy) > @@ -693,6 +693,16 @@ verify_def (basic_block bb, basic_block > goto err; > } > > + if (bb == NULL > + /* ??? Too many latent cases in the main opt pipeline. But it's > + worth to fix all cases before inlining as that reduces the > + amount of garbage kept live. */ > + && !cfun->after_inlining) > + { > + error ("removed STMT failed to release SSA name"); > + goto err; > + } > + I was building the verification step into the ssa name manager. Essentially at the point where we flush from the pending to the free list, we should have a consistent state. Thus we ought to be able to walk the IL marking everything we can see, combine that with the contents of the freelist and the result ought to be every SSA_NAME ever created. Reality is somewhat different, of course. Yours takes a slightly different approach. Ultimately if we get the leaks plugged, we might even consider using both. jeff
On Fri, Oct 2, 2015 at 5:24 PM, Jeff Law <law@redhat.com> wrote: > On 10/02/2015 01:37 AM, Richard Biener wrote: >> >> >> The following patch doesn't pass bootstrap & regtest. It did at some >> point though and its comment hints that fixing leaks after inlining >> was too interesting a problem to solve ;) >> >> Thus patch is FYI. >> >> Richard. >> >> Index: tree-ssa.c >> =================================================================== >> --- tree-ssa.c (revision 228320) >> +++ tree-ssa.c (working copy) >> @@ -693,6 +693,16 @@ verify_def (basic_block bb, basic_block >> goto err; >> } >> >> + if (bb == NULL >> + /* ??? Too many latent cases in the main opt pipeline. But it's >> + worth to fix all cases before inlining as that reduces the >> + amount of garbage kept live. */ >> + && !cfun->after_inlining) >> + { >> + error ("removed STMT failed to release SSA name"); >> + goto err; >> + } >> + > > I was building the verification step into the ssa name manager. Essentially > at the point where we flush from the pending to the free list, we should > have a consistent state. Yeah, though when SSA verifiers run the state should also be consistent and we'd get to pinpoint the offending pass easier. > Thus we ought to be able to walk the IL marking everything we can see, > combine that with the contents of the freelist and the result ought to be > every SSA_NAME ever created. > > Reality is somewhat different, of course. > > Yours takes a slightly different approach. Ultimately if we get the leaks > plugged, we might even consider using both. Sure. Note that the above is from simply walking all SSA names. Richard. > jeff > > >
On 10/05/2015 02:56 AM, Richard Biener wrote: >> I was building the verification step into the ssa name manager. Essentially >> at the point where we flush from the pending to the free list, we should >> have a consistent state. > > Yeah, though when SSA verifiers run the state should also be consistent > and we'd get to pinpoint the offending pass easier. Agreed. > >> Thus we ought to be able to walk the IL marking everything we can see, >> combine that with the contents of the freelist and the result ought to be >> every SSA_NAME ever created. >> >> Reality is somewhat different, of course. >> >> Yours takes a slightly different approach. Ultimately if we get the leaks >> plugged, we might even consider using both. > > Sure. Note that the above is from simply walking all SSA names. Right. That's precisely what I was referring to. Yours walks the SSA_NAMEs and declares those with an empty block for the defining statement as leaks. Mine walks the IL and declares any name that was allocated, but not found in the IL as a leak. Mine is obviously more expensive, but possibly catches more leaks. The one conclusion I did come to over the weekend is that without a recycle immediate policy, there's no value in explicitly releasing the names. ie, we could just drop all the explicit management and rely on garbage collection at safe points. If we added either a mode to allow immediate recycling or an adaptive behaviour in the manager to recycle immediately until it wasn't safe to do so, then there's obviously value in the explicit releases and plugging the leaks. jeff
Index: tree-ssa.c =================================================================== --- tree-ssa.c (revision 228320) +++ tree-ssa.c (working copy) @@ -693,6 +693,16 @@ verify_def (basic_block bb, basic_block goto err; } + if (bb == NULL + /* ??? Too many latent cases in the main opt pipeline. But it's + worth to fix all cases before inlining as that reduces the + amount of garbage kept live. */ + && !cfun->after_inlining) + { + error ("removed STMT failed to release SSA name"); + goto err; + } + if (definition_block[SSA_NAME_VERSION (ssa_name)]) { error ("SSA_NAME created in two different blocks %i and %i",