Add codegen test for equality.

Add codegen test for equality.

Adds a DDC test tracking the generated code and its correctness. Some
test harness changes to support running codegen tests.

BUG=
R=jmesserly@google.com

Committed: https://github.com/dart-lang/sdk/commit/cb8cc2526b832fb8a3bb3049b57cf83c4887376b

[Leaf, 2017-06-07 17:06:47]

leafp@google.com changed reviewers:
+ jmesserly@google.com, vsm@google.com

[Leaf, 2017-06-07 17:06:48]

These are the tests for the equality changes that I'll send in separately.  I'm
splitting this out as a separate CL to establish a baseline so that the changes
in equality implementation have context via a diff in the test expectations.

[Jennifer Messerly, 2017-06-07 17:45:44]

I'm going to assume the equality tests are right :)

so normally I would put something like this under the top level SDK tests
directory, such as tests/language_strong or perhaps tests/compiler/dev_compiler
(to go with tests/compiler/dart2js).

Is the idea that you want to diff the output? do you think that will be useful
in the long term for perf regressions, or is it mostly for the initial CL to see
which call sites are being optimized?

I have a slight worry that we're going to need a different approach for codegen
tests as we integrate more with SDK build/test infrastructure. A big test like
this has the possibility of churning a lot on the baseline.

That said I'm not sure of a better approach here to suggest :). Perf regressions
are hard to catch. Possibly we could get some actual Dart unit tests for the
compiler (e.g. nullability analysis would benefit from that) ... that would let
us narrow in on the relevant CodeGenerator transformation for equality
expressions. But that'll take some work to get unit test infra set up for that.

So I guess something to think about. It LGTM tho :)

[Jennifer Messerly, 2017-06-07 17:46:36]

On 2017/06/07 17:45:44, Jennifer Messerly wrote:
> I'm going to assume the equality tests are right :)
> 
> so normally I would put something like this under the top level SDK tests
> directory, such as tests/language_strong or perhaps
tests/compiler/dev_compiler
> (to go with tests/compiler/dart2js).
> 
> Is the idea that you want to diff the output? do you think that will be useful
> in the long term for perf regressions, or is it mostly for the initial CL to
see
> which call sites are being optimized?
> 
> I have a slight worry that we're going to need a different approach for
codegen
> tests as we integrate more with SDK build/test infrastructure. A big test like
> this has the possibility of churning a lot on the baseline.
> 
> That said I'm not sure of a better approach here to suggest :). Perf
regressions
> are hard to catch. Possibly we could get some actual Dart unit tests for the
> compiler (e.g. nullability analysis would benefit from that) ... that would
let
> us narrow in on the relevant CodeGenerator transformation for equality
> expressions. But that'll take some work to get unit test infra set up for
that.
> 
> So I guess something to think about. It LGTM tho :)

Oh, another idea: get DDC on perf bot of some sort.

[Leaf, 2017-06-07 18:47:10]

On 2017/06/07 17:45:44, Jennifer Messerly wrote:
> I'm going to assume the equality tests are right :)
> 
> so normally I would put something like this under the top level SDK tests
> directory, such as tests/language_strong or perhaps
tests/compiler/dev_compiler
> (to go with tests/compiler/dart2js).
> 
> Is the idea that you want to diff the output? do you think that will be useful
> in the long term for perf regressions, or is it mostly for the initial CL to
see
> which call sites are being optimized?
> 
> I have a slight worry that we're going to need a different approach for
codegen
> tests as we integrate more with SDK build/test infrastructure. A big test like
> this has the possibility of churning a lot on the baseline.
> 
> That said I'm not sure of a better approach here to suggest :). Perf
regressions
> are hard to catch. Possibly we could get some actual Dart unit tests for the
> compiler (e.g. nullability analysis would benefit from that) ... that would
let
> us narrow in on the relevant CodeGenerator transformation for equality
> expressions. But that'll take some work to get unit test infra set up for
that.
> 
> So I guess something to think about. It LGTM tho :)

Thanks.  This was the best I could think of.  Part of the point of the tests is
correctness for which running them is ok, but a big part of the point is
tracking the form of the generated code - being sure that we don't regress if
something changes in the element model, or nullability analysis, or whatever. 
Churn is always a concern with golden files.  We could try to replace this with
unit tests at some point.

[Leaf, 2017-06-07 18:48:29]

On 2017/06/07 17:46:36, Jennifer Messerly wrote:
> On 2017/06/07 17:45:44, Jennifer Messerly wrote:
> > I'm going to assume the equality tests are right :)
> > 
> > so normally I would put something like this under the top level SDK tests
> > directory, such as tests/language_strong or perhaps
> tests/compiler/dev_compiler
> > (to go with tests/compiler/dart2js).
> > 
> > Is the idea that you want to diff the output? do you think that will be
useful
> > in the long term for perf regressions, or is it mostly for the initial CL to
> see
> > which call sites are being optimized?
> > 
> > I have a slight worry that we're going to need a different approach for
> codegen
> > tests as we integrate more with SDK build/test infrastructure. A big test
like
> > this has the possibility of churning a lot on the baseline.
> > 
> > That said I'm not sure of a better approach here to suggest :). Perf
> regressions
> > are hard to catch. Possibly we could get some actual Dart unit tests for the
> > compiler (e.g. nullability analysis would benefit from that) ... that would
> let
> > us narrow in on the relevant CodeGenerator transformation for equality
> > expressions. But that'll take some work to get unit test infra set up for
> that.
> > 
> > So I guess something to think about. It LGTM tho :)
> 
> Oh, another idea: get DDC on perf bot of some sort.

I'd actually really like to do this at some point.  Not sure if it would
successfully track these micro-optimizations, but probably could set it up to
catch it.  It also doesn't capture readability, but that's a fuzzier thing
anyway.

[Leaf, 2017-06-07 19:42:33]

Description was changed from

==========
Add codegen test for equality.

Adds a DDC test tracking the generated code and its correctness. Some
test harness changes to support running codegen tests.

BUG=
==========

to

==========
Add codegen test for equality.

Adds a DDC test tracking the generated code and its correctness. Some
test harness changes to support running codegen tests.

BUG=
R=jmesserly@google.com

Committed:
https://github.com/dart-lang/sdk/commit/cb8cc2526b832fb8a3bb3049b57cf83c4887376b
==========

[Leaf, 2017-06-07 19:42:35]

Committed patchset #1 (id:1) manually as
cb8cc2526b832fb8a3bb3049b57cf83c4887376b (presubmit successful).