diff --git a/Core/MIPS/IR/IRAnalysis.cpp b/Core/MIPS/IR/IRAnalysis.cpp index 8ce6388de8..0aa303fef6 100644 --- a/Core/MIPS/IR/IRAnalysis.cpp +++ b/Core/MIPS/IR/IRAnalysis.cpp @@ -59,9 +59,9 @@ bool IRReadsFromFPR(const IRInstMeta &inst, int reg, bool *directly) { if (inst.m.types[2] == '2' && reg >= inst.src2 && reg < inst.src2 + 2) return true; if ((inst.m.flags & (IRFLAG_SRC3 | IRFLAG_SRC3DST)) != 0) { - if (inst.m.types[0] == 'V' && reg >= inst.src3 && reg <= inst.src3 + 4) + if (inst.m.types[0] == 'V' && reg >= inst.src3 && reg < inst.src3 + 4) return true; - if (inst.m.types[0] == '2' && reg >= inst.src3 && reg <= inst.src3 + 2) + if (inst.m.types[0] == '2' && reg >= inst.src3 && reg < inst.src3 + 2) return true; } return false; @@ -77,9 +77,8 @@ static int IRReadsFromList(const IRInstMeta &inst, IRReg regs[4], char type) { if ((inst.m.flags & (IRFLAG_SRC3 | IRFLAG_SRC3DST)) != 0 && inst.m.types[0] == type) regs[c++] = inst.src3; - if (inst.op == IROp::Interpret || inst.op == IROp::CallReplacement || inst.op == IROp::Syscall || inst.op == IROp::SyscallUnresolved ||inst.op == IROp::Break) - return -1; - if (inst.op == IROp::Breakpoint || inst.op == IROp::MemoryCheck) + // Exits read every reg that lives on, and barriers may read anything. + if ((inst.m.flags & (IRFLAG_EXIT | IRFLAG_BARRIER)) != 0) return -1; return c; @@ -142,8 +141,8 @@ int IRReadsFromGPRs(const IRInstMeta &inst, IRReg regs[4]) { int IRReadsFromFPRs(const IRInstMeta &inst, IRReg regs[16]) { int c = IRReadsFromList(inst, regs, 'F'); - if (c != 0) - return c; + if (c == -1) + return -1; // We also need to check V and 2. Indirect reads already checked, don't check again. if (inst.m.types[1] == 'V' || inst.m.types[1] == '2') { diff --git a/Core/MIPS/IR/IRPassSimplify.cpp b/Core/MIPS/IR/IRPassSimplify.cpp index e616140521..98274d17fa 100644 --- a/Core/MIPS/IR/IRPassSimplify.cpp +++ b/Core/MIPS/IR/IRPassSimplify.cpp @@ -1007,6 +1007,21 @@ bool PurgeTemps(const IRWriter &in, IRWriter &out, const IROptions &opts) { memset(lastWrittenTo, -1, sizeof(lastWrittenTo)); memset(lastReadFrom, -1, sizeof(lastReadFrom)); + auto writesToFPRCheck = [](const IRInstMeta &inst, const Check &check) { + for (int i = 0; i < check.fplen; ++i) { + if (IRWritesToFPR(inst, check.reg - 32 + i)) + return true; + } + return false; + }; + + auto nukeCheckedInst = [&](Check &check) { + insts[check.index].op = IROp::Mov; + insts[check.index].dest = 0; + insts[check.index].src1 = 0; + check.reg = 0; + }; + auto readsFromFPRCheck = [](IRInstMeta &inst, Check &check, bool *directly) { if (check.reg < 32) return false; @@ -1112,25 +1127,29 @@ bool PurgeTemps(const IRWriter &in, IRWriter &out, const IROptions &opts) { checkMismatch(inst.src1, inst.m.types[1]); checkMismatch(inst.src2, inst.m.types[2]); if ((inst.m.flags & (IRFLAG_SRC3 | IRFLAG_SRC3DST)) != 0) - checkMismatch(inst.src3, inst.m.types[3]); + checkMismatch(inst.src3, inst.m.types[0]); bool cannotReplace = !readsDirectly || lenMismatch; if (!cannotReplace && check.srcReg >= 32 && lastWrittenTo[check.srcReg] < check.index) { // This is probably not worth doing unless we can get rid of a temp. if (!check.readByExit) { - if (insts[check.index].dest == inst.src1) - inst.src1 = check.srcReg - 32; - else if (insts[check.index].dest == inst.src2) - inst.src2 = check.srcReg - 32; - else - _assert_msg_(false, "Unexpected src3 read of FPR"); + // Replace every F operand that reads it (it might be read twice). + const IRReg reg = (IRReg)(check.reg - 32); + const IRReg srcReg = (IRReg)(check.srcReg - 32); + if (inst.m.types[1] == 'F' && inst.src1 == reg) + inst.src1 = srcReg; + if (inst.m.types[2] == 'F' && inst.src2 == reg) + inst.src2 = srcReg; + if ((inst.m.flags & (IRFLAG_SRC3 | IRFLAG_SRC3DST)) != 0 && inst.m.types[0] == 'F' && inst.src3 == reg) + inst.src3 = srcReg; - // Check if we've clobbered it entirely. - if (inst.dest == check.reg) { + // If this also writes the reg, the check ends here. A full overwrite leaves the + // original write dead, since every read in between now uses srcReg. + if (writesToFPRCheck(inst, check)) { + IRReg destFPRs[4]; + if (IRDestFPRs(inst, destFPRs) == check.fplen && inst.dest + 32 == check.reg) + nukeCheckedInst(check); check.reg = 0; - insts[check.index].op = IROp::Mov; - insts[check.index].dest = 0; - insts[check.index].src1 = 0; } } else { // Let's not bother. @@ -1176,17 +1195,14 @@ bool PurgeTemps(const IRWriter &in, IRWriter &out, const IROptions &opts) { insts[check.index].dest = 0; insts[check.index].src1 = 0; check.reg = 0; - } else if (IRWritesToFPR(inst, check.reg - 32) && check.fplen >= 1) { + } else if (check.fplen >= 1 && writesToFPRCheck(inst, check)) { IRReg destFPRs[4]; int numFPRs = IRDestFPRs(inst, destFPRs); if (numFPRs == check.fplen && inst.dest + 32 == check.reg) { // This means we've clobbered it, and with full overlap. // Sometimes this happens for non-temps, i.e. vmmov + vinit last row. - insts[check.index].op = IROp::Mov; - insts[check.index].dest = 0; - insts[check.index].src1 = 0; - check.reg = 0; + nukeCheckedInst(check); } else { // Since there's an overlap, we simply cannot optimize. check.reg = 0; @@ -1230,6 +1246,10 @@ bool PurgeTemps(const IRWriter &in, IRWriter &out, const IROptions &opts) { // These might sometimes be implicitly read/written by other instructions. break; } + if (inst.op == IROp::Store32Conditional || inst.op == IROp::Load32Linked) { + // These do more than write the reg (the store, and LLBIT), so they must stay. + break; + } checks.push_back(Check(dest, i, true)); break; diff --git a/unittest/TestIRPassSimplify.cpp b/unittest/TestIRPassSimplify.cpp index 2f47b80f56..1e0745dd54 100644 --- a/unittest/TestIRPassSimplify.cpp +++ b/unittest/TestIRPassSimplify.cpp @@ -69,7 +69,9 @@ static bool VerifyPass(const IRVerification &v) { continue; } - printf("%s FAILED: #%d expected '%s' but was '%s'", v.name, (int)i, expectedBuf, actualBuf); + printf("%s FAILED: #%d expected '%s' but was '%s'\n", v.name, (int)i, expectedBuf, actualBuf); + printf("Actual:\n"); + LogInstructions(actual); return false; } } @@ -166,15 +168,115 @@ static const IRVerification tests[] = { }, { &PropagateConstants }, }, + { + // The FNeg reads the temp and overwrites it, so the FAdd must see the negation. + "PurgeTempsFPRRewrittenInPlace", + { + { IROp::FMov, { IRVTEMP_PFX_S }, 5 }, + { IROp::FNeg, { IRVTEMP_PFX_S }, IRVTEMP_PFX_S }, + { IROp::FAdd, { 0 }, IRVTEMP_PFX_S, 1 }, + }, + { + { IROp::FNeg, { IRVTEMP_PFX_S }, 5 }, + { IROp::FAdd, { 0 }, IRVTEMP_PFX_S, 1 }, + }, + { &PurgeTemps }, + }, + { + "PurgeTempsFPRReadTwice", + { + { IROp::FMov, { IRVTEMP_PFX_S }, 5 }, + { IROp::FMul, { 0 }, IRVTEMP_PFX_S, IRVTEMP_PFX_S }, + }, + { + { IROp::FMul, { 0 }, 5, 5 }, + }, + { &PurgeTemps }, + }, + { + // The FPR temp has the same number as IRTEMP_0, which is the address here. + "PurgeTempsFPRStoreSrc3", + { + { IROp::FMov, { IRVTEMP_PFX_S }, 5 }, + { IROp::StoreFloat, { IRVTEMP_PFX_S }, IRTEMP_0, 0, 0x10 }, + }, + { + { IROp::StoreFloat, { 5 }, IRTEMP_0, 0, 0x10 }, + }, + { &PurgeTemps }, + }, + { + // The FMov writes lane 1 of the temp between its write and the Vec4Mov. + "PurgeTempsVec4LaneWrite", + { + { IROp::Vec4Add, { IRVTEMP_0 }, 32, 36 }, + { IROp::FMov, { IRVTEMP_0 + 1 }, 20 }, + { IROp::Vec4Mov, { 48 }, IRVTEMP_0 }, + }, + { + { IROp::Vec4Add, { IRVTEMP_0 }, 32, 36 }, + { IROp::FMov, { IRVTEMP_0 + 1 }, 20 }, + { IROp::Vec4Mov, { 48 }, IRVTEMP_0 }, + }, + { &PurgeTemps }, + }, + { + // The Vec4Scale reads 48 before the Vec4Mov writes it. + "PurgeTempsVec4ScaleRead", + { + { IROp::Vec4Add, { IRVTEMP_0 }, 32, 36 }, + { IROp::Vec4Scale, { 40 }, 48, 1 }, + { IROp::Vec4Mov, { 48 }, IRVTEMP_0 }, + }, + { + { IROp::Vec4Add, { IRVTEMP_0 }, 32, 36 }, + { IROp::Vec4Scale, { 40 }, 48, 1 }, + { IROp::Vec4Mov, { 48 }, IRVTEMP_0 }, + }, + { &PurgeTemps }, + }, + { + // Writing 48 before the exit would change it on the path that exits. + "PurgeTempsSwapAcrossExit", + { + { IROp::Vec4Add, { IRVTEMP_0 }, 32, 36 }, + { IROp::ExitToConstIfEq, { 0 }, MIPS_REG_A0, MIPS_REG_A1, 0x08804000 }, + { IROp::Vec4Mov, { 48 }, IRVTEMP_0 }, + }, + { + { IROp::Vec4Add, { IRVTEMP_0 }, 32, 36 }, + { IROp::ExitToConstIfEq, { 0 }, MIPS_REG_A0, MIPS_REG_A1, 0x08804000 }, + { IROp::Vec4Mov, { 48 }, IRVTEMP_0 }, + }, + { &PurgeTemps }, + }, + { + // sc stores and ll sets LLBIT, so neither goes away when the reg is overwritten. + "PurgeTempsKeepsLLSC", + { + { IROp::Load32Linked, { MIPS_REG_V1 }, MIPS_REG_A0, 0, 0 }, + { IROp::SetConst, { MIPS_REG_V1 }, 0, 0, 1 }, + { IROp::Store32Conditional, { MIPS_REG_V0 }, MIPS_REG_A0, 0, 0 }, + { IROp::SetConst, { MIPS_REG_V0 }, 0, 0, 1 }, + }, + { + { IROp::Load32Linked, { MIPS_REG_V1 }, MIPS_REG_A0, 0, 0 }, + { IROp::SetConst, { MIPS_REG_V1 }, 0, 0, 1 }, + { IROp::Store32Conditional, { MIPS_REG_V0 }, MIPS_REG_A0, 0, 0 }, + { IROp::SetConst, { MIPS_REG_V0 }, 0, 0, 1 }, + }, + { &PurgeTemps }, + }, }; bool TestIRPassSimplify() { InitIR(); + bool success = true; for (const auto &test : tests) { if (!VerifyPass(test)) - return false; + success = false; } - return true; + return success; }