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.

Sign up for GitHub

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

Jump to bottom

spirv-fuzz: TransformationPropagateInstructionDown #3692

Merged
merged 14 commits into from Oct 6, 2020
Merged

spirv-fuzz: TransformationPropagateInstructionDown #3692

merged 14 commits into from Oct 6, 2020

Conversation

Vasniktel
Copy link
Collaborator

@Vasniktel Vasniktel commented Aug 13, 2020 •
edited

Fixes #3691.

@Vasniktel Vasniktel marked this pull request as draft August 13, 2020 13:47
afd
afd suggested changes Aug 27, 2020
View reviewed changes
Copy link
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 cool! Some comments on everything but the actual transformation cpp file - didn't get time to review that yet, but it would be slightly easier to review it after you address these changes.

source/fuzz/protobufs/spvtoolsfuzz.proto Outdated Show resolved Hide resolved
source/fuzz/protobufs/spvtoolsfuzz.proto Outdated Show resolved Hide resolved
source/fuzz/protobufs/spvtoolsfuzz.proto Outdated Show resolved Hide resolved
source/fuzz/protobufs/spvtoolsfuzz.proto Outdated Show resolved Hide resolved
source/fuzz/fuzzer_pass_propagate_instructions_down.cpp Outdated Show resolved Hide resolved
source/fuzz/fuzzer_pass_propagate_instructions_down.cpp Outdated Show resolved Hide resolved
source/fuzz/transformation_propagate_instruction_down.h Outdated Show resolved Hide resolved
source/fuzz/transformation_propagate_instruction_down.h Outdated Show resolved Hide resolved
source/fuzz/transformation_propagate_instruction_down.h Outdated Show resolved Hide resolved
source/fuzz/transformation_propagate_instruction_down.h Outdated Show resolved Hide resolved
@Vasniktel Vasniktel requested a review from afd October 1, 2020 12:54
afd
afd reviewed Oct 2, 2020
View reviewed changes
Copy link
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.

Some review feedback.

source/fuzz/protobufs/spvtoolsfuzz.proto Outdated Show resolved Hide resolved
source/fuzz/transformation_propagate_instruction_down.cpp Outdated Show resolved Hide resolved
source/fuzz/transformation_propagate_instruction_down.cpp Outdated Show resolved Hide resolved
afd
afd reviewed Oct 2, 2020
View reviewed changes
Copy link
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.

Some ideas for where examples would be useful.

source/fuzz/transformation_propagate_instruction_down.h Show resolved Hide resolved
source/fuzz/transformation_propagate_instruction_down.h Outdated Show resolved Hide resolved
source/fuzz/transformation_propagate_instruction_down.h Outdated Show resolved Hide resolved
@Vasniktel Vasniktel requested a review from afd October 2, 2020 12:57
afd
afd reviewed Oct 2, 2020
View reviewed changes
Copy link
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.

Thanks for adding the example. This looks really good and I'll be happy to merge once there are tests.

I noted a couple of nits in comments since you're going to do another revision anyway.

source/fuzz/transformation_propagate_instruction_down.h Outdated Show resolved Hide resolved
source/fuzz/transformation_propagate_instruction_down.h Outdated Show resolved Hide resolved
@Vasniktel Vasniktel marked this pull request as ready for review October 5, 2020 12:30
@Vasniktel Vasniktel requested a review from afd October 5, 2020 12:30
afd
afd approved these changes Oct 6, 2020
View reviewed changes
Copy link
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!

@afd afd merged commit 63cc22d into KhronosGroup:master Oct 6, 2020
@Vasniktel Vasniktel deleted the propagate_instructions_down branch October 16, 2020 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@afd afd afd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

spirv-fuzz: Propagate instructions down
2 participants
@Vasniktel @afd