[cfe-dev] FYI, Intel folks might be looking to add the __iso_volatile_Xxx family for MSVC STL <atomic> soon

Thu Mar 28 14:25:59 PDT 2019

Hmmmm now that I think about this maybe we should be using a totally different set of builtins on clang anyway.

We currently are doing this:

#define _Compiler_barrier() _ReadWriteBarrier()

#if defined(_M_ARM) || defined(_M_ARM64)
#define _Memory_barrier() __dmb(0xB) // inner shared data memory barrier
#define _Compiler_or_memory_barrier() _Memory_barrier()

#define _ISO_VOLATILE_STORE8(_Storage, _Value) __iso_volatile_store8(_Atomic_address_as<char>(_Storage), _Value)
#define _ISO_VOLATILE_STORE16(_Storage, _Value) __iso_volatile_store16(_Atomic_address_as<short>(_Storage), _Value)
#define _ISO_VOLATILE_STORE32(_Storage, _Value) __iso_volatile_store32(_Atomic_address_as<int>(_Storage), _Value)
#define _ISO_VOLATILE_STORE64(_Storage, _Value) __iso_volatile_store64(_Atomic_address_as<long long>(_Storage), _Value)
#define _ISO_VOLATILE_LOAD8(_Storage) __iso_volatile_load8(_Atomic_address_as<const char>(_Storage))
#define _ISO_VOLATILE_LOAD16(_Storage) __iso_volatile_load16(_Atomic_address_as<const short>(_Storage))

#elif defined(_M_IX86) || defined(_M_X64)
// x86/x64 hardware only emits memory barriers inside _Interlocked intrinsics
#define _Compiler_or_memory_barrier() _Compiler_barrier()

#define _ISO_VOLATILE_STORE8(_Storage, _Value) (*_Atomic_address_as<char>(_Storage) = _Value)
#define _ISO_VOLATILE_STORE16(_Storage, _Value) (*_Atomic_address_as<short>(_Storage) = _Value)
#define _ISO_VOLATILE_STORE32(_Storage, _Value) (*_Atomic_address_as<long>(_Storage) = _Value)
#define _ISO_VOLATILE_STORE64(_Storage, _Value) (*_Atomic_address_as<long long>(_Storage) = _Value)
#define _ISO_VOLATILE_LOAD8(_Storage) (*_Atomic_address_as<const char>(_Storage))
#define _ISO_VOLATILE_LOAD16(_Storage) (*_Atomic_address_as<const short>(_Storage))

#else // ^^^ x86/x64 / unsupported hardware vvv
#error Unsupported hardware
#endif // hardware

...
// in _Atomic_storage<_Ty, 1>:

    _NODISCARD _Ty load() const noexcept { // load with sequential consistency
        char _As_bytes = _ISO_VOLATILE_LOAD8(_Storage);
        _Compiler_or_memory_barrier();
        return reinterpret_cast<_Ty&>(_As_bytes);
    }

________________________________
From: Billy O'Neal (VC LIBS)
Sent: Thursday, March 28, 2019 02:20 PM
To: Reid Kleckner; JF Bastien
Cc: cfe-dev
Subject: Re: [cfe-dev] FYI, Intel folks might be looking to add the __iso_volatile_Xxx family for MSVC STL <atomic> soon

> all they do is emit a volatile load

If I understand correctly the point of these is to make an ordinary volatile load/store happen on ARM under /volatile:ms<https://docs.microsoft.com/en-us/cpp/build/reference/volatile-volatile-keyword-interpretation?view=vs-2017>. We asked for the intrinsic to also be provided on Intel because it reduces the amount of macro-tastic-ness we need in <atomic>.

Billy3
________________________________
From: Reid Kleckner <rnk at google.com>
Sent: Thursday, March 28, 2019 02:18 PM
To: JF Bastien
Cc: Billy O'Neal (VC LIBS); cfe-dev
Subject: Re: [cfe-dev] FYI, Intel folks might be looking to add the __iso_volatile_Xxx family for MSVC STL <atomic> soon

On Thu, Mar 28, 2019 at 2:14 PM Reid Kleckner <rnk at google.com<mailto:rnk at google.com>> wrote:
On Thu, Mar 28, 2019 at 1:46 PM JF Bastien <jfbastien at apple.com<mailto:jfbastien at apple.com>> wrote:
On Mar 28, 2019, at 12:02 PM, Billy O'Neal (VC LIBS) <bion at microsoft.com<mailto:bion at microsoft.com>> wrote:

Update: VS2019 Update 2 is likely to do this; who do I need to prod such that I get to use these unconditionally in our <atomic> ?🙂

You want the same feature in clang-cl, so I suggest talking to Reid, sending a patch to clang, and having him review it ;-)

I think it's just a matter of moving code from BuiltinsAArch64.def<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project%2Fblob%2Fmaster%2Fclang%2Finclude%2Fclang%2FBasic%2FBuiltinsAArch64.def%23L82&data=02%7C01%7Cbion%40microsoft.com%7C83aeb1e5d31d4b7b3bbd08d6b3c2ec83%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C1%7C636894047106237010&sdata=BYsRUy%2FwIT9OchMsEoez7TIPa9iHTqVPI3bjlv25ztE%3D&reserved=0> (and ARM) to Builtins.def, and maybe some followon changes.

The naming for these intrinsics is confusing. It reads as, "do an ISO C++ standard volatile load", but it really means "do an atomic (not volatile) load", but I suppose the time to address that is long gone.

Huh, maybe I'm the one confused, because all they do is emit a volatile load. What exactly do you want to use these for, then? I'm not sure we've implemented them correctly.

I went ahead and started a patch to make them available.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20190328/9e8c734b/attachment.html>