Implement Nop #320
Implement Nop #320
Conversation
src/jit/Analysis.cpp
Outdated
| @@ -455,7 +455,7 @@ void JITCompiler::buildVariables(uint32_t requiredStackSize) | |||
| } | |||
|
|
|||
| if (instr->opcode() == ByteCode::ThrowOpcode || instr->opcode() == ByteCode::UnreachableOpcode | |||
| || instr->opcode() == ByteCode::EndOpcode) { | |||
| || instr->opcode() == ByteCode::EndOpcode || instr->opcode() == ByteCode::NopOpcode) { | |||
There was a problem hiding this comment.
Nop is not a control transfer instruction.
| @@ -25,28 +25,29 @@ | |||
|
|
|||
| #include "wabt/walrus/binary-reader-walrus.h" | |||
|
|
|||
| #define EXECUTE_VALIDATOR(expr) \ | |||
There was a problem hiding this comment.
Why this file is changed?
There was a problem hiding this comment.
Because the autoformat formatted it. Should I turn off the autoformat for this file?
There was a problem hiding this comment.
This file is probably not checked by the CI, you can just ignore it without changes.
| @@ -1447,6 +1447,13 @@ ByteCodeStackOffset* Interpreter::interpret(ExecutionState& state, | |||
| NEXT_INSTRUCTION(); | |||
| } | |||
|
|
|||
| DEFINE_OPCODE(Nop) | |||
There was a problem hiding this comment.
This should be debug only.
| @@ -1404,6 +1404,10 @@ static void compileFunction(JITCompiler* compiler) | |||
| compiler->append(byteCode, group, opcode, 0, 0); | |||
| break; | |||
| } | |||
| case ByteCode::NopOpcode: { | |||
vorosl
force-pushed
the
nop
branch
4 times, most recently
from
145521e to
aa3a087
Compare
|
Where is this |
|
That is the plan. It should be debug only. In some time sensitive tasks, it could do something, but WebAssembly is high level for such cases. |
| case ByteCode::NopOpcode: { | ||
| sljit_emit_op0(m_compiler, SLJIT_NOP); | ||
| break; | ||
| } |
There was a problem hiding this comment.
Nit) what about enabling this code only in debug mode? Or it doesn't matter?
There was a problem hiding this comment.
I agree! The patch is not ready yet
src/interpreter/ByteCode.h
Outdated
| @@ -30,6 +30,7 @@ namespace Walrus { | |||
| class FunctionType; | |||
|
|
|||
| #define FOR_EACH_BYTECODE_OP(F) \ | |||
| F(Nop) \ | |||
There was a problem hiding this comment.
Does it have any side effects?
| @@ -2475,6 +2476,22 @@ class Throw : public ByteCode { | |||
| uint32_t m_tagIndex; | |||
| uint32_t m_offsetsSize; | |||
| }; | |||
| #if !defined(NDEBUG) | |||
| @@ -1446,7 +1446,14 @@ ByteCodeStackOffset* Interpreter::interpret(ExecutionState& state, | |||
| ASSERT_NOT_REACHED(); | |||
| NEXT_INSTRUCTION(); | |||
| } | |||
|
|
|||
| #if !defined(NDEBUG) | |||
vorosl
force-pushed
the
nop
branch
3 times, most recently
from
38b674d to
3ee510a
Compare
| @@ -78,6 +79,58 @@ class FunctionType; | |||
| F(Store32) \ | |||
| F(Store64) \ | |||
| F(FillOpcodeTable) | |||
| #else /* !NDEBUG */ | |||
| #define FOR_EACH_BYTECODE_OP(F) \ | |||
There was a problem hiding this comment.
Duplicating the entire macro is not a good idea.
What about defining a F_NOP macro which is empty or F(Nop) depending on debug?
Signed-off-by: Laszlo Voros <[email protected]>
I've implemented Nop, it is useful for debugging.