Page MenuHomePhabricator

[WebAssembly] Compare functions by names in Emscripten Sjlj

Authored by aheejin on Sep 3 2019, 3:05 PM.



This removes all string constants for function names and compares
functions by string directly when needed. Many of these constants are
used only once or twice so the benefit of defining them separately is
not very clear, and this actually fixes a bug.

When we already have a malloc declaration which is an alias to
something else within the module,

@malloc = weak hidden alias i8* (i32), i8* (i32)* @dlmalloc

(this happens compiling with emscripten with -s WASM_OBJECT_FILES=0
because all bc files are merged before being fed into wasm-ld which
runs the backend optimizations as LTO)

Module::getFunction("malloc") in canLongjmp returns nullptr
because Module::getFunction dyncasts pointer into Function, but the
alias is a GlobalValue but not a Function. This makes canLongjmp
return false for malloc in this case, and we end up adding a lot of
longjmp handling code around malloc. This is not only a code size
increase but actually a bug because malloc is used in the entry block
when preparing for setjmp tables for emscripten sjlj handling, and this
makes initial setjmp preparation, which has to happen in the entry
block, move to another split block, and this interferes with SSA update

This also adds two more functions, getTempRet0 and setTempRet0, in
the list of not longjmp-able functions.


Diff Detail


Event Timeline

aheejin created this revision.Sep 3 2019, 3:05 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 3 2019, 3:05 PM
aheejin edited the summary of this revision. (Show Details)Sep 3 2019, 3:08 PM
aheejin edited the summary of this revision. (Show Details)
aheejin edited the summary of this revision. (Show Details)Sep 3 2019, 3:10 PM
sbc100 accepted this revision.Sep 3 2019, 3:13 PM
sbc100 added inline comments.
1 ↗(On Diff #218540)

Seems like this test is more about "aliases" that it is about malloc in particular. Perhaps it should be called lower-em-sjlj-alias.ll?

This revision is now accepted and ready to land.Sep 3 2019, 3:13 PM
aheejin updated this revision to Diff 218544.Sep 3 2019, 3:21 PM
aheejin marked an inline comment as done.
  • Renamed to lower-em-sjlj-alias.ll
This revision was automatically updated to reflect the committed changes.