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

the ICode optimizer doesn't DCE-away trivial branches #7560

Closed
scabug opened this issue Jun 6, 2013 · 3 comments
Closed

the ICode optimizer doesn't DCE-away trivial branches #7560

scabug opened this issue Jun 6, 2013 · 3 comments

Comments

@scabug
Copy link

scabug commented Jun 6, 2013

The discussion at scala/scala#2620
uncovered that the ICode-based optimizer's DCE doesn't eliminate a redundant conditional jump,
where both branches lead to one and the same instruction.

The example showing the above is test/files/jvm/bytecode-test-example/Foo_1.scala reproduced below:

class Foo_1 {
  def foo(x: AnyRef): Int = {
    val bool = x == null
    if (x != null)
      1
    else
      0
  }
}

With GenASM and -optimise we get the two conditional jumps that bytecode-test-example.check expects:

  public foo(Ljava/lang/Object;)I
   L0
    ALOAD 1
    IFNONNULL L1
   L1
    ALOAD 1
    IFNONNULL L2
   L3
    ICONST_0
    GOTO L4
   L2
    ICONST_1
   L4
    IRETURN
   L5
    LOCALVARIABLE this LFoo_1; L0 L5 0
    LOCALVARIABLE x Ljava/lang/Object; L0 L5 1
    MAXSTACK = 1
    MAXLOCALS = 2

including a useless

    ALOAD 1
    IFNONNULL L1
   L1

(useless because, whether the conditional jump is taken or not, L1 is the target anyway)

@scabug
Copy link
Author

scabug commented Jun 6, 2013

@scabug
Copy link
Author

scabug commented Jun 6, 2013

@magarciaEPFL said:
@JamesIry
@gkossakowski

If you need help fixing this bug, take a look at how the new optimizer ( http://magarciaepfl.github.io/scala/ ) does it, in particular:

https://github.com/magarciaEPFL/scala/blob/44abfdb886363a19126a68f5f0b24037a73569db/src/asm/scala/tools/asm/optimiz/JumpReducer.java

Quoting from its documentation:

/**
 *  Simplifies branches that need not be taken to get to their destination:
 *    (1) conditional jump followed by unconditional jump, both to the same destination.
 *    (2) (conditional or unconditional) jump to a destination that is the next program point anyway.
 *  Details in `branchOverGoto()` and in `jumpToNext()`
 *
 *  @author  Miguel Garcia, http://lamp.epfl.ch/~magarcia/ScalaCompilerCornerReloaded/
 */

@scabug
Copy link
Author

scabug commented May 16, 2014

@adriaanm said:
This will be fixed by switching to the new back-end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant