Replace RegisterExternalFunction with GetOrRegisterExternalFunction using an...

Replace RegisterExternalFunction with GetOrRegisterExternalFunction using an unordered_map in codegen_utils

src/backend/codegen/exec_qual_codegen.cc

@@ -74,7 +74,7 @@ bool ExecQualCodegen::GenerateExecQual(
   codegen_utils->CreateElog(DEBUG1, "Falling back to regular ExecQual.");
 
   codegen_utils->CreateFallback<ExecQualFn>(
-      codegen_utils->RegisterExternalFunction(GetRegularFuncPointer()),
+      codegen_utils->GetOrRegisterExternalFunction(GetRegularFuncPointer()),
 	  exec_qual_func);
   return true;
 }

src/backend/codegen/exec_variable_list_codegen.cc

@@ -133,7 +133,7 @@ bool ExecVariableListCodegen::GenerateExecVariableList(
       "fallback", exec_variable_list_func);
 
   // External functions
-  llvm::Function* llvm_memset = codegen_utils->RegisterExternalFunction(memset);
+  llvm::Function* llvm_memset = codegen_utils->GetOrRegisterExternalFunction(memset);
 
   // Generation-time constants
   llvm::Value* llvm_max_attr = codegen_utils->GetConstant(max_attr);
@@ -587,7 +587,7 @@ bool ExecVariableListCodegen::GenerateExecVariableList(
       llvm_error);
 
   codegen_utils->CreateFallback<ExecVariableListFn>(
-      codegen_utils->RegisterExternalFunction(GetRegularFuncPointer()),
+      codegen_utils->GetOrRegisterExternalFunction(GetRegularFuncPointer()),
       exec_variable_list_func);
   return true;
 }

src/backend/codegen/include/codegen/utils/codegen_utils.h

@@ -23,6 +23,7 @@
 #include <string>
 #include <type_traits>
 #include <utility>
+#include <unordered_map>
 #include <vector>
 
 #include "codegen/utils/annotated_type.h"
@@ -268,66 +269,6 @@ class CodegenUtils {
     return llvm::BasicBlock::Create(context_, name, parent, nullptr);
   }
 
