The way I understand a bitcast instruction in SPIR-V (OpBitcast in https://www.khronos.org/registry/SPIR-V/specs/unified1/SPIRV.html#_conversion_instructions) is that it can only apply to pointer types which are distinct from function types. Note that I believe that function pointers are illegal, at least we disallow them in OpenCL.

In D127579#3585516, @beanz wrote:

@nikic the most important thing you need to know about SPIR-V is that it is a virtual ISA based on LLVM IR. The ISA itself encodes types for pointers just like LLVM IR would.

LGTM. Thanks!

LGTM, just one suggestion.
It would be great to get @aaron.ballman approve too.

In D115708#3192495, @tstellar wrote:

LGTM. I'm not sure the best place to document this.

In D110622#3174113, @tra wrote:

The patch looks OK for the time being. That said, I do have concerns that we may be organically growing something that will be troublesome to deal with long-term.

TBH, I still can't quite make sense of where/how SPIR-V fits in the offloading nomenclature.

Right now we have multiple levels of offloading-related control points.

• offload targets, specified by --offload-arch. Determines the ISA of the GPU binary we produce.
• offload mechanism: OpenMP, CUDA runtime, HSA. Determines how we compile/pack/launch the GPU binaries.
• front-end: CUDA/HIP/ C/C++ w/ OpenMP.
• Driver: Determines compilation pipeline to glue everything together,

SPIR-V in these patches appears to be wearing multiple hats.
It changes compilation pipeline, it changes offload mechanism and it changes offload targets.

In D109818#3169531, @linjamaki wrote:

The patch is ready to land. @Anastasia, @bader, could you commit this patch to the LLVM for us? Thanks.

LGTM, with a couple of minor suggestions.

@tra, ping.
@steffenlarsen, does it make sense to add support for recently released 11.5 as well?

This part looks good to me. Just a couple of minor style comments.

@vlastik, your commit fixes function pointers on AVR - https://github.com/llvm/llvm-project/commit/57fd86de879cf2b4c7001b6d0a09df60877ce24d. I suppose this change is required for fixing lvalue references to function pointers on AVR as well. Right?

In D71016#3063762, @tschuett wrote:

Would a codegenSYCL directory help you to separate Sema from code generation?

In D71016#3063457, @tschuett wrote:

It feels like you are doing codegen(OpenCL kernel) in Sema. Are OpenCL kernels the only approach.

In D109144#3042247, @Anastasia wrote:

1. Implementing SPIR-V target as SPIR target. @bader do you suggest that we add spirv triple to clang and map it into SPIR taget or do you suggest something different?

In D109144#3032865, @Anastasia wrote:

It would be good to get closure on this asap.

@bader We had related discussions on the other reviews about the approach in this patch. If you have any concerns/suggestions can you please notify asap...

LGTM, just one typo in addition to linter reports and I'd like John to take a look.

LGTM.

LGTM, although it would be good if some else take a look as well.

Closed by https://reviews.llvm.org/rG18e165f50d8c1ab3afe7098dc00557d5f1a43cfa.

By commit https://reviews.llvm.org/rGc2006f857d80f54b90ed7d911d3e7acf4f46001b.

Sorry for the delay.
LGTM. Thanks!

Added documentation to cover conversion rules and rebased on ToT.

Added explicit cast checks to Sema tests.

Apply code review suggestions and rebase on ToT.

Committed alternative version - https://reviews.llvm.org/D89909.

Thanks! I've uploaded this version to https://reviews.llvm.org/D101549.

In D99488#2725435, @Anastasia wrote:

If I build docs now I get the following output:

llvm-project/build-doc/tools/clang/docs/SYCLSupport.rst:102: WARNING: Error in "code-block" directive:
1 argument(s) required, 0 supplied.

.. code-block::

   TODO: add support for `__attribute__((opencl_global_host))` and
   `__attribute__((opencl_global_device))`.

Is this something already being looked at?

Rebased on ToT to resolve merge conflicts.

Rebase on ToT

Rebased patch to unblock commit.

@Anastasia, I've updated https://reviews.llvm.org/D99488 and refactored getStringLiteralAddressSpace to handle non-string constants as well. Please, take a look.

Incorporate https://reviews.llvm.org/D89909 review feedback.

Generalize getStringLiteralAddressSpace to GetGlobalConstantAddressSpace

Added SYCL address spaces mangling for targets without address space map

Applied more review comments.

Applied more comments.

Applied more code review suggestions.

Applied code review suggestions.

In D99488#2671906, @Anastasia wrote:

LGTM! Thanks for working on this. The expected sematic seems fairly clear now.

Add ReST marks to hyperlinks.

Address comments from @Anastasia.