IMPALA-10196: Remove LlvmCodeGen::CastPtrToLlvmPtr

LlvmCodeGen::CastPtrToLlvmPtr embeds a pointer that points to data in
the current process's memory into codegen'd IR code. Our long term goal
is to share the codegen'd IR among processes working on the same
fragment, which is not possible if the IR contains pointers pointing to
data of a specific process. A step in making the IR independent of the
process generating it is removing LlvmCodeGen::CastPtrToLlvmPtr.

Change-Id: I046a06fbf23629a90cc2cca164176a89e557c7c4
Reviewed-on: http://gerrit.cloudera.org:8080/16517
Reviewed-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
This commit is contained in:
Daniel Becker
2020-09-22 09:31:34 +02:00
committed by Impala Public Jenkins
parent 0e96ee8e99
commit aeeff53e88
10 changed files with 62 additions and 216 deletions

View File

@@ -75,28 +75,6 @@ ir_functions = [
"_Z14TimestampValEqRKN10impala_udf12TimestampValES2_"],
["CODEGEN_ANYVAL_TIMESTAMP_VALUE_EQ",
"_Z16TimestampValueEqRKN10impala_udf12TimestampValERKN6impala14TimestampValueE"],
["SCALAR_EXPR_GET_BOOLEAN_VAL",
"_ZN6impala10ScalarExpr24GetBooleanValInterpretedEPS0_PNS_19ScalarExprEvaluatorEPKNS_8TupleRowE"],
["SCALAR_EXPR_GET_TINYINT_VAL",
"_ZN6impala10ScalarExpr24GetTinyIntValInterpretedEPS0_PNS_19ScalarExprEvaluatorEPKNS_8TupleRowE"],
["SCALAR_EXPR_GET_SMALLINT_VAL",
"_ZN6impala10ScalarExpr25GetSmallIntValInterpretedEPS0_PNS_19ScalarExprEvaluatorEPKNS_8TupleRowE"],
["SCALAR_EXPR_GET_INT_VAL",
"_ZN6impala10ScalarExpr20GetIntValInterpretedEPS0_PNS_19ScalarExprEvaluatorEPKNS_8TupleRowE"],
["SCALAR_EXPR_GET_BIGINT_VAL",
"_ZN6impala10ScalarExpr23GetBigIntValInterpretedEPS0_PNS_19ScalarExprEvaluatorEPKNS_8TupleRowE"],
["SCALAR_EXPR_GET_FLOAT_VAL",
"_ZN6impala10ScalarExpr22GetFloatValInterpretedEPS0_PNS_19ScalarExprEvaluatorEPKNS_8TupleRowE"],
["SCALAR_EXPR_GET_DOUBLE_VAL",
"_ZN6impala10ScalarExpr23GetDoubleValInterpretedEPS0_PNS_19ScalarExprEvaluatorEPKNS_8TupleRowE"],
["SCALAR_EXPR_GET_STRING_VAL",
"_ZN6impala10ScalarExpr23GetStringValInterpretedEPS0_PNS_19ScalarExprEvaluatorEPKNS_8TupleRowE"],
["SCALAR_EXPR_GET_TIMESTAMP_VAL",
"_ZN6impala10ScalarExpr26GetTimestampValInterpretedEPS0_PNS_19ScalarExprEvaluatorEPKNS_8TupleRowE"],
["SCALAR_EXPR_GET_DECIMAL_VAL",
"_ZN6impala10ScalarExpr24GetDecimalValInterpretedEPS0_PNS_19ScalarExprEvaluatorEPKNS_8TupleRowE"],
["SCALAR_EXPR_GET_DATE_VAL",
"_ZN6impala10ScalarExpr21GetDateValInterpretedEPS0_PNS_19ScalarExprEvaluatorEPKNS_8TupleRowE"],
["HASH_CRC", "IrCrcHash"],
["HASH_MURMUR", "IrMurmurHash"],
["PHJ_PROCESS_BUILD_BATCH",

View File

@@ -173,35 +173,33 @@ TEST_F(LlvmCodeGenTest, BadIRFile) {
codegen->Close();
}
// IR for the generated linner loop
// define void @JittedInnerLoop() {
// IR for the generated linner loop:
// define void @JittedInnerLoop(i64* %counter) {
// entry:
// call void @DebugTrace(i8* inttoptr (i64 18970856 to i8*))
// %0 = load i64* inttoptr (i64 140735197627800 to i64*)
// %1 = add i64 %0, <delta>
// store i64 %1, i64* inttoptr (i64 140735197627800 to i64*)
// %0 = call i32 (i8*, ...) @printf(
// i8* getelementptr inbounds ([19 x i8], [19 x i8]* @0, i32 0, i32 0))
// %1 = load i64, i64* %counter
// %2 = add i64 %1, 1
// store i64 %2, i64* %counter
// ret void
// }
// The random int in there is the address of jitted_counter
llvm::Function* CodegenInnerLoop(
LlvmCodeGen* codegen, int64_t* jitted_counter, int delta) {
LlvmCodeGen* codegen, int delta) {
llvm::LLVMContext& context = codegen->context();
LlvmBuilder builder(context);
LlvmCodeGen::FnPrototype fn_prototype(codegen, "JittedInnerLoop", codegen->void_type());
llvm::Function* jitted_loop_call = fn_prototype.GeneratePrototype();
llvm::BasicBlock* entry_block =
llvm::BasicBlock::Create(context, "entry", jitted_loop_call);
builder.SetInsertPoint(entry_block);
fn_prototype.AddArgument(
LlvmCodeGen::NamedVariable("counter", codegen->i64_ptr_type()));
llvm::Value* counter;
llvm::Function* jitted_loop_call = fn_prototype.GeneratePrototype(&builder, &counter);
codegen->CodegenDebugTrace(&builder, "Jitted\n");
// Store &jitted_counter as a constant.
llvm::Value* const_delta = codegen->GetI64Constant(delta);
llvm::Value* counter_ptr =
codegen->CastPtrToLlvmPtr(codegen->i64_ptr_type(), jitted_counter);
llvm::Value* loaded_counter = builder.CreateLoad(counter_ptr);
llvm::Value* loaded_counter = builder.CreateLoad(counter);
llvm::Value* incremented_value = builder.CreateAdd(loaded_counter, const_delta);
builder.CreateStore(incremented_value, counter_ptr);
builder.CreateStore(incremented_value, counter);
builder.CreateRetVoid();
return codegen->FinalizeFunction(jitted_loop_call);
@@ -211,21 +209,17 @@ llvm::Function* CodegenInnerLoop(
// The test contains two functions, an outer loop function and an inner loop function.
// The outer loop calls the inner loop function.
// The test will
// 1. create a LlvmCodegen object from the precompiled file
// 2. add another function to the module with the same signature as the inner
// 1. Create a LlvmCodegen object from the precompiled file
// 2. Add another function to the module with the same signature as the inner
// loop function.
// 3. Replace the call instruction in a clone of the outer loop to a call to the new
// inner loop function.
// 4. Update the original loop with another jitted inner loop function
// 5. Run both loops and make sure the correct inner loop is called
// 4. Run the loop and make sure the inner loop is called.
// 5. Updated the jitted loop in place with another jitted inner loop function
// 6. Run the loop and make sure the updated is called.
// 4. Similar to 3, but replace the call with a different inner loop function.
// 5. Compile and run all loops and make sure the correct inner loop is called
TEST_F(LlvmCodeGenTest, ReplaceFnCall) {
const string loop_call_name("_Z21DefaultImplementationv");
const string loop_name("_Z8TestLoopi");
typedef void (*TestLoopFn)(int);
const string loop_call_name("_Z21DefaultImplementationPl");
const string loop_name("_Z8TestLoopiPl");
typedef void (*TestLoopFn)(int, int64_t*);
string module_file;
PathBuilder::GetFullPath("llvm-ir/test-loop.bc", &module_file);
@@ -233,26 +227,24 @@ TEST_F(LlvmCodeGenTest, ReplaceFnCall) {
// Part 1: Load the module and make sure everything is loaded correctly.
scoped_ptr<LlvmCodeGen> codegen;
ASSERT_OK(CreateFromFile(module_file.c_str(), &codegen));
EXPECT_TRUE(codegen.get() != NULL);
EXPECT_TRUE(codegen.get() != nullptr);
llvm::Function* loop_call = codegen->GetFunction(loop_call_name, false);
EXPECT_TRUE(loop_call != NULL);
EXPECT_TRUE(loop_call->arg_empty());
EXPECT_TRUE(loop_call != nullptr);
EXPECT_EQ(loop_call->arg_size(), 1);
llvm::Function* loop = codegen->GetFunction(loop_name, false);
EXPECT_TRUE(loop != NULL);
EXPECT_EQ(loop->arg_size(), 1);
EXPECT_TRUE(loop != nullptr);
EXPECT_EQ(loop->arg_size(), 2);
// Part 2: Generate a new inner loop function.
//
// The c++ version of the code is
// static int64_t* counter;
// void JittedInnerLoop() {
// void JittedInnerLoop(int64_t* counter) {
// printf("LLVM Trace: Jitted\n");
// ++*counter;
// }
//
int64_t jitted_counter = 0;
llvm::Function* jitted_loop_call = CodegenInnerLoop(codegen.get(), &jitted_counter, 1);
llvm::Function* jitted_loop_call = CodegenInnerLoop(codegen.get(), 1);
// Part 3: Clone 'loop' and replace the call instruction to the normal function with a
// call to the jitted one
@@ -264,9 +256,10 @@ TEST_F(LlvmCodeGenTest, ReplaceFnCall) {
// Part 4: Generate a new inner loop function and a new loop function
llvm::Function* jitted_loop_call2 =
CodegenInnerLoop(codegen.get(), &jitted_counter, -2);
CodegenInnerLoop(codegen.get(), -2);
llvm::Function* jitted_loop2 = codegen->CloneFunction(loop);
num_replaced = codegen->ReplaceCallSites(jitted_loop2, jitted_loop_call2, loop_call_name);
num_replaced = codegen->ReplaceCallSites(
jitted_loop2, jitted_loop_call2, loop_call_name);
EXPECT_EQ(1, num_replaced);
EXPECT_TRUE(VerifyFunction(codegen.get(), jitted_loop2));
@@ -284,19 +277,20 @@ TEST_F(LlvmCodeGenTest, ReplaceFnCall) {
ASSERT_TRUE(new_loop.load() != nullptr);
ASSERT_TRUE(new_loop2.load() != nullptr);
int64_t counter = 0;
TestLoopFn original_loop_fn = original_loop.load();
original_loop_fn(5);
EXPECT_EQ(0, jitted_counter);
original_loop_fn(5, &counter);
EXPECT_EQ(0, counter);
TestLoopFn new_loop_fn = new_loop.load();
new_loop_fn(5);
EXPECT_EQ(5, jitted_counter);
new_loop_fn(5);
EXPECT_EQ(10, jitted_counter);
new_loop_fn(5, &counter);
EXPECT_EQ(5, counter);
new_loop_fn(5, &counter);
EXPECT_EQ(10, counter);
TestLoopFn new_loop_fn2 = new_loop2.load();
new_loop_fn2(5);
EXPECT_EQ(0, jitted_counter);
new_loop_fn2(5, &counter);
EXPECT_EQ(0, counter);
codegen->Close();
}
@@ -445,10 +439,9 @@ TEST_F(LlvmCodeGenTest, HashTest) {
const auto close_codegen =
MakeScopeExitTrigger([&codegen]() { codegen->Close(); });
llvm::Value* llvm_data1 =
codegen->CastPtrToLlvmPtr(codegen->ptr_type(), const_cast<char*>(data1));
llvm::Value* llvm_data2 =
codegen->CastPtrToLlvmPtr(codegen->ptr_type(), const_cast<char*>(data2));
LlvmBuilder builder(codegen->context());
llvm::Value* llvm_data1 = codegen->GetStringConstant(&builder, data1, strlen(data1));
llvm::Value* llvm_data2 = codegen->GetStringConstant(&builder, data2, strlen(data2));
llvm::Value* llvm_len1 = codegen->GetI32Constant(strlen(data1));
llvm::Value* llvm_len2 = codegen->GetI32Constant(strlen(data2));
@@ -461,7 +454,6 @@ TEST_F(LlvmCodeGenTest, HashTest) {
// The tuple/values to hash are baked into the codegen for simplicity.
LlvmCodeGen::FnPrototype prototype(
codegen.get(), "HashTest", codegen->i32_type());
LlvmBuilder builder(codegen->context());
// Test both byte-size specific hash functions and the generic loop hash function
llvm::Function* fn_fixed = prototype.GeneratePrototype(&builder, NULL);

View File

@@ -602,13 +602,6 @@ llvm::PointerType* LlvmCodeGen::GetNamedPtrPtrType(const string& name) {
return llvm::PointerType::get(GetNamedPtrType(name), 0);
}
// Llvm doesn't let you create a PointerValue from a c-side ptr. Instead
// cast it to an int and then to 'type'.
llvm::Value* LlvmCodeGen::CastPtrToLlvmPtr(llvm::Type* type, const void* ptr) {
llvm::Constant* const_int = GetI64Constant((int64_t)ptr);
return llvm::ConstantExpr::getIntToPtr(const_int, type);
}
llvm::Constant* LlvmCodeGen::GetIntConstant(
int num_bytes, uint64_t low_bits, uint64_t high_bits) {
DCHECK_GE(num_bytes, 1);
@@ -1366,19 +1359,15 @@ void LlvmCodeGen::CodegenDebugTrace(
LlvmBuilder* builder, const char* str, llvm::Value* v1) {
LOG(ERROR) << "Remove IR codegen debug traces before checking in.";
// Make a copy of str into memory owned by this object. This is no guarantee that str is
// still around when the debug printf is executed.
debug_strings_.push_back(Substitute("LLVM Trace: $0", str));
str = debug_strings_.back().c_str();
// Call printf by embedding the string into the module and getting a pointer to it.
llvm::Value* const llvm_str =
GetStringConstant(builder, Substitute("LLVM Trace: $0", str));
llvm::Function* printf = module_->getFunction("printf");
DCHECK(printf != NULL);
// Call printf by turning 'str' into a constant ptr value
llvm::Value* str_ptr = CastPtrToLlvmPtr(ptr_type_, const_cast<char*>(str));
DCHECK(printf != nullptr);
vector<llvm::Value*> calling_args;
calling_args.push_back(str_ptr);
calling_args.push_back(llvm_str);
if (v1 != NULL) calling_args.push_back(v1);
builder->CreateCall(printf, calling_args);
}

View File

@@ -520,10 +520,6 @@ class LlvmCodeGen {
llvm::BasicBlock** if_block, llvm::BasicBlock** else_block,
llvm::BasicBlock* insert_before = NULL);
/// Create a llvm pointer value from 'ptr'. This is used to pass pointers between
/// c-code and code-generated IR. The resulting value will be of 'type'.
llvm::Value* CastPtrToLlvmPtr(llvm::Type* type, const void* ptr);
/// Returns a constant int of 'byte_size' bytes based on 'low_bits' and 'high_bits'
/// which stand for the lower and upper 64-bits of the constant respectively. For
/// values less than or equal to 64-bits, 'high_bits' is not used. This function
@@ -896,10 +892,6 @@ class LlvmCodeGen {
/// The vector of functions to automatically JIT compile after FinalizeModule().
std::vector<std::pair<llvm::Function*, CodegenFnPtrBase*>> fns_to_jit_compile_;
/// Debug strings that will be outputted by jitted code. This is a copy of all
/// strings passed to CodegenDebugTrace.
std::vector<std::string> debug_strings_;
/// llvm representation of a few common types. Owned by context.
llvm::PointerType* ptr_type_; // int8_t*
llvm::Type* void_type_; // void

View File

@@ -155,11 +155,11 @@ Status TextConverter::CodegenWriteSlot(LlvmCodeGen* codegen,
is_null = builder.CreateCall(
is_null_string_fn, llvm::ArrayRef<llvm::Value*>({args[1], args[2]}));
} else {
llvm::Value* const null_col_ir_str =
codegen->GetStringConstant(&builder, null_col_val, len);
is_null = builder.CreateCall(is_null_string_fn,
llvm::ArrayRef<llvm::Value*>(
{args[1], args[2], codegen->CastPtrToLlvmPtr(codegen->ptr_type(),
const_cast<char*>(null_col_val)),
codegen->GetI32Constant(len)}));
{args[1], args[2], null_col_ir_str, codegen->GetI32Constant(len)}));
}
} else {
// Constant FALSE as branch condition. We rely on later optimization passes

View File

@@ -32,68 +32,3 @@ void dummy(impala_udf::FunctionContext*, impala_udf::BooleanVal*, impala_udf::Ti
impala_udf::TimestampVal*, impala_udf::DecimalVal*, impala_udf::DateVal*,
impala_udf::CollectionVal*, impala::ScalarExprEvaluator*) { }
#endif
/// The following are compute functions that are cross-compiled to both native and IR
/// libraries. In the interpreted path, these functions are executed as-is from the native
/// code. In the codegen'd path, we load the IR functions and replace the Get*Val() calls
/// with the appropriate child's codegen'd compute function.
using namespace impala;
using namespace impala_udf;
/// Static wrappers around Get*ValInterpreted() functions. We'd like to be able to call
/// these from directly from native code as well as from generated IR functions.
BooleanVal ScalarExpr::GetBooleanValInterpreted(
ScalarExpr* expr, ScalarExprEvaluator* eval, const TupleRow* row) {
return expr->GetBooleanValInterpreted(eval, row);
}
TinyIntVal ScalarExpr::GetTinyIntValInterpreted(
ScalarExpr* expr, ScalarExprEvaluator* eval, const TupleRow* row) {
return expr->GetTinyIntValInterpreted(eval, row);
}
SmallIntVal ScalarExpr::GetSmallIntValInterpreted(
ScalarExpr* expr, ScalarExprEvaluator* eval, const TupleRow* row) {
return expr->GetSmallIntValInterpreted(eval, row);
}
IntVal ScalarExpr::GetIntValInterpreted(
ScalarExpr* expr, ScalarExprEvaluator* eval, const TupleRow* row) {
return expr->GetIntValInterpreted(eval, row);
}
BigIntVal ScalarExpr::GetBigIntValInterpreted(
ScalarExpr* expr, ScalarExprEvaluator* eval, const TupleRow* row) {
return expr->GetBigIntValInterpreted(eval, row);
}
FloatVal ScalarExpr::GetFloatValInterpreted(
ScalarExpr* expr, ScalarExprEvaluator* eval, const TupleRow* row) {
return expr->GetFloatValInterpreted(eval, row);
}
DoubleVal ScalarExpr::GetDoubleValInterpreted(
ScalarExpr* expr, ScalarExprEvaluator* eval, const TupleRow* row) {
return expr->GetDoubleValInterpreted(eval, row);
}
StringVal ScalarExpr::GetStringValInterpreted(
ScalarExpr* expr, ScalarExprEvaluator* eval, const TupleRow* row) {
return expr->GetStringValInterpreted(eval, row);
}
TimestampVal ScalarExpr::GetTimestampValInterpreted(
ScalarExpr* expr, ScalarExprEvaluator* eval, const TupleRow* row) {
return expr->GetTimestampValInterpreted(eval, row);
}
DecimalVal ScalarExpr::GetDecimalValInterpreted(
ScalarExpr* expr, ScalarExprEvaluator* eval, const TupleRow* row) {
return expr->GetDecimalValInterpreted(eval, row);
}
DateVal ScalarExpr::GetDateValInterpreted(
ScalarExpr* expr, ScalarExprEvaluator* eval, const TupleRow* row) {
return expr->GetDateValInterpreted(eval, row);
}

View File

@@ -335,39 +335,6 @@ int ScalarExpr::GetSlotIds(vector<SlotId>* slot_ids) const {
return n;
}
llvm::Function* ScalarExpr::GetStaticGetValWrapper(
ColumnType type, LlvmCodeGen* codegen) {
switch (type.type) {
case TYPE_BOOLEAN:
return codegen->GetFunction(IRFunction::SCALAR_EXPR_GET_BOOLEAN_VAL, false);
case TYPE_TINYINT:
return codegen->GetFunction(IRFunction::SCALAR_EXPR_GET_TINYINT_VAL, false);
case TYPE_SMALLINT:
return codegen->GetFunction(IRFunction::SCALAR_EXPR_GET_SMALLINT_VAL, false);
case TYPE_INT:
return codegen->GetFunction(IRFunction::SCALAR_EXPR_GET_INT_VAL, false);
case TYPE_BIGINT:
return codegen->GetFunction(IRFunction::SCALAR_EXPR_GET_BIGINT_VAL, false);
case TYPE_FLOAT:
return codegen->GetFunction(IRFunction::SCALAR_EXPR_GET_FLOAT_VAL, false);
case TYPE_DOUBLE:
return codegen->GetFunction(IRFunction::SCALAR_EXPR_GET_DOUBLE_VAL, false);
case TYPE_STRING:
case TYPE_CHAR:
case TYPE_VARCHAR:
return codegen->GetFunction(IRFunction::SCALAR_EXPR_GET_STRING_VAL, false);
case TYPE_TIMESTAMP:
return codegen->GetFunction(IRFunction::SCALAR_EXPR_GET_TIMESTAMP_VAL, false);
case TYPE_DECIMAL:
return codegen->GetFunction(IRFunction::SCALAR_EXPR_GET_DECIMAL_VAL, false);
case TYPE_DATE:
return codegen->GetFunction(IRFunction::SCALAR_EXPR_GET_DATE_VAL, false);
default:
DCHECK(false) << "Invalid type: " << type.DebugString();
return NULL;
}
}
llvm::Function* ScalarExpr::CreateIrFunctionPrototype(
const string& name, LlvmCodeGen* codegen, llvm::Value* (*args)[2]) {
llvm::Type* return_type = CodegenAnyVal::GetLoweredType(codegen, type());

View File

@@ -339,11 +339,6 @@ class ScalarExpr : public Expr {
/// tree.
llvm::Function* CreateIrFunctionPrototype(const std::string& name, LlvmCodeGen* codegen,
llvm::Value* (*args)[2]);
/// Returns the cross-compiled IR function of the static Get*Val wrapper function for
/// return type 'type'. Must not be called for a type that doesn't have a wrapper
/// function in the built-in IR module. Never returns NULL.
llvm::Function* GetStaticGetValWrapper(ColumnType type, LlvmCodeGen* codegen);
protected:
/// Return true if we should codegen this expression node, based on query options

View File

@@ -332,15 +332,7 @@ Status ScalarFnCall::GetCodegendComputeFnImpl(LlvmCodeGen* codegen, llvm::Functi
llvm::Function* child_fn = NULL;
vector<llvm::Value*> child_fn_args;
// Set 'child_fn' to the codegen'd function, sets child_fn == NULL if codegen fails
Status status = children_[i]->GetCodegendComputeFn(codegen, false, &child_fn);
if (UNLIKELY(!status.ok())) {
DCHECK(child_fn == NULL);
// Set 'child_fn' to the interpreted function
child_fn = GetStaticGetValWrapper(children_[i]->type(), codegen);
// First argument to interpreted function is children_[i]
llvm::Type* expr_ptr_type = codegen->GetStructPtrType<ScalarExpr>();
child_fn_args.push_back(codegen->CastPtrToLlvmPtr(expr_ptr_type, children_[i]));
}
RETURN_IF_ERROR(children_[i]->GetCodegendComputeFn(codegen, false, &child_fn));
child_fn_args.push_back(eval);
child_fn_args.push_back(row);

View File

@@ -19,14 +19,20 @@
// which is used by the unit test to exercise loading precompiled
// ir.
#include <stdio.h>
#include <cstdint>
__attribute__ ((noinline)) void DefaultImplementation() {
printf("Default\n");
/// The default implementation does nothing with the pointer argument. The codegen
/// implementation that it will be replaced with will increment it.
__attribute__ ((noinline)) void DefaultImplementation(int64_t* ptr) {
// We need to use 'ptr' otherwise clang marks it as 'readnone' and doesn't actually pass
// the value from the caller. We need the value to be passed because the codegen'd
// function this function will be replaced with does use the argument.
printf("Default, value is %ld.\n", *ptr);
}
void TestLoop(int n) {
void TestLoop(int n, int64_t* counter) {
for (int i = 0; i < n; ++i) {
DefaultImplementation();
DefaultImplementation(counter);
}
}