armv6: tests/atomicops_unittest.cc fails to compile due to 'dmb' in 64bit Barrier_AtomicIncrement

[GoogleCodeExporter]

What steps will reproduce the problem?
1. clone repo on a raspberry pi
2. Get current HEAD (a15115271cc475509b17bf7fecbe1ac4966baf2e)
3. compile on the pi

What is the expected output? What do you see instead?

Expected it to compile, at least since issue #493 was fixed 
(beb78cc05babf0a49d21aed0ec789f19fc0f2d28) back in March 2013.

Instead the code does not compile.

What version of the product are you using? On what operating system?

Current master (a15115271cc475509b17bf7fecbe1ac4966baf2e)

Please provide any additional information below.

After looking throughout the commit log, it looks a lot like this is due to the 
patch fixing issue #490 (b591d53af951eac60683237204464ebfec2c3afa), which was 
committed to master soon before #493's patch.  This patch does not introduce 
any new feature I can ascertain, but does make use of Barrier_AtomicIncrement, 
which was not fixed by beb78cc05babf0a49d21aed0ec789f19fc0f2d28 -- my guess, 
this was tested prior to Barrier_AtomicIncrement being used anywhere in the 
source.

Unfortunately I'm not versed in ASM (much less in the ARM instruction set) to 
be able to confidently fix this. I hope this is not much of a pain to fix -- if 
I were to do it, I'd find a way to mimic 
beb78cc05babf0a49d21aed0ec789f19fc0f2d28 approach.

As a workaround, I simply made sure that most of atomicops_unittest.cc isn't 
compiled -- and that gets the lib compiling.

Hope this helps.

Cheers!

  -Joao

Original issue reported on code.google.com by joao.l...@inktank.com on 27 Dec 2013 at 2:08

[GoogleCodeExporter]

Unfortunately I don't have access to raspberry or any other arm 6.

But the code looks more or less correct. It's detecting arm6 and uses 
alternative to dmb barrier instruction.

If it fails at exactly this place then we simply need to find out why we fail 
to detect arm6-ness of your machine.

Can you add output of:

gcc -dM -E - < /dev/null

? So that we can find right defines ?

Original comment by alkondratenko on 30 Dec 2013 at 7:20

[GoogleCodeExporter]

Hello,

AFAICT, only MemoryBarrier() has the appropriate code to decide whether 
to use the 'dmb' barrier instruction or not.

It does not fail where the alternative is set in place.  This is the 
function where the compilation fails:

inline Atomic64 Barrier_AtomicIncrement(volatile Atomic64* ptr,
                                         Atomic64 increment) {
   int store_failed;
   Atomic64 res;
   __asm__ __volatile__(
       "1:\n"
       "ldrexd  %1, [%2]\n"
       "adds    %Q1, %Q1, %Q3\n"
       "adc     %R1, %R1, %R3\n"
       "dmb\n"
       "strexd  %0, %1, [%2]\n"
       "teq     %0, #0\n"
       "bne     1b"
       : "=&r" (store_failed), "=&r"(res)
       : "r" (ptr), "r"(increment)
       : "cc", "memory");
   return res;
}

On that 'dmb' instruction in the middle.  More precisely, it fails 
whilst compiling src/tests/atomicops_unittest.cc, which makes use of the 
above function.

Compilation fails during

  g++ -DHAVE_CONFIG_H -I. -I./src  -I./src   -Wall -Wwrite-strings 
-Woverloaded-virtual -Wno-sign-compare -fno-builtin-malloc 
-fno-builtin-free -fno-builtin-realloc -fno-builtin-calloc 
-fno-builtin-cfree -fno-builtin-memalign -fno-builtin-posix_memalign 
-fno-builtin-valloc -fno-builtin-pvalloc  -Wno-unused-result 
-D__ARMCC__ -D__ARM_ARCH_6K__ -MT atomicops_unittest.o -MD -MP -MF 
.deps/atomicops_unittest.Tpo -c -o atomicops_unittest.o `test -f 
'src/tests/atomicops_unittest.cc' || echo 
'./'`src/tests/atomicops_unittest.cc

with

/tmp/ccKFB1a0.s: Assembler messages:
/tmp/ccKFB1a0.s:232: Error: selected processor does not support ARM mode 
`dmb'

Furthermore, please note that the -D__ARMCC__ and -D__ARM_ARCH_6K__ have 
been manually supplied to C{XX,}FLAGS during configure and make.

I have attached the full output, but let me paste here the result of a 
'grep -i arm' on the result:

#define __ARMEL__ 1
#define __ARM_PCS_VFP 1
#define __ARM_ARCH_6__ 1
#define __arm__ 1
#define __ARM_EABI__ 1
#define __ARM_FEATURE_DSP 1

Let me know how I can help further.

   -Joao

Original comment by joao.l...@inktank.com on 30 Dec 2013 at 11:02

[GoogleCodeExporter]

Looks like attaching to the reply email didn't work so well (at least no 
attachment shows up as I'm currently checking the online version). So let me 
try attaching the file via google-code reply interface.

Also, while dumping the defines I forgot to mention what I assume could be 
useful information:

$ gcc --version
gcc (Debian 4.6.3-14+rpi1) 4.6.3
Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

  -Joao

Original comment by jecl...@gmail.com on 30 Dec 2013 at 11:06

Attachments:

• rpi-gcc-defines.txt

[GoogleCodeExporter]

Indeed. Thanks for clarifying. It's a fact that we don't really need 64-bit 
atomics on 32-bit platforms. I'll see what I can do.

Original comment by alkondratenko on 30 Dec 2013 at 8:01

• Changed title: armv6: tests/atomicops_unittest.cc fails to compile due to 'dmb' in 64bit Barrier_AtomicIncrement

[GoogleCodeExporter]

Thanks for the update!

  -Joao

Original comment by jecl...@gmail.com on 1 Jan 2014 at 11:13

[GoogleCodeExporter]

fix merged. thanks for reporting it.

Original comment by alkondratenko on 4 Jan 2014 at 10:52

• Changed state: Fixed

[GoogleCodeExporter]

Yep. Current master (4c274b9e20132230e62117ff583ebadd83081d90) successfully 
compiles on armv6. That ought to make me a happy pi owner. :)

Thanks!

  -Joao

Original comment by jecl...@gmail.com on 5 Jan 2014 at 1:16