[PATCH] D81242: [StackSafety] Run ThinLTO

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis added inline comments.


================
Comment at: llvm/lib/Analysis/StackSafetyAnalysis.cpp:618
+    ConstantRange Access = Found->sextOrTrunc(Use.Range.getBitWidth());
+    if (Access.signedAddMayOverflow(C.Offset) !=
+        ConstantRange::OverflowResult::NeverOverflows)
----------------
Do we have a test for this overflow check?


================
Comment at: llvm/lib/Analysis/StackSafetyAnalysis.cpp:924
+      if (FS)
+        FS->setParamAccesses({});
+    }
----------------
Why is this necessary? This affects only !live || !dso_local functions, right? The rest is overwritten in the loop below. As I understand, the post-lto function analysis ignores such functions, so that's strictly a space saving thing. Explain what's going on in a comment, and why removing this access data won't cause false positive results.


================
Comment at: llvm/lib/LTO/LTO.cpp:1396
 
+  processParamAccessSummary(ThinLTO.CombinedIndex);
+
----------------
Bad name - "process" is meaningless here. Consider "generateParamAccessSummary" ?


================
Comment at: llvm/test/Analysis/StackSafetyAnalysis/ipa-alias.ll:55
+; RUN:  -r %t.summ1.bc,Write1,px 2>&1 | FileCheck %s --check-prefixes=CHECK,GLOBAL,LTO
+
 target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
----------------
For my education - how do you come up with these symbol lists?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81242/new/

https://reviews.llvm.org/D81242