[armel] fio fails to build with ARMv7 optimizations

In the process of fixing this for ARMv7 I have added the correct calls to ensure SMP safety.

Tested on iMX51 ubuntu-netbook form 05/03/10

Of course that should of read "Tested on the iMX51 ubuntu-netbook image from 05/03/10"

I have a concern about the proposed patch: if the code is ever built for an arch > 7-a (in the future) the wrong code will silently be built.

Is it not feasible to use configure tests to determine whether asm("nop") and __sync_* are supported? This is more future-proof...

As long as __ARCH_ARM_7A__ isn't defined the build will fail as the defines will not be declared.

New arch's will have to be added to the file in the future which is how the packages does it now. We can investigate a better fix if desired.

Note that this package doesn't use configure so we are left with some kind of Makefile or debian/rules solution.

i agree, that in the long run we might need something better; we can carry that forward upstream when pushing our patch there though. For now in ubuntu the patch jamie proposed seems fine.

Attaching .debdiff

OK, fair enough; I hadn't looked at the ifdefs preceding the patch.

Note that ARMv6 is missing: it may be a good idea to add some extra checks:

#ifdef defined(__ARM_ARCH_4__) || defined(__ARM_ARCH_4T__) || defined(__ARM_ARCH_5__) defined(__ARM_ARCH_5T__) || defined(__ARM_ARCH_5TE__) || defined(__ARM_ARCH_5TEJ__) || defined(__ARM_ARCH_6__) || defined(__ARM_ARCH_6J__) || defined(__ARM_ARCH_6ZK__) || defined(__ARM_ARCH_6T2__)
    /* old definitions */
#elif defined(__ARM_ARCH_6K__) || defined(__ARM_ARCH_7A__)
   /* new, GCC intrinsics based definitions and native "nop" instruction */
#endif

Alternatively, the attached mini configure script might be OK instead. If it looks useful, let me know and I'll add it to the wiki

This bug was fixed in the package fio - 1.33.1-1ubuntu1

---------------
fio (1.33.1-1ubuntu1) lucid; urgency=low

  * arch/arch-arm.h: Fix fail to build issue on ARMv7 and make it SMP
    safe (LP: #532722).
 -- Jamie Bennett <email address hidden> Thu, 04 Mar 2010 20:38:38 +0000

Dave, i think adding that script to wiki would be helpful. Together with an example macro that shows how to use it for folks not familiar with m4 etc.

If you could add that jamie would probably be happy to update his patch to do it that way.

Added more architecture defines as suggested by Dave Martin.

Fixed fail to build on ia64.

Probably going to change this fix for the configure script Dave Martin suggested, seems the more sensible approach.

My configure program Is just a shell script btw: comments at the start of the script explain how to use it.

Hopefully this is a reasonable approach when a package hasn't moved to a heavyweight autotools-based flow yet, but I'm open to suggestions...

Some small suggestions on the shell script:
- mktemp /tmp/"$me-XXXXXX" => mktemp -t "$me-XXXXXXXX"
- instead of echo -n + final echo, why not just accumulate defines in a string which you output? e.g. DEFINES="", DEFINES="$DEFINES -DFOO" and echo "CFLAGS += $DEFINES"
- you could eval this from make instead of generating an intermediate make, but that's a bit harder to read I guess (avoids the chicken and egg between the include being missing before it's generated though)
- I personally do the tests directly in make, but you could also consider autoconf itself I guess

Thanks!

> Some small suggestions on the shell script:
> - mktemp /tmp/"$me-XXXXXX" => mktemp -t "$me-XXXXXXXX"

Hmmm, I learn something every day :) However, man mktemp says -t is
deprecated.

But mktemp -p /tmp "$me-XXXXXXXX" would work equally well.

> - instead of echo -n + final echo, why not just accumulate
> defines in a string which you output? e.g. DEFINES="",
> DEFINES="$DEFINES -DFOO" and echo "CFLAGS += $DEFINES"

Sure, that's a bit nicer.

> - you could eval this from make instead of generating an
> intermediate make, but that's a bit harder to read I guess
> (avoids the chicken and egg between the include being missing
> before it's generated though)

Sure, although the tests will get re-run every time you type "make" that
way, which might annoy some people.

I am looking into generating some more useful predefines for the information
suppiled by __ARM_ARCH_* --- this is more cumbersome though and really needs
to be a separate script, but it has potential to simplify all the #if
defined(this) || defined(that) || defined(the other) || ... and reduce the
maintenance effort.

> - I personally do the tests directly in make, but you could
> also consider autoconf itself I guess

Sure. I'm still too scared of autoconf to set it up from scratch though :O