[PATCH] D80908: [StackSafety] Add info into function summary

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In D80908#2124996 <https://reviews.llvm.org/D80908#2124996>, @vitalybuka wrote:

> Thanks for feedback. Docs are here https://reviews.llvm.org/D82941


Saw that, thanks.

> I'll create a new patch for unique_ptr<> change

Sounds good.



================
Comment at: llvm/include/llvm/IR/ModuleSummaryIndex.h:556
 
+  static constexpr uint32_t StackSafetyRangeWidth = 64;
+  /// Describes the use of a value in a call instruction, specifying the call's
----------------
vitalybuka wrote:
> tejohnson wrote:
> > vitalybuka wrote:
> > > eugenis wrote:
> > > > This can be safely reduced to 32 bits or even less in practice, if that would help with the binary size.
> > > Writer/Parser push them as uint64 and smart encoded store them efficiently
> > > on StackSafety side i plan to cut-off large values, e.g. 2^32 but it's going to be a separate patch
> > If these will be max 32 bits can you use that here? Even though they will be compressed on serialization, I assume it would affect in-memory size.
> If you don't mind I'll fix "max 32" later, with other algorithm optimizations.
> 
Sure, it doesn't affect the common case where this is empty, so doing as a follow on sounds fine.


================
Comment at: llvm/include/llvm/IR/ModuleSummaryIndex.h:625
+  /// Uses for every parameter to this function.
+  std::vector<ParamAccess> ParamAccesses;
+
----------------
vitalybuka wrote:
> tejohnson wrote:
> > Since this will only be used under stack safety (which I assume is optional and uncommon?), might this be better as a unique_ptr like we do for type id info declared just above? The overhead of an empty vector is going to be higher (3x on my system), which will add up in the combined summary.
> I'll create a separate patch for this tomorrow.
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80908