[PATCH] D53764: [OpenCL] Enable address spaces for references in C++

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: cfe/trunk/lib/CodeGen/CGExpr.cpp:4268
+        DestTy.getAddressSpace(), ConvertType(DestTy));
+    return MakeNaturalAlignPointeeAddrLValue(V, DestTy);
+  }
----------------
romanovvlad wrote:
> Hi,
> 
> It seems this code doesn't work correctly(repro at the end). TBAA information is lost here because MakeNaturalAlignPointeeAddrLValue constructs LValue with alignment of poinee type but TBAA info is taken from pointer itself what is strange enough. As a result, for example, memcpy with wrong size is generated for copy constructors. 
> 
> Repro:
> 
> ```
> class P {
> public:
>   P(const P &Rhs) = default;
> 
>   long a;
>   long b;
> };
> 
> __kernel void foo(__global P* GPtr) {
>   P Val = GPtr[0];
> }
> ```
> 
> As a solution the line could be replaced with the following:
> ```
> return MakeAddrLValue(Address(V, LV.getAddress().getAlignment()),
>                                        E->getType(), LV.getBaseInfo(),
>                                        CGM.getTBAAInfoForSubobject(LV, E->getType()));
> ```
> To take all the information from the original pointer.
> 
> What do you think about solution?
> 
Oh, yes, this should absolutely not be using `MakeNaturalAlignPointerAddrLValue`; it should be preserving all of the extra information from the original l-value, as you say.

I think TBAA information is independent of address-space qualification and can just be taken from the original LV directly instead of using `getTBAAInfoForSubobject`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D53764