This is an archive of the discontinued LLVM Phabricator instance.

[OpenCL] Add ocl and spir version for spir target
ClosedPublic

Authored by pxli168 on Feb 25 2016, 12:47 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

pxli168 updated this revision to Diff 49018.Feb 25 2016, 12:47 AM
pxli168 retitled this revision from to [OpenCL] Add ocl and spir version for spir target.
pxli168 updated this object.
pxli168 added reviewers: Anastasia, yaxunl.
pxli168 added a subscriber: pekka.jaaskelainen.
pxli168 added a subscriber: cfe-commits.
Anastasia edited edge metadata.Feb 25 2016, 7:46 AM

I was just wondering with SPIRV coming these days, how long should we be supporting and maintaining previous SPIR versions. Might be worth clarifying that...

lib/CodeGen/TargetInfo.cpp
7022 ↗(On Diff #49018)

Could we please separate this from XCore code?

Add separate anonymous namespace for it and proper heading comment. See XCore above as an example.

yaxunl accepted this revision.Feb 25 2016, 8:34 AM
yaxunl edited edge metadata.

Pls revise by Anastasia's comments. Otherwise LGTM.

We have two options about SPIR-V support:

  1. drop SPIR support and move on to SPIR-V
  2. keep supporting both SPIR and SPIR-V

Option 1 is cleaner and easier to maintain, however it will require changes in backends which expect SPIR format for OCL programs.

This revision is now accepted and ready to land.Feb 25 2016, 8:34 AM
pxli168 updated this revision to Diff 49136.Feb 25 2016, 6:00 PM
pxli168 edited edge metadata.
pxli168 marked an inline comment as done.Feb 25 2016, 6:12 PM

I think this is just a small change to help identify the which standard llvm-ir is using.

And there is a SPIRV branch from Khronos
https://github.com/KhronosGroup/SPIRV-LLVM
We can start there and try to merge the spir-v support into the llvm.org trunk.

Anastasia added inline comments.Feb 26 2016, 2:40 AM
lib/CodeGen/TargetInfo.cpp
7211 ↗(On Diff #49136)

Could you change the comment please to something more specific. For example:
"Emit SPIR specific metadata: OpenCL and SPIR version."

7214 ↗(On Diff #49136)

Remove \n

test/CodeGenOpenCL/spir_version.cl
15 ↗(On Diff #49136)

Forgotten to check OpenCL version here?

pxli168 updated this revision to Diff 51126.Mar 20 2016, 3:16 AM
pxli168 added inline comments.
test/CodeGenOpenCL/spir_version.cl
15 ↗(On Diff #49136)

The metadate will be merged into the same one, so there will be only one 2.0 here.

pxli168 marked 2 inline comments as done.Mar 20 2016, 3:16 AM
test/CodeGenOpenCL/spir_version.cl
15 ↗(On Diff #51126)

Can you test 'spir64' too?

pxli168 updated this revision to Diff 51253.Mar 21 2016, 9:28 PM

Add test for spir64.

Anastasia accepted this revision.Mar 22 2016, 11:25 AM
Anastasia edited edge metadata.

LGTM!

This revision was automatically updated to reflect the committed changes.