Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

References to F64 in CmdDispatcher on a platform that doesn't support F64 #3390

Open
celskeggs opened this issue Mar 17, 2025 · 8 comments
Open
Assignees
Labels
bug FPP Issues related to FPP needs investigation Issue needs further investigation

Comments

@celskeggs
Copy link
Contributor

F´ Version v3.6.1
Affected Component FPP autocoders

Problem Description

I'm using F Prime on a platform that doesn't have support for 64-bit floats, so the F64 type is not defined. This has been working fine, but I updated to the latest devel commit earlier today, and I encountered this error while compiling autocode:

[build] [321/768] Building CXX object F-Prime/Svc/CmdDispatcher/CMakeFiles/Svc_CmdDispatcher.dir/CommandDispatcherComponentAc.cpp.obj
[build] FAILED: F-Prime/Svc/CmdDispatcher/CMakeFiles/Svc_CmdDispatcher.dir/CommandDispatcherComponentAc.cpp.obj 
[build] /usr/local/bin/clang-19 -DFW_BAREMETAL_SCHEDULER=0 -I/workspaces/[...]/cmake/platform/types -I/workspaces/[...]/lib/fprime -I/workspaces/[...]/config -I/workspaces/[...] -I/workspaces/[...]/build-fprime-automatic-[...] -I/workspaces/[...]/build-fprime-automatic-[...]/F-Prime -I/workspaces/[...]/build-fprime-automatic-[...]/config -I/workspaces/[...] -I/workspaces/[...]/lib/fprime-vorago [... other flags ...] -std=c++14 -DASSERT_RELATIVE_PATH='"build-fprime-automatic-[...]/F-Prime/Svc/CmdDispatcher/CommandDispatcherComponentAc.cpp"' -MD -MT F-Prime/Svc/CmdDispatcher/CMakeFiles/Svc_CmdDispatcher.dir/CommandDispatcherComponentAc.cpp.obj -MF F-Prime/Svc/CmdDispatcher/CMakeFiles/Svc_CmdDispatcher.dir/CommandDispatcherComponentAc.cpp.obj.d -o F-Prime/Svc/CmdDispatcher/CMakeFiles/Svc_CmdDispatcher.dir/CommandDispatcherComponentAc.cpp.obj -c /workspaces/[...]/build-fprime-automatic-[...]/F-Prime/Svc/CmdDispatcher/CommandDispatcherComponentAc.cpp
[build] /workspaces/[...]/build-fprime-automatic-[...]/F-Prime/Svc/CmdDispatcher/CommandDispatcherComponentAc.cpp:2542:21: error: unknown type name 'F64'
[build]  2542 |         static_cast<F64>(arg2),
[build]       |                     ^
[build] 1 error generated.
[build] ninja: build stopped: subcommand failed.
[proc] The command: /usr/bin/cmake --build /workspaces/[...]/build-fprime-automatic-[...] --parallel 6 -- exited with code: 1
[driver] Build completed: 00:00:18.310
[build] Build finished with exit code 1

For reference, here is the command that causes this problem:

    @ No-op command
    async command CMD_TEST_CMD_1(
                                  arg1: I32 @< The I32 command argument
                                  arg2: F32 @< The F32 command argument
                                  arg3: U8 @< The U8 command argument
                                ) \
      opcode 2

You'll note that it does not reference F64, but rather F32.

Context / Environment

Execute fprime-util version-check and share the output.

Operating System: Linux
CPU Architecture: x86_64
Platform: Linux-4.18.0-553.34.1.el8_10.x86_64-x86_64-with-glibc2.34
Python version: 3.9.21
CMake version: 3.26.5
Pip version: 21.3.1
Pip packages:
    fprime-tools==3.6.1
    fprime-gds==3.6.1
    fprime-fpp-*==3.0.0a2
Project submodules:
    https://github.com/nasa/fprime.git @ v3.6.0-40-g58f4f9691

How to Reproduce

  1. Build built in F Prime components on a platform without F64 support.
  2. Observe error.

Expected Behavior

The FPP autocoder should not substitute F64s for F32s on platforms that do not support F64s.

@celskeggs celskeggs added the bug label Mar 17, 2025
@bocchino
Copy link
Collaborator

It looks like this is a regression introduced by #3319. The issue is that on platforms that do support F64, an implicit promotion from F32 to F64 can cause a warning. Perhaps we should define a type that is F64 on platforms that support F64, otherwise F32, and cast to that.

@bocchino bocchino added the FPP Issues related to FPP label Mar 17, 2025
@bocchino
Copy link
Collaborator

The FPP autocoder should not substitute F64s for F32s on platforms that do not support F64s.

Can you clarify what is going on with this platform? The type double is part of the C++ language standard. What happens if you try to use the double type on this platform? Is the issue that F64 is not defined to be double, or that double itself is unsupported?

@bocchino bocchino added the needs investigation Issue needs further investigation label Mar 19, 2025
@jwest115 jwest115 self-assigned this Mar 19, 2025
@celskeggs
Copy link
Contributor Author

celskeggs commented Mar 19, 2025

I did a little more digging. I inherited the configuration (including HAS_F64 unset) from another engineer, so I'm not familiar with the rationale behind the decision to unset HAS_F64. It looks like double is supported on my platform but only as a soft double, so it will be significantly slower than floats. So I certainly don't want anything to be using doubles if I can help it.

Of course, I can confirm that calling float f = [...]; printf("%f\n", f); on my platform results in a soft conversion from float to double, so it looks like I have an implicit promotion whether I want one or not.

I wonder whether this is a sign that the HAS_F64 flag shouldn't exist. After all, if all platforms supported by C++ are required to have double support (at least soft doubles), why do we have that flag in the first place?

@timcanham
Copy link
Collaborator

It was mainly for deeply embedded targets that didn't have any FP support. We (I) wanted to be able to turn it off to get compiles to work. The caveat was that certain components wouldn't be portable to that environment if they required F64.

@bocchino
Copy link
Collaborator

All environments that support C++ should provide at least soft F64, because double is part of the C++ language standard. It's different for integer types, where the standard does not require any particular bit width.

@LeStarch
Copy link
Collaborator

Implicit float to double promotion is part of C++, as is the type double. We provide F32 and F64 to help users avoid soft float support, however; there are some functions that (*printf) where promotion will happen regardless.

In future versions of F´ we will be removing the HAS_F64 flag because every platform has double defined. Thus the flag can only cause false understanding of the code, and cause breakages when calling into code that will automatically promote double.

@jwest115
Copy link
Collaborator

@LeStarch is this addressed by some of the work you are doing related to #3202? If not, shall I get a PR ready for removing HAS_F64?

@LeStarch
Copy link
Collaborator

LeStarch commented Apr 1, 2025

@jwest115 I am removing HAS_F64 from the hand code, are there references in the auto code?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug FPP Issues related to FPP needs investigation Issue needs further investigation
Projects
None yet
Development

No branches or pull requests

5 participants