[PATCH] D47280: [Sema][ObjC] Do not DiagnoseUseOfDecl in LookupMemberExpr

[Steven Wu via Phabricator via cfe-commits]

steven_wu created this revision.
steven_wu added reviewers: erik.pilkington, arphaman, doug.gregor.

Remove the call to DiagnoseUseOfDecl in LookupMemberExpr because:

1. LookupMemberExpr eagerly lookup both getter and setter, reguardless

if they are used or not. It causes wrong diagnostics if you are only
using getter.

2. LookupMemberExpr only diagnoses getter, but not setter.
3. ObjCPropertyOpBuilder already DiagnoseUseOfDecl when building getter

and setter. Doing it again in LookupMemberExpr causes duplicated
diagnostics.

rdar://problem/38479756


Repository:
  rC Clang

https://reviews.llvm.org/D47280

Files:
  lib/Sema/SemaExprMember.cpp
  test/SemaObjC/property-deprecated-warning.m


Index: test/SemaObjC/property-deprecated-warning.m
===
--- test/SemaObjC/property-deprecated-warning.m
+++ test/SemaObjC/property-deprecated-warning.m
@@ -167,3 +167,14 @@
foo.x = foo.x; // expected-error {{property access is using 'x' method 
which is unavailable}} \
   // expected-warning {{property access is using 'setX:' 
method which is deprecated}}
 }
+
+// test implicit property does not emit duplicated warning.
+@protocol Foo
+- (int)size __attribute__((availability(ios,deprecated=3.0))); // 
expected-note {{'size' has been explicitly marked deprecated here}}
+- (void)setSize: (int)x __attribute__((availability(ios,deprecated=2.0))); // 
expected-note {{'setSize:' has been explicitly marked deprecated here}}
+@end
+
+void testImplicitProperty(id object) {
+  object.size = object.size; // expected-warning {{'size' is deprecated: first 
deprecated in iOS 3.0}} \
+ // expected-warning {{'setSize:' is deprecated: 
first deprecated in iOS 2.0}}
+}
Index: lib/Sema/SemaExprMember.cpp
===
--- lib/Sema/SemaExprMember.cpp
+++ lib/Sema/SemaExprMember.cpp
@@ -1490,9 +1490,6 @@
 }
 
 if (ObjCMethodDecl *OMD = dyn_cast(PMDecl)) {
-  // Check the use of this method.
-  if (S.DiagnoseUseOfDecl(OMD, MemberLoc))
-return ExprError();
   Selector SetterSel =
 SelectorTable::constructSetterSelector(S.PP.getIdentifierTable(),
S.PP.getSelectorTable(),


Index: test/SemaObjC/property-deprecated-warning.m
===
--- test/SemaObjC/property-deprecated-warning.m
+++ test/SemaObjC/property-deprecated-warning.m
@@ -167,3 +167,14 @@
 	foo.x = foo.x; // expected-error {{property access is using 'x' method which is unavailable}} \
 		   // expected-warning {{property access is using 'setX:' method which is deprecated}}
 }
+
+// test implicit property does not emit duplicated warning.
+@protocol Foo
+- (int)size __attribute__((availability(ios,deprecated=3.0))); // expected-note {{'size' has been explicitly marked deprecated here}}
+- (void)setSize: (int)x __attribute__((availability(ios,deprecated=2.0))); // expected-note {{'setSize:' has been explicitly marked deprecated here}}
+@end
+
+void testImplicitProperty(id object) {
+  object.size = object.size; // expected-warning {{'size' is deprecated: first deprecated in iOS 3.0}} \
+ // expected-warning {{'setSize:' is deprecated: first deprecated in iOS 2.0}}
+}
Index: lib/Sema/SemaExprMember.cpp
===
--- lib/Sema/SemaExprMember.cpp
+++ lib/Sema/SemaExprMember.cpp
@@ -1490,9 +1490,6 @@
 }
 
 if (ObjCMethodDecl *OMD = dyn_cast(PMDecl)) {
-  // Check the use of this method.
-  if (S.DiagnoseUseOfDecl(OMD, MemberLoc))
-return ExprError();
   Selector SetterSel =
 SelectorTable::constructSetterSelector(S.PP.getIdentifierTable(),
S.PP.getSelectorTable(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47280: [Sema][ObjC] Do not DiagnoseUseOfDecl in LookupMemberExpr

[Steven Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC333148: [Sema][ObjC] Do not DiagnoseUseOfDecl in 
LookupMemberExpr (authored by steven_wu, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47280?vs=148279&id=148327#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47280

Files:
  lib/Sema/SemaExprMember.cpp
  test/SemaObjC/property-deprecated-warning.m


Index: test/SemaObjC/property-deprecated-warning.m
===
--- test/SemaObjC/property-deprecated-warning.m
+++ test/SemaObjC/property-deprecated-warning.m
@@ -167,3 +167,14 @@
foo.x = foo.x; // expected-error {{property access is using 'x' method 
which is unavailable}} \
   // expected-warning {{property access is using 'setX:' 
method which is deprecated}}
 }
+
+// test implicit property does not emit duplicated warning.
+@protocol Foo
+- (int)size __attribute__((availability(ios,deprecated=3.0))); // 
expected-note {{'size' has been explicitly marked deprecated here}}
+- (void)setSize: (int)x __attribute__((availability(ios,deprecated=2.0))); // 
expected-note {{'setSize:' has been explicitly marked deprecated here}}
+@end
+
+void testImplicitProperty(id object) {
+  object.size = object.size; // expected-warning {{'size' is deprecated: first 
deprecated in iOS 3.0}} \
+ // expected-warning {{'setSize:' is deprecated: 
first deprecated in iOS 2.0}}
+}
Index: lib/Sema/SemaExprMember.cpp
===
--- lib/Sema/SemaExprMember.cpp
+++ lib/Sema/SemaExprMember.cpp
@@ -1490,9 +1490,6 @@
 }
 
 if (ObjCMethodDecl *OMD = dyn_cast(PMDecl)) {
-  // Check the use of this method.
-  if (S.DiagnoseUseOfDecl(OMD, MemberLoc))
-return ExprError();
   Selector SetterSel =
 SelectorTable::constructSetterSelector(S.PP.getIdentifierTable(),
S.PP.getSelectorTable(),


Index: test/SemaObjC/property-deprecated-warning.m
===
--- test/SemaObjC/property-deprecated-warning.m
+++ test/SemaObjC/property-deprecated-warning.m
@@ -167,3 +167,14 @@
 	foo.x = foo.x; // expected-error {{property access is using 'x' method which is unavailable}} \
 		   // expected-warning {{property access is using 'setX:' method which is deprecated}}
 }
+
+// test implicit property does not emit duplicated warning.
+@protocol Foo
+- (int)size __attribute__((availability(ios,deprecated=3.0))); // expected-note {{'size' has been explicitly marked deprecated here}}
+- (void)setSize: (int)x __attribute__((availability(ios,deprecated=2.0))); // expected-note {{'setSize:' has been explicitly marked deprecated here}}
+@end
+
+void testImplicitProperty(id object) {
+  object.size = object.size; // expected-warning {{'size' is deprecated: first deprecated in iOS 3.0}} \
+ // expected-warning {{'setSize:' is deprecated: first deprecated in iOS 2.0}}
+}
Index: lib/Sema/SemaExprMember.cpp
===
--- lib/Sema/SemaExprMember.cpp
+++ lib/Sema/SemaExprMember.cpp
@@ -1490,9 +1490,6 @@
 }
 
 if (ObjCMethodDecl *OMD = dyn_cast(PMDecl)) {
-  // Check the use of this method.
-  if (S.DiagnoseUseOfDecl(OMD, MemberLoc))
-return ExprError();
   Selector SetterSel =
 SelectorTable::constructSetterSelector(S.PP.getIdentifierTable(),
S.PP.getSelectorTable(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51189: [Sema][ObjC] Infer availability of +new from availability of -init

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

I feel like this is a much tricky situation than just new and init. Following 
example is the same situation.

  __attribute__((objc_root_class))
  @interface NSObject
  - (void) foo;
  - (void) bar;
  @end
  
  @implementation NSObject
  - (void) foo {}
  - (void) bar { [self foo]; }
  @end
  
  @interface MyObject : NSObject
  - (void) foo __attribute__((unavailable));
  @end
  
  void test(MyObject *obj) {
[obj bar];
  }

We can do something about [NSObject new] because we know it's implementation 
but we have to live with more general cases.


Repository:
  rC Clang

https://reviews.llvm.org/D51189



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

[PATCH] D51440: [ToolChains] Link to compiler-rt with -L + -l when possible

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

I do prefer the current approach especially on Darwin. Some concerns of 
switching to use "-L + -l" are:

1. clang and compiler-rt are rev-locked. Inferring the compiler-rt from 
resource-dir and passing to linker with the full path can prevent mistakes of 
mixing them up.
2. This change does change linker semantics on Darwin. ld64 treats the inputs 
from command line with highest priority. Currently ld64 will use compiler-rt 
symbols before any other libraries passed in with -l flag. If clang decide to 
pass compiler-rt with -l flag, any other libraries (static or dynamic) that 
passed with -l flag will takes the priority over compiler-rt. This can lead to 
unexpected behavior.


Repository:
  rC Clang

https://reviews.llvm.org/D51440



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

[PATCH] D41087: [Preprocessor] Implement __is_target_{arch|vendor|os|environment} function-like builtin macros

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.

LGTM


https://reviews.llvm.org/D41087



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

[PATCH] D41076: [driver][darwin] Set the 'simulator' environment when it's specified in '-target'

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

Other than the comment inline, LGTM.




Comment at: lib/Driver/ToolChains/Darwin.cpp:1603
   // Recognize iOS targets with an x86 architecture as the iOS simulator.
-  if (Platform != MacOS && (getTriple().getArch() == llvm::Triple::x86 ||
-getTriple().getArch() == llvm::Triple::x86_64))
+  if (Environment == NativeEnvironment && Platform != MacOS &&
+  (getTriple().getArch() == llvm::Triple::x86 ||

Will be good to emit a warning for this. User should use -target with simulator 
or simulator deployment target.


Repository:
  rC Clang

https://reviews.llvm.org/D41076



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

[PATCH] D41425: [darwin][driver] Warn about mismatching --version-min rather than superfluous --version-min compiler option

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.

Just a small suggestion. Looks good otherwise.




Comment at: lib/Driver/ToolChains/Darwin.cpp:1536
+   Driver::GetReleaseVersion(OSVersionArgTarget->getOSVersion(),
+ ArgMajor, ArgMinor, ArgMicro, HadExtra) &&
+   VersionTuple(TargetMajor, TargetMinor, TargetMicro) !=

HadExtra is not ok right? macos10.11.0.1 is not the same as macos10.11.0?
Or HadExtra is an error somewhere else already?


Repository:
  rC Clang

https://reviews.llvm.org/D41425



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

[PATCH] D41425: [darwin][driver] Warn about mismatching --version-min rather than superfluous --version-min compiler option

[Steven Wu via Phabricator via cfe-commits]

steven_wu added inline comments.



Comment at: lib/Driver/ToolChains/Darwin.cpp:1536
+   Driver::GetReleaseVersion(OSVersionArgTarget->getOSVersion(),
+ ArgMajor, ArgMinor, ArgMicro, HadExtra) &&
+   VersionTuple(TargetMajor, TargetMinor, TargetMicro) !=

arphaman wrote:
> steven_wu wrote:
> > HadExtra is not ok right? macos10.11.0.1 is not the same as macos10.11.0?
> > Or HadExtra is an error somewhere else already?
> Right, it would be nice to check the extra stuff too. I will commit with the 
> check and a test for the extra part.
And what about "-target x86_64-apple-macos -mmacos-version-min=10.6"?


Repository:
  rC Clang

https://reviews.llvm.org/D41425



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

[PATCH] D41780: Preserve unknown STDC pragma through preprocessor

[Steven Wu via Phabricator via cfe-commits]

steven_wu created this revision.
steven_wu added reviewers: efriedma, rsmith, arphaman.

should keep the unknown STDC pragma through preprocessor and we also
should not emit warning for unknown STDC pragma during preprocessor.

rdar://problem/35724351


Repository:
  rC Clang

https://reviews.llvm.org/D41780

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Parse/Parser.h
  lib/Frontend/PrintPreprocessedOutput.cpp
  lib/Lex/Pragma.cpp
  lib/Parse/ParsePragma.cpp
  test/Preprocessor/pragma_unknown.c

Index: test/Preprocessor/pragma_unknown.c
===
--- test/Preprocessor/pragma_unknown.c
+++ test/Preprocessor/pragma_unknown.c
@@ -1,14 +1,18 @@
 // RUN: %clang_cc1 -fsyntax-only -Wunknown-pragmas -verify %s
-// RUN: %clang_cc1 -E %s | FileCheck --strict-whitespace %s
+// RUN: %clang_cc1 -E %s 2>&1 | FileCheck --strict-whitespace %s
 
 // GCC doesn't expand macro args for unrecognized pragmas.
 #define bar xX
 #pragma foo bar   // expected-warning {{unknown pragma ignored}}
+// CHECK-NOT: unknown pragma in STDC namespace
 // CHECK: {{^}}#pragma foo bar{{$}}
 
 #pragma STDC FP_CONTRACT ON
 #pragma STDC FP_CONTRACT OFF
 #pragma STDC FP_CONTRACT DEFAULT
+// CHECK: {{^}}#pragma STDC FP_CONTRACT ON{{$}}
+// CHECK: {{^}}#pragma STDC FP_CONTRACT OFF{{$}}
+// CHECK: {{^}}#pragma STDC FP_CONTRACT DEFAULT{{$}}
 #pragma STDC FP_CONTRACT IN_BETWEEN  // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
 
 #pragma STDC FENV_ACCESS ON  // expected-warning {{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
Index: lib/Parse/ParsePragma.cpp
===
--- lib/Parse/ParsePragma.cpp
+++ lib/Parse/ParsePragma.cpp
@@ -95,6 +95,17 @@
 Token &FirstToken) override;
 };
 
+/// PragmaSTDC_UnknownHandler - "\#pragma STDC ...".
+struct PragmaSTDC_UnknownHandler : public PragmaHandler {
+  PragmaSTDC_UnknownHandler() = default;
+
+  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+Token &UnknownTok) override {
+// C99 6.10.6p2, unknown forms are not allowed.
+PP.Diag(UnknownTok, diag::ext_stdc_pragma_ignored);
+  }
+};
+
 struct PragmaFPHandler : public PragmaHandler {
   PragmaFPHandler() : PragmaHandler("fp") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
@@ -233,6 +244,9 @@
   FPContractHandler.reset(new PragmaFPContractHandler());
   PP.AddPragmaHandler("STDC", FPContractHandler.get());
 
+  STDCUnknownHandler.reset(new PragmaSTDC_UnknownHandler());
+  PP.AddPragmaHandler("STDC", STDCUnknownHandler.get());
+
   PCSectionHandler.reset(new PragmaClangSectionHandler(Actions));
   PP.AddPragmaHandler("clang", PCSectionHandler.get());
 
@@ -371,6 +385,9 @@
   PP.RemovePragmaHandler("STDC", FPContractHandler.get());
   FPContractHandler.reset();
 
+  PP.RemovePragmaHandler("STDC", STDCUnknownHandler.get());
+  STDCUnknownHandler.reset();
+
   PP.RemovePragmaHandler("clang", OptimizeHandler.get());
   OptimizeHandler.reset();
 
Index: lib/Lex/Pragma.cpp
===
--- lib/Lex/Pragma.cpp
+++ lib/Lex/Pragma.cpp
@@ -1628,17 +1628,6 @@
   }
 };
 
-/// PragmaSTDC_UnknownHandler - "\#pragma STDC ...".
-struct PragmaSTDC_UnknownHandler : public PragmaHandler {
-  PragmaSTDC_UnknownHandler() = default;
-
-  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-Token &UnknownTok) override {
-// C99 6.10.6p2, unknown forms are not allowed.
-PP.Diag(UnknownTok, diag::ext_stdc_pragma_ignored);
-  }
-};
-
 /// PragmaARCCFCodeAuditedHandler - 
 ///   \#pragma clang arc_cf_code_audited begin/end
 struct PragmaARCCFCodeAuditedHandler : public PragmaHandler {
@@ -1815,9 +1804,9 @@
   ModuleHandler->AddPragma(new PragmaModuleBuildHandler());
   ModuleHandler->AddPragma(new PragmaModuleLoadHandler());
 
+  // #pragma STDC
   AddPragmaHandler("STDC", new PragmaSTDC_FENV_ACCESSHandler());
   AddPragmaHandler("STDC", new PragmaSTDC_CX_LIMITED_RANGEHandler());
-  AddPragmaHandler("STDC", new PragmaSTDC_UnknownHandler());
 
   // MS extensions.
   if (LangOpts.MicrosoftExt) {
@@ -1843,17 +1832,5 @@
   // in Preprocessor::RegisterBuiltinPragmas().
   AddPragmaHandler("GCC", new EmptyPragmaHandler());
   AddPragmaHandler("clang", new EmptyPragmaHandler());
-  if (PragmaHandler *NS = PragmaHandlers->FindHandler("STDC")) {
-// Preprocessor::RegisterBuiltinPragmas() already registers
-// PragmaSTDC_UnknownHandler as the empty handler, so remove it first,
-// otherwise there will be an assert about a duplicate handler.
-PragmaNamespace *STDCNamespace = NS->getIfNamespace();
-assert(STDCNamespace &&
-   "Invalid namespace, registered as a regular pragma handler!");
-if (PragmaHandler *Existing = STDCNamespace->FindHandler("", false

[PATCH] D41780: Preserve unknown STDC pragma through preprocessor

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In https://reviews.llvm.org/D41780#968664, @efriedma wrote:

> If you move all the #pragma STDC handlers from the lexer to the parser, you 
> might be able to avoid adding an explicit STDC handler in 
> PrintPreprocessedOutput.cpp.


If it is safe to do that, I can change it. I am not sure if the other STDC 
pragmas would affect preprocessor output in any other way (which seems not).


Repository:
  rC Clang

https://reviews.llvm.org/D41780



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

[PATCH] D41780: Preserve unknown STDC pragma through preprocessor

[Steven Wu via Phabricator via cfe-commits]

steven_wu updated this revision to Diff 128794.
steven_wu added a comment.

Move STDC pragma handler to parser.


Repository:
  rC Clang

https://reviews.llvm.org/D41780

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Parse/Parser.h
  lib/Lex/Pragma.cpp
  lib/Parse/ParsePragma.cpp
  test/Preprocessor/pragma_unknown.c

Index: test/Preprocessor/pragma_unknown.c
===
--- test/Preprocessor/pragma_unknown.c
+++ test/Preprocessor/pragma_unknown.c
@@ -1,29 +1,45 @@
 // RUN: %clang_cc1 -fsyntax-only -Wunknown-pragmas -verify %s
-// RUN: %clang_cc1 -E %s | FileCheck --strict-whitespace %s
+// RUN: %clang_cc1 -E %s 2>&1 | FileCheck --strict-whitespace %s
 
 // GCC doesn't expand macro args for unrecognized pragmas.
 #define bar xX
 #pragma foo bar   // expected-warning {{unknown pragma ignored}}
+// CHECK-NOT: unknown pragma in STDC namespace
 // CHECK: {{^}}#pragma foo bar{{$}}
 
 #pragma STDC FP_CONTRACT ON
 #pragma STDC FP_CONTRACT OFF
 #pragma STDC FP_CONTRACT DEFAULT
 #pragma STDC FP_CONTRACT IN_BETWEEN  // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
+// CHECK: {{^}}#pragma STDC FP_CONTRACT ON{{$}}
+// CHECK: {{^}}#pragma STDC FP_CONTRACT OFF{{$}}
+// CHECK: {{^}}#pragma STDC FP_CONTRACT DEFAULT{{$}}
+// CHECK: {{^}}#pragma STDC FP_CONTRACT IN_BETWEEN{{$}}
 
 #pragma STDC FENV_ACCESS ON  // expected-warning {{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
 #pragma STDC FENV_ACCESS OFF
 #pragma STDC FENV_ACCESS DEFAULT
 #pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
+// CHECK: {{^}}#pragma STDC FENV_ACCESS ON{{$}}
+// CHECK: {{^}}#pragma STDC FENV_ACCESS OFF{{$}}
+// CHECK: {{^}}#pragma STDC FENV_ACCESS DEFAULT{{$}}
+// CHECK: {{^}}#pragma STDC FENV_ACCESS IN_BETWEEN{{$}}
 
 #pragma STDC CX_LIMITED_RANGE ON
 #pragma STDC CX_LIMITED_RANGE OFF
 #pragma STDC CX_LIMITED_RANGE DEFAULT 
 #pragma STDC CX_LIMITED_RANGE IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
+// CHECK: {{^}}#pragma STDC CX_LIMITED_RANGE ON{{$}}
+// CHECK: {{^}}#pragma STDC CX_LIMITED_RANGE OFF{{$}}
+// CHECK: {{^}}#pragma STDC CX_LIMITED_RANGE DEFAULT{{$}}
+// CHECK: {{^}}#pragma STDC CX_LIMITED_RANGE IN_BETWEEN{{$}}
 
 #pragma STDC CX_LIMITED_RANGE// expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
 #pragma STDC CX_LIMITED_RANGE ON FULL POWER  // expected-warning {{expected end of directive in pragma}}
+// CHECK: {{^}}#pragma STDC CX_LIMITED_RANGE{{$}}
+// CHECK: {{^}}#pragma STDC CX_LIMITED_RANGE ON FULL POWER{{$}}
 
 #pragma STDC SO_GREAT  // expected-warning {{unknown pragma in STDC namespace}}
 #pragma STDC   // expected-warning {{unknown pragma in STDC namespace}}
-
+// CHECK: {{^}}#pragma STDC SO_GREAT{{$}}
+// CHECK: {{^}}#pragma STDC{{$}}
Index: lib/Parse/ParsePragma.cpp
===
--- lib/Parse/ParsePragma.cpp
+++ lib/Parse/ParsePragma.cpp
@@ -95,6 +95,44 @@
 Token &FirstToken) override;
 };
 
+// Pragma STDC implementations.
+
+/// PragmaSTDC_FENV_ACCESSHandler - "\#pragma STDC FENV_ACCESS ...".
+struct PragmaSTDC_FENV_ACCESSHandler : public PragmaHandler {
+  PragmaSTDC_FENV_ACCESSHandler() : PragmaHandler("FENV_ACCESS") {}
+
+  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+Token &Tok) override {
+tok::OnOffSwitch OOS;
+if (PP.LexOnOffSwitch(OOS))
+ return;
+if (OOS == tok::OOS_ON)
+  PP.Diag(Tok, diag::warn_stdc_fenv_access_not_supported);
+  }
+};
+
+/// PragmaSTDC_CX_LIMITED_RANGEHandler - "\#pragma STDC CX_LIMITED_RANGE ...".
+struct PragmaSTDC_CX_LIMITED_RANGEHandler : public PragmaHandler {
+  PragmaSTDC_CX_LIMITED_RANGEHandler() : PragmaHandler("CX_LIMITED_RANGE") {}
+
+  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+Token &Tok) override {
+tok::OnOffSwitch OOS;
+PP.LexOnOffSwitch(OOS);
+  }
+};
+
+/// PragmaSTDC_UnknownHandler - "\#pragma STDC ...".
+struct PragmaSTDC_UnknownHandler : public PragmaHandler {
+  PragmaSTDC_UnknownHandler() = default;
+
+  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+Token &UnknownTok) override {
+// C99 6.10.6p2, unknown forms are not allowed.
+PP.Diag(UnknownTok, diag::ext_stdc_pragma_ignored);
+  }
+};
+
 struct PragmaFPHandler : public PragmaHandler {
   PragmaFPHandler() : PragmaHandler("fp") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
@@ -233,6 +271,15 @@
   FPContractHandler.reset(new PragmaFPContractHandler());
   PP.AddPragmaHandler("STDC", FPContractHandler.get());
 
+  STDCFENVHandler.reset(new PragmaSTDC_FENV_ACCESSHandler());
+  PP.AddPragmaHandler("STDC", STDCFENVHandler.get());
+
+  STDCCXLIMITHandler.reset(new PragmaSTD

[PATCH] D41780: Preserve unknown STDC pragma through preprocessor

[Steven Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
steven_wu marked an inline comment as done.
Closed by commit rL321909: Preserve unknown STDC pragma through preprocessor 
(authored by steven_wu, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D41780

Files:
  cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
  cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
  cfe/trunk/include/clang/Parse/Parser.h
  cfe/trunk/lib/Lex/Pragma.cpp
  cfe/trunk/lib/Parse/ParsePragma.cpp
  cfe/trunk/test/Preprocessor/pragma_unknown.c

Index: cfe/trunk/lib/Parse/ParsePragma.cpp
===
--- cfe/trunk/lib/Parse/ParsePragma.cpp
+++ cfe/trunk/lib/Parse/ParsePragma.cpp
@@ -95,6 +95,44 @@
 Token &FirstToken) override;
 };
 
+// Pragma STDC implementations.
+
+/// PragmaSTDC_FENV_ACCESSHandler - "\#pragma STDC FENV_ACCESS ...".
+struct PragmaSTDC_FENV_ACCESSHandler : public PragmaHandler {
+  PragmaSTDC_FENV_ACCESSHandler() : PragmaHandler("FENV_ACCESS") {}
+
+  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+Token &Tok) override {
+tok::OnOffSwitch OOS;
+if (PP.LexOnOffSwitch(OOS))
+ return;
+if (OOS == tok::OOS_ON)
+  PP.Diag(Tok, diag::warn_stdc_fenv_access_not_supported);
+  }
+};
+
+/// PragmaSTDC_CX_LIMITED_RANGEHandler - "\#pragma STDC CX_LIMITED_RANGE ...".
+struct PragmaSTDC_CX_LIMITED_RANGEHandler : public PragmaHandler {
+  PragmaSTDC_CX_LIMITED_RANGEHandler() : PragmaHandler("CX_LIMITED_RANGE") {}
+
+  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+Token &Tok) override {
+tok::OnOffSwitch OOS;
+PP.LexOnOffSwitch(OOS);
+  }
+};
+
+/// PragmaSTDC_UnknownHandler - "\#pragma STDC ...".
+struct PragmaSTDC_UnknownHandler : public PragmaHandler {
+  PragmaSTDC_UnknownHandler() = default;
+
+  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
+Token &UnknownTok) override {
+// C99 6.10.6p2, unknown forms are not allowed.
+PP.Diag(UnknownTok, diag::ext_stdc_pragma_ignored);
+  }
+};
+
 struct PragmaFPHandler : public PragmaHandler {
   PragmaFPHandler() : PragmaHandler("fp") {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
@@ -233,6 +271,15 @@
   FPContractHandler.reset(new PragmaFPContractHandler());
   PP.AddPragmaHandler("STDC", FPContractHandler.get());
 
+  STDCFENVHandler.reset(new PragmaSTDC_FENV_ACCESSHandler());
+  PP.AddPragmaHandler("STDC", STDCFENVHandler.get());
+
+  STDCCXLIMITHandler.reset(new PragmaSTDC_CX_LIMITED_RANGEHandler());
+  PP.AddPragmaHandler("STDC", STDCCXLIMITHandler.get());
+
+  STDCUnknownHandler.reset(new PragmaSTDC_UnknownHandler());
+  PP.AddPragmaHandler("STDC", STDCUnknownHandler.get());
+
   PCSectionHandler.reset(new PragmaClangSectionHandler(Actions));
   PP.AddPragmaHandler("clang", PCSectionHandler.get());
 
@@ -371,6 +418,15 @@
   PP.RemovePragmaHandler("STDC", FPContractHandler.get());
   FPContractHandler.reset();
 
+  PP.RemovePragmaHandler("STDC", STDCFENVHandler.get());
+  STDCFENVHandler.reset();
+
+  PP.RemovePragmaHandler("STDC", STDCCXLIMITHandler.get());
+  STDCCXLIMITHandler.reset();
+
+  PP.RemovePragmaHandler("STDC", STDCUnknownHandler.get());
+  STDCUnknownHandler.reset();
+
   PP.RemovePragmaHandler("clang", OptimizeHandler.get());
   OptimizeHandler.reset();
 
Index: cfe/trunk/lib/Lex/Pragma.cpp
===
--- cfe/trunk/lib/Lex/Pragma.cpp
+++ cfe/trunk/lib/Lex/Pragma.cpp
@@ -1601,44 +1601,6 @@
   }
 };
 
