Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support OpenCL.DebugInfo.100 extended instruction set #3080

Merged
merged 16 commits into from
Dec 19, 2019

Conversation

dneto0
Copy link
Collaborator

@dneto0 dneto0 commented Nov 29, 2019

The interesting bits are:

  • The extended instruction set adds enum types, with names that overlap with enum types from the older DebugInfo extension. This affects spv_operand_type_t enum values.
  • So I added a mechanism to prefix the operand-kind names, so distinguish the cases.
  • The enums from the old DebugInfo are left unprefixed, but we do add a CLDEBUG100_ prefix to the new ones.

@dneto0 dneto0 requested a review from alan-baker November 29, 2019 17:00
@dneto0
Copy link
Collaborator Author

dneto0 commented Nov 29, 2019

The OpenCL.DebugInfo.100 extended instruction has been ratified by Khronos but is not yet on the SPIR-V registry.

@dneto0
Copy link
Collaborator Author

dneto0 commented Nov 29, 2019

cc: @AlexeySotkin

@dneto0
Copy link
Collaborator Author

dneto0 commented Nov 29, 2019

I don't have an active GN setup, so I took a guess at the BUILD.gn changes. Cc: @dj2 @zoddicus

@dneto0 dneto0 requested a review from dj2 November 29, 2019 19:48
@AlexeySotkin
Copy link

👍 @dneto0 Thank you very much!

@ehsannas
Copy link
Contributor

ehsannas commented Dec 3, 2019

Is there going to be a header file for the opcodes of this instruction set?
(similar to this for the GLSL ext set.)

{ "kind" : "IdRef", "name" : "'Source'" },
{ "kind" : "LiteralInteger", "name" : "'Version'" },
{ "kind" : "LiteralInteger", "name" : "'DWARF Version'" }
]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing some fields, (thanks @jaebaek for finding this)

@dneto0 dneto0 changed the title Support OpenCL.DebugInfo.100 extended instruction set WIP: Support OpenCL.DebugInfo.100 extended instruction set Dec 3, 2019
@jaebaek jaebaek self-requested a review December 3, 2019 19:21
@dneto0
Copy link
Collaborator Author

dneto0 commented Dec 3, 2019

Hey @jaebaek please take a look now. I think I fixed the operands to all the instructions now.

@dneto0 dneto0 changed the title WIP: Support OpenCL.DebugInfo.100 extended instruction set Support OpenCL.DebugInfo.100 extended instruction set Dec 3, 2019
@dneto0
Copy link
Collaborator Author

dneto0 commented Dec 3, 2019

Is there going to be a header file for the opcodes of this instruction set?

The build generates an OpenCLDebugInfo.h file in the build output directly.
Ideally all these grammar files would be migrated into the KhronosGroup/SPIRV-Headers repo, and the tooling to generate headers too.

@dneto0
Copy link
Collaborator Author

dneto0 commented Dec 3, 2019

The generated header file is at this gist: https://gist.github.com/dneto0/a8fe7e0362b817092fde746a97322204

@ehsannas
Copy link
Contributor

ehsannas commented Dec 3, 2019

SG. Thanks!

Copy link
Contributor

@jaebaek jaebaek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to check test/ext_inst.cldebug100_test.cpp once more, but before doing that, I found 4 mismatches with the OpenCL.100.debug extension spec.

