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

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 5 12:37:16 PDT 2019


jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:687
+  /// (ii) A value is associated with AANonNull and its isKnownNonNull() is
+  /// true.
+  std::function<bool(Value &)> generatePredicate(Attributor &);
----------------
Why isn't `isAssumedNonNull()` sufficient here?


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:740
+  std::function<bool(Value &)> Pred = [&](Value &RV) -> bool {
+
+    if (isKnownNonZero(&RV, getAnchorScope().getParent()->getDataLayout()))
----------------
Please add here something like:

```
FIXME: The `AAReturnedValues` should provide the predicate with the `ReturnInst` vector as well such that we can use the control flow sensitive version of `isKnownNonZero`. This should fix `test11` in `test/Transforms/FunctionAttrs/nonnull.ll`
```


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:764
+  indicatePessimisticFixpoint();
+  return ChangeStatus::UNCHANGED;
+}
----------------
CHANGED


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:770
+
+  auto *AARetValImpl = A.getAAFor<AAReturnedValuesImpl>(*this, F);
+  if (!AARetValImpl) {
----------------
I don't think you need the "Impl" part.


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

https://reviews.llvm.org/D63604





More information about the llvm-commits mailing list