[llvm] [InstCombine] Improve handling of `not` and free inversion. (PR #66787)

[Nikita Popov via llvm-commits]

================
@@ -2205,12 +2223,22 @@ Instruction *InstCombinerImpl::visitSub(BinaryOperator &I) {
   // (~X) - (~Y) --> Y - X
   // This is placed after the other reassociations and explicitly excludes a
   // sub-of-sub pattern to avoid infinite looping.
-  if (isFreeToInvert(Op0, Op0->hasOneUse()) &&
-      isFreeToInvert(Op1, Op1->hasOneUse()) &&
-      !match(Op0, m_Sub(m_ImmConstant(), m_Value()))) {
-    Value *NotOp0 = Builder.CreateNot(Op0);
-    Value *NotOp1 = Builder.CreateNot(Op1);
-    return BinaryOperator::CreateSub(NotOp1, NotOp0);
+  {
+    // Need to ensure we can consume at least one of the `not` instructions,
+    // otherwise this can inf loop.
+    bool ConsumesOp0, ConsumesOp1;
+    if ((match(Op0, m_Add(m_Value(), m_ImmConstant())) &&
+         match(Op1, m_Add(m_Value(), m_ImmConstant()))) ||
----------------
nikic wrote:

Isn't this going to do the wrong thing (or maybe assert?) if Op0 or Op1 are adds with extra uses?

Do I understand correctly that this folds `(X + C1) - (Y + C2)` into `(~C1 - X) - (~C2 - Y)` and then some other fold is going to fold that into `(X + Y) + (C1 - C2)`? If so, that seems super convoluted, should we just have an explicit fold to do that instead?

https://github.com/llvm/llvm-project/pull/66787