The data layout of a module specifies alignment properties of the target to be used.
This allows to specify a non-trivial (>8) alignment of i8.
The current status seems to be unsatisfactory:
Overaligned i8 can be specified without any errors or warnings.
Some passes (e.g. SROA) rely on natural alignment of i8, using a byte offset as GEP index in an i8 GEP. If i8 is overaligned, its alloc size is no longer 1, and the GEP offset is incorrect.
There does not seem to be any explicit documentation (at least I could not find any) on restrictions of data layouts, or i8 alignment specifically.
I’d like to discuss options to improve this situation.
Details
This is motivated by https://reviews.llvm.org/D138708, which also shows a small testcase that is miscompiled by SROA due to overaligned i8s.
Overaligned i8 are used in DXIL, although there seems to be a move towards a new, more natural data layout: DXC data layouts. DXIL is based on LLVM version 3.7, but it is still possible to import DXIL modules using current LLVM, so this might be relevant. Also, there are ongoing efforts to add an HLSL frontend to LLVM: [RFC] Adding HLSL and DirectX support to Clang & LLVM. The first implementation stub (⚙ D122080 Add stub DirectX backend) uses natural alignment for i8. @beanz, can you comment on this?
Options
If we decide that overaligned i8s are not supported in LLVM, we could document that and error out when trying to create a data layout with overaligned i8s. This would break the import of old modules that use such a layout. There might be a way to auto-upgrade such modules by manually adding padding to preserve struct layouts if importing such old modules is absolutely necessary.
cc @nlopes
We would not have this problem either if we had a true Byte type.
I wouldn’t characterize this as ‘some passes’.
This assumption is prevalent throught LLVM.
I do believe this is the correct way forward, at least at present.
Currently, those modules aren’t really supported anyway,
since there is no single thing that assumes that i8’s aren’t overaligned,
but rather the entirety of LLVM does. I’m not sure about auto-update in this case.
As a practical matter, existing DXIL has an overaligned data layout, and being able to continue importing it with the existing bitcode reader would be nicer than forcing GPU compilers to carry two copies of the bitcode reader.
On the plus side, existing DXIL should be fairly tame in terms of where i8 is used, so maybe that helps with the auto upgrade problem.
DXIL is a bit odd, and unfortunately you can’t quite trust DXC’s data layout strings. We have a bunch of requirements that data be 32-bit aligned in ways that LLVM’s data layout can’t quite codify. One area is our vector alignments, which I posted a patch a while back about (⚙ D133379 [DL] Make vector ABI align bound by element align).
As we relate specifically to i8 the DXIL specification states we only support i8 as input parameters to a handful of intrinsics, as constant values, or in metadata. We don’t support i8 for memory access or computation, so we mostly skirt not needing to worry about i8 alignment.
I’ll soon put up a patch to explicitly forbid overaligned i8. This will be enforced during parsing of data layout strings.
As i8 alignment is not relevant in DXIL, in order to continue supporting import of DXIL modules it seems to suffice to just change the i8 alignment during module import.
We already have data layout override callbacks (DataLayoutCallbackTy), but the callback is currently applied after import of the module’s data layout string. I’m planning to change that mechanism to a) apply the callback override before parsing the data layout string (so modules with invalid data layout strings are allowed), and b) provide the old data layout string to the callback as additional argument (in addition to the target triple) so the callback has the option to just fix the i8 alignment and keep the rest as-is.
I’m planning a separate patch for that before the actual i8 alignment change.