Skip to content
This repository was archived by the owner on May 9, 2024. It is now read-only.

Commit bfdb831

Browse files
committed
Forbid CompilationOptions default constructor
1 parent 2602ad1 commit bfdb831

File tree

6 files changed

+37
-23
lines changed

6 files changed

+37
-23
lines changed

omniscidb/QueryEngine/ColumnIR.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -362,11 +362,10 @@ std::vector<llvm::Value*> CodeGenerator::codegenOuterJoinNullPlaceholder(
362362
}
363363
const auto null_constant =
364364
hdk::ir::makeExpr<hdk::ir::Constant>(null_type, true, Datum{0});
365-
const auto null_target_lvs =
366-
codegen(null_constant.get(),
367-
false,
368-
CompilationOptions{
369-
ExecutorDeviceType::CPU, false, ExecutorOptLevel::Default, false});
365+
// this looks incorrect, should we just use co instead?
366+
CompilationOptions in_co =
367+
CompilationOptions::makeCpuOnly(co).withHoistedLiterals(false);
368+
const auto null_target_lvs = codegen(null_constant.get(), false, in_co);
370369
cgen_state_->ir_builder_.CreateBr(phi_bb);
371370
CHECK_EQ(orig_lvs.size(), null_target_lvs.size());
372371
cgen_state_->ir_builder_.SetInsertPoint(phi_bb);

omniscidb/QueryEngine/CompilationOptions.h

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,26 @@ struct CompilationOptions {
6262
compiler::cpu_cgen_traits_desc};
6363
}
6464

65+
CompilationOptions withHoistedLiterals(bool hoist = true) {
66+
CompilationOptions co = *this;
67+
co.hoist_literals = hoist;
68+
return co;
69+
}
70+
71+
static CompilationOptions reductionDefaults() {
72+
return CompilationOptions{
73+
ExecutorDeviceType::CPU,
74+
/*hoist_literals=*/false,
75+
/*opt_level=*/ExecutorOptLevel::ReductionJIT,
76+
/*with_dynamic_watchdog=*/false,
77+
/*allow_lazy_fetch=*/true,
78+
/*filter_on_delted_column=*/true,
79+
/*explain_type=*/ExecutorExplainType::Default,
80+
/*register_intel_jit_listener=*/false,
81+
/*use_groupby_buffer_desc=*/false,
82+
/*codegen_traits_desc=*/getCgenTraitsDesc(ExecutorDeviceType::CPU)};
83+
}
84+
6585
static compiler::CodegenTraitsDescriptor getCgenTraitsDesc(
6686
const ExecutorDeviceType device_type,
6787
const bool is_l0 = false) {
@@ -89,6 +109,9 @@ struct CompilationOptions {
89109
/*use_groupby_buffer_desc=*/false,
90110
/*codegen_traits_desc=*/getCgenTraitsDesc(device_type, is_l0)};
91111
}
112+
113+
private:
114+
CompilationOptions() = default;
92115
};
93116

94117
enum class ExecutorType { Native, Extern };

