On 9/13/21 4:18 PM, Michael Matz wrote:
> Hello,
>
> On Mon, 13 Sep 2021, Jeff Law via Gcc-patches wrote:
>
>>> So it looks like there's some undefined behavior going on, even before
>>> my patch. I'd like to get some feedback, because this is usually the
>>> type of problems I see in the presence of a smarter threader... things
>>> get shuffled around, problematic code gets isolated, and warning
>>> passes have an easier time (or sometimes harder time) diagnosing
>>> things.
>> The original issue was PRE hanging, so I'd lean towards keeping the test
as-is
>> and instead twiddling any warning flags we can to make the diagnostics go
>> away.
>
> Or use this changed test avoiding the issues that I see with -W -Wall on
> this testcase. I've verified that it still hangs before r194358 and is
> fixed by that revision.
>
> Generally I think, our testsuite, even for ICEs or these kinds of hangs,
> should make an effort to try to write conforming code; if at all possible.
> Here it is possible.
>
> (I don't know if the new threader causes additional warnings, of course,
> but at least the problems with sequence points and uninitialized use of
> 'j' aren't necessary to reproduce the bug)
>
>
> Ciao,
> Michael.
>
> /* { dg-do compile } */
> /* { dg-additional-options "-fno-split-loops" } */
>
> typedef unsigned short uint16_t;
>
> uint16_t a, b;
>
> int *j_global;
> uint16_t f(void)
> {
> int c, **p;
> short d = 2, e = 4;
>
> for (;; b++)
> {
> int *j = j_global, k = 0;
>
> for (; *j; j++)
> {
> for(; c; c++)
> for(; k < 1; k++)
> {
> short *f = &d;
>
> if(b)
> return *f;
> }
> }
>
> if(!c)
> d *= e;
>
> a = d;
> if ((a ? b = 0 : (**p ? : 1) != (d != 1 ? 1 : (b = 0))) != ((k ? a
: 0)
> < (a * (c = k))))
> **p = 0;
> }
> }
>
Thanks for getting rid of the noise here.
I've simplified the above to show what's going on in the warning on
nds32-elf:
int george, *global;
int stuff(), readme();
int
f (void)
{
int store;
for (;;)
{
int k = 0;
while (global)
{
for (; store; ++store)
{
for (; k < 1; k++)
{
if (readme())
return 0;
}
}
}
store = k;
if (george)
stuff();
}
}
The -Waggressive-loop-optimizations pass is complaining because of an
undefined iteration in the for(;store;++store) loop. But this looks
like it's getting confused by threader having isolated an undefined path.
At the warning, the IL looks like this on entry:
<bb 2> [local count: 55807730]:
goto <bb 4>; [100.00%]
<bb 4> [local count: 57254340]:
# store_4 = PHI <k_42(3), store_14(D)(2)>
global.0_25 = global;
if (global.0_25 != 0B)
goto <bb 12>; [94.50%]
else
goto <bb 13>; [5.50%]
...
...
<bb 12> [local count: 54105352]:
if (store_4 != 0)
goto <bb 7>; [99.64%]
else
goto <bb 10>; [0.36%]
If global.0_25 was true on entry, the read from store_4 would be
undefined. Presumably the warning pass is assuming this path always
gets executed.
This looks like a latent bug. For that matter, the above snippet warns
with -fdisable-tree-thread2, even on x86-64 (and before my
patch).
Aldy
