Better constant fold identity checks. (100 bytes saved on swarm, yeah! :))

Better constant fold identity checks. (100 bytes saved on swarm, yeah! :))
Committed: https://code.google.com/p/dart/source/detail?r=6999

[Lasse Reichstein Nielsen, 2012-04-26 07:24:02]

LGTM

https://chromiumcodereview.appspot.com/10174040/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/nodes.dart (right):

https://chromiumcodereview.appspot.com/10174040/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/nodes.dart:877: bool isConstantBoolean() =>
false;
How did we live without that?!?

https://chromiumcodereview.appspot.com/10174040/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/optimize.dart (right):

https://chromiumcodereview.appspot.com/10174040/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/optimize.dart:296: HInstruction
handleIdentityCheck(HInvokeBinary node) {
With the new == semantics (which we haven't implemented yet, but still...), we
can also optimize tests if we know that either side is null.
In that case, we only need to compare the other to null.

[floitsch, 2012-04-26 08:31:15]

LGTM.
Please add tests for the types I mentioned.
conflicting phi types, 0 === 0.0 and so on.

https://chromiumcodereview.appspot.com/10174040/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/optimize.dart (right):

https://chromiumcodereview.appspot.com/10174040/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/optimize.dart:299: if
(left.propagatedType.union(right.propagatedType) === HType.CONFLICTING) {
This should be intersection, except for numbers.
That is 0 === 0.0 is (semantically) false, but we compile it to 0 === 0.
Therefore, if we want to keep the runtime semantics, we need to fold it using
the 'union' operations.
For all the others intersection is fine. A string and a read-able array can
never be ===.

https://chromiumcodereview.appspot.com/10174040/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/optimize.dart:300: return
graph.addConstantBool(false);
I don't think this is correct: A phi with inputs integer and string will say
that its input is conflicting.
var x = 3;
while (true) { if (x == "done") return; x = done; }

[ngeoffray, 2012-04-26 09:25:19]

http://codereview.chromium.org/10174040/diff/1/lib/compiler/implementation/ss...
File lib/compiler/implementation/ssa/optimize.dart (right):

http://codereview.chromium.org/10174040/diff/1/lib/compiler/implementation/ss...
lib/compiler/implementation/ssa/optimize.dart:296: HInstruction
handleIdentityCheck(HInvokeBinary node) {
On 2012/04/26 07:24:02, Lasse Reichstein Nielsen wrote:
> With the new == semantics (which we haven't implemented yet, but still...), we
> can also optimize tests if we know that either side is null.
> In that case, we only need to compare the other to null.

True. Let's wait until we start implementing the new changes in the spec.

http://codereview.chromium.org/10174040/diff/1/lib/compiler/implementation/ss...
lib/compiler/implementation/ssa/optimize.dart:299: if
(left.propagatedType.union(right.propagatedType) === HType.CONFLICTING) {
On 2012/04/26 08:31:16, floitsch wrote:
> This should be intersection, except for numbers.
> That is 0 === 0.0 is (semantically) false, but we compile it to 0 === 0.
> Therefore, if we want to keep the runtime semantics, we need to fold it using
> the 'union' operations.
> For all the others intersection is fine. A string and a read-able array can
> never be ===.

I used union because I was mostly thinking of the number types. But yes, for the
others, it must be intersection.

So I added a check if both inputs are numbers. In that case, I don't fold to
false.

http://codereview.chromium.org/10174040/diff/1/lib/compiler/implementation/ss...
lib/compiler/implementation/ssa/optimize.dart:300: return
graph.addConstantBool(false);
On 2012/04/26 08:31:16, floitsch wrote:
> I don't think this is correct: A phi with inputs integer and string will say
> that its input is conflicting.
> var x = 3;
> while (true) { if (x == "done") return; x = done; }

I'm not sure I understand how this relates to the code.

[ngeoffray, 2012-04-26 10:09:53]

http://codereview.chromium.org/10174040/diff/1/lib/compiler/implementation/ss...
File lib/compiler/implementation/ssa/optimize.dart (right):

http://codereview.chromium.org/10174040/diff/1/lib/compiler/implementation/ss...
lib/compiler/implementation/ssa/optimize.dart:300: return
graph.addConstantBool(false);
On 2012/04/26 09:25:19, ngeoffray wrote:
> On 2012/04/26 08:31:16, floitsch wrote:
> > I don't think this is correct: A phi with inputs integer and string will say
> > that its input is conflicting.
> > var x = 3;
> > while (true) { if (x == "done") return; x = done; }
> 
> I'm not sure I understand how this relates to the code.

Actually, it looks like a phi never gets a conflicting type: see
HPhi::computeTypeFromInputTypes in nodes.dart. I added a test still and some
asserts, to make sure we don't regress on that case.