Skip to content

Commit

Permalink
Preventing some Status copies.
Browse files Browse the repository at this point in the history
  • Loading branch information
benvanik committed Aug 17, 2020
1 parent f802ed8 commit d6dc56a
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 31 deletions.
11 changes: 5 additions & 6 deletions iree/base/internal/status.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@

namespace iree {

std::ostream& operator<<(std::ostream& os, const StatusCode& x) {
os << StatusCodeToString(x);
return os;
}

Status::Status(StatusCode code, absl::string_view message) {
if (code != StatusCode::kOk) {
state_ = absl::make_unique<State>();
Expand Down Expand Up @@ -73,12 +78,6 @@ std::string Status::ToString() const {
return text;
}

bool operator==(const Status& lhs, const Status& rhs) {
return lhs.code() == rhs.code();
}

bool operator!=(const Status& lhs, const Status& rhs) { return !(lhs == rhs); }

std::ostream& operator<<(std::ostream& os, const Status& x) {
os << x.ToString();
return os;
Expand Down
33 changes: 27 additions & 6 deletions iree/base/internal/status.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ static inline const char* StatusCodeToString(StatusCode code) {
return iree_status_code_string(static_cast<iree_status_code_t>(code));
}

// Prints a human-readable representation of `x` to `os`.
std::ostream& operator<<(std::ostream& os, const StatusCode& x);

// A Status value can be either OK or not-OK
// * OK indicates that the operation succeeded.
// * A not-OK value indicates that the operation failed and contains details
Expand Down Expand Up @@ -104,12 +107,30 @@ class ABSL_MUST_USE_RESULT Status final {
// Return a combination of the error code name and message.
std::string ToString() const;

friend bool operator==(const Status&, const Status&);
friend bool operator!=(const Status&, const Status&);

// Ignores any errors, potentially suppressing complaints from any tools.
void IgnoreError() {}

friend bool operator==(const Status& lhs, const Status& rhs) {
return lhs.code() == rhs.code();
}
friend bool operator!=(const Status& lhs, const Status& rhs) {
return !(lhs == rhs);
}

friend bool operator==(const Status& lhs, const StatusCode& rhs) {
return lhs.code() == rhs;
}
friend bool operator!=(const Status& lhs, const StatusCode& rhs) {
return !(lhs == rhs);
}

friend bool operator==(const StatusCode& lhs, const Status& rhs) {
return lhs == rhs.code();
}
friend bool operator!=(const StatusCode& lhs, const Status& rhs) {
return !(lhs == rhs);
}

private:
// TODO(#265): remove message().
absl::string_view message() const;
Expand Down Expand Up @@ -204,9 +225,9 @@ ABSL_MUST_USE_RESULT static inline bool IsUnknown(const Status& status) {
return status.code() == StatusCode::kUnknown;
}

#define CHECK_OK(val) CHECK_EQ(::iree::OkStatus(), (val))
#define QCHECK_OK(val) QCHECK_EQ(::iree::OkStatus(), (val))
#define DCHECK_OK(val) DCHECK_EQ(::iree::OkStatus(), (val))
#define CHECK_OK(val) CHECK_EQ(::iree::StatusCode::kOk, (val))
#define QCHECK_OK(val) QCHECK_EQ(::iree::StatusCode::kOk, (val))
#define DCHECK_OK(val) DCHECK_EQ(::iree::StatusCode::kOk, (val))

} // namespace iree

Expand Down
9 changes: 3 additions & 6 deletions iree/base/internal/status_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,16 @@ StatusCode StatusBuilder::code() const { return status_.code(); }
SourceLocation StatusBuilder::source_location() const { return loc_; }

Status StatusBuilder::CreateStatus() && {
Status result = JoinMessageToStatus(status_, rep_->stream_message);
Status result = rep_->stream_message.empty()
? status_
: Annotate(status_, rep_->stream_message);

// Reset the status after consuming it.
status_ = Status(StatusCode::kUnknown, "");
rep_ = nullptr;
return result;
}

Status StatusBuilder::JoinMessageToStatus(Status s, absl::string_view msg) {
if (msg.empty()) return s;
return Annotate(s, msg);
}

std::ostream& operator<<(std::ostream& os, const StatusBuilder& builder) {
return os << static_cast<Status>(builder);
}
Expand Down
17 changes: 15 additions & 2 deletions iree/base/internal/status_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,23 @@ class ABSL_MUST_USE_RESULT StatusBuilder {
operator Status() const&;
operator Status() &&;

friend bool operator==(const StatusBuilder& lhs, const StatusCode& rhs) {
return lhs.code() == rhs;
}
friend bool operator!=(const StatusBuilder& lhs, const StatusCode& rhs) {
return !(lhs == rhs);
}

friend bool operator==(const StatusCode& lhs, const StatusBuilder& rhs) {
return lhs == rhs.code();
}
friend bool operator!=(const StatusCode& lhs, const StatusBuilder& rhs) {
return !(lhs == rhs);
}

private:
Status CreateStatus() &&;

static Status JoinMessageToStatus(Status s, absl::string_view msg);

// The status that the result will be based on.
Status status_;

Expand Down Expand Up @@ -109,6 +121,7 @@ StatusBuilder& StatusBuilder::operator<<(const T& value) & {
rep_->stream << value;
return *this;
}

template <typename T>
StatusBuilder&& StatusBuilder::operator<<(const T& value) && {
return std::move(operator<<(value));
Expand Down
6 changes: 3 additions & 3 deletions iree/base/status_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ TEST(StatusBuilder, StreamInsertionFlag) {

TEST(StatusMacro, ReturnIfError) {
auto returnIfError = [](Status status) -> Status {
RETURN_IF_ERROR(status) << "annotation";
RETURN_IF_ERROR(std::move(status)) << "annotation";
return OkStatus();
};
Status status = InvalidArgumentErrorBuilder(IREE_LOC) << "message";
status = returnIfError(status);
status = returnIfError(std::move(status));
EXPECT_THAT(status, StatusIs(StatusCode::kInvalidArgument));
EXPECT_THAT(status.ToString(), HasSubstr("message"));
EXPECT_THAT(status.ToString(), HasSubstr("annotation"));
Expand All @@ -84,7 +84,7 @@ TEST(StatusMacro, AssignOrReturn) {
};
StatusOr<std::string> statusOr = InvalidArgumentErrorBuilder(IREE_LOC)
<< "message";
Status status = assignOrReturn(statusOr);
Status status = assignOrReturn(std::move(statusOr));
EXPECT_THAT(status, StatusIs(StatusCode::kInvalidArgument));
EXPECT_THAT(status.ToString(), HasSubstr("message"));
EXPECT_THAT(status.ToString(), HasSubstr("annotation"));
Expand Down
4 changes: 2 additions & 2 deletions iree/hal/cts/cts_test_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@
#include <set>

#include "iree/base/status.h"
#include "iree/testing/status_matchers.h"
#include "iree/hal/driver_registry.h"
#include "iree/testing/gtest.h"
#include "iree/testing/status_matchers.h"

namespace iree {
namespace hal {
Expand Down Expand Up @@ -95,7 +95,7 @@ class CtsTestBase : public ::testing::TestWithParam<std::string> {
if (IsUnavailable(driver_or.status())) {
unavailable_driver_names.insert(driver_name);
}
RETURN_IF_ERROR(driver_or.status());
IREE_RETURN_IF_ERROR(driver_or.status());
return std::move(driver_or.value());
}
};
Expand Down
2 changes: 1 addition & 1 deletion iree/hal/host/condvar_semaphore.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ Status CondVarSemaphore::Signal(uint64_t value) {

void CondVarSemaphore::Fail(Status status) {
absl::MutexLock lock(&mutex_);
status_ = status;
status_ = std::move(status);
value_.store(UINT64_MAX, std::memory_order_release);
}

Expand Down
7 changes: 3 additions & 4 deletions iree/hal/vmla/op_kernels_generic.h
Original file line number Diff line number Diff line change
Expand Up @@ -972,10 +972,9 @@ Status GenericPooling(absl::Span<const T> src_buffer,
for (int j = 0; j < rank; ++j) {
src_indices[j] = dst_indices[j] * strides[j] - pad_low[j];
}
auto status = ComputePoolingWindow<T, KernelImpl>(
src_buffer, src_indices, src_shape, init_buffer[0], window_dimensions,
&dst_buffer[i]);
RETURN_IF_ERROR(status);
ComputePoolingWindow<T, KernelImpl>(src_buffer, src_indices, src_shape,
init_buffer[0], window_dimensions,
&dst_buffer[i]);
IncrementShapeIndex(absl::MakeSpan(dst_indices), dst_shape);
}
return OkStatus();
Expand Down
2 changes: 1 addition & 1 deletion iree/vm/module_abi_packing.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ struct Unpacker {
Unpacker unpacker(registers, argument_list, variadic_segment_size_list);
ApplyLoad<Ts...>(&unpacker, params,
std::make_index_sequence<sizeof...(Ts)>());
RETURN_IF_ERROR(unpacker.status);
RETURN_IF_ERROR(std::move(unpacker.status));
return std::move(params);
}

Expand Down

0 comments on commit d6dc56a

Please sign in to comment.