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

[llvm-profgen] Print DWP related warnings under show-detailed-warning #68019

Merged
merged 1 commit into from
Oct 4, 2023

Conversation

htyu
Copy link
Contributor

@htyu htyu commented Oct 2, 2023

Printing DWP related warnings under show-detailed-warning so that they won't flood user log.

@htyu htyu requested a review from WenleiHe October 2, 2023 19:00
@llvmbot llvmbot added the PGO Profile Guided Optimizations label Oct 2, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 2, 2023

@llvm/pr-subscribers-pgo

Changes

Printing DWP related warnings under show-detailed-warning so that they won't flood user log.


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

1 Files Affected:

  • (modified) llvm/tools/llvm-profgen/ProfiledBinary.cpp (+13-7)
diff --git a/llvm/tools/llvm-profgen/ProfiledBinary.cpp b/llvm/tools/llvm-profgen/ProfiledBinary.cpp
index f26b2bd5df5eda5..eade3b46bce773a 100644
--- a/llvm/tools/llvm-profgen/ProfiledBinary.cpp
+++ b/llvm/tools/llvm-profgen/ProfiledBinary.cpp
@@ -810,24 +810,30 @@ void ProfiledBinary::loadSymbolsFromDWARF(ObjectFile &Obj) {
     loadSymbolsFromDWARFUnit(*CompilationUnit.get());
 
   // Handles DWO sections that can either be in .o, .dwo or .dwp files.
+  uint32_t NumOfDWOMissing = 0;
   for (const auto &CompilationUnit : DebugContext->compile_units()) {
     DWARFUnit *const DwarfUnit = CompilationUnit.get();
     if (DwarfUnit->getDWOId()) {
       DWARFUnit *DWOCU = DwarfUnit->getNonSkeletonUnitDIE(false).getDwarfUnit();
       if (!DWOCU->isDWOUnit()) {
-        std::string DWOName = dwarf::toString(
-            DwarfUnit->getUnitDIE().find(
-                {dwarf::DW_AT_dwo_name, dwarf::DW_AT_GNU_dwo_name}),
-            "");
-        WithColor::warning()
-            << "DWO debug information for " << DWOName
-            << " was not loaded. Please check the .o, .dwo or .dwp path.\n";
+        NumOfDWOMissing++;
+        if (ShowDetailedWarning) {
+          std::string DWOName = dwarf::toString(
+              DwarfUnit->getUnitDIE().find(
+                  {dwarf::DW_AT_dwo_name, dwarf::DW_AT_GNU_dwo_name}),
+              "");
+          WithColor::warning()
+              << "DWO debug information for " << DWOName
+              << " was not loaded. Please check the .o, .dwo or .dwp path.\n";
+        }
         continue;
       }
       loadSymbolsFromDWARFUnit(*DWOCU);
     }
   }
 
+  if (NumOfDWOMissing)
+    WithColor::warning() << NumOfDWOMissing << " DWO not loaded\n";
   if (BinaryFunctions.empty())
     WithColor::warning() << "Loading of DWARF info completed, but no binary "
                             "functions have been retrieved.\n";

"");
WithColor::warning()
<< "DWO debug information for " << DWOName
<< " was not loaded. Please check the .o, .dwo or .dwp path.\n";
Copy link
Member

Choose a reason for hiding this comment

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

How about this:

For detailed warning, simply say "DWO debug information for .. was not loaded."

For the summary warning below, say "DWO debug information was not loaded for N modules, Please check the .o, .dwo or .dwp path."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good.

@htyu htyu force-pushed the profgenwarning branch 2 times, most recently from b2ae5f4 to 409e750 Compare October 3, 2023 16:47
if (NumOfDWOMissing)
WithColor::warning()
<< " DWO debug information was not loaded for " << NumOfDWOMissing
<< " modules, Please check the .o, .dwo or .dwp path.\n";
Copy link
Member

Choose a reason for hiding this comment

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

, Please -> . Please

@htyu htyu merged commit 9677678 into llvm:main Oct 4, 2023
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants