Re: Move createEliminateAvailableExternallyPass earlier

[Yaron Keren via cfe-commits]

r246619, thanks!
About createEliminateAvailableExternallyPass for LTO, I'm missing
something. At link time, aren't available externally function supposed to
be, well, available as external linkage? Won't the link fail without these
versions?
if so, why keep the available externally versions at all for LTO instead of
using the external linkage versions?


2015-09-01 23:56 GMT+03:00 Teresa Johnson :

> Hi Yaron,
>
> Thanks, moving it earlier in general seems ok to me. I originally put
> this right before the GlobalDCE calls because it was somewhat related.
> I wonder if the createEliminateAvailableExternallyPass call for the
> LTO pipeline should similarly be moved up closer to inlining, perhaps
> after any IP alias analysis passes? Hopefully someone more familiar
> with the passes than I will comment on how early we can do this.
>
> Teresa
>
> On Tue, Sep 1, 2015 at 1:32 PM, Yaron Keren via cfe-commits
>  wrote:
> > Following the discussion on cfe-dev, this moves
> > createEliminateAvailableExternallyPass earlier in the pass pipeline to
> save
> > running many ModulePasses on available external functions that are thrown
> > away anyhow.
> >
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12369: [clang-format] Fix alignConsecutiveAssignments not working properly

[Matt Oakes via cfe-commits]

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

This looks much better than the code I wrote. If all the unit tests are 
passing, the one change I can see to make is a misspelling in one of the 
comments. The rest looks excellent though!



Comment at: lib/Format/WhitespaceManager.cpp:171
@@ +170,3 @@
+  //
+  // Wee need to adjust the StartOfTokenColumn of each Change that is on a line
+  // containing any assignment to be aligned and located after such assignment

Nit: //We// instead of //Wee//


Comment at: lib/Format/WhitespaceManager.cpp:218
@@ -217,3 +229,1 @@
-  bool AlignedAssignment = false;
-  int PreviousShift = 0;
   for (unsigned i = Start; i != End; ++i) {

berenm wrote:
> What was the purpose of `PreviousShift`?
If I remember correctly it was to measure the maximum shift needed on the lines 
previous. The name `Shift` makes more sense actually.


Comment at: lib/Format/WhitespaceManager.cpp:229
@@ -228,3 +244,3 @@
 assert(Shift >= 0);
-Changes[i].Spaces += Shift;
 if (i + 1 != Changes.size())

berenm wrote:
> Why shifting by `Shift` //and then// also by `PreviousShift`?
I don't remember entirely. I think though that in the cases where both are 
applied the value will be the same. Either way, what you have now looks much 
cleaner.


http://reviews.llvm.org/D12369



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

Re: [PATCH] D12369: [clang-format] Fix alignConsecutiveAssignments not working properly

[Beren Minor via cfe-commits]

berenm updated this revision to Diff 33783.
berenm added a comment.

Here it is, with the typos in comments corrected, and rebased over the latest 
trunk.

Thanks for the review!


http://reviews.llvm.org/D12369

Files:
  lib/Format/WhitespaceManager.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -8537,6 +8537,10 @@
"int oneTwoThree = 123;\n"
"int oneTwo  = 12;",
Alignment);
+  verifyFormat("int oneTwoThree = 123;\n"
+   "int oneTwo  = 12;\n"
+   "method();\n",
+   Alignment);
   verifyFormat("int oneTwoThree = 123; // comment\n"
"int oneTwo  = 12;  // comment",
Alignment);
Index: lib/Format/WhitespaceManager.cpp
===
--- lib/Format/WhitespaceManager.cpp
+++ lib/Format/WhitespaceManager.cpp
@@ -156,41 +156,53 @@
   unsigned EndOfSequence = 0;
   bool FoundAssignmentOnLine = false;
   bool FoundLeftParenOnLine = false;
-  unsigned CurrentLine = 0;
 
