Subzero: Basic Block Profiler.

src/IceGlobalContext.cpp

Index: src/IceGlobalContext.cpp
diff --git a/src/IceGlobalContext.cpp b/src/IceGlobalContext.cpp
index 8de57f3030fd4d3ace4e789030ef48123acb5df3..af594b55673188e4709de8668a782ad648a9c739 100644
--- a/src/IceGlobalContext.cpp
+++ b/src/IceGlobalContext.cpp
@@ -19,6 +19,7 @@
 #include "llvm/Support/Timer.h"
 
 #include "IceCfg.h"
+#include "IceCfgNode.h"
 #include "IceClFlags.h"
 #include "IceDefs.h"
 #include "IceELFObjectWriter.h"
@@ -255,6 +256,17 @@ GlobalContext::GlobalContext(Ostream *OsDump, Ostream *OsEmit, Ostream *OsError,
   }
 }
 
+namespace {
+void addCallToProfileSummary(Cfg *Func) {
+  auto *Context = Func->getContext();
+  auto *ProfileSummarySym = Context->getConstantExternSym("profile_summary");
+  constexpr bool HasTailCall = false;

[Jim Stichnoth, 2015/06/08 23:45:32]

We've been using "const bool ..." instead of constexpr in the rest of the code
base.  Wondering if there is any special benefit to using constexpr on a
primitive type?

[John, 2015/06/09 15:36:18]

constexpr is a compile time constant, and the compiler should be able to
propagate the value where it is used. const is a constant variable, not
necessarily a constant. Happy to change.

+  auto *Call =
+      InstCall::create(Func, 0, nullptr, ProfileSummarySym, HasTailCall);
+  Func->getEntryNode()->getInsts().push_front(Call);
+}
+} // end of namespace

[Jim Stichnoth, 2015/06/08 23:45:31]

end of anonymous namespace

[John, 2015/06/09 15:36:19]

Done.

