dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.04k stars 1.55k forks source link

Multiple-comparison optimization suggestion #16597

Open stevemessick opened 10 years ago

stevemessick commented 10 years ago

[user feedback]

Dart2Js generates the following code for me:

removeEntity$1: [function(entity) {       var t1 = J.getInterceptor(entity);       if (typeof entity === "object" && entity !== null && !!t1.$isAmbientLightEntity)         C.JSArray_methods.remove$1(this._ambientLights, entity);       if (typeof entity === "object" && entity !== null && !!t1.$isDirectionalLightEntity)         C.JSArray_methods.remove$1(this._directionalLights, entity);       if (typeof entity === "object" && entity !== null && !!t1.$isPointLightEntity)         C.JSArray_methods.remove$1(this._pointLights, entity);       if (typeof entity === "object" && entity !== null && !!t1.$isSpotLightEntity)         C.JSArray_methods.remove$1(this._spotLights, entity);       E.RendererComponent.prototype.removeEntity$1.call(this, entity);     }, "call$1", "get$removeEntity", 2, 0, null, 676],

based on:

  void removeEntity(Entity entity) {     if (entity is AmbientLightEntity) {       _ambientLights.remove(entity);     }

    if (entity is DirectionalLightEntity) {       _directionalLights.remove(entity);     }

    if (entity is PointLightEntity) {       _pointLights.remove(entity);     }

    if (entity is SpotLightEntity) {       _spotLights.remove(entity);     }

    super.removeEntity(entity);   }

Dart2Js should notice that it is checking the same thing multiple times (entity == null) and move it into an outer if statement. //////////////////////////////////////////////////////////////////////////////////// Editor: 1.2.0.dev_02_04 (2014-01-31) OS: Windows 8 - amd64 (6.2) JVM: 1.7.0_21

projects: 1

open dart files: 0

auto-run pub: true localhost resolves to: 127.0.0.1 mem max/total/free: 1778 / 784 / 113 MB thread count: 29 index: 559247 relationships in 108843 keys in 15293 sources

SDK installed: true Dartium installed: true

floitschG commented 10 years ago

Added Optimization label.

rakudrama commented 10 years ago

True. We don't have an optimization for

if (a && b) ... if (a && c) ...

---> if (a) {   if (b) ...   if (c) ... }

In fact we have no optimizations the change the shape of the SSA graph.

In this case we could make a different improvement. The canned check

  typeof entity === "object" && entity !== null && !!t1.$isXXX

could be replaced by

  t1.$isXXX

since it is reading from an interceptor.

There are some cases where we don't use the interceptor. To enable the control flow optimization, we would have to introduce the null/primitive dereference check as a separate instruction.

rakudrama commented 10 years ago

This reduces the code size by always using the interceptor https://codereview.chromium.org/188433002/

floitschG commented 10 years ago

Removed the owner.