[PATCH] D31735: ThinLTOBitcodeWriter: delete comdats if their keys are renamed

Peter Collingbourne via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 6 16:35:36 PDT 2017


pcc added inline comments.


================
Comment at: lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp:87
+    // delete the COMDAT. This is safe, because this COMDAT is not going to
+    // be merged anyway.
+    if (const Comdat *C = ExportGV.getComdat())
----------------
And because globals not in comdats are GC roots.


================
Comment at: lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp:161
+void removeComdats(Module &M, const std::set<StringRef> &ComdatsToRemove) {
+  for (auto &GV : M.globals())
+    if (GV.hasComdat() && ComdatsToRemove.count(GV.getComdat()->getName()))
----------------
This can just enumerate global_objects instead of checking that the global is a GlobalObject.


================
Comment at: test/Transforms/ThinLTOBitcodeWriter/comdat.ll:7-8
+; with the original name.
+; CHECK0-NOT: comdat baz
+; CHECK0-NOT: comdat foo
+; CHECK0: @al = internal
----------------
This isn't checking for the correct syntax for comdats.


================
Comment at: test/Transforms/ThinLTOBitcodeWriter/comdat.ll:18
+
+; CHECK1-NOT;: $foo = comdat
+; CHECK1-NOT: $baz = comdat
----------------
remove ; before :


================
Comment at: test/Transforms/ThinLTOBitcodeWriter/comdat.ll:34-40
+ at al = internal unnamed_addr global i8* bitcast ({ [1 x i8*] }* @baz to i8*), comdat ($baz)
+
+ at anon = private unnamed_addr constant { [1 x i8*] } { [1 x i8*] [i8* null] }, comdat($foo), !type !0
+
+ at baz = internal unnamed_addr constant { [1 x i8*] } { [1 x i8*] [i8* null] }, comdat, !type !0
+
+ at foo = internal unnamed_addr alias i8*, getelementptr inbounds ({ [1 x i8*] }, { [1 x i8*] }* @anon, i32 0, i32 0, i32 1)
----------------
This appears to be one comdat with a single member (the leader) with metadata and another where the leader has metadata and the non-leader doesn't. Please add a non-leader to the former and change the latter so that the non-leader has metadata and the leader doesn't.

Please also give the globals less confusing names and group them by comdat.

There should also be a negative test showing that we keep the comdat if no global has type metadata.


https://reviews.llvm.org/D31735





More information about the llvm-commits mailing list