[PATCH] D64232: [analyzer] Prune calls to functions with linear CFGs that return a non-zero constrained value

2019-07-14 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1052
+// to return that every time.
+if (N->getCFG().isLinear())
+  WouldEventBeMeaningless = true;

This time i'd rather go for "has exactly 3 blocks", because we're mostly 
interested in the CFG being "syntactically linear" rather than "semantically 
linear". I.e., if the CFG has an if-statement with a compile-time-constant 
condition, i'd rather show the path, because who knows what does the programmer 
think about this condition.



Comment at: clang/test/Analysis/track-control-dependency-conditions.cpp:185
 return true;
   return coin(); // tracking-note{{Returning value}}
 }

Szelethus wrote:
> This note is meaningful, the bug would not have occurred if `coin()` wasn't 
> assumed to be false.
M. M. Dunno, this is getting complicated very quickly. Say, if you 
replace `return true` with `return false` on the previous line, the note 
becomes meaningless again. I don't see a direct connection between 
meaningfulness and linearness.

The note in this example is relatively meaningful indeed, but i'm not sure it's 
so much meaningful that it's worth the noise. It's still not surprising for me 
that `flipCoin()` occasionally returns false. I do believe that it may be 
sometimes surprising that `flipCoin()` may return false on the *current path*, 
which would make the note meaningful. However, note that we don't prove that it 
may return false, we only push the assumption one step deeper, i.e. put the 
blame on `coin()` instead of `flipCoin()`, but we still fully trust our 
assumption about `coin()` which may have the same problem. And if we had an 
actual proof that it may return false, we would have had a concrete-false 
return value, so the patch wouldn't apply.

I guess some experimental data would be great to have. 



Comment at: clang/test/Analysis/uninit-vals.c:181
 void testUseHalfPoint() {
-  struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-   // expected-note@-1{{Returning from 
'getHalfPoint'}}
-   // expected-note@-2{{'p' initialized here}}
+  struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}}
   use(p); // expected-warning{{uninitialized}}

Szelethus wrote:
> Szelethus wrote:
> > NoQ wrote:
> > > Huh, so there's not even a note in `getHalfPoint()`, just 
> > > calling..returning? This definitely needs some attention from 
> > > `NoStoreFuncVisitor`.
> > > 
> > > Generally, i think this is probably the single place where we do really 
> > > want some info about what happens in `getHalfPoint()`. The report that 
> > > consists only of "p is initialized..." and "...p is uninitialized" is 
> > > pretty weird. Btw, could you write down the full warning text in this 
> > > test? How bad this actually is?
> > Wild idea: `UninitializedObjectChecker` exposes it's main logic through a 
> > header file, how about we use it when we find a read of an uninitialized 
> > region?
> Passed-by-value struct argument contains uninitialized data (e.g., field: 'y')
> 
> Quite good imo.
What specific logic are you talking about? You mean it'd scan the structure for 
uninitialized fields and present the results of the scan to the user in a note?


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

https://reviews.llvm.org/D64232



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

[PATCH] D64283: [PowerPC] Support -mabi=ieeelongdouble and -mabi=ibmlongdouble

2019-07-14 Thread Hal Finkel via Phabricator via cfe-commits 
hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D64283



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

[PATCH] D64283: [PowerPC] Support -mabi=ieeelongdouble and -mabi=ibmlongdouble

2019-07-14 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added a comment.

Woohoo, this is the last :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D64283



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

[PATCH] D64272: [analyzer] Note last writes to a condition only in a nested stackframe

2019-07-14 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Yup, looks good!




Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:146-149
+  ///This is useful, because for non-tracked regions, notes about
+  ///changes to its value in a nested stackframe could be pruned, and
+  ///this visitor can prevent that without polluting the bugpath too
+  ///much.

I definitely don't don't don't don't dislike quadruple negations.


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

https://reviews.llvm.org/D64272



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

[PATCH] D64287: [analyzer] Track the right hand side of the last store regardless of its value

2019-07-14 Thread Artem Dergachev via Phabricator via cfe-commits 
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Yeah, i think this is good to go :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64287



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

[PATCH] D63954: Add lifetime categories attributes

2019-07-14 Thread Matthias Gehre via Phabricator via cfe-commits 
mgehre updated this revision to Diff 209750.
mgehre marked 23 inline comments as done.
mgehre added a comment.

