Bug 151335 - B3 Patchpoint and Check opcodes should be able to specify WarmAny, ColdAny, and LateColdAny
Summary: B3 Patchpoint and Check opcodes should be able to specify WarmAny, ColdAny, a...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
: 151811 (view as bug list)
Depends on: 151214
Blocks: 151808
  Show dependency treegraph
 
Reported: 2015-11-16 18:12 PST by Filip Pizlo
Modified: 2015-12-03 19:30 PST (History)
12 users (show)

See Also:


Attachments
work in progress (18.80 KB, patch)
2015-12-03 16:42 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (36.75 KB, patch)
2015-12-03 18:57 PST, Filip Pizlo
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2015-11-16 18:12:03 PST
That would give us the equivalent of early clobber.
Comment 1 Filip Pizlo 2015-12-01 11:42:28 PST
This is a bit tricky.  Currently, we have five opcodes that have stackmaps:

Patchpoint
Check
CheckAdd
CheckSub
CheckMul

CheckMul already uses LateUse for its stackmap, because the multiply cannot be undone.  Hence we need to ensure that if we want to recover any of the input values, they cannot overlap with the result.

The other opcodes currently have no way of specifying LateUse for their stackmaps.  It's tricky because currently the machinery for forcing LateUse in CheckMul relies on making the *entire* stackmap into LateUse.  Specifying LateUse on the entire stackmap is super dangerous because it also implies cold use, which is incompatible with constraints like SomeRegister.  What we really want is to specify LateUse on only some of the stackmap arguments. Ideally, there would be some way of saying LateUse in the ValueRep input constraints.  For example, we could have a ValueRep::Late, which is like Any but means LateUse.
Comment 2 Filip Pizlo 2015-12-03 14:44:18 PST
*** Bug 151811 has been marked as a duplicate of this bug. ***
Comment 3 Filip Pizlo 2015-12-03 16:42:14 PST
Created attachment 266575 [details]
work in progress
Comment 4 Filip Pizlo 2015-12-03 18:57:09 PST
Created attachment 266586 [details]
the patch
Comment 5 WebKit Commit Bot 2015-12-03 18:59:01 PST
Attachment 266586 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/b3/testb3.cpp:3681:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
Total errors found: 1 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Geoffrey Garen 2015-12-03 19:10:29 PST
Comment on attachment 266586 [details]
the patch

It feels a little weird to tie time of use, warmth of use, and location of use into a single value. But it seems not to be out of control just yet.
Comment 7 Filip Pizlo 2015-12-03 19:19:47 PST
(In reply to comment #6)
> Comment on attachment 266586 [details]
> the patch
> 
> It feels a little weird to tie time of use, warmth of use, and location of
> use into a single value. But it seems not to be out of control just yet.

It has a good chance of staying under control.  The only location of use for which warmth matters is *Any.  Warmth only influences register allocation.  All other locations have predetermined roles for the register allocator: Register means we have already picked a register, SomeRegister means we need it to be in some register no matter what (i.e. highest possible "warmth"), and StackArgument means it definitely should not be in a register.  Of the non-*Any locations, only SomeRegister could care about time of use: you could imagine LateSomeRegister. But currently late uses only arise for the purpose of OSR, so they are all cold (i.e. they don't care if they're in a register).

So then you could argue that this will get out of hand as soon as someone needs a warm late use. Then they might want LateWarmAny and LateSomeRegister. But I don't believe that they will need it. If you want a child to be constrained as LateWarmAny or LateSomeRegister, you could achieve that by append the child twice: once as WarmAny (or SomeRegister) and then again as LateColdAny.
Comment 8 Filip Pizlo 2015-12-03 19:30:21 PST
Landed in http://trac.webkit.org/changeset/193393