Rename most AsmJS* names to Wasm

RESOLVED FIXED in Firefox 52

Status

()

P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: bbouvier, Assigned: bbouvier)

Tracking

Trunk
mozilla52
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(16 attachments)

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
(Assignee)

Description

2 years ago
Let's celebrate 0xc. Patches based on Luke's error patch based on Dan's 0xc patch.
(Assignee)

Comment 1

2 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.
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

2 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

2 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

2 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

2 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

2 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

2 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

2 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

2 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

2 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

2 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

2 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

2 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

2 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

2 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

2 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

2 years ago
Pushing the first few patches, before the requested changes (and a compilation failure, win only)
Keywords: leave-open
(Assignee)

Comment 32

2 years ago
Created attachment 8807219 [details] [diff] [review]
fix-win-build.patch

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

2 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 35

2 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

2 years ago
Keywords: leave-open
Priority: P5 → P1

Comment 36

2 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
Last Resolved: 2 years ago
status-firefox52: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
(Assignee)

Comment 37

2 years ago
Created attachment 8809056 [details] [diff] [review]
undo-renaming.patch

Undo the renaming of debuggerObservesAsmJS => debuggerObservesWasm, as discussed.
Attachment #8809056 - Flags: review?(luke)

Updated

2 years ago
Attachment #8809056 - Flags: review?(luke) → review+

Comment 38

2 years ago
Pushed by bbouvier@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/56f7c0be1dda
Undo the renaming of debuggerObservesAsmJS => Wasm; r=luke
You need to log in before you can comment on or make changes to this bug.