+
 void GlobalContext::translateFunctions() {
   while (std::unique_ptr<Cfg> Func = optQueueBlockingPop()) {
     // Install Func in TLS for Cfg-specific container allocators.
@@ -277,6 +289,18 @@ void GlobalContext::translateFunctions() {
       Cfg::setCurrentCfg(nullptr);
       continue; // Func goes out of scope and gets deleted
     }
+
+    // ProfilerInits contains the VariableDeclarations for all the

[Jim Stichnoth, 2015/06/08 23:45:32]

Would it be reasonable to move this new code into Cfg::translate()?  in the
interest of trying to limit translateFunctions() to work queue management

I guess this would mean that Cfg needs to hold ProfileInits, but that seems in
line with other Cfg responsibilities.

[John, 2015/06/09 15:36:20]

I thought about that in the beginning, but then I decided to go with the current
design. No strong opinions. Done.

+    // profiler-related symbols. If profiling is disabled, this array
+    // will be null.
+    std::unique_ptr<VariableDeclarationList> ProfilerInits;
+    if (getFlags().getEnableBlockProfile()) {
+      ProfilerInits = Func->profileBlocks();
+      if (matchSymbolName(Func->getFunctionName(), "exit")) {

[Jim Stichnoth, 2015/06/08 23:45:32]

Does a normal return from main() lead to a call to exit() ?

[John, 2015/06/09 15:36:19]

It does, at least in the current library:

        subl    $16, %esp
        movl    %edi, (%esp)    # END=%edi
        movl    %ebx, 4(%esp)   # END=%ebx
        call    main
        addl    $16, %esp


        subl    $16, %esp
        movl    %eax, (%esp)    # END=%eax,%esp
        call    exit

        ud2
                                # LiveOut=

+        addCallToProfileSummary(Func.get());
+      }
+    }
+
     Func->translate();
     EmitterWorkItem *Item = nullptr;
     if (Func->hasError()) {
@@ -309,6 +333,7 @@ void GlobalContext::translateFunctions() {
     }
     Cfg::setCurrentCfg(nullptr);
     assert(Item);
+    Item->setProfilerInits(std::move(ProfilerInits));
     emitQueueBlockingPush(Item);
     // The Cfg now gets deleted as Func goes out of scope.
   }
@@ -316,6 +341,31 @@ void GlobalContext::translateFunctions() {
 
 namespace {
 
+VariableDeclaration *blockProfileInfo(const VariableDeclarationList &Globals) {
+  auto *Var = VariableDeclaration::create();
+  Var->setAlignment(8);
+  Var->setIsConstant(true);
+  Var->setName("block_profile_info");
+  Var->setLinkage(llvm::GlobalValue::ExternalLinkage);
+  for (const auto *Global : Globals) {
+    if (Cfg::isProfileGlobal(*Global)) {
+      Var->addInitializer(new VariableDeclaration::RelocInitializer(Global, 0));

[Jim Stichnoth, 2015/06/08 23:45:32]

Can you "document" the 0, and the 8 a few lines down, by passing in a const
whatever value that is initialized to 0/8?

[John, 2015/06/09 15:36:20]

Done.

+    }
+  }
+
+  Var->addInitializer(new VariableDeclaration::ZeroInitializer(8));
+
+  return Var;
+}
+
+void addBlockProfileInfoArrayToGlobals(VariableDeclarationList *Globals) {
+  // Purposefully create the Var temp to prevent bugs in case the compilers

[Jim Stichnoth, 2015/06/08 23:45:32]

compilers ==> compiler

[John, 2015/06/09 15:36:19]

Done.

+  // reorders instructions in a way that Globals is extended before the call
+  // to profileInfoArray.
+  auto *Var = blockProfileInfo(*Globals);
+  Globals->push_back(Var);
+}
+
 void lowerGlobals(GlobalContext *Ctx,
                   std::unique_ptr<VariableDeclarationList> VariableDeclarations,
                   TargetDataLowering *DataLowering) {
@@ -331,6 +381,13 @@ void lowerGlobals(GlobalContext *Ctx,
   }
   if (Ctx->getFlags().getDisableTranslation())
     return;
+
+  // There should be no need to emit the block_profile_info array if profiling
+  // is disabled. In practice, given that szrt_profiler.o will always be embeded

[Jim Stichnoth, 2015/06/08 23:45:31]

embedded

[John, 2015/06/09 15:36:18]

Done.

+  // in the application, we need to add it. In a non-profiled build this array
+  // will only contain the nullptr terminator.
+  addBlockProfileInfoArrayToGlobals(VariableDeclarations.get());
+
   DataLowering->lowerGlobals(std::move(VariableDeclarations));
 }
 
@@ -340,6 +397,13 @@ void resizePending(std::vector<EmitterWorkItem *> &Pending, uint32_t Index) {
     Pending.resize(Index + 1);
 }
 
+void AddAllIfNotNull(std::unique_ptr<VariableDeclarationList> src,

[Jim Stichnoth, 2015/06/08 23:45:32]

AddAllIfNotNull ==> addAllIfNotNull

[John, 2015/06/09 15:36:20]

Done.

+                     VariableDeclarationList *dst) {
+  if (src != nullptr) {
+    dst->insert(dst->end(), src->begin(), src->end());
+  }
+}
+
 } // end of anonymous namespace
 
 void GlobalContext::emitItems() {
@@ -350,6 +414,8 @@ void GlobalContext::emitItems() {
   // the work queue, and if it's not the item we're waiting for, we
   // insert it into Pending and repeat.  The work item is deleted
   // after it is processed.
+  std::unique_ptr<VariableDeclarationList> GlobalInits(
+      new VariableDeclarationList());
   std::vector<EmitterWorkItem *> Pending;
   uint32_t DesiredSequenceNumber = getFirstSequenceNumber();
   while (true) {
@@ -359,7 +425,7 @@ void GlobalContext::emitItems() {
     if (RawItem == nullptr)
       RawItem = emitQueueBlockingPop();
     if (RawItem == nullptr)
-      return;
+      break;
     uint32_t ItemSeq = RawItem->getSequenceNumber();
     if (Threaded && ItemSeq != DesiredSequenceNumber) {
       resizePending(Pending, ItemSeq);
@@ -373,10 +439,10 @@ void GlobalContext::emitItems() {
     case EmitterWorkItem::WI_Nop:
       break;
     case EmitterWorkItem::WI_GlobalInits: {
-      lowerGlobals(this, Item->getGlobalInits(),
-                   TargetDataLowering::createLowering(this).get());
+      AddAllIfNotNull(Item->getGlobalInits(), GlobalInits.get());
     } break;
     case EmitterWorkItem::WI_Asm: {
+      AddAllIfNotNull(Item->getProfilerInits(), GlobalInits.get());
       std::unique_ptr<Assembler> Asm = Item->getAsm();
       Asm->alignFunction();
       IceString MangledName = mangleName(Asm->getFunctionName());
@@ -398,6 +464,9 @@ void GlobalContext::emitItems() {
     case EmitterWorkItem::WI_Cfg: {
       if (!ALLOW_DUMP)
         llvm::report_fatal_error("WI_Cfg work item created inappropriately");
+
+      AddAllIfNotNull(Item->getProfilerInits(), GlobalInits.get());
+
       assert(getFlags().getOutFileType() == FT_Asm);
       std::unique_ptr<Cfg> Func = Item->getCfg();
       // Unfortunately, we have to temporarily install the Cfg in TLS
@@ -410,6 +479,9 @@ void GlobalContext::emitItems() {
     } break;
     }
   }
+
+  lowerGlobals(this, std::move(GlobalInits),
+               TargetDataLowering::createLowering(this).get());
 }
 
 // Scan a string for S[0-9A-Z]*_ patterns and replace them with