[vm] Removing IntegerValue and Value wrapper
#17693
Open
+534
−801
Conversation
This file contains hidden or 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
16 tasks
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
16 tasks
wrwg
force-pushed
the
wrwg/int_wrapper
branch
2 times, most recently
from
82a3415 to
9647ebd
Compare
junxzm1990
changed the base branch from
jun/sint-compiler
to
graphite-base/17693
wrwg
requested review from
georgemitenkov,
junxzm1990,
rahxephon89 and
vgao1996
vgao1996
approved these changes
Sep 27, 2025
|
|
||
| /// Perform a binary operation to two values at the top of the stack. | ||
| fn binop<F, T>(&mut self, f: F) -> PartialVMResult<()> | ||
| #[inline(always)] |
There was a problem hiding this comment.
Later we'll need to run this through the profiler and see if the Rust compiler actually performs the inlining or not. FYI with the current impl, it's not inlining F.
wrwg
force-pushed
the
graphite-base/17693
branch
from
432eb78 to
86f6df1
Compare
wrwg
force-pushed
the
wrwg/int_wrapper
branch
from
9647ebd to
36cc47f
Compare
junxzm1990
changed the base branch from
jun/sint-compiler
to
graphite-base/17693
wrwg
force-pushed
the
wrwg/int_wrapper
branch
from
36cc47f to
b539694
Compare
wrwg
force-pushed
the
graphite-base/17693
branch
from
86f6df1 to
1dc20f4
Compare
georgemitenkov
approved these changes
Sep 28, 2025
| } | ||
|
|
||
| fn compare( | ||
| pub fn compare_with_depth( |
There was a problem hiding this comment.
This and equals_with_depth can be private or pub(crate)?
wrwg
force-pushed
the
wrwg/int_wrapper
branch
from
b539694 to
0ab338a
Compare
wrwg
force-pushed
the
jun/sint-compiler
branch
2 times, most recently
from
b6b60fd to
a71a230
Compare
wrwg
force-pushed
the
wrwg/int_wrapper
branch
from
0ab338a to
2980225
Compare
junxzm1990
force-pushed
the
wrwg/int_wrapper
branch
from
2980225 to
df83d80
Compare
junxzm1990
force-pushed
the
jun/sint-compiler
branch
from
a71a230 to
f65de35
Compare
16 tasks
junxzm1990
force-pushed
the
jun/sint-compiler
branch
from
f65de35 to
c186797
Compare
junxzm1990
force-pushed
the
wrwg/int_wrapper
branch
from
df83d80 to
5aa57e1
Compare
wrwg
force-pushed
the
jun/sint-compiler
branch
from
c186797 to
4ae9850
Compare
junxzm1990
force-pushed
the
wrwg/int_wrapper
branch
from
fbbd9fa to
bd22709
Compare
junxzm1990
force-pushed
the
jun/sint-compiler
branch
2 times, most recently
from
901ac24 to
f4a07b3
Compare
junxzm1990
force-pushed
the
wrwg/int_wrapper
branch
2 times, most recently
from
38fe6d3 to
37febd6
Compare
junxzm1990
force-pushed
the
jun/sint-compiler
branch
from
f4a07b3 to
a8aff80
Compare
22 tasks
junxzm1990
force-pushed
the
jun/sint-compiler
branch
from
a8aff80 to
9e39a96
Compare
junxzm1990
force-pushed
the
wrwg/int_wrapper
branch
from
37febd6 to
281400b
Compare
wrwg
force-pushed
the
jun/sint-compiler
branch
from
9e39a96 to
4cf379f
Compare
junxzm1990
force-pushed
the
jun/sint-compiler
branch
from
4cf379f to
9e39a96
Compare
junxzm1990
force-pushed
the
wrwg/int_wrapper
branch
from
93d20aa to
281400b
Compare
junxzm1990
force-pushed
the
jun/sint-compiler
branch
from
9e39a96 to
a561800
Compare
junxzm1990
force-pushed
the
wrwg/int_wrapper
branch
from
281400b to
2bb080b
Compare
wrwg
force-pushed
the
jun/sint-compiler
branch
from
a561800 to
a464842
Compare
The `values_impl::IntegerValue` type introduced a rather unnecessary wrapper for integer values, creating to conversions back and force to `Value`. Moreover, the newtype wrapper `Value` for `ValueImpl` was unnecessary as well and created overhead because it cannot be eliminated by the compiler when applied in iterators (as in `values.into_iter().map(Value).collect()`). Both have been removed.
wrwg
force-pushed
the
jun/sint-compiler
branch
from
a464842 to
753b223
Compare
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.
Description
The
values_impl::IntegerValuetype introduced a rather unnecessary wrapper for integer values, creating to conversions back and force toValue. Moreover, the newtype wrapperValueforValueImplwas unnecessary as well and createdoverhead because it cannot be eliminated by the compiler when applied in iterators (as in
values.into_iter().map(Value).collect()). Both have been removed.How Has This Been Tested?
Existing tests