llvm dev - Jan 2011 - [LLVMdev] Erasing dead blocks

Dear LLVM developers,

I have a question regarding the IPSCCP class and the handling of dead blocks:

The file lib/Transforms/Scalar/SCCP.cpp of llvm 2.8 from contains some
code to deal with a dead block which could not be folded (line
1909ff). This happens when a user of this dead block is a branch or
switch with an undef condition. The action taken than is to replace
this terminator with an unconditional branch to the first successor of
this undef branch/switch.
I do not see how this ensures that the dead block is disconnected from
the control flow graph. If the dead block is the first successor of
the switch resp. branch the dead block will be still reachable after
this code. Am I wrong here? Is there somewhere else code ensuring that
the dead block is not the first successor of an undef branch/switch
instruction?
Thanks for your support.

Kind regards,
Gordon Haak

Hi Gordan,
> I have a question regarding the IPSCCP class and the handling of dead
blocks:
>
> The file lib/Transforms/Scalar/SCCP.cpp of llvm 2.8 from contains some
> code to deal with a dead block which could not be folded (line
> 1909ff). This happens when a user of this dead block is a branch or
> switch with an undef condition. The action taken than is to replace
> this terminator with an unconditional branch to the first successor of
> this undef branch/switch.
> I do not see how this ensures that the dead block is disconnected from
> the control flow graph. If the dead block is the first successor of
> the switch resp. branch the dead block will be still reachable after
> this code. Am I wrong here? Is there somewhere else code ensuring that
> the dead block is not the first successor of an undef branch/switch
> instruction?
I don't know much about SCCP, but I noticed that here a branch or switch on
undef causes SCCP to choose the *second* successor:

     // Check to see if we have a branch or switch on an undefined value.  If so
     // we force the branch to go one way or the other to make the successor
     // values live.  It doesn't really matter which way we force it.
     TerminatorInst *TI = BB->getTerminator();
     if (BranchInst *BI = dyn_cast<BranchInst>(TI)) {
       if (!BI->isConditional()) continue;
       if (!getValueState(BI->getCondition()).isUndefined())
         continue;

       // If the input to SCCP is actually branch on undef, fix the undef to
       // false.
       if (isa<UndefValue>(BI->getCondition())) {
         BI->setCondition(ConstantInt::getFalse(BI->getContext()));
         markEdgeExecutable(BB, TI->getSuccessor(1));
         return true;
       }
...

So in order to be consistent I would have expected the dead block erasing code
to also choose the second successor.  It looks like a bug to me.

Ciao, Duncan.