День добрый, необходимо написать свою реализацию InterlockedExchange, корректен ли такой код?

LONG MyInterlockedExchange(LPLONG Target, LONG Value)
{
    volatile long l = *Target; // Save previouse value

    __asm
    {
        mov        eax, Value
        mov        edx, Target
        lock    xchg DWORD PTR[edx],eax
    }

    return l;
}

Здравствуйте, slavo, Вы писали:

S> volatile long l = *Target; // Save previouse value
Completely broken

LONG MyInterlockedExchange(LPLONG Target, LONG Value)
{
    long r;
    __asm
    {
        mov        eax, Value
        mov        edx, Target
        lock       xchg DWORD PTR[edx],eax
        mov        r, eax
    }
    return r;
}

А лучше смотри дизассемблер в оригинальных функциях, там точно правильно.

Здравствуйте, remark, Вы писали:

R>Здравствуйте, slavo, Вы писали:

S>> volatile long l = *Target; // Save previouse value
R>Completely broken

Исправил на

LONG InterlockedExchange_NWa(LPLONG Target, LONG Value)
{
    volatile long lPrev;

    __asm
    {
        mov        eax, Value
        mov        edx, Target
        lock    xchg DWORD PTR[edx],eax
        mov        lPrev, eax
    }

    return lPrev;
}

Здравствуйте, remark, Вы писали:

Я пользую такой

__forceinline DWORD InterlockedExchange (DWORD &value, DWORD data)
{
__asm
{
mov ecx, value
mov eax, DWORD ptr [data]
lock xchg DWORD ptr [ecx],eax
}
}

Значение тут в любом случае возвращается в EAX, поэтому загонять ее в локальную переменную а потом делать return — напрасная работа.

Здравствуйте, CreatorCray, Вы писали:

CC>Здравствуйте, remark, Вы писали:

CC>Я пользую такой

CC>__forceinline DWORD InterlockedExchange (DWORD &value, DWORD data)
CC>{
CC> __asm
CC> {
CC> mov ecx, value
CC> mov eax, DWORD ptr [data]
CC> lock xchg DWORD ptr [ecx],eax
CC> }
CC>}

CC>Значение тут в любом случае возвращается в EAX, поэтому загонять ее в локальную переменную а потом делать return — напрасная работа.

Это некорректный код. При оптимизациях он может некорректно компилироваться.
Проблема в том, что ты пытаешься надуть компилятор, а он мстительный. Если ты пишешь "обычную" функцию (т.е. без __declspec(naked)), то ты *должен* писать return.


Тут тебе надо либо идти до конца:

__declspec(naked) DWORD InterlockedExchange (DWORD &value, DWORD data)
{
  __asm
  {
    mov ecx, [esp+4]
    mov eax, DWORD ptr [esp+8]
    lock xchg DWORD ptr [ecx],eax
    ret
  }
}

Либо не мухлевать:

DWORD InterlockedExchange (DWORD &value, DWORD data)
{
        DWORD res
    __asm 
    {
        mov    ecx, value
        mov    eax, DWORD ptr [data]
        lock xchg DWORD ptr [ecx],eax 
                mov     res, eax
    }
        return res;
}

По поводу напрасной работы. Вообще мой опыт показывает, что второй вариант лучше.
По крайней мере студия прекрасно встраивает эту функцию и на месте вызова остаётся только:

        mov    ecx, value
        mov    eax, DWORD ptr [data]
        lock xchg DWORD ptr [ecx],eax

Где соотв. value и data заменены на аргументы.
Никакой напрасной работы по копированию eax в res не остаётся.

Здравствуйте, remark, Вы писали:

R>Это некорректный код. При оптимизациях он может некорректно компилироваться.
R>Проблема в том, что ты пытаешься надуть компилятор, а он мстительный. Если ты пишешь "обычную" функцию (т.е. без __declspec(naked)), то ты *должен* писать return.
Теория также говорит что асм блоки будут встраиваться как они встраиваются и результат функции ожидается из EAX.
А 4 года практического использования этой функции показали что проблем с ней нет никаких.

R>Тут тебе надо либо идти до конца:
Так не проинлайнится. Я только из за инлайна свои набросал. А так пользовал бы WinAPIшные.

