From 7951727101679bbf9fa422f712265abfef97c57e Mon Sep 17 00:00:00 2001 From: Karthikeyan Jambu Rajaraman Date: Tue, 8 Nov 2016 11:06:25 -0800 Subject: [PATCH] Support overflow checks for doubles. Signed-off-by: Nikos Armenatzoglou --- .../include/codegen/pg_arith_func_generator.h | 46 ++++---- .../include/codegen/utils/codegen_utils.h | 108 +++++++++++++++--- .../codegen/tests/codegen_utils_unittest.cc | 88 ++++++++++++++ src/backend/codegen/utils/codegen_utils.cc | 25 ++++ 4 files changed, 228 insertions(+), 39 deletions(-) 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 ea785bec5a..3de90cff5e 100644 --- a/src/backend/codegen/include/codegen/pg_arith_func_generator.h +++ b/src/backend/codegen/include/codegen/pg_arith_func_generator.h @@ -56,7 +56,7 @@ template <> class ArithOpOverFlowErrorMsg< double> { public: - static const char* OverFlowErrMsg() { return "float8 out of range"; } + static const char* OverFlowErrMsg() { return "value out of range: overflow"; } }; } // namespace gpcodegen_ArithOp_detail @@ -385,30 +385,26 @@ static bool CreateOverflowCheckLogic( assert(nullptr != llvm_error_msg); llvm::IRBuilder<>* irb = codegen_utils->ir_builder(); - // Check if it is a Integer type - if (std::is_integral::value) { - llvm::BasicBlock* llvm_non_overflow_block = codegen_utils->CreateBasicBlock( - "arith_non_overflow_block", pg_func_info.llvm_main_func); - llvm::BasicBlock* llvm_overflow_block = codegen_utils->CreateBasicBlock( - "arith_overflow_block", pg_func_info.llvm_main_func); - - *llvm_out_value = irb->CreateExtractValue(llvm_arith_output, 0); - llvm::Value* llvm_overflow_flag = - irb->CreateExtractValue(llvm_arith_output, 1); - - irb->CreateCondBr(llvm_overflow_flag, llvm_overflow_block, - llvm_non_overflow_block); - - irb->SetInsertPoint(llvm_overflow_block); - EXPAND_CREATE_ELOG(codegen_utils, - ERROR, - "%s", llvm_error_msg); - irb->CreateBr(pg_func_info.llvm_error_block); - - irb->SetInsertPoint(llvm_non_overflow_block); - } else { - *llvm_out_value = llvm_arith_output; - } + + llvm::BasicBlock* llvm_non_overflow_block = codegen_utils->CreateBasicBlock( + "arith_non_overflow_block", pg_func_info.llvm_main_func); + llvm::BasicBlock* llvm_overflow_block = codegen_utils->CreateBasicBlock( + "arith_overflow_block", pg_func_info.llvm_main_func); + + *llvm_out_value = irb->CreateExtractValue(llvm_arith_output, 0); + llvm::Value* llvm_overflow_flag = + irb->CreateExtractValue(llvm_arith_output, 1); + + irb->CreateCondBr(llvm_overflow_flag, llvm_overflow_block, + llvm_non_overflow_block); + + irb->SetInsertPoint(llvm_overflow_block); + EXPAND_CREATE_ELOG(codegen_utils, + ERROR, + "%s", llvm_error_msg); + irb->CreateBr(pg_func_info.llvm_error_block); + + irb->SetInsertPoint(llvm_non_overflow_block); return true; } diff --git a/src/backend/codegen/include/codegen/utils/codegen_utils.h b/src/backend/codegen/include/codegen/utils/codegen_utils.h index fd85e7d15d..f0ef029760 100644 --- a/src/backend/codegen/include/codegen/utils/codegen_utils.h +++ b/src/backend/codegen/include/codegen/utils/codegen_utils.h @@ -346,6 +346,20 @@ class CodegenUtils { template llvm::Value* CreateCast(llvm::Value* value); + /** + * @brief Similar to tuple in C++, this helps to create fixed-size collection + * of hetrogeneous values. + * + * @param members Different types of values that needs to be in the + * collection + * @param name Optional arguments to specify the name of the returned + * variable in IR + * + * @return Struct object allocated in stack, that contains all members. + **/ + llvm::AllocaInst* CreateMakeTuple(const std::vector& members, + const std::string& name = ""); + /** * @brief Register an external function if previously unregistered. Otherwise * return a pointer to the previously registered llvm::Function @@ -593,13 +607,6 @@ class CodegenUtils { llvm::Type* cast_type, const std::size_t cumulative_offset); - // 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 // 'MemberType' to '*cast_type' at the penultimate level of recursion before @@ -614,6 +621,13 @@ class CodegenUtils { MemberType StructType::* pointer_to_member, TailPointerToMemberTypes&&... tail_pointers_to_members); + // 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 RegisterExternalFunction() that appends an entry for an // external function to 'named_external_functions_'. template @@ -1378,26 +1392,55 @@ class ArithOpMaker { llvm::Value* arg1) { Checker(arg0, arg1); - // TODO(armenatzoglou) Support overflow - return generator->ir_builder()->CreateFAdd(arg0, arg1); + auto irb = generator->ir_builder(); + llvm::Value* llvm_result = irb->CreateFAdd(arg0, arg1); + + llvm::AllocaInst* llvm_tuple_ptr = + CreateResultWithOverflow(generator, + llvm_result, + arg0, + arg1, + generator->GetConstant(true)); + return irb->CreateLoad(llvm_tuple_ptr); } static llvm::Value* CreateSubOverflow(CodegenUtils* generator, llvm::Value* arg0, llvm::Value* arg1) { Checker(arg0, arg1); + auto irb = generator->ir_builder(); + llvm::Value* llvm_result = irb->CreateFSub(arg0, arg1); - // TODO(armenatzoglou) Support overflow - return generator->ir_builder()->CreateFSub(arg0, arg1); + llvm::AllocaInst* llvm_tuple_ptr = + CreateResultWithOverflow(generator, + llvm_result, + arg0, + arg1, + generator->GetConstant(true)); + return irb->CreateLoad(llvm_tuple_ptr); } static llvm::Value* CreateMulOverflow(CodegenUtils* generator, llvm::Value* arg0, llvm::Value* arg1) { Checker(arg0, arg1); - - // TODO(armenatzoglou) Support overflow - return generator->ir_builder()->CreateFMul(arg0, arg1); + auto irb = generator->ir_builder(); + llvm::Value* llvm_result = irb->CreateFMul(arg0, arg1); + + llvm::Value* llvm_arg0_zero = irb-> + CreateFCmpOEQ(arg0, generator->GetConstant(0), "is_arg0_zero"); + llvm::Value* llvm_arg1_zero = irb-> + CreateFCmpOEQ(arg1, generator->GetConstant(0), "is_arg1_zero"); + llvm::Value* llvm_zero_valid = irb->CreateOr(llvm_arg0_zero, + llvm_arg1_zero, + "llvm_zero_valid"); + llvm::AllocaInst* llvm_tuple_ptr = + CreateResultWithOverflow(generator, + llvm_result, + arg0, + arg1, + llvm_zero_valid); + return irb->CreateLoad(llvm_tuple_ptr); } @@ -1409,6 +1452,43 @@ class ArithOpMaker { assert(arg0->getType()->isDoubleTy()); assert(arg1->getType()->isDoubleTy()); } + + + // Implements CHECKFLOATVAL(float_utils.h), but instead of error out, + // it returns true when an overflow occurs. + static bool CheckDoubleVal(double val, + double arg0, + double arg1, + bool zero_is_valid) { + bool inf_is_valid = isinf(arg0) || isinf(arg1); + if (isinf(val) && !(inf_is_valid)) { + return true; + } + if ((val) == 0.0 && !(zero_is_valid)) { + return true; + } + return false; + } + + // Returns a struct that contains the result value and the overflow flag of + // the operation. It mimics llvm integer intrinsics. + static llvm::AllocaInst* CreateResultWithOverflow( + CodegenUtils* generator, + llvm::Value* llvm_result, + llvm::Value* arg0, + llvm::Value* arg1, + llvm::Value* zero_is_valid) { + auto irb = generator->ir_builder(); + llvm::Function* llvm_float_overflow_func = + generator->GetOrRegisterExternalFunction( + CheckDoubleVal, "CheckDoubleVal"); + llvm::Value* llvm_overflow = irb->CreateCall( + llvm_float_overflow_func, + { llvm_result, arg0, arg1, zero_is_valid}); + llvm::AllocaInst* llvm_tuple_ptr = generator->CreateMakeTuple( + { llvm_result, llvm_overflow }, "float_result"); + return llvm_tuple_ptr; + } }; } // namespace codegen_utils_detail diff --git a/src/backend/codegen/tests/codegen_utils_unittest.cc b/src/backend/codegen/tests/codegen_utils_unittest.cc index 717f0aa21f..ad3d5f73c6 100644 --- a/src/backend/codegen/tests/codegen_utils_unittest.cc +++ b/src/backend/codegen/tests/codegen_utils_unittest.cc @@ -1255,6 +1255,64 @@ class CodegenUtilsTest : public ::testing::Test { delete[] output_array; } + // Helper method for CreateMakeTupleTest. This method creates LLVM function of + // type int(*)(MemberType... args). This LLVM function will create tuple using + // passed arguments. Then it checks if the value of the member in the tuple + // matches with arguments passed in functions. If the check fails, it will + // return the argument index. + template + void MakeTupleFunc(const std::string& func_name) { + auto irb = codegen_utils_->ir_builder(); + using LLVMFuncType = int(*)(MemberTypes... args); + + llvm::Function* check_tuple_fn = + codegen_utils_->CreateFunction( + func_name); + + irb->SetInsertPoint(codegen_utils_->CreateBasicBlock( + "main", check_tuple_fn)); + llvm::BasicBlock* return_block = codegen_utils_->CreateBasicBlock( + "return_block", check_tuple_fn); + llvm::Value* llvm_ret_ptr = irb->CreateAlloca( + codegen_utils_->GetType(), nullptr, "ret_ptr"); + irb->CreateStore(codegen_utils_->GetConstant(200), llvm_ret_ptr); + std::vector members; + llvm::Function::arg_iterator itr = check_tuple_fn->arg_begin(); + for (; itr != check_tuple_fn->arg_end(); ++itr) { + members.push_back(&(*itr)); + } + llvm::Value* llvm_struct_ptr = codegen_utils_->CreateMakeTuple( + members, "tuple_struct"); + + llvm::Value* llvm_struct_a = irb->CreateLoad(llvm_struct_ptr); + + for (size_t idx = 0; idx < members.size(); ++idx) { + llvm::BasicBlock* next_block = codegen_utils_->CreateBasicBlock( + "member_" + std::to_string(idx), check_tuple_fn); + llvm::Value* llvm_mem_val = irb->CreateExtractValue(llvm_struct_a, idx); + irb->CreateStore(codegen_utils_->GetConstant(idx), llvm_ret_ptr); + llvm::Type* arg_type = members[idx]->getType(); + ASSERT_TRUE(arg_type->isIntegerTy() || + arg_type->isFloatingPointTy()); + llvm::Value* llvm_comp_res = nullptr; + if (arg_type->isIntegerTy()) { + llvm_comp_res = irb->CreateICmpEQ(members[idx], llvm_mem_val); + } else { + llvm_comp_res = irb->CreateFCmpOEQ(members[idx], llvm_mem_val); + } + irb->CreateCondBr( + llvm_comp_res, + next_block /* true */, + return_block /* false */); + irb->SetInsertPoint(next_block); + } + irb->CreateStore(codegen_utils_->GetConstant( + members.size()), llvm_ret_ptr); + irb->CreateBr(return_block); + irb->SetInsertPoint(return_block); + irb->CreateRet(irb->CreateLoad(llvm_ret_ptr)); + } + std::unique_ptr codegen_utils_; }; @@ -2930,6 +2988,36 @@ TEST_F(CodegenUtilsTest, InlineFunctionTest) { EXPECT_EQ(compiled_add_two_fn(-5), -3); } +// Test CreateMakeTuple in CodegenUtils. +TEST_F(CodegenUtilsTest, CreateMakeTupleTest) { + MakeTupleFunc( + "MakeTupleFunc"); + using MakeTupleFuncType = + int (*)(int64_t, bool, int32_t, float, int16_t, double); + + int total_arg = 6; + + // Compiled module + EXPECT_TRUE(codegen_utils_->PrepareForExecution( + CodegenUtils::OptimizationLevel::kNone, false)); + MakeTupleFuncType compiled_tuple_fn = + codegen_utils_->GetFunctionPointer("MakeTupleFunc"); + + EXPECT_EQ(total_arg, compiled_tuple_fn(42, false, 23, 36.23, 12, 25.23)); + EXPECT_EQ(total_arg, compiled_tuple_fn(std::numeric_limits::max(), + true, + std::numeric_limits::max(), + std::numeric_limits::max(), + std::numeric_limits::max(), + std::numeric_limits::max())); + EXPECT_EQ(total_arg, compiled_tuple_fn(std::numeric_limits::min(), + true, + std::numeric_limits::min(), + std::numeric_limits::min(), + std::numeric_limits::min(), + std::numeric_limits::min())); +} + #ifdef CODEGEN_DEBUG diff --git a/src/backend/codegen/utils/codegen_utils.cc b/src/backend/codegen/utils/codegen_utils.cc index 3e5b94bdb2..eb1653f13b 100644 --- a/src/backend/codegen/utils/codegen_utils.cc +++ b/src/backend/codegen/utils/codegen_utils.cc @@ -97,6 +97,31 @@ bool CodegenUtils::InitializeGlobal() { && !llvm::InitializeNativeTargetAsmParser(); } +llvm::AllocaInst* CodegenUtils::CreateMakeTuple( + const std::vector& members, const std::string& name) { + std::vector argument_types(members.size(), nullptr); + std::transform(members.begin(), + members.end(), + argument_types.begin(), + [](llvm::Value* arg) { + assert(nullptr != arg); return arg->getType();}); + llvm::StructType* llvm_struct_type = llvm::StructType::create( + context_, argument_types, name.empty() ? "" : name + "_type"); + llvm::AllocaInst* llvm_struct_ptr = ir_builder()->CreateAlloca( + llvm_struct_type, + nullptr, + name); + for (size_t member_idx = 0; member_idx < members.size(); ++member_idx) { + llvm::Value* llvm_mem_ptr = ir_builder()->CreateInBoundsGEP( + llvm_struct_type, + llvm_struct_ptr, + // This doesn't work if type is not int32_t. + {GetConstant(0), GetConstant(member_idx)}); + ir_builder()->CreateStore(members[member_idx], llvm_mem_ptr); + } + return llvm_struct_ptr; +} + bool CodegenUtils::Optimize(const OptimizationLevel generic_opt_level, const SizeLevel size_level, const bool optimize_for_host_cpu) { -- GitLab