[PATCH] D20979: [OpenCL] Use function metadata to represent kernel attributes

Tue Aug 2 07:16:14 PDT 2016

The LLVM IR generated by Clang trunk for spir target is not conformant to SPIR spec 1.2 or 2.0 even without my change. For example, it differs from SPIR spec about LLVM version, image type name, sampler type representation, blocks representation, etc. It is the result of evolution of the original SPIR for a better solution of many issues of SPIR.

If we look back at the RFC about the new kernel argument metadata, there was agreement that the original kernel argument metadata is difficult to work with, therefore the proposal to use function metadata was accepted to alleviate this issue. The new function metadata was more concise and easier to work with.

For SPIR consumer, the new format conveys equivalent information as the old format. There is no reason they cannot consume this new format.

If users want to generate SPIR conformant LLVM IR, they should use khronos SPIR producer instead of clang trunk.

Sam

-----Original Message-----
From: Pan Xiuli [mailto:xiulipan at outlook.com] 
Sent: Monday, August 1, 2016 10:40 PM
To: 'Anastasia Stulova' <Anastasia.Stulova at arm.com>; reviews+D20979+public+e2872c9c869f1bc7 at reviews.llvm.org; alexey.bader at intel.com; Liu, Yaxun (Sam) <Yaxun.Liu at amd.com>
Cc: Stellard, Thomas <Tom.Stellard at amd.com>; cfe-commits at lists.llvm.org; 'nd' <nd at arm.com>
Subject: RE: [PATCH] D20979: [OpenCL] Use function metadata to represent kernel attributes

Hi,

What I mean is the spec of spir 1.2/2.0 has very clear example about how the metadata should organized. But now with this patch, the result of spir is changed and not like it before. So what I want to know is why we change the old spir example style code, but not add something alongside.
Now we have metadata after each function but spir shows a opencl.kernels metadata is need for all modules:
"Each SPIR module has a opencl.kernels named metadata node containing a list of metadata objects."

I think this patch maybe useful for some other target, but we should not break the old spir as now we have not had a substitute one.

Thanks
Xiuli

-----Original Message-----
From: Anastasia Stulova [mailto:Anastasia.Stulova at arm.com] 
Sent: Tuesday, August 2, 2016 2:14 AM
To: Pan Xiuli <xiulipan at outlook.com>; reviews+D20979+public+e2872c9c869f1bc7 at reviews.llvm.org; alexey.bader at intel.com; Liu, Yaxun (Sam) (Yaxun.Liu at amd.com) <Yaxun.Liu at amd.com>
Cc: thomas.stellard at amd.com; cfe-commits at lists.llvm.org; nd <nd at arm.com>
Subject: RE: [PATCH] D20979: [OpenCL] Use function metadata to represent kernel attributes

Hi Xiuli,

Could you please elaborate what do you think it broken exactly?

Because we haven't actually removed opencl.kernels metadata but just changed the format of it. 

Basically, we are using function metadata instead of generic metadata as it was before.

Thanks,
Anastasia

-----Original Message-----
From: Pan Xiuli [mailto:xiulipan at outlook.com] 
Sent: 01 August 2016 05:54
To: reviews+D20979+public+e2872c9c869f1bc7 at reviews.llvm.org; Anastasia Stulova; alexey.bader at intel.com
Cc: thomas.stellard at amd.com; cfe-commits at lists.llvm.org
Subject: RE: [PATCH] D20979: [OpenCL] Use function metadata to represent kernel attributes

Hi Sam,

I am now using llvm39 to enable our opencl driver, but I found this patch broke the spir target as well.
The opencl.kernels metadata is required by spir spec. I think we should keep the old code for spir target.

Thanks
Xiuli

-----Original Message-----
From: Yaxun Liu [mailto:yaxun.liu at amd.com] 
Sent: Wednesday, June 22, 2016 11:04 PM
To: yaxun.liu at amd.com; xiulipan at outlook.com; anastasia.stulova at arm.com; alexey.bader at intel.com
Cc: thomas.stellard at amd.com; cfe-commits at lists.llvm.org
Subject: Re: [PATCH] D20979: [OpenCL] Use function metadata to represent kernel attributes

This revision was automatically updated to reflect the committed changes.
Closed by commit rL273425: [OpenCL] Use function metadata to represent kernel attributes (authored by yaxunl).

Changed prior to commit:
  http://reviews.llvm.org/D20979?vs=60545&id=61555#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D20979

Files:
  cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
  cfe/trunk/test/CodeGenOpenCL/kernel-arg-info.cl
  cfe/trunk/test/CodeGenOpenCL/kernel-attributes.cl
  cfe/trunk/test/CodeGenOpenCL/kernel-metadata.cl