On Wed, Mar 29, 2023 at 7:22 PM Andrew MacLeod <amacl...@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
