Revert "[maglev] Spill nodes that we'd otherwise fail to merge"
This reverts commit a63f9912b7
.
Reason for revert: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64/50370/overview
Original change's description:
> [maglev] Spill nodes that we'd otherwise fail to merge
>
> This makes sure that catch-blocks don't accidentally drop values that
> are only in registers, which can happen if we throw in deferred throwing
> code (e.g., in ThrowReferenceErrorIfHole). At the latest we'll discover
> such values when trying to merge after the catch block, noticing we
> can't find the value through the catch-block. Unfortunately it's not
> trivial to figure out where that merge happens, so we just
> unconditionally spill the value.
>
> For liveness holes (as the comment previously mentioned) the value
> should already be dead and dropped on the merge. Running --maglev-stress
> etc shows that no code currently hits this path, except for the added
> test that shows the issue with catch blocks.
>
> Bug: chromium:1392061
> Change-Id: Ied0b1d4b430c9af2e7ae3dfc004ecb45037c5735
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4051605
> Reviewed-by: Leszek Swirski <leszeks@chromium.org>
> Commit-Queue: Leszek Swirski <leszeks@chromium.org>
> Commit-Queue: Toon Verwaest <verwaest@chromium.org>
> Auto-Submit: Toon Verwaest <verwaest@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#84448}
Bug: chromium:1392061
Change-Id: Iddbd7b19bc73e352dbd6867db990238f80adbdda
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4055504
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84455}
This commit is contained in:
parent
1b8a23b246
commit
b18d3e8c06
@ -308,7 +308,6 @@ class MaglevGraphBuilder {
|
||||
MarkBytecodeDead();
|
||||
return;
|
||||
}
|
||||
graph_->set_has_catch_block();
|
||||
ProcessMergePointAtExceptionHandlerStart(offset);
|
||||
} else {
|
||||
ProcessMergePoint(offset);
|
||||
|
@ -76,9 +76,6 @@ class Graph final : public ZoneObject {
|
||||
nan_ = nan;
|
||||
}
|
||||
|
||||
bool has_catch_block() const { return has_catch_block_; }
|
||||
void set_has_catch_block() { has_catch_block_ = true; }
|
||||
|
||||
private:
|
||||
uint32_t tagged_stack_slots_ = kMaxUInt32;
|
||||
uint32_t untagged_stack_slots_ = kMaxUInt32;
|
||||
@ -90,7 +87,6 @@ class Graph final : public ZoneObject {
|
||||
ZoneVector<InitialValue*> parameters_;
|
||||
compiler::ZoneRefMap<compiler::ObjectRef, Constant*> constants_;
|
||||
Float64Constant* nan_ = nullptr;
|
||||
bool has_catch_block_ = false;
|
||||
};
|
||||
|
||||
} // namespace maglev
|
||||
|
@ -1884,14 +1884,19 @@ void StraightForwardRegisterAllocator::MergeRegisterValues(ControlNode* control,
|
||||
}
|
||||
|
||||
if (node != nullptr && !node->is_loadable() && !node->has_register()) {
|
||||
// We've discovered a node that's not yet loadable but also isn't in a
|
||||
// register. Make sure we spill it at definition so we can reload it here.
|
||||
// This can happen because catch blocks drop all register state, but if
|
||||
// there's a JumpLoop after the catch we might need to restore a value
|
||||
// that wasn't spilled yet (e.g., because the only throw in the try-block
|
||||
// didn't force spill values).
|
||||
CHECK(graph_->has_catch_block());
|
||||
Spill(node);
|
||||
// If we have a node already, but can't load it here, we must be in a
|
||||
// liveness hole for it, so nuke the merge state.
|
||||
// This can only happen for conversion nodes, as they can split and take
|
||||
// over the liveness of the node they are converting.
|
||||
// TODO(v8:7700): Overeager DCHECK.
|
||||
// DCHECK(node->properties().is_conversion());
|
||||
if (v8_flags.trace_maglev_regalloc) {
|
||||
printing_visitor_->os() << " " << reg << " - can't load "
|
||||
<< PrintNodeLabel(graph_labeller(), node)
|
||||
<< ", dropping the merge\n";
|
||||
}
|
||||
state = {nullptr, initialized_node};
|
||||
return;
|
||||
}
|
||||
|
||||
const size_t size = sizeof(RegisterMerge) +
|
||||
|
@ -1,37 +0,0 @@
|
||||
// Copyright 2022 the V8 project authors. All rights reserved.
|
||||
// Use of this source code is governed by a BSD-style license that can be
|
||||
// found in the LICENSE file.
|
||||
|
||||
// Flags: --maglev --allow-natives-syntax
|
||||
|
||||
const obj7 = -1;
|
||||
function foo(arg) {
|
||||
let obj10 = 0;
|
||||
let obj11 = 0;
|
||||
for(var i= 0 ;i<2;i++){
|
||||
const obj15 = 1;
|
||||
const obj17 = obj11 + 1;
|
||||
const obj18 = 2;
|
||||
const obj19 = 3;
|
||||
const obj20 = obj10 / 3;
|
||||
obj10 = obj20;
|
||||
let obj21 = 0;
|
||||
do {
|
||||
try {
|
||||
const obj23 = !obj7;
|
||||
|
||||
} catch(e) {
|
||||
}
|
||||
obj21++;
|
||||
} while (obj21 < 2);
|
||||
}
|
||||
|
||||
}
|
||||
const obj32 = [1,2,3,4];
|
||||
|
||||
|
||||
%PrepareFunctionForOptimization(foo);
|
||||
foo(obj32);foo(obj32);foo(obj32);
|
||||
console.log("maglev");
|
||||
%OptimizeMaglevOnNextCall(foo);
|
||||
foo(obj32);
|
Loading…
Reference in New Issue
Block a user