Skip to content

Commit

Permalink
Remove [[clang::preserve_most]] (#4319)
Browse files Browse the repository at this point in the history
These appear to be causing some subtle misinteractions with MSan that we
don't understand, and may be a compiler bug. =/ Fortunately, they
weren't essential to the performance gains so just remove them for now.
When benchmarked on an x86 server, where I would expect this to be more
important due to relatively few named registers, the performance change
appears to be either an improvement or in the noise.

Huge credit to Jon for tracking down that this is related to the MSan
issues.
  • Loading branch information
chandlerc authored Sep 16, 2024
1 parent 06344ae commit 1d90455
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 4 deletions.
4 changes: 2 additions & 2 deletions common/check_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ namespace Carbon::Internal {
// `llvm::formatv` which handles all of the formatting of output.
template <TemplateString Kind, TemplateString File, int Line,
TemplateString ConditionStr, TemplateString FormatStr, typename... Ts>
[[noreturn, gnu::cold, clang::noinline, clang::preserve_most]] auto CheckFail(
Ts&&... values) -> void {
[[noreturn, gnu::cold, clang::noinline]] auto CheckFail(Ts&&... values)
-> void {
if constexpr (llvm::StringRef(FormatStr).empty()) {
// Skip the format string rendering if empty. Note that we don't skip it
// even if there are no values as we want to have consistent handling of
Expand Down
4 changes: 2 additions & 2 deletions common/vlog.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ namespace Carbon::Internal {
// Internally uses `llvm::formatv` to render the format string with any value
// arguments, and streams the result to the provided stream.
template <TemplateString FormatStr, typename... Ts>
[[clang::noinline, clang::preserve_most]] auto VLogImpl(
llvm::raw_ostream* stream, Ts&&... values) -> void {
[[clang::noinline]] auto VLogImpl(llvm::raw_ostream* stream, Ts&&... values)
-> void {
*stream << llvm::formatv(FormatStr.c_str(), std::forward<Ts>(values)...);
}

Expand Down

0 comments on commit 1d90455

Please sign in to comment.