Skip to content

PrivateMemoryResolution: align the stack-call per-thread private memory stride - #435

Draft
pvelesko wants to merge 2 commits into
intel:masterfrom
pvelesko:fix/stackcall-private-mem-stride
Draft

PrivateMemoryResolution: align the stack-call per-thread private memory stride#435
pvelesko wants to merge 2 commits into
intel:masterfrom
pvelesko:fix/stackcall-private-mem-stride

Conversation

@pvelesko

Copy link
Copy Markdown

When a function group places private memory on the stack, each hardware thread's slab is located as

SP = privateBase + HWTID * (PrivateMemoryPerFG * simdSize)

and allocas are laid out relative to that base keeping the alignment they state in the IR. PrivateMemoryResolution.cpp:615 on master never rounds the per-work-item size, so the stride is an arbitrary byte count and every odd hardware thread gets a base that is only as aligned as the stride happens to be. With an alloca align 64 and a stride of 32 mod 64, thread 1's copy is 32-byte aligned while the IR still claims 64, and downstream codegen folds constant offsets into a bitwise OR on the low half of the pointer, which is only valid when those bits are zero. The kernel then reads the wrong fields on every odd thread.

That is why the reproducer in #392 passes at global size 1 (only hardware thread 0) and fails 9 runs in 10 at global size 4096. Real strides seen there: 8352 on dg2 and 8864 on rpl-s, both 32 mod 64, and pinning the size with ForcePerThreadPrivateMemorySize flips the result deterministically (1108, 1116, 1124 fail; 1112, 1120, 1128 pass). FP64 emulation is what creates the stack calls, so dg2 and Gen12LP fail while parts with native FP64 generate no stack calls and pass.

Round the per-work-item size up to the group's strictest alloca alignment, which is what ModuleAllocaAnalysis::getPerThreadOffset() already does on the non-stack path. The condition matches privateOnStack; groups that do not place private memory on the stack are untouched.

Measured on the test kernel (alloca align 64, size pinned to 2020, SIMD8), same input and same ocloc, only libigc differing:

master       mul (M1_NM, 1) V0042(0,0)<1> ... 0x3f20:ud    16160, 32 mod 64
this change  mul (M1_NM, 1) V0042(0,0)<1> ... 0x4000:ud    16384

Related to #423, which reaches an under-aligned private copy through LowerByValAttribute instead.

Note that IGC/ocloc_tests only builds with -DIGC_OPTION__ENABLE_OCLOC_LIT_TESTS=ON.

Fixes #392

When a function group contains a stack call the private memory of a HW
thread starts at

    SP = privateBase + HWTID * (PrivateMemoryPerFG * simdSize)

and the kernel's allocas are laid out relative to that base keeping the
alignment they ask for in the IR. That only holds when the per-thread
stride is a multiple of the strictest alloca alignment in the group.

The kernel added here has an `alloca align 64` and a stack call. With the
per-work-item size pinned to 2020 and the dispatch pinned to SIMD8 the
emitted stride is 2020 * 8 = 0x3f20, i.e. 32 mod 64, so every odd HW
thread gets a base that is only 32-byte aligned. The test checks for the
rounded stride 2048 * 8 = 0x4000 and fails without the following commit.

Signed-off-by: Paulius Velesko <pvelesko@pglc.io>
…stride

For a function group whose private memory lives on the stack,
PrivateMemoryPerFG is the per-work-item size and EmitPass turns it into

    SP = privateBase + HWTID * (PrivateMemoryPerFG * simdSize)

Allocas are then laid out relative to SP on the assumption that they keep
the alignment stated in the IR. prevFPOffset is already padded up to the
strictest alloca alignment for exactly that reason, but the padding is
useless when the base it is relative to is itself misaligned, and the
stride was never rounded.

It often is not aligned: fp64 emulation on a platform without native
double turns an ordinary callee into a stack call, and the private memory
size that the call depth analysis arrives at is an arbitrary byte count.
With a per-work-item size congruent to 4 mod 8 at SIMD8 the stride is 32
mod 64, so every odd hardware thread gets a base that is only 32-byte
aligned and an `alloca align 64` is under-aligned there.

That is not a benign loss of performance. Downstream codegen trusts the
stated alignment and folds constant offsets into a bitwise OR on the low
half of the pointer, which is only valid when the low bits really are
zero. On the misaligned threads those ORs alias earlier elements, so a
struct read back through the pointer returns the wrong fields. A kernel
therefore produces correct results at global size 1, where the only work
item lands on hardware thread 0, and wrong results as soon as enough work
items are dispatched to reach thread 1.

Round the per-work-item size up to the group's strictest alloca alignment
so that any multiple of it stays aligned, whatever SIMD width is picked.
ModuleAllocaAnalysis::getPerThreadOffset() already performs the equivalent
rounding for the non-stack private memory path, so this only brings the
stack path in line. Groups that do not place private memory on the stack
never derive a per-thread base from this value and are left untouched.

Fixes the test added in the preceding commit.

Fixes intel#392

Signed-off-by: Paulius Velesko <pvelesko@pglc.io>
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.

IGC miscompiles byval align 64 struct fields when inlining SPIR-V function

1 participant