[llvm] r208364 - Reapply r207876 (Try simplifying LexicalScopes ownership again) including a workaround for an MSVC2012 bug regarding forward_as_tuple

David Blaikie dblaikie at gmail.com
Thu May 8 15:24:52 PDT 2014


Author: dblaikie
Date: Thu May  8 17:24:51 2014
New Revision: 208364

URL: http://llvm.org/viewvc/llvm-project?rev=208364&view=rev
Log:
Reapply r207876 (Try simplifying LexicalScopes ownership again) including a workaround for an MSVC2012 bug regarding forward_as_tuple

(r207876 was reverted in r208131 after seeing some consistent buildbot
failure for MSVC 2012. The original commits were in r207724-r207726)

Takumi was nice enough to dig into this and locate this Microsoft
Connect issue:
http://connect.microsoft.com/VisualStudio/feedback/details/814899/forward-as-tuple-debug-implementation-error
describing a bug in MSVC2012's forward_as_tuple implementation.

Since the parameters in this instance are trivial/small, pass them by
value (using make_tuple) instead of perfectly-forwarded tuple of rvalue
references (involving the broken forward_as_tuple). Hopefully this will
satisfy MSVC2012.

Modified:
    llvm/trunk/include/llvm/CodeGen/LexicalScopes.h
    llvm/trunk/lib/CodeGen/LexicalScopes.cpp

Modified: llvm/trunk/include/llvm/CodeGen/LexicalScopes.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/LexicalScopes.h?rev=208364&r1=208363&r2=208364&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/LexicalScopes.h (original)
+++ llvm/trunk/include/llvm/CodeGen/LexicalScopes.h Thu May  8 17:24:51 2014
@@ -25,12 +25,12 @@
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/ValueHandle.h"
 #include <utility>
+#include <unordered_map>
 namespace llvm {
 
 class MachineInstr;
 class MachineBasicBlock;
 class MachineFunction;
-class LexicalScope;
 
 //===----------------------------------------------------------------------===//
 /// InsnRange - This is used to track range of instructions with identical
@@ -39,13 +39,103 @@ class LexicalScope;
 typedef std::pair<const MachineInstr *, const MachineInstr *> InsnRange;
 
 //===----------------------------------------------------------------------===//
+/// LexicalScope - This class is used to track scope information.
+///
+class LexicalScope {
+
+public:
+  LexicalScope(LexicalScope *P, const MDNode *D, const MDNode *I, bool A)
+      : Parent(P), Desc(D), InlinedAtLocation(I), AbstractScope(A),
+        LastInsn(nullptr), FirstInsn(nullptr), DFSIn(0), DFSOut(0) {
+    if (Parent)
+      Parent->addChild(this);
+  }
+
+  // Accessors.
+  LexicalScope *getParent() const { return Parent; }
+  const MDNode *getDesc() const { return Desc; }
+  const MDNode *getInlinedAt() const { return InlinedAtLocation; }
+  const MDNode *getScopeNode() const { return Desc; }
+  bool isAbstractScope() const { return AbstractScope; }
+  SmallVectorImpl<LexicalScope *> &getChildren() { return Children; }
+  SmallVectorImpl<InsnRange> &getRanges() { return Ranges; }
+
+  /// addChild - Add a child scope.
+  void addChild(LexicalScope *S) { Children.push_back(S); }
+
+  /// openInsnRange - This scope covers instruction range starting from MI.
+  void openInsnRange(const MachineInstr *MI) {
+    if (!FirstInsn)
+      FirstInsn = MI;
+
+    if (Parent)
+      Parent->openInsnRange(MI);
+  }
+
+  /// extendInsnRange - Extend the current instruction range covered by
+  /// this scope.
+  void extendInsnRange(const MachineInstr *MI) {
+    assert(FirstInsn && "MI Range is not open!");
+    LastInsn = MI;
+    if (Parent)
+      Parent->extendInsnRange(MI);
+  }
+
+  /// closeInsnRange - Create a range based on FirstInsn and LastInsn collected
+  /// until now. This is used when a new scope is encountered while walking
+  /// machine instructions.
+  void closeInsnRange(LexicalScope *NewScope = nullptr) {
+    assert(LastInsn && "Last insn missing!");
+    Ranges.push_back(InsnRange(FirstInsn, LastInsn));
+    FirstInsn = nullptr;
+    LastInsn = nullptr;
+    // If Parent dominates NewScope then do not close Parent's instruction
+    // range.
+    if (Parent && (!NewScope || !Parent->dominates(NewScope)))
+      Parent->closeInsnRange(NewScope);
+  }
+
+  /// dominates - Return true if current scope dominates given lexical scope.
+  bool dominates(const LexicalScope *S) const {
+    if (S == this)
+      return true;
+    if (DFSIn < S->getDFSIn() && DFSOut > S->getDFSOut())
+      return true;
+    return false;
+  }
+
+  // Depth First Search support to walk and manipulate LexicalScope hierarchy.
+  unsigned getDFSOut() const { return DFSOut; }
+  void setDFSOut(unsigned O) { DFSOut = O; }
+  unsigned getDFSIn() const { return DFSIn; }
+  void setDFSIn(unsigned I) { DFSIn = I; }
+
+  /// dump - print lexical scope.
+  void dump(unsigned Indent = 0) const;
+
+private:
+  LexicalScope *Parent;                        // Parent to this scope.
+  AssertingVH<const MDNode> Desc;              // Debug info descriptor.
+  AssertingVH<const MDNode> InlinedAtLocation; // Location at which this
+                                               // scope is inlined.
+  bool AbstractScope;                          // Abstract Scope
+  SmallVector<LexicalScope *, 4> Children;     // Scopes defined in scope.
+                                               // Contents not owned.
+  SmallVector<InsnRange, 4> Ranges;
+
+  const MachineInstr *LastInsn;  // Last instruction of this scope.
+  const MachineInstr *FirstInsn; // First instruction of this scope.
+  unsigned DFSIn, DFSOut;        // In & Out Depth use to determine
+                                 // scope nesting.
+};
+
+//===----------------------------------------------------------------------===//
 /// LexicalScopes -  This class provides interface to collect and use lexical
 /// scoping information from machine instruction.
 ///
 class LexicalScopes {
 public:
   LexicalScopes() : MF(nullptr), CurrentFnLexicalScope(nullptr) {}
-  ~LexicalScopes();
 
   /// initialize - Scan machine function and constuct lexical scope nest, resets
   /// the instance if necessary.
@@ -87,9 +177,10 @@ public:
     return AbstractScopesList;
   }
 
-  /// findAbstractScope - Find an abstract scope or return NULL.
+  /// findAbstractScope - Find an abstract scope or return null.
   LexicalScope *findAbstractScope(const MDNode *N) {
-    return AbstractScopeMap.lookup(N);
+    auto I = AbstractScopeMap.find(N);
+    return I != AbstractScopeMap.end() ? &I->second : nullptr;
   }
 
   /// findInlinedScope - Find an inlined scope for the given DebugLoc or return
@@ -98,9 +189,10 @@ public:
     return InlinedLexicalScopeMap.lookup(DL);
   }
 