R>Либо не мухлевать:
R>По поводу напрасной работы. Вообще мой опыт показывает, что второй вариант лучше.
R>По крайней мере студия прекрасно встраивает эту функцию и на месте вызова остаётся только:

ICC 10

;;;     value2 = InterlockedExchange1 (value1,1);
        mov       DWORD PTR [esp+20], 1
        lea       eax, DWORD PTR [esp+8]
        mov       DWORD PTR [esp+16], eax
; Begin ASM
        mov       ecx, DWORD PTR [esp+16]
        mov       eax, DWORD PTR [esp+20]
        lock       xchg DWORD PTR [ecx], eax
; End ASM
        mov       edx, eax

;;;     value4 = InterlockedExchange2 (value3,1);
        mov       DWORD PTR [esp+28], 1
        lea       eax, DWORD PTR [esp+4]
        mov       DWORD PTR [esp+24], eax
; Begin ASM
        mov       ecx, DWORD PTR [esp+24]
        mov       eax, DWORD PTR [esp+28]
        lock       xchg DWORD PTR [ecx], eax
        mov       DWORD PTR [esp+12], eax
; End ASM
        mov       eax, DWORD PTR [esp+12]

VC 7.1

; 35   :     value2 = InterlockedExchange1 (value1,1);
    lea    eax, DWORD PTR _value1$[esp+20]
    mov    DWORD PTR $T55583[esp+20], 1
    mov    DWORD PTR $T55584[esp+20], eax

    mov    ecx, DWORD PTR $T55584[esp+20]
    mov    eax, DWORD PTR $T55583[esp+20]
    lock     xchg     DWORD PTR [ecx], eax

    mov    edx, eax

; 37   :     value4 = InterlockedExchange2 (value3,1);
    lea    ecx, DWORD PTR _value3$[esp+20]
    mov    DWORD PTR $T55589[esp+20], 1
    mov    DWORD PTR $T55590[esp+20], ecx

    mov    ecx, DWORD PTR $T55590[esp+20]
    mov    eax, DWORD PTR $T55589[esp+20]
    lock     xchg     DWORD PTR [ecx], eax
    mov    DWORD PTR _res$55587[esp+20], eax

    mov    eax, DWORD PTR _res$55587[esp+20]

сурс

#include <windows.h>
#include <stdio.h>

__forceinline DWORD InterlockedExchange1 (DWORD &value, DWORD data)
{
    __asm 
    {
        mov    ecx, value
        mov    eax, DWORD ptr [data]
        lock xchg DWORD ptr [ecx],eax 
    }
}

DWORD InterlockedExchange2 (DWORD &value, DWORD data)
{
    DWORD res;
    __asm 
    {
        mov    ecx, value
        mov    eax, DWORD ptr [data]
        lock xchg DWORD ptr [ecx],eax 
        mov     res, eax
    }
    return res;
}

int main ()
{
    DWORD value1, value2, value3, value4;

    value1 = value3 = 0;

    value2 = InterlockedExchange1 (value1,1);
    value4 = InterlockedExchange2 (value3,1);

    wprintf(L"%i %i %i %i\n",value1,value2,value3,value4);    // Чтоб интел не выоптимизировал их вычисление
}

R>Где соотв. value и data заменены на аргументы.
R>Никакой напрасной работы по копированию eax в res не остаётся.
Как видишь — мимо кассы. Какая у тя MSVC кстати?

Кроме того, компилятор не имеет права "оптимизировать" тот ASM что написал программер.
То, что твоя MSVC лезет куда ее не просят — это ИМХО баг.

R>
Разумеется

Здравствуйте, CreatorCray, Вы писали:

CC>Здравствуйте, remark, Вы писали:

R>>Это некорректный код. При оптимизациях он может некорректно компилироваться.
R>>Проблема в том, что ты пытаешься надуть компилятор, а он мстительный. Если ты пишешь "обычную" функцию (т.е. без __declspec(naked)), то ты *должен* писать return.
CC>Теория также говорит что асм блоки будут встраиваться как они встраиваются и результат функции ожидается из EAX.

Но теория не говорит, что делает сгенерированный компилятором блок выхода из функции.

CC>А 4 года практического использования этой функции показали что проблем с ней нет никаких.

У меня на mavc8sp1 пару недель назад слетело.



R>>Тут тебе надо либо идти до конца:
CC>Так не проинлайнится. Я только из за инлайна свои набросал. А так пользовал бы WinAPIшные.

