I came across this message from Linus to a contributor and it reminds me of a thing I’ve learned over the years using Zig:
No. This is garbage…[snip]…Like this crazy and pointless make_u32_from_two_u16() “helper”.
That thing makes the world actively a worse place to live. It’s
useless garbage that makes any user incomprehensible, and actively WORSE than not using that stupid “helper”.
If you write the code out as “(a << 16) + b”, you know what it does
and which is the high word. Maybe you need to add a cast to make sure
that ‘b’ doesn’t have high bits that pollutes the end result, so maybe
it’s not going to be exactly pretty, but it’s not going to be wrong
and incomprehensible either.
In contrast, if you write make_u32_from_two_u16(a,b) you have not a
f%^5ing clue what the word order is. IOW, you just made things WORSE, and you added that “helper” to a generic non-RISC-V file
where people are apparently supposed to use it to make other code
worse too.
The key insight for me is:
If you write the code out as (a << 16) + b, you know what it does…if you write make_u32_from_two_u16(a, b) you have not a f%^5ing clue…
To me this is very “Zig”. I’ve learned to respect the downside of abstracting details away. Having to look in two different places to understand something is a cost that took me a long time to realize and start considering in my own code. I’d fallen into the trap that all code should be as DRY (don’t repeat yourself) as possible, thinking that calling a function with a name was less error prone than writing language primitives that everyone already understands.
It’s also ironic that programming this way can be efficient when working alone. It’s similar to creating a domain-specific language: at first it feels empowering and fast, but over time it raises the barrier for anyone else (including your future self) to understand and modify the code.
Linus’s frustration here actually echoes a lot of Zig’s Zen:
Communicate intent precisely: (a << 16) + b communicates the exact operation, while make_u32_from_two_u16(a, b) hides detail.
Favor reading code over writing code: The cost of understanding the code later outweighs the few keystrokes saved today.
Only one obvious way to do things: Bit-shifting is the clear and standard idiom; wrapping it in a helper just adds ambiguity.
Reduce the amount one must remember: You shouldn’t have to remember (or look up) what “order” a helper function assumes.
The DRY principle isn’t wrong, but if applying it makes the code less obvious, you’re trading a short-term convenience for a long-term liability.
I kinda disagree.
Consider a utility function like:
rgb(u8, u8, u8) u24
rgba(u8, u8, u8, u8) u32
You could write the bit shifts with all the parens, it’s clear and all, but why go through the trouble in your whole codebase instead of using a simple to understand utility function. The order is also apparent from the function’s name and from convention.
In Zig in particular, you could also describe exact behaviour for the utility function by turning its arguments into a struct type: fn u32_from_u16s(u16s: struct{upper: u16, lower: u16}) u32
That way, the user is forced to specify exactly which u16 is the upper and which is the lower.
I think Linus is still based in reference to the original problem - for something as simple as this, it’s actually fewer keystrokes to write the bit-shifting and anyone who understands bit operators understands exactly what it does.
As far as “your whole codebase” goes, it’s less trouble.
The one thing I do appreciate about utility functions like this in API design is that they remind users that you can do something in the first place.
That’s probably what the original designers were going for.
This is especially ironic because what Linus is really ranting about it that you can’t call C functions with keyword arguments like “upper” or “lower” in order to document which u16 is what. Erm, Linus, perhaps a better programming language might help?
However, to me, Linus’ solution would get big red flags in a code review as it is quite specific to x86-like architectures.
(a << 16) + b only works if your “int” is 32-bits (or more) which causes integral promotion to work correctly. And if you use “+” instead of “|” for bitwise splicing, you’re about to get a loooooong rant from me about the perils of sign-extension and why we almost exclusively use unsigned types and bitwise operators when mangling bits.
If I’m writing a one-off: ((uint32_t)a << 16) | (b & 0xFFFF); is the minimum I would do. The cast ensures that my left shift does what I expect it to do regardless of what int is on my architecture (including masking off bits above bit 31). The mask ensures that I get no more than the lower 16 bits regardless of what type b is. And the bitwise OR means that I never have to worry about accidental overflows.
If we’re on an x86 machine, the cast and mask are free as the compiler will make them no-ops. If we’re not on an x86 machine, I’m covered.
However, if I were writing that line of code a lot, sorry, Linus, but I’m going to factor that out into an inline function. Especially since now I can ensure that my incoming and outgoing types are exactly what I expect them to be and I will get errors/warnings when they are not.
Except that Zig doesn’t let you do this, you’ll have to add all sorts of explicit typecasting builtins - which then in turn almost certainly ends up in a helper function because it’s more readable
…so I’m not sure if Torvalds would actually vibe with Zig
(FWIW this is also a topic where I’m heavily divided - on one hand the required casts are more ‘correct’, OTH it’s a royal PITA which I will probably never get used to, tbf though Rust is even worse in this specific area - but Rust is for masochists, so it doesn’t really count)
This is really just window dressing when a and b are both uint16_t and int is 32-bits. The only downside of C’s integer promotion to int is that int is stuck on 32 bits even on 64-bit machines, which has been a Really Bad Idea [TM] in hindsight.
Tbf, you can do the same in C, you just can’t define the struct right in the function signature (well you can, but unfortunately the struct will then only be visible inside the function). But other then that detail the idea translates just fine to C:
Your code is a wonderful example of why we don’t use add for bit splicing.
Take a look at how big the code for add is because of checks:
Versus the code for bitwise-OR (which doesn’t need those checks):
Sure, we can turn those checks off with “+%” and get the same size code. However, if we don’t try to be clever, we don’t need to turn those checks off.
The problem here is that Linus is exhibiting “compiler brain” or “(premature) optimization brain” of the x86 variety. “load immediate” generally is a pseudo-instruction that will decompose into the various whacky architecture specific instructions (generally a load of high bits/shift immediate combined with a sign-extended add/subtract immediate) depending upon what’s efficient for the architecture.
Linus’ code is an optimization only if your immediates are 16-bits or larger. That’s isn’t true for RISC-V. RISC-V immediates are only 12 bits (we’re ignoring the 20-bit lui immediate you pedants). And do note that the submitted code was specifically for RISC-V.
I understand what Linus was getting at here, and he’s not wrong. However, he’s also not unequivocally right from the technical side.
(Note: Linus’ vitriol is also complaining about a bunch of things unrelated to the purely technical aspects–polluting global areas with RISC-V specific stuff as well as the late merge requests. The technical comment I’m sure was kind of an off the cuff thing.)
Note that the correct compile flag is -OReleaseSmall, not -DReleaseSmall. It still matters in the debug build but if you actually compile as ReleaseSmall, you end up with the same output.
Nevertheless, the point stands that the logic version doesn’t need to do anything to get small code while the arithmetic version requires active intervention that one can screw up (as I did ).
Tbf, the compiler should be able to figure out that adding two u16’s into an u32 cannot overflow, and with the left-shift by 16 bits +, +% and | are all equivalent. Personally I also prefer bit operations, but I was just running with the original example
Littering your code with tons of small helper functions adds indirection to your code. Even if you have a nice editor with jump-to-definition, it still means that reading your code becomes more and more non-linear over time.
Using named arguments doesn’t solve this problem. It will make the callsites more descriptive, but you still need to jump all over the place to understand the code.
Other abstractions like interfaces/traits also make your code more indirect, which is another reason to use them sparingly. They are not “zero cost”, even if they have no runtime cost…they have a readability cost.
I aggressively inline code not only to make it read more linearly, but also to prevent premature abstraction. Inlined code can’t be reused; prematurely deduplicating often leads to ugly helper functions with tons of options to configure them. In those cases, duplication can be cleaner.
I don’t know if this is very “Zig” or just my own taste. People like Jon Blow and Casey Muratori have talked about how long, imperative functions have been given a bad rap. I agree with them…bring back big functions :^D
).