Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

Logical error performed by the Optimization of double to float conversion. #101

Closed
ak-alessard opened this issue Jun 12, 2015 · 6 comments
Closed

Comments

@ak-alessard
Copy link

ak-alessard commented Jun 12, 2015

Hi,

I am Alexandre and have been working with Emscripten for a little while and I am quite impressed to the capabilities of Emscripten. We have been working on prototyping a port of the Wwise sound engine to Emscripten. So far so good.

(Currently using \1.29.0 straight from the installer as my Emscripten environment.)

I have been investigating on an issue that was initially blamed to be caused by a rounding error on a Float32 type variable. (internally js will use doubles)

It was clear from the start that this was caused by optimizations -O1 -O2 or -O3, and it is already sorted out that specifying -s PRECISE_F32=1 or --llvm-opts 0 do make the problem go away.
Understanding that we wanted to preserve the optimizations we proceeded to pad the code against bad rounding and imprecisions, but I ended up on this:

The simplified behavior I am witnessing is the following:

//////////////////////////////////////////////////////////////////////////////////////////////
// Pseudo code representation of the error of the compiler:
// Don’t try this one, to get real repro you need fValueB to be a parameter inside a struct of the
// function passed in reference.
//////////////////////////////////////////////////////////////////////////////////////////////
 double dblVal = 0.99999999362240710177;
 float fValueA = dblVal;
 float fValueB = fValueA;

 if (fValueA == fValueB)
     printf("Values are the same\n");

 if (fValueA >= 1.f)
     printf("A >=1 \n");

 if (fValueB >= 1.f)
     printf("B >=1 \n");
//////////////////////////////////////////////////////////////////////////////////////////////

I understand the passing of double to float can make a rounding inaccuracy and taht fValueA and fValueB could be >=1 after conversion.

But then surprisingly I get:
-------OUTPUT:---------
Values are the same
B >= 1

My understanding is that the optimizer made two optimizations, each one was valid separately, but not if both was done because they were self invalidating.
Optimization 1: the double is never used and is only assigned to floatA, so we can consider is the same value.
Optimization 2: the floatB is same as the floatA

But then the optimizer preserved floatA in the double world and kept supposing floatB was in the float world because the format was enforced by the type that was passed in reference to the function, which was an illegal assumption since then float A was not same as floatB anymore.
The conversions from double to float that was supposed to be executed on the line:
float fValueA = dblVal;
But optimization did not think it mattered and only made it in the floatB.

And the real code to repro the problem is the following, but it needs some more repro stepd to repro it:

//////////////////////////////////////////////////////////////////////////////////////////////
//File main.cpp
//////////////////////////////////////////////////////////////////////////////////////////////
# include <emscripten.h>

static unsigned int uiPreventIfOptimizeValue = 0;

struct StructToFillContainingPhase
{
    float fPhase;
};

// This function is asking to fill a value passed in reference (the phase)
// it also Ensure phase is in range [ 0 <= phase < 1 ]
inline void GenerateValueLessThanOne( StructToFillContainingPhase& io_myStructToFill )
{
    float fNewPhase = 0;

```
// In my original problem, I did not have any double, only floats.
// But for the purpose of reproing this bug in a smaller setup, i enforced the usage of a double explicitely.
volatile double dblAlexValue = 0.0;
if (uiPreventIfOptimizeValue == 0 )
{
    // This is my trick to force the compiler to optimize the computations without considering dblAlexValue as a constant number, since it appears to fix the issue.
    // dblAlexValue must be non deterministically initialized to the exact value I need to repro the problem.
    dblAlexValue = 0.99999999362240710177;
}

// I identified the following line to be the culprit of a bad decision of the optimization is doing.
// the non-optimized version does it right and there is no bug.
// My belief is that the optimization thinks fNewPhase = dblAlexValue is a pointless operation and can be ignored, but a transformation from double to float does cause change in data.
// There might(will) be some loss of precision.
// This loss of precision is accepted and legal and we have to live with it.
// But the decision of the compiler does (only when optimizing) to believe this assignation is facultative and that fNewPhase and dblAlexValue are the same value is illegal and to not perform the other assignation "io_myStructToFill.fPhase" later will be fatal.
fNewPhase = dblAlexValue;

printf("dblAlexValue : %.20f\n", dblAlexValue); // will print 0.99999999362240710177
printf("fNewPhase : %f\n", fNewPhase);          // will print 1.000000
// I also made sure that fNewPhase hex value is exactly 0x3F800000, which is very very exactly 1.0f
// printf("fNewPhase : %.20f\n", fNewPhase);
// unsigned int * pREK = reinterpret_cast<unsigned int*>(&fNewPhase);
// printf("%8x\n", *pREK);

// And here comes the problem, the assignation: "fNewPhase = dblAlexValue" was not done yet or the machine is in fact using the registry dblAlexValue instead of fNewPhase here

// Ensure phase is in range [ 0 <= phase < 1 ]
while (fNewPhase >= 1.f)
{
    // never passes here.
    fNewPhase -= 1.f;
}

if (fNewPhase < 0.f || fNewPhase >= 1.f)
{
    // This printf will not print. is ok.
    printf("This would be impossible\n");
}

//assert( fNewPhase >= 0.f && fNewPhase < 1.f ); // will not assert
if (fNewPhase < 0.f || fNewPhase >= 1.f) // checking opposite condition. just in case
{
    // will not print
    printf("The Assert should have complained\n");// will not assert
}

// Here we assign the float to the value that neded to be filled, a float, 
io_myStructToFill.fPhase = fNewPhase;

if (io_myStructToFill.fPhase >= 1.f)// Here again, the system is making the if on the wrong value!
{
    // will Print!!!!!!!!!!!
    printf("Last line of the function Call GenerateValueLessThanOne() : io_myStructToFill.fPhase %f\n", io_myStructToFill.fPhase);
}
if (fNewPhase >= 1.f)// Here again, the system is making the if on the wrong value!
{
    // will not print
    printf("Last line of the function Call GenerateValueLessThanOne() : fNewPhase %f\n", fNewPhase);
}
```

}

static void main_loop()
{  
    // Note here that StructToFillContainingPhase is a structure that contains a float.
    // If it was simply a float, there would be no repro.
    StructToFillContainingPhase MyStructToFill;
    GenerateValueLessThanOne( MyStructToFill );
    if (MyStructToFill.fPhase >= 1.f)
    {
        // Detect the error and Bail out.
        printf("The function provided me an illegal value\n");
        emscripten_cancel_main_loop();
    }
}

int main( int argc, char *argv[] )
{
    emscripten_set_main_loop(main_loop, 0, false);

```
return 0;
```

}
//////////////////////////////////////////////////////////////////////////////////////////////
@kripken
Copy link
Member

kripken commented Jun 12, 2015

You can use four backticks to properly escape multiline code like that.

I assume the issue is that sometimes optimizations end up avoiding a store to memory. When stored to f32, rounding occurs, but when still in a variable, it doesn't. Yes, this can be surprising.

The only real solution is to enable PRECISE_F32, which as you mention works. Disabling llvm opts on the specific file you see the issue on can also be a workaround. But if your code is this sensitive to floating point values, I think enabling PRECISE_F32 is the best solution by far. It should be quite fast in modern browsers, but not always in older ones.

@ak-alessard
Copy link
Author

Thank you for the fast answer.

My solution for now will be to enable PRECISE_F32 and disable it only where performances will be potentially an issue.

@juj
Copy link
Collaborator

juj commented Jun 13, 2015

You can find more about this behavior in floating point and the nonguarantees there from http://gcc.gnu.org/wiki/x87note , which discusses the same behavior in native x87 fpu. Instead of 80bit vs 32/64bit issue in native, in Emscripten when compiling without PRECISE_F32, the "cpu register" contains a 64bit float, and the memory contains a 32bit float. That behavior is valid by the IEEE floating point spec, although it is extremely unintuitive and difficult to reason about. A good rule of thumb to avoid these issues is to follow these rules:

  • comparing floats for equality or inequality with == or != is undefined, except in the special case where the float was first explicitly assigned a small integer literal value (float f = 0;), and the comparison is afterwards done against that same literal value (if (f == 0)) without having done any computation on it in between.
  • any two sequences of operations (float <-> double cast being an operation as well) that are mathematically the same, but where the operations are permuted or reordered, will not be guaranteed to give the same result. This means that the commutativity, associativity and transitivity rules don't apply.
  • when comparing floats that are a result of computation instead of direct assignment of a small integer value, always compare against an epsilon threshold.

It seems that these rules, although being quite on the strict side of assumptions, avoid the float issues in practice.

@kripken
Copy link
Member

kripken commented Jun 13, 2015

Perhaps we should add this (or a link to this) to the FAQ?

@urkle
Copy link

urkle commented Jun 15, 2015

I agree that this would be very useful to have in a FAQ.

@juj
Copy link
Collaborator

juj commented Jun 15, 2015

Yeah agreed. I'll write a note about this, since this does seem to come up quite often.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants