mantognini added reviewers: Anastasia, rjmccall.
mantognini marked 6 inline comments as done.
mantognini added inline comments.
================
Comment at: clang/lib/CodeGen/CGClass.cpp:2016
CGF.EmitCXXDestructorCall(dtor, Dtor_Complete, /*for vbase*/ false,
- /*Delegating=*/false, addr);
+ /*Delegating=*/false, addr, type);
}
----------------
This is the only place where the new parameter has an interesting value. In the
~10 other calls to `EmitCXXDestructorCall` overloads, the default value of this
new parameter is used instead.
================
Comment at: clang/lib/CodeGen/CGExprCXX.cpp:96
+ llvm::Value *ImplicitParam, QualType ImplicitParamTy, const CallExpr *CE,
+ QualType ThisTy) {
+ const CXXMethodDecl *DtorDecl = cast<CXXMethodDecl>(Dtor.getDecl());
----------------
This new parameter is required in order to call `performAddrSpaceCast`.
================
Comment at: clang/lib/CodeGen/CGExprCXX.cpp:117-118
+ llvm::Type *NewType = CGM.getTypes().ConvertType(DstTy);
+ This = getTargetHooks().performAddrSpaceCast(*this, This, SrcAS, DstAS,
+ NewType);
+ }
----------------
This is effectively the fix for the third point mentioned in the description.
Alternatively, `This = Builder.CreatePointerBitCastOrAddrSpaceCast(This,
NewType);` seems to work equally well and does not require the extra new
parameter.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8428-8439
+ bool DiagOccured = false;
FTI.MethodQualifiers->forEachQualifier(
[&](DeclSpec::TQ TypeQual, StringRef QualName, SourceLocation SL) {
+ // This diagnostic should be emitted on any qualifier except an addr
+ // space qualifier. However, forEachQualifier currently doesn't visit
+ // addr space qualifiers, so there's no way to write this condition
+ // right now; we just diagnose on everything.
----------------
This is the fix for the first point in the description.
================
Comment at: clang/lib/Sema/SemaOverload.cpp:5092-5097
- Qualifiers Quals;
+ Qualifiers Quals = Method->getMethodQualifiers();
if (isa<CXXDestructorDecl>(Method)) {
Quals.addConst();
Quals.addVolatile();
- } else {
- Quals = Method->getMethodQualifiers();
----------------
This is the fix for the second point in the description.
================
Comment at: clang/test/CodeGenOpenCLCXX/addrspace-with-class.cl:1
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -emit-llvm -O0
-o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -emit-llvm -O0
-o - | FileCheck %s --check-prefix=CHECK-DEFINITIONS
----------------
This test extends `addrspace-ctor.cl`, which is therefore deleted.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D64569/new/
https://reviews.llvm.org/D64569
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
