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: Implement FuzzerPassAddParameters #3399

Merged
merged 9 commits into from Jun 23, 2020
Merged

spirv-fuzz: Implement FuzzerPassAddParameters #3399

merged 9 commits into from Jun 23, 2020

Conversation

Vasniktel
Copy link
Collaborator

@Vasniktel Vasniktel commented Jun 4, 2020 •
edited

Fixes #3384.

@Vasniktel Vasniktel marked this pull request as draft June 4, 2020 16:18
afd
afd reviewed Jun 5, 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 very promising! Some comments.

source/fuzz/fuzzer_context.cpp Outdated Show resolved Hide resolved
source/fuzz/fuzzer_context.h Outdated Show resolved Hide resolved
source/fuzz/fuzzer_pass_add_parameters.cpp Show resolved Hide resolved
source/fuzz/fuzzer_pass_add_parameters.cpp Outdated Show resolved Hide resolved
source/fuzz/fuzzer_pass_add_parameters.cpp Outdated Show resolved Hide resolved
source/fuzz/fuzzer_pass_add_parameters.cpp Show resolved Hide resolved
source/fuzz/fuzzer_pass_add_parameters.cpp Outdated Show resolved Hide resolved
source/fuzz/fuzzer_pass_add_parameters.cpp Outdated Show resolved Hide resolved
source/fuzz/transformation_add_parameters.cpp Show resolved Hide resolved
@Vasniktel Vasniktel force-pushed the add_parameters_to_functions branch from c393709 to 167cc6e Compare June 5, 2020 17:11
@Vasniktel Vasniktel marked this pull request as ready for review June 5, 2020 18:36
@Vasniktel Vasniktel requested a review from afd June 5, 2020 18:36
@afd afd added the kokoro:run label Jun 9, 2020
afd
afd suggested changes Jun 9, 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.

Looks great! I recommend that you add a break in your default switch case. Once done I will merge (if the bots pass).

source/fuzz/fuzzer_pass_add_parameters.cpp Outdated Show resolved Hide resolved
@Vasniktel Vasniktel requested a review from afd June 9, 2020 15:31
@afd
Copy link
Contributor

afd commented Jun 10, 2020

@Vasniktel can you rebase please?

@Vasniktel Vasniktel force-pushed the add_parameters_to_functions branch from 9f3603f to a69c89b Compare June 10, 2020 04:59
@Vasniktel
Copy link
Collaborator Author

@afd Done.

@afd afd added the kokoro:run label Jun 10, 2020
@afd afd added the kokoro:run label Jun 10, 2020
@Vasniktel Vasniktel force-pushed the add_parameters_to_functions branch from fc9529a to e6f70fb Compare June 11, 2020 19:32
@afd afd added the kokoro:run label Jun 12, 2020
@Vasniktel Vasniktel requested a review from afd June 15, 2020 13:19
@Vasniktel Vasniktel force-pushed the add_parameters_to_functions branch from e6f70fb to 48fda01 Compare June 17, 2020 07:31
afd
afd suggested changes Jun 18, 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.

Well done for spotting that issue regarding irrelevant pointee facts - it would be good to get that fixed.

source/fuzz/transformation_add_parameters.cpp Outdated Show resolved Hide resolved
@Vasniktel Vasniktel force-pushed the add_parameters_to_functions branch from 48fda01 to 5eed6c7 Compare June 20, 2020 11:05
@Vasniktel Vasniktel requested a review from afd June 20, 2020 11:06
afd
afd suggested changes Jun 23, 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 is basically ready to go but there's a bit of inconsistency as to whether or not pointers are supported. See comment in the review.

source/fuzz/transformation_add_parameters.cpp Outdated Show resolved Hide resolved
@Vasniktel Vasniktel requested a review from afd June 23, 2020 13:38
@Vasniktel Vasniktel force-pushed the add_parameters_to_functions branch from 9f75c77 to bb14820 Compare June 23, 2020 14:08
@afd afd added the kokoro:run label Jun 23, 2020
afd
afd approved these changes Jun 23, 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 29ba53f into KhronosGroup:master Jun 23, 2020
@Vasniktel Vasniktel deleted the add_parameters_to_functions branch June 24, 2020 08:46
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: Transformation to add new parameters to the existing function
4 participants
@Vasniktel @afd @paulthomson @kokoro-team