[PATCH] D66079: [SimplifyLibCalls] Add dereferenceable bytes from known callsites [WIP]

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 12 16:36:55 PDT 2019


jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

One minor request, a few nits, LGTM.



================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:193
+  for (unsigned ArgNo : ArgNos) {
+    if (CI->getDereferenceableBytes(ArgNo + 1) < DerefBytes) {
+      CI->removeParamAttr(ArgNo, Attribute::Dereferenceable);
----------------
xbolva00 wrote:
> if (CI->getDereferenceableBytes(**ArgNo + 1**) < DerefBytes) {
> 
> quite surprising :) spent some time on find this :D
My bad, I added `AttributeList::FirstArgIndex` (`= 1`) below but forgot it above.
Can you replace `1` with `AttributeList::FirstArgIndex` please.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:784
+    if (LenC->isZero())
+      return Constant::getNullValue(CI->getType());
+  }
----------------
Maybe call `annotateDereferenceableBytes` after the zero check.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:949
       return Res;
+  }
 
----------------
Again, I guess if the inner condition hits the annotations are lost (are they?). We could annotate afterwards.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:987
+  B.CreateMemCpy(Op0, 1, Op1, 1, Size);
+  return Op0;
 }
----------------
I don't mind but you don't even need the `Op0` .. changes anymore.


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

https://reviews.llvm.org/D66079





More information about the llvm-commits mailing list