[PATCH] D54543: [stack-safety] Inter-Procedural Analysis implementation

[Vlad Tsyrklevich via Phabricator via llvm-commits]

vlad.tsyrklevich added a comment.

The local tests that check for use 'empty-set, $CALL_INFO' for the local and 'full-set, $CALL_INFO' for the global analysis is sort of confusing. It seems like the use is safe in local analysis (empty-set means no uses!) but unsafe in the global, when really the situation is actually local analysis has 'no local uses but used in function calls' and the global analysis is just 'it's safe/unsafe (function call info is integrated into the previous distintion)'. Maybe we should clear all of the UseInfo::Calls fields at the end of StackSafetyDataFlowAnalysis?



================
Comment at: llvm/lib/Analysis/StackSafetyAnalysis.cpp:147
   explicit StackSafetyFunctionInfo(const Function *F) : GV(F){};
+  // Constructors that create a 'function' summary that forwards all the
+  // parameters to the aliasee.
----------------
s/'function' summary/FunctionInfo/


================
Comment at: llvm/lib/Analysis/StackSafetyAnalysis.cpp:599
+
+  // Without ThinLTO, run the local analysis for every function in the TU and
+  // then run the DFA and annotate allocas
----------------
Do we need this comment here and in StackSafetyDataFlowAnalysis?


================
Comment at: llvm/lib/Analysis/StackSafetyAnalysis.cpp:634
+bool StackSafetyGlobalInfoWrapperPass::runOnModule(Module &M) {
+  // Without ThinLTO, run the local analysis for every function in the TU and
+  // then run the DFA and annotate allocas
----------------
Do we need this comment here and in StackSafetyDataFlowAnalysis?


Repository:
  rL LLVM

https://reviews.llvm.org/D54543