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

Rewrite Cop1.cpp to conform to strict-aliasing rules #214

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Souzooka
Copy link
Contributor

@Souzooka Souzooka commented May 3, 2019

…ict-aliasing rules

@Souzooka Souzooka force-pushed the typepunningisntpunny branch from 330b548 to cfc25ed Compare May 3, 2019 05:42
@Souzooka Souzooka force-pushed the typepunningisntpunny branch from cfc25ed to cb956c6 Compare May 3, 2019 05:55
Souzooka added 2 commits May 2, 2019 22:23
this is actually slightly broken right now but someone broke the autotests and I can't find the bug
@Souzooka Souzooka changed the title Rewrite float Cop1::convert(uint32_t) so that it no longer breaks str… Rewrite Cop1.cpp to conform to strict-aliasing rules May 3, 2019
{
switch(value & 0x7F800000)
uint32_t test;
memcpy(&test, &value, sizeof(float));
Copy link

Choose a reason for hiding this comment

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

Could also use union

Copy link

Choose a reason for hiding this comment

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

Valid in C99 (and above) and C++11 (and above)

{
gpr[dest].u = gpr[source].u ^ 0x80000000;
gpr[dest] = -gpr[source];
Copy link
Contributor

Choose a reason for hiding this comment

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

NEG only flips the sign bit, not the whole value, not sure how this would behave, and what if the original value is already negative? surely doing - will keep it the same, which is not a negation

temp.f = sqrt(fabs(convert(gpr[reg2].u)));
gpr[dest].f = convert(gpr[reg1].u) / convert(temp.u);
float temp = sqrtf(fabsf(convert(gpr[reg2])));
gpr[dest] = convert(gpr[reg1]) / convert(temp);
Copy link
Contributor

Choose a reason for hiding this comment

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

why not use ureg1/2 ?

control.i = true;
control.si = true;
}

gpr[dest].f = sqrt(fabs(convert(gpr[source].u)));
gpr[dest] = sqrtf(fabsf(convert(gpr[source])));
Copy link
Contributor

Choose a reason for hiding this comment

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

why not use usource?

Copy link
Contributor

Choose a reason for hiding this comment

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

also why only memcpy back the dest on zero and not in normal operation?

}
else
gpr[dest].f = convert(numerator) / convert(denominator);
gpr[dest] = convert(gpr[reg1]) / convert(gpr[reg2]);
Copy link
Contributor

Choose a reason for hiding this comment

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

numerator / denominator

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