- Don't crash when pretty-printing an Attribute with optional type argument
- Don't crash AST dump when Owner/Pointer has no Type argument
- gsl::Owner/gsl::Pointer: Make DerefType parameter optional
- Unify type and non-type attribute parsing; don't add OwnerAttr/PointerAttr 
twice
- Improve documentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63954

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaCXX/attr-gsl-owner-pointer-parsing.cpp
  clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -303,6 +303,8 @@
 std::string getIsOmitted() const override {
   if (type == "IdentifierInfo *")
 return "!get" + getUpperName().str() + "()";
+  if (type == "TypeSourceInfo *")
+return "!get" + getUpperName().str() + "Loc()";
   if (type == "ParamIdx")
 return "!get" + getUpperName().str() + "().isValid()";
   return "false";
@@ -336,6 +338,8 @@
<< "  OS << \" \" << SA->get" << getUpperName()
<< "()->getName();\n";
   } else if (type == "TypeSourceInfo *") {
+if (isOptional())
+  OS << "if (SA->get" << getUpperName() << "Loc())";
 OS << "OS << \" \" << SA->get" << getUpperName()
<< "().getAsString();\n";
   } else if (type == "bool") {
Index: clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-gsl-owner-pointer.cpp
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -ast-dump %s | \
+// RUN: FileCheck --implicit-check-not OwnerAttr --implicit-check-not PointerAttr %s
+
+class [[gsl::Owner]] OwnerMissingParameter{};
+// CHECK: CXXRecordDecl {{.*}} OwnerMissingParameter
+// CHECK: OwnerAttr
+
+class [[gsl::Pointer]] PointerMissingParameter{};
+// CHECK: CXXRecordDecl {{.*}} PointerMissingParameter
+// CHECK: PointerAttr
+
+class [[gsl::Owner()]] OwnerWithEmptyParameterList{};
+// CHECK: CXXRecordDecl {{.*}} OwnerWithEmptyParameterList
+// CHECK: OwnerAttr {{.*}}
+
+class [[gsl::Pointer()]] PointerWithEmptyParameterList{};
+// CHECK: CXXRecordDecl {{.*}} PointerWithEmptyParameterList
+// CHECK: PointerAttr {{.*}}
+
+class [[gsl::Owner(int)]] AnOwner{};
+// CHECK: CXXRecordDecl {{.*}} AnOwner
+// CHECK: OwnerAttr {{.*}} int
+
+struct S;
+class [[gsl::Pointer(S)]] APointer{};
+// CHECK: CXXRecordDecl {{.*}} APointer
+// CHECK: PointerAttr {{.*}} S
+
+class [[gsl::Owner(int)]] [[gsl::Owner(int)]] DuplicateOwner{};
+// CHECK: CXXRecordDecl {{.*}} DuplicateOwner
+// CHECK: OwnerAttr {{.*}} int
+
+class [[gsl::Pointer(int)]] [[gsl::Pointer(int)]] DuplicatePointer{};
+// CHECK: CXXRecordDecl {{.*}} DuplicatePointer
+// CHECK: PointerAttr {{.*}} int
+
+class [[gsl::Owner(int)]] AddTheSameLater{};
+// CHECK: CXXRecordDecl {{.*}} AddTheSameLater
+// CHECK: OwnerAttr {{.*}} int
+
+class [[gsl::Owner(int)]] AddTheSameLater;
+// CHECK: CXXRecordDecl {{.*}} prev {{.*}} AddTheSameLater
+// CHECK: OwnerAttr {{.*}} int
Index: clang/test/SemaCXX/attr-gsl-owner-pointer-parsing.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-gsl-owner-pointer-parsing.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int [[gsl::Owner]] i;
+// expected-error@-1 {{'Owner' attribute cannot be applied to types}}
+void [[gsl::Owner]] f();
+// expected-error@-1 {{'Owner' attribute cannot be applied to types}}
+
+[[gsl::Owner]] void f();
+// expected-warning@-1 {{'Owner' attribute only applies to classes}}
+
+struct S {
+};
+
+S [[gsl::Owner]] Instance;
+// expected-error@-1 {{'Owner' attribute cannot be applied to types}}
+
+class [[gsl::Owner(7)]] OwnerDerefNoType{};
+// expected-error@-1 {{expected a type}}
+
+class [[gsl::Pointer("int")]] PointerDerefNoType{};
+// expected-error@-1 {{expected a type}}
+
+class [[gsl::Owner(int)]] [[gsl::Pointer(int)]] BothOwnerPointer{};
+// expected-error@-1 {{'Pointer' and 'Owner' attributes are not compatible}}
+// expected-note@-2 {{conflicting attribute is here}}
+
+class [[gsl::Owner(void)]] OwnerVoidDerefType{};
+// expected-error@-1 {{'void' is an invalid argument to attribute 'Owner'}}
+class [[gsl::Pointer(void)]] PointerVoidDerefType{};
+// expected-error@-1 {{'void' is an invalid argument to attribute

[PATCH] D63954: Add lifetime categories attributes

2019-07-14 Thread Matthias Gehre via Phabricator via cfe-commits 
mgehre added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:2770
+def Owner : InheritableAttr {
+  let Spellings = [CXX11<"gsl", "Owner">];
+  let Subjects = SubjectList<[CXXRecord]>;

aaron.ballman wrote:
> xazax.hun wrote:
> > aaron.ballman wrote:
> > > xazax.hun wrote:
> > > > aaron.ballman wrote:
> > > > > Has Microsoft already added support for this? We had an unfortunate 
> > > > > problem with `gsl::suppress` where we introduced the attribute first 
> > > > > and MSVC introduced it under a different, incompatible syntax. I 
> > > > > would strongly like to avoid repeating that.
> > > > I will double check this. Her Sutter's paper have this spelling and I 
> > > > believe that the Microsoft implementation is following the same 
> > > > spelling. The main difference is that the MSVC version does not have a 
> > > > type argument at this point but they do plan to add it as an optional 
> > > > argument. (They want to infer the pointee type when it is not 
> > > > provided.) The clang community do not want the inference, hence we made 
> > > > the type argument required. Always providing the type argument should 
> > > > be compatible with future versions of the MSVC implementation.
> > > I take this to mean that we're not implementing the same attribute as 
> > > MSVC is, and that worries me.
> > > 
> > > > Always providing the type argument should be compatible with future 
> > > > versions of the MSVC implementation.
> > > 
> > > The problem is that code written for MSVC (where the type argument is not 
> > > required) will not compile with Clang (where the type argument is 
> > > required), if I understand properly.
> > > 
> > > Generally speaking, when we implement an attribute from another vendor 
> > > namespace, we expect the attribute to have the same semantics as whoever 
> > > "owns" that vendor namespace. It's a bit trickier here because `gsl` 
> > > isn't really a vendor so much as a collective, so I don't know who is the 
> > > authority to turn to.
> > > 
> > > On a completely different note: would this attribute also make sense 
> > > within C with a C2x spelling?
> > I see, that is a valid concern. A followup patch makes the type argument 
> > optional so it will be fully compatible with MSVC.  I don't think it makes 
> > sense with C or at least I am not sure what would I annotate using these 
> > attributes in C code as the subjects are classes. I know that it is 
> > possible to simulate classes in C but I am not sure how well the analysis 
> > would work in such cases. 
> > 
> > Are you OK with making the argument optional in a followup patch or do you 
> > want us to cherry-pick that modification into this one?
> > I don't think it makes sense with C or at least I am not sure what would I 
> > annotate using these attributes in C code as the subjects are classes. I 
> > know that it is possible to simulate classes in C but I am not sure how 
> > well the analysis would work in such cases.
> 
> I couldn't think of a reason to expose this in C either, so I think the 
> current spelling is good as-is.
> 
> > Are you OK with making the argument optional in a followup patch or do you 
> > want us to cherry-pick that modification into this one?
> 
> I'd prefer the argument be optional in this patch.
The argument is now optional.



Comment at: clang/include/clang/Basic/AttrDocs.td:4164
+  let Content = [{
+When annotating a class ``O`` with ``[[gsl::Owner(T)]]``, then each function
+that returns cv-qualified ``T&`` or ``T*`` is assumed to return a

gribozavr wrote:
> Slightly better: "In a class annotated with ..., each function that returns 
> ... is assumed to ..."
> 
> However, the "is assumed" part throws me off. There's no assumption here -- 
> this is what this attribute means, intentionally, by design, no guessing 
> involved.
> 
> So how about this?
> 
> The attribute `[[gsl::Owner(T)]]` applies to structs and classes that own an 
> object of type `T`:
> 
> ```
> [[gsl::Owner(int)]] class IntOwner {
> private:
>   int value;
> public:
>   int *getInt() { return  }
>   long *getLong();
> };
> ```
> 
> In a class annotated like this each member function that returns `T&` or `T*` 
> returns a pointer/reference to the data owned by the class instance:
> 
> ```
> IntOwner int_owner;
> int *x = int_owner.getInt();
> // `x` points to memory owned by `int_owner`.
> ```
> 
> Therefore, that the lifetime of that data ends when ... For example:
> 
> ```
> int *x = IntOwner().getInt();
> // `x` points to memory owned by the temporary `IntOwner()`, which has been 
> destroyed now.
> 
> long *y = IntOwner().getLong();
> // Can't make any conclusion about validity of `y`.
> ```
> 
What I meant to express by "is assumed to" is that this attribute does not 
change what member functions do. E.g. if I had `int* f() { static int i; return 
 }`, then `f` would not return data owned by the class instance, even when

[PATCH] D64232: [analyzer] Prune calls to functions with linear CFGs that return a non-zero constrained value

2019-07-14 Thread Kristóf Umann via Phabricator via cfe-commits 
Szelethus marked an inline comment as done.
Szelethus added inline comments.



Comment at: clang/test/Analysis/uninit-vals.c:181
 void testUseHalfPoint() {
-  struct Point p = getHalfPoint(); // expected-note{{Calling 'getHalfPoint'}}
-   // expected-note@-1{{Returning from 
'getHalfPoint'}}
-   // expected-note@-2{{'p' initialized here}}
+  struct Point p = getHalfPoint(); // expected-note{{'p' initialized here}}
   use(p); // expected-warning{{uninitialized}}

Szelethus wrote:
> NoQ wrote:
> > Huh, so there's not even a note in `getHalfPoint()`, just 
> > calling..returning? This definitely needs some attention from 
> > `NoStoreFuncVisitor`.
> > 
> > Generally, i think this is probably the single place where we do really 
> > want some info about what happens in `getHalfPoint()`. The report that 
> > consists only of "p is initialized..." and "...p is uninitialized" is 
> > pretty weird. Btw, could you write down the full warning text in this test? 
> > How bad this actually is?
> Wild idea: `UninitializedObjectChecker` exposes it's main logic through a 
> header file, how about we use it when we find a read of an uninitialized 
> region?
Passed-by-value struct argument contains uninitialized data (e.g., field: 'y')

Quite good imo.


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

https://reviews.llvm.org/D64232



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

[PATCH] D64048: [TargetParser][ARM] Account dependencies when processing target features

2019-07-14 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL366031: [TargetParser][ARM] Account dependencies when 
processing target features (authored by alelab01, committed by ).
Herald added a subscriber: kristina.

Changed prior to commit:
  https://reviews.llvm.org/D64048?vs=208010=209748#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64048

Files:
  cfe/trunk/test/Preprocessor/arm-target-features.c
  llvm/trunk/include/llvm/Support/ARMTargetParser.def
  llvm/trunk/lib/Support/ARMTargetParser.cpp

Index: llvm/trunk/lib/Support/ARMTargetParser.cpp
===
--- llvm/trunk/lib/Support/ARMTargetParser.cpp
+++ llvm/trunk/lib/Support/ARMTargetParser.cpp
@@ -490,16 +490,30 @@
   return ARM::FK_INVALID;
 }
 
+static unsigned getAEKID(StringRef ArchExtName) {
+  for (const auto AE : ARM::ARCHExtNames)
+if (AE.getName() == ArchExtName)
+  return AE.ID;
+  return ARM::AEK_INVALID;
+}
+
 bool ARM::appendArchExtFeatures(
   StringRef CPU, ARM::ArchKind AK, StringRef ArchExt,
   std::vector ) {
-  StringRef StandardFeature = getArchExtFeature(ArchExt);
-  if (!StandardFeature.empty()) {
-Features.push_back(StandardFeature);
-return true;
-  }
 
+  size_t StartingNumFeatures = Features.size();
   const bool Negated = stripNegationPrefix(ArchExt);
+  unsigned ID = getAEKID(ArchExt);
+
+  if (ID == AEK_INVALID)
+return false;
+
+  for (const auto AE : ARCHExtNames) {
+if (Negated && (AE.ID & ID) == ID && AE.NegFeature)
+  Features.push_back(AE.NegFeature);
+else if (AE.ID == ID && AE.Feature)
+  Features.push_back(AE.Feature);
+  }
 
   if (CPU == "")
 CPU = "generic";
@@ -519,7 +533,7 @@
 }
 return ARM::getFPUFeatures(FPUKind, Features);
   }
-  return false;
+  return StartingNumFeatures != Features.size();
 }
 
 StringRef ARM::getHWDivName(unsigned HWDivKind) {
Index: llvm/trunk/include/llvm/Support/ARMTargetParser.def
===
--- llvm/trunk/include/llvm/Support/ARMTargetParser.def
+++ llvm/trunk/include/llvm/Support/ARMTargetParser.def
@@ -149,8 +149,8 @@
 ARM_ARCH_EXT_NAME("dsp",  ARM::AEK_DSP,  "+dsp",   "-dsp")
 ARM_ARCH_EXT_NAME("fp",   ARM::AEK_FP,   nullptr,  nullptr)
 ARM_ARCH_EXT_NAME("fp.dp",ARM::AEK_FP_DP,nullptr,  nullptr)
-ARM_ARCH_EXT_NAME("mve",  ARM::AEK_SIMD, "+mve",   "-mve")
-ARM_ARCH_EXT_NAME("mve.fp",   (ARM::AEK_SIMD | ARM::AEK_FP), "+mve.fp", "-mve.fp")
+ARM_ARCH_EXT_NAME("mve", (ARM::AEK_DSP | ARM::AEK_SIMD), "+mve", "-mve")
+ARM_ARCH_EXT_NAME("mve.fp",  (ARM::AEK_DSP | ARM::AEK_SIMD | ARM::AEK_FP), "+mve.fp", "-mve.fp")
 ARM_ARCH_EXT_NAME("idiv", (ARM::AEK_HWDIVARM | ARM::AEK_HWDIVTHUMB), nullptr, nullptr)
 ARM_ARCH_EXT_NAME("mp",   ARM::AEK_MP,   nullptr,  nullptr)
 ARM_ARCH_EXT_NAME("simd", ARM::AEK_SIMD, nullptr,  nullptr)
Index: cfe/trunk/test/Preprocessor/arm-target-features.c
===
--- cfe/trunk/test/Preprocessor/arm-target-features.c
+++ cfe/trunk/test/Preprocessor/arm-target-features.c
@@ -762,12 +762,29 @@
 // CHECK-V81M-MVE: #define __ARM_FEATURE_MVE 1
 // CHECK-V81M-MVE: #define __ARM_FEATURE_SIMD32 1
 
-// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE-FP %s
-// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_DSP 1
-// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
-// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_MVE 3
-// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_SIMD32 1
-// CHECK-V81M-MVE-FP: #define __ARM_FPV5__ 1
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVEFP %s
+// CHECK-V81M-MVEFP: #define __ARM_FEATURE_DSP 1
+// CHECK-V81M-MVEFP: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
+// CHECK-V81M-MVEFP: #define __ARM_FEATURE_MVE 3
+// CHECK-V81M-MVEFP: #define __ARM_FEATURE_SIMD32 1
+// CHECK-V81M-MVEFP: #define __ARM_FPV5__ 1
+
+// nofp discards mve.fp
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp+nofp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVEFP-NOFP %s
+// CHECK-V81M-MVEFP-NOFP-NOT: #define __ARM_FEATURE_MVE
+
+// nomve discards mve.fp
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp+nomve -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVEFP-NOMVE %s
+// CHECK-V81M-MVEFP-NOMVE-NOT: #define __ARM_FEATURE_MVE
+
+// mve+fp doesn't imply mve.fp
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve+fp -x c -E -dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE-FP %s
+// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_MVE 1
+
+// nodsp discards both

r366031 - [TargetParser][ARM] Account dependencies when processing target features

2019-07-14 Thread Alexandros Lamprineas via cfe-commits 
Author: alelab01
Date: Sun Jul 14 13:31:15 2019
New Revision: 366031

URL: http://llvm.org/viewvc/llvm-project?rev=366031=rev
Log:
[TargetParser][ARM] Account dependencies when processing target features

Teaches ARM::appendArchExtFeatures to account dependencies when processing
target features: i.e. when you say -march=armv8.1-m.main+mve.fp+nofp it
means mve.fp should get discarded too. (Split from D63936)

Differential Revision: https://reviews.llvm.org/D64048

Modified:
cfe/trunk/test/Preprocessor/arm-target-features.c

Modified: cfe/trunk/test/Preprocessor/arm-target-features.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/arm-target-features.c?rev=366031=366030=366031=diff
==
--- cfe/trunk/test/Preprocessor/arm-target-features.c (original)
+++ cfe/trunk/test/Preprocessor/arm-target-features.c Sun Jul 14 13:31:15 2019
@@ -762,12 +762,29 @@
 // CHECK-V81M-MVE: #define __ARM_FEATURE_MVE 1
 // CHECK-V81M-MVE: #define __ARM_FEATURE_SIMD32 1
 
-// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp -x c -E 
-dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE-FP %s
-// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_DSP 1
-// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
-// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_MVE 3
-// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_SIMD32 1
-// CHECK-V81M-MVE-FP: #define __ARM_FPV5__ 1
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp -x c -E 
-dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVEFP %s
+// CHECK-V81M-MVEFP: #define __ARM_FEATURE_DSP 1
+// CHECK-V81M-MVEFP: #define __ARM_FEATURE_FP16_SCALAR_ARITHMETIC 1
+// CHECK-V81M-MVEFP: #define __ARM_FEATURE_MVE 3
+// CHECK-V81M-MVEFP: #define __ARM_FEATURE_SIMD32 1
+// CHECK-V81M-MVEFP: #define __ARM_FPV5__ 1
+
+// nofp discards mve.fp
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp+nofp -x 
c -E -dM %s -o - | FileCheck -match-full-lines 
--check-prefix=CHECK-V81M-MVEFP-NOFP %s
+// CHECK-V81M-MVEFP-NOFP-NOT: #define __ARM_FEATURE_MVE
+
+// nomve discards mve.fp
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve.fp+nomve -x 
c -E -dM %s -o - | FileCheck -match-full-lines 
--check-prefix=CHECK-V81M-MVEFP-NOMVE %s
+// CHECK-V81M-MVEFP-NOMVE-NOT: #define __ARM_FEATURE_MVE
+
+// mve+fp doesn't imply mve.fp
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve+fp -x c -E 
-dM %s -o - | FileCheck -match-full-lines --check-prefix=CHECK-V81M-MVE-FP %s
+// CHECK-V81M-MVE-FP: #define __ARM_FEATURE_MVE 1
+
+// nodsp discards both dsp and mve
+// RUN: %clang -target arm-arm-none-eabi -march=armv8.1-m.main+mve+nodsp -x c 
-E -dM %s -o - | FileCheck -match-full-lines 
--check-prefix=CHECK-V81M-MVE-NODSP %s
+// CHECK-V81M-MVE-NODSP-NOT: #define __ARM_FEATURE_MVE
+// CHECK-V81M-MVE-NODSP-NOT: #define __ARM_FEATURE_DSP
 
 // RUN: %clang -target armv8.1a-none-none-eabi -x c -E -dM %s -o - | FileCheck 
-match-full-lines --check-prefix=CHECK-V81A %s
 // CHECK-V81A: #define __ARM_ARCH 8


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

r366029 - Fix uninitialized variable analyzer warning. NFCI.

2019-07-14 Thread Simon Pilgrim via cfe-commits 
Author: rksimon
Date: Sun Jul 14 12:13:09 2019
New Revision: 366029

URL: http://llvm.org/viewvc/llvm-project?rev=366029=rev
Log:
Fix uninitialized variable analyzer warning. NFCI.

Modified:
cfe/trunk/lib/Sema/SemaDecl.cpp

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=366029=366028=366029=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Sun Jul 14 12:13:09 2019
@@ -12192,7 +12192,7 @@ void Sema::CheckCompleteVariableDeclarat
 
   // Cache the result of checking for constant initialization.
   Optional CacheHasConstInit;
-  const Expr *CacheCulprit;
+  const Expr *CacheCulprit = nullptr;
   auto checkConstInit = [&]() mutable {
 if (!CacheHasConstInit)
   CacheHasConstInit = var->getInit()->isConstantInitializer(


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

[PATCH] D64676: Support __seg_fs and __seg_gs on x86

2019-07-14 Thread JF Bastien via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL366028: Support __seg_fs and __seg_gs on x86 (authored by 
jfb, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64676?vs=209645=209739#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64676

Files:
  cfe/trunk/docs/LanguageExtensions.rst
  cfe/trunk/lib/Basic/Targets/X86.cpp
  cfe/trunk/test/Preprocessor/x86_seg_fs_gs.c


Index: cfe/trunk/lib/Basic/Targets/X86.cpp
===
--- cfe/trunk/lib/Basic/Targets/X86.cpp
+++ cfe/trunk/lib/Basic/Targets/X86.cpp
@@ -917,6 +917,11 @@
 DefineStd(Builder, "i386", Opts);
   }
 
+  Builder.defineMacro("__SEG_GS");
+  Builder.defineMacro("__SEG_FS");
+  Builder.defineMacro("__seg_gs", "__attribute__((address_space(256)))");
+  Builder.defineMacro("__seg_fs", "__attribute__((address_space(257)))");
+
   // Subtarget options.
   // FIXME: We are hard-coding the tune parameters based on the CPU, but they
   // truly should be based on -mtune options.
Index: cfe/trunk/docs/LanguageExtensions.rst
===
--- cfe/trunk/docs/LanguageExtensions.rst
+++ cfe/trunk/docs/LanguageExtensions.rst
@@ -2465,6 +2465,10 @@
   movl%gs:(%eax), %eax
   ret
 
+You can also use the GCC compatibility macros ``__seg_fs`` and ``__seg_gs`` for
+the same purpose. The preprocessor symbols ``__SEG_FS`` and ``__SEG_GS``
+indicate their support.
+
 PowerPC Language Extensions
 --
 
Index: cfe/trunk/test/Preprocessor/x86_seg_fs_gs.c
===
--- cfe/trunk/test/Preprocessor/x86_seg_fs_gs.c
+++ cfe/trunk/test/Preprocessor/x86_seg_fs_gs.c
@@ -0,0 +1,7 @@
+// RUN: %clang -target i386-unknown-unknown -x c -E -dM -o - %s | FileCheck 
-match-full-lines %s
+// RUN: %clang -target x86_64-unknown-unknown -x c -E -dM -o - %s | FileCheck 
-match-full-lines %s
+
+// CHECK: #define __SEG_FS 1
+// CHECK: #define __SEG_GS 1
+// CHECK: #define __seg_fs __attribute__((address_space(257)))
+// CHECK: #define __seg_gs __attribute__((address_space(256)))


Index: cfe/trunk/lib/Basic/Targets/X86.cpp
===
--- cfe/trunk/lib/Basic/Targets/X86.cpp
+++ cfe/trunk/lib/Basic/Targets/X86.cpp
@@ -917,6 +917,11 @@
 DefineStd(Builder, "i386", Opts);
   }
 
+  Builder.defineMacro("__SEG_GS");
+  Builder.defineMacro("__SEG_FS");
+  Builder.defineMacro("__seg_gs", "__attribute__((address_space(256)))");
+  Builder.defineMacro("__seg_fs", "__attribute__((address_space(257)))");
+
   // Subtarget options.
   // FIXME: We are hard-coding the tune parameters based on the CPU, but they
   // truly should be based on -mtune options.
Index: cfe/trunk/docs/LanguageExtensions.rst
===
--- cfe/trunk/docs/LanguageExtensions.rst
+++ cfe/trunk/docs/LanguageExtensions.rst
@@ -2465,6 +2465,10 @@
   movl%gs:(%eax), %eax
   ret
 
+You can also use the GCC compatibility macros ``__seg_fs`` and ``__seg_gs`` for
+the same purpose. The preprocessor symbols ``__SEG_FS`` and ``__SEG_GS``
+indicate their support.
+
 PowerPC Language Extensions
 --
 
Index: cfe/trunk/test/Preprocessor/x86_seg_fs_gs.c
===
--- cfe/trunk/test/Preprocessor/x86_seg_fs_gs.c
+++ cfe/trunk/test/Preprocessor/x86_seg_fs_gs.c
@@ -0,0 +1,7 @@
+// RUN: %clang -target i386-unknown-unknown -x c -E -dM -o - %s | FileCheck -match-full-lines %s
+// RUN: %clang -target x86_64-unknown-unknown -x c -E -dM -o - %s | FileCheck -match-full-lines %s
+
+// CHECK: #define __SEG_FS 1
+// CHECK: #define __SEG_GS 1
+// CHECK: #define __seg_fs __attribute__((address_space(257)))
+// CHECK: #define __seg_gs __attribute__((address_space(256)))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63936: [clang][Driver][ARM] Favor -mfpu over default CPU features

2019-07-14 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL366027: [clang][Driver][ARM] Favor -mfpu over default CPU 
features (authored by alelab01, committed by ).
Herald added a subscriber: kristina.

Changed prior to commit:
  https://reviews.llvm.org/D63936?vs=207436=209738#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63936

Files:
  cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
  cfe/trunk/test/CodeGen/arm-target-features.c
  llvm/trunk/include/llvm/Support/ARMTargetParser.def
  llvm/trunk/lib/Support/ARMTargetParser.cpp
  llvm/trunk/unittests/Support/TargetParserTest.cpp

Index: cfe/trunk/test/CodeGen/arm-target-features.c
===
--- cfe/trunk/test/CodeGen/arm-target-features.c
+++ cfe/trunk/test/CodeGen/arm-target-features.c
@@ -32,7 +32,7 @@
 
 // RUN: %clang_cc1 -triple thumbv8-linux-gnueabihf -target-cpu exynos-m4 -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-BASIC-V82
 // RUN: %clang_cc1 -triple thumbv8-linux-gnueabihf -target-cpu exynos-m5 -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-BASIC-V82
-// CHECK-BASIC-V82: "target-features"="+armv8.2-a,+crc,+crypto,+d32,+dotprod,+dsp,+fp-armv8,+fp-armv8d16,+fp-armv8d16sp,+fp-armv8sp,+fp16,+fp64,+fpregs,+hwdiv,+hwdiv-arm,+neon,+ras,+thumb-mode,+vfp2,+vfp2d16,+vfp2d16sp,+vfp2sp,+vfp3,+vfp3d16,+vfp3d16sp,+vfp3sp,+vfp4,+vfp4d16,+vfp4d16sp,+vfp4sp"
+// CHECK-BASIC-V82: "target-features"="+armv8.2-a,+crc,+crypto,+d32,+dotprod,+dsp,+fp-armv8,+fp-armv8d16,+fp-armv8d16sp,+fp-armv8sp,+fp16,+fp64,+fpregs,+fullfp16,+hwdiv,+hwdiv-arm,+neon,+ras,+thumb-mode,+vfp2,+vfp2d16,+vfp2d16sp,+vfp2sp,+vfp3,+vfp3d16,+vfp3d16sp,+vfp3sp,+vfp4,+vfp4d16,+vfp4d16sp,+vfp4sp"
 
 // RUN: %clang_cc1 -triple armv8-linux-gnueabi -target-cpu cortex-a53 -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-BASIC-V8-ARM
 // CHECK-BASIC-V8-ARM: "target-features"="+armv8-a,+crc,+crypto,+d32,+dsp,+fp-armv8,+fp-armv8d16,+fp-armv8d16sp,+fp-armv8sp,+fp16,+fp64,+fpregs,+hwdiv,+hwdiv-arm,+neon,+vfp2,+vfp2d16,+vfp2d16sp,+vfp2sp,+vfp3,+vfp3d16,+vfp3d16sp,+vfp3sp,+vfp4,+vfp4d16,+vfp4d16sp,+vfp4sp,-thumb-mode"
Index: cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -376,7 +376,11 @@
 Features.push_back(
 Args.MakeArgString((F.second ? "+" : "-") + F.first()));
   } else if (!CPUName.empty()) {
-DecodeARMFeaturesFromCPU(D, CPUName, ExtensionFeatures);
+// This sets the default features for the specified CPU. We certainly don't
+// want to override the features that have been explicitly specified on the
+// command line. Therefore, process them directly instead of appending them
+// at the end later.
+DecodeARMFeaturesFromCPU(D, CPUName, Features);
   }
 
   if (CPUArg)
Index: llvm/trunk/lib/Support/ARMTargetParser.cpp
===
--- llvm/trunk/lib/Support/ARMTargetParser.cpp
+++ llvm/trunk/lib/Support/ARMTargetParser.cpp
@@ -409,30 +409,12 @@
   if (Extensions == AEK_INVALID)
 return false;
 
-  if (Extensions & AEK_CRC)
-Features.push_back("+crc");
-  else
-Features.push_back("-crc");
-
-  if (Extensions & AEK_DSP)
-Features.push_back("+dsp");
-  else
-Features.push_back("-dsp");
-
-  if (Extensions & AEK_FP16FML)
-Features.push_back("+fp16fml");
-  else
-Features.push_back("-fp16fml");
-
-  if (Extensions & AEK_RAS)
-Features.push_back("+ras");
-  else
-Features.push_back("-ras");
-
-  if (Extensions & AEK_DOTPROD)
-Features.push_back("+dotprod");
-  else
-Features.push_back("-dotprod");
+  for (const auto AE : ARCHExtNames) {
+if ((Extensions & AE.ID) == AE.ID && AE.Feature)
+  Features.push_back(AE.Feature);
+else if (AE.NegFeature)
+  Features.push_back(AE.NegFeature);
+  }
 
   return getHWDivFeatures(Extensions, Features);
 }
