[stable/21.x][ObjCARC] Run ObjCARCContract before PreISelIntrinsicLowering#12687
[stable/21.x][ObjCARC] Run ObjCARCContract before PreISelIntrinsicLowering#12687SiliconA-Z wants to merge 1 commit intoswiftlang:stable/21.xfrom
Conversation
…ering (llvm#184149) Link: llvm@55322f2 74e4694 moved ObjCARCContract from running before the codegen pipeline into addISelPrepare(), which runs after PreISelIntrinsicLowering. This broke ObjCARCContract's retainRV-to-claimRV optimization because ObjCARCContract identifies ARC calls via intrinsics, not their lowered counterparts. This patch restores the pre-74e4694 ordering by moving ObjCARCContract to addISelPasses. The IntrinsicInst.cpp change looks extraneous but is required here: ObjCARCContract may now rewrite the bundle operand from retainRV to claimRV. When PreISelIntrinsicLowering then encounters this new intrinsic use, lowerObjCCall asserts mayLowerToFunctionCall. Assisted-by: claude rdar://137997453
|
@cachemeifyoucan Can we please do the @swift-ci please test I don't have perms |
|
@SiliconA-Z please do not ping many random people in a short period of time. |
|
@swift-ci test |
|
Thank you and I'm sorry! |
|
Passed! |
|
@citymarina @ahatanaka Can you confirm we want this in the stable branch? |
|
@SiliconA-Z could you let us know a bit about the motivation for this, is there a particular reason that the patch needs to go on this branch? Thanks! |
Yes: a core feature related to ARC optimization is being shipped broken, and waiting a whole year for that to be addressed come macOS 27 is not a good plan. Most bugs aren't like this and I'm fine waiting for, but also realize that there's low risk here, even if it feels like a big change for a 21.x release. |
|
It seems the Windows test hasn't ran (even though this doesn't affect Windows) |
|
@swift-ci test windows |
|
Yay it passed! |
3 participants
Link: llvm@55322f2
74e4694 moved ObjCARCContract from running before the codegen pipeline into addISelPrepare(), which runs after PreISelIntrinsicLowering.
This broke ObjCARCContract's retainRV-to-claimRV optimization because ObjCARCContract identifies ARC calls via intrinsics, not their lowered counterparts.
This patch restores the pre-74e4694 ordering by moving ObjCARCContract to addISelPasses.
The IntrinsicInst.cpp change looks extraneous but is required here: ObjCARCContract may now rewrite the bundle operand from retainRV to claimRV. When PreISelIntrinsicLowering then encounters this new intrinsic use, lowerObjCCall asserts mayLowerToFunctionCall.
Assisted-by: claude
rdar://137997453