On Thu, May 16, 2019 at 1:09 AM Vladislav Ivanishin <v...@ispras.ru> wrote:
>
> Hi!
>
> Here is a simple patch fixing the regression introduced in r270660.
>
>
> Regtested and bootstrapped with `BOOT_CFLAGS="-O
> -Wno-error=maybe-uninitialized -Wuninitialized"` on x86_64-pc-linux-gnu.
> OK for trunk?
OK.
> It is not the ideal solution, but it is the simplest (and safest). So
> let's roll with it and then I've already prepared an improvement, but
> need your input.
>
> The case where code2 == NE_EXPR and code1 == BIT_AND_EXPR is now (with
> this patch) caught by
>
> if (code1 != code2)
> return false;
>
> This means a false positive in some cases: e.g. if we change 5u to 16u
> in the gimple test case above, we'll have
>
> if (v != 16)
> u = sth;
> ...
> if (v & 3)
> use (u);
>
> for which a warning will be generated (and it's an FP: u is uninit iff
> v == 16, but in that case u is not used).
>
> This can be fixed with something like this:
>
> if (code2 == NE_EXPR)
> {
> if (code1 == BIT_AND_EXPR)
> return !((wi::to_wide (expr1.pred_rhs) & wi::to_wide (expr2.pred_rhs))
> .to_uhwi());
> else
> return !is_value_included_in (expr2.pred_rhs, expr1.pred_rhs, code1);
> }
>
> But actually I would rather extend is_value_included_in to handle
> BIT_AND_EXPR. OTOH, its other callers don't seem to need it. My
> concern is that handling BIT_AND_EXPR in is_value_included_in as I would
> like to do would make the assertion in is_value_included_in weaker.
>
> Frankly, I don't completely understand where BIT_AND_EXPR can and cannot
> appear.
>
> Would you have any suggestions?
I don't remember enough around this code to give an advice without digging in
myself. svn blame might help to uncover reasoning from the original patch
submission adding BIT_AND_EXPR support.
Richard.
> Thank you.
> Vlad
