Skip to content

Revert "[llvm-cov][WebAssembly] Read __llvm_prf_names from data segments" #112520

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

Merged

Conversation

kateinoigakukun
Copy link
Member

@kateinoigakukun kateinoigakukun commented Oct 16, 2024

This reverts commit efc9dd4 in order to fix Windows test failure: #111332 (comment)

@llvmbot llvmbot added the PGO Profile Guided Optimizations label Oct 16, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 16, 2024

@llvm/pr-subscribers-pgo

Author: Yuta Saito (kateinoigakukun)

Changes

This reverts commit efc9dd4 in order to fix Windows test failure


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

4 Files Affected:

  • (modified) llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp (+9-55)
  • (removed) llvm/test/tools/llvm-cov/Inputs/binary-formats.v6.wasm32 ()
  • (removed) llvm/test/tools/llvm-cov/Inputs/binary-formats.wasm.proftext (-4)
  • (modified) llvm/test/tools/llvm-cov/binary-formats.c (-7)
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
index 461fc43d32f8df..8881bffe41c57c 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -18,14 +18,12 @@
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/BinaryFormat/Wasm.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Object/Error.h"
 #include "llvm/Object/MachOUniversal.h"
 #include "llvm/Object/ObjectFile.h"
-#include "llvm/Object/Wasm.h"
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compression.h"
@@ -1079,53 +1077,6 @@ lookupSections(ObjectFile &OF, InstrProfSectKind IPSK) {
   return Sections;
 }
 
