[PATCH] D54604: Automatic variable initialization

glider added a comment. Herald added a project: LLVM. I think doing separate stores of 0xAA into struct members instead of copying part of .rodata over it will let us do a much better job in optimizing away redundant stores. I've opened https://bugs.llvm.org/show_bug.cgi?id=40605 to track that.

[PATCH] D54604: Automatic variable initialization

jfb added a comment. In D54604#1375514 , @glider wrote: > Not sure if a similar problem was mentioned already or not, but the following > program: Yes please file a bug. Seems like the optimizer should sink the store enough that it appears only once on

[PATCH] D54604: Automatic variable initialization

rjmccall added a comment. I think it's absolutely fair game to file bugs for this. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54604/new/ https://reviews.llvm.org/D54604 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D54604: Automatic variable initialization

glider added a comment. (Overall, since this is an "unsupported" feature, is it ok to file Bugzilla bugs for it?) Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54604/new/ https://reviews.llvm.org/D54604 ___ cfe-commi

[PATCH] D54604: Automatic variable initialization

glider added a comment. Not sure if a similar problem was mentioned already or not, but the following program: void alloc_sock(int *err); int try_send(); int send(int len) { int err; if (len) { err = -1; alloc_sock(&err); err = try_send(); } return -1;

[PATCH] D54604: Automatic variable initialization

pcc added inline comments. Comment at: cfe/trunk/include/clang/Driver/Options.td:1657 + " | pattern">, Values<"uninitialized,pattern">; +def enable_trivial_var_init_zero : Joined<["-"], "enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang">, + Flags<[CC1Opt

[PATCH] D54604: Automatic variable initialization

jfb marked an inline comment as done. jfb added inline comments. Comment at: cfe/trunk/include/clang/Driver/Options.td:1657 + " | pattern">, Values<"uninitialized,pattern">; +def enable_trivial_var_init_zero : Joined<["-"], "enable-trivial-auto-var-init-zero-knowing-it-will-be-

[PATCH] D54604: Automatic variable initialization

pcc added inline comments. Comment at: cfe/trunk/include/clang/Driver/Options.td:1657 + " | pattern">, Values<"uninitialized,pattern">; +def enable_trivial_var_init_zero : Joined<["-"], "enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang">, + Flags<[CC1Opt

[PATCH] D54604: Automatic variable initialization

This revision was automatically updated to reflect the committed changes. Closed by commit rL349442: Automatic variable initialization (authored by jfb, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D54604?vs=178566&id=178590#toc Rep

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 178566. jfb added a comment. - Update attribute test. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54604/new/ https://reviews.llvm.org/D54604 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td include/clan

[PATCH] D54604: Automatic variable initialization

pcc accepted this revision. pcc added a comment. This revision is now accepted and ready to land. LGTM Comment at: test/CodeGenCXX/trivial-auto-var-init-attribute.cpp:13 +void test_attribute_uninitialized() { + [[clang::trivial_auto_init("uninitialized")]] int i; + used(i); -

[PATCH] D54604: Automatic variable initialization

rsmith added a comment. Thanks, you've addressed my comments to my satisfaction. (Not approving as I've not done a detailed review of the implementation, only of the approach, but it looks like your review with @pcc is nearly complete.) In D54604#1330356

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 178304. jfb added a comment. - Rebase Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54604/new/ https://reviews.llvm.org/D54604 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td include/clang/Basic/Diagnost

[PATCH] D54604: Automatic variable initialization

jfb added a comment. In D54604#1331711 , @pcc wrote: > Mostly looking good, a few style nits. Thanks! I think I addressed all of your comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54604/new/ https://reviews.ll

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 178301. jfb marked 6 inline comments as done. jfb added a comment. - Address @pcc's comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54604/new/ https://reviews.llvm.org/D54604 Files: include/clang/Basic/Attr.td includ

[PATCH] D54604: Automatic variable initialization

pcc added a comment. Mostly looking good, a few style nits. Comment at: include/clang/Basic/DiagnosticDriverKinds.td:409 + +def err_drv_trivial_auto_var_init_zero_disabled : Warning< + "-ftrivial-auto-var-init=zero hasn't been enabled. Enable it at your own peril for benchmar

[PATCH] D54604: Automatic variable initialization

jfb added a comment. In D54604#1327893 , @rsmith wrote: > For the record: I'm OK with this direction. (I somewhat prefer removing the > `-enable-long-wordy-thing` option and instead automatically disabling the > `zero` option for clang release builds, bu

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 178129. jfb marked 3 inline comments as done. jfb added a comment. - Don't document 'zero'. - Make drv_trivial_auto_var_init_zero_disabled an error instead of a warning. - Change the parameter: we only have [[clang::uninitialized]] now. Repository: rC Clang C

[PATCH] D54604: Automatic variable initialization

rsmith added a comment. For the record: I'm OK with this direction. (I somewhat prefer removing the `-enable-long-wordy-thing` option and instead automatically disabling the `zero` option for clang release builds, but I'm OK with either approach.) Comment at: include/clang/Ba

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 177600. jfb added a comment. - Make sure uninit-variables.c doesn't break. - Address Peter's comments, improve tests. - Add an ugly option to enable zero init - Update warning-flags.c - Fix typo - Use negative NaN with repeated 0xFF payload for all floating-point

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 177341. jfb added a comment. - Fix typo Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54604/new/ https://reviews.llvm.org/D54604 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td include/clang/Basic/Diagno

[PATCH] D54604: Automatic variable initialization

dexonsmith added inline comments. Comment at: include/clang/Basic/DiagnosticDriverKinds.td:412 + "Enable it at your own peril for benchmarking purpose only with " + "-enable-trivial-auto-var-init-zero-knowning-it-will-be-removed-from-clang">; + s/knowning/knowi

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 177331. jfb added a comment. - Update warning-flags.c Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54604/new/ https://reviews.llvm.org/D54604 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td include/clan

[PATCH] D54604: Automatic variable initialization

jfb added a comment. I added an option that's required when using `clang` directly (*not* `-cc1`!) If you use `-ftrivial-auto-var-init=zero` without `-enable-trivial-auto-var-init-zero-knowning-it-will-be-removed-from-clang` in `clang` it'll generate a warning and change initialization to patte

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 177328. jfb added a comment. - Add an ugly option to enable zero init Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54604/new/ https://reviews.llvm.org/D54604 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.t

[PATCH] D54604: Automatic variable initialization

jfb added a comment. I've addressed @pcc's comments. I'll now update the patch to mandate a `-Xclang` option for zero-init (otherwise it'll warn and pattern-init instead), and remove documentation for zero-init. I'm also thinking of changing float init to negative NaN with infinite scream paylo

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 177262. jfb marked 10 inline comments as done. jfb added a comment. - Make sure uninit-variables.c doesn't break. - Address Peter's comments, improve tests. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54604/new/ https://reviews

[PATCH] D54604: Automatic variable initialization

pcc added inline comments. Comment at: include/clang/Driver/ToolChain.h:355 + virtual LangOptions::TrivialAutoVarInitKind + GetDefaultTrivialAutoVarInit() const { +return LangOptions::TrivialAutoVarInitKind::Uninitialized; I wouldn't introduce this function

[PATCH] D54604: Automatic variable initialization

jfb added a comment. In https://reviews.llvm.org/D54604#1301807, @kcc wrote: > Would it be possible to extend test/Sema/uninit-variables.c to verify that > the new option > doesn't break -Wuninitialized? Done. Repository: rC Clang https://reviews.llvm.org/D54604 __

[PATCH] D54604: Automatic variable initialization

jfb updated this revision to Diff 174471. jfb added a comment. - Make sure uninit-variables.c doesn't break. Repository: rC Clang https://reviews.llvm.org/D54604 Files: include/clang/Basic/Attr.td include/clang/Basic/AttrDocs.td include/clang/Basic/LangOptions.def include/clang/Basic

[PATCH] D54604: Automatic variable initialization

kcc added a comment. Would it be possible to extend test/Sema/uninit-variables.c to verify that the new option doesn't break -Wuninitialized? Repository: rC Clang https://reviews.llvm.org/D54604 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D54604: Automatic variable initialization

lebedev.ri added a comment. There is also https://reviews.llvm.org/D54473 `[sanitizers] Initial implementation for -fsanitize=init-locals`. Repository: rC Clang https://reviews.llvm.org/D54604 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D54604: Automatic variable initialization

jfb added a comment. In https://reviews.llvm.org/D54604#1300544, @t.p.northover wrote: > > This isn't meant to change the semantics of C and C++. > > As I said in the cfe-dev thread, that's exactly what it does. Specifically I > think this is essentially defining a new dialect of C++, which I ha

[PATCH] D54604: Automatic variable initialization

kcc added reviewers: glider, vitalybuka, pcc, eugenis, vlad.tsyrklevich. kcc added a comment. exciting. adding more folks FYI and to help review Repository: rC Clang https://reviews.llvm.org/D54604 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D54604: Automatic variable initialization

t.p.northover added a comment. > This isn't meant to change the semantics of C and C++. As I said in the cfe-dev thread, that's exactly what it does. Specifically I think this is essentially defining a new dialect of C++, which I have massive concerns about. Additionally, as much as we might cl

[PATCH] D54604: Automatic variable initialization

jfb created this revision. jfb added reviewers: kcc, rjmccall, rsmith. Herald added subscribers: dexonsmith, jkorous, JDevlieghere. Add an option to initialize automatic variables with either a pattern or with zeroes. The default is still that automatic variables are uninitialized. Also add attrib