Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Wrong optimization: conditional equivalence vs. values with several representations #44071

Open Quuxplusone opened 4 years ago

Quuxplusone commented 4 years ago
Bugzilla Link PR45101
Status NEW
Importance P enhancement
Reported by Alexander Cherepanov (ch3root@openwall.com)
Reported on 2020-03-04 09:50:49 -0800
Last modified on 2020-03-10 09:50:14 -0700
Version trunk
Hardware PC Linux
CC efriedma@quicinc.com, htmldeveloper@gmail.com, listmail@philipreames.com, llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also
The problem happens when:
- conditional equivalence propagation replaces an expression with a variable or
a constant that has the same value but a different representation, and
- this happens in a computation where representation is accessed.

Example with a pseudo-denormal in long double:

----------------------------------------------------------------------
#include <stdio.h>

__attribute__((noipa,optnone)) // imagine it in a separate TU
static void *opaque(void *p) { return p; }

int main()
{
    long double x, y;

    unsigned char *px = (unsigned char *)&x;
    unsigned char *py = (unsigned char *)&y;

    // make x pseudo-denormal
    x = 0;
    px[7] = 0x80;
    opaque(&x); // hide it from the optimizer

    y = x;

    if (y == 0x1p-16382l)
        printf("py[8] = %d\n", py[8]);
    printf("py[8] = %d\n", py[8]);
}
----------------------------------------------------------------------
$ clang -std=c11 -Weverything -Wno-unknown-attributes -O3 test.c && ./a.out
py[8] = 1
py[8] = 0
----------------------------------------------------------------------
clang x86-64 version: clang version 11.0.0 (https://github.com/llvm/llvm-
project 9284abd0040afecfd619dbcf1b244a8b533291c9)
----------------------------------------------------------------------

The value 0x1p-16382l admits two representations:

00 00 80 00 00 00 00 00 00 00  pseudo-denormal
00 01 80 00 00 00 00 00 00 00  normalized value

So both 0 and 1 for py[8] are fine but the testcase should print the same value
both times, i.e. the representation of y should be stable.

DR 260 Q1 allows for unstable representation but IMO this is wrong.

gcc bug -- https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94035
Quuxplusone commented 4 years ago

I guess this is specifically an issue with x87 long double? I'm pretty sure that for all IEEE float formats, given two non-zero, non-NaN values, x==y implies that x and y are bit-wise identical.

Quuxplusone commented 4 years ago

I'm not exactly sure. I would expect IBM double-double and IEEE 754 decimal floating-point formats to exhibit the same behavior. But I haven't checked this, plus I'm not sure about their support in LLVM.

Quuxplusone commented 4 years ago

This is controlled by impliesEquivalanceIfTrue in llvm/lib/Transforms/Scalar/GVN.cpp .

Quuxplusone commented 4 years ago
  1. clang doesn't support decimal floating-point formats. For an example of potential future problems please see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94035#c5 .

  2. x86 extended precision format (x86_fp80): gcc devs seems to consider pseudo-denormals (and other pseudo-*, unnormals etc.) as trap representations -- please see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94035#c2 .

  3. IBM extended double (double-double) (ppc_fp128): affected but please see what gcc devs say -- https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94035#c4 .


include

attribute((noipa,optnone)) // imagine it in a separate TU static long double opaque(long double d) { return d; }

int main() { union { long double d; unsigned long l[2]; } x = {opaque(-(long double)-1)};

if (x.d == 1)
    printf("%016lx %016lx\n", x.l[0], x.l[1]);
printf("%016lx %016lx\n", x.l[0], x.l[1]);

}

$ clang -target ppc64-linux-gnu -std=c11 -pedantic -Wall -Wextra -Wno-unknown-attributes test.c && qemu-ppc64 /usr/powerpc-linux-gnu/lib64/ld64.so.1 --library-path /usr/powerpc-linux-gnu/lib64 ./a.out 3ff0000000000000 8000000000000000 3ff0000000000000 8000000000000000 $ clang -target ppc64-linux-gnu -std=c11 -pedantic -Wall -Wextra -Wno-unknown-attributes -O3 test.c && qemu-ppc64 /usr/powerpc-linux-gnu/lib64/ld64.so.1 --library-path /usr/powerpc-linux-gnu/lib64 ./a.out 0000000000000000 3ff0000000000000 3ff0000000000000 8000000000000000

clang x86-64 version: clang version 11.0.0 (https://github.com/llvm/llvm-project feb20a159410dfec4055428911c7eb2bc908cf12)

For comparison:


$ powerpc64-linux-gnu-gcc -std=c11 -pedantic -Wall -Wextra -Wno-attributes test.c && qemu-ppc64 /usr/powerpc-linux-gnu/lib64/ld64.so.1 --library-path /usr/powerpc-linux-gnu/lib64 ./a.out 3ff0000000000000 0000000000000000 3ff0000000000000 0000000000000000 $ powerpc64-linux-gnu-gcc -std=c11 -pedantic -Wall -Wextra -Wno-attributes -O3 test.c && qemu-ppc64 /usr/powerpc-linux-gnu/lib64/ld64.so.1 --library-path /usr/powerpc-linux-gnu/lib64 ./a.out 3ff0000000000000 0000000000000000 3ff0000000000000 0000000000000000

gcc x86-64 version: powerpc64-linux-gnu-gcc (Debian 8.3.0-2) 8.3.0

Here, x has long double value 1. which is represented as the pair (1., -0.) of ordinary doubles, the constant 1 in the if is converted to the pair (1., 0.). They are the same value (and are necessarily equal) but have different (non-trap) representations.

There are 3 issues here:

1) the sign of the low part of -(long double)-1 differs from gcc -- AIUI both variants are fine so not sure if anybody cares; 2) high and low parts of ppc_fp128 computed at compile-time are interchanged -- I remember seeing a discussion of it somewhere but I cannot find it right now; 3) after we swap high and low parts, the low part computed at compile-time (0000000000000000, i.e. 0.) differs from run-time (8000000000000000, i.e. -0.) -- that's what this PR is about.

Quuxplusone commented 4 years ago
(In reply to Alexander Cherepanov from comment #4)
> 2) high and low parts of ppc_fp128 computed at compile-time are interchanged
> -- I remember seeing a discussion of it somewhere but I cannot find it right
> now;

I've searched the bugzilla a little bit more and still cannot find it. Filed as
bug 45137.
Quuxplusone commented 4 years ago

At the IR level, I'd be happy to say that arithmetic on unnormal/pseudo-denormal long double produces poison, sure. Which would imply that the arithmetic is UB at the C level. It simplifies our reasoning without affecting any legitimate use cases. Someone would need to write that up in LangRef, though.

For double-double, yes, it's sort of on its way out, but people who are using it care that it doesn't break in weird ways.