You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2020/02/20 23:49:25 UTC

[impala] 03/03: IMPALA-9366: Remove embedded pointer reference in PhjBuilder::CodegenInsertRuntimeFilters

This is an automated email from the ASF dual-hosted git repository.

tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit e04a839d9c000c9652519d5e76bbe280ceb1114d
Author: Bikramjeet Vig <bi...@cloudera.com>
AuthorDate: Fri Feb 7 12:06:14 2020 -0800

    IMPALA-9366: Remove embedded pointer reference in
    PhjBuilder::CodegenInsertRuntimeFilters
    
    The handcrafted codegen method in
    PhjBuilder::CodegenInsertRuntimeFilters embeds direct pointer
    references to filter contexts. This patch replaces those with
    references to the input parameters instead.
    
    Testing:
    Successfully ran core tests for joins and runtime filters.
    
    Change-Id: I09037b5832b037536bde9324f7a2a10778511114
    Reviewed-on: http://gerrit.cloudera.org:8080/15186
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/exec/partitioned-hash-join-builder-ir.cc |  2 +-
 be/src/exec/partitioned-hash-join-builder.cc    | 46 ++++++++++++++-----------
 be/src/exec/partitioned-hash-join-builder.h     |  4 +--
 3 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/be/src/exec/partitioned-hash-join-builder-ir.cc b/be/src/exec/partitioned-hash-join-builder-ir.cc
index a4bfcdb..51d2239 100644
--- a/be/src/exec/partitioned-hash-join-builder-ir.cc
+++ b/be/src/exec/partitioned-hash-join-builder-ir.cc
@@ -57,7 +57,7 @@ Status PhjBuilder::ProcessBuildBatch(
     if (build_filters) {
       DCHECK_EQ(ctx->level(), 0)
           << "Runtime filters should not be built during repartitioning.";
-      InsertRuntimeFilters(build_row);
+      InsertRuntimeFilters(filter_ctxs_.data(), build_row);
     }
     const uint32_t hash = expr_vals_cache->CurExprValuesHash();
     const uint32_t partition_idx = hash >> (32 - NUM_PARTITIONING_BITS);
diff --git a/be/src/exec/partitioned-hash-join-builder.cc b/be/src/exec/partitioned-hash-join-builder.cc
index c0e85e0..ac38216 100644
--- a/be/src/exec/partitioned-hash-join-builder.cc
+++ b/be/src/exec/partitioned-hash-join-builder.cc
@@ -758,7 +758,10 @@ void PhjBuilder::AllocateRuntimeFilters() {
   }
 }
 
-void PhjBuilder::InsertRuntimeFilters(TupleRow* build_row) noexcept {
+void PhjBuilder::InsertRuntimeFilters(
+    FilterContext filter_ctxs[], TupleRow* build_row) noexcept {
+  // For the interpreted path we can directly use the filter_ctxs_ member variable.
+  DCHECK_EQ(filter_ctxs_.data(), filter_ctxs);
   for (const FilterContext& ctx : filter_ctxs_) ctx.Insert(build_row);
 }
 
@@ -1323,18 +1326,21 @@ Status PhjBuilder::CodegenInsertBatch(LlvmCodeGen* codegen,
 
 // An example of the generated code for a query with two filters built by this node.
 //
-// ; Function Attrs: noinline
-// define void @InsertRuntimeFilters(%"class.impala::PhjBuilder"* %this,
-//     %"class.impala::TupleRow"* %row) #46 {
-// entry:
-//   call void @FilterContextInsert(%"struct.impala::FilterContext"* inttoptr (
-//       i64 197870464 to %"struct.impala::FilterContext"*),
-//       %"class.impala::TupleRow"* %row)
-//   call void @FilterContextInsert.14(%"struct.impala::FilterContext"* inttoptr (
-//       i64 197870496 to %"struct.impala::FilterContext"*),
-//       %"class.impala::TupleRow"* %row)
-//   ret void
-// }
+//  ; Function Attrs: noinline
+//  define void @InsertRuntimeFilters(%"class.impala::PhjBuilder"* %this,
+//      %"struct.impala::FilterContext"* %filter_ctxs,
+//      %"class.impala::TupleRow"* %row) #49 {
+//  entry:
+//    %0 = getelementptr %"struct.impala::FilterContext",
+//        %"struct.impala::FilterContext"* %filter_ctxs, i32 0
+//    call void @FilterContextInsert(%"struct.impala::FilterContext"* %0,
+//        %"class.impala::TupleRow"* %row)
+//    %1 = getelementptr %"struct.impala::FilterContext",
+//        %"struct.impala::FilterContext"* %filter_ctxs, i32 1
+//    call void @FilterContextInsert.3(%"struct.impala::FilterContext"* %1,
+//        %"class.impala::TupleRow"* %row)
+//    ret void
+//  }
 Status PhjBuilder::CodegenInsertRuntimeFilters(
     LlvmCodeGen* codegen, const vector<ScalarExpr*>& filter_exprs, llvm::Function** fn) {
   llvm::LLVMContext& context = codegen->context();
@@ -1342,25 +1348,25 @@ Status PhjBuilder::CodegenInsertRuntimeFilters(
 
   *fn = nullptr;
   llvm::Type* this_type = codegen->GetStructPtrType<PhjBuilder>();
+  llvm::PointerType* filters_ctx_arr_type = codegen->GetStructPtrType<FilterContext>();
   llvm::PointerType* tuple_row_ptr_type = codegen->GetStructPtrType<TupleRow>();
   LlvmCodeGen::FnPrototype prototype(
       codegen, "InsertRuntimeFilters", codegen->void_type());
   prototype.AddArgument(LlvmCodeGen::NamedVariable("this", this_type));
+  prototype.AddArgument(LlvmCodeGen::NamedVariable("filter_ctxs", filters_ctx_arr_type));
   prototype.AddArgument(LlvmCodeGen::NamedVariable("row", tuple_row_ptr_type));
 
-  llvm::Value* args[2];
+  llvm::Value* args[3];
   llvm::Function* insert_runtime_filters_fn = prototype.GeneratePrototype(&builder, args);
-  llvm::Value* row_arg = args[1];
+  llvm::Value* filter_ctxs = args[1];
+  llvm::Value* row_arg = args[2];
 
   int num_filters = filter_exprs.size();
   for (int i = 0; i < num_filters; ++i) {
     llvm::Function* insert_fn;
     RETURN_IF_ERROR(FilterContext::CodegenInsert(
-        codegen, filter_exprs_[i], &filter_ctxs_[i], &insert_fn));
-    llvm::PointerType* filter_context_type = codegen->GetStructPtrType<FilterContext>();
-    llvm::Value* filter_context_ptr =
-        codegen->CastPtrToLlvmPtr(filter_context_type, &filter_ctxs_[i]);
-
+        codegen, filter_exprs[i], &filter_ctxs_[i], &insert_fn));
+    llvm::Value* filter_context_ptr = builder.CreateConstGEP1_32(filter_ctxs, i);
     llvm::Value* insert_args[] = {filter_context_ptr, row_arg};
     builder.CreateCall(insert_fn, insert_args);
   }
diff --git a/be/src/exec/partitioned-hash-join-builder.h b/be/src/exec/partitioned-hash-join-builder.h
index 204d0b1..db56334 100644
--- a/be/src/exec/partitioned-hash-join-builder.h
+++ b/be/src/exec/partitioned-hash-join-builder.h
@@ -596,9 +596,9 @@ class PhjBuilder : public JoinBuilder {
   /// phase.
   void AllocateRuntimeFilters();
 
-  /// Iterates over the runtime filters in filters_ and inserts each row into each filter.
+  /// Iterates over the runtime filters and inserts each row into each filter.
   /// This is replaced at runtime with code generated by CodegenInsertRuntimeFilters().
-  void InsertRuntimeFilters(TupleRow* build_row) noexcept;
+  void InsertRuntimeFilters(FilterContext filter_ctxs[], TupleRow* build_row) noexcept;
 
   /// Publish the runtime filters to the fragment-local RuntimeFilterBank.
   /// 'num_build_rows' is used to determine whether the computed filters have an