-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[ML Inliner] Fix inconsistancy between CallGraph and FunctionLevels #122830
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When building our internal code, one of the function annotated with `co_await` (c++20) will cause inconsistancy: The function exists in CG but not FunctionLevels. This patch fixes it by using FunctionLevels only. Signed-off-by: Peter Rong <[email protected]>
|
@llvm/pr-subscribers-llvm-analysis Author: Peter Rong (DataCorrupted) ChangesWhen building our internal code, one of the function annotated with This patch fixes it by using FunctionLevels only. Full diff: https://github.com/llvm/llvm-project/pull/122830.diff 1 Files Affected:
diff --git a/llvm/lib/Analysis/MLInlineAdvisor.cpp b/llvm/lib/Analysis/MLInlineAdvisor.cpp
index 2db58d1c2578bf..68d8f794d23897 100644
--- a/llvm/lib/Analysis/MLInlineAdvisor.cpp
+++ b/llvm/lib/Analysis/MLInlineAdvisor.cpp
@@ -189,7 +189,9 @@ MLInlineAdvisor::MLInlineAdvisor(
}
unsigned MLInlineAdvisor::getInitialFunctionLevel(const Function &F) const {
- return CG.lookup(F) ? FunctionLevels.at(CG.lookup(F)) : 0;
+ return FunctionLevels.find(CG.lookup(F)) != FunctionLevels.end()
+ ? FunctionLevels.at(CG.lookup(F))
+ : 0;
}
void MLInlineAdvisor::onPassEntry(LazyCallGraph::SCC *CurSCC) {
|
|
@llvm/pr-subscribers-mlgo Author: Peter Rong (DataCorrupted) ChangesWhen building our internal code, one of the function annotated with This patch fixes it by using FunctionLevels only. Full diff: https://github.com/llvm/llvm-project/pull/122830.diff 1 Files Affected:
diff --git a/llvm/lib/Analysis/MLInlineAdvisor.cpp b/llvm/lib/Analysis/MLInlineAdvisor.cpp
index 2db58d1c2578bf..68d8f794d23897 100644
--- a/llvm/lib/Analysis/MLInlineAdvisor.cpp
+++ b/llvm/lib/Analysis/MLInlineAdvisor.cpp
@@ -189,7 +189,9 @@ MLInlineAdvisor::MLInlineAdvisor(
}
unsigned MLInlineAdvisor::getInitialFunctionLevel(const Function &F) const {
- return CG.lookup(F) ? FunctionLevels.at(CG.lookup(F)) : 0;
+ return FunctionLevels.find(CG.lookup(F)) != FunctionLevels.end()
+ ? FunctionLevels.at(CG.lookup(F))
+ : 0;
}
void MLInlineAdvisor::onPassEntry(LazyCallGraph::SCC *CurSCC) {
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How difficult is it to construct a regression test for this?
I'm happy to, but I've spent a few hours trying to create one with no luck. I don't have enough understanding of co_await nor CG to understand the root cause of the inconsistency. It appears that there will be a cleanup function that exists in CG but not FunctionLevels. Any hints? |
Co-authored-by: Ellis Hoag <[email protected]>
Have you tried |
|
I put the task as low priority and after three month the file that used to trigger the error compiles fine now. Maybe it's because we moved from LLVM 17 -> 19, maybe the file structure changed. Either way I'm closing this and won't reopen until I can spot similar bugs again. |
When building our internal code, one of the function annotated with
co_await(c++20) will cause inconsistancy: The function exists in CG but not FunctionLevels.This patch fixes it by using FunctionLevels only.