On 06/06/17 11:23 +0100, Jonathan Wakely wrote:
>On 06/06/17 11:07 +0100, James Greenhalgh wrote:
>>On Fri, Jun 02, 2017 at 07:13:17PM +0100, Jonathan Wakely wrote:
>>>On 02/06/17 19:19 +0200, Ulrich Drepper wrote:
>>>>On Fri, Jun 2, 2017 at 5:46 PM, Michael Collison
>>>><Michael.Collison@arm.com> wrote:
>>>>>This implementation includes "arm_neon.h" when including the optimized <ext/random>. This has the effect of polluting the global namespace with the Neon intrinsics, so user macros and functions could potentially clash with them. Is this acceptable given this only happens when <ext/random> is explicitly included?
>>>>
>>>>I don't think it is. Ideally the sfmt classes would get added to the
>>>>standard some day; they are much better performance-wise than the mt
>>>>classes. At that point you'd have no excuse anymore.
>>>>
>>>>Why are the symbols in arm_neon.h not prefixed with an underscore as
>>>>it is the case for equivalent definitions of other platforms (notably
>>>>x86)? If you start adding such optimizations I suggest you create
>>>>headers which do not pollute the namespace and include it in
>>>>arm_neon.h and whatever other header is needed to define the symbols
>>>>needed for compatibility.
>>>>
>>>>Nip the problem in the butt now, this is probably just the beginning.
>>
>>We're a good number of years late to do that without causing some pain.
>>
>>The arm_neon.h names for AArch64 come from those for the ARM port, which
>>themselves started out in the mid 2000s in ARM's proprietary compiler. These
>>names are now all over the place; from LLVM, through popular proprietary
>>toolchains, through a variety of developer guides and codebases, and in
>>to GCC.
>>
>>Forging a new interface is possible, but expecting migration to the
>>namespace-safe names seems unlikely given the wide spread of the current
>>names. Essentially, we'd be doubling our maintenance, and asking all
>>other compilers to follow. It is possible, but we'd not want to do it
>>lightly.
>>
>>In particular, my immediate worry is needing other compilers to support the
>>new names if they are to use libstdc++.
>>
>>If we don't mind that, and instead take a GCC-centric view, we could avoid
>>namespace polution by using the GCC-internal names for the intrinsics
>>(__builtin_aarch64_...). As those names don't form a guaranteed interface,
>>that would tie us to a GCC version.
>
>Libstdc++ is already tied to a GCC version when used with GCC. Using
>the internal names might cause problems when using libstdc++ with
>non-GNU compilers if they don't use the same names (it's used with at
>least Clang and ICC).
>
>>So we have a few solutions to choose from, each of which invokes a trade-off:
>>
>>1 Use the current names and pollute the namespace.
>>2 Use the GCC internal __builtin_aarch64* names and tie libstdc++ to GCC
>> internals.
>>3 Define a new set of namespace-clean names and complicate the Neon
>> intrinsic interface while we migrate old users to new names.
>>
>>I can see why the libstdc++ community would prefer 3) over the other options,
>>but I'm reticent to take that route as the cost to our intrinsic maintainers
>>and users looks high. I've added some of the other ARM port maintainers
>>for their opinion.
>>
>>Are there any other options I'm missing?
>
>3b) Define a new subset of names for the parts needed by this patch,
> but don't expect users to migrate to them.
>
>That probably doesn't have any advantage over 2.
>
I suppose <ext/random> could do something like:
#if __GNUC__ >= 8 // This is GCC not Clang pretending to be GCC 4.2
namespace __detail {
using __int32x4_t = __Int32x4_t;
using __int8x16_t = __Int8x16_t;
}
#else
#include "arm_neon.h"
namespace __detail {
using __int32x4_t = int32x4_t;
using __int8x16_t = int8x16_t;
}
#endif
But we'd need to redefine or wrap the operations like veorq_u8 which
would be a pain.