date:20200326

Re: [clang] 857bf5d - [FIX] Do not copy an llvm::function_ref if it has to be reused

2020-03-26 Thread Arthur O'Dwyer via cfe-commits

On Thu, Mar 26, 2020 at 11:49 PM Richard Smith 
wrote:

> On Thu, 26 Mar 2020 at 17:07, David Blaikie via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> On Thu, Mar 26, 2020 at 3:12 PM Arthur O'Dwyer 
>> wrote:
>>
>>> I'm not sure, but I do see that the call stack contains a call to
>>>
>>> bool llvm::function_ref>> bool)>::callback_fn 
>>> const>(long, clang::Expr*&, bool)
>>>
>>> Notice that this is function_ref::callback_fn instantiated with
>>> T=function_ref itself; i.e., we do have a function_ref to a function_ref.
>>> This is the thing I was saying can happen (in my "lamb/sheep" example) but
>>> which I thought should never happen in this codepath.
>>> Here's function_ref's copy constructor:
>>>
>>>
>>>   template 
>>>
>>>   function_ref(Callable &,
>>>
>>>
>>> std::enable_if_t,
>>>
>>>   function_ref>::value> * =
>>> nullptr)
>>>
>>>   : callback(callback_fn>> std::remove_reference::type>),
>>>
>>> callable(reinterpret_cast()) {}
>>>
>>>
>>> I believe that it should be using std::remove_cvref_t, not
>>> std::remove_reference_t, so as not to conflict with the compiler's
>>> implicitly generated copy constructor.
>>>
>>> Thoughts? [...]
>>>
>>
> OK, so: we're calling the wrong constructor for the inner capture due to
> the implicit 'const' that's added because the outer lambda is not mutable
> (and the fix suggested by Arthur is the right one: we should be using
> remove_cvref_t here not remove_reference_t -- or rather
> std::remove_cv_t>, since we don't require
> C++20 yet). And this means that copying a function_ref from a *const*
> function_ref gives you a new function_ref that refers to the old one, not a
> new function_ref that refers to the same function the old one did.
>

Argh! That is insanely sneaky, and it is quite probable IMHO that this is a
core-language bug in GCC, because GCC behaves differently from EDG and
Clang here.
https://godbolt.org/z/oCvLpv
On GCC, when you have a lambda nested within a lambda, and you
capture-by-copy a variable which was captured-by-copy using [=] or [i] (but
not C++14 init-capture [i=i]), then your data member for that capture will
have type `const I`, not just `I`.
On Clang and EDG, [=], [i], and [i=i] all behave equivalently, and capture
a data member with type `I`.

I don't see anything about this on
https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=lambda%20capture%20const
— Richard, you might be best able to describe the issue correctly ;) but if
you want me to file it, I will.  (Might be a corollary of bug 86697
.)

Regardless, llvm::function_ref's SFINAE should still be fixed. This is a
bug in llvm::function_ref, exposed by a bug in GCC. (Or vice versa.)

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

Re: [clang] 857bf5d - [FIX] Do not copy an llvm::function_ref if it has to be reused

2020-03-26 Thread Richard Smith via cfe-commits

On Thu, 26 Mar 2020 at 17:07, David Blaikie via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On Thu, Mar 26, 2020 at 3:12 PM Arthur O'Dwyer 
> wrote:
>
>> I'm not sure, but I do see that the call stack contains a call to
>>
>> bool llvm::function_ref> bool)>::callback_fn 
>> const>(long, clang::Expr*&, bool)
>>
>> Notice that this is function_ref::callback_fn instantiated with
>> T=function_ref itself; i.e., we do have a function_ref to a function_ref.
>> This is the thing I was saying can happen (in my "lamb/sheep" example) but
>> which I thought should never happen in this codepath.
>> Here's function_ref's copy constructor:
>>
>>
>>   template 
>>
>>   function_ref(Callable &,
>>
>>
>> std::enable_if_t,
>>
>>   function_ref>::value> * =
>> nullptr)
>>
>>   : callback(callback_fn> std::remove_reference::type>),
>>
>> callable(reinterpret_cast()) {}
>>
>>
>> I believe that it should be using std::remove_cvref_t, not
>> std::remove_reference_t, so as not to conflict with the compiler's
>> implicitly generated copy constructor.
>>
>> Thoughts?
>>
>
> Yep, looks like you're on to something here - I still don't understand why
> calling that function rather than the real trivial copy ctor would be
> problematic,
>

OK, so: we're calling the wrong constructor for the inner capture due to
the implicit 'const' that's added because the outer lambda is not mutable
(and the fix suggested by Arthur is the right one: we should be using
remove_cvref_t here not remove_reference_t -- or rather
std::remove_cv_t>, since we don't require
C++20 yet). And this means that copying a function_ref from a *const*
function_ref gives you a new function_ref that refers to the old one, not a
new function_ref that refers to the same function the old one did. In
particular, this happens when copying a lambda that captures a function_ref
by value.

The next part of the puzzle is that llvm::any_of takes its callback by
value and passes it by value to std::any_of. And inside any_of, in
libstdc++, the predicate is passed through __pred_iter (
https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/bits/predefined_ops.h#L323)
before being used. That results in building a function_ref referring to
part of the function parameter in __pred_iter, whose lifetime ends before
we call it.

With libc++, the predicate is not copied around inside any_of, which is why
this was only failing on some buildbots.

but I managed to reproduce this locally with GCC 7.5 (seems to be an issue
> with the 7 series - the buildbot used 7.3) & if I capture by value in both
> outer and inner lambdas it reproduces, but if I mark the outer lambda as
> mutable it succeeds (because this would remove the const & not trip over
> the SFINAE issue you pointed out)...
>
> Investigation continues.
>
> - Dave
>
>
>> –Arthur
>>
>>
>>
>> On Thu, Mar 26, 2020 at 5:08 PM David Blaikie  wrote:
>>
>>> Hey Richard - could you help try to diagnose what's happening here?
>>>
>>> I reverted this patch in:
>>> https://github.com/llvm/llvm-project/commit/0d0b90105f92f6cd9cc7004d565834f4429183fb
>>> But that did actually cause buildbot failures, such as these ones:
>>> http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/24491
>>>   eg:
>>> http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/24491/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Adeclare_variant_mixed_codegen.cpp
>>> I "fixed" these failures blind by reapplying part of this original
>>> commit (the lambda capture by reference rather than by value):
>>> https://github.com/llvm/llvm-project/commit/2ec59a0a40f4ec02e6b2dbe5f12261959c191aa9
>>>
>>> I've stared at this a fair bit and can't spot any undefined behavior,
>>> but I guess it's probably in there somewhere - and I'm worried that this
>>> fix is blind, not fully justified & might be hiding some latent issues.
>>>
>>> The full buildbot example failure quoted here in case it times out/gets
>>> deleted on the buildbot itself:
>>>
>>>  TEST 'Clang ::
>>> OpenMP/declare_variant_mixed_codegen.cpp' FAILED 
>>> Script:
>>> --
>>> : 'RUN: at line 1';
>>> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/clang
>>> -cc1 -internal-isystem
>>> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/lib/clang/11.0.0/include
>>> -nostdsysteminc -verify -fopenmp -x c++ -triple x86_64-unknown-linux
>>> -emit-llvm
>>> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
>>> -fexceptions -fcxx-exceptions -o - -fsanitize-address-use-after-scope |
>>> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/FileCheck
>>> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
>>> : 'RUN: at line 2';
>>>

Re: [clang] d264f02 - Fix `-Wreturn-type` warning. NFC.

2020-03-26 Thread Michael LIAO via cfe-commits

Thanks for catching that.

On Thu, Mar 26, 2020 at 11:14 PM David Blaikie  wrote:
>
> Usually this sort of thing is addressed with llvm_unreachable, rather than a 
> return statement that's not expected to be reached by any valid execution of 
> LLVM (it'd require a carefully hand-crafted CPU kind to reach that return 
> (since all the actual enumerators result in returns earlier, in the switch 
> statement above), which probably isn't an intended code path?)
>
> I've made that change in 819e540208d5d62e7841d0dbdef3580eecc2c2b6
>
> On Wed, Mar 25, 2020 at 9:59 PM Michael Liao via cfe-commits 
>  wrote:
>>
>>
>> Author: Michael Liao
>> Date: 2020-03-26T00:53:24-04:00
>> New Revision: d264f02c6f502960e2bcdd332f250efc702d09f2
>>
>> URL: 
>> https://github.com/llvm/llvm-project/commit/d264f02c6f502960e2bcdd332f250efc702d09f2
>> DIFF: 
>> https://github.com/llvm/llvm-project/commit/d264f02c6f502960e2bcdd332f250efc702d09f2.diff
>>
>> LOG: Fix `-Wreturn-type` warning. NFC.
>>
>> Added:
>>
>>
>> Modified:
>> clang/lib/Basic/Targets/X86.cpp
>>
>> Removed:
>>
>>
>>
>> 
>> diff  --git a/clang/lib/Basic/Targets/X86.cpp 
>> b/clang/lib/Basic/Targets/X86.cpp
>> index f35b520de657..8a7d0f17760e 100644
>> --- a/clang/lib/Basic/Targets/X86.cpp
>> +++ b/clang/lib/Basic/Targets/X86.cpp
>> @@ -1842,6 +1842,7 @@ Optional 
>> X86TargetInfo::getCPUCacheLineSize() const {
>>  case CK_Generic:
>>return None;
>>}
>> +  return None;
>>  }
>>
>>  bool X86TargetInfo::validateOutputSize(const llvm::StringMap 
>> ,
>>
>>
>>
>> ___
>> 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

[clang] 819e540 - Use llvm_unreachable after a fully covered/always-returning switch

2020-03-26 Thread David Blaikie via cfe-commits


Author: David Blaikie
Date: 2020-03-26T20:09:57-07:00
New Revision: 819e540208d5d62e7841d0dbdef3580eecc2c2b6

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

LOG: Use llvm_unreachable after a fully covered/always-returning switch

Added: 


Modified: 
clang/lib/Basic/Targets/X86.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index 8a7d0f17760e..f06bf84cfd99 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -1842,7 +1842,7 @@ Optional X86TargetInfo::getCPUCacheLineSize() 
const {
 case CK_Generic:
   return None;
   }
-  return None;
+  llvm_unreachable("Unknown CPU kind");
 }
 
 bool X86TargetInfo::validateOutputSize(const llvm::StringMap ,



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

Re: [clang] d264f02 - Fix `-Wreturn-type` warning. NFC.

2020-03-26 Thread David Blaikie via cfe-commits

Usually this sort of thing is addressed with llvm_unreachable, rather than
a return statement that's not expected to be reached by any valid execution
of LLVM (it'd require a carefully hand-crafted CPU kind to reach that
return (since all the actual enumerators result in returns earlier, in the
switch statement above), which probably isn't an intended code path?)

I've made that change in 819e540208d5d62e7841d0dbdef3580eecc2c2b6

On Wed, Mar 25, 2020 at 9:59 PM Michael Liao via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Michael Liao
> Date: 2020-03-26T00:53:24-04:00
> New Revision: d264f02c6f502960e2bcdd332f250efc702d09f2
>
> URL:
> https://github.com/llvm/llvm-project/commit/d264f02c6f502960e2bcdd332f250efc702d09f2
> DIFF:
> https://github.com/llvm/llvm-project/commit/d264f02c6f502960e2bcdd332f250efc702d09f2.diff
>
> LOG: Fix `-Wreturn-type` warning. NFC.
>
> Added:
>
>
> Modified:
> clang/lib/Basic/Targets/X86.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/lib/Basic/Targets/X86.cpp
> b/clang/lib/Basic/Targets/X86.cpp
> index f35b520de657..8a7d0f17760e 100644
> --- a/clang/lib/Basic/Targets/X86.cpp
> +++ b/clang/lib/Basic/Targets/X86.cpp
> @@ -1842,6 +1842,7 @@ Optional
> X86TargetInfo::getCPUCacheLineSize() const {
>  case CK_Generic:
>return None;
>}
> +  return None;
>  }
>
>  bool X86TargetInfo::validateOutputSize(const llvm::StringMap
> ,
>
>
>
> ___
> 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] D76757: Fix typo, targetFeature should be lowercase.

2020-03-26 Thread Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG64fe84185602: Fix typo, targetFeature should be lowercase. 
(authored by khchen, committed by Zakk Chen zakk.c...@sifive.com).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76757

Files:
  clang/tools/driver/cc1_main.cpp
  llvm/lib/MC/MCSubtargetInfo.cpp


Index: llvm/lib/MC/MCSubtargetInfo.cpp
===
--- llvm/lib/MC/MCSubtargetInfo.cpp
+++ llvm/lib/MC/MCSubtargetInfo.cpp
@@ -185,7 +185,7 @@
 // Check for help
 if (Feature == "+help")
   Help(ProcDesc, ProcFeatures);
-else if (Feature == "+cpuHelp")
+else if (Feature == "+cpuhelp")
   cpuHelp(ProcDesc);
 else
   ApplyFeatureFlag(Bits, Feature, ProcFeatures);
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -177,7 +177,7 @@
   // the target machine will handle the mcpu printing
   llvm::TargetOptions Options;
   std::unique_ptr TheTargetMachine(
-  TheTarget->createTargetMachine(TargetStr, "", "+cpuHelp", Options, 
None));
+  TheTarget->createTargetMachine(TargetStr, "", "+cpuhelp", Options, 
None));
   return 0;
 }
 


Index: llvm/lib/MC/MCSubtargetInfo.cpp
===
--- llvm/lib/MC/MCSubtargetInfo.cpp
+++ llvm/lib/MC/MCSubtargetInfo.cpp
@@ -185,7 +185,7 @@
 // Check for help
 if (Feature == "+help")
   Help(ProcDesc, ProcFeatures);
-else if (Feature == "+cpuHelp")
+else if (Feature == "+cpuhelp")
   cpuHelp(ProcDesc);
 else
   ApplyFeatureFlag(Bits, Feature, ProcFeatures);
Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -177,7 +177,7 @@
   // the target machine will handle the mcpu printing
   llvm::TargetOptions Options;
   std::unique_ptr TheTargetMachine(
-  TheTarget->createTargetMachine(TargetStr, "", "+cpuHelp", Options, None));
+  TheTarget->createTargetMachine(TargetStr, "", "+cpuhelp", Options, None));
   return 0;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 64fe841 - Fix typo, targetFeature should be lowercase.

2020-03-26 Thread Zakk Chen via cfe-commits


Author: Zakk Chen
Date: 2020-03-26T19:40:04-07:00
New Revision: 64fe84185602b59c2e07c142b9772c6e855153cb

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

LOG: Fix typo, targetFeature should be lowercase.

this fixing also enable llc -mattr=+cpuhelp

Reviewers: ziangwan, kongyi

Reviewed By: kongyi

Tags: #llvm

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

Added: 


Modified: 
clang/tools/driver/cc1_main.cpp
llvm/lib/MC/MCSubtargetInfo.cpp

Removed: 




diff  --git a/clang/tools/driver/cc1_main.cpp b/clang/tools/driver/cc1_main.cpp
index d8c22f21706b..33477852d285 100644
--- a/clang/tools/driver/cc1_main.cpp
+++ b/clang/tools/driver/cc1_main.cpp
@@ -177,7 +177,7 @@ static int PrintSupportedCPUs(std::string TargetStr) {
   // the target machine will handle the mcpu printing
   llvm::TargetOptions Options;
   std::unique_ptr TheTargetMachine(
-  TheTarget->createTargetMachine(TargetStr, "", "+cpuHelp", Options, 
None));
+  TheTarget->createTargetMachine(TargetStr, "", "+cpuhelp", Options, 
None));
   return 0;
 }
 

diff  --git a/llvm/lib/MC/MCSubtargetInfo.cpp b/llvm/lib/MC/MCSubtargetInfo.cpp
index 7d38cd0f729b..ac4f590d6cf3 100644
--- a/llvm/lib/MC/MCSubtargetInfo.cpp
+++ b/llvm/lib/MC/MCSubtargetInfo.cpp
@@ -185,7 +185,7 @@ static FeatureBitset getFeatures(StringRef CPU, StringRef 
FS,
 // Check for help
 if (Feature == "+help")
   Help(ProcDesc, ProcFeatures);
-else if (Feature == "+cpuHelp")
+else if (Feature == "+cpuhelp")
   cpuHelp(ProcDesc);
 else
   ApplyFeatureFlag(Bits, Feature, ProcFeatures);



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

[PATCH] D75779: [OpenMP] `omp begin/end declare variant` - part 2, sema (+"CG")

2020-03-26 Thread Hal Finkel via Phabricator via cfe-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75779



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

[PATCH] D76896: Color dependent names based on their heuristic target if they have one

2020-03-26 Thread Nathan Ridge via Phabricator via cfe-commits

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

Fixes https://github.com/clangd/clangd/issues/297


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76896

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -654,6 +654,20 @@
 static const int $StaticField[[Value]] = $TemplateParameter[[T]]
 ::$DependentType[[Resolver]]::$DependentName[[Value]];
   };
+)cpp",
+  // Dependent name with heuristic target
+  R"cpp(
+  template 
+  struct $Class[[Foo]] {
+int $Field[[Waldo]];
+void $Method[[bar]]() {
+  $Class[[Foo]]().$Field[[Waldo]];
+}
+template 
+void $Method[[bar1]]() {
+  $Class[[Foo]]<$TemplateParameter[[U]]>().$Field[[Waldo]];
+}
+  };
 )cpp",
   // Concepts
   R"cpp(
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -829,11 +829,11 @@
 "0: targets = {x}, decl\n"
 "1: targets = {vector}\n"
 "2: targets = {x}\n"},
-   // Handle UnresolvedLookupExpr.
-   // FIXME
-   // This case fails when expensive checks are enabled.
-   // Seems like the order of ns1::func and ns2::func isn't defined.
-   #ifndef EXPENSIVE_CHECKS
+// Handle UnresolvedLookupExpr.
+// FIXME
+// This case fails when expensive checks are enabled.
+// Seems like the order of ns1::func and ns2::func isn't defined.
+#ifndef EXPENSIVE_CHECKS
{R"cpp(
 namespace ns1 { void func(char*); }
 namespace ns2 { void func(int*); }
@@ -847,7 +847,7 @@
 )cpp",
 "0: targets = {ns1::func, ns2::func}\n"
 "1: targets = {t}\n"},
-#endif
+#endif
// Handle UnresolvedMemberExpr.
{R"cpp(
 struct X {
@@ -1134,7 +1134,7 @@
   namespace foo {
 template  requires $1^Drawable<$2^T>
 void $3^bar($4^T $5^t) {
-  $6^t.draw();
+  $6^t.$7^draw();
 }
   }
   )cpp",
@@ -1144,7 +1144,8 @@
"3: targets = {foo::bar}, decl\n"
"4: targets = {T}\n"
"5: targets = {t}, decl\n"
-   "6: targets = {t}\n"},
+   "6: targets = {t}\n"
+   "7: targets = {}\n"},
// Objective-C: properties
{
R"cpp(
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -143,6 +143,27 @@
   return getHighlightableSpellingToken(SM.getImmediateSpellingLoc(L), SM);
 }
 
+enum class TokenQuality { Resolved, Heuristic };
+
+TokenQuality evaluateTokenQuality(HighlightingKind Kind) {
+  return Kind == HighlightingKind::DependentType ||
+ Kind == HighlightingKind::DependentName
+ ? TokenQuality::Heuristic
+ : TokenQuality::Resolved;
+}
+
+llvm::Optional
+resolveConflict(ArrayRef Tokens) {
+  if (Tokens.size() != 2)
+return llvm::None;
+
+  TokenQuality Quality1 = evaluateTokenQuality(Tokens[0].Kind);
+  TokenQuality Quality2 = evaluateTokenQuality(Tokens[1].Kind);
+  if (Quality1 == Quality2)
+return llvm::None;
+  return Quality1 == TokenQuality::Resolved ? Tokens[0] : Tokens[1];
+}
+
 /// Consumes source locations and maps them to text ranges for highlightings.
 class HighlightingsBuilder {
 public:
@@ -187,6 +208,8 @@
   // should be in the highlightings.
   if (Conflicting.size() == 1)
 NonConflicting.push_back(TokRef.front());
+  else if (auto Resolved = resolveConflict(Conflicting))
+NonConflicting.push_back(*Resolved);
   // TokRef[Conflicting.size()] is the next token with a different range (or
   // the end of the Tokens).
   TokRef = TokRef.drop_front(Conflicting.size());
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -646,6 +646,15 @@
   /*IsDecl=*/false,
   {E->getNamedConcept()}});
 }
+
+void
+

Re: [clang] 857bf5d - [FIX] Do not copy an llvm::function_ref if it has to be reused

2020-03-26 Thread David Blaikie via cfe-commits

On Thu, Mar 26, 2020 at 3:12 PM Arthur O'Dwyer 
wrote:

> I'm not sure, but I do see that the call stack contains a call to
>
> bool llvm::function_ref bool)>::callback_fn 
> const>(long, clang::Expr*&, bool)
>
> Notice that this is function_ref::callback_fn instantiated with
> T=function_ref itself; i.e., we do have a function_ref to a function_ref.
> This is the thing I was saying can happen (in my "lamb/sheep" example) but
> which I thought should never happen in this codepath.
> Here's function_ref's copy constructor:
>
>
>   template 
>
>   function_ref(Callable &,
>
>
> std::enable_if_t,
>
>   function_ref>::value> * =
> nullptr)
>
>   : callback(callback_fn std::remove_reference::type>),
>
> callable(reinterpret_cast()) {}
>
>
> I believe that it should be using std::remove_cvref_t, not
> std::remove_reference_t, so as not to conflict with the compiler's
> implicitly generated copy constructor.
>
> Thoughts?
>

Yep, looks like you're on to something here - I still don't understand why
calling that function rather than the real trivial copy ctor would be
problematic, but I managed to reproduce this locally with GCC 7.5 (seems to
be an issue with the 7 series - the buildbot used 7.3) & if I capture by
value in both outer and inner lambdas it reproduces, but if I mark the
outer lambda as mutable it succeeds (because this would remove the const &
not trip over the SFINAE issue you pointed out)...

Investigation continues.

- Dave


> –Arthur
>
>
>
> On Thu, Mar 26, 2020 at 5:08 PM David Blaikie  wrote:
>
>> Hey Richard - could you help try to diagnose what's happening here?
>>
>> I reverted this patch in:
>> https://github.com/llvm/llvm-project/commit/0d0b90105f92f6cd9cc7004d565834f4429183fb
>> But that did actually cause buildbot failures, such as these ones:
>> http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/24491
>>   eg:
>> http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/24491/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Adeclare_variant_mixed_codegen.cpp
>> I "fixed" these failures blind by reapplying part of this original commit
>> (the lambda capture by reference rather than by value):
>> https://github.com/llvm/llvm-project/commit/2ec59a0a40f4ec02e6b2dbe5f12261959c191aa9
>>
>> I've stared at this a fair bit and can't spot any undefined behavior, but
>> I guess it's probably in there somewhere - and I'm worried that this fix is
>> blind, not fully justified & might be hiding some latent issues.
>>
>> The full buildbot example failure quoted here in case it times out/gets
>> deleted on the buildbot itself:
>>
>>  TEST 'Clang ::
>> OpenMP/declare_variant_mixed_codegen.cpp' FAILED 
>> Script:
>> --
>> : 'RUN: at line 1';
>> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/clang
>> -cc1 -internal-isystem
>> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/lib/clang/11.0.0/include
>> -nostdsysteminc -verify -fopenmp -x c++ -triple x86_64-unknown-linux
>> -emit-llvm
>> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
>> -fexceptions -fcxx-exceptions -o - -fsanitize-address-use-after-scope |
>> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/FileCheck
>> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
>> : 'RUN: at line 2';
>> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/clang
>> -cc1 -internal-isystem
>> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/lib/clang/11.0.0/include
>> -nostdsysteminc -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-linux
>> -fexceptions -fcxx-exceptions -emit-pch -o
>> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/tools/clang/test/OpenMP/Output/declare_variant_mixed_codegen.cpp.tmp
>> -fopenmp-version=50
>> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
>> : 'RUN: at line 3';
>> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/clang
>> -cc1 -internal-isystem
>> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/lib/clang/11.0.0/include
>> -nostdsysteminc -fopenmp -x c++ -triple x86_64-unknown-linux -fexceptions
>> -fcxx-exceptions -std=c++11 -include-pch
>> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/tools/clang/test/OpenMP/Output/declare_variant_mixed_codegen.cpp.tmp
>> -verify
>> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
>> -emit-llvm -o - -fopenmp-version=50 |
>>

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

2020-03-26 Thread Nathan Ridge via Phabricator via cfe-commits

nridge requested review of this revision.
nridge added a comment.

Thanks for the suggested simplification. As the change is not completely 
trivial, could you have one more look before landing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75286



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

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

2020-03-26 Thread Nathan Ridge via Phabricator via cfe-commits

nridge updated this revision to Diff 253005.
nridge added a comment.

Finish an incomplete variable extraction


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75286

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -61,9 +61,7 @@
  arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
 }
 
-MATCHER_P(FixMessage, Message, "") {
-  return arg.Message == Message;
-}
+MATCHER_P(FixMessage, Message, "") { return arg.Message == Message; }
 
 MATCHER_P(EqualToLSPDiag, LSPDiag,
   "LSP diagnostic " + llvm::to_string(LSPDiag)) {
@@ -258,13 +256,13 @@
   Diag(Test.range("macroarg"),
"multiple unsequenced modifications to 'y'"),
   AllOf(
-Diag(Test.range("main"),
- "use a trailing return type for this function"),
-DiagSource(Diag::ClangTidy),
-DiagName("modernize-use-trailing-return-type"),
-// Verify that we don't have "[check-name]" suffix in the message.
-WithFix(FixMessage("use a trailing return type for this function")))
-  ));
+  Diag(Test.range("main"),
+   "use a trailing return type for this function"),
+  DiagSource(Diag::ClangTidy),
+  DiagName("modernize-use-trailing-return-type"),
+  // Verify that we don't have "[check-name]" suffix in the message.
+  WithFix(FixMessage(
+  "use a trailing return type for this function");
 }
 
 TEST(DiagnosticTest, ClangTidySuppressionComment) {
@@ -274,7 +272,11 @@
   double d = 8 / i;  // NOLINT
   // NOLINTNEXTLINE
   double e = 8 / i;
-  double f = [[8]] / i;
+  #define BAD 8 / i
+  double f = BAD;  // NOLINT
+  double g = [[8]] / i;
+  #define BAD2 BAD
+  double h = BAD2;  // NOLINT
 }
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());
@@ -836,8 +838,9 @@
   auto Index = buildIndexWithSymbol({});
   TU.ExternalIndex = Index.get();
 
