Merge pull request #898 from alfalfa-star/feat/agc-cond-indirect-buffer

feat(agc): run COND_INDIRECT_BUFFER branches whose comparison always passes
This commit is contained in:
Adrià 2026-10-07 18:26:59 +02:00 committed by GitHub
commit d95ce1f29c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 74 additions and 3 deletions

View File

@ -18,9 +18,9 @@ std::uint32_t* APS5_VABI sceAgcCbBranch(CommandBuffer* buf, std::uint8_t mode, s
const auto address = reinterpret_cast<std::uintptr_t>(compareAddress);
const auto first = reinterpret_cast<std::uintptr_t>(buffer1);
const auto second = reinterpret_cast<std::uintptr_t>(buffer2);
Agc::Command::CheckAddress(address, 8, __func__);
Agc::Command::CheckAddress(first, 4, __func__);
Agc::Command::CheckAddress(second, 4, __func__);
if (compareFunction != 0) Agc::Command::CheckAddress(address, 8, __func__);
if (first != 0 || sizeInDwords1 != 0) Agc::Command::CheckAddress(first, 4, __func__);
if (second != 0 || sizeInDwords2 != 0) Agc::Command::CheckAddress(second, 4, __func__);
return Agc::Command::Emit(buf, 0x3fu, {mode | (static_cast<std::uint32_t>(compareFunction) << 8u), static_cast<std::uint32_t>(address), static_cast<std::uint32_t>(address >> 32u), static_cast<std::uint32_t>(mask), static_cast<std::uint32_t>(mask >> 32u), static_cast<std::uint32_t>(reference), static_cast<std::uint32_t>(reference >> 32u), static_cast<std::uint32_t>(first), static_cast<std::uint32_t>(first >> 32u), sizeInDwords1 | (static_cast<std::uint32_t>(cachePolicy1) << 28u), static_cast<std::uint32_t>(second), static_cast<std::uint32_t>(second >> 32u), sizeInDwords2 | (static_cast<std::uint32_t>(cachePolicy2) << 28u)}, __func__);
}

View File

