r294872 - CodeGen: annotate ObjC ARC functions with ABI constraints
Akira Hatanaka via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 31 09:15:54 PST 2018
> On Jan 30, 2018, at 4:32 PM, Saleem Abdulrasool <compnerd at compnerd.org> wrote:
>
> Thanks for the note here. I’ll try to take a look at that, but, yes, the frontend change itself is correct. I’ve seen many, many places where the ObjCARC passes break down in the backend, and I’ve filed a few bugs on it in the hope that someone from Apple would take a look at some point. I’m happy to work on it if someone could help with some of the issues that crop up.
>
Sure, I can help you.
> Instruction bundles would only work partially and not really solve the problem. The issue is that even the Post RA scheduler can break apart call sequences, reordering things. The Machine Constant Island Pass is another place this breaks down, as it can create water in between the sequence. The MI passes tend to expect the bundles to have been broken up, and we need something which is kept all the way through the MC level.
>
Yes, that’s true, instruction bundles work only when the code is in LLVM IR. Maybe we can use MachineInstr bundles to prevent backend passes from inserting instructions between the calls?
> On Tue, Jan 30, 2018 at 12:38 PM Akira Hatanaka <ahatanaka at apple.com <mailto:ahatanaka at apple.com>> wrote:
> Hi Saleem,
>
> I had to revert this patch since this patch caused crashes in code that was working fine before.
>
> As I mentioned in the commit message, I believe this patch is correct, but it causes the objc_retainAutoreleasedReturnValue/objc_autoreleaseReturnValue handshake to fail in some cases because middle-end and backend passes insert instructions between the call to the function returning an object and the call to objc_retainAutoreleasedReturnValue. We probably should find a way to prevent inserting instructions between the calls (maybe using instruction bundles), but we haven’t had the time to implement the fix. Feel free to fix the bug if you’d like to do so.
>
> > On Feb 11, 2017, at 1:34 PM, Saleem Abdulrasool via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
> >
> > Author: compnerd
> > Date: Sat Feb 11 15:34:18 2017
> > New Revision: 294872
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=294872&view=rev <http://llvm.org/viewvc/llvm-project?rev=294872&view=rev>
> > Log:
> > CodeGen: annotate ObjC ARC functions with ABI constraints
> >
> > Certain ARC runtime functions have an ABI contract of being forwarding.
> > Annotate the functions with the appropriate `returned` attribute on the
> > arguments. This hoists some of the runtime ABI contract information
> > into the frontend rather than the backend transformations.
> >
> > The test adjustments are to mark the returned function parameter as
> > such. The minor change to the IR output is due to the fact that the
> > returned reference of the object causes it to extend the lifetime of the
> > object by returning an autoreleased return value. The result is that
> > the explicit objc_autorelease call is no longer formed, as autorelease
> > elision is now possible on the return.
> >
> > Modified:
> > cfe/trunk/lib/CodeGen/CGObjC.cpp
> > cfe/trunk/test/CodeGenObjC/arc.m
> >
> > Modified: cfe/trunk/lib/CodeGen/CGObjC.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?rev=294872&r1=294871&r2=294872&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?rev=294872&r1=294871&r2=294872&view=diff>
> > ==============================================================================
> > --- cfe/trunk/lib/CodeGen/CGObjC.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/CGObjC.cpp Sat Feb 11 15:34:18 2017
> > @@ -1802,6 +1802,22 @@ void CodeGenFunction::EmitARCIntrinsicUs
> > }
> >
> >
> > +static bool IsForwarding(StringRef Name) {
> > + return llvm::StringSwitch<bool>(Name)
> > + .Cases("objc_autoreleaseReturnValue", // ARCInstKind::AutoreleaseRV
> > + "objc_autorelease", // ARCInstKind::Autorelease
> > + "objc_retainAutoreleaseReturnValue", // ARCInstKind::FusedRetainAutoreleaseRV
> > + "objc_retainAutoreleasedReturnValue", // ARCInstKind::RetainRV
> > + "objc_retainAutorelease", // ARCInstKind::FusedRetainAutorelease
> > + "objc_retainedObject", // ARCInstKind::NoopCast
> > + "objc_retain", // ARCInstKind::Retain
> > + "objc_unretainedObject", // ARCInstKind::NoopCast
> > + "objc_unretainedPointer", // ARCInstKind::NoopCast
> > + "objc_unsafeClaimAutoreleasedReturnValue", // ARCInstKind::ClaimRV
> > + true)
> > + .Default(false);
> > +}
> > +
> > static llvm::Constant *createARCRuntimeFunction(CodeGenModule &CGM,
> > llvm::FunctionType *FTy,
> > StringRef Name) {
> > @@ -1819,6 +1835,13 @@ static llvm::Constant *createARCRuntimeF
> > // performance.
> > F->addFnAttr(llvm::Attribute::NonLazyBind);
> > }
> > +
> > + if (IsForwarding(Name)) {
> > + llvm::AttrBuilder B;
> > + B.addAttribute(llvm::Attribute::Returned);
> > +
> > + F->arg_begin()->addAttr(llvm::AttributeSet::get(F->getContext(), 1, B));
> > + }
> > }
> >
> > return RTF;
> >
> > Modified: cfe/trunk/test/CodeGenObjC/arc.m
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/arc.m?rev=294872&r1=294871&r2=294872&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/arc.m?rev=294872&r1=294871&r2=294872&view=diff>
> > ==============================================================================
> > --- cfe/trunk/test/CodeGenObjC/arc.m (original)
> > +++ cfe/trunk/test/CodeGenObjC/arc.m Sat Feb 11 15:34:18 2017
> > @@ -7,30 +7,30 @@
> > // RUN: %clang_cc1 -fobjc-runtime=macosx-10.7.0 -triple x86_64-apple-darwin11 -Wno-objc-root-class -Wno-incompatible-pointer-types -Wno-arc-unsafe-retained-assign -emit-llvm -fblocks -fobjc-arc -fobjc-runtime-has-weak -o - %s | FileCheck -check-prefix=ARC-NATIVE %s
> >
> > // ARC-ALIEN: declare extern_weak void @objc_storeStrong(i8**, i8*)
> > -// ARC-ALIEN: declare extern_weak i8* @objc_retain(i8*)
> > -// ARC-ALIEN: declare extern_weak i8* @objc_autoreleaseReturnValue(i8*)
> > +// ARC-ALIEN: declare extern_weak i8* @objc_retain(i8* returned)
> > +// ARC-ALIEN: declare extern_weak i8* @objc_autoreleaseReturnValue(i8* returned)
> > // ARC-ALIEN: declare i8* @objc_msgSend(i8*, i8*, ...) [[NLB:#[0-9]+]]
> > // ARC-ALIEN: declare extern_weak void @objc_release(i8*)
> > -// ARC-ALIEN: declare extern_weak i8* @objc_retainAutoreleasedReturnValue(i8*)
> > +// ARC-ALIEN: declare extern_weak i8* @objc_retainAutoreleasedReturnValue(i8* returned)
> > // ARC-ALIEN: declare extern_weak i8* @objc_initWeak(i8**, i8*)
> > // ARC-ALIEN: declare extern_weak i8* @objc_storeWeak(i8**, i8*)
> > // ARC-ALIEN: declare extern_weak i8* @objc_loadWeakRetained(i8**)
> > // ARC-ALIEN: declare extern_weak void @objc_destroyWeak(i8**)
> > -// ARC-ALIEN: declare extern_weak i8* @objc_autorelease(i8*)
> > -// ARC-ALIEN: declare extern_weak i8* @objc_retainAutorelease(i8*)
> > +// declare extern_weak i8* @objc_autorelease(i8*)
> > +// ARC-ALIEN: declare extern_weak i8* @objc_retainAutorelease(i8* returned)
> >
> > // ARC-NATIVE: declare void @objc_storeStrong(i8**, i8*)
> > -// ARC-NATIVE: declare i8* @objc_retain(i8*) [[NLB:#[0-9]+]]
> > -// ARC-NATIVE: declare i8* @objc_autoreleaseReturnValue(i8*)
> > +// ARC-NATIVE: declare i8* @objc_retain(i8* returned) [[NLB:#[0-9]+]]
> > +// ARC-NATIVE: declare i8* @objc_autoreleaseReturnValue(i8* returned)
> > // ARC-NATIVE: declare i8* @objc_msgSend(i8*, i8*, ...) [[NLB]]
> > // ARC-NATIVE: declare void @objc_release(i8*) [[NLB]]
> > -// ARC-NATIVE: declare i8* @objc_retainAutoreleasedReturnValue(i8*)
> > +// ARC-NATIVE: declare i8* @objc_retainAutoreleasedReturnValue(i8* returned)
> > // ARC-NATIVE: declare i8* @objc_initWeak(i8**, i8*)
> > // ARC-NATIVE: declare i8* @objc_storeWeak(i8**, i8*)
> > // ARC-NATIVE: declare i8* @objc_loadWeakRetained(i8**)
> > // ARC-NATIVE: declare void @objc_destroyWeak(i8**)
> > -// ARC-NATIVE: declare i8* @objc_autorelease(i8*)
> > -// ARC-NATIVE: declare i8* @objc_retainAutorelease(i8*)
> > +// declare i8* @objc_autorelease(i8*)
> > +// ARC-NATIVE: declare i8* @objc_retainAutorelease(i8* returned)
> >
> > // CHECK-LABEL: define void @test0
> > void test0(id x) {
> > @@ -1504,7 +1504,9 @@ void test68(void) {
> > // CHECK: [[SELF:%.*]] = alloca [[TEST69:%.*]]*, align 8
> > // CHECK: [[T0:%.*]] = load [[TEST69]]*, [[TEST69]]** [[SELF]], align 8
> > // CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST69]]* [[T0]] to i8*
> > -// CHECK-NEXT: ret i8* [[T1]]
> > +// CHECK-NEXT: [[RETAIN:%.*]] = call i8* @objc_retain(i8* [[T1]])
> > +// CHECK-NEXT: [[AUTORELEASE:%.*]] = tail call i8* @objc_autoreleaseReturnValue(i8* [[RETAIN]])
> > +// CHECK-NEXT: ret i8* [[AUTORELEASE]]
> >
> > // rdar://problem/10907547
> > void test70(id i) {
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
>
> --
> Saleem Abdulrasool
> compnerd (at) compnerd (dot) org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180131/41279b4a/attachment-0001.html>
More information about the cfe-commits
mailing list