FWIW, I also tagged bug 47123 for release-11.0.0.

In D85691#2212015, @cuviper wrote:

Oh, when LLDB uses this flag, they have LLDBStandalone.cmake with include(CheckAtomic).

And you were observing this in an lld standalone build? Because otherwise we have llvm/CMakeLists.txt -> llvm/cmake/config-ix.cmake -> llvm/cmake/modules/CheckAtomic.cmake. So yeah, you should probably make sure this is included somehow.

include(CheckAtomic) for standalone builds

Oh, when LLDB uses this flag, they have LLDBStandalone.cmake with include(CheckAtomic).

Well, I now have evidence that this may indeed try to link a nonexistent library -- a new Rust CI run failed on MSVC:

@jfb You did see that this is only for NOT HAVE_CXX_ATOMICS64_WITHOUT_LIB, right? So I'm only adding that link (and hidden locking) for targets that can't do 64-bit atomics natively, otherwise you get link errors like this Rust CI failure on arm. This kind of conditional atomic link exists in several places in the monorepo already.

More whitespace cleanup

In D85691#2208323, @jfb wrote:

You're probably better off putting the std::chrono::nanoseconds::rep behind a lock if it's not always lock free on all platforms. Some platforms just don't have libatomic, so this patch might cause other issues.

Fixed indentation

Can you add tests that show what this is intended to look like for alignments greater than the probe step size?

LGTM, and I confirmed that it works in a full rustc bootstrap, thanks!

I applied this to the Rust fork to measure it in rust#72703, and here are the perf results. It looks like a reasonable gain -- no real effect on check/debug builds, but around 1% fewer instructions executed for optimized builds.

Ping for review, please.

I'll have to review that discussion of this pointers wrt Rust, since self can be different things, but it hasn't caused us problems yet AFAIK.

FWIW, here's more context for how I arrived at this:
https://github.com/rust-lang/rust/pull/57675#issuecomment-456210064

@glaubitz, I was specifically wondering if the author, @LionNatsu, would like to commit this on their own. I'm even more hesitant to intrude on your other review, where I wasn't involved at all.

In D51108#1232917, @LionNatsu wrote:

Thank you all very much. It seems all reviewers have now approved the revision. When will it be landed, just by the way? Did it miss the merge window (if there is one)? 😄

I don't think you need approval from all reviewers, as long as nobody actively disagrees. But FWIW, it looks good to me too.

• Removed the duplicate (and incorrect) declaration in debug-insts.ll debug_declare
• Added debug-cpp.ll which more directly targets this change.

In D51749#1226417, @rnk wrote:

I think this change could use a more direct test, consider starting with the IR generated for a C++ object, like:

struct NTCopy {
  NTCopy();
  NTCopy(const NTCopy &);
  int x;
};
int foo(NTCopy o) {
  return o.x;
}

Superseded by D50096.

D50018 was spurred by failures in the Rust testsuite, and I've confirmed this patch also resolves those failures. Thanks!

To be clear, this function actually combines SHL, SRA, SRL, and ROTL, so they should all be addressed accordingly. I only called out SHL because this was the particular one that bit me.