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

Adding OpEntryPoint validation #2862

Merged
merged 2 commits into from
Sep 16, 2019

Conversation

raunraun
Copy link
Contributor

OpEntryPoint

Declare an entry point, its execution model, and its interface.

Execution Model is the execution model for the entry point and its static call tree. See Execution Model.

Entry Point must be the Result  of an OpFunction instruction.

Name is a name string for the entry point. A module cannot have two OpEntryPoint instructions with the same Execution Model and the same Name string.

@CLAassistant
Copy link

CLAassistant commented Sep 12, 2019

CLA assistant check
All committers have signed the CLA.

@zoddicus
Copy link
Contributor

I am adding @alan-baker as a reviewer since they are familiar with this code and I am going to be away for a few days, but I have some thoughts/comments.

There already exists a more strict version of this check for WebGPU in the validator, 16a0da3. Specifically WebGPU requires entry points are unique regardless of the execution model.

I suspect the existing solution and yours could be merged together, so that there is a function to get all non-unique entry point names, and then the check would use that. If the environment is WebGPU then any non-unique entry points would fail validation, but for other environments the execution models check would then be performed.

@zoddicus zoddicus self-requested a review September 12, 2019 20:55
@raunraun
Copy link
Contributor Author

I completely missed the WebGPU check for unique entry point names. I'll go take a look.

Copy link
Contributor

@alan-baker alan-baker left a comment

Choose a reason for hiding this comment

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

This would be simpler to just modify the first loop post-parse in validate.cpp that registers entry points. Keep a set of pairs (execution mode, name) to check against.

@raunraun raunraun force-pushed the OpEntryPointValidation branch from 2d77f9a to 6b042ab Compare September 16, 2019 13:07
@raunraun
Copy link
Contributor Author

Alright, take 2. Thansk @alan-baker

@alan-baker
Copy link
Contributor

Can you please rebase to pick up the 1.5 tooling updates?

@alan-baker alan-baker merged commit 99793fa into KhronosGroup:master Sep 16, 2019
@zoddicus zoddicus removed their request for review January 19, 2023 14:56
dneto0 pushed a commit to dneto0/SPIRV-Tools that referenced this pull request Sep 14, 2024
Roll third_party/effcee/ 6527fb254..cd25ec17e (2 commits)

google/effcee@6527fb2...cd25ec1

$ git log 6527fb254..cd25ec17e --date=short --no-merges --format='%ad %ae %s'
2019-09-18 dneto Start v2019.1-dev
2019-09-18 dneto Finalize v2019.0

Roll third_party/glslang/ 664ad41..caca1d1 (12 commits)

KhronosGroup/glslang@664ad41...caca1d1

$ git log 664ad41..caca1d1 --date=short --no-merges --format='%ad %ae %s'
2019-09-17 cepheus SPV_KHR_physical_storage_buffer/SPV: Add GL_EXT_buffer_reference_uvec2
2019-09-16 digit Fix Fuchsia build.
2019-09-18 cepheus README: Fix WASM typos.
2019-09-18 cepheus HLSL: Fix KhronosGroup#1903 Catch 0-argument case to constructors.
2019-09-08 jbolz Add GL_EXT_shader_subgroup_extended_types support
2019-08-28 cepheus GLSL: Only require constant for subgroupBroadcast when SPV < 1.5.
2019-08-18 cepheus SPV: Support SPIR-V 1.5; five extensions no longer need OpExtension.
2019-09-09 laddoc Add flags for local size values ( compute shader )
2019-09-13 cepheus SPV 1.5: Switch to the 1.5 header, for SPIR-V 1.5.
2019-09-11 dsinclair Comment out params instead of removing
2019-09-10 dsinclair Remove unused params
2019-09-09 rex.xu Fix incorrect function prototypes of 64-bit findLSB/findMSB

Roll third_party/googletest/ 3f05f651a..f2fb48c3b (9 commits)

google/googletest@3f05f65...f2fb48c

$ git log 3f05f651a..f2fb48c3b --date=short --no-merges --format='%ad %ae %s'
2019-09-16 krystian.kuzniarek Googletest export
2019-09-13 misterg Googletest export
2019-09-12 hgsilverman Googletest export
2019-09-11 absl-team Googletest export
2019-09-10 absl-team Googletest export
2019-09-09 absl-team Googletest export
2019-09-06 absl-team Googletest export
2019-09-06 absl-team Googletest export
2019-08-12 krystian.kuzniarek restore mistakenly removed iffs in their explicit form

