- Revision
- 252306
- Author
- tzaga...@apple.com
- Date
- 2019-11-08 23:05:16 -0800 (Fri, 08 Nov 2019)
Log Message
[WebAssembly] LLIntGenerator should not retain VirtualRegisters used for constants
https://bugs.webkit.org/show_bug.cgi?id=204028
Reviewed by Yusuke Suzuki.
The LLIntGenerator was keeping track of which RegisterIDs contained constants in order to materialize
the OSR entry data, since constants were not included in the OSR entry buffer. This was originally done
by adding the registers that contained constants to a vector and never reusing them. This is bad because
the bytecode generator reclaims registers by popping unused registers from the end of the vector and
stops as soon as it finds a used register. As it turns out, constants *should* be included in the buffer,
so we don't need to worry about whether registers contain constants and we can just stop retaining the
registers. An assertion was added to doOSREntry to ensure that the size of the scratch buffer matches the
size of the values to be written, which was not true before.
Additionally, add m_constantMap to LLIntGenerator to avoid adding duplicate constants to code blocks.
* bytecompiler/BytecodeGenerator.h:
* bytecompiler/BytecodeGeneratorBase.h:
* wasm/WasmB3IRGenerator.cpp:
(JSC::Wasm::B3IRGenerator::addLoop):
* wasm/WasmLLIntGenerator.cpp:
(JSC::Wasm::LLIntGenerator::addConstant):
(JSC::Wasm::LLIntGenerator::addLoop):
* wasm/WasmOperations.cpp:
(JSC::Wasm::doOSREntry):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (252305 => 252306)
--- trunk/Source/_javascript_Core/ChangeLog 2019-11-09 06:27:57 UTC (rev 252305)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-11-09 07:05:16 UTC (rev 252306)
@@ -1,3 +1,30 @@
+2019-11-08 Tadeu Zagallo <tzaga...@apple.com>
+
+ [WebAssembly] LLIntGenerator should not retain VirtualRegisters used for constants
+ https://bugs.webkit.org/show_bug.cgi?id=204028
+
+ Reviewed by Yusuke Suzuki.
+
+ The LLIntGenerator was keeping track of which RegisterIDs contained constants in order to materialize
+ the OSR entry data, since constants were not included in the OSR entry buffer. This was originally done
+ by adding the registers that contained constants to a vector and never reusing them. This is bad because
+ the bytecode generator reclaims registers by popping unused registers from the end of the vector and
+ stops as soon as it finds a used register. As it turns out, constants *should* be included in the buffer,
+ so we don't need to worry about whether registers contain constants and we can just stop retaining the
+ registers. An assertion was added to doOSREntry to ensure that the size of the scratch buffer matches the
+ size of the values to be written, which was not true before.
+ Additionally, add m_constantMap to LLIntGenerator to avoid adding duplicate constants to code blocks.
+
+ * bytecompiler/BytecodeGenerator.h:
+ * bytecompiler/BytecodeGeneratorBase.h:
+ * wasm/WasmB3IRGenerator.cpp:
+ (JSC::Wasm::B3IRGenerator::addLoop):
+ * wasm/WasmLLIntGenerator.cpp:
+ (JSC::Wasm::LLIntGenerator::addConstant):
+ (JSC::Wasm::LLIntGenerator::addLoop):
+ * wasm/WasmOperations.cpp:
+ (JSC::Wasm::doOSREntry):
+
2019-11-08 Yusuke Suzuki <ysuz...@apple.com>
Unreviewed, fix debug JSC tests failures due to missing exception check
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h (252305 => 252306)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2019-11-09 06:27:57 UTC (rev 252305)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGenerator.h 2019-11-09 07:05:16 UTC (rev 252306)
@@ -1225,6 +1225,7 @@
SegmentedVector<RegisterID, 32> m_parameters;
SegmentedVector<LabelScope, 32> m_labelScopes;
+ SegmentedVector<RegisterID, 32> m_constantPoolRegisters;
unsigned m_finallyDepth { 0 };
unsigned m_localScopeDepth { 0 };
const CodeType m_codeType;
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGeneratorBase.h (252305 => 252306)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGeneratorBase.h 2019-11-09 06:27:57 UTC (rev 252305)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGeneratorBase.h 2019-11-09 07:05:16 UTC (rev 252306)
@@ -85,7 +85,6 @@
SegmentedVector<GenericLabel<Traits>, 32> m_labels;
SegmentedVector<RegisterID, 32> m_calleeLocals;
- SegmentedVector<RegisterID, 32> m_constantPoolRegisters;
};
} // namespace JSC
Modified: trunk/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp (252305 => 252306)
--- trunk/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp 2019-11-09 06:27:57 UTC (rev 252305)
+++ trunk/Source/_javascript_Core/wasm/WasmB3IRGenerator.cpp 2019-11-09 07:05:16 UTC (rev 252306)
@@ -1299,8 +1299,10 @@
B3::InsertionSet insertionSet(m_proc);
for (unsigned i = 0; i < expressionStack.size(); i++) {
auto* value = expressionStack[i];
- if (value->isConstant())
+ if (value->isConstant()) {
+ ++indexInBuffer;
continue;
+ }
if (value->owner != sourceBlock) {
insertionSet.execute(sourceBlock);
Modified: trunk/Source/_javascript_Core/wasm/WasmLLIntGenerator.cpp (252305 => 252306)
--- trunk/Source/_javascript_Core/wasm/WasmLLIntGenerator.cpp 2019-11-09 06:27:57 UTC (rev 252305)
+++ trunk/Source/_javascript_Core/wasm/WasmLLIntGenerator.cpp 2019-11-09 07:05:16 UTC (rev 252306)
@@ -39,6 +39,7 @@
#include "WasmFunctionParser.h"
#include "WasmThunks.h"
#include <wtf/RefPtr.h>
+#include <wtf/StdUnorderedMap.h>
#include <wtf/Variant.h>
namespace JSC { namespace Wasm {
@@ -251,18 +252,6 @@
return m_jsNullConstant;
}
- bool isConstant(RefPtr<RegisterID> reg)
- {
- VirtualRegister virtualRegister = reg->virtualRegister();
- if (virtualRegister.isConstant())
- return true;
- for (auto& constant : m_constantPoolRegisters) {
- if (constant.virtualRegister() == virtualRegister)
- return true;
- }
- return false;
- }
-
struct SwitchEntry {
InstructionStream::Offset offset;
InstructionStream::Offset* jumpTarget;
@@ -275,6 +264,7 @@
HashMap<Label*, Vector<SwitchEntry>> m_switches;
ExpressionType m_jsNullConstant;
ExpressionList m_unitializedLocals;
+ StdUnorderedMap<uint64_t, VirtualRegister> m_constantMap;
};
Expected<std::unique_ptr<FunctionCodeBlock>, String> parseAndCompileBytecode(const uint8_t* functionStart, size_t functionLength, const Signature& signature, const ModuleInformation& info, uint32_t functionIndex, ThrowWasmException throwWasmException)
@@ -511,10 +501,12 @@
auto LLIntGenerator::addConstant(Type, uint64_t value) -> ExpressionType
{
VirtualRegister source(FirstConstantRegisterIndex + m_codeBlock->m_constants.size());
+ auto result = m_constantMap.emplace(value, source);
+ if (result.second)
+ m_codeBlock->m_constants.append(value);
+ else
+ source = result.first->second;
auto target = newTemporary();
- target->ref();
- m_constantPoolRegisters.append(target);
- m_codeBlock->m_constants.append(value);
WasmMov::emit(this, target, source);
return target;
}
@@ -570,11 +562,8 @@
osrEntryData.append(::JSC::virtualRegisterForLocal(i));
for (unsigned controlIndex = 0; controlIndex < m_parser->controlStack().size(); ++controlIndex) {
ExpressionList& expressionStack = m_parser->controlStack()[controlIndex].enclosedExpressionStack;
- for (auto& _expression_ : expressionStack) {
- if (isConstant(_expression_))
- continue;
+ for (auto& _expression_ : expressionStack)
osrEntryData.append(_expression_->virtualRegister());
- }
}
WasmLoopHint::emit(this);
Modified: trunk/Source/_javascript_Core/wasm/WasmOperations.cpp (252305 => 252306)
--- trunk/Source/_javascript_Core/wasm/WasmOperations.cpp 2019-11-09 06:27:57 UTC (rev 252305)
+++ trunk/Source/_javascript_Core/wasm/WasmOperations.cpp 2019-11-09 07:05:16 UTC (rev 252306)
@@ -126,6 +126,8 @@
context.gpr(GPRInfo::argumentGPR0) = 0;
};
+ RELEASE_ASSERT(osrEntryCallee.osrEntryScratchBufferSize() == osrEntryData.values().size());
+
uint64_t* buffer = instance->context()->scratchBufferForSize(osrEntryCallee.osrEntryScratchBufferSize());
if (!buffer)
return returnWithoutOSREntry();
