Alignment in LLVM is currently represented with an
unsigned, sometimes an
FWIU the value has the following possible semantics:
- 0 means alignment is unknown,
- 1 means no alignment requirement,
- a power of two means a required alignment in bytes.
unsigned throughout the codebase has several disadvantages:
- comparing alignments may compare different things, what does
A < Bor
std::min(A, B)mean when A or B are allowed to be 0?
- integer promotion can kick in and turn a
boolin an alignment when calling a function (1)
- masking may lead to incorrect result when value is 0 (2)
- integer promotion leads to not obviously correct code in the presence of signed and unsigned values (3)
- dividing an alignment by 2 can change the associated semantic from
- developers have to be defensive to make sure assumptions hold (5)
- checking that an offset is aligned is sometimes done backward
Alignment % Offset == 0instead of
Offset % Alignment == 0(6) (7)
- MachineConstantPoolEntry::Alignment encodes special information in its topmost bit (8) but
AsmPrinter::GetCPISymbolseems to use it directly (9) instead of calling
I have a patch to introduce alignment object in LLVM.
This patch does not fix the code but replaces the unsigned value by a type so it’s easier to introduce proper semantic later on.
The patch (11) is too big to be sent to Phabricator, arc diff complains about server’s
post_max_size being too small.
I would like to seek guidance from the community:
- Is this patch worthwhile?
- If so, how should it be reviewed? Should it be split?
PS: If you intend to have a look at it you should start with
1 - https://github.com/llvm/llvm-project/blob/b725d27350ffdda9e8e9144668ad54c922297f59/llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp#L2593
2 - https://github.com/llvm/llvm-project/blob/b725d27350ffdda9e8e9144668ad54c922297f59/llvm/lib/CodeGen/SafeStack.cpp#L545
3 - https://github.com/llvm/llvm-project/blob/b725d27350ffdda9e8e9144668ad54c922297f59/llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp#L1533
4 - https://github.com/llvm/llvm-project/blob/b725d27350ffdda9e8e9144668ad54c922297f59/llvm/lib/CodeGen/CodeGenPrepare.cpp#L6751
5 - https://github.com/llvm/llvm-project/blob/d0307f93a7658e6d0eef1ffd0b0ed4f1506bfc13/llvm/include/llvm/Analysis/VectorUtils.h#L278
6 - https://github.com/llvm/llvm-project/blob/fafec5155e39f5dad098376c1beb4a56604aa655/llvm/lib/Target/ARM/ARMCallingConv.cpp#L207
7 - https://github.com/llvm/llvm-project/blob/d0307f93a7658e6d0eef1ffd0b0ed4f1506bfc13/llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp#L563
8 - https://github.com/llvm/llvm-project/blob/7eeb82b58554163962d2696ce9be7d021d5b25d4/llvm/include/llvm/CodeGen/MachineConstantPool.h#L76
9 - https://github.com/llvm/llvm-project/blob/7eeb82b58554163962d2696ce9be7d021d5b25d4/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp#L2809
10 - https://github.com/llvm/llvm-project/blob/7eeb82b58554163962d2696ce9be7d021d5b25d4/llvm/include/llvm/CodeGen/MachineConstantPool.h#L96