[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

[Ten Tzen via Phabricator via cfe-commits]

tentzen updated this revision to Diff 256836.
tentzen marked an inline comment as done.
tentzen added a comment.

Per Eli's feedbacks:
(1) a test case (windows-seh-abnormal-exit.c) is added under clang\test\CodeGen 
directory.  
(2) an assert is added to catch the extremely rare case that the number of 
JumpDests in a function is greater than 255.  The max. number of JumpDests is 1 
(return) + 2*M + N, where M is the number of for/while-loops and N is the 
number of Goto targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77936

Files:
  clang/lib/CodeGen/CGCleanup.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/EHScopeStack.h
  clang/test/CodeGen/windows-seh-abnormal-exits.c

Index: clang/test/CodeGen/windows-seh-abnormal-exits.c
===
--- /dev/null
+++ clang/test/CodeGen/windows-seh-abnormal-exits.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple x86_64-windows -Wno-implicit-function-declaraton -S -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: %[[src:[0-9-]+]] = call i8* @llvm.localaddress()
+// CHECK-NEXT: %cleanup.dest = load i32, i32* %cleanup.dest.slot, align 4
+// CHECK-NEXT: %[[src2:[0-9-]+]] = trunc i32 %cleanup.dest to i8
+// CHECK-NEXT: call void @"?fin$0@0@seh_abnormal_exits@@"(i8 %[[src2]], i8* %[[src]])
+
+void seh_abnormal_exits(int *Counter) {
+  for (int i = 0; i < 5; i++) {
+__try {
+  if (i == 0)
+continue;   // abnormal termination
+  else if (i == 1)
+goto t10;   // abnormal termination
+  else if (i == 2)
+__leave;  // normal execution
+  else if (i == 4)
+return;  // abnormal termination
+}
+__finally {
+  if (AbnormalTermination()) {
+*Counter += 1;
+  }
+}
+  t10:;
+  }
+  return; // *Counter == 3
+}
+
Index: clang/lib/CodeGen/EHScopeStack.h
===
--- clang/lib/CodeGen/EHScopeStack.h
+++ clang/lib/CodeGen/EHScopeStack.h
@@ -158,9 +158,10 @@
 /// Generation flags.
 class Flags {
   enum {
-F_IsForEH = 0x1,
+F_IsForEH = 0x1,
 F_IsNormalCleanupKind = 0x2,
-F_IsEHCleanupKind = 0x4
+F_IsEHCleanupKind = 0x4,
+F_HasSehAbnormalExits = 0x8,
   };
   unsigned flags;
 
@@ -179,8 +180,10 @@
   /// cleanup.
   bool isEHCleanupKind() const { return flags & F_IsEHCleanupKind; }
   void setIsEHCleanupKind() { flags |= F_IsEHCleanupKind; }
-};
 
+  bool hasSehAbnormalExits() const { return flags & F_HasSehAbnormalExits; }
+  void setHasSehAbnormalExits() { flags |= F_HasSehAbnormalExits; }
+};
 
 /// Emit the cleanup.  For normal cleanups, this is run in the
 /// same EH context as when the cleanup was pushed, i.e. the
Index: clang/lib/CodeGen/CGException.cpp
===
--- clang/lib/CodeGen/CGException.cpp
+++ clang/lib/CodeGen/CGException.cpp
@@ -1639,6 +1639,19 @@
 
 llvm::Value *IsForEH =
 llvm::ConstantInt::get(CGF.ConvertType(ArgTys[0]), F.isForEHCleanup());
+
+// Except _leave and fall-through at the end, all other exits in a _try
+//   (return/goto/continue/break) are considered as abnormal terminations
+//   since _leave/fall-through is always Indexed 0,
+//   just use NormalCleanupDestSlot (>= 1 for goto/return/..),
+//   as 1st Arg to indicate abnormal termination
+if (!F.isForEHCleanup() && F.hasSehAbnormalExits()) {
+  assert(CGF.NextCleanupDestIndex < 256 && " JumpDest Index exceeds 255");
+  Address Addr = CGF.getNormalCleanupDestSlot();
+  llvm::Value* Load = CGF.Builder.CreateLoad(Addr, "cleanup.dest");
+  IsForEH = CGF.Builder.CreateTrunc(Load, CGM.Int8Ty);
+}
+
 Args.add(RValue::get(IsForEH), ArgTys[0]);
 Args.add(RValue::get(FP), ArgTys[1]);
 
Index: clang/lib/CodeGen/CGCleanup.cpp
===
--- clang/lib/CodeGen/CGCleanup.cpp
+++ clang/lib/CodeGen/CGCleanup.cpp
@@ -860,6 +860,9 @@
 // TODO: base this on the number of branch-afters and fixups
 const unsigned SwitchCapacity = 10;
 
+// pass the abnormal exit flag to Fn (SEH cleanup)
+cleanupFlags.setHasSehAbnormalExits();
+
 llvm::LoadInst *Load =
   createLoadInstBefore(getNormalCleanupDestSlot(), "cleanup.dest",
nullptr);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77882: [clang-tidy] Add option to use alpha checkers from clang-analyzer when using `run-clang-tidy.py`

[Andi via Phabricator via cfe-commits]

Abpostelnicu updated this revision to Diff 256825.
Abpostelnicu added a comment.

Add release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77882

Files:
  clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
  clang-tools-extra/docs/ReleaseNotes.rst


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -188,6 +188,11 @@
 - The 'fuchsia-restrict-system-headers' check was renamed to 
:doc:`portability-restrict-system-includes
   `
 
+Other improvements
+^^
+
+- For 'run-clang-tidy.py' add option to use alpha checkers from clang-analyzer.
+
 Improvements to include-fixer
 -
 
@@ -210,4 +215,3 @@
 
 Clang-tidy visual studio plugin
 ---
-
Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -78,10 +78,12 @@
 
 
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
-header_filter, extra_arg, extra_arg_before, quiet,
-config):
+header_filter, allow_enabling_alpha_checkers,
+extra_arg, extra_arg_before, quiet, config):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
+  if allow_enabling_alpha_checkers is not None:
+start.append('-allow-enabling-analyzer-alpha-checkers')
   if header_filter is not None:
 start.append('-header-filter=' + header_filter)
   if checks:
@@ -159,6 +161,7 @@
 name = queue.get()
 invocation = get_tidy_invocation(name, args.clang_tidy_binary, args.checks,
  tmpdir, build_path, args.header_filter,
+ args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg_before,
  args.quiet, args.config)
 
@@ -179,6 +182,9 @@
'in a compilation database. Requires '
'clang-tidy and clang-apply-replacements in 
'
'$PATH.')
+  parser.add_argument('-allow-enabling-alpha-checkers',
+  action='store_true', help='allow alpha checkers from '
+'clang-analyzer.')
   parser.add_argument('-clang-tidy-binary', metavar='PATH',
   default='clang-tidy',
   help='path to clang-tidy binary')
@@ -238,6 +244,8 @@
 
   try:
 invocation = [args.clang_tidy_binary, '-list-checks']
+if args.allow_enabling_alpha_checkers:
+  invocation.append('-allow-enabling-analyzer-alpha-checkers')
 invocation.append('-p=' + build_path)
 if args.checks:
   invocation.append('-checks=' + args.checks)


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -188,6 +188,11 @@
 - The 'fuchsia-restrict-system-headers' check was renamed to :doc:`portability-restrict-system-includes
   `
 
+Other improvements
+^^
+
+- For 'run-clang-tidy.py' add option to use alpha checkers from clang-analyzer.
+
 Improvements to include-fixer
 -
 
@@ -210,4 +215,3 @@
 
 Clang-tidy visual studio plugin
 ---
-
Index: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
===
--- clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
+++ clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
@@ -78,10 +78,12 @@
 
 
 def get_tidy_invocation(f, clang_tidy_binary, checks, tmpdir, build_path,
-header_filter, extra_arg, extra_arg_before, quiet,
-config):
+header_filter, allow_enabling_alpha_checkers,
+extra_arg, extra_arg_before, quiet, config):
   """Gets a command line for clang-tidy."""
   start = [clang_tidy_binary]
+  if allow_enabling_alpha_checkers is not None:
+start.append('-allow-enabling-analyzer-alpha-checkers')
   if header_filter is not None:
 start.append('-header-filter=' + header_filter)
   if checks:
@@ -159,6 +161,7 @@
 name = queue.get()
 invocation = get_tidy_invocation(name, args.clang_tidy_binary, args.checks,
  tmpdir, build_path, args.header_filter,
+ args.allow_enabling_alpha_checkers,
  args.extra_arg, args.extra_arg

[PATCH] D77882: [clang-tidy] Add option to use alpha checkers from clang-analyzer when using `run-clang-tidy.py`

[Andi via Phabricator via cfe-commits]

Abpostelnicu added a comment.

In D77882#1976216 , @sylvestre.ledru 
wrote:

> please add this to the release notes too :)
>  something like "new option -allow-enabling-alpha-checkers added to 
> run-clang-tidy to enable alpha checkers"


Is it ok just to add this to the `clang-tools-extra/docs/ReleaseNotes.rst` 
release notes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77882



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

[PATCH] D77962: PR38490: Support reading values out of __uuidof(X) expressions during constant evaluation.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith created this revision.
rsmith added reviewers: aeubanks, rnk.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We try to avoid materializing a full _GUID APValue wherever possible;
instead, when accessing an individual field, we only extract the
corresponding portion of the UUID string and convert that.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77962

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/test/CodeGenCXX/microsoft-uuidof.cpp
  clang/test/Parser/MicrosoftExtensions.cpp
  clang/test/SemaCXX/ms-uuid.cpp

