[PATCH] D49434: Put "built-in" function definitions in global Used list, for LTO. (fix bug 34169)

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: test/ThinLTO/X86/builtin-nostrip.ll:1
+; REQUIRES: x86-registered-target
+; Compile with thinlto indices, to enable thinlto.
----------------
pcc wrote:
> A lot of this test case can be removed I think.
> - You just need one libcall function, testing more than one wouldn't give you additional coverage.
> - A lot of the test input (including the second input file) is irrelevant to what you're trying to test. Basically I think this test just needs two functions, both with resolution `p`, one which is a libcall and one which isn't, and you would test that the first one is kept and the second one isn't.
> - Remove the parts about the old LTO API because the problem is solved in a different way there.
Can you simplify the test case as I suggested? I think the first two bullet points should still apply.


https://reviews.llvm.org/D49434