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

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

79–85

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

[ruiu]

lld/wasm/InputChunks.cpp

79–85

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–85

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–85

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–85

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–85

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

lld/wasm/InputChunks.h

public:
   enum Kind { DataSegment, Function };
 
   Kind kind() const { return SectionKind; }
 
   uint32_t getSize() const { return data().size(); }
 
   void copyRelocations(const WasmSection &Section);
 
+  // Copy this input chunk to an mmap'ed output file and apply relocations.
   void writeTo(uint8_t *SectionStart) const;
 
   void setOutputOffset(uint32_t Offset) {
     OutputOffset = Offset;
     calcRelocations();
   }
 
   uint32_t getOutputOffset() const { return OutputOffset; }

lld/wasm/InputChunks.cpp

for (const WasmRelocation &R : Section.Relocations)
     if (R.Offset >= Start && R.Offset < Start + Size)
       Relocations.push_back(R);
 }
 
 static void applyRelocation(uint8_t *Buf, const OutputRelocation &Reloc) {
   DEBUG(dbgs() << "write reloc: type=" << Reloc.Reloc.Type
                << " index=" << Reloc.Reloc.Index << " value=" << Reloc.Value
                << " offset=" << Reloc.Reloc.Offset << "\n");
+
   Buf += Reloc.Reloc.Offset;
   int64_t ExistingValue;
+
   switch (Reloc.Reloc.Type) {
   case R_WEBASSEMBLY_TYPE_INDEX_LEB:
   case R_WEBASSEMBLY_FUNCTION_INDEX_LEB:
   case R_WEBASSEMBLY_GLOBAL_INDEX_LEB:
     ExistingValue = decodeULEB128(Buf);
     // Additional check to verify that the existing value that the location
     // matches our expectations.
     if (ExistingValue != Reloc.Reloc.Index) {
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()