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

Fix quant/dequant table bug #731

Merged
merged 1 commit into from Aug 5, 2014

Conversation

FioraAeterna
Copy link
Contributor

1<<31 is not 2^31 because 1 is a signed int in C; this bug affected both the
JIT and interpreter quantized store implementations, though I don't know if
any games were actually affected.

@degasus
Copy link
Member

degasus commented Aug 4, 2014

Oh, I've thought you're talking about this table: https://github.com/dolphin-emu/dolphin/blob/master/Source/Core/VideoCommon/VertexLoader.cpp#L67

Why do we have this table so often in memory?

imo it's better to always write "1u << xx", else LGTM

@FioraAeterna
Copy link
Contributor Author

The table's in memory once for the interpreter and once for the JIT, I think.

@FioraAeterna
Copy link
Contributor Author

Changed the tables to a consist 1ULL on Jasper's request.

@degasus
Copy link
Member

degasus commented Aug 4, 2014

@FioraAeterna Nope, all of them: 1ULL << 0, 1ULL << 1, 1ULL << 2, ...

1<<31 is not 2^31 because 1 is a signed int in C; this bug affected both the
JIT and interpreter quantized store implementations, though I don't know if
any games were actually affected.
@degasus
Copy link
Member

degasus commented Aug 4, 2014

LGTM

But tbh, isn't there a nicer way to add an integer to the exponent but a multiplication? Let's hope SSE didn't miss this kind of bit operations. But of course of of scope of this PR.

@FioraAeterna
Copy link
Contributor Author

I'm not sure there's a nicer way; adding/subtracting to the exponent directly isn't any fewer uops and might break terribly in cases like 0, denormals, and so on. I don't think SSE has an equivalent of an integer bitshift on a float.

delroth added a commit that referenced this pull request Aug 5, 2014
@delroth delroth merged commit b6dac8f into dolphin-emu:master Aug 5, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants