Commited in af6d2a0b6825e71965f3e2701a63c239fa0ad70f

Addressing comments, stylistic corrections.

In D127812#3585249, @erichkeane wrote:

I'm concerned as to the design of this addition, I don't particularly appreciate the reasons for making 'target_clones' different, nor the purpose for adding a new attribute instead of using 'target' for what seems like exactly that? IF the new spelling is THAT necessary, we perhaps don't need a whole new attribute for it either.

Thank you for fair concern, "target_clones" for AArch64 has different format, semantic, e.g. "default" is not required. Therefore it diverges with X86 in these parts. "target" attribute has been already used and supported on AArch64 in a different sense, like target("arm"), target("dotprod"), target("branch-protection=bti"). The intention of creating new "target_version" attribute is not to overlap with that. It also has different format, mangling and semantic, e.g. treating function without attribute as "default", and option to disable attribute droping function multi versions. Do these explanations dispel your concern?

I think it looks reasonable to define 5th memory model, add barriers __sync_* builtins and to outline-atomics calls as well.

Just a friendly ping. Jessica @paquette, do you have any further suggestions?

Doxygen & comments update.

Work on comments.

In D98564#2624774, @dmgreen wrote:

I see that this is an extension to the existing peephole optimizations, but is there any reason not to do this during ISel?

Tim, thank you for review, noticed typos and overall support to outline atomics patches!
Notes pushed rG98f06b16a313ece593f5711778d7da9037f3a2ef

Fixing typos

@tambre , as a fallback, is it possible to disable building LSE atomics via a cmake option? I tried COMPILER_RT_HAS_ASM_LSE=OFF, but it didn't work.

There is no such option, as clang outline atomics (enabled by default for Linux so far with intend to extend it for other targets) are rely on compiler-rt support as well.
I would recommend to backport this workaround if statement separator for MSVC takes long.

LGTM, thank you for quick fix!

Tests for "-m[no]outline-atomics" options added.

Yes, I missed it, thanks for fix!

Clang driver tests for outline atomics were added.

@t.p.northover sorry for pinging you, do you have any concerns about latest patch? It enables outline atomics by default on Aarch64/Linux/Clang with libraries check. I am hoping to get this in before LLVM 12 branch.

RT library detection and check for outline atomics support added to the driver.

You said Android is planning to adopt this. Do you know how? I thought they had their own libc; does it also implement getauxval, or do they have a separate compiler-rt, or something entirely different?

Android uses compiler-rt and has getauxval support: https://android.googlesource.com/platform/bionic/+/master/libc/bionic/getauxval.cpp

There would need to be more checks in the CompilerRT case because most platforms won't benefit. There's already Clang logic to discover GCC installations (search for GCCInstallationDetector); I hope you'll be able to use that instead of trying to redo it.

Thanks for the tip, I'll try to utilize it.

I think this ought to be a Clang patch that detects which platform and libgcc it's targeting before adding the attribute.

Do you mean implementing in Clang Driver something like

Outline atomics were added with gcc 9.3.1 and turned on by default in gcc 10.1. Consequently most of distributions had libgcc with outline atomics already.
Besides Linux, Android will utilize them as well. Don't know about iOs and MacOs, I guess they are compiling with LSE enabled, so outline atomics should not affect them.
As for benchmarks, I rely on investigations completed during gcc outline atomics enablement work.
Choice of solution ../gcc/libgcc/config/aarch64/lse.S:

Thanks for fixing this.

lse.S syntax fixed, compiler-rt with outline atomics should be built fine. If there are no any other issues can we abandon this patch?

It'd be great if you could figure out a way to have the macro expand in a way that doesn't hit the assembly parser bug.

Newline for .cfi_startproc https://reviews.llvm.org/D93236

In D93211#2452123, @tambre wrote:

@ilinpv D92530 helped, but CFI start and end directives still seem to cause problems. Any ideas?

In D93199#2451569, @tambre wrote:

In D93199#2451557, @ilinpv wrote:

In D93199#2451470, @tambre wrote:

@ilinpv Any ideas why the Apple buildbots might be failing (example)? The errrors seem to be about explicit relocation types missing, but I can't see anything in my changes that could've caused those.

Fix for lse.S is not landed yet: https://reviews.llvm.org/D92530

Any reason for that to not have landed yet? If not, I could probably land that prior to recommiting mine.

I guess because of patch https://reviews.llvm.org/D92836 removing outline atomics for Apple.
Anyway it would be great to have Apple syntax fix landed, thank you for help.

In D93199#2451470, @tambre wrote:

@ilinpv Any ideas why the Apple buildbots might be failing (example)? The errrors seem to be about explicit relocation types missing, but I can't see anything in my changes that could've caused those.

Thanks for fix, creating symlinks is more elegant and firm solution indeed.

lse.S syntax could be fixed: https://reviews.llvm.org/D92530
The question is, should outline atomics be in compiler-rt for iOS and ARM-based macOS?

LGTM. Thanks!

In D91156#2430388, @mstorsjo wrote:

This broke building for non-ELF targets, I'll send a patch for fixing that.

In D91156#2430422, @mstorsjo wrote:

Also, I see that this wasn't actually sent to any mailing list - I guess we should have some list auto-added for anything touching compiler-rt?

Thank you, does anyone have any more concerns? Could you accept revision?

Correct getauxval check.

Comments were taken into account.

LCALL4 removed in TargetLoweringBase.

Have you got any further comments?

Work on comments has been done.

Need to resolve license issue first.

In D91156#2385574, @t.p.northover wrote:

This is a port of libgcc implementation

I take it the licensing stuff all works out because Linaro is the original author and you're working with them?

The patch landed b9a6fb64281b6836e565ee39fb0d543bf184fd88

vbit/vbif encoding tests added.

Patterns corrected to comply with encoding 4eca1c06a4a9183fcf7bb230d894617caf3cf3be