[PATCH] D115973: [llvm-profgen] Support symbol loading for debug fission

[Lei Wang via Phabricator via llvm-commits]

wlei added a comment.

Sorry for the late reply, I'm taking time off recently.

@MaskRay @dblaikie @jhenderson  Thanks for your helpful feedbacks!

> It's recommended to use yaml2obj to generate .o files.
>
>> ! In D115973#3228784 <https://reviews.llvm.org/D115973#3228784>, @MaskRay wrote:
>
> In 2021-01, https://lists.llvm.org/pipermail/llvm-dev/2021-January/148048.html (`[RFC] Cross-project lit test suite`) discussed some pain in cross-project debug info testing. Perhaps @jhenderson has some idea for the clang usage in the test.

I've updated all binaries using yaml2obj to generate the .o/.dwo/executable binary. I assume the .o files you mentioned also include the executable binary. As llvm-profgen always require an executable binary(`--binary=<binary>`), here I converted all the .o/.dwo/exe to yaml format.

In D115973#3230696 <https://reviews.llvm.org/D115973#3230696>, @jhenderson wrote:

> In D115973#3228784 <https://reviews.llvm.org/D115973#3228784>, @MaskRay wrote:
>
>> In 2021-01, https://lists.llvm.org/pipermail/llvm-dev/2021-January/148048.html (`[RFC] Cross-project lit test suite`) discussed some pain in cross-project debug info testing. Perhaps @jhenderson has some idea for the clang usage in the test.
>
> I've not dug into this patch at all, but in general terms, the way I see it, you have about three options to craft DWARF in test objects these days:
>
> 1. yaml2obj with DWARF, if the support is sufficiently mature for your use-case, and the output is easy enough to read - one advantage with DWARF yaml2obj is that you can omit many of the required fields, unlike assembly, so depending on the complexity you need, this may be a better option for long-term maintenance.
> 2. Directly write assembly, and then use llvm-mc to compile it into the object file you want. If necessary, you might generate this assembly via clang, and then document the input source, the command line, the version of clang used to generate it, and any post-generation modifications you made. (Other variations might be to compile from IR or other similar input).
> 3. Add the test to cross-project-tests somewhere, and then use clang directly within the test (together with a `REQUIRES: clang` directive). However, this should really only be done if either of the above don't sensibly work OR you are testing the interaction between clang (explicitly clang, not the LLVM backend, for which you can use other tools like llc etc) and llvm-profdata.

Thanks for the detailed instructions. Updated the test using yaml2obj.

> Not sure I follow - you can use a relative path with -fdebug-compilation-dir/-fdebug-prefix-map, but I think for lit tests you'd still need to cd into the test directory because I don't think lit guarantees which directory is the cwd of the test at the start.
> I'd rather not implement (or test using a quirk/unintended feature, even if it doesn't require explicit code to support the use case) a semi-confusing --dwp flag that supports dwo files for testing when there's an alternative (^) that we use for all other dwo testing

Thanks! Updated the test using `-fdebug-compilation-dir=.`, now --dwp is only for dwp file. Yeah, it have to cd into the test directory to make the lit work.



================
Comment at: llvm/test/tools/llvm-profgen/split-dwarf.test:18
+
+;CHECK-SPLIT-DWARF: bar:0:0
+;CHECK-SPLIT-DWARF:  1: 0
----------------
MaskRay wrote:
> 
Fixed, thanks!


================
Comment at: llvm/test/tools/llvm-profgen/split-dwarf.test:19
+;CHECK-SPLIT-DWARF: bar:0:0
+;CHECK-SPLIT-DWARF:  1: 0
+;CHECK-SPLIT-DWARF:  5: 0
----------------
MaskRay wrote:
> Add `-NEXT:` whenever applicable to have a better FileCheck diagnostic in case of an error. Tests should be written in the mind that others may need to update them.
Fixed, thanks!


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.cpp:674
   if (!DebugContext)
     exitWithError("Misssing debug info.", Path);
 
----------------
MaskRay wrote:
> wenlei wrote:
> > typo: Misssing->Missing
> https://llvm.org/docs/CodingStandards.html#error-and-warning-messages
Fixed, thanks!


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.cpp:677
   for (const auto &CompilationUnit : DebugContext->compile_units()) {
-    for (const auto &DieInfo : CompilationUnit->dies()) {
-      llvm::DWARFDie Die(CompilationUnit.get(), &DieInfo);
-
-      if (!Die.isSubprogramDIE())
-        continue;
-      auto Name = Die.getName(llvm::DINameKind::LinkageName);
-      if (!Name)
-        Name = Die.getName(llvm::DINameKind::ShortName);
-      if (!Name)
-        continue;
-
-      auto RangesOrError = Die.getAddressRanges();
-      if (!RangesOrError)
-        continue;
-      const DWARFAddressRangesVector &Ranges = RangesOrError.get();
+    loadSymbolsFromDWARFUnit(*CompilationUnit.get());
+  }
----------------
MaskRay wrote:
> https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
Fixed, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115973/new/

https://reviews.llvm.org/D115973