Bug 16098 – align(N) not respected for stack variables if N > platform stack alignment

Status
RESOLVED
Resolution
FIXED
Severity
blocker
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2016-05-30T14:22:17Z
Last change time
2023-10-11T16:03:05Z
Keywords
backend, pull, SIMD
Assigned to
No Owner
Creator
Marco Leise
See also
https://issues.dlang.org/show_bug.cgi?id=24184

Comments

Comment #0 by Marco.Leise — 2016-05-30T14:22:17Z
This is especially true for float8 (AVX, 32-byte alignment) in 64-bit compiles where the stack is aligned to 16-bytes. Note that AVX is theoretically available in 32-bit, too, but currently dmd does neither expose intrinsics nor YMM0-YMM7 in inline asm, so we can't bump into this issue there yet. From what I've heard the major compilers gcc, icc, llvm support correct stack alignment of anything with an alignment >= 16. So this should also be investigated in the light of C interop.
Comment #1 by ketmar — 2016-05-30T22:05:23Z
afair, *any* `align` is simply ignored for stack vars in DMD.
Comment #2 by bugzilla — 2016-11-21T09:29:23Z
(In reply to Ketmar Dark from comment #1) > afair, *any* `align` is simply ignored for stack vars in DMD. No, this is not true, if the align <= the stack alignment.
Comment #3 by dlang-bugzilla — 2019-01-30T23:02:48Z
Well, this is annoying. Broken code: align(16) uint[128] state; asm { fxsave state; } "Fixed" code: uint[128 + 4] buf; auto state = cast(uint*)((cast(size_t)buf.ptr + 0xF) & ~size_t(0xF)); version (X86_64) asm { mov RAX, state; fxsave 0[RAX]; } else asm { mov EAX, state; fxsave 0[EAX]; }
Comment #4 by johan_forsberg_86 — 2021-04-30T18:57:24Z
Can we fix this?
Comment #5 by kinke — 2022-06-20T12:42:22Z
This issue blocks https://github.com/dlang/phobos/pull/8460. On Win32 with its 4-bytes stack alignment, the following assertion fails most of the time: void main() { byte a; align(8) byte b; assert((cast(size_t) &b) % 8 == 0); }
Comment #6 by bugzilla — 2022-12-23T09:14:51Z
One way to fix this is to: 1. gather all the locals with larger alignment and put them as fields in a struct variable 2. the first field in the struct is enough alignment padding to cover the worst case alignment bytes needed. For example, if the stack is aligned at 4, and the alignment of the fields is 16, then this field needs to be 12 bytes in size. 3. create a special pointer variable P that points to the aligned start of the struct fields, computed at function entry: P = (&struct + 15) & ~0xF; // for 16 byte alignment 4. rewrite all references to the variables in the struct as P->field 5. be careful that it works with closures allocated with the gc This should be doable all in the glue code, no need to modify the backend. Of course, this can be done explicitly for the moment, to unblock code.
Comment #7 by dkorpel — 2022-12-23T10:03:53Z
(In reply to Walter Bright from comment #6) > One way to fix this is to: I think it's easier to just align the stack, which is already done for 64-bit platforms and OSX, but not for Win32. I tried to enable it for win32 too: https://github.com/dlang/dmd/pull/14401 But the test suite failed so it requires some debugging.
Comment #8 by bugzilla — 2022-12-23T21:16:12Z
Increasing the alignment has some problems: 1. won't fit with the C ABI. I.e. if other functions are not following those alignments, and you call them from D and pass a callback, the callback won't be aligned. Or if the C functions call D functions, no alignment. 2. alignment requirements for some SIMD instructions keeps getting bigger and bigger. Increasing the alignment for functions is not very future proof. 3. larger alignments are rare. But increasing the alignment means it is done for every function making for a lot more stack space being used. But, it turns out that the code in DMD to do closures is almost exactly what we need for implementing an "aligned closure" allocated on the stack. The code generator doesn't need to be touched.
Comment #9 by dlang-bot — 2022-12-26T08:14:04Z
@WalterBright created dlang/dmd pull request #14740 "partial Issue 16098 - align(N) not respected for stack variables if N…" mentioning this issue: - partial Issue 16098 - align(N) not respected for stack variables if N > platform stack alignment https://github.com/dlang/dmd/pull/14740
Comment #10 by bugzilla — 2022-12-26T08:17:06Z
Comment #11 by dkorpel — 2022-12-26T12:40:01Z
(In reply to Walter Bright from comment #8) > Increasing the alignment has some problems: Those problems describe the situation where you increase the assumed alignment for all functions to a single larger constant, but that's not what I'm saying. The thing is, this issue has already been mostly fixed by Suleyman Sahmi, but the bot didn't close this issue when the PR was merged in 2019: https://github.com/dlang/dmd/pull/9143 The algorithm is this: Find the maximum alignment by iterating over the types of all local variables in the function. If it's larger than the minimum stack alignment guaranteed by the platform, generate code in the function prolog to align the stack to the required alignment. For example: ``` struct S { align(128) int i; } void f() { S x; } ``` Generates: ``` void onlineapp.f(): push RBP mov RBP,RSP and RSP,0FFFFFF80h // < alignment of stack pointer to multiple of 128 sub RSP,080h ``` The limitations are this: - It's only enabled for 64-bit builds and OSX (`&& (I64 || config.exe == EX_OSX)`) - It only looks at variable types, so an `align(128) int x;` is still assumed to have alignment 4
Comment #12 by bugzilla — 2022-12-30T04:27:46Z
I'm a little curious how SSoulaimane's solutions fixes things like two base pointers are required, one for the parameters and one for the rest. Then there's the problem with nested functions accessing the stack frame of the outer function - wouldn't two base pointers be needed? What about aligning the variables in a dynamic closure? I can't say I understand his solution. My PR fixes the dynamic closure problem. Once that is merged, I have a followup fix ready to go that solves the other problems. No changes to the code generator are required. It's also a lot simpler.
Comment #13 by dlang-bugzilla — 2022-12-30T13:53:24Z
@Walter Bright, I think you may have posted the above comment to the wrong issue.
Comment #14 by dlang-bot — 2022-12-31T19:22:23Z
dlang/dmd pull request #14740 "partial Issue 16098 - align(N) not respected for stack variables if N…" was merged into master: - f846e92fab15ad0bb9bab0e2d91835bfa64b52d8 by Walter Bright: partial Issue 16098 - align(N) not respected for stack variables if N > platform stack alignment https://github.com/dlang/dmd/pull/14740
Comment #15 by dlang-bot — 2022-12-31T19:44:14Z
@WalterBright created dlang/dmd pull request #14764 "Fix16098" fixing this issue: - fix 16098 - fix Issue 16098 - align(N) not respected for stack variables if N > platform stack alignment https://github.com/dlang/dmd/pull/14764
Comment #16 by bugzilla — 2023-01-02T21:08:58Z
Comment #17 by dlang-bot — 2023-02-25T03:51:29Z
dlang/dmd pull request #14764 "fix Issue 16098 - align(N) not respected for stack variables if N > platform stack alignment" was merged into master: - 6f32092a89599d7432f67e6e16d215dd81cbbec0 by Walter Bright: fix 16098 - 4380342554f6481e5999b6ff0f24fe986fd67009 by Walter Bright: fix Issue 16098 - align(N) not respected for stack variables if N > platform stack alignment https://github.com/dlang/dmd/pull/14764