From c8aaa503242167ede34141b06505776fb31d509b Mon Sep 17 00:00:00 2001 From: Yue Ni Date: Thu, 9 Nov 2023 19:39:33 +0800 Subject: [PATCH] Rename C interface function to C function for simplicity. --- cpp/src/gandiva/CMakeLists.txt | 2 +- cpp/src/gandiva/engine.cc | 4 +-- cpp/src/gandiva/exported_funcs.h | 4 +-- ...e_functions.cc => external_c_functions.cc} | 25 +++++++++---------- .../gandiva/function_holder_maker_registry.cc | 13 ++++------ cpp/src/gandiva/function_registry.cc | 10 ++++---- cpp/src/gandiva/function_registry.h | 12 ++++----- cpp/src/gandiva/gdv_string_function_stubs.cc | 2 -- cpp/src/gandiva/llvm_generator_test.cc | 4 +-- cpp/src/gandiva/tests/projector_test.cc | 9 +++---- cpp/src/gandiva/tests/test_util.cc | 17 +++++-------- cpp/src/gandiva/tests/test_util.h | 2 +- 12 files changed, 46 insertions(+), 58 deletions(-) rename cpp/src/gandiva/{external_c_interface_functions.cc => external_c_functions.cc} (81%) diff --git a/cpp/src/gandiva/CMakeLists.txt b/cpp/src/gandiva/CMakeLists.txt index 0290edd44c2a5..abbf0ede03168 100644 --- a/cpp/src/gandiva/CMakeLists.txt +++ b/cpp/src/gandiva/CMakeLists.txt @@ -62,7 +62,7 @@ set(SRC_FILES expression_registry.cc exported_funcs_registry.cc exported_funcs.cc - external_c_interface_functions.cc + external_c_functions.cc filter.cc function_ir_builder.cc function_holder_maker_registry.cc diff --git a/cpp/src/gandiva/engine.cc b/cpp/src/gandiva/engine.cc index a3868d754cb76..1cea1fd2cbf30 100644 --- a/cpp/src/gandiva/engine.cc +++ b/cpp/src/gandiva/engine.cc @@ -449,8 +449,8 @@ void Engine::AddGlobalMappingForFunc(const std::string& name, llvm::Type* ret_ty arrow::Status Engine::AddGlobalMappings() { ARROW_RETURN_NOT_OK(ExportedFuncsRegistry::AddMappings(this)); - ExternalCInterfaceFunctions c_interface_funcs(function_registry_); - return c_interface_funcs.AddMappings(this); + ExternalCFunctions c_funcs(function_registry_); + return c_funcs.AddMappings(this); } std::string Engine::DumpIR() { diff --git a/cpp/src/gandiva/exported_funcs.h b/cpp/src/gandiva/exported_funcs.h index 7bd8c762d84d6..586d598091501 100644 --- a/cpp/src/gandiva/exported_funcs.h +++ b/cpp/src/gandiva/exported_funcs.h @@ -63,9 +63,9 @@ class ExportedHashFunctions : public ExportedFuncsBase { arrow::Status AddMappings(Engine* engine) const override; }; -class ExternalCInterfaceFunctions : public ExportedFuncsBase { +class ExternalCFunctions : public ExportedFuncsBase { public: - explicit ExternalCInterfaceFunctions( + explicit ExternalCFunctions( std::shared_ptr function_registry) : function_registry_(std::move(function_registry)) {} diff --git a/cpp/src/gandiva/external_c_interface_functions.cc b/cpp/src/gandiva/external_c_functions.cc similarity index 81% rename from cpp/src/gandiva/external_c_interface_functions.cc rename to cpp/src/gandiva/external_c_functions.cc index c9f664a32e36f..2011d499f1c56 100644 --- a/cpp/src/gandiva/external_c_interface_functions.cc +++ b/cpp/src/gandiva/external_c_functions.cc @@ -51,39 +51,38 @@ static arrow::Result AsLLVMType(const DataTypePtr& from_type, // map from a NativeFunction's signature to the corresponding LLVM signature static arrow::Result, llvm::Type*>> MapToLLVMSignature( const FunctionSignature& sig, const NativeFunction& func, LLVMTypes* types) { - std::vector args; - args.reserve(sig.param_types().size()); + std::vector arg_llvm_types; + arg_llvm_types.reserve(sig.param_types().size()); if (func.NeedsContext()) { - args.emplace_back(types->i64_type()); + arg_llvm_types.emplace_back(types->i64_type()); } if (func.NeedsFunctionHolder()) { - args.emplace_back(types->i64_type()); + arg_llvm_types.emplace_back(types->i64_type()); } for (auto const& arg : sig.param_types()) { if (arg->id() == arrow::Type::STRING) { - args.emplace_back(types->i8_ptr_type()); - args.emplace_back(types->i32_type()); + arg_llvm_types.emplace_back(types->i8_ptr_type()); + arg_llvm_types.emplace_back(types->i32_type()); } else { ARROW_ASSIGN_OR_RAISE(auto arg_llvm_type, AsLLVMType(arg, types)); - args.emplace_back(arg_llvm_type); + arg_llvm_types.emplace_back(arg_llvm_type); } } llvm::Type* ret_llvm_type; if (sig.ret_type()->id() == arrow::Type::STRING) { // for string output, the last arg is the output length - args.emplace_back(types->i32_ptr_type()); + arg_llvm_types.emplace_back(types->i32_ptr_type()); ret_llvm_type = types->i8_ptr_type(); } else { ARROW_ASSIGN_OR_RAISE(ret_llvm_type, AsLLVMType(sig.ret_type(), types)); } - auto return_type = AsLLVMType(sig.ret_type(), types); - return std::make_pair(args, ret_llvm_type); + return std::make_pair(arg_llvm_types, ret_llvm_type); } -arrow::Status ExternalCInterfaceFunctions::AddMappings(Engine* engine) const { - auto const& c_interface_funcs = function_registry_->GetCInterfaceFunctions(); +arrow::Status ExternalCFunctions::AddMappings(Engine* engine) const { + auto const& c_funcs = function_registry_->GetCFunctions(); auto types = engine->types(); - for (auto& [func, func_ptr] : c_interface_funcs) { + for (auto& [func, func_ptr] : c_funcs) { for (auto const& sig : func.signatures()) { ARROW_ASSIGN_OR_RAISE(auto llvm_signature, MapToLLVMSignature(sig, func, types)); auto& [args, ret_llvm_type] = llvm_signature; diff --git a/cpp/src/gandiva/function_holder_maker_registry.cc b/cpp/src/gandiva/function_holder_maker_registry.cc index 169147697d7a7..bb93402475ae8 100644 --- a/cpp/src/gandiva/function_holder_maker_registry.cc +++ b/cpp/src/gandiva/function_holder_maker_registry.cc @@ -19,6 +19,7 @@ #include +#include "arrow/util/string.h" #include "gandiva/function_holder.h" #include "gandiva/interval_holder.h" #include "gandiva/random_generator_holder.h" @@ -27,18 +28,14 @@ namespace gandiva { +using arrow::internal::AsciiToLower; + FunctionHolderMakerRegistry::FunctionHolderMakerRegistry() : function_holder_makers_(DefaultHolderMakers()) {} -static std::string to_lower(const std::string& str) { - std::string data = str; - std::transform(data.begin(), data.end(), data.begin(), - [](unsigned char c) { return std::tolower(c); }); - return data; -} arrow::Status FunctionHolderMakerRegistry::Register(const std::string& name, FunctionHolderMaker holder_maker) { - function_holder_makers_.emplace(to_lower(name), std::move(holder_maker)); + function_holder_makers_.emplace(AsciiToLower(name), std::move(holder_maker)); return arrow::Status::OK(); } @@ -51,7 +48,7 @@ static arrow::Result HolderMaker(const FunctionNode& node) { arrow::Result FunctionHolderMakerRegistry::Make( const std::string& name, const FunctionNode& node) { - auto lowered_name = to_lower(name); + auto lowered_name = AsciiToLower(name); auto found = function_holder_makers_.find(lowered_name); if (found == function_holder_makers_.end()) { return Status::Invalid("function holder not registered for function " + name); diff --git a/cpp/src/gandiva/function_registry.cc b/cpp/src/gandiva/function_registry.cc index 939627c9c25fb..714e116e23823 100644 --- a/cpp/src/gandiva/function_registry.cc +++ b/cpp/src/gandiva/function_registry.cc @@ -110,7 +110,7 @@ arrow::Status FunctionRegistry::Register(const std::vector& func } arrow::Status FunctionRegistry::Register( - NativeFunction func, void* stub_function_ptr, + NativeFunction func, void* c_function_ptr, std::optional function_holder_maker) { if (function_holder_maker.has_value()) { // all signatures should have the same base name, use the first signature's base name @@ -118,7 +118,7 @@ arrow::Status FunctionRegistry::Register( ARROW_RETURN_NOT_OK(holder_maker_registry_.Register( func_base_name, std::move(function_holder_maker.value()))); } - c_interface_functions_.emplace_back(func, stub_function_ptr); + c_functions_.emplace_back(func, c_function_ptr); ARROW_RETURN_NOT_OK(FunctionRegistry::Add(std::move(func))); return Status::OK(); } @@ -128,9 +128,9 @@ const std::vector>& FunctionRegistry::GetBitcodeB return bitcode_memory_buffers_; } -const std::vector>& -FunctionRegistry::GetCInterfaceFunctions() const { - return c_interface_functions_; +const std::vector>& FunctionRegistry::GetCFunctions() + const { + return c_functions_; } const FunctionHolderMakerRegistry& FunctionRegistry::GetFunctionHolderMakerRegistry() diff --git a/cpp/src/gandiva/function_registry.h b/cpp/src/gandiva/function_registry.h index c1860188d17a2..24b64fac5f3fa 100644 --- a/cpp/src/gandiva/function_registry.h +++ b/cpp/src/gandiva/function_registry.h @@ -58,21 +58,21 @@ class GANDIVA_EXPORT FunctionRegistry { arrow::Status Register(const std::vector& funcs, std::shared_ptr bitcode_buffer); - /// \brief register a C interface function into the function registry + /// \brief register a C function into the function registry /// @param func the registered function's metadata - /// @param c_interface_function_ptr the function pointer to the + /// @param c_function_ptr the function pointer to the /// registered function's implementation /// @param function_holder_maker this will be used as the function holder if the /// function requires a function holder arrow::Status Register( - NativeFunction func, void* c_interface_function_ptr, + NativeFunction func, void* c_function_ptr, std::optional function_holder_maker = std::nullopt); /// \brief get a list of bitcode memory buffers saved in the registry const std::vector>& GetBitcodeBuffers() const; - /// \brief get a list of C interface functions saved in the registry - const std::vector>& GetCInterfaceFunctions() const; + /// \brief get a list of C functions saved in the registry + const std::vector>& GetCFunctions() const; const FunctionHolderMakerRegistry& GetFunctionHolderMakerRegistry() const; @@ -86,7 +86,7 @@ class GANDIVA_EXPORT FunctionRegistry { std::vector pc_registry_; SignatureMap pc_registry_map_; std::vector> bitcode_memory_buffers_; - std::vector> c_interface_functions_; + std::vector> c_functions_; FunctionHolderMakerRegistry holder_maker_registry_; Status Add(NativeFunction func); diff --git a/cpp/src/gandiva/gdv_string_function_stubs.cc b/cpp/src/gandiva/gdv_string_function_stubs.cc index 51b1f3b80075c..9f5b5ce64b4a9 100644 --- a/cpp/src/gandiva/gdv_string_function_stubs.cc +++ b/cpp/src/gandiva/gdv_string_function_stubs.cc @@ -15,8 +15,6 @@ // specific language governing permissions and limitations // under the License. -// #pragma once - #include "gandiva/gdv_function_stubs.h" #include diff --git a/cpp/src/gandiva/llvm_generator_test.cc b/cpp/src/gandiva/llvm_generator_test.cc index 9cd2b945fe2d8..853d8ae6c3b8d 100644 --- a/cpp/src/gandiva/llvm_generator_test.cc +++ b/cpp/src/gandiva/llvm_generator_test.cc @@ -139,9 +139,9 @@ TEST_F(TestLLVMGenerator, VerifyExtendedPCFunctions) { }); } -TEST_F(TestLLVMGenerator, VerifyExtendedCInterfaceFunctions) { +TEST_F(TestLLVMGenerator, VerifyExtendedCFunctions) { VerifyFunctionMapping("multiply_by_three_int32", [](auto registry) { - return TestConfigWithCInterfaceFunction(std::move(registry)); + return TestConfigWithCFunction(std::move(registry)); }); VerifyFunctionMapping("multiply_by_n_int32_int32", [](auto registry) { diff --git a/cpp/src/gandiva/tests/projector_test.cc b/cpp/src/gandiva/tests/projector_test.cc index 16f7f994d5856..59eeb3d92f19a 100644 --- a/cpp/src/gandiva/tests/projector_test.cc +++ b/cpp/src/gandiva/tests/projector_test.cc @@ -3608,7 +3608,7 @@ TEST_F(TestProjector, TestExtendedFunctions) { EXPECT_ARROW_ARRAY_EQUALS(out, outs.at(0)); } -TEST_F(TestProjector, TestExtendedCInterfaceFunctions) { +TEST_F(TestProjector, TestExtendedCFunctions) { auto in_field = field("in", arrow::int32()); auto schema = arrow::schema({in_field}); auto out_field = field("out", arrow::int64()); @@ -3617,8 +3617,7 @@ TEST_F(TestProjector, TestExtendedCInterfaceFunctions) { std::shared_ptr projector; auto external_registry = std::make_shared(); - auto config_with_func_registry = - TestConfigWithCInterfaceFunction(std::move(external_registry)); + auto config_with_func_registry = TestConfigWithCFunction(std::move(external_registry)); ARROW_EXPECT_OK( Projector::Make(schema, {multiply}, config_with_func_registry, &projector)); @@ -3632,7 +3631,7 @@ TEST_F(TestProjector, TestExtendedCInterfaceFunctions) { EXPECT_ARROW_ARRAY_EQUALS(out, outs.at(0)); } -TEST_F(TestProjector, TestExtendedCInterfaceFunctionsWithFunctionHolder) { +TEST_F(TestProjector, TestExtendedCFunctionsWithFunctionHolder) { auto multiple = TreeExprBuilder::MakeLiteral(5); auto in_field = field("in", arrow::int32()); auto schema = arrow::schema({in_field}); @@ -3660,7 +3659,7 @@ TEST_F(TestProjector, TestExtendedCInterfaceFunctionsWithFunctionHolder) { EXPECT_ARROW_ARRAY_EQUALS(out, outs.at(0)); } -TEST_F(TestProjector, TestExtendedCInterfaceFunctionThatNeedsContext) { +TEST_F(TestProjector, TestExtendedCFunctionThatNeedsContext) { auto in_field = field("in", arrow::utf8()); auto schema = arrow::schema({in_field}); auto out_field = field("out", arrow::utf8()); diff --git a/cpp/src/gandiva/tests/test_util.cc b/cpp/src/gandiva/tests/test_util.cc index 8ef30a907e983..84d19de685297 100644 --- a/cpp/src/gandiva/tests/test_util.cc +++ b/cpp/src/gandiva/tests/test_util.cc @@ -46,7 +46,7 @@ NativeFunction GetTestExternalFunction() { return multiply_by_two_func; } -static NativeFunction GetTestExternalCInterfaceFunction() { +static NativeFunction GetTestExternalCFunction() { NativeFunction multiply_by_three_func( "multiply_by_three", {}, {arrow::int32()}, arrow::int64(), ResultNullableType::kResultNullIfNull, "multiply_by_three_int32"); @@ -89,7 +89,7 @@ class MultiplyHolder : public FunctionHolder { public: explicit MultiplyHolder(int32_t num) : num_(num) {} - static Status Make(const FunctionNode& node, std::shared_ptr* holder) { + static arrow::Result> Make(const FunctionNode& node) { ARROW_RETURN_IF(node.children().size() != 2, Status::Invalid("'multiply_by_n' function requires two parameters")); @@ -105,9 +105,8 @@ class MultiplyHolder : public FunctionHolder { Status::Invalid( "'multiply_by_n' function requires an int32 literal as the 2nd parameter")); - *holder = std::make_shared( + return std::make_shared( literal->is_null() ? 0 : std::get(literal->holder())); - return Status::OK(); } int32_t operator()() const { return num_; } @@ -143,10 +142,10 @@ static const char* multiply_by_two_formula(int64_t ctx, const char* value, } } -std::shared_ptr TestConfigWithCInterfaceFunction( +std::shared_ptr TestConfigWithCFunction( std::shared_ptr registry) { return BuildConfigurationWithRegistry(std::move(registry), [](auto reg) { - return reg->Register(GetTestExternalCInterfaceFunction(), + return reg->Register(GetTestExternalCFunction(), reinterpret_cast(multiply_by_three)); }); } @@ -156,11 +155,7 @@ std::shared_ptr TestConfigWithHolderFunction( return BuildConfigurationWithRegistry(std::move(registry), [](auto reg) { return reg->Register( GetTestFunctionWithFunctionHolder(), reinterpret_cast(multiply_by_n), - [](const FunctionNode& node) -> arrow::Result { - std::shared_ptr derived_instance; - ARROW_RETURN_NOT_OK(MultiplyHolder::Make(node, &derived_instance)); - return derived_instance; - }); + [](const FunctionNode& node) { return MultiplyHolder::Make(node); }); }); } diff --git a/cpp/src/gandiva/tests/test_util.h b/cpp/src/gandiva/tests/test_util.h index a5401d938953f..502bcfbd76e17 100644 --- a/cpp/src/gandiva/tests/test_util.h +++ b/cpp/src/gandiva/tests/test_util.h @@ -105,7 +105,7 @@ std::shared_ptr TestConfigWithFunctionRegistry( // helper function to create a Configuration with an external stub function registered to // the given function registry -std::shared_ptr TestConfigWithCInterfaceFunction( +std::shared_ptr TestConfigWithCFunction( std::shared_ptr registry); // helper function to create a Configuration with an external function registered