Keep boxed class-method params boxed - #3043
Open
plajjan wants to merge 2 commits into
Open
Conversation
added 2 commits
July 29, 2026 00:27
Golden files (Boxing output and generated C) for a small test
program, boxparam.act.
Base is a generic class whose methods take an argument of type T.
Deriv inherits from Base[int], so its versions of those methods take
an int. Because the methods can also be called through Base, where
the argument type is not known, the argument is always passed boxed
(a B_int), never as a raw machine integer -- even when the Deriv
version is called directly.
The goldens cover such an argument being compared, reassigned, and
passed on to another method. Two of the recorded outputs are plain
bugs, committed on purpose so that the diff of the next commit shows
exactly what the fix changes:
bool ...cmp (... self, B_int x) {
bool N_tmp = (x > 3LL); // compares the pointer
B_int ...bump (... self, B_int x) {
x += 1LL; // moves the pointer
return toB_int(((B_int)x)->val); // reads through it
A method slot's C calling convention is decided by the oldest
declaration of the slot in the hierarchy (rtypeOf). When that
declaration is generic, the slot passes the argument as a boxed
object, because callers may invoke it through the base class's method
table, where every argument is a pointer-sized word:
class Base[T](object):
def f(self, x: T) -> bool:
return True
An override in a class that narrows T to a scalar type inherits that
convention: Deriv(Base[int]).f receives a boxed B_int, not a raw
int64_t, even though its body computes with raw machine integers.
(Only this shape is affected. A slot whose oldest declaration already
names a scalar type passes it raw everywhere.)
The Boxing pass, however, assumed that every scalar-typed local holds
a raw value. It wrapped reads of such parameters in Box like raw
locals, and the Box/UnBox cancellation in the primitive operator
reduction then dropped the representation conversion entirely, so
class Deriv(Base[int]):
def f(self, x: int) -> bool:
return x > 3
Deriv().f(1) # returned True: compared the B_int pointer to 3
Track boxed parameters in the Boxing env and leave their reads
unwrapped. Where the body needs the raw value, a real UnBox now
survives to CodeGen and renders as ->val: the parameter is unboxed at
each use inside the callee.
Assignments cannot follow the read convention: an augmented
assignment would update the caller's box in place, and a plain
rebinding would store a raw value that later reads would unbox again.
A boxed parameter assigned in the body is therefore renamed aside and
copied into an ordinary raw local at function entry, after which
every read and store follows the standard raw-local convention and
the caller's box is never written.
Contributor
Author
|
@nordlander do you want to take a look at this? it's a correctness fix but also a general speedup since we avoid boxing+unboxing for variables in quite a lot of common scenarios. I think the updated testing snapshots show pretty clearly what is going on |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
A method that overrides a method declared generic in its base class
receives its arguments boxed: calls can arrive through the base class's
method table, where the argument type is not known, so a Deriv(Base[int])
override of Base[T].f takes a B_int, never a raw machine integer.
The Boxing pass assumed that every scalar-typed local holds a raw value.
For these parameters that assumption is wrong. Code generation quietly
compensated at call boundaries, at the cost of box/unbox round-trips, but
the primitive operator paths cancelled the compensation away entirely, so
a body like "return x > 3" compared the pointer instead of the value:
Deriv().f(1) returned True on main.
Boxing now tracks these parameters and leaves their reads boxed; where a
raw value is needed, the unboxing survives to code generation and is
emitted as ->val. Assignments cannot follow the read convention: x += 1
would update the caller's box in place, and a plain rebinding would store
a raw value that later reads unbox again. So a boxed parameter that the
body assigns is renamed aside and copied into an ordinary raw local at
function entry, after which reads and stores follow the standard
raw-local rules and the caller's box is never written.
The first commit adds a small fixture and commits the golden files
produced by the unfixed compiler, so the second commit's diff shows
exactly what the fix changes in the generated code. The churn in the
deact and lines goldens is the removal of the now-unnecessary box/unbox
round-trips at call boundaries; behavior there was already correct.