On 01/05/2013 01:10 PM, Steven Bosscher wrote:
Presumably BB7 was created in response to the builtin_unreachable?
Yes. The block only contains the BB_UNREACHABLE call. It is cleaned up
at the end of the GIMPLE passes pipeline, in the fold-all-builtins
pass (most __builtin_unreachable calls are, but not all).
I think if you eliminate the block and the cleanup the CFG
appropriately, the right thing will just happen.
The problem with this, is that __builtin_unreachable actually exposes
optimization opportunities: more const/copy props of "implicit sets"
in the predicate guarding the __builtin_unreachable call, more
optimistic value numbering, etc. It also helps improve maybe-unused
warnings accuracy. So simply removing these "dead ends" in the CFG is
probably not a good idea.
?!? By removing the empty unreachable block that's precisely what you
enable. The block itself goes away and the branch leading to the block
is simplified appropriately. That in turn will create larger basic
blocks, enabling the const/copy propagations and similar optimizations.
Finally removing unreachable paths was insired by the desire to
eliminate false positives from maybe-{unused,uninitialized} and similar
warnings.
I'd be very curious to see the conditions under which removing the empty
unreachable and appropriate cleanup of the CFG (including the
underlying control statement) results in less optimizations and less
precision in our may-warnings.
I think there is one important difference: In the thread you referred
to, you're removing paths in the CFG that are implicitly not
executable (for some languages anyway), whereas a
__builtin_unreachable call is an explicit marker for "this can never
happen". I think this difference is important:
I don't see this as an important distinction. The difference is in the
implicit "can't happen" case, eliminating the "can't happen" path may
eliminate a user visible side effect (for example a segfault).
* The explicit marker may have been put there on purpose (e.g. to get
rid of a false-positive warning). The compiler should respect that. An
implicit unreachable path can be optimized away without regard for the
user's intent.
Right, but if done right, eliminating the unreachable path correctly
should eliminate false positive warnings.
* The explicit marker should not inhibit optimizations. For an
implicit unreachable path the compiler should be conservative. But for
a __builtin_unreachable call that is the only statement in a basic
block, the compiler should be allowed to work as if the block really
is never reached.
Right.
The attached patch implements these ideas. During a tree-CFG cleanup,
basic blocks containing only a __builtin_unreachable call are marked
with a new flag BB_NEVER_REACHED. The flag is used in post-dominance:
A marked block is not considered in the computations of the immediate
post-dominator of the predecessor blocks. The result is a more
optimistic post-dominance tree: Without the patch all predecessors of
these BB_NEVER_REACHED blocks have the EXIT block as their
post-dominator, but with the patch this non-executable path in the CFG
is ignored and the post-dominators are those that would result if the
BB_NEVER_REACHED blocks are not there at all (the BB_NEVER_REACHED
blocks themselves are only post-dominated by the EXIT block).
I've also added a control dependence calculation function. It's not
currently used, but it shows how the BB_NEVER_REACHED flag is used in
this case to avoid the "false" control dependences that I showed in
the graphs in http://gcc.gnu.org/ml/gcc/2013-01/msg00021.html.
Bootstrapped&tested on powerpc64-unknown-linux-gnu.
What do you think of this approach?
Before diving into the patch I think we should figure out why we see
such different effects of eliminating these paths from the CFG. Your
assertion is eliminating them will result in more false positives and
less optimization while my assertion is the opposite.
Jeff
