[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7611e16fce9c: [clang][objc][codegen] Skip emitting ObjC
category metadata when the (authored by guitard0g, committed by ahatanak).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113455/new/
https://reviews.llvm.org/D113455
Files:
clang/lib/CodeGen/CGObjCMac.cpp
clang/test/CodeGenObjC/category-class-empty.m
clang/test/CodeGenObjC/non-lazy-classes.m
Index: clang/test/CodeGenObjC/non-lazy-classes.m
===
--- clang/test/CodeGenObjC/non-lazy-classes.m
+++ clang/test/CodeGenObjC/non-lazy-classes.m
@@ -42,4 +42,7 @@
@implementation E @end
__attribute__((objc_nonlazy_class))
-@implementation E (MyCat) @end
+@implementation E (MyCat)
+-(void) load {
+}
+@end
Index: clang/test/CodeGenObjC/category-class-empty.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/category-class-empty.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -O0 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s
+// PR7431
+
+// CHECK-NOT: @"OBJC_LABEL_CATEGORY_$" = private global [1 x i8*] [i8* bitcast (%struct._category_t* @"_OBJC_$_CATEGORY_A_$_foo"
+
+@interface A
+@end
+__attribute__((objc_direct_members))
+@interface A(foo)
+- (void)foo_myStuff;
+@end
+@implementation A(foo)
+- (void)foo_myStuff {
+}
+@end
+
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -6672,33 +6672,53 @@
}
}
- values.add(emitMethodList(listName, MethodListType::CategoryInstanceMethods,
-instanceMethods));
- values.add(emitMethodList(listName, MethodListType::CategoryClassMethods,
-classMethods));
+ auto instanceMethodList = emitMethodList(
+ listName, MethodListType::CategoryInstanceMethods, instanceMethods);
+ auto classMethodList = emitMethodList(
+ listName, MethodListType::CategoryClassMethods, classMethods);
+ values.add(instanceMethodList);
+ values.add(classMethodList);
+ // Keep track of whether we have actual metadata to emit.
+ bool isEmptyCategory =
+ instanceMethodList->isNullValue() && classMethodList->isNullValue();
const ObjCCategoryDecl *Category =
-Interface->FindCategoryDeclaration(OCD->getIdentifier());
+ Interface->FindCategoryDeclaration(OCD->getIdentifier());
if (Category) {
SmallString<256> ExtName;
-llvm::raw_svector_ostream(ExtName) << Interface->getObjCRuntimeNameAsString() << "_$_"
- << OCD->getName();
-values.add(EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_"
- + Interface->getObjCRuntimeNameAsString() + "_$_"
- + Category->getName(),
-Category->protocol_begin(),
-Category->protocol_end()));
-values.add(EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, false));
-values.add(EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, true));
+llvm::raw_svector_ostream(ExtName)
+<< Interface->getObjCRuntimeNameAsString() << "_$_" << OCD->getName();
+auto protocolList =
+EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_" +
+ Interface->getObjCRuntimeNameAsString() + "_$_" +
+ Category->getName(),
+ Category->protocol_begin(), Category->protocol_end());
+auto propertyList = EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
+ OCD, Category, ObjCTypes, false);
+auto classPropertyList =
+EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(), OCD,
+ Category, ObjCTypes, true);
+values.add(protocolList);
+values.add(propertyList);
+values.add(classPropertyList);
+isEmptyCategory &= protocolList->isNullValue() &&
+ propertyList->isNullValue() &&
+ classPropertyList->isNullValue();
} else {
values.addNullPointer(ObjCTypes.ProtocolListnfABIPtrTy);
values.addNullPointer(ObjCTypes.PropertyListPtrTy);
values.addNullPointer(ObjCTypes.PropertyListPtrTy);
}
- unsigned Size = CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
+ if (isEmptyCategory) {
+// Empty category, don't emit any metadata.
+values.abandon();
+MethodDefinitions.clear();
+return;
+ }
+
+ unsigned Size =
+ CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
values.addInt(ObjCTypes.IntTy, Size);
llvm::GlobalVariable *GCATV =
[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty
guitard0g added a comment. @ahatanak I don't have commit access, any chance you could commit this for me? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113455/new/ https://reviews.llvm.org/D113455 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty
ahatanak accepted this revision. ahatanak added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113455/new/ https://reviews.llvm.org/D113455 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty
guitard0g updated this revision to Diff 386949.
guitard0g marked 5 inline comments as done.
guitard0g added a comment.
Nit: deleted empty line.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113455/new/
https://reviews.llvm.org/D113455
Files:
clang/lib/CodeGen/CGObjCMac.cpp
clang/test/CodeGenObjC/category-class-empty.m
clang/test/CodeGenObjC/non-lazy-classes.m
Index: clang/test/CodeGenObjC/non-lazy-classes.m
===
--- clang/test/CodeGenObjC/non-lazy-classes.m
+++ clang/test/CodeGenObjC/non-lazy-classes.m
@@ -42,4 +42,7 @@
@implementation E @end
__attribute__((objc_nonlazy_class))
-@implementation E (MyCat) @end
+@implementation E (MyCat)
+-(void) load {
+}
+@end
Index: clang/test/CodeGenObjC/category-class-empty.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/category-class-empty.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -O0 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s
+// PR7431
+
+// CHECK-NOT: @"OBJC_LABEL_CATEGORY_$" = private global [1 x i8*] [i8* bitcast (%struct._category_t* @"_OBJC_$_CATEGORY_A_$_foo"
+
+@interface A
+@end
+__attribute__((objc_direct_members))
+@interface A(foo)
+- (void)foo_myStuff;
+@end
+@implementation A(foo)
+- (void)foo_myStuff {
+}
+@end
+
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -6672,33 +6672,53 @@
}
}
- values.add(emitMethodList(listName, MethodListType::CategoryInstanceMethods,
-instanceMethods));
- values.add(emitMethodList(listName, MethodListType::CategoryClassMethods,
-classMethods));
+ auto instanceMethodList = emitMethodList(
+ listName, MethodListType::CategoryInstanceMethods, instanceMethods);
+ auto classMethodList = emitMethodList(
+ listName, MethodListType::CategoryClassMethods, classMethods);
+ values.add(instanceMethodList);
+ values.add(classMethodList);
+ // Keep track of whether we have actual metadata to emit.
+ bool isEmptyCategory =
+ instanceMethodList->isNullValue() && classMethodList->isNullValue();
const ObjCCategoryDecl *Category =
-Interface->FindCategoryDeclaration(OCD->getIdentifier());
+ Interface->FindCategoryDeclaration(OCD->getIdentifier());
if (Category) {
SmallString<256> ExtName;
-llvm::raw_svector_ostream(ExtName) << Interface->getObjCRuntimeNameAsString() << "_$_"
- << OCD->getName();
-values.add(EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_"
- + Interface->getObjCRuntimeNameAsString() + "_$_"
- + Category->getName(),
-Category->protocol_begin(),
-Category->protocol_end()));
-values.add(EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, false));
-values.add(EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, true));
+llvm::raw_svector_ostream(ExtName)
+<< Interface->getObjCRuntimeNameAsString() << "_$_" << OCD->getName();
+auto protocolList =
+EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_" +
+ Interface->getObjCRuntimeNameAsString() + "_$_" +
+ Category->getName(),
+ Category->protocol_begin(), Category->protocol_end());
+auto propertyList = EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
+ OCD, Category, ObjCTypes, false);
+auto classPropertyList =
+EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(), OCD,
+ Category, ObjCTypes, true);
+values.add(protocolList);
+values.add(propertyList);
+values.add(classPropertyList);
+isEmptyCategory &= protocolList->isNullValue() &&
+ propertyList->isNullValue() &&
+ classPropertyList->isNullValue();
} else {
values.addNullPointer(ObjCTypes.ProtocolListnfABIPtrTy);
values.addNullPointer(ObjCTypes.PropertyListPtrTy);
values.addNullPointer(ObjCTypes.PropertyListPtrTy);
}
- unsigned Size = CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
+ if (isEmptyCategory) {
+// Empty category, don't emit any metadata.
+values.abandon();
+MethodDefinitions.clear();
+return;
+ }
+
+ unsigned Size =
+ CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
values.addInt(ObjCTypes.IntTy, Size);
llvm::GlobalVariable *GCATV =
___
cfe-commits mailing list
[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty
guitard0g marked 6 inline comments as done.
guitard0g added inline comments.
Comment at: clang/lib/CodeGen/CGObjCMac.cpp:6683
+ values.add(classMethodList);
+ isEmptyCategory &=
+ instanceMethodList->isNullValue() && classMethodList->isNullValue();
zoecarver wrote:
> Does this really need to be an `&=`? Is there any case where
> `isEmptyCategory` isn't `true` here?
Good point, fixed.
Comment at: clang/lib/CodeGen/CGObjCMac.cpp:6713
}
+ unsigned Size =
ahatanak wrote:
> Is it not possible to check whether the category is empty here? If it's
> empty, you can avoid creating the global variable and abandon the builder.
Good point, done.
Comment at: clang/test/CodeGenObjC/category-class-empty.m:1
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -O3 -emit-llvm -o - %s |
FileCheck %s
+// PR7431
jkorous wrote:
> zoecarver wrote:
> > Is `-O3` needed here?
> I would even think `-O0` is better and consider also `-disable-llvm-passes`.
> Ultimately the goal is to make sure frontend codegen doesn't emit the
> metadata so the less processing of the IR in the backend the more sensitive
> the test will be.
Good catch, changed to -O0 with -disable-llvm-passes.
Comment at: clang/test/CodeGenObjC/category-class-empty.m:10
+@interface A(foo)
+- (void)foo_myStuff;
+@end
jkorous wrote:
> I assume all the cases when we want to emit the metadata (at least one
> instance method, at least one class method, ...) are covered by other tests,
> right?
> If there's a case that's missing we should add a test for it.
Yeah a fair number of tests would break if we started removing categories with
anything inside of them.
Comment at: clang/test/CodeGenObjC/non-lazy-classes.m:45
__attribute__((objc_nonlazy_class))
-@implementation E (MyCat) @end
+@implementation E (MyCat)
+-(void) load {
zoecarver wrote:
> This is to prevent another test from failing? Makes sense.
Yeah this is the one case where the IRGen check was looking for the category
even though it's empty. Should be a harmless change.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113455/new/
https://reviews.llvm.org/D113455
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty
guitard0g updated this revision to Diff 386947.
guitard0g added a comment.
Abandon builder earlier before creating global variable.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113455/new/
https://reviews.llvm.org/D113455
Files:
clang/lib/CodeGen/CGObjCMac.cpp
clang/test/CodeGenObjC/category-class-empty.m
clang/test/CodeGenObjC/non-lazy-classes.m
Index: clang/test/CodeGenObjC/non-lazy-classes.m
===
--- clang/test/CodeGenObjC/non-lazy-classes.m
+++ clang/test/CodeGenObjC/non-lazy-classes.m
@@ -42,4 +42,7 @@
@implementation E @end
__attribute__((objc_nonlazy_class))
-@implementation E (MyCat) @end
+@implementation E (MyCat)
+-(void) load {
+}
+@end
Index: clang/test/CodeGenObjC/category-class-empty.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/category-class-empty.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -O0 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s
+// PR7431
+
+// CHECK-NOT: @"OBJC_LABEL_CATEGORY_$" = private global [1 x i8*] [i8* bitcast (%struct._category_t* @"_OBJC_$_CATEGORY_A_$_foo"
+
+@interface A
+@end
+__attribute__((objc_direct_members))
+@interface A(foo)
+- (void)foo_myStuff;
+@end
+@implementation A(foo)
+- (void)foo_myStuff {
+}
+@end
+
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -6672,37 +6672,58 @@
}
}
- values.add(emitMethodList(listName, MethodListType::CategoryInstanceMethods,
-instanceMethods));
- values.add(emitMethodList(listName, MethodListType::CategoryClassMethods,
-classMethods));
+ auto instanceMethodList = emitMethodList(
+ listName, MethodListType::CategoryInstanceMethods, instanceMethods);
+ auto classMethodList = emitMethodList(
+ listName, MethodListType::CategoryClassMethods, classMethods);
+ values.add(instanceMethodList);
+ values.add(classMethodList);
+ // Keep track of whether we have actual metadata to emit.
+ bool isEmptyCategory =
+ instanceMethodList->isNullValue() && classMethodList->isNullValue();
const ObjCCategoryDecl *Category =
-Interface->FindCategoryDeclaration(OCD->getIdentifier());
+ Interface->FindCategoryDeclaration(OCD->getIdentifier());
if (Category) {
SmallString<256> ExtName;
-llvm::raw_svector_ostream(ExtName) << Interface->getObjCRuntimeNameAsString() << "_$_"
- << OCD->getName();
-values.add(EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_"
- + Interface->getObjCRuntimeNameAsString() + "_$_"
- + Category->getName(),
-Category->protocol_begin(),
-Category->protocol_end()));
-values.add(EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, false));
-values.add(EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, true));
+llvm::raw_svector_ostream(ExtName)
+<< Interface->getObjCRuntimeNameAsString() << "_$_" << OCD->getName();
+auto protocolList =
+EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_" +
+ Interface->getObjCRuntimeNameAsString() + "_$_" +
+ Category->getName(),
+ Category->protocol_begin(), Category->protocol_end());
+auto propertyList = EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
+ OCD, Category, ObjCTypes, false);
+auto classPropertyList =
+EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(), OCD,
+ Category, ObjCTypes, true);
+values.add(protocolList);
+values.add(propertyList);
+values.add(classPropertyList);
+isEmptyCategory &= protocolList->isNullValue() &&
+ propertyList->isNullValue() &&
+ classPropertyList->isNullValue();
} else {
values.addNullPointer(ObjCTypes.ProtocolListnfABIPtrTy);
values.addNullPointer(ObjCTypes.PropertyListPtrTy);
values.addNullPointer(ObjCTypes.PropertyListPtrTy);
}
- unsigned Size = CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
+ if (isEmptyCategory) {
+// Empty category, don't emit any metadata.
+values.abandon();
+MethodDefinitions.clear();
+return;
+ }
+
+ unsigned Size =
+ CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
values.addInt(ObjCTypes.IntTy, Size);
llvm::GlobalVariable *GCATV =
finishAndCreateGlobal(values, ExtCatName.str(), CGM);
+
[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty
guitard0g updated this revision to Diff 386943.
guitard0g added a comment.
Address comments and fix fragile test to use -O0 and -disable-llvm-passes.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113455/new/
https://reviews.llvm.org/D113455
Files:
clang/lib/CodeGen/CGObjCMac.cpp
clang/test/CodeGenObjC/category-class-empty.m
clang/test/CodeGenObjC/non-lazy-classes.m
Index: clang/test/CodeGenObjC/non-lazy-classes.m
===
--- clang/test/CodeGenObjC/non-lazy-classes.m
+++ clang/test/CodeGenObjC/non-lazy-classes.m
@@ -42,4 +42,7 @@
@implementation E @end
__attribute__((objc_nonlazy_class))
-@implementation E (MyCat) @end
+@implementation E (MyCat)
+-(void) load {
+}
+@end
Index: clang/test/CodeGenObjC/category-class-empty.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/category-class-empty.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -O0 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s
+// PR7431
+
+// CHECK-NOT: @"OBJC_LABEL_CATEGORY_$" = private global [1 x i8*] [i8* bitcast (%struct._category_t* @"_OBJC_$_CATEGORY_A_$_foo"
+
+@interface A
+@end
+__attribute__((objc_direct_members))
+@interface A(foo)
+- (void)foo_myStuff;
+@end
+@implementation A(foo)
+- (void)foo_myStuff {
+}
+@end
+
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -6672,46 +6672,62 @@
}
}
- values.add(emitMethodList(listName, MethodListType::CategoryInstanceMethods,
-instanceMethods));
- values.add(emitMethodList(listName, MethodListType::CategoryClassMethods,
-classMethods));
+ auto instanceMethodList = emitMethodList(
+ listName, MethodListType::CategoryInstanceMethods, instanceMethods);
+ auto classMethodList = emitMethodList(
+ listName, MethodListType::CategoryClassMethods, classMethods);
+ values.add(instanceMethodList);
+ values.add(classMethodList);
+ // Keep track of whether we have actual metadata to emit.
+ bool isEmptyCategory =
+ instanceMethodList->isNullValue() && classMethodList->isNullValue();
const ObjCCategoryDecl *Category =
-Interface->FindCategoryDeclaration(OCD->getIdentifier());
+ Interface->FindCategoryDeclaration(OCD->getIdentifier());
if (Category) {
SmallString<256> ExtName;
-llvm::raw_svector_ostream(ExtName) << Interface->getObjCRuntimeNameAsString() << "_$_"
- << OCD->getName();
-values.add(EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_"
- + Interface->getObjCRuntimeNameAsString() + "_$_"
- + Category->getName(),
-Category->protocol_begin(),
-Category->protocol_end()));
-values.add(EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, false));
-values.add(EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, true));
+llvm::raw_svector_ostream(ExtName)
+<< Interface->getObjCRuntimeNameAsString() << "_$_" << OCD->getName();
+auto protocolList =
+EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_" +
+ Interface->getObjCRuntimeNameAsString() + "_$_" +
+ Category->getName(),
+ Category->protocol_begin(), Category->protocol_end());
+auto propertyList = EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
+ OCD, Category, ObjCTypes, false);
+auto classPropertyList =
+EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(), OCD,
+ Category, ObjCTypes, true);
+values.add(protocolList);
+values.add(propertyList);
+values.add(classPropertyList);
+isEmptyCategory &= protocolList->isNullValue() &&
+ propertyList->isNullValue() &&
+ classPropertyList->isNullValue();
} else {
values.addNullPointer(ObjCTypes.ProtocolListnfABIPtrTy);
values.addNullPointer(ObjCTypes.PropertyListPtrTy);
values.addNullPointer(ObjCTypes.PropertyListPtrTy);
}
- unsigned Size = CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
+ unsigned Size =
+ CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
values.addInt(ObjCTypes.IntTy, Size);
llvm::GlobalVariable *GCATV =
finishAndCreateGlobal(values, ExtCatName.str(), CGM);
- CGM.addCompilerUsedGlobal(GCATV);
- if (Interface->hasAttr())
-DefinedStubCategories.push_back(GCATV);
- else
-DefinedCategories.push_back(GCATV);
-
[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty
ahatanak added inline comments. Comment at: clang/lib/CodeGen/CGObjCMac.cpp:6713 } + unsigned Size = Is it not possible to check whether the category is empty here? If it's empty, you can avoid creating the global variable and abandon the builder. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113455/new/ https://reviews.llvm.org/D113455 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty
jkorous accepted this revision. jkorous added a comment. LGTM if we have test coverage for all the cases. Comment at: clang/test/CodeGenObjC/category-class-empty.m:1 +// RUN: %clang_cc1 -triple i386-apple-darwin9 -O3 -emit-llvm -o - %s | FileCheck %s +// PR7431 zoecarver wrote: > Is `-O3` needed here? I would even think `-O0` is better and consider also `-disable-llvm-passes`. Ultimately the goal is to make sure frontend codegen doesn't emit the metadata so the less processing of the IR in the backend the more sensitive the test will be. Comment at: clang/test/CodeGenObjC/category-class-empty.m:10 +@interface A(foo) +- (void)foo_myStuff; +@end I assume all the cases when we want to emit the metadata (at least one instance method, at least one class method, ...) are covered by other tests, right? If there's a case that's missing we should add a test for it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113455/new/ https://reviews.llvm.org/D113455 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty
zoecarver accepted this revision.
zoecarver added inline comments.
This revision is now accepted and ready to land.
Comment at: clang/lib/CodeGen/CGObjCMac.cpp:6683
+ values.add(classMethodList);
+ isEmptyCategory &=
+ instanceMethodList->isNullValue() && classMethodList->isNullValue();
Does this really need to be an `&=`? Is there any case where `isEmptyCategory`
isn't `true` here?
Comment at: clang/test/CodeGenObjC/category-class-empty.m:1
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -O3 -emit-llvm -o - %s |
FileCheck %s
+// PR7431
Is `-O3` needed here?
Comment at: clang/test/CodeGenObjC/non-lazy-classes.m:45
__attribute__((objc_nonlazy_class))
-@implementation E (MyCat) @end
+@implementation E (MyCat)
+-(void) load {
This is to prevent another test from failing? Makes sense.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113455/new/
https://reviews.llvm.org/D113455
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty
guitard0g updated this revision to Diff 385870.
guitard0g added a comment.
Fixed code formatting.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113455/new/
https://reviews.llvm.org/D113455
Files:
clang/lib/CodeGen/CGObjCMac.cpp
clang/test/CodeGenObjC/category-class-empty.m
clang/test/CodeGenObjC/non-lazy-classes.m
Index: clang/test/CodeGenObjC/non-lazy-classes.m
===
--- clang/test/CodeGenObjC/non-lazy-classes.m
+++ clang/test/CodeGenObjC/non-lazy-classes.m
@@ -42,4 +42,7 @@
@implementation E @end
__attribute__((objc_nonlazy_class))
-@implementation E (MyCat) @end
+@implementation E (MyCat)
+-(void) load {
+}
+@end
Index: clang/test/CodeGenObjC/category-class-empty.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/category-class-empty.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -O3 -emit-llvm -o - %s | FileCheck %s
+// PR7431
+
+// CHECK-NOT: @"_OBJC_$_CATEGORY_A_$_foo" = internal global %struct._category_t
+
+@interface A
+@end
+__attribute__((objc_direct_members))
+@interface A(foo)
+- (void)foo_myStuff;
+@end
+@implementation A(foo)
+- (void)foo_myStuff {
+}
+@end
+
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -6659,6 +6659,8 @@
values.add(GetClassGlobal(Interface, /*metaclass*/ false, NotForDefinition));
std::string listName =
(Interface->getObjCRuntimeNameAsString() + "_$_" + OCD->getName()).str();
+ // Keep track of whether we have actual metadata to emit.
+ bool isEmptyCategory = true;
SmallVector instanceMethods;
SmallVector classMethods;
@@ -6672,46 +6674,61 @@
}
}
- values.add(emitMethodList(listName, MethodListType::CategoryInstanceMethods,
-instanceMethods));
- values.add(emitMethodList(listName, MethodListType::CategoryClassMethods,
-classMethods));
+ auto instanceMethodList = emitMethodList(
+ listName, MethodListType::CategoryInstanceMethods, instanceMethods);
+ auto classMethodList = emitMethodList(
+ listName, MethodListType::CategoryClassMethods, classMethods);
+ values.add(instanceMethodList);
+ values.add(classMethodList);
+ isEmptyCategory &=
+ instanceMethodList->isNullValue() && classMethodList->isNullValue();
const ObjCCategoryDecl *Category =
-Interface->FindCategoryDeclaration(OCD->getIdentifier());
+ Interface->FindCategoryDeclaration(OCD->getIdentifier());
if (Category) {
SmallString<256> ExtName;
-llvm::raw_svector_ostream(ExtName) << Interface->getObjCRuntimeNameAsString() << "_$_"
- << OCD->getName();
-values.add(EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_"
- + Interface->getObjCRuntimeNameAsString() + "_$_"
- + Category->getName(),
-Category->protocol_begin(),
-Category->protocol_end()));
-values.add(EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, false));
-values.add(EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, true));
+llvm::raw_svector_ostream(ExtName)
+<< Interface->getObjCRuntimeNameAsString() << "_$_" << OCD->getName();
+auto protocolList =
+EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_" +
+ Interface->getObjCRuntimeNameAsString() + "_$_" +
+ Category->getName(),
+ Category->protocol_begin(), Category->protocol_end());
+auto propertyList = EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
+ OCD, Category, ObjCTypes, false);
+auto classPropertyList =
+EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(), OCD,
+ Category, ObjCTypes, true);
+values.add(protocolList);
+values.add(propertyList);
+values.add(classPropertyList);
+isEmptyCategory &= protocolList->isNullValue() &&
+ propertyList->isNullValue() &&
+ classPropertyList->isNullValue();
} else {
values.addNullPointer(ObjCTypes.ProtocolListnfABIPtrTy);
values.addNullPointer(ObjCTypes.PropertyListPtrTy);
values.addNullPointer(ObjCTypes.PropertyListPtrTy);
}
- unsigned Size = CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
+ unsigned Size =
+ CGM.getDataLayout().getTypeAllocSize(ObjCTypes.CategorynfABITy);
values.addInt(ObjCTypes.IntTy, Size);
llvm::GlobalVariable *GCATV =
finishAndCreateGlobal(values,
[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty
guitard0g created this revision.
guitard0g requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D113455
Files:
clang/lib/CodeGen/CGObjCMac.cpp
clang/test/CodeGenObjC/category-class-empty.m
clang/test/CodeGenObjC/non-lazy-classes.m
Index: clang/test/CodeGenObjC/non-lazy-classes.m
===
--- clang/test/CodeGenObjC/non-lazy-classes.m
+++ clang/test/CodeGenObjC/non-lazy-classes.m
@@ -42,4 +42,7 @@
@implementation E @end
__attribute__((objc_nonlazy_class))
-@implementation E (MyCat) @end
+@implementation E (MyCat)
+-(void) load {
+}
+@end
Index: clang/test/CodeGenObjC/category-class-empty.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/category-class-empty.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple i386-apple-darwin9 -O3 -emit-llvm -o - %s | FileCheck %s
+// PR7431
+
+// CHECK-NOT: @"_OBJC_$_CATEGORY_A_$_foo" = internal global %struct._category_t
+
+@interface A
+@end
+__attribute__((objc_direct_members))
+@interface A(foo)
+- (void)foo_myStuff;
+@end
+@implementation A(foo)
+- (void)foo_myStuff {
+}
+@end
+
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -6659,6 +6659,7 @@
values.add(GetClassGlobal(Interface, /*metaclass*/ false, NotForDefinition));
std::string listName =
(Interface->getObjCRuntimeNameAsString() + "_$_" + OCD->getName()).str();
+ bool isEmptyCategory = true; // Keep track of whether we have actual metadata to emit.
SmallVector instanceMethods;
SmallVector classMethods;
@@ -6672,10 +6673,13 @@
}
}
- values.add(emitMethodList(listName, MethodListType::CategoryInstanceMethods,
-instanceMethods));
- values.add(emitMethodList(listName, MethodListType::CategoryClassMethods,
-classMethods));
+ auto instanceMethodList = emitMethodList(listName, MethodListType::CategoryInstanceMethods,
+ instanceMethods);
+ auto classMethodList = emitMethodList(listName, MethodListType::CategoryClassMethods,
+classMethods);
+ values.add(instanceMethodList);
+ values.add(classMethodList);
+ isEmptyCategory &= instanceMethodList->isNullValue() && classMethodList->isNullValue();
const ObjCCategoryDecl *Category =
Interface->FindCategoryDeclaration(OCD->getIdentifier());
@@ -6683,15 +6687,20 @@
SmallString<256> ExtName;
llvm::raw_svector_ostream(ExtName) << Interface->getObjCRuntimeNameAsString() << "_$_"
<< OCD->getName();
-values.add(EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_"
- + Interface->getObjCRuntimeNameAsString() + "_$_"
- + Category->getName(),
-Category->protocol_begin(),
-Category->protocol_end()));
-values.add(EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, false));
-values.add(EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(),
-OCD, Category, ObjCTypes, true));
+auto protocolList = EmitProtocolList("_OBJC_CATEGORY_PROTOCOLS_$_"
+ + Interface->getObjCRuntimeNameAsString() + "_$_"
+ + Category->getName(),
+ Category->protocol_begin(),
+ Category->protocol_end());
+auto propertyList = EmitPropertyList("_OBJC_$_PROP_LIST_" + ExtName.str(),
+ OCD, Category, ObjCTypes, false);
+auto classPropertyList = EmitPropertyList("_OBJC_$_CLASS_PROP_LIST_" + ExtName.str(),
+ OCD, Category, ObjCTypes, true);
+values.add(protocolList);
+values.add(propertyList);
+values.add(classPropertyList);
+isEmptyCategory &= protocolList->isNullValue() && propertyList->isNullValue() &&
+ classPropertyList->isNullValue();
} else {
values.addNullPointer(ObjCTypes.ProtocolListnfABIPtrTy);
values.addNullPointer(ObjCTypes.PropertyListPtrTy);
@@ -6703,15 +6712,18 @@
llvm::GlobalVariable *GCATV =
finishAndCreateGlobal(values, ExtCatName.str(), CGM);
- CGM.addCompilerUsedGlobal(GCATV);
- if (Interface->hasAttr())
-DefinedStubCategories.push_back(GCATV);
- else
-DefinedCategories.push_back(GCATV);
- // Determine if this category is also "non-lazy".
- if (ImplementationIsNonLazy(OCD))
-DefinedNonLazyCategories.push_back(GCATV);
+ if
