[PATCH] D44343: [WebAssembly] Identify COMDATs by index rather than string. NFC

Sam Clegg via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 9 19:11:42 PST 2018


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

Nice!  LVGTM



================
Comment at: lib/Object/WasmObjectFile.cpp:928
     Segment.Data.Content = ArrayRef<uint8_t>(Ptr, Size);
     Segment.Data.Alignment = 0;
     Segment.Data.Flags = 0;
----------------
I feel like make comment here might make sense.  Something like "The rest of these fields get set in the linking metadata section"?


================
Comment at: tools/obj2yaml/wasm2yaml.cpp:69
     for (auto &Func : Obj.functions()) {
-      if (!Func.Comdat.empty()) {
-        auto &Comdat = LinkingSec->Comdats[ComdatIndexes[Func.Comdat]];
-        Comdat.Entries.emplace_back(
+      if (Func.Comdat < Comdats.size()) {
+        LinkingSec->Comdats[Func.Comdat].Entries.emplace_back(
----------------
Should this be a little more strict?   Like `if (Func.Comdat != UINT32_MAX)`  more explicitly expressed the indent no?


================
Comment at: tools/obj2yaml/wasm2yaml.cpp:84
       }
-      if (!Segment.Data.Comdat.empty()) {
-        auto &Comdat = LinkingSec->Comdats[ComdatIndexes[Segment.Data.Comdat]];
-        Comdat.Entries.emplace_back(
+      if (Segment.Data.Comdat < Comdats.size()) {
+        LinkingSec->Comdats[Segment.Data.Comdat].Entries.emplace_back(
----------------
ditto


Repository:
  rL LLVM

https://reviews.llvm.org/D44343





More information about the llvm-commits mailing list