[PATCH] [CodeGen] tighten objc ivar invariant.load attribution
John McCall
rjmccall at apple.com
Thu Feb 14 11:23:46 PST 2013
On Feb 14, 2013, at 9:37 AM, Saleem Abdulrasool <compnerd at compnerd.org> wrote:
> Add a few test cases and fix an inverted check.
>
> Hi rjmccall,
>
> http://llvm-reviews.chandlerc.com/D406
>
> CHANGE SINCE LAST DIFF
> http://llvm-reviews.chandlerc.com/D406?vs=976&id=982#toc
>
> Files:
> lib/CodeGen/CGObjCMac.cpp
> test/CodeGenObjC/ivar-invariant-unimplemented.m
> test/CodeGenObjC/ivar-invariant.m
>
> Index: lib/CodeGen/CGObjCMac.cpp
> ===================================================================
> --- lib/CodeGen/CGObjCMac.cpp
> +++ lib/CodeGen/CGObjCMac.cpp
> @@ -6449,10 +6449,23 @@
> unsigned CVRQualifiers) {
> ObjCInterfaceDecl *ID = ObjectTy->getAs<ObjCObjectType>()->getInterface();
> llvm::Value *Offset = EmitIvarOffset(CGF, ID, Ivar);
> +
> + // Annotate the load as an invariant load iff the object type is the type, or
> + // a derived type, of the class containing the ivar within an ObjC method.
> + // This check is needed because the ivar offset is a lazily initialised value
> + // that may depend on objc_msgSend to perform a fixup on the first message
> + // dispatch.
> + //
> + // An additional opportunity to mark the load as invariant arises when the
> + // base of the ivar access is a parameter to an Objective C method. However,
> + // because the parameters are not available in the current interface, we
> + // cannot perform this check.
Go ahead and tweak the interface to propagate through an optional
base expression and then implement your check based on that.
> if (llvm::LoadInst *LI = dyn_cast<llvm::LoadInst>(Offset))
I know this is old code, but you can just use cast<llvm::LoadInst> here;
the dyn_cast is not necessary.
> - LI->setMetadata(CGM.getModule().getMDKindID("invariant.load"),
> - llvm::MDNode::get(VMContext,
> - ArrayRef<llvm::Value*>()));
> + if (dyn_cast<ObjCMethodDecl>(CGF.CurFuncDecl))
> + if (Ivar->getContainingInterface()->isSuperClassOf(ID))
Please extract this condition out into a separate function.
> + LI->setMetadata(CGM.getModule().getMDKindID("invariant.load"),
> + llvm::MDNode::get(VMContext, ArrayRef<llvm::Value*>()));
> +
> return EmitValueForIvarAtOffset(CGF, ID, BaseValue, Ivar, CVRQualifiers,
> Offset);
> }
> Index: test/CodeGenObjC/ivar-invariant-unimplemented.m
> ===================================================================
> --- /dev/null
> +++ test/CodeGenObjC/ivar-invariant-unimplemented.m
> @@ -0,0 +1,32 @@
> +// RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm -x objective-c %s -o - | FileCheck %s
> +// XFAIL: *
We usually don't add xfailed tests for missing optimizations and features.
The proper way to keep track of this is in the bug tracker, not the test suite.
> Index: test/CodeGenObjC/ivar-invariant.m
> ===================================================================
> --- /dev/null
> +++ test/CodeGenObjC/ivar-invariant.m
> @@ -0,0 +1,55 @@
> +// RUN: %clang_cc1 -triple x86_64-apple-darwin -x objective-c %s -o - | FileCheck %s
> +
> + at interface NSObject
> ++ (id) new;
> +- (id) init;
> + at end
> +
> + at interface Base : NSObject @end
> +
> +// @implementation Base
> +// {
> +// int dummy;
> +// }
> +// @end
> +
> + at interface Derived : Base
> +{
> + @public int member;
> +}
> + at end
> +
> + at implementation Derived
> +- (id) init
> +{
> + self = [super init];
> + member = 42;
> + return self;
> +}
> + at end
> +
> +// CHECK: %ivar = load i64* @"OBJC_IVAR_$_Derived.member", !invariant.load
You can't test for LLVM IR names like this, because we strip names in non-asserts
builds. If you actually needed the name for later test lines, you would need to match
a pattern against it, like so:
[[IVAR:%.*]] = load ...
In this case, you don't, so you're fine.
> +
> +void * variant_load_1(int i) {
> + void *ptr;
> + while (i--) {
> + Derived *d = [Derived new];
> + ptr = &d->member;
> + }
> + return ptr;
> +}
> +
> +// CHECK: %ivar = load i64* @"OBJC_IVAR_$_Derived.member"
> +// CHECK-NOT: %ivar = load i64* @"OBJC_IVAR_$_Derived.member", !invariant.load
This is not how CHECK-NOT works. What you actually want to do is
// CHECK: %ivar = load i64* @"OBJC_IVAR_$_Derived.member"{{$}}
John.
More information about the cfe-commits
mailing list