[PATCH] D64569: [OpenCL] Improve destructor support in C++ for OpenCL

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.


================
Comment at: clang/lib/CodeGen/CGExprCXX.cpp:117-118
+      llvm::Type *NewType = CGM.getTypes().ConvertType(DstTy);
+      This = getTargetHooks().performAddrSpaceCast(*this, This, SrcAS, DstAS,
+                                                   NewType);
+    }
----------------
mantognini wrote:
> 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.
Yes, I agree just using `CreatePointerBitCastOrAddrSpaceCast` would be easier.

As far as I remember (@rjmccall can correct me if I am wrong) `performAddrSpaceCast` was added to workaround the fact that `nullptr` constant might not be value 0 for some address spaces. However, considering that it's not the first place where some big change has to be done in CodeGen to be able to use the new API I am wondering if it's worth looking at moving this logic to LLVM lowering phases that can map `nullptr` constant into some arbitrary value. I think the challenge is to find where LLVM assumes that `nullptr` is always 0. That might not be an easy task.

PS, I am not suggesting to do it for this patch but just an idea to investigate in the future. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64569