omniscidb/QueryEngine/ResultSetReductionInterpreterStubs.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,9 +187,7 @@ StubGenerator::Stub StubGenerator::generateStub(const std::string& name,
187187
const Type ret_type,
188188
const bool is_external,
189189
const Executor* executor) {
190-
CompilationOptions co{
191-
ExecutorDeviceType::CPU, false, ExecutorOptLevel::ReductionJIT, false};
192-
co.codegen_traits_desc = co.getCgenTraitsDesc(ExecutorDeviceType::CPU);
190+
CompilationOptions co = CompilationOptions::reductionDefaults();
193191
// Multiple executors may trigger the generation of the same
194192
// stub. We'll use get_or_wait/put methods of code cache accessor to
195193
// let the first executor to generate the stub while other executors

omniscidb/QueryEngine/ResultSetReductionJIT.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -617,10 +617,7 @@ ResultSetReductionJIT::ResultSetReductionJIT(const QueryMemoryDescriptor& query_
617617

618618
ReductionCode ResultSetReductionJIT::codegen() const {
619619
const auto hash_type = query_mem_desc_.getQueryDescriptionType();
620-
CompilationOptions co{
621-
ExecutorDeviceType::CPU, false, ExecutorOptLevel::ReductionJIT, false};
622-
623-
co.codegen_traits_desc = co.getCgenTraitsDesc(ExecutorDeviceType::CPU);
620+
CompilationOptions co = CompilationOptions::reductionDefaults();
624621

625622
if (query_mem_desc_.didOutputColumnar() || !is_aggregate_query(hash_type)) {
626623
return {};
@@ -1354,8 +1351,7 @@ void ResultSetReductionJIT::finalizeReductionCode(
13541351
const llvm::Function* ir_reduce_one_entry,
13551352
const llvm::Function* ir_reduce_one_entry_idx,
13561353
const CodeCacheKey& key) const {
1357-
CompilationOptions co{
1358-
ExecutorDeviceType::CPU, false, ExecutorOptLevel::ReductionJIT, false};
1354+
CompilationOptions co = CompilationOptions::reductionDefaults();
13591355
#ifdef NDEBUG
13601356
LOG(IR) << "Reduction Loop:\n"
13611357
<< serialize_llvm_object(reduction_code.llvm_reduce_loop);

omniscidb/Tests/ArrowBasedExecuteTest.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9657,9 +9657,7 @@ TEST_F(Select, Joins_LeftJoinFiltered) {
96579657
auto check_explain_result = [](const std::string& query,
96589658
const ExecutorDeviceType dt,
96599659
const bool enable_filter_hoisting) {
9660-
CompilationOptions co;
9661-
co.device_type = dt;
9662-
co.hoist_literals = true;
9660+
CompilationOptions co = getCompilationOptions(dt);
96639661
ExecutionOptions eo = ExecutionOptions::fromConfig(config());
96649662
eo.allow_loop_joins = false;
96659663
eo.just_explain = true;

python/pyhdk/_sql.pyx

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -192,21 +192,21 @@ cdef class RelAlgExecutor:
192192

193193
def execute(self, **kwargs):
194194
cdef const CConfig *config = self.c_rel_alg_executor.get().getExecutor().getConfigPtr().get()
195-
cdef CCompilationOptions c_co
195+
cdef unique_ptr[CCompilationOptions] c_co
196196
if kwargs.get("device_type", "auto") == "GPU" and not config.exec.cpu_only:
197-
c_co = CCompilationOptions.defaults(CExecutorDeviceType.GPU, False)
197+
c_co = make_unique[CCompilationOptions](CCompilationOptions.defaults(CExecutorDeviceType.GPU, False))
198198
else:
199-
c_co = CCompilationOptions.defaults(CExecutorDeviceType.CPU, False)
200-
c_co.allow_lazy_fetch = kwargs.get("enable_lazy_fetch", config.rs.enable_lazy_fetch)
201-
c_co.with_dynamic_watchdog = kwargs.get("enable_dynamic_watchdog", config.exec.watchdog.enable_dynamic)
199+
c_co = make_unique[CCompilationOptions](CCompilationOptions.defaults(CExecutorDeviceType.CPU, False))
200+
c_co.get().allow_lazy_fetch = kwargs.get("enable_lazy_fetch", config.rs.enable_lazy_fetch)
201+
c_co.get().with_dynamic_watchdog = kwargs.get("enable_dynamic_watchdog", config.exec.watchdog.enable_dynamic)
202202
cdef unique_ptr[CExecutionOptions] c_eo = make_unique[CExecutionOptions](CExecutionOptions.fromConfig(dereference(config)))
203203
c_eo.get().output_columnar_hint = kwargs.get("enable_columnar_output", config.rs.enable_columnar_output)
204204
c_eo.get().with_watchdog = kwargs.get("enable_watchdog", config.exec.watchdog.enable)
205205
c_eo.get().with_dynamic_watchdog = kwargs.get("enable_dynamic_watchdog", config.exec.watchdog.enable_dynamic)
206206
c_eo.get().just_explain = kwargs.get("just_explain", False)
207207
c_eo.get().forced_gpu_proportion = kwargs.get("forced_gpu_proportion", config.exec.heterogeneous.forced_gpu_proportion)
208208
c_eo.get().forced_cpu_proportion = 100 - c_eo.get().forced_gpu_proportion
209-
cdef CExecutionResult c_res = self.c_rel_alg_executor.get().executeRelAlgQuery(c_co, dereference(c_eo.get()), False)
209+
cdef CExecutionResult c_res = self.c_rel_alg_executor.get().executeRelAlgQuery(dereference(c_co.get()), dereference(c_eo.get()), False)
210210
cdef ExecutionResult res = ExecutionResult()
211211
res.c_result = move(c_res)
212212
res.c_data_mgr = self.c_data_mgr

0 commit comments

Comments
 (0)