Open Bug 966957 Opened 6 years ago Updated 3 months ago

Float32: Specialize ToString for floats

Categories

(Core :: JavaScript Engine: JIT, task, P5)

task

Tracking

()

People

(Reporter: bbouvier, Unassigned)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(2 files)

That will allow us to emit MConcat when there's a Float32 incoming.
13:47:52 < h4writer> bbouvier, if one of the inputs is a "String" it will generate MConcat
13:49:31 < bbouvier> h4writer: true, it actually just doesn't happen with float32 - that's something i could solve
13:50:04 < h4writer> bbouvier, s/could/should ;)

This implements ToString specialization for Float32. MConcat specialization comes in the next patch with tests included.
Attachment #8369423 - Flags: review?(hv1989)
Comment on attachment 8369424 [details] [diff] [review]
Directly concat add(String, Float32) + tests

Review of attachment 8369424 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch !
Attachment #8369424 - Flags: review?(hv1989) → review+
Comment on attachment 8369423 [details] [diff] [review]
Convert Float to Strings directly

Review of attachment 8369423 [details] [diff] [review]:
-----------------------------------------------------------------

Nice

::: js/src/jit/CodeGenerator.cpp
@@ +785,5 @@
> +static JSString *
> +FloatToStringPar(ForkJoinContext *cx, float f)
> +{
> +    return NumberToString<NoGC>(cx, double(f));
> +}

We want these in jit/ParallelFunctions.cpp and jit/VMFunctions.cpp
Attachment #8369423 - Flags: review?(hv1989) → review+
Annnnnnnnnd it's a backout.

https://hg.mozilla.org/integration/mozilla-inbound/rev/3983d91df904

Let's the try server dance begin.
Soooooooooooo. After some hardcore debugging, it seems that one of the tests is passing by the out of line VM call, that doesn't know how to handle float arguments, hence the corrupted stack, hence the crashes all the way.
Depends on: 968855
Priority: -- → P5
Type: defect → task
Assignee: bbouvier → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.