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

fuzz: Add libfuzzer compatible bounds fuzzer #7549

Merged
merged 8 commits into from
May 22, 2023

Conversation

nathaniel-brough
Copy link
Contributor

No description provided.

@nathaniel-brough
Copy link
Contributor Author

See #4088

Copy link
Member

@rootjalex rootjalex 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 great, thanks for doing this! Lgtm pending green buildbots.

@@ -96,108 +97,98 @@ Expr random_condition(Type T, int depth, bool maybe_scalar) {
};
const int op_count = sizeof(make_bin_op) / sizeof(make_bin_op[0]);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is unused, it must be removed

@steven-johnson
Copy link
Contributor

steven-johnson commented May 8, 2023

I took the liberty of harvesting fixes from #7551 and #7548 and applying them here (since our buildbot doesn't test this stuff, I pulled this into Google to test)... unfortunately it's still broken, as running it with default args ends up trying to construct a binary operation with vector elements with mismatched lanes. Gonna debug some more.

Specifically:

t IS uint32x6
a IS x3(ramp((uint32)b, (uint32)e, 2))
b IS x2(ramp((uint32)e, (uint32)4117721957, 3))

...it seems puzzling that we would be generating random expressions that return vector-expressions with a lane count different from the input types; I wouldn't think that would ever work.

@steven-johnson
Copy link
Contributor

hrm, this may have found a real bug: it seems we are generating two uint32x6:

// Both are uint32x6
a=x3(ramp((uint32)b, (uint32)e, 2)) 
b=x2(ramp((uint32)e, (uint32)4117721957, 3))

But if we attempt to cast either to uint1x6 (ie, bool), we fail inside cast at IROperator.cpp:1946 because we don't expect the type to be a combination of Broadcast-and-Ramp. Pondering the right way to handle this.

@steven-johnson
Copy link
Contributor

See #7556 for possible fix

@steven-johnson
Copy link
Contributor

LGTM. There are still issues to be resolved with the fuzzers but this change seems OK.

@steven-johnson
Copy link
Contributor

ready to land?

@nathaniel-brough
Copy link
Contributor Author

All good on my end!

@steven-johnson steven-johnson merged commit 6a98655 into halide:main May 22, 2023
ardier pushed a commit to ardier/Halide-mutation that referenced this pull request Mar 3, 2024
* fuzz: Add libfuzzer compatible bounds fuzzer

* Remove unused constant

* Style fix

* Fix handling of binary ops

* Handle casting to vector-of-bool properly

* fuzz: Alphabetically sort targets in CMake

---------

Co-authored-by: Steven Johnson <srj@google.com>
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