Merge two small functions and add comments.

[ruiu]

Summary

Merge two small functions and add comments.

[ruiu]

By the way, what is InputChunk::copyRelocations for? That function copies relocations from other InputChunk, but it's not clear to me why we need to do that.

[sbc100]

lld/wasm/InputChunks.cpp

77 ↗

(On Diff #134696)

I think this static function was probably a legacy of time where it have many calls sites.. Thanks for cleaning this up.

79 ↗

(On Diff #134696)

How do you feel about documentation here vs declaration in the .h file?

[ruiu]

lld/wasm/InputChunks.cpp

79 ↗

(On Diff #134696)

Why not both? :)

[ruiu]

• update as per Sam's comments

[sbc100]

In D43406#1010680, @ruiu wrote:

By the way, what is InputChunk::copyRelocations for? That function copies relocations from other InputChunk, but it's not clear to me why we need to do that.

Its makes a copy of the relocations that apply to that particular chunk. They are used later in calcRelocations, but I imagine they could be merged.

(BTW, We are considering moving to a model where relocations would be stored on a per-chunk, rather than per section basis, to avoid this linear search, which seems rather pointless).

[sbc100]

lld/wasm/InputChunks.cpp

79 ↗

(On Diff #134696)

Not sure that makes sense. It means the old probably of comments being out-of-date is now double because the can also be out of date with themselves! :)

[ruiu]

lld/wasm/InputChunks.cpp

79 ↗

(On Diff #134696)

writeTo() is actually pretty stable interface and I don't think it would change. But if you don't want to have duplicate comments, I can remove it from, perhaps, .h file, because we have bunch of comments in this file rather than in the .h file.

[sbc100]

lld/wasm/InputChunks.cpp

79 ↗

(On Diff #134696)

I don't feel strongly but I feel like we should probably be consistent and decide which we prefer (.h or .cpp). No urgent need to fix this one now.

[ruiu]

lld/wasm/InputChunks.cpp

79 ↗

(On Diff #134696)

I'll go ahead with removing the comment from .h file because it is at least consistent with the local convention in this file.

[ruiu]

• removed a comment from the .h file

wasm/InputChunks.cpp

static void applyRelocation(uint8_t *Buf, const OutputRelocation &Reloc) {
   case R_WEBASSEMBLY_MEMORY_ADDR_I32:
     write32le(Buf, Reloc.Value);
     break;
   default:
     llvm_unreachable("unknown relocation type");
   }
 }
 
-static void applyRelocations(uint8_t *Buf, ArrayRef<OutputRelocation> Relocs) {
-  if (!Relocs.size())
+// Copy this input chunk to an mmap'ed output file.
+void InputChunk::writeTo(uint8_t *Buf) const {
+  // Copy contents
+  memcpy(Buf + getOutputOffset(), data().data(), data().size());
+
+  // Apply relocations
+  if (OutRelocations.empty())
     return;
-  DEBUG(dbgs() << "applyRelocations: count=" << Relocs.size() << "\n");
-  for (const OutputRelocation &Reloc : Relocs)
+  DEBUG(dbgs() << "applyRelocations: count=" << OutRelocations.size() << "\n");
+  for (const OutputRelocation &Reloc : OutRelocations)
     applyRelocation(Buf, Reloc);
 }
 
-void InputChunk::writeTo(uint8_t *SectionStart) const {
-  memcpy(SectionStart + getOutputOffset(), data().data(), data().size());
-  applyRelocations(SectionStart, OutRelocations);
-}
-
 // Populate OutRelocations based on the input relocations and offset within the
 // output section.  Calculates the updated index and offset for each relocation
 // as well as the value to write out in the final binary.
 void InputChunk::calcRelocations() {
   if (Relocations.empty())
     return;
   int32_t Off = getOutputOffset() - getInputSectionOffset();
   DEBUG(dbgs() << "calcRelocations: " << File->getName()