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.

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

Reduced lower scope for local objects #18581

Merged
merged 3 commits into from
Dec 20, 2023
Merged

Conversation

GermanAizek
Copy link
Contributor

@hrydgard,
Reducing scope object leads to early destruction, as well as faster local variables for conditional branches. There is no reason to create an object earlier before conditional branching, which in some ways does not access to object.

@@ -508,6 +507,7 @@ static void DataProcessingRegister(uint32_t w, uint64_t addr, Instruction *instr
if (opn < 4 && Ra == 31) {
snprintf(instr->text, sizeof(instr->text), "%cmull x%d, w%d, w%d", sign, Rd, Rn, Rm);
} else {
const char* opnames[8] = { 0, 0, "maddl", "msubl", "smulh", 0, 0, 0 };
Copy link
Owner

Choose a reason for hiding this comment

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

while at it, can make this static const char * const

Copy link
Contributor Author

Choose a reason for hiding this comment

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

check this fix in here: 2a31f8c

int ls0 = id.Bits(VS_BIT_LS0, 2);
int ls1 = id.Bits(VS_BIT_LS1, 2);

if (uvgMode) desc << uvgModes[uvgMode];
if (uvgMode) {
const char* uvgModes[4] = { "UV ", "UVMtx ", "UVEnv ", "UVUnk " };
Copy link
Owner

Choose a reason for hiding this comment

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

same

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed it here commit: ca94de8

// Better specular
// Vec3f toViewer = (viewer - pos).NormalizedOr001(cpu_info.bSSE4_1);

if (doSpecular) {
// Real PSP specular
Vec3f toViewer(0, 0, 1);
Copy link
Owner

Choose a reason for hiding this comment

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

make static const

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed it here ca94de8

const int mipLevel = value & 0xFFFF;
const int biasFixed = (s8)(value >> 16);
const float bias = (float)biasFixed / 16.0f;

if (mipLevel == 0 || mipLevel == 1 || mipLevel == 2) {
const char* mipLevelModes[] = {
Copy link
Owner

Choose a reason for hiding this comment

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

same, and more in this file

Copy link
Owner

Choose a reason for hiding this comment

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

Also it's not really needed to move those into the closest scope, since they'll be preinitialized anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix it here 95f535d

@hrydgard hrydgard merged commit dd1396e into hrydgard:master Dec 20, 2023
18 checks passed
if (uri.NavigateUp()) {
std::string newDirName = uri.GetLastPart();
Copy link
Owner

@hrydgard hrydgard Dec 20, 2023

Choose a reason for hiding this comment

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

This change broke directory creation, because uri.NavigateUp actually changes the URI object, so the order changes the outcome. Fix coming up

Copy link
Collaborator

@unknownbrackets unknownbrackets left a comment

Choose a reason for hiding this comment

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

I'm not sure I've seen a compiler where many of these would be any better.

My personal preference would be to instead optimize for code readability rather than a trash toy compiler. Maybe even as an added benefit, find a way to make PPSSPP fail to even compile on a compiler so bad that the scope of static const char *foo[] changes generated code performance.

-[Unknown]

@@ -364,7 +364,6 @@ namespace SaveState
}

// Memory is a bit tricky when jit is enabled, since there's emuhacks in it.
auto savedReplacements = SaveAndClearReplacements();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change looks very bad and probably broke save states?

-[Unknown]

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah, sigh. Should have reviewed this one more closely.

@@ -619,7 +619,6 @@ void ClearCallback() {

static void FinishRecording() {
// We're done - this was just to write the result out.
Path filename = WriteRecording();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can't be good. Certainly broke frame dumps.

-[Unknown]

Copy link
Owner

Choose a reason for hiding this comment

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

Yup, already reverted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants