Commit 5f77a1b3 authored by Victor Gomes's avatar Victor Gomes Committed by V8 LUCI CQ

[maglev] Fix float64 add deoptimization

CheckedFloat64Unbox mutates the input value, but the register allocator
does not expects this behaviour and propagates a wrong value in the register.
In particular we deopt with the wrong value if the second Float64Unbox
in a Float64Add needs to deopt.

This fixes the input value after we convert to double.

Bug: v8:7700
Change-Id: Ib89573e9f728dc3a34b817fc84f1afcb96f14d18
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3610422
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Auto-Submit: Victor Gomes <victorgomes@chromium.org>
Reviewed-by: 's avatarLeszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#80214}
parent 3f695391
......@@ -792,6 +792,12 @@ void CheckedFloat64Unbox::GenerateCode(MaglevCodeGenState* code_gen_state,
// If Smi, convert to Float64.
__ sarl(value, Immediate(1));
__ Cvtlsi2sd(ToDoubleRegister(result()), value);
// TODO(v8:7700): Add a constraint to the register allocator to indicate that
// the value in the input register is "trashed" by this node. Currently we
// have the invariant that the input register should not be mutated when it is
// not the same as the output register or the function does not call a
// builtin. So, we recover the Smi value here.
__ addl(value, value);
__ jmp(&done);
__ bind(&is_not_smi);
// Check if HeapNumber, deopt otherwise.
......
......@@ -23,8 +23,40 @@
// We deopt if not a number.
assertEquals("42", add("4", "2"));
assertFalse(isMaglevved(add));
})();
// Deopt in the second Float64Unbox when the first argument is a Smi.
(function() {
function add(x, y) {
return x + y;
}
%PrepareFunctionForOptimization(add);
assertEquals(4.2, add(2.1, 2.1));
%OptimizeMaglevOnNextCall(add);
assertEquals(4.2, add(2.1, 2.1));
assertTrue(isMaglevved(add));
// TODO(victorgomes): Fix deopt when we have a float,
// i.e., add(4, "2") will create a float with number 4
// and correctly deopt, but the state is bogus.
// We deopt if not a number.
assertEquals("42", add(4, "2"));
assertFalse(isMaglevved(add));
})();
// Deopt in the second Float64Unbox when the first argument is a double.
(function() {
function add(x, y) {
return x + y;
}
%PrepareFunctionForOptimization(add);
assertEquals(4.2, add(2.1, 2.1));
%OptimizeMaglevOnNextCall(add);
assertEquals(4.2, add(2.1, 2.1));
assertTrue(isMaglevved(add));
// We deopt if not a number.
assertEquals("4.2!", add(4.2, "!"));
assertFalse(isMaglevved(add));
})();
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment