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.

Sign up for GitHub

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

Jump to bottom

Remove -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang #44842

Closed
kees opened this issue Apr 10, 2020 · 10 comments
Closed

Remove -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang #44842

kees opened this issue Apr 10, 2020 · 10 comments
Assignees
@kees
Labels
bugzilla Issues migrated from bugzilla c clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl'

Comments

@kees
Copy link
Contributor

kees commented Apr 10, 2020

Bugzilla Link 45497
Version trunk
OS Linux
CC @dwblaikie,@DougGregor,@ramosian-glider,@kcc,@jfbastien,@MattPD,@zygoloid,@seanm,@stephenhines,@TNorthover

Extended Description

Using -ftrivial-auto-var-init=zero is preferred by the Linux kernel for both semantics and performance.

It's worth noting that Linus Torvalds would very much like to see zero-init be a by-default change to how the Linux kernel's C "dialect" behaves:
https://lore.kernel.org/lkml/CAHk-=wgTM+cN7zyUZacGQDv3DuuoA4LORNPWgb1Y_Z1p4iedNQ@mail.gmail.com/
"So I'd like the zeroing of local variables to be a native compiler option..."
"This, btw, is why I also think that the "initialize with poison" is
pointless and wrong."

The performance issue is also very real (as noted in the thread above too). Zero init is faster, and for making such a large change to the kernel, we want to more efficient option. Can we please remove -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang ?
@dwblaikie
Copy link
Collaborator

This will deserve a cfe-dev thread, with all the people who originally asked/requested/required this feature - and I'd agree with the original sentiment: We don't want to create such a C dialect.

@kees
Copy link
Contributor Author

kees commented Apr 21, 2020

Thread kicked off here:
http://lists.llvm.org/pipermail/cfe-dev/2020-April/065221.html

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@AaronBallman
Copy link
Collaborator