Да, так скорее всего не проинлайниться. Поэтому я это и не рекомендую.
Если ты используешь студию, то там начиная с msvc71 есть набор аналогичных Interlocked интринсик функций, начинающихся с _Interlocked. Начиная с msvc8 для них даже есть хидер <intrin.h>
Их надо использовать вместо WinAPI. Они встраиваются оптимальным образом, как на асме руками ты просто не можешь написать.



R>>Либо не мухлевать:
R>>По поводу напрасной работы. Вообще мой опыт показывает, что второй вариант лучше.
R>>По крайней мере студия прекрасно встраивает эту функцию и на месте вызова остаётся только:
CC>Как видишь — мимо кассы. Какая у тя MSVC кстати?

msvc8sp1


CC>Кроме того, компилятор не имеет права "оптимизировать" тот ASM что написал программер.

Но встроить он может.


CC>То, что твоя MSVC лезет куда ее не просят — это ИМХО баг.

А он асм не трогает, он его только встраивает.


CC>Разумеется

Здравствуйте, CreatorCray, Вы писали:

CC>__forceinline DWORD InterlockedExchange1 (DWORD &value, DWORD data)

Судя по __forceinline этот код не компилялся g++. Возможно ли, что g++ потребует явного return?

Здравствуйте, remark, Вы писали:

R>Но теория не говорит, что делает сгенерированный компилятором блок выхода из функции.
ничего, если в функции нет локальных переменных

CC>>А 4 года практического использования этой функции показали что проблем с ней нет никаких.
R>У меня на mavc8sp1 пару недель назад слетело.
Потому и слетало у тя видимо что компилер полез менять то, что написано в asm блоках, ну и доменялся...
Я MSVC8 вообще за нормальный C++ компилер не считаю. Пусть сперва POSIX вернут, инициаторы хреновы

R>Если ты используешь студию, то там начиная с msvc71 есть набор аналогичных Interlocked интринсик функций, начинающихся с _Interlocked.
R> Начиная с msvc8 для них даже есть хидер <intrin.h>
R>Их надо использовать вместо WinAPI. Они встраиваются оптимальным образом, как на асме руками ты просто не можешь написать.
Это все хорошо, но Microsoft specific и для ICC их нет.
Мой вариант от них отличается только тем, что параметры не встраиваются.
Да и кажется мне, что твой код встроился только потому, что был распознан как intrinsic — попробуй у себя функцию самописную назвать по-другому

CC>>Кроме того, компилятор не имеет права "оптимизировать" тот ASM что написал программер.
R>Но встроить он может.
Если для этого ему надо менять ASM код — то не может.

CC>>То, что твоя MSVC лезет куда ее не просят — это ИМХО баг.
R>А он асм не трогает, он его только встраивает.
он у тя убрал асм инструкцию, написанную руками.

CC>>Разумеется
R>
2 *

Здравствуйте, slavo, Вы писали:

S>Здравствуйте, CreatorCray, Вы писали:

CC>>__forceinline DWORD InterlockedExchange1 (DWORD &value, DWORD data)

S>Судя по __forceinline этот код не компилялся g++. Возможно ли, что g++ потребует явного return?
Вижуалка и Intel выкидывают warning, который я у себя давлю
Вообще remark подкинул хорошую мысль — посмотри нет ли у g++ intrinsic варианта для этих функций
для тебя наверное это было бы лучшим вариантом

Здравствуйте, slavo, Вы писали:

S>День добрый, необходимо написать свою реализацию InterlockedExchange, корректен ли такой код?

Тут уже советовали посмотреть в отладчике как это сделано в Windows. По-моему видел такую штуку: реализация Interlocked* отличается в одно- и мульти-процессорных вариантах ОС, но проверить сейчас не могу — под рукой нет мультипроц машины. Например LOCK — это очень накладно, а на однопроц системе вызывать не имеет смысла.

Если остро стоит проблема производительности, вообще имеет смысл генерировать два бинарника — для одно- и мультипроц соответственно. Или вывести все такие функции в DLL и иметь два варианта этого DLL.

(Сейчас еще вспомил о существовании мультикор процессоров, и не имею понятия как там все устроено. Надо бы почитать!)

Здравствуйте, slavo, Вы писали:

