[PATCH] D141188: [MergeICmps] Adapt to non-eq comparisons

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jan 7 01:11:17 PST 2023


nikic added a comment.

> Fix https://github.com/llvm/llvm-project/issues/59740.

This issue should remain open, because this only handles the ne case, not other predicates.



================
Comment at: llvm/lib/Transforms/Scalar/MergeICmps.cpp:658
+  auto *CmpI = cast<ICmpInst>(Phi.getIncomingValueForBlock(LastBlock));
+  ICmpInst::Predicate Predicate = CmpI->getPredicate();
+
----------------
We should remember the correct predicate earlier and pass it in here, rather than rederiving it.


================
Comment at: llvm/lib/Transforms/Scalar/MergeICmps.cpp:668
+    if (Predicate == ICmpInst::ICMP_NE)
+      IsEqual = Builder.CreateICmpNE(LhsLoad, RhsLoad);
+    else
----------------
`Builder.CreateICmp(Predicate, LhsLoad, RhsLoad)`


================
Comment at: llvm/test/Transforms/MergeICmps/X86/pr59740.ll:48
+  ret i1 %cmp
+}
----------------
Missing negative test coverage, e.g. wrong constants in phi.


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

https://reviews.llvm.org/D141188



More information about the llvm-commits mailing list