@ -58,6 +58,21 @@ void testWriter() {
check(storage.packet[expected.size()] == 0xabcdef01u, "branch command overwrote following word");
}
void testUnusedFields() {
std::array<std::uint32_t, 64> words{};
CommandBuffer buffer{words.data(), words.data() + words.size(), words.data(), words.data() + words.size(), nullptr, nullptr, 0};
auto* packet = sceAgcCbBranch(&buffer, 1, 0, compareAt(0x0000001234567894ull), 0, 0, 0, targetAt(0x0000002233445564ull), 0x123u, 0, nullptr, 0);
const std::array expected{0xc00c3f00u, 0x1u, 0x34567894u, 0x12u, 0u, 0u, 0u, 0u, 0x33445564u, 0x22u, 0x123u, 0u, 0u, 0u};
check(std::equal(expected.begin(), expected.end(), packet), "an always-taken branch without an else target was not written");
packet = sceAgcCbBranch(&buffer, 1, 0, nullptr, 0, 0, 0, nullptr, 0, 0, nullptr, 0);
check(packet[1] == 1u && packet[2] == 0u && packet[8] == 0u && packet[10] == 0u, "an empty always-taken branch was not written");
const auto cursor = buffer.cursor_up;
expectFailure([&] { sceAgcCbBranch(&buffer, 1, 3, compareAt(0x0000001234567894ull), 0, 0, 0, targetAt(0x2000u), 1, 0, nullptr, 0); });
expectFailure([&] { sceAgcCbBranch(&buffer, 1, 0, nullptr, 0, 0, 0, nullptr, 1, 0, nullptr, 0); });
expectFailure([&] { sceAgcCbBranch(&buffer, 2, 0, nullptr, 0, 0, 0, targetAt(0x2000u), 1, 0, nullptr, 1); });
check(buffer.cursor_up == cursor, "a rejected branch advanced the command buffer");
}
void testCompareAddress() {
Storage storage;
auto expected = storage.words;
@ -109,6 +124,7 @@ void testMalformedPackets() {
int main() {
try {
testWriter();
testUnusedFields();
testCompareAddress();
testTarget(sceAgcBranchPatchSetThenTarget, 8, 0x10000000u, "then target setter wrote the wrong words");
testTarget(sceAgcBranchPatchSetElseTarget, 11, 0x20000000u, "else target setter wrote the wrong words");

View File

@ -43,6 +43,23 @@ bool Driver::copySegment(Submission& submission, const std::uint32_t* guest, std
}
reach(cursor, cursor + count);
const auto opcode = (header >> 8u) & 0xffu;
if (opcode == 0x3fu && count == 14) {
const auto mode = guest[cursor + 1] & 3u;
require(mode == 1u || mode == 2u, "invalid COND_INDIRECT_BUFFER mode");
require(((guest[cursor + 1] >> 8u) & 7u) == 0u, "a COND_INDIRECT_BUFFER with a comparison is not implemented");
require(!Pm4::Predicated(header), "a predicated COND_INDIRECT_BUFFER is not implemented");
const auto* target = reinterpret_cast<const std::uint32_t*>(static_cast<std::uintptr_t>(guest[cursor + 8] & ~3u) | (static_cast<std::uintptr_t>(guest[cursor + 9] & 0xffffu) << 32u));
const std::size_t targetWords = guest[cursor + 10] & 0xfffffu;
if (targetWords != 0) {
GuestMemory::CheckRange(target, targetWords * sizeof(std::uint32_t), alignof(std::uint32_t));
if (copySegment(submission, target, targetWords, budget)) {
require(guarded.empty(), "a REWIND inside a conditional execution range is not implemented");
return true;
}
}
cursor += count;
continue;
}
if (opcode == 0x3fu) {
require(count == 4, "invalid INDIRECT_BUFFER size");
const auto* target = reinterpret_cast<const std::uint32_t*>(static_cast<std::uintptr_t>(guest[cursor + 1] & ~3u) | (static_cast<std::uintptr_t>(guest[cursor + 2] & 0xffffu) << 32u));

View File

@ -13,6 +13,7 @@
#include <set>
#include <stdexcept>
#include <thread>
#include <tuple>
#include <vector>
#ifdef _WIN32
#ifndef NOMINMAX
@ -738,6 +739,41 @@ void testConditionalSubmission() {
check(results[13] == 0, "a rejected conditional submission executed a guarded packet");
}
std::vector<std::uint32_t> branch(std::uint32_t mode, std::uint32_t function, const std::vector<std::uint32_t>* first, const std::vector<std::uint32_t>* second) {
const auto address = [](const std::vector<std::uint32_t>* target) { return target ? reinterpret_cast<std::uintptr_t>(target->data()) : std::uintptr_t{0}; };
const auto size = [](const std::vector<std::uint32_t>* target) { return target ? static_cast<std::uint32_t>(target->size()) : 0u; };
return makePacket(0x3f, {mode | (function << 8u), 0, 0, 0, 0, 0, 0, static_cast<std::uint32_t>(address(first)), static_cast<std::uint32_t>(address(first) >> 32u), size(first), static_cast<std::uint32_t>(address(second)), static_cast<std::uint32_t>(address(second) >> 32u), size(second)});
}
void testBranchSubmission() {
static std::array<std::uint32_t, 4> results{};
static std::vector<std::uint32_t> first, second;
results.fill(0);
first = joinPackets({writeWord(results[0], 71)});
second = joinPackets({writeWord(results[1], 72)});
auto words = joinPackets({branch(1, 0, &first, nullptr), writeWord(results[2], 73)});
submitWords(words);
AgcDriverWaitIdle_nid_postfix();
check(results[0] == 71 && results[1] == 0 && results[2] == 73, "an always-taken if-then COND_INDIRECT_BUFFER did not run its buffer");
results.fill(0);
words = joinPackets({branch(2, 0, &first, &second), writeWord(results[2], 74)});
submitWords(words);
AgcDriverWaitIdle_nid_postfix();
check(results[0] == 71 && results[1] == 0 && results[2] == 74, "an always-taken if-then-else COND_INDIRECT_BUFFER ran the wrong buffer");
results.fill(0);
words = joinPackets({branch(1, 0, nullptr, nullptr), writeWord(results[2], 75)});
submitWords(words);
AgcDriverWaitIdle_nid_postfix();
check(results[2] == 75, "an empty COND_INDIRECT_BUFFER skipped the next packet");
for (const auto& [mode, function, text] : {std::tuple{1u, 3u, "with a comparison"}, std::tuple{0u, 0u, "invalid COND_INDIRECT_BUFFER mode"}}) {
results.fill(0);
words = joinPackets({writeWord(results[3], 1), branch(mode, function, &first, nullptr)});
expectFailure([&] { submitWords(words); }, text);
AgcDriverWaitIdle_nid_postfix();
check(results[0] == 0 && results[3] == 0, "a rejected COND_INDIRECT_BUFFER submission executed a packet");
}
}
void testPredicatedSubmission() {
alignas(16) std::uint64_t flag[2] = {0, 0};
alignas(16) std::array<std::uint32_t, 4> written{};
@ -906,6 +942,7 @@ int main(int argc, char** argv) {
testDriverSubmission();
testPredicatedSubmission();
testConditionalSubmission();
testBranchSubmission();
LibcRunShutdown_nid_postfix();
std::puts("PM4 catalog, registers, state, memory, conditional execution and submission tests passed");
return 0;

View File

@ -230,6 +230,7 @@ Throughout the project, every function at every stage either **does exactly what
- [COND_EXEC](../../core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/PacketExecution.cpp) reads its condition after the earlier packets of its queue complete. Without PFP_SYNC_ME the console's graphics PFP reads it ahead of the ME, so a title that races the read sees a later value here.
- A [flip inside a COND_EXEC range](../../core/libs/prx/libSceAgcDriver/Execution/src/Driver/Packets/CommandValidation.cpp) throws: the flip is reserved with the video output when the buffer is submitted, and how the console withdraws a skipped one is unknown.
- A [COND_EXEC range](../../core/libs/prx/libSceAgcDriver/Execution/src/Driver/Queues/Submission.cpp) must hold whole packets and end inside its command buffer. A chained INDIRECT_BUFFER or a REWIND inside a range throws, as what the CP does with the skipped dwords then is unknown.
- A [COND_INDIRECT_BUFFER](../../core/libs/prx/libSceAgcDriver/Execution/src/Driver/Queues/Submission.cpp) (`sceAgcCbBranch`) runs only with the always-pass compare function; a comparison or a predicated packet throws. Modes 1 and 2 run the first buffer, assumed to be the if-then and if-then-else modes of AMD's PM4 packet. [sceAgcCbBranch](../../core/libs/prx/libSceAgc/Cb/src/Branch.cpp) does not check the compare address of an always-pass branch or the address of an empty buffer, as PPSA14632 passes an uninitialised compare address and a null else buffer of 0 dwords.
- [v_mullit_f32](../../core/shader/recompiler/Translation/src/FloatInstructions.cpp) follows the hardware measured with the MODE register at IEEE=1 and denormals kept (a signaling NaN src0 is quieted, a positive denormal src2 counts as positive): the recompiler does not read MODE, so other IEEE/denormal settings are not modelled.
- [v_fma_legacy_f32](../../core/shader/recompiler/Translation/src/FloatInstructions.cpp) (VOP3 0x140) is assumed to be fused, which has not been measured on hardware. LLVM decodes this encoding as `v_mad_legacy_f32` for gfx1013, and `v_mac_legacy_f32` (VOP2 0x06) was measured to round its product before the add, so 0x140 may do the same.
- The executable file that [relinker](../../core/relinker/elfpatcher/src/windows/WindowsPeWriter.cpp) generates opens the console when launched, which is inconvenient for playability.