-// Pragma STDC implementations.
-
-/// PragmaSTDC_FENV_ACCESSHandler - "\#pragma STDC FENV_ACCESS ...".
-struct PragmaSTDC_FENV_ACCESSHandler : public PragmaHandler {
-  PragmaSTDC_FENV_ACCESSHandler() : PragmaHandler("FENV_ACCESS") {}
-
-  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-Token &Tok) override {
-tok::OnOffSwitch OOS;
-if (PP.LexOnOffSwitch(OOS))
- return;
-if (OOS == tok::OOS_ON)
-  PP.Diag(Tok, diag::warn_stdc_fenv_access_not_supported);
-  }
-};
-
-/// PragmaSTDC_CX_LIMITED_RANGEHandler - "\#pragma STDC CX_LIMITED_RANGE ...".
-struct PragmaSTDC_CX_LIMITED_RANGEHandler : public PragmaHandler {
-  PragmaSTDC_CX_LIMITED_RANGEHandler() : PragmaHandler("CX_LIMITED_RANGE") {}
-
-  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-Token &Tok) override {
-tok::OnOffSwitch OOS;
-PP.LexOnOffSwitch(OOS);
-  }
-};
-
-/// PragmaSTDC_UnknownHandler - "\#pragma STDC ...".
-struct PragmaSTDC_UnknownHandler : public PragmaHandler {
-  PragmaSTDC_UnknownHandler() = default;
-
-  void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
-Token &UnknownTok) override {
-// C99 6.10.6p2, unknown forms are not allowed.
-PP.Diag(UnknownTok, diag::ext_stdc_pragma_ignored

[PATCH] D44670: [CXX] Templates specialization visibility can be wrong

[Steven Wu via Phabricator via cfe-commits]

steven_wu created this revision.
steven_wu added reviewers: rsmith, arphaman.

Under some conditions, LinkageComputer can get the visibility for
ClassTemplateSpecializationDecl wrong because it failed to find the Decl
that has the explicit visibility.

This fixes:
llvm.org/bugs/pr36810
rdar://problem/38080953


Repository:
  rC Clang

https://reviews.llvm.org/D44670

Files:
  lib/AST/Decl.cpp
  test/CodeGenCXX/visibility-pr36810.cpp


Index: test/CodeGenCXX/visibility-pr36810.cpp
===
--- /dev/null
+++ test/CodeGenCXX/visibility-pr36810.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx -std=c++11 -fvisibility hidden 
-emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx -DUNDEF_G -std=c++11 
-fvisibility hidden -emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s
+
+namespace std {
+template 
+class __attribute__((__type_visibility__("default"))) shared_ptr {
+  template  friend class shared_ptr;
+};
+}
+struct dict;
+#ifndef UNDEF_G
+std::shared_ptr g;
+#endif
+class __attribute__((visibility("default"))) Bar;
+template >
+class __attribute__((visibility("default"))) i {
+  std::shared_ptr foo() const;
+};
+
+// CHECK: define void @_ZNK1iISt10shared_ptrI3BarEE3fooEv
+template <> std::shared_ptr i<>::foo() const {
+  return std::shared_ptr();
+}
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -1073,9 +1073,15 @@
   // If there wasn't explicit visibility there, and this is a
   // specialization of a class template, check for visibility
   // on the pattern.
-  if (const auto *spec = dyn_cast(ND))
-return getVisibilityOf(spec->getSpecializedTemplate()->getTemplatedDecl(),
-   kind);
+  if (const auto *spec = dyn_cast(ND)) {
+for (const auto *RD :
+ spec->getSpecializedTemplate()->getTemplatedDecl()->redecls()) {
+  auto Vis = getVisibilityOf(RD, kind);
+  if (Vis != None)
+return Vis;
+}
+return None;
+  }
 
   // Use the most recent declaration.
   if (!IsMostRecent && !isa(ND)) {


Index: test/CodeGenCXX/visibility-pr36810.cpp
===
--- /dev/null
+++ test/CodeGenCXX/visibility-pr36810.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx -std=c++11 -fvisibility hidden -emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx -DUNDEF_G -std=c++11 -fvisibility hidden -emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s
+
+namespace std {
+template 
+class __attribute__((__type_visibility__("default"))) shared_ptr {
+  template  friend class shared_ptr;
+};
+}
+struct dict;
+#ifndef UNDEF_G
+std::shared_ptr g;
+#endif
+class __attribute__((visibility("default"))) Bar;
+template >
+class __attribute__((visibility("default"))) i {
+  std::shared_ptr foo() const;
+};
+
+// CHECK: define void @_ZNK1iISt10shared_ptrI3BarEE3fooEv
+template <> std::shared_ptr i<>::foo() const {
+  return std::shared_ptr();
+}
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -1073,9 +1073,15 @@
   // If there wasn't explicit visibility there, and this is a
   // specialization of a class template, check for visibility
   // on the pattern.
-  if (const auto *spec = dyn_cast(ND))
-return getVisibilityOf(spec->getSpecializedTemplate()->getTemplatedDecl(),
-   kind);
+  if (const auto *spec = dyn_cast(ND)) {
+for (const auto *RD :
+ spec->getSpecializedTemplate()->getTemplatedDecl()->redecls()) {
+  auto Vis = getVisibilityOf(RD, kind);
+  if (Vis != None)
+return Vis;
+}
+return None;
+  }
 
   // Use the most recent declaration.
   if (!IsMostRecent && !isa(ND)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

[Steven Wu via Phabricator via cfe-commits]

steven_wu created this revision.
steven_wu added reviewers: arphaman, dexonsmith.
Herald added subscribers: jkorous, inglorion, mehdi_amini.

After r327851, Driver::GetTemporaryPath will create the file rather than
just create a potientially unqine filename. If clang driver pass the
file as parameter as -object_path_lto, ld64 will pass it back to libLTO
as GeneratedObjectsDirectory, which is going to cause a LLVM ERROR if it
is not a directory.
Now during thinLTO, pass a temp directory path to linker instread.

rdar://problem/47194182


Repository:
  rC Clang

https://reviews.llvm.org/D56608

Files:
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Darwin.cpp


Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -224,13 +224,20 @@
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  if (D.isUsingLTO()) {
-// If we are using LTO, then automatically create a temporary file path for
-// the linker to use, so that it's lifetime will extend past a possible
-// dsymutil step.
-if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
-  const char *TmpPath = C.getArgs().MakeArgString(
-  D.GetTemporaryPath("cc", 
types::getTypeTempSuffix(types::TY_Object)));
+  if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
+std::string TmpPathName;
+if (D.getLTOMode() == LTOK_Full) {
+  // If we are using full LTO, then automatically create a temporary file
+  // path for the linker to use, so that it's lifetime will extend past a
+  // possible dsymutil step.
+  TmpPathName =
+  D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object));
+} else if (D.getLTOMode() == LTOK_Thin)
+  // If we are using thin LTO, then create a directory instead.
+  TmpPathName = D.GetTemporaryDirectory("thinlto");
+
+if (!TmpPathName.empty()) {
+  auto *TmpPath = C.getArgs().MakeArgString(TmpPathName);
   C.addTempFile(TmpPath);
   CmdArgs.push_back("-object_path_lto");
   CmdArgs.push_back(TmpPath);
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -4478,6 +4478,17 @@
   return Path.str();
 }
 
+std::string Driver::GetTemporaryDirectory(StringRef Prefix) const {
+  SmallString<128> Path;
+  std::error_code EC = llvm::sys::fs::createUniqueDirectory(Prefix, Path);
+  if (EC) {
+Diag(clang::diag::err_unable_to_make_temp) << EC.message();
+return "";
+  }
+
+  return Path.str();
+}
+
 std::string Driver::GetClPchPath(Compilation &C, StringRef BaseName) const {
   SmallString<128> Output;
   if (Arg *FpArg = C.getArgs().getLastArg(options::OPT__SLASH_Fp)) {
Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -505,6 +505,10 @@
   /// GCC goes to extra lengths here to be a bit more robust.
   std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
 
+  /// GetTemporaryPath - Return the pathname of a temporary directory to use
+  /// as part of compilation; the directory will have the given prefix.
+  std::string GetTemporaryDirectory(StringRef Prefix) const;
+
   /// Return the pathname of the pch file in clang-cl mode.
   std::string GetClPchPath(Compilation &C, StringRef BaseName) const;
 


Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -224,13 +224,20 @@
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  if (D.isUsingLTO()) {
-// If we are using LTO, then automatically create a temporary file path for
-// the linker to use, so that it's lifetime will extend past a possible
-// dsymutil step.
-if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
-  const char *TmpPath = C.getArgs().MakeArgString(
-  D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object)));
+  if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
+std::string TmpPathName;
+if (D.getLTOMode() == LTOK_Full) {
+  // If we are using full LTO, then automatically create a temporary file
+  // path for the linker to use, so that it's lifetime will extend past a
+  // possible dsymutil step.
+  TmpPathName =
+  D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object));
+} else if (D.getLTOMode() == LTOK_Thin)
+  // If we are using thin LTO, then create a directory instead.
+  TmpPathName = D.GetTemporaryDirectory("thinlto");
+
+if (!TmpPathName.empty()) {
+  auto *TmpPath = C.getArgs().M

[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

[Steven Wu via Phabricator via cfe-commits]

steven_wu updated this revision to Diff 181324.
steven_wu added a comment.

I was planning to add a test but I am not sure how to check the file type of 
temporary files.

I add a test to check for temp file names because I do create file and 
directory with different prefix.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56608/new/

https://reviews.llvm.org/D56608

Files:
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Darwin.cpp
  test/Driver/darwin-ld-lto.c


Index: test/Driver/darwin-ld-lto.c
===
--- test/Driver/darwin-ld-lto.c
+++ test/Driver/darwin-ld-lto.c
@@ -17,3 +17,14 @@
 // RUN: %clang -target x86_64-apple-darwin10 -### %s \
 // RUN:   -ccc-install-dir %S/dummytestdir -mlinker-version=133 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s -input-file %t.log
+
+
+// Check that -object_lto_path is passed correctly to ld64
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=full -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
+// FULL_LTO_OBJECT_PATH: /usr/bin/ld
+// FULL_LTO_OBJECT_PATH-SAME: "-object_path_lto" 
"{{[a-zA-Z0-9_\/]+\/cc\-[a-zA-Z0-9_]+.o}}"
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=thin -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=THIN_LTO_OBJECT_PATH %s
+// THIN_LTO_OBJECT_PATH: /usr/bin/ld
+// THIN_LTO_OBJECT_PATH-SAME: "-object_path_lto" 
"{{[a-zA-Z0-9_\/]+\/thinlto\-[a-zA-Z0-9_]+}}"
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -224,13 +224,20 @@
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  if (D.isUsingLTO()) {
-// If we are using LTO, then automatically create a temporary file path for
-// the linker to use, so that it's lifetime will extend past a possible
-// dsymutil step.
-if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
-  const char *TmpPath = C.getArgs().MakeArgString(
-  D.GetTemporaryPath("cc", 
types::getTypeTempSuffix(types::TY_Object)));
+  if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
+std::string TmpPathName;
+if (D.getLTOMode() == LTOK_Full) {
+  // If we are using full LTO, then automatically create a temporary file
+  // path for the linker to use, so that it's lifetime will extend past a
+  // possible dsymutil step.
+  TmpPathName =
+  D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object));
+} else if (D.getLTOMode() == LTOK_Thin)
+  // If we are using thin LTO, then create a directory instead.
+  TmpPathName = D.GetTemporaryDirectory("thinlto");
+
+if (!TmpPathName.empty()) {
+  auto *TmpPath = C.getArgs().MakeArgString(TmpPathName);
   C.addTempFile(TmpPath);
   CmdArgs.push_back("-object_path_lto");
   CmdArgs.push_back(TmpPath);
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -4478,6 +4478,17 @@
   return Path.str();
 }
 
+std::string Driver::GetTemporaryDirectory(StringRef Prefix) const {
+  SmallString<128> Path;
+  std::error_code EC = llvm::sys::fs::createUniqueDirectory(Prefix, Path);
+  if (EC) {
+Diag(clang::diag::err_unable_to_make_temp) << EC.message();
+return "";
+  }
+
+  return Path.str();
+}
+
 std::string Driver::GetClPchPath(Compilation &C, StringRef BaseName) const {
   SmallString<128> Output;
   if (Arg *FpArg = C.getArgs().getLastArg(options::OPT__SLASH_Fp)) {
Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -505,6 +505,10 @@
   /// GCC goes to extra lengths here to be a bit more robust.
   std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
 
+  /// GetTemporaryPath - Return the pathname of a temporary directory to use
+  /// as part of compilation; the directory will have the given prefix.
+  std::string GetTemporaryDirectory(StringRef Prefix) const;
+
   /// Return the pathname of the pch file in clang-cl mode.
   std::string GetClPchPath(Compilation &C, StringRef BaseName) const;
 


Index: test/Driver/darwin-ld-lto.c
===
--- test/Driver/darwin-ld-lto.c
+++ test/Driver/darwin-ld-lto.c
@@ -17,3 +17,14 @@
 // RUN: %clang -target x86_64-apple-darwin10 -### %s \
 // RUN:   -ccc-install-dir %S/dummytestdir -mlinker-version=133 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s -input-file %t.log
+
+
+// Check that -object_lto_path is passed correctly to ld64
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=full -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=FULL_LTO_OBJECT_PATH

[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

[Steven Wu via Phabricator via cfe-commits]

steven_wu updated this revision to Diff 181325.
steven_wu added a comment.

Fix the comment


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56608/new/

https://reviews.llvm.org/D56608

Files:
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Darwin.cpp
  test/Driver/darwin-ld-lto.c


Index: test/Driver/darwin-ld-lto.c
===
--- test/Driver/darwin-ld-lto.c
+++ test/Driver/darwin-ld-lto.c
@@ -17,3 +17,14 @@
 // RUN: %clang -target x86_64-apple-darwin10 -### %s \
 // RUN:   -ccc-install-dir %S/dummytestdir -mlinker-version=133 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s -input-file %t.log
+
+
+// Check that -object_lto_path is passed correctly to ld64
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=full -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
+// FULL_LTO_OBJECT_PATH: /usr/bin/ld
+// FULL_LTO_OBJECT_PATH-SAME: "-object_path_lto" 
"{{[a-zA-Z0-9_\/]+\/cc\-[a-zA-Z0-9_]+.o}}"
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=thin -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=THIN_LTO_OBJECT_PATH %s
+// THIN_LTO_OBJECT_PATH: /usr/bin/ld
+// THIN_LTO_OBJECT_PATH-SAME: "-object_path_lto" 
"{{[a-zA-Z0-9_\/]+\/thinlto\-[a-zA-Z0-9_]+}}"
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -224,13 +224,20 @@
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  if (D.isUsingLTO()) {
-// If we are using LTO, then automatically create a temporary file path for
-// the linker to use, so that it's lifetime will extend past a possible
-// dsymutil step.
-if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
-  const char *TmpPath = C.getArgs().MakeArgString(
-  D.GetTemporaryPath("cc", 
types::getTypeTempSuffix(types::TY_Object)));
+  if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
+std::string TmpPathName;
+if (D.getLTOMode() == LTOK_Full) {
+  // If we are using full LTO, then automatically create a temporary file
+  // path for the linker to use, so that it's lifetime will extend past a
+  // possible dsymutil step.
+  TmpPathName =
+  D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object));
+} else if (D.getLTOMode() == LTOK_Thin)
+  // If we are using thin LTO, then create a directory instead.
+  TmpPathName = D.GetTemporaryDirectory("thinlto");
+
+if (!TmpPathName.empty()) {
+  auto *TmpPath = C.getArgs().MakeArgString(TmpPathName);
   C.addTempFile(TmpPath);
   CmdArgs.push_back("-object_path_lto");
   CmdArgs.push_back(TmpPath);
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -4478,6 +4478,17 @@
   return Path.str();
 }
 
+std::string Driver::GetTemporaryDirectory(StringRef Prefix) const {
+  SmallString<128> Path;
+  std::error_code EC = llvm::sys::fs::createUniqueDirectory(Prefix, Path);
+  if (EC) {
+Diag(clang::diag::err_unable_to_make_temp) << EC.message();
+return "";
+  }
+
+  return Path.str();
+}
+
 std::string Driver::GetClPchPath(Compilation &C, StringRef BaseName) const {
   SmallString<128> Output;
   if (Arg *FpArg = C.getArgs().getLastArg(options::OPT__SLASH_Fp)) {
Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -505,6 +505,10 @@
   /// GCC goes to extra lengths here to be a bit more robust.
   std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
 
+  /// GetTemporaryDirectory - Return the pathname of a temporary directory to
+  /// use as part of compilation; the directory will have the given prefix.
+  std::string GetTemporaryDirectory(StringRef Prefix) const;
+
   /// Return the pathname of the pch file in clang-cl mode.
   std::string GetClPchPath(Compilation &C, StringRef BaseName) const;
 


Index: test/Driver/darwin-ld-lto.c
===
--- test/Driver/darwin-ld-lto.c
+++ test/Driver/darwin-ld-lto.c
@@ -17,3 +17,14 @@
 // RUN: %clang -target x86_64-apple-darwin10 -### %s \
 // RUN:   -ccc-install-dir %S/dummytestdir -mlinker-version=133 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s -input-file %t.log
+
+
+// Check that -object_lto_path is passed correctly to ld64
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=full -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
+// FULL_LTO_OBJECT_PATH: /usr/bin/ld
+// FULL_LTO_OBJECT_PATH-SAME: "-object_path_lto" "{{[a-zA-Z0-9_\/]+\/cc\-[a-zA-Z0-9_]+.o}}"
+// RUN: %clang -target x86_64-apple-darwin10

[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO

[Steven Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC350970: [Darwin][Driver] Don't pass a file as 
object_path_lto during ThinLTO (authored by steven_wu, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D56608?vs=181325&id=181361#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56608/new/

https://reviews.llvm.org/D56608

Files:
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Darwin.cpp
  test/Driver/darwin-ld-lto.c


Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -505,6 +505,10 @@
   /// GCC goes to extra lengths here to be a bit more robust.
   std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
 
+  /// GetTemporaryDirectory - Return the pathname of a temporary directory to
+  /// use as part of compilation; the directory will have the given prefix.
+  std::string GetTemporaryDirectory(StringRef Prefix) const;
+
   /// Return the pathname of the pch file in clang-cl mode.
   std::string GetClPchPath(Compilation &C, StringRef BaseName) const;
 
Index: test/Driver/darwin-ld-lto.c
===
--- test/Driver/darwin-ld-lto.c
+++ test/Driver/darwin-ld-lto.c
@@ -17,3 +17,14 @@
 // RUN: %clang -target x86_64-apple-darwin10 -### %s \
 // RUN:   -ccc-install-dir %S/dummytestdir -mlinker-version=133 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s -input-file %t.log
+
+
+// Check that -object_lto_path is passed correctly to ld64
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=full -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
+// FULL_LTO_OBJECT_PATH: /usr/bin/ld
+// FULL_LTO_OBJECT_PATH-SAME: "-object_path_lto" 
"{{[a-zA-Z0-9_\/]+\/cc\-[a-zA-Z0-9_]+.o}}"
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=thin -### 2>&1 | \
+// RUN:   FileCheck -check-prefix=THIN_LTO_OBJECT_PATH %s
+// THIN_LTO_OBJECT_PATH: /usr/bin/ld
+// THIN_LTO_OBJECT_PATH-SAME: "-object_path_lto" 
"{{[a-zA-Z0-9_\/]+\/thinlto\-[a-zA-Z0-9_]+}}"
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -224,13 +224,20 @@
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  if (D.isUsingLTO()) {
-// If we are using LTO, then automatically create a temporary file path for
-// the linker to use, so that it's lifetime will extend past a possible
-// dsymutil step.
-if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
-  const char *TmpPath = C.getArgs().MakeArgString(
-  D.GetTemporaryPath("cc", 
types::getTypeTempSuffix(types::TY_Object)));
+  if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
+std::string TmpPathName;
+if (D.getLTOMode() == LTOK_Full) {
+  // If we are using full LTO, then automatically create a temporary file
+  // path for the linker to use, so that it's lifetime will extend past a
+  // possible dsymutil step.
+  TmpPathName =
+  D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object));
+} else if (D.getLTOMode() == LTOK_Thin)
+  // If we are using thin LTO, then create a directory instead.
+  TmpPathName = D.GetTemporaryDirectory("thinlto");
+
+if (!TmpPathName.empty()) {
+  auto *TmpPath = C.getArgs().MakeArgString(TmpPathName);
   C.addTempFile(TmpPath);
   CmdArgs.push_back("-object_path_lto");
   CmdArgs.push_back(TmpPath);
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -4478,6 +4478,17 @@
   return Path.str();
 }
 
+std::string Driver::GetTemporaryDirectory(StringRef Prefix) const {
+  SmallString<128> Path;
+  std::error_code EC = llvm::sys::fs::createUniqueDirectory(Prefix, Path);
+  if (EC) {
+Diag(clang::diag::err_unable_to_make_temp) << EC.message();
+return "";
+  }
+
+  return Path.str();
+}
+
 std::string Driver::GetClPchPath(Compilation &C, StringRef BaseName) const {
   SmallString<128> Output;
   if (Arg *FpArg = C.getArgs().getLastArg(options::OPT__SLASH_Fp)) {


Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -505,6 +505,10 @@
   /// GCC goes to extra lengths here to be a bit more robust.
   std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
 
+  /// GetTemporaryDirectory - Return the pathname of a temporary directory to
+  /// use as part of compilation; the directory will have the given prefix.
+  std::string GetTemporaryDirectory(StringRef Prefix) const;
+
   ///

[PATCH] D55525: [Driver] Add support for -fembed-bitcode for assembly file

[Steven Wu via Phabricator via cfe-commits]

steven_wu created this revision.
steven_wu added reviewers: compnerd, dexonsmith.
Herald added a subscriber: jkorous.

Handle -fembed-bitcode for assembly inputs. When the input file is
assembly, write a marker as "__LLVM,__asm" section.

Fix llvm.org/pr39659


Repository:
  rC Clang

https://reviews.llvm.org/D55525

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/embed-bitcode.s
  tools/driver/cc1as_main.cpp

Index: tools/driver/cc1as_main.cpp
===
--- tools/driver/cc1as_main.cpp
+++ tools/driver/cc1as_main.cpp
@@ -33,6 +33,7 @@
 #include "llvm/MC/MCParser/MCAsmParser.h"
 #include "llvm/MC/MCParser/MCTargetAsmParser.h"
 #include "llvm/MC/MCRegisterInfo.h"
+#include "llvm/MC/MCSectionMachO.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCTargetOptions.h"
@@ -132,6 +133,7 @@
   unsigned NoExecStack : 1;
   unsigned FatalWarnings : 1;
   unsigned IncrementalLinkerCompatible : 1;
+  unsigned EmbedBitcode : 1;
 
   /// The name of the relocation model to use.
   std::string RelocationModel;
@@ -153,6 +155,7 @@
 FatalWarnings = 0;
 IncrementalLinkerCompatible = 0;
 DwarfVersion = 0;
+EmbedBitcode = 0;
   }
 
   static bool CreateFromArgs(AssemblerInvocation &Res,
@@ -284,6 +287,16 @@
   Args.hasArg(OPT_mincremental_linker_compatible);
   Opts.SymbolDefs = Args.getAllArgValues(OPT_defsym);
 
+  // EmbedBitcode Option. If -fembed-bitcode is enabled, set the flag.
+  // EmbedBitcode behaves the same for all embed options for assembly files.
+  if (auto *A = Args.getLastArg(OPT_fembed_bitcode_EQ)) {
+Opts.EmbedBitcode = llvm::StringSwitch(A->getValue())
+.Case("all", 1)
+.Case("bitcode", 1)
+.Case("marker", 1)
+.Default(0);
+  }
+
   return Success;
 }
 
@@ -449,6 +462,16 @@
 Str.get()->InitSections(Opts.NoExecStack);
   }
 
+  // When -fembed-bitcode is passed to clang_as, a 1-byte marker
+  // is emitted in __LLVM,__asm section if the object file is MachO format.
+  if (Opts.EmbedBitcode && Ctx.getObjectFileInfo()->getObjectFileType() ==
+   MCObjectFileInfo::IsMachO) {
+MCSection *AsmLabel = Ctx.getMachOSection(
+"__LLVM", "__asm", MachO::S_REGULAR, 4, SectionKind::getReadOnly());
+Str.get()->SwitchSection(AsmLabel);
+Str.get()->EmitZeros(1);
+  }
+
   // Assembly to object compilation should leverage assembly info.
   Str->setUseAssemblerInfoForParsing(true);
 
Index: test/Driver/embed-bitcode.s
===
--- /dev/null
+++ test/Driver/embed-bitcode.s
@@ -0,0 +1,12 @@
+// REQUIRES: arm-registered-target
+
+// RUN: %clang -c -target armv7-apple-ios10 %s -fembed-bitcode -### 2>&1 | FileCheck %s -check-prefix=CHECK-AS
+// RUN: %clang -c -target armv7-apple-ios10 %s -fembed-bitcode-marker -### 2>&1 | FileCheck %s -check-prefix=CHECK-AS-MARKER
+// CHECK-AS: -cc1as
+// CHECK-AS: -fembed-bitcode=all
+// CHECK-AS-MARKER: -fembed-bitcode=marker
+
+// RUN: %clang -c -target armv7-apple-ios10 %s -fembed-bitcode -o %t.o
+// RUN: llvm-readobj -section-headers %t.o | FileCheck --check-prefix=CHECK-SECTION %s
+// CHECK-SECTION: Name: __asm
+// CHECK-SECTION-NEXT: Segment: __LLVM
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2167,6 +2167,11 @@
 CmdArgs.push_back("-target-feature");
 CmdArgs.push_back(MipsTargetFeature);
   }
+
+  // forward -fembed-bitcode to assmebler
+  if (C.getDriver().embedBitcodeEnabled() ||
+  C.getDriver().embedBitcodeMarkerOnly())
+Args.AddLastArg(CmdArgs, options::OPT_fembed_bitcode_EQ);
 }
 
 static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -675,7 +675,7 @@
   Flags<[DriverOption]>;
 
 def fembed_bitcode_EQ : Joined<["-"], "fembed-bitcode=">,
-Group, Flags<[DriverOption, CC1Option]>, MetaVarName<"">,
+Group, Flags<[DriverOption, CC1Option, CC1AsOption]>, MetaVarName<"">,
 HelpText<"Embed LLVM bitcode (option: off, all, bitcode, marker)">;
 def fembed_bitcode : Flag<["-"], "fembed-bitcode">, Group,
   Alias, AliasArgs<["all"]>,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D21230: Do not embed all the cc1 options in bitcode commandline

[Steven Wu via Phabricator via cfe-commits]

steven_wu abandoned this revision.
steven_wu added a comment.
Herald added subscribers: jkorous, mehdi_amini.

This is upstreamed by Saleem already


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D21230/new/

https://reviews.llvm.org/D21230



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

[PATCH] D60302: [CodeGen][ObjC] Emit the retainRV marker as a module flag instead of named metadata.

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

I talked with Akira offline and we think this is probably the best approach to 
fix this LTO issue. I will leave others to comment if they think otherwise.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60302/new/

https://reviews.llvm.org/D60302



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

[PATCH] D60516: [LTO] Add plumbing to save stats during LTO on Darwin.

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

LGTM with one additional small comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60516/new/

https://reviews.llvm.org/D60516



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

[PATCH] D60516: [LTO] Add plumbing to save stats during LTO on Darwin.

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

forgot to save the inline comments.




Comment at: llvm/lib/LTO/LTOCodeGenerator.cpp:601
   // If statistics were requested, print them out after codegen.
-  if (llvm::AreStatisticsEnabled())
+  if (llvm::AreStatisticsEnabled() && !StatsFile)
 llvm::PrintStatistics();

You can simplify the logic a bit here.
```
if (StatsFile)
...
else if (llvm::AreStatisticsEnabled())
...
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60516/new/

https://reviews.llvm.org/D60516



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

[PATCH] D61627: [clang driver] Allow -fembed-bitcode combined with -mno-red-zone

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

The main concern for adding this `blacklist` was because of long term 
maintainability since the option is going to be embedded into bitcode. Looking 
at this specific option, I have no reason against because it doesn't affect 
embedded compiler flags.

I added Tim to see if allowing disable redzone can affect ABI and if it can be 
supported.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61627/new/

https://reviews.llvm.org/D61627



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

[PATCH] D61627: [clang driver] Allow -fembed-bitcode combined with -mno-red-zone

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In D61627#1493674 , @compnerd wrote:

> @steven_wu - yeah, `-mred-zone`, `-mno-red-zone`  does impact the ABI, at 
> least on x86_64.  It changes the way that the arguments are spilled and the 
> stack layout.


Well, I am not concern about ABI in that sense and not x86_64. I am concerning 
about ABI re-targeting from armv7k to arm64_32. It might be just fine but I 
can't tell for sure.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61627/new/

https://reviews.llvm.org/D61627



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

[PATCH] D61627: [clang driver] Allow -fembed-bitcode combined with -mno-red-zone

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

Like I said, I am not worried that -mno-red-zone itself changes the ABI. As 
long as LLVM still respect the attribute the same way, it is fine. I want to 
consult Tim to make sure we can support re-targeting no red zone from armv7k to 
arm64_32.

The intention for the black list is for maintainability. Limiting the ability 
to change the backend behavior means less corner case to worry about when we 
re-compile armv7k to arm64_32. Why are you interested in expending this list? 
Do you have any specific use case in mind? The other option is factor the 
blacklist out to be a Darwin toolchain specific overwrite so you can further 
relax it on other platforms.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61627/new/

https://reviews.llvm.org/D61627



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

[PATCH] D52252: Driver: render arguments for the embedded bitcode correctly

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

Thanks for doing this!

Can you add some test cases just to be complete? Other than that, LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D52252



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

[PATCH] D43737: Improve -Winfinite-recursion

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.
Herald added a project: LLVM.

Sorry for following up late on the patch. Removing the reachability testing for 
the exit block causes false positive for infinite loop cases like this:

  void l() {
static int count = 5;
if (count >0) {
  count--;
  l();
}
while (true) {}
  }

Can you take a look? I was attempting to write a fix but then I figure out it 
is very much the same as old algorithm.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D43737/new/

https://reviews.llvm.org/D43737



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

[PATCH] D57991: [Driver][Darwin] Emit an error when using -pg on OS without support for it.

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

LGTM with a suggestion to make code cleaner.




Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:101
+  "the clang compiler does not support -pg option on Darwin">;
+def err_drv_clang_unsupported_opt_pg_darwin_osx: Error<
+  "the clang compiler does not support -pg option on versions of OS X 10.9 and 
later">;

Might be cleaner if you use %select here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D57991/new/

https://reviews.llvm.org/D57991



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

[PATCH] D58122: Restore Check for Unreachable Exit Block in -Winfinite-recursion

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.

LGTM


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58122/new/

https://reviews.llvm.org/D58122



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

[PATCH] D61627: [clang driver] Allow -fembed-bitcode combined with -mno-red-zone

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In D61627#1497919 , @wanders wrote:

> > Why are you interested in expending this list?
>
> I have a (kernel) that is compiled with `-mno-red-zone` and `-mcmodel=large` 
> which I want to compile with `-fembed-bitcode` for a debugging tool that 
> needs the bitcode.
>
> But I can carry these patches locally for now.


I think the best option for this situation might be just make this blacklist to 
be darwin toolchain specific.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61627/new/

https://reviews.llvm.org/D61627



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

[PATCH] D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

Thanks for doing this. I think module flag is a good idea. Some comments inline.




Comment at: clang/test/CodeGen/svml-calls.ll:16
+
+define void @sin_f64(double* nocapture %varray) {
+; CHECK-LABEL: @sin_f64(

Personally, I think codegen tests like this will be cleaner to keep in LLVM. 
Clang tests just test the IRGen of the module flag and LLVM tests check that 
those flags are respected and module flag merge is respected.



Comment at: llvm/lib/LTO/LTOBackend.cpp:221
 
+static TargetLibraryInfoImpl *createTLII(Module &Mod, TargetMachine *TM) {
+  TargetLibraryInfoImpl *TLII =

Should this be done not just for LTOBackend but for regular compilation as 
well? LegacyCodegenerator and llc can all be benefit from a matching 
TargetLibraryInfo?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60162/new/

https://reviews.llvm.org/D60162



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

[PATCH] D60162: [ThinLTO] Add module flags for TargetLibraryInfoImpl and use in LTO backends

[Steven Wu via Phabricator via cfe-commits]

steven_wu added inline comments.



Comment at: llvm/lib/LTO/LTOBackend.cpp:221
 
+static TargetLibraryInfoImpl *createTLII(Module &Mod, TargetMachine *TM) {
+  TargetLibraryInfoImpl *TLII =

tejohnson wrote:
> tejohnson wrote:
> > steven_wu wrote:
> > > Should this be done not just for LTOBackend but for regular compilation 
> > > as well? LegacyCodegenerator and llc can all be benefit from a matching 
> > > TargetLibraryInfo?
> > Yeah, probably. I think the best way to have this utilized everywhere is to 
> > move the below code into the TargetLibraryInfoImpl itself - by having it 
> > also take the Module as a parameter). Probably as a required parameter, to 
> > ensure it is used consistently. WDYT? 
> I meant, "into the TargetLibraryInfoImpl constructor"
SGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60162/new/

https://reviews.llvm.org/D60162



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

[PATCH] D55525: [Driver] Add support for -fembed-bitcode for assembly file

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In D55525#1327742 , @compnerd wrote:

> This really feels odd.  Why not expect that the developer will add the 
> content themselves?  I'm not sure I understand the motivation for this change.


The main motivation for upstreaming this is to make -fembed-bitcode behaves the 
same as Apple clang.
The section is just a marker for ld64 to tell the linker there is no bitcode 
available for this specific module because it is built from assembly. If ld64 
sees this marker, it will pull the object file into the bitcode bundle, rather 
than error out and complaining about missing bitcode.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55525/new/

https://reviews.llvm.org/D55525



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

[PATCH] D55525: [Driver] Add support for -fembed-bitcode for assembly file

[Steven Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC348943: [Driver] Add support for -fembed-bitcode for 
assembly file (authored by steven_wu, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D55525?vs=177563&id=177867#toc

Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55525/new/

https://reviews.llvm.org/D55525

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/embed-bitcode.s
  tools/driver/cc1as_main.cpp

Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -675,7 +675,7 @@
   Flags<[DriverOption]>;
 
 def fembed_bitcode_EQ : Joined<["-"], "fembed-bitcode=">,
-Group, Flags<[DriverOption, CC1Option]>, MetaVarName<"">,
+Group, Flags<[DriverOption, CC1Option, CC1AsOption]>, MetaVarName<"">,
 HelpText<"Embed LLVM bitcode (option: off, all, bitcode, marker)">;
 def fembed_bitcode : Flag<["-"], "fembed-bitcode">, Group,
   Alias, AliasArgs<["all"]>,
Index: test/Driver/embed-bitcode.s
===
--- test/Driver/embed-bitcode.s
+++ test/Driver/embed-bitcode.s
@@ -0,0 +1,12 @@
+// REQUIRES: arm-registered-target
+
+// RUN: %clang -c -target armv7-apple-ios10 %s -fembed-bitcode -### 2>&1 | FileCheck %s -check-prefix=CHECK-AS
+// RUN: %clang -c -target armv7-apple-ios10 %s -fembed-bitcode-marker -### 2>&1 | FileCheck %s -check-prefix=CHECK-AS-MARKER
+// CHECK-AS: -cc1as
+// CHECK-AS: -fembed-bitcode=all
+// CHECK-AS-MARKER: -fembed-bitcode=marker
+
+// RUN: %clang -c -target armv7-apple-ios10 %s -fembed-bitcode -o %t.o
+// RUN: llvm-readobj -section-headers %t.o | FileCheck --check-prefix=CHECK-SECTION %s
+// CHECK-SECTION: Name: __asm
+// CHECK-SECTION-NEXT: Segment: __LLVM
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2167,6 +2167,11 @@
 CmdArgs.push_back("-target-feature");
 CmdArgs.push_back(MipsTargetFeature);
   }
+
+  // forward -fembed-bitcode to assmebler
+  if (C.getDriver().embedBitcodeEnabled() ||
+  C.getDriver().embedBitcodeMarkerOnly())
+Args.AddLastArg(CmdArgs, options::OPT_fembed_bitcode_EQ);
 }
 
 static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
Index: tools/driver/cc1as_main.cpp
===
--- tools/driver/cc1as_main.cpp
+++ tools/driver/cc1as_main.cpp
@@ -33,6 +33,7 @@
 #include "llvm/MC/MCParser/MCAsmParser.h"
 #include "llvm/MC/MCParser/MCTargetAsmParser.h"
 #include "llvm/MC/MCRegisterInfo.h"
+#include "llvm/MC/MCSectionMachO.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCTargetOptions.h"
@@ -132,6 +133,7 @@
   unsigned NoExecStack : 1;
   unsigned FatalWarnings : 1;
   unsigned IncrementalLinkerCompatible : 1;
+  unsigned EmbedBitcode : 1;
 
   /// The name of the relocation model to use.
   std::string RelocationModel;
@@ -153,6 +155,7 @@
 FatalWarnings = 0;
 IncrementalLinkerCompatible = 0;
 DwarfVersion = 0;
+EmbedBitcode = 0;
   }
 
   static bool CreateFromArgs(AssemblerInvocation &Res,
@@ -284,6 +287,16 @@
   Args.hasArg(OPT_mincremental_linker_compatible);
   Opts.SymbolDefs = Args.getAllArgValues(OPT_defsym);
 
+  // EmbedBitcode Option. If -fembed-bitcode is enabled, set the flag.
+  // EmbedBitcode behaves the same for all embed options for assembly files.
+  if (auto *A = Args.getLastArg(OPT_fembed_bitcode_EQ)) {
+Opts.EmbedBitcode = llvm::StringSwitch(A->getValue())
+.Case("all", 1)
+.Case("bitcode", 1)
+.Case("marker", 1)
+.Default(0);
+  }
+
   return Success;
 }
 
@@ -449,6 +462,16 @@
 Str.get()->InitSections(Opts.NoExecStack);
   }
 
+  // When -fembed-bitcode is passed to clang_as, a 1-byte marker
+  // is emitted in __LLVM,__asm section if the object file is MachO format.
+  if (Opts.EmbedBitcode && Ctx.getObjectFileInfo()->getObjectFileType() ==
+   MCObjectFileInfo::IsMachO) {
+MCSection *AsmLabel = Ctx.getMachOSection(
+"__LLVM", "__asm", MachO::S_REGULAR, 4, SectionKind::getReadOnly());
+Str.get()->SwitchSection(AsmLabel);
+Str.get()->EmitZeros(1);
+  }
+
   // Assembly to object compilation should leverage assembly info.
   Str->setUseAssemblerInfoForParsing(true);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

See comments inline.




Comment at: include/clang/Driver/DarwinSDKInfo.h:1
+//===--- DarwinSDKInfo.h - SDK Information parser for darwin *- C++ 
-*-===//
+//

Can this just be in Toolchains/Darwin.h?



Comment at: include/clang/Driver/DarwinSDKInfo.h:36
+/// SDK has no SDKSettings.json, or a valid \c DarwinSDKInfo otherwise.
+Expected> parseDarwinSDKInfo(llvm::vfs::FileSystem 
&VFS,
+ StringRef SDKRootPath);

Isn't parseSDKSettings enough? And it can just return Optional?



Comment at: lib/Driver/ToolChains/Darwin.cpp:2053
+return None;
+  }
+  return *SDKInfoOrErr;

We also has this InferredFromSDK when we infer deployment target, which can be 
used as a fallback method.

The result of parsing JSON should be available to InferredFromSDK in deployment 
target setting.

Bonus point is to set "-sdk_version" flag passing to ld64.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55673/new/

https://reviews.llvm.org/D55673



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

[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

Other than a small design choice commented inline, LGTM.




Comment at: include/clang/Driver/DarwinSDKInfo.h:36
+/// SDK has no SDKSettings.json, or a valid \c DarwinSDKInfo otherwise.
+Expected> parseDarwinSDKInfo(llvm::vfs::FileSystem 
&VFS,
+ StringRef SDKRootPath);

arphaman wrote:
> steven_wu wrote:
> > Isn't parseSDKSettings enough? And it can just return 
> > Optional?
> We will support other fields besides `VersionTuple` in the SDKSettings, so 
> that's why we have a structure.
I feel like for this usage, it is better to return Expected with 
all the fields being Optional?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55673/new/

https://reviews.llvm.org/D55673



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

[PATCH] D55673: [darwin] parse the SDK settings from SDKSettings.json if it exists and pass in the -target-sdk-version to the compiler and backend

[Steven Wu via Phabricator via cfe-commits]

steven_wu added inline comments.



Comment at: include/clang/Driver/DarwinSDKInfo.h:36
+/// SDK has no SDKSettings.json, or a valid \c DarwinSDKInfo otherwise.
+Expected> parseDarwinSDKInfo(llvm::vfs::FileSystem 
&VFS,
+ StringRef SDKRootPath);

arphaman wrote:
> steven_wu wrote:
> > arphaman wrote:
> > > steven_wu wrote:
> > > > Isn't parseSDKSettings enough? And it can just return 
> > > > Optional?
> > > We will support other fields besides `VersionTuple` in the SDKSettings, 
> > > so that's why we have a structure.
> > I feel like for this usage, it is better to return Expected 
> > with all the fields being Optional?
> Hmm, we want to assume that version exists for future uses. I feel like the 
> current type captures the intention better.
I think it is fine for current use. We can always change in the future.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55673/new/

https://reviews.llvm.org/D55673



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

[PATCH] D67559: [Sema] Split of versions of -Wimplicit-{float,int}-conversion for Objective-C BOOL

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

The diagnostics message looks good to me.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67559/new/

https://reviews.llvm.org/D67559



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

[PATCH] D45699: [Availability] Improve availability to consider functions run at load time

[Steven Wu via Phabricator via cfe-commits]

steven_wu created this revision.
steven_wu added a reviewer: arphaman.

There are some functions/methods that run when the application launches
or the library loads. Those functions will run reguardless the OS
version as long as it satifies the minimum deployment target. Annotate
them with availability attributes doesn't really make sense because they
are essentially available on all targets since minimum deployment
target.

rdar://problem/36093384


Repository:
  rC Clang

https://reviews.llvm.org/D45699

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaDeclObjC.cpp
  test/SemaObjC/unguarded-availability.m

Index: test/SemaObjC/unguarded-availability.m
===
--- test/SemaObjC/unguarded-availability.m
+++ test/SemaObjC/unguarded-availability.m
@@ -7,7 +7,7 @@
 
 typedef int AVAILABLE_10_12 new_int; // expected-note + {{marked partial here}}
 
-int func_10_11() AVAILABLE_10_11; // expected-note 4 {{'func_10_11' has been explicitly marked partial here}}
+int func_10_11() AVAILABLE_10_11; // expected-note 8 {{'func_10_11' has been explicitly marked partial here}}
 
 #ifdef OBJCPP
 // expected-note@+2 6 {{marked partial here}}
@@ -311,3 +311,41 @@
   (void)AK_Cat; // no warning
   (void)AK_CyborgCat; // expected-warning{{'AK_CyborgCat' is only available on macOS 10.12 or newer}} expected-note {{@available}}
 }
+
+
+// test static initializers has the same availability as the deployment target and it cannot be overwritten.
+@interface HasStaticInitializer : BaseClass
++ (void)load AVAILABLE_10_11; // expected-warning{{ignoring availability attribute on '+load' method}}
+@end
+
+@implementation HasStaticInitializer
++ (void)load {
+  func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+}
+@end
+
+// test availability from interface is ignored when checking the unguarded availability in +load method.
+AVAILABLE_10_11
+@interface HasStaticInitializer1 : BaseClass
++ (void)load;
+@end
+
+@implementation HasStaticInitializer1
++ (void)load {
+  func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+}
+@end
+
+__attribute__((constructor))
+void is_constructor();
+
+AVAILABLE_10_11 // expected-warning{{ignoring availability attribute with constructor attribute}}
+void is_constructor() {
+  func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+}
+
+AVAILABLE_10_11 // expected-warning{{ignoring availability attribute with desctructor attribute}}
+__attribute__((destructor))
+void is_destructor() {
+  func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+}
Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -4734,6 +4734,20 @@
   Context.getTargetInfo().getTriple().getArch() == llvm::Triple::x86)
 checkObjCMethodX86VectorTypes(*this, ObjCMethod);
 
+  // + load method cannot have availability attributes. It has to be the same
+  // as deployment target.
+  for (const auto& attr: ObjCMethod->attrs()) {
+if (!isa(attr))
+  continue;
+
+if (ObjCMethod->isClassMethod() &&
+ObjCMethod->getNameAsString() == "load") {
+  Diag(attr->getLocation(), diag::warn_availability_on_static_initializer)
+  << 0;
+  ObjCMethod->dropAttr();
+}
+  }
+
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod;
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6823,6 +6823,11 @@
   return false;
 
 // An implementation implicitly has the availability of the interface.
+// Unless it is "+load" method.
+if (const auto *MethodD = dyn_cast(Ctx))
+  if (MethodD->isClassMethod() && MethodD->getNameAsString() == "load")
+return true;
+
 if (const auto *CatOrImpl = dyn_cast(Ctx)) {
   if (const ObjCInterfaceDecl *Interface = CatOrImpl->getClassInterface())
 if (CheckContext(Interface))
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -9131,6 +9131,25 @@
 AddToScope = false;
   }
 
+  // Diagnose availability attributes. Availability cannot be used on functions
+  // that are run during load/unload.
+  for (const auto& attr: NewFD->attrs()) {
+if (!isa(attr))
+  continue;
+
+if (NewFD->hasAttr()) {
+

[PATCH] D45699: [Availability] Improve availability to consider functions run at load time

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

Thanks for reviewing Erik!




Comment at: lib/Sema/SemaDecl.cpp:9134-9151
+  // Diagnose availability attributes. Availability cannot be used on functions
+  // that are run during load/unload.
+  for (const auto& attr: NewFD->attrs()) {
+if (!isa(attr))
+  continue;
+
+if (NewFD->hasAttr()) {

erik.pilkington wrote:
> Shouldn't this be in ProcessDeclAttributeList in SemaDeclAttr.cpp?
This has to happen after mergeDeclAttributes, otherwise, you won't catch the 
case which the availability attributes and constructor attributes are on 
different decl that gets merged. It can't be in mergeDeclAttributes as well, 
then it won't catch the case they are on a single decl.



Comment at: lib/Sema/SemaDecl.cpp:9136
+  // that are run during load/unload.
+  for (const auto& attr: NewFD->attrs()) {
+if (!isa(attr))

erik.pilkington wrote:
> Can't this just be: `if (NewFD->hasAttr())`?
The location of the diagnostics need to be on attributes and that is why it is 
a for loop.


Repository:
  rC Clang

https://reviews.llvm.org/D45699



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

[PATCH] D45699: [Availability] Improve availability to consider functions run at load time

[Steven Wu via Phabricator via cfe-commits]

steven_wu updated this revision to Diff 142713.
steven_wu added a comment.

Address review feedback. Fix typos and comments.


Repository:
  rC Clang

https://reviews.llvm.org/D45699

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaDeclObjC.cpp
  test/SemaObjC/unguarded-availability.m

Index: test/SemaObjC/unguarded-availability.m
===
--- test/SemaObjC/unguarded-availability.m
+++ test/SemaObjC/unguarded-availability.m
@@ -7,7 +7,7 @@
 
 typedef int AVAILABLE_10_12 new_int; // expected-note + {{marked partial here}}
 
-int func_10_11() AVAILABLE_10_11; // expected-note 4 {{'func_10_11' has been explicitly marked partial here}}
+int func_10_11() AVAILABLE_10_11; // expected-note 8 {{'func_10_11' has been explicitly marked partial here}}
 
 #ifdef OBJCPP
 // expected-note@+2 6 {{marked partial here}}
@@ -311,3 +311,45 @@
   (void)AK_Cat; // no warning
   (void)AK_CyborgCat; // expected-warning{{'AK_CyborgCat' is only available on macOS 10.12 or newer}} expected-note {{@available}}
 }
+
+
+// test static initializers has the same availability as the deployment target and it cannot be overwritten.
+@interface HasStaticInitializer : BaseClass
++ (void)load AVAILABLE_10_11; // expected-warning{{ignoring availability attribute on '+load' method}}
+@end
+
+@implementation HasStaticInitializer
++ (void)load {
+  func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+}
+@end
+
+// test availability from interface is ignored when checking the unguarded availability in +load method.
+AVAILABLE_10_11
+@interface HasStaticInitializer1 : BaseClass
++ (void)load;
++ (void)load: (int)x; // no warning.
+@end
+
+@implementation HasStaticInitializer1
++ (void)load {
+  func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+}
++ (void)load: (int)x {
+  func_10_11(); // no warning.
+}
+@end
+
+__attribute__((constructor))
+void is_constructor();
+
+AVAILABLE_10_11 // expected-warning{{ignoring availability attribute with constructor attribute}}
+void is_constructor() {
+  func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+}
+
+AVAILABLE_10_11 // expected-warning{{ignoring availability attribute with destructor attribute}}
+__attribute__((destructor))
+void is_destructor() {
+  func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+}
Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -4734,6 +4734,20 @@
   Context.getTargetInfo().getTriple().getArch() == llvm::Triple::x86)
 checkObjCMethodX86VectorTypes(*this, ObjCMethod);
 
+  // + load method cannot have availability attributes. It get called on
+  // startup, so it has to have the availability of the deployment target.
+  for (const auto& attr: ObjCMethod->attrs()) {
+if (!isa(attr))
+  continue;
+
+if (ObjCMethod->isClassMethod() &&
+ObjCMethod->getSelector().getAsString() == "load") {
+  Diag(attr->getLocation(), diag::warn_availability_on_static_initializer)
+  << 0;
+  ObjCMethod->dropAttr();
+}
+  }
+
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod;
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6823,6 +6823,12 @@
   return false;
 
 // An implementation implicitly has the availability of the interface.
+// Unless it is "+load" method.
+if (const auto *MethodD = dyn_cast(Ctx))
+  if (MethodD->isClassMethod() &&
+  MethodD->getSelector().getAsString() == "load")
+return true;
+
 if (const auto *CatOrImpl = dyn_cast(Ctx)) {
   if (const ObjCInterfaceDecl *Interface = CatOrImpl->getClassInterface())
 if (CheckContext(Interface))
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -9131,6 +9131,25 @@
 AddToScope = false;
   }
 
+  // Diagnose availability attributes. Availability cannot be used on functions
+  // that are run during load/unload.
+  for (const auto& attr: NewFD->attrs()) {
+if (!isa(attr))
+  continue;
+
+if (NewFD->hasAttr()) {
+  Diag(attr->getLocation(), diag::warn_availability_on_static_initializer)
+  << 1;
+  NewFD->dropAttr();
+}
+if (NewFD->hasAttr()) {
+   

[PATCH] D45699: [Availability] Improve availability to consider functions run at load time

[Steven Wu via Phabricator via cfe-commits]

steven_wu updated this revision to Diff 142717.
steven_wu added a comment.

Address review feedback


Repository:
  rC Clang

https://reviews.llvm.org/D45699

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaDeclObjC.cpp
  test/SemaObjC/unguarded-availability.m

Index: test/SemaObjC/unguarded-availability.m
===
--- test/SemaObjC/unguarded-availability.m
+++ test/SemaObjC/unguarded-availability.m
@@ -7,7 +7,7 @@
 
 typedef int AVAILABLE_10_12 new_int; // expected-note + {{marked partial here}}
 
-int func_10_11() AVAILABLE_10_11; // expected-note 4 {{'func_10_11' has been explicitly marked partial here}}
+int func_10_11() AVAILABLE_10_11; // expected-note 8 {{'func_10_11' has been explicitly marked partial here}}
 
 #ifdef OBJCPP
 // expected-note@+2 6 {{marked partial here}}
@@ -311,3 +311,45 @@
   (void)AK_Cat; // no warning
   (void)AK_CyborgCat; // expected-warning{{'AK_CyborgCat' is only available on macOS 10.12 or newer}} expected-note {{@available}}
 }
+
+
+// test static initializers has the same availability as the deployment target and it cannot be overwritten.
+@interface HasStaticInitializer : BaseClass
++ (void)load AVAILABLE_10_11; // expected-warning{{ignoring availability attribute on '+load' method}}
+@end
+
+@implementation HasStaticInitializer
++ (void)load {
+  func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+}
+@end
+
+// test availability from interface is ignored when checking the unguarded availability in +load method.
+AVAILABLE_10_11
+@interface HasStaticInitializer1 : BaseClass
++ (void)load;
++ (void)load: (int)x; // no warning.
+@end
+
+@implementation HasStaticInitializer1
++ (void)load {
+  func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+}
++ (void)load: (int)x {
+  func_10_11(); // no warning.
+}
+@end
+
+__attribute__((constructor))
+void is_constructor();
+
+AVAILABLE_10_11 // expected-warning{{ignoring availability attribute with constructor attribute}}
+void is_constructor() {
+  func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+}
+
+AVAILABLE_10_11 // expected-warning{{ignoring availability attribute with destructor attribute}}
+__attribute__((destructor))
+void is_destructor() {
+  func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+}
Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -4734,6 +4734,17 @@
   Context.getTargetInfo().getTriple().getArch() == llvm::Triple::x86)
 checkObjCMethodX86VectorTypes(*this, ObjCMethod);
 
+  // + load method cannot have availability attributes. It get called on
+  // startup, so it has to have the availability of the deployment target.
+  if (const auto *attr = ObjCMethod->getAttr()) {
+if (ObjCMethod->isClassMethod() &&
+ObjCMethod->getSelector().getAsString() == "load") {
+  Diag(attr->getLocation(), diag::warn_availability_on_static_initializer)
+  << 0;
+  ObjCMethod->dropAttr();
+}
+  }
+
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod;
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6823,6 +6823,12 @@
   return false;
 
 // An implementation implicitly has the availability of the interface.
+// Unless it is "+load" method.
+if (const auto *MethodD = dyn_cast(Ctx))
+  if (MethodD->isClassMethod() &&
+  MethodD->getSelector().getAsString() == "load")
+return true;
+
 if (const auto *CatOrImpl = dyn_cast(Ctx)) {
   if (const ObjCInterfaceDecl *Interface = CatOrImpl->getClassInterface())
 if (CheckContext(Interface))
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -9131,6 +9131,21 @@
 AddToScope = false;
   }
 
+  // Diagnose availability attributes. Availability cannot be used on functions
+  // that are run during load/unload.
+  if (const auto *attr = NewFD->getAttr()) {
+if (NewFD->hasAttr()) {
+  Diag(attr->getLocation(), diag::warn_availability_on_static_initializer)
+  << 1;
+  NewFD->dropAttr();
+}
+if (NewFD->hasAttr()) {
+  Diag(attr->getLocation(), diag::warn_availability_on_static_initializer)
+  << 2;
+  N

[PATCH] D45699: [Availability] Improve availability to consider functions run at load time

[Steven Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC330166: [Availability] Improve availability to consider 
functions run at load time (authored by steven_wu, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D45699?vs=142717&id=142718#toc

Repository:
  rC Clang

https://reviews.llvm.org/D45699

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaDeclObjC.cpp
  test/SemaObjC/unguarded-availability.m

Index: lib/Sema/SemaDeclObjC.cpp
===
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -4734,6 +4734,17 @@
   Context.getTargetInfo().getTriple().getArch() == llvm::Triple::x86)
 checkObjCMethodX86VectorTypes(*this, ObjCMethod);
 
+  // + load method cannot have availability attributes. It get called on
+  // startup, so it has to have the availability of the deployment target.
+  if (const auto *attr = ObjCMethod->getAttr()) {
+if (ObjCMethod->isClassMethod() &&
+ObjCMethod->getSelector().getAsString() == "load") {
+  Diag(attr->getLocation(), diag::warn_availability_on_static_initializer)
+  << 0;
+  ObjCMethod->dropAttr();
+}
+  }
+
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod;
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6823,6 +6823,12 @@
   return false;
 
 // An implementation implicitly has the availability of the interface.
+// Unless it is "+load" method.
+if (const auto *MethodD = dyn_cast(Ctx))
+  if (MethodD->isClassMethod() &&
+  MethodD->getSelector().getAsString() == "load")
+return true;
+
 if (const auto *CatOrImpl = dyn_cast(Ctx)) {
   if (const ObjCInterfaceDecl *Interface = CatOrImpl->getClassInterface())
 if (CheckContext(Interface))
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -9139,6 +9139,21 @@
 AddToScope = false;
   }
 
+  // Diagnose availability attributes. Availability cannot be used on functions
+  // that are run during load/unload.
+  if (const auto *attr = NewFD->getAttr()) {
+if (NewFD->hasAttr()) {
+  Diag(attr->getLocation(), diag::warn_availability_on_static_initializer)
+  << 1;
+  NewFD->dropAttr();
+}
+if (NewFD->hasAttr()) {
+  Diag(attr->getLocation(), diag::warn_availability_on_static_initializer)
+  << 2;
+  NewFD->dropAttr();
+}
+  }
+
   return NewFD;
 }
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2912,6 +2912,10 @@
   "%select{the protocol method it implements|its overridden method}1 is "
   "available">,
   InGroup;
+def warn_availability_on_static_initializer : Warning<
+  "ignoring availability attribute %select{on '+load' method|"
+  "with constructor attribute|with destructor attribute}0">,
+  InGroup;
 def note_overridden_method : Note<
   "overridden method is here">;
 def note_protocol_method : Note<
Index: test/SemaObjC/unguarded-availability.m
===
--- test/SemaObjC/unguarded-availability.m
+++ test/SemaObjC/unguarded-availability.m
@@ -7,7 +7,7 @@
 
 typedef int AVAILABLE_10_12 new_int; // expected-note + {{marked partial here}}
 
-int func_10_11() AVAILABLE_10_11; // expected-note 4 {{'func_10_11' has been explicitly marked partial here}}
+int func_10_11() AVAILABLE_10_11; // expected-note 8 {{'func_10_11' has been explicitly marked partial here}}
 
 #ifdef OBJCPP
 // expected-note@+2 6 {{marked partial here}}
@@ -311,3 +311,45 @@
   (void)AK_Cat; // no warning
   (void)AK_CyborgCat; // expected-warning{{'AK_CyborgCat' is only available on macOS 10.12 or newer}} expected-note {{@available}}
 }
+
+
+// test static initializers has the same availability as the deployment target and it cannot be overwritten.
+@interface HasStaticInitializer : BaseClass
++ (void)load AVAILABLE_10_11; // expected-warning{{ignoring availability attribute on '+load' method}}
+@end
+
+@implementation HasStaticInitializer
++ (void)load {
+  func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
+}
+@end
+
+// test availability from interface is ignored when checking the unguarded availability in +load method.
+AVAILABLE_10_11
+@interface HasStaticInitializer1 : BaseClass
++ (void)load;
++ (void)load: (int)x; // no warning.
+@end
+
+@implementation HasStaticInitializer1
++ (void)load {
+  func_10_11(); // expected-warning{{'func_10_11' is only 

[PATCH] D44670: [CXX] Templates specialization visibility can be wrong

[Steven Wu via Phabricator via cfe-commits]

steven_wu updated this revision to Diff 142961.
steven_wu added a comment.

Address review feedback


Repository:
  rC Clang

https://reviews.llvm.org/D44670

Files:
  lib/AST/Decl.cpp
  test/CodeGenCXX/visibility-pr36810.cpp


Index: test/CodeGenCXX/visibility-pr36810.cpp
===
--- /dev/null
+++ test/CodeGenCXX/visibility-pr36810.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx -std=c++11 -fvisibility hidden 
-emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx -DUNDEF_G -std=c++11 
-fvisibility hidden -emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s
+
+namespace std {
+template 
+class __attribute__((__type_visibility__("default"))) shared_ptr {
+  template  friend class shared_ptr;
+};
+}
+struct dict;
+#ifndef UNDEF_G
+std::shared_ptr g;
+#endif
+class __attribute__((visibility("default"))) Bar;
+template >
+class __attribute__((visibility("default"))) i {
+  std::shared_ptr foo() const;
+};
+
+// CHECK: define void @_ZNK1iISt10shared_ptrI3BarEE3fooEv
+template <> std::shared_ptr i<>::foo() const {
+  return std::shared_ptr();
+}
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -1092,9 +1092,18 @@
   // If there wasn't explicit visibility there, and this is a
   // specialization of a class template, check for visibility
   // on the pattern.
-  if (const auto *spec = dyn_cast(ND))
-return getVisibilityOf(spec->getSpecializedTemplate()->getTemplatedDecl(),
-   kind);
+  if (const auto *spec = dyn_cast(ND)) {
+// Walk all the template decl till this point to see if there are
+// explicit visibility attributes.
+const auto *TD = spec->getSpecializedTemplate()->getTemplatedDecl();
+while (TD != nullptr) {
+  auto Vis = getVisibilityOf(TD, kind);
+  if (Vis != None)
+return Vis;
+  TD = TD->getPreviousDecl();
+}
+return None;
+  }
 
   // Use the most recent declaration.
   if (!IsMostRecent && !isa(ND)) {


Index: test/CodeGenCXX/visibility-pr36810.cpp
===
--- /dev/null
+++ test/CodeGenCXX/visibility-pr36810.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx -std=c++11 -fvisibility hidden -emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx -DUNDEF_G -std=c++11 -fvisibility hidden -emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s
+
+namespace std {
+template 
+class __attribute__((__type_visibility__("default"))) shared_ptr {
+  template  friend class shared_ptr;
+};
+}
+struct dict;
+#ifndef UNDEF_G
+std::shared_ptr g;
+#endif
+class __attribute__((visibility("default"))) Bar;
+template >
+class __attribute__((visibility("default"))) i {
+  std::shared_ptr foo() const;
+};
+
+// CHECK: define void @_ZNK1iISt10shared_ptrI3BarEE3fooEv
+template <> std::shared_ptr i<>::foo() const {
+  return std::shared_ptr();
+}
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -1092,9 +1092,18 @@
   // If there wasn't explicit visibility there, and this is a
   // specialization of a class template, check for visibility
   // on the pattern.
-  if (const auto *spec = dyn_cast(ND))
-return getVisibilityOf(spec->getSpecializedTemplate()->getTemplatedDecl(),
-   kind);
+  if (const auto *spec = dyn_cast(ND)) {
+// Walk all the template decl till this point to see if there are
+// explicit visibility attributes.
+const auto *TD = spec->getSpecializedTemplate()->getTemplatedDecl();
+while (TD != nullptr) {
+  auto Vis = getVisibilityOf(TD, kind);
+  if (Vis != None)
+return Vis;
+  TD = TD->getPreviousDecl();
+}
+return None;
+  }
 
   // Use the most recent declaration.
   if (!IsMostRecent && !isa(ND)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44670: [CXX] Templates specialization visibility can be wrong

[Steven Wu via Phabricator via cfe-commits]

steven_wu added inline comments.



Comment at: lib/AST/Decl.cpp:1078
+for (const auto *RD :
+ spec->getSpecializedTemplate()->getTemplatedDecl()->redecls()) {
+  auto Vis = getVisibilityOf(RD, kind);

doug.gregor wrote:
> Do we want to look at *all* redeclarations, or only those declarations that 
> precede the declaration that we found? The latter seems more correct, but 
> IIRC visibility has often been able to "look forward" to declarations that 
> come later in the translation unit.
I agree the latter is more correct. Update the patch. 


Repository:
  rC Clang

https://reviews.llvm.org/D44670



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

[PATCH] D44670: [CXX] Templates specialization visibility can be wrong

[Steven Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL330338: [CXX] Templates specialization visibility can be 
wrong (authored by steven_wu, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D44670

Files:
  cfe/trunk/lib/AST/Decl.cpp
  cfe/trunk/test/CodeGenCXX/visibility-pr36810.cpp


Index: cfe/trunk/test/CodeGenCXX/visibility-pr36810.cpp
===
--- cfe/trunk/test/CodeGenCXX/visibility-pr36810.cpp
+++ cfe/trunk/test/CodeGenCXX/visibility-pr36810.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx -std=c++11 -fvisibility hidden 
-emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx -DUNDEF_G -std=c++11 
-fvisibility hidden -emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s
+
+namespace std {
+template 
+class __attribute__((__type_visibility__("default"))) shared_ptr {
+  template  friend class shared_ptr;
+};
+}
+struct dict;
+#ifndef UNDEF_G
+std::shared_ptr g;
+#endif
+class __attribute__((visibility("default"))) Bar;
+template >
+class __attribute__((visibility("default"))) i {
+  std::shared_ptr foo() const;
+};
+
+// CHECK: define void @_ZNK1iISt10shared_ptrI3BarEE3fooEv
+template <> std::shared_ptr i<>::foo() const {
+  return std::shared_ptr();
+}
Index: cfe/trunk/lib/AST/Decl.cpp
===
--- cfe/trunk/lib/AST/Decl.cpp
+++ cfe/trunk/lib/AST/Decl.cpp
@@ -1092,9 +1092,18 @@
   // If there wasn't explicit visibility there, and this is a
   // specialization of a class template, check for visibility
   // on the pattern.
-  if (const auto *spec = dyn_cast(ND))
-return getVisibilityOf(spec->getSpecializedTemplate()->getTemplatedDecl(),
-   kind);
+  if (const auto *spec = dyn_cast(ND)) {
+// Walk all the template decl till this point to see if there are
+// explicit visibility attributes.
+const auto *TD = spec->getSpecializedTemplate()->getTemplatedDecl();
+while (TD != nullptr) {
+  auto Vis = getVisibilityOf(TD, kind);
+  if (Vis != None)
+return Vis;
+  TD = TD->getPreviousDecl();
+}
+return None;
+  }
 
   // Use the most recent declaration.
   if (!IsMostRecent && !isa(ND)) {


Index: cfe/trunk/test/CodeGenCXX/visibility-pr36810.cpp
===
--- cfe/trunk/test/CodeGenCXX/visibility-pr36810.cpp
+++ cfe/trunk/test/CodeGenCXX/visibility-pr36810.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx -std=c++11 -fvisibility hidden -emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx -DUNDEF_G -std=c++11 -fvisibility hidden -emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s
+
+namespace std {
+template 
+class __attribute__((__type_visibility__("default"))) shared_ptr {
+  template  friend class shared_ptr;
+};
+}
+struct dict;
+#ifndef UNDEF_G
+std::shared_ptr g;
+#endif
+class __attribute__((visibility("default"))) Bar;
+template >
+class __attribute__((visibility("default"))) i {
+  std::shared_ptr foo() const;
+};
+
+// CHECK: define void @_ZNK1iISt10shared_ptrI3BarEE3fooEv
+template <> std::shared_ptr i<>::foo() const {
+  return std::shared_ptr();
+}
Index: cfe/trunk/lib/AST/Decl.cpp
===
--- cfe/trunk/lib/AST/Decl.cpp
+++ cfe/trunk/lib/AST/Decl.cpp
@@ -1092,9 +1092,18 @@
   // If there wasn't explicit visibility there, and this is a
   // specialization of a class template, check for visibility
   // on the pattern.
-  if (const auto *spec = dyn_cast(ND))
-return getVisibilityOf(spec->getSpecializedTemplate()->getTemplatedDecl(),
-   kind);
+  if (const auto *spec = dyn_cast(ND)) {
+// Walk all the template decl till this point to see if there are
+// explicit visibility attributes.
+const auto *TD = spec->getSpecializedTemplate()->getTemplatedDecl();
+while (TD != nullptr) {
+  auto Vis = getVisibilityOf(TD, kind);
+  if (Vis != None)
+return Vis;
+  TD = TD->getPreviousDecl();
+}
+return None;
+  }
 
   // Use the most recent declaration.
   if (!IsMostRecent && !isa(ND)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D44670: [CXX] Templates specialization visibility can be wrong

[Steven Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC330338: [CXX] Templates specialization visibility can be 
wrong (authored by steven_wu, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D44670?vs=142961&id=143103#toc

Repository:
  rC Clang

https://reviews.llvm.org/D44670

Files:
  lib/AST/Decl.cpp
  test/CodeGenCXX/visibility-pr36810.cpp


Index: test/CodeGenCXX/visibility-pr36810.cpp
===
--- test/CodeGenCXX/visibility-pr36810.cpp
+++ test/CodeGenCXX/visibility-pr36810.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx -std=c++11 -fvisibility hidden 
-emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx -DUNDEF_G -std=c++11 
-fvisibility hidden -emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s
+
+namespace std {
+template 
+class __attribute__((__type_visibility__("default"))) shared_ptr {
+  template  friend class shared_ptr;
+};
+}
+struct dict;
+#ifndef UNDEF_G
+std::shared_ptr g;
+#endif
+class __attribute__((visibility("default"))) Bar;
+template >
+class __attribute__((visibility("default"))) i {
+  std::shared_ptr foo() const;
+};
+
+// CHECK: define void @_ZNK1iISt10shared_ptrI3BarEE3fooEv
+template <> std::shared_ptr i<>::foo() const {
+  return std::shared_ptr();
+}
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -1092,9 +1092,18 @@
   // If there wasn't explicit visibility there, and this is a
   // specialization of a class template, check for visibility
   // on the pattern.
-  if (const auto *spec = dyn_cast(ND))
-return getVisibilityOf(spec->getSpecializedTemplate()->getTemplatedDecl(),
-   kind);
+  if (const auto *spec = dyn_cast(ND)) {
+// Walk all the template decl till this point to see if there are
+// explicit visibility attributes.
+const auto *TD = spec->getSpecializedTemplate()->getTemplatedDecl();
+while (TD != nullptr) {
+  auto Vis = getVisibilityOf(TD, kind);
+  if (Vis != None)
+return Vis;
+  TD = TD->getPreviousDecl();
+}
+return None;
+  }
 
   // Use the most recent declaration.
   if (!IsMostRecent && !isa(ND)) {


Index: test/CodeGenCXX/visibility-pr36810.cpp
===
--- test/CodeGenCXX/visibility-pr36810.cpp
+++ test/CodeGenCXX/visibility-pr36810.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx -std=c++11 -fvisibility hidden -emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx -DUNDEF_G -std=c++11 -fvisibility hidden -emit-llvm -o - %s -O2 -disable-llvm-passes | FileCheck %s
+
+namespace std {
+template 
+class __attribute__((__type_visibility__("default"))) shared_ptr {
+  template  friend class shared_ptr;
+};
+}
+struct dict;
+#ifndef UNDEF_G
+std::shared_ptr g;
+#endif
+class __attribute__((visibility("default"))) Bar;
+template >
+class __attribute__((visibility("default"))) i {
+  std::shared_ptr foo() const;
+};
+
+// CHECK: define void @_ZNK1iISt10shared_ptrI3BarEE3fooEv
+template <> std::shared_ptr i<>::foo() const {
+  return std::shared_ptr();
+}
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -1092,9 +1092,18 @@
   // If there wasn't explicit visibility there, and this is a
   // specialization of a class template, check for visibility
   // on the pattern.
-  if (const auto *spec = dyn_cast(ND))
-return getVisibilityOf(spec->getSpecializedTemplate()->getTemplatedDecl(),
-   kind);
+  if (const auto *spec = dyn_cast(ND)) {
+// Walk all the template decl till this point to see if there are
+// explicit visibility attributes.
+const auto *TD = spec->getSpecializedTemplate()->getTemplatedDecl();
+while (TD != nullptr) {
+  auto Vis = getVisibilityOf(TD, kind);
+  if (Vis != None)
+return Vis;
+  TD = TD->getPreviousDecl();
+}
+return None;
+  }
 
   // Use the most recent declaration.
   if (!IsMostRecent && !isa(ND)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48849: [Driver][Darwin] Use Host Triple to infer target os version

[Steven Wu via Phabricator via cfe-commits]

steven_wu created this revision.
steven_wu added reviewers: arphaman, dexonsmith.

When clang required to infer target os version from --target option and
the os version is not specified in targets, check the host triple. If the
host and target are both macOS, use host triple to infer target os
version.

rdar://problem/41651999


Repository:
  rC Clang

https://reviews.llvm.org/D48849

Files:
  lib/Driver/ToolChains/Darwin.cpp
  test/Driver/clang-g-opts.c


Index: test/Driver/clang-g-opts.c
===
--- test/Driver/clang-g-opts.c
+++ test/Driver/clang-g-opts.c
@@ -3,7 +3,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
 
 // Assert that the toolchains which should default to a lower Dwarf version do 
so.
-// RUN: %clang -### -S %s -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
 // RUN: %clang -### -S %s -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
@@ -21,7 +21,7 @@
 //
 // RUN: %clang -### -S %s -g0 -g -target x86_64-linux-gnu 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
-// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-STANDALONE %s
 // RUN: %clang -### -S %s -g0 -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -1512,9 +1512,22 @@
   else if (MachOArchName == "armv7k")
 OSTy = llvm::Triple::WatchOS;
   else if (MachOArchName != "armv6m" && MachOArchName != "armv7m" &&
-   MachOArchName != "armv7em")
+   MachOArchName != "armv7em") {
+// Default to macOS for other architectures.
 OSTy = llvm::Triple::MacOSX;
 
+// If there is no version specified on triple, and both host and target are
+// macos, use the host triple to infer OS version.
+llvm::Triple SystemTriple(llvm::sys::getProcessTriple());
+if (SystemTriple.isMacOSX()) {
+  unsigned Major, Minor, Micro;
+  Triple.getOSVersion(Major, Minor, Micro);
+  if (Major == 0)
+return DarwinPlatform::createFromArch(
+OSTy, getOSVersion(OSTy, SystemTriple, TheDriver));
+}
+  }
+
   if (OSTy == llvm::Triple::UnknownOS)
 return None;
   return DarwinPlatform::createFromArch(OSTy,


Index: test/Driver/clang-g-opts.c
===
--- test/Driver/clang-g-opts.c
+++ test/Driver/clang-g-opts.c
@@ -3,7 +3,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
 
 // Assert that the toolchains which should default to a lower Dwarf version do so.
-// RUN: %clang -### -S %s -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
 // RUN: %clang -### -S %s -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
@@ -21,7 +21,7 @@
 //
 // RUN: %clang -### -S %s -g0 -g -target x86_64-linux-gnu 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
-// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-STANDALONE %s
 // RUN: %clang -### -S %s -g0 -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -1512,9 +1512,22 @@
   else if (MachOArchName == "armv7k")
 OSTy = llvm::Triple::WatchOS;
   else if (MachOArchName != "armv6m" && MachOArchName != "armv7m" &&
-   MachOArchName != "armv7em")
+   MachOArchName != "armv7em") {
+// Default to macOS for other architectures.
 OSTy = llvm::Triple::MacOSX;
 
+// If there is no version specified on triple, and both host and target are
+// macos, use the host triple to infer OS version.
+llvm::Triple SystemTriple(llvm::sys::getProcessTriple());
+if (SystemTriple.isMacOSX()) {
+  unsigned Major, Minor, Micro;
+  Triple.getOSVersion(Major, Minor, Micro);
+  if (Major == 0)
+return DarwinPlatform::createFromArch(
+OSTy, getOSVersion(OSTy, SystemTriple, TheDriver));
+}
+  }
+
   if (OSTy == llvm::Triple::UnknownOS)
 return None;
   return DarwinPlatform::createFromArch(OSTy,
__

[PATCH] D48849: [Driver][Darwin] Use Host Triple to infer target os version

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

Unfortunately, I wasn't able to write a test for this because the host triple 
in the configuration can either be x86_64-apple-darwin* or 
x86_64-apple-macosx*, but the one used passed by driver is always macosx one. I 
can't reliably compare those two.


Repository:
  rC Clang

https://reviews.llvm.org/D48849



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

[PATCH] D48849: [Driver][Darwin] Use Host Triple to infer target os version

[Steven Wu via Phabricator via cfe-commits]

steven_wu updated this revision to Diff 153814.
steven_wu added a comment.

Update patch. Use a better API.


Repository:
  rC Clang

https://reviews.llvm.org/D48849

Files:
  lib/Driver/ToolChains/Darwin.cpp
  test/Driver/clang-g-opts.c


Index: test/Driver/clang-g-opts.c
===
--- test/Driver/clang-g-opts.c
+++ test/Driver/clang-g-opts.c
@@ -3,7 +3,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
 
 // Assert that the toolchains which should default to a lower Dwarf version do 
so.
-// RUN: %clang -### -S %s -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
 // RUN: %clang -### -S %s -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
@@ -21,7 +21,7 @@
 //
 // RUN: %clang -### -S %s -g0 -g -target x86_64-linux-gnu 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
-// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-STANDALONE %s
 // RUN: %clang -### -S %s -g0 -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -1512,9 +1512,20 @@
   else if (MachOArchName == "armv7k")
 OSTy = llvm::Triple::WatchOS;
   else if (MachOArchName != "armv6m" && MachOArchName != "armv7m" &&
-   MachOArchName != "armv7em")
+   MachOArchName != "armv7em") {
+// Default to macOS for other architectures.
 OSTy = llvm::Triple::MacOSX;
 
+// If there is no version specified on triple, and both host and target are
+// macos, use the host triple to infer OS version.
+llvm::Triple SystemTriple(llvm::sys::getProcessTriple());
+if (SystemTriple.isMacOSX()) {
+  if (!Triple.getOSMajorVersion())
+return DarwinPlatform::createFromArch(
+OSTy, getOSVersion(OSTy, SystemTriple, TheDriver));
+}
+  }
+
   if (OSTy == llvm::Triple::UnknownOS)
 return None;
   return DarwinPlatform::createFromArch(OSTy,


Index: test/Driver/clang-g-opts.c
===
--- test/Driver/clang-g-opts.c
+++ test/Driver/clang-g-opts.c
@@ -3,7 +3,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
 
 // Assert that the toolchains which should default to a lower Dwarf version do so.
-// RUN: %clang -### -S %s -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
 // RUN: %clang -### -S %s -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
@@ -21,7 +21,7 @@
 //
 // RUN: %clang -### -S %s -g0 -g -target x86_64-linux-gnu 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
-// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-STANDALONE %s
 // RUN: %clang -### -S %s -g0 -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -1512,9 +1512,20 @@
   else if (MachOArchName == "armv7k")
 OSTy = llvm::Triple::WatchOS;
   else if (MachOArchName != "armv6m" && MachOArchName != "armv7m" &&
-   MachOArchName != "armv7em")
+   MachOArchName != "armv7em") {
+// Default to macOS for other architectures.
 OSTy = llvm::Triple::MacOSX;
 
+// If there is no version specified on triple, and both host and target are
+// macos, use the host triple to infer OS version.
+llvm::Triple SystemTriple(llvm::sys::getProcessTriple());
+if (SystemTriple.isMacOSX()) {
+  if (!Triple.getOSMajorVersion())
+return DarwinPlatform::createFromArch(
+OSTy, getOSVersion(OSTy, SystemTriple, TheDriver));
+}
+  }
+
   if (OSTy == llvm::Triple::UnknownOS)
 return None;
   return DarwinPlatform::createFromArch(OSTy,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48849: [Driver][Darwin] Use Host Triple to infer target os version

[Steven Wu via Phabricator via cfe-commits]

steven_wu updated this revision to Diff 153816.
steven_wu added a comment.

Rebase the commit correctly


Repository:
  rC Clang

https://reviews.llvm.org/D48849

Files:
  lib/Driver/ToolChains/Darwin.cpp
  test/Driver/clang-g-opts.c


Index: test/Driver/clang-g-opts.c
===
--- test/Driver/clang-g-opts.c
+++ test/Driver/clang-g-opts.c
@@ -3,7 +3,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
 
 // Assert that the toolchains which should default to a lower Dwarf version do 
so.
-// RUN: %clang -### -S %s -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
 // RUN: %clang -### -S %s -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
@@ -21,7 +21,7 @@
 //
 // RUN: %clang -### -S %s -g0 -g -target x86_64-linux-gnu 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
-// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-STANDALONE %s
 // RUN: %clang -### -S %s -g0 -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -1512,9 +1512,18 @@
   else if (MachOArchName == "armv7k")
 OSTy = llvm::Triple::WatchOS;
   else if (MachOArchName != "armv6m" && MachOArchName != "armv7m" &&
-   MachOArchName != "armv7em")
+   MachOArchName != "armv7em") {
+// Default to macOS for other architectures.
 OSTy = llvm::Triple::MacOSX;
 
+// If there is no version specified on triple, and both host and target are
+// macos, use the host triple to infer OS version.
+llvm::Triple SystemTriple(llvm::sys::getProcessTriple());
+if (SystemTriple.isMacOSX() && !Triple.getOSMajorVersion())
+  return DarwinPlatform::createFromArch(
+  OSTy, getOSVersion(OSTy, SystemTriple, TheDriver));
+  }
+
   if (OSTy == llvm::Triple::UnknownOS)
 return None;
   return DarwinPlatform::createFromArch(OSTy,


Index: test/Driver/clang-g-opts.c
===
--- test/Driver/clang-g-opts.c
+++ test/Driver/clang-g-opts.c
@@ -3,7 +3,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
 
 // Assert that the toolchains which should default to a lower Dwarf version do so.
-// RUN: %clang -### -S %s -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
 // RUN: %clang -### -S %s -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
@@ -21,7 +21,7 @@
 //
 // RUN: %clang -### -S %s -g0 -g -target x86_64-linux-gnu 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
-// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-STANDALONE %s
 // RUN: %clang -### -S %s -g0 -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -1512,9 +1512,18 @@
   else if (MachOArchName == "armv7k")
 OSTy = llvm::Triple::WatchOS;
   else if (MachOArchName != "armv6m" && MachOArchName != "armv7m" &&
-   MachOArchName != "armv7em")
+   MachOArchName != "armv7em") {
+// Default to macOS for other architectures.
 OSTy = llvm::Triple::MacOSX;
 
+// If there is no version specified on triple, and both host and target are
+// macos, use the host triple to infer OS version.
+llvm::Triple SystemTriple(llvm::sys::getProcessTriple());
+if (SystemTriple.isMacOSX() && !Triple.getOSMajorVersion())
+  return DarwinPlatform::createFromArch(
+  OSTy, getOSVersion(OSTy, SystemTriple, TheDriver));
+  }
+
   if (OSTy == llvm::Triple::UnknownOS)
 return None;
   return DarwinPlatform::createFromArch(OSTy,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48849: [Driver][Darwin] Use Host Triple to infer target os version

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In https://reviews.llvm.org/D48849#1150246, @arphaman wrote:

> Hmm, the driver should not call `inferDeploymentTargetFromArch` when 
> `-target` is passed. Or am I missing something?


Good call. This only handles the *-apple-darwin case. Maybe the same should 
happen to *-apple-macosx.


Repository:
  rC Clang

https://reviews.llvm.org/D48849



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

[PATCH] D48849: [Driver][Darwin] Use Host Triple to infer target os version

[Steven Wu via Phabricator via cfe-commits]

steven_wu updated this revision to Diff 153822.
steven_wu added a comment.

handle *-apple-macosx target option


Repository:
  rC Clang

https://reviews.llvm.org/D48849

Files:
  lib/Driver/ToolChains/Darwin.cpp
  test/Driver/clang-g-opts.c
  test/Driver/target-triple-deployment.c


Index: test/Driver/target-triple-deployment.c
===
--- test/Driver/target-triple-deployment.c
+++ test/Driver/target-triple-deployment.c
@@ -1,5 +1,5 @@
 // RUN: touch %t.o
-// RUN: %clang -target x86_64-apple-macosx -### %t.o 2> %t.log
+// RUN: %clang -target x86_64-apple-macosx10.4 -### %t.o 2> %t.log
 // RUN: %clang -target x86_64-apple-darwin9 -### %t.o 2>> %t.log
 // RUN: %clang -target x86_64-apple-macosx10.7 -### %t.o 2>> %t.log
 //
Index: test/Driver/clang-g-opts.c
===
--- test/Driver/clang-g-opts.c
+++ test/Driver/clang-g-opts.c
@@ -3,7 +3,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
 
 // Assert that the toolchains which should default to a lower Dwarf version do 
so.
-// RUN: %clang -### -S %s -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
 // RUN: %clang -### -S %s -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
@@ -21,7 +21,7 @@
 //
 // RUN: %clang -### -S %s -g0 -g -target x86_64-linux-gnu 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
-// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-STANDALONE %s
 // RUN: %clang -### -S %s -g0 -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -1512,9 +1512,18 @@
   else if (MachOArchName == "armv7k")
 OSTy = llvm::Triple::WatchOS;
   else if (MachOArchName != "armv6m" && MachOArchName != "armv7m" &&
-   MachOArchName != "armv7em")
+   MachOArchName != "armv7em") {
+// Default to macOS for other architectures.
 OSTy = llvm::Triple::MacOSX;
 
+// If there is no version specified on triple, and both host and target are
+// macos, use the host triple to infer OS version.
+llvm::Triple SystemTriple(llvm::sys::getProcessTriple());
+if (SystemTriple.isMacOSX() && !Triple.getOSMajorVersion())
+  return DarwinPlatform::createFromArch(
+  OSTy, getOSVersion(OSTy, SystemTriple, TheDriver));
+  }
+
   if (OSTy == llvm::Triple::UnknownOS)
 return None;
   return DarwinPlatform::createFromArch(OSTy,
@@ -1529,7 +1538,17 @@
   if (Triple.getOS() == llvm::Triple::Darwin ||
   Triple.getOS() == llvm::Triple::UnknownOS)
 return None;
-  std::string OSVersion = getOSVersion(Triple.getOS(), Triple, TheDriver);
+
+  // If the host and target are both macos, and the OS version is not set in
+  // the target, infer the os version from host triple.
+  std::string OSVersion;
+  llvm::Triple SystemTriple(llvm::sys::getProcessTriple());
+  if (Triple.isMacOSX() && SystemTriple.isMacOSX() &&
+  !Triple.getOSMajorVersion())
+OSVersion = getOSVersion(SystemTriple.getOS(), SystemTriple, TheDriver);
+  else
+OSVersion = getOSVersion(Triple.getOS(), Triple, TheDriver);
+
   return DarwinPlatform::createFromTarget(Triple, OSVersion,
   
Args.getLastArg(options::OPT_target));
 }


Index: test/Driver/target-triple-deployment.c
===
--- test/Driver/target-triple-deployment.c
+++ test/Driver/target-triple-deployment.c
@@ -1,5 +1,5 @@
 // RUN: touch %t.o
-// RUN: %clang -target x86_64-apple-macosx -### %t.o 2> %t.log
+// RUN: %clang -target x86_64-apple-macosx10.4 -### %t.o 2> %t.log
 // RUN: %clang -target x86_64-apple-darwin9 -### %t.o 2>> %t.log
 // RUN: %clang -target x86_64-apple-macosx10.7 -### %t.o 2>> %t.log
 //
Index: test/Driver/clang-g-opts.c
===
--- test/Driver/clang-g-opts.c
+++ test/Driver/clang-g-opts.c
@@ -3,7 +3,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
 
 // Assert that the toolchains which should default to a lower Dwarf version do so.
-// RUN: %clang -### -S %s -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
 // RUN: %clang -### -S %s -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
@@ -21,7 +21,7 @@
 //
 // RUN: 

[PATCH] D48849: [Driver][Darwin] Use Host Triple to infer target os version

[Steven Wu via Phabricator via cfe-commits]

steven_wu updated this revision to Diff 153836.
steven_wu added a comment.

It is easier and cleaner if I just fold everything into getOSVersion.


Repository:
  rC Clang

https://reviews.llvm.org/D48849

Files:
  lib/Driver/ToolChains/Darwin.cpp
  test/Driver/clang-g-opts.c
  test/Driver/target-triple-deployment.c


Index: test/Driver/target-triple-deployment.c
===
--- test/Driver/target-triple-deployment.c
+++ test/Driver/target-triple-deployment.c
@@ -1,5 +1,5 @@
 // RUN: touch %t.o
-// RUN: %clang -target x86_64-apple-macosx -### %t.o 2> %t.log
+// RUN: %clang -target x86_64-apple-macosx10.4 -### %t.o 2> %t.log
 // RUN: %clang -target x86_64-apple-darwin9 -### %t.o 2>> %t.log
 // RUN: %clang -target x86_64-apple-macosx10.7 -### %t.o 2>> %t.log
 //
Index: test/Driver/clang-g-opts.c
===
--- test/Driver/clang-g-opts.c
+++ test/Driver/clang-g-opts.c
@@ -3,7 +3,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
 
 // Assert that the toolchains which should default to a lower Dwarf version do 
so.
-// RUN: %clang -### -S %s -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
 // RUN: %clang -### -S %s -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
@@ -21,7 +21,7 @@
 //
 // RUN: %clang -### -S %s -g0 -g -target x86_64-linux-gnu 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
-// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-STANDALONE %s
 // RUN: %clang -### -S %s -g0 -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -1472,10 +1472,16 @@
 std::string getOSVersion(llvm::Triple::OSType OS, const llvm::Triple &Triple,
  const Driver &TheDriver) {
   unsigned Major, Minor, Micro;
+  llvm::Triple SystemTriple(llvm::sys::getProcessTriple());
   switch (OS) {
   case llvm::Triple::Darwin:
   case llvm::Triple::MacOSX:
-if (!Triple.getMacOSXVersion(Major, Minor, Micro))
+// If there is no version specified on triple, and both host and target are
+// macos, use the host triple to infer OS version.
+if (Triple.isMacOSX() && SystemTriple.isMacOSX() &&
+!Triple.getOSMajorVersion())
+  SystemTriple.getMacOSXVersion(Major, Minor, Micro);
+else if (!Triple.getMacOSXVersion(Major, Minor, Micro))
   TheDriver.Diag(diag::err_drv_invalid_darwin_version)
   << Triple.getOSName();
 break;


Index: test/Driver/target-triple-deployment.c
===
--- test/Driver/target-triple-deployment.c
+++ test/Driver/target-triple-deployment.c
@@ -1,5 +1,5 @@
 // RUN: touch %t.o
-// RUN: %clang -target x86_64-apple-macosx -### %t.o 2> %t.log
+// RUN: %clang -target x86_64-apple-macosx10.4 -### %t.o 2> %t.log
 // RUN: %clang -target x86_64-apple-darwin9 -### %t.o 2>> %t.log
 // RUN: %clang -target x86_64-apple-macosx10.7 -### %t.o 2>> %t.log
 //
Index: test/Driver/clang-g-opts.c
===
--- test/Driver/clang-g-opts.c
+++ test/Driver/clang-g-opts.c
@@ -3,7 +3,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
 
 // Assert that the toolchains which should default to a lower Dwarf version do so.
-// RUN: %clang -### -S %s -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
 // RUN: %clang -### -S %s -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
@@ -21,7 +21,7 @@
 //
 // RUN: %clang -### -S %s -g0 -g -target x86_64-linux-gnu 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
-// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-STANDALONE %s
 // RUN: %clang -### -S %s -g0 -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -1472,10 +1472,16 @@
 std::string getOSVersion(llvm::Triple::OSType OS, const llvm::Triple &Triple,
   

[PATCH] D48849: [Driver][Darwin] Use Host Triple to infer target os version

[Steven Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL336168: [Driver][Darwin] Use Host Triple to infer target os 
version (authored by steven_wu, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D48849

Files:
  cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
  cfe/trunk/test/Driver/clang-g-opts.c
  cfe/trunk/test/Driver/target-triple-deployment.c


Index: cfe/trunk/test/Driver/target-triple-deployment.c
===
--- cfe/trunk/test/Driver/target-triple-deployment.c
+++ cfe/trunk/test/Driver/target-triple-deployment.c
@@ -1,5 +1,5 @@
 // RUN: touch %t.o
-// RUN: %clang -target x86_64-apple-macosx -### %t.o 2> %t.log
+// RUN: %clang -target x86_64-apple-macosx10.4 -### %t.o 2> %t.log
 // RUN: %clang -target x86_64-apple-darwin9 -### %t.o 2>> %t.log
 // RUN: %clang -target x86_64-apple-macosx10.7 -### %t.o 2>> %t.log
 //
Index: cfe/trunk/test/Driver/clang-g-opts.c
===
--- cfe/trunk/test/Driver/clang-g-opts.c
+++ cfe/trunk/test/Driver/clang-g-opts.c
@@ -3,7 +3,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
 
 // Assert that the toolchains which should default to a lower Dwarf version do 
so.
-// RUN: %clang -### -S %s -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
 // RUN: %clang -### -S %s -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
@@ -21,7 +21,7 @@
 //
 // RUN: %clang -### -S %s -g0 -g -target x86_64-linux-gnu 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
-// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-STANDALONE %s
 // RUN: %clang -### -S %s -g0 -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
Index: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
@@ -1472,10 +1472,16 @@
 std::string getOSVersion(llvm::Triple::OSType OS, const llvm::Triple &Triple,
  const Driver &TheDriver) {
   unsigned Major, Minor, Micro;
+  llvm::Triple SystemTriple(llvm::sys::getProcessTriple());
   switch (OS) {
   case llvm::Triple::Darwin:
   case llvm::Triple::MacOSX:
-if (!Triple.getMacOSXVersion(Major, Minor, Micro))
+// If there is no version specified on triple, and both host and target are
+// macos, use the host triple to infer OS version.
+if (Triple.isMacOSX() && SystemTriple.isMacOSX() &&
+!Triple.getOSMajorVersion())
+  SystemTriple.getMacOSXVersion(Major, Minor, Micro);
+else if (!Triple.getMacOSXVersion(Major, Minor, Micro))
   TheDriver.Diag(diag::err_drv_invalid_darwin_version)
   << Triple.getOSName();
 break;


Index: cfe/trunk/test/Driver/target-triple-deployment.c
===
--- cfe/trunk/test/Driver/target-triple-deployment.c
+++ cfe/trunk/test/Driver/target-triple-deployment.c
@@ -1,5 +1,5 @@
 // RUN: touch %t.o
-// RUN: %clang -target x86_64-apple-macosx -### %t.o 2> %t.log
+// RUN: %clang -target x86_64-apple-macosx10.4 -### %t.o 2> %t.log
 // RUN: %clang -target x86_64-apple-darwin9 -### %t.o 2>> %t.log
 // RUN: %clang -target x86_64-apple-macosx10.7 -### %t.o 2>> %t.log
 //
Index: cfe/trunk/test/Driver/clang-g-opts.c
===
--- cfe/trunk/test/Driver/clang-g-opts.c
+++ cfe/trunk/test/Driver/clang-g-opts.c
@@ -3,7 +3,7 @@
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
 
 // Assert that the toolchains which should default to a lower Dwarf version do so.
-// RUN: %clang -### -S %s -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
 // RUN: %clang -### -S %s -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
@@ -21,7 +21,7 @@
 //
 // RUN: %clang -### -S %s -g0 -g -target x86_64-linux-gnu 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G %s
-// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin 2>&1 \
+// RUN: %clang -### -S %s -g0 -g -target x86_64-apple-darwin8 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-STANDALONE %s
 // RUN: %clang -### -S %s -g0 -g -target i686-pc-openbsd 2>&1 \
 // RUN: | FileCheck --check-prefix=CHECK-WITH-G-DWARF2 %s
Index: cfe/trunk/lib/Driver/ToolC

[PATCH] D69327: [Clang][ThinLTO] Add a cache for compile phase output.

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

Can you clarify what do you mean by 'waste time optimizing a file that finally 
hit the object file cache'?

No matter what build system to use, it should figure out during an incremental 
build that the input wasn't changed and not rerun the clang invocation on the 
same input. If you are looking to achieve what `ccache` is doing, I don't think 
implement in the compiler is a good option. That should really be done on the 
build system level. This solution is strictly worse than `ccache` because 
`ccache` is able to fetch the optimized bitcode without even running clang 
frontend and ir-gen.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69327/new/

https://reviews.llvm.org/D69327



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

[PATCH] D69327: [Clang][ThinLTO] Add a cache for compile phase output.

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In D69327#1719411 , @ychen wrote:

> Thanks for the inputs @steven_wu @tejohnson. Totally agree with the points 
> you brought up. One last thing I'm not quite sure is the caching of 
> `-fthin-link-bitcode`. It is a `-cc1` option since it is a kind of 
> implementation of ThinLTO, right? I'm a little hesitant to begin writing up 
> patches to teach build system/caching tool (I could think of at least three 
> for our workload) to recognize this option because of that. If there are any 
> changes to the option, the same thing needs to be done again. Do you have any 
> thoughts on that? Is the option in use for your workload and do you think it 
> is stable enough to have build systems caching for it? (Another option is to 
> produce `-fthin-link-bitcode` output post compile time which I assume having 
> total build time impact to some degree).


`-fthin-link-bitcode` option is used to run distributed thin link. The format 
is not stable but it is deterministic for a fixed compiler version. You should 
be able to cache the thin-link-bitcode and expected it to be used only by the 
same compiler version.

For any build system that implements caching, it must take compiler version 
into consideration because different compiler will produce different output. I 
don't think the rule to cache thin-link-bitcode is any different from any other 
output during the build.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69327/new/

https://reviews.llvm.org/D69327



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

[PATCH] D69327: [Clang][ThinLTO] Add a cache for compile phase output.

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

`ccache` does not have the support for this, I am just saying that this can be 
easily implemented in `ccache` and that would be much better than the proposed 
solution here.

If we need to add a clang driver flag so build system can better support to 
detect thin bitcode as output and caching them, we should just add the official 
driver flag.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69327/new/

https://reviews.llvm.org/D69327



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

[PATCH] D69406: [clang][ThinLTO] Promote cc1 -fthin_link_bitcode to driver -fthinlto_link_bitcode

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69406/new/

https://reviews.llvm.org/D69406



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

[PATCH] D79511: [ObjC] Add compatibility mode for type checking of qualified id block parameters.

[Steven Wu via Phabricator via cfe-commits]

steven_wu added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:2382
+  // Foundation/NSItemProvider.h.
+  CC1Args.push_back("-fcompatibility-qualified-id-block-type-checking");
 }

Not that I know there is a better place to put this option, this is not really 
a ClangTarget option.

Putting this in `addClangTargetOptions` will put this onto codegen flags for 
`-fembed-bitcode`, but this is just a frontend option that doesn't affect code 
generation.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79511/new/

https://reviews.llvm.org/D79511



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

[PATCH] D84564: [darwin] build and link with a separate compiler-rt builtins library for device simulators

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84564/new/

https://reviews.llvm.org/D84564



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

[PATCH] D83813: [clang] Teach -fembed-bitcode option not to embed W_value Group

[Steven Wu via Phabricator via cfe-commits]

steven_wu created this revision.
steven_wu added reviewers: zixuw, arphaman.
Herald added subscribers: ributzka, dexonsmith, jkorous.
Herald added a project: clang.

-fembed-bitcode options doesn't embed warning options since they are
useless to code generation. Make sure it handles the W_value group and
not embed those options in the output.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83813

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Frontend/embed-bitcode.ll


Index: clang/test/Frontend/embed-bitcode.ll
===
--- clang/test/Frontend/embed-bitcode.ll
+++ clang/test/Frontend/embed-bitcode.ll
@@ -37,6 +37,11 @@
 ; CHECK: @llvm.cmdline = private constant
 ; CHECK: section "__LLVM,__cmdline"
 
+; check warning options are not embedded
+; RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -emit-llvm \
+; RUN:-fembed-bitcode=all -x ir %s -o - -Wall -Wundef-prefix=TEST \
+; RUN:| FileCheck %s -check-prefix=CHECK-WARNING
+
 ; CHECK-ELF: @llvm.embedded.module
 ; CHECK-ELF: section ".llvmbc"
 ; CHECK-ELF: @llvm.cmdline
@@ -54,6 +59,9 @@
 ; CHECK-MARKER: @llvm.cmdline
 ; CHECK-MARKER: section "__LLVM,__cmdline"
 
+; CHECK-WARNING-NOT: Wall
+; CHECK-WARNING-NOT: Wundef-prefix
+
 define i32 @f0() {
   ret i32 0
 }
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1087,7 +1087,9 @@
   A->getOption().getID() == options::OPT_x ||
   A->getOption().getID() == options::OPT_fembed_bitcode ||
   (A->getOption().getGroup().isValid() &&
-   A->getOption().getGroup().getID() == options::OPT_W_Group))
+   A->getOption().getGroup().getID() == options::OPT_W_Group) ||
+  (A->getOption().getGroup().isValid() &&
+   A->getOption().getGroup().getID() == options::OPT_W_value_Group))
 continue;
   ArgStringList ASL;
   A->render(Args, ASL);


Index: clang/test/Frontend/embed-bitcode.ll
===
--- clang/test/Frontend/embed-bitcode.ll
+++ clang/test/Frontend/embed-bitcode.ll
@@ -37,6 +37,11 @@
 ; CHECK: @llvm.cmdline = private constant
 ; CHECK: section "__LLVM,__cmdline"
 
+; check warning options are not embedded
+; RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -emit-llvm \
+; RUN:-fembed-bitcode=all -x ir %s -o - -Wall -Wundef-prefix=TEST \
+; RUN:| FileCheck %s -check-prefix=CHECK-WARNING
+
 ; CHECK-ELF: @llvm.embedded.module
 ; CHECK-ELF: section ".llvmbc"
 ; CHECK-ELF: @llvm.cmdline
@@ -54,6 +59,9 @@
 ; CHECK-MARKER: @llvm.cmdline
 ; CHECK-MARKER: section "__LLVM,__cmdline"
 
+; CHECK-WARNING-NOT: Wall
+; CHECK-WARNING-NOT: Wundef-prefix
+
 define i32 @f0() {
   ret i32 0
 }
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1087,7 +1087,9 @@
   A->getOption().getID() == options::OPT_x ||
   A->getOption().getID() == options::OPT_fembed_bitcode ||
   (A->getOption().getGroup().isValid() &&
-   A->getOption().getGroup().getID() == options::OPT_W_Group))
+   A->getOption().getGroup().getID() == options::OPT_W_Group) ||
+  (A->getOption().getGroup().isValid() &&
+   A->getOption().getGroup().getID() == options::OPT_W_value_Group))
 continue;
   ArgStringList ASL;
   A->render(Args, ASL);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83813: [clang] Teach -fembed-bitcode option not to embed W_value Group

[Steven Wu via Phabricator via cfe-commits]

steven_wu updated this revision to Diff 277975.
steven_wu added a comment.

Use `Option::match` instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83813/new/

https://reviews.llvm.org/D83813

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Frontend/embed-bitcode.ll


Index: clang/test/Frontend/embed-bitcode.ll
===
--- clang/test/Frontend/embed-bitcode.ll
+++ clang/test/Frontend/embed-bitcode.ll
@@ -37,6 +37,11 @@
 ; CHECK: @llvm.cmdline = private constant
 ; CHECK: section "__LLVM,__cmdline"
 
+; check warning options are not embedded
+; RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -emit-llvm \
+; RUN:-fembed-bitcode=all -x ir %s -o - -Wall -Wundef-prefix=TEST \
+; RUN:| FileCheck %s -check-prefix=CHECK-WARNING
+
 ; CHECK-ELF: @llvm.embedded.module
 ; CHECK-ELF: section ".llvmbc"
 ; CHECK-ELF: @llvm.cmdline
@@ -54,6 +59,9 @@
 ; CHECK-MARKER: @llvm.cmdline
 ; CHECK-MARKER: section "__LLVM,__cmdline"
 
+; CHECK-WARNING-NOT: Wall
+; CHECK-WARNING-NOT: Wundef-prefix
+
 define i32 @f0() {
   ret i32 0
 }
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1086,8 +1086,7 @@
   A->getOption().getID() == options::OPT_INPUT ||
   A->getOption().getID() == options::OPT_x ||
   A->getOption().getID() == options::OPT_fembed_bitcode ||
-  (A->getOption().getGroup().isValid() &&
-   A->getOption().getGroup().getID() == options::OPT_W_Group))
+  A->getOption().matches(options::OPT_W_Group))
 continue;
   ArgStringList ASL;
   A->render(Args, ASL);


Index: clang/test/Frontend/embed-bitcode.ll
===
--- clang/test/Frontend/embed-bitcode.ll
+++ clang/test/Frontend/embed-bitcode.ll
@@ -37,6 +37,11 @@
 ; CHECK: @llvm.cmdline = private constant
 ; CHECK: section "__LLVM,__cmdline"
 
+; check warning options are not embedded
+; RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -emit-llvm \
+; RUN:-fembed-bitcode=all -x ir %s -o - -Wall -Wundef-prefix=TEST \
+; RUN:| FileCheck %s -check-prefix=CHECK-WARNING
+
 ; CHECK-ELF: @llvm.embedded.module
 ; CHECK-ELF: section ".llvmbc"
 ; CHECK-ELF: @llvm.cmdline
@@ -54,6 +59,9 @@
 ; CHECK-MARKER: @llvm.cmdline
 ; CHECK-MARKER: section "__LLVM,__cmdline"
 
+; CHECK-WARNING-NOT: Wall
+; CHECK-WARNING-NOT: Wundef-prefix
+
 define i32 @f0() {
   ret i32 0
 }
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1086,8 +1086,7 @@
   A->getOption().getID() == options::OPT_INPUT ||
   A->getOption().getID() == options::OPT_x ||
   A->getOption().getID() == options::OPT_fembed_bitcode ||
-  (A->getOption().getGroup().isValid() &&
-   A->getOption().getGroup().getID() == options::OPT_W_Group))
+  A->getOption().matches(options::OPT_W_Group))
 continue;
   ArgStringList ASL;
   A->render(Args, ASL);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83813: [clang] Teach -fembed-bitcode option not to embed W_value Group

[Steven Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2b42080b51c9: [clang] Teach -fembed-bitcode option not to 
embed W_value Group (authored by steven_wu).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83813/new/

https://reviews.llvm.org/D83813

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Frontend/embed-bitcode.ll


Index: clang/test/Frontend/embed-bitcode.ll
===
--- clang/test/Frontend/embed-bitcode.ll
+++ clang/test/Frontend/embed-bitcode.ll
@@ -37,6 +37,11 @@
 ; CHECK: @llvm.cmdline = private constant
 ; CHECK: section "__LLVM,__cmdline"
 
+; check warning options are not embedded
+; RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -emit-llvm \
+; RUN:-fembed-bitcode=all -x ir %s -o - -Wall -Wundef-prefix=TEST \
+; RUN:| FileCheck %s -check-prefix=CHECK-WARNING
+
 ; CHECK-ELF: @llvm.embedded.module
 ; CHECK-ELF: section ".llvmbc"
 ; CHECK-ELF: @llvm.cmdline
@@ -54,6 +59,9 @@
 ; CHECK-MARKER: @llvm.cmdline
 ; CHECK-MARKER: section "__LLVM,__cmdline"
 
+; CHECK-WARNING-NOT: Wall
+; CHECK-WARNING-NOT: Wundef-prefix
+
 define i32 @f0() {
   ret i32 0
 }
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1086,8 +1086,7 @@
   A->getOption().getID() == options::OPT_INPUT ||
   A->getOption().getID() == options::OPT_x ||
   A->getOption().getID() == options::OPT_fembed_bitcode ||
-  (A->getOption().getGroup().isValid() &&
-   A->getOption().getGroup().getID() == options::OPT_W_Group))
+  A->getOption().matches(options::OPT_W_Group))
 continue;
   ArgStringList ASL;
   A->render(Args, ASL);


Index: clang/test/Frontend/embed-bitcode.ll
===
--- clang/test/Frontend/embed-bitcode.ll
+++ clang/test/Frontend/embed-bitcode.ll
@@ -37,6 +37,11 @@
 ; CHECK: @llvm.cmdline = private constant
 ; CHECK: section "__LLVM,__cmdline"
 
+; check warning options are not embedded
+; RUN: %clang_cc1 -triple thumbv7-apple-ios8.0.0 -emit-llvm \
+; RUN:-fembed-bitcode=all -x ir %s -o - -Wall -Wundef-prefix=TEST \
+; RUN:| FileCheck %s -check-prefix=CHECK-WARNING
+
 ; CHECK-ELF: @llvm.embedded.module
 ; CHECK-ELF: section ".llvmbc"
 ; CHECK-ELF: @llvm.cmdline
@@ -54,6 +59,9 @@
 ; CHECK-MARKER: @llvm.cmdline
 ; CHECK-MARKER: section "__LLVM,__cmdline"
 
+; CHECK-WARNING-NOT: Wall
+; CHECK-WARNING-NOT: Wundef-prefix
+
 define i32 @f0() {
   ret i32 0
 }
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1086,8 +1086,7 @@
   A->getOption().getID() == options::OPT_INPUT ||
   A->getOption().getID() == options::OPT_x ||
   A->getOption().getID() == options::OPT_fembed_bitcode ||
-  (A->getOption().getGroup().isValid() &&
-   A->getOption().getGroup().getID() == options::OPT_W_Group))
+  A->getOption().matches(options::OPT_W_Group))
 continue;
   ArgStringList ASL;
   A->render(Args, ASL);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82428: [clang][driver] allow `-arch arm64` to be used to build for mac when on Apple Silicon Mac without explicit `-target`

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

LGTM.

Not sure if it makes more sense to break the patch into two commits:

- config.guess change is for building the correct host triple on apple silicon 
machine without explicitly specify it.
- the driver change is for better default on Apple silicon Mac.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82428/new/

https://reviews.llvm.org/D82428



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

[PATCH] D82696: [darwin][driver] isMacosxVersionLT should check against the minimum supported OS version

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82696/new/

https://reviews.llvm.org/D82696



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

[PATCH] D82777: Clang Driver: Use Apple ld64's new @response-file support.

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

LGTM.

Agree this is ugly but the clean up looks fine. Not sure how to write a test as 
well but I can see the `@` file path is triggered correctly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82777/new/

https://reviews.llvm.org/D82777



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

[PATCH] D82782: Clang Driver: refactor support for writing response files to be specified at Command creation, rather than as part of the Tool.

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82782/new/

https://reviews.llvm.org/D82782



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

[PATCH] D85367: [clang, test, Darwin] Fix tests expecting Darwin target

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85367/new/

https://reviews.llvm.org/D85367

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

[PATCH] D84908: [darwin][compiler-rt] build libclang_rt.sim.a Apple Silicon slice, if SDK supports it

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84908/new/

https://reviews.llvm.org/D84908

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

[PATCH] D41035: [driver][darwin] Refactor the target selection code, NFC

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

LGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D41035



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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In D131469#3768308 , @dongjunduo 
wrote:

> In D131469#3768288 , @dyung wrote:
>
>> In D131469#3768283 , @dongjunduo 
>> wrote:
>>
>>> In D131469#3768282 , @dyung wrote:
>>>
 In D131469#3768274 , @dongjunduo 
 wrote:

> In D131469#3768260 , @dyung 
> wrote:
>
>> The test you added is failing on the PS4 linux bot because the PS4 
>> platform requires an external linker that isn't present. Is linking 
>> necessary for your test? Or can -S or even -c work instead to accomplish 
>> what you are trying to test?
>
> Yeah the new test case is designed to test the compiling jobs with a 
> linking stage.
>
> Are there any options or measures to avoid this test running on the PS4?

 You could mark it as XFAIL: ps4

 Your change also seems to have possibly the same issue when run on our PS5 
 Windows bot:
 https://lab.llvm.org/buildbot/#/builders/216/builds/9260

   $ ":" "RUN: at line 2"
   $ "z:\test\build\bin\clang.exe" "--driver-mode=g++" "-ftime-trace" 
 "-ftime-trace-granularity=0" "-o" 
 "Z:\test\build\tools\clang\test\Driver\Output/exe/check-time-trace" 
 "Z:\test\llvm-project\clang\test\Driver\check-time-trace.cpp"
   # command stderr:
   clang: error: unable to execute command: program not executable
   clang: error: linker command failed with exit code 1 (use -v to see 
 invocation)
>>>
>>> How about "**UNSUPPORTED: ps4, ps5**"
>>
>> That would likely work I think.
>
> Done with the commit 39221ad 
> .

I don't like how this done. This generally won't work for Darwin platform as 
well since you don't know where SDK is. Whether you have a linker or not has 
nothing to do with the host platform, you need to check for if the linker is 
available.

This is also a regression in test coverage since all the supported tests in 
lines after you added are no longer executed on the platform you excludes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131469/new/

https://reviews.llvm.org/D131469

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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In D131469#3768500 , @dongjunduo 
wrote:

> These related commits have been reverted temporarily.

Thanks. Another way to do this is that as you don't really care what linker 
does in this test case, you just need to fake a linker with a shell script then 
let the clang driver to invoke that script as a linker.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131469/new/

https://reviews.llvm.org/D131469

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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.

Thanks


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131469/new/

https://reviews.llvm.org/D131469

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

[PATCH] D133509: Frontend: Adopt llvm::vfs::OutputBackend in CompilerInstance

[Steven Wu via Phabricator via cfe-commits]

steven_wu created this revision.
steven_wu added reviewers: sammccall, benlangmuir, raghavmedicherla, kzhuravl, 
dexonsmith.
Herald added a subscriber: ributzka.
Herald added a project: All.
steven_wu requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Adopt new virtual output backend in CompilerInstance.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133509

Files:
  clang/include/clang/Frontend/CompilerInstance.h
  clang/lib/Frontend/CompilerInstance.cpp

Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -53,6 +53,7 @@
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TimeProfiler.h"
 #include "llvm/Support/Timer.h"
+#include "llvm/Support/VirtualOutputBackends.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -521,6 +522,10 @@
 collectVFSEntries(*this, ModuleDepCollector);
   }
 
+  // Modules need an output manager.
+  if (!hasOutputBackend())
+createOutputBackend();
+
   for (auto &Listener : DependencyCollectors)
 Listener->attachToPreprocessor(*PP);
 
@@ -764,36 +769,19 @@
   // The ASTConsumer can own streams that write to the output files.
   assert(!hasASTConsumer() && "ASTConsumer should be reset");
   // Ignore errors that occur when trying to discard the temp file.
-  for (OutputFile &OF : OutputFiles) {
-if (EraseFiles) {
-  if (OF.File)
-consumeError(OF.File->discard());
-  if (!OF.Filename.empty())
-llvm::sys::fs::remove(OF.Filename);
-  continue;
-}
-
-if (!OF.File)
-  continue;
-
-if (OF.File->TmpName.empty()) {
-  consumeError(OF.File->discard());
-  continue;
-}
-
-// If '-working-directory' was passed, the output filename should be
-// relative to that.
-SmallString<128> NewOutFile(OF.Filename);
-FileMgr->FixupRelativePath(NewOutFile);
-
-llvm::Error E = OF.File->keep(NewOutFile);
-if (!E)
-  continue;
-
-getDiagnostics().Report(diag::err_unable_to_rename_temp)
-<< OF.File->TmpName << OF.Filename << std::move(E);
-
-llvm::sys::fs::remove(OF.File->TmpName);
+  if (!EraseFiles) {
+for (auto &O : OutputFiles)
+  llvm::handleAllErrors(
+  O.keep(),
+  [&](const llvm::vfs::TempFileOutputError &E) {
+getDiagnostics().Report(diag::err_unable_to_rename_temp)
+<< E.getTempPath() << E.getOutputPath()
+<< E.convertToErrorCode().message();
+  },
+  [&](const llvm::vfs::OutputError &E) {
+getDiagnostics().Report(diag::err_fe_unable_to_open_output)
+<< E.getOutputPath() << E.convertToErrorCode().message();
+  });
   }
   OutputFiles.clear();
   if (DeleteBuiltModules) {
@@ -827,6 +815,30 @@
   return std::make_unique();
 }
 
+void CompilerInstance::setOutputBackend(
+IntrusiveRefCntPtr NewOutputs) {
+  assert(!TheOutputBackend && "Already has an output manager");
+  TheOutputBackend = std::move(NewOutputs);
+}
+
+void CompilerInstance::createOutputBackend() {
+  assert(!TheOutputBackend && "Already has an output manager");
+  TheOutputBackend =
+  llvm::makeIntrusiveRefCnt();
+}
+
+llvm::vfs::OutputBackend &CompilerInstance::getOutputBackend() {
+  assert(TheOutputBackend);
+  return *TheOutputBackend;
+}
+
+llvm::vfs::OutputBackend &CompilerInstance::getOrCreateOutputBackend() {
+  if (!hasOutputBackend())
+createOutputBackend();
+  return getOutputBackend();
+}
+
+
 std::unique_ptr
 CompilerInstance::createOutputFile(StringRef OutputPath, bool Binary,
bool RemoveFileOnSignal, bool UseTemporary,
@@ -849,94 +861,29 @@
   assert((!CreateMissingDirectories || UseTemporary) &&
  "CreateMissingDirectories is only allowed when using temporary files");
 
-  std::unique_ptr OS;
-  Optional OSFile;
-
-  if (UseTemporary) {
-if (OutputPath == "-")
-  UseTemporary = false;
-else {
-  llvm::sys::fs::file_status Status;
-  llvm::sys::fs::status(OutputPath, Status);
-  if (llvm::sys::fs::exists(Status)) {
-// Fail early if we can't write to the final destination.
-if (!llvm::sys::fs::can_write(OutputPath))
-  return llvm::errorCodeToError(
-  make_error_code(llvm::errc::operation_not_permitted));
-
-// Don't use a temporary if the output is a special file. This handles
-// things like '-o /dev/null'
-if (!llvm::sys::fs::is_regular_file(Status))
-  UseTemporary = false;
-  }
-}
-  }
-
-  Optional Temp;
-  if (UseTemporary) {
-// Create a temporary file.
-// Insert - before the extension (if any), and because some tools
-// (noticeable, clang's own GlobalModuleIndex.cpp) glob for build
-// artifacts, also append .tmp.
-StringRef OutputExtension = llv

[PATCH] D95497: Frontend: Respect -working-directory when checking if output files can be written

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

LGTM. I will commit this if no objection.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95497/new/

https://reviews.llvm.org/D95497

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

[PATCH] D95497: Frontend: Respect -working-directory when checking if output files can be written

[Steven Wu via Phabricator via cfe-commits]

steven_wu requested changes to this revision.
steven_wu added a comment.
This revision now requires changes to proceed.

Actually, we need to fix the case when output is '-'.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95497/new/

https://reviews.llvm.org/D95497

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

[PATCH] D95497: Frontend: Respect -working-directory when checking if output files can be written

[Steven Wu via Phabricator via cfe-commits]

steven_wu updated this revision to Diff 458823.
steven_wu edited the summary of this revision.
steven_wu added a comment.

Rebase patch and fix the problem when the input is '-'


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95497/new/

https://reviews.llvm.org/D95497

Files:
  clang/lib/Frontend/CompilerInstance.cpp
  clang/test/Frontend/output-paths.c


Index: clang/test/Frontend/output-paths.c
===
--- clang/test/Frontend/output-paths.c
+++ clang/test/Frontend/output-paths.c
@@ -2,3 +2,12 @@
 // RUN: FileCheck -check-prefix=OUTPUTFAIL -DMSG=%errc_ENOENT -input-file=%t %s
 
 // OUTPUTFAIL: error: unable to open output file 
'{{.*}}doesnotexist{{.}}somename': '[[MSG]]'
+
+// Check that -working-directory is respected when diagnosing output failures.
+//
+// RUN: rm -rf %t.d && mkdir -p %t.d/%basename_t-inner.d
+// RUN: %clang_cc1 -emit-llvm -working-directory %t.d -E -o 
%basename_t-inner.d/somename %s -verify
+// expected-no-diagnostics
+
+// RUN: %clang_cc1 -working-directory %t.d -E %s -o - | FileCheck %s
+// CHECK: # 1
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -781,12 +781,7 @@
   continue;
 }
 
-// If '-working-directory' was passed, the output filename should be
-// relative to that.
-SmallString<128> NewOutFile(OF.Filename);
-FileMgr->FixupRelativePath(NewOutFile);
-
-llvm::Error E = OF.File->keep(NewOutFile);
+llvm::Error E = OF.File->keep(OF.Filename);
 if (!E)
   continue;
 
@@ -849,6 +844,15 @@
   assert((!CreateMissingDirectories || UseTemporary) &&
  "CreateMissingDirectories is only allowed when using temporary 
files");
 
+  // If '-working-directory' was passed, the output filename should be
+  // relative to that.
+  Optional> AbsPath;
+  if (OutputPath != "-" && !llvm::sys::path::is_absolute(OutputPath)) {
+AbsPath.emplace(OutputPath);
+FileMgr->FixupRelativePath(*AbsPath);
+OutputPath = *AbsPath;
+  }
+
   std::unique_ptr OS;
   Optional OSFile;
 


Index: clang/test/Frontend/output-paths.c
===
--- clang/test/Frontend/output-paths.c
+++ clang/test/Frontend/output-paths.c
@@ -2,3 +2,12 @@
 // RUN: FileCheck -check-prefix=OUTPUTFAIL -DMSG=%errc_ENOENT -input-file=%t %s
 
 // OUTPUTFAIL: error: unable to open output file '{{.*}}doesnotexist{{.}}somename': '[[MSG]]'
+
+// Check that -working-directory is respected when diagnosing output failures.
+//
+// RUN: rm -rf %t.d && mkdir -p %t.d/%basename_t-inner.d
+// RUN: %clang_cc1 -emit-llvm -working-directory %t.d -E -o %basename_t-inner.d/somename %s -verify
+// expected-no-diagnostics
+
+// RUN: %clang_cc1 -working-directory %t.d -E %s -o - | FileCheck %s
+// CHECK: # 1
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -781,12 +781,7 @@
   continue;
 }
 
-// If '-working-directory' was passed, the output filename should be
-// relative to that.
-SmallString<128> NewOutFile(OF.Filename);
-FileMgr->FixupRelativePath(NewOutFile);
-
-llvm::Error E = OF.File->keep(NewOutFile);
+llvm::Error E = OF.File->keep(OF.Filename);
 if (!E)
   continue;
 
@@ -849,6 +844,15 @@
   assert((!CreateMissingDirectories || UseTemporary) &&
  "CreateMissingDirectories is only allowed when using temporary files");
 
+  // If '-working-directory' was passed, the output filename should be
+  // relative to that.
+  Optional> AbsPath;
+  if (OutputPath != "-" && !llvm::sys::path::is_absolute(OutputPath)) {
+AbsPath.emplace(OutputPath);
+FileMgr->FixupRelativePath(*AbsPath);
+OutputPath = *AbsPath;
+  }
+
   std::unique_ptr OS;
   Optional OSFile;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95497: Frontend: Respect -working-directory when checking if output files can be written

[Steven Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG493766e06847: Frontend: Respect -working-directory when 
checking if output files can be… (authored by steven_wu).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95497/new/

https://reviews.llvm.org/D95497

Files:
  clang/lib/Frontend/CompilerInstance.cpp
  clang/test/Frontend/output-paths.c


Index: clang/test/Frontend/output-paths.c
===
--- clang/test/Frontend/output-paths.c
+++ clang/test/Frontend/output-paths.c
@@ -2,3 +2,12 @@
 // RUN: FileCheck -check-prefix=OUTPUTFAIL -DMSG=%errc_ENOENT -input-file=%t %s
 
 // OUTPUTFAIL: error: unable to open output file 
'{{.*}}doesnotexist{{.}}somename': '[[MSG]]'
+
+// Check that -working-directory is respected when diagnosing output failures.
+//
+// RUN: rm -rf %t.d && mkdir -p %t.d/%basename_t-inner.d
+// RUN: %clang_cc1 -emit-llvm -working-directory %t.d -E -o 
%basename_t-inner.d/somename %s -verify
+// expected-no-diagnostics
+
+// RUN: %clang_cc1 -working-directory %t.d -E %s -o - | FileCheck %s
+// CHECK: # 1
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -781,12 +781,7 @@
   continue;
 }
 
-// If '-working-directory' was passed, the output filename should be
-// relative to that.
-SmallString<128> NewOutFile(OF.Filename);
-FileMgr->FixupRelativePath(NewOutFile);
-
-llvm::Error E = OF.File->keep(NewOutFile);
+llvm::Error E = OF.File->keep(OF.Filename);
 if (!E)
   continue;
 
@@ -849,6 +844,15 @@
   assert((!CreateMissingDirectories || UseTemporary) &&
  "CreateMissingDirectories is only allowed when using temporary 
files");
 
+  // If '-working-directory' was passed, the output filename should be
+  // relative to that.
+  Optional> AbsPath;
+  if (OutputPath != "-" && !llvm::sys::path::is_absolute(OutputPath)) {
+AbsPath.emplace(OutputPath);
+FileMgr->FixupRelativePath(*AbsPath);
+OutputPath = *AbsPath;
+  }
+
   std::unique_ptr OS;
   Optional OSFile;
 


Index: clang/test/Frontend/output-paths.c
===
--- clang/test/Frontend/output-paths.c
+++ clang/test/Frontend/output-paths.c
@@ -2,3 +2,12 @@
 // RUN: FileCheck -check-prefix=OUTPUTFAIL -DMSG=%errc_ENOENT -input-file=%t %s
 
 // OUTPUTFAIL: error: unable to open output file '{{.*}}doesnotexist{{.}}somename': '[[MSG]]'
+
+// Check that -working-directory is respected when diagnosing output failures.
+//
+// RUN: rm -rf %t.d && mkdir -p %t.d/%basename_t-inner.d
+// RUN: %clang_cc1 -emit-llvm -working-directory %t.d -E -o %basename_t-inner.d/somename %s -verify
+// expected-no-diagnostics
+
+// RUN: %clang_cc1 -working-directory %t.d -E %s -o - | FileCheck %s
+// CHECK: # 1
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -781,12 +781,7 @@
   continue;
 }
 
-// If '-working-directory' was passed, the output filename should be
-// relative to that.
-SmallString<128> NewOutFile(OF.Filename);
-FileMgr->FixupRelativePath(NewOutFile);
-
-llvm::Error E = OF.File->keep(NewOutFile);
+llvm::Error E = OF.File->keep(OF.Filename);
 if (!E)
   continue;
 
@@ -849,6 +844,15 @@
   assert((!CreateMissingDirectories || UseTemporary) &&
  "CreateMissingDirectories is only allowed when using temporary files");
 
+  // If '-working-directory' was passed, the output filename should be
+  // relative to that.
+  Optional> AbsPath;
+  if (OutputPath != "-" && !llvm::sys::path::is_absolute(OutputPath)) {
+AbsPath.emplace(OutputPath);
+FileMgr->FixupRelativePath(*AbsPath);
+OutputPath = *AbsPath;
+  }
+
   std::unique_ptr OS;
   Optional OSFile;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129220: [clang] Cleanup ASTContext before output files in crash recovery for modules

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.

LGTM. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129220/new/

https://reviews.llvm.org/D129220

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

[PATCH] D106316: [clang][darwin] Add support for the -mtargetos= option to the driver

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

Looks good in general. Just one corner case that we need to decide with 
direction we go, following command builds arm64-ios and x86_64-ios-simulator:
`clang -arch arm64 -arch x86_64 -c -o test.o test.c -mios-version-min=14`
Should we document and deprecate that behavior?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106316/new/

https://reviews.llvm.org/D106316

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

[PATCH] D118352: [clang][ABI] New c++20 modules mangling scheme

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In D118352#3368922 , @ChuanqiXu wrote:

> In D118352#3368919 , @phosek wrote:
>
>> We're also seeing this issue on our Mac bots, is it possible to revert it?
>
> I've reverted it. Since this is the new feature, I think it wouldn't so hurry 
> to land it. BTW, I think it would be helpful to provide more information to 
> fix it.

It breaks the macOS bot for LLVM, you can click the test case for its output: 
https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/27745/testReport/

You can see the error output. The problem might be that `dso_local` does not 
get codegen for macho targets.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118352/new/

https://reviews.llvm.org/D118352

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

[PATCH] D121332: Cleanup includes: DebugInfo & CodeGen

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

This breaks implicit module on macOS bots: 
https://green.lab.llvm.org/green/job/lldb-cmake/42098/console

Error message:

  
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/include/llvm/CodeGen/MachinePipeliner.h:136:3:
 error: missing '#include "llvm/ADT/SetVector.h"'; 'SetVector' must be declared 
before it is used
SetVector NodeOrder;
^
  
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/include/llvm/ADT/SetVector.h:40:7:
 note: declaration here is not visible
  class SetVector {
^
  
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/lib/CodeGen/AtomicExpandPass.cpp:21:10:
 fatal error: could not build module 'LLVM_Backend'
  #include "llvm/CodeGen/AtomicExpandUtils.h"
   ^~
  2 errors generated.

This can be fixed by adding `ADT/SetVector.h` to 
`include/llvm/CodeGen/MachinePipeliner.h`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121332/new/

https://reviews.llvm.org/D121332

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

[PATCH] D72404: [ThinLTO/FullLTO] Support Os and Oz

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

I just saw this. I know it is a good idea to have choice during link time for 
the pipeline configuration and from your benchmark it also has size impact on 
the output. I also feel like this is going in the wrong direction as if I have 
part of the object files built with -O3 and part built with -Oz, I need to make 
a choice of unoptimized part of the program.

Before I say yes or no to this patch, have you figured out what are the passes 
that causes the most size regression? Ideally, with function attributes on the 
function, it shouldn't be much size impact on the output.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72404/new/

https://reviews.llvm.org/D72404

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

[PATCH] D118862: [clang][driver] add clang driver support for emitting macho files with two build version load commands

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118862/new/

https://reviews.llvm.org/D118862

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

[PATCH] D130205: [Darwin toolchain] Tune the logic for finding arclite.

[Steven Wu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd0728260577d: [Darwin toolchain] Tune the logic for finding 
arclite. (authored by steven_wu).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130205/new/

https://reviews.llvm.org/D130205

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp


Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1141,25 +1141,38 @@
   SmallString<128> P(getDriver().ClangExecutable);
   llvm::sys::path::remove_filename(P); // 'clang'
   llvm::sys::path::remove_filename(P); // 'bin'
+  llvm::sys::path::append(P, "lib", "arc");
 
   // 'libarclite' usually lives in the same toolchain as 'clang'. However, the
   // Swift open source toolchains for macOS distribute Clang without 
libarclite.
   // In that case, to allow the linker to find 'libarclite', we point to the
   // 'libarclite' in the XcodeDefault toolchain instead.
-  if (getXcodeDeveloperPath(P).empty()) {
-if (const Arg *A = Args.getLastArg(options::OPT_isysroot)) {
+  if (!getVFS().exists(P)) {
+auto updatePath = [&](const Arg *A) {
   // Try to infer the path to 'libarclite' in the toolchain from the
   // specified SDK path.
   StringRef XcodePathForSDK = getXcodeDeveloperPath(A->getValue());
-  if (!XcodePathForSDK.empty()) {
-P = XcodePathForSDK;
-llvm::sys::path::append(P, "Toolchains/XcodeDefault.xctoolchain/usr");
-  }
+  if (XcodePathForSDK.empty())
+return false;
+
+  P = XcodePathForSDK;
+  llvm::sys::path::append(P, "Toolchains/XcodeDefault.xctoolchain/usr",
+  "lib", "arc");
+  return getVFS().exists(P);
+};
+
+bool updated = false;
+if (const Arg *A = Args.getLastArg(options::OPT_isysroot))
+  updated = updatePath(A);
+
+if (!updated) {
+  if (const Arg *A = Args.getLastArg(options::OPT__sysroot_EQ))
+updatePath(A);
 }
   }
 
   CmdArgs.push_back("-force_load");
-  llvm::sys::path::append(P, "lib", "arc", "libarclite_");
+  llvm::sys::path::append(P, "libarclite_");
   // Mash in the platform.
   if (isTargetWatchOSSimulator())
 P += "watchsimulator";


Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1141,25 +1141,38 @@
   SmallString<128> P(getDriver().ClangExecutable);
   llvm::sys::path::remove_filename(P); // 'clang'
   llvm::sys::path::remove_filename(P); // 'bin'
+  llvm::sys::path::append(P, "lib", "arc");
 
   // 'libarclite' usually lives in the same toolchain as 'clang'. However, the
   // Swift open source toolchains for macOS distribute Clang without libarclite.
   // In that case, to allow the linker to find 'libarclite', we point to the
   // 'libarclite' in the XcodeDefault toolchain instead.
-  if (getXcodeDeveloperPath(P).empty()) {
-if (const Arg *A = Args.getLastArg(options::OPT_isysroot)) {
+  if (!getVFS().exists(P)) {
+auto updatePath = [&](const Arg *A) {
   // Try to infer the path to 'libarclite' in the toolchain from the
   // specified SDK path.
   StringRef XcodePathForSDK = getXcodeDeveloperPath(A->getValue());
-  if (!XcodePathForSDK.empty()) {
-P = XcodePathForSDK;
-llvm::sys::path::append(P, "Toolchains/XcodeDefault.xctoolchain/usr");
-  }
+  if (XcodePathForSDK.empty())
+return false;
+
+  P = XcodePathForSDK;
+  llvm::sys::path::append(P, "Toolchains/XcodeDefault.xctoolchain/usr",
+  "lib", "arc");
+  return getVFS().exists(P);
+};
+
+bool updated = false;
+if (const Arg *A = Args.getLastArg(options::OPT_isysroot))
+  updated = updatePath(A);
+
+if (!updated) {
+  if (const Arg *A = Args.getLastArg(options::OPT__sysroot_EQ))
+updatePath(A);
 }
   }
 
   CmdArgs.push_back("-force_load");
-  llvm::sys::path::append(P, "lib", "arc", "libarclite_");
+  llvm::sys::path::append(P, "libarclite_");
   // Mash in the platform.
   if (isTargetWatchOSSimulator())
 P += "watchsimulator";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125974: [clang] Limit bitcode option ignorelist to Darwin

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

LGTM. Can you add a test for your usecase?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125974/new/

https://reviews.llvm.org/D125974

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

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

Is there any issues currently if we just always use opaque pointer in LTO?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125847/new/

https://reviews.llvm.org/D125847

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

[PATCH] D125847: LTO: Decide upfront whether to use opaque/non-opaque pointer types

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In D125847#3528111 , @aeubanks wrote:

> I'm still looking into a whole program devirtualization bug that only repros 
> with opaque pointers, and there are still potential performance issues to 
> look into

Thanks. Maybe here is another way to look at this problem, instead of giving 
people a temporary switch to use opaque_pointer if their first bitcode is 
no_opaque_pointer, we opt as aggressive as possible into opaque_pointer and 
give people a temporary switch to go back to old behavior just like how clang 
is configured.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125847/new/

https://reviews.llvm.org/D125847

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

[PATCH] D88114: [clang]Test ensuring -fembed-bitcode passed to cc1 captures pre-opt bitcode.

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

I am not sure what exactly is expected here. What is your definition for 
pre-optimized bitcode and how your test case ensures that? Can you explain a 
bit more for context?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88114/new/

https://reviews.llvm.org/D88114

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

[PATCH] D88114: [clang]Test ensuring -fembed-bitcode passed to cc1 captures pre-opt bitcode.

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In D88114#2288737 , @mtrofin wrote:

> In D88114#2288732 , @steven_wu wrote:
>
>> I am not sure what exactly is expected here. What is your definition for 
>> pre-optimized bitcode and how your test case ensures that? Can you explain a 
>> bit more for context?
>
> Pre-optimized meaning before the llvm optimization pipeline is called. That's 
> the current implementation, and the test explicitly checks that the inlining 
> of bar into foo doesn't happen.
>
> I could add an "alwaysinline" to bar, to further stress that.

I think the current implementation does run optimization passes if the input is 
c family language and we need to keep it that way (just so that we don't do 
most of the optimization again). The reason you don't see it running because 
you are using IR as input. For Apple's implementation, we actually pass 
`-disable-llvm-passes` when the input is IR to ensure no optimization passes 
are running.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88114/new/

https://reviews.llvm.org/D88114

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

[PATCH] D88114: [clang]Test ensuring -fembed-bitcode passed to cc1 captures pre-opt bitcode.

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

Ok, I guess we are on the same page. The idea sounds fine to me.

I would suggest just check that the output matches the input file as much as 
possible, rather than just check a label and a call instruction.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88114/new/

https://reviews.llvm.org/D88114

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

[PATCH] D88114: [clang]Test ensuring -fembed-bitcode passed to cc1 captures pre-opt bitcode.

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88114/new/

https://reviews.llvm.org/D88114

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

[PATCH] D102479: [clang][driver] Treat unkonwn -flto= values as -flto

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

I don't think current implementation is the best idea. I think is better to 
make the gcc compatible LTO value to be a clang Driver only option, give them 
an option name, and handle them separately in driver to invoke correct fullLTO 
cc1 command, so that:

- The value remain invalid option for cc1 (CompilerInvocation should just error 
on that, just like now).
- The ignored value should not appear in the help or autocompletion.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102479/new/

https://reviews.llvm.org/D102479

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

[PATCH] D102479: [clang][driver] Treat unkonwn -flto= values as -flto

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D102479/new/

https://reviews.llvm.org/D102479

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

[PATCH] D103547: Don't delete the module you're inspecting

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

Can you create a new test for clang Driver instead of rewrite the cc1 test?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103547/new/

https://reviews.llvm.org/D103547

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

[PATCH] D103547: Don't delete the module you're inspecting

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103547/new/

https://reviews.llvm.org/D103547

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

[PATCH] D93003: [libunwind][ELF] Hide unw_getcontext with LIBUNWIND_HIDE_SYMBOLS

[Steven Wu via Phabricator via cfe-commits]

steven_wu added inline comments.



Comment at: libunwind/src/assembly.h:82
   .globl SYMBOL_NAME(aliasname) SEPARATOR  
\
-  WEAK_SYMBOL(aliasname) SEPARATOR 
\
+  EXPORT_SYMBOL(SYMBOL_NAME(aliasname)) SEPARATOR  
\
   SYMBOL_NAME(aliasname) = SYMBOL_NAME(name)

rprichard wrote:
> compnerd wrote:
> > Does this not change the behaviour on MachO?  This symbol is now 
> > `private_extern` rather than a `weak_reference`.  A weak reference will be 
> > set to 0 by the loader if it is not found, and a `private_extern` is a 
> > strong internal reference.
> Is `.weak_reference` the right directive to use here, instead of 
> `.weak_definition`? We're defining a symbol (`aliasname`) and setting its 
> value to that of another symbol (`name`).
> 
> I think marking `unw_*` weak is intended to let some other strong definition 
> override it. Its value won't ever be set to 0.
> 
> Currently on Mach-O, the hide-symbols flag hides almost everything (including 
> `_Unwind_*`) but leaves all of the `unw_*` alias symbols as extern (and not 
> private-extern) and not weak. With my change, they're still not weak, but 
> they're private-extern.
> 
> libunwind's current assembly.h behavior for a weak alias:
> 
> .globl aliasname
> .weak_reference aliasname
> aliasname = name
> 
> The LLVM Mach-O assembler ignores the `.weak_reference` directive. If I 
> change it to `.weak_definition`, it is still ignored. AFAICT, the LLVM 
> assembler uses the WeakDef/WeakRef attributes from the `name` symbol.
> 
> e.g.
> 
> ```
> $ cat test.S
> .text
> .space 0x42
> 
> // Define foo.
> .globl foo
> foo:
> ret
> 
> // Define a weak alias, bar.
> .globl bar
> .weak_reference bar
> bar = foo
> 
> $ ~/clang11/bin/clang test.S -c && ~/clang11/bin/llvm-readobj --syms test.o
> 
> File: test.o
> Format: Mach-O 64-bit x86-64
> Arch: x86_64
> AddressSize: 64bit
> Symbols [
>   Symbol {
> Name: bar (1)
> Extern
> Type: Section (0xE)
> Section: __text (0x1)
> RefType: UndefinedNonLazy (0x0)
> Flags [ (0x0)
> ]
> Value: 0x42
>   }
>   Symbol {
> Name: foo (5)
> Extern
> Type: Section (0xE)
> Section: __text (0x1)
> RefType: UndefinedNonLazy (0x0)
> Flags [ (0x0)
> ]
> Value: 0x42
>   }
> ]
> ```
> 
> The Flags are empty.
> 
> OTOH, if I flip things around:
> 
> ```
> .text
> .space 0x42
> 
> // Define a weak function, foo.
> .globl foo
> .weak_reference foo
> foo:
> ret
> 
> // Define an alias, bar.
> .globl bar
> bar = foo
> ```
> 
> Now both symbols are WeakRef:
> 
> ```
> File: test.o
> Format: Mach-O 64-bit x86-64
> Arch: x86_64
> AddressSize: 64bit
> Symbols [
>   Symbol {
> Name: bar (1)
> Extern
> Type: Section (0xE)
> Section: __text (0x1)
> RefType: UndefinedNonLazy (0x0)
> Flags [ (0x40)
>   WeakRef (0x40)
> ]
> Value: 0x42
>   }
>   Symbol {
> Name: foo (5)
> Extern
> Type: Section (0xE)
> Section: __text (0x1)
> RefType: UndefinedNonLazy (0x0)
> Flags [ (0x40)
>   WeakRef (0x40)
> ]
> Value: 0x42
>   }
> ]
> ```
> 
> I'm wondering if this LLVM behavior is actually correct, but I'm also 
> unfamiliar with Mach-O. (i.e. We want to copy the symbol's address, but 
> should we be copying the WeakRef/WeakDef flags?) It looks like the behavior 
> is controlled in `MachObjectWriter::writeNlist`. `writeNlist` finds the 
> aliased symbol and uses its flags:
> ```
>   // The Mach-O streamer uses the lowest 16-bits of the flags for the 'desc'
>   // value.
>   bool EncodeAsAltEntry =
> IsAlias && cast(OrigSymbol).isAltEntry();
>   
> W.write(cast(Symbol)->getEncodedFlags(EncodeAsAltEntry));
> ```
> 
> The PrivateExtern attribute, on the other hand, isn't part of these encoded 
> flags:
> ```
>   if (Data.isPrivateExtern())
> Type |= MachO::N_PEXT;
> ```
> `Data` continues to refer to the alias symbol rather than the aliased symbol. 
> However, apparently the author isn't completely sure about things?
> ```
> // FIXME: Should this update Data as well?
> ```
> 
> In libunwind, there is one usage of assembly.h's WEAK_ALIAS. 
> UnwindRegistersSave.S defines a hidden non-weak __unw_getcontext function, 
> and also a weak alias unw_getcontext. My patch's goal is to make 
> unw_getcontext hidden or not-hidden depending on a CMake config variable.
> 
> Currently, on Mach-O and on Windows, `WEAK_ALIAS` doesn't actually make the 
> alias weak. I'm just making it a bit more explicit on Mach-O.
> 
> Alternatively:
>  - Instead of a weak alias, we could output a weak thunk -- a weak function 
> with a branch to the internal hidden symbol. That's more of a functional 
> change, though.
>  - Or, on Mach-O, both the `unw_*` and `__unw_*` functions could be WeakDef.
>  - Maybe the hide-symbols flag sho

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

In D93003#2566906 , @rprichard wrote:

> Maybe this is blocked on someone from Apple reviewing the Mach-O parts?

This is fine with me for how Apple builds libunwind. I am not sure if the open 
source libunwind build for MachO do expect different behavior but I don't think 
there are other reasonable behaviors.

I can give my LGTM if @ldionne has no problem with that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93003/new/

https://reviews.llvm.org/D93003

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

[PATCH] D93003: [libunwind] unw_* alias fixes for ELF and Mach-O

[Steven Wu via Phabricator via cfe-commits]

steven_wu accepted this revision.
steven_wu added a comment.

My main concern is just that weak alias for libc++abi that never worked for 
Darwin :)

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93003/new/

https://reviews.llvm.org/D93003

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