-
Notifications
You must be signed in to change notification settings - Fork 1k
compiler: simplify createObjectLayout #5145
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
base: dev
Are you sure you want to change the base?
Conversation
This simplifies the process of constructing and encoding layout bitmaps. Instead of creating big integers and merging them, we can create a pre-sized bitmap and set positions within it. This also changes the encoding logic to allow larger layouts to be encoded inline. We would previously not encode a layout inline unless the size was less than the width of the data field. This is overly conservative. A layout can be encoded inline as long as: 1. The size fits within the size field. 2. All set bits in the bitmap fit into the data field.
|
Are there any stdlib tests or hand-coded tests that exercise the new "larger layouts" logic? |
|
I added a test to ensure that it is encoded correctly. We do not currently have a meaningful way to test the scanning logic. |
|
Test corpus passes but I wouldn't mind having @aykevl take a second look. |
dgryski
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks correct but I don't have enough experience in this area to be confident.
| // Create the global initializer. | ||
| bitmapBytes := make([]byte, int(objectSizeWords+7)/8) | ||
| bitmap.FillBytes(bitmapBytes) | ||
| reverseBytes(bitmapBytes) // big-endian to little-endian |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This big/little endian conversion doesn't seem to have an equivalent in the new code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. The old getPointerBitmap code used big.Int which is stored in big-endian so the reverseBytes code was needed to fix it. buildPointerBitmap writes the bitmap normally so no endian logic is necessary,
This simplifies the process of constructing and encoding layout bitmaps. Instead of creating big integers and merging them, we can create a pre-sized bitmap and set positions within it.
This also changes the encoding logic to allow larger layouts to be encoded inline. We would previously not encode a layout inline unless the size was less than the width of the data field. This is overly conservative.
A layout can be encoded inline as long as: