1577508 - Allow WebAssembly blocks to return multiple values

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, task, P1)

Description

[Andy Wingo [:wingo]]

The attached patch is a version of https://bugzilla.mozilla.org/attachment.cgi?id=9044748 from bug 1401675, which expands the representation of block types in the compiler to allow blocks to take and return values.

Comment 1

[Andy Wingo [:wingo]]

Attachment: Bug 1577508 - Allow WebAssembly blocks to return multiple values

Comment 2

[Andreea Pavel [:apavel]]

On Fri, September 20, 2019, 1:04 PM GMT+3, by [e-mail].
Revisions: D44142 diff 165585 ← D43604 diff 165588 ← D43977 diff 165604
Details: We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. applying /tmp/tmpzK4bTJ js/src/wasm/WasmBaselineCompile.cpp Hunk #6 FAILED at 6744. Hunk #8 FAILED at 8214. 2 out of 20 hunks FAILED -- saving rejects to file js/src/wasm/WasmBaselineCompile.cpp.rej abort: patch command failed: exited with status 256

Comment 3

[Andreea Pavel [:apavel]]

Tried to land this again after landing the other ones separately:

On Fri, September 20, 2019, 1:14 PM GMT+3, by [e-mail].
Revisions: D43977 diff 165604
Details: We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. applying /tmp/tmpZq6S2R js/src/wasm/WasmBaselineCompile.cpp Hunk #6 FAILED at 6744. Hunk #8 FAILED at 8214. 2 out of 20 hunks FAILED -- saving rejects to file js/src/wasm/WasmBaselineCompile.cpp.rej abort: patch command failed: exited with status 256

Comment 4

[Andy Wingo [:wingo]]

Thanks for landing and apologies for the mess. Rebased and uploaded.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8a9ae25b1019
Allow WebAssembly blocks to return multiple values r=luke

Comment 6

[Andreea Pavel [:apavel]]

(In reply to Andy Wingo [:wingo] from comment #4)

Thanks for landing and apologies for the mess. Rebased and uploaded.

No problem, thanks for fixing this.

Comment 7

[Andy Wingo [:wingo]]

(note to sheriffs: it looks from the bug like the patch has been applied already but in fact it was rolled out, has new modifications newly approved by luke, and needs applying. thank a million!)

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bf0900c02d78
Allow WebAssembly blocks to return multiple values r=luke

Comment 9

[Andreea Pavel [:apavel]]

Backed out for build bustages

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=bf0900c02d78aa1816a465ece607bbd2d5a81566&selectedJob=269579755

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=269579755&repo=autoland&lineNumber=3209

Backout: https://hg.mozilla.org/integration/autoland/rev/52bf76c9b4a484cd518d0ceb0ce8943a9e4f285f

Comment 10

[Andy Wingo [:wingo]]

Okeysmokes, fixed that, did a try build (https://treeherder.mozilla.org/#/jobs?repo=try&revision=daeaaa4127a7137688da2c48af91a4d0bc56f9ba&selectedJob=269601291), and fixed the one error there too. Let's give this another go!

Comment 11

[Andy Wingo [:wingo]]

(the arc diff completed after my last message but before this one)

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ed20677d8a91
Allow WebAssembly blocks to return multiple values r=luke

Comment 13

[Andreea Pavel [:apavel]]

Backed out for build bustages

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=269608054&resultStatus=testfailed%2Cbusted%2Cexception&revision=ed20677d8a912b1f4659d834e249345e57bc6b8e

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=269608054&repo=autoland&lineNumber=5241

Backout: https://hg.mozilla.org/integration/autoland/rev/64610aca4c7585f75f19b40b158ee1d136e4ba60

Comment 14

[Andy Wingo [:wingo]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=51486e7e508ff49ec4ec8a8bd2d64931ce74ca8a for a full try build

Comment 15

[Andy Wingo [:wingo]]

My last revision to fix a minor build problem on MacOS accidentally squished bug 1578418 onto the patch :/ Really sorry; will fix.

Comment 16

[Andy Wingo [:wingo]]

I think we're all green here AFAIU; last problem with the hazard check segfaulting (!) was fixed by the GCC upgrade on Wednesday. Let's go! :-)

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b868b0719ca6
Allow WebAssembly blocks to return multiple values r=luke

Comment 18

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/b868b0719ca6

Comment 19

[Lars T Hansen [:lth]]

I've requested a backpout for this, the sheriffs will attempt to back it out on mozilla-central.

Comment 20

[Raul Gurzau (:RaulG)]

Backout link: https://hg.mozilla.org/mozilla-central/rev/0d0bf1a4fa878505bc6fa36f5748107b42cebe12

Comment 21

[Andy Wingo [:wingo]]

Hi Lars -- I uploaded a fixed patch (https://phabricator.services.mozilla.com/D43977?vs=172731&id=176774#toc) and went through and verified that all fuzzer bugs were fixed by that patch. OK to reland?

Comment 22

[Lars T Hansen [:lth]]

OK, but please do it today if you can -- we're in a soft code freeze until Oct 21 and we don't want to risk any instability.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9f7109703bd0
Allow WebAssembly blocks to return multiple values r=luke

Comment 24

[Ryan VanderMeulen [:RyanVM]]

Can/should we land any of the fuzzer testcases from the previous landing attempt?

Comment 25

[Andy Wingo [:wingo]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #24)

Can/should we land any of the fuzzer testcases from the previous landing attempt?

I am not sure. On the one side, the problems they showed are only on nightly shell, and it was already known that the codegen wasn't complete. On the other, the expected behavior will change once multi-value lands (though of course it should never crash); and we don't have the facility to write text-format tests so that we can easily know what's being tested (https://bugzilla.mozilla.org/show_bug.cgi?id=1527871). Honestly I would punt; tests for this functionality will land shortly, the fuzzer found these issues quite promptly, and I think the fix is robust.

Comment 26

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/9f7109703bd0