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

Fix #14773: Reuse the param slots for the tailrec local mutable vars. #14865

Merged

Conversation

sjrd
Copy link
Member

@sjrd sjrd commented Apr 6, 2022

The tailrec phase generates code that looks like

  class Bar {
    def foo(x1: Int, x2: Int): Int = {
      var this$: Bar = this;
      var taillocal$x1: Int = x1;
      var taillocal$x2: Int = x2;
      ...
      // body where `this`, `x1` and `x2` are replaced by
      // `this$`, `taillocal$x1` and `taillocal$x2`
    }
  }

This generates bytecode where the this value and the parameters never actually change, and are never used. Instead, the synthetic mutable variables are used instead.

As described in the linked issue, this confuses debuggers, which only display the never-changing original this value and parameters.

In this commit, we intercept this shape of code in the back-end. We reliable identify tailrec-generated ValDefs from their semantic names, with an additional safety check that they are Synthetic | Mutable. When we find this shape, we do not allocate local slots for the vars, and instead reuse the slots for this and the parameters. We skip past the ValDefs so that code generation does not re-emit useless assignments.

sjrd added 2 commits April 6, 2022 17:03
…vars.

The tailrec phase generates code that looks like

  class Bar {
    def foo(x1: Int, x2: Int): Int = {
      var this$: Bar = this;
      var taillocal$x1: Int = x1;
      var taillocal$x2: Int = x2;
      ...
      // body where `this`, `x1` and `x2` are replaced by
      // `this$`, `taillocal$x1` and `taillocal$x2`
    }
  }

This generates bytecode where the `this` value and the parameters
never actually change, and are never used. Instead, the synthetic
mutable variables are used instead.

As described in the linked issue, this confuses debuggers, which
only display the never-changing original `this` value and
parameters.

In this commit, we intercept this shape of code in the back-end.
We reliable identify tailrec-generated `ValDef`s from their
semantic names, with an additional safety check that they are
`Synthetic | Mutable`. When we find this shape, we do not allocate
local slots for the `var`s, and instead reuse the slots for `this`
and the parameters. We skip past the `ValDef`s so that code
generation does not re-emit useless assignments.
@smarter
Copy link
Member

smarter commented Apr 6, 2022

@lrytz do you want to review this one?

@smarter smarter requested a review from lrytz April 6, 2022 15:43
@smarter smarter assigned lrytz and unassigned smarter Apr 6, 2022
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.

👍 LGTM!

@smarter smarter merged commit 8873eb0 into scala:main Apr 7, 2022
@smarter smarter deleted the better-bytecode-for-tailrec-methods branch April 7, 2022 13:39
@Kordyjan Kordyjan added this to the 3.2.0 milestone Aug 1, 2023
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.

Bytecode generation for tailrec methods uses temporary variables which confuses debuggers
4 participants