[llvm-bugs] [Bug 31928] New: After r280351: if/else blocks incorrectly optimized away?

[via llvm-bugs]

https://llvm.org/bugs/show_bug.cgi?id=31928

            Bug ID: 31928
           Summary: After r280351: if/else blocks incorrectly optimized
                    away?
           Product: new-bugs
           Version: 4.0
          Hardware: PC
                OS: All
            Status: NEW
          Severity: normal
          Priority: P
         Component: new bugs
          Assignee: unassignedbugs at nondot.org
          Reporter: dimitry at andric.com
                CC: llvm-bugs at lists.llvm.org
    Classification: Unclassified

Created attachment 17965
  --> https://llvm.org/bugs/attachment.cgi?id=17965&action=edit
Test case reduces from gdtoa's ratio() function

>From https://bugs.freebsd.org/216770, where we had some trouble building the
Julia port, we found out that clang 4.0.0 seems to miscompile gdtoa's [1]
ratio() function in smisc.c [2], leading to incorrect strtrod(3) output.

What seems to be happening is that after r280351 ("[SimplifyCFG] Change the
algorithm in SinkThenElseCodeToEnd"), the if/else block is either entirely
optimized away, or only the else block is.  

The function itself takes unions of doubles and unsigned, and a parameter 'k'
which determines the fork to take in the if/else statement.  Simplified:

typedef union {
  double d;
  unsigned L[2];
} U;

double ratio(U da, U db, int k)
{
  if (k > 0) {
    da.L[1] += k * 0x100000;
  } else {
    k = -k;
    db.L[1] += k * 0x100000;
  }
  return da.d / db.d;
}

Trunk r280350 with -O2, targeting x86_64, compiles this to:

ratio:                                  # @ratio
        .cfi_startproc
# BB#0:                                 # %entry
        testl   %edx, %edx
        jle     .LBB0_2
# BB#1:                                 # %if.then
        shll    $20, %edx
        movq    %rdi, %rax
        shrq    $32, %rax
        addl    %edx, %eax
        shlq    $32, %rax
        movl    %edi, %edi
        orq     %rax, %rdi
        jmp     .LBB0_3
.LBB0_2:                                # %if.else
        shll    $20, %edx
        movq    %rsi, %rax
        shrq    $32, %rax
        subl    %edx, %eax
        shlq    $32, %rax
        movl    %esi, %esi
        orq     %rax, %rsi
.LBB0_3:                                # %if.end
        movd    %rdi, %xmm0
        movd    %rsi, %xmm1
        divsd   %xmm1, %xmm0
        retq

while r280351 and later compile this to just:

ratio:                                  # @ratio
        .cfi_startproc
# BB#0:                                 # %entry
        movd    %rdi, %xmm0
        movd    %rsi, %xmm1
        divsd   %xmm1, %xmm0
        retq

E.g., the whole if/else block is now gone.  I do not think the C code exhibits
undefined behavior, so getting rid of the if/else block is incorrect.

When targeting 32-bit x86, the results are slightly different, because adding a
printf of either the da or db parameter's d member changes the result.  Without
the printf, the end result is correct, with the printf, the result is
incorrect.  Also, printing the unsigned member of the union makes the result
correct again.  Test case:

#include <stdio.h>
typedef union {
  double d;
  unsigned L[2];
} U;

double ratio(U da, U db, int k)
{
  printf("%f\n", da.d);
  if (k > 0) {
    da.L[1] += k * 0x100000;
  } else {
    k = -k;
    db.L[1] += k * 0x100000;
  }
  return da.d / db.d;
}

Trunk r280350 using -m32 -O2 gives:

ratio:                                  # @ratio
# BB#0:                                 # %entry
        pushl   %edi
        pushl   %esi
        subl    $36, %esp
        movsd   48(%esp), %xmm0         # xmm0 = mem[0],zero
        movl    64(%esp), %edi
        leal    56(%esp), %esi
        movsd   %xmm0, 4(%esp)
        movsd   %xmm0, 16(%esp)         # 8-byte Spill
        movl    $.L.str, (%esp)
        calll   printf
        testl   %edi, %edi
        jle     .LBB0_2
# BB#1:                                 # %if.then
        leal    48(%esp), %eax
        shll    $20, %edi
        addl    %edi, 4(%eax)
        movsd   (%eax), %xmm0           # xmm0 = mem[0],zero
        jmp     .LBB0_3
.LBB0_2:                                # %if.else                             
                                      shll    $20, %edi
        movsd   16(%esp), %xmm0         # 8-byte Reload
                                        # xmm0 = mem[0],zero
        subl    %edi, 4(%esi)
.LBB0_3:                                # %if.end
        divsd   (%esi), %xmm0
        movsd   %xmm0, 24(%esp)
        fldl    24(%esp)
        addl    $36, %esp
        popl    %esi
        popl    %edi
        retl

Trunk r280351 using -m32 -O2 gives:

ratio:                                  # @ratio
# BB#0:                                 # %entry
        pushl   %edi
        pushl   %esi
        subl    $36, %esp
        movl    64(%esp), %esi
        movsd   48(%esp), %xmm0         # xmm0 = mem[0],zero
        movl    %esi, %edi
        movsd   %xmm0, 4(%esp)
        movsd   %xmm0, 16(%esp)         # 8-byte Spill
        movl    $.L.str, (%esp)
        negl    %edi
        cmovll  %esi, %edi
        calll   printf
        testl   %esi, %esi
        leal    48(%esp), %eax
        leal    56(%esp), %ecx
        movsd   16(%esp), %xmm0         # 8-byte Reload
                                        # xmm0 = mem[0],zero
        cmovgl  %eax, %ecx
        shll    $20, %edi                                                      
                                      orl     $4, %ecx
        addl    %edi, (%ecx)
        divsd   56(%esp), %xmm0
        movsd   %xmm0, 24(%esp)
        fldl    24(%esp)
        addl    $36, %esp
        popl    %esi
        popl    %edi
        retl

Again, the if/else block seems to have been optimized away.

[1] http://www.netlib.org/fp/
[2] https://github.com/jwiegley/gdtoa/blob/master/smisc.c#L72

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20170210/c64bbcc9/attachment-0001.html>