Subzero: Fix labels for block profiling.

The problem is that the block profiling pass runs at the very beginning and commits to particular label strings, but the actual label names might change by emission time because of node reordering. There was actually something of a workaround - given a label string from the profile output, inspect the *profiled* asm code and search for the block containing the increment of the counter location, as the name of the counter location label is related to the label string in the profile output. However, it's tedious to mentally filter out the counter update code, and the counter update code has a huge impact on register allocation. The solution is to use a persistent number in CfgNode for constructing the label string, which doesn't change when the nodes are reordered. One note (independent of this change): Without block profiling, empty basic blocks are deleted and don't appear in the asm output. But with block profiling, these blocks are never empty because they contain profile update instructions. This means the profile output may contain labels that don't exist in the non-profiled asm. Another note: New nodes created as a result of edge splitting from advanced phi lowering are not profiled. BUG= none R=ascull@google.com, jpp@chromium.org Review URL: https://codereview.chromium.org/1341613002 .

Subzero: Fix labels for block profiling.
e7dbc0bc · Jim Stichnoth · 1921fba6 · e7dbc0bc · e7dbc0bc · e7dbc0bc
Commit e7dbc0bc authored Sep 15, 2015 by Jim Stichnoth
Showing with 13 additions and 4 deletions

IceCfg.cpp src/IceCfg.cpp +1 -0

IceCfg.h src/IceCfg.h +2 -1

IceCfgNode.cpp src/IceCfgNode.cpp +2 -2

IceCfgNode.h src/IceCfgNode.h +2 -1

IceClFlags.cpp src/IceClFlags.cpp +6 -0

No files found.
--- a/src/IceCfg.cpp
+++ b/src/IceCfg.cpp
@@ -70,6 +70,7 @@ CfgNode *Cfg::makeNode() {
 }

 void Cfg::swapNodes(NodeList &NewNodes) {
+  assert(Nodes.size() == NewNodes.size());
  Nodes.swap(NewNodes);
  for (SizeT I = 0, NumNodes = getNumNodes(); I < NumNodes; ++I)
    Nodes[I]->resetIndex(I);

--- a/src/IceCfg.h
+++ b/src/IceCfg.h
@@ -92,7 +92,8 @@ public:
  CfgNode *makeNode();
  SizeT getNumNodes() const { return Nodes.size(); }
  const NodeList &getNodes() const { return Nodes; }
-  /// Swap nodes of Cfg with given list of nodes.
+  /// Swap nodes of Cfg with given list of nodes.  The number of nodes must
+  /// remain unchanged.
  void swapNodes(NodeList &NewNodes);
  /// @}


--- a/src/IceCfgNode.cpp
+++ b/src/IceCfgNode.cpp
@@ -27,14 +27,14 @@
 namespace Ice {

 CfgNode::CfgNode(Cfg *Func, SizeT LabelNumber)
-    : Func(Func), Number(LabelNumber) {}
+    : Func(Func), Number(LabelNumber), LabelNumber(LabelNumber) {}

 // Returns the name the node was created with.  If no name was given,
 // it synthesizes a (hopefully) unique name.
 IceString CfgNode::getName() const {
  if (NameIndex >= 0)
    return Func->getIdentifierName(NameIndex);
-  return "__" + std::to_string(getIndex());
+  return "__" + std::to_string(LabelNumber);
 }

 // Adds an instruction to either the Phi list or the regular

--- a/src/IceCfgNode.h
+++ b/src/IceCfgNode.h
@@ -112,7 +112,8 @@ public:
 private:
  CfgNode(Cfg *Func, SizeT LabelIndex);
  Cfg *const Func;
-  SizeT Number; /// label index
+  SizeT Number;            /// invariant: Func->Nodes[Number]==this
+  const SizeT LabelNumber; /// persistent number for label generation
  Cfg::IdentifierIndexType NameIndex =
      Cfg::IdentifierIndexInvalid; /// index into Cfg::NodeNames table
  SizeT LoopNestDepth = 0;         /// the loop nest depth of this node

--- a/src/IceClFlags.cpp
+++ b/src/IceClFlags.cpp
@@ -76,6 +76,12 @@ cl::opt<bool>
    DumpStats("szstats",
              cl::desc("Print statistics after translating each function"));

+// TODO(stichnot): The implementation of block profiling introduces some
+// oddities to be aware of.  First, empty basic blocks that don't normally
+// appear in the asm output, may be profiled anyway, so one might see profile
+// counts for blocks not in the original asm output.  Second, edge-split nodes
+// for advanced phi lowering are added too late, at which point it is not
+// practical to add profiling.
 cl::opt<bool> EnableBlockProfile(
    "enable-block-profile",
    cl::desc("If true, instrument basic blocks, and output profiling "