Skip to content

Remove wrong text, as pointed out by colleagues #3492

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

goldfirere
Copy link
Collaborator

I think @stedolan would make a good reviewer here.

@goldfirere goldfirere requested a review from stedolan January 20, 2025 21:21
```

Yet this would take 2 words in memory, as `t1` would be padded in order to be
32-bit aligned.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This text isn't very wrong and is helpful, so how about fixing it instead of deleting it?

All you need to do is change 2 words to 12 bytes: C is padding t1 to 32-bit alignment, not 64-bit alignment. The analysis is otherwise correct, though.

@stedolan
Copy link
Contributor

stedolan commented Jan 21, 2025

I don't really understand the layout algorithm presented here. What does it do on these examples?

type a = #{ p : int32#; q: int16#; r: int32# }
type b = #{ s: int16#; t: a }

From the description, I think we end up with 2 bytes of padding between q and r in an a, to ensure that r is well aligned. But when inlined into b, it looks like a will pack differently, and the padding will move. Is this the intent? If so, it means we can never have anything like interior pointers, because the layout of a depends on what it's embedded into. (Maybe that's a reasonable decision, but I'd like to know we're not making it accidentally)

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.

2 participants