[PATCH] D59343: [WebAssembly] Use passive segments when memory is shared

Mon Apr 15 19:02:34 PDT 2019

tlively added inline comments.

================
Comment at: lld/test/wasm/data-segment-merging.ll:61
+; MERGE-SHARED:        - Type:            DATA
+; MERGE-SHARED:          Segments:
+; MERGE-SHARED:             InitFlags:       1
----------------
sbc100 wrote:
> Can we switch to MERGE-SHARED-NEXT here and below?
We're deliberately skipping SectionOffset lines in the output because they're not that very interesting. I could include them and switch to MERGE-SHARED-NEXT if you want, though.

I've also de-duplicated a lot of the checking here.

================
Comment at: lld/wasm/OutputSections.cpp:164
+         Segments.size() <= 1 &&
+             "Currenly only a single data segment is supported in PIC mode");
+
----------------
sbc100 wrote:
> Perhaps we should make this into a fatal error?  And we could perhaps catch it earlier.  Doesn't need to happen in this CL though I guess,
Makes sense, but I think that can be a separate PR.

================
Comment at: lld/wasm/Writer.cpp:1350
+    // initialize passive data segments
+    for (const OutputSegment *S : Segments) {
+      if (S->InitFlags & WASM_SEGMENT_IS_PASSIVE) {
----------------
sbc100 wrote:
> I'm tempted to make this a new function of its own that is than called as constructor 0.  What do you think?
Sounds good. Done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59343