-  /**
-   * @brief Register an external function from the calling environment so that
-   *        it can be called from generated code.
-   *
-   * @note This works for plain functions and for static methods of classes,
-   *       but not instance methods of classes.
-   * @note If a function has multiple overloaded versions, then the template
-   *       parameters of this method can be explicitly set to disambiguate
-   *       which version of the function is desired. For non-overloaded
-   *       functions, this is not necessary and the function's type signature
-   *       can be automatically inferred.
-   * @warning This method returns a pointer to an llvm::Function object. The
-   *          caller should NOT attempt to add BasicBlocks to the function, as
-   *          that would cause conflicts when mapping the function to its
-   *          external implementation during PrepareForExecution().
-   *
-   * @tparam ReturnType The return type of the external_function. This does not
-   *         need to be specified if external_function is not overloaded (it
-   *         will be inferred automatically).
-   * @tparam argument_types The types of the arguments to external_function.
-   *         These do not need to be specified if external_function is not
-   *         overloaded (they will be inferred automatically).
-   * @param external_function A function pointer to install for use in this
-   *        CodegenUtils.
-   * @param name An optional name to refer to the external function by. If
-   *        non-empty, this CodegenUtils will record additional information
-   *        so that the registered function will also be callable by its name
-   *        in C++ source code compiled by ClangCompiler (see
-   *        ClangCompiler::GenerateExternalFunctionDeclarations()).
-   * @param is_var_arg Whether the function has trailing variable arguments list
-   * @return A callable LLVM function.
-   **/
-  template <typename ReturnType, typename... ArgumentTypes>
-  llvm::Function* RegisterExternalFunction(
-      ReturnType (*external_function)(ArgumentTypes...),
-      const std::string& name = "",
-      const bool is_var_arg = false) {
-    external_functions_.emplace_back(
-        name.empty() ? GenerateExternalFunctionName()
-                     : name,
-        reinterpret_cast<std::uint64_t>(external_function));
-
-    if (!name.empty()) {
-      RecordNamedExternalFunction<ReturnType, ArgumentTypes...>(name);
-    }
-
-    return CreateFunctionImpl<ReturnType, ArgumentTypes...>(
-        external_functions_.back().first, is_var_arg);
-  }
-
-  template <typename ReturnType, typename... ArgumentTypes>
-  llvm::Function* RegisterExternalFunction(
-      ReturnType (*external_function)(ArgumentTypes..., ...),
-      const std::string& name = "") {
-    return RegisterExternalFunction(
-        reinterpret_cast<ReturnType (*)(ArgumentTypes...)>(external_function),
-        name,
-        true);
-  }
-
   /*
    * @brief Register an external function if previously unregistered. Otherwise
    *        return a pointer to the previously registered llvm::Function
@@ -358,18 +299,24 @@ class CodegenUtils {
       ReturnType (*external_function)(ArgumentTypes...),
       const std::string& name = "",
       const bool is_var_arg = false) {
-    auto it = std::find_if(
-        external_functions_.begin(),
-        external_functions_.end(),
-        [external_function] (decltype(external_functions_)::value_type val) -> bool {
-          return val.second == reinterpret_cast<std::uint64_t>(external_function);
-    });
-
-    if (it == external_functions_.end()) {
-      // If not found
-      return RegisterExternalFunction(external_function, name, is_var_arg);
+
+    std::unordered_map<std::uint64_t, std::string>::iterator it;
+    bool key_absent;
+
+    std::tie(it, key_absent) = external_functions_.emplace(
+        reinterpret_cast<std::uint64_t>(external_function),
+        name.empty() ? GenerateExternalFunctionName()
+                     : name);
+
+    if (!key_absent) {
+      return module()->getFunction(it->second);
     } else {
-      return module()->getFunction(it->first);
+
+      if (!name.empty()) {
+        RecordNamedExternalFunction<ReturnType, ArgumentTypes...>(name);
+      }
+      return CreateFunctionImpl<ReturnType, ArgumentTypes...>(
+          it->second, is_var_arg);
     }
   }
 
@@ -565,11 +512,10 @@ class CodegenUtils {
 
   std::unique_ptr<llvm::ExecutionEngine> engine_;
 
-  // Pairs of (function_name, address) for each external function registered by
-  // RegisterExternalFunction(). PrepareForExecution() adds a mapping for each
-  // such function to '*engine_' after creating it.
-  std::vector<std::pair<const std::string, const std::uint64_t>>
-      external_functions_;
+  // Map of (address, function_name) for each external function registered by
+  // GetOrRegisterExternalFunction(). PrepareForExecution() adds a mapping for
+  // each such function to '*engine_' after creating it.
+  std::unordered_map<std::uint64_t, std::string> external_functions_;
 
   // Keep track of additional information about named external functions so that
   // ClangCompiler can reconstruct accurate declarations for them.

src/backend/codegen/tests/clang_compiler_unittest.cc

@@ -372,7 +372,7 @@ static const char kExternalFunctionBinomialCoefficientSource[] =
 }  // namespace
 
 TEST_F(ClangCompilerTest, ExternalFunctionTest) {
-  codegen_utils_->RegisterExternalFunction(&factorial, "factorial");
+  codegen_utils_->GetOrRegisterExternalFunction(&factorial, "factorial");
   EXPECT_TRUE(clang_compiler_->CompileCppSource(
       llvm::Twine(clang_compiler_->GenerateExternalFunctionDeclarations())
           .concat(kExternalFunctionBinomialCoefficientSource)));
@@ -429,16 +429,16 @@ static const char kExternalMethodInvocationSource[] =
 }  // namespace
 
 TEST_F(ClangCompilerTest, ExternalMethodTest) {
-  codegen_utils_->RegisterExternalFunction(
+  codegen_utils_->GetOrRegisterExternalFunction(
       &WrapNew<Accumulator<double>, double>,
       "new_Accumulator");
-  codegen_utils_->RegisterExternalFunction(
+  codegen_utils_->GetOrRegisterExternalFunction(
       &WrapDelete<Accumulator<double>>,
       "delete_Accumulator");
-  codegen_utils_->RegisterExternalFunction(
+  codegen_utils_->GetOrRegisterExternalFunction(
       &GPCODEGEN_WRAP_METHOD(&Accumulator<double>::Accumulate),
       "Accumulator_Accumulate");
-  codegen_utils_->RegisterExternalFunction(
+  codegen_utils_->GetOrRegisterExternalFunction(
       &GPCODEGEN_WRAP_METHOD(&Accumulator<double>::Get),
       "Accumulator_Get");
 

src/backend/codegen/tests/codegen_utils_unittest.cc

@@ -907,7 +907,7 @@ class CodegenUtilsTest : public ::testing::Test {
     // Register 'external_function' in 'codegen_utils_' and check that it has
     // the expected type-signature.
     llvm::Function* llvm_external_function
-        = codegen_utils_->RegisterExternalFunction(external_function);
+        = codegen_utils_->GetOrRegisterExternalFunction(external_function);
     ASSERT_NE(llvm_external_function, nullptr);
     EXPECT_EQ(
         (codegen_utils_->GetFunctionType<ReturnType, ArgumentTypes...>()
@@ -2042,7 +2042,7 @@ TEST_F(CodegenUtilsTest, VariadicExternalFunctionTest) {
 
   // Register printf with takes variable arguments past char*
   llvm::Function* llvm_printf_function =
-      codegen_utils_->RegisterExternalFunction(printf);
+      codegen_utils_->GetOrRegisterExternalFunction(printf);
   ASSERT_NE(llvm_printf_function, nullptr);
   ASSERT_EQ((codegen_utils_->
         GetFunctionType<int, char*>(true)->getPointerTo()),
@@ -2050,7 +2050,7 @@ TEST_F(CodegenUtilsTest, VariadicExternalFunctionTest) {
 
   // Register sprintf with takes variable arguments past char*, char*
   llvm::Function* llvm_sprintf_function =
-      codegen_utils_->RegisterExternalFunction(sprintf);
+      codegen_utils_->GetOrRegisterExternalFunction(sprintf);
   ASSERT_NE(llvm_sprintf_function, nullptr);
   ASSERT_EQ((codegen_utils_->
         GetFunctionType<int, char*, char*>(true)->getPointerTo()),
@@ -2667,16 +2667,16 @@ TEST_F(CodegenUtilsTest, OptimizationTest) {
 TEST_F(CodegenUtilsTest, CppClassObjectTest) {
   // Register method wrappers for Accumulator<double>
   llvm::Function* new_accumulator_double
-      = codegen_utils_->RegisterExternalFunction(
+      = codegen_utils_->GetOrRegisterExternalFunction(
           &WrapNew<Accumulator<double>, double>);
   llvm::Function* delete_accumulator_double
-      = codegen_utils_->RegisterExternalFunction(
+      = codegen_utils_->GetOrRegisterExternalFunction(
           &WrapDelete<Accumulator<double>>);
   llvm::Function* accumulator_double_accumulate
-      = codegen_utils_->RegisterExternalFunction(
+      = codegen_utils_->GetOrRegisterExternalFunction(
           &GPCODEGEN_WRAP_METHOD(&Accumulator<double>::Accumulate));
   llvm::Function* accumulator_double_get
-      = codegen_utils_->RegisterExternalFunction(
+      = codegen_utils_->GetOrRegisterExternalFunction(
           &GPCODEGEN_WRAP_METHOD(&Accumulator<double>::Get));
 
   typedef double (*AccumulateTestFn) (double);
@@ -2736,9 +2736,9 @@ TEST_F(CodegenUtilsTest, CppClassObjectTest) {
   EXPECT_EQ(-12.75, (*accumulate_test_fn_compiled)(-22.75));
 }
 
-// Test GetOrRegisterExternalFunction to return the right llvm::Function if
+// Test GetOrGetOrRegisterExternalFunction to return the right llvm::Function if
 // previously registered or else register it anew
-TEST_F(CodegenUtilsTest, GetOrRegisterExternalFunctionTest) {
+TEST_F(CodegenUtilsTest, GetOrGetOrRegisterExternalFunctionTest) {
 
   // Test previous unregistered function
   EXPECT_EQ(nullptr, codegen_utils_->module()->getFunction("floor"));
@@ -2746,13 +2746,13 @@ TEST_F(CodegenUtilsTest, GetOrRegisterExternalFunctionTest) {
   EXPECT_EQ(floor_func, codegen_utils_->module()->getFunction("floor"));
 
   // Test previous registered Non vararg function
-  llvm::Function* expected_fabs_func = codegen_utils_->RegisterExternalFunction(ceil);
+  llvm::Function* expected_fabs_func = codegen_utils_->GetOrRegisterExternalFunction(ceil);
   llvm::Function* fabs_func = codegen_utils_->GetOrRegisterExternalFunction(ceil);
 
   EXPECT_EQ(expected_fabs_func, fabs_func);
 
   // Test previously registered vararg function
-  llvm::Function* expected_vprintf_func = codegen_utils_->RegisterExternalFunction(vprintf);
+  llvm::Function* expected_vprintf_func = codegen_utils_->GetOrRegisterExternalFunction(vprintf);
   llvm::Function* vprintf_func = codegen_utils_->GetOrRegisterExternalFunction(vprintf);
 
   EXPECT_EQ(expected_vprintf_func, vprintf_func);
@@ -2782,7 +2782,7 @@ TEST_F(CodegenUtilsDeathTest, WrongFunctionTypeTest) {
 TEST_F(CodegenUtilsDeathTest, ModifyExternalFunctionTest) {
   // Register an external function, then try to add a BasicBlock to it.
   llvm::Function* external_function
-      = codegen_utils_->RegisterExternalFunction(&std::mktime);
+      = codegen_utils_->GetOrRegisterExternalFunction(&std::mktime);
 
   EXPECT_DEATH(codegen_utils_->CreateBasicBlock("body", external_function),
                "");

src/backend/codegen/utils/codegen_utils.cc

@@ -248,16 +248,16 @@ bool CodegenUtils::PrepareForExecution(const OptimizationLevel cpu_opt_level,
   }
 
   // Map registered external functions to their actual locations in memory.
-  for (const std::pair<const std::string,
-                       const std::uint64_t>& external_function
+  for (const std::pair<const std::uint64_t,
+                       const std::string>& external_function
        : external_functions_) {
     engine_->addGlobalMapping(
 #ifdef __APPLE__
-        std::string(1, '_') + external_function.first,
+        std::string(1, '_') + external_function.second,
 #else  // !__APPLE__
-        external_function.first,
+        external_function.second,
 #endif
-        external_function.second);
+        external_function.first);
   }
 
   return true;