Skip to content

Commit

Permalink
[maglev] Check NewTarget is live in entry stack check
Browse files Browse the repository at this point in the history
Instead of checking if the NewTarget is valid (via the bytecode), we
check if the register is live in the register snapshot. Since its
use could be in Maglev's dead code.

Fixed: chromium:1462501
Bug: v8:7700
Change-Id: I61bd383999e2c0cf7d0dae9f826c6bed9cf7b977
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4677157
Auto-Submit: Victor Gomes <victorgomes@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#88828}
  • Loading branch information
victorgomes authored and V8 LUCI CQ committed Jul 11, 2023
1 parent d3534e8 commit 3807e28
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 17 deletions.
4 changes: 1 addition & 3 deletions src/maglev/maglev-graph-builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -269,9 +269,7 @@ class MaglevGraphBuilder {
// Don't use the AddNewNode helper for the function entry stack check, so
// that we can set a custom deopt frame on it.
FunctionEntryStackCheck* function_entry_stack_check =
NodeBase::New<FunctionEntryStackCheck>(
zone(), {},
bytecode().incoming_new_target_or_generator_register().is_valid());
NodeBase::New<FunctionEntryStackCheck>(zone(), {});
new (function_entry_stack_check->lazy_deopt_info()) LazyDeoptInfo(
zone(), GetDeoptFrameForEntryStackCheck(),
interpreter::Register::invalid_value(), 0, compiler::FeedbackSource());
Expand Down
6 changes: 5 additions & 1 deletion src/maglev/maglev-ir.cc
Original file line number Diff line number Diff line change
Expand Up @@ -855,6 +855,7 @@ uint32_t InitialValue::stack_slot() const {
return stack_slot(source_.index());
}

int FunctionEntryStackCheck::MaxCallStackArgs() const { return 0; }
void FunctionEntryStackCheck::SetValueLocationConstraints() {
set_temporaries_needed(2);
// kReturnRegister0 should not be one of the available temporary registers.
Expand All @@ -868,8 +869,11 @@ void FunctionEntryStackCheck::GenerateCode(MaglevAssembler* masm,
// stack limit or tighter. By ensuring we have space until that limit
// after building the frame we can quickly precheck both at once.
const int stack_check_offset = masm->code_gen_state()->stack_check_offset();
// Only NewTarget can be live at this point.
DCHECK_LE(register_snapshot().live_registers.Count(), 1);
Builtin builtin =
new_target_or_generator_register_is_valid()
register_snapshot().live_tagged_registers.has(
kJavaScriptCallNewTargetRegister)
? Builtin::kMaglevFunctionEntryStackCheck_WithNewTarget
: Builtin::kMaglevFunctionEntryStackCheck_WithoutNewTarget;
ZoneLabelRef done(masm);
Expand Down
17 changes: 4 additions & 13 deletions src/maglev/maglev-ir.h
Original file line number Diff line number Diff line change
Expand Up @@ -5049,28 +5049,19 @@ class FunctionEntryStackCheck
using Base = FixedInputNodeT<0, FunctionEntryStackCheck>;

public:
explicit FunctionEntryStackCheck(
uint64_t bitfield, bool new_target_or_generator_register_is_valid)
: Base(bitfield),
new_target_or_generator_register_is_valid_(
new_target_or_generator_register_is_valid) {}
explicit FunctionEntryStackCheck(uint64_t bitfield) : Base(bitfield) {}

// Although FunctionEntryStackCheck calls a builtin, we don't mark it as Call
// here. The register allocator should not spill any live registers, since the
// builtin already handles it. The only possible live register is
// kJavaScriptCallNewTargetRegister.
static constexpr OpProperties kProperties = OpProperties::LazyDeopt();
static constexpr OpProperties kProperties =
OpProperties::LazyDeopt() | OpProperties::DeferredCall();

int MaxCallStackArgs() const;
void SetValueLocationConstraints();
void GenerateCode(MaglevAssembler*, const ProcessingState&);
void PrintParams(std::ostream&, MaglevGraphLabeller*) const {}

bool new_target_or_generator_register_is_valid() const {
return new_target_or_generator_register_is_valid_;
}

private:
bool new_target_or_generator_register_is_valid_ = false;
};

class CheckedInternalizedString
Expand Down

0 comments on commit 3807e28

Please sign in to comment.