[llvm-commits] [llvm] r137237 - in /llvm/trunk: include/llvm/CodeGen/LexicalScopes.h lib/CodeGen/CMakeLists.txt lib/CodeGen/LexicalScopes.cpp

[Jakob Stoklund Olesen]

On Aug 10, 2011, at 12:04 PM, Devang Patel wrote:

> Author: dpatel
> Date: Wed Aug 10 14:04:06 2011
> New Revision: 137237
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=137237&view=rev
> Log:
> Provide utility to extract and use lexical scoping information from machine instructions.
> 
> Added:
>    llvm/trunk/include/llvm/CodeGen/LexicalScopes.h
>    llvm/trunk/lib/CodeGen/LexicalScopes.cpp
> Modified:
>    llvm/trunk/lib/CodeGen/CMakeLists.txt
> 
> Added: llvm/trunk/include/llvm/CodeGen/LexicalScopes.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/LexicalScopes.h?rev=137237&view=auto
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/LexicalScopes.h (added)
> +++ llvm/trunk/include/llvm/CodeGen/LexicalScopes.h Wed Aug 10 14:04:06 2011

> +  /// getMachineBasicBlocks - Populate given set using machine basic blocks which
> +  /// have machine instructions that belong to lexical scope identified by 
> +  /// DebugLoc.
> +  void getMachineBasicBlocks(DebugLoc DL,
> +                             SmallPtrSet<const MachineBasicBlock*, 4> &MBBs);
> +
> +  /// dominates - Return true if DebugLoc's lexical scope dominates at least one
> +  /// machine instruction's lexical scope in a given machine basic block.
> +  bool dominates(DebugLoc DL, MachineBasicBlock *MBB);

These two functions are potentially expensive for the common case where the lexical scope is the function being compiled.

How about adding a fast path for that?

An example of how the interface is supposed to be used would be nice.

> +  /// getAbstractScopesList - Return a reference to list of abstract scopes.
> +  SmallVector<LexicalScope *, 4> &getAbstractScopesList() {
> +    return AbstractScopesList;
> +  }

Is this supposed to return a mutable vector? If not, please use ArrayRef.

> +//===----------------------------------------------------------------------===//
> +/// LexicalScope - This class is used to track scope information.
> +///
> +class LexicalScope {
> +
> 
> +  /// dominates - Return true if current scope dominsates given lexical scope.
> +  bool dominates(const LexicalScope *S) {
> +    if (S == this)
> +      return true;
> +    if (DFSIn < S->getDFSIn() && DFSOut > S->getDFSOut())
> +      return true;
> +    return false;
> +  }

Should this be a const function?

The public interface on LexicalScope is a bit confusing. It seems that dominates() is the only function that was supposed to be public.

How about hiding the entire class from the public interface? You could provide LexicalScopes::dominates(A, B) instead, just like Dominators does.

/jakob