[PATCH] D105284: Greedy set cover implementation of `Merger::Merge`
Aristotelis Koutsouridis via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 30 06:35:51 PDT 2021
arisKoutsou added inline comments.
================
Comment at: compiler-rt/lib/fuzzer/FuzzerMerge.cpp:358
+ if (MaxNumFeatures == 0)
+ continue;
+
----------------
morehouse wrote:
> Could this continue cause an infinite loop? i.e. when `Remaining` is empty
It wouldn't cause an infinite loop when Remaining is empty because the `while` condition `(CoveredSize != AllFeatures.size())` would be false. On the other hand, I noticed that there would be a problem if a feature had a value greater than `1 << 21`, which is the size of the bitvector. In that case, there could be an infinite loop because the `while` condition would never become false. I am addressing this problem in the latest patch.
================
Comment at: compiler-rt/lib/fuzzer/tests/FuzzerUnittest.cpp:953
+ "",
+ true));
+ EXPECT_EQ(M.SetCoverMerge(Features, &NewFeatures, Cov, &NewCov, &NewFiles),
----------------
morehouse wrote:
> IIUC, this test doesn't quite do what's intended.
>
> We choose input C because it has the most unique features, but after that A only has `{0}` as a unique feature while B has `{4, 5}`. So we do in fact choose B, but not because it is smaller.
Correct, I will adjust the features correctly so that we can test breaking ties between files with equal number of features.
================
Comment at: compiler-rt/test/fuzzer/set_cover_merge.test:48
+CHECK_OVERLAP: MERGE-OUTER: 10 files, 1 in the initial corpus
+CHECK_OVERLAP: MERGE-OUTER: 2 new files with 6 new features added
+# Make sure that we are prefering smaller files (T3/3 over T3/a).
----------------
morehouse wrote:
> I think we would get the same results with `-merge`. Perhaps we should make some feature-poor inputs smaller so that `-merge` would pick those first, while `-set_cover_merge` picks the feature-rich ones.
Should I also include a test of `-merge=1` in this source file to highlight the difference in behavior?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105284/new/
https://reviews.llvm.org/D105284
More information about the llvm-commits
mailing list