This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Remove restriction on main name mangling
ClosedPublic

Authored by sbc100 on Feb 27 2020, 11:24 AM.

Details

Summary

Emscripten now handles/supports this new mode.

Diff Detail

Event Timeline

sbc100 created this revision.Feb 27 2020, 11:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 27 2020, 11:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 31 2022, 10:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 31 2022, 10:56 AM
sbc100 updated this revision to Diff 433173.May 31 2022, 1:43 PM
  • update
sunfish added inline comments.May 31 2022, 1:46 PM
llvm/lib/Target/WebAssembly/WebAssemblyFixFunctionBitcasts.cpp
264 ↗(On Diff #433173)

Why is this code going away? This isn't Emscripten-specific code.

sbc100 added inline comments.May 31 2022, 2:06 PM
llvm/lib/Target/WebAssembly/WebAssemblyFixFunctionBitcasts.cpp
264 ↗(On Diff #433173)

Ah.. I was hoping we could delete these codepaths, but maybe not.. :(

sbc100 updated this revision to Diff 433191.May 31 2022, 2:21 PM
  • revert parts
sunfish added inline comments.May 31 2022, 3:02 PM
clang/lib/CodeGen/CodeGenModule.cpp
6355

Can this use GA->setVisibility(GlobalValue::HiddenVisibility)? __main_void is an implementation detail, so in theory users shouldn't be setting interesting visibility attributes on it.

sbc100 added inline comments.May 31 2022, 3:22 PM
clang/lib/CodeGen/CodeGenModule.cpp
6355

The caller main / __main_void could live in different shared library the main function itself.

IIRC the caller of main is normally something like crt1.o and it should be able to see __main_void if, and only if, it can see main. i.e. if should have the same visibility as main itself, no?

sunfish added inline comments.May 31 2022, 3:28 PM
clang/lib/CodeGen/CodeGenModule.cpp
6355

Typically crt1.o is linked into the same exe/dso as main, so it needs main to have external linkage, but it doesn't need main to have any particular visibility.

In Emscripten, is main being called from a different dso, or from JS following visibility rules for different dsos?

sbc100 added inline comments.May 31 2022, 3:32 PM
clang/lib/CodeGen/CodeGenModule.cpp
6355

No, emscirpten is not different that respect. Typically main lives in the same DSO as crt1.o.

But it doesn't have to, and we have a few tests that check it can be in a separate dso.

sbc100 updated this revision to Diff 433386.Jun 1 2022, 7:09 AM
  • feedback
sunfish accepted this revision.Jun 1 2022, 9:29 AM
This revision is now accepted and ready to land.Jun 1 2022, 9:29 AM
sbc100 updated this revision to Diff 434605.Jun 6 2022, 2:05 PM
  • rebase
sbc100 added inline comments.Jun 6 2022, 2:06 PM
clang/lib/CodeGen/CodeGenModule.cpp
6355

Actually I think verified that setting this to hidden works for emscripten (at least the tests pass).

@sunfish.. do you know if you were relying on this symbols being marked as "used"?

This revision was landed with ongoing or failed builds.Jun 6 2022, 2:09 PM
This revision was automatically updated to reflect the committed changes.