[PATCH] D63604: [Attributor] Deduce "nonnull" on return value

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

Please add tests that do pointer arithmetic, e.g., `return a + 1`.



================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:676
+
+  /// Return true if we know that underlying value is noalias.
+  virtual bool isKnownNonNull() const = 0;
----------------
copy and past comment, also above


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:719
+    if (Constant *C = dyn_cast<Constant>(&RV)) {
+      if (C->isNullValue() || isa<UndefValue>(C))
+        return false;
----------------
Undef values are fine. `undef` basically allows you to "chose" a value, so we can "chose" something other than `null`.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:724
+    }
+
+    // When the return value is an argument, deduce nonnull if it is
----------------
You should (also) ask for the `AANonNull` attribute for the value and if you get an abstract attribute that says `nonnull` we are fine. That should subsume some of the code below and also trigger ones we have `AANonNullArgument`.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:731
+    // Otherwise, for now, we can only deduce nonnull if we have call sites.
+    // FIXME: add more support.
+    ImmutableCallSite ICS(&RV);
----------------
You should ask value tracking via `isKnownNonZero` here as well. It will need to be improved still to use optimistic information but it is better than nothing.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:743
+    return true;
+  };
+  if (!AARetValImpl->checkForallReturnedValues(Pred)) {
----------------
The predicate above will be reused later on. Maybe "generate" it in a function in `AANonNullImpl` for a given `Attributor`.


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

https://reviews.llvm.org/D63604