[libcxx-commits] [libcxx] [libc++] Use builtin for `std::is_sufficiently_aligned` (PR #173582)

[Nikolas Klauser via libcxx-commits]

philnik777 wrote:

> > > > Why?
> > > 
> > > 
> > > Isn't using compiler builtins the preferred way of implementing the functionality?
> > 
> > 
> > Again, why? The only thing we're doing here is adding code. How is this an improvement compared to doing nothing? What is the benefit of using the builtin compared to the branch we need anyways for GCC?
> 
> @davebayer does have a point: we strive to use compiler builtins to implement functionality instead of writing our own code whenever they are available, even when the manually-written code would be pretty simple (like here). So even if only for consistency, I understand why they had the reflex to use the builtin.
> 
> That being said, in this case since the builtin is not supported by GCC, the "code cleanup" benefit is effectively non-existent (at least for now), so I am somewhat more neutral. But I do think the consistency argument holds water (look at e.g. the gazillion places in type traits where we are using builtins but still have (or had for a while) workarounds for GCC).
> 
> WDYT?

Usually builtins have some sort of benefit. E.g. in the type traits they improve compile times. In some places builtin functions improve the actually generated code (e.g. `__builtin_operator_new`), and in others they are simply more efficient at generating the same code (i.e. they emit the IR directly that would be achieved through optimization passes, e.g. `__builtin_popcountg` and friends). Here I can't think of anything that's improved. We have strictly more code, and the generated IR is identical. I also don't see how this would improve compile times, since the expression is incredibly simple. If we had this builtin available on all compilers it would make the code easier to read, but that's not the case here either, since we can't remove the old code.

TL;DR I don't think the reasons for the use of other builtins applies here.

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