This is an archive of the discontinued LLVM Phabricator instance.

Allow Module name to be used to generate a unique Module ID
AbandonedPublic

Authored by tmsriram on Jan 23 2020, 4:50 PM.

Details

Summary

Modify getUniqueModuleID to use Module name for better uniqueness guarantees.

Module ID helps when there are no globals exported or when multiple definitions/ of the same globals are allowed in different modules (with "-z muldefs"). While it is highly likely that Module identifier is unique, it is not guaranteed.

Diff Detail

Event Timeline

tmsriram created this revision.Jan 23 2020, 4:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 23 2020, 4:50 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
pcc added a comment.Jan 24 2020, 8:17 AM

If you don't need a guarantee of uniqueness wouldn't it be better to just take a hash of Module::getSourceFileName() instead of looking at the names of the globals?

In D73310#1838950, @pcc wrote:

If you don't need a guarantee of uniqueness wouldn't it be better to just take a hash of Module::getSourceFileName() instead of looking at the names of the globals?

So you are suggesting that when "UseModuleId" is true, we just return the hash of getModuleIdentifier()? That works for us and I can modify this function accordingly.

pcc added a comment.Jan 24 2020, 9:48 AM

I am suggesting that you wouldn't change this function at all but would hash getSourceFileName yourself. (You shouldn't use getModuleIdentifier for anything that affects the contents of the output file because it isn't serialized.)

In D73310#1839155, @pcc wrote:

I am suggesting that you wouldn't change this function at all but would hash getSourceFileName yourself. (You shouldn't use getModuleIdentifier for anything that affects the contents of the output file because it isn't serialized.)

I modified https://reviews.llvm.org/D73307 to do exactly this. I will delete this patch if the new change is alright.

tmsriram abandoned this revision.Mar 30 2020, 9:40 AM

D73307 directly computes the hash so this is no longer necessary.