Anastasia added inline comments.
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2260
- return CGF.CGM.getNullPointer(cast<llvm::PointerType>(ConvertType(DestTy)),
- DestTy);
+ // The type may be a target extension type instead of a pointer type
+ // (e.g., OpenCL types mapped for SPIR-V). In the former case, emit a
----------------
Ok, yet this looks strange to me... do you have an example that hits this code?
At some point we added `CK_ZeroToOCLOpaqueType` so I wonder if we should be
using this instead of `CK_NullToPointer` here i.e. ideally clang should not
assume too early how type are mapped into target specific representation.
================
Comment at: clang/lib/CodeGen/CGOpenCLRuntime.cpp:200
+
+ // Choose the dimension of the image--this corresponds to the Dim parameter,
+ // so (e.g.) a 2D image has value 1, not 2.
----------------
Any reason for this? Can we create a `constexpr` map or enum type that would
contain those numbers instead of using hard coded ones scattered around?
================
Comment at: clang/lib/CodeGen/CGOpenCLRuntime.cpp:226
+
+llvm::Type *CGSpirVOpenCLRuntime::convertOpenCLSpecificType(const Type *T) {
+ assert(T->isOpenCLSpecificType() && "Not an OpenCL specific type!");
----------------
This looks good in general but we keep CodeGen hierarchy target agnostic so the
way to add specifics of an individual target is to extend `TargetCodeGenInfo`
adding a new target hook member function for example it could be
`getOpenCLSpecificType` or something like that. Then you can add SPIR-V
specific logic into either `CommonSPIRTargetCodeGenInfo` or
`SPIRVTargetCodeGenInfo` subclass. I would recommend to look at
`createEnqueuedBlockKernel` for inspiration
https://clang.llvm.org/doxygen/CodeGen_2TargetInfo_8cpp_source.html#l12407.
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:239
+ // extension types.
+ if (getTriple().isSPIRV() || getTriple().isSPIR())
+ OpenCLRuntime.reset(new CGSpirVOpenCLRuntime(*this));
----------------
Do we want to change old SPIR representation or keep it as is? It seems that
SPIR spec defined them as LLVM's opaque pointer types... but I appreciate that
for maintenance purposes it's easier to keep those in sync.
================
Comment at: llvm/docs/SPIRVUsage.rst:103
+
+All integer arguments take the same value as they do in the SPIR-V type name.
+For example, the OpenCL type ``image2d_depth_ro_t`` would be represented in
----------------
svenvh wrote:
>
Any reference to the document/section explaining these instructions would be
useful here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141008/new/
https://reviews.llvm.org/D141008
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