-/// Find a section that matches \p Name and is allocatable at runtime.
-///
-/// Returns the contents of the section and its start offset in the object file.
-static Expected<std::pair<StringRef, uint64_t>>
-lookupAllocatableSection(ObjectFile &OF, InstrProfSectKind IPSK) {
-  // On Wasm, allocatable sections can live only in data segments.
-  if (auto *WOF = dyn_cast<WasmObjectFile>(&OF)) {
-    std::vector<const WasmSegment *> Segments;
-    auto ObjFormat = OF.getTripleObjectFormat();
-    auto Name =
-        getInstrProfSectionName(IPSK, ObjFormat, /*AddSegmentInfo=*/false);
-    for (const auto &DebugName : WOF->debugNames()) {
-      if (DebugName.Type != wasm::NameType::DATA_SEGMENT ||
-          DebugName.Name != Name)
-        continue;
-      if (DebugName.Index >= WOF->dataSegments().size())
-        return make_error<CoverageMapError>(coveragemap_error::malformed);
-      auto &Segment = WOF->dataSegments()[DebugName.Index];
-      Segments.push_back(&Segment);
-    }
-    if (Segments.empty())
-      return make_error<CoverageMapError>(coveragemap_error::no_data_found);
-    if (Segments.size() != 1)
-      return make_error<CoverageMapError>(coveragemap_error::malformed);
-
-    const auto &Segment = *Segments.front();
-    auto &Data = Segment.Data;
-    StringRef Content(reinterpret_cast<const char *>(Data.Content.data()),
-                      Data.Content.size());
-    return std::make_pair(Content, Segment.SectionOffset);
-  }
-
-  // On other object file types, delegate to lookupSections to find the section.
-  auto Sections = lookupSections(OF, IPSK);
-  if (!Sections)
-    return Sections.takeError();
-  if (Sections->size() != 1)
-    return make_error<CoverageMapError>(
-        coveragemap_error::malformed,
-        "the size of coverage mapping section is not one");
-  auto &Section = Sections->front();
-  auto ContentsOrErr = Section.getContents();
-  if (!ContentsOrErr)
-    return ContentsOrErr.takeError();
-  return std::make_pair(*ContentsOrErr, Section.getAddress());
-}
-
 static Expected<std::unique_ptr<BinaryCoverageReader>>
 loadBinaryFormat(std::unique_ptr<Binary> Bin, StringRef Arch,
                  StringRef CompilationDir = "",
@@ -1156,20 +1107,23 @@ loadBinaryFormat(std::unique_ptr<Binary> Bin, StringRef Arch,
 
   // Look for the sections that we are interested in.
   auto ProfileNames = std::make_unique<InstrProfSymtab>();
+  std::vector<SectionRef> NamesSectionRefs;
   // If IPSK_name is not found, fallback to search for IPK_covname, which is
   // used when binary correlation is enabled.
-  auto NamesSection = lookupAllocatableSection(*OF, IPSK_name);
+  auto NamesSection = lookupSections(*OF, IPSK_name);
   if (auto E = NamesSection.takeError()) {
     consumeError(std::move(E));
-    NamesSection = lookupAllocatableSection(*OF, IPSK_covname);
+    NamesSection = lookupSections(*OF, IPSK_covname);
     if (auto E = NamesSection.takeError())
       return std::move(E);
   }
+  NamesSectionRefs = *NamesSection;
 
-  uint64_t NamesAddress;
-  StringRef NamesContent;
-  std::tie(NamesContent, NamesAddress) = *NamesSection;
-  if (Error E = ProfileNames->create(NamesContent, NamesAddress))
+  if (NamesSectionRefs.size() != 1)
+    return make_error<CoverageMapError>(
+        coveragemap_error::malformed,
+        "the size of coverage mapping section is not one");
+  if (Error E = ProfileNames->create(NamesSectionRefs.back()))
     return std::move(E);
 
   auto CoverageSection = lookupSections(*OF, IPSK_covmap);
diff --git a/llvm/test/tools/llvm-cov/Inputs/binary-formats.v6.wasm32 b/llvm/test/tools/llvm-cov/Inputs/binary-formats.v6.wasm32
deleted file mode 100755
index 5a606d5a2f69fd..00000000000000
Binary files a/llvm/test/tools/llvm-cov/Inputs/binary-formats.v6.wasm32 and /dev/null differ
diff --git a/llvm/test/tools/llvm-cov/Inputs/binary-formats.wasm.proftext b/llvm/test/tools/llvm-cov/Inputs/binary-formats.wasm.proftext
deleted file mode 100644
index 20fc3816c2255a..00000000000000
--- a/llvm/test/tools/llvm-cov/Inputs/binary-formats.wasm.proftext
+++ /dev/null
@@ -1,4 +0,0 @@
-__main_argc_argv
-0x0
-1
-100
diff --git a/llvm/test/tools/llvm-cov/binary-formats.c b/llvm/test/tools/llvm-cov/binary-formats.c
index bb61b288cfc62e..a5bfc012860ec3 100644
--- a/llvm/test/tools/llvm-cov/binary-formats.c
+++ b/llvm/test/tools/llvm-cov/binary-formats.c
@@ -10,11 +10,4 @@ int main(int argc, const char *argv[]) {}
 // RUN: llvm-cov show %S/Inputs/binary-formats.v3.macho64l -instr-profile %t.profdata -path-equivalence=/tmp,%S %s | FileCheck %s
 // RUN: llvm-cov show %S/Inputs/binary-formats.v6.linux64l -instr-profile %t.profdata -path-equivalence=/tmp,%S %s | FileCheck %s
 
-// RUN: llvm-profdata merge %S/Inputs/binary-formats.wasm.proftext -o %t.wasm.profdata
-// NOTE: The wasm binary is built with the following command:
-//   clang -target wasm32-unknown-wasi %s -o %S/Inputs/binary-formats.v6.wasm32 \
-//     -mllvm -enable-name-compression=false \
-//     -fprofile-instr-generate -fcoverage-mapping -lwasi-emulated-getpid -lwasi-emulated-mman
-// RUN: llvm-cov show %S/Inputs/binary-formats.v6.wasm32 -instr-profile %t.wasm.profdata -path-equivalence=/tmp,%S %s | FileCheck %s
-
 // RUN: llvm-cov export %S/Inputs/binary-formats.macho64l -instr-profile %t.profdata | FileCheck %S/Inputs/binary-formats.canonical.json

Copy link
Member

@mstorsjo mstorsjo left a comment

Choose a reason for hiding this comment

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

LGTM, this seems to fix my issues with llvm-cov.

@kateinoigakukun kateinoigakukun merged commit 157f10d into llvm:main Oct 16, 2024
10 checks passed
kateinoigakukun added a commit to kateinoigakukun/llvm-project that referenced this pull request Oct 24, 2024
…ments" (llvm#112520)

This reverts commit efc9dd4 in order to
fix Windows test failure:
llvm#111332 (comment)
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.

3 participants