eliminate -fwrapv

[tonycoz]

This was added late in the 5.20 release cycle to eliminate optimizations done by gcc that reflect the limits of well-defined behavior from the C standard.

Recent testing and updates mean that such behavior has been eliminated in the core itself, so remove what was intended to be a temporary change (at least to me.)

The fixes the last remnants of #13690.

Now we just need to eliminate the need for -fno-strict-aliasing, which was also "only temporary" 34d1710

[xenu]

Unless there are benchmarks that show that -fwrapv noticeably decreases Perl's performance, I really don't see the point in asking the compiler to punish us with more UB.

[demerphq]

@xenu @tonycoz can you provide a bit more explanation about this for the uninitiated?

[jkeenan]

Unless there are benchmarks that show that -fwrapv noticeably decreases Perl's performance, I really don't see the point in asking the compiler to punish us with more UB.

How would we benchmark this, before and after the change?

[tonycoz]

The -fwrapv was originally added as a workaround for undefined behavior in perl.

Leaving it in masks undefined behavior defined by the C standard and means for our most common test cases (gcc) we aren't testing against what the C standard defines (or undefines).

Leaving -fwrapv in for gcc doesn't stop this being undefined behavior for other implementations.

I certainly expected it to be removed when I proposed the change.

[xenu]

Leaving -fwrapv in for gcc doesn't stop this being undefined behavior for other implementations.

I agree we should avoid relying on signed integer overflow to keep our code portable.

Leaving it in masks undefined behavior defined by the C standard and means for our most common test cases (gcc) we aren't testing against what the C standard defines (or undefines).

The problem is that signed integer overflow doesn't cause hard errors. The compiler can silently and subtly break the code in ways that our test suite won't detect, even with with ASan. The behaviour is unpredictable, it varies from compiler to compiler and it may change in the future versions. It's scary. There's a reason why so many C codebases (for example, the Linux kernel) use both -fwrapv and -fno-strict-aliasing. Frankly, the C standard is broken.

My personal opinion is we should catch accidental signed overflows in our CI with e.g. ASan, but -fwrapv should stay enabled in production builds.

[iabyn]

On Mon, Dec 05, 2022 at 05:07:11AM -0800, James E Keenan wrote: > Unless there are benchmarks that show that `-fwrapv` noticeably decreases Perl's performance, I really don't see the point in asking the compiler to punish us with more UB. How would we benchmark this, before and after the change?

With Porting/bench.pl, which is particularly good at detecting changes involving just executing one extra CPU instruction or one more memory read or write.

…

-- 31 Dec 1661: "I have newly taken a solemne oath about abstaining from plays". 1 Jan 1662: "And after ... we went by coach to the play". -- The Diary of Samuel Pepys

[tonycoz]

You've largely convinced me, though we'll need to find a similar option for MSVC, since that also optimizes assuming no signed integer overflows (the mentioned switch wasn't accepted for me in compiler explorer).

I haven't been able to find any documentation of the optimization changes caused by -fwrapv in gcc (though the MSVC article gives one example for its optimization), but -fno-strict-aliasing has fairly obvious effects, eg:

int f(int * p1, float *p2) {
    if (*p1 == 2)
       *p2 = 0;
    
    return *p1;
}

Compiles to:

f:
        movl    (%rdi), %eax
        cmpl    $2, %eax
        je      .L4
        ret
.L4:
        movl    $0x00000000, (%rsi)
        movl    (%rdi), %eax  ; extra memory access added by -fno-strict-aliasing
        ret

Unfortunately perl uses type punning so extensively that I don't think we'll ever be able to use strict aliasing, though maybe we'll be able to use restrict in some places strategically to avoid this type of de-optimization.

Closing.