diff mbox series

libstdc++: Don't use reserved identifiers in simd headers

Message ID ydda6so567m.fsf@CeBiTec.Uni-Bielefeld.DE
State New
Headers show
Series libstdc++: Don't use reserved identifiers in simd headers | expand

Commit Message

Rainer Orth Feb. 1, 2021, 12:21 p.m. UTC
Two simd tests FAIL on Solaris, both SPARC and x86:

FAIL: experimental/simd/standard_abi_usable.cc -msse2 -O2 -Wno-psabi (test for excess errors)
FAIL: experimental/simd/standard_abi_usable_2.cc -msse2 -O2 -Wno-psabi (test for excess errors)

This happens because the simd headers use identifiers documented in the
libstdc++ manual as reserved by system headers.

Fixed as follows, tested on i386-pc-solaris2.11, sparc-sun-solaris2.11,
and x86_64-pc-linux-gnu.

Ok for master?

As an aside, the use of vim: markers initially confused the hell out of
me.  As an Emacs user, I rarely use vi for much more than a pager, but
when I wanted to check the lines mentioned in the g++ errors, I had no
idea what was going on or how to disable the folding enabled there:

// vim: foldmethod=marker sw=2 noet ts=8 sts=2 tw=80

I can't help but feel that this is just a personal preference and
doesn't belong into the upstream code.

For avoidance of doubt, I'd consider equivalent Emacs local variables
equally inappropriate.

	Rainer

Comments

Matthias Kretz Feb. 1, 2021, 1:07 p.m. UTC | #1
On Montag, 1. Februar 2021 13:21:33 CET Rainer Orth wrote:
> Two simd tests FAIL on Solaris, both SPARC and x86:
> 
> FAIL: experimental/simd/standard_abi_usable.cc -msse2 -O2 -Wno-psabi (test
> for excess errors) FAIL: experimental/simd/standard_abi_usable_2.cc -msse2
> -O2 -Wno-psabi (test for excess errors)
> 
> This happens because the simd headers use identifiers documented in the
> libstdc++ manual as reserved by system headers.

Sorry, this code was originally written as non-stdlib code, i.e. without any 
reserved identifiers. I had hoped I found all issues...

> Fixed as follows, tested on i386-pc-solaris2.11, sparc-sun-solaris2.11,
> and x86_64-pc-linux-gnu.
> 
> Ok for master?

Looks good to me.

> As an aside, the use of vim: markers initially confused the hell out of
> me.  As an Emacs user, I rarely use vi for much more than a pager, but
> when I wanted to check the lines mentioned in the g++ errors, I had no
> idea what was going on or how to disable the folding enabled there:
> 
> // vim: foldmethod=marker sw=2 noet ts=8 sts=2 tw=80
> 
> I can't help but feel that this is just a personal preference and
> doesn't belong into the upstream code.

Yes. I guess it's better to remove at least foldmethod. The rest isn't 
personal preference, but coding style requirements. However, I don't need any 
of it anymore: by now my vim config autodetects GCC / libstdc++ code. If the 
rest of libstdc++ doesn't have it, the simd headers probably shouldn't have it 
either.

Best,
  Matthias
Jonathan Wakely Feb. 8, 2021, 10:27 p.m. UTC | #2
On 01/02/21 13:21 +0100, Rainer Orth wrote:
>Two simd tests FAIL on Solaris, both SPARC and x86:
>
>FAIL: experimental/simd/standard_abi_usable.cc -msse2 -O2 -Wno-psabi (test for excess errors)
>FAIL: experimental/simd/standard_abi_usable_2.cc -msse2 -O2 -Wno-psabi (test for excess errors)
>
>This happens because the simd headers use identifiers documented in the
>libstdc++ manual as reserved by system headers.
>
>Fixed as follows, tested on i386-pc-solaris2.11, sparc-sun-solaris2.11,
>and x86_64-pc-linux-gnu.
>
>Ok for master?

OK, thanks.


>As an aside, the use of vim: markers initially confused the hell out of
>me.  As an Emacs user, I rarely use vi for much more than a pager, but
>when I wanted to check the lines mentioned in the g++ errors, I had no
>idea what was going on or how to disable the folding enabled there:
>
>// vim: foldmethod=marker sw=2 noet ts=8 sts=2 tw=80
>
>I can't help but feel that this is just a personal preference and
>doesn't belong into the upstream code.
>
>For avoidance of doubt, I'd consider equivalent Emacs local variables
>equally inappropriate.

We do have Emacs mode lines in all the libstdc++ headers FWIW. I use a
modified copy of https://www.vim.org/scripts/script.php?script_id=3381
to parse those modelines and set Vim config like sw=2 noet ts=2 etc.
when it sees them. So I don't need vim: lines, because the Emacs ones
work for me.

I agree with removing the vim: lines if Matthias is happy to. The
foldmethod is definitely awkward if that's not your preference.
diff mbox series

Patch

# HG changeset patch
# Parent  585afa0bd4a3567358ed9efd210d1a1dab740005
libstdc++: Don't use reserved identifiers in simd headers

diff --git a/libstdc++-v3/include/experimental/bits/simd.h b/libstdc++-v3/include/experimental/bits/simd.h
--- a/libstdc++-v3/include/experimental/bits/simd.h
+++ b/libstdc++-v3/include/experimental/bits/simd.h
@@ -201,8 +201,8 @@  template <size_t _Np>
   inline constexpr overaligned_tag<_Np> overaligned = {};
 
 // }}}
-template <size_t _X>
-  using _SizeConstant = integral_constant<size_t, _X>;
+template <size_t _Xp>
+  using _SizeConstant = integral_constant<size_t, _Xp>;
 
 // unrolled/pack execution helpers
 // __execute_n_times{{{
@@ -4060,11 +4060,11 @@  template <template <int> class _A0, temp
 	      return typename __decay_abi<_A0<_Bytes>>::type{};
 	    else
 	      {
-		using _B =
+		using _Bp =
 		  typename __find_next_valid_abi<_A0, _Bytes, _Tp>::type;
-		if constexpr (_B::template _S_is_valid_v<
-				_Tp> && _B::template _S_size<_Tp> <= _Np)
-		  return _B{};
+		if constexpr (_Bp::template _S_is_valid_v<
+				_Tp> && _Bp::template _S_size<_Tp> <= _Np)
+		  return _Bp{};
 		else
 		  return
 		    typename _AbiList<_Rest...>::template _BestAbi<_Tp, _Np>{};
diff --git a/libstdc++-v3/include/experimental/bits/simd_builtin.h b/libstdc++-v3/include/experimental/bits/simd_builtin.h
--- a/libstdc++-v3/include/experimental/bits/simd_builtin.h
+++ b/libstdc++-v3/include/experimental/bits/simd_builtin.h
@@ -894,12 +894,12 @@  template <typename _Tp, typename _Mp, ty
     class _SimdCastType2
     {
       using _Ap = __intrinsic_type_t<_Tp, _Np>;
-      using _B = __vector_type_t<_Tp, _Np>;
+      using _Bp = __vector_type_t<_Tp, _Np>;
       _SimdMember _M_data;
 
     public:
       _SimdCastType2(_Ap __a) : _M_data(__vector_bitcast<_Tp>(__a)) {}
-      _SimdCastType2(_B __b) : _M_data(__b) {}
+      _SimdCastType2(_Bp __b) : _M_data(__b) {}
       operator _SimdMember() const { return _M_data; }
     };
 
diff --git a/libstdc++-v3/include/experimental/bits/simd_x86.h b/libstdc++-v3/include/experimental/bits/simd_x86.h
--- a/libstdc++-v3/include/experimental/bits/simd_x86.h
+++ b/libstdc++-v3/include/experimental/bits/simd_x86.h
@@ -55,10 +55,10 @@  template <typename _TV,
 
 // }}}
 // __interleave128_lo {{{
-template <typename _Ap, typename _B, typename _Tp = common_type_t<_Ap, _B>,
+template <typename _Ap, typename _Bp, typename _Tp = common_type_t<_Ap, _Bp>,
 	  typename _Trait = _VectorTraits<_Tp>>
   _GLIBCXX_SIMD_INTRINSIC constexpr _Tp
-  __interleave128_lo(const _Ap& __av, const _B& __bv)
+  __interleave128_lo(const _Ap& __av, const _Bp& __bv)
   {
     const _Tp __a(__av);
     const _Tp __b(__bv);