[CIR] Implement virtual destructor call #1856
Conversation
ahmedshakill
requested review from
lanza,
bcardosolopes,
xlauko and
andykaylor
as code owners
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
clang/test/CIR/CodeGen/virtual-destructor-explicit-unqualified-call.cpp
Outdated
Show resolved
Hide resolved
clang/test/CIR/CodeGen/virtual-destructor-explicit-unqualified-call.cpp
Outdated
Show resolved
Hide resolved
clang/test/CIR/CodeGen/virtual-destructor-explicit-unqualified-call.cpp
Outdated
Show resolved
Hide resolved
clang/test/CIR/CodeGen/virtual-destructor-explicit-unqualified-call.cpp
Outdated
Show resolved
Hide resolved
|
|
||
| QualType thisTy = | ||
| IsArrow ? Base->getType()->getPointeeType() : Base->getType(); | ||
| emitCXXDestructorCall(globalDecl, callee, This.getPointer(), thisTy, |
There was a problem hiding this comment.
I see some code repetition from the else part, that can be improved overall
There was a problem hiding this comment.
Classic codegen, calls CGM.getCXXABI().EmitVirtualDestructorCall() here. Is there a reason we can't do that?
There was a problem hiding this comment.
CGM.getCXXABI().EmitVirtualDestructorCall()
Thanks for the suggestion. We can use this call instead. Got the same result.
|
Qualified explicit destructor call has different behavior compared to Non-Qualified destructor call. 
Qualified explicit destructor call is already handled in the CIR Codegen. Only Unqualified call needs to be handled. 
|
Sounds good, is there any issue here or did you just want to clarify? |
|
@tommymcm Just wanted to clarify. |
| @@ -19,5 +24,27 @@ A A::B(A) { | |||
| // CIR-NEXT: cir.trap | |||
| // CIR-NEXT: } | |||
|
|
|||
|
|
|||
| // LLVM-LABEL: define dso_local %class.A @_ZN1A1BES_( | |||
There was a problem hiding this comment.
This test has some loose ends.
Could you add the this argument binding and store, that way we can tie everything together. Something like
// LLVM-LABEL: ...
// LLVM-SAME: %[[THIS_ARG:.*]]
// ...
// LLVM: %[[THIS_VAR:.*]] = alloca ptr
// ...
// LLVM: store ptr %[[THIS]], ptr %[[THIS_VAR]]
// ...
// LLVM: %[[THIS:.*]] = load ptr, ptr %[[THIS_VAR]]
| // LLVM: %[[VAL_0:.*]] = alloca ptr, i64 1, align 8 | ||
| // LLVM: %[[VAL_6:.*]] = load ptr, ptr %[[VAL_0]], align 8 | ||
| // LLVM-NEXT: %[[VAL_7:.*]] = load ptr, ptr %[[VAL_6]], align 8 | ||
| // LLVM-NEXT: %[[VAL_8:.*]] = getelementptr inbounds ptr, ptr %[[VAL_7]], i32 0 |
There was a problem hiding this comment.
Name these variables, for example:
| // LLVM-NEXT: %[[VAL_8:.*]] = getelementptr inbounds ptr, ptr %[[VAL_7]], i32 0 | |
| // LLVM-NEXT: %[[VIRT_DTOR_ADDR:.*]] = getelementptr inbounds ptr, ptr %[[THIS]], i32 0 |
There was a problem hiding this comment.
Sure, I'll look into them.
Implements #1812