Skip to content

Commit

Permalink
Revert "[compiler-rt] Implement ARM atomic operations for architectur…
Browse files Browse the repository at this point in the history
…es without SMP support"

This reverts commit 910a642.

There are serious correctness issues with the current approach: __sync_*
routines which are not actually atomic should not be enabled by default.

I'll continue discussion on the review.
  • Loading branch information
efriedma-quic committed Feb 17, 2022
1 parent d20e01b commit 0389f2e
Show file tree
Hide file tree
Showing 4 changed files with 0 additions and 49 deletions.
10 changes: 0 additions & 10 deletions compiler-rt/cmake/Modules/CompilerRTUtils.cmake
Expand Up @@ -110,16 +110,6 @@ function(check_compile_definition def argstring out_var)
cmake_pop_check_state()
endfunction()

macro(test_arm_smp_support arch cflags_var)
if (${arch} STREQUAL "arm")
try_compile(HAS_${arch}_SMP ${CMAKE_BINARY_DIR}
${ARM_SMP_CHECK_SRC} COMPILE_DEFINITIONS "${CMAKE_C_FLAGS} ${_TARGET_${arch}_CFLAGS}")
if (HAS_${arch}_SMP)
list(APPEND ${cflags_var} -DCOMPILER_RT_HAS_SMP_SUPPORT)
endif()
endif()
endmacro()

# test_target_arch(<arch> <def> <target flags...>)
# Checks if architecture is supported: runs host compiler with provided
# flags to verify that:
Expand Down
5 changes: 0 additions & 5 deletions compiler-rt/cmake/config-ix.cmake
Expand Up @@ -200,11 +200,6 @@ set(COMPILER_RT_SUPPORTED_ARCH)
set(SIMPLE_SOURCE ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/simple.cc)
file(WRITE ${SIMPLE_SOURCE} "#include <stdlib.h>\n#include <stdio.h>\nint main() { printf(\"hello, world\"); }\n")

# Check if we have SMP support for particular ARM architecture
# If not use stubs instead of real atomic operations - see sync-ops.h
set(ARM_SMP_CHECK_SRC ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/arm-barrier.cc)
file(WRITE ${ARM_SMP_CHECK_SRC} "int main() { asm(\"dmb\"); return 0; }")

# Detect whether the current target platform is 32-bit or 64-bit, and setup
# the correct commandline flags needed to attempt to target 32-bit and 64-bit.
if (NOT CMAKE_SIZEOF_VOID_P EQUAL 4 AND
Expand Down
1 change: 0 additions & 1 deletion compiler-rt/lib/builtins/CMakeLists.txt
Expand Up @@ -740,7 +740,6 @@ else ()
list(APPEND BUILTIN_CFLAGS_${arch} -fomit-frame-pointer -DCOMPILER_RT_ARMHF_TARGET)
endif()

test_arm_smp_support(${arch} BUILTIN_CFLAGS_${arch})
# For RISCV32, we must force enable int128 for compiling long
# double routines.
if("${arch}" STREQUAL "riscv32")
Expand Down
33 changes: 0 additions & 33 deletions compiler-rt/lib/builtins/arm/sync-ops.h
Expand Up @@ -14,8 +14,6 @@

#include "../assembly.h"

#ifdef COMPILER_RT_HAS_SMP_SUPPORT

#define SYNC_OP_4(op) \
.p2align 2; \
.thumb; \
Expand Down Expand Up @@ -47,37 +45,6 @@
dmb; \
pop { r4, r5, r6, pc }

#else

#define SYNC_OP_4(op) \
.p2align 2; \
DEFINE_COMPILERRT_THUMB_FUNCTION(__sync_fetch_and_##op) \
LOCAL_LABEL(tryatomic_##op) : \
mov r12, r0; \
op(r2, r0, r1); \
str r2, [r12]; \
ldr r12, [r12]; \
cmp r12, r2; \
bne LOCAL_LABEL(tryatomic_##op); \
bx lr

#define SYNC_OP_8(op) \
.p2align 2; \
DEFINE_COMPILERRT_THUMB_FUNCTION(__sync_fetch_and_##op) \
push {r4, r5, r6, lr}; \
LOCAL_LABEL(tryatomic_##op) : \
mov r12, r0; \
op(r4, r5, r0, r1, r2, r3); \
stm r12, {r4, r5}; \
ldm r12, {r6, r12}; \
cmp r6, r4; \
bne LOCAL_LABEL(tryatomic_##op); \
cmp r12, r5; \
bne LOCAL_LABEL(tryatomic_##op); \
pop { r4, r5, r6, pc }

#endif

#define MINMAX_4(rD, rN, rM, cmp_kind) \
cmp rN, rM; \
mov rD, rM; \
Expand Down

0 comments on commit 0389f2e

Please sign in to comment.