-  EXPECT_THAT(TU.build().getDiagnostics(),
-  ElementsAre(Diag(Test.range(), "use of undeclared identifier 'a'")));
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  ElementsAre(Diag(Test.range(), "use of undeclared identifier 'a'")));
 }
 
 TEST(DiagsInHeaders, DiagInsideHeader) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -310,14 +310,13 @@
   // Check for suppression comment. Skip the check for diagnostics not
   // in the main file, because we don't want that function to query the
   // source buffer for preamble files. For the same reason, we ask
-  // shouldSuppressDiagnostic not to follow macro expansions, since
-  // those might take us into a preamble file as well.
+  // shouldSuppressDiagnostic to avoid I/O.
   bool IsInsideMainFile =
   Info.hasSourceManager() &&
   isInsideMainFile(Info.getLocation(), Info.getSourceManager());
-  if (IsInsideMainFile && tidy::shouldSuppressDiagnostic(
-  DiagLevel, Info, *CTContext,
-  /* CheckMacroExpansion = */ false)) {
+  if (IsInsideMainFile &&
+  tidy::shouldSuppressDiagnostic(DiagLevel, Info, *CTContext,
+ /*AllowIO=*/false)) {
 return DiagnosticsEngine::Ignored;
   }
 }
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -216,15 +216,12 @@
 /// This does not handle suppression of notes following a suppressed diagnostic;
 /// that is left to the caller is it requires maintaining state in between calls
 /// to this function.
-/// The `CheckMacroExpansion` parameter determines whether the function should
-/// handle the case where the diagnostic is inside a macro expansion. A degree
-/// of control over this is needed because handling this case can require
-/// examining source files other than the one in which the diagnostic is
-/// located, and in some use cases we cannot rely on such other files being
-/// mapped in the SourceMapper.
+/// If `AllowIO` is

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

2020-03-26 Thread Nathan Ridge via Phabricator via cfe-commits

nridge updated this revision to Diff 253004.
nridge marked 2 inline comments as done.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75286

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -61,9 +61,7 @@
  arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
 }
 
-MATCHER_P(FixMessage, Message, "") {
-  return arg.Message == Message;
-}
+MATCHER_P(FixMessage, Message, "") { return arg.Message == Message; }
 
 MATCHER_P(EqualToLSPDiag, LSPDiag,
   "LSP diagnostic " + llvm::to_string(LSPDiag)) {
@@ -258,13 +256,13 @@
   Diag(Test.range("macroarg"),
"multiple unsequenced modifications to 'y'"),
   AllOf(
-Diag(Test.range("main"),
- "use a trailing return type for this function"),
-DiagSource(Diag::ClangTidy),
-DiagName("modernize-use-trailing-return-type"),
-// Verify that we don't have "[check-name]" suffix in the message.
-WithFix(FixMessage("use a trailing return type for this function")))
-  ));
+  Diag(Test.range("main"),
+   "use a trailing return type for this function"),
+  DiagSource(Diag::ClangTidy),
+  DiagName("modernize-use-trailing-return-type"),
+  // Verify that we don't have "[check-name]" suffix in the message.
+  WithFix(FixMessage(
+  "use a trailing return type for this function");
 }
 
 TEST(DiagnosticTest, ClangTidySuppressionComment) {
@@ -274,7 +272,11 @@
   double d = 8 / i;  // NOLINT
   // NOLINTNEXTLINE
   double e = 8 / i;
-  double f = [[8]] / i;
+  #define BAD 8 / i
+  double f = BAD;  // NOLINT
+  double g = [[8]] / i;
+  #define BAD2 BAD
+  double h = BAD2;  // NOLINT
 }
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());
@@ -836,8 +838,9 @@
   auto Index = buildIndexWithSymbol({});
   TU.ExternalIndex = Index.get();
 
-  EXPECT_THAT(TU.build().getDiagnostics(),
-  ElementsAre(Diag(Test.range(), "use of undeclared identifier 'a'")));
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  ElementsAre(Diag(Test.range(), "use of undeclared identifier 'a'")));
 }
 
 TEST(DiagsInHeaders, DiagInsideHeader) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -310,14 +310,13 @@
   // Check for suppression comment. Skip the check for diagnostics not
   // in the main file, because we don't want that function to query the
   // source buffer for preamble files. For the same reason, we ask
-  // shouldSuppressDiagnostic not to follow macro expansions, since
-  // those might take us into a preamble file as well.
+  // shouldSuppressDiagnostic to avoid I/O.
   bool IsInsideMainFile =
   Info.hasSourceManager() &&
   isInsideMainFile(Info.getLocation(), Info.getSourceManager());
-  if (IsInsideMainFile && tidy::shouldSuppressDiagnostic(
-  DiagLevel, Info, *CTContext,
-  /* CheckMacroExpansion = */ false)) {
+  if (IsInsideMainFile &&
+  tidy::shouldSuppressDiagnostic(DiagLevel, Info, *CTContext,
+ /*AllowIO=*/false)) {
 return DiagnosticsEngine::Ignored;
   }
 }
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -216,15 +216,12 @@
 /// This does not handle suppression of notes following a suppressed diagnostic;
 /// that is left to the caller is it requires maintaining state in between calls
 /// to this function.
-/// The `CheckMacroExpansion` parameter determines whether the function should
-/// handle the case where the diagnostic is inside a macro expansion. A degree
-/// of control over this is needed because handling this case can require
-/// examining source files other than the one in which the diagnostic is
-/// located, and in some use cases we cannot rely on such other files being
-/// mapped in the

[PATCH] D74324: Tools emit the bug report URL on crash

2020-03-26 Thread Paul Robinson via Phabricator via cfe-commits

probinson added a comment.

Hi @leonardchan 
Owen is on UK time so I took a look.  I think Owen inadvertently put the new 
API in a place that is guarded by `#if ENABLE_BACKTRACES` and I'm guessing that 
your build has that turned off.
I've posted D76893  because something has gone 
wrong with my git environment and I can't commit it myself right now.
I didn't actually try a build with backtraces off, because I figured you'd 
rather have the likely fix sooner.  Hope I did fix it and sorry for the 
breakage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74324



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

[PATCH] D75638: [Hexagon] Support for Linux/Musl ABI.

2020-03-26 Thread Sid Manning via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb0da09498394: [Hexagon] Add support for Linux/Musl ABI (part 
2) (authored by sidneym).

Changed prior to commit:
  https://reviews.llvm.org/D75638?vs=252950=253000#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75638

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/hexagon-linux-vararg.c

Index: clang/test/CodeGen/hexagon-linux-vararg.c
===
--- /dev/null
+++ clang/test/CodeGen/hexagon-linux-vararg.c
@@ -0,0 +1,81 @@
+// REQUIRES: hexagon-registered-target
+// RUN: %clang_cc1 -emit-llvm -triple hexagon-unknown-linux-musl %s -o - | FileCheck %s
+#include 
+
+struct AAA {
+  int x;
+  int y;
+  int z;
+  int d;
+};
+
+// CHECK:   call void @llvm.va_start(i8* %arraydecay1)
+// CHECK:   %arraydecay2 = getelementptr inbounds [1 x %struct.__va_list_tag],
+// [1 x %struct.__va_list_tag]* %ap, i32 0, i32 0
+// CHECK:   br label %vaarg.maybe_reg
+
+// CHECK: vaarg.maybe_reg:  ; preds = %entry
+// CHECK:   %__current_saved_reg_area_pointer_p = getelementptr inbounds
+// %struct.__va_list_tag, %struct.__va_list_tag* %arraydecay2, i32 0, i32 0
+// CHECK:   %__current_saved_reg_area_pointer = load i8*, i8**
+// %__current_saved_reg_area_pointer_p
+// CHECK:   %__saved_reg_area_end_pointer_p = getelementptr inbounds
+// %struct.__va_list_tag, %struct.__va_list_tag* %arraydecay2, i32 0, i32 1
+// CHECK:   %__saved_reg_area_end_pointer = load i8*, i8**
+// %__saved_reg_area_end_pointer_p
+// CHECK:   %__new_saved_reg_area_pointer = getelementptr i8, i8*
+// %__current_saved_reg_area_pointer, i32 4
+// CHECK:   %0 = icmp sgt i8* %__new_saved_reg_area_pointer,
+// %__saved_reg_area_end_pointer
+// CHECK:   br i1 %0, label %vaarg.on_stack, label %vaarg.in_reg
+
+// CHECK: vaarg.in_reg: ; preds =
+// %vaarg.maybe_reg
+// CHECK:   %1 = bitcast i8* %__current_saved_reg_area_pointer to i32*
+// CHECK:   store i8* %__new_saved_reg_area_pointer, i8**
+// %__current_saved_reg_area_pointer_p
+// CHECK:   br label %vaarg.end
+
+// CHECK: vaarg.on_stack:   ; preds =
+// %vaarg.maybe_reg
+// CHECK:   %__overflow_area_pointer_p = getelementptr inbounds
+// %struct.__va_list_tag, %struct.__va_list_tag* %arraydecay2, i32 0, i32 2
+// CHECK:   %__overflow_area_pointer = load i8*, i8** %__overflow_area_pointer_p
+// CHECK:   %__overflow_area_pointer.next = getelementptr i8, i8*
+// %__overflow_area_pointer, i32 4
+// CHECK:   store i8* %__overflow_area_pointer.next, i8**
+// %__overflow_area_pointer_p
+// CHECK:   store i8* %__overflow_area_pointer.next, i8**
+// %__current_saved_reg_area_pointer_p
+// CHECK:   %2 = bitcast i8* %__overflow_area_pointer to i32*
+// CHECK:   br label %vaarg.end
+
+// CHECK: vaarg.end:; preds =
+// %vaarg.on_stack, %vaarg.in_reg
+// CHECK:   %vaarg.addr = phi i32* [ %1, %vaarg.in_reg ], [ %2, %vaarg.on_stack
+// ]
+// CHECK:   %3 = load i32, i32* %vaarg.addr
+
+struct AAA aaa = {100, 200, 300, 400};
+
+int foo(int xx, ...) {
+  va_list ap;
+  int d;
+  int ret = 0;
+  struct AAA bbb;
+  va_start(ap, xx);
+  d = va_arg(ap, int);
+  ret += d;
+  bbb = va_arg(ap, struct AAA);
+  ret += bbb.d;
+  d = va_arg(ap, int);
+  ret += d;
+  va_end(ap);
+  return ret;
+}
+
+int main(void) {
+  int x;
+  x = foo(1, 2, aaa, 4);
+  return x;
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -7584,18 +7584,25 @@
 
 namespace {
 
-class HexagonABIInfo : public ABIInfo {
+class HexagonABIInfo : public DefaultABIInfo {
 public:
-  HexagonABIInfo(CodeGenTypes ) : ABIInfo(CGT) {}
+  HexagonABIInfo(CodeGenTypes ) : DefaultABIInfo(CGT) {}
 
 private:
   ABIArgInfo classifyReturnType(QualType RetTy) const;
   ABIArgInfo classifyArgumentType(QualType RetTy) const;
+  ABIArgInfo classifyArgumentType(QualType RetTy, unsigned *RegsLeft) const;
 
   void computeInfo(CGFunctionInfo ) const override;
 
   Address EmitVAArg(CodeGenFunction , Address VAListAddr,
 QualType Ty) const override;
+  Address EmitVAArgFromMemory(CodeGenFunction , Address VAListAddr,
+  QualType Ty) const;
+  Address EmitVAArgForHexagon(CodeGenFunction , Address VAListAddr,
+  QualType Ty) const;
+  Address EmitVAArgForHexagonLinux(CodeGenFunction , Address VAListAddr,
+   QualType Ty) const;
 };
 
 class HexagonTargetCodeGenInfo : public TargetCodeGenInfo {
@@ -7606,23 +7613,63 @@
   int

[PATCH] D76612: [Matrix] Add draft specification for matrix support in Clang.

2020-03-26 Thread Florian Hahn via Phabricator via cfe-commits

fhahn updated this revision to Diff 252995.
fhahn added a comment.

Update according to comments on cfe-dev.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76612

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/MatrixSupport.rst

Index: clang/docs/MatrixSupport.rst
===
--- /dev/null
+++ clang/docs/MatrixSupport.rst
@@ -0,0 +1,300 @@
+==
+Matrix Support
+==
+
+.. contents::
+   :local:
+
+.. _matrixsupport:
+
+Clang provides a language extension that allows users to express high-level
+matrix math on the C/C++ level. The draft specification can be found :ref:`below `.
+
+Note that the implementation is currently in progress.
+
+.. _matrixsupport-draftspec:
+
+Draft Specification
+===
+
+Matrix Type Attribute
+-
+
+The *attribute-token* ``matrix_type`` is used to declare a matrix type. It shall
+appear at most once in each *attribute-list*. The attribute shall only appertain
+to a *typedef-name* of a typedef of a non-volatile type that is a *signed 
+integer type*, an *unsigned integer type*, or a *floating-point type*. The
+element type must be unqualified and qualifiers are applied to the matrix type
+directly. An *attribute-argument-clause* must be present and it shall have the
+form:
+
+``(constant-expression, constant-expression)``
+
+Both *constant-expressions* shall be a positive non-zero integral constant
+expressions. The maximum of the product of the constants is implementation
+defined. If that implementation defined limit is exceeded, the program is
+ill-formed.
+
+An *attribute* of the form ``matrix_type(R, C)`` forms a matrix type with an
+element type of the cv-qualified type the attribute appertains to and *R* rows
+and *C* columns.
+
+If a declaration of a *typedef-name* has a ``matrix_type`` attribute, then all
+declaration of that *typedef-name* shall have a matrix_type attribute with the
+same element type, number of rows, and number of columns.
+
+Matrix Type
+---
+
+A matrix type has an underlying *element type*, a constant number of rows, and
+a constant number of columns. Matrix types with the same element type, rows,
+and columns are the same type. A value of a matrix type contains ``rows *
+columns`` values of the *element type* laid out in column-major order without
+padding in a way compatible with an array of at least that many elements of the
+underlying *element type*.
+
+A matrix type is a *scalar type* and its alignment is implementation defined.
+
+TODO: Does it make sense to allow M::element_type, M::rows, and M::columns
+where M is a matrix type? We don’t support this anywhere else, but it’s
+convenient. The alternative is using template deduction to extract this
+information. Also add spelling for C.
+
+TODO: Specify how matrix values are passed to functions.
+
+Future Work: Initialization syntax.
+
+Standard Conversions
+
+
+The standard conversions are extended as follows.
+
+For integral promotions, floating-point promotion, integral conversions,
+floating-point conversions, and floating-integral conversions: apply the rules
+to the underlying type of the matrix type. The resulting type is a matrix type
+with that underlying element type. If the number of rows or columns differ
+between the original and resulting type, the program is ill-formed. Otherwise
+the resulting value is as follows:
+
+* If the original value was of matrix type, each element is converted element
+  by element.
+* If the original value was not of matrix type, each element takes the value of
+  the original value.
+
+Arithmetic Conversions
+--
+
+The usual arithmetic conversions are extended as follows.
+
+Insert at the start:
+
+* If either operand is of matrix type, apply the usual arithmetic conversions
+  using its underlying element type. The resulting type is a matrix type with
+  that underlying element type.
+
+Matrix Type Element Access Operator
+---
+
+An expression of the form ``postfix-expression [expression][expression]`` where
+the ``postfix-expression`` is of matrix type is a matrix element access
+expression. ``expression`` shall not be a comma expression, and shall be a
+prvalue of unscoped enumeration or integral type. Given the expression
+``E1[E2][E3]`` the result is an lvalue of the same type as the underlying
+element type of the matrix that refers to the value at E2 row and E3 column in
+the matrix. The expression E1 is sequenced before E2 and E3. The expressions
+E2 and E3 are unsequenced.
+
+**Note**: We considered providing an expression of the form
+``postfix-expression [expression]`` to access columns of a matrix. We think
+that such an expression would be problematic once both column and row major
+matrixes are supported: depending on the memory layout, either

[PATCH] D76547: [WebAssembly] Add wasm-exported function attribute

2020-03-26 Thread Sam Clegg via Phabricator via cfe-commits

sbc100 added a comment.

I was thinking of dropping the new clang attr in favor of the `default` keyword 
in the current attr, but actually keeping the llvm attr, since it corresponds 
quite nicely to the existing EXPORTED symbol attribute in the binary format and 
avoid duplication in the `.ll`, `.s` and `.o` formats.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76547



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

[PATCH] D33029: [clang-format] add option for dangling parenthesis

2020-03-26 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

In D33029#1944919 , @bbassi wrote:

> @MyDeveloperDay Is someone working on fixing the breaking tests and merging 
> it? I need this feature so if someone isn't working on it already, I can take 
> it.


as @stringham isn't working on it feel free to take it. Include me as a 
reviewer and I'll try and give you a timely response.


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

https://reviews.llvm.org/D33029



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

[clang] 5db37f3 - Make PS4 use -fno-use-init-array only as the ABI does not support .init_array.

2020-03-26 Thread Douglas Yung via cfe-commits


Author: Douglas Yung
Date: 2020-03-26T15:45:40-07:00
New Revision: 5db37f3bca3d404b0d7fcbe1dc764ee67665e6c2

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

LOG: Make PS4 use -fno-use-init-array only as the ABI does not support 
.init_array.

Reviewed by Paul Robinson

Added: 
clang/test/Driver/ps4cpu.c

Modified: 
clang/lib/Driver/ToolChains/PS4CPU.cpp
clang/lib/Driver/ToolChains/PS4CPU.h

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/PS4CPU.cpp 
b/clang/lib/Driver/ToolChains/PS4CPU.cpp
index 9ecbb7241d45..2c0d8d05d7c0 100644
--- a/clang/lib/Driver/ToolChains/PS4CPU.cpp
+++ b/clang/lib/Driver/ToolChains/PS4CPU.cpp
@@ -434,3 +434,15 @@ SanitizerMask toolchains::PS4CPU::getSupportedSanitizers() 
const {
   Res |= SanitizerKind::Vptr;
   return Res;
 }
+
+void toolchains::PS4CPU::addClangTargetOptions(
+  const ArgList ,
+  ArgStringList ,
+  Action::OffloadKind DeviceOffloadingKind) const {
+  // PS4 does not use init arrays.
+  if (DriverArgs.hasArg(clang::driver::options::OPT_fuse_init_array))
+getDriver().Diag(clang::diag::err_drv_unsupported_opt_for_target)
+  << "-fuse-init-array" << getTriple().str();
+
+  CC1Args.push_back("-fno-use-init-array");
+}

diff  --git a/clang/lib/Driver/ToolChains/PS4CPU.h 
b/clang/lib/Driver/ToolChains/PS4CPU.h
index c82b0c3a1b56..8fedb6eda0ef 100644
--- a/clang/lib/Driver/ToolChains/PS4CPU.h
+++ b/clang/lib/Driver/ToolChains/PS4CPU.h
@@ -88,6 +88,11 @@ class LLVM_LIBRARY_VISIBILITY PS4CPU : public Generic_ELF {
   // capable of unit splitting.
   bool canSplitThinLTOUnit() const override { return false; }
 
+  void addClangTargetOptions(
+const llvm::opt::ArgList ,
+llvm::opt::ArgStringList ,
+Action::OffloadKind DeviceOffloadingKind) const override;
+
   llvm::DenormalMode getDefaultDenormalModeForType(
 const llvm::opt::ArgList ,
 Action::OffloadKind DeviceOffloadKind,

diff  --git a/clang/test/Driver/ps4cpu.c b/clang/test/Driver/ps4cpu.c
new file mode 100644
index ..ac5f3fcdc844
--- /dev/null
+++ b/clang/test/Driver/ps4cpu.c
@@ -0,0 +1,17 @@
+// REQUIRES: x86-registered-target
+
+// Test that the driver always emits -fno-use-init-array on the PS4 target
+// since its ABI does not support the .init_array section.
+
+// RUN: %clang -c %s -target x86_64-scei-ps4 -### 2>&1  \
+// RUN:   | FileCheck %s
+// RUN: %clang -c %s -target x86_64-scei-ps4 -fno-use-init-array -### 2>&1  \
+// RUN:   | FileCheck %s
+// RUN: %clang -c %s -target x86_64-scei-ps4 -fuse-init-array -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ERROR
+
+// CHECK: "-fno-use-init-array"
+// CHECK-NOT: "-fuse-init-array"
+
+// CHECK-ERROR: unsupported option '-fuse-init-array' for target 
'x86_64-scei-ps4'
+



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

[clang] b0da094 - [Hexagon] Add support for Linux/Musl ABI (part 2)

2020-03-26 Thread Sid Manning via cfe-commits


Author: Sid Manning
Date: 2020-03-26T17:19:46-05:00
New Revision: b0da094983948c8a82f1a26eaa0702920c2b2b36

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

LOG: [Hexagon] Add support for Linux/Musl ABI (part 2)

A continuation of https://reviews.llvm.org/D72701.  This
adds support needed in clang.

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

Added: 
clang/test/CodeGen/hexagon-linux-vararg.c

Modified: 
clang/include/clang/Basic/TargetInfo.h
clang/lib/AST/ASTContext.cpp
clang/lib/Basic/Targets/Hexagon.h
clang/lib/CodeGen/TargetInfo.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/TargetInfo.h 
b/clang/include/clang/Basic/TargetInfo.h
index 615f09985132..81760ec82838 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -273,7 +273,14 @@ class TargetInfo : public virtual TransferrableTargetInfo,
 // void *__overflow_arg_area;
 // void *__reg_save_area;
 //   } va_list[1];
-SystemZBuiltinVaList
+SystemZBuiltinVaList,
+
+// typedef struct __va_list_tag {
+//void *__current_saved_reg_area_pointer;
+//void *__saved_reg_area_end_pointer;
+//void *__overflow_area_pointer;
+//} va_list;
+HexagonBuiltinVaList
   };
 
 protected:

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index dca6523d5176..19f67fc2bb3f 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -7795,6 +7795,57 @@ CreateSystemZBuiltinVaListDecl(const ASTContext 
*Context) {
   return Context->buildImplicitTypedef(VaListTagArrayType, 
"__builtin_va_list");
 }
 
+static TypedefDecl *CreateHexagonBuiltinVaListDecl(const ASTContext *Context) {
+  // typedef struct __va_list_tag {
+  RecordDecl *VaListTagDecl;
+  VaListTagDecl = Context->buildImplicitRecord("__va_list_tag");
+  VaListTagDecl->startDefinition();
+
+  const size_t NumFields = 3;
+  QualType FieldTypes[NumFields];
+  const char *FieldNames[NumFields];
+
+  //   void *CurrentSavedRegisterArea;
+  FieldTypes[0] = Context->getPointerType(Context->VoidTy);
+  FieldNames[0] = "__current_saved_reg_area_pointer";
+
+  //   void *SavedRegAreaEnd;
+  FieldTypes[1] = Context->getPointerType(Context->VoidTy);
+  FieldNames[1] = "__saved_reg_area_end_pointer";
+
+  //   void *OverflowArea;
+  FieldTypes[2] = Context->getPointerType(Context->VoidTy);
+  FieldNames[2] = "__overflow_area_pointer";
+
+  // Create fields
+  for (unsigned i = 0; i < NumFields; ++i) {
+FieldDecl *Field = FieldDecl::Create(
+const_cast(*Context), VaListTagDecl, SourceLocation(),
+SourceLocation(), >Idents.get(FieldNames[i]), FieldTypes[i],
+/*TInfo=*/0,
+/*BitWidth=*/0,
+/*Mutable=*/false, ICIS_NoInit);
+Field->setAccess(AS_public);
+VaListTagDecl->addDecl(Field);
+  }
+  VaListTagDecl->completeDefinition();
+  Context->VaListTagDecl = VaListTagDecl;
+  QualType VaListTagType = Context->getRecordType(VaListTagDecl);
+
+  // } __va_list_tag;
+  TypedefDecl *VaListTagTypedefDecl =
+  Context->buildImplicitTypedef(VaListTagType, "__va_list_tag");
+
+  QualType VaListTagTypedefType = 
Context->getTypedefType(VaListTagTypedefDecl);
+
+  // typedef __va_list_tag __builtin_va_list[1];
+  llvm::APInt Size(Context->getTypeSize(Context->getSizeType()), 1);
+  QualType VaListTagArrayType = Context->getConstantArrayType(
+  VaListTagTypedefType, Size, nullptr, ArrayType::Normal, 0);
+
+  return Context->buildImplicitTypedef(VaListTagArrayType, 
"__builtin_va_list");
+}
+
 static TypedefDecl *CreateVaListDecl(const ASTContext *Context,
  TargetInfo::BuiltinVaListKind Kind) {
   switch (Kind) {
@@ -7814,6 +7865,8 @@ static TypedefDecl *CreateVaListDecl(const ASTContext 
*Context,
 return CreateAAPCSABIBuiltinVaListDecl(Context);
   case TargetInfo::SystemZBuiltinVaList:
 return CreateSystemZBuiltinVaListDecl(Context);
+  case TargetInfo::HexagonBuiltinVaList:
+return CreateHexagonBuiltinVaListDecl(Context);
   }
 
   llvm_unreachable("Unhandled __builtin_va_list type kind");

diff  --git a/clang/lib/Basic/Targets/Hexagon.h 
b/clang/lib/Basic/Targets/Hexagon.h
index f58f594b104f..89e3fa3fa6bf 100644
--- a/clang/lib/Basic/Targets/Hexagon.h
+++ b/clang/lib/Basic/Targets/Hexagon.h
@@ -103,6 +103,8 @@ class LLVM_LIBRARY_VISIBILITY HexagonTargetInfo : public 
TargetInfo {
 DiagnosticsEngine ) override;
 
   BuiltinVaListKind getBuiltinVaListKind() const override {
+if (getTriple().isMusl())
+  return TargetInfo::HexagonBuiltinVaList;
 return TargetInfo::CharPtrBuiltinVaList;
   }
 

diff  --git a/clang/lib/CodeGen/TargetInfo.cpp 
b/clang/lib/CodeGen/TargetInfo.cpp
index

[PATCH] D76547: [WebAssembly] Add wasm-exported function attribute

2020-03-26 Thread Sam Clegg via Phabricator via cfe-commits

sbc100 added a comment.

What about your idea of using the `default` keyword rather than adding a new 
clang attr?  I quite liked that approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76547



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

[PATCH] D76452: Use LLD by default for Android.

2020-03-26 Thread Stephen Hines via Phabricator via cfe-commits

srhines added a comment.

In D76452#1945029 , @MaskRay wrote:

> In D76452#1944786 , @srhines wrote:
>
> > In D76452#1944733 , @MaskRay wrote:
> >
> > > Another approach will be `-DCLANG_DEFAULT_LINKER=lld`. It provides a 
> > > default value for `-fuse-ld=`.
> >
> >
> > How does that work for Darwin builds? Is lld fully supported for MachO at 
> > this point, or will Clang still choose to use the preferred Apple linker?
>
>
> I am not clear about your use case. lld can be used as 
> ELF/Mach-O/COFF/WebAssembly linker. If you want to build ELF objects, 
> -fuse-ld=lld (via clangDriver) or configure clang with 
> -DCLANG_DEFAULT_LINKER=lld.
>  If you want to build Mach-O objects, lld's current Mach-O port lacks 
> features (and will be removed). The new Mach-O port is still under 
> development.


Right, so this is a problem when we ship a multi-target toolchain that does 
support targeting regular Linux, Windows, and Darwin (for host tools) in 
addition to Android. If we switched `CLANG_DEFAULT_LINKER`, then we would need 
to pass explicit flags to any Darwin builds to go back to using the system 
linker.

> To cross build ELF object on macOS, another alternative is a wrapper named 
> `ld` which invokes `lld -flavor gnu "$@"`

@danalbert Would this kind of idea work with your other reverted patch? I'm not 
sure exactly what broke on those builds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76452



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

[PATCH] D33029: [clang-format] add option for dangling parenthesis

2020-03-26 Thread Ryan Stringham via Phabricator via cfe-commits

stringham added a comment.

@bbassi, I don't have plans to address this, and am supportive of you finishing 
it up.

The plan was to move this as one of the options in the `BracketAlignmentStyle` 
configuration (value of `AlwaysBreakWithDanglingParenthesis`) rather than have 
it be a brand new config property.

So we need to do that switch, and write new tests for this feature.


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

https://reviews.llvm.org/D33029



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

[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits

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



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4984
+if (Q && isApprox(Q->getAsType(), T))
+  addType(NNS->getAsIdentifier());
+  }

kadircet wrote:
> sammccall wrote:
> > kadircet wrote:
> > > as T is dependent i suppose NNS should always be an identifier, but would 
> > > be nice to spell it out explicitly with a comment.
> > It doesn't need to be an identifier - it returns null and addType handles 
> > null.
> i thought NNS->getAsIdentifier had an assertion instead of returning null.
> 
> out of curiosity, when can this be non identifier though ?
In general (non-dependent code) NNS is normally a decl rather than an 
identifier.

But here in particular it can certainly be a typespec: that's the 
T::foo::bar case mentioned in the comment, I think the NNS stores a 
TemplateSpecializationType for foo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73649



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

[PATCH] D76547: [WebAssembly] Add wasm-exported function attribute

2020-03-26 Thread Dan Gohman via Phabricator via cfe-commits

sunfish added a comment.

Instead of creating a new LLVM-IR-level attribute here, could you have clang 
translate the attribute to "wasm-export-name", to keep the LLVM-IR level 
simpler?

Also, I myself would be more comfortable with this change if it were restricted 
to Emscripten for now. `export_name` already exists and works in both 
Emscripten and non-Emscripten targets. If there's demand for this new syntax 
outside of Emscripten, I'm happy to reconsider, but until then it seems better 
to be conservative. Obviously it's not possible to completely prevent people 
from becoming dependent on C++ ABI details, but we can avoid giving them tools 
that make it easy to do the wrong thing. And we can keep the ecosystem simpler 
if we don't have multiple ways to do the same thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76547



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

Re: [clang] 857bf5d - [FIX] Do not copy an llvm::function_ref if it has to be reused

2020-03-26 Thread Arthur O'Dwyer via cfe-commits

I'm not sure, but I do see that the call stack contains a call to

bool llvm::function_ref::callback_fn
const>(long, clang::Expr*&, bool)

Notice that this is function_ref::callback_fn instantiated with
T=function_ref itself; i.e., we do have a function_ref to a function_ref.
This is the thing I was saying can happen (in my "lamb/sheep" example) but
which I thought should never happen in this codepath.
Here's function_ref's copy constructor:


  template 

  function_ref(Callable &,


std::enable_if_t,

  function_ref>::value> * =
nullptr)

  : callback(callback_fn::type>),

callable(reinterpret_cast()) {}


I believe that it should be using std::remove_cvref_t, not
std::remove_reference_t, so as not to conflict with the compiler's
implicitly generated copy constructor.

Thoughts?
–Arthur



On Thu, Mar 26, 2020 at 5:08 PM David Blaikie  wrote:

> Hey Richard - could you help try to diagnose what's happening here?
>
> I reverted this patch in:
> https://github.com/llvm/llvm-project/commit/0d0b90105f92f6cd9cc7004d565834f4429183fb
> But that did actually cause buildbot failures, such as these ones:
> http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/24491
>   eg:
> http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/24491/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Adeclare_variant_mixed_codegen.cpp
> I "fixed" these failures blind by reapplying part of this original commit
> (the lambda capture by reference rather than by value):
> https://github.com/llvm/llvm-project/commit/2ec59a0a40f4ec02e6b2dbe5f12261959c191aa9
>
> I've stared at this a fair bit and can't spot any undefined behavior, but
> I guess it's probably in there somewhere - and I'm worried that this fix is
> blind, not fully justified & might be hiding some latent issues.
>
> The full buildbot example failure quoted here in case it times out/gets
> deleted on the buildbot itself:
>
>  TEST 'Clang ::
> OpenMP/declare_variant_mixed_codegen.cpp' FAILED 
> Script:
> --
> : 'RUN: at line 1';
> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/clang
> -cc1 -internal-isystem
> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/lib/clang/11.0.0/include
> -nostdsysteminc -verify -fopenmp -x c++ -triple x86_64-unknown-linux
> -emit-llvm
> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
> -fexceptions -fcxx-exceptions -o - -fsanitize-address-use-after-scope |
> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/FileCheck
> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
> : 'RUN: at line 2';
> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/clang
> -cc1 -internal-isystem
> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/lib/clang/11.0.0/include
> -nostdsysteminc -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-linux
> -fexceptions -fcxx-exceptions -emit-pch -o
> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/tools/clang/test/OpenMP/Output/declare_variant_mixed_codegen.cpp.tmp
> -fopenmp-version=50
> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
> : 'RUN: at line 3';
> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/clang
> -cc1 -internal-isystem
> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/lib/clang/11.0.0/include
> -nostdsysteminc -fopenmp -x c++ -triple x86_64-unknown-linux -fexceptions
> -fcxx-exceptions -std=c++11 -include-pch
> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/tools/clang/test/OpenMP/Output/declare_variant_mixed_codegen.cpp.tmp
> -verify
> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
> -emit-llvm -o - -fopenmp-version=50 |
> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/FileCheck
> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
> --
> Exit Code: 2
>
> Command Output (stderr):
> --
> Stack dump:
> 0. Program arguments:
> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/clang
> -cc1 -internal-isystem
> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/lib/clang/11.0.0/include
> -nostdsysteminc -verify -fopenmp -x c++ -triple x86_64-unknown-linux
> -emit-llvm
> /home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
> -fexceptions -fcxx-exceptions -o -

[PATCH] D76452: Use LLD by default for Android.

2020-03-26 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D76452#1944786 , @srhines wrote:

> In D76452#1944733 , @MaskRay wrote:
>
> > Another approach will be `-DCLANG_DEFAULT_LINKER=lld`. It provides a 
> > default value for `-fuse-ld=`.
>
>
> How does that work for Darwin builds? Is lld fully supported for MachO at 
> this point, or will Clang still choose to use the preferred Apple linker?


I am not clear about your use case. lld can be used as 
ELF/Mach-O/COFF/WebAssembly linker. If you want to build ELF objects, 
-fuse-ld=lld (via clangDriver) or configure clang with 
-DCLANG_DEFAULT_LINKER=lld.
If you want to build Mach-O objects, lld's current Mach-O port lacks features 
(and will be removed). The new Mach-O port is still under development.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76452



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

[PATCH] D59321: AMDGPU: Teach toolchain to link rocm device libs

2020-03-26 Thread Matt Arsenault via Phabricator via cfe-commits

arsenm updated this revision to Diff 252978.
arsenm retitled this revision from "WIP: AMDGPU: Teach toolchain to link rocm 
device libs" to "AMDGPU: Teach toolchain to link rocm device libs".
arsenm edited the summary of this revision.
Herald added a subscriber: Anastasia.

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

https://reviews.llvm.org/D59321

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h
  clang/test/CodeGenOpenCL/amdgpu-debug-info-pointer-address-space.cl
  clang/test/CodeGenOpenCL/amdgpu-debug-info-variable-expression.cl
  clang/test/Driver/Inputs/rocm-device-libs/lib/hip.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/ockl.amdgcn.bc
  
clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_correctly_rounded_sqrt_off.amdgcn.bc
  
clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_correctly_rounded_sqrt_on.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_daz_opt_off.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_daz_opt_on.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_finite_only_off.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_finite_only_on.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_isa_version_1010.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_isa_version_1011.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_isa_version_1012.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_isa_version_803.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_isa_version_900.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_unsafe_math_off.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_unsafe_math_on.amdgcn.bc
  
clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_wavefrontsize64_off.amdgcn.bc
  
clang/test/Driver/Inputs/rocm-device-libs/lib/oclc_wavefrontsize64_on.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/ocml.amdgcn.bc
  clang/test/Driver/Inputs/rocm-device-libs/lib/opencl.amdgcn.bc
  clang/test/Driver/amdgpu-visibility.cl
  clang/test/Driver/rocm-detect.cl
  clang/test/Driver/rocm-device-libs.cl
  clang/test/Driver/rocm-not-found.cl
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/Support/TargetParser.cpp

Index: llvm/lib/Support/TargetParser.cpp
===
--- llvm/lib/Support/TargetParser.cpp
+++ llvm/lib/Support/TargetParser.cpp
@@ -99,9 +99,9 @@
   {{"gfx906"},{"gfx906"},  GK_GFX906,  FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32},
   {{"gfx908"},{"gfx908"},  GK_GFX908,  FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32},
   {{"gfx909"},{"gfx909"},  GK_GFX909,  FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32},
-  {{"gfx1010"},   {"gfx1010"}, GK_GFX1010, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32},
-  {{"gfx1011"},   {"gfx1011"}, GK_GFX1011, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32},
-  {{"gfx1012"},   {"gfx1012"}, GK_GFX1012, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32},
+  {{"gfx1010"},   {"gfx1010"}, GK_GFX1010, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32},
+  {{"gfx1011"},   {"gfx1011"}, GK_GFX1011, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32},
+  {{"gfx1012"},   {"gfx1012"}, GK_GFX1012, FEATURE_FAST_FMA_F32|FEATURE_FAST_DENORMAL_F32|FEATURE_WAVE32},
 };
 
 const GPUInfo *getArchEntry(AMDGPU::GPUKind AK, ArrayRef Table) {
Index: llvm/include/llvm/Support/TargetParser.h
===
--- llvm/include/llvm/Support/TargetParser.h
+++ llvm/include/llvm/Support/TargetParser.h
@@ -151,7 +151,10 @@
 
   // Common features.
   FEATURE_FAST_FMA_F32 = 1 << 4,
-  FEATURE_FAST_DENORMAL_F32 = 1 << 5
+  FEATURE_FAST_DENORMAL_F32 = 1 << 5,
+
+  // Wavefront 32 is available.
+  FEATURE_WAVE32 = 1 << 6
 };
 
 StringRef getArchNameAMDGCN(GPUKind AK);
Index: clang/test/Driver/rocm-not-found.cl
===
--- /dev/null
+++ clang/test/Driver/rocm-not-found.cl
@@ -0,0 +1,11 @@
+// REQUIRES: clang-driver
+
+// Check that we raise an error if we're trying to compile OpenCL for amdhsa code but can't
+// find a ROCm install, unless -nogpulib was passed.
+
+// RUN: %clang -### --sysroot=%s/no-rocm-there -target amdgcn--amdhsa %s 2>&1 | FileCheck %s --check-prefix ERR
+// RUN: %clang -### --rocm-path=%s/no-rocm-there -target amdgcn--amdhsa %s 2>&1 | FileCheck %s --check-prefix ERR
+// ERR: cannot find ROCm installation. Provide its path via --rocm-path, or pass -nogpulib.
+
+// RUN: %clang -### -nogpulib --rocm-path=%s/no-rocm-there %s 2>&1 | FileCheck %s --check-prefix OK
+// OK-NOT: cannot find ROCm installation.
Index: clang/test/Driver/rocm-device-libs.cl
===
---

[clang] 2a43a16 - [OPENMP50]Fix the checks for the nesting of scan directives.

2020-03-26 Thread Alexey Bataev via cfe-commits


Author: Alexey Bataev
Date: 2020-03-26T17:30:02-04:00
New Revision: 2a43a1610db335afcccd1a179a33a0886a5a2c4d

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

LOG: [OPENMP50]Fix the checks for the nesting of scan directives.

Fixed the check for the orhaned scan directives and improved checks for
parallel for and parallel for simd directives.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/nesting_of_regions.cpp
clang/test/OpenMP/scan_messages.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 05645c43f7b7..5605200e6926 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9863,7 +9863,7 @@ def err_omp_prohibited_region : Error<
   "; perhaps you forget to enclose 'omp %3' directive into a for or a parallel 
for region with 'ordered' clause?|"
   "; perhaps you forget to enclose 'omp %3' directive into a target region?|"
   "; perhaps you forget to enclose 'omp %3' directive into a teams region?|"
-  "; perhaps you forget to enclose 'omp %3' directive into a for, simd, or for 
simd region?}2">;
+  "; perhaps you forget to enclose 'omp %3' directive into a for, simd, for 
simd, parallel for, or parallel for simd region?}2">;
 def err_omp_prohibited_region_simd : Error<
   "OpenMP constructs may not be nested inside a simd region%select{| except 
for ordered simd, simd, scan, or atomic directive}0">;
 def err_omp_prohibited_region_atomic : Error<
@@ -10060,7 +10060,8 @@ def warn_omp_nesting_simd : Warning<
   InGroup;
 def err_omp_orphaned_device_directive : Error<
   "orphaned 'omp %0' directives are prohibited"
-  "; perhaps you forget to enclose the directive into a %select{|||target 
|teams|for, simd, or for simd }1region?">;
+  "; perhaps you forget to enclose the directive into a "
+  "%select{|||target |teams|for, simd, for simd, parallel for, or parallel for 
simd }1region?">;
 def err_omp_reduction_non_addressable_expression : Error<
   "expected addressable reduction item for the task-based directives">;
 def err_omp_reduction_with_nogroup : Error<

diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 11cc43a16db1..31921ea728a4 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -4177,7 +4177,7 @@ static bool checkNestingOfRegions(Sema , const 
DSAStackTy *Stack,
 if (ParentRegion == OMPD_unknown &&
 !isOpenMPNestingTeamsDirective(CurrentRegion) &&
 CurrentRegion != OMPD_cancellation_point &&
-CurrentRegion != OMPD_cancel)
+CurrentRegion != OMPD_cancel && CurrentRegion != OMPD_scan)
   return false;
 if (CurrentRegion == OMPD_cancellation_point ||
 CurrentRegion == OMPD_cancel) {
@@ -4298,7 +4298,8 @@ static bool checkNestingOfRegions(Sema , const 
DSAStackTy *Stack,
   NestingProhibited =
   SemaRef.LangOpts.OpenMP < 50 ||
   (ParentRegion != OMPD_simd && ParentRegion != OMPD_for &&
-   ParentRegion != OMPD_for_simd);
+   ParentRegion != OMPD_for_simd && ParentRegion != OMPD_parallel_for 
&&
+   ParentRegion != OMPD_parallel_for_simd);
   OrphanSeen = ParentRegion == OMPD_unknown;
   Recommend = ShouldBeInLoopSimdRegion;
 }

diff  --git a/clang/test/OpenMP/nesting_of_regions.cpp 
b/clang/test/OpenMP/nesting_of_regions.cpp
index d987d84c79e3..4bfaf3f7aeb6 100644
--- a/clang/test/OpenMP/nesting_of_regions.cpp
+++ b/clang/test/OpenMP/nesting_of_regions.cpp
@@ -84,7 +84,7 @@ void foo() {
   }
 #pragma omp parallel
   {
-#pragma omp scan // expected-error {{region cannot be closely nested inside 
'parallel' region; perhaps you forget to enclose 'omp scan' directive into a 
for, simd, or for simd region?}}
+#pragma omp scan // expected-error {{region cannot be closely nested inside 
'parallel' region; perhaps you forget to enclose 'omp scan' directive into a 
for, simd, for simd, parallel for, or parallel for simd region?}}
 bar();
   }
 #pragma omp parallel
@@ -618,7 +618,7 @@ void foo() {
   }
 #pragma omp for
   for (int i = 0; i < 10; ++i) {
-#pragma omp scan // omp45-error {{region cannot be closely nested inside 'for' 
region; perhaps you forget to enclose 'omp scan' directive into a for, simd, or 
for simd region?}} omp50-error {{exactly one of 'inclusive' or 'exclusive' 
clauses is expected}}
+#pragma omp scan // omp45-error {{region cannot be closely nested inside 'for' 
region; perhaps you forget to enclose 'omp scan' directive into a for, simd, 
for simd, parallel for, or parallel for simd region?}} omp50-error {{exactly 
one of 'inclusive' or 'exclusive'

[PATCH] D76653: [clang] Allow -DDEFAULT_SYSROOT to be a relative path

2020-03-26 Thread Sam Clegg via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
sbc100 marked an inline comment as done.
Closed by commit rG0731372ee25c: [clang] Allow -DDEFAULT_SYSROOT to be a 
relative path (authored by sbc100).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76653

Files:
  clang/CMakeLists.txt
  clang/lib/Driver/Driver.cpp


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -140,6 +140,13 @@
   Dir = std::string(llvm::sys::path::parent_path(ClangExecutable));
   InstalledDir = Dir; // Provide a sensible default installed dir.
 
+  if ((!SysRoot.empty()) && llvm::sys::path::is_relative(SysRoot)) {
+// Prepend InstalledDir if SysRoot is relative
+SmallString<128> P(InstalledDir);
+llvm::sys::path::append(P, SysRoot);
+SysRoot = std::string(P);
+  }
+
 #if defined(CLANG_CONFIG_FILE_SYSTEM_DIR)
   SystemConfigDir = CLANG_CONFIG_FILE_SYSTEM_DIR;
 #endif
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -226,7 +226,7 @@
   "Colon separated list of directories clang will search for headers.")
 
 set(GCC_INSTALL_PREFIX "" CACHE PATH "Directory where gcc is installed." )
-set(DEFAULT_SYSROOT "" CACHE PATH
+set(DEFAULT_SYSROOT "" CACHE STRING
   "Default  to all compiler invocations for --sysroot=." )
 
 set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld")


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -140,6 +140,13 @@
   Dir = std::string(llvm::sys::path::parent_path(ClangExecutable));
   InstalledDir = Dir; // Provide a sensible default installed dir.
 
+  if ((!SysRoot.empty()) && llvm::sys::path::is_relative(SysRoot)) {
+// Prepend InstalledDir if SysRoot is relative
+SmallString<128> P(InstalledDir);
+llvm::sys::path::append(P, SysRoot);
+SysRoot = std::string(P);
+  }
+
 #if defined(CLANG_CONFIG_FILE_SYSTEM_DIR)
   SystemConfigDir = CLANG_CONFIG_FILE_SYSTEM_DIR;
 #endif
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -226,7 +226,7 @@
   "Colon separated list of directories clang will search for headers.")
 
 set(GCC_INSTALL_PREFIX "" CACHE PATH "Directory where gcc is installed." )
-set(DEFAULT_SYSROOT "" CACHE PATH
+set(DEFAULT_SYSROOT "" CACHE STRING
   "Default  to all compiler invocations for --sysroot=." )
 
 set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.

2020-03-26 Thread Kadir Cetinkaya via Phabricator via cfe-commits

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

LGTM. thanks!




Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4984
+if (Q && isApprox(Q->getAsType(), T))
+  addType(NNS->getAsIdentifier());
+  }

sammccall wrote:
> kadircet wrote:
> > as T is dependent i suppose NNS should always be an identifier, but would 
> > be nice to spell it out explicitly with a comment.
> It doesn't need to be an identifier - it returns null and addType handles 
> null.
i thought NNS->getAsIdentifier had an assertion instead of returning null.

out of curiosity, when can this be non identifier though ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73649



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

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

In D76696#1944825 , 
@hubert.reinterpretcast wrote:

> In D76696#1944784 , @sammccall wrote:
>
> > The general scheme is probably common: unresolved expr -> ??? -> an 
> > expression is dependent but not marked as such -> constant evaluation 
> > crashes.
> >
> > But the ??? matters, as that's where the fix is.
> >  In the case above: expr is used in a member of X, and X is not a dependent 
> > type, so sizeof(X) is not considered dependent
>
>
> The context, if I understand correctly for the cases I am seeing, boil down 
> to:
>
> - Value of a member initializer for a constexpr constructor




  struct X {
int Y;
constexpr X() : Y(foo()) {]
  };

This will need a different fix I think. Maybe just isPotentialConstantExpr 
needs to bail out if there are errors.

> - Bitfield width



  struct X { int Y : foo(); };
  constexpr int Z = sizeof(X);

I think this one is just another case of marking the fielddecl as invalid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76696



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

[PATCH] D33029: [clang-format] add option for dangling parenthesis

2020-03-26 Thread Bhopesh Bassi via Phabricator via cfe-commits

bbassi added a comment.

@MyDeveloperDay Is someone working on fixing the breaking tests and merging it? 
I need this feature so if someone isn't working on it already, I can take it.


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

https://reviews.llvm.org/D33029



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

[PATCH] D76887: AMDGPU: Make HIPToolChain a subclass of ROCMToolChain

2020-03-26 Thread Matt Arsenault via Phabricator via cfe-commits

arsenm created this revision.
arsenm added reviewers: yaxunl, gregrodgers.
Herald added subscribers: t-tye, tpr, dstuttard, wdng, kzhuravl.
arsenm added parent revisions: D59321: WIP: AMDGPU: Teach toolchain to link 
rocm device libs, D76862: HIP: Ensure new denormal mode attributes are set.

This fixes some code duplication. This is also a step towards
consolidating builtin library handling.


https://reviews.llvm.org/D76887

Files:
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/lib/Driver/ToolChains/HIP.h


Index: clang/lib/Driver/ToolChains/HIP.h
===
--- clang/lib/Driver/ToolChains/HIP.h
+++ clang/lib/Driver/ToolChains/HIP.h
@@ -11,6 +11,7 @@
 
 #include "clang/Driver/ToolChain.h"
 #include "clang/Driver/Tool.h"
+#include "AMDGPU.h"
 
 namespace clang {
 namespace driver {
@@ -72,7 +73,7 @@
 
 namespace toolchains {
 
-class LLVM_LIBRARY_VISIBILITY HIPToolChain : public ToolChain {
+class LLVM_LIBRARY_VISIBILITY HIPToolChain final : public ROCMToolChain {
 public:
   HIPToolChain(const Driver , const llvm::Triple ,
 const ToolChain , const llvm::opt::ArgList );
@@ -115,11 +116,6 @@
 
   unsigned GetDefaultDwarfVersion() const override { return 4; }
 
-  llvm::DenormalMode getDefaultDenormalModeForType(
-const llvm::opt::ArgList ,
-Action::OffloadKind DeviceOffloadKind,
-const llvm::fltSemantics *FPType = nullptr) const override;
-
   const ToolChain 
 
 protected:
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -268,40 +268,12 @@
 
 HIPToolChain::HIPToolChain(const Driver , const llvm::Triple ,
  const ToolChain , const ArgList )
-: ToolChain(D, Triple, Args), HostTC(HostTC) {
+: ROCMToolChain(D, Triple, Args), HostTC(HostTC) {
   // Lookup binaries into the driver directory, this is used to
   // discover the clang-offload-bundler executable.
   getProgramPaths().push_back(getDriver().Dir);
 }
 
-// FIXME: Duplicated in AMDGPUToolChain
-llvm::DenormalMode HIPToolChain::getDefaultDenormalModeForType(
-const llvm::opt::ArgList , Action::OffloadKind 
DeviceOffloadKind,
-const llvm::fltSemantics *FPType) const {
-  // Denormals should always be enabled for f16 and f64.
-  if (!FPType || FPType != ::APFloat::IEEEsingle())
-return llvm::DenormalMode::getIEEE();
-
-  if (DeviceOffloadKind == Action::OFK_Cuda) {
-if (FPType && FPType == ::APFloat::IEEEsingle() &&
-DriverArgs.hasFlag(options::OPT_fcuda_flush_denormals_to_zero,
-   options::OPT_fno_cuda_flush_denormals_to_zero,
-   false))
-  return llvm::DenormalMode::getPreserveSign();
-  }
-
-  const StringRef GpuArch = DriverArgs.getLastArgValue(options::OPT_mcpu_EQ);
-  auto Kind = llvm::AMDGPU::parseArchAMDGCN(GpuArch);
-
-  // TODO: There are way too many flags that change this. Do we need to check
-  // them all?
-  bool DAZ = DriverArgs.hasArg(options::OPT_cl_denorms_are_zero) ||
-!AMDGPUToolChain::getDefaultDenormsAreZeroForTarget(Kind);
-  // Outputs are flushed to zero, preserving sign
-  return DAZ ? llvm::DenormalMode::getPreserveSign() :
-   llvm::DenormalMode::getIEEE();
-}
-
 void HIPToolChain::addClangTargetOptions(
 const llvm::opt::ArgList ,
 llvm::opt::ArgStringList ,


Index: clang/lib/Driver/ToolChains/HIP.h
===
--- clang/lib/Driver/ToolChains/HIP.h
+++ clang/lib/Driver/ToolChains/HIP.h
@@ -11,6 +11,7 @@
 
 #include "clang/Driver/ToolChain.h"
 #include "clang/Driver/Tool.h"
+#include "AMDGPU.h"
 
 namespace clang {
 namespace driver {
@@ -72,7 +73,7 @@
 
 namespace toolchains {
 
-class LLVM_LIBRARY_VISIBILITY HIPToolChain : public ToolChain {
+class LLVM_LIBRARY_VISIBILITY HIPToolChain final : public ROCMToolChain {
 public:
   HIPToolChain(const Driver , const llvm::Triple ,
 const ToolChain , const llvm::opt::ArgList );
@@ -115,11 +116,6 @@
 
   unsigned GetDefaultDwarfVersion() const override { return 4; }
 
-  llvm::DenormalMode getDefaultDenormalModeForType(
-const llvm::opt::ArgList ,
-Action::OffloadKind DeviceOffloadKind,
-const llvm::fltSemantics *FPType = nullptr) const override;
-
   const ToolChain 
 
 protected:
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -268,40 +268,12 @@
 
 HIPToolChain::HIPToolChain(const Driver , const llvm::Triple ,
  const ToolChain , const ArgList )
-: ToolChain(D, Triple, Args), HostTC(HostTC) {
+: ROCMToolChain(D, Triple, Args), HostTC(HostTC) {
   // Lookup binaries into the driver directory, this is used to
   // discover the clang-offload-bundler

Re: [clang] 857bf5d - [FIX] Do not copy an llvm::function_ref if it has to be reused

2020-03-26 Thread David Blaikie via cfe-commits

Hey Richard - could you help try to diagnose what's happening here?

I reverted this patch in:
https://github.com/llvm/llvm-project/commit/0d0b90105f92f6cd9cc7004d565834f4429183fb
But that did actually cause buildbot failures, such as these ones:
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/24491
  eg:
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/24491/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Adeclare_variant_mixed_codegen.cpp
I "fixed" these failures blind by reapplying part of this original commit
(the lambda capture by reference rather than by value):
https://github.com/llvm/llvm-project/commit/2ec59a0a40f4ec02e6b2dbe5f12261959c191aa9

I've stared at this a fair bit and can't spot any undefined behavior, but I
guess it's probably in there somewhere - and I'm worried that this fix is
blind, not fully justified & might be hiding some latent issues.

The full buildbot example failure quoted here in case it times out/gets
deleted on the buildbot itself:

 TEST 'Clang ::
OpenMP/declare_variant_mixed_codegen.cpp' FAILED 
Script:
--
: 'RUN: at line 1';
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/clang
-cc1 -internal-isystem
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/lib/clang/11.0.0/include
-nostdsysteminc -verify -fopenmp -x c++ -triple x86_64-unknown-linux
-emit-llvm
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
-fexceptions -fcxx-exceptions -o - -fsanitize-address-use-after-scope |
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/FileCheck
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
: 'RUN: at line 2';
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/clang
-cc1 -internal-isystem
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/lib/clang/11.0.0/include
-nostdsysteminc -fopenmp -x c++ -std=c++11 -triple x86_64-unknown-linux
-fexceptions -fcxx-exceptions -emit-pch -o
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/tools/clang/test/OpenMP/Output/declare_variant_mixed_codegen.cpp.tmp
-fopenmp-version=50
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
: 'RUN: at line 3';
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/clang
-cc1 -internal-isystem
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/lib/clang/11.0.0/include
-nostdsysteminc -fopenmp -x c++ -triple x86_64-unknown-linux -fexceptions
-fcxx-exceptions -std=c++11 -include-pch
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/tools/clang/test/OpenMP/Output/declare_variant_mixed_codegen.cpp.tmp
-verify
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
-emit-llvm -o - -fopenmp-version=50 |
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/FileCheck
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
--
Exit Code: 2

Command Output (stderr):
--
Stack dump:
0. Program arguments:
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/clang
-cc1 -internal-isystem
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/lib/clang/11.0.0/include
-nostdsysteminc -verify -fopenmp -x c++ -triple x86_64-unknown-linux
-emit-llvm
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
-fexceptions -fcxx-exceptions -o - -fsanitize-address-use-after-scope
1.
/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/llvm/clang/test/OpenMP/declare_variant_mixed_codegen.cpp:38:92:
at annotation token
 #0 0x12e3ebd0 llvm::sys::PrintStackTrace(llvm::raw_ostream&)
(/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/clang+0x12e3ebd0)
 #1 0x12e3ece8 PrintStackTraceSignalHandler(void*)
(/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/clang+0x12e3ece8)
 #2 0x12e3c4e0 llvm::sys::RunSignalHandlers()
(/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/clang+0x12e3c4e0)
 #3 0x12e3d3a4 SignalHandler(int)
(/home/buildbots/ppc64be-clang-multistage-test/clang-ppc64be-multistage/stage1/bin/clang+0x12e3d3a4)
 #4 0x7fffb50f04d8 (linux-vdso64.so.1+0x4d8)
 #5 0x14df41ac bool llvm::function_ref::callback_fn
const>(long, clang::Expr*&, bool)

[clang] 0731372 - [clang] Allow -DDEFAULT_SYSROOT to be a relative path

2020-03-26 Thread Sam Clegg via cfe-commits


Author: Sam Clegg
Date: 2020-03-26T13:48:57-07:00
New Revision: 0731372ee25c8db93e0d976db4be4ffb7c7329c5

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

LOG: [clang] Allow -DDEFAULT_SYSROOT to be a relative path

In this case we interpret the path as relative the clang driver binary.

This allows SDKs to be built that include clang along with a custom
sysroot without requiring users to specify --sysroot to point to the
directory where they installed the SDK.

See https://github.com/WebAssembly/wasi-sdk/issues/58

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

Added: 


Modified: 
clang/CMakeLists.txt
clang/lib/Driver/Driver.cpp

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index db82d87b181f..7809d6529195 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -226,7 +226,7 @@ set(C_INCLUDE_DIRS "" CACHE STRING
   "Colon separated list of directories clang will search for headers.")
 
 set(GCC_INSTALL_PREFIX "" CACHE PATH "Directory where gcc is installed." )
-set(DEFAULT_SYSROOT "" CACHE PATH
+set(DEFAULT_SYSROOT "" CACHE STRING
   "Default  to all compiler invocations for --sysroot=." )
 
 set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld")

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index e3b7793ca266..4afe3e635fc8 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -140,6 +140,13 @@ Driver::Driver(StringRef ClangExecutable, StringRef 
TargetTriple,
   Dir = std::string(llvm::sys::path::parent_path(ClangExecutable));
   InstalledDir = Dir; // Provide a sensible default installed dir.
 
+  if ((!SysRoot.empty()) && llvm::sys::path::is_relative(SysRoot)) {
+// Prepend InstalledDir if SysRoot is relative
+SmallString<128> P(InstalledDir);
+llvm::sys::path::append(P, SysRoot);
+SysRoot = std::string(P);
+  }
+
 #if defined(CLANG_CONFIG_FILE_SYSTEM_DIR)
   SystemConfigDir = CLANG_CONFIG_FILE_SYSTEM_DIR;
 #endif



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

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast added a comment.

In D76696#1944784 , @sammccall wrote:

> The general scheme is probably common: unresolved expr -> ??? -> an 
> expression is dependent but not marked as such -> constant evaluation crashes.
>
> But the ??? matters, as that's where the fix is.
>  In the case above: expr is used in a member of X, and X is not a dependent 
> type, so sizeof(X) is not considered dependent


The context, if I understand correctly for the cases I am seeing, boil down to:

- Value of a member initializer for a constexpr constructor
- Bitfield width


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76696



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

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

In D76696#1944625 , 
@hubert.reinterpretcast wrote:

> All of them appear to have unresolved names in constexpr evaluation. It is 
> likely to be the same issue.

The general scheme is probably common: unresolved expr -> ??? -> an expression 
is dependent but not marked as such -> constant evaluation crashes.

But the ??? matters, as that's where the fix is.
In the case above: expr is used in a member of X, and X is not a dependent 
type, so sizeof(X) is not considered dependent

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76696

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

[PATCH] D76452: Use LLD by default for Android.

2020-03-26 Thread Stephen Hines via Phabricator via cfe-commits

srhines added a comment.

In D76452#1944733 , @MaskRay wrote:

> Another approach will be `-DCLANG_DEFAULT_LINKER=lld`. It provides a default 
> value for `-fuse-ld=`.


How does that work for Darwin builds? Is lld fully supported for MachO at this 
point, or will Clang still choose to use the preferred Apple linker?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76452



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

[PATCH] D75638: [Hexagon] Support for Linux/Musl ABI.

2020-03-26 Thread Sid Manning via Phabricator via cfe-commits

sidneym updated this revision to Diff 252950.
sidneym added a comment.

update for new tidy checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75638

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/hexagon-linux-vararg.c

Index: clang/test/CodeGen/hexagon-linux-vararg.c
===
--- /dev/null
+++ clang/test/CodeGen/hexagon-linux-vararg.c
@@ -0,0 +1,81 @@
+// REQUIRES: hexagon-registered-target
+// RUN: %clang_cc1 -emit-llvm -triple hexagon-unknown-linux-musl %s -o - | FileCheck %s
+#include 
+
+struct AAA {
+  int x;
+  int y;
+  int z;
+  int d;
+};
+
+// CHECK:   call void @llvm.va_start(i8* %arraydecay1)
+// CHECK:   %arraydecay2 = getelementptr inbounds [1 x %struct.__va_list_tag],
+// [1 x %struct.__va_list_tag]* %ap, i32 0, i32 0
+// CHECK:   br label %vaarg.maybe_reg
+
+// CHECK: vaarg.maybe_reg:  ; preds = %entry
+// CHECK:   %__current_saved_reg_area_pointer_p = getelementptr inbounds
+// %struct.__va_list_tag, %struct.__va_list_tag* %arraydecay2, i32 0, i32 0
+// CHECK:   %__current_saved_reg_area_pointer = load i8*, i8**
+// %__current_saved_reg_area_pointer_p
+// CHECK:   %__saved_reg_area_end_pointer_p = getelementptr inbounds
+// %struct.__va_list_tag, %struct.__va_list_tag* %arraydecay2, i32 0, i32 1
+// CHECK:   %__saved_reg_area_end_pointer = load i8*, i8**
+// %__saved_reg_area_end_pointer_p
+// CHECK:   %__new_saved_reg_area_pointer = getelementptr i8, i8*
+// %__current_saved_reg_area_pointer, i32 4
+// CHECK:   %0 = icmp sgt i8* %__new_saved_reg_area_pointer,
+// %__saved_reg_area_end_pointer
+// CHECK:   br i1 %0, label %vaarg.on_stack, label %vaarg.in_reg
+
+// CHECK: vaarg.in_reg: ; preds =
+// %vaarg.maybe_reg
+// CHECK:   %1 = bitcast i8* %__current_saved_reg_area_pointer to i32*
+// CHECK:   store i8* %__new_saved_reg_area_pointer, i8**
+// %__current_saved_reg_area_pointer_p
+// CHECK:   br label %vaarg.end
+
+// CHECK: vaarg.on_stack:   ; preds =
+// %vaarg.maybe_reg
+// CHECK:   %__overflow_area_pointer_p = getelementptr inbounds
+// %struct.__va_list_tag, %struct.__va_list_tag* %arraydecay2, i32 0, i32 2
+// CHECK:   %__overflow_area_pointer = load i8*, i8** %__overflow_area_pointer_p
+// CHECK:   %__overflow_area_pointer.next = getelementptr i8, i8*
+// %__overflow_area_pointer, i32 4
+// CHECK:   store i8* %__overflow_area_pointer.next, i8**
+// %__overflow_area_pointer_p
+// CHECK:   store i8* %__overflow_area_pointer.next, i8**
+// %__current_saved_reg_area_pointer_p
+// CHECK:   %2 = bitcast i8* %__overflow_area_pointer to i32*
+// CHECK:   br label %vaarg.end
+
+// CHECK: vaarg.end:; preds =
+// %vaarg.on_stack, %vaarg.in_reg
+// CHECK:   %vaarg.addr = phi i32* [ %1, %vaarg.in_reg ], [ %2, %vaarg.on_stack
+// ]
+// CHECK:   %3 = load i32, i32* %vaarg.addr
+
+struct AAA aaa = {100, 200, 300, 400};
+
+int foo(int xx, ...) {
+  va_list ap;
+  int d;
+  int ret = 0;
+  struct AAA bbb;
+  va_start(ap, xx);
+  d = va_arg(ap, int);
+  ret += d;
+  bbb = va_arg(ap, struct AAA);
+  ret += bbb.d;
+  d = va_arg(ap, int);
+  ret += d;
+  va_end(ap);
+  return ret;
+}
+
+int main(void) {
+  int x;
+  x = foo(1, 2, aaa, 4);
+  return x;
+}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -7584,18 +7584,25 @@
 
 namespace {
 
-class HexagonABIInfo : public ABIInfo {
+class HexagonABIInfo : public DefaultABIInfo {
 public:
-  HexagonABIInfo(CodeGenTypes ) : ABIInfo(CGT) {}
+  HexagonABIInfo(CodeGenTypes ) : DefaultABIInfo(CGT) {}
 
 private:
   ABIArgInfo classifyReturnType(QualType RetTy) const;
   ABIArgInfo classifyArgumentType(QualType RetTy) const;
+  ABIArgInfo classifyArgumentType(QualType RetTy, unsigned *RegsLeft) const;
 
   void computeInfo(CGFunctionInfo ) const override;
 
   Address EmitVAArg(CodeGenFunction , Address VAListAddr,
 QualType Ty) const override;
+  Address EmitVAArgFromMemory(CodeGenFunction , Address VAListAddr,
+  QualType Ty) const;
+  Address EmitVAArgForHexagon(CodeGenFunction , Address VAListAddr,
+  QualType Ty) const;
+  Address EmitVAArgForHexagonLinux(CodeGenFunction , Address VAListAddr,
+   QualType Ty) const;
 };
 
 class HexagonTargetCodeGenInfo : public TargetCodeGenInfo {
@@ -7606,23 +7613,63 @@
   int getDwarfEHStackPointer(CodeGen::CodeGenModule ) const override {
 return 29;
   }
+
+  void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
+

[clang] f9e71f4 - Revert "[OPENMP50]Add basic support for inscan reduction modifier."

2020-03-26 Thread Alexey Bataev via cfe-commits


Author: Alexey Bataev
Date: 2020-03-26T15:57:19-04:00
New Revision: f9e71f4d9d39871390da48207d7fd6b116e370dc

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

LOG: Revert "[OPENMP50]Add basic support for inscan reduction modifier."

This reverts commit 8099e0fe82ce78c15bc6c4cf52caca5b6fbe28f5 to fix the
problems with the Windows-based buildbots.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/OpenMPKinds.def
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/nesting_of_regions.cpp
clang/test/OpenMP/parallel_for_reduction_messages.cpp
clang/test/OpenMP/parallel_reduction_messages.c
clang/test/OpenMP/scan_ast_print.cpp
clang/test/OpenMP/scan_messages.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a71749cddbc6..05645c43f7b7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9863,7 +9863,7 @@ def err_omp_prohibited_region : Error<
   "; perhaps you forget to enclose 'omp %3' directive into a for or a parallel 
for region with 'ordered' clause?|"
   "; perhaps you forget to enclose 'omp %3' directive into a target region?|"
   "; perhaps you forget to enclose 'omp %3' directive into a teams region?|"
-  "; perhaps you forget to enclose 'omp %3' directive into a for, simd, for 
simd, parallel for, or parallel for simd region?}2">;
+  "; perhaps you forget to enclose 'omp %3' directive into a for, simd, or for 
simd region?}2">;
 def err_omp_prohibited_region_simd : Error<
   "OpenMP constructs may not be nested inside a simd region%select{| except 
for ordered simd, simd, scan, or atomic directive}0">;
 def err_omp_prohibited_region_atomic : Error<
@@ -10060,8 +10060,7 @@ def warn_omp_nesting_simd : Warning<
   InGroup;
 def err_omp_orphaned_device_directive : Error<
   "orphaned 'omp %0' directives are prohibited"
-  "; perhaps you forget to enclose the directive into a "
-  "%select{|||target |teams|for, simd, for simd, parallel for, or parallel for 
simd }1region?">;
+  "; perhaps you forget to enclose the directive into a %select{|||target 
|teams|for, simd, or for simd }1region?">;
 def err_omp_reduction_non_addressable_expression : Error<
   "expected addressable reduction item for the task-based directives">;
 def err_omp_reduction_with_nogroup : Error<
@@ -10101,19 +10100,6 @@ def err_omp_depobj_single_clause_expected : Error<
   "exactly one of 'depend', 'destroy', or 'update' clauses is expected">;
 def err_omp_scan_single_clause_expected : Error<
   "exactly one of 'inclusive' or 'exclusive' clauses is expected">;
-def err_omp_inclusive_exclusive_not_reduction : Error<
-  "the list item must appear in 'reduction' clause with the 'inscan' modifier "
-  "of the parent directive">;
-def err_omp_reduction_not_inclusive_exclusive : Error<
-  "the inscan reduction list item must appear as a list item in an 'inclusive' 
or"
-  " 'exclusive' clause on an inner 'omp scan' directive">;
-def err_omp_wrong_inscan_reduction : Error<
-  "'inscan' modifier can be used only in 'omp for', 'omp simd', 'omp for 
simd',"
-  " 'omp parallel for', or 'omp parallel for simd' directive">;
-def err_omp_inscan_reduction_expected : Error<
-  "expected 'reduction' clause with the 'inscan' modifier">;
-def note_omp_previous_inscan_reduction : Note<
-  "'reduction' clause with 'inscan' modifier is used here">;
 def err_omp_expected_predefined_allocator : Error<
   "expected one of the predefined allocators for the variables with the static 
"
   "storage: 'omp_default_mem_alloc', 'omp_large_cap_mem_alloc', "

diff  --git a/clang/include/clang/Basic/OpenMPKinds.def 
b/clang/include/clang/Basic/OpenMPKinds.def
index 3cf92ead9560..bfb41ab105ea 100644
--- a/clang/include/clang/Basic/OpenMPKinds.def
+++ b/clang/include/clang/Basic/OpenMPKinds.def
@@ -1112,7 +1112,6 @@ OPENMP_DEPOBJ_CLAUSE(update)
 
 // Modifiers for 'reduction' clause.
 OPENMP_REDUCTION_MODIFIER(default)
-OPENMP_REDUCTION_MODIFIER(inscan)
 
 #undef OPENMP_REDUCTION_MODIFIER
 #undef OPENMP_SCAN_CLAUSE

diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index d0c304d37336..11cc43a16db1 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -62,15 +62,14 @@ class DSAStackTy {
   struct DSAVarData {
 OpenMPDirectiveKind DKind = OMPD_unknown;
 OpenMPClauseKind CKind = OMPC_unknown;
-unsigned Modifier = 0;
 const Expr *RefExpr = nullptr;
 DeclRefExpr *PrivateCopy = nullptr;
 SourceLocation ImplicitDSALoc;
 DSAVarData() = default;
 DSAVarData(OpenMPDirectiveKind DKind, OpenMPClauseKind CKind,
const Expr *RefExpr,

[PATCH] D76452: Use LLD by default for Android.

2020-03-26 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

Another approach will be `-DCLANG_DEFAULT_LINKER=lld`. It provides a default 
value for `-fuse-ld=`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76452



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

[PATCH] D76452: Use LLD by default for Android.

2020-03-26 Thread Dan Albert via Phabricator via cfe-commits

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

It seems I'd goofed something in my testing earlier (I think I still had 
`-fuse-ld=lld` force on in my build system). While Clang will find `ld` in the 
driver directory and prefer it, LLD defaults to the Darwin driver mode when 
`argv[0]` is `ld` when run on Darwin. We need GNU mode, and the best way to get 
this behavior is to have Clang invoke `ld.lld` instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76452



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

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast added a comment.

> In D76696#1944014 , 
> @hubert.reinterpretcast wrote:
> 
>> We have also encountered crashes in downstream testing caused by this using 
>> just the vanilla source from trunk. When there is a proposed fix, please let 
>> us know so we can test. Thanks.
> 
> 
> We can try but it would be very useful to have details, no idea whether it's 
> going to be the same issue and the more we know about the easier it is to 
> improve the design.
>  Can you describe the case at all? Any chance of a minimal example? Ideally 
> we'd check it in.

All of them appear to have unresolved names in constexpr evaluation. It is 
likely to be the same issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76696



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

[clang] 8099e0f - [OPENMP50]Add basic support for inscan reduction modifier.

2020-03-26 Thread Alexey Bataev via cfe-commits


Author: Alexey Bataev
Date: 2020-03-26T14:51:09-04:00
New Revision: 8099e0fe82ce78c15bc6c4cf52caca5b6fbe28f5

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

LOG: [OPENMP50]Add basic support for inscan reduction modifier.

Added basic support (parsing/sema checks) for the inscan modifier in the
reduction clauses.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/OpenMPKinds.def
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/nesting_of_regions.cpp
clang/test/OpenMP/parallel_for_reduction_messages.cpp
clang/test/OpenMP/parallel_reduction_messages.c
clang/test/OpenMP/scan_ast_print.cpp
clang/test/OpenMP/scan_messages.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 05645c43f7b7..a71749cddbc6 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9863,7 +9863,7 @@ def err_omp_prohibited_region : Error<
   "; perhaps you forget to enclose 'omp %3' directive into a for or a parallel 
for region with 'ordered' clause?|"
   "; perhaps you forget to enclose 'omp %3' directive into a target region?|"
   "; perhaps you forget to enclose 'omp %3' directive into a teams region?|"
-  "; perhaps you forget to enclose 'omp %3' directive into a for, simd, or for 
simd region?}2">;
+  "; perhaps you forget to enclose 'omp %3' directive into a for, simd, for 
simd, parallel for, or parallel for simd region?}2">;
 def err_omp_prohibited_region_simd : Error<
   "OpenMP constructs may not be nested inside a simd region%select{| except 
for ordered simd, simd, scan, or atomic directive}0">;
 def err_omp_prohibited_region_atomic : Error<
@@ -10060,7 +10060,8 @@ def warn_omp_nesting_simd : Warning<
   InGroup;
 def err_omp_orphaned_device_directive : Error<
   "orphaned 'omp %0' directives are prohibited"
-  "; perhaps you forget to enclose the directive into a %select{|||target 
|teams|for, simd, or for simd }1region?">;
+  "; perhaps you forget to enclose the directive into a "
+  "%select{|||target |teams|for, simd, for simd, parallel for, or parallel for 
simd }1region?">;
 def err_omp_reduction_non_addressable_expression : Error<
   "expected addressable reduction item for the task-based directives">;
 def err_omp_reduction_with_nogroup : Error<
@@ -10100,6 +10101,19 @@ def err_omp_depobj_single_clause_expected : Error<
   "exactly one of 'depend', 'destroy', or 'update' clauses is expected">;
 def err_omp_scan_single_clause_expected : Error<
   "exactly one of 'inclusive' or 'exclusive' clauses is expected">;
+def err_omp_inclusive_exclusive_not_reduction : Error<
+  "the list item must appear in 'reduction' clause with the 'inscan' modifier "
+  "of the parent directive">;
+def err_omp_reduction_not_inclusive_exclusive : Error<
+  "the inscan reduction list item must appear as a list item in an 'inclusive' 
or"
+  " 'exclusive' clause on an inner 'omp scan' directive">;
+def err_omp_wrong_inscan_reduction : Error<
+  "'inscan' modifier can be used only in 'omp for', 'omp simd', 'omp for 
simd',"
+  " 'omp parallel for', or 'omp parallel for simd' directive">;
+def err_omp_inscan_reduction_expected : Error<
+  "expected 'reduction' clause with the 'inscan' modifier">;
+def note_omp_previous_inscan_reduction : Note<
+  "'reduction' clause with 'inscan' modifier is used here">;
 def err_omp_expected_predefined_allocator : Error<
   "expected one of the predefined allocators for the variables with the static 
"
   "storage: 'omp_default_mem_alloc', 'omp_large_cap_mem_alloc', "

diff  --git a/clang/include/clang/Basic/OpenMPKinds.def 
b/clang/include/clang/Basic/OpenMPKinds.def
index bfb41ab105ea..3cf92ead9560 100644
--- a/clang/include/clang/Basic/OpenMPKinds.def
+++ b/clang/include/clang/Basic/OpenMPKinds.def
@@ -1112,6 +1112,7 @@ OPENMP_DEPOBJ_CLAUSE(update)
 
 // Modifiers for 'reduction' clause.
 OPENMP_REDUCTION_MODIFIER(default)
+OPENMP_REDUCTION_MODIFIER(inscan)
 
 #undef OPENMP_REDUCTION_MODIFIER
 #undef OPENMP_SCAN_CLAUSE

diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 11cc43a16db1..d0c304d37336 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -62,14 +62,15 @@ class DSAStackTy {
   struct DSAVarData {
 OpenMPDirectiveKind DKind = OMPD_unknown;
 OpenMPClauseKind CKind = OMPC_unknown;
+unsigned Modifier = 0;
 const Expr *RefExpr = nullptr;
 DeclRefExpr *PrivateCopy = nullptr;
 SourceLocation ImplicitDSALoc;
 DSAVarData() = default;
 DSAVarData(OpenMPDirectiveKind DKind, OpenMPClauseKind CKind,
const Expr *RefExpr, DeclRefExpr *PrivateCopy,
-

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

2020-03-26 Thread Michael Liao via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6a9ad5f3f4ac: [cuda][hip] Add CUDA builtin surface/texture 
reference support. (authored by hliao).

Changed prior to commit:
  https://reviews.llvm.org/D76365?vs=252828=252929#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76365

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/Type.cpp
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CGCUDARuntime.h
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/CodeGen/TargetInfo.h
  clang/lib/Headers/__clang_cuda_runtime_wrapper.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCUDA/surface.cu
  clang/test/CodeGenCUDA/texture.cu
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaCUDA/attr-declspec.cu
  clang/test/SemaCUDA/attributes-on-non-cuda.cu
  clang/test/SemaCUDA/bad-attributes.cu
  llvm/include/llvm/IR/Operator.h

Index: llvm/include/llvm/IR/Operator.h
===
--- llvm/include/llvm/IR/Operator.h
+++ llvm/include/llvm/IR/Operator.h
@@ -599,6 +599,25 @@
   }
 };
 
+class AddrSpaceCastOperator
+: public ConcreteOperator {
+  friend class AddrSpaceCastInst;
+  friend class ConstantExpr;
+
+public:
+  Value *getPointerOperand() { return getOperand(0); }
+
+  const Value *getPointerOperand() const { return getOperand(0); }
+
+  unsigned getSrcAddressSpace() const {
+return getPointerOperand()->getType()->getPointerAddressSpace();
+  }
+
+  unsigned getDestAddressSpace() const {
+return getType()->getPointerAddressSpace();
+  }
+};
+
 } // end namespace llvm
 
 #endif // LLVM_IR_OPERATOR_H
Index: clang/test/SemaCUDA/bad-attributes.cu
===
--- clang/test/SemaCUDA/bad-attributes.cu
+++ clang/test/SemaCUDA/bad-attributes.cu
@@ -70,3 +70,27 @@
 __device__ void device_fn() {
   __constant__ int c; // expected-error {{__constant__ variables must be global}}
 }
+
+typedef __attribute__((device_builtin_surface_type)) unsigned long long s0_ty; // expected-warning {{'device_builtin_surface_type' attribute only applies to classes}}
+typedef __attribute__((device_builtin_texture_type)) unsigned long long t0_ty; // expected-warning {{'device_builtin_texture_type' attribute only applies to classes}}
+
+struct __attribute__((device_builtin_surface_type)) s1_ref {}; // expected-error {{illegal device builtin surface reference type 's1_ref' declared here}}
+// expected-note@-1 {{'s1_ref' needs to be instantiated from a class template with proper template arguments}}
+struct __attribute__((device_builtin_texture_type)) t1_ref {}; // expected-error {{illegal device builtin texture reference type 't1_ref' declared here}}
+// expected-note@-1 {{'t1_ref' needs to be instantiated from a class template with proper template arguments}}
+
+template 
+struct __attribute__((device_builtin_surface_type)) s2_cls_template {}; // expected-error {{illegal device builtin surface reference class template 's2_cls_template' declared here}}
+// expected-note@-1 {{'s2_cls_template' needs to have exactly 2 template parameters}}
+template 
+struct __attribute__((device_builtin_texture_type)) t2_cls_template {}; // expected-error {{illegal device builtin texture reference class template 't2_cls_template' declared here}}
+// expected-note@-1 {{'t2_cls_template' needs to have exactly 3 template parameters}}
+
+template 
+struct __attribute__((device_builtin_surface_type)) s3_cls_template {}; // expected-error {{illegal device builtin surface reference class template 's3_cls_template' declared here}}
+// expected-note@-1 {{the 1st template parameter of 's3_cls_template' needs to be a type}}
+// expected-note@-2 {{the 2nd template parameter of 's3_cls_template' needs to be an integer or enum value}}
+template 
+struct __attribute__((device_builtin_texture_type)) t3_cls_template {}; // expected-error {{illegal device builtin texture reference class template 't3_cls_template' declared here}}
+// expected-note@-1 {{the 1st template parameter of 't3_cls_template' needs to be a type}}
+// expected-note@-2 {{the 3rd template parameter of 't3_cls_template' needs to be an integer or enum value}}
Index: clang/test/SemaCUDA/attributes-on-non-cuda.cu
===
--- clang/test/SemaCUDA/attributes-on-non-cuda.cu
+++ clang/test/SemaCUDA/attributes-on-non-cuda.cu
@@ -7,16 +7,19 @@
 // RUN: %clang_cc1 -DEXPECT_WARNINGS -fsyntax-only -verify -x c %s
 
 #if defined(EXPECT_WARNINGS)
-// expected-warning@+12 {{'device' attribute ignored}}
-//

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

2020-03-26 Thread Michael Liao via Phabricator via cfe-commits

hliao added a comment.

In D76365#1944272 , @tra wrote:

> LGTM. Next step is to figure out what various 
> __nv_tex_surf_handler(...) maps to for various strings (there are 
> ~110 of them in CUDA-10.2) and implement its replacement. I think we should 
> be able to do it in the wrapper file.

Besides the texture/surface functions for their reference types, we also need 
to add corresponding ones for surface/texture object types as well. Even there 
are many but most of them are straight-forward, I will do that in my spare 
time. Thanks for review.

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] D74324: Tools emit the bug report URL on crash

2020-03-26 Thread Leonard Chan via Phabricator via cfe-commits

leonardchan added a comment.

Hi. This patch seems to cause the undefined symbol error we're seeing when 
linking clang:

  FAILED: bin/clang-11 
  : && /b/s/w/ir/k/cipd/bin/clang++ --sysroot=/b/s/w/ir/k/cipd/linux-amd64  
-I/b/s/w/ir/k/recipe_cleanup/clangzmv99P/zlib_install/include -fPIC 
-fvisibility-inlines-hidden -Werror=date-time 
-Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
-Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default 
-Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wno-comment 
-Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections 
-ffile-prefix-map=/b/s/w/ir/k/recipe_cleanup/clangzmv99P/llvm_build_dir=../recipe_cleanup/clangzmv99P/llvm_build_dir
 -ffile-prefix-map=/b/s/w/ir/k/llvm-project/= -no-canonical-prefixes 
-fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3  -static-libstdc++ 
-fuse-ld=lld -Wl,--color-diagnostics -Wl,-allow-shlib-undefined-Wl,-O3 
-Wl,--gc-sections tools/clang/tools/driver/CMakeFiles/clang.dir/driver.cpp.o 
tools/clang/tools/driver/CMakeFiles/clang.dir/cc1_main.cpp.o 
tools/clang/tools/driver/CMakeFiles/clang.dir/cc1as_main.cpp.o 
tools/clang/tools/driver/CMakeFiles/clang.dir/cc1gen_reproducer_main.cpp.o  -o 
bin/clang-11  -Wl,-rpath,"\$ORIGIN/../lib" lib/libLLVMX86CodeGen.a 
lib/libLLVMX86AsmParser.a lib/libLLVMX86Desc.a lib/libLLVMX86Disassembler.a 
lib/libLLVMX86Info.a lib/libLLVMX86Utils.a lib/libLLVMARMCodeGen.a 
lib/libLLVMARMAsmParser.a lib/libLLVMARMDesc.a lib/libLLVMARMDisassembler.a 
lib/libLLVMARMInfo.a lib/libLLVMARMUtils.a lib/libLLVMAArch64CodeGen.a 
lib/libLLVMAArch64AsmParser.a lib/libLLVMAArch64Desc.a 
lib/libLLVMAArch64Disassembler.a lib/libLLVMAArch64Info.a 
lib/libLLVMAArch64Utils.a lib/libLLVMRISCVCodeGen.a lib/libLLVMRISCVAsmParser.a 
lib/libLLVMRISCVDesc.a lib/libLLVMRISCVDisassembler.a lib/libLLVMRISCVInfo.a 
lib/libLLVMRISCVUtils.a lib/libLLVMAnalysis.a lib/libLLVMCodeGen.a 
lib/libLLVMCore.a lib/libLLVMipo.a lib/libLLVMAggressiveInstCombine.a 
lib/libLLVMInstCombine.a lib/libLLVMInstrumentation.a lib/libLLVMMC.a 
lib/libLLVMMCParser.a lib/libLLVMObjCARCOpts.a lib/libLLVMOption.a 
lib/libLLVMScalarOpts.a lib/libLLVMSupport.a lib/libLLVMTransformUtils.a 
lib/libLLVMVectorize.a -lpthread lib/libclangBasic.a lib/libclangCodeGen.a 
lib/libclangDriver.a lib/libclangFrontend.a lib/libclangFrontendTool.a 
lib/libclangSerialization.a lib/libLLVMARMDesc.a lib/libLLVMARMInfo.a 
lib/libLLVMARMUtils.a lib/libLLVMCFGuard.a lib/libLLVMAArch64Desc.a 
lib/libLLVMAArch64Info.a lib/libLLVMAArch64Utils.a lib/libLLVMAsmPrinter.a 
lib/libLLVMDebugInfoDWARF.a lib/libLLVMGlobalISel.a lib/libLLVMSelectionDAG.a 
lib/libLLVMMCDisassembler.a lib/libclangCodeGen.a lib/libLLVMCoverage.a 
lib/libLLVMLTO.a lib/libLLVMObjCARCOpts.a lib/libLLVMPasses.a 
lib/libLLVMCodeGen.a lib/libLLVMTarget.a lib/libLLVMCoroutines.a 
lib/libLLVMipo.a lib/libLLVMInstrumentation.a lib/libLLVMVectorize.a 
lib/libLLVMBitWriter.a lib/libLLVMIRReader.a lib/libLLVMAsmParser.a 
lib/libLLVMLinker.a lib/libLLVMScalarOpts.a lib/libLLVMAggressiveInstCombine.a 
lib/libLLVMInstCombine.a lib/libclangRewriteFrontend.a 
lib/libclangStaticAnalyzerFrontend.a lib/libclangStaticAnalyzerCheckers.a 
lib/libclangStaticAnalyzerCore.a lib/libclangCrossTU.a lib/libclangIndex.a 
lib/libclangFrontend.a lib/libclangDriver.a lib/libLLVMOption.a 
lib/libclangParse.a lib/libclangSerialization.a lib/libclangSema.a 
lib/libclangAnalysis.a lib/libclangASTMatchers.a lib/libclangEdit.a 
lib/libclangFormat.a lib/libclangToolingInclusions.a lib/libclangToolingCore.a 
lib/libclangAST.a lib/libLLVMFrontendOpenMP.a lib/libLLVMTransformUtils.a 
lib/libLLVMAnalysis.a lib/libLLVMProfileData.a lib/libLLVMObject.a 
lib/libLLVMMCParser.a lib/libLLVMBitReader.a lib/libLLVMTextAPI.a 
lib/libclangRewrite.a lib/libclangLex.a lib/libclangBasic.a lib/libLLVMCore.a 
lib/libLLVMRemarks.a lib/libLLVMBitstreamReader.a lib/libLLVMMC.a 
lib/libLLVMBinaryFormat.a lib/libLLVMDebugInfoCodeView.a 
lib/libLLVMDebugInfoMSF.a lib/libLLVMSupport.a -lrt -ldl -lpthread -lm 
lib/libLLVMDemangle.a && :
  ld.lld: error: undefined symbol: llvm::setBugReportMsg(char const*)
  >>> referenced by driver.cpp
  >>>   
tools/clang/tools/driver/CMakeFiles/clang.dir/driver.cpp.o:(main)

Would you mind taking a look? Thanks.

Builder: 
https://luci-milo.appspot.com/p/fuchsia/builders/ci/clang-linux-x64/b8884769398556395840


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74324



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

[PATCH] D76653: [clang] Allow -DDEFAULT_SYSROOT to be a relative path

2020-03-26 Thread Petr Hosek via Phabricator via cfe-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76653



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

[PATCH] D76856: Fix TBAA for unsigned fixed-point types

2020-03-26 Thread Leonard Chan via Phabricator via cfe-commits

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

LGTM. Thanks for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76856



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

[PATCH] D73967: Implement _ExtInt as an extended int type specifier.

2020-03-26 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added a comment.

Ping!


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

https://reviews.llvm.org/D73967



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

[clang] 6a9ad5f - [cuda][hip] Add CUDA builtin surface/texture reference support.

2020-03-26 Thread Michael Liao via cfe-commits


Author: Michael Liao
Date: 2020-03-26T14:44:52-04:00
New Revision: 6a9ad5f3f4ac66f0cae592e911f4baeb6ee5eca6

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

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

Summary:
- Even though the bindless surface/texture interfaces are promoted,
  there are still code using surface/texture references. For example,
  [PR#26400](https://bugs.llvm.org/show_bug.cgi?id=26400) reports the
  compilation issue for code using `tex2D` with texture references. For
  better compatibility, this patch proposes the support of
  surface/texture references.
- Due to the absent documentation and magic headers, it's believed that
  `nvcc` does use builtins for texture support. From the limited NVVM
  documentation[^nvvm] and NVPTX backend texture/surface related
  tests[^test], it's believed that surface/texture references are
  supported by replacing their reference types, which are annotated with
  `device_builtin_surface_type`/`device_builtin_texture_type`, with the
  corresponding handle-like object types, `cudaSurfaceObject_t` or
  `cudaTextureObject_t`, in the device-side compilation. On the host
  side, that global handle variables are registered and will be
  established and updated later when corresponding binding/unbinding
  APIs are called[^bind]. Surface/texture references are most like
  device global variables but represented in different types on the host
  and device sides.
- In this patch, the following changes are proposed to support that
  behavior:
  + Refine `device_builtin_surface_type` and
`device_builtin_texture_type` attributes to be applied on `Type`
decl only to check whether a variable is of the surface/texture
reference type.
  + Add hooks in code generation to replace that reference types with
the correponding object types as well as all accesses to them. In
particular, `nvvm.texsurf.handle.internal` should be used to load
object handles from global reference variables[^texsurf] as well as
metadata annotations.
  + Generate host-side registration with proper template argument
parsing.

---
[^nvvm]: https://docs.nvidia.com/cuda/pdf/NVVM_IR_Specification.pdf
[^test]: 
https://raw.githubusercontent.com/llvm/llvm-project/master/llvm/test/CodeGen/NVPTX/tex-read-cuda.ll
[^bind]: See section 3.2.11.1.2 ``Texture reference API` in [CUDA C Programming 
Guide](https://docs.nvidia.com/cuda/pdf/CUDA_C_Programming_Guide.pdf).
[^texsurf]: According to NVVM IR, `nvvm.texsurf.handle` should be used.  But, 
the current backend doesn't have that supported. We may revise that later.

Reviewers: tra, rjmccall, yaxunl, a.sidorin

Subscribers: cfe-commits

Tags: #clang

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

Added: 
clang/test/CodeGenCUDA/surface.cu
clang/test/CodeGenCUDA/texture.cu

Modified: 
clang/include/clang/AST/Type.h
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/Type.cpp
clang/lib/CodeGen/CGCUDANV.cpp
clang/lib/CodeGen/CGCUDARuntime.h
clang/lib/CodeGen/CGExprAgg.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/CodeGenTypes.cpp
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/CodeGen/TargetInfo.h
clang/lib/Headers/__clang_cuda_runtime_wrapper.h
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/Misc/pragma-attribute-supported-attributes-list.test
clang/test/SemaCUDA/attr-declspec.cu
clang/test/SemaCUDA/attributes-on-non-cuda.cu
clang/test/SemaCUDA/bad-attributes.cu
llvm/include/llvm/IR/Operator.h

Removed: 




diff  --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index b8f49127bbd0..673d37109eb6 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -2111,6 +2111,11 @@ class alignas(8) Type : public ExtQualsTypeCommonBase {
   /// than implicitly __strong.
   bool isObjCARCImplicitlyUnretainedType() const;
 
+  /// Check if the type is the CUDA device builtin surface type.
+  bool isCUDADeviceBuiltinSurfaceType() const;
+  /// Check if the type is the CUDA device builtin texture type.
+  bool isCUDADeviceBuiltinTextureType() const;
+
   /// Return the implicit lifetime for this type, which must not be dependent.
   Qualifiers::ObjCLifetime getObjCARCImplicitLifetime() const;
 

diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index 5a90b2be2cbf..96bfdd313f47 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1064,16 +1064,20 @@ def CUDADeviceBuiltin : IgnoredAttr {
   let LangOpts = [CUDA];
 }
 
-def CUDADeviceBuiltinSurfaceType : IgnoredAttr {
+def

[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

Thanks!




Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4905
+  // A requires(){...} lets us infer members from each requirement.
+  for (const concepts::Requirement *Req : RE->getRequirements()) {
+if (!Req->isDependent())

kadircet wrote:
> nit s/concepts::Req../auto
I actually don't think the type is obvious here... before I got closely 
familiar with the concepts AST, I assumed these were expressions.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4910
+  // Do a full traversal so we get `foo` from `typename T::foo::bar`.
+  QualType AssertedType = TR->getType()->getType();
+  ValidVisitor(this, T).TraverseType(AssertedType);

kadircet wrote:
> TR->getType might fail if there was a substitution failure. check for it 
> before ?
> 
> if(TR->isSubstitutionFailure()) continue;
substitution failures aren't dependent, so we already bailed out in that case. 
Added a comment.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4984
+if (Q && isApprox(Q->getAsType(), T))
+  addType(NNS->getAsIdentifier());
+  }

kadircet wrote:
> as T is dependent i suppose NNS should always be an identifier, but would be 
> nice to spell it out explicitly with a comment.
It doesn't need to be an identifier - it returns null and addType handles null.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5007
+  if (/*Inserted*/ R.second ||
+  std::make_tuple(M.Operator, M.ArgTypes.hasValue(),
+  M.ResultType != nullptr) >

kadircet wrote:
> so we'll give up result in case of (syntax might be wrong):
> ```
> ... concept C requires { T::foo; { t.foo() }-> bar }
> ```
> 
> assuming we first traversed t.foo and then T::foo ? I would rather move 
> operator to the end.
Done. This won't ever happen, though.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5077
+  // variables associated with DC (as returned by getTemplatedEntity()).
+  static ::SmallVector
+  constraintsForTemplatedEntity(DeclContext *DC) {

kadircet wrote:
> s/::SmallVector/llvm::SmallVector
What the heck?

(preferred spelling seems to be SmallVector here)



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:5169
+  } else if (BaseType->isObjCObjectPointerType() ||
+ BaseType->isTemplateTypeParmType())
 /*Do nothing*/;

kadircet wrote:
> nit:
> ```
> // ObjcPointers(properties) and TTPT(concepts) are handled below, bail out 
> for the resst.
> else if (!objcPointer && ! TTPT) return false;
> ```
I actually find the 3 cases much harder to spot here, and there's no nice place 
to put the comment.
Added more braces and extended the comment, WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73649



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

[PATCH] D73649: [CodeComplete] Member completion for concept-constrained types.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits

sammccall updated this revision to Diff 252917.
sammccall marked 13 inline comments as done.
sammccall added a comment.

address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73649

Files:
  clang/include/clang/Sema/Scope.h
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/concepts.cpp

Index: clang/test/CodeCompletion/concepts.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/concepts.cpp
@@ -0,0 +1,59 @@
+template  concept convertible_to = true;
+template  concept same_as = true;
+template  concept integral = true;
+
+template 
+concept W = requires(A a, B b) {
+  { b.www } noexcept -> integral;
+};
+
+template  concept X = requires(T t) {
+  t.xxx(42);
+  typename T::xxx_t;
+  T::xyz::member;
+};
+
+template 
+concept Y = requires(T t, U u) { t.yyy(u); };
+
+template 
+concept Z = requires(T t) {
+  { t.zzz() } -> same_as;
+  requires W;
+};
+
+// Concept constraints in all three slots require X, Y, Z, and ad-hoc stuff.
+template 
+requires Y && requires(T *t) { { t->aaa() } -> convertible_to; }
+void foo(T t) requires Z || requires(T ) { t.bbb(); t->bb(); } {
+  t.x;
+  t->x;
+  T::x;
+
+  // RUN: %clang_cc1 -std=c++2a -code-completion-with-fixits -code-completion-at=%s:29:5 %s \
+  // RUN: | FileCheck %s -check-prefix=DOT -implicit-check-not=xxx_t
+  // DOT: Pattern : [#convertible_to#]aaa()
+  // DOT: Pattern : bb() (requires fix-it: {{.*}} to "->")
+  // DOT: Pattern : bbb()
+  // DOT: Pattern : [#integral#]www
+  // DOT: Pattern : xxx(<#int#>)
+  // FIXME: it would be nice to have int instead of U here.
+  // DOT: Pattern : yyy(<#U#>)
+  // DOT: Pattern : [#int#]zzz()
+
+  // RUN: %clang_cc1 -std=c++2a -code-completion-with-fixits -code-completion-at=%s:30:6 %s \
+  // RUN: | FileCheck %s -check-prefix=ARROW -implicit-check-not=xxx_t
+  // ARROW: Pattern : [#convertible_to#]aaa() (requires fix-it: {{.*}} to ".")
+  // ARROW: Pattern : bb()
+  // ARROW: Pattern : bbb() (requires fix-it
+  // ARROW: Pattern : [#integral#]www (requires fix-it
+  // ARROW: Pattern : xxx(<#int#>) (requires fix-it
+  // ARROW: Pattern : yyy(<#U#>) (requires fix-it
+  // ARROW: Pattern : [#int#]zzz() (requires fix-it
+
+  // RUN: %clang_cc1 -std=c++2a -code-completion-with-fixits -code-completion-at=%s:31:6 %s \
+  // RUN: | FileCheck %s -check-prefix=COLONS -implicit-check-not=yyy
+  // COLONS: Pattern : xxx_t
+  // COLONS: Pattern : xyz
+}
+
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -9,6 +9,7 @@
 //  This file defines the code-completion semantic actions.
 //
 //===--===//
+#include "clang/AST/ASTConcept.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
@@ -16,8 +17,11 @@
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/ExprConcepts.h"
 #include "clang/AST/ExprObjC.h"
+#include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/QualTypeNames.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/Specifiers.h"
@@ -4746,6 +4750,369 @@
   return nullptr;
 }
 
+namespace {
+// Collects completion-relevant information about a concept-constrainted type T.
+// In particular, examines the constraint expressions to find members of T.
+//
+// The design is very simple: we walk down each constraint looking for
+// expressions of the form T.foo().
+// If we're extra lucky, the return type is specified.
+// We don't do any clever handling of && or || in constraint expressions, we
+// take members from both branches.
+//
+// For example, given:
+//   template  concept X = requires (T t, string& s) { t.print(s); };
+//   template  void foo(U u) { u.^ }
+// We want to suggest the inferred member function 'print(string)'.
+// We see that u has type U, so X holds.
+// X requires t.print(s) to be valid, where t has type U (substituted for T).
+// By looking at the CallExpr we find the signature of print().
+//
+// While we tend to know in advance which kind of members (access via . -> ::)
+// we want, it's simpler just to gather them all and post-filter.
+//
+// FIXME: some of this machinery could be used for non-concept type-parms too,
+// enabling completion for type parameters based on other uses of that param.
+//
+// FIXME: there are other cases where a type can be constrained by a concept,
+// e.g. inside `if constexpr(ConceptSpecializationExpr) { ... }`
+class ConceptInfo {
+public:
+  // Describes a likely member of a type, inferred by concept constraints.
+  // Offered as a code completion for

[PATCH] D76653: [clang] Allow -DDEFAULT_SYSROOT to be a relative path

2020-03-26 Thread Sam Clegg via Phabricator via cfe-commits

sbc100 updated this revision to Diff 252909.
sbc100 added a comment.

feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76653

Files:
  clang/CMakeLists.txt
  clang/lib/Driver/Driver.cpp


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -140,6 +140,13 @@
   Dir = std::string(llvm::sys::path::parent_path(ClangExecutable));
   InstalledDir = Dir; // Provide a sensible default installed dir.
 
+  if ((!SysRoot.empty()) && llvm::sys::path::is_relative(SysRoot)) {
+// Prepend InstalledDir if SysRoot is relative
+SmallString<128> P(InstalledDir);
+llvm::sys::path::append(P, SysRoot);
+SysRoot = std::string(P);
+  }
+
 #if defined(CLANG_CONFIG_FILE_SYSTEM_DIR)
   SystemConfigDir = CLANG_CONFIG_FILE_SYSTEM_DIR;
 #endif
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -226,7 +226,7 @@
   "Colon separated list of directories clang will search for headers.")
 
 set(GCC_INSTALL_PREFIX "" CACHE PATH "Directory where gcc is installed." )
-set(DEFAULT_SYSROOT "" CACHE PATH
+set(DEFAULT_SYSROOT "" CACHE STRING
   "Default  to all compiler invocations for --sysroot=." )
 
 set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld")


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -140,6 +140,13 @@
   Dir = std::string(llvm::sys::path::parent_path(ClangExecutable));
   InstalledDir = Dir; // Provide a sensible default installed dir.
 
+  if ((!SysRoot.empty()) && llvm::sys::path::is_relative(SysRoot)) {
+// Prepend InstalledDir if SysRoot is relative
+SmallString<128> P(InstalledDir);
+llvm::sys::path::append(P, SysRoot);
+SysRoot = std::string(P);
+  }
+
 #if defined(CLANG_CONFIG_FILE_SYSTEM_DIR)
   SystemConfigDir = CLANG_CONFIG_FILE_SYSTEM_DIR;
 #endif
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -226,7 +226,7 @@
   "Colon separated list of directories clang will search for headers.")
 
 set(GCC_INSTALL_PREFIX "" CACHE PATH "Directory where gcc is installed." )
-set(DEFAULT_SYSROOT "" CACHE PATH
+set(DEFAULT_SYSROOT "" CACHE STRING
   "Default  to all compiler invocations for --sysroot=." )
 
 set(ENABLE_LINKER_BUILD_ID OFF CACHE BOOL "pass --build-id to ld")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76663: [clangd] Support new semanticTokens request from LSP 3.16.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:452
+  const HighlightingToken *Last = nullptr;
+  for (const HighlightingToken  : Tokens) {
+Result.emplace_back();

hokein wrote:
> note that we don't calculate the column offset for `InactiveCode` token 
> (`{(line, 0), (line, 0)}`), I think we probably calculate the range with the 
> new implementation, maybe add a FIXME.
Right, yes. Just skipping over them for now, with a fixme.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76663



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

[PATCH] D76663: [clangd] Support new semanticTokens request from LSP 3.16.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits

sammccall updated this revision to Diff 252906.
sammccall marked 6 inline comments as done.
sammccall added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76663

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -720,6 +720,41 @@
   ASSERT_EQ(Counter.Count, 1);
 }
 
+TEST(SemanticHighlighting, toSemanticTokens) {
+  auto CreatePosition = [](int Line, int Character) -> Position {
+Position Pos;
+Pos.line = Line;
+Pos.character = Character;
+return Pos;
+  };
+
+  std::vector Tokens = {
+  {HighlightingKind::Variable,
+   Range{CreatePosition(1, 1), CreatePosition(1, 5)}},
+  {HighlightingKind::Function,
+   Range{CreatePosition(3, 4), CreatePosition(3, 7)}},
+  {HighlightingKind::Variable,
+   Range{CreatePosition(3, 8), CreatePosition(3, 12)}},
+  };
+
+  std::vector Results = toSemanticTokens(Tokens);
+  EXPECT_EQ(Tokens.size(), Results.size());
+  EXPECT_EQ(Results[0].tokenType, unsigned(HighlightingKind::Variable));
+  EXPECT_EQ(Results[0].deltaLine, 1u);
+  EXPECT_EQ(Results[0].deltaStart, 1u);
+  EXPECT_EQ(Results[0].length, 4u);
+
+  EXPECT_EQ(Results[1].tokenType, unsigned(HighlightingKind::Function));
+  EXPECT_EQ(Results[1].deltaLine, 2u);
+  EXPECT_EQ(Results[1].deltaStart, 4u);
+  EXPECT_EQ(Results[1].length, 3u);
+
+  EXPECT_EQ(Results[2].tokenType, unsigned(HighlightingKind::Variable));
+  EXPECT_EQ(Results[2].deltaLine, 0u);
+  EXPECT_EQ(Results[2].deltaStart, 4u);
+  EXPECT_EQ(Results[2].length, 4u);
+}
+
 TEST(SemanticHighlighting, toTheiaSemanticHighlightingInformation) {
   auto CreatePosition = [](int Line, int Character) -> Position {
 Position Pos;
Index: clang-tools-extra/clangd/test/semantic-tokens.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/semantic-tokens.test
@@ -0,0 +1,22 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.cpp","languageId":"cpp","text":"int x = 2;"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/semanticTokens","params":{"textDocument":{"uri":"test:///foo.cpp"}}}
+# CHECK:   "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": {
+# CHECK-NEXT:"data": [
+#  First line, char 5, variable, no modifiers.
+# CHECK-NEXT:  0,
+# CHECK-NEXT:  4,
+# CHECK-NEXT:  1,
+# CHECK-NEXT:  0,
+# CHECK-NEXT:  0
+# CHECK-NEXT:]
+# CHECK-NEXT:  }
+---
+{"jsonrpc":"2.0","id":2,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
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
@@ -38,6 +38,16 @@
 # CHECK-NEXT:  "referencesProvider": true,
 # CHECK-NEXT:  "renameProvider": true,
 # CHECK-NEXT:  "selectionRangeProvider": true,
+# CHECK-NEXT:  "semanticTokensProvider": {
+# CHECK-NEXT:"documentProvider": true,
+# CHECK-NEXT:"legend": {
+# CHECK-NEXT:  "tokenModifiers": [],
+# CHECK-NEXT:  "tokenTypes": [
+# CHECK-NEXT:"variable",
+# CHECK:   ]
+# CHECK-NEXT:},
+# CHECK-NEXT:"rangeProvider": false
+# CHECK-NEXT:  },
 # CHECK-NEXT:  "signatureHelpProvider": {
 # CHECK-NEXT:"triggerCharacters": [
 # CHECK-NEXT:  "(",
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -6,8 +6,21 @@
 //
 //===--===//
 //
-// An implementation of semantic highlighting based on this proposal:
-// https://github.com/microsoft/vscode-languageserver-node/pull/367 in clangd.
+// This file supports semantic highlighting:

[PATCH] D76653: [clang] Allow -DDEFAULT_SYSROOT to be a relative path

2020-03-26 Thread Sam Clegg via Phabricator via cfe-commits

sbc100 marked 3 inline comments as done.
sbc100 added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:143
 
+  if (llvm::sys::path::is_relative(SysRoot)) {
+// Prepend InstalledDir if SysRoot is relative

phosek wrote:
> Does this return `true` or `false` when sysroot is empty?
Good questions.  As it happens it looks like it returns true, which is not the 
result we want here.  Added explicit check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76653



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

[PATCH] D76653: [clang] Allow -DDEFAULT_SYSROOT to be a relative path

2020-03-26 Thread Petr Hosek via Phabricator via cfe-commits

phosek added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:143
 
+  if (llvm::sys::path::is_relative(SysRoot)) {
+// Prepend InstalledDir if SysRoot is relative

Does this return `true` or `false` when sysroot is empty?



Comment at: clang/lib/Driver/Driver.cpp:145
+// Prepend InstalledDir if SysRoot is relative
+SmallString<128> fullpath(InstalledDir);
+llvm::sys::path::append(fullpath, SysRoot);

Nit: according to LLVM conventions, this should `FullPath` (or just `P` which 
is also very common temporary path variables).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76653



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

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

@ebevhan Thanks again for the testcase. I've added a reduced version in 
47e7bdb10732e6f140adce39a1bc34e3ee2a6ea6 
 to ensure 
this is fixed on re-land.
This particular case can be fixed by marking the decl as invalid I think, I'm 
curious how this generalizes.

In D76696#1944014 , 
@hubert.reinterpretcast wrote:

> We have also encountered crashes in downstream testing caused by this using 
> just the vanilla source from trunk. When there is a proposed fix, please let 
> us know so we can test. Thanks.

We can try but it would be very useful to have details, no idea whether it's 
going to be the same issue and the more we know about the easier it is to 
improve the design.
Can you describe the case at all? Any chance of a minimal example? Ideally we'd 
check it in.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76696

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

[PATCH] D74324: Tools emit the bug report URL on crash

2020-03-26 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added inline comments.



Comment at: llvm/lib/Support/PrettyStackTrace.cpp:145
 
+static const char* customBugReportMsg;
+

MaskRay wrote:
> jhenderson wrote:
> > jhenderson wrote:
> > > `static const char *BugReportMsg;`
> > > 
> > > Why not just have this set to the default in the first place, and 
> > > overwrite it if somebody needs to? That'll remove the need for the new 
> > > `if` in the `CrashHandler` function.
> > Again, this hasn't been properly addresssed. This should use an upper-case 
> > letter for the first letter of variable names. Please see [[ 
> > https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
> >  | the coding standards ]].
> `static const char BugReportMsg[] = ...`
> 
> `static const char *BugReportMsg` defines a writable variable.
> 
> In IR, the former does not have the unnamed_addr attribute while the latter 
> has. The latter can lower to .rodata.str1.1 (SHF_MERGE | SHF_STRINGS) which 
> can be optimized by the linker. This is nuance and the optimization probably 
> weighs nothing (it is hardly to think there will be another thing having the 
> same byte sequence.) The former just seems more correct.
Nevermind. I did not see `setBugReportMsg`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74324



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

[PATCH] D76772: [AMDGPU] Add __builtin_amdgcn_workgroup_size_x/y/z

2020-03-26 Thread Yaxun Liu via Phabricator via cfe-commits

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



Comment at: clang/test/CodeGenCUDA/amdgpu-workgroup-size.cu:2
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa \
+// RUN: -fcuda-is-device -emit-llvm -o - -x hip %s \
+// RUN: | FileCheck %s

arsenm wrote:
> I assume the addrspacecast got optimized out? Should this disable llvm passes?
We did not emit addrspacecast here since we only need return the loaded value.

HIP by default uses -O0, therefore no need to disable llvm passes.


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

https://reviews.llvm.org/D76772



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

[clang] 47e7bdb - Test that would have caught recovery-expr crashes in 0788acbccbec. NFC

2020-03-26 Thread Sam McCall via cfe-commits


Author: Sam McCall
Date: 2020-03-26T18:43:29+01:00
New Revision: 47e7bdb10732e6f140adce39a1bc34e3ee2a6ea6

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

LOG: Test that would have caught recovery-expr crashes in 0788acbccbec. NFC

Added: 
clang/test/Sema/invalid-member.cpp

Modified: 


Removed: 




diff  --git a/clang/test/Sema/invalid-member.cpp 
b/clang/test/Sema/invalid-member.cpp
new file mode 100644
index ..5475157e936e
--- /dev/null
+++ b/clang/test/Sema/invalid-member.cpp
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+void foo(); // expected-note {{requires 0 arguments}}
+class X {
+  decltype(foo(42)) invalid; // expected-error {{no matching function}}
+};
+// Should be able to evaluate sizeof without crashing.
+static_assert(sizeof(X) == 1, "No valid members");



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

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

2020-03-26 Thread Jason Liu via Phabricator via cfe-commits

jasonliu added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4205
 
-CharUnits PPC32_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const {
+CharUnits PowerPC32ABIInfo::getParamTypeAlignment(QualType Ty) const {
   // Complex types are passed just like their elements

sfertile wrote:
> jasonliu wrote:
> > So for AIX we are taking PowerPC32ABIInfo right now. And we know EmitVAArg 
> > of this target does the right thing on AIX after the change. 
> > But for other functions, for example, getParamTypeAlignment, 
> > initDwarfEHRegSizeTable... Are we sure it's doing the right thing on AIX?
> > If we are not sure, is there anything we want to do (etc, put a comment on 
> > where it gets used or at the function definition)? Or are we fine to just 
> > leave it as it is and have a TODO in our head?
> Looking at the values in `initDwarfEHRegSizeTable` it has the right sizes for 
> all the registers. Even though the OS is different the underlying hardware is 
> the same. I'm not sure it's something that makes sense to support for AIX 
> though, in which case I think its valid to return `true` to indicate its not 
> supported. 
> 
> `getParamTypeAlignment` is used only in the context of the alignment for 
> vaarg, we should make sure its correct for AIX since supporting vaarg is the 
> scope of this patch.
In that case, is it better if we have lit test to actually exercise the path in 
getParamTypeAlignment to show that we actually confirmed the behavior is 
correct for AIX? And if it is some path we do not care for now(ComplexType? 
VectorType?), then we would want TODOs on them to show we need further 
investigation later. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76360



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

[PATCH] D74324: Tools emit the bug report URL on crash

2020-03-26 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added inline comments.

Comment at: llvm/lib/Support/PrettyStackTrace.cpp:145

+static const char* customBugReportMsg;
+

jhenderson wrote:
> jhenderson wrote:
> > `static const char *BugReportMsg;`
> > 
> > Why not just have this set to the default in the first place, and overwrite 
> > it if somebody needs to? That'll remove the need for the new `if` in the 
> > `CrashHandler` function.
> Again, this hasn't been properly addresssed. This should use an upper-case 
> letter for the first letter of variable names. Please see [[ 
> https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
>  | the coding standards ]].
`static const char BugReportMsg[] = ...`

`static const char *BugReportMsg` defines a writable variable.

In IR, the former does not have the unnamed_addr attribute while the latter 
has. The latter can lower to .rodata.str1.1 (SHF_MERGE | SHF_STRINGS) which can 
be optimized by the linker. This is nuance and the optimization probably weighs 
nothing (it is hardly to think there will be another thing having the same byte 
sequence.) The former just seems more correct.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74324

___
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.

2020-03-26 Thread Artem Belevich via Phabricator via cfe-commits

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

LGTM. Next step is to figure out what various 
__nv_tex_surf_handler(...) maps to for various strings (there are ~110 
of them in CUDA-10.2) and implement its replacement. I think we should be able 
to do it in the wrapper file.




Comment at: clang/lib/Headers/__clang_cuda_runtime_wrapper.h:82-94
 #undef __CUDACC__
 #if CUDA_VERSION < 9000
 #define __CUDABE__
 #else
+#define __CUDACC__
 #define __CUDA_LIBDEVICE__
 #endif

hliao wrote:
> tra wrote:
> > hliao wrote:
> > > hliao wrote:
> > > > tra wrote:
> > > > > hliao wrote:
> > > > > > tra wrote:
> > > > > > > Please add comments on why __CUDACC__ is needed for 
> > > > > > > driver_types.h here? AFAICT, driver_types.h does not have any 
> > > > > > > conditionals that depend on __CUDACC__. What happens if it's not 
> > > > > > > defined.
> > > > > > > 
> > > > > > > 
> > > > > > `driver_types.h` includes `host_defines.h`, where macros 
> > > > > > `__device_builtin_surface_type__` and 
> > > > > > `__device_builtin_texture_type__` are conditional defined if 
> > > > > > `__CUDACC__`.
> > > > > > 
> > > > > > The following is extracted from `cuda/crt/host_defines.h`
> > > > > > 
> > > > > > ```
> > > > > > #if !defined(__CUDACC__)
> > > > > > #define __device_builtin__
> > > > > > #define __device_builtin_texture_type__
> > > > > > #define __device_builtin_surface_type__
> > > > > > #define __cudart_builtin__
> > > > > > #else /* defined(__CUDACC__) */
> > > > > > #define __device_builtin__ \
> > > > > > __location__(device_builtin)
> > > > > > #define __device_builtin_texture_type__ \
> > > > > > __location__(device_builtin_texture_type)
> > > > > > #define __device_builtin_surface_type__ \
> > > > > > __location__(device_builtin_surface_type)
> > > > > > #define __cudart_builtin__ \
> > > > > > __location__(cudart_builtin)
> > > > > > #endif /* !defined(__CUDACC__) */
> > > > > > ```
> > > > > My concern is -- what else is going to get defined? There are ~60 
> > > > > references to __CUDACC__ in CUDA-10.1 headers. The wrappers are 
> > > > > fragile enough that there's a good chance something may break. It 
> > > > > does not help that my CUDA build bot decided to die just after we 
> > > > > switched to work-from-home, so there will be no early warning if 
> > > > > something goes wrong.
> > > > > 
> > > > > If all we need are the macros above, we may just define them. 
> > > > Let me check all CUDA SDK through their dockers. Redefining sounds good 
> > > > me as wll.
> > > I checked headers from 7.0 to 10.0, `__device_builtin_texture_type__` and 
> > > `__builtin_builtin_surface_type__` are only defined with that attributes 
> > > if `__CUDACC__` is defined. As we only pre-define `__CUDA_ARCH__` in 
> > > clang but flip `__CUDACC__` on and off in the wrapper headers to 
> > > selectively reuse CUDA's headers. I would hear your suggestion on that.
> > > BTW, macros like `__device__` are defined regardless of `__CUDACC__` from 
> > > 7.0 to 10.0 as `__location(device)`. `__location__` is defined if 
> > > `__CUDACC__` is present. But, different from `__device__`, 
> > > `__device_builtin_texture_type__` is defined only `__CUDACC__` is defined.
> > `__device_builtin_texture_type__` is defined in `host_defines.h`, which 
> > does not seem to include any other files or does anything suspicious with 
> > `__CUDACC__`
> > 
> > It may be OK to move inclusion of `host_defines.h` to the point before 
> > `driver_types.h`, which happens to include the host_defines.h first, and 
> > define __CUDACC__ only around `host_defines.h`.
> > 
> > An alternative is to add the macros just after inclusion of `host_defines.h`
> > 
> > In either case please verify that these attributes are the only things 
> > that's changed by diffing output of `clang++ -x cuda /dev/null 
> > --cuda-host-only -dD -E -o -` before and after the change.
> With `__CUDACC__`, the only difference is the additional attributes added, 
> such as `device_builtin_texture_type`. Attributes like `cudart_builtin` are 
> also defined correctly. That should be used to start the support CUDART 
> features.
> I revised the change to include `host_defines.h` first and found there's no 
> changes from the one using `driver_types.h`. We should be OK for that change.
SGTM. Thank you for verifying this.


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] D74934: [Clang interpreter] Rename Block.{h,cpp} to InterpBlock.{h,cpp}

2020-03-26 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added a comment.

@gribozavr2 - hey, an annoying quirk of Phabricator is that it doesn't send 
mail to the mailing list on state changes with no text, which means reviews 
like these look like tehy were committed without approval (on the mailing list 
there's no record of the approval). In the future, can you add some comment to 
your approval ("Looks good", "thanks", etc) to ensure mail is sent to the 
mailing list to document that approval there?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74934



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

[PATCH] D76725: [clangd] Build ASTs only with fresh preambles or after building a new preamble

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:8
 
//===--===//
 // For each file, managed by TUScheduler, we create a single ASTWorker that
+// manages an AST and a preamble for that file. All operations that modify or

This is explaining a pretty complicated thing, so I think it's particularly 
important to clearly organize the explanation, use consistent terminology, and 
avoid including unneccesary details.

I'd suggest introducing with paragraphs in this order:

- TUScheduler and the idea of a worker per TU.
- ASTWorker thread, the queue of interleaved updates and reads, elision of dead 
updates.
- Compatibility of preambles and updates, picky reads. 
- PreambleWorker thread, elision of preambles, golden ASTs.
- Published ASTs, epochs/monotonicity.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:11
+// read the AST are run on a separate dedicated thread asynchronously in FIFO
+// order. There is a second thread for preamble builds, ASTWorker thread issues
+// updates on that preamble thread as preamble becomes stale.

nit: name the other thread (there is a second PreambleWorker thread ...)



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:12
+// order. There is a second thread for preamble builds, ASTWorker thread issues
+// updates on that preamble thread as preamble becomes stale.
+//

"issues updates" is vague. What about "... enqueues rebuilds on the 
PreambleWorker thread as preamble becomes stale. Currently, it then immediately 
blocks on that request."



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:19
+// FIXME: Get rid of the synchronization between ASTWorker::update and
+// PreambleThread builds by using patched ASTs instead of compatible preambles
+// for reads. Only keep the blocking behaviour for picky read requests, e.g.

You haven't defined "compatible" anywhere, and it's not obvious what it means.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:23
+//
+// Updates are processed in two phases, by issuing a preamble and an ast build.
+// The preamble build gets issued into a different worker and might be

The wording here (particularly the word "phases") implies sequencing: an update 
results in a preamble build followed by an ast build in sequence, when in fact 
it usually results in an ast build and preamble build in parallel (the former 
usually finishing first) with a second ast build sequenced after the preamble 
build.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:25
+// The preamble build gets issued into a different worker and might be
+// overwritten by subsequent updates. An AST will be built for an update if
+// latest built preamble is compatible for it or a new preamble gets built for

This isn't true, an AST is built for an update if it is needed (a read is based 
on it, wantdiagnostics=yes, it changed the preamble, or wantdiagnostics=maybe 
and the debounce expired)



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:29
+// AST builds are always executed on ASTWorker thread, either immediately
+// following an update request with a compatible preamble, or through a new
+// request, inserted at front of the queue, from PreambleThread after building 
a

I'm not sure what compatible means here, but we will certainly build ASTs for 
incompatible preambles.
(Or is this describing the intermediate state as of this patch, with the plan 
to rewrite it all next patch? I think we should rather describe the new model, 
and then document the current deviations from it)



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:29
+// AST builds are always executed on ASTWorker thread, either immediately
+// following an update request with a compatible preamble, or through a new
+// request, inserted at front of the queue, from PreambleThread after building 
a

sammccall wrote:
> I'm not sure what compatible means here, but we will certainly build ASTs for 
> incompatible preambles.
> (Or is this describing the intermediate state as of this patch, with the plan 
> to rewrite it all next patch? I think we should rather describe the new 
> model, and then document the current deviations from it)
I'm not sure what "immediately following" is relative to: if it's after the 
update() call, it's not immediate - it has to wait in the queue.



Comment at: clang-tools-extra/clangd/TUScheduler.cpp:33
+//
+// Diagnostics and index is updated as a result of building AST for write
+// requests. Progress on those updates are ensured with golden ASTs, as they'll

"and *the* index *are* updated"... "building *the* AST"



Comment at:

[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1687
+  if (ArgValKnown) {
+if (!KernelZeroSizePtrValue)
+  KernelZeroSizePtrValue =

Szelethus wrote:
> martong wrote:
> > balazske wrote:
> > > martong wrote:
> > > > martong wrote:
> > > > > This is a bit confusing for me. Perhaps alternatively we could have a 
> > > > > free function `isInitialized(KernelZero...)` instead. Or maybe having 
> > > > > a separate bool variable to indicate whether it was initialized could 
> > > > > be cleaner?
> > > > Another idea: Adding a helper struct to contain the bool `initialized`? 
> > > > E.g. (draft):
> > > > ```
> > > > struct LazyOptional {
> > > >   bool initialized = false;
> > > >   Opt value;
> > > >   Opt& get();
> > > >   void set(const Opt&);
> > > > };
> > > > ```
> > > It may be OK to have a function `lazyInitKernelZeroSizePtrValue`?
> > I don't insist, given we have a better described type for 
> > `KernelZeroSizePtrValue` (e.g. having a `using` `template`)
> `AnalysisManager` has access to the `Preprocessor`, and it is also 
> responsible for the construction of the `CheckerManager` object. This would 
> make moving such code to the checker registry function! I'll gladly take this 
> issue off your hand and patch it in once 
> rG2aac0c47aed8d1eff7ab6d858173c532b881d948 settles :)
Just pushed rG4dc8472942ce60f29de9587b9451cc3d49df7abf. It it settles (no 
buildbots complain), you could rebase and the lazy initialization could be a 
thing of the past! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830



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

[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added inline comments.



Comment at: clang/test/Analysis/kmalloc-linux-1.c:15
+
+// FIXME: malloc checker expects kfree with 2 arguments, is this correct?
+// (recent kernel source code contains a `kfree(const void *)`)

balazske wrote:
> Szelethus wrote:
> > martong wrote:
> > > Do you plan to sniff around a bit about the arguments (as part of another 
> > > patch)? Would be good to handle both old and new signature kernel 
> > > allocation functions.
> > I'll take a quick look as well, I am quite familiar with MallocChecker.
> I found that `kfree` has one argument, not two (even in the 2.6 kernel). 
> Probably argument count was wrong in the last change when `CallDescription` 
> was introduced.
Yup, you're totally right, this was on me :) I'll get that fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830



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

[PATCH] D76416: [WIP][ASan] Apply -ffile-prefix-map mappings to ASan instrumentation

2020-03-26 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

> Putting this on hold for now. Although this implementation works for ASan, it 
> would be have to be repeated for other tools like SourceBasedCoverage or 
> other sanitizers.

(You can click "Add Action..."->"Plan Changes" so that this Differential will 
disappear from others' review list for a while..)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76416



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

[PATCH] D76862: HIP: Ensure new denormal mode attributes are set

2020-03-26 Thread Matt Arsenault via Phabricator via cfe-commits

arsenm created this revision.
arsenm added reviewers: yaxunl, gregrodgers.
Herald added subscribers: kerbowa, tpr, nhaehnle, wdng, jvesely.

Apparently HIPToolChain does not subclass from AMDGPUToolChain, so
this was not applying the new denormal attributes. I'm not sure why
this doesn't subclass. Just copy the implementation for now.


https://reviews.llvm.org/D76862

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h
  clang/lib/Driver/ToolChains/HIP.cpp
  clang/lib/Driver/ToolChains/HIP.h
  clang/test/Driver/cuda-flush-denormals-to-zero.cu

Index: clang/test/Driver/cuda-flush-denormals-to-zero.cu
===
--- clang/test/Driver/cuda-flush-denormals-to-zero.cu
+++ clang/test/Driver/cuda-flush-denormals-to-zero.cu
@@ -7,6 +7,16 @@
 // RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c -march=haswell --cuda-gpu-arch=sm_70 -fcuda-flush-denormals-to-zero -nocudainc -nocudalib %s 2>&1 | FileCheck -check-prefix=FTZ %s
 // RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c -march=haswell --cuda-gpu-arch=sm_70 -fno-cuda-flush-denormals-to-zero -nocudainc -nocudalib %s 2>&1 | FileCheck -check-prefix=NOFTZ %s
 
+// Test explicit argument.
+// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c -march=haswell --cuda-gpu-arch=gfx803 -fcuda-flush-denormals-to-zero -nocudainc -nogpulib %s 2>&1 | FileCheck -check-prefix=FTZ %s
+// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c -march=haswell --cuda-gpu-arch=gfx803 -fno-cuda-flush-denormals-to-zero -nocudainc -nogpulib %s 2>&1 | FileCheck -check-prefix=NOFTZ %s
+// RUN: %clang -x hip -no-canonical-prefixes -### -target x86_64-linux-gnu -c -march=haswell --cuda-gpu-arch=gfx900 -fcuda-flush-denormals-to-zero -nocudainc -nogpulib %s 2>&1 | FileCheck -check-prefix=FTZ %s
+// RUN: %clang -x hip -no-canonical-prefixes -### -target x86_64-linux-gnu -c -march=haswell --cuda-gpu-arch=gfx900 -fno-cuda-flush-denormals-to-zero -nocudainc -nogpulib %s 2>&1 | FileCheck -check-prefix=NOFTZ %s
+
+// Test the default changing with no argument based on the subtarget.
+// RUN: %clang -x hip -no-canonical-prefixes -### -target x86_64-linux-gnu -c -march=haswell --cuda-gpu-arch=gfx803 -nocudainc -nogpulib %s 2>&1 | FileCheck -check-prefix=FTZ %s
+// RUN: %clang -x hip -no-canonical-prefixes -### -target x86_64-linux-gnu -c -march=haswell --cuda-gpu-arch=gfx900 -nocudainc -nogpulib %s 2>&1 | FileCheck -check-prefix=NOFTZ %s
+
 // CPUFTZ-NOT: -fdenormal-fp-math
 
 // FTZ-NOT: -fdenormal-fp-math-f32=
Index: clang/lib/Driver/ToolChains/HIP.h
===
--- clang/lib/Driver/ToolChains/HIP.h
+++ clang/lib/Driver/ToolChains/HIP.h
@@ -115,6 +115,11 @@
 
   unsigned GetDefaultDwarfVersion() const override { return 4; }
 
+  llvm::DenormalMode getDefaultDenormalModeForType(
+const llvm::opt::ArgList ,
+Action::OffloadKind DeviceOffloadKind,
+const llvm::fltSemantics *FPType = nullptr) const override;
+
   const ToolChain 
 
 protected:
Index: clang/lib/Driver/ToolChains/HIP.cpp
===
--- clang/lib/Driver/ToolChains/HIP.cpp
+++ clang/lib/Driver/ToolChains/HIP.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "HIP.h"
+#include "AMDGPU.h"
 #include "CommonArgs.h"
 #include "InputInfo.h"
 #include "clang/Basic/Cuda.h"
@@ -16,6 +17,7 @@
 #include "clang/Driver/Options.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/TargetParser.h"
 
 using namespace clang::driver;
 using namespace clang::driver::toolchains;
@@ -272,6 +274,34 @@
   getProgramPaths().push_back(getDriver().Dir);
 }
 
+// FIXME: Duplicated in AMDGPUToolChain
+llvm::DenormalMode HIPToolChain::getDefaultDenormalModeForType(
+const llvm::opt::ArgList , Action::OffloadKind DeviceOffloadKind,
+const llvm::fltSemantics *FPType) const {
+  // Denormals should always be enabled for f16 and f64.
+  if (!FPType || FPType != ::APFloat::IEEEsingle())
+return llvm::DenormalMode::getIEEE();
+
+  if (DeviceOffloadKind == Action::OFK_Cuda) {
+if (FPType && FPType == ::APFloat::IEEEsingle() &&
+DriverArgs.hasFlag(options::OPT_fcuda_flush_denormals_to_zero,
+   options::OPT_fno_cuda_flush_denormals_to_zero,
+   false))
+  return llvm::DenormalMode::getPreserveSign();
+  }
+
+  const StringRef GpuArch = DriverArgs.getLastArgValue(options::OPT_mcpu_EQ);
+  auto Kind = llvm::AMDGPU::parseArchAMDGCN(GpuArch);
+
+  // TODO: There are way too many flags that change this. Do we need to check
+  // them all?
+  bool DAZ = DriverArgs.hasArg(options::OPT_cl_denorms_are_zero) ||
+!AMDGPUToolChain::getDefaultDenormsAreZeroForTarget(Kind);
+  // Outputs are flushed to zero, preserving sign
+

[clang] 4dc8472 - [analyzer] Add the Preprocessor to CheckerManager

2020-03-26 Thread Kirstóf Umann via cfe-commits


Author: Kirstóf Umann
Date: 2020-03-26T17:29:52+01:00
New Revision: 4dc8472942ce60f29de9587b9451cc3d49df7abf

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

LOG: [analyzer] Add the Preprocessor to CheckerManager

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
clang/unittests/StaticAnalyzer/Reusables.h

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h 
b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 4635ca35736a..f34f5c239290 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -122,9 +122,10 @@ enum class ObjCMessageVisitKind {
 };
 
 class CheckerManager {
-  ASTContext *Context;
+  ASTContext *Context = nullptr;
   const LangOptions LangOpts;
-  AnalyzerOptions 
+  const AnalyzerOptions 
+  const Preprocessor *PP = nullptr;
   CheckerNameRef CurrentCheckerName;
   DiagnosticsEngine 
   std::unique_ptr Registry;
@@ -137,15 +138,16 @@ class CheckerManager {
   // dependencies look like this: Core -> Checkers -> Frontend.
 
   CheckerManager(
-  ASTContext , AnalyzerOptions ,
+  ASTContext , AnalyzerOptions , const Preprocessor ,
   ArrayRef plugins,
   ArrayRef> checkerRegistrationFns);
 
   /// Constructs a CheckerManager that ignores all non TblGen-generated
   /// checkers. Useful for unit testing, unless the checker infrastructure
   /// itself is tested.
-  CheckerManager(ASTContext , AnalyzerOptions )
-  : CheckerManager(Context, AOptions, {}, {}) {}
+  CheckerManager(ASTContext , AnalyzerOptions ,
+ const Preprocessor )
+  : CheckerManager(Context, AOptions, PP, {}, {}) {}
 
   /// Constructs a CheckerManager without requiring an AST. No checker
   /// registration will take place. Only useful for retrieving the
@@ -163,7 +165,11 @@ class CheckerManager {
   void finishedCheckerRegistration();
 
   const LangOptions () const { return LangOpts; }
-  AnalyzerOptions () const { return AOptions; }
+  const AnalyzerOptions () const { return AOptions; }
+  const Preprocessor () const {
+assert(PP);
+return *PP;
+  }
   const CheckerRegistry () const { return *Registry; }
   DiagnosticsEngine () const { return Diags; }
   ASTContext () const {

diff  --git 
a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
index 6f8cb1432bb1..1af93cc45363 100644
--- 
a/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ 
b/clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -1485,7 +1485,7 @@ bool ento::shouldRegisterRetainCountBase(const 
LangOptions ) {
 // it should be possible to enable the NS/CF retain count checker as
 // osx.cocoa.RetainCount, and it should be possible to disable
 // osx.OSObjectRetainCount using osx.cocoa.RetainCount:CheckOSObject=false.
-static bool getOption(AnalyzerOptions ,
+static bool getOption(const AnalyzerOptions ,
   StringRef Postfix,
   StringRef Value) {
   auto I = Options.Config.find(

diff  --git 
a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
 
b/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
index 14f551403d98..562d553b8a84 100644
--- 
a/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ 
b/clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -608,7 +608,7 @@ std::string clang::ento::getVariableName(const FieldDecl 
*Field) {
 void ento::registerUninitializedObjectChecker(CheckerManager ) {
   auto Chk = Mgr.registerChecker();
 
-  AnalyzerOptions  = Mgr.getAnalyzerOptions();
+  const AnalyzerOptions  = Mgr.getAnalyzerOptions();
   UninitObjCheckerOptions  = Chk->Opts;
 
   ChOpts.IsPedantic = AnOpts.getCheckerBooleanOption(Chk, "Pedantic");

diff  --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp 
b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
index 6c123521e5f8..74c689730e58 100644
--- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -208,7 +208,7 @@ class AnalysisConsumer : public AnalysisASTConsumer,
 
   void Initialize(ASTContext ) override {
 Ctx = 
-checkerMgr = std::make_unique(*Ctx, *Opts,

[clang] 40076c1 - CUDA: Fix broken test run lines

2020-03-26 Thread Matt Arsenault via cfe-commits


Author: Matt Arsenault
Date: 2020-03-26T12:19:34-04:00
New Revision: 40076c14fef509d0304cbdad49ba64113f4816fd

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

LOG: CUDA: Fix broken test run lines

There was a misisng space between the -march and --cuda-gpu-arch
arguments, so --cuda-gpu-arch wasn't actually being parsed. I'm not
sure what the intent of the sm_10 run lines were, but they error as an
unsupported architecture. Switch these to something else.

Added: 


Modified: 
clang/test/Driver/cuda-flush-denormals-to-zero.cu

Removed: 




diff  --git a/clang/test/Driver/cuda-flush-denormals-to-zero.cu 
b/clang/test/Driver/cuda-flush-denormals-to-zero.cu
index 84ef358758fd..74f4bbc1585e 100644
--- a/clang/test/Driver/cuda-flush-denormals-to-zero.cu
+++ b/clang/test/Driver/cuda-flush-denormals-to-zero.cu
@@ -2,10 +2,10 @@
 // -fcuda-flush-denormals-to-zero. This should be translated to
 // -fdenormal-fp-math-f32=preserve-sign
 
-// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c 
-march=haswell--cuda-gpu-arch=sm_20 -fcuda-flush-denormals-to-zero -nocudainc 
-nocudalib %s 2>&1 | FileCheck -check-prefix=FTZ %s
-// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c 
-march=haswell--cuda-gpu-arch=sm_20 -fno-cuda-flush-denormals-to-zero 
-nocudainc -nocudalib %s 2>&1 | FileCheck -check-prefix=NOFTZ %s
-// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c 
-march=haswell--cuda-gpu-arch=sm_10 -fcuda-flush-denormals-to-zero -nocudainc 
-nocudalib %s 2>&1 | FileCheck -check-prefix=FTZ %s
-// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c 
-march=haswell--cuda-gpu-arch=sm_10 -fno-cuda-flush-denormals-to-zero 
-nocudainc -nocudalib %s 2>&1 | FileCheck -check-prefix=NOFTZ %s
+// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c 
-march=haswell --cuda-gpu-arch=sm_20 -fcuda-flush-denormals-to-zero -nocudainc 
-nocudalib %s 2>&1 | FileCheck -check-prefix=FTZ %s
+// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c 
-march=haswell --cuda-gpu-arch=sm_20 -fno-cuda-flush-denormals-to-zero 
-nocudainc -nocudalib %s 2>&1 | FileCheck -check-prefix=NOFTZ %s
+// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c 
-march=haswell --cuda-gpu-arch=sm_70 -fcuda-flush-denormals-to-zero -nocudainc 
-nocudalib %s 2>&1 | FileCheck -check-prefix=FTZ %s
+// RUN: %clang -no-canonical-prefixes -### -target x86_64-linux-gnu -c 
-march=haswell --cuda-gpu-arch=sm_70 -fno-cuda-flush-denormals-to-zero 
-nocudainc -nocudalib %s 2>&1 | FileCheck -check-prefix=NOFTZ %s
 
 // CPUFTZ-NOT: -fdenormal-fp-math
 



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

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Hubert Tong via Phabricator via cfe-commits

hubert.reinterpretcast added a comment.

We have also encountered crashes in downstream testing caused by this using 
just the vanilla source from trunk. When there is a proposed fix, please let us 
know so we can test. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76696



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

[PATCH] D76140: [InlineFunction] update attributes during inlining

2020-03-26 Thread Anna Thomas via Phabricator via cfe-commits

anna updated this revision to Diff 252868.
anna added a comment.

NFC w.r.t prev diff. Use VMap.lookup instead of a lambda which does the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76140

Files:
  clang/test/CodeGen/builtins-systemz-zvector.c
  clang/test/CodeGen/builtins-systemz-zvector2.c
  clang/test/CodeGen/movbe-builtins.c
  clang/test/CodeGen/rot-intrinsics.c
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/test/Transforms/Inline/ret_attr_update.ll

Index: llvm/test/Transforms/Inline/ret_attr_update.ll
===
--- /dev/null
+++ llvm/test/Transforms/Inline/ret_attr_update.ll
@@ -0,0 +1,112 @@
+; RUN: opt < %s -inline-threshold=0 -always-inline -S | FileCheck %s
+; RUN: opt < %s -passes=always-inline -S | FileCheck %s
+
+declare i8* @foo(i8*) argmemonly nounwind
+
+define i8* @callee(i8 *%p) alwaysinline {
+; CHECK: @callee(
+; CHECK: call i8* @foo(i8* noalias %p)
+  %r = call i8* @foo(i8* noalias %p)
+  ret i8* %r
+}
+
+define i8* @caller(i8* %ptr, i64 %x) {
+; CHECK-LABEL: @caller
+; CHECK: call nonnull i8* @foo(i8* noalias
+  %gep = getelementptr inbounds i8, i8* %ptr, i64 %x
+  %p = call nonnull i8* @callee(i8* %gep)
+  ret i8* %p
+}
+
+declare void @llvm.experimental.guard(i1,...)
+; Cannot add nonnull attribute to foo
+; because the guard is a throwing call
+define internal i8* @callee_with_throwable(i8* %p) alwaysinline {
+; CHECK-NOT: callee_with_throwable
+  %r = call i8* @foo(i8* %p)
+  %cond = icmp ne i8* %r, null
+  call void (i1, ...) @llvm.experimental.guard(i1 %cond) [ "deopt"() ]
+  ret i8* %r
+}
+
+declare i8* @bar(i8*) readonly nounwind
+; Here also we cannot add nonnull attribute to the call bar.
+define internal i8* @callee_with_explicit_control_flow(i8* %p) alwaysinline {
+; CHECK-NOT: callee_with_explicit_control_flow
+  %r = call i8* @bar(i8* %p)
+  %cond = icmp ne i8* %r, null
+  br i1 %cond, label %ret, label %orig
+
+ret:
+  ret i8* %r
+
+orig:
+  ret i8* %p
+}
+
+define i8* @caller2(i8* %ptr, i64 %x, i1 %cond) {
+; CHECK-LABEL: @caller2
+; CHECK: call i8* @foo
+; CHECK: call i8* @bar
+  %gep = getelementptr inbounds i8, i8* %ptr, i64 %x
+  %p = call nonnull i8* @callee_with_throwable(i8* %gep)
+  %q = call nonnull i8* @callee_with_explicit_control_flow(i8* %gep)
+  br i1 %cond, label %pret, label %qret
+
+pret:
+  ret i8* %p
+
+qret:
+  ret i8* %q
+}
+
+define internal i8* @callee3(i8 *%p) alwaysinline {
+; CHECK-NOT: callee3
+  %r = call noalias i8* @foo(i8* %p)
+  ret i8* %r
+}
+
+; add the deref attribute to the existing attributes on foo.
+define i8* @caller3(i8* %ptr, i64 %x) {
+; CHECK-LABEL: caller3
+; CHECK: call noalias dereferenceable_or_null(12) i8* @foo
+  %gep = getelementptr inbounds i8, i8* %ptr, i64 %x
+  %p = call dereferenceable_or_null(12) i8* @callee3(i8* %gep)
+  ret i8* %p
+}
+
+declare i8* @inf_loop_call(i8*) nounwind
+; We cannot propagate attributes to foo because we do not know whether inf_loop_call
+; will return execution.
+define internal i8* @callee_with_sideeffect_callsite(i8* %p) alwaysinline {
+; CHECK-NOT: callee_with_sideeffect_callsite
+  %r = call i8* @foo(i8* %p)
+  %v = call i8* @inf_loop_call(i8* %p)
+  ret i8* %r
+}
+
+; do not add deref attribute to foo
+define i8* @test4(i8* %ptr, i64 %x) {
+; CHECK-LABEL: test4
+; CHECK: call i8* @foo
+  %gep = getelementptr inbounds i8, i8* %ptr, i64 %x
+  %p = call dereferenceable_or_null(12) i8* @callee_with_sideeffect_callsite(i8* %gep)
+  ret i8* %p
+}
+
+declare i8* @baz(i8*) nounwind readonly
+define internal i8* @callee5(i8* %p) alwaysinline {
+; CHECK-NOT: callee5
+  %r = call i8* @foo(i8* %p)
+  %v = call i8* @baz(i8* %p)
+  ret i8* %r
+}
+
+; add the deref attribute to foo.
+define i8* @test5(i8* %ptr, i64 %x) {
+; CHECK-LABEL: test5
+; CHECK: call dereferenceable_or_null(12) i8* @foo
+  %gep = getelementptr inbounds i8, i8* %ptr, i64 %x
+  %s = call dereferenceable_or_null(12) i8* @callee5(i8* %gep)
+  ret i8* %s
+}
Index: llvm/lib/Transforms/Utils/InlineFunction.cpp
===
--- llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -80,11 +80,21 @@
   cl::Hidden,
   cl::desc("Convert noalias attributes to metadata during inlining."));
 
+static cl::opt UpdateReturnAttributes(
+"update-return-attrs", cl::init(true), cl::Hidden,
+cl::desc("Update return attributes on calls within inlined body"));
+
 static cl::opt
 PreserveAlignmentAssumptions("preserve-alignment-assumptions-during-inlining",
   cl::init(true), cl::Hidden,
   cl::desc("Convert align attributes to assumptions during inlining."));
 
+static cl::opt MaxInstCheckedForThrow(
+"max-inst-checked-for-throw-during-inlining", cl::Hidden,
+cl::desc("the maximum number of instructions analyzed for may throw during "
+ "attribute inference in

[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Balázs Kéri via Phabricator via cfe-commits

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



Comment at: clang/test/Analysis/kmalloc-linux-1.c:15
+
+// FIXME: malloc checker expects kfree with 2 arguments, is this correct?
+// (recent kernel source code contains a `kfree(const void *)`)

Szelethus wrote:
> martong wrote:
> > Do you plan to sniff around a bit about the arguments (as part of another 
> > patch)? Would be good to handle both old and new signature kernel 
> > allocation functions.
> I'll take a quick look as well, I am quite familiar with MallocChecker.
I found that `kfree` has one argument, not two (even in the 2.6 kernel). 
Probably argument count was wrong in the last change when `CallDescription` was 
introduced.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830



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

[PATCH] D76856: Fix TBAA for unsigned fixed-point types

2020-03-26 Thread mattias.v.eriks...@ericsson.com via Phabricator via cfe-commits

materi created this revision.
materi added reviewers: ebevhan, leonardchan.
Herald added a subscriber: kosarev.
Herald added a project: clang.

Unsigned types can alias the corresponding signed types. I don't see
that this is explicitly mentioned in the Embedded-C specification, but
I think it should work the same as for the integer types.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76856

Files:
  clang/lib/CodeGen/CodeGenTBAA.cpp
  clang/test/CodeGen/fixed-point-tbaa.c

Index: clang/test/CodeGen/fixed-point-tbaa.c
===
--- /dev/null
+++ clang/test/CodeGen/fixed-point-tbaa.c
@@ -0,0 +1,109 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -O1 -ffixed-point %s -emit-llvm -o - | FileCheck %s  -check-prefixes=CHECK
+//
+// Check that we generate correct TBAA metadata for fixed-point types.
+
+void sfract(unsigned short _Fract *p, short _Fract *q,
+unsigned _Sat short _Fract *r, _Sat short _Fract *s) {
+  // CHECK-LABEL: define void @sfract
+  // CHECK: store i8 -128, i8* %p, align 1, !tbaa [[TAG_sf:!.*]]
+  // CHECK: store i8 -64, i8* %q, align 1, !tbaa [[TAG_sf]]
+  // CHECK: store i8 -128, i8* %r, align 1, !tbaa [[TAG_sat_sf:!.*]]
+  // CHECK: store i8 -64, i8* %s, align 1, !tbaa [[TAG_sat_sf]]
+  *p = 0.5hur;
+  *q = -0.5hr;
+  *r = 0.5hur;
+  *s = -0.5hr;
+}
+
+void fract(unsigned _Fract *p, _Fract *q,
+   unsigned _Sat _Fract *r, _Sat _Fract *s) {
+  // CHECK-LABEL: define void @fract
+  // CHECK: store i16 -32768, i16* %p, align 2, !tbaa [[TAG_f:!.*]]
+  // CHECK: store i16 -16384, i16* %q, align 2, !tbaa [[TAG_f]]
+  // CHECK: store i16 -32768, i16* %r, align 2, !tbaa [[TAG_sat_f:!.*]]
+  // CHECK: store i16 -16384, i16* %s, align 2, !tbaa [[TAG_sat_f]]
+  *p = 0.5ur;
+  *q = -0.5r;
+  *r = 0.5ur;
+  *s = -0.5r;
+}
+
+void lfract(unsigned long _Fract *p, long _Fract *q,
+unsigned _Sat long _Fract *r, _Sat long _Fract *s) {
+  // CHECK-LABEL: define void @lfract
+  // CHECK: store i32 -2147483648, i32* %p, align 4, !tbaa [[TAG_lf:!.*]]
+  // CHECK: store i32 -1073741824, i32* %q, align 4, !tbaa [[TAG_lf]]
+  // CHECK: store i32 -2147483648, i32* %r, align 4, !tbaa [[TAG_sat_lf:!.*]]
+  // CHECK: store i32 -1073741824, i32* %s, align 4, !tbaa [[TAG_sat_lf]]
+  *p = 0.5ulr;
+  *q = -0.5lr;
+  *r = 0.5ulr;
+  *s = -0.5lr;
+}
+
+void saccum(unsigned short _Accum *p, short _Accum *q,
+unsigned _Sat short _Accum *r, _Sat short _Accum *s) {
+  // CHECK-LABEL: define void @saccum
+  // CHECK: store i16 128, i16* %p, align 2, !tbaa [[TAG_sk:!.*]]
+  // CHECK: store i16 -64, i16* %q, align 2, !tbaa [[TAG_sk]]
+  // CHECK: store i16 128, i16* %r, align 2, !tbaa [[TAG_sat_sk:!.*]]
+  // CHECK: store i16 -64, i16* %s, align 2, !tbaa [[TAG_sat_sk]]
+  *p = 0.5huk;
+  *q = -0.5hk;
+  *r = 0.5huk;
+  *s = -0.5hk;
+}
+
+void accum(unsigned _Accum *p, _Accum *q,
+   unsigned _Sat _Accum *r, _Sat _Accum *s) {
+  // CHECK-LABEL: define void @accum
+  // CHECK: store i32 32768, i32* %p, align 4, !tbaa [[TAG_k:!.*]]
+  // CHECK: store i32 -16384, i32* %q, align 4, !tbaa [[TAG_k]]
+  // CHECK: store i32 32768, i32* %r, align 4, !tbaa [[TAG_sat_k:!.*]]
+  // CHECK: store i32 -16384, i32* %s, align 4, !tbaa [[TAG_sat_k]]
+  *p = 0.5uk;
+  *q = -0.5k;
+  *r = 0.5uk;
+  *s = -0.5k;
+}
+
+void laccum(unsigned long _Accum *p, long _Accum *q,
+unsigned _Sat long _Accum *r, _Sat long _Accum *s) {
+  // CHECK-LABEL: define void @laccum
+  // CHECK: store i64 2147483648, i64* %p, align 8, !tbaa [[TAG_lk:!.*]]
+  // CHECK: store i64 -1073741824, i64* %q, align 8, !tbaa [[TAG_lk]]
+  // CHECK: store i64 2147483648, i64* %r, align 8, !tbaa [[TAG_sat_lk:!.*]]
+  // CHECK: store i64 -1073741824, i64* %s, align 8, !tbaa [[TAG_sat_lk]]
+  *p = 0.5ulk;
+  *q = -0.5lk;
+  *r = 0.5ulk;
+  *s = -0.5lk;
+}
+
+// CHECK-DAG: [[TAG_sf]] = !{[[TYPE_sf:!.*]], [[TYPE_sf]], i64 0}
+// CHECK-DAG: [[TYPE_sf]] = !{!"short _Fract"
+// CHECK-DAG: [[TAG_f]] = !{[[TYPE_f:!.*]], [[TYPE_f]], i64 0}
+// CHECK-DAG: [[TYPE_f]] = !{!"_Fract"
+// CHECK-DAG: [[TAG_lf]] = !{[[TYPE_lf:!.*]], [[TYPE_lf]], i64 0}
+// CHECK-DAG: [[TYPE_lf]] = !{!"long _Fract"
+
+// CHECK-DAG: [[TAG_sat_sf]] = !{[[TYPE_sat_sf:!.*]], [[TYPE_sat_sf]], i64 0}
+// CHECK-DAG: [[TYPE_sat_sf]] = !{!"_Sat short _Fract"
+// CHECK-DAG: [[TAG_sat_f]] = !{[[TYPE_sat_f:!.*]], [[TYPE_sat_f]], i64 0}
+// CHECK-DAG: [[TYPE_sat_f]] = !{!"_Sat _Fract"
+// CHECK-DAG: [[TAG_sat_lf]] = !{[[TYPE_sat_lf:!.*]], [[TYPE_sat_lf]], i64 0}
+// CHECK-DAG: [[TYPE_sat_lf]] = !{!"_Sat long _Fract"
+
+// CHECK-DAG: [[TAG_sk]] = !{[[TYPE_sk:!.*]], [[TYPE_sk]], i64 0}
+// CHECK-DAG: [[TYPE_sk]] = !{!"short _Accum"
+// CHECK-DAG: [[TAG_k]] = !{[[TYPE_k:!.*]], [[TYPE_k]], i64 0}
+// CHECK-DAG: [[TYPE_k]] = !{!"_Accum"
+// CHECK-DAG: [[TAG_lk]] = !{[[TYPE_lk:!.*]], [[TYPE_lk]], i64 0}
+// CHECK-DAG: [[TYPE_lk]] = !{!"long _Accum"
+
+// CHECK-DAG: [[TAG_sat_sk]] =

[PATCH] D74144: [OPENMP50]Add basic support for array-shaping operation.

2020-03-26 Thread John McCall via Phabricator via cfe-commits

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

Thanks, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74144



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

[PATCH] D76830: [Analyzer][MallocChecker] No warning for kfree of ZERO_SIZE_PTR.

2020-03-26 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added a comment.

In D76830#1943133 , @balazske wrote:

> FIXME: There is a test file "kmalloc-linux.c" but it seems to be 
> non-maintained and buggy (has no //-verify// option so it passes always but 
> produces multiple warnings).


Crap, even I did some changes on that file, and never noticed the lack of 
verify, or the lack of `-analyzer-checker` flags.




Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1687
+  if (ArgValKnown) {
+if (!KernelZeroSizePtrValue)
+  KernelZeroSizePtrValue =

martong wrote:
> balazske wrote:
> > martong wrote:
> > > martong wrote:
> > > > This is a bit confusing for me. Perhaps alternatively we could have a 
> > > > free function `isInitialized(KernelZero...)` instead. Or maybe having a 
> > > > separate bool variable to indicate whether it was initialized could be 
> > > > cleaner?
> > > Another idea: Adding a helper struct to contain the bool `initialized`? 
> > > E.g. (draft):
> > > ```
> > > struct LazyOptional {
> > >   bool initialized = false;
> > >   Opt value;
> > >   Opt& get();
> > >   void set(const Opt&);
> > > };
> > > ```
> > It may be OK to have a function `lazyInitKernelZeroSizePtrValue`?
> I don't insist, given we have a better described type for 
> `KernelZeroSizePtrValue` (e.g. having a `using` `template`)
`AnalysisManager` has access to the `Preprocessor`, and it is also responsible 
for the construction of the `CheckerManager` object. This would make moving 
such code to the checker registry function! I'll gladly take this issue off 
your hand and patch it in once rG2aac0c47aed8d1eff7ab6d858173c532b881d948 
settles :)



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1690-1700
+  // If there is a macro called ZERO_SIZE_PTR, it could be a kernel source code
+  // and this value indicates a special value used for a zero-sized memory
+  // block. It is a constant value that is allowed to be freed.
+  const llvm::APSInt *ArgValKnown =
+  C.getSValBuilder().getKnownValue(State, ArgVal);
+  if (ArgValKnown) {
+initKernelZeroSizePtrValue(C.getPreprocessor());

I found this article on the subject:

https://lwn.net/Articles/236920/

> That brings us to the third possibility: this patch from Christoph Lameter 
> which causes kmalloc(0) to return a special ZERO_SIZE_PTR value. It is a 
> non-NULL value which looks like a legitimate pointer, but which causes a 
> fault on any attempt at dereferencing it. Any attempt to call kfree() with 
> this special value will do the right thing, of course.

But I would argue that using `delete` on such a pointer might still be a sign 
of code smell. Granted, if the source code is hacking the kernel this is very 
unlikely, but still, I think this code should be placed...



Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1707-1708
   if (!R) {
 ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr, Family);
 return nullptr;
   }

...here!

```lang=cpp
bool isArgZERO_SIZE_PTR(ArgVal) const {
  const llvm::APSInt *ArgValKnown =
  C.getSValBuilder().getKnownValue(State, ArgVal);
  if (ArgValKnown) {
initKernelZeroSizePtrValue(C.getPreprocessor());
if (*KernelZeroSizePtrValue &&
ArgValKnown->getSExtValue() == **KernelZeroSizePtrValue)
  return true;
  }
  return false;
}

// ...

if (ArgVal has no associated MemRegion)
  // If there is a macro called ZERO_SIZE_PTR, it could be a kernel source code
  // and this value indicates a special value used for a zero-sized memory
  // block. It is a constant value that is allowed to be freed.
  if (!isArgZERO_SIZE_PTR(ArgVal)
ReportBadFree(C, ArgVal, ArgExpr->getSourceRange(), ParentExpr, Family);
return nullptr;
  }
  // Still check for incorrect deallocator usage, etc.
}
```

Or something like this. WDYT?



Comment at: clang/test/Analysis/kmalloc-linux-1.c:15
+
+// FIXME: malloc checker expects kfree with 2 arguments, is this correct?
+// (recent kernel source code contains a `kfree(const void *)`)

martong wrote:
> Do you plan to sniff around a bit about the arguments (as part of another 
> patch)? Would be good to handle both old and new signature kernel allocation 
> functions.
I'll take a quick look as well, I am quite familiar with MallocChecker.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76830



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

[PATCH] D75677: [Analyzer] Only add iterator note tags to the operations of the affected iterators

2020-03-26 Thread Balogh, Ádám via Phabricator via cfe-commits

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



Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:541-542
+BR.markInteresting(It1);
+if (const auto  = It1.getAs()) {
+  BR.markInteresting(LCV1->getRegion());
+}

baloghadamsoftware wrote:
> baloghadamsoftware wrote:
> > Szelethus wrote:
> > > NoQ wrote:
> > > > baloghadamsoftware wrote:
> > > > > NoQ wrote:
> > > > > > I'm opposed to this code for the same reason that i'm opposed to it 
> > > > > > in the debug checker. Parent region is an undocumented 
> > > > > > implementation detail of `RegionStore`. It is supposed to be 
> > > > > > immaterial to the user. You should not rely on its exact value.
> > > > > > 
> > > > > > @baloghadamsoftware Can we eliminate all such code from iterator 
> > > > > > checkers and instead identify all iterators by regions in which 
> > > > > > they're stored? Does my improved C++ support help with this anyhow 
> > > > > > whenever it kicks in?
> > > > > How to find the region where it is stored? I am open to find better 
> > > > > solutions, but it was the only one I could find so far. If we ignore 
> > > > > `LazyCompoundVal` then everything falls apart, we can remove all the 
> > > > > iterator-related checkers.
> > > > It's the region from which you loaded it. If you obtained it as 
> > > > `Call.getArgSVal()` then it's the parameter region for the call. If you 
> > > > obtained it as `Call.getReturnValue()` then it's the target region can 
> > > > be obtained by looking at the //construction context// for the call.
> > > `LazyCompoundVal` and friends seem to be a constantly emerging headache 
> > > for almost everyone. For how long I've spent in the analyzer, and have 
> > > religiously studied conversations and your workbook about it, I still 
> > > feel anxious whenever it comes up.
> > > 
> > > It would be great to have this documented in the code sometime.
> > Do you mean `CallEvent::getParameterLocation()` for arguments? What is the 
> > //construction context// for the call? How can it be obtained?
> I do not know exactly how many place `LazyCompoundVal`  appears, but one 
> place for sure is in `MaterializeTemporaryExpr::getSubExpr()`. What to use 
> there instead?
I also get it in the `Val` parameter of `checkBind()`.


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

https://reviews.llvm.org/D75677



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

[clang] 0766d1d - Make a windows buildbot happy

2020-03-26 Thread Kirstóf Umann via cfe-commits


Author: Kirstóf Umann
Date: 2020-03-26T17:04:23+01:00
New Revision: 0766d1dca8685e77e8672b13f26797a5d4c8b4d7

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

LOG: Make a windows buildbot happy

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h 
b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
index dde2409ed72c..73594bb8b380 100644
--- a/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
+++ b/clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
@@ -241,8 +241,9 @@ class CheckerRegistry {
   bool IsHidden = false) {
 // Avoid MSVC's Compiler Error C2276:
 // http://msdn.microsoft.com/en-us/library/850cstw1(v=VS.80).aspx
-addChecker(, , FullName,
-   Desc, DocsUri, IsHidden);
+addChecker(::initializeManager,
+   ::returnTrue, FullName, Desc, DocsUri,
+   IsHidden);
   }
 
   /// Makes the checker with the full name \p fullName depends on the checker



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

[PATCH] D75360: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes

2020-03-26 Thread Kristóf Umann via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2aac0c47aed8: Reland [analyzer][NFC] Tie 
CheckerRegistry to CheckerManager, allow… (authored by Szelethus).

Changed prior to commit:
  https://reviews.llvm.org/D75360?vs=252073=252861#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75360

Files:
  clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
  clang/include/clang/StaticAnalyzer/Frontend/AnalyzerHelpFlags.h
  clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistration.h
  clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
  clang/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalyzerHelpFlags.cpp
  clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
  clang/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp
  clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
  clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp

Index: clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp
@@ -0,0 +1,52 @@
+//===- CheckerManager.h - Static Analyzer Checker Manager ---*- C++ -*-===//
+//
+// 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
+//
+//===--===//
+//
+// Defines the Static Analyzer Checker Manager.
+//
+//===--===//
+
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+#include 
+
+namespace clang {
+namespace ento {
+
+CheckerManager::CheckerManager(
+ASTContext , AnalyzerOptions ,
+ArrayRef plugins,
+ArrayRef> checkerRegistrationFns)
+: Context(), LangOpts(Context.getLangOpts()), AOptions(AOptions),
+  Diags(Context.getDiagnostics()),
+  Registry(
+  std::make_unique(plugins, Context.getDiagnostics(),
+AOptions, checkerRegistrationFns)) {
+  Registry->initializeRegistry(*this);
+  Registry->initializeManager(*this);
+  finishedCheckerRegistration();
+}
+
+/// Constructs a CheckerManager without requiring an AST. No checker
+/// registration will take place. Only useful for retrieving the
+/// CheckerRegistry and print for help flags where the AST is unavalaible.
+CheckerManager::CheckerManager(AnalyzerOptions ,
+   const LangOptions ,
+   DiagnosticsEngine ,
+   ArrayRef plugins)
+: LangOpts(LangOpts), AOptions(AOptions), Diags(Diags),
+  Registry(std::make_unique(plugins, Diags, AOptions)) {
+  Registry->initializeRegistry(*this);
+}
+
+CheckerManager::~CheckerManager() {
+  for (const auto  : CheckerDtors)
+CheckerDtor();
+}
+
+} // namespace ento
+} // namespace clang
Index: clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
+++ clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp
@@ -109,9 +109,9 @@
 
 CheckerRegistry::CheckerRegistry(
 ArrayRef Plugins, DiagnosticsEngine ,
-AnalyzerOptions , const LangOptions ,
+AnalyzerOptions ,
 ArrayRef> CheckerRegistrationFns)
-: Diags(Diags), AnOpts(AnOpts), LangOpts(LangOpts) {
+: Diags(Diags), AnOpts(AnOpts) {
 
   // Register builtin checkers.
 #define GET_CHECKERS
@@ -179,12 +179,16 @@
   addDependency(FULLNAME, DEPENDENCY);
 
 #define GET_CHECKER_OPTIONS
-#define CHECKER_OPTION(TYPE, FULLNAME, CMDFLAG, DESC, DEFAULT_VAL, DEVELOPMENT_STATUS, IS_HIDDEN)  \
-  addCheckerOption(TYPE, FULLNAME, CMDFLAG, DEFAULT_VAL, DESC, DEVELOPMENT_STATUS, IS_HIDDEN);
+#define CHECKER_OPTION(TYPE, FULLNAME, CMDFLAG, DESC, DEFAULT_VAL, \
+   DEVELOPMENT_STATUS, IS_HIDDEN)  \
+  addCheckerOption(TYPE, FULLNAME, CMDFLAG, DEFAULT_VAL, DESC, \
+   DEVELOPMENT_STATUS, IS_HIDDEN);
 
 #define GET_PACKAGE_OPTIONS
-#define PACKAGE_OPTION(TYPE, FULLNAME, CMDFLAG, DESC, DEFAULT_VAL, DEVELOPMENT_STATUS, IS_HIDDEN)  \
-  addPackageOption(TYPE, FULLNAME, CMDFLAG, DEFAULT_VAL, DESC, DEVELOPMENT_STATUS, IS_HIDDEN);
+#define PACKAGE_OPTION(TYPE, FULLNAME, CMDFLAG, DESC, DEFAULT_VAL, \
+   DEVELOPMENT_STATUS, IS_HIDDEN)

[PATCH] D74144: [OPENMP50]Add basic support for array-shaping operation.

2020-03-26 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev updated this revision to Diff 252859.
ABataev added a comment.

Rebase + fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74144

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/BuiltinTypes.def
  clang/include/clang/AST/ComputeDependence.h
  clang/include/clang/AST/ExprOpenMP.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/StmtNodes.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ComputeDependence.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprClassification.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/AST/StmtPrinter.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Checkers/IdenticalExprChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/OpenMP/depobj_messages.cpp
  clang/test/OpenMP/parallel_reduction_messages.c
  clang/test/OpenMP/task_ast_print.cpp
  clang/test/OpenMP/task_depend_messages.cpp
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CXCursor.cpp

Index: clang/tools/libclang/CXCursor.cpp
===
--- clang/tools/libclang/CXCursor.cpp
+++ clang/tools/libclang/CXCursor.cpp
@@ -423,6 +423,10 @@
 K = CXCursor_OMPArraySectionExpr;
 break;
 
+  case Stmt::OMPArrayShapingExprClass:
+K = CXCursor_OMPArrayShapingExpr;
+break;
+
   case Stmt::BinaryOperatorClass:
 K = CXCursor_BinaryOperator;
 break;
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -5183,6 +5183,8 @@
 return cxstring::createRef("ArraySubscriptExpr");
   case CXCursor_OMPArraySectionExpr:
 return cxstring::createRef("OMPArraySectionExpr");
+  case CXCursor_OMPArrayShapingExpr:
+return cxstring::createRef("OMPArrayShapingExpr");
   case CXCursor_BinaryOperator:
 return cxstring::createRef("BinaryOperator");
   case CXCursor_CompoundAssignOperator:
Index: clang/test/OpenMP/task_depend_messages.cpp
===
--- clang/test/OpenMP/task_depend_messages.cpp
+++ clang/test/OpenMP/task_depend_messages.cpp
@@ -35,14 +35,14 @@
   #pragma omp task depend (source) // expected-error {{expected expression}} expected-warning {{missing ':' after dependency type - ignoring}} omp45-error {{expected 'in', 'out', 'inout' or 'mutexinoutset' in OpenMP clause 'depend'}} omp50-error {{expected 'in', 'out', 'inout', 'mutexinoutset' or 'depobj' in OpenMP clause 'depend'}}
   #pragma omp task depend (in : argc)) // expected-warning {{extra tokens at the end of '#pragma omp task' are ignored}}
   #pragma omp task depend (out: ) // expected-error {{expected expression}}
-  #pragma omp task depend (inout : foobool(argc)), depend (in, argc) // expected-error {{expected addressable lvalue expression, array element or array section}} expected-warning {{missing ':' after dependency type - ignoring}} expected-error {{expected expression}}
+  #pragma omp task depend (inout : foobool(argc)), depend (in, argc) // omp50-error {{expected addressable lvalue expression, array element, array section or array shaping expression}} omp45-error {{expected addressable lvalue expression, array element or array section}} expected-warning {{missing ':' after dependency type - ignoring}} expected-error {{expected expression}}
   #pragma omp task depend (out :S1) // expected-error {{'S1' does not refer to a value}}
   #pragma omp task depend(in : argv[1][1] = '2')
-  #pragma omp task depend (in : vec[1]) // expected-error {{expected addressable lvalue expression, array element or array section}}
+  #pragma omp task depend (in : vec[1]) // omp50-error {{expected addressable lvalue expression, array element, array section or array shaping expression}} omp45-error {{expected addressable lvalue expression, array element or array section}}
   #pragma omp task depend (in : argv[0])
   #pragma omp task depend (in : ) // expected-error {{expected expression}}
   #pragma omp task depend (in : main)
-  #pragma omp task depend(in : a[0]) // expected-error{{expected addressable lvalue expression, array element or array section}}
+  #pragma omp task depend(in : a[0]) // omp50-error

[PATCH] D76850: clang-format: Fix pointer alignment for overloaded operators (PR45107)

2020-03-26 Thread Krasimir Georgiev via Phabricator via cfe-commits

krasimir added a comment.

@MyDeveloperDay : sorry, I must have not refreshed this page and didn't realize 
you already gave an LGTM until I posted the comment.


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

https://reviews.llvm.org/D76850



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

[PATCH] D76850: clang-format: Fix pointer alignment for overloaded operators (PR45107)

2020-03-26 Thread Krasimir Georgiev via Phabricator via cfe-commits

krasimir accepted this revision.
krasimir added a comment.

I think this is an improvement. Accepting with a nit. Thank you!




Comment at: clang/lib/Format/TokenAnnotator.cpp:2806
+  if (Previous->is(tok::identifier) || Previous->isSimpleTypeSpecifier()) {
+Previous = Previous->Previous;
+continue;

Consider using `Previous->getPreviousNonComment()` here and below to jump over 
comments.


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

https://reviews.llvm.org/D76850



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

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Haojian Wu via Phabricator via cfe-commits

hokein added a comment.

Thanks for the test case. Reverted in 
https://github.com/llvm/llvm-project/commit/62dea6e9be31b100962f9ad41c1b79467a53f6cd
 for now. Adding the error-bit to type looks like a right direction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76696



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

[PATCH] D75677: [Analyzer] Only add iterator note tags to the operations of the affected iterators

2020-03-26 Thread Balogh, Ádám via Phabricator via cfe-commits

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



Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:541-542
+BR.markInteresting(It1);
+if (const auto  = It1.getAs()) {
+  BR.markInteresting(LCV1->getRegion());
+}

baloghadamsoftware wrote:
> Szelethus wrote:
> > NoQ wrote:
> > > baloghadamsoftware wrote:
> > > > NoQ wrote:
> > > > > I'm opposed to this code for the same reason that i'm opposed to it 
> > > > > in the debug checker. Parent region is an undocumented implementation 
> > > > > detail of `RegionStore`. It is supposed to be immaterial to the user. 
> > > > > You should not rely on its exact value.
> > > > > 
> > > > > @baloghadamsoftware Can we eliminate all such code from iterator 
> > > > > checkers and instead identify all iterators by regions in which 
> > > > > they're stored? Does my improved C++ support help with this anyhow 
> > > > > whenever it kicks in?
> > > > How to find the region where it is stored? I am open to find better 
> > > > solutions, but it was the only one I could find so far. If we ignore 
> > > > `LazyCompoundVal` then everything falls apart, we can remove all the 
> > > > iterator-related checkers.
> > > It's the region from which you loaded it. If you obtained it as 
> > > `Call.getArgSVal()` then it's the parameter region for the call. If you 
> > > obtained it as `Call.getReturnValue()` then it's the target region can be 
> > > obtained by looking at the //construction context// for the call.
> > `LazyCompoundVal` and friends seem to be a constantly emerging headache for 
> > almost everyone. For how long I've spent in the analyzer, and have 
> > religiously studied conversations and your workbook about it, I still feel 
> > anxious whenever it comes up.
> > 
> > It would be great to have this documented in the code sometime.
> Do you mean `CallEvent::getParameterLocation()` for arguments? What is the 
> //construction context// for the call? How can it be obtained?
I do not know exactly how many place `LazyCompoundVal`  appears, but one place 
for sure is in `MaterializeTemporaryExpr::getSubExpr()`. What to use there 
instead?


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

https://reviews.llvm.org/D75677



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

[PATCH] D31343: Add an attribute plugin example

2020-03-26 Thread John Brawn via Phabricator via cfe-commits

john.brawn added a comment.

Release note added: https://reviews.llvm.org/rG0cff81cff05d


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D31343



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

[PATCH] D76197: clang-format: Use block indentation for braced initializations

2020-03-26 Thread Daan De Meyer via Phabricator via cfe-commits

DaanDeMeyer added a comment.

Of course, if there's interest in adding this I'll fix all the tests but I 
wanted to make sure there was interest in adding this since it changes 
clang-format's behavior. Can you confirm that this change in behavior is a good 
thing and might be merged if I fix all the tests? If the existing output cannot 
be changed in any way then this revision can be closed since it fundamentally 
requires changing clang-format's output.


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

https://reviews.llvm.org/D76197



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

[PATCH] D75360: [analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes

2020-03-26 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added a comment.

Tried to recommit -- I'll keep a close eye on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75360



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

[clang] 62dea6e - Revert "[AST] Build recovery expressions by default for C++."

2020-03-26 Thread Haojian Wu via cfe-commits


Author: Haojian Wu
Date: 2020-03-26T16:25:32+01:00
New Revision: 62dea6e9be31b100962f9ad41c1b79467a53f6cd

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

LOG: Revert "[AST] Build recovery expressions by default for C++."

This reverts commit 0788acbccbec094903a3425ffe5a98f8d55cbd64.
This reverts commit c2d7a1f79cedfc9fcb518596aa839da4de0adb69:  Revert "[clangd] 
Add test for FindTarget+RecoveryExpr (which already works). NFC"

It causes a crash on invalid code:

class X {
  decltype(unresolved()) foo;
};
constexpr int s = sizeof(X);

Added: 


Modified: 
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
clang/include/clang/Basic/LangOptions.def
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/OpenMP/target_update_from_messages.cpp
clang/test/OpenMP/target_update_to_messages.cpp
clang/test/Parser/objcxx0x-lambda-expressions.mm
clang/test/Parser/objcxx11-invalid-lambda.cpp
clang/test/SemaCXX/builtins.cpp
clang/test/SemaCXX/cast-conversion.cpp
clang/test/SemaCXX/constructor-initializer.cpp
clang/test/SemaCXX/cxx1z-copy-omission.cpp
clang/test/SemaCXX/decltype-crash.cpp
clang/test/SemaCXX/varargs.cpp
clang/test/SemaOpenCLCXX/address-space-references.cl
clang/test/SemaTemplate/instantiate-init.cpp
clang/unittests/Sema/CodeCompleteTest.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp 
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index c691c6d9e61a..24197485f68a 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1195,9 +1195,7 @@ TEST(SignatureHelpTest, OpeningParen) {
 int foo(int a, int b, int c);
 int main() {
 #define ID(X) X
-  // FIXME: figure out why ID(foo (foo(10), )) doesn't work when preserving
-  // the recovery expression.
-  ID(foo $p^( 10, ^ ))
+  ID(foo $p^( foo(10), ^ ))
 })cpp"};
 
   for (auto Test : Tests) {

diff  --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp 
b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index cd6f2039c888..c38ccc3f9441 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -132,16 +132,6 @@ TEST_F(TargetDeclTest, Exprs) {
   EXPECT_DECLS("CXXOperatorCallExpr", "void operator()(int n)");
 }
 
-TEST_F(TargetDeclTest, Recovery) {
-  Code = R"cpp(
-// error-ok: testing behavior on broken code
-int f();
-int f(int, int);
-int x = [[f]](42);
-  )cpp";
-  EXPECT_DECLS("UnresolvedLookupExpr", "int f()", "int f(int, int)");
-}
-
 TEST_F(TargetDeclTest, UsingDecl) {
   Code = R"cpp(
 namespace foo {
@@ -695,15 +685,6 @@ TEST_F(FindExplicitReferencesTest, All) {
 )cpp",
 "0: targets = {x}\n"
 "1: targets = {X::a}\n"},
-   {R"cpp(
-   // error-ok: testing with broken code
-   int bar();
-   int foo() {
- return $0^bar() + $1^bar(42);
-   }
-   )cpp",
-   "0: targets = {bar}\n"
-   "1: targets = {bar}\n"},
// Namespaces and aliases.
{R"cpp(
   namespace ns {}

diff  --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index 22c4a87918e9..6152e227d599 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -148,7 +148,7 @@ LANGOPT(RelaxedTemplateTemplateArgs, 1, 0, "C++17 relaxed 
matching of template t
 
 LANGOPT(DoubleSquareBracketAttributes, 1, 0, "'[[]]' attributes extension for 
all language standard modes")
 
-COMPATIBLE_LANGOPT(RecoveryAST, 1, CPlusPlus, "Preserve expressions in AST 
when encountering errors")
+COMPATIBLE_LANGOPT(RecoveryAST, 1, 0, "Preserve expressions in AST when 
encountering errors")
 
 BENIGN_LANGOPT(ThreadsafeStatics , 1, 1, "thread-safe static initializers")
 LANGOPT(POSIXThreads  , 1, 0, "POSIX thread support")

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 08e0700671a8..dc5e932c9460 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -2909,7 +2909,7 @@ static void ParseLangArgs(LangOptions , ArgList 
, InputKind IK,
   if (Args.hasArg(OPT_fconcepts_ts))
 Diags.Report(diag::warn_fe_concepts_ts_flag);
   Opts.RecoveryAST =
-  Args.hasFlag(OPT_frecovery_ast, OPT_fno_recovery_ast, Opts.CPlusPlus);
+  Args.hasFlag(OPT_frecovery_ast, OPT_fno_recovery_ast, false);
   Opts.HeinousExtensions = Args.hasArg(OPT_fheinous_gnu_extensions);
   Opts.AccessControl =

[clang] 2aac0c4 - Reland "[analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow CheckerManager to be constructed for non-analysis purposes"

2020-03-26 Thread Kirstóf Umann via cfe-commits


Author: Kristóf Umann
Date: 2020-03-26T16:12:38+01:00
New Revision: 2aac0c47aed8d1eff7ab6d858173c532b881d948

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

LOG: Reland "[analyzer][NFC] Tie CheckerRegistry to CheckerManager, allow 
CheckerManager to be constructed for non-analysis purposes"

Originally commited in rG57b8a407493c34c3680e7e1e4cb82e097f43744a, but
it broke the modules bot. This is solved by putting the contructors of
the CheckerManager class to the Frontend library.

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

Added: 
clang/include/clang/StaticAnalyzer/Frontend/AnalyzerHelpFlags.h
clang/lib/StaticAnalyzer/Frontend/AnalyzerHelpFlags.cpp
clang/lib/StaticAnalyzer/Frontend/CreateCheckerManager.cpp

Modified: 
clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistry.h
clang/include/clang/StaticAnalyzer/Frontend/FrontendActions.h
clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
clang/lib/StaticAnalyzer/Frontend/CMakeLists.txt
clang/lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp

Removed: 
clang/include/clang/StaticAnalyzer/Frontend/CheckerRegistration.h
clang/lib/StaticAnalyzer/Frontend/CheckerRegistration.cpp



diff  --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h 
b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 4454d7603b27..4635ca35736a 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -14,6 +14,7 @@
 #define LLVM_CLANG_STATICANALYZER_CORE_CHECKERMANAGER_H
 
 #include "clang/Analysis/ProgramPoint.h"
+#include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/Store.h"
@@ -121,14 +122,36 @@ enum class ObjCMessageVisitKind {
 };
 
 class CheckerManager {
-  ASTContext 
+  ASTContext *Context;
   const LangOptions LangOpts;
   AnalyzerOptions 
   CheckerNameRef CurrentCheckerName;
+  DiagnosticsEngine 
+  std::unique_ptr Registry;
 
 public:
+  // These constructors are defined in the Frontend library, because
+  // CheckerRegistry, a crucial component of the initialization is in there.
+  // CheckerRegistry cannot be moved to the Core library, because the checker
+  // registration functions are defined in the Checkers library, and the 
library
+  // dependencies look like this: Core -> Checkers -> Frontend.
+
+  CheckerManager(
+  ASTContext , AnalyzerOptions ,
+  ArrayRef plugins,
+  ArrayRef> checkerRegistrationFns);
+
+  /// Constructs a CheckerManager that ignores all non TblGen-generated
+  /// checkers. Useful for unit testing, unless the checker infrastructure
+  /// itself is tested.
   CheckerManager(ASTContext , AnalyzerOptions )
-  : Context(Context), LangOpts(Context.getLangOpts()), AOptions(AOptions) 
{}
+  : CheckerManager(Context, AOptions, {}, {}) {}
+
+  /// Constructs a CheckerManager without requiring an AST. No checker
+  /// registration will take place. Only useful for retrieving the
+  /// CheckerRegistry and print for help flags where the AST is unavalaible.
+  CheckerManager(AnalyzerOptions , const LangOptions ,
+ DiagnosticsEngine , ArrayRef plugins);
 
   ~CheckerManager();
 
@@ -141,7 +164,12 @@ class CheckerManager {
 
   const LangOptions () const { return LangOpts; }
   AnalyzerOptions () const { return AOptions; }
-  ASTContext () const { return Context; }
+  const CheckerRegistry () const { return *Registry; }
+  DiagnosticsEngine () const { return Diags; }
+  ASTContext () const {
+assert(Context);
+return *Context;
+  }
 
   /// Emits an error through a DiagnosticsEngine about an invalid user supplied
   /// checker option value.

diff  --git a/clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h 
b/clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
index 2d24e6a9586b..bcc29a60ad70 100644
--- a/clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
+++ b/clang/include/clang/StaticAnalyzer/Frontend/AnalysisConsumer.h
@@ -55,7 +55,7 @@ class AnalysisASTConsumer : public ASTConsumer {
 std::unique_ptr
 CreateAnalysisConsumer(CompilerInstance );
 
-} // end GR namespace
+} // namespace ento
 
 } // end clang namespace
 

diff  --git a/clang/include/clang/StaticAnalyzer/Frontend/AnalyzerHelpFlags.h 
b/clang/include/clang/StaticAnalyzer/Frontend/AnalyzerHelpFlags.h
new file mode 100644
index ..a30c241e1350
---

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

2020-03-26 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

In D76696#1943657 , @ebevhan wrote:

> However, this just means that `baz` has a very odd type; it's a variable of 
> closure type that contains a dependent member that will never be resolved. 
> Our diagnostic breaks, since the variable's type isn't dependent, but it has 
> a member which is, and we can't take the size of that.

Interesting. Simply turning this off for lambdas isn't enough. The following 
crashes clang for me:

  class X {
decltype(unresolved()) foo;
  };
  constexpr int s = sizeof(X);

@hokein I think we need to revert until we find a solution for this. I suspect 
this is going to mean adding the error bit to type and... dropping members that 
have it set? Or marking them as invalid?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76696

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

[PATCH] D75677: [Analyzer] Only add iterator note tags to the operations of the affected iterators

2020-03-26 Thread Balogh, Ádám via Phabricator via cfe-commits

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



Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:541-542
+BR.markInteresting(It1);
+if (const auto  = It1.getAs()) {
+  BR.markInteresting(LCV1->getRegion());
+}

Szelethus wrote:
> NoQ wrote:
> > baloghadamsoftware wrote:
> > > NoQ wrote:
> > > > I'm opposed to this code for the same reason that i'm opposed to it in 
> > > > the debug checker. Parent region is an undocumented implementation 
> > > > detail of `RegionStore`. It is supposed to be immaterial to the user. 
> > > > You should not rely on its exact value.
> > > > 
> > > > @baloghadamsoftware Can we eliminate all such code from iterator 
> > > > checkers and instead identify all iterators by regions in which they're 
> > > > stored? Does my improved C++ support help with this anyhow whenever it 
> > > > kicks in?
> > > How to find the region where it is stored? I am open to find better 
> > > solutions, but it was the only one I could find so far. If we ignore 
> > > `LazyCompoundVal` then everything falls apart, we can remove all the 
> > > iterator-related checkers.
> > It's the region from which you loaded it. If you obtained it as 
> > `Call.getArgSVal()` then it's the parameter region for the call. If you 
> > obtained it as `Call.getReturnValue()` then it's the target region can be 
> > obtained by looking at the //construction context// for the call.
> `LazyCompoundVal` and friends seem to be a constantly emerging headache for 
> almost everyone. For how long I've spent in the analyzer, and have 
> religiously studied conversations and your workbook about it, I still feel 
> anxious whenever it comes up.
> 
> It would be great to have this documented in the code sometime.
Do you mean `CallEvent::getParameterLocation()` for arguments? What is the 
//construction context// for the call? How can it be obtained?


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

https://reviews.llvm.org/D75677



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

[PATCH] D76818: [clang-tidy] Add check llvmlibc-implementation-in-namespace.

2020-03-26 Thread Nathan James via Phabricator via cfe-commits

njames93 added a comment.

If you want to make it a general check, you should consider making the default 
module options set the correct namespace
RequiredNamespace

  ClangTidyOptions getModuleOptions() override {
ClangTidyOptions Options;

Options.CheckOptions["llvmlibc-implementation-in-namespace.RequiredNamespace"] 
= "__llvm_libc";
return Options;
  }




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/llvmlibc-implementation-in-namespace.cpp:6
+class ClassB;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: CXXRecord is not defined in a 
namespace, please wrap implentation in '__llvm_libc' namespace.
+struct StructC {};

Small nit : Not a fan of the diagnostic saying `CXX Record`. Maybe a pain but 
`getDeclKindName` isn't the best to expose to users. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76818



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

[PATCH] D76724: Prevent immediate evaluations inside of decltype

2020-03-26 Thread Wyatt Childers via Phabricator via cfe-commits

wchilders abandoned this revision.
wchilders added a comment.

Thanks Tyker, you're right, there is a general problem that needs handled, if 
this is to be handled; I had a bit of tunnel vision here. However, I'm 
retracting/abandoning this patch entirely as upon further review, it's not 
standard compliant. The current situation, is actually the correct one (for 
now?).

From [7.7.13] of the working draft:

> Note: A manifestly constant-evaluated expression is evaluated even in an 
> unevaluated operand.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76724



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

[PATCH] D76197: clang-format: Use block indentation for braced initializations

2020-03-26 Thread MyDeveloperDay via Phabricator via cfe-commits

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

> New implementation that breaks fewer tests.

`fewer` is not an option, `no` is all thats going to get accepted.

Please add tests for you own use case


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

https://reviews.llvm.org/D76197



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

[clang] 0cff81c - Add a release note for attribute plugins

2020-03-26 Thread John Brawn via cfe-commits


Author: John Brawn
Date: 2020-03-26T15:01:57Z
New Revision: 0cff81cff05d8e0a24391e3dec5b97174351ea55

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

LOG: Add a release note for attribute plugins

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ad13fb1b3e95..a8163cad9fde 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -111,7 +111,9 @@ New Pragmas in Clang
 Attribute Changes in Clang
 --
 
-- ...
+- Attributes can now be specified by clang plugins. See the
+  `Clang Plugins `_ documentation for
+  details.
 
 Windows Support
 ---



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

[PATCH] D74183: [IRGen] Add an alignment attribute to underaligned sret parameters

2020-03-26 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added subscribers: hfinkel, jdoerfert.
efriedma added a comment.

That makes sense.

I would slightly lean towards not generating the assumptions, given the current 
state of assumptions and the likely benefit in this context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74183



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

1 2 >

1 - 100 of 160 matches

Mail list logo