Improvements #128
Merged
Improvements #128
+1,718
−130
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Most implementations of the `Operation.getInputResultHandles()` method return a `LinkedHashSet`, even if the interface demands just `Set`. A few existing implementations returned just `HashSet`, and this commit makes them consistent with the rest.
The code that emits LADD had a typo: the correct descriptor for the `long` type is `J` and not `Z`.
|
Note it is probably easiest to review individual commits separately. |
Ladicek
force-pushed
the
improvements
branch
2 times, most recently
from
ada1ba0 to
8af0225
Compare
Previously, all constants were loaded from the constant pool and pushed on the stack using the LDC instruction. This is less efficient than necessary, because for some very commonly used constants, the JVM ISA has dedicated instructions: ICONST_0, ICONST_1 etc. With this commit, all the common constats are loaded using these instructions.
Ladicek
force-pushed
the
improvements
branch
3 times, most recently
from
ec56176 to
b4e76f0
Compare
mkouba
reviewed
Oct 17, 2022
| @@ -1177,7 +1177,7 @@ void writeBytecode(MethodVisitor methodVisitor) { | |||
|
|
|||
| @Override | |||
| Set<ResultHandle> getInputResultHandles() { | |||
| return new HashSet<>(Arrays.asList(a1, a2)); | |||
| return new LinkedHashSet<>(Arrays.asList(a1, a2)); | |||
There was a problem hiding this comment.
Hmm, would it make sense to use HashSet everywhere instead? Or clarify the contract of BytecodeCreatorImpl.Operation.getInputResultHandles()?
There was a problem hiding this comment.
I'd like to do some experiments to improve the generated code, and as part of them, I'd probably make this a List, actually.
Previously, the only arithmetic operation available through Gizmo API was addition (IADD etc.). This commit adds multiplication (IMUL etc.), as well as internal helper methods that will help implement more arithmetic operations in the future.
As opposed to comparing `int` values, where the JVM ISA provides instructions that branch directly, comparing `long`, `float` and `double` values is more involved. The LCMP, FCMP* and DCMP* instructions push a comparison result on the stack as an `int` and an `int` branching instruction must be used afterwards.
The `ifReferencesEqual` method emits the IF_ACMPEQ instruction. This commit adds `ifReferencesNotEqual`, which emits IF_ACMPNE. This is useful to generate bytecode similar to what javac generates.
Notably, 4 methods are added to simplify common `returnValue` invocations. The `returnBoolean` and `returnInt` methods simplify returning a constant, `returnNull` simplifies returning the `null` reference, and `returnVoid` makes return from a `void` method or constructor more obvious.
Most notably, `JdkMap.instance(ResultHandle)` is deprecated and replaced by `JdkMap.on(ResultHandle)` to be consistent with other classes.
mkouba
approved these changes
Oct 17, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Several improvements:
getInputResultHandles()implementations consistentlongvalues (typo in the type descriptor)returnBoolean,returnInt,returnNullandreturnVoidto simplify common method exitsStringBuilderchainsequals,hashCodeandtoString, similarly to what IDEs doGizmoclass to USAGE.adoc