Index: llvm/trunk/unittests/Support/TargetParserTest.cpp
===
--- llvm/trunk/unittests/Support/TargetParserTest.cpp
+++ llvm/trunk/unittests/Support/TargetParserTest.cpp
@@ -571,17 +571,18 @@
 TEST(TargetParserTest, ARMExtensionFeatures) {
   std::map> Extensions;
 
-  Extensions[ARM::AEK_CRC]= { "+crc",   "-crc" };
-  Extensions[ARM::AEK_DSP]= { "+dsp",   "-dsp" };
+  for (auto  : ARM::ARCHExtNames) {
+if (Ext.Feature && Ext.NegFeature)
+  Extensions[Ext.ID] = { StringRef(Ext.Feature),
+ StringRef(Ext.NegFeature) };
+  }
+
   Extensions[ARM::AEK_HWDIVARM]   = { "+hwdiv-arm", "-hwdiv-arm" };
   Extensions[ARM::AEK_HWDIVTHUMB] = { "+hwdiv", "-hwdiv" };
-  Extensions[ARM::AEK_RAS]= { "+ras",   "-ras" };
-  Extensions[ARM::AEK_FP16FML]

r366028 - Support __seg_fs and __seg_gs on x86

2019-07-14 Thread JF Bastien via cfe-commits 
Author: jfb
Date: Sun Jul 14 11:33:51 2019
New Revision: 366028

URL: http://llvm.org/viewvc/llvm-project?rev=366028=rev
Log:
Support __seg_fs and __seg_gs on x86

Summary:
GCC supports named address spaces macros:
  https://gcc.gnu.org/onlinedocs/gcc/Named-Address-Spaces.html

clang does as well with address spaces:
  
https://clang.llvm.org/docs/LanguageExtensions.html#memory-references-to-specified-segments

Add the __seg_fs and __seg_gs macros for compatibility with GCC.



Subscribers: jkorous, dexonsmith, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D64676

Added:
cfe/trunk/test/Preprocessor/x86_seg_fs_gs.c
Modified:
cfe/trunk/docs/LanguageExtensions.rst
cfe/trunk/lib/Basic/Targets/X86.cpp

Modified: cfe/trunk/docs/LanguageExtensions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LanguageExtensions.rst?rev=366028=366027=366028=diff
==
--- cfe/trunk/docs/LanguageExtensions.rst (original)
+++ cfe/trunk/docs/LanguageExtensions.rst Sun Jul 14 11:33:51 2019
@@ -2465,6 +2465,10 @@ Which compiles to (on X86-32):
   movl%gs:(%eax), %eax
   ret
 
+You can also use the GCC compatibility macros ``__seg_fs`` and ``__seg_gs`` for
+the same purpose. The preprocessor symbols ``__SEG_FS`` and ``__SEG_GS``
+indicate their support.
+
 PowerPC Language Extensions
 --
 

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=366028=366027=366028=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Sun Jul 14 11:33:51 2019
@@ -917,6 +917,11 @@ void X86TargetInfo::getTargetDefines(con
 DefineStd(Builder, "i386", Opts);
   }
 
+  Builder.defineMacro("__SEG_GS");
+  Builder.defineMacro("__SEG_FS");
+  Builder.defineMacro("__seg_gs", "__attribute__((address_space(256)))");
+  Builder.defineMacro("__seg_fs", "__attribute__((address_space(257)))");
+
   // Subtarget options.
   // FIXME: We are hard-coding the tune parameters based on the CPU, but they
   // truly should be based on -mtune options.

Added: cfe/trunk/test/Preprocessor/x86_seg_fs_gs.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/x86_seg_fs_gs.c?rev=366028=auto
==
--- cfe/trunk/test/Preprocessor/x86_seg_fs_gs.c (added)
+++ cfe/trunk/test/Preprocessor/x86_seg_fs_gs.c Sun Jul 14 11:33:51 2019
@@ -0,0 +1,7 @@
+// RUN: %clang -target i386-unknown-unknown -x c -E -dM -o - %s | FileCheck 
-match-full-lines %s
+// RUN: %clang -target x86_64-unknown-unknown -x c -E -dM -o - %s | FileCheck 
-match-full-lines %s
+
+// CHECK: #define __SEG_FS 1
+// CHECK: #define __SEG_GS 1
+// CHECK: #define __seg_fs __attribute__((address_space(257)))
+// CHECK: #define __seg_gs __attribute__((address_space(256)))


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

r366027 - [clang][Driver][ARM] Favor -mfpu over default CPU features

2019-07-14 Thread Alexandros Lamprineas via cfe-commits 
Author: alelab01
Date: Sun Jul 14 11:32:42 2019
New Revision: 366027

URL: http://llvm.org/viewvc/llvm-project?rev=366027=rev
Log:
[clang][Driver][ARM] Favor -mfpu over default CPU features

When processing the command line options march, mcpu and mfpu, we store
the implied target features on a vector. The change D62998 introduced a
temporary vector, where the processed features get accumulated. When
calling DecodeARMFeaturesFromCPU, which sets the default features for
the specified CPU, we certainly don't want to override the features
that have been explicitly specified on the command line. Therefore, the
default features should appear first in the final vector. This problem
became evident once I added the missing (unhandled) target features in
ARM::getExtensionFeatures.

Differential Revision: https://reviews.llvm.org/D63936

Modified:
cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
cfe/trunk/test/CodeGen/arm-target-features.c

Modified: cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp?rev=366027=366026=366027=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Arch/ARM.cpp Sun Jul 14 11:32:42 2019
@@ -376,7 +376,11 @@ void arm::getARMTargetFeatures(const Too
 Features.push_back(
 Args.MakeArgString((F.second ? "+" : "-") + F.first()));
   } else if (!CPUName.empty()) {
-DecodeARMFeaturesFromCPU(D, CPUName, ExtensionFeatures);
+// This sets the default features for the specified CPU. We certainly don't
+// want to override the features that have been explicitly specified on the
+// command line. Therefore, process them directly instead of appending them
+// at the end later.
+DecodeARMFeaturesFromCPU(D, CPUName, Features);
   }
 
   if (CPUArg)

Modified: cfe/trunk/test/CodeGen/arm-target-features.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/arm-target-features.c?rev=366027=366026=366027=diff
==
--- cfe/trunk/test/CodeGen/arm-target-features.c (original)
+++ cfe/trunk/test/CodeGen/arm-target-features.c Sun Jul 14 11:32:42 2019
@@ -32,7 +32,7 @@
 
 // RUN: %clang_cc1 -triple thumbv8-linux-gnueabihf -target-cpu exynos-m4 
-emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-BASIC-V82
 // RUN: %clang_cc1 -triple thumbv8-linux-gnueabihf -target-cpu exynos-m5 
-emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-BASIC-V82
-// CHECK-BASIC-V82: 
"target-features"="+armv8.2-a,+crc,+crypto,+d32,+dotprod,+dsp,+fp-armv8,+fp-armv8d16,+fp-armv8d16sp,+fp-armv8sp,+fp16,+fp64,+fpregs,+hwdiv,+hwdiv-arm,+neon,+ras,+thumb-mode,+vfp2,+vfp2d16,+vfp2d16sp,+vfp2sp,+vfp3,+vfp3d16,+vfp3d16sp,+vfp3sp,+vfp4,+vfp4d16,+vfp4d16sp,+vfp4sp"
+// CHECK-BASIC-V82: 
"target-features"="+armv8.2-a,+crc,+crypto,+d32,+dotprod,+dsp,+fp-armv8,+fp-armv8d16,+fp-armv8d16sp,+fp-armv8sp,+fp16,+fp64,+fpregs,+fullfp16,+hwdiv,+hwdiv-arm,+neon,+ras,+thumb-mode,+vfp2,+vfp2d16,+vfp2d16sp,+vfp2sp,+vfp3,+vfp3d16,+vfp3d16sp,+vfp3sp,+vfp4,+vfp4d16,+vfp4d16sp,+vfp4sp"
 
 // RUN: %clang_cc1 -triple armv8-linux-gnueabi -target-cpu cortex-a53 
-emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-BASIC-V8-ARM
 // CHECK-BASIC-V8-ARM: 
"target-features"="+armv8-a,+crc,+crypto,+d32,+dsp,+fp-armv8,+fp-armv8d16,+fp-armv8d16sp,+fp-armv8sp,+fp16,+fp64,+fpregs,+hwdiv,+hwdiv-arm,+neon,+vfp2,+vfp2d16,+vfp2d16sp,+vfp2sp,+vfp3,+vfp3d16,+vfp3d16sp,+vfp3sp,+vfp4,+vfp4d16,+vfp4d16sp,+vfp4sp,-thumb-mode"


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

r366024 - consistency in the release notes

2019-07-14 Thread Sylvestre Ledru via cfe-commits 
Author: sylvestre
Date: Sun Jul 14 11:25:09 2019
New Revision: 366024

URL: http://llvm.org/viewvc/llvm-project?rev=366024=rev
Log:
consistency in the release notes

Modified:
cfe/trunk/docs/ReleaseNotes.rst

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=366024=366023=366024=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Sun Jul 14 11:25:09 2019
@@ -184,8 +184,8 @@ AST Matchers
 clang-format
 
 
-- Add language support for clang-formatting C# files
-- Add Microsoft coding style to encapsulate default C# formatting style
+- Add language support for clang-formatting C# files.
+- Add Microsoft coding style to encapsulate default C# formatting style.
 - Added new option `PPDIS_BeforeHash` (in configuration: `BeforeHash`) to
   `IndentPPDirectives` which indents preprocessor directives before the hash.
 - Added new option `AlignConsecutiveMacros` to align the C/C++ preprocessor


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

[PATCH] D64075: [ASTImporter] Fix structural eq of lambdas

2019-07-14 Thread Aleksei Sidorin via Phabricator via cfe-commits 
a_sidorin accepted this revision.
a_sidorin added a comment.
This revision is now accepted and ready to land.

Hello Gabor,
This patch looks good to me.
Regarding the related patch: can we use getLambdaManglingNumber() for the 
comparison?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64075



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

[PATCH] D64271: [analyzer] Don't track the right hand side of the last store for conditions

2019-07-14 Thread Kristóf Umann via Phabricator via cfe-commits 
Szelethus added a comment.

In D64271#1576872 , @NoQ wrote:

> I'd rather not abandon this patch, because it looks like a strict improvement 
> over the lack of condition tracking, and it might as well still be an 
> improvement over "zealous" condition tracking, as my counterexample is fairly 
> artificial. It indicates that a slightly more sophisticated algorithm is 
> necessary (i'm not sure if it's single-pass or even linear). But i'll be 
> perfectly happy with simply adding it as a FIXME test.


My other patches actually deal with this far more nicely. D64272 
 + D64232  
combined almost achieve the same thing, but I'm running analyses to find gather 
some real world experience.
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64271



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

[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

2019-07-14 Thread Manikishan Ghantasala via Phabricator via cfe-commits 
Manikishan marked an inline comment as done.
Manikishan added a comment.

In D64695#1584706 , @MyDeveloperDay 
wrote:

> There also seems like alot of duplication between the existing sortCppIncludes
>
> I think the only difference here is really just
>
> 
>
>   SmallVector Indices;
> SmallVector Includes_p;
> for (unsigned i = 0, e = Includes.size(); i != e; ++i) {
>   unsigned pl = getNetBSDIncludePriority(Includes[i].Filename);
>   Includes_p.push_back(pl);
>   Indices.push_back(i);
> }
>
>
> vs the original
>
>   SmallVector Indices;
>   for (unsigned i = 0, e = Includes.size(); i != e; ++i)
> Indices.push_back(i);
>   
>
> plus way the sorting is performed, are we sure we couldn't have just made the 
> original sorting more powerful based on style settings?


Does it mean that adding the priority to sort based on style? 
like this:

  if (Style== NetBSD)
 // set priority to netbsd's priority

I didn't want to mess up the original sorting and made up this patch, if we 
have parameterise this solution, I will go for it.




Comment at: lib/Format/Format.cpp:1878
+  SmallVector Indices;
+  SmallVector Includes_p;
+  for (unsigned i = 0, e = Includes.size(); i != e; ++i) {

MyDeveloperDay wrote:
> _p? I don't understand what it stands for?
> 
> IncludesPriority?
Yes , it means priority


Repository:
  rC Clang

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

https://reviews.llvm.org/D64695



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

[PATCH] D64272: [analyzer] Note last writes to a condition only in a nested stackframe

2019-07-14 Thread Kristóf Umann via Phabricator via cfe-commits 
Szelethus added a comment.

Gentle ping.


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

https://reviews.llvm.org/D64272



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

[PATCH] D64287: [analyzer] Track the right hand side of the last store regardless of its value

2019-07-14 Thread Kristóf Umann via Phabricator via cfe-commits 
Szelethus marked 2 inline comments as done.
Szelethus added a comment.

Would you say it's good to go? :)




Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1436-1437
-}
-ReturnVisitor::addVisitorIfNecessary(StoreSite, InitE->IgnoreParenCasts(),
- BR, EnableNullFPSuppression);
   }

NoQ wrote:
> You're removing this visitor, does it cause any removal of notes?
Nope, `trackExpressionValue` will add it back, if appropriate.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1989-1991
+}
+else if (const auto *AR = L->getRegionAs())
+  CanDereference = false;

