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

JIT: don't rely on undefined behavior for constant overflow checking #812

Merged
merged 1 commit into from Aug 15, 2014

Conversation

FioraAeterna
Copy link
Contributor

I have no idea what the compiler does with these, and this code probably
isn't triggered in most games, but it's probably better not to taunt the
undefined behavior demon.

@Tilka
Copy link
Member

Tilka commented Aug 15, 2014

lgtm if you add #include <limits>.

I have no idea what the compiler does with these, and this code probably
isn't triggered in most games, but it's probably better not to taunt the
undefined behavior demon.
@Sonicadvance1
Copy link
Contributor

@FioraAeterna: This comment grants you the permission to merge this pull request whenever you think it is ready. After addressing the remaining comments, click this link to merge.


@dolphin-emu-bot allowmerge

dolphin-emu-bot added a commit that referenced this pull request Aug 15, 2014
JIT: don't rely on undefined behavior for constant overflow checking
@dolphin-emu-bot dolphin-emu-bot merged commit 444e47a into dolphin-emu:master Aug 15, 2014
@comex
Copy link
Contributor

comex commented Aug 15, 2014

That's really ugly. A good compiler will optimize (s64)(i - j) != (s64)i - (s64)j (where i and j are signed) to false. However, to correctly avoid undefined behavior you also need to modify the lines that compute the values, e.g. gpr.SetImmediate32(d, i + j); should have the argument changed to (s32) ((u32) i + (u32) j)).

(Not to butt in too much, but I'd also prefer it if you didn't use the same name, GenerateConstantOverflow, for a different function.)

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