1699271 - [meta] Auto generate MIR, LIR, and WarpCacheIRTranspiler boilerplate

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement, P3)

Description

[Caroline Cullen [:caroline]]

Much of MIR class definition, LIR code, and WarpCacheIRTranspiler is very mechanical and can be generated similarly to what was done for auto generating CacheIR code.

Comment 1

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 1: Generate MIR Opcodes. r?jandem!

Comment 2

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 2: Auto generate simple nullary MIR Ops. r?jandem!

Depends on D111043

Comment 3

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 3: Auto generate simple MIR Ops with constructor args. r?iain!

Depends on D111044

Comment 4

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 4: Auto generate MIR ops with congruentTo methods. r?jandem!

Depends on D111045

Comment 5

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 5: Auto generate MIR ops that contain foldsTo methods. r?iain!

Depends on D111046

Comment 6

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 6: Auto generate ops that possibly call. r?jandem!

Depends on D111047

Comment 7

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 7: Auto generate MIR ops that compute range. r?iain!

Depends on D111048

Comment 8

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 8: Auto generate more MIR ops. r?iain!

Depends on D111049

Comment 9

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 9: Auto generate MIR ops that can recover from bailouts. r?jandem!

Depends on D111050

Comment 10

[Lars T Hansen [:lth]]

The obvious question for me is, "does this reduce essential complexity?", and the answer is imo not obviously "yes". Looking at a few of the patches here, I see that generally the YAML is about as long (lines of code) as the C++ it replaces. Add to that that the YAML has ad-hoc semantics, is untyped and indentation-structured, and is processed by a dynamically typed program, and I have a hard time seeing the upside compared to, say, further macroization of the MIR. I agree that the MIR boilerplate is annoying, but does this patch set address anything deep or open up for other important work in the future?

Is there a backgrounder for this work somewhere, a design doc or notes from a discussion?

Re the implementation of the idea, why barely-structured YAML and not well-structured JSON? Why untyped Python and not TypeScript or Go?

