[PATCH] D59924: [PowerPC] [Clang] Port MMX intrinsics and basic test cases to Power

[Chaofan Qiu via Phabricator via cfe-commits]

qiucf updated this revision to Diff 194446.
qiucf added a comment.

Fix error about a mismatch in mmintrin test cases.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59924

Files:
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/PPCLinux.cpp
  clang/lib/Driver/ToolChains/PPCLinux.h
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/ppc_wrappers/mmintrin.h
  clang/test/CodeGen/ppc-mmintrin.c
  clang/test/Headers/ppc-intrinsics.c

Index: clang/test/Headers/ppc-intrinsics.c
===
--- /dev/null
+++ clang/test/Headers/ppc-intrinsics.c
@@ -0,0 +1,13 @@
+// REQUIRES: powerpc-registered-target
+
+// RUN: %clang -S -emit-llvm -DNO_WARN_X86_INTRINSICS -target powerpc64-gnu-linux %s -Xclang -verify -o - | FileCheck %s
+// RUN: %clang -S -emit-llvm -DNO_WARN_X86_INTRINSICS -target powerpc64-gnu-linux %s -Xclang -verify -x c++ -o - | FileCheck %s
+// expected-no-diagnostics
+
+// RUN: not %clang -S -emit-llvm -target powerpc64-gnu-linux %s -o /dev/null 2>&1 | FileCheck %s -check-prefix=CHECK-ERROR
+
+#include 
+// CHECK-ERROR: mmintrin.h:{{[0-9]+}}:{{[0-9]+}}: error: "Please read comment above. Use -DNO_WARN_X86_INTRINSICS to disable this error."
+
+// CHECK: target triple = "powerpc64-
+// CHECK: !llvm.module.flags =
Index: clang/test/CodeGen/ppc-mmintrin.c
===
--- /dev/null
+++ clang/test/CodeGen/ppc-mmintrin.c
@@ -0,0 +1,60 @@
+// REQUIRES: powerpc-registered-target
+
+// RUN: %clang -S -emit-llvm -DNO_WARN_X86_INTRINSICS -mcpu=pwr8 -target powerpc64-gnu-linux %s \
+// RUN:-mllvm -disable-llvm-optzns -o - | llvm-cxxfilt | FileCheck %s --check-prefixes=CHECK,CHECK-BE
+// RUN: %clang -S -emit-llvm -DNO_WARN_X86_INTRINSICS -mcpu=pwr8 -target powerpc64le-gnu-linux %s \
+// RUN:-mllvm -disable-llvm-optzns -o - | llvm-cxxfilt | FileCheck %s --check-prefixes=CHECK,CHECK-LE
+
+#include 
+
+unsigned long long int ull1, ull2;
+__m64 m1, m2, res;
+
+void __attribute__((noinline))
+test_packs() {
+  res = _mm_packs_pu16((__m64)ull1, (__m64)ull2);
+  res = _mm_packs_pi16((__m64)ull1, (__m64)ull2);
+  res = _mm_packs_pi32((__m64)ull1, (__m64)ull2);
+}
+
+// CHECK-LABEL: @test_packs
+
+// CHECK: define available_externally i64 @_mm_packs_pu16(i64 [[REG1:[0-9a-zA-Z_%.]+]], i64 [[REG2:[0-9a-zA-Z_%.]+]])
+// CHECK: store i64 [[REG1]], i64* [[REG3:[0-9a-zA-Z_%.]+]], align 8
+// CHECK-NEXT: store i64 [[REG2]], i64* [[REG4:[0-9a-zA-Z_%.]+]], align 8
+// CHECK-LE: load i64, i64* [[REG3]], align 8
+// CHECK: load i64, i64* [[REG4]], align 8
+// CHECK-BE: load i64, i64* [[REG3]], align 8
+// CHECK: [[REG5:[0-9a-zA-Z_%.]+]] = call <8 x i16> @vec_cmplt
+// CHECK-NEXT: store <8 x i16> [[REG5]], <8 x i16>* [[REG6:[0-9a-zA-Z_%.]+]], align 16
+// CHECK-NEXT: [[REG7:[0-9a-zA-Z_%.]+]] = load <8 x i16>, <8 x i16>* [[REG8:[0-9a-zA-Z_%.]+]], align 16
+// CHECK-NEXT: [[REG9:[0-9a-zA-Z_%.]+]] = load <8 x i16>, <8 x i16>* [[REG8]], align 16
+// CHECK-NEXT: [[REG10:[0-9a-zA-Z_%.]+]] = call <16 x i8> @vec_packs(unsigned short vector[8], unsigned short vector[8])(<8 x i16> [[REG7]], <8 x i16> [[REG9]])
+// CHECK-NEXT: store <16 x i8> [[REG10]], <16 x i8>* [[REG11:[0-9a-zA-Z_%.]+]], align 16
+// CHECK-NEXT: [[REG12:[0-9a-zA-Z_%.]+]] = load <8 x i16>, <8 x i16>* [[REG6]], align 16
+// CHECK-NEXT: [[REG13:[0-9a-zA-Z_%.]+]] = load <8 x i16>, <8 x i16>* [[REG6]], align 16
+// CHECK-NEXT: [[REG14:[0-9a-zA-Z_%.]+]] = call <16 x i8> @vec_pack(bool vector[8], bool vector[8])(<8 x i16> [[REG12]], <8 x i16> [[REG13]])
+// CHECK-NEXT: store <16 x i8> [[REG14]], <16 x i8>* [[REG15:[0-9a-zA-Z_%.]+]], align 16
+// CHECK-NEXT: [[REG16:[0-9a-zA-Z_%.]+]] = load <16 x i8>, <16 x i8>* [[REG11]], align 16
+// CHECK-NEXT: [[REG17:[0-9a-zA-Z_%.]+]] = load <16 x i8>, <16 x i8>* [[REG15]], align 16
+// CHECK-NEXT: call <16 x i8> @vec_sel(unsigned char vector[16], unsigned char vector[16], bool vector[16])(<16 x i8> [[REG16]], <16 x i8> zeroinitializer, <16 x i8> [[REG17]])
+
+// CHECK: define available_externally i64 @_mm_packs_pi16(i64 [[REG18:[0-9a-zA-Z_%.]+]], i64 [[REG19:[0-9a-zA-Z_%.]+]])
+// CHECK: store i64 [[REG18]], i64* [[REG20:[0-9a-zA-Z_%.]+]], align 8
+// CHECK-NEXT: store i64 [[REG19]], i64* [[REG21:[0-9a-zA-Z_%.]+]], align 8
+// CHECK-LE: load i64, i64* [[REG20]], align 8
+// CHECK: load i64, i64* [[REG21]], align 8
+// CHECK-BE: load i64, i64* [[REG20]], align 8
+// CHECK: [[REG22:[0-9a-zA-Z_%.]+]] = load <8 x i16>, <8 x i16>* [[REG23:[0-9a-zA-Z_%.]+]], align 16
+// CHECK-NEXT: [[REG24:[0-9a-zA-Z_%.]+]] = load <8 x i16>, <8 x i16>* [[REG23]], align 16
+// CHECK-NEXT: call <16 x i8> @vec_packs(short vector[8], short vector[8])(<8 x i16> [[REG22]], <8 x i16> [[REG24]])
+
+// CHECK: define available_externally i64 @_mm_packs_pi32(i64 [[REG25:[0-9a-zA-Z_%.]+]], i64 [[REG26:[0-9a-zA-Z_%.]+]])
+// CHECK: store 

[PATCH] D60161: Expose non-trivial struct helpers for Swift.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

Just one minor comment, but otherwise the patch looks good to me.




