Anastasia added inline comments.

================
Comment at: clang/lib/Basic/TargetInfo.cpp:405
+      const auto &OpenCLFeaturesMap = getSupportedOpenCLOpts();
+      Opts.OpenCLGenericAddressSpace = hasFeatureEnabled(
+          OpenCLFeaturesMap, "__opencl_c_generic_address_space");
----------------
svenvh wrote:
> This means we now have two separate places that set 
> `OpenCLGenericAddressSpace`, the other place being in 
> `CompilerInvocation::setLangDefaults()`.  That feels like a maintenance 
> hazard.  Do you think it makes sense to set this field in one single place 
> instead?
I think we should try to set it in `CompilerInvocation.cpp` directly, we should 
be able to query `TargetOptions` there. Although that place is expected to be 
for the language-specific defaults but we broke the standard flow by having the 
language mode controlled by the target settings anyway.

I can't remember though why we have decided to add dedicated `LangOpts` entries 
for generic address space instead of just using `OpenCLOptions` from `Sema`? I 
think it simplifies the handling of some builtin functions?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103401/new/

https://reviews.llvm.org/D103401

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to