lebedev.ri marked 2 inline comments as done. lebedev.ri added a comment. In D73380#1839603 <https://reviews.llvm.org/D73380#1839603>, @xbolva00 wrote:
> LLVM already infers noalias nonnull for eg. _Znwm so noalias and nonnull info > added by clang will not increase power of LLVM. Or? To be honest, i don't really believe it is LLVM place to deduce attributes on standard library functions like these, frontend should be doing that. Because as noted in patch description, if `noalias` is blindly "deduced" by LLVM, that is a miscompile if `-fno-assume-sane-operator-new` was specificed. > Alignment info is useful I think. That is the main motivation, yes. ================ Comment at: clang/test/CodeGenCXX/builtin-operator-new-delete.cpp:54 } -// CHECK: declare noalias i8* @_ZnwmSt11align_val_t(i64, i64) [[ATTR_NOBUILTIN:#[^ ]*]] +// CHECK: declare nonnull i8* @_ZnwmSt11align_val_t(i64, i64) [[ATTR_NOBUILTIN:#[^ ]*]] // CHECK: declare void @_ZdlPvSt11align_val_t(i8*, i64) [[ATTR_NOBUILTIN_NOUNWIND:#[^ ]*]] ---------------- xbolva00 wrote: > We lost noalias here? See two lines above, it migrated to the callsite. I'm going to argue that the previous behavior was erroneous: despite what `-fno-assume-sane-operator-new` is supposed to be doing, i **suspect** it will magically break with LTO. (we'd have `declare noalias nonnull i8* @_ZnwmSt11align_val_t(i64, i64)` in one TU and `declare nonnull i8* @_ZnwmSt11align_val_t(i64, i64)` in another - one with `noalias` and one without - how would they be merged? by dropping `noalias`, or keeping it?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73380/new/ https://reviews.llvm.org/D73380 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
