Skip to content

Remove uses_fp parameter from RTAPI and HAL APIs#3901

Open
grandixximo wants to merge 1 commit intoLinuxCNC:masterfrom
grandixximo:fix/remove-uses-fp-2.10
Open

Remove uses_fp parameter from RTAPI and HAL APIs#3901
grandixximo wants to merge 1 commit intoLinuxCNC:masterfrom
grandixximo:fix/remove-uses-fp-2.10

Conversation

@grandixximo
Copy link
Copy Markdown

Complete removal of the uses_fp parameter for 2.10. All threads now unconditionally save and restore FPU/SSE state.

RTAPI: remove uses_fp from rtapi_task_new(), remove RTAPI_NO_FP and RTAPI_USES_FP constants, hardcode FPU save in rt_task_init_cpuid.

HAL: remove uses_fp from hal_export_funct(), hal_export_functf(), hal_create_thread(). Remove uses_fp field from hal_funct_t and hal_thread_t structs. Remove addf FP compatibility check. Remove FP column from halcmd and halrmt display.

Components: remove uses_fp argument from all hal_export_funct and hal_export_functf call sites. Remove base_thread_fp from motion module, fp1/fp2/fp3 from threads component.

halcompile: remove OptFP grammar rule, fp/nofp parsing, and fp-related code generation and documentation output. Remove fp/nofp from all in-tree .comp files, conv.comp.in and mkconv.sh.

Documentation: remove uses_fp from API man pages, remove FP thread references from tutorials and guides.

Out-of-tree components must be updated to the new API signatures.

Ref: #3895

@grandixximo
Copy link
Copy Markdown
Author

Hopefully I did not miss any...

@BsAtHome
Copy link
Copy Markdown
Contributor

BsAtHome commented Apr 3, 2026

You should not remove the OptFP rule in halcompile just yet. Instead you should add the warning from my patch (and always return 1). That would give people an indication what is actually happening and gives them time to fix their code. The next release beyond 2.10 we can remove the rule completely.

It is not clear whether it is appropriate to change the HAL API in 2.10. It feels as too much too crude too soon. It is more appropriate to warn users appropriately that nofp no longer works, is deprecated and will be removed in the future. We need give people some grace period to fix their stuff, just like in halcompile.

@grandixximo
Copy link
Copy Markdown
Author

what about #3286
If that is planned for 2.10 might as well no?

@grandixximo grandixximo force-pushed the fix/remove-uses-fp-2.10 branch from 9486d25 to a02aacf Compare April 3, 2026 12:33
@andypugh
Copy link
Copy Markdown
Collaborator

andypugh commented Apr 4, 2026

what about #3286 If that is planned for 2.10 might as well no?

I am not sure it still is, but even if it is, I think that the discussion has made it clear that the API for out-of-tree components shouldn't change.

@grandixximo
Copy link
Copy Markdown
Author

grandixximo commented Apr 4, 2026

I'm not so sure the discussion points in that direction anymore.
If anything it is clear, the API for out-of-tree components will change, when is the question.
If noparam is to be merged, that breaks stuff already, we are breaking a bit this version, and bit more the next? I think @rmu75 is on the rip the band aid now team, got a couple of integrators on Discord with out of tree components, they don't seem to mind 2.10 breaking their comps, as long as we make the transition painless, a .comp and .c component helper when building ala updat_ini, and clear warning that the comps are incompatible and need rebuilding if they try to run old components.

I think is a reasonable goal, but we are moving it to next release?

@andypugh
Copy link
Copy Markdown
Collaborator

andypugh commented Apr 4, 2026

I am not sure that "noparam" will actually break the API, as the only way to set parameters that I am aware of is the "setp" hal command, and that works equally well when parameters become pins.

We have been steadily changing parameters to pins for many years now, and don't seem to have been breaking things.

@grandixximo grandixximo force-pushed the fix/remove-uses-fp-2.10 branch 2 times, most recently from 5905b4d to d7d39a9 Compare April 5, 2026 01:16
@grandixximo
Copy link
Copy Markdown
Author

Refactored for milder version, which will give deprecation warnings, docs to match.
This should give some grace period to integrators to fix their stuff,

@grandixximo
Copy link
Copy Markdown
Author

I am not sure that "noparam" will actually break the API, as the only way to set parameters that I am aware of is the "setp" hal command, and that works equally well when parameters become pins.

We have been steadily changing parameters to pins for many years now, and don't seem to have been breaking things.

