[PATCH] D23824: [ADCE] Add handling of PHI nodes when removing control flow

[Daniel Berlin via llvm-commits]

This is .. not quite right.
It marks terminators of predecessor edges live, but that's not what you
should do. You should mark terminators of CD(predecessors) as live.
See keith cooper's slides  at
https://www.clear.rice.edu/comp512/Lectures/10Dead-Clean-SCCP.pdf

You can also find the same thing in the original SSA paper
http://compilers.cs.uni-saarland.de/teaching/cc/2013/papers/p451-cytron.pdf
on page 480

On Wed, Sep 7, 2016 at 7:13 PM, David Callahan <dcallahan at fb.com> wrote:

> david2050 updated this revision to Diff 70635.
> david2050 added a comment.
>
> Change handling of PHI nodes to force predecessors live
>
>
> https://reviews.llvm.org/D23824
>
> Files:
>   lib/Transforms/Scalar/ADCE.cpp
>
> Index: lib/Transforms/Scalar/ADCE.cpp
> ===================================================================
> --- lib/Transforms/Scalar/ADCE.cpp
> +++ lib/Transforms/Scalar/ADCE.cpp
> @@ -58,6 +58,8 @@
>    bool Live = false;
>    /// True when this block ends in an unconditional branch.
>    bool UnconditionalBranch = false;
> +  /// True when this block is known to have live PHI nodes.
> +  bool HasLivePhiNodes = false;
>
>    /// Quick access to the LiveInfo for the terminator,
>    /// holds the value &InstInfo[Terminator]
> @@ -109,6 +111,9 @@
>    void markLiveInstructions();
>    /// Mark an instruction as live.
>    void markLive(Instruction *I);
> +
> +  /// Mark terminators of control predecessors of a PHI node live.
> +  void markPhiLive(PHINode *PN);
>
>    /// Record the Debug Scopes which surround live debug information.
>    void collectLiveScopes(const DILocalScope &LS);
> @@ -269,15 +274,18 @@
>      // where we need to mark the inputs as live.
>      while (!Worklist.empty()) {
>        Instruction *LiveInst = Worklist.pop_back_val();
> +      DEBUG(dbgs() << "work live: "; LiveInst->dump(););
>
>        // Collect the live debug info scopes attached to this instruction.
>        if (const DILocation *DL = LiveInst->getDebugLoc())
>          collectLiveScopes(*DL);
>
> -      DEBUG(dbgs() << "work live: "; LiveInst->dump(););
>        for (Use &OI : LiveInst->operands())
>          if (Instruction *Inst = dyn_cast<Instruction>(OI))
>            markLive(Inst);
> +
> +      if (auto *PN = dyn_cast<PHINode>(LiveInst))
> +        markPhiLive(PN);
>      }
>      markLiveBranchesFromControlDependences();
>
> @@ -348,6 +356,18 @@
>      collectLiveScopes(*IA);
>  }
>
> +void AggressiveDeadCodeElimination::markPhiLive(llvm::PHINode *PN) {
> +  auto &Info = BlockInfo[PN->getParent()];
> +  // Only need to check this once per block.
> +  if(Info.HasLivePhiNodes)
> +    return;
> +  Info.HasLivePhiNodes = true;
> +
> +  // Mark terminators of all predecessors live.
> +  for (auto *PredBB : predecessors(Info.BB))
> +    markLive(PredBB->getTerminator());
> +}
> +
>  void AggressiveDeadCodeElimination::markLiveBranchesFromControlDependences()
> {
>
>    if (BlocksWithDeadTerminators.empty())
> @@ -382,6 +402,11 @@
>    }
>  }
>
> +//===------------------------------------------------------
> ----------------===//
> +//
> +//  Routines to update the CFG and SSA information before removing dead
> code.
> +//
> +//===------------------------------------------------------
> ----------------===//
>  bool AggressiveDeadCodeElimination::removeDeadInstructions() {
>
>    // The inverse of the live set is the dead set.  These are those
> instructions
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160907/b1fa2342/attachment.html>