Skip to content

Use explicit destinations in codegen to avoid uselessly jumping around. #14890

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

Merged
merged 2 commits into from
May 2, 2022

Conversation

sjrd
Copy link
Member

@sjrd sjrd commented Apr 8, 2022

CI for now. This needs some cleanup and better tests that the bytecode is indeed improved in a number of situations. (notably with pattern matching)

Previously, the codegen's main method genLoad always generated code that loaded the value on the stack before continuing. There were a number of situations where genLoad would be directly followed by unconditional jumps to instructions performing more jumps, returns and throws. This generated more spurious jumps than necessary, along with artifact dead code.

We solve these limitations by introducing LoadDestinations that specify the destination of a loaded value:

  • FallThrough: as previously, load the value on the stack and continue.
  • Jump(label): load the value on the stack and jump to the given label.
  • Return: return the value from the enclosing method.
  • Throw: throw the value.

We generalize genLoad as genLoadTo, taking a specific destination for the loaded value. genLoadTo can "push down" its destination into all control flow structures (except Trys, because of their cleanups). With that, when we get to the end of what amounts to "basic blocks", we know exactly the ultimate destination of the loaded value. We can therefore directly jump, return or throw to the final destination.

This produces less bytecode, notably because fewer labels are necessary. For example, the method:

  def abs(x: Int): Int = if x < 0 then -x else x

previously generated bytecode like

  ILOAD 1
  ICONST_0
  IF_ICMPGE Label(1)
  ILOAD 1
  INEG
  GOTO Label(2)
  Label(1):
  ILOAD 1
  Label(2):
  IRETURN

Now, instead of jumping to Label(2), we directly perform an IRETURN:

  ILOAD 1
  ICONST_0
  IF_ICMPGE Label(1)
  ILOAD 1
  INEG
  IRETURN
  Label(1):
  ILOAD 1
  IRETURN

While the changes are not very impressive on that simple example, they become more important in more complex cases, notably with pattern matching. Examples can be found in the changed bytecode tests.

An added benefit is that genLoadTo knows when loading a value results in an unconditional control flow change (jump, return or throw). It can then avoid inserting any useless adaptation. This removes all the dead bytecode that the codegen used to generate as artifacts of its own compilation scheme. (It will still generate dead bytecode if the original source code/inlined code contains dead code.)

@sjrd sjrd self-assigned this Apr 8, 2022
@sjrd sjrd force-pushed the codegen-destinations branch 3 times, most recently from 75c1f23 to d86e68b Compare April 13, 2022 07:30
sjrd added 2 commits April 13, 2022 10:49
Previously, the codegen's main method `genLoad` always generated
code that loaded the value on the stack before continuing. There
were a number of situations where `genLoad` would be directly
followed by unconditional jumps to instructions performing more
jumps, returns and throws. This generated more spurious jumps than
necessary, along with artifact dead code.

We solve these limitations by introducing `LoadDestination`s that
specify the destination of a loaded value:

* FallThrough: as previously, load the value on the stack and continue.
* Jump(label): load the value on the stack and jump to the given label.
* Return: return the value from the enclosing method.
* Throw: throw the value.

We generalize `genLoad` as `genLoadTo`, taking a specific
destination for the loaded value. `genLoadTo` can "push down" its
destination into all control flow structures (except `Try`s,
because of their cleanups). With that, when we get to the end of
what amounts to "basic blocks", we know exactly the ultimate
destination of the loaded value. We can therefore directly jump,
return or throw to the final destination.

This produces less bytecode, notably because fewer labels are
necessary. For example, the method:

  def abs(x: Int): Int = if x < 0 then -x else x

previously generated bytecode like

  ILOAD 1
  ICONST_0
  IF_ICMPGE Label(1)
  ILOAD 1
  INEG
  GOTO Label(2)
  Label(1):
  ILOAD 1
  Label(2):
  IRETURN

Now, instead of jumping to Label(2), we directly perform an
IRETURN:

  ILOAD 1
  ICONST_0
  IF_ICMPGE Label(1)
  ILOAD 1
  INEG
  IRETURN
  Label(1):
  ILOAD 1
  IRETURN

While the changes are not very impressive on that simple example,
they become more important in more complex cases, notably with
pattern matching. Examples can be found in the changed bytecode
tests.

An added benefit is that `genLoadTo` knows when loading a value
results in an unconditional control flow change (jump, return or
throw). It can then avoid inserting any useless adaptation. This
removes all the dead bytecode that the codegen used to generate
as artifacts of its own compilation scheme. (It will still
generate dead bytecode if the original source code/inlined code
contains dead code.)
@sjrd sjrd force-pushed the codegen-destinations branch from d86e68b to 4a2889f Compare April 13, 2022 09:45
@sjrd sjrd changed the title WiP Use explicit destinations in codegen to avoid uselessly jumping around. Use explicit destinations in codegen to avoid uselessly jumping around. Apr 13, 2022
@sjrd sjrd marked this pull request as ready for review April 13, 2022 09:47
@sjrd sjrd requested a review from lrytz April 13, 2022 09:47
@sjrd sjrd assigned lrytz and unassigned sjrd Apr 13, 2022
Comment on lines -595 to -600
case Labeled(bind, expr) if tpeTK(body) == UNIT =>
// this is the shape of tailrec methods
val loop = programPoint(bind.symbol)
markProgramPoint(loop)
genLoad(expr, UNIT)
bc goTo loop
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This optimization is now performed "by construction" of the generic LoadDestination infrastructure. :)

Comment on lines -805 to -806
case (_: Return) | Block(_, (_: Return)) => ()
case (_: Apply) | Block(_, (_: Apply)) if trimmedRhs.symbol eq defn.throwMethod => ()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two optimizations are also taken care of by the general LoadDestination infrastructure.

@sjrd
Copy link
Member Author

sjrd commented Apr 27, 2022

Ping @lrytz ?

Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, definitely worth backporting to Scala 2! Not that trivial, but LGTM, I couldn't spot any mistakes.

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

Successfully merging this pull request may close these issues.

3 participants