[PATCH] D105284: Greedy set cover implementation of `Merger::Merge`
Kostya Serebryany via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 12 12:43:52 PDT 2021
kcc added a comment.
Thanks for the change!
Indeed, the current single-pass merge is far from perfect, and it's nice to see your numbers.
Some high-level notes:
- not sure about the flag syntax (-merge=2), I'd prefer something more descriptive. Maybe add an extra flag -set_cover_merge=1?
- The current single-pass algorithm is also greedy (just less so :)), so maybe use a different name to distinguish the new algorithm
- please try to follow the coding style e.g. with respect to {} around single-statement blocks.
- please add a unit test and a .lit test
I'll let Matt make another round of review.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105284/new/
https://reviews.llvm.org/D105284
More information about the llvm-commits
mailing list