Skip to content

Write out file names to GCOV file#244

Merged
snehasish merged 7 commits intogoogle:masterfrom
dc03-work:gcov-file-names
Jan 14, 2026
Merged

Write out file names to GCOV file#244
snehasish merged 7 commits intogoogle:masterfrom
dc03-work:gcov-file-names

Conversation

@dc03-work
Copy link
Contributor

@dc03-work dc03-work commented Jun 18, 2025

This patch modifies the profile readers and writers to support writing out the file names for functions into the GCOV file to enable using file names for more accurate profile annotation in GCC's auto-profile pass. This patch goes along with the RFC posted to the GCC mailing lists at https://gcc.gnu.org/pipermail/gcc-patches/2025-June/686835.html.

This also adds support for GCOV version 3, to support the breaking change in the file format.

@snehasish
Copy link
Collaborator

snehasish commented Jun 30, 2025

@erozenfeld can you take a look?

@erozenfeld
Copy link
Contributor

erozenfeld commented Jul 1, 2025

@dc03-work I think it would be better to make this backwards compatible. I.e., if the user specifies --gcov_version=2, it shouldn't read/write file names. Some people are using AutoFDO with older versions of GCC and it would be good if they can continue to build create_gcov from the latest in this repo.

@dc03-work
Copy link
Contributor Author

dc03-work commented Jul 3, 2025

I've changed the code to enable file names only for versions >= 3 (for future compatibility). Have not tested extensively but it seems to work okay.

@janhubicka
Copy link

janhubicka commented Jul 4, 2025

I looked into llvm and it solves the problem by making symbol names in debug info unique. I think it is better solution so will write bit more about how it works.

@snehasish
Copy link
Collaborator

snehasish commented Jul 7, 2025

@janhubicka Are you referring to -funique-internal-linkage-names? If so, we have faced issues with this flag where assumptions on the stability of the linkage name led to failures (e.g. in the linux kernel). LLVM is currently implementing a new guid format to assist profile mapping which avoids these issues. You can find more information here: https://discourse.llvm.org/t/rfc-keep-globalvalue-guids-stable/84801

@janhubicka
Copy link

janhubicka commented Jul 8, 2025

@janhubicka Are you referring to -funique-internal-linkage-names? If so, we have faced issues with this flag where assumptions on the stability of the linkage name led to failures (e.g. in the linux kernel). LLVM is currently implementing a new guid format to assist profile mapping which avoids these issues. You can find more information here: https://discourse.llvm.org/t/rfc-keep-globalvalue-guids-stable/84801

Thanks for pointing this out. Only when implementing -funique-internal-linkage-names, I noticed that it changes symbol names, not only DW_AT_linkage_name of the debug info. I also worried about top-level asm statements as well as the need to process much longer symbol names everywhere, not only in Auto-FDO.

GCC has profile-id which is used for indirect call profiling under -fprofile-generate. It is 64bit hash based on symbol name for public symbols and sourcename:symbol_name for private symbols. Some work was done to make it stable across builds. So it seems mostly identical to GUIDS discussed above. What is the status of the GUIDS proposal and how exactly it is planned to be plumbed into auto-FDO?

@snehasish
Copy link
Collaborator

snehasish commented Jul 8, 2025

The work is ongoing with no fundamental blockers (latest pr). For use by AutoFDO, we plan to store it in a separate section in the binary. From the RFC --

A section will be added to the binary which associates symbols with their GUID, to enable sample-based profiling to use the same GUIDs. This will cost 8 bytes per symbol (but note however that for sample PGO, removing the existing metadata / name mangling will save at least that much).

@shenhanc78
Copy link
Collaborator

shenhanc78 commented Oct 14, 2025

Hi, since this is an ongoing review, I'll merge this once @snehasish or @erozenfeld LGTMs this. Thanks.

@erozenfeld
Copy link
Contributor

erozenfeld commented Oct 20, 2025

I'll defer to @janhubicka and @snehasish on this one. It looks the discussion was leaning towards an approach different from what's in this PR.

@dc03-work
Write out file names to GCOV file
202337a 
This patch modifies the profile readers and writers to support writing
out the file names for functions into the GCOV file to enable using
file names for more accurate profile annotation in GCC's auto-profile
pass. This patch goes along with the RFC posted to the GCC mailing lists at
https://gcc.gnu.org/pipermail/gcc-patches/2025-June/686835.html.

This also adds support for GCOV version 3, to support the breaking
change in the file format.
@dc03-work
Read and write file name information only for GCOV version 3
2c4f834
@dc03-work
Fix crash when function name is NULL
8545e1f
@dc03-work
Write full file path to GCOV if present
5037520
@dc03-work
Simplify version comparison
f80a8e3
@dc03-work
Fix incomplete file paths being written out to GCOV
81c98c3 
This was being caused due to DW_AT_comp_dir not being prepended
before DW_AT_decl_file.
@dc03-work
Copy link
Contributor Author

dc03-work commented Dec 23, 2025

@erozenfeld The corresponding GCC patches have been committed as:

Can we please land this patch (if there are no immediate issues)? AutoFDO on GCC won't be usable without this.

@dc03-work dc03-work mentioned this pull request Dec 23, 2025
Merged
hubot pushed a commit to gcc-mirror/gcc that referenced this pull request Dec 23, 2025
@dc03-work
AutoFDO: Implement streaming of file names from GCOV profile
bbac517 
This patch requires the changes from google/autofdo#244
to work correctly.

Signed-off-by: Dhruv Chawla <dhruvc@nvidia.com>

