Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alias barriers; a replacement for the ICU hack #67

Open
tahonermann opened this issue Mar 6, 2021 · 7 comments
Open

Alias barriers; a replacement for the ICU hack #67

tahonermann opened this issue Mar 6, 2021 · 7 comments
Assignees
Labels
enhancement New feature or request paper needed A paper proposing a specific solution is needed

Comments

@tahonermann
Copy link
Member

tahonermann commented Mar 6, 2021

ICU defines a U_ALIASING_BARRIER macro that is used to allow ICU to use char16_t internally while also providing interfaces that work with text stored in wchar_t (when it is a 16-bit type) or uint16_t (when available) without having to copy the text to and from char16_t based storage. This is important for efficient operation on Windows and with other libraries that use UTF-16 internally, but that do not use char16_t as their UTF-16 character type.

For most compilers, the U_ALIASING_BARRIER macro is a no-op and ICU relies on the compiler not taking advantage of char16_t being a distinct non-aliasing type of the other ICU supported UTF-16 character types.

For Clang and gcc, ICU defines the macro as follows and invokes it immediately before using reinterpret_cast to convert between pointers to char16_t and other supported UTF-16 character types. The (volatile) inline assembly prevents the optimizer from reordering loads and stores across the inline assembly and the "memory" clobber informs the compiler that memory read before the inline assembly must be re-read, thus forming a read/write memory barrier. See the gcc documentation for more details.

#define U_ALIASING_BARRIER(ptr) asm volatile("" : : "rm"(ptr) : "memory")

The introduction of char8_t as a non-aliasing type in C++20 creates a similar need for some form of an alias barrier that allows limited interchange between libraries that use char8_t for UTF-8 data internally and those that use char or unsigned char for UTF-8 data internally. Though the same problem applies in principle for char8_t with respect to char16_t, in practice this is less of a concern because char and unsigned char are aliasing types.

Converting a pointer to one type to a pointer to another unrelated type requires use of reinterpret_cast and that prevents performing such conversions in constant expressions and, likely, introduces UB. An alias barrier could potentially allow such conversions in constant expressions between types that meet certain compatibility requirements; for example, a common underlying type.

Recent exploration of this area has uncovered some simple test cases that demonstrate that an alias barrier is needed in practice for some compilers. The following links contain code that does not perform as intended. In each case, three attempts are made to "fix" the example using various approaches. Of the approaches tried, ICU's volatile inline assembly trick is the only one that works in all cases. In each case, the intended behavior is that the program output "Hi" when run.

Though ICU's inline assembly trick does seem to work for all of these cases, it has the downsize of pessimizing optimizers more than is necessary or desired. A more targeted solution is therefore desired.

@tahonermann tahonermann added enhancement New feature or request help wanted Extra attention is needed paper needed A paper proposing a specific solution is needed labels Mar 6, 2021
@jensmaurer
Copy link
Collaborator

jensmaurer commented Mar 6, 2021 via email

@tahonermann
Copy link
Member Author

tahonermann commented Mar 7, 2021

That is a daring approach, and I'm flabbergasted that it appears to work for "most compilers".

It is, and I suspect it does not actually suffice for "most compilers". My "most compilers" statement was derived from the fact that ICU defines the U_ALIASING_BARRIER macro as empty for all compilers other than gcc and Clang. However, someone building the library can define U_ALIASING_BARRIER as needed for the compiler being used.

ICU's platform support is documented here.

Markus Scherer has reported discussing alias concerns with Microsoft engineers where he was assured that Visual C++ will never treat wchar_t, char16_t, and unsigned short as non-aliasing types. I suspect other compilers that target Windows, like Intel's icc, follow suit.

Outside of Windows, gcc and Clang probably cover most of the real world use of ICU these days.

I suspect that, even where ICU is using the U_ALIASING_BARRIER macro today, there may be a fair amount of "getting lucky" going on.

@jensmaurer
Copy link
Collaborator

With Richard's example code,

template<typename T, typename U> U f(T *p, U *q) {
  *p = 1;
  U u = *q;
  *p = 2;
  return u;
}

only clang optimizes unsigned short / char16_t; icc, MSVC, and gcc do not. This feels like a bug. MSVC doesn't even optimize the long/int combination (but icc does). I've used "/Ox /Og /O2" for MSVC, knowing nothing about that compiler.

https://www.godbolt.org/z/Mxbf6W

@tahonermann
Copy link
Member Author

I've found reports online that MSVC doesn't perform TBAA at all.

I agree the gcc behavior feels like a bug. The relevant code is here and here.

@tahonermann
Copy link
Member Author

In off-list discussion, Richard Smith noted that P0593R6 discusses a std::start_lifetime_as() function template that could be used to address this issue. This would require one call to produce an object of the alias type from the storage of an existing object, and then another call to transition the (possibly modified) storage back to the original object type.

@tahonermann
Copy link
Member Author

This issue was discussed in the context of P2626R0 (charN_t incremental adoption: Casting pointers of UTF character types) during the 2024-05-22 SG16 meeting.

No polls were taken, but it is clear that we need to get a better understanding of core language limitations to make further progress on this issue.

@pinskia
Copy link

pinskia commented Jun 26, 2024

With Richard's example code,
...
only clang optimizes unsigned short / char16_t; icc, MSVC, and gcc do not. This feels like a bug. MSVC doesn't even optimize the long/int combination (but icc does). I've used "/Ox /Og /O2" for MSVC, knowing nothing about that compiler.

So looking again at the GCC code, I see char8_t was handled here:
gcc-mirror/gcc@2d91f79

But when char16_t was added:
gcc-mirror/gcc@c466b2c

Was not done the same. It is conseratively correct. Let me file a bug.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115658

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request paper needed A paper proposing a specific solution is needed
Development

No branches or pull requests

4 participants