[PATCH] D62701: [PDB] Copy inlinee lines records into the PDB

Alexandre Ganea via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 31 13:35:26 PDT 2019


aganea added inline comments.


================
Comment at: lld/test/COFF/pdb-inlinees-extrafiles.s:6
+
+# The assembly was hand written to model the following C code. As of this
+# writing, clang does not emit extra files for inlinees, so it had to be hand
----------------
rnk wrote:
> aganea wrote:
> > Why not:
> > ```
> > $ cl /Z7 /c /O2 t.c
> > $ obj2yaml t.obj >t.yaml
> > ```
> > Which changes the test to:
> > ```
> > # REQUIRES: x86
> > # RUN: yaml2obj %s -o=%t.obj
> > # RUN: lld-link -entry:main -nodefaultlib %t.obj -out:%t.exe -pdb:%t.pdb -debug
> > # RUN: llvm-pdbutil dump -il %t.pdb | FileCheck %s
> > ```
> > I think it should be made clear/easy to re-generate the tests, if we want to change it, or if someone wants to duplicate it.
> > 
> Basically it boils down to, which do we think is a more useful test format, YAML or assembly? My preference is for assembly, and I'd like to replace a lot of the .test YAML inputs with .s inputs. Maybe that's unique to me, but it mirrors the direction the ELF linker took, where they moved away from YAML object input tests to assembly tests.
> 
> I can't generate assembly from MSVC, so I started with clang assembly output, and modified it to exercise the corner case in question.
Would there be a way to better represent "structured data" in the assembly below? Can you show a mock of what your intern is doing?

If you choose to go the ELF way for coherence, that is justified. Was the move in ELF because of file size concerns?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62701





More information about the llvm-commits mailing list