Bug 164904 – We should support CreateThis in the FTL

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Saam Barati]

...

[Saam Barati]

Created attachment 295471 [details]
patch

[Filip Pizlo]

Comment on attachment 295471 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=295471&action=review

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9506
> +        LValue callee = lowCell(m_node->child1());

You should do LBasicBlock lastNext = m_out.insertNewBlocksBefore(isFunctionBlock) here.  That way, if lowCell produces blocks, they will be nicely ordered.

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9507
> +        m_out.branch(isFunction(callee, provenType(m_node->child1())), unsure(isFunctionBlock), unsure(slowPath));

Could this be usually(isFunctionBlock), rarely(slowPath)?  Our register allocator will do a better job on the slow path call this way.

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9508
> +        LBasicBlock lastNext = m_out.appendTo(isFunctionBlock, hasRareData);

Then you don't need to save lastNext here.

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9511
> +        m_out.branch(m_out.isZero64(rareData), unsure(slowPath), unsure(hasRareData));

Ditto.

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9528
> +    }

You need m_out.mutatorFence() here because you just allocated an object.

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9961
> +        LValue id = m_out.load32(structure, m_heaps.Structure_structureID);
> +        LValue blob = m_out.load32(structure, m_heaps.Structure_initializationBlob);

You should have code to attempt to constant-fold these loads.  Maybe you can even add a helper in FTL::Output called m_out.constLoad32, which will check if its input LValue is a constant, and if so, it will just run the load and return the constant.  If you do this, you don't need to use templates and you don't need to duplicate the code for storeStructure.

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9998
> +    template <typename StructureType>
>      LValue allocateObject(
> -        LValue allocator, Structure* structure, LValue butterfly, LBasicBlock slowPath)
> +        LValue allocator, StructureType structure, LValue butterfly, LBasicBlock slowPath)

This is strange: we're handling the possible constness of the allocator by using LValue and then querying if it's a constant, but instead of doing that for structure, you used templates.

I think that using LValue structure is better.  Places that want to do smart things in case it's a constant can extract that constant from the LValue.

[Saam Barati]

Comment on attachment 295471 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=295471&action=review

>> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9506
>> +        LValue callee = lowCell(m_node->child1());
> 
> You should do LBasicBlock lastNext = m_out.insertNewBlocksBefore(isFunctionBlock) here.  That way, if lowCell produces blocks, they will be nicely ordered.

We don't really seem to do this elsewhere, do you think it's worth changing?

>> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9507
>> +        m_out.branch(isFunction(callee, provenType(m_node->child1())), unsure(isFunctionBlock), unsure(slowPath));
> 
> Could this be usually(isFunctionBlock), rarely(slowPath)?  Our register allocator will do a better job on the slow path call this way.

I'll do this.

>> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9511
>> +        m_out.branch(m_out.isZero64(rareData), unsure(slowPath), unsure(hasRareData));
> 
> Ditto.

I'll change this too.

>> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9528
>> +    }
> 
> You need m_out.mutatorFence() here because you just allocated an object.

Will add.

>> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:9998
>> +        LValue allocator, StructureType structure, LValue butterfly, LBasicBlock slowPath)
> 
> This is strange: we're handling the possible constness of the allocator by using LValue and then querying if it's a constant, but instead of doing that for structure, you used templates.
> 
> I think that using LValue structure is better.  Places that want to do smart things in case it's a constant can extract that constant from the LValue.

Ok I'll make it an LValue everwhere.

[Saam Barati]

Created attachment 295518 [details]
patch

Addressed Fil's feedback.

[Build Bot]

Comment on attachment 295518 [details]
patch

Attachment 295518 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2586718

New failing tests:
intersection-observer/intersection-observer-entry-interface.html

[Build Bot]

Created attachment 295582 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6

[Saam Barati]

(In reply to comment #5)
> Comment on attachment 295518 [details]
> patch
> 
> Attachment 295518 [details] did not pass ios-sim-ews (ios-simulator-wk2):
> Output: http://webkit-queues.webkit.org/results/2586718
> 
> New failing tests:
> intersection-observer/intersection-observer-entry-interface.html

I seriously doubt this is actually from this patch.

[Geoffrey Garen]

Comment on attachment 295518 [details]
patch

r=me

[WebKit Commit Bot]

Comment on attachment 295518 [details]
patch

Clearing flags on attachment: 295518

Committed r209112: <http://trac.webkit.org/changeset/209112>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Yusuke Suzuki]

Hm, it seems that this patch causes regression in Octane/Raytrace.

[Saam Barati]

(In reply to comment #11)
> Hm, it seems that this patch causes regression in Octane/Raytrace.

This is weird. I'm investigating now.

[Saam Barati]

(In reply to comment #12)
> (In reply to comment #11)
> > Hm, it seems that this patch causes regression in Octane/Raytrace.
> 
> This is weird. I'm investigating now.

I'm still not sure why it's slower. It's not because we're hitting the slow path more. So it must be because something else.

My current hypothesis is that it might be related to emitting a MultiGetByOffset instead of a GetById somewhere, which causes us to recompile. The reason I think this is that if I disable access inlining, the patch with FTL CreateThis runs faster. But if accessInlining is enabled, it's slower.

[Chris Dumez]

(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > Hm, it seems that this patch causes regression in Octane/Raytrace.
> > 
> > This is weird. I'm investigating now.
> 
> I'm still not sure why it's slower. It's not because we're hitting the slow
> path more. So it must be because something else.
> 
> My current hypothesis is that it might be related to emitting a
> MultiGetByOffset instead of a GetById somewhere, which causes us to
> recompile. The reason I think this is that if I disable access inlining, the
> patch with FTL CreateThis runs faster. But if accessInlining is enabled,
> it's slower.

JetStream also regressed.

[Filip Pizlo]

(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > Hm, it seems that this patch causes regression in Octane/Raytrace.
> > 
> > This is weird. I'm investigating now.
> 
> I'm still not sure why it's slower. It's not because we're hitting the slow
> path more. So it must be because something else.
> 
> My current hypothesis is that it might be related to emitting a
> MultiGetByOffset instead of a GetById somewhere, which causes us to
> recompile. The reason I think this is that if I disable access inlining, the
> patch with FTL CreateThis runs faster. But if accessInlining is enabled,
> it's slower.

You probably broke object allocation sinking.

That's only effective in Raytrace if all arguments forwarding is consistently blown away.

It would be good to get this fixed rapidly so we don't lose test coverage on object allocation sinking.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 165208

[Saam Barati]

(In reply to comment #15)
> (In reply to comment #13)
> > (In reply to comment #12)
> > > (In reply to comment #11)
> > > > Hm, it seems that this patch causes regression in Octane/Raytrace.
> > > 
> > > This is weird. I'm investigating now.
> > 
> > I'm still not sure why it's slower. It's not because we're hitting the slow
> > path more. So it must be because something else.
> > 
> > My current hypothesis is that it might be related to emitting a
> > MultiGetByOffset instead of a GetById somewhere, which causes us to
> > recompile. The reason I think this is that if I disable access inlining, the
> > patch with FTL CreateThis runs faster. But if accessInlining is enabled,
> > it's slower.
> 
> You probably broke object allocation sinking.
> 
> That's only effective in Raytrace if all arguments forwarding is
> consistently blown away.
> 
> It would be good to get this fixed rapidly so we don't lose test coverage on
> object allocation sinking.
Yeah it appears to be because of this. But interestingly, it appears to be because we end up not inlining the constructor function as much. If I explicitly prevent the constructor from being FTL compiled, the regression is gone. This is extremely weird. I'm still trying to figure out why the FTL compilation is preventing inlining.

[Saam Barati]

I'm still quite confused as to why this is happening. One symptom (or maybe the cause) I've noticed is happening is that we're not constant folding some GetByIds that we were constant folding before. This obviously prevents allocation sinking. But it also prevents inlining of a particular function call. So I think my next step is to figure out why we're not constant folding this anymore.

Also another interesting tidbit:
If I prevent the compilation of a particular constructor in the FTL (i.e, the top-level compilation), the perf regression goes away. So it's somewhat confusing that compiling the top-level function is somehow preventing optimizations when it gets inlined later on.

[Geoffrey Garen]

> Also another interesting tidbit:
> If I prevent the compilation of a particular constructor in the FTL (i.e,
> the top-level compilation), the perf regression goes away. So it's somewhat
> confusing that compiling the top-level function is somehow preventing
> optimizations when it gets inlined later on.

Maybe the non-inlined version of the function produces a value profile that enables constant folding, whereas, for some reason, the inlined version does not propagate type information that's as precise.

[Filip Pizlo]

(In reply to comment #19)
> > Also another interesting tidbit:
> > If I prevent the compilation of a particular constructor in the FTL (i.e,
> > the top-level compilation), the perf regression goes away. So it's somewhat
> > confusing that compiling the top-level function is somehow preventing
> > optimizations when it gets inlined later on.
> 
> Maybe the non-inlined version of the function produces a value profile that
> enables constant folding, whereas, for some reason, the inlined version does
> not propagate type information that's as precise.

Right!

This could be one of those bugs caused by someone having things return Top in the prediction propagation phase.

[Saam Barati]

Ok, so I'm pretty sure that the bug is timing related. It's definitely timing related when running without the concurrent JIT, but it's probably timing related when running with the concurrent JIT, too, but that's hard to verify 100%.

Without the concurrent JIT, this is what happens:

In the benchmark, there is a function defined as:
```
var Class = {
  create: function(s) {
    function r() {
      this.initialize.apply(this, arguments);
    };
    return r;
  }
};
```

and then this is used like this:

```
var c1 = Class.create();
c1.prototype = {
  ... some properties
  initialize: function(...) { ... }
  ... more properties
}

var c2 = Class.create();
c2.prototype = {
  ... some properties
  initialize: function(...) { ... }
  ... more properties
}

... a few more Class.create() 

```

The issue is occurring with the "initialize" GetById inside the function "r". What happens is this:

We tier up that function to the FTL before all the various Class.create() prototypes are seen by the inline cache. This isn't problematic if the get_by_id is compiled as GetById by the DFG, which it is. But it becomes a problem in the FTL, because we compile the get_by_id as MultiGetByOffset. Because the inline caching hasn't yet seen all the various prototype structures, the MultiGetByOffset will eventually exit when it starts to see new incoming structures. This exit then pollutes the GetByIdStatus when the function "r" gets inlined later on, causing us to not constant fold it because GetByIdStatus claims to take the slow path any time it sees an exit site with BadCache. Therefore, when "r" gets inlined, we end up with a GetById, instead of a constant value. This is one part of the slow down. The GetById prevents many occurrences of allocation sinking and inlining. Secondly, the DFG compile also slows us down because it continually exits after it gets compiled because it sees structures it hadn't anticipated. These together cause the total slow down.

I'm trying to figure out the best way to prevent this behavior when we try to inline "r" because when we inline "r" we know exactly which structure will be produced by the incoming value to the get_by_id because the base always is a NewObject node. Because of this, I think it's safe to ignore the BadCache exit site because we've proven what the incoming Structure is. That said, I'm not sure if this will entirely eliminate the slow down, because the FTL compilation of the standalone "r" is slower than the DFG version because of the exits from MultiGetByOffset. Note that if I manually force this one MultiGetByOffset to become a GetById, the entire perf regression goes away and things might even be a little faster.

[Filip Pizlo]

Is this the 'this.initialize' GetById?

If so, I think there are two bugs here.

1) We should constant fold this no matter what.

This one could be constant folded by AI if we taught AI how to constant-fold prototype loads.  This wouldn't be terribly hard:

- Structure needs to be able to tell you if there is a chance of any additional properties on top of the ones that the PropertyTable reports.
- If the base object has a known structure that excludes the property in question and the structure indicates that the property table exhaustively lists all properties, then we can proceed through the prototype chain.

See the code in GetByIdStatus that gets called from AI and ConstantFoldingPhase; it should have a FIXME to implement this.

2) The polyvariant GetById profiler should have chosen to trust the DFG's context-sensitive profile over the FTL's context-insensitive exit site.

The FTL was previously using the DFG's context-sensitive profiling of the GetById to observe that the GetById only ever needs one case.

This breaks if the FTL exits.

I think we just need to ensure that a context-insensitive exit site from the FTL does not affect decisions we make based on context-sensitive profiling.

[Saam Barati]

(In reply to comment #22)
> Is this the 'this.initialize' GetById?
> 
> If so, I think there are two bugs here.
> 
> 1) We should constant fold this no matter what.
> 
> This one could be constant folded by AI if we taught AI how to constant-fold
> prototype loads.  This wouldn't be terribly hard:
> 
> - Structure needs to be able to tell you if there is a chance of any
> additional properties on top of the ones that the PropertyTable reports.
> - If the base object has a known structure that excludes the property in
> question and the structure indicates that the property table exhaustively
> lists all properties, then we can proceed through the prototype chain.
> 
> See the code in GetByIdStatus that gets called from AI and
> ConstantFoldingPhase; it should have a FIXME to implement this.
> 
> 2) The polyvariant GetById profiler should have chosen to trust the DFG's
> context-sensitive profile over the FTL's context-insensitive exit site.
> 
> The FTL was previously using the DFG's context-sensitive profiling of the
> GetById to observe that the GetById only ever needs one case.
> 
> This breaks if the FTL exits.
> 
> I think we just need to ensure that a context-insensitive exit site from the
> FTL does not affect decisions we make based on context-sensitive profiling.

I've filed bugs for these:

(1): https://bugs.webkit.org/show_bug.cgi?id=164904
(2): https://bugs.webkit.org/show_bug.cgi?id=165396

[Saam Barati]

(In reply to comment #23)
> (In reply to comment #22)
> > Is this the 'this.initialize' GetById?
> > 
> > If so, I think there are two bugs here.
> > 
> > 1) We should constant fold this no matter what.
> > 
> > This one could be constant folded by AI if we taught AI how to constant-fold
> > prototype loads.  This wouldn't be terribly hard:
> > 
> > - Structure needs to be able to tell you if there is a chance of any
> > additional properties on top of the ones that the PropertyTable reports.
> > - If the base object has a known structure that excludes the property in
> > question and the structure indicates that the property table exhaustively
> > lists all properties, then we can proceed through the prototype chain.
> > 
> > See the code in GetByIdStatus that gets called from AI and
> > ConstantFoldingPhase; it should have a FIXME to implement this.
> > 
> > 2) The polyvariant GetById profiler should have chosen to trust the DFG's
> > context-sensitive profile over the FTL's context-insensitive exit site.
> > 
> > The FTL was previously using the DFG's context-sensitive profiling of the
> > GetById to observe that the GetById only ever needs one case.
> > 
> > This breaks if the FTL exits.
> > 
> > I think we just need to ensure that a context-insensitive exit site from the
> > FTL does not affect decisions we make based on context-sensitive profiling.
> 
> I've filed bugs for these:
> 
> (1): https://bugs.webkit.org/show_bug.cgi?id=164904
> (2): https://bugs.webkit.org/show_bug.cgi?id=165396
Oops, (1) should be: https://bugs.webkit.org/show_bug.cgi?id=165395

[Yusuke Suzuki]

I've a bit investigated it.
I crafted GetById AI rule to load value from prototype or upper, introduced CreateThis -> NewObject conversion rule in AI, and updated this patch.
Yeah, now, GetById is gone. But I still saw performance regression.
I think that early MultiGetByOffset causes FTL OSR exit, and it involves materializations, and running this FTL code with OSR exit causes severe performance regression.

[Filip Pizlo]

We should fix this.

[Filip Pizlo]

Thinking about this more, I think we need this approach:

1) Implement the GetById proto folding.  We should do that anyway, independent of this patch.

2) Figure out if we can make the exit profiling smarter.  The way that the GetById gets turned into a monomorphic access is via the polyvariant profiler, if I remember right.  That profiler is apparently getting overridden by the MultiGetByOffset's exit site. But, if we're in polyvariant mode then we're asking about the previous outcomes of that DFG or FTL code block being inlined at our inline stack. Maybe all we need to do is modify FrequentExitSite's ExitingJITType to differentiate between uninlined contexts and inlined contexts when using exit sites as an override for the outcome of a polyvariant profiling decision.

3) Figure out how to not use MultiGetByOffset here.  It seems that the IC probably grows between when we tier up from baseline to DFG and when we tier up from DFG to FTL.  Maybe we can use that as a hint that MultiGetByOffset is now a good idea: if more crap was added to it in the time between finishing the DFG compile and when started the FTL compile then we probably don't want to inline it.  Note that the DFG's version of the IC might have fewer cases than the baseline version.  So the heuristic shouldn't be number of cases, it should be: if the DFG's IC for this get_by_id saw any case that can't be handled by the cases of the baseline's IC, then don't do MultiGetByOffset.  If that doesn't work at preventing emission of the MultiGetByOffset, then we can explore trying to understand the timing of repatchings of the MultiGetByOffset - it sorta knows whether it saw all of the structures it would see in a short period of time or if it saw them in a slow trickle.  Or maybe we can figure out how to make the IC track this.  With that information, we could probably pick with high confidence if MultiGetByOffset would be profitable.

4) We have an awesome optimization for richards that tries to prevent tiny functions from being FTL compiled because we are going to inline them anyway.  I wonder why that doesn't work here!

[Filip Pizlo]

Created attachment 341446 [details]
attempting a fix

I found out what's going on and it's more subtle than I expected.

First of all, there's no easy way of getting the FTL to emit anything other than a MultiGetByOffset inside initialize without also breaking MultiGetByOffset for other benchmarks in JetStream.  There's a MultiGetByOffset in richards that looks indistinguishable in all ways, and that one wants to be a MultiGetByOffset.

The reason why OSR exiting from FTL creates a bad situation is disturbing.  A DFG or FTL compile with a GetById means that the baseline get_by_id stops seeing new structures.  Baseline ICs are the ones used for GetByIdStatus etc, so this means that optimized compiles hide future polymorphism from the ICs used as profiling input to other compilations of that same code block, for example in case of inlining.  If the FTL emits a GetById or if the FTL compile doesn't happen, raytrace runs quickly because the IC for this GetById never gets crazy in baseline.  But if FTL emits a MultiGetByOffset and then exits, then baseline sees the full polymorphism and ends up claiming that it takes slow path.

The biggest bug here is that DFG and FTL ICs hide information from profiling in this way. The polyvariant profiler does save us sometimes, but not when the code block gets inlined in a place where it's never been inlined before.

This may be fixable with improved polyvariant profiling. The biggest hole in the polyvariant profile is that if the DFG ends up inlining the access then we don't get a polyvariant profile at all. This patch tries to fix that issue, but I'm not sure if it's the right approach.  It has the benefit that FTL compiles of DFG compiles that got good profiling will also get the same good profiling.

One observation that I made is that this problem mostly goes away if we allow PolymorphicAccess to cache up to 13 variants.  If we do this, then the DFG bytecode parser's filtering of GetByIdVariants filters this down to one case.  It's importnat that this filtering happens in the bytecode parser since that's what unlocks the subsequent inlining.

I think that means that the right approach not only adds GetByIdStatus prototype folding but invokes it in the DFG bytecode parser.

In a weird way, getting the FTL to emit a MultiGetByOffset is what fixes the baseline profile.  But we do need to fix that problem as well.  If we're sucking profiling data from a baseline code block then we should also look at whether it has a replacement, and if that replacement has an IC as well.

[Filip Pizlo]

(In reply to Filip Pizlo from comment #27)
> Thinking about this more, I think we need this approach:
> 
> 1) Implement the GetById proto folding.  We should do that anyway,
> independent of this patch.

The key here is to implement proto folding that also runs in the bytecode parser!

raytrace depends on us chain-folding a bunch of things to fold this GetById down to a constant so that the call instruction that uses it can get inlined.  So, whatever folding we do to benefit this GetById needs to run both in the AI and at bytecode parse time.

> 
> 2) Figure out if we can make the exit profiling smarter.  The way that the
> GetById gets turned into a monomorphic access is via the polyvariant
> profiler, if I remember right.  That profiler is apparently getting
> overridden by the MultiGetByOffset's exit site. But, if we're in polyvariant
> mode then we're asking about the previous outcomes of that DFG or FTL code
> block being inlined at our inline stack. Maybe all we need to do is modify
> FrequentExitSite's ExitingJITType to differentiate between uninlined
> contexts and inlined contexts when using exit sites as an override for the
> outcome of a polyvariant profiling decision.
> 
> 3) Figure out how to not use MultiGetByOffset here.  It seems that the IC
> probably grows between when we tier up from baseline to DFG and when we tier
> up from DFG to FTL.  Maybe we can use that as a hint that MultiGetByOffset
> is now a good idea: if more crap was added to it in the time between
> finishing the DFG compile and when started the FTL compile then we probably
> don't want to inline it.  Note that the DFG's version of the IC might have
> fewer cases than the baseline version.  So the heuristic shouldn't be number
> of cases, it should be: if the DFG's IC for this get_by_id saw any case that
> can't be handled by the cases of the baseline's IC, then don't do
> MultiGetByOffset.  If that doesn't work at preventing emission of the
> MultiGetByOffset, then we can explore trying to understand the timing of
> repatchings of the MultiGetByOffset - it sorta knows whether it saw all of
> the structures it would see in a short period of time or if it saw them in a
> slow trickle.  Or maybe we can figure out how to make the IC track this. 
> With that information, we could probably pick with high confidence if
> MultiGetByOffset would be profitable.

This doesn't work here.  At the time the FTL runs, the baseline and DFG IC both report four cases.

> 
> 4) We have an awesome optimization for richards that tries to prevent tiny
> functions from being FTL compiled because we are going to inline them
> anyway.  I wonder why that doesn't work here!

[Filip Pizlo]

Created attachment 341462 [details]
more

I've confirmed that if the DFG never figures out how to inline that dreaded IC, but the FTL always figures it out, then we will be within 4% of trunk perf.  This means that all I need to do is figure out how to make this work using polyvariant profiling.

This means strengthening polyvariant profiling as follows:

- Need to record the GetByIdStatus that bytecode parser or constant folder used to fold a GetById.  This allows us to see polyvariant profiles of things that got folded.  This recored GetByIdStatus needs to be filtered by the backend, in case we proved some of the cases away.  This lets future compiles start out with the proof in hand.

- Polyvariant profiling needs to search all available stacks, not just the ones attached to the machine code block.

- Need to ignore uninlined FTL OSR exits when making decisions using polyvariant profiles.

I think that the way that I'm improving the polyvariant profiler will implicitly fix the problem where DFG or FTL code blocks' ICs can hide future polymorphism from the IC profiling.  Now, if we are compiling foo and we have are looking at something in bar with the inline stack foo->bar, then we will look for profiling associated with optimized foo->bar, optimized bar, and baseline bar.

[Filip Pizlo]

Created attachment 341542 [details]
more

Still more work to do

[Filip Pizlo]

Created attachment 341545 [details]
CallLinkStatus is starting to look sensible

[Filip Pizlo]

Created attachment 341604 [details]
DFG/FTL filtering of statuses seems to be written

[Filip Pizlo]

Created attachment 341611 [details]
GetByIdStatus is starting to look good

[Filip Pizlo]

Created attachment 341614 [details]
maybe it's done

[EWS Watchlist]

Attachment 341614 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecode/ExitFlag.h:54:  Missing spaces around |  [whitespace/operators] [3]
ERROR: Source/JavaScriptCore/bytecode/BytecodeDumper.h:42:  The parameter name "statusMap" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/BytecodeDumper.h:43:  The parameter name "statusMap" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/RecordedStatuses.h:49:  The parameter name "slotVisitor" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGNodeType.h:479:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 5 in 59 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 341693 [details]
slowly getting it to compile

[Filip Pizlo]

Created attachment 341703 [details]
more

Still not compiling :-)