(Sorry to come across as negative; I'm only looking for enlightenment.)

Comment 11

[Iain Ireland [:iain]]

The main background for this work is that we are currently using a very similar system for supporting CacheIR (see CacheIROps.yaml and GenerateCacheIRFiles.py), and it has been an overwhelming success. Adding a new CacheIR op in baseline is very smooth: you define it in the yaml file, you use it in CacheIR.cpp, and you implement it in CacheIRCompiler.cpp. We previously had a macro-based system for handling CacheIR boilerplate, but it was already creaking under its own weight, and we had several new features to add. Switching to a code generator was an unambiguous improvement.

Transpiling a new CacheIR op in Warp is less nice: if you add a new MIR op, you need to write somewhat finicky code in at least five places, most of which is boilerplate. The purpose of this project is to explore whether we can get similar benefits when auto-generating MIR that we saw from CacheIR. In a perfect world, we could add new functionality -- say, supporting a new inlineable native function -- while only touching the yaml files (to define boilerplate), CacheIR.cpp (to decide when to use the new op), CacheIRCompiler.cpp (to generate IC code), and CodeGenerator.cpp (to generate Warp/Ion code). We might not be able to get there, but it seems valuable to at least try writing a prototype and see how it feels. (Note that the current patches are just Caroline's first draft being put up for feedback. There will likely be significant changes. If it turns out that, even after polishing, some of the boilerplate we're hoping to remove was better without the code generator, we won't land those parts.)

One other benefit of auto-generation is that it makes it easier to experiment with different MIR implementations: for example, devirtualizing MIR in favour of auto-generated switch statements.

In terms of implementation choices: Python is nice because it's already well-supported in our build system. YAML over JSON was pretty arbitrary: YAML is a little nicer to write by hand, and JSON doesn't support comments. Neither choice has had any issues in the CacheIR implementation, It's certainly possible that we could run into problems in the future, but this code runs as part of the build process and produces the same output every time, so there's not a lot of room for bugs to slip in.

Comment 12

[Lars T Hansen [:lth]]

Thanks for the backgrounder / perspective. I'll be interested in following this work and see where it goes. Clearly the amount of boilerplate in Ion is smaller these days than it used to be but is still annoyingly large, and I have no arguments against generating code where we can.

(I admit to strong negative biases re untyped languages, untyped data, and indentation-based structuring; that said, I don't really think justifying them with "it's what we've done in the past" is all that great either. A discussion for another day. "JSON doesn't support comments" seems highly relevant, though.)

Comment 13

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 10: Auto generate more MIR instructions. r?iain!

Depends on D111051

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bbce3487d14f
Part 1: Generate MIR Opcodes. r=jandem,iain
https://hg.mozilla.org/integration/autoland/rev/bf6f19cc2331
Part 2: Auto generate simple nullary MIR Ops. r=iain
https://hg.mozilla.org/integration/autoland/rev/2cb147cc9ae2
Part 3: Auto generate simple MIR Ops with constructor args. r=iain
https://hg.mozilla.org/integration/autoland/rev/9ce262e1f44b
Part 4: Auto generate MIR ops with congruentTo methods. r=iain
https://hg.mozilla.org/integration/autoland/rev/ed0d74615437
Part 5: Auto generate MIR ops that contain foldsTo methods. r=iain
https://hg.mozilla.org/integration/autoland/rev/5f6a27d7e131
Part 6: Auto generate ops that possibly call. r=iain
https://hg.mozilla.org/integration/autoland/rev/566c2b19754e
Part 7: Auto generate MIR ops that compute range. r=iain
https://hg.mozilla.org/integration/autoland/rev/822533d1632f
Part 8: Auto generate more MIR ops. r=iain
https://hg.mozilla.org/integration/autoland/rev/b13552f6d81e
Part 9: Auto generate MIR ops that can recover from bailouts. r=iain
https://hg.mozilla.org/integration/autoland/rev/cbebfcacd934
Part 10: Auto generate more MIR instructions. r=iain

Comment 15

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/bbce3487d14f
https://hg.mozilla.org/mozilla-central/rev/bf6f19cc2331
https://hg.mozilla.org/mozilla-central/rev/2cb147cc9ae2
https://hg.mozilla.org/mozilla-central/rev/9ce262e1f44b
https://hg.mozilla.org/mozilla-central/rev/ed0d74615437
https://hg.mozilla.org/mozilla-central/rev/5f6a27d7e131
https://hg.mozilla.org/mozilla-central/rev/566c2b19754e
https://hg.mozilla.org/mozilla-central/rev/822533d1632f
https://hg.mozilla.org/mozilla-central/rev/b13552f6d81e
https://hg.mozilla.org/mozilla-central/rev/cbebfcacd934

Comment 16

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 11: Add autogeneration for MIR ops that take arguments. r?iain!

Comment 17

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 12: Add ops that require TempAllocator. r?iain!

Depends on D116775

Comment 18

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 12: Remove unnecessary temporary allocator MIR ops. r?iain!

Depends on D116775

Comment 19

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 13: Add auto generated static assertions for non gc pointer arguments to MIR ops. r?iain!

Depends on D117120

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/599f833f08c1
Part 11: Add autogeneration for MIR ops that take arguments. r=iain
https://hg.mozilla.org/integration/autoland/rev/f7abfb7d50d2
Part 12: Remove unnecessary temporary allocator MIR ops. r=iain
https://hg.mozilla.org/integration/autoland/rev/26a855a6149e
Part 13: Add auto generated static assertions for non gc pointer arguments to MIR ops. r=iain

Comment 21

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/599f833f08c1
https://hg.mozilla.org/mozilla-central/rev/f7abfb7d50d2
https://hg.mozilla.org/mozilla-central/rev/26a855a6149e

Comment 22

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 14: Fix up mir auto gen assertion patch and add assertion message. r?iain!

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/00819b6018f9
Part 14: Fix up mir auto gen assertion patch and add assertion message. r=iain

Comment 24

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/00819b6018f9

Comment 25

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 15: Generate LIR opcodes from YAML. r?iain!

Comment 26

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 16: Generate LIR nodes. r?iain!

Depends on D119720

Comment 27

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 17: Generate LIR call instructions. r?iain!

Depends on D119721

Comment 28

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 18: Generate LIR instructions that produce LDefintions. r?iain!

Depends on D119722

Comment 29

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 19: Generate LIR ops that take operands. r?iain!

Depends on D119723

Comment 30

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 20: Generate LIR ops that use temporary registers. r?iain!

Depends on D119724

Comment 31

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 21: Generate more LIR nodes. r?iain!

Depends on D119725

Comment 32

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 22: Generate more LIR ops. r?iain!

Depends on D119726

Comment 33

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 23: Generate LIR nodes that take arguments. r?iain!

Depends on D119727

Comment 34

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 15: Generate LIR opcodes from YAML. r?iain!

Comment 35

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 16: Generate LIR nodes. r?iain!

Depends on D120277

Comment 36

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 17: Generate LIR call instructions. r?iain!

Depends on D120278

Comment 37

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 18: Generate LIR instructions that produce LDefintions. r?iain!

Depends on D120279

Comment 38

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 19: Generate LIR ops that take operands. r?iain!

Depends on D120280

Comment 39

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 20: Generate LIR ops that use temporary registers. r?iain!

Depends on D120282

Comment 40

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 21: Generate more LIR nodes. r?iain!

Depends on D120283

Comment 41

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 22: Generate LIR ops that produce value or int64 definitions. r?iain!

Depends on D120284

Comment 42

[Caroline Cullen [:caroline]]

Attachment: Bug 1699271 - Part 23: Generate LIR nodes that take arguments. r?iain!

Depends on D120285

Comment 43

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/98e70cb9f81a
Part 15: Generate LIR opcodes from YAML. r=iain
https://hg.mozilla.org/integration/autoland/rev/59114f24f348
Part 16: Generate LIR nodes. r=iain
https://hg.mozilla.org/integration/autoland/rev/9c17091087a8
Part 17: Generate LIR call instructions. r=iain
https://hg.mozilla.org/integration/autoland/rev/778ead7a44ba
Part 18: Generate LIR instructions that produce LDefintions. r=iain
https://hg.mozilla.org/integration/autoland/rev/e25df64152c2
Part 19: Generate LIR ops that take operands. r=iain
https://hg.mozilla.org/integration/autoland/rev/66491995508e
Part 20: Generate LIR ops that use temporary registers. r=iain
https://hg.mozilla.org/integration/autoland/rev/898028ee73ec
Part 21: Generate more LIR nodes. r=iain
https://hg.mozilla.org/integration/autoland/rev/54c739cc8d2f
Part 22: Generate more LIR ops. r=iain
https://hg.mozilla.org/integration/autoland/rev/1053a0c611ba
Part 23: Generate LIR nodes that take arguments. r=iain

Comment 44

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/98e70cb9f81a
https://hg.mozilla.org/mozilla-central/rev/59114f24f348
https://hg.mozilla.org/mozilla-central/rev/9c17091087a8
https://hg.mozilla.org/mozilla-central/rev/778ead7a44ba
https://hg.mozilla.org/mozilla-central/rev/e25df64152c2
https://hg.mozilla.org/mozilla-central/rev/66491995508e
https://hg.mozilla.org/mozilla-central/rev/898028ee73ec
https://hg.mozilla.org/mozilla-central/rev/54c739cc8d2f
https://hg.mozilla.org/mozilla-central/rev/1053a0c611ba

Comment 45

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 46

[BugBot [:suhaib / :marco/ :calixte]]

The meta keyword is there, the bug doesn't depend on other bugs and there is no activity for 12 months.
:willyelm, maybe it's time to close this bug?

Comment 47

[Will Medina [:willyelm]]

Hi Jan, do you have additional information about this bug?

Comment 48

[Jan de Mooij [:jandem]]

Patches landed in this bug so let's close it. We can file a new bug for other work in this area.