This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Don't pass -ffunction-section/-fdata-sections
ClosedPublic

Authored by sbc100 on Sep 13 2017, 3:43 PM.

Diff Detail

Repository
rC Clang

Event Timeline

sbc100 created this revision.Sep 13 2017, 3:43 PM
sbc100 added a subscriber: llvm-commits.
dschuff edited edge metadata.Sep 13 2017, 4:03 PM

I guess the reason for removing this is that we don't really have an analogue of different text sections in the wasm object format, but that dead function stripping can be done by any linker smart enough to link wasm at all. But this isn't necessarily true of data sections though, is it?

Also I don't understand why removing the hardcoding on the LLVM side requires removing the explicit passing of the options here?

Making the change here first means that I can test the new default and land it in llvm. If clang is continues to set its own default then anyone using clang (i.e. a lot of the tests) won't see the effect of the change.

Shouldn't the LLVM-side tests be using the LLVM utils and not clang? Or did you mean lld tests, or waterfall tests or what?

Once we have the support, I think we do want the default behavior to be to dead strip functions and data, and I agree that it shouldn't be hardcoded in LLVM. So this would be the natural way to do that, no?

Yes, I was refering to the waterfall tests.

I don't see why webassembly should default to -fdata-sections/-ffunction-sections any more than any other platform should.

But I can hold off on this change if you like? I guess it isn't essential to the work i'm doing on the llvm side..

I think the reasoning was really just that code size reduction is even more important on wasm than other platforms, and because it's a new ABI there's no one depending on the default behavior that would break.

And, I didn't mean to push back really hard or block you or anything, I just genuinely didn't understand what you meant in the commit description.

sbc100 abandoned this revision.Dec 8 2017, 4:03 PM
sbc100 updated this revision to Diff 132098.Jan 30 2018, 8:59 PM
  • update tests
sbc100 retitled this revision from [WebAssembly] Don't pass -ffunction-section/-fdata-sections by default to [WebAssembly] Don't pass -ffunction-section/-fdata-sections .Jan 30 2018, 8:59 PM
sbc100 edited the summary of this revision. (Show Details)

After a little discussion about this and the -gc-sections linker flag, the augments for the tools having sensible defaults seem to be winning.

This means we don't need the driver to pass these options, which makes our commands lines shorter, and it also means that people who use the tools directly get the sensible defaults too.

sbc100 removed a subscriber: llvm-commits.
sunfish accepted this revision.Jan 31 2018, 8:47 AM

I think the reasoning was really just that code size reduction is even more important on wasm than other platforms, and because it's a new ABI there's no one depending on the default behavior that would break.

I agree. Also, on some platforms, -ffunction-sections and -fdata-sections can result in less efficient code, however that's not the case on wasm.

This revision is now accepted and ready to land.Jan 31 2018, 8:47 AM
dschuff accepted this revision.Jan 31 2018, 9:20 AM

After a little discussion about this and the -gc-sections linker flag, the augments for the tools having sensible defaults seem to be winning.

Well sure, when you put it that way, nobody can argue against sensible defaults, can they :D

This revision was automatically updated to reflect the committed changes.