[PATCH] D111566: [SYCL] Fix function pointer address space

[Elizabeth Andrews via Phabricator via cfe-commits]

eandrews marked 2 inline comments as done.
eandrews added inline comments.


================
Comment at: clang/lib/AST/ASTContext.cpp:11496
+                   .getProgramAddressSpace()
+             : getTargetAddressSpace(T.getQualifiers());
+}
----------------
rjmccall wrote:
> Oh, I'm sorry I missed this.  Parsing the data layout string into an `llvm::DataLayout` is definitely not an okay thing to be doing here.  The IRGen version of this had a cached `DataLayout` object which it queried, which was okay, but this function is used too tightly to be doing that much redundant work.
> 
> We could just cache a `DataLayout` in the `clang::TargetInfo`, but I think we've been trying to avoid that as a layering violation.  Instead, `TargetInfo` should just have a `getProgramAddressSpace` method or something like that, and the targets with non-default address spaces for code should set that manually.
> Instead, `TargetInfo` should just have a `getProgramAddressSpace` method or something like that, and the targets with non-default address spaces for code should set that manually.

Ok. Thanks for review! I'll make this change



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

https://reviews.llvm.org/D111566