[PATCH] D36850: [ThinLTO] Add norecurse function attribute propagation

[Charles Saternos via Phabricator via llvm-commits]

ncharlie added inline comments.


================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:434
+        [&F](const FunctionSummary::EdgeTy &E) {
+          if (E.first.getGUID() == 0 || !E.first.getSummaryList().size())
+            return true; // might recurse - we can't reason about external
----------------
tejohnson wrote:
> ncharlie wrote:
> > This is currently failing a test case that uses printf. Since printf doesn't have a summary associated with it (i.e. the SummaryList is empty), I can't determine if it recurses and have to fail out early.
> > 
> > Is there some spot I should add code to create a FunctionSummary for external functions?
> How is printf handled in the full LTO case? Does it have attributes indicating that it is no recurse? Otherwise I think treating it conservatively is the only option.
In FullLTO it's able to determine that printf doesn't recurse by analyzing the callsite (http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/FunctionAttrs.cpp#1068). Maybe I could modify ModuleSummaryAnalysis so it adds inserts the flags into the FunctionSummary while it still has access to the CallSite?


https://reviews.llvm.org/D36850