Skip to content

spirv-fuzz: TransformationAddSynonyms#3447

Merged
afd merged 16 commits intoKhronosGroup:masterfrom
Vasniktel:adjust_math_instructions
Jul 12, 2020
Merged

spirv-fuzz: TransformationAddSynonyms#3447
afd merged 16 commits intoKhronosGroup:masterfrom
Vasniktel:adjust_math_instructions

Conversation

@Vasniktel
Copy link
Copy Markdown
Collaborator

@Vasniktel Vasniktel commented Jun 18, 2020

Part of #3440.

@Vasniktel Vasniktel marked this pull request as draft June 18, 2020 14:58
@Vasniktel Vasniktel force-pushed the adjust_math_instructions branch 2 times, most recently from 8213aa6 to 947f228 Compare June 23, 2020 14:11
Copy link
Copy Markdown
Contributor

@afd afd left a comment

Choose a reason for hiding this comment

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

It looks like this PR contains three distinct transformations.

I know it can be a bit painful but I would prefer us to stick - where possible - to one transformation per PR.

Could you split this into multiple PRs? I have inserted a few initial comments on each transformation.

I felt I understood the float-cast transformation well and I really like it, so I suggest you do that one first.

The others look interesting but - as per the comments in the review - I didn't fully understand them.

@Vasniktel Vasniktel force-pushed the adjust_math_instructions branch from 947f228 to fa3db6d Compare June 25, 2020 09:55
@Vasniktel Vasniktel requested a review from afd June 25, 2020 09:57
@Vasniktel Vasniktel force-pushed the adjust_math_instructions branch from fa3db6d to 844ff16 Compare June 28, 2020 12:10
@Vasniktel Vasniktel changed the title spirv-fuzz: Add synonyms and invert comparison operators spirv-fuzz: TransformationAddSynonyms Jun 28, 2020
Copy link
Copy Markdown
Contributor

@afd afd left a comment

Choose a reason for hiding this comment

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

See comment inline - I think this one requires a bit of thought; we should discuss it.

@Vasniktel Vasniktel force-pushed the adjust_math_instructions branch 2 times, most recently from 4532dd3 to ecf877b Compare July 2, 2020 17:14
@Vasniktel Vasniktel requested a review from afd July 2, 2020 17:33
@Vasniktel Vasniktel force-pushed the adjust_math_instructions branch from 60e604d to d06156b Compare July 7, 2020 16:52
Copy link
Copy Markdown
Contributor

@afd afd left a comment

Choose a reason for hiding this comment

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

I have reviewed all classes except for the actual transformation itself. Looks excellent so far - some small changes requested.

@Vasniktel Vasniktel requested a review from afd July 8, 2020 11:05
Copy link
Copy Markdown
Contributor

@afd afd left a comment

Choose a reason for hiding this comment

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

This looks really good. Some comments inline.

I suggest that you get tests done and get this one merged without doing anything to FuzzerPassCopyObject nor TransformationCopyObject, but open an issue indicating that TransformationCopyObject should go and should be replaced with appropriate uses of TransformationAddSynonym. Then in a follow-up PR we can rip out TransformationCopyObject.

I think that keeping FuzzerPassCopyObject would be OK - does no harm to have it in addition to the pass here being capable of producing an object copy synonym (it just bumps up the probability of an object-copy synonym being created).

@Vasniktel Vasniktel force-pushed the adjust_math_instructions branch from d4f303c to 8589b81 Compare July 10, 2020 10:08
@Vasniktel Vasniktel requested a review from afd July 10, 2020 10:10
@Vasniktel Vasniktel force-pushed the adjust_math_instructions branch from 2f83b9f to c3a97cc Compare July 10, 2020 13:16
@Vasniktel Vasniktel marked this pull request as ready for review July 10, 2020 13:17
Copy link
Copy Markdown
Contributor

@afd afd left a comment

Choose a reason for hiding this comment

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

LGTM.

@Vasniktel Vasniktel force-pushed the adjust_math_instructions branch from 43e4ea5 to d2f28c1 Compare July 11, 2020 09:00
@afd afd added the kokoro:run label Jul 11, 2020
@afd afd added the kokoro:run label Jul 12, 2020
@afd afd merged commit 40c3c1c into KhronosGroup:master Jul 12, 2020
@Vasniktel Vasniktel deleted the adjust_math_instructions branch July 13, 2020 13:44
dnovillo pushed a commit to dnovillo/SPIRV-Tools that referenced this pull request Aug 19, 2020
dneto0 pushed a commit to dneto0/SPIRV-Tools that referenced this pull request Sep 14, 2024
Roll third_party/effcee/ 5af957bbf..2ec8f8738 (3 commits)

google/effcee@5af957b...2ec8f87

$ git log 5af957bbf..2ec8f8738 --date=short --no-merges --format='%ad %ae %s'
2020-06-16 dneto Start v2020.0-dev
2020-06-16 dneto Finalize v2019.1
2020-06-15 dneto Update CHANGES

Created with:
  roll-dep third_party/effcee

Roll third_party/glslang/ e8c9fd6..b481744 (19 commits)

KhronosGroup/glslang@e8c9fd6...b481744

