[libcxx-commits] [PATCH] D108906: Add std::assume_aligned

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

Also, I just re-read the thread in https://reviews.llvm.org/D54966 (in particular Chandler's comment in https://reviews.llvm.org/D54966#1340163) and my understanding was that we really want to use `__builtin_assume_aligned` because that will propagate the correct alignment assumption in the IR.

I think the `constexpr` issue can be solved as:

  template <size_t N, typename T>
  [[nodiscard]]
  _LIBCPP_HIDE_FROM_ABI
  constexpr T* assume_aligned(T* __ptr) {
    if (is_constant_evaluated()) {
      return __ptr;
    } else {
      return __builtin_assume_aligned(__ptr, N);
    }
  }

As a side benefit, this should also work on GCC (and in fact this implementation is basically what libstdc++ does).

What do you think?



================
Comment at: libcxx/include/memory:957
+#if _LIBCPP_STD_VER >= 20
+template <size_t N, typename T>
+[[nodiscard]]
----------------
Those parameters should be uglified (`<size_t _Np, class _Tp>`). Also, we use `class` instead of `typename` -- I don't like that, but it's consistent with the rest of the code base.


================
Comment at: libcxx/test/std/utilities/memory/ptr.align/assume_aligned.fail.cpp:1
+//===----------------------------------------------------------------------===//
+//
----------------
ldionne wrote:
> Can you please rename this to `assume_alligned.verify.cpp`?
`assume_aligned.verify.cpp`


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

https://reviews.llvm.org/D108906