Anastasia added inline comments.
================
Comment at: clang/lib/CodeGen/CGOpenCLRuntime.cpp:40
switch (cast<BuiltinType>(T)->getKind()) {
default:
----------------
I am not sure if this can be called a generic implementation as it has always
been a workaround to compensate for absence of representation in LLVM IR... we
should probably move such code into default definition of
`TargetInfo::getOpenCLType`...
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:10986
+ // for more details).
+ SmallVector<unsigned, 7> IntParams = {0, 0, 0, 0, 0, 0};
+
----------------
I think the list initialization doesn't do what you are trying to achieve with
`SmallVector` as it appends the elements. You should probably just be using
constructor with size i.e. IntParams(6)?
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:10989
+ // Choose the dimension of the image--this corresponds to the Dim parameter
+ // (https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_dim), or
+ // the instances of Dim in llvm/lib/Target/SPIRV/SPIRVSymbolicOperands.td.
----------------
Is this URL likely to stay the same? We normally just add a spec section number
e.g.
```
SPIR-V spec v1.6 rev2 s3.8.
```
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:10990
+ // (https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#_dim), or
+ // the instances of Dim in llvm/lib/Target/SPIRV/SPIRVSymbolicOperands.td.
+ if (OpenCLName.startswith("image2d"))
----------------
We should avoid adding file paths and names as they can change without updating
the comment.
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:10992
+ if (OpenCLName.startswith("image2d"))
+ IntParams[0] = 1; // 1D
+ else if (OpenCLName.startswith("image3d"))
----------------
Ok, is the order of parameters defined or documented somewhere? Would it make
sense to create some kind of a local enum map containing the indices and use
enum members instead of constants to improve readability/maintenance?
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:11015
+
+llvm::Type *CommonSPIRTargetCodeGenInfo::getOpenCLType(CodeGenModule &CGM,
+ const Type *Ty) const {
----------------
Ok, so the old SPIR format will change too!
I don't have any objections but want to make sure it's not accidental.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141008/new/
https://reviews.llvm.org/D141008
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
