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

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

In D53342#1639210 <https://reviews.llvm.org/D53342#1639210>, @xbolva00 wrote:

> I would like to hear @hfinkel’s opinion too :)


As I recall, there are two issues. One is: do we add the attributes based on the function names. The second is: What to do with the attributes that already exist in, e.g., some glibc header files.

My opinion is that:

1. We should definitely have this optimization capability
2. But we should have this after we filter out the annotations from memcpy/memset (etc.) in Clang (when the size is dynamic or zero). There should be a flag to turn off the filtering, but at least for now, it should be on by default.
3. We should have a function attribute reflecting the same filtering setting to appropriately control adding the attributes in the optimizer.

Yes, NULL pointers with memcpy/memset are UB from the standards perspective, but it's unclear to me that the value in exploiting this UB outweighs the potential problems introduced. While I generally agree with the viewpoint that says that people should use sanitizers, etc., and believe that they should in this case as well, because memcpy/memset are *so* common, because there's a wide-spread and reasonable disagreement as to whether this behavior should be undefined at all, and because there's no strong and broad case for performance improvements (unlike, for example, exploiting the lack of signed-integer overflow), I think that we should filter.


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

https://reviews.llvm.org/D53342