{
"opname" : "DebugTypeFunction",
"opcode" : 8,
"operands" : [
Copy link
Contributor

@jaebaek jaebaek Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first operand must be DebugInfoFlags. I mean { "kind" : "DebugInfoFlags", "name" : "'Flags'" }, is missing.

{ "kind" : "IdRef", "name" : "'Source'" },
{ "kind" : "LiteralInteger", "name" : "'Line'" },
{ "kind" : "LiteralInteger", "name" : "'Column'" },
{ "kind" : "IdRef", "name" : "'Parent'" },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{ "kind" : "IdRef", "name" : "'Linkage Name'" }, is missing

{ "kind" : "IdRef", "name" : "'Source'" },
{ "kind" : "LiteralInteger", "name" : "'Line'" },
{ "kind" : "LiteralInteger", "name" : "'Column'" },
{ "kind" : "IdRef", "name" : "'Parent'" },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{ "kind" : "DebugInfoFlags", "name" : "'Flags'" } is missing

{
"opname" : "DebugValue",
"opcode" : 29,
"operands" : [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{ "kind" : "IdRef", "name" : "'Local Variable'" }, is missing.

@dneto0 dneto0 changed the title Support OpenCL.DebugInfo.100 extended instruction set WIP: Support OpenCL.DebugInfo.100 extended instruction set Dec 3, 2019
@dneto0
Copy link
Collaborator Author

dneto0 commented Dec 3, 2019

Hi @jaebaek I think you reviewed the old commit. I posted another commit after that. Please take another look.

@jaebaek
Copy link
Contributor

jaebaek commented Dec 3, 2019

Oh, you are right. It looks good to me now.

jaebaek added a commit to jaebaek/SPIRV-Tools that referenced this pull request Dec 16, 2019
@dneto0 dneto0 force-pushed the opencl.debuginfo.100 branch from 3fcf01f to 717f281 Compare December 19, 2019 20:15
@dneto0 dneto0 changed the title WIP: Support OpenCL.DebugInfo.100 extended instruction set Support OpenCL.DebugInfo.100 extended instruction set Dec 19, 2019
@dneto0
Copy link
Collaborator Author

dneto0 commented Dec 19, 2019

Rebased. Keeping the approval.

@dneto0 dneto0 merged commit 64f36ea into KhronosGroup:master Dec 19, 2019
dneto0 pushed a commit to dneto0/SPIRV-Tools that referenced this pull request Sep 14, 2024
Roll third_party/glslang/ ebf634b..40801e3 (8 commits)

KhronosGroup/glslang@ebf634b...40801e3

$ git log ebf634b..40801e3 --date=short --no-merges --format='%ad %ae %s'
2020-01-06 cepheus Bump revision
2020-01-06 laddoc Add builtin constants
2019-12-26 cepheus HLSL: Fix KhronosGroup#2037: Integer dot used incorrect input for adds.
2019-12-25 laddoc atomic counter offset should align to 4
2019-11-26 laddoc Add support for ARB_uniform_buffer_object
2019-11-26 laddoc Add support for ARB_texture_multisample
2019-11-26 laddoc Add support for ARB_sample_shading
2019-12-20 cepheus Command-line: Give better error messages. From KhronosGroup#1829.

Roll third_party/googletest/ 5b162a79d..306f3754a (17 commits)

google/googletest@5b162a7...306f375

$ git log 5b162a79d..306f3754a --date=short --no-merges --format='%ad %ae %s'
2019-12-30 absl-team Googletest export
2019-12-30 absl-team Googletest export
2019-12-26 absl-team Googletest export
2019-12-19 absl-team Googletest export
2019-12-18 absl-team Googletest export
2019-12-18 absl-team Googletest export
2019-12-20 kontakt Make move operation noexcept.
2019-12-20 kontakt Define default destructor for test classes
2019-12-20 kontakt Deleted functions as part of public interface
2019-12-20 kontakt Review notes: Return T& from assignment operators
2019-12-17 kontakt Disable move constructor and assignment operator for test classes.
2019-12-16 krzysio Googletest export
2019-12-10 syoussefi Revert "Googletest export": disallow empty prefix
2019-12-10 syoussefi Revert "Googletest export": Remove test for empty prefix
2019-12-16 syoussefi Workaround VS bug w.r.t empty arguments to macros
2019-12-13 kravlala.1 Activate GNU extensions in case of MSYS generator
2019-11-22 krystian.kuzniarek remove stale comments about older GCC versions

Roll third_party/re2/ 7470f4d02..00af5b44d (2 commits)

google/re2@7470f4d...00af5b4

$ git log 7470f4d02..00af5b44d --date=short --no-merges --format='%ad %ae %s'
2020-01-05 junyer Fix a comment.
2019-12-29 junyer Make DFA use hints.

Roll third_party/spirv-cross/ f912c3289..961b9014a (6 commits)

KhronosGroup/SPIRV-Cross@f912c32...961b901

$ git log f912c3289..961b9014a --date=short --no-merges --format='%ad %ae %s'
2020-01-06 post Fix Clang warnings.
2020-01-06 post Roll custom versions of isalpha/isalnum.
2020-01-06 post Add test shader for OpCopyLogical with packing/unpacking.
2020-01-06 post Go through access chain path for OpCopyLogical.
2020-01-06 post Basic implementation of OpCopyLogical.
2019-12-21 dm86.jang Add debug prefix on Windows

Roll third_party/spirv-tools/ 96354f5..c8bf143 (12 commits)

KhronosGroup/SPIRV-Tools@96354f5...c8bf143

$ git log 96354f5..c8bf143 --date=short --no-merges --format='%ad %ae %s'
2020-01-06 dneto GetOperandConstants operand can be const (KhronosGroup#3126)
2019-12-27 dneto Avoid pessimizing std::move (KhronosGroup#3124)
2019-12-27 kburjack Fix typo in validation message (KhronosGroup#3122)
2019-12-27 greg Change default version for CreatInstBindlessCheckPass to 2 (KhronosGroup#3119)
2019-12-20 greg Fix convert-relax-to-half invalid code (KhronosGroup#3099) (KhronosGroup#3106)
2019-12-19 dneto Support OpenCL.DebugInfo.100 extended instruction set (KhronosGroup#3080)
2019-12-19 afdx spirv-fuzz: Always add new globals to entry point interfaces (KhronosGroup#3113)
2019-12-19 afdx spirv-fuzz: Transformation to add a new function to a module (KhronosGroup#3114)
2019-12-19 afdx spirv-fuzz: Avoid passing access chains as parameters (KhronosGroup#3112)
2019-12-18 dneto Add support for SPV_KHR_non_semantic_info (KhronosGroup#3110)
2019-12-16 afdx spirv-fuzz: Transformations to add types, constants and variables (KhronosGroup#3101)
2019-12-16 greg Make Instrumentation format version 2 the default (Step 1) (KhronosGroup#3096)

Created with:
  roll-dep third_party/effcee third_party/glslang third_party/googletest third_party/re2 third_party/spirv-cross third_party/spirv-headers third_party/spirv-tools
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants