AaronBallman wrote: > > > Just to clarify this particular case, `-internal-isystem <path>` is > > > crucial for AArch64 code-gen tests. Indeed, they all depend on Arm > > > headers (e.g. `#include <arm_neon.h>`). > > > > > > Yes, but don't those headers always live in the same place in tree > > (`clang/lib/Headers`) and so "where do I find the contents of the header > > being included" is almost never in question? > > Yes. > > > The discussion is mostly so I can understand what situations we _want_ > > substitutions for because I don't think we've ever discussed it as a > > community (at least on the Clang side). > > Sure, we want canonical substitutions (i.e. set of flags) that can be used > code-gen tests for builtins. > > For now I've just extracted the common denominator from a few tests, but I > guess it's a good time to discuss what fits and what does not. I want to > focus on the bare minimum that is required for lowering the builtins: > > * `-triple arm64-none-linux-gnu` (this should be sufficient for most > cases that I will focus on, i.e. AArch64) (*).
So there is no Windows-specific testing needed? > * `-disable-O0-optnone` (many tests pipe things to `opt`) (**). Why are Clang tests piping things to opt in the first place? > * Neon, SVE and SME are the major ISA extensions, hence: > > * `-target-feature +neon` for `%clang_cc1_arm64_neon`, > * `-target-feature +sve` for `%clang_cc1_arm64_sve`, and > * `-target-feature +sme` for `%clang_cc1_arm64_sme` . Oof, so a substitution per ISA extension. > * Flags for rigorous semantics checks, > e.g.`-flax-vector-conversions=none`alongside `-Werror -Wall` (suggestions are > welcome). I'm not sold on this... `-Wall` enables a *lot* of diagnostics, including ones that are likely to be unrelated to what the test file is after. `-Werror` in a test file is definitely odd as well -- why do we need the diagnostics to be *errors*, is there something about that code path which could impact test behavior? In general, I think that the more we put behind a substitution, the harder it is for that substitution to apply to every test scenario. I can sort of see benefit to a substitution which handles the triple and the target feature because those are verbose command line options to repeat and the substitution name captures the details nicely. But I'm less comfortable with the other suggestions because those aren't really implied by the name of the substitution but those details matter for the test or don't apply consistently to all test scenarios. > In the end, the following `RUN` lines: > > ```c > // RUN: %clang_cc1 -triple arm64-none-linux-gnu -disable-O0-optnone > -flax-vector-conversions=none -target-feature +neon <other flags> | FileCheck > // RUN: %clang_cc1 -triple arm64-none-linux-gnu -disable-O0-optnone > -flax-vector-conversions=none -target-feature +sve <other flags> | FileCheck > // RUN: %clang_cc1 -triple arm64-none-linux-gnu -disable-O0-optnone > -flax-vector-conversions=none -target-feature +neon -target-feature +fullfp16 > <other flags> | FileCheck > ``` > > would be replaced with: > > ```c > // RUN: %clang_cc1_arm64_neon <other flags> | > FileCheck > // RUN: %clang_cc1_arm64_sve <other flags> | > FileCheck > // RUN: %clang_cc1_arm64_neon -target-feature +fullfp16 <other flags> | > FileCheck > ``` > > One other downside of all of this is that, due the number of test files, I > will have do it incrementally. > > -Andrzej > > P.S. I am away next week, so my next reply will be delayed. > > (*) I have no plans to work on older architectures like `ArmV7` that require > other triple - that's a minority though). Also, I actually expect `-triple > aarch64` to be sufficient. > (**) We might just switch to `-O1` at some later point. Clang tests usually aren't expected to be sensitive to opt levels, so I'm not certain if switching to `-O1` makes sense or not. CC @efriedma-quic for CodeGen opinions on that. (Also, if these are purely for codegen tests, perhaps we want to add `-emit-llvm -o -` to the substitution and put "codegen" or "cg" in the name?) https://github.com/llvm/llvm-project/pull/188547 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
