FunctionSignatureOptimization: fix a miscompile in owned->guaranteed return value specialization #31581
Conversation
…return value specialization FSO can handle self-recursive calls. But this only works if the result of the self-recursive call is actually returned and not used otherwise. The check for this was missing. https://bugs.swift.org/browse/SR-12677 rdar://problem/62895040
|
@swift-ci test |
|
Build failed |
|
@swift-ci test macOS |
|
Looking at this now. Some of this code is interesting. @eeckstein are you sure we don't need to do the same thing on line 172? I am still digesting the code (I haven't read it in a while). |
|
@eeckstein nm. I looked in more detail. |
|
@gottesmm line 172 is just a conservative bail-out check. So I don't think we need this for functional correctness here |
| @@ -180,7 +180,7 @@ bb3: | |||
| } | |||
|
|
|||
| // CHECK-LABEL: START: epilogue_retains_in_diamond_top_and_left_split_block | |||
| // CHECK: apply | |||
| // CHECK-NOT: apply | |||
There was a problem hiding this comment.
Can you add in a follow on commit a test to this file of this form that still has the old behavior if we don't currently have a test. I didn't check.
| if (auto *AI = dyn_cast<ApplyInst>(II)) | ||
| if (AI->getCalleeFunction() == II->getParent()->getParent()) | ||
| return true; | ||
| if (auto *AI = dyn_cast<ApplyInst>(II)) { |
There was a problem hiding this comment.
LGTM. Can you in a follow on commit add a comment here reminding the user that arg is going to be a return value of the function since we are performing retain motion. I was confused at first b/c of getArg.
FSO can handle self-recursive calls.
But this only works if the result of the self-recursive call is actually returned and not used otherwise.
The check for this was missing.
https://bugs.swift.org/browse/SR-12677
rdar://problem/62895040