Skip to content

Fix #570: Honor #![no_builtins] attribute by passing -fno-builtin to GCC#854

Merged
antoyo merged 6 commits intorust-lang:masterfrom
harin-ramesh:issue-570-fix
Mar 27, 2026
Merged

Fix #570: Honor #![no_builtins] attribute by passing -fno-builtin to GCC#854
antoyo merged 6 commits intorust-lang:masterfrom
harin-ramesh:issue-570-fix

Conversation

@harin-ramesh
Copy link
Copy Markdown
Contributor

@harin-ramesh harin-ramesh commented Feb 21, 2026

This fixes #570

@rustbot

This comment has been minimized.

Sign in to view
@antoyo
Copy link
Copy Markdown
Contributor

antoyo commented Feb 21, 2026

Rebasing on master should fix the CI.

@rustbot

This comment has been minimized.

Sign in to view
antoyo
antoyo requested changes Feb 22, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@antoyo antoyo left a comment

Choose a reason for hiding this comment

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

Could you please add a test for this?
Thanks!

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Feb 22, 2026

⚠️ Warning ⚠️

  • There are issue links (such as #123) in the commit messages of the following commits.
    Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.

@harin-ramesh
Copy link
Copy Markdown
Contributor Author

harin-ramesh commented Feb 22, 2026

image I don’t think adding the flag will resolve the issue. It appears the -fno-builtin flag isn’t valid GIMPLE. I initially missed the warning, but I can now see memset present in the linked object file. Sorry for the oversight.

@antoyo
Copy link
Copy Markdown
Contributor

antoyo commented Feb 22, 2026

It might require context.add_driver_option("-fno-builtin"); instead to have an effect.

@harin-ramesh
Copy link
Copy Markdown
Contributor Author

harin-ramesh commented Feb 22, 2026

image still getting same, am I missing something

@antoyo
Copy link
Copy Markdown
Contributor

antoyo commented Feb 22, 2026

Oh sorry, I didn't see the libgccjit warning in the first image.
It seems this flag is not currently supported for the jit frontend.
Let me check if that can be fixed easily.

@antoyo
Copy link
Copy Markdown
Contributor

antoyo commented Feb 22, 2026

It seems this flag mostly do one thing: disable a pass.
So, please try this flag instead -fno-tree-loop-distribute-patterns.

@harin-ramesh
Copy link
Copy Markdown
Contributor Author

harin-ramesh commented Feb 23, 2026

it seems working with -fno-tree-loop-distribute-patterns, I’ve updated the code and added tests—please review.

antoyo
antoyo reviewed Feb 23, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@antoyo antoyo left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

Thanks!
A few changes to make:

View changes since this review

@harin-ramesh
Copy link
Copy Markdown
Contributor Author

harin-ramesh commented Mar 4, 2026

@antoyo just a heads-up that I’ve addressed all the feedback. Whenever you have a moment, I’d appreciate a fresh look at the PR. Thanks!

@antoyo
Copy link
Copy Markdown
Contributor

antoyo commented Mar 4, 2026

I'm busy these days, so I might not get to your PRs until next week.
Sorry about the delay and thanks for your work.

antoyo
antoyo reviewed Mar 26, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@antoyo antoyo left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

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

This is good.
We only need to adjust the CI to run this test.
Sorry for the delay for reviewing this PR.

View changes since this review

build_system/src/test.rs
test_libcore(env, args)?;
extended_sysroot_tests(env, args)?;
cargo_tests(env, args)?;
no_builtins_tests(env, args)?;
Copy link
Copy Markdown
Contributor

@antoyo antoyo Mar 26, 2026

Choose a reason for hiding this comment

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

The CI doesn't run tests through run_all, so please also add the flag to run these tests on this line.
Do not add it to matrix.commands since those tests are fast and as such, we don't need a separate CI job for them.

@antoyo antoyo merged commit daf8bb9 into rust-lang:master Mar 27, 2026
38 checks passed
@antoyo
Copy link
Copy Markdown
Contributor

antoyo commented Mar 27, 2026

Thanks for your contribution!

@rustbot rustbot mentioned this pull request Mar 27, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@antoyo antoyo antoyo approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

#![no_builtins] attribute isn't honored

3 participants

@harin-ramesh @rustbot @antoyo