ARM-software / CMSIS_5

CMSIS Version 5 Development Repository
http://arm-software.github.io/CMSIS_5/index.html
Apache License 2.0
1.34k stars 1.08k forks source link

Missing #define in arm_math.h? #148

Closed rogersnm closed 7 years ago

rogersnm commented 7 years ago

In arm_math.h (https://github.com/ARM-software/CMSIS_5/blob/develop/CMSIS/DSP/Include/arm_math.h) lines 306-314 we have:

#if defined(ARM_MATH_CM7)
  #include "core_cm7.h"
  #define ARM_MATH_DSP
#elif defined (ARM_MATH_CM4)
  #include "core_cm4.h"
  #define ARM_MATH_DSP
#elif defined (ARM_MATH_CM3)
  #include "core_cm3.h"
#elif defined (ARM_MATH_CM0)

Is the Cortex-M3 missing the #define ARM_MATH_DSP statement?

In functions such as arm_radix4_butterfly_q15 (https://github.com/ARM-software/CMSIS_5/blob/develop/CMSIS/DSP/Source/TransformFunctions/arm_cfft_radix4_q15.c) we have:

#if defined (ARM_MATH_DSP)

  /* Run the below code for Cortex-M4 and Cortex-M3 */

However, contrary to the comment, the code below won't run on the Cortex-M3 as the flag will not be defined. Previous versions of the DSP lib (eg. 1.4.5) used a different flag in the ifdef that meant that the code would be executed on the Cortex-M3:

(https://github.com/ARM-software/CMSIS/blob/master/CMSIS/DSP_Lib/Source/TransformFunctions/arm_cfft_radix4_q15.c)

#ifndef ARM_MATH_CM0_FAMILY

  /* Run the below code for Cortex-M4 and Cortex-M3 */

Is this a bug or is this intentional?

ghost commented 7 years ago

Hello,

the intention of macro ARM_MATH_DSP is to indicate if SIMD instructions are available. If SIMD instructions are not available they are replaced by inline functions delivering the same result. As Cortex-M3 does not support SIMD instructions the missing define for Cortex-M3 is not a bug.

You are correct. In DSP V1.4.5 a different macro was used to selcect a 'more efficient' code for an algorithm. We need to check if the 'more efficient' code could also be used with the SIMD replacement functions and if the execution speed is faster or the same as the 'non effisient' code. If this is the case we could simplify the DSP functions.

Thank you for pointing out.

Best Regards, Martin

rogersnm commented 7 years ago

Hi Martin,

Thanks for pointing that out; due to the code comments I was under the impression that the Cortex-M3 had a basic SIMD unit but after checking the assembly you're correct that no SIMD instructions are generated.

I'm going to do some basic tests to see which one performs faster on the M3 (with my inputs), but it would be useful if you could independently confirm which branch is faster.

Thanks, Nick

ps. Also I guess my revised bug is that the code comments need to be updated to reflect that the below code is no longer run on the M3.

ReinhardKeil commented 7 years ago

I think this is clear now. Let me know if you have further questions.

Your comment "ps. Also I guess my revised bug is that the code comments need to be updated to reflect that the below code is no longer run on the M3." indicates that you have raise other issues. Is this the case? If so, how can I find it.

rogersnm commented 7 years ago

Hi Reinhard,

Sorry, I should have been more clear. I have just raised #153 that expands upon my ps., I am happy for this issue to be closed now.

Thanks, Nick