Creates bitcode files suitable for use with ThinLTO, then checks that the linker can build an executable from them.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Maybe the testing is obvious to someone use to COFF, in which case I'm just out-of-place :)
Otherwise I'd rather have comment explaining what every check is doing.
test/COFF/thinlto.ll | ||
---|---|---|
9 ↗ | (On Diff #89430) | is .obj the standard extension for object on Windows? (and .lib for archive?) |
11 ↗ | (On Diff #89430) | I don't know what is the /include directive doing? Is it preventing dead-stripping somehow? Or affecting the symbol resolution otherwise? |
34 ↗ | (On Diff #89430) | I don't know enough about COFF to know if what you're testing is relevant here. Especially I have no idea why you're checking the HEADERS (or what you're checking). |
106 ↗ | (On Diff #89430) | Why do we check this exact sequence of assembly? Overall I don't know what this test is enforcing / verifying right now. |
For this test I wouldn't expect to see anything more than a basic test that uses temporaries to verify that importing is working. See http://llvm-cs.pcc.me.uk/tools/lld/test/ELF/lto/thinlto.ll for example.
See also r295901 which changes the ELF test to be closer to what I'd expect this test to be doing.
test/COFF/thinlto.ll | ||
---|---|---|
9 ↗ | (On Diff #89430) | Yes. |
11 ↗ | (On Diff #89430) | Right. It instructs the linker to retain that symbol, regardless of what it would otherwise do. |
106 ↗ | (On Diff #89430) | This was essentially performing the same checks as lto.ll. However, per @pcc's suggestion, I've changed the test to be much simpler and not actually look at the disassembly. |
Nit: I'd use llvm-nm to check the symbol tables of the object files so that we're checking the inputs that the linker actually reads, rather than a byproduct of LTO. Other than that, LGTM.
test/COFF/thinlto.ll | ||
---|---|---|
106 ↗ | (On Diff #89430) | Thanks, looks good! |
Nit: I'd use llvm-nm to check the symbol tables of the object files so that we're checking the inputs that the linker actually reads, rather than a byproduct of LTO.
That was my original plan, but it looks like llvm-nm only tells us about main. Is that enough? Is there a way to get it to give us more information? I wrote the test the way I wrote it because I felt just looking at the output from llvm-nm doesn't really tell us if ThinLTO is working as intended.
Do you mean that main.exe.lto.obj only contains a definition of main? That is what I'd expect to see if foo was imported into main and inlined.
test/COFF/thinlto.ll | ||
---|---|---|
8 ↗ | (On Diff #89554) | assuming llvm-nm on COFF looks like ELF, what you need to check is not that main is present, but that foo isn't. |
test/COFF/thinlto.ll | ||
---|---|---|
8 ↗ | (On Diff #89554) | ; CHECK-NOT: U foo? |