Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_stack fail on E2K in 64-bit mode #59

Closed
ivmai opened this issue Oct 23, 2023 · 11 comments
Closed

test_stack fail on E2K in 64-bit mode #59

ivmai opened this issue Oct 23, 2023 · 11 comments

Comments

@ivmai
Copy link
Owner

ivmai commented Oct 23, 2023

Source: master (30db756)
Host: Linux ... 5.4.0-6.9-e8c #1 SMP ... e2k E8C E8C-SWTX GNU/Linux
Compiler: lcc:1.26.20:Jun--6-2023:e2k-v4-linux (gcc (GCC) 9.3.0 compatible)
How to reproduce: ./autogen.sh && ./configure && make check CFLAGS_EXTRA="-m64 -D AO_PREFER_BUILTIN_ATOMICS" LDFLAGS="-latomic"
Output (tests/test_stack.log):
Failed - nothing to pop

@ilyakurdyukov
Copy link

На v4 нет 128-bit атомарных операций, для v5 и далее - есть, так в МЦСТ ответили и баг закрыли. Версию архитектуры можно узнать на этапе компиляции по значению макроса __iset__.

@ivmai
Copy link
Owner Author

ivmai commented Oct 24, 2023

Tip for me: code of libatomic - https://github.com/OpenE2K/gcc/tree/gcc-runtime-mcst/libatomic

@ivmai
Copy link
Owner Author

ivmai commented Oct 24, 2023

На v4 нет 128-bit атомарных операций, для v5 и далее - есть, так в МЦСТ ответили и баг закрыли. Версию архитектуры можно узнать на этапе компиляции по значению макроса __iset__.

Got it. Thank you! I will do a compile-time check based on the macro.

@ivmai
Copy link
Owner Author

ivmai commented Oct 24, 2023

Note: there was also a suggestion to use a runtime check (`__builtin_cpu_arch()') but according to the design of libatomic_ops availability of a particular atomic operation should be detectable thru a macro.

@ilyakurdyukov
Copy link

И еще добавили, что в v4 есть атомарные 128-бит запись/чтение, это для указателей 128-бит режима. Но нет эквивалента exchange/compare_exchange на 128 бит.

@ivmai
Copy link
Owner Author

ivmai commented Oct 24, 2023

И еще добавили, что в v4 есть атомарные 128-бит запись/чтение, это для указателей 128-бит режима. Но нет эквивалента exchange/compare_exchange на 128 бит.

О, спасибо, это понадобится когда буду добавлять поддержку ЗР в libgc.

@ilyakurdyukov
Copy link

ilyakurdyukov commented Oct 26, 2023

Предлагают вместо __iset__ использовать проверку на макрос __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 для определения наличия интринсика в компиляторе, для Clang тоже работает.

@ivmai
Copy link
Owner Author

ivmai commented Oct 26, 2023

Предлагают вместо __iset__ использовать проверку на макрос __GCC_HAVE_SYNC_COMPARE_AND_SWAP_16 для определения наличия интринсика в компиляторе, для Clang тоже работает.

I agree that about gcc your suggestion is good, but not for clang: clang -m64 -dM -E ... | grep GCC_HAVE_SYNC gives nothing.
But I will use it at least for gcc. Thank you!

@ilyakurdyukov
Copy link

Для Clang-9 на x86_64 это работает (https://godbolt.org/z/P7Gqoa3qM):

#ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8
#error
#endif

На 16 байт у x86_64 нет.

В Clang от МЦСТ интринсики вроде есть, а макроса нет. На это баг уже завели.

Но это был совет от МЦСТ с этим макросом, я бы предпочёл использовать __iset__, чем непонятные макросы от компилятора.

@ivmai
Copy link
Owner Author

ivmai commented Oct 28, 2023

Для Clang-9 на x86_64 это работает (https://godbolt.org/z/P7Gqoa3qM):
На 16 байт у x86_64 нет.

Да, интринсик есть для 16 байт а макроса нет. У Clang.

@ivmai
Copy link
Owner Author

ivmai commented Oct 28, 2023

В Clang от МЦСТ интринсики вроде есть, а макроса нет. На это баг уже завели.

There 2 extra issues with clang 16-byte compare and exchange (I mean difference from gcc) besides absence of the feature macro:

src/atomic_ops/sysdeps/gcc/generic.h:227:19: warning: large atomic operation may incur significant performance penalty; the access size (16 bytes) exceeds the max lock-free size (8  bytes) [-Watomic-alignment]
      return (int)__atomic_compare_exchange_n(&addr->AO_whole,
                  ^
1 warning generated.
/usr/bin/ld: /tmp/atomic_ops_stack-eb3351.o: в функции «AO_stack_pop_acquire»:
-:(.text+0x26c): неопределённая ссылка на «__atomic_compare_exchange»
/usr/bin/ld: -:(.text+0x36c): неопределённая ссылка на «__atomic_compare_exchange»
clang-13: error: linker command failed with exit code 1 (use -v to see invocation)

As a workaround this could be used by client: CFLAGS=-Wno-atomic-alignment LDFLAGS=-latomic

ivmai added a commit that referenced this issue Oct 30, 2023
(supersede commits ca5d76e, e744433)

Issue #59 (libatomic_ops)

This requires CPU elbrus-v5 or later.

* src/atomic_ops/sysdeps/gcc/e2k.h: Replace defined(__ptr32__) to
__SIZEOF_SIZE_T__==4.
* src/atomic_ops/sysdeps/gcc/e2k.h [__GCC_HAVE_SYNC_COMPARE_AND_SWAP_16
&& !__clang__]: Define AO_GCC_HAVE_double_SYNC_CAS and include
standard_ao_double_t.h.
* src/atomic_ops/sysdeps/gcc/e2k.h [!__ptr32__
&& AO_PREFER_BUILTIN_ATOMICS]: Define AO_GCC_HAVE_double_SYNC_CAS and
include standard_ao_double_t.h only if __clang__ and __iset__>=5;
add comment; remove TODO item.
@ivmai ivmai closed this as completed Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants