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?

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

Good to know! I think cleaning that up in test files is a good tech debt to pay 
down.

> > 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? 

For targets, we rely on the LLVM target component owners in Clang. So for ARM, 
that would be 
https://github.com/llvm/llvm-project/blob/main/llvm/Maintainers.md#arm-and-aarch64-backends

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

Someone would have to come up with that infrastructure, yes.

> @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!

I don't intend to block this if ARM folks find it to be helpful. 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). Basically, when is a substitution appropriate to add at all, what 
is the rule of thumb for which flags are "safe" to put there, etc.


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