Commit 3b230165 authored by ahaas's avatar ahaas Committed by Commit bot

[wasm] Do float constant folding if the origin is not wasm

I removed some constant folding optimizations for float instruction in
https://codereview.chromium.org/2647353007 because they were incorrect
if the input was a signalling NaN. Removing these optimizations, however
had an unexpectedly big impact on asm.js performance. With this CL I
restore the optimizations again when the source origin is not wasm. In
JavaScript signalling NaNs are not observable and therefore the
optimizations are correct.

R=titzer@chromium.org
BUG=chromium:686654

Review-Url: https://codereview.chromium.org/2666903002
Cr-Commit-Position: refs/heads/master@{#42850}
parent 42011d29
...@@ -17,9 +17,9 @@ namespace v8 { ...@@ -17,9 +17,9 @@ namespace v8 {
namespace internal { namespace internal {
namespace compiler { namespace compiler {
MachineOperatorReducer::MachineOperatorReducer(JSGraph* jsgraph) MachineOperatorReducer::MachineOperatorReducer(JSGraph* jsgraph,
: jsgraph_(jsgraph) {} bool allow_signalling_nan)
: jsgraph_(jsgraph), allow_signalling_nan_(allow_signalling_nan) {}
MachineOperatorReducer::~MachineOperatorReducer() {} MachineOperatorReducer::~MachineOperatorReducer() {}
...@@ -316,7 +316,10 @@ Reduction MachineOperatorReducer::Reduce(Node* node) { ...@@ -316,7 +316,10 @@ Reduction MachineOperatorReducer::Reduce(Node* node) {
} }
case IrOpcode::kFloat32Sub: { case IrOpcode::kFloat32Sub: {
Float32BinopMatcher m(node); Float32BinopMatcher m(node);
// TODO(ahaas): We could do x - 0.0 = x if we knew that x is not an sNaN. if (allow_signalling_nan_ && m.right().Is(0) &&
(copysign(1.0, m.right().Value()) > 0)) {
return Replace(m.left().node()); // x - 0 => x
}
if (m.right().IsNaN()) { // x - NaN => NaN if (m.right().IsNaN()) { // x - NaN => NaN
// Do some calculation to make a signalling NaN quiet. // Do some calculation to make a signalling NaN quiet.
return ReplaceFloat32(m.right().Value() - m.right().Value()); return ReplaceFloat32(m.right().Value() - m.right().Value());
...@@ -360,7 +363,10 @@ Reduction MachineOperatorReducer::Reduce(Node* node) { ...@@ -360,7 +363,10 @@ Reduction MachineOperatorReducer::Reduce(Node* node) {
} }
case IrOpcode::kFloat64Sub: { case IrOpcode::kFloat64Sub: {
Float64BinopMatcher m(node); Float64BinopMatcher m(node);
// TODO(ahaas): We could do x - 0.0 = x if we knew that x is not an sNaN. if (allow_signalling_nan_ && m.right().Is(0) &&
(Double(m.right().Value()).Sign() > 0)) {
return Replace(m.left().node()); // x - 0 => x
}
if (m.right().IsNaN()) { // x - NaN => NaN if (m.right().IsNaN()) { // x - NaN => NaN
// Do some calculation to make a signalling NaN quiet. // Do some calculation to make a signalling NaN quiet.
return ReplaceFloat64(m.right().Value() - m.right().Value()); return ReplaceFloat64(m.right().Value() - m.right().Value());
...@@ -393,7 +399,8 @@ Reduction MachineOperatorReducer::Reduce(Node* node) { ...@@ -393,7 +399,8 @@ Reduction MachineOperatorReducer::Reduce(Node* node) {
} }
case IrOpcode::kFloat64Mul: { case IrOpcode::kFloat64Mul: {
Float64BinopMatcher m(node); Float64BinopMatcher m(node);
// TODO(ahaas): We could do x * 1.0 = x if we knew that x is not an sNaN. if (allow_signalling_nan_ && m.right().Is(1))
return Replace(m.left().node()); // x * 1.0 => x
if (m.right().Is(-1)) { // x * -1.0 => -0.0 - x if (m.right().Is(-1)) { // x * -1.0 => -0.0 - x
node->ReplaceInput(0, Float64Constant(-0.0)); node->ReplaceInput(0, Float64Constant(-0.0));
node->ReplaceInput(1, m.left().node()); node->ReplaceInput(1, m.left().node());
...@@ -416,6 +423,8 @@ Reduction MachineOperatorReducer::Reduce(Node* node) { ...@@ -416,6 +423,8 @@ Reduction MachineOperatorReducer::Reduce(Node* node) {
} }
case IrOpcode::kFloat64Div: { case IrOpcode::kFloat64Div: {
Float64BinopMatcher m(node); Float64BinopMatcher m(node);
if (allow_signalling_nan_ && m.right().Is(1))
return Replace(m.left().node()); // x / 1.0 => x
// TODO(ahaas): We could do x / 1.0 = x if we knew that x is not an sNaN. // TODO(ahaas): We could do x / 1.0 = x if we knew that x is not an sNaN.
if (m.right().IsNaN()) { // x / NaN => NaN if (m.right().IsNaN()) { // x / NaN => NaN
// Do some calculation to make a signalling NaN quiet. // Do some calculation to make a signalling NaN quiet.
......
...@@ -24,7 +24,8 @@ class JSGraph; ...@@ -24,7 +24,8 @@ class JSGraph;
class V8_EXPORT_PRIVATE MachineOperatorReducer final class V8_EXPORT_PRIVATE MachineOperatorReducer final
: public NON_EXPORTED_BASE(Reducer) { : public NON_EXPORTED_BASE(Reducer) {
public: public:
explicit MachineOperatorReducer(JSGraph* jsgraph); explicit MachineOperatorReducer(JSGraph* jsgraph,
bool allow_signalling_nan = true);
~MachineOperatorReducer(); ~MachineOperatorReducer();
Reduction Reduce(Node* node) override; Reduction Reduce(Node* node) override;
...@@ -104,6 +105,7 @@ class V8_EXPORT_PRIVATE MachineOperatorReducer final ...@@ -104,6 +105,7 @@ class V8_EXPORT_PRIVATE MachineOperatorReducer final
MachineOperatorBuilder* machine() const; MachineOperatorBuilder* machine() const;
JSGraph* jsgraph_; JSGraph* jsgraph_;
bool allow_signalling_nan_;
}; };
} // namespace compiler } // namespace compiler
......
...@@ -640,13 +640,15 @@ class PipelineWasmCompilationJob final : public CompilationJob { ...@@ -640,13 +640,15 @@ class PipelineWasmCompilationJob final : public CompilationJob {
explicit PipelineWasmCompilationJob( explicit PipelineWasmCompilationJob(
CompilationInfo* info, JSGraph* jsgraph, CallDescriptor* descriptor, CompilationInfo* info, JSGraph* jsgraph, CallDescriptor* descriptor,
SourcePositionTable* source_positions, SourcePositionTable* source_positions,
ZoneVector<trap_handler::ProtectedInstructionData>* protected_insts) ZoneVector<trap_handler::ProtectedInstructionData>* protected_insts,
bool allow_signalling_nan)
: CompilationJob(info->isolate(), info, "TurboFan", : CompilationJob(info->isolate(), info, "TurboFan",
State::kReadyToExecute), State::kReadyToExecute),
zone_stats_(info->isolate()->allocator()), zone_stats_(info->isolate()->allocator()),
data_(&zone_stats_, info, jsgraph, source_positions, protected_insts), data_(&zone_stats_, info, jsgraph, source_positions, protected_insts),
pipeline_(&data_), pipeline_(&data_),
linkage_(descriptor) {} linkage_(descriptor),
allow_signalling_nan_(allow_signalling_nan) {}
protected: protected:
Status PrepareJobImpl() final; Status PrepareJobImpl() final;
...@@ -658,6 +660,7 @@ class PipelineWasmCompilationJob final : public CompilationJob { ...@@ -658,6 +660,7 @@ class PipelineWasmCompilationJob final : public CompilationJob {
PipelineData data_; PipelineData data_;
PipelineImpl pipeline_; PipelineImpl pipeline_;
Linkage linkage_; Linkage linkage_;
bool allow_signalling_nan_;
}; };
PipelineWasmCompilationJob::Status PipelineWasmCompilationJob::Status
...@@ -682,7 +685,8 @@ PipelineWasmCompilationJob::ExecuteJobImpl() { ...@@ -682,7 +685,8 @@ PipelineWasmCompilationJob::ExecuteJobImpl() {
DeadCodeElimination dead_code_elimination(&graph_reducer, data->graph(), DeadCodeElimination dead_code_elimination(&graph_reducer, data->graph(),
data->common()); data->common());
ValueNumberingReducer value_numbering(scope.zone(), data->graph()->zone()); ValueNumberingReducer value_numbering(scope.zone(), data->graph()->zone());
MachineOperatorReducer machine_reducer(data->jsgraph()); MachineOperatorReducer machine_reducer(data->jsgraph(),
allow_signalling_nan_);
CommonOperatorReducer common_reducer(&graph_reducer, data->graph(), CommonOperatorReducer common_reducer(&graph_reducer, data->graph(),
data->common(), data->machine()); data->common(), data->machine());
AddReducer(data, &graph_reducer, &dead_code_elimination); AddReducer(data, &graph_reducer, &dead_code_elimination);
...@@ -1749,10 +1753,11 @@ CompilationJob* Pipeline::NewCompilationJob(Handle<JSFunction> function) { ...@@ -1749,10 +1753,11 @@ CompilationJob* Pipeline::NewCompilationJob(Handle<JSFunction> function) {
CompilationJob* Pipeline::NewWasmCompilationJob( CompilationJob* Pipeline::NewWasmCompilationJob(
CompilationInfo* info, JSGraph* jsgraph, CallDescriptor* descriptor, CompilationInfo* info, JSGraph* jsgraph, CallDescriptor* descriptor,
SourcePositionTable* source_positions, SourcePositionTable* source_positions,
ZoneVector<trap_handler::ProtectedInstructionData>* ZoneVector<trap_handler::ProtectedInstructionData>* protected_instructions,
protected_instructions) { bool allow_signalling_nan) {
return new PipelineWasmCompilationJob( return new PipelineWasmCompilationJob(
info, jsgraph, descriptor, source_positions, protected_instructions); info, jsgraph, descriptor, source_positions, protected_instructions,
allow_signalling_nan);
} }
bool Pipeline::AllocateRegistersForTesting(const RegisterConfiguration* config, bool Pipeline::AllocateRegistersForTesting(const RegisterConfiguration* config,
......
...@@ -41,7 +41,8 @@ class Pipeline : public AllStatic { ...@@ -41,7 +41,8 @@ class Pipeline : public AllStatic {
CompilationInfo* info, JSGraph* jsgraph, CallDescriptor* descriptor, CompilationInfo* info, JSGraph* jsgraph, CallDescriptor* descriptor,
SourcePositionTable* source_positions, SourcePositionTable* source_positions,
ZoneVector<trap_handler::ProtectedInstructionData>* ZoneVector<trap_handler::ProtectedInstructionData>*
protected_instructions); protected_instructions,
bool wasm_origin);
// Run the pipeline on a machine graph and generate code. The {schedule} must // Run the pipeline on a machine graph and generate code. The {schedule} must
// be valid, hence the given {graph} does not need to be schedulable. // be valid, hence the given {graph} does not need to be schedulable.
......
...@@ -3960,9 +3960,9 @@ void WasmCompilationUnit::ExecuteCompilation() { ...@@ -3960,9 +3960,9 @@ void WasmCompilationUnit::ExecuteCompilation() {
descriptor = module_env_->module_env.GetI32WasmCallDescriptor( descriptor = module_env_->module_env.GetI32WasmCallDescriptor(
&compilation_zone_, descriptor); &compilation_zone_, descriptor);
} }
job_.reset(Pipeline::NewWasmCompilationJob(&info_, jsgraph_, descriptor, job_.reset(Pipeline::NewWasmCompilationJob(
source_positions, &info_, jsgraph_, descriptor, source_positions, &protected_instructions_,
&protected_instructions_)); module_env_->module_env.module->origin != wasm::kWasmOrigin));
ok_ = job_->ExecuteJob() == CompilationJob::SUCCEEDED; ok_ = job_->ExecuteJob() == CompilationJob::SUCCEEDED;
// TODO(bradnelson): Improve histogram handling of size_t. // TODO(bradnelson): Improve histogram handling of size_t.
// TODO(ahaas): The counters are not thread-safe at the moment. // TODO(ahaas): The counters are not thread-safe at the moment.
......
...@@ -169,7 +169,7 @@ struct WasmExport { ...@@ -169,7 +169,7 @@ struct WasmExport {
uint32_t index; // index into the respective space. uint32_t index; // index into the respective space.
}; };
enum ModuleOrigin { kWasmOrigin, kAsmJsOrigin }; enum ModuleOrigin : uint8_t { kWasmOrigin, kAsmJsOrigin };
struct ModuleWireBytes; struct ModuleWireBytes;
// Static representation of a module. // Static representation of a module.
......
...@@ -602,7 +602,7 @@ class WasmFunctionCompiler : private GraphAndBuilders { ...@@ -602,7 +602,7 @@ class WasmFunctionCompiler : private GraphAndBuilders {
CompilationInfo info(CStrVector("wasm"), this->isolate(), this->zone(), CompilationInfo info(CStrVector("wasm"), this->isolate(), this->zone(),
Code::ComputeFlags(Code::WASM_FUNCTION)); Code::ComputeFlags(Code::WASM_FUNCTION));
std::unique_ptr<CompilationJob> job(Pipeline::NewWasmCompilationJob( std::unique_ptr<CompilationJob> job(Pipeline::NewWasmCompilationJob(
&info, &jsgraph, desc, &source_position_table_, nullptr)); &info, &jsgraph, desc, &source_position_table_, nullptr, false));
if (job->ExecuteJob() != CompilationJob::SUCCEEDED || if (job->ExecuteJob() != CompilationJob::SUCCEEDED ||
job->FinalizeJob() != CompilationJob::SUCCEEDED) job->FinalizeJob() != CompilationJob::SUCCEEDED)
return Handle<Code>::null(); return Handle<Code>::null();
......
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