Stabilize const_mul_add

[tgross35]

Newly stable API:

impl {f32, f64} {
    pub const fn mul_add(self, a: Self, b: Self) -> Self;
}

This includes making the intrinsics fmaf{16,32,64,128} const stable for indirect use, matching similar intrinsics.

Closes: #146724

[rustbot]

⚠️ #[rustc_intrinsic_const_stable_indirect] controls whether intrinsics can be exposed to stable const
code; adding it needs t-lang approval.

cc @rust-lang/wg-const-eval

Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter
gets adapted for the changes, if necessary.

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

[rustbot]

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[traviscross]

Over on #146724, @RalfJung said:

Under the hood this is an intrinsic which needs to be const-stabilized to complete this... However the intrinsic just computes a pure function on scalar values so there's nothing really interesting happening on the language level.

@scottmcm noted that there are

no new NAN questions or new table-maker dilemna issues with mul_add that don't already exist for *+, as far as I could come up with.

@tgross35 noted that:

From a quick check I tried, I'm getting some iffy results e.g. fma(0.0000015, 15.36, 0.01563) for f16. I need to verify my methodology though.

However, we can probably start the lang-side proposed FCP while this matter is investigated.

@rfcbot fcp merge

[rust-rfcbot]

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @scottmcm
• @tmandry
• @traviscross

Concerns:

• iffy-results resolved by Stabilize const_mul_add #148052 (comment)

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[tmandry]

@rfcbot reviewed

[rust-rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[nikomatsakis]

@rfcbot reviewed

[Mark-Simulacrum]

From a quick check I tried, I'm getting some iffy results e.g. fma(0.0000015, 15.36, 0.01563) for f16. I need to verify my methodology though.

My assumption is that this is not a blocker for merging, because f16 remains unstable. If so, r=me.

[RalfJung]

As I explained in the tracking issue:

The const implementation of this intrinsic is one uniform function that handles all sizes. So if f16 is wrong, chances are the other types have the same bug -- f16 is just much easier to exhaustively test.

[RalfJung]

As a lang team advisor, could I please get this registered as a concern, so we avoid completing FCP until this is figured out?

AFAIK the bot still won't let me register concerns myself, but I guess I can try...
@rfcbot concern iffy-results

[tgross35]

Thanks for checking this, I never got the chance to circle back here. If that was an f16-specific problem then, I don't think there is anything to block on: @scottmcm (or somebody else) would you mind resolving the concern?

[scottmcm]

It has to be me, so
@rfcbot resolve iffy-results

@RalfJung, if it wasn't actually answered to your satisfaction please let me know and I'll re-register a concern.

[rust-rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[RalfJung]

@tgross35

If that was an f16-specific problem then, I don't think there is anything to block on

How do you conclude it was f16-specific? I don't even know what the bug was, all we have is your vague "fma(0.0000015, 15.36, 0.01563) is iffy for f16". What does "iffy" mean? What results do you get, what would you have expected instead? Should we open an issue to track this?

OTOH, worst-case we have a const-time mul_add that gives slightly wrong results, and we just fix that. 🤷

[quaternic]

@RalfJung
What very likely must have happened before is that @tgross35 saw that as a test case where f16::mul_add differs between const-eval and runtime. But it turned out that it is the runtime evaluation that is incorrect (due to an LLVM bug, which is specific to f16, see the link in my previous message).

[tgross35]

I don't even know what the bug was, all we have is your vague "fma(0.0000015, 15.36, 0.01563) is iffy for f16". What does "iffy" mean? What results do you get, what would you have expected instead? Should we open an issue to track this?

I was intentionally vague because this was a test of rustc_apfloat vs. f16::mul_add that I wrote way too quick. Just noted that was a specific value to check, if I or somebody showed a correct result against MPFR then I'd happily attribute the mismatch to a bug in the test I wrote. But @quaternic went beyond that and pointed to the well-known f16-specific bug.

How do you conclude it was f16-specific?

I'm deferring to @quaternic's analysis here but if (1) the values I mentioned match a known f16-specific bug and (2) apfloat actually has the correct answer, I have no reason to believe there is a bug in rustc_apfloat.

[RalfJung]

Ah, I didn't properly understand your earlier message before, @quaternic. Now it all makes sense :)

[rust-rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

[RalfJung]

LGTM, thanks!

@bors r+ rollup

[bors]

📌 Commit 033711c has been approved by RalfJung

It is now in the queue for this repository.