[PATCH] D52440: Emit lifetime markers for temporary function parameter aggregates

2018-11-16 Thread Ian Tessier via Phabricator via cfe-commits
itessier added a comment.

Thanks for the detailed info! I will try to get something out when I get some 
free cycles.


Repository:
  rC Clang

https://reviews.llvm.org/D52440



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52440: Emit lifetime markers for temporary function parameter aggregates

2018-11-14 Thread Ian Tessier via Phabricator via cfe-commits
itessier added a comment.

> This is problematic because we're not necessarily in a scope that usefully 
> limits the duration of cleanups — we don't push full-expression scopes when 
> emitting an arbitrary statement. Probably we should, but we don't.



> If you'd like to take a look at solving that problem first, that would be 
> great.

Sure I'd like to take a look at this, but I'm not familiar with the code gen 
module. Can you point me to where I should start looking to understand and 
figure out where to add scopes?


Repository:
  rC Clang

https://reviews.llvm.org/D52440



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D52440: Emit lifetime markers for temporary function parameter aggregates

2018-09-24 Thread Ian Tessier via Phabricator via cfe-commits
itessier created this revision.
itessier added a reviewer: rjmccall.
Herald added a subscriber: cfe-commits.

Clang is not emitting lifetime markers for temporary function parameters, so 
more stack space is potentially being allocated than necessary.

A new parameter is added to the CreateAggTemp and EmitAnyExprToTemp functions 
to indicate whether to emit lifetime markers for aggregates, so that the 
EmitCallArg function can request them when evaluating an arg. The parameter is 
defaulted to false so that the behaviour of other callers is not affected.


Repository:
  rC Clang

https://reviews.llvm.org/D52440

Files:
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp
  test/CodeGenCXX/stack-reuse-miscompile.cpp
  test/CodeGenCXX/stack-reuse.cpp

Index: test/CodeGenCXX/stack-reuse.cpp
===
--- test/CodeGenCXX/stack-reuse.cpp
+++ test/CodeGenCXX/stack-reuse.cpp
@@ -27,7 +27,9 @@
 extern S_small foo_small();
 extern S_large foo_large();
 extern void bar_small(S_small*);
+extern void bar_small(S_small);
 extern void bar_large(S_large*);
+extern void bar_large(S_large);
 
 // Prevent mangling of function names.
 extern "C" {
@@ -130,6 +132,36 @@
   }
 }
 
