purduesigbots / pros

Source code for PROS kernel: open source C/C++ development for the VEX V5 microcontroller
https://pros.cs.purdue.edu
Other
262 stars 76 forks source link

Configure pros to allow neon support #274

Closed nathan-moore closed 2 years ago

nathan-moore commented 3 years ago

Expected Behavior:

Code using NEON to compile, as, according to the data sheet, it is supported.

Actual Behavior:

Code doesn't compile.

HotelCalifornia commented 3 years ago

Do we have a minimal example of code that doesn't compile? Could it be as simple as changing the floating point ABI from soft to hard?

nathan-moore commented 3 years ago

Daniel was getting a failure with int8x8_t, which leads to this nice gcc page: https://gcc.gnu.org/onlinedocs/gcc-4.4.1/gcc/ARM-NEON-Intrinsics.html.

Do you know why we're using the soft abi? I remember checking in the past and believe that floats and doubles were being passed in a fpu reg anyway. We probably should wait till a major revision to change that though.

I expect it to be orthogonal to that, as it covers more than floating point operations.

HotelCalifornia commented 3 years ago

Do you know why we're using the soft abi?

I think we decided on it after looking at vexos docs or something. Since we've confirmed that there is hard floating point available, might as well switch that yep

HotelCalifornia commented 3 years ago

Daniel was getting a failure with int8x8_t

yeah I know, but he was using eigen. a minimal example of something that uses neon integer types would be useful for verifying any fixes that come up in response to this

nathan-moore commented 3 years ago

A minimal repo is:

#include <arm_neon.h>

int8x8_t Test2(int8x8_t test, int8x8_t test2)
{
    return vadd_s8(test, test2);
}

Though you might also want to check out types of different widths to make sure it's all supported.

edjubuh commented 3 years ago

Do you know why we're using the soft abi?

I think we decided on it after looking at vexos docs or something. Since we've confirmed that there is hard floating point available, might as well switch that yep

We shouldn't need to switch to hard float abi. Hard ABI would cause compile conflicts with libv5rts which is compiled with one of the soft ABIs.

We compile with these flags: https://github.com/purduesigbots/pros/blob/develop/common.mk#L4

-mfpu=neon-fp16 -mfloat-abi=softfp

From GCC documentation:

Specifying soft causes GCC to generate output containing library calls for floating-point operations. softfp allows the generation of code using hardware floating-point instructions, but still uses the soft-float calling conventions. hard allows generation of floating-point instructions and uses FPU-specific calling conventions.

HotelCalifornia commented 3 years ago

so it sounds like this may be a non-issue... again having a minimal repro case would be helpful in tracking down what exactly is going wrong. simply including <arm_neon.h> allowed me to define an int8x8_t with no issues.

Richard-Stump commented 2 years ago

Could the issue with this be due to the v5 runtime? If the v5 runtime uses the soft abi, could we the project to use the soft abi for the vex api, but the NEON abi for everything else? https://gcc.gnu.org/onlinedocs/gcc/ARM-Function-Attributes.html#ARM-Function-Attributes

nathan-moore commented 2 years ago

Maybe? I wouldn't expect the ABI to impact which instructions are capable of being emitted, though NEON abi would likely make floating code a bit faster. I'm just going to close this though as its likely I misunderstood something.