-  /// findLexicalScope - Find regular lexical scope or return NULL.
+  /// findLexicalScope - Find regular lexical scope or return null.
   LexicalScope *findLexicalScope(const MDNode *N) {
-    return LexicalScopeMap.lookup(N);
+    auto I = LexicalScopeMap.find(N);
+    return I != LexicalScopeMap.end() ? &I->second : nullptr;
   }
 
   /// dump - Print data structures to dbgs().
@@ -132,17 +224,17 @@ private:
 private:
   const MachineFunction *MF;
 
-  /// LexicalScopeMap - Tracks the scopes in the current function.  Owns the
-  /// contained LexicalScope*s.
-  DenseMap<const MDNode *, LexicalScope *> LexicalScopeMap;
+  /// LexicalScopeMap - Tracks the scopes in the current function.
+  // Use an unordered_map to ensure value pointer validity over insertion.
+  std::unordered_map<const MDNode *, LexicalScope> LexicalScopeMap;
 
   /// InlinedLexicalScopeMap - Tracks inlined function scopes in current
   /// function.
   DenseMap<DebugLoc, LexicalScope *> InlinedLexicalScopeMap;
 
   /// AbstractScopeMap - These scopes are  not included LexicalScopeMap.
-  /// AbstractScopes owns its LexicalScope*s.
-  DenseMap<const MDNode *, LexicalScope *> AbstractScopeMap;
+  // Use an unordered_map to ensure value pointer validity over insertion.
+  std::unordered_map<const MDNode *, LexicalScope> AbstractScopeMap;
 
   /// AbstractScopesList - Tracks abstract scopes constructed while processing
   /// a function.
@@ -153,97 +245,6 @@ private:
   LexicalScope *CurrentFnLexicalScope;
 };
 
-//===----------------------------------------------------------------------===//
-/// LexicalScope - This class is used to track scope information.
-///
-class LexicalScope {
-
-public:
-  LexicalScope(LexicalScope *P, const MDNode *D, const MDNode *I, bool A)
-      : Parent(P), Desc(D), InlinedAtLocation(I), AbstractScope(A),
-        LastInsn(nullptr), FirstInsn(nullptr), DFSIn(0), DFSOut(0) {
-    if (Parent)
-      Parent->addChild(this);
-  }
-
-  // Accessors.
-  LexicalScope *getParent() const { return Parent; }
-  const MDNode *getDesc() const { return Desc; }
-  const MDNode *getInlinedAt() const { return InlinedAtLocation; }
-  const MDNode *getScopeNode() const { return Desc; }
-  bool isAbstractScope() const { return AbstractScope; }
-  SmallVectorImpl<LexicalScope *> &getChildren() { return Children; }
-  SmallVectorImpl<InsnRange> &getRanges() { return Ranges; }
-
-  /// addChild - Add a child scope.
-  void addChild(LexicalScope *S) { Children.push_back(S); }
-
-  /// openInsnRange - This scope covers instruction range starting from MI.
-  void openInsnRange(const MachineInstr *MI) {
-    if (!FirstInsn)
-      FirstInsn = MI;
-
-    if (Parent)
-      Parent->openInsnRange(MI);
-  }
-
-  /// extendInsnRange - Extend the current instruction range covered by
-  /// this scope.
-  void extendInsnRange(const MachineInstr *MI) {
-    assert(FirstInsn && "MI Range is not open!");
-    LastInsn = MI;
-    if (Parent)
-      Parent->extendInsnRange(MI);
-  }
-
-  /// closeInsnRange - Create a range based on FirstInsn and LastInsn collected
-  /// until now. This is used when a new scope is encountered while walking
-  /// machine instructions.
-  void closeInsnRange(LexicalScope *NewScope = nullptr) {
-    assert(LastInsn && "Last insn missing!");
-    Ranges.push_back(InsnRange(FirstInsn, LastInsn));
-    FirstInsn = nullptr;
-    LastInsn = nullptr;
-    // If Parent dominates NewScope then do not close Parent's instruction
-    // range.
-    if (Parent && (!NewScope || !Parent->dominates(NewScope)))
-      Parent->closeInsnRange(NewScope);
-  }
-
-  /// dominates - Return true if current scope dominates given lexical scope.
-  bool dominates(const LexicalScope *S) const {
-    if (S == this)
-      return true;
-    if (DFSIn < S->getDFSIn() && DFSOut > S->getDFSOut())
-      return true;
-    return false;
-  }
-
-  // Depth First Search support to walk and manipulate LexicalScope hierarchy.
-  unsigned getDFSOut() const { return DFSOut; }
-  void setDFSOut(unsigned O) { DFSOut = O; }
-  unsigned getDFSIn() const { return DFSIn; }
-  void setDFSIn(unsigned I) { DFSIn = I; }
-
-  /// dump - print lexical scope.
-  void dump(unsigned Indent = 0) const;
-
-private:
-  LexicalScope *Parent;                        // Parent to this scope.
-  AssertingVH<const MDNode> Desc;              // Debug info descriptor.
-  AssertingVH<const MDNode> InlinedAtLocation; // Location at which this
-                                               // scope is inlined.
-  bool AbstractScope;                          // Abstract Scope
-  SmallVector<LexicalScope *, 4> Children;     // Scopes defined in scope.
-                                               // Contents not owned.
-  SmallVector<InsnRange, 4> Ranges;
-
-  const MachineInstr *LastInsn;  // Last instruction of this scope.
-  const MachineInstr *FirstInsn; // First instruction of this scope.
-  unsigned DFSIn, DFSOut;        // In & Out Depth use to determine
-                                 // scope nesting.
-};
-
 } // end llvm namespace
 
 #endif

Modified: llvm/trunk/lib/CodeGen/LexicalScopes.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/LexicalScopes.cpp?rev=208364&r1=208363&r2=208364&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/LexicalScopes.cpp (original)
+++ llvm/trunk/lib/CodeGen/LexicalScopes.cpp Thu May  8 17:24:51 2014
@@ -26,15 +26,12 @@ using namespace llvm;
 
 #define DEBUG_TYPE "lexicalscopes"
 
-/// ~LexicalScopes - final cleanup after ourselves.
-LexicalScopes::~LexicalScopes() { reset(); }
-
 /// reset - Reset the instance so that it's prepared for another function.
 void LexicalScopes::reset() {
   MF = nullptr;
   CurrentFnLexicalScope = nullptr;
-  DeleteContainerSeconds(LexicalScopeMap);
-  DeleteContainerSeconds(AbstractScopeMap);
+  LexicalScopeMap.clear();
+  AbstractScopeMap.clear();
   InlinedLexicalScopeMap.clear();
   AbstractScopesList.clear();
 }
@@ -124,7 +121,7 @@ LexicalScope *LexicalScopes::findLexical
 
   if (IA)
     return InlinedLexicalScopeMap.lookup(DebugLoc::getFromDILocation(IA));
