[PATCH] D63349: [EarlyCSE] Fix hashing of self-compares

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 14 12:02:33 PDT 2019


nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/lib/Transforms/Scalar/EarlyCSE.cpp:185
+    CmpInst::Predicate SwappedPred = CI->getSwappedPredicate();
+    if (std::tie(LHS, Pred) > std::tie(RHS, SwappedPred)) {
       std::swap(LHS, RHS);
----------------
JosephTremoulet wrote:
> nikic wrote:
> > Would it be possible to make the condition just `Pred > SwappedPred`, without ordering by LHS/RHS at all?
> No, some predicates (eq, ne) are their own swap duals, so we need to check the operands to handle e.g. `icmp eq A, B` vs `icmp eq B, A`.
Right you are! In that case, I'm wondering if we're missing the operand swap for equality/inequality for the select case below? We canonicalize by inverse predicate, but I don't think there's anything handling swapping of equality comparison arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63349





More information about the llvm-commits mailing list