This is an archive of the discontinued LLVM Phabricator instance.

[StackSafety,NFC] Update documentation
ClosedPublic

Authored by vitalybuka on Jul 1 2020, 2:55 AM.

Diff Detail

Event Timeline

vitalybuka created this revision.Jul 1 2020, 2:55 AM
Herald added a project: Restricted Project. · View Herald Transcript

Thanks, a few follow on questions and nits below.

llvm/docs/LangRef.rst
6855

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]?

6858

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 updated this revision to Diff 276624.Jul 8 2020, 7:59 PM
vitalybuka marked 4 inline comments as done.

update

vitalybuka marked an inline comment as done.Jul 8 2020, 7:59 PM
vitalybuka added inline comments.
llvm/docs/LangRef.rst
6855

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 updated this revision to Diff 276626.Jul 8 2020, 8:01 PM
vitalybuka marked an inline comment as done.

update

Harbormaster completed remote builds in B63531: Diff 276626.
tejohnson accepted this revision.Jul 8 2020, 8:43 PM

LGTM, thanks!

This revision is now accepted and ready to land.Jul 8 2020, 8:43 PM
This revision was automatically updated to reflect the committed changes.