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

[Nikita Popov via llvm-commits]

================
@@ -233,58 +233,114 @@ class LLVM_LIBRARY_VISIBILITY InstCombiner {
                                                 PatternMatch::m_Value()));
   }
 
-  /// Return true if the specified value is free to invert (apply ~ to).
-  /// This happens in cases where the ~ can be eliminated.  If WillInvertAllUses
-  /// is true, work under the assumption that the caller intends to remove all
-  /// uses of V and only keep uses of ~V.
-  ///
-  /// See also: canFreelyInvertAllUsersOf()
-  static bool isFreeToInvert(Value *V, bool WillInvertAllUses,
-                             unsigned Depth = 0) {
+  /// Return nonnull value if V is free to invert (with condition) regarding
+  /// WillInvertAllUses.
+  /// If Builder is nonnull, it will return a simplified ~V
+  /// If Builder is null, it will return an arbitrary nonnull value (not
+  /// dereferenceable).
+  static Value *getFreelyInverted(Value *V, bool WillInvertAllUses,
+                                  BuilderTy *Builder, unsigned Depth = 0) {
     using namespace llvm::PatternMatch;
+    static Value *const NonNull = reinterpret_cast<Value *>(uintptr_t(1));
     // ~(~(X)) -> X.
-    if (match(V, m_Not(m_Value())))
-      return true;
+    Value *A, *B;
+    if (match(V, m_Not(m_Value(A))))
+      return A;
 
+    Constant *C;
     // Constants can be considered to be not'ed values.
-    if (match(V, m_AnyIntegralConstant()))
-      return true;
+    if (match(V, m_ImmConstant(C)))
+      return ConstantExpr::getNot(C);
 
     if (Depth++ >= MaxAnalysisRecursionDepth)
-      return false;
+      return nullptr;
 
     // The rest of the cases require that we invert all uses so don't bother
     // doing the analysis if we know we can't use the result.
     if (!WillInvertAllUses)
-      return false;
+      return nullptr;
 
     // Compares can be inverted if all of their uses are being modified to use
     // the ~V.
-    if (isa<CmpInst>(V))
-      return true;
+    if (auto *I = dyn_cast<CmpInst>(V)) {
+      if (Builder != nullptr)
+        return Builder->CreateCmp(I->getInversePredicate(), I->getOperand(0),
+                                  I->getOperand(1));
+      return NonNull;
+    }
 
-    Value *A, *B;
-    // If `V` is of the form `A + B` then `-1 - V` can be folded into
-    // `~B - A` or `~A - B` if we are willing to invert all of the uses.
-    if (match(V, m_Add(m_Value(A), m_Value(B))))
-      return isFreeToInvert(A, A->hasOneUse(), Depth) ||
-             isFreeToInvert(B, B->hasOneUse(), Depth);
+    // If `V` is of the form `A + Constant` then `-1 - V` can be folded into
+    // `(-1 - Constant) - A` if we are willing to invert all of the uses.
----------------
nikic wrote:

You've changed the comment here, but I think the new one is worse? It now uses `Constant` instead of `B`, even though the actual code has `B` and `B` is not required to be constant?

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