[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