Fix alignment of fields for atomic accesses

[cme]

Fix for issue #749.

Atomic accesses on AArch64 require atomically accessed fields to be naturally aligned. The packed structure layouts used on macOS to ensure than x86 and ARM processes can share memory breaks this constraint in some instances.

This change

1. Introduces alignment checks that ensure atomically-accessed fields are aligned within their parent structure (and that these parent structures are aligned within any struct containing them). These checks are static and have no runtime overhead.
2. Changes order of some fields, and adds some padding in one instance, to meet the alignment constraints.

[amiartus]

could alignas be used as well to make this more robust and explicit?

[cme]

could alignas be used as well to make this more robust and explicit?

I think the same thing as __attribute__((aligned())) applies to alignas -- having the containing struct packed forces alignment of 1 on the struct as a whole and hence all the members too.

Unless there's a usage of alignas I'm not aware of?

Thanks!

[cme]

could alignas be used as well to make this more robust and explicit?

I think the same thing as __attribute__((aligned())) applies to alignas -- having the containing struct packed forces alignment of 1 on the struct as a whole and hence all the members too.

This seems to be true, but also, I now realise that these would actually work so long as instances included in containing structs are also declared as aligned. This would make the static asserts here redundant and document the intention explicitly.

This sounds better.

[cme]

Now using alignas to set the alignment explicitly, and retaining the static_asserts as an additional check on the alignment of the hierarchy.

[falkTX]

Let's merge this and give it a try for 1 release.
If it works fine on all systems, we might be able to re-enable struct packing for linux arm architectures too.

[0EVSG]

This reportedly breaks build with Clang on PowerPC (but not with GCC on PowerPC):

    In file included from ../common/JackDebugClient.cpp:21:
    ../common/JackEngineControl.h:67:5: error: requested alignment is less than minimum alignment of 8 for type 'Jack::JackTransportEngine'
        alignas(UInt32) JackTransportEngine fTransport;
        ^
    ../common/JackEngineControl.h:89:5: error: requested alignment is less than minimum alignment of 8 for type 'Jack::JackFrameTimer'
        alignas(UInt32) JackFrameTimer fFrameTimer;
        ^

While I do not understand what causes Clang to require 8 byte alignment for these packed classes, it's probably not against the standard. Is there a reason not to use e.g.

alignas(JackTransportEngine) JackTransportEngine fTransport;

here?

[cme]

Ah, there's a semantic difference between alignas and __attribute__((aligned)) that I hadn't noticed. The attribute sets the minimum alignment requirement but alignas makes it explicitly ill-formed to set a lower alignment than the natural alignment. Changing it as you suggest is probably a good idea, yes.

I wonder why clang for AArch64 didn't complain.

[0EVSG]

Funny thing, 10 minutes before your answer I get a bug report of this failing also on FreeBSD / Aarch64 - could you provide me the compiler version and build flags of a successful build?

Regarding correctness, I think that alignas(JackTransportEngine) would be the right approach for single platforms. But if you mix x86_64 and Arm64 you'd actually want the maximum alignment of both platforms. So that may be a good reason to use fixed alignment, we'd have to verify.

Microsoft seems to have a similar translation technology BTW, but I've never seen it in the wild.

[0EVSG]

Ok, did an experiment with alignas(JackTransportEngine) here, and it breaks CI for MacOS / GCC. GCC does not propagate the alignment requirements of JackTransportEngine correctly. The (presumably newer) GCC on Linux works fine, though.

So I leave that be at the moment.