yelite commented on code in PR #15140:
URL: https://github.com/apache/tvm/pull/15140#discussion_r1243094535
##########
tests/python/relax/test_transform_merge_composite_functions.py:
##########
@@ -41,7 +41,7 @@ def main(
R.output(gv)
return gv
- @R.function
+ @R.function(private=True)
Review Comment:
With this diff, these tests will pass without making those function private
```diff
diff --git a/src/relax/transform/dead_code_elimination.cc
b/src/relax/transform/dead_code_elimination.cc
index fe36eb28ef..596c68ef3e 100644
--- a/src/relax/transform/dead_code_elimination.cc
+++ b/src/relax/transform/dead_code_elimination.cc
@@ -96,7 +96,7 @@ IRModule RemoveUnusedFunctions(IRModule mod_,
Array<runtime::String> entry_funcs
for (auto f : existing_functions) {
// If a function has an external linkage type, we do not remove it.
// Otherwise, we check the function and remove it if it is not used
anywhere.
- if (f.second->GetLinkageType() == LinkageType::kInternal &&
!tracer.check_if_called(f.first)) {
+ if (!tracer.check_if_called(f.first)) {
mod_->Remove(f.first);
}
}
diff --git a/src/relax/transform/merge_composite_functions.cc
b/src/relax/transform/merge_composite_functions.cc
index 0bc92ba923..bdae719e31 100644
--- a/src/relax/transform/merge_composite_functions.cc
+++ b/src/relax/transform/merge_composite_functions.cc
@@ -60,6 +60,8 @@
#include <tvm/tir/function.h>
#include "../../support/arena.h"
+#include "tvm/ir/attrs.h"
+#include "tvm/ir/function.h"
#include "utils.h"
namespace tvm {
@@ -311,7 +313,8 @@ class CompositeInliner : public ExprMutator {
if (func->GetAttr<String>(attr::kComposite)) {
if (!inlined_functions_.count(func)) {
- inlined_functions_.Set(func, CopyWithNewVars(func));
+ inlined_functions_.Set(func,
+ WithoutAttr(CopyWithNewVars(func),
tvm::attr::kGlobalSymbol));
}
return Call(inlined_functions_[func], call->args);
}
```
At the end of MergeCompositeFunction, a DCE pass will be called to removed
the merged functions. However, DCE will not remove public functions, i.e. those
with a global symbol. Making those functions public prevents them from being
cleaned up by DCE.
I think the logic here is correct. Those fused_xxx functions should be
private as they are not meant to be called externally. So we probably don't
want to change anything here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
