Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dart2js: optimize redundant and contradictory nested conditions in if-tests #12571

Closed
rakudrama opened this issue Aug 20, 2013 · 4 comments
Closed
Labels
P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug web-dart2js

Comments

@rakudrama
Copy link
Member

Inlining sometimes creates redundant or contradicting tests when all levels of the call tree have the same test.

if (a) { if (a) ... } --> if (a) { if (true) ... }

From swarm:

    t1 = this._node == null;
    if (!t1) {
      if (t1) {
        if (t1)
          this._node = this.render$0();
        this.afterRender$1(this._node);
      }
      t1 = $.get$body$x($.get$document$x(this._node));
      t2 = this._node == null;
      if (t2) {
        if (t2)
          this._node = this.render$0();
        this.afterRender$1(this._node);
      }
      t1 = t1.contains(this._node) === true;
    } else
      t1 = false;
    if (t1) {
      view._enterDocument$0();
      this.doLayout$0();
    }

Should reduce to:

    t1 = this._node;
    if ($.get$body$x($.get$document$x(t1)).contains(t1)) {
      view._enterDocument$0();
      this.doLayout$0();
    }

@kasperl
Copy link

kasperl commented Sep 3, 2013

After r27035 the code has improved slightly:

if (this._node != null) {
  t3 = $.get$body$x($.get$document$x(this._node));
  if (this._node == null) {
    this._node = this.render$0();
    this.afterRender$1(this._node);
  }
  t3 = t3.contains(this._node) === true;
} else
  t3 = false;
if (t3) {
  t1._enterDocument$0();
  this.doLayout$0();
}

@kasperl
Copy link

kasperl commented Jul 10, 2014

Removed this from the Later milestone.
Added Oldschool-Milestone-Later label.

@kasperl
Copy link

kasperl commented Aug 4, 2014

Removed Oldschool-Milestone-Later label.

@kevmoo kevmoo added P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug and removed triaged labels Feb 29, 2016
@rakudrama
Copy link
Member Author

Closing as fixed.

The current code still needs to re-load this._null since it appears to be potentially modified by get document / get ownerDocument.
These would have to be hand-annotated as not having a field effect to know that this._node does not change across the call.

            t3 = this._node;
            if (t3 != null)
              if (!!J.getInterceptor(J.get$ownerDocument$x(t3)).$isHtmlDocument) {
                if (this._node == null) {
                  t3 = this.render$0();
                  this._node = t3;
                  this.afterRender$1(t3);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug web-dart2js
Projects
None yet
Development

No branches or pull requests

3 participants