Roll third_party/spirv-cross/ b32a1b415..5431e1da2 (7 commits)

KhronosGroup/SPIRV-Cross@b32a1b4...5431e1d

$ git log b32a1b415..5431e1da2 --date=short --no-merges --format='%ad %ae %s'
2019-09-19 post Update SPIR-V headers.
2019-09-19 post MSL: Fix 16-bit integer literals.
2019-09-18 rharrison Update external/ to SPIR-V 1.5
2019-09-18 post CMake: Add option to force -fPIC.
2019-09-17 post Fix -Wshorten-64-to-32 warnings.
2019-09-16 post CMake: Add option to skip installation targets.
2019-09-12 post Consider discard and demote as impure statements.

Roll third_party/spirv-headers/ 38cafab..601d738 (2 commits)

KhronosGroup/SPIRV-Headers@38cafab...601d738

$ git log 38cafab..601d738 --date=short --no-merges --format='%ad %ae %s'
2019-09-18 cepheus Add SPV_KHR_physical_storage_buffer.
2019-09-13 cepheus SPIR-V 1.5.

Roll third_party/spirv-tools/ 76261e2..08fcf8a (28 commits)

KhronosGroup/SPIRV-Tools@76261e2...08fcf8a

$ git log 76261e2..08fcf8a --date=short --no-merges --format='%ad %ae %s'
2019-09-19 ehsannas Fix header include syntax. (KhronosGroup#2882)
2019-09-19 stevenperron Handle OpConstantNull in copy-prop-arrays. (KhronosGroup#2870)
2019-09-19 dneto Fix comment typo found by protobufs linter (KhronosGroup#2884)
2019-09-19 dsinclair Move docs into docs/ folder (KhronosGroup#2872)
2019-09-18 dsinclair Add WebGPU SPIR-V Assembler in JavaScript. (KhronosGroup#2876)
2019-09-18 dneto Android.mk: Add dependency from optimizer file to amd-shader-ballot-insts.inc (KhronosGroup#2883)
2019-09-18 dneto Update SPIRV-Headers in DEPS (KhronosGroup#2880)
2019-09-18 afdx  Fix detection of blocks bypassed by new edge (KhronosGroup#2874)
2019-09-18 afdx Fix CMake issue related to spirv-fuzz (KhronosGroup#2877)
2019-09-18 afdx Add fuzzer pass to replace ids with synonyms (KhronosGroup#2857)
2019-09-18 alanbaker Relaxed bitcast with pointers (KhronosGroup#2878)
2019-09-17 52076061+digit-google Fix Fuchsia build. (KhronosGroup#2868)
2019-09-16 raun.krisch Adding valilidation checks for OpEntryPoint duplicate names and execution mode (KhronosGroup#2862)
2019-09-16 alanbaker Extra resource interface validation (KhronosGroup#2864)
2019-09-13 alanbaker Split capability tests (KhronosGroup#2866)
2019-09-13 alanbaker SPIRV-Tools support for SPIR-V 1.5 (KhronosGroup#2865)
2019-09-11 afdx Add fuzzer pass to copy objects (KhronosGroup#2853)
2019-09-11 rharrison Handle another case where creating a constant can fail (KhronosGroup#2854)
2019-09-11 stevenperron Don't inline function containing OpKill (KhronosGroup#2842)
2019-09-11 stevenperron Handle id overflow in wrap op kill. (KhronosGroup#2851)
2019-09-11 dneto Assembler: Can't set an ID in instruction without result ID (KhronosGroup#2852)
2019-09-10 zoddicus Handle creating a new constant failing gracefully (KhronosGroup#2848)
2019-09-10 afdx Rework management of probabilities in spirv-fuzz (KhronosGroup#2839)
2019-09-10 afdx Fix add-dead-break and add-dead-continue passes to respect dominance (KhronosGroup#2838)
2019-09-10 stevenperron Handle id overflow in the ssa rewriter. (KhronosGroup#2845)
2019-09-09 stevenperron Handle id overflow in the constant manager. (KhronosGroup#2844)
2019-09-09 alanbaker Add generic builtin validation of target (KhronosGroup#2843)
2019-09-09 stevenperron Don't register duplicate decoration in validator. (KhronosGroup#2841)

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants