[llvm-dev] RFC: On removing magic numbers assuming 8-bit bytes

[Jesper Antonsson via llvm-dev]

On Thu, 2019-05-09 at 15:59 -0400, James Y Knight wrote:
> From: JF Bastien via llvm-dev <llvm-dev at lists.llvm.org>
> Date: Thu, May 9, 2019 at 1:30 PM
> To: Jesper Antonsson
> Cc: dag at cray.com, llvm-dev at lists.llvm.org
> 
> > I don’t think you have consensus to move forward at this point in
> > time. My expectation, which I think represents LLVM’s historical
> > approach, is that a path to full support be planned out before this
> > effort starts. Concretely, I expect a real-world backend to be
> > committed to LLVM as a necessary step. What I meant upthread was:
> > yes it makes sense to do cleanups before landing a backend, but
> > someone has to commit to upstreaming a backend before you start the
> > cleanups. When I say a backend I don’t mean a toy, I mean a real
> > backend.
> > 
> 
> IMO, the argument of removing "magic constants", and replacing them
> with a semantically meaningful value does have some merit, even
> lacking any backend that uses a number other than "8".
> 
> I guess I'd say that my feeling is that if llvm's codebase already
> had a "bitsPerUnit()" accessor (as GCC calls this concept), I would
> probably not say that we should replace all the calls with literal
> '8's across the code-base simply because no current target uses any
> value other than 8. It's an abstraction which does make sense on its
> own, even if it's currently always 8.
> 
> So, I'm somewhat agnostic here -- the change itself has little value
> upstream, but if the state of the codebase afterwards is not degraded
> by the change (and on the contrary, can be ever-so-marginally
> improved), that seems like it could be basically okay.

This is a salient point, I think: If we had the abstraction, I could
never hope to get consensus to replace it with 8. While there is no
agreement, it seems to me that the community reaction leans toward the
abstraction-positive.

As to the argument that a path to full support is a prerequisite, and
that that is how it has always been done, I think every suggestion is
worth considering on it's own merits. If this abstraction is a slight
improvement in itself, is closing a gap towards gcc and represents a
significant improvement for OOT backends, is there sufficient rationale
for not allowing it?

> > Right now we have no commitment on anybody landing a backend, and
> > we don’t really have a concrete idea of what you’re even proposing
> > to change or how. You’re focusing on “magic numbers” like everyone
> > agrees 8 is the root of all evil, and it’s really not. Let’s say
> > someone promises to upstream a backend, what concretely do you need
> > to change, and in which projects, to get there? Are you changing
> > clang, and how? What about libc++? Linker? LLVM, and how? Is IR
> > going to change? If not, do you keep all the i8* around, and how do
> > you work around not having void* in IR?
> > 
> 
> This is a great list of questions that it would be good to have
> answers to, though.

By putting a patch in Phabricator and linking it from the RFC, I tried
to convey a very concrete idea of what we propose to change and how. We
don't propose changes outside of llvm.

The 8's are not the root of all evil, but again, magic numbers are
generally considered bad style. Also, these 8's represent the bulk of
our changes to handle 16-bit bytes in llvm and as they are typically
straightforward to remove, they represent low-hanging fruit. John
McCall mentioned that clang needs fewer changes, which is something we
can confirm and we also experience very few merge conflicts there.

The questions are indeed good in a sense, and I could make a writeup on
them, e.g llvm IR is general and good as it is, except that you may
want an additon to the DataLayout string to signal AU width. However,
as we don't suggest providing support, only this abstraction, it seems
premature to go into these other details. Of course, when someone is
ready to upstream a target with non-8-bit addressable units, we'll
engage and help out with  concrete suggestions as we've been doing with
fixedpoint numbers recently.