Skip to content
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

[BOLT][NFC] Don't assign YAML profile to functions with no CFG #92487

Merged
merged 2 commits into from
May 20, 2024

Conversation

aaupov
Copy link
Contributor

@aaupov aaupov commented May 17, 2024

YAML profile for non-simple functions without CFG is

  1. useless for optimizations,
  2. can't be attached, similar to fdata profile,
  3. would be reported as invalid/stale even if the profile is valid.

Don't attempt to attach the profile in this case, aligning the behavior
to DataReader.

Test Plan: added yaml-non-simple.test

YAML profile for non-simple functions without CFG is
  1) useless for optimizations,
  2) can't be attached, similar to fdata profile,
  3) would be reported as invalid/stale even if the profile is valid.

Don't attempt to attach the profile in this case, aligning the behavior
to DataReader.
@llvmbot
Copy link
Collaborator

llvmbot commented May 17, 2024

@llvm/pr-subscribers-bolt

Author: Amir Ayupov (aaupov)

Changes

YAML profile for non-simple functions without CFG is

  1. useless for optimizations,
  2. can't be attached, similar to fdata profile,
  3. would be reported as invalid/stale even if the profile is valid.

Don't attempt to attach the profile in this case, aligning the behavior
to DataReader.


Full diff: https://github.com/llvm/llvm-project/pull/92487.diff

1 Files Affected:

  • (modified) bolt/lib/Profile/YAMLProfileReader.cpp (+3)
diff --git a/bolt/lib/Profile/YAMLProfileReader.cpp b/bolt/lib/Profile/YAMLProfileReader.cpp
index 978a7cadfe798..29d94067f459f 100644
--- a/bolt/lib/Profile/YAMLProfileReader.cpp
+++ b/bolt/lib/Profile/YAMLProfileReader.cpp
@@ -99,6 +99,9 @@ bool YAMLProfileReader::parseFunctionProfile(
       FuncRawBranchCount += YamlSI.Count;
   BF.setRawBranchCount(FuncRawBranchCount);
 
+  if (BF.empty())
+    return true;
+
   if (!opts::IgnoreHash &&
       YamlBF.Hash != BF.computeHash(IsDFSOrder, HashFunction)) {
     if (opts::Verbosity >= 1)

Copy link
Member

@dcci dcci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you trigger this somehow from YAML? Like adding a diagnostic and checking if it's hit?

Copy link
Member

@dcci dcci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

@aaupov aaupov merged commit 91423d7 into llvm:main May 20, 2024
4 checks passed
@aaupov aaupov deleted the yaml-fix branch May 20, 2024 03:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants