[PATCH] D34448: bug33527 - Linker::LinkOnlyNeeded should import AppendingLinkage globals

Benoit Belley via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 22 07:42:27 PDT 2017


belleyb marked 2 inline comments as done.
belleyb added inline comments.


================
Comment at: lib/LTO/ThinLTOCodeGenerator.cpp:35
 #include "llvm/LTO/LTO.h"
-#include "llvm/Linker/Linker.h"
 #include "llvm/MC/SubtargetFeature.h"
----------------
tejohnson wrote:
> belleyb wrote:
> > Linker isn't used by ThinLTO. Include can be removed.
> Can you submit this one as a separate NFC change (no patch review needed)? Thanks.
I'm not sure how to proceed as I don't have commit access myself. :-(


================
Comment at: unittests/Linker/LinkModulesTest.cpp:364
 
+TEST_F(LinkModuleTest, ImportIntrinsicGlobalVariables_ctors1) {
+  SMDiagnostic Err;
----------------
tejohnson wrote:
> belleyb wrote:
> > Starting from here are added unit tests demonstrating the actual issue and proving the suggested fix is correct.
> > 
> > I was wondering if it would have been more appropriate to write those using llvm-link and FileCheck instead. Let me know if you would prefer that alternate approach instead.
> The unit test is quite long - I typically add tests using FileCheck for this type of issue, if you can demonstrate the issue.
Sure, I'll try...


https://reviews.llvm.org/D34448





More information about the llvm-commits mailing list