Baldr: tidy up some things about wasm::Code

RESOLVED FIXED in Firefox 55

Status

()

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: luke, Assigned: luke)

Tracking

unspecified
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(4 attachments)

A couple small cleanup patches.
This patch adds a new UniquePtr typedef that releases code which makes ownership of code more foolproof.
Attachment #8867409 - Flags: review?(lhansen)
Posted patch rm-unlinkSplinter Review
Attachment #8867410 - Flags: review?(lhansen)
(Oops, hit enter too soon.)  The patch in comment 2 avoids the somewhat strange unlinkedBytesForDebugging() by instead making a copy of the bytes directly from the MacroAssembler before linking.  Also should be a tiny bit faster.
We're no longer doing mixed X/W mappings so, as the comment says...
Attachment #8867411 - Flags: review?(jdemooij)
This patch removes maybeBytecode from wasm::Code since wasm::Code basically has no need for it.  Instead, the 1 use (passing maybeBytecode_ to Metadata::getFuncName) is done in Instance instead which already has access to maybeBytecode via debug_.  This keeps wasm::Code more focused on just executable code, which I think is better.
Attachment #8867412 - Flags: review?(lhansen)
Comment on attachment 8867411 [details] [diff] [review]
rm-alloc-limit

Review of attachment 8867411 [details] [diff] [review]:
-----------------------------------------------------------------

Nice.
Attachment #8867411 - Flags: review?(jdemooij) → review+
Comment on attachment 8867409 [details] [diff] [review]
unique-code-bytes

Review of attachment 8867409 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/wasm/WasmCode.h
@@ +57,5 @@
>  typedef UniquePtr<const CodeSegment> UniqueConstCodeSegment;
>  
>  class CodeSegment
>  {
> +    // Executable code must be release deallocated specially.

Presumably "deallocated" is all you mean and "release" should be removed.
Attachment #8867409 - Flags: review?(lhansen) → review+
Attachment #8867410 - Flags: review?(lhansen) → review+
Comment on attachment 8867412 [details] [diff] [review]
rm-code-bytecode

Review of attachment 8867412 [details] [diff] [review]:
-----------------------------------------------------------------

Nice.  I strongly favor this, (even if it will interact in an ugly way with a long patch queue I have).
Attachment #8867412 - Flags: review?(lhansen) → review+
Pushed by lwagner@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/94bc2a2c7274
Baldr: use UniquePtr to own code (r=lth)
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8bbd7e9579a
Baldr: remove unlinkedBytesForDebugging (r=lth)
https://hg.mozilla.org/integration/mozilla-inbound/rev/4901d885b1a4
Baldr: remove MaxWasmCodeAllocations (r=jandem)
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e27f56d21b4
Baldr: remove bytecode from wasm::Code (r=lth)
You need to log in before you can comment on or make changes to this bug.