[Bug tree-optimization/102631] New: -Wmaybe-uninitialized cannot see through a series of PHIs

[Bug tree-optimization/102631] New: -Wmaybe-uninitialized cannot see through a series of PHIs

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102631

            Bug ID: 102631
           Summary: -Wmaybe-uninitialized cannot see through a series of
                    PHIs
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: aldyh at gcc dot gnu.org
  Target Milestone: ---

Created attachment 51561
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51561&action=edit
-Wmaybe-uninitialized false positive from libgomp with -O2 -Wall

The read from start_data_870 is being flagged as uninitialized, but it is not. 
The uninitialized source (start_data_518(D)) only happens for _1 == 0, and that
can't happen because the entire problematic read is predicated on _1 != 0.

Here is the read (notice it is predicated by _1 != 0:

EXHIBIT A:
  <bb 174> [count: 0]:
  _239 = gomp_tls_data.place;
  _593 = pthread_self ();
  gomp_display_affinity_thread (_593, &MEM <struct gomp_team_state> [(void
*)&gomp_tls_data + 16B], _239);
  if (_1 != 0)
    goto <bb 175>; [0.00%]
  else
    goto <bb 297>; [0.00%]

  <bb 297> [count: 0]:
  goto <bb 181>; [100.00%]

  <bb 175> [count: 0]:
  _240 = nthreads_414(D) + 4294967295;
  _241 = (long unsigned int) _240;
  _242 = _241 * 120;
  _1016 = 16 - _242;
  _341 = start_data_870 + _1016;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-Wmaybe-uninitailized on the read from start_data
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  ivtmp.98_342 = (unsigned long) _341;
  goto <bb 177>; [0.00%]

If we chase start_data_870, we see:

EXHIBIT B:
  <bb 165> [local count: 33009259]:
  # start_data_781 = PHI <start_data_876(289), start_data_518(D)(265)>
  # old_threads_used_887 = PHI <old_threads_used_782(289),
old_threads_used_454(265)>
  # affinity_count_825 = PHI <affinity_count_885(289), affinity_count_343(265)>
  # affinity_thr_904 = PHI <affinity_thr_867(289), 0B(265)>
  # force_display_840 = PHI <force_display_612(289), force_display_192(265)>
  _589 = &MEM[(struct gomp_simple_barrier_t *)pool_410 + 64B].bar;
  gomp_barrier_wait (_589);

  <bb 166> [local count: 66018519]:
  # start_data_870 = PHI <start_data_876(164), start_data_781(165)>
  # old_threads_used_908 = PHI <old_threads_used_782(164),
old_threads_used_887(165)>
  # affinity_count_848 = PHI <affinity_count_885(164), affinity_count_825(165)>
  # affinity_thr_169 = PHI <affinity_thr_867(164), affinity_thr_904(165)>
  # force_display_830 = PHI <force_display_612(164), force_display_840(165)>

start_data_870 could be uninitialized if it came from BB165 because it would be
start_data_781, which in turn could be start_data_518(D) from BB265.  But
notice, we still haven't read from start_data_870.  As I have shown in the
EXHIBIT A, the read from start_data_870 is predicated by _1 != 0.

Perhaps the uninit code is treating the series of PHIs as an uninitialized
read:

  # start_data_781 = PHI <start_data_876(289), start_data_518(D)(265)>
...
...
  # start_data_870 = PHI <start_data_876(164), start_data_781(165)>
...

But AFAIK, the actual uninitialized read from start_data_870 wouldn't happen
until we use it, and that only happens under proper predication:

  if (_1 != 0)
     read from start_data_870

[Bug tree-optimization/102631] -Wmaybe-uninitialized cannot see through a series of PHIs

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102631

--- Comment #1 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
$ ./cc1 team.i -O2 -quiet -Wall
/home/aldyh/src/gcc/libgomp/team.c: In function ‘gomp_team_start’:
/home/aldyh/src/gcc/libgomp/team.c:315:34: warning: ‘start_data’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
  315 |   struct gomp_thread_start_data *start_data = NULL;
      |                                  ^~~~~~~~~~

[Bug tree-optimization/102631] -Wmaybe-uninitialized cannot see through a series of PHIs

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102631

--- Comment #2 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
Created attachment 51562
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51562&action=edit
similar problem on aarch64 bootstrap

[Bug tree-optimization/102631] -Wmaybe-uninitialized cannot see through a series of PHIs

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102631

--- Comment #3 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
(In reply to Aldy Hernandez from comment #2)
> Created attachment 51562 [details]
> similar problem on aarch64 bootstrap

$ ./cc1plus calls-aarch64.ii -O2 -quiet -Wall
In function ‘void mark_stack_region_used(poly_uint64, poly_uint64)’,
    inlined from ‘rtx_def* emit_library_call_value_1(int, rtx, rtx,
libcall_type, machine_mode, int, rtx_mode_t*)’ at
/home/aldyh/src/gcc/gcc/calls.c:4536:29:
/home/aldyh/src/gcc/gcc/calls.c:206:26: warning: ‘const_upper’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
  206 |       stack_usage_map[i] = 1;
      |       ~~~~~~~~~~~~~~~~~~~^~~
/home/aldyh/src/gcc/gcc/calls.c: In function ‘rtx_def*
emit_library_call_value_1(int, rtx, rtx, libcall_type, machine_mode, int,
rtx_mode_t*)’:
/home/aldyh/src/gcc/gcc/calls.c:202:30: note: ‘const_upper’ was declared here
  202 |   unsigned HOST_WIDE_INT const_lower, const_upper;
      |                              ^~~~~~~~~~~

As I've described here:

   https://gcc.gnu.org/pipermail/gcc-patches/2021-October/581045.html

If you take the calls.ii file from the aarch64 bootstrap and break on
the warning, you can see that the uninitalized use is for
const_upper_3934 here:

 <bb 102> [local count: 315357954]:
  # const_upper_3934 = PHI <const_upper_3937(D)(101), _6707(293)>
  if (_881 != 0)
    goto <bb 103>; [50.00%]
  else
    goto <bb 106>; [50.00%]

  <bb 103> [local count: 157678977]:
  if (const_upper_3934 > _6699)
    goto <bb 105>; [89.00%]
  else
    goto <bb 294>; [11.00%]

  <bb 294> [local count: 17344687]:

  <bb 104> [local count: 157678977]:
  goto <bb 107>; [100.00%]

  <bb 105> [local count: 140334290]:
  stack_usage_map.481_3930 = stack_usage_map;
  _6441 = const_upper_3934 - _6699;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
PROBLEMATIC READ HERE
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  _4819 = stack_usage_map.481_3930 + _6699;
  __builtin_memset (_4819, 1, _6441);
  goto <bb 104>; [11.00%]

const_upper_3934 could be undefined if it comes from BB101
(const_upper_3937(D)), but it only gets read for _881 != 0, so it
shouldn't warn.

This looks very similar.

The source is here, which is obviously properly guarded:

static void
mark_stack_region_used (poly_uint64 lower_bound, poly_uint64 upper_bound)
{
  unsigned HOST_WIDE_INT const_lower, const_upper;
  const_lower = constant_lower_bound (lower_bound);
  if (upper_bound.is_constant (&const_upper))
    for (unsigned HOST_WIDE_INT i = const_lower; i < const_upper; ++i)
      stack_usage_map[i] = 1;
  else
    stack_usage_watermark = MIN (stack_usage_watermark, const_lower);
}

[Bug tree-optimization/102631] -Wmaybe-uninitialized cannot see through a series of PHIs

[aldyh at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102631

--- Comment #4 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
For the calls-aarch64.ii testcase, there's some additional information in the
upstream thread.  Quoted here for convenience:

There's some missing context.

The only way to get to BB101->BB102 is through:

   <bb 100>
   if (_6711 != 0)
     goto <bb 101>; [5.50%]
   else
     goto <bb 293>; [94.50%]

And there's an implicit relation between _6711 and _811:

<bb 86>
...
   if (_6711 != 0)
     goto <bb 287>; [5.50%]
   else
     goto <bb 87>; [94.50%]

   <bb 287> [local count: 17344687]:
   goto <bb 88>; [100.00%]

   <bb 87> [local count: 298013267]:

   <bb 88> [local count: 315357954]:
   # _881 = PHI <1(87), 0(287)>

That is, _6711 == !_881.

Presumably the threader shuffled things sufficiently so that the above
relationship is difficult to devise.  It seemsthat  it's having a hard time
noticing that the _6711 and _811 guards are inversely related.

[Bug tree-optimization/102631] -Wmaybe-uninitialized cannot see through a series of PHIs

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102631

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
We run into

MAX_CHAIN_LEN exceeded: 6

and when I up that also into

cycle detected

and then

param_uninit_control_dep_attempts exceeded: 1001

so we get a pruned predicate for the DEF.

The testcase is too unwieldly to more thoroughly analyze :/