gcc/ChangeLog:

	* auto-profile.cc (AUTO_PROFILE_VERSION): Bump to 3.
	(class function_instance_descriptor): New class.
	(get_normalized_path): New function.
	(string_table::~string_table): Update to free filenames.
	(string_table::vector_): Rename to ...
	(string_table::symbol_names_): ... this.
	(string_table::map_): Rename to ...
	(string_table::symbol_name_map_): ... this.
	(string_table::filenames_): New member.
	(string_table::filename_map_): Likewise.
	(string_table::symbol_to_filename_map_): Likewise.
	(string_table::get_index): Update to lookup symbol_name_map_.
	(string_table::get_name): Rename to ...
	(string_table::get_symbol_name): ... this.
	(string_table::add_name): Rename to ...
	(string_table::add_symbol_name): ... this.
	(string_table::get_filename): New function.
	(string_table::get_filename_by_symbol): Likewise.
	(string_table::get_filename_index): Likewise.
	(string_table::add_filename): Likewise.
	(string_table::read): Read file names from the GCOV profile.
	(function_instance::offline): Call
	get_function_instance_by_descriptor.
	(string_table::get_cgraph_node): Call get_symbol_name and
	symbol_name.
	(function_instance::get_function_instance_by_decl): Likewise.
	(function_instance::get_cgraph_node): Likewise.
	(function_instance::merge): Likewise.
	(match_with_target): Likewise.
	(function_instance::match): Likewise.
	(function_instance::dump): Likewise.
	(function_instance::dump_inline_stack): Likewise.
	(function_instance::find_icall_target_map): Likewise.
	(autofdo_source_profile::offline_unrealized_inlines): Likewise.
	(autofdo_source_profile::offline_external_functions): Likewise.
	(function_instance::read_function_instance): Likewise.
	(afdo_indirect_call):
	Also call find_function_instance, add_function_instance and
	remove_function_instance.
	(autofdo_source_profile::read): Likewise.
	(autofdo_source_profile::get_function_instance_by_decl): Call
	find_function_instance.
	(autofdo_source_profile::get_function_instance_by_name_index):
	Rename to ...
	(autofdo_source_profile::get_function_instance_by_descriptor):
	... this.
	(autofdo_source_profile::find_iter_for_function_instance): New
	function.
	(autofdo_source_profile::find_function_instance): Likewise.
	(autofdo_source_profile::add_function_instance): Likewise.
	(autofdo_source_profile::remove_function_instance): Likewise.
erozenfeld
erozenfeld approved these changes Jan 6, 2026
View reviewed changes
Copy link
Contributor

@erozenfeld erozenfeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

@dc03-work
Copy link
Contributor Author

dc03-work commented Jan 14, 2026

Hi @snehasish , can you take a look at landing these changes? The AutoFDO flow in GCC is currently broken without this and #251 (Eugene has approved both). We have been running tests for quite a while with these changes and they seem quite stable, so I feel these should be pretty safe to go in.

snehasish
snehasish approved these changes Jan 14, 2026
View reviewed changes
Copy link
Collaborator

@snehasish snehasish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm with some minor nits. Let me know when you're ready to merge and I'll hit the button. Thanks!

legacy_addr2line.cc Outdated
if (start_line == 0)
start_line = declaration->callsite_line();
comp_dir = subprog->comp_directory();
if (comp_dir == 0)
Copy link
Collaborator

@snehasish snehasish Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put controlled statements within braces. See style guide

legacy_addr2line.cc Outdated
}

std::filesystem::path dir_name = LI.file.first;
if (!dir_name.is_absolute() && comp_dir != nullptr)
Copy link
Collaborator

@snehasish snehasish Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put controlled statements within braces. See style guide

legacy_addr2line.cc Outdated

dir_name = subprog->callsite_directory();
if (!dir_name.is_absolute()) {
if (subprog->comp_directory())
Copy link
Collaborator

@snehasish snehasish Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put controlled statements within braces. See style guide

profile_writer.cc Outdated
// File names in the profile are a feature of GCOV version 3.
if (absl::GetFlag(FLAGS_gcov_version) >= 3) {
gcov_write_unsigned(file_map.Size());
for (const auto &file_name : file_map.GetFileNames())
Copy link
Collaborator

@snehasish snehasish Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put controlled statements within braces. See style guide

profile_writer.cc Outdated
}
gcov_write_string(c);
if (absl::GetFlag(FLAGS_gcov_version) >= 3) {
if (int lookup = file_map.GetFileIndex(name); lookup != -1)
Copy link
Collaborator

@snehasish snehasish Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put controlled statements within braces. See style guide

profile_writer.h Outdated
protected:
void Visit(const Symbol *node) override {
if (node->info.func_name != nullptr) {
if (node->info.dir_name != "")
Copy link
Collaborator

@snehasish snehasish Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put controlled statements within braces. See style guide

profile_writer.h Outdated

void AddFileName(const std::string &symbol_name,
const std::string &file_name) {
if (auto it = file_map_.find(file_name); it != file_map_.end())
Copy link
Collaborator

@snehasish snehasish Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put controlled statements within braces. See style guide

@dc03-work
Copy link
Contributor Author

dc03-work commented Jan 14, 2026

lgtm with some minor nits. Let me know when you're ready to merge and I'll hit the button. Thanks!

Hope this looks good now. Feel free to merge :) Can you also please take a look at #251?

@snehasish
Copy link
Collaborator

snehasish commented Jan 14, 2026

gcov tests passed but llvm_propeller_perf_branch_frequencies_aggregator_test failed. I'll go ahead and merge since it's unrelated to this pull request.

@snehasish snehasish merged commit 46c27b0 into google:master Jan 14, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@snehasish snehasish snehasish approved these changes

+1 more reviewer

@erozenfeld erozenfeld erozenfeld approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@dc03-work @snehasish @erozenfeld @janhubicka @shenhanc78