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

Sat Nov 22 02:18:52 PST 2014

Hello Ribulous,

 From the attachments, it seems like you have already done some work to 
port the changes from the Khronos SPIR generator to Clang. But it is not 
clear if you want to have them reviewed and submitted. As far as I know, 
there is no reason why they can't be submitted ... it's just that no one 
has bothered to do it before. It would be useful if you submit each 
patch as a separate review request.

Like Pekka said, SPIR 1.2 is based on LLVM 3.2, and there is no 
guarantee that the generated IR will be valid SPIR. But there is no harm 
in making sure that the SPIR target is maintained so that the generated 
IR is as close to SPIR 1.2 as possible. One could write a side-project 
that translates the LLVM IR to version 3.2 in order to produce perfect 
SPIR 1.2 if necessary. On the other hand, SPIR 2.0 is still provisional, 
so it's too early to say if those changes should be submitted to Clang.

Sameer.

On 11/22/2014 10:01 AM, Ribulous Barnulous wrote:
> 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 
> <mailto:pekka.jaaskelainen at tut.fi>>
> Date: Tue, Nov 18, 2014 at 5:18 AM
> To: Ribulous Barnulous <rnickb731 at gmail.com 
> <mailto:rnickb731 at gmail.com>>, cfe-dev at cs.uiuc.edu 
> <mailto: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 
> <mailto:rnickb731 at gmail.com>>
> Date: Thu, Nov 20, 2014 at 1:01 AM
> To: Pekka Jääskeläinen <pekka.jaaskelainen at tut.fi 
> <mailto: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 
> <mailto:pekka.jaaskelainen at tut.fi>>
> Date: Thu, Nov 20, 2014 at 9:52 AM
> To: Ribulous Barnulous <rnickb731 at gmail.com <mailto: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 
> <mailto:rnickb731 at gmail.com>>
> Date: Fri, Nov 21, 2014 at 1:02 AM
> To: Pekka Jääskeläinen <pekka.jaaskelainen at tut.fi 
> <mailto: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 
> <mailto:pekka.jaaskelainen at tut.fi>>
> Date: Fri, Nov 21, 2014 at 4:34 AM
> To: Ribulous Barnulous <rnickb731 at gmail.com <mailto:rnickb731 at gmail.com>>
>
>
> Thanks,
>
> Can you post these to the cfe-dev list as well?
>
> ----------
> From: *Ribulous Barnulous* <rnickb731 at gmail.com 
> <mailto:rnickb731 at gmail.com>>
> Date: Fri, Nov 21, 2014 at 11:18 PM
> To: Pekka Jääskeläinen <pekka.jaaskelainen at tut.fi 
> <mailto:pekka.jaaskelainen at tut.fi>>
>
>
> Yep.
>
> They're attached.
>
>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev