After Jeff's explanation of the symbiosis between jump threading and
the uninit pass, I'm beginning to see that (almost) every
Wuninitialized warning is cause for reflection. It usually hides a
missing jump thread. I investigated one such false positive
(uninit-pred-7_a.c) and indeed, there's a missing thread. The
question is what to do about it.
This seemingly simple test is now regressing as can be seen by the
xfail I added.
What happens is that we now thread far more than before, causing the
distance from definition to use to expand. The threading candidate
that would make the Wuninitialized go away is there, and the backward
threader can see it, but it refuses to thread it because the number of
statements would be too large.
This is interesting because it means threading is causing larger IL
that in turn keeps us from threading some unreachable paths later on
because the paths are too large.
If you look at the *.threadfull2 dump for the attached simplified
test, you can see that the 3->5->6-8->10->13 path would elide the
unreachable read, but alas we can't look past BB5, because it would
thread too many statements:
Checking profitability of path (backwards): bb:10 (2 insns) bb:8 (2
insns) bb:6 (6 insns) bb:5
Control statement insns: 2
Overall: 8 insns
FAIL: Did not thread around loop and would copy too many statements.
The "problem" we have is that if there's a path in the IL, the new
threader *will* exploit it (ranger dependent). This in turns opens up
opportunities for other threaders (even DOM) creating a cascading
effect.
For the attached test, we can squelched the warning with a mere:
--param=max-jump-thread-duplication-stmts=19
I don't know how we decided on the default 15 param, and if it makes
sense to tweak this, but our current threading passes, and how they
relate to VRP and the uninit pass look like this:
$ ls a.c.* | grep -e thread -e dom[23] -e vrp[12] -e uninit
a.c.034t.ethread
a.c.111t.threadfull1
a.c.112t.vrp1
a.c.126t.thread1
a.c.127t.dom2
a.c.191t.thread2
a.c.192t.dom3
a.c.194t.threadfull2
a.c.195t.vrp2
a.c.209t.uninit1
Perhaps we could turn down the knobs for thread[12] and increase them
for threadfull[12]? I really don't know. For this particular test,
we could even turn off thread1, increase the duplication statements,
and eliminate the warning. This would leave DOM2 without the threader
that runs before it though.
I'm out of my depth here, plus I'm a bit hesitant to make performance
decisions to improve warnings. On the other hand, it's sad that
improved threading is causing regressions on a test as simple as this
one. That being said, I generally don't mention it, but the threading
improvements so far solve more problems than they introduce, so
perhaps we should do nothing??
I'd be curious to hear what others think. Perhaps others could play
with the different knobs and see if there's a better combination that
could keep warnings and optimizers in better equilibrium.
[FWIW Martin, you could revisit some of the uninit regressions and see
if tweaking the above --param would silence the bogus warning. In
which case, it's a hint that the regression may not be due to the
uninit code itself. FTR, I'm not saying we _should_ thread more, just
that this could be a tool to help diagnose].
Aldy
/* { dg-do compile } */
/* { dg-options "-Wuninitialized -O2" } */
int g;
void blah1(int);
void blah2(int);
void crapola(int);
int foo (int n, int l, int m, int r)
{
int v;
if (n || l)
v = r;
if (m)
g++;
if ( n && l)
blah1(v); /* { dg-bogus "uninitialized" "bogus warning" } */
if ( n )
blah2(v); /* { dg-bogus "uninitialized" "bogus warning" } */
if ( l )
crapola(v); /* { dg-bogus "uninitialized" "bogus warning" { xfail *-*-* } } */
return 0;
}
