Title: [290421] trunk/Source/_javascript_Core
- Revision
- 290421
- Author
- [email protected]
- Date
- 2022-02-24 03:13:46 -0800 (Thu, 24 Feb 2022)
Log Message
[JSC] Respect bytecode alignment in BytecodeRewriter
https://bugs.webkit.org/show_bug.cgi?id=237092
Patch by Geza Lore <[email protected]> on 2022-02-24
Reviewed by Yusuke Suzuki.
Note: This patch only affects bytecode generation on platforms which
set CPU(NEEDS_ALIGNED_ACCESS), which are ARMv7 and MIPS. On all other
platforms the generated bytecode is identical.
The previous BytecodeRewriter::removeBytecode method unconditionally
removed the given instruction, which could then break the required
alignment of subsequent wide ops. While this could be fixed by
inserting padding after the removal, all current uses of
removeBytecode are such that they constitute one half of a replace.
Instead of adding unnecessary padding, added an explicit
replaceBytecodeWithFragment method that removes the old instruction
and replaces it with the given fragment, while maintaining alignment
of the subsequent bytecode. This yields fewer nops. If removeBytecode
turns out to be necessary later, use replaceBytecodeWithFragment with
an empty fragment.
* bytecode/BytecodeGeneratorification.cpp:
(JSC::BytecodeGeneratorification::run):
* bytecode/BytecodeRewriter.h:
(JSC::BytecodeRewriter::Fragment::align):
(JSC::BytecodeRewriter::insertFragmentAfter):
(JSC::BytecodeRewriter::replaceBytecodeWithFragment):
* bytecompiler/BytecodeGeneratorBaseInlines.h:
(JSC::BytecodeGeneratorBase<Traits>::alignWideOpcode32):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (290420 => 290421)
--- trunk/Source/_javascript_Core/ChangeLog 2022-02-24 10:22:01 UTC (rev 290420)
+++ trunk/Source/_javascript_Core/ChangeLog 2022-02-24 11:13:46 UTC (rev 290421)
@@ -1,3 +1,35 @@
+2022-02-24 Geza Lore <[email protected]>
+
+ [JSC] Respect bytecode alignment in BytecodeRewriter
+ https://bugs.webkit.org/show_bug.cgi?id=237092
+
+ Reviewed by Yusuke Suzuki.
+
+ Note: This patch only affects bytecode generation on platforms which
+ set CPU(NEEDS_ALIGNED_ACCESS), which are ARMv7 and MIPS. On all other
+ platforms the generated bytecode is identical.
+
+ The previous BytecodeRewriter::removeBytecode method unconditionally
+ removed the given instruction, which could then break the required
+ alignment of subsequent wide ops. While this could be fixed by
+ inserting padding after the removal, all current uses of
+ removeBytecode are such that they constitute one half of a replace.
+ Instead of adding unnecessary padding, added an explicit
+ replaceBytecodeWithFragment method that removes the old instruction
+ and replaces it with the given fragment, while maintaining alignment
+ of the subsequent bytecode. This yields fewer nops. If removeBytecode
+ turns out to be necessary later, use replaceBytecodeWithFragment with
+ an empty fragment.
+
+ * bytecode/BytecodeGeneratorification.cpp:
+ (JSC::BytecodeGeneratorification::run):
+ * bytecode/BytecodeRewriter.h:
+ (JSC::BytecodeRewriter::Fragment::align):
+ (JSC::BytecodeRewriter::insertFragmentAfter):
+ (JSC::BytecodeRewriter::replaceBytecodeWithFragment):
+ * bytecompiler/BytecodeGeneratorBaseInlines.h:
+ (JSC::BytecodeGeneratorBase<Traits>::alignWideOpcode32):
+
2022-02-23 Yusuke Suzuki <[email protected]>
[JSC] WeakMapImpl do not need to take cellLock in visitOutputConstraints and main thread
Modified: trunk/Source/_javascript_Core/bytecode/BytecodeGeneratorification.cpp (290420 => 290421)
--- trunk/Source/_javascript_Core/bytecode/BytecodeGeneratorification.cpp 2022-02-24 10:22:01 UTC (rev 290420)
+++ trunk/Source/_javascript_Core/bytecode/BytecodeGeneratorification.cpp 2022-02-24 11:13:46 UTC (rev 290421)
@@ -255,7 +255,7 @@
});
// Emit resume sequence.
- rewriter.insertFragmentAfter(instruction, [&] (BytecodeRewriter::Fragment& fragment) {
+ rewriter.replaceBytecodeWithFragment(instruction, [&] (BytecodeRewriter::Fragment& fragment) {
data.liveness.forEachSetBit([&](size_t index) {
VirtualRegister operand = virtualRegisterForLocal(index);
Storage storage = storageForGeneratorLocal(vm, index);
@@ -270,14 +270,11 @@
);
});
});
-
- // Clip the unnecessary bytecodes.
- rewriter.removeBytecode(instruction);
}
if (m_generatorFrameData) {
auto instruction = m_instructions.at(m_generatorFrameData->m_point);
- rewriter.insertFragmentAfter(instruction, [&] (BytecodeRewriter::Fragment& fragment) {
+ rewriter.replaceBytecodeWithFragment(instruction, [&] (BytecodeRewriter::Fragment& fragment) {
if (!m_generatorFrameSymbolTable->scopeSize()) {
// This will cause us to put jsUndefined() into the generator frame's scope value.
fragment.appendInstruction<OpMov>(m_generatorFrameData->m_dst, m_generatorFrameData->m_initialValue);
@@ -284,7 +281,6 @@
} else
fragment.appendInstruction<OpCreateLexicalEnvironment>(m_generatorFrameData->m_dst, m_generatorFrameData->m_scope, m_generatorFrameData->m_symbolTable, m_generatorFrameData->m_initialValue);
});
- rewriter.removeBytecode(instruction);
}
rewriter.execute();
Modified: trunk/Source/_javascript_Core/bytecode/BytecodeRewriter.h (290420 => 290421)
--- trunk/Source/_javascript_Core/bytecode/BytecodeRewriter.h 2022-02-24 10:22:01 UTC (rev 290420)
+++ trunk/Source/_javascript_Core/bytecode/BytecodeRewriter.h 2022-02-24 11:13:46 UTC (rev 290421)
@@ -157,11 +157,13 @@
});
}
- void align()
+ void align(size_t congruent = 0)
{
+ UNUSED_PARAM(congruent);
#if CPU(NEEDS_ALIGNED_ACCESS)
+ congruent = congruent % OpcodeSize::Wide32;
m_bytecodeGenerator.withWriter(m_writer, [&] {
- while (m_bytecodeGenerator.instructions().size() % OpcodeSize::Wide32)
+ while (m_bytecodeGenerator.instructions().size() % OpcodeSize::Wide32 != congruent)
OpNop::emit<OpcodeSize::Narrow>(&m_bytecodeGenerator);
});
#endif
@@ -193,19 +195,22 @@
}
template<class Function>
- void insertFragmentAfter(const InstructionStream::Ref& instruction, Function function)
+ void insertFragmentAfter(const InstructionStream::Ref& instruction, Function function, size_t alignCongruent = 0)
{
IncludeBranch includeBranch = IncludeBranch::No;
InstructionStreamWriter writer;
Fragment fragment(m_bytecodeGenerator, writer, includeBranch);
function(fragment);
- fragment.align();
+ fragment.align(alignCongruent);
insertImpl(InsertionPoint(instruction.offset(), Position::After), includeBranch, WTFMove(writer));
}
- void removeBytecode(const InstructionStream::Ref& instruction)
+ template<class Function>
+ void replaceBytecodeWithFragment(const InstructionStream::Ref& instruction, Function function)
{
+ // Note: This function preserves the alignment of the subsequent bytecode (on targets where this matters)
m_insertions.append(Insertion { InsertionPoint(instruction.offset(), Position::OriginalBytecodePoint), Insertion::Type::Remove, IncludeBranch::No, instruction->size(), { } });
+ insertFragmentAfter(instruction, function, instruction->size());
}
void execute();
Modified: trunk/Source/_javascript_Core/bytecompiler/BytecodeGeneratorBaseInlines.h (290420 => 290421)
--- trunk/Source/_javascript_Core/bytecompiler/BytecodeGeneratorBaseInlines.h 2022-02-24 10:22:01 UTC (rev 290420)
+++ trunk/Source/_javascript_Core/bytecompiler/BytecodeGeneratorBaseInlines.h 2022-02-24 11:13:46 UTC (rev 290421)
@@ -114,7 +114,7 @@
{
#if CPU(NEEDS_ALIGNED_ACCESS)
size_t opcodeSize = 1;
- size_t prefixAndOpcodeSize = opcodeSize + PaddingBySize<OpcodeSize::Wide16>::value;
+ size_t prefixAndOpcodeSize = opcodeSize + PaddingBySize<OpcodeSize::Wide32>::value;
while ((m_writer.position() + prefixAndOpcodeSize) % OpcodeSize::Wide32)
Traits::OpNop::template emit<OpcodeSize::Narrow>(this);
#endif
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
