[PATCH] D54504: [stack-safety] Local analysis implementation
Vlad Tsyrklevich via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 15 06:47:21 PST 2018
vlad.tsyrklevich added inline comments.
================
Comment at: llvm/lib/Analysis/StackSafetyAnalysis.cpp:119
+
+uint64_t getStaticAllocaAllocationSize(const AllocaInst *AI) {
+ const DataLayout &DL = AI->getModule()->getDataLayout();
----------------
Would be worth copying over the comment from the SafeStack pass:
/// Calculate the allocation size of a given alloca. Returns 0 if the
/// size can not be statically determined.
================
Comment at: llvm/lib/Analysis/StackSafetyAnalysis.cpp:133
+
+// FunctionStackSummary could also describe return value as depending on one or
+// more of its arguments.
----------------
Mark this a TODO and update the struct name.
================
Comment at: llvm/lib/Analysis/StackSafetyAnalysis.cpp:135
+// more of its arguments.
+struct llvm::StackSafetyFunctionInfo {
+ // May be a Function or a GlobalAlias
----------------
This uses a different naming convention than the structs above though it's part of the same 'type set' as the *Info structs above which is confusing. Couldn't immediately think of a fix that makes it nicer though. Might just be worth a comment to at least be explicit about why this is.
================
Comment at: llvm/lib/Analysis/StackSafetyAnalysis.cpp:321
+ }
+ // Don't visit function return value: if it depends on the alloca, then
+ // argument range would be full-set.
----------------
Is this comment outdated? I can't figure out what it's referring to, Maybe @eugenis knows.
Repository:
rL LLVM
https://reviews.llvm.org/D54504
More information about the llvm-commits
mailing list