+  // Aligns a sequence of assignment tokens, on the MinColumn column.
+  //
+  // Sequences start from the first assignment token to align, and end at the
+  // first token of the first line that doesn't need to be aligned.
+  //
+  // We need to adjust the StartOfTokenColumn of each Change that is on a line
+  // containing any assignment to be aligned and located after such assignment
   auto AlignSequence = [&] {
-alignConsecutiveAssignments(StartOfSequence, EndOfSequence, MinColumn);
+if (StartOfSequence > 0 && StartOfSequence < EndOfSequence)
+  alignConsecutiveAssignments(StartOfSequence, EndOfSequence, MinColumn);
 MinColumn = 0;
 StartOfSequence = 0;
 EndOfSequence = 0;
   };
 
   for (unsigned i = 0, e = Changes.size(); i != e; ++i) {
-if (Changes[i].NewlinesBefore != 0) {
-  CurrentLine += Changes[i].NewlinesBefore;
-  if (StartOfSequence > 0 &&
-  (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine)) {
-EndOfSequence = i;
+if (Changes[i].NewlinesBefore > 0) {
+  EndOfSequence = i;
+  // If there is a blank line or if the last line didn't contain any
+  // assignment, the sequence ends here.
+  if (Changes[i].NewlinesBefore > 1 || !FoundAssignmentOnLine) {
+// NB: In the latter case, the sequence should end at the beggining of
+// the previous line, but it doesn't really matter as there is no
+// assignment on it
 AlignSequence();
   }
+
   FoundAssignmentOnLine = false;
   FoundLeftParenOnLine = false;
 }
 
-if ((Changes[i].Kind == tok::equal &&
- (FoundAssignmentOnLine || ((Changes[i].NewlinesBefore > 0 ||
- Changes[i + 1].NewlinesBefore > 0 ||
-(!FoundLeftParenOnLine && Changes[i].Kind == tok::r_paren)) {
-  if (StartOfSequence > 0)
-AlignSequence();
+// If there is more than one "=" per line, or if the "=" appears first on
+// the line of if it appears last, end the sequence
+if (Changes[i].Kind == tok::equal &&
+(FoundAssignmentOnLine || Changes[i].NewlinesBefore > 0 ||
+ Changes[i + 1].NewlinesBefore > 0)) {
+  AlignSequence();
+} else if (!FoundLeftParenOnLine && Changes[i].Kind == tok::r_paren) {
+  AlignSequence();
 } else if (Changes[i].Kind == tok::l_paren) {
   FoundLeftParenOnLine = true;
-  if (!FoundAssignmentOnLine && StartOfSequence > 0)
+  if (!FoundAssignmentOnLine)
 AlignSequence();
 } else if (!FoundAssignmentOnLine && !FoundLeftParenOnLine &&
Changes[i].Kind == tok::equal) {
   FoundAssignmentOnLine = true;
-  EndOfSequence = i;
   if (StartOfSequence == 0)
 StartOfSequence = i;
 
@@ -199,36 +211,34 @@
 }
   }
 
-  if (StartOfSequence > 0) {
-EndOfSequence = Changes.size();
-AlignSequence();
-  }
+  EndOfSequence = Changes.size();
+  AlignSequence();
 }
 
 void WhitespaceManager::alignConsecutiveAssignments(unsigned Start,
 unsigned End,
 unsigned Column) {
-  bool AlignedAssignment = false;
-  int PreviousShift = 0;
+  bool FoundAssignmentOnLine = false;
+  int Shift = 0;
   for (unsigned i = Start; i != End; ++i) {
-int Shift = 0;
-if (Changes[i].NewlinesBefore > 0)
-  AlignedAssignment = false;
-if (!AlignedAssignment && Changes[i].Kind == tok::equal) {
+if (Changes[i].NewlinesBefore > 0) {
+  FoundAssignmentOnLine = false;
+  Shift = 0;
+}
+
+// If this is the first assignment to be aligned, remember by how many
+// spaces it has to be shifted, so the rest of the changes on the line are
+// shifted by the same amoun

Re: [PATCH] D12369: [clang-format] Fix alignConsecutiveAssignments not working properly

[Beren Minor via cfe-commits]

berenm marked 5 inline comments as done.
berenm added a comment.

http://reviews.llvm.org/D12369



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

Re: [PATCH] D11797: [LIbClang] Report the named type for ElaboratedType

[Sergey Kalinichev via cfe-commits]

skalinichev retitled this revision from "[libclang] Expose the ElaboratedType" 
to "[LIbClang] Report the named type for ElaboratedType".
skalinichev updated the summary for this revision.
skalinichev updated this revision to Diff 33787.
skalinichev added a comment.

It just occured to me, why do we need to expose the ElaboratedType if we can 
retrieve the original type and return it to user. Especially when we already do 
that for Attributed and Decayed types. So here is a much simplier solution.


http://reviews.llvm.org/D11797

Files:
  test/Index/print-type.c
  test/Index/print-type.cpp
  tools/libclang/CXType.cpp

Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -121,6 +121,11 @@
 if (const DecayedType *DT = T->getAs()) {
   return MakeCXType(DT->getOriginalType(), TU);
 }
+
+/* Handle elaborated types as the original type */
+if (const auto* ET = T->getAs()) {
+  return MakeCXType(ET->getNamedType(), TU);
+}
   }
   if (TK == CXType_Invalid)
 TK = GetTypeKind(T);
@@ -394,7 +399,6 @@
 
   Decl *D = nullptr;
 
-try_again:
   switch (TP->getTypeClass()) {
   case Type::Typedef:
 D = cast(TP)->getDecl();
@@ -423,10 +427,6 @@
 
   // FIXME: Template type parameters!  
 
-  case Type::Elaborated:
-TP = cast(TP)->getNamedType().getTypePtrOrNull();
-goto try_again;
-
   default:
 break;
   }
Index: test/Index/print-type.cpp
===
--- test/Index/print-type.cpp
+++ test/Index/print-type.cpp
@@ -48,6 +48,8 @@
 };
 int Blob::*member_pointer;
 
+namespace NS { struct Type{}; } NS::Type elaboratedNamespaceType();
+
 // RUN: c-index-test -test-print-type %s -std=c++11 | FileCheck %s
 // CHECK: Namespace=outer:1:11 (Definition) [type=] [typekind=Invalid] 
[isPOD=0]
 // CHECK: ClassTemplate=Foo:4:8 (Definition) [type=] [typekind=Invalid] 
[isPOD=0]
@@ -61,7 +63,7 @@
 // CHECK: Namespace=inner:14:11 (Definition) [type=] [typekind=Invalid] 
[isPOD=0]
 // CHECK: StructDecl=Bar:16:8 (Definition) [type=outer::inner::Bar] 
[typekind=Record] [isPOD=0] [nbFields=3]
 // CHECK: CXXConstructor=Bar:17:3 (Definition) [type=void (outer::Foo 
*){{.*}}] [typekind=FunctionProto] [canonicaltype=void (outer::Foo 
*){{.*}}] [canonicaltypekind=FunctionProto] [resulttype=void] 
[resulttypekind=Void] [args= [outer::Foo *] [Pointer]] [isPOD=0]
-// CHECK: ParmDecl=foo:17:25 (Definition) [type=outer::Foo *] 
[typekind=Pointer] [canonicaltype=outer::Foo *] 
[canonicaltypekind=Pointer] [isPOD=1] [pointeetype=outer::Foo] 
[pointeekind=Unexposed]
+// CHECK: ParmDecl=foo:17:25 (Definition) [type=outer::Foo *] 
[typekind=Pointer] [canonicaltype=outer::Foo *] 
[canonicaltypekind=Pointer] [isPOD=1] [pointeetype=Foo] 
[pointeekind=Unexposed]
 // CHECK: NamespaceRef=outer:1:11 [type=] [typekind=Invalid] [isPOD=0]
 // CHECK: TemplateRef=Foo:4:8 [type=] [typekind=Invalid] [isPOD=0]
 // CHECK: CompoundStmt= [type=] [typekind=Invalid] [isPOD=0]
@@ -119,3 +121,6 @@
 // CHECK: StructDecl=Blob:45:8 (Definition) [type=Blob] [typekind=Record] 
[isPOD=1] [nbFields=2]
 // CHECK: FieldDecl=i:46:7 (Definition) [type=int] [typekind=Int] [isPOD=1]
 // CHECK: VarDecl=member_pointer:49:12 (Definition) [type=int Blob::*] 
[typekind=MemberPointer] [isPOD=1]
+// CHECK: FunctionDecl=elaboratedNamespaceType:51:42 [type=NS::Type ()] 
[typekind=FunctionProto] [canonicaltype=NS::Type ()] 
[canonicaltypekind=FunctionProto] [resulttype=NS::Type] [resulttypekind=Record] 
[isPOD=0]
+// CHECK: NamespaceRef=NS:51:11 [type=] [typekind=Invalid] [isPOD=0]
+// CHECK: TypeRef=struct NS::Type:51:23 [type=NS::Type] [typekind=Record] 
[isPOD=1]
Index: test/Index/print-type.c
===
--- test/Index/print-type.c
+++ test/Index/print-type.c
@@ -12,6 +12,9 @@
 
 int f2(int incompletearray[]);
 
+enum Enum{i}; enum Enum elaboratedEnumType();
+struct Struct{}; struct Struct elaboratedStructType();
+
 // RUN: c-index-test -test-print-type %s | FileCheck %s
 // CHECK: FunctionDecl=f:3:6 (Definition) [type=int *(int *, char *, FooType, 
int *, void (*)(int))] [typekind=FunctionProto] [canonicaltype=int *(int *, 
char *, int, int *, void (*)(int))] [canonicaltypekind=FunctionProto] 
[resulttype=int *] [resulttypekind=Pointer] [args= [int *] [Pointer] [char *] 
[Pointer] [FooType] [Typedef] [int [5]] [ConstantArray] [void (*)(int)] 
[Pointer]] [isPOD=0]
 // CHECK: ParmDecl=p:3:13 (Definition) [type=int *] [typekind=Pointer] 
[isPOD=1] [pointeetype=int] [pointeekind=Int]
@@ -45,3 +48,8 @@
 // CHECK: VarDecl=x:10:38 [type=__attribute__((__vector_size__(4 * 
sizeof(int int] [typekind=Vector] [isPOD=1]
 // CHECK: TypedefDecl=int4_t:11:46 (Definition) [type=int4_t] 
[typekind=Typedef] [canonicaltype=__attribute__((__vector_size__(4 * 
sizeof(int int] [canonicaltypekind=Vector] [isPOD=1]
 // CHECK: ParmDecl=inco

Re: [PATCH] D12262: [OpenMP] Capture global variables in target regions.

[Alexey Bataev via cfe-commits]

ABataev added a comment.

LGTM


http://reviews.llvm.org/D12262



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

[PATCH] D12551: Fix use-auto-check.

[Angel Garcia via cfe-commits]

angelgarcia created this revision.
angelgarcia added a reviewer: alexfh.
angelgarcia added subscribers: klimek, cfe-commits.

Fix a bug where use-auto check would crash when the definition of a type is in 
the same statement than its instantiation with new.

http://reviews.llvm.org/D12551

Files:
  clang-tidy/modernize/UseAutoCheck.cpp
  test/clang-tidy/modernize-use-auto-new.cpp

Index: test/clang-tidy/modernize-use-auto-new.cpp
===
--- test/clang-tidy/modernize-use-auto-new.cpp
+++ test/clang-tidy/modernize-use-auto-new.cpp
@@ -37,6 +37,8 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with 
new
   // CHECK-FIXES: auto  volatile vol = new MyType();
 
+  struct SType {} *stype = new SType;
+
   int (**func)(int, int) = new (int(*[5])(int,int));
 
   int *array = new int[5];
Index: clang-tidy/modernize/UseAutoCheck.cpp
===
--- clang-tidy/modernize/UseAutoCheck.cpp
+++ clang-tidy/modernize/UseAutoCheck.cpp
@@ -298,7 +298,7 @@
 }
 
 void UseAutoCheck::replaceNew(const DeclStmt *D, ASTContext *Context) {
-  const auto *FirstDecl = cast(*D->decl_begin());
+  const auto *FirstDecl = dyn_cast(*D->decl_begin());
   // Ensure that there is at least one VarDecl within the DeclStmt.
   if (!FirstDecl)
 return;


Index: test/clang-tidy/modernize-use-auto-new.cpp
===
--- test/clang-tidy/modernize-use-auto-new.cpp
+++ test/clang-tidy/modernize-use-auto-new.cpp
@@ -37,6 +37,8 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with new
   // CHECK-FIXES: auto  volatile vol = new MyType();
 
+  struct SType {} *stype = new SType;
+
   int (**func)(int, int) = new (int(*[5])(int,int));
 
   int *array = new int[5];
Index: clang-tidy/modernize/UseAutoCheck.cpp
===
--- clang-tidy/modernize/UseAutoCheck.cpp
+++ clang-tidy/modernize/UseAutoCheck.cpp
@@ -298,7 +298,7 @@
 }
 
 void UseAutoCheck::replaceNew(const DeclStmt *D, ASTContext *Context) {
-  const auto *FirstDecl = cast(*D->decl_begin());
+  const auto *FirstDecl = dyn_cast(*D->decl_begin());
   // Ensure that there is at least one VarDecl within the DeclStmt.
   if (!FirstDecl)
 return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12551: Fix use-auto-check.

[Alexander Kornienko via cfe-commits]

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

Looks good. Thank you!


http://reviews.llvm.org/D12551



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

[clang-tools-extra] r246638 - Fix use-auto-check.

[Angel Garcia Gomez via cfe-commits]

Author: angelgarcia
Date: Wed Sep  2 05:20:00 2015
New Revision: 246638

URL: http://llvm.org/viewvc/llvm-project?rev=246638&view=rev
Log:
Fix use-auto-check.

Summary: Fix a bug where use-auto check would crash when the definition of a 
type is in the same statement than its instantiation with new.

Reviewers: alexfh

Subscribers: cfe-commits, klimek

Differential Revision: http://reviews.llvm.org/D12551

Modified:
clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-use-auto-new.cpp

Modified: clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp?rev=246638&r1=246637&r2=246638&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/UseAutoCheck.cpp Wed Sep  2 
05:20:00 2015
@@ -298,7 +298,7 @@ void UseAutoCheck::replaceIterators(cons
 }
 
 void UseAutoCheck::replaceNew(const DeclStmt *D, ASTContext *Context) {
-  const auto *FirstDecl = cast(*D->decl_begin());
+  const auto *FirstDecl = dyn_cast(*D->decl_begin());
   // Ensure that there is at least one VarDecl within the DeclStmt.
   if (!FirstDecl)
 return;

Modified: clang-tools-extra/trunk/test/clang-tidy/modernize-use-auto-new.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-use-auto-new.cpp?rev=246638&r1=246637&r2=246638&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/modernize-use-auto-new.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-use-auto-new.cpp Wed Sep  
2 05:20:00 2015
@@ -37,6 +37,8 @@ void auto_new() {
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use auto when initializing with 
new
   // CHECK-FIXES: auto  volatile vol = new MyType();
 
+  struct SType {} *stype = new SType;
+
   int (**func)(int, int) = new (int(*[5])(int,int));
 
   int *array = new int[5];


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

Re: [PATCH] D12400: Fix store detection for return value in CGCall

[Jakub Kuderski via cfe-commits]

kuhar added a comment.

ping


Repository:
  rL LLVM

http://reviews.llvm.org/D12400



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

Re: [PATCH] D3583: Sema: Implement DR244

[Johnny Willemsen via cfe-commits]

jwillemsen added a comment.

Just tested 3.7.0, this issue is not fixed in this new release of today


Repository:
  rL LLVM

http://reviews.llvm.org/D3583



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

Re: [PATCH] D12244: Implement ACLE 2.0 macros of chapters 6.4 and 6.5 for [ARM] and [Aarch64] targets

[Alexandros Lamprineas via cfe-commits]

labrinea added a comment.

Could I have some feedback please?


http://reviews.llvm.org/D12244



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

[clang-tools-extra] r246643 - [clang-tidy] Updated the check name in the doc.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Wed Sep  2 07:01:51 2015
New Revision: 246643

URL: http://llvm.org/viewvc/llvm-project?rev=246643&view=rev
Log:
[clang-tidy] Updated the check name in the doc.

Modified:
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-shrink-to-fit.rst

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-shrink-to-fit.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-shrink-to-fit.rst?rev=246643&r1=246642&r2=246643&view=diff
==
--- clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-shrink-to-fit.rst 
(original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/modernize-shrink-to-fit.rst 
Wed Sep  2 07:01:51 2015
@@ -1,5 +1,5 @@
-readability-shrink-to-fit
-=
+modernize-shrink-to-fit
+===
 
 
 Replace copy and swap tricks on shrinkable containers with the


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

Re: [PATCH] D11815: Pass subtarget feature "force-align-stack"

[Vasileios Kalintiris via cfe-commits]

vkalintiris added a comment.

In http://reviews.llvm.org/D11815#237541, @ahatanak wrote:

> In http://reviews.llvm.org/D11815#236368, @vkalintiris wrote:
>
> > In http://reviews.llvm.org/D11815#235394, @ahatanak wrote:
> >
> > >
> >
>
>
>
>
> > For example, on a Mips target, where the O32 ABI requires either way an 
> > 8-byte alignment, we would generate redundant code for realigning the stack 
> > to a 4-byte alignment if a function contains objects with maximum alignment 
> > of 4-bytes (see attached files to get an idea).
>
>
> I wonder if there is a target or a use case that requires or prefers 
> realigning the stack to an alignment that is smaller than the default stack 
> alignment.  If there is no such target or use case, I think we can just using 
> the existing attribute StackAlignment (with value 0) rather than adding a new 
> function attribute "stackrealign", which will ensure the stack is at least 
> aligned to the default value and force realigning the stack.


I was thinking something similar. I'm not quite sure but the only case I can 
think of, where we might get away with an alignment smaller than the default, 
is when we emit code for the fast calling convention. Other than that, I 
believe that we should duplicate the functionality of 
X86FrameLowering::calculateMaxStackAlign() (or override it in each target 
accordingly).

> The purpose of this patch is to make sure -mstackrealign works when doing LTO


Out of curiosity, I was wondering why dynamic stack realignment isn't enough 
for LTO. I would guess that the alignment of the data types used under SSE 
might have a smaller alignment than the one required by the ABI.


http://reviews.llvm.org/D11815



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

Re: [PATCH] D12400: Fix store detection for return value in CGCall

[Arnaud de Grandmaison via cfe-commits]

aadg added a subscriber: aadg.
aadg added a comment.

Just a minor nitpick (see below)



Comment at: lib/CodeGen/CGCall.cpp:2332
@@ -2331,2 +2331,3 @@
   if (!store) return nullptr;
+  if (store->getPointerOperand() != CGF.ReturnValue) return nullptr;
 

It might be worth stating in a comment why this is needed --- for the benefit 
of people who will read this code later.


Repository:
  rL LLVM

http://reviews.llvm.org/D12400



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

r246646 - Silence a -Wsign-compare warning; NFC.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Sep  2 07:50:12 2015
New Revision: 246646

URL: http://llvm.org/viewvc/llvm-project?rev=246646&view=rev
Log:
Silence a -Wsign-compare warning; NFC.

Modified:
cfe/trunk/lib/Serialization/MultiOnDiskHashTable.h

Modified: cfe/trunk/lib/Serialization/MultiOnDiskHashTable.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/MultiOnDiskHashTable.h?rev=246646&r1=246645&r2=246646&view=diff
==
--- cfe/trunk/lib/Serialization/MultiOnDiskHashTable.h (original)
+++ cfe/trunk/lib/Serialization/MultiOnDiskHashTable.h Wed Sep  2 07:50:12 2015
@@ -215,7 +215,7 @@ public:
 if (!PendingOverrides.empty())
   removeOverriddenTables();
 
-if (Tables.size() > Info::MaxTables)
+if (Tables.size() > static_cast(Info::MaxTables))
   condense();
 
 internal_key_type Key = Info::GetInternalKey(EKey);


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

Re: [PATCH] D12400: Fix store detection for return value in CGCall

[Arnaud de Grandmaison via cfe-commits]

aadg added a comment.

Yet another comment (see below).



Comment at: lib/CodeGen/CGCall.cpp:2332
@@ -2331,2 +2331,3 @@
   if (!store) return nullptr;
+  if (store->getPointerOperand() != CGF.ReturnValue) return nullptr;
 

aadg wrote:
> It might be worth stating in a comment why this is needed --- for the benefit 
> of people who will read this code later.
I also note that those lines are looking very much like those in the if above 
(handling the multiple use of the return value case)... This issue was fixed 
above, but not here. Would it be possible to avoid code duplication ?


Repository:
  rL LLVM

http://reviews.llvm.org/D12400



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

Re: Move createEliminateAvailableExternallyPass earlier

[Teresa Johnson via cfe-commits]

Hi Yaron,

This is to handle the case of pre-built (real .o) files.
The available externally functions are kept in the -c -flto compile
(the pass isn't run) so that they are available for inlining in the
LTO pipeline. If the available externally function is in a pre-built
object file it wouldn't be subject to inlining or other IPO.
A subsequent round of EliminateAvailableExternallyPass during the LTO
pipeline cleans up the copies kept initially. I'm wondering if the
call I placed in the LTO pipeline (in addLateLTOOptimizationPasses)
should similarly be moved earlier, sometime shortly after inlining in
addLTOOptimizationPasses().

Teresa

On Wed, Sep 2, 2015 at 12:02 AM, Yaron Keren  wrote:
> r246619, thanks!
> About createEliminateAvailableExternallyPass for LTO, I'm missing something.
> At link time, aren't available externally function supposed to be, well,
> available as external linkage? Won't the link fail without these versions?
> if so, why keep the available externally versions at all for LTO instead of
> using the external linkage versions?
>
>
> 2015-09-01 23:56 GMT+03:00 Teresa Johnson :
>>
>> Hi Yaron,
>>
>> Thanks, moving it earlier in general seems ok to me. I originally put
>> this right before the GlobalDCE calls because it was somewhat related.
>> I wonder if the createEliminateAvailableExternallyPass call for the
>> LTO pipeline should similarly be moved up closer to inlining, perhaps
>> after any IP alias analysis passes? Hopefully someone more familiar
>> with the passes than I will comment on how early we can do this.
>>
>> Teresa
>>
>> On Tue, Sep 1, 2015 at 1:32 PM, Yaron Keren via cfe-commits
>>  wrote:
>> > Following the discussion on cfe-dev, this moves
>> > createEliminateAvailableExternallyPass earlier in the pass pipeline to
>> > save
>> > running many ModulePasses on available external functions that are
>> > thrown
>> > away anyhow.
>> >
>> >
>> > ___
>> > cfe-commits mailing list
>> > cfe-commits@lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>> >
>>
>>
>>
>> --
>> Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
>
>



-- 
Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r246650 - [Sparc]: GCCInstallationDetector should not care if little-endian

[Douglas Katzman via cfe-commits]

Author: dougk
Date: Wed Sep  2 08:33:42 2015
New Revision: 246650

URL: http://llvm.org/viewvc/llvm-project?rev=246650&view=rev
Log:
[Sparc]: GCCInstallationDetector should not care if little-endian

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

Modified: cfe/trunk/lib/Driver/ToolChains.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=246650&r1=246649&r2=246650&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains.cpp Wed Sep  2 08:33:42 2015
@@ -1351,7 +1351,6 @@ bool Generic_GCC::GCCInstallationDetecto
   if (TargetTriple.getOS() == llvm::Triple::Solaris) {
 LibDirs.append(begin(SolarisSPARCLibDirs), end(SolarisSPARCLibDirs));
 TripleAliases.append(begin(SolarisSPARCTriples), end(SolarisSPARCTriples));
-
 return;
   }
 
@@ -1448,6 +1447,7 @@ bool Generic_GCC::GCCInstallationDetecto
 TripleAliases.append(begin(PPC64LETriples), end(PPC64LETriples));
 break;
   case llvm::Triple::sparc:
+  case llvm::Triple::sparcel:
 LibDirs.append(begin(SPARCv8LibDirs), end(SPARCv8LibDirs));
 TripleAliases.append(begin(SPARCv8Triples), end(SPARCv8Triples));
 BiarchLibDirs.append(begin(SPARCv9LibDirs), end(SPARCv9LibDirs));


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

r246652 - [Shave]: pass through more clang options to moviCompile

[Douglas Katzman via cfe-commits]

Author: dougk
Date: Wed Sep  2 08:42:43 2015
New Revision: 246652

URL: http://llvm.org/viewvc/llvm-project?rev=246652&view=rev
Log:
[Shave]: pass through more clang options to moviCompile

Modified:
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/test/Driver/shave-toolchain.c

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=246652&r1=246651&r2=246652&view=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Wed Sep  2 08:42:43 2015
@@ -9606,6 +9606,7 @@ void tools::SHAVE::Compiler::ConstructJo
options::OPT_f_Group,
options::OPT_f_clang_Group,
options::OPT_g_Group,
+   options::OPT_M_Group,
options::OPT_O_Group,
options::OPT_W_Group});
   CmdArgs.push_back("-fno-exceptions"); // Always do this even if unspecified.

Modified: cfe/trunk/test/Driver/shave-toolchain.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/shave-toolchain.c?rev=246652&r1=246651&r2=246652&view=diff
==
--- cfe/trunk/test/Driver/shave-toolchain.c (original)
+++ cfe/trunk/test/Driver/shave-toolchain.c Wed Sep  2 08:42:43 2015
@@ -22,7 +22,7 @@
 // INCLUDES: "-iquote" "quotepath" "-isystem" "syspath"
 
 // RUN: %clang -target shave -c -### %s -g -fno-inline-functions \
-// RUN: -fno-inline-functions-called-once -Os -Wall \
-// RUN: -ffunction-sections 2>&1 | FileCheck %s -check-prefix=F_G_O_W_OPTIONS
-// F_G_O_W_OPTIONS: "-g" "-fno-inline-functions" 
"-fno-inline-functions-called-once"
-// F_G_O_W_OPTIONS: "-Os" "-Wall" "-ffunction-sections"
+// RUN: -fno-inline-functions-called-once -Os -Wall -MF dep.d \
+// RUN: -ffunction-sections 2>&1 | FileCheck %s -check-prefix=PASSTHRU_OPTIONS
+// PASSTHRU_OPTIONS: "-g" "-fno-inline-functions" 
"-fno-inline-functions-called-once"
+// PASSTHRU_OPTIONS: "-Os" "-Wall" "-MF" "dep.d" "-ffunction-sections"


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

clang-tidy unit tests

[Aaron Ballman via cfe-commits]

I ran into an issue with the runCheckOnCode() function from
clang-tidy's unit tests, and I am not familiar enough with the way
tool invocation works to suggest a correct fix.

ClangTidyContext::setASTContext() is what sets up the language options
for the ClangTidyContext object currently. However, runCheckOnCode()
does not end up calling setASTContext() at any point. So when unit
testing code, it behaves differently than when running clang-tidy.

I am guessing that setting the language options should be separated
from setting the AST context, however, I'm not certain of the proper
way to accomplish this. As best I can tell, by the time
registerMatchers() is called in runCheckOnCode(), we don't have access
to a LangOptions object that's valid.

Suggestions?

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

Re: r246610 - Migrate the target attribute parsing code into an extension off of

[Benjamin Kramer via cfe-commits]

> On 02.09.2015, at 02:12, Eric Christopher via cfe-commits 
>  wrote:
> 
> Author: echristo
> Date: Tue Sep  1 19:12:02 2015
> New Revision: 246610
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=246610&view=rev
> Log:
> Migrate the target attribute parsing code into an extension off of
> the main attribute and cache the results so we don't have to parse
> a single attribute more than once.
> 
> This reapplies r246596 with a fix for an uninitialized class member,
> and a couple of cleanups and formatting changes.
> 
> Modified:
>cfe/trunk/include/clang/Basic/Attr.td
>cfe/trunk/lib/CodeGen/CGCall.cpp
> 
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=246610&r1=246609&r2=246610&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Tue Sep  1 19:12:02 2015
> @@ -1315,9 +1315,52 @@ def Pascal : InheritableAttr {
> 
> def Target : InheritableAttr {
>   let Spellings = [GCC<"target">];
> -  let Args = [StringArgument<"features">];
> +  let Args = [StringArgument<"featuresStr">];
>   let Subjects = SubjectList<[Function], ErrorDiag>;
>   let Documentation = [TargetDocs];
> +  let AdditionalMembers = [{
> +StringRef CPU;
> +std::vector Features;

Attributes are never destroyed so this vector will leak memory.

- Ben

> +bool Parsed = false;
> +StringRef getCPU() {
> +  if (!Parsed)
> +parse();
> +  return CPU;
> +}
> +std::vector &getFeatures() {
> +  if (!Parsed)
> +parse();
> +  return Features;
> +}
> +void parse() {
> +  SmallVector AttrFeatures;
> +  getFeaturesStr().split(AttrFeatures, ",");
> +
> +  // Grab the various features and prepend a "+" to turn on the feature 
> to
> +  // the backend and add them to our existing set of features.
> +  for (auto &Feature : AttrFeatures) {
> +// Go ahead and trim whitespace rather than either erroring or
> +// accepting it weirdly.
> +Feature = Feature.trim();
> +
> +// We don't support cpu tuning this way currently.
> +// TODO: Support the fpmath option. It will require checking
> +// overall feature validity for the function with the rest of the
> +// attributes on the function.
> +if (Feature.startswith("fpmath=") || Feature.startswith("tune="))
> +   continue;
> +
> +// While we're here iterating check for a different target cpu.
> +if (Feature.startswith("arch="))
> +  CPU = Feature.split("=").second.trim();
> +else if (Feature.startswith("no-"))
> +  Features.push_back("-" + Feature.split("-").second.str());
> +else
> +  Features.push_back("+" + Feature.str());
> +  }
> +  Parsed = true;
> +}
> +  }];
> }
> 
> def TransparentUnion : InheritableAttr {
> 
> Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=246610&r1=246609&r2=246610&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGCall.cpp Tue Sep  1 19:12:02 2015
> @@ -1499,40 +1499,19 @@ void CodeGenModule::ConstructAttributeLi
> const FunctionDecl *FD = dyn_cast_or_null(TargetDecl);
> if (FD && FD->hasAttr()) {
>   llvm::StringMap FeatureMap;
> -  const auto *TD = FD->getAttr();
> +  auto *TD = FD->getAttr();
> 
>   // Make a copy of the features as passed on the command line.
>   std::vector FnFeatures =
>   getTarget().getTargetOpts().FeaturesAsWritten;
> 
> -  // Grab the target attribute string.
> -  StringRef FeaturesStr = TD->getFeatures();
> -  SmallVector AttrFeatures;
> -  FeaturesStr.split(AttrFeatures, ",");
> +  std::vector &AttrFeatures = TD->getFeatures();
> +  std::copy(AttrFeatures.begin(), AttrFeatures.end(),
> +std::back_inserter(FnFeatures));
> 
> -  // Grab the various features and prepend a "+" to turn on the feature 
> to
> -  // the backend and add them to our existing set of features.
> -  for (auto &Feature : AttrFeatures) {
> -// Go ahead and trim whitespace rather than either erroring or
> -// accepting it weirdly.
> -Feature = Feature.trim();
> +  if (TD->getCPU() != "")
> + TargetCPU = TD->getCPU();
> 
> -// While we're here iterating check for a different target cpu.
> -if (Feature.startswith("arch="))
> -  TargetCPU = Feature.split("=").second.trim();
> -else if (Feature.startswith("tune="))
> -  // We don't support cpu tuning this way currently.
> -  ;
> -else if (Feature.startswith("fpmath="))
> -  // TODO: Support the fpmath option this way. It will req

[PATCH] D12555: Fix loop-convert crash.

[Angel Garcia via cfe-commits]

angelgarcia created this revision.
angelgarcia added a reviewer: alexfh.
angelgarcia added subscribers: klimek, cfe-commits.

loop-convert no longer crashes when calling a member function using a member 
pointer which is a member of another record.

http://reviews.llvm.org/D12555

Files:
  clang-tidy/modernize/LoopConvertUtils.cpp
  test/clang-tidy/modernize-loop-convert-basic.cpp

Index: test/clang-tidy/modernize-loop-convert-basic.cpp
===
--- test/clang-tidy/modernize-loop-convert-basic.cpp
+++ test/clang-tidy/modernize-loop-convert-basic.cpp
@@ -156,6 +156,17 @@
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & elem : mfpArr)
   // CHECK-FIXES-NEXT: (v.*elem)();
+
+  struct Foo {
+int (Val::*f)();
+  } foo[N];
+
+  for (int i = 0; i < N; ++i)
+int r = (v.*(foo[i].f))();
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & elem : foo)
+  // CHECK-FIXES-NEXT: int r = (v.*(elem.f))();
+
 }
 
 } // namespace Array
Index: clang-tidy/modernize/LoopConvertUtils.cpp
===
--- clang-tidy/modernize/LoopConvertUtils.cpp
+++ clang-tidy/modernize/LoopConvertUtils.cpp
@@ -370,7 +370,10 @@
 
   case Stmt::CXXMemberCallExprClass: {
 const auto *MemCall = cast(Init);
-if (MemCall->getMethodDecl()->getName() == "at") {
+// This check is needed because getMethodDecl can return nullptr if the
+// callee is a member function pointer.
+if (MemCall->getMethodDecl() &&
+MemCall->getMethodDecl()->getName() == "at") {
   assert(MemCall->getNumArgs() == 1);
   return isIndexInSubscriptExpr(MemCall->getArg(0), IndexVar);
 }


Index: test/clang-tidy/modernize-loop-convert-basic.cpp
===
--- test/clang-tidy/modernize-loop-convert-basic.cpp
+++ test/clang-tidy/modernize-loop-convert-basic.cpp
@@ -156,6 +156,17 @@
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & elem : mfpArr)
   // CHECK-FIXES-NEXT: (v.*elem)();
+
+  struct Foo {
+int (Val::*f)();
+  } foo[N];
+
+  for (int i = 0; i < N; ++i)
+int r = (v.*(foo[i].f))();
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & elem : foo)
+  // CHECK-FIXES-NEXT: int r = (v.*(elem.f))();
+
 }
 
 } // namespace Array
Index: clang-tidy/modernize/LoopConvertUtils.cpp
===
--- clang-tidy/modernize/LoopConvertUtils.cpp
+++ clang-tidy/modernize/LoopConvertUtils.cpp
@@ -370,7 +370,10 @@
 
   case Stmt::CXXMemberCallExprClass: {
 const auto *MemCall = cast(Init);
-if (MemCall->getMethodDecl()->getName() == "at") {
+// This check is needed because getMethodDecl can return nullptr if the
+// callee is a member function pointer.
+if (MemCall->getMethodDecl() &&
+MemCall->getMethodDecl()->getName() == "at") {
   assert(MemCall->getNumArgs() == 1);
   return isIndexInSubscriptExpr(MemCall->getArg(0), IndexVar);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: include/clang/Basic/Attr.td:894
@@ -893,1 +893,3 @@
 
+def DisableTailCalls : InheritableAttr {
+  let Spellings = [GNU<"disable_tail_calls">];

We already have an attribute that is tangentially related: OptimizeNone. Would 
it make sense to have that attribute take arguments that control what 
optimizations are enabled/disabled? How should these two attributes (and the 
resulting functionality) interact? For instance, if I specify optnone on a 
function, should that also disable tail call optimizations by creating an 
implicit instance of this new attribute?


Comment at: include/clang/Basic/Attr.td:895
@@ +894,3 @@
+def DisableTailCalls : InheritableAttr {
+  let Spellings = [GNU<"disable_tail_calls">];
+  let Subjects = SubjectList<[Function]>;

Do we also want a C++ spelling for this attribute, under the clang namespace?


Comment at: include/clang/Basic/Attr.td:896
@@ +895,3 @@
+  let Spellings = [GNU<"disable_tail_calls">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];

Should this also apply to Objective-C methods?


Comment at: include/clang/Basic/Attr.td:897
@@ +896,3 @@
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];
+}

Please document this attribute.


Comment at: lib/CodeGen/CGCall.cpp:1477
@@ -1476,3 +1476,3 @@
 
-FuncAttrs.addAttribute("disable-tail-calls",
-   llvm::toStringRef(CodeGenOpts.DisableTailCalls));
+if ((TargetDecl && TargetDecl->hasAttr()) ||
+CodeGenOpts.DisableTailCalls)

Better to move this into the block that already tests TargetDecl (around line 
1403 or so).


Comment at: lib/CodeGen/CGCall.cpp:1480
@@ +1479,3 @@
+  FuncAttrs.addAttribute("disable-tail-calls", "true");
+else
+  FuncAttrs.addAttribute("disable-tail-calls", "false");

Can do this in a single line with ?:


Comment at: lib/Sema/SemaDeclAttr.cpp:4882
@@ +4881,3 @@
+  case AttributeList::AT_DisableTailCalls:
+handleSimpleAttribute(S, D, Attr);
+break;

Are there semantic checks we would like to perform on the declaration for 
nonsense uses of this attribute, such as functions marked [[noreturn]], etc?


http://reviews.llvm.org/D12547



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

Re: r246610 - Migrate the target attribute parsing code into an extension off of

[Aaron Ballman via cfe-commits]

A few nits in addition to what Ben pointed out...

On Tue, Sep 1, 2015 at 8:12 PM, Eric Christopher via cfe-commits
 wrote:
> Author: echristo
> Date: Tue Sep  1 19:12:02 2015
> New Revision: 246610
>
> URL: http://llvm.org/viewvc/llvm-project?rev=246610&view=rev
> Log:
> Migrate the target attribute parsing code into an extension off of
> the main attribute and cache the results so we don't have to parse
> a single attribute more than once.
>
> This reapplies r246596 with a fix for an uninitialized class member,
> and a couple of cleanups and formatting changes.
>
> Modified:
> cfe/trunk/include/clang/Basic/Attr.td
> cfe/trunk/lib/CodeGen/CGCall.cpp
>
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=246610&r1=246609&r2=246610&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Tue Sep  1 19:12:02 2015
> @@ -1315,9 +1315,52 @@ def Pascal : InheritableAttr {
>
>  def Target : InheritableAttr {
>let Spellings = [GCC<"target">];
> -  let Args = [StringArgument<"features">];
> +  let Args = [StringArgument<"featuresStr">];
>let Subjects = SubjectList<[Function], ErrorDiag>;
>let Documentation = [TargetDocs];
> +  let AdditionalMembers = [{
> +StringRef CPU;
> +std::vector Features;
> +bool Parsed = false;

Additional members are all public, can you privatize the data members?
Then you can make them mutable and fix the const-correctness
regression. ;-)

> +StringRef getCPU() {
> +  if (!Parsed)
> +parse();
> +  return CPU;
> +}
> +std::vector &getFeatures() {
> +  if (!Parsed)
> +parse();
> +  return Features;
> +}
> +void parse() {
> +  SmallVector AttrFeatures;
> +  getFeaturesStr().split(AttrFeatures, ",");
> +
> +  // Grab the various features and prepend a "+" to turn on the feature 
> to
> +  // the backend and add them to our existing set of features.
> +  for (auto &Feature : AttrFeatures) {
> +// Go ahead and trim whitespace rather than either erroring or
> +// accepting it weirdly.
> +Feature = Feature.trim();
> +
> +// We don't support cpu tuning this way currently.
> +// TODO: Support the fpmath option. It will require checking
> +// overall feature validity for the function with the rest of the
> +// attributes on the function.
> +if (Feature.startswith("fpmath=") || Feature.startswith("tune="))
> + continue;
> +
> +// While we're here iterating check for a different target cpu.
> +if (Feature.startswith("arch="))
> +  CPU = Feature.split("=").second.trim();
> +else if (Feature.startswith("no-"))
> +  Features.push_back("-" + Feature.split("-").second.str());
> +else
> +  Features.push_back("+" + Feature.str());
> +  }
> +  Parsed = true;
> +}
> +  }];
>  }
>
>  def TransparentUnion : InheritableAttr {
>
> Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=246610&r1=246609&r2=246610&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGCall.cpp Tue Sep  1 19:12:02 2015
> @@ -1499,40 +1499,19 @@ void CodeGenModule::ConstructAttributeLi
>  const FunctionDecl *FD = dyn_cast_or_null(TargetDecl);
>  if (FD && FD->hasAttr()) {
>llvm::StringMap FeatureMap;
> -  const auto *TD = FD->getAttr();
> +  auto *TD = FD->getAttr();
>
>// Make a copy of the features as passed on the command line.
>std::vector FnFeatures =
>getTarget().getTargetOpts().FeaturesAsWritten;
>
> -  // Grab the target attribute string.
> -  StringRef FeaturesStr = TD->getFeatures();
> -  SmallVector AttrFeatures;
> -  FeaturesStr.split(AttrFeatures, ",");
> +  std::vector &AttrFeatures = TD->getFeatures();
> +  std::copy(AttrFeatures.begin(), AttrFeatures.end(),
> +std::back_inserter(FnFeatures));
>
> -  // Grab the various features and prepend a "+" to turn on the feature 
> to
> -  // the backend and add them to our existing set of features.
> -  for (auto &Feature : AttrFeatures) {
> -// Go ahead and trim whitespace rather than either erroring or
> -// accepting it weirdly.
> -Feature = Feature.trim();
> +  if (TD->getCPU() != "")
> +   TargetCPU = TD->getCPU();

Would this be a slight improvement, or am I splitting hairs too much? ;-)

if ((StringRef S = TD->getCPU()) != "")
  TargetCPU = S;

~Aaron

>
> -// While we're here iterating check for a different target cpu.
> -if (Feature.startswith("arch="))
> -  TargetCPU = Feature

Re: [PATCH] D12555: Fix loop-convert crash.

[Manuel Klimek via cfe-commits]

klimek accepted this revision.
klimek added a reviewer: klimek.
klimek added a comment.
This revision is now accepted and ready to land.

lg


http://reviews.llvm.org/D12555



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

[clang-tools-extra] r246655 - Fix loop-convert crash.

[Angel Garcia Gomez via cfe-commits]

Author: angelgarcia
Date: Wed Sep  2 09:25:08 2015
New Revision: 246655

URL: http://llvm.org/viewvc/llvm-project?rev=246655&view=rev
Log:
Fix loop-convert crash.

Summary: loop-convert no longer crashes when calling a member function using a 
member pointer which is a member of another record.

Reviewers: alexfh, klimek

Subscribers: cfe-commits, klimek

Differential Revision: http://reviews.llvm.org/D12555

Modified:
clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp
clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp

Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp?rev=246655&r1=246654&r2=246655&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertUtils.cpp Wed Sep  
2 09:25:08 2015
@@ -370,7 +370,10 @@ static bool isAliasDecl(const Decl *TheD
 
   case Stmt::CXXMemberCallExprClass: {
 const auto *MemCall = cast(Init);
-if (MemCall->getMethodDecl()->getName() == "at") {
+// This check is needed because getMethodDecl can return nullptr if the
+// callee is a member function pointer.
+if (MemCall->getMethodDecl() &&
+MemCall->getMethodDecl()->getName() == "at") {
   assert(MemCall->getNumArgs() == 1);
   return isIndexInSubscriptExpr(MemCall->getArg(0), IndexVar);
 }

Modified: 
clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp?rev=246655&r1=246654&r2=246655&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/modernize-loop-convert-basic.cpp 
Wed Sep  2 09:25:08 2015
@@ -156,6 +156,17 @@ void memberFunctionPointer() {
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & elem : mfpArr)
   // CHECK-FIXES-NEXT: (v.*elem)();
+
+  struct Foo {
+int (Val::*f)();
+  } foo[N];
+
+  for (int i = 0; i < N; ++i)
+int r = (v.*(foo[i].f))();
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & elem : foo)
+  // CHECK-FIXES-NEXT: int r = (v.*(elem.f))();
+
 }
 
 } // namespace Array


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

Re: [PATCH] D12359: New warning -Wnonconst-parameter when a pointer parameter can be const

[Aaron Ballman via cfe-commits]

On Wed, Sep 2, 2015 at 2:19 AM, Daniel Marjamäki
 wrote:
> danielmarjamaki added a comment.
>
>> > > The checker isn't currently path sensitive as it doesn't pay attention
>
>>
>
>> >
>
>>
>
>> > >  to control flow graphs or how pointer values flow through a function
>
>>
>
>> >
>
>>
>
>> > >  body. I suppose this is a matter of scope more than anything; I see a
>
>>
>
>> >
>
>>
>
>> > >  logical extension of this functionality being with local variables as
>
>>
>
>> >
>
>>
>
>> > >  well as parameters. So, for instance:
>
>>
>
>> >
>
>>
>
>> > >
>
>>
>
>> >
>
>>
>
>> > > void f(int *p) {
>
>>
>
>> >
>
>>
>
>> > >
>
>>
>
>> >
>
>>
>
>> > >   int *i = p;
>
>>
>
>> >
>
>>
>
>> > >   std::cout << *i;
>
>>
>
>> >
>
>>
>
>> > >
>
>>
>
>> >
>
>>
>
>> > > }
>
>>
>
>> >
>
>>
>
>> >
>
>>
>
>> > Imho that path analysis is not very interesting. The "p" can't be const 
>> > until we see that "i" is const.
>
>>
>
>>
>
>> Correct, but from the user's perspective, why are we not telling them
>
>>  both can be const?
>
>
> We want to have simple and clear warning messages. I will currently just 
> write "parameter p can be const." Imho that is simple and clear. In your 
> example I believe it would be required with a more complex message. Because p 
> can't be const. It can only be const if i is made const first.

That is true.

> As I see it "my" analysis does not have any false negatives that would be 
> avoided. It's just that 2 separate simple messages are clumped together into 
> 1 more complex message.

Fair.

> I also believe that solving this iteratively in small steps is less error 
> prone.

Definitely agreed.

>
>> > if we talk about the user interface.. imho it would be nice that this is a 
>> > compiler warning. the analysis is quick and there should be little noise.
>> I'm not certain about the performance of the analysis (I suspect it's
>>  relatively cheap though), but we do not usually want off-by-default
>>  warnings in the frontend, and I suspect that this analysis would have
>>  to be off by default due to the chattiness on well-formed code.
>
> hmm.. I believe that this is common practice. I can see that people want to 
> turn it off for legacy code though.

Legacy code is what I'm most concerned about. These diagnostics aren't
warning the user that their code is incorrect in some way, they're
it's showing where the code could be safer with improved
const-correctness. If it's producing 30 new diagnostics per project on
average, that's a lot of noise for very little benefit to those
projects. We usually put those sort of diagnostics in other parts of
the compiler (analysis, clang-tidy, etc). Experience has shown that
off-by-default warnings generally never get turned on.

> but we can look at the warnings on real code and discuss those. I have 
> results from Clang.
>
>
> 
> Comment at: lib/Sema/SemaDecl.cpp:10334
> @@ +10333,3 @@
> +  continue;
> +// To start with we don't warn about structs.
> +if (T->getPointeeType().getTypePtr()->isRecordType())
> 
> aaron.ballman wrote:
>> This seems *really* limiting (especially for C++ code), why the restriction?
> 2 reasons...
>
> 1. I don't think we should warn for structs whenever it's possible to make 
> them const.
>
> Example code:
>
> struct FRED { char *str; };
> void f(struct FRED *fred) {
> strcpy(fred->str, "abc");
> }
>
> fred can be const but imho we should not warn about this. Imho it would be 
> misleading to make fred const.

Agreed. However, as a counter-point:

struct S {
  void f() const;
};

void f(S *s) {
  s->f();
}

> 2. I wanted to keep the scope of this checker limited to start with. If we 
> want to warn about structs also I need to write much more code in the 
> MarkWritten etc.

That's reasonable, but it would be good to put some test cases in as a
FIXME for code that you think should be handled eventually, but isn't
handled currently.

~Aaron

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

Re: clang-tidy unit tests

[Alexander Kornienko via cfe-commits]

Looks like we need to call both ClangTidyContext::setASTContext,
ClangTidyCheck::registerMatchers and ClangTidyContext::setCurrentFile
in TestClangTidyAction::CreateASTConsumer. Maybe something else will need
to move there as well.

-- Alex

On Wed, Sep 2, 2015 at 4:09 PM, Aaron Ballman 
wrote:

> I ran into an issue with the runCheckOnCode() function from
> clang-tidy's unit tests, and I am not familiar enough with the way
> tool invocation works to suggest a correct fix.
>
> ClangTidyContext::setASTContext() is what sets up the language options
> for the ClangTidyContext object currently. However, runCheckOnCode()
> does not end up calling setASTContext() at any point. So when unit
> testing code, it behaves differently than when running clang-tidy.
>
> I am guessing that setting the language options should be separated
> from setting the AST context, however, I'm not certain of the proper
> way to accomplish this. As best I can tell, by the time
> registerMatchers() is called in runCheckOnCode(), we don't have access
> to a LangOptions object that's valid.
>
> Suggestions?
>
> ~Aaron
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: clang-tidy unit tests

[Aaron Ballman via cfe-commits]

On Wed, Sep 2, 2015 at 10:44 AM, Alexander Kornienko  wrote:
> Looks like we need to call both ClangTidyContext::setASTContext,
> ClangTidyCheck::registerMatchers and ClangTidyContext::setCurrentFile in
> TestClangTidyAction::CreateASTConsumer. Maybe something else will need to
> move there as well.

I'll give it a shot, thank you!

~Aaron

>
> -- Alex
>
>
> On Wed, Sep 2, 2015 at 4:09 PM, Aaron Ballman 
> wrote:
>>
>> I ran into an issue with the runCheckOnCode() function from
>> clang-tidy's unit tests, and I am not familiar enough with the way
>> tool invocation works to suggest a correct fix.
>>
>> ClangTidyContext::setASTContext() is what sets up the language options
>> for the ClangTidyContext object currently. However, runCheckOnCode()
>> does not end up calling setASTContext() at any point. So when unit
>> testing code, it behaves differently than when running clang-tidy.
>>
>> I am guessing that setting the language options should be separated
>> from setting the AST context, however, I'm not certain of the proper
>> way to accomplish this. As best I can tell, by the time
>> registerMatchers() is called in runCheckOnCode(), we don't have access
>> to a LangOptions object that's valid.
>>
>> Suggestions?
>>
>> ~Aaron
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

[Sean Eveson via cfe-commits]

seaneveson added a comment.

In http://reviews.llvm.org/D12358#237099, @krememek wrote:

> To get the conservative invalidation that Anna suggests (actually, a little 
> less conservative), I think you can just pass in a two MemRegions as the 
> input to that method and get a new ProgramState with the appropriate regions 
> invalidated.


Thank you for the suggestion. Unfortunately nothing seems to get invalidated 
when I call invalidateRegions, in the following code:

  const StackFrameContext *STC = LCtx->getCurrentStackFrame();
  MemRegionManager &MRMgr = svalBuilder.getRegionManager();
  const MemRegion *Regions[] = {
MRMgr.getStackLocalsRegion(STC),
MRMgr.getStackArgumentsRegion(STC),
MRMgr.getGlobalsRegion()
  };
  ProgramStateRef State;
  State = PrevState->invalidateRegions(Regions, Cond, BlockCount, LCtx, false);

Do you have any suggestions on what I have done wrong?

If there is no simple way to invalidate all three regions I will work on 
implementing something like the following:

In http://reviews.llvm.org/D12358#234975, @krememek wrote:

> A general scheme for widening, which can just be invalidation of anything 
> touched within a loop.  I think that can be done by having an algorithm that 
> does an AST walk, and looks at all variables whose lvalues are taken (used to 
> store values or otherwise get their address using '&') or anything 
> passed-by-reference to a function.  That set of VarDecls can be cached in a 
> side table, mapping ForStmt*'s (other loops as well) to the set of VarDecls 
> that are invalidated.  Those could then be passed to something that does the 
> invalidation using the currently invalidation logic we have in place.  The 
> invalidation logic should also handle checker state, which also needs to get 
> invalidated alongside variable values.



http://reviews.llvm.org/D12358



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

r246657 - [OpenMP] Make helper functoin static. NFC.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Wed Sep  2 10:31:05 2015
New Revision: 246657

URL: http://llvm.org/viewvc/llvm-project?rev=246657&view=rev
Log:
[OpenMP] Make helper functoin static. NFC.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=246657&r1=246656&r2=246657&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Wed Sep  2 10:31:05 2015
@@ -2091,7 +2091,7 @@ emitTaskPrivateMappingFunction(CodeGenMo
   return TaskPrivatesMap;
 }
 
-llvm::Value *getTypeSize(CodeGenFunction &CGF, QualType Ty) {
+static llvm::Value *getTypeSize(CodeGenFunction &CGF, QualType Ty) {
   auto &C = CGF.getContext();
   llvm::Value *Size;
   auto SizeInChars = C.getTypeSizeInChars(Ty);


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

r246659 - [MS ABI] Number unnamed TagDecls which aren't externally visible

[David Majnemer via cfe-commits]

Author: majnemer
Date: Wed Sep  2 10:50:38 2015
New Revision: 246659

URL: http://llvm.org/viewvc/llvm-project?rev=246659&view=rev
Log:
[MS ABI] Number unnamed TagDecls which aren't externally visible

TagDecls (structs, enums, etc.) may have the same name for linkage
purposes of one another; to disambiguate, we add a number to the mangled
named.  However, we didn't do this if the TagDecl has a pseudo-name for
linkage purposes (it was defined alongside a DeclaratorDecl or a
TypeNameDecl).

This fixes PR24651.

Modified:
cfe/trunk/lib/AST/MicrosoftMangle.cpp
cfe/trunk/test/CodeGenCXX/mangle-ms-cxx11.cpp

Modified: cfe/trunk/lib/AST/MicrosoftMangle.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftMangle.cpp?rev=246659&r1=246658&r2=246659&view=diff
==
--- cfe/trunk/lib/AST/MicrosoftMangle.cpp (original)
+++ cfe/trunk/lib/AST/MicrosoftMangle.cpp Wed Sep  2 10:50:38 2015
@@ -180,7 +180,9 @@ public:
 
 // Anonymous tags are already numbered.
 if (const TagDecl *Tag = dyn_cast(ND)) {
-  if (Tag->getName().empty() && !Tag->getTypedefNameForAnonDecl())
+  if (!Tag->hasNameForLinkage() &&
+  !getASTContext().getDeclaratorForUnnamedTagDecl(Tag) &&
+  !getASTContext().getTypedefNameForUnnamedTagDecl(Tag))
 return false;
 }
 

Modified: cfe/trunk/test/CodeGenCXX/mangle-ms-cxx11.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/mangle-ms-cxx11.cpp?rev=246659&r1=246658&r2=246659&view=diff
==
--- cfe/trunk/test/CodeGenCXX/mangle-ms-cxx11.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/mangle-ms-cxx11.cpp Wed Sep  2 10:50:38 2015
@@ -261,3 +261,19 @@ struct B {
 void f(decltype(B::e)) {}
 // CHECK-DAG: @"\01?f@UnnamedType@@YAXPAW4@?$B@H@1@@Z
 }
+
+namespace PR24651 {
+template 
+void f(T) {}
+
+void g() {
+  enum {} E;
+  f(E);
+  {
+enum {} E;
+f(E);
+  }
+}
+// CHECK-DAG: 
@"\01??$f@W4@?1??g@PR24651@@YAXXZ@@PR24651@@YAXW4@?1??g@0@YAXXZ@@Z"
+// CHECK-DAG: 
@"\01??$f@W4@?2??g@PR24651@@YAXXZ@@PR24651@@YAXW4@?2??g@0@YAXXZ@@Z"
+}


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

Re: r246659 - [MS ABI] Number unnamed TagDecls which aren't externally visible

[Rafael Espíndola via cfe-commits]

Thanks!

On 2 September 2015 at 11:50, David Majnemer via cfe-commits
 wrote:
> Author: majnemer
> Date: Wed Sep  2 10:50:38 2015
> New Revision: 246659
>
> URL: http://llvm.org/viewvc/llvm-project?rev=246659&view=rev
> Log:
> [MS ABI] Number unnamed TagDecls which aren't externally visible
>
> TagDecls (structs, enums, etc.) may have the same name for linkage
> purposes of one another; to disambiguate, we add a number to the mangled
> named.  However, we didn't do this if the TagDecl has a pseudo-name for
> linkage purposes (it was defined alongside a DeclaratorDecl or a
> TypeNameDecl).
>
> This fixes PR24651.
>
> Modified:
> cfe/trunk/lib/AST/MicrosoftMangle.cpp
> cfe/trunk/test/CodeGenCXX/mangle-ms-cxx11.cpp
>
> Modified: cfe/trunk/lib/AST/MicrosoftMangle.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftMangle.cpp?rev=246659&r1=246658&r2=246659&view=diff
> ==
> --- cfe/trunk/lib/AST/MicrosoftMangle.cpp (original)
> +++ cfe/trunk/lib/AST/MicrosoftMangle.cpp Wed Sep  2 10:50:38 2015
> @@ -180,7 +180,9 @@ public:
>
>  // Anonymous tags are already numbered.
>  if (const TagDecl *Tag = dyn_cast(ND)) {
> -  if (Tag->getName().empty() && !Tag->getTypedefNameForAnonDecl())
> +  if (!Tag->hasNameForLinkage() &&
> +  !getASTContext().getDeclaratorForUnnamedTagDecl(Tag) &&
> +  !getASTContext().getTypedefNameForUnnamedTagDecl(Tag))
>  return false;
>  }
>
>
> Modified: cfe/trunk/test/CodeGenCXX/mangle-ms-cxx11.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/mangle-ms-cxx11.cpp?rev=246659&r1=246658&r2=246659&view=diff
> ==
> --- cfe/trunk/test/CodeGenCXX/mangle-ms-cxx11.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/mangle-ms-cxx11.cpp Wed Sep  2 10:50:38 2015
> @@ -261,3 +261,19 @@ struct B {
>  void f(decltype(B::e)) {}
>  // CHECK-DAG: @"\01?f@UnnamedType@@YAXPAW4@?$B@H@1@@Z
>  }
> +
> +namespace PR24651 {
> +template 
> +void f(T) {}
> +
> +void g() {
> +  enum {} E;
> +  f(E);
> +  {
> +enum {} E;
> +f(E);
> +  }
> +}
> +// CHECK-DAG: 
> @"\01??$f@W4@?1??g@PR24651@@YAXXZ@@PR24651@@YAXW4@?1??g@0@YAXXZ@@Z"
> +// CHECK-DAG: 
> @"\01??$f@W4@?2??g@PR24651@@YAXXZ@@PR24651@@YAXW4@?2??g@0@YAXXZ@@Z"
> +}
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r246660 - Move some more functionality into the AST consumer creation factory function, before registering matchers with the MatchFinder object. This allows us to set the language

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Sep  2 11:04:15 2015
New Revision: 246660

URL: http://llvm.org/viewvc/llvm-project?rev=246660&view=rev
Log:
Move some more functionality into the AST consumer creation factory function, 
before registering matchers with the MatchFinder object. This allows us to set 
the language options for the ClangTidyContext object appropriately so that they 
can be used from registerMatchers(), and more closely models the way the 
clang-tidy tool works.

Modified:
clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h

Modified: clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h?rev=246660&r1=246659&r2=246660&view=diff
==
--- clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h (original)
+++ clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h Wed Sep  2 
11:04:15 2015
@@ -33,6 +33,10 @@ private:
   std::unique_ptr CreateASTConsumer(CompilerInstance &Compiler,
  StringRef File) override {
 Context.setSourceManager(&Compiler.getSourceManager());
+Context.setCurrentFile(File);
+Context.setASTContext(&Compiler.getASTContext());
+
+Check.registerMatchers(&Finder);
 Check.registerPPCallbacks(Compiler);
 return Finder.newASTConsumer();
   }
@@ -56,9 +60,6 @@ runCheckOnCode(StringRef Code, std::vect
   ClangTidyGlobalOptions(), Options));
   ClangTidyDiagnosticConsumer DiagConsumer(Context);
   T Check("test-check", &Context);
-  ast_matchers::MatchFinder Finder;
-  Check.registerMatchers(&Finder);
-  Context.setCurrentFile(Filename.str());
 
   std::vector ArgCXX11(1, "clang-tidy");
   ArgCXX11.push_back("-fsyntax-only");
@@ -66,6 +67,8 @@ runCheckOnCode(StringRef Code, std::vect
   ArgCXX11.push_back("-Iinclude");
   ArgCXX11.insert(ArgCXX11.end(), ExtraArgs.begin(), ExtraArgs.end());
   ArgCXX11.push_back(Filename.str());
+
+  ast_matchers::MatchFinder Finder;
   llvm::IntrusiveRefCntPtr Files(
   new FileManager(FileSystemOptions()));
   tooling::ToolInvocation Invocation(


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

[clang-tools-extra] r246661 - Disable clang-tidy readability checkers when not compiling in C++ mode. None of the checkers require additional testing as the tests will not compile for other languages

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Sep  2 11:05:21 2015
New Revision: 246661

URL: http://llvm.org/viewvc/llvm-project?rev=246661&view=rev
Log:
Disable clang-tidy readability checkers when not compiling in C++ mode. None of 
the checkers require additional testing as the tests will not compile for other 
languages or modes, or the checkers would never match a valid construct.

Modified:
clang-tools-extra/trunk/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/RedundantStringCStrCheck.cpp

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/ContainerSizeEmptyCheck.cpp?rev=246661&r1=246660&r2=246661&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/ContainerSizeEmptyCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/ContainerSizeEmptyCheck.cpp 
Wed Sep  2 11:05:21 2015
@@ -54,6 +54,11 @@ ContainerSizeEmptyCheck::ContainerSizeEm
 : ClangTidyCheck(Name, Context) {}
 
 void ContainerSizeEmptyCheck::registerMatchers(MatchFinder *Finder) {
+  // Only register the matchers for C++; the functionality currently does not
+  // provide any benefit to other languages, despite being benign.
+  if (!getLangOpts().CPlusPlus)
+return;
+
   const auto WrongUse = anyOf(
   hasParent(
   binaryOperator(

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp?rev=246661&r1=246660&r2=246661&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/NamespaceCommentCheck.cpp 
Wed Sep  2 11:05:21 2015
@@ -34,7 +34,10 @@ void NamespaceCommentCheck::storeOptions
 }
 
 void NamespaceCommentCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(namespaceDecl().bind("namespace"), this);
+  // Only register the matchers for C++; the functionality currently does not
+  // provide any benefit to other languages, despite being benign.
+  if (getLangOpts().CPlusPlus)
+Finder->addMatcher(namespaceDecl().bind("namespace"), this);
 }
 
 static bool locationsInSameFile(const SourceManager &Sources,

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.cpp?rev=246661&r1=246660&r2=246661&view=diff
==
--- 
clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.cpp 
(original)
+++ 
clang-tools-extra/trunk/clang-tidy/readability/RedundantSmartptrGetCheck.cpp 
Wed Sep  2 11:05:21 2015
@@ -74,6 +74,11 @@ void registerMatchersForGetEquals(MatchF
 }  // namespace
 
 void RedundantSmartptrGetCheck::registerMatchers(MatchFinder *Finder) {
+  // Only register the matchers for C++; the functionality currently does not
+  // provide any benefit to other languages, despite being benign.
+  if (!getLangOpts().CPlusPlus)
+return;
+
   registerMatchersForGetArrowStart(Finder, this);
   registerMatchersForGetEquals(Finder, this);
 }

Modified: 
clang-tools-extra/trunk/clang-tidy/readability/RedundantStringCStrCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/RedundantStringCStrCheck.cpp?rev=246661&r1=246660&r2=246661&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/RedundantStringCStrCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/readability/RedundantStringCStrCheck.cpp 
Wed Sep  2 11:05:21 2015
@@ -80,6 +80,11 @@ namespace readability {
 
 void RedundantStringCStrCheck::registerMatchers(
 ast_matchers::MatchFinder *Finder) {
+  // Only register the matchers for C++; the functionality currently does not
+  // provide any benefit to other languages, despite being benign.
+  if (!getLangOpts().CPlusPlus)
+return;
+
   Finder->addMatcher(
   constructExpr(
   hasDeclaration(methodDecl(hasName(StringConstructor))),


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

[clang-tools-extra] r246663 - Disable clang-tidy Google checkers when not compiling in C++ mode. None of the checkers require additional testing as the tests will not compile for other languages or mo

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Sep  2 11:20:42 2015
New Revision: 246663

URL: http://llvm.org/viewvc/llvm-project?rev=246663&view=rev
Log:
Disable clang-tidy Google checkers when not compiling in C++ mode. None of the 
checkers require additional testing as the tests will not compile for other 
languages or modes, or the checkers would never match a valid construct.

Modified:
clang-tools-extra/trunk/clang-tidy/google/ExplicitConstructorCheck.cpp
clang-tools-extra/trunk/clang-tidy/google/ExplicitMakePairCheck.cpp
clang-tools-extra/trunk/clang-tidy/google/IntegerTypesCheck.cpp
clang-tools-extra/trunk/clang-tidy/google/OverloadedUnaryAndCheck.cpp
clang-tools-extra/trunk/clang-tidy/google/StringReferenceMemberCheck.cpp
clang-tools-extra/trunk/clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp
clang-tools-extra/trunk/clang-tidy/google/UsingNamespaceDirectiveCheck.cpp

Modified: clang-tools-extra/trunk/clang-tidy/google/ExplicitConstructorCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/ExplicitConstructorCheck.cpp?rev=246663&r1=246662&r2=246663&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/google/ExplicitConstructorCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/google/ExplicitConstructorCheck.cpp Wed 
Sep  2 11:20:42 2015
@@ -20,8 +20,11 @@ namespace tidy {
 namespace google {
 
 void ExplicitConstructorCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(constructorDecl(unless(isInstantiated())).bind("ctor"),
- this);
+  // Only register the matchers for C++; the functionality currently does not
+  // provide any benefit to other languages, despite being benign.
+  if (getLangOpts().CPlusPlus)
+Finder->addMatcher(constructorDecl(unless(isInstantiated())).bind("ctor"),
+   this);
 }
 
 // Looks for the token matching the predicate and returns the range of the 
found

Modified: clang-tools-extra/trunk/clang-tidy/google/ExplicitMakePairCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/ExplicitMakePairCheck.cpp?rev=246663&r1=246662&r2=246663&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/google/ExplicitMakePairCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/google/ExplicitMakePairCheck.cpp Wed Sep 
 2 11:20:42 2015
@@ -27,6 +27,11 @@ namespace build {
 
 void
 ExplicitMakePairCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
+  // Only register the matchers for C++; the functionality currently does not
+  // provide any benefit to other languages, despite being benign.
+  if (!getLangOpts().CPlusPlus)
+return;
+
   // Look for std::make_pair with explicit template args. Ignore calls in
   // templates.
   Finder->addMatcher(

Modified: clang-tools-extra/trunk/clang-tidy/google/IntegerTypesCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/IntegerTypesCheck.cpp?rev=246663&r1=246662&r2=246663&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/google/IntegerTypesCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/google/IntegerTypesCheck.cpp Wed Sep  2 
11:20:42 2015
@@ -22,15 +22,12 @@ namespace runtime {
 using namespace ast_matchers;
 
 void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
-  // Find all TypeLocs.
-  Finder->addMatcher(typeLoc().bind("tl"), this);
+  // Find all TypeLocs. The relevant Style Guide rule only applies to C++.
+  if (getLangOpts().CPlusPlus)
+Finder->addMatcher(typeLoc().bind("tl"), this);
 }
 
 void IntegerTypesCheck::check(const MatchFinder::MatchResult &Result) {
-  // The relevant Style Guide rule only applies to C++.
-  if (!Result.Context->getLangOpts().CPlusPlus)
-return;
-
   auto TL = *Result.Nodes.getNodeAs("tl");
   SourceLocation Loc = TL.getLocStart();
 

Modified: clang-tools-extra/trunk/clang-tidy/google/OverloadedUnaryAndCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/OverloadedUnaryAndCheck.cpp?rev=246663&r1=246662&r2=246663&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/google/OverloadedUnaryAndCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/google/OverloadedUnaryAndCheck.cpp Wed 
Sep  2 11:20:42 2015
@@ -21,6 +21,11 @@ namespace runtime {
 
 void
 OverloadedUnaryAndCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
+  // Only register the matchers for C++; the functionality currently does not
+  // provide any benefit to other languages, despite being benign.
+  if (!getLangOpts().CPlusPlus)
+return;
+
   // Match unary methods that overload operator&.
   Finder->addMatcher(methodDecl(parameterC

clang-tools-extra code owners

[Aaron Ballman via cfe-commits]

I happened to notice that CODE_OWNERS.TXT for clang-tools-extra is
rather sparse. These tools are starting to get more attention from the
community, and I am wondering whether it would make sense to populate
that somewhat? If so, I would nominate:

Alexander Kornienko for clang-tidy
Peter Collingbourne for clang-query
Manuel Klimek for clang-rename, and clang-tools-extra as a whole

I'm basing these nominations on who is actively maintaining the tools already.

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

Re: clang-tools-extra code owners

[Alexander Kornienko via cfe-commits]

LG for clang-tidy.

On Wed, Sep 2, 2015 at 6:38 PM, Aaron Ballman 
wrote:

> I happened to notice that CODE_OWNERS.TXT for clang-tools-extra is
> rather sparse. These tools are starting to get more attention from the
> community, and I am wondering whether it would make sense to populate
> that somewhat? If so, I would nominate:
>
> Alexander Kornienko for clang-tidy
> Peter Collingbourne for clang-query
> Manuel Klimek for clang-rename, and clang-tools-extra as a whole
>
> I'm basing these nominations on who is actively maintaining the tools
> already.
>
> ~Aaron
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: clang-tools-extra code owners

[Manuel Klimek via cfe-commits]

I think that basically encodes what the current state is, and we should
have kept that file in a better shape. +Edwin in case he still has a stake
in clang-modernize, or knows who might have.

On Wed, Sep 2, 2015 at 6:38 PM Aaron Ballman  wrote:

> I happened to notice that CODE_OWNERS.TXT for clang-tools-extra is
> rather sparse. These tools are starting to get more attention from the
> community, and I am wondering whether it would make sense to populate
> that somewhat? If so, I would nominate:
>
> Alexander Kornienko for clang-tidy
> Peter Collingbourne for clang-query
> Manuel Klimek for clang-rename, and clang-tools-extra as a whole
>
> I'm basing these nominations on who is actively maintaining the tools
> already.
>
> ~Aaron
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: clang-tools-extra code owners

[Richard Smith via cfe-commits]

On Sep 2, 2015 9:38 AM, "Aaron Ballman via cfe-commits" <
cfe-commits@lists.llvm.org> wrote:
>
> I happened to notice that CODE_OWNERS.TXT for clang-tools-extra is
> rather sparse. These tools are starting to get more attention from the
> community, and I am wondering whether it would make sense to populate
> that somewhat? If so, I would nominate:
>
> Alexander Kornienko for clang-tidy
> Peter Collingbourne for clang-query
> Manuel Klimek for clang-rename, and clang-tools-extra as a whole

These make sense to me.

> I'm basing these nominations on who is actively maintaining the tools
already.
>
> ~Aaron
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r246610 - Migrate the target attribute parsing code into an extension off of

[Eric Christopher via cfe-commits]

On Wed, Sep 2, 2015 at 7:24 AM Aaron Ballman  wrote:

> A few nits in addition to what Ben pointed out...
>
> On Tue, Sep 1, 2015 at 8:12 PM, Eric Christopher via cfe-commits
>  wrote:
> > Author: echristo
> > Date: Tue Sep  1 19:12:02 2015
> > New Revision: 246610
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=246610&view=rev
> > Log:
> > Migrate the target attribute parsing code into an extension off of
> > the main attribute and cache the results so we don't have to parse
> > a single attribute more than once.
> >
> > This reapplies r246596 with a fix for an uninitialized class member,
> > and a couple of cleanups and formatting changes.
> >
> > Modified:
> > cfe/trunk/include/clang/Basic/Attr.td
> > cfe/trunk/lib/CodeGen/CGCall.cpp
> >
> > Modified: cfe/trunk/include/clang/Basic/Attr.td
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=246610&r1=246609&r2=246610&view=diff
> >
> ==
> > --- cfe/trunk/include/clang/Basic/Attr.td (original)
> > +++ cfe/trunk/include/clang/Basic/Attr.td Tue Sep  1 19:12:02 2015
> > @@ -1315,9 +1315,52 @@ def Pascal : InheritableAttr {
> >
> >  def Target : InheritableAttr {
> >let Spellings = [GCC<"target">];
> > -  let Args = [StringArgument<"features">];
> > +  let Args = [StringArgument<"featuresStr">];
> >let Subjects = SubjectList<[Function], ErrorDiag>;
> >let Documentation = [TargetDocs];
> > +  let AdditionalMembers = [{
> > +StringRef CPU;
> > +std::vector Features;
> > +bool Parsed = false;
>
> Additional members are all public, can you privatize the data members?
> Then you can make them mutable and fix the const-correctness
> regression. ;-)
>
>
Heh. Sure. I don't really like mutable unless there's no other choice, but
I can do it here, however, it also sounds like you don't want them to be
data members at all? Did you want me to just reparse them or try to put a
cache somewhere?


> > +StringRef getCPU() {
> > +  if (!Parsed)
> > +parse();
> > +  return CPU;
> > +}
> > +std::vector &getFeatures() {
> > +  if (!Parsed)
> > +parse();
> > +  return Features;
> > +}
> > +void parse() {
> > +  SmallVector AttrFeatures;
> > +  getFeaturesStr().split(AttrFeatures, ",");
> > +
> > +  // Grab the various features and prepend a "+" to turn on the
> feature to
> > +  // the backend and add them to our existing set of features.
> > +  for (auto &Feature : AttrFeatures) {
> > +// Go ahead and trim whitespace rather than either erroring or
> > +// accepting it weirdly.
> > +Feature = Feature.trim();
> > +
> > +// We don't support cpu tuning this way currently.
> > +// TODO: Support the fpmath option. It will require checking
> > +// overall feature validity for the function with the rest of
> the
> > +// attributes on the function.
> > +if (Feature.startswith("fpmath=") ||
> Feature.startswith("tune="))
> > + continue;
> > +
> > +// While we're here iterating check for a different target cpu.
> > +if (Feature.startswith("arch="))
> > +  CPU = Feature.split("=").second.trim();
> > +else if (Feature.startswith("no-"))
> > +  Features.push_back("-" + Feature.split("-").second.str());
> > +else
> > +  Features.push_back("+" + Feature.str());
> > +  }
> > +  Parsed = true;
> > +}
> > +  }];
> >  }
> >
> >  def TransparentUnion : InheritableAttr {
> >
> > Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=246610&r1=246609&r2=246610&view=diff
> >
> ==
> > --- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
> > +++ cfe/trunk/lib/CodeGen/CGCall.cpp Tue Sep  1 19:12:02 2015
> > @@ -1499,40 +1499,19 @@ void CodeGenModule::ConstructAttributeLi
> >  const FunctionDecl *FD = dyn_cast_or_null(TargetDecl);
> >  if (FD && FD->hasAttr()) {
> >llvm::StringMap FeatureMap;
> > -  const auto *TD = FD->getAttr();
> > +  auto *TD = FD->getAttr();
> >
> >// Make a copy of the features as passed on the command line.
> >std::vector FnFeatures =
> >getTarget().getTargetOpts().FeaturesAsWritten;
> >
> > -  // Grab the target attribute string.
> > -  StringRef FeaturesStr = TD->getFeatures();
> > -  SmallVector AttrFeatures;
> > -  FeaturesStr.split(AttrFeatures, ",");
> > +  std::vector &AttrFeatures = TD->getFeatures();
> > +  std::copy(AttrFeatures.begin(), AttrFeatures.end(),
> > +std::back_inserter(FnFeatures));
> >
> > -  // Grab the various features and prepend a "+" to turn on the
> feature to
> > -  // the backend and add them to our existing set of features.
> > -  for (auto &Feature : Att

Re: r246610 - Migrate the target attribute parsing code into an extension off of

[Aaron Ballman via cfe-commits]

On Wed, Sep 2, 2015 at 1:07 PM, Eric Christopher  wrote:
>
>
> On Wed, Sep 2, 2015 at 7:24 AM Aaron Ballman  wrote:
>>
>> A few nits in addition to what Ben pointed out...
>>
>> On Tue, Sep 1, 2015 at 8:12 PM, Eric Christopher via cfe-commits
>>  wrote:
>> > Author: echristo
>> > Date: Tue Sep  1 19:12:02 2015
>> > New Revision: 246610
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=246610&view=rev
>> > Log:
>> > Migrate the target attribute parsing code into an extension off of
>> > the main attribute and cache the results so we don't have to parse
>> > a single attribute more than once.
>> >
>> > This reapplies r246596 with a fix for an uninitialized class member,
>> > and a couple of cleanups and formatting changes.
>> >
>> > Modified:
>> > cfe/trunk/include/clang/Basic/Attr.td
>> > cfe/trunk/lib/CodeGen/CGCall.cpp
>> >
>> > Modified: cfe/trunk/include/clang/Basic/Attr.td
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=246610&r1=246609&r2=246610&view=diff
>> >
>> > ==
>> > --- cfe/trunk/include/clang/Basic/Attr.td (original)
>> > +++ cfe/trunk/include/clang/Basic/Attr.td Tue Sep  1 19:12:02 2015
>> > @@ -1315,9 +1315,52 @@ def Pascal : InheritableAttr {
>> >
>> >  def Target : InheritableAttr {
>> >let Spellings = [GCC<"target">];
>> > -  let Args = [StringArgument<"features">];
>> > +  let Args = [StringArgument<"featuresStr">];
>> >let Subjects = SubjectList<[Function], ErrorDiag>;
>> >let Documentation = [TargetDocs];
>> > +  let AdditionalMembers = [{
>> > +StringRef CPU;
>> > +std::vector Features;
>> > +bool Parsed = false;
>>
>> Additional members are all public, can you privatize the data members?
>> Then you can make them mutable and fix the const-correctness
>> regression. ;-)
>>
>
> Heh. Sure. I don't really like mutable unless there's no other choice, but I
> can do it here, however, it also sounds like you don't want them to be data
> members at all? Did you want me to just reparse them or try to put a cache
> somewhere?

I'm fine with them being data members. The usual pattern is:

let AdditionalMembers = [{
private:
  

public:
  
}];

>
>>
>> > +StringRef getCPU() {
>> > +  if (!Parsed)
>> > +parse();
>> > +  return CPU;
>> > +}
>> > +std::vector &getFeatures() {
>> > +  if (!Parsed)
>> > +parse();
>> > +  return Features;
>> > +}
>> > +void parse() {
>> > +  SmallVector AttrFeatures;
>> > +  getFeaturesStr().split(AttrFeatures, ",");
>> > +
>> > +  // Grab the various features and prepend a "+" to turn on the
>> > feature to
>> > +  // the backend and add them to our existing set of features.
>> > +  for (auto &Feature : AttrFeatures) {
>> > +// Go ahead and trim whitespace rather than either erroring or
>> > +// accepting it weirdly.
>> > +Feature = Feature.trim();
>> > +
>> > +// We don't support cpu tuning this way currently.
>> > +// TODO: Support the fpmath option. It will require checking
>> > +// overall feature validity for the function with the rest of
>> > the
>> > +// attributes on the function.
>> > +if (Feature.startswith("fpmath=") ||
>> > Feature.startswith("tune="))
>> > + continue;
>> > +
>> > +// While we're here iterating check for a different target cpu.
>> > +if (Feature.startswith("arch="))
>> > +  CPU = Feature.split("=").second.trim();
>> > +else if (Feature.startswith("no-"))
>> > +  Features.push_back("-" + Feature.split("-").second.str());
>> > +else
>> > +  Features.push_back("+" + Feature.str());
>> > +  }
>> > +  Parsed = true;
>> > +}
>> > +  }];
>> >  }
>> >
>> >  def TransparentUnion : InheritableAttr {
>> >
>> > Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=246610&r1=246609&r2=246610&view=diff
>> >
>> > ==
>> > --- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
>> > +++ cfe/trunk/lib/CodeGen/CGCall.cpp Tue Sep  1 19:12:02 2015
>> > @@ -1499,40 +1499,19 @@ void CodeGenModule::ConstructAttributeLi
>> >  const FunctionDecl *FD =
>> > dyn_cast_or_null(TargetDecl);
>> >  if (FD && FD->hasAttr()) {
>> >llvm::StringMap FeatureMap;
>> > -  const auto *TD = FD->getAttr();
>> > +  auto *TD = FD->getAttr();
>> >
>> >// Make a copy of the features as passed on the command line.
>> >std::vector FnFeatures =
>> >getTarget().getTargetOpts().FeaturesAsWritten;
>> >
>> > -  // Grab the target attribute string.
>> > -  StringRef FeaturesStr = TD->getFeatures();
>> > -  SmallVector AttrFeatures;
>> > -  FeaturesStr.split(AttrFeatures, ",");
>> > +  std::vector &AttrFeatures = TD->g

Re: clang-tools-extra code owners

[Edwin Vane via cfe-commits]

I don't know of anyone who has a stake in clang-modernize and I probably
should be removed from the code-owner role.

On 2 September 2015 at 12:48, Manuel Klimek  wrote:

> I think that basically encodes what the current state is, and we should
> have kept that file in a better shape. +Edwin in case he still has a stake
> in clang-modernize, or knows who might have.
>
> On Wed, Sep 2, 2015 at 6:38 PM Aaron Ballman 
> wrote:
>
>> I happened to notice that CODE_OWNERS.TXT for clang-tools-extra is
>> rather sparse. These tools are starting to get more attention from the
>> community, and I am wondering whether it would make sense to populate
>> that somewhat? If so, I would nominate:
>>
>> Alexander Kornienko for clang-tidy
>> Peter Collingbourne for clang-query
>> Manuel Klimek for clang-rename, and clang-tools-extra as a whole
>>
>> I'm basing these nominations on who is actively maintaining the tools
>> already.
>>
>> ~Aaron
>>
>


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

r246680 - [modules] Don't waste time reading in the names the module file writer gave to blocks. We don't need these names, and decoding the corresponding bitcode has a significant cost.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Wed Sep  2 12:45:54 2015
New Revision: 246680

URL: http://llvm.org/viewvc/llvm-project?rev=246680&view=rev
Log:
[modules] Don't waste time reading in the names the module file writer gave to 
blocks. We don't need these names, and decoding the corresponding bitcode has a 
significant cost.

Modified:
cfe/trunk/lib/Serialization/ASTReader.cpp

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=246680&r1=246679&r2=246680&view=diff
==
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Sep  2 12:45:54 2015
@@ -3710,14 +3710,7 @@ ASTReader::ReadASTCore(StringRef FileNam
   break;
 }
 
-// We only know the control subblock ID.
 switch (Entry.ID) {
-case llvm::bitc::BLOCKINFO_BLOCK_ID:
-  if (Stream.ReadBlockInfoBlock()) {
-Error("malformed BlockInfoBlock in AST file");
-return Failure;
-  }
-  break;
 case CONTROL_BLOCK_ID:
   HaveReadControlBlock = true;
   switch (ReadControlBlock(F, Loaded, ImportedBy, ClientLoadCapabilities)) 
{
@@ -3744,6 +3737,7 @@ ASTReader::ReadASTCore(StringRef FileNam
   case HadErrors: return HadErrors;
   }
   break;
+
 case AST_BLOCK_ID:
   if (!HaveReadControlBlock) {
 if ((ClientLoadCapabilities & ARR_VersionMismatch) == 0)


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

Re: [PATCH] D10586: Add HSAIL support

[Pekka Jääskeläinen via cfe-commits]

pekka.jaaskelainen added a comment.

Is there a matching HLC-HSAIL branch for the latest version of the patch? It 
doesn't work with with hsail-stable-3.7 anymore.


http://reviews.llvm.org/D10586



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

Re: [PATCH] D12123: [analyzer] Skip Pre/Post handlers for ObjC calls when receiver is nil.

[Devin Coughlin via cfe-commits]

dcoughlin updated this revision to Diff 33836.
dcoughlin added a comment.

I've updated ExprEngine::VisitObjCMessage to create a pre-statement program 
point node for the ObjCMessageExpr when the receiver is definitely nil and 
changed the MessageNil checker handlers to run on a post-statement program 
point node. This makes sure we always have both a pre- and a post-statement 
program point for the message expression -- an invariant the path diagnostic 
machinery relies upon.


http://reviews.llvm.org/D12123

Files:
  include/clang/StaticAnalyzer/Core/Checker.h
  include/clang/StaticAnalyzer/Core/CheckerManager.h
  lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
  lib/StaticAnalyzer/Checkers/CheckerDocumentation.cpp
  lib/StaticAnalyzer/Core/CheckerManager.cpp
  lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
  test/Analysis/NSContainers.m
  test/Analysis/objc-message.m

Index: test/Analysis/objc-message.m
===
--- /dev/null
+++ test/Analysis/objc-message.m
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-store=region -verify -Wno-objc-root-class %s
+
+extern void clang_analyzer_warnIfReached();
+void clang_analyzer_eval(int);
+
+@interface SomeClass
+-(id)someMethodWithReturn;
+-(void)someMethod;
+@end
+
+void consistencyOfReturnWithNilReceiver(SomeClass *o) {
+  id result = [o someMethodWithReturn];
+  if (result) {
+if (!o) {
+  // It is impossible for both o to be nil and result to be non-nil,
+  // so this should not be reached.
+  clang_analyzer_warnIfReached(); // no-warning
+}
+  }
+}
+
+void maybeNilReceiverIsNotNilAfterMessage(SomeClass *o) {
+  [o someMethod];
+
+  // We intentionally drop the nil flow (losing coverage) after a method
+  // call when the receiver may be nil in order to avoid inconsistencies of
+  // the kind tested for in consistencyOfReturnWithNilReceiver().
+  clang_analyzer_eval(o != 0); // expected-warning{{TRUE}}
+}
+
+void nilReceiverIsStillNilAfterMessage(SomeClass *o) {
+  if (o == 0) {
+id result = [o someMethodWithReturn];
+
+// Both the receiver and the result should be nil after a message
+// sent to a nil receiver returning a value of type id.
+clang_analyzer_eval(o == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(result == 0); // expected-warning{{TRUE}}
+  }
+}
Index: test/Analysis/NSContainers.m
===
--- test/Analysis/NSContainers.m
+++ test/Analysis/NSContainers.m
@@ -24,6 +24,8 @@
 @interface NSObject  {}
 - (id)init;
 + (id)alloc;
+
+- (id)mutableCopy;
 @end
 
 typedef struct {
@@ -292,3 +294,20 @@
   [arr addObject:0 safe:1]; // no-warning
 }
 
+@interface MyView : NSObject
+-(NSArray *)subviews;
+@end
+
+void testNoReportWhenReceiverNil(NSMutableArray *array, int b) {
+  // Don't warn about adding nil to a container when the receiver is also
+  // definitely nil.
+  if (array == 0) {
+[array addObject:0]; // no-warning
+  }
+
+  MyView *view = b ? [[MyView alloc] init] : 0;
+  NSMutableArray *subviews = [[view subviews] mutableCopy];
+  // When view is nil, subviews is also nil so there should be no warning
+  // here either.
+  [subviews addObject:view]; // no-warning
+}
Index: lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineObjC.cpp
@@ -139,6 +139,68 @@
   CallEventRef Msg =
 CEMgr.getObjCMethodCall(ME, Pred->getState(), Pred->getLocationContext());
 
+  // There are three cases for the receiver:
+  //   (1) it is definitely nil,
+  //   (2) it is definitely non-nil, and
+  //   (3) we don't know.
+  //
+  // If the receiver is definitely nil, we skip the pre/post callbacks and
+  // instead call the ObjCMessageNil callbacks and return.
+  //
+  // If the receiver is definitely non-nil, we call the pre- callbacks,
+  // evaluate the call, and call the post- callbacks.
+  //
+  // If we don't know, we drop the potential nil flow and instead
+  // continue from the assumed non-nil state as in (2). This approach
+  // intentionally drops coverage in order to prevent false alarms
+  // in the following scenario:
+  //
+  // id result = [o someMethod]
+  // if (result) {
+  //   if (!o) {
+  // // <-- This program point should be unreachable because if o is nil
+  // // it must the case that result is nil as well.
+  //   }
+  // }
+  //
+  // We could avoid dropping coverage by performing an explicit case split
+  // on each method call -- but this would get very expensive. An alternative
+  // would be to introduce lazy constraints.
+  // FIXME: This ignores many potential bugs ().
+  // Revisit once we have lazier constraints.
+  if (Msg->isInstanceMessage()) {
+SVal recVal = Msg->getReceiverSVal();
+if (!recVal.isUndef()) {
+  // Bifurcate the state into nil and 

[PATCH] D12571: [Analyzer] Fix assertions in commit r246345 (pr22954).

[pierre gousseau via cfe-commits]

pgousseau created this revision.
pgousseau added reviewers: ayartsev, zaks.anna, dcoughlin, xazax.hun, ClockMan.
pgousseau added a subscriber: cfe-commits.

Fix assertions kindly reported by Gabor Horvath and Piotr Zegar caused by 
commit r246345 and reverted in r246479.
Original review in http://reviews.llvm.org/D11832
Issue is caused by 'IsFirstBufInBound(...)' I wrongly assumed that no 'Unknown' 
values could reach it.
Added tests for unknown and tainted arguments being passed to 'memcpy' call.

Please let me know if this is an acceptable change ?

Regards,

Pierre Gousseau
SN-Systems - Sony Computer Entertainment

http://reviews.llvm.org/D12571

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/pr22954.c

Index: test/Analysis/pr22954.c
===
--- /dev/null
+++ test/Analysis/pr22954.c
@@ -0,0 +1,756 @@
+// Given code 'struct aa { char s1[4]; char * s2;} a; memcpy(a.s1, ...);',
+// this test checks that the CStringChecker only invalidates the destination buffer array a.s1 (instead of a.s1 and a.s2).
+// At the moment the whole of the destination array content is invalidated.
+// If a.s1 region has a symbolic offset, the whole region of 'a' is invalidated.
+// Specific triple set to test structures of size 0.
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store=region -verify %s
+
+typedef __typeof(sizeof(int)) size_t;
+
+char *strdup(const char *s);
+void free(void *);
+void *memcpy(void *dst, const void *src, size_t n); // expected-note{{passing argument to parameter 'dst' here}}
+void *malloc(size_t n);
+
+void clang_analyzer_eval(int);
+
+struct aa {
+char s1[4];
+char *s2;
+};
+
+// Test different types of structure initialisation.
+int f0() {
+  struct aa a0 = {{1, 2, 3, 4}, 0};
+  a0.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a0.s1, input, 4);
+  clang_analyzer_eval(a0.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a0.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a0.s2); // no warning
+  return 0;
+}
+
+int f1() {
+  struct aa a1;
+  a1.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a1.s1, input, 4);
+  clang_analyzer_eval(a1.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a1.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a1.s2); // no warning
+  return 0;
+}
+
+int f2() {
+  struct aa a2 = {{1, 2}};
+  a2.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  memcpy(a2.s1, input, 4);
+  clang_analyzer_eval(a2.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a2.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a2.s2); // no warning
+  return 0;
+}
+
+int f3() {
+  struct aa a3 = {{1, 2, 3, 4}, 0};
+  a3.s2 = strdup("hello");
+  char input[] = {'a', 'b', 'c', 'd'};
+  int * dest = (int*)a3.s1;
+  memcpy(dest, input, 4);
+  clang_analyzer_eval(a3.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[2] == 'c'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s1[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[3] == 'd'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(a3.s2 == 0); // expected-warning{{UNKNOWN}}
+  free(a3.s2); // no warning
+  return 0;
+}
+
+struct bb {
+  struct aa a;
+  char * s2;
+};
+
+int f4() {
+  struct bb b0 = {{1, 2, 3, 4}, 0};
+  b0.s2 = strdup("hello");
+  b0.a.s2 = strdup("hola");
+  char input[] = {'a', 'b', 'c', 'd'};
+  char * dest = (char*)(b0.a.s1);
+  memcpy(dest, input, 4);
+  clang_analyzer_eval(b0.a.s1[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[0] == 'a'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(dest[1] == 'b'); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b0.a.s1[2] == 'c'

Re: clang-tools-extra code owners

[Peter Collingbourne via cfe-commits]

SGTM.

Peter

On Wed, Sep 02, 2015 at 12:38:41PM -0400, Aaron Ballman wrote:
> I happened to notice that CODE_OWNERS.TXT for clang-tools-extra is
> rather sparse. These tools are starting to get more attention from the
> community, and I am wondering whether it would make sense to populate
> that somewhat? If so, I would nominate:
> 
> Alexander Kornienko for clang-tidy
> Peter Collingbourne for clang-query
> Manuel Klimek for clang-rename, and clang-tools-extra as a whole
> 
> I'm basing these nominations on who is actively maintaining the tools already.
> 
> ~Aaron
> 

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

[clang-tools-extra] r246698 - Updating the code owners list.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Sep  2 15:00:41 2015
New Revision: 246698

URL: http://llvm.org/viewvc/llvm-project?rev=246698&view=rev
Log:
Updating the code owners list.

Modified:
clang-tools-extra/trunk/CODE_OWNERS.TXT

Modified: clang-tools-extra/trunk/CODE_OWNERS.TXT
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/CODE_OWNERS.TXT?rev=246698&r1=246697&r2=246698&view=diff
==
--- clang-tools-extra/trunk/CODE_OWNERS.TXT (original)
+++ clang-tools-extra/trunk/CODE_OWNERS.TXT Wed Sep  2 15:00:41 2015
@@ -8,6 +8,14 @@ beautification by scripts.  The fields a
 (W), PGP key ID and fingerprint (P), description (D), and snail-mail address
 (S).
 
-N: Edwin Vane
-E: rev...@gmail.com
-D: clang-modernize
+N: Peter Collingbourne
+E: pe...@pcc.me.uk
+D: clang-query
+
+N: Manuel Klimek
+E: kli...@google.com
+D: clang-rename, all parts of clang-tools-extra not covered by someone else
+
+N: Alexander Kornienko
+E: ale...@google.com
+D: clang-tidy


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

Re: clang-tools-extra code owners

[Aaron Ballman via cfe-commits]

I've updated the code owners list in r246698. Thanks!

~Aaron

On Wed, Sep 2, 2015 at 3:33 PM, Peter Collingbourne  wrote:
> SGTM.
>
> Peter
>
> On Wed, Sep 02, 2015 at 12:38:41PM -0400, Aaron Ballman wrote:
>> I happened to notice that CODE_OWNERS.TXT for clang-tools-extra is
>> rather sparse. These tools are starting to get more attention from the
>> community, and I am wondering whether it would make sense to populate
>> that somewhat? If so, I would nominate:
>>
>> Alexander Kornienko for clang-tidy
>> Peter Collingbourne for clang-query
>> Manuel Klimek for clang-rename, and clang-tools-extra as a whole
>>
>> I'm basing these nominations on who is actively maintaining the tools 
>> already.
>>
>> ~Aaron
>>
>
> --
> Peter
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12544: Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output.

[Ivan Krasin via cfe-commits]

krasin updated this revision to Diff 33849.
krasin added a comment.

Addressed a nit.


http://reviews.llvm.org/D12544

Files:
  include/clang/Driver/Options.td
  include/clang/Driver/SanitizerArgs.h
  lib/Driver/SanitizerArgs.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Driver/Inputs/resource_dir/asan_blacklist.txt
  test/Driver/fsanitize-blacklist.c
  test/Frontend/print-header-includes.c

Index: test/Frontend/print-header-includes.c
===
--- test/Frontend/print-header-includes.c
+++ test/Frontend/print-header-includes.c
@@ -14,7 +14,7 @@
 // MS-NOT: Note
 
 // RUN: echo "fun:foo" > %t.blacklist
-// RUN: %clang_cc1 -fsanitize=address -fsanitize-blacklist=%t.blacklist -E --show-includes -o %t.out %s > %t.stdout
+// RUN: %clang_cc1 -fsanitize=address -fdepfile-entry=%t.blacklist -E --show-includes -o %t.out %s > %t.stdout
 // RUN: FileCheck --check-prefix=MS-BLACKLIST < %t.stdout %s
 // MS-BLACKLIST: Note: including file: {{.*\.blacklist}}
 // MS-BLACKLIST: Note: including file: {{.*test.h}}
Index: test/Driver/fsanitize-blacklist.c
===
--- test/Driver/fsanitize-blacklist.c
+++ test/Driver/fsanitize-blacklist.c
@@ -13,13 +13,25 @@
 // RUN: echo "badline" > %t.bad
 
 // RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fsanitize-blacklist=%t.second %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST
-// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.good
-// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.second
+// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.good" "-fsanitize-blacklist={{.*}}.second
+
+// Now, check for -fdepfile-entry flags.
+// RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fsanitize-blacklist=%t.second %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST2
+// CHECK-BLACKLIST2: -fdepfile-entry={{.*}}.good" "-fdepfile-entry={{.*}}.second
+
+// Check that the default blacklist is not added as an extra dependency.
+// RUN: %clang -fsanitize=address -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT-BLACKLIST --implicit-check-not=fdepfile-entry
+// CHECK-DEFAULT-BLACKLIST: -fsanitize-blacklist={{.*}}asan_blacklist.txt
 
 // Ignore -fsanitize-blacklist flag if there is no -fsanitize flag.
 // RUN: %clang -fsanitize-blacklist=%t.good %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SANITIZE --check-prefix=DELIMITERS
 // CHECK-NO-SANITIZE-NOT: -fsanitize-blacklist
 
+// Ignore -fsanitize-blacklist flag if there is no -fsanitize flag.
+// Now, check for the absense of -fdepfile-entry flags.
+// RUN: %clang -fsanitize-blacklist=%t.good %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SANITIZE2 --check-prefix=DELIMITERS
+// CHECK-NO-SANITIZE2-NOT: -fdepfile-entry
+
 // Flag -fno-sanitize-blacklist wins if it is specified later.
 // RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fno-sanitize-blacklist %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-BLACKLIST --check-prefix=DELIMITERS
 // CHECK-NO-BLACKLIST-NOT: -fsanitize-blacklist
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -709,7 +709,7 @@
   // Add sanitizer blacklists as extra dependencies.
   // They won't be discovered by the regular preprocessor, so
   // we let make / ninja to know about this implicit dependency.
-  Opts.ExtraDeps = Args.getAllArgValues(OPT_fsanitize_blacklist);
+  Opts.ExtraDeps = Args.getAllArgValues(OPT_fdepfile_entry);
   auto ModuleFiles = Args.getAllArgValues(OPT_fmodule_file);
   Opts.ExtraDeps.insert(Opts.ExtraDeps.end(), ModuleFiles.begin(),
 ModuleFiles.end());
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -176,6 +176,7 @@
   RecoverableSanitizers.clear();
   TrapSanitizers.clear();
   BlacklistFiles.clear();
+  ExtraDeps.clear();
   CoverageFeatures = 0;
   MsanTrackOrigins = 0;
   MsanUseAfterDtor = false;
@@ -383,13 +384,16 @@
 if (Arg->getOption().matches(options::OPT_fsanitize_blacklist)) {
   Arg->claim();
   std::string BLPath = Arg->getValue();
-  if (llvm::sys::fs::exists(BLPath))
+  if (llvm::sys::fs::exists(BLPath)) {
 BlacklistFiles.push_back(BLPath);
-  else
+ExtraDeps.push_back(BLPath);
+  } else
 D.Diag(clang::diag::err_drv_no_such_file) << BLPath;
+
 } else if (Arg->getOption().matches(options::OPT_fno_sanitize_blacklist)) {
   Arg->claim();
   BlacklistFiles.clear();
+  ExtraDeps.clear();
 }
   }
   // Validate blacklists format.
@@ -563,6 +567,11 @@
 BlacklistOpt += BLPath;
 CmdArgs.push_back(Args.MakeArgString(BlacklistOpt));
   }
+  for (const auto &Dep : ExtraDeps) {
+SmallString<64> Extra

r246699 - add __builtin_unpredictable and convert to metadata

[Sanjay Patel via cfe-commits]

Author: spatel
Date: Wed Sep  2 15:01:30 2015
New Revision: 246699

URL: http://llvm.org/viewvc/llvm-project?rev=246699&view=rev
Log:
add __builtin_unpredictable and convert to metadata

This patch depends on r246688 (D12341).

The goal is to make LLVM generate different code for these functions for a 
target that
has cheap branches (see PR23827 for more details):

int foo();

int normal(int x, int y, int z) {
   if (x != 0 && y != 0) return foo();
   return 1;
}

int crazy(int x, int y) {
   if (__builtin_unpredictable(x != 0 && y != 0)) return foo();
   return 1;
}

Differential Revision: http://reviews.llvm.org/D12458


Added:
cfe/trunk/test/CodeGen/builtin-unpredictable.c
Modified:
cfe/trunk/docs/LanguageExtensions.rst
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp

Modified: cfe/trunk/docs/LanguageExtensions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/LanguageExtensions.rst?rev=246699&r1=246698&r2=246699&view=diff
==
--- cfe/trunk/docs/LanguageExtensions.rst (original)
+++ cfe/trunk/docs/LanguageExtensions.rst Wed Sep  2 15:01:30 2015
@@ -1540,6 +1540,33 @@ takes no arguments and produces a void r
 
 Query for this feature with ``__has_builtin(__builtin_unreachable)``.
 
+``__builtin_unpredictable``
+---
+
+``__builtin_unpredictable`` is used to indicate that a branch condition is
+unpredictable by hardware mechanisms such as branch prediction logic.
+
+**Syntax**:
+
+.. code-block:: c++
+
+__builtin_unpredictable(long long)
+
+**Example of use**:
+
+.. code-block:: c++
+
+  if (__builtin_unpredictable(x > 0)) {
+ foo();
+  }
+
+**Description**:
+
+The ``__builtin_unpredictable()`` builtin is expected to be used with control
+flow conditions such as in ``if`` and ``switch`` statements.
+
+Query for this feature with ``__has_builtin(__builtin_unpredictable)``.
+
 ``__sync_swap``
 ---
 

Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=246699&r1=246698&r2=246699&view=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Wed Sep  2 15:01:30 2015
@@ -489,6 +489,7 @@ BUILTIN(__builtin___printf_chk, "iicC*."
 BUILTIN(__builtin___vfprintf_chk, "iP*icC*a", "FP:2:")
 BUILTIN(__builtin___vprintf_chk, "iicC*a", "FP:1:")
 
+BUILTIN(__builtin_unpredictable, "LiLi"   , "nc")
 BUILTIN(__builtin_expect, "LiLiLi"   , "nc")
 BUILTIN(__builtin_prefetch, "vvC*.", "nc")
 BUILTIN(__builtin_readcyclecounter, "ULLi", "n")

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=246699&r1=246698&r2=246699&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Wed Sep  2 15:01:30 2015
@@ -455,6 +455,12 @@ RValue CodeGenFunction::EmitBuiltinExpr(
  "cast");
 return RValue::get(Result);
   }
+  case Builtin::BI__builtin_unpredictable: {
+// Always return the argument of __builtin_unpredictable. LLVM does not
+// handle this builtin. Metadata for this builtin should be added directly
+// to instructions such as branches or switches that use it.
+return RValue::get(EmitScalarExpr(E->getArg(0)));
+  }
   case Builtin::BI__builtin_expect: {
 Value *ArgValue = EmitScalarExpr(E->getArg(0));
 llvm::Type *ArgType = ArgValue->getType();

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=246699&r1=246698&r2=246699&view=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Sep  2 15:01:30 2015
@@ -24,6 +24,7 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/StmtCXX.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "clang/Frontend/CodeGenOptions.h"
@@ -1203,6 +1204,22 @@ void CodeGenFunction::EmitBranchOnBoolEx
 return;
   }
 
+  // If the branch has a condition wrapped by __builtin_unpredictable,
+  // create metadata that specifies that the branch is unpredictable.
+  // Don't bother if not optimizing because that metadata would not be used.
+  llvm::MDNode *Unpredictable = nullptr;
+  if (CGM.getCodeGenOpts().OptimizationLevel != 0) {
+if (const CallExpr *Call = dyn_cast(Cond)) {
+  const Decl *TargetDecl = Call->getCa

Re: [PATCH] D12458: [clang] add __builtin_unpredictable and convert to metadata

[Sanjay Patel via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL246699: add __builtin_unpredictable and convert to metadata 
(authored by spatel).

Changed prior to commit:
  http://reviews.llvm.org/D12458?vs=33491&id=33850#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D12458

Files:
  cfe/trunk/docs/LanguageExtensions.rst
  cfe/trunk/include/clang/Basic/Builtins.def
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
  cfe/trunk/test/CodeGen/builtin-unpredictable.c

Index: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
@@ -24,6 +24,7 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/StmtCXX.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "clang/Frontend/CodeGenOptions.h"
@@ -1203,6 +1204,22 @@
 return;
   }
 
+  // If the branch has a condition wrapped by __builtin_unpredictable,
+  // create metadata that specifies that the branch is unpredictable.
+  // Don't bother if not optimizing because that metadata would not be used.
+  llvm::MDNode *Unpredictable = nullptr;
+  if (CGM.getCodeGenOpts().OptimizationLevel != 0) {
+if (const CallExpr *Call = dyn_cast(Cond)) {
+  const Decl *TargetDecl = Call->getCalleeDecl();
+  if (const FunctionDecl *FD = dyn_cast_or_null(TargetDecl)) {
+if (FD->getBuiltinID() == Builtin::BI__builtin_unpredictable) {
+  llvm::MDBuilder MDHelper(getLLVMContext());
+  Unpredictable = MDHelper.createUnpredictable();
+}
+  }
+}
+  }
+
   // Create branch weights based on the number of times we get here and the
   // number of times the condition should be true.
   uint64_t CurrentCount = std::max(getCurrentProfileCount(), TrueCount);
@@ -1215,7 +1232,7 @@
 ApplyDebugLocation DL(*this, Cond);
 CondV = EvaluateExprAsBool(Cond);
   }
-  Builder.CreateCondBr(CondV, TrueBlock, FalseBlock, Weights);
+  Builder.CreateCondBr(CondV, TrueBlock, FalseBlock, Weights, Unpredictable);
 }
 
 /// ErrorUnsupported - Print out an error that codegen doesn't support the
Index: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
===
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp
@@ -455,6 +455,12 @@
  "cast");
 return RValue::get(Result);
   }
+  case Builtin::BI__builtin_unpredictable: {
+// Always return the argument of __builtin_unpredictable. LLVM does not
+// handle this builtin. Metadata for this builtin should be added directly
+// to instructions such as branches or switches that use it.
+return RValue::get(EmitScalarExpr(E->getArg(0)));
+  }
   case Builtin::BI__builtin_expect: {
 Value *ArgValue = EmitScalarExpr(E->getArg(0));
 llvm::Type *ArgType = ArgValue->getType();
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -41,11 +41,12 @@
   default:
 return false;
 
+  case Builtin::BI__builtin_unpredictable:
   case Builtin::BI__builtin_expect:
   case Builtin::BI__builtin_assume_aligned:
   case Builtin::BI__builtin_addressof: {
-// For __builtin_expect and __builtin_assume_aligned, just return the value
-// of the subexpression.
+// For __builtin_unpredictable, __builtin_expect, and
+// __builtin_assume_aligned, just return the value of the subexpression.
 // __builtin_addressof is going from a reference to a pointer, but those
 // are represented the same way in the analyzer.
 assert (CE->arg_begin() != CE->arg_end());
Index: cfe/trunk/docs/LanguageExtensions.rst
===
--- cfe/trunk/docs/LanguageExtensions.rst
+++ cfe/trunk/docs/LanguageExtensions.rst
@@ -1540,6 +1540,33 @@
 
 Query for this feature with ``__has_builtin(__builtin_unreachable)``.
 
+``__builtin_unpredictable``
+---
+
+``__builtin_unpredictable`` is used to indicate that a branch condition is
+unpredictable by hardware mechanisms such as branch prediction logic.
+
+**Syntax**:
+
+.. code-block:: c++
+
+__builtin_unpredictable(long long)
+
+**Example of use**:
+
+.. code-block:: c++
+
+  if (__builtin_unpredictable(x > 0)) {
+ foo();
+  }
+
+**Description**:
+
+The ``__builtin_unpredictable()`` builtin is expected to be used with control
+flow conditions such as in ``if`` and ``switch`` statements.
+
+Query for this feature with ``__has_builtin(__builtin_unpredictable)``.
+
 ``__s

Re: [PATCH] D12544: Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output.

[Ivan Krasin via cfe-commits]

krasin added inline comments.


Comment at: test/Driver/fsanitize-blacklist.c:25
@@ -18,2 +24,3 @@
+// CHECK-DEFAULT-BLACKLIST: -fsanitize-blacklist={{.*}}asan_blacklist.txt
 
 // Ignore -fsanitize-blacklist flag if there is no -fsanitize flag.

Good idea. Done.


http://reviews.llvm.org/D12544



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

r246700 - Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output.

[Ivan Krasin via cfe-commits]

Author: krasin
Date: Wed Sep  2 15:02:38 2015
New Revision: 246700

URL: http://llvm.org/viewvc/llvm-project?rev=246700&view=rev
Log:
Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output.

Summary:
Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output.

Introduce a frontend option -fdepfile-entry, and only insert them
for the user-defined sanitizer blacklists. In frontend, grab ExtraDeps
from -fdepfile-entry, instead of -fsanitize-blacklist.

Reviewers: rsmith, pcc

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D12544

Added:
cfe/trunk/test/Driver/Inputs/resource_dir/asan_blacklist.txt
Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Driver/SanitizerArgs.h
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/Driver/fsanitize-blacklist.c
cfe/trunk/test/Frontend/print-header-includes.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=246700&r1=246699&r2=246700&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Wed Sep  2 15:02:38 2015
@@ -495,6 +495,8 @@ def fcxx_modules : Flag <["-"], "fcxx-mo
   Flags<[DriverOption]>;
 def fdebug_pass_arguments : Flag<["-"], "fdebug-pass-arguments">, 
Group;
 def fdebug_pass_structure : Flag<["-"], "fdebug-pass-structure">, 
Group;
+def fdepfile_entry : Joined<["-"], "fdepfile-entry=">,
+Group, Flags<[CC1Option]>;
 def fdiagnostics_fixit_info : Flag<["-"], "fdiagnostics-fixit-info">, 
Group;
 def fdiagnostics_parseable_fixits : Flag<["-"], 
"fdiagnostics-parseable-fixits">, Group,
 Flags<[CoreOption, CC1Option]>, HelpText<"Print fix-its in machine 
parseable form">;

Modified: cfe/trunk/include/clang/Driver/SanitizerArgs.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/SanitizerArgs.h?rev=246700&r1=246699&r2=246700&view=diff
==
--- cfe/trunk/include/clang/Driver/SanitizerArgs.h (original)
+++ cfe/trunk/include/clang/Driver/SanitizerArgs.h Wed Sep  2 15:02:38 2015
@@ -27,6 +27,7 @@ class SanitizerArgs {
   SanitizerSet TrapSanitizers;
 
   std::vector BlacklistFiles;
+  std::vector ExtraDeps;
   int CoverageFeatures;
   int MsanTrackOrigins;
   bool MsanUseAfterDtor;

Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=246700&r1=246699&r2=246700&view=diff
==
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original)
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Wed Sep  2 15:02:38 2015
@@ -176,6 +176,7 @@ void SanitizerArgs::clear() {
   RecoverableSanitizers.clear();
   TrapSanitizers.clear();
   BlacklistFiles.clear();
+  ExtraDeps.clear();
   CoverageFeatures = 0;
   MsanTrackOrigins = 0;
   MsanUseAfterDtor = false;
@@ -383,13 +384,16 @@ SanitizerArgs::SanitizerArgs(const ToolC
 if (Arg->getOption().matches(options::OPT_fsanitize_blacklist)) {
   Arg->claim();
   std::string BLPath = Arg->getValue();
-  if (llvm::sys::fs::exists(BLPath))
+  if (llvm::sys::fs::exists(BLPath)) {
 BlacklistFiles.push_back(BLPath);
-  else
+ExtraDeps.push_back(BLPath);
+  } else
 D.Diag(clang::diag::err_drv_no_such_file) << BLPath;
+
 } else if (Arg->getOption().matches(options::OPT_fno_sanitize_blacklist)) {
   Arg->claim();
   BlacklistFiles.clear();
+  ExtraDeps.clear();
 }
   }
   // Validate blacklists format.
@@ -563,6 +567,11 @@ void SanitizerArgs::addArgs(const ToolCh
 BlacklistOpt += BLPath;
 CmdArgs.push_back(Args.MakeArgString(BlacklistOpt));
   }
+  for (const auto &Dep : ExtraDeps) {
+SmallString<64> ExtraDepOpt("-fdepfile-entry=");
+ExtraDepOpt += Dep;
+CmdArgs.push_back(Args.MakeArgString(ExtraDepOpt));
+  }
 
   if (MsanTrackOrigins)
 CmdArgs.push_back(Args.MakeArgString("-fsanitize-memory-track-origins=" +

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=246700&r1=246699&r2=246700&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Wed Sep  2 15:02:38 2015
@@ -709,7 +709,7 @@ static void ParseDependencyOutputArgs(De
   // Add sanitizer blacklists as extra dependencies.
   // They won't be discovered by the regular preprocessor, so
   // we let make / ninja to know about this implicit dependency.
-  Opts.ExtraDeps = Args.getAllArgValues(OPT_fsanitize_blacklist);
+  Opts.ExtraDeps = Args.getA

Re: clang-tools-extra code owners

[Manuel Klimek via cfe-commits]

Thanks for the cleanup!

On Wed, Sep 2, 2015 at 10:02 PM Aaron Ballman 
wrote:

> I've updated the code owners list in r246698. Thanks!
>
> ~Aaron
>
> On Wed, Sep 2, 2015 at 3:33 PM, Peter Collingbourne 
> wrote:
> > SGTM.
> >
> > Peter
> >
> > On Wed, Sep 02, 2015 at 12:38:41PM -0400, Aaron Ballman wrote:
> >> I happened to notice that CODE_OWNERS.TXT for clang-tools-extra is
> >> rather sparse. These tools are starting to get more attention from the
> >> community, and I am wondering whether it would make sense to populate
> >> that somewhat? If so, I would nominate:
> >>
> >> Alexander Kornienko for clang-tidy
> >> Peter Collingbourne for clang-query
> >> Manuel Klimek for clang-rename, and clang-tools-extra as a whole
> >>
> >> I'm basing these nominations on who is actively maintaining the tools
> already.
> >>
> >> ~Aaron
> >>
> >
> > --
> > Peter
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r246701 - Update comment for AdditionalMembers with a note to avoid using

[Eric Christopher via cfe-commits]

Author: echristo
Date: Wed Sep  2 15:13:41 2015
New Revision: 246701

URL: http://llvm.org/viewvc/llvm-project?rev=246701&view=rev
Log:
Update comment for AdditionalMembers with a note to avoid using
additional data members in attributes as they'll leak and provide
some guidance as to where they should be allocated if necessary.

Modified:
cfe/trunk/include/clang/Basic/Attr.td

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=246701&r1=246700&r2=246701&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Wed Sep  2 15:13:41 2015
@@ -290,6 +290,8 @@ class Attr {
   // attribute to be applicable. If empty, no language options are required.
   list LangOpts = [];
   // Any additional text that should be included verbatim in the class.
+  // Note: Any additional data members will leak and should be constructed
+  // externally on the ASTContext.
   code AdditionalMembers = [{}];
   // Any documentation that should be associated with the attribute. Since an
   // attribute may be documented under multiple categories, more than one


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

Re: r246701 - Update comment for AdditionalMembers with a note to avoid using

[Aaron Ballman via cfe-commits]

On Wed, Sep 2, 2015 at 4:13 PM, Eric Christopher via cfe-commits
 wrote:
> Author: echristo
> Date: Wed Sep  2 15:13:41 2015
> New Revision: 246701
>
> URL: http://llvm.org/viewvc/llvm-project?rev=246701&view=rev
> Log:
> Update comment for AdditionalMembers with a note to avoid using
> additional data members in attributes as they'll leak and provide
> some guidance as to where they should be allocated if necessary.

Thank you for this!

~Aaron

>
> Modified:
> cfe/trunk/include/clang/Basic/Attr.td
>
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=246701&r1=246700&r2=246701&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Wed Sep  2 15:13:41 2015
> @@ -290,6 +290,8 @@ class Attr {
>// attribute to be applicable. If empty, no language options are required.
>list LangOpts = [];
>// Any additional text that should be included verbatim in the class.
> +  // Note: Any additional data members will leak and should be constructed
> +  // externally on the ASTContext.
>code AdditionalMembers = [{}];
>// Any documentation that should be associated with the attribute. Since an
>// attribute may be documented under multiple categories, more than one
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r246610 - Migrate the target attribute parsing code into an extension off of

[Eric Christopher via cfe-commits]

On Wed, Sep 2, 2015 at 10:11 AM Aaron Ballman 
wrote:

> On Wed, Sep 2, 2015 at 1:07 PM, Eric Christopher 
> wrote:
> >
> >
> > On Wed, Sep 2, 2015 at 7:24 AM Aaron Ballman 
> wrote:
> >>
> >> A few nits in addition to what Ben pointed out...
> >>
> >> On Tue, Sep 1, 2015 at 8:12 PM, Eric Christopher via cfe-commits
> >>  wrote:
> >> > Author: echristo
> >> > Date: Tue Sep  1 19:12:02 2015
> >> > New Revision: 246610
> >> >
> >> > URL: http://llvm.org/viewvc/llvm-project?rev=246610&view=rev
> >> > Log:
> >> > Migrate the target attribute parsing code into an extension off of
> >> > the main attribute and cache the results so we don't have to parse
> >> > a single attribute more than once.
> >> >
> >> > This reapplies r246596 with a fix for an uninitialized class member,
> >> > and a couple of cleanups and formatting changes.
> >> >
> >> > Modified:
> >> > cfe/trunk/include/clang/Basic/Attr.td
> >> > cfe/trunk/lib/CodeGen/CGCall.cpp
> >> >
> >> > Modified: cfe/trunk/include/clang/Basic/Attr.td
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=246610&r1=246609&r2=246610&view=diff
> >> >
> >> >
> ==
> >> > --- cfe/trunk/include/clang/Basic/Attr.td (original)
> >> > +++ cfe/trunk/include/clang/Basic/Attr.td Tue Sep  1 19:12:02 2015
> >> > @@ -1315,9 +1315,52 @@ def Pascal : InheritableAttr {
> >> >
> >> >  def Target : InheritableAttr {
> >> >let Spellings = [GCC<"target">];
> >> > -  let Args = [StringArgument<"features">];
> >> > +  let Args = [StringArgument<"featuresStr">];
> >> >let Subjects = SubjectList<[Function], ErrorDiag>;
> >> >let Documentation = [TargetDocs];
> >> > +  let AdditionalMembers = [{
> >> > +StringRef CPU;
> >> > +std::vector Features;
> >> > +bool Parsed = false;
> >>
> >> Additional members are all public, can you privatize the data members?
> >> Then you can make them mutable and fix the const-correctness
> >> regression. ;-)
> >>
> >
> > Heh. Sure. I don't really like mutable unless there's no other choice,
> but I
> > can do it here, however, it also sounds like you don't want them to be
> data
> > members at all? Did you want me to just reparse them or try to put a
> cache
> > somewhere?
>
> I'm fine with them being data members. The usual pattern is:
>
> let AdditionalMembers = [{
> private:
>   
>
> public:
>   
> }];
>
>
Yeah, there's only one case and I think Richard is going to fix it. In
general, anything there is going to leak so I'm going to do this a
different way and just reparse for now. If it becomes an issue (I really
only plan on using this in about 3 places) then we can look into caching it
on the ASTContext somewhere (or allocating it via the bump pointer
allocator there).

I've added a comment here:

Committing to https://llvm.org/svn/llvm-project/cfe/trunk ...
M include/clang/Basic/Attr.td
Committed r246701

to basically tell people not to use this pattern :)

-eric


> >
> >>
> >> > +StringRef getCPU() {
> >> > +  if (!Parsed)
> >> > +parse();
> >> > +  return CPU;
> >> > +}
> >> > +std::vector &getFeatures() {
> >> > +  if (!Parsed)
> >> > +parse();
> >> > +  return Features;
> >> > +}
> >> > +void parse() {
> >> > +  SmallVector AttrFeatures;
> >> > +  getFeaturesStr().split(AttrFeatures, ",");
> >> > +
> >> > +  // Grab the various features and prepend a "+" to turn on the
> >> > feature to
> >> > +  // the backend and add them to our existing set of features.
> >> > +  for (auto &Feature : AttrFeatures) {
> >> > +// Go ahead and trim whitespace rather than either erroring
> or
> >> > +// accepting it weirdly.
> >> > +Feature = Feature.trim();
> >> > +
> >> > +// We don't support cpu tuning this way currently.
> >> > +// TODO: Support the fpmath option. It will require checking
> >> > +// overall feature validity for the function with the rest of
> >> > the
> >> > +// attributes on the function.
> >> > +if (Feature.startswith("fpmath=") ||
> >> > Feature.startswith("tune="))
> >> > + continue;
> >> > +
> >> > +// While we're here iterating check for a different target
> cpu.
> >> > +if (Feature.startswith("arch="))
> >> > +  CPU = Feature.split("=").second.trim();
> >> > +else if (Feature.startswith("no-"))
> >> > +  Features.push_back("-" + Feature.split("-").second.str());
> >> > +else
> >> > +  Features.push_back("+" + Feature.str());
> >> > +  }
> >> > +  Parsed = true;
> >> > +}
> >> > +  }];
> >> >  }
> >> >
> >> >  def TransparentUnion : InheritableAttr {
> >> >
> >> > Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=246610&r1=246609&r2=246610&view=diff
> >> >
> >> >
> =

r246702 - [modules] Tighten up this test a bit.

[Sean Silva via cfe-commits]

Author: silvas
Date: Wed Sep  2 15:16:09 2015
New Revision: 246702

URL: http://llvm.org/viewvc/llvm-project?rev=246702&view=rev
Log:
[modules] Tighten up this test a bit.

Modified:
cfe/trunk/test/Modules/module-map-path-hash.cpp

Modified: cfe/trunk/test/Modules/module-map-path-hash.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/module-map-path-hash.cpp?rev=246702&r1=246701&r2=246702&view=diff
==
--- cfe/trunk/test/Modules/module-map-path-hash.cpp (original)
+++ cfe/trunk/test/Modules/module-map-path-hash.cpp Wed Sep  2 15:16:09 2015
@@ -1,9 +1,10 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -x c++ -Rmodule-build 
-I%S/Inputs/module-map-path-hash -fmodules-cache-path=%t -fsyntax-only %s
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -x c++ -Rmodule-build 
-I%S/Inputs//module-map-path-hash -fmodules-cache-path=%t -fsyntax-only %s 2>&1 
| FileCheck -allow-empty %s
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -x c++ -Rmodule-build 
-I%S/Inputs/./module-map-path-hash -fmodules-cache-path=%t -fsyntax-only %s 
2>&1 | FileCheck -allow-empty %s
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -x c++ -Rmodule-build 
-I%S/Inputs/../Inputs/module-map-path-hash -fmodules-cache-path=%t 
-fsyntax-only %s 2>&1 | FileCheck -allow-empty %s
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -x c++ -Rmodule-build 
-I%S/Inputs/module-map-path-hash -fmodules-cache-path=%t -fsyntax-only %s 2>&1 
| FileCheck %s --check-prefix=BUILD
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -x c++ -Rmodule-build 
-I%S/Inputs//module-map-path-hash -fmodules-cache-path=%t -fsyntax-only %s 2>&1 
| FileCheck -allow-empty %s --check-prefix=NOBUILD
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -x c++ -Rmodule-build 
-I%S/Inputs/./module-map-path-hash -fmodules-cache-path=%t -fsyntax-only %s 
2>&1 | FileCheck -allow-empty %s --check-prefix=NOBUILD
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -x c++ -Rmodule-build 
-I%S/Inputs/../Inputs/module-map-path-hash -fmodules-cache-path=%t 
-fsyntax-only %s 2>&1 | FileCheck -allow-empty %s --check-prefix=NOBUILD
 
 #include "a.h"
 
-// CHECK-NOT: remark: building module
+// BUILD: remark: building module
+// NOBUILD-NOT: remark: building module


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

r246706 - Migrate the target attribute parsing code to returning an instance

[Eric Christopher via cfe-commits]

Author: echristo
Date: Wed Sep  2 15:40:12 2015
New Revision: 246706

URL: http://llvm.org/viewvc/llvm-project?rev=246706&view=rev
Log:
Migrate the target attribute parsing code to returning an instance
every time it's called rather than attempting to cache the result.
It's unlikely to be called frequently and the overhead of using
it in the first place is already factored out.

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/lib/CodeGen/CGCall.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=246706&r1=246705&r2=246706&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Wed Sep  2 15:40:12 2015
@@ -1321,20 +1321,9 @@ def Target : InheritableAttr {
   let Subjects = SubjectList<[Function], ErrorDiag>;
   let Documentation = [TargetDocs];
   let AdditionalMembers = [{
-StringRef CPU;
-std::vector Features;
-bool Parsed = false;
-StringRef getCPU() {
-  if (!Parsed)
-parse();
-  return CPU;
-}
-std::vector &getFeatures() {
-  if (!Parsed)
-parse();
-  return Features;
-}
-void parse() {
+typedef std::pair, StringRef> ParsedTargetAttr;
+ParsedTargetAttr parse() const {
+  ParsedTargetAttr Ret;
   SmallVector AttrFeatures;
   getFeaturesStr().split(AttrFeatures, ",");
 
@@ -1354,13 +1343,13 @@ def Target : InheritableAttr {
 
 // While we're here iterating check for a different target cpu.
 if (Feature.startswith("arch="))
-  CPU = Feature.split("=").second.trim();
+  Ret.second = Feature.split("=").second.trim();
 else if (Feature.startswith("no-"))
-  Features.push_back("-" + Feature.split("-").second.str());
+  Ret.first.push_back("-" + Feature.split("-").second.str());
 else
-  Features.push_back("+" + Feature.str());
+  Ret.first.push_back("+" + Feature.str());
   }
-  Parsed = true;
+  return Ret;
 }
   }];
 }

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=246706&r1=246705&r2=246706&view=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Sep  2 15:40:12 2015
@@ -1499,24 +1499,24 @@ void CodeGenModule::ConstructAttributeLi
 const FunctionDecl *FD = dyn_cast_or_null(TargetDecl);
 if (FD && FD->hasAttr()) {
   llvm::StringMap FeatureMap;
-  auto *TD = FD->getAttr();
+  const auto *TD = FD->getAttr();
+  TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse();
 
-  // Make a copy of the features as passed on the command line.
-  std::vector FnFeatures =
-  getTarget().getTargetOpts().FeaturesAsWritten;
+  // Make a copy of the features as passed on the command line into the
+  // beginning of the additional features from the function to override.
+  ParsedAttr.first.insert(
+  ParsedAttr.first.begin(),
+  getTarget().getTargetOpts().FeaturesAsWritten.begin(),
+  getTarget().getTargetOpts().FeaturesAsWritten.end());
 
-  std::vector &AttrFeatures = TD->getFeatures();
-  std::copy(AttrFeatures.begin(), AttrFeatures.end(),
-std::back_inserter(FnFeatures));
-
-  if (TD->getCPU() != "")
-   TargetCPU = TD->getCPU();
+  if (ParsedAttr.second != "")
+   TargetCPU = ParsedAttr.second;
 
   // Now populate the feature map, first with the TargetCPU which is either
   // the default or a new one from the target attribute string. Then we'll
   // use the passed in features (FeaturesAsWritten) along with the new ones
   // from the attribute.
-  getTarget().initFeatureMap(FeatureMap, Diags, TargetCPU, FnFeatures);
+  getTarget().initFeatureMap(FeatureMap, Diags, TargetCPU, 
ParsedAttr.first);
 
   // Produce the canonical string for this set of features.
   std::vector Features;


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

Re: r246610 - Migrate the target attribute parsing code into an extension off of

[Eric Christopher via cfe-commits]

And I think I've fixed the other bits in

dzur:~/sources/llvm/tools/clang> git svn dcommit
Committing to https://llvm.org/svn/llvm-project/cfe/trunk ...
M include/clang/Basic/Attr.td
M lib/CodeGen/CGCall.cpp
Committed r246706

We could eventually cache the parsing etc, but I don't think it's hugely
necessary. What would end up being more helpful is to cache the attribute
and then cache by attribute parsed if that makes any sense.

-eric

On Wed, Sep 2, 2015 at 1:16 PM Eric Christopher  wrote:

> On Wed, Sep 2, 2015 at 10:11 AM Aaron Ballman 
> wrote:
>
>> On Wed, Sep 2, 2015 at 1:07 PM, Eric Christopher 
>> wrote:
>> >
>> >
>> > On Wed, Sep 2, 2015 at 7:24 AM Aaron Ballman 
>> wrote:
>> >>
>> >> A few nits in addition to what Ben pointed out...
>> >>
>> >> On Tue, Sep 1, 2015 at 8:12 PM, Eric Christopher via cfe-commits
>> >>  wrote:
>> >> > Author: echristo
>> >> > Date: Tue Sep  1 19:12:02 2015
>> >> > New Revision: 246610
>> >> >
>> >> > URL: http://llvm.org/viewvc/llvm-project?rev=246610&view=rev
>> >> > Log:
>> >> > Migrate the target attribute parsing code into an extension off of
>> >> > the main attribute and cache the results so we don't have to parse
>> >> > a single attribute more than once.
>> >> >
>> >> > This reapplies r246596 with a fix for an uninitialized class member,
>> >> > and a couple of cleanups and formatting changes.
>> >> >
>> >> > Modified:
>> >> > cfe/trunk/include/clang/Basic/Attr.td
>> >> > cfe/trunk/lib/CodeGen/CGCall.cpp
>> >> >
>> >> > Modified: cfe/trunk/include/clang/Basic/Attr.td
>> >> > URL:
>> >> >
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=246610&r1=246609&r2=246610&view=diff
>> >> >
>> >> >
>> ==
>> >> > --- cfe/trunk/include/clang/Basic/Attr.td (original)
>> >> > +++ cfe/trunk/include/clang/Basic/Attr.td Tue Sep  1 19:12:02 2015
>> >> > @@ -1315,9 +1315,52 @@ def Pascal : InheritableAttr {
>> >> >
>> >> >  def Target : InheritableAttr {
>> >> >let Spellings = [GCC<"target">];
>> >> > -  let Args = [StringArgument<"features">];
>> >> > +  let Args = [StringArgument<"featuresStr">];
>> >> >let Subjects = SubjectList<[Function], ErrorDiag>;
>> >> >let Documentation = [TargetDocs];
>> >> > +  let AdditionalMembers = [{
>> >> > +StringRef CPU;
>> >> > +std::vector Features;
>> >> > +bool Parsed = false;
>> >>
>> >> Additional members are all public, can you privatize the data members?
>> >> Then you can make them mutable and fix the const-correctness
>> >> regression. ;-)
>> >>
>> >
>> > Heh. Sure. I don't really like mutable unless there's no other choice,
>> but I
>> > can do it here, however, it also sounds like you don't want them to be
>> data
>> > members at all? Did you want me to just reparse them or try to put a
>> cache
>> > somewhere?
>>
>> I'm fine with them being data members. The usual pattern is:
>>
>> let AdditionalMembers = [{
>> private:
>>   
>>
>> public:
>>   
>> }];
>>
>>
> Yeah, there's only one case and I think Richard is going to fix it. In
> general, anything there is going to leak so I'm going to do this a
> different way and just reparse for now. If it becomes an issue (I really
> only plan on using this in about 3 places) then we can look into caching it
> on the ASTContext somewhere (or allocating it via the bump pointer
> allocator there).
>
> I've added a comment here:
>
> Committing to https://llvm.org/svn/llvm-project/cfe/trunk ...
> M include/clang/Basic/Attr.td
> Committed r246701
>
> to basically tell people not to use this pattern :)
>
> -eric
>
>
>> >
>> >>
>> >> > +StringRef getCPU() {
>> >> > +  if (!Parsed)
>> >> > +parse();
>> >> > +  return CPU;
>> >> > +}
>> >> > +std::vector &getFeatures() {
>> >> > +  if (!Parsed)
>> >> > +parse();
>> >> > +  return Features;
>> >> > +}
>> >> > +void parse() {
>> >> > +  SmallVector AttrFeatures;
>> >> > +  getFeaturesStr().split(AttrFeatures, ",");
>> >> > +
>> >> > +  // Grab the various features and prepend a "+" to turn on the
>> >> > feature to
>> >> > +  // the backend and add them to our existing set of features.
>> >> > +  for (auto &Feature : AttrFeatures) {
>> >> > +// Go ahead and trim whitespace rather than either erroring
>> or
>> >> > +// accepting it weirdly.
>> >> > +Feature = Feature.trim();
>> >> > +
>> >> > +// We don't support cpu tuning this way currently.
>> >> > +// TODO: Support the fpmath option. It will require checking
>> >> > +// overall feature validity for the function with the rest
>> of
>> >> > the
>> >> > +// attributes on the function.
>> >> > +if (Feature.startswith("fpmath=") ||
>> >> > Feature.startswith("tune="))
>> >> > + continue;
>> >> > +
>> >> > +// While we're here iterating check for a different target
>> cpu.
>> >> > +if (Feature.startswith("arch

Re: [PATCH] D12087: always_inline codegen rewrite

[Richard Smith via cfe-commits]

rsmith added inline comments.


Comment at: lib/CodeGen/CodeGenModule.cpp:485
@@ +484,3 @@
+  std::string Name = Fn->getName();
+  std::string InlineName = Name + ".inlinefunction";
+  Fn->setName(InlineName);

I have a slight preference for ".alwaysinline" over ".inlinefunction", since we 
don't do this for all inline functions.


Comment at: lib/CodeGen/CodeGenModule.cpp:491-492
@@ +490,4 @@
+  FindNonDirectCallUses(Fn, &NonDirectCallUses);
+  // Do not create the wrapper if there are no non-direct call uses, and we are
+  // not required to emit an external definition.
+  if (NonDirectCallUses.empty() &&

Should we check this before we rename the function?


Comment at: lib/CodeGen/CodeGenModule.cpp:491-492
@@ +490,4 @@
+  FindNonDirectCallUses(Fn, &NonDirectCallUses);
+  // Do not create the wrapper if there are no non-direct call uses, and we are
+  // not required to emit an external definition.
+  if (NonDirectCallUses.empty() &&

rsmith wrote:
> Should we check this before we rename the function?
Should we also skip this if the function has local linkage, regardless of how 
it's used?


Comment at: lib/CodeGen/CodeGenModule.cpp:494
@@ +493,3 @@
+  if (NonDirectCallUses.empty() &&
+  (Fn->hasAvailableExternallyLinkage() || Fn->isDiscardableIfUnused()))
+return;

I would expect `available_externally` globals to be considered discardable if 
unused. Can you fix that in LLVM rather than here?


Comment at: lib/CodeGen/CodeGenModule.cpp:540-541
@@ +539,4 @@
+RewriteAlwaysInlineFunction(Fn);
+Fn->addFnAttr(llvm::Attribute::AlwaysInline);
+Fn->setLinkage(llvm::GlobalValue::InternalLinkage);
+Fn->setDLLStorageClass(llvm::GlobalVariable::DefaultStorageClass);

Swap over these two, so we never have a non-`internal` `always_inline` function.


Comment at: lib/CodeGen/CodeGenModule.h:505
@@ +504,3 @@
+
+  llvm::SetVector AlwaysInlineFunctions;
+

You only call `AddAlwaysInlineFunction` when creating a definition, which 
should happen at most once per function. Do you really need a `SetVector` here, 
or could you use a `SmallVector` instead?


Comment at: test/CodeGen/alwaysinline-unused.c:1
@@ +1,2 @@
+// Test alwaysinline definitions w/o any non-direct-call uses.
+// None of the declarations are emitted. Stub are only emitted when the 
original

Rename this to always_inline-unused.c for consistency with the existing 
always_inline.c.


Comment at: test/CodeGen/alwaysinline.c:1
@@ +1,2 @@
+// Test different kinds of alwaysinline definitions.
+

Seems weird to have both a test/CodeGen/always_inline.c and a 
test/CodeGen/alwaysinline.c. Either merge these together or rename this to 
test/CodeGen/always_inline-wrappers.c or similar.


Comment at: test/CodeGen/alwaysinline.c:47-53
@@ +46,9 @@
+
+// CHECK-DAG: define internal void @f1.inlinefunction() #[[AI:[01-9]+]]
+// CHECK-DAG: define internal void @f2.inlinefunction() #[[AI:[01-9]+]]
+// CHECK-DAG: define internal void @f3.inlinefunction() #[[AI:[01-9]+]]
+// CHECK-DAG: define internal void @f4.inlinefunction() #[[AI:[01-9]+]]
+// CHECK-DAG: define internal void @f5.inlinefunction() #[[AI:[01-9]+]]
+// CHECK-DAG: define internal void @f6.inlinefunction() #[[AI:[01-9]+]]
+// CHECK-DAG: define internal void @f7.inlinefunction() #[[AI:[01-9]+]]
+

Only the first of these should have the `:[01-9]+`, otherwise you'll overwrite 
`AI` rather than checking it's the same. Also, use `[0-9]+` instead of 
`[01-9]+`?


Comment at: test/CodeGen/alwaysinline.c:55-59
@@ +54,7 @@
+
+// CHECK-DAG: musttail call void @f1.inlinefunction()
+// CHECK-DAG: musttail call void @f3.inlinefunction()
+// CHECK-DAG: musttail call void @f4.inlinefunction()
+// CHECK-DAG: musttail call void @f5.inlinefunction()
+// CHECK-DAG: musttail call void @f7.inlinefunction()
+

I don't think we need to split `f3` and `f5` here.


Comment at: test/CodeGen/alwaysinline.c:55-67
@@ +54,15 @@
+
+// CHECK-DAG: musttail call void @f1.inlinefunction()
+// CHECK-DAG: musttail call void @f3.inlinefunction()
+// CHECK-DAG: musttail call void @f4.inlinefunction()
+// CHECK-DAG: musttail call void @f5.inlinefunction()
+// CHECK-DAG: musttail call void @f7.inlinefunction()
+
+// CHECK-DAG: define void @f1() #[[NOAI:[01-9]+]]
+// CHECK-DAG: declare void @f2() #[[NOAI:[01-9]+]]
+// CHECK-DAG: define internal void @f3() #[[NOAI:[01-9]+]]
+// CHECK-DAG: define void @f4() #[[NOAI:[01-9]+]]
+// CHECK-DAG: define internal void @f5() #[[NOAI:[01-9]+]]
+// CHECK-DAG: declare hidden void @f6() #[[NOAI:[01-9]+]]
+// CHECK-DAG: define hidden void @f7() #[[NOAI:[01-9]+]]
+

rsmith wrote:
> I don't think we need to split `f3` and `f5` here.
Can you interleave

r246710 - [analyzer] Refactoring: bring together scan-build options and environment variables.

[Anton Yartsev via cfe-commits]

Author: ayartsev
Date: Wed Sep  2 16:01:59 2015
New Revision: 246710

URL: http://llvm.org/viewvc/llvm-project?rev=246710&view=rev
Log:
[analyzer] Refactoring: bring together scan-build options and environment 
variables.

Full list of changes:
- all scan-build command-line arguments are now kept in %Options hash.
- most of environment variables scan-build operates with are stored in %EnvVars 
hash.
- moved processing of command-line arguments to the ProcessArgs subroutine.

Modified:
cfe/trunk/tools/scan-build/scan-build

Modified: cfe/trunk/tools/scan-build/scan-build
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build/scan-build?rev=246710&r1=246709&r2=246710&view=diff
==
--- cfe/trunk/tools/scan-build/scan-build (original)
+++ cfe/trunk/tools/scan-build/scan-build Wed Sep  2 16:01:59 2015
@@ -24,8 +24,8 @@ use Term::ANSIColor;
 use Term::ANSIColor qw(:constants);
 use Cwd qw/ getcwd abs_path /;
 use Sys::Hostname;
+use Hash::Util qw(lock_keys);
 
-my $Verbose = 0;   # Verbose output from this script.
 my $Prog = "scan-build";
 my $BuildName;
 my $BuildDate;
@@ -39,15 +39,40 @@ my $UseColor = (defined $TERM and $TERM
 my $UserName = HtmlEscape(getlogin() || getpwuid($<) || 'unknown');
 my $HostName = HtmlEscape(hostname() || 'unknown');
 my $CurrentDir = HtmlEscape(getcwd());
-my $CurrentDirSuffix = basename($CurrentDir);
 
-my @PluginsToLoad;
 my $CmdArgs;
 
-my $HtmlTitle;
-
 my $Date = localtime();
 
+# Command-line/config arguments.
+my %Options = (
+  Verbose => 0,  # Verbose output from this script.
+  AnalyzeHeaders => 0,
+  OutputDir => undef,# Parent directory to store HTML files.
+  HtmlTitle => basename($CurrentDir)." - scan-build results",
+  IgnoreErrors => 0, # Ignore build errors.
+  ViewResults => 0,  # View results when the build terminates.
+  ExitStatusFoundBugs => 0,  # Exit status reflects whether bugs were found
+  KeepEmpty => 0,# Don't remove output directory even with 0 
results.
+  EnableCheckers => [],
+  DisableCheckers => [],
+  UseCC => undef,# C compiler to use for compilation.
+  UseCXX => undef,   # C++ compiler to use for compilation.
+  AnalyzerTarget => undef,
+  StoreModel => undef,
+  ConstraintsModel => undef,
+  InternalStats => undef,
+  OutputFormat => "html",
+  ConfigOptions => [],   # Options to pass through to the analyzer's 
-analyzer-config flag.
+  ReportFailures => undef,
+  AnalyzerStats => 0,
+  MaxLoop => 0,
+  PluginsToLoad => [],
+  AnalyzerDiscoveryMethod => undef,
+  OverrideCompiler => 0  # The flag corresponding to the 
--override-compiler command line option.
+);
+lock_keys(%Options);
+
 
####
 # Diagnostics
 
####
@@ -231,7 +256,7 @@ sub SetHtmlEnv {
 return;
   }
 
-  if ($Verbose) {
+  if ($Options{Verbose}) {
 Diag("Emitting reports for this run to '$Dir'.\n");
   }
 
@@ -582,7 +607,7 @@ sub Postprocess {
 print OUT <
 
-${HtmlTitle}
+${Options{HtmlTitle}}
 
 
 

Re: [PATCH] D3197: Remove undefined behavior from list::push_back/front, emplace_back/front.

[Marshall Clow via cfe-commits]

mclow.lists abandoned this revision.
mclow.lists added a comment.

There are other suggested fixes for this problem, and this has gotten stale.


http://reviews.llvm.org/D3197



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

Re: [PATCH] D6551: Improvements to scan-build.

[Антон Ярцев via cfe-commits]

ayartsev added a comment.

Removed trailing whitespaces and committed as r246710.
Thanks for reviews and assistance!


http://reviews.llvm.org/D6551



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

r246714 - Use new utility function to clean leading junk from pathnames. NFC

[Douglas Katzman via cfe-commits]

Author: dougk
Date: Wed Sep  2 16:14:53 2015
New Revision: 246714

URL: http://llvm.org/viewvc/llvm-project?rev=246714&view=rev
Log:
Use new utility function to clean leading junk from pathnames. NFC

Modified:
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/lib/Frontend/DependencyFile.cpp

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=246714&r1=246713&r2=246714&view=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Wed Sep  2 16:14:53 2015
@@ -264,6 +264,7 @@ void Clang::AddPreprocessingOptions(Comp
 const char *DepFile;
 if (Arg *MF = Args.getLastArg(options::OPT_MF)) {
   DepFile = MF->getValue();
+  llvm::errs()<<"Your depfile is "getName();
-
-// Remove leading "./" (or ".//" or "././" etc.)
-while (Filename.size() > 2 && Filename[0] == '.' &&
-   llvm::sys::path::is_separator(Filename[1])) {
-  Filename = Filename.substr(1);
-  while (llvm::sys::path::is_separator(Filename[0]))
-Filename = Filename.substr(1);
-}
+StringRef Filename =
+llvm::sys::path::remove_leading_dotslash(FE->getName());
 
 DepCollector.maybeAddDependency(Filename, /*FromModule*/false,
FileType != SrcMgr::C_User,
@@ -295,15 +288,7 @@ void DFGImpl::FileChanged(SourceLocation
   if (!FileMatchesDepCriteria(Filename.data(), FileType))
 return;
 
-  // Remove leading "./" (or ".//" or "././" etc.)
-  while (Filename.size() > 2 && Filename[0] == '.' &&
- llvm::sys::path::is_separator(Filename[1])) {
-Filename = Filename.substr(1);
-while (llvm::sys::path::is_separator(Filename[0]))
-  Filename = Filename.substr(1);
-  }
-
-  AddFilename(Filename);
+  AddFilename(llvm::sys::path::remove_leading_dotslash(Filename));
 }
 
 void DFGImpl::InclusionDirective(SourceLocation HashLoc,


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

r246715 - Remove inadvertent debug output from prior change.

[Douglas Katzman via cfe-commits]

Author: dougk
Date: Wed Sep  2 16:18:10 2015
New Revision: 246715

URL: http://llvm.org/viewvc/llvm-project?rev=246715&view=rev
Log:
Remove inadvertent debug output from prior change.

Modified:
cfe/trunk/lib/Driver/Tools.cpp

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=246715&r1=246714&r2=246715&view=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Wed Sep  2 16:18:10 2015
@@ -264,7 +264,6 @@ void Clang::AddPreprocessingOptions(Comp
 const char *DepFile;
 if (Arg *MF = Args.getLastArg(options::OPT_MF)) {
   DepFile = MF->getValue();
-  llvm::errs()<<"Your depfile is "

Re: [PATCH] D12541: [Sparc][Shave]: Empower the toolchain formerly known as SHAVE to do more.

[James Y Knight via cfe-commits]

jyknight added inline comments.


Comment at: lib/Driver/ToolChains.cpp:3936
@@ -3914,9 +3935,3 @@
 
-SHAVEToolChain::SHAVEToolChain(const Driver &D, const llvm::Triple &Triple,
-   const ArgList &Args)
-: Generic_GCC(D, Triple, Args) {}
-
-SHAVEToolChain::~SHAVEToolChain() {}
-
-/// Following are methods necessary to avoid having moviClang be an abstract
-/// class.
+void MyriadToolChain::getCompilerSupportDir(std::string &Dir) const {
+  // This directory contains crt{i,n,begin,end}.o as well as libgcc.

I wonder if it would be a good idea to add support for other suffixes like "le" 
to the findBiarchMultilibs or somewhere around there instead of hardcoding le/ 
here.

It looks like gcc has a whole bunch more things listed as possible 
MULTILIB_DIRNAMES for various targets. But LLVM seems to implement all the 
generality only for MIPS.


Comment at: lib/Driver/ToolChains.h:934
@@ +933,3 @@
+// Or looked at another way - T might be the uncanonical triple.
+return !(T.getArch() == llvm::Triple::sparcel ||
+ T.getArch() == llvm::Triple::sparc);

Why is this not "T.getArch() == llvm::Triple::shave"?


Comment at: lib/Driver/Tools.cpp:9689
@@ +9688,3 @@
+if (T.getOS() == llvm::Triple::RTEMS) {
+  CmdArgs.push_back(Args.MakeArgString(StartFilesDir + "/crti.o"));
+  CmdArgs.push_back(Args.MakeArgString(StartFilesDir + "/crtbegin.o"));

Isn't whether this is needed dependent upon c++ vs not-c++, rather than rtems 
vs not-rtems?


http://reviews.llvm.org/D12541



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

Re: [PATCH] D12087: always_inline codegen rewrite

[Evgeniy Stepanov via cfe-commits]

eugenis updated this revision to Diff 33868.
eugenis marked 6 inline comments as done.

Repository:
  rL LLVM

http://reviews.llvm.org/D12087

Files:
  lib/CodeGen/CGCXX.cpp
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/ItaniumCXXABI.cpp
  test/CodeGen/always-inline.c
  test/CodeGen/always_inline-unused.c
  test/CodeGen/always_inline-wrappers.c
  test/CodeGen/always_inline.c
  test/CodeGen/function-attributes.c
  test/CodeGen/pr9614.c
  test/CodeGenCXX/alwaysinline.cpp
  test/Frontend/optimization-remark-line-directive.c
  test/Frontend/optimization-remark.c
  test/Modules/cxx-irgen.cpp

Index: test/Modules/cxx-irgen.cpp
===
--- test/Modules/cxx-irgen.cpp
+++ test/Modules/cxx-irgen.cpp
@@ -26,14 +26,14 @@
   };
 }
 
-// CHECK-DAG: define available_externally hidden {{.*}}{{signext i32|i32}} @_ZN1SIiE1gEv({{.*}} #[[ALWAYS_INLINE:.*]] align
+// CHECK-DAG: define internal i32 @_ZN1SIiE1gEv.alwaysinline() #[[ALWAYS_INLINE:.*]] align
 int a = S::g();
 
 int b = h();
 
 // CHECK-DAG: define linkonce_odr {{.*}}{{signext i32|i32}} @_Z3minIiET_S0_S0_(i32
 int c = min(1, 2);
-// CHECK: define available_externally {{.*}}{{signext i32|i32}} @_ZN1SIiE1fEv({{.*}} #[[ALWAYS_INLINE]] align
+// CHECK-DAG: define internal {{.*}}{{signext i32|i32}} @_ZN1SIiE1fEv.alwaysinline() #[[ALWAYS_INLINE]] align
 
 namespace ImplicitSpecialMembers {
   // CHECK-LABEL: define {{.*}} @_ZN22ImplicitSpecialMembers1BC2ERKS0_(
Index: test/Frontend/optimization-remark.c
===
--- test/Frontend/optimization-remark.c
+++ test/Frontend/optimization-remark.c
@@ -32,6 +32,8 @@
 // CHECK-NOT: !llvm.dbg.cu = !{
 
 int foo(int x, int y) __attribute__((always_inline));
+// expected-remark@+2 {{foo.alwaysinline should always be inlined}}
+// expected-remark@+1 {{foo.alwaysinline inlined into foo}}
 int foo(int x, int y) { return x + y; }
 
 float foz(int x, int y) __attribute__((noinline));
@@ -45,7 +47,7 @@
 // expected-remark@+5 {{foz will not be inlined into bar}}
 // expected-remark@+4 {{foz should never be inlined}}
 // expected-remark@+3 {{foz will not be inlined into bar}}
-// expected-remark@+2 {{foo should always be inlined}}
-// expected-remark@+1 {{foo inlined into bar}}
+// expected-remark@+2 {{foo.alwaysinline should always be inlined}}
+// expected-remark@+1 {{foo.alwaysinline inlined into bar}}
   return foo(j, j - 2) * foz(j - 2, j);
 }
Index: test/Frontend/optimization-remark-line-directive.c
===
--- test/Frontend/optimization-remark-line-directive.c
+++ test/Frontend/optimization-remark-line-directive.c
@@ -5,8 +5,9 @@
 // RUN: %clang_cc1 %s -Rpass=inline -gline-tables-only -dwarf-column-info -emit-llvm-only -verify
 
 int foo(int x, int y) __attribute__((always_inline));
+// expected-remark@+1 {{foo.alwaysinline inlined into foo}}
 int foo(int x, int y) { return x + y; }
 
-// expected-remark@+2 {{foo inlined into bar}} expected-note@+2 {{could not determine the original source location for /bad/path/to/original.c:1230:25}}
+// expected-remark@+2 {{foo.alwaysinline inlined into bar}} expected-note@+2 {{could not determine the original source location for /bad/path/to/original.c:1230:25}}
 #line 1230 "/bad/path/to/original.c"
 int bar(int j) { return foo(j, j - 2); }
Index: test/CodeGenCXX/alwaysinline.cpp
===
--- /dev/null
+++ test/CodeGenCXX/alwaysinline.cpp
@@ -0,0 +1,68 @@
+// Test different kinds of alwaysinline *structor definitions.
+
+// RUN: %clang_cc1 -disable-llvm-optzns -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK
+// RUN: %clang_cc1 -disable-llvm-optzns -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-CALL
+
+// RUN: %clang_cc1 -mconstructor-aliases -disable-llvm-optzns -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK
+// RUN: %clang_cc1 -mconstructor-aliases -disable-llvm-optzns -emit-llvm %s -o - | FileCheck %s --check-prefix=CHECK-CALL
+
+struct A1 {
+  __attribute__((__always_inline__)) A1() {}
+  __attribute__((__always_inline__)) ~A1() {}
+};
+
+void g1() {
+  A1 a1;
+}
+
+struct A2 {
+  inline __attribute__((__always_inline__)) A2() {}
+  inline __attribute__((__always_inline__)) ~A2() {}
+};
+
+void g2() {
+  A2 a2;
+}
+
+struct A3 {
+  inline __attribute__((gnu_inline, __always_inline__)) A3() {}
+  inline __attribute__((gnu_inline, __always_inline__)) ~A3() {}
+};
+
+void g3() {
+  A3 a3;
+}
+
+// CHECK-DAG: define internal void @_ZN2A1C1Ev.alwaysinline(%struct.A1* %this) unnamed_addr #[[AI:[01-9]+]]
+// CHECK-DAG: define internal void @_ZN2A1C2Ev.alwaysinline(%struct.A1* %this) unnamed_addr #[[AI]]
+// CHECK-DAG: define internal void @_ZN2A1D1Ev.alwaysinline(%struct.A1* %this) unnamed_addr #[[AI]]
+// CHECK-DAG: define internal void @_ZN2A1D2Ev.alwaysi

Re: [PATCH] D12169: Relax constexpr rules to improve __builtin_object_size's accuracy

[George Burgess IV via cfe-commits]

george.burgess.iv updated this revision to Diff 33875.
george.burgess.iv marked 16 inline comments as done.
george.burgess.iv added a comment.

Addressed all feedback; backed out support for more questionable features 
(negative indices, non-object-boundary offsets, etc), added logic to ignore 
pointless casts, and added tests.

We're down to one new EvalMode (which is only used on `Type=1|3`).


http://reviews.llvm.org/D12169

Files:
  lib/AST/ExprConstant.cpp
  test/CodeGen/object-size.c

Index: test/CodeGen/object-size.c
===
--- test/CodeGen/object-size.c
+++ test/CodeGen/object-size.c
@@ -161,6 +161,15 @@
   gi = __builtin_object_size(&foo.a, 2);
   // CHECK: store i32 4
   gi = __builtin_object_size(&foo.a, 3);
+
+  // CHECK: store i32 4
+  gi = __builtin_object_size(&foo.b, 0);
+  // CHECK: store i32 4
+  gi = __builtin_object_size(&foo.b, 1);
+  // CHECK: store i32 4
+  gi = __builtin_object_size(&foo.b, 2);
+  // CHECK: store i32 4
+  gi = __builtin_object_size(&foo.b, 3);
 }
 
 // CHECK: @test20
@@ -221,25 +230,59 @@
   gi = __builtin_object_size(&t[9].t[10], 2);
   // CHECK: store i32 0
   gi = __builtin_object_size(&t[9].t[10], 3);
+
+  // CHECK: store i32 0
+  gi = __builtin_object_size((char*)&t[0] + sizeof(t), 0);
+  // CHECK: store i32 0
+  gi = __builtin_object_size((char*)&t[0] + sizeof(t), 1);
+  // CHECK: store i32 0
+  gi = __builtin_object_size((char*)&t[0] + sizeof(t), 2);
+  // CHECK: store i32 0
+  gi = __builtin_object_size((char*)&t[0] + sizeof(t), 3);
+
+  // CHECK: store i32 0
+  gi = __builtin_object_size((char*)&t[9].t[0] + 10*sizeof(t[0].t), 0);
+  // CHECK: store i32 0
+  gi = __builtin_object_size((char*)&t[9].t[0] + 10*sizeof(t[0].t), 1);
+  // CHECK: store i32 0
+  gi = __builtin_object_size((char*)&t[9].t[0] + 10*sizeof(t[0].t), 2);
+  // CHECK: store i32 0
+  gi = __builtin_object_size((char*)&t[9].t[0] + 10*sizeof(t[0].t), 3);
 }
 
-struct Test23Ty { int t[10]; };
+struct Test23Ty { int a; int t[10]; };
 
 // CHECK: @test23
-void test23(struct Test22Ty *p) {
+void test23(struct Test23Ty *p) {
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size(p, 0);
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size(p, 1);
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true)
   gi = __builtin_object_size(p, 2);
-
   // Note: this is currently fixed at 0 because LLVM doesn't have sufficient
   // data to correctly handle type=3
   // CHECK: store i32 0
   gi = __builtin_object_size(p, 3);
-}
 
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(&p->a, 0);
+  // CHECK: store i32 4
+  gi = __builtin_object_size(&p->a, 1);
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true)
+  gi = __builtin_object_size(&p->a, 2);
+  // CHECK: store i32 4
+  gi = __builtin_object_size(&p->a, 3);
+
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(&p->t[5], 0);
+  // CHECK: store i32 20
+  gi = __builtin_object_size(&p->t[5], 1);
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true)
+  gi = __builtin_object_size(&p->t[5], 2);
+  // CHECK: store i32 20
+  gi = __builtin_object_size(&p->t[5], 3);
+}
 
 // PR24493 -- ICE if __builtin_object_size called with NULL and (Type & 1) != 0
 // CHECK: @test24
@@ -280,3 +323,71 @@
   // CHECK: store i32 0
   gi = __builtin_object_size((void*)0 + 0x1000, 3);
 }
+
+// CHECK: @test26
+void test26() {
+  struct { int v[10]; } t[10];
+
+  // CHECK: store i32 316
+  gi = __builtin_object_size(&t[1].v[11], 0);
+  // CHECK: store i32 312
+  gi = __builtin_object_size(&t[1].v[12], 1);
+  // CHECK: store i32 308
+  gi = __builtin_object_size(&t[1].v[13], 2);
+  // CHECK: store i32 0
+  gi = __builtin_object_size(&t[1].v[14], 3);
+}
+
+struct Test27IncompleteTy;
+
+// CHECK: @test27
+void test27(struct Test27IncompleteTy *t) {
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(t, 0);
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(t, 1);
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true)
+  gi = __builtin_object_size(t, 2);
+  // Note: this is currently fixed at 0 because LLVM doesn't have sufficient
+  // data to correctly handle type=3
+  // CHECK: store i32 0
+  gi = __builtin_object_size(t, 3);
+
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* {{.*}}, i1 false)
+  gi = __builtin_object_size(&test27, 0);
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* {{.*}}, i1 false)
+  gi = __builtin_object_size(&test27, 1);
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* {{.*}}, i1 true)
+  gi = __builtin_object_size(&test27, 2);
+  // Note: this is currently fixed at 0 because LLVM doesn't have sufficient
+  // data to correctly handle type=3
+  // 

Re: [PATCH] D11279: Initial patch for PS4 toolchain

[Katya Romanova via cfe-commits]

kromanova added inline comments.


Comment at: lib/Frontend/InitHeaderSearch.cpp:325-344
@@ +324,22 @@
+  case llvm::Triple::PS4: {
+//  gets prepended later in AddPath().
+std::string BaseSDKPath = "";
+if (!HasSysroot) {
+  const char *envValue = getenv("SCE_PS4_SDK_DIR");
+  if (envValue)
+BaseSDKPath = envValue;
+  else {
+// HSOpts.ResourceDir variable contains the location of Clang's
+// resource files.
+// Assuming that Clang is configured for PS4 without
+// --with-clang-resource-dir option, the location of Clang's resource
+// files is /host_tools/lib/clang
+SmallString<128> P = StringRef(HSOpts.ResourceDir);
+llvm::sys::path::append(P, "../../..");
+BaseSDKPath = P.str();
+  }
+}
+AddPath(BaseSDKPath + "/target/include", System, false);
+if (triple.isPS4CPU())
+  AddPath(BaseSDKPath + "/target/include_common", System, false);
+  }

echristo wrote:
> This all seems odd, what's going on here?
Our SDK layout structure is different from other platforms. Some of the PS4 
specific headers (e.g. graphic headers, etc) are placed into 
target/include_common directory and we want to have them in the search path.


http://reviews.llvm.org/D11279



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

Re: [PATCH] D11279: Initial patch for PS4 toolchain

[Eric Christopher via cfe-commits]

echristo added a comment.

Replying to the inline comments. I know we're waiting on the linker stuff, but 
Katya made me realize I hadn't replied to your comments.

-eric



Comment at: include/clang/Basic/DiagnosticDriverKinds.td:203-208
@@ +202,8 @@
+  InGroup;
+def warn_drv_ps4_sdk_include : Warning<
+  "unable to find PS4 system headers directory, expected to be in '%0'">,
+  InGroup;
+def warn_drv_ps4_sdk_lib : Warning<
+  "unable to find PS4 system libraries directory, expected to be in '%0'">,
+  InGroup;
+

filcab wrote:
> echristo wrote:
> > Are the existing header warnings insufficient?
> Yes. We have an expected location and would like the whole "PS4 SDK headers" 
> thing in there.
> But I can change it to be more general, like:
> 
>   def warn_drv_unable_find_directory_expected : Warning<
> "unable to find %0 directory, expected to be in '%1'">,
> InGroup;
> 
> That might be preferable. That way other toolchains can use it (and we can 
> merge the "libraries" and "headers" warnings.
Seems totally reasonable :)

I can see it being something that you'd want to enable to make sure that 
sysroot prefixes are paid attention to and, e.g. /usr/include is never touched.


Comment at: lib/Driver/Tools.cpp:3590-3591
@@ -3580,4 +3589,4 @@
   Args.ClaimAllArgs(options::OPT_g_flags_Group);
   if (Args.hasFlag(options::OPT_gcolumn_info, options::OPT_gno_column_info,
-   /*Default*/ true))
+   /*Default*/ !Triple.isPS4CPU()))
 CmdArgs.push_back("-dwarf-column-info");

filcab wrote:
> echristo wrote:
> > Hmm?
> We have different defaults from other platforms.
*nod* disabling column info yes? I have "opinions" on this, but it's not my 
platform. Needs a comment.


Comment at: lib/Driver/Tools.cpp:3613
@@ -3603,3 +3612,3 @@
   // backend.
-  if (Args.hasArg(options::OPT_gdwarf_aranges)) {
+  if (Args.hasArg(options::OPT_gdwarf_aranges) || Triple.isPS4CPU()) {
 CmdArgs.push_back("-backend-option");

filcab wrote:
> echristo wrote:
> > Ditto.
> Ditto, different defaults.
> But I guess I can hoist out the Triple.isPS4CPU() on both cases, if you 
> prefer, like it's done for other toolchains like:
>   bool IsWindowsMSVC = getToolChain().getTriple().isWindowsMSVCEnvironment();
Probably better to just handle it the same way as the column info I'd think. 
Also, this is all terrible and needs to change, that said, not your problem. :)


Comment at: lib/Driver/Tools.h:783
@@ -782,1 +782,3 @@
 
+namespace PS4cpu {
+class LLVM_LIBRARY_VISIBILITY Assemble : public Tool {

filcab wrote:
> echristo wrote:
> > Hrm. PS4 maybe? PS4cpu seems to say that it's a custom cpu target and not a 
> > custom environment?
> PS4cpu (or PS4CPU) is more consistent with how we've been naming things in 
> open source.
Sure.


Comment at: lib/Frontend/InitHeaderSearch.cpp:325-344
@@ +324,22 @@
+  case llvm::Triple::PS4: {
+//  gets prepended later in AddPath().
+std::string BaseSDKPath = "";
+if (!HasSysroot) {
+  const char *envValue = getenv("SCE_PS4_SDK_DIR");
+  if (envValue)
+BaseSDKPath = envValue;
+  else {
+// HSOpts.ResourceDir variable contains the location of Clang's
+// resource files.
+// Assuming that Clang is configured for PS4 without
+// --with-clang-resource-dir option, the location of Clang's resource
+// files is /host_tools/lib/clang
+SmallString<128> P = StringRef(HSOpts.ResourceDir);
+llvm::sys::path::append(P, "../../..");
+BaseSDKPath = P.str();
+  }
+}
+AddPath(BaseSDKPath + "/target/include", System, false);
+if (triple.isPS4CPU())
+  AddPath(BaseSDKPath + "/target/include_common", System, false);
+  }

kromanova wrote:
> echristo wrote:
> > This all seems odd, what's going on here?
> Our SDK layout structure is different from other platforms. Some of the PS4 
> specific headers (e.g. graphic headers, etc) are placed into 
> target/include_common directory and we want to have them in the search path.
You also check for ps4cpu in here?


http://reviews.llvm.org/D11279



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

Re: [PATCH] D11737: Add -linker (and -linker=) alias for -fuse-ld=

[Katya Romanova via cfe-commits]

kromanova added inline comments.


Comment at: include/clang/Driver/Options.td:1853-1854
@@ -1853,2 +1852,4 @@
+def fuse_ld_EQ : Joined<["-"], "fuse-ld=">, HelpText<"Use linker ">, 
Group;
+def linker_EQ : Joined<["-"], "linker=">, Alias, 
MetaVarName<"">;
 
 defm align_functions : BooleanFFlag<"align-functions">, 
Group;

We will remove -linker alias. We had to confirm with the interested parties 
that they are OK with this option being removed.


http://reviews.llvm.org/D11737



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

Re: [PATCH] D11279: Initial patch for PS4 toolchain

[Katya Romanova via cfe-commits]

kromanova added inline comments.


Comment at: lib/Frontend/InitHeaderSearch.cpp:325-344
@@ +324,22 @@
+  case llvm::Triple::PS4: {
+//  gets prepended later in AddPath().
+std::string BaseSDKPath = "";
+if (!HasSysroot) {
+  const char *envValue = getenv("SCE_PS4_SDK_DIR");
+  if (envValue)
+BaseSDKPath = envValue;
+  else {
+// HSOpts.ResourceDir variable contains the location of Clang's
+// resource files.
+// Assuming that Clang is configured for PS4 without
+// --with-clang-resource-dir option, the location of Clang's resource
+// files is /host_tools/lib/clang
+SmallString<128> P = StringRef(HSOpts.ResourceDir);
+llvm::sys::path::append(P, "../../..");
+BaseSDKPath = P.str();
+  }
+}
+AddPath(BaseSDKPath + "/target/include", System, false);
+if (triple.isPS4CPU())
+  AddPath(BaseSDKPath + "/target/include_common", System, false);
+  }

echristo wrote:
> kromanova wrote:
> > echristo wrote:
> > > This all seems odd, what's going on here?
> > Our SDK layout structure is different from other platforms. Some of the PS4 
> > specific headers (e.g. graphic headers, etc) are placed into 
> > target/include_common directory and we want to have them in the search path.
> You also check for ps4cpu in here?
I don't think this is needed since we are under Triple::PS4 switch now. Sorry, 
this is an oversight on our part, caused by refactoring of old code that didn't 
has PS4 switch case before. We will remove 'if' part.


http://reviews.llvm.org/D11279



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

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

[Akira Hatanaka via cfe-commits]

ahatanak updated this revision to Diff 33882.

http://reviews.llvm.org/D12547

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  lib/CodeGen/CGCall.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGen/attr-disable-tail-calls.c

Index: test/CodeGen/attr-disable-tail-calls.c
===
--- test/CodeGen/attr-disable-tail-calls.c
+++ test/CodeGen/attr-disable-tail-calls.c
@@ -1,11 +1,19 @@
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.7.0 %s -emit-llvm 
-mdisable-tail-calls -o - | FileCheck %s -check-prefix=CHECK 
-check-prefix=DISABLE
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.7.0 %s -emit-llvm -o - | 
FileCheck %s -check-prefix=CHECK -check-prefix=ENABLE
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.7.0 %s -emit-llvm 
-mdisable-tail-calls -o - | FileCheck %s -check-prefix=DISABLE
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.7.0 %s -emit-llvm -o - | 
FileCheck %s -check-prefix=ENABLE
 
-// CHECK: define i32 @f1() [[ATTR:#[0-9]+]] {
+// DISABLE: define i32 @f1() [[ATTRTRUE:#[0-9]+]] {
+// DISABLE: define i32 @f2() [[ATTRTRUE]] {
+// ENABLE: define i32 @f1() [[ATTRFALSE:#[0-9]+]] {
+// ENABLE: define i32 @f2() [[ATTRTRUE:#[0-9]+]] {
 
 int f1() {
   return 0;
 }
 
-// DISABLE: attributes [[ATTR]] = { {{.*}} "disable-tail-calls"="true" {{.*}} }
-// ENABLE: attributes [[ATTR]] = { {{.*}} "disable-tail-calls"="false" {{.*}} }
+int f2() __attribute__((disable_tail_calls)) {
+  return 0;
+}
+
+// DISABLE: attributes [[ATTRTRUE]] = { {{.*}} "disable-tail-calls"="true" 
{{.*}} }
+// ENABLE: attributes [[ATTRFALSE]] = { {{.*}} "disable-tail-calls"="false" 
{{.*}} }
+// ENABLE: attributes [[ATTRTRUE]] = { {{.*}} "disable-tail-calls"="true" 
{{.*}} }
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -4878,6 +4878,9 @@
   case AttributeList::AT_ReturnsTwice:
 handleSimpleAttribute(S, D, Attr);
 break;
+  case AttributeList::AT_DisableTailCalls:
+handleSimpleAttribute(S, D, Attr);
+break;
   case AttributeList::AT_Used:
 handleUsedAttr(S, D, Attr);
 break;
Index: lib/CodeGen/CGCall.cpp
===
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -1474,8 +1474,12 @@
   FuncAttrs.addAttribute("no-frame-pointer-elim-non-leaf");
 }
 
+bool DisableTailCalls =
+(TargetDecl && TargetDecl->hasAttr()) ||
+CodeGenOpts.DisableTailCalls;
 FuncAttrs.addAttribute("disable-tail-calls",
-   llvm::toStringRef(CodeGenOpts.DisableTailCalls));
+   llvm::toStringRef(DisableTailCalls));
+
 FuncAttrs.addAttribute("less-precise-fpmad",
llvm::toStringRef(CodeGenOpts.LessPreciseFPMAD));
 FuncAttrs.addAttribute("no-infs-fp-math",
Index: include/clang/Basic/AttrDocs.td
===
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -1612,3 +1612,10 @@
 arguments, with arbitrary offsets.
   }];
 }
+
+def DisableTailCallsDocs : Documentation {
+  let Category = DocCatFunction;
+  let Content = [{
+Use ``__attribute__((disable_tail_calls)))`` to instruct the backend not to do 
tail call optimization.
+  }];
+}
Index: include/clang/Basic/Attr.td
===
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -891,6 +891,13 @@
   let Documentation = [Undocumented];
 }
 
+def DisableTailCalls : InheritableAttr {
+  let Spellings = [GNU<"disable_tail_calls">,
+   CXX11<"clang", "disable_tail_calls">];
+  let Subjects = SubjectList<[Function, ObjCMethod]>;
+  let Documentation = [DisableTailCallsDocs];
+}
+
 def NoAlias : InheritableAttr {
   let Spellings = [Declspec<"noalias">];
   let Subjects = SubjectList<[Function]>;


Index: test/CodeGen/attr-disable-tail-calls.c
===
--- test/CodeGen/attr-disable-tail-calls.c
+++ test/CodeGen/attr-disable-tail-calls.c
@@ -1,11 +1,19 @@
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.7.0 %s -emit-llvm -mdisable-tail-calls -o - | FileCheck %s -check-prefix=CHECK -check-prefix=DISABLE
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.7.0 %s -emit-llvm -o - | FileCheck %s -check-prefix=CHECK -check-prefix=ENABLE
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.7.0 %s -emit-llvm -mdisable-tail-calls -o - | FileCheck %s -check-prefix=DISABLE
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.7.0 %s -emit-llvm -o - | FileCheck %s -check-prefix=ENABLE
 
-// CHECK: define i32 @f1() [[ATTR:#[0-9]+]] {
+// DISABLE: define i32 @f1() [[ATTRTRUE:#[0-9]+]] {
+// DISABLE: define i32 @f2() [[ATTRTRUE]] {
+// ENABLE: define i32 @f1() [[ATTRFALSE:#[0-9]+]] {
+// ENABLE: define i32 @f2() [[ATTRTRUE:#[0-9]+

Re: [PATCH] D12547: Add support for function attribute "disable_tail_calls"

[Akira Hatanaka via cfe-commits]

ahatanak added inline comments.


Comment at: include/clang/Basic/Attr.td:894
@@ -893,1 +893,3 @@
 
+def DisableTailCalls : InheritableAttr {
+  let Spellings = [GNU<"disable_tail_calls">];

aaron.ballman wrote:
> We already have an attribute that is tangentially related: OptimizeNone. 
> Would it make sense to have that attribute take arguments that control what 
> optimizations are enabled/disabled? How should these two attributes (and the 
> resulting functionality) interact? For instance, if I specify optnone on a 
> function, should that also disable tail call optimizations by creating an 
> implicit instance of this new attribute?
optnone (or -O0) doesn't disable tail call optimization if clang attaches 
"tail" to a call instruction (see lib/CodeGen/CGObjC.cpp for examples). I think 
tail call optimization should be disabled even for functions marked as "tail" 
by clang if the user adds an attribute to the function in the source code with 
the intention of disabling tail call optimization. For that reason, I don't 
think we can use OptimizeNone for this purpose.


Comment at: include/clang/Basic/Attr.td:895
@@ +894,3 @@
+def DisableTailCalls : InheritableAttr {
+  let Spellings = [GNU<"disable_tail_calls">];
+  let Subjects = SubjectList<[Function]>;

aaron.ballman wrote:
> Do we also want a C++ spelling for this attribute, under the clang namespace?
Done.


Comment at: include/clang/Basic/Attr.td:896
@@ +895,3 @@
+  let Spellings = [GNU<"disable_tail_calls">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];

aaron.ballman wrote:
> Should this also apply to Objective-C methods?
Done.


Comment at: include/clang/Basic/Attr.td:897
@@ +896,3 @@
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [Undocumented];
+}

aaron.ballman wrote:
> Please document this attribute.
Done.


Comment at: lib/CodeGen/CGCall.cpp:1477
@@ -1476,3 +1476,3 @@
 
-FuncAttrs.addAttribute("disable-tail-calls",
-   llvm::toStringRef(CodeGenOpts.DisableTailCalls));
+if ((TargetDecl && TargetDecl->hasAttr()) ||
+CodeGenOpts.DisableTailCalls)

aaron.ballman wrote:
> Better to move this into the block that already tests TargetDecl (around line 
> 1403 or so).
It looks like moving this code into the block would make the code mode complex, 
so I followed your suggestion below.


Comment at: lib/CodeGen/CGCall.cpp:1480
@@ +1479,3 @@
+  FuncAttrs.addAttribute("disable-tail-calls", "true");
+else
+  FuncAttrs.addAttribute("disable-tail-calls", "false");

aaron.ballman wrote:
> Can do this in a single line with ?:
I defined a boolean variable and passed it to llvm::toStringRef to avoid 
calling addAttribute in two places.


Comment at: lib/Sema/SemaDeclAttr.cpp:4882
@@ +4881,3 @@
+  case AttributeList::AT_DisableTailCalls:
+handleSimpleAttribute(S, D, Attr);
+break;

aaron.ballman wrote:
> Are there semantic checks we would like to perform on the declaration for 
> nonsense uses of this attribute, such as functions marked [[noreturn]], etc?
Some of the users asked for this function attribute to ensure stack frames are 
fully preserved for backtrace, so it's entirely reasonable for a user to mark a 
function "noreturn" that calls a non-returning function and at the same time 
expect the backend to avoid tail call optimization. I'm thinking we shouldn't 
prohibit attaching "noreturn" and "disable_tail_calls" to the same function. 
What do you think?

Other than "noreturn", I didn't find any attributes that shouldn't be used with 
"disable_tail_calls" in the following link:

http://clang.llvm.org/docs/AttributeReference.html


http://reviews.llvm.org/D12547



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

Re: [PATCH] D11279: Initial patch for PS4 toolchain

[Jonathan Roelofs via cfe-commits]

jroelofs added a subscriber: jroelofs.


Comment at: lib/Frontend/InitHeaderSearch.cpp:340
@@ +339,3 @@
+BaseSDKPath = P.str();
+  }
+}

The lifetime of `P` ends here, yet a reference to it lives on because of the `= 
P.str()`. Using it later on line 344 is UB.


http://reviews.llvm.org/D11279



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

Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

[Evgeniy Stepanov via cfe-commits]

eugenis added inline comments.


Comment at: lib/CodeGen/CGCXX.cpp:45
@@ +44,3 @@
+  // destructors.
+  if (getCodeGenOpts().SanitizeMemoryUseAfterDtor)
+return true;

This simply suppresses all dtor alias under UseAfterDtor, effectively disabling 
the second check below.
Remove it.


Comment at: lib/CodeGen/CGCXX.cpp:130
@@ -115,3 +129,3 @@
  bool InEveryTU) {
-  if (!getCodeGenOpts().CXXCtorDtorAliases)
+  if(!getCodeGenOpts().CXXCtorDtorAliases)
 return true;

formatting


Comment at: lib/CodeGen/CGCXX.cpp:147
@@ +146,3 @@
+  // If sanitizing memory to check for use-after-dtor, do not emit as
+  //  an alias, unless this class owns no members.
+  const CXXMethodDecl *MD =

... with trivial destructors


Comment at: lib/CodeGen/CGClass.cpp:1547
@@ +1546,3 @@
+  // Nothing to poison.
+  if (Layout.getFieldCount() == 0)
+return;

Probably better check Dtor->getParent()->field_empty() for consistency.


Comment at: lib/CodeGen/CGClass.cpp:1551
@@ +1550,3 @@
+  // Prevent the current stack frame from disappearing from the stack 
trace.
+  CGF.CurFn->addFnAttr("disable-tail-calls", "true");
+

disable tail calls only when !field_empty()


http://reviews.llvm.org/D12022



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

Re: [PATCH] D11279: Initial patch for PS4 toolchain

[Katya Romanova via cfe-commits]

kromanova added inline comments.


Comment at: lib/Frontend/InitHeaderSearch.cpp:340
@@ +339,3 @@
+BaseSDKPath = P.str();
+  }
+}

jroelofs wrote:
> The lifetime of `P` ends here, yet a reference to it lives on because of the 
> `= P.str()`. Using it later on line 344 is UB.
Good catch. Thank you! We will fix it.


http://reviews.llvm.org/D11279



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

[PATCH] D12579: AST: simplify handling of the mangling

[Saleem Abdulrasool via cfe-commits]

compnerd created this revision.
compnerd added reviewers: rafael, rnk.
compnerd added a subscriber: cfe-commits.

Simplify the mangling handling.  This allows for the correct handling of extern
"C" symbols in C++ input.  The observable difference with this simplification 
that additional symbols are marked as naked (\01 prefix), namely asm
labels and redefine_extname.

http://reviews.llvm.org/D12579

Files:
  lib/AST/Mangle.cpp
  test/CodeGen/asm-label.c
  test/CodeGen/mangle.c
  test/CodeGen/pr9614.c
  test/CodeGen/redefine_extname.c
  test/CodeGenCXX/redefine_extname.cpp
  test/CodeGenObjCXX/mangling.mm

Index: test/CodeGenObjCXX/mangling.mm
===
--- /dev/null
+++ test/CodeGenObjCXX/mangling.mm
@@ -0,0 +1,88 @@
+// RUN: %clang_cc1 -triple i686-pc-linux -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-IA
+// RUN: %clang_cc1 -triple i386-darwin -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-IA
+// RUN: %clang_cc1 -triple i686-pc-windows-msvc -emit-llvm -o - %s | FileCheck %s -check-prefix CHECK -check-prefix CHECK-MS
+
+extern "C" void extern_c_function(void) { }
+// CHECK: @extern_c_function
+
+extern "C++" void extern_cpp_function(void) { }
+// CHECK-IA: @_Z19extern_cpp_function
+// CHECK-MS: @"\01?extern_cpp_function@@YAXXZ"
+
+void asm_label(void) __asm__("alternate_label");
+void asm_label(void) { }
+// CHECK: @"\01alternate_label"
+
+extern "C++" void extern_cpp_asm_label(void) __asm__("_ZN3cppE");
+extern "C++" void extern_cpp_asm_label(void) { }
+// CHECK: @"\01_ZN3cppE"
+
+#if !defined(_MSC_VER)
+__attribute__ (( objc_root ))
+@interface I
++ (void) class_method;
+- (void) instance_method;
++ (void) class_stdcall_method __attribute__ (( stdcall ));
+- (void) instance_stdcall_method __attribute__ (( stdcall ));
++ (void) class_fastcall_method __attribute__ (( fastcall ));
+- (void) instance_fastcall_method __attribute__ (( fastcall ));
+@end
+
+@implementation I
++ (void) class_method { }
+- (void) instance_method { }
++ (void) class_stdcall_method __attribute__ (( stdcall )) { }
+- (void) instance_stdcall_method __attribute__ (( stdcall )) { }
++ (void) class_fastcall_method __attribute__ (( fastcall )) { }
+- (void) instance_fastcall_method __attribute__ (( fastcall )) { }
+@end
+
+// CHECK-IA: @"\01+[I class_method]"
+// CHECK-IA: @"\01-[I instance_method]"
+// CHECK-IA: @"\01+[I class_stdcall_method]"
+// CHECK-IA: @"\01-[I instance_stdcall_method]"
+// CHECK-IA: @"\01+[I class_fastcall_method]"
+// CHECK-IA: @"\01-[I instance_fastcall_method]"
+#endif
+
+extern "C" void __attribute__ (( stdcall )) stdcall_c_function(void) { }
+// CHECK-IA: @stdcall_c_function
+// CHECK-MS: @"\01_stdcall_c_function@0"
+extern "C++" void __attribute__ (( stdcall )) stdcall_cpp_function(void) { }
+// CHECK-IA: @_Z20stdcall_cpp_functionv
+// CHECK-MS: @"\01?stdcall_cpp_function@@YGXXZ"
+
+extern "C" void __attribute__ (( fastcall )) fastcall_c_function(void) { }
+// CHECK-IA: @fastcall_c_function
+// CHECK-MS: @"\01@fastcall_c_function@0"
+extern "C++" void __attribute__ (( fastcall )) fastcall_cpp_function(void) { }
+// CHECK-IA: @_Z21fastcall_cpp_functionv
+// CHECK-MS: @"\01?fastcall_cpp_function@@YIXXZ"
+
+extern "C" void __attribute__ (( vectorcall )) vectorcall_c_function(void) { }
+// CHECK-IA: @vectorcall_c_function
+// CHECK-MS: @"\01vectorcall_c_function@@0"
+extern "C++" void __attribute__ (( vectorcall )) vectorcall_cpp_function(void) { }
+// CHECK-IA: @_Z23vectorcall_cpp_functionv
+// CHECK-MS: @"\01?vectorcall_cpp_function@@YQXXZ"
+
+struct __attribute__ (( dllexport )) S {
+  void __attribute__ (( thiscall )) thiscall_method(void);
+  void __attribute__ (( stdcall )) stdcall_method(void);
+  void __attribute__ (( fastcall )) fastcall_method(void);
+  void __attribute__ (( vectorcall )) vectorcall_method(void);
+};
+
+void __attribute__ (( thiscall )) S::thiscall_method(void) { }
+// CHECK-IA: @_ZN1S15thiscall_methodEv
+// CHECK-MS: @"\01?thiscall_method@S@@QAEXXZ"
+void __attribute__ (( stdcall )) S::stdcall_method(void) { }
+// CHECK-IA: @_ZN1S14stdcall_methodEv
+// CHECK-MS: @"\01?stdcall_method@S@@QAGXXZ"
+void __attribute__ (( fastcall )) S::fastcall_method(void) { }
+// CHECK-IA: @_ZN1S15fastcall_methodEv
+// CHECK-MS: @"\01?fastcall_method@S@@QAIXXZ"
+void __attribute__ (( vectorcall )) S::vectorcall_method(void) { }
+// CHECK-IA: @_ZN1S17vectorcall_methodEv
+// CHECK-MS: @"\01?fastcall_method@S@@QAQXXZ"
+
Index: test/CodeGenCXX/redefine_extname.cpp
===
--- test/CodeGenCXX/redefine_extname.cpp
+++ test/CodeGenCXX/redefine_extname.cpp
@@ -13,7 +13,7 @@
   statvfs64(&st);
 // Check that even if there is a structure with redefined name before the
 // pragma, subsequent function name redefined properly. PR5172, Comment 11.
-// CHECK:  call i32 @statvfs(%struct.statvfs64* %st)
+// CHECK:  call i32 @"\01statvfs"(%struct.sta

Re: [PATCH] D12579: AST: simplify handling of the mangling

[Saleem Abdulrasool via cfe-commits]

compnerd added inline comments.


Comment at: lib/AST/Mangle.cpp:191
@@ -199,3 +190,3 @@
   else
-Out << "_block_invoke_" << discriminator+1;
+Out << "_block_invoke_" << discriminator + 1;
 }

Drive by reformat.


Comment at: test/CodeGen/pr9614.c:21
@@ -20,3 +20,3 @@
 
-extern inline __attribute__((__always_inline__, __gnu_inline__)) void 
*memchr(void *__s, int __c, __SIZE_TYPE__ __n) {
+extern inline __attribute__((__always_inline__, __gnu_inline__)) void 
*memchr(const void *__s, int __c, unsigned long __n) {
   return __builtin_memchr(__s, __c, __n);

Drive-by redeclaration to reduce a warnings.


http://reviews.llvm.org/D12579



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

Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

[Naomi Musgrave via cfe-commits]

nmusgrave updated this revision to Diff 33887.
nmusgrave marked 4 inline comments as done.
nmusgrave added a comment.

- Update comments, consistent style for attribute checking.


http://reviews.llvm.org/D12022

Files:
  lib/CodeGen/CGCXX.cpp
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CodeGenModule.h
  test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
  test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp

Index: test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-repress-aliasing.cpp
@@ -0,0 +1,30 @@
+// Test -fsanitize-memory-use-after-dtor
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O2 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template 
+class Vector {
+public:
+  int size;
+  ~Vector() {}
+};
+
+// Virtual function table for the derived class only contains
+// its own destructors, with no aliasing to base class dtors.
+struct Base {
+  Vector v;
+  int x;
+  Base() { x = 5; }
+  virtual ~Base() {}
+};
+
+struct Derived : public Base {
+  int z;
+  Derived() { z = 10; }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Definition of virtual function table
+// CHECK: @_ZTV7Derived = {{.*}}@_ZN7DerivedD1Ev{{.*}}@_ZN7DerivedD0Ev
Index: test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
===
--- /dev/null
+++ test/CodeGenCXX/sanitize-dtor-nontrivial-virtual-base.cpp
@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 -fsanitize=memory -O0 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -fsanitize=memory -O1 -fsanitize-memory-use-after-dtor -std=c++11 -triple=x86_64-pc-linux -emit-llvm -o - %s | FileCheck %s
+
+template 
+class Vector {
+public:
+  int size;
+  ~Vector() {
+size += 1;
+  }
+};
+
+struct Base {
+  int b1;
+  double b2;
+  Base() {
+b1 = 5;
+b2 = 10.989;
+  }
+  virtual ~Base() {}
+};
+
+struct VirtualBase {
+  int vb1;
+  int vb2;
+  VirtualBase() {
+vb1 = 10;
+vb2 = 11;
+  }
+  virtual ~VirtualBase() {}
+};
+
+struct Derived : public Base, public virtual VirtualBase {
+  int d1;
+  Vector v;
+  int d2;
+  Derived() {
+d1 = 10;
+  }
+  ~Derived() {}
+};
+
+Derived d;
+
+// Destruction order:
+// Derived: int, Vector, Base, VirtualBase
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD1Ev
+// CHECK: call void {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN7DerivedD0Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD1Ev
+// CHECK: ret void
+
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD0Ev
+// CHECK: ret void
+
+// poison 2 ints
+// CHECK-LABEL: define {{.*}}ZN11VirtualBaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 8)
+// CHECK: ret void
+
+// poison int and double
+// CHECK-LABEL: define {{.*}}ZN4BaseD2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 16)
+// CHECK: ret void
+
+// poison int, ignore vector, poison int
+// CHECK-LABEL: define {{.*}}ZN7DerivedD2Ev
+// CHECK: call void {{.*}}ZN6VectorIiED1Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: call void {{.*}}ZN4BaseD2Ev
+// CHECK: ret void
+
+// poison int
+// CHECK-LABEL: define {{.*}}ZN6VectorIiED2Ev
+// CHECK: call void {{.*}}sanitizer_dtor_callback({{.*}}, i64 4)
+// CHECK: ret void
Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -1099,6 +1099,13 @@
   /// are emitted lazily.
   void EmitGlobal(GlobalDecl D);
 
+  bool
+  HasTrivialDestructorBody(ASTContext &Context,
+   const CXXRecordDecl *BaseClassDecl,
+   const CXXRecordDecl *MostDerivedClassDecl);
+  bool
+  FieldHasTrivialDestructorBody(ASTContext &Context, const FieldDecl *Field);
+
   bool TryEmitDefinitionAsAlias(GlobalDecl Alias, GlobalDecl Target,
 bool InEveryTU);
   bool TryEmitBaseDestructorAsAlias(const CXXDestructorDecl *D);
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -1286,11 +1286,7 @@
   CM.finish();
 }
 
-static bool
-FieldHasTrivialDestructorBody(ASTContext &Context, const FieldDecl *Field);
-
-static bool
-HasTrivialDestructorBody(ASTContext &Context,
+bool CodeGenModule::HasTrivialDestructorBody(ASTContext &Context,
  const CXXRecordDecl *BaseClassDecl,
  const CXXRecordDecl *MostDerivedClassDecl)
 {
@@ -1332,9 +1328,8 @@
   return true;
 }
 
-static bool
-FieldHa

Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

[Evgeniy Stepanov via cfe-commits]

eugenis added a comment.

LGTM but please wait for Richard's answer



Comment at: lib/CodeGen/CGClass.cpp:1551
@@ +1550,3 @@
+  // Prevent the current stack frame from disappearing from the stack 
trace.
+  CGF.CurFn->addFnAttr("disable-tail-calls", "true");
+

oh, right, that's already the case.


http://reviews.llvm.org/D12022



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

Re: [PATCH] D12022: Refactored dtor sanitizing into EHScopeStack

[Richard Smith via cfe-commits]

rsmith added a comment.

I'd like to see some tests for poisoning bitfields, and particularly anonymous 
bitfields (I think the code works fine in those cases, but they're a bit 
special so explicitly testing them is useful).



Comment at: lib/CodeGen/CGCXX.cpp:151
@@ +150,3 @@
+  if (isa(MD) && getCodeGenOpts().SanitizeMemoryUseAfterDtor
+  && HasFieldWithTrivialDestructor(*this, MD->getParent()))
+return true;

&& goes on previous line.


Comment at: lib/CodeGen/CGClass.cpp:1547
@@ +1546,3 @@
+  // Nothing to poison.
+  if (Layout.getFieldCount() == 0)
+return;

eugenis wrote:
> Probably better check Dtor->getParent()->field_empty() for consistency.
This check is equivalent and more efficient.


Comment at: lib/CodeGen/CGClass.cpp:1559
@@ +1558,3 @@
+  RecordDecl::field_iterator Field;
+  for (Field = Dtor->getParent()->field_begin();
+   Field != Dtor->getParent()->field_end(); Field++) {

Can you use a range for?


Comment at: lib/CodeGen/CGClass.cpp:1587
@@ +1586,3 @@
+// layoutEndOffset: index of the ASTRecordLayout field to end poisoning
+//   (exclusive)
+void PoisonBlock(CodeGenFunction &CGF, unsigned layoutStartOffset,

Doxygenize this comment  (use ///, \param).


http://reviews.llvm.org/D12022



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

[PATCH] D12580: Added StrictVTablePointers linking requirement

[Piotr Padlewski via cfe-commits]

Prazek created this revision.
Prazek added reviewers: rsmith, rjmccall, majnemer, nlewycky.
Prazek added a subscriber: cfe-commits.

Generates code like this
!llvm.module.flags = !{!0, !1}


!0 = !{i32 1, !"StrictVTablePointers", i32 1}
!1 = !{i32 3, !"StrictVTablePointersRequirement", !2}
!2 = !{!"StrictVTablePointers", i32 1}


http://reviews.llvm.org/D12580

Files:
  lib/CodeGen/CodeGenModule.cpp

Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -394,6 +394,22 @@
 // Indicate that we want CodeView in the metadata.
 getModule().addModuleFlag(llvm::Module::Warning, "CodeView", 1);
   }
+  if (CodeGenOpts.OptimizationLevel > 0 && CodeGenOpts.StrictVTablePointers) {
+// We don't support LTO with 2 with different StrictVTablePointers
+// FIXME: we could support it by stripping all the information introduced
+// by StrictVTablePointers.
+
+getModule().addModuleFlag(llvm::Module::Error, "StrictVTablePointers",1);
+
+llvm::Metadata *Ops[2] = {
+  llvm::MDString::get(VMContext, "StrictVTablePointers"),
+  llvm::ConstantAsMetadata::get(llvm::ConstantInt::get(
+  llvm::Type::getInt32Ty(VMContext), 1))};
+
+getModule().addModuleFlag(llvm::Module::Require,
+  "StrictVTablePointersRequirement",
+  llvm::MDNode::get(VMContext, Ops));
+  }
   if (DebugInfo)
 // We support a single version in the linked module. The LLVM
 // parser will drop debug info with a different version number


Index: lib/CodeGen/CodeGenModule.cpp
===
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -394,6 +394,22 @@
 // Indicate that we want CodeView in the metadata.
 getModule().addModuleFlag(llvm::Module::Warning, "CodeView", 1);
   }
+  if (CodeGenOpts.OptimizationLevel > 0 && CodeGenOpts.StrictVTablePointers) {
+// We don't support LTO with 2 with different StrictVTablePointers
+// FIXME: we could support it by stripping all the information introduced
+// by StrictVTablePointers.
+
+getModule().addModuleFlag(llvm::Module::Error, "StrictVTablePointers",1);
+
+llvm::Metadata *Ops[2] = {
+  llvm::MDString::get(VMContext, "StrictVTablePointers"),
+  llvm::ConstantAsMetadata::get(llvm::ConstantInt::get(
+  llvm::Type::getInt32Ty(VMContext), 1))};
+
+getModule().addModuleFlag(llvm::Module::Require,
+  "StrictVTablePointersRequirement",
+  llvm::MDNode::get(VMContext, Ops));
+  }
   if (DebugInfo)
 // We support a single version in the linked module. The LLVM
 // parser will drop debug info with a different version number
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] __attribute__((enable_if)) and non-overloaded member functions

[Ettore Speziale via cfe-commits]

Gently ping.

> On Aug 26, 2015, at 2:40 PM, Ettore Speziale  
> wrote:
> 
> Forward to the right ML:
> 
>>> Sorry about the extreme delay. This patch slipped through the cracks, and I 
>>> only noticed it again when searching my email for enable_if. Committed in 
>>> r245985! In the future, please feel free to continue pinging weekly!
>> 
>> NP, thank you for committing the patch.
>> 
>> Unfortunately it contains a little error in the case of no candidate has 
>> been found. For instance consider the following test case:
>> 
>> struct Incomplete;
>> 
>> struct X {
>> void hidden_by_argument_conversion(Incomplete n, int m = 0) 
>> __attribute((enable_if(m == 10, "chosen when 'm' is ten")));
>> };
>> 
>> x.hidden_by_argument_conversion(10);
>> 
>> I would expect to get an error about Incomplete, as the compiler cannot 
>> understand how to convert 10 into an instance of Incomplete. However right 
>> now the enable_if diagnostic is emitted, thus masking the more useful 
>> message about Incomplete.
>> 
>> The attached patch solved the problem by delaying the point where the 
>> enable_if diagnostic is issued.
>> 
>> Thanks,
>> Ettore Speziale
> 
> 
> 

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

Re: [PATCH] D12580: Added StrictVTablePointers linking requirement

[Richard Smith via cfe-commits]

rsmith added a comment.

Functionally LGTM, but please add a test case. Is there any precedent for how 
to spell these module flags (StrictVTablePointers / strict_vtable_pointers / 
strict.vtable.pointers)?


http://reviews.llvm.org/D12580



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

Re: [PATCH] D12462: [PATCH] [clang-tidy] Add inconsistent declaration parameter name check

[Piotr Dziwinski via cfe-commits]

piotrdz updated this revision to Diff 33879.
piotrdz added a comment.

In this third version I did the following:

- fixed problems which I noticed with template specializations,
- changed output diagnostics to be more detailed, covering multiple declarations
- added FixIt hints to refactor inconsistent declarations to parameter names we 
see in definition
- added new template-related testcases and extended existing ones with checking 
of note diagnostics and FixIt hints


http://reviews.llvm.org/D12462

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
  clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-inconsistent-declaration-parameter-name.rst
  test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp

Index: test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
===
--- test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
+++ test/clang-tidy/readability-inconsistent-declaration-parameter-name.cpp
@@ -0,0 +1,153 @@
+// RUN: %python %S/check_clang_tidy.py %s readability-inconsistent-declaration-parameter-name %t
+
+void consistentFunction(int a, int b, int c);
+void consistentFunction(int a, int b, int c);
+void consistentFunction(int a, int b, int /*c*/);
+void consistentFunction(int /*c*/, int /*c*/, int /*c*/);
+
+//
+
+// CHECK-MESSAGES: :[[@LINE+1]]:6: warning: function 'inconsistentFunction' has 2 other declarations with differently named parameters [readability-inconsistent-declaration-parameter-name]
+void inconsistentFunction(int a, int b, int c);
+// CHECK-MESSAGES: :[[@LINE+4]]:6: note: 1st inconsistent declaration seen here
+// CHECK-MESSAGES: :[[@LINE+3]]:31: note: parameter 1 is named 'd' here, but 'a' in compared declaration
+// CHECK-MESSAGES: :[[@LINE+2]]:38: note: parameter 2 is named 'e' here, but 'b' in compared declaration
+// CHECK-MESSAGES: :[[@LINE+1]]:45: note: parameter 3 is named 'f' here, but 'c' in compared declaration
+void inconsistentFunction(int d, int e, int f);
+// CHECK-MESSAGES: :[[@LINE+4]]:6: note: 2nd inconsistent declaration seen here
+// CHECK-MESSAGES: :[[@LINE+3]]:31: note: parameter 1 is named 'x' here, but 'a' in compared declaration
+// CHECK-MESSAGES: :[[@LINE+2]]:38: note: parameter 2 is named 'y' here, but 'b' in compared declaration
+// CHECK-MESSAGES: :[[@LINE+1]]:45: note: parameter 3 is named 'z' here, but 'c' in compared declaration
+void inconsistentFunction(int x, int y, int z);
+
+//
+
+// CHECK-MESSAGES: :[[@LINE+5]]:6: warning: function 'inconsistentFunctionWithVisibleDefinition' has 1 other declaration with differently named parameters [readability-inconsistent-declaration-parameter-name]
+// CHECK-MESSAGES: :[[@LINE+3]]:6: note: 1st inconsistent declaration seen here
+// CHECK-MESSAGES: :[[@LINE+2]]:52: note: parameter 1 is named 'a' here, but 'b' in compared declaration
+// CHECK-FIXES: void inconsistentFunctionWithVisibleDefinition(int b);
+void inconsistentFunctionWithVisibleDefinition(int a);
+void inconsistentFunctionWithVisibleDefinition(int b) {}
+
+//
+
+struct Struct {
+// CHECK-MESSAGES: :[[@LINE+7]]:14: warning:  function 'Struct::inconsistentFunction' has 1 other declaration with differently named parameters [readability-inconsistent-declaration-parameter-name]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: 1st inconsistent declaration seen here
+// CHECK-MESSAGES: :[[@LINE+2]]:33: note: parameter 1 is named 'a' here, but 'b' in compared declaration
+// CHECK-FIXES: void inconsistentFunction(int b);
+  void inconsistentFunction(int a);
+};
+
+void Struct::inconsistentFunction(int b) {}
+
+//
+
+struct SpecialFunctions {
+// CHECK-MESSAGES: :[[@LINE+13]]:19: warning:  function 'SpecialFunctions::SpecialFunctions' has 1 other declaration with differently named parameters [readability-inconsistent-declaration-parameter-name]
+// CHECK-MESSAGES: :[[@LINE+3]]:3: note: 1st inconsistent declaration seen here
+// CHECK-MESSAGES: :[[@LINE+2]]:24: note: parameter 1 is named 'a' here, but 'b' in compared declaration
+// CHECK-FIXES: SpecialFunctions(int b);
+  SpecialFunctions(int a);
+
+// CHECK-MESSAGES: :[[@LINE+9]]:37: warning:  function 'SpecialFunctions::operator=' has 1 other declaration with differently named parameters [readability-inconsistent-declaration-parameter-name]
+// CHECK-MESSAGES: :[[@LINE+3]]:21: note: 1st inconsistent declaration seen here
+// CHECK-MESSAGES: :[[@LINE+2]]:55: note: parameter 1 is named 'a' here, but 'b' in compared declaration
+// CHECK-FIXES: SpecialFunctions& operator=(const SpecialFunc

Re: [PATCH] D12002: Initial WebAssembly support in clang

[Eric Christopher via cfe-commits]

echristo accepted this revision.
echristo added a reviewer: echristo.
echristo added a comment.

A couple things inline that need changing, but feel free to commit after 
without a repost.

-eric



Comment at: lib/Basic/Targets.cpp:6943-6944
@@ +6942,4 @@
+
+  Diags.Report(diag::err_opt_not_valid_with_opt) << Feature
+ << "-target-feature";
+  return false;

The backend should handle any weirdness here with missing features especially 
as this will report an error based on -cc1 compilation and not the main command 
line.

I.e. it's not necessary, that said if you feel wedded to it there's no problem 
either.


Comment at: lib/Basic/Targets.cpp:7635-7639
@@ +7634,7 @@
+// Until specific variations are defined, don't permit any.
+if (!(Triple == llvm::Triple("wasm32-unknown-unknown")) ||
+(!Triple.getVendorName().empty() &&
+ Triple.getVendorName() != "unknown") ||
+(!Triple.getOSName().empty() && Triple.getOSName() != "unknown") ||
+(Triple.hasEnvironment() && Triple.getEnvironmentName() != "unknown"))
+  return nullptr;

Why not just a positive test?


Comment at: lib/Basic/Targets.cpp:7643-7649
@@ +7642,9 @@
+  case llvm::Triple::wasm64:
+// Until specific variations are defined, don't permit any.
+if (!(Triple == llvm::Triple("wasm64-unknown-unknown")) ||
+(!Triple.getVendorName().empty() &&
+ Triple.getVendorName() != "unknown") ||
+(!Triple.getOSName().empty() && Triple.getOSName() != "unknown") ||
+(Triple.hasEnvironment() && Triple.getEnvironmentName() != "unknown"))
+  return nullptr;
+return new WebAssemblyOSTargetInfo(Triple);

Ditto.

(I said this just below, but it seems to have gotten munged in the newer 
version)


Repository:
  rL LLVM

http://reviews.llvm.org/D12002



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

RE: r246646 - Silence a -Wsign-compare warning; NFC.

[Daniel Marjamäki via cfe-commits]

Hello!

In general I am scared of such fixes.

I assume we know that Info::MaxTables is never a negative value now. However if 
it would (by mistake) get a negative value in the future then you are hiding an 
important TP warning here.

I assume we know that Info::MaxTables is never larger than int since you casted 
to unsigned. However if it would (by mistake) get a large value and there is 
loss of precision we wont know it because the TP warnings are hidden.

In my opinion, hiding -Wsign-compare FP warnings makes the code weaker.

Best regards,
Daniel Marjamäki

..
Daniel Marjamäki Senior Engineer
Evidente ES East AB  Warfvinges väg 34  SE-112 51 Stockholm  Sweden

Mobile: +46 (0)709 12 42 62
E-mail: 
Daniel.Marjamaki@evidente.se

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

Re: [PATCH] D12579: AST: simplify handling of the mangling

[David Majnemer via cfe-commits]

majnemer added a subscriber: majnemer.


Comment at: lib/AST/Mangle.cpp:130
@@ +129,3 @@
+  // ObjectiveC/C++ cannot support alternate calling conventions due to method
+  // dispatch.  Honour the mangling irrespective of the annotations.
+  if (const ObjCMethodDecl *OMD = dyn_cast(D))

Please use the American spelling, "honor."


Comment at: lib/AST/Mangle.cpp:131
@@ +130,3 @@
+  // dispatch.  Honour the mangling irrespective of the annotations.
+  if (const ObjCMethodDecl *OMD = dyn_cast(D))
+return mangleObjCMethodName(OMD, Out);

The type is obvious here, please use `auto *`.


Comment at: lib/AST/Mangle.cpp:139
@@ -149,2 +138,3 @@
 
-  Out << '\01';
+  // shouldMangleCXXName lies sometimes for the MS-ABI, so check the CC 
mangling
+  // when targeting MS-ABI (it doesnt consider CC which take precedence over

What do you mean by lie?  Are we patching over a bug in the MS mangler?


Comment at: lib/AST/Mangle.cpp:140
@@ +139,3 @@
+  // shouldMangleCXXName lies sometimes for the MS-ABI, so check the CC 
mangling
+  // when targeting MS-ABI (it doesnt consider CC which take precedence over
+  // language).

doesn't


http://reviews.llvm.org/D12579



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

Re: [PATCH] D12579: AST: simplify handling of the mangling

[Saleem Abdulrasool via cfe-commits]

compnerd added inline comments.


Comment at: lib/AST/Mangle.cpp:139
@@ -149,2 +138,3 @@
 
-  Out << '\01';
+  // shouldMangleCXXName lies sometimes for the MS-ABI, so check the CC 
mangling
+  // when targeting MS-ABI (it doesnt consider CC which take precedence over

majnemer wrote:
> What do you mean by lie?  Are we patching over a bug in the MS mangler?
The shouldMangleCXXName seems to be insufficient of a check for the MS ABI 
case, and will return `false` for cases where it shouldn't.  The comment is to 
clear up the reason for the

CC = CM_Other && TI.getCXXABI() == TargetCXXABI::Microsoft

check which we need to add.


http://reviews.llvm.org/D12579



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

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

[Ted Kremenek via cfe-commits]

krememek added a comment.

In http://reviews.llvm.org/D12358#238303, @seaneveson wrote:

> In http://reviews.llvm.org/D12358#237099, @krememek wrote:
>
> > To get the conservative invalidation that Anna suggests (actually, a little 
> > less conservative), I think you can just pass in a two MemRegions as the 
> > input to that method and get a new ProgramState with the appropriate 
> > regions invalidated.
>
>
> Thank you for the suggestion. Unfortunately nothing seems to get invalidated 
> when I call invalidateRegions, in the following code:
>
>   const StackFrameContext *STC = LCtx->getCurrentStackFrame();
>   MemRegionManager &MRMgr = svalBuilder.getRegionManager();
>   const MemRegion *Regions[] = {
> MRMgr.getStackLocalsRegion(STC),
> MRMgr.getStackArgumentsRegion(STC),
> MRMgr.getGlobalsRegion()
>   };
>   ProgramStateRef State;
>   State = PrevState->invalidateRegions(Regions, Cond, BlockCount, LCtx, 
> false);
>
>
> Do you have any suggestions on what I have done wrong?


I suspect this has to do with `invalidateRegions` itself.  I will take a look.  
In the meantime, can you provide an updated patch that I can try out so I can 
step through the algorithm (if necessary) in the debugger and reproduce what 
you are seeing?


http://reviews.llvm.org/D12358



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