Page MenuHomePhabricator

Put profile variables of COMDAT functions to it's own COMDAT group
ClosedPublic

Authored by xur on Aug 21 2015, 11:44 AM.

Details

Summary

In -fprofile-instr-generate compilation, to remove the redundant profile variables for the COMDAT functions, these variables are placed in the same COMDAT group as its associated function.
This way when the COMDAT function is not picked by the linker, those profile variables will also not be output in the final binary.

One problem here is that we lose the ability to link the objects built w and w/o -fprofile-instr-generate. The linker may choose one version of COMDAT that not expanded (i.e. not built with -fprofile-instr-generate). All the references to the variables
become dangling. The result binary will segfalut. If ld.gold is used, we usually see a warning like:
foo.o:foo.bc:function __llvm_profile_register_functions: warning: relocation refers to discarded section

This patch puts the profile variables for COMDAT functions to its
own COMDAT group to avoid the problem.

Event Timeline

xur updated this revision to Diff 32850.Aug 21 2015, 11:44 AM
xur retitled this revision from to Put profile variables of COMDAT functions to it's own COMDAT group.
xur updated this object.
xur added reviewers: bogner, dnovillo.
xur added a subscriber: llvm-commits.
davidxl added inline comments.
InstrProfiling.cpp
206 ↗(On Diff #32850)

How about "var" --> "vars" to indicate the comdat section includes _counters, _data, _name ?

dnovillo edited edge metadata.Sep 11 2015, 7:17 AM

With David's suggestions and a test case, this LGTM.

InstrProfiling.cpp
199 ↗(On Diff #32850)

s/Make/Place/

xur updated this revision to Diff 34566.Sep 11 2015, 11:41 AM
xur edited edge metadata.

The new patch integrated David and Diego's comments.
I found that there is an existing test case that can test the new behavior. So I will not add a new test case.

This revision was automatically updated to reflect the committed changes.