Re: [PATCH] D16251: There seems to be a fundamental problem in SimplifyCFG: Dead code removal can result inuninitialized variables. The impact is an “endless” loop which can be consideredthe consequence of searching for the initialization. More details are...

Wed Jan 27 20:51:58 PST 2016

Gerolf added a comment.

Philip, the code does not claim it preserves the dominator tree. Nor does it need to.

The reason why I think the code is correct although SImplifyCFG may modify the cfg is that
a) blocks only get removed when dominance has been computed for them
b) any block that is dominated by an unreachable block can be removed also (by definition)
c) if there is an new block introduce that is dominated by an unreachable block but dominance is missing the unreachable block will still be removed. Then the new block will be removed later in the SimplifyCFGPass loop.. So there is no change from the behavior of the existing code.

The only problematic scenario I can think of is that a) a new block is inserted and b) a block with an initialization becomes unreachable and is removed. Clearly there is no dominance info and thus a block with the PHI node (for.cond.i in the example) would not be removed which could then result in that endless loop. But in this scenario there needs to be a definition from the new block which then avoids the issue.

I was mostly concerned about compile-time, but my data from the LLVM test suite and benchmark (SPEC 2000, 2006 etc) also shows a few small improvements (likely in the noise) on x86 O3:
Performance Improvements - Execution Time	Δ	Previous	Current	σ
SingleSource/Benchmarks/Polybench/linear-algebra/kernels/syrk/syrk	-2.69%	2.1296	2.0724	0.0179
MultiSource/Benchmarks/TSVC/CrossingThresholds-dbl/CrossingThresholds-dbl	-2.04%	2.9327	2.8728	0.0222
SingleSource/Benchmarks/Polybench/linear-algebra/kernels/syr2k/syr2k	-1.75%	3.4523	3.3920	0.0173
MultiSource/Benchmarks/TSVC/CrossingThresholds-flt/CrossingThresholds-flt	-1.64%	2.1694	2.1339	0.0083
MultiSource/Benchmarks/TSVC/Packing-dbl/Packing-dbl	-1.56%	2.9577	2.9115	0.0175
SingleSource/Benchmarks/Misc-C++/Large/ray	-1.18%	1.7727	1.7517	0.0009

Could you give this patch a second thought? Thanks!

http://reviews.llvm.org/D16251