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

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added a comment.

Missing roundtrip textual and binary IR encoding test. And I think it won't pass because you are reading getUpper and writing getSignedMax (to textual) and those seem off-by-one.



================
Comment at: llvm/docs/LangRef.rst:6815
+where the first ``param`` is the number of the parameter it describes,
+``offset`` is the know access range of the paramenter inside of the function.
+
----------------
"known access range"


================
Comment at: llvm/include/llvm/Analysis/StackSafetyAnalysis.h:48
+  /// Argument use for a FunctionSummary.
+  std::vector<FunctionSummary::StackSafetyParam> getStackSafetyParams() const;
 };
----------------
vitalybuka wrote:
> do you have preference Params vs Args? 
Parameters. Arguments are concrete values passed to a function call.


================
Comment at: llvm/include/llvm/Bitcode/LLVMBitCodes.h:297
+  // [n x (paramno, range, numcalls, numcalls x (callee_guid, paramno, range))]
+  FS_STACK_SAFETY_PARAMS = 25,
 };
----------------
Should we call it something like ParamAccessRange here and everywhere? W/o stack safety in the name.



================
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
----------------
This can be safely reduced to 32 bits or even less in practice, if that would help with the binary size.


================
Comment at: llvm/lib/Analysis/StackSafetyAnalysis.cpp:702
+      Call.Callee = C.Callee->getGUID();
+      Call.Offsets = C.Offset;
+    }
----------------
Why not Arg.Calls.emplace_back(C.ParamNo, C.Callee->getGUID(), C.Offset) ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80908