Page MenuHomePhabricator

[ThinLTO] Fix printing of module paths for distributed backend indexes
ClosedPublic

Authored by tejohnson on Jun 27 2018, 7:13 PM.

Details

Summary

In the individual index files emitted for distributed ThinLTO backends,
the module path ids are not contiguous. Assign slots to module paths in
order to handle this better and also to get contiguous numbering in the
summary assembly.

Diff Detail

Repository
rL LLVM

Event Timeline

tejohnson created this revision.Jun 27 2018, 7:13 PM

What was the numbering before?

davidxl added inline comments.Jul 2 2018, 10:55 AM
lib/IR/AsmWriter.cpp
790 ↗(On Diff #153243)

make it an inline function.

1019 ↗(On Diff #153243)

Can these two loops be fused ?

1119 ↗(On Diff #153243)

probably better to rename the function as initializeIndexIfNeeded or something, but can be done separately.

tejohnson marked an inline comment as done.Jul 2 2018, 1:42 PM
tejohnson added inline comments.
lib/IR/AsmWriter.cpp
1019 ↗(On Diff #153243)

No, there are 2 loops for the reason noted in the comments: Since the StringMap iteration order isn't

// guaranteed, use a std::map to order by module ID before assigning slots.
1119 ↗(On Diff #153243)

Will do as a follow-on (I named this similar to the existing "initialize()" which does lazy initialization of slots needed for printing the Module.

tejohnson updated this revision to Diff 153781.Jul 2 2018, 1:42 PM

Address comments

davidxl accepted this revision.Jul 2 2018, 1:53 PM

lgtm

lib/IR/AsmWriter.cpp
790 ↗(On Diff #153243)

CreateModulePathSlot is a tiny wrapper which should be marked as inline. I saw a different function is marked instead.

This revision is now accepted and ready to land.Jul 2 2018, 1:53 PM
tejohnson added inline comments.Jul 2 2018, 1:56 PM
lib/IR/AsmWriter.cpp
790 ↗(On Diff #153243)

Woops, will fix.

tejohnson updated this revision to Diff 153784.Jul 2 2018, 1:58 PM

Mark correct function as inline

This revision was automatically updated to reflect the committed changes.