[llvm-dev] Introducing an Alignment object in LLVM

[Guillaume Chatelet via llvm-dev]

Alignment in LLVM is currently represented with an `unsigned`, sometimes an
`uint32_t`, `uint64_t` or `uint16_t`.
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.

Using `unsigned` throughout the codebase has several disadvantages:
 - comparing alignments may compare different things, what does `A < B` or
`std::min(A, B)` mean when A or B are allowed to be 0?
 - integer promotion can kick in and turn a `bool` in 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
`unaligned` to `unknown alignment` (4)
 - developers have to be defensive to make sure assumptions hold (5)
 - checking that an offset is aligned is sometimes done backward `Alignment
% Offset == 0` instead of `Offset % Alignment == 0` (6) (7)
 - MachineConstantPoolEntry::Alignment encodes special information in its
topmost bit (8) but `AsmPrinter::GetCPISymbol` seems to use it directly (9)
instead of calling `getAlignment()` (10)

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?

-- Guillaume
PS: If you intend to have a look at it you should start with
`llvm/include/llvm/Support/Alignment.h`

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
11 -
https://drive.google.com/file/d/1PgrEuNjIcSH9hOs6REe9FedCH5mf43Q9/view?usp=sharing
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20190712/cb309c24/attachment.html>