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 CI for macos-latest #5142

Merged
merged 1 commit into from
May 24, 2024

Conversation

matetokodi
Copy link
Contributor

Add a non-inlineable function for negating a number on macos, bacause due to a compiler bug r = -r was being optimized out, resulting in some modulo operations failing such as -1 % -1 or -1 % 1.

Disable -Wliteral-range for test-math.c on macos, because it was falsely raised for isnan, isinf, and isfinite macros.

Copy link
Member

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -370,7 +373,11 @@ ecma_number_remainder (ecma_number_t left_num, /**< left operand */

if (ecma_number_is_zero (r) && ecma_number_is_negative (left_num))
{
#ifdef __APPLE__
r = ecma_number_negate (r);
#else /* !__APPLE__ */
Copy link
Member

Choose a reason for hiding this comment

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

Is this the right macro guard to use? If this is a compiler bug then is it OK to use a guard on the platform? Which versions of which compiler are affected? Is there an issue report for this anywhere that could be linked for reference? Is there no cli option for the compiler to ensure that signed/negative zeros are properly handled, to work around the issue without code base change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This bug only occurs on macos-14, which by default uses Clang, Version: 15.0.0.15000040
this seems to be describing the same problem: llvm/llvm-project#82744 (for the first issue, I was unable to find a report for the second)

These issues first popped up when macos-latest in github actions switched from macos-13 to macos-14.
The easiest fix for now without a codebase change would be to go back to using macos-13 for the time being, and switch back to macos-latest at a leter date when these issues have hopefully been resolved.

Copy link
Member

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

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

LGTM

Due to compiler bugs present in the latest version of clang on macos:

Roll back macos version used by github actions from `macos-latest`
(`macos-14`) to `macos-13`: Some modulo operations were failing,
such as `-1 % -1` when compiler optimizations were enabled

Disable `-Wliteral-range` for test-math.c on macos, because it was
falsely raised for `isnan`, `isinf`, and `isfinite ` macros.

JerryScript-DCO-1.0-Signed-off-by: Máté Tokodi [email protected]
Copy link
Member

@akosthekiss akosthekiss left a comment

Choose a reason for hiding this comment

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

lgtm

@akosthekiss akosthekiss merged commit 35465ed into jerryscript-project:master May 24, 2024
26 checks passed
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