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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:682
+  /// Generate a function to check whether the value is assumed as nonnull.
+  std::function<bool(Value &)> generatePredicate(Attributor &);
+};
----------------
Explain the behavior of the generate better, what does it return, true for nonnull?


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:727
+    else
+      indicatePessimisticFixpoint();
+  }
----------------
Why would you indicate a fixpoint here? If there is no attribute present in the IR we can still deduce it.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:738
+      return !C->isNullValue();
+    }
+
----------------
No braces around a single statement


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:759
+      return false;
+    }
+    return true;
----------------
Why do we need to duplicate the `A.getAAFor ... ` code and the ceck afterwards?
Couldn't we just ask for ` auto *NonNullAA = A.getAAFor<AANonNull>(*this, RV);` regardless whether it is an argument, call, or something else?


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:767
+ChangeStatus AANonNullArgument::updateImpl(Attributor &A) {
+  // TODO: Currently, we have no deduction for argument nonnull attribute.
+  return ChangeStatus::UNCHANGED;
----------------
In that case, move the "indicatePessimisticFixpoint" from the init method here. Than one can at least see what is going on.


================
Comment at: llvm/test/Transforms/FunctionAttrs/nonnull.ll:107
+}
+
+
----------------
One more with inbounds (below, untested!) and maybe some without inbounds while we are at it.
```
; CHECK: define i8* @test10
; FIXME: missing nonnull
; ATTRIBUTOR: define i8* @test10
define i8* @test10(i8* %a, i64 %n) {
  %cmp = icmp ne i64 %n, null 
  call void @llvm.assume(i1 %cmp)
  %b = getelementptr inbounds i8, i8* %a, i64 %n
  ret i8* %b
}
```


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

https://reviews.llvm.org/D63604