Skip to content

Commit

Permalink
Export of internal Abseil changes
Browse files Browse the repository at this point in the history
--
406622c43f296eeedf00e0e9246acfb4ea6ecd5e by Abseil Team <[email protected]>:

Avoid the compiler reloading __is_long() on string_view(const string&)

The underlying cause is that the compiler assume a scenario where string_view is created with placement new into memory occupied by the input, so the store to 'ptr' can affect the value / result of size(); i.e., __is_long() reloads the __size value).

Example code: string_view1 demonstrates the problem, string_view2 DTRT.

=== string_view1
struct string_view1 {
    string_view1(const char* ptr, size_t n) : ptr(ptr), n(n) {}
    string_view1(const std::string& s) : ptr(s.data()), n(s.length()) {}
    const char* ptr;
    size_t n;
};

struct S1 {
    S1(const std::string& s);
    string_view1 sv;
};
S1::S1(const std::string& s) : sv(s) {}

S1::S1
        test    byte ptr [rsi], 1
        je      .LBB0_1
        mov     rax, qword ptr [rsi + 16]
        mov     qword ptr [rdi], rax
        movzx   eax, byte ptr [rsi]
        test    al, 1
        jne     .LBB0_5
.LBB0_4:
        shr     rax
        mov     qword ptr [rdi + 8], rax
        ret
.LBB0_1:
        lea     rax, [rsi + 1]
        mov     qword ptr [rdi], rax
        movzx   eax, byte ptr [rsi]
        test    al, 1
        je      .LBB0_4
.LBB0_5:
        mov     rax, qword ptr [rsi + 8]
        mov     qword ptr [rdi + 8], rax
        ret

=== string_view2
struct string_view2 {
    string_view2(const char* ptr, size_t n) : ptr(ptr), n(n) {}
    string_view2(const std::string& s) : string_view2(s.data(), s.size()) {}
    const char* ptr;
    size_t n;
};

struct S2 {
    S2(const std::string& s);
    string_view2 sv;
};
S2::S2(const std::string& s) : sv(s) {}

S2::S2
        movzx   eax, byte ptr [rsi]
        test    al, 1
        je      .LBB1_1
        mov     rax, qword ptr [rsi + 8]
        mov     rsi, qword ptr [rsi + 16]
        mov     qword ptr [rdi], rsi
        mov     qword ptr [rdi + 8], rax
        ret
.LBB1_1:
        add     rsi, 1
        shr     rax
        mov     qword ptr [rdi], rsi
        mov     qword ptr [rdi + 8], rax
        ret
PiperOrigin-RevId: 272096771
GitOrigin-RevId: 406622c43f296eeedf00e0e9246acfb4ea6ecd5e
Change-Id: I70173a2db68cd9b597fff1c09e00198c632cfe95
  • Loading branch information
Abseil Team authored and CJ-Johnson committed Oct 1, 2019
1 parent debac94 commit 8fe7214
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 1 deletion.
4 changes: 3 additions & 1 deletion absl/strings/string_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,9 @@ class string_view {
string_view( // NOLINT(runtime/explicit)
const std::basic_string<char, std::char_traits<char>, Allocator>&
str) noexcept
: ptr_(str.data()), length_(CheckLengthInternal(str.size())) {}
// This is implement in terms of `string_view(p, n)` so `str.size()`
// doesn't need to be reevaluated after `ptr_` is set.
: string_view(str.data(), str.size()) {}

// Implicit constructor of a `string_view` from nul-terminated `str`. When
// accepting possibly null strings, use `absl::NullSafeStringView(str)`
Expand Down
18 changes: 18 additions & 0 deletions absl/strings/string_view_benchmark.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,24 @@

namespace {

void BM_StringViewFromString(benchmark::State& state) {
std::string s(state.range(0), 'x');
std::string* ps = &s;
struct SV {
SV() = default;
explicit SV(const std::string& s) : sv(s) {}
absl::string_view sv;
} sv;
SV* psv = &sv;
benchmark::DoNotOptimize(ps);
benchmark::DoNotOptimize(psv);
for (auto _ : state) {
new (psv) SV(*ps);
benchmark::DoNotOptimize(sv);
}
}
BENCHMARK(BM_StringViewFromString)->Arg(12)->Arg(128);

// Provide a forcibly out-of-line wrapper for operator== that can be used in
// benchmarks to measure the impact of inlining.
ABSL_ATTRIBUTE_NOINLINE
Expand Down

0 comments on commit 8fe7214

Please sign in to comment.