[Filip Pizlo]

Created attachment 341728 [details]
wow it compiles!

[Filip Pizlo]

Created attachment 341779 [details]
recursive polyvariant profiling appears to work

[Filip Pizlo]

Created attachment 341807 [details]
more

It fails a lot of tests and still cannot run raytrace, but it does polyvariant type inference better than what we had before.

[Filip Pizlo]

Created attachment 341811 [details]
so close

Raytrace appears to benefit nicely from this polyvariant profiler.  It seems to infer all of the calls.  To make raytrace neutral, I just need to make object allocation sinking ignore the new Filter nodes.  I'll do that later.

[Filip Pizlo]

Created attachment 341817 [details]
it makes raytrace neutral!

[Filip Pizlo]

Created attachment 341825 [details]
passing so many tests

[Filip Pizlo]

Created attachment 341848 [details]
it works!

[Filip Pizlo]

Created attachment 341849 [details]
rebased patch

It turns out that I had to implement all of the same things for InById.

[EWS Watchlist]

Attachment 341849 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecode/PutByIdStatus.h:77:  The parameter name "contextStack" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/ExitFlag.h:54:  Missing spaces around |  [whitespace/operators] [3]
ERROR: Source/JavaScriptCore/bytecode/BytecodeDumper.h:42:  The parameter name "statusMap" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/BytecodeDumper.h:43:  The parameter name "statusMap" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGNodeType.h:479:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/bytecode/RecordedStatuses.h:51:  The parameter name "slotVisitor" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/InByIdStatus.h:64:  The parameter name "contextStack" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 7 in 80 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 341850 [details]
fix build

