Details

Reviewers

Commits

rMOZILLACENTRALef79fb63a43c: Bug 1891182 - part 10 - remove Metadata and make AsmJSMetadata standalone.
rMOZILLACENTRAL819d998f9604: Bug 1891182 - part 10 - remove Metadata and make AsmJSMetadata standalone.

Bugzilla Bug ID

1891182

Summary

The #09 patch removed all wasm-specific fields from wasm::Metadata, but did
not remove wasm::Metadata itself, because it is inherited from by
AsmJSMetadata, and used to provide different behaviour for wasm vs asm.js in a
few obscure cases related to the profiler.

This patch restricts wasm::Metadata to be an abstract class that provides
access to (is the pure virtual base class of) AsmJSMetadata. wasm::Metadata is
removed from WasmCode.h and instead reappears in AsmJS.h in pure virtual form.
Any place that previously took a Metadata& now takes takes a Metadata*, and
that is non-null only in the case when we are compiling asm.js.

The effect is to restrict wasm::Metadata and js::AsmJSMetadata to providing
support for asm.js compilation only. The next patch in the series (#11)
completes the transformation by renaming those two types appropriately.

Diff Detail

Repository: rMOZILLACENTRAL mozilla-central

Event Timeline

jseward created this revision.May 22 2024, 9:14 AM

Herald added a project: secure-revision. · View Herald TranscriptMay 22 2024, 9:14 AM

Harbormaster completed remote builds in B662988: Diff 865365.May 22 2024, 9:14 AM

phab-bot published this revision for review.May 22 2024, 9:14 AM

phab-bot changed the visibility from "Custom Policy" to "Public (No Login Required)".

phab-bot changed the edit policy from "Custom Policy" to "Restricted Project (Project)".

phab-bot removed a project: secure-revision.

jseward added a child revision: D211167: Bug 1891182 - part 11 - rename what remains of Metadata to CodeMetadataForAsmJS. r=rhunt..May 22 2024, 9:14 AM

rhunt accepted this revision.Jun 4 2024, 5:10 PM

rhunt added a project: testing-exception-unchanged (Doesn't change behavior for users).

rhunt added inline comments.

js/src/wasm/AsmJS.cpp
417–419	Yeah this is not correct, but it wasn't correct before this patch either. You'd need to include the sizes of all the malloc data. It's probably not worth it for now?
js/src/wasm/WasmCode.h
256–267	This should be a method on CodeMetadata, it's only ever called using fields from that class.

This revision is now accepted and ready to land.Jun 4 2024, 5:10 PM

jseward marked an inline comment as done.Jun 13 2024, 10:11 AM

jseward updated this revision to Diff 878325.Jun 18 2024, 3:08 PM

Harbormaster completed remote builds in B673751: Diff 878325.Jun 18 2024, 3:08 PM

Code analysis found 3 defects in diff 878325:

1 defect found by clang-format
2 defects found by clang-tidy

WARNING: Found 3 defects (warning level) that can be dismissed.

You can run this analysis locally with:

./mach static-analysis check --outgoing (C/C++)
./mach clang-format -p js/src/wasm/AsmJS.h

For your convenience, here is a patch that fixes all the clang-format defects (use it in your repository with hg import or git apply -p0).

If you see a problem in this automated review, please report it here.

You can view these defects in the Diff Detail section of Phabricator diff 878325.

jseward updated this revision to Diff 878819.Jun 19 2024, 11:12 AM

Harbormaster completed remote builds in B674137: Diff 878819.Jun 19 2024, 11:12 AM

Closed by commit rMOZILLACENTRAL819d998f9604: Bug 1891182 - part 10 - remove Metadata and make AsmJSMetadata standalone. (authored by jseward). · Explain WhyJun 19 2024, 5:30 PM

This revision was automatically updated to reflect the committed changes.

jseward added a commit: rMOZILLACENTRAL819d998f9604: Bug 1891182 - part 10 - remove Metadata and make AsmJSMetadata standalone..

ctuns reopened this revision.Jun 19 2024, 9:46 PM

This revision is now accepted and ready to land.Jun 19 2024, 9:46 PM

ctuns added a reverting change: rMOZILLACENTRAL669ab0842db8: Backed out 14 changesets (bug 1891182) for causing failures in….Jun 19 2024, 9:46 PM

jseward updated this revision to Diff 879687.Jun 20 2024, 2:53 PM

Harbormaster completed remote builds in B674810: Diff 879687.Jun 20 2024, 2:53 PM

Code analysis found 3 defects in diff 879687:

1 defect found by clang-format
2 defects found by clang-tidy

WARNING: Found 3 defects (warning level) that can be dismissed.

You can run this analysis locally with:

./mach static-analysis check --outgoing (C/C++)
./mach clang-format -p js/src/wasm/AsmJS.h

For your convenience, here is a patch that fixes all the clang-format defects (use it in your repository with hg import or git apply -p0).

If you see a problem in this automated review, please report it here.

You can view these defects in the Diff Detail section of Phabricator diff 879687.

Closed by commit rMOZILLACENTRALef79fb63a43c: Bug 1891182 - part 10 - remove Metadata and make AsmJSMetadata standalone. (authored by jseward). · Explain WhyJun 21 2024, 4:46 AM

This revision was automatically updated to reflect the committed changes.

jseward added a commit: rMOZILLACENTRALef79fb63a43c: Bug 1891182 - part 10 - remove Metadata and make AsmJSMetadata standalone..

Bug 1891182 - part 10 - remove Metadata and make AsmJSMetadata standalone. r=rhunt.
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 880138

js/src/debugger/Source.cpp

js/src/vm/MemoryMetrics.cpp

js/src/wasm/AsmJS.h

js/src/wasm/AsmJS.cpp

js/src/wasm/WasmCode.h

js/src/wasm/WasmCode.cpp

js/src/wasm/WasmDebug.h

js/src/wasm/WasmDebug.cpp

js/src/wasm/WasmFrameIter.cpp

js/src/wasm/WasmGenerator.h

js/src/wasm/WasmGenerator.cpp

js/src/wasm/WasmInstance-inl.h

js/src/wasm/WasmInstance.h

js/src/wasm/WasmInstance.cpp

js/src/wasm/WasmModule.h

js/src/wasm/WasmSerialize.cpp

js/src/wasm/WasmTypeDecls.h

Bug 1891182 - part 10 - remove Metadata and make AsmJSMetadata standalone. r=rhunt.ClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 880138

js/src/debugger/Source.cpp

js/src/vm/MemoryMetrics.cpp

js/src/wasm/AsmJS.h

js/src/wasm/AsmJS.cpp

js/src/wasm/WasmCode.h

js/src/wasm/WasmCode.cpp

js/src/wasm/WasmDebug.h

js/src/wasm/WasmDebug.cpp

js/src/wasm/WasmFrameIter.cpp

js/src/wasm/WasmGenerator.h

js/src/wasm/WasmGenerator.cpp

js/src/wasm/WasmInstance-inl.h

js/src/wasm/WasmInstance.h

js/src/wasm/WasmInstance.cpp

js/src/wasm/WasmModule.h

js/src/wasm/WasmSerialize.cpp

js/src/wasm/WasmTypeDecls.h

Bug 1891182 - part 10 - remove Metadata and make AsmJSMetadata standalone. r=rhunt.
ClosedPublic
Actions

Revision Contents
Changeset List