[clang] [ExposeObjCDirect] Adding a flag to expose symbols with objc_direct (PR #170616)
https://github.com/rjmccall commented: I'm not sure how I feel about this use of "expose" to specifically mean "don't put the prefix byte on it". What linkage and visibility do these end up getting? The patch otherwise seems fine. https://github.com/llvm/llvm-project/pull/170616 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ExposeObjCDirect] Adding a flag to expose symbols with objc_direct (PR #170616)
@@ -4096,6 +4096,10 @@ static void RenderObjCOptions(const ToolChain &TC, const Driver &D, } } + // Forward -fobjc-expose-direct-methods to cc1 + if (Args.hasArg(options::OPT_fobjc_expose_direct_methods)) tarunprabhu wrote: If you use `hasArg` here, would it return true in the following case? ``` clang -fobjc-expose-direct-methods -fno-objc-expose-direct-methods ``` If so, is this the expected behavior? Please add a test that checks that the `f(no)-objc-expose-direct-method options add the correct options to the `cc1` command line. https://github.com/llvm/llvm-project/pull/170616 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ExposeObjCDirect] Adding a flag to expose symbols with objc_direct (PR #170616)
https://github.com/tarunprabhu edited https://github.com/llvm/llvm-project/pull/170616 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ExposeObjCDirect] Adding a flag to expose symbols with objc_direct (PR #170616)
https://github.com/tarunprabhu requested changes to this pull request. I have only reviewed the driver-related code. I cannot comment on anything else. https://github.com/llvm/llvm-project/pull/170616 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ExposeObjCDirect] Adding a flag to expose symbols with objc_direct (PR #170616)
DataCorrupted wrote: TODO: We switch to `-fobjc-direct-caller-thunks` or `-fobjc-direct-nil-check-thunk`. Would love to hear ppl's opinions if any. Naming things are hard. I will do that after later PRs are merged into this branch, otherwise there will be a lot of conflicts to resolve (later PRs already used the flag in the test) cc @kyulee-com https://github.com/llvm/llvm-project/pull/170616 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ExposeObjCDirect] Adding a flag to expose symbols with objc_direct (PR #170616)
https://github.com/DataCorrupted edited https://github.com/llvm/llvm-project/pull/170616 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ExposeObjCDirect] Adding a flag to expose symbols with objc_direct (PR #170616)
DataCorrupted wrote: @rjmccall I believe this feature to expose objc_direct symbols is read for review. Also tagging @MadCoder since you are the original author, @ahatanak as you helped draft the [first version of the RFC](https://docs.google.com/document/d/16CsNCA2OKWkUM_LCw_qabTcMtPiq9ODVkZHALlDvzR8) https://github.com/llvm/llvm-project/pull/170616 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ExposeObjCDirect] Adding a flag to expose symbols with objc_direct (PR #170616)
llvmbot wrote:
@llvm/pr-subscribers-clang-codegen
Author: Peter Rong (DataCorrupted)
Changes
## TL;DR
This is a stack of PRs implementing features to expose direct methods ABI.
You can see the RFC, design, and discussion
[here](https://discourse.llvm.org/t/rfc-optimizing-code-size-of-objc-direct-by-exposing-function-symbols-and-moving-nil-checks-to-thunks/88866).
https://github.com/llvm/llvm-project/pull/170616 **Flag -fexpose-objc-direct
set up**
https://github.com/llvm/llvm-project/pull/170617 Code refactoring to ease later
reviews
https://github.com/llvm/llvm-project/pull/170618 Thunk generation
https://github.com/llvm/llvm-project/pull/170619 Optimizations, some class
objects can be known to be realized
## Implementation details
1. Add a flag. I used `expose-direct-method` instead of
`-fobjc-direct-caller-thunks` in the proposal for two reasons:
a. It conveys our intention more clearly, that we are trying to remove the
`\01` byte before the name of direct method so the developers can expose a
symbol if they want
b. Much of the code is actually handling corner cases of var_arg, which
don't have a thunk. Thus the name `thunk` can be confusing.
2. Clean up and set up helper functions to implement later
a. `canMessageReceiverBeNull` / `canClassObjectBeUnrealized` these two
functions will be helpful later to determine which function (true
implementation or nil check thunk) we should dispatch a call to. Formatting.
b. `getSymbolNameForMethod` has a new argument `includePrefixByte`, which
allows us to erase the prefixing `\01` when the flag is enabled
c. `shouldExposeSymbol` is the single source of truth of what we should do.
It not only checks for the flag, but also whether the method is qualified and
we are in the right runtime. A method that `shouldExposeSymbol` is either
`shouldHaveNilCheckThunk` or `shouldHaveNilCheckInline`.
## Tests
None, existing ones shouldn't break
---
Full diff: https://github.com/llvm/llvm-project/pull/170616.diff
7 Files Affected:
- (modified) clang/include/clang/AST/DeclObjC.h (+6)
- (modified) clang/include/clang/Basic/CodeGenOptions.def (+2)
- (modified) clang/include/clang/Options/Options.td (+5)
- (modified) clang/lib/CodeGen/CGObjCRuntime.cpp (+15-11)
- (modified) clang/lib/CodeGen/CGObjCRuntime.h (+14-1)
- (modified) clang/lib/CodeGen/CodeGenModule.h (+26)
- (modified) clang/lib/Driver/ToolChains/Clang.cpp (+4)
``diff
diff --git a/clang/include/clang/AST/DeclObjC.h
b/clang/include/clang/AST/DeclObjC.h
index 2541edba83855..e2292cbdea042 100644
--- a/clang/include/clang/AST/DeclObjC.h
+++ b/clang/include/clang/AST/DeclObjC.h
@@ -482,6 +482,12 @@ class ObjCMethodDecl : public NamedDecl, public
DeclContext {
/// True if the method is tagged as objc_direct
bool isDirectMethod() const;
+ /// Check if this direct method can move nil-check to thunk.
+ /// Variadic functions cannot use thunks (musttail incompatible with va_arg)
+ bool canHaveNilCheckThunk() const {
+return isDirectMethod() && !isVariadic();
+ }
+
/// True if the method has a parameter that's destroyed in the callee.
bool hasParamDestroyedInCallee() const;
diff --git a/clang/include/clang/Basic/CodeGenOptions.def
b/clang/include/clang/Basic/CodeGenOptions.def
index 76a6463881c6f..a7e8564c9e83c 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -210,6 +210,8 @@ ENUM_CODEGENOPT(ObjCDispatchMethod, ObjCDispatchMethodKind,
2, Legacy, Benign)
/// Replace certain message sends with calls to ObjC runtime entrypoints
CODEGENOPT(ObjCConvertMessagesToRuntimeCalls , 1, 1, Benign)
CODEGENOPT(ObjCAvoidHeapifyLocalBlocks, 1, 0, Benign)
+/// Expose objc_direct method symbols publicly and optimize nil checks.
+CODEGENOPT(ObjCExposeDirectMethods, 1, 0, Benign)
// The optimization options affect frontend options, which in turn do affect
the AST.
diff --git a/clang/include/clang/Options/Options.td
b/clang/include/clang/Options/Options.td
index 756d6deed7130..ddc04b30cbaa2 100644
--- a/clang/include/clang/Options/Options.td
+++ b/clang/include/clang/Options/Options.td
@@ -3775,6 +3775,11 @@ defm objc_avoid_heapify_local_blocks :
BoolFOption<"objc-avoid-heapify-local-blo
PosFlag,
NegFlag,
BothFlags<[], [CC1Option], " to avoid heapifying local blocks">>;
+defm objc_expose_direct_methods : BoolFOption<"objc-expose-direct-methods",
+ CodeGenOpts<"ObjCExposeDirectMethods">, DefaultFalse,
+ PosFlag,
+ NegFlag>;
defm disable_block_signature_string :
BoolFOption<"disable-block-signature-string",
CodeGenOpts<"DisableBlockSignatureString">, DefaultFalse,
PosFlag,
diff --git a/clang/lib/CodeGen/CGObjCRuntime.cpp
b/clang/lib/CodeGen/CGObjCRuntime.cpp
index 76e0054f4c9da..a4b4460fdc49c 100644
--- a/clang/lib/CodeGen/CGObjCRuntime.cpp
+++ b/clang/lib/CodeGen/CGObjCRuntime.cpp
@@ -382,11 +382,9 @@ CGObjCRuntime::getMessageSendInfo(const ObjCMethodDecl
*method,
[clang] [ExposeObjCDirect] Adding a flag to expose symbols with objc_direct (PR #170616)
llvmbot wrote:
@llvm/pr-subscribers-clang-driver
Author: Peter Rong (DataCorrupted)
Changes
## TL;DR
This is a stack of PRs implementing features to expose direct methods ABI.
You can see the RFC, design, and discussion
[here](https://discourse.llvm.org/t/rfc-optimizing-code-size-of-objc-direct-by-exposing-function-symbols-and-moving-nil-checks-to-thunks/88866).
https://github.com/llvm/llvm-project/pull/170616 **Flag -fexpose-objc-direct
set up**
https://github.com/llvm/llvm-project/pull/170617 Code refactoring to ease later
reviews
https://github.com/llvm/llvm-project/pull/170618 Thunk generation
https://github.com/llvm/llvm-project/pull/170619 Optimizations, some class
objects can be known to be realized
## Implementation details
1. Add a flag. I used `expose-direct-method` instead of
`-fobjc-direct-caller-thunks` in the proposal for two reasons:
a. It conveys our intention more clearly, that we are trying to remove the
`\01` byte before the name of direct method so the developers can expose a
symbol if they want
b. Much of the code is actually handling corner cases of var_arg, which
don't have a thunk. Thus the name `thunk` can be confusing.
2. Clean up and set up helper functions to implement later
a. `canMessageReceiverBeNull` / `canClassObjectBeUnrealized` these two
functions will be helpful later to determine which function (true
implementation or nil check thunk) we should dispatch a call to. Formatting.
b. `getSymbolNameForMethod` has a new argument `includePrefixByte`, which
allows us to erase the prefixing `\01` when the flag is enabled
c. `shouldExposeSymbol` is the single source of truth of what we should do.
It not only checks for the flag, but also whether the method is qualified and
we are in the right runtime. A method that `shouldExposeSymbol` is either
`shouldHaveNilCheckThunk` or `shouldHaveNilCheckInline`.
## Tests
None, existing ones shouldn't break
---
Full diff: https://github.com/llvm/llvm-project/pull/170616.diff
7 Files Affected:
- (modified) clang/include/clang/AST/DeclObjC.h (+6)
- (modified) clang/include/clang/Basic/CodeGenOptions.def (+2)
- (modified) clang/include/clang/Options/Options.td (+5)
- (modified) clang/lib/CodeGen/CGObjCRuntime.cpp (+15-11)
- (modified) clang/lib/CodeGen/CGObjCRuntime.h (+14-1)
- (modified) clang/lib/CodeGen/CodeGenModule.h (+26)
- (modified) clang/lib/Driver/ToolChains/Clang.cpp (+4)
``diff
diff --git a/clang/include/clang/AST/DeclObjC.h
b/clang/include/clang/AST/DeclObjC.h
index 2541edba83855..e2292cbdea042 100644
--- a/clang/include/clang/AST/DeclObjC.h
+++ b/clang/include/clang/AST/DeclObjC.h
@@ -482,6 +482,12 @@ class ObjCMethodDecl : public NamedDecl, public
DeclContext {
/// True if the method is tagged as objc_direct
bool isDirectMethod() const;
+ /// Check if this direct method can move nil-check to thunk.
+ /// Variadic functions cannot use thunks (musttail incompatible with va_arg)
+ bool canHaveNilCheckThunk() const {
+return isDirectMethod() && !isVariadic();
+ }
+
/// True if the method has a parameter that's destroyed in the callee.
bool hasParamDestroyedInCallee() const;
diff --git a/clang/include/clang/Basic/CodeGenOptions.def
b/clang/include/clang/Basic/CodeGenOptions.def
index 76a6463881c6f..a7e8564c9e83c 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -210,6 +210,8 @@ ENUM_CODEGENOPT(ObjCDispatchMethod, ObjCDispatchMethodKind,
2, Legacy, Benign)
/// Replace certain message sends with calls to ObjC runtime entrypoints
CODEGENOPT(ObjCConvertMessagesToRuntimeCalls , 1, 1, Benign)
CODEGENOPT(ObjCAvoidHeapifyLocalBlocks, 1, 0, Benign)
+/// Expose objc_direct method symbols publicly and optimize nil checks.
+CODEGENOPT(ObjCExposeDirectMethods, 1, 0, Benign)
// The optimization options affect frontend options, which in turn do affect
the AST.
diff --git a/clang/include/clang/Options/Options.td
b/clang/include/clang/Options/Options.td
index 756d6deed7130..ddc04b30cbaa2 100644
--- a/clang/include/clang/Options/Options.td
+++ b/clang/include/clang/Options/Options.td
@@ -3775,6 +3775,11 @@ defm objc_avoid_heapify_local_blocks :
BoolFOption<"objc-avoid-heapify-local-blo
PosFlag,
NegFlag,
BothFlags<[], [CC1Option], " to avoid heapifying local blocks">>;
+defm objc_expose_direct_methods : BoolFOption<"objc-expose-direct-methods",
+ CodeGenOpts<"ObjCExposeDirectMethods">, DefaultFalse,
+ PosFlag,
+ NegFlag>;
defm disable_block_signature_string :
BoolFOption<"disable-block-signature-string",
CodeGenOpts<"DisableBlockSignatureString">, DefaultFalse,
PosFlag,
diff --git a/clang/lib/CodeGen/CGObjCRuntime.cpp
b/clang/lib/CodeGen/CGObjCRuntime.cpp
index 76e0054f4c9da..a4b4460fdc49c 100644
--- a/clang/lib/CodeGen/CGObjCRuntime.cpp
+++ b/clang/lib/CodeGen/CGObjCRuntime.cpp
@@ -382,11 +382,9 @@ CGObjCRuntime::getMessageSendInfo(const ObjCMethodDecl
*method,
[clang] [ExposeObjCDirect] Adding a flag to expose symbols with objc_direct (PR #170616)
https://github.com/DataCorrupted ready_for_review https://github.com/llvm/llvm-project/pull/170616 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [ExposeObjCDirect] Adding a flag to expose symbols with objc_direct (PR #170616)
https://github.com/DataCorrupted edited https://github.com/llvm/llvm-project/pull/170616 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
