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

JitIL: Fix a bug in floatpoint load/store instructions. #2272

Merged
merged 2 commits into from May 26, 2015

Conversation

phire
Copy link
Member

@phire phire commented Apr 5, 2015

The regBuildMemAddress function already clears the address register.

Not only is clearing it again pointless, regBuildMemAddress uses the bits in IInfo slightly differently and the second clear can clear the wrong register, causing bugs if something else actually needs to use those registers.

Fixes issue 8437 and #2166

The regBuildMemAddress function already clears the address register.
Not only is clearing it again pointless, regBuildMemAddress uses the
bits in IInfo slightly diffrently and the second clear can clear
the wrong registers causing bugs if something else actually needs to
use those registers.
@phire
Copy link
Member Author

@phire phire commented Apr 5, 2015

Add some comments, could you check them @magumagu?

@@ -420,6 +433,7 @@ static void regNormalRegClear(RegInfo& RI, InstLoc I)
regClearInst(RI, getOp2(I));
}

// Clear any floating point registers which end their lifetime at I

This comment has been minimized.

@magumagu
Copy link
Contributor

@magumagu magumagu commented Apr 5, 2015

The change looks correct... as far as it goes. Ideally, the code would be changed so there's only one place where we decide which instructions are actually "used" by an instruction, as in regMarkMemAddress etc.

@@ -412,6 +423,8 @@ static X64Reg regBinLHSReg(RegInfo& RI, InstLoc I)
return reg;
}

// Clear any registers which end their lifetime at I
// Don't use this for special instructions like memory load/stores

This comment has been minimized.

@bb010g
Copy link
Contributor

@bb010g bb010g commented Apr 22, 2015

Bump?

@JMC47
Copy link
Contributor

@JMC47 JMC47 commented May 25, 2015

We need someone to review this? @FioraAeterna @magumagu ? It's important for JITIL to at least be functional.

Sonicadvance1 added a commit that referenced this issue May 26, 2015
JitIL: Fix a bug in floatpoint load/store instructions.
@Sonicadvance1 Sonicadvance1 merged commit 3817dd0 into dolphin-emu:master May 26, 2015
10 checks passed
@phire phire deleted the jitil-floatbug branch Jul 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants