awarzynski added a comment.
In D96771#2571855 <https://reviews.llvm.org/D96771#2571855>, @Anastasia wrote:
> This is only the initial patch and for the moment the primary goal is to
> remove the need for the flag at least from the clang perspective.
Shorter compiler invocation is always nice! Does OpenCL (will) require a flag
to specify the C++ standard used in the kernel? Or is that going to be
controlled with `-cl-std={cl2.1|cl2.2|cl2.3`? This would be very weird
`-cl-std=clc++ -cl-std=cl2.1`, so I see why you would want to remove
`-cl-std=clc++`.
> I just dislike the fact that moving files in the repo complicates the commit
> history lookup so I was not sure if the renaming was good or bad thing. Do
> you have any suggestions?
Have you tried it? Git is very clever in this respect and will track the file
changes very accurately anyway. IMHO this wouldn't be disruptive in terms of
browsing the history at all. If you are making this change to remove the need
for `-cl-std=clc++` and to simplify things, you could as well organize tests
per kernel language (i.e. OpenCL C vs OpenCL C++). Also, `-cl-std=clc++` in
tests will look very confusing (i.e., why keep OpenCL C++ tests in OpenCL C
tests and force the standard with `-cl-std=clc++`)? Lastly, such change would
be a very through verification of your changes :)
> I was thinking to introduce as a guideline that the new tests should
> definitely use the new extension though.
+1
> I find the way driver is setup for the languages quite different so I was
> wondering if we have any guidelines?
I'm not aware of any documentation for the driver, sorry.
By scanning your patch I get the impression that you don't need `TY_CLCXX` just
yet. The language for the kernel is set in
`clang/lib/Frontend/CompilerInvocation.cpp`:
.Case("clcpp", Language::OpenCLCXX)
In Types.cpp this should be sufficient for now:
.Case("clcpp", TY_CLCXX)
Basically, you could limit your changes to the frontend driver and the changes
in the compiler driver keep to a tiny minimum. I think! :)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D96771/new/
https://reviews.llvm.org/D96771
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
