[PATCH] D142702: [Clang][AArch64][SME] Generate target features from +(no)sme.* options

Thu Jul 20 13:36:37 PDT 2023

bryanpkc added inline comments.

================
Comment at: clang/test/Driver/aarch64-implied-sme-features.c:49
+// RUN: %clang -target aarch64-linux-gnu -march=armv8-a+nosme+sme-i16i64 %s -### 2>&1 | FileCheck %s --check-prefix=SME-SUBFEATURE-CONFLICT-REV
+// SME-SUBFEATURE-CONFLICT-REV: "-target-feature" "-sme-f64f64" "-target-feature" "+sme-i16i64" "-target-feature" "+sme" "-target-feature" "+bf16"
----------------
MaskRay wrote:
> michaelplatings wrote:
> > Hi Bryan, this test fails in our downstream toolchain because it has additional features that show up in between these features. Breaking the line up as shown would fix this. Another option could be to add `{{.*}}` between the features. Doing likewise for other checks in this file would make them less brittle as well.
> First: `-target ` has been deprecated since Clang 3.4, Use `--target=` for new tests.
> 
> Do you know why you have additional features? I think placing `"-target-feature"` on the same line is generally a good practice. It strengthens the test to ensure the feature set is compressive is not interleaved by other stuff.
@michaelplatings @MaskRay  I can submit a patch quickly to fix `-target`. I am curious what features show up between those ones in my test, because the options to toggle dependent features should be added immediately after one another. When I placed the `"-target-feature"` strings on the same line, I was following the example in `aarch64-implied-sve-features.c`.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142702/new/

https://reviews.llvm.org/D142702