rsmith added inline comments.
================
Comment at: cfe/trunk/lib/Basic/TargetInfo.cpp:351
+LangAS::ID TargetInfo::getOpenCLTypeAddrSpace(const Type *T) const {
+ auto BT = dyn_cast<BuiltinType>(T);
----------------
Anastasia wrote:
> chapuni wrote:
> > Excuse me for old commit, I think it might be layering violation.
> > Could we avoid using AST/Type here?
> One of the problems is that even if we could write another layer of enums to
> map OpenCL specific AST types into the TargetInfo representation it doesn't
> really help creating a layered structure between AST and Basic libs. They
> seem to have a large logical dependence despite not including the library
> headers physically. So modification in AST would require modifications in
> Basic anyway resulting in overhead of changing 2 places instead of 1. So I am
> wondering if there is any better approach here e.g. revisiting the library
> dependencies or what classes they should contain?
I don't know what large logical dependencies you're talking about here. `Basic`
is supposed to be answering questions about lowering that are independent of
our choice of AST model. It should never look at a `clang::Type` for any reason
to do this, and should never need to do so. If you find it does need to do so,
you're asking the `TargetInfo` questions at the wrong level of abstraction.
As far as I can see, there's only one question you actually need to ask the
`TargetInfo` here, and that's whether builtin types are in the `opencl_global`
or `opencl_constant` address space.
Repository:
rL LLVM
https://reviews.llvm.org/D33989
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
