This is an archive of the discontinued LLVM Phabricator instance.

Store and emit original name in combined index
ClosedPublic

Authored by mehdi_amini on Apr 22 2016, 11:03 PM.

Details

Summary

As discussed in D18298, some local globals can't
be renamed/promoted (because they have a section, or because
they are referenced from inline assembly).
To be able to detect naming collision, we need to keep around
the "GUID" using their original name without taking the linkage
into account.

Diff Detail

Event Timeline

mehdi_amini retitled this revision from to Store and emit original name in combined index.
mehdi_amini updated this object.
mehdi_amini added a reviewer: tejohnson.
mehdi_amini added a subscriber: llvm-commits.
tejohnson added inline comments.Apr 23 2016, 3:54 PM
lib/Bitcode/Reader/BitcodeReader.cpp
5825

Maybe a comment here and in the below case that for the combined index the original name comes from a separate bitcode record (which is why both are set to the same GUID)?

6081

Don't FS_ALIAS and FS_PERMODULE_GLOBALVAR_INIT_REFS need to setOriginalName as well?

mehdi_amini marked an inline comment as done.

Address comments, add an alias to the test

Add a global variable in the test

mehdi_amini marked an inline comment as done.Apr 23 2016, 4:15 PM
tejohnson accepted this revision.Apr 23 2016, 4:38 PM
tejohnson edited edge metadata.

LGTM, except I have one concern about the test, noted below.

test/Bitcode/thinlto-function-summary-originalnames.ll
15

These GUIDs (not the original ones above) might be dangerous to put here as they include the path to the module (which I believe could vary on some bots).

This revision is now accepted and ready to land.Apr 23 2016, 4:38 PM
mehdi_amini closed this revision.Apr 23 2016, 4:44 PM
mehdi_amini marked an inline comment as done.

r267304

test/Bitcode/thinlto-function-summary-originalnames.ll
15

Good point.

mehdi_amini marked an inline comment as done.Apr 23 2016, 5:11 PM
mehdi_amini added inline comments.
test/Bitcode/thinlto-function-summary-originalnames.ll
15

I added them back, after defining the "source_filename" in the IR.
It turns out that the order of the entries in the GLOBALVAL_SUMMARY_BLOCK are dependent on the path as well, so the test with the -NEXT lines was broken.