[PATCH] D54504: [stack-safety] Local analysis implementation

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka added inline comments.


================
Comment at: llvm/lib/Analysis/StackSafetyAnalysis.cpp:135
+// more of its arguments.
+struct llvm::StackSafetyFunctionInfo {
+  // May be a Function or a GlobalAlias
----------------
vlad.tsyrklevich wrote:
> 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.
renamed
WDYT?


================
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.
----------------
vlad.tsyrklevich wrote:
> Is this comment outdated? I can't figure out what it's referring to, Maybe @eugenis knows.
This is correct but not optimal behavior. I've added FIXME: into "case Instruction::Ret:" and new test into IPA analysis


Repository:
  rL LLVM

https://reviews.llvm.org/D54504