[PATCH] D24523: [ObjCARC] Traverse chain downwards to replace uses of argument passed to ObjC library call with call return

[Akira Hatanaka via llvm-commits]

Thanks Pete. Committed in r281419 and r281421.

On Tue, Sep 13, 2016 at 3:29 PM, Pete Cooper <peter_cooper at apple.com> wrote:

> Hi Akira
>
> Just a single nit.  Can you please define *OrigArg on its own line as its
> easier to read.
>
> Otherwise LGTM.  I’m surprised we didn’t come across this issue sooner!
>
> Cheers,
> Pete
> > On Sep 13, 2016, at 2:55 PM, Akira Hatanaka <ahatanak at gmail.com> wrote:
> >
> > ahatanak created this revision.
> > ahatanak added reviewers: gottesmm, pete, rjmccall.
> > ahatanak added a subscriber: llvm-commits.
> >
> > ObjCARCContract::runOnFunction tries to replace uses of an argument
> passed to an objective-c library call with the call
> > return value. For example, in the following IR, it replaces uses of
> argument %9 and uses of the values discovered traversing the chain upwards
> (%7 and %8) with the call return %10, if they are dominated by the call to
> @objc_autoreleaseReturnValue. This transformation enables code-gen to
> tail-call the call to @objc_autoreleaseReturnValue, which is necessary to
> enable auto release return value optimization.
> >
> > ```
> > %7 = tail call i8* @objc_loadWeakRetained(i8** %6)
> > %8 = bitcast i8* %7 to %0*
> > %9 = bitcast %0* %8 to i8*
> > %10 = tail call i8* @objc_autoreleaseReturnValue(i8* %9)
> > ret %0* %8
> > ```
> >
> > Since r276727, llvm started removing redundant bitcasts and as a result
> started feeding the following IR to ARC contraction:
> >
> > ```
> > %7 = tail call i8* @objc_loadWeakRetained(i8** %6)
> > %8 = bitcast i8* %7 to %0*
> > %9 = tail call i8* @objc_autoreleaseReturnValue(i8* %7)
> > ret %0* %8
> > ```
> >
> > ARC contraction no longer does the optimization described above since it
> only traverses the chain upwards and fails to recognize that the function
> return can be replaced by the call return from
> @objc_autoreleaseReturnValue. This patch changes
> ObjCARCContract::runOnFunction to traverse the chain downwards too and
> replace uses of bitcasts with the call return.
> >
> > rdar://problem/28011339
> >
> > https://reviews.llvm.org/D24523
> >
> > Files:
> >  lib/Transforms/ObjCARC/ObjCARCContract.cpp
> >  test/Transforms/ObjCARC/contract-replace-arg-use.ll
> >
> > Index: test/Transforms/ObjCARC/contract-replace-arg-use.ll
> > ===================================================================
> > --- /dev/null
> > +++ test/Transforms/ObjCARC/contract-replace-arg-use.ll
> > @@ -0,0 +1,18 @@
> > +; RUN: opt -objc-arc-contract -S < %s | FileCheck %s
> > +
> > +declare i8* @objc_autoreleaseReturnValue(i8*)
> > +declare i8* @foo1()
> > +
> > +; Check that ARC contraction replaces the function return with the value
> > +; returned by @objc_autoreleaseReturnValue.
> > +
> > +; CHECK: %[[V0:[0-9]+]] = tail call i8* @objc_autoreleaseReturnValue(
> > +; CHECK: %[[V1:[0-9]+]] = bitcast i8* %[[V0]] to i32*
> > +; CHECK: ret i32* %[[V1]]
> > +
> > +define i32* @autoreleaseRVTailCall() {
> > +  %1 = call i8* @foo1()
> > +  %2 = bitcast i8* %1 to i32*
> > +  %3 = tail call i8* @objc_autoreleaseReturnValue(i8* %1)
> > +  ret i32* %2
> > +}
> > Index: lib/Transforms/ObjCARC/ObjCARCContract.cpp
> > ===================================================================
> > --- lib/Transforms/ObjCARC/ObjCARCContract.cpp
> > +++ lib/Transforms/ObjCARC/ObjCARCContract.cpp
> > @@ -547,13 +547,13 @@
> >
> >     // Don't use GetArgRCIdentityRoot because we don't want to look
> through bitcasts
> >     // and such; to do the replacement, the argument must have type i8*.
> > -    Value *Arg = cast<CallInst>(Inst)->getArgOperand(0);
> >
> > -    // TODO: Change this to a do-while.
> > -    for (;;) {
> > +    // Function for replacing uses of Arg dominated by Inst.
> > +    auto ReplaceArgUses = [Inst, this](Value *Arg) {
> >       // If we're compiling bugpointed code, don't get in trouble.
> >       if (!isa<Instruction>(Arg) && !isa<Argument>(Arg))
> > -        break;
> > +        return;
> > +
> >       // Look through the uses of the pointer.
> >       for (Value::use_iterator UI = Arg->use_begin(), UE =
> Arg->use_end();
> >            UI != UE; ) {
> > @@ -598,6 +598,14 @@
> >           }
> >         }
> >       }
> > +    };
> > +
> > +
> > +    Value *Arg = cast<CallInst>(Inst)->getArgOperand(0), *OrigArg =
> Arg;
> > +
> > +    // TODO: Change this to a do-while.
> > +    for (;;) {
> > +      ReplaceArgUses(Arg);
> >
> >       // If Arg is a no-op casted pointer, strip one level of casts and
> iterate.
> >       if (const BitCastInst *BI = dyn_cast<BitCastInst>(Arg))
> > @@ -611,6 +619,24 @@
> >       else
> >         break;
> >     }
> > +
> > +    // Replace bitcast users of Arg that are dominated by Inst.
> > +    SmallVector<BitCastInst *, 2> BitCastUsers;
> > +
> > +    // Add all bitcast users of the function argument first.
> > +    for (User *U : OrigArg->users())
> > +      if (auto *BC = dyn_cast<BitCastInst>(U))
> > +        BitCastUsers.push_back(BC);
> > +
> > +    // Replace the bitcasts with the call return. Iterate until list is
> empty.
> > +    while (!BitCastUsers.empty()) {
> > +      auto *BC = BitCastUsers.pop_back_val();
> > +      for (User *U : BC->users())
> > +        if (auto *B = dyn_cast<BitCastInst>(U))
> > +          BitCastUsers.push_back(B);
> > +
> > +      ReplaceArgUses(BC);
> > +    }
> >   }
> >
> >   // If this function has no escaping allocas or suspicious vararg usage,
> >
> >
> > <D24523.71239.patch>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160913/62bb145d/attachment-0001.html>