Repository: incubator-impala Updated Branches: refs/heads/master f51c4435c -> b70acf92b
IMPALA-2689: Log why codegen is disabled in TextConverter::CodegenWriteSlot() Plumbing statuses through TextConverter::CodegenWriteSlot(), which eventually propagate to the runtime profile. Change-Id: I355efe2d5566ba6a1b72648763794a79c80c3089 Reviewed-on: http://gerrit.cloudera.org:8080/7574 Reviewed-by: Tim Armstrong <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/dfb158b4 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/dfb158b4 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/dfb158b4 Branch: refs/heads/master Commit: dfb158b474afc26e761caf4b53ec4b4a48f842d3 Parents: f51c443 Author: aphadke <[email protected]> Authored: Wed Aug 2 18:47:56 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Tue Aug 15 02:04:31 2017 +0000 ---------------------------------------------------------------------- be/src/exec/hdfs-scanner.cc | 6 +-- be/src/exec/text-converter.cc | 81 ++++++++++++++++++++++---------------- be/src/exec/text-converter.h | 7 ++-- 3 files changed, 53 insertions(+), 41 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/dfb158b4/be/src/exec/hdfs-scanner.cc ---------------------------------------------------------------------- diff --git a/be/src/exec/hdfs-scanner.cc b/be/src/exec/hdfs-scanner.cc index 23fcc20..0ef62ab 100644 --- a/be/src/exec/hdfs-scanner.cc +++ b/be/src/exec/hdfs-scanner.cc @@ -320,11 +320,11 @@ Status HdfsScanner::CodegenWriteCompleteTuple(HdfsScanNodeBase* node, TupleDescriptor* tuple_desc = const_cast<TupleDescriptor*>(node->tuple_desc()); vector<Function*> slot_fns; for (int i = 0; i < node->materialized_slots().size(); ++i) { + Function *fn = nullptr; SlotDescriptor* slot_desc = node->materialized_slots()[i]; - Function* fn = TextConverter::CodegenWriteSlot(codegen, tuple_desc, slot_desc, + RETURN_IF_ERROR(TextConverter::CodegenWriteSlot(codegen, tuple_desc, slot_desc, &fn, node->hdfs_table()->null_column_value().data(), - node->hdfs_table()->null_column_value().size(), true, state->strict_mode()); - if (fn == NULL) return Status("CodegenWriteSlot failed."); + node->hdfs_table()->null_column_value().size(), true, state->strict_mode())); if (i >= LlvmCodeGen::CODEGEN_INLINE_EXPRS_THRESHOLD) codegen->SetNoInline(fn); slot_fns.push_back(fn); } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/dfb158b4/be/src/exec/text-converter.cc ---------------------------------------------------------------------- diff --git a/be/src/exec/text-converter.cc b/be/src/exec/text-converter.cc index db390c4..431a11b 100644 --- a/be/src/exec/text-converter.cc +++ b/be/src/exec/text-converter.cc @@ -66,51 +66,52 @@ void TextConverter::UnescapeString(const char* src, char* dest, int* len, } // Codegen for a function to parse one slot. The IR for a int slot looks like: -// define i1 @WriteSlot({ i8, i32 }* %tuple_arg, i8* %data, i32 %len) { +// define i1 @WriteSlot(<{ i32, i8 }>* %tuple_arg, i8* %data, i32 %len) #38 { // entry: // %parse_result = alloca i32 -// %0 = call i1 @IsNullString(i8* %data, i32 %len) +// %0 = call i1 @IrIsNullString(i8* %data, i32 %len) // br i1 %0, label %set_null, label %check_zero // // set_null: ; preds = %check_zero, %entry -// call void @SetNull({ i8, i32 }* %tuple_arg) +// %1 = bitcast <{ i32, i8 }>* %tuple_arg to i8* +// %null_byte_ptr2 = getelementptr inbounds i8, i8* %1, i32 4 +// %null_byte3 = load i8, i8* %null_byte_ptr2 +// %null_bit_set4 = or i8 %null_byte3, 1 +// store i8 %null_bit_set4, i8* %null_byte_ptr2 // ret i1 true // // parse_slot: ; preds = %check_zero -// %slot = getelementptr inbounds { i8, i32 }* %tuple_arg, i32 0, i32 1 -// %1 = call i32 @IrStringToInt32(i8* %data, i32 %len, i32* %parse_result) -// %parse_result1 = load i32* %parse_result +// %slot = getelementptr inbounds <{ i32, i8 }>, <{ i32, i8 }>* %tuple_arg, i32 0, i32 0 +// %2 = call i32 @IrStringToInt32(i8* %data, i32 %len, i32* %parse_result) +// %parse_result1 = load i32, i32* %parse_result // %failed = icmp eq i32 %parse_result1, 1 // br i1 %failed, label %parse_fail, label %parse_success // // check_zero: ; preds = %entry -// %2 = icmp eq i32 %len, 0 -// br i1 %2, label %set_null, label %parse_slot +// %3 = icmp eq i32 %len, 0 +// br i1 %3, label %set_null, label %parse_slot // // parse_success: ; preds = %parse_slot -// store i32 %1, i32* %slot +// store i32 %2, i32* %slot // ret i1 true // // parse_fail: ; preds = %parse_slot -// call void @SetNull({ i8, i32 }* %tuple_arg) +// %4 = bitcast <{ i32, i8 }>* %tuple_arg to i8* +// %null_byte_ptr = getelementptr inbounds i8, i8* %4, i32 4 +// %null_byte = load i8, i8* %null_byte_ptr +// %null_bit_set = or i8 %null_byte, 1 +// store i8 %null_bit_set, i8* %null_byte_ptr // ret i1 false -// } -// -// If strict_mode = true, then 'parse_slot' also treats overflows errors, e.g.: -// parse_slot: ; preds = %check_zero -// %slot = getelementptr inbounds { i8, i32 }* %tuple_arg, i32 0, i32 1 -// %1 = call i32 @IrStringToInt32(i8* %data, i32 %len, i32* %parse_result) -// %parse_result1 = load i32, i32* %parse_result -// %failed = icmp eq i32 %parse_result1, 1 -// %overflowed = icmp eq i32 %parse_result1, 2 -// %failed_or = or i1 %failed, %overflowed -// br i1 %failed_or, label %parse_fail, label %parse_success -Function* TextConverter::CodegenWriteSlot(LlvmCodeGen* codegen, - TupleDescriptor* tuple_desc, SlotDescriptor* slot_desc, +//} + + +Status TextConverter::CodegenWriteSlot(LlvmCodeGen* codegen, + TupleDescriptor* tuple_desc, SlotDescriptor* slot_desc, Function** fn, const char* null_col_val, int len, bool check_null, bool strict_mode) { + DCHECK(fn != nullptr); if (slot_desc->type().type == TYPE_CHAR) { - LOG(INFO) << "Char isn't supported for CodegenWriteSlot"; - return NULL; + return Status("TextConverter::CodegenWriteSlot(): Char isn't supported for" + " CodegenWriteSlot"); } SCOPED_TIMER(codegen->codegen_timer()); @@ -122,10 +123,14 @@ Function* TextConverter::CodegenWriteSlot(LlvmCodeGen* codegen, } else { is_null_string_fn = codegen->GetFunction(IRFunction::GENERIC_IS_NULL_STRING, false); } - if (is_null_string_fn == NULL) return NULL; + + DCHECK(is_null_string_fn != NULL); StructType* tuple_type = tuple_desc->GetLlvmStruct(codegen); - if (tuple_type == NULL) return NULL; + if (tuple_type == NULL) { + return Status("TextConverter::CodegenWriteSlot(): Failed to generate " + "tuple type"); + } PointerType* tuple_ptr_type = tuple_type->getPointerTo(); LlvmCodeGen::FnPrototype prototype( @@ -136,14 +141,14 @@ Function* TextConverter::CodegenWriteSlot(LlvmCodeGen* codegen, LlvmBuilder builder(codegen->context()); Value* args[3]; - Function* fn = prototype.GeneratePrototype(&builder, &args[0]); + *fn = prototype.GeneratePrototype(&builder, &args[0]); BasicBlock* set_null_block, *parse_slot_block, *check_zero_block = NULL; - codegen->CreateIfElseBlocks(fn, "set_null", "parse_slot", + codegen->CreateIfElseBlocks(*fn, "set_null", "parse_slot", &set_null_block, &parse_slot_block); if (!slot_desc->type().IsVarLenStringType()) { - check_zero_block = BasicBlock::Create(codegen->context(), "check_zero", fn); + check_zero_block = BasicBlock::Create(codegen->context(), "check_zero", *fn); } // Check if the data matches the configured NULL string. @@ -175,7 +180,8 @@ Function* TextConverter::CodegenWriteSlot(LlvmCodeGen* codegen, // Codegen parse slot block builder.SetInsertPoint(parse_slot_block); - Value* slot = builder.CreateStructGEP(NULL, args[0], slot_desc->llvm_field_idx(), "slot"); + Value* slot = builder.CreateStructGEP(NULL, args[0], slot_desc->llvm_field_idx(), + "slot"); if (slot_desc->type().IsVarLenStringType()) { Value* ptr = builder.CreateStructGEP(NULL, slot, 0, "string_ptr"); @@ -225,17 +231,18 @@ Function* TextConverter::CodegenWriteSlot(LlvmCodeGen* codegen, break; default: DCHECK(false); - return NULL; + return Status("TextConverter::CodegenWriteSlot(): Codegen'd parser NYI for the" + "slot_desc type"); } parse_fn = codegen->GetFunction(parse_fn_enum, false); DCHECK(parse_fn != NULL); // Set up trying to parse the string to the slot type BasicBlock* parse_success_block, *parse_failed_block; - codegen->CreateIfElseBlocks(fn, "parse_success", "parse_fail", + codegen->CreateIfElseBlocks(*fn, "parse_success", "parse_fail", &parse_success_block, &parse_failed_block); LlvmCodeGen::NamedVariable parse_result("parse_result", codegen->GetType(TYPE_INT)); - Value* parse_result_ptr = codegen->CreateEntryBlockAlloca(fn, parse_result); + Value* parse_result_ptr = codegen->CreateEntryBlockAlloca(*fn, parse_result); Value* parse_return; // Call Impala's StringTo* function @@ -281,5 +288,9 @@ Function* TextConverter::CodegenWriteSlot(LlvmCodeGen* codegen, slot_desc->CodegenSetNullIndicator(codegen, &builder, args[0], codegen->true_value()); builder.CreateRet(codegen->true_value()); - return codegen->FinalizeFunction(fn); + if (codegen->FinalizeFunction(*fn) == NULL) { + return Status("TextConverter::CodegenWriteSlot(): codegen'd " + "WriteSlot function failed verification, see log"); + } + return Status::OK(); } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/dfb158b4/be/src/exec/text-converter.h ---------------------------------------------------------------------- diff --git a/be/src/exec/text-converter.h b/be/src/exec/text-converter.h index 1a78af0..59d1968 100644 --- a/be/src/exec/text-converter.h +++ b/be/src/exec/text-converter.h @@ -70,7 +70,8 @@ class TextConverter { void UnescapeString(const char* src, char* dest, int* len, int64_t maxlen = -1); /// Codegen the function to write a slot for slot_desc. - /// Returns NULL if codegen was not succesful. + /// Returns Status::OK() if codegen was successful. If codegen was successful + /// llvm::Function** fn points to the codegen'd function /// The signature of the generated function is: /// bool WriteSlot(Tuple* tuple, const char* data, int len); /// The codegen function returns true if the slot could be written and false @@ -81,8 +82,8 @@ class TextConverter { /// be used for partitions that contain escapes. /// strict_mode: If set, numerical overflow/underflow are considered to be parse /// errors. - static llvm::Function* CodegenWriteSlot(LlvmCodeGen* codegen, - TupleDescriptor* tuple_desc, SlotDescriptor* slot_desc, + static Status CodegenWriteSlot(LlvmCodeGen* codegen, + TupleDescriptor* tuple_desc, SlotDescriptor* slot_desc, llvm::Function** fn, const char* null_col_val, int len, bool check_null, bool strict_mode = false); private:
