[BOLT][BFC] Refactor code for adding secondary function entries

Summary:
In non-relocation mode, the code for marking a function non-simple was
decoupled from the code that added new entry points.  Fix that.

(cherry picked from FBD21264247)

bolt/src/BinaryContext.cpp

@@ -1018,14 +1018,6 @@ void BinaryContext::processInterproceduralReferences() {
       if (ContainingFunction->getAddress() != Addr) {
         ContainingFunction->
           addEntryPointAtOffset(Addr - ContainingFunction->getAddress());
-        if (!HasRelocations) {
-          if (opts::Verbosity >= 1) {
-            errs() << "BOLT-WARNING: Function " << *ContainingFunction
-                   << " has internal BBs that are target of a reference "
-                   << "located in another function. Skipping the function.\n";
-          }
-          ContainingFunction->setSimple(false);
-        }
       }
     } else if (Addr) {
       // Check if address falls in function padding space - this could be

bolt/src/BinaryFunction.cpp

@@ -3069,6 +3069,63 @@ void BinaryFunction::postProcessBranches() {
   assert(validateCFG() && "invalid CFG");
 }
 
+MCSymbol *BinaryFunction::addEntryPointAtOffset(uint64_t Offset) {
+  assert(Offset && "cannot add primary entry point");
+  assert(CurrentState == State::Empty || CurrentState == State::Disassembled);
+
+  const uint64_t EntryPointAddress = getAddress() + Offset;
+  MCSymbol *LocalSymbol = getOrCreateLocalLabel(EntryPointAddress);
+
+  MCSymbol *EntrySymbol = getSecondaryEntryPointSymbol(LocalSymbol);
+  if (EntrySymbol)
+    return EntrySymbol;
+
+  if (auto *EntryBD = BC.getBinaryDataAtAddress(EntryPointAddress)) {
+    EntrySymbol = EntryBD->getSymbol();
+  } else {
+    EntrySymbol = BC.Ctx->getOrCreateSymbol(
+        "__ENTRY_0x" + Twine::utohexstr(Offset) + "_" + getOneName());
+  }
+  SecondaryEntryPoints[LocalSymbol] = EntrySymbol;
+
+  BC.setSymbolToFunctionMap(EntrySymbol, this);
+
+  // In non-relocation mode there's potentially an external undetectable
+  // reference to the entry point and hence we cannot move this entry
+  // point. Optimizing without moving could be difficult.
+  if (!BC.HasRelocations) {
+    if (opts::Verbosity >= 1) {
+      outs() << "BOLT-INFO: function " << *this
+             << " has an internal address that is a potential target of a "
+             << " reference from another function. Skipping the function.\n";
+    }
+    setSimple(false);
+  }
+
+  return EntrySymbol;
+}
+
+MCSymbol *BinaryFunction::addEntryPoint(const BinaryBasicBlock &BB) {
+  assert(CurrentState == State::CFG &&
+         "basic block can be added as an entry only in a function with CFG");
+
+  if (&BB == BasicBlocks.front())
+    return getSymbol();
+
+  auto *EntrySymbol = getSecondaryEntryPointSymbol(BB);
+  if (EntrySymbol)
+    return EntrySymbol;
+
+  EntrySymbol =
+    BC.Ctx->getOrCreateSymbol("__ENTRY_" + BB.getLabel()->getName());
+
+  SecondaryEntryPoints[BB.getLabel()] = EntrySymbol;
+
+  BC.setSymbolToFunctionMap(EntrySymbol, this);
+
+  return EntrySymbol;
+}
+
 const MCSymbol *BinaryFunction::getSymbolForEntryID(uint64_t EntryID) const {
   if (EntryID == 0)
     return getSymbol();

bolt/src/BinaryFunction.h

@@ -580,29 +580,7 @@ class BinaryFunction {
 
   /// Register secondary entry point at a given \p Offset into the function.
   /// Return global symbol for use by extern function references.
-  MCSymbol *addEntryPointAtOffset(uint64_t Offset) {
-    assert(Offset && "cannot add primary entry point");
-    assert(CurrentState == State::Empty || CurrentState == State::Disassembled);
-
-    const uint64_t EntryPointAddress = getAddress() + Offset;
-    MCSymbol *LocalSymbol = getOrCreateLocalLabel(EntryPointAddress);
-
-    MCSymbol *EntrySymbol = getSecondaryEntryPointSymbol(LocalSymbol);
-    if (EntrySymbol)
-      return EntrySymbol;
-
-    if (auto *EntryBD = BC.getBinaryDataAtAddress(EntryPointAddress)) {
-      EntrySymbol = EntryBD->getSymbol();
-    } else {
-      EntrySymbol = BC.Ctx->getOrCreateSymbol(
-          "__ENTRY_0x" + Twine::utohexstr(Offset) + "_" + getOneName());
-    }
-    SecondaryEntryPoints[LocalSymbol] = EntrySymbol;
-
-    BC.setSymbolToFunctionMap(EntrySymbol, this);
-
-    return EntrySymbol;
-  }
+  MCSymbol *addEntryPointAtOffset(uint64_t Offset);
 
   /// Register an internal offset in a function referenced from outside.
   void registerReferencedOffset(uint64_t Offset) {
@@ -1522,25 +1500,7 @@ class BinaryFunction {
 
   /// Add basic block \BB as an entry point to the function. Return global
   /// symbol associated with the entry.
-  MCSymbol *addEntryPoint(const BinaryBasicBlock &BB) {
-    assert(CurrentState == State::CFG);
-
-    if (&BB == BasicBlocks.front())
-      return getSymbol();
-
-    auto *EntrySymbol = getSecondaryEntryPointSymbol(BB);
-    if (EntrySymbol)
-      return EntrySymbol;
-
-    EntrySymbol =
-      BC.Ctx->getOrCreateSymbol("__ENTRY_" + BB.getLabel()->getName());
-
-    SecondaryEntryPoints[BB.getLabel()] = EntrySymbol;
-
-    BC.setSymbolToFunctionMap(EntrySymbol, this);
-
-    return EntrySymbol;
-  }
+  MCSymbol *addEntryPoint(const BinaryBasicBlock &BB);
 
   /// Mark all blocks that are unreachable from a root (entry point
   /// or landing pad) as invalid.

bolt/src/RewriteInstance.cpp

@@ -1129,9 +1129,6 @@ void RewriteInstance::discoverFileObjects() {
         PreviousFunction->
           addEntryPointAtOffset(Address - PreviousFunction->getAddress());
 
-        if (!BC->HasRelocations)
-          PreviousFunction->setSimple(false);
-
         // Remove the symbol from FileSymRefs so that we can skip it from
         // in the future.
         auto SI = FileSymRefs.find(Address);
@@ -1435,11 +1432,6 @@ void RewriteInstance::adjustFunctionBoundaries() {
       DEBUG(dbgs() << "BOLT-DEBUG: adding entry point to function " << Function
                    << " at offset 0x" << Twine::utohexstr(EntryOffset) << '\n');
       Function.addEntryPointAtOffset(EntryOffset);
-      // In non-relocation mode there's potentially an external undetectable
-      // reference to the entry point and hence we cannot move this entry
-      // point. Optimizing without moving could be difficult.
-      if (!BC->HasRelocations)
-        Function.setSimple(false);
 
       ++NextSymRefI;
     }