[PATCH] D53342: [SimplifyLibCalls] Mark known arguments with nonnull

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

Some comments and questions.



================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:183
+  return isa<ConstantInt>(Arg) && cast<ConstantInt>(Arg)->getZExtValue() > 0;
+}
+
----------------
Any reason you do not use `isKnownNonZero` ?



================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:196
+  if (CI->getArgOperand(ArgNo)->getType()->getPointerAddressSpace())
+    return false;
+
----------------
I don't get this check. Can you explain it to me (potentially in a comment)?


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:255
 
+  setNonNullParam(CI, 0);
   // We don't do anything if length is not constant.
----------------
Why is non-null supposed to be "allowed" for the destination but not the source in case the length is 0?


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:416
+    dropNonNullParam(CI, 0);
+    dropNonNullParam(CI, 1);
     return nullptr;
----------------
It seems fragile to drop the non-null conditionally only. Shouldn't we always drop it and add it back if possible?


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:518
 
+  setNonNullParam(CI, 0);
   // See if we can get the length of the input string.
----------------
As above.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:535
+  } else {
+    dropNonNullParam(CI, 1);
     return nullptr;
----------------
As above.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:998
+  if (isIntrinsic) {
+    if (isPositiveSize(CI->getArgOperand(2))) {
+      setNonNullParam(CI, 0);
----------------
Here, and actually everywhere we currently look for a constant size, we could reuse the more powerful `isKnownNonZero` function.


https://reviews.llvm.org/D53342