[PATCH] D96949: [FuncAttrs] Add willreturn to readonly callsites in mustprogress fns.

Thu Feb 18 09:15:28 PST 2021

fhahn updated this revision to Diff 324674.
fhahn added a comment.

addressed comments.

In D96949#2571913 <https://reviews.llvm.org/D96949#2571913>, @jeroen.dobbelaere wrote:

> In D96949#2571873 <https://reviews.llvm.org/D96949#2571873>, @nikic wrote:
>
>> After D96960 <https://reviews.llvm.org/D96960> I don't think this change is really necessary anymore (at least it does not need an LLVM 12 backport). If the language has forward progress, then all function definitions will get annotated anyway, so only extern declarations are relevant. However, those will not be readyonly either -- unless they are explicitly annotated such, in which case they will also be annotated as willreturn after D96960 <https://reviews.llvm.org/D96960>.
>
> I am not sure if this is completely true. When looking at the test changes for D96960 <https://reviews.llvm.org/D96960>, I have the impression that a lot of __builtin_xxx functions might also need the 'willreturn' attribute. (and for 'C', it should  be clang that adds the attribute).

For C++ only, D96960 <https://reviews.llvm.org/D96960> is probably sufficient.

But I think it should still be helpful when doing LTO with mixed C/C++ code (a C function definition can be determined to be readonly and will be willreturn when called in a C++ function). And there are other languages with different ways of marking functions as readonly.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96949

Files:
  llvm/include/llvm/IR/InstrTypes.h
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/test/Transforms/FunctionAttrs/willreturn-callsites.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D96949.324674.patch
Type: text/x-patch
Size: 6284 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210218/ad975ec4/attachment.bin>