Comment at: clang/include/clang/CodeGen/CodeGenABITypes.h:92
+llvm::Function *generateNonTrivialCStructDefaultConstructor(CodeGenModule ,
+CharUnits 
Alignment,
+bool IsVolatile,

The parameter name is different from the one that is used in the function 
definition (`CharUnits DstAlignment`). Ditto for 
`generateNonTrivialCStructDestructor`.



Comment at: clang/lib/CodeGen/CGNonTrivialStruct.cpp:845
+  // actually use them (it overwrites them with the addresses of the arguments
+  // of the created function).
+  return Gen.getFunction(FuncName, QT, createNullAddressArray(), Alignments,

I think `getFunction` shouldn't require passing addresses, but that's not this 
patch's fault. I'll see if I can remove the parameter. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60161



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

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

Have a working prototype for aliases, but underneath, an option is an option, 
so should be able to support both without any trouble.  Will upload as soon as 
I clean it up and add tests, etc...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D59347: [DebugInfo] Combine Trivial and NonTrivial flags

[Aaron Smith via Phabricator via cfe-commits]

asmith marked 2 inline comments as done.
asmith added a comment.

This should be ready to commit


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

https://reviews.llvm.org/D59347



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

[PATCH] D59923: [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

> is that to imply that the first block all do not use split DWARF?

The first block do not use split DWARF.

> In a previous message I think you said that the only change was "-gmlt 
> -gsplit-dwarf -fno-split-dwarf-inlining => 1 (before) 2 (after)" - which I'm 
> not sure is an improvement.

Yes, this is the only behavioral change.

> You mentioned that the inconsistency between "-g0 -gsplit-dwarf" and "-gmlt 
> -gsplit-dwarf -fno-split-dwarf-inlining" was confusing. But still there will 
> be an inconsistency between "-gsplit-dwarf -g0" and "-gsplit-dwarf -gmlt 
> -fno-split-dwarf-inlining", yes?

The debug info level will be consistent after this patch:  the last of 
`-gsplit-dwarf -g0 -g1 -g2 -g3 -ggdb[0-3] -gdwarf-*` will decide the debug info 
level (`-gsplit-dwarf -gdwarf-*` have level 2). Next, a separate rule decides 
if the `-gsplit-dwarf` takes effect (not if `DebugInfoKind == 
codegenoptions::NoDebugInfo || DebugInfoKind == 
codegenoptions::DebugDirectivesOnly || (DebugInfoKind == 
codegenoptions::DebugLineTablesOnly && SplitDWARFInlining)`)

> I think that under -fno-split-dwarf-inlining, -gmlt and -gsplit-dwarf should 
> be order independent and compositional rather than overriding. Having them 
> compose in one order but not the other seems confusing to me.

The existence of `-fno-split-dwarf-inlining` changing the position dependence 
makes me confused:

- Without it, the latter of `-gmlt` and `-gsplit-dwarf` decides the debug info 
level
- With it, `-gmlt` decides the debug info level

I think the order of different options relative to one another should not 
matter, unless the options are documented as mutually-exclusive and such an 
option is documented to override any incompatible options preceding it. (POSIX 
Utility Syntax Guideline 11) Unfortunately GCC `-gsplit-dwarf` disobeys the 
guideline and makes the semantics not orthogonal. Considering the need to 
preserve its semantics, I think it'd be better to make `-gsplit-dwarf` 
consistently positional dependent, rather than make it sometimes positional 
dependent and sometimes (in the presence of `-fno-split-dwarf-inlining`) 
positional independent.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59923



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Adrian Prantl via Phabricator via cfe-commits]

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

Okay, so it looks like dexter might be a replacement for the lldgb.py wrapper 
and would support gdb, lldb, and the visual studio debugger. I think it would 
be nice to migrate the bulk of the debuginfo-tests repository to that. Until 
someone implements a cdb driver in dexter, however, I suppose there is no harm 
in adding a few files that directly talk to cdb. And I'm in no position to 
estimate how much work a cdb driver would be since I'm neither familiar with 
dexter or cdb :-)


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

https://reviews.llvm.org/D54187



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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang updated this revision to Diff 194435.
axzhang marked an inline comment as not done.

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

https://reviews.llvm.org/D55044

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-make-unique.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-make-unique.rst
  test/clang-tidy/abseil-make-unique.cpp

Index: test/clang-tidy/abseil-make-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-make-unique.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s abseil-make-unique %t -- -- -std=c++11 \
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr ) = delete;
+  unique_ptr(unique_ptr &) {}
+  ~unique_ptr() {}
+  type *() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr =(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr =(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+
+}  // namespace std
+
+class A {
+ int x;
+ int y;
+
+ public:
+   A(int _x, int _y): x(_x), y(_y) {}
+};
+
+struct Base {
+  Base();
+};
+
+struct Derived : public Base {
+  Derived();
+};
+
+int* returnPointer();
+void expectPointer(std::unique_ptr p);
+
+std::unique_ptr makeAndReturnPointer() {
+  return std::unique_ptr(new int(0));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: return absl::make_unique(0);
+}
+
+void Positives() {
+  std::unique_ptr P1 = std::unique_ptr(new int(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P1 = absl::make_unique(1);
+
+  P1.reset(new int(2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P1 = absl::make_unique(2);
+
+  // Non-primitive paramter
+  std::unique_ptr P2 = std::unique_ptr(new A(1, 2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P2 = absl::make_unique(1, 2);
+
+  P2.reset(new A(3, 4));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P2 = absl::make_unique(3, 4);
+
+  // No arguments to new expression
+  std::unique_ptr P3 = std::unique_ptr(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P3 = absl::make_unique();
+
+  P3.reset(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P3 = absl::make_unique();
+
+  // Nested parentheses
+  std::unique_ptr P4 = std::unique_ptr((new int(3)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: std::unique_ptr P4 = absl::make_unique(3);
+
+  P4 = std::unique_ptr(((new int(4;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P4 = absl::make_unique(4);
+
+  P4.reset((new int(5)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: P4 = absl::make_unique(5);
+
+  // With auto
+  auto P5 = std::unique_ptr(new int());
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: auto P5 = absl::make_unique();
+
+  {
+// No std
+using namespace std;
+unique_ptr Q = unique_ptr(new int());
+// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: use absl::make_unique instead [abseil-make-unique]
+// CHECK-FIXES: unique_ptr Q = absl::make_unique();
+
+Q = unique_ptr(new int());
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: use absl::make_unique instead [abseil-make-unique]
+// CHECK-FIXES: Q = absl::make_unique();
+  }
+
+  // Create the unique_ptr as a parameter to a function
+  expectPointer(std::unique_ptr(new int()));
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use absl::make_unique instead [abseil-make-unique]
+  // CHECK-FIXES: expectPointer(absl::make_unique());
+}
+
+void Negatives() {
+  // Only warn if explicitly allocating a new object
+  std::unique_ptr R = std::unique_ptr(returnPointer());
+  R.reset(returnPointer());
+
+  // Only replace if the template type is same as new type
+  auto Pderived = std::unique_ptr(new Derived());
+}
Index: docs/clang-tidy/checks/modernize-make-unique.rst
===
--- 

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

[Jan Korous via Phabricator via cfe-commits]

jkorous marked an inline comment as done.
jkorous added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:372
   D = adjustDeclToTemplate(D);
+  const Decl* CanonicalDecl = D->getCanonicalDecl();
 

arphaman wrote:
> Why are we now checking for the canonical declaration instead of `D` as 
> before?
Before this we were caching comment for every redeclaration separately but for 
every redeclaration the comment was the same.

As I understand it - for a given declaration we found the first comment in the 
redeclaration chain and then saved it to the cache for every redeclaration (the 
same comment).
Later, during lookup, we iterated the redeclaration chain again and did a 
lookup for every redeclaration (see L392 in the original implementation).

Unless I missed something, it's suboptimal in both memory consumption and 
runtime overhead.

That's the reason why I want to cache the comment for the redeclaration group 
as a whole. The only thing I am not entirely sure about is what key to use to 
represent the whole redeclaration chain - maybe the first declaration in the 
redeclaration chain would be better then the canonical declaration...

WDYT?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60432



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

[PATCH] D59673: [Driver] Allow setting the DWO name DWARF attribute separately

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

Ok, here is an idea. We introduce `-split-dwarf-output` in Clang instead of 
`-fsplit-dwarf-dwo-name-attr`. If given, it overrides the output file name for 
the Split DWARF file, which we otherwise take from `-split-dwarf-file`. The 
option is obviously incompatible with `-enable-split-dwarf=single`, so we will 
disallow that. This should be backwards-compatible, and bring the behavior of 
`llc` and `clang -cc1` closer together. What do you think?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59673



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

[PATCH] D40381: Parse concept definition

[Saar Raz via Phabricator via cfe-commits]

saar.raz updated this revision to Diff 194417.
saar.raz added a comment.
Herald added a subscriber: jfb.
Herald added a project: clang.

Address most of rsmith's CR comments, rebase onto trunk


Repository:
  rC Clang

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

https://reviews.llvm.org/D40381

Files:
  include/clang/AST/DeclTemplate.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/Basic/DeclNodes.td
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/TemplateKinds.h
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTDumper.cpp
  lib/AST/DeclBase.cpp
  lib/AST/DeclPrinter.cpp
  lib/AST/DeclTemplate.cpp
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CodeGenModule.cpp
  lib/Index/IndexDecl.cpp
  lib/Parse/ParseTemplate.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Serialization/ASTCommon.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriter.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
  test/Driver/Inputs/CUDA-nolibdevice/usr/local/cuda/bin/ptxas
  test/Driver/Inputs/CUDA-symlinks/opt/cuda/bin/ptxas
  test/Driver/Inputs/CUDA/usr/local/cuda/bin/ptxas
  test/Parser/cxx-concept-declaration.cpp
  test/Parser/cxx-concepts-ambig-constraint-expr.cpp
  test/Parser/cxx-concepts-requires-clause.cpp
  test/Parser/cxx2a-concept-declaration.cpp
  test/Parser/cxx2a-concepts-ambig-constraint-expr.cpp
  test/Parser/cxx2a-concepts-requires-clause.cpp
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -6252,6 +6252,7 @@
   case Decl::PragmaComment:
   case Decl::PragmaDetectMismatch:
   case Decl::UsingPack:
+  case Decl::Concept:
 return C;
 
   // Declaration kinds that don't make any sense here, but are
Index: test/Parser/cxx-concepts-requires-clause.cpp
===
--- /dev/null
+++ test/Parser/cxx-concepts-requires-clause.cpp
@@ -1,82 +0,0 @@
-// RUN: %clang_cc1 -std=c++14 -fconcepts-ts -x c++ %s -verify
-// expected-no-diagnostics
-
-// Test parsing of the optional requires-clause in a template-declaration.
-
-template  requires true
-void foo() { }
-
-
-template  requires !0
-struct A {
-  void foo();
-  struct AA;
-  enum E : int;
-  static int x;
-
-  template  requires true
-  void Mfoo();
-
-  template  requires true
-  struct M;
-
-  template  requires true
-  static int Mx;
-
-  template  requires true
-  using MQ = M;
-};
-
-template  requires !0
-void A::foo() { }
-
-template  requires !0
-struct A::AA { };
-
-template  requires !0
-enum A::E : int { E0 };
-
-template  requires !0
-int A::x = 0;
-
-template  requires !0
-template  requires true
-void A::Mfoo() { }
-
-template  requires !0
-template  requires true
-struct A::M { };
-
-template  requires !0
-template  requires true
-int A::Mx = 0;
-
-
-template  requires true
-int x = 0;
-
-template  requires true
-using Q = A;
-
-struct C {
-  template  requires true
-  void Mfoo();
-
-  template  requires true
-  struct M;
-
-  template  requires true
-  static int Mx;
-
-  template  requires true
-  using MQ = M;
-};
-
-template  requires true
-void C::Mfoo() { }
-
-template  requires true
-struct C::M { };
-
-template  requires true
-int C::Mx = 0;
Index: test/Parser/cxx-concepts-ambig-constraint-expr.cpp
===
--- /dev/null
+++ test/Parser/cxx-concepts-ambig-constraint-expr.cpp
@@ -1,29 +0,0 @@
-// RUN: %clang_cc1 -std=c++14 -fconcepts-ts -x c++ %s -verify
-
-// Test parsing of constraint-expressions in cases where the grammar is
-// ambiguous with the expectation that the longest token sequence which matches
-// the syntax is consumed without backtracking.
-
-// type-specifier-seq in conversion-type-id
-template  requires (bool)::operator short
-unsigned int foo(); // expected-error {{C++ requires a type specifier for all declarations}}
-
-// type-specifier-seq in new-type-id
-template  requires (bool)sizeof new (T::f()) short
-unsigned int bar(); // expected-error {{C++ requires a type specifier for all declarations}}
-
-template requires (bool)sizeof new (T::f()) unsigned // expected-error {{'struct' cannot be signed or unsigned}}
-struct X { }; // expected-error {{'X' cannot be defined in a type specifier}}
-
-// C-style cast
-// of function call on function-style cast
-template  requires (bool(T()))
-T (*fp)(); // expected-error {{use of undeclared identifier 'fp'}}
-
-// function-style cast
-// as the callee in a function call
-struct A {
-  static int t;
-  template  requires bool(T())
-  (A(T ())) { } // expected-error {{called object type 'bool' is not a 

[PATCH] D40381: Parse concept definition

[Saar Raz via Phabricator via cfe-commits]

saar.raz marked 21 inline comments as done.
saar.raz added a comment.

Only the TemplatedDecl issue remains, all other comments have been addressed 
and we're rebased onto master.
@rsmith please reply to the last comment, and lets finally start merging these 
:)


Repository:
  rC Clang

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

https://reviews.llvm.org/D40381



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

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Ronan Keryell via Phabricator via cfe-commits]

keryell added a comment.

While we are thinking about recycling some attributes across languages, we have 
to think about the fact that real applications are often made from various 
languages, such as SYCL + OpenMP 5 or whatever.
It would be nice not to forbid such a combination inside the same TU by design.




Comment at: clang/include/clang/Basic/Attr.td:1000
+def SYCLDevice : InheritableAttr {
+  let Spellings = [GNU<"sycl_device">];
+  let Subjects = SubjectList<[Function, Var]>;

Fznamznon wrote:
> aaron.ballman wrote:
> > Is there a reason to not also introduce a C++11 and C2x style spelling in 
> > the `clang` namespace? e.g., `[[clang::sycl_device]]`
> I don't think that it makes sense because these attributes not for public 
> consumption. These attributes is needed to separate code which is supposed to 
> be offloaded from regular host code. I think SYCLDevice attribute actually 
> doesn't need a spelling because it will be added only implicitly by compiler.

If we go towards this direction, `[[clang::sycl::device]]` or 
`[[clang::sycl::kernel]]` look more compatible with the concept of name space.
While not a public interface, if we have a kind of "standard" outlining in 
Clang/LLVM, some people might want to use it in some other contexts too.



Comment at: clang/include/clang/Basic/Attr.td:1003
+  let LangOpts = [SYCL];
+  let Documentation = [Undocumented];
+}

Fznamznon wrote:
> aaron.ballman wrote:
> > No new, undocumented attributes, please.
> As I said, these attributes are not for public consumption. Should I add 
> documentation in this case too?
Yes, documentation and comments are always appreciated.



Comment at: clang/include/clang/Basic/Attr.td:1010
+  let LangOpts = [SYCL];
+  let Documentation = [Undocumented];
+}

Cf supra.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D59923: [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

I'm still not entirely clear on how this is all changing, sorry - in the patch 
description summary, the first block of examples doesn't mention which of those 
disables split DWARF (the second block of examples all say "+ split" - is that 
to imply that the first block all do not use split DWARF?

In a previous message I think you said that the only change was "-gmlt 
-gsplit-dwarf -fno-split-dwarf-inlining => 1 (before) 2 (after)" - which I'm 
not sure is an improvement. You mentioned that the inconsistency between "-g0 
-gsplit-dwarf" and "-gmlt -gsplit-dwarf -fno-split-dwarf-inlining" was 
confusing. But still there will be an inconsistency between "-gsplit-dwarf -g0" 
and "-gsplit-dwarf -gmlt -fno-split-dwarf-inlining", yes?

I think that under -fno-split-dwarf-inlining, -gmlt and -gsplit-dwarf should be 
order independent and compositional rather than overriding. Having them compose 
in one order but not the other seems confusing to me.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59923



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

[PATCH] D59425: Explicitly Craft a Path to Compiler-RT Builtins on Bare Metal Targets

[Robert Widmann via Phabricator via cfe-commits]

CodaFi added a reviewer: phosek.
CodaFi added a subscriber: phosek.
CodaFi added a comment.

+@phosek Who seems to be the last significant contributor to this file.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59425



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

[PATCH] D60161: Expose non-trivial struct helpers for Swift.

[John McCall via Phabricator via cfe-commits]

rjmccall added a reviewer: ahatanak.
rjmccall added inline comments.
Herald added a subscriber: dexonsmith.



Comment at: clang/include/clang/CodeGen/CodeGenABITypes.h:140
+QualType QT);
+
 }  // end namespace CodeGen

Would you mind calling these `get...` rather than `generate...`?  It'd make it 
clearer that they will re-use an existing definition if possible.

Otherwise this looks great, thanks!  Akira, any comments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60161



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

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D60302#1460379 , @ahatanak wrote:

> Thanks. Does this require changes to swift too?


Yes, it will, to `getObjCRetainAutoreleasedReturnValueMarker` in `GenObjC.cpp`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60302



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

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

To give more context to the question, I'd like to clarify the use case of new 
attributes.

As @Fznamznon already mentioned in previous comments SYCL is not supposed to 
expose any non-standard extensions to a user.

Here is code example of the SYCL program, which demonstrate the need for these 
attributes:

  C++
  int foo(int x) { return ++x; }
  int bar(int x) { throw std::exception("CPU code only!"); }
  …
  using namespace cl::sycl;
  queue Q;
  buffer a(range<1>{1024});
  Q.submit([&](handler& cgh) {
auto A = a.get_access(cgh);
cgh.parallel_for(range<1>{1024}, [=](id<1> index) {
  A[index] = index[0] * 2 + index[1] + foo(42);
});
  }
  ...

SYCL compiler need to compile lambda function passed to 
`cl::sycl::handler::parallel_for` method and function `foo` called from this 
lambda function.

NOTE: compiler must ignore `bar` function when we "device" part of the single 
source code.

Our current approach is to add an attribute, which SYCL runtime will use to 
mark code passed to `cl::sycl::handler::parallel_for` as "kernel functions". 
Obviously runtime library can't mark `foo` as "device" code - this is a 
compiler job: to traverse all symbols accessible from kernel functions and add 
them to the "device part" of the code.

Here is a link to the code in the SYCL runtime using `sycl_kernel` attribute: 
https://github.com/intel/llvm/blob/sycl/sycl/include/CL/sycl/handler.hpp#L267

I'm quite sure something similar should happen for other "single source" 
programming models like OpenMP/CUDA, except these attributes are exposed to the 
user and there is a specific requirement on attributes/pragma/keyword names.

What we are looking for is whether we should add SYCL specific code or we can 
come up with something more generic to avoid logic/code duplication with 
already existing functionality.

BTW: Mariya, I think we might need to use `sycl_device` attribute to mark 
functions, which are called from the different translation units, i.e. compiler 
can't identify it w/o user's help.
SYCL specification proposes to use special macro as "device function marker", 
but I guess we can have additional "spellings" in the clang.

NOTE2: @Anastasia, https://reviews.llvm.org/D60454 makes impossible to replace 
`sycl_kernel` attribute with `__kernel` attribute. I mean we still can enable 
it for SYCL extension, but we will need SYCL specific customization in this 
case as we apply `kernel` attribute to a template function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D60485: [AArch64] Add support for MTE intrinsics

[Javed Absar via Phabricator via cfe-commits]

javed.absar created this revision.
javed.absar added reviewers: DavidSpickett, olista01.
Herald added a subscriber: kristof.beyls.

This patch provides intrinsics support for Memory Tagging Extension (MTE),

  which was introduced with the Armv8.5-a architecture.

These intrinsics are available when __ARM_FEATURE_MEMORY_TAGGING is defined.
Each intrinsic is described in detail in   the latest ACLE Q1 2019 
documentation:

  https://developer.arm.com/docs/101028/latest

However, below we also list the intrinsics:

1. T* __arm_mte_create_random_tag(T* src, uint64_t mask); This intrinsic 
returns a pointer containing a randomly created logical address tag.
2. T* __arm_mte_increment_tag(T* src, unsigned offset); This intrinsic returns 
a pointer which is a copy of the input pointer src but with the logical address 
tag part offset by a specified offset value.
3. uint64_t __arm_mte_exclude_tag(T* src, uint64_t excluded); This intrinsic 
adds a logical tag to the set of excluded logical tags.
4. void __arm_mte_set_tag(T* tag_address); This intrinsic stores an allocation 
tag, computed from the logical tag, to the tag memory thereby setting the 
allocation tag for the 16-byte granule of memory.
5. T* __arm_mte_get_tag(T* address); This intrinsic loads the allocation tag 
from tag memory and returns the corresponding logical tag as part of the 
returned pointer value.
6. ptrdiff_t __arm_mte_ptrdiff(T* a, T* b); The intrinsic calculates the 
difference between the address parts of the two pointers, ignoring the tags.


https://reviews.llvm.org/D60485

Files:
  include/clang/Basic/BuiltinsAArch64.def
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Basic/Targets/AArch64.cpp
  lib/Basic/Targets/AArch64.h
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/arm_acle.h
  lib/Sema/SemaChecking.cpp
  test/CodeGen/arm64-mte.c
  test/Preprocessor/aarch64-target-features.c
  test/Sema/builtins-arm64-mte.c
  test/Sema/builtins-arm64-mte2.cpp

Index: test/Sema/builtins-arm64-mte2.cpp
===
--- /dev/null
+++ test/Sema/builtins-arm64-mte2.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple arm64-arm-eabi %s -target-feature +mte -x c++ -fsyntax-only -verify
+#include 
+#include 
+
+ptrdiff_t subtract_pointers() {
+  // expected-error@+1 {{at least one argument must be a pointer ('nullptr_t', 'nullptr_t' invalid)}}
+  return __arm_mte_ptrdiff(nullptr, nullptr);
+}
Index: test/Sema/builtins-arm64-mte.c
===
--- /dev/null
+++ test/Sema/builtins-arm64-mte.c
@@ -0,0 +1,129 @@
+// RUN: %clang_cc1 -triple arm64-arm-eabi %s -target-feature +mte -fsyntax-only -verify
+// RUN: %clang_cc1 -triple arm64-arm-eabi %s -target-feature +mte -x c++ -fsyntax-only -verify
+#include 
+#include 
+
+int  *create_tag1(int a, unsigned b) {
+  // expected-error@+1 {{first argument must be a pointer ('int' invalid)}}
+  return __arm_mte_create_random_tag(a,b);
+}
+
+int  *create_tag2(int *a, unsigned *b) {
+  // expected-error@+1 {{second argument must be an integer type ('unsigned int *' invalid)}}
+  return __arm_mte_create_random_tag(a,b);
+}
+
+int  *create_tag3(const int *a, unsigned b) {
+#ifdef __cplusplus
+  // expected-error@+1 {{cannot initialize return object of type 'int *' with an rvalue of type 'const int *'}}
+  return __arm_mte_create_random_tag(a,b);
+#else
+  // expected-warning@+1 {{returning 'const int *' from a function with result type 'int *' discards qualifiers}}
+  return __arm_mte_create_random_tag(a,b);
+#endif
+}
+
+int  *create_tag4(volatile int *a, unsigned b) {
+#ifdef __cplusplus
+  // expected-error@+1 {{cannot initialize return object of type 'int *' with an rvalue of type 'volatile int *'}}
+  return __arm_mte_create_random_tag(a,b);
+#else
+  // expected-warning@+1 {{returning 'volatile int *' from a function with result type 'int *' discards qualifiers}}
+  return __arm_mte_create_random_tag(a,b);
+#endif
+}
+
+int  *increment_tag1(int *a, unsigned b) {
+  // expected-error@+1 {{argument to '__builtin_arm_addg' must be a constant integer}}
+  return __arm_mte_increment_tag(a,b);
+}
+
+int  *increment_tag2(int *a) {
+  // expected-error@+1 {{argument value 16 is outside the valid range [0, 15]}}
+  return __arm_mte_increment_tag(a,16);
+}
+
+int  *increment_tag3(int *a) {
+  // expected-error@+1 {{argument value -1 is outside the valid range [0, 15]}}
+  return __arm_mte_increment_tag(a,-1);
+}
+
+int  *increment_tag4(const int *a) {
+#ifdef __cplusplus
+  // expected-error@+1 {{cannot initialize return object of type 'int *' with an rvalue of type 'const int *'}}
+  return __arm_mte_increment_tag(a,5);
+#else
+  // expected-warning@+1 {{returning 'const int *' from a function with result type 'int *' discards qualifiers}}
+  return __arm_mte_increment_tag(a,5);
+#endif
+}
+
+int *increment_tag5(const volatile int *a) {
+#ifdef __cplusplus
+  // expected-error@+1 {{cannot 

[PATCH] D40381: Parse concept definition

[Saar Raz via Phabricator via cfe-commits]

saar.raz added inline comments.
Herald added a subscriber: arphaman.



Comment at: include/clang/AST/DeclTemplate.h:3035
+  SourceRange getSourceRange() const override LLVM_READONLY {
+return SourceRange(getLocation(), getLocation());
+  }

rsmith wrote:
> faisalv wrote:
> > why not just fix it now?
> >   return SourceRange(getTemplateParameters()->getTemplateLoc(), 
> > ConstraintExpr->getLocEnd(); 
> > 
> > ?
> There's a bigger problem here:
> 
> ```
> TemplateDecl *TD = /*some ConceptDecl*/;
> auto SR = TD->getSourceRange(); // crash
> ```
> 
> `TemplateDecl` has a hard assumption that it contains a `TemplatedDecl`. So, 
> three options:
> 
> 1) Change `TemplateDecl` and all its users to remove this assumption (hard 
> and leads to an awkward-to-use AST)
> 2) Add a `Decl` subclass that acts as the templated declaration in a concept 
> declaration (corresponding to the C++ grammar's //concept-definition// 
> production)
> 3) Make `ConceptDecl` //not// derive from a `TemplateDecl` at all
> 
> I think option 2 is my preference, but option 3 is also somewhat appealing 
> (there are other ways in which a concept is not a template: for instance, it 
> cannot be constrained, and it cannot be classified as either a type template 
> or a non-type template, because its kind depends on the context in which it 
> appears).
> 
> Of course, this leads to one of the Hard Problems Of Computer Science: naming 
> things. `ConceptDecl` for a //concept-definition// and `ConceptTemplateDecl` 
> for the //template-head concept-definition// would be consistent with the 
> rest of the AST. It's a little unfortunate for the longer name to be the AST 
> node that we actually interact with, but the consistency is probably worth 
> the cost.
The dummy decl is pretty confusing and will be a very weird decl in and of 
itself. I can't think of a good name right now, but your proposed naming will 
be pretty confusing given that a ConceptTemplateDecl does not template a 
concept declaration given the meaning of the phrase in the standard... Maybe 
ConceptBodyDecl? ConceptDummyDecl? ConceptDefinitionDecl? We need something 
that screams "this is not interesting" for the AST usage to be reasonable. 
Option 3 feels like loads of code duplication.
I'm not entirely sure how many blind (through TemplateDecl) uses of 
getTemplatedDecl there are, but there might not be that many, in which case the 
first option might be the lesser of all these evils.


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

https://reviews.llvm.org/D40381



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

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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

Thanks. Does this require changes to swift too?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60302



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

[PATCH] D59924: [PowerPC] [Clang] Port MMX intrinsics and basic test cases to Power

[Jinsong Ji via Phabricator via cfe-commits]

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

Please make sure you run all test before updating patch!




Comment at: clang/test/CodeGen/ppc-mmintrin.c:23
+// CHECK: i64 @_mm_packs_pu16(i64 [[REG1:[0-9a-zA-Z_%.]+]], i64 
[[REG2:[0-9a-zA-Z_%.]+]])
+// CHECK: store i64 [[REG1]], i64* [[REG3:[0-9a-zA-Z_%.]+]], align 8
+// CHECK-NEXT: store i64 [[REG2]], i64* [[REG4:[0-9a-zA-Z_%.]+]], align 8

.../clang/test/CodeGen/ppc-mmintrin.c:23:11: error: CHECK: expected string not 
found in input
// CHECK: store i64 [[REG1]], i64* [[REG3:[0-9a-zA-Z_%.]+]], align 8


Repository:
  rC Clang

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

https://reviews.llvm.org/D59924



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

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

[John McCall via Phabricator via cfe-commits]

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

Okay.  In that case, I have no objections to this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60302



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

[PATCH] D60454: [OpenCL] Prevent mangling kernel functions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I think it would be more reasonable to just change `getDeclLanguageLinkage` to 
check for a kernel function.


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

https://reviews.llvm.org/D60454



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

[PATCH] D60161: Expose non-trivial struct helpers for Swift.

[Tony Allevato via Phabricator via cfe-commits]

allevato updated this revision to Diff 194379.
allevato added a comment.

- Apply review changes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60161

Files:
  clang/include/clang/CodeGen/CodeGenABITypes.h
  clang/lib/CodeGen/CGNonTrivialStruct.cpp

Index: clang/lib/CodeGen/CGNonTrivialStruct.cpp
===
--- clang/lib/CodeGen/CGNonTrivialStruct.cpp
+++ clang/lib/CodeGen/CGNonTrivialStruct.cpp
@@ -14,6 +14,7 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "clang/AST/NonTrivialTypeVisitor.h"
+#include "clang/CodeGen/CodeGenABITypes.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include 
 
@@ -822,6 +823,30 @@
   Gen.callFunc(FuncName, QT, Addrs, CGF);
 }
 
+template  std::array createNullAddressArray();
+
+template <> std::array createNullAddressArray() {
+  return std::array({Address(nullptr, CharUnits::Zero())});
+}
+
+template <> std::array createNullAddressArray() {
+  return std::array({Address(nullptr, CharUnits::Zero()),
+ Address(nullptr, CharUnits::Zero())});
+}
+
+template 
+static llvm::Function *
+generateSpecialFunction(G &, StringRef FuncName, QualType QT,
+bool IsVolatile, std::array Alignments,
+CodeGenModule ) {
+  QT = IsVolatile ? QT.withVolatile() : QT;
+  // The following call requires an array of addresses as arguments, but doesn't
+  // actually use them (it overwrites them with the addresses of the arguments
+  // of the created function).
+  return Gen.getFunction(FuncName, QT, createNullAddressArray(), Alignments,
+ CGM);
+}
+
 // Functions to emit calls to the special functions of a non-trivial C struct.
 void CodeGenFunction::callCStructDefaultConstructor(LValue Dst) {
   bool IsVolatile = Dst.isVolatile();
@@ -907,3 +932,70 @@
   callSpecialFunction(GenMoveAssignment(getContext()), FuncName, QT, IsVolatile,
   *this, std::array({{DstPtr, SrcPtr}}));
 }
+
+llvm::Function *clang::CodeGen::generateNonTrivialCStructDefaultConstructor(
+CodeGenModule , CharUnits DstAlignment, bool IsVolatile, QualType QT) {
+  ASTContext  = CGM.getContext();
+  GenDefaultInitializeFuncName GenName(DstAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return generateSpecialFunction(
+  GenDefaultInitialize(Ctx), FuncName, QT, IsVolatile,
+  std::array({{DstAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::generateNonTrivialCStructCopyConstructor(
+CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT) {
+  ASTContext  = CGM.getContext();
+  GenBinaryFuncName GenName("__copy_constructor_", DstAlignment,
+   SrcAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return generateSpecialFunction(
+  GenCopyConstructor(Ctx), FuncName, QT, IsVolatile,
+  std::array({{DstAlignment, SrcAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::generateNonTrivialCStructMoveConstructor(
+CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT) {
+  ASTContext  = CGM.getContext();
+  GenBinaryFuncName GenName("__move_constructor_", DstAlignment,
+  SrcAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return generateSpecialFunction(
+  GenMoveConstructor(Ctx), FuncName, QT, IsVolatile,
+  std::array({{DstAlignment, SrcAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::generateNonTrivialCStructCopyAssignmentOperator(
+CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT) {
+  ASTContext  = CGM.getContext();
+  GenBinaryFuncName GenName("__copy_assignment_", DstAlignment,
+   SrcAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return generateSpecialFunction(
+  GenCopyAssignment(Ctx), FuncName, QT, IsVolatile,
+  std::array({{DstAlignment, SrcAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::generateNonTrivialCStructMoveAssignmentOperator(
+CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT) {
+  ASTContext  = CGM.getContext();
+  GenBinaryFuncName GenName("__move_assignment_", DstAlignment,
+  SrcAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return generateSpecialFunction(
+  GenMoveAssignment(Ctx), FuncName, QT, IsVolatile,
+  std::array({{DstAlignment, SrcAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::generateNonTrivialCStructDestructor(
+CodeGenModule , CharUnits Alignment, bool IsVolatile, QualType QT) {
+  ASTContext  = CGM.getContext();
+  GenDestructorFuncName 

[PATCH] D60186: Support CLANG_ENABLE_DEFAULT_PIE like gcc --enable-default-pie

[Jiang Yi via Phabricator via cfe-commits]

jiangyi added a comment.

Ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60186



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

[PATCH] D60139: [clang-tidy] Add bugprone-placement-new-target-type-mismatch check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:38
+  const CastExpr *Cast = dyn_cast(PlacementExpr);
+  if (nullptr == Cast) {
+return;

JonasToth wrote:
> braces, above the comment does not follow the correct-sentence style
I would put nullptr as second argument for == operator.


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

https://reviews.llvm.org/D60139



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

[PATCH] D60139: [clang-tidy] Add bugprone-placement-new-target-type-mismatch check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Hey Dennis,

my 2cents on the check. I think it is very good to have! Did you check coding 
guidelines if they say something to this issue? (e.g. cppcoreguidelines, hicpp, 
cert) As we have modules for them it would be great to make aliases to this 
check if they demand this to be checked.




Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:21
+void PlacementNewTargetTypeMismatch::registerMatchers(MatchFinder *Finder) {
+  // We only want the records that call 'new' with an adress parameter
+  Finder->addMatcher(cxxNewExpr(hasDescendant(castExpr())).bind("NewExpr"), 
this);

Please write full sentences with punctuation in comments.



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:22
+  // We only want the records that call 'new' with an adress parameter
+  Finder->addMatcher(cxxNewExpr(hasDescendant(castExpr())).bind("NewExpr"), 
this);
+}

placement-new? please make a new matcher (locally) to match that, because its 
is more specific.
You can use other checks as example, grep for "AST_MATCHER"



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:30
+
+  if (0 == NewExpr->getNumPlacementArgs()) {
+return;

Please ellide braces.



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:38
+  const CastExpr *Cast = dyn_cast(PlacementExpr);
+  if (nullptr == Cast) {
+return;

braces, above the comment does not follow the correct-sentence style



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:42
+
+  assert((Cast->getSubExpr()->getType()->isPointerType() ||
+ Cast->getSubExpr()->getType()->isArrayType()) &&

Is this universally true? What about the nothrow-overload, would that interfere?



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:45
+ "Cast of placement parameter requires a pointer or an array type");
+  const QualType  =
+  
Cast->getSubExpr()->getType()->getPointeeOrArrayElementType()->getCanonicalTypeInternal();

`QualType` is usually used a value-type, not const-ref. Please follow that for 
consistency.



Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:50
+
+  if (PlacementParameterType != AllocatedType) {
+diag(PlacementExpr->getExprLoc(), "placement new parameter and allocated 
type mismatch");

braces.



Comment at: 
docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst:7
+Finds placement-new calls where the pointer type of the adress mismatches the
+type of the created value.
+

Please add examples to demonstrate good and bad code for the user.



Comment at: test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp:5
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);

Please add a test for the nothrow overload.



Comment at: test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp:86
+  new (array) char('A');
+}

Please add test, where everything is hidden behind templates and only the 
type-substitution leads to the error.
A test including macros helps as well, to show they do not interfere (as should 
be the case with this check).


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

https://reviews.llvm.org/D60139



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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[Tiancong Wang via Phabricator via cfe-commits]

tcwang updated this revision to Diff 194371.
tcwang marked an inline comment as done.
tcwang added a comment.

Fix some comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/PPC.cpp
  clang/lib/Driver/ToolChains/Arch/PPC.h
  clang/test/Driver/aarch64-crypto.c
  clang/test/Driver/armv8-crypto.c

Index: clang/test/Driver/armv8-crypto.c
===
--- /dev/null
+++ clang/test/Driver/armv8-crypto.c
@@ -0,0 +1,18 @@
+// REQUIRES: arm-registered-target
+// RUN: %clang -target armv8 -mcrypto -mhard-float -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-V8-CRYPTO < %t %s
+// CHECK-V8-CRYPTO: "-target-feature" "+crypto"
+
+// RUN: %clang -target armv8 -mno-crypto -mhard-float -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-V8-NOCRYPTO < %t %s
+// CHECK-V8-NOCRYPTO: "-target-feature" "-crypto"
+
+// RUN: %clang -target armv8 -mcrypto -msoft-float -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-V8-CRYPTO-BEFORE-SOFT-FLOAT < %t %s
+// CHECK-V8-CRYPTO-BEFORE-SOFT-FLOAT: "-target-feature" "-crypto"
+// CHECK-V8-CRYPTO-BEFORE-SOFT-FLOAT-NOT: "-target-feature" "+crypto"
+
+// RUN: %clang -target armv8 -msoft-float -mcrypto -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-V8-CRYPTO-AFTER-SOFT-FLOAT < %t %s
+// CHECK-V8-CRYPTO-AFTER-SOFT-FLOAT: "-target-feature" "-crypto"
+// CHECK-V8-CRYPTO-AFTER-SOFT-FLOAT-NOT: "-target-feature" "+crypto"
Index: clang/test/Driver/aarch64-crypto.c
===
--- /dev/null
+++ clang/test/Driver/aarch64-crypto.c
@@ -0,0 +1,8 @@
+// REQUIRES: aarch64-registered-target
+// RUN: %clang -target aarch64 -mcrypto -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-AARCH64-CRYPTO < %t %s
+// CHECK-AARCH64-CRYPTO: "-target-feature" "+crypto"
+
+// RUN: %clang -target aarch64 -mno-crypto -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-AARCH64-NOCRYPTO < %t %s
+// CHECK-AARCH64-NOCRYPTO: "-target-feature" "-crypto"
Index: clang/lib/Driver/ToolChains/Arch/PPC.h
===
--- clang/lib/Driver/ToolChains/Arch/PPC.h
+++ clang/lib/Driver/ToolChains/Arch/PPC.h
@@ -22,6 +22,8 @@
 
 bool hasPPCAbiArg(const llvm::opt::ArgList , const char *Value);
 
+bool hasCryptoFeatureEnabled(const llvm::opt::ArgList );
+
 enum class FloatABI {
   Invalid,
   Soft,
Index: clang/lib/Driver/ToolChains/Arch/PPC.cpp
===
--- clang/lib/Driver/ToolChains/Arch/PPC.cpp
+++ clang/lib/Driver/ToolChains/Arch/PPC.cpp
@@ -109,6 +109,9 @@
   ppc::ReadGOTPtrMode ReadGOT = ppc::getPPCReadGOTPtrMode(D, Triple, Args);
   if (ReadGOT == ppc::ReadGOTPtrMode::SecurePlt)
 Features.push_back("+secure-plt");
+
+  if (ppc::hasCryptoFeatureEnabled(Args))
+Features.push_back("+crypto");
 }
 
 ppc::ReadGOTPtrMode ppc::getPPCReadGOTPtrMode(const Driver , const llvm::Triple ,
@@ -154,3 +157,10 @@
   Arg *A = Args.getLastArg(options::OPT_mabi_EQ);
   return A && (A->getValue() == StringRef(Value));
 }
+
+bool ppc::hasCryptoFeatureEnabled(const ArgList ) {
+  if (Arg *A = Args.getLastArg(options::OPT_mcrypto, options::OPT_mnocrypto))
+if (A->getOption().matches(options::OPT_mcrypto))
+  return true;
+  return false;
+}
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -448,6 +448,14 @@
   Features.push_back("-crc");
   }
 
+  // En/disable crypto
+  if (Arg *A = Args.getLastArg(options::OPT_mcrypto, options::OPT_mnocrypto)) {
+if (A->getOption().matches(options::OPT_mcrypto) && ABI != arm::FloatABI::Soft)
+  Features.push_back("+crypto");
+else
+  Features.push_back("-crypto");
+  }
+
   // For Arch >= ARMv8.0:  crypto = sha2 + aes
   // FIXME: this needs reimplementation after the TargetParser rewrite
   if (ArchName.find_lower("armv8a") != StringRef::npos ||
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -188,6 +188,15 @@
   if (!success)
 D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
 
+  // En/disable crypto
+  if (Arg *A = Args.getLastArg(options::OPT_mcrypto, options::OPT_mnocrypto,
+   options::OPT_mgeneral_regs_only)) {
+if (A->getOption().matches(options::OPT_mcrypto))
+  Features.push_back("+crypto");
+else
+  Features.push_back("-crypto");
+  }
+
   if 

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[Tiancong Wang via Phabricator via cfe-commits]

tcwang marked 4 inline comments as done.
tcwang added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:192
+  // En/disable crypto
+  if (Arg *A = Args.getLastArg(options::OPT_mcrypto, options::OPT_mnocrypto,
+   options::OPT_mgeneral_regs_only)) {

manojgupta wrote:
> I believe this should be merged with the code for OPT_mgeneral_regs_only 
> otherwise  the next if statement  for mgeneral-regs-only  would force 
> "-crypto" .
> 
> if (A->getOption().matches(OPT_mgeneral_regs_only)))
> ..// disable crypto, neon etc.
> else if (A->getOption().matches(options::OPT_mcrypto))
> // enable crypto
> ...
> 
> Please also add tests when mgeneral-regs-only is specified with "-mcrypto"  
> before/after.
Understand. My question of the logic is that if -mgeneral-regs-only comes 
before -mcrypto, do we want to set "-fp-armv8" "-neon" or not? In other words, 
if -mcrypto comes after -mgeneral-regs-only, does it only override '+crypto' or 
override all the three features? I'll change the logic and add tests after 
making sure what we really want to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

(Also, dexter has seen 7 commits over its lifetime, so I'm not sure that's 
something we'd want to depend on.)


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

https://reviews.llvm.org/D54187



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D54187#1460181 , @aprantl wrote:

> Did anyone take the time to look at dexter and whether it would fit the bill 
> here? It would be great to avoid reimplementing its functionality just to 
> have something that then only works on Windows.
>
> My position is that for the kind of very basic end-to-end testing that we do 
> in this repository (set breakpoints+print variables), it should be possible 
> to have one wrapper that supports multiple debuggers. I'm also fine with 
> adding platform-specific tests into a subdirectory, but that should be the 
> exception, not the rule. What I would like to avoid is growing a large 
> parallel body of basic tests that only work on one platform.
>
> It sounded like dexter would be the missing glue layer that supports windows 
> debuggers and unix debuggers alike. If that is feasible I would prefer to 
> extend dexter to support WinDbg and migrating debuginfo-tests to use dexter 
> over adding new code and more complexity to debuginfo-tests.


I just took a quick look. dexter is 7.3kLoC of python while this here is 50 
lines. Dexter has code to talk to visual studio over COM 
(dex/debugger/visualstudio/windows/ComInterface.py etc) but doesn't have a way 
to call cdb. Visual Studio is a way bigger dependency than cdb.

$ cd ~/src
$ git clone https://github.com/SNSystems/dexter.git
$ cd dexter
$ find dex -name '*.py' -type f | xargs wc -l | grep total

  7382 total

thakis@thakis:~/src/dexter$ find dex -name '*.py' -type f | xargs grep cdb

nothing
===

As zturner said above, we want explicit smoke testing for cdb. There's no 
desire to do all or even most of our debug info testing this way, but we'd like 
to have some integration tests for the debug engine we care about. dexter 
doesn't seem to help with this use case.


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

https://reviews.llvm.org/D54187



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

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

[ben via Phabricator via cfe-commits]

bd1976llvm marked 5 inline comments as done.
bd1976llvm added inline comments.



Comment at: lld/ELF/InputFiles.cpp:402
+  if (Config->DepLibs)
+if (fs::exists(Specifier))
+  Driver->addFile(Specifier, /*WithLOption=*/false);

jyknight wrote:
> bd1976llvm wrote:
> > jyknight wrote:
> > > bd1976llvm wrote:
> > > > jyknight wrote:
> > > > > This bit seems unfortunate. "-lfoo" won't search for a file named 
> > > > > "foo", and having this code do so does not seem like a good idea. I'd 
> > > > > want this feature to work just like -l.
> > > > > 
> > > > > Actually, I think it'd be better to preserve the "-l" prefix in the 
> > > > > deplibs section. (I'd note that even if libs in the search path are 
> > > > > spelled "-lfoo", that doesn't mean we need accept any other options)
> > > > > 
> > > > > If we want to support loading a filename which not on the search path 
> > > > > (but...do we, even? That seems like a kinda scary feature to me...), 
> > > > > then it'd be obvious how to spell that: "libfoo.a" and 
> > > > > "/baz/bar/foo.a" would open files directly, vs "-lfoo" and "-l:foo.a" 
> > > > > would search for them on the search path.
> > > > What you  have described is the behavior of the INPUT linker script 
> > > > command:  
> > > > https://sourceware.org/binutils/docs/ld/File-Commands.html#File-Commands.
> > > > 
> > > > We have carefully designed this "dependent libraries" feature to avoid 
> > > > mapping "comment lib" pragmas to --library command line options. My 
> > > > reasons:
> > > > 
> > > > - I don't want the compiler doing string processing to try to satisfy 
> > > > the command line parsing behaviour of the target linker.
> > > > 
> > > > - I don't want to couple the source code to a GNU-style linker. After 
> > > > all there are other ELF linkers. Your proposal isn't merely an ELF 
> > > > linking proposal, it's a proposal for ELF toolchains with GNU-like 
> > > > linkers (e.g. the arm linker doesn't support the colon prefix 
> > > > http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474c/Cjahbdei.html).
> > > > 
> > > > - The syntax is #pragma comment(lib, ...) not #pragma 
> > > > linker-option(library, ...) i.e. the only thing this (frankly rather 
> > > > bizarre) syntax definitely implies is that the argument is related to 
> > > > libraries (and comments ¯\_(ツ)_/¯); it is a bit of a stretch to 
> > > > interpret "comment lib" pragmas as mapping directly to "specifying an 
> > > > additional --library command line option".
> > > > 
> > > > - I want to avoid GNUism's and use a "general" mechanism. MSVC source 
> > > > code compatibility is a usecase.
> > > > 
> > > > - It is important to support loading a filename which not on the search 
> > > > path. For example I have seen developers use the following: #pragma 
> > > > comment(lib, __FILE__"\\..\\foo.lib")
> > > > 
> > > > I would like the following to work on for ELF:
> > > > 
> > > > #pragma comment(lib, "foo") => add libfoo.a to the link.
> > > > #pragma comment(lib, "foo.lib") => add foo.lib to the link
> > > > #pragma comment(lib, "c:\\foo.lib") => add c:\foo.lib to the link
> > > > 
> > > > The way the code is now these will work on both LLD and MSVC (and I 
> > > > think it will work for Apple's linker as well although I haven't 
> > > > checked). In addition, we have been careful to come up with a design 
> > > > that can be implemented by the GNU linkers... with the cost that some 
> > > > complicated links will not be possible to do via autolinking; in these 
> > > > (IMO rare) cases developers will need to use the command line directly 
> > > > instead.
> > > > 
> > > > What I am trying to accomplish is to try to keep #pragma comment(lib, 
> > > > "foo") compatible across linkers. Otherwise we will be in a situation 
> > > > where you will have to have the equivalent of...
> > > > 
> > > > #ifdef COFF_LIBRARIES:
> > > > #pragma comment(lib, "/DEFAULTLIB:foo.lib")
> > > > #'elif ELF_LIBRARIES_GNU_STYLE_LINKER:
> > > > #pragma comment(lib, "-lfoo")
> > > > #'elif DARWIN_FRAMEWORKS:
> > > > #pragma comment(lib, "-framework foo")
> > > > #esle
> > > > #error Please specify linking model
> > > > #endif
> > > > 
> > > > ... in your source code (or the moral equivalent somewhere else). We 
> > > > can avoid this.
> > > > 
> > > > Also note that I am not proposing to remove the .linker-options 
> > > > extension. We can add support for .linker-options to LLD in the future 
> > > > if we find a usecase where developers want pragmas that map directly to 
> > > > the linkers command line options for ELF. If this is a usecase then, in 
> > > > the future, we can implement support for #pragma comment(linker, ) 
> > > > pragmas that lower to .linker-options; #pragma comment(lib, ...) 
> > > > pragmas can continue to lower to .deplibs.
> > > It just seems to me a really bad idea to have a situation where 
> > > specifying `#pragma comment(lib, "foo")` can either open a 

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2218
 Group;
-def mpower8_crypto : Flag<["-"], "mcrypto">,
-Group;
-def mnopower8_crypto : Flag<["-"], "mno-crypto">,
-Group;
+def mcrypto : Flag<["-"], "mcrypto">, Group,
+HelpText<"Add use of cryptographic instructions (ARM/PowerPC only)">;

Can you move it out of ppc specific options area to more generic options 
location e.g. like hard-float?



Comment at: clang/include/clang/Driver/Options.td:2219
+def mcrypto : Flag<["-"], "mcrypto">, Group,
+HelpText<"Add use of cryptographic instructions (ARM/PowerPC only)">;
+def mnocrypto : Flag<["-"], "mno-crypto">, Group,

ARM/AArch64/PowerPC



Comment at: clang/include/clang/Driver/Options.td:2221
+def mnocrypto : Flag<["-"], "mno-crypto">, Group,
+HelpText<"Disallow use of cryptographic instructions (ARM/PowerPC only)">;
 def mdirect_move : Flag<["-"], "mdirect-move">,

ARM/AArch64/PowerPC



Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:192
+  // En/disable crypto
+  if (Arg *A = Args.getLastArg(options::OPT_mcrypto, options::OPT_mnocrypto,
+   options::OPT_mgeneral_regs_only)) {

I believe this should be merged with the code for OPT_mgeneral_regs_only 
otherwise  the next if statement  for mgeneral-regs-only  would force "-crypto" 
.

if (A->getOption().matches(OPT_mgeneral_regs_only)))
..// disable crypto, neon etc.
else if (A->getOption().matches(options::OPT_mcrypto))
// enable crypto
...

Please also add tests when mgeneral-regs-only is specified with "-mcrypto"  
before/after.



Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:453
+  if (Arg *A = Args.getLastArg(options::OPT_mcrypto, options::OPT_mnocrypto)) {
+if (A->getOption().matches(options::OPT_mcrypto) && ABI != 
arm::FloatABI::Soft)
+  Features.push_back("+crypto");

Please add a test for interaction with soft-float ABI option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Did anyone take the time to look at dexter and whether it would fit the bill 
here? It would be great to avoid reimplementing its functionality just to have 
something that then only works on Windows.

My position is that for the kind of very basic end-to-end testing that we do in 
this repository (set breakpoints+print variables), it should be possible to 
have one wrapper that supports multiple debuggers. I'm also fine with adding 
platform-specific tests into a subdirectory, but that should be the exception, 
not the rule. What I would like to avoid is growing a large parallel body of 
basic tests that only work on one platform.

It sounded like dexter would be the missing glue layer that supports windows 
debuggers and unix debuggers alike. If that is feasible I would prefer to 
extend dexter to support WinDbg and migrating debuginfo-tests to use dexter 
over adding new code and more complexity to debuginfo-tests.


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

https://reviews.llvm.org/D54187



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

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

In D60455#1459935 , @Anastasia wrote:

> I was just wondering since SYCL is intended to be a single source C++ for 
> OpenCL and this attribute is for internal use is it possible to just reuse 
> existing OpenCL kernel attribute?
>
> I have raised a couple of related questions on the earlier RFC btw but there 
> hasn't been any follow up since then I believe.


Good point. But in SYCL actually two types of functions that can be compiled 
for device: SYCL kernels and device functions. SYCL kernels are analog for 
OpenCL kernels, these are some functions that can be called from host. SYCL 
kernels should also have OpenCL kernel attribute to be OpenCL kernels. But if 
SYCL kernel calls any function - this function is compiled for device and 
called a “device function”. We need to detect it in compiler. And "device 
function" cannot be called from host and cannot be OpenCL kernel. I think there 
is no attribute for SYCL "device functions" in OpenCL because there is no 
"device functions".
So, I think I can reuse OpenCL kernel attribute (possible with __kernel 
keyword) for SYCL kernels but I still need SYCLDevice attribute to mark "device 
functions".
What do you think about it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

aprantl: Ping on Hans's question from Dec 18.


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

https://reviews.llvm.org/D54187



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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto.

[Tiancong Wang via Phabricator via cfe-commits]

tcwang created this revision.
Herald added subscribers: cfe-commits, jsji, kbarton, kristof.beyls, 
javed.absar, nemanjai.
Herald added a project: clang.
tcwang added reviewers: kristof.beyls, dlj, manojgupta.

This patch enhances Clang flags -mcrypto and -mno-crypto to enable/disable 
crypto feature.
Before this change, -m(no-)crypto is exclusively for Power8. We want to add 
this feature
to ARM/AArch64 as well. With -mcrypto is specified, '+crypto' will be passed as
a target feature to ARM/AArch64/Power8, and -mno-cypto will remove target 
feature '-crypto'.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60472

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/PPC.cpp
  clang/lib/Driver/ToolChains/Arch/PPC.h
  clang/test/Driver/aarch64-crypto.c
  clang/test/Driver/armv8-crypto.c

Index: clang/test/Driver/armv8-crypto.c
===
--- /dev/null
+++ clang/test/Driver/armv8-crypto.c
@@ -0,0 +1,8 @@
+// REQUIRES: arm-registered-target
+// RUN: %clang -target armv8 -mcrypto -mhard-float -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-V8-CRYPTO < %t %s
+// CHECK-V8-CRYPTO: "-target-feature" "+crypto"
+
+// RUN: %clang -target armv8 -mno-crypto -mhard-float -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-V8-NOCRYPTO < %t %s
+// CHECK-V8-NOCRYPTO: "-target-feature" "-crypto"
Index: clang/test/Driver/aarch64-crypto.c
===
--- /dev/null
+++ clang/test/Driver/aarch64-crypto.c
@@ -0,0 +1,8 @@
+// REQUIRES: aarch64-registered-target
+// RUN: %clang -target aarch64 -mcrypto -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-AARCH64-CRYPTO < %t %s
+// CHECK-AARCH64-CRYPTO: "-target-feature" "+crypto"
+
+// RUN: %clang -target aarch64 -mno-crypto -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-AARCH64-NOCRYPTO < %t %s
+// CHECK-AARCH64-NOCRYPTO: "-target-feature" "-crypto"
Index: clang/lib/Driver/ToolChains/Arch/PPC.h
===
--- clang/lib/Driver/ToolChains/Arch/PPC.h
+++ clang/lib/Driver/ToolChains/Arch/PPC.h
@@ -22,6 +22,8 @@
 
 bool hasPPCAbiArg(const llvm::opt::ArgList , const char *Value);
 
+bool hasCryptoFeatureEnabled(const llvm::opt::ArgList );
+
 enum class FloatABI {
   Invalid,
   Soft,
Index: clang/lib/Driver/ToolChains/Arch/PPC.cpp
===
--- clang/lib/Driver/ToolChains/Arch/PPC.cpp
+++ clang/lib/Driver/ToolChains/Arch/PPC.cpp
@@ -109,6 +109,9 @@
   ppc::ReadGOTPtrMode ReadGOT = ppc::getPPCReadGOTPtrMode(D, Triple, Args);
   if (ReadGOT == ppc::ReadGOTPtrMode::SecurePlt)
 Features.push_back("+secure-plt");
+
+  if (ppc::hasCryptoFeatureEnabled(Args))
+Features.push_back("+crypto");
 }
 
 ppc::ReadGOTPtrMode ppc::getPPCReadGOTPtrMode(const Driver , const llvm::Triple ,
@@ -154,3 +157,10 @@
   Arg *A = Args.getLastArg(options::OPT_mabi_EQ);
   return A && (A->getValue() == StringRef(Value));
 }
+
+bool ppc::hasCryptoFeatureEnabled(const ArgList ) {
+  if (Arg *A = Args.getLastArg(options::OPT_mcrypto, options::OPT_mnocrypto))
+if (A->getOption().matches(options::OPT_mcrypto))
+  return true;
+  return false;
+}
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -448,6 +448,14 @@
   Features.push_back("-crc");
   }
 
+  // En/disable crypto
+  if (Arg *A = Args.getLastArg(options::OPT_mcrypto, options::OPT_mnocrypto)) {
+if (A->getOption().matches(options::OPT_mcrypto) && ABI != arm::FloatABI::Soft)
+  Features.push_back("+crypto");
+else
+  Features.push_back("-crypto");
+  }
+
   // For Arch >= ARMv8.0:  crypto = sha2 + aes
   // FIXME: this needs reimplementation after the TargetParser rewrite
   if (ArchName.find_lower("armv8a") != StringRef::npos ||
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -188,6 +188,15 @@
   if (!success)
 D.Diag(diag::err_drv_clang_unsupported) << A->getAsString(Args);
 
+  // En/disable crypto
+  if (Arg *A = Args.getLastArg(options::OPT_mcrypto, options::OPT_mnocrypto,
+   options::OPT_mgeneral_regs_only)) {
+if (A->getOption().matches(options::OPT_mcrypto))
+  Features.push_back("+crypto");
+else
+  Features.push_back("-crypto");
+  }
+
   if (Args.getLastArg(options::OPT_mgeneral_regs_only)) {
 Features.push_back("-fp-armv8");
 Features.push_back("-crypto");
Index: clang/include/clang/Driver/Options.td

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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

The upgrader rewrites the named metadata into a module flag (see 
https://reviews.llvm.org/D60303).


Repository:
  rC Clang

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

https://reviews.llvm.org/D60302



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

r358016 - [OPENMP]Allow allocate directive on parameters.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Apr  9 09:31:37 2019
New Revision: 358016

URL: http://llvm.org/viewvc/llvm-project?rev=358016=rev
Log:
[OPENMP]Allow allocate directive on parameters.

Patch allows to use allocate directives on the function parameters.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/allocate_codegen.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=358016=358015=358016=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Tue Apr  9 09:31:37 2019
@@ -2361,9 +2361,6 @@ Sema::DeclGroupPtrTy Sema::ActOnOpenMPAl
 (VD->getStorageClass() == SC_Register && VD->hasAttr() &&
  !VD->isLocalVarDecl()))
   continue;
-// Do not apply for parameters.
-if (isa(VD))
-  continue;
 
 // If the used several times in the allocate directive, the same allocator
 // must be used.

Modified: cfe/trunk/test/OpenMP/allocate_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/allocate_codegen.cpp?rev=358016=358015=358016=diff
==
--- cfe/trunk/test/OpenMP/allocate_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/allocate_codegen.cpp Tue Apr  9 09:31:37 2019
@@ -91,4 +91,18 @@ int main () {
 
 // CHECK-NOT:  call {{.+}} {{__kmpc_alloc|__kmpc_free}}
 extern template int ST::m;
+
+// CHECK: define void @{{.+}}bar{{.+}}(i32 %{{.+}}, float* {{.+}})
+void bar(int a, float ) {
+// CHECK: [[A_VOID_PTR:%.+]] = call i8* @__kmpc_alloc(i32 [[GTID:%.+]], i64 4, 
i8* inttoptr (i64 1 to i8*))
+// CHECK: [[A_ADDR:%.+]] = bitcast i8* [[A_VOID_PTR]] to i32*
+// CHECK: store i32 %{{.+}}, i32* [[A_ADDR]],
+// CHECK: [[Z_VOID_PTR:%.+]] = call i8* @__kmpc_alloc(i32 [[GTID]], i64 8, i8* 
inttoptr (i64 1 to i8*))
+// CHECK: [[Z_ADDR:%.+]] = bitcast i8* [[Z_VOID_PTR]] to float**
+// CHECK: store float* %{{.+}}, float** [[Z_ADDR]],
+#pragma omp allocate(a,z) allocator(omp_default_mem_alloc)
+// CHECK: call void @__kmpc_free(i32 [[GTID]], i8* [[Z_VOID_PTR]], i8* 
inttoptr (i64 1 to i8*))
+// CHECK: call void @__kmpc_free(i32 [[GTID]], i8* [[A_VOID_PTR]], i8* 
inttoptr (i64 1 to i8*))
+// CHECK: ret void
+}
 #endif


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

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Anastasia Stulova via Phabricator via cfe-commits]

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

I was just wondering since SYCL is intended to be a single source C++ for 
OpenCL and this attribute is for internal use is it possible to just reuse 
existing OpenCL kernel attribute?

I have raised a couple of related questions on the earlier RFC btw but there 
hasn't been any follow up since then I believe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D54881#1449848 , @russellmcc wrote:

> klimek: I'm sorry, I don't fully understand your proposed fix.  Could you 
> explain it in more detail?
>
> Without being able to respond to your proposal in detail, I strongly believe 
> if you pass in a line range to clang-format, it should not change lines 
> outside that range OR we should modify the documentation to clearly explain 
> what the line filter option does.


That is a good point - generally, the problem is that one use case for 
clang-format is to give it some point in an expression, and have it fix that 
expression and currently "things around it that are off". I'm not saying that's 
the right solution, but if we want to change that it's a significant change to 
clang-format's workflow, so I think we'll need to carefully think about the 
effect this has, or provide reasons why we think this will not change those use 
cases.

For example, if I put {} around a range I typically use clang-format in some 
position in that range to fix up the whole range for me. This is a feature. I'm 
open to hearing more opinions, though.


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

https://reviews.llvm.org/D54881



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

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59746#1459906 , @hintonda wrote:

> In D59746#1459826 , @hintonda wrote:
>
> > In D59746#1459672 , @klimek wrote:
> >
> > > If we can extend cl::alias to support this, we could give it a priority 
> > > and take the highest prio :)
> >
> >
> > Okay, I'll give it that old college try, and see if I can come up with 
> > something not too kludgy.  ;-)
>
>
> Btw, do you just want default aliases, or default options in general?


I hope you don't mind, but I've got a few more design questions:

- should the default api be public, usable by Clang Tooling, or just usable 
within CommandLine.cpp?
- which options should be hard-coded, i.e., status quo, and which should be 
defaulted.
- should we provide some sort of diagnostic to user to alert them that they 
have overwritten a defaulted options -- note this has to happen at runtime.  
Guess it could be a help option, a la hidden-help, that shows status of each 
option. I actually like the idea of option origin in general.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D60323: [clangd] Include compile command inference in fileStatus API

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

What about surfacing compile command itself as well? IMO, it is an important 
information that we always extract from logs by digging currently.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60323



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

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59746#1459826 , @hintonda wrote:

> In D59746#1459672 , @klimek wrote:
>
> > In D59746#1458539 , @hintonda 
> > wrote:
> >
> > > In D59746#1458461 , @hintonda 
> > > wrote:
> > >
> > > > In D59746#1458432 , @klimek 
> > > > wrote:
> > > >
> > > > > If we make it an alias by default, can somebody overwrite that?
> > > >
> > > >
> > > > Unfortunately, that produces a runtime error:
> > > >
> > > >   lan1:/Users/dhinton/projects/llvm_project/monorepo/build/Debug $ 
> > > > bin/llvm-objdump -h
> > > >   : CommandLine Error: Option 'h' registered more than once!
> > > >   LLVM ERROR: inconsistency in registered CommandLine options
> > > >
> > > >
> > > > The operative lines:
> > > >
> > > >   llvm/tools/llvm-objdump/llvm-objdump.cpp:187:static cl::alias 
> > > > SectionHeadersShorter("h",
> > > >   llvm/lib/Support/CommandLine.cpp:2149:static cl::alias HOpA("h", 
> > > > cl::desc("Alias for -help"), cl::aliasopt(HOp));
> > > >
> > >
> > >
> > > The other problem is that these are statics, so you can't count on the 
> > > order, i.e., did the user overwrite get processed before or after the one 
> > > in `CommandLine.cpp`?
> >
> >
> > If we can extend cl::alias to support this, we could give it a priority and 
> > take the highest prio :)
>
>
> Okay, I'll give it that old college try, and see if I can come up with 
> something not too kludgy.  ;-)


Btw, do you just want default aliases, or default options in general?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:372
   D = adjustDeclToTemplate(D);
+  const Decl* CanonicalDecl = D->getCanonicalDecl();
 

Why are we now checking for the canonical declaration instead of `D` as before?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60432



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

[PATCH] D60456: [RISCV][WIP/RFC] Hard float ABI support

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Basic/Targets/RISCV.h:102
 // TODO: support lp64f and lp64d ABIs.
-if (Name == "lp64") {
+if (Name == "lp64" || Name == "lp64f" || Name == "lp64d") {
   ABI = Name;

You can remove the TODO here, assuming that this CC support is all that's 
necessary to support these ABIs.



Comment at: lib/CodeGen/TargetInfo.cpp:9223
+
+  bool IsInt = Ty->isIntegralOrEnumerationType();
+  bool IsFloat = Ty->isRealFloatingType();

Should this include pointers?  Pointers are often interchangeably with integers 
by ABIs.

The same question also applies to C++ references and data-member-pointer types, 
and maybe others that I'm not thinking of.



Comment at: lib/CodeGen/TargetInfo.cpp:9298
+return true;
+  }
+

This definitely needs to handle bit-fields in some way.



Comment at: lib/CodeGen/TargetInfo.cpp:9352
+  CharUnits::fromQuantity(getDataLayout().getTypeStoreSize(Field1Ty));
+  CharUnits Field2OffNoPadNoPack = std::max(Field2Align, Field1Size);
+

This should use `alignTo`, not `max`.  It's possible that they're equivalent 
for the narrow cases that can come out of all the checks above, but it's both 
clearer and safer to use the correct computation.



Comment at: lib/CodeGen/TargetInfo.cpp:9374
+
+  return ABIArgInfo::getCoerceAndExpand(CoerceToType, UnpaddedCoerceToType);
+}

This seems like a reasonable use of coerce-and-expand.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60456



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

[PATCH] D60465: [ASTImporter] Error handling fix in ImportDefinition_New.

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 194333.
balazske added a comment.

- Removed unneeded and wrong type cast.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60465

Files:
  lib/AST/ASTImporter.cpp


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -8382,17 +8382,16 @@
 }
 
 Error ASTImporter::ImportDefinition_New(Decl *From) {
-  Decl *To = Import(From);
-  if (!To)
-return llvm::make_error();
+  Decl *To;
+  if (Error Err = importInto(To, From))
+return Err;
 
-  auto *FromDC = cast(From);
   ASTNodeImporter Importer(*this);
 
   if (auto *ToRecord = dyn_cast(To)) {
 if (!ToRecord->getDefinition()) {
   return Importer.ImportDefinition(
-  cast(FromDC), ToRecord,
+  cast(From), ToRecord,
   ASTNodeImporter::IDK_Everything);
 }
   }
@@ -8400,14 +8399,14 @@
   if (auto *ToEnum = dyn_cast(To)) {
 if (!ToEnum->getDefinition()) {
   return Importer.ImportDefinition(
-  cast(FromDC), ToEnum, ASTNodeImporter::IDK_Everything);
+  cast(From), ToEnum, ASTNodeImporter::IDK_Everything);
 }
   }
 
   if (auto *ToIFace = dyn_cast(To)) {
 if (!ToIFace->getDefinition()) {
   return Importer.ImportDefinition(
-  cast(FromDC), ToIFace,
+  cast(From), ToIFace,
   ASTNodeImporter::IDK_Everything);
 }
   }
@@ -8415,12 +8414,15 @@
   if (auto *ToProto = dyn_cast(To)) {
 if (!ToProto->getDefinition()) {
   return Importer.ImportDefinition(
-  cast(FromDC), ToProto,
+  cast(From), ToProto,
   ASTNodeImporter::IDK_Everything);
 }
   }
 
-  return Importer.ImportDeclContext(FromDC, true);
+  if (auto *FromDC = dyn_cast(From))
+return Importer.ImportDeclContext(FromDC, true);
+
+  return Error::success();
 }
 
 void ASTImporter::ImportDefinition(Decl *From) {


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -8382,17 +8382,16 @@
 }
 
 Error ASTImporter::ImportDefinition_New(Decl *From) {
-  Decl *To = Import(From);
-  if (!To)
-return llvm::make_error();
+  Decl *To;
+  if (Error Err = importInto(To, From))
+return Err;
 
-  auto *FromDC = cast(From);
   ASTNodeImporter Importer(*this);
 
   if (auto *ToRecord = dyn_cast(To)) {
 if (!ToRecord->getDefinition()) {
   return Importer.ImportDefinition(
-  cast(FromDC), ToRecord,
+  cast(From), ToRecord,
   ASTNodeImporter::IDK_Everything);
 }
   }
@@ -8400,14 +8399,14 @@
   if (auto *ToEnum = dyn_cast(To)) {
 if (!ToEnum->getDefinition()) {
   return Importer.ImportDefinition(
-  cast(FromDC), ToEnum, ASTNodeImporter::IDK_Everything);
+  cast(From), ToEnum, ASTNodeImporter::IDK_Everything);
 }
   }
 
   if (auto *ToIFace = dyn_cast(To)) {
 if (!ToIFace->getDefinition()) {
   return Importer.ImportDefinition(
-  cast(FromDC), ToIFace,
+  cast(From), ToIFace,
   ASTNodeImporter::IDK_Everything);
 }
   }
@@ -8415,12 +8414,15 @@
   if (auto *ToProto = dyn_cast(To)) {
 if (!ToProto->getDefinition()) {
   return Importer.ImportDefinition(
-  cast(FromDC), ToProto,
+  cast(From), ToProto,
   ASTNodeImporter::IDK_Everything);
 }
   }
 
-  return Importer.ImportDeclContext(FromDC, true);
+  if (auto *FromDC = dyn_cast(From))
+return Importer.ImportDeclContext(FromDC, true);
+
+  return Error::success();
 }
 
 void ASTImporter::ImportDefinition(Decl *From) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55463: Introduce a source minimizer that reduces source to directives that might affect the dependency list for a compilation

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 194331.
arphaman added a comment.

Updated to the new LLVM license comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55463

Files:
  include/clang/Basic/DiagnosticFrontendKinds.td
  include/clang/Driver/CC1Options.td
  include/clang/Frontend/FrontendActions.h
  include/clang/Frontend/FrontendOptions.h
  include/clang/Lex/DependencyDirectivesSourceMinimizer.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/FrontendActions.cpp
  lib/FrontendTool/ExecuteCompilerInvocation.cpp
  lib/Lex/CMakeLists.txt
  lib/Lex/DependencyDirectivesSourceMinimizer.cpp
  test/Frontend/minimize_source_to_dependency_directives.c
  test/Frontend/minimize_source_to_dependency_directives_error.c
  unittests/Lex/CMakeLists.txt
  unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp

Index: unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
===
--- /dev/null
+++ unittests/Lex/DependencyDirectivesSourceMinimizerTest.cpp
@@ -0,0 +1,494 @@
+//===- unittests/Lex/DependencyDirectivesSourceMinimizer.cpp -  ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Lex/DependencyDirectivesSourceMinimizer.h"
+#include "llvm/ADT/SmallString.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+using namespace clang::minimize_source_to_dependency_directives;
+
+namespace clang {
+
+bool minimizeSourceToDependencyDirectives(StringRef Input,
+  SmallVectorImpl ) {
+  SmallVector Tokens;
+  return minimizeSourceToDependencyDirectives(Input, Out, Tokens);
+}
+
+} // end namespace clang
+
+namespace {
+
+TEST(MinimizeSourceToDependencyDirectivesTest, Empty) {
+  SmallVector Out;
+  SmallVector Tokens;
+
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives("", Out, Tokens));
+  EXPECT_TRUE(Out.empty());
+  ASSERT_EQ(1u, Tokens.size());
+  ASSERT_EQ(pp_eof, Tokens.back().K);
+
+  ASSERT_FALSE(
+  minimizeSourceToDependencyDirectives("abc def\nxyz", Out, Tokens));
+  EXPECT_TRUE(Out.empty());
+  ASSERT_EQ(1u, Tokens.size());
+  ASSERT_EQ(pp_eof, Tokens.back().K);
+}
+
+TEST(MinimizeSourceToDependencyDirectivesTest, AllTokens) {
+  SmallVector Out;
+  SmallVector Tokens;
+
+  ASSERT_FALSE(
+  minimizeSourceToDependencyDirectives("#define A\n"
+   "#undef A\n"
+   "#endif\n"
+   "#if A\n"
+   "#ifdef A\n"
+   "#ifndef A\n"
+   "#elif A\n"
+   "#else\n"
+   "#include \n"
+   "#include_next \n"
+   "#__include_macros \n"
+   "#import \n"
+   "@import A;\n"
+   "#pragma clang module import A\n",
+   Out, Tokens));
+  EXPECT_EQ(pp_define, Tokens[0].K);
+  EXPECT_EQ(pp_undef, Tokens[1].K);
+  EXPECT_EQ(pp_endif, Tokens[2].K);
+  EXPECT_EQ(pp_if, Tokens[3].K);
+  EXPECT_EQ(pp_ifdef, Tokens[4].K);
+  EXPECT_EQ(pp_ifndef, Tokens[5].K);
+  EXPECT_EQ(pp_elif, Tokens[6].K);
+  EXPECT_EQ(pp_else, Tokens[7].K);
+  EXPECT_EQ(pp_include, Tokens[8].K);
+  EXPECT_EQ(pp_include_next, Tokens[9].K);
+  EXPECT_EQ(pp___include_macros, Tokens[10].K);
+  EXPECT_EQ(pp_import, Tokens[11].K);
+  EXPECT_EQ(pp_at_import, Tokens[12].K);
+  EXPECT_EQ(pp_pragma_import, Tokens[13].K);
+  EXPECT_EQ(pp_eof, Tokens[14].K);
+}
+
+TEST(MinimizeSourceToDependencyDirectivesTest, Define) {
+  SmallVector Out;
+  SmallVector Tokens;
+
+  ASSERT_FALSE(
+  minimizeSourceToDependencyDirectives("#define MACRO", Out, Tokens));
+  EXPECT_STREQ("#define MACRO\n", Out.data());
+  ASSERT_EQ(2u, Tokens.size());
+  ASSERT_EQ(pp_define, Tokens.front().K);
+}
+
+TEST(MinimizeSourceToDependencyDirectivesTest, DefineSpacing) {
+  SmallVector Out;
+
+  ASSERT_FALSE(
+  minimizeSourceToDependencyDirectives("#define MACRO\n\n\n", Out));
+  EXPECT_STREQ("#define MACRO\n", Out.data());
+
+  ASSERT_FALSE(
+  minimizeSourceToDependencyDirectives("#define MACRO \n\n\n", Out));
+  EXPECT_STREQ("#define MACRO\n", Out.data());
+
+  ASSERT_FALSE(
+  minimizeSourceToDependencyDirectives("#define MACRO a \n\n\n", Out));
+  EXPECT_STREQ("#define MACRO a\n", Out.data());
+
+  ASSERT_FALSE(
+  

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: lld/ELF/InputFiles.cpp:402
+  if (Config->DepLibs)
+if (fs::exists(Specifier))
+  Driver->addFile(Specifier, /*WithLOption=*/false);

bd1976llvm wrote:
> jyknight wrote:
> > bd1976llvm wrote:
> > > jyknight wrote:
> > > > This bit seems unfortunate. "-lfoo" won't search for a file named 
> > > > "foo", and having this code do so does not seem like a good idea. I'd 
> > > > want this feature to work just like -l.
> > > > 
> > > > Actually, I think it'd be better to preserve the "-l" prefix in the 
> > > > deplibs section. (I'd note that even if libs in the search path are 
> > > > spelled "-lfoo", that doesn't mean we need accept any other options)
> > > > 
> > > > If we want to support loading a filename which not on the search path 
> > > > (but...do we, even? That seems like a kinda scary feature to me...), 
> > > > then it'd be obvious how to spell that: "libfoo.a" and "/baz/bar/foo.a" 
> > > > would open files directly, vs "-lfoo" and "-l:foo.a" would search for 
> > > > them on the search path.
> > > What you  have described is the behavior of the INPUT linker script 
> > > command:  
> > > https://sourceware.org/binutils/docs/ld/File-Commands.html#File-Commands.
> > > 
> > > We have carefully designed this "dependent libraries" feature to avoid 
> > > mapping "comment lib" pragmas to --library command line options. My 
> > > reasons:
> > > 
> > > - I don't want the compiler doing string processing to try to satisfy the 
> > > command line parsing behaviour of the target linker.
> > > 
> > > - I don't want to couple the source code to a GNU-style linker. After all 
> > > there are other ELF linkers. Your proposal isn't merely an ELF linking 
> > > proposal, it's a proposal for ELF toolchains with GNU-like linkers (e.g. 
> > > the arm linker doesn't support the colon prefix 
> > > http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0474c/Cjahbdei.html).
> > > 
> > > - The syntax is #pragma comment(lib, ...) not #pragma 
> > > linker-option(library, ...) i.e. the only thing this (frankly rather 
> > > bizarre) syntax definitely implies is that the argument is related to 
> > > libraries (and comments ¯\_(ツ)_/¯); it is a bit of a stretch to interpret 
> > > "comment lib" pragmas as mapping directly to "specifying an additional 
> > > --library command line option".
> > > 
> > > - I want to avoid GNUism's and use a "general" mechanism. MSVC source 
> > > code compatibility is a usecase.
> > > 
> > > - It is important to support loading a filename which not on the search 
> > > path. For example I have seen developers use the following: #pragma 
> > > comment(lib, __FILE__"\\..\\foo.lib")
> > > 
> > > I would like the following to work on for ELF:
> > > 
> > > #pragma comment(lib, "foo") => add libfoo.a to the link.
> > > #pragma comment(lib, "foo.lib") => add foo.lib to the link
> > > #pragma comment(lib, "c:\\foo.lib") => add c:\foo.lib to the link
> > > 
> > > The way the code is now these will work on both LLD and MSVC (and I think 
> > > it will work for Apple's linker as well although I haven't checked). In 
> > > addition, we have been careful to come up with a design that can be 
> > > implemented by the GNU linkers... with the cost that some complicated 
> > > links will not be possible to do via autolinking; in these (IMO rare) 
> > > cases developers will need to use the command line directly instead.
> > > 
> > > What I am trying to accomplish is to try to keep #pragma comment(lib, 
> > > "foo") compatible across linkers. Otherwise we will be in a situation 
> > > where you will have to have the equivalent of...
> > > 
> > > #ifdef COFF_LIBRARIES:
> > > #pragma comment(lib, "/DEFAULTLIB:foo.lib")
> > > #'elif ELF_LIBRARIES_GNU_STYLE_LINKER:
> > > #pragma comment(lib, "-lfoo")
> > > #'elif DARWIN_FRAMEWORKS:
> > > #pragma comment(lib, "-framework foo")
> > > #esle
> > > #error Please specify linking model
> > > #endif
> > > 
> > > ... in your source code (or the moral equivalent somewhere else). We can 
> > > avoid this.
> > > 
> > > Also note that I am not proposing to remove the .linker-options 
> > > extension. We can add support for .linker-options to LLD in the future if 
> > > we find a usecase where developers want pragmas that map directly to the 
> > > linkers command line options for ELF. If this is a usecase then, in the 
> > > future, we can implement support for #pragma comment(linker, ) 
> > > pragmas that lower to .linker-options; #pragma comment(lib, ...) pragmas 
> > > can continue to lower to .deplibs.
> > It just seems to me a really bad idea to have a situation where specifying 
> > `#pragma comment(lib, "foo")` can either open a file named "foo" in the 
> > current directory, or search for libfoo.{a,so} in the library search path.
> > 
> > That is pretty much guaranteed to cause problems due to unintentional 
> > filename collision in the current-directory.
> > 
> Linkers 

[PATCH] D60465: [ASTImporter] Error handling fix in ImportDefinition_New.

[Balázs Kéri via Phabricator via cfe-commits]

balazske created this revision.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp.
Herald added a reviewer: martong.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: clang.

ASTImporter::ImportDefinition_New contained a call to Import
that should be Import_New now. This is fixed now.


Repository:
  rC Clang

https://reviews.llvm.org/D60465

Files:
  lib/AST/ASTImporter.cpp


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -8382,45 +8382,48 @@
 }
 
 Error ASTImporter::ImportDefinition_New(Decl *From) {
-  Decl *To = Import(From);
-  if (!To)
-return llvm::make_error();
+  Decl *To;
+  if (Error Err = importInto(To, From))
+return Err;
 
-  auto *FromDC = cast(From);
-  ASTNodeImporter Importer(*this);
+  if (auto *FromDC = cast(From)) {
+ASTNodeImporter Importer(*this);
 
-  if (auto *ToRecord = dyn_cast(To)) {
-if (!ToRecord->getDefinition()) {
-  return Importer.ImportDefinition(
-  cast(FromDC), ToRecord,
-  ASTNodeImporter::IDK_Everything);
+if (auto *ToRecord = dyn_cast(To)) {
+  if (!ToRecord->getDefinition()) {
+return Importer.ImportDefinition(
+cast(FromDC), ToRecord,
+ASTNodeImporter::IDK_Everything);
+  }
 }
-  }
 
-  if (auto *ToEnum = dyn_cast(To)) {
-if (!ToEnum->getDefinition()) {
-  return Importer.ImportDefinition(
-  cast(FromDC), ToEnum, ASTNodeImporter::IDK_Everything);
+if (auto *ToEnum = dyn_cast(To)) {
+  if (!ToEnum->getDefinition()) {
+return Importer.ImportDefinition(
+cast(FromDC), ToEnum, ASTNodeImporter::IDK_Everything);
+  }
 }
-  }
 
-  if (auto *ToIFace = dyn_cast(To)) {
-if (!ToIFace->getDefinition()) {
-  return Importer.ImportDefinition(
-  cast(FromDC), ToIFace,
-  ASTNodeImporter::IDK_Everything);
+if (auto *ToIFace = dyn_cast(To)) {
+  if (!ToIFace->getDefinition()) {
+return Importer.ImportDefinition(
+cast(FromDC), ToIFace,
+ASTNodeImporter::IDK_Everything);
+  }
 }
-  }
 
-  if (auto *ToProto = dyn_cast(To)) {
-if (!ToProto->getDefinition()) {
-  return Importer.ImportDefinition(
-  cast(FromDC), ToProto,
-  ASTNodeImporter::IDK_Everything);
+if (auto *ToProto = dyn_cast(To)) {
+  if (!ToProto->getDefinition()) {
+return Importer.ImportDefinition(
+cast(FromDC), ToProto,
+ASTNodeImporter::IDK_Everything);
+  }
 }
+
+return Importer.ImportDeclContext(FromDC, true);
   }
 
-  return Importer.ImportDeclContext(FromDC, true);
+  return Error::success();
 }
 
 void ASTImporter::ImportDefinition(Decl *From) {


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -8382,45 +8382,48 @@
 }
 
 Error ASTImporter::ImportDefinition_New(Decl *From) {
-  Decl *To = Import(From);
-  if (!To)
-return llvm::make_error();
+  Decl *To;
+  if (Error Err = importInto(To, From))
+return Err;
 
-  auto *FromDC = cast(From);
-  ASTNodeImporter Importer(*this);
+  if (auto *FromDC = cast(From)) {
+ASTNodeImporter Importer(*this);
 
-  if (auto *ToRecord = dyn_cast(To)) {
-if (!ToRecord->getDefinition()) {
-  return Importer.ImportDefinition(
-  cast(FromDC), ToRecord,
-  ASTNodeImporter::IDK_Everything);
+if (auto *ToRecord = dyn_cast(To)) {
+  if (!ToRecord->getDefinition()) {
+return Importer.ImportDefinition(
+cast(FromDC), ToRecord,
+ASTNodeImporter::IDK_Everything);
+  }
 }
-  }
 
-  if (auto *ToEnum = dyn_cast(To)) {
-if (!ToEnum->getDefinition()) {
-  return Importer.ImportDefinition(
-  cast(FromDC), ToEnum, ASTNodeImporter::IDK_Everything);
+if (auto *ToEnum = dyn_cast(To)) {
+  if (!ToEnum->getDefinition()) {
+return Importer.ImportDefinition(
+cast(FromDC), ToEnum, ASTNodeImporter::IDK_Everything);
+  }
 }
-  }
 
-  if (auto *ToIFace = dyn_cast(To)) {
-if (!ToIFace->getDefinition()) {
-  return Importer.ImportDefinition(
-  cast(FromDC), ToIFace,
-  ASTNodeImporter::IDK_Everything);
+if (auto *ToIFace = dyn_cast(To)) {
+  if (!ToIFace->getDefinition()) {
+return Importer.ImportDefinition(
+cast(FromDC), ToIFace,
+ASTNodeImporter::IDK_Everything);
+  }
 }
-  }
 
-  if (auto *ToProto = dyn_cast(To)) {
-if (!ToProto->getDefinition()) {
-  return Importer.ImportDefinition(
-  cast(FromDC), ToProto,
-  ASTNodeImporter::IDK_Everything);
+if (auto *ToProto = dyn_cast(To)) {
+  if (!ToProto->getDefinition()) {
+return Importer.ImportDefinition(
+cast(FromDC), ToProto,
+

[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

[ben via Phabricator via cfe-commits]

bd1976llvm marked 25 inline comments as done.
bd1976llvm added a comment.

I have updated the diff to address review comments. I think we can continue to 
refine this patch in parallel with discussing the design further (I am not 
dismissing the concerns raised by @compnerd and @jyknight).

I am unsure if I should be doing validation on the structure of the 
llvm.dependent-libraries named metadata e.g. what if there are zero operands or 
more than one operand for a metadata node. Does anyone know what  LLVM's policy 
is?




Comment at: lld/ELF/InputFiles.cpp:406
+  Driver->addFile(*S, /*WithLOption=*/true);
+else if (Optional S = searchLibrary(Specifier))
+  Driver->addFile(*S, /*WithLOption=*/true);

pcc wrote:
> `searchLibrary` contains the support for handling flags like `-l:foo`; I 
> guess if you don't want that behaviour here then it will need to be factored 
> out of `searchLibrary`.
I was undecided here. Refactoring the code to remove the colon prefix behavior 
and adding other checks like only allowing libraries (not objects) to be added 
to the link adds complexity that might not be justified. Maybe we should simply 
document that some behaviors might work but are unsupported and liable to 
change without warning?



Comment at: lld/ELF/InputFiles.cpp:657
+  CHECK(this->getObj().getSectionContents(), this);
+  for (const uint8_t *P = Contents.begin(), *E = Contents.end(); P < E;) {
+StringRef A = reinterpret_cast(P);

ruiu wrote:
> Instead of allowing multiple names in a single .autolink section, why don't 
> you create multiple .autolink sections?
This would simplify the implementation but I don't think that the size 
increases in the input files is worth it. Also, it is easier to dump the 
dependent libraries if they are all in one section.



Comment at: llvm/include/llvm/BinaryFormat/ELF.h:841
 // for safe ICF.
+  SHT_LLVM_DEPLIBS = 0x6fff4c04,// LLVM Dependent Library Specifiers.
   // Android's experimental support for SHT_RELR sections.

peter.smith wrote:
> pcc wrote:
> > peter.smith wrote:
> > > This is the same value as has been proposed in another ELF extension in 
> > > https://reviews.llvm.org/D60242 , May be worth coordinating here. It is 
> > > probably worth us having a central place to document the LLVM specific 
> > > ELF extensions as we are accumulating enough of them.
> > I don't have any existing dependencies on the value that I've chosen. If 
> > Ben has no dependencies of his own, I reckon that whoever ends up 
> > committing first can just get this value, and the next person will get the 
> > next one. I think this can be our general policy.
> > 
> > Do you think that https://llvm.org/docs/Extensions.html#elf-dependent is 
> > not sufficient as a place to document our extensions?
> I think it could be, now that I see it fully expanding with some of the 
> extensions mentioning the elf terminology. I think we should mention the 
> SHT_... and hex value so that someone doesn't have to dig for that in the 
> source code.
I think that it is better if the hex value is in this one place and we use the 
SHT_ token everywhere else. This reduces the potential for mistakes.



Comment at: llvm/include/llvm/Object/IRSymtab.h:157
+/// Dependent Library Specifiers
+  Range DepLibs;
 };

pcc wrote:
> Maybe just `Range`?
Thanks! I went for the struct with a single element representation because that 
is used for comdats. Should we make this same change for comdats?


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

https://reviews.llvm.org/D60274



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

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1000
+def SYCLDevice : InheritableAttr {
+  let Spellings = [GNU<"sycl_device">];
+  let Subjects = SubjectList<[Function, Var]>;

aaron.ballman wrote:
> Is there a reason to not also introduce a C++11 and C2x style spelling in the 
> `clang` namespace? e.g., `[[clang::sycl_device]]`
I don't think that it makes sense because these attributes not for public 
consumption. These attributes is needed to separate code which is supposed to 
be offloaded from regular host code. I think SYCLDevice attribute actually 
doesn't need a spelling because it will be added only implicitly by compiler.



Comment at: clang/include/clang/Basic/Attr.td:1003
+  let LangOpts = [SYCL];
+  let Documentation = [Undocumented];
+}

aaron.ballman wrote:
> No new, undocumented attributes, please.
As I said, these attributes are not for public consumption. Should I add 
documentation in this case too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59746#1459672 , @klimek wrote:

> In D59746#1458539 , @hintonda wrote:
>
> > In D59746#1458461 , @hintonda 
> > wrote:
> >
> > > In D59746#1458432 , @klimek 
> > > wrote:
> > >
> > > > If we make it an alias by default, can somebody overwrite that?
> > >
> > >
> > > Unfortunately, that produces a runtime error:
> > >
> > >   lan1:/Users/dhinton/projects/llvm_project/monorepo/build/Debug $ 
> > > bin/llvm-objdump -h
> > >   : CommandLine Error: Option 'h' registered more than once!
> > >   LLVM ERROR: inconsistency in registered CommandLine options
> > >
> > >
> > > The operative lines:
> > >
> > >   llvm/tools/llvm-objdump/llvm-objdump.cpp:187:static cl::alias 
> > > SectionHeadersShorter("h",
> > >   llvm/lib/Support/CommandLine.cpp:2149:static cl::alias HOpA("h", 
> > > cl::desc("Alias for -help"), cl::aliasopt(HOp));
> > >
> >
> >
> > The other problem is that these are statics, so you can't count on the 
> > order, i.e., did the user overwrite get processed before or after the one 
> > in `CommandLine.cpp`?
>
>
> If we can extend cl::alias to support this, we could give it a priority and 
> take the highest prio :)


Okay, I'll give it that old college try, and see if I can come up with 
something not too kludgy.  ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D60463: [ASTImporter] Add check for correct import of source locations.

[Balázs Kéri via Phabricator via cfe-commits]

balazske created this revision.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp.
Herald added a reviewer: martong.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: clang.

Unit tests extended with a new check for source locations.
Currently a simple match check in the node text dumps is done.
The new check is disabled for now, until the failures are fixed.


Repository:
  rC Clang

https://reviews.llvm.org/D60463

Files:
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -29,6 +29,8 @@
 #include "gmock/gmock.h"
 #include "llvm/ADT/StringMap.h"
 
+#include 
+
 namespace clang {
 namespace ast_matchers {
 
@@ -56,6 +58,81 @@
llvm::MemoryBuffer::getMemBuffer(Code));
 }
 
+void checkImportedSourceLocations(const Decl *FromD, const Decl *ToD) {
+  // FIXME: Remove this line when all failures are fixed.
+  return;
+
+  // Check for matching source locations in From and To AST.
+  // FIXME: The check can be improved by using AST visitor and manually check
+  // all source locations for equality.
+  // (That check can be made more general by checking for other attributes.)
+
+  // Print debug information.
+  const bool Print = false;
+
+  SmallString<1024> ToPrinted;
+  SmallString<1024> FromPrinted;
+  llvm::raw_svector_ostream ToStream(ToPrinted);
+  llvm::raw_svector_ostream FromStream(FromPrinted);
+
+  ToD->dump(ToStream);
+  FromD->dump(FromStream);
+  // The AST dump additionally traverses the AST and can catch certain bugs like
+  // poorly or not implemented subtrees.
+
+  // search for SourceLocation strings:
+  // ::
+  // or
+  // line::
+  // or
+  // col:
+  // or
+  // ''
+  // If a component (filename or line) is same as in the last location
+  // it is not printed.
+  // Filename component is grouped into sub-expression to make it extractable.
+  std::regex MatchSourceLoc(
+  "|((\\w|\\.)+):\\d+:\\d+|line:\\d+:\\d+|col:\\d+");
+
+  std::string ToString(ToStream.str());
+  std::string FromString(FromStream.str());
+  auto ToLoc =
+  std::sregex_iterator(ToString.begin(), ToString.end(), MatchSourceLoc);
+  auto FromLoc = std::sregex_iterator(FromString.begin(), FromString.end(),
+  MatchSourceLoc);
+  if (Print) {
+llvm::errs() << ToString << "\n\n\n" << FromString << "\n";
+llvm::errs() << "\n";
+  }
+  if (ToLoc->size() > 1 && FromLoc->size() > 1 && (*ToLoc)[1] != (*FromLoc)[1])
+// Different filenames in To and From.
+// This should mean that a to-be-imported decl was mapped to an existing
+// (these normally reside in different files) and the check is
+// not applicable.
+return;
+
+  bool SourceLocationMismatch = false;
+  while (ToLoc != std::sregex_iterator() && FromLoc != std::sregex_iterator()) {
+if (Print)
+  llvm::errs() << ToLoc->str() << "|" << FromLoc->str() << "\n";
+SourceLocationMismatch =
+SourceLocationMismatch || (ToLoc->str() != FromLoc->str());
+++ToLoc;
+++FromLoc;
+  }
+  if (Print)
+llvm::errs() << "\n";
+
+  // If the from AST is bigger it may have a matching prefix, ignore this case:
+  // ToLoc == std::sregex_iterator() && FromLoc != std::sregex_iterator()
+
+  // If the To AST is bigger (or has more source locations), indicate error.
+  if (FromLoc == std::sregex_iterator() && ToLoc != std::sregex_iterator())
+SourceLocationMismatch = true;
+
+  EXPECT_FALSE(SourceLocationMismatch);
+}
+
 const StringRef DeclToImportID = "declToImport";
 const StringRef DeclToVerifyID = "declToVerify";
 
@@ -112,6 +189,8 @@
   // This traverses the AST to catch certain bugs like poorly or not
   // implemented subtrees.
   (*Imported)->dump(ToNothing);
+  // More detailed source location checks.
+  checkImportedSourceLocations(Node, *Imported);
 }
 
 return Imported;
@@ -480,7 +559,10 @@
 lazyInitToAST(ToLang, "", OutputFileName);
 TU *FromTU = findFromTU(From);
 assert(LookupTablePtr);
-return FromTU->import(*LookupTablePtr, ToAST.get(), From);
+Decl *To = FromTU->import(*LookupTablePtr, ToAST.get(), From);
+if (To)
+  checkImportedSourceLocations(From, To);
+return To;
   }
 
   template  DeclT *Import(DeclT *From, Language Lang) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r358006 - Fixed comment as pointed out by post-commit review of D59845

[Raphael Isemann via cfe-commits]

Author: teemperor
Date: Tue Apr  9 07:18:23 2019
New Revision: 358006

URL: http://llvm.org/viewvc/llvm-project?rev=358006=rev
Log:
Fixed comment as pointed out by post-commit review of D59845

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=358006=358005=358006=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Tue Apr  9 07:18:23 2019
@@ -1947,7 +1947,7 @@ bool ASTNodeImporter::IsStructuralMatch(
 
 bool ASTNodeImporter::IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToEnum) {
   // Eliminate a potential failure point where we attempt to re-import
-  // something we're trying to import while completin ToEnum
+  // something we're trying to import while completing ToEnum.
   if (Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum))
 if (auto *ToOriginEnum = dyn_cast(ToOrigin))
 ToEnum = ToOriginEnum;


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

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

Thanks for the review!




Comment at: clangd/CodeComplete.cpp:1320
+llvm::IntrusiveRefCntPtr VFS) && {
+auto CompletionFilter = speculateCompletionFilter(Content, Pos);
+if (!CompletionFilter) {

sammccall wrote:
> hmm, this function should likely also move to SourceCode, use Offset instead 
> of Position, and move to SourceCode.h. But probably a different patch.
ack



Comment at: clangd/CodeComplete.cpp:1341
+
+// Carve out the typed filter from the content so that we don't treat it as
+// an identifier.

sammccall wrote:
> you could just erase the typed filter from the suggestion list.
> (It may be a valid word spelled elsewhere, but there's no point suggesting it)
This is following conventions of other sources. Both index and sema provide 
results for fully-typed names. Considering that users might be already used to 
this, and completion results tend to give reassurance for correctly typed 
names, I am inclined to keep the typed filter if it's seen somewhere else in 
the file.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60126



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

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 194315.
ioeric added a comment.

- minor cleanup


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60126

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Headers.cpp
  clangd/Headers.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/HeadersTests.cpp
  unittests/clangd/SourceCodeTests.cpp

Index: unittests/clangd/SourceCodeTests.cpp
===
--- unittests/clangd/SourceCodeTests.cpp
+++ unittests/clangd/SourceCodeTests.cpp
@@ -9,6 +9,7 @@
 #include "Context.h"
 #include "Protocol.h"
 #include "SourceCode.h"
+#include "clang/Format/Format.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_os_ostream.h"
 #include "llvm/Testing/Support/Error.h"
@@ -304,6 +305,23 @@
   }
 }
 
+TEST(SourceCodeTests, CollectIdentifiers) {
+  auto Style = format::getLLVMStyle();
+  auto IDs = collectIdentifiers(R"cpp(
+  #include "a.h"
+  void foo() { int xyz; int abc = xyz; return foo(); }
+  )cpp",
+Style);
+  EXPECT_EQ(IDs.size(), 7u);
+  EXPECT_EQ(IDs["include"], 1u);
+  EXPECT_EQ(IDs["void"], 1u);
+  EXPECT_EQ(IDs["int"], 2u);
+  EXPECT_EQ(IDs["xyz"], 2u);
+  EXPECT_EQ(IDs["abc"], 1u);
+  EXPECT_EQ(IDs["return"], 1u);
+  EXPECT_EQ(IDs["foo"], 2u);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/HeadersTests.cpp
===
--- unittests/clangd/HeadersTests.cpp
+++ unittests/clangd/HeadersTests.cpp
@@ -90,7 +90,7 @@
 
 IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
  CDB.getCompileCommand(MainFile)->Directory,
- Clang->getPreprocessor().getHeaderSearchInfo());
+ >getPreprocessor().getHeaderSearchInfo());
 for (const auto  : Inclusions)
   Inserter.addExisting(Inc);
 auto Declaring = ToHeaderFile(Original);
@@ -110,7 +110,7 @@
 
 IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
  CDB.getCompileCommand(MainFile)->Directory,
- Clang->getPreprocessor().getHeaderSearchInfo());
+ >getPreprocessor().getHeaderSearchInfo());
 auto Edit = Inserter.insert(VerbatimHeader);
 Action.EndSourceFile();
 return Edit;
@@ -252,6 +252,24 @@
   EXPECT_TRUE(StringRef(Edit->newText).contains(""));
 }
 
+TEST(Headers, NoHeaderSearchInfo) {
+  std::string MainFile = testPath("main.cpp");
+  IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
+   /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr);
+
+  auto HeaderPath = testPath("sub/bar.h");
+  auto Declaring = HeaderFile{HeaderPath, /*Verbatim=*/false};
+  auto Inserting = HeaderFile{HeaderPath, /*Verbatim=*/false};
+  auto Verbatim = HeaderFile{"", /*Verbatim=*/true};
+
+  EXPECT_EQ(Inserter.calculateIncludePath(Declaring, Inserting),
+"\"" + HeaderPath + "\"");
+  EXPECT_EQ(Inserter.shouldInsertInclude(Declaring, Inserting), false);
+
+  EXPECT_EQ(Inserter.calculateIncludePath(Declaring, Verbatim), "");
+  EXPECT_EQ(Inserter.shouldInsertInclude(Declaring, Verbatim), true);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -20,6 +20,7 @@
 #include "TestTU.h"
 #include "index/MemIndex.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
@@ -32,7 +33,6 @@
 using ::llvm::Failed;
 using ::testing::AllOf;
 using ::testing::Contains;
-using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Field;
 using ::testing::HasSubstr;
@@ -139,6 +139,25 @@
  FilePath);
 }
 
+// Builds a server and runs code completion.
+// If IndexSymbols is non-empty, an index will be built and passed to opts.
+CodeCompleteResult completionsNoCompile(llvm::StringRef Text,
+std::vector IndexSymbols = {},
+clangd::CodeCompleteOptions Opts = {},
+PathRef FilePath = "foo.cpp") {
+  std::unique_ptr OverrideIndex;
+  if (!IndexSymbols.empty()) {
+assert(!Opts.Index && "both Index and IndexSymbols given!");
+OverrideIndex = memIndex(std::move(IndexSymbols));
+Opts.Index = OverrideIndex.get();
+  }
+
+  MockFSProvider FS;
+  Annotations Test(Text);
+  

[PATCH] D60126: [clangd] Use identifiers in file as completion candidates when build is not ready.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 194311.
ioeric marked 9 inline comments as done.
ioeric added a comment.

- address review comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60126

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  clangd/Headers.cpp
  clangd/Headers.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/HeadersTests.cpp
  unittests/clangd/SourceCodeTests.cpp

Index: unittests/clangd/SourceCodeTests.cpp
===
--- unittests/clangd/SourceCodeTests.cpp
+++ unittests/clangd/SourceCodeTests.cpp
@@ -9,6 +9,7 @@
 #include "Context.h"
 #include "Protocol.h"
 #include "SourceCode.h"
+#include "clang/Format/Format.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/raw_os_ostream.h"
 #include "llvm/Testing/Support/Error.h"
@@ -304,6 +305,23 @@
   }
 }
 
+TEST(SourceCodeTests, CollectIdentifiers) {
+  auto Style = format::getLLVMStyle();
+  auto IDs = collectIdentifiers(R"cpp(
+  #include "a.h"
+  void foo() { int xyz; int abc = xyz; return foo(); }
+  )cpp",
+Style);
+  EXPECT_EQ(IDs.size(), 7u);
+  EXPECT_EQ(IDs["include"], 1u);
+  EXPECT_EQ(IDs["void"], 1u);
+  EXPECT_EQ(IDs["int"], 2u);
+  EXPECT_EQ(IDs["xyz"], 2u);
+  EXPECT_EQ(IDs["abc"], 1u);
+  EXPECT_EQ(IDs["return"], 1u);
+  EXPECT_EQ(IDs["foo"], 2u);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/HeadersTests.cpp
===
--- unittests/clangd/HeadersTests.cpp
+++ unittests/clangd/HeadersTests.cpp
@@ -90,7 +90,7 @@
 
 IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
  CDB.getCompileCommand(MainFile)->Directory,
- Clang->getPreprocessor().getHeaderSearchInfo());
+ >getPreprocessor().getHeaderSearchInfo());
 for (const auto  : Inclusions)
   Inserter.addExisting(Inc);
 auto Declaring = ToHeaderFile(Original);
@@ -110,7 +110,7 @@
 
 IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
  CDB.getCompileCommand(MainFile)->Directory,
- Clang->getPreprocessor().getHeaderSearchInfo());
+ >getPreprocessor().getHeaderSearchInfo());
 auto Edit = Inserter.insert(VerbatimHeader);
 Action.EndSourceFile();
 return Edit;
@@ -252,6 +252,24 @@
   EXPECT_TRUE(StringRef(Edit->newText).contains(""));
 }
 
+TEST(Headers, NoHeaderSearchInfo) {
+  std::string MainFile = testPath("main.cpp");
+  IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(),
+   /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr);
+
+  auto HeaderPath = testPath("sub/bar.h");
+  auto Declaring = HeaderFile{HeaderPath, /*Verbatim=*/false};
+  auto Inserting = HeaderFile{HeaderPath, /*Verbatim=*/false};
+  auto Verbatim = HeaderFile{"", /*Verbatim=*/true};
+
+  EXPECT_EQ(Inserter.calculateIncludePath(Declaring, Inserting),
+"\"" + HeaderPath + "\"");
+  EXPECT_EQ(Inserter.shouldInsertInclude(Declaring, Inserting), false);
+
+  EXPECT_EQ(Inserter.calculateIncludePath(Declaring, Verbatim), "");
+  EXPECT_EQ(Inserter.shouldInsertInclude(Declaring, Verbatim), true);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -20,6 +20,7 @@
 #include "TestTU.h"
 #include "index/MemIndex.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
@@ -32,7 +33,6 @@
 using ::llvm::Failed;
 using ::testing::AllOf;
 using ::testing::Contains;
-using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Field;
 using ::testing::HasSubstr;
@@ -139,6 +139,25 @@
  FilePath);
 }
 
+// Builds a server and runs code completion.
+// If IndexSymbols is non-empty, an index will be built and passed to opts.
+CodeCompleteResult completionsNoCompile(llvm::StringRef Text,
+std::vector IndexSymbols = {},
+clangd::CodeCompleteOptions Opts = {},
+PathRef FilePath = "foo.cpp") {
+  std::unique_ptr OverrideIndex;
+  if (!IndexSymbols.empty()) {
+assert(!Opts.Index && "both Index and IndexSymbols given!");
+OverrideIndex = memIndex(std::move(IndexSymbols));
+Opts.Index = OverrideIndex.get();
+  }
+
+  

[PATCH] D60461: [ASTImporter] Import TemplateParameterLists in function templates.

[Balázs Kéri via Phabricator via cfe-commits]

balazske created this revision.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp.
Herald added a reviewer: martong.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: clang.

Correct missing import of TemplateParameterList in function decl.


Repository:
  rC Clang

https://reviews.llvm.org/D60461

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


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -4991,6 +4991,24 @@
   EXPECT_FALSE(ImportedD->getUnderlyingType()->isIncompleteType());
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportTemplateParameterLists) {
+  auto Code =
+  R"(
+  template
+  int f() { return 0; }
+  template <> int f() { return 4; }
+  )";
+
+  Decl *FromTU = getTuDecl(Code, Lang_CXX);
+  auto *FromD = FirstDeclMatcher().match(FromTU,
+  functionDecl(hasName("f"), isExplicitTemplateSpecialization()));
+  ASSERT_EQ(FromD->getNumTemplateParameterLists(), 1);
+
+  auto *ToD = Import(FromD, Lang_CXX);
+  // The template parameter list should exist.
+  EXPECT_EQ(ToD->getNumTemplateParameterLists(), 1);
+}
+
 struct ASTImporterLookupTableTest : ASTImporterOptionSpecificTestBase {};
 
 TEST_P(ASTImporterLookupTableTest, OneDecl) {
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -412,6 +412,8 @@
 Expected
 ImportFunctionTemplateWithTemplateArgsFromSpecialization(
 FunctionDecl *FromFD);
+Error ImportTemplateParameterLists(const DeclaratorDecl *FromD,
+   DeclaratorDecl *ToD);
 
 Error ImportTemplateInformation(FunctionDecl *FromFD, FunctionDecl *ToFD);
 
@@ -2770,6 +2772,22 @@
   return ToEnumerator;
 }
 
+Error ASTNodeImporter::ImportTemplateParameterLists(const DeclaratorDecl 
*FromD,
+DeclaratorDecl *ToD) {
+  unsigned int Num = FromD->getNumTemplateParameterLists();
+  if (Num == 0)
+return Error::success();
+  SmallVector ToTPLists(Num);
+  for (unsigned int I = 0; I < Num; ++I)
+if (Expected ToTPListOrErr =
+import(FromD->getTemplateParameterList(I)))
+  ToTPLists[I] = *ToTPListOrErr;
+else
+  return ToTPListOrErr.takeError();
+  ToD->setTemplateParameterListsInfo(Importer.ToContext, ToTPLists);
+  return Error::success();
+}
+
 Error ASTNodeImporter::ImportTemplateInformation(
 FunctionDecl *FromFD, FunctionDecl *ToFD) {
   switch (FromFD->getTemplatedKind()) {
@@ -2816,6 +2834,9 @@
 if (!POIOrErr)
   return POIOrErr.takeError();
 
+if (Error Err = ImportTemplateParameterLists(FromFD, ToFD))
+  return Err;
+
 TemplateSpecializationKind TSK = FTSInfo->getTemplateSpecializationKind();
 ToFD->setFunctionTemplateSpecialization(
 std::get<0>(*FunctionAndArgsOrErr), ToTAList, /* InsertPos= */ nullptr,


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -4991,6 +4991,24 @@
   EXPECT_FALSE(ImportedD->getUnderlyingType()->isIncompleteType());
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportTemplateParameterLists) {
+  auto Code =
+  R"(
+  template
+  int f() { return 0; }
+  template <> int f() { return 4; }
+  )";
+
+  Decl *FromTU = getTuDecl(Code, Lang_CXX);
+  auto *FromD = FirstDeclMatcher().match(FromTU,
+  functionDecl(hasName("f"), isExplicitTemplateSpecialization()));
+  ASSERT_EQ(FromD->getNumTemplateParameterLists(), 1);
+
+  auto *ToD = Import(FromD, Lang_CXX);
+  // The template parameter list should exist.
+  EXPECT_EQ(ToD->getNumTemplateParameterLists(), 1);
+}
+
 struct ASTImporterLookupTableTest : ASTImporterOptionSpecificTestBase {};
 
 TEST_P(ASTImporterLookupTableTest, OneDecl) {
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -412,6 +412,8 @@
 Expected
 ImportFunctionTemplateWithTemplateArgsFromSpecialization(
 FunctionDecl *FromFD);
+Error ImportTemplateParameterLists(const DeclaratorDecl *FromD,
+   DeclaratorDecl *ToD);
 
 Error ImportTemplateInformation(FunctionDecl *FromFD, FunctionDecl *ToFD);
 
@@ -2770,6 +2772,22 @@
   return ToEnumerator;
 }
 
+Error ASTNodeImporter::ImportTemplateParameterLists(const DeclaratorDecl *FromD,
+DeclaratorDecl *ToD) {
+  unsigned int Num = FromD->getNumTemplateParameterLists();
+  if (Num == 0)
+return Error::success();
+  SmallVector ToTPLists(Num);
+  for (unsigned int I = 0; I < Num; ++I)
+if (Expected 

[PATCH] D59987: Add support for detection of devtoolset-8

[Tom Stellard via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL358002: Add support for detection of devtoolset-8 (authored 
by tstellar, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59987?vs=192800=194304#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59987

Files:
  cfe/trunk/lib/Driver/ToolChains/Gnu.cpp


Index: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
@@ -1875,6 +1875,7 @@
   // Non-Solaris is much simpler - most systems just go with "/usr".
   if (SysRoot.empty() && TargetTriple.getOS() == llvm::Triple::Linux) {
 // Yet, still look for RHEL devtoolsets.
+Prefixes.push_back("/opt/rh/devtoolset-8/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-7/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-6/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-4/root/usr");


Index: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
@@ -1875,6 +1875,7 @@
   // Non-Solaris is much simpler - most systems just go with "/usr".
   if (SysRoot.empty() && TargetTriple.getOS() == llvm::Triple::Linux) {
 // Yet, still look for RHEL devtoolsets.
+Prefixes.push_back("/opt/rh/devtoolset-8/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-7/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-6/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-4/root/usr");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r358002 - Add support for detection of devtoolset-8

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Tue Apr  9 06:26:10 2019
New Revision: 358002

URL: http://llvm.org/viewvc/llvm-project?rev=358002=rev
Log:
Add support for detection of devtoolset-8

Summary:
The current llvm/clang et al. project can be built with the latest developer 
toolset (devtoolset-8) on RHEL, which provides GCC 8.2.1.
However, the result compiler will not identify this toolset itself when 
compiling programs, which is of course not desirable.

After the patch - which simply adds the name of the developer toolset to the 
existing list - it gets identified and selected, as shown below:

[bamboo@bamboo llvm-project]$ clang -v
clang version 9.0.0 (https://github.com/llvm/llvm-project.git 
e5ac385fb1ffa4bd3875ea6a4d24efdbd7814572)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /home/bamboo/llvm/bin
Found candidate GCC installation: 
/opt/rh/devtoolset-4/root/usr/lib/gcc/x86_64-redhat-linux/5.2.1
Found candidate GCC installation: 
/opt/rh/devtoolset-7/root/usr/lib/gcc/x86_64-redhat-linux/7
Found candidate GCC installation: 
/opt/rh/devtoolset-8/root/usr/lib/gcc/x86_64-redhat-linux/8
Found candidate GCC installation: /usr/lib/gcc/x86_64-redhat-linux/4.8.2
Found candidate GCC installation: /usr/lib/gcc/x86_64-redhat-linux/4.8.5
Selected GCC installation: 
/opt/rh/devtoolset-8/root/usr/lib/gcc/x86_64-redhat-linux/8
Candidate multilib: .;@m64
Candidate multilib: 32;@m32
Selected multilib: .;@m64

Patch By: Radu-Adrian Popescu

Reviewers: tstellar, fedor.sergeev

Reviewed By: tstellar

Subscribers: jdoerfert, cfe-commits

Tags: #clang

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

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

Modified: cfe/trunk/lib/Driver/ToolChains/Gnu.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Gnu.cpp?rev=358002=358001=358002=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Gnu.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Gnu.cpp Tue Apr  9 06:26:10 2019
@@ -1875,6 +1875,7 @@ void Generic_GCC::GCCInstallationDetecto
   // Non-Solaris is much simpler - most systems just go with "/usr".
   if (SysRoot.empty() && TargetTriple.getOS() == llvm::Triple::Linux) {
 // Yet, still look for RHEL devtoolsets.
+Prefixes.push_back("/opt/rh/devtoolset-8/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-7/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-6/root/usr");
 Prefixes.push_back("/opt/rh/devtoolset-4/root/usr");


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

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D60455#1459678 , @Fznamznon wrote:

> Hi all,
>
> I assume that something to mark the code which is supposed to be offloaded 
> already is implemented for OpenMP and CUDA. For example I found CUDA-specific 
> CUDADevice attribute.
>  I can't just use CUDA-specific attributes for SYCL but I think that these 
> resources for marking device code could be unified and re-used for all 
> supported single-source offload programming models (like SYCL, CUDA, OpenMP 
> etc).
>  Please let me know If you have any suggestions about it.


The problem is that all thee attributes have different semantics. 
OpenMP-specific attributes, for example, cannot be specified explcitly as the 
attributes, because according to the standard they are represented as pragmas. 
Because of thatmost of the OpenMP attributes can be created only implicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

Hi all,

I assume that something to mark the code which is supposed to be offloaded 
already is implemented for OpenMP and CUDA. For example I found CUDA-specific 
CUDADevice attribute.
I can't just use CUDA-specific attributes for SYCL but I think that these 
resources for marking device code could be unified and re-used for all 
supported single-source offload programming models (like SYCL, CUDA, OpenMP 
etc).
Please let me know If you have any suggestions about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D59746: [LibTooling] Fix '-h' option

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D59746#1458539 , @hintonda wrote:

> In D59746#1458461 , @hintonda wrote:
>
> > In D59746#1458432 , @klimek wrote:
> >
> > > If we make it an alias by default, can somebody overwrite that?
> >
> >
> > Unfortunately, that produces a runtime error:
> >
> >   lan1:/Users/dhinton/projects/llvm_project/monorepo/build/Debug $ 
> > bin/llvm-objdump -h
> >   : CommandLine Error: Option 'h' registered more than once!
> >   LLVM ERROR: inconsistency in registered CommandLine options
> >
> >
> > The operative lines:
> >
> >   llvm/tools/llvm-objdump/llvm-objdump.cpp:187:static cl::alias 
> > SectionHeadersShorter("h",
> >   llvm/lib/Support/CommandLine.cpp:2149:static cl::alias HOpA("h", 
> > cl::desc("Alias for -help"), cl::aliasopt(HOp));
> >
>
>
> The other problem is that these are statics, so you can't count on the order, 
> i.e., did the user overwrite get processed before or after the one in 
> `CommandLine.cpp`?


If we can extend cl::alias to support this, we could give it a priority and 
take the highest prio :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D60456: [RISCV][WIP/RFC] Hard float ABI support

[Alex Bradbury via Phabricator via cfe-commits]

asb created this revision.
asb added a reviewer: rjmccall.
Herald added subscribers: benna, psnobl, jocewei, PkmX, rkruppe, the_o, 
brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, shiva0217, 
kito-cheng, niosHD, sabuasal, apazos, simoncook, johnrusso, rbar.
Herald added a project: clang.

The RISC-V hard float calling convention 

 requires the frontend to:

- Detect cases where, once "flattened", a struct can be passed using int+fp or 
fp+fp registers under the hard float ABI and coerce to the appropriate type(s)
- Track usage of GPRs and FPRs in order to gate the above, and to determine 
when signext/zeroext attributes must be added to integer scalars

This patch attempts to do this in compliance with the documented ABI, and uses 
ABIArgInfo::CoerceAndExpand in order to do this. @rjmccall, as author of that 
code I've tagged you as reviewer for initial feedback on my usage.

Note that a previous version of the ABI indicated that when passing an int+fp 
struct using a GPR+FPR, the int would need to be sign or zero-extended 
appropriately. GCC never did this 
 and the ABI was changed 
, which makes life easier 
as ABIArgInfo::CoerceAndExpand can't currently handle sign/zero-extension 
attributes.

I'm sharing just in case there are any concerns about this general approach. 
Known deficiencies in this patch are all related to testing:

- Needs more basic ilp32d/lp64f/lp64d tests
- Not enough coverage of bitfields in structs, packed+aligned structs etc


Repository:
  rC Clang

https://reviews.llvm.org/D60456

Files:
  lib/Basic/Targets/RISCV.h
  lib/CodeGen/TargetInfo.cpp
  test/CodeGen/riscv32-ilp32-ilp32f-abi.c
  test/CodeGen/riscv32-ilp32-ilp32f-ilp32d-abi.c
  test/CodeGen/riscv32-ilp32f-abi.c
  test/CodeGen/riscv32-ilp32f-ilp32d-abi.c

Index: test/CodeGen/riscv32-ilp32f-ilp32d-abi.c
===
--- /dev/null
+++ test/CodeGen/riscv32-ilp32f-ilp32d-abi.c
@@ -0,0 +1,230 @@
+// RUN: %clang_cc1 -triple riscv32 -target-feature +f -target-abi ilp32f -emit-llvm %s -o - \
+// RUN: | FileCheck %s
+
+#include 
+
+// Verify that the tracking of used GPRs and FPRs works correctly by checking
+// that small integers are sign/zero extended when passed in registers.
+
+// Floats are passed in FPRs, so argument 'i' will be passed zero-extended 
+// because it will be passed in a GPR.
+
+// CHECK: define void @f_fpr_tracking(float %a, float %b, float %c, float %d, float %e, float %f, float %g, float %h, i8 zeroext %i)
+void f_fpr_tracking(float a, float b, float c, float d, float e, float f,
+float g, float h, uint8_t i) {}
+
+// Check that fp, fp+fp, and int+fp structs are lowered correctly. These will 
+// be passed in FPR, FPR+FPR, or GPR+FPR regs if sufficient registers are 
+// available the widths are <= XLEN and FLEN, and should be expanded to
+// separate arguments in IR. They are passed by the same rules for returns,
+// but will be lowered to simple two-element structs if necessary (as LLVM IR
+// functions cannot return multiple values).
+
+// A struct containing just one floating-point real is passed as though it 
+// were a standalone floating-point real.
+
+struct float_s { float f; };
+
+// CHECK: define void @f_float_s_arg(float)
+void f_float_s_arg(struct float_s a) {}
+
+// CHECK: define float @f_ret_float_s()
+struct float_s f_ret_float_s() {
+  return (struct float_s){1.0};
+}
+
+// Check that structs containing two float values (FLEN <= width) are expanded
+// provided sufficient FPRs are available.
+
+struct float_float_s { float f; float g; };
+
+// CHECK: define void @f_float_float_s_arg(float, float)
+void f_float_float_s_arg(struct float_float_s a) {}
+
+// CHECK: define { float, float } @f_ret_float_float_s()
+struct float_float_s f_ret_float_float_s() {
+  return (struct float_float_s){1.0, 2.0};
+}
+
+// CHECK: define void @f_float_float_s_arg_insufficient_fprs(float %a, float %b, float %c, float %d, float %e, float %f, float %g, [2 x i32] %h.coerce)
+void f_float_float_s_arg_insufficient_fprs(float a, float b, float c, float d, 
+float e, float f, float g, struct float_float_s h) {}
+
+// Check that structs containing int+float values are expanded, provided
+// sufficient FPRs and GPRs are available. The integer components are neither
+// sign or zero-extended.
+
+struct float_int8_s { float f; int8_t i; };
+struct float_uint8_s { float f; uint8_t i; };
+struct float_int32_s { float f; int32_t i; };
+struct float_int64_s { float f; int64_t i; };
+
+// CHECK: define void @f_float_int8_s_arg(float, i8)
+void f_float_int8_s_arg(struct float_int8_s a) {}
+
+// CHECK: define { float, i8 } @f_ret_float_int8_s()
+struct float_int8_s f_ret_float_int8_s() {
+  return (struct 

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a comment.

These attributes are being parsed and silently ignored -- are there semantics 
expected for the attributes?




Comment at: clang/include/clang/Basic/Attr.td:1000
+def SYCLDevice : InheritableAttr {
+  let Spellings = [GNU<"sycl_device">];
+  let Subjects = SubjectList<[Function, Var]>;

Is there a reason to not also introduce a C++11 and C2x style spelling in the 
`clang` namespace? e.g., `[[clang::sycl_device]]`



Comment at: clang/include/clang/Basic/Attr.td:1003
+  let LangOpts = [SYCL];
+  let Documentation = [Undocumented];
+}

No new, undocumented attributes, please.



Comment at: clang/test/SemaSYCL/kernel-attribute.cpp:4-5
+
+__attribute((sycl_kernel)) void foo() {
+}

Missing some tests:
* test that both attributes can be applied to whatever subjects they appertain 
to
* test that neither attribute can be applied to an incorrect subject
* test that the attributes do not accept arguments
* test that the attribute is ignored when SYCL is not enabled

Are there situations where the attribute does not make sense, such as member 
functions, virtual functions, etc? If so, those are good test cases (and 
diagnostics) to add as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D60455: [SYCL] Add support for SYCL device attributes

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon created this revision.
Herald added subscribers: cfe-commits, Anastasia, ebevhan.
Herald added a project: clang.

SYCL runtime is supposed to use sycl_kernel attribute to mark functions in
the single source which are supposed to be compiled for the device.
Compiler is supposed to understand if there are other functions/variables
which are supposed to be compiled for the device and mark them with
sycl_device attribute.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaSYCL/kernel-attribute.cpp


Index: clang/test/SemaSYCL/kernel-attribute.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/kernel-attribute.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsycl-is-device -fsyntax-only -verify -pedantic %s
+// expected-no-diagnostics
+
+__attribute((sycl_kernel)) void foo() {
+}
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
@@ -124,6 +124,8 @@
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, 
SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, 
SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLDevice (SubjectMatchRule_function, 
SubjectMatchRule_variable)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, 
SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, 
SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6755,6 +6755,9 @@
   case ParsedAttr::AT_Flatten:
 handleSimpleAttribute(S, D, AL);
 break;
+  case ParsedAttr::AT_SYCLKernel:
+handleSimpleAttribute(S, D, AL);
+break;
   case ParsedAttr::AT_Format:
 handleFormatAttr(S, D, AL);
 break;
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -291,6 +291,7 @@
 def MicrosoftExt : LangOpt<"MicrosoftExt">;
 def Borland : LangOpt<"Borland">;
 def CUDA : LangOpt<"CUDA">;
+def SYCL : LangOpt<"SYCLIsDevice">;
 def COnly : LangOpt<"CPlusPlus", 1>;
 def CPlusPlus : LangOpt<"CPlusPlus">;
 def OpenCL : LangOpt<"OpenCL">;
@@ -995,6 +996,20 @@
   let Documentation = [Undocumented];
 }
 
+def SYCLDevice : InheritableAttr {
+  let Spellings = [GNU<"sycl_device">];
+  let Subjects = SubjectList<[Function, Var]>;
+  let LangOpts = [SYCL];
+  let Documentation = [Undocumented];
+}
+
+def SYCLKernel : InheritableAttr {
+  let Spellings = [GNU<"sycl_kernel">];
+  let Subjects = SubjectList<[Function]>;
+  let LangOpts = [SYCL];
+  let Documentation = [Undocumented];
+}
+
 def C11NoReturn : InheritableAttr {
   let Spellings = [Keyword<"_Noreturn">];
   let Subjects = SubjectList<[Function], ErrorDiag>;


Index: clang/test/SemaSYCL/kernel-attribute.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/kernel-attribute.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -fsycl-is-device -fsyntax-only -verify -pedantic %s
+// expected-no-diagnostics
+
+__attribute((sycl_kernel)) void foo() {
+}
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
@@ -124,6 +124,8 @@
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLDevice (SubjectMatchRule_function, SubjectMatchRule_variable)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6755,6 +6755,9 @@
   case 

[PATCH] D60055: Check i < FD->getNumParams() before querying

[Violet via Phabricator via cfe-commits]

Violet added a comment.

In D60055#1459602 , @lebedev.ri wrote:

> I won't be able to properly review this, sorry.


Can you suggest someone with familiarity for this part of code? The reviewers I 
randomly added by `git blame` doesn't seem to be reviewing.

> While i'm sure this fix silences the assert, i don't know what that means for 
> the original caller.
>  Does it fail? Does it retry in some other scope? Should it be more picky 
> about the original scope in the first place?

This small function is just to get the canonical ParmValDecl for a given one. 
If the check doesn't pass, it simply mean there is no canonical one (it's 
obvious for a function pointer), so we just use the passed one. There is 
nothing wrong here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60055



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

[PATCH] D59711: PR41183: Don't emit Wstrict-prototypes warning for an implicit function declaration.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.
Herald added a subscriber: dexonsmith.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59711



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

[PATCH] D60055: Check i < FD->getNumParams() before querying

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri resigned from this revision.
lebedev.ri added a comment.

I won't be able to properly review this, sorry.
While i'm sure this fix silences the assert, i don't know what that means for 
the original caller.
Does it fail? Does it retry in some other scope? Should it be more picky about 
the original scope in the first place?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60055



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

[clang-tools-extra] r357994 - ClangTidy: Avoid mixing stdout with stderror when dealing with a large number of files.

[Andi-Bogdan Postelnicu via cfe-commits]

Author: abpostelnicu
Date: Tue Apr  9 04:17:02 2019
New Revision: 357994

URL: http://llvm.org/viewvc/llvm-project?rev=357994=rev
Log:
ClangTidy: Avoid mixing stdout with stderror when dealing with a large number 
of files.

Summary:
At Mozilla we are using this tool in order to perform review-time 
static-analysis, since some patches contain a large number of files we've 
discovered this issue, where `stderr` gets mixed with `stdout` thus obfuscating 
our possibility to parse the output.
The patch that we are currently use can be found 
[here](https://searchfox.org/mozilla-central/source/build/build-clang/clang-tidy-8.patch).

This is just an upstream of the original patch.

Reviewers: JonasToth

Reviewed By: JonasToth

Subscribers: cfe-commits

Tags: #clang, #clang-tools-extra

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

Modified:
clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py

Modified: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py?rev=357994=357993=357994=diff
==
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py Tue Apr  9 
04:17:02 2019
@@ -172,6 +172,7 @@ def run_tidy(args, tmpdir, build_path, q
 with lock:
   sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + 
'\n')
   if len(err) > 0:
+sys.stdout.flush()
 sys.stderr.write(err.decode('utf-8') + '\n')
 queue.task_done()
 


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

[PATCH] D60379: Make precompiled headers reproducible

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Serialization/ASTWriter.cpp:4283
+  // Sort to allow reproducible .pch files - https://bugs.debian.org/877359
+  std::map> sortedOpenCLTypeExtMap;
   for (const auto  : SemaRef.OpenCLTypeExtMap) {

riccibruno wrote:
> lebedev.ri wrote:
> > Anastasia wrote:
> > > lebedev.ri wrote:
> > > > Would it be better to just change the actual type of 
> > > > `SemaRef.OpenCLTypeExtMap`?
> > > > https://github.com/llvm-mirror/clang/blob/18917301298ad6df9e989983ed1e22cb0f9dff29/include/clang/Sema/Sema.h#L8710-L8712
> > > What data structure would you suggest to use instead? I think sorting 
> > > during serialization or de-serialization of AST isn't too bad because 
> > > it's not a common use case.
> > Just `std::map<>`?
> Why is this a problem ? I thought that pch files where not supposed to be 
> distributed. If we come to the conclusion that pch files must be 
> reproducible, then I believe that this is by far not the only example.
Ok, I would normally prefer to use `DenseMap` for the general case because 
since the number of elements in `OpenCLTypeExtMap` is very small we can take 
advantage of non-heap allocation. But at the same time probably a couple of 
elements won't make any difference for `std::map` either and we can keep the 
code base more maintainable. So I don't mind either way.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60379



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

[PATCH] D60454: [OpenCL] Prevent mangling kernel functions

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added reviewers: rjmccall, bader.
Herald added subscribers: ebevhan, yaxunl.

Kernel function names have to be preserved as in the original source to be able 
to access them from the host API side.

This patch adds restriction to kernels that prevents them to be used in 
overloading, templates, etc. This is implemented by enclosing the kernels 
implicitly into C linkage clause. Therefore all C linkage restrictions will 
apply to the kernels.


https://reviews.llvm.org/D60454

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDecl.cpp
  test/CodeGenOpenCLCXX/addrspace-of-this.cl
  test/CodeGenOpenCLCXX/local_addrspace_init.cl
  test/SemaOpenCLCXX/kernel_invalid.cl

Index: test/SemaOpenCLCXX/kernel_invalid.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/kernel_invalid.cl
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -cl-std=c++ -pedantic -verify -fsyntax-only
+
+struct C {
+  kernel void bar(); //expected-error{{kernel functions cannot be class members}}
+};
+
+template 
+kernel void templ(T par) { //expected-error{{kernel functions cannot be used in a template declaration, instantiation or specialization}}
+}
+
+template 
+kernel void bar(int par) { //expected-error{{kernel functions cannot be used in a template declaration, instantiation or specialization}}
+}
+
+kernel void foo(int); //expected-note{{previous declaration is here}}
+
+kernel void foo(float); //expected-error{{conflicting types for 'foo'}}
Index: test/CodeGenOpenCLCXX/local_addrspace_init.cl
===
--- test/CodeGenOpenCLCXX/local_addrspace_init.cl
+++ test/CodeGenOpenCLCXX/local_addrspace_init.cl
@@ -1,8 +1,8 @@
 // RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
 
 // Test that we don't initialize local address space objects.
-//CHECK: @_ZZ4testvE1i = internal addrspace(3) global i32 undef
-//CHECK: @_ZZ4testvE2ii = internal addrspace(3) global %class.C undef
+//CHECK: @_ZZ4testE1i = internal addrspace(3) global i32 undef
+//CHECK: @_ZZ4testE2ii = internal addrspace(3) global %class.C undef
 class C {
   int i;
 };
Index: test/CodeGenOpenCLCXX/addrspace-of-this.cl
===
--- test/CodeGenOpenCLCXX/addrspace-of-this.cl
+++ test/CodeGenOpenCLCXX/addrspace-of-this.cl
@@ -82,7 +82,7 @@
 // EXPL-LABEL: @__cxx_global_var_init()
 // EXPL: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast (%class.C addrspace(1)* @c to %class.C addrspace(4)*))
 
-// COMMON-LABEL: @_Z12test__globalv()
+// COMMON-LABEL: @test__global()
 
 // Test the address space of 'this' when invoking a method.
 // COMMON: %call = call i32 @_ZNU3AS41C3getEv(%class.C addrspace(4)* addrspacecast (%class.C addrspace(1)* @c to %class.C addrspace(4)*))
@@ -149,19 +149,19 @@
 
 TEST(__local)
 
-// COMMON-LABEL: _Z11test__localv
+// COMMON-LABEL: @test__local
 
 // Test that we don't initialize an object in local address space
 // EXPL-NOT: call void @_ZNU3AS41CC1Ev(%class.C addrspace(4)* addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
 
 // Test the address space of 'this' when invoking a method.
-// COMMON: %call = call i32 @_ZNU3AS41C3getEv(%class.C addrspace(4)* addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+// COMMON: %call = call i32 @_ZNU3AS41C3getEv(%class.C addrspace(4)* addrspacecast (%class.C addrspace(3)* @_ZZ11test__localE1c to %class.C addrspace(4)*))
 
 // Test the address space of 'this' when invoking copy-constructor.
 // COMMON: [[C1GEN:%[0-9]+]] = addrspacecast %class.C* %c1 to %class.C addrspace(4)*
-// EXPL: call void @_ZNU3AS41CC1ERU3AS4KS_(%class.C addrspace(4)* [[C1GEN]], %class.C addrspace(4)* dereferenceable(4) addrspacecast (%class.C addrspace(3)* @_ZZ11test__localvE1c to %class.C addrspace(4)*))
+// EXPL: call void @_ZNU3AS41CC1ERU3AS4KS_(%class.C addrspace(4)* [[C1GEN]], %class.C addrspace(4)* dereferenceable(4) addrspacecast (%class.C addrspace(3)* @_ZZ11test__localE1c to %class.C addrspace(4)*))
 // IMPL:  [[C1VOID:%[0-9]+]] = bitcast %class.C* %c1 to i8*
-// IMPL:  call void @llvm.memcpy.p0i8.p4i8.i32(i8* {{.*}}[[C1VOID]], i8 addrspace(4)* {{.*}}addrspacecast (i8 addrspace(3)* bitcast (%class.C addrspace(3)* @_ZZ11test__localvE1c to i8 addrspace(3)*) to i8 addrspace(4)*), i32 4, i1 false)
+// IMPL:  call void @llvm.memcpy.p0i8.p4i8.i32(i8* {{.*}}[[C1VOID]], i8 addrspace(4)* {{.*}}addrspacecast (i8 addrspace(3)* bitcast (%class.C addrspace(3)* @_ZZ11test__localE1c to i8 addrspace(3)*) to i8 addrspace(4)*), i32 4, i1 false)
 
 // Test the address space of 'this' when invoking a constructor.
 // EXPL: [[C2GEN:%[0-9]+]] = addrspacecast %class.C* %c2 to %class.C addrspace(4)*
@@ -177,7 +177,7 @@
 
 TEST(__private)
 
-// CHECK-LABEL: @_Z13test__privatev
+// CHECK-LABEL: @test__private
 
 // Test the address 

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl updated this revision to Diff 194285.
r.stahl marked 4 inline comments as done.
r.stahl added a comment.

- addressed review comments
- created cross_tu::containsConst
- fixed bug that unions were not exported
- added TODO test for constructor case


Repository:
  rC Clang

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

https://reviews.llvm.org/D46421

Files:
  include/clang/CrossTU/CrossTranslationUnit.h
  lib/CrossTU/CrossTranslationUnit.cpp
  lib/StaticAnalyzer/Core/RegionStore.cpp
  lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  test/Analysis/Inputs/ctu-other.cpp
  test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  test/Analysis/ctu-main.cpp
  test/Analysis/func-mapping-test.cpp
  test/Analysis/redecl.c
  tools/clang-extdef-mapping/ClangExtDefMapGen.cpp

Index: tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
===
--- tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
+++ tools/clang-extdef-mapping/ClangExtDefMapGen.cpp
@@ -34,20 +34,22 @@
 class MapExtDefNamesConsumer : public ASTConsumer {
 public:
   MapExtDefNamesConsumer(ASTContext )
-  : SM(Context.getSourceManager()) {}
+  : Ctx(Context), SM(Context.getSourceManager()) {}
 
   ~MapExtDefNamesConsumer() {
 // Flush results to standard output.
 llvm::outs() << createCrossTUIndexString(Index);
   }
 
-  void HandleTranslationUnit(ASTContext ) override {
-handleDecl(Ctx.getTranslationUnitDecl());
+  void HandleTranslationUnit(ASTContext ) override {
+handleDecl(Context.getTranslationUnitDecl());
   }
 
 private:
   void handleDecl(const Decl *D);
+  void addIfInMain(const DeclaratorDecl *DD, SourceLocation defStart);
 
+  ASTContext 
   SourceManager 
   llvm::StringMap Index;
   std::string CurrentFileName;
@@ -58,30 +60,13 @@
 return;
 
   if (const auto *FD = dyn_cast(D)) {
-if (FD->isThisDeclarationADefinition()) {
-  if (const Stmt *Body = FD->getBody()) {
-if (CurrentFileName.empty()) {
-  CurrentFileName =
-  SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
-  if (CurrentFileName.empty())
-CurrentFileName = "invalid_file";
-}
-
-switch (FD->getLinkageInternal()) {
-case ExternalLinkage:
-case VisibleNoLinkage:
-case UniqueExternalLinkage:
-  if (SM.isInMainFile(Body->getBeginLoc())) {
-std::string LookupName =
-CrossTranslationUnitContext::getLookupName(FD);
-Index[LookupName] = CurrentFileName;
-  }
-  break;
-default:
-  break;
-}
-  }
-}
+if (FD->isThisDeclarationADefinition())
+  if (const Stmt *Body = FD->getBody())
+addIfInMain(FD, Body->getBeginLoc());
+  } else if (const auto *VD = dyn_cast(D)) {
+if (cross_tu::containsConst(VD, Ctx) && VD->hasInit())
+  if (const Expr *Init = VD->getInit())
+addIfInMain(VD, Init->getBeginLoc());
   }
 
   if (const auto *DC = dyn_cast(D))
@@ -89,6 +74,27 @@
   handleDecl(D);
 }
 
+void MapExtDefNamesConsumer::addIfInMain(const DeclaratorDecl *DD,
+ SourceLocation defStart) {
+  std::string LookupName = CrossTranslationUnitContext::getLookupName(DD);
+  if (CurrentFileName.empty()) {
+CurrentFileName =
+SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
+if (CurrentFileName.empty())
+  CurrentFileName = "invalid_file";
+  }
+
+  switch (DD->getLinkageInternal()) {
+  case ExternalLinkage:
+  case VisibleNoLinkage:
+  case UniqueExternalLinkage:
+if (SM.isInMainFile(defStart))
+  Index[LookupName] = CurrentFileName;
+  default:
+break;
+  }
+}
+
 class MapExtDefNamesAction : public ASTFrontendAction {
 protected:
   std::unique_ptr CreateASTConsumer(CompilerInstance ,
Index: test/Analysis/redecl.c
===
--- /dev/null
+++ test/Analysis/redecl.c
@@ -0,0 +1,13 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+// XFAIL: *
+
+void clang_analyzer_eval(int);
+
+extern const int extInt;
+
+int main()
+{
+clang_analyzer_eval(extInt == 2); // expected-warning{{TRUE}}
+}
+
+extern const int extInt = 2;
Index: test/Analysis/func-mapping-test.cpp
===
--- test/Analysis/func-mapping-test.cpp
+++ test/Analysis/func-mapping-test.cpp
@@ -1,7 +1,43 @@
-// RUN: %clang_extdef_map %s -- | FileCheck %s
+// RUN: %clang_extdef_map %s -- | FileCheck --implicit-check-not "c:@y" --implicit-check-not "c:@z" %s
 
 int f(int) {
   return 0;
 }
+// CHECK-DAG: c:@F@f#I#
 
-// CHECK: c:@F@f#I#
+extern const int x = 5;
+// CHECK-DAG: c:@x
+
+// Non-const variables should not be collected.
+int y = 5;
+
+// In C++, const implies internal linkage, so not collected.
+const int z = 5;
+
+struct S {
+  int a;
+};
+extern S const s = 

[PATCH] D60453: ClangTidy: Avoid mixing stdout with stderror when dealing with a large number of files.

[Andi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357994: ClangTidy: Avoid mixing stdout with stderror when 
dealing with a large number… (authored by Abpostelnicu, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60453

Files:
  clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py


Index: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
@@ -172,6 +172,7 @@
 with lock:
   sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + 
'\n')
   if len(err) > 0:
+sys.stdout.flush()
 sys.stderr.write(err.decode('utf-8') + '\n')
 queue.task_done()
 


Index: clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/trunk/clang-tidy/tool/run-clang-tidy.py
@@ -172,6 +172,7 @@
 with lock:
   sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + '\n')
   if len(err) > 0:
+sys.stdout.flush()
 sys.stderr.write(err.decode('utf-8') + '\n')
 queue.task_done()
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46421: [analyzer][CrossTU] Extend CTU to VarDecls with initializer

[Rafael Stahl via Phabricator via cfe-commits]

r.stahl added inline comments.



Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:357
+  return true;
+if (!RTy->hasConstFields())
+  return true;

xazax.hun wrote:
> I wonder what would happen with types that has const fields and user written 
> constructors. In case we will not simulate the effect of the constructor and 
> will not be able to set the const fields maybe we should exclude those as 
> well?
I added a test for that and it doesn't seem to work. It just ends up as unknown 
which is fine, right? Eventually it would be nice to do that of course.


Repository:
  rC Clang

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

https://reviews.llvm.org/D46421



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

[PATCH] D60453: ClangTidy: Avoid mixing stdout with stderror when dealing with a large number of files.

[Jonas Toth via Phabricator via cfe-commits]

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

How does that happen to be a problem? It feels that `stdout.write` and 
`stderr.write` should be unconnected? (if you know the reason I would like to 
know it too :))
Please note, that `run-clang-tidy` has an export-feature for the diagnostics 
and fix-its which might make it easier to analyze the output of clang-tidy.

I suppose you verified the fix already, then LGTM


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60453



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

r357993 - [RISCV] Unbreak test from r357989

[Alex Bradbury via cfe-commits]

Author: asb
Date: Tue Apr  9 03:44:47 2019
New Revision: 357993

URL: http://llvm.org/viewvc/llvm-project?rev=357993=rev
Log:
[RISCV] Unbreak test from r357989

There were some errors in the committed test checks, left in due to a git
stash apply mishap.


Modified:
cfe/trunk/test/CodeGen/riscv32-ilp32-ilp32f-abi.c
cfe/trunk/test/CodeGen/riscv32-ilp32-ilp32f-ilp32d-abi.c

Modified: cfe/trunk/test/CodeGen/riscv32-ilp32-ilp32f-abi.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/riscv32-ilp32-ilp32f-abi.c?rev=357993=357992=357993=diff
==
--- cfe/trunk/test/CodeGen/riscv32-ilp32-ilp32f-abi.c (original)
+++ cfe/trunk/test/CodeGen/riscv32-ilp32-ilp32f-abi.c Tue Apr  9 03:44:47 2019
@@ -35,8 +35,8 @@ int f_scalar_stack_1(int32_t a, int64_t
 // the presence of large return values that consume a register due to the need
 // to pass a pointer.
 
-// CHECK-LABEL: define void @f_scalar_stack_2(%struct.large* noalias sret 
%agg.result, int32_t %a, i64 %b, i64 %c, fp128 %d, i8 zeroext %e, i8 %f, i8 %g)
-struct large f_scalar_stack_2(int32_t a, int64_t b, double c, long double d,
+// CHECK-LABEL: define void @f_scalar_stack_2(%struct.large* noalias sret 
%agg.result, i32 %a, i64 %b, i64 %c, fp128 %d, i8 zeroext %e, i8 %f, i8 %g)
+struct large f_scalar_stack_2(int32_t a, int64_t b, int64_t c, long double d,
   uint8_t e, int8_t f, uint8_t g) {
   return (struct large){a, e, f, g};
 }
@@ -44,7 +44,7 @@ struct large f_scalar_stack_2(int32_t a,
 // Aggregates and >=XLen scalars passed on the stack should be lowered just as
 // they would be if passed via registers.
 
-// CHECK-LABEL: define void @f_scalar_stack_3(double %a, i64 %b, double %c, 
i64 %d, i32 %e, i64 %f, int32_t %g, double %h, fp128 %i)
+// CHECK-LABEL: define void @f_scalar_stack_3(double %a, i64 %b, double %c, 
i64 %d, i32 %e, i64 %f, i32 %g, double %h, fp128 %i)
 void f_scalar_stack_3(double a, int64_t b, double c, int64_t d, int e,
   int64_t f, int32_t g, double h, long double i) {}
 

Modified: cfe/trunk/test/CodeGen/riscv32-ilp32-ilp32f-ilp32d-abi.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/riscv32-ilp32-ilp32f-ilp32d-abi.c?rev=357993=357992=357993=diff
==
--- cfe/trunk/test/CodeGen/riscv32-ilp32-ilp32f-ilp32d-abi.c (original)
+++ cfe/trunk/test/CodeGen/riscv32-ilp32-ilp32f-ilp32d-abi.c Tue Apr  9 
03:44:47 2019
@@ -203,13 +203,13 @@ int f_scalar_stack_1(struct tiny a, stru
 // the presence of large return values that consume a register due to the need
 // to pass a pointer.
 
-// CHECK-LABEL: define void @f_scalar_stack_2(%struct.large* noalias sret 
%agg.result, i32 %a, i64 %b, double %c, fp128 %d, i8 zeroext %e, i8 %f, i8 %g)
+// CHECK-LABEL: define void @f_scalar_stack_2(%struct.large* noalias sret 
%agg.result, i32 %a, i64 %b, i64 %c, fp128 %d, i8 zeroext %e, i8 %f, i8 %g)
 struct large f_scalar_stack_2(int32_t a, int64_t b, int64_t c, long double d,
   uint8_t e, int8_t f, uint8_t g) {
   return (struct large){a, e, f, g};
 }
 
-// CHECK-LABEL: define fp128 @f_scalar_stack_4(i32 %a, i64 %b, double %c, 
fp128 %d, i8 zeroext %e, i8 %f, i8 %g)
+// CHECK-LABEL: define fp128 @f_scalar_stack_4(i32 %a, i64 %b, i64 %c, fp128 
%d, i8 zeroext %e, i8 %f, i8 %g)
 long double f_scalar_stack_4(int32_t a, int64_t b, int64_t c, long double d,
  uint8_t e, int8_t f, uint8_t g) {
   return d;


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

[PATCH] D60453: ClangTidy: Avoid mixing stdout with stderror when dealing with a large number of files.

[Andi via Phabricator via cfe-commits]

Abpostelnicu created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D60453

Files:
  clang-tidy/tool/run-clang-tidy.py


Index: clang-tidy/tool/run-clang-tidy.py
===
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -172,6 +172,7 @@
 with lock:
   sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + 
'\n')
   if len(err) > 0:
+sys.stdout.flush()
 sys.stderr.write(err.decode('utf-8') + '\n')
 queue.task_done()
 


Index: clang-tidy/tool/run-clang-tidy.py
===
--- clang-tidy/tool/run-clang-tidy.py
+++ clang-tidy/tool/run-clang-tidy.py
@@ -172,6 +172,7 @@
 with lock:
   sys.stdout.write(' '.join(invocation) + '\n' + output.decode('utf-8') + '\n')
   if len(err) > 0:
+sys.stdout.flush()
 sys.stderr.write(err.decode('utf-8') + '\n')
 queue.task_done()
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60139: [clang-tidy] Add bugprone-placement-new-target-type-mismatch check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL updated this revision to Diff 194283.
DennisL added a comment.

- handle array to ptr decay
- updated error msg
- additional tests for arry to ptr decay


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

https://reviews.llvm.org/D60139

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp
  clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp

Index: test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp
@@ -0,0 +1,86 @@
+// RUN: %check_clang_tidy %s bugprone-placement-new-target-type-mismatch %t
+
+// definitions
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);
+void *operator new[](size_type, void *);
+
+namespace std {
+template T* addressof(T& arg) noexcept;
+} // namespace std
+
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+};
+
+template
+T& getT() {
+  static T f;
+  return f;
+}
+
+// instances emitting warnings
+
+void f1() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int *ptr = new int;
+  new (ptr) Dummy;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f2() {
+  int * ptr = new int;
+  new (ptr) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f3() {
+  char *ptr = new char[17*sizeof(char)];
+  new (ptr) float[13];
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f4() {
+  new (std::addressof(getT())) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f5() {
+  char *ptr = new char[17*sizeof(char)];
+  new (ptr) float{13.f};
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f6() {
+  new ((void *)std::addressof(getT())) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f7() {
+  char array[17*sizeof(char)];
+  new (array) float{13.f};
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+// instances not emitting a warning
+
+void g1() {
+  Foo * ptr = new Foo;
+  new (ptr) Foo;
+}
+
+void g2() {
+  char *ptr = new char[17*sizeof(char)];
+  new ((float *)ptr) float{13.f};
+}
+
+void g3() {
+  char array[17*sizeof(char)];
+  new (array) char('A');
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -55,6 +55,7 @@
bugprone-move-forwarding-reference
bugprone-multiple-statement-macro
bugprone-parent-virtual-call
+   bugprone-placement-new-target-type-mismatch
bugprone-sizeof-container
bugprone-sizeof-expression
bugprone-string-constructor
Index: docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
@@ -0,0 +1,8 @@
+.. title:: clang-tidy - misc-placement-new-target-type-mismatch
+
+misc-placement-new-target-type-mismatch
+===
+
+Finds placement-new calls where the pointer type of the adress mismatches the
+type of the created value.
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -130,6 +130,12 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`bugprone-placement-new-target-type-mismatch
+  ` check.
+
+  Finds placement-new calls where the pointer type of the adress mismatches the
+  type of the created value.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
===
--- /dev/null
+++ clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
@@ -0,0 +1,35 @@
+//===--- PlacementNewTargetTypeMismatch.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See 

r357991 - [RISCV][NFC] Minor fixup for r357989

[Alex Bradbury via cfe-commits]

Author: asb
Date: Tue Apr  9 03:25:05 2019
New Revision: 357991

URL: http://llvm.org/viewvc/llvm-project?rev=357991=rev
Log:
[RISCV][NFC] Minor fixup for r357989

One of the tests in riscv64-lp64-lp64f-lp64d would have had a different
lowering for lp64f/lp64d as a float argument was missed.


Modified:
cfe/trunk/test/CodeGen/riscv64-lp64-lp64f-lp64d-abi.c

Modified: cfe/trunk/test/CodeGen/riscv64-lp64-lp64f-lp64d-abi.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/riscv64-lp64-lp64f-lp64d-abi.c?rev=357991=357990=357991=diff
==
--- cfe/trunk/test/CodeGen/riscv64-lp64-lp64f-lp64d-abi.c (original)
+++ cfe/trunk/test/CodeGen/riscv64-lp64-lp64f-lp64d-abi.c Tue Apr  9 03:25:05 
2019
@@ -188,8 +188,8 @@ int f_scalar_stack_1(struct tiny a, stru
   return g + h;
 }
 
-// CHECK-LABEL: define signext i32 @f_scalar_stack_2(i32 signext %a, i128 %b, 
float %c, fp128 %d, <32 x i8>*, i8 zeroext %f, i8 %g, i8 %h)
-int f_scalar_stack_2(int32_t a, __int128_t b, float c, long double d, v32i8 e,
+// CHECK-LABEL: define signext i32 @f_scalar_stack_2(i32 signext %a, i128 %b, 
i64 %c, fp128 %d, <32 x i8>*, i8 zeroext %f, i8 %g, i8 %h)
+int f_scalar_stack_2(int32_t a, __int128_t b, int64_t c, long double d, v32i8 
e,
  uint8_t f, int8_t g, uint8_t h) {
   return g + h;
 }


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

r357989 - [RISCV][NFC] Refactor RISC-V ABI lowering tests in preparation for hard float patches

[Alex Bradbury via cfe-commits]

Author: asb
Date: Tue Apr  9 03:12:49 2019
New Revision: 357989

URL: http://llvm.org/viewvc/llvm-project?rev=357989=rev
Log:
[RISCV][NFC] Refactor RISC-V ABI lowering tests in preparation for hard float 
patches

Split tests in to files representing the subset of RISC-V ABIs they should
have identical output for.


Added:
cfe/trunk/test/CodeGen/riscv32-ilp32-abi.c
cfe/trunk/test/CodeGen/riscv32-ilp32-ilp32f-abi.c
cfe/trunk/test/CodeGen/riscv32-ilp32-ilp32f-ilp32d-abi.c
cfe/trunk/test/CodeGen/riscv64-lp64-abi.c
cfe/trunk/test/CodeGen/riscv64-lp64-lp64f-abi.c
cfe/trunk/test/CodeGen/riscv64-lp64-lp64f-lp64d-abi.c
Removed:
cfe/trunk/test/CodeGen/riscv32-abi.c
cfe/trunk/test/CodeGen/riscv64-abi.c

Removed: cfe/trunk/test/CodeGen/riscv32-abi.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/riscv32-abi.c?rev=357988=auto
==
--- cfe/trunk/test/CodeGen/riscv32-abi.c (original)
+++ cfe/trunk/test/CodeGen/riscv32-abi.c (removed)
@@ -1,430 +0,0 @@
-// RUN: %clang_cc1 -triple riscv32 -emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple riscv32 -emit-llvm -fforce-enable-int128 %s -o - \
-// RUN: | FileCheck %s -check-prefixes=CHECK,CHECK-FORCEINT128
-
-#include 
-#include 
-
-// CHECK-LABEL: define void @f_void()
-void f_void(void) {}
-
-// Scalar arguments and return values smaller than the word size are extended
-// according to the sign of their type, up to 32 bits
-
-// CHECK-LABEL: define zeroext i1 @f_scalar_0(i1 zeroext %x)
-_Bool f_scalar_0(_Bool x) { return x; }
-
-// CHECK-LABEL: define signext i8 @f_scalar_1(i8 signext %x)
-int8_t f_scalar_1(int8_t x) { return x; }
-
-// CHECK-LABEL: define zeroext i8 @f_scalar_2(i8 zeroext %x)
-uint8_t f_scalar_2(uint8_t x) { return x; }
-
-// CHECK-LABEL: define i32 @f_scalar_3(i32 %x)
-int32_t f_scalar_3(int32_t x) { return x; }
-
-// CHECK-LABEL: define i64 @f_scalar_4(i64 %x)
-int64_t f_scalar_4(int64_t x) { return x; }
-
-#ifdef __SIZEOF_INT128__
-// CHECK-FORCEINT128-LABEL: define i128 @f_scalar_5(i128 %x)
-__int128_t f_scalar_5(__int128_t x) { return x; }
-#endif
-
-// CHECK-LABEL: define float @f_fp_scalar_1(float %x)
-float f_fp_scalar_1(float x) { return x; }
-
-// CHECK-LABEL: define double @f_fp_scalar_2(double %x)
-double f_fp_scalar_2(double x) { return x; }
-
-// Scalars larger than 2*xlen are passed/returned indirect. However, the
-// RISC-V LLVM backend can handle this fine, so the function doesn't need to
-// be modified.
-
-// CHECK-LABEL: define fp128 @f_fp_scalar_3(fp128 %x)
-long double f_fp_scalar_3(long double x) { return x; }
-
-// Empty structs or unions are ignored.
-
-struct empty_s {};
-
-// CHECK-LABEL: define void @f_agg_empty_struct()
-struct empty_s f_agg_empty_struct(struct empty_s x) {
-  return x;
-}
-
-union empty_u {};
-
-// CHECK-LABEL: define void @f_agg_empty_union()
-union empty_u f_agg_empty_union(union empty_u x) {
-  return x;
-}
-
-// Aggregates <= 2*xlen may be passed in registers, so will be coerced to
-// integer arguments. The rules for return are the same.
-
-struct tiny {
-  uint8_t a, b, c, d;
-};
-
-// CHECK-LABEL: define void @f_agg_tiny(i32 %x.coerce)
-void f_agg_tiny(struct tiny x) {
-  x.a += x.b;
-  x.c += x.d;
-}
-
-// CHECK-LABEL: define i32 @f_agg_tiny_ret()
-struct tiny f_agg_tiny_ret() {
-  return (struct tiny){1, 2, 3, 4};
-}
-
-typedef uint8_t v4i8 __attribute__((vector_size(4)));
-typedef int32_t v1i32 __attribute__((vector_size(4)));
-
-// CHECK-LABEL: define void @f_vec_tiny_v4i8(i32 %x.coerce)
-void f_vec_tiny_v4i8(v4i8 x) {
-  x[0] = x[1];
-  x[2] = x[3];
-}
-
-// CHECK-LABEL: define i32 @f_vec_tiny_v4i8_ret()
-v4i8 f_vec_tiny_v4i8_ret() {
-  return (v4i8){1, 2, 3, 4};
-}
-
-// CHECK-LABEL: define void @f_vec_tiny_v1i32(i32 %x.coerce)
-void f_vec_tiny_v1i32(v1i32 x) {
-  x[0] = 114;
-}
-
-// CHECK-LABEL: define i32 @f_vec_tiny_v1i32_ret()
-v1i32 f_vec_tiny_v1i32_ret() {
-  return (v1i32){1};
-}
-
-struct small {
-  int32_t a, *b;
-};
-
-// CHECK-LABEL: define void @f_agg_small([2 x i32] %x.coerce)
-void f_agg_small(struct small x) {
-  x.a += *x.b;
-  x.b = 
-}
-
-// CHECK-LABEL: define [2 x i32] @f_agg_small_ret()
-struct small f_agg_small_ret() {
-  return (struct small){1, 0};
-}
-
-typedef uint8_t v8i8 __attribute__((vector_size(8)));
-typedef int64_t v1i64 __attribute__((vector_size(8)));
-
-// CHECK-LABEL: define void @f_vec_small_v8i8(i64 %x.coerce)
-void f_vec_small_v8i8(v8i8 x) {
-  x[0] = x[7];
-}
-
-// CHECK-LABEL: define i64 @f_vec_small_v8i8_ret()
-v8i8 f_vec_small_v8i8_ret() {
-  return (v8i8){1, 2, 3, 4, 5, 6, 7, 8};
-}
-
-// CHECK-LABEL: define void @f_vec_small_v1i64(i64 %x.coerce)
-void f_vec_small_v1i64(v1i64 x) {
-  x[0] = 114;
-}
-
-// CHECK-LABEL: define i64 @f_vec_small_v1i64_ret()
-v1i64 f_vec_small_v1i64_ret() {
-  return (v1i64){1};
-}
-
-// Aggregates of 2*xlen size and 2*xlen alignment should be coerced to a
-// single 2*xlen-sized argument, to 

[PATCH] D60225: [clang-format] [PR19056] Add support for indenting class members and methods one level under the modifiers

[Reuben Thomas via Phabricator via cfe-commits]

reuk added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2009
+  // classes case
+  if (Style.AccessModifierIndentation && Line->Level % 2 == 0)
+--Line->Level;

klimek wrote:
> What if the class starts at level 1? (for example, inside a function or due 
> to namespace indentation)
> 
> namespace A {
>   class B {
> public:
>   ..
>   };
> }
Probably worth adding a test or two that format nested classes, classes inside 
namespaces, classes defined inside functions etc.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2253
   parseBlock(/*MustBeDeclaration=*/true, /*AddLevel=*/true,
- /*MunchSemi=*/false);
+ /*MunchSemi=*/false,
+ /*IndentedAccessModifiers=*/ContainsAccessModifier);

Yeah, this would look way nicer using a settings struct.



Comment at: clang/lib/Format/UnwrappedLineParser.h:89
   void parseBlock(bool MustBeDeclaration, bool AddLevel = true,
-  bool MunchSemi = true);
+  bool MunchSemi = true, bool IndentedAccessModifiers = false);
   void parseChildBlock();

Adjacent `bool` parameters is a bit nasty. Maybe pass a struct of settings 
instead? 
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#i4-make-interfaces-precisely-and-strongly-typed


Repository:
  rC Clang

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

https://reviews.llvm.org/D60225



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

r357985 - [ASTImporter] Fix in ASTImporter::Import_New(const Decl *)

[Bjorn Pettersson via cfe-commits]

Author: bjope
Date: Tue Apr  9 02:12:32 2019
New Revision: 357985

URL: http://llvm.org/viewvc/llvm-project?rev=357985=rev
Log:
[ASTImporter] Fix in ASTImporter::Import_New(const Decl *)

Make sure ASTImporter::Import_New(const Decl *) returns
a Expected and not Expected to
make the clang/unittests/AST/ASTImporterTest.cpp compile
without the warning

 clang/unittests/AST/ASTImporterTest.cpp:117:12: error: no viable conversion 
from 'Expected' to 'Expected'
return Imported;

(I got the above when building with clang 3.6).

Modified:
cfe/trunk/include/clang/AST/ASTImporter.h

Modified: cfe/trunk/include/clang/AST/ASTImporter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTImporter.h?rev=357985=357984=357985=diff
==
--- cfe/trunk/include/clang/AST/ASTImporter.h (original)
+++ cfe/trunk/include/clang/AST/ASTImporter.h Tue Apr  9 02:12:32 2019
@@ -215,7 +215,7 @@ class TypeSourceInfo;
 /// \returns The equivalent declaration in the "to" context, or the import
 /// error.
 llvm::Expected Import_New(Decl *FromD);
-llvm::Expected Import_New(const Decl *FromD) {
+llvm::Expected Import_New(const Decl *FromD) {
   return Import_New(const_cast(FromD));
 }
 // FIXME: Remove this version.


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

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas added a comment.

Ok, I just fixed a remaining typo and rebased the patch to have it work with a 
more recent version of the code base, so I guess this patch is ready for commit.
Jonas, can you commit this change for me? I still don't have commit access, I 
just applied for it some days ago.


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

https://reviews.llvm.org/D59870



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

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas updated this revision to Diff 194261.
ztamas added a comment.

Fixed a typo in release notes.
Rebased the patch withh git pull -r.


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

https://reviews.llvm.org/D59870

Files:
  clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
  
clang-tools-extra/test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp
  clang-tools-extra/test/clang-tidy/bugprone-too-small-loop-variable.cpp

Index: clang-tools-extra/test/clang-tidy/bugprone-too-small-loop-variable.cpp
===
--- clang-tools-extra/test/clang-tidy/bugprone-too-small-loop-variable.cpp
+++ clang-tools-extra/test/clang-tidy/bugprone-too-small-loop-variable.cpp
@@ -1,4 +1,8 @@
-// RUN: %check_clang_tidy %s bugprone-too-small-loop-variable %t -- -- --target=x86_64-linux
+// RUN: %check_clang_tidy %s bugprone-too-small-loop-variable %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: bugprone-too-small-loop-variable.MagnitudeBitsUpperLimit, \
+// RUN:   value: 1024}]}" \
+// RUN:   -- --target=x86_64-linux
 
 long size() { return 294967296l; }
 
Index: clang-tools-extra/test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/bugprone-too-small-loop-variable-magniute-bits-upper-limit.cpp
@@ -0,0 +1,23 @@
+// RUN: %check_clang_tidy %s bugprone-too-small-loop-variable %t -- -- --target=x86_64-linux
+
+// MagnitudeBitsUpperLimit = 16 (default value)
+
+unsigned long size() { return 294967296l; }
+
+void voidFilteredOutForLoop1() {
+  for (long i = 0; i < size(); ++i) {
+// no warning
+  }
+}
+
+void voidCaughtForLoop1() {
+  for (int i = 0; i < size(); ++i) {
+// no warning
+  }
+}
+
+void voidCaughtForLoop2() {
+  for (short i = 0; i < size(); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower type 'short' than iteration's upper bound 'unsigned long' [bugprone-too-small-loop-variable]
+  }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
@@ -27,3 +27,20 @@
 
 This algorithm works for small amount of objects, but will lead to freeze for a
 a larger user input.
+
+.. option:: MagnitudeBitsUpperLimit
+
+  Upper limit for the magnitude bits of the loop variable. If it's set the check
+  filters out those catches in which the loop variable's type has more magnitude
+  bits as the specified upper limit. The default value is 16.
+  For example, if the user sets this option to 31 (bits), then a 32-bit ``unsigend int``
+  is ignored by the check, however a 32-bit ``int`` is not (A 32-bit ``signed int``
+  has 31 magnitude bits).
+
+.. code-block:: c++
+
+  int main() {
+long size = 294967296l;
+for (unsigned i = 0; i < size; ++i) {} // no warning with MagnitudeBitsUpperLimit = 31 on a system where unsigned is 32-bit
+for (int i = 0; i < size; ++i) {} // warning with MagnitudeBitsUpperLimit = 31 on a system where int is 32-bit
+  }
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -119,6 +119,12 @@
   `CommentUserDefiniedLiterals`, `CommentStringLiterals`,
   `CommentCharacterLiterals` & `CommentNullPtrs` options.
 
+- The :doc:`bugprone-too-small-loop-variable
+  ` now supports
+  `MagnitudeBitsUpperLimit` option. The default value was set to 16,
+  which greatly reduces warnings related to loops which are unlikely to
+  cause an actual functional bug.
+
 - The :doc:`google-runtime-int `
   check has been disabled in Objective-C++.
 
Index: clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/TooSmallLoopVariableCheck.h
@@ -29,10 +29,14 @@
 /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-too-small-loop-variable.html
 class TooSmallLoopVariableCheck : public ClangTidyCheck {
 public:
-  TooSmallLoopVariableCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  TooSmallLoopVariableCheck(StringRef Name, ClangTidyContext *Context);
+
+  void storeOptions(ClangTidyOptions::OptionMap ) override;
   void 

[PATCH] D60225: [clang-format] [PR19056] Add support for indenting class members and methods one level under the modifiers

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: clang/include/clang/Format/Format.h:50
 struct FormatStyle {
+  /// Indents after access modifiers. i.e.
+  /// \code

I think we need to explain this a bit more:
What this does is:
Indent classes with access modifiers at 2x indent compared to classes without 
access modifiers, while keeping the access modifiers at a normal indent.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2009
+  // classes case
+  if (Style.AccessModifierIndentation && Line->Level % 2 == 0)
+--Line->Level;

What if the class starts at level 1? (for example, inside a function or due to 
namespace indentation)

namespace A {
  class B {
public:
  ..
  };
}



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2022
+  // After we wrap for Access modifier then indent a level if desired
+  if (Style.AccessModifierIndentation && Line->Level >= 1)
+++Line->Level;

Similarly, I think we need to remember whether we unindented, as otherwise I 
think we can run into cases where this is true, but the previous was false 
(class declared at level > 1).



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2230
+  Next = Tokens->getNextToken();
+  if (!Next)
+break;

We should structure this like other parse loops in this file, using switch and 
eof (instead of !Next). See parseParens() for a good example.



Repository:
  rC Clang

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

https://reviews.llvm.org/D60225



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

[PATCH] D59408: [clang-format] [PR25010] Extend AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:401
+  * ``SIS_AlwaysNoElse`` (in configuration: ``AlwaysNoElse``)
+Allow short if/else if statements even if the else is a compound statement.
+

MyDeveloperDay wrote:
> klimek wrote:
> > I'd try to make this either unconditionally what we do, or decide against 
> > doing it.
> see note before, this is really about maintaining compatibility, I don't want 
> to make the assumption that everyone likes
> 
> 
> ```
> if (x) return 1;
> else if (x) return 2;
> else return 3;
> 
> ```
> 
> There could easily be users who want this.
> 
> ```
> if (x) return 1;
> else if (x) return 2;
> else 
>return 3;
> ```
> 
> I don't think it over complicates it, but it keeps the flexibility. The name 
> of the option may not be the best 'suggestions welcome'
The second one seems simply broken. I think fixing this is great, but keeping 
the broken version seems odd :)



Comment at: clang/unittests/Format/FormatTest.cpp:520
+  verifyFormat("if (a) f();\n"
+   "else if (b) g();\n",
+   AllowsMergedIf);

krasimir wrote:
> I don't understand why is this preferred over:
> ```
> if (a) f();
> else
>   if (b) g();
> ```
> 
> Isn't syntactically the nested `if (b) g();` a compound statement? I might be 
> wrong.
There's no compound statement, but the else-stmt is another if-stmt. That said, 
I don't think this is about what structure the C++ standard has (as that's not 
what clang-format understands anyway), but whether the structure we put in 
helps readability. In this case, why does the irregular break before the if 
help readability? It seems like we're having a break for no good reason.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59408



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

[PATCH] D59332: [clang-format] AlignConsecutiveDeclarations fails with attributes

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.cpp:468
C.Tok->is(TT_FunctionDeclarationName) ||
+   C.Tok->startsSequence(
+   tok::l_paren, tok::l_paren, tok::identifier, 
tok::coloncolon,

Can you add a comment explaining the newly added startsSequences, or perhaps 
even pull out a function?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59332



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

[PATCH] D60363: [clang-format] [PR41170] Break after return type ignored with certain comments positions

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D60363#1459297 , @lebedev.ri wrote:

> Please either subscribe the correct lists or set the correct repo in 
> differential params.


Thanks for the correction, I was never really clear which one to use for the 
project as the blurb at the top of clang-tools-extra says "clang-format"

https://reviews.llvm.org/tag/clang-tools-extra/

But going forward I'll follow your example here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60363



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

[PATCH] D60363: [clang-format] [PR41170] Break after return type ignored with certain comments positions

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Please either subscribe the correct lists or set the correct repo in 
differential params.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60363



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

[PATCH] D52527: [clang-format] fix Bug 38686: add AfterCaseLabel to BraceWrapping

[Owen Pan via Phabricator via cfe-commits]

owenpan closed this revision.
owenpan added a comment.

llvm-svn: 357957


Repository:
  rC Clang

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

https://reviews.llvm.org/D52527



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

[PATCH] D59924: [PowerPC] [Clang] Port MMX intrinsics and basic test cases to Power

[Chaofan Qiu via Phabricator via cfe-commits]

qiucf updated this revision to Diff 194253.
qiucf retitled this revision from "[PowerPC][Clang] Port MMX intrinsics and 
basic test cases to Power" to "[PowerPC] [Clang] Port MMX intrinsics and basic 
test cases to Power".
qiucf edited the summary of this revision.
qiucf added a comment.

- Add more check of mmintrin generated IR against endianness.
- Fix typo in PPCLinux header.
- Fix command error in ppc intrinsics header test.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59924

Files:
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/PPCLinux.cpp
  clang/lib/Driver/ToolChains/PPCLinux.h
  clang/lib/Headers/CMakeLists.txt
  clang/lib/Headers/ppc_wrappers/mmintrin.h
  clang/test/CodeGen/ppc-mmintrin.c
  clang/test/Headers/ppc-intrinsics.c

Index: clang/test/Headers/ppc-intrinsics.c
===
--- /dev/null
+++ clang/test/Headers/ppc-intrinsics.c
@@ -0,0 +1,13 @@
+// REQUIRES: powerpc-registered-target
+
+// RUN: %clang -S -emit-llvm -DNO_WARN_X86_INTRINSICS -target powerpc64-gnu-linux %s -Xclang -verify -o - | FileCheck %s
+// RUN: %clang -S -emit-llvm -DNO_WARN_X86_INTRINSICS -target powerpc64-gnu-linux %s -Xclang -verify -x c++ -o - | FileCheck %s
+// expected-no-diagnostics
+
+// RUN: not %clang -S -emit-llvm -target powerpc64-gnu-linux %s -o /dev/null 2>&1 | FileCheck %s -check-prefix=CHECK-ERROR
+
+#include 
+// CHECK-ERROR: mmintrin.h:{{[0-9]+}}:{{[0-9]+}}: error: "Please read comment above. Use -DNO_WARN_X86_INTRINSICS to disable this error."
+
+// CHECK: target triple = "powerpc64-
+// CHECK: !llvm.module.flags =
Index: clang/test/CodeGen/ppc-mmintrin.c
===
--- /dev/null
+++ clang/test/CodeGen/ppc-mmintrin.c
@@ -0,0 +1,60 @@
+// REQUIRES: powerpc-registered-target
+
+// RUN: %clang -S -emit-llvm -DNO_WARN_X86_INTRINSICS -mcpu=pwr8 -target powerpc64-gnu-linux %s \
+// RUN:-mllvm -disable-llvm-optzns -o - | llvm-cxxfilt | FileCheck %s --check-prefixes=CHECK,CHECK-BE
+// RUN: %clang -S -emit-llvm -DNO_WARN_X86_INTRINSICS -mcpu=pwr8 -target powerpc64le-gnu-linux %s \
+// RUN:-mllvm -disable-llvm-optzns -o - | llvm-cxxfilt | FileCheck %s --check-prefixes=CHECK,CHECK-LE
+
+#include 
+
+unsigned long long int ull1, ull2;
+__m64 m1, m2, res;
+
+void __attribute__((noinline))
+test_packs() {
+  res = _mm_packs_pu16((__m64)ull1, (__m64)ull2);
+  res = _mm_packs_pi16((__m64)ull1, (__m64)ull2);
+  res = _mm_packs_pi32((__m64)ull1, (__m64)ull2);
+}
+
+// CHECK-LABEL: @test_packs
+
+// CHECK: i64 @_mm_packs_pu16(i64 [[REG1:[0-9a-zA-Z_%.]+]], i64 [[REG2:[0-9a-zA-Z_%.]+]])
+// CHECK: store i64 [[REG1]], i64* [[REG3:[0-9a-zA-Z_%.]+]], align 8
+// CHECK-NEXT: store i64 [[REG2]], i64* [[REG4:[0-9a-zA-Z_%.]+]], align 8
+// CHECK-LE: load i64, i64* [[REG3]], align 8
+// CHECK: load i64, i64* [[REG4]], align 8
+// CHECK-BE: load i64, i64* [[REG3]], align 8
+// CHECK: [[REG5:[0-9a-zA-Z_%.]+]] = call <8 x i16> @vec_cmplt
+// CHECK-NEXT: store <8 x i16> [[REG5]], <8 x i16>* [[REG6:[0-9a-zA-Z_%.]+]], align 16
+// CHECK-NEXT: [[REG7:[0-9a-zA-Z_%.]+]] = load <8 x i16>, <8 x i16>* [[REG8:[0-9a-zA-Z_%.]+]], align 16
+// CHECK-NEXT: [[REG9:[0-9a-zA-Z_%.]+]] = load <8 x i16>, <8 x i16>* [[REG8]], align 16
+// CHECK-NEXT: [[REG10:[0-9a-zA-Z_%.]+]] = call <16 x i8> @vec_packs(unsigned short vector[8], unsigned short vector[8])(<8 x i16> [[REG7]], <8 x i16> [[REG9]])
+// CHECK-NEXT: store <16 x i8> [[REG10]], <16 x i8>* [[REG11:[0-9a-zA-Z_%.]+]], align 16
+// CHECK-NEXT: [[REG12:[0-9a-zA-Z_%.]+]] = load <8 x i16>, <8 x i16>* [[REG6]], align 16
+// CHECK-NEXT: [[REG13:[0-9a-zA-Z_%.]+]] = load <8 x i16>, <8 x i16>* [[REG6]], align 16
+// CHECK-NEXT: [[REG14:[0-9a-zA-Z_%.]+]] = call <16 x i8> @vec_pack(bool vector[8], bool vector[8])(<8 x i16> [[REG12]], <8 x i16> [[REG13]])
+// CHECK-NEXT: store <16 x i8> [[REG14]], <16 x i8>* [[REG15:[0-9a-zA-Z_%.]+]], align 16
+// CHECK-NEXT: [[REG16:[0-9a-zA-Z_%.]+]] = load <16 x i8>, <16 x i8>* [[REG11]], align 16
+// CHECK-NEXT: [[REG17:[0-9a-zA-Z_%.]+]] = load <16 x i8>, <16 x i8>* [[REG15]], align 16
+// CHECK-NEXT: call <16 x i8> @vec_sel(unsigned char vector[16], unsigned char vector[16], bool vector[16])(<16 x i8> [[REG16]], <16 x i8> zeroinitializer, <16 x i8> [[REG17]])
+
+// CHECK: i64 @_mm_packs_pi16(i64 [[REG18:[0-9a-zA-Z_%.]+]], i64 [[REG19:[0-9a-zA-Z_%.]+]])
+// CHECK: store i64 [[REG18]], i64* [[REG20:[0-9a-zA-Z_%.]+]], align 8
+// CHECK-NEXT: store i64 [[REG19]], i64* [[REG21:[0-9a-zA-Z_%.]+]], align 8
+// CHECK-LE: load i64, i64* [[REG20]], align 8
+// CHECK: load i64, i64* [[REG21]], align 8
+// CHECK-BE: load i64, i64* [[REG20]], align 8
+// CHECK: [[REG22:[0-9a-zA-Z_%.]+]] = load <8 x i16>, <8 x i16>* [[REG23:[0-9a-zA-Z_%.]+]], align 16
+// CHECK-NEXT: [[REG24:[0-9a-zA-Z_%.]+]] = load <8 x i16>, <8 x i16>* [[REG23]], align 16

[PATCH] D60417: [libunwind] Add support for ARMv7-M architecture which uses the Thumb 2 ISA (unified syntax)

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D60417#1459128 , @jeremfg wrote:

> In D60417#1458778 , @mstorsjo wrote:
>
> > The change looks sensible to me, but should we maybe even skip the `#if` 
> > altogether? If the files uses unified syntax and can't be parsed in thumb 
> > mode otherwise, there's maybe no need for conditionals at all?
>
>
> I'm no expert but I just read that UAL (Unified Assembler Language), while 
> being backward compatible with the old ARM syntax, is **not** compatible with 
> the previous Thumb syntax.
>  
> http://downloads.ti.com/docs/esd/SPNU118/unified-assembly-language-syntax-support-spnu118.html
>
> But again, perhaps it doesn't matter in the specific case that concerns us 
> here? I guess it depends on whether the code in the ###if 
> !defined(__ARM_ARCH_ISA_ARM) && __ARM_ARCH_ISA_THUMB == 1## condition that 
> follows contains only valid UAL syntax as well. Which I am not qualified to 
> confirm or not.


Oh, sorry, I was sloppy and didn't actually check the rest of the file to see 
the existing conditional alternative codepaths with 
`!defined(__ARM_ARCH_ISA_ARM) && __ARM_ARCH_ISA_THUMB == 1`.

Given that, the suggested form of this patch is indeed correct - sorry for the 
noise.

I can commit the patch for you, but I'd need approval by either of @EricWF, 
@ldionne or @mclow.lists first.


Repository:
  rUNW libunwind

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

https://reviews.llvm.org/D60417



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

[PATCH] D59347: [DebugInfo] Combine Trivial and NonTrivial flags

[Aaron Smith via Phabricator via cfe-commits]

asmith updated this revision to Diff 194249.

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

https://reviews.llvm.org/D59347

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGenCXX/debug-info-composite-triviality.cpp


Index: test/CodeGenCXX/debug-info-composite-triviality.cpp
===
--- test/CodeGenCXX/debug-info-composite-triviality.cpp
+++ test/CodeGenCXX/debug-info-composite-triviality.cpp
@@ -25,34 +25,40 @@
 
 // Cases to test composite type's triviality
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "Union",{{.*}}flags: 
{{.*}}DIFlagTrivial
+// CHECK-DAG:  {{.*}}!DIGlobalVariable(name: "Union",
+// CHECK-DAG-NEXT: {{^((?!\bDIFlagNonTrivial\b).)*$}}
 union Union {
   int a;
 } Union;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "Trivial",{{.*}}flags: 
{{.*}}DIFlagTrivial
+// CHECK-DAG:  {{.*}}!DIGlobalVariable(name: "Trivial",
+// CHECK-DAG-NEXT: {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct Trivial {
   int i;
 } Trivial;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialA",{{.*}}flags: 
{{.*}}DIFlagTrivial
+// CHECK-DAG:   {{.*}}!DIGlobalVariable(name: "TrivialA",
+// CHECK-DAG-NEXT:  {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct TrivialA {
   TrivialA() = default;
 } TrivialA;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialB",{{.*}}flags: 
{{.*}}DIFlagTrivial
+// CHECK-DAG:   {{.*}}!DIGlobalVariable(name: "TrivialB",
+// CHECK-DAG-NEXT:  {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct TrivialB {
   int m;
   TrivialB(int x) { m = x; }
   TrivialB() = default;
 } TrivialB;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialC",{{.*}}flags: 
{{.*}}DIFlagTrivial
+// CHECK-DAG:   {{.*}}!DIGlobalVariable(name: "TrivialC",
+// CHECK-DAG-NEXT:  {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct TrivialC {
   struct Trivial x;
 } TrivialC;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialD",{{.*}}flags: 
{{.*}}DIFlagTrivial
+// CHECK-DAG:   {{.*}}!DIGlobalVariable(name: "TrivialD",
+// CHECK-DAG-NEXT:  {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct NT {
   NT() {};
 };
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -3034,10 +3034,8 @@
 else
   Flags |= llvm::DINode::FlagTypePassByValue;
 
-// Record if a C++ record is trivial type.
-if (CXXRD->isTrivial())
-  Flags |= llvm::DINode::FlagTrivial;
-else
+// Record if a C++ record is non-trivial type.
+if (!CXXRD->isTrivial())
   Flags |= llvm::DINode::FlagNonTrivial;
   }
 


Index: test/CodeGenCXX/debug-info-composite-triviality.cpp
===
--- test/CodeGenCXX/debug-info-composite-triviality.cpp
+++ test/CodeGenCXX/debug-info-composite-triviality.cpp
@@ -25,34 +25,40 @@
 
 // Cases to test composite type's triviality
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "Union",{{.*}}flags: {{.*}}DIFlagTrivial
+// CHECK-DAG:  {{.*}}!DIGlobalVariable(name: "Union",
+// CHECK-DAG-NEXT: {{^((?!\bDIFlagNonTrivial\b).)*$}}
 union Union {
   int a;
 } Union;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "Trivial",{{.*}}flags: {{.*}}DIFlagTrivial
+// CHECK-DAG:  {{.*}}!DIGlobalVariable(name: "Trivial",
+// CHECK-DAG-NEXT: {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct Trivial {
   int i;
 } Trivial;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialA",{{.*}}flags: {{.*}}DIFlagTrivial
+// CHECK-DAG:   {{.*}}!DIGlobalVariable(name: "TrivialA",
+// CHECK-DAG-NEXT:  {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct TrivialA {
   TrivialA() = default;
 } TrivialA;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialB",{{.*}}flags: {{.*}}DIFlagTrivial
+// CHECK-DAG:   {{.*}}!DIGlobalVariable(name: "TrivialB",
+// CHECK-DAG-NEXT:  {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct TrivialB {
   int m;
   TrivialB(int x) { m = x; }
   TrivialB() = default;
 } TrivialB;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialC",{{.*}}flags: {{.*}}DIFlagTrivial
+// CHECK-DAG:   {{.*}}!DIGlobalVariable(name: "TrivialC",
+// CHECK-DAG-NEXT:  {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct TrivialC {
   struct Trivial x;
 } TrivialC;
 
-// CHECK-DAG: !DICompositeType({{.*}}, name: "TrivialD",{{.*}}flags: {{.*}}DIFlagTrivial
+// CHECK-DAG:   {{.*}}!DIGlobalVariable(name: "TrivialD",
+// CHECK-DAG-NEXT:  {{^((?!\bDIFlagNonTrivial\b).)*$}}
 struct NT {
   NT() {};
 };
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -3034,10 +3034,8 @@
 else
   Flags |= llvm::DINode::FlagTypePassByValue;
 
-// Record if a C++ record is trivial type.
-if (CXXRD->isTrivial())
-  Flags |= llvm::DINode::FlagTrivial;
-else
+// Record if a C++ record is non-trivial type.
+if (!CXXRD->isTrivial())
   Flags |= 

[PATCH] D59924: [PowerPC][Clang] Port MMX intrinsics and basic test cases to Power

[Chaofan Qiu via Phabricator via cfe-commits]

qiucf marked 2 inline comments as done.
qiucf added inline comments.



Comment at: clang/lib/Driver/ToolChains/PPCLinux.cpp:22
+  if (getArch() != llvm::Triple::ppc &&
+  !DriverArgs.hasArg(clang::driver::options::OPT_nostdinc) &&
+  !DriverArgs.hasArg(options::OPT_nobuiltininc)) {

jsji wrote:
> Why we want to exclude these includes when `nostdinc` is on? These include 
> files are NOT standard include files.
> `-nostdinc`: Do not search the standard system directories or compiler 
> builtin directories for include files.
> From Clang's man page

And on Windows toolchain (MSVC.cpp), if turning on `-nostdinc`, Windows SDK 
files will also not be added into path.



Comment at: clang/test/Headers/ppc-intrinsics.c:7
+
+// RUN: NOT %clang -S -emit-llvm -target powerpc64-gnu-linux %s -o /dev/null 
2>&1 | FileCheck %s -check-prefix=CHECK-ERROR
+

jsji wrote:
> NOT? 
> ` NOT: command not found ^`
It should be `not`, in downcase. This is what other test cases (see 
`test/CXX/temp/temp.spec/no-body.cpp`) use, to assert a non-zero exit code of a 
command. Otherwise, `llvm-lit` would complain and give a failure.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59924



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

[PATCH] D59415: Do not resolve directory junctions for `-fdiagnostics-absolute-paths` on Windows.

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin added a comment.

Ping.

Please note, that, in contrary to POSIX OSes, on Windows a path like 
`dir1\\..` refers to `dir1`. That's why we do not need to ask 
the system for the fully expanded path and can do all calculations internally.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59415



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