Skip to content

Conversation

@vasyoid
Copy link
Contributor

@vasyoid vasyoid commented Jun 18, 2020

No description provided.

Comment on lines 1264 to 1278
__ bind(Lnan1);
if (double_precision) {
__ fmind(F10_RET, F10_ARG0, F10_ARG0);
} else {
__ fmins(F10_RET, F10_ARG0, F10_ARG0);
}
__ mv(R2_SP, R21_sender_SP);
__ ret();

__ bind(Lnan2);
if (double_precision) {
__ fmind(F10_RET, F11_ARG1, F11_ARG1);
} else {
__ fmins(F10_RET, F11_ARG1, F11_ARG1);
}
Copy link
Member

Choose a reason for hiding this comment

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

https://docs.oracle.com/javase/7/docs/api/java/lang/Math.html#min(double,%20double)
-- If either value is NaN, then the result is NaN
Are you trying to canonicalize the value?
Spec doesn't impose which NaN should it be, I think it safe to return any (the NaN argument for example)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

replaced fmind/fmins with fmvd/fmvs.

Comment on lines 812 to 813
do_intrinsic(_min, java_lang_Math, min_name, int2_int_signature, F_S) \
do_intrinsic(_max, java_lang_Math, max_name, int2_int_signature, F_S) \
Copy link
Member

Choose a reason for hiding this comment

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

It's better to preserve original identifiers even if we would have min and minL. Various shared code could depend on names (JIT compilers for example)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Restored _min/_max and abs symbols.

Comment on lines 192 to 194
case vmIntrinsics::_dabs : return java_lang_math_absD ;
#else
case vmIntrinsics::_dabs : return java_lang_math_abs ;
Copy link
Member

Choose a reason for hiding this comment

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

same here, let's stick to original identifiers

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