Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix crash in Beats on x86-64, minor other fixes #18131

Merged
merged 4 commits into from
Sep 11, 2023
Merged

Fix crash in Beats on x86-64, minor other fixes #18131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3acbeb3
Add an assert in JitBlockCache
hrydgard Sep 11, 2023
875112e
Improve an assert
hrydgard Sep 11, 2023
57edb9f
Gate the PPGE texture windowing trick behind the software rendering s…
hrydgard Sep 11, 2023
accd9b1
sc instruction: Make sure the rt register is mapped. Fixes Beats.
hrydgard Sep 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Common/GPU/Vulkan/VulkanMemory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ VulkanPushPool::Block VulkanPushPool::CreateBlock(size_t size) {
_assert_(result == VK_SUCCESS);

result = vmaMapMemory(vulkan_->Allocator(), block.allocation, (void **)(&block.writePtr));
_assert_(result == VK_SUCCESS);
_assert_msg_(result == VK_SUCCESS, "VulkanPushPool: Failed to map memory (result = %08x)", result);

_assert_msg_(block.writePtr != nullptr, "VulkanPushPool: Failed to map memory on block of size %d", (int)block.size);
return block;
Expand Down
2 changes: 2 additions & 0 deletions Core/MIPS/JitCommon/JitBlockCache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,8 @@ static void ExpandRange(std::pair<u32, u32> &range, u32 newStart, u32 newEnd) {
void JitBlockCache::FinalizeBlock(int block_num, bool block_link) {
JitBlock &b = blocks_[block_num];

_assert_msg_(Memory::IsValidAddress(b.originalAddress), "FinalizeBlock: Bad originalAddress %08x in block %d", b.originalAddress, block_num);

b.originalFirstOpcode = Memory::Read_Opcode_JIT(b.originalAddress);
MIPSOpcode opcode = GetEmuHackOpForBlock(block_num);
Memory::Write_Opcode_JIT(b.originalAddress, opcode);
Expand Down
1 change: 1 addition & 0 deletions Core/MIPS/x86/CompLoadStore.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,6 +429,7 @@ namespace MIPSComp {
skipStore = J_CC(CC_NE);

CompITypeMemWrite(op, 32, safeMemFuncs.writeU32);
gpr.MapReg(rt, true, true);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, this looks wrong. You can't map in a conditional, it could corrupt other regs from the perspective of the other condition. This needs to happen before the J_CC?

-[Unknown]

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the reg is only written to in both branches, I actually think it should be fine here?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But hm, I'll clean it up.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because another reg might spill. If it only spills in one conditional, then that other reg is corrupted.

-[Unknown]

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A problem is that CompITypeMemWrite itself locks and unlocks regs, so I can't reliably do it that way, can I?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I think the actual bug is in CompITypeMemWrite, in the gpr.R(rt).IsImm() case, since normally we'd end up with rt being mapped coming out of it.

I'll fix that.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, not so much a bug as CompITypeMemWrite just not being designed to leave rt writable, fixable with a flag.

MOV(32, gpr.R(rt), Imm32(1));
finish = J();

Expand Down
2 changes: 1 addition & 1 deletion Core/Util/PPGeDraw.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,7 @@ static void PPGeResetCurrentText() {
// Draws some text using the one font we have in the atlas.
void PPGeDrawCurrentText(u32 color) {
// If the atlas is larger than 512x512, need to use windows into it.
bool useTextureWindow = atlasWidth > 512 || atlasHeight > 512;
bool useTextureWindow = g_Config.bSoftwareRendering && atlasWidth > 512 || atlasHeight > 512;
uint32_t texturePosX = 0;
uint32_t texturePosY = 0;

Expand Down