New user self-registration is disabled due to spam. For an account please email bugs-admin@lists.llvm.org with your e-mail address and full name.

Bug 44012 - [WebAssembly] Float cast after !! can be wrong when compiled with -x c++
Summary: [WebAssembly] Float cast after !! can be wrong when compiled with -x c++
Status: RESOLVED FIXED
Alias: None
Product: libraries
Classification: Unclassified
Component: Backend: WebAssembly (show other bugs)
Version: trunk
Hardware: PC other
: P release blocker
Assignee: Thomas Lively
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-11-15 08:59 PST by Bernhard
Modified: 2019-11-16 08:32 PST (History)
4 users (show)

See Also:
Fixed By Commit(s): G194d7ec081c31ee4ed82bfa3cade4ef30afab912


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Bernhard 2019-11-15 08:59:07 PST
With all llvm/clang versions that support the wasm32 target (8, 9, current build of 10) the following function returns the correct 0.0f when compiled with '-x c' but it wrongfully returns 1.0f when compiled with '-x c++'.

    #ifdef __cplusplus
    extern "C"
    #endif
    float test()
    {
        static unsigned int mask = 2;
        mask |= 4;
        return (float)(!!(mask & 1));
    }

Here is how I built it to a wasm module:
    clang -x c -nostdinc -O3 -target wasm32 -c -o test.c.o test.c
    wasm-ld -no-entry -export=test -o test.c.wasm test.c.o

    clang -x c++ -nostdinc -O3 -target wasm32 -c -o test.cpp.o test.c
    wasm-ld -no-entry -export=test -o test.cpp.wasm test.cpp.o

And here is how I run the test in node.js v12
    node -e "WebAssembly.instantiate(new Uint8Array(require('fs').readFileSync('test.c.wasm'))).then(m => { console.log('c: ' + m.instance.exports.test()); })"
    node -e "WebAssembly.instantiate(new Uint8Array(require('fs').readFileSync('test.cpp.wasm'))).then(m => { console.log('cpp: ' + m.instance.exports.test()); })"

The output I get is
    c: 0
    cpp: 1

And this is the disassembly when running it through the wasm2wat tool from wabt:
    wasm2wat -f test.c.wasm
      (func $test (type 0) (result f32)
        (local i32)
        (i32.store offset=1024
          (i32.const 0)
          (i32.and
            (local.tee 0
              (i32.load offset=1024
                (i32.const 0)))
            (i32.const 5)))
        (f32.convert_i32_s
          (i32.and
            (local.get 0)
            (i32.const 1))))

    wasm2wat -f test.cpp.wasm
      (func $test (type 0) (result f32)
        (local i32)
        (i32.store offset=1024
          (i32.const 0)
          (i32.and
            (local.tee 0
              (i32.load offset=1024
                (i32.const 0)))
            (i32.const 5)))
        (select
          (f32.const 0x1p+0 (;=1;))
          (f32.const 0x0p+0 (;=0;))
          (local.get 0)))

The disassembly of the '-x c' module reads basically what the original C code was.
But it looks like the '-x c++' module returns something like (value_of_mask_at_function_entry ? 1 : 0) which makes no sense to me.
If the code line `mask |= 4` is changed into `mask &= 1`, the function returns 1 the first time it is called, then 0 afterwards which matches that assumption (the '-x c' variant stays correct).

Playing with compilation flag variations like -O0 did not affect the result.

Here's the versions I tested with (all on Win64)
  LLD 10.0.0 / clang version 10.0.0 (trunk) Target: x86_64-pc-windows-msvc 
  LLD 8.0.0 / clang version 8.0.0 (tags/RELEASE_800/final) Target: x86_64-pc-windows-msvc
  LLD 9.0.0 / clang version 9.0.0 (tags/RELEASE_900/final) Target: x86_64-pc-windows-msvc

They all produced identical assembly as far as I can tell.
Comment 1 Thomas Lively 2019-11-15 13:24:58 PST
Nice find! This looks rather serious. Clang generates slightly different LLVM IR for C and C++, and our backend bug only affects the IR generated by C++.

An even more reduced IR test case:


```
target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
target triple = "wasm32"

define float @test(i32 %0) {
  %and = and i32 %0, 1
  %tobool = icmp ne i32 %and, 0
  %conv = uitofp i1 %tobool to float
  ret float %conv
}
```

compiles to:

```
test:
        .functype       test (i32) -> (f32)
        f32.const       0x1p0
        f32.const       0x0p0
        local.get       0
        f32.select
        end_function
```

This is clearly incorrect. I'll investigate further and get a fix out ASAP.
Comment 2 Roman Lebedev 2019-11-16 08:15:32 PST
https://godbolt.org/z/eUCQTj
In C mode, clang lowers it into

  %4 = and i32 %3, 1, !dbg !18
  %5 = icmp ne i32 %4, 0, !dbg !19
  %6 = xor i1 %5, true, !dbg !19
  %7 = xor i1 %6, true, !dbg !20
  %8 = zext i1 %7 to i32, !dbg !20
  %9 = sitofp i32 %8 to float, !dbg !21

while in C++:

  %4 = and i32 %3, 1, !dbg !18
  %5 = icmp ne i32 %4, 0, !dbg !19
  %6 = xor i1 %5, true, !dbg !20
  %7 = xor i1 %6, true, !dbg !21
  %8 = uitofp i1 %7 to float, !dbg !22

i.e.
  %8 = zext i1 %7 to i32, !dbg !20
  %9 = sitofp i32 %8 to float, !dbg !21
vs
  %8 = uitofp i1 %7 to float, !dbg !22

Probably related to the fact that there is no bool in C,
but there is one in C++. Not backend-specific.