From 8a33b814d68eec30fd5973bf853389b6e66a5f6b Mon Sep 17 00:00:00 2001 From: kobalicek Date: Sun, 8 Jan 2023 13:50:35 +0100 Subject: [PATCH] [Bug] Assign inline comments to Invoke/Func nodes, annotate without Logger --- .github/workflows/build.yml | 2 +- src/asmjit/arm/a64assembler.cpp | 8 ++------ src/asmjit/core/builder.cpp | 4 +--- src/asmjit/core/builder_p.h | 35 ++++++++++++++++++++++++++++++++ src/asmjit/core/compiler.cpp | 32 +++++++++++++++++++---------- src/asmjit/core/emitter.h | 24 ++++++++++++++++++++++ src/asmjit/core/emitterutils.cpp | 4 +--- src/asmjit/core/rapass.cpp | 7 +++++-- src/asmjit/x86/x86assembler.cpp | 9 ++------ src/asmjit/x86/x86instapi.cpp | 3 +++ tools/tablegen-x86.js | 3 ++- 11 files changed, 97 insertions(+), 34 deletions(-) create mode 100644 src/asmjit/core/builder_p.h diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 38a48a4..2442548 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -14,7 +14,7 @@ jobs: steps: - name: "Checkout" - uses: actions/checkout@v2 + uses: actions/checkout@v3 - name: "Setup node.js" uses: actions/setup-node@v1 diff --git a/src/asmjit/arm/a64assembler.cpp b/src/asmjit/arm/a64assembler.cpp index 4e39546..ec698de 100644 --- a/src/asmjit/arm/a64assembler.cpp +++ b/src/asmjit/arm/a64assembler.cpp @@ -4997,9 +4997,7 @@ EmitDone: #endif } - resetExtraReg(); - resetInstOptions(); - resetInlineComment(); + resetState(); writer.done(this); return kErrorOk; @@ -5025,9 +5023,7 @@ Failed: #ifndef ASMJIT_NO_LOGGING return EmitterUtils::logInstructionFailed(this, err, instId, options, o0, o1, o2, opExt); #else - resetExtraReg(); - resetInstOptions(); - resetInlineComment(); + resetState(); return reportError(err); #endif } diff --git a/src/asmjit/core/builder.cpp b/src/asmjit/core/builder.cpp index ad2cf7a..77f94e7 100644 --- a/src/asmjit/core/builder.cpp +++ b/src/asmjit/core/builder.cpp @@ -597,9 +597,7 @@ Error BaseBuilder::_emit(InstId instId, const Operand_& o0, const Operand_& o1, #ifndef ASMJIT_NO_LOGGING return EmitterUtils::logInstructionFailed(this, err, instId, options, o0, o1, o2, opExt); #else - resetInstOptions(); - resetExtraReg(); - resetInlineComment(); + resetState(); return reportError(err); #endif } diff --git a/src/asmjit/core/builder_p.h b/src/asmjit/core/builder_p.h new file mode 100644 index 0000000..303358f --- /dev/null +++ b/src/asmjit/core/builder_p.h @@ -0,0 +1,35 @@ +// This file is part of AsmJit project +// +// See asmjit.h or LICENSE.md for license and copyright information +// SPDX-License-Identifier: Zlib + +#ifndef ASMJIT_CORE_BUILDER_P_H_INCLUDED +#define ASMJIT_CORE_BUILDER_P_H_INCLUDED + +#include "../core/api-config.h" +#ifndef ASMJIT_NO_BUILDER + +#include "../core/builder.h" + +ASMJIT_BEGIN_NAMESPACE + +//! \addtogroup asmjit_builder +//! \{ + +static inline void BaseBuilder_assignInlineComment(BaseBuilder* self, BaseNode* node, const char* comment) noexcept { + if (comment) + node->setInlineComment(static_cast(self->_dataZone.dup(comment, strlen(comment), true))); +} + +static inline void BaseBuilder_assignInstState(BaseBuilder* self, InstNode* node, const BaseEmitter::State& state) noexcept { + node->setOptions(state.options); + node->setExtraReg(state.extraReg); + BaseBuilder_assignInlineComment(self, node, state.comment); +} + +//! \} + +ASMJIT_END_NAMESPACE + +#endif // !ASMJIT_NO_BUILDER +#endif // ASMJIT_CORE_BUILDER_P_H_INCLUDED diff --git a/src/asmjit/core/compiler.cpp b/src/asmjit/core/compiler.cpp index b1c6b80..ee959f7 100644 --- a/src/asmjit/core/compiler.cpp +++ b/src/asmjit/core/compiler.cpp @@ -7,6 +7,7 @@ #ifndef ASMJIT_NO_COMPILER #include "../core/assembler.h" +#include "../core/builder_p.h" #include "../core/compiler.h" #include "../core/cpuinfo.h" #include "../core/logger.h" @@ -103,9 +104,13 @@ Error BaseCompiler::newFuncNode(FuncNode** out, const FuncSignature& signature) } Error BaseCompiler::addFuncNode(FuncNode** out, const FuncSignature& signature) { + State state = _grabState(); + ASMJIT_PROPAGATE(newFuncNode(out, signature)); ASMJIT_ASSUME(*out != nullptr); + BaseBuilder_assignInlineComment(this, *out, state.comment); + addFunc(*out); return kErrorOk; } @@ -127,7 +132,13 @@ Error BaseCompiler::newFuncRetNode(FuncRetNode** out, const Operand_& o0, const } Error BaseCompiler::addFuncRetNode(FuncRetNode** out, const Operand_& o0, const Operand_& o1) { + State state = _grabState(); + ASMJIT_PROPAGATE(newFuncRetNode(out, o0, o1)); + ASMJIT_ASSUME(*out != nullptr); + + BaseBuilder_assignInlineComment(this, *out, state.comment); + addNode(*out); return kErrorOk; } @@ -146,6 +157,7 @@ FuncNode* BaseCompiler::addFunc(FuncNode* func) { Error BaseCompiler::endFunc() { FuncNode* func = _func; + resetState(); if (ASMJIT_UNLIKELY(!func)) return reportError(DebugUtils::errored(kErrorInvalidState)); @@ -196,7 +208,12 @@ Error BaseCompiler::newInvokeNode(InvokeNode** out, InstId instId, const Operand } Error BaseCompiler::addInvokeNode(InvokeNode** out, InstId instId, const Operand_& o0, const FuncSignature& signature) { + State state = _grabState(); + ASMJIT_PROPAGATE(newInvokeNode(out, instId, o0, signature)); + ASMJIT_ASSUME(*out != nullptr); + + BaseBuilder_assignInstState(this, *out, state); addNode(*out); return kErrorOk; } @@ -481,20 +498,13 @@ Error BaseCompiler::newJumpNode(JumpNode** out, InstId instId, InstOptions instO } Error BaseCompiler::emitAnnotatedJump(InstId instId, const Operand_& o0, JumpAnnotation* annotation) { - InstOptions options = instOptions() | forcedInstOptions(); - RegOnly extra = extraReg(); - const char* comment = inlineComment(); - - resetInstOptions(); - resetInlineComment(); - resetExtraReg(); + State state = _grabState(); JumpNode* node; - ASMJIT_PROPAGATE(newJumpNode(&node, instId, options, o0, annotation)); + ASMJIT_PROPAGATE(newJumpNode(&node, instId, state.options, o0, annotation)); - node->setExtraReg(extra); - if (comment) - node->setInlineComment(static_cast(_dataZone.dup(comment, strlen(comment), true))); + node->setExtraReg(state.extraReg); + BaseBuilder_assignInlineComment(this, node, state.comment); addNode(node); return kErrorOk; diff --git a/src/asmjit/core/emitter.h b/src/asmjit/core/emitter.h index b8afd6b..6499c07 100644 --- a/src/asmjit/core/emitter.h +++ b/src/asmjit/core/emitter.h @@ -233,6 +233,13 @@ public: //! Native GP register signature and signature related information. OperandSignature _gpSignature {}; + //! Emitter state that can be used to specify options and inline comment of a next node or instruction. + struct State { + InstOptions options; + RegOnly extraReg; + const char* comment; + }; + //! Next instruction options (affects the next instruction). InstOptions _instOptions = InstOptions::kNone; //! Extra register (op-mask {k} on AVX-512) (affects the next instruction). @@ -530,6 +537,23 @@ public: //! \} + //! \name Emitter State + //! \{ + + inline void resetState() noexcept { + resetInstOptions(); + resetExtraReg(); + resetInlineComment(); + } + + inline State _grabState() noexcept { + State s{_instOptions | _forcedInstOptions, _extraReg, _inlineComment}; + resetState(); + return s; + } + + //! \} + //! \name Sections //! \{ diff --git a/src/asmjit/core/emitterutils.cpp b/src/asmjit/core/emitterutils.cpp index da12217..d0a6872 100644 --- a/src/asmjit/core/emitterutils.cpp +++ b/src/asmjit/core/emitterutils.cpp @@ -116,9 +116,7 @@ Error logInstructionFailed( sb.append(self->inlineComment()); } - self->resetInstOptions(); - self->resetExtraReg(); - self->resetInlineComment(); + self->resetState(); return self->reportError(err, sb.data()); } diff --git a/src/asmjit/core/rapass.cpp b/src/asmjit/core/rapass.cpp index a1a4490..29b7dea 100644 --- a/src/asmjit/core/rapass.cpp +++ b/src/asmjit/core/rapass.cpp @@ -114,11 +114,14 @@ Error BaseRAPass::runOnFunction(Zone* zone, Logger* logger, FuncNode* func) { #ifndef ASMJIT_NO_LOGGING _logger = logger; _formatOptions.reset(); - _diagnosticOptions = DiagnosticOptions::kNone; + _diagnosticOptions = _cb->diagnosticOptions(); if (logger) { _formatOptions = logger->options(); - _diagnosticOptions = _cb->diagnosticOptions(); + } + else { + _diagnosticOptions &= ~(DiagnosticOptions::kRADebugCFG | + DiagnosticOptions::kRADebugUnreachable); } #else DebugUtils::unused(logger); diff --git a/src/asmjit/x86/x86assembler.cpp b/src/asmjit/x86/x86assembler.cpp index 4b07049..ee2f746 100644 --- a/src/asmjit/x86/x86assembler.cpp +++ b/src/asmjit/x86/x86assembler.cpp @@ -4950,10 +4950,7 @@ EmitDone: #endif } - resetExtraReg(); - resetInstOptions(); - resetInlineComment(); - + resetState(); writer.done(this); return kErrorOk; @@ -4987,9 +4984,7 @@ Failed: #ifndef ASMJIT_NO_LOGGING return EmitterUtils::logInstructionFailed(this, err, instId, options, o0, o1, o2, opExt); #else - resetExtraReg(); - resetInstOptions(); - resetInlineComment(); + resetState(); return reportError(err); #endif } diff --git a/src/asmjit/x86/x86instapi.cpp b/src/asmjit/x86/x86instapi.cpp index 0bf7b89..410e009 100644 --- a/src/asmjit/x86/x86instapi.cpp +++ b/src/asmjit/x86/x86instapi.cpp @@ -794,6 +794,9 @@ Error InstInternal::queryRWInfo(Arch arch, const BaseInst& inst, const Operand_* if (ASMJIT_UNLIKELY(!Inst::isDefinedId(instId))) return DebugUtils::errored(kErrorInvalidInstruction); + if (instId == Inst::kIdPop) + printf("HERE\n"); + // Read/Write flags. const InstDB::InstInfo& instInfo = InstDB::_instInfoTable[instId]; const InstDB::CommonInfo& commonInfo = InstDB::_commonInfoTable[instInfo._commonInfoIndex]; diff --git a/tools/tablegen-x86.js b/tools/tablegen-x86.js index 26ebd03..e96e9cf 100644 --- a/tools/tablegen-x86.js +++ b/tools/tablegen-x86.js @@ -2247,8 +2247,9 @@ class InstRWInfoTable extends core.Task { const operands = dbInst.operands; const rwOps = nullOps(); - for (var j = 0; j < operands.length; j++) + for (var j = 0; j < operands.length; j++) { rwOps[j] = makeRwFromOp(operands[j]) + } var match = 0; for (var j = 0; j < rwOpsArray.length; j++)