tyb0807 marked 2 inline comments as done. tyb0807 added inline comments.
================ Comment at: clang/test/Driver/aarch64-archs.c:296 -// NO-LS64-NOT: "-target-feature" "+ls64" -// LS64: "-target-feature" "+ls64" - ---------------- tmatheson wrote: > Looks like these were duplicate tests? Besides `armv8.7a` vs `armv8.7-a` Hmm, the first one tests that LS64 is not on by default on `armv8.7a`, and the second tests that `armv8.7a+ls64` enables it. Why these are duplicates? If you mean L276/277, I guess the intent is to test both syntaxes that we support ================ Comment at: clang/test/Driver/aarch64-ras.c:14 +// RAS is on by default for v8.2a, but can be disabled by +noras +// FIXME: in the current implementation, RAS is not on by default at all for v8.2a (the test says it all...) +// RUN: %clang -target aarch64 -march=armv8.2a -### -c %s 2>&1 | FileCheck -check-prefix=V82ARAS %s ---------------- tmatheson wrote: > The RAS test changes are the only functional change to this review, which is > otherwise moving lines around and changing comments. I would keep the > original tests even if they're wrong, so that the history remains clearer. > Alternatively describe the RAS changes in the commit message and description. Actually, I added the FIXME because I don't want to fix the test in this commit. There's no functional change to this review I think Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121093/new/ https://reviews.llvm.org/D121093 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
