1635375 - Improve WarpBuilder IC logic

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, task, P1)

Description

[Jan de Mooij [:jandem]]

Currently if we don't inline CacheIR, we always use an Ion IC. If an IC site is cold, it's better to bail out so that Baseline can collect type information. This means there are 3 states we want the oracle to be able to communicate to the builder: bailout, inline CacheIR, use Ion IC.

The current WarpBuilder logic makes that awkward, so as a first step I have a patch that adds a buildIC method that takes a CacheKind + the list of inputs and then either inlines CacheIR or adds an Ion IC.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1635375 - Add a buildIC method to WarpBuilder that handles both transpiling and Ion IC cases. r?iain!

Having all logic for an IC site in WarpBuilder::buildIC will make it easier to
make future changes (like bailing out for cold Baseline ICs). It also enforces
all ICs are handled consistently and it de-duplicates code for JSOps that use
the same CacheKind.

This patch also adds transpiler support for some JSOps (In, InstanceOf, etc) and
we now assert the transpiler (buildIC) is called with the right number of inputs
(based on the CacheKind).

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aba1602191ce
Add a buildIC method to WarpBuilder that handles both transpiling and Ion IC cases. r=iain

Comment 3

[Narcis Beleuzu [:NarcisB]]

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

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1635375 part 2 - Tweak some jit-tests. r?iain!

When we bail out for cold ICs, some jit-tests fail because they expect to run in
Ion/Warp at certain points.

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1635375 part 3 - Eagerly recompile on osr-pc mismatch with --ion-eager. r?iain!

This fixes another test failure because we were not getting back into Warp code
quickly enough after a cold-IC bailout. With --ion-eager we want to compile without
waiting and that should probably also apply to recompiles.

Depends on D81063

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1635375 part 4 - Bail out in Warp for cold Baseline ICs. r?iain!

Until now we always used an Ion IC if there was no Baseline IC stub to transpile.
This isn't great for Baseline ICs that were never hit because we could get stuck
in Ion ICs instead of using the transpiler.

With this patch we bail out and if that happens more than 10 times we invalidate
the Warp code and then warm up and try a second time.

Depends on D81064

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1d8cc7e5439d
part 2 - Tweak some jit-tests. r=iain
https://hg.mozilla.org/integration/autoland/rev/97090086b8ed
part 3 - Eagerly recompile on osr-pc mismatch with --ion-eager. r=iain
https://hg.mozilla.org/integration/autoland/rev/5efefa92861d
part 4 - Bail out in Warp for cold Baseline ICs. r=iain

Comment 8

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/1d8cc7e5439d
https://hg.mozilla.org/mozilla-central/rev/97090086b8ed
https://hg.mozilla.org/mozilla-central/rev/5efefa92861d

Comment 9

[Jan de Mooij [:jandem]]

Attachment: Bug 1635375 part 5 - Move more IC stub code into ICCacheIR_Trait. r?iain!

Comment 10

[Jan de Mooij [:jandem]]

Attachment: Bug 1635375 part 6 - Use a single bit for the PreliminaryObject flag. r?iain!

Goal is to make it easy to add a second flag in the next patch.

Depends on D81820

Comment 11

[Jan de Mooij [:jandem]]

Attachment: Bug 1635375 part 7 - Invalidate Warp code when transpiled IC stubs change. r?iain!

Set a flag on stubs used by the transpiler and then treat those similar to a
constraint in the old system: invalidate when attaching a new IC stub (in front of
it), when updating the stub, or when unlinking the stub.

This is necessary to avoid repeated bailouts followed by a deoptimization in
CheckFrequentBailouts.

Depends on D81821

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7cab599bff96
part 5 - Move more IC stub code into ICCacheIR_Trait. r=iain
https://hg.mozilla.org/integration/autoland/rev/b65a3176a8a3
part 6 - Use a single bit for the PreliminaryObject flag. r=iain

Comment 13

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/7cab599bff96
https://hg.mozilla.org/mozilla-central/rev/b65a3176a8a3

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e3163482cc62
part 7 - Invalidate Warp code when transpiled IC stubs change. r=iain

Comment 15

[Jan de Mooij [:jandem]]

I think this is good enough for now.

Comment 16

[Natalia Csoregi [:nataliaCs]]

Backed out for bustages on BaselineIC.h and TrialInlining.cpp

backout: https://hg.mozilla.org/integration/autoland/rev/c32e0fa855e4fc5cce10f101382e779d088641ae

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=e3163482cc62d0c7aed592c6edcd733cf9475330&group_state=expanded

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=308797063&repo=autoland&lineNumber=14870

[task 2020-07-07T06:29:01.327Z] 06:29:01 INFO - In file included from Unified_cpp_js_src_jit10.cpp:38:
[task 2020-07-07T06:29:01.327Z] 06:29:01 INFO - /builds/worker/checkouts/gecko/js/src/jit/TrialInlining.cpp(51,34): error: too few arguments to function call, expected 2, have 1
[task 2020-07-07T06:29:01.327Z] 06:29:01 INFO - fallbackStub->discardStubs(cx());
[task 2020-07-07T06:29:01.327Z] 06:29:01 INFO - ~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
[task 2020-07-07T06:29:01.327Z] 06:29:01 INFO - /builds/worker/checkouts/gecko/js/src/jit/BaselineIC.h(708,3): note: 'discardStubs' declared here
[task 2020-07-07T06:29:01.327Z] 06:29:01 INFO - void discardStubs(JSContext* cx, JSScript* script);
[task 2020-07-07T06:29:01.327Z] 06:29:01 INFO - ^
[task 2020-07-07T06:29:01.327Z] 06:29:01 INFO - 1 error generated.
[task 2020-07-07T06:29:01.328Z] 06:29:01 ERROR - make[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:748: Unified_cpp_js_src_jit10.obj] Error 1
[task 2020-07-07T06:29:01.330Z] 06:29:01 INFO - make[4]: Leaving directory '/builds/worker/workspace/obj-build/js/src/jit'
[task 2020-07-07T06:29:01.331Z] 06:29:01 INFO - make[4]: *** Waiting for unfinished jobs....

Comment 17

[Jan de Mooij [:jandem]]

(In reply to Natalia Csoregi [:nataliaCs] from comment #16)

Backed out for bustages on BaselineIC.h and TrialInlining.cpp

Oops sorry, this conflicted with changes that are only on autoland.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c587c71cb1c8
part 7 - Invalidate Warp code when transpiled IC stubs change. r=iain

Comment 19

[Atila Butkovits]

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

Comment 20

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

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.