Anastasia added a comment.
Generally LGTM!
I only have one suggestion regarding testing - I feel that it makes more sense
to just omit the tests that are not specific to the target itself. This is
mainly because SPIR-V is now inherited from SPIR so the same logic applies in
most of places. I think it would make sense that we only add tests for the
logic which is not identical to SPIR. For example, tests that verify the target
properties and target specific macros, etc make sense to keep but most of the
tests that are just checking the language semantic using SPIR triple are not
going to be different for SPIR-V (at least not now). So we can slowly build up
relevant testing in the subsequent patches and therefore avoid unnessasary test
time increase.
================
Comment at: clang/lib/Basic/Targets.cpp:609
}
+ case llvm::Triple::spirv32: {
+ if (os != llvm::Triple::UnknownOS ||
----------------
I wonder how complete is the support of logical addressing SPIR-V triple? It
seems like you don't test it in the clang invocation at the moment and it is
therefore missing from `TargetInfo`.
Do you have plans to implement it in the subsequent patches? If not it might be
better to leave it out for now.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D109144/new/
https://reviews.llvm.org/D109144
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