+void small_temp_object() {
+// CHECK-LABEL: define void @small_temp_object
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9foo_smallv
+// CHECK: call void @_Z9bar_small7S_small
+// CHECK: call void @llvm.lifetime.end
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9foo_smallv
+// CHECK: call void @_Z9bar_small7S_small
+// CHECK: call void @llvm.lifetime.end
+
+  bar_small(foo_small());
+  bar_small(foo_small());
+}
+
+void large_temp_object() {
+// CHECK-LABEL: define void @large_temp_object
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9foo_largev
+// CHECK: call void @_Z9bar_large7S_large
+// CHECK: call void @llvm.lifetime.end
+// CHECK: call void @llvm.lifetime.start
+// CHECK: call void @_Z9foo_largev
+// CHECK: call void @_Z9bar_large7S_large
+// CHECK: call void @llvm.lifetime.end
+
+  bar_large(foo_large());
+  bar_large(foo_large());
+}
+
 int large_combiner_test(S_large s) {
 // CHECK-LABEL: define i32 @large_combiner_test
 // CHECK: [[T2:%.*]] = alloca %struct.Combiner
Index: test/CodeGenCXX/stack-reuse-miscompile.cpp
===
--- test/CodeGenCXX/stack-reuse-miscompile.cpp
+++ test/CodeGenCXX/stack-reuse-miscompile.cpp
@@ -19,33 +19,39 @@
 
 const char * f(S s)
 {
-// It's essential that the lifetimes of all three T temporaries here are
+// It's essential that the lifetimes of the first three T temporaries here are
 // overlapping. They must all remain alive through the call to str().
 //
 // CHECK: [[T1:%.*]] = alloca %class.T, align 4
 // CHECK: [[T2:%.*]] = alloca %class.T, align 4
 // CHECK: [[T3:%.*]] = alloca %class.T, align 4
+// CHECK: [[T4:%.*]] = alloca %class.S, align 4
 //
 // FIXME: We could defer starting the lifetime of the return object of concat
 // until the call.
 // CHECK: [[T1i8:%.*]] = bitcast %class.T* [[T1]] to i8*
 // CHECK: call void @llvm.lifetime.start.p0i8(i64 16, i8* [[T1i8]])
 //
 // CHECK: [[T2i8:%.*]] = bitcast %class.T* [[T2]] to i8*
 // CHECK: call void @llvm.lifetime.start.p0i8(i64 16, i8* [[T2i8]])
-// CHECK: [[T4:%.*]] = call %class.T* @_ZN1TC1EPKc(%class.T* [[T2]], i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.str, i32 0, i32 0))
+// CHECK: [[T5:%.*]] = call %class.T* @_ZN1TC1EPKc(%class.T* [[T2]], i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.str, i32 0, i32 0))
 //
 // CHECK: [[T3i8:%.*]] = bitcast %class.T* [[T3]] to i8*
 // CHECK: call void @llvm.lifetime.start.p0i8(i64 16, i8* [[T3i8]])
-// CHECK: [[T5:%.*]] = call %class.T* @_ZN1TC1E1S(%class.T* [[T3]], [2 x i32] %{{.*}})
+//
+// CHECK: [[T4i8:%.*]] = bitcast %class.S* [[T4]] to i8*
+// CHECK: call void @llvm.lifetime.start.p0i8(i64 8, i8* [[T4i8]])
+//
+// CHECK: [[T6:%.*]] = call %class.T* @_ZN1TC1E1S(%class.T* [[T3]], [2 x i32] %{{.*}})
 //
 // CHECK: call void @_ZNK1T6concatERKS_(%class.T* sret [[T1]], %class.T* [[T2]], %class.T* dereferenceable(16) [[T3]])
-// CHECK: [[T6:%.*]] = call i8* @_ZNK1T3strEv(%class.T* [[T1]])
+// CHECK: [[T7:%.*]] = call i8* @_ZNK1T3strEv(%class.T* [[T1]])
 //
 // CHECK: call void @llvm.lifetime.end.p0i8(
 // CHECK: call void @llvm.lifetime.end.p0i8(
 // CHECK: call void @llvm.lifetime.end.p0i8(
-// CHECK: ret i8* [[T6]]
+// CHECK: call void @llvm.lifetime.end.p0i8(
+// CHECK: ret i8* [[T7]]
 
   return T("[").concat(T(s)).str();
 }
Index: test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp
===
--- test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp
+++ test/CodeGenCXX/microsoft-abi-eh-cleanups.cpp
@@ -322,6 +322,10 @@
 // WIN32: call void @llvm.memset.p0i8.i32(
 // WIN32: call i32 

[PATCH] D41720: [clang-tidy] Add a -show-color flag.

2018-01-26 Thread Ian Tessier via Phabricator via cfe-commits
itessier added inline comments.



Comment at: clang-tidy/ClangTidyOptions.h:93
+  /// \brief Show color diagnostics.
+  llvm::Optional ShowColor;
+

alexfh wrote:
> This doesn't belong to ClangTidyOptions. It's specific to the CLI, but CLI is 
> not the only frontend for clang-tidy.
Since we have to propagate the value to the ErrorReporter, how about adding a 
bool param to the ErrorReporter ctor? We could add a setter instead, but that 
would require moving a diag printer call out of the ctor since it uses the 
DiagOpts instance.

The colour logic would then be moved into either clangTidyMain or handleErrors.



Comment at: clang-tidy/tool/ClangTidyMain.cpp:150-152
+Show color diagnostics. If not specified,
+defaults to on when a color-capable terminal
+is detected.)"),

