I pushed the attached patch. I kept the operator names... too late, there were already operator names in the stdx::simd implemenation anyway. ;) - Matthias On Monday, 22 May 2023 22:51:49 CEST Jonathan Wakely wrote: > On Mon, 22 May 2023 at 21:27, Matthias Kretz wrote: > > On Monday, 22 May 2023 18:25:15 CEST Jonathan Wakely wrote: > > > I note that using if (not __builtin_constant_evaluated()) will fail if > > > compiled with -fno-operator-names, which is why we don't use 'not', > > > > 'and', > > > > > etc. elsewhere in libstdc++. I don't know if (or why) anybody uses that > > > option though, so I don't think you need to hange anything in > > > stdx::simd. > > > > Ah, I just recently convinced myself that "operator-names" are more > > readable > > (=> easier to maintain). > > I tend to agree, but every time I decide to start using them some testcases > start to fail and I remember why we don't use them :-( > > > But OTOH a mix isn't necessarily better. I'm fine > > with keeping it consistent. > > > > > > * subscripting vector builtins is not allowed in constant expressions > > > > > > Is that just because nobody made it work (yet)? > > > > That is a good question. I guess I should open a PR. > > > > > * if the implementation needs/uses memcpy > > > > > > > * if the implementation would otherwise call SIMD intrinsics/builtins > > > > > > The indentation looks off here and in the _M_set member function > > > > following > > > > > it: > > Yes. I had to put an #if between an else and an if. Looks like this: > > else > > > > #ifdef _GLIBCXX_SIMD_USE_ALIASING_LOADS > > > > if (not __builtin_is_constant_evaluated()) > > return reinterpret_cast*>(this)[__i]; > > > > else > > > > #endif > > > > if constexpr (__is_scalar_abi<_Abi0>()) > > Ah yes, so the if is indented two spaces from the else above it. > What looks wrong to me is that the return is the at the same indentation as > the if controlling it. > > > Should the `if` be aligned to the `else` instead? > > How about moving the two else tokens? > > #ifdef _GLIBCXX_SIMD_USE_ALIASING_LOADS > else if (not __builtin_is_constant_evaluated()) > return reinterpret_cast*>(this)[__i]; > #endif > else if constexpr (__is_scalar_abi<_Abi0>()) > > I think that avoids the issue. > > > > Are the copyright years on > > > testsuite/experimental/simd/pr109261_constexpr_simd.cc correct, or just > > > copy&paste? > > > > Right, copy&paste. Should I simply remove the complete header? > > You could do. I don't think there's much in that test that's novel or worth > asserting copyright over - but if you disagree and want to assign whatever > is copyrightable to the FSF, keep the header but fix the years. Either way > is fine by me. > > OK for trunk and backports, with the comments above suitably resolved. -- ────────────────────────────────────────────────────────────────────────── Dr. Matthias Kretz https://mattkretz.github.io GSI Helmholtz Centre for Heavy Ion Research https://gsi.de stdₓ::simd ──────────────────────────────────────────────────────────────────────────