[PATCH] D110123: [clang][objc] Speed up populating the global method pool from modules.
vsapsai added a comment. Thanks for the review! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110123/new/ https://reviews.llvm.org/D110123 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D110123: [clang][objc] Speed up populating the global method pool from modules.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0a35cc40b881: [clang][objc] Speed up populating the global
method pool from modules. (authored by vsapsai).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110123/new/
https://reviews.llvm.org/D110123
Files:
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/Modules/lookup.m
clang/test/Modules/method_pool_transitive.m
Index: clang/test/Modules/method_pool_transitive.m
===
--- /dev/null
+++ clang/test/Modules/method_pool_transitive.m
@@ -0,0 +1,40 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -Wobjc-multiple-method-names -fsyntax-only -fmodules-cache-path=%t/modules.cache -fmodules -fimplicit-module-maps -F %t/Frameworks %t/test.m -verify
+
+// Verify we are handling methods from transitive modules, not just from immediate ones.
+
+//--- Frameworks/Indirect.framework/Headers/Indirect.h
+@interface NSObject
+@end
+
+@interface Indirect : NSObject
+- (int)method;
+@end
+
+//--- Frameworks/Indirect.framework/Modules/module.modulemap
+framework module Indirect {
+ header "Indirect.h"
+ export *
+}
+
+//--- Frameworks/Immediate.framework/Headers/Immediate.h
+#import
+@interface Immediate : NSObject
+- (void)method;
+@end
+
+//--- Frameworks/Immediate.framework/Modules/module.modulemap
+framework module Immediate {
+ header "Immediate.h"
+ export *
+}
+
+//--- test.m
+#import
+
+void test(id obj) {
+ [obj method]; // expected-warning{{multiple methods named 'method' found}}
+ // expected-note@Frameworks/Indirect.framework/Headers/Indirect.h:5{{using}}
+ // expected-note@Frameworks/Immediate.framework/Headers/Immediate.h:3{{also found}}
+}
Index: clang/test/Modules/lookup.m
===
--- clang/test/Modules/lookup.m
+++ clang/test/Modules/lookup.m
@@ -10,8 +10,8 @@
void test(id x) {
[x method];
// expected-warning@-1{{multiple methods named 'method' found}}
-// expected-note@Inputs/lookup_left.h:2{{using}}
-// expected-note@Inputs/lookup_right.h:3{{also found}}
+// expected-note@Inputs/lookup_right.h:3{{using}}
+// expected-note@Inputs/lookup_left.h:2{{also found}}
}
// CHECK-PRINT: - (int)method;
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -3045,11 +3045,11 @@
unsigned DataLen = 4 + 2 + 2; // 2 bytes for each of the method counts
for (const ObjCMethodList *Method = &Methods.Instance; Method;
Method = Method->getNext())
- if (Method->getMethod())
+ if (ShouldWriteMethodListNode(Method))
DataLen += 4;
for (const ObjCMethodList *Method = &Methods.Factory; Method;
Method = Method->getNext())
- if (Method->getMethod())
+ if (ShouldWriteMethodListNode(Method))
DataLen += 4;
return emitULEBKeyDataLength(KeyLen, DataLen, Out);
}
@@ -3080,13 +3080,13 @@
unsigned NumInstanceMethods = 0;
for (const ObjCMethodList *Method = &Methods.Instance; Method;
Method = Method->getNext())
- if (Method->getMethod())
+ if (ShouldWriteMethodListNode(Method))
++NumInstanceMethods;
unsigned NumFactoryMethods = 0;
for (const ObjCMethodList *Method = &Methods.Factory; Method;
Method = Method->getNext())
- if (Method->getMethod())
+ if (ShouldWriteMethodListNode(Method))
++NumFactoryMethods;
unsigned InstanceBits = Methods.Instance.getBits();
@@ -3107,15 +3107,20 @@
LE.write(FullFactoryBits);
for (const ObjCMethodList *Method = &Methods.Instance; Method;
Method = Method->getNext())
- if (Method->getMethod())
+ if (ShouldWriteMethodListNode(Method))
LE.write(Writer.getDeclID(Method->getMethod()));
for (const ObjCMethodList *Method = &Methods.Factory; Method;
Method = Method->getNext())
- if (Method->getMethod())
+ if (ShouldWriteMethodListNode(Method))
LE.write(Writer.getDeclID(Method->getMethod()));
assert(Out.tell() - Start == DataLen && "Data length is wrong");
}
+
+private:
+ static bool ShouldWriteMethodListNode(const ObjCMethodList *Node) {
+return (Node->getMethod() && !Node->getMethod()->isFromASTFile());
+ }
};
} // namespace
@@ -3158,15 +3163,21 @@
if (Chain && ID < FirstSelectorID) {
// Selector already exists. Did it change?
bool changed = false;
-for (ObjCMethodList *M = &Data.Instance;
- !changed && M && M->getMethod(); M = M->getNext()) {
- if (!M->getMethod()->isFromASTFile())
+for (ObjCMethodList *M = &Data.Instance; M && M->getMethod();
+ M = M->getNext()) {
+ if (!M->getMethod()->isFromASTFile
[PATCH] D110123: [clang][objc] Speed up populating the global method pool from modules.
vsapsai updated this revision to Diff 384606.
vsapsai added a comment.
Rebase. Hopefully MLIR build is fixed and I'll be able to see relevant
pre-merge check results.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110123/new/
https://reviews.llvm.org/D110123
Files:
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/Modules/lookup.m
clang/test/Modules/method_pool_transitive.m
Index: clang/test/Modules/method_pool_transitive.m
===
--- /dev/null
+++ clang/test/Modules/method_pool_transitive.m
@@ -0,0 +1,40 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -Wobjc-multiple-method-names -fsyntax-only -fmodules-cache-path=%t/modules.cache -fmodules -fimplicit-module-maps -F %t/Frameworks %t/test.m -verify
+
+// Verify we are handling methods from transitive modules, not just from immediate ones.
+
+//--- Frameworks/Indirect.framework/Headers/Indirect.h
+@interface NSObject
+@end
+
+@interface Indirect : NSObject
+- (int)method;
+@end
+
+//--- Frameworks/Indirect.framework/Modules/module.modulemap
+framework module Indirect {
+ header "Indirect.h"
+ export *
+}
+
+//--- Frameworks/Immediate.framework/Headers/Immediate.h
+#import
+@interface Immediate : NSObject
+- (void)method;
+@end
+
+//--- Frameworks/Immediate.framework/Modules/module.modulemap
+framework module Immediate {
+ header "Immediate.h"
+ export *
+}
+
+//--- test.m
+#import
+
+void test(id obj) {
+ [obj method]; // expected-warning{{multiple methods named 'method' found}}
+ // expected-note@Frameworks/Indirect.framework/Headers/Indirect.h:5{{using}}
+ // expected-note@Frameworks/Immediate.framework/Headers/Immediate.h:3{{also found}}
+}
Index: clang/test/Modules/lookup.m
===
--- clang/test/Modules/lookup.m
+++ clang/test/Modules/lookup.m
@@ -10,8 +10,8 @@
void test(id x) {
[x method];
// expected-warning@-1{{multiple methods named 'method' found}}
-// expected-note@Inputs/lookup_left.h:2{{using}}
-// expected-note@Inputs/lookup_right.h:3{{also found}}
+// expected-note@Inputs/lookup_right.h:3{{using}}
+// expected-note@Inputs/lookup_left.h:2{{also found}}
}
// CHECK-PRINT: - (int)method;
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -3045,11 +3045,11 @@
unsigned DataLen = 4 + 2 + 2; // 2 bytes for each of the method counts
for (const ObjCMethodList *Method = &Methods.Instance; Method;
Method = Method->getNext())
- if (Method->getMethod())
+ if (ShouldWriteMethodListNode(Method))
DataLen += 4;
for (const ObjCMethodList *Method = &Methods.Factory; Method;
Method = Method->getNext())
- if (Method->getMethod())
+ if (ShouldWriteMethodListNode(Method))
DataLen += 4;
return emitULEBKeyDataLength(KeyLen, DataLen, Out);
}
@@ -3080,13 +3080,13 @@
unsigned NumInstanceMethods = 0;
for (const ObjCMethodList *Method = &Methods.Instance; Method;
Method = Method->getNext())
- if (Method->getMethod())
+ if (ShouldWriteMethodListNode(Method))
++NumInstanceMethods;
unsigned NumFactoryMethods = 0;
for (const ObjCMethodList *Method = &Methods.Factory; Method;
Method = Method->getNext())
- if (Method->getMethod())
+ if (ShouldWriteMethodListNode(Method))
++NumFactoryMethods;
unsigned InstanceBits = Methods.Instance.getBits();
@@ -3107,15 +3107,20 @@
LE.write(FullFactoryBits);
for (const ObjCMethodList *Method = &Methods.Instance; Method;
Method = Method->getNext())
- if (Method->getMethod())
+ if (ShouldWriteMethodListNode(Method))
LE.write(Writer.getDeclID(Method->getMethod()));
for (const ObjCMethodList *Method = &Methods.Factory; Method;
Method = Method->getNext())
- if (Method->getMethod())
+ if (ShouldWriteMethodListNode(Method))
LE.write(Writer.getDeclID(Method->getMethod()));
assert(Out.tell() - Start == DataLen && "Data length is wrong");
}
+
+private:
+ static bool ShouldWriteMethodListNode(const ObjCMethodList *Node) {
+return (Node->getMethod() && !Node->getMethod()->isFromASTFile());
+ }
};
} // namespace
@@ -3158,15 +3163,21 @@
if (Chain && ID < FirstSelectorID) {
// Selector already exists. Did it change?
bool changed = false;
-for (ObjCMethodList *M = &Data.Instance;
- !changed && M && M->getMethod(); M = M->getNext()) {
- if (!M->getMethod()->isFromASTFile())
+for (ObjCMethodList *M = &Data.Instance; M && M->getMethod();
+ M = M->getNext()) {
+ if (!M->getMethod()->isFromASTFile()) {
changed = tru
[PATCH] D110123: [clang][objc] Speed up populating the global method pool from modules.
vsapsai added a comment. Lots of relevant discussion can be found at https://reviews.llvm.org/D109632 I'll add the link to it to the summary later, for now the summary is the same as commit message. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110123/new/ https://reviews.llvm.org/D110123 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D110123: [clang][objc] Speed up populating the global method pool from modules.
vsapsai added a comment. /cc @v.g.vassilev as he mentioned an issue with late-parsed templates can be similar to this one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110123/new/ https://reviews.llvm.org/D110123 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