itessier wrote:
> aaron.ballman wrote:
> > I think this raw string can be reflowed a bit?
> > 
> > Instead of "defaults to on", how about "Defaults to true when a 
> > color-capable terminal is detected."?
> I copied that part from the -fcolor-diagnostics flag to be consistent. I 
> don't changing "on" to "true" if you prefer that instead.
That should have read "I don't mind changing..."


https://reviews.llvm.org/D41720



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D35338: Add the -fdestroy-globals flag

2018-01-26 Thread Ian Tessier via Phabricator via cfe-commits
itessier added a comment.

> That seems like a nice win and I like the convenience of this approach. That 
> said I've just remembered that there's a thread on cfe-dev about this:
> [RFC] Suppress C++ static destructor registration
> I don't think a consensus was reached. From what I gather, some people think 
> that the convenience of this flag makes it worth adding to clang, while 
> others think that adding a non-standard compiler-specific flag is asking for 
> trouble.

Given that firmware is a much different (or controlled) environment than a 
binary running on a full blown OS, would it be acceptable to name the flag 
-fbaremetal-destroy-globals, and only allow its use if the target triple's OS 
is set to none (e.g.: arm-**none**-eabi)?


https://reviews.llvm.org/D35338



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D41720: [clang-tidy] Add a -show-color flag.

2018-01-04 Thread Ian Tessier via Phabricator via cfe-commits
itessier marked an inline comment as done.
itessier added a comment.

> This patch is missing test coverage.

Done.




Comment at: clang-tidy/tool/ClangTidyMain.cpp:150-152
+Show color diagnostics. If not specified,
+defaults to on when a color-capable terminal
+is detected.)"),

aaron.ballman wrote:
> I think this raw string can be reflowed a bit?
> 
> Instead of "defaults to on", how about "Defaults to true when a color-capable 
> terminal is detected."?
I copied that part from the -fcolor-diagnostics flag to be consistent. I don't 
changing "on" to "true" if you prefer that instead.


https://reviews.llvm.org/D41720



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D41720: [clang-tidy] Add a -show-color flag.

2018-01-04 Thread Ian Tessier via Phabricator via cfe-commits
itessier updated this revision to Diff 128652.

https://reviews.llvm.org/D41720

Files:
  clang-tidy/ClangTidy.cpp
  clang-tidy/ClangTidyOptions.cpp
  clang-tidy/ClangTidyOptions.h
  clang-tidy/tool/ClangTidyMain.cpp
  test/clang-tidy/show-color.cpp


