@philipp.tomsich Reverse Ping. What are your plans for this change?

This change should probably be marked [NFC] - because the disassembly output won't change in either case, and nor will the result of assembling into an object - the only difference in behaviour is if you parse assembly and then immediately print it again (like these tests do).

To clarify the changes to the tests: they check that we can parse both hint #N and the mnemonic, and when printed they both print out exactly the same way, based on features (the mnemonic if you have pauth, or as hint #n otherwise).

I only have one comment, beyond that this looks pretty good to me. Please don't forget the licensing info, it's fairly important to add either with or before the test sources are added, so we are totally clear on what is going on. Note with the C test suite, I landed the CMake first, and then the GPL-licensed test sources, rather than the other way around.

In D142638#4202443, @karineek wrote:

@lenary: I assume the .ll files shall go to the tests in the llvm project (e.g. llvm-project/tree/main/llvm/test/CodeGen/X86), right?
Also, how shall we build these? (we did not use emit-llvm flag, but maybe should have?)

LGTM.

Don't change the source code, please change the CMakeLists.txt to disable this test if it's incorrect.

@jhenderson @int3 I think I have addressed your feedback - are you happy for me to land this?

The CHKFEAT instruction here is not exactly as described in https://developer.arm.com/downloads/-/exploration-tools today - but the next release of that description will be updated with the new syntax.

Tagging reviewers. @mehdi_amini I added you as you did the same change to the Support library in rG402461beb051b6a5c158f1e36d8e2c2b676e8804.

In D143712#4165622, @dmgreen wrote:

It's a bit odd to read code with the filter-out push/pop, but LGTM.

Tests and comment updated. I also split this patch stack away from the unrelated changes for inline asm handling of fp16/bf16.

Most recent change adds tests with multiple uses of the returned value.

In D144638#4155217, @michaelplatings wrote:

I see some of these tests previously had UNSUPPORTED: system-netbsd but not UNSUPPORTED: system-windows - do you know why?

Hopefully this addresses the feedback so far.

Updates incoming to add a specific check for netbsd and windows, which are currently excluding the affected tests anyway.

In D144638#4147133, @jhenderson wrote:

This looks reasonable to me, with the caveat that I don't know a huge amount about how the different OSes access time systems work. One question though: if your antivirus was causing flakiness (as opposed to outright always-fails), won't it just move that flakiness into whether the REQUIRES calculation returns true or not (i.e. it could spuriously do so, causing the tests to be enabled but then potential still be flaky?

Adding more reviewers. @mgorny you disabled some tests on NetBSD in the past for the same reason (rG92dc7dce4a6f117a497ced1650bc48e5b658f0ea), this just updates how they were disabled.

In D143713#4139346, @dmgreen wrote:

Do you mean the AES pass tests? Do you think that should happen before we land this, they've been an issue for a little while.

I think I just meant https://reviews.llvm.org/D143712#4117617 for "clear up the tests". A Multiple uses test would be useful to check that nothing odd goes on when the returned value is used elsewhere too.

In D143713#4122025, @dmgreen wrote:

Sounds OK if you can clear up the tests. It might be worth making sure there is a test with multiple uses too.

In D143885#4122133, @rogfer01 wrote:

In D143885#4122115, @lenary wrote:

Which configurations are still using this? Just Polly (via isl), or others too?

flang still uses this header.

Which configurations are still using this? Just Polly (via isl), or others too?

@steven_wu @aprantl @dblaikie I'm abandoning this change, because as of rGd768bf994f50 these headers are not used in LLVM (apart from in Polly's isl import, which I have contacted the isl maintainer about). Hopefully this means you can update your clang modules/bazel configurations to properly consider the TargetParser separately from Support, if you wish.

Offline, I suggested to Simon that the info about whether you have thumb2 is available in two places: in the subtarget, and also in the target triple's subarch, and that maybe he could use the latter instead of adding an arm-architecture-specific hook to TTI (which would be better to avoid). He reported that the final subarch during LTO depends on the object order during the link, rather than on something more determinstic, and therefore using the triple feels to me less reliable than the current approach in this patch.

I dislike adding this to the target-independent API - is there a way we can express this with a downcast to an arm-backend-specific class and query on it, rather than adding it globally?

Broadly, I think this is clearer, and shows that RCPC3 is more specific than LSE128 which is more specific than LSE2. I do have one concern shown in the tests.

Most recent diff was to clang-format the patch, which has removed some duplicate includes.

In D140999#4030131, @MaskRay wrote:

I don't think it is necessary to deprecate the old header then delete it after 16.0.0 is branched.
llvm/Support/AArch64TargetParser.h has very few open-source out-of-tree uses. Perhaps only ldc driver/targetmachine.cpp uses the header. So it is not worth extra expedience.
Just deleting it in another change should be fine.

Changing the include to #include "llvm/TargetParser/AArch64TargetParser.h" is totally fine, though.

How many other places do we have to manually model that disabling feature X disables all features that require feature X? It's a little infuriating to be honest.

I reported the same issue directly on the commit:

This commit causes an issue with -DBUILD_SHARED_LIBS - You need to add Demangle to the LINK_COMPONENTS in llvm/lib/IR/CMakeLists.txt

I'm not sure I agree with "map naturally", when CRm is a 4-bit field, and both bit 7 and bit 5 are different in the dsb <option> vs dsb <option>nXS instructions.

From your description this sounds right, and GCC matches the new behaviour as far as I can see: https://godbolt.org/z/f1M41dhvv

Thanks, I had been thinking this was the right way to go.

I think we have tests for implies now? I'm not sure what the equivalent API is.