-  return LexicalScopeMap.lookup(Scope);
+  return findLexicalScope(Scope);
 }
 
 /// getOrCreateLexicalScope - Find lexical scope for the given DebugLoc. If
@@ -152,35 +149,43 @@ LexicalScope *LexicalScopes::getOrCreate
     D = DIDescriptor(Scope);
   }
 
-  LexicalScope *WScope = LexicalScopeMap.lookup(Scope);
-  if (WScope)
-    return WScope;
+  auto I = LexicalScopeMap.find(Scope);
+  if (I != LexicalScopeMap.end())
+    return &I->second;
 
   LexicalScope *Parent = nullptr;
   if (D.isLexicalBlock())
     Parent = getOrCreateLexicalScope(DebugLoc::getFromDILexicalBlock(Scope));
-  WScope = new LexicalScope(Parent, DIDescriptor(Scope), nullptr, false);
-  LexicalScopeMap.insert(std::make_pair(Scope, WScope));
+  // FIXME: Use forward_as_tuple instead of make_tuple, once MSVC2012
+  // compatibility is no longer required.
+  I = LexicalScopeMap.emplace(std::piecewise_construct, std::make_tuple(Scope),
+                              std::make_tuple(Parent, DIDescriptor(Scope),
+                                              nullptr, false)).first;
+
   if (!Parent && DIDescriptor(Scope).isSubprogram() &&
       DISubprogram(Scope).describes(MF->getFunction()))
-    CurrentFnLexicalScope = WScope;
+    CurrentFnLexicalScope = &I->second;
 
-  return WScope;
+  return &I->second;
 }
 
 /// getOrCreateInlinedScope - Find or create an inlined lexical scope.
 LexicalScope *LexicalScopes::getOrCreateInlinedScope(MDNode *Scope,
                                                      MDNode *InlinedAt) {
-  LexicalScope *InlinedScope = LexicalScopeMap.lookup(InlinedAt);
-  if (InlinedScope)
-    return InlinedScope;
+  auto I = LexicalScopeMap.find(InlinedAt);
+  if (I != LexicalScopeMap.end())
+    return &I->second;
 
   DebugLoc InlinedLoc = DebugLoc::getFromDILocation(InlinedAt);
-  InlinedScope = new LexicalScope(getOrCreateLexicalScope(InlinedLoc),
-                                  DIDescriptor(Scope), InlinedAt, false);
-  InlinedLexicalScopeMap[InlinedLoc] = InlinedScope;
-  LexicalScopeMap[InlinedAt] = InlinedScope;
-  return InlinedScope;
+  // FIXME: Use forward_as_tuple instead of make_tuple, once MSVC2012
+  // compatibility is no longer required.
+  I = LexicalScopeMap.emplace(
+                          std::piecewise_construct, std::make_tuple(InlinedAt),
+                          std::make_tuple(getOrCreateLexicalScope(InlinedLoc),
+                                          DIDescriptor(Scope), InlinedAt,
+                                          false)).first;
+  InlinedLexicalScopeMap[InlinedLoc] = &I->second;
+  return &I->second;
 }
 
 /// getOrCreateAbstractScope - Find or create an abstract lexical scope.
@@ -190,9 +195,9 @@ LexicalScope *LexicalScopes::getOrCreate
   DIDescriptor Scope(N);
   if (Scope.isLexicalBlockFile())
     Scope = DILexicalBlockFile(Scope).getScope();
-  LexicalScope *AScope = AbstractScopeMap.lookup(N);
-  if (AScope)
-    return AScope;
+  auto I = AbstractScopeMap.find(N);
+  if (I != AbstractScopeMap.end())
+    return &I->second;
 
   LexicalScope *Parent = nullptr;
   if (Scope.isLexicalBlock()) {
@@ -200,11 +205,13 @@ LexicalScope *LexicalScopes::getOrCreate
     DIDescriptor ParentDesc = DB.getContext();
     Parent = getOrCreateAbstractScope(ParentDesc);
   }
-  AScope = new LexicalScope(Parent, DIDescriptor(N), nullptr, true);
-  AbstractScopeMap[N] = AScope;
+  I = AbstractScopeMap.emplace(std::piecewise_construct,
+                               std::forward_as_tuple(N),
+                               std::forward_as_tuple(Parent, DIDescriptor(N),
+                                                     nullptr, true)).first;
   if (DIDescriptor(N).isSubprogram())
-    AbstractScopesList.push_back(AScope);
-  return AScope;
+    AbstractScopesList.push_back(&I->second);
+  return &I->second;
 }
 
 /// constructScopeNest





More information about the llvm-commits mailing list