[llvm] [DataLayout] Add byte specification (PR #106536)

[Björn Pettersson via llvm-commits]

bjope wrote:

(Just some comments for now, haven't really reviewed the code.)

Thanks! This patch may be valuable for people who wants to support other byte-sizes downstream. Even if it never gets merged.

My idea here https://discourse.llvm.org/t/rfc-on-non-8-bit-bytes-and-the-target-for-it/53455/44?u=bjope was to try to add patches "bottom-up" rather than "top-down".
Such as trying to refactor/fix code in a way that it either would magically work for other byte sizes (sometimes with just a little bit of adaption), or make it easy to spot code that really assume 8-bit bytes. This to reduce amount of places that would need to be touch for people who use different byte sizes down stream.
But the idea was not to support changing the byte size upstream. Officially LLVM would still only support 8 bit bytes (at least for now).

I kind of fear that we would hit dead-end again (as any previous attempt), if we open up pandoras box like this. People will say things like "Hey, now when there is a way to specify byte width in the DataLayout, then we need lots of tests to guarantee that it works all over the place. Or how do we describe what the limitations are? And what is a byte really? And do I need to think about this whenever I add a new pass or optimization?".

Maybe one could need a top-level `DataLaytout::getByteWidth() { return 8; }`, just to let us downstream maintainers annotate/assert (in upstream code) that a function has been identified as not supporting other byte sizes.  

I don't know if that makes sense. Nor if it actually would be more feasible doing it in such a "bottom-up" way, compared to starting "top-down" with adding support to set the byte-size without really supporting it throughout LLVM.

If you read this far you probably wonder how we would test that any refactoring in SimplifyLibCalls, or isBytewiseValue, or DAGCombiner, etc that would aim at making life simpler for non-8-bit-bytes. I don't know really. Obviously we rely on this being tested downstream today. So it wouldn't be much different? Although in for example SimplifyLibCalls lots of things should map to C and be described as char rather than i8. So there should really be a "char size" member somewhere (just like one can change CharUnit in clang). And then maybe lit test might use some option to set that when testing SimplifyLibCalls pass? I don't know really, but maybe we can find ways around it without having official support for changing byte size in DataLayout.

(With all that said, it wouldn't mind getting support in-tree for 16-bit bytes. Just being afraid that if we aim too high, then we end up with nothing, just like all previous RFC-discussion about byte size....)

https://github.com/llvm/llvm-project/pull/106536