Page MenuHomePhabricator

[MemProf] Don't instrument stack accesses unless requested
ClosedPublic

Authored by tejohnson on Sep 15 2021, 10:09 PM.

Details

Summary

Skip stack accesses unless requested, as the memory profiler runtime
does not currently look at or report accesses for these addresses.

Diff Detail

Event Timeline

tejohnson created this revision.Sep 15 2021, 10:09 PM
tejohnson requested review of this revision.Sep 15 2021, 10:09 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 15 2021, 10:09 PM
MaskRay added inline comments.
llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
459

the coding standard prefer pre-increment.

llvm/test/Instrumentation/HeapProfiler/stack.ll
5

Legacy PM tests are being deleted. For new tests, just avoid them.

tejohnson updated this revision to Diff 372963.Sep 16 2021, 8:38 AM

Address comments

tejohnson marked 2 inline comments as done.Sep 16 2021, 8:39 AM
tejohnson added inline comments.
llvm/test/Instrumentation/HeapProfiler/stack.ll
5

I see. Just skimmed the legacy PM deprecation email and saw note about this. Might be worth a wider announcement?

snehasish added inline comments.Sep 16 2021, 9:08 AM
llvm/lib/Transforms/Instrumentation/MemProfiler.cpp
129

Perhaps call this memprof-instrument-stack, the usage will be clear from the name itself?

Also I think moving this to L110 is better since the comment at L111 seems to indicate all flags following it are debug flags.

tejohnson updated this revision to Diff 372981.Sep 16 2021, 9:58 AM
tejohnson marked an inline comment as done.

Address comments

tejohnson marked an inline comment as done.Sep 16 2021, 9:58 AM
This revision is now accepted and ready to land.Sep 16 2021, 10:30 AM