Skip to content
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

Proposal: Change ABI alignment of zero-sized types from 0 to 1 #7221

Open
SpexGuy opened this issue Nov 25, 2020 · 4 comments
Open

Proposal: Change ABI alignment of zero-sized types from 0 to 1 #7221

SpexGuy opened this issue Nov 25, 2020 · 4 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@SpexGuy
Copy link
Contributor

SpexGuy commented Nov 25, 2020

In #6706 it was decided that pointers to zero-sized types should not be zero sized. As a consequence of this, zero-sized types now need an ABI alignment. This was chosen to be 0, to preserve the invariant that @alignOf(T) <= @sizeOf(T). However, in #6954, @marler8997 made a strong argument for making it 1, to preserve the invariant @alignOf(T) >= 1. This issue exists to continue this discussion now that #6945 has been closed.

Arguments for align 0:

  • Preserves @alignOf(T) <= @sizeOf(T) (is this actually useful though?)
  • You could say that zero sized types are bitpacked, if you put multiple of them together they share the same address
  • Alignment being 0 is always a special case that needs to be handled in pointer type info, so having this special case in @alignOf isn't anything new.

Arguments for align 1:

  • Preserves @alignOf(T) >= 1, removing special cases when using this value.
  • It's kind of weird to make a bit pointer if a zero-sized field follows an align(0) u3 in a non-packed ordered struct. Then again, Zig doesn't have a non-packed ordered struct type that can contain align(0) fields, so this may be a moot point.
@ghost
Copy link

ghost commented Nov 27, 2020

One thing which could make this cleaner: changing align() to take a bit alignment rather than a byte alignment. So @alignOf(u8) would be 8, @alignOf(bool) would be the size of the smallest addressable unit (not necessarily 8 bits on all architectures), and @alignOf(ZST) would be 1. Thus, we eliminate special cases, and allow ZSTs to have arbitrary bit offsets. (Plus: we accommodate architectures with sub-byte or super-byte addressing, and align() now takes a clean u32 rather than a lopsided u29, which I suspect was also LLVM's motivation for choosing that number. See my comment on #5185.)

@kyle-github
Copy link

Hmm, I like that idea, @EleanorNB. This would be nice on platforms that had bytes that were not 8 bits, such as some DSPs. On those, in C, it is sometimes difficult to tell how big something is because sizeof returns a size in the native bytes and not in units of 8 bits and alignment is in those byte units.

@Vexu Vexu added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Nov 27, 2020
@Vexu Vexu added this to the 0.8.0 milestone Nov 27, 2020
@SpexGuy
Copy link
Contributor Author

SpexGuy commented Nov 27, 2020 via email

@ghost
Copy link

ghost commented Nov 27, 2020

We could solve that by defaulting the alignment of sub-addressable types to the size of the platform address unit (not necessarily 8 bits, as noted above). This introduces a platform dependence in the alignment of some values, but there is precedent for that, it is easily checked at comptime, and the alternative is to inflict this same variability on all known-size types (how many address units in a u32? How long is a piece of string?). Even if we say that all supported platforms address memory in bytes, that just means that there is no variability -- so, we lose nothing, and gain consistency in case of platforms with other address units.

Related: #5185 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

4 participants