Two years have passed and -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang still hasn't been removed. I tried to see if there was a conclusion in the mailing list thread, but I didn't see a consensus position (please correct me if I'm wrong).

What needs to happen to remove the feature flag that we've told people we're absolutely going to be removing at some point?

@EugeneZelenko EugeneZelenko added the clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' label May 5, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented May 5, 2022

@llvm/issue-subscribers-clang-driver

@nickdesaulniers
Copy link
Member

https://reviews.llvm.org/D125142

@jfbastien
Copy link
Member

IIRC from the original commit, the expectation was that we’d have numbers showing that the performance difference just can’t be fixed between zero and pattern. My initial numbers had a pretty substantial effort to narrow that gap, and I don’t think there remains much performance gap to be closed.

@zygoloid had separately proposed a new IR node which would either zero or trap (when UB can be proven), but the patch was never advanced.

kees added a commit that referenced this issue Oct 2, 2022
@kees
[clang][auto-init] Deprecate -enable-trivial-auto-var-init-zero-knowi…
aef03c9 
…ng-it-will-be-removed-from-clang

GCC 12 has been released and contains unconditional support for
-ftrivial-auto-var-init=zero:
https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#index-ftrivial-auto-var-init

Maintain compatibility with GCC, and remove the -enable flag for "zero"
mode. The flag is left to generate an "unused" warning, though, to not
break all the existing users. The flag will be fully removed in Clang 17.

Link: #44842

Reviewed By: nickdesaulniers, MaskRay, srhines, xbolva00

Differential Revision: https://reviews.llvm.org/D125142
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Feb 10, 2023
@intel-lab-lkp
rust: allow to use INIT_STACK_ALL_ZERO
af6fb58 
With CONFIG_INIT_STACK_ALL_ZERO enabled, bindgen passes
-ftrivial-auto-var-init=zero to clang, that triggers the following
error:

 error: '-ftrivial-auto-var-init=zero' hasn't been enabled; enable it at your own peril for benchmarking purpose only with '-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang'

However, this additional option that is currently required by clang is
going to be removed in the future (as the name of the option suggests),
likely with clang-17.

So, make sure bindgen is using this extra option if the major version of
the libclang used by bindgen is < 17.

In this way we can enable CONFIG_INIT_STACK_ALL_ZERO with CONFIG_RUST
without triggering any build error.

Link: llvm/llvm-project#44842
Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
ojeda pushed a commit to Rust-for-Linux/linux that referenced this issue Apr 6, 2023
@ojeda
rust: allow to use INIT_STACK_ALL_ZERO
1921ea5 
With CONFIG_INIT_STACK_ALL_ZERO enabled, bindgen passes
-ftrivial-auto-var-init=zero to clang, that triggers the following
error:

 error: '-ftrivial-auto-var-init=zero' hasn't been enabled; enable it at your own peril for benchmarking purpose only with '-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang'

However, this additional option that is currently required by clang is
deprecated since clang-16 and going to be removed in the future (as
the name of the option suggests), likely with clang-18.

So, make sure bindgen is using this extra option if the major version of
the libclang used by bindgen is < 16.

In this way we can enable CONFIG_INIT_STACK_ALL_ZERO with CONFIG_RUST
without triggering any build error.

Link: llvm/llvm-project#44842
Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
[Changed to < 16 and reworded]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda pushed a commit to Rust-for-Linux/linux that referenced this issue Apr 6, 2023
@ojeda
rust: allow to use INIT_STACK_ALL_ZERO
adad7a4 
With CONFIG_INIT_STACK_ALL_ZERO enabled, bindgen passes
-ftrivial-auto-var-init=zero to clang, that triggers the following
error:

 error: '-ftrivial-auto-var-init=zero' hasn't been enabled; enable it at your own peril for benchmarking purpose only with '-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang'

However, this additional option that is currently required by clang is
deprecated since clang-16 and going to be removed in the future (as
the name of the option suggests), likely with clang-18.

So, make sure bindgen is using this extra option if the major version of
the libclang used by bindgen is < 16.

In this way we can enable CONFIG_INIT_STACK_ALL_ZERO with CONFIG_RUST
without triggering any build error.

Link: llvm/llvm-project#44842
Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
[Changed to < 16 and reworded]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda pushed a commit to Rust-for-Linux/linux that referenced this issue Apr 9, 2023
@ojeda
rust: allow to use INIT_STACK_ALL_ZERO
6327df6 
With CONFIG_INIT_STACK_ALL_ZERO enabled, bindgen passes
-ftrivial-auto-var-init=zero to clang, that triggers the following
error:

 error: '-ftrivial-auto-var-init=zero' hasn't been enabled; enable it at your own peril for benchmarking purpose only with '-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang'

However, this additional option that is currently required by clang is
deprecated since clang-16 and going to be removed in the future,
likely with clang-18.

So, make sure bindgen is using this extra option if the major version of
the libclang used by bindgen is < 16.

In this way we can enable CONFIG_INIT_STACK_ALL_ZERO with CONFIG_RUST
without triggering any build error.

Link: llvm/llvm-project#44842
Link: https://github.com/llvm/llvm-project/blob/llvmorg-16.0.0-rc2/clang/docs/ReleaseNotes.rst#deprecated-compiler-flags
Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
[Changed to < 16 and reworded]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda pushed a commit to Rust-for-Linux/linux that referenced this issue Apr 9, 2023
@ojeda
rust: allow to use INIT_STACK_ALL_ZERO
8197cc3 
With CONFIG_INIT_STACK_ALL_ZERO enabled, bindgen passes
-ftrivial-auto-var-init=zero to clang, that triggers the following
error:

 error: '-ftrivial-auto-var-init=zero' hasn't been enabled; enable it at your own peril for benchmarking purpose only with '-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang'

However, this additional option that is currently required by clang is
deprecated since clang-16 and going to be removed in the future,
likely with clang-18.

So, make sure bindgen is using this extra option if the major version of
the libclang used by bindgen is < 16.

In this way we can enable CONFIG_INIT_STACK_ALL_ZERO with CONFIG_RUST
without triggering any build error.

Link: llvm/llvm-project#44842
Link: https://github.com/llvm/llvm-project/blob/llvmorg-16.0.0-rc2/clang/docs/ReleaseNotes.rst#deprecated-compiler-flags
Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
[Changed to < 16, added link and reworded]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda pushed a commit to Rust-for-Linux/linux that referenced this issue Apr 19, 2023
@ojeda
rust: allow to use INIT_STACK_ALL_ZERO
d966c3c 
With CONFIG_INIT_STACK_ALL_ZERO enabled, bindgen passes
-ftrivial-auto-var-init=zero to clang, that triggers the following
error:

 error: '-ftrivial-auto-var-init=zero' hasn't been enabled; enable it at your own peril for benchmarking purpose only with '-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang'

However, this additional option that is currently required by clang is
deprecated since clang-16 and going to be removed in the future,
likely with clang-18.

So, make sure bindgen is using this extra option if the major version of
the libclang used by bindgen is < 16.

In this way we can enable CONFIG_INIT_STACK_ALL_ZERO with CONFIG_RUST
without triggering any build error.

Link: llvm/llvm-project#44842
Link: https://github.com/llvm/llvm-project/blob/llvmorg-16.0.0-rc2/clang/docs/ReleaseNotes.rst#deprecated-compiler-flags
Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
[Changed to < 16, added link and reworded]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
dcambie pushed a commit to dcambie/linux that referenced this issue Apr 27, 2023
rust: allow to use INIT_STACK_ALL_ZERO
462b93b 
With CONFIG_INIT_STACK_ALL_ZERO enabled, bindgen passes
-ftrivial-auto-var-init=zero to clang, that triggers the following
error:

 error: '-ftrivial-auto-var-init=zero' hasn't been enabled; enable it at your own peril for benchmarking purpose only with '-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang'

However, this additional option that is currently required by clang is
deprecated since clang-16 and going to be removed in the future,
likely with clang-18.

So, make sure bindgen is using this extra option if the major version of
the libclang used by bindgen is < 16.

In this way we can enable CONFIG_INIT_STACK_ALL_ZERO with CONFIG_RUST
without triggering any build error.

Link: llvm/llvm-project#44842
Link: https://github.com/llvm/llvm-project/blob/llvmorg-16.0.0-rc2/clang/docs/ReleaseNotes.rst#deprecated-compiler-flags
Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
[Changed to < 16, added link and reworded]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
pratiksampat pushed a commit to pratiksampat/linux that referenced this issue Jun 19, 2023
rust: allow to use INIT_STACK_ALL_ZERO
f717fe2 
With CONFIG_INIT_STACK_ALL_ZERO enabled, bindgen passes
-ftrivial-auto-var-init=zero to clang, that triggers the following
error:

 error: '-ftrivial-auto-var-init=zero' hasn't been enabled; enable it at your own peril for benchmarking purpose only with '-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang'

However, this additional option that is currently required by clang is
deprecated since clang-16 and going to be removed in the future,
likely with clang-18.

So, make sure bindgen is using this extra option if the major version of
the libclang used by bindgen is < 16.

In this way we can enable CONFIG_INIT_STACK_ALL_ZERO with CONFIG_RUST
without triggering any build error.

Link: llvm/llvm-project#44842
Link: https://github.com/llvm/llvm-project/blob/llvmorg-16.0.0-rc2/clang/docs/ReleaseNotes.rst#deprecated-compiler-flags
Signed-off-by: Andrea Righi <andrea.righi@canonical.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
[Changed to < 16, added link and reworded]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
@thesamesam
Copy link
Member

@kees aef03c9 says it'll be removed in LLVM 17 - is there a commit for that?

@AaronBallman
Copy link
Collaborator

@kees aef03c9 says it'll be removed in LLVM 17 - is there a commit for that?

It was augmented here to say Clang 18, which is what the final 16.x release notes said:

llvm-project/clang/docs/ReleaseNotes.rst

Line 507 in 7cbf1a2

- ``-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang``

Now that we've branched 17.x, we're clear to start removing the option for 18.

@kees
Copy link
Contributor Author

kees commented Sep 1, 2023

Thanks for the reminder! I've proposed it here: https://reviews.llvm.org/D159373

@kees
Copy link
Contributor Author

kees commented Sep 4, 2023

Finished in commit 00e54d0.

@kees kees closed this as completed Sep 4, 2023
@thesamesam thesamesam mentioned this issue Oct 28, 2023
Closed
zeta96 added a commit to zeta96/L_soul_santoni_msm4.9 that referenced this issue Mar 17, 2024
@zeta96
enable clang zero start from clang 17
61e367c 
from:
llvm/llvm-project#44842

llvm/llvm-project@aef03c9
zeta96 added a commit to zeta96/L_soul_santoni_msm4.9 that referenced this issue Mar 18, 2024
@zeta96
Makefile : Enable Clang's zero from clang 17 and later
224dcbe 
from:
llvm/llvm-project#44842

llvm/llvm-project@aef03c9
zeta96 added a commit to zeta96/L_soul_santoni_msm4.9 that referenced this issue Mar 18, 2024
@zeta96
Makefile : Enable Clang's zero from clang 17 and later
bc8a8b4 
from:
llvm/llvm-project#44842

llvm/llvm-project@aef03c9
zeta96 added a commit to zeta96/L_soul_santoni_msm4.9 that referenced this issue Mar 19, 2024
@zeta96
Makefile : Enable Clang's zero from clang 17 and later
4d4fe11 
from:
llvm/llvm-project#44842

llvm/llvm-project@aef03c9
zeta96 added a commit to zeta96/L_soul_santoni_msm4.9 that referenced this issue Mar 21, 2024
@zeta96
Makefile : Enable Clang's zero from clang 17 and later
d4a7ca3 
from:
llvm/llvm-project#44842

llvm/llvm-project@aef03c9
veselypeta pushed a commit to veselypeta/cherillvm that referenced this issue May 22, 2024
@kees @veselypeta
[clang][auto-init] Deprecate -enable-trivial-auto-var-init-zero-knowi…
fcc5091 
…ng-it-will-be-removed-from-clang

GCC 12 has been released and contains unconditional support for
-ftrivial-auto-var-init=zero:
https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html#index-ftrivial-auto-var-init

Maintain compatibility with GCC, and remove the -enable flag for "zero"
mode. The flag is left to generate an "unused" warning, though, to not
break all the existing users. The flag will be fully removed in Clang 17.

Link: llvm/llvm-project#44842

Reviewed By: nickdesaulniers, MaskRay, srhines, xbolva00

Differential Revision: https://reviews.llvm.org/D125142
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kees kees

Labels
bugzilla Issues migrated from bugzilla c clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl'
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@jfbastien @kees @nickdesaulniers @AaronBallman @EugeneZelenko @thesamesam @dwblaikie @llvmbot