Skip to content

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

Merged
antoyo merged 6 commits into
rust-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 into
rust-lang:masterfrom
harin-ramesh:issue-570-fix

Conversation

@harin-ramesh

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

Copy link
Copy Markdown
Contributor

This fixes #570

@rustbot

This comment has been minimized.

Sign in to view
@antoyo

antoyo commented Feb 21, 2026

Copy link
Copy Markdown
Contributor

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

@antoyo antoyo left a comment

Copy link
Copy Markdown
Contributor

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

rustbot commented Feb 22, 2026

Copy link
Copy Markdown
Collaborator

⚠️ 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

harin-ramesh commented Feb 22, 2026

Copy link
Copy Markdown
Contributor Author
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

antoyo commented Feb 22, 2026

Copy link
Copy Markdown
Contributor

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

@harin-ramesh

harin-ramesh commented Feb 22, 2026

Copy link
Copy Markdown
Contributor Author
image still getting same, am I missing something

@antoyo

antoyo commented Feb 22, 2026

Copy link
Copy Markdown
Contributor

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

antoyo commented Feb 22, 2026

Copy link
Copy Markdown
Contributor

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

@harin-ramesh

harin-ramesh commented Feb 23, 2026

Copy link
Copy Markdown
Contributor Author

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

@antoyo antoyo left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

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

Comment thread tests/no_builtins/no_builtins.rs Outdated
Comment thread src/base.rs
Comment thread build_system/src/test.rs Outdated
@harin-ramesh

harin-ramesh commented Mar 4, 2026

Copy link
Copy Markdown
Contributor Author

@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

antoyo commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

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

@antoyo antoyo left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

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

Comment thread build_system/src/test.rs
test_libcore(env, args)?;
extended_sysroot_tests(env, args)?;
cargo_tests(env, args)?;
no_builtins_tests(env, args)?;

@antoyo antoyo Mar 26, 2026

Copy link
Copy Markdown
Contributor

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

antoyo commented Mar 27, 2026

Copy link
Copy Markdown
Contributor

Thanks for your contribution!

@rustbot rustbot mentioned this pull request Mar 27, 2026
Open
@antoyo antoyo mentioned this pull request Apr 9, 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