S>День добрый, необходимо написать свою реализацию InterlockedExchange, корректен ли такой код?

посмтри здесь — у тебя крыша съедет
http://www.audiomulch.com/~rossb/code/lockfree/ATOMIC.H

Здравствуйте, CreatorCray, Вы писали:

CC>Здравствуйте, slavo, Вы писали:

S>>Здравствуйте, CreatorCray, Вы писали:

CC>>>__forceinline DWORD InterlockedExchange1 (DWORD &value, DWORD data)

S>>Судя по __forceinline этот код не компилялся g++. Возможно ли, что g++ потребует явного return?
CC>Вижуалка и Intel выкидывают warning, который я у себя давлю
CC>Вообще remark подкинул хорошую мысль — посмотри нет ли у g++ intrinsic варианта для этих функций
CC>для тебя наверное это было бы лучшим вариантом

Built-in functions for atomic memory access

Здравствуйте, CreatorCray, Вы писали:

CC>Здравствуйте, remark, Вы писали:

R>>Но теория не говорит, что делает сгенерированный компилятором блок выхода из функции.
CC>ничего, если в функции нет локальных переменных

А положить в eax возвращаемое значение?
Если функция не __declspec(naked), то компилятор пытается (т.к. должен!) сувать что-то в eax, что он опознал как возвращаемое значение. В обычной функции он ориентируется по стейтментам return. А тут?


R>>Их надо использовать вместо WinAPI. Они встраиваются оптимальным образом, как на асме руками ты просто не можешь написать.
CC>Это все хорошо, но Microsoft specific и для ICC их нет.

Там тоже должно быть что-то аналогичное. Я уверен.

CC>Мой вариант от них отличается только тем, что параметры не встраиваются.

Нет, не только.
А имена регистров под операнды, в твоей функции кто выбрал? А где гарантия, что выбранные тобой имена регистров, обеспечивают оптимальное использование регистров в объемлющей функции?


CC>Да и кажется мне, что твой код встроился только потому, что был распознан как intrinsic — попробуй у себя функцию самописную назвать по-другому

Она у меня называлась singlecore_cas


CC>>>Кроме того, компилятор не имеет права "оптимизировать" тот ASM что написал программер.
R>>Но встроить он может.
CC>Если для этого ему надо менять ASM код — то не может.

CC>>>То, что твоя MSVC лезет куда ее не просят — это ИМХО баг.
R>>А он асм не трогает, он его только встраивает.
CC>он у тя убрал асм инструкцию, написанную руками.

Могу только сказать, что когда я сделал всё честно, то всё замечательно встроилось и никаких глюков не было. Если он что-то и менял, то очень грамотно.


CC>>>Разумеется
R>>
CC>2 *
4 *

Здравствуйте, crontab, Вы писали:

C>Здравствуйте, slavo, Вы писали:

S>>День добрый, необходимо написать свою реализацию InterlockedExchange, корректен ли такой код?

C>Тут уже советовали посмотреть в отладчике как это сделано в Windows. По-моему видел такую штуку: реализация Interlocked* отличается в одно- и мульти-процессорных вариантах ОС, но проверить сейчас не могу — под рукой нет мультипроц машины.

У них отличается в Win2k и WinXP. Причём на одноядерной Win2k у меня без LOCK, а на одноядерной WinXP уже с LOCK.

C>Например LOCK — это очень накладно, а на однопроц системе вызывать не имеет смысла.

Это зависит. AMD практически не налагает никакого пенальти.


C>Если остро стоит проблема производительности, вообще имеет смысл генерировать два бинарника — для одно- и мультипроц соответственно. Или вывести все такие функции в DLL и иметь два варианта этого DLL.

Это не всегда возможно, т.к. если надо распространять бинарник, то не известно, где он будет запущен.
В ACE применяется следующий трюк.
Имеется набор указателей на функции, при старте приложения он проверяет, сколько процессоров на машине, и устанавливает указатели соотв. либо на функции с LOCK, либо без LOCK.

Здравствуйте, remark, Вы писали:

R>А положить в eax возвращаемое значение?
R>Если функция не __declspec(naked), то компилятор пытается (т.к. должен!) сувать что-то в eax, что он опознал как возвращаемое значение. В обычной функции он ориентируется по стейтментам return. А тут?
Не должен и соответственно не пытается.

