This is an archive of the discontinued LLVM Phabricator instance.

[DWARFLinker] Refactor clang modules loading code.
ClosedPublic

Authored by avl on Aug 31 2022, 1:29 PM.

Details

Summary

Current implementation of registerModuleReference() function not only
"registers" module reference, but also clones referenced module
(inside loadClangModule()). That may lead to cloning the module with
incorrect options (registerModuleReference() examines module references
and additionally accumulates MaxDwarfVersion and accel tables info).
Since accumulated options may differ from the current values,
it is incorrect to clone modules before options are fully accumulated.

This patch separates "cloning" code from "registering" code. So,
that accumulating option is done in the "registering stage" and
"cloning" is done after all modules are registered and options accumulated.
It also adds a callback for loaded compile units which can be used for
D132755 and D132371(to allow doing options accumulation outside
of DWARFLinker).

Diff Detail

Event Timeline

avl created this revision.Aug 31 2022, 1:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 31 2022, 1:29 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
avl requested review of this revision.Aug 31 2022, 1:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 31 2022, 1:29 PM
aprantl accepted this revision.Sep 1 2022, 10:08 AM

This looks reasonable to me.

llvm/include/llvm/DWARFLinker/DWARFLinker.h
416

Doxygen comment?

llvm/tools/llvm-dwarfutil/DebugInfoLinker.cpp
303 ↗(On Diff #457076)

should this be a default parameter?

This revision is now accepted and ready to land.Sep 1 2022, 10:08 AM
avl updated this revision to Diff 457562.Sep 2 2022, 4:55 AM

Thank you for the review. addressed comments.

This revision was automatically updated to reflect the committed changes.