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

Teach backend to emit iinc instructions #13214

Merged
merged 1 commit into from
Aug 2, 2021
Merged

Conversation

harpocrates
Copy link
Contributor

The backend is now able to turn x += 42 into an iinc 42
instruction. The optimization only applies to += and -=, provided
the the net increment fits inside a signed 16-bit value (the ASM library
handles choosing iinc or wide iinc as is appropriate).

Ported from scala/scala#9713

@harpocrates
Copy link
Contributor Author

I'm not sure what to do about tests. The initial PR had tests, but they used scala.tools.partest.BytecodeTest. I tried grepping inside Dotty test directory for JVM bytecode tests but didn't find any. In fact, I couldn't find anything like the Scala 2 test/files/jvm folder.

@harpocrates
Copy link
Contributor Author

Thanks @smarter! I completely forgot that there are tests in the compiler directory.

The backend is now able to turn `x += 42` into an `iinc 42`
instruction. The optimization only applies to `+=` and `-=`, provided
the the net increment fits inside a signed 16-bit value (the ASM library
handles choosing `iinc` or `wide iinc` as is appropriate).

Ported from scala/scala#9713
@odersky
Copy link
Contributor

odersky commented Aug 1, 2021

Is that something worth doing? I guess it won't make code any faster but it gives some small improvement in bytecode size. Is that worth the additional effort of maintaining the code? What does the Scala 2.13 backend do?

@bishabosha
Copy link
Member

bishabosha commented Aug 1, 2021

What does the Scala 2.13 backend do?

This is a port of scala/scala#9713 which was just merged

@lrytz
Copy link
Member

lrytz commented Aug 2, 2021

It's mostly a cosmetic change. It could potentially benefit performance by pushing some methods below the JVM's inlining size limits. I also see it as going with the flow of the platform, javac emits iinc too. And we have more complicated code in the backend to maintain 🙂

@smarter smarter merged commit 7389b63 into scala:master Aug 2, 2021
@Kordyjan Kordyjan added this to the 3.1.0 milestone Aug 2, 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.

6 participants