From 40910644be367ecbe0dae8212e9d686a24c7fa6c Mon Sep 17 00:00:00 2001 From: PixelyIon Date: Thu, 25 Mar 2021 21:39:44 +0530 Subject: [PATCH] Address CR Comments (#151) + Fix SvcWaitSynchronization Trace An RAII scoped trace was used for SvcWaitSynchronization but it was placed within a condition scope which led to an incorrect lifetime for the traces. Minor changes regarding the CR not affecting functionality were made aside from that. --- .idea/inspectionProfiles/Project_Default.xml | 11 +++++++++++ app/src/main/cpp/skyline/common/trace.h | 2 +- app/src/main/cpp/skyline/kernel/svc.cpp | 4 ++-- .../soc/gm20b/engines/maxwell/macro_interpreter.cpp | 2 +- .../soc/gm20b/engines/maxwell/macro_interpreter.h | 2 +- .../main/cpp/skyline/soc/gm20b/engines/maxwell_3d.cpp | 4 ---- .../main/cpp/skyline/soc/gm20b/engines/maxwell_3d.h | 4 +++- 7 files changed, 19 insertions(+), 10 deletions(-) diff --git a/.idea/inspectionProfiles/Project_Default.xml b/.idea/inspectionProfiles/Project_Default.xml index 82d04e98..34e1abac 100644 --- a/.idea/inspectionProfiles/Project_Default.xml +++ b/.idea/inspectionProfiles/Project_Default.xml @@ -949,6 +949,17 @@ + + + diff --git a/app/src/main/cpp/skyline/common/trace.h b/app/src/main/cpp/skyline/common/trace.h index 78435aa9..2305deb1 100644 --- a/app/src/main/cpp/skyline/common/trace.h +++ b/app/src/main/cpp/skyline/common/trace.h @@ -4,7 +4,7 @@ #include #include -#define TRACE_EVENT_FMT(category, formatString, ...) TRACE_EVENT("kernel", nullptr, [&](perfetto::EventContext ctx) { \ +#define TRACE_EVENT_FMT(category, formatString, ...) TRACE_EVENT(category, nullptr, [&](perfetto::EventContext ctx) { \ ctx.event()->set_name(skyline::util::Format(formatString, __VA_ARGS__)); \ }) diff --git a/app/src/main/cpp/skyline/kernel/svc.cpp b/app/src/main/cpp/skyline/kernel/svc.cpp index 5690f790..c24ad589 100644 --- a/app/src/main/cpp/skyline/kernel/svc.cpp +++ b/app/src/main/cpp/skyline/kernel/svc.cpp @@ -610,15 +610,15 @@ namespace skyline::kernel::svc { i64 timeout{static_cast(state.ctx->gpr.x3)}; if (waitHandles.size() == 1) { state.logger->Debug("Waiting on 0x{:X} for {}ns", waitHandles[0], timeout); - TRACE_EVENT_FMT("kernel", "WaitSynchronization 0x{:X}", waitHandles[0]); } else if (Logger::LogLevel::Debug <= state.logger->configLevel) { std::string handleString; for (const auto &handle : waitHandles) handleString += fmt::format("* 0x{:X}\n", handle); state.logger->Debug("Waiting on handles:\n{}Timeout: {}ns", handleString, timeout); - TRACE_EVENT("kernel", "WaitSynchronizationMultiple"); } + TRACE_EVENT_FMT("kernel", waitHandles.size() == 1 ? "WaitSynchronization 0x{:X}" : "WaitSynchronizationMultiple 0x{:X}", waitHandles[0]); + std::unique_lock lock(type::KSyncObject::syncObjectMutex); if (state.thread->cancelSync) { state.thread->cancelSync = false; diff --git a/app/src/main/cpp/skyline/soc/gm20b/engines/maxwell/macro_interpreter.cpp b/app/src/main/cpp/skyline/soc/gm20b/engines/maxwell/macro_interpreter.cpp index c3706840..ecbc0d7f 100644 --- a/app/src/main/cpp/skyline/soc/gm20b/engines/maxwell/macro_interpreter.cpp +++ b/app/src/main/cpp/skyline/soc/gm20b/engines/maxwell/macro_interpreter.cpp @@ -80,7 +80,7 @@ namespace skyline::soc::gm20b::engine::maxwell3d { throw exception("Cannot branch while inside a delay slot"); u32 value{registers[opcode->srcA]}; - bool branch{(opcode->branchCondition == Opcode::BranchCondition::Zero) == (value == 0)}; + bool branch{(opcode->branchCondition == Opcode::BranchCondition::Zero) ? (value == 0) : (value != 0)}; if (branch) { if (opcode->noDelay) { diff --git a/app/src/main/cpp/skyline/soc/gm20b/engines/maxwell/macro_interpreter.h b/app/src/main/cpp/skyline/soc/gm20b/engines/maxwell/macro_interpreter.h index 7c28fc35..981d76f3 100644 --- a/app/src/main/cpp/skyline/soc/gm20b/engines/maxwell/macro_interpreter.h +++ b/app/src/main/cpp/skyline/soc/gm20b/engines/maxwell/macro_interpreter.h @@ -9,7 +9,7 @@ namespace skyline::soc::gm20b::engine::maxwell3d { class Maxwell3D; // A forward declaration of Maxwell3D as we don't want to import it here /** - * @brief The MacroInterpreter class handles interpreting macros. Macros are small programs that run on the GPU and are used for things like instanced rendering. + * @brief The MacroInterpreter class handles interpreting macros. Macros are small programs that run on the GPU and are used for things like instanced rendering */ class MacroInterpreter { private: diff --git a/app/src/main/cpp/skyline/soc/gm20b/engines/maxwell_3d.cpp b/app/src/main/cpp/skyline/soc/gm20b/engines/maxwell_3d.cpp index bbda7e43..dfdcfa3a 100644 --- a/app/src/main/cpp/skyline/soc/gm20b/engines/maxwell_3d.cpp +++ b/app/src/main/cpp/skyline/soc/gm20b/engines/maxwell_3d.cpp @@ -99,8 +99,6 @@ namespace skyline::soc::gm20b::engine::maxwell3d { else if (shadowRegisters.mme.shadowRamControl == Registers::MmeShadowRamControl::MethodReplay) params.argument = shadowRegisters.raw[params.method]; - #define MAXWELL3D_OFFSET(field) U32_OFFSET(Registers, field) - switch (params.method) { case MAXWELL3D_OFFSET(mme.instructionRamLoad): if (registers.mme.instructionRamPointer >= macroCode.size()) @@ -138,8 +136,6 @@ namespace skyline::soc::gm20b::engine::maxwell3d { registers.raw[0xD00] = 1; break; } - - #undef MAXWELL3D_OFFSET } void Maxwell3D::HandleSemaphoreCounterOperation() { diff --git a/app/src/main/cpp/skyline/soc/gm20b/engines/maxwell_3d.h b/app/src/main/cpp/skyline/soc/gm20b/engines/maxwell_3d.h index 8b1b96f1..fba18554 100644 --- a/app/src/main/cpp/skyline/soc/gm20b/engines/maxwell_3d.h +++ b/app/src/main/cpp/skyline/soc/gm20b/engines/maxwell_3d.h @@ -6,6 +6,8 @@ #include "engine.h" #include "maxwell/macro_interpreter.h" +#define MAXWELL3D_OFFSET(field) U32_OFFSET(Registers, field) + namespace skyline::soc::gm20b::engine::maxwell3d { /** * @brief The Maxwell 3D engine handles processing 3D graphics @@ -555,7 +557,7 @@ namespace skyline::soc::gm20b::engine::maxwell3d { Registers registers{}; Registers shadowRegisters{}; //!< The shadow registers, their function is controlled by the 'shadowRamControl' register - std::array macroCode{}; //!< This stores GPU macros, the 256kb size is from Ryujinx + std::array macroCode{}; //!< This stores GPU macros, the 256KiB size is from Ryujinx Maxwell3D(const DeviceState &state);