Why was PR #67155 (Use clang.arc.attachedcall) reverted?

Hi folks,

I was investigating a runtime crash for an iOS app and found out the root cause seems to be Swift compiler not emitting clang.arc.attachedcall for objc_retainAutoreleasedReturnValue() like what Clang does. By blaming the commit history I found that it was once implemented but soon reverted.

Original PR: https://github.com/swiftlang/swift/pull/67155

Revert PR: https://github.com/swiftlang/swift/pull/68095

The revert pull request only mentions the original PR and a radar link. I was wondering if anyone could provide some context or background on why this change was backed out? Was there a regression or an issue found?

It looks like there was some circumstance that we discovered under which the intrinsic couldn't be used. @natechan did the revert and might know more; I think @Arnold was also involved.

I have commented on the revert PR: Revert "Use clang.arc.attachedcall for emission of objc_retainAutoreleasedReturnValue" by nate-chandler · Pull Request #68095 · swiftlang/swift · GitHub

The reason why this was reverted is because the IR that gets generated relies on an LLVM pass to be run so that an intrinsic is properly lowered (otherwise you get a compiler crash because later stages of the LLVM pipeline don't know about the intrinsic).

The pass that does that is ObjcARCContract. Apparently, if a function is marked with the optnone llvm attribute this pass does not get run. So to reenable the reverted PR one would have to fix this issue.

The fix is likely to mark the ObjCARCContract pass "required".

https://llvm.org/docs/WritingAnLLVMNewPMPass.html#required-passes

1 Like