[PATCH] D63672: Added Delta IR Reduction Tool

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 2 16:00:02 PDT 2019


dblaikie added inline comments.


================
Comment at: llvm/tools/llvm-reduce/deltas/Chunk.h:27-30
+  /// Operator used for set<Chunk>
+  friend bool operator<(const Chunk &C1, const Chunk &C2) {
+    return C1.end < C2.begin;
+  }
----------------
diegotf wrote:
> dblaikie wrote:
> > Is this comparison sufficient for std::set (strict weak ordering - specifically I'd be worried about !({1, 4} < {3, 6}) && !({3, 6} < {1, 4}) - so perhaps this comparison should have an assertion that the two ranges are non-overlapping?) 
> I actually thought about that case but since I'm just using sets for lookup (and dont really care about the order) I left the comparison as is, would you still recommend I add the assertion? 
The issue would be that if the ordering requirement is violated, it could break even a lookup - in the example I gave above, those two objects would be considered "equivalent", for instance.

If the Chunks are necessarily non-overlapping, perhaps it'd be simpler/accurate to only compare begin to begin (rather than begin to end)? (I'd do that and/or add the assertion)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63672/new/

https://reviews.llvm.org/D63672





More information about the llvm-commits mailing list