17

From what I understood about strict aliasing rule, this code for fast inverse square root will result in undefined behavior in C++:

float Q_rsqrt( float number )
{
    long i;
    float x2, y;
    const float threehalfs = 1.5F;

    x2 = number * 0.5F;
    y  = number;
    i  = * ( long * ) &y; // type punning
    i  = 0x5f3759df - ( i >> 1 );
    y  = * ( float * ) &i;
    y  = y * ( threehalfs - ( x2 * y * y ) );

    return y;
}

Does this code indeed cause UB? If yes, how can it be reimplemented in standard compliant way? If not, why not?

Assumptions: before calling this function we have somehow checked that floats are in IEEE 754 32-bit format, sizeof(long)==sizeof(float) and the platform is little-endian.

Community
  • 1
  • 1
Ruslan
  • 18,162
  • 8
  • 67
  • 136
  • 1
    As long as your compiler supports type punning through unions(in practice most do) that would probably be a safer way to do it as an [alternative to undefined violation of strict aliasing](http://stackoverflow.com/a/20956250/1708801), both `gcc` and `clang` do. – Shafik Yaghmour Jun 25 '14 at 09:39
  • 1
    @ShafikYaghmour maybe, but I could as well just use `-fno-strict-aliasing`, and this is not more compliant anyway. – Ruslan Jun 25 '14 at 09:40
  • Are we supposed to assume IEEE floats? I do not think that is valid, but I may be wrong. Regardless, change endianness of `float` but not `long`, and above does not work. C++ does not dictate endianness. What exactly do you mean by 'defined behaviour'? – Yakk - Adam Nevraumont Jun 25 '14 at 09:48
  • @Yakk good point. But defined behavior I mean that given some endianness and IEEE floats, we should get what we expect. – Ruslan Jun 25 '14 at 09:49
  • The union solution also fixes alighment issues – M.M Jun 25 '14 at 09:51
  • OT: this technique is badly named; inverse square-root is square! – M.M Jun 25 '14 at 09:52
  • 5
    @MattMcNabb multiplicative inverse, not function inverse. Bad is the natural language we use to ambiguously name things :) – Ruslan Jun 25 '14 at 09:54
  • Use `memcpy`. See [this question](http://stackoverflow.com/questions/20762952/most-efficient-and-standard-compliant-way-of-reinterpreting-int-as-float). – yuri kilochek Jun 25 '14 at 11:17
  • @yurikilochek WOW! If you put this as an answer, I'd be glad to accept it. – Ruslan Jun 25 '14 at 11:31
  • @Ruslan: you can't *just* as well use `-fno-strict-aliasing`, because that disables certain optimizations wherever they might occur in the TU. If you're writing a fast inverse square root function then you probably care about performance, and should not discard opportunities for automatic compiler optimizations, when all you need is that it doesn't optimize this specific code in a way that breaks it. If the compiler does allow for type punning through unions then it's not disabling all those optimizations (it only avoids them as they would apply to unions). – Steve Jessop Jun 25 '14 at 16:02
  • ... that said, if this function is the only thing in your TU then at a glance I don't see any obvious benefit for `-fstrict-aliasing` so you could go ahead :-) Just make sure that whole-program optimization eliminates the call overhead that would naturally result from the function not being inlined into the TUs that call it. – Steve Jessop Jun 25 '14 at 16:07

4 Answers4

15

The standard compliant way would be std::memcpy. This should be sufficiently standard-compliant under your specified assumptions. Any reasonable compiler will turn that into a bunch of register moves if possible anyway. Furthermore, we can also alleviate (or at least check) some of your made assumptions using C++11's static_assert and fixed width integer types from <cstdint>. Endianness is irrelevant anyway, since we're not working on any arrays here, if an integer type is little-endian, a floating point type is also.

float Q_rsqrt( float number )
{
    static_assert(std::numeric_limits<float>::is_iec559, 
                  "fast inverse square root requires IEEE-comliant 'float'");
    static_assert(sizeof(float)==sizeof(std::uint32_t), 
                  "fast inverse square root requires 'float' to be 32-bit");
    float x2 = number * 0.5F, y = number;
    std::uint32_t i;
    std::memcpy(&i, &y, sizeof(float));
    i  = 0x5f3759df - ( i >> 1 );
    std::memcpy(&y, &i, sizeof(float));
    return y * ( 1.5F - ( x2 * y * y ) );
}
Christian Rau
  • 45,360
  • 10
  • 108
  • 185
  • There is no need to cast to `char*` here, these pointers will be cast to `void*` anyway before anything will try to access the values they point to. – yuri kilochek Jun 25 '14 at 13:10
  • Add a `static_assert` for `static constexpr bool is_iec559` from numeric limits, and use a 32 bit fixed size integer, and I think we can get closer to actually cross platform. Dunno about endianness. – Yakk - Adam Nevraumont Jun 25 '14 at 13:39
  • @Yakk Not neccessary, since the question assumes it to work anyway. But ok, for the sake of completeness it's a good idea. – Christian Rau Jun 25 '14 at 13:40
  • One remaining problem seems to be that `i` could be negative and the algorithm seems to assume an arithmetic right shift in this case, which is implementation-defined though...Ah, scratch that, it's sqrt we're talking about. – Christian Rau Jun 25 '14 at 13:45
  • Hmm, only now noticed your answer. Thanks for actual implementation. Anyway, I've already accepted yuri's answer, also because he was the first to point to `memcpy` in the comments, and I'm not sure if it would be a good practice to unaccept a correct answer. – Ruslan Jun 25 '14 at 15:08
6

You should use memcpy. AFAIK this is the only standard compliant way and compilers are smart enough to replace the call with a single word move instruction. See this question for reasoning behind these claims.

Community
  • 1
  • 1
yuri kilochek
  • 12,709
  • 2
  • 32
  • 59
4

I don't think you can do this without UB in the strict standardese sense, simply because it relies on things like a particular value representation of float and long. The standard does not specify these (on purpose), and to give implementations the freedom to do what they see fit in this regard, specifically applies the "UB" label to all behaviour which would depend on such things.

That doesn't mean this will be a "Heisenbug" or "nasal demons" type of UB; most likely, the implementation will provide definitions for this behaviour. As long as these definitions satisfy the preconditions of the code, it's fine. The preconditions here being:

  • Type punning is allowed by the implementation.
  • sizeof(float) == sizeof(long).
  • The internal representation of float is such that if the bits are interpreted as a long, 0x5f3759df and >> 1 have the meaning reuqired for this hack to function.

However, it is impossible to re-write this in a fully portable way (i.e. without UB) - what would you do about platforms which use a different implementation of floating-point arithmetic, for example. As long as you're relying a particular platform's specifics, you're relying on behaviour which is undefined in the standard, but defined by the platform.

Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
  • Works also for `sizeof(long)>sizeof(float)`. – Ruslan Jun 25 '14 at 10:25
  • What if we have checked that the preconditions are satisfied before calling the function? – Ruslan Jun 25 '14 at 10:28
  • @Ruslan I don't think it necessarily works for different sizes - garbage values following `y` would be subsumed into the `long` value. And if you check the preconditions are satisfied (or just rely on them being such), then of course you can use it. But this doesn't make the behaviour defined in the standard - it's what I said, the platform providing a definition for behaviour not defined by the standard. – Angew is no longer proud of SO Jun 25 '14 at 10:32
  • Ah, indeed, seems it's just that `float`s are aligned on my g++ amd64 that it worked. What I mean by defined is that when the representation is checked for validity we could somehow do the type punning and still not break strict aliasing rules. The main problem I see here is not the representation of `float`s and `long`s, but the strict aliasing rule. – Ruslan Jun 25 '14 at 10:37
  • @Ruslan But the strict aliasing rule exists (among other reasons) to abstract away from different object representations. So you have to break it to rely on them. – Angew is no longer proud of SO Jun 25 '14 at 10:47
3

Not. The algorithm starts out by assuming IEEE754 layout, which isn't a valid Standard-compliant assumption. The particular constant 0x5f3759df would be utterly wrong for any other layout. Even the assumption that such a constant exists is flawed. I think it breaks as soon as you'd reverse the order of the fields inside a float, for instance.

I'm also not sure if Q_rsqrt(-0.0f) works correctly even assuming IEEE754. Both zeroes (positive and negative) should be equal and sqrt(x) is only undefined for x<-0.0f

MSalters
  • 173,980
  • 10
  • 155
  • 350
  • If I have heard correctly the C99 standard introduces the constant `__STDC_IEC_559__` indicating ISO floats. If it is 1 one can rely on layouts etc. Edit_ oh, I see; C++ tag. I'm not sure what applies there. – Peter - Reinstate Monica Jun 25 '14 at 10:07
  • 1
    The C++11 standard, in 18.3.2.3, has `static constexpr bool is_iec559 = false;` in numeric_limits. An implementation can specify specializations. – Peter - Reinstate Monica Jun 25 '14 at 10:14
  • Asking if `Q_rsqrt(-0.0f)` works correctly doesn't have any sense, because it's not meant to equal `1./sqrt(-0.0f)`, it's _approximate_. As any approximation, it'll break when the value is outside some range, i.e. its error will exceed acceptable value. – Ruslan Jun 25 '14 at 10:23
  • @Ruslan: I know it's an approximation of the correct IEEE754 value (+INF). And an approximation will therefore be either correct or off by -INF. But I'd still hope the result is equal to `Q_rsqrtf(+0.0f)` and bigger than any other result. – MSalters Jun 25 '14 at 10:29
  • 1
    It's, of course, true, and `Q_rsqrt(-0.0f)` is indeed close to zero instead of close to infinity, but the question is not about validity of algorithm, it's about possibility to implement it and get defined expected results when the assumptions are satisfied (which I've listed in updated question). – Ruslan Jun 25 '14 at 10:32
  • This page states that float, double, and long double have `is_iec559` as "usually true" in C++. http://en.cppreference.com/w/cpp/types/numeric_limits/is_iec559 I don't know much about IEC/IEEE or endianness but I'm interested in ways to make fast approximations more portable. Does anyone know if the magic number can be calculated from the properties of an arithmetic type to make it implementation-invariant? (The *assumption* of the existence in the general case might be incorrect but its existence in a given implementation might be deducible.) – John P Oct 17 '17 at 20:36
  • 1
    @JohnP: I still think the trick fails as soon as the mantissa precedes the exponent. The problem is that the bit shift moves one bit from the higher field to the lower field. This does _not_ break the mantissa in IEC559, but it _would_ break an exponent. Moving a 1 into the sign bit would also be rather disastrous, for obvious reasons ;) – MSalters Oct 18 '17 at 07:38
  • @MSalters That makes sense, but it sounds like you could determine the format from at least runtime introspection from a few floats, with help from `radix` / `{FLT,DBL,LDBL}_EPSILON` / `{min,max}_exponent`. As far as the sign bit, (real) square root of a negative is undefined, so you could either mask off the sign bit or follow convention, e.g. `A domain error occurs if the argument is less than zero.` The sign bit should be the xor of 1.0 and -1.0. What about an intermediate representation in emulated IEC559 between implementation-float endpoints instead? – John P Oct 18 '17 at 18:49