Closed
Bug 1304672
Opened 7 years ago
Closed 7 years ago
Rename most AsmJS* names to Wasm
Categories
(Core :: JavaScript Engine: JIT, defect, P1)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla52
| Tracking | Status | |
|---|---|---|
| firefox52 | --- | fixed |
People
(Reporter: bbouvier, Assigned: bbouvier)
References
Details
Attachments
(16 files)
|
58 bytes,
text/x-review-board-request
|
luke
:
review+
|
Details |
|
58 bytes,
text/x-review-board-request
|
luke
:
review+
|
Details |
|
58 bytes,
text/x-review-board-request
|
luke
:
review+
|
Details |
|
58 bytes,
text/x-review-board-request
|
luke
:
review+
|
Details |
|
58 bytes,
text/x-review-board-request
|
luke
:
review+
|
Details |
|
58 bytes,
text/x-review-board-request
|
luke
:
review+
|
Details |
|
58 bytes,
text/x-review-board-request
|
luke
:
review+
|
Details |
|
58 bytes,
text/x-review-board-request
|
luke
:
review+
|
Details |
|
58 bytes,
text/x-review-board-request
|
luke
:
review+
|
Details |
|
58 bytes,
text/x-review-board-request
|
luke
:
review+
|
Details |
|
58 bytes,
text/x-review-board-request
|
luke
:
review+
|
Details |
|
58 bytes,
text/x-review-board-request
|
luke
:
review+
|
Details |
|
58 bytes,
text/x-review-board-request
|
luke
:
review+
|
Details |
|
58 bytes,
text/x-review-board-request
|
luke
:
review+
|
Details |
|
2.38 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
|
8.48 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
Let's celebrate 0xc. Patches based on Luke's error patch based on Dan's 0xc patch.
|
Assignee | |
Comment 1•7 years ago
|
||
Hmm, actually, I'll hold off making and uploading these patches because they might get easily out of sync, and although 0xc should land pretty soon, it's still unclear how soon is that.
|
||
Updated•7 years ago
|
Priority: -- → P5
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
||
Comment 16•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8806808 [details] Bug 1304672: Rename AsmReinterpret to WasmReinterpret; https://reviewboard.mozilla.org/r/90108/#review89876
Attachment #8806808 -
Flags: review?(luke) → review+
|
|
||
Comment 17•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8806809 [details] Bug 1304672: Rename AsmSelect to WasmSelect; https://reviewboard.mozilla.org/r/90110/#review89880
Attachment #8806809 -
Flags: review?(luke) → review+
|
|
||
Comment 18•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8806810 [details] Bug 1304672: Rename AsmJSUnsignedToDouble to WasmUnsignedToDouble; https://reviewboard.mozilla.org/r/90112/#review89882
Attachment #8806810 -
Flags: review?(luke) → review+
|
|
||
Comment 19•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8806811 [details] Bug 1304672: Rename AsmJSUnsignedToFloat32 to WasmUnsignedToFloat32; https://reviewboard.mozilla.org/r/90114/#review89884
Attachment #8806811 -
Flags: review?(luke) → review+
|
|
||
Comment 20•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8806812 [details] Bug 1304672: WasmBaselineCompiler should refer to the renamed Wasm{Load,Store}GlobalVar; https://reviewboard.mozilla.org/r/90116/#review89886
Attachment #8806812 -
Flags: review?(luke) → review+
|
|
||
Comment 21•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8806813 [details] Bug 1304672: Rename AsmJSReturn/AsmJSVoidReturn to WasmReturn/WasmReturnVoid; https://reviewboard.mozilla.org/r/90118/#review89888
Attachment #8806813 -
Flags: review?(luke) → review+
|
|
||
Comment 22•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8806814 [details] Bug 1304672: Rename AsmJSParameter to WasmParameter and AsmJSPassStackArg to WasmStackArg; https://reviewboard.mozilla.org/r/90120/#review89890
Attachment #8806814 -
Flags: review?(luke) → review+
|
|
||
Comment 23•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8806815 [details] Bug 1304672: Group AsmJS and Wasm MIR/LIR nodes together in opcodes list; https://reviewboard.mozilla.org/r/90122/#review89894 Ah, looks nice! AsmJSNeg is a funny outlier.
Attachment #8806815 -
Flags: review?(luke) → review+
|
|
||
Comment 24•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8806816 [details] Bug 1304672: Rename MBasicBlock::NewAsmJS into MBasicBlock::New; https://reviewboard.mozilla.org/r/90124/#review89896
Attachment #8806816 -
Flags: review?(luke) → review+
|
|
||
Comment 25•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8806817 [details] Bug 1304672: Rename NewAsmJS factory functions into New; https://reviewboard.mozilla.org/r/90126/#review89898
Attachment #8806817 -
Flags: review?(luke) → review+
|
|
||
Comment 26•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8806818 [details] Bug 1304672: Rename isCompilingAsmJS into isCompilingWasm; https://reviewboard.mozilla.org/r/90128/#review89900
Attachment #8806818 -
Flags: review?(luke) → review+
|
|
||
Comment 27•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8806819 [details] Bug 1304672: More jit/ renamings from asm.js to wasm; https://reviewboard.mozilla.org/r/90130/#review89866 ::: js/src/jit/shared/Assembler-shared.h:669 (Diff revision 1) > -// As an invariant across architectures, within asm.js code: > -// $sp % AsmJSStackAlignment = (sizeof(AsmJSFrame) + masm.framePushed) % AsmJSStackAlignment > -// Thus, AsmJSFrame represents the bytes pushed after the call (which occurred > -// with a AsmJSStackAlignment-aligned StackPointer) that are not included in > +// As an invariant across architectures, within wasm code: > +// $sp % WasmStackAlignment = (sizeof(WasmFrame) + masm.framePushed) % WasmStackAlignment > +// Thus, WasmFrame represents the bytes pushed after the call (which occurred > +// with a WasmStackAlignment-aligned StackPointer) that are not included in > // masm.framePushed. > -struct AsmJSFrame > +struct WasmFrame Could this become wasm::Frame instead? (Or maybe wasm::StackFrame?) ::: js/src/jit/shared/Assembler-shared.h:687 (Diff revision 1) > -static const uint32_t AsmJSFrameBytesAfterReturnAddress = sizeof(void*); > +static const uint32_t WasmFrameBytesAfterReturnAddress = sizeof(void*); > > // Represents an instruction to be patched and the intended pointee. These > // links are accumulated in the MacroAssembler, but patching is done outside > // the MacroAssembler (in Module::staticallyLink). > -struct AsmJSAbsoluteAddress > +struct WasmAbsoluteAddress I was going to suggest this be renamed to wasm::AbsoluteAddress but I expect, with unified builds, that'll conflict at some point with jit::AbsoluteAddress. Since this is parallel to wasm::GlobalAccess (just with a wasm::SymbolicAddress instead of globalDataOffset), how about renaming this to wasm::SymbolicAccess? In that case, the `namespace wasm {` can be moved up above wasm::Frame.
Attachment #8806819 -
Flags: review?(luke) → review+
|
|
||
Comment 28•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8806820 [details] Bug 1304672: Renamings outside asmjs/ and jit/; https://reviewboard.mozilla.org/r/90132/#review89902
Attachment #8806820 -
Flags: review?(luke) → review+
|
|
||
Comment 29•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8806821 [details] Bug 1304672: Rename asmjs/ directory to wasm/; https://reviewboard.mozilla.org/r/90134/#review89904 \o/ Nice job on all these patches. So I guess after 18 months (bug 1157624) we can now probably say "Mission Accomplished" to the incremental refactoring of asm.js into wasm ;) (And all without, to my knowledge, ever breaking or significantly asm.js in release, but I probably shouldn't jinx it with the last few months' patches still in flight...)
Attachment #8806821 -
Flags: review?(luke) → review+
|
||
Comment 30•7 years ago
|
||
Pushed by bbouvier@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/507bd72e0ff2 Rename AsmReinterpret to WasmReinterpret; r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/4b874e0db3b4 Rename AsmSelect to WasmSelect; r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/9ef4cab999be Rename AsmJSUnsignedToDouble to WasmUnsignedToDouble; r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/ebd1e91c164d Rename AsmJSUnsignedToFloat32 to WasmUnsignedToFloat32; r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/48bed4286905 WasmBaselineCompiler should refer to the renamed Wasm{Load,Store}GlobalVar; r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/4e2659c60071 Rename AsmJSReturn/AsmJSVoidReturn to WasmReturn/WasmReturnVoid; r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/f8565f00c8df Rename AsmJSParameter to WasmParameter and AsmJSPassStackArg to WasmStackArg; r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/b03e9ee30ba4 Group AsmJS and Wasm MIR/LIR nodes together in opcodes list; r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/0374ad289cf7 Rename MBasicBlock::NewAsmJS into MBasicBlock::New; r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/d44758ac4efe Rename NewAsmJS factory functions into New; r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/e1e289638d6a Rename isCompilingAsmJS into isCompilingWasm; r=luke
|
|
Assignee | |
Comment 31•7 years ago
|
||
Pushing the first few patches, before the requested changes (and a compilation failure, win only)
Keywords: leave-open
|
|
Assignee | |
Comment 32•7 years ago
|
||
The last patch seems to have changed the order of grouping files for unified compilation. vm/WeakMapPtr.cpp is now grouped with a file that contains a "using namespace jit;" line; the former defines a struct called DataType in the anonymous namespace, but there's also a jit::DataType somewhere in tree, and so MSVC is confused which one it should choose when instantiating a template below, see for instance https://treeherder.mozilla.org/#/jobs?repo=try&revision=eeb506ef6cdd&selectedJob=30396299 This changes the anonymous namespace to be a details:: namespace instead, which seems to do the trick locally at least.
Attachment #8807219 -
Flags: review?(luke)
|
|
||
Comment 33•7 years ago
|
||
Comment on attachment 8807219 [details] [diff] [review] fix-win-build.patch Review of attachment 8807219 [details] [diff] [review]: ----------------------------------------------------------------- Good fix! It'd be nice if we could segregate unified builds by directory...
Attachment #8807219 -
Flags: review?(luke) → review+
|
||
Comment 34•7 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/507bd72e0ff2 https://hg.mozilla.org/mozilla-central/rev/4b874e0db3b4 https://hg.mozilla.org/mozilla-central/rev/9ef4cab999be https://hg.mozilla.org/mozilla-central/rev/ebd1e91c164d https://hg.mozilla.org/mozilla-central/rev/48bed4286905 https://hg.mozilla.org/mozilla-central/rev/4e2659c60071 https://hg.mozilla.org/mozilla-central/rev/f8565f00c8df https://hg.mozilla.org/mozilla-central/rev/b03e9ee30ba4 https://hg.mozilla.org/mozilla-central/rev/0374ad289cf7 https://hg.mozilla.org/mozilla-central/rev/d44758ac4efe https://hg.mozilla.org/mozilla-central/rev/e1e289638d6a
|
|
||
Comment 35•7 years ago
|
||
Pushed by bbouvier@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/520c46902b45 More jit/ renamings from asm.js to wasm; r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/b135b519487e Renamings outside asmjs/ and jit/; r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/f37699322f20 Rename asmjs/ directory to wasm/; r=luke https://hg.mozilla.org/integration/mozilla-inbound/rev/28921ac795fa Fix for MSVC unified build bustage; r=luke
|
|
Assignee | |
Updated•7 years ago
|
Keywords: leave-open
Priority: P5 → P1
|
|
||
Comment 36•7 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/520c46902b45 https://hg.mozilla.org/mozilla-central/rev/b135b519487e https://hg.mozilla.org/mozilla-central/rev/f37699322f20 https://hg.mozilla.org/mozilla-central/rev/28921ac795fa
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
|
|
Assignee | |
Comment 37•6 years ago
|
||
Undo the renaming of debuggerObservesAsmJS => debuggerObservesWasm, as discussed.
Attachment #8809056 -
Flags: review?(luke)
|
|
||
Updated•6 years ago
|
Attachment #8809056 -
Flags: review?(luke) → review+
|
|
||
Comment 38•6 years ago
|
||
Pushed by bbouvier@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/56f7c0be1dda Undo the renaming of debuggerObservesAsmJS => Wasm; r=luke
|
||
Comment 39•6 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/56f7c0be1dda
You need to log in
before you can comment on or make changes to this bug.
Description
•