[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...

[Gerolf Hoflehner via llvm-commits]

Gerolf created this revision.
Gerolf added a reviewer: Gerolf.
Gerolf added a subscriber: llvm-commits.

...below.

The proposed patch fixes the problem by eliminating *all* blocks dominated by a
block that has become unreachable. This requires dominance information. The
upside is that using dominance will also allow cleaning up code in SimplifyCFG
that computes “local” dominance eg. DominatesMergePoint(). The potential
downside is an increase in compile time, which I’m still collecting data on.
Anecdotally I hear the argument that computing dominance is expensive. I’m
curious if anyone has specifics about this Theoretically dominance is computed
by a linear run O(CFG), except when the CFG is irreducible (which should be
rare). In that case Tarjan gives an almost linear algorithm. So I would not
expect a material compile-time impact from it.

The patch also externs SimplifyCFG to indicate when more than one block is
delated, so the caller can take care of the stale iterator problem when
necessary.

More details:

The actual endless loop is in the constant compare gather() routine in
Utils/SimplifyCFG.cpp. The same value ret.0.off0.i is pushed back into the
queue:
%.ret.0.off0.i = or i1 %.ret.0.off0.i, %cmp10.i

Here is what happens at the IR level:

for.cond.i:                                       ; preds = %if.end6.i,
%if.end.i54
 %ix.0.i = phi i32 [ 0, %if.end.i54 ], [ %inc.i55, %if.end6.i ]
 %ret.0.off0.i = phi i1 [false, %if.end.i54], [%.ret.0.off0.i, %if.end6.i] <<<
 %cmp2.i = icmp ult i32 %ix.0.i, %11
 br i1 %cmp2.i, label %for.body.i, label %LBJ_TmpSimpleNeedExt.exit

if.end6.i:                                        ; preds = %for.body.i
 %cmp10.i = icmp ugt i32 %conv.i, %add9.i
 %.ret.0.off0.i = or i1 %ret.0.off0.i, %cmp10.i <<<

When if.end.i54 gets eliminated which removes the definition of ret.0.off0.i.
The result is the expression %.ret.0.off0.i = or i1 %.ret.0.off0.i, %cmp10.i
(Note the first ‘or’ operand is now %.ret.0.off0.i, and *NOT* %ret.0.off0.i). And
now there is use of .ret.0.off0.i before a definition which triggers the
“endless” loop in gather():

while(!DFT.empty()) {

     V = DFT.pop_back_val();   // V is .ret.0.off0.i

     if (Instruction *I = dyn_cast<Instruction>(V)) {
       // If it is a || (or && depending on isEQ), process the operands.
       if (I->getOpcode() == (isEQ ? Instruction::Or : Instruction::And)) {
         DFT.push_back(I->getOperand(1));  // This is now .ret.0.off0.i also
         DFT.push_back(I->getOperand(0));

         continue; // “endless loop” for .ret.0.off0.i
       }

[SimplifyCFG] Fix for "endless" loop after dead code removal

http://reviews.llvm.org/D16251

Files:
  include/llvm/Transforms/Utils/Local.h
  lib/Transforms/Scalar/SimplifyCFGPass.cpp
  lib/Transforms/Utils/SimplifyCFG.cpp
  test/Transforms/SimplifyCFG/InfLoop.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D16251.45063.patch
Type: text/x-patch
Size: 27867 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160116/0092aef4/attachment.bin>