[RFC] Add fast-math flags to atomicrmw

IR & Optimizations
1

Hi all,

I would like to propose allowing floating-point atomicrmw instructions to carry fast-math flags.

A prototype implementation is available in PR #211267⁠.

Overview

The proposed syntax allows existing fast-math flags to appear on an atomicrmw:

%old = atomicrmw nnan fadd ptr %ptr, float %value monotonic
%old = atomicrmw fast fadd ptr %ptr, double %value seq_cst

The flags would have the same meaning as documented for other instructions in the LLVM LangRef⁠, with one atomicrmw-specific clarification:

Fast-math flags additionally apply to the value written to memory.

This clarification is necessary because the current definitions of nnan and ninf apply only to input operands and the result, which does not provide a sufficiently strong constraint for atomicrmw node.

Motivation

The motivating case is the AArch64 FEAT_LSFE extension, which provides native atomic floating-point memory instructions such as LDFADD. Without using these instructions, an atomicrmw fadd is lowered to a compare-and-swap loop that calculates the new value using an ordinary FADD floating-point instruction.

Unlike the FADD instruction present in the loop, whose behaviour is determined by FPCR register, the LDFADD has fixed floating-point behaviour which can differ from the FADD one. In particular, it behaves as if default-NaN mode is enabled. We therefore want an explicit IR-level indication that NaN behaviour is not relevant before selecting LDFADD. The nnan flag provides this information and would allow the AArch64 backend to lower the operation directly to LDFADD.

Note: The current LangRef already permits an unflagged floating-point operation to produce the preferred NaN. However, relying on this would mean that the observable result of the same IR could change when compiling for a newer architecture that supports LDFADD.

Although such a transformation may technically be permitted by the current semantics, this behaviour is counter-intuitive. Users would reasonably expect enabling a new architectural feature to improve code generation without changing floating-point results. Requiring nnan makes this change explicit and ensures that LDFADD is selected only when the IR states that NaN behaviour is irrelevant.

Applicability

The proposal applies only to atomicrmw operations that perform floating-point arithmetic.

Fast-math flags would therefore be accepted for operations such as:

fadd
fsub
fmax
fmin
fmaximum
fminimum
fmaximumnum
fminimumnum

Questions

  1. Should atomicrmw accept the complete set of fast-math flags, as proposed, or only a subset such as nnan, ninf and nsz?
  2. Is it reasonable for the AArch64 backend to require nnan before selecting an LSFE instruction, even if the current LangRef could technically permit the instruction’s default-NaN behaviour for unflagged IR?
  3. More generally, should LLVM allow the result of an atomic floating-point operation to change solely because a newer target feature enables a different lowering, as the current LangRef appears to permit, or should such a change require an explicit fast-math assumption in the IR?

Feedback on the proposed semantics would be appreciated.

2

I think we shouldn’t require fast-math flags in order to generate LDFADD on Aarch64.

We specified the semantics for NaN payloads as allowing multiple choices in order to give the compiler flexibility. We take advantage of that flexibility in many other places in the compiler. (For example, if you have 2-register instruction, “FADD reg1, reg2” which writes the result back to one of the source registers, we treat it as symmetric and let the compiler choose arbitrarily which source to reuse as destination – despite that it is typically NOT fully symmetric when given 2 NaNs with different payloads.)

I see no reason not to do so here, as well; It is extremely rare for any user to care what NaN payloads they get from an operation.

1 Like
3

Thank you for the response @jyknight! I agree that since we are already ignoring it for FADD there is no reason to care about it for LSFE. I will close the patch now.