rjmccall added inline comments.
================
Comment at: lib/AST/DeclCXX.cpp:2187
ClassTy = C.getQualifiedType(ClassTy,
Qualifiers::fromCVRUMask(getTypeQualifiers()));
+
----------------
I feel like the right design now is for `getTypeQualifiers()` to return a
`Qualifiers` and not just a mask.
That will also help identify other places that need to be updated to support
address-space qualification.
================
Comment at: lib/CodeGen/CGCall.cpp:71
/// Derives the 'this' type for codegen purposes, i.e. ignoring method
/// qualification.
+static CanQualType GetThisType(ASTContext &Context, const CXXRecordDecl *RD,
const CXXMethodDecl *MD) {
----------------
Please update the comment to say "ignoring method CVR qualification" instead.
Also, maybe we should just eliminate this and use `CXXMethodDecl::getThisType`.
If we want to preserve the micro-optimization of sharing `CGFunctionInfo`s for
method types that only differ by CV qualification, maybe we should add a
parameter to `getThisType` to not add ABI-compatible qualification.
================
Comment at: lib/CodeGen/CGClass.cpp:2020
+ unsigned TargetAS = getContext().getTargetAddressSpace(AS);
+ llvm::Type *NewType = ThisPtr->getType()->getPointerTo(TargetAS);
+ ThisPtr = getTargetHooks().performAddrSpaceCast(
----------------
This is adding an extra level of pointer, I think.
================
Comment at: lib/CodeGen/CGClass.cpp:2023
+ *this, This.getPointer(),
+ getLangASFromTargetAS(This.getAddressSpace()), AS,
+ NewType);
----------------
...I'm not sure when `getLangASFromTargetAS` was added, but it shouldn't exist;
you need to pass down the language address space of `This`, which in general
will come from the pointer type. And then the `AS != LangAS::Default` check
should just be comparing the constructor's address space to that address space,
not specifically `LangAS::Default`.
Same comments apply to the code in `commonEmitCXXMemberOrOperatorCall`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54862/new/
https://reviews.llvm.org/D54862
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
