aaron.ballman added a comment.
Just a few minor nits from me, but I'm mostly wondering: where are we at with
this and are there still substantive changes required? (I looked through the
comments, but there's a lot of back-and-forth since Oct and I'm not certain
what's holding the patch back currently.)
================
Comment at: clang/include/clang/AST/Type.h:492
+ (isPtrSizeAddressSpace(B) || B == LangAS::Default)) ||
+ // Default is a superset of SYCL address spaces
+ (A == LangAS::Default &&
----------------
================
Comment at: clang/lib/AST/ItaniumMangle.cpp:2379
unsigned TargetAS = Context.getASTContext().getTargetAddressSpace(AS);
- if (TargetAS != 0)
+ if (TargetAS != 0 ||
(Context.getASTContext().getLangOpts().SYCLIsDevice))
ASString = "AS" + llvm::utostr(TargetAS);
----------------
================
Comment at: clang/lib/Basic/Targets/SPIR.h:71-75
+ if (Triple.getEnvironment() == llvm::Triple::SYCLDevice) {
+ AddrSpaceMap = &SYCLAddrSpaceMap;
+ } else {
+ AddrSpaceMap = &SPIRAddrSpaceMap;
+ }
----------------
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:9997
+ if (auto ConstAS = CGM.getTarget().getConstantAddressSpace())
+ return ConstAS.getValue();
+ }
----------------
I called it out because it wasn't clear to me what the type actually was, but I
see now that it's an `Optional<LangAS>`. I think it's useful to know that we're
dealing with an optional, but it's not crucial.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D89909/new/
https://reviews.llvm.org/D89909
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
