r294872 - CodeGen: annotate ObjC ARC functions with ABI constraints
Saleem Abdulrasool via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 30 16:32:40 PST 2018
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.
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.
On Tue, Jan 30, 2018 at 12:38 PM Akira Hatanaka <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> 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
> > 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
> >
> ==============================================================================
> > --- 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
> >
> ==============================================================================
> > --- 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
> > 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/cdf10d16/attachment-0001.html>
More information about the cfe-commits
mailing list