> On Jan 30, 2018, at 4:32 PM, Saleem Abdulrasool <compn...@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 <ahatan...@apple.com > <mailto:ahatan...@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@lists.llvm.org <mailto:cfe-commits@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@lists.llvm.org <mailto:cfe-commits@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
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
