[PATCH] D57579: [analyzer][WIP] Enable subcheckers to possess checker options

2019-02-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.
Herald added a subscriber: cfe-commits.

Ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D57579



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


[PATCH] D57896: Variable names rule

2019-02-07 Thread Chris Lattner via Phabricator via cfe-commits
lattner accepted this revision.
lattner added a comment.
This revision is now accepted and ready to land.

I am very much +1 on this.  That said, this isn't the sort of thing we just use 
patch review for.  Please agitate a robust discussion about this on llvm-dev.  
:-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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


[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-07 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 added inline comments.



Comment at: lib/Analysis/CFG.cpp:3137
+
+  Block = createBlock(false);
+  Block->setTerminator(G);

efriedma wrote:
> efriedma wrote:
> > Passing add_successor=false seems suspect; this could probably use more 
> > test coverage.
> Did you ever look at this?
Sorry, I missed this. 

Yes. add_successor has to be true, since asm goto is kind of contional goto.   
I changed.

Thanks.





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

https://reviews.llvm.org/D56571



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


[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-07 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 185911.
jyu2 marked an inline comment as done.
jyu2 added a comment.

Remove check for %lN
Fix missing successor of asm goto in CFG build.


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

https://reviews.llvm.org/D56571

Files:
  include/clang/AST/Stmt.h
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/AST/ASTImporter.cpp
  lib/AST/Stmt.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/Analysis/CFG.cpp
  lib/CodeGen/CGStmt.cpp
  lib/Parse/ParseStmtAsm.cpp
  lib/Sema/JumpDiagnostics.cpp
  lib/Sema/SemaStmtAsm.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/Analysis/asm-goto.cpp
  test/CodeGen/asm-goto.c
  test/CodeGen/asm.c
  test/CodeGen/inline-asm-mixed-style.c
  test/Coverage/c-language-features.inc
  test/PCH/asm.h
  test/Parser/asm.c
  test/Parser/asm.cpp
  test/Sema/asm-goto.cpp
  test/Sema/asm.c
  test/Sema/inline-asm-validate-tmpl.cpp
  test/Sema/scope-check.c

Index: test/Sema/scope-check.c
===
--- test/Sema/scope-check.c
+++ test/Sema/scope-check.c
@@ -232,3 +232,19 @@
 
 // rdar://9024687
 int test16(int [sizeof &]); // expected-error {{use of address-of-label extension outside of a function body}}
+
+//Asm goto:
+int test16(int n)
+{
+  // expected-error@+2 {{cannot jump from this asm goto statement to one of its possible targets}}
+  // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
+  asm volatile goto("testl %0, %0; jne %l1;" :: "r"(n)::label_true, loop);
+  // expected-note@+2 {{jump bypasses initialization of variable length array}}
+  // expected-note@+1 {{possible target of asm goto statement}}
+  return ({int a[n];label_true: 2;});
+  // expected-note@+1 {{jump bypasses initialization of variable length array}}
+  int b[n];
+// expected-note@+1 {{possible target of asm goto statement}}
+loop:
+  return 0;
+}
Index: test/Sema/inline-asm-validate-tmpl.cpp
===
--- test/Sema/inline-asm-validate-tmpl.cpp
+++ test/Sema/inline-asm-validate-tmpl.cpp
@@ -23,3 +23,13 @@
 	asm("rol %1, %0" :"=r"(value): "I"(N + 1));
 }
 int	foo() { testc<2>(10); }
+
+// these should compile without error
+template  bool testd()
+{
+  __asm goto ("" : : : : lab);
+  return true;
+lab:
+  return false;
+}
+bool foox() { return testd<0> (); }
Index: test/Sema/asm.c
===
--- test/Sema/asm.c
+++ test/Sema/asm.c
@@ -295,3 +295,24 @@
   return r0 + r1;
 }
 
+void test18()
+{
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm goto ("" : : : : lab, lab, lab2, lab);
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm goto ("xorw %[lab], %[lab]; je %l[lab]" : : [lab] "i" (0) : : lab);
+lab:;
+lab2:;
+  int x,x1;
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm ("" : [lab] "=r" (x),[lab] "+r" (x) : [lab1] "r" (x));
+  // expected-error@+2 {{duplicate use of asm operand name "lab"}}
+  // expected-note@+1 {{asm operand name "lab" first referenced here}}
+  asm ("" : [lab] "=r" (x1) : [lab] "r" (x));
+  // expected-error@+1 {{invalid operand number in inline asm string}}
+  asm ("jne %l0":::);
+  asm goto ("jne %l0"lab);
+}
Index: test/Sema/asm-goto.cpp
===
--- /dev/null
+++ test/Sema/asm-goto.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 %s -triple i386-pc-linux-gnu -verify -fsyntax-only
+
+struct NonTrivial {
+  ~NonTrivial();
+  int f(int);
+private:
+  int k;
+};
+void JumpDiagnostics(int n) {
+// expected-error@+1 {{cannot jump from this goto statement to its label}}
+  goto DirectJump;
+// expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
+  NonTrivial tnp1;
+
+DirectJump:
+// expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
+  asm goto("jmp %l0;" Later);
+// expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
+  NonTrivial tnp2;
+// expected-note@+1 {{possible target of asm goto statement}}
+Later:
+  return;
+}
+
+struct S { ~S(); };
+void foo(int a) {
+  if (a) {
+FOO:
+// expected-note@+2 {{jump exits scope of variable with non-trivial destructor}}
+// expected-note@+1 {{jump exits scope of variable with non-trivial destructor}}
+S s;
+void *p = &
+// expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
+  asm goto("jmp %l0;" BAR);
+// expected-error@+1 {{cannot jump from this indirect goto 

[PATCH] D57464: Generalize method overloading on addr spaces to C++

2019-02-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added a reviewer: rsmith.
rjmccall added a comment.

Hmm.  Richard, I've mostly let you stay out of this, but do you have any 
thoughts about the right manage to parse attributes here?  We want to allow 
`address_space` attributes to be written in the method-qualifiers list, but 
when we do that, it's hard to avoid parsing arbitrary attributes and then 
potentially needing to move them to the declarator; also, doing that might mean 
propagating awkward information in to deal with of delayed attribute parsing.


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

https://reviews.llvm.org/D57464



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


[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

2019-02-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:986
+  }
+  }];
+}

Probably worth clarifying somewhere in here that only a specific set of stdlib 
functions are supported.  I don't know that it's important to spell that set 
out.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1490
 
+  if (auto *Attr = FD->getAttr()) {
+SmallVector ArgVals;

Could you move the body of this into a helper function?  This function is 
really far too large as it is.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:1516
+  // void *memset(void *b, int c, size_t len);
+  // void *__memset_chk(void *b, int c, size_t len, size_t);
+  ArgVals.push_back(emitObjSize());

I think this might be over-commented. :)

Could you just put smaller comments on groups of cases, like:
```
  // All of these take the destination object size as the final argument.
```
or
```
  // These take flags and the destination object size immediately before the 
format string.
```



Comment at: clang/test/Sema/fortify-std-lib.c:6
+__attribute__((fortify_stdlib(0, 0)))
+int not_anyting_special(); // expected-error {{'fortify_stdlib' attribute 
applied to an unknown function}}
+

Silly comment, but: you typo'd "anything" here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57918



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


[PATCH] D57936: [CodeGenObjC] When available, emit a direct call to objc_alloc_init(cls) instead of [objc_alloc(cls) init]

2019-02-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGObjC.cpp:459
+  llvm::Value *Receiver =
+  CGF.CGM.getObjCRuntime().GetClass(CGF, ObjTy->getInterface());
+  return CGF.EmitObjCAllocInit(Receiver, CGF.ConvertType(OME->getType()));

You might need to check for a `super` message send, which can be a class 
message send in a class method.

Also, I think `getInterface()` can return null here, although it might need to 
be contrived to fit the other requirements.  `Class`, where that 
protocol declares `+alloc`?  If there isn't a way to just emit the receiver 
pointer of a normal message send from an `ObjCMessageExpr`, that's really too 
bad.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57936



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


[PATCH] D57797: Variable auto-init: fix __block initialization

2019-02-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

SGTM, thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D57797



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


[PATCH] D57936: [CodeGenObjC] When available, emit a direct call to objc_alloc_init(cls) instead of [objc_alloc(cls) init]

2019-02-07 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, pete, ahatanak.
Herald added subscribers: dexonsmith, jkorous.
Herald added a project: clang.

This provides a code size win on the caller side, since the `init` message send 
is done in the runtime function.

rdar://44987038

Thanks!
Erik


Repository:
  rC Clang

https://reviews.llvm.org/D57936

Files:
  clang/include/clang/Basic/ObjCRuntime.h
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenObjC/objc-alloc-init.m

Index: clang/test/CodeGenObjC/objc-alloc-init.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/objc-alloc-init.m
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 %s -fobjc-runtime=macosx-10.14.4 -emit-llvm -O0 -o - | FileCheck %s --check-prefix=OPTIMIZED
+// RUN: %clang_cc1 %s -fobjc-runtime=macosx-10.14.3 -emit-llvm -O0 -o - | FileCheck %s --check-prefix=NOT_OPTIMIZED
+// RUN: %clang_cc1 %s -fobjc-runtime=ios-12.2 -emit-llvm -O0 -o - | FileCheck %s --check-prefix=OPTIMIZED
+// RUN: %clang_cc1 %s -fobjc-runtime=ios-12.1 -emit-llvm -O0 -o - | FileCheck %s --check-prefix=NOT_OPTIMIZED
+
+@interface X
++(X *)alloc;
+-(X *)init;
+@end
+
+void f() {
+  [[X alloc] init];
+  // OPTIMIZED: call i8* @objc_alloc_init(
+  // NOT_OPTIMIZED: call i8* @objc_alloc(
+}
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -124,6 +124,9 @@
   /// void objc_allocWithZone(id);
   llvm::FunctionCallee objc_allocWithZone;
 
+  /// void objc_alloc_init(id);
+  llvm::FunctionCallee objc_alloc_init;
+
   /// void objc_autoreleasePoolPop(void*);
   llvm::FunctionCallee objc_autoreleasePoolPop;
 
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -3827,6 +3827,8 @@
  llvm::Type *returnType);
   llvm::Value *EmitObjCAllocWithZone(llvm::Value *value,
  llvm::Type *returnType);
+  llvm::Value *EmitObjCAllocInit(llvm::Value *value, llvm::Type *resultType);
+
   llvm::Value *EmitObjCThrowOperand(const Expr *expr);
   llvm::Value *EmitObjCConsumeObject(QualType T, llvm::Value *Ptr);
   llvm::Value *EmitObjCExtendObjectLifetime(QualType T, llvm::Value *Ptr);
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -425,6 +425,41 @@
   return None;
 }
 
+/// Instead of '[[MyClass alloc] init]', try to generate
+/// 'objc_alloc_init(MyClass)'. This provides a code size improvement on the
+/// caller side, as well as the optimized objc_alloc.
+static Optional
+tryEmitSpecializedAllocInit(CodeGenFunction , const ObjCMessageExpr *OME) {
+  auto  = CGF.getLangOpts().ObjCRuntime;
+  if (!Runtime.shouldUseRuntimeFunctionForCombinedAllocInit())
+return None;
+
+  // Match the exact pattern '[[MyClass alloc] init]'.
+  Selector Sel = OME->getSelector();
+  if (!OME->isInstanceMessage() || !OME->getType()->isObjCObjectPointerType() ||
+  !Sel.isUnarySelector() || Sel.getNameForSlot(0) != "init")
+return None;
+
+  // Okay, this is '[receiver init]', check if 'receiver' is '[cls alloc]'.
+  auto *SubOME =
+  dyn_cast(OME->getInstanceReceiver()->IgnoreParens());
+  if (!SubOME)
+return None;
+  Selector SubSel = SubOME->getSelector();
+  if (!SubOME->isClassMessage() ||
+  !SubOME->getType()->isObjCObjectPointerType() ||
+  !SubSel.isUnarySelector() || SubSel.getNameForSlot(0) != "alloc")
+return None;
+
+  QualType ReceiverType = SubOME->getClassReceiver();
+  const ObjCObjectType *ObjTy = ReceiverType->getAs();
+  if (!ObjTy)
+return None;
+  llvm::Value *Receiver =
+  CGF.CGM.getObjCRuntime().GetClass(CGF, ObjTy->getInterface());
+  return CGF.EmitObjCAllocInit(Receiver, CGF.ConvertType(OME->getType()));
+}
+
 RValue CodeGenFunction::EmitObjCMessageExpr(const ObjCMessageExpr *E,
 ReturnValueSlot Return) {
   // Only the lookup mechanism and first two arguments of the method
@@ -446,6 +481,9 @@
 }
   }
 
+  if (Optional Val = tryEmitSpecializedAllocInit(*this, E))
+return AdjustObjCObjectType(*this, E->getType(), RValue::get(*Val));
+
   // We don't retain the receiver in delegate init calls, and this is
   // safe because the receiver value is always loaded from 'self',
   // which we zero out.  We don't want to Block_copy block receivers,
@@ -2506,6 +2544,13 @@
 "objc_allocWithZone", /*MayThrow=*/true);
 }
 
+llvm::Value *CodeGenFunction::EmitObjCAllocInit(llvm::Value *value,
+llvm::Type *resultType) {
+  return 

[PATCH] D57935: [Sema] Make string literal init an rvalue.

2019-02-07 Thread Eli Friedman via Phabricator via cfe-commits
efriedma created this revision.
efriedma added a reviewer: rsmith.
Herald added a project: clang.

This allows substantially simplifying the expression evaluation code, because 
we don't have to special-case lvalues which are actually string literal 
initialization.

This currently throws away an optimization where we would avoid creating an 
array APValue for string literal initialization.  If we really want to optimize 
this case, we should fix APValue so it can store simple arrays more 
efficiently, like llvm::ConstantDataArray.  This shouldn't affect the memory 
usage for other string literals.  (Not sure if this is a blocker; I don't think 
string literal init is common enough for this to be a serious issue, but I 
could be wrong.)

The change to test/CodeGenObjC/encode-test.m is a weird side-effect of these 
changes: we currently don't constant-evaluate arrays in C, so the strlen call 
shouldn't be folded, but lvalue string init managed to get around that check.  
I this this is fine.

Fixes https://bugs.llvm.org/show_bug.cgi?id=40430 .


Repository:
  rC Clang

https://reviews.llvm.org/D57935

Files:
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/Sema/SemaInit.cpp
  test/AST/ast-dump-wchar.cpp
  test/CodeGenObjC/encode-test.m
  test/SemaCXX/constant-expression-cxx11.cpp

Index: test/SemaCXX/constant-expression-cxx11.cpp
===
--- test/SemaCXX/constant-expression-cxx11.cpp
+++ test/SemaCXX/constant-expression-cxx11.cpp
@@ -2220,3 +2220,11 @@
   constexpr int *q = ( + 1) - (unsigned __int128)-1; // expected-error {{constant expression}} expected-note {{cannot refer to element -3402}}
   constexpr int *r = &( + 1)[(unsigned __int128)-1]; // expected-error {{constant expression}} expected-note {{cannot refer to element 3402}}
 }
+
+namespace PR40430 {
+  struct S {
+char c[10] = "asdf";
+constexpr char foo() const { return c[3]; }
+  };
+  static_assert(S().foo() == 'f', "");
+}
Index: test/CodeGenObjC/encode-test.m
===
--- test/CodeGenObjC/encode-test.m
+++ test/CodeGenObjC/encode-test.m
@@ -186,7 +186,8 @@
 
 // CHECK-LABEL: @test_strlen(
 // CHECK: %[[i:.*]] = alloca i32
-// CHECK: store i32 1, i32* %[[i]]
+// CHECK: %[[call:.*]] = call i32 @strlen
+// CHECK: store i32 %[[call]], i32* %[[i]]
 void test_strlen() {
   const char array[] = @encode(int);
   int i = strlen(array);
Index: test/AST/ast-dump-wchar.cpp
===
--- test/AST/ast-dump-wchar.cpp
+++ test/AST/ast-dump-wchar.cpp
@@ -1,13 +1,13 @@
 // RUN: %clang_cc1 -std=c++11 -ast-dump %s -triple x86_64-linux-gnu | FileCheck %s 
 
 char c8[] = u8"test\0\\\"\a\b\f\n\r\t\v\234";
-// CHECK: StringLiteral {{.*}} lvalue u8"test\000\\\"\a\b\f\n\r\t\v\234"
+// CHECK: StringLiteral {{.*}} u8"test\000\\\"\a\b\f\n\r\t\v\234"
 
 char16_t c16[] = u"test\0\\\"\t\a\b\234\u1234";
-// CHECK: StringLiteral {{.*}} lvalue u"test\000\\\"\t\a\b\234\u1234"
+// CHECK: StringLiteral {{.*}} u"test\000\\\"\t\a\b\234\u1234"
 
 char32_t c32[] = U"test\0\\\"\t\a\b\234\u1234\U0010"; // \
-// CHECK: StringLiteral {{.*}} lvalue U"test\000\\\"\t\a\b\234\u1234\U0010"
+// CHECK: StringLiteral {{.*}} U"test\000\\\"\t\a\b\234\u1234\U0010"
 
 wchar_t wc[] = L"test\0\\\"\t\a\b\234\u1234\x"; // \
-// CHECK: StringLiteral {{.*}} lvalue L"test\000\\\"\t\a\b\234\x1234\x"
+// CHECK: StringLiteral {{.*}} L"test\000\\\"\t\a\b\234\x1234\x"
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -144,6 +144,7 @@
 static void updateStringLiteralType(Expr *E, QualType Ty) {
   while (true) {
 E->setType(Ty);
+E->setValueKind(VK_RValue);
 if (isa(E) || isa(E))
   break;
 else if (ParenExpr *PE = dyn_cast(E))
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1649,16 +1649,6 @@
 llvm::Constant *ConstantLValueEmitter::tryEmit() {
   const APValue::LValueBase  = Value.getLValueBase();
 
-  // Certain special array initializers are represented in APValue
-  // as l-values referring to the base expression which generates the
-  // array.  This happens with e.g. string literals.  These should
-  // probably just get their own representation kind in APValue.
-  if (DestType->isArrayType()) {
-assert(!hasNonZeroOffset() && "offset on array initializer");
-auto expr = const_cast(base.get());
-return ConstExprEmitter(Emitter).Visit(expr, DestType);
-  }
-
   // Otherwise, the destination type should be a pointer or reference
   // type, but it might also be a cast thereof.
   //
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ 

Buildbot numbers for the week of 01/27/2019 - 02/02/2019

2019-02-07 Thread Galina Kistanova via cfe-commits
Hello everyone,

Below are some buildbot numbers for the last week of 01/27/2019 -
02/02/2019.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
 buildername | was_red
-+-
 llvm-sphinx-docs| 95:36:05
 lld-sphinx-docs | 93:03:08
 clang-sphinx-docs   | 72:30:16
 clang-x64-windows-msvc  | 37:28:11
 clang-cmake-armv7-selfhost-neon | 29:24:58
 clang-atom-d525-fedora-rel  | 21:35:27
 clang-ppc64le-linux-multistage  | 21:13:59
 clang-cmake-armv7-selfhost  | 20:34:20
 clang-x86_64-linux-selfhost-modules | 20:18:27
 clang-lld-x86_64-2stage | 19:54:58
 clang-cmake-thumbv7-full-sh | 18:37:23
 clang-s390x-linux-multistage| 17:31:06
 clang-cmake-thumbv8-full-sh | 15:28:11
 clang-cmake-armv8-lld   | 15:20:16
 clang-s390x-linux   | 14:50:35
 clang-ppc64be-linux-lnt | 14:50:20
 clang-ppc64le-linux | 14:49:34
 clang-cmake-armv8-quick | 14:48:37
 clang-s390x-linux-lnt   | 14:47:04
 clang-ppc64be-linux | 14:39:06
 clang-cmake-armv7-full  | 14:32:57
 clang-ppc64be-linux-multistage  | 14:30:38
 clang-cmake-armv8-full  | 14:30:15
 clang-cmake-armv7-quick | 14:23:35
 clang-cmake-armv8-global-isel   | 14:13:36
 clang-cmake-armv7-global-isel   | 14:03:46
 clang-cmake-aarch64-global-isel | 13:54:58
 clang-cmake-aarch64-quick   | 13:54:15
 clang-ppc64le-linux-lnt | 13:51:15
 llvm-clang-x86_64-expensive-checks-win  | 13:43:15
 lld-x86_64-win7 | 11:45:30
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast| 11:40:10
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast  | 11:36:16
 clang-cmake-aarch64-full| 11:33:37
 clang-cmake-x86_64-avx2-linux   | 11:20:05
 clang-cmake-x86_64-sde-avx512-linux | 11:15:49
 clang-cmake-aarch64-lld | 10:06:27
 clang-cmake-armv8-selfhost-neon | 08:38:45
 clang-with-lto-ubuntu   | 06:27:01
 sanitizer-x86_64-linux-fuzzer   | 05:45:40
 lldb-x64-windows-ninja  | 05:08:08
 llvm-hexagon-elf| 04:32:31
 clang-with-thin-lto-ubuntu  | 03:51:07
 sanitizer-ppc64be-linux | 03:36:03
 clang-cuda-build| 03:16:35
 sanitizer-x86_64-linux-bootstrap-msan   | 03:06:25
 clang-x86_64-debian-fast| 02:59:14
 sanitizer-x86_64-linux-bootstrap| 02:52:34
 sanitizer-x86_64-linux-fast | 02:52:28
 clang-cmake-armv7-lnt   | 02:48:57
 clang-cmake-armv8-lnt   | 02:32:07
 sanitizer-x86_64-linux  | 02:31:49
 sanitizer-x86_64-linux-bootstrap-ubsan  | 02:29:56
 clang-cmake-x86_64-avx2-linux-perf  | 02:13:54
 clang-x86_64-linux-abi-test | 02:12:35
 lld-x86_64-darwin13 | 01:59:44
 libcxx-libcxxabi-libunwind-armv8-linux  | 01:53:06
 libcxx-libcxxabi-libunwind-armv8-linux-noexceptions | 01:50:58
 lldb-x86_64-ubuntu-14.04-buildserver| 01:48:46
 sanitizer-x86_64-linux-autoconf | 01:44:54
 sanitizer-x86_64-linux-android  | 01:44:52
 clang-armv7-linux-build-cache   | 01:43:04
 clang-aarch64-linux-build-cache | 01:32:11
 lld-perf-testsuite  | 01:20:13
 libcxx-libcxxabi-libunwind-armv7-linux  | 01:16:05
 polly-arm-linux | 01:09:20
 

Buildbot numbers for the week of 01/20/2019 - 01/26/2019

2019-02-07 Thread Galina Kistanova via cfe-commits
Hello everyone,

Below are some buildbot numbers for the week of 01/20/2019 - 01/26/2019.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
   buildername| was_red
--+-
 polly-amd64-linux| 54:43:39
 clang-cmake-armv7-selfhost-neon  | 51:27:08
 clang-cmake-armv7-selfhost   | 51:13:20
 polly-arm-linux  | 50:55:00
 libcxx-libcxxabi-libunwind-x86_64-linux-debian   | 50:32:01
 aosp-O3-polly-before-vectorizer-unprofitable | 50:21:22
 clang-cmake-armv8-selfhost-neon  | 49:55:48
 lldb-amd64-ninja-netbsd8 | 49:25:35
 lldb-x86_64-ubuntu-14.04-buildserver | 41:19:57
 lldb-x64-windows-ninja   | 40:43:22
 sanitizer-x86_64-linux-android   | 34:33:51
 clang-x64-windows-msvc   | 34:05:39
 clang-cmake-armv7-full   | 30:59:43
 clang-cmake-aarch64-lld  | 30:59:32
 lld-x86_64-win7  | 29:54:35
 clang-cmake-thumbv7-full-sh  | 27:52:57
 clang-ppc64le-linux-multistage   | 24:42:16
 llvm-clang-x86_64-expensive-checks-win   | 23:49:57
 clang-cmake-aarch64-full | 20:58:52
 sanitizer-x86_64-linux-fuzzer| 19:14:21
 clang-lld-x86_64-2stage  | 18:39:44
 sanitizer-x86_64-linux   | 14:59:35
 clang-x86_64-linux-selfhost-modules  | 12:21:39
 clang-with-lto-ubuntu| 12:18:56
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast | 11:44:33
 clang-x86_64-linux-abi-test  | 10:53:35
 clang-with-thin-lto-ubuntu   | 10:31:20
 clang-cmake-thumbv8-full-sh  | 07:54:10
 sanitizer-ppc64be-linux  | 05:10:12
 sanitizer-ppc64le-linux  | 04:08:49
 clang-cmake-armv8-lld| 04:07:43
 clang-ppc64le-linux  | 04:01:47
 clang-ppc64be-linux-multistage   | 03:49:19
 sanitizer-x86_64-linux-bootstrap | 03:48:00
 clang-s390x-linux| 03:35:46
 clang-ppc64be-linux-lnt  | 03:31:59
 clang-ppc64le-linux-lnt  | 03:25:42
 clang-ppc64be-linux  | 03:24:08
 clang-cmake-armv8-global-isel| 03:20:35
 clang-cmake-armv7-lnt| 03:14:56
 sanitizer-x86_64-linux-bootstrap-ubsan   | 03:12:37
 sanitizer-x86_64-linux-bootstrap-msan| 03:08:59
 clang-cmake-armv8-full   | 03:02:18
 lld-x86_64-freebsd   | 03:00:34
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast   | 03:00:13
 sanitizer-x86_64-linux-fast  | 02:56:18
 lld-x86_64-darwin13  | 02:46:39
 clang-cmake-armv8-quick  | 02:46:32
 clang-cmake-armv8-lnt| 02:44:01
 sanitizer-x86_64-linux-autoconf  | 02:41:36
 clang-s390x-linux-multistage | 02:10:03
 clang-s390x-linux-lnt| 02:06:27
 clang-cmake-armv7-quick  | 02:06:05
 clang-cmake-x86_64-avx2-linux-perf   | 02:00:43
 clang-cmake-aarch64-global-isel  | 01:20:18
 clang-cmake-aarch64-quick| 01:19:20
 clang-cuda-build | 01:18:18
 clang-atom-d525-fedora-rel   | 01:12:04
 clang-hexagon-elf| 01:10:08
 clang-cmake-x86_64-sde-avx512-linux  | 00:58:02
 clang-cmake-x86_64-avx2-linux| 00:53:46
 clang-armv7-linux-build-cache| 00:42:58
 clang-x86_64-debian-fast | 00:42:40
 sanitizer-windows| 00:41:04
 lld-perf-testsuite   | 00:37:18
 clang-aarch64-linux-build-cache  | 00:34:42
 llvm-hexagon-elf | 00:31:25
 clang-tools-sphinx-docs  | 00:24:23
 clang-sphinx-docs| 00:15:35
(69 rows)


"Status change ratio" by 

LLVM buildmaster will be updated and restarted tonight

2019-02-07 Thread Galina Kistanova via cfe-commits
 Hello everyone,

LLVM buildmaster will be updated and restarted after 7PM Pacific time today.

Thanks

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


[PATCH] D56554: [ELF] Add '-z nognustack' opt to suppress emitting PT_GNU_STACK

2019-02-07 Thread Rui Ueyama via Phabricator via cfe-commits
ruiu added a comment.

Overall looks good.

> Do you mean that of those three options, the last one specified should take 
> precedence?

Yes.




Comment at: ELF/Config.h:191
   bool ZGlobal;
+  GnuStackKind ZGnustack;
   bool ZHazardplt;

Members are (roughly) sorted alphabetically, so move this below the last 
boolean member.



Comment at: ELF/Driver.cpp:352
+  return GnuStackKind::Exec;
+else if (StringRef("noexecstack") == Arg->getValue())
+  return GnuStackKind::NoExec;

nit: no `else` after `return`.



Comment at: ELF/Driver.cpp:369
  S == "nodelete" || S == "nodlopen" || S == "noexecstack" ||
+ S == "nognustack" ||
  S == "nokeep-text-section-prefix" || S == "norelro" || S == "notext" 
||

If you have clang-format, please run it on this file.


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

https://reviews.llvm.org/D56554



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


[PATCH] D57874: [WebAssembly] Make thread-related options consistent

2019-02-07 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added a comment.

In D57874#1389981 , @sunfish wrote:

> > - `-matomics` means `-mthread-model posix`
>
> The others sound reasonable, though this one seems a little surprising -- a 
> user might have -matomics enabled because they're targeting a VM that has 
> atomics, but still not want to use -mthread-model posix because their code 
> doesn't actually using threads.


If you look at what "-mthread-model posix" actually means in the codebase it 
basically means "don't lower away atomics ops".

Its only used by WebAssembly and ARMTargetMachine to select the 
`createLowerAtomicPass`.   Given that these are the only uses it seems like it 
should be removed or renamed.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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


[PATCH] D57797: Variable auto-init: fix __block initialization

2019-02-07 Thread JF Bastien via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC353495: Variable auto-init: fix __block initialization 
(authored by jfb, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57797?vs=185896=185902#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57797

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGenCXX/trivial-auto-var-init.cpp


Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1633,11 +1633,15 @@
   ? LangOptions::TrivialAutoVarInitKind::Uninitialized
   : getContext().getLangOpts().getTrivialAutoVarInit()));
 
-  auto initializeWhatIsTechnicallyUninitialized = [&]() {
+  auto initializeWhatIsTechnicallyUninitialized = [&](Address Loc) {
 if (trivialAutoVarInit ==
 LangOptions::TrivialAutoVarInitKind::Uninitialized)
   return;
 
+// Only initialize a __block's storage: we always initialize the header.
+if (emission.IsEscapingByRef)
+  Loc = emitBlockByrefAddress(Loc, , /*follow=*/false);
+
 CharUnits Size = getContext().getTypeSizeInChars(type);
 if (!Size.isZero()) {
   switch (trivialAutoVarInit) {
@@ -1714,7 +1718,7 @@
   };
 
   if (isTrivialInitializer(Init)) {
-initializeWhatIsTechnicallyUninitialized();
+initializeWhatIsTechnicallyUninitialized(Loc);
 return;
   }
 
@@ -1728,7 +1732,7 @@
   }
 
   if (!constant) {
-initializeWhatIsTechnicallyUninitialized();
+initializeWhatIsTechnicallyUninitialized(Loc);
 LValue lv = MakeAddrLValue(Loc, type);
 lv.setNonGC(true);
 return EmitExprAsInit(Init, , lv, capturedByInit);
Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -30,6 +30,32 @@
   used(block);
 }
 
+// Using the variable being initialized is typically UB in C, but for blocks we
+// can be nice: they imply extra book-keeping and we can do the auto-init 
before
+// any of said book-keeping.
+//
+// UNINIT-LABEL:  test_block_self_init(
+// ZERO-LABEL:test_block_self_init(
+// ZERO:  %block = alloca <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor*, i8* }>, align 8
+// ZERO:  %captured1 = getelementptr inbounds 
%struct.__block_byref_captured, %struct.__block_byref_captured* %captured, i32 
0, i32 4
+// ZERO-NEXT: store %struct.XYZ* null, %struct.XYZ** %captured1, align 8
+// ZERO:  %call = call %struct.XYZ* @create(
+// PATTERN-LABEL: test_block_self_init(
+// PATTERN:   %block = alloca <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor*, i8* }>, align 8
+// PATTERN:   %captured1 = getelementptr inbounds 
%struct.__block_byref_captured, %struct.__block_byref_captured* %captured, i32 
0, i32 4
+// PATTERN-NEXT:  store %struct.XYZ* inttoptr (i64 -6148914691236517206 to 
%struct.XYZ*), %struct.XYZ** %captured1, align 8
+// PATTERN:   %call = call %struct.XYZ* @create(
+void test_block_self_init() {
+  using Block = void (^)();
+  typedef struct XYZ {
+Block block;
+  } * xyz_t;
+  extern xyz_t create(Block block);
+  __block xyz_t captured = create(^() {
+(void)captured;
+  });
+}
+
 // This type of code is currently not handled by zero / pattern initialization.
 // The test will break when that is fixed.
 // UNINIT-LABEL:  test_goto_unreachable_value(


Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1633,11 +1633,15 @@
   ? LangOptions::TrivialAutoVarInitKind::Uninitialized
   : getContext().getLangOpts().getTrivialAutoVarInit()));
 
-  auto initializeWhatIsTechnicallyUninitialized = [&]() {
+  auto initializeWhatIsTechnicallyUninitialized = [&](Address Loc) {
 if (trivialAutoVarInit ==
 LangOptions::TrivialAutoVarInitKind::Uninitialized)
   return;
 
+// Only initialize a __block's storage: we always initialize the header.
+if (emission.IsEscapingByRef)
+  Loc = emitBlockByrefAddress(Loc, , /*follow=*/false);
+
 CharUnits Size = getContext().getTypeSizeInChars(type);
 if (!Size.isZero()) {
   switch (trivialAutoVarInit) {
@@ -1714,7 +1718,7 @@
   };
 
   if (isTrivialInitializer(Init)) {
-initializeWhatIsTechnicallyUninitialized();
+initializeWhatIsTechnicallyUninitialized(Loc);
 return;
   }
 
@@ -1728,7 +1732,7 @@
   }
 
   if (!constant) {
-initializeWhatIsTechnicallyUninitialized();
+initializeWhatIsTechnicallyUninitialized(Loc);
 LValue lv = MakeAddrLValue(Loc, type);
 lv.setNonGC(true);
 return EmitExprAsInit(Init, , lv, capturedByInit);
Index: 

r353495 - Variable auto-init: fix __block initialization

2019-02-07 Thread JF Bastien via cfe-commits
Author: jfb
Date: Thu Feb  7 17:29:17 2019
New Revision: 353495

URL: http://llvm.org/viewvc/llvm-project?rev=353495=rev
Log:
Variable auto-init: fix __block initialization

Summary:
Automatic initialization [1] of __block variables was trampling over the block's
headers after they'd been initialized, which caused self-init usage to crash,
such as here:

  typedef struct XYZ { void (^block)(); } *xyz_t;
  __attribute__((noinline))
  xyz_t create(void (^block)()) {
xyz_t myself = malloc(sizeof(struct XYZ));
myself->block = block;
return myself;
  }
  int main() {
__block xyz_t captured = create(^(){ (void)captured; });
  }

This type of code shouldn't be broken by variable auto-init, even if it's
sketchy.

[1] With -ftrivial-auto-var-init=pattern



Reviewers: rjmccall, pcc, kcc

Subscribers: jkorous, dexonsmith, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/test/CodeGenCXX/trivial-auto-var-init.cpp

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=353495=353494=353495=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Thu Feb  7 17:29:17 2019
@@ -1633,11 +1633,15 @@ void CodeGenFunction::EmitAutoVarInit(co
   ? LangOptions::TrivialAutoVarInitKind::Uninitialized
   : getContext().getLangOpts().getTrivialAutoVarInit()));
 
-  auto initializeWhatIsTechnicallyUninitialized = [&]() {
+  auto initializeWhatIsTechnicallyUninitialized = [&](Address Loc) {
 if (trivialAutoVarInit ==
 LangOptions::TrivialAutoVarInitKind::Uninitialized)
   return;
 
+// Only initialize a __block's storage: we always initialize the header.
+if (emission.IsEscapingByRef)
+  Loc = emitBlockByrefAddress(Loc, , /*follow=*/false);
+
 CharUnits Size = getContext().getTypeSizeInChars(type);
 if (!Size.isZero()) {
   switch (trivialAutoVarInit) {
@@ -1714,7 +1718,7 @@ void CodeGenFunction::EmitAutoVarInit(co
   };
 
   if (isTrivialInitializer(Init)) {
-initializeWhatIsTechnicallyUninitialized();
+initializeWhatIsTechnicallyUninitialized(Loc);
 return;
   }
 
@@ -1728,7 +1732,7 @@ void CodeGenFunction::EmitAutoVarInit(co
   }
 
   if (!constant) {
-initializeWhatIsTechnicallyUninitialized();
+initializeWhatIsTechnicallyUninitialized(Loc);
 LValue lv = MakeAddrLValue(Loc, type);
 lv.setNonGC(true);
 return EmitExprAsInit(Init, , lv, capturedByInit);

Modified: cfe/trunk/test/CodeGenCXX/trivial-auto-var-init.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/trivial-auto-var-init.cpp?rev=353495=353494=353495=diff
==
--- cfe/trunk/test/CodeGenCXX/trivial-auto-var-init.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/trivial-auto-var-init.cpp Thu Feb  7 17:29:17 2019
@@ -30,6 +30,32 @@ void test_block() {
   used(block);
 }
 
+// Using the variable being initialized is typically UB in C, but for blocks we
+// can be nice: they imply extra book-keeping and we can do the auto-init 
before
+// any of said book-keeping.
+//
+// UNINIT-LABEL:  test_block_self_init(
+// ZERO-LABEL:test_block_self_init(
+// ZERO:  %block = alloca <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor*, i8* }>, align 8
+// ZERO:  %captured1 = getelementptr inbounds 
%struct.__block_byref_captured, %struct.__block_byref_captured* %captured, i32 
0, i32 4
+// ZERO-NEXT: store %struct.XYZ* null, %struct.XYZ** %captured1, align 8
+// ZERO:  %call = call %struct.XYZ* @create(
+// PATTERN-LABEL: test_block_self_init(
+// PATTERN:   %block = alloca <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor*, i8* }>, align 8
+// PATTERN:   %captured1 = getelementptr inbounds 
%struct.__block_byref_captured, %struct.__block_byref_captured* %captured, i32 
0, i32 4
+// PATTERN-NEXT:  store %struct.XYZ* inttoptr (i64 -6148914691236517206 to 
%struct.XYZ*), %struct.XYZ** %captured1, align 8
+// PATTERN:   %call = call %struct.XYZ* @create(
+void test_block_self_init() {
+  using Block = void (^)();
+  typedef struct XYZ {
+Block block;
+  } * xyz_t;
+  extern xyz_t create(Block block);
+  __block xyz_t captured = create(^() {
+(void)captured;
+  });
+}
+
 // This type of code is currently not handled by zero / pattern initialization.
 // The test will break when that is fixed.
 // UNINIT-LABEL:  test_goto_unreachable_value(


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


[PATCH] D57902: [AST] Fix structural inequivalence of operators

2019-02-07 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added inline comments.



Comment at: unittests/AST/StructuralEquivalenceTest.cpp:251
+
+TEST_F(StructuralEquivalenceFunctionTest, CtorVsDtor) {
+  auto t = makeDecls(

Curious, is this test just for completeness or is this somehow related to the 
overloaded operator check?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57902



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


r353493 - [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

2019-02-07 Thread Eli Friedman via cfe-commits
Author: efriedma
Date: Thu Feb  7 17:17:49 2019
New Revision: 353493

URL: http://llvm.org/viewvc/llvm-project?rev=353493=rev
Log:
[COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

r344765 added those intrinsics, but used the wrong types.

Patch by Mike Hommey

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


Modified:
cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Headers/intrin.h
cfe/trunk/test/CodeGen/arm64-microsoft-status-reg.cpp

Modified: cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsAArch64.def?rev=353493=353492=353493=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsAArch64.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsAArch64.def Thu Feb  7 17:17:49 2019
@@ -203,8 +203,8 @@ TARGET_HEADER_BUILTIN(_InterlockedDecrem
 
 TARGET_HEADER_BUILTIN(_ReadWriteBarrier, "v", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(__getReg, "ULLii", "nh", "intrin.h", ALL_MS_LANGUAGES, 
"")
-TARGET_HEADER_BUILTIN(_ReadStatusReg,  "ii",  "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
-TARGET_HEADER_BUILTIN(_WriteStatusReg, "vii", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(_ReadStatusReg,  "LLii",  "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(_WriteStatusReg, "viLLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_AddressOfReturnAddress, "v*", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 
 #undef BUILTIN

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=353493=353492=353493=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Thu Feb  7 17:17:49 2019
@@ -7076,19 +7076,16 @@ Value *CodeGenFunction::EmitAArch64Built
 llvm::Value *Metadata = llvm::MetadataAsValue::get(Context, RegName);
 
 llvm::Type *RegisterType = Int64Ty;
-llvm::Type *ValueType = Int32Ty;
 llvm::Type *Types[] = { RegisterType };
 
 if (BuiltinID == AArch64::BI_ReadStatusReg) {
   llvm::Function *F = CGM.getIntrinsic(llvm::Intrinsic::read_register, 
Types);
-  llvm::Value *Call = Builder.CreateCall(F, Metadata);
 
-  return Builder.CreateTrunc(Call, ValueType);
+  return Builder.CreateCall(F, Metadata);
 }
 
 llvm::Function *F = CGM.getIntrinsic(llvm::Intrinsic::write_register, 
Types);
 llvm::Value *ArgValue = EmitScalarExpr(E->getArg(1));
-ArgValue = Builder.CreateZExt(ArgValue, RegisterType);
 
 return Builder.CreateCall(F, { Metadata, ArgValue });
   }

Modified: cfe/trunk/lib/Headers/intrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/intrin.h?rev=353493=353492=353493=diff
==
--- cfe/trunk/lib/Headers/intrin.h (original)
+++ cfe/trunk/lib/Headers/intrin.h Thu Feb  7 17:17:49 2019
@@ -554,8 +554,8 @@ __nop(void) {
 #if defined(__aarch64__)
 unsigned __int64 __getReg(int);
 long _InterlockedAdd(long volatile *Addend, long Value);
-int _ReadStatusReg(int);
-void _WriteStatusReg(int, int);
+__int64 _ReadStatusReg(int);
+void _WriteStatusReg(int, __int64);
 
 static inline unsigned short _byteswap_ushort (unsigned short val) {
   return __builtin_bswap16(val);

Modified: cfe/trunk/test/CodeGen/arm64-microsoft-status-reg.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/arm64-microsoft-status-reg.cpp?rev=353493=353492=353493=diff
==
--- cfe/trunk/test/CodeGen/arm64-microsoft-status-reg.cpp (original)
+++ cfe/trunk/test/CodeGen/arm64-microsoft-status-reg.cpp Thu Feb  7 17:17:49 
2019
@@ -23,88 +23,112 @@
 #define ARM64_TPIDRRO_EL0   ARM64_SYSREG(3,3,13, 0,3)  // Thread ID 
Register, User Read Only [CP15_TPIDRURO]
 #define ARM64_TPIDR_EL1 ARM64_SYSREG(3,0,13, 0,4)  // Thread ID 
Register, Privileged Only [CP15_TPIDRPRW]
 
-void check_ReadWriteStatusReg(int v) {
-  int ret;
+// From intrin.h
+__int64 _ReadStatusReg(int);
+void _WriteStatusReg(int, __int64);
+
+void check_ReadWriteStatusReg(__int64 v) {
+  __int64 ret;
   ret = _ReadStatusReg(ARM64_CNTVCT);
-// CHECK-ASM: mrs x8, CNTVCT_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD2:.*]])
+// CHECK-ASM: mrs x0, CNTVCT_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata 
![[MD2:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_PMCCNTR_EL0);
-// CHECK-ASM: mrs x8, PMCCNTR_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD3:.*]])
+// CHECK-ASM: mrs x0, PMCCNTR_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata 

[PATCH] D57874: [WebAssembly] Make thread-related options consistent

2019-02-07 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added inline comments.



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:66
+if (Args.hasFlag(clang::driver::options::OPT_pthread,
+ clang::driver::options::OPT_no_pthread),
+false)

aheejin wrote:
> This code is not strictly related, but `hasFlag` is better than `hasArg` when 
> there are both positive and negative versions of an option exist.
Hmm.. there are currently no other references to OPT_no_pthread in the whole 
codebase.   Maybe better to simply remove the option?

I wouldn't want to commit this as that first use of the option as it might make 
it hard to remove :)



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:201
+  if (HasAtomics || HasPthread)
+return "posix";
   return "single";

We are currently the only platform that overrides this.. i hope it can be 
removed completely at some point ..


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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


[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

2019-02-07 Thread Eli Friedman via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL353493: [COFF, ARM64] Fix types for _ReadStatusReg, 
_WriteStatusReg (authored by efriedma, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57636?vs=185218=185899#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57636

Files:
  cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/lib/Headers/intrin.h
  cfe/trunk/test/CodeGen/arm64-microsoft-status-reg.cpp

Index: cfe/trunk/lib/Headers/intrin.h
===
--- cfe/trunk/lib/Headers/intrin.h
+++ cfe/trunk/lib/Headers/intrin.h
@@ -554,8 +554,8 @@
 #if defined(__aarch64__)
 unsigned __int64 __getReg(int);
 long _InterlockedAdd(long volatile *Addend, long Value);
-int _ReadStatusReg(int);
-void _WriteStatusReg(int, int);
+__int64 _ReadStatusReg(int);
+void _WriteStatusReg(int, __int64);
 
 static inline unsigned short _byteswap_ushort (unsigned short val) {
   return __builtin_bswap16(val);
Index: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
===
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp
@@ -7076,19 +7076,16 @@
 llvm::Value *Metadata = llvm::MetadataAsValue::get(Context, RegName);
 
 llvm::Type *RegisterType = Int64Ty;
-llvm::Type *ValueType = Int32Ty;
 llvm::Type *Types[] = { RegisterType };
 
 if (BuiltinID == AArch64::BI_ReadStatusReg) {
   llvm::Function *F = CGM.getIntrinsic(llvm::Intrinsic::read_register, Types);
-  llvm::Value *Call = Builder.CreateCall(F, Metadata);
 
-  return Builder.CreateTrunc(Call, ValueType);
+  return Builder.CreateCall(F, Metadata);
 }
 
 llvm::Function *F = CGM.getIntrinsic(llvm::Intrinsic::write_register, Types);
 llvm::Value *ArgValue = EmitScalarExpr(E->getArg(1));
-ArgValue = Builder.CreateZExt(ArgValue, RegisterType);
 
 return Builder.CreateCall(F, { Metadata, ArgValue });
   }
Index: cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
===
--- cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
+++ cfe/trunk/include/clang/Basic/BuiltinsAArch64.def
@@ -203,8 +203,8 @@
 
 TARGET_HEADER_BUILTIN(_ReadWriteBarrier, "v", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(__getReg, "ULLii", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
-TARGET_HEADER_BUILTIN(_ReadStatusReg,  "ii",  "nh", "intrin.h", ALL_MS_LANGUAGES, "")
-TARGET_HEADER_BUILTIN(_WriteStatusReg, "vii", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(_ReadStatusReg,  "LLii",  "nh", "intrin.h", ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(_WriteStatusReg, "viLLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_AddressOfReturnAddress, "v*", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 
 #undef BUILTIN
Index: cfe/trunk/test/CodeGen/arm64-microsoft-status-reg.cpp
===
--- cfe/trunk/test/CodeGen/arm64-microsoft-status-reg.cpp
+++ cfe/trunk/test/CodeGen/arm64-microsoft-status-reg.cpp
@@ -23,88 +23,112 @@
 #define ARM64_TPIDRRO_EL0   ARM64_SYSREG(3,3,13, 0,3)  // Thread ID Register, User Read Only [CP15_TPIDRURO]
 #define ARM64_TPIDR_EL1 ARM64_SYSREG(3,0,13, 0,4)  // Thread ID Register, Privileged Only [CP15_TPIDRPRW]
 
-void check_ReadWriteStatusReg(int v) {
-  int ret;
+// From intrin.h
+__int64 _ReadStatusReg(int);
+void _WriteStatusReg(int, __int64);
+
+void check_ReadWriteStatusReg(__int64 v) {
+  __int64 ret;
   ret = _ReadStatusReg(ARM64_CNTVCT);
-// CHECK-ASM: mrs x8, CNTVCT_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD2:.*]])
+// CHECK-ASM: mrs x0, CNTVCT_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD2:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_PMCCNTR_EL0);
-// CHECK-ASM: mrs x8, PMCCNTR_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD3:.*]])
+// CHECK-ASM: mrs x0, PMCCNTR_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD3:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_PMSELR_EL0);
-// CHECK-ASM: mrs x8, PMSELR_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD4:.*]])
+// CHECK-ASM: mrs x0, PMSELR_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD4:.*]])
+// CHECK-IR-NEXT: store i64 %[[VAR]]
 
   ret = _ReadStatusReg(ARM64_PMXEVCNTR_EL0);
-// CHECK-ASM: mrs x8, PMXEVCNTR_EL0
-// CHECK-IR: call i64 @llvm.read_register.i64(metadata ![[MD5:.*]])
+// CHECK-ASM: mrs x0, PMXEVCNTR_EL0
+// CHECK-IR: %[[VAR:.*]] = call i64 @llvm.read_register.i64(metadata ![[MD5:.*]])
+// 

[PATCH] D57930: [Driver] Verify GCCInstallation is valid

2019-02-07 Thread Stephen Hines via Phabricator via cfe-commits
srhines added a comment.

Would it be reasonable to have a test for this with perhaps an invalid GCC 
installation? There is some mock GCC/sysroot testing in 
https://github.com/llvm/llvm-project/blob/master/clang/test/Driver/android-gcc-toolchain.c
 and 
https://github.com/llvm/llvm-project/blob/master/clang/test/Driver/android-ndk-standalone.cpp.
 I am not sure that it will be easy to trip this same bug that way, but I think 
it is possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57930



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


[PATCH] D57874: [WebAssembly] Make thread-related options consistent

2019-02-07 Thread Dan Gohman via Phabricator via cfe-commits
sunfish added a comment.

> - `-matomics` means `-mthread-model posix`

The others sound reasonable, though this one seems a little surprising -- a 
user might have -matomics enabled because they're targeting a VM that has 
atomics, but still not want to use -mthread-model posix because their code 
doesn't actually using threads.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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


[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

2019-02-07 Thread Mike Hommey via Phabricator via cfe-commits
glandium added a comment.

In D57636#1389971 , @efriedma wrote:

> LGTM. Do you want me to commit this for you?


Yes, please. Thank you.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57636



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


[PATCH] D57797: Variable auto-init: fix __block initialization

2019-02-07 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 185896.
jfb added a comment.

- Remove whitespace changes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57797

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGenCXX/trivial-auto-var-init.cpp


Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -30,6 +30,32 @@
   used(block);
 }
 
+// Using the variable being initialized is typically UB in C, but for blocks we
+// can be nice: they imply extra book-keeping and we can do the auto-init 
before
+// any of said book-keeping.
+//
+// UNINIT-LABEL:  test_block_self_init(
+// ZERO-LABEL:test_block_self_init(
+// ZERO:  %block = alloca <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor*, i8* }>, align 8
+// ZERO:  %captured1 = getelementptr inbounds 
%struct.__block_byref_captured, %struct.__block_byref_captured* %captured, i32 
0, i32 4
+// ZERO-NEXT: store %struct.XYZ* null, %struct.XYZ** %captured1, align 8
+// ZERO:  %call = call %struct.XYZ* @create(
+// PATTERN-LABEL: test_block_self_init(
+// PATTERN:   %block = alloca <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor*, i8* }>, align 8
+// PATTERN:   %captured1 = getelementptr inbounds 
%struct.__block_byref_captured, %struct.__block_byref_captured* %captured, i32 
0, i32 4
+// PATTERN-NEXT:  store %struct.XYZ* inttoptr (i64 -6148914691236517206 to 
%struct.XYZ*), %struct.XYZ** %captured1, align 8
+// PATTERN:   %call = call %struct.XYZ* @create(
+void test_block_self_init() {
+  using Block = void (^)();
+  typedef struct XYZ {
+Block block;
+  } * xyz_t;
+  extern xyz_t create(Block block);
+  __block xyz_t captured = create(^() {
+(void)captured;
+  });
+}
+
 // This type of code is currently not handled by zero / pattern initialization.
 // The test will break when that is fixed.
 // UNINIT-LABEL:  test_goto_unreachable_value(
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1633,11 +1633,15 @@
   ? LangOptions::TrivialAutoVarInitKind::Uninitialized
   : getContext().getLangOpts().getTrivialAutoVarInit()));
 
-  auto initializeWhatIsTechnicallyUninitialized = [&]() {
+  auto initializeWhatIsTechnicallyUninitialized = [&](Address Loc) {
 if (trivialAutoVarInit ==
 LangOptions::TrivialAutoVarInitKind::Uninitialized)
   return;
 
+// Only initialize a __block's storage: we always initialize the header.
+if (emission.IsEscapingByRef)
+  Loc = emitBlockByrefAddress(Loc, , /*follow=*/false);
+
 CharUnits Size = getContext().getTypeSizeInChars(type);
 if (!Size.isZero()) {
   switch (trivialAutoVarInit) {
@@ -1714,7 +1718,7 @@
   };
 
   if (isTrivialInitializer(Init)) {
-initializeWhatIsTechnicallyUninitialized();
+initializeWhatIsTechnicallyUninitialized(Loc);
 return;
   }
 
@@ -1728,7 +1732,7 @@
   }
 
   if (!constant) {
-initializeWhatIsTechnicallyUninitialized();
+initializeWhatIsTechnicallyUninitialized(Loc);
 LValue lv = MakeAddrLValue(Loc, type);
 lv.setNonGC(true);
 return EmitExprAsInit(Init, , lv, capturedByInit);


Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -30,6 +30,32 @@
   used(block);
 }
 
+// Using the variable being initialized is typically UB in C, but for blocks we
+// can be nice: they imply extra book-keeping and we can do the auto-init before
+// any of said book-keeping.
+//
+// UNINIT-LABEL:  test_block_self_init(
+// ZERO-LABEL:test_block_self_init(
+// ZERO:  %block = alloca <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i8* }>, align 8
+// ZERO:  %captured1 = getelementptr inbounds %struct.__block_byref_captured, %struct.__block_byref_captured* %captured, i32 0, i32 4
+// ZERO-NEXT: store %struct.XYZ* null, %struct.XYZ** %captured1, align 8
+// ZERO:  %call = call %struct.XYZ* @create(
+// PATTERN-LABEL: test_block_self_init(
+// PATTERN:   %block = alloca <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i8* }>, align 8
+// PATTERN:   %captured1 = getelementptr inbounds %struct.__block_byref_captured, %struct.__block_byref_captured* %captured, i32 0, i32 4
+// PATTERN-NEXT:  store %struct.XYZ* inttoptr (i64 -6148914691236517206 to %struct.XYZ*), %struct.XYZ** %captured1, align 8
+// PATTERN:   %call = call %struct.XYZ* @create(
+void test_block_self_init() {
+  using Block = void (^)();
+  typedef struct XYZ {
+Block block;
+  } * xyz_t;
+  extern xyz_t create(Block block);
+  __block xyz_t captured = create(^() {
+(void)captured;
+  });

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

2019-02-07 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM. Do you want me to commit this for you?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57636



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


[PATCH] D57874: [WebAssembly] Set '-matomics' when '-pthread' is set

2019-02-07 Thread Heejin Ahn via Phabricator via cfe-commits
aheejin marked an inline comment as done.
aheejin added inline comments.



Comment at: lib/Driver/ToolChains/WebAssembly.cpp:66
+if (Args.hasFlag(clang::driver::options::OPT_pthread,
+ clang::driver::options::OPT_no_pthread),
+false)

This code is not strictly related, but `hasFlag` is better than `hasArg` when 
there are both positive and negative versions of an option exist.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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


[PATCH] D57874: [WebAssembly] Set '-matomics' when '-pthread' is set

2019-02-07 Thread Heejin Ahn via Phabricator via cfe-commits
aheejin updated this revision to Diff 185895.
aheejin added a comment.

Sorry nevermind my previous code. There was not hacky and much cleaner way to 
do everything in the driver layer. (Before I tried to do everything in the cc1 
compilation layer :( )

Anyway, moved all logic to the driver layer and did this:

- `-matomics` means `-mthread-model posix`
- `-mthread-model posix` means `-matomics`
- `-pthread` means both `-matomics` and `-mthread-model posix`

It currently does not check mismatches and crash. If either of `-matomics` or 
`-mthread-model posix` is set it is gonna set both. THe same for `-pthread`. 
Not sure which flag is the *canonical* one to trust.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874

Files:
  include/clang/Driver/ToolChain.h
  lib/Driver/Driver.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/WebAssembly.cpp
  lib/Driver/ToolChains/WebAssembly.h
  test/Driver/wasm-toolchain.c
  test/Driver/wasm-toolchain.cpp

Index: test/Driver/wasm-toolchain.cpp
===
--- test/Driver/wasm-toolchain.cpp
+++ test/Driver/wasm-toolchain.cpp
@@ -38,3 +38,17 @@
 
 // RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl --sysroot=/foo --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=COMPILE %s
 // COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" "/foo/include/wasm32-wasi-musl/c++/v1" "-internal-isystem" "/foo/include/c++/v1" "-internal-isystem" "/foo/include/wasm32-wasi-musl" "-internal-isystem" "/foo/include"
+
+// Thread-related command line tests.
+// - '-matomics' sets '-mthread-model posix'
+// - '-mthread-model' sets '-matomics'
+// - '-phread' sets both '-matomics' and '-mthread-model posix'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s 2>&1 -matomics | FileCheck -check-prefix=ATOMICS %s
+// ATOMICS: clang{{.*}}" "-cc1" {{.*}} "-mthread-model" "posix"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s 2>&1 -mthread-model posix | FileCheck -check-prefix=POSIX %s
+// POSIX: clang{{.*}}" "-cc1" {{.*}} "-target-feature +atomics"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s 2>&1 -pthread | FileCheck -check-prefix=PTHREAD %s
+// PTHREAD: clang{{.*}}" "-cc1" {{.*}} "-mthread-model" "posix" {{.*}} "-target-feature +atomics"
Index: test/Driver/wasm-toolchain.c
===
--- test/Driver/wasm-toolchain.c
+++ test/Driver/wasm-toolchain.c
@@ -38,3 +38,17 @@
 
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-wasi-musl --sysroot=/foo %s 2>&1 | FileCheck -check-prefix=COMPILE %s
 // COMPILE: clang{{.*}}" "-cc1" {{.*}} "-internal-isystem" "/foo/include/wasm32-wasi-musl" "-internal-isystem" "/foo/include"
+
+// Thread-related command line tests.
+// - '-matomics' sets '-mthread-model posix'
+// - '-mthread-model' sets '-matomics'
+// - '-phread' sets both '-matomics' and '-mthread-model posix'
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s 2>&1 -matomics | FileCheck -check-prefix=ATOMICS %s
+// ATOMICS: clang{{.*}}" "-cc1" {{.*}} "-mthread-model" "posix"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s 2>&1 -mthread-model posix | FileCheck -check-prefix=POSIX %s
+// POSIX: clang{{.*}}" "-cc1" {{.*}} "-target-feature +atomics"
+
+// RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s 2>&1 -pthread | FileCheck -check-prefix=PTHREAD %s
+// PTHREAD: clang{{.*}}" "-cc1" {{.*}} "-mthread-model" "posix" {{.*}} "-target-feature +atomics"
Index: lib/Driver/ToolChains/WebAssembly.h
===
--- lib/Driver/ToolChains/WebAssembly.h
+++ lib/Driver/ToolChains/WebAssembly.h
@@ -64,7 +64,7 @@
   llvm::opt::ArgStringList ) const override;
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList ,
llvm::opt::ArgStringList ) const override;
-  std::string getThreadModel() const override;
+  std::string getThreadModel(const llvm::opt::ArgList &) const override;
 
   const char *getDefaultLinker() const override { return "wasm-ld"; }
 
Index: lib/Driver/ToolChains/WebAssembly.cpp
===
--- lib/Driver/ToolChains/WebAssembly.cpp
+++ lib/Driver/ToolChains/WebAssembly.cpp
@@ -62,7 +62,9 @@
 if (ToolChain.ShouldLinkCXXStdlib(Args))
   ToolChain.AddCXXStdlibLibArgs(Args, CmdArgs);
 
-if (Args.hasArg(options::OPT_pthread))
+if (Args.hasFlag(clang::driver::options::OPT_pthread,
+ clang::driver::options::OPT_no_pthread),
+false)
   CmdArgs.push_back("-lpthread");
 
 CmdArgs.push_back("-lc");
@@ -123,6 +125,13 @@
   if 

[PATCH] D57636: [COFF, ARM64] Fix types for _ReadStatusReg, _WriteStatusReg

2019-02-07 Thread Mike Hommey via Phabricator via cfe-commits
glandium added a comment.

@efriedma can you take another look? Ideally, this should be backported to the 
release_80 branch, so that would need to be landed asap.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57636



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


[PATCH] D57797: Variable auto-init: fix __block initialization

2019-02-07 Thread JF Bastien via Phabricator via cfe-commits
jfb marked an inline comment as done.
jfb added inline comments.



Comment at: lib/CodeGen/CGDecl.cpp:1642
 
 CharUnits Size = getContext().getTypeSizeInChars(type);
 if (!Size.isZero()) {

jfb wrote:
> rjmccall wrote:
> > Does this check handle flexible arrays on zero-sized structures properly?  
> > I suppose they're always initialized.
> You mean something like
> ```
> void foo(int size) {
>   struct Z {};
>   Z arr[size];
> }
> ```
> ?
Just to circle back, the above code lowers to:

```
%struct.Z = type { i8 }

@__const._Z3fooi.arr = private unnamed_addr constant %struct.Z { i8 -86 }, 
align 1

; Function Attrs: noinline nounwind optnone ssp uwtable
define void @_Z3fooi(i32 %size) #0 {
entry:
  %size.addr = alloca i32, align 4
  %saved_stack = alloca i8*, align 8
  %__vla_expr0 = alloca i64, align 8
  store i32 %size, i32* %size.addr, align 4
  %0 = load i32, i32* %size.addr, align 4
  %1 = zext i32 %0 to i64
  %2 = call i8* @llvm.stacksave()
  store i8* %2, i8** %saved_stack, align 8
  %vla = alloca %struct.Z, i64 %1, align 16
  store i64 %1, i64* %__vla_expr0, align 8
  %vla.iszerosized = icmp eq i64 %1, 0
  br i1 %vla.iszerosized, label %vla-init.cont, label %vla-setup.loop

vla-setup.loop:   ; preds = %entry
  %vla.begin = bitcast %struct.Z* %vla to i8*
  %vla.end = getelementptr inbounds i8, i8* %vla.begin, i64 %1
  br label %vla-init.loop

vla-init.loop:; preds = %vla-init.loop, 
%vla-setup.loop
  %vla.cur = phi i8* [ %vla.begin, %vla-setup.loop ], [ %vla.next, 
%vla-init.loop ]
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 %vla.cur, i8* align 1 
getelementptr inbounds (%struct.Z, %struct.Z* @__const._Z3fooi.arr, i32 0, i32 
0), i64 1, i1 false)
  %vla.next = getelementptr inbounds i8, i8* %vla.cur, i64 1
  %vla-init.isdone = icmp eq i8* %vla.next, %vla.end
  br i1 %vla-init.isdone, label %vla-init.cont, label %vla-init.loop

vla-init.cont:; preds = %vla-init.loop, 
%entry
  %3 = load i8*, i8** %saved_stack, align 8
  call void @llvm.stackrestore(i8* %3)
  ret void
}
```

So yes it's handled.



Comment at: lib/CodeGen/CGDecl.cpp:1663
 const VariableArrayType *VlaType =
 dyn_cast_or_null(getContext().getAsArrayType(type));
 if (!VlaType)

jfb wrote:
> rjmccall wrote:
> > This is a late comment on the main patch, but there's an 
> > `ASTContext::getAsVariableArrayType` method.
> OK I can do in a separate follow-up.
https://reviews.llvm.org/rC353490



Comment at: lib/CodeGen/CGDecl.cpp:1605
-emitByrefStructureInit(emission);
-
   // Initialize the variable here if it doesn't have a initializer and it is a

jfb wrote:
> rjmccall wrote:
> > Are these changes still needed?
> I believe so, see concurrent comment here:
> https://reviews.llvm.org/D57797#inline-512702
Actually I'm wrong, updated patch drops this and the test makes sure the call 
is after this initialization. Sorry for the confusion. This is much cleaner.



Comment at: lib/CodeGen/CGDecl.cpp:1726
+emitByrefStructureInit(emission);
+  }
+

rjmccall wrote:
> jfb wrote:
> > Note that we still want this to be pulled out in this way because 
> > `emitByrefStructureInit` emits the call to the initializer (in 
> > `test_block_self_init` the call to `create`). Were we to leave this as it 
> > was before, one of the initializations below would be emitted, but it would 
> > be *after* the call.
> > 
> > Similarly, we also want the little dance I added to only initialize once.
> Oh, that's interesting, I hadn't remembered that.  Alright, in that case 
> LGTM, I guess.
> 
> ...unless you want to refactor this so that `emitByrefStructureInit` only 
> handles the header initialization and the bit about handling the initializer 
> is handled more cleanly down in the rest of this code.
As noted above, I was wrong here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57797



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


[PATCH] D57797: Variable auto-init: fix __block initialization

2019-02-07 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 185892.
jfb marked 4 inline comments as done.
jfb added a comment.

- Simplify patch greatly.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57797

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGenCXX/trivial-auto-var-init.cpp


Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -30,6 +30,32 @@
   used(block);
 }
 
+// Using the variable being initialized is typically UB in C, but for blocks we
+// can be nice: they imply extra book-keeping and we can do the auto-init 
before
+// any of said book-keeping.
+//
+// UNINIT-LABEL:  test_block_self_init(
+// ZERO-LABEL:test_block_self_init(
+// ZERO:  %block = alloca <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor*, i8* }>, align 8
+// ZERO:  %captured1 = getelementptr inbounds 
%struct.__block_byref_captured, %struct.__block_byref_captured* %captured, i32 
0, i32 4
+// ZERO-NEXT: store %struct.XYZ* null, %struct.XYZ** %captured1, align 8
+// ZERO:  %call = call %struct.XYZ* @create(
+// PATTERN-LABEL: test_block_self_init(
+// PATTERN:   %block = alloca <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor*, i8* }>, align 8
+// PATTERN:   %captured1 = getelementptr inbounds 
%struct.__block_byref_captured, %struct.__block_byref_captured* %captured, i32 
0, i32 4
+// PATTERN-NEXT:  store %struct.XYZ* inttoptr (i64 -6148914691236517206 to 
%struct.XYZ*), %struct.XYZ** %captured1, align 8
+// PATTERN:   %call = call %struct.XYZ* @create(
+void test_block_self_init() {
+  using Block = void (^)();
+  typedef struct XYZ {
+Block block;
+  } * xyz_t;
+  extern xyz_t create(Block block);
+  __block xyz_t captured = create(^() {
+(void)captured;
+  });
+}
+
 // This type of code is currently not handled by zero / pattern initialization.
 // The test will break when that is fixed.
 // UNINIT-LABEL:  test_goto_unreachable_value(
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1598,7 +1598,7 @@
 if (!Init || !ContainsLabel(Init)) return;
 EnsureInsertPoint();
   }
-
+  
   // Initialize the structure of a __block variable.
   if (emission.IsEscapingByRef)
 emitByrefStructureInit(emission);
@@ -1606,9 +1606,8 @@
   // Initialize the variable here if it doesn't have a initializer and it is a
   // C struct that is non-trivial to initialize or an array containing such a
   // struct.
-  if (!Init &&
-  type.isNonTrivialToPrimitiveDefaultInitialize() ==
-  QualType::PDIK_Struct) {
+  if (!Init && type.isNonTrivialToPrimitiveDefaultInitialize() ==
+   QualType::PDIK_Struct) {
 LValue Dst = MakeAddrLValue(emission.getAllocatedAddress(), type);
 if (emission.IsEscapingByRef)
   drillIntoBlockVariable(*this, Dst, );
@@ -1633,11 +1632,15 @@
   ? LangOptions::TrivialAutoVarInitKind::Uninitialized
   : getContext().getLangOpts().getTrivialAutoVarInit()));
 
-  auto initializeWhatIsTechnicallyUninitialized = [&]() {
+  auto initializeWhatIsTechnicallyUninitialized = [&](Address Loc) {
 if (trivialAutoVarInit ==
 LangOptions::TrivialAutoVarInitKind::Uninitialized)
   return;
 
+// Only initialize a __block's storage: we always initialize the header.
+if (emission.IsEscapingByRef)
+  Loc = emitBlockByrefAddress(Loc, , /*follow=*/false);
+
 CharUnits Size = getContext().getTypeSizeInChars(type);
 if (!Size.isZero()) {
   switch (trivialAutoVarInit) {
@@ -1715,7 +1718,7 @@
   };
 
   if (isTrivialInitializer(Init)) {
-initializeWhatIsTechnicallyUninitialized();
+initializeWhatIsTechnicallyUninitialized(Loc);
 return;
   }
 
@@ -1729,7 +1732,7 @@
   }
 
   if (!constant) {
-initializeWhatIsTechnicallyUninitialized();
+initializeWhatIsTechnicallyUninitialized(Loc);
 LValue lv = MakeAddrLValue(Loc, type);
 lv.setNonGC(true);
 return EmitExprAsInit(Init, , lv, capturedByInit);


Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -30,6 +30,32 @@
   used(block);
 }
 
+// Using the variable being initialized is typically UB in C, but for blocks we
+// can be nice: they imply extra book-keeping and we can do the auto-init before
+// any of said book-keeping.
+//
+// UNINIT-LABEL:  test_block_self_init(
+// ZERO-LABEL:test_block_self_init(
+// ZERO:  %block = alloca <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i8* }>, align 8
+// ZERO:  %captured1 = getelementptr inbounds %struct.__block_byref_captured, %struct.__block_byref_captured* 

r353490 - [NFC] Variable auto-init: use getAsVariableArrayType helper

2019-02-07 Thread JF Bastien via cfe-commits
Author: jfb
Date: Thu Feb  7 16:51:05 2019
New Revision: 353490

URL: http://llvm.org/viewvc/llvm-project?rev=353490=rev
Log:
[NFC] Variable auto-init: use getAsVariableArrayType helper

As suggested by @rjmccall in D57797.

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

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=353490=353489=353490=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Thu Feb  7 16:51:05 2019
@@ -1658,8 +1658,7 @@ void CodeGenFunction::EmitAutoVarInit(co
 // Technically zero-sized or negative-sized VLAs are undefined, and UBSan
 // will catch that code, but there exists code which generates zero-sized
 // VLAs. Be nice and initialize whatever they requested.
-const VariableArrayType *VlaType =
-dyn_cast_or_null(getContext().getAsArrayType(type));
+const auto *VlaType = getContext().getAsVariableArrayType(type);
 if (!VlaType)
   return;
 auto VlaSize = getVLASize(VlaType);


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


[PATCH] D57930: [Driver] Verify GCCInstallation is valid

2019-02-07 Thread Daniel Mentz via Phabricator via cfe-commits
danielmentz created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Values returned by GCCInstallation.getParentLibPath() and
GCCInstallation.getTriple() are not valid unless
GCCInstallation.isValid() returns true. This has previously been
ignored, and the former two values were used without checking whether
GCCInstallation is valid. This led to the bad path "/../bin" being added
to the list of program paths.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57930

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


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -229,9 +229,11 @@
   // used to target i386.
   // FIXME: This seems unlikely to be Linux-specific.
   ToolChain::path_list  = getProgramPaths();
-  PPaths.push_back(Twine(GCCInstallation.getParentLibPath() + "/../" +
- GCCInstallation.getTriple().str() + "/bin")
-   .str());
+  if (GCCInstallation.isValid()) {
+PPaths.push_back(Twine(GCCInstallation.getParentLibPath() + "/../" +
+   GCCInstallation.getTriple().str() + "/bin")
+ .str());
+  }
 
   Distro Distro(D.getVFS());
 


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -229,9 +229,11 @@
   // used to target i386.
   // FIXME: This seems unlikely to be Linux-specific.
   ToolChain::path_list  = getProgramPaths();
-  PPaths.push_back(Twine(GCCInstallation.getParentLibPath() + "/../" +
- GCCInstallation.getTriple().str() + "/bin")
-   .str());
+  if (GCCInstallation.isValid()) {
+PPaths.push_back(Twine(GCCInstallation.getParentLibPath() + "/../" +
+   GCCInstallation.getTriple().str() + "/bin")
+ .str());
+  }
 
   Distro Distro(D.getVFS());
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D57797: Variable auto-init: fix __block initialization

2019-02-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: lib/CodeGen/CGDecl.cpp:1726
+emitByrefStructureInit(emission);
+  }
+

jfb wrote:
> Note that we still want this to be pulled out in this way because 
> `emitByrefStructureInit` emits the call to the initializer (in 
> `test_block_self_init` the call to `create`). Were we to leave this as it was 
> before, one of the initializations below would be emitted, but it would be 
> *after* the call.
> 
> Similarly, we also want the little dance I added to only initialize once.
Oh, that's interesting, I hadn't remembered that.  Alright, in that case LGTM, 
I guess.

...unless you want to refactor this so that `emitByrefStructureInit` only 
handles the header initialization and the bit about handling the initializer is 
handled more cleanly down in the rest of this code.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57797



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


[PATCH] D57797: Variable auto-init: fix __block initialization

2019-02-07 Thread JF Bastien via Phabricator via cfe-commits
jfb marked 2 inline comments as done.
jfb added inline comments.



Comment at: lib/CodeGen/CGDecl.cpp:1605
-emitByrefStructureInit(emission);
-
   // Initialize the variable here if it doesn't have a initializer and it is a

rjmccall wrote:
> Are these changes still needed?
I believe so, see concurrent comment here:
https://reviews.llvm.org/D57797#inline-512702


Repository:
  rC Clang

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

https://reviews.llvm.org/D57797



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


[PATCH] D57797: Variable auto-init: fix __block initialization

2019-02-07 Thread JF Bastien via Phabricator via cfe-commits
jfb marked 2 inline comments as done.
jfb added inline comments.



Comment at: lib/CodeGen/CGDecl.cpp:1726
+emitByrefStructureInit(emission);
+  }
+

Note that we still want this to be pulled out in this way because 
`emitByrefStructureInit` emits the call to the initializer (in 
`test_block_self_init` the call to `create`). Were we to leave this as it was 
before, one of the initializations below would be emitted, but it would be 
*after* the call.

Similarly, we also want the little dance I added to only initialize once.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57797



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


[PATCH] D57797: Variable auto-init: fix __block initialization

2019-02-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: lib/CodeGen/CGDecl.cpp:1605
-emitByrefStructureInit(emission);
-
   // Initialize the variable here if it doesn't have a initializer and it is a

Are these changes still needed?



Comment at: lib/CodeGen/CGDecl.cpp:1644
+if (emission.IsEscapingByRef)
+  Loc = emitBlockByrefAddress(Loc, , /*follow=*/false);
 

Thanks, this is what I was looking for.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57797



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


[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-07 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: lib/AST/Stmt.cpp:628
+  DiagOffs = CurPtr-StrStart-1;
+  return diag::err_asm_invalid_operand_for_goto_labels;
+}

efriedma wrote:
> jyu2 wrote:
> > rsmith wrote:
> > > jyu2 wrote:
> > > > rsmith wrote:
> > > > > I'm curious why we're checking this here, when all the other 
> > > > > validation for the modifier letter happens in LLVM. Does this check 
> > > > > need to be done differently from the others?
> > > > This is to verify following in spec:
> > > > =
> > > > To reference a label in the assembler template, prefix it with ‘%l’ 
> > > > (lowercase ‘L’) followed by its (zero-based) position in GotoLabels 
> > > > plus the number of input operands. For example, if the asm has three 
> > > > inputs and references two labels, refer to the first label as ‘%l3’ and 
> > > > the second as ‘%l4’).
> > > > 
> > > > Was request from Eli.
> > > Sorry, I think my question was unclear. I understand why you need to do 
> > > this check somewhere. My question is why this check needs to be *here*, 
> > > given that all the other checks for the modifier letter are performed 
> > > elsewhere. Splitting the checks up between multiple locations makes the 
> > > system harder to maintain.
> > > 
> > > As it happens, there's a FIXME for this here: 
> > > http://llvm.org/doxygen/AsmPrinterInlineAsm_8cpp_source.html#l00436 -- 
> > > and a test case like `void f(void *a) { asm("mov %l0, %%eax" :: "r"(a)); 
> > > }` currently causes LLVM to assert in a function that's supposed to be 
> > > validating these modifier letters and producing a clean error if they're 
> > > bad. So I think that's an LLVM bug, and fixing that LLVM bug would 
> > > enforce the property we need here (that `%lN` only names labels).
> > > 
> > > 
> > > [It's not clear to me whether we should be enforcing that `%lN` only 
> > > names label //operands//, which would require the check to be in Clang 
> > > rather than LLVM; GCC doesn't do that, and for instance accepts
> > > 
> > > ```
> > > void f() { asm goto("jmp %l0" :: "i"(&)::foo); foo:; }
> > > ```
> > > 
> > > (at least for x86_64), though this construct doesn't seem hugely useful 
> > > given that the input operand would need to be a literal address-of-label 
> > > expression and you'd need to name the target label as a label operand 
> > > anyway.]
> > Hi Richard,
> > 
> > Thanks for the detail explanation. 
> > I agree with you.  Our compiler FE don't emit error for this also.
> > 
> > I'd happy to remove this, If we all agree.  
> > 
> > Hi Eli, 
> > what do you think?
> > 
> > Thanks.
> > Jennifer  
> > 
> Yes, that's okay.
In the llvm asm-goto patch, I removed the FIXME from AsmPrinterInlineAsm.cpp 
and set the Error flag if the operand is not an MBB or BlockAddress. This 
prevents tripping an assert on Richard's case. The error message could probably 
be better, but at leasts it not a crash on release builds now.


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

https://reviews.llvm.org/D56571



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


[PATCH] D57797: Variable auto-init: fix __block initialization

2019-02-07 Thread JF Bastien via Phabricator via cfe-commits
jfb updated this revision to Diff 185883.
jfb added a comment.

- Only initialize __block's storage.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57797

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGenCXX/trivial-auto-var-init.cpp
  test/CodeGenObjC/arc-blocks.m
  test/CodeGenObjC/blocks-2.m
  test/CodeGenObjC/blocks.m
  test/CodeGenObjCXX/arc-blocks.mm

Index: test/CodeGenObjCXX/arc-blocks.mm
===
--- test/CodeGenObjCXX/arc-blocks.mm
+++ test/CodeGenObjCXX/arc-blocks.mm
@@ -24,14 +24,14 @@
   }
   // CHECK-LABEL:define void @_ZN5test03fooEv() 
   // CHECK:  [[V:%.*]] = alloca [[BYREF_A:%.*]], align 8
+  // CHECK:  [[V1:%.*]] = getelementptr inbounds [[BYREF_A]], [[BYREF_A]]* [[V]], i32 0, i32 7
   // CHECK:  [[T0:%.*]] = getelementptr inbounds [[BYREF_A]], [[BYREF_A]]* [[V]], i32 0, i32 4
   // CHECK-NEXT: store i8* bitcast (void (i8*, i8*)* [[COPY_HELPER:@.*]] to i8*), i8** [[T0]]
   // CHECK-NEXT: [[T0:%.*]] = getelementptr inbounds [[BYREF_A]], [[BYREF_A]]* [[V]], i32 0, i32 5
   // CHECK-NEXT: store i8* bitcast (void (i8*)* [[DISPOSE_HELPER:@.*]] to i8*), i8** [[T0]]
   // CHECK-NEXT: [[T0:%.*]] = getelementptr inbounds [[BYREF_A]], [[BYREF_A]]* [[V]], i32 0, i32 6
   // CHECK-NEXT: store i8* getelementptr inbounds ([3 x i8], [3 x i8]* [[LAYOUT0]], i32 0, i32 0), i8** [[T0]]
-  // CHECK-NEXT: [[T0:%.*]] = getelementptr inbounds [[BYREF_A]], [[BYREF_A]]* [[V]], i32 0, i32 7
-  // CHECK-NEXT: call void @_ZN5test01AC1Ev([[A]]* [[T0]])
+  // CHECK-NEXT: call void @_ZN5test01AC1Ev([[A]]* [[V1]])
   // CHECK-NEXT: [[T0:%.*]] = getelementptr inbounds [[BYREF_A]], [[BYREF_A]]* [[V]], i32 0, i32 7
   // CHECK: bitcast [[BYREF_A]]* [[V]] to i8*
   // CHECK: [[T1:%.*]] = bitcast [[BYREF_A]]* [[V]] to i8*
Index: test/CodeGenObjC/blocks.m
===
--- test/CodeGenObjC/blocks.m
+++ test/CodeGenObjC/blocks.m
@@ -48,6 +48,7 @@
   // CHECK-NEXT: [[WEAKX:%.*]] = alloca [[WEAK_T:%.*]],
   // CHECK-NEXT: [[BLOCK:%.*]] = alloca [[BLOCK_T:<{.*}>]],
   // CHECK-NEXT: store [[TEST2]]*
+  // CHECK-NEXT: [[T6:%.*]] = getelementptr inbounds [[WEAK_T]], [[WEAK_T]]* [[WEAKX]], i32 0, i32 6
 
   // isa=1 for weak byrefs.
   // CHECK-NEXT: [[T0:%.*]] = getelementptr inbounds [[WEAK_T]], [[WEAK_T]]* [[WEAKX]], i32 0, i32 0
@@ -72,7 +73,6 @@
   // CHECK-NEXT: store i8* bitcast (void (i8*)* @__Block_byref_object_dispose_{{.*}} to i8*), i8** [[T5]]
 
   // Actually capture the value.
-  // CHECK-NEXT: [[T6:%.*]] = getelementptr inbounds [[WEAK_T]], [[WEAK_T]]* [[WEAKX]], i32 0, i32 6
   // CHECK-NEXT: [[CAPTURE:%.*]] = load [[TEST2]]*, [[TEST2]]** [[X]]
   // CHECK-NEXT: store [[TEST2]]* [[CAPTURE]], [[TEST2]]** [[T6]]
 
Index: test/CodeGenObjC/blocks-2.m
===
--- test/CodeGenObjC/blocks-2.m
+++ test/CodeGenObjC/blocks-2.m
@@ -20,7 +20,7 @@
 
   // CHECK:  [[N:%.*]] = alloca [[N_T:%.*]], align 8
   // CHECK:  [[T0:%.*]] = getelementptr inbounds [[N_T]], [[N_T]]* [[N]], i32 0, i32 4
-  // CHECK-NEXT: store double 1.00e+{{0?}}01, double* [[T0]], align 8
+  // CHECK:  store double 1.00e+{{0?}}01, double* [[T0]], align 8
   __block double n = 10;
 
   // CHECK:  invoke void @{{.*}}test1_help
Index: test/CodeGenObjC/arc-blocks.m
===
--- test/CodeGenObjC/arc-blocks.m
+++ test/CodeGenObjC/arc-blocks.m
@@ -122,11 +122,11 @@
   // CHECK-LABEL:define void @test4()
   // CHECK:  [[VAR:%.*]] = alloca [[BYREF_T:%.*]],
   // CHECK-NEXT: [[BLOCK:%.*]] = alloca [[BLOCK_T:<{.*}>]],
+  // CHECK:  [[SLOT:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* [[VAR]], i32 0, i32 6
   // CHECK:  [[T0:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* [[VAR]], i32 0, i32 2
   // 0x0200 - has copy/dispose helpers strong
   // CHECK-NEXT: store i32 838860800, i32* [[T0]]
-  // CHECK:  [[SLOT:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* [[VAR]], i32 0, i32 6
-  // CHECK-NEXT: [[T0:%.*]] = call i8* @test4_source()
+  // CHECK:  [[T0:%.*]] = call i8* @test4_source()
   // CHECK-NEXT: [[T1:%.*]] = call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* [[T0]])
   // CHECK-NEXT: store i8* [[T1]], i8** [[SLOT]]
   // CHECK-NEXT: [[SLOT:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* [[VAR]], i32 0, i32 6
@@ -207,11 +207,11 @@
   // CHECK-NEXT: [[BLOCK:%.*]] = alloca [[BLOCK_T:<{.*}>]],
   // CHECK-NEXT: [[VARPTR1:%.*]] = bitcast [[BYREF_T]]* [[VAR]] to i8*
   // CHECK-NEXT: call void @llvm.lifetime.start.p0i8(i64 48, i8* [[VARPTR1]])
+  // CHECK:  [[SLOT:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* [[VAR]], i32 0, i32 6
   // CHECK:  [[T0:%.*]] = getelementptr inbounds [[BYREF_T]], [[BYREF_T]]* [[VAR]], i32 0, i32 2
   // 0x0200 - 

[PATCH] D57797: Variable auto-init: fix __block initialization

2019-02-07 Thread JF Bastien via Phabricator via cfe-commits
jfb marked 3 inline comments as done.
jfb added inline comments.



Comment at: lib/CodeGen/CGDecl.cpp:1643
 CharUnits Size = getContext().getTypeSizeInChars(type);
 if (!Size.isZero()) {
   switch (trivialAutoVarInit) {

rjmccall wrote:
> jfb wrote:
> > rjmccall wrote:
> > > Can't you just drill to the byref storage right here and avoid all the 
> > > other complexity and subtle ordering interactions?
> > We're in the lambda that does the initialization here. The tricky order 
> > part is that code that calls the lambda does:
> > 
> > - Block (which was missing the early auto-init)
> > - Trivial initializer (which has auto-init, then early exit)
> > - Constant aggregate / constexpr (which might auto-init if it wasn't 
> > constant, and then early-exit)
> > - The other stuff
> > 
> > I don't think here is the right place to do anything... and I'm not sure 
> > what you're suggesting.
> Escaping `__block` variables are basically a fixed-layout header followed by 
> storage of the variable's formal type.  Anything you do at this point in the 
> function to auto-initialize the header is a waste of time and code size 
> because it is precisely at this point in the function that we perform a bunch 
> of stores to initialize that header.   You are mitigating nothing by covering 
> the header.  So what I'm saying is that, in this lambda which is meant to 
> initialize the user-exposed storage of a variable, you should just make sure 
> you're pointing at the user-exposed storage of the variable by calling 
> `emitBlockByrefAddress(false)` (which just does a GEP), and then you can 
> initialize only that.
Thanks, I now understand what you meant. It's super simple indeed, sorry I took 
a while to grok. Updated patch should do this now.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57797



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


[PATCH] D57915: [COFF, ARM64] Remove definitions for _byteswap library functions

2019-02-07 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

I guess we can track inlining separately, if you want to merge this quickly to 
unblock the Chrome build.  LGTM

> the former provides global declaration which seems inherited by the 
> definition in intrin.h.

This is a bug, actually; we support a non-standard Microsoft extension for 
mismatched "static" markings, but the "static" is supposed to win.

Please make sure to file bugs for both the wrong linkage and inlining bswap.


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

https://reviews.llvm.org/D57915



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


[PATCH] D57906: [CTU] Do not allow different CPP dialects in CTU

2019-02-07 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin added a comment.

Hi Gabor,
Please find my comments inline.




Comment at: lib/CrossTU/CrossTranslationUnit.cpp:232
+
+  // We do not support CTU across languages (C vs C++).
   if (LangTo.CPlusPlus != LangFrom.CPlusPlus) {

The comment change looks strange.



Comment at: lib/CrossTU/CrossTranslationUnit.cpp:255
+  LangTo.CPlusPlus2a != LangFrom.CPlusPlus2a) {
+++NumLangMismatch;
+return llvm::make_error(index_error_code::lang_mismatch);

Should we create another counter, like NumLangDialectMismatch?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57906



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


[PATCH] D57905: [ASTImporter][ASTImporterSpecificLookup] Add test for different operators

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

LGTM!




Comment at: unittests/AST/ASTImporterTest.cpp:4869
+
+  // FromPlus have a different TU, thus its DeclarationName is different too.
+  Res = LT.lookup(ToTU, FromPlus->getDeclName());

I think this comment should also be put before ASSERT_NE.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57905



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


[PATCH] D57915: [COFF, ARM64] Remove definitions for _byteswap library functions

2019-02-07 Thread Tom Tan via Phabricator via cfe-commits
TomTan added a comment.

In D57915#1389788 , @efriedma wrote:

> I did some quick testing with MSVC; apparently it inlines the implementations 
> of these functions when optimizations are on.  We definitely want to support 
> inlining these. Since these are commonly used in performance-sensitive code, 
> I'd prefer to implement the required changes to 
> CodeGenFunction::EmitMSVCBuiltinExpr now, rather than chase after weird 
> performance regressions in the future.
>
>  ---
>
> I'm not sure how you could end up with a "duplicate symbols" error from the 
> current implementation, though; these functions are marked "static", so they 
> shouldn't conflict with the real _byteswap_* functions.


It is a great idea to inline these in Clang, could this be tracked for a 
separate bug? The issue blocks Chromium build for Windows ARM64.

Yes, they are marked as `static inline` in `intrin.h` and are expected to link 
fine, but there are cases `stdlib.h` is included before including `intrin.h`, 
the former provides global declaration which seems inherited by the definition 
in `intrin.h`.


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

https://reviews.llvm.org/D57915



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


[PATCH] D57902: [AST] Fix structural inequivalence of operators

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

Looks good, thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D57902



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


[PATCH] D57915: [COFF, ARM64] Remove definitions for _byteswap library functions

2019-02-07 Thread Tom Tan via Phabricator via cfe-commits
TomTan added a comment.

In D57915#1389750 , @lebedev.ri wrote:

> In D57915#1389722 , @TomTan wrote:
>
> > In D57915#1389560 , @lebedev.ri 
> > wrote:
> >
> > > In D57915#1389549 , @TomTan 
> > > wrote:
> > >
> > > > Added the tests back. Clang IR should not lower these to bswap calls 
> > > > because they are global library functions. It might be slower to make 
> > > > the call to library function than bswap, but this is the same for other 
> > > > architectures supported by Windows. And just redefine global library 
> > > > function triggers link error in some scenarios.
> > >
> > >
> > > I think there is some deeper reasoning is being omitted here.
> > >  //Why// does the fact that those are "global library functions" bans 
> > > clang from lowering them into IR?
> > >  (and thus, "prevents" LLVM form doing optimizations)
> >
> >
> > The current issue could be caused by the implementation of comdat selection 
> > in LLD as below which provides more strict conflict check than MSVC link 
> > does.
> >
> > These `_bytewap_*` in `intrin.h` were not intended as optimization, 
> > instead, it is expected to be consistent with declarations in `stdlib.h`. 
> > For optimization, it makes sense to enable it for all architectures 
> > supported by Windows, and make sure it works with LLD.
> >
> > https://github.com/llvm-mirror/lld/blob/b6584c3fab115aa46dad27681af7eb3d4e5d2b35/COFF/InputFiles.cpp#L494
>
>
> These functions are also listed in 
> https://github.com/llvm-mirror/clang/blob/c18e7e9007970a3105617f03bc9d1de89fa1a3e1/include/clang/Basic/Builtins.def#L773-L775
>  Are you sure they shouldn't be simply dropped from `lib/Headers/intrin.h`?
>
> I notice they were just added in D56685 , 
> but that review has no commit message, so the problem it addressed is not 
> documented..
>  So as-is this all looks a bit like hand-waving //to me//.


The list in 
https://github.com/llvm-mirror/clang/blob/c18e7e9007970a3105617f03bc9d1de89fa1a3e1/include/clang/Basic/Builtins.def#L773-L775
 doesn't require any implementation on Clang side,it provides below warning if 
they are not declared (include `intrin.h` or `stdlib.h`) before using. So it 
would not be affected.

test.cpp(3,12):  warning: implicitly declaring library function 
'_byteswap_ushort' with type 'unsigned short (unsigned short)' 
[-Wimplicit-function-declaration]

  return _byteswap_ushort(42);
 ^

test.cpp(3,12):  note: include the header  or explicitly provide a 
declaration for '_byteswap_ushort'


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

https://reviews.llvm.org/D57915



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


[PATCH] D57915: [COFF, ARM64] Remove definitions for _byteswap library functions

2019-02-07 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

I did some quick testing with MSVC; apparently it inlines the implementations 
of these functions when optimizations are on.  We definitely want to support 
inlining these. Since these are commonly used in performance-sensitive code, 
I'd prefer to implement the required changes to 
CodeGenFunction::EmitMSVCBuiltinExpr now, rather than chase after weird 
performance regressions in the future.

---

I'm not sure how you could end up with a "duplicate symbols" error from the 
current implementation, though; these functions are marked "static", so they 
shouldn't conflict with the real _byteswap_* functions.


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

https://reviews.llvm.org/D57915



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


[PATCH] D57230: [analyzer] Toning down invalidation a bit

2019-02-07 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

That's the one:

  typedef __typeof(sizeof(int)) size_t;
  void *malloc(size_t);
  
  void escape(int **);
  
  struct S {
int *ptr;
  };
  
  void foo() {
struct S s1;
s1.ptr = malloc(sizeof(int));
escape();
  }

After the patch the allocated symbol no longer escapes. It didn't end up having 
much to do with destructors. I'll also think about it a bit more.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57230



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


[PATCH] D57915: [COFF, ARM64] Remove definitions for _byteswap library functions

2019-02-07 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In D57915#1389722 , @TomTan wrote:

> In D57915#1389560 , @lebedev.ri 
> wrote:
>
> > In D57915#1389549 , @TomTan wrote:
> >
> > > Added the tests back. Clang IR should not lower these to bswap calls 
> > > because they are global library functions. It might be slower to make the 
> > > call to library function than bswap, but this is the same for other 
> > > architectures supported by Windows. And just redefine global library 
> > > function triggers link error in some scenarios.
> >
> >
> > I think there is some deeper reasoning is being omitted here.
> >  //Why// does the fact that those are "global library functions" bans clang 
> > from lowering them into IR?
> >  (and thus, "prevents" LLVM form doing optimizations)
>
>
> The current issue could be caused by the implementation of comdat selection 
> in LLD as below which provides more strict conflict check than MSVC link does.
>
> These `_bytewap_*` in `intrin.h` were not intended as optimization, instead, 
> it is expected to be consistent with declarations in `stdlib.h`. For 
> optimization, it makes sense to enable it for all architectures supported by 
> Windows, and make sure it works with LLD.
>
> https://github.com/llvm-mirror/lld/blob/b6584c3fab115aa46dad27681af7eb3d4e5d2b35/COFF/InputFiles.cpp#L494


These functions are also listed in 
https://github.com/llvm-mirror/clang/blob/c18e7e9007970a3105617f03bc9d1de89fa1a3e1/include/clang/Basic/Builtins.def#L773-L775
Are you sure they shouldn't be simply dropped from `lib/Headers/intrin.h`?

I notice they were just added in D56685 , but 
that review has no commit message, so the problem it addressed is not 
documented..
So as-is this all looks a bit like hand-waving //to me//.


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

https://reviews.llvm.org/D57915



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


r353479 - bpf: teach BPF driver about the new CPU "v3"

2019-02-07 Thread Jiong Wang via cfe-commits
Author: jiwang
Date: Thu Feb  7 14:51:56 2019
New Revision: 353479

URL: http://llvm.org/viewvc/llvm-project?rev=353479=rev
Log:
bpf: teach BPF driver about the new CPU "v3"

This patch simply teach BPF driver about the new CPU "v3" introduced in
LLVM backend.

Acked-by: Yonghong Song 
Signed-off-by: Jiong Wang 


Modified:
cfe/trunk/lib/Basic/Targets/BPF.cpp
cfe/trunk/test/Misc/target-invalid-cpu-note.c

Modified: cfe/trunk/lib/Basic/Targets/BPF.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/BPF.cpp?rev=353479=353478=353479=diff
==
--- cfe/trunk/lib/Basic/Targets/BPF.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/BPF.cpp Thu Feb  7 14:51:56 2019
@@ -25,7 +25,7 @@ void BPFTargetInfo::getTargetDefines(con
 }
 
 static constexpr llvm::StringLiteral ValidCPUNames[] = {"generic", "v1", "v2",
-"probe"};
+"v3", "probe"};
 
 bool BPFTargetInfo::isValidCPUName(StringRef Name) const {
   return llvm::find(ValidCPUNames, Name) != std::end(ValidCPUNames);

Modified: cfe/trunk/test/Misc/target-invalid-cpu-note.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/target-invalid-cpu-note.c?rev=353479=353478=353479=diff
==
--- cfe/trunk/test/Misc/target-invalid-cpu-note.c (original)
+++ cfe/trunk/test/Misc/target-invalid-cpu-note.c Thu Feb  7 14:51:56 2019
@@ -101,7 +101,7 @@
 
 // RUN: not %clang_cc1 -triple bpf--- -target-cpu not-a-cpu -fsyntax-only %s 
2>&1 | FileCheck %s --check-prefix BPF
 // BPF: error: unknown target CPU 'not-a-cpu'
-// BPF: note: valid target CPU values are: generic, v1, v2, probe
+// BPF: note: valid target CPU values are: generic, v1, v2, v3, probe
 
 // RUN: not %clang_cc1 -triple avr--- -target-cpu not-a-cpu -fsyntax-only %s 
2>&1 | FileCheck %s --check-prefix AVR
 // AVR: error: unknown target CPU 'not-a-cpu'


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


[PATCH] D57915: [COFF, ARM64] Remove definitions for _byteswap library functions

2019-02-07 Thread Tom Tan via Phabricator via cfe-commits
TomTan added a comment.

In D57915#1389560 , @lebedev.ri wrote:

> In D57915#1389549 , @TomTan wrote:
>
> > Added the tests back. Clang IR should not lower these to bswap calls 
> > because they are global library functions. It might be slower to make the 
> > call to library function than bswap, but this is the same for other 
> > architectures supported by Windows. And just redefine global library 
> > function triggers link error in some scenarios.
>
>
> I think there is some deeper reasoning is being omitted here.
>  //Why// does the fact that those are "global library functions" bans clang 
> from lowering them into IR?
>  (and thus, "prevents" LLVM form doing optimizations)


The current issue could be caused by the implementation of comdat selection in 
LLD as below which provides more strict conflict check than MSVC link does.

These `_bytewap_*` in `intrin.h` were not intended as optimization, instead, it 
is expected to be consistent with declarations in `stdlib.h`. For optimization, 
it makes sense to enable it for all architectures supported by Windows, and 
make sure it works with LLD.

https://github.com/llvm-mirror/lld/blob/b6584c3fab115aa46dad27681af7eb3d4e5d2b35/COFF/InputFiles.cpp#L494


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

https://reviews.llvm.org/D57915



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


[PATCH] D57896: Variable names rule

2019-02-07 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment.

I am generally in favour of this direction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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


[PATCH] D57855: [analyzer] Reimplement checker options

2019-02-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a reviewer: aaron.ballman.
Szelethus added a comment.

Aaron, you recently changed these file too, if you have the time, I'll happily 
listen to your feedback :)

Also, I was thinking that maybe we could just `std::move` 
`CheckerRegisrty::Checkers` and `CheckerRegisrty::Packages` to 
`AnalyzerOptions` in `CheckerRegisrty`'s destructor, which will finally enable 
us to access all registered checkers, and a bunch of other things (like whether 
they are enabled, what options they have, etc).


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

https://reviews.llvm.org/D57855



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


[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

2019-02-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from a testing nit.




Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:231
+bool ArgumentCommentCheck::shouldAddComment(const Expr *Arg) const {
+  return (CommentBoolLiterals && isa(Arg)) ||
+ (CommentIntegerLiterals && isa(Arg)) ||

MyDeveloperDay wrote:
> aaron.ballman wrote:
> > I think we may want to do some additional work here. Consider:
> > ```
> > #define FOO 1
> > 
> > void g(int);
> > void h(double);
> > void i(const char *);
> > 
> > void f() {
> >   g(FOO); // Probably don't want to suggest comments here
> >   g((1)); // Probably do want to suggest comments here
> >   h(1.0f); // Probably do want to suggest comments here
> >   i(__FILE__); // Probably don't want to suggest comments here
> > }
> > ```
> > I think this code should do two things: 1) check whether the location of 
> > the arg is a macro expansion (if so, return false), 2) do `Arg = 
> > Arg->IgnoreParenImpCasts();` at the start of the function and drop the 
> > `Arg->IgnoreImpCasts()` for the string and pointer literal cases. However, 
> > that may be a bridge too far, so you should add a test case like this:
> > ```
> > g(_Generic(0, int : 0)); // Definitely do not want to see comments here
> > ```
> > If it turns out the above case gives the comment suggestions, then I'd go 
> > with `IgnoreImpCasts()` rather than `IgnoreParenImpCasts()`.
> I agree, tried all combinations but g((1); and g(_Generic(0, int : 0)); would 
> otherwise get comments
Yeah, I kind of figured that would be the case. Thank you for checking!



Comment at: test/clang-tidy/bugprone-argument-comment-literals.cpp:104
+  g(FOO);
+  g((1));
+  h(1.0f);

Can you add a FIXME comment that says we'd like to handle this case at some 
point? Maybe move the test closer to the `_Generic` example so you can mention 
that case as being a problem when handling paren expressions.


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

https://reviews.llvm.org/D57674



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


[PATCH] D57855: [analyzer] Reimplement checker options

2019-02-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus updated this revision to Diff 185861.
Szelethus edited the summary of this revision.
Szelethus added a comment.

Moved all `AnalyzerOptions` related changes to a separate revision.


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

https://reviews.llvm.org/D57855

Files:
  include/clang/Basic/DiagnosticCommonKinds.td
  include/clang/StaticAnalyzer/Checkers/CheckerBase.td
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  lib/Frontend/CompilerInvocation.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  test/Analysis/disable-all-checks.c
  test/Analysis/invalid-checker-option.c
  utils/TableGen/ClangSACheckersEmitter.cpp

Index: utils/TableGen/ClangSACheckersEmitter.cpp
===
--- utils/TableGen/ClangSACheckersEmitter.cpp
+++ utils/TableGen/ClangSACheckersEmitter.cpp
@@ -90,6 +90,23 @@
   .str();
 }
 
+static std::string getCheckerOptionType(const Record ) {
+  if (BitsInit *BI = R.getValueAsBitsInit("Type")) {
+switch(getValueFromBitsInit(BI, R)) {
+  case 0:
+return "int";
+  case 1:
+return "string";
+  case 2:
+return "bool";
+}
+  }
+  PrintFatalError(R.getLoc(),
+  "unable to parse command line option type for "
+  + getCheckerFullName());
+  return "";
+}
+
 static void printChecker(llvm::raw_ostream , const Record ) {
 OS << "CHECKER(" << "\"";
 OS.write_escaped(getCheckerFullName()) << "\", ";
@@ -134,6 +151,45 @@
   OS << "#endif // GET_PACKAGES\n"
 "\n";
 
+  // Emit a package option.
+  //
+  // PACKAGE_OPTION(OPTIONTYPE, PACKAGENAME, OPTIONNAME, DESCRIPTION, DEFAULT)
+  //   - OPTIONTYPE: Type of the option, whether it's integer or boolean etc.
+  // This is important for validating user input. Note that
+  // it's a string, rather than an actual type: since we can
+  // load checkers runtime, we can't use template hackery for
+  // sorting this out compile-time.
+  //   - PACKAGENAME: Name of the package.
+  //   - OPTIONNAME: Name of the option.
+  //   - DESCRIPTION
+  //   - DEFAULT: The default value for this option.
+  //
+  // The full option can be specified in the command like like this:
+  //   -analyzer-config PACKAGENAME:OPTIONNAME=VALUE
+  OS << "\n"
+"#ifdef GET_PACKAGE_OPTIONS\n";
+  for (const Record *package : packages) {
+
+if (package->isValueUnset("PackageOptions"))
+  continue;
+
+std::vector PackageOptions = package
+   ->getValueAsListOfDefs("PackageOptions");
+for (Record *PackageOpt : PackageOptions) {
+  OS << "PACKAGE_OPTION(\"";
+  OS.write_escaped(getCheckerOptionType(*PackageOpt)) << "\", \"";
+  OS.write_escaped(getPackageFullName(package)) << "\", ";
+  OS << '\"' << getStringValue(*PackageOpt, "CmdFlag") << "\", ";
+  OS << '\"';
+  OS.write_escaped(getStringValue(*PackageOpt, "Desc")) << "\", ";
+  OS << '\"';
+  OS.write_escaped(getStringValue(*PackageOpt, "DefaultVal")) << "\"";
+  OS << ")\n";
+}
+  }
+  OS << "#endif // GET_PACKAGE_OPTIONS\n"
+"\n";
+
   // Emit checkers.
   //
   // CHECKER(FULLNAME, CLASS, HELPTEXT)
@@ -176,5 +232,46 @@
   }
   OS << "\n"
 "#endif // GET_CHECKER_DEPENDENCIES\n";
+
+  // Emit a package option.
+  //
+  // CHECKER_OPTION(OPTIONTYPE, CHECKERNAME, OPTIONNAME, DESCRIPTION, DEFAULT)
+  //   - OPTIONTYPE: Type of the option, whether it's integer or boolean etc.
+  // This is important for validating user input. Note that
+  // it's a string, rather than an actual type: since we can
+  // load checkers runtime, we can't use template hackery for
+  // sorting this out compile-time.
+  //   - CHECKERNAME: Name of the package.
+  //   - OPTIONNAME: Name of the option.
+  //   - DESCRIPTION
+  //   - DEFAULT: The default value for this option.
+  //
+  // The full option can be specified in the command like like this:
+  //   -analyzer-config CHECKERNAME:OPTIONNAME=VALUE
+  OS << "\n"
+"#ifdef GET_CHECKER_OPTIONS\n";
+  for (const Record *checker : checkers) {
+
+if (checker->isValueUnset("CheckerOptions"))
+  continue;
+
+
+std::vector CheckerOptions = checker
+   ->getValueAsListOfDefs("CheckerOptions");
+for (Record *CheckerOpt : CheckerOptions) {
+  OS << "CHECKER_OPTION(\"";
+  OS << getCheckerOptionType(*CheckerOpt) << "\", \"";
+  OS.write_escaped(getCheckerFullName(checker)) << "\", ";
+  OS << '\"' << getStringValue(*CheckerOpt, "CmdFlag") << "\", ";
+  OS << '\"';
+  OS.write_escaped(getStringValue(*CheckerOpt, "Desc")) << "\", ";
+  OS << '\"';
+  OS.write_escaped(getStringValue(*CheckerOpt, 

[PATCH] D57922: [analyzer] Insert checker options into AnalyzerOption::ConfigTable

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



Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:183
   SearchInParents)
- .getAsInteger(10, Ret);
+ .getAsInteger(0, Ret);
   assert(!HasFailed && "analyzer-config option should be numeric");

`alpha.security.MmapWriteExec:MmapProtRead` expects it's option to be of a 
hexadecimal value -- as of now, using that option crashes.

From the [[ 
https://llvm.org/doxygen/classllvm_1_1StringRef.html#a9e373829f3f1775d30eae9053067f8c3
 | doxygen]]:
```
llvm::StringRef::getAsInteger(unsigned Radix, T & Result) const
```
Parse the current string as an integer of the specified radix.

If Radix is specified as zero, this does radix autosensing using extended C 
rules: 0 is octal, 0x is hex, 0b is binary.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57922



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


[PATCH] D57923: [Format/ObjC] Fix [foo bar]->baz formatting as lambda arrow

2019-02-07 Thread Ben Hamilton via Phabricator via cfe-commits
benhamilton updated this revision to Diff 185859.
benhamilton added a comment.

Tidy up code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57923

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTestObjC.cpp


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -611,6 +611,7 @@
 
 TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
   verifyFormat("[foo bar:baz];");
+  verifyFormat("[foo bar]->baz;");
   verifyFormat("return [foo bar:baz];");
   verifyFormat("return (a)[foo bar:baz];");
   verifyFormat("f([foo bar:baz]);");
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1426,6 +1426,9 @@
   nextToken();
   break;
 case tok::arrow:
+  // This might or might not actually be a lambda arrow (this could be an
+  // ObjC method invocation followed by a dereferencing arrow). We might
+  // reset this back to TT_Unknown in TokenAnnotator.
   FormatTok->Type = TT_LambdaArrow;
   nextToken();
   break;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -520,6 +520,10 @@
   if (Parent && Parent->is(TT_PointerOrReference))
 Parent->Type = TT_BinaryOperator;
 }
+// An arrow after an ObjC method expression is not a lambda arrow.
+if (CurrentToken->Type == TT_ObjCMethodExpr && CurrentToken->Next &&
+CurrentToken->Next->is(TT_LambdaArrow))
+  CurrentToken->Next->Type = TT_Unknown;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
 // FirstObjCSelectorName is set when a colon is found. This does


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -611,6 +611,7 @@
 
 TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
   verifyFormat("[foo bar:baz];");
+  verifyFormat("[foo bar]->baz;");
   verifyFormat("return [foo bar:baz];");
   verifyFormat("return (a)[foo bar:baz];");
   verifyFormat("f([foo bar:baz]);");
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1426,6 +1426,9 @@
   nextToken();
   break;
 case tok::arrow:
+  // This might or might not actually be a lambda arrow (this could be an
+  // ObjC method invocation followed by a dereferencing arrow). We might
+  // reset this back to TT_Unknown in TokenAnnotator.
   FormatTok->Type = TT_LambdaArrow;
   nextToken();
   break;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -520,6 +520,10 @@
   if (Parent && Parent->is(TT_PointerOrReference))
 Parent->Type = TT_BinaryOperator;
 }
+// An arrow after an ObjC method expression is not a lambda arrow.
+if (CurrentToken->Type == TT_ObjCMethodExpr && CurrentToken->Next &&
+CurrentToken->Next->is(TT_LambdaArrow))
+  CurrentToken->Next->Type = TT_Unknown;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
 // FirstObjCSelectorName is set when a colon is found. This does
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D57923: [Format/ObjC] Fix [foo bar]->baz formatting as lambda arrow

2019-02-07 Thread Ben Hamilton via Phabricator via cfe-commits
benhamilton created this revision.
benhamilton added reviewers: krasimir, djasper.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently, `UnwrappedLineParser` thinks an arrow token after
an ObjC method expression is a C++ lambda arrow, so it formats:

  [foo bar]->baz

as:

  [foo bar] -> baz

Because `UnwrappedLineParser` runs before `TokenAnnotator`, it can't
know if the arrow token is after an ObjC method expression or not.

This diff makes `TokenAnnotator` remove the TT_LambdaArrow on
the arrow token if it follows an ObjC method expression.

Test Plan: New test added. Ran test with:

  % ninja FormatTests && ./tools/clang/unittests/Format/FormatTests
  Confirmed test failed before diff and passed after diff.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57923

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTestObjC.cpp


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -611,6 +611,7 @@
 
 TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
   verifyFormat("[foo bar:baz];");
+  verifyFormat("[foo bar]->baz;");
   verifyFormat("return [foo bar:baz];");
   verifyFormat("return (a)[foo bar:baz];");
   verifyFormat("f([foo bar:baz]);");
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1426,6 +1426,9 @@
   nextToken();
   break;
 case tok::arrow:
+  // This might or might not actually be a lambda arrow (this could be an
+  // ObjC method invocation followed by a dereferencing arrow). We might
+  // reset this back to TT_Unknown in TokenAnnotator.
   FormatTok->Type = TT_LambdaArrow;
   nextToken();
   break;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -520,6 +520,11 @@
   if (Parent && Parent->is(TT_PointerOrReference))
 Parent->Type = TT_BinaryOperator;
 }
+// An arrow after an ObjC method expression is not a lambda arrow.
+if (CurrentToken->Type == TT_ObjCMethodExpr) {
+  if (CurrentToken->Next && CurrentToken->Next->is(TT_LambdaArrow))
+CurrentToken->Next->Type = TT_Unknown;
+}
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
 // FirstObjCSelectorName is set when a colon is found. This does


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -611,6 +611,7 @@
 
 TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
   verifyFormat("[foo bar:baz];");
+  verifyFormat("[foo bar]->baz;");
   verifyFormat("return [foo bar:baz];");
   verifyFormat("return (a)[foo bar:baz];");
   verifyFormat("f([foo bar:baz]);");
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1426,6 +1426,9 @@
   nextToken();
   break;
 case tok::arrow:
+  // This might or might not actually be a lambda arrow (this could be an
+  // ObjC method invocation followed by a dereferencing arrow). We might
+  // reset this back to TT_Unknown in TokenAnnotator.
   FormatTok->Type = TT_LambdaArrow;
   nextToken();
   break;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -520,6 +520,11 @@
   if (Parent && Parent->is(TT_PointerOrReference))
 Parent->Type = TT_BinaryOperator;
 }
+// An arrow after an ObjC method expression is not a lambda arrow.
+if (CurrentToken->Type == TT_ObjCMethodExpr) {
+  if (CurrentToken->Next && CurrentToken->Next->is(TT_LambdaArrow))
+CurrentToken->Next->Type = TT_Unknown;
+}
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
 // FirstObjCSelectorName is set when a colon is found. This does
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D57922: [analyzer] Insert checker options into AnalyzerOption::ConfigTable

2019-02-07 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs, 
baloghadamsoftware.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity.
Herald added a project: clang.

The more entries we have in `AnalyzerOptions::ConfigTable`, the more helpful 
`debug.ConfigDumper` is. With this patch, I'm pretty confident that it'll now 
emit the entire state of the analyzer, minus the frontend flags.

It would be nice to reserve the config table specifically to checker options 
only, as storing the regular analyzer configs is kinda redundant.


Repository:
  rC Clang

https://reviews.llvm.org/D57922

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  test/Analysis/analyzer-config.c

Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -3,6 +3,16 @@
 
 // CHECK: [config]
 // CHECK-NEXT: aggressive-binary-operation-simplification = false
+// CHECK-NEXT: alpha.clone.CloneChecker:IgnoredFilesPattern = ""
+// CHECK-NEXT: alpha.clone.CloneChecker:MinimumCloneComplexity = 50
+// CHECK-NEXT: alpha.clone.CloneChecker:ReportNormalClones = true
+// CHECK-NEXT: alpha.cplusplus.UninitializedObject:CheckPointeeInitialization = false
+// CHECK-NEXT: alpha.cplusplus.UninitializedObject:IgnoreGuardedFields = false
+// CHECK-NEXT: alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField = ""
+// CHECK-NEXT: alpha.cplusplus.UninitializedObject:NotesAsWarnings = false
+// CHECK-NEXT: alpha.cplusplus.UninitializedObject:Pedantic = false
+// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtExec = 0x04
+// CHECK-NEXT: alpha.security.MmapWriteExec:MmapProtRead = 0x01
 // CHECK-NEXT: avoid-suppressing-null-argument-paths = false
 // CHECK-NEXT: c++-allocator-inlining = true
 // CHECK-NEXT: c++-container-inlining = false
@@ -18,9 +28,26 @@
 // CHECK-NEXT: cfg-rich-constructors = true
 // CHECK-NEXT: cfg-scopes = false
 // CHECK-NEXT: cfg-temporary-dtors = true
+// CHECK-NEXT: cplusplus.Move:WarnOn = KnownsAndLocals
 // CHECK-NEXT: crosscheck-with-z3 = false
 // CHECK-NEXT: ctu-dir = ""
 // CHECK-NEXT: ctu-index-name = externalDefMap.txt
+// CHECK-NEXT: debug.AnalysisOrder:* = false
+// CHECK-NEXT: debug.AnalysisOrder:Bind = false
+// CHECK-NEXT: debug.AnalysisOrder:EndFunction = false
+// CHECK-NEXT: debug.AnalysisOrder:LiveSymbols = false
+// CHECK-NEXT: debug.AnalysisOrder:NewAllocator = false
+// CHECK-NEXT: debug.AnalysisOrder:PostCall = false
+// CHECK-NEXT: debug.AnalysisOrder:PostStmtArraySubscriptExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXNewExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PostStmtCastExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PostStmtOffsetOfExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PreCall = false
+// CHECK-NEXT: debug.AnalysisOrder:PreStmtArraySubscriptExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PreStmtCXXNewExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PreStmtCastExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:PreStmtOffsetOfExpr = false
+// CHECK-NEXT: debug.AnalysisOrder:RegionChanges = false
 // CHECK-NEXT: display-ctu-progress = false
 // CHECK-NEXT: eagerly-assume = true
 // CHECK-NEXT: elide-constructors = true
@@ -40,7 +67,16 @@
 // CHECK-NEXT: mode = deep
 // CHECK-NEXT: model-path = ""
 // CHECK-NEXT: notes-as-events = false
+// CHECK-NEXT: nullability:NoDiagnoseCallsToSystemHeaders = false
+// CHECK-NEXT: nullability:Optimistic = false
 // CHECK-NEXT: objc-inlining = true
+// CHECK-NEXT: optin.cplusplus.VirtualCall:PureOnly = false
+// CHECK-NEXT: optin.osx.cocoa.localizability.NonLocalizedStringChecker:AggressiveReport = false
+// CHECK-NEXT: optin.performance.Padding:AllowedPad = 24
+// CHECK-NEXT: osx.NumberObjectConversion:Pedantic = false
+// CHECK-NEXT: osx.cocoa.RetainCount:CheckOSObject = true
+// CHECK-NEXT: osx.cocoa.RetainCount:TrackNSCFStartParam = false
+// CHECK-NEXT: osx.cocoa.RetainCount:leak-diagnostics-reference-allocation = false
 // CHECK-NEXT: prune-paths = true
 // CHECK-NEXT: region-store-small-struct-limit = 2
 // CHECK-NEXT: report-in-main-source-file = false
@@ -49,7 +85,8 @@
 // CHECK-NEXT: suppress-c++-stdlib = true
 // CHECK-NEXT: suppress-inlined-defensive-checks = true
 // CHECK-NEXT: suppress-null-return-paths = true
+// CHECK-NEXT: unix.DynamicMemoryModeling:Optimistic = false
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 49
+// CHECK-NEXT: num-entries = 86
Index: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -329,6 +329,8 @@
 
   

[PATCH] D57915: [COFF, ARM64] Remove definitions for _byteswap library functions

2019-02-07 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In D57915#1389549 , @TomTan wrote:

> Added the tests back. Clang IR should not lower these to bswap calls because 
> they are global library functions. It might be slower to make the call to 
> library function than bswap, but this is the same for other architectures 
> supported by Windows. And just redefine global library function triggers link 
> error in some scenarios.


I think there is some deeper reasoning is being omitted here.
//Why// does the fact that those are "global library functions" bans clang from 
lowering them into IR?
(and thus, "prevents" LLVM form doing optimizations)


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

https://reviews.llvm.org/D57915



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


[PATCH] D57915: [COFF, ARM64] Remove definitions for _byteswap library functions

2019-02-07 Thread Tom Tan via Phabricator via cfe-commits
TomTan updated this revision to Diff 185849.
TomTan added a comment.

Added the tests back. Clang IR should not lower these to bswap calls because 
they are global library functions. It might be slower to make the call to 
library function than bswap, but this is the same for other architectures 
supported by Windows. And just redefine global library function triggers link 
error in some scenarios.


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

https://reviews.llvm.org/D57915

Files:
  lib/Headers/intrin.h
  test/Headers/ms-arm64-intrin.cpp


Index: test/Headers/ms-arm64-intrin.cpp
===
--- test/Headers/ms-arm64-intrin.cpp
+++ test/Headers/ms-arm64-intrin.cpp
@@ -14,16 +14,16 @@
 }
 
 unsigned short check_byteswap_ushort(unsigned short val) {
-// CHECK: call i16 @llvm.bswap.i16(i16 %val)
+// CHECK: call i16 @_byteswap_ushort(i16 %val)
   return _byteswap_ushort(val);
 }
 
 unsigned long check_byteswap_ulong(unsigned long val) {
-// CHECK: call i32 @llvm.bswap.i32(i32 %val)
+// CHECK: call i32 @_byteswap_ulong(i32 %val)
   return _byteswap_ulong(val);
 }
 
 unsigned __int64 check_byteswap_uint64(unsigned __int64 val) {
-// CHECK: call i64 @llvm.bswap.i64(i64 %val)
+// CHECK: call i64 @_byteswap_uint64(i64 %val)
   return _byteswap_uint64(val);
 }
Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -557,15 +557,9 @@
 int _ReadStatusReg(int);
 void _WriteStatusReg(int, int);
 
-static inline unsigned short _byteswap_ushort (unsigned short val) {
-  return __builtin_bswap16(val);
-}
-static inline unsigned long _byteswap_ulong (unsigned long val) {
-  return __builtin_bswap32(val);
-}
-static inline unsigned __int64 _byteswap_uint64 (unsigned __int64 val) {
-  return __builtin_bswap64(val);
-}
+unsigned short __cdecl _byteswap_ushort(unsigned short val);
+unsigned long __cdecl _byteswap_ulong (unsigned long val);
+unsigned __int64 __cdecl _byteswap_uint64(unsigned __int64 val);
 #endif
 
 
/**\


Index: test/Headers/ms-arm64-intrin.cpp
===
--- test/Headers/ms-arm64-intrin.cpp
+++ test/Headers/ms-arm64-intrin.cpp
@@ -14,16 +14,16 @@
 }
 
 unsigned short check_byteswap_ushort(unsigned short val) {
-// CHECK: call i16 @llvm.bswap.i16(i16 %val)
+// CHECK: call i16 @_byteswap_ushort(i16 %val)
   return _byteswap_ushort(val);
 }
 
 unsigned long check_byteswap_ulong(unsigned long val) {
-// CHECK: call i32 @llvm.bswap.i32(i32 %val)
+// CHECK: call i32 @_byteswap_ulong(i32 %val)
   return _byteswap_ulong(val);
 }
 
 unsigned __int64 check_byteswap_uint64(unsigned __int64 val) {
-// CHECK: call i64 @llvm.bswap.i64(i64 %val)
+// CHECK: call i64 @_byteswap_uint64(i64 %val)
   return _byteswap_uint64(val);
 }
Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -557,15 +557,9 @@
 int _ReadStatusReg(int);
 void _WriteStatusReg(int, int);
 
-static inline unsigned short _byteswap_ushort (unsigned short val) {
-  return __builtin_bswap16(val);
-}
-static inline unsigned long _byteswap_ulong (unsigned long val) {
-  return __builtin_bswap32(val);
-}
-static inline unsigned __int64 _byteswap_uint64 (unsigned __int64 val) {
-  return __builtin_bswap64(val);
-}
+unsigned short __cdecl _byteswap_ushort(unsigned short val);
+unsigned long __cdecl _byteswap_ulong (unsigned long val);
+unsigned __int64 __cdecl _byteswap_uint64(unsigned __int64 val);
 #endif
 
 /**\
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

2019-02-07 Thread Ronald Wampler via Phabricator via cfe-commits
rdwampler updated this revision to Diff 185848.
rdwampler added a comment.

Update to include an option `Inline` to only put short lambda on a single line 
if used as an argument. See the following code style guide from catboost: 
ps://github.com/catboost/catboost/blob/master/CPP_STYLE_GUIDE.md#lambda-functions

I'm not sure if `Inline` is the best name. I welcome suggestions.

Thanks!


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

https://reviews.llvm.org/D57687

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11954,6 +11954,43 @@
   "> {\n"
   "  //\n"
   "});");
+
+  FormatStyle DoNotMerge = getLLVMStyle();
+  DoNotMerge.AllowShortLambdasOnASingleLine = FormatStyle::SLS_None;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() { return b; };", DoNotMerge);
+  verifyFormat("auto c = []() {\n"
+   "};",
+   " auto c = []() {};", DoNotMerge);
+
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Empty;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() {\n"
+   "  return b;\n"
+   " };",
+   MergeEmptyOnly);
+  verifyFormat("auto c = []() {};",
+   "auto c = []() {\n"
+   "};",
+   MergeEmptyOnly);
+
+  FormatStyle MergeInline = getLLVMStyle();
+  MergeInline.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Inline;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() { return b; };", MergeInline);
+  verifyFormat("function([]() { return b; })", "function([]() { return b; })",
+   MergeInline);
+  verifyFormat("function([]() { return b; }, a)",
+   "function([]() { return b; }, a)", MergeInline);
+  verifyFormat("function(a, []() { return b; })",
+   "function(a, []() { return b; })", MergeInline);
 }
 
 TEST_F(FormatTest, EmptyLinesInLambdas) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1433,6 +1433,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -365,7 +365,7 @@
   // specifier parameter, although this is technically valid:
   // [[foo(:)]]
   if (AttrTok->is(tok::colon) ||
-  AttrTok->startsSequence(tok::identifier, tok::identifier) || 
+  AttrTok->startsSequence(tok::identifier, tok::identifier) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))
 return false;
   if (AttrTok->is(tok::ellipsis))
@@ -1138,11 +1138,11 @@
 
 // Reset token type in case we have already looked at it and then
 // recovered from an error (e.g. failure to find the matching >).
-if (!CurrentToken->isOneOf(TT_LambdaLSquare, TT_ForEachMacro,
-   TT_FunctionLBrace, TT_ImplicitStringLiteral,
-   TT_InlineASMBrace, TT_JsFatArrow, TT_LambdaArrow,
-   TT_OverloadedOperator, TT_RegexLiteral,
-   TT_TemplateString, TT_ObjCStringLiteral))
+if (!CurrentToken->isOneOf(
+TT_LambdaLSquare, TT_LambdaLBrace, TT_ForEachMacro,
+TT_FunctionLBrace, TT_ImplicitStringLiteral, TT_InlineASMBrace,
+TT_JsFatArrow, TT_LambdaArrow, TT_OverloadedOperator,
+TT_RegexLiteral, TT_TemplateString, TT_ObjCStringLiteral))
   CurrentToken->Type = TT_Unknown;
 CurrentToken->Role.reset();
 CurrentToken->MatchingParen = nullptr;
@@ -2835,7 +2835,7 @@
 // Returns 'true' if 'Tok' is a brace we'd want to break before in Allman style.
 static bool isAllmanBrace(const FormatToken ) {
   return Tok.is(tok::l_brace) && Tok.BlockKind == BK_Block &&
- !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral);
+ !Tok.isOneOf(TT_ObjCBlockLBrace, TT_LambdaLBrace, TT_DictLiteral);
 }
 
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine ,
@@ -2963,6 +2963,18 @@
  

[PATCH] D57553: [Fixed Point Arithmetic] Avoid resizing for types with the same width

2019-02-07 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan abandoned this revision.
leonardchan marked an inline comment as done.
leonardchan added a comment.

In D57553#1388493 , @ebevhan wrote:

> In D57553#1381920 , @leonardchan 
> wrote:
>
> > In regards to solving the problem of resizing for int conversions, I'm 
> > starting to think that we will need that initial resize since if we want to 
> > retain the min-max pattern for all conversions, then it would be necessary 
> > to extend and shift when converting from an int to fixed point. Otherwise, 
> > I think we'd have the initial pattern I proposed where we check against the 
> > source value, but not have this pattern.
>
>
> Yes, I'm starting to think so too. It's just not possible to not resize and 
> keep the minmax pattern at the same time. We can't upshift without resizing 
> first (because any bits above the scale can affect saturation), and if we 
> wanted to saturate purely on the non-rescaled value, then the (mandatory) 
> rescaling after saturation could destroy the possibly saturated result (since 
> it would shift in zeroes from the right if upscaling, which breaks the result 
> if it saturated to max).
>
> Sorry for going down this path, that was rather pointless.


No problem. Abandoning this patch.




Comment at: clang/test/Frontend/fixed_point_conversions.c:194
+  // DEFAULT-NEXT: [[RESULT2:%[0-9a-z]+]] = select i1 [[USE_MIN]], i32 
-2147483648, i32 [[RESULT]]
+  // DEFAULT-NEXT: store i32 [[RESULT2]], i32* %sat_lf, align 4
 

ebevhan wrote:
> This seems off. We're upshifting, then saturating on the upshifted value. But 
> the top bits could have been shifted out, so the result might not be correct.
> 
> We can't upshift before saturating if the upshift could shift out bits.
> 
> If the upshift is moved after the saturation, then the saturation no longer 
> works since it would be upshifting zeros into the saturation result, which is 
> also wrong.
Ah you're right. I completely missed this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57553



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


[PATCH] D55659: [Sema][ObjC] Disallow non-trivial C struct fields in unions

2019-02-07 Thread Akira Hatanaka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
ahatanak marked 2 inline comments as done.
Closed by commit rL353459: [Sema][ObjC] Disallow non-trivial C struct fields in 
unions. (authored by ahatanak, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55659?vs=185466=185846#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55659

Files:
  cfe/trunk/include/clang/AST/Type.h
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/AST/Type.cpp
  cfe/trunk/lib/Sema/SemaDecl.cpp
  cfe/trunk/test/SemaObjC/arc-decls.m

Index: cfe/trunk/lib/AST/Type.cpp
===
--- cfe/trunk/lib/AST/Type.cpp
+++ cfe/trunk/lib/AST/Type.cpp
@@ -22,6 +22,7 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/NonTrivialTypeVisitor.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/TemplateName.h"
@@ -2244,6 +2245,62 @@
  getObjCLifetime() != Qualifiers::OCL_Weak;
 }
 
+namespace {
+// Helper class that determines whether this is a type that is non-trivial to
+// primitive copy or move, or is a struct type that has a field of such type.
+template 
+struct IsNonTrivialCopyMoveVisitor
+: CopiedTypeVisitor, IsMove, bool> {
+  using Super =
+  CopiedTypeVisitor, IsMove, bool>;
+  IsNonTrivialCopyMoveVisitor(const ASTContext ) : Ctx(C) {}
+  void preVisit(QualType::PrimitiveCopyKind PCK, QualType QT) {}
+
+  bool visitWithKind(QualType::PrimitiveCopyKind PCK, QualType QT) {
+if (const auto *AT = this->Ctx.getAsArrayType(QT))
+  return this->asDerived().visit(QualType(AT, 0));
+return Super::visitWithKind(PCK, QT);
+  }
+
+  bool visitARCStrong(QualType QT) { return true; }
+  bool visitARCWeak(QualType QT) { return true; }
+  bool visitTrivial(QualType QT) { return false; }
+  // Volatile fields are considered trivial.
+  bool visitVolatileTrivial(QualType QT) { return false; }
+
+  bool visitStruct(QualType QT) {
+const RecordDecl *RD = QT->castAs()->getDecl();
+// We don't want to apply the C restriction in C++ because C++
+// (1) can apply the restriction at a finer grain by banning copying or
+// destroying the union, and
+// (2) allows users to override these restrictions by declaring explicit
+// constructors/etc, which we're not proposing to add to C.
+if (isa(RD))
+  return false;
+for (const FieldDecl *FD : RD->fields())
+  if (this->asDerived().visit(FD->getType()))
+return true;
+return false;
+  }
+
+  const ASTContext 
+};
+
+} // namespace
+
+bool QualType::isNonTrivialPrimitiveCType(const ASTContext ) const {
+  if (isNonTrivialToPrimitiveDefaultInitialize())
+return true;
+  DestructionKind DK = isDestructedType();
+  if (DK != DK_none && DK != DK_cxx_destructor)
+return true;
+  if (IsNonTrivialCopyMoveVisitor(Ctx).visit(*this))
+return true;
+  if (IsNonTrivialCopyMoveVisitor(Ctx).visit(*this))
+return true;
+  return false;
+}
+
 QualType::PrimitiveDefaultInitializeKind
 QualType::isNonTrivialToPrimitiveDefaultInitialize() const {
   if (const auto *RT =
Index: cfe/trunk/lib/Sema/SemaDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaDecl.cpp
+++ cfe/trunk/lib/Sema/SemaDecl.cpp
@@ -15916,6 +15916,10 @@
 Record->setHasObjectMember(true);
   if (Record && FDTTy->getDecl()->hasVolatileMember())
 Record->setHasVolatileMember(true);
+  if (Record && Record->isUnion() &&
+  FD->getType().isNonTrivialPrimitiveCType(Context))
+Diag(FD->getLocation(),
+ diag::err_nontrivial_primitive_type_in_union);
 } else if (FDTy->isObjCObjectType()) {
   /// A field cannot be an Objective-c object
   Diag(FD->getLocation(), diag::err_statically_allocated_object)
Index: cfe/trunk/include/clang/AST/Type.h
===
--- cfe/trunk/include/clang/AST/Type.h
+++ cfe/trunk/include/clang/AST/Type.h
@@ -1121,6 +1121,12 @@
   };
 
   /// Check if this is a non-trivial type that would cause a C struct
+  /// transitively containing this type to be non-trivial. This function can be
+  /// used to determine whether a field of this type can be declared inside a C
+  /// union.
+  bool isNonTrivialPrimitiveCType(const ASTContext ) const;
+
+  /// Check if this is a non-trivial type that would cause a C struct
   /// transitively containing this type to be non-trivial to copy and return the
   /// kind.
   PrimitiveCopyKind isNonTrivialToPrimitiveCopy() const;
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- 

r353459 - [Sema][ObjC] Disallow non-trivial C struct fields in unions.

2019-02-07 Thread Akira Hatanaka via cfe-commits
Author: ahatanak
Date: Thu Feb  7 12:21:46 2019
New Revision: 353459

URL: http://llvm.org/viewvc/llvm-project?rev=353459=rev
Log:
[Sema][ObjC] Disallow non-trivial C struct fields in unions.

This patch fixes a bug where clang doesn’t reject union fields of
non-trivial C struct types. For example:

```
// This struct is non-trivial under ARC.
struct S0 {
  id x;
};

union U0 {
  struct S0 s0; // clang should reject this.
  struct S0 s1; // clang should reject this.
};

void test(union U0 a) {
  // Previously, both 'a.s0.x' and 'a.s1.x' were released in this
  // function.
}
```

rdar://problem/46677858

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

Modified:
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/SemaObjC/arc-decls.m

Modified: cfe/trunk/include/clang/AST/Type.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=353459=353458=353459=diff
==
--- cfe/trunk/include/clang/AST/Type.h (original)
+++ cfe/trunk/include/clang/AST/Type.h Thu Feb  7 12:21:46 2019
@@ -1121,6 +1121,12 @@ public:
   };
 
   /// Check if this is a non-trivial type that would cause a C struct
+  /// transitively containing this type to be non-trivial. This function can be
+  /// used to determine whether a field of this type can be declared inside a C
+  /// union.
+  bool isNonTrivialPrimitiveCType(const ASTContext ) const;
+
+  /// Check if this is a non-trivial type that would cause a C struct
   /// transitively containing this type to be non-trivial to copy and return 
the
   /// kind.
   PrimitiveCopyKind isNonTrivialToPrimitiveCopy() const;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=353459=353458=353459=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Feb  7 12:21:46 
2019
@@ -609,6 +609,8 @@ def warn_cstruct_memaccess : Warning<
   InGroup;
 def note_nontrivial_field : Note<
   "field is non-trivial to %select{copy|default-initialize}0">;
+def err_nontrivial_primitive_type_in_union : Error<
+  "non-trivial C types are disallowed in union">;
 def warn_dyn_class_memaccess : Warning<
   "%select{destination for|source of|first operand of|second operand of}0 this 
"
   "%1 call is a pointer to %select{|class containing a }2dynamic class %3; "

Modified: cfe/trunk/lib/AST/Type.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=353459=353458=353459=diff
==
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Thu Feb  7 12:21:46 2019
@@ -22,6 +22,7 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/NestedNameSpecifier.h"
+#include "clang/AST/NonTrivialTypeVisitor.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/TemplateName.h"
@@ -2244,6 +2245,62 @@ bool QualType::isNonWeakInMRRWithObjCWea
  getObjCLifetime() != Qualifiers::OCL_Weak;
 }
 
+namespace {
+// Helper class that determines whether this is a type that is non-trivial to
+// primitive copy or move, or is a struct type that has a field of such type.
+template 
+struct IsNonTrivialCopyMoveVisitor
+: CopiedTypeVisitor, IsMove, bool> {
+  using Super =
+  CopiedTypeVisitor, IsMove, bool>;
+  IsNonTrivialCopyMoveVisitor(const ASTContext ) : Ctx(C) {}
+  void preVisit(QualType::PrimitiveCopyKind PCK, QualType QT) {}
+
+  bool visitWithKind(QualType::PrimitiveCopyKind PCK, QualType QT) {
+if (const auto *AT = this->Ctx.getAsArrayType(QT))
+  return this->asDerived().visit(QualType(AT, 0));
+return Super::visitWithKind(PCK, QT);
+  }
+
+  bool visitARCStrong(QualType QT) { return true; }
+  bool visitARCWeak(QualType QT) { return true; }
+  bool visitTrivial(QualType QT) { return false; }
+  // Volatile fields are considered trivial.
+  bool visitVolatileTrivial(QualType QT) { return false; }
+
+  bool visitStruct(QualType QT) {
+const RecordDecl *RD = QT->castAs()->getDecl();
+// We don't want to apply the C restriction in C++ because C++
+// (1) can apply the restriction at a finer grain by banning copying or
+// destroying the union, and
+// (2) allows users to override these restrictions by declaring explicit
+// constructors/etc, which we're not proposing to add to C.
+if (isa(RD))
+  return false;
+for (const FieldDecl *FD : RD->fields())
+  if (this->asDerived().visit(FD->getType()))
+return true;
+return false;
+  }
+
+  const ASTContext 
+};
+
+} // 

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-07 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments.



Comment at: lib/AST/Stmt.cpp:628
+  DiagOffs = CurPtr-StrStart-1;
+  return diag::err_asm_invalid_operand_for_goto_labels;
+}

jyu2 wrote:
> rsmith wrote:
> > jyu2 wrote:
> > > rsmith wrote:
> > > > I'm curious why we're checking this here, when all the other validation 
> > > > for the modifier letter happens in LLVM. Does this check need to be 
> > > > done differently from the others?
> > > This is to verify following in spec:
> > > =
> > > To reference a label in the assembler template, prefix it with ‘%l’ 
> > > (lowercase ‘L’) followed by its (zero-based) position in GotoLabels plus 
> > > the number of input operands. For example, if the asm has three inputs 
> > > and references two labels, refer to the first label as ‘%l3’ and the 
> > > second as ‘%l4’).
> > > 
> > > Was request from Eli.
> > Sorry, I think my question was unclear. I understand why you need to do 
> > this check somewhere. My question is why this check needs to be *here*, 
> > given that all the other checks for the modifier letter are performed 
> > elsewhere. Splitting the checks up between multiple locations makes the 
> > system harder to maintain.
> > 
> > As it happens, there's a FIXME for this here: 
> > http://llvm.org/doxygen/AsmPrinterInlineAsm_8cpp_source.html#l00436 -- and 
> > a test case like `void f(void *a) { asm("mov %l0, %%eax" :: "r"(a)); }` 
> > currently causes LLVM to assert in a function that's supposed to be 
> > validating these modifier letters and producing a clean error if they're 
> > bad. So I think that's an LLVM bug, and fixing that LLVM bug would enforce 
> > the property we need here (that `%lN` only names labels).
> > 
> > 
> > [It's not clear to me whether we should be enforcing that `%lN` only names 
> > label //operands//, which would require the check to be in Clang rather 
> > than LLVM; GCC doesn't do that, and for instance accepts
> > 
> > ```
> > void f() { asm goto("jmp %l0" :: "i"(&)::foo); foo:; }
> > ```
> > 
> > (at least for x86_64), though this construct doesn't seem hugely useful 
> > given that the input operand would need to be a literal address-of-label 
> > expression and you'd need to name the target label as a label operand 
> > anyway.]
> Hi Richard,
> 
> Thanks for the detail explanation. 
> I agree with you.  Our compiler FE don't emit error for this also.
> 
> I'd happy to remove this, If we all agree.  
> 
> Hi Eli, 
> what do you think?
> 
> Thanks.
> Jennifer  
> 
Yes, that's okay.



Comment at: lib/Analysis/CFG.cpp:3137
+
+  Block = createBlock(false);
+  Block->setTerminator(G);

efriedma wrote:
> Passing add_successor=false seems suspect; this could probably use more test 
> coverage.
Did you ever look at this?


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

https://reviews.llvm.org/D56571



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


[PATCH] D57915: [COFF, ARM64] Remove definitions for _byteswap library functions

2019-02-07 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

Should clang IR generation be lowering these to bswap calls anyway?  Even if 
the function technically exists in the ucrt, it's going to be pretty slow to 
call it.

Please leave the tests; fix the CHECK lines, if necessary, but we should still 
check it compiles.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57915



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


[PATCH] D57908: [SEMA]Generalize deferred diagnostic interface, NFC.

2019-02-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

LGTM as well.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57908



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


[PATCH] D57896: Variable names rule

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

does the readability-identifier-naming check need to be changed to support 
multiple allowed case types?

  - key: readability-identifier-naming.VariableCase
  value:camelBack,CamelBack


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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


[PATCH] D57908: [SEMA]Generalize deferred diagnostic interface, NFC.

2019-02-07 Thread Alexey Bataev via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL353456: [SEMA]Generalize deferred diagnostic interface, NFC. 
(authored by ABataev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57908

Files:
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/Sema/Sema.cpp
  cfe/trunk/lib/Sema/SemaCUDA.cpp

Index: cfe/trunk/lib/Sema/Sema.cpp
===
--- cfe/trunk/lib/Sema/Sema.cpp
+++ cfe/trunk/lib/Sema/Sema.cpp
@@ -1325,6 +1325,168 @@
   return Builder;
 }
 
+// Print notes showing how we can reach FD starting from an a priori
+// known-callable function.
+static void emitCallStackNotes(Sema , FunctionDecl *FD) {
+  auto FnIt = S.DeviceKnownEmittedFns.find(FD);
+  while (FnIt != S.DeviceKnownEmittedFns.end()) {
+DiagnosticBuilder Builder(
+S.Diags.Report(FnIt->second.Loc, diag::note_called_by));
+Builder << FnIt->second.FD;
+Builder.setForceEmit();
+
+FnIt = S.DeviceKnownEmittedFns.find(FnIt->second.FD);
+  }
+}
+
+// Emit any deferred diagnostics for FD and erase them from the map in which
+// they're stored.
+static void emitDeferredDiags(Sema , FunctionDecl *FD) {
+  auto It = S.DeviceDeferredDiags.find(FD);
+  if (It == S.DeviceDeferredDiags.end())
+return;
+  bool HasWarningOrError = false;
+  for (PartialDiagnosticAt  : It->second) {
+const SourceLocation  = PDAt.first;
+const PartialDiagnostic  = PDAt.second;
+HasWarningOrError |= S.getDiagnostics().getDiagnosticLevel(
+ PD.getDiagID(), Loc) >= DiagnosticsEngine::Warning;
+DiagnosticBuilder Builder(S.Diags.Report(Loc, PD.getDiagID()));
+Builder.setForceEmit();
+PD.Emit(Builder);
+  }
+  S.DeviceDeferredDiags.erase(It);
+
+  // FIXME: Should this be called after every warning/error emitted in the loop
+  // above, instead of just once per function?  That would be consistent with
+  // how we handle immediate errors, but it also seems like a bit much.
+  if (HasWarningOrError)
+emitCallStackNotes(S, FD);
+}
+
+// In CUDA, there are some constructs which may appear in semantically-valid
+// code, but trigger errors if we ever generate code for the function in which
+// they appear.  Essentially every construct you're not allowed to use on the
+// device falls into this category, because you are allowed to use these
+// constructs in a __host__ __device__ function, but only if that function is
+// never codegen'ed on the device.
+//
+// To handle semantic checking for these constructs, we keep track of the set of
+// functions we know will be emitted, either because we could tell a priori that
+// they would be emitted, or because they were transitively called by a
+// known-emitted function.
+//
+// We also keep a partial call graph of which not-known-emitted functions call
+// which other not-known-emitted functions.
+//
+// When we see something which is illegal if the current function is emitted
+// (usually by way of CUDADiagIfDeviceCode, CUDADiagIfHostCode, or
+// CheckCUDACall), we first check if the current function is known-emitted.  If
+// so, we immediately output the diagnostic.
+//
+// Otherwise, we "defer" the diagnostic.  It sits in Sema::DeviceDeferredDiags
+// until we discover that the function is known-emitted, at which point we take
+// it out of this map and emit the diagnostic.
+
+Sema::DeviceDiagBuilder::DeviceDiagBuilder(Kind K, SourceLocation Loc,
+   unsigned DiagID, FunctionDecl *Fn,
+   Sema )
+: S(S), Loc(Loc), DiagID(DiagID), Fn(Fn),
+  ShowCallStack(K == K_ImmediateWithCallStack || K == K_Deferred) {
+  switch (K) {
+  case K_Nop:
+break;
+  case K_Immediate:
+  case K_ImmediateWithCallStack:
+ImmediateDiag.emplace(S.Diag(Loc, DiagID));
+break;
+  case K_Deferred:
+assert(Fn && "Must have a function to attach the deferred diag to.");
+PartialDiag.emplace(S.PDiag(DiagID));
+break;
+  }
+}
+
+Sema::DeviceDiagBuilder::~DeviceDiagBuilder() {
+  if (ImmediateDiag) {
+// Emit our diagnostic and, if it was a warning or error, output a callstack
+// if Fn isn't a priori known-emitted.
+bool IsWarningOrError = S.getDiagnostics().getDiagnosticLevel(
+DiagID, Loc) >= DiagnosticsEngine::Warning;
+ImmediateDiag.reset(); // Emit the immediate diag.
+if (IsWarningOrError && ShowCallStack)
+  emitCallStackNotes(S, Fn);
+  } else if (PartialDiag) {
+assert(ShowCallStack && "Must always show call stack for deferred diags.");
+S.DeviceDeferredDiags[Fn].push_back({Loc, std::move(*PartialDiag)});
+  }
+}
+
+// Indicate that this function (and thus everything it transtively calls) will
+// be codegen'ed, and emit any deferred diagnostics on 

r353456 - [SEMA]Generalize deferred diagnostic interface, NFC.

2019-02-07 Thread Alexey Bataev via cfe-commits
Author: abataev
Date: Thu Feb  7 11:46:42 2019
New Revision: 353456

URL: http://llvm.org/viewvc/llvm-project?rev=353456=rev
Log:
[SEMA]Generalize deferred diagnostic interface, NFC.

Summary:
Deferred diagnostic interface is going to be used for OpenMP device
compilation. Generalized previously existed deferred diagnostic
interface for CUDA to be used with OpenMP and, possibly, other models.

Reviewers: rjmccall, tra

Subscribers: caomhin, cfe-commits, kkwli0

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaCUDA.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=353456=353455=353456=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Feb  7 11:46:42 2019
@@ -10111,7 +10111,7 @@ public:
   /// compilation, this is currently only enabled for CUDA compilations.
   llvm::DenseMap,
  std::vector>
-  CUDADeferredDiags;
+  DeviceDeferredDiags;
 
   /// A pair of a canonical FunctionDecl and a SourceLocation.  When used as 
the
   /// key in a hashtable, both the FD and location are hashed.
@@ -10132,21 +10132,22 @@ public:
   /// map.
   llvm::DenseMap,
  /* Caller = */ FunctionDeclAndLoc>
-  CUDAKnownEmittedFns;
+  DeviceKnownEmittedFns;
 
-  /// A partial call graph maintained during CUDA compilation to support
-  /// deferred diagnostics.
+  /// A partial call graph maintained during CUDA/OpenMP device code 
compilation
+  /// to support deferred diagnostics.
   ///
   /// Functions are only added here if, at the time they're considered, they 
are
   /// not known-emitted.  As soon as we discover that a function is
   /// known-emitted, we remove it and everything it transitively calls from 
this
-  /// set and add those functions to CUDAKnownEmittedFns.
+  /// set and add those functions to DeviceKnownEmittedFns.
   llvm::DenseMap,
  /* Callees = */ 
llvm::MapVector,
  SourceLocation>>
-  CUDACallGraph;
+  DeviceCallGraph;
 
-  /// Diagnostic builder for CUDA errors which may or may not be deferred.
+  /// Diagnostic builder for CUDA/OpenMP devices errors which may or may not be
+  /// deferred.
   ///
   /// In CUDA, there exist constructs (e.g. variable-length arrays, try/catch)
   /// which are not allowed to appear inside __device__ functions and are
@@ -10160,7 +10161,7 @@ public:
   /// diagnostic, or no diagnostic at all, according to an argument you pass to
   /// its constructor, thus simplifying the process of creating these "maybe
   /// deferred" diagnostics.
-  class CUDADiagBuilder {
+  class DeviceDiagBuilder {
   public:
 enum Kind {
   /// Emit no diagnostics.
@@ -10177,25 +10178,25 @@ public:
   K_Deferred
 };
 
-CUDADiagBuilder(Kind K, SourceLocation Loc, unsigned DiagID,
-FunctionDecl *Fn, Sema );
-~CUDADiagBuilder();
+DeviceDiagBuilder(Kind K, SourceLocation Loc, unsigned DiagID,
+  FunctionDecl *Fn, Sema );
+~DeviceDiagBuilder();
 
 /// Convertible to bool: True if we immediately emitted an error, false if
 /// we didn't emit an error or we created a deferred error.
 ///
 /// Example usage:
 ///
-///   if (CUDADiagBuilder(...) << foo << bar)
+///   if (DeviceDiagBuilder(...) << foo << bar)
 /// return ExprError();
 ///
 /// But see CUDADiagIfDeviceCode() and CUDADiagIfHostCode() -- you probably
-/// want to use these instead of creating a CUDADiagBuilder yourself.
+/// want to use these instead of creating a DeviceDiagBuilder yourself.
 operator bool() const { return ImmediateDiag.hasValue(); }
 
 template 
-friend const CUDADiagBuilder <<(const CUDADiagBuilder ,
- const T ) {
+friend const DeviceDiagBuilder <<(const DeviceDiagBuilder ,
+   const T ) {
   if (Diag.ImmediateDiag.hasValue())
 *Diag.ImmediateDiag << Value;
   else if (Diag.PartialDiag.hasValue())
@@ -10216,7 +10217,15 @@ public:
 llvm::Optional PartialDiag;
   };
 
-  /// Creates a CUDADiagBuilder that emits the diagnostic if the current 
context
+  /// Indicate that this function (and thus everything it transtively calls)
+  /// will be codegen'ed, and emit any deferred diagnostics on this function 
and
+  /// its (transitive) callees.
+  void markKnownEmitted(
+  Sema , FunctionDecl *OrigCaller, FunctionDecl *OrigCallee,
+  SourceLocation OrigLoc,
+  const llvm::function_ref IsKnownEmitted);
+
+  /// Creates a DeviceDiagBuilder that emits the diagnostic if the current 
context
   /// is "used as device code".
   

[PATCH] D57918: Add an attribute that causes clang to emit fortified calls to C stdlib functions

2019-02-07 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington created this revision.
erik.pilkington added reviewers: aaron.ballman, rjmccall, eli.friedman.
Herald added subscribers: dexonsmith, jkorous.
Herald added a project: clang.

This attribute applies to declarations of C stdlib functions (sprintf, 
memcpy...) that have known fortified variants (__sprintf_chk, __memcpy_chk, 
...). When applied, clang will emit calls to the fortified variant functions. 
Without this attribute, its impossible to write `gnu_inline`-style wrappers to 
the variadic functions because we don't support `__builtin_va_arg_pack`, and 
don't intend to (see https://reviews.llvm.org/D57635).

Thanks for taking a look!
Erik


Repository:
  rC Clang

https://reviews.llvm.org/D57918

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Builtins.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Basic/Builtins.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/fortify-std-lib.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/fortify-std-lib.c

Index: clang/test/Sema/fortify-std-lib.c
===
--- /dev/null
+++ clang/test/Sema/fortify-std-lib.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -fsyntax-only %s -verify
+
+typedef unsigned long size_t;
+
+__attribute__((fortify_stdlib(0, 0)))
+int not_anyting_special(); // expected-error {{'fortify_stdlib' attribute applied to an unknown function}}
+
+__attribute__((fortify_stdlib(4, 0))) // expected-error {{'fortify_stdlib' attribute requires integer constant between 0 and 3 inclusive}}
+int sprintf(char *, const char *, ...);
+
+__attribute__((fortify_stdlib())) // expected-error {{'fortify_stdlib' attribute requires exactly 2 arguments}}
+int sprintf(char *, const char *, ...);
+
+__attribute__((fortify_stdlib(1, 2, 3))) // expected-error {{'fortify_stdlib' attribute requires exactly 2 arguments}}
+int sprintf(char *, const char *, ...);
+
+__attribute__((fortify_stdlib(-1, 2))) // expected-error {{'fortify_stdlib' attribute requires a non-negative integral compile time constant expression}}
+int sprintf(char *, const char *, ...);
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -52,6 +52,7 @@
 // CHECK-NEXT: ExternalSourceSymbol ((SubjectMatchRule_record, SubjectMatchRule_enum, SubjectMatchRule_enum_constant, SubjectMatchRule_field, SubjectMatchRule_function, SubjectMatchRule_namespace, SubjectMatchRule_objc_category, SubjectMatchRule_objc_interface, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_objc_protocol, SubjectMatchRule_record, SubjectMatchRule_type_alias, SubjectMatchRule_variable))
 // CHECK-NEXT: FlagEnum (SubjectMatchRule_enum)
 // CHECK-NEXT: Flatten (SubjectMatchRule_function)
+// CHECK-NEXT: FortifyStdLib (SubjectMatchRule_function)
 // CHECK-NEXT: GNUInline (SubjectMatchRule_function)
 // CHECK-NEXT: Hot (SubjectMatchRule_function)
 // CHECK-NEXT: IBAction (SubjectMatchRule_objc_method_is_instance)
Index: clang/test/CodeGen/fortify-std-lib.c
===
--- /dev/null
+++ clang/test/CodeGen/fortify-std-lib.c
@@ -0,0 +1,220 @@
+// RUN: %clang_cc1   -triple x86_64-apple-macosx10.14.0 %s -emit-llvm -O0 -disable-llvm-passes -o - -Wno-format-security | FileCheck %s
+// RUN: %clang_cc1 -xc++ -triple x86_64-apple-macosx10.14.0 %s -emit-llvm -O0 -disable-llvm-passes -o - -Wno-format-security | FileCheck %s
+
+#ifdef __cplusplus
+#define EXTERN extern "C"
+#else
+#define EXTERN
+#endif
+
+#define FSL(x,y) __attribute__((fortify_stdlib(x,y)))
+typedef unsigned long size_t;
+
+FSL(0, 0) EXTERN
+void *memcpy(void *dst, const void *src, size_t sz);
+
+EXTERN
+void call_memcpy(void *dst, const void *src, size_t sz) {
+  memcpy(dst, src, sz);
+  // CHECK-LABEL: define void @call_memcpy
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i64.p0i8(i8*{{.*}}, i1 false, i1 true, i1 false)
+  // CHECK: call i8* @__memcpy_chk(i8* {{.*}}, i8* {{.*}}, i64 {{.*}}, i64 [[REG]])
+}
+
+FSL(0, 0) EXTERN
+void *memmove(void *dst, const void *src, size_t sz);
+
+EXTERN
+void call_memmove(void *dst, const void *src, size_t sz) {
+  memmove(dst, src, sz);
+  // CHECK-LABEL: define void @call_memmove
+  // CHECK: [[REG:%[0-9]+]] = call i64 @llvm.objectsize.i64.p0i8(i8*{{.*}}, i1 false, i1 true, i1 false)
+  // CHECK: call i8* @__memmove_chk(i8* {{.*}}, i8* {{.*}}, i64 {{.*}}, i64 [[REG]])
+}
+
+FSL(0, 0) EXTERN
+void *memset(void *dst, int c, size_t sz);
+
+EXTERN
+void call_memset(void *dst, int c, size_t sz) {
+  memset(dst, c, sz);
+  // CHECK-LABEL: define void @call_memset
+  // CHECK: 

[PATCH] D43737: Improve -Winfinite-recursion

2019-02-07 Thread Steven Wu via Phabricator via cfe-commits
steven_wu added a comment.
Herald added a project: LLVM.

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

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

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


Repository:
  rL LLVM

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

https://reviews.llvm.org/D43737



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


[PATCH] D57874: [WebAssembly] Set '-matomics' when '-pthread' is set

2019-02-07 Thread Dan Gohman via Phabricator via cfe-commits
sunfish added a comment.

That sounds reasonable to me too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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


[PATCH] D57874: [WebAssembly] Set '-matomics' when '-pthread' is set

2019-02-07 Thread Sam Clegg via Phabricator via cfe-commits
sbc100 added a comment.

Yes it makes sense to me to set `-mthread-model=posix` when `-pthread` is 
passed on the commandline.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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


[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

2019-02-07 Thread pierre gousseau via Phabricator via cfe-commits
pgousseau created this revision.
pgousseau added reviewers: rsmith, probinson, gbedwell, filcab, lebedev.ri, 
wristow.
Herald added subscribers: cfe-commits, cryptoad.
Herald added a project: clang.

enum SanitizerOrdinal has reached maximum capacity, this change extends the 
capacity to 128 sanitizer checks.
Uses APInt to represent the bit mask.
This can eventually allow us to add gcc 8's options 
"-fsanitize=pointer-substract" and "-fsanitize=pointer-compare".

Fixes: https://llvm.org/PR39425


Repository:
  rC Clang

https://reviews.llvm.org/D57914

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/Sanitizers.h
  include/clang/Driver/ToolChain.h
  lib/Basic/SanitizerSpecialCaseList.cpp
  lib/Basic/Sanitizers.cpp
  lib/CodeGen/CGExpr.cpp
  lib/Driver/SanitizerArgs.cpp

Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -25,29 +25,32 @@
 using namespace clang::driver;
 using namespace llvm::opt;
 
-enum : SanitizerMask {
-  NeedsUbsanRt = Undefined | Integer | ImplicitConversion | Nullability | CFI,
-  NeedsUbsanCxxRt = Vptr | CFI,
-  NotAllowedWithTrap = Vptr,
-  NotAllowedWithMinimalRuntime = Vptr,
-  RequiresPIE = DataFlow | HWAddress | Scudo,
-  NeedsUnwindTables = Address | HWAddress | Thread | Memory | DataFlow,
-  SupportsCoverage = Address | HWAddress | KernelAddress | KernelHWAddress |
- Memory | KernelMemory | Leak | Undefined | Integer |
- ImplicitConversion | Nullability | DataFlow | Fuzzer |
- FuzzerNoLink,
-  RecoverableByDefault = Undefined | Integer | ImplicitConversion | Nullability,
-  Unrecoverable = Unreachable | Return,
-  AlwaysRecoverable = KernelAddress | KernelHWAddress,
-  LegacyFsanitizeRecoverMask = Undefined | Integer,
-  NeedsLTO = CFI,
-  TrappingSupported = (Undefined & ~Vptr) | UnsignedIntegerOverflow |
-  ImplicitConversion | Nullability | LocalBounds | CFI,
-  TrappingDefault = CFI,
-  CFIClasses =
-  CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported | Scudo | ShadowCallStack,
-};
+SanitizerMask NeedsUbsanRt =
+Undefined | Integer | ImplicitConversion | Nullability | CFI;
+SanitizerMask NeedsUbsanCxxRt = Vptr | CFI;
+SanitizerMask NotAllowedWithTrap = Vptr;
+SanitizerMask NotAllowedWithMinimalRuntime = Vptr;
+SanitizerMask RequiresPIE = DataFlow | HWAddress | Scudo;
+SanitizerMask NeedsUnwindTables =
+Address | HWAddress | Thread | Memory | DataFlow;
+SanitizerMask SupportsCoverage =
+Address | HWAddress | KernelAddress | KernelHWAddress | Memory |
+KernelMemory | Leak | Undefined | Integer | ImplicitConversion |
+Nullability | DataFlow | Fuzzer | FuzzerNoLink;
+SanitizerMask RecoverableByDefault =
+Undefined | Integer | ImplicitConversion | Nullability;
+SanitizerMask Unrecoverable = Unreachable | Return;
+SanitizerMask AlwaysRecoverable = KernelAddress | KernelHWAddress;
+SanitizerMask LegacyFsanitizeRecoverMask = Undefined | Integer;
+SanitizerMask NeedsLTO = CFI;
+SanitizerMask TrappingSupported = (Undefined & ~Vptr) |
+  UnsignedIntegerOverflow | ImplicitConversion |
+  Nullability | LocalBounds | CFI;
+SanitizerMask TrappingDefault = CFI;
+SanitizerMask CFIClasses =
+CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast;
+SanitizerMask CompatibleWithMinimalRuntime =
+TrappingSupported | Scudo | ShadowCallStack;
 
 enum CoverageFeature {
   CoverageFunc = 1 << 0,
@@ -136,10 +139,10 @@
 
 static SanitizerMask parseSanitizeTrapArgs(const Driver ,
const llvm::opt::ArgList ) {
-  SanitizerMask TrapRemove = 0; // During the loop below, the accumulated set of
+  SanitizerMask TrapRemove(0);  // During the loop below, the accumulated set of
 // sanitizers disabled by the current sanitizer
 // argument or any argument after it.
-  SanitizerMask TrappingKinds = 0;
+  SanitizerMask TrappingKinds(0);
   SanitizerMask TrappingSupportedWithGroups = setGroupBits(TrappingSupported);
 
   for (ArgList::const_reverse_iterator I = Args.rbegin(), E = Args.rend();
@@ -203,24 +206,26 @@
 }
 
 bool SanitizerArgs::needsUnwindTables() const {
-  return Sanitizers.Mask & NeedsUnwindTables;
+  return static_cast(Sanitizers.Mask & NeedsUnwindTables);
 }
 
-bool SanitizerArgs::needsLTO() const { return Sanitizers.Mask & NeedsLTO; }
+bool SanitizerArgs::needsLTO() const {
+  return static_cast(Sanitizers.Mask & NeedsLTO);
+}
 
 SanitizerArgs::SanitizerArgs(const ToolChain ,
  const llvm::opt::ArgList ) {
-  SanitizerMask AllRemove = 0;  // During the loop below, the accumulated set of
+  SanitizerMask AllRemove(0);   // During the loop below, the accumulated set of
 

[PATCH] D57915: [COFF, ARM64] Remove definitions for _byteswap library functions

2019-02-07 Thread Tom Tan via Phabricator via cfe-commits
TomTan created this revision.
TomTan added reviewers: mgrang, efriedma, mstorsjo.
Herald added subscribers: cfe-commits, kristof.beyls, javed.absar.
Herald added a project: clang.

_byteswap_* functions are are implemented in below file as normal function
from libucrt.lib and declared in stdlib.h. Define them in intrin.h triggers
lld error "conflicting comdat type" and "duplicate symbols" which was just
added to LLD (https://reviews.llvm.org/D57324).

C:\Program Files (x86)\Windows 
Kits\10\Source\10.0.17763.0\ucrt\stdlib\byteswap.cpp


Repository:
  rC Clang

https://reviews.llvm.org/D57915

Files:
  lib/Headers/intrin.h
  test/Headers/ms-arm64-intrin.cpp


Index: test/Headers/ms-arm64-intrin.cpp
===
--- test/Headers/ms-arm64-intrin.cpp
+++ test/Headers/ms-arm64-intrin.cpp
@@ -12,18 +12,3 @@
 // CHECK: "nop"
   __nop();
 }
-
-unsigned short check_byteswap_ushort(unsigned short val) {
-// CHECK: call i16 @llvm.bswap.i16(i16 %val)
-  return _byteswap_ushort(val);
-}
-
-unsigned long check_byteswap_ulong(unsigned long val) {
-// CHECK: call i32 @llvm.bswap.i32(i32 %val)
-  return _byteswap_ulong(val);
-}
-
-unsigned __int64 check_byteswap_uint64(unsigned __int64 val) {
-// CHECK: call i64 @llvm.bswap.i64(i64 %val)
-  return _byteswap_uint64(val);
-}
Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -557,15 +557,9 @@
 int _ReadStatusReg(int);
 void _WriteStatusReg(int, int);
 
-static inline unsigned short _byteswap_ushort (unsigned short val) {
-  return __builtin_bswap16(val);
-}
-static inline unsigned long _byteswap_ulong (unsigned long val) {
-  return __builtin_bswap32(val);
-}
-static inline unsigned __int64 _byteswap_uint64 (unsigned __int64 val) {
-  return __builtin_bswap64(val);
-}
+unsigned short __cdecl _byteswap_ushort(unsigned short val);
+unsigned long __cdecl _byteswap_ulong (unsigned long val);
+unsigned __int64 __cdecl _byteswap_uint64(unsigned __int64 val);
 #endif
 
 
/**\


Index: test/Headers/ms-arm64-intrin.cpp
===
--- test/Headers/ms-arm64-intrin.cpp
+++ test/Headers/ms-arm64-intrin.cpp
@@ -12,18 +12,3 @@
 // CHECK: "nop"
   __nop();
 }
-
-unsigned short check_byteswap_ushort(unsigned short val) {
-// CHECK: call i16 @llvm.bswap.i16(i16 %val)
-  return _byteswap_ushort(val);
-}
-
-unsigned long check_byteswap_ulong(unsigned long val) {
-// CHECK: call i32 @llvm.bswap.i32(i32 %val)
-  return _byteswap_ulong(val);
-}
-
-unsigned __int64 check_byteswap_uint64(unsigned __int64 val) {
-// CHECK: call i64 @llvm.bswap.i64(i64 %val)
-  return _byteswap_uint64(val);
-}
Index: lib/Headers/intrin.h
===
--- lib/Headers/intrin.h
+++ lib/Headers/intrin.h
@@ -557,15 +557,9 @@
 int _ReadStatusReg(int);
 void _WriteStatusReg(int, int);
 
-static inline unsigned short _byteswap_ushort (unsigned short val) {
-  return __builtin_bswap16(val);
-}
-static inline unsigned long _byteswap_ulong (unsigned long val) {
-  return __builtin_bswap32(val);
-}
-static inline unsigned __int64 _byteswap_uint64 (unsigned __int64 val) {
-  return __builtin_bswap64(val);
-}
+unsigned short __cdecl _byteswap_ushort(unsigned short val);
+unsigned long __cdecl _byteswap_ulong (unsigned long val);
+unsigned __int64 __cdecl _byteswap_uint64(unsigned __int64 val);
 #endif
 
 /**\
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D57829: [HIP] Disable emitting llvm.linker.options in device compilation

2019-02-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Also, why is this HIP-specific?  I thought the toolchain was largely shared 
with CUDA and there were just a few runtime differences.


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

https://reviews.llvm.org/D57829



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


[PATCH] D57908: [SEMA]Generalize deferred diagnostic interface, NFC.

2019-02-07 Thread Artem Belevich via Phabricator via cfe-commits
tra accepted this revision.
tra added a comment.
This revision is now accepted and ready to land.

LGTM. I've added jlebar@ as he's originally written the code.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57908



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


[PATCH] D54978: Move the SMT API to LLVM

2019-02-07 Thread George Karpenkov via Phabricator via cfe-commits
george.karpenkov added a comment.

@mikhail.ramalho could you revert then?
In general, we should not use Z3 unless it's explicitly requested.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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


[PATCH] D57874: [WebAssembly] Set '-matomics' when '-pthread' is set

2019-02-07 Thread Derek Schuff via Phabricator via cfe-commits
dschuff added a comment.

Oh I guess another option would just be to pin all 3 flags together here, but 
since `-pthread` sets a preprocessor define and may also affect linker 
behavior, I think it's fine to allow atomic codegen without setting `-pthread`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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


[PATCH] D57874: [WebAssembly] Set '-matomics' when '-pthread' is set

2019-02-07 Thread Derek Schuff via Phabricator via cfe-commits
dschuff added a comment.

So this CL has the effect that setting `-pthreads` will also set `-matomics`.
Currently as you mentioned we have the problem that we can't make our current 
logic of "do we lower away the atomics" be controlled by the target features 
because it's done at pass config time and not codegen time (where there's no 
access to the per-function subtarget). 
Also IIUC on ARM, it's `-mthread-model` that controls generation of atomic 
instructions, so it makes sense that `-mthread-model` does the same thing for 
Wasm. But it's also true that for wasm we use `-m` to enable codegen 
for a particular wasm feature, so it would be good to make `-matomics` do the 
same for consistency.
So, can we just pin those 2 flags together here? i.e. setting one will just 
cause the other to be set?
(I guess we'd also need consistency check so that if someone does something 
like `-mthread-model=posix -mno-atomics` we either throw an error or make it so 
that one of those always overrides the other).


Repository:
  rC Clang

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

https://reviews.llvm.org/D57874



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


[PATCH] D55659: [Sema][ObjC] Disallow non-trivial C struct fields in unions

2019-02-07 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

Two minor tweaks, but otherwise LGTM.




Comment at: lib/AST/Type.cpp:2273
+const RecordDecl *RD = QT->castAs()->getDecl();
+// C++ classes are ignored.
+if (isa(RD))

Might be good to spell this out: we don't want to apply the C restriction in 
C++ because C++ (1) can apply the restriction at a finer grain by banning 
copying/destroying the union and (2) allows users to override these 
restrictions by declaring explicit constructors/etc., which we're not proposing 
to add to C.



Comment at: test/SemaObjC/arc-decls.m:24
+
+union  u_trivial_c {
+  volatile int b;

spacing


Repository:
  rC Clang

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

https://reviews.llvm.org/D55659



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


[PATCH] D54978: Move the SMT API to LLVM

2019-02-07 Thread Shakeel Mahate via Phabricator via cfe-commits
shakeel.mahate added a comment.

The ninja build also fails

ninja -C out/gn
ninja: Entering directory `out/gn'
[1/3182] ACTION //clang/test:lit_site_cfg(//llvm/utils/gn/build/toolchain:unix)
FAILED: gen/clang/test/lit.site.cfg.py 
python ../../llvm/utils/gn/build/write_cmake_config.py -o 
gen/clang/test/lit.site.cfg.py ../../clang/test/lit.site.cfg.py.in 
LIT_SITE_CFG_IN_HEADER=\#\#\ Autogenerated\ from\ 
//clang/test/lit.site.cfg.py.in,\ do\ not\ edit 
CLANG_BINARY_DIR=/home/shakeel/devel/llvm-project/out/gn/obj/clang 
CLANG_SOURCE_DIR=/home/shakeel/devel/llvm-project/clang ENABLE_SHARED=0 
LLVM_BINARY_DIR=/home/shakeel/devel/llvm-project/out/gn/obj/llvm LLVM_LIBS_DIR= 
LLVM_SOURCE_DIR=/home/shakeel/devel/llvm-project/llvm 
LLVM_TOOLS_DIR=/home/shakeel/devel/llvm-project/out/gn/bin 
TARGET_TRIPLE=x86_64-unknown-linux-gnu 
SHLIBDIR=/home/shakeel/devel/llvm-project/out/gn/lib CLANG_ANALYZER_WITH_Z3= 
CLANG_BUILD_EXAMPLES=0 CLANG_DEFAULT_CXX_STDLIB= 
CLANG_TOOLS_DIR=/home/shakeel/devel/llvm-project/out/gn/bin 
CMAKE_CXX_COMPILER=c++ ENABLE_BACKTRACES=1 
LLVM_HOST_TRIPLE=x86_64-unknown-linux-gnu LLVM_LIT_TOOLS_DIR= 
LLVM_USE_SANITIZER= PYTHON_EXECUTABLE=python USE_Z3_SOLVER= 
CLANG_ENABLE_ARCMT=1 CLANG_ENABLE_STATIC_ANALYZER=1 HAVE_LIBZ=1 
HOST_ARCH=x86_64 LLVM_PLUGIN_EXT=.so
Traceback (most recent call last):

  File "../../llvm/utils/gn/build/write_cmake_config.py", line 108, in 
sys.exit(main())
  File "../../llvm/utils/gn/build/write_cmake_config.py", line 72, in main
in_line = var_re.sub(repl, in_line)
  File "../../llvm/utils/gn/build/write_cmake_config.py", line 71, in repl
return values[key]

KeyError: 'LLVM_WITH_Z3'
[2/3182] ACTION 
//llvm/include/llvm/Config:config(//llvm/utils/gn/build/toolchain:unix)
FAILED: gen/llvm/include/llvm/Config/config.h 
python ../../llvm/utils/gn/build/write_cmake_config.py -o 
gen/llvm/include/llvm/Config/config.h 
../../llvm/include/llvm/Config/config.h.cmake 
BUG_REPORT_URL=https://bugs.llvm.org/ ENABLE_BACKTRACES=1 
ENABLE_CRASH_OVERRIDES=1 BACKTRACE_HEADER=execinfo.h 
HAVE_CRASHREPORTERCLIENT_H= HAVE_DECL_FE_ALL_EXCEPT=1 HAVE_DECL_FE_INEXACT=1 
LLVM_ENABLE_DIA_SDK= LLVM_ENABLE_CRASH_DUMPS= HAVE_ERRNO_H=1 HAVE_FCNTL_H=1 
HAVE_FENV_H=1 HAVE_FFI_CALL= HAVE_FFI_FFI_H= HAVE_FFI_H= HAVE_LIBPFM= 
HAVE_LIBPSAPI= HAVE_MALLCTL= HAVE_SIGNAL_H=1 HAVE_STD_IS_TRIVIALLY_COPYABLE=1 
HAVE_STRERROR=1 HAVE_SYS_STAT_H=1 HAVE_SYS_TYPES_H=1 HAVE_VALGRIND_VALGRIND_H= 
HAVE__ALLOCA= HAVE___ALLOCA= HAVE___ASHLDI3= HAVE___ASHRDI3= HAVE___CHKSTK= 
HAVE___CHKSTK_MS= HAVE___CMPDI2= HAVE___DIVDI3= HAVE___FIXDFDI= HAVE___FIXSFDI= 
HAVE___FLOATDIDF= HAVE___LSHRDI3= HAVE___MAIN= HAVE___MODDI3= HAVE___UDIVDI3= 
HAVE___UMODDI3= HAVECHKSTK= HAVECHKSTK_MS= HOST_LINK_VERSION= 
LLVM_TARGET_TRIPLE_ENV= LLVM_VERSION_INFO= 
LLVM_VERSION_PRINTER_SHOW_HOST_TARGET_INFO=1 
PACKAGE_BUGREPORT=https://bugs.llvm.org/ PACKAGE_NAME=LLVM PACKAGE_STRING=LLVM\ 
9.0.0svn PACKAGE_VERSION=9.0.0svn PACKAGE_VENDOR= RETSIGTYPE=void 
LLVM_GISEL_COV_ENABLED= LLVM_GISEL_COV_PREFIX= 
LLVM_DEFAULT_TARGET_TRIPLE=x86_64-unknown-linux-gnu HAVE_FUTIMENS=1 
HAVE_LINK_H=1 HAVE_LSEEK64=1 HAVE_MALLINFO=1 HAVE_POSIX_FALLOCATE=1 
HAVE_SCHED_GETAFFINITY=1 HAVE_CPU_COUNT=1 HAVE_STRUCT_STAT_ST_MTIM_TV_NSEC=1 
HAVE_CRASHREPORTER_INFO= HAVE_DECL_ARC4RANDOM= HAVE_DLADDR= HAVE_MACH_MACH_H= 
HAVE_MALLOC_MALLOC_H= HAVE_MALLOC_ZONE_STATISTICS= 
HAVE_STRUCT_STAT_ST_MTIMESPEC_TV_NSEC= HAVE_BACKTRACE=1 HAVE_POSIX_SPAWN=1 
HAVE_PTHREAD_GETNAME_NP=1 HAVE_DECL_STRERROR_S= HAVE_DLFCN_H=1 HAVE_DLOPEN=1 
HAVE_FUTIMES=1 HAVE_GETPAGESIZE=1 HAVE_GETRLIMIT=1 HAVE_GETRUSAGE=1 
HAVE_ISATTY=1 HAVE_LIBPTHREAD=1 HAVE_PTHREAD_SETNAME_NP=1 HAVE_LIBZ=1 
HAVE_PREAD=1 HAVE_PTHREAD_GETSPECIFIC=1 HAVE_PTHREAD_H=1 
HAVE_PTHREAD_MUTEX_LOCK=1 HAVE_PTHREAD_RWLOCK_INIT=1 HAVE_REALPATH=1 
HAVE_SBRK=1 HAVE_SETENV=1 HAVE_SETRLIMIT=1 HAVE_SIGALTSTACK=1 HAVE_STRERROR_R=1 
HAVE_SYSCONF=1 HAVE_SYS_IOCTL_H=1 HAVE_SYS_MMAN_H=1 HAVE_SYS_PARAM_H=1 
HAVE_SYS_RESOURCE_H=1 HAVE_SYS_TIME_H=1 HAVE_TERMIOS_H=1 HAVE_UNISTD_H=1 
HAVE_ZLIB_H=1 HAVE__CHSIZE_S= HAVE__UNWIND_BACKTRACE=1 stricmp= strdup= 
LTDL_SHLIB_EXT=.so HAVE_LIBEDIT= HAVE_LIBXAR= HAVE_TERMINFO= LLVM_ENABLE_ZLIB=1 
LLVM_LIBXML2_ENABLED=1
Traceback (most recent call last):

  File "../../llvm/utils/gn/build/write_cmake_config.py", line 108, in 
sys.exit(main())
  File "../../llvm/utils/gn/build/write_cmake_config.py", line 72, in main
in_line = var_re.sub(repl, in_line)
  File "../../llvm/utils/gn/build/write_cmake_config.py", line 71, in repl
return values[key]

KeyError: 'LLVM_WITH_Z3'
[3/3182] ACTION 
//clang/include/clang/Config:Config(//llvm/utils/gn/build/toolchain:unix)
FAILED: gen/clang/include/clang/Config/config.h 
python ../../llvm/utils/gn/build/write_cmake_config.py -o 
gen/clang/include/clang/Config/config.h 
../../clang/include/clang/Config/config.h.cmake 
BUG_REPORT_URL=https://bugs.llvm.org/ CLANG_DEFAULT_LINKER= 
CLANG_DEFAULT_STD_C= CLANG_DEFAULT_STD_CXX= CLANG_DEFAULT_CXX_STDLIB= 

[PATCH] D57910: [ASTImporter] Find previous friend function template

2019-02-07 Thread Gabor Marton via Phabricator via cfe-commits
martong created this revision.
martong added reviewers: a_sidorin, shafik.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.

Repository:
  rC Clang

https://reviews.llvm.org/D57910

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -5083,6 +5083,33 @@
 ParameterizedTests, CanonicalRedeclChain,
 ::testing::Values(ArgVector()),);
 
+struct ImportFriendFunctionTemplates : ASTImporterOptionSpecificTestBase {};
+
+TEST_P(ImportFriendFunctionTemplates, LookupShouldFindPreviousFriend) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+  class X {
+template  friend void foo();
+  };
+  )",
+  Lang_CXX);
+  auto *Friend = FirstDeclMatcher().match(
+  ToTU, functionTemplateDecl(hasName("foo")));
+
+  Decl *FromTU = getTuDecl(
+  R"(
+  template  void foo();
+  )",
+  Lang_CXX);
+  auto *FromFoo = FirstDeclMatcher().match(
+  FromTU, functionTemplateDecl(hasName("foo")));
+  auto *Imported = Import(FromFoo, Lang_CXX);
+
+  // Currently chains of FunctionTemplateDecls are not implemented
+  //EXPECT_EQ(Imported->getPreviousDecl(), Friend);
+  EXPECT_EQ(Imported, Friend);
+}
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
 DefaultTestValuesForRunOptions, );
 
@@ -5101,6 +5128,9 @@
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctions,
 DefaultTestValuesForRunOptions, );
 
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFriendFunctionTemplates,
+DefaultTestValuesForRunOptions, );
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportClasses,
 DefaultTestValuesForRunOptions, );
 
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -5529,7 +5529,7 @@
   // Try to find a function in our own ("to") context with the same name, same
   // type, and in the same context as the function we're importing.
   if (!LexicalDC->isFunctionOrMethod()) {
-unsigned IDNS = Decl::IDNS_Ordinary;
+unsigned IDNS = Decl::IDNS_Ordinary | Decl::IDNS_OrdinaryFriend;
 auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
 for (auto *FoundDecl : FoundDecls) {
   if (!FoundDecl->isInIdentifierNamespace(IDNS))


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -5083,6 +5083,33 @@
 ParameterizedTests, CanonicalRedeclChain,
 ::testing::Values(ArgVector()),);
 
+struct ImportFriendFunctionTemplates : ASTImporterOptionSpecificTestBase {};
+
+TEST_P(ImportFriendFunctionTemplates, LookupShouldFindPreviousFriend) {
+  Decl *ToTU = getToTuDecl(
+  R"(
+  class X {
+template  friend void foo();
+  };
+  )",
+  Lang_CXX);
+  auto *Friend = FirstDeclMatcher().match(
+  ToTU, functionTemplateDecl(hasName("foo")));
+
+  Decl *FromTU = getTuDecl(
+  R"(
+  template  void foo();
+  )",
+  Lang_CXX);
+  auto *FromFoo = FirstDeclMatcher().match(
+  FromTU, functionTemplateDecl(hasName("foo")));
+  auto *Imported = Import(FromFoo, Lang_CXX);
+
+  // Currently chains of FunctionTemplateDecls are not implemented
+  //EXPECT_EQ(Imported->getPreviousDecl(), Friend);
+  EXPECT_EQ(Imported, Friend);
+}
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
 DefaultTestValuesForRunOptions, );
 
@@ -5101,6 +5128,9 @@
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctions,
 DefaultTestValuesForRunOptions, );
 
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFriendFunctionTemplates,
+DefaultTestValuesForRunOptions, );
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportClasses,
 DefaultTestValuesForRunOptions, );
 
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -5529,7 +5529,7 @@
   // Try to find a function in our own ("to") context with the same name, same
   // type, and in the same context as the function we're importing.
   if (!LexicalDC->isFunctionOrMethod()) {
-unsigned IDNS = Decl::IDNS_Ordinary;
+unsigned IDNS = Decl::IDNS_Ordinary | Decl::IDNS_OrdinaryFriend;
 auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
 for (auto *FoundDecl : FoundDecls) {
   if (!FoundDecl->isInIdentifierNamespace(IDNS))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D57908: [SEMA]Generalize deferred diagnostic interface, NFC.

2019-02-07 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev created this revision.
ABataev added reviewers: rjmccall, tra.
Herald added a project: clang.

Deferred diagnostic interface is going to be used for OpenMP device
compilation. Generalized previously existed deferred diagnostic
interface for CUDA to be used with OpenMP and, possibly, other models.


Repository:
  rC Clang

https://reviews.llvm.org/D57908

Files:
  include/clang/Sema/Sema.h
  lib/Sema/Sema.cpp
  lib/Sema/SemaCUDA.cpp

Index: lib/Sema/SemaCUDA.cpp
===
--- lib/Sema/SemaCUDA.cpp
+++ lib/Sema/SemaCUDA.cpp
@@ -586,78 +586,6 @@
   NewD->addAttr(CUDADeviceAttr::CreateImplicit(Context));
 }
 
-// In CUDA, there are some constructs which may appear in semantically-valid
-// code, but trigger errors if we ever generate code for the function in which
-// they appear.  Essentially every construct you're not allowed to use on the
-// device falls into this category, because you are allowed to use these
-// constructs in a __host__ __device__ function, but only if that function is
-// never codegen'ed on the device.
-//
-// To handle semantic checking for these constructs, we keep track of the set of
-// functions we know will be emitted, either because we could tell a priori that
-// they would be emitted, or because they were transitively called by a
-// known-emitted function.
-//
-// We also keep a partial call graph of which not-known-emitted functions call
-// which other not-known-emitted functions.
-//
-// When we see something which is illegal if the current function is emitted
-// (usually by way of CUDADiagIfDeviceCode, CUDADiagIfHostCode, or
-// CheckCUDACall), we first check if the current function is known-emitted.  If
-// so, we immediately output the diagnostic.
-//
-// Otherwise, we "defer" the diagnostic.  It sits in Sema::CUDADeferredDiags
-// until we discover that the function is known-emitted, at which point we take
-// it out of this map and emit the diagnostic.
-
-Sema::CUDADiagBuilder::CUDADiagBuilder(Kind K, SourceLocation Loc,
-   unsigned DiagID, FunctionDecl *Fn,
-   Sema )
-: S(S), Loc(Loc), DiagID(DiagID), Fn(Fn),
-  ShowCallStack(K == K_ImmediateWithCallStack || K == K_Deferred) {
-  switch (K) {
-  case K_Nop:
-break;
-  case K_Immediate:
-  case K_ImmediateWithCallStack:
-ImmediateDiag.emplace(S.Diag(Loc, DiagID));
-break;
-  case K_Deferred:
-assert(Fn && "Must have a function to attach the deferred diag to.");
-PartialDiag.emplace(S.PDiag(DiagID));
-break;
-  }
-}
-
-// Print notes showing how we can reach FD starting from an a priori
-// known-callable function.
-static void EmitCallStackNotes(Sema , FunctionDecl *FD) {
-  auto FnIt = S.CUDAKnownEmittedFns.find(FD);
-  while (FnIt != S.CUDAKnownEmittedFns.end()) {
-DiagnosticBuilder Builder(
-S.Diags.Report(FnIt->second.Loc, diag::note_called_by));
-Builder << FnIt->second.FD;
-Builder.setForceEmit();
-
-FnIt = S.CUDAKnownEmittedFns.find(FnIt->second.FD);
-  }
-}
-
-Sema::CUDADiagBuilder::~CUDADiagBuilder() {
-  if (ImmediateDiag) {
-// Emit our diagnostic and, if it was a warning or error, output a callstack
-// if Fn isn't a priori known-emitted.
-bool IsWarningOrError = S.getDiagnostics().getDiagnosticLevel(
-DiagID, Loc) >= DiagnosticsEngine::Warning;
-ImmediateDiag.reset(); // Emit the immediate diag.
-if (IsWarningOrError && ShowCallStack)
-  EmitCallStackNotes(S, Fn);
-  } else if (PartialDiag) {
-assert(ShowCallStack && "Must always show call stack for deferred diags.");
-S.CUDADeferredDiags[Fn].push_back({Loc, std::move(*PartialDiag)});
-  }
-}
-
 // Do we know that we will eventually codegen the given function?
 static bool IsKnownEmitted(Sema , FunctionDecl *FD) {
   // Templates are emitted when they're instantiated.
@@ -689,147 +617,59 @@
 
   // Otherwise, the function is known-emitted if it's in our set of
   // known-emitted functions.
-  return S.CUDAKnownEmittedFns.count(FD) > 0;
+  return S.DeviceKnownEmittedFns.count(FD) > 0;
 }
 
-Sema::CUDADiagBuilder Sema::CUDADiagIfDeviceCode(SourceLocation Loc,
- unsigned DiagID) {
+Sema::DeviceDiagBuilder Sema::CUDADiagIfDeviceCode(SourceLocation Loc,
+   unsigned DiagID) {
   assert(getLangOpts().CUDA && "Should only be called during CUDA compilation");
-  CUDADiagBuilder::Kind DiagKind = [&] {
+  DeviceDiagBuilder::Kind DiagKind = [this] {
 switch (CurrentCUDATarget()) {
 case CFT_Global:
 case CFT_Device:
-  return CUDADiagBuilder::K_Immediate;
+  return DeviceDiagBuilder::K_Immediate;
 case CFT_HostDevice:
   // An HD function counts as host code if we're compiling for host, and
   // device code if we're compiling for device.  Defer any errors in device
   // mode 

[PATCH] D57896: Variable names rule

2019-02-07 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment.
Herald added a project: LLVM.

In D57896#1389067 , @lebedev.ri wrote:

> 2. It might be best to give this more visibility, by submitting a mail to 
> llvm-dev, with a **noticeable** subject, like "RFC: changing variable naming 
> rules in LLVM codebase"


+1. I know the discussion took place there originally, but "RFC" will catch 
more people's eyes. Also the prior discussion had some digressions (ahem) which 
a new RFC can be more focused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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


[PATCH] D57906: [CTU] Do not allow different CPP dialects in CTU

2019-02-07 Thread Gabor Marton via Phabricator via cfe-commits
martong created this revision.
martong added reviewers: xazax.hun, a_sidorin, r.stahl.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs.
Herald added a project: clang.

If CPP dialects are different then return with error.

Consider this STL code:

  template
struct __alloc_traits
  #if __cplusplus >= 201103L
: std::allocator_traits<_Alloc>
  #endif
{ // ...
};

This class template would create ODR errors during merging the two units,
since in one translation unit the class template has a base class, however
in the other unit it has none.


Repository:
  rC Clang

https://reviews.llvm.org/D57906

Files:
  lib/CrossTU/CrossTranslationUnit.cpp


Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -228,13 +228,34 @@
 
   const auto  = Context.getLangOpts();
   const auto  = Unit->getASTContext().getLangOpts();
-  // FIXME: Currenty we do not support CTU across C++ and C and across
-  // different dialects of C++.
+
+  // We do not support CTU across languages (C vs C++).
   if (LangTo.CPlusPlus != LangFrom.CPlusPlus) {
 ++NumLangMismatch;
 return llvm::make_error(index_error_code::lang_mismatch);
   }
 
+  // If CPP dialects are different then return with error.
+  //
+  // Consider this STL code:
+  //   template
+  // struct __alloc_traits
+  //   #if __cplusplus >= 201103L
+  // : std::allocator_traits<_Alloc>
+  //   #endif
+  // { // ...
+  // };
+  // This class template would create ODR errors during merging the two units,
+  // since in one translation unit the class template has a base class, however
+  // in the other unit it has none.
+  if (LangTo.CPlusPlus11 != LangFrom.CPlusPlus11 ||
+  LangTo.CPlusPlus14 != LangFrom.CPlusPlus14 ||
+  LangTo.CPlusPlus17 != LangFrom.CPlusPlus17 ||
+  LangTo.CPlusPlus2a != LangFrom.CPlusPlus2a) {
+++NumLangMismatch;
+return llvm::make_error(index_error_code::lang_mismatch);
+  }
+
   TranslationUnitDecl *TU = Unit->getASTContext().getTranslationUnitDecl();
   if (const FunctionDecl *ResultDecl =
   findFunctionInDeclContext(TU, LookupFnName))


Index: lib/CrossTU/CrossTranslationUnit.cpp
===
--- lib/CrossTU/CrossTranslationUnit.cpp
+++ lib/CrossTU/CrossTranslationUnit.cpp
@@ -228,13 +228,34 @@
 
   const auto  = Context.getLangOpts();
   const auto  = Unit->getASTContext().getLangOpts();
-  // FIXME: Currenty we do not support CTU across C++ and C and across
-  // different dialects of C++.
+
+  // We do not support CTU across languages (C vs C++).
   if (LangTo.CPlusPlus != LangFrom.CPlusPlus) {
 ++NumLangMismatch;
 return llvm::make_error(index_error_code::lang_mismatch);
   }
 
+  // If CPP dialects are different then return with error.
+  //
+  // Consider this STL code:
+  //   template
+  // struct __alloc_traits
+  //   #if __cplusplus >= 201103L
+  // : std::allocator_traits<_Alloc>
+  //   #endif
+  // { // ...
+  // };
+  // This class template would create ODR errors during merging the two units,
+  // since in one translation unit the class template has a base class, however
+  // in the other unit it has none.
+  if (LangTo.CPlusPlus11 != LangFrom.CPlusPlus11 ||
+  LangTo.CPlusPlus14 != LangFrom.CPlusPlus14 ||
+  LangTo.CPlusPlus17 != LangFrom.CPlusPlus17 ||
+  LangTo.CPlusPlus2a != LangFrom.CPlusPlus2a) {
+++NumLangMismatch;
+return llvm::make_error(index_error_code::lang_mismatch);
+  }
+
   TranslationUnitDecl *TU = Unit->getASTContext().getTranslationUnitDecl();
   if (const FunctionDecl *ResultDecl =
   findFunctionInDeclContext(TU, LookupFnName))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D54978: Move the SMT API to LLVM

2019-02-07 Thread Brian Rzycki via Phabricator via cfe-commits
brzycki added a comment.

This commit is causing a build-break for our nightly cross compilers of arm and 
aarch64. The immediately preceding commit of D54977 
 does not break with the exact same invocation.

The problem is our build machine (Ubuntu 18.04 LTS) installs an old version of 
Z3 that is incompatible with LLVM's tip of tree. To deal with this we add `-D 
CLANG_ANALYZER_ENABLE_Z3_SOLVER=OFF` to force disabling Z3 even if CMake 
detects the library is installed.

With this patch I am unable to disable Z3 because 
`CLANG_ANALYZER_ENABLE_Z3_SOLVER` is no longer in the codebase and setting `-D 
LLVM_ENABLE_Z3_SOLVER=OFF` still builds Z3 support (which fails):

  [74/187] Building CXX object 
lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o
  FAILED: lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o
  /usr/bin/clang++-6.0  -DGTEST_HAS_RTTI=0 -D_GNU_SOURCE 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-Ilib/Support -I/tmp/tmp.UCDOJjmgJ1/src/llvm/lib/Support -I/usr/include/libxml2 
-Iinclude -I/tmp/tmp.UCDOJjmgJ1/src/llvm/include -fPIC 
-fvisibility-inlines-hidden -Werror=date-time 
-Werror=unguarded-availability-new -std=c++11 -Wall -Wextra 
-Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers 
-pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default 
-Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor 
-Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -O3 
-DNDEBUG-fno-exceptions -fno-rtti -MD -MT 
lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o -MF 
lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o.d -o 
lib/Support/CMakeFiles/LLVMSupport.dir/Z3Solver.cpp.o -c 
/tmp/tmp.UCDOJjmgJ1/src/llvm/lib/Support/Z3Solver.cpp
  /tmp/tmp.UCDOJjmgJ1/src/llvm/lib/Support/Z3Solver.cpp:44:40: error: no 
matching function for call to 'Z3_get_error_msg'
 llvm::Twine(Z3_get_error_msg(Context, Error)));
 ^~~~


Repository:
  rC Clang

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

https://reviews.llvm.org/D54978



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


[PATCH] D57905: [ASTImporter][ASTImporterSpecificLookup] Add test for different operators

2019-02-07 Thread Gabor Marton via Phabricator via cfe-commits
martong created this revision.
martong added reviewers: a_sidorin, shafik.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.

This is to check that operators are handled properly in
`ASTImporterSpecificLookup`.  Note, this lookup table is not used in LLDB, only
in CTU.


Repository:
  rC Clang

https://reviews.llvm.org/D57905

Files:
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -4812,6 +4812,65 @@
   EXPECT_EQ(Res.count(F2), 1u);
 }
 
+TEST_P(ASTImporterLookupTableTest,
+   DifferentOperatorsShouldHaveDifferentResultSet) {
+  TranslationUnitDecl *ToTU = getToTuDecl(
+  R"(
+  struct X{};
+  void operator+(X, X);
+  void operator-(X, X);
+  )",
+  Lang_CXX);
+
+  ASTImporterLookupTable LT(*ToTU);
+  auto *FPlus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("+")));
+  auto *FMinus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("-")));
+  DeclarationName NamePlus = FPlus->getDeclName();
+  auto ResPlus = LT.lookup(ToTU, NamePlus);
+  EXPECT_EQ(ResPlus.size(), 1u);
+  EXPECT_EQ(ResPlus.count(FPlus), 1u);
+  EXPECT_EQ(ResPlus.count(FMinus), 0u);
+  DeclarationName NameMinus = FMinus->getDeclName();
+  auto ResMinus = LT.lookup(ToTU, NameMinus);
+  EXPECT_EQ(ResMinus.size(), 1u);
+  EXPECT_EQ(ResMinus.count(FMinus), 1u);
+  EXPECT_EQ(ResMinus.count(FPlus), 0u);
+  EXPECT_NE(*ResMinus.begin(), *ResPlus.begin());
+}
+
+TEST_P(ASTImporterLookupTableTest, LookupDeclNamesFromDifferentTUs) {
+  TranslationUnitDecl *ToTU = getToTuDecl(
+  R"(
+  struct X {};
+  void operator+(X, X);
+  )",
+  Lang_CXX);
+  auto *ToPlus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("+")));
+
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct X {};
+  void operator+(X, X);
+  )",
+  Lang_CXX);
+  auto *FromPlus = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasOverloadedOperatorName("+")));
+
+  ASSERT_NE(ToPlus->getDeclName(), FromPlus->getDeclName());
+
+  ASTImporterLookupTable LT(*ToTU);
+  auto Res = LT.lookup(ToTU, ToPlus->getDeclName());
+  ASSERT_EQ(Res.size(), 1u);
+  EXPECT_EQ(*Res.begin(), ToPlus);
+
+  // FromPlus have a different TU, thus its DeclarationName is different too.
+  Res = LT.lookup(ToTU, FromPlus->getDeclName());
+  ASSERT_EQ(Res.size(), 0u);
+}
+
 static const RecordDecl * getRecordDeclOfFriend(FriendDecl *FD) {
   QualType Ty = FD->getFriendType()->getType();
   QualType NamedTy = cast(Ty)->getNamedType();


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -4812,6 +4812,65 @@
   EXPECT_EQ(Res.count(F2), 1u);
 }
 
+TEST_P(ASTImporterLookupTableTest,
+   DifferentOperatorsShouldHaveDifferentResultSet) {
+  TranslationUnitDecl *ToTU = getToTuDecl(
+  R"(
+  struct X{};
+  void operator+(X, X);
+  void operator-(X, X);
+  )",
+  Lang_CXX);
+
+  ASTImporterLookupTable LT(*ToTU);
+  auto *FPlus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("+")));
+  auto *FMinus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("-")));
+  DeclarationName NamePlus = FPlus->getDeclName();
+  auto ResPlus = LT.lookup(ToTU, NamePlus);
+  EXPECT_EQ(ResPlus.size(), 1u);
+  EXPECT_EQ(ResPlus.count(FPlus), 1u);
+  EXPECT_EQ(ResPlus.count(FMinus), 0u);
+  DeclarationName NameMinus = FMinus->getDeclName();
+  auto ResMinus = LT.lookup(ToTU, NameMinus);
+  EXPECT_EQ(ResMinus.size(), 1u);
+  EXPECT_EQ(ResMinus.count(FMinus), 1u);
+  EXPECT_EQ(ResMinus.count(FPlus), 0u);
+  EXPECT_NE(*ResMinus.begin(), *ResPlus.begin());
+}
+
+TEST_P(ASTImporterLookupTableTest, LookupDeclNamesFromDifferentTUs) {
+  TranslationUnitDecl *ToTU = getToTuDecl(
+  R"(
+  struct X {};
+  void operator+(X, X);
+  )",
+  Lang_CXX);
+  auto *ToPlus = FirstDeclMatcher().match(
+  ToTU, functionDecl(hasOverloadedOperatorName("+")));
+
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct X {};
+  void operator+(X, X);
+  )",
+  Lang_CXX);
+  auto *FromPlus = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasOverloadedOperatorName("+")));
+
+  ASSERT_NE(ToPlus->getDeclName(), FromPlus->getDeclName());
+
+  ASTImporterLookupTable LT(*ToTU);
+  auto Res = LT.lookup(ToTU, ToPlus->getDeclName());
+  ASSERT_EQ(Res.size(), 1u);
+  EXPECT_EQ(*Res.begin(), ToPlus);
+
+  // FromPlus have a different TU, thus its DeclarationName is different too.
+  Res = LT.lookup(ToTU, FromPlus->getDeclName());
+  ASSERT_EQ(Res.size(), 0u);
+}
+
 static const RecordDecl * 

[PATCH] D57896: Variable names rule

2019-02-07 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

In D57896#1389046 , @michaelplatings 
wrote:

> In D57896#1389042 , @lebedev.ri 
> wrote:
>
> > Pretty sure this patch should have gone to llvm-commits, not cfe-commits.
>
>
> I just set the repository, Phabricator did the rest - apparently the magic 
> isn't working so well.




1. Does clang-tidy warn on every single existing variable now?
2. It might be best to give this more visibility, by submitting a mail to 
llvm-dev, with a **noticeable** subject, like "RFC: changing variable naming 
rules in LLVM codebase"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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


[PATCH] D57824: [OpenCL][PR40603] In C++ preserve backwards compatibility with OpenCL C v2.0

2019-02-07 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL353431: [OpenCL][PR40603] In C++ preserve compatibility with 
OpenCL C v2.0 (authored by stulova, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57824?vs=185774=185794#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57824

Files:
  cfe/trunk/include/clang/Basic/OpenCLOptions.h
  cfe/trunk/lib/Frontend/InitPreprocessor.cpp
  cfe/trunk/lib/Parse/ParsePragma.cpp
  cfe/trunk/lib/Sema/Sema.cpp
  cfe/trunk/test/SemaOpenCL/extension-version.cl
  cfe/trunk/test/SemaOpenCL/extensions.cl

Index: cfe/trunk/include/clang/Basic/OpenCLOptions.h
===
--- cfe/trunk/include/clang/Basic/OpenCLOptions.h
+++ cfe/trunk/include/clang/Basic/OpenCLOptions.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_CLANG_BASIC_OPENCLOPTIONS_H
 #define LLVM_CLANG_BASIC_OPENCLOPTIONS_H
 
+#include "clang/Basic/LangOptions.h"
 #include "llvm/ADT/StringMap.h"
 
 namespace clang {
@@ -41,25 +42,29 @@
 
   // Is supported as either an extension or an (optional) core feature for
   // OpenCL version \p CLVer.
-  bool isSupported(llvm::StringRef Ext, unsigned CLVer) const {
+  bool isSupported(llvm::StringRef Ext, LangOptions LO) const {
+// In C++ mode all extensions should work at least as in v2.0.
+auto CLVer = LO.OpenCLCPlusPlus ? 200 : LO.OpenCLVersion;
 auto I = OptMap.find(Ext)->getValue();
 return I.Supported && I.Avail <= CLVer;
   }
 
   // Is supported (optional) OpenCL core features for OpenCL version \p CLVer.
   // For supported extension, return false.
-  bool isSupportedCore(llvm::StringRef Ext, unsigned CLVer) const {
+  bool isSupportedCore(llvm::StringRef Ext, LangOptions LO) const {
+// In C++ mode all extensions should work at least as in v2.0.
+auto CLVer = LO.OpenCLCPlusPlus ? 200 : LO.OpenCLVersion;
 auto I = OptMap.find(Ext)->getValue();
-return I.Supported && I.Avail <= CLVer &&
-  I.Core != ~0U && CLVer >= I.Core;
+return I.Supported && I.Avail <= CLVer && I.Core != ~0U && CLVer >= I.Core;
   }
 
   // Is supported OpenCL extension for OpenCL version \p CLVer.
   // For supported (optional) core feature, return false.
- bool isSupportedExtension(llvm::StringRef Ext, unsigned CLVer) const {
+  bool isSupportedExtension(llvm::StringRef Ext, LangOptions LO) const {
+// In C++ mode all extensions should work at least as in v2.0.
+auto CLVer = LO.OpenCLCPlusPlus ? 200 : LO.OpenCLVersion;
 auto I = OptMap.find(Ext)->getValue();
-return I.Supported && I.Avail <= CLVer &&
-  (I.Core == ~0U || CLVer < I.Core);
+return I.Supported && I.Avail <= CLVer && (I.Core == ~0U || CLVer < I.Core);
   }
 
   void enable(llvm::StringRef Ext, bool V = true) {
@@ -121,10 +126,10 @@
   I->second.Enabled = false;
   }
 
-  void enableSupportedCore(unsigned CLVer) {
-for (llvm::StringMap::iterator I = OptMap.begin(),
- E = OptMap.end(); I != E; ++I)
-  if (isSupportedCore(I->getKey(), CLVer))
+  void enableSupportedCore(LangOptions LO) {
+for (llvm::StringMap::iterator I = OptMap.begin(), E = OptMap.end();
+ I != E; ++I)
+  if (isSupportedCore(I->getKey(), LO))
 I->second.Enabled = true;
   }
 
@@ -132,6 +137,6 @@
   friend class ASTReader;
 };
 
-}  // end namespace clang
+} // end namespace clang
 
 #endif
Index: cfe/trunk/test/SemaOpenCL/extension-version.cl
===
--- cfe/trunk/test/SemaOpenCL/extension-version.cl
+++ cfe/trunk/test/SemaOpenCL/extension-version.cl
@@ -2,12 +2,14 @@
 // RUN: %clang_cc1 -x cl -cl-std=CL1.1 %s -verify -triple spir-unknown-unknown
 // RUN: %clang_cc1 -x cl -cl-std=CL1.2 %s -verify -triple spir-unknown-unknown
 // RUN: %clang_cc1 -x cl -cl-std=CL2.0 %s -verify -triple spir-unknown-unknown
+// RUN: %clang_cc1 -x cl -cl-std=c++ %s -verify -triple spir-unknown-unknown
 // RUN: %clang_cc1 -x cl -cl-std=CL %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 // RUN: %clang_cc1 -x cl -cl-std=CL1.1 %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 // RUN: %clang_cc1 -x cl -cl-std=CL1.2 %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 // RUN: %clang_cc1 -x cl -cl-std=CL2.0 %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
+// RUN: %clang_cc1 -x cl -cl-std=c++ %s -verify -triple spir-unknown-unknown -Wpedantic-core-features -DTEST_CORE_FEATURES
 
-#if __OPENCL_C_VERSION__ >= 200 && ! defined TEST_CORE_FEATURES
+#if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200) && !defined(TEST_CORE_FEATURES)
 // expected-no-diagnostics
 #endif
 
@@ -47,44 +49,44 @@
 #ifndef cl_khr_byte_addressable_store
 #error 

r353431 - [OpenCL][PR40603] In C++ preserve compatibility with OpenCL C v2.0

2019-02-07 Thread Anastasia Stulova via cfe-commits
Author: stulova
Date: Thu Feb  7 09:32:37 2019
New Revision: 353431

URL: http://llvm.org/viewvc/llvm-project?rev=353431=rev
Log:
[OpenCL][PR40603] In C++ preserve compatibility with OpenCL C v2.0

Valid OpenCL C code should still compile in C++ mode.

This change enables extensions and OpenCL types.

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


Modified:
cfe/trunk/include/clang/Basic/OpenCLOptions.h
cfe/trunk/lib/Frontend/InitPreprocessor.cpp
cfe/trunk/lib/Parse/ParsePragma.cpp
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/test/SemaOpenCL/extension-version.cl
cfe/trunk/test/SemaOpenCL/extensions.cl

Modified: cfe/trunk/include/clang/Basic/OpenCLOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/OpenCLOptions.h?rev=353431=353430=353431=diff
==
--- cfe/trunk/include/clang/Basic/OpenCLOptions.h (original)
+++ cfe/trunk/include/clang/Basic/OpenCLOptions.h Thu Feb  7 09:32:37 2019
@@ -14,6 +14,7 @@
 #ifndef LLVM_CLANG_BASIC_OPENCLOPTIONS_H
 #define LLVM_CLANG_BASIC_OPENCLOPTIONS_H
 
+#include "clang/Basic/LangOptions.h"
 #include "llvm/ADT/StringMap.h"
 
 namespace clang {
@@ -41,25 +42,29 @@ public:
 
   // Is supported as either an extension or an (optional) core feature for
   // OpenCL version \p CLVer.
-  bool isSupported(llvm::StringRef Ext, unsigned CLVer) const {
+  bool isSupported(llvm::StringRef Ext, LangOptions LO) const {
+// In C++ mode all extensions should work at least as in v2.0.
+auto CLVer = LO.OpenCLCPlusPlus ? 200 : LO.OpenCLVersion;
 auto I = OptMap.find(Ext)->getValue();
 return I.Supported && I.Avail <= CLVer;
   }
 
   // Is supported (optional) OpenCL core features for OpenCL version \p CLVer.
   // For supported extension, return false.
-  bool isSupportedCore(llvm::StringRef Ext, unsigned CLVer) const {
+  bool isSupportedCore(llvm::StringRef Ext, LangOptions LO) const {
+// In C++ mode all extensions should work at least as in v2.0.
+auto CLVer = LO.OpenCLCPlusPlus ? 200 : LO.OpenCLVersion;
 auto I = OptMap.find(Ext)->getValue();
-return I.Supported && I.Avail <= CLVer &&
-  I.Core != ~0U && CLVer >= I.Core;
+return I.Supported && I.Avail <= CLVer && I.Core != ~0U && CLVer >= I.Core;
   }
 
   // Is supported OpenCL extension for OpenCL version \p CLVer.
   // For supported (optional) core feature, return false.
- bool isSupportedExtension(llvm::StringRef Ext, unsigned CLVer) const {
+  bool isSupportedExtension(llvm::StringRef Ext, LangOptions LO) const {
+// In C++ mode all extensions should work at least as in v2.0.
+auto CLVer = LO.OpenCLCPlusPlus ? 200 : LO.OpenCLVersion;
 auto I = OptMap.find(Ext)->getValue();
-return I.Supported && I.Avail <= CLVer &&
-  (I.Core == ~0U || CLVer < I.Core);
+return I.Supported && I.Avail <= CLVer && (I.Core == ~0U || CLVer < 
I.Core);
   }
 
   void enable(llvm::StringRef Ext, bool V = true) {
@@ -121,10 +126,10 @@ public:
   I->second.Enabled = false;
   }
 
-  void enableSupportedCore(unsigned CLVer) {
-for (llvm::StringMap::iterator I = OptMap.begin(),
- E = OptMap.end(); I != E; ++I)
-  if (isSupportedCore(I->getKey(), CLVer))
+  void enableSupportedCore(LangOptions LO) {
+for (llvm::StringMap::iterator I = OptMap.begin(), E = OptMap.end();
+ I != E; ++I)
+  if (isSupportedCore(I->getKey(), LO))
 I->second.Enabled = true;
   }
 
@@ -132,6 +137,6 @@ public:
   friend class ASTReader;
 };
 
-}  // end namespace clang
+} // end namespace clang
 
 #endif

Modified: cfe/trunk/lib/Frontend/InitPreprocessor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/InitPreprocessor.cpp?rev=353431=353430=353431=diff
==
--- cfe/trunk/lib/Frontend/InitPreprocessor.cpp (original)
+++ cfe/trunk/lib/Frontend/InitPreprocessor.cpp Thu Feb  7 09:32:37 2019
@@ -1058,10 +1058,9 @@ static void InitializePredefinedMacros(c
 
   // OpenCL definitions.
   if (LangOpts.OpenCL) {
-#define OPENCLEXT(Ext) \
-if (TI.getSupportedOpenCLOpts().isSupported(#Ext, \
-LangOpts.OpenCLVersion)) \
-  Builder.defineMacro(#Ext);
+#define OPENCLEXT(Ext) 
\
+  if (TI.getSupportedOpenCLOpts().isSupported(#Ext, LangOpts)) 
\
+Builder.defineMacro(#Ext);
 #include "clang/Basic/OpenCLExtensions.def"
 
 auto Arch = TI.getTriple().getArch();

Modified: cfe/trunk/lib/Parse/ParsePragma.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParsePragma.cpp?rev=353431=353430=353431=diff
==
--- cfe/trunk/lib/Parse/ParsePragma.cpp (original)
+++ cfe/trunk/lib/Parse/ParsePragma.cpp Thu Feb  7 09:32:37 2019
@@ -692,13 +692,12 @@ void Parser::HandlePragmaOpenCLExtension
   if (Name == "all") 

[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

2019-02-07 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 marked an inline comment as done.
jyu2 added inline comments.



Comment at: lib/AST/Stmt.cpp:628
+  DiagOffs = CurPtr-StrStart-1;
+  return diag::err_asm_invalid_operand_for_goto_labels;
+}

rsmith wrote:
> jyu2 wrote:
> > rsmith wrote:
> > > I'm curious why we're checking this here, when all the other validation 
> > > for the modifier letter happens in LLVM. Does this check need to be done 
> > > differently from the others?
> > This is to verify following in spec:
> > =
> > To reference a label in the assembler template, prefix it with ‘%l’ 
> > (lowercase ‘L’) followed by its (zero-based) position in GotoLabels plus 
> > the number of input operands. For example, if the asm has three inputs and 
> > references two labels, refer to the first label as ‘%l3’ and the second as 
> > ‘%l4’).
> > 
> > Was request from Eli.
> Sorry, I think my question was unclear. I understand why you need to do this 
> check somewhere. My question is why this check needs to be *here*, given that 
> all the other checks for the modifier letter are performed elsewhere. 
> Splitting the checks up between multiple locations makes the system harder to 
> maintain.
> 
> As it happens, there's a FIXME for this here: 
> http://llvm.org/doxygen/AsmPrinterInlineAsm_8cpp_source.html#l00436 -- and a 
> test case like `void f(void *a) { asm("mov %l0, %%eax" :: "r"(a)); }` 
> currently causes LLVM to assert in a function that's supposed to be 
> validating these modifier letters and producing a clean error if they're bad. 
> So I think that's an LLVM bug, and fixing that LLVM bug would enforce the 
> property we need here (that `%lN` only names labels).
> 
> 
> [It's not clear to me whether we should be enforcing that `%lN` only names 
> label //operands//, which would require the check to be in Clang rather than 
> LLVM; GCC doesn't do that, and for instance accepts
> 
> ```
> void f() { asm goto("jmp %l0" :: "i"(&)::foo); foo:; }
> ```
> 
> (at least for x86_64), though this construct doesn't seem hugely useful given 
> that the input operand would need to be a literal address-of-label expression 
> and you'd need to name the target label as a label operand anyway.]
Hi Richard,

Thanks for the detail explanation. 
I agree with you.  Our compiler FE don't emit error for this also.

I'd happy to remove this, If we all agree.  

Hi Eli, 
what do you think?

Thanks.
Jennifer  



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

https://reviews.llvm.org/D56571



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


[PATCH] D57902: [AST] Fix structural inequivalence of operators

2019-02-07 Thread Gabor Marton via Phabricator via cfe-commits
martong created this revision.
martong added reviewers: shafik, a_sidorin, aaron.ballman.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs.
Herald added a project: clang.

Operators kind was not checked, so we reported e.g. op- to be equal with op+


Repository:
  rC Clang

https://reviews.llvm.org/D57902

Files:
  lib/AST/ASTStructuralEquivalence.cpp
  unittests/AST/StructuralEquivalenceTest.cpp


Index: unittests/AST/StructuralEquivalenceTest.cpp
===
--- unittests/AST/StructuralEquivalenceTest.cpp
+++ unittests/AST/StructuralEquivalenceTest.cpp
@@ -230,6 +230,33 @@
   EXPECT_FALSE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceFunctionTest, DifferentOperators) {
+  auto t = makeDecls(
+  "struct X{}; bool operator<(X, X);",
+  "struct X{}; bool operator==(X, X);", Lang_CXX,
+  functionDecl(hasOverloadedOperatorName("<")),
+  functionDecl(hasOverloadedOperatorName("==")));
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceFunctionTest, SameOperators) {
+  auto t = makeDecls(
+  "struct X{}; bool operator<(X, X);",
+  "struct X{}; bool operator<(X, X);", Lang_CXX,
+  functionDecl(hasOverloadedOperatorName("<")),
+  functionDecl(hasOverloadedOperatorName("<")));
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceFunctionTest, CtorVsDtor) {
+  auto t = makeDecls(
+  "struct X{ X(); };",
+  "struct X{ ~X(); };", Lang_CXX,
+  cxxConstructorDecl(),
+  cxxDestructorDecl());
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
 TEST_F(StructuralEquivalenceFunctionTest, ParamConstWithRef) {
   auto t = makeNamedDecls("void foo(int&);",
   "void foo(const int&);", Lang_CXX);
Index: lib/AST/ASTStructuralEquivalence.cpp
===
--- lib/AST/ASTStructuralEquivalence.cpp
+++ lib/AST/ASTStructuralEquivalence.cpp
@@ -1650,6 +1650,12 @@
 }
   } else if (FunctionDecl *FD1 = dyn_cast(D1)) {
 if (FunctionDecl *FD2 = dyn_cast(D2)) {
+  if (FD1->isOverloadedOperator()) {
+if (!FD2->isOverloadedOperator())
+  return false;
+if (FD1->getOverloadedOperator() != FD2->getOverloadedOperator())
+  return false;
+  }
   if (!::IsStructurallyEquivalent(FD1->getIdentifier(),
   FD2->getIdentifier()))
 return false;


Index: unittests/AST/StructuralEquivalenceTest.cpp
===
--- unittests/AST/StructuralEquivalenceTest.cpp
+++ unittests/AST/StructuralEquivalenceTest.cpp
@@ -230,6 +230,33 @@
   EXPECT_FALSE(testStructuralMatch(t));
 }
 
+TEST_F(StructuralEquivalenceFunctionTest, DifferentOperators) {
+  auto t = makeDecls(
+  "struct X{}; bool operator<(X, X);",
+  "struct X{}; bool operator==(X, X);", Lang_CXX,
+  functionDecl(hasOverloadedOperatorName("<")),
+  functionDecl(hasOverloadedOperatorName("==")));
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceFunctionTest, SameOperators) {
+  auto t = makeDecls(
+  "struct X{}; bool operator<(X, X);",
+  "struct X{}; bool operator<(X, X);", Lang_CXX,
+  functionDecl(hasOverloadedOperatorName("<")),
+  functionDecl(hasOverloadedOperatorName("<")));
+  EXPECT_TRUE(testStructuralMatch(t));
+}
+
+TEST_F(StructuralEquivalenceFunctionTest, CtorVsDtor) {
+  auto t = makeDecls(
+  "struct X{ X(); };",
+  "struct X{ ~X(); };", Lang_CXX,
+  cxxConstructorDecl(),
+  cxxDestructorDecl());
+  EXPECT_FALSE(testStructuralMatch(t));
+}
+
 TEST_F(StructuralEquivalenceFunctionTest, ParamConstWithRef) {
   auto t = makeNamedDecls("void foo(int&);",
   "void foo(const int&);", Lang_CXX);
Index: lib/AST/ASTStructuralEquivalence.cpp
===
--- lib/AST/ASTStructuralEquivalence.cpp
+++ lib/AST/ASTStructuralEquivalence.cpp
@@ -1650,6 +1650,12 @@
 }
   } else if (FunctionDecl *FD1 = dyn_cast(D1)) {
 if (FunctionDecl *FD2 = dyn_cast(D2)) {
+  if (FD1->isOverloadedOperator()) {
+if (!FD2->isOverloadedOperator())
+  return false;
+if (FD1->getOverloadedOperator() != FD2->getOverloadedOperator())
+  return false;
+  }
   if (!::IsStructurallyEquivalent(FD1->getIdentifier(),
   FD2->getIdentifier()))
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D57896: Variable names rule

2019-02-07 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings added a comment.

In D57896#1389042 , @lebedev.ri wrote:

> Pretty sure this patch should have gone to llvm-commits, not cfe-commits.


I just set the repository, Phabricator did the rest - apparently the magic 
isn't working so well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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


[PATCH] D57896: Variable names rule

2019-02-07 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

Pretty sure this patch should have gone to llvm-commits, not cfe-commits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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


[PATCH] D57901: [ASTImporter] Add test RedeclChainShouldBeCorrectAmongstNamespaces

2019-02-07 Thread Gabor Marton via Phabricator via cfe-commits
martong created this revision.
martong added reviewers: a_sidorin, shafik.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp, rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.

We add a new test to show that redecl chains are not handled properly
amongst namespaces. We cannot pass this test now, so this is disabled.
Subsequent patches will make this test pass.


Repository:
  rC Clang

https://reviews.llvm.org/D57901

Files:
  unittests/AST/ASTImporterTest.cpp


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -5083,6 +5083,47 @@
 ParameterizedTests, CanonicalRedeclChain,
 ::testing::Values(ArgVector()),);
 
+// FIXME This test is disabled currently, upcoming patches will make it
+// possible to enable.
+TEST_P(ASTImporterOptionSpecificTestBase,
+   DISABLED_RedeclChainShouldBeCorrectAmongstNamespaces) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  namespace NS {
+struct X;
+struct Y {
+  static const int I = 3;
+};
+  }
+  namespace NS {
+struct X {  // <--- To be imported
+  void method(int i = Y::I) {}
+  int f;
+};
+  }
+  )",
+  Lang_CXX);
+  auto *FromFwd = FirstDeclMatcher().match(
+  FromTU, cxxRecordDecl(hasName("X"), unless(isImplicit(;
+  auto *FromDef = LastDeclMatcher().match(
+  FromTU,
+  cxxRecordDecl(hasName("X"), isDefinition(), unless(isImplicit(;
+  ASSERT_NE(FromFwd, FromDef);
+  ASSERT_FALSE(FromFwd->isThisDeclarationADefinition());
+  ASSERT_TRUE(FromDef->isThisDeclarationADefinition());
+  ASSERT_EQ(FromFwd->getCanonicalDecl(), FromDef->getCanonicalDecl());
+
+  auto *ToDef = cast_or_null(Import(FromDef, Lang_CXX));
+  auto *ToFwd = cast_or_null(Import(FromFwd, Lang_CXX));
+  EXPECT_NE(ToFwd, ToDef);
+  EXPECT_FALSE(ToFwd->isThisDeclarationADefinition());
+  EXPECT_TRUE(ToDef->isThisDeclarationADefinition());
+  EXPECT_EQ(ToFwd->getCanonicalDecl(), ToDef->getCanonicalDecl());
+  auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  // We expect no (ODR) warning during the import.
+  EXPECT_EQ(0u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
+}
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
 DefaultTestValuesForRunOptions, );
 


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -5083,6 +5083,47 @@
 ParameterizedTests, CanonicalRedeclChain,
 ::testing::Values(ArgVector()),);
 
+// FIXME This test is disabled currently, upcoming patches will make it
+// possible to enable.
+TEST_P(ASTImporterOptionSpecificTestBase,
+   DISABLED_RedeclChainShouldBeCorrectAmongstNamespaces) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  namespace NS {
+struct X;
+struct Y {
+  static const int I = 3;
+};
+  }
+  namespace NS {
+struct X {  // <--- To be imported
+  void method(int i = Y::I) {}
+  int f;
+};
+  }
+  )",
+  Lang_CXX);
+  auto *FromFwd = FirstDeclMatcher().match(
+  FromTU, cxxRecordDecl(hasName("X"), unless(isImplicit(;
+  auto *FromDef = LastDeclMatcher().match(
+  FromTU,
+  cxxRecordDecl(hasName("X"), isDefinition(), unless(isImplicit(;
+  ASSERT_NE(FromFwd, FromDef);
+  ASSERT_FALSE(FromFwd->isThisDeclarationADefinition());
+  ASSERT_TRUE(FromDef->isThisDeclarationADefinition());
+  ASSERT_EQ(FromFwd->getCanonicalDecl(), FromDef->getCanonicalDecl());
+
+  auto *ToDef = cast_or_null(Import(FromDef, Lang_CXX));
+  auto *ToFwd = cast_or_null(Import(FromFwd, Lang_CXX));
+  EXPECT_NE(ToFwd, ToDef);
+  EXPECT_FALSE(ToFwd->isThisDeclarationADefinition());
+  EXPECT_TRUE(ToDef->isThisDeclarationADefinition());
+  EXPECT_EQ(ToFwd->getCanonicalDecl(), ToDef->getCanonicalDecl());
+  auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
+  // We expect no (ODR) warning during the import.
+  EXPECT_EQ(0u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
+}
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ASTImporterLookupTableTest,
 DefaultTestValuesForRunOptions, );
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

2019-02-07 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: clang-tidy/utils/ExceptionAnalyzer.h:31-192
+enum class ExceptionState : std::int8_t {
+  Throwing,///< The function can definitly throw given an AST.
+  NotThrowing, ///< This function can not throw, given an AST.
+  Unknown, ///< This can happen for extern functions without available
+   ///< definition.
+};
+

JonasToth wrote:
> lebedev.ri wrote:
> > Would it be better to add them into `ExceptionAnalyzer` class?
> > 
> `ThrownExceptions`? I think it fits here. Removing would shrink 
> `ExceptionInfo` (and `ContainsUnknown` and `State` could be packed into one 
> byte) which is a plus.
No, i meant all these classes/enums.
```
class ExceptionAnalyzer {
public:
enum class ExceptionState : std::int8_t {

}


class ExceptionInfo {

}

} // class ExceptionAnalyzer
```


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57883



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


[PATCH] D57768: [SYCL] Add SYCL device compilation flow.

2019-02-07 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In D57768#1386862 , @Anastasia wrote:

> - SYCL seem to require adding tight dependencies from the standard libraries 
> into the compiler because many language features are hidden behind library 
> classes. This is not common for Clang. We had a discussion about this issue 
> during the implementation of OpenCL C++ and it was decided not to go this 
> route for upstream Clang. Can you explain your current approach to implement 
> this? I think @rjmccall  or @rsmith might need to be involved in this.


I'd like to know more about this, but I'll point out that this isn't 
unprecedented:

- C compilers have hard-coded knowledge about `va_list`.
- C++ compilers have hard-coded knowledge about `std::type_info` and 
`std::initializer_list` (and possibly others I've forgotten).

Whether that's the right direction for SYCL, though, I can't say until I 
understand more about what dependencies are being proposed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768



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


  1   2   >