Fix a bad merge that lead to unused optimization: implement the dataEquals in HInterceptor in order…

Fix a bad merge that lead to unused optimization: implement the dataEquals in HInterceptor in order to GVN it.
Committed: https://code.google.com/p/dart/source/detail?r=15453

[ahe, 2012-11-27 18:29:01]

https://codereview.chromium.org/11418183/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/nodes.dart (right):

https://codereview.chromium.org/11418183/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/nodes.dart:2391: // interceptor of
the right kind.
How do you know that?

[floitsch, 2012-11-27 18:51:38]

LGTM.

https://codereview.chromium.org/11418183/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/nodes.dart (right):

https://codereview.chromium.org/11418183/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/nodes.dart:2391: // interceptor of
the right kind.
On 2012/11/27 18:29:01, ahe wrote:
> How do you know that?

Because the input is the same.

[ahe, 2012-11-27 19:03:36]

https://codereview.chromium.org/11418183/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/nodes.dart (right):

https://codereview.chromium.org/11418183/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/nodes.dart:2391: // interceptor of
the right kind.
On 2012/11/27 18:51:38, floitsch wrote:
> On 2012/11/27 18:29:01, ahe wrote:
> > How do you know that?
> 
> Because the input is the same.

I'm still not sure I understand.  Don't worry, I'll ask Nicolas for more details
tomorrow :-)

[ngeoffray, 2012-11-28 12:05:18]

Unfortunately, the gvn and code motion phases use both dataEquals, which should
be specialized for code motion. The way I implement dataEquals only works for
GVN, so I added an explicit check and a TODO in the code motion code.

[floitsch, 2012-11-28 12:49:51]

LGTM.

[ngeoffray, 2012-11-28 15:41:48]

PTAL. After discussing with Kasper, we decided it was simpler to use a different
phase that will specialize the list of intercepted classes for all dominated
interceptor uses. And HInterceptor instances are dataEquals only if they have
the same set of intercepted classes.

The new phase should not just be for interceptors though, we are thinking of
also specializing dominated calls of regular non-intercepted calls. But that's
for another CL.

[floitsch, 2012-11-28 15:46:16]

Still LGTM.

[kasperl, 2012-11-29 08:22:27]

LGTM.

https://codereview.chromium.org/11418183/diff/7002/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/ssa/optimize.dart (right):

https://codereview.chromium.org/11418183/diff/7002/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/optimize.dart:1441: if (user is
HInterceptor && interceptor.dominates(user)) {
This is probably fine, but an alternative implementation would be to keep a
scoped mapping from interceptor receivers to intercepted classes -- and let the
dominator tree walk eliminate the need for calling interceptor.dominates(user).

[ngeoffray, 2012-11-29 08:31:56]

Thanks Kasper.

https://codereview.chromium.org/11418183/diff/7002/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/ssa/optimize.dart (right):

https://codereview.chromium.org/11418183/diff/7002/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/optimize.dart:1441: if (user is
HInterceptor && interceptor.dominates(user)) {
On 2012/11/29 08:22:27, kasperl wrote:
> This is probably fine, but an alternative implementation would be to keep a
> scoped mapping from interceptor receivers to intercepted classes -- and let
the
> dominator tree walk eliminate the need for calling
interceptor.dominates(user).

Good point. Filed http://code.google.com/p/dart/issues/detail?id=7022 to not
forget about it.