Floating-point <=> lowering correctness issues

[jonatanlinden]

Issue Summary

The new run-time NaN handling for floating-point <=> in lower_cpp20_spaceship lowers x <=> y to:

(__isnan(temp2 = y) || __isnan(temp1 = x)) ? unordered
  : temp1 < temp2 ? less : temp2 < temp1 ? greater : equivalent

The generated C has two problems:

1. x is evaluated only inside the right operand of ||. When y is NaN, x is never evaluated, so its side effects are lost.
2. When y has no side effects, it is not copied to a temporary, and it is read both before x is evaluated (in the NaN test) and after (in the < tests). If x modifies y, the NaN test and the ordering tests see different values.

Reproducing Source Code

#include <compare>

float x();
float y();
auto issue1() { return x() <=> y(); }

float g;
float h();
auto issue2() { return h() <=> g; }

<compare> is the front end's own header (include_c++/compare.stdh).

Command-line Options

cpfe --c++20 -I/include_c++ repro.cpp

Type of Issue

[Back End] There is a problem with the generated C code.

Additional Details

Generated C for the two functions (temporaries renamed tx, ty, th and r):

// issue1() body snippet
  r = (__builtin_isnanf(ty = _Z1yv()) || __builtin_isnanf(tx = _Z1xv()))
        ? unordered
        : (tx < ty) ? less: (ty < tx) ? greater : equivalent;

// issue2() body snippet
  r = (__builtin_isnanf(g) || __builtin_isnanf(th = _Z1hv()))
        ? unordered
        : (th < g) ? less: (g < th) ? greater : equivalent;

Problem 1: the left operand is not evaluated when the right operand is NaN. In issue1, the only call of x() is in the right operand of ||. If y() returns NaN, x() is never called. The order of evaluation of the two operands of <=> is unspecified, so calling y() first is allowed, but both operands must be evaluated. (The order also differs from the comment in the source, which says __isnan(temp1 = x) || __isnan(temp2 = y).)

Problem 2: a side-effect-free operand is re-read after the other operand has been evaluated. In issue2, g is read by __builtin_isnanf(g) before h() is called, and again by both < tests after it. If h() stores a NaN into g while g was not NaN before, the NaN test finds no NaN, both < tests are false, and the result is EQUIVALENT. A single read of g, before or after h(), could only give LESS/GREATER or UNORDERED.

Godbolt: https://godbolt.org/z/KfMWorjb3 (two asserts, comment out the first one to make the second one trigger)

Reproducing (Standard) Configuration(s)

linux-gcc-release

Reproducing (Non-standard) Configuration

No response

I acknowledge that:

• I have checked open issues and existing discussions before filing this report.
• The reproducing source code that I have provided does not depend on headers that I have not included with my submission.
• I have reduced the reproducing source code to the best of my ability.
• I have selected at least one standard configuration or uploaded a non-standard configuration dump.
• I understand code shared via this form is NOT confidential.
• I have permission to share the reproducing source code and grant permission for the project to use the source code (or derivatives of the source code) to create tests.
• I agree that any tests created using the reproducing source code (or deriatives of it) will be licensed under project license unless I included a LICENSE.txt file in my .zip or .tar.gz upload. If I included a LICENSE.txt file, any tests created using the reproducing source code will be governed by the terms of the license I provided instead.

[jonatanlinden]

I'd like to add that I have a suggested fix, that I'm on the way to create a PR for. (I'm working for Qt/IAR)