Re: recomputation and PR 109154

[Richard Biener <richard.guenther@gmail.com>]

On Wed, Mar 29, 2023 at 7:22 PM Andrew MacLeod <amacleod@redhat.com> wrote:
>
> The patch, or a slight variation (attached), in the PR allows us to
> generate better ranges be recomputing longer instruction sequences on
> outgoing edges.
>
> This in fact also fixes
> XPASS: gcc.dg/Walloca-13.c  (test for bogus messages, line 11)
>
>    <bb 2> [local count: 1073741824]:
>    _1 = p_5(D) - q_6(D);
>    _2 = _1 /[ex] 4;
>    n_7 = (long unsigned int) _2;
>    _11 = (long unsigned int) _1;
>    if (_11 <= 396)
>      goto <bb 3>; [33.00%]
>    else
>      goto <bb 4>; [67.00%]
>
>    <bb 3> [local count: 354334800]:
>    _3 = __builtin_alloca (n_7);
>
> Where _2 was recomputed before, but n_7 was not.  Now it is, and we
> correctly do not issue the warning any more.  awesome.,
>
> however, as seems to be the case often, better ranges result in, I now get:
>
> FAIL: 23_containers/vector/bool/allocator/copy.cc (test for excess errors)
>
> because we now generate:
>
> /opt/notnfs/amacleod/master/build/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h:437:
> warning: ‘void* __builtin_memmove(void*, const void*, long unsigned
> int)’ writing between 9 and 9223372036854775807 bytes into a region of
> size 8 overflows the destination [-Wstringop-overflow=]
>
>   I see:
> <BB 2>
>   ....
>      _216 = operator new (8);
>
> _216 : [irange] long unsigned int * [1, +INF]
>    ......
>
>      <bb 8> [local count: 86938296]:
>      D.245552 ={v} {CLOBBER(eol)};
>      _74 = v1.D.217578._M_impl.D.217043._M_start.D.58619._M_p;
>      _638 = (long int) _74;
>      _261 = -_638;
>      _383 = (long unsigned int) _261;
>      if (_638 < -8)
>        goto <bb 12>; [90.00%]
>      else
>        goto <bb 13>; [10.00%]
>
> _261 : [irange] long int [-9223372036854775807, +INF]
> _383 : [irange] long unsigned int [0,
> 9223372036854775807][9223372036854775809, +INF]
> 8->12  (T) _74 :        [irange] _Bit_type * [1, +INF]
> 8->12  (T) _261 :       [irange] long int [9, +INF] NONZERO
> 0x7fffffffffffffff
> 8->12  (T) _383 :       [irange] long unsigned int [9,
> 9223372036854775807] NONZERO 0x7fffffffffffffff
> 8->12  (T) _638 :       [irange] long int [-INF, -9]
>
> =========== BB 12 ============
> _74     [irange] _Bit_type * [9223372036854775808, 18446744073709551607]
> _383    [irange] long unsigned int [9, 9223372036854775807] NONZERO
> 0x7fffffffffffffff
>      <bb 12> [local count: 78244465]:
>      __builtin_memmove (_216, _74, _383);
>
>
>
> The change is that we now recompute _383 which we didnt before. so we
> are seeing memmove being called on what is effectively:
> memmove (operator new (8), _74, [9, 9223372036854775807])
> And thus the warning.
>
> IS this one of the warnings that has been causing issues?  and now Im
> triggering it again?

Yeah, we see these kind of diagnostics on code that's supposed to be
not reachable but we don't figure that out (missed-optimization) or the
code is written in a way that doesn't make this obvious.

>
> Back at fixup_cfg3 time, it looks like:
>
>   _261 = __last$D58797$_M_p_245 - _247;
>    _262 = _261 > 8;
>    _263 = (long int) _262;
>    _264 = __builtin_expect (_263, 1);
>    if (_264 != 0)
>      goto <bb 47>; [90.00%]
>    else
>      goto <bb 48>; [10.00%]
> ..................
>    <bb 47> [local count: 78244465]:
>    _265 = (long unsigned int) _261;
>    __builtin_memmove (_246, _247, _265);
>
> So the builtin expect certainly implies it is expecting to have a value > 8
>
> Early on the code looks like:
> _1 = __last_10(D) - __first_11(D);
>    _Num_12 = _1 /[ex] 8;
>    _2 = _Num_12 > 1;
>    _3 = (long int) _2;
>    _4 = __builtin_expect (_3, 1);
>    if (_4 != 0)
>      goto <bb 3>; [INV]
>    else
>      goto <bb 4>; [INV]
>
>    <bb 3> :
>    _Num.28_5 = (long unsigned int) _Num_12;
>    _6 = _Num.28_5 * 8;
>    __builtin_memmove (__result_14(D), __first_11(D), _6);
>
>
> SO it does still do basically the same thing.
>
> Im not sure whether this is pointing out something real or another false
> positive...
>
> Andrew