Andy, I respectfully disagree. While setp works on both params and pins (so HAL files are fine), the noparam branch completely removes the C API. All hal_param_*_new() functions, hal_param_dir_t, hal_param_*_set(), etc. are deleted from hal.h. Any out-of-tree component calling those functions won't compile. That's a hard API break, not a soft one like gradually converting individual components.

The uses_fp removal is actually a simpler migration per component, just dropping one argument from hal_export_funct(), whereas noparam requires changing every hal_param_*_new() call to hal_pin_*_new() plus updating value access from direct to pointer dereference.

For now, I've rolled this back to a milder change similar to what was done in 2.9, just adding deprecation warnings without breaking the API, as Bertho suggested.

That said, IMO when we do decide to break API compatibility, we should do it all in one shot. The uses_fp removal, noparam, and the 32-to-64 bit pin change are all intertwined, they are essentially one big compatibility break. Asking out-of-tree component authors to update their code three separate times across three releases doesn't make sense when we could bundle it into a single migration with proper tooling and docs. Either all three make it into 2.10, or none of them do.

@Sigma1912
Copy link
Copy Markdown
Contributor

got a couple of integrators on Discord with out of tree components, they don't seem to mind 2.10 breaking their comps, as long as we make the transition painless, a .comp and .c component helper when building ala updat_ini, and clear warning that the comps are incompatible and need rebuilding if they try to run old components.

+1

Makes no difference to me if my comps break with 2.10 or the one after it.
And yes, I would certainly prefer to only have to update them once.

@BsAtHome
Copy link
Copy Markdown
Contributor

BsAtHome commented Apr 5, 2026

In hal/components/threads.c: You might as well remove the fp1/fp2/fp3 in the call to hal_create_thread() too and fix it to 1. And add a comment above the RTAPI_MP_INT declaring fp1/fp2/fp3.

All threads now unconditionally save and restore FPU/SSE state,
making the uses_fp parameter obsolete. Rather than removing it
from the API (which would break out-of-tree components), this
commit deprecates it with a grace period:

RTAPI: uses_fp parameter is accepted but ignored; FPU state is
always saved in rtapi_task_new() regardless of the value passed.

HAL: uses_fp parameter is accepted but ignored in
hal_export_funct(), hal_export_functf(), and hal_create_thread().
All functions and threads are always marked as FP-capable
internally. The addf FP compatibility check is removed since
all threads are now FP-capable.

halcompile: fp/nofp keywords in function declarations now emit
a deprecation warning and are treated as fp (always return 1).
The keywords will be removed in a future version.

Remove fp/nofp from all in-tree .comp files, conv.comp.in
template, and mkconv.sh generator. Remove fp1= from test .hal
files. Out-of-tree .comp files will still parse but emit a
deprecation warning.

Documentation: API man pages, tutorials, and guides updated with
deprecation notices. Removed references to FP thread restrictions
that no longer apply.

No API signatures changed — out-of-tree components continue to
compile unchanged but will see warnings from halcompile if they
use fp/nofp.

Based on patch by BsAtHome.

Ref: LinuxCNC#3895
@grandixximo grandixximo force-pushed the fix/remove-uses-fp-2.10 branch from d7d39a9 to 50f7f1d Compare April 5, 2026 09:15
@grandixximo
Copy link
Copy Markdown
Author

grandixximo commented Apr 5, 2026

like that?
I kept module parameters for API compatibility...

That was weird, Somehow I pressed enter and gihub decided that meant to cancel the PR, go figure...

@grandixximo grandixximo closed this Apr 5, 2026
@grandixximo grandixximo reopened this Apr 5, 2026
@BsAtHome
Copy link
Copy Markdown
Contributor

BsAtHome commented Apr 5, 2026

like that? I kept module parameters for API compatibility...

Yes.

That was weird, Somehow I pressed enter and gihub decided that meant to cancel the PR, go figure...

That is the fun you get for allowing a machine to interpret your actions?
Or did you just push the "Close with comment" button?

@grandixximo
Copy link
Copy Markdown
Author

That was weird, Somehow I pressed enter and gihub decided that meant to cancel the PR, go figure...

That is the fun you get for allowing a machine to interpret your actions? Or did you just push the "Close with comment" button?

I typed in the first sentence, pressed enter, instead of doing return to the next line, it closed the PR by itself, the close button was not even in view as I was typing, this is some JS crap, or whatever they use to write GitHub, unless my brain is turning into mush....

