[llvm-commits] [PATCH] Add single entry / single exit accessors.

[Tobias Grosser]

On 01/13/2011 06:27 AM, Andreas Simbuerger wrote:
> Hi,
>
> this time from the correct mail-address;-)
> This patch adds support for single entry / single exit edge
> accessors to regions.

Thanks, Andreas. Just some small comments inline.

> A (refined) region that has only 2 transition edges (1 entry, 1 exit)
> to its parent region is defined as a simple region.
>
> Both methods added, return null if there is more than one entry/exit edge.
>
> Cheers,
> Andreas
>
>
>
> 0001-Add-single-entry-single-exit-accessors.patch
>
>
> diff --git include/llvm/Analysis/RegionInfo.h include/llvm/Analysis/RegionInfo.h
> index 737d46c..4c06ec0 100644
> --- include/llvm/Analysis/RegionInfo.h
> +++ include/llvm/Analysis/RegionInfo.h
> @@ -305,6 +305,20 @@ public:
>     ///         NULL if such a basic block does not exist.
>     Region *getExpandedRegion() const;
>
> +  /// @brief Return the first block of this region's single entry edge,
> +  ///        if existing.
> +  ///
> +  /// @return The BasicBlock starting this region's single entry edge,
> +  ///         else NULL.
> +  BasicBlock *getSingleEntryBlock() const;
I would call this getEnteringBlock() following LoopInfo in this case.


> +
> +  /// @brief Return the first block of this region's single exit edge,
> +  ///        if existing.
> +  ///
> +  /// @return The BasicBlock starting this region's single exit edge,
> +  ///         else NULL.
> +  BasicBlock *getSingleExitBlock() const;
getExitingBlock() would follow LoopInfo probably.


>     /// @brief Is this a simple region?
>     ///
>     /// A region is simple if it has exactly one exit and one entry edge.
> diff --git lib/Analysis/RegionInfo.cpp lib/Analysis/RegionInfo.cpp
> index cf48a71..22e822b 100644
> --- lib/Analysis/RegionInfo.cpp
> +++ lib/Analysis/RegionInfo.cpp
> @@ -131,43 +131,55 @@ Loop *Region::outermostLoopInRegion(Loop *L) const {
>   Loop*Region::outermostLoopInRegion(LoopInfo *LI, BasicBlock*  BB) const {
>     assert(LI&&  BB&&  "LI and BB cannot be null!");
>     Loop *L = LI->getLoopFor(BB);
> +
>     return outermostLoopInRegion(L);
>   }
>
> -bool Region::isSimple() const {
> -  bool isSimple = true;
> -  bool found = false;
> -
> -  BasicBlock *entry = getEntry(), *exit = getExit();
> -
> -  if (isTopLevelRegion())
> -    return false;
> +BasicBlock *Region::getSingleEntryBlock() const {
> +  BasicBlock *entry = getEntry();
> +  BasicBlock *Pred;
> +  BasicBlock *ee = 0;
>
>     for (pred_iterator PI = pred_begin(entry), PE = pred_end(entry); PI != PE;
>          ++PI) {
> -    BasicBlock *Pred = *PI;
> +    Pred = *PI;
>       if (DT->getNode(Pred)&&  !contains(Pred)) {
> -      if (found) {
> -        isSimple = false;
> -        break;
> -      }
> -      found = true;
> +      if (ee)
> +        return 0;
> +
> +      ee = Pred;
Can you combine the two if conditions?

>       }
>     }
>
> -  found = false;
> +  return ee;
> +}
Maybe use a more expressive variable name instead of 'ee'. Maybe 
something like 'enteringBlock'?

> +BasicBlock *Region::getSingleExitBlock() const {
> +  BasicBlock *exit = getExit();
> +  BasicBlock *Pred;
> +  BasicBlock *ee = 0;
> +
> +  if (!exit)
> +    return 0;
>
>     for (pred_iterator PI = pred_begin(exit), PE = pred_end(exit); PI != PE;
> -       ++PI)
> -    if (contains(*PI)) {
> -      if (found) {
> -        isSimple = false;
> -        break;
> -      }
> -      found = true;
> +       ++PI) {
> +    Pred = *PI;
> +    if (contains(Pred)) {
> +      if (ee)
> +        return 0;
> +
> +      ee = Pred;
>       }
> +  }
>
> -  return isSimple;
> +  return ee;
> +}
Same comments apply here.


> +bool Region::isSimple() const {
> +  return !(isTopLevelRegion())
> +&&  (getSingleEntryBlock() != 0)
> +&&  (getSingleExitBlock() != 0);
I am not sure if my email has removed some spaces/tabs. But please 
recheck indentation.

Thanks

Tobi