Just a few quick, knee-jerk-style, questions. My apologies if these are too simplistic or vague.

• Do you happen to have a pointer to some more documentation about this new Swift feature?
• I wonder if any features from other languages could also map to this IR/backend implementation? E.g. I haven't looked at how C++ co-routines are implemented, but would they be able to reuse this feature (assuming they would need a somewhat similar feature in mid- and back-end).
• As this seems to be introducing new ABI - do you have an idea of what the most appropriate ABI specification is this would be best documented in? Do I guess correctly that this is not a Darwin-only feature? If this is really swift-specific (i.e. it does not seem to be a good trade-off to have this mid/back-end feature support similar language features for multiple front-ends), I guess this is Swift-specific ABI. Do I guess correctly that there isn't really a specification for Swift-specific ABI - it's purely defined by its primary implementation?
• I just had one very detailed nit-pick comment on the documentation of frame layout - see specific comment.

My understanding is that AArch64 gnu_ilp32 support is present in mainline gcc, but not in mainline glibc nor in the mainline linux kernel. My understanding is also that it's unlikely that AArch64 gnu_ilp32 support will be added to mainline glibc or linux kernel anytime soon.
That makes me wonder how useful it is to support AArch64 gnu_ilp32 in mainline LLVM. It seems unlikely that it can be used for linux targets anytime soon. Are there non-linux targets using AArch64 gnu_ilp32?

In D93221#2459903, @ostannard wrote:

Why is this restricted to v7-A or later? The DSB and ISB instructions have existed since v6T2 and v6M.

Updated patch based on review comments + rebased to ToT

Updated patch based on review comments + rebased to ToT

Updated patch based on review feedback.

LGTM too, thanks!

Thanks for this.
The proposed changes all LGTM.
I did do a grep on the term 'master' in llvm/docs, and found quite a few more uses of 'master', especially in links.
The command I used was: grep -R master llvm/docs/
Just two examples are:

In D92328#2423327, @george.burgess.iv wrote:

thanks for this!

i think the preferred way to PGO LLVM is cmake: https://llvm.org/docs/AdvancedBuilds.html

that said, while we have this script & the docs pointing to it, seems good to keep it up-to-date. LGTM

In D89068#2326910, @pietroalbini wrote:

This seems to be approved, what's the process of landing the commit? I don't have commit access to LLVM.

Thanks for this @pietroalbini !
This LGTM.
I'd also like to hear @mattdr's or @jfb's opinion before committing though.

LGTM, thanks!

This looks fine to me; I just have a number of nit picks.
The only part where I don't understand the code logic is around the comment starting with "If this is the final byte of a multi-byte sequence".

In D81403#2097148, @zbrid wrote:

LGTM

Could you explain how not adding the MBB entry block is different from removing extra MBB later? Curious on the details about the differences here since the change makes sense functionally and I don't see any issues, but I can't understand why adding the extra MBB entry block at the point we create the machine function and then deleting extra blocks results in something different than the automatically created entry block. Is it some attributes or configuration in the basic block or something?

Updated version based on @ostannard feedback.

This new revision fixes the root cause of GlobalISel asserting on an IndirectThunk: the IndirectThunk creation code was producing a MachineFunction with a slightly different structure than what gets produced from a naked function implemented in C code and processed by clang.
This fixes this by letting the IndirectThunk generator produce a MachineFunction structure that is the same as what gets produced by clang for an empty naked function written in C.

Based on the review feedback that using X16 or X17 could be problematic since a linker can clobber those registers on a call, this patch goes for a different approach to achieve compatibility with BTI-enabled callees.
It simply moves the register to be called always to be in X16, and then does BR X16.
It seems it's simplest to do that unconditionally, as it may not always be easy to find out if the call target has been BTI-enabled.

The updated patch addresses Oliver's review comments.
Especially, it addresses the review feedback regarding potential clobbering of X16 or X17 when the call to the thunks go through a long veneer.
It addresses this by avoiding generating indirect calls that use X16 or X17.

Thanks for the feedback!

I ended up pushing a fix for the racy tests in 7e6f891df85

I ended up pushing a fix for the racy tests in 7e6f891df85

I think I'd like the proposed organization better; but I only have a weak preference.
LGTM.

LGTM

Assuming there is no change in output, LGTM.

This seems OK to me.
I'd just remark that these days, I think most people using LNT to compile/run/benchmark test-suite use "lnt runtest test-suite" rather than "lnt runtest nt".
Therefore, maybe it'd be better if you'd look into how to make that working on the VE architecture (i.e. look at file lnt/tests/test_suite.py).

LGTM. Thanks!

In D74918#1923151, @zoecarver wrote:

There are a lot of different ways we could implement the feature. We may want to only enable it when -march=native, or maybe only in the unstable ABI, and maybe we want to support aligned pairs on some architectures. I think that's an important discussion to have but I'm not sure _this_ patch is the best place to have that discussion.

Even if we don't use this patch in the implementation I think it would still be a good utility to have. Here's what I suggest: I commit this, create another patch to add a builtin that exposes this API, and then open a libc++ patch with a _possible_ implementation. In that patch, we can discuss how we should actually implement the feature and after we have a consensus I can do the work to implement it. Any objections to that plan?

In D74918#1898191, @__simt__ wrote:

In D74918#1897636, @kristof.beyls wrote:

If these values are part of the C++ target platform ABI, it seems to me the values for std::hardware_{constructive,destructive}_interference_size should be set by whoever has the authority to decide C++ platform ABI for specific platforms.
Assuming my thought in the previous sentence is correct; discussions on which values to chose for std::hardware_{constructive,destructive}_interference_size should happen in whichever forums decide C++ platform ABI for the various platforms? (Maybe for some platforms that forum might be clang-related fora like reviews.llvm.org, but probably not for all platforms).
With my (probably limited) understanding of the requirements, it seems like deriving std::hardware_{constructive,destructive}_interference_size from actual cache line size on a specific micro-architecture doesn't seem to be the right approach?

They will be in the library ABI, meaning the libc++ ABI.

It's valid for libstdc++ and libc++ to have different values there. If we wish, we could try for an alignment (no pun intended) on these values, but even then that's just between these two libraries.

Which is good and encouraging, because I don't know what forum we would have to go to.

In D74918#1896930, @__simt__ wrote:

(I assume I'm not seeing a code review being used to veto a C++ Standard feature, but actually the other points are the reason for the red flag.)

I can see a desire for hyper-precise definitions to achieve the best possible performance, but we really need a healthy does of conservatism here.

The C++ values can't change as a result of selecting between microarchitecture variations that are supposed to link, it takes an ABI break to change these.

As someone who isn't up to speed on the design of vector-function-abi-variant, this release note makes sense to me.
It'd probably be best though for this to be approved by someone who understands the design.

After chatting with Oliver, he states that he did run ubsan before and after this change, and the undefined behaviour that was causing the segfaults is gone with this change.
(There are apparently other undefined behaviours flagged by ubsan, but they are unrelated).
I also don't know much about H264 decoding, but the removal of undefined behaviour as indicated by ubsan together with no change in output makes me believe this change is good enough for how the test-suite is used.
LGTM.