azabaznov added a comment. In D96524#2692376 <https://reviews.llvm.org/D96524#2692376>, @Anastasia wrote:
> In D96524#2691428 <https://reviews.llvm.org/D96524#2691428>, @azabaznov wrote: > >> My main idea was to provide an interface which will not make users to >> specify `-cl-ext=+__opencl_c_fp64,+cl_khr_fp64`/ >> `-cl-ext=-__opencl_c_fp64,-cl_khr_fp64` if they need to enable/disable >> functionality in OpenCL C 3.0 because I believe that is not a right thing to >> do: why anyone should care about those extensions if there are features >> already? Also, this may lead to confusions since, for example, >> `__opencl_c_subgroups` and `cl_khr_subgroups `are not the same (subgroup >> independent forward progress is required in extension while it is optional >> in OpenCL C 3.0, thus implementation may support the extension but not the >> feature). >> >> To be clear: I'm OK with providing a validation of correct option settings >> (`__opencl_c_fp64/cl_khr_fp64` and >> `__opencl_c_3d_image_writes/cl_khr_3d_image_writes` should both be set to >> the same value). Also, it makes sense to unify a check within header and >> clang to the only macro, I'm OK with that too. But I would prefer to keep >> the option interface without redundant mentioning of extension. > > Ok, let's implement both - we can add an early check of consistency of > options and therefore will only need to use one for checking during the > parsing and let's simplify the interface of `-cl-ext`. Since it is a > frontend option and new functionality doesn't cause backward compatibility > issues i.e. only affects OpenCL 3.0 onwards, so I see no risk. Could you > please update the help documentation of the option explaining the new > behavior? Then let's concentrate the whole logic for setting the options and > keeping the consistency between the equivalent ones in > `TargetInfo::setCommandLineOpenCLOpts` directly? This will provide cleaner > flow due to encapsulation and will make it clear where the use case comes > from. Does it make sense? Sure, agree. FYI I'm looking to a proper way of validating a target already. There already exists `TargetInfo::validateTarget`. I wanted to reuse that (just don't want to introduce yet another interface for OpenCL in Target), but I'm again bumping into some problems because of our //special// OpenCL options settings: a target should be immutable once created; and language options are not used to create a target. I'm just wondering if there is some more refactoring is needed... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96524/new/ https://reviews.llvm.org/D96524 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
