From 45f2aa96130bd50cd599429701c306ebfe7074b5 Mon Sep 17 00:00:00 2001 From: Karthikeyan Jambu Rajaraman Date: Tue, 21 Jun 2016 16:18:07 -0700 Subject: [PATCH] Fix test and update coding style This closes #874 --- src/backend/codegen/codegen_wrapper.cc | 1 - .../codegen/const_expr_tree_generator.cc | 24 +-- src/backend/codegen/exec_eval_expr_codegen.cc | 89 ++--------- .../codegen/exec_variable_list_codegen.cc | 14 +- src/backend/codegen/expr_tree_generator.cc | 36 ++--- .../codegen/include/codegen/base_codegen.h | 3 +- .../codegen/const_expr_tree_generator.h | 22 ++- .../include/codegen/exec_eval_expr_codegen.h | 32 ++-- .../codegen/exec_variable_list_codegen.h | 4 +- .../include/codegen/expr_tree_generator.h | 43 +++++- .../include/codegen/op_expr_tree_generator.h | 33 +++- .../include/codegen/pg_arith_func_generator.h | 112 ++++++++++---- .../include/codegen/pg_func_generator.h | 138 ++++++++++++----- .../codegen/pg_func_generator_interface.h | 32 +++- .../include/codegen/utils/codegen_utils.h | 143 +++++++++++------- .../include/codegen/var_expr_tree_generator.h | 18 ++- src/backend/codegen/op_expr_tree_generator.cc | 83 +++++----- .../tests/codegen_framework_unittest.cc | 30 ++-- .../codegen/tests/codegen_utils_unittest.cc | 16 +- src/backend/codegen/utils/codegen_utils.cc | 9 +- .../codegen/var_expr_tree_generator.cc | 29 ++-- src/backend/executor/execProcnode.c | 14 +- src/include/codegen/codegen_wrapper.h | 11 +- src/test/unit/mock/gpcodegen_mock.c | 1 + 24 files changed, 576 insertions(+), 361 deletions(-) diff --git a/src/backend/codegen/codegen_wrapper.cc b/src/backend/codegen/codegen_wrapper.cc index 82612c7e9df6..34a60c9e72e9 100644 --- a/src/backend/codegen/codegen_wrapper.cc +++ b/src/backend/codegen/codegen_wrapper.cc @@ -24,7 +24,6 @@ using gpcodegen::ExecEvalExprCodegen; // Current code generator manager that oversees all code generators static void* ActiveCodeGeneratorManager = nullptr; -static bool is_codegen_initalized = false; extern bool codegen; // defined from guc extern bool init_codegen; // defined from guc diff --git a/src/backend/codegen/const_expr_tree_generator.cc b/src/backend/codegen/const_expr_tree_generator.cc index cddd528325e8..bd2a93864bb9 100644 --- a/src/backend/codegen/const_expr_tree_generator.cc +++ b/src/backend/codegen/const_expr_tree_generator.cc @@ -3,10 +3,10 @@ // Copyright (C) 2016 Pivotal Software, Inc. // // @filename: -// base_codegen.h +// const_expr_tree_generator.cc // // @doc: -// Base class for expression tree to generate code +// Object that generate code for const expression. // //--------------------------------------------------------------------------- @@ -16,8 +16,7 @@ #include "llvm/IR/Value.h" extern "C" { -#include "postgres.h" -#include "utils/elog.h" +#include "postgres.h" // NOLINT(build/include) #include "nodes/execnodes.h" } @@ -27,15 +26,17 @@ using gpcodegen::ExprTreeGenerator; bool ConstExprTreeGenerator::VerifyAndCreateExprTree( ExprState* expr_state, ExprContext* econtext, - std::unique_ptr& expr_tree) { - assert(nullptr != expr_state && nullptr != expr_state->expr && T_Const == nodeTag(expr_state->expr)); - expr_tree.reset(new ConstExprTreeGenerator(expr_state)); + std::unique_ptr* expr_tree) { + assert(nullptr != expr_state && + nullptr != expr_state->expr && + T_Const == nodeTag(expr_state->expr) && + nullptr != expr_tree); + expr_tree->reset(new ConstExprTreeGenerator(expr_state)); return true; } ConstExprTreeGenerator::ConstExprTreeGenerator(ExprState* expr_state) : ExprTreeGenerator(expr_state, ExprTreeNodeType::kConst) { - } bool ConstExprTreeGenerator::GenerateCode(CodegenUtils* codegen_utils, @@ -43,8 +44,9 @@ bool ConstExprTreeGenerator::GenerateCode(CodegenUtils* codegen_utils, llvm::Function* llvm_main_func, llvm::BasicBlock* llvm_error_block, llvm::Value* llvm_isnull_arg, - llvm::Value* & value) { - Const* const_expr = (Const*)expr_state()->expr; - value = codegen_utils->GetConstant(const_expr->constvalue); + llvm::Value** llvm_out_value) { + assert(nullptr != llvm_out_value); + Const* const_expr = reinterpret_cast(expr_state()->expr); + *llvm_out_value = codegen_utils->GetConstant(const_expr->constvalue); return true; } diff --git a/src/backend/codegen/exec_eval_expr_codegen.cc b/src/backend/codegen/exec_eval_expr_codegen.cc index ab1338b8c588..cf2d59729658 100644 --- a/src/backend/codegen/exec_eval_expr_codegen.cc +++ b/src/backend/codegen/exec_eval_expr_codegen.cc @@ -38,7 +38,7 @@ #include "llvm/Support/Casting.h" extern "C" { -#include "postgres.h" +#include "postgres.h" // NOLINT(build/include) #include "utils/elog.h" #include "nodes/execnodes.h" } @@ -47,69 +47,6 @@ using gpcodegen::ExecEvalExprCodegen; constexpr char ExecEvalExprCodegen::kExecEvalExprPrefix[]; -class ElogWrapper { - public: - ElogWrapper(gpcodegen::CodegenUtils* codegen_utils) : - codegen_utils_(codegen_utils) { - SetupElog(); - } - ~ElogWrapper() { - TearDownElog(); - } - - template - void CreateElog( - llvm::Value* llvm_elevel, - llvm::Value* llvm_fmt, - V ... args ) { - - assert(NULL != llvm_elevel); - assert(NULL != llvm_fmt); - - codegen_utils_->ir_builder()->CreateCall( - llvm_elog_start_, { - codegen_utils_->GetConstant(""), // Filename - codegen_utils_->GetConstant(0), // line number - codegen_utils_->GetConstant("") // function name - }); - codegen_utils_->ir_builder()->CreateCall( - llvm_elog_finish_, { - llvm_elevel, - llvm_fmt, - args... - }); - } - template - void CreateElog( - int elevel, - const char* fmt, - V ... args ) { - CreateElog(codegen_utils_->GetConstant(elevel), - codegen_utils_->GetConstant(fmt), - args...); - } - private: - llvm::Function* llvm_elog_start_; - llvm::Function* llvm_elog_finish_; - - gpcodegen::CodegenUtils* codegen_utils_; - - void SetupElog(){ - assert(codegen_utils_ != nullptr); - llvm_elog_start_ = codegen_utils_->RegisterExternalFunction(elog_start); - assert(llvm_elog_start_ != nullptr); - llvm_elog_finish_ = codegen_utils_->RegisterExternalFunction(elog_finish); - assert(llvm_elog_finish_ != nullptr); - } - - void TearDownElog(){ - llvm_elog_start_ = nullptr; - llvm_elog_finish_ = nullptr; - } - -}; - - ExecEvalExprCodegen::ExecEvalExprCodegen ( ExecEvalExprFn regular_func_ptr, @@ -119,12 +56,12 @@ ExecEvalExprCodegen::ExecEvalExprCodegen BaseCodegen(kExecEvalExprPrefix, regular_func_ptr, ptr_to_regular_func_ptr), exprstate_(exprstate), - econtext_(econtext){ + econtext_(econtext) { } bool ExecEvalExprCodegen::GenerateExecEvalExpr( - gpcodegen::CodegenUtils* codegen_utils) { + gpcodegen::GpCodegenUtils* codegen_utils) { assert(NULL != codegen_utils); if (nullptr == exprstate_ || @@ -132,9 +69,7 @@ bool ExecEvalExprCodegen::GenerateExecEvalExpr( nullptr == econtext_) { return false; } - - ElogWrapper elogwrapper(codegen_utils); - //TODO : krajaraman move to better place + // TODO(krajaraman): move to better place OpExprTreeGenerator::InitializeSupportedFunction(); llvm::Function* exec_eval_expr_func = CreateFunction( @@ -147,10 +82,6 @@ bool ExecEvalExprCodegen::GenerateExecEvalExpr( llvm::Value* llvm_isnull_arg = ArgumentByPosition(exec_eval_expr_func, 2); llvm::Value* llvm_isDone_arg = ArgumentByPosition(exec_eval_expr_func, 3); - // External functions - llvm::Function* llvm_slot_getattr = - codegen_utils->RegisterExternalFunction(slot_getattr); - // BasicBlock of function entry. llvm::BasicBlock* llvm_entry_block = codegen_utils->CreateBasicBlock( "entry", exec_eval_expr_func); @@ -161,14 +92,14 @@ bool ExecEvalExprCodegen::GenerateExecEvalExpr( irb->SetInsertPoint(llvm_entry_block); - elogwrapper.CreateElog( + codegen_utils->CreateElog( DEBUG1, "Calling codegen'ed expression evaluation"); // Check if we can codegen. If so create ExprTreeGenerator std::unique_ptr expr_tree_generator(nullptr); bool can_generate = ExprTreeGenerator::VerifyAndCreateExprTree( - exprstate_, econtext_, expr_tree_generator); + exprstate_, econtext_, &expr_tree_generator); if (!can_generate || expr_tree_generator.get() == nullptr) { return false; @@ -181,7 +112,7 @@ bool ExecEvalExprCodegen::GenerateExecEvalExpr( exec_eval_expr_func, llvm_error_block, llvm_isnull_arg, - value); + &value); if (!is_generated || nullptr == value) { return false; @@ -192,19 +123,17 @@ bool ExecEvalExprCodegen::GenerateExecEvalExpr( irb->SetInsertPoint(llvm_error_block); irb->CreateRet(codegen_utils->GetConstant(0)); - exec_eval_expr_func->dump(); return true; } -bool ExecEvalExprCodegen::GenerateCodeInternal(CodegenUtils* codegen_utils) { +bool ExecEvalExprCodegen::GenerateCodeInternal(GpCodegenUtils* codegen_utils) { bool isGenerated = GenerateExecEvalExpr(codegen_utils); if (isGenerated) { elog(DEBUG1, "ExecEvalExpr was generated successfully!"); return true; - } - else { + } else { elog(DEBUG1, "ExecEvalExpr generation failed!"); return false; } diff --git a/src/backend/codegen/exec_variable_list_codegen.cc b/src/backend/codegen/exec_variable_list_codegen.cc index c6d009cc549a..e7378e2c2763 100644 --- a/src/backend/codegen/exec_variable_list_codegen.cc +++ b/src/backend/codegen/exec_variable_list_codegen.cc @@ -37,7 +37,7 @@ #include "llvm/Support/Casting.h" extern "C" { -#include "postgres.h" +#include "postgres.h" // NOLINT(build/include) #include "utils/elog.h" #include "access/htup.h" #include "nodes/execnodes.h" @@ -133,14 +133,16 @@ bool ExecVariableListCodegen::GenerateExecVariableList( "fallback", exec_variable_list_func); // External functions - llvm::Function* llvm_memset = codegen_utils->GetOrRegisterExternalFunction(memset); + llvm::Function* llvm_memset = codegen_utils->GetOrRegisterExternalFunction( + memset); // Generation-time constants llvm::Value* llvm_max_attr = codegen_utils->GetConstant(max_attr); llvm::Value* llvm_slot = codegen_utils->GetConstant(slot_); // Function arguments to ExecVariableList - llvm::Value* llvm_projInfo_arg = ArgumentByPosition(exec_variable_list_func, 0); + llvm::Value* llvm_projInfo_arg = ArgumentByPosition(exec_variable_list_func, + 0); llvm::Value* llvm_values_arg = ArgumentByPosition(exec_variable_list_func, 1); llvm::Value* llvm_isnull_arg = ArgumentByPosition(exec_variable_list_func, 2); @@ -404,7 +406,6 @@ bool ExecVariableListCodegen::GenerateExecVariableList( irb->SetInsertPoint(is_not_null_block); } // End of if ( !thisatt->attnotnull ) - // TODO : hardikar & nikos verify compatibility with 8.3 merge off = att_align_nominal(off, thisatt->attalign); // values[attnum] = fetchatt(thisatt, tp + off) {{{ @@ -487,8 +488,9 @@ bool ExecVariableListCodegen::GenerateExecVariableList( llvm::Value* llvm_slot_PRIVATE_tts_off_ptr /* long* */ = codegen_utils->GetPointerToMember( llvm_slot, &TupleTableSlot::PRIVATE_tts_off); - irb->CreateStore(codegen_utils->GetConstant(off), - llvm_slot_PRIVATE_tts_off_ptr); + irb->CreateStore( + codegen_utils->GetConstant(off), // NOLINT(runtime/int) + llvm_slot_PRIVATE_tts_off_ptr); // slot->PRIVATE_tts_nvalid = attnum; irb->CreateStore(llvm_max_attr, llvm_slot_PRIVATE_tts_nvalid_ptr); diff --git a/src/backend/codegen/expr_tree_generator.cc b/src/backend/codegen/expr_tree_generator.cc index c02d0048a391..344d7826bfa9 100644 --- a/src/backend/codegen/expr_tree_generator.cc +++ b/src/backend/codegen/expr_tree_generator.cc @@ -3,7 +3,7 @@ // Copyright (C) 2016 Pivotal Software, Inc. // // @filename: -// base_codegen.h +// expr_tree_generator.cc // // @doc: // Base class for expression tree to generate code @@ -18,7 +18,7 @@ #include "llvm/IR/Value.h" extern "C" { -#include "postgres.h" +#include "postgres.h" // NOLINT(build/include) #include "utils/elog.h" #include "nodes/execnodes.h" } @@ -28,35 +28,35 @@ using gpcodegen::ExprTreeGenerator; bool ExprTreeGenerator::VerifyAndCreateExprTree( ExprState* expr_state, ExprContext* econtext, - std::unique_ptr& expr_tree) { - assert(nullptr != expr_state && nullptr != expr_state->expr); - expr_tree.reset(nullptr); + std::unique_ptr* expr_tree) { + assert(nullptr != expr_state && + nullptr != expr_state->expr && + nullptr != expr_tree); + expr_tree->reset(nullptr); bool supported_expr_tree = false; - switch(nodeTag(expr_state->expr)) { + switch (nodeTag(expr_state->expr)) { case T_OpExpr: { - supported_expr_tree = OpExprTreeGenerator::VerifyAndCreateExprTree(expr_state, - econtext, - expr_tree); + supported_expr_tree = OpExprTreeGenerator::VerifyAndCreateExprTree( + expr_state, econtext, expr_tree); break; } case T_Var: { - supported_expr_tree = VarExprTreeGenerator::VerifyAndCreateExprTree(expr_state, - econtext, - expr_tree); + supported_expr_tree = VarExprTreeGenerator::VerifyAndCreateExprTree( + expr_state, econtext, expr_tree); break; } case T_Const: { - supported_expr_tree = ConstExprTreeGenerator::VerifyAndCreateExprTree(expr_state, - econtext, - expr_tree); + supported_expr_tree = ConstExprTreeGenerator::VerifyAndCreateExprTree( + expr_state, econtext, expr_tree); break; } default : { supported_expr_tree = false; - elog(DEBUG1, "Unsupported expression tree %d found", nodeTag(expr_state->expr)); + elog(DEBUG1, "Unsupported expression tree %d found", + nodeTag(expr_state->expr)); } } - assert((!supported_expr_tree && nullptr == expr_tree.get()) || - (supported_expr_tree && nullptr != expr_tree.get())); + assert((!supported_expr_tree && nullptr == expr_tree->get()) || + (supported_expr_tree && nullptr != expr_tree->get())); return supported_expr_tree; } diff --git a/src/backend/codegen/include/codegen/base_codegen.h b/src/backend/codegen/include/codegen/base_codegen.h index 9d0d75485528..f044be92033f 100644 --- a/src/backend/codegen/include/codegen/base_codegen.h +++ b/src/backend/codegen/include/codegen/base_codegen.h @@ -180,7 +180,8 @@ class BaseCodegen: public CodegenInterface { * @param codegen_utils Utility to ease the code generation process. * @return true on successful generation. **/ - virtual bool GenerateCodeInternal(gpcodegen::GpCodegenUtils* codegen_utils) = 0; + virtual bool GenerateCodeInternal( + gpcodegen::GpCodegenUtils* codegen_utils) = 0; /** * @brief Create llvm Function for given type and store the function pointer diff --git a/src/backend/codegen/include/codegen/const_expr_tree_generator.h b/src/backend/codegen/include/codegen/const_expr_tree_generator.h index 6294ac21e30d..2b3c47475e33 100644 --- a/src/backend/codegen/include/codegen/const_expr_tree_generator.h +++ b/src/backend/codegen/include/codegen/const_expr_tree_generator.h @@ -3,10 +3,10 @@ // Copyright (C) 2016 Pivotal Software, Inc. // // @filename: -// base_codegen.h +// const_expr_tree_generator.h // // @doc: -// Base class for expression tree to generate code +// Object that generate code for const expression. // //--------------------------------------------------------------------------- #ifndef GPCODEGEN_CONST_EXPR_TREE_GENERATOR_H_ // NOLINT(build/header_guard) @@ -22,22 +22,30 @@ namespace gpcodegen { * @{ */ +/** + * @brief Object that generate code for const expression. + **/ class ConstExprTreeGenerator : public ExprTreeGenerator { public: static bool VerifyAndCreateExprTree( - ExprState* expr_state, - ExprContext* econtext, - std::unique_ptr& expr_tree); + ExprState* expr_state, + ExprContext* econtext, + std::unique_ptr* expr_tree); bool GenerateCode(gpcodegen::CodegenUtils* codegen_utils, ExprContext* econtext, llvm::Function* llvm_main_func, llvm::BasicBlock* llvm_error_block, llvm::Value* llvm_isnull_arg, - llvm::Value* & value) final; + llvm::Value** llvm_out_value) final; protected: - ConstExprTreeGenerator(ExprState* expr_state); + /** + * @brief Constructor. + * + * @param expr_state Expression state from epxression tree + **/ + explicit ConstExprTreeGenerator(ExprState* expr_state); }; /** @} */ diff --git a/src/backend/codegen/include/codegen/exec_eval_expr_codegen.h b/src/backend/codegen/include/codegen/exec_eval_expr_codegen.h index 8123f9adce3f..2efa9efb4a6a 100644 --- a/src/backend/codegen/include/codegen/exec_eval_expr_codegen.h +++ b/src/backend/codegen/include/codegen/exec_eval_expr_codegen.h @@ -27,18 +27,19 @@ class ExecEvalExprCodegen: public BaseCodegen { /** * @brief Constructor * - * @param regular_func_ptr Regular version of the target function. - * @param ptr_to_chosen_func_ptr Reference to the function pointer that the caller will call. - * @param exprstate The ExprState to use for generating code. + * @param regular_func_ptr Regular version of the target function. + * @param ptr_to_chosen_func_ptr Reference to the function pointer that the + * caller will call. + * @param exprstate The ExprState to use for generating code. * - * @note The ptr_to_chosen_func_ptr can refer to either the generated function or the - * corresponding regular version. + * @note The ptr_to_chosen_func_ptr can refer to either the generated + * function or the corresponding regular version. * **/ explicit ExecEvalExprCodegen(ExecEvalExprFn regular_func_ptr, - ExecEvalExprFn* ptr_to_regular_func_ptr, - ExprState *exprstate, - ExprContext *econtext); + ExecEvalExprFn* ptr_to_regular_func_ptr, + ExprState *exprstate, + ExprContext *econtext); virtual ~ExecEvalExprCodegen() = default; @@ -50,9 +51,18 @@ class ExecEvalExprCodegen: public BaseCodegen { * * @return true on successful generation; false otherwise. * - * @note Currently, it simply falls back to regular function for expression evaluation. + * @note Walks down expression tree and create respective ExprTreeGenerator + * to generate code. + * + * This implementation does not support: + * (1) Null attributes + * (2) Variable length attributes + * + * If at execution time, we see any of the above types of attributes, + * we fall backs to the regular function. + * */ - bool GenerateCodeInternal(gpcodegen::CodegenUtils* codegen_utils) final; + bool GenerateCodeInternal(gpcodegen::GpCodegenUtils* codegen_utils) final; private: ExprState *exprstate_; @@ -66,7 +76,7 @@ class ExecEvalExprCodegen: public BaseCodegen { * @param codegen_utils Utility to ease the code generation process. * @return true on successful generation. **/ - bool GenerateExecEvalExpr(gpcodegen::CodegenUtils* codegen_utils); + bool GenerateExecEvalExpr(gpcodegen::GpCodegenUtils* codegen_utils); }; /** @} */ diff --git a/src/backend/codegen/include/codegen/exec_variable_list_codegen.h b/src/backend/codegen/include/codegen/exec_variable_list_codegen.h index 80d2cfb1df2a..c80ce0491a99 100644 --- a/src/backend/codegen/include/codegen/exec_variable_list_codegen.h +++ b/src/backend/codegen/include/codegen/exec_variable_list_codegen.h @@ -73,8 +73,8 @@ class ExecVariableListCodegen: public BaseCodegen { * (2) Variable length attributes * (3) Attributes passed by reference * - * If at execution time, we see any of the above types of attributes, we fall backs to - * the regular function. + * If at execution time, we see any of the above types of attributes, + * we fall backs to the regular function. */ bool GenerateCodeInternal(gpcodegen::GpCodegenUtils* codegen_utils) final; diff --git a/src/backend/codegen/include/codegen/expr_tree_generator.h b/src/backend/codegen/include/codegen/expr_tree_generator.h index 2f165004010c..5fa1d8c63d49 100644 --- a/src/backend/codegen/include/codegen/expr_tree_generator.h +++ b/src/backend/codegen/include/codegen/expr_tree_generator.h @@ -3,7 +3,7 @@ // Copyright (C) 2016 Pivotal Software, Inc. // // @filename: -// base_codegen.h +// expr_tree_generator.h // // @doc: // Base class for expression tree to generate code @@ -43,23 +43,58 @@ enum class ExprTreeNodeType { class ExprTreeGenerator { public: + virtual ~ExprTreeGenerator() = default; + + /** + * @brief Verify if we support the given expression tree, and create an + * instance of ExprTreeGenerator if supported. + * + * @param expr_state Expression state from expression tree. + * @param econtext Respective expression context. + * @param expr_tree Hold the new instance of expression tree class. + * + * @return true when it can codegen otherwise it return false. + **/ static bool VerifyAndCreateExprTree( ExprState* expr_state, ExprContext* econtext, - std::unique_ptr& expr_tree); + std::unique_ptr* expr_tree); + /** + * @brief Generate the code for given expression. + * + * @param codegen_utils Utility to easy code generation. + * @param econtext Respective expression context. + * @param llvm_main_func Current function for which we are generating code + * @param llvm_isnull_arg Set to true if current expr is null + * @param llvm_out_value Store the expression results + * + * @return true when it generated successfully otherwise it return false. + **/ virtual bool GenerateCode(gpcodegen::CodegenUtils* codegen_utils, ExprContext* econtext, llvm::Function* llvm_main_func, llvm::BasicBlock* llvm_error_block, llvm::Value* llvm_isnull_arg, - llvm::Value* & value) = 0; + llvm::Value** value) = 0; + + /** + * @return Expression state + **/ ExprState* expr_state() { return expr_state_; } -protected: + + protected: + /** + * @brief Constructor. + * + * @param expr_state Expression state + * @param node_type Type of the ExprTreeGenerator + **/ ExprTreeGenerator(ExprState* expr_state, ExprTreeNodeType node_type) : expr_state_(expr_state), node_type_(node_type) {} + private: ExprState* expr_state_; ExprTreeNodeType node_type_; diff --git a/src/backend/codegen/include/codegen/op_expr_tree_generator.h b/src/backend/codegen/include/codegen/op_expr_tree_generator.h index cd3c5f3d3870..29154c7369c3 100644 --- a/src/backend/codegen/include/codegen/op_expr_tree_generator.h +++ b/src/backend/codegen/include/codegen/op_expr_tree_generator.h @@ -3,15 +3,17 @@ // Copyright (C) 2016 Pivotal Software, Inc. // // @filename: -// base_codegen.h +// op_expr_tree_generator.h // // @doc: -// Base class for expression tree to generate code +// Object that generate code for operator expression. // //--------------------------------------------------------------------------- #ifndef GPCODEGEN_OP_EXPR_TREE_GENERATOR_H_ // NOLINT(build/header_guard) #define GPCODEGEN_OP_EXPR_TREE_GENERATOR_H_ +#include + #include "codegen/expr_tree_generator.h" #include "codegen/pg_func_generator_interface.h" #include "codegen/pg_func_generator.h" @@ -28,25 +30,44 @@ namespace gpcodegen { using CodeGenFuncMap = std::unordered_map>; +/** + * @brief Object that generate code for operator expression. + **/ class OpExprTreeGenerator : public ExprTreeGenerator { public: + /** + * @brief Initialize PG operator function that we support for code generation. + **/ static void InitializeSupportedFunction(); + static bool VerifyAndCreateExprTree( ExprState* expr_state, ExprContext* econtext, - std::unique_ptr& expr_tree); + std::unique_ptr* expr_tree); bool GenerateCode(gpcodegen::CodegenUtils* codegen_utils, ExprContext* econtext, llvm::Function* llvm_main_func, llvm::BasicBlock* llvm_error_block, llvm::Value* llvm_isnull_arg, - llvm::Value* & value) final; + llvm::Value** llvm_out_value) final; + protected: - OpExprTreeGenerator(ExprState* expr_state, - std::vector>& arguments); + /** + * @brief Constructor. + * + * @param expr_state Expression state + * @param arguments Arguments to operator as list of ExprTreeGenerator + **/ + OpExprTreeGenerator( + ExprState* expr_state, + std::vector< + std::unique_ptr< + ExprTreeGenerator>>&& arguments); // NOLINT(build/c++11) + private: std::vector> arguments_; + // Map of supported function with respective generator to generate code static CodeGenFuncMap supported_function_; }; diff --git a/src/backend/codegen/include/codegen/pg_arith_func_generator.h b/src/backend/codegen/include/codegen/pg_arith_func_generator.h index 1da0c98b003b..71760a1c0b7a 100644 --- a/src/backend/codegen/include/codegen/pg_arith_func_generator.h +++ b/src/backend/codegen/include/codegen/pg_arith_func_generator.h @@ -3,10 +3,10 @@ // Copyright (C) 2016 Pivotal Software, Inc. // // @filename: -// base_codegen.h +// pg_arith_func_generator.h // // @doc: -// Base class for expression tree to generate code +// Class with Static member function to generate code for +, - and * operator // //--------------------------------------------------------------------------- #ifndef GPCODEGEN_PG_ARITH_FUNC_GENERATOR_H_ // NOLINT(build/header_guard) @@ -26,34 +26,86 @@ namespace gpcodegen { /** \addtogroup gpcodegen * @{ */ + +/** + * @brief Class with Static member function to generate code for +, - and * + * operator + * + * @tparam rtype Return type of function + * @tparam Arg0 First argument's type + * @tparam Arg1 Second argument's type + **/ template class PGArithFuncGenerator { public: + /** + * @brief Create LLVM Mul instruction with check for overflow + * + * @param codegen_utils Utility to easy code generation. + * @param llvm_main_func Current function for which we are generating code + * @param llvm_error_block Basic Block to jump when error happens + * @param llvm_args Vector of llvm arguments for the function + * @param llvm_out_value Store the results of function + * + * @return true if generation was successful otherwise return false + * + * @note If there is overflow, it will do elog::ERROR and then jump to given + * error block. + **/ static bool MulWithOverflow(gpcodegen::CodegenUtils* codegen_utils, llvm::Function* llvm_main_func, llvm::BasicBlock* llvm_error_block, - std::vector& llvm_args, - llvm::Value* & llvm_out_value); - + const std::vector& llvm_args, + llvm::Value** llvm_out_value); + + /** + * @brief Create LLVM Add instruction with check for overflow + * + * @param codegen_utils Utility to easy code generation. + * @param llvm_main_func Current function for which we are generating code + * @param llvm_error_block Basic Block to jump when error happens + * @param llvm_args Vector of llvm arguments for the function + * @param llvm_out_value Store the results of function + * + * @return true if generation was successful otherwise return false + * + * @note If there is overflow, it will do elog::ERROR and then jump to given + * error block. + **/ static bool AddWithOverflow(gpcodegen::CodegenUtils* codegen_utils, llvm::Function* llvm_main_func, llvm::BasicBlock* llvm_error_block, - std::vector& llvm_args, - llvm::Value* & llvm_out_value); - + const std::vector& llvm_args, + llvm::Value** llvm_out_value); + /** + * @brief Create LLVM Sub instruction with check for overflow + * + * @param codegen_utils Utility to easy code generation. + * @param llvm_main_func Current function for which we are generating code + * @param llvm_error_block Basic Block to jump when error happens + * @param llvm_args Vector of llvm arguments for the function + * @param llvm_out_value Store the results of function + * + * @return true if generation was successful otherwise return false + * + * @note If there is overflow, it will do elog::ERROR and then jump to given + * error block. + **/ static bool SubWithOverflow(gpcodegen::CodegenUtils* codegen_utils, llvm::Function* llvm_main_func, llvm::BasicBlock* llvm_error_block, - std::vector& llvm_args, - llvm::Value* & llvm_out_value); + const std::vector& llvm_args, + llvm::Value** llvm_out_value); }; template -bool PGArithFuncGenerator::MulWithOverflow(gpcodegen::CodegenUtils* codegen_utils, - llvm::Function* llvm_main_func, - llvm::BasicBlock* llvm_error_block, - std::vector& llvm_args, - llvm::Value* & llvm_out_value) { +bool PGArithFuncGenerator::MulWithOverflow( + gpcodegen::CodegenUtils* codegen_utils, + llvm::Function* llvm_main_func, + llvm::BasicBlock* llvm_error_block, + const std::vector& llvm_args, + llvm::Value** llvm_out_value) { + assert(nullptr != llvm_out_value); // Assumed caller checked vector size and nullptr for codegen_utils llvm::BasicBlock* llvm_non_overflow_block = codegen_utils->CreateBasicBlock( "mul_non_overflow_block", llvm_main_func); @@ -65,17 +117,17 @@ bool PGArithFuncGenerator::MulWithOverflow(gpcodegen::Codegen llvm::IRBuilder<>* irb = codegen_utils->ir_builder(); - llvm_out_value = irb->CreateExtractValue(llvm_mul_output, 0); + *llvm_out_value = irb->CreateExtractValue(llvm_mul_output, 0); llvm::Value* llvm_overflow_flag = irb->CreateExtractValue(llvm_mul_output, 1); irb->CreateCondBr( irb->CreateICmpEQ(llvm_overflow_flag, codegen_utils->GetConstant(true)), llvm_overflow_block, - llvm_non_overflow_block ); + llvm_non_overflow_block); irb->SetInsertPoint(llvm_overflow_block); - // TODO : krajaraman Elog::ERROR after ElogWrapper integrad. + // TODO(krajaraman): Elog::ERROR after ElogWrapper integrated. irb->CreateBr(llvm_error_block); irb->SetInsertPoint(llvm_non_overflow_block); @@ -88,9 +140,9 @@ bool PGArithFuncGenerator::AddWithOverflow( gpcodegen::CodegenUtils* codegen_utils, llvm::Function* llvm_main_func, llvm::BasicBlock* llvm_error_block, - std::vector& llvm_args, - llvm::Value* & llvm_out_value) { - + const std::vector& llvm_args, + llvm::Value** llvm_out_value) { + assert(nullptr != llvm_out_value); // Assumed caller checked vector size and nullptr for codegen_utils llvm::BasicBlock* llvm_non_overflow_block = codegen_utils->CreateBasicBlock( "add_non_overflow_block", llvm_main_func); @@ -102,17 +154,17 @@ bool PGArithFuncGenerator::AddWithOverflow( llvm::IRBuilder<>* irb = codegen_utils->ir_builder(); - llvm_out_value = irb->CreateExtractValue(llvm_mul_output, 0); + *llvm_out_value = irb->CreateExtractValue(llvm_mul_output, 0); llvm::Value* llvm_overflow_flag = irb->CreateExtractValue(llvm_mul_output, 1); irb->CreateCondBr( irb->CreateICmpEQ(llvm_overflow_flag, codegen_utils->GetConstant(true)), llvm_overflow_block, - llvm_non_overflow_block ); + llvm_non_overflow_block); irb->SetInsertPoint(llvm_overflow_block); - // TODO : krajaraman Elog::ERROR after ElogWrapper integrad. + // TODO(krajaraman): Elog::ERROR after ElogWrapper integrated. irb->CreateBr(llvm_error_block); irb->SetInsertPoint(llvm_non_overflow_block); @@ -125,9 +177,9 @@ bool PGArithFuncGenerator::SubWithOverflow( gpcodegen::CodegenUtils* codegen_utils, llvm::Function* llvm_main_func, llvm::BasicBlock* llvm_error_block, - std::vector& llvm_args, - llvm::Value* & llvm_out_value) { - + const std::vector& llvm_args, + llvm::Value** llvm_out_value) { + assert(nullptr != llvm_out_value); // Assumed caller checked vector size and nullptr for codegen_utils llvm::BasicBlock* llvm_non_overflow_block = codegen_utils->CreateBasicBlock( "sub_non_overflow_block", llvm_main_func); @@ -139,17 +191,17 @@ bool PGArithFuncGenerator::SubWithOverflow( llvm::IRBuilder<>* irb = codegen_utils->ir_builder(); - llvm_out_value = irb->CreateExtractValue(llvm_mul_output, 0); + *llvm_out_value = irb->CreateExtractValue(llvm_mul_output, 0); llvm::Value* llvm_overflow_flag = irb->CreateExtractValue(llvm_mul_output, 1); irb->CreateCondBr( irb->CreateICmpEQ(llvm_overflow_flag, codegen_utils->GetConstant(true)), llvm_overflow_block, - llvm_non_overflow_block ); + llvm_non_overflow_block); irb->SetInsertPoint(llvm_overflow_block); - // TODO : krajaraman Elog::ERROR after ElogWrapper integrad. + // TODO(krajaraman): Elog::ERROR after ElogWrapper integrated. irb->CreateBr(llvm_error_block); irb->SetInsertPoint(llvm_non_overflow_block); diff --git a/src/backend/codegen/include/codegen/pg_func_generator.h b/src/backend/codegen/include/codegen/pg_func_generator.h index 2eb6b8e70976..8dfcc8ae9bae 100644 --- a/src/backend/codegen/include/codegen/pg_func_generator.h +++ b/src/backend/codegen/include/codegen/pg_func_generator.h @@ -3,10 +3,10 @@ // Copyright (C) 2016 Pivotal Software, Inc. // // @filename: -// base_codegen.h +// pg_func_generator.h // // @doc: -// Base class for expression tree to generate code +// Object to manage code generation for Greenplum's function // //--------------------------------------------------------------------------- #ifndef GPCODEGEN_PG_FUNC_GENERATOR_H_ // NOLINT(build/header_guard) @@ -28,105 +28,163 @@ namespace gpcodegen { * @{ */ -typedef bool (*ExprCodeGenerator)(gpcodegen::CodegenUtils* codegen_utils, +typedef bool (*PGFuncGenerator)(gpcodegen::CodegenUtils* codegen_utils, llvm::Function* llvm_main_func, llvm::BasicBlock* llvm_error_block, - std::vector& llvm_args, - llvm::Value* & llvm_out_value); - - - + const std::vector& llvm_args, + llvm::Value** llvm_out_value); + + +/** + * @brief Base class for Greenplum Function generator. + * + * @tparam FuncPtrType Function type that generate code + * @tparam Arg0 First Argument CppType + * @tparam Arg1 Second Argument CppType + * + **/ +// TODO(krajaraman) : Make Variadic template to take +// variable length argument instead of two arguments template -class PGFuncGenerator : public PGFuncGeneratorInterface { +class PGFuncBaseGenerator : public PGFuncGeneratorInterface { public: + virtual ~PGFuncBaseGenerator() = default; std::string GetName() final { return pg_func_name_; }; size_t GetTotalArgCount() final { return 2; } protected: - - // TODO : krajaraman - // For now treating all argument. Later make template to take - // variable length instead of two arguments + /** + * @brief Check for expected arguments length and cast all the + * argument to given type. + * + * @param codegen_utils Utility to easy code generation. + * @param llvm_in_args Vector of llvm arguments + * @param llvm_out_args Vector of processed llvm arguments + * + * @return true on successful preprocess otherwise return false. + **/ bool PreProcessArgs(gpcodegen::CodegenUtils* codegen_utils, - std::vector& llvm_in_args, - std::vector& llvm_out_args) { - assert(nullptr != codegen_utils); + const std::vector& llvm_in_args, + std::vector* llvm_out_args) { + assert(nullptr != codegen_utils && + nullptr != llvm_out_args); if (llvm_in_args.size() != GetTotalArgCount()) { return false; } - llvm_out_args.push_back(codegen_utils->CreateCast(llvm_in_args[0])); - llvm_out_args.push_back(codegen_utils->CreateCast(llvm_in_args[1])); + llvm_out_args->push_back(codegen_utils->CreateCast(llvm_in_args[0])); + llvm_out_args->push_back(codegen_utils->CreateCast(llvm_in_args[1])); return true; } + /** + * @return Function Pointer that generate code + **/ FuncPtrType func_ptr() { return func_ptr_; } - PGFuncGenerator(int pg_func_oid, + /** + * @brief Constructor. + * + * @param pg_func_oid Greenplum function Oid + * @param pg_func_name Greenplum function name + * @param func_ptr Function pointer that generate code + **/ + PGFuncBaseGenerator(int pg_func_oid, const std::string& pg_func_name, - FuncPtrType mem_func_ptr) : pg_func_oid_(pg_func_oid), + FuncPtrType func_ptr) : pg_func_oid_(pg_func_oid), pg_func_name_(pg_func_name), - func_ptr_(mem_func_ptr) { - + func_ptr_(func_ptr) { } + private: - std::string pg_func_name_; unsigned int pg_func_oid_; + std::string pg_func_name_; FuncPtrType func_ptr_; - }; +/** + * @brief Object that use IRBuilder member function to generate code for + * given Greenplum function + * + * @tparam FuncPtrType Function type that generate code + * @tparam Arg0 First Argument CppType + * @tparam Arg1 Second Argument CppType + * + **/ template -class PGIRBuilderFuncGenerator : public PGFuncGenerator { public: + /** + * @brief Constructor. + * + * @param pg_func_oid Greenplum function Oid + * @param pg_func_name Greenplum function name + * @param mem_func_ptr IRBuilder member function pointer + **/ PGIRBuilderFuncGenerator(int pg_func_oid, const std::string& pg_func_name, - FuncPtrType func_ptr) : - PGFuncGenerator(pg_func_oid, pg_func_name, - func_ptr) { - + mem_func_ptr) { } bool GenerateCode(gpcodegen::CodegenUtils* codegen_utils, llvm::Function* llvm_main_func, llvm::BasicBlock* llvm_error_block, - std::vector& llvm_args, - llvm::Value* & llvm_out_value) final { + const std::vector& llvm_args, + llvm::Value** llvm_out_value) final { + assert(nullptr != llvm_out_value); std::vector llvm_preproc_args; - if (!this->PreProcessArgs(codegen_utils, llvm_args, llvm_preproc_args)) { + if (!this->PreProcessArgs(codegen_utils, llvm_args, &llvm_preproc_args)) { return false; } llvm::IRBuilder<>* irb = codegen_utils->ir_builder(); - llvm_out_value = (irb->*this->func_ptr())(llvm_preproc_args[0], + *llvm_out_value = (irb->*this->func_ptr())(llvm_preproc_args[0], llvm_preproc_args[1], ""); return true; } }; +/** + * @brief Object that use static member function to generate code for + * given Greenplum's function + * + * @tparam FuncPtrType Function type that generate code + * @tparam Arg0 First Argument CppType + * @tparam Arg1 Second Argument CppType + * + **/ template -class PGGenericFuncGenerator : public PGFuncGenerator { public: + /** + * @brief Constructor. + * + * @param pg_func_oid Greenplum function Oid + * @param pg_func_name Greenplum function name + * @param func_ptr function pointer to static function + **/ PGGenericFuncGenerator(int pg_func_oid, const std::string& pg_func_name, - ExprCodeGenerator func_ptr) : - PGFuncGenerator(pg_func_oid, pg_func_name, func_ptr) { - } bool GenerateCode(gpcodegen::CodegenUtils* codegen_utils, llvm::Function* llvm_main_func, llvm::BasicBlock* llvm_error_block, - std::vector& llvm_args, - llvm::Value* & llvm_out_value) final { - assert(nullptr != codegen_utils); + const std::vector& llvm_args, + llvm::Value** llvm_out_value) final { + assert(nullptr != codegen_utils && + nullptr != llvm_out_value); if (llvm_args.size() != this->GetTotalArgCount()) { return false; } diff --git a/src/backend/codegen/include/codegen/pg_func_generator_interface.h b/src/backend/codegen/include/codegen/pg_func_generator_interface.h index f698227b17f0..f23adbc579da 100644 --- a/src/backend/codegen/include/codegen/pg_func_generator_interface.h +++ b/src/backend/codegen/include/codegen/pg_func_generator_interface.h @@ -3,10 +3,10 @@ // Copyright (C) 2016 Pivotal Software, Inc. // // @filename: -// base_codegen.h +// pg_func_generator_interface.h // // @doc: -// Base class for expression tree to generate code +// Interface for all Greenplum Function generator. // //--------------------------------------------------------------------------- #ifndef GPCODEGEN_PG_FUNC_GENERATOR_INTERFACE_H_ // NOLINT(build/header_guard) @@ -26,15 +26,39 @@ namespace gpcodegen { * @{ */ +/** + * @brief Interface for all code generators. + **/ class PGFuncGeneratorInterface { public: + virtual ~PGFuncGeneratorInterface() = default; + + /** + * @return Greenplum function name for which it generate code + **/ virtual std::string GetName() = 0; + + /** + * @return Total number of arguments for the function. + **/ virtual size_t GetTotalArgCount() = 0; + + /** + * @brief Generate the code for Greenplum function. + * + * @param codegen_utils Utility to easy code generation. + * @param llvm_main_func Current function for which we are generating code + * @param llvm_error_block Basic Block to jump when error happens + * @param llvm_args Vector of llvm arguments for the function + * @param llvm_out_value Store the results of function + * + * @return true when it generated successfully otherwise it return false. + **/ virtual bool GenerateCode(gpcodegen::CodegenUtils* codegen_utils, llvm::Function* llvm_main_func, llvm::BasicBlock* llvm_error_block, - std::vector& llvm_args, - llvm::Value* & llvm_out_value) = 0; + const std::vector& llvm_args, + llvm::Value** llvm_out_value) = 0; }; /** @} */ diff --git a/src/backend/codegen/include/codegen/utils/codegen_utils.h b/src/backend/codegen/include/codegen/utils/codegen_utils.h index 090ba8058d1f..69b136f2bc7e 100644 --- a/src/backend/codegen/include/codegen/utils/codegen_utils.h +++ b/src/backend/codegen/include/codegen/utils/codegen_utils.h @@ -271,32 +271,71 @@ class CodegenUtils { return llvm::BasicBlock::Create(context_, name, parent, nullptr); } - template + /** + * @brief Create a Cast instruction to convert given llvm::Value to given Cpp + * type + * + * @tparam CppType Destination cpp type + * @param value LLVM Value on which casting has to be done. + * + * @return LLVM Value that casted to given Cpp type. + * + * @note Depend on type's size, it will do extent or trunc or bit cast. + **/ + template llvm::Value* CreateCast(llvm::Value* value) { assert(nullptr != value); - llvm::Type* llvm_dest_type = GetType(); + llvm::Type* llvm_dest_type = GetType(); unsigned dest_size = llvm_dest_type->getScalarSizeInBits(); llvm::Type* llvm_src_type = value->getType(); unsigned src_size = llvm_src_type->getScalarSizeInBits(); if (src_size < dest_size) { return ir_builder()->CreateZExt(value, llvm_dest_type); - } - else if (src_size > dest_size) { + } else if (src_size > dest_size) { return ir_builder()->CreateTrunc(value, llvm_dest_type); - } - else if (llvm_src_type->getTypeID() != llvm_dest_type->getTypeID()) { + } else if (llvm_src_type->getTypeID() != llvm_dest_type->getTypeID()) { return ir_builder()->CreateBitCast(value, llvm_dest_type); } return value; } + /** + * @brief Use LLVM intrinsic to create Multiplication with overflow + * instruction + * + * @tparam CppType CppType for multiplication + * @param arg0 First argument + * @param arg1 Second argument + * + * @return LLVM Value as a pair of results and overflow flag. + **/ template llvm::Value* CreateMulOverflow(llvm::Value* arg0, llvm::Value* arg1); + /** + * @brief Use LLVM intrinsic to create Add with overflow + * instruction + * + * @tparam CppType CppType for add + * @param arg0 First argument + * @param arg1 Second argument + * + * @return LLVM Value as a pair of results and overflow flag. + **/ template llvm::Value* CreateAddOverflow(llvm::Value* arg0, llvm::Value* arg1); + /** + * @brief Use LLVM intrinsic to create Subtract with overflow + * instruction + * + * @tparam CppType CppType for subtract + * @param arg0 First argument + * @param arg1 Second argument + * + * @return LLVM Value as a pair of results and overflow flag. + **/ template llvm::Value* CreateSubOverflow(llvm::Value* arg0, llvm::Value* arg1); @@ -330,7 +369,6 @@ class CodegenUtils { ReturnType (*external_function)(ArgumentTypes...), const std::string& name = "", const bool is_var_arg = false) { - std::unordered_map::iterator it; bool key_absent; @@ -342,7 +380,6 @@ class CodegenUtils { if (!key_absent) { return module()->getFunction(it->second); } else { - if (!name.empty()) { RecordNamedExternalFunction(name); } @@ -516,10 +553,12 @@ class CodegenUtils { llvm::Type* cast_type, const std::size_t cumulative_offset); - llvm::Value* CreateArithOp(llvm::Intrinsic::ID Id, - llvm::ArrayRef Tys, - llvm::Value* arg0, - llvm::Value* arg1); + // Helper method to call any llvm intrinsic feature. E.g. CreateMulOverflow. + // TODO(krajaramn) : Support any number of arguments + llvm::Value* CreateIntrinsicInstrCall(llvm::Intrinsic::ID Id, + llvm::ArrayRef Tys, + llvm::Value* arg0, + llvm::Value* arg1); // Helper method for GetPointerToMember(). This variadic template recursively // consumes pointers-to-members, adding up 'cumulative_offset' and resolving @@ -1157,8 +1196,12 @@ void CodegenUtils::CreateFallback(llvm::Function* regular_function, // part of the public API. namespace codegen_utils_detail { -// ArithOpMaker has various template specializations to handle -// different C++ types. +// ArithOpMaker has various template specializations to handle different +// categories of C++ types. The specializations provide a static method +// CreateMulOverflow(), CreateAddOverflow(), CreateSubOverflow() etc.. +// that takes an CodegenUtils and arguments and call respective llvm instruction +// for given CppType. +// The base version of this template is empty. template class ArithOpMaker { }; @@ -1178,10 +1221,11 @@ std::is_integral::value llvm::Value* casted_arg0 = generator->CreateCast(arg0); llvm::Value* casted_arg1 = generator->CreateCast(arg1); - return generator->CreateArithOp(llvm::Intrinsic::uadd_with_overflow, - generator->GetType(), - casted_arg0, - casted_arg1); + return generator->CreateIntrinsicInstrCall( + llvm::Intrinsic::uadd_with_overflow, + generator->GetType(), + casted_arg0, + casted_arg1); } static llvm::Value* CreateSubOverflow(CodegenUtils* generator, llvm::Value* arg0, @@ -1190,10 +1234,11 @@ std::is_integral::value llvm::Value* casted_arg0 = generator->CreateCast(arg0); llvm::Value* casted_arg1 = generator->CreateCast(arg1); - return generator->CreateArithOp(llvm::Intrinsic::usub_with_overflow, - generator->GetType(), - casted_arg0, - casted_arg1); + return generator->CreateIntrinsicInstrCall( + llvm::Intrinsic::usub_with_overflow, + generator->GetType(), + casted_arg0, + casted_arg1); } static llvm::Value* CreateMulOverflow(CodegenUtils* generator, llvm::Value* arg0, @@ -1202,13 +1247,14 @@ std::is_integral::value llvm::Value* casted_arg0 = generator->CreateCast(arg0); llvm::Value* casted_arg1 = generator->CreateCast(arg1); - return generator->CreateArithOp(llvm::Intrinsic::umul_with_overflow, - generator->GetType(), - casted_arg0, - casted_arg1); + return generator->CreateIntrinsicInstrCall( + llvm::Intrinsic::umul_with_overflow, + generator->GetType(), + casted_arg0, + casted_arg1); } - private: + private: static void Checker(llvm::Value* arg0, llvm::Value* arg1) { assert(nullptr != arg0 && nullptr != arg0->getType()); @@ -1226,17 +1272,17 @@ typename std::enable_if< std::is_integral::value && std::is_signed::value>::type> { public: - static llvm::Value* CreateAddOverflow(CodegenUtils* generator, llvm::Value* arg0, llvm::Value* arg1) { Checker(arg0, arg1); llvm::Value* casted_arg0 = generator->CreateCast(arg0); llvm::Value* casted_arg1 = generator->CreateCast(arg1); - return generator->CreateArithOp(llvm::Intrinsic::sadd_with_overflow, - generator->GetType(), - casted_arg0, - casted_arg1); + return generator->CreateIntrinsicInstrCall( + llvm::Intrinsic::sadd_with_overflow, + generator->GetType(), + casted_arg0, + casted_arg1); } static llvm::Value* CreateSubOverflow(CodegenUtils* generator, @@ -1246,10 +1292,11 @@ std::is_integral::value llvm::Value* casted_arg0 = generator->CreateCast(arg0); llvm::Value* casted_arg1 = generator->CreateCast(arg1); - return generator->CreateArithOp(llvm::Intrinsic::ssub_with_overflow, - generator->GetType(), - casted_arg0, - casted_arg1); + return generator->CreateIntrinsicInstrCall( + llvm::Intrinsic::ssub_with_overflow, + generator->GetType(), + casted_arg0, + casted_arg1); } static llvm::Value* CreateMulOverflow(CodegenUtils* generator, @@ -1259,10 +1306,11 @@ std::is_integral::value llvm::Value* casted_arg0 = generator->CreateCast(arg0); llvm::Value* casted_arg1 = generator->CreateCast(arg1); - return generator->CreateArithOp(llvm::Intrinsic::smul_with_overflow, - generator->GetType(), - casted_arg0, - casted_arg1); + return generator->CreateIntrinsicInstrCall( + llvm::Intrinsic::smul_with_overflow, + generator->GetType(), + casted_arg0, + casted_arg1); } private: static void Checker(llvm::Value* arg0, @@ -1280,33 +1328,18 @@ class ArithOpMaker< EnumType, typename std::enable_if::value>::type> { public: - static llvm::Value* Get(CodegenUtils* generator, - llvm::Value* arg0, - llvm::Value* arg1) { - static_assert("Not support for enum"); - } }; // Explicit specialization for 32-bit float. template <> class ArithOpMaker { public: - static llvm::Value* Get(CodegenUtils* generator, - llvm::Value* arg0, - llvm::Value* arg1) { - static_assert("Not support for float"); - } }; // Explicit specialization for 64-bit double. template <> class ArithOpMaker { public: - static llvm::Value* Get(CodegenUtils* generator, - llvm::Value* arg0, - llvm::Value* arg1) { - static_assert("Not support for double"); - } }; } // namespace codegen_utils_detail diff --git a/src/backend/codegen/include/codegen/var_expr_tree_generator.h b/src/backend/codegen/include/codegen/var_expr_tree_generator.h index 31d756b90103..b5bb3d8794fa 100644 --- a/src/backend/codegen/include/codegen/var_expr_tree_generator.h +++ b/src/backend/codegen/include/codegen/var_expr_tree_generator.h @@ -3,10 +3,10 @@ // Copyright (C) 2016 Pivotal Software, Inc. // // @filename: -// base_codegen.h +// var_expr_tree_generator.h // // @doc: -// Base class for expression tree to generate code +// Object that generator code for variable expression. // //--------------------------------------------------------------------------- #ifndef GPCODEGEN_VAR_EXPR_TREE_GENERATOR_H_ // NOLINT(build/header_guard) @@ -22,21 +22,29 @@ namespace gpcodegen { * @{ */ +/** + * @brief Object that generator code for variable expression. + **/ class VarExprTreeGenerator : public ExprTreeGenerator { public: static bool VerifyAndCreateExprTree( ExprState* expr_state, ExprContext* econtext, - std::unique_ptr& expr_tree); + std::unique_ptr* expr_tree); bool GenerateCode(gpcodegen::CodegenUtils* codegen_utils, ExprContext* econtext, llvm::Function* llvm_main_func, llvm::BasicBlock* llvm_error_block, llvm::Value* llvm_isnull_arg, - llvm::Value* & value) final; + llvm::Value** llvm_out_value) final; protected: - VarExprTreeGenerator(ExprState* expr_state); + /** + * @brief Constructor. + * + * @param expr_state Expression state + **/ + explicit VarExprTreeGenerator(ExprState* expr_state); }; /** @} */ diff --git a/src/backend/codegen/op_expr_tree_generator.cc b/src/backend/codegen/op_expr_tree_generator.cc index 7865a6706bea..077619df37a7 100644 --- a/src/backend/codegen/op_expr_tree_generator.cc +++ b/src/backend/codegen/op_expr_tree_generator.cc @@ -3,10 +3,10 @@ // Copyright (C) 2016 Pivotal Software, Inc. // // @filename: -// base_codegen.h +// op_expr_tree_generator.h // // @doc: -// Base class for expression tree to generate code +// Object that generate code for operator expression. // //--------------------------------------------------------------------------- @@ -17,7 +17,7 @@ #include "llvm/IR/Value.h" extern "C" { -#include "postgres.h" +#include "postgres.h" // NOLINT(build/include) #include "utils/elog.h" #include "nodes/execnodes.h" } @@ -46,32 +46,42 @@ void OpExprTreeGenerator::InitializeSupportedFunction() { supported_function_[177] = std::unique_ptr( new PGGenericFuncGenerator( - 177, "int4pl", &PGArithFuncGenerator::AddWithOverflow)); + 177, + "int4pl", + &PGArithFuncGenerator::AddWithOverflow)); supported_function_[181] = std::unique_ptr( new PGGenericFuncGenerator( - 181, "int4mi", &PGArithFuncGenerator::SubWithOverflow)); + 181, + "int4mi", + &PGArithFuncGenerator::SubWithOverflow)); supported_function_[141] = std::unique_ptr( new PGGenericFuncGenerator( - 141, "int4mul", &PGArithFuncGenerator::MulWithOverflow)); + 141, + "int4mul", + &PGArithFuncGenerator::MulWithOverflow)); } OpExprTreeGenerator::OpExprTreeGenerator( ExprState* expr_state, - std::vector>& arguments) : - arguments_(std::move(arguments)), - ExprTreeGenerator(expr_state, ExprTreeNodeType::kOperator) { + std::vector< + std::unique_ptr>&& arguments) // NOLINT(build/c++11) + : ExprTreeGenerator(expr_state, ExprTreeNodeType::kOperator), + arguments_(std::move(arguments)) { } bool OpExprTreeGenerator::VerifyAndCreateExprTree( ExprState* expr_state, ExprContext* econtext, - std::unique_ptr& expr_tree) { - assert(nullptr != expr_state && nullptr != expr_state->expr && T_OpExpr == nodeTag(expr_state->expr)); - - OpExpr* op_expr = (OpExpr*)expr_state->expr; - expr_tree.reset(nullptr); + std::unique_ptr* expr_tree) { + assert(nullptr != expr_state && + nullptr != expr_state->expr && + T_OpExpr == nodeTag(expr_state->expr) && + nullptr != expr_tree); + + OpExpr* op_expr = reinterpret_cast(expr_state->expr); + expr_tree->reset(nullptr); CodeGenFuncMap::iterator itr = supported_function_.find(op_expr->opfuncid); if (itr == supported_function_.end()) { // Operators are stored in pg_proc table. See postgres.bki for more details. @@ -79,29 +89,22 @@ bool OpExprTreeGenerator::VerifyAndCreateExprTree( return false; } - List *arguments = ((FuncExprState *)expr_state)->args; - if (nullptr == arguments) { - elog(DEBUG1, "No argument for the function"); - return false; - } + List *arguments = reinterpret_cast(expr_state)->args; + assert(nullptr != arguments); // In ExecEvalFuncArgs - if (list_length(arguments) != itr->second->GetTotalArgCount()) { - elog(DEBUG1, "Wrong number of arguments (!= %d)", itr->second->GetTotalArgCount()); - return false; - } + assert(list_length(arguments) == itr->second->GetTotalArgCount()); ListCell *arg = nullptr; bool supported_tree = true; std::vector> expr_tree_arguments; - foreach(arg, arguments) - { + foreach(arg, arguments) { // retrieve argument's ExprState - ExprState *argstate = (ExprState *) lfirst(arg); + ExprState *argstate = reinterpret_cast(lfirst(arg)); assert(nullptr != argstate); std::unique_ptr arg(nullptr); supported_tree &= ExprTreeGenerator::VerifyAndCreateExprTree(argstate, econtext, - arg); + &arg); if (!supported_tree) { break; } @@ -111,7 +114,8 @@ bool OpExprTreeGenerator::VerifyAndCreateExprTree( if (!supported_tree) { return supported_tree; } - expr_tree.reset(new OpExprTreeGenerator(expr_state, expr_tree_arguments)); + expr_tree->reset(new OpExprTreeGenerator(expr_state, + std::move(expr_tree_arguments))); return true; } @@ -120,31 +124,34 @@ bool OpExprTreeGenerator::GenerateCode(CodegenUtils* codegen_utils, llvm::Function* llvm_main_func, llvm::BasicBlock* llvm_error_block, llvm::Value* llvm_isnull_arg, - llvm::Value* & value) { - value = nullptr; - OpExpr* op_expr = (OpExpr*)expr_state()->expr; + llvm::Value** llvm_out_value) { + assert(nullptr != llvm_out_value); + *llvm_out_value = nullptr; + OpExpr* op_expr = reinterpret_cast(expr_state()->expr); CodeGenFuncMap::iterator itr = supported_function_.find(op_expr->opfuncid); if (itr == supported_function_.end()) { - // Operators are stored in pg_proc table. See postgres.bki for more details. - elog(WARNING, "Unsupported operator %d.", op_expr->opfuncid); - return false; + // Operators are stored in pg_proc table. + // See postgres.bki for more details. + elog(WARNING, "Unsupported operator %d.", op_expr->opfuncid); + return false; } if (arguments_.size() != itr->second->GetTotalArgCount()) { - elog(WARNING, "Expected argument size to be %d\n", itr->second->GetTotalArgCount()); + elog(WARNING, "Expected argument size to be %lu\n", + itr->second->GetTotalArgCount()); return false; } bool arg_generated = true; std::vector llvm_arguments; - for(auto& arg : arguments_) { + for (auto& arg : arguments_) { llvm::Value* llvm_arg = nullptr; arg_generated &= arg->GenerateCode(codegen_utils, econtext, llvm_main_func, llvm_error_block, llvm_isnull_arg, - llvm_arg); + &llvm_arg); if (!arg_generated) { return false; } @@ -154,5 +161,5 @@ bool OpExprTreeGenerator::GenerateCode(CodegenUtils* codegen_utils, llvm_main_func, llvm_error_block, llvm_arguments, - value); + llvm_out_value); } diff --git a/src/backend/codegen/tests/codegen_framework_unittest.cc b/src/backend/codegen/tests/codegen_framework_unittest.cc index aa9a4589d0f4..1d2885b7e993 100644 --- a/src/backend/codegen/tests/codegen_framework_unittest.cc +++ b/src/backend/codegen/tests/codegen_framework_unittest.cc @@ -124,7 +124,7 @@ class MulOverflowCodeGenerator : public BaseCodegen { virtual ~MulOverflowCodeGenerator() = default; protected: - bool GenerateCodeInternal(gpcodegen::CodegenUtils* codegen_utils) final { + bool GenerateCodeInternal(gpcodegen::GpCodegenUtils* codegen_utils) final { llvm::Function* mul2_func = CreateFunction(codegen_utils, GetUniqueFuncName()); llvm::BasicBlock* mul2_body = codegen_utils->CreateBasicBlock("body", @@ -138,20 +138,25 @@ class MulOverflowCodeGenerator : public BaseCodegen { llvm::Value* arg0 = ArgumentByPosition(mul2_func, 0); llvm::Value* arg1 = ArgumentByPosition(mul2_func, 1); - llvm::Function* llvm_mul_overflow = llvm::Intrinsic::getDeclaration(codegen_utils->module(), - llvm::Intrinsic::smul_with_overflow, - arg0->getType()); - llvm::Value* llvm_umul_results = codegen_utils->ir_builder()->CreateCall(llvm_mul_overflow, {arg0, arg1}); - llvm::Value* llvm_overflow_flag = codegen_utils->ir_builder()->CreateExtractValue(llvm_umul_results, 1); - llvm::Value* llvm_results = codegen_utils->ir_builder()->CreateExtractValue(llvm_umul_results, 0); + llvm::Function* llvm_mul_overflow = llvm::Intrinsic::getDeclaration( + codegen_utils->module(), + llvm::Intrinsic::smul_with_overflow, + arg0->getType()); + llvm::Value* llvm_umul_results = codegen_utils->ir_builder()->CreateCall( + llvm_mul_overflow, {arg0, arg1}); + llvm::Value* llvm_overflow_flag = codegen_utils->ir_builder()-> + CreateExtractValue(llvm_umul_results, 1); + llvm::Value* llvm_results = codegen_utils->ir_builder()->CreateExtractValue( + llvm_umul_results, 0); codegen_utils->ir_builder()->CreateCondBr( - codegen_utils->ir_builder()->CreateICmpSLE(llvm_overflow_flag, codegen_utils->GetConstant(true)), - return_overflow_block, - result_result_block ); + codegen_utils->ir_builder()->CreateICmpSLE( + llvm_overflow_flag, codegen_utils->GetConstant(true)), + return_overflow_block, + result_result_block); codegen_utils->ir_builder()->SetInsertPoint(return_overflow_block); - codegen_utils->ir_builder()->CreateAdd(llvm_results, codegen_utils->GetConstant(1)); + codegen_utils->ir_builder()->CreateRet(codegen_utils->GetConstant(1)); codegen_utils->ir_builder()->SetInsertPoint(result_result_block); codegen_utils->ir_builder()->CreateRet(llvm_results); @@ -429,7 +434,8 @@ TEST_F(CodegenManagerTest, UnCompilablePassedGenerationTest) { TEST_F(CodegenManagerTest, MulOverFlowTest) { // Test if generation happens successfully mul_func_ptr = nullptr; - EnrollCodegen(MulFuncRegular, &mul_func_ptr); + EnrollCodegen(MulFuncRegular, + &mul_func_ptr); EXPECT_EQ(1, manager_->GenerateCode()); diff --git a/src/backend/codegen/tests/codegen_utils_unittest.cc b/src/backend/codegen/tests/codegen_utils_unittest.cc index 0a11fa861386..eb0f291cf3f1 100644 --- a/src/backend/codegen/tests/codegen_utils_unittest.cc +++ b/src/backend/codegen/tests/codegen_utils_unittest.cc @@ -2739,21 +2739,25 @@ TEST_F(CodegenUtilsTest, CppClassObjectTest) { // Test GetOrGetOrRegisterExternalFunction to return the right llvm::Function if // previously registered or else register it anew TEST_F(CodegenUtilsTest, GetOrGetOrRegisterExternalFunctionTest) { - // Test previous unregistered function EXPECT_EQ(nullptr, codegen_utils_->module()->getFunction("floor")); - llvm::Function* floor_func = codegen_utils_->GetOrRegisterExternalFunction(floor, "floor"); + llvm::Function* floor_func = codegen_utils_->GetOrRegisterExternalFunction( + floor, "floor"); EXPECT_EQ(floor_func, codegen_utils_->module()->getFunction("floor")); // Test previous registered Non vararg function - llvm::Function* expected_fabs_func = codegen_utils_->GetOrRegisterExternalFunction(ceil); - llvm::Function* fabs_func = codegen_utils_->GetOrRegisterExternalFunction(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_->GetOrRegisterExternalFunction(vprintf); - llvm::Function* vprintf_func = codegen_utils_->GetOrRegisterExternalFunction(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); } diff --git a/src/backend/codegen/utils/codegen_utils.cc b/src/backend/codegen/utils/codegen_utils.cc index 4f1184ae0a5f..89ef02e6f72a 100644 --- a/src/backend/codegen/utils/codegen_utils.cc +++ b/src/backend/codegen/utils/codegen_utils.cc @@ -358,10 +358,11 @@ llvm::Value* CodegenUtils::GetPointerToMemberImpl( } } -llvm::Value* CodegenUtils::CreateArithOp(llvm::Intrinsic::ID Id, - llvm::ArrayRef Tys, - llvm::Value* arg0, - llvm::Value* arg1) { +llvm::Value* CodegenUtils::CreateIntrinsicInstrCall( + llvm::Intrinsic::ID Id, + llvm::ArrayRef Tys, + llvm::Value* arg0, + llvm::Value* arg1) { llvm::Function* llvm_intr_func = llvm::Intrinsic::getDeclaration(module(), Id, Tys); diff --git a/src/backend/codegen/var_expr_tree_generator.cc b/src/backend/codegen/var_expr_tree_generator.cc index 3f2d9504e576..f70b0e892555 100644 --- a/src/backend/codegen/var_expr_tree_generator.cc +++ b/src/backend/codegen/var_expr_tree_generator.cc @@ -3,10 +3,10 @@ // Copyright (C) 2016 Pivotal Software, Inc. // // @filename: -// base_codegen.h +// var_expr_tree_generator.cc // // @doc: -// Base class for expression tree to generate code +// Object that generator code for variable expression. // //--------------------------------------------------------------------------- @@ -16,7 +16,7 @@ #include "llvm/IR/Value.h" extern "C" { -#include "postgres.h" +#include "postgres.h" // NOLINT(build/include) #include "utils/elog.h" #include "nodes/execnodes.h" } @@ -28,9 +28,12 @@ using gpcodegen::CodegenUtils; bool VarExprTreeGenerator::VerifyAndCreateExprTree( ExprState* expr_state, ExprContext* econtext, - std::unique_ptr& expr_tree) { - assert(nullptr != expr_state && nullptr != expr_state->expr && T_Var == nodeTag(expr_state->expr)); - expr_tree.reset(new VarExprTreeGenerator(expr_state)); + std::unique_ptr* expr_tree) { + assert(nullptr != expr_state && + nullptr != expr_state->expr && + T_Var == nodeTag(expr_state->expr) && + nullptr != expr_tree); + expr_tree->reset(new VarExprTreeGenerator(expr_state)); return true; } @@ -43,9 +46,10 @@ bool VarExprTreeGenerator::GenerateCode(CodegenUtils* codegen_utils, llvm::Function* llvm_main_func, llvm::BasicBlock* llvm_error_block, llvm::Value* llvm_isnull_arg, - llvm::Value* & value) { - value = nullptr; - Var* var_expr = (Var *)expr_state()->expr; + llvm::Value** llvm_out_value) { + assert(nullptr != llvm_out_value); + *llvm_out_value = nullptr; + Var* var_expr = reinterpret_cast(expr_state()->expr); int attnum = var_expr->varattno; auto irb = codegen_utils->ir_builder(); @@ -54,8 +58,7 @@ bool VarExprTreeGenerator::GenerateCode(CodegenUtils* codegen_utils, // For that reason, we keep a double pointer to slot and at execution time // we load slot. TupleTableSlot **ptr_to_slot_ptr = NULL; - switch (var_expr->varno) - { + switch (var_expr->varno) { case INNER: /* get the tuple from the inner node */ ptr_to_slot_ptr = &econtext->ecxt_innertuple; break; @@ -78,10 +81,10 @@ bool VarExprTreeGenerator::GenerateCode(CodegenUtils* codegen_utils, // External functions llvm::Function* llvm_slot_getattr = - codegen_utils->RegisterExternalFunction(slot_getattr); + codegen_utils->GetOrRegisterExternalFunction(slot_getattr); // retrieve variable - value = irb->CreateCall( + *llvm_out_value = irb->CreateCall( llvm_slot_getattr, { llvm_slot, llvm_variable_varattno, diff --git a/src/backend/executor/execProcnode.c b/src/backend/executor/execProcnode.c index 8e7e84b04292..3c08bd8b6a21 100644 --- a/src/backend/executor/execProcnode.c +++ b/src/backend/executor/execProcnode.c @@ -162,10 +162,10 @@ static void ExecCdbTraceNode(PlanState *node, bool entry, TupleTableSlot *result void *context, int flags); - void + static void EnrollQualList(PlanState* result); - void + static void EnrollProjInfoTargetList(ProjectionInfo* ProjInfo); /* @@ -770,9 +770,9 @@ ExecInitNode(Plan *node, EState *estate, int eflags) } /* ---------------------------------------------------------------- - * EnrollTargetAndQualList + * EnrollQualList * - * Enroll Target and Qual List from PlanState to Codegen + * Enroll Qual List's expr state from PlanState for codegen. * ---------------------------------------------------------------- */ void @@ -797,6 +797,12 @@ EnrollQualList(PlanState* result) #endif } +/* ---------------------------------------------------------------- + * EnrollProjInfoTargetList + * + * Enroll Targetlist from ProjectionInfo to Codegen + * ---------------------------------------------------------------- + */ void EnrollProjInfoTargetList(ProjectionInfo* ProjInfo) { diff --git a/src/include/codegen/codegen_wrapper.h b/src/include/codegen/codegen_wrapper.h index b0d78366d577..dc0957636111 100644 --- a/src/include/codegen/codegen_wrapper.h +++ b/src/include/codegen/codegen_wrapper.h @@ -29,13 +29,15 @@ struct ProjectionInfo; struct ExprContext; struct ExprState; -/* Enum used to mimic ExprDoneCond in ExecEvalExpr function pointer. */ +/* + * Enum used to mimic ExprDoneCond in ExecEvalExpr function pointer. + */ typedef enum tmp_enum{ TmpResult -}; +}tmp_enum; typedef void (*ExecVariableListFn) (struct ProjectionInfo *projInfo, Datum *values, bool *isnull); -typedef Datum (*ExecEvalExprFn) (struct ExprState *expression, struct ExprContext *econtext, bool *isNull, /*ExprDoneCond*/ enum tmp_enum *isDone); +typedef Datum (*ExecEvalExprFn) (struct ExprState *expression, struct ExprContext *econtext, bool *isNull, /*ExprDoneCond*/ tmp_enum *isDone); #ifndef USE_CODEGEN @@ -145,6 +147,9 @@ ExecVariableListCodegenEnroll(ExecVariableListFn regular_func_ptr, struct ProjectionInfo* proj_info, struct TupleTableSlot* slot); +/* + * Enroll and returns the pointer to ExecEvalExprGenerator + */ void* ExecEvalExprCodegenEnroll(ExecEvalExprFn regular_func_ptr, ExecEvalExprFn* ptr_to_regular_func_ptr, diff --git a/src/test/unit/mock/gpcodegen_mock.c b/src/test/unit/mock/gpcodegen_mock.c index 5bce62521c8a..880f75de7df7 100644 --- a/src/test/unit/mock/gpcodegen_mock.c +++ b/src/test/unit/mock/gpcodegen_mock.c @@ -77,6 +77,7 @@ ExecVariableListCodegenEnroll(ExecVariableListFn regular_func_ptr, return NULL; } +// Enroll and returns the pointer to ExecEvalExprGenerator void* ExecEvalExprCodegenEnroll(ExecEvalExprFn regular_func_ptr, ExecEvalExprFn* ptr_to_regular_func_ptr,