banach-space wrote:

> What's more, tests are not supposed to include non-local files anyway (aside 
> from header search path tests, obviously) and so what isystem resolves to is 
> usually unimportant to reproducing the test outside of very few circumstances.

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>`).

>  But in this case, is passing -flax-vector-conversions=none actually 
> necessary for every test? Is it uninteresting to every test? Is the fact that 
> it's a linux-gnu target always necessary or can there be other ARM64 neon 
> targets like win32?

These are very good points and most of the time I am puzzled myself. I've also 
noticed that many flags are outright redundant (e.g. 
`-flax-vector-conversions=none` that you mendioned) and that lowering these 
builtins rarely depends on the actual triple. My long-term aspiration is to 
update all test to use one good common denominator.

>  I think that's a call to action to reviewers and maintainers to please pay 
> closer attention to RUN lines.

Sadly, looks like there are no maintainers for this part of Clang? And, given 
the state of things, relying on folks to do the right thing doesn't seem to 
scale. Hence my preference to find an automated way to enforce better 
practices. I like your point re pre-commit, but we don't really have 
infrastructure to test such things, do we? And LIT substitutions can be 
introduced instantly.

@AaronBallman , I really appreciate all the great work that you have been doing 
in Clang and obviously I won't be merging this with your objections in place. 
In particular, I don't want to make maintaining Clang any harder for you. From 
my perspective, as a reviewer trying to bring some consistency here, this 
change would definitely help. But its not mission critical.

Thank you for the discussion!

https://github.com/llvm/llvm-project/pull/188547
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to