[llvm-commits] [PATCH] Add SeSeRegionInfo transform pass

[Tobias Grosser]

On 01/14/2011 07:06 AM, Andreas Simbuerger wrote:
> Hi,
>
> this patch adds a small transform pass that converts a refined region
> into a simple region by adding the required edges at the region entry /
> exit.

Thanks Andreas,

> Basically it should update the RegionInfo analysis correctly, but
> sometimes it fails to do so.
> This requires some work on the RegionInfo::splitBlock methods.
> I'll try to figure it out and send a patch for that at a later time.
 >
> Test cases will follow in a separate patch.

Here my review. Let's commit it as soon as splitBlock() is enhanced and
we have got some test cases.

> 0001-Add-SeSeRegionInfo-transform-pass.patch
>
>
>  From cf305190c65a72505408d663b0e4bf01bb7afa74 Mon Sep 17 00:00:00 2001
> From: Andreas Simbuerger<simbuerg at fim.uni-passau.de>
> Date: Fri, 14 Jan 2011 10:01:19 +0100
> Subject: [PATCH] Add SeSeRegionInfo transform pass.

As this is a transformation pass, I believe calling it "Info" is 
misleading. What about "SimplifyRegions" or "SimpleRegions"?

> This adds a single entry / single exit edge transformation pass.
> The RegionInfo analysis constructs a region tree that contains
> of 2 types of regions:
>   * Refined regions
>   * Simple regions
>
> While simple regions only have two transitions inside
> (1 entry, 1 exit), refined regions don't have that property.
> However, refined regions can be transformed into simple regions
> by merging multiple edges into a new split BasicBlock and forming
> a new edge.
> ---
>   include/llvm/InitializePasses.h          |    1 +
>   include/llvm/LinkAllPasses.h             |    1 +
>   include/llvm/Transforms/Scalar.h         |    6 +
>   lib/Transforms/Scalar/CMakeLists.txt     |    1 +
>   lib/Transforms/Scalar/Scalar.cpp         |    1 +
>   lib/Transforms/Scalar/SeSeRegionInfo.cpp |  167 ++++++++++++++++++++++++++++++
>   6 files changed, 177 insertions(+), 0 deletions(-)
>   create mode 100644 lib/Transforms/Scalar/SeSeRegionInfo.cpp
>
> diff --git a/include/llvm/InitializePasses.h b/include/llvm/InitializePasses.h
> index d86b199..a50db8d 100644
> --- a/include/llvm/InitializePasses.h
> +++ b/include/llvm/InitializePasses.h
> @@ -197,6 +197,7 @@ void initializeSROA_DFPass(PassRegistry&);
>   void initializeSROA_SSAUpPass(PassRegistry&);
>   void initializeScalarEvolutionAliasAnalysisPass(PassRegistry&);
>   void initializeScalarEvolutionPass(PassRegistry&);
> +void initializeSeSeRegionInfoPass(PassRegistry&);
>   void initializeSimpleInlinerPass(PassRegistry&);
>   void initializeSimpleRegisterCoalescingPass(PassRegistry&);
>   void initializeSimplifyHalfPowrLibCallsPass(PassRegistry&);
> diff --git a/include/llvm/LinkAllPasses.h b/include/llvm/LinkAllPasses.h
> index 7dd8ebd..bb7a68a 100644
> --- a/include/llvm/LinkAllPasses.h
> +++ b/include/llvm/LinkAllPasses.h
> @@ -115,6 +115,7 @@ namespace {
>         (void) llvm::createRegionViewerPass();
>         (void) llvm::createSCCPPass();
>         (void) llvm::createScalarReplAggregatesPass();
> +      (void) llvm::createSeSeRegionInfoPass();
>         (void) llvm::createSimplifyLibCallsPass();
>         (void) llvm::createSimplifyHalfPowrLibCallsPass();
>         (void) llvm::createSingleLoopExtractorPass();
> diff --git a/include/llvm/Transforms/Scalar.h b/include/llvm/Transforms/Scalar.h
> index 9a87eab..6c63eb8 100644
> --- a/include/llvm/Transforms/Scalar.h
> +++ b/include/llvm/Transforms/Scalar.h
> @@ -349,6 +349,12 @@ Pass *createCorrelatedValuePropagationPass();
>   FunctionPass *createInstructionSimplifierPass();
>   extern char&InstructionSimplifierID;
>
> +//===----------------------------------------------------------------------===//
> +//
> +// SeSeRegionInfo - Convert refined regions to simple regions.
> +//
> +Pass *createSeSeRegionInfoPass();
> +
>   } // End llvm namespace
>
>   #endif
> diff --git a/lib/Transforms/Scalar/CMakeLists.txt b/lib/Transforms/Scalar/CMakeLists.txt
> index 106fb8f..836c22c 100644
> --- a/lib/Transforms/Scalar/CMakeLists.txt
> +++ b/lib/Transforms/Scalar/CMakeLists.txt
> @@ -26,6 +26,7 @@ add_llvm_library(LLVMScalarOpts
>     SCCP.cpp
>     Scalar.cpp
>     ScalarReplAggregates.cpp
> +  SeSeRegionInfo.cpp
>     SimplifyCFGPass.cpp
>     SimplifyHalfPowrLibCalls.cpp
>     SimplifyLibCalls.cpp
> diff --git a/lib/Transforms/Scalar/Scalar.cpp b/lib/Transforms/Scalar/Scalar.cpp
> index 1d0ca7d..7ab83b9 100644
> --- a/lib/Transforms/Scalar/Scalar.cpp
> +++ b/lib/Transforms/Scalar/Scalar.cpp
> @@ -53,6 +53,7 @@ void llvm::initializeScalarOpts(PassRegistry&Registry) {
>     initializeRegToMemPass(Registry);
>     initializeSCCPPass(Registry);
>     initializeIPSCCPPass(Registry);
> +  initializeSeSeRegionInfoPass(Registry);
>     initializeSROA_DFPass(Registry);
>     initializeSROA_SSAUpPass(Registry);
>     initializeCFGSimplifyPassPass(Registry);
> diff --git a/lib/Transforms/Scalar/SeSeRegionInfo.cpp b/lib/Transforms/Scalar/SeSeRegionInfo.cpp
> new file mode 100644
> index 0000000..6005e64
> --- /dev/null
> +++ b/lib/Transforms/Scalar/SeSeRegionInfo.cpp
> @@ -0,0 +1,167 @@
> +//===- SeSeRegionInfo.cpp -------------------------------------------------===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
> +//===----------------------------------------------------------------------===//
> +//
> +// This file converts refined regions detected by the RegionInfo analysis
> +// into simple regions.
> +//
> +//===----------------------------------------------------------------------===//
> +
> +#include "llvm/Instructions.h"
> +#include "llvm/ADT/Statistic.h"
> +#include "llvm/Analysis/RegionPass.h"
> +#include "llvm/Analysis/RegionInfo.h"
> +#include "llvm/Transforms/Utils/BasicBlockUtils.h"
> +
> +#define DEBUG_TYPE "sese-regions"
> +
> +using namespace llvm;
> +
> +STATISTIC(NumEntries, "The # of created entry edges");
> +STATISTIC(NumExits, "The # of created exit edges");
> +
> +namespace {
> +  class SeSeRegionInfo : public RegionPass {
> +    bool modified;
> +    Region *CR;
> +    BasicBlock *createSingleEntryEdge(Region *R);
> +    BasicBlock *createSingleExitEdge(Region *R);
> +  public:
> +    static char ID;
> +    explicit SeSeRegionInfo() : RegionPass(ID) {
> +      initializeSeSeRegionInfoPass(*PassRegistry::getPassRegistry());
> +    }
> +
> +    virtual void print(raw_ostream&O, const Module *M) const;
> +
> +    virtual bool runOnRegion(Region *R, RGPassManager&RGM);
> +    virtual void getAnalysisUsage(AnalysisUsage&AU) const;
> +  };
> +}
> +
> +INITIALIZE_PASS(SeSeRegionInfo, "sese-regions",
I believe sese is difficult to understand. Can we use simple-regions for 
this?

> +                "Transform refined regions into simple regions", false, false)
> +
> +char SeSeRegionInfo::ID = 0;
> +namespace llvm {
> +  Pass *createSeSeRegionInfoPass() {
> +    return new SeSeRegionInfo();
> +  }
> +}
> +
> +void SeSeRegionInfo::print(raw_ostream&O, const Module *M) const {
> +  BasicBlock *enteringBlock;
> +  BasicBlock *exitingBlock;
> +
> +  if (modified) {
> +    enteringBlock = CR->getEnteringBlock();
> +    exitingBlock = CR->getExitingBlock();
> +
> +    O<<  "\nRegion: ["<<  CR->getNameStr()<<  "] Edges:\n";
> +    if (enteringBlock)
> +      O<<  "  Entry: ]"
> +<<  enteringBlock->getNameStr()
> +<<  " =>  "
> +<<  enteringBlock->getNameStr()<<  "]\n";
> +    if (exitingBlock)
> +      O<<  "  Exit: ["
> +<<  exitingBlock->getNameStr()
> +<<  " =>  "
> +<<  exitingBlock->getNameStr()<<  "[\n";
> +
> +    O<<  "\n";
> +  }
> +}
> +
> +void SeSeRegionInfo::getAnalysisUsage(AnalysisUsage&AU) const {
> +  AU.setPreservesAll();
Is this correct?

> +  AU.addRequired<RegionInfo>();
> +}
> +
> +// createSingleEntryEdge - Split the entry BasicBlock of the given
> +// region after the last PHINode to form a single entry edge.
> +// This does not update RegionInfo analysis.
> +BasicBlock *SeSeRegionInfo::createSingleEntryEdge(Region *R) {
> +  BasicBlock *BB = R->getEntry();
> +  BasicBlock::iterator SplitIt = BB->begin();
> +
> +  while (isa<PHINode>(SplitIt))
> +    ++SplitIt;
> +
> +  BasicBlock *newBB = SplitBlock(BB, SplitIt, this);
> +
> +  for (BasicBlock::iterator PI = BB->begin(); isa<PHINode>(PI); ++PI) {
> +    PHINode *PN = cast<PHINode>(PI);
> +    PHINode *NPN =
> +      PHINode::Create(PN->getType(), PN->getName()+".ph", newBB->begin());
> +
> +    for (pred_iterator PI = pred_begin(BB), PE = pred_end(BB); PI != PE; ++PI) {
> +      if (R->contains(*PI)) {
> +        Value *V = PN->removeIncomingValue(*PI, false);
> +        NPN->addIncoming(V, *PI);
> +      }
> +    }
You can get rid of a set of parenthesis.

> +    PN->replaceAllUsesWith(NPN);
> +    NPN->addIncoming(PN,BB);
> +  }
> +
> +  for (pred_iterator PI = pred_begin(BB), PE = pred_end(BB); PI != PE; ++PI)
> +    if (R->contains(*PI))
> +      (*PI)->getTerminator()->replaceUsesOfWith(BB, newBB);
> +
> +  return newBB;
> +}
> +
> +// createSingleExitEdge - Split the exit BasicBlock of the given region
> +// to form a single exit edge.
> +// This does not update RegionInfo analysis.
> +BasicBlock *SeSeRegionInfo::createSingleExitEdge(Region *R) {
> +  BasicBlock *BB = R->getExit();
> +
> +  SmallVector<BasicBlock*, 4>  Preds;
> +  for (pred_iterator PI = pred_begin(BB), PE = pred_end(BB); PI != PE; ++PI)
> +    if (R->contains(*PI))
> +      Preds.push_back(*PI);
> +
> +  return SplitBlockPredecessors(BB, Preds.data(), Preds.size(), ".region", this);
> +}
> +
> +bool SeSeRegionInfo::runOnRegion(Region *R, RGPassManager&RGM) {
> +  RegionInfo *RI =&getAnalysis<RegionInfo>();
> +  modified = false;
> +
> +  CR = R;
> +  if (!R->isTopLevelRegion()) {
> +    BasicBlock *nBB;
> +    BasicBlock *oBB;
Can you use more descriptive variable names?

> +
> +    BasicBlock *enteringBlock = R->getEnteringBlock();
> +    if (!enteringBlock) {
You can probably move the R->getEnteringBlock() into the if condition. 
enteringBlock is not used anywhere else.

> +      oBB = R->getEntry();
> +      nBB = createSingleEntryEdge(R);
> +
> +      RI->splitBlock(nBB, oBB);
> +
> +      modified |= true;
> +      ++NumEntries;
> +    }
> +
> +    BasicBlock *exitingBlock = R->getExitingBlock();
> +    if (!exitingBlock) {
Dito.

> +      oBB = R->getExit();
> +      nBB = createSingleExitEdge(R);
> +
> +      RI->splitBlock(nBB, oBB);
> +
> +      modified |= true;
> +      ++NumExits;
> +    }
> +  }
> +
> +  return modified;
> +}
> -- 1.7.1