[PATCH] D43171: [AMDGPU] Change constant addr space to 4 for clang

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.


================
Comment at: test/CodeGenOpenCL/address-spaces.cl:37
+// SPIR: i32 addrspace(2)* %arg
+// GIZ: i32 addrspace(4)* %arg
 void f__c(__constant int *arg) {}
----------------
t-tye wrote:
> Suggest using the same name across all the OpenCL tests as some are using GIZ, some AMD and some AMDGCN. AMDGCN seems the clearer now that only have a single address space mapping? Similar comment for other places GIZ/AMD is being mentioned.
> 
> 
Will have a separate patch to cleanup the tests.


================
Comment at: test/CodeGenOpenCL/amdgpu-env-amdgiz.cl:4
 
-// CHECK: target datalayout = "e-p:64:64-p1:64:64-p2:64:64-p3:32:32-p4:32:32-p5:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5"
+// CHECK: target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-A5"
 void foo(void) {}
----------------
t-tye wrote:
> Should this test be renamed to amdgpu-env-amdgcn.cl now there is a single address space mapping (and delete any relating to the old mapping if they exist)?
will rename it when committing.


https://reviews.llvm.org/D43171