IMPALA-3181: Add noexcept to some functions This commit adds noexcept specifier to some cross-compiled functions which are known to not throw exceptions. This helps avoid some exception related instructions (e.g. invoke, landingpad) in the IR.
Change-Id: I96bd2fec6c14771acae1e700bed958951368ee77 Reviewed-on: http://gerrit.cloudera.org:8080/3256 Reviewed-by: Dan Hecht <[email protected]> Tested-by: Internal 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/5f3996e6 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/5f3996e6 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/5f3996e6 Branch: refs/heads/master Commit: 5f3996e6d1c53a5255b84f9e32da9324f3f972b3 Parents: 5231301 Author: Michael Ho <[email protected]> Authored: Sat May 28 12:38:59 2016 -0700 Committer: Tim Armstrong <[email protected]> Committed: Thu Jun 2 09:32:54 2016 -0700 ---------------------------------------------------------------------- be/CMakeLists.txt | 1 - be/src/common/status.cc | 4 ++-- be/src/common/status.h | 4 ++-- be/src/runtime/mem-pool.cc | 2 +- be/src/runtime/mem-pool.h | 12 ++++++------ be/src/udf/udf-internal.h | 4 ++-- be/src/udf/udf.cc | 14 +++++++------- be/src/udf/udf.h | 10 +++++----- be/src/udf_samples/CMakeLists.txt | 2 +- 9 files changed, 26 insertions(+), 27 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5f3996e6/be/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/be/CMakeLists.txt b/be/CMakeLists.txt index 630bae9..ed4a8c4 100644 --- a/be/CMakeLists.txt +++ b/be/CMakeLists.txt @@ -138,7 +138,6 @@ add_definitions(-DHAVE_INTTYPES_H -DHAVE_NETINET_IN_H -DHAVE_NETDB_H) # Empirically, the runtime JIT produces slightly better code when starting with IR that # was optimized at -O1. Higher optimization levels tend to bloat the code. # -Wno-deprecated: gutil contains deprecated headers -# -Wno-c++11-extensions: c++11 extensions are ok as long as clang supports them # -Wno-return-type-c-linkage: UDFs return C++ classes but use C linkage to prevent # mangling # -DBOOST_NO_EXCEPTIONS: call a custom error handler for exceptions in codegen'd code. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5f3996e6/be/src/common/status.cc ---------------------------------------------------------------------- diff --git a/be/src/common/status.cc b/be/src/common/status.cc index ede4144..42dc41c 100644 --- a/be/src/common/status.cc +++ b/be/src/common/status.cc @@ -204,11 +204,11 @@ void Status::ToThrift(TStatus* status) const { } } -void Status::FreeMessage() { +void Status::FreeMessage() noexcept { delete msg_; } -void Status::CopyMessageFrom(const Status& status) { +void Status::CopyMessageFrom(const Status& status) noexcept { delete msg_; msg_ = status.msg_ == NULL ? NULL : new ErrorMsg(*status.msg_); } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5f3996e6/be/src/common/status.h ---------------------------------------------------------------------- diff --git a/be/src/common/status.h b/be/src/common/status.h index f88bf50..e4dc591 100644 --- a/be/src/common/status.h +++ b/be/src/common/status.h @@ -239,10 +239,10 @@ class Status { Status(const std::string& error_msg, bool silent); // A non-inline function for copying status' message. - void CopyMessageFrom(const Status& status); + void CopyMessageFrom(const Status& status) noexcept; // A non-inline function for freeing status' message. - void FreeMessage(); + void FreeMessage() noexcept; /// Status uses a naked pointer to ensure the size of an instance on the stack is only /// the sizeof(ErrorMsg*). Every Status owns its ErrorMsg instance. http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5f3996e6/be/src/runtime/mem-pool.cc ---------------------------------------------------------------------- diff --git a/be/src/runtime/mem-pool.cc b/be/src/runtime/mem-pool.cc index e5fe39a..ef3d358 100644 --- a/be/src/runtime/mem-pool.cc +++ b/be/src/runtime/mem-pool.cc @@ -97,7 +97,7 @@ void MemPool::FreeAll() { } } -bool MemPool::FindChunk(int64_t min_size, bool check_limits) { +bool MemPool::FindChunk(int64_t min_size, bool check_limits) noexcept { // Try to allocate from a free chunk. The first free chunk, if any, will be immediately // after the current chunk. int first_free_idx = current_chunk_idx_ + 1; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5f3996e6/be/src/runtime/mem-pool.h ---------------------------------------------------------------------- diff --git a/be/src/runtime/mem-pool.h b/be/src/runtime/mem-pool.h index fdf38e9..15325c0 100644 --- a/be/src/runtime/mem-pool.h +++ b/be/src/runtime/mem-pool.h @@ -86,7 +86,7 @@ class MemPool { /// Allocates 8-byte aligned section of memory of 'size' bytes at the end /// of the the current chunk. Creates a new chunk if there aren't any chunks /// with enough capacity. - uint8_t* Allocate(int64_t size) { + uint8_t* Allocate(int64_t size) noexcept { return Allocate<false>(size); } @@ -94,7 +94,7 @@ class MemPool { /// this call will fail (returns NULL) if it does. /// The caller must handle the NULL case. This should be used for allocations /// where the size can be very big to bound the amount by which we exceed mem limits. - uint8_t* TryAllocate(int64_t size) { + uint8_t* TryAllocate(int64_t size) noexcept { return Allocate<true>(size); } @@ -193,7 +193,7 @@ class MemPool { /// if a new chunk needs to be created. /// If check_limits is true, this call can fail (returns false) if adding a /// new chunk exceeds the mem limits. - bool FindChunk(int64_t min_size, bool check_limits); + bool FindChunk(int64_t min_size, bool check_limits) noexcept; /// Check integrity of the supporting data structures; always returns true but DCHECKs /// all invariants. @@ -207,7 +207,7 @@ class MemPool { } template <bool CHECK_LIMIT_FIRST> - uint8_t* Allocate(int64_t size) { + uint8_t* Allocate(int64_t size) noexcept { if (UNLIKELY(size == 0)) return reinterpret_cast<uint8_t *>(&zero_length_region_); int64_t num_bytes = BitUtil::RoundUp(size, 8); @@ -229,8 +229,8 @@ class MemPool { }; // Stamp out templated implementations here so they're included in IR module -template uint8_t* MemPool::Allocate<false>(int64_t size); -template uint8_t* MemPool::Allocate<true>(int64_t size); +template uint8_t* MemPool::Allocate<false>(int64_t size) noexcept; +template uint8_t* MemPool::Allocate<true>(int64_t size) noexcept; } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5f3996e6/be/src/udf/udf-internal.h ---------------------------------------------------------------------- diff --git a/be/src/udf/udf-internal.h b/be/src/udf/udf-internal.h index 10ce43b..599e42f 100644 --- a/be/src/udf/udf-internal.h +++ b/be/src/udf/udf-internal.h @@ -79,10 +79,10 @@ class FunctionContextImpl { /// FreeLocalAllocations(). This is used where the lifetime of the allocation is clear. /// For UDFs, the allocations can be freed at the row level. /// TODO: free them at the batch level and save some copies? - uint8_t* AllocateLocal(int byte_size); + uint8_t* AllocateLocal(int byte_size) noexcept; /// Frees all allocations returned by AllocateLocal(). - void FreeLocalAllocations(); + void FreeLocalAllocations() noexcept; /// Returns true if there are any allocations returned by AllocateLocal(). bool HasLocalAllocations() const { return !local_allocations_.empty(); } http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5f3996e6/be/src/udf/udf.cc ---------------------------------------------------------------------- diff --git a/be/src/udf/udf.cc b/be/src/udf/udf.cc index 895efc7..4cadb63 100644 --- a/be/src/udf/udf.cc +++ b/be/src/udf/udf.cc @@ -284,7 +284,7 @@ inline bool FunctionContextImpl::CheckAllocResult(const char* fn_name, return true; } -uint8_t* FunctionContext::Allocate(int byte_size) { +uint8_t* FunctionContext::Allocate(int byte_size) noexcept { assert(!impl_->closed_); if (byte_size == 0) return NULL; uint8_t* buffer = impl_->pool_->Allocate(byte_size); @@ -302,7 +302,7 @@ uint8_t* FunctionContext::Allocate(int byte_size) { return buffer; } -uint8_t* FunctionContext::Reallocate(uint8_t* ptr, int byte_size) { +uint8_t* FunctionContext::Reallocate(uint8_t* ptr, int byte_size) noexcept { assert(!impl_->closed_); VLOG_ROW << "Reallocate: FunctionContext=" << this << " size=" << byte_size @@ -321,7 +321,7 @@ uint8_t* FunctionContext::Reallocate(uint8_t* ptr, int byte_size) { return new_ptr; } -void FunctionContext::Free(uint8_t* buffer) { +void FunctionContext::Free(uint8_t* buffer) noexcept { assert(!impl_->closed_); if (buffer == NULL) return; VLOG_ROW << "Free: FunctionContext=" << this << " " @@ -416,7 +416,7 @@ void FunctionContext::SetFunctionState(FunctionStateScope scope, void* ptr) { } } -uint8_t* FunctionContextImpl::AllocateLocal(int byte_size) { +uint8_t* FunctionContextImpl::AllocateLocal(int byte_size) noexcept { assert(!closed_); if (byte_size == 0) return NULL; uint8_t* buffer = pool_->Allocate(byte_size); @@ -431,7 +431,7 @@ uint8_t* FunctionContextImpl::AllocateLocal(int byte_size) { return buffer; } -void FunctionContextImpl::FreeLocalAllocations() { +void FunctionContextImpl::FreeLocalAllocations() noexcept { assert(!closed_); if (VLOG_ROW_IS_ON) { stringstream ss; @@ -454,7 +454,7 @@ void FunctionContextImpl::SetConstantArgs(const vector<AnyVal*>& constant_args) // Note: this function crashes LLVM's JIT in expr-test if it's xcompiled. Do not move to // expr-ir.cc. This could probably use further investigation. -StringVal::StringVal(FunctionContext* context, int len) +StringVal::StringVal(FunctionContext* context, int len) noexcept : len(len), ptr(NULL) { if (UNLIKELY(len > StringVal::MAX_LENGTH)) { std::cout << "MAX_LENGTH, Trying to allocate " << len; @@ -474,7 +474,7 @@ StringVal::StringVal(FunctionContext* context, int len) } } -StringVal StringVal::CopyFrom(FunctionContext* ctx, const uint8_t* buf, size_t len) { +StringVal StringVal::CopyFrom(FunctionContext* ctx, const uint8_t* buf, size_t len) noexcept { StringVal result(ctx, len); if (LIKELY(!result.is_null)) { memcpy(result.ptr, buf, len); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5f3996e6/be/src/udf/udf.h ---------------------------------------------------------------------- diff --git a/be/src/udf/udf.h b/be/src/udf/udf.h index 83551c1..1b1cdab 100644 --- a/be/src/udf/udf.h +++ b/be/src/udf/udf.h @@ -140,7 +140,7 @@ class FunctionContext { /// The UDF/UDA is responsible for calling Free() on all buffers returned by Allocate(). /// If Allocate() fails or causes the memory limit to be exceeded, the error will be /// set in this object causing the query to fail. - uint8_t* Allocate(int byte_size); + uint8_t* Allocate(int byte_size) noexcept; /// Wrapper around Allocate() to allocate a buffer of the given type "T". template<typename T> @@ -155,10 +155,10 @@ class FunctionContext { /// memory limit to be exceeded, the error will be set in this object. /// /// This should be used for buffers that constantly get appended to. - uint8_t* Reallocate(uint8_t* ptr, int byte_size); + uint8_t* Reallocate(uint8_t* ptr, int byte_size) noexcept; /// Frees a buffer returned from Allocate() or Reallocate() - void Free(uint8_t* buffer); + void Free(uint8_t* buffer) noexcept; /// For allocations that cannot use the Allocate() API provided by this /// object, TrackAllocation()/Free() can be used to just keep count of the @@ -573,7 +573,7 @@ struct StringVal : public AnyVal { /// If the memory allocation fails, e.g. because the intermediate value would be too /// large, the constructor will construct a NULL string and set an error on the function /// context. - StringVal(FunctionContext* context, int len); + StringVal(FunctionContext* context, int len) noexcept; /// Will create a new StringVal with the given dimension and copy the data from the /// parameters. In case of an error will return a NULL string and set an error on the @@ -582,7 +582,7 @@ struct StringVal : public AnyVal { /// Note that the memory for the buffer of the new StringVal is managed by Impala. /// Impala will handle freeing it. Callers should not call Free() on the 'ptr' of /// the StringVal returned. - static StringVal CopyFrom(FunctionContext* ctx, const uint8_t* buf, size_t len); + static StringVal CopyFrom(FunctionContext* ctx, const uint8_t* buf, size_t len) noexcept; static StringVal null() { StringVal sv; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/5f3996e6/be/src/udf_samples/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/be/src/udf_samples/CMakeLists.txt b/be/src/udf_samples/CMakeLists.txt index 52cdc81..d316016 100644 --- a/be/src/udf_samples/CMakeLists.txt +++ b/be/src/udf_samples/CMakeLists.txt @@ -21,7 +21,7 @@ set(EXECUTABLE_OUTPUT_PATH "${BUILD_OUTPUT_ROOT_DIRECTORY}/udf_samples") # This should be called with the .cc src file and it will generate a # src-file-ir target that can be built. # e.g. COMPILE_TO_IR(test.cc) generates the "test-ir" make target. -set(IR_COMPILE_FLAGS "-emit-llvm" "-O3" "-c" "-I../" ${CLANG_BASE_FLAGS}) +set(IR_COMPILE_FLAGS "-emit-llvm" "-O3" "-std=c++14" "-c" "-I../" ${CLANG_BASE_FLAGS}) function(COMPILE_TO_IR SRC_FILE) get_filename_component(BASE_NAME ${SRC_FILE} NAME_WE) set(OUTPUT_FILE "${LIBRARY_OUTPUT_PATH}/${BASE_NAME}.ll")
