[PATCH] D33989: [OpenCL] Allow targets to select address space per type

[David Blaikie via cfe-commits]

Ping on this layering violation. A simple way to demonstrate this is to
move the definition of clang::Type::getTypeClass out of line: This results
in an unresolved symbol due to incorrect/broken dependencies.

Richard? Anyone else? Ideas on how to address this layering violation?

Anastasia: Could you explain more what you mean by "A large logical
dependency" - which bits of code expose/demonstrate that dependency? (I
know very little about this code, I came across the layering violation
while trying to build more of Clang & LLVM with modular code generation)

On Thu, Oct 12, 2017 at 6:51 AM Anastasia Stulova via Phabricator via
cfe-commits <cfe-commits at lists.llvm.org> wrote:

> Anastasia 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);
> ----------------
> 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?
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D33989
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20171116/0407b68b/attachment.html>