[AArch64] add a target guard to FMV test #133
Conversation
The FMV test uses FMV feature names (e.g. flagm2) which are not available to -march and therefore arguably should not be available to the target attribute. This change adds a way to specify the string for the target attribute independently from the FMV feature. This is necessary for 2cf1439 (#94279) which had to be reverted because it caused this test to fail.
| asm volatile ( | ||
| "cfinv" "\n" | ||
| "cfinv" "\n" | ||
| ); | ||
| }) | ||
| CHECK(flagm2, { | ||
| CHECK(flagm2, arch=armv8.5-a, { |
There was a problem hiding this comment.
should this one's TARGET_GUARD be altnzcv?
There was a problem hiding this comment.
That is the internal SubtargetFeature/-target-feature name, and I don't think they should be exposed via the target attribute. llvm/llvm-project@2cf1439 was trying to remove the ability to use internal names. Unfortunately these FMV features lack a corresponding -march extension. There are some discussions about adding them, but until now the architecture is the only way for a user to enable them.
There was a problem hiding this comment.
The same applies to ccpp and ccdp.
There was a problem hiding this comment.
It looks fine in the medium term. IMHO ideally we should try to unify FMV features with target-attribute/cmdline features, but that has not yet been concluded in ARM-software/acle#315. I have tested this change locally after relanding the reverted compiler patch and works fine on my machine. FYI the alternative medium term fix in the compiler would be llvm/llvm-project#92319 but @tmatheson-arm suggested this fix is the right way.
My reverted attempt to decouple feature dependency expansion (see llvm#95056) made it evident that some features are still using the FMV dependencies in the target attribute. The original commit broke the llvm test suite. This was addressed here: llvm/llvm-test-suite#133. I am now relanding it.
…95231) My reverted attempt to decouple feature dependency expansion (see #95056) made it evident that some features are still using the FMV dependencies in the target attribute. The original commit broke the llvm test suite. This was addressed here: llvm/llvm-test-suite#133. I am now relanding it.
|
It looks like this broke buildbot https://lab.llvm.org/buildbot/#/builders/183/builds/22093 Shouldn't the |
Yes, fixed by 9144576 |
The FMV test uses FMV feature names (e.g. flagm2) which are not available to -march and therefore arguably should not be available to the target attribute. This change adds a way to specify the string for the target attribute independently from the FMV feature.
This is necessary for llvm/llvm-project@2cf1439 which had to be reverted because it caused this test to fail.