[PATCH] D124382: [Clang] Recognize target address space in superset calculation

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.


================
Comment at: clang/include/clang/AST/Type.h:508
+              static_cast<unsigned>(LangAS::FirstTargetAddressSpace));
+        // When dealing with target AS return true if:
+        // * A is equal to B, or
----------------
jchlanda wrote:
> tra wrote:
> > Is the code above intended to ensure that both A and B are target AS at this point?
> > If so, then it could be simplified to something like this:
> > ```
> > if (ASMap) {
> >   if (!isTargetAddressSpace(A))
> >     A = getLangASFromTargetAS((*ASMap)[static_cast<unsigned>(A)]);
> >   if (!isTargetAddressSpace(B))
> >     B = getLangASFromTargetAS((*ASMap)[static_cast<unsigned>(B)]);
> > 
> >   Generic = getLangASFromTargetAS((*ASMap)[static_cast<unsigned>(LangAS::opencl_generic)])
> >   Constant = getLangASFromTargetAS((*ASMap)[static_cast<unsigned>(LangAS::opencl_constant)]);
> > 
> >   // ... proceed inferring whether A is superset of B in target AS.
> >   return;
> > }
> > assert (isTargetAddressSpace(A) && isTargetAddressSpace(B));
> > ```
> Yes at the end of AS map accesses all address spaces have to be expressed in therms of HW values, but I want it to happen only in the case of mixed AS (Language and HW). I will add assert and use helpers, like you suggested in the snippet, but would like to keep the `^` condition.
> would like to keep the ^ condition.

OK. Adding a comment explaining what's going on would be helpful here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124382