Skip to content

Commit

Permalink
Fix control flow analysis bug
Browse files Browse the repository at this point in the history
  • Loading branch information
wheremyfoodat committed Sep 2, 2024
1 parent 0e7697d commit e332ab2
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 14 deletions.
2 changes: 2 additions & 0 deletions include/PICA/draw_acceleration.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ namespace PICA {
u32 size;
u32 stride;

u8 inputReg; // Which input reg should this attribute go to in the vertex shader?
u8 type;
u8 componentCount;
bool fixed;
Expand All @@ -27,6 +28,7 @@ namespace PICA {
// Minimum and maximum index in the index buffer for a draw call
u16 minimumIndex, maximumIndex;
u32 totalAttribCount;
u32 enabledAttributeMask;
u32 vertexDataSize;

std::array<AttributeInfo, maxAttribCount> attributeInfo;
Expand Down
3 changes: 3 additions & 0 deletions include/renderer_gl/renderer_gl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ class RendererGL final : public Renderer {
GLuint maximumIndex = 0;
void* hwIndexBufferOffset = nullptr;

// When doing hw shaders, we cache which attributes are enabled in our VAO to avoid having to enable/disable all attributes on each draw
u32 previousAttributeMask = 0;

// Cached pointer to the current vertex shader when using HW accelerated shaders
OpenGL::Shader* generatedVertexShader = nullptr;

Expand Down
12 changes: 11 additions & 1 deletion src/core/PICA/draw_acceleration.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
void GPU::getAcceleratedDrawInfo(PICA::DrawAcceleration& accel, bool indexed) {
accel.indexed = indexed;
accel.totalAttribCount = totalAttribCount;

accel.enabledAttributeMask = 0;

const u32 vertexBase = ((regs[PICA::InternalRegs::VertexAttribLoc] >> 1) & 0xfffffff) * 16;
const u32 vertexCount = regs[PICA::InternalRegs::VertexCountReg]; // Total # of vertices to transfer

Expand Down Expand Up @@ -50,6 +51,8 @@ void GPU::getAcceleratedDrawInfo(PICA::DrawAcceleration& accel, bool indexed) {
}

const u64 vertexCfg = u64(regs[PICA::InternalRegs::AttribFormatLow]) | (u64(regs[PICA::InternalRegs::AttribFormatHigh]) << 32);
const u64 inputAttrCfg = getVertexShaderInputConfig();

u32 buffer = 0;
u32 attrCount = 0;
accel.vertexDataSize = 0;
Expand Down Expand Up @@ -94,7 +97,11 @@ void GPU::getAcceleratedDrawInfo(PICA::DrawAcceleration& accel, bool indexed) {

// Size of each component based on the attribute type
static constexpr u32 sizePerComponent[4] = {1, 1, 2, 4};
const u32 inputReg = (inputAttrCfg >> (attrCount * 4)) & 0xf;
// Mark the attribute as enabled
accel.enabledAttributeMask |= 1 << inputReg;

attr.inputReg = inputReg;
attr.componentCount = size;
attr.offset = attributeOffset;
attr.size = size * sizePerComponent[attribType];
Expand Down Expand Up @@ -123,6 +130,9 @@ void GPU::getAcceleratedDrawInfo(PICA::DrawAcceleration& accel, bool indexed) {
attr.fixedValue[i] = fixedAttr[i].toFloat32();
}

const u32 inputReg = (inputAttrCfg >> (attrCount * 4)) & 0xf;

attr.inputReg = inputReg;
attrCount += 1;
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/core/PICA/shader_decompiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ ExitMode ControlFlow::analyzeFunction(const PICAShader& shader, u32 start, u32 e

// This opens up 2 parallel paths of execution
auto branchTakenExit = analyzeFunction(shader, dest, end, labels);
auto branchNotTakenExit = analyzeFunction(shader, pc + 1, dest, labels);
auto branchNotTakenExit = analyzeFunction(shader, pc + 1, end, labels);
it->second = exitParallel(branchTakenExit, branchNotTakenExit);
return it->second;
}
Expand Down Expand Up @@ -122,6 +122,7 @@ ExitMode ControlFlow::analyzeFunction(const PICAShader& shader, u32 start, u32 e
}
break;
}

case ShaderOpcodes::CALL: {
const u32 num = instruction & 0xff;
const u32 dest = getBits<10, 12>(instruction);
Expand Down
2 changes: 0 additions & 2 deletions src/core/PICA/shader_gen_glsl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -778,8 +778,6 @@ void main() {
gl_ClipDistance[1] = dot(clipCoords, a_coords);
#endif
})";

std::cout << ret << "\n";
return ret;
}
}
46 changes: 36 additions & 10 deletions src/core/renderer_gl/renderer_gl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <stb_image_write.h>

#include <bit>
#include <cmrc/cmrc.hpp>

#include "PICA/float_types.hpp"
Expand Down Expand Up @@ -987,7 +988,7 @@ bool RendererGL::prepareForDraw(ShaderUnit& shaderUnit, PICA::DrawAcceleration*
shaderUnit.vs, *emulatorConfig, shaderUnit.vs.entrypoint, PICA::ShaderGen::API::GL, PICA::ShaderGen::Language::GLSL
);

// Empty source means compilation error, if the source is not empty then we convert the rcompiled PICA code into a valid shader and upload
// Empty source means compilation error, if the source is not empty then we convert the recompiled PICA code into a valid shader and upload
// it to the GPU
if (!picaShaderSource.empty()) {
std::string vertexShaderSource = fragShaderGen.getVertexShaderAccelerated(picaShaderSource, vertexConfig, usingUbershader);
Expand Down Expand Up @@ -1167,24 +1168,49 @@ void RendererGL::accelerateVertexUpload(ShaderUnit& shaderUnit, PICA::DrawAccele

gl.bindVAO(hwShaderVAO);

// Enable or disable vertex attributes as needed
const u32 currentAttributeMask = accel->enabledAttributeMask;
// Use bitwise xor to calculate which attributes chanced
u32 attributeMaskDiff = currentAttributeMask ^ previousAttributeMask;

while (attributeMaskDiff != 0) {
// Get index of next different attribute and turn it off
const u32 index = 31 - std::countl_zero<u32>(attributeMaskDiff);
const u32 mask = 1u << index;
attributeMaskDiff ^= mask;

if ((currentAttributeMask & mask) != 0) {
// Attribute was disabled and is now enabled
hwShaderVAO.enableAttribute(index);
} else {
// Attribute was enabled and is now disabled
hwShaderVAO.disableAttribute(index);
}
}

previousAttributeMask = currentAttributeMask;

for (int i = 0; i < totalAttribCount; i++) {
const auto& attrib = accel->attributeInfo[i];

if (attrib.fixed) {
Helpers::panic("Fixed attribute!");
if ((currentAttributeMask & (1u << i)) == 0) {
glVertexAttrib4f(attrib.inputReg, attrib.fixedValue[0], attrib.fixedValue[1], attrib.fixedValue[2], attrib.fixedValue[3]);
}
} else {
if (attrib.isPadding) {
if (attrib.isPadding) [[unlikely]] {
continue;
}

glVertexAttribPointer(i, attrib.componentCount, attributeFormats[attrib.type], GL_FALSE, attrib.stride, reinterpret_cast<GLvoid*>(vertexBufferRes.buffer_offset + attrib.offset));
// TODO: Disable unused attributes as well
hwShaderVAO.enableAttribute(i);


const u32 attributeSize = attrib.size * vertexCount;
std::memcpy(vertexData, attrib.data, attributeSize);

vertexData += attributeSize;

glVertexAttribPointer(
attrib.inputReg, attrib.componentCount, attributeFormats[attrib.type], GL_FALSE, attrib.stride,
reinterpret_cast<GLvoid*>(vertexBufferRes.buffer_offset + attrib.offset)
);
}
}

Expand Down

0 comments on commit e332ab2

Please sign in to comment.