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.
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.