diff options
| author |  Martijn Vels <mvels@google.com> | 2024-02-01 05:37:01 -0800 |
|---|---|---|
| committer |  Copybara-Service <copybara-worker@google.com> | 2024-02-01 05:38:02 -0800 |
| commit | 513a6f93997844a931c68506840a2ca9f5485753 (patch) | |
| tree | 062f45b3e07a97fd981c51955b548dfe22038e60 | |
| parent | 4c7e7c7d94eaaa3bff3142c257d880a468a35934 (diff) | |
Optimize `Cord::Swap()` for missed compiler optimization in clang.
PiperOrigin-RevId: 603342563
Change-Id: I1cd80103377f457770d5178dad8b56ae459cbd55
| -rw-r--r-- | absl/strings/cord.h | 3 | ||||
| -rw-r--r-- | absl/strings/internal/cord_internal.h | 20 |
2 files changed, 22 insertions, 1 deletions
diff --git a/absl/strings/cord.h b/absl/strings/cord.h index b3e556b6..d2ba9673 100644 --- a/absl/strings/cord.h +++ b/absl/strings/cord.h @@ -1170,7 +1170,8 @@ inline void Cord::InlineRep::Swap(absl::Nonnull<Cord::InlineRep*> rhs) { if (rhs == this) { return; } - std::swap(data_, rhs->data_); + using std::swap; + swap(data_, rhs->data_); } inline absl::Nullable<const char*> Cord::InlineRep::data() const { diff --git a/absl/strings/internal/cord_internal.h b/absl/strings/internal/cord_internal.h index 8744540e..549f9175 100644 --- a/absl/strings/internal/cord_internal.h +++ b/absl/strings/internal/cord_internal.h @@ -520,6 +520,7 @@ class InlineData { constexpr InlineData(const InlineData& rhs) noexcept; InlineData& operator=(const InlineData& rhs) noexcept; + friend void swap(InlineData& lhs, InlineData& rhs) noexcept; friend bool operator==(const InlineData& lhs, const InlineData& rhs) { #ifdef ABSL_INTERNAL_CORD_HAVE_SANITIZER @@ -770,6 +771,12 @@ class InlineData { char data[kMaxInline + 1]; AsTree as_tree; }; + + // TODO(b/145829486): see swap(InlineData, InlineData) for more info. + inline void SwapValue(Rep rhs, Rep& refrhs) { + memcpy(&refrhs, this, sizeof(*this)); + memcpy(this, &rhs, sizeof(*this)); + } }; // Private implementation of `Compare()` @@ -884,6 +891,19 @@ inline void CordRep::Unref(CordRep* rep) { } } +inline void swap(InlineData& lhs, InlineData& rhs) noexcept { + lhs.unpoison(); + rhs.unpoison(); + // TODO(b/145829486): `std::swap(lhs.rep_, rhs.rep_)` results in bad codegen + // on clang, spilling the temporary swap value on the stack. Since `Rep` is + // trivial, we can make clang DTRT by calling a hand-rolled `SwapValue` where + // we pass `rhs` both by value (register allocated) and by reference. The IR + // then folds and inlines correctly into an optimized swap without spill. + lhs.rep_.SwapValue(rhs.rep_, rhs.rep_); + rhs.poison(); + lhs.poison(); +} + } // namespace cord_internal ABSL_NAMESPACE_END |