[PATCH] D114666: [InstSimplify] Simplify bool icmp with not in LHS

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

I don't think the logic is correct. You should start by creating a complete set of baseline tests based on the existing tests in:
https://github.com/llvm/llvm-project/blob/main/llvm/test/Transforms/InstSimplify/icmp-bool-constant.ll

We can then pre-commit those tests ahead of this patch to make sure we get correct results in all cases with this patch.

You may also want to check how these patterns are handled by "opt -instcombine" (assuming they are handled correctly there).

Finally, I'm worried about changing the values of LHS or Pred directly in this code because those values are used in other transforms later within `simplifyICmpOfBools`. We could be silently breaking those transforms if the test coverage for those is not good?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114666