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

implemented alignas pragma #12643

Merged
merged 5 commits into from
Nov 13, 2019
Merged

implemented alignas pragma #12643

merged 5 commits into from
Nov 13, 2019

Conversation

krux02
Copy link
Contributor

@krux02 krux02 commented Nov 11, 2019

Well this implements https://en.cppreference.com/w/c/language/_Alignas for Nim. A special word in TSpecialWord was already reserved for this, it was just waiting for the implementation.

The difference compared to the C version is, this implementation requires an integer argument, in C a type argument is allowed as well. The reason for this limitation is simple: A solution that would cover imported types (unknown alignment to Nim) would be too much effort, at least for now. For non-imported types it is possible it is possible to align to another type with alignof inside of alignas: {.alignas(alignof(float64)).}.

This feature is actually needed in Nim itself. During my work on newSeq on Aligned Type I stumbled upon this code:

Nim/lib/system/alloc.nim

Lines 63 to 70 in 7e68987

SmallChunk = object of BaseChunk
next, prev: PSmallChunk # chunks of the same size
freeList: ptr FreeCell
free: int # how many bytes remain
acc: int # accumulator for small object allocation
when defined(cpu32):
align: int
data: AlignType # start of usable memory

With this feature the code could be rewritten as following:

  SmallChunk = object of BaseChunk
      next, prev: PSmallChunk  # chunks of the same size
      freeList: ptr FreeCell
      free: int            # how many bytes remain
      acc: int             # accumulator for small object allocation
      data {.alignas(MemAlign).}: UncheckedArray[byte]  # start of usable memory

The when statement isn't needed anymore to make it work for 32 bit systems. The constant MemAlign could be changed into a constant that can be changed with the "compile time define pragmas", as they are listed in nim --fullhelp, and it will work with any alignment value.

closes #1930
closes #5315

@krux02
Copy link
Contributor Author

krux02 commented Nov 12, 2019

@mratsim since you are working with many types that do have alignment constraints. Would this feature be beneficial for you in any way? Note though allocations and seq members still ignore these alignment constraints, I hope that this might change in the future.

@mratsim
Copy link
Collaborator

mratsim commented Nov 12, 2019

Yes, I have 2 very-specific use-cases that need alignment:

  1. Thread-safe data structures. I need fine-grained control on where they reside on the cache line.
    This is enabled by this PR. ✔️
  2. SIMD. In that case it's the heap allocation that should be aligned. This is ideal but not required, unaligned loads/stores or a custom allocator work.

The first is actually the most critical for me because I had no workaround besides spraying padding.

That was actually one of my complaints in nim-lang/RFCs#160 (comment) (at the very bottom):

https://github.com/mratsim/weave/blob/796b9cc3/picasso/channels/channels_mpsc_bounded_lock.nim#L13-L56

type
  ChannelMpscBounded*[T] = object
    pad0: array[PI_CacheLineSize - 3*sizeof(int32), byte]
    backLock: Lock # Padding? - pthread_lock is 40 bytes on Linux, unknown on windows.
    capacity: int32
    buffer: ptr UncheckedArray[T]
    pad1: array[PI_CacheLineSize - sizeof(int32), byte]
    front: Atomic[int32]
    pad2: array[PI_CacheLineSize - sizeof(int32), byte]
    back: Atomic[int32]

https://github.com/mratsim/weave/blob/796b9cc3/picasso/memory/persistacks.nim#L13-L53

type
  Persistack*[N: static int8, T: object] = object
    pad: array[PI_CacheLineSize - N*sizeof(ptr T) - sizeof(pointer) - sizeof(int8), byte]
    stack: array[N, ptr T]
    rawMem: ptr array[N, T]
    len*: int8

And even the Nim system needs it:

type
Barrier {.compilerProc.} = object
entered: int
cv: Semaphore # Semaphore takes 3 words at least
when sizeof(int) < 8:
cacheAlign: array[CacheLineSize-4*sizeof(int), byte]
left: int
cacheAlign2: array[CacheLineSize-sizeof(int), byte]
interest: bool # whether the master is interested in the "all done" event

@mratsim
Copy link
Collaborator

mratsim commented Nov 12, 2019

Related issues: #1930 #5315.

does the {.align: 16.} pragma still exist? I don't see it in the manual. Anyway I think even though those issues want type-level alignment, if we have field-level alignment we can just align the first field. So this would close both issues.

@cooldome
Copy link
Member

I have reviewed looks great. One minor question is why new word alignas was taken instead of extending existing one: align?
Otherwise ready for submit.

@krux02
Copy link
Contributor Author

krux02 commented Nov 12, 2019

@mratsim thanks a lot for the feedback. I also marked the two issues to be closed when this PR gets merged. Regarding the {.align: 16.} pragma. No that one does not exist (anymore?). I found some dead and outdated code regarding the align pragma and I removed it in this PR. The reason I picked field alignment annotations instead of type annotations is, The field annotations are supported in both C and C++. C does not allow to annotate the type with alignment, see the notes section here. So I decided to implement the feature that can be implemented for both C and C++ backend easily. And yes, type alignment can be achieved by aligning the first field in the object.

@cooldome There are reasons for alignas. First of all as just mentioned align doesn't really exist anymore. It is currently just one of many reserved words for pragmas. alignas is also one of them, so I thought there isn't any preferece from the nim side anymore. alignas is the name that is used in both C and C++, since that naming isn't a particularly bad naming, I think being consistent with that name towards C and C++ could be a good thing.

wAlignas, wAlignof, wConstexpr, wDecltype, wNullptr, wNoexcept,

@cooldome cooldome merged commit 0496a66 into nim-lang:devel Nov 13, 2019
alehander92 pushed a commit to alehander92/Nim that referenced this pull request Dec 2, 2019
* implemented alignas pragma

* fix bootstrap

* generate c++ compatible syntax for alignas

* Make it work.

* Multiple alignof expressions. Implement top level alignof.
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.

Align pragma broken Request: Pragma to specify the alignment of object fields
3 participants