[StackSafety,NFC] Update documentation

[vitalybuka]

Summary

It's follow up for D80908

[vitalybuka]

format

[vitalybuka]

fixes

[tejohnson]

Thanks, a few follow on questions and nits below.

llvm/docs/LangRef.rst

6857

nit: s/Then/The/

I assume if it is passed directly to another call and we don't know the accesses within that callee (this is the per-module summary) that it would have a calls entry with offset [0,0]?

6860

nit: "increments or decrements the parameter pointer and passes it.."

llvm/include/llvm/IR/ModuleSummaryIndex.h

557

nit: s/in in/in/

The "by all of the call targets it is passed to" sounds ambiguous to me, as if we already have knowledge of how it is accessed within those callees. I guess this is the situation after the thin link, but not initially.

Probably best to be more explicit. I.e., from my understanding reading your LangRef writeup:

• In the per-module summary, the Calls vector summarize the byte offset applied to each pointer parameter before passing to each corresponding callee. I.e. this structure describes offsets computed from the pointer parameter within the function only
• In the combined summary, the offsets may include the offsets within each called function consuming this pointer parameter. Two questions: I assume that happens after some kind of inter-procedural propagation across the combined summary? After that propagation, is the Calls list empty?

[vitalybuka]

update

[vitalybuka]

llvm/docs/LangRef.rst

6857

I assume if it is passed directly to another call and we don't know the accesses within that callee (this is the per-module summary) that it would have a calls entry with offset [0,0]?

calls list is only needed when we don't know accesses inside of callee. If we know accesses we can directly apply them to offset of parameter (offset: [5, 5])
if we don't know accesses we still know offset used to pass the parameter
I've added function body into the example.

llvm/include/llvm/IR/ModuleSummaryIndex.h

557

I moved this description close to fields.

[vitalybuka]

update

[tejohnson]

LGTM, thanks!

llvm/docs/LangRef.rst

 where each ``Param`` describes pointer parameter access inside of the
 function and looks like:
 
 .. code-block:: text
 
     param: 4, offset: [0, 5][, calls: ((Callee)[, (Callee)]*)]?
 
 where the first ``param`` is the number of the parameter it describes,
-``offset`` is the known access range of the paramenter inside of the function.
+``offset`` is the inclusive range of offsets from the pointer parameter to bytes
+which can be accessed by the function. This range does not include accesses by
+function calls from ``calls`` list.
 
 where each ``Callee`` decribes how parameter is forwared into other
 functions and looks like:
 
 .. code-block:: text
 
     callee: ^3, param: 5, offset: [-3, 3]
 
 The ``callee`` refers to the summary entry id of the callee,  ``param`` is
 the number of the callee parameter which points into the callers parameter
-with offset known to be inside of the ``offset`` range.
+with offset known to be inside of the ``offset`` range. ``calls`` will be
+consumed and removed by thin link stage to update ``Param::offset`` so it
+covers all accesses possible by ``calls``.
+
+Pointer parameter without corresponding ``Param`` is considered unsafe and we
+assume that access with any offset is possible.
+
+Example:
+
+.. code-block:: text
+
+    params: ((param: 2, offset: [0, 15]),(param: 3, offset: [5, 5], calls: ((callee: ^3, param: 5, offset: [-1, 1]))))
+
+Parameter #0 and #1 are either not pointers or access is unknown. Parameter #2
+will not be accessed outside of 16 bytes starting from the byte pointed by the
+parameter. ``calls`` is empty, so the parameter is either not used for function
+calls or ``offset`` already covers all accesses from nested function calls. Then
+function itself can access just a single byte of the parameter #3. Additional
+access is possible when the function calls ``^3``. The function increment or
+decrement parameter pointer and pass it as the argument #5 into ``^3``. This
+record itself does not tell us how ``^3`` will access the parameter.
 
 .. _refs_summary:
 
 Refs
 ^^^^
 
 The optional ``Refs`` field looks like:
 

llvm/include/llvm/Analysis/StackSafetyAnalysis.h

public:
   ~StackSafetyInfo();
 
   const InfoTy &getInfo() const;
 
   // TODO: Add useful for client methods.
   void print(raw_ostream &O) const;
 
   /// Parameters use for a FunctionSummary.
