[PATCH] D85092: [InstSimplify] Peephole optimization for icmp (urem X, Y), X

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:2967
 
+  // icmp pred (urem Y, X), Y
+  if (LBO && match(LBO, m_URem(m_Specific(RHS), m_Value()))) {
----------------
xldenis wrote:
> nikic wrote:
> > lebedev.ri wrote:
> > > This appears correct, but you also need to handle ` icmp pred Y, (urem Y, X)` case.
> > > I know that naively one would expect it to be canonicalized by complexity sorting,
> > > and i think said complexity sorting is a subtle evil.
> > I've applied some cleanup in https://github.com/llvm/llvm-project/commit/a0addbb4ec8c7bf791139699d46b08413c46eed7 to only explicitly handle the binop on the LHS and treat the other side via swapped predicate.
> ah I just added the mirrored version of the opt, should I remove it then? I'll leave the test cases.
You need to rebase the patch, moving a single variant of the fold into `simplifyICmpWithBinOpOnLHS()`


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

https://reviews.llvm.org/D85092