NoQ wrote:
> Fair enough. Did we already have a test case for this? I wonder if function 
> pointers might also cause problems here.
> 
> Ugh, this code is so underdebugged.
Yup, `test/Analysis/exercise-ps.c` flopped. And I agree, this looks kinda nasty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64287



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

[PATCH] D64232: [analyzer] Prune calls to functions with linear CFGs that return a non-zero constrained value

2019-07-14 Thread Kristóf Umann via Phabricator via cfe-commits 
Szelethus added a comment.

Gentle ping.


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

https://reviews.llvm.org/D64232



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

[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with '-mframe-pointer'

2019-07-14 Thread Fangrui Song via Phabricator via cfe-commits 
MaskRay added inline comments.



Comment at: clang/include/clang/Basic/CodeGenOptions.h:120
 
+  enum FramePointerKind {
+FP_All, // Keep all frame pointers.

We can probably use `enum class FramePointerKind { None, NonLeaf, All };` 
here.. (`enum class ClassABI` is an example in this file)



Comment at: clang/lib/CodeGen/CGCall.cpp:1723
+else
+  llvm_unreachable("unknown frame-pointer");
 

`llvm_unreachable` can be deleted if a switch statement is used.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:49
 
+using FramePointerKind = clang::CodeGenOptions::FramePointerKind;
 using namespace clang::driver;

This is probably not necessary. The enum is only used in 2 places below.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3961
+  else
+llvm_unreachable("unknown FramePointerKind");
+

`llvm_unreachable("unknown FramePointerKind");` can be deleted if a switch 
statement is used.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:845
+  if (Args.hasArg(OPT_pg))
+Opts.setFramePointer(CodeGenOptions::FP_All);
+

I guess this can be deleted now.

`-pg` + FramePointerKind::None is rejected by the driver.

```
  if (Arg *A = Args.getLastArg(options::OPT_pg))
if (FPKeepKind == FramePointerKind::None)
  D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer"
  << A->getAsString(Args);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56353



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

[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

2019-07-14 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added a comment.

There also seems like alot of duplication between the existing sortCppIncludes

I think the only difference here is really just

  SmallVector Indices;
SmallVector Includes_p;
for (unsigned i = 0, e = Includes.size(); i != e; ++i) {
  unsigned pl = getNetBSDIncludePriority(Includes[i].Filename);
  Includes_p.push_back(pl);
  Indices.push_back(i);
}

vs the original

  SmallVector Indices;
  for (unsigned i = 0, e = Includes.size(); i != e; ++i)
Indices.push_back(i);

plus way the sorting is performed, are we sure we couldn't have just made the 
original sorting more powerful based on style settings?




Comment at: lib/Format/Format.cpp:1878
+  SmallVector Indices;
+  SmallVector Includes_p;
+  for (unsigned i = 0, e = Includes.size(); i != e; ++i) {

_p? I don't understand what it stands for?

IncludesPriority?


Repository:
  rC Clang

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

https://reviews.llvm.org/D64695



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

[PATCH] D64278: Rename libclang_shared to libclang-cpp

2019-07-14 Thread Sylvestre Ledru via Phabricator via cfe-commits 
sylvestre.ledru added a comment.

@beanz By the way, maybe we should add this in the 9 release notes?
Thanks


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64278



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

[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

2019-07-14 Thread Manikishan Ghantasala via Phabricator via cfe-commits 
Manikishan added a comment.

In D64695#1584630 , @MyDeveloperDay 
wrote:

> Do you think there is anything about this algorithm that could be 
> parameterized so that other projects could utilize it?


I think we can take input from users the header files which should be grouped 
together and pass them to regex searches which then sets up priority. 
But I dob the feasibility of this approach. Will  come up with one.

> I guess its not completely clear how this differs from just using the 
> IncludeCategories?

This style works only via NetBSD includes order instead of alphabetical .


Repository:
  rC Clang

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

https://reviews.llvm.org/D64695



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

[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

2019-07-14 Thread MyDeveloperDay via Phabricator via cfe-commits 
MyDeveloperDay added a comment.

Do you think there is anything about this algorithm that could be parameterized 
so that other projects could utilize it? I guess its not completely clear how 
this differs from just using the IncludeCategories?


Repository:
  rC Clang

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

https://reviews.llvm.org/D64695



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

[PATCH] D63048: Update __VERSION__ to remove the hardcoded 4.2.1 version

2019-07-14 Thread Sylvestre Ledru via Phabricator via cfe-commits 
sylvestre.ledru updated this revision to Diff 209715.
sylvestre.ledru added a comment.

Implement the change (just update __VERSION__)


Repository:
  rC Clang

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

https://reviews.llvm.org/D63048

Files:
  docs/LanguageExtensions.rst
  docs/ReleaseNotes.rst
  lib/Frontend/InitPreprocessor.cpp
  test/Preprocessor/init.c


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -8169,7 +8169,7 @@
 // SPARC:#define __UINT_LEAST8_MAX__ 255
 // SPARC:#define __UINT_LEAST8_TYPE__ unsigned char
 // SPARC:#define __USER_LABEL_PREFIX__
-// SPARC:#define __VERSION__ "4.2.1 Compatible{{.*}}
+// SPARC:#define __VERSION__ "Clang{{.*}}
 // SPARC:#define __WCHAR_MAX__ 2147483647
 // SPARC:#define __WCHAR_TYPE__ int
 // SPARC:#define __WCHAR_WIDTH__ 32
@@ -9041,7 +9041,7 @@
 // X86_64-CLOUDABI:#define __UINT_LEAST8_MAX__ 255
 // X86_64-CLOUDABI:#define __UINT_LEAST8_TYPE__ unsigned char
 // X86_64-CLOUDABI:#define __USER_LABEL_PREFIX__
-// X86_64-CLOUDABI:#define __VERSION__ "4.2.1 Compatible{{.*}}
+// X86_64-CLOUDABI:#define __VERSION__ "Clang{{.*}}
 // X86_64-CLOUDABI:#define __WCHAR_MAX__ 2147483647
 // X86_64-CLOUDABI:#define __WCHAR_TYPE__ int
 // X86_64-CLOUDABI:#define __WCHAR_WIDTH__ 32
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -604,10 +604,9 @@
   // Support for #pragma redefine_extname (Sun compatibility)
   Builder.defineMacro("__PRAGMA_REDEFINE_EXTNAME", "1");
 
-  // As sad as it is, enough software depends on the __VERSION__ for version
-  // checks that it is necessary to report 4.2.1 (the base GCC version we claim
-  // compatibility with) first.
-  Builder.defineMacro("__VERSION__", "\"4.2.1 Compatible " +
+  // Previously this macro was set to a string aiming to achieve compatibility
+  // with GCC 4.2.1. Now, just return the full Clang version
+  Builder.defineMacro("__VERSION__", "\"" +
   Twine(getClangFullCPPVersion()) + "\"");
 
   // Initialize language-specific preprocessor defines.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -56,6 +56,12 @@
 Non-comprehensive list of changes in this release
 -
 
+- The ``__VERSION__`` macro has been updated.
+  Previously this macro contained the string '4.2.1 Compatible' to achieve
+  compatibility with GCC 4.2.1, but that should no longer be necessary.
+  However, to retrieve Clang's version, please favor the one of the macro
+  defined in :ref:`clang namespaced version macros 
`.
+
 - ...
 
 
Index: docs/LanguageExtensions.rst
===
--- docs/LanguageExtensions.rst
+++ docs/LanguageExtensions.rst
@@ -324,6 +324,8 @@
   ...
   #endif
 
+.. _languageextensions-builtin-macros:
+
 Builtin Macros
 ==
 


Index: test/Preprocessor/init.c
===
--- test/Preprocessor/init.c
+++ test/Preprocessor/init.c
@@ -8169,7 +8169,7 @@
 // SPARC:#define __UINT_LEAST8_MAX__ 255
 // SPARC:#define __UINT_LEAST8_TYPE__ unsigned char
 // SPARC:#define __USER_LABEL_PREFIX__
-// SPARC:#define __VERSION__ "4.2.1 Compatible{{.*}}
+// SPARC:#define __VERSION__ "Clang{{.*}}
 // SPARC:#define __WCHAR_MAX__ 2147483647
 // SPARC:#define __WCHAR_TYPE__ int
 // SPARC:#define __WCHAR_WIDTH__ 32
@@ -9041,7 +9041,7 @@
 // X86_64-CLOUDABI:#define __UINT_LEAST8_MAX__ 255
 // X86_64-CLOUDABI:#define __UINT_LEAST8_TYPE__ unsigned char
 // X86_64-CLOUDABI:#define __USER_LABEL_PREFIX__
-// X86_64-CLOUDABI:#define __VERSION__ "4.2.1 Compatible{{.*}}
+// X86_64-CLOUDABI:#define __VERSION__ "Clang{{.*}}
 // X86_64-CLOUDABI:#define __WCHAR_MAX__ 2147483647
 // X86_64-CLOUDABI:#define __WCHAR_TYPE__ int
 // X86_64-CLOUDABI:#define __WCHAR_WIDTH__ 32
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -604,10 +604,9 @@
   // Support for #pragma redefine_extname (Sun compatibility)
   Builder.defineMacro("__PRAGMA_REDEFINE_EXTNAME", "1");
 
-  // As sad as it is, enough software depends on the __VERSION__ for version
-  // checks that it is necessary to report 4.2.1 (the base GCC version we claim
-  // compatibility with) first.
-  Builder.defineMacro("__VERSION__", "\"4.2.1 Compatible " +
+  // Previously this macro was set to a string aiming to achieve compatibility
+  // with GCC 4.2.1. Now, just return the full Clang version
+  Builder.defineMacro("__VERSION__", "\"" +
   Twine(getClangFullCPPVersion()) + "\"");
 
   // Initialize

r366013 - [Driver] Simplify AddLibgcc

2019-07-14 Thread Fangrui Song via cfe-commits 
Author: maskray
Date: Sun Jul 14 00:16:13 2019
New Revision: 366013

URL: http://llvm.org/viewvc/llvm-project?rev=366013=rev
Log:
[Driver] Simplify AddLibgcc

Modified:
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=366013=366012=366013=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Sun Jul 14 00:16:13 2019
@@ -1194,17 +1194,11 @@ static void AddUnwindLibrary(const ToolC
 
 static void AddLibgcc(const ToolChain , const Driver ,
   ArgStringList , const ArgList ) {
-  bool isAndroid = TC.getTriple().isAndroid();
-
   LibGccType LGT = getLibGccType(D, Args);
-  bool LibGccFirst = (D.CCCIsCC() && LGT == LibGccType::UnspecifiedLibGcc) ||
- LGT == LibGccType::StaticLibGcc;
-  if (LibGccFirst)
+  if (LGT != LibGccType::SharedLibGcc)
 CmdArgs.push_back("-lgcc");
-
   AddUnwindLibrary(TC, D, CmdArgs, Args);
-
-  if (!LibGccFirst)
+  if (LGT == LibGccType::SharedLibGcc)
 CmdArgs.push_back("-lgcc");
 
   // According to Android ABI, we have to link with libdl if we are
@@ -1212,7 +1206,7 @@ static void AddLibgcc(const ToolChain 
   //
   // NOTE: This fixes a link error on Android MIPS as well.  The non-static
   // libgcc for MIPS relies on _Unwind_Find_FDE and dl_iterate_phdr from libdl.
-  if (isAndroid && getLibGccType(D, Args) != LibGccType::StaticLibGcc)
+  if (TC.getTriple().isAndroid() && LGT != LibGccType::StaticLibGcc)
 CmdArgs.push_back("-ldl");
 }
 


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

r366012 - [Driver] Simplify -lgcc & -lgcc_s

2019-07-14 Thread Fangrui Song via cfe-commits 
Author: maskray
Date: Sat Jul 13 23:46:46 2019
New Revision: 366012

URL: http://llvm.org/viewvc/llvm-project?rev=366012=rev
Log:
[Driver] Simplify -lgcc & -lgcc_s

gcc defaults to -shared-libgcc in C++ mode.
Letting getLibGccType() return SharedLibGcc simplifies the logic.

Modified:
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=366012=366011=366012=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Sat Jul 13 23:46:46 2019
@@ -1135,16 +1135,12 @@ bool tools::isObjCAutoRefCount(const Arg
 
 enum class LibGccType { UnspecifiedLibGcc, StaticLibGcc, SharedLibGcc };
 
-static LibGccType getLibGccType(const ArgList ) {
-  bool Static = Args.hasArg(options::OPT_static_libgcc) ||
-Args.hasArg(options::OPT_static) ||
-Args.hasArg(options::OPT_static_pie);
-
-  bool Shared = Args.hasArg(options::OPT_shared_libgcc);
-  if (Shared)
-return LibGccType::SharedLibGcc;
-  if (Static)
+static LibGccType getLibGccType(const Driver , const ArgList ) {
+  if (Args.hasArg(options::OPT_static_libgcc) ||
+  Args.hasArg(options::OPT_static) || Args.hasArg(options::OPT_static_pie))
 return LibGccType::StaticLibGcc;
+  if (Args.hasArg(options::OPT_shared_libgcc) || D.CCCIsCXX())
+return LibGccType::SharedLibGcc;
   return LibGccType::UnspecifiedLibGcc;
 }
 
@@ -1170,8 +1166,8 @@ static void AddUnwindLibrary(const ToolC
   UNW == ToolChain::UNW_None)
 return;
 
-  LibGccType LGT = getLibGccType(Args);
-  bool AsNeeded = D.CCCIsCC() && LGT == LibGccType::UnspecifiedLibGcc &&
+  LibGccType LGT = getLibGccType(D, Args);
+  bool AsNeeded = LGT == LibGccType::UnspecifiedLibGcc &&
   !TC.getTriple().isAndroid() && !TC.getTriple().isOSCygMing();
   if (AsNeeded)
 CmdArgs.push_back("--as-needed");
@@ -1180,11 +1176,11 @@ static void AddUnwindLibrary(const ToolC
   case ToolChain::UNW_None:
 return;
   case ToolChain::UNW_Libgcc: {
-LibGccType LGT = getLibGccType(Args);
-if (LGT == LibGccType::UnspecifiedLibGcc || LGT == 
LibGccType::SharedLibGcc)
-  CmdArgs.push_back("-lgcc_s");
-else if (LGT == LibGccType::StaticLibGcc)
+LibGccType LGT = getLibGccType(D, Args);
+if (LGT == LibGccType::StaticLibGcc)
   CmdArgs.push_back("-lgcc_eh");
+else
+  CmdArgs.push_back("-lgcc_s");
 break;
   }
   case ToolChain::UNW_CompilerRT:
@@ -1200,7 +1196,7 @@ static void AddLibgcc(const ToolChain 
   ArgStringList , const ArgList ) {
   bool isAndroid = TC.getTriple().isAndroid();
 
-  LibGccType LGT = getLibGccType(Args);
+  LibGccType LGT = getLibGccType(D, Args);
   bool LibGccFirst = (D.CCCIsCC() && LGT == LibGccType::UnspecifiedLibGcc) ||
  LGT == LibGccType::StaticLibGcc;
   if (LibGccFirst)
@@ -1216,7 +1212,7 @@ static void AddLibgcc(const ToolChain 
   //
   // NOTE: This fixes a link error on Android MIPS as well.  The non-static
   // libgcc for MIPS relies on _Unwind_Find_FDE and dl_iterate_phdr from libdl.
-  if (isAndroid && getLibGccType(Args) != LibGccType::StaticLibGcc)
+  if (isAndroid && getLibGccType(D, Args) != LibGccType::StaticLibGcc)
 CmdArgs.push_back("-ldl");
 }
 


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