This is an archive of the discontinued LLVM Phabricator instance.

[MemProf] Recognize hot/cold operator new as replaceable allocations
ClosedPublic

Authored by tejohnson on May 1 2023, 11:10 AM.

Details

Summary

Follow up to LLVM-side change to allow conversion to hot/cold hinted
operator new, not yet standard but supported by open source tcmalloc.
The LLVM change in a35206d78280e0ebcf48cd21bc5fff5c3b9c73fa added the
necessary support to recognize these as library functions, and we
subsequently found that a clang-side change is needed to give them
builtin/nobuiltin attributes as appropriate so they have consistent
removeability.

Based on discussion with Richard Smith, converted the parameter type to
a reserved identifier (39f7b48671dae5fbe3afc49f33f50c2b6340bb89) and
added support in this patch to recognize it in clang.

Diff Detail

Event Timeline

tejohnson created this revision.May 1 2023, 11:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 1 2023, 11:10 AM
tejohnson requested review of this revision.May 1 2023, 11:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 1 2023, 11:10 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
snehasish accepted this revision.May 1 2023, 11:29 AM

lgtm

clang/test/CodeGenCXX/new_hot_cold.cpp
17

Can we skip the typedef and just say enum class __hot_cold_t : unsigned char;?

Also it might be useful to link to the tcmalloc documentation for hot_cold_t to document the expected underlying type?
https://github.com/google/tcmalloc/blob/220043886d4e2efff7a5702d5172cb8065253664/tcmalloc/malloc_extension.h#L53

This revision is now accepted and ready to land.May 1 2023, 11:29 AM
tejohnson added inline comments.May 1 2023, 12:01 PM
clang/test/CodeGenCXX/new_hot_cold.cpp
17

Can we skip the typedef and just say enum class __hot_cold_t : unsigned char;?

I did that specifically to test the way it is declared in tcmalloc. In particular, this tests the TypedefType while loop at lines 3301-2 in Decl.cpp which was required to "see through" that and handle that case properly. I can add a comment here to the test. Will also add the link to tcmalloc here and in Decl.cpp

tejohnson updated this revision to Diff 518526.May 1 2023, 12:26 PM

Address comment

This revision was landed with ongoing or failed builds.May 1 2023, 1:38 PM
This revision was automatically updated to reflect the committed changes.