[PATCH] D26220: Emit one CodeView S_COMPILE3 record per TU rather than per function

[Reid Kleckner via llvm-commits]

rnk added a comment.

Looks good, mostly a bunch of comments about how to make the tests less fragile.



================
Comment at: test/DebugInfo/COFF/asm.ll:29
 ; X86-NEXT: .long   241
 ; X86-NEXT: .long [[F1_END:.*]]-[[F1_START:.*]] #
 ; Compiler information record
----------------
I think this test is really supposed to be about the line table. I think we should keep the .cv_loc directive matching above, but delete all this .debug$S directive matching to make the test less fragile.

See the FIXME comment below and http://llvm.org/PR18679. I think that explains why this test is here.


================
Comment at: test/DebugInfo/COFF/asm.ll:69-74
 ; OBJ32:      Relocations [
-; OBJ32-NEXT:   0x59 IMAGE_REL_I386_SECREL _f
-; OBJ32-NEXT:   0x5D IMAGE_REL_I386_SECTION _f
-; OBJ32-NEXT:   0x70 IMAGE_REL_I386_SECREL _f
-; OBJ32-NEXT:   0x74 IMAGE_REL_I386_SECTION _f
+; OBJ32-NEXT:   0x64 IMAGE_REL_I386_SECREL _f
+; OBJ32-NEXT:   0x68 IMAGE_REL_I386_SECTION _f
+; OBJ32-NEXT:   0x7C IMAGE_REL_I386_SECREL _f
+; OBJ32-NEXT:   0x80 IMAGE_REL_I386_SECTION _f
 ; OBJ32-NEXT: ]
----------------
Let's delete the relocation matching. They aren't interesting in this test.


================
Comment at: test/DebugInfo/COFF/asm.ll:134-137
+; X64-NEXT: [[F1_END]]:
+; X64-NEXT: .p2align	2
+; X64-NEXT: .long	241                     # Symbol subsection for f
+; X64-NEXT: .long [[SUBSEC_END:.*]]-[[SUBSEC_START:.*]] # Subsection size
----------------
Ditto, I don't think we need x64 .debug$S assembly matching, just line table matching below.


================
Comment at: test/DebugInfo/COFF/multifile.ll:35
 ; X86-NEXT: .p2align 2
 ; X86-NEXT: .long   4
 ; Symbol subsection
----------------
This test is also entirely concerned with what happens to the line table when you use #line inside a function. I think we can do the same here: delete the .debug$S assembly matching portion and relocation entries.

The FilenameSegment checks below are the interesting part.


================
Comment at: test/DebugInfo/COFF/multifunction.ll:58
 ; X86-NEXT: .long   241
 ; X86-NEXT: .long [[F1_END:.*]]-[[F1_START:.*]] #
 ; Compiler information record
----------------
This is supposed to test how we emit one symbol subsection per function, so I think it's worth matching the compile info subsection.

The names F1_START and F1_END were intended to refer to the symbol subsection for x, but this is actually the compile info subection now. Let's use F1_START / F1_END in place of SUBSEC_START / SUBSEC_END and use COMPILE_START / COMPILE_END here.


================
Comment at: test/DebugInfo/COFF/multifunction.ll:146-158
 ; OBJ32:      Relocations [
-; OBJ32-NEXT:   0x59 IMAGE_REL_I386_SECREL _x
-; OBJ32-NEXT:   0x5D IMAGE_REL_I386_SECTION _x
-; OBJ32-NEXT:   0x70 IMAGE_REL_I386_SECREL _x
-; OBJ32-NEXT:   0x74 IMAGE_REL_I386_SECTION _x
-; OBJ32-NEXT:   0xF5 IMAGE_REL_I386_SECREL _y
-; OBJ32-NEXT:   0xF9 IMAGE_REL_I386_SECTION _y
-; OBJ32-NEXT:   0x10C IMAGE_REL_I386_SECREL _y
-; OBJ32-NEXT:   0x110 IMAGE_REL_I386_SECTION _y
-; OBJ32-NEXT:   0x191 IMAGE_REL_I386_SECREL _f
-; OBJ32-NEXT:   0x195 IMAGE_REL_I386_SECTION _f
-; OBJ32-NEXT:   0x1A8 IMAGE_REL_I386_SECREL _f
-; OBJ32-NEXT:   0x1AC IMAGE_REL_I386_SECTION _f
+; OBJ32-NEXT:   0x64 IMAGE_REL_I386_SECREL _x
+; OBJ32-NEXT:   0x68 IMAGE_REL_I386_SECTION _x
+; OBJ32-NEXT:   0x7C IMAGE_REL_I386_SECREL _x
+; OBJ32-NEXT:   0x80 IMAGE_REL_I386_SECTION _x
+; OBJ32-NEXT:   0xD4 IMAGE_REL_I386_SECREL _y
+; OBJ32-NEXT:   0xD8 IMAGE_REL_I386_SECTION _y
----------------
I don't think the relocation entries are interesting anymore, let's delete them.


================
Comment at: test/DebugInfo/COFF/simple.ll:28
 ; X86-NEXT: .long   241
 ; X86-NEXT: .long [[F1_END:.*]]-[[F1_START:.*]] #
 ; Compiler information record
----------------
Let's update the names to COMPILE_START/END and use F1_START/END below.


https://reviews.llvm.org/D26220