255u8.leading_zeros() is wrong on emscripten

@kripken

Running some tests this program is incorrect when run through emscripten currently:

fn main() {                                               
    println!("{}", 255u8.leading_zeros());                
}

This program prints 232 when I'd expect 0. This may be due to the implementation in emscripten-core/emscripten#4544 which looks like:

function _llvm_ctlz_i8(x, isZeroUndef) { 
    x = x | 0;                           
    isZeroUndef = isZeroUndef | 0;       
    return (Math_clz32(x) | 0) - 24 | 0; 
}

(in the compiled JS at least)

x coming in is -1, so Math_clz32(x) returns 0, and then chopping of 24 returns -24. When interpreted as a u8 that's 232 (what we see).

@kripken I'm not familiar with asmjs internals/representations, but is this a bug in the intrinsic? Or perhaps in our own representation of integers along the line? Or did I get unlucky with an LLVM mismatch or something?

It does seem like the asm.js implementation of that intrinsic should be modified to just look at the lower 8 bits, instead of assuming the top 24 are zero. I'll do that. However, I worry there is something more generally wrong here.

I would expect 255u8 to be an llvm i8, which is then sent into the LLVM intrinsic which receives an i8. After compilation to JS, any math operation on it should preserve that only the lower 8 bits are used (since we &255 after e.g. addition etc. - that's why we assume we can look at only the lower 8 bits; but, in certain ffi conditions, that might be false, which is why I want to fix it in emscripten as I said).

But that seems to not be what's happening here. It's apparently sent as 0xffffffff, which means it was sign-extended, despite being unsigned, and despite the intrinsic expecting an i8 so no extension should have been done.. Did that happen on the rust side, llvm side, or emscripten side? Bitcode could help.

@kripken

@kripken sure yeah, for this rust program:

pub fn main() -> u32 {
    255u8.leading_zeros()
}

I get this IR (unoptimized):

; ModuleID = 'foo.cgu-0.rs'
source_filename = "foo.cgu-0.rs"
target datalayout = "e-p:32:32-i64:64-v128:32:128-n32-S128"
target triple = "asmjs-unknown-emscripten"

; Function Attrs: inlinehint uwtable
define internal i32 @"_ZN4core3num20_$LT$impl$u20$u8$GT$13leading_zeros17h0d6a1b849828bff0E"(i8) unnamed_addr #0 {
entry-block:
  %tmp_ret = alloca i8
  br label %start

start:                                            ; preds = %entry-block
  %1 = call i8 @llvm.ctlz.i8(i8 %0, i1 false)
  store i8 %1, i8* %tmp_ret
  %2 = load i8, i8* %tmp_ret
  br label %bb1

bb1:                                              ; preds = %start
  %3 = zext i8 %2 to i32
  ret i32 %3
}

; Function Attrs: uwtable
define i32 @_ZN3foo4main17ha0ecef40c3578f1eE() unnamed_addr #1 {
entry-block:
  br label %start

start:                                            ; preds = %entry-block
  %0 = call i32 @"_ZN4core3num20_$LT$impl$u20$u8$GT$13leading_zeros17h0d6a1b849828bff0E"(i8 -1)
  br label %bb1

bb1:                                              ; preds = %start
  ret i32 %0
}

; Function Attrs: nounwind readnone
declare i8 @llvm.ctlz.i8(i8, i1) #2

attributes #0 = { inlinehint uwtable }
attributes #1 = { uwtable }
attributes #2 = { nounwind readnone }

I agree that something's getting sign extended, I'm just not sure where :(

do not assume anything about the top bits in llvm intrinsics receivin…

Great, perfect. So the issue is clang emits i8 255 for such constants, but rust i8 -1, and we just never handled the latter case, and indeed the intrinsics need fixing, but there is no deeper issue hidden here. Fix in emscripten-core/emscripten#4865

@kripken

Awesome, thanks for the quick fix @kripken!

@kripken

Thanks @kripken !

do not assume anything about the top bits in llvm intrinsics receivin…

travis: Get an emscripten builder online

Rollup merge of rust-lang#39120 - alexcrichton:emscripten-tests, r=brson

I believe this has since been fixed

remove FIXME(rust-lang#39119) and allow running test on emscripten

Navigation Menu

Repository navigation

255u8.leading_zeros() is wrong on emscripten #39119

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

255u8.leading_zeros() is wrong on emscripten #39119

Description

Activity

kripken commented on Jan 17, 2017

alexcrichton commented on Jan 17, 2017

kripken commented on Jan 17, 2017

alexcrichton commented on Jan 17, 2017

brson commented on Jan 18, 2017

alexcrichton commented on Aug 25, 2017

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions