stuart added inline comments.

================
Comment at: clang/test/CodeGenOpenCL/spir-debug-info-pointer-address-space.cl:22
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar5", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_GLOBAL]], 
isLocal: false, isDefinition: true)
+global int *global FileVar5;
+// CHECK-DAG: distinct !DIGlobalVariable(name: "FileVar6", scope: !{{[0-9]+}}, 
file: !{{[0-9]+}}, line: {{[0-9]+}}, type: ![[DWARF_ADDRESS_SPACE_CONSTANT]], 
isLocal: false, isDefinition: true)
----------------
Anastasia wrote:
> btw this variable is a duplicate of `FileVar0` for your change. In clang 
> parser:
> `global int *ptr;`
> is the same as 
> `global int *global ptr;`
> 
> 
> The same applies to local variables of `Type *` and `Type *private` as they 
> are equivalent on AST level too.
> 
> This is due to the address space inference rules if you are interested in 
> more details: 
> https://www.khronos.org/registry/OpenCL/specs/3.0-unified/html/OpenCL_C.html#addr-spaces-inference
> 
> Perhaps you could reduce the test case a bit by removing the duplicate 
> testing?
> 
> Also I would suggest separating with empty lines every variable declaration 
> with its corresponding CHECK line to improve the readability.
> 
> 
> ```
> CHECK: <...>
> Type var1;
> 
> CHECK: <...>
> Type var2;
> ```
> 
In case this review feeds into changes made for other test files, it may be 
worth noting that the test in question uses OpenCL C 2.0 (`-cl-std=CL2.0`), and 
therefore the generic address space as the default in many contexts, rather 
than `private`. (This comment is made not for direct benefit for this review 
itself, but for the benefit of anyone who may be reading who is not already 
especially familiar with OpenCL address spaces.)

The duplicated testing has now been removed from the newly added test, though.


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

https://reviews.llvm.org/D103097

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to