Page MenuHomePhabricator

[ObjC][ARC] Don't add operand bundle `clang.arc.attachedcall` to a call if the call already has the operand bundle
Needs ReviewPublic

Authored by ahatanak on Mar 2 2021, 5:50 PM.

Details

Reviewers
rjmccall
Summary

This bug was causing the call to replaceAllUsesWith to crash because the old call instruction and the new call instruction were the same.

rdar://74957948

Diff Detail

Event Timeline

ahatanak created this revision.Mar 2 2021, 5:50 PM
ahatanak requested review of this revision.Mar 2 2021, 5:50 PM

We could emit a call to @llvm.objc.retain instead of the call to @llvm.objc.retainAutoreleasedReturnValue, but this doesn't seem to happen often and ARC optimizer can remove or replace the call anyway, so I'm just letting the front-end emit @llvm.objc.retainAutoreleasedReturnValue for simplicity.

ahatanak planned changes to this revision.Mar 3 2021, 11:31 AM

On second thought, I think we should emit a call to @llvm.objc.retain instead of a call to @llvm.objc.retainAutoreleasedReturnValue. I think we might want to remove ARC optimizer's code that converts calls to @llvm.objc.retainAutoreleasedReturnValue to calls to @llvm.objc.retain since it sometimes does the transformation when it shouldn't and we won't need that transformation once we switch to using the clang.arc.attachedcall operand bundle.

ahatanak updated this revision to Diff 327888.Mar 3 2021, 12:25 PM

Emit a call to @llvm.objc.retain instead of a call to @llvm.objc.retainAutoreleasedReturnValue.