Page MenuHomePhabricator

[WebAssembly] LTO: Honor comdat groups when loading bitcode files
ClosedPublic

Authored by sbc100 on Tue, May 14, 4:48 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

sbc100 created this revision.Tue, May 14, 4:48 PM
sbc100 added a reviewer: ruiu.Tue, May 14, 4:49 PM
sbc100 updated this revision to Diff 199536.Tue, May 14, 4:51 PM
  • comment
ruiu added a comment.Tue, May 14, 11:00 PM

Even though what this code does is the same as what we do for the ELF linker, the code looks different. In ELF, we pass ComdatGroups (which is a set of comdat signature strings we've seen so far) around. I don't remember if there was a reason to do it differently for Wasm. Do you remmeber?

Even though what this code does is the same as what we do for the ELF linker, the code looks different. In ELF, we pass ComdatGroups (which is a set of comdat signature strings we've seen so far) around. I don't remember if there was a reason to do it differently for Wasm. Do you remmeber?

I found this version more explicit. ELF currently passes a new DummyGroups to each LTO object's parse function as way of implicitly disabling comdats, but I think explicitly disabling them improves clarity. Perhaps I should change the ELF linker to do the same thing either in this change or as a followup? Alternaitvely I would update this PR to match the ELF method and propose a separate PR to convert them both over as a followup?

ruiu accepted this revision.Wed, May 15, 6:06 AM

LGTM

Yeah, maybe we should change ELF to match this, but it will likely to conflict with my ongoing refactoring, I'll take a look later about that.

This revision is now accepted and ready to land.Wed, May 15, 6:06 AM
This revision was automatically updated to reflect the committed changes.