Index: test/clang-tidy/show-color.cpp
===
--- /dev/null
+++ test/clang-tidy/show-color.cpp
@@ -0,0 +1,10 @@
+// RUN: clang-tidy -checks='-*,clang-analyzer-core.NullDereference' 
-show-color %s -- | FileCheck --check-prefix=CHECK-COLOR %s
+// RUN: clang-tidy -checks='-*,clang-analyzer-core.NullDereference' 
-show-color=0 %s -- | FileCheck --check-prefix=CHECK-NO-COLOR %s
+// REQUIRES: ansi-escape-sequences
+
+void test() {
+  int *value = 0;
+  // CHECK-COLOR: [[BOLD:.\[1m]]{{.*}}[[@LINE+2]]:10: 
[[RESET:.\[0m]][[MAGENTA:.\[0;1;35m]]warning: [[RESET]][[BOLD]]Dereference of 
null pointer (loaded from variable 'value') 
[clang-analyzer-core.NullDereference][[RESET]]
+  // CHECK-NO-COLOR: [[@LINE+1]]:10: warning: Dereference of null pointer 
(loaded from variable 'value') [clang-analyzer-core.NullDereference]
+  *value = 1;
+}
Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -146,6 +146,12 @@
cl::init("none"),
cl::cat(ClangTidyCategory));
 
+static cl::opt ShowColor("show-color", cl::desc(R"(
+Show color diagnostics. If not specified,
+defaults to on when a color-capable terminal
+is detected.)"),
+   cl::ValueOptional, cl::cat(ClangTidyCategory));
+
 static cl::opt ListChecks("list-checks", cl::desc(R"(
 List all enabled checks and exit. Use with
 -checks=* to list all available checks.
@@ -304,6 +310,7 @@
   DefaultOptions.SystemHeaders = SystemHeaders;
   DefaultOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
   DefaultOptions.FormatStyle = FormatStyle;
+  DefaultOptions.ShowColor = ShowColor;
   DefaultOptions.User = llvm::sys::Process::GetEnv("USER");
   // USERNAME is used on Windows.
   if (!DefaultOptions.User)
@@ -322,6 +329,8 @@
 OverrideOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
   if (FormatStyle.getNumOccurrences() > 0)
 OverrideOptions.FormatStyle = FormatStyle;
+  if (ShowColor.getNumOccurrences() > 0)
+OverrideOptions.ShowColor = ShowColor;
 
   if (!Config.empty()) {
 if (llvm::ErrorOr ParsedConfig =
Index: clang-tidy/ClangTidyOptions.h
===
--- clang-tidy/ClangTidyOptions.h
+++ clang-tidy/ClangTidyOptions.h
@@ -89,6 +89,9 @@
   /// See clang-format documentation for more about configuring format style.
   llvm::Optional FormatStyle;
 
+  /// \brief Show color diagnostics.
+  llvm::Optional ShowColor;
+
   /// \brief Specifies the name or e-mail of the user running clang-tidy.
   ///
   /// This option is used, for example, to place the correct user name in 
TODO()
Index: clang-tidy/ClangTidyOptions.cpp
===
--- clang-tidy/ClangTidyOptions.cpp
+++ clang-tidy/ClangTidyOptions.cpp
@@ -88,6 +88,7 @@
 IO.mapOptional("HeaderFilterRegex", Options.HeaderFilterRegex);
 IO.mapOptional("AnalyzeTemporaryDtors", Options.AnalyzeTemporaryDtors);
 IO.mapOptional("FormatStyle", Options.FormatStyle);
+IO.mapOptional("ShowColor", Options.ShowColor);
 IO.mapOptional("User", Options.User);
 IO.mapOptional("CheckOptions", NOpts->Options);
 IO.mapOptional("ExtraArgs", Options.ExtraArgs);
@@ -149,6 +150,7 @@
   overrideValue(Result.SystemHeaders, Other.SystemHeaders);
   overrideValue(Result.AnalyzeTemporaryDtors, Other.AnalyzeTemporaryDtors);
   overrideValue(Result.FormatStyle, Other.FormatStyle);
+  overrideValue(Result.ShowColor, Other.ShowColor);
   overrideValue(Result.User, Other.User);
   mergeVectors(Result.ExtraArgs, Other.ExtraArgs);
   mergeVectors(Result.ExtraArgsBefore, Other.ExtraArgsBefore);
Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -96,7 +96,8 @@
   DiagPrinter),
 SourceMgr(Diags, Files), Context(Context), ApplyFixes(ApplyFixes),
 TotalFixes(0), AppliedFixes(0), WarningsAsErrors(0) {
-DiagOpts->ShowColors = llvm::sys::Process::StandardOutHasColors();
+DiagOpts->ShowColors = Context.getOptions().ShowColor.getValueOr(
+llvm::sys::Process::StandardOutHasColors());
 DiagPrinter->BeginSourceFile(LangOpts);
   }
 


Index: test/clang-tidy/show-color.cpp
===
--- /dev/null
+++ test/clang-tidy/show-color.cpp
@@ -0,0 +1,10 @@
+// RUN: clang-tidy -checks='-*,clang-analyzer-core.NullDereference' -show-color %s -- | FileCheck --check-prefix=CHECK-COLOR %s

[PATCH] D41720: [clang-tidy] Add a -show-color flag.

2018-01-03 Thread Ian Tessier via Phabricator via cfe-commits
itessier created this revision.
itessier added a project: clang-tools-extra.
Herald added a subscriber: xazax.hun.

This change will allow enabling of colour diagnostics when not directly running 
within a terminal, but colour output is possible. For example, if stdout is 
being captured and redirected to a real terminal.


https://reviews.llvm.org/D41720

Files:
  clang-tidy/ClangTidy.cpp
  clang-tidy/ClangTidyOptions.cpp
  clang-tidy/ClangTidyOptions.h
  clang-tidy/tool/ClangTidyMain.cpp


Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -146,6 +146,12 @@
cl::init("none"),
cl::cat(ClangTidyCategory));
 
+static cl::opt ShowColor("show-color", cl::desc(R"(
+Show color diagnostics. If not specified,
+defaults to on when a color-capable terminal
+is detected.)"),
+   cl::ValueOptional, cl::cat(ClangTidyCategory));
+
 static cl::opt ListChecks("list-checks", cl::desc(R"(
 List all enabled checks and exit. Use with
 -checks=* to list all available checks.
@@ -304,6 +310,7 @@
   DefaultOptions.SystemHeaders = SystemHeaders;
   DefaultOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
   DefaultOptions.FormatStyle = FormatStyle;
+  DefaultOptions.ShowColor = ShowColor;
   DefaultOptions.User = llvm::sys::Process::GetEnv("USER");
   // USERNAME is used on Windows.
   if (!DefaultOptions.User)
@@ -322,6 +329,8 @@
 OverrideOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
   if (FormatStyle.getNumOccurrences() > 0)
 OverrideOptions.FormatStyle = FormatStyle;
+  if (ShowColor.getNumOccurrences() > 0)
+OverrideOptions.ShowColor = ShowColor;
 
   if (!Config.empty()) {
 if (llvm::ErrorOr ParsedConfig =
Index: clang-tidy/ClangTidyOptions.h
===
--- clang-tidy/ClangTidyOptions.h
+++ clang-tidy/ClangTidyOptions.h
@@ -89,6 +89,9 @@
   /// See clang-format documentation for more about configuring format style.
   llvm::Optional FormatStyle;
 
+  /// \brief Show color diagnostics.
+  llvm::Optional ShowColor;
+
   /// \brief Specifies the name or e-mail of the user running clang-tidy.
   ///
   /// This option is used, for example, to place the correct user name in 
TODO()
Index: clang-tidy/ClangTidyOptions.cpp
===
--- clang-tidy/ClangTidyOptions.cpp
+++ clang-tidy/ClangTidyOptions.cpp
@@ -88,6 +88,7 @@
 IO.mapOptional("HeaderFilterRegex", Options.HeaderFilterRegex);
 IO.mapOptional("AnalyzeTemporaryDtors", Options.AnalyzeTemporaryDtors);
 IO.mapOptional("FormatStyle", Options.FormatStyle);
+IO.mapOptional("ShowColor", Options.ShowColor);
 IO.mapOptional("User", Options.User);
 IO.mapOptional("CheckOptions", NOpts->Options);
 IO.mapOptional("ExtraArgs", Options.ExtraArgs);
@@ -149,6 +150,7 @@
   overrideValue(Result.SystemHeaders, Other.SystemHeaders);
   overrideValue(Result.AnalyzeTemporaryDtors, Other.AnalyzeTemporaryDtors);
   overrideValue(Result.FormatStyle, Other.FormatStyle);
+  overrideValue(Result.ShowColor, Other.ShowColor);
   overrideValue(Result.User, Other.User);
   mergeVectors(Result.ExtraArgs, Other.ExtraArgs);
   mergeVectors(Result.ExtraArgsBefore, Other.ExtraArgsBefore);
Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -96,7 +96,11 @@
   DiagPrinter),
 SourceMgr(Diags, Files), Context(Context), ApplyFixes(ApplyFixes),
 TotalFixes(0), AppliedFixes(0), WarningsAsErrors(0) {
-DiagOpts->ShowColors = llvm::sys::Process::StandardOutHasColors();
+if (Context.getOptions().ShowColor.hasValue()) {
+  DiagOpts->ShowColors = Context.getOptions().ShowColor.getValue();
+} else {
+  DiagOpts->ShowColors = llvm::sys::Process::StandardOutHasColors();
+}
 DiagPrinter->BeginSourceFile(LangOpts);
   }
 


Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -146,6 +146,12 @@
cl::init("none"),
cl::cat(ClangTidyCategory));
 
+static cl::opt ShowColor("show-color", cl::desc(R"(
+Show color diagnostics. If not specified,
+defaults to on when a color-capable terminal
+is detected.)"),
+   cl::ValueOptional, cl::cat(ClangTidyCategory));
+
 static cl::opt ListChecks("list-checks", cl::desc(R"(
 List all enabled checks and exit. Use with
 -checks=* to list all available checks.
@@ -304,6 +310,7 @@
   DefaultOptions.SystemHeaders = SystemHeaders;
   DefaultOptions.AnalyzeTemporaryDtors = AnalyzeTemporaryDtors;
   

[PATCH] D37496: Fix validation of the -mthread-model flag in the Clang driver

2017-09-07 Thread Ian Tessier via Phabricator via cfe-commits
itessier added a comment.

Do you mind submitting this CL? I don't have access to SVN.


https://reviews.llvm.org/D37496



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37496: Fix validation of the -mthread-model flag in the Clang driver

2017-09-06 Thread Ian Tessier via Phabricator via cfe-commits
itessier added a comment.

In https://reviews.llvm.org/D37496#861746, @compnerd wrote:

> The change looks good.  Can you please add some test cases for this?  Or do 
> existing test cases cover this already?


Should have added this to the description, but yes there are existing tests 
that cover this flag (test/Driver/thread-model.c). 
https://reviews.llvm.org/D37493 removes the only overridden thread model 
methods so we can't test the fix in this CL.


https://reviews.llvm.org/D37496



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37493: Fix ARM bare metal driver to support atomics

2017-09-05 Thread Ian Tessier via Phabricator via cfe-commits
itessier added a comment.

I don't have SVN access, can somebody commit this change for me?


https://reviews.llvm.org/D37493



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D37496: Fix validation of the -mthread-model flag in the Clang driver

2017-09-05 Thread Ian Tessier via Phabricator via cfe-commits
itessier created this revision.

The ToolChain class validates the -mthread-model flag in the constructor which 
doesn't work correctly since the thread model methods are virtual methods. The 
check is moved into Clang::ConstructJob() when constructing the internal 
command line.


https://reviews.llvm.org/D37496

Files:
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/Clang.cpp


Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3254,8 +3254,12 @@
   }
 
   CmdArgs.push_back("-mthread-model");
-  if (Arg *A = Args.getLastArg(options::OPT_mthread_model))
+  if (Arg *A = Args.getLastArg(options::OPT_mthread_model)) {
+if (!getToolChain().isThreadModelSupported(A->getValue()))
+  D.Diag(diag::err_drv_invalid_thread_model_for_target)
+  << A->getValue() << A->getAsString(Args);
 CmdArgs.push_back(A->getValue());
+  }
   else
 CmdArgs.push_back(Args.MakeArgString(getToolChain().getThreadModel()));
 
Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -74,11 +74,6 @@
 : D(D), Triple(T), Args(Args), CachedRTTIArg(GetRTTIArgument(Args)),
   CachedRTTIMode(CalculateRTTIMode(Args, Triple, CachedRTTIArg)),
   EffectiveTriple() {
-  if (Arg *A = Args.getLastArg(options::OPT_mthread_model))
-if (!isThreadModelSupported(A->getValue()))
-  D.Diag(diag::err_drv_invalid_thread_model_for_target)
-  << A->getValue() << A->getAsString(Args);
-
   std::string CandidateLibPath = getArchSpecificLibPath();
   if (getVFS().exists(CandidateLibPath))
 getFilePaths().push_back(CandidateLibPath);


Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3254,8 +3254,12 @@
   }
 
   CmdArgs.push_back("-mthread-model");
-  if (Arg *A = Args.getLastArg(options::OPT_mthread_model))
+  if (Arg *A = Args.getLastArg(options::OPT_mthread_model)) {
+if (!getToolChain().isThreadModelSupported(A->getValue()))
+  D.Diag(diag::err_drv_invalid_thread_model_for_target)
+  << A->getValue() << A->getAsString(Args);
 CmdArgs.push_back(A->getValue());
+  }
   else
 CmdArgs.push_back(Args.MakeArgString(getToolChain().getThreadModel()));
 
Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -74,11 +74,6 @@
 : D(D), Triple(T), Args(Args), CachedRTTIArg(GetRTTIArgument(Args)),
   CachedRTTIMode(CalculateRTTIMode(Args, Triple, CachedRTTIArg)),
   EffectiveTriple() {
-  if (Arg *A = Args.getLastArg(options::OPT_mthread_model))
-if (!isThreadModelSupported(A->getValue()))
-  D.Diag(diag::err_drv_invalid_thread_model_for_target)
-  << A->getValue() << A->getAsString(Args);
-
   std::string CandidateLibPath = getArchSpecificLibPath();
   if (getVFS().exists(CandidateLibPath))
 getFilePaths().push_back(CandidateLibPath);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D35338: Add the -fdestroy-globals flag

2017-07-18 Thread Ian Tessier via Phabricator via cfe-commits
itessier added a comment.

In https://reviews.llvm.org/D35338#809146, @vsk wrote:

> This is interesting. Do you have any results/metrics to share (e.g some any 
> binary size reduction for projects you've looked at)?


I only tested this with Project Loon's avionics firmware which freed up ~1.2% 
of ROM space. A small amount, but for us every bit counts.

I did look at creating a templated wrapper class instead which uses 
placement-new to construct the wrapped type in a private char buffer. This 
results in the same effect as this patch (no dtor references are emitted). The 
only issue is the code is a big uglier to read, and we might forgot to use the 
wrapper (though a static checker could be added for this case). I figured it'd 
be best to add compiler support so other C++ based firmware projects can 
benefit as well.


https://reviews.llvm.org/D35338



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D35338: Add the -fdestroy-globals flag

2017-07-12 Thread Ian Tessier via Phabricator via cfe-commits
itessier created this revision.

The -fdestroy-globals flag can be used to disable global variable destructor
registration. It is intended to be used with embedded code that never exits.
Disabling registration allows the linker to garbage collect unused destructors
and vtables.


https://reviews.llvm.org/D35338

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CGDeclCXX.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Driver/fdestroy-globals.cpp

Index: test/Driver/fdestroy-globals.cpp
===
--- /dev/null
+++ test/Driver/fdestroy-globals.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang -fno-destroy-globals -### %s 2>&1 | FileCheck %s --check-prefix=CHECK-FLAG-DISABLE
+// RUN: %clang -fdestroy-globals -### %s 2>&1| FileCheck %s --check-prefix=CHECK-FLAG-ENABLE1
+// RUN: %clang -### %s 2>&1  | FileCheck %s --check-prefix=CHECK-FLAG-ENABLE2
+
+// RUN: %clang_cc1 -fno-destroy-globals -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-CODE-DISABLE
+// RUN: %clang_cc1 -fdestroy-globals -emit-llvm -o - %s| FileCheck %s --check-prefix=CHECK-CODE-ENABLE
+
+// CHECK-FLAG-DISABLE: "-cc1"
+// CHECK-FLAG-DISABLE: "-fno-destroy-globals"
+
+// CHECK-FLAG-ENABLE1: "-cc1"
+// CHECK-FLAG-ENABLE1-NOT: "-fno-destroy-globals"
+
+// CHECK-FLAG-ENABLE2: "-cc1"
+// CHECK-FLAG-ENABLE2-NOT: "-fno-destroy-globals"
+
+// CHECK-CODE-DISABLE-LABEL: define {{.*}} @__cxx_global_var_init
+// CHECK-CODE-DISABLE: call void @_ZN1AC1Ev{{.*}}
+// CHECK-CODE-DISABLE: ret void
+
+// CHECK-CODE-ENABLE-LABEL: define {{.*}} @__cxx_global_var_init
+// CHECK-CODE-ENABLE: call void @_ZN1AC1Ev{{.*}}
+// CHECK-CODE-ENABLE: %{{.*}} = call i32 @__cxa_atexit{{.*}}_ZN1AD1Ev
+// CHECK-CODE-ENABLE: ret void
+
+struct A {
+  virtual ~A() {}
+};
+
+A a;
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -582,6 +582,7 @@
   Opts.ObjCAutoRefCountExceptions = Args.hasArg(OPT_fobjc_arc_exceptions);
   Opts.CXAAtExit = !Args.hasArg(OPT_fno_use_cxa_atexit);
   Opts.CXXCtorDtorAliases = Args.hasArg(OPT_mconstructor_aliases);
+  Opts.DestroyGlobals = !Args.hasArg(OPT_fno_destroy_globals);
   Opts.CodeModel = getCodeModel(Args, Diags);
   Opts.DebugPass = Args.getLastArgValue(OPT_mdebug_pass);
   Opts.DisableFPElim =
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3697,6 +3697,11 @@
   KernelOrKext)
 CmdArgs.push_back("-fno-use-cxa-atexit");
 
+  // -fdestroy-globals=1 is default.
+  if (!Args.hasFlag(options::OPT_fdestroy_globals,
+options::OPT_fno_destroy_globals, true))
+  CmdArgs.push_back("-fno-destroy-globals");
+
   // -fms-extensions=0 is default.
   if (Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
IsWindowsMSVC))
Index: lib/CodeGen/CGDeclCXX.cpp
===
--- lib/CodeGen/CGDeclCXX.cpp
+++ lib/CodeGen/CGDeclCXX.cpp
@@ -180,7 +180,7 @@
   EmitDeclInit(*this, D, DeclAddr);
 if (CGM.isTypeConstant(D.getType(), true))
   EmitDeclInvariant(*this, D, DeclPtr);
-else
+else if (CGM.getCodeGenOpts().DestroyGlobals)
   EmitDeclDestroy(*this, D, DeclAddr);
 return;
   }
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -44,6 +44,7 @@
 CODEGENOPT(CXAAtExit , 1, 1) ///< Use __cxa_atexit for calling destructors.
 CODEGENOPT(CXXCtorDtorAliases, 1, 0) ///< Emit complete ctors/dtors as linker
  ///< aliases to base ctors when possible.
+CODEGENOPT(DestroyGlobals, 1, 1) ///< -fdestroy-globals
 CODEGENOPT(DataSections  , 1, 0) ///< Set when -fdata-sections is enabled.
 CODEGENOPT(UniqueSectionNames, 1, 1) ///< Set for -funique-section-names.
 CODEGENOPT(DisableFPElim , 1, 0) ///< Set when -fomit-frame-pointer is enabled.
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1235,6 +1235,7 @@
   HelpText<"Don't use __cxa_atexit for calling destructors">;
 def fno_use_init_array : Flag<["-"], "fno-use-init-array">, Group, Flags<[CC1Option]>,
   HelpText<"Don't use .init_array instead of .ctors">;
+def fno_destroy_globals : Flag<["-"], "fno-destroy-globals">, Group, Flags<[CC1Option]>;
 def fno_unit_at_a_time : Flag<["-"], "fno-unit-at-a-time">, Group;
 def fno_unwind_tables : Flag<["-"], "fno-unwind-tables">, Group;
 def fno_verbose_asm :