Skip to content

Commit 08b818e

Browse files
committed
Tell the optimizer the value in unwrap_unchecked
Result::unwrap_unchecked writes to state_ but also assumes it is always an Ok value. The optimizer fails to remove branches that construct the Result currently, but succeeds if we tell it that the state_ is an Ok value before clobbering it. Repeat the same thing in Result::unwrap_err_unchecked and in Option::unwrap_unchecked for good measure. The difference can be seen in here: https://godbolt.org/z/Gax47shsb Without it, the signed code generation is terrible. But with the optimizer hint, it's as good as working with ints directly.
1 parent 8a2541a commit 08b818e

File tree

2 files changed

+21
-1
lines changed

2 files changed

+21
-1
lines changed

sus/option/option.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -847,7 +847,14 @@ class Option final {
847847
/// verified the value appropriately before use in order to not panic.
848848
constexpr inline T unwrap_unchecked(
849849
::sus::marker::UnsafeFnMarker) && noexcept {
850-
return t_.take_and_set_none();
850+
if (t_.state() == Some) {
851+
return t_.take_and_set_none();
852+
} else {
853+
// Result::unwrap_unchecked benefits from telling the compiler explicitly
854+
// that the other states are never set. Match that here until shown it's
855+
// actually not useful.
856+
sus::unreachable_unchecked(::sus::marker::unsafe_fn);
857+
}
851858
}
852859
constexpr inline T unwrap_unchecked(
853860
::sus::marker::UnsafeFnMarker) const& noexcept

sus/result/result.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -857,6 +857,14 @@ class [[nodiscard]] Result final {
857857
/// Behavior.
858858
constexpr inline T unwrap_unchecked(
859859
::sus::marker::UnsafeFnMarker) && noexcept {
860+
if (state_ != ResultState::IsOk) {
861+
// This enables Clang to drop any branches that were used to construct
862+
// the Result. Without this, the optimizer keeps the whole Result
863+
// construction, possibly because the `state_` gets clobbered below?
864+
// The signed code version at https://godbolt.org/z/Gax47shsb improves
865+
// greatly when the compiler is informed about the UB here.
866+
sus::unreachable_unchecked(::sus::marker::unsafe_fn);
867+
}
860868
state_ = ResultState::IsMoved;
861869
if constexpr (!std::is_void_v<T>) {
862870
return ::sus::mem::take_and_destruct(::sus::marker::unsafe_fn,
@@ -897,6 +905,11 @@ class [[nodiscard]] Result final {
897905
/// Behavior.
898906
constexpr inline E unwrap_err_unchecked(
899907
::sus::marker::UnsafeFnMarker) && noexcept {
908+
if (state_ != ResultState::IsErr) {
909+
// Match the code in unwrap_unchecked, and tell the compiler that the
910+
// `state_` is an Err before clobbering it.
911+
sus::unreachable_unchecked(::sus::marker::unsafe_fn);
912+
}
900913
state_ = ResultState::IsMoved;
901914
return ::sus::mem::take_and_destruct(::sus::marker::unsafe_fn,
902915
storage_.err_);

0 commit comments

Comments
 (0)