[PATCH] D81242: [StackSafety] Run ThinLTO

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

In D81242#2188467 <https://reviews.llvm.org/D81242#2188467>, @tejohnson wrote:

> In D81242#2188441 <https://reviews.llvm.org/D81242#2188441>, @vitalybuka wrote:
>
>> In D81242#2183383 <https://reviews.llvm.org/D81242#2183383>, @tejohnson wrote:
>>
>>> Is the stack safety analysis meant to be always on with ThinLTO?
>>
>> During compilation most of the time it should be off.
>> However during linking I assume that most build FS->paramAccesses() is empty, so no hash lookup is expected. So I assume empty looks should be cheap:
>>
>>   for (auto &GVS : Index) {
>>       for (auto &GV : GVS.second.SummaryList) {
>>   }}
>>
>> As it paramAccesses suppose to be non-empty for MTE builds for now, so if it's not empty on internal build, then the bug it likely around why it's not empty there.
>> Can you send me email with internal build details?
>
> Will do

Thanks, I was able to reproduce.
As expected hash table lookup was not reached. However code inserts empty FunctionInfo<FunctionSummary> into std::map Functions. Which does not affect correctness, but totally unnecessary and noticeable on profile.
https://reviews.llvm.org/rG08cf49658c1d resolves the issue.

I'll still check if we can replace guids with ValueInfo to optimize case when StackSafety is active.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81242