[PATCH] D41489: [ThinLTO] Don't try to import noinline functions

Teresa Johnson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 21 08:04:58 PST 2017


tejohnson added a comment.

Thanks! Couple suggestions.



================
Comment at: lib/Analysis/ModuleSummaryAnalysis.cpp:309
       // FIXME: refactor this to use the same code that inliner is using.
       F.isVarArg();
   GlobalValueSummary::GVFlags Flags(F.getLinkage(), NotEligibleForImport,
----------------
Move it up here and just OR into this bool.


================
Comment at: test/ThinLTO/X86/noinline.ll:11
+
+; CHECK-NOT: bar
+
----------------
It would be better to check for the non-import of foo directly. E.g. pass -save-temps to llvm-lto2, then ensure a define of foo does not exist in the llvm-dis of the resulting %t1.bc.imports file.

This would enable you to simplify the test - no need to have bar at all. Also the test will still work as expected in a possible future where we might decide to import bar as a local copy to avoid the promotion.


Repository:
  rL LLVM

https://reviews.llvm.org/D41489





More information about the llvm-commits mailing list