[PATCH] D27264: Refactor code to create getFallThrough method in MachineBasicBlock.

[Duncan P. N. Exon Smith via llvm-commits]

> On 2016-Nov-30, at 09:38, Jan Sjödin via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> jsjodin created this revision.
> jsjodin added reviewers: asl, dexonsmith, tstellarAMD, arsenm.
> jsjodin added a subscriber: llvm-commits.
> Herald added a subscriber: wdng.
> 
> This will be needed by the MachineCFGStructurizer.
> 
> 
> https://reviews.llvm.org/D27264
> 
> Files:
>  include/llvm/CodeGen/MachineBasicBlock.h
>  lib/CodeGen/MachineBasicBlock.cpp
> 
> 
> Index: lib/CodeGen/MachineBasicBlock.cpp
> ===================================================================
> --- lib/CodeGen/MachineBasicBlock.cpp
> +++ lib/CodeGen/MachineBasicBlock.cpp
> 
> @@ -705,25 +705,26 @@
>     // is possible. The isPredicated check is needed because this code can be
>     // called during IfConversion, where an instruction which is normally a
>     // Barrier is predicated and thus no longer an actual control barrier.
> -    return empty() || !back().isBarrier() || TII->isPredicated(back());
> +    return (empty() || !back().isBarrier() || TII->isPredicated(back())) ?
> +      &*Fallthrough : nullptr;

IMO, the redundant parens don't add any clarity.  However, clang-format adds some:

    return empty() || !back().isBarrier() || TII->isPredicated(back())
               ? &*Fallthrough
               : nullptr;

http://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting

>   }
> 
>   // If there is no branch, control always falls through.
> -  if (!TBB) return true;
> +  if (!TBB) return &*Fallthrough;
> 
>   // If there is some explicit branch to the fallthrough block, it can obviously
>   // reach, even though the branch should get folded to fall through implicitly.
>   if (MachineFunction::iterator(TBB) == Fallthrough ||
>       MachineFunction::iterator(FBB) == Fallthrough)
> -    return true;
> +    return &*Fallthrough;
> 
>   // If it's an unconditional branch to some block not the fall through, it
>   // doesn't fall through.
> -  if (Cond.empty()) return false;
> +  if (Cond.empty()) return nullptr;
> 
>   // Otherwise, if it is conditional and has no explicit false block, it falls
>   // through.
> -  return FBB == nullptr;
> +  return (FBB == nullptr) ? &*Fallthrough : nullptr;

This seems easier to follow:

    return FBB ? nullptr : &*Fallthrough;

> }
> 
> MachineBasicBlock *MachineBasicBlock::SplitCriticalEdge(MachineBasicBlock *Succ,
> Index: include/llvm/CodeGen/MachineBasicBlock.h
> ===================================================================
> --- include/llvm/CodeGen/MachineBasicBlock.h
> +++ include/llvm/CodeGen/MachineBasicBlock.h
> @@ -441,11 +441,18 @@
>   /// other block.
>   bool isLayoutSuccessor(const MachineBasicBlock *MBB) const;
> 
> +  /// Return the fallthrough block if the block can implicitly
> +  /// transfer control to the block after it by falling off the end of
> +  /// it.  This should return null if it can reach the block after
> +  /// it, but it uses an explicit branch to do so (e.g., a table
> +  /// jump).  Non-null return  is a conservative answer.
> +  MachineBasicBlock *getFallThrough();
> +
>   /// Return true if the block can implicitly transfer control to the block
>   /// after it by falling off the end of it.  This should return false if it can
>   /// reach the block after it, but it uses an explicit branch to do so (e.g., a
>   /// table jump).  True is a conservative answer.
> -  bool canFallThrough();
> +  bool canFallThrough() {return getFallThrough() != nullptr;}

I suggest running `clang-format-diff` on your patch to clean up the whitespace in your patch (see the link from earlier).  But there should be spaces inside the {}.

> 
>   /// Returns a pointer to the first instruction in this block that is not a
>   /// PHINode instruction. When adding instructions to the beginning of the
> 
> 
> <D27264.79766.patch>