[PATCH] D63662: Changing CodeView Debug info Type record output

[NILANJANA BASU via Phabricator via llvm-commits]

nilanjana_basu added inline comments.


================
Comment at: llvm/lib/DebugInfo/CodeView/TypeRecordMapping.cpp:269
   uint16_t Size;
-  if (IO.isWriting()) {
+  if (!IO.isReading()) {
     ArrayRef<VFTableSlotKind> Slots = Record.getSlots();
----------------
dblaikie wrote:
> What else can you be doing other than reading or writing? (what is the semantic difference in this (& similar) code change?)
There are 3 modes - reading (for deserializing), writing (for serializing), & streaming (which uses MCStreamer to emit directives to the assembly file). 


================
Comment at: llvm/test/DebugInfo/COFF/asm.ll:2
 ; RUN: llc -mcpu=core2 -mtriple=i686-pc-win32 -O0 < %s | FileCheck --check-prefix=X86 %s
+; RUN: llc -mcpu=core2 -mtriple=i686-pc-win32 -o - -O0 -filetype=obj < %s | llvm-readobj -S --sr --codeview | FileCheck --check-prefix=OBJ32 %s
 ; RUN: llc -mcpu=core2 -mtriple=i686-pc-win32 -o - -O0 < %s | llvm-mc -triple=i686-pc-win32 -filetype=obj | llvm-readobj -S --sr --codeview | FileCheck --check-prefix=OBJ32 %s
----------------
dblaikie wrote:
> Seems like overkill to double all these tests, perhaps?
These tests emit & check the same end result, but they go through different paths of the code. One creates the code view record from the object file directly, & checks it. The other one creates the assembly file (the current changes get executed in this path), converts it to object file, & then checks the code view record from it. This is to double check that both the newly added code, & the original code path work correctly. Also, when I run all the tests in COFF using llvm-lit, there is a delay of 0.6s added (originally runtime 1.4s, now takes 2s).


================
Comment at: llvm/test/DebugInfo/COFF/types-basic.ll:515
+; ASM: .short	4611
+; ASM: .ascii	"\r\025\003\000t\000\000\000\000\000a\000\021\025\003\000\t\020\000\000A::f\000\363\362\361"
+; ASM: # FieldList (0x100A) {
----------------
rnk wrote:
> Hm, we should do something about LF_FIELDLIST. The members should get directives. Maybe that's out of scope for this change. I think it's already an improvement.
I changed it to the old style of emitting a set of bytes. I can improve this later in another commit.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63662