On Wed, Jan 8, 2020 at 7:50 PM Richard Biener
<richard.guent...@gmail.com> wrote:
>
> 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?
hmm, the code is now as:
+ cmp_code = find_var_cmp_const (preds, phi, &flag_def, &boundary_cst, false);
+ if (cmp_code == ERROR_MARK)
+ cmp_code = find_var_cmp_const (preds, phi, &flag_def, &boundary_cst, true);
+ if (cmp_code == ERROR_MARK)
return false;
First call to find_var_cmp_const preserves the original behavior,
while the second call
only finds value range case if there is no comparison cont is found by
the first call.
So there is no 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?
I am not quite follow here. Do you mean we collect three cases "i <
j", "i < min(j)", "max(i) < j" then
call prune_uninit_phi_opnds for all three conditions?
This is another question? because now we simply break out of for loop
for finding such condition:
- if ((gimple_code (flag_def) == GIMPLE_PHI)
- && (gimple_bb (flag_def) == gimple_bb (phi))
- && find_matching_predicate_in_rest_chains (the_pred, preds,
- num_preds))
- break;
It's always possible that this flag_def can't prune use predicates
against undefined path predicates, while a later one can prune but is
skipped?
Thanks,
bin
>
> 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.
