[code-assembler] Make phi creation deterministic

Phi creation in the code assembler was dependent on the addresses
of the CodeAssemblerVariable::Impl object. This caused non-determinism
in mksnapshot which sometimes occurred on Windows.

This CL adds IDs to CodeAssemblerVariable::Impl objects and ensures
all iterations are done in ID order instead of object address order.


Change-Id: I2b370dc5153202be864a5c13289e70f5ebd59e2e
Bug: v8:8391
Reviewed-on: https://chromium-review.googlesource.com/c/1319749
Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57276}
This commit is contained in:
Sigurd Schneider 2018-11-06 13:23:53 +01:00 committed by Commit Bot
parent e896905547
commit 62a8054d9b
2 changed files with 34 additions and 7 deletions

View File

@ -1545,13 +1545,14 @@ RawMachineAssembler* CodeAssembler::raw_assembler() const {
// properly be verified. // properly be verified.
class CodeAssemblerVariable::Impl : public ZoneObject { class CodeAssemblerVariable::Impl : public ZoneObject {
public: public:
explicit Impl(MachineRepresentation rep) explicit Impl(MachineRepresentation rep, CodeAssemblerState::VariableId id)
: :
#if DEBUG #if DEBUG
debug_info_(AssemblerDebugInfo(nullptr, nullptr, -1)), debug_info_(AssemblerDebugInfo(nullptr, nullptr, -1)),
#endif #endif
value_(nullptr), value_(nullptr),
rep_(rep) { rep_(rep),
var_id_(id) {
} }
#if DEBUG #if DEBUG
@ -1562,13 +1563,25 @@ class CodeAssemblerVariable::Impl : public ZoneObject {
AssemblerDebugInfo debug_info_; AssemblerDebugInfo debug_info_;
#endif // DEBUG #endif // DEBUG
bool operator<(const CodeAssemblerVariable::Impl& other) const {
return var_id_ < other.var_id_;
}
Node* value_; Node* value_;
MachineRepresentation rep_; MachineRepresentation rep_;
CodeAssemblerState::VariableId var_id_;
}; };
bool CodeAssemblerVariable::ImplComparator::operator()(
const CodeAssemblerVariable::Impl* a,
const CodeAssemblerVariable::Impl* b) const {
return *a < *b;
}
CodeAssemblerVariable::CodeAssemblerVariable(CodeAssembler* assembler, CodeAssemblerVariable::CodeAssemblerVariable(CodeAssembler* assembler,
MachineRepresentation rep) MachineRepresentation rep)
: impl_(new (assembler->zone()) Impl(rep)), state_(assembler->state()) { : impl_(new (assembler->zone())
Impl(rep, assembler->state()->NextVariableId())),
state_(assembler->state()) {
state_->variables_.insert(impl_); state_->variables_.insert(impl_);
} }
@ -1583,7 +1596,9 @@ CodeAssemblerVariable::CodeAssemblerVariable(CodeAssembler* assembler,
CodeAssemblerVariable::CodeAssemblerVariable(CodeAssembler* assembler, CodeAssemblerVariable::CodeAssemblerVariable(CodeAssembler* assembler,
AssemblerDebugInfo debug_info, AssemblerDebugInfo debug_info,
MachineRepresentation rep) MachineRepresentation rep)
: impl_(new (assembler->zone()) Impl(rep)), state_(assembler->state()) { : impl_(new (assembler->zone())
Impl(rep, assembler->state()->NextVariableId())),
state_(assembler->state()) {
impl_->set_debug_info(debug_info); impl_->set_debug_info(debug_info);
state_->variables_.insert(impl_); state_->variables_.insert(impl_);
} }

View File

@ -1389,6 +1389,10 @@ class CodeAssemblerVariable {
friend class CodeAssemblerState; friend class CodeAssemblerState;
friend std::ostream& operator<<(std::ostream&, const Impl&); friend std::ostream& operator<<(std::ostream&, const Impl&);
friend std::ostream& operator<<(std::ostream&, const CodeAssemblerVariable&); friend std::ostream& operator<<(std::ostream&, const CodeAssemblerVariable&);
struct ImplComparator {
bool operator()(const CodeAssemblerVariable::Impl* a,
const CodeAssemblerVariable::Impl* b) const;
};
Impl* impl_; Impl* impl_;
CodeAssemblerState* state_; CodeAssemblerState* state_;
DISALLOW_COPY_AND_ASSIGN(CodeAssemblerVariable); DISALLOW_COPY_AND_ASSIGN(CodeAssemblerVariable);
@ -1478,10 +1482,14 @@ class CodeAssemblerLabel {
RawMachineLabel* label_; RawMachineLabel* label_;
// Map of variables that need to be merged to their phi nodes (or placeholders // Map of variables that need to be merged to their phi nodes (or placeholders
// for those phis). // for those phis).
std::map<CodeAssemblerVariable::Impl*, Node*> variable_phis_; std::map<CodeAssemblerVariable::Impl*, Node*,
CodeAssemblerVariable::ImplComparator>
variable_phis_;
// Map of variables to the list of value nodes that have been added from each // Map of variables to the list of value nodes that have been added from each
// merge path in their order of merging. // merge path in their order of merging.
std::map<CodeAssemblerVariable::Impl*, std::vector<Node*>> variable_merges_; std::map<CodeAssemblerVariable::Impl*, std::vector<Node*>,
CodeAssemblerVariable::ImplComparator>
variable_merges_;
}; };
class CodeAssemblerParameterizedLabelBase { class CodeAssemblerParameterizedLabelBase {
@ -1592,10 +1600,14 @@ class V8_EXPORT_PRIVATE CodeAssemblerState {
uint32_t stub_key_; uint32_t stub_key_;
int32_t builtin_index_; int32_t builtin_index_;
bool code_generated_; bool code_generated_;
ZoneSet<CodeAssemblerVariable::Impl*> variables_; ZoneSet<CodeAssemblerVariable::Impl*, CodeAssemblerVariable::ImplComparator>
variables_;
CodeAssemblerCallback call_prologue_; CodeAssemblerCallback call_prologue_;
CodeAssemblerCallback call_epilogue_; CodeAssemblerCallback call_epilogue_;
std::vector<CodeAssemblerExceptionHandlerLabel*> exception_handler_labels_; std::vector<CodeAssemblerExceptionHandlerLabel*> exception_handler_labels_;
typedef uint32_t VariableId;
VariableId next_variable_id_ = 0;
VariableId NextVariableId() { return next_variable_id_++; }
DISALLOW_COPY_AND_ASSIGN(CodeAssemblerState); DISALLOW_COPY_AND_ASSIGN(CodeAssemblerState);
}; };