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

compilers/clike: Speedup cross_compute_int #13926

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sp1ritCS
Copy link
Contributor

Expand the expression passed into cross_compute_int using the preprocessor first and then try to evaluate the expanded expression using the host machine compiler and test if the result is valid.

See #13910 for previous discussion

@sp1ritCS sp1ritCS requested a review from dcbaker as a code owner November 19, 2024 13:38
@sp1ritCS sp1ritCS force-pushed the speedup_cross_compute_int branch 2 times, most recently from 357fc38 to 95390c0 Compare November 20, 2024 13:38
Copy link
Member

@nirbheek nirbheek left a comment

Choose a reason for hiding this comment

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

LGTM now, can we rebase and check CI?

@nirbheek
Copy link
Member

nirbheek commented Jan 9, 2025

Also could you please verify that this codepath is indeed tested on the CI in one of the cross-compile jobs? We also need to ensure that the old codepath is hit in one of the other jobs.

@nirbheek nirbheek added this to the 1.7 milestone Jan 9, 2025
@sp1ritCS
Copy link
Contributor Author

LGTM now, can we rebase and check CI?

Given the CI failures, this isn't ready to merge yet. Methinks I'll need to catch the exception thrown by get_define in cross_compute_int

@sp1ritCS sp1ritCS force-pushed the speedup_cross_compute_int branch 2 times, most recently from 8c25d74 to 2e70987 Compare January 10, 2025 09:14
Expand the expression passed into cross_compute_int using the
preprocessor first and then try to evaluate the expanded expression
using the host machine compiler and test if the result is valid.

Co-authored-by: Charles Brunet <[email protected]>
@sp1ritCS sp1ritCS force-pushed the speedup_cross_compute_int branch from 3c4fdd8 to e8c3fb1 Compare January 10, 2025 09:16
@sp1ritCS
Copy link
Contributor Author

Also could you please verify that this codepath is indeed tested on the CI in one of the cross-compile jobs? We also need to ensure that the old codepath is hit in one of the other jobs.

that'd mean common/134 compute int will have to be ran with a cross file without exe_wrapper. How would I do that? Both branches should be hit (sizeof(int) won't work with get_define).

@nirbheek
Copy link
Member

You want to add such a test to unittests/linuxcrosstests.py that will let you customize it the way you want.

@sp1ritCS sp1ritCS force-pushed the speedup_cross_compute_int branch from e8c3fb1 to 0834eb0 Compare January 13, 2025 11:42
@sp1ritCS sp1ritCS requested a review from jpakkane as a code owner January 13, 2025 11:42
@sp1ritCS sp1ritCS force-pushed the speedup_cross_compute_int branch from 0834eb0 to e8c3fb1 Compare January 13, 2025 21:36
Copy link
Member

@nirbheek nirbheek left a comment

Choose a reason for hiding this comment

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

LGTM, will merge on Friday if there are no objections.

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.

4 participants