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.
This commit is contained in:
PixelyIon 2021-03-25 21:39:44 +05:30 committed by ◱ Mark
parent 3f7373209a
commit 8056b80073
7 changed files with 19 additions and 10 deletions

View File

@ -949,6 +949,17 @@
<inspection_tool class="SimplifiableEqualsExpression" enabled="true" level="WARNING" enabled_by_default="true" /> <inspection_tool class="SimplifiableEqualsExpression" enabled="true" level="WARNING" enabled_by_default="true" />
<inspection_tool class="SimplifiableJUnitAssertion" enabled="true" level="WARNING" enabled_by_default="true" /> <inspection_tool class="SimplifiableJUnitAssertion" enabled="true" level="WARNING" enabled_by_default="true" />
<inspection_tool class="SimplifiedTestNGAssertion" enabled="true" level="WARNING" enabled_by_default="true" /> <inspection_tool class="SimplifiedTestNGAssertion" enabled="true" level="WARNING" enabled_by_default="true" />
<inspection_tool class="Simplify" enabled="true" level="WARNING" enabled_by_default="true">
<option name="clangTidyCheckOptions">
<list>
<ClangTidyCheckOption>
<option name="optionName" value="clion-simplify.SimplifyIfWithReturn" />
<option name="optionValue" value="1" />
</ClangTidyCheckOption>
</list>
</option>
<option name="enableSimplifyIfWithReturn" value="true" />
</inspection_tool>
<inspection_tool class="SimplifyNestedEachInScopeFunction" enabled="false" level="WEAK WARNING" enabled_by_default="false" /> <inspection_tool class="SimplifyNestedEachInScopeFunction" enabled="false" level="WEAK WARNING" enabled_by_default="false" />
<inspection_tool class="SingleCharacterStartsWith" enabled="true" level="WARNING" enabled_by_default="true" /> <inspection_tool class="SingleCharacterStartsWith" enabled="true" level="WARNING" enabled_by_default="true" />
<inspection_tool class="SingleClassImport" enabled="true" level="WARNING" enabled_by_default="true" /> <inspection_tool class="SingleClassImport" enabled="true" level="WARNING" enabled_by_default="true" />

View File

@ -4,7 +4,7 @@
#include <perfetto.h> #include <perfetto.h>
#include <common.h> #include <common.h>
#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__)); \ ctx.event()->set_name(skyline::util::Format(formatString, __VA_ARGS__)); \
}) })

View File

@ -610,15 +610,15 @@ namespace skyline::kernel::svc {
i64 timeout{static_cast<i64>(state.ctx->gpr.x3)}; i64 timeout{static_cast<i64>(state.ctx->gpr.x3)};
if (waitHandles.size() == 1) { if (waitHandles.size() == 1) {
state.logger->Debug("Waiting on 0x{:X} for {}ns", waitHandles[0], timeout); 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) { } else if (Logger::LogLevel::Debug <= state.logger->configLevel) {
std::string handleString; std::string handleString;
for (const auto &handle : waitHandles) for (const auto &handle : waitHandles)
handleString += fmt::format("* 0x{:X}\n", handle); handleString += fmt::format("* 0x{:X}\n", handle);
state.logger->Debug("Waiting on handles:\n{}Timeout: {}ns", handleString, timeout); 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); std::unique_lock lock(type::KSyncObject::syncObjectMutex);
if (state.thread->cancelSync) { if (state.thread->cancelSync) {
state.thread->cancelSync = false; state.thread->cancelSync = false;

View File

@ -80,7 +80,7 @@ namespace skyline::soc::gm20b::engine::maxwell3d {
throw exception("Cannot branch while inside a delay slot"); throw exception("Cannot branch while inside a delay slot");
u32 value{registers[opcode->srcA]}; 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 (branch) {
if (opcode->noDelay) { if (opcode->noDelay) {

View File

@ -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 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 { class MacroInterpreter {
private: private:

View File

@ -99,8 +99,6 @@ namespace skyline::soc::gm20b::engine::maxwell3d {
else if (shadowRegisters.mme.shadowRamControl == Registers::MmeShadowRamControl::MethodReplay) else if (shadowRegisters.mme.shadowRamControl == Registers::MmeShadowRamControl::MethodReplay)
params.argument = shadowRegisters.raw[params.method]; params.argument = shadowRegisters.raw[params.method];
#define MAXWELL3D_OFFSET(field) U32_OFFSET(Registers, field)
switch (params.method) { switch (params.method) {
case MAXWELL3D_OFFSET(mme.instructionRamLoad): case MAXWELL3D_OFFSET(mme.instructionRamLoad):
if (registers.mme.instructionRamPointer >= macroCode.size()) if (registers.mme.instructionRamPointer >= macroCode.size())
@ -138,8 +136,6 @@ namespace skyline::soc::gm20b::engine::maxwell3d {
registers.raw[0xD00] = 1; registers.raw[0xD00] = 1;
break; break;
} }
#undef MAXWELL3D_OFFSET
} }
void Maxwell3D::HandleSemaphoreCounterOperation() { void Maxwell3D::HandleSemaphoreCounterOperation() {

View File

@ -6,6 +6,8 @@
#include "engine.h" #include "engine.h"
#include "maxwell/macro_interpreter.h" #include "maxwell/macro_interpreter.h"
#define MAXWELL3D_OFFSET(field) U32_OFFSET(Registers, field)
namespace skyline::soc::gm20b::engine::maxwell3d { namespace skyline::soc::gm20b::engine::maxwell3d {
/** /**
* @brief The Maxwell 3D engine handles processing 3D graphics * @brief The Maxwell 3D engine handles processing 3D graphics
@ -555,7 +557,7 @@ namespace skyline::soc::gm20b::engine::maxwell3d {
Registers registers{}; Registers registers{};
Registers shadowRegisters{}; //!< The shadow registers, their function is controlled by the 'shadowRamControl' register Registers shadowRegisters{}; //!< The shadow registers, their function is controlled by the 'shadowRamControl' register
std::array<u32, 0x10000> macroCode{}; //!< This stores GPU macros, the 256kb size is from Ryujinx std::array<u32, 0x10000> macroCode{}; //!< This stores GPU macros, the 256KiB size is from Ryujinx
Maxwell3D(const DeviceState &state); Maxwell3D(const DeviceState &state);