On Fri, 12 Oct 2012, Jan Hubicka wrote:
> > On Thu, 11 Oct 2012, Jan Hubicka wrote:
> >
> > > Hi,
> > > this patch address problem I run into with strenghtened cunroll pass. I
> > > made
> > > cunroll to use loop_max_iterations bounds into an account that makes us to
> > > occasionally produce out of bounds loop accesses in loop like:
> > >
> > > for (i=0;i<n;i++)
> > > {
> > > <something>
> > > if (test)
> > > break
> > > a[i]=1;
> > > }
> > > Here the constantly sized array appears in loop with multiple exits and
> > > we then
> > > in the last iteration produce out of bound array (since we need to
> > > duplicate
> > > <something> and tree-ssa-vrp warns.
> > >
> > > I think this is more generic problem (though appearing rarely): as soon
> > > as we
> > > specialize the code, we can no longer warn about out of bounds accesses
> > > when we
> > > prove array ref to be constant. We have no idea if this code path will
> > > execute
> > > in practice.
> > >
> > > Seems resonable? I think similar problems can be constructed by
> > > inlining, too.
> > > Regtested/bootstrapped x86_64-linux.
> >
> > No, I don't think this is reasonable. There are other PRs for this
> > as well, btw.
> >
> > The array-bounds warning in VRP needs to be conditional if the
> > path to it isn't always executed, too (thus a may-be warning).
>
> Well, this is, of course, in full generality a whole program analysis that we
> do not want to solve in the compiler :)
>
> OK, I am also not very happy about this patch, but I need something for the
> cunroll
> change. I wonder what are the options:
> 1) leave the warning and say that -O3 bootstrap need -Wno-error.
> We do used unitialized warnings during -O3 bootstrap too, but I do not
> like
> this variant, since used uninitialized can be silenced by extra
> initialization,
> while this warning can not. Also these warnings are very confusing and
> bogus.
> 2) make complette unrolling to only silence the warnings in the last copy of
> the
> loop only when it knows it may contain out of bound accesses
> 3) make loop-niter really collect basic blocks that must be unreachable in
> the
> last iteration and make cunroll to take this into account and insert
> unreachable builtin on beggining of those blocks.
>
> Obviously 3) is most complette. I am however not sure how to add it to niter
> API. I do not think we want to record the lists of basic block into loop
> bound
> structure since they wil be hard to update. But then we need to someone
> extern max_niter API to optionally return it when needed.
>
> You are more familiar with that code than me, any preferences?
My preference is to do what was requested in some bugreport,
split the array-bounds warning into a may and a must case
(obviously giving away the possibility a function is not executed).
For always executed stmts issue the 'must' case, for not always
executed stmts issue the 'may' case. Disable the may case
for -O3 bootstrap.
The other preference is to attach to-be issued warnings to stmts
and issue them only if the stmt didn't turn out to be dead.
In general there is a conflict between warning late to expose
knowledge from optimization and not emit too many false positives.
That said, I don't worry about -O3 bootstrap warnings - simply
disable those warnings that turn out to be too noisy and have
false positives. I thought we had -Wno-error=array-bounds
for example.
Richard.