[EWS Watchlist]

Attachment 341850 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecode/PutByIdStatus.h:77:  The parameter name "contextStack" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/ExitFlag.h:54:  Missing spaces around |  [whitespace/operators] [3]
ERROR: Source/JavaScriptCore/bytecode/BytecodeDumper.h:42:  The parameter name "statusMap" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/BytecodeDumper.h:43:  The parameter name "statusMap" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGNodeType.h:479:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/bytecode/RecordedStatuses.h:51:  The parameter name "slotVisitor" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/InByIdStatus.h:64:  The parameter name "contextStack" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 7 in 80 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Created attachment 341851 [details]
fix build

[EWS Watchlist]

Attachment 341851 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecode/PutByIdStatus.h:77:  The parameter name "contextStack" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/ExitFlag.h:54:  Missing spaces around |  [whitespace/operators] [3]
ERROR: Source/JavaScriptCore/bytecode/BytecodeDumper.h:42:  The parameter name "statusMap" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/BytecodeDumper.h:43:  The parameter name "statusMap" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGNodeType.h:479:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/bytecode/RecordedStatuses.h:51:  The parameter name "slotVisitor" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/InByIdStatus.h:64:  The parameter name "contextStack" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 7 in 80 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Yusuke Suzuki]

Comment on attachment 341849 [details]
rebased patch