@andypugh
Copy link
Copy Markdown
Collaborator

andypugh commented Apr 6, 2026

Andy, I respectfully disagree. While setp works on both params and pins (so HAL files are fine), the noparam branch completely removes the C API. All hal_param_*_new() functions,

You are right, of course. I should have said that the user interface or integrator interface remains the same.

It would be possible to alias the existing parameter functions to create pins instead, possibly with a warning.

I have forgotten where I was with 64-bit-ints branch, but doing that there was something I considered. I think that the current state has 32-bit pin creation disabled to force compilation errors to make it easier to find in-tree changes that need to be made, but I wasn't sure that was going to be the long-term situation.

@grandixximo
Copy link
Copy Markdown
Author

For the 64bit-int-branch
I think changing the approach to Bertho's getter/setter approach is the way forward, but Bertho doesn't think we can make for 2.10, I'm not sure where that is going, but it does break compatibility with out of tree components, so if we are trying not to break that for 2.10 we have to limit the scope, only fix the in tree components that do need 64bit for 2.10.
@BsAtHome agreed?

@BsAtHome
Copy link
Copy Markdown
Contributor

BsAtHome commented Apr 6, 2026

For the 64bit-int-branch I think changing the approach to Bertho's getter/setter approach is the way forward, but Bertho doesn't think we can make for 2.10, I'm not sure where that is going, but it does break compatibility with out of tree components, so if we are trying not to break that for 2.10 we have to limit the scope, only fix the in tree components that do need 64bit for 2.10. @BsAtHome agreed?

If we do major break compatibility with out-of-tree, then we should break all we can now and not bother users with different breakage with following releases. But that is a choice. Users may not like it/us either way. We do need to have a clear description and test procedure of how to update user code either way.

The point is that my getter/setter idea is just that, an idea in my head that has not yet seen any coding. I do think it is possible without much problems (based on some experience), but to be sure I'd need to do some testing. Is Andy's 32-bit elimination the contingency or parallel or what?

The next question would be: when should 2.10 be released? Are we talking days, weeks or months? I might be able to whip something working up in 3..4 weeks, but it would also need to be tested thoroughly.

This also assumes that I get the ini stuff done (still missing fixing major parts of documentation and half of python usage).

@grandixximo
Copy link
Copy Markdown
Author

@andypugh @BsAtHome

To answer Bertho's first question: I don't think Andy's branch is contingency or parallel at this point. The getter/setter approach supersedes it.

Andy's work brought this problem to the surface and drove the discussion that led us here, that's valuable. And Bertho's getter/setter idea gives us a clean path that solves the type bridging properly without pointer tricks or sign extension issues.

Looking at the codebase, encoder.c and stepgen.c are C files, not .comp files, so the halcompile remapping doesn't actually fix the rollover for the components that need it most. And any stop-gap fix (like changing those pins to s64 now) just creates converter friction in HAL files that gets undone when the getter/setter lands. It's not worth the churn.

Here's what I'd propose, tentatively, pending your agreement:

Let @BsAtHome work on the getter/setter prototype on his own timeline. If it's ready for 2.10, we merge it together with the uses_fp deprecation and noparam, one clean break, one migration for users, proper tooling and docs. If it's not ready in time, all of it goes into the next release. No partial breaks, no intermediate friction.

Either way, nothing from Andy's branch needs to merge now. The getter/setter will handle everything it was trying to do, and do it better.

Does that make sense to both of you?

@BsAtHome
Copy link
Copy Markdown
Contributor

BsAtHome commented Apr 6, 2026

We may want to move the discussion to #3286.

For this PR I suggest that it gets modified as not to change the API just yet (just like 2.9). Just make uses_fp obsolete by fixing it to 1 throughout and modify the components to get rid of fp/nofp. Also, the warning in halcompile should remain because 2.10 is the dev-branch and that may involve warnings. It does no break, but just warns. We can break the API all we want after this PR.

As for the getter/setter approach, it would definitely be better, but I need time to investigate in detail before I can give a more firm answer. And I also agree that the counter wrapping must be and can only be solved by fixing the code and not the hal types.

@grandixximo
Copy link
Copy Markdown
Author

Tomorrow I'll fix the merge, I think this PR it already is in the status you suggest, I have the full removal ready to go, I can make a separate PR for that, which you guys can merge whenever is the correct time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants