[PATCH] D97013: [LTO] Fix cloning of llvm*.used when splitting module

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 19 21:37:10 PST 2021


MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp:204
+  SmallPtrSet<GlobalValue *, 8> NewUsed;
+  // First collect those in the llvm.used set.
+  collectUsedGlobalVariables(SrcM, Used, CompilerUsed);
----------------
llvm.used -> llvm[.compiler].used

"First" here made me think whether there should be another comment about "then" :)


================
Comment at: llvm/test/Transforms/ThinLTOBitcodeWriter/split-used.ll:15
+; M1: @g1 = global i8 42, !type !0
+; M1: @g2 = global i8 42, !type !0
+ at g1 = global i8 42, !type !0
----------------
Perhaps add `; M1-NOT: @g`

or change the following `; M1: @llvm.used` to `; M1-NEXT: @llvm.used`, to check that @g3 or @g4 is not copied.


================
Comment at: llvm/test/Transforms/ThinLTOBitcodeWriter/split-used.ll:25
+; M1: @llvm.compiler.used = appending global [1 x i8*] [i8* @g2]
+ at llvm.used = appending global [2 x i8*] [ i8* @g1, i8* @g3]
+ at llvm.compiler.used = appending global [2 x i8*] [ i8* @g2, i8* @g4]
----------------
The canonical form has `, section("llvm.metadata")`.

With this new patch, `, section("llvm.metadata")` will be appended for M1. I assume that this different does not matter.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97013/new/

https://reviews.llvm.org/D97013



More information about the llvm-commits mailing list