View in context: https://bugs.webkit.org/attachment.cgi?id=341849&action=review

r=me

> Source/JavaScriptCore/ChangeLog:21
> +          that the helper was compiled by the DFG, the baseline get_by_id would not see those cases.

Yeah, this should be fixed!!!

> Source/JavaScriptCore/bytecode/CodeBlock.cpp:1446
> +        if (JITCode::isOptimizingJIT(jitType())) {
> +            DFG::CommonData* dfgCommon = m_jitCode->dfgCommon();
> +            for (auto& pair : dfgCommon->recordedStatuses.calls)
> +                result.add(pair.first, ICStatus()).iterator->value.callStatus = pair.second.get();
> +            for (auto& pair : dfgCommon->recordedStatuses.gets)
> +                result.add(pair.first, ICStatus()).iterator->value.getStatus = pair.second.get();
> +            for (auto& pair : dfgCommon->recordedStatuses.puts)
> +                result.add(pair.first, ICStatus()).iterator->value.putStatus = pair.second.get();
> +            for (auto& pair : dfgCommon->recordedStatuses.ins)
> +                result.add(pair.first, ICStatus()).iterator->value.inStatus = pair.second.get();
> +        }

Yay

> Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:6678
> +    if (Options::usePolyvariantDevirtualization()) {
> +        CodeBlock* optimizedBlock = m_profiledBlock->replacement();
> +        m_optimizedContext.optimizedCodeBlock = optimizedBlock;
> +        if (optimizedBlock) {
> +            ConcurrentJSLocker locker(optimizedBlock->m_lock);
> +            optimizedBlock->getICStatusMap(locker, m_optimizedContext.map);
>          }
>      }
> +    byteCodeParser->m_icContextStack.append(&m_optimizedContext);

Nice.

[EWS Watchlist]

Comment on attachment 341851 [details]
fix build

Attachment 341851 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/7952595

New failing tests:
http/tests/security/canvas-remote-read-remote-video-blocked-no-crossorigin.html

[EWS Watchlist]

Created attachment 341861 [details]
Archive of layout-test-results from ews205 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews205  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Filip Pizlo]

Created attachment 341924 [details]
working to fix ARES-6 regression

[Filip Pizlo]

Created attachment 342117 [details]
fixed a bunch of perf bugs

- CallLinkInfo forgets to record when it goes virtual.  It's crazy that this ever worked.

- PutByIdVariant in Transition mode doesn't know how to merge with itself.  This creates weird pathologies now that we rely on variant merging more.

[Filip Pizlo]

Created attachment 342182 [details]
neutral on JetStream, ARES-6, and Speedometer2

Still gotta test Speedometer2.  The last version was a definitely Speedometer2 regression in the neighborhood of 1.5%, but that was before I fixed issues in PutByIdVariant and CallLinkInfo.

[Filip Pizlo]

Comment on attachment 342182 [details]
neutral on JetStream, ARES-6, and Speedometer2

It's neutral on Speedometer2 now!

[Filip Pizlo]

Created attachment 342188 [details]
the patch

[EWS Watchlist]

Attachment 342188 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecode/ExitFlag.h:54:  Missing spaces around |  [whitespace/operators] [3]
ERROR: Source/JavaScriptCore/bytecode/PutByIdStatus.h:98:  The parameter name "contextStack" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGNodeType.h:481:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/bytecode/BytecodeDumper.h:42:  The parameter name "statusMap" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/BytecodeDumper.h:43:  The parameter name "statusMap" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/RecordedStatuses.h:51:  The parameter name "slotVisitor" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/InByIdStatus.h:84:  The parameter name "contextStack" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 7 in 91 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[EWS Watchlist]

Comment on attachment 342188 [details]
the patch

Attachment 342188 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/8061581

New failing tests:
accessibility/smart-invert-reference.html

[EWS Watchlist]

Created attachment 342200 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Filip Pizlo]

(In reply to Build Bot from comment #61)
> Comment on attachment 342188 [details]
> the patch
> 
> Attachment 342188 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://webkit-queues.webkit.org/results/8061581
> 
> New failing tests:
> accessibility/smart-invert-reference.html

Seems super unlikely that this is my patch.

[Yusuke Suzuki]

Comment on attachment 342188 [details]
the patch

r=me with windows fix.

[Filip Pizlo]

Created attachment 343463 [details]
rebased patch

[EWS Watchlist]

Attachment 343463 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/bytecode/ExitFlag.h:54:  Missing spaces around |  [whitespace/operators] [3]
ERROR: Source/JavaScriptCore/bytecode/PutByIdStatus.h:98:  The parameter name "contextStack" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/dfg/DFGNodeType.h:481:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/bytecode/BytecodeDumper.h:42:  The parameter name "statusMap" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/BytecodeDumper.h:43:  The parameter name "statusMap" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/RecordedStatuses.h:51:  The parameter name "slotVisitor" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/bytecode/InByIdStatus.h:84:  The parameter name "contextStack" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 7 in 90 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Filip Pizlo]

Landed in https://trac.webkit.org/changeset/234086/webkit

[Radar WebKit Bug Importer]

<rdar://problem/42472172>

[Yusuke Suzuki]

It seems that https://arewefastyet.com/#machine=29&view=single&suite=six-speed&subtest=destructuring-es6 is regressed. Due to breaking object allocation sinking? (for iterator object)?

[Filip Pizlo]