$ git log e8c9fd6..b481744 --date=short --no-merges --format='%ad %ae %s'
2020-07-14 bclayton Give build_info.py the executable bit
2020-07-14 cepheus Fix recently found non-determinism with gl_WorldToObject3x4EXT.
2020-07-14 cepheus Non-determinism: Remove test file that seems to trigger non-determinism.
2020-07-13 bclayton Add bison license to LICENSE.txt
2020-07-13 bclayton CMake: Move project() to top of CMakeLists.txt
2020-07-13 bclayton Kokoro: Print test output to stdout
2020-07-13 cepheus Fix comma in licence checker.
2020-07-13 cepheus Revert "Merge pull request KhronosGroup#2330 from ShabbyX/optimize_for_angle"
2020-07-13 cepheus Fix a couple lines that were too long, to retrigger bots.
2020-07-13 cepheus Fix KhronosGroup#2329: don't use invalid initializers.
2020-07-12 bclayton Add missing comma from license-checker.cfg
2020-07-12 ccom Common: include standard headers before doing any defines
2020-07-10 bclayton Fix CMake rules when nesting CMake projects
2020-07-10 bclayton Attempt to fix chromium builds
2020-06-17 bclayton Generate build information from CHANGES.md
2020-07-03 ShabbyX Customize glslang.y to GLSLANG_ANGLE
2020-07-03 ShabbyX Use GLSLANG_ANGLE to strip features to what ANGLE requires
2020-07-07 rharrison Make sure glslang_angle has a definition in BUILD.gn
2020-07-07 bclayton Use CMake's builtin functionality for PCHs

Created with:
  roll-dep third_party/glslang

Roll third_party/googletest/ 356f2d264..70b90929b (7 commits)

google/googletest@356f2d2...70b9092

$ git log 356f2d264..70b90929b --date=short --no-merges --format='%ad %ae %s'
2020-07-09 absl-team Googletest export
2020-07-07 ofats Googletest export
2020-07-07 absl-team Googletest export
2020-07-07 absl-team Googletest export
2020-04-11 olivier.ldff use target_compile_features to use c++11 if cmake > 3.8
2020-05-30 eli fix compilation on OpenBSD 6.7
2020-01-22 mjvk Fixes extensions missing for QNX

Created with:
  roll-dep third_party/googletest

Roll third_party/spirv-cross/ 559b21c6c..6575e451f (2 commits)

KhronosGroup/SPIRV-Cross@559b21c...6575e45

$ git log 559b21c6c..6575e451f --date=short --no-merges --format='%ad %ae %s'
2020-07-11 post MSVC 2013: Fix silently broken builds.
2020-07-07 troughton MSL: Ensure OpStore source operands are marked for inclusion in function arguments

Created with:
  roll-dep third_party/spirv-cross

Roll third_party/spirv-tools/ 6a4da9d..c9b254d (17 commits)

KhronosGroup/SPIRV-Tools@6a4da9d...c9b254d

$ git log 6a4da9d..c9b254d --date=short --no-merges --format='%ad %ae %s'
2020-07-14 andreperezmaselco.developer spirv-fuzz: Support adding dead break from back-edge block (KhronosGroup#3519)
2020-07-14 andreperezmaselco.developer Support OpPhi when replacing boolean constant operand (KhronosGroup#3518)
2020-07-12 vasniktel spirv-fuzz: TransformationAddSynonyms (KhronosGroup#3447)
2020-07-11 vasniktel spirv-fuzz: Remove unused functions (KhronosGroup#3510)
2020-07-11 vasniktel spirv-fuzz: Minor refactoring (KhronosGroup#3507)
2020-07-10 greg Preserve OpenCL.DebugInfo.100 through elim-local-single-store (KhronosGroup#3498)
2020-07-10 jaebaek Preserve debug info in vector DCE pass (KhronosGroup#3497)
2020-07-10 stefano.milizia00 Implement transformation to record synonymous constants. (KhronosGroup#3494)
2020-07-09 jaebaek Fix build failure (KhronosGroup#3508)
2020-07-09 greg Upgrade elim-local-single-block for OpenCL.DebugInfo.100 (KhronosGroup#3451)
2020-07-09 vasniktel spirv-fuzz: TransformationReplaceParameterWithGlobal (KhronosGroup#3434)
2020-07-09 andreperezmaselco.developer Implement the OpMatrixTimesVector linear algebra case (KhronosGroup#3500)
2020-07-08 jaebaek Preserve OpenCL.100.DebugInfo in reduce-load-size pass (KhronosGroup#3492)
2020-07-08 andreperezmaselco.developer spirv-fuzz: Add image sample unused components transformation (KhronosGroup#3439)
2020-07-07 andreperezmaselco.developer spirv-fuzz: Add variables with workgroup storage class (KhronosGroup#3485)
2020-07-07 andreperezmaselco.developer spirv-fuzz: Implement the OpVectorTimesMatrix linear algebra case (KhronosGroup#3489)
2020-07-07 vasniktel spirv-fuzz: fuzzerutil::MaybeGetConstant* KhronosGroup#3487

Created with:
  roll-dep 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.

3 participants