Reland "[ThinLTO][Bitcode] Generate import type in bitcode" #97253

mingmingl-llvm · 2024-07-01T04:37:48Z

#87600 was reverted in order to revert 6262763. Now #95482 is fix forward for 6262763. This patch is a reland for #87600

Changes on top of original patch
In llvm/include/llvm/IR/ModuleSummaryIndex.h, make the type of GVSummaryPtrSet an unordered_set which is more memory efficient when the number of elements is smaller than 128 [1]

Original commit message

For distributed ThinLTO, the LTO indexing step generates combined summary for each module, and postlink pipeline reads the combined summary which stores the information for link-time optimization.

This patch populates the 'import type' of a summary in bitcode, and updates bitcode reader to parse the bit correctly.

[1]

llvm-project/llvm/lib/Support/SmallPtrSet.cpp

Line 43 in 393eff4

Grow(CurArraySize < 64 ? 128 : CurArraySize * 2);

…eSummary::GVFlags - This change doesn't set the bit in the summaries, just make the code changes.

…spr/summary2

Update this one accordingly

1. Set 'import-instr-evolution-factor' to 1.0 (default 0.7) to make sure import limit remains 7 (otherwise import limit decays through each edge) 2. Updated bar* function names 3. Explain why aliasee is null for bar* functions

github-actions · 2024-07-01T04:53:15Z

✅ With the latest revision this PR passed the C/C++ code formatter.

jvoung · 2024-07-08T19:54:46Z

llvm/include/llvm/IR/ModuleSummaryIndex.h

@@ -1277,7 +1278,7 @@ using ModulePathStringTableTy = StringMap<ModuleHash>;
 using GVSummaryMapTy = DenseMap<GlobalValue::GUID, GlobalValueSummary *>;

 /// A set of global value summary pointers.
-using GVSummaryPtrSet = SmallPtrSet<GlobalValueSummary *, 4>;
+using GVSummaryPtrSet = std::unordered_set<GlobalValueSummary *>;


Ah nice -- interesting that SmallPtrSet also has a big jump when growing at small sizes! (

llvm-project/llvm/lib/Support/SmallPtrSet.cpp

Line 43 in 393eff4

Grow(CurArraySize < 64 ? 128 : CurArraySize * 2);

)

yes I corrected the patch description with this link. thanks!

jvoung · 2024-07-08T19:56:04Z

llvm/lib/Bitcode/Writer/BitcodeWriter.cpp

