This is an archive of the discontinued LLVM Phabricator instance.

[LTO] Use names from more GlobalValue types to generate the ModuleID
Needs ReviewPublic

Authored by ormris on Apr 18 2022, 5:39 PM.

Details

Summary

In many situations the current version of getUniqueModuleId returns no hash. This causes problems for ThinLTOBitcodeWriter pass, and it will fall back to generating a regular LTO module in these cases. Since it's preferable to generate a ThinLTO module for performance reasons, this patch allows getUniqueModuleId to use more types of GlobalValues when generating the ModuleId hash. This mix has been fairly reliable for us.

This change is part of the Unified LTO RFC: https://discourse.llvm.org/t/rfc-a-unified-lto-bitcode-frontend/61774

Diff Detail

Event Timeline

ormris created this revision.Apr 18 2022, 5:39 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 18 2022, 5:39 PM
ormris requested review of this revision.Apr 18 2022, 5:39 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 18 2022, 5:39 PM
ormris updated this revision to Diff 423695.Apr 19 2022, 11:56 AM

Update patch with fixes for some PowerPC tests.

pcc requested changes to this revision.Apr 19 2022, 12:51 PM

I think this will introduce a correctness issue as it will become possible for two modules that can be linked together to have the same unique module ID.

This revision now requires changes to proceed.Apr 19 2022, 12:51 PM
ormris planned changes to this revision.Apr 19 2022, 1:31 PM

This patch causes several test failures. Currently working on resolving those.

I think this will introduce a correctness issue as it will become possible for two modules that can be linked together to have the same unique module ID.

Our goal with this change was to reduce the hash collisions we were finding in internal testing of the unified LTO pipeline. The previous method wasn't quite broad enough for our internal tests. While we've certainly constructed test cases that cause a collision when using the changes in this patch, we've never seen a collision when working with real code.

ormris updated this revision to Diff 523203.May 17 2023, 3:55 PM

Changelog:

  • Changes are now implemented as a fallback, reducing necessary test changes.
  • Add option to allow testing cases using empty module IDs.
  • Reduce complexity of symbol filter
ormris updated this revision to Diff 523474.May 18 2023, 11:37 AM

Update to remove unnecessary -unified-lto flag from tests.

ormris updated this revision to Diff 526108.May 26 2023, 10:14 AM

clang-format

gentle ping