[cfe-dev] Fwd: making the SPIR target work

[Ribulous Barnulous]

Hi,

I've been looking into making the spir target work with the latest version
of clang.

Comparing the current version to this spir branch of clang
https://github.com/KhronosGroup/SPIR
I found these pieces missing that are necessary to generate confirming spir
code:

1) the "-cl-kernel-arg-info" option currently suppresses the generation of
all kernel metadata when it should only omit the "kernel_arg_name" name node

2) The "spir_kernel" calling convention needs to be set on kernel functions

3) Additional spir metadata needs to be added to the generated module. Done
by this code:
https://github.com/KhronosGroup/SPIR/blob/spir_12/lib/CodeGen/CGSPIRMetadataAdder.cpp

4) An additional "cl-spir-compile-options" option is added to clang to
allow options to be passed through in the SPIR metadata

Do anyone see any obstacles to checking in these pieces? or know why they
haven't been committed already?

Thanks


----------
From: *Pekka Jääskeläinen* <pekka.jaaskelainen at tut.fi>
Date: Tue, Nov 18, 2014 at 5:18 AM
To: Ribulous Barnulous <rnickb731 at gmail.com>, cfe-dev at cs.uiuc.edu


Hi,

On 11/17/2014 03:48 AM, Ribulous Barnulous wrote:

> Do anyone see any obstacles to checking in these pieces? or know why they
> haven't been committed already?
>

I think the main problem is the fact that the SPIR format is fixed to
the IR produced by certain LLVM versions. So, if later LLVM versions
have changes in the IR output, it might not produce valid SPIR
binaries anymore.

SPIR 1.2 is fixed to LLVM 3.2 IR and SPIR 2.0 to LLVM 3.4.

However, if there are other non-SPIR-specific features in that branch,
I hope Someone breaks them up in smaller patches so they get reviewed and
upstreamed.

-- 
Pekka

----------
From: *Ribulous Barnulous* <rnickb731 at gmail.com>
Date: Thu, Nov 20, 2014 at 1:01 AM
To: Pekka Jääskeläinen <pekka.jaaskelainen at tut.fi>


Hey Pekka,

LLVM already has some of the SPIR code checked in. Running

clang -x cl -fno-builtin -target spir -c -emit-llvm <cl-file>

comes close to generated correct SPIR code. If SPIR code's going to be
included at all, shouldn't the other missing pieces be checked in?

Even if the official version of SPIR is pegged to a particular version of
LLVM, I think adding the functionality to make the SPIR annotations and
calling conventions in the IR code would still be very useful.
Implementations and future versions of SPIR will track the LLVM IR
versions; so if changes were introduced that made the IR incompatible with
the latest version of SPIR, it's likely that many implementations would
support it anyways and it would surely be adopted into the next iteration
of the SPIR standard to eventually be officially supported.

Under this view, if you wanted to generate SPIR, you'd tell clang the SPIR
and OCL version; it would add SPIR annotations and set calling conventions
but wouldn't guarantee that the IR was compatible with the SPIR standard.

This would make a lot more sense to me than having a completely separate
fork that only makes minor modifications to the IR for the SPIR target, and
has to be continually synced up.

Thanks

----------
From: *Pekka Jääskeläinen* <pekka.jaaskelainen at tut.fi>
Date: Thu, Nov 20, 2014 at 9:52 AM
To: Ribulous Barnulous <rnickb731 at gmail.com>


Hi,

On 11/20/2014 08:01 AM, Ribulous Barnulous wrote:

> This would make a lot more sense to me than having a completely separate
> fork that only makes minor modifications to the IR for the SPIR target,
> and has to be continually synced up.
>

Right. In case the LLVM IR of the later versions is
a superset of the previous versions, one can even try and add
a switch that (tries to) restrict the output to the subset so
it's SPIR compatible.

So, if someone breaks up the patches and upstreams them, I can
try to help at least by reviewing them (with my limited Clang
knowledge).

-- 
--Pekka Jääskeläinen


----------
From: *Ribulous Barnulous* <rnickb731 at gmail.com>
Date: Fri, Nov 21, 2014 at 1:02 AM
To: Pekka Jääskeläinen <pekka.jaaskelainen at tut.fi>


Thanks Pekka.

 I attached two small patches to these issue reports:

http://llvm.org/bugs/show_bug.cgi?id=21554 -- patch to set the SPIR calling
convention

http://llvm.org/bugs/show_bug.cgi?id=21555 -- patch to fix behavior of
"-cl-kernel-arg-info option" to follow current spec:
https://www.khronos.org/registry/spir/specs/spir_spec-1.2.pdf

Let me know if you'd rather receive them through a different medium or if
you want me to make any changes to them.

And I'll work on putting together some patches for the other parts.


----------
From: *Pekka Jääskeläinen* <pekka.jaaskelainen at tut.fi>
Date: Fri, Nov 21, 2014 at 4:34 AM
To: Ribulous Barnulous <rnickb731 at gmail.com>


Thanks,

Can you post these to the cfe-dev list as well?

----------
From: *Ribulous Barnulous* <rnickb731 at gmail.com>
Date: Fri, Nov 21, 2014 at 11:18 PM
To: Pekka Jääskeläinen <pekka.jaaskelainen at tut.fi>


Yep.

They're attached.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20141121/66a78e6c/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: spir_target.patch
Type: application/octet-stream
Size: 1605 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20141121/66a78e6c/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: opencl_arginfo.patch
Type: application/octet-stream
Size: 1134 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20141121/66a78e6c/attachment-0001.obj>