LGTM

I don't know too much about statepoints or GC, but you have addressed all of @reames comments, and the AArch64 parts and code style look correct to me, so LGTM.

LGTM

This test case has been failing on many of the ARM and AArch64 bots all weekend, e.g. http://lab.llvm.org:8011/builders/clang-cmake-armv7-quick/builds/19408, would you mind looking into this?

LGTM

You haven't addressed my earlier inline comments.

LGTM

LGTM

This was blocked on D76291 for a while, but that is now committed, so I'll re-land this today.

That sounds very fragile - I can easily imagine future, unrelated changes removing that COPY, or emitting one in the case which doesn't currently have one, which would completely change the effect of this patch.

only if the register holding function's address got spilled

LGTM

I don't think it makes sense to make f16 legal for targets which don't have any arithmetic operations on it, since that would be contrary to the definition of "legal". I'd also expect doing so to introduce a lot more complexity than this patch.

I'd expect to see some extra checks to decide if reverting the indirect-call optimisation is profitable or not. If the loaded address is used more than three times, without being spilled, then I think it's better to leave the calls as indirect ones. If you don't think this is ever likely to be worthwhile for thumb 1, then it would be better to modify ARMTargetLowering::LowerCall to not do the transformation in the first place.

LGTM

LGTM

LGTM

I think we could make this conditional based on whether the caller has BTI enabled, because:

• If the caller has BTI enabled, then we correctly use X16 for a BTI callee, or wastefully (but still with correct behaviour) use X16 for a non-BTI callee.
• If the caller has BTI disabled, then it must be in a page with BTI disabled, and so is able to use BR with any register, even if the destination is BTI-protected.

LGTM, with some comments which can be addressed later given the time-sensitivity of this.

LGTM. Maybe we should rename MQPR and DPR_VFP2 since they're not really specific to those architectures, but obviously not in this patch.

Is this expected to work for the soft-float calling convention, or is clang still passing half-precision values as integer types for that? If the former, then this needs some tests for that case.

LGTM, thanks for working on this patch series, this looks much clearer than doing it in clang.

LGTM

I have the same concern as in D81402: will this still work if the call to the thunk goes through a long-branch veneer or PLT, which could clobber x16 and x17?

LGTM, inline comment could be done as a follow-up patch given that this is time-sensitive (recently published security vulnerability).

A few nits, but given the time sensitivity of this the only blocking one is how this will work with PLTs or long-branch veneers,

LGTM

The pre-commit test failures are in the newly added test, so need to be fixed before this can be committed. My inline comments could be addressed as follow up patches given that this is related to a recently-published security vulnerability.

LGTM

Since this patch adds loads, stores, moves and the calling convention, couldn't those be tested here, without waiting for intrinsics?

LGTM

This is causing the 32-bit ARM bots to fail test tools/obj2yaml/ELF/program-headers.yaml. It looks like an out of memory error, but it has been happening consistently for a large number of builds now, and looks related to this change.

Ok, LGTM.

LGTM

Should poly128_t be available on AArch32 too? I don't see anything in the ACLE version you linked restricting it to AArch64 only, and the intrinsics reference has a number of intrinsics available for both ISAs using it.

LGTM

LGTM

LGTM

• Testing that the expression is the subtraction of two SymbolRefs seems needlessly restrictive, could we accept any expression, and reject it in the AsmBackend if it can't be fully resolved or converted to a relocation?
• Does this check that the value fits into a 16 bit immediate? The GNU assembler also seems to accept expressions which fit into a MOVZ/MOVN with a shift, though I can't see that being very useful for expressions involving symbol addresses.
• This needs tests.

Actually, if the destination is XZR, the adds won't have any effect, so surely we don't need to emit any instructions? Could we just early-return without emitting anything if DestReg == XZR?

Committed as 5acafc540ad61c4cab90fbe3323c795d5210b72f.

It would be nice to also print the mnemonic as a comment on the hint instruction, as these will commonly show up in compiled code. It looks like AArch64InstPrinter has access to a CommentStream which could be used for this.

Ping.

Ping.

I think the failure was a bug in the machine verifier: D77783.

LGTM

Thanks for the reproducer, I'll look into this.

This is causing a compile failure on the 32-bit ARM bots:

LGTM

It looks like the issues this is warning about isn't the .debug_line section, but the DW_AT_low/high_pc attributes of the compilation unit DIE. With DWARF3 we can emit a DW_AT_ranges attribute instead, but that didn't exist in DWARF2.

Committed as rG6739805e24

I agree with @dnsampaio here, it's better to match the existing style, and avoid irrelevant churn in the git history.

• I managed to reproduce the lsl-zero.s test failure on windows, this turned out to be because the test merges stdout and stderr, which can result in them being interleaved in ways which breaks tests. The test doesn't check anything in stderr, so fixed by redirecting that to /dev/null.
• The unit test was failing when built with MSVC, fixed by explicitly using UTF-8 string literals.

See D76291 for that split-put patch.

@thakis, thanks for reverting, sorry I wasn't more proactive about that yesterday.

My reply by email doesn't seem to have made it into phab, so repeating here (sorry for spamming everyone who did get the email):

The pre-commit test failure is also failing on the buildbots, not related to this patch.

I forgot to restrict the new tests to only run when the relevant disassembler is built, should be fixed by rG161f70eae6cd and rG2878c6693875.