On Tue, 21 Feb 2017, Jeff Law wrote:
> On 02/21/2017 02:01 AM, Richard Biener wrote:
> > On Mon, 20 Feb 2017, Marc Glisse wrote:
> >
> > > On Mon, 20 Feb 2017, Jakub Jelinek wrote:
> > >
> > > > As mentioned by Jason in the PR, we've regressed on the following
> > > > testcase
> > > > since we started emitting CLOBBERs at the start of ctors (and we warn as
> > > > before with -fno-lifetime-dse -Wuninitialized).
> > > > With -fno-lifetime-dse, the vuse on the b.x read stmt is default def
> > > > and thus we warn, but if there are clobbers before that, that is not the
> > > > case and we don't warn. The patch is quick hack to bypass the initial
> > > > clobbers as long as there aren't really many. If we wanted to handle
> > > > all initial clobbers, I bet the first time we run into this we could
> > > > recursively walk vop uses from the default def and build say a bitmap
> > > > of vop SSA_NAMEs which are vdefs of clobbers that only have clobbers
> > > > before it as vdef stmts.
> > >
> > > MEM[(struct &)&b] ={v} {CLOBBER};
> > > _4 = b.x;
> > > if (_4 != 0)
> > >
> > > It would be nice (at some point in a distant future) to turn that into
> > >
> > > MEM[(struct &)&b] ={v} {CLOBBER};
> > > if (_4(D) != 0)
> > >
> > > i.e. replace reads from clobbered memory with a default def (with a
> > > gimple_nop
> > > defining statement).
> > >
> > > IIRC I tried to do it in FRE once, but failed.
> >
> > Yeah, FRE isn't too happy about VN_TOP in the lattice (which is what
> > you'd use here). I also have/had patches trying to make it (more) happy
> > about VN_TOP but it still broke stuff like for example tail-merging...
> >
> > Need to dissect PRE from tail-merging again, and also possibly PRE
> > from value-numbering (run eliminate() before doing PRE).
> Couldn't this be easily modeled as redundant load elimination? Pretend the
> clobber is a read, which would make the read of b.x redundant.
>
> I haven't prototyped it, but that'd be the first approach I'd try.
Sure, but that only works for must-be-uninit reads. The question is
also what to replace the redundant reads with, esp. if you consider
the read could be of aggregate type. Obvious choices would be
default-defs for regiser type reads and clobbers for aggregate typed
ones (but then clobbers are not valid in call argument context for
example). Another choice would be {} (or zero for register types),
same issue for call arg context.
For maybe-uninit reads you could model it as load hoisting -- you'd
hoist towards may-defs and must-uninit points and insert appropriate
PHIs -- again difficult for the aggregate case.
But yes, currently FRE/PRE treat uninitialized as varying, that's
something we can improve on.
Richard.
