You are right, yes, the implicit landing pad in PACIxSP is configurable through SCTRL_EL1.BT0. It can behave either as BTI c or BTI jc. Since the value of a sysreg is not known at compile time, I took the safe approach here and assumed that PACIxSP could allow all kinds of jumps and therefore they would be unsafe: better have an explicit BTI c than an implicit BTI jc.

LGTM too.

Suggest clarifying the commit message to something like:

I'll leave the review open for a couple of days in case anyone sees a problem with the additional .addReg(AArch64::LR), otherwise I'll commit.

The following commit added a register in the definition of the PAC instructions:

@efriedma thank you for the review. Commit updated without the special outlining handling.

Ping

Ping

All comments addressed (thank you @danielkiss). Does it look reasonable now?

Updated PAC handling in the outliner. Now all outlined functions should use v8.3a instructions where possible.

Added handling of outlining for PACI* similarly to PACI*SP etc, and RegState info to SP.

Sure thing, thank you for the review. @MaskRay please let me know if you want to further discuss the changes to the patchable-function-entry test, otherwise I will commit this sometime next week.

Ping.

For patchable functions are there any other cases where we wouldn't generate a BTI at the start of the function? If there is we may be alter the test so that it is still checking that the nop is still inserted. If there is no way to avoid a BTI then we may have to alter the expected result and the comment.

I've dug up https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92424 and https://reviews.llvm.org/D72215 for context.

Reintroduced the removed fpatchable-function-entry test, but modified so that it expects a landing pad at the beginning of the function.

In D99417#2653048, @peter.smith wrote:

My guess is that this would only affect a small number of functions?

@MaskRay sorry to pull you into this review - feel free to ignore if you're not interested.

Diogo, Victor, thanks for the review. I am going to attribute the failure in the HWAddressSanitizer to something else, since it looks x86-related and this patch doesn't touch anything near x86 support.

This is the same logic that we applied to PAC mnemonics in https://reviews.llvm.org/D78372. I'll approve but I would recommend to wait for a few days in case anyone else has something to say.

Another rebase to pass C.I. - looks like it was broken outside this commit

Rebase to trigger C.I.

Updated tests and rebase conflict with new "isAuthenticated" predicate

Ping

Reworded all the FP & SIMD constraints

Discussing with @chill on a chat, he was happier with the following wording:

Hi, CMSE upstreaming is indeed one of our priorities. So yes, we are very interested in your feedback. And no, CMSE upstreaming is not abandoned, just going a bit slow ATM :( but any help reviewing is much appreciated! :)

Oooops sorry, not sure how I missed that. I've added SSBS to all the CPUs now.

Add Cortex-A65 and a few features that were missed in the first patch. I think all the features should be in now.

These are trace extensions that will be used by a niche group of developers. They add no instructions (only system registers) and they will not be generated by the compiler. Adding them by default does not have any side-effect, i.e. everyone will still see the same behavior in their code unless they start using these registers on purpose.

Same idea as https://reviews.llvm.org/D54961 but for two other command line options. Approved!

Rebased onto master after a recent refactoring of the AArch64 target parser.

Thank you for the review, Sam :)

The Armv8.5-A specification is available here: https://developer.arm.com/products/architecture/cpu-architecture/a-profile/exploration-tools

Buildbot failing here: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap-msan/builds/2998

If nobody does it before, I'll give it a try early next week.

I'll commit this fix now to prevent other buildbots to fail.

Related fix for a silly errata in one of the tests that is breaking some Windows buildbots:

Committed now. @rengolin many thanks for the review!

What about 32-bit integers?

There is still the possibility that someone tries to use 't' for a vector of two doubles. Only single-precision is allowed in vector operations for 32-bit architectures, so doing something like this would be illegal:

Added tests for int vectors. Allowing integers to go to FP/vector registers is
useful because FP/int conversion instructions (i.e. VCVT) need that.

AFAICS, the current approach just checks the size of the type, not the size of the sub-type. f64 or even integer types could still leak in, no?

To prove they're not, we need tests making sure they break if you try.

In D42962#1005332, @rengolin wrote:

In D42962#1005311, @pbarrio wrote:

I was wrong when I said the GNU modifiers are q/e, which actually makes things easier. The correct operand modifiers to select a quad/double vector register in GCC are q/P. These already work in LLVM (they are just ignored according to the documentation and also my local testing). So, I think there is no need for an additional patch; we should be able to handle inline assembly written for GCC with the 't' constraint.

I'm not sure I get this. Are you saying this patch can be abandoned?

In D42962#1005102, @rengolin wrote:

This goes against the documentation, which only supports sN:
https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html#Machine-Constraints

Though it's not completely wrong to support the low part of D/Q registers, I'm not sure the code in question is making sure this is true.

In D42962#1000360, @pbarrio wrote:

In D42962#999509, @efriedma wrote:

This behaviour still differs from that of GCC but I think it is actually more correct, since LLVM picks up the right register type based on the datatype of x, while GCC would need an extra operand modifier to achieve the same result

If we're not going to match gcc, what's the point?

This patch allows specifying the lower Q/D vector registers from inline assembly, which is something that can be done in GCC but not in LLVM. In order to mimic the GCC behaviour completely, we should also add support for the q/e/f operand modifiers with the 't' constraint. These modifiers are already allowed with the 'w' constraint for the complete vector register set, so it shouldn't be hard to do. However, I think it should be a separate patch with additional testing.

Ping

In D42962#999509, @efriedma wrote:

This behaviour still differs from that of GCC but I think it is actually more correct, since LLVM picks up the right register type based on the datatype of x, while GCC would need an extra operand modifier to achieve the same result

If we're not going to match gcc, what's the point?

Sorry, I missed the sanitizer failure in yesterday's buildbot message and thought it wasn't related to this patch. I will leave some time for @thebolt to fix it, otherwise maybe I can take a look.

Committed now. Thanks for the patch @thebolt!

I'll do it.

LGTM now. In other circumstances, I would wait for someone more experienced than me, but this is a small peephole optimization. I've also tested the patch and it works correctly, so I think it has very little risk.