[PATCH] D76140: [InlineFunction] update attributes during inlining

This revision was automatically updated to reflect the committed changes. Closed by commit rGbf7a16a76871: [InlineFunction] Update valid return attributes at callsite within callee body (authored by anna). Changed prior to commit: https://reviews.llvm.org/D76140?vs=254222=254573#toc

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna added a comment. In D76140#1957416 , @reames wrote: > LGTM again, with minor change. will update it. > p.s. Sorry for missing the functional issue the first time. All of the test > changes should have made the issue obvious, but despite reading

[PATCH] D76140: [InlineFunction] update attributes during inlining

reames accepted this revision. reames added a comment. This revision is now accepted and ready to land. LGTM again, with minor change. p.s. Sorry for missing the functional issue the first time. All of the test changes should have made the issue obvious, but despite reading the LangRef

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna updated this revision to Diff 254222. anna added a comment. fixed missing code left out during rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76140/new/ https://reviews.llvm.org/D76140 Files:

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna requested review of this revision. anna added a comment. fixed buildbot failure. see above comments and added testcase `test8`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76140/new/ https://reviews.llvm.org/D76140

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna updated this revision to Diff 254213. anna added a comment. This revision is now accepted and ready to land. whitelist valid return attributes and only add those. Added testcase for signext. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna reopened this revision. anna added a comment. This revision is now accepted and ready to land. In D76140#1953201 , @anna wrote: > I got a failure in one of the binaryFormats: > lib/BinaryFormat/CMakeFiles/LLVMBinaryFormat.dir/MsgPackReader.cpp > >

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna marked an inline comment as done. anna added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1175 + continue; +// Sanity check that the cloned return instruction exists and is a return +// instruction itself. anna

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna planned changes to this revision. anna added a comment. see above comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76140/new/ https://reviews.llvm.org/D76140 ___ cfe-commits mailing list

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna added a comment. I got a failure in one of the binaryFormats: lib/BinaryFormat/CMakeFiles/LLVMBinaryFormat.dir/MsgPackReader.cpp Attributes 'zeroext and signext' are incompatible! %rev.i.i.i.i.i.i.i.i = tail call signext zeroext i16 @llvm.bswap.i16(i16 %ret.0.copyload.i.i.i.i) #6

[PATCH] D76140: [InlineFunction] update attributes during inlining

This revision was automatically updated to reflect the committed changes. Closed by commit rG28518d9ae39f: [InlineFunction] Handle return attributes on call within inlined body (authored by anna). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna marked 3 inline comments as done. anna added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1175 + continue; +// Sanity check that the cloned return instruction exists and is a return +// instruction itself. anna

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna updated this revision to Diff 253704. anna added a comment. addressed review comments. Added two test cases: deref value being different, inlined callee body better optimized compared to callee. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna marked 3 inline comments as done. anna added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1159 + + auto MayContainThrowingOrExitingCall = [&](Instruction *RVal, + Instruction *RInst) {

[PATCH] D76140: [InlineFunction] update attributes during inlining

reames accepted this revision. reames added a comment. This revision is now accepted and ready to land. LGTM, but with two specific required follow ups. If you're not comfortable committing to both, please don't land this one. Comment at:

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna updated this revision to Diff 252868. anna added a comment. NFC w.r.t prev diff. Use VMap.lookup instead of a lambda which does the same. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76140/new/ https://reviews.llvm.org/D76140 Files:

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna added a comment. ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76140/new/ https://reviews.llvm.org/D76140 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna marked an inline comment as done. anna added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1172 + if (NumInstChecked++ > MaxInstCheckedForThrow || + isGuaranteedToTransferExecutionToSuccessor()) +return true;

[PATCH] D76140: [InlineFunction] update attributes during inlining

lebedev.ri added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1172 + if (NumInstChecked++ > MaxInstCheckedForThrow || + isGuaranteedToTransferExecutionToSuccessor()) +return true; anna wrote: > lebedev.ri wrote:

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna marked an inline comment as done. anna added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1172 + if (NumInstChecked++ > MaxInstCheckedForThrow || + isGuaranteedToTransferExecutionToSuccessor()) +return true;

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna updated this revision to Diff 251901. anna added a comment. Noticed while adding couple more tests, there were 2 bugs: 1 the isGuaranteedToTransferExecutionToSuccessor check should be inverted 2. make_range should be until the return instruction - so we do not want std::prev on the

[PATCH] D76140: [InlineFunction] update attributes during inlining

lebedev.ri added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1172 + if (NumInstChecked++ > MaxInstCheckedForThrow || + isGuaranteedToTransferExecutionToSuccessor()) +return true; anna wrote: > Noticed while

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna marked an inline comment as done. anna added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1172 + if (NumInstChecked++ > MaxInstCheckedForThrow || + isGuaranteedToTransferExecutionToSuccessor()) +return true;

[PATCH] D76140: [InlineFunction] update attributes during inlining

jdoerfert accepted this revision. jdoerfert added a comment. I'm unsure about the `zeroext` and `signext` on the call sites now but other than that I think this is good. wait for @reames OK though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna updated this revision to Diff 251615. anna added a comment. use isGuaranteedToTransferExecutionToSuccessor instead of `MayThrow` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76140/new/ https://reviews.llvm.org/D76140 Files:

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna marked an inline comment as done. anna added inline comments. Herald added a reviewer: aartbik. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1172 +return true; +return false; + }; jdoerfert wrote: > `mayThrow` is not sufficient. As

[PATCH] D76140: [InlineFunction] update attributes during inlining

jdoerfert added inline comments. Comment at: llvm/lib/Transforms/Utils/InlineFunction.cpp:1172 +return true; +return false; + }; `mayThrow` is not sufficient. As with my earlier example, a potential `exit` is sufficient to break this, thus you need

[PATCH] D76140: [InlineFunction] update attributes during inlining

anna updated this revision to Diff 251447. anna added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. fixed clang tests. rot-intrinsics.c testcase has 5 different RUNs with 3 prefixes. Depending on target-triple, the attribute is added to the caller, so I've