(In reply to Yusuke Suzuki from comment #69)
> It seems that
> https://arewefastyet.com/#machine=29&view=single&suite=six-
> speed&subtest=destructuring-es6 is regressed. Due to breaking object
> allocation sinking? (for iterator object)?

It's possible!  Ideally, adding coverage to the FTL should be neutral, but it usually isn't since it knocks us off our local maximum of tuning.  This patch is 20x larger than just the patch adding CreateThis to FTL to make it neutral on our own tuning benchmarks.  It achieved neutrality by essentially retuning the whole engine.

One option is to roll out the patch and have the patch grow some more to add more retuning.

But I think that it's better if we do more tuning in follow-on patches.

[Yusuke Suzuki]

(In reply to Filip Pizlo from comment #70)
> (In reply to Yusuke Suzuki from comment #69)
> > It seems that
> > https://arewefastyet.com/#machine=29&view=single&suite=six-
> > speed&subtest=destructuring-es6 is regressed. Due to breaking object
> > allocation sinking? (for iterator object)?
> 
> It's possible!  Ideally, adding coverage to the FTL should be neutral, but
> it usually isn't since it knocks us off our local maximum of tuning.  This
> patch is 20x larger than just the patch adding CreateThis to FTL to make it
> neutral on our own tuning benchmarks.  It achieved neutrality by essentially
> retuning the whole engine.
> 
> One option is to roll out the patch and have the patch grow some more to add
> more retuning.
> 
> But I think that it's better if we do more tuning in follow-on patches.

I've a bit investigated. And I've found that in DFGByteCodeParser, looking up "return" property at bc#212 is changed from,

CheckStructure() + JSConstant(undefined) => GetById(id{return}).

I think this causes materialization in FTL. Not sure why this happens yet.

[Yusuke Suzuki]

(In reply to Yusuke Suzuki from comment #71)
> (In reply to Filip Pizlo from comment #70)
> > (In reply to Yusuke Suzuki from comment #69)
> > > It seems that
> > > https://arewefastyet.com/#machine=29&view=single&suite=six-
> > > speed&subtest=destructuring-es6 is regressed. Due to breaking object
> > > allocation sinking? (for iterator object)?
> > 
> > It's possible!  Ideally, adding coverage to the FTL should be neutral, but
> > it usually isn't since it knocks us off our local maximum of tuning.  This
> > patch is 20x larger than just the patch adding CreateThis to FTL to make it
> > neutral on our own tuning benchmarks.  It achieved neutrality by essentially
> > retuning the whole engine.
> > 
> > One option is to roll out the patch and have the patch grow some more to add
> > more retuning.
> > 
> > But I think that it's better if we do more tuning in follow-on patches.
> 
> I've a bit investigated. And I've found that in DFGByteCodeParser, looking
> up "return" property at bc#212 is changed from,
> 
> CheckStructure() + JSConstant(undefined) => GetById(id{return}).
> 
> I think this causes materialization in FTL. Not sure why this happens yet.

I think it reveals a bit interesting existing bug.
When we have Absent state OPC, and multiple Absent state OPCs are merged in GetByVariant merging phase, it seems that it becomes invalid since `!mergedConditionSet.hasOneSlotBaseCondition()` becomes true.
I think this condition seems incorrect... I'll upload a patch in a separate bug.

[Yusuke Suzuki]

(In reply to Yusuke Suzuki from comment #72)
> (In reply to Yusuke Suzuki from comment #71)
> > (In reply to Filip Pizlo from comment #70)
> > > (In reply to Yusuke Suzuki from comment #69)
> > > > It seems that
> > > > https://arewefastyet.com/#machine=29&view=single&suite=six-
> > > > speed&subtest=destructuring-es6 is regressed. Due to breaking object
> > > > allocation sinking? (for iterator object)?
> > > 
> > > It's possible!  Ideally, adding coverage to the FTL should be neutral, but
> > > it usually isn't since it knocks us off our local maximum of tuning.  This
> > > patch is 20x larger than just the patch adding CreateThis to FTL to make it
> > > neutral on our own tuning benchmarks.  It achieved neutrality by essentially
> > > retuning the whole engine.
> > > 
> > > One option is to roll out the patch and have the patch grow some more to add
> > > more retuning.
> > > 
> > > But I think that it's better if we do more tuning in follow-on patches.
> > 
> > I've a bit investigated. And I've found that in DFGByteCodeParser, looking
> > up "return" property at bc#212 is changed from,
> > 
> > CheckStructure() + JSConstant(undefined) => GetById(id{return}).
> > 
> > I think this causes materialization in FTL. Not sure why this happens yet.
> 
> I think it reveals a bit interesting existing bug.
> When we have Absent state OPC, and multiple Absent state OPCs are merged in
> GetByVariant merging phase, it seems that it becomes invalid since
> `!mergedConditionSet.hasOneSlotBaseCondition()` becomes true.
> I think this condition seems incorrect... I'll upload a patch in a separate
> bug.

https://bugs.webkit.org/show_bug.cgi?id=187891 WIP, but the pre-reviews are welcome!

[Dawei Fenton (:realdawei)]

(In reply to Filip Pizlo from comment #67)
> Landed in https://trac.webkit.org/changeset/234086/webkit

Windows is failing to build after this revision

Sample Log:
https://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/10497/steps/compile-webkit/logs/stdio

C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/GetByIdStatus.cpp(323): error C3861: 'computeForStubInfoWithoutExitSiteFeedback': identifier not found (compiling source file C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-sources\UnifiedSource29.cpp) [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
  UnifiedSource33.cpp
C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/CallLinkStatus.cpp(344): error C2027: use of undefined type 'JSC::CallLinkInfo' (compiling source file C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-sources\UnifiedSource26.cpp) [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
  c:\cygwin\home\buildbot\slave\win-debug\build\source\javascriptcore\bytecode\CallLinkStatus.h(42): note: see declaration of 'JSC::CallLinkInfo' (compiling source file C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-sources\UnifiedSource26.cpp)
C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/CallLinkStatus.cpp(344): error C2227: left of '->codeOrigin' must point to class/struct/union/generic type (compiling source file C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-sources\UnifiedSource26.cpp) [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/CallLinkStatus.cpp(350): error C2661: 'JSC::CallLinkStatus::computeFor': no overloaded function takes 5 arguments (compiling source file C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-sources\UnifiedSource26.cpp) [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
  UnifiedSource34.cpp
C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/InByIdStatus.cpp(95): error C3861: 'computeForStubInfoWithoutExitSiteFeedback': identifier not found (compiling source file C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-sources\UnifiedSource30.cpp) [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
  UnifiedSource35.cpp
C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/CodeBlock.cpp(1420): error C2027: use of undefined type 'JSC::DFG::CommonData' (compiling source file C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-sources\UnifiedSource27.cpp) [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
  C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\jit\JITCode.h(39): note: see declaration of 'JSC::DFG::CommonData' (compiling source file C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-sources\UnifiedSource27.cpp)
C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/CodeBlock.cpp(1420): error C2227: left of '->recordedStatuses' must point to class/struct/union/generic type (compiling source file C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-sources\UnifiedSource27.cpp) [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/CodeBlock.cpp(1420): error C2228: left of '.finalize' must have class/struct/union (compiling source file C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-sources\UnifiedSource27.cpp) [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
  UnifiedSource36.cpp
  UnifiedSource37.cpp
  UnifiedSource38.cpp
  UnifiedSource39.cpp
C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/PutByIdStatus.cpp(250): error C3861: 'hasExitSite': identifier not found (compiling source file C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-sources\UnifiedSource33.cpp) [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/PutByIdStatus.cpp(272): error C3861: 'computeForStubInfo': identifier not found (compiling source file C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-sources\UnifiedSource33.cpp) [C:\cygwin\home\buildbot\slave\win-debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
  UnifiedSource40.cpp

[Filip Pizlo]

(In reply to David Fenton (:realdawei) from comment #74)
> (In reply to Filip Pizlo from comment #67)
> > Landed in https://trac.webkit.org/changeset/234086/webkit
> 
> Windows is failing to build after this revision
> 
> Sample Log:
> https://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/
> 10497/steps/compile-webkit/logs/stdio
> 
> C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> GetByIdStatus.cpp(323): error C3861:
> 'computeForStubInfoWithoutExitSiteFeedback': identifier not found (compiling
> source file
> C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> sources\UnifiedSource29.cpp)
> [C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
>   UnifiedSource33.cpp
> C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> CallLinkStatus.cpp(344): error C2027: use of undefined type
> 'JSC::CallLinkInfo' (compiling source file
> C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> sources\UnifiedSource26.cpp)
> [C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
>  
> c:\cygwin\home\buildbot\slave\win-
> debug\build\source\javascriptcore\bytecode\CallLinkStatus.h(42): note: see
> declaration of 'JSC::CallLinkInfo' (compiling source file
> C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> sources\UnifiedSource26.cpp)
> C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> CallLinkStatus.cpp(344): error C2227: left of '->codeOrigin' must point to
> class/struct/union/generic type (compiling source file
> C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> sources\UnifiedSource26.cpp)
> [C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> CallLinkStatus.cpp(350): error C2661: 'JSC::CallLinkStatus::computeFor': no
> overloaded function takes 5 arguments (compiling source file
> C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> sources\UnifiedSource26.cpp)
> [C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
>   UnifiedSource34.cpp
> C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> InByIdStatus.cpp(95): error C3861:
> 'computeForStubInfoWithoutExitSiteFeedback': identifier not found (compiling
> source file
> C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> sources\UnifiedSource30.cpp)
> [C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
>   UnifiedSource35.cpp
> C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> CodeBlock.cpp(1420): error C2027: use of undefined type
> 'JSC::DFG::CommonData' (compiling source file
> C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> sources\UnifiedSource27.cpp)
> [C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
>  
> C:\cygwin\home\buildbot\slave\win-
> debug\build\Source\JavaScriptCore\jit\JITCode.h(39): note: see declaration
> of 'JSC::DFG::CommonData' (compiling source file
> C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> sources\UnifiedSource27.cpp)
> C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> CodeBlock.cpp(1420): error C2227: left of '->recordedStatuses' must point to
> class/struct/union/generic type (compiling source file
> C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> sources\UnifiedSource27.cpp)
> [C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> CodeBlock.cpp(1420): error C2228: left of '.finalize' must have
> class/struct/union (compiling source file
> C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> sources\UnifiedSource27.cpp)
> [C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
>   UnifiedSource36.cpp
>   UnifiedSource37.cpp
>   UnifiedSource38.cpp
>   UnifiedSource39.cpp
> C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> PutByIdStatus.cpp(250): error C3861: 'hasExitSite': identifier not found
> (compiling source file
> C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> sources\UnifiedSource33.cpp)
> [C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> PutByIdStatus.cpp(272): error C3861: 'computeForStubInfo': identifier not
> found (compiling source file
> C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> sources\UnifiedSource33.cpp)
> [C:\cygwin\home\buildbot\slave\win-
> debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
>   UnifiedSource40.cpp

Looking.

[Dawei Fenton (:realdawei)]

(In reply to Filip Pizlo from comment #75)
> (In reply to David Fenton (:realdawei) from comment #74)
> > (In reply to Filip Pizlo from comment #67)
> > > Landed in https://trac.webkit.org/changeset/234086/webkit
> > 
> > Windows is failing to build after this revision
> > 
> > Sample Log:
> > https://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/
> > 10497/steps/compile-webkit/logs/stdio
> > 
> > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > GetByIdStatus.cpp(323): error C3861:
> > 'computeForStubInfoWithoutExitSiteFeedback': identifier not found (compiling
> > source file
> > C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > sources\UnifiedSource29.cpp)
> > [C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> >   UnifiedSource33.cpp
> > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > CallLinkStatus.cpp(344): error C2027: use of undefined type
> > 'JSC::CallLinkInfo' (compiling source file
> > C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > sources\UnifiedSource26.cpp)
> > [C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> >  
> > c:\cygwin\home\buildbot\slave\win-
> > debug\build\source\javascriptcore\bytecode\CallLinkStatus.h(42): note: see
> > declaration of 'JSC::CallLinkInfo' (compiling source file
> > C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > sources\UnifiedSource26.cpp)
> > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > CallLinkStatus.cpp(344): error C2227: left of '->codeOrigin' must point to
> > class/struct/union/generic type (compiling source file
> > C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > sources\UnifiedSource26.cpp)
> > [C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > CallLinkStatus.cpp(350): error C2661: 'JSC::CallLinkStatus::computeFor': no
> > overloaded function takes 5 arguments (compiling source file
> > C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > sources\UnifiedSource26.cpp)
> > [C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> >   UnifiedSource34.cpp
> > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > InByIdStatus.cpp(95): error C3861:
> > 'computeForStubInfoWithoutExitSiteFeedback': identifier not found (compiling
> > source file
> > C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > sources\UnifiedSource30.cpp)
> > [C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> >   UnifiedSource35.cpp
> > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > CodeBlock.cpp(1420): error C2027: use of undefined type
> > 'JSC::DFG::CommonData' (compiling source file
> > C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > sources\UnifiedSource27.cpp)
> > [C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> >  
> > C:\cygwin\home\buildbot\slave\win-
> > debug\build\Source\JavaScriptCore\jit\JITCode.h(39): note: see declaration
> > of 'JSC::DFG::CommonData' (compiling source file
> > C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > sources\UnifiedSource27.cpp)
> > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > CodeBlock.cpp(1420): error C2227: left of '->recordedStatuses' must point to
> > class/struct/union/generic type (compiling source file
> > C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > sources\UnifiedSource27.cpp)
> > [C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > CodeBlock.cpp(1420): error C2228: left of '.finalize' must have
> > class/struct/union (compiling source file
> > C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > sources\UnifiedSource27.cpp)
> > [C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> >   UnifiedSource36.cpp
> >   UnifiedSource37.cpp
> >   UnifiedSource38.cpp
> >   UnifiedSource39.cpp
> > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > PutByIdStatus.cpp(250): error C3861: 'hasExitSite': identifier not found
> > (compiling source file
> > C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > sources\UnifiedSource33.cpp)
> > [C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > PutByIdStatus.cpp(272): error C3861: 'computeForStubInfo': identifier not
> > found (compiling source file
> > C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > sources\UnifiedSource33.cpp)
> > [C:\cygwin\home\buildbot\slave\win-
> > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> >   UnifiedSource40.cpp
> 
> Looking.

thanks, it also broke LLINT CLoop
https://build.webkit.org/builders/Apple%20High%20Sierra%20LLINT%20CLoop%20%28BuildAndTest%29/builds/7507/steps/compile-webkit/logs/stdio

./bytecode/CallLinkStatus.cpp:81:18: error: unused parameter 'exitSiteData' [-Werror,-Wunused-parameter]
./bytecode/CallLinkStatus.cpp:344:84: error: member access into incomplete type 'JSC::CallLinkInfo'
./bytecode/CallLinkStatus.cpp:348:26: error: no matching function for call to 'computeFor'

[Filip Pizlo]

(In reply to David Fenton (:realdawei) from comment #76)
> (In reply to Filip Pizlo from comment #75)
> > (In reply to David Fenton (:realdawei) from comment #74)
> > > (In reply to Filip Pizlo from comment #67)
> > > > Landed in https://trac.webkit.org/changeset/234086/webkit
> > > 
> > > Windows is failing to build after this revision
> > > 
> > > Sample Log:
> > > https://build.webkit.org/builders/Apple%20Win%20Debug%20%28Build%29/builds/
> > > 10497/steps/compile-webkit/logs/stdio
> > > 
> > > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > > GetByIdStatus.cpp(323): error C3861:
> > > 'computeForStubInfoWithoutExitSiteFeedback': identifier not found (compiling
> > > source file
> > > C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > > sources\UnifiedSource29.cpp)
> > > [C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> > >   UnifiedSource33.cpp
> > > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > > CallLinkStatus.cpp(344): error C2027: use of undefined type
> > > 'JSC::CallLinkInfo' (compiling source file
> > > C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > > sources\UnifiedSource26.cpp)
> > > [C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> > >  
> > > c:\cygwin\home\buildbot\slave\win-
> > > debug\build\source\javascriptcore\bytecode\CallLinkStatus.h(42): note: see
> > > declaration of 'JSC::CallLinkInfo' (compiling source file
> > > C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > > sources\UnifiedSource26.cpp)
> > > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > > CallLinkStatus.cpp(344): error C2227: left of '->codeOrigin' must point to
> > > class/struct/union/generic type (compiling source file
> > > C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > > sources\UnifiedSource26.cpp)
> > > [C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> > > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > > CallLinkStatus.cpp(350): error C2661: 'JSC::CallLinkStatus::computeFor': no
> > > overloaded function takes 5 arguments (compiling source file
> > > C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > > sources\UnifiedSource26.cpp)
> > > [C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> > >   UnifiedSource34.cpp
> > > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > > InByIdStatus.cpp(95): error C3861:
> > > 'computeForStubInfoWithoutExitSiteFeedback': identifier not found (compiling
> > > source file
> > > C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > > sources\UnifiedSource30.cpp)
> > > [C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> > >   UnifiedSource35.cpp
> > > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > > CodeBlock.cpp(1420): error C2027: use of undefined type
> > > 'JSC::DFG::CommonData' (compiling source file
> > > C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > > sources\UnifiedSource27.cpp)
> > > [C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> > >  
> > > C:\cygwin\home\buildbot\slave\win-
> > > debug\build\Source\JavaScriptCore\jit\JITCode.h(39): note: see declaration
> > > of 'JSC::DFG::CommonData' (compiling source file
> > > C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > > sources\UnifiedSource27.cpp)
> > > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > > CodeBlock.cpp(1420): error C2227: left of '->recordedStatuses' must point to
> > > class/struct/union/generic type (compiling source file
> > > C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > > sources\UnifiedSource27.cpp)
> > > [C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> > > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > > CodeBlock.cpp(1420): error C2228: left of '.finalize' must have
> > > class/struct/union (compiling source file
> > > C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > > sources\UnifiedSource27.cpp)
> > > [C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> > >   UnifiedSource36.cpp
> > >   UnifiedSource37.cpp
> > >   UnifiedSource38.cpp
> > >   UnifiedSource39.cpp
> > > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > > PutByIdStatus.cpp(250): error C3861: 'hasExitSite': identifier not found
> > > (compiling source file
> > > C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > > sources\UnifiedSource33.cpp)
> > > [C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> > > C:\cygwin\home\buildbot\slave\win-debug\build\Source\JavaScriptCore\bytecode/
> > > PutByIdStatus.cpp(272): error C3861: 'computeForStubInfo': identifier not
> > > found (compiling source file
> > > C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\DerivedSources\JavaScriptCore\unified-
> > > sources\UnifiedSource33.cpp)
> > > [C:\cygwin\home\buildbot\slave\win-
> > > debug\build\WebKitBuild\Debug\Source\JavaScriptCore\JavaScriptCore.vcxproj]
> > >   UnifiedSource40.cpp
> > 
> > Looking.
> 
> thanks, it also broke LLINT CLoop
> https://build.webkit.org/builders/
> Apple%20High%20Sierra%20LLINT%20CLoop%20%28BuildAndTest%29/builds/7507/steps/
> compile-webkit/logs/stdio
> 
> ./bytecode/CallLinkStatus.cpp:81:18: error: unused parameter 'exitSiteData'
> [-Werror,-Wunused-parameter]
> ./bytecode/CallLinkStatus.cpp:344:84: error: member access into incomplete
> type 'JSC::CallLinkInfo'
> ./bytecode/CallLinkStatus.cpp:348:26: error: no matching function for call
> to 'computeFor'

No-JIT build fix landed in r234097.

[Dawei Fenton (:realdawei)]

*** Bug 187908 has been marked as a duplicate of this bug. ***