[PATCH] D32248: CodeGen: Cast alloca to expected address space

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 18 inline comments as done.
yaxunl added inline comments.


================
Comment at: lib/CodeGen/CGDecl.cpp:1115
+  if (AddrTy->getAddressSpace() != ExpectedAddrSpace &&
+      Ty.getAddressSpace() != LangAS::opencl_constant) {
+    address = Address(Builder.CreateAddrSpaceCast(Addr,
----------------
t-tye wrote:
> Anastasia wrote:
> > Do you need to check this? I thought your change doesn't affect OpenCL because alloca AS == private AS in OpenCL  and constant AS local objects are treated as private const objects anyways.
> Given patch D32977 that makes function local variables marked with the constant address space treated as static, will they ever be processed by this function now? If so then the extra test for opencl_constant would not be needed.
Will remove this.


================
Comment at: test/CodeGenCXX/amdgcn-automatic-variable.cpp:25
+  // CHECK: store i32 2, i32* %[[r1]]
+  int lv1;
+  lv1 = 1;
----------------
Anastasia wrote:
> I am wondering if all these different test cases are really needed. Are we testing any different program paths from the patch?
> 
> Also would it make sense to add a test case with an object in an AS different to 0 (i.e. with `__attribute__((address_space(n)))`)
I think at least I should cover the typical use cases of auto var.

I will add a test for __attribute__((address_space(n))))


https://reviews.llvm.org/D32248