@@ -453,11 +458,16 @@ class IndexBitcodeWriter : public BitcodeWriterBase {
  /// Constructs a IndexBitcodeWriter object for the given combined index,
  /// writing to the provided \p Buffer. When writing a subset of the index
  /// for a distributed backend, provide a \p ModuleToSummariesForIndex map.
+  /// If provided, \p ModuleToDecSummaries specifies the set of summaries for


nit: DecSummaries?

mingmingl-llvm · 2024-07-09T05:20:13Z

The pre-commit buildbot failure is irrelevant

 cat github-pull-requests_build_79148_linux-linux-x64.log | grep -B 10 -A 10  "Failed Tests "
[0.70s,0.80s) :: [                                        ] :: [  3/442]
[0.60s,0.70s) :: [                                        ] :: [  2/442]
[0.50s,0.60s) :: [                                        ] :: [ 10/442]
[0.40s,0.50s) :: [                                        ] :: [  9/442]
[0.30s,0.40s) :: [*                                       ] :: [ 22/442]
[0.20s,0.30s) :: [*************                           ] :: [151/442]
[0.10s,0.20s) :: [****************                        ] :: [183/442]
[0.00s,0.10s) :: [*****                                   ] :: [ 61/442]
--------------------------------------------------------------------------
********************
Failed Tests (1):
  BOLT :: X86/reader-stale-yaml-std.test


Testing Time: 2.55s

Total Discovered Tests: 454
  Skipped          :   7 (1.54%)
  Unsupported      :  13 (2.86%)
  Passed           : 431 (94.93%)
  Expectedly Failed:   2 (0.44%)

llvm#87600 was reverted in order to revert llvm@6262763. Now llvm#95482 is fix forward for llvm@6262763. This patch is a reland for llvm#87600 **Changes on top of original patch** In `llvm/include/llvm/IR/ModuleSummaryIndex.h`, make the type of `GVSummaryPtrSet` an `unordered_set` which is more memory efficient when the number of elements is smaller than 128 [1] **Original commit message** For distributed ThinLTO, the LTO indexing step generates combined summary for each module, and postlink pipeline reads the combined summary which stores the information for link-time optimization. This patch populates the 'import type' of a summary in bitcode, and updates bitcode reader to parse the bit correctly. [1] https://github.com/llvm/llvm-project/blob/393eff4e02e7ab3d234d246a8d6912c8e745e6f9/llvm/lib/Support/SmallPtrSet.cpp#L43

mingmingl-llvm added 26 commits April 3, 2024 21:46

[ThinLTO]Record import type (declaration or definition) in GlobalValu…

78fa2e0

…eSummary::GVFlags - This change doesn't set the bit in the summaries, just make the code changes.

[ThinLTO]Generate import type in bitcode writer

4b4c33f

function import changes

cfb63d7

'git merge main' and resolve merge conflict

b0a4060

Merge branch 'main' into users/minglotus-6/spr/summary1

e07d423

rename Dec to Decl

175febc

Merge branch 'main' into users/minglotus-6/spr/summary1

8bc4d7a

Merge branch 'users/minglotus-6/spr/summary1' into users/minglotus-6/…

7252e11

…spr/summary2

simplify code

23a3fa4

Flag gate the import-declaration change.

a0277b4

stage changes

5edeccc

Add test coverage for import delcaration

a139e4a

add test coverage for function alias

916dc96

llvm-lto test coverage

248fbfb

add test coverage for debugging log, and a RUN line for internalization

5e731ec

Merge branch 'main' into users/minglotus-6/spr/summary1

c5e168e

resolve review feedback

2c38b47

Merge branch 'users/minglotus-6/spr/summary1' into users/minglotus-6/…

3da9b8b

…spr/summary2

Diffbase is updated (2c38b47).

f1d22e1

Update this one accordingly

resolve review feedback

86f3f44

update this patch as the second one

001a785

update stale comment and use 'DAG' for check lines

321f6aa

add comment for 'DecSummaries' parameter in FunctionImport.h

31d9bd2

run 'git merge main'

ac8e9fa

resolve review feedback

bde377c

Update regression test:

779dcf3

1. Set 'import-instr-evolution-factor' to 1.0 (default 0.7) to make sure import limit remains 7 (otherwise import limit decays through each edge) 2. Updated bar* function names 3. Explain why aliasee is null for bar* functions

mingmingl-llvm mentioned this pull request Jul 1, 2024

Reland "[ThinLTO][Bitcode] Generate import type in bitcode (#87600)" #97233

Closed

resolve merge conflict

41f8e2b

use std::unordered_set

bf4bdeb

mingmingl-llvm requested review from teresajohnson and jvoung July 8, 2024 18:43

mingmingl-llvm marked this pull request as ready for review July 8, 2024 18:43

jvoung approved these changes Jul 8, 2024

View reviewed changes

resolve comments

73953c1

mingmingl-llvm merged commit 50fea99 into main Jul 9, 2024
5 of 7 checks passed

mingmingl-llvm deleted the users/minglotus-6/spr/summary1 branch July 9, 2024 05:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Reland "[ThinLTO][Bitcode] Generate import type in bitcode" #97253

Reland "[ThinLTO][Bitcode] Generate import type in bitcode" #97253

Uh oh!

mingmingl-llvm commented Jul 1, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Jul 1, 2024 •

edited

Loading

Uh oh!

jvoung Jul 8, 2024

Uh oh!

mingmingl-llvm Jul 8, 2024

Uh oh!

jvoung Jul 8, 2024

Uh oh!

mingmingl-llvm Jul 8, 2024

Uh oh!

mingmingl-llvm commented Jul 9, 2024

Uh oh!

Uh oh!

Uh oh!

Reland "[ThinLTO][Bitcode] Generate import type in bitcode" #97253

Reland "[ThinLTO][Bitcode] Generate import type in bitcode" #97253

Uh oh!

Conversation

mingmingl-llvm commented Jul 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jul 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jvoung Jul 8, 2024

Choose a reason for hiding this comment

Uh oh!

mingmingl-llvm Jul 8, 2024

Choose a reason for hiding this comment

Uh oh!

jvoung Jul 8, 2024

Choose a reason for hiding this comment

Uh oh!

mingmingl-llvm Jul 8, 2024

Choose a reason for hiding this comment

Uh oh!

mingmingl-llvm commented Jul 9, 2024

Uh oh!

Uh oh!

Uh oh!

mingmingl-llvm commented Jul 1, 2024 •

edited

Loading

github-actions bot commented Jul 1, 2024 •

edited

Loading