Index: clang/test/SemaCXX/ms-uuid.cpp
===
--- clang/test/SemaCXX/ms-uuid.cpp
+++ clang/test/SemaCXX/ms-uuid.cpp
@@ -2,10 +2,10 @@
 // RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify -fms-extensions %s -Wno-deprecated-declarations
 
 typedef struct _GUID {
-  unsigned long Data1;
-  unsigned short Data2;
-  unsigned short Data3;
-  unsigned char Data4[8];
+  __UINT32_TYPE__ Data1;
+  __UINT16_TYPE__ Data2;
+  __UINT16_TYPE__ Data3;
+  __UINT8_TYPE__ Data4[8];
 } GUID;
 
 namespace {
@@ -111,4 +111,24 @@
 // declaration has a uuid attribute
 struct X{};
 
-struct __declspec(uuid("----")) X;
\ No newline at end of file
+struct __declspec(uuid("----")) X;
+
+namespace ConstantEvaluation {
+  class __declspec(uuid("1babb1ed-feed-c01d-1ced-decafc0ffee5")) Request;
+  constexpr GUID a = __uuidof(Request);
+  static_assert(a.Data1 == 0x1babb1ed, "");
+  static_assert(__uuidof(Request).Data1 == 0x1babb1ed, "");
+  static_assert(a.Data2 == 0xfeed, "");
+  static_assert(__uuidof(Request).Data2 == 0xfeed, "");
+  static_assert(a.Data3 == 0xc01d, "");
+  static_assert(__uuidof(Request).Data3 == 0xc01d, "");
+  static_assert(a.Data4[0] == 0x1c, "");
+  static_assert(__uuidof(Request).Data4[0] == 0x1c, "");
+  static_assert(a.Data4[1] == 0xed, "");
+  static_assert(__uuidof(Request).Data4[1] == 0xed, "");
+  static_assert(a.Data4[2] == 0xde, "");
+  static_assert(__uuidof(Request).Data4[2] == 0xde, "");
+  static_assert(a.Data4[7] == 0xe5, "");
+  static_assert(__uuidof(Request).Data4[7] == 0xe5, "");
+  constexpr int k = __uuidof(Request).Data4[8]; // expected-error {{constant expression}} expected-note {{past-the-end}}
+}
Index: clang/test/Parser/MicrosoftExtensions.cpp
===
--- clang/test/Parser/MicrosoftExtensions.cpp
+++ clang/test/Parser/MicrosoftExtensions.cpp
@@ -137,9 +137,7 @@
 
 COM_CLASS_TEMPLATE_REF good_template_arg;
 
-COM_CLASS_TEMPLATE bad_template_arg; // expected-error {{non-type template argument of type 'const _GUID' is not a constant expression}}
-// expected-note@-1 {{read of object '__uuidof(struct_with_uuid)' whose value is not known}}
-// expected-note@-2 {{temporary created here}}
+COM_CLASS_TEMPLATE bad_template_arg; // expected-error {{non-type template argument of type 'const _GUID' cannot be converted to a value of type 'const GUID *'}}
 
 namespace PR16911 {
 struct __declspec(uuid("{12345678-1234-1234-1234-1234567890aB}")) uuid;
Index: clang/test/CodeGenCXX/microsoft-uuidof.cpp
===
--- clang/test/CodeGenCXX/microsoft-uuidof.cpp
+++ clang/test/CodeGenCXX/microsoft-uuidof.cpp
@@ -30,16 +30,22 @@
 struct __declspec(uuid("{12345678-1234-1234-1234-1234567890ac}")) Curly;
 #endif
 
+void side_effect();
+
 #ifdef DEFINE_GUID
 // Make sure we can properly generate code when the UUID has curly braces on it.
-GUID thing = __uuidof(Curly);
+GUID thing = (side_effect(), __uuidof(Curly));
 // CHECK-DEFINE-GUID: @thing = global %struct._GUID zeroinitializer, align 4
 // CHECK-DEFINE-WRONG-GUID: @thing = global %struct._GUID zeroinitializer, align 4
 
 // This gets initialized in a static initializer.
 // CHECK-DEFINE-GUID: @g = global %struct._GUID zeroinitializer, align 4
 // CHECK-DEFINE-WRONG-GUID: @g = global %struct._GUID zeroinitializer, align 4
-GUID g = __uuidof(S1);
+GUID g = (side_effect(), __uuidof(S1));
+
+// CHECK-DEFINE-GUID: @const_init = global %struct._GUID { i32 305419896, i16 4660, i16 4660, [8 x i8] c"\124\124Vx\90\AC" }
+// CHECK-DEFINE-WRONG-GUID: @const_init = global %struct._GUID { i32 305419896 }
+GUID const_init = __uuidof(Curly);
 #endif
 
 // First global use of __uuidof(S1) forces the creation of the global.
@@ -83,19 +89,19 @@
   // CHECK-DEFINE-WRONG-GUID: [[U1:%.+]] = bitcast %struct._GUID* %s1_1 to i8*
   // CHECK-DEFINE-GUID: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 [[U1]], i8* align 4 bitcast ({ i32, i16, i16, [8 x i8] }* @_GUID_12345678_1234_1234_1234_1234567890ab to i8*), i32 16, i1 false)
   // CHECK-DEFINE-WRONG-GUID: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 [[U1]], i8* align 4 bitcast ({ i32, i16, i16, [8 x i8] }* @_GUID_12345678_1234_1234_1234_1234567890ab to i8*), i32 4, i1 false)
-  

[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/lib/Sema/SemaCast.cpp:264
 
+  CheckNoderef(*this, E->getType(), TInfo->getType(), OpLoc);
+

Warning on this seems reasonable for `static_cast` and `dynamic_cast`, but 
should `reinterpret_cast` (or the `reinterpret_cast` interpretation of a 
C-style cast) warn? Presumably we want to leave open some type system holes for 
the case where the programmer really does know what they're doing.



Comment at: clang/lib/Sema/SemaInit.cpp:8182-8184
+// Do not check static casts here because they are checked earlier
+// in Sema::ActOnCXXNamedCast()
+if (!Kind.isStaticCast()) {

Are there any `static_cast` cases that don't get here? I'm also a little 
surprised that `static_cast` would get here but the `static_cast` 
interpretation of a C-style or functional cast would not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77836



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

[PATCH] D77923: OpenCL: Fix some missing predefined macros

[Jan Vesely via Phabricator via cfe-commits]

jvesely added a comment.

OPENCL_VERSION sounds like something that should be really set by the opencl 
driver rather than the compiler.
coarse-grained SVM can be done without FEATURE_FLAT_ADDRESS_SPACE, so those 
gpus can get over 1.2, while the compiler can be used in an implementation that 
doesn't go above 1.2 even for gfx700+


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

https://reviews.llvm.org/D77923



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

[PATCH] D77954: [CUDA][HIP] Fix overload resolution issue for device host functions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D77954#1976456 , @yaxunl wrote:

> In D77954#1976378 , @rjmccall wrote:
>
> > I'm not saying that we need to be bug-for-bug-compatible with `nvcc`, I'm 
> > just saying that we should be able to point to *something* to justify our 
> > behavior.  I take it that the CUDA spec has rules for some amount of 
> > host/device-based overloading?  What are they based on?
>
>
> I checked CUDA SDK documentation and did not find useful information about 
> overloading resolution based on host/device attributes. I guess the rule can 
> only be deduced from nvcc behavior.
>
> Based on https://reviews.llvm.org/D12453, https://reviews.llvm.org/D18416, 
> and 
> https://bcain-llvm.readthedocs.io/projects/llvm/en/latest/CompileCudaWithLLVM/#overloading-based-on-host-and-device-attributes,
>  cuda-clang has different overload resolution rules based host/device 
> attributes. This is intentional design decision.


Okay, thanks, that's all I needed.  We don't need to re-litigate it.

That spec says that there's a preference given to functions according to 
host/device-ness.  The question, then, is how that actually interacts with the 
normal overload resolution rules.  The "deletion" approach suggests that it's 
meant to be the most important thing in the comparison.  It seems to me that, 
given the wording of the specification, deletion is the wrong implementation 
approach, and that instead this check should just be performed in 
`isBetterOverloadCandidate` so that a candidate that better matches the 
host/device-ness of the caller is always considered a better candidate.


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

https://reviews.llvm.org/D77954



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

[PATCH] D77954: [CUDA][HIP] Fix overload resolution issue for device host functions

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D77954#1976378 , @rjmccall wrote:

> I'm not saying that we need to be bug-for-bug-compatible with `nvcc`, I'm 
> just saying that we should be able to point to *something* to justify our 
> behavior.  I take it that the CUDA spec has rules for some amount of 
> host/device-based overloading?  What are they based on?


I checked CUDA SDK documentation and did not find useful information about 
overloading resolution based on host/device attributes. I guess the rule can 
only be deduced from nvcc behavior.

Based on https://reviews.llvm.org/D12453, https://reviews.llvm.org/D18416, and 
https://bcain-llvm.readthedocs.io/projects/llvm/en/latest/CompileCudaWithLLVM/#overloading-based-on-host-and-device-attributes,
 cuda-clang has different overload resolution rules based host/device 
attributes. This is intentional design decision.


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

https://reviews.llvm.org/D77954



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

[PATCH] D77390: Fix __builtin_amdgcn_workgroup_size_x/y/z return type

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGenOpenCL/builtins-amdgcn.cl:541
switch (d) {
-   case 0: *out = __builtin_amdgcn_workgroup_size_x(); break;
+   case 0: *out = __builtin_amdgcn_workgroup_size_x() + 1; break;
case 1: *out = __builtin_amdgcn_workgroup_size_y(); break;

JonChesterfield wrote:
> This looks unrelated to the return type change
Without that change, the builtin return type is wrong and clang does not insert 
conversion for this "add" operation and causes assertion. Therefore this small 
change makes sure that error will not happen again.


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

https://reviews.llvm.org/D77390



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

[PATCH] D77062: [analyzer] Added check for unacceptable equality operation between Loc and NonLoc types

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

In D77062#1959286 , @ASDenysPetrov 
wrote:

> @Szelethus , @NoQ please, look at my solution. I'm not sure if it is correct, 
> but it actually passes all the tests and throw off the issue.


Sorry for the late answer, I've been kinda busy with other things! And thank 
you for attending to these bugs! I admit, this isn't really within the realms 
of my expertise, but nevertheless, here are my two cents.

I think what what be great to see here (and this seems to be the thing @NoQ is 
fishing for) is not to change an if branch and avoid running into the crash, 
but rather find out why `assumeZero` was ever called with a `nonloc` value, 
which shouldn't really happen. We're treating the symptom, not curing the 
illness, if you will. The `SVal` (if its `DefinedSVal`) is supposed to be 
always `MemRegionVal` here, is that right? Maybe if we tried to add an assert 
here, that could help nail where the actual issue is coming from. It's probably 
in `evalStrcpyCommon`, judging from the bug report you linked in your summary.




Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:272-274
   Optional val = V.getAs();
-  if (!val)
-return std::pair(state, state);
+  if (val && !V.getAs()) {
+ // return pair shall be {null, non-null} so reorder states


>>Why are we getting a compound value here? 
> We are getting **nonloc::SymbolVal** here, not a compound value. SA thinks 
> that double-dereference of the first agrument applies to unsigned char* *, 
> instead of char* * * (look at the test sample).

So, in other words the condition seems to be this:
```lang=cpp
if (V.getAs(&& !V.getAs())
```

But `nonloc::SymbolVal` is a subclass of `DefinedSVal` (and is not a 
`nonloc::LazyCompoundVal`), so we're definitely running into the assume call, 
isn't that right?
https://clang.llvm.org/doxygen/classclang_1_1ento_1_1SVal.html

What really seems to be the problem solver here is that you changed the 
assumption from `x == 0` (creating a state where this expression is false, and 
one where it isn't) into `x` (whether `x` is true or not).


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

https://reviews.llvm.org/D77062



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

[PATCH] D77621: ADT: SmallVector size & capacity use word-size integers when elements are small.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: llvm/include/llvm/ADT/SmallVector.h:53
+  // The maximum size depends on size_type used.
+  size_t SizeMax() { return size_type(-1ULL); }
 

dblaikie wrote:
> I'd probably use numeric_limits here & make this static constexpr
Making it a constexpr /variable/ gets a bit more complicated - do you happen to 
know off-hand whether this requires a separate/out-of-line definition if it's 
ODR used (ah, seems it does - in C++14 which LLVM Uses, in 17 ("A constexpr 
specifier used in a function or static member variable (since C++17) 
declaration implies inline.") it would be OK)

I think it's best not to leave a trap that might catch someone up in the future 
if SizeMax ends up getting ODR used (eg: in a call to std::less, rather than an 
explicit op<, etc) & then the lack of definition would lead to linking failure, 
etc. So best to leave it as a static constexpr function rather than static 
constexpr variable.



Comment at: llvm/lib/Support/SmallVector.cpp:40-47
+// Check that SmallVector with 1 byte elements takes extra space due to using
+// uintptr_r instead of uint32_t for size and capacity.
+static_assert(sizeof(SmallVector) > sizeof(SmallVector) 
||
+  sizeof(uintptr_t) == sizeof(uint32_t),
+  "1 byte elements should cause larger size and capacity type");
+static_assert(sizeof(SmallVector) ==
+  sizeof(SmallVector),

I think it's probably best to assert the size more like the above (but using 
zero-sized inline buffer), rather than using relative sizes - seems like it'd 
be more readable to me at least. Maybe @dexonsmith has a perspective here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621



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

[PATCH] D77954: [CUDA][HIP] Fix overload resolution issue for device host functions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I'm not saying that we need to be bug-for-bug-compatible with `nvcc`, I'm just 
saying that we should be able to point to *something* to justify our behavior.  
I take it that the CUDA spec has rules for some amount of host/device-based 
overloading?  What are they based on?


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

https://reviews.llvm.org/D77954



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

[PATCH] D77952: [TLI] Reduce copies for TLI and TLA

[Wenlei He via Phabricator via cfe-commits]

wenlei updated this revision to Diff 256806.
wenlei added a comment.

address feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77952

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Analysis/TargetLibraryInfo.h
  llvm/lib/Analysis/TargetLibraryInfo.cpp

Index: llvm/lib/Analysis/TargetLibraryInfo.cpp
===
--- llvm/lib/Analysis/TargetLibraryInfo.cpp
+++ llvm/lib/Analysis/TargetLibraryInfo.cpp
@@ -583,8 +583,8 @@
   ShouldSignExtI32Param(TLI.ShouldSignExtI32Param) {
   std::move(std::begin(TLI.AvailableArray), std::end(TLI.AvailableArray),
 AvailableArray);
-  for (unsigned i = 0; i < NumVecLibs; i++)
-VecLibDescs[i] = TLI.VecLibDescs[i];
+  std::move(std::begin(TLI.VecLibDescs), std::end(TLI.VecLibDescs),
+VecLibDescs);
 }
 
 TargetLibraryInfoImpl &TargetLibraryInfoImpl::operator=(const TargetLibraryInfoImpl &TLI) {
@@ -593,6 +593,8 @@
   ShouldExtI32Return = TLI.ShouldExtI32Return;
   ShouldSignExtI32Param = TLI.ShouldSignExtI32Param;
   memcpy(AvailableArray, TLI.AvailableArray, sizeof(AvailableArray));
+  for (unsigned i = 0; i < NumVecLibs; i++)
+VecLibDescs[i] = TLI.VecLibDescs[i];
   return *this;
 }
 
@@ -603,6 +605,8 @@
   ShouldSignExtI32Param = TLI.ShouldSignExtI32Param;
   std::move(std::begin(TLI.AvailableArray), std::end(TLI.AvailableArray),
 AvailableArray);
+  std::move(std::begin(TLI.VecLibDescs), std::end(TLI.VecLibDescs),
+VecLibDescs);
   return *this;
 }
 
@@ -1677,6 +1681,12 @@
   initializeTargetLibraryInfoWrapperPassPass(*PassRegistry::getPassRegistry());
 }
 
+TargetLibraryInfoWrapperPass::TargetLibraryInfoWrapperPass(
+TargetLibraryInfoImpl &&TLIImpl)
+: ImmutablePass(ID), TLA(std::move(TLIImpl)) {
+  initializeTargetLibraryInfoWrapperPassPass(*PassRegistry::getPassRegistry());
+}
+
 AnalysisKey TargetLibraryAnalysis::Key;
 
 // Register the basic pass.
Index: llvm/include/llvm/Analysis/TargetLibraryInfo.h
===
--- llvm/include/llvm/Analysis/TargetLibraryInfo.h
+++ llvm/include/llvm/Analysis/TargetLibraryInfo.h
@@ -452,7 +452,11 @@
   /// Construct a library analysis with baseline Module-level info.
   ///
   /// This will be supplemented with Function-specific info in the Result.
-  TargetLibraryAnalysis(TargetLibraryInfoImpl BaselineInfoImpl)
+  TargetLibraryAnalysis(const Triple &T)
+  : BaselineInfoImpl(TargetLibraryInfoImpl(T)) {}
+  TargetLibraryAnalysis(const TargetLibraryInfoImpl &BaselineInfoImpl)
+  : BaselineInfoImpl(BaselineInfoImpl) {}
+  TargetLibraryAnalysis(TargetLibraryInfoImpl &&BaselineInfoImpl)
   : BaselineInfoImpl(std::move(BaselineInfoImpl)) {}
 
   TargetLibraryInfo run(const Function &F, FunctionAnalysisManager &);
@@ -475,6 +479,7 @@
   TargetLibraryInfoWrapperPass();
   explicit TargetLibraryInfoWrapperPass(const Triple &T);
   explicit TargetLibraryInfoWrapperPass(const TargetLibraryInfoImpl &TLI);
+  explicit TargetLibraryInfoWrapperPass(TargetLibraryInfoImpl &&TLI);
 
   TargetLibraryInfo &getTLI(const Function &F) {
 FunctionAnalysisManager DummyFAM;
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -352,10 +352,6 @@
   PM.add(createStackSafetyGlobalInfoWrapperPass(/*SetMetadata=*/true));
 }
 
-static TargetLibraryInfoImpl *createTLII(llvm::Triple &TargetTriple) {
-  return new TargetLibraryInfoImpl(TargetTriple);
-}
-
 static void addSymbolRewriterPass(const CodeGenOptions &Opts,
   legacy::PassManager *MPM) {
   llvm::SymbolRewriter::RewriteDescriptorList DL;
@@ -541,12 +537,12 @@
   if (CodeGenOpts.DisableLLVMPasses)
 return;
 
-  // Figure out TargetLibraryInfo.  This needs to be added to MPM and FPM
+  // Figure out TargetLibraryInfo. This needs to be added to MPM and FPM
   // manually (and not via PMBuilder), since some passes (eg. InstrProfiling)
   // are inserted before PMBuilder ones - they'd get the default-constructed
   // TLI with an unknown target otherwise.
   Triple TargetTriple(TheModule->getTargetTriple());
-  std::unique_ptr TLII(createTLII(TargetTriple));
+  TargetLibraryInfoImpl TLII(TargetTriple);
 
   // If we reached here with a non-empty index file name, then the index file
   // was empty and we are not performing ThinLTO backend compilation (used in
@@ -591,7 +587,7 @@
   PMBuilder.PrepareForLTO = CodeGenOpts.PrepareForLTO;
   PMBuilder.RerollLoops = CodeGenOpts.RerollLoops;
 
-  MPM.add(new TargetLibraryInfoWrapperPass(*TLII));
+  MPM.add(new TargetLibraryInfoWrapperPass(TLII));
 
   if (TM)
 TM->adjustPassManager(PMBuilder);
@@ -692,7 +688,7 @@
   }
 
   // Set up the per-function pass manager.
-  FPM.add(new TargetLibraryInfoWrapperPas

[PATCH] D77952: [TLI] Reduce copies for TLI and TLA

[Wenlei He via Phabricator via cfe-commits]

wenlei marked an inline comment as done.
wenlei added a comment.

In D77952#1976336 , @tejohnson wrote:

> Some parts of this are dependent on the patch that got reverted, but I have 
> some other questions below about the changes in BackendUtil.cpp.


Thanks for quick review.. I will remove the changes dependent on the reverted 
change before commit.




Comment at: clang/lib/CodeGen/BackendUtil.cpp:689
   // Set up the per-function pass manager.
-  FPM.add(new TargetLibraryInfoWrapperPass(*TLII));
+  FPM.add(new TargetLibraryInfoWrapperPass(TargetTriple));
   if (CodeGenOpts.VerifyModule)

tejohnson wrote:
> These changes mean we now construct a new TLII multiple times (e.g. both when 
> we add the TargetLibraryInfoWrapperPass to the MPM earlier and to the FPM 
> here, rather that just copying. Is this actually faster? It seems like it 
> would be slower overall.
Oops, this one isn't intentional... changed it back. Though for other instances 
where TLII isn't reused, similar change turns extra copy into move.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77952



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

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D77632#1976240 , @wenlei wrote:

> In D77632#1974409 , @tejohnson wrote:
>
> > In D77632#1974363 , @wenlei wrote:
> >
> > > In D77632#1974015 , @nikic wrote:
> > >
> > > > This change causes a ~0.5% compile-time regressions: 
> > > > http://llvm-compile-time-tracker.com/compare.php?from=5b18b6e9a84d985c0a907009fb71de7c1943bc88&to=60c642e74be6af86906d9f3d982728be7bd4329f&stat=instructions
> > > >  This is quite a lot as these things go, so it would be great if you 
> > > > could double check if there's any optimization potential here. In 
> > > > particular I'm wondering why this affects normal builds so much, even 
> > > > though they (presumably?) don't use any veclib at all.
> > >
> > >
> > > Thanks for the heads-up. This is surprising but there is a change even 
> > > when veclib is not used - in order to allow each function to use 
> > > different veclib without duplicating the work of populating vector 
> > > function list for each function, we now always pre-populate vector 
> > > function list for three supported vector libraries for each module. 
> > > However 0.5% compile-time for that work given it's per-module is not 
> > > expected. I suspect we may be passing/copying TLII around more than we 
> > > anticipated (now we always have more stuff to copy). I will take a look. 
> > > We could also turn this into a lazy initialization - only populate the 
> > > needed list for module level TLII when it's first queried by a function 
> > > level TLI.
> >
> >
> > Hmm, yeah that is surprising, because the TLII should be built once per 
> > module per TLI analysis, which is never invalidated. We've gone from 
> > populating one set of vec libs to 3, I wouldn't have thought that was 
> > particularly expensive, so it would be good to see what is going on here 
> > and confirm we are only building this once as expected.
> >
> > Looking at the compile time data at that link, interestingly the 
> > "instructions" metric increased, but not wall time or cycles or task clock 
> > - they were all neutral.
>
>
> Turns out there're a few places where we call copy ctor for TLI unnecessarily.


I assume you mean the TargetLibraryInfoImpl (TLII) here, not the 
TargetLibraryInfo (TLI), right? The latter should be cheap to copy. Are these 
the changes in BackendUtil.cpp in D77952 ? I 
had a question about that on that patch as I think we will be calling the 
initializer more. Mostly we should only be copying the TargetLibraryInfo during 
optimization though, and not the TLII impl object.

> Made some changes in D77952  to use move 
> when possible. In addition, I should have used move for `TLI.VecLibDescs` in 
> move ctor of `TargetLibraryInfoImpl` too.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77632



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

[PATCH] D77621: ADT: SmallVector size & capacity use word-size integers when elements are small.

[Andrew via Phabricator via cfe-commits]

browneee updated this revision to Diff 256803.
browneee marked 3 inline comments as done.
browneee added a comment.

Address comments from dblaikie.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77621

Files:
  llvm/include/llvm/ADT/SmallVector.h
  llvm/lib/Support/SmallVector.cpp

Index: llvm/lib/Support/SmallVector.cpp
===
--- llvm/lib/Support/SmallVector.cpp
+++ llvm/lib/Support/SmallVector.cpp
@@ -37,29 +37,11 @@
   sizeof(unsigned) * 2 + sizeof(void *) * 2,
   "wasted space in SmallVector size 1");
 
-/// grow_pod - This is an implementation of the grow() method which only works
-/// on POD-like datatypes and is out of line to reduce code duplication.
-void SmallVectorBase::grow_pod(void *FirstEl, size_t MinCapacity,
-   size_t TSize) {
-  // Ensure we can fit the new capacity in 32 bits.
-  if (MinCapacity > UINT32_MAX)
-report_bad_alloc_error("SmallVector capacity overflow during allocation");
-
-  size_t NewCapacity = 2 * capacity() + 1; // Always grow.
-  NewCapacity =
-  std::min(std::max(NewCapacity, MinCapacity), size_t(UINT32_MAX));
-
-  void *NewElts;
-  if (BeginX == FirstEl) {
-NewElts = safe_malloc(NewCapacity * TSize);
-
-// Copy the elements over.  No need to run dtors on PODs.
-memcpy(NewElts, this->BeginX, size() * TSize);
-  } else {
-// If this wasn't grown from the inline copy, grow the allocated space.
-NewElts = safe_realloc(this->BeginX, NewCapacity * TSize);
-  }
-
-  this->BeginX = NewElts;
-  this->Capacity = NewCapacity;
-}
+// Check that SmallVector with 1 byte elements takes extra space due to using
+// uintptr_r instead of uint32_t for size and capacity.
+static_assert(sizeof(SmallVector) > sizeof(SmallVector) ||
+  sizeof(uintptr_t) == sizeof(uint32_t),
+  "1 byte elements should cause larger size and capacity type");
+static_assert(sizeof(SmallVector) ==
+  sizeof(SmallVector),
+  "larger elements should keep the same size and capacity type");
Index: llvm/include/llvm/ADT/SmallVector.h
===
--- llvm/include/llvm/ADT/SmallVector.h
+++ llvm/include/llvm/ADT/SmallVector.h
@@ -16,10 +16,10 @@
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/AlignOf.h"
 #include "llvm/Support/Compiler.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MemAlloc.h"
 #include "llvm/Support/type_traits.h"
-#include "llvm/Support/ErrorHandling.h"
 #include 
 #include 
 #include 
@@ -27,6 +27,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -34,11 +35,21 @@
 
 namespace llvm {
 
-/// This is all the non-templated stuff common to all SmallVectors.
-class SmallVectorBase {
+/// This is all the stuff common to all SmallVectors.
+///
+/// The template parameter specifies the type which should be used to hold the
+/// Size and Capacity of the SmallVector, so it can be adjusted.
+/// Using 32 bit size is desirable to shink the size of the SmallVector.
+/// Using 64 bit size is desirable for cases like SmallVector, where a
+/// 32 bit size would limit the vector to ~4GB. SmallVectors are used for
+/// buffering bitcode output - which can exceed 4GB.
+template  class SmallVectorBase {
 protected:
   void *BeginX;
-  unsigned Size = 0, Capacity;
+  Size_T Size = 0, Capacity;
+
+  // The maximum size depends on size_type used.
+  static constexpr size_t SizeMax = std::numeric_limits::max();
 
   SmallVectorBase() = delete;
   SmallVectorBase(void *FirstEl, size_t TotalCapacity)
@@ -46,7 +57,39 @@
 
   /// This is an implementation of the grow() method which only works
   /// on POD-like data types and is out of line to reduce code duplication.
-  void grow_pod(void *FirstEl, size_t MinCapacity, size_t TSize);
+  /// This function will report a fatal error if it cannot increase capacity.
+  void grow_pod(void *FirstEl, size_t MinCapacity, size_t TSize) {
+// Ensure we can fit the new capacity.
+// This is only going to be applicable if the when the capacity is 32 bit.
+if (MinCapacity > SizeMax)
+  report_bad_alloc_error("SmallVector capacity overflow during allocation");
+
+// Ensure we can meet the guarantee of space for at least one more element.
+// The above check alone will not catch the case where grow is called with a
+// default MinCapacity of 0, but the current capacity cannot be increased.
+// This is only going to be applicable if the when the capacity is 32 bit.
+if (capacity() == SizeMax)
+  report_bad_alloc_error("SmallVector capacity unable to grow");
+
+// In theory 2*capacity can overflow if the capacity is 64 bit, but the
+// original capacity would never be large enough for this to be a proble

[PATCH] D77952: [TLI] Reduce copies for TLI and TLA

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

Some parts of this are dependent on the patch that got reverted, but I have 
some other questions below about the changes in BackendUtil.cpp.




Comment at: clang/lib/CodeGen/BackendUtil.cpp:689
   // Set up the per-function pass manager.
-  FPM.add(new TargetLibraryInfoWrapperPass(*TLII));
+  FPM.add(new TargetLibraryInfoWrapperPass(TargetTriple));
   if (CodeGenOpts.VerifyModule)

These changes mean we now construct a new TLII multiple times (e.g. both when 
we add the TargetLibraryInfoWrapperPass to the MPM earlier and to the FPM here, 
rather that just copying. Is this actually faster? It seems like it would be 
slower overall.



Comment at: llvm/lib/Analysis/TargetLibraryInfo.cpp:586
 AvailableArray);
-  for (unsigned i = 0; i < NumVecLibs; i++)
-VecLibDescs[i] = TLI.VecLibDescs[i];
+  std::move(std::begin(TLI.VecLibDescs), std::end(TLI.VecLibDescs),
+VecLibDescs);

Woops, sorry, missed this one in the original review.



Comment at: llvm/lib/Analysis/TargetLibraryInfo.cpp:596
   memcpy(AvailableArray, TLI.AvailableArray, sizeof(AvailableArray));
   return *this;
 }

This is missing copying of the VecLibDescs (looks like I missed this as well 
originally).



Comment at: llvm/lib/Analysis/TargetLibraryInfo.cpp:606
 AvailableArray);
   return *this;
 }

Ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77952



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

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/CodeGen/CGException.cpp:1651
+  llvm::Value* Load = CGF.Builder.CreateLoad(Addr, "cleanup.dest");
+  IsForEH = CGF.Builder.CreateTrunc(Load, CGM.Int8Ty);
+}

Is just truncating the value really correct?  What's the possible range of 
values stored in getNormalCleanupDestSlot()?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77936



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

[PATCH] D77390: Fix __builtin_amdgcn_workgroup_size_x/y/z return type

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: clang/test/CodeGenOpenCL/builtins-amdgcn.cl:541
switch (d) {
-   case 0: *out = __builtin_amdgcn_workgroup_size_x(); break;
+   case 0: *out = __builtin_amdgcn_workgroup_size_x() + 1; break;
case 1: *out = __builtin_amdgcn_workgroup_size_y(); break;

This looks unrelated to the return type change


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

https://reviews.llvm.org/D77390



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

[PATCH] D77954: [CUDA][HIP] Fix overload resolution issue for device host functions

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D77954#1976316 , @yaxunl wrote:

> In D77954#1976294 , @rjmccall wrote:
>
> > If `nvcc` ignores host/device-ness when selecting overloads, that's 
> > probably the specified behavior, right?  I agree that it would be better to 
> > not ignore it, but Clang shouldn't just make up better rules for languages 
> > with external specifications.
>
>
> cuda-clang does not always follow nvcc's behavior. For example, cuda-clang 
> only allows incomplete array type for extern shared variables, whereas nvcc 
> allows other types. If cuda-clang is supposed to follow nvcc's behavior in 
> every aspects, we should approve https://reviews.llvm.org/D73979 , but it is 
> not the case.
>
> Therefore, I think we should discuss whether this is really a bug, and 
> whether the fix can cause any unwanted side effect.


BTW cuda-clang is already quite different than nvcc regarding host/device-based 
overloading resolution. For example, the following code is valid in cuda-clang 
before my change but invalid in nvcc https://cuda.godbolt.org/z/qwpKZe . So if 
we want to follow nvcc's resolution rule we need a total revamp of device/host 
related resolution in cuda-clang.

  __host__ int foo(int x) {
   return 1;
  }
  
  template
  __device__ int foo(T x) {
  return 2;
  }
  
  __device__ int bar() {
  return foo(1);
  }
  
  __global__ void test(int *a) {
  *a = bar();
  }


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

https://reviews.llvm.org/D77954



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

[PATCH] D77954: [CUDA][HIP] Fix overload resolution issue for device host functions

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D77954#1976294 , @rjmccall wrote:

> If `nvcc` ignores host/device-ness when selecting overloads, that's probably 
> the specified behavior, right?  I agree that it would be better to not ignore 
> it, but Clang shouldn't just make up better rules for languages with external 
> specifications.


cuda-clang does not always follow nvcc's behavior. For example, cuda-clang only 
allows incomplete array type for extern shared variables, whereas nvcc allows 
other types. If cuda-clang is supposed to follow nvcc's behavior in every 
aspects, we should approve https://reviews.llvm.org/D73979 , but it is not the 
case.

Therefore, I think we should discuss whether this is really a bug, and whether 
the fix can cause any unwanted side effect.


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

https://reviews.llvm.org/D77954



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

[PATCH] D77952: [TLI] Reduce copies for TLI and TLA

[Hongtao Yu via Phabricator via cfe-commits]

hoyFB accepted this revision.
hoyFB added inline comments.
This revision is now accepted and ready to land.



Comment at: llvm/include/llvm/Analysis/TargetLibraryInfo.h:456
+  TargetLibraryAnalysis(const Triple &T)
+  : BaselineInfoImpl(TargetLibraryInfoImpl(T)) {}
+  TargetLibraryAnalysis(const TargetLibraryInfoImpl &BaselineInfoImpl)

hoyFB wrote:
> Can `T` just be used here? `TargetLibraryInfoImpl` comes with a such 
> constructor: `TargetLibraryInfoImpl(const Triple &T)`
Unlikely not. That is an explicit constructor and `BaselineInfoImpl` is an 
`Optional` object.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77952



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

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

In D77632#1976231 , @wenlei wrote:

> And agree with @tejohnson, if the openness is a feature, it should be covered 
> in tests, otherwise it can feel somewhat like a loophole and prone to breakage


The thing is that LLVM does not have much C++ unittests in general, so most of 
the "features" like this one that LLVM offers as a library are just an artifact 
of being "designed as a library" and being mindful about the layering.
From this point of view this patch is changing the design of a component that 
was modular/pluggable into a closed system. I'm perfectly fine with trying to 
add a unit-test, I just don't know yet where it would fit in the LLVM testing 
though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77632



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

[PATCH] D77542: [PowerPC] Treat 'Z' inline asm constraint as a true memory constraint

[Amy Kwan via Phabricator via cfe-commits]

amyk added inline comments.



Comment at: clang/lib/Basic/Targets/PPC.h:277
   break;
 case 'Q': // Memory operand that is an offset from a register (it is
   // usually better to use `m' or `es' in asm statements)

Just curious, but does this case still require `Info.setAllowsMemory();` as 
well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77542



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

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Needs a testcase in clang/test/CodeGen to verify the generated IR.

I haven't looked at this code in a while, but this looks reasonable.




Comment at: clang/lib/CodeGen/EHScopeStack.h:164
+F_IsEHCleanupKind = 0x4,
+F_HasSehAbnormalExits = 0x8
   };

Please add a trailing comma here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77936



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

[PATCH] D77955: [clangd] Add target_info test

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
Herald added subscribers: cfe-commits, phosek, usaxena95, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77955

Files:
  clang-tools-extra/clangd/test/target_info.test


Index: clang-tools-extra/clangd/test/target_info.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/target_info.test
@@ -0,0 +1,35 @@
+# REQUIRES: arm-registered-target
+# Mock 'compile_commands.json' to contain a driver name targeting fuchsia OS.
+# Afterwards check that correct target is passed into clang.
+
+# RUN: rm -rf %t.dir && mkdir -p %t.dir
+
+# RUN: echo '[{"directory": "%/t.dir", "command": "%/t.dir/armv7-fuchsia-clang 
-x c++ the-file.cpp -v", "file": "the-file.cpp"}]' > 
%t.dir/compile_commands.json
+
+# RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
+# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
+# (with the extra slash in the front), so we add it here.
+# RUN: sed -E -e "s|file://([A-Z]):/|file:///\1:/|g" %t.test.1 > %t.test
+
+# RUN: clangd -lit-test < %t.test 2>&1 | FileCheck -strict-whitespace %t.test
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}
+---
+{
+  "jsonrpc":"2.0",
+  "method":"textDocument/didOpen",
+  "params": {
+"textDocument": {
+  "uri": "file://INPUT_DIR/the-file.cpp",
+  "languageId":"cpp",
+  "version":1,
+  "text":""
+}
+  }
+}
+# Make sure we have target passed into cc1 driver, which is printed due to -v 
in
+# the compile_commands.json
+# CHECK: Target: armv7-unknown-fuchsia
+---
+{"jsonrpc":"2.0","id":1,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}


Index: clang-tools-extra/clangd/test/target_info.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/target_info.test
@@ -0,0 +1,35 @@
+# REQUIRES: arm-registered-target
+# Mock 'compile_commands.json' to contain a driver name targeting fuchsia OS.
+# Afterwards check that correct target is passed into clang.
+
+# RUN: rm -rf %t.dir && mkdir -p %t.dir
+
+# RUN: echo '[{"directory": "%/t.dir", "command": "%/t.dir/armv7-fuchsia-clang -x c++ the-file.cpp -v", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+
+# RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
+# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
+# (with the extra slash in the front), so we add it here.
+# RUN: sed -E -e "s|file://([A-Z]):/|file:///\1:/|g" %t.test.1 > %t.test
+
+# RUN: clangd -lit-test < %t.test 2>&1 | FileCheck -strict-whitespace %t.test
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}
+---
+{
+  "jsonrpc":"2.0",
+  "method":"textDocument/didOpen",
+  "params": {
+"textDocument": {
+  "uri": "file://INPUT_DIR/the-file.cpp",
+  "languageId":"cpp",
+  "version":1,
+  "text":""
+}
+  }
+}
+# Make sure we have target passed into cc1 driver, which is printed due to -v in
+# the compile_commands.json
+# CHECK: Target: armv7-unknown-fuchsia
+---
+{"jsonrpc":"2.0","id":1,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77952: [TLI] Reduce copies for TLI and TLA

[Hongtao Yu via Phabricator via cfe-commits]

hoyFB added a comment.

Thanks for the nice cleanup!




Comment at: llvm/include/llvm/Analysis/TargetLibraryInfo.h:456
+  TargetLibraryAnalysis(const Triple &T)
+  : BaselineInfoImpl(TargetLibraryInfoImpl(T)) {}
+  TargetLibraryAnalysis(const TargetLibraryInfoImpl &BaselineInfoImpl)

Can `T` just be used here? `TargetLibraryInfoImpl` comes with a such 
constructor: `TargetLibraryInfoImpl(const Triple &T)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77952



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

[PATCH] D77954: [CUDA][HIP] Fix overload resolution issue for device host functions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

If `nvcc` ignores host/device-ness when selecting overloads, that's probably 
the specified behavior, right?  I agree that it would be better to not ignore 
it, but Clang shouldn't just make up better rules for languages with external 
specifications.


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

https://reviews.llvm.org/D77954



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

[PATCH] D77944: [clangd][test] Provide registered targets to lit tests

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
kadircet marked an inline comment as done.
Closed by commit rGd34a91a10f7a: [clangd][test] Provide registered targets to 
lit tests (authored by kadircet).

Changed prior to commit:
  https://reviews.llvm.org/D77944?vs=256774&id=256797#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77944

Files:
  clang-tools-extra/clangd/test/lit.cfg.py


Index: clang-tools-extra/clangd/test/lit.cfg.py
===
--- clang-tools-extra/clangd/test/lit.cfg.py
+++ clang-tools-extra/clangd/test/lit.cfg.py
@@ -10,10 +10,19 @@
 config.test_source_root = config.clangd_source_dir + "/test"
 config.test_exec_root = config.clangd_binary_dir + "/test"
 
+
+# Used to enable tests based on the required targets. Can be queried with e.g.
+#REQUIRES: x86-registered-target
+def calculate_arch_features(arch_string):
+  return [arch.lower() + '-registered-target' for arch in arch_string.split()]
+
+
+lit.llvm.llvm_config.feature_config([('--targets-built',
+  calculate_arch_features)])
+
 # Clangd-specific lit environment.
 config.substitutions.append(('%clangd-benchmark-dir',
  config.clangd_binary_dir + "/benchmarks"))
 
 if config.clangd_build_xpc:
   config.available_features.add('clangd-xpc-support')
-


Index: clang-tools-extra/clangd/test/lit.cfg.py
===
--- clang-tools-extra/clangd/test/lit.cfg.py
+++ clang-tools-extra/clangd/test/lit.cfg.py
@@ -10,10 +10,19 @@
 config.test_source_root = config.clangd_source_dir + "/test"
 config.test_exec_root = config.clangd_binary_dir + "/test"
 
+
+# Used to enable tests based on the required targets. Can be queried with e.g.
+#REQUIRES: x86-registered-target
+def calculate_arch_features(arch_string):
+  return [arch.lower() + '-registered-target' for arch in arch_string.split()]
+
+
+lit.llvm.llvm_config.feature_config([('--targets-built',
+  calculate_arch_features)])
+
 # Clangd-specific lit environment.
 config.substitutions.append(('%clangd-benchmark-dir',
  config.clangd_binary_dir + "/benchmarks"))
 
 if config.clangd_build_xpc:
   config.available_features.add('clangd-xpc-support')
-
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77954: [CUDA][HIP] Fix overload resolution issue for device host functions

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: tra, rjmccall.
yaxunl edited the summary of this revision.

Currently clang fails to compile the following CUDA program in device 
compilation:

  __host__ int foo(int x) {
   return 1;
  }
  
  template
  __device__ __host__ int foo(T x) {
  return 2;
  }
  
  __device__ __host__ int bar() {
  return foo(1);
  }
  
  __global__ void test(int *a) {
  *a = bar();
  }

This is due to foo is resolved to the `__host__ foo` instead of `__device__ 
__host__ foo`.
This seems to be a bug since `__device__ __host__ foo` is a viable callee for 
foo whereas
clang is unable to choose it.

nvcc has similar issue

https://cuda.godbolt.org/z/bGijLc

Although it only emits a warning and does not fail to compile. It emits a trap 
in the code
so that it will fail at run time.

This patch fixes that.


https://reviews.llvm.org/D77954

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCUDA/function-overload.cu


Index: clang/test/SemaCUDA/function-overload.cu
===
--- clang/test/SemaCUDA/function-overload.cu
+++ clang/test/SemaCUDA/function-overload.cu
@@ -331,9 +331,6 @@
 // If we have a mix of HD and H-only or D-only candidates in the overload set,
 // normal C++ overload resolution rules apply first.
 template  TemplateReturnTy template_vs_hd_function(T arg)
-#ifdef __CUDA_ARCH__
-//expected-note@-2 {{declared here}}
-#endif
 {
   return TemplateReturnTy();
 }
@@ -342,11 +339,13 @@
 }
 
 __host__ __device__ void test_host_device_calls_hd_template() {
-  HostDeviceReturnTy ret1 = template_vs_hd_function(1.0f);
-  TemplateReturnTy ret2 = template_vs_hd_function(1);
 #ifdef __CUDA_ARCH__
-  // expected-error@-2 {{reference to __host__ function 
'template_vs_hd_function' in __host__ __device__ function}}
+  typedef HostDeviceReturnTy ExpectedReturnTy;
+#else
+  typedef TemplateReturnTy ExpectedReturnTy;
 #endif
+  HostDeviceReturnTy ret1 = template_vs_hd_function(1.0f);
+  ExpectedReturnTy ret2 = template_vs_hd_function(1);
 }
 
 __host__ void test_host_calls_hd_template() {
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -9821,8 +9821,10 @@
 llvm::any_of(Candidates, [&](OverloadCandidate *Cand) {
   // Check viable function only.
   return Cand->Viable && Cand->Function &&
- S.IdentifyCUDAPreference(Caller, Cand->Function) ==
- Sema::CFP_SameSide;
+ (S.IdentifyCUDAPreference(Caller, Cand->Function) ==
+  Sema::CFP_SameSide ||
+  S.IdentifyCUDAPreference(Caller, Cand->Function) ==
+  Sema::CFP_HostDevice);
 });
 if (ContainsSameSideCandidate) {
   auto IsWrongSideCandidate = [&](OverloadCandidate *Cand) {


Index: clang/test/SemaCUDA/function-overload.cu
===
--- clang/test/SemaCUDA/function-overload.cu
+++ clang/test/SemaCUDA/function-overload.cu
@@ -331,9 +331,6 @@
 // If we have a mix of HD and H-only or D-only candidates in the overload set,
 // normal C++ overload resolution rules apply first.
 template  TemplateReturnTy template_vs_hd_function(T arg)
-#ifdef __CUDA_ARCH__
-//expected-note@-2 {{declared here}}
-#endif
 {
   return TemplateReturnTy();
 }
@@ -342,11 +339,13 @@
 }
 
 __host__ __device__ void test_host_device_calls_hd_template() {
-  HostDeviceReturnTy ret1 = template_vs_hd_function(1.0f);
-  TemplateReturnTy ret2 = template_vs_hd_function(1);
 #ifdef __CUDA_ARCH__
-  // expected-error@-2 {{reference to __host__ function 'template_vs_hd_function' in __host__ __device__ function}}
+  typedef HostDeviceReturnTy ExpectedReturnTy;
+#else
+  typedef TemplateReturnTy ExpectedReturnTy;
 #endif
+  HostDeviceReturnTy ret1 = template_vs_hd_function(1.0f);
+  ExpectedReturnTy ret2 = template_vs_hd_function(1);
 }
 
 __host__ void test_host_calls_hd_template() {
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -9821,8 +9821,10 @@
 llvm::any_of(Candidates, [&](OverloadCandidate *Cand) {
   // Check viable function only.
   return Cand->Viable && Cand->Function &&
- S.IdentifyCUDAPreference(Caller, Cand->Function) ==
- Sema::CFP_SameSide;
+ (S.IdentifyCUDAPreference(Caller, Cand->Function) ==
+  Sema::CFP_SameSide ||
+  S.IdentifyCUDAPreference(Caller, Cand->Function) ==
+  Sema::CFP_HostDevice);
 });
 if (ContainsSameSideCandidate) {
   auto IsWrongSideCandidate = [&](OverloadCandidate *Cand) {
___
cfe-commits mailing list
cfe

[PATCH] D77942: [Preamble] Invalidate preamble when missing headers become present.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

It is unfortunate that we are testing `PrecompiledPreamble` through clangd 
rather than its own unittest :/




Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:101
+
+  // We always receive FileNotFound followed by InclusionDirective.
+  // We want the former, so we're completely sure the file was missing.

is there a common recovery logic implemented in clang that makes this logic 
worth it?
because none of the existing clients seems to be implementing it.

I would rather check for `File == nullptr` in `InclusionDirective` and maybe 
put a fixme saying that this might still be set by a "recovering ppcallback".

Up to you though.



Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:137
+// ...relative to the including file.
+if (!IsAngled)
+  if (const FileEntry *IncludingFile =

nit: braces



Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:590
+  for (const auto &F : MissingFiles) {
+// A missing file may be "provided" by an override buffer.
+if (OverridenFileBuffers.count(F.getKey()))

i believe it can also be provided by an overridefile. maybe also check this 
above while going over `RemappedFiles`. i.e:

```
for(... &R : ..RemappedFiles) {
   if(!stat(R.second)) return false;
   // A missing file has been remapped into an existing file.
   if(MissingFiles.count(R.first)) return false;
 
}
```



Comment at: clang/lib/Frontend/PrecompiledPreamble.cpp:591
+// A missing file may be "provided" by an override buffer.
+if (OverridenFileBuffers.count(F.getKey()))
+  return false;

i am not sure if this works in all cases.

Assume there's a mapped buffer for "a.h", which is relative to main file.

I suppose this will be recorded with fullpath in MissingFiles, hence this 
lookup will fail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77942



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

[clang-tools-extra] 5d56712 - [clangd] Disable failing target_info test

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-04-11T22:17:01+02:00
New Revision: 5d5671242eb731bf3bc504b94b6a472cab23a359

URL: 
https://github.com/llvm/llvm-project/commit/5d5671242eb731bf3bc504b94b6a472cab23a359
DIFF: 
https://github.com/llvm/llvm-project/commit/5d5671242eb731bf3bc504b94b6a472cab23a359.diff

LOG: [clangd] Disable failing target_info test

Added: 


Modified: 


Removed: 
clang-tools-extra/clangd/test/target_info.test



diff  --git a/clang-tools-extra/clangd/test/target_info.test 
b/clang-tools-extra/clangd/test/target_info.test
deleted file mode 100644
index ae45c34b31d8..
--- a/clang-tools-extra/clangd/test/target_info.test
+++ /dev/null
@@ -1,35 +0,0 @@
-# REQUIRES: x86-registered-target
-# Mock 'compile_commands.json' to contain a driver name targeting fuchsia OS.
-# Afterwards check that correct target is passed into clang.
-
-# RUN: rm -rf %t.dir && mkdir -p %t.dir
-
-# RUN: echo '[{"directory": "%/t.dir", "command": 
"%/t.dir/x86_64-fuchsia-clang -x c++ the-file.cpp -v", "file": 
"the-file.cpp"}]' > %t.dir/compile_commands.json
-
-# RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
-# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
-# (with the extra slash in the front), so we add it here.
-# RUN: sed -E -e "s|file://([A-Z]):/|file:///\1:/|g" %t.test.1 > %t.test
-
-# RUN: clangd -lit-test < %t.test 2>&1 | FileCheck -strict-whitespace %t.test
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}

-{
-  "jsonrpc":"2.0",
-  "method":"textDocument/didOpen",
-  "params": {
-"textDocument": {
-  "uri": "file://INPUT_DIR/the-file.cpp",
-  "languageId":"cpp",
-  "version":1,
-  "text":""
-}
-  }
-}
-# Make sure we have target passed into cc1 driver, which is printed due to -v 
in
-# the compile_commands.json
-# CHECK: Target: x86_64-unknown-fuchsia

-{"jsonrpc":"2.0","id":1,"method":"shutdown"}

-{"jsonrpc":"2.0","method":"exit"}



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

[clang-tools-extra] d34a91a - [clangd][test] Provide registered targets to lit tests

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-04-11T22:03:08+02:00
New Revision: d34a91a10f7a25e3046f2d74df830a77ef81aebb

URL: 
https://github.com/llvm/llvm-project/commit/d34a91a10f7a25e3046f2d74df830a77ef81aebb
DIFF: 
https://github.com/llvm/llvm-project/commit/d34a91a10f7a25e3046f2d74df830a77ef81aebb.diff

LOG: [clangd][test] Provide registered targets to lit tests

Summary:
We had tests in clangd (target_info.test) that got enabled only on
systems that know about x86. But they were always disabled as clangd lit config
never registered those targets.

This patch adds those targets as `$TARGET$-registered-target`

Reviewers: sammccall

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/test/lit.cfg.py

Removed: 




diff  --git a/clang-tools-extra/clangd/test/lit.cfg.py 
b/clang-tools-extra/clangd/test/lit.cfg.py
index 5030ca356ef8..93e4463450e5 100644
--- a/clang-tools-extra/clangd/test/lit.cfg.py
+++ b/clang-tools-extra/clangd/test/lit.cfg.py
@@ -10,10 +10,19 @@
 config.test_source_root = config.clangd_source_dir + "/test"
 config.test_exec_root = config.clangd_binary_dir + "/test"
 
+
+# Used to enable tests based on the required targets. Can be queried with e.g.
+#REQUIRES: x86-registered-target
+def calculate_arch_features(arch_string):
+  return [arch.lower() + '-registered-target' for arch in arch_string.split()]
+
+
+lit.llvm.llvm_config.feature_config([('--targets-built',
+  calculate_arch_features)])
+
 # Clangd-specific lit environment.
 config.substitutions.append(('%clangd-benchmark-dir',
  config.clangd_binary_dir + "/benchmarks"))
 
 if config.clangd_build_xpc:
   config.available_features.add('clangd-xpc-support')
-



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

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

[Wenlei He via Phabricator via cfe-commits]

wenlei added a comment.

In D77632#1975619 , @mehdi_amini wrote:

> The existing TLI provides a very convenient way to define a VecLib without 
> LLVM knowing about it ahead of time. This feature is important for any 
> embedded use of LLVM as a library out-of-tree (I'll add a unit-test in-tree).
>  I don't think it is a big change to this patch to preserve the current 
> ability but I wanted to check first (and in the meantime I reverted in 
> temporarily in https://reviews.llvm.org/D77925 to avoid the feature 
> regression).
>
> At the moment the place where you seem to use this knowledge is with the 
> `enum VectorLibrary`  in the `TargetLibraryInfoImpl` class, and the 
> `VecLibDescs` array which statically contains the known VecLib.
>  It seems to me that if we replace this enum with a string instead to 
> identify the VecLib everything should still hold together and this would fit 
> with minor changes to this path. The `VecLibDescs` could just be a 
> `StringMap` in this case.
>
> That was a third-party (in my case the XLA compiler) can still register its 
> own "XLA" VecLib and add all the descriptors.
>
> How does it sound?


Thanks for the explanation about the revert. The proposal of using a StringMap 
to maintain the openness sounds good to me. And agree with @tejohnson, if the 
openness is a feature, it should be covered in tests, otherwise it can feel 
somewhat like a loophole and prone to breakage, though I can see how it can be 
useful.. Hope this patch can be restored with tweaks soon (we have workloads 
with very visible vectorization that relies on this).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77632



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

[PATCH] D77952: [TLI] Reduce copies for TLI and TLA

[Wenlei He via Phabricator via cfe-commits]

wenlei created this revision.
wenlei added reviewers: tejohnson, nikic, mehdi_amini, hoyFB.
Herald added subscribers: cfe-commits, hiraditya.
Herald added a project: clang.

Minor changes to reduce the copying needed for TLI and TLA by using move 
whenever possible.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77952

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Analysis/TargetLibraryInfo.h
  llvm/lib/Analysis/TargetLibraryInfo.cpp

Index: llvm/lib/Analysis/TargetLibraryInfo.cpp
===
--- llvm/lib/Analysis/TargetLibraryInfo.cpp
+++ llvm/lib/Analysis/TargetLibraryInfo.cpp
@@ -583,8 +583,8 @@
   ShouldSignExtI32Param(TLI.ShouldSignExtI32Param) {
   std::move(std::begin(TLI.AvailableArray), std::end(TLI.AvailableArray),
 AvailableArray);
-  for (unsigned i = 0; i < NumVecLibs; i++)
-VecLibDescs[i] = TLI.VecLibDescs[i];
+  std::move(std::begin(TLI.VecLibDescs), std::end(TLI.VecLibDescs),
+VecLibDescs);
 }
 
 TargetLibraryInfoImpl &TargetLibraryInfoImpl::operator=(const TargetLibraryInfoImpl &TLI) {
@@ -1677,6 +1677,12 @@
   initializeTargetLibraryInfoWrapperPassPass(*PassRegistry::getPassRegistry());
 }
 
+TargetLibraryInfoWrapperPass::TargetLibraryInfoWrapperPass(
+TargetLibraryInfoImpl &&TLIImpl)
+: ImmutablePass(ID), TLA(std::move(TLIImpl)) {
+  initializeTargetLibraryInfoWrapperPassPass(*PassRegistry::getPassRegistry());
+}
+
 AnalysisKey TargetLibraryAnalysis::Key;
 
 // Register the basic pass.
Index: llvm/include/llvm/Analysis/TargetLibraryInfo.h
===
--- llvm/include/llvm/Analysis/TargetLibraryInfo.h
+++ llvm/include/llvm/Analysis/TargetLibraryInfo.h
@@ -452,7 +452,11 @@
   /// Construct a library analysis with baseline Module-level info.
   ///
   /// This will be supplemented with Function-specific info in the Result.
-  TargetLibraryAnalysis(TargetLibraryInfoImpl BaselineInfoImpl)
+  TargetLibraryAnalysis(const Triple &T)
+  : BaselineInfoImpl(TargetLibraryInfoImpl(T)) {}
+  TargetLibraryAnalysis(const TargetLibraryInfoImpl &BaselineInfoImpl)
+  : BaselineInfoImpl(BaselineInfoImpl) {}
+  TargetLibraryAnalysis(TargetLibraryInfoImpl &&BaselineInfoImpl)
   : BaselineInfoImpl(std::move(BaselineInfoImpl)) {}
 
   TargetLibraryInfo run(const Function &F, FunctionAnalysisManager &);
@@ -475,6 +479,7 @@
   TargetLibraryInfoWrapperPass();
   explicit TargetLibraryInfoWrapperPass(const Triple &T);
   explicit TargetLibraryInfoWrapperPass(const TargetLibraryInfoImpl &TLI);
+  explicit TargetLibraryInfoWrapperPass(TargetLibraryInfoImpl &&TLI);
 
   TargetLibraryInfo &getTLI(const Function &F) {
 FunctionAnalysisManager DummyFAM;
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -352,10 +352,6 @@
   PM.add(createStackSafetyGlobalInfoWrapperPass(/*SetMetadata=*/true));
 }
 
-static TargetLibraryInfoImpl *createTLII(llvm::Triple &TargetTriple) {
-  return new TargetLibraryInfoImpl(TargetTriple);
-}
-
 static void addSymbolRewriterPass(const CodeGenOptions &Opts,
   legacy::PassManager *MPM) {
   llvm::SymbolRewriter::RewriteDescriptorList DL;
@@ -541,13 +537,6 @@
   if (CodeGenOpts.DisableLLVMPasses)
 return;
 
-  // Figure out TargetLibraryInfo.  This needs to be added to MPM and FPM
-  // manually (and not via PMBuilder), since some passes (eg. InstrProfiling)
-  // are inserted before PMBuilder ones - they'd get the default-constructed
-  // TLI with an unknown target otherwise.
-  Triple TargetTriple(TheModule->getTargetTriple());
-  std::unique_ptr TLII(createTLII(TargetTriple));
-
   // If we reached here with a non-empty index file name, then the index file
   // was empty and we are not performing ThinLTO backend compilation (used in
   // testing in a distributed build environment). Drop any the type test
@@ -558,6 +547,7 @@
  /*ImportSummary=*/nullptr,
  /*DropTypeTests=*/true));
 
+  Triple TargetTriple(TheModule->getTargetTriple());
   PassManagerBuilderWrapper PMBuilder(TargetTriple, CodeGenOpts, LangOpts);
 
   // At O0 and O1 we only run the always inliner which is more efficient. At
@@ -591,7 +581,11 @@
   PMBuilder.PrepareForLTO = CodeGenOpts.PrepareForLTO;
   PMBuilder.RerollLoops = CodeGenOpts.RerollLoops;
 
-  MPM.add(new TargetLibraryInfoWrapperPass(*TLII));
+  // Figure out TargetLibraryInfo. This needs to be added to MPM and FPM
+  // manually (and not via PMBuilder), since some passes (eg. InstrProfiling)
+  // are inserted before PMBuilder ones - they'd get the default-constructed
+  // TLI with an unknown target otherwise.
+  MPM.add(new TargetLibraryInfoWrapperPass(TargetTriple));
 
   if (TM)
 TM->adjustPassManager(PMBuilder)

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

[Wenlei He via Phabricator via cfe-commits]

wenlei added a comment.

In D77632#1974409 , @tejohnson wrote:

> In D77632#1974363 , @wenlei wrote:
>
> > In D77632#1974015 , @nikic wrote:
> >
> > > This change causes a ~0.5% compile-time regressions: 
> > > http://llvm-compile-time-tracker.com/compare.php?from=5b18b6e9a84d985c0a907009fb71de7c1943bc88&to=60c642e74be6af86906d9f3d982728be7bd4329f&stat=instructions
> > >  This is quite a lot as these things go, so it would be great if you 
> > > could double check if there's any optimization potential here. In 
> > > particular I'm wondering why this affects normal builds so much, even 
> > > though they (presumably?) don't use any veclib at all.
> >
> >
> > Thanks for the heads-up. This is surprising but there is a change even when 
> > veclib is not used - in order to allow each function to use different 
> > veclib without duplicating the work of populating vector function list for 
> > each function, we now always pre-populate vector function list for three 
> > supported vector libraries for each module. However 0.5% compile-time for 
> > that work given it's per-module is not expected. I suspect we may be 
> > passing/copying TLII around more than we anticipated (now we always have 
> > more stuff to copy). I will take a look. We could also turn this into a 
> > lazy initialization - only populate the needed list for module level TLII 
> > when it's first queried by a function level TLI.
>
>
> Hmm, yeah that is surprising, because the TLII should be built once per 
> module per TLI analysis, which is never invalidated. We've gone from 
> populating one set of vec libs to 3, I wouldn't have thought that was 
> particularly expensive, so it would be good to see what is going on here and 
> confirm we are only building this once as expected.
>
> Looking at the compile time data at that link, interestingly the 
> "instructions" metric increased, but not wall time or cycles or task clock - 
> they were all neutral.


Turns out there're a few places where we call copy ctor for TLI unnecessarily. 
Made some changes in D77952  to use move when 
possible. In addition, I should have used move for `TLI.VecLibDescs` in move 
ctor of `TargetLibraryInfoImpl` too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77632



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

[PATCH] D77777: [nvptx] Add `nvvm.texsurf.handle` internalizer.

[David Majnemer via Phabricator via cfe-commits]

majnemer added a comment.

In D7#1975618 , @hliao wrote:

> In D7#1975406 , @tra wrote:
>
> > In D7#1975178 , @hliao wrote:
> >
> > > the 1st argument in `llvm.nvvm.texsurf.hande.internal` or the 2nd one in 
> > > `llvm.nvvm.texsurf.handle` must be kept as an immediate or constant 
> > > value, i.e. that global variable. However, optimizations will find common 
> > > code in the following
> > >
> > >   if (cond) {
> > > %hnd = texsurf.handle.internal(@tex1);
> > >   } else {
> > > %hnd = texsurf.handle.internal(@tex2)
> > >   }
> > >   = use(%hnd)
> > >
> > >
> > > and hoist or sink it into
> > >
> > >   if (cond) {
> > > %ptr = @tex1;
> > >   } else {
> > > %ptr = @tex2;
> > >   }
> > >   %hnd = texsurf.handle.intenal(%ptr);
> > >   = use(%hnd)
> > >
> > >
> > > The backend cannot handle non immediate operand in `texsurf.handle`. The 
> > > similar thing happens to `read.register` as well as it also assumes its 
> > > argument is always an immediate value.
> >
> >
> > I wonder if we can use `token` types to represent the handle? 
> > https://reviews.llvm.org/D11861
> >  @majnemer -- would this use case be suitable for the `token` type?
>
>
> If we still could make PHI over token, it canont serve this purpose. Check 
> `llvm::canReplaceOperandWithVariable` for operand for details.


It is not possible to PHI a token value. Token values disable the call to 
canReplaceOperandWithVariable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D7



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

[PATCH] D77882: [clang-tidy] Add option to use alpha checkers from clang-analyzer when using `run-clang-tidy.py`

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

please add this to the release notes too :)
something like "new option -allow-enabling-alpha-checkers added to 
run-clang-tidy to enable alpha checkers"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77882



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

[PATCH] D75788: [OpenMP] Provide math functions in OpenMP device code via OpenMP variants

[Johannes Doerfert via Phabricator via cfe-commits]

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



Comment at: clang/test/Headers/nvptx_device_math_macro.cpp:11
+  double a(0);
+// CHECK-NOT:  call
+// CHECK:  call double @llvm.fabs.f64(double

sammccall wrote:
> Hmm, this fails if the test is run from a directory containing "call".
> 
> > ; ModuleID = 
> > '/usr/local/google/home/sammccall/src/llvm-project/clang/test/Headers/nvptx_device_math_macro.cpp
> 
> I guess I can blame my parents :-)
> Is `CHECK-NOT: call double` correct for both of these?
Oh my, ... sorry.

I guess `call{{.*}}@` should do the trick. No globals are referenced in a call


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75788



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

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

In D77632#1976126 , @tejohnson wrote:

> I think this should work. Just reiterating something we chatted about off 
> patch yesterday, we really need a unit test that mimics the behavior utilized 
> by the XLA compiler, for regression testing.


Yes I pinged some of the XLA folks to make it happen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77632



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

[PATCH] D77910: AMDGPU: Define cl_khr_gl_sharing as a supported extension

[Brian Sumner via Phabricator via cfe-commits]

b-sumner added a comment.

I don't think we can guarantee this is or will be supported on all devices.  
The language runtime makes this decision.


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

https://reviews.llvm.org/D77910



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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza added a comment.

> If someone writes up a short proposal for this, with motivation and impact, 
> we'd be happy to present it internally.

I have a rough draft that I'll be touching up and submitting Monday most 
likely! Thanks, John!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574



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

[PATCH] D77947: [clangd] Send the correct error code when cancelling requests.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, jfb, arphaman, jkorous, 
MaskRay, javed.absar, ilya-biryukov.
Herald added a project: clang.

I couldn't quite bring myself to make Cancellation depend on LSP ErrorCode.
Magic numbers instead...


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77947

Files:
  clang-tools-extra/clangd/Cancellation.cpp
  clang-tools-extra/clangd/Cancellation.h
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/JSONTransport.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/unittests/CancellationTests.cpp
  clang-tools-extra/clangd/unittests/JSONTransportTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -414,7 +414,9 @@
 EXPECT_FALSE(bool(AST));
 llvm::Error E = AST.takeError();
 EXPECT_TRUE(E.isA());
-consumeError(std::move(E));
+handleAllErrors(std::move(E), [&](const CancelledError &E) {
+  EXPECT_EQ(E.Reason, static_cast(ErrorCode::ContentModified));
+});
   },
   TUScheduler::InvalidateOnUpdate);
   S.runWithAST(
Index: clang-tools-extra/clangd/unittests/JSONTransportTests.cpp
===
--- clang-tools-extra/clangd/unittests/JSONTransportTests.cpp
+++ clang-tools-extra/clangd/unittests/JSONTransportTests.cpp
@@ -5,6 +5,7 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===--===//
+#include "Cancellation.h"
 #include "Protocol.h"
 #include "Transport.h"
 #include "gmock/gmock.h"
@@ -70,6 +71,9 @@
 if (Method == "err")
   Target.reply(
   ID, llvm::make_error("trouble at mill", ErrorCode(88)));
+else if (Method == "invalidated") // gone out skew on treadle
+  Target.reply(ID, llvm::make_error(
+   static_cast(ErrorCode::ContentModified)));
 else
   Target.reply(ID, std::move(Params));
 return true;
@@ -140,7 +144,7 @@
 ---
 {"jsonrpc": "2.0", "id": "xyz", "error": {"code": 99, "message": "bad!"}}
 ---
-{"jsonrpc": "2.0", "method": "err", "id": "wxyz", "params": "boom!"}
+{"jsonrpc": "2.0", "method": "invalidated", "id": "wxyz", "params": "boom!"}
 ---
 {"jsonrpc": "2.0", "method": "exit"}
   )jsonrpc",
@@ -154,7 +158,7 @@
 Reply(1234): 5678
 Call foo("abcd"): "efgh"
 Reply("xyz"): error = 99: bad!
-Call err("wxyz"): "boom!"
+Call invalidated("wxyz"): "boom!"
 Notification exit: null
   )";
   EXPECT_EQ(trim(E.log()), trim(WantLog));
@@ -171,11 +175,11 @@
   "jsonrpc": "2.0",
   "result": "efgh"
 })"
-   "Content-Length: 105\r\n\r\n"
+   "Content-Length: 145\r\n\r\n"
R"({
   "error": {
-"code": 88,
-"message": "trouble at mill"
+"code": -32801,
+"message": "Request cancelled because the document was modified"
   },
   "id": "wxyz",
   "jsonrpc": "2.0"
Index: clang-tools-extra/clangd/unittests/CancellationTests.cpp
===
--- clang-tools-extra/clangd/unittests/CancellationTests.cpp
+++ clang-tools-extra/clangd/unittests/CancellationTests.cpp
@@ -45,12 +45,13 @@
 }
 
 struct NestedTasks {
+  enum { OuterReason = 1, InnerReason = 2 };
   std::pair Outer, Inner;
   NestedTasks() {
-Outer = cancelableTask();
+Outer = cancelableTask(OuterReason);
 {
   WithContext WithOuter(Outer.first.clone());
-  Inner = cancelableTask();
+  Inner = cancelableTask(InnerReason);
 }
   }
 };
@@ -59,13 +60,13 @@
   // Cancelling inner task works but leaves outer task unaffected.
   NestedTasks CancelInner;
   CancelInner.Inner.second();
-  EXPECT_TRUE(isCancelled(CancelInner.Inner.first));
+  EXPECT_EQ(NestedTasks::InnerReason, isCancelled(CancelInner.Inner.first));
   EXPECT_FALSE(isCancelled(CancelInner.Outer.first));
   // Cancellation of outer task is inherited by inner task.
   NestedTasks CancelOuter;
   CancelOuter.Outer.second();
-  EXPECT_TRUE(isCancelled(CancelOuter.Inner.first));
-  EXPECT_TRUE(isCancelled(CancelOuter.Outer.first));
+  EXPECT_EQ(NestedTasks::OuterReason, isCancelled(CancelOuter.Inner.first));
+  EXPECT_EQ(NestedTasks::OuterReason, isCancelled(CancelOuter.Outer.first));
 }
 
 TEST(CancellationTest, AsynCancellationTest) {
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -648,8 +648,8 @@
   

[PATCH] D77632: [TLI] Per-function fveclib for math library used for vectorization

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D77632#1975619 , @mehdi_amini wrote:

> The existing TLI provides a very convenient way to define a VecLib without 
> LLVM knowing about it ahead of time. This feature is important for any 
> embedded use of LLVM as a library out-of-tree (I'll add a unit-test in-tree).
>  I don't think it is a big change to this patch to preserve the current 
> ability but I wanted to check first (and in the meantime I reverted in 
> temporarily in https://reviews.llvm.org/D77925 to avoid the feature 
> regression).
>
> At the moment the place where you seem to use this knowledge is with the 
> `enum VectorLibrary`  in the `TargetLibraryInfoImpl` class, and the 
> `VecLibDescs` array which statically contains the known VecLib.
>  It seems to me that if we replace this enum with a string instead to 
> identify the VecLib everything should still hold together and this would fit 
> with minor changes to this path. The `VecLibDescs` could just be a 
> `StringMap` in this case.
>
> That was a third-party (in my case the XLA compiler) can still register its 
> own "XLA" VecLib and add all the descriptors.
>
> How does it sound?


I think this should work. Just reiterating something we chatted about off patch 
yesterday, we really need a unit test that mimics the behavior utilized by the 
XLA compiler, for regression testing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77632



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

[PATCH] D76365: [cuda][hip] Add CUDA builtin surface/texture reference support.

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

In D76365#1975784 , @tra wrote:

> It appears I can crash clang with some texture code: 
> https://godbolt.org/z/5vdEwC


`llvm.nvvm.tex.unified.2d.v4f32.f32` has a vector output, the alias

  __attribute__((device)) float tex2d_ld(tex_t, float, float) 
asm("llvm.nvvm.tex.unified.2d.v4f32.f32");

needs replacing with

  __attribute__((device)) v4f tex2d_ld(tex_t, float, float) 
asm("llvm.nvvm.tex.unified.2d.v4f32.f32");

see this revised sample code https://godbolt.org/z/B7rtxR


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76365



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

[PATCH] D77847: [clangd] Rebuild dependent files when a header is saved.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D77847#1976077 , @sammccall wrote:

> OK, I misunderstood what we're testing, please tell me if I got it right this 
> time :-)
>
> Plausible bad behavior: we send a no-op change to a relatively inactive file. 
> TUScheduler builds an AST (wasting CPU) and caches it (wasting CPU + latency 
> by displacing a useful entry).
>
> We want to prevent either of those from happening. Caching is indirectly 
> observable, building an AST isn't particularly observable.


Building an AST is also observable(at least in the absence of read requests), 
as we always generate diagnostics afterwards and that's what 
`TUSchedulerTests.NoopOnEmptyChange` actually asserts(that we don't build ASTs 
on noop changes).
I was suggesting to test the "cache" behaviour separately from building an ast. 
I know that's redundant today, as we never cache an ast if we haven't build 
any, but there are no tests ensuring it stays that way.

> If building an AST *was* observable, then we have an alternate (IMO more 
> meaningful) way of measuring the cache effects: try to use read the 
> maybe-evicted AST and see if it rebuilds.
>  So I think it would be nicer to structure a test around rebuilds.
> 
> A couple of options I can think of:
> 
> - add an API like `unsigned TUScheduler::buildCount(PathRef)`, exposing a 
> counter only used for testing? (Or extend the memory usage API to provide 
> this info too)
> - register a tracer for the test, and count `BuildAST` events (not too 
> fragile if we assert there are exactly N > 0 of them). This seems hacky, 
> technique/tracer may be reusable for other tests. Not sure whether that's a 
> good thing.

I suppose that could be a more robust way of checking if an AST was build 
rather than waiting for diags to be released. I believe the former would be 
enough(preferably just extending the existing API to not create more test-only 
endpoints).

> (Unless you object, I'll land this patch once I have a test out for review so 
> we're sure the existing behavior this patch depends on is there)

I totally agree, please do so.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77847



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

Re: [PATCH] D63194: [clangd] Link and initialize target infos

[Kadir Çetinkaya via cfe-commits]

Hi Filipe! Thanks for letting me know, I've sent
https://reviews.llvm.org/D77944 to enable this in clangd lit config.

Regarding the failure, I can't seem to repro on machines I have access to.
The premerge-tests also seems to be succeeding, please see
https://results.llvm-merge-guard.org/BETA_amd64_debian_testing_clang8-626/console-log.txt
.

Can you provide more info about how you've ran the test?

On Fri, Apr 10, 2020 at 4:23 PM Filipe Cabecinhas  wrote:

> Hi Kadir,
>
> Can you fix the target_info.test clangd test you committed in this
> revision, please?
> I see you've tried fixing it later by adding `REQUIRES:
> x86-registered-target`, but now it's never running because that feature
> isn't (ever) set.
> Here's a buildbot run showing it as unsupported (x86 target is built):
> http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/26746/steps/test-check-all/logs/stdio
>
> I saw you tried to fix the test in r364413 ([clangd] Disable failing
> unittest on non-x86 platforms) because it was always failing. The problem
> is that, even though the feature check is needed, the test still isn't
> working well. I just ran it manually and didn't get the target changed from
> my host one.
>
> Here's a diff for the feature check that you can apply to propagate the
> feature. But we still need to get the clangd code fixed so it picks up the
> target:
>
> ```
> diff --git a/clang-tools-extra/clangd/test/lit.cfg.py
> b/clang-tools-extra/clangd/test/lit.cfg.py
> index 5030ca356ef..54406498af0 100644
> --- a/clang-tools-extra/clangd/test/lit.cfg.py
> +++ b/clang-tools-extra/clangd/test/lit.cfg.py
> @@ -3,6 +3,19 @@ import lit.llvm
>  lit.llvm.initialize(lit_config, config)
>  lit.llvm.llvm_config.use_clang()
>
> +# required for target_info.test
> +def calculate_arch_features(arch_string):
> +features = []
> +for arch in arch_string.split():
> +features.append(arch.lower() + '-registered-target')
> +return features
> +
> +lit.llvm.llvm_config.feature_config(
> +[('--assertion-mode', {'ON': 'asserts'}),
> + ('--cxxflags', {r'-D_GLIBCXX_DEBUG\b': 'libstdcxx-safe-mode'}),
> + ('--targets-built', calculate_arch_features)
> + ])
> +
>  config.name = 'Clangd'
>  config.suffixes = ['.test']
>  config.excludes = ['Inputs']
> ```
>
> Thank you,
>
>   Filipe
>
>   Filipe
>
>
> On Wed, Jun 26, 2019 at 8:48 AM Kadir Cetinkaya via Phabricator via
> cfe-commits  wrote:
>
>> kadircet closed this revision.
>> kadircet added a comment.
>>
>> Landed as rL364387 
>>
>>
>> Repository:
>>   rG LLVM Github Monorepo
>>
>> CHANGES SINCE LAST ACTION
>>   https://reviews.llvm.org/D63194/new/
>>
>> https://reviews.llvm.org/D63194
>>
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77944: [clangd][test] Provide registered targets to lit tests

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/test/lit.cfg.py:17
+def calculate_arch_features(arch_string):
+  features = []
+  for arch in arch_string.split():

nit: is this `return [arch.lower() + '-registered-target' for arch in 
arch_string.split()]`?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77944



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

[PATCH] D77942: [Preamble] Invalidate preamble when missing headers become present.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

At first I thought this would be easy to test via clang/test/PCH, but the outer 
layers of PrecompiledPreamble seem to be totally distinct from PCHGenerator 
etc. This is only used in ASTUnit and clangd.
Maybe it's reachable in libclang but I couldn't easily work out how.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77942



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

[PATCH] D77938: [clangd] Extend YAML Serialization

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks for doing this!




Comment at: clang-tools-extra/clangd/unittests/YAMLTests.cpp:1
+//===-- YAMLTests.cpp - YAML container unit tests 
-===//
+//

Ack, sorry for not mentioning this in the previous review.
The YAML stuff is tested mixed together with the binary format stuff in 
SerializationTests.cpp.
Is it possible to extend the YAML literal in that file with a synthetic Source 
and Cmd entry, and test it in a similar way?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77938



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

[PATCH] D77944: [clangd][test] Provide registered targets to lit tests

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

We had tests in clangd (target_info.test) that got enabled only on
systems that know about x86. But they were always disabled as clangd lit config
never registered those targets.

This patch adds those targets as `$TARGET$-registered-target`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77944

Files:
  clang-tools-extra/clangd/test/lit.cfg.py


Index: clang-tools-extra/clangd/test/lit.cfg.py
===
--- clang-tools-extra/clangd/test/lit.cfg.py
+++ clang-tools-extra/clangd/test/lit.cfg.py
@@ -10,10 +10,22 @@
 config.test_source_root = config.clangd_source_dir + "/test"
 config.test_exec_root = config.clangd_binary_dir + "/test"
 
+
+# Used to enable tests based on the required targets. Can be queried with e.g.
+#REQUIRES: x86-registered-target
+def calculate_arch_features(arch_string):
+  features = []
+  for arch in arch_string.split():
+features.append(arch.lower() + '-registered-target')
+  return features
+
+
+lit.llvm.llvm_config.feature_config([('--targets-built',
+  calculate_arch_features)])
+
 # Clangd-specific lit environment.
 config.substitutions.append(('%clangd-benchmark-dir',
  config.clangd_binary_dir + "/benchmarks"))
 
 if config.clangd_build_xpc:
   config.available_features.add('clangd-xpc-support')
-


Index: clang-tools-extra/clangd/test/lit.cfg.py
===
--- clang-tools-extra/clangd/test/lit.cfg.py
+++ clang-tools-extra/clangd/test/lit.cfg.py
@@ -10,10 +10,22 @@
 config.test_source_root = config.clangd_source_dir + "/test"
 config.test_exec_root = config.clangd_binary_dir + "/test"
 
+
+# Used to enable tests based on the required targets. Can be queried with e.g.
+#REQUIRES: x86-registered-target
+def calculate_arch_features(arch_string):
+  features = []
+  for arch in arch_string.split():
+features.append(arch.lower() + '-registered-target')
+  return features
+
+
+lit.llvm.llvm_config.feature_config([('--targets-built',
+  calculate_arch_features)])
+
 # Clangd-specific lit environment.
 config.substitutions.append(('%clangd-benchmark-dir',
  config.clangd_binary_dir + "/benchmarks"))
 
 if config.clangd_build_xpc:
   config.available_features.add('clangd-xpc-support')
-
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77847: [clangd] Rebuild dependent files when a header is saved.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D77847#1976020 , @kadircet wrote:

> Thanks, LGTM!
>
> In D77847#1975940 , @sammccall wrote:
>
> > In D77847#1974126 , @kadircet 
> > wrote:
> >
> > > should we also have a unittest for checking ast caching works as expected?
> >
> >
> > TUSchedulerTests has `NoopOnEmptyChanges` which I think tests exactly this, 
> > and `EvictedAST` which checks it's sensitive to the cache.
>
>
> Right, but none of them checks exactly this behaviour.  The 
> `NoopOnEmptyChanges` test only ensures we don't publish diagnostics again
>  and `EvictedAST` test actually asserts on the opposite, it checks the last 
> updated file is "always" cached. Maybe extending `EvictedAST`
>  test with a "noop update" might be a better fit. Up to you though.


OK, I misunderstood what we're testing, please tell me if I got it right this 
time :-)

Plausible bad behavior: we send a no-op change to a relatively inactive file. 
TUScheduler builds an AST (wasting CPU) and caches it (wasting CPU + latency by 
displacing a useful entry).

We want to prevent either of those from happening. Caching is indirectly 
observable, building an AST isn't particularly observable. If building an AST 
*was* observable, then we have an alternate (IMO more meaningful) way of 
measuring the cache effects: try to use read the maybe-evicted AST and see if 
it rebuilds.
So I think it would be nicer to structure a test around rebuilds.

A couple of options I can think of:

- add an API like `unsigned TUScheduler::buildCount(PathRef)`, exposing a 
counter only used for testing? (Or extend the memory usage API to provide this 
info too)
- register a tracer for the test, and count `BuildAST` events (not too fragile 
if we assert there are exactly N > 0 of them). This seems hacky, 
technique/tracer may be reusable for other tests. Not sure whether that's a 
good thing.

(Unless you object, I'll land this patch once I have a test out for review so 
we're sure the existing behavior this patch depends on is there)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77847



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

[clang] 52dcbcb - Simplify string joins. NFCI.

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2020-04-11T17:20:11+02:00
New Revision: 52dcbcbfe07acfe79ac16b343684f8786f1fd0d7

URL: 
https://github.com/llvm/llvm-project/commit/52dcbcbfe07acfe79ac16b343684f8786f1fd0d7
DIFF: 
https://github.com/llvm/llvm-project/commit/52dcbcbfe07acfe79ac16b343684f8786f1fd0d7.diff

LOG: Simplify string joins. NFCI.

Added: 


Modified: 
clang/lib/Format/BreakableToken.cpp
clang/lib/Sema/Sema.cpp
clang/tools/driver/cc1as_main.cpp
llvm/lib/Support/Triple.cpp
llvm/tools/llvm-cov/SourceCoverageViewHTML.cpp

Removed: 




diff  --git a/clang/lib/Format/BreakableToken.cpp 
b/clang/lib/Format/BreakableToken.cpp
index 1d2fefeef26e..4294c2a653fa 100644
--- a/clang/lib/Format/BreakableToken.cpp
+++ b/clang/lib/Format/BreakableToken.cpp
@@ -588,8 +588,7 @@ void BreakableBlockComment::insertBreak(unsigned LineIndex, 
unsigned TailOffset,
   unsigned CharsToRemove = Split.second;
   assert(LocalIndentAtLineBreak >= Prefix.size());
   std::string PrefixWithTrailingIndent = std::string(Prefix);
-  for (unsigned I = 0; I < ContentIndent; ++I)
-PrefixWithTrailingIndent += " ";
+  PrefixWithTrailingIndent.append(ContentIndent, ' ');
   Whitespaces.replaceWhitespaceInToken(
   tokenAt(LineIndex), BreakOffsetInToken, CharsToRemove, "",
   PrefixWithTrailingIndent, InPPDirective, /*Newlines=*/1,

diff  --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 6c1666162c81..5e5a90ad0143 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -2354,16 +2354,8 @@ std::string 
Sema::getOpenCLExtensionsFromTypeExtMap(FunctionType *FT) {
 
 template 
 std::string Sema::getOpenCLExtensionsFromExtMap(T *FDT, MapT &Map) {
-  std::string ExtensionNames = "";
   auto Loc = Map.find(FDT);
-
-  for (auto const& I : Loc->second) {
-ExtensionNames += I;
-ExtensionNames += " ";
-  }
-  ExtensionNames.pop_back();
-
-  return ExtensionNames;
+  return llvm::join(Loc->second, " ");
 }
 
 bool Sema::isOpenCLDisabledDecl(Decl *FD) {

diff  --git a/clang/tools/driver/cc1as_main.cpp 
b/clang/tools/driver/cc1as_main.cpp
index 016df0be1e15..77b99b201364 100644
--- a/clang/tools/driver/cc1as_main.cpp
+++ b/clang/tools/driver/cc1as_main.cpp
@@ -429,12 +429,7 @@ static bool ExecuteAssembler(AssemblerInvocation &Opts,
 SrcMgr.getMemoryBuffer(BufferIndex)->getBuffer());
 
   // Build up the feature string from the target feature list.
-  std::string FS;
-  if (!Opts.Features.empty()) {
-FS = Opts.Features[0];
-for (unsigned i = 1, e = Opts.Features.size(); i != e; ++i)
-  FS += "," + Opts.Features[i];
-  }
+  std::string FS = llvm::join(Opts.Features, ",");
 
   std::unique_ptr Str;
 

diff  --git a/llvm/lib/Support/Triple.cpp b/llvm/lib/Support/Triple.cpp
index 1a210d29e1bf..da6b877a8504 100644
--- a/llvm/lib/Support/Triple.cpp
+++ b/llvm/lib/Support/Triple.cpp
@@ -9,6 +9,7 @@
 #include "llvm/ADT/Triple.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Host.h"
@@ -985,12 +986,7 @@ std::string Triple::normalize(StringRef Str) {
   }
 
   // Stick the corrected components back together to form the normalized 
string.
-  std::string Normalized;
-  for (unsigned i = 0, e = Components.size(); i != e; ++i) {
-if (i) Normalized += '-';
-Normalized += Components[i];
-  }
-  return Normalized;
+  return join(Components, "-");
 }
 
 StringRef Triple::getArchName() const {

diff  --git a/llvm/tools/llvm-cov/SourceCoverageViewHTML.cpp 
b/llvm/tools/llvm-cov/SourceCoverageViewHTML.cpp
index 1363e0a52c11..9d10def0a211 100644
--- a/llvm/tools/llvm-cov/SourceCoverageViewHTML.cpp
+++ b/llvm/tools/llvm-cov/SourceCoverageViewHTML.cpp
@@ -30,8 +30,7 @@ std::string escape(StringRef Str, const CoverageViewOptions 
&Opts) {
 if (C == '\t') {
   // Replace '\t' with up to TabSize spaces.
   unsigned NumSpaces = Opts.TabSize - (ColNum % Opts.TabSize);
-  for (unsigned I = 0; I < NumSpaces; ++I)
-TabExpandedResult += ' ';
+  TabExpandedResult.append(NumSpaces, ' ');
   ColNum += NumSpaces;
 } else {
   TabExpandedResult += C;



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

[clang-tools-extra] a50df66 - [clangd] Remove redundant code in test. NFC

[via cfe-commits]

Author: Sam McCall
Date: 2020-04-11T17:14:31+02:00
New Revision: a50df668f6889707cad53f1b5339179e337a9eef

URL: 
https://github.com/llvm/llvm-project/commit/a50df668f6889707cad53f1b5339179e337a9eef
DIFF: 
https://github.com/llvm/llvm-project/commit/a50df668f6889707cad53f1b5339179e337a9eef.diff

LOG: [clangd] Remove redundant code in test. NFC

Added: 


Modified: 
clang-tools-extra/clangd/unittests/SerializationTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/SerializationTests.cpp 
b/clang-tools-extra/clangd/unittests/SerializationTests.cpp
index 89739f420298..391761a29bad 100644
--- a/clang-tools-extra/clangd/unittests/SerializationTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SerializationTests.cpp
@@ -104,9 +104,6 @@ TEST(SerializationTest, NoCrashOnEmptyYAML) {
 }
 
 TEST(SerializationTest, YAMLConversions) {
-  auto In = readIndexFile(YAML);
-  EXPECT_TRUE(bool(In)) << In.takeError();
-
   auto ParsedYAML = readIndexFile(YAML);
   ASSERT_TRUE(bool(ParsedYAML)) << ParsedYAML.takeError();
   ASSERT_TRUE(bool(ParsedYAML->Symbols));



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

[PATCH] D76768: [analyzer] Added support of scan-build and exploded-graph-rewriter regression tests for Windows

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@thakis,

> ^ This is not usually true. (But I think lit adds build/bin to the PATH for 
> tests, so it's possibly true for tests, which would be enough.)

AFAIK lit do not add anything to PATH. You should do it by yourself.

> If you run `del bin\*` followed by `make check-clang` (or `ninja check-clang` 
> or what have you), then I think bin/scan-build won't be built since it's not 
> a dependency of the check-clang target. It doesn't have to be a dependency on 
> non-Win, but on Win the execution flow is different due to the bat trampoline.

I did as you ask. Confirm your suggestion: scan-build have not appeared after 
`ninja check-clang`. All the tests except one passed. Log attached F11713079: 
log_ninja_check-clang.txt .
Intresting but scan-build tests haven't been run through `ninja check-clang`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76768



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

[PATCH] D77942: [Preamble] Invalidate preamble when missing headers become present.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, 
javed.absar.
Herald added a project: clang.

To avoid excessive extra stat()s, only check the possible locations of
headers that weren't found at all (leading to a compile error).
For headers that *were* found, we don't check for files earlier on the
search path that could override them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77942

Files:
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang/include/clang/Frontend/PrecompiledPreamble.h
  clang/lib/Frontend/PrecompiledPreamble.cpp

Index: clang/lib/Frontend/PrecompiledPreamble.cpp
===
--- clang/lib/Frontend/PrecompiledPreamble.cpp
+++ clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -19,15 +19,19 @@
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/FrontendOptions.h"
+#include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Serialization/ASTWriter.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSet.h"
+#include "llvm/ADT/iterator_range.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include 
@@ -74,6 +78,78 @@
   bool needSystemDependencies() override { return true; }
 };
 
+// Collects files whose existence would invalidate the preamble.
+// Collecting *all* of these would make validating it too slow though, so we
+// just find all the candidates for 'file not found' diagnostics.
+//
+// A caveat that may be significant for generated files: we'll omit files under
+// search path entries whose roots don't exist when the preamble is built.
+// These are pruned by InitHeaderSearch and so we don't see the search path.
+// It would be nice to include them but we don't want to duplicate all the rest
+// of the InitHeaderSearch logic to reconstruct them.
+class MissingFileCollector : public PPCallbacks {
+  llvm::StringSet<> &Out;
+  const HeaderSearch &Search;
+  std::string LastNotFound;
+  const SourceManager &SM;
+
+public:
+  MissingFileCollector(llvm::StringSet<> &Out, const HeaderSearch &Search,
+   const SourceManager &SM)
+  : Out(Out), Search(Search), SM(SM) {}
+
+  // We always receive FileNotFound followed by InclusionDirective.
+  // We want the former, so we're completely sure the file was missing.
+  // We also need the latter to see whether the include was .
+  bool FileNotFound(StringRef FileName,
+SmallVectorImpl &RecoveryPath) override {
+// If it's a rare absolute include, we know the full path already.
+if (llvm::sys::path::is_absolute(FileName)) {
+  Out.insert(FileName);
+  LastNotFound.clear();
+}
+LastNotFound = FileName.str();
+return false;
+  }
+
+  void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
+  StringRef FileName, bool IsAngled,
+  CharSourceRange FilenameRange, const FileEntry *File,
+  StringRef SearchPath, StringRef RelativePath,
+  const Module *Imported,
+  SrcMgr::CharacteristicKind FileType) override {
+// Only process this if we match it with a previous FileNotFound.
+// Allow this even if File is set, it may be set from recovery.
+bool Match = LastNotFound == FileName;
+LastNotFound.clear();
+if (!Match)
+  return;
+
+// Reconstruct the filenames that would satisfy this directive...
+llvm::SmallString<256> Buf;
+auto NotFoundRelativeTo = [&](const DirectoryEntry *DE) {
+  Buf = DE->getName();
+  llvm::sys::path::append(Buf, FileName);
+  llvm::sys::path::remove_dots(Buf, /*remove_dot_dot=*/true);
+  Out.insert(Buf);
+};
+// ...relative to the including file.
+if (!IsAngled)
+  if (const FileEntry *IncludingFile =
+  SM.getFileEntryForID(SM.getFileID(IncludeTok.getLocation(
+if (IncludingFile->getDir())
+  NotFoundRelativeTo(IncludingFile->getDir());
+// ...relative to the search paths.
+for (const auto &Dir : llvm::make_range(
+ IsAngled ? Search.angled_dir_begin() : Search.search_dir_begin(),
+ Search.search_dir_end())) {
+  // No support for frameworks or header maps yet.
+  if (Dir.isNormalDir())
+NotFoundRelativeTo(Dir.getDir());
+}
+  }
+};
+
 /// Keeps a track of files to be deleted in destructor.
 class TemporaryFiles {
 public:
@@ -353,6 +4

[PATCH] D75788: [OpenMP] Provide math functions in OpenMP device code via OpenMP variants

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/test/Headers/nvptx_device_math_macro.cpp:11
+  double a(0);
+// CHECK-NOT:  call
+// CHECK:  call double @llvm.fabs.f64(double

Hmm, this fails if the test is run from a directory containing "call".

> ; ModuleID = 
> '/usr/local/google/home/sammccall/src/llvm-project/clang/test/Headers/nvptx_device_math_macro.cpp

I guess I can blame my parents :-)
Is `CHECK-NOT: call double` correct for both of these?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75788



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

[PATCH] D77908: [WebAssembly] Enable nontrapping-fptoint for `default` cpu

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

Looks good! I don't have anything to add beyond Thomas' review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77908



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

[PATCH] D77847: [clangd] Rebuild dependent files when a header is saved.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Thanks, LGTM!

In D77847#1975940 , @sammccall wrote:

> In D77847#1974126 , @kadircet wrote:
>
> > should we also have a unittest for checking ast caching works as expected?
>
>
> TUSchedulerTests has `NoopOnEmptyChanges` which I think tests exactly this, 
> and `EvictedAST` which checks it's sensitive to the cache.


Right, but none of them checks exactly this behaviour.  The 
`NoopOnEmptyChanges` test only ensures we don't publish diagnostics again
and `EvictedAST` test actually asserts on the opposite, it checks the last 
updated file is "always" cached. Maybe extending `EvictedAST`
test with a "noop update" might be a better fit. Up to you though.

> I don't think testing it again indirectly at this level makes is a good 
> tradeoff, especially if we need subtle assertions via the memory usage.

To be clear I wasn't suggesting a test in ClangdLSPServer layer, but rather on 
ClangdServer layer. but as mentioned above, tuscheduler might
be a better level.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77847



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

[PATCH] D77940: [AArch64] Add NVIDIA Carmel support

[Raul Tambre via Phabricator via cfe-commits]

tambre updated this revision to Diff 256764.
tambre added a comment.

Fix formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77940

Files:
  clang/test/Driver/aarch64-cpus.c
  clang/test/Preprocessor/aarch64-target-features.c
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/lib/Support/Host.cpp
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/test/CodeGen/AArch64/cpus.ll
  llvm/unittests/Support/Host.cpp
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -974,9 +974,15 @@
   AArch64::AEK_SIMD | AArch64::AEK_FP16 | AArch64::AEK_RAS |
   AArch64::AEK_LSE | AArch64::AEK_SVE | AArch64::AEK_RDM,
   "8.2-A"));
+  EXPECT_TRUE(testAArch64CPU(
+  "carmel", "armv8.2-a", "crypto-neon-fp-armv8",
+  AArch64::AEK_CRC | AArch64::AEK_CRYPTO | AArch64::AEK_FP |
+  AArch64::AEK_SIMD | AArch64::AEK_FP16 | AArch64::AEK_RAS |
+  AArch64::AEK_LSE | AArch64::AEK_SVE | AArch64::AEK_RDM,
+  "8.2-A"));
 }
 
-static constexpr unsigned NumAArch64CPUArchs = 37;
+static constexpr unsigned NumAArch64CPUArchs = 38;
 
 TEST(TargetParserTest, testAArch64CPUArchList) {
   SmallVector List;
@@ -1125,6 +1131,12 @@
AArch64::ArchKind::INVALID, "sve"));
   EXPECT_FALSE(testAArch64Extension("a64fx",
AArch64::ArchKind::INVALID, "sve2"));
+  EXPECT_TRUE(
+  testAArch64Extension("carmel", AArch64::ArchKind::INVALID, "fp16"));
+  EXPECT_TRUE(
+  testAArch64Extension("carmel", AArch64::ArchKind::INVALID, "sve"));
+  EXPECT_FALSE(
+  testAArch64Extension("carmel", AArch64::ArchKind::INVALID, "sve2"));
 
   EXPECT_FALSE(testAArch64Extension(
   "generic", AArch64::ArchKind::ARMV8A, "ras"));
Index: llvm/unittests/Support/Host.cpp
===
--- llvm/unittests/Support/Host.cpp
+++ llvm/unittests/Support/Host.cpp
@@ -262,6 +262,21 @@
 )";
 
   EXPECT_EQ(sys::detail::getHostCPUNameForARM(A64FXProcCpuInfo), "a64fx");
+
+  // Verify Nvidia Carmel.
+  const std::string CarmelProcCpuInfo = R"(
+processor   : 0
+model name  : ARMv8 Processor rev 0 (v8l)
+BogoMIPS: 62.50
+Features: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm dcpop
+CPU implementer : 0x4e
+CPU architecture: 8
+CPU variant : 0x0
+CPU part: 0x004
+CPU revision: 0
+)";
+
+  EXPECT_EQ(sys::detail::getHostCPUNameForARM(CarmelProcCpuInfo), "carmel");
 }
 
 #if defined(__APPLE__) || defined(_AIX)
Index: llvm/test/CodeGen/AArch64/cpus.ll
===
--- llvm/test/CodeGen/AArch64/cpus.ll
+++ llvm/test/CodeGen/AArch64/cpus.ll
@@ -2,6 +2,7 @@
 
 
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=generic 2>&1 | FileCheck %s
+; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=carmel 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=cortex-a35 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=cortex-a34 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=cortex-a53 2>&1 | FileCheck %s
Index: llvm/lib/Target/AArch64/AArch64Subtarget.h
===
--- llvm/lib/Target/AArch64/AArch64Subtarget.h
+++ llvm/lib/Target/AArch64/AArch64Subtarget.h
@@ -44,6 +44,7 @@
 AppleA11,
 AppleA12,
 AppleA13,
+Carmel,
 CortexA35,
 CortexA53,
 CortexA55,
Index: llvm/lib/Target/AArch64/AArch64Subtarget.cpp
===
--- llvm/lib/Target/AArch64/AArch64Subtarget.cpp
+++ llvm/lib/Target/AArch64/AArch64Subtarget.cpp
@@ -66,6 +66,7 @@
   // this in the future so we can specify it together with the subtarget
   // features.
   switch (ARMProcFamily) {
+  case Carmel:
   case Others:
 break;
   case CortexA35:
Index: llvm/lib/Target/AArch64/AArch64.td
===
--- llvm/lib/Target/AArch64/AArch64.td
+++ llvm/lib/Target/AArch64/AArch64.td
@@ -597,6 +597,15 @@
   FeatureComplxNum
   ]>;
 
+def ProcCarmel : SubtargetFeature<"carmel", "ARMProcFamily", "Carmel",
+  "Nvidia Carmel processors", [
+   HasV8_2aOps,
+   FeatureNEON,
+   FeatureCrypto,
+   FeatureFullFP16,
+   FeatureSVE
+

[PATCH] D77940: [AArch64] Add NVIDIA Carmel support

[Raul Tambre via Phabricator via cfe-commits]

tambre updated this revision to Diff 256763.
tambre added a comment.

Remove cacheline size


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77940

Files:
  clang/test/Driver/aarch64-cpus.c
  clang/test/Preprocessor/aarch64-target-features.c
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/lib/Support/Host.cpp
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/test/CodeGen/AArch64/cpus.ll
  llvm/unittests/Support/Host.cpp
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -974,9 +974,15 @@
   AArch64::AEK_SIMD | AArch64::AEK_FP16 | AArch64::AEK_RAS |
   AArch64::AEK_LSE | AArch64::AEK_SVE | AArch64::AEK_RDM,
   "8.2-A"));
+  EXPECT_TRUE(testAArch64CPU(
+  "carmel", "armv8.2-a", "crypto-neon-fp-armv8",
+  AArch64::AEK_CRC | AArch64::AEK_CRYPTO | AArch64::AEK_FP |
+  AArch64::AEK_SIMD | AArch64::AEK_FP16 | AArch64::AEK_RAS |
+  AArch64::AEK_LSE | AArch64::AEK_SVE | AArch64::AEK_RDM,
+  "8.2-A"));
 }
 
-static constexpr unsigned NumAArch64CPUArchs = 37;
+static constexpr unsigned NumAArch64CPUArchs = 38;
 
 TEST(TargetParserTest, testAArch64CPUArchList) {
   SmallVector List;
@@ -1125,6 +1131,12 @@
AArch64::ArchKind::INVALID, "sve"));
   EXPECT_FALSE(testAArch64Extension("a64fx",
AArch64::ArchKind::INVALID, "sve2"));
+  EXPECT_TRUE(testAArch64Extension("carmel",
+   AArch64::ArchKind::INVALID, "fp16"));
+  EXPECT_TRUE(testAArch64Extension("carmel",
+   AArch64::ArchKind::INVALID, "sve"));
+  EXPECT_FALSE(testAArch64Extension("carmel",
+   AArch64::ArchKind::INVALID, "sve2"));
 
   EXPECT_FALSE(testAArch64Extension(
   "generic", AArch64::ArchKind::ARMV8A, "ras"));
Index: llvm/unittests/Support/Host.cpp
===
--- llvm/unittests/Support/Host.cpp
+++ llvm/unittests/Support/Host.cpp
@@ -262,6 +262,21 @@
 )";
 
   EXPECT_EQ(sys::detail::getHostCPUNameForARM(A64FXProcCpuInfo), "a64fx");
+
+  // Verify Nvidia Carmel.
+  const std::string CarmelProcCpuInfo = R"(
+processor   : 0
+model name  : ARMv8 Processor rev 0 (v8l)
+BogoMIPS: 62.50
+Features: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm dcpop
+CPU implementer : 0x4e
+CPU architecture: 8
+CPU variant : 0x0
+CPU part: 0x004
+CPU revision: 0
+)";
+
+  EXPECT_EQ(sys::detail::getHostCPUNameForARM(CarmelProcCpuInfo), "carmel");
 }
 
 #if defined(__APPLE__) || defined(_AIX)
Index: llvm/test/CodeGen/AArch64/cpus.ll
===
--- llvm/test/CodeGen/AArch64/cpus.ll
+++ llvm/test/CodeGen/AArch64/cpus.ll
@@ -2,6 +2,7 @@
 
 
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=generic 2>&1 | FileCheck %s
+; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=carmel 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=cortex-a35 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=cortex-a34 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=cortex-a53 2>&1 | FileCheck %s
Index: llvm/lib/Target/AArch64/AArch64Subtarget.h
===
--- llvm/lib/Target/AArch64/AArch64Subtarget.h
+++ llvm/lib/Target/AArch64/AArch64Subtarget.h
@@ -44,6 +44,7 @@
 AppleA11,
 AppleA12,
 AppleA13,
+Carmel,
 CortexA35,
 CortexA53,
 CortexA55,
Index: llvm/lib/Target/AArch64/AArch64Subtarget.cpp
===
--- llvm/lib/Target/AArch64/AArch64Subtarget.cpp
+++ llvm/lib/Target/AArch64/AArch64Subtarget.cpp
@@ -66,6 +66,7 @@
   // this in the future so we can specify it together with the subtarget
   // features.
   switch (ARMProcFamily) {
+  case Carmel:
   case Others:
 break;
   case CortexA35:
Index: llvm/lib/Target/AArch64/AArch64.td
===
--- llvm/lib/Target/AArch64/AArch64.td
+++ llvm/lib/Target/AArch64/AArch64.td
@@ -597,6 +597,15 @@
   FeatureComplxNum
   ]>;
 
+def ProcCarmel : SubtargetFeature<"carmel", "ARMProcFamily", "Carmel",
+  "Nvidia Carmel processors", [
+   HasV8_2aOps,
+   FeatureNEON,
+   FeatureCrypto,
+   FeatureFullFP1

[PATCH] D77940: [AArch64] Add NVIDIA Carmel support

[Raul Tambre via Phabricator via cfe-commits]

tambre created this revision.
tambre added reviewers: sdesmalen, paquette.
Herald added subscribers: cfe-commits, danielkiss, jfb, hiraditya, 
kristof.beyls.
Herald added a project: clang.
tambre updated this revision to Diff 256763.
tambre added a comment.
tambre edited the summary of this revision.

Remove cacheline size


NVIDIA's Carmel ARM64 cores are used in Tegra194 chips found in Jetson AGX 
Xavier, DRIVE AGX Xavier and DRIVE AGX Pegasus.

References:
https://devblogs.nvidia.com/nvidia-jetson-agx-xavier-32-teraops-ai-robotics/#h.huq9xtg75a5e


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77940

Files:
  clang/test/Driver/aarch64-cpus.c
  clang/test/Preprocessor/aarch64-target-features.c
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/lib/Support/Host.cpp
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/test/CodeGen/AArch64/cpus.ll
  llvm/unittests/Support/Host.cpp
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -974,9 +974,15 @@
   AArch64::AEK_SIMD | AArch64::AEK_FP16 | AArch64::AEK_RAS |
   AArch64::AEK_LSE | AArch64::AEK_SVE | AArch64::AEK_RDM,
   "8.2-A"));
+  EXPECT_TRUE(testAArch64CPU(
+  "carmel", "armv8.2-a", "crypto-neon-fp-armv8",
+  AArch64::AEK_CRC | AArch64::AEK_CRYPTO | AArch64::AEK_FP |
+  AArch64::AEK_SIMD | AArch64::AEK_FP16 | AArch64::AEK_RAS |
+  AArch64::AEK_LSE | AArch64::AEK_SVE | AArch64::AEK_RDM,
+  "8.2-A"));
 }
 
-static constexpr unsigned NumAArch64CPUArchs = 37;
+static constexpr unsigned NumAArch64CPUArchs = 38;
 
 TEST(TargetParserTest, testAArch64CPUArchList) {
   SmallVector List;
@@ -1125,6 +1131,12 @@
AArch64::ArchKind::INVALID, "sve"));
   EXPECT_FALSE(testAArch64Extension("a64fx",
AArch64::ArchKind::INVALID, "sve2"));
+  EXPECT_TRUE(testAArch64Extension("carmel",
+   AArch64::ArchKind::INVALID, "fp16"));
+  EXPECT_TRUE(testAArch64Extension("carmel",
+   AArch64::ArchKind::INVALID, "sve"));
+  EXPECT_FALSE(testAArch64Extension("carmel",
+   AArch64::ArchKind::INVALID, "sve2"));
 
   EXPECT_FALSE(testAArch64Extension(
   "generic", AArch64::ArchKind::ARMV8A, "ras"));
Index: llvm/unittests/Support/Host.cpp
===
--- llvm/unittests/Support/Host.cpp
+++ llvm/unittests/Support/Host.cpp
@@ -262,6 +262,21 @@
 )";
 
   EXPECT_EQ(sys::detail::getHostCPUNameForARM(A64FXProcCpuInfo), "a64fx");
+
+  // Verify Nvidia Carmel.
+  const std::string CarmelProcCpuInfo = R"(
+processor   : 0
+model name  : ARMv8 Processor rev 0 (v8l)
+BogoMIPS: 62.50
+Features: fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp asimdhp cpuid asimdrdm dcpop
+CPU implementer : 0x4e
+CPU architecture: 8
+CPU variant : 0x0
+CPU part: 0x004
+CPU revision: 0
+)";
+
+  EXPECT_EQ(sys::detail::getHostCPUNameForARM(CarmelProcCpuInfo), "carmel");
 }
 
 #if defined(__APPLE__) || defined(_AIX)
Index: llvm/test/CodeGen/AArch64/cpus.ll
===
--- llvm/test/CodeGen/AArch64/cpus.ll
+++ llvm/test/CodeGen/AArch64/cpus.ll
@@ -2,6 +2,7 @@
 
 
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=generic 2>&1 | FileCheck %s
+; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=carmel 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=cortex-a35 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=cortex-a34 2>&1 | FileCheck %s
 ; RUN: llc < %s -mtriple=arm64-unknown-unknown -mcpu=cortex-a53 2>&1 | FileCheck %s
Index: llvm/lib/Target/AArch64/AArch64Subtarget.h
===
--- llvm/lib/Target/AArch64/AArch64Subtarget.h
+++ llvm/lib/Target/AArch64/AArch64Subtarget.h
@@ -44,6 +44,7 @@
 AppleA11,
 AppleA12,
 AppleA13,
+Carmel,
 CortexA35,
 CortexA53,
 CortexA55,
Index: llvm/lib/Target/AArch64/AArch64Subtarget.cpp
===
--- llvm/lib/Target/AArch64/AArch64Subtarget.cpp
+++ llvm/lib/Target/AArch64/AArch64Subtarget.cpp
@@ -66,6 +66,7 @@
   // this in the future so we can specify it together with the subtarget
   // features.
   switch (ARMProcFamily) {
+  case Carmel:
   case Others:
 break;
   case CortexA35:
Index: llvm/lib/Target/AArch64/AArch64.td
===
--- llvm/lib/Target/AArch64/AArch64.td
+++ llvm/lib/Target/AArch64/AArch64.td
@@ -597,6 +597,15 @@
   

[PATCH] D77776: [Driver] Drop support for FreeBSD < 10

[Greg V via Phabricator via cfe-commits]

myfreeweb added a comment.

> But the default should indeed be either the host version

yep, I've had this patch:

  --- lib/Support/Triple.cpp.orig   2020-04-10 20:51:46 UTC
  +++ lib/Support/Triple.cpp
  @@ -14,6 +14,9 @@
   #include "llvm/Support/Host.h"
   #include "llvm/Support/TargetParser.h"
   #include 
  +#ifdef __FreeBSD__
  +#include 
  +#endif
   using namespace llvm;
   
   StringRef Triple::getArchTypeName(ArchType Kind) {
  @@ -1073,6 +1076,13 @@ void Triple::getOSVersion(unsigned &Major, unsigned &M
   OSName.consume_front("macos");
   
 parseVersionFromName(OSName, Major, Minor, Micro);
  +
  +  if (getOS() == FreeBSD && Major == 0)
  +#ifdef __FreeBSD__
  +   Major = __FreeBSD_version / 10;
  +#else
  +   Major = 12;
  +#endif
   }
   
   bool Triple::getMacOSXVersion(unsigned &Major, unsigned &Minor,

I don't see any other OS headers included in this file, so idk about code 
style, but the general idea should be like this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D6



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

[PATCH] D77334: [AVR] Remove duplicate specification of lib directory

[Dennis van der Schagt via Phabricator via cfe-commits]

dennisschagt added a comment.

Thanks!

Yes, can you commit it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77334



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

[PATCH] D77938: [clangd] Extend YAML Serialization

[Mark Nauwelaerts via Phabricator via cfe-commits]

mnauw created this revision.
mnauw added a reviewer: sammccall.
mnauw added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov, mgorny.
Herald added a project: clang.
mnauw added a comment.

This was previously part of D77385 .  Rather 
than a separate file (which seems a bit tricky), the test YAML has simply been 
included in the unit test itself.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77938

Files:
  clang-tools-extra/clangd/index/YAMLSerialization.cpp
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/YAMLTests.cpp

Index: clang-tools-extra/clangd/unittests/YAMLTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/YAMLTests.cpp
@@ -0,0 +1,244 @@
+//===-- YAMLTests.cpp - YAML container unit tests -===//
+//
+// 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 "index/Serialization.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+std::string YAMLData =
+R"|(--- !Symbol
+ID:  13C6B898A37B5E70
+Name:changedFilesTask
+Scope:   'clang::clangd::BackgroundIndex::'
+SymInfo:
+  Kind:InstanceMethod
+  Lang:Cpp
+CanonicalDeclaration:
+  FileURI: 'file:///llvm-project/clang-tools-extra/clangd/index/Background.h'
+  Start:
+Line:197
+Column:  2
+  End:
+Line:197
+Column:  18
+Definition:
+  FileURI: 'file:///llvm-project/clang-tools-extra/clangd/index/Background.cpp'
+  Start:
+Line:166
+Column:  39
+  End:
+Line:166
+Column:  55
+Origin:  4
+Flags:   8
+Signature:   ''
+TemplateSpecializationArgs: ''
+CompletionSnippetSuffix: ''
+Documentation:   ''
+ReturnType:  ''
+Type:''
+...
+--- !Refs
+ID:  FA89F28EEE2EDA4F
+References:
+  - Kind:4
+Location:
+  FileURI: 'file:///llvm-project/clang-tools-extra/clangd/index/Background.cpp'
+  Start:
+Line:408
+Column:  40
+  End:
+Line:408
+Column:  42
+  - Kind:4
+Location:
+  FileURI: 'file:///llvm-project/clang-tools-extra/clangd/index/Background.cpp'
+  Start:
+Line:409
+Column:  13
+  End:
+Line:409
+Column:  15
+  - Kind:4
+Location:
+  FileURI: 'file:///llvm-project/clang-tools-extra/clangd/index/Background.cpp'
+  Start:
+Line:412
+Column:  30
+  End:
+Line:412
+Column:  32
+  - Kind:4
+Location:
+  FileURI: 'file:///llvm-project/clang-tools-extra/clangd/index/Background.cpp'
+  Start:
+Line:415
+Column:  8
+  End:
+Line:415
+Column:  10
+  - Kind:4
+Location:
+  FileURI: 'file:///llvm-project/clang-tools-extra/clangd/index/Background.cpp'
+  Start:
+Line:428
+Column:  24
+  End:
+Line:428
+Column:  26
+  - Kind:4
+Location:
+  FileURI: 'file:///llvm-project/clang-tools-extra/clangd/index/Background.cpp'
+  Start:
+Line:429
+Column:  24
+  End:
+Line:429
+Column:  26
+  - Kind:4
+Location:
+  FileURI: 'file:///llvm-project/clang-tools-extra/clangd/index/Background.cpp'
+  Start:
+Line:460
+Column:  18
+  End:
+Line:460
+Column:  20
+  - Kind:4
+Location:
+  FileURI: 'file:///llvm-project/clang-tools-extra/clangd/index/Background.cpp'
+  Start:
+Line:461
+Column:  64
+  End:
+Line:461
+Column:  66
+  - Kind:4
+Location:
+  FileURI: 'file:///llvm-project/clang-tools-extra/clangd/index/Background.cpp'
+  Start:
+Line:463
+Column:  24
+  End:
+Line:463
+Column:  26
+  - Kind:4
+Location:
+  FileURI: 'file:///llvm-project/clang-tools-extra/clangd/index/Background.cpp'
+  Start

[PATCH] D77938: [clangd] Extend YAML Serialization

[Mark Nauwelaerts via Phabricator via cfe-commits]

mnauw added a comment.

This was previously part of D77385 .  Rather 
than a separate file (which seems a bit tricky), the test YAML has simply been 
included in the unit test itself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77938



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

[PATCH] D77847: [clangd] Rebuild dependent files when a header is saved.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 256756.
sammccall marked an inline comment as done.
sammccall added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77847

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -126,6 +126,27 @@
   EXPECT_THAT(Client.diagnostics("foo.cpp"), llvm::ValueIs(testing::IsEmpty()));
 }
 
+TEST_F(LSPTest, DiagnosticsHeaderSaved) {
+  auto &Client = start();
+  FS.Files["foo.h"] = "#define VAR original";
+  Client.didOpen("foo.cpp", R"cpp(
+#include "foo.h"
+int x = VAR;
+  )cpp");
+  EXPECT_THAT(Client.diagnostics("foo.cpp"),
+  llvm::ValueIs(testing::ElementsAre(
+  DiagMessage("Use of undeclared identifier 'original'";
+  // Now modify the header from within the "editor".
+  FS.Files["foo.h"] = "#define VAR changed";
+  Client.notify(
+  "textDocument/didSave",
+  llvm::json::Object{{"textDocument", Client.documentID("foo.h")}});
+  // Foo.cpp should be rebuilt with new diagnostics.
+  EXPECT_THAT(Client.diagnostics("foo.cpp"),
+  llvm::ValueIs(testing::ElementsAre(
+  DiagMessage("Use of undeclared identifier 'changed'";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -56,7 +56,11 @@
 # CHECK-NEXT:  ","
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "textDocumentSync": 2,
+# CHECK-NEXT:  "textDocumentSync": {
+# CHECK-NEXT:"change": 2,
+# CHECK-NEXT:"openClose": true,
+# CHECK-NEXT:"save": true
+# CHECK-NEXT:  },
 # CHECK-NEXT:  "typeHierarchyProvider": true
 # CHECK-NEXT:  "workspaceSymbolProvider": true
 # CHECK-NEXT:},
Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -653,6 +653,12 @@
 };
 bool fromJSON(const llvm::json::Value &, DidCloseTextDocumentParams &);
 
+struct DidSaveTextDocumentParams {
+  /// The document that was saved.
+  TextDocumentIdentifier textDocument;
+};
+bool fromJSON(const llvm::json::Value &, DidSaveTextDocumentParams &);
+
 struct TextDocumentContentChangeEvent {
   /// The range of the document that changed.
   llvm::Optional range;
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -452,6 +452,11 @@
   return O && O.map("textDocument", R.textDocument);
 }
 
+bool fromJSON(const llvm::json::Value &Params, DidSaveTextDocumentParams &R) {
+  llvm::json::ObjectMapper O(Params);
+  return O && O.map("textDocument", R.textDocument);
+}
+
 bool fromJSON(const llvm::json::Value &Params, DidChangeTextDocumentParams &R) {
   llvm::json::ObjectMapper O(Params);
   if (!O)
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -75,6 +75,7 @@
   void onDocumentDidOpen(const DidOpenTextDocumentParams &);
   void onDocumentDidChange(const DidChangeTextDocumentParams &);
   void onDocumentDidClose(const DidCloseTextDocumentParams &);
+  void onDocumentDidSave(const DidSaveTextDocumentParams &);
   void onDocumentOnTypeFormatting(const DocumentOnTypeFormattingParams &,
   Callback>);
   void onDocumentRangeFormatting(const DocumentRangeFormattingParams &,
@@ -131,10 +132,12 @@
   /// produce '->' and '::', respectively.
   bool shouldRunCompletion(const CompletionParams &Params) const;
 
-  /// Forces a reparse of all currently opened files which were modified. As a
-  /// result, this method may be very expensive. This method is normally called
-  /// when the compilation database is changed.
-  void reparseOpenedFiles(const llvm::StringSet<> &ModifiedFiles);
+  /// Requests a reparse of currently opened files using their latest source.
+  /// This will typically only rebuild if something other than the source has
+  /// changed (e.g. the CDB yields differen

[PATCH] D77847: [clangd] Rebuild dependent files when a header is saved.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 3 inline comments as done.
sammccall added a comment.

In D77847#1974126 , @kadircet wrote:

> should we also have a unittest for checking ast caching works as expected?


TUSchedulerTests has `NoopOnEmptyChanges` which I think tests exactly this, and 
`EvictedAST` which checks it's sensitive to the cache.

I don't think testing it again indirectly at this level makes is a good 
tradeoff, especially if we need subtle assertions via the memory usage.




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:575
+ {"change", (int)TextDocumentSyncKind::Incremental},
+ {"save", true},
+ }},

kadircet wrote:
> spec also specifies this as `property name (optional): 
> textDocumentSync.didSave` near didSave notification (in addition to defining 
> it as `save` in the struct). :(
> 
> it also says textDocumentSync can also be a number for backward compat 
> reasons, but doesn't say when the struct was added. I hope it is not recent 
> and we don't end up breaking clients with our capab response :/
> 
> no action needed just complaining ...
> spec also specifies this as property name (optional): textDocumentSync.didSave

Good catch. VSCode and every other impl I could find implement `save` though, 
so I think that's just a spec bug. Sent 
https://github.com/microsoft/language-server-protocol/pull/958

(We could set both here, but I don't think it's a good idea to muddy the water)

> it also says textDocumentSync can also be a number for backward compat 
> reasons, but doesn't say when the struct was added. I hope it is not recent 
> and we don't end up breaking clients with our capab response :/

yeah. I hadn't really internalized that all the nice compatibility hacks in the 
spec are for servers, and don't help much with clients.
I think this is pretty old though, and there's not much we can do about it 
anyway.

> no action needed just complaining ...

Ack, and agreed :-)



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77847



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

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

[Ten Tzen via Phabricator via cfe-commits]

tentzen created this revision.
tentzen added reviewers: rnk, eli.friedman, JosephTremoulet, asmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Per Windows SEH Spec, except _leave, all other early exits of a _try 
(goto/return/continue/break) are considered abnormal exits.  In those cases, 
the first parameter passes to its _finally funclet should be TRUE to indicate 
an abnormal-termination.

One way to implement abnormal exits in _try is to invoke Windows runtime 
_local_unwind() (MSVC approach) that will invoke _dtor funclet where 
abnormal-termination flag is always TRUE when calling _finally.  Obviously this 
approach is less optimal and is complicated to implement in Clang.

Clang today has a NormalCleanupDestSlot mechanism to dispatch multiple exits at 
the end of _try.  Since  _leave (or try-end fall-through) is always Indexed 
with 0 in that NormalCleanupDestSlot,  this fix takes the advantage of that 
mechanism and just passes NormalCleanupDest ID as 1st Arg to _finally.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77936

Files:
  clang/lib/CodeGen/CGCleanup.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/EHScopeStack.h


Index: clang/lib/CodeGen/EHScopeStack.h
===
--- clang/lib/CodeGen/EHScopeStack.h
+++ clang/lib/CodeGen/EHScopeStack.h
@@ -160,7 +160,8 @@
   enum {
 F_IsForEH = 0x1,
 F_IsNormalCleanupKind = 0x2,
-F_IsEHCleanupKind = 0x4
+F_IsEHCleanupKind = 0x4,
+F_HasSehAbnormalExits = 0x8
   };
   unsigned flags;
 
@@ -179,6 +180,9 @@
   /// cleanup.
   bool isEHCleanupKind() const { return flags & F_IsEHCleanupKind; }
   void setIsEHCleanupKind() { flags |= F_IsEHCleanupKind; }
+
+  bool hasSehAbnormalExits() const { return flags & F_HasSehAbnormalExits; 
}
+  void setHasSehAbnormalExits() { flags |= F_HasSehAbnormalExits; }
 };
 
 
Index: clang/lib/CodeGen/CGException.cpp
===
--- clang/lib/CodeGen/CGException.cpp
+++ clang/lib/CodeGen/CGException.cpp
@@ -1639,6 +1639,18 @@
 
 llvm::Value *IsForEH =
 llvm::ConstantInt::get(CGF.ConvertType(ArgTys[0]), F.isForEHCleanup());
+
+// Except _leave and fall-through at the end, all other exits in a _try
+//   (return/goto/continue/break) are considered as abnormal terminations
+//   since _leave/fall-through is always Indexed 0,
+//   just use NormalCleanupDestSlot (>= 1 for goto/return/..),
+//   as 1st Arg to indicate abnormal termination
+if (!F.isForEHCleanup() && F.hasSehAbnormalExits()) {
+  Address Addr = CGF.getNormalCleanupDestSlot();
+  llvm::Value* Load = CGF.Builder.CreateLoad(Addr, "cleanup.dest");
+  IsForEH = CGF.Builder.CreateTrunc(Load, CGM.Int8Ty);
+}
+
 Args.add(RValue::get(IsForEH), ArgTys[0]);
 Args.add(RValue::get(FP), ArgTys[1]);
 
Index: clang/lib/CodeGen/CGCleanup.cpp
===
--- clang/lib/CodeGen/CGCleanup.cpp
+++ clang/lib/CodeGen/CGCleanup.cpp
@@ -860,6 +860,9 @@
 // TODO: base this on the number of branch-afters and fixups
 const unsigned SwitchCapacity = 10;
 
+// pass the abnormal exit flag to Fn (SEH cleanup)
+cleanupFlags.setHasSehAbnormalExits();
+
 llvm::LoadInst *Load =
   createLoadInstBefore(getNormalCleanupDestSlot(), "cleanup.dest",
nullptr);


Index: clang/lib/CodeGen/EHScopeStack.h
===
--- clang/lib/CodeGen/EHScopeStack.h
+++ clang/lib/CodeGen/EHScopeStack.h
@@ -160,7 +160,8 @@
   enum {
 F_IsForEH = 0x1,
 F_IsNormalCleanupKind = 0x2,
-F_IsEHCleanupKind = 0x4
+F_IsEHCleanupKind = 0x4,
+F_HasSehAbnormalExits = 0x8
   };
   unsigned flags;
 
@@ -179,6 +180,9 @@
   /// cleanup.
   bool isEHCleanupKind() const { return flags & F_IsEHCleanupKind; }
   void setIsEHCleanupKind() { flags |= F_IsEHCleanupKind; }
+
+  bool hasSehAbnormalExits() const { return flags & F_HasSehAbnormalExits; }
+  void setHasSehAbnormalExits() { flags |= F_HasSehAbnormalExits; }
 };
 
 
Index: clang/lib/CodeGen/CGException.cpp
===
--- clang/lib/CodeGen/CGException.cpp
+++ clang/lib/CodeGen/CGException.cpp
@@ -1639,6 +1639,18 @@
 
 llvm::Value *IsForEH =
 llvm::ConstantInt::get(CGF.ConvertType(ArgTys[0]), F.isForEHCleanup());
+
+// Except _leave and fall-through at the end, all other exits in a _try
+//   (return/goto/continue/break) are considered as abnormal terminations
+//   since _leave/fall-through is always Indexed 0,
+//   just use NormalCleanupDestSlot (>= 1 for goto/return/..),
+//   as 1st Arg to indi

[PATCH] D77334: [AVR] Remove duplicate specification of lib directory

[Dylan McKay via Phabricator via cfe-commits]

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

Nice catch, cheers.

Do you need someone to commit this for you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77334



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