[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

2017-08-03 Thread Simon Dardis via Phabricator via cfe-commits
sdardis added a comment. https://reviews.llvm.org/rL309978 for the revert. Repository: rL LLVM https://reviews.llvm.org/D35917 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

2017-08-03 Thread Simon Dardis via Phabricator via cfe-commits
sdardis added a comment. In https://reviews.llvm.org/D35917#830898, @joerg wrote: > I don't see any reason why zero-initialised constants should be emitted in > BSS. I know that GCC does that and I just fixed bugs in that because created > wrong section flags for it. So yes, I'd prefer to rever

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

2017-08-03 Thread Joerg Sonnenberger via Phabricator via cfe-commits
joerg added a comment. I don't see any reason why zero-initialised constants should be emitted in BSS. I know that GCC does that and I just fixed bugs in that because created wrong section flags for it. So yes, I'd prefer to revert this and fix the real problem. Repository: rL LLVM https:/

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

2017-08-03 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. LLVM never puts constant data into the BSS. See isSuitableForBSS in lib/Target/TargetLoweringObjectFile.cpp. (gcc's behavior is just weird... apparently, whether or not constant data is placed in the BSS with -fno-common depends on the syntactic form of the initializ

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

2017-08-03 Thread Simon Dardis via Phabricator via cfe-commits
sdardis added inline comments. Comment at: cfe/trunk/lib/CodeGen/TargetInfo.cpp: + GVar->setLinkage(llvm::GlobalValue::ExternalLinkage); + GVar->setSection("rodata"); +} efriedma wrote: > Also, this change is clearly unacceptable; am

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

2017-08-03 Thread Simon Dardis via Phabricator via cfe-commits
sdardis added a comment. It's required for feature parity with GCC. -fno-common will place globals into the bss section which uses memory at runtime. The -membedded-data and -muninit-const-in-rodata options are for reducing RAM usage in some embedded environments. Repository: rL LLVM https

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

2017-08-03 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments. Comment at: cfe/trunk/lib/CodeGen/TargetInfo.cpp: + GVar->setLinkage(llvm::GlobalValue::ExternalLinkage); + GVar->setSection("rodata"); +} Also, this change is clearly unacceptable; among other things,

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

2017-08-03 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. Do you actually need a new flag for this? "-fno-common" will ensure clang doesn't generate globals with common linkage. Repository: rL LLVM https://reviews.llvm.org/D35917 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

2017-08-03 Thread Simon Dardis via Phabricator via cfe-commits
sdardis added a comment. Thanks for the reviews of all of these options. Repository: rL LLVM https://reviews.llvm.org/D35917 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

2017-08-03 Thread Simon Dardis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL309940: [mips] Implement -muninit-const-in-rodata (authored by sdardis). Repository: rL LLVM https://reviews.llvm.org/D35917 Files: cfe/trunk/include/clang/Driver/Options.td cfe/trunk/include/clan

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

2017-08-03 Thread Simon Atanasyan via Phabricator via cfe-commits
atanasyan accepted this revision. atanasyan added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D35917 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

2017-08-03 Thread Simon Dardis via Phabricator via cfe-commits
sdardis added inline comments. Comment at: lib/Driver/ToolChains/Clang.cpp:1523 + CmdArgs.push_back("-muninit-const-in-rodata"); + A->claim(); +} atanasyan wrote: > What's happened if the `-muninit-const-in-rodata` is used without > `-m

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

2017-08-03 Thread Simon Dardis via Phabricator via cfe-commits
sdardis updated this revision to Diff 109545. sdardis marked an inline comment as done. sdardis added a comment. Address review comment. https://reviews.llvm.org/D35917 Files: include/clang/Driver/Options.td include/clang/Frontend/CodeGenOptions.def lib/CodeGen/TargetInfo.cpp lib/Driver

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

2017-08-03 Thread Simon Atanasyan via Phabricator via cfe-commits
atanasyan added inline comments. Comment at: include/clang/Driver/Options.td:2065 +def mno_uninit_const_in_rodata : Flag<["-"], "mno-uninit-const-in-rodata">, + Group, HelpText<"Do not Place uninitialized constants in the " + "read-only data section ins

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

2017-08-03 Thread Simon Dardis via Phabricator via cfe-commits
sdardis added a comment. Ping. https://reviews.llvm.org/D35917 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

2017-07-26 Thread Simon Dardis via Phabricator via cfe-commits
sdardis created this revision. Herald added a subscriber: arichardson. This option when combined with -mgpopt and -membedded-data places all uninitialized constant variables in the read-only section. https://reviews.llvm.org/D35917 Files: include/clang/Driver/Options.td include/clang/Fronte