On Wed, Jan 8, 2020 at 12:30 PM Bin.Cheng <amker.ch...@gmail.com> wrote:
>
> On Wed, Jan 8, 2020 at 6:31 PM Richard Biener
> <richard.guent...@gmail.com> wrote:
> >
> > On Wed, Jan 8, 2020 at 6:01 AM bin.cheng <bin.ch...@linux.alibaba.com> 
> > wrote:
> > >
> > > Sorry, here is the patch.
> > > ------------------------------------------------------------------
> > > Sender:bin.cheng <bin.ch...@linux.alibaba.com>
> > > Sent At:2020 Jan. 8 (Wed.) 12:58
> > > Recipient:GCC Patches <gcc-patches@gcc.gnu.org>
> > > Subject:[PATCH GCC11]Improve uninitialized warning with value range info
> > >
> > >
> > > Hi,
> > >
> > > Function use_pred_not_overlap_with_undef_path_pred of 
> > > pass_late_warn_uninitialized
> > > checks if predicate of variable use overlaps with predicate of undefined 
> > > control flow path.
> > > For now, it only checks ssa_var comparing against constant, this can be 
> > > improved where
> > > ssa_var compares against another ssa_var with value range info, as 
> > > described in comment:
> > >
> > > +         /* Check value range info of rhs, do following transforms:
> > > +              flag_var < [min, max]  ->  flag_var < max
> > > +              flag_var > [min, max]  ->  flag_var > min
> > > +
> > > +            We can also transform LE_EXPR/GE_EXPR to LT_EXPR/GT_EXPR:
> > > +              flag_var <= [min, max] ->  flag_var < [min, max+1]
> > > +              flag_var >= [min, max] ->  flag_var > [min-1, max]
> > > +            if no overflow/wrap.  */
> > >
> > > This change can avoid some false warning.  Bootstrap and test on x86_64, 
> > > any comment?
> >
> > Definitely a good idea - the refactoring makes the patch hard to
> > follow though.  The
> > original code seems to pick any (the "first") compare against a constant.  
> > You
> > return the "first" but maybe from range info that might also be
> > [-INF,+INF].  It seems
> > that we'd want to pick the best so eventually sort the predicate chain
> > so that compares
> > against constants come first at least?  Not sure if it really makes a
> > difference but
> I don't know either, but I simply tried to not break existing code int
> the patch.
> Function prune_uninit_phi_opnds is called for the first compares against
> constant, actually it should be called for each comparison, but I guess it's
> just avoiding O(M*N) complexity here.

Yeah.  I'm just worried finding a "bad" value-range predicate cuts the search
in a way that causes extra bogus warnings?

>
> > even currently we could have i < 5, i < 1 so the "better" one later?
> > It might also make
> > sense to simply push three predicates for i < j, namely i < j (if ever
> > useful), i < min(j)
> > and max(i) < j to avoid repeatedly doing the range computations.
> IIUC, with current implementation, it's not useful to check value rang
> info for both sides of comparison because prune_uninit_phi_opnds
> requires the flag_var be defined by PHI node in the same basic block
> as PHI parameter.

Yes, but without remembering the code very well my suggestion allows
"new" predicates to be gathered during collecting phase while your patch
adjusts the query phase?

Richard.

> Thanks,
> bin
> >
> > Thanks,
> > Richard.
> >
> > > Thanks,
> > > bin
> > >
> > > 2020-01-08  Bin Cheng  <bin.li...@linux.alibaba.com>
> > >
> > >  * tree-ssa-uninit.c (find_var_cmp_const): New function.
> > >  (use_pred_not_overlap_with_undef_path_pred): Call above.

Reply via email to