[LLVMdev] Erasing dead blocks

[Duncan Sands]

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.