to268 wrote:

I have a broader concern outside of the highlighted changes.
I do not know the Frontend and the Driver area well, so this would need to be 
confirmed by someone who knows better before implementing what I will be 
suggesting below.

I think the check and the diagnostic should be moved from the Frontend part to 
the Driver part because we have already a [similar diagnostic for 
HLSL](https://github.com/llvm/llvm-project/blob/0c0ae3786ef4ec04ba0dc9cdd565b68ec486498a/clang/include/clang/Basic/DiagnosticDriverKinds.td#L867)
 in 
[CompilerInvocation.cpp](https://github.com/llvm/llvm-project/blob/0c0ae3786ef4ec04ba0dc9cdd565b68ec486498a/clang/lib/Frontend/CompilerInvocation.cpp#L4626).
 Moreover, at this stage, we already have a [triple 
object](https://github.com/llvm/llvm-project/blob/0c0ae3786ef4ec04ba0dc9cdd565b68ec486498a/clang/lib/Frontend/CompilerInvocation.cpp#L5073)
 constructed for checks based on the triple. 

The test case would also look like the [empty Vulkan environment test for 
HLSL](https://github.com/llvm/llvm-project/blob/0c0ae3786ef4ec04ba0dc9cdd565b68ec486498a/clang/test/Driver/hlsl-lang-targets-spirv.hlsl#L15)

The only reason to place the check in 
[CompilerInstance.cpp](https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/CompilerInstance.cpp)
 would be to diagnose it earlier.

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

Reply via email to