+  /// Function collects access information of all pointer parameters.
+  /// Information includes a range of direct access of parameters by the
+  /// functions and all call sites accepting the parameter.
+  /// StackSafety assumes that missing parameter information means possibility
+  /// of access to the parameter with any offset, so we can correctly link
+  /// code without StackSafety information, e.g. non-ThinLTO.
   std::vector<FunctionSummary::ParamAccess> getParamAccesses() const;
 };
 
 class StackSafetyGlobalInfo {
 public:
   struct InfoTy;
 
 private:

llvm/include/llvm/IR/ModuleSummaryIndex.h

struct FFlags {
     unsigned ReturnDoesNotAlias : 1;
 
     // Indicate if the global value cannot be inlined.
     unsigned NoInline : 1;
     // Indicate if function should be always inlined.
     unsigned AlwaysInline : 1;
   };
 
-  /// Describes the uses of a parameter by the range of offsets accessed in the
-  /// function and all of the call targets it is passed to.
+  /// Describes the uses of a parameter by the range of byte offsets of direct
+  /// access in in the function and by all of the call targets it is passed to.
   struct ParamAccess {
     static constexpr uint32_t RangeWidth = 64;
 
     /// Describes the use of a value in a call instruction, specifying the
     /// call's target, the value's parameter number, and the possible range of
     /// offsets from the beginning of the value that are passed.
     struct Call {
       uint64_t ParamNo = 0;
       GlobalValue::GUID Callee = 0;
-      ConstantRange Offsets{RangeWidth, true};
+      ConstantRange Offsets{/*BitWidth=*/RangeWidth, /*isFullSet=*/true};
 
       Call() = default;
       Call(uint64_t ParamNo, GlobalValue::GUID Callee,
            const ConstantRange &Offsets)
           : ParamNo(ParamNo), Callee(Callee), Offsets(Offsets) {}
     };
 
     uint64_t ParamNo = 0;
-    ConstantRange Use{RangeWidth, true};
+    ConstantRange Use{/*BitWidth=*/RangeWidth, /*isFullSet=*/true};
     std::vector<Call> Calls;
 
     ParamAccess() = default;
     ParamAccess(uint64_t ParamNo, const ConstantRange &Use)
         : ParamNo(ParamNo), Use(Use) {}
   };
 
   /// Create an empty FunctionSummary (with specified call edges).

llvm/lib/Analysis/StackSafetyAnalysis.cpp

for (auto &FnKV : Info->Info) {
       }
     }
     if (StackSafetyPrint)
       print(errs());
   }
   return *Info;
 }
 
-// Converts a StackSafetyFunctionInfo to the relevant FunctionSummary
-// constructor fields
 std::vector<FunctionSummary::ParamAccess>
 StackSafetyInfo::getParamAccesses() const {
+  // Implementation transforms internal representation of parameter information
+  // into FunctionSummary format.
   std::vector<FunctionSummary::ParamAccess> ParamAccesses;
   for (const auto &KV : getInfo().Info.Params) {
     auto &PS = KV.second;
+    // Parameter accessed by any or unknown offset, represented as FullSet by
+    // StackSafety, is handled as the parameter for which we have no
+    // StackSafety info at all. So drop it to reduce summary size.
     if (PS.Range.isFullSet())
       continue;
 
     ParamAccesses.emplace_back(KV.first, PS.Range);
     FunctionSummary::ParamAccess &Param = ParamAccesses.back();
 
     Param.Calls.reserve(PS.Calls.size());
     for (auto &C : PS.Calls) {
+      // Parameter forwarded into another function by any or unknown offset
+      // will make ParamAccess::Range as FullSet anyway. So we can drop the
+      // entire parameter like we did above.
+      // TODO(vitalybuka): Return already filtered parameters from getInfo().
       if (C.Offset.isFullSet()) {
         ParamAccesses.pop_back();
         break;
       }
       Param.Calls.emplace_back(C.ParamNo, C.Callee->getGUID(), C.Offset);
     }
   }
   return ParamAccesses;