[PATCH] D64790: [LLVM][NFC] Adding an Alignment type to LLVM

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet added a subscriber: lattner.
gchatelet added a comment.

In D64790#1667895 <https://reviews.llvm.org/D64790#1667895>, @jhenderson wrote:

> Thanks for looking at this. I haven't looked at this or the related patches at all, and I know this has landed already. I haven't really looked at the details of the patch, but I did want to highlight a possible conflict in interpretation of 0 alignment. In the elf gABI, the two alignment fields (sh_addralign and p_align) both have the statement "Values 0 and 1 mean no alignment is required". In other words, 0 is not technically an undefined alignment any more than 1 is, at least in the ELF context. That being said, if you are always treating an alignment of 0 as an alignment of 1, I don't think there's any issue.


Thx for the feedback @jhenderson!
I had a quick exchange with @lattner a while ago. He told me that `0` most probably means 1 byte aligned, most of the time, which is inline with what you say.

The type has landed but there's a lot more work to get the codebase converted to use it.
This query <https://reviews.llvm.org/differential/query/R4XuhwYa2YTf/> shows the patches submitted so far.

I'm actively working on it, along the way I learned that the `unsigned` sometimes means an alignment in bytes (powers of two) but also sometimes the log2 of it, in which case 0 means `1ULL << 0`, i.e. 1 byte aligned (see D65945 <https://reviews.llvm.org/D65945>).
I'm as careful as possible and I make sure I understand the consequences when I introduce the type to a part of the codebase, the whole migration will certainly take time.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64790/new/

https://reviews.llvm.org/D64790