R>>>Их надо использовать вместо WinAPI. Они встраиваются оптимальным образом, как на асме руками ты просто не можешь написать.
CC>>Это все хорошо, но Microsoft specific и для ICC их нет.
R>Там тоже должно быть что-то аналогичное. Я уверен.
Нету — факт. Сам удивился.

R>А имена регистров под операнды, в твоей функции кто выбрал?
Я

R> А где гарантия, что выбранные тобой имена регистров, обеспечивают оптимальное использование регистров в объемлющей функции?
Гарантия — правила встраивания ICC асм блоков. При встраивании асма он проводит анализ задействованых в нем регистров и с учетом этого строит обвязку.

CC>>>>Разумеется
R>>>
CC>>2 *
R>4 *
гм. 8 * и орешки с фисташками

Здравствуйте, CreatorCray, Вы писали:

CC>Здравствуйте, remark, Вы писали:

R>>А положить в eax возвращаемое значение?
R>>Если функция не __declspec(naked), то компилятор пытается (т.к. должен!) сувать что-то в eax, что он опознал как возвращаемое значение. В обычной функции он ориентируется по стейтментам return. А тут?
CC> Не должен и соответственно не пытается.

Ну а как же тогда возвращаемое значение попадает в eax?



R>> А где гарантия, что выбранные тобой имена регистров, обеспечивают оптимальное использование регистров в объемлющей функции?
CC>Гарантия — правила встраивания ICC асм блоков. При встраивании асма он проводит анализ задействованых в нем регистров и с учетом этого строит обвязку.

Имхо, очевидно, что если и имена регистров в самой функции мог бы выбрать компилятор, то он мог бы сгенерировать более оптимальный код.
А тут ему приходится подстраиваться под данность.


CC>>>>>Разумеется
R>>>>
CC>>>2 *
R>>4 *
CC>гм. 8 * и орешки с фисташками

Здравствуйте, remark, Вы писали:

R>>>А положить в eax возвращаемое значение?
R>>>Если функция не __declspec(naked), то компилятор пытается (т.к. должен!) сувать что-то в eax, что он опознал как возвращаемое значение. В обычной функции он ориентируется по стейтментам return. А тут?
CC>> Не должен и соответственно не пытается.
R>Ну а как же тогда возвращаемое значение попадает в eax?
код для этого генерируется при компиляции return, но никак не блок выхода.
Впрочем ты наверное под блоком выхода понимаешь что то иное чем я. Для меня блок выхода из функции в общем виде выглядит так:

  add       esp, ?
  pop       ebp
  ret       ?

R>>> А где гарантия, что выбранные тобой имена регистров, обеспечивают оптимальное использование регистров в объемлющей функции?
CC>>Гарантия — правила встраивания ICC асм блоков. При встраивании асма он проводит анализ задействованых в нем регистров и с учетом этого строит обвязку.
R>Имхо, очевидно, что если и имена регистров в самой функции мог бы выбрать компилятор, то он мог бы сгенерировать более оптимальный код.
Кому очевидно? Мне например ничуть не очевидно. Тем более сомнения мои в том, что компилер будучи свободен в выборе регистров в данном конкретном случае сгенерил бы более оптимальный код, подтверждены практикой...

R>
На этой лирической ноте предлагаю закончить

Здравствуйте, Awaken, Вы писали:

A>Здравствуйте, slavo, Вы писали:

S>>День добрый, необходимо написать свою реализацию InterlockedExchange, корректен ли такой код?

A>посмтри здесь — у тебя крыша съедет
A>http://www.audiomulch.com/~rossb/code/lockfree/ATOMIC.H


Когда я последний раз смотрел, там были баги. В самом интересном классе AtomicCountedPtr.
Не знаю, может сейчас что-нибудь изменилось, это было 2 года назад.

Лучше смотрите это:
atomic-ptr-plus
Там, я правда, к своему большому удивлению, тоже обнаружил пару багов...

R>Когда я последний раз смотрел, там были баги. В самом интересном классе AtomicCountedPtr.
R>Не знаю, может сейчас что-нибудь изменилось, это было 2 года назад.

я тоже смотрел ее давно, и похоже ничего не изменилось. или автор потерял интерес к своему детищу

R>atomic-ptr-plus
R>Там, я правда, к своему большому удивлению, тоже обнаружил пару багов...
+1
спасибо, гляну