[PATCH] D103562: [NFC][compiler-rt][hwasan] Refactor hwasan functions

2021-06-02 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka accepted this revision.
vitalybuka added inline comments.
This revision is now accepted and ready to land.



Comment at: compiler-rt/lib/hwasan/hwasan.cpp:192
+
+void HandleTagMismatch(AccessInfo ai, uptr pc, uptr frame,
+   void *uc, uptr *registers_frame) {

please clang-format the patch



Comment at: compiler-rt/lib/hwasan/hwasan.cpp:226
+
+  __hwasan::HandleTagMismatch(ai, (uptr)__builtin_return_address(0),
+  (uptr)__builtin_frame_address(0), nullptr,

__hwasan:: redundant


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103562

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


[PATCH] D103048: [IR] make -stack-alignment= into a module attr

2021-06-02 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson added a comment.
Herald added a subscriber: ormris.

> Curiously, using ModFlagBehavior::Error doesn't error if one of two modules 
> being linked together doesn't have such a module level attribute.

Yeah, there's a Require behavior, but that only allows you to specify what the 
value should be after linking modules. Barring changing the behavior of Error, 
which I'm guessing is relied upon too many places, the main thing I can think 
of is to add a new module flag behavior with tbd name that is essentially like 
Error but treats a module without the module flag as having a conflicting value 
and issues an error for that as well.




Comment at: llvm/test/Linker/stack-alignment.ll:11
+;--- main.ll
+; NONE: error: linking module flags 'override-stack-alignment': IDs have 
conflicting values
+; CHECK-16: error: linking module flags 'override-stack-alignment': IDs have 
conflicting values

Will you get this error currently? I thought per comment and behavior of Error 
that it shouldn't give an error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103048

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


[PATCH] D103589: [Format] Fix incorrect pointer detection

2021-06-02 Thread Yilong Guo via Phabricator via cfe-commits
Nuu added a comment.

In D103589#2795447 , @curdeius wrote:

> Thanks for looking into this, but please add a test case that demonstrates 
> the bug to `FormatTests.cpp` 
> Also, is there, by any chance, an associated bugzilla ticket?

Thanks for the comment.
I've added a test and linked to the bugzilla ticket 
 in summary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103589

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


[PATCH] D103589: [Format] Fix incorrect pointer detection

2021-06-02 Thread Yilong Guo via Phabricator via cfe-commits
Nuu updated this revision to Diff 349455.
Nuu edited the summary of this revision.
Nuu added a comment.

Add test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103589

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8664,6 +8664,7 @@
 
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
+  verifyFormat("void f() { f(float(1), a * a); }");
   // FIXME: Is there a way to make this work?
   // verifyIndependentOfContext("MACRO(A *a);");
   verifyFormat("MACRO(A &B);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -399,7 +399,7 @@
 HasMultipleParametersOnALine = true;
   if ((CurrentToken->Previous->isOneOf(tok::kw_const, tok::kw_auto) ||
CurrentToken->Previous->isSimpleTypeSpecifier()) &&
-  !CurrentToken->is(tok::l_brace))
+  !CurrentToken->isOneOf(tok::l_brace, tok::l_paren))
 Contexts.back().IsExpression = false;
   if (CurrentToken->isOneOf(tok::semi, tok::colon)) {
 MightBeObjCForRangeLoop = false;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8664,6 +8664,7 @@
 
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
+  verifyFormat("void f() { f(float(1), a * a); }");
   // FIXME: Is there a way to make this work?
   // verifyIndependentOfContext("MACRO(A *a);");
   verifyFormat("MACRO(A &B);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -399,7 +399,7 @@
 HasMultipleParametersOnALine = true;
   if ((CurrentToken->Previous->isOneOf(tok::kw_const, tok::kw_auto) ||
CurrentToken->Previous->isSimpleTypeSpecifier()) &&
-  !CurrentToken->is(tok::l_brace))
+  !CurrentToken->isOneOf(tok::l_brace, tok::l_paren))
 Contexts.back().IsExpression = false;
   if (CurrentToken->isOneOf(tok::semi, tok::colon)) {
 MightBeObjCForRangeLoop = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D103589: [Format] Fix incorrect pointer detection

2021-06-02 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision.
curdeius added a comment.
This revision now requires changes to proceed.

Thanks for looking into this, but please add a test case that demonstrates the 
bug to `FormatTests.cpp` 
Also, is there, by any chance, an associated bugzilla ticket?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103589

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


[PATCH] D103434: [analyzer] Allow visitors to run callbacks on completion

2021-06-02 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

> Tests to verify that ReturnVisitor actually does what we intend it to do 
> (call the callback at the right place).

You mean write a test that demonstrates that? I guess unless we're willing to 
wait for the checker to catch up, a good approach to this would be to write a 
unit test. See `unittests/StaticAnalyzer` - there aren't many of them because 
they're relatively hard to write but these days they're getting more and more 
popular as we're trying to eliminate mutually exclusive bugs invisible on LIT 
tests. You can mock some code, emit a warning against it from a mocked checker, 
and test directly that the visitor is stopped at, say, a given node or a given 
line number.

Or you mean like, add an assert that ensures that you're doing right thing? I 
think this is also possible to some extent. For example, you can check when the 
visitor is stopped that no other visitors were added during the current 
`VisitNode()` invocation (otherwise the process has to continue). Or you could 
assert the opposite: if the visitor has run to completion and no other visitors 
were added then the callback has to be invoked. I think this can get messy 
really quickly (esp. knowing that you can't simply count all visitors to see if 
more were added, given how newly added visitors may be duplicates of existing 
visitors). But I suspect that it could be easy to check this in at least one 
direction.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:109
+// Callback type for trackExpressionValue
+using VisitorCallback = llvm::function_ref;

I think we have to use `std::function` here.
>   lang=c++
>   /// An efficient, type-erasing, non-owning reference to a callable. This is
>   /// intended for use as the type of a function parameter that is not used
>   /// after the function in question returns.
>   ///
>   /// This class does not own the callable, so it is not in general safe to 
> store
>   /// a function_ref.
>   template class function_ref;

An `llvm::function_ref` becomes a dangling reference as soon as 
`trackExpressionValue()` returns. This is too early.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103434

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


[PATCH] D103589: [Format] Fix incorrect pointer detection

2021-06-02 Thread Yilong Guo via Phabricator via cfe-commits
Nuu created this revision.
Nuu added a project: clang-format.
Nuu requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Before:

  void foo() { bar(float(1), a *b); }

After:

  void foo() { bar(float(1), a * b); }

Signed-off-by: Yilong Guo 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103589

Files:
  clang/lib/Format/TokenAnnotator.cpp


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -399,7 +399,7 @@
 HasMultipleParametersOnALine = true;
   if ((CurrentToken->Previous->isOneOf(tok::kw_const, tok::kw_auto) ||
CurrentToken->Previous->isSimpleTypeSpecifier()) &&
-  !CurrentToken->is(tok::l_brace))
+  !CurrentToken->isOneOf(tok::l_brace, tok::l_paren))
 Contexts.back().IsExpression = false;
   if (CurrentToken->isOneOf(tok::semi, tok::colon)) {
 MightBeObjCForRangeLoop = false;


Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -399,7 +399,7 @@
 HasMultipleParametersOnALine = true;
   if ((CurrentToken->Previous->isOneOf(tok::kw_const, tok::kw_auto) ||
CurrentToken->Previous->isSimpleTypeSpecifier()) &&
-  !CurrentToken->is(tok::l_brace))
+  !CurrentToken->isOneOf(tok::l_brace, tok::l_paren))
 Contexts.back().IsExpression = false;
   if (CurrentToken->isOneOf(tok::semi, tok::colon)) {
 MightBeObjCForRangeLoop = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

2021-06-02 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

In D97183#2794256 , @RedDocMD wrote:

> Important question from @vsavchenko:
>
>> I have two major questions about this implementation:
>>
>> - Why don't we need an actual check for `IfStmt`? Won't it trigger on `bool 
>> unused = !pointer;`? And if so it won't mean **constrained**.
>> - Why do we only care about implicit pointer-to-bool conversion? What about 
>> situations like `pointer == nullptr`, `NULL != pointer`, 
>> `__builtin_expect(pointer, 0)`, etc?

I think there's no way around re-using/generalizing the logic from 
`ConditionBRVisitor::VisitNode` in some form. I guess you could try to separate 
the part where it looks at the current program point and finds out what's 
constrained. Then apply it to the moment of time where the interesting 
constraint appears (whereas `ConditionBRVisitor` continously scans all program 
points with the same hopefully-reusable logic).




Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:258
+  const Decl *D = DS->getSingleDecl();
+  assert(D && "DeclStmt should have at least one Decl");
+  const auto *VD = llvm::dyn_cast(D);

RedDocMD wrote:
> RedDocMD wrote:
> > NoQ wrote:
> > > That's not what the assert is saying; the assert is saying that the 
> > > `DeclStmt` has //exactly// one `Decl`. It basically forbids code like
> > > ```
> > > int x = 1, y = 2;
> > > ```
> > > . You may wonder why don't you crash all over the place. That's because 
> > > Clang CFG [[ 
> > > https://github.com/llvm/llvm-project/blob/llvmorg-12.0.0/clang/lib/Analysis/CFG.cpp#L2826
> > >  | creates its own ]] `DeclStmt`s that aren't normally present in the 
> > > AST, that always have exactly one declaration. This is necessary because 
> > > there may be non-trivial control flow between these declarations (due to, 
> > > say, presence of operator `?:` in the initializer) so they have to be 
> > > represented as different elements (possibly even in different blocks) in 
> > > the CFG.
> > So I guess the tests at lines `317` and `378` of 
> > `smart-ptr-text-output.cpp` work because of the CFG messing with the AST? 
> > So should I remove the assert?
> @NoQ?
> So I guess the tests at lines 317 and 378 of smart-ptr-text-output.cpp work 
> because of the CFG messing with the AST?

Yes. The rest of the static analyzer works for the same reason; a lot of code 
relies on it.

> So should I remove the assert?

The assert is correct but the message is wrong / misleading.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183

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


[PATCH] D103386: [PowerPC] Fix x86 vector intrinsics wrapper compilation under C++

2021-06-02 Thread Qiu Chaofan via Phabricator via cfe-commits
qiucf added inline comments.



Comment at: clang/test/CodeGen/ppc-xmmintrin.c:10
 // RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns -o - | 
llvm-cxxfilt -n | FileCheck %s --check-prefixes=CHECK,CHECK-LE
+// RUN: %clang -x c++ -fsyntax-only -target powerpc64le-unknown-linux-gnu 
-mcpu=pwr8 -ffreestanding -DNO_WARN_X86_INTRINSICS %s \
+// RUN:   -fno-discard-value-names -mllvm -disable-llvm-optzns

bjope wrote:
> Unfortunately I get some failures with this. Maybe because of an unstandard 
> build setup.
> 
> We've started to use `-DCLANG_DEFAULT_RTLIB=compiler-rt 
> -DCLANG_DEFAULT_CXX_STDLIB=libc++` when building clang. And we also use 
> `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON`. Not sure if that is a setup that 
> is asking for trouble. Anyway, when running this test case we end up with
> 
> ```
> : 'RUN: at line 10';   /workspace/llvm/build/bin/clang -x c++ -fsyntax-only 
> -target powerpc64le-unknown-linux-gnu -mcpu=pwr8 -ffreestanding 
> -DNO_WARN_X86_INTRINSICS /workspace/clang/test/CodeGen/ppc-xmmintrin.c
> -fno-discard-value-names -mllvm -disable-llvm-optzns
> --
> Exit Code: 1
> 
> Command Output (stderr):
> --
> In file included from /workspace/clang/test/CodeGen/ppc-xmmintrin.c:13:
> In file included from 
> /workspace/llvm/build/lib/clang/13.0.0/include/ppc_wrappers/xmmintrin.h:42:
> In file included from 
> /workspace/llvm/build/lib/clang/13.0.0/include/altivec.h:44:
> In file included from /workspace/llvm/build/bin/../include/c++/v1/stddef.h:39:
> /workspace/llvm/build/bin/../include/c++/v1/__config:13:10: fatal error: 
> '__config_site' file not found
> #include <__config_site>
>  ^~~
> 1 error generated.
> ```
> 
> Not sure really how to solve that.
> 
> Maybe we should stop building like that?
> 
> Or there is a bug somewhere (such as that we only get the __config_site 
> headers in target specific dirs for targets that we actually build runtimes 
> for, maybe something that was missing in https://reviews.llvm.org/D97572)?
> (Maybe @phosek  could comment on that?)
> 
> Or this test case is missing some options to make it a bit more independent 
> on the runtimes build setup?
> 
The tests relies on some system stuff. Is the failure related to this change? 
(or exposed by `-x c++` option?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103386

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


[PATCH] D103184: [AArch64] handle -Wa,-march=

2021-06-02 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 349450.
jcai19 marked an inline comment as done.
jcai19 added a comment.

Add a test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103184

Files:
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/aarch64-target-as-march.s

Index: clang/test/Driver/aarch64-target-as-march.s
===
--- /dev/null
+++ clang/test/Driver/aarch64-target-as-march.s
@@ -0,0 +1,42 @@
+/// These tests make sure that options passed to the assembler
+/// via -Wa or -Xassembler are applied correctly to assembler inputs.
+
+/// -Wa/-Xassembler doesn't apply to non assembly files
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.1-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TARGET-FEATURE-1 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Xassembler -march=armv8.1-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TARGET-FEATURE-1 %s
+
+// TARGET-FEATURE-1-NOT: "-target-feature" "+v8.1a"
+
+/// -Wa/-Xassembler does apply to assembler input
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.2-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TARGET-FEATURE-2 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Xassembler -march=armv8.2-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TARGET-FEATURE-2 %s
+
+// TARGET-FEATURE-2: "-target-feature" "+v8.2a"
+
+/// No unused argument warnings when there are multiple values
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.1-a -Wa,-march=armv8.2-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=UNUSED-WARNING %s
+
+// UNUSED-WARNING-NOT: warning: argument unused during compilation
+
+/// Last march to assembler wins
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.2-a -Wa,-march=armv8.1-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=MULTIPLE-VALUES %s
+
+// MULTIPLE-VALUES: "-target-feature" "+v8.1a
+// MULTIPLE-VALUES-NOT: "-target-feature" "+v8.2a
+
+/// march to compiler and assembler, we choose the one suited to the input file type
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.3-a -march=armv8.4-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=TARGET-FEATURE-3 %s
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.3-a -march=armv8.4-a \
+// RUN: %S/Inputs/wildcard1.c 2>&1 | FileCheck --check-prefix=TARGET-FEATURE-4 %s
+
+// TARGET-FEATURE-3: "-target-feature" "+v8.3a"
+// TARGET-FEATURE-3-NOT: "-target-feature" "+v8.4a"
+// TARGET-FEATURE-4: "-target-feature" "+v8.4a"
+// TARGET-FEATURE-4-NOT: "-target-feature" "+v8.3a"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -344,7 +344,7 @@
   case llvm::Triple::aarch64:
   case llvm::Triple::aarch64_32:
   case llvm::Triple::aarch64_be:
-aarch64::getAArch64TargetFeatures(D, Triple, Args, Features);
+aarch64::getAArch64TargetFeatures(D, Triple, Args, Features, ForAS);
 break;
   case llvm::Triple::x86:
   case llvm::Triple::x86_64:
Index: clang/lib/Driver/ToolChains/Arch/AArch64.h
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.h
+++ clang/lib/Driver/ToolChains/Arch/AArch64.h
@@ -22,7 +22,8 @@
 
 void getAArch64TargetFeatures(const Driver &D, const llvm::Triple &Triple,
   const llvm::opt::ArgList &Args,
-  std::vector &Features);
+  std::vector &Features,
+  bool ForAS);
 
 std::string getAArch64TargetCPU(const llvm::opt::ArgList &Args,
 const llvm::Triple &Triple, llvm::opt::Arg *&A);
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -185,12 +185,20 @@
 void aarch64::getAArch64TargetFeatures(const Driver &D,
const llvm::Triple &Triple,
const ArgList &Args,
-   std::vector &Features) {
+   std::vector &Features,
+   bool ForAS) {
   Arg *A;
   bool success = true;
   // Enable NEON by default.
   Features.push_back("+neon");
-  if ((A = Args.getLastArg(options::OPT_march_EQ)))
+  if (ForAS &&
+  (A = Args.getLastArg(options::OPT_Wa_COMMA, options::OPT_Xassembler))) {
+llvm::StringRef WaMArch;
+for (StringRef Value : A->getValues())
+  if (Value.

[PATCH] D103587: Add AIX predefined macros

2021-06-02 Thread Jake Egan via Phabricator via cfe-commits
Jake-Egan created this revision.
Herald added subscribers: jfb, kbarton, nemanjai.
Jake-Egan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103587

Files:
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/PPC.cpp
  clang/test/Preprocessor/init-aix.c


Index: clang/test/Preprocessor/init-aix.c
===
--- /dev/null
+++ clang/test/Preprocessor/init-aix.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -E -dM < /dev/null | FileCheck -match-full-lines 
-check-prefix AIX %s
+// AIX:#define __THW_PPC__ 1
+// AIX:#define __TOS_AIX__ 1
+// AIX-NOT:#define __STDC_NO_ATOMICS__ 1
+// AIX-NOT:#define __STDC_NO_THREADS__ 1
+//
+// RUN: %clang_cc1 -x c -std=c11 -E -dM < /dev/null | FileCheck 
-match-full-lines -check-prefix AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=c1x -E -dM < /dev/null | FileCheck 
-match-full-lines -check-prefix AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=iso9899:2011 -E -dM < /dev/null | FileCheck 
-match-full-lines -check-prefix AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=gnu11 -E -dM < /dev/null | FileCheck 
-match-full-lines -check-prefix AIX-STDC %s
+// AIX-STDC:#define __STDC_NO_ATOMICS__ 1
+// AIX-STDC:#define __STDC_NO_THREADS__ 1
Index: clang/lib/Basic/Targets/PPC.cpp
===
--- clang/lib/Basic/Targets/PPC.cpp
+++ clang/lib/Basic/Targets/PPC.cpp
@@ -92,6 +92,7 @@
   Builder.defineMacro("__ppc__");
   Builder.defineMacro("__PPC__");
   Builder.defineMacro("_ARCH_PPC");
+  Builder.defineMacro("__THW_PPC__");
   Builder.defineMacro("__powerpc__");
   Builder.defineMacro("__POWERPC__");
   if (PointerWidth == 64) {
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -674,8 +674,15 @@
 Builder.defineMacro("_IBMR2");
 Builder.defineMacro("_POWER");
 
+Builder.defineMacro("__TOS_AIX__");
 Builder.defineMacro("_AIX");
 
+if (Opts.LangStd == LangStandard::lang_c11 ||
+Opts.LangStd == LangStandard::lang_gnu11){
+  Builder.defineMacro("__STDC_NO_ATOMICS__");
+  Builder.defineMacro("__STDC_NO_THREADS__");
+}
+
 if (Opts.EnableAIXExtendedAltivecABI)
   Builder.defineMacro("__EXTABI__");
 


Index: clang/test/Preprocessor/init-aix.c
===
--- /dev/null
+++ clang/test/Preprocessor/init-aix.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix AIX %s
+// AIX:#define __THW_PPC__ 1
+// AIX:#define __TOS_AIX__ 1
+// AIX-NOT:#define __STDC_NO_ATOMICS__ 1
+// AIX-NOT:#define __STDC_NO_THREADS__ 1
+//
+// RUN: %clang_cc1 -x c -std=c11 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=c1x -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=iso9899:2011 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix AIX-STDC %s
+// RUN: %clang_cc1 -x c -std=gnu11 -E -dM < /dev/null | FileCheck -match-full-lines -check-prefix AIX-STDC %s
+// AIX-STDC:#define __STDC_NO_ATOMICS__ 1
+// AIX-STDC:#define __STDC_NO_THREADS__ 1
Index: clang/lib/Basic/Targets/PPC.cpp
===
--- clang/lib/Basic/Targets/PPC.cpp
+++ clang/lib/Basic/Targets/PPC.cpp
@@ -92,6 +92,7 @@
   Builder.defineMacro("__ppc__");
   Builder.defineMacro("__PPC__");
   Builder.defineMacro("_ARCH_PPC");
+  Builder.defineMacro("__THW_PPC__");
   Builder.defineMacro("__powerpc__");
   Builder.defineMacro("__POWERPC__");
   if (PointerWidth == 64) {
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -674,8 +674,15 @@
 Builder.defineMacro("_IBMR2");
 Builder.defineMacro("_POWER");
 
+Builder.defineMacro("__TOS_AIX__");
 Builder.defineMacro("_AIX");
 
+if (Opts.LangStd == LangStandard::lang_c11 ||
+Opts.LangStd == LangStandard::lang_gnu11){
+  Builder.defineMacro("__STDC_NO_ATOMICS__");
+  Builder.defineMacro("__STDC_NO_THREADS__");
+}
+
 if (Opts.EnableAIXExtendedAltivecABI)
   Builder.defineMacro("__EXTABI__");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D102180: [Clang][OpenMP] Emit dependent PreInits before directive.

2021-06-02 Thread Michael Kruse via Phabricator via cfe-commits
Meinersbur added a comment.

Instead of reverting, I fixed the test in rG64e5a3bbdde2 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102180

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


[clang] 64e5a3b - [clang] Fix fail of OpenMP/tile_codegen_tile_for.cpp.

2021-06-02 Thread Michael Kruse via cfe-commits

Author: Michael Kruse
Date: 2021-06-02T21:02:05-05:00
New Revision: 64e5a3bbdde25af0fd9f2b9b8539e23f36c80601

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

LOG: [clang] Fix fail of OpenMP/tile_codegen_tile_for.cpp.

Clang's version string can be customized using CLANG_VENDOR which the
test did not consider. Change the test to accept any version string.

Added: 


Modified: 
clang/test/OpenMP/tile_codegen_tile_for.cpp

Removed: 




diff  --git a/clang/test/OpenMP/tile_codegen_tile_for.cpp 
b/clang/test/OpenMP/tile_codegen_tile_for.cpp
index 314446ca203bf..e4e5548b2f933 100644
--- a/clang/test/OpenMP/tile_codegen_tile_for.cpp
+++ b/clang/test/OpenMP/tile_codegen_tile_for.cpp
@@ -247,7 +247,7 @@ extern "C" void func(int start, int end, int step) {
 
 #endif /* HEADER */
 // IR: ![[META0:[0-9]+]] = !{i32 1, !"wchar_size", i32 4}
-// IR: ![[META1:[0-9]+]] = !{!"clang version {{[^"]*}}"}
+// IR: ![[META1:[0-9]+]] = !{!"{{[^"]*}}"}
 // IR: ![[LOOP2]] = distinct !{![[LOOP2]], ![[LOOPPROP3:[0-9]+]]}
 // IR: ![[LOOPPROP3]] = !{!"llvm.loop.mustprogress"}
 // IR: ![[LOOP4]] = distinct !{![[LOOP4]], ![[LOOPPROP3]]}



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


[PATCH] D103564: [NFC][compiler-rt][hwasan] Wrap vfork interceptor with HWASAN_WITH_INTERCEPTORS

2021-06-02 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added a comment.

I think probably the better refactoring here would be to split this file in 
two: one containing just the allocation functions; and a second with everything 
else.
On Fuchsia, we'll use the allocation functions but none of the rest of it.  So 
the second whole file could just be under `#if !SANITIZER_FUCHSIA`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103564

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


[PATCH] D102180: [Clang][OpenMP] Emit dependent PreInits before directive.

2021-06-02 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

`OpenMP/tile_codegen_tile_for.cpp` is failing on all our bots, would it be 
possible to revert the change?

  FAIL: Clang :: OpenMP/tile_codegen_tile_for.cpp (9951 of 28034)
   TEST 'Clang :: OpenMP/tile_codegen_tile_for.cpp' FAILED 

  Script:
  --
  : 'RUN: at line 2';   /opt/s/w/ir/x/w/staging/llvm_build/bin/clang -cc1 
-internal-isystem /opt/s/w/ir/x/w/staging/llvm_build/lib/clang/13.0.0/include 
-nostdsysteminc -verify -triple x86_64-pc-linux-gnu -fopenmp 
-fopenmp-version=51 -emit-llvm 
/opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/tile_codegen_tile_for.cpp -o - | 
/opt/s/w/ir/x/w/staging/llvm_build/bin/FileCheck --allow-unused-prefixes 
/opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/tile_codegen_tile_for.cpp 
--check-prefix=IR
  : 'RUN: at line 5';   /opt/s/w/ir/x/w/staging/llvm_build/bin/clang -cc1 
-internal-isystem /opt/s/w/ir/x/w/staging/llvm_build/lib/clang/13.0.0/include 
-nostdsysteminc -verify -triple x86_64-pc-linux-gnu -fopenmp 
-fopenmp-version=51 -emit-pch -o 
/opt/s/w/ir/x/w/staging/llvm_build/tools/clang/test/OpenMP/Output/tile_codegen_tile_for.cpp.tmp
 /opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/tile_codegen_tile_for.cpp
  : 'RUN: at line 6';   /opt/s/w/ir/x/w/staging/llvm_build/bin/clang -cc1 
-internal-isystem /opt/s/w/ir/x/w/staging/llvm_build/lib/clang/13.0.0/include 
-nostdsysteminc -verify -triple x86_64-pc-linux-gnu -fopenmp 
-fopenmp-version=51 -include-pch 
/opt/s/w/ir/x/w/staging/llvm_build/tools/clang/test/OpenMP/Output/tile_codegen_tile_for.cpp.tmp
 -emit-llvm 
/opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/tile_codegen_tile_for.cpp -o - | 
/opt/s/w/ir/x/w/staging/llvm_build/bin/FileCheck --allow-unused-prefixes 
/opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/tile_codegen_tile_for.cpp 
--check-prefix=IR
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  
/opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/tile_codegen_tile_for.cpp:250:8: 
error: IR: expected string not found in input
  // IR: ![[META1:[0-9]+]] = !{!"clang version {{[^"]*}}"}
 ^
  :260:36: note: scanning from here
  !0 = !{i32 1, !"wchar_size", i32 4}
 ^
  :261:1: note: possible intended match here
  !1 = !{!"Fuchsia clang version 13.0.0 
(https://llvm.googlesource.com/a/llvm-project 
07a6beb402150d25ec7c93a5747520ac2804731d)"}
  ^
  
  Input file: 
  Check file: 
/opt/s/w/ir/x/w/llvm-project/clang/test/OpenMP/tile_codegen_tile_for.cpp
  
  -dump-input=help explains the following input dump.
  
  Input was:
  <<
   .
   .
   .
 255: attributes #2 = { convergent nounwind } 
 256:  
 257: !llvm.module.flags = !{!0} 
 258: !llvm.ident = !{!1} 
 259:  
 260: !0 = !{i32 1, !"wchar_size", i32 4} 
  check:250'0X error: no match found
 261: !1 = !{!"Fuchsia clang version 13.0.0 
(https://llvm.googlesource.com/a/llvm-project 
07a6beb402150d25ec7c93a5747520ac2804731d)"} 
  check:250'0 

  check:250'1 ? 
   possible 
intended match
 262: !2 = distinct !{!2, !3} 
  check:250'0 
 263: !3 = !{!"llvm.loop.mustprogress"} 
  check:250'0 ~~
 264: !4 = distinct !{!4, !3} 
  check:250'0 
  >>
  
  --
  
  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102180

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


[PATCH] D103565: Fix "control reaches end of non-void function" warnings on ppc64le

2021-06-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb2c8bcbab8a4: Fix "control reaches end of non-void 
function" warnings on ppc64le (authored by zhaomo, committed by ymandel).

Changed prior to commit:
  https://reviews.llvm.org/D103565?vs=349407&id=349437#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103565

Files:
  clang/lib/ASTMatchers/GtestMatchers.cpp


Index: clang/lib/ASTMatchers/GtestMatchers.cpp
===
--- clang/lib/ASTMatchers/GtestMatchers.cpp
+++ clang/lib/ASTMatchers/GtestMatchers.cpp
@@ -53,6 +53,7 @@
   case GtestCmp::Lt:
 return functionDecl(hasName("::testing::internal::CmpHelperLT"));
   }
+  llvm_unreachable("Unhandled GtestCmp enum");
 }
 
 static llvm::StringRef getMacroTypeName(MacroType Macro) {
@@ -64,6 +65,7 @@
   case MacroType::On:
 return "ON";
   }
+  llvm_unreachable("Unhandled MacroType enum");
 }
 
 static llvm::StringRef getComparisonTypeName(GtestCmp Cmp) {
@@ -81,6 +83,7 @@
   case GtestCmp::Lt:
 return "LT";
   }
+  llvm_unreachable("Unhandled GtestCmp enum");
 }
 
 static std::string getMacroName(MacroType Macro, GtestCmp Cmp) {
@@ -104,6 +107,7 @@
   default:
 llvm_unreachable("Unhandled MacroType enum");
   }
+  llvm_unreachable("Unhandled MacroType enum");
 }
 
 // In general, AST matchers cannot match calls to macros. However, we can
@@ -170,6 +174,7 @@
 hasOverloadedOperatorName("()"), argumentCountIs(3),
 hasArgument(0, ignoringImplicit(MockCall));
   }
+  llvm_unreachable("Unhandled MockArgs enum");
 }
 
 static internal::BindableMatcher


Index: clang/lib/ASTMatchers/GtestMatchers.cpp
===
--- clang/lib/ASTMatchers/GtestMatchers.cpp
+++ clang/lib/ASTMatchers/GtestMatchers.cpp
@@ -53,6 +53,7 @@
   case GtestCmp::Lt:
 return functionDecl(hasName("::testing::internal::CmpHelperLT"));
   }
+  llvm_unreachable("Unhandled GtestCmp enum");
 }
 
 static llvm::StringRef getMacroTypeName(MacroType Macro) {
@@ -64,6 +65,7 @@
   case MacroType::On:
 return "ON";
   }
+  llvm_unreachable("Unhandled MacroType enum");
 }
 
 static llvm::StringRef getComparisonTypeName(GtestCmp Cmp) {
@@ -81,6 +83,7 @@
   case GtestCmp::Lt:
 return "LT";
   }
+  llvm_unreachable("Unhandled GtestCmp enum");
 }
 
 static std::string getMacroName(MacroType Macro, GtestCmp Cmp) {
@@ -104,6 +107,7 @@
   default:
 llvm_unreachable("Unhandled MacroType enum");
   }
+  llvm_unreachable("Unhandled MacroType enum");
 }
 
 // In general, AST matchers cannot match calls to macros. However, we can
@@ -170,6 +174,7 @@
 hasOverloadedOperatorName("()"), argumentCountIs(3),
 hasArgument(0, ignoringImplicit(MockCall));
   }
+  llvm_unreachable("Unhandled MockArgs enum");
 }
 
 static internal::BindableMatcher
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] b2c8bcb - Fix "control reaches end of non-void function" warnings on ppc64le

2021-06-02 Thread Yitzhak Mandelbaum via cfe-commits

Author: Zhaomo Yang
Date: 2021-06-03T00:53:53Z
New Revision: b2c8bcbab8a44c4582632845697b9425c3200230

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

LOG: Fix "control reaches end of non-void function" warnings on ppc64le

Warnings can be found here: 
https://lab.llvm.org/buildbot/#/builders/76/builds/2640

Reviewed By: ymandel

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

Added: 


Modified: 
clang/lib/ASTMatchers/GtestMatchers.cpp

Removed: 




diff  --git a/clang/lib/ASTMatchers/GtestMatchers.cpp 
b/clang/lib/ASTMatchers/GtestMatchers.cpp
index 79f69c63c10f0..6e4c12f319692 100644
--- a/clang/lib/ASTMatchers/GtestMatchers.cpp
+++ b/clang/lib/ASTMatchers/GtestMatchers.cpp
@@ -53,6 +53,7 @@ static DeclarationMatcher getComparisonDecl(GtestCmp Cmp) {
   case GtestCmp::Lt:
 return functionDecl(hasName("::testing::internal::CmpHelperLT"));
   }
+  llvm_unreachable("Unhandled GtestCmp enum");
 }
 
 static llvm::StringRef getMacroTypeName(MacroType Macro) {
@@ -64,6 +65,7 @@ static llvm::StringRef getMacroTypeName(MacroType Macro) {
   case MacroType::On:
 return "ON";
   }
+  llvm_unreachable("Unhandled MacroType enum");
 }
 
 static llvm::StringRef getComparisonTypeName(GtestCmp Cmp) {
@@ -81,6 +83,7 @@ static llvm::StringRef getComparisonTypeName(GtestCmp Cmp) {
   case GtestCmp::Lt:
 return "LT";
   }
+  llvm_unreachable("Unhandled GtestCmp enum");
 }
 
 static std::string getMacroName(MacroType Macro, GtestCmp Cmp) {
@@ -104,6 +107,7 @@ static llvm::StringRef getSpecSetterName(MacroType Macro) {
   default:
 llvm_unreachable("Unhandled MacroType enum");
   }
+  llvm_unreachable("Unhandled MacroType enum");
 }
 
 // In general, AST matchers cannot match calls to macros. However, we can
@@ -170,6 +174,7 @@ gtestCallInternal(MacroType Macro, StatementMatcher 
MockCall, MockArgs Args) {
 hasOverloadedOperatorName("()"), argumentCountIs(3),
 hasArgument(0, ignoringImplicit(MockCall));
   }
+  llvm_unreachable("Unhandled MockArgs enum");
 }
 
 static internal::BindableMatcher



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


[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 349434.
feg208 added a comment.

Adds a simple lit test for some sanity checks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/WhitespaceManager.cpp
  clang/lib/Format/WhitespaceManager.h
  clang/test/Format/struct-array-initializer.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16401,6 +16401,229 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructures) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = true;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"}, // first line\n"
+   "{-1, 93463, \"world\"}, // second line\n"
+   "{ 7, 5,\"!!\"}  // third line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[4] = {\n"
+   "{ 56,23, 21,   \"oh\"}, // first line\n"
+   "{ -1, 93463, 22,   \"my\"}, // second line\n"
+   "{  7, 5,  1, \"goodness\"}  // third line\n"
+   "{234, 5,  1, \"gracious\"}  // fourth line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[3] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[3] = {\n"
+   "{int{56},23, \"hello\"},\n"
+   "{int{-1}, 93463, \"world\"},\n"
+   "{ int{7}, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+  verifyFormat("demo = std::array{\n"
+   "test{56,23, \"hello\"},\n"
+   "test{-1, 93463, \"world\"},\n"
+   "test{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "#if X\n"
+   "{-1, 93463, \"world\"},\n"
+   "#endif\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat(
+  "test demo[] = {\n"
+  "{ 7,23,\n"
+  " \"hello world i am a very long line that really, in any\"\n"
+  " \"just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  \"world\"},\n"
+  "{56, 5, \"!!\"}\n"
+  "};\n",
+  Style);
+  verifyFormat("return GradForUnaryCwise(g, {\n"
+   "{{\"sign\"}, \"Sign\",  "
+   "  {\"x\", \"dy\"}},\n"
+   "{  {\"dx\"},  \"Mul\", {\"dy\""
+   ", \"sign\"}},\n"
+   "});\n",
+   Style);
+
+  Style.ColumnLimit = 0;
+  EXPECT_EQ(
+  "test demo[] = {\n"
+  "{56,23, \"hello world i am a very long line that really, "
+  "in any just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  "
+  " \"world\"},\n"
+  "{ 7, 5,  "
+  "\"!!\"},\n"
+  "};",
+  format("test demo[] = {{56, 23, \"hello world i am a very long line "
+ "that rea

[PATCH] D103579: [LTO] Fix -fwhole-program-vtables handling after HIP ThinLTO patch

2021-06-02 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson added inline comments.



Comment at: clang/test/Driver/hip-options.hip:72
+// HIPTHINLTO-NOT: error: invalid argument '-fwhole-program-vtables' only 
allowed with '-flto'
+// HIPTHINLTO-NOT: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} 
"-flto-unit"
+// HIPTHINLTO: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-flto=thin" 
"-flto-unit" {{.*}} "-fwhole-program-vtables"

These are essentially the same checks as before, but I renamed this tag to 
HIPTHINLTO, added a check to ensure we don't get the error with 
-fwhole-program-vtables, ensure -fwhole-program-vtables passed through for the 
offload cc1 command, and added a duplicate of the HIPTHINLTO-NOT check (since 
in my local build the non-offload cc1 is emitted after the offload cc1, not 
sure if there is some variation).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103579

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


[PATCH] D103579: [LTO] Fix -fwhole-program-vtables handling after HIP ThinLTO patch

2021-06-02 Thread Teresa Johnson via Phabricator via cfe-commits
tejohnson created this revision.
tejohnson added reviewers: yaxunl, tra.
Herald added a subscriber: inglorion.
tejohnson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

A recent change (D99683 ) to support ThinLTO 
for HIP caused a regression
when compiling cuda code with -flto=thin -fwhole-program-vtables.
Specifically, we now get an error:
error: invalid argument '-fwhole-program-vtables' only allowed with '-flto'

This error is coming from the device offload cc1 action being set up for
the cuda compile, for which -flto=thin doesn't apply and gets dropped.
This is a regression, but points to a potential issue that was silently
occurring before the patch, details below.

Before D99683 , the check for 
fwhole-program-vtables in the driver looked
like:

  if (WholeProgramVTables) {
if (!D.isUsingLTO())
  D.Diag(diag::err_drv_argument_only_allowed_with)
  << "-fwhole-program-vtables"
  << "-flto";
CmdArgs.push_back("-fwhole-program-vtables");
  }

And D.isUsingLTO() returned true since we have -flto=thin. However,
because the cuda cc1 compile is doing device offloading, which didn't
support any LTO, there was other code that suppressed -flto* options
from being passed to the cc1 invocation. So the cc1 invocation silently
had -fwhole-program-vtables without any -flto*. This seems potentially
problematic, since if we had any virtual calls we would get type test
assume sequences without the corresponding LTO pass that handles them.

However, with the patch, which adds support for device offloading LTO
option -foffload-lto=thin, the code has changed so that we set a bool
IsUsingLTO based on either -flto* or -foffload-lto*, depending on
whether this is the device offloading action. For the device offload
action in our compile, since we don't have -foffload-lto, IsUsingLTO is
false, and the check for LTO with -fwhole-program-vtables now fails.

What we should do is only pass through -fwhole-program-vtables to the
cc1 invocation that has LTO enabled (either the device offload action
with -foffload-lto, or the non-device offload action with -flto), and
otherwise drop the -fwhole-program-vtables for the non-LTO action.
Then we should error only if we have -fwhole-program-vtables without any
-f*lto* options.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103579

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/hip-options.hip


Index: clang/test/Driver/hip-options.hip
===
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -60,13 +60,30 @@
 // Check -foffload-lto=thin translated correctly.
 
 // RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
-// RUN:   --cuda-gpu-arch=gfx906 -foffload-lto=thin %s 2>&1 \
-// RUN:   | FileCheck -check-prefix=THINLTO %s
+// RUN:   --cuda-gpu-arch=gfx906 -foffload-lto=thin -fwhole-program-vtables %s 
2>&1 \
+// RUN:   | FileCheck -check-prefix=HIPTHINLTO %s
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --cuda-gpu-arch=gfx906 -fgpu-rdc -foffload-lto=thin 
-fwhole-program-vtables %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=HIPTHINLTO %s
 
+// Ensure we don't error about -fwhole-program-vtables for the non-device 
offload compile.
+// HIPTHINLTO-NOT: error: invalid argument '-fwhole-program-vtables' only 
allowed with '-flto'
+// HIPTHINLTO-NOT: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} 
"-flto-unit"
+// HIPTHINLTO: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-flto=thin" 
"-flto-unit" {{.*}} "-fwhole-program-vtables"
+// HIPTHINLTO-NOT: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} 
"-flto-unit"
+// HIPTHINLTO: lld{{.*}}"-plugin-opt=mcpu=gfx906" "-plugin-opt=thinlto" 
"-plugin-opt=-force-import-all"
+
+// Check that -flto=thin is handled correctly, particularly with 
-fwhole-program-vtables.
+//
 // RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
-// RUN:   --cuda-gpu-arch=gfx906 -fgpu-rdc -foffload-lto=thin %s 2>&1 \
+// RUN:   --cuda-gpu-arch=gfx906 -flto=thin -fwhole-program-vtables %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=THINLTO %s
 
-// THINLTO-NOT: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} 
"-flto-unit"
-// THINLTO: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} "-flto=thin" 
"-flto-unit"
-// THINLTO: lld{{.*}}"-plugin-opt=mcpu=gfx906" "-plugin-opt=thinlto" 
"-plugin-opt=-force-import-all"
+// Ensure we don't error about -fwhole-program-vtables for the device offload 
compile. We should
+// drop -fwhole-program-vtables for the device offload compile and pass it 
through for the
+// non-device offload compile along with -flto=thin.
+// THINLTO-NOT: error: invalid argument '-fwhole-program-vtables' only allowed 
with '-flto'
+// THINLTO-NOT: clang{{.*}}" "-triple" "

[PATCH] D101741: [clangd] Improve resolution of static method calls in HeuristicResolver

2021-06-02 Thread Nathan Ridge via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf976b9997ee5: [clangd] Improve resolution of static method 
calls in HeuristicResolver (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101741

Files:
  clang-tools-extra/clangd/HeuristicResolver.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -285,7 +285,6 @@
   void bar(A a, T t) {
 nonmember($par1[[t]]);
 a.member($par2[[t]]);
-// FIXME: This one does not work yet.
 A::static_member($par3[[t]]);
 // We don't want to arbitrarily pick between
 // "anInt" or "aDouble", so just show no hint.
@@ -294,7 +293,8 @@
 };
   )cpp",
ExpectedHint{"par1: ", "par1"},
-   ExpectedHint{"par2: ", "par2"});
+   ExpectedHint{"par2: ", "par2"},
+   ExpectedHint{"par3: ", "par3"});
 }
 
 TEST(ParameterHints, VariadicFunction) {
Index: clang-tools-extra/clangd/HeuristicResolver.cpp
===
--- clang-tools-extra/clangd/HeuristicResolver.cpp
+++ clang-tools-extra/clangd/HeuristicResolver.cpp
@@ -16,6 +16,7 @@
 
 // Convenience lambdas for use as the 'Filter' parameter of
 // HeuristicResolver::resolveDependentMember().
+const auto NoFilter = [](const NamedDecl *D) { return true; };
 const auto NonStaticFilter = [](const NamedDecl *D) {
   return D->isCXXInstanceMember();
 };
@@ -90,6 +91,28 @@
 
 std::vector HeuristicResolver::resolveMemberExpr(
 const CXXDependentScopeMemberExpr *ME) const {
+  // If the expression has a qualifier, first try resolving the member
+  // inside the qualifier's type.
+  // Note that we cannot use a NonStaticFilter in either case, for a couple
+  // of reasons:
+  //   1. It's valid to access a static member using instance member syntax,
+  //  e.g. `instance.static_member`.
+  //   2. We can sometimes get a CXXDependentScopeMemberExpr for static
+  //  member syntax too, e.g. if `X::static_member` occurs inside
+  //  an instance method, it's represented as a CXXDependentScopeMemberExpr
+  //  with `this` as the base expression as `X` as the qualifier
+  //  (which could be valid if `X` names a base class after instantiation).
+  if (NestedNameSpecifier *NNS = ME->getQualifier()) {
+if (const Type *QualifierType = resolveNestedNameSpecifierToType(NNS)) {
+  auto Decls =
+  resolveDependentMember(QualifierType, ME->getMember(), NoFilter);
+  if (!Decls.empty())
+return Decls;
+}
+  }
+
+  // If that didn't yield any results, try resolving the member inside
+  // the expression's base type.
   const Type *BaseType = ME->getBaseType().getTypePtrOrNull();
   if (ME->isArrow()) {
 BaseType = getPointeeType(BaseType);
@@ -105,7 +128,7 @@
   BaseType = resolveExprToType(Base);
 }
   }
-  return resolveDependentMember(BaseType, ME->getMember(), NonStaticFilter);
+  return resolveDependentMember(BaseType, ME->getMember(), NoFilter);
 }
 
 std::vector HeuristicResolver::resolveDeclRefExpr(


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -285,7 +285,6 @@
   void bar(A a, T t) {
 nonmember($par1[[t]]);
 a.member($par2[[t]]);
-// FIXME: This one does not work yet.
 A::static_member($par3[[t]]);
 // We don't want to arbitrarily pick between
 // "anInt" or "aDouble", so just show no hint.
@@ -294,7 +293,8 @@
 };
   )cpp",
ExpectedHint{"par1: ", "par1"},
-   ExpectedHint{"par2: ", "par2"});
+   ExpectedHint{"par2: ", "par2"},
+   ExpectedHint{"par3: ", "par3"});
 }
 
 TEST(ParameterHints, VariadicFunction) {
Index: clang-tools-extra/clangd/HeuristicResolver.cpp
===
--- clang-tools-extra/clangd/HeuristicResolver.cpp
+++ clang-tools-extra/clangd/HeuristicResolver.cpp
@@ -16,6 +16,7 @@
 
 // Convenience lambdas for use as the 'Filter' parameter of
 // HeuristicResolver::resolveDependentMember().
+const auto NoFilter = [](const NamedDecl *D) { return true; };
 const auto NonStaticFilter = [](const NamedDecl *D) {
   return D->isCXXInstanceMember();
 };
@@ -90,6 +91,28 @@
 
 std::vector HeuristicResolver::resolveMemberExpr(
 const CXXDependentSco

[clang-tools-extra] f976b99 - [clangd] Improve resolution of static method calls in HeuristicResolver

2021-06-02 Thread Nathan Ridge via cfe-commits

Author: Nathan Ridge
Date: 2021-06-02T20:30:19-04:00
New Revision: f976b9997ee55a130b139efe7b6e6f3b0384016b

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

LOG: [clangd] Improve resolution of static method calls in HeuristicResolver

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

Added: 


Modified: 
clang-tools-extra/clangd/HeuristicResolver.cpp
clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/HeuristicResolver.cpp 
b/clang-tools-extra/clangd/HeuristicResolver.cpp
index 316b7d11224b..b0a7448a04ef 100644
--- a/clang-tools-extra/clangd/HeuristicResolver.cpp
+++ b/clang-tools-extra/clangd/HeuristicResolver.cpp
@@ -16,6 +16,7 @@ namespace clangd {
 
 // Convenience lambdas for use as the 'Filter' parameter of
 // HeuristicResolver::resolveDependentMember().
+const auto NoFilter = [](const NamedDecl *D) { return true; };
 const auto NonStaticFilter = [](const NamedDecl *D) {
   return D->isCXXInstanceMember();
 };
@@ -90,6 +91,28 @@ const Type *HeuristicResolver::getPointeeType(const Type *T) 
const {
 
 std::vector HeuristicResolver::resolveMemberExpr(
 const CXXDependentScopeMemberExpr *ME) const {
+  // If the expression has a qualifier, first try resolving the member
+  // inside the qualifier's type.
+  // Note that we cannot use a NonStaticFilter in either case, for a couple
+  // of reasons:
+  //   1. It's valid to access a static member using instance member syntax,
+  //  e.g. `instance.static_member`.
+  //   2. We can sometimes get a CXXDependentScopeMemberExpr for static
+  //  member syntax too, e.g. if `X::static_member` occurs inside
+  //  an instance method, it's represented as a CXXDependentScopeMemberExpr
+  //  with `this` as the base expression as `X` as the qualifier
+  //  (which could be valid if `X` names a base class after instantiation).
+  if (NestedNameSpecifier *NNS = ME->getQualifier()) {
+if (const Type *QualifierType = resolveNestedNameSpecifierToType(NNS)) {
+  auto Decls =
+  resolveDependentMember(QualifierType, ME->getMember(), NoFilter);
+  if (!Decls.empty())
+return Decls;
+}
+  }
+
+  // If that didn't yield any results, try resolving the member inside
+  // the expression's base type.
   const Type *BaseType = ME->getBaseType().getTypePtrOrNull();
   if (ME->isArrow()) {
 BaseType = getPointeeType(BaseType);
@@ -105,7 +128,7 @@ std::vector 
HeuristicResolver::resolveMemberExpr(
   BaseType = resolveExprToType(Base);
 }
   }
-  return resolveDependentMember(BaseType, ME->getMember(), NonStaticFilter);
+  return resolveDependentMember(BaseType, ME->getMember(), NoFilter);
 }
 
 std::vector HeuristicResolver::resolveDeclRefExpr(

diff  --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp 
b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index 026479275a47..3c5503448ab8 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -285,7 +285,6 @@ TEST(ParameterHints, DependentCalls) {
   void bar(A a, T t) {
 nonmember($par1[[t]]);
 a.member($par2[[t]]);
-// FIXME: This one does not work yet.
 A::static_member($par3[[t]]);
 // We don't want to arbitrarily pick between
 // "anInt" or "aDouble", so just show no hint.
@@ -294,7 +293,8 @@ TEST(ParameterHints, DependentCalls) {
 };
   )cpp",
ExpectedHint{"par1: ", "par1"},
-   ExpectedHint{"par2: ", "par2"});
+   ExpectedHint{"par2: ", "par2"},
+   ExpectedHint{"par3: ", "par3"});
 }
 
 TEST(ParameterHints, VariadicFunction) {



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


[PATCH] D103549: [POC] Put annotation strings into debuginfo.

2021-06-02 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song added a comment.

@dblaikie Thanks for the pointer! I will work on the new patch, starting with 
DW_AT_BTF_annotation (to limit the scope where annotations will be processed) 
and post a new patch soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103549

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


[PATCH] D103565: Fix "control reaches end of non-void function" warnings on ppc64le

2021-06-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added a comment.

Actually, the removal of the default case in the switch gives this warning:
...llvm-project/clang/lib/ASTMatchers/GtestMatchers.cpp:102:11: warning: 
enumeration value 'Assert' not handled in switch [-Wswitch]

I'll reinstate the default case before comitting the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103565

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


[clang] 9d070b2 - Recommit "Fix tmp files being left on Windows builds." with a fix for

2021-06-02 Thread Amy Huang via cfe-commits

Author: Amy Huang
Date: 2021-06-02T16:50:37-07:00
New Revision: 9d070b2f4889887f9ce497592ef01df7b9601a1c

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

LOG: Recommit "Fix tmp files being left on Windows builds." with a fix for
incorrect std::string use. (Also remove redundant call to
RemoveFileOnSignal.)

Clang writes object files by first writing to a .tmp file and then
renaming to the final .obj name. On Windows, if a compile is killed
partway through the .tmp files don't get deleted.

Currently it seems like RemoveFileOnSignal takes care of deleting the
tmp files on Linux, but on Windows we need to call
setDeleteDisposition on tmp files so that they are deleted when
closed.

This patch switches to using TempFile to create the .tmp files we write
when creating object files, since it uses setDeleteDisposition on Windows.
This change applies to both Linux and Windows for consistency.

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

This reverts commit 20797b129f844d4b12ffb2b12cf33baa2d42985c.

Added: 


Modified: 
clang/include/clang/Frontend/CompilerInstance.h
clang/lib/Frontend/CompilerInstance.cpp
llvm/lib/Support/Path.cpp
llvm/lib/Support/Windows/Path.inc

Removed: 




diff  --git a/clang/include/clang/Frontend/CompilerInstance.h 
b/clang/include/clang/Frontend/CompilerInstance.h
index 5e879d672d801..9d6dd8fa1006f 100644
--- a/clang/include/clang/Frontend/CompilerInstance.h
+++ b/clang/include/clang/Frontend/CompilerInstance.h
@@ -22,6 +22,7 @@
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/BuryPointer.h"
+#include "llvm/Support/FileSystem.h"
 #include 
 #include 
 #include 
@@ -165,11 +166,10 @@ class CompilerInstance : public ModuleLoader {
   /// failed.
   struct OutputFile {
 std::string Filename;
-std::string TempFilename;
+Optional File;
 
-OutputFile(std::string filename, std::string tempFilename)
-: Filename(std::move(filename)), TempFilename(std::move(tempFilename)) 
{
-}
+OutputFile(std::string filename, Optional file)
+: Filename(std::move(filename)), File(std::move(file)) {}
   };
 
   /// The list of active output files.

diff  --git a/clang/lib/Frontend/CompilerInstance.cpp 
b/clang/lib/Frontend/CompilerInstance.cpp
index 5df40c742d469..32e7fdf64fa93 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -703,31 +703,37 @@ void CompilerInstance::createSema(TranslationUnitKind 
TUKind,
 // Output Files
 
 void CompilerInstance::clearOutputFiles(bool EraseFiles) {
+  // Ignore errors that occur when trying to discard the temp file.
   for (OutputFile &OF : OutputFiles) {
 if (EraseFiles) {
-  if (!OF.TempFilename.empty()) {
-llvm::sys::fs::remove(OF.TempFilename);
-continue;
-  }
+  if (OF.File)
+consumeError(OF.File->discard());
   if (!OF.Filename.empty())
 llvm::sys::fs::remove(OF.Filename);
   continue;
 }
 
-if (OF.TempFilename.empty())
+if (!OF.File)
   continue;
 
+if (OF.File->TmpName.empty()) {
+  consumeError(OF.File->discard());
+  continue;
+}
+
 // If '-working-directory' was passed, the output filename should be
 // relative to that.
 SmallString<128> NewOutFile(OF.Filename);
 FileMgr->FixupRelativePath(NewOutFile);
-std::error_code EC = llvm::sys::fs::rename(OF.TempFilename, NewOutFile);
-if (!EC)
+
+llvm::Error E = OF.File->keep(NewOutFile);
+if (!E)
   continue;
+
 getDiagnostics().Report(diag::err_unable_to_rename_temp)
-<< OF.TempFilename << OF.Filename << EC.message();
+<< OF.File->TmpName << OF.Filename << std::move(E);
 
-llvm::sys::fs::remove(OF.TempFilename);
+llvm::sys::fs::remove(OF.File->TmpName);
   }
   OutputFiles.clear();
   if (DeleteBuiltModules) {
@@ -809,7 +815,7 @@ CompilerInstance::createOutputFileImpl(StringRef 
OutputPath, bool Binary,
 }
   }
 
-  std::string TempFile;
+  Optional Temp;
   if (UseTemporary) {
 // Create a temporary file.
 // Insert - before the extension (if any), and because some tools
@@ -821,25 +827,34 @@ CompilerInstance::createOutputFileImpl(StringRef 
OutputPath, bool Binary,
 TempPath += "-";
 TempPath += OutputExtension;
 TempPath += ".tmp";
-int fd;
-std::error_code EC = llvm::sys::fs::createUniqueFile(
-TempPath, fd, TempPath,
-Binary ? llvm::sys::fs::OF_None : llvm::sys::fs::OF_Text);
-
-if (CreateMissingDirectories &&
-EC == llvm::errc::no_such_file_or_directory) {
-  StringRef Parent = llvm::sys::path::parent_path(OutputPath);
-  EC = llvm::sys::fs::create_directories(Parent);
-  if (!EC) {
-

[PATCH] D103565: Fix "control reaches end of non-void function" warnings on ppc64le

2021-06-02 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel accepted this revision.
ymandel added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103565

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


[PATCH] D103549: [POC] Put annotation strings into debuginfo.

2021-06-02 Thread Alexei Starovoitov via Phabricator via cfe-commits
ast added a comment.

> Yep LLVM does have some custom attributes and such, eg: 
> https://github.com/llvm/llvm-project/blob/effb87dfa810a28e763f914fe3e6e984782cc846/llvm/include/llvm/BinaryFormat/Dwarf.def#L592

This is great insight! That should work. We need to make sure that 
DW_AT_LLVM_annotation in structure/field/func/argument/variable won't confuse 
gdb and such.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103549

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


[PATCH] D103565: Fix "control reaches end of non-void function" warnings on ppc64le

2021-06-02 Thread Zhaomo Yang via Phabricator via cfe-commits
zhaomo added a comment.

Warnings can be found here: 
https://lab.llvm.org/buildbot/#/builders/76/builds/2640


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103565

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


[PATCH] D103549: [POC] Put annotation strings into debuginfo.

2021-06-02 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

In D103549#2794462 , @yonghong-song 
wrote:

>> As for supporting it in DWARF, probably with a custom attribute 
>> (DW_AT_BTF_annotation? (or "LLVM" instead of "BTF" perhaps, I'm not sure)) 
>> with a standard form (DW_FORM_strp/strxN/etc - the usual way we emit 
>> strings).
>
> This is a good idea. Could you give some pointers where I could add one 
> custom attribute? Does llvm currently have any custom attribute?

Yep LLVM does have some custom attributes and such, eg: 
https://github.com/llvm/llvm-project/blob/effb87dfa810a28e763f914fe3e6e984782cc846/llvm/include/llvm/BinaryFormat/Dwarf.def#L592

Used here: 
https://github.com/llvm/llvm-project/blob/effb87dfa810a28e763f914fe3e6e984782cc846/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp#L1083


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103549

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


[PATCH] D103564: [NFC][compiler-rt][hwasan] Wrap vfork interceptor with HWASAN_WITH_INTERCEPTORS

2021-06-02 Thread Evgenii Stepanov via Phabricator via cfe-commits
eugenis added a comment.

s/vfork/fork/ in the description

I'm afraid removing the fork interceptor will break Android. This is a pretty 
uncommon condition, but still. It is interesting to note that ASan does not 
have this fork protection, see https://github.com/google/sanitizers/issues/774 
for some history.

I wonder if this could be replaced with pthread_atfork. The order of callback 
execution seems right, as long as hwasan is registered first. Or, if you want 
to make progress, add SANITIZER_ANDROID ifdef somewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103564

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


[PATCH] D103565: Fix "control reaches end of non-void function" warnings on ppc64le

2021-06-02 Thread Zhaomo Yang via Phabricator via cfe-commits
zhaomo created this revision.
zhaomo added reviewers: ymandel, hokein.
Herald added a subscriber: shchenz.
zhaomo requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103565

Files:
  clang/lib/ASTMatchers/GtestMatchers.cpp


Index: clang/lib/ASTMatchers/GtestMatchers.cpp
===
--- clang/lib/ASTMatchers/GtestMatchers.cpp
+++ clang/lib/ASTMatchers/GtestMatchers.cpp
@@ -53,6 +53,7 @@
   case GtestCmp::Lt:
 return functionDecl(hasName("::testing::internal::CmpHelperLT"));
   }
+  llvm_unreachable("Unhandled GtestCmp enum");
 }
 
 static llvm::StringRef getMacroTypeName(MacroType Macro) {
@@ -64,6 +65,7 @@
   case MacroType::On:
 return "ON";
   }
+  llvm_unreachable("Unhandled MacroType enum");
 }
 
 static llvm::StringRef getComparisonTypeName(GtestCmp Cmp) {
@@ -81,6 +83,7 @@
   case GtestCmp::Lt:
 return "LT";
   }
+  llvm_unreachable("Unhandled GtestCmp enum");
 }
 
 static std::string getMacroName(MacroType Macro, GtestCmp Cmp) {
@@ -101,9 +104,8 @@
 return "InternalDefaultActionSetAt";
   case MacroType::Expect:
 return "InternalExpectedAt";
-  default:
-llvm_unreachable("Unhandled MacroType enum");
   }
+  llvm_unreachable("Unhandled MacroType enum");
 }
 
 // In general, AST matchers cannot match calls to macros. However, we can
@@ -170,6 +172,7 @@
 hasOverloadedOperatorName("()"), argumentCountIs(3),
 hasArgument(0, ignoringImplicit(MockCall));
   }
+  llvm_unreachable("Unhandled MockArgs enum");
 }
 
 static internal::BindableMatcher


Index: clang/lib/ASTMatchers/GtestMatchers.cpp
===
--- clang/lib/ASTMatchers/GtestMatchers.cpp
+++ clang/lib/ASTMatchers/GtestMatchers.cpp
@@ -53,6 +53,7 @@
   case GtestCmp::Lt:
 return functionDecl(hasName("::testing::internal::CmpHelperLT"));
   }
+  llvm_unreachable("Unhandled GtestCmp enum");
 }
 
 static llvm::StringRef getMacroTypeName(MacroType Macro) {
@@ -64,6 +65,7 @@
   case MacroType::On:
 return "ON";
   }
+  llvm_unreachable("Unhandled MacroType enum");
 }
 
 static llvm::StringRef getComparisonTypeName(GtestCmp Cmp) {
@@ -81,6 +83,7 @@
   case GtestCmp::Lt:
 return "LT";
   }
+  llvm_unreachable("Unhandled GtestCmp enum");
 }
 
 static std::string getMacroName(MacroType Macro, GtestCmp Cmp) {
@@ -101,9 +104,8 @@
 return "InternalDefaultActionSetAt";
   case MacroType::Expect:
 return "InternalExpectedAt";
-  default:
-llvm_unreachable("Unhandled MacroType enum");
   }
+  llvm_unreachable("Unhandled MacroType enum");
 }
 
 // In general, AST matchers cannot match calls to macros. However, we can
@@ -170,6 +172,7 @@
 hasOverloadedOperatorName("()"), argumentCountIs(3),
 hasArgument(0, ignoringImplicit(MockCall));
   }
+  llvm_unreachable("Unhandled MockArgs enum");
 }
 
 static internal::BindableMatcher
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D103184: [AArch64] handle -Wa,-march=

2021-06-02 Thread Nick Desaulniers via Phabricator via cfe-commits
nickdesaulniers added inline comments.



Comment at: clang/test/Driver/aarch64-target-as-march.s:20-25
+/// No unused argument warnings when there are multiple values
+// RUN: %clang --target=aarch64-linux-gnueabi -### -c -Wa,-march=armv8.1-a 
-Wa,-march=armv8.2-a %s 2>&1 | \
+// RUN: FileCheck --check-prefix=MULTIPLE-VALUES %s
+
+// MULTIPLE-VALUES: "-target-feature" "+v8.2a
+// MULTIPLE-VALUES-NOT: "-target-feature" "+v8.1a

perhaps add a test with 8.1 and 8.2 in opposite order that 8.1 is chosen, not 
8.2?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103184

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


[PATCH] D103495: [static initializers] Don't put ordered dynamic initializers of static variables into global_ctors

2021-06-02 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

In D103495#2794870 , @wolfgangp wrote:

> For PS4, we use the .ctors scheme, and so the initialization order was 
> suddenly reversed, which was not noticed for a while until the user had a  
> dependency on the previous initialization. And using init_array or not has 
> currently no effect on the order of emission of global_ctors.

Exactly, that's my suggestion: if fno-use-init-array is set, emit global_ctors 
into the .ctors section in reverse order. That way -fno-use-init-array doesn't 
reverse the order of initialization *inside* a given LLVM module, even if it 
reverses the order in which *modules* are initialized.


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

https://reviews.llvm.org/D103495

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


[PATCH] D103562: [NFC][compiler-rt][hwasan] Refactor hwasan functions

2021-06-02 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan updated this revision to Diff 349404.
leonardchan edited the summary of this revision.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103562

Files:
  compiler-rt/lib/hwasan/hwasan.cpp
  compiler-rt/lib/hwasan/hwasan.h
  compiler-rt/lib/hwasan/hwasan_linux.cpp

Index: compiler-rt/lib/hwasan/hwasan_linux.cpp
===
--- compiler-rt/lib/hwasan/hwasan_linux.cpp
+++ compiler-rt/lib/hwasan/hwasan_linux.cpp
@@ -261,18 +261,6 @@
   return p >= kHighMemStart || (p >= kLowMemStart && p <= kLowMemEnd);
 }
 
-static void HwasanAtExit(void) {
-  if (common_flags()->print_module_map)
-DumpProcessMap();
-  if (flags()->print_stats && (flags()->atexit || hwasan_report_count > 0))
-ReportStats();
-  if (hwasan_report_count > 0) {
-// ReportAtExitStatistics();
-if (common_flags()->exitcode)
-  internal__exit(common_flags()->exitcode);
-  }
-}
-
 void InstallAtExitHandler() {
   atexit(HwasanAtExit);
 }
@@ -357,14 +345,6 @@
   return hwasanThreadList().GetThreadByBufferAddress((uptr)R->Next());
 }
 
-struct AccessInfo {
-  uptr addr;
-  uptr size;
-  bool is_store;
-  bool is_load;
-  bool recover;
-};
-
 static AccessInfo GetAccessInfo(siginfo_t *info, ucontext_t *uc) {
   // Access type is passed in a platform dependent way (see below) and encoded
   // as 0xXY, where X&1 is 1 for store, 0 for load, and X&2 is 1 if the error is
@@ -415,28 +395,6 @@
   return AccessInfo{addr, size, is_store, !is_store, recover};
 }
 
-static void HandleTagMismatch(AccessInfo ai, uptr pc, uptr frame,
-  ucontext_t *uc, uptr *registers_frame = nullptr) {
-  InternalMmapVector stack_buffer(1);
-  BufferedStackTrace *stack = stack_buffer.data();
-  stack->Reset();
-  stack->Unwind(pc, frame, uc, common_flags()->fast_unwind_on_fatal);
-
-  // The second stack frame contains the failure __hwasan_check function, as
-  // we have a stack frame for the registers saved in __hwasan_tag_mismatch that
-  // we wish to ignore. This (currently) only occurs on AArch64, as x64
-  // implementations use SIGTRAP to implement the failure, and thus do not go
-  // through the stack saver.
-  if (registers_frame && stack->trace && stack->size > 0) {
-stack->trace++;
-stack->size--;
-  }
-
-  bool fatal = flags()->halt_on_error || !ai.recover;
-  ReportTagMismatch(stack, ai.addr, ai.size, ai.is_store, fatal,
-registers_frame);
-}
-
 static bool HwasanOnSIGTRAP(int signo, siginfo_t *info, ucontext_t *uc) {
   AccessInfo ai = GetAccessInfo(info, uc);
   if (!ai.is_store && !ai.is_load)
@@ -476,20 +434,7 @@
 // rest of the mismatch handling code (C++).
 void __hwasan_tag_mismatch4(uptr addr, uptr access_info, uptr *registers_frame,
 size_t outsize) {
-  __hwasan::AccessInfo ai;
-  ai.is_store = access_info & 0x10;
-  ai.is_load = !ai.is_store;
-  ai.recover = access_info & 0x20;
-  ai.addr = addr;
-  if ((access_info & 0xf) == 0xf)
-ai.size = outsize;
-  else
-ai.size = 1 << (access_info & 0xf);
-
-  __hwasan::HandleTagMismatch(ai, (uptr)__builtin_return_address(0),
-  (uptr)__builtin_frame_address(0), nullptr,
-  registers_frame);
-  __builtin_unreachable();
+  __hwasan::HwasanTagMismatch(addr, access_info, registers_frame, outsize);
 }
 
 #endif // SANITIZER_FREEBSD || SANITIZER_LINUX || SANITIZER_NETBSD
Index: compiler-rt/lib/hwasan/hwasan.h
===
--- compiler-rt/lib/hwasan/hwasan.h
+++ compiler-rt/lib/hwasan/hwasan.h
@@ -136,6 +136,7 @@
 
 void HwasanTSDInit();
 void HwasanTSDThreadInit();
+void HwasanAtExit();
 
 void HwasanOnDeadlySignal(int signo, void *info, void *context);
 
@@ -145,6 +146,26 @@
 
 void AndroidTestTlsSlot();
 
+// This is a compiler-generated struct that can be shared between hwasan
+// implementations.
+struct AccessInfo {
+  uptr addr;
+  uptr size;
+  bool is_store;
+  bool is_load;
+  bool recover;
+};
+
+// Given access info and frame information, unwind the stack and report the tag
+// mismatch.
+void HandleTagMismatch(AccessInfo ai, uptr pc, uptr frame,
+   void *uc, uptr *registers_frame = nullptr);
+
+// This dispatches to HandleTagMismatch but sets up the AccessInfo, program
+// counter, and frame pointer.
+void HwasanTagMismatch(uptr addr, uptr access_info,
+   uptr *registers_frame, size_t outsize);
+
 }  // namespace __hwasan
 
 #define HWASAN_MALLOC_HOOK(ptr, size)   \
Index: compiler-rt/lib/hwasan/hwasan.cpp
===
--- compiler-rt/lib/hwasan/hwasan.cpp
+++ compiler-rt/lib/hwasan/hwasan.cpp
@@ -177,6 +177,58 @@
 void UpdateMemoryUsage() {}
 #endif
 
+void HwasanAtExit() {
+  if (common_flags()->print_module_map)
+DumpProcessMap();
+  if (

[PATCH] D103564: [NFC][compiler-rt][hwasan] Wrap vfork interceptor with HWASAN_WITH_INTERCEPTORS

2021-06-02 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan created this revision.
leonardchan added reviewers: phosek, mcgrathr, eugenis, vitalybuka.
leonardchan added a project: Sanitizers.
Herald added a subscriber: dberris.
leonardchan requested review of this revision.
Herald added a subscriber: Sanitizers.

This prevents emitting the vfork interceptor when interceptors are disabled in 
a hwasan implementation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103564

Files:
  compiler-rt/lib/hwasan/hwasan_interceptors.cpp


Index: compiler-rt/lib/hwasan/hwasan_interceptors.cpp
===
--- compiler-rt/lib/hwasan/hwasan_interceptors.cpp
+++ compiler-rt/lib/hwasan/hwasan_interceptors.cpp
@@ -303,6 +303,7 @@
 
 #endif // HWASAN_WITH_INTERCEPTORS && __aarch64__
 
+#if HWASAN_WITH_INTERCEPTORS
 static void BeforeFork() {
   StackDepotLockAll();
 }
@@ -318,6 +319,7 @@
   AfterFork();
   return pid;
 }
+#endif  // HWASAN_WITH_INTERCEPTORS
 
 namespace __hwasan {
 
@@ -334,9 +336,9 @@
   static int inited = 0;
   CHECK_EQ(inited, 0);
 
+#if HWASAN_WITH_INTERCEPTORS
   INTERCEPT_FUNCTION(fork);
 
-#if HWASAN_WITH_INTERCEPTORS
 #if defined(__linux__)
   INTERCEPT_FUNCTION(vfork);
 #endif  // __linux__


Index: compiler-rt/lib/hwasan/hwasan_interceptors.cpp
===
--- compiler-rt/lib/hwasan/hwasan_interceptors.cpp
+++ compiler-rt/lib/hwasan/hwasan_interceptors.cpp
@@ -303,6 +303,7 @@
 
 #endif // HWASAN_WITH_INTERCEPTORS && __aarch64__
 
+#if HWASAN_WITH_INTERCEPTORS
 static void BeforeFork() {
   StackDepotLockAll();
 }
@@ -318,6 +319,7 @@
   AfterFork();
   return pid;
 }
+#endif  // HWASAN_WITH_INTERCEPTORS
 
 namespace __hwasan {
 
@@ -334,9 +336,9 @@
   static int inited = 0;
   CHECK_EQ(inited, 0);
 
+#if HWASAN_WITH_INTERCEPTORS
   INTERCEPT_FUNCTION(fork);
 
-#if HWASAN_WITH_INTERCEPTORS
 #if defined(__linux__)
   INTERCEPT_FUNCTION(vfork);
 #endif  // __linux__
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D103563: [HIP] Fix amdgcn builtin for long type

2021-06-02 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl created this revision.
yaxunl added reviewers: tra, rjmccall, arsenm, rampitec, b-sumner.
Herald added subscribers: kerbowa, nhaehnle, jvesely.
yaxunl requested review of this revision.
Herald added a subscriber: wdng.

Currently some amdgcn builtins are defined with long int type,
which causes invalid IR on Windows since long int is 32 bit
on Windows whereas these builtins have 64 bit arguments.

As a comparison, generic clang builtins with 64 bit int arguments
or return use long long int to avoid this issue, since long long int
is 64 bit on Linux and Windows.

This patch uses long long int instead of long int to define 64 bit int
arguments or return for amdgcn builtins.


https://reviews.llvm.org/D103563

Files:
  clang/include/clang/Basic/BuiltinsAMDGPU.def
  clang/test/CodeGenCUDA/builtins-amdgcn.cu

Index: clang/test/CodeGenCUDA/builtins-amdgcn.cu
===
--- clang/test/CodeGenCUDA/builtins-amdgcn.cu
+++ clang/test/CodeGenCUDA/builtins-amdgcn.cu
@@ -1,4 +1,11 @@
-// RUN: %clang_cc1 -triple amdgcn -fcuda-is-device -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx906 \
+// RUN:  -aux-triple x86_64-unknown-linux-gnu -fcuda-is-device -emit-llvm %s \
+// RUN:  -o - | FileCheck %s
+
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -target-cpu gfx906 \
+// RUN:  -aux-triple x86_64-pc-windows-msvc -fcuda-is-device -emit-llvm %s \
+// RUN:  -o - | FileCheck %s
+
 #include "Inputs/cuda.h"
 
 // CHECK-LABEL: @_Z16use_dispatch_ptrPi(
@@ -22,3 +29,32 @@
 __global__ void endpgm() {
   __builtin_amdgcn_endpgm();
 }
+
+// Check the 64 bit argument is correctly passed to the intrinsic without truncation or assertion.
+
+// CHECK-LABEL: @_Z14test_uicmp_i64
+// CHECK:  store i64* %out, i64** %out.addr.ascast
+// CHECK-NEXT:  store i64 %a, i64* %a.addr.ascast
+// CHECK-NEXT:  store i64 %b, i64* %b.addr.ascast
+// CHECK-NEXT:  %[[V0:.*]] = load i64, i64* %a.addr.ascast
+// CHECK-NEXT:  %[[V1:.*]] = load i64, i64* %b.addr.ascast
+// CHECK-NEXT:  %[[V2:.*]] = call i64 @llvm.amdgcn.icmp.i64.i64(i64 %0, i64 %1, i32 35)
+// CHECK-NEXT:  %[[V3:.*]] = load i64*, i64** %out.addr.ascast
+// CHECK-NEXT:  store i64 %[[V2]], i64* %[[V3]]
+// CHECK-NEXT:  ret void
+__global__ void test_uicmp_i64(unsigned long long *out, unsigned long long a, unsigned long long b)
+{
+  *out = __builtin_amdgcn_uicmpl(a, b, 30+5);
+}
+
+// Check the 64 bit return value is correctly returned without truncation or assertion.
+
+// CHECK-LABEL: @_Z14test_s_memtime
+// CHECK: %[[V1:.*]] = call i64 @llvm.amdgcn.s.memtime()
+// CHECK-NEXT: %[[PTR:.*]] = load i64*, i64** %out.addr.ascast
+// CHECK-NEXT:  store i64 %[[V1]], i64* %[[PTR]]
+// CHECK-NEXT:  ret void
+__global__ void test_s_memtime(unsigned long long* out)
+{
+  *out = __builtin_amdgcn_s_memtime();
+}
Index: clang/include/clang/Basic/BuiltinsAMDGPU.def
===
--- clang/include/clang/Basic/BuiltinsAMDGPU.def
+++ clang/include/clang/Basic/BuiltinsAMDGPU.def
@@ -9,6 +9,10 @@
 // This file defines the AMDGPU-specific builtin function database. Users of
 // this file must define the BUILTIN macro to make use of this information.
 //
+// Note: (unsigned) long int type should be avoided in builtin definitions
+// since it has different size on Linux (64 bit) and Windows (32 bit). Use
+// (unsigned) long long int type instead, which is 64 bit on both Linux and
+// Windows.
 //===--===//
 
 // The format of this database matches clang/Basic/Builtins.def.
@@ -44,14 +48,14 @@
 BUILTIN(__builtin_amdgcn_mbcnt_hi, "UiUiUi", "nc")
 BUILTIN(__builtin_amdgcn_mbcnt_lo, "UiUiUi", "nc")
 
-TARGET_BUILTIN(__builtin_amdgcn_s_memtime, "LUi", "n", "s-memtime-inst")
+TARGET_BUILTIN(__builtin_amdgcn_s_memtime, "LLUi", "n", "s-memtime-inst")
 
 //===--===//
 // Instruction builtins.
 //===--===//
 BUILTIN(__builtin_amdgcn_s_getreg, "UiIi", "n")
 BUILTIN(__builtin_amdgcn_s_setreg, "vIiUi", "n")
-BUILTIN(__builtin_amdgcn_s_getpc, "LUi", "n")
+BUILTIN(__builtin_amdgcn_s_getpc, "LLUi", "n")
 BUILTIN(__builtin_amdgcn_s_waitcnt, "vIi", "n")
 BUILTIN(__builtin_amdgcn_s_sendmsg, "vIiUi", "n")
 BUILTIN(__builtin_amdgcn_s_sendmsghalt, "vIiUi", "n")
@@ -111,12 +115,12 @@
 BUILTIN(__builtin_amdgcn_s_sleep, "vIi", "n")
 BUILTIN(__builtin_amdgcn_s_incperflevel, "vIi", "n")
 BUILTIN(__builtin_amdgcn_s_decperflevel, "vIi", "n")
-BUILTIN(__builtin_amdgcn_uicmp, "LUiUiUiIi", "nc")
-BUILTIN(__builtin_amdgcn_uicmpl, "LUiLUiLUiIi", "nc")
-BUILTIN(__builtin_amdgcn_sicmp, "LUiiiIi", "nc")
-BUILTIN(__builtin_amdgcn_sicmpl, "LUiLiLiIi", "nc")
-BUILTIN(__builtin_amdgcn_fcmp, "LUiddIi", "nc")
-BUILTIN(__builtin_amdgcn_fcmpf, "LUiffIi", "nc")
+BUILTIN(__builtin_amdgcn_uicmp, "LLUiUiUi

[PATCH] D103495: [static initializers] Don't put ordered dynamic initializers of static variables into global_ctors

2021-06-02 Thread Wolfgang Pieb via Phabricator via cfe-commits
wolfgangp added a comment.

In D103495#2794329 , @rnk wrote:

> +@rsmith @hans @aeubanks
>
>> specifically when init_array is not used
>
> Can you elaborate on why that is? Here's what I remember, and what I guess is 
> happening. ELF has two initializer schemes: .init_array and .ctors. They are 
> essentially equivalent, they are both arrays of function pointers, but one is 
> called in reverse order and the other is called in forward order. The 
> compiler knows which scheme is in use and it is controlled by 
> -fuse-init-array.

For PS4, we use the .ctors scheme, and so the initialization order was suddenly 
reversed, which was not noticed for a while until the user had a  dependency on 
the previous initialization. And using init_array or not has currently no 
effect on the order of emission of global_ctors.

> The LLVM IR langref says that llvm.global_ctors are called in ascending 
> priority order, and the order between initializers is not defined. That's not 
> very helpful and often doesn't reflect reality. I wonder if we could do two 
> things, perhaps separately:
>
> 1. emit llvm.global_ctors so that they are called in order of appearance in 
> the IR array (is this not already true?)

AFAICT, this is already happening. After sorting by priority, the order remains 
intact at emission. So with -fno-use-init-array0we're going to emit 
global_ctors in reverse order, I suppose.

> 2. define the order of initialization in LangRef
>
> With the first change in place, we can be confident that so long as all 
> includers of the `StaticsClass` in the test emit both initializers in the 
> correct order, no matter which initializers prevail, they will be called in 
> the correct order. Of course, this could break users relying on the existing 
> ordering, nevermind that it is explicitly documented as undefined.
>
> The second change is only a documentation change, but I would like to find a 
> way to justify what LLVM does in GlobalOpt. GlobalOpt can effectively fold a 
> dynamic initializer to constant memory in certain cases. That can only ever 
> have the effect of making an initializer run earlier. As long as no 
> initializers depend on observing uninitialized globals, that should be safe. 
> The guarantee that we want to provide is that, for each initializer, all 
> initializers prior to it in the global_ctors array will have run when it 
> runs. Initializers that appear later may run earlier. Hopefully that covers 
> both the usual way that .init_array sections are linked together and the way 
> globalopt optimizes dynamic initialization.




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

https://reviews.llvm.org/D103495

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


[PATCH] D103544: [compiler-rt][Fuchsia] Disable interceptors while enabling new/delete replacements

2021-06-02 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan updated this revision to Diff 349399.
leonardchan marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103544

Files:
  compiler-rt/CMakeLists.txt
  compiler-rt/lib/hwasan/CMakeLists.txt


Index: compiler-rt/lib/hwasan/CMakeLists.txt
===
--- compiler-rt/lib/hwasan/CMakeLists.txt
+++ compiler-rt/lib/hwasan/CMakeLists.txt
@@ -43,6 +43,11 @@
 set(HWASAN_DEFINITIONS)
 append_list_if(COMPILER_RT_HWASAN_WITH_INTERCEPTORS HWASAN_WITH_INTERCEPTORS=1 
HWASAN_DEFINITIONS)
 
+if(FUCHSIA)
+  # Set this explicitly on Fuchsia, otherwise the default value is set to 
HWASAN_WITH_INTERCEPTORS.
+  list(APPEND HWASAN_DEFINITIONS HWASAN_REPLACE_OPERATORS_NEW_AND_DELETE=1)
+endif()
+
 set(HWASAN_RTL_CFLAGS ${SANITIZER_COMMON_CFLAGS})
 append_rtti_flag(OFF HWASAN_RTL_CFLAGS)
 append_list_if(COMPILER_RT_HAS_FPIC_FLAG -fPIC HWASAN_RTL_CFLAGS)
Index: compiler-rt/CMakeLists.txt
===
--- compiler-rt/CMakeLists.txt
+++ compiler-rt/CMakeLists.txt
@@ -67,8 +67,12 @@
   -D${COMPILER_RT_ASAN_SHADOW_SCALE_DEFINITION})
 endif()
 
-set(COMPILER_RT_HWASAN_WITH_INTERCEPTORS ON CACHE BOOL
-"Enable libc interceptors in HWASan (testing mode)")
+if(FUCHSIA)
+  set(COMPILER_RT_HWASAN_WITH_INTERCEPTORS_DEFAULT OFF)
+else()
+  set(COMPILER_RT_HWASAN_WITH_INTERCEPTORS_DEFAULT ON)
+endif()
+set(COMPILER_RT_HWASAN_WITH_INTERCEPTORS 
${COMPILER_RT_HWASAN_WITH_INTERCEPTORS_DEFAULT} CACHE BOOL "Enable libc 
interceptors in HWASan (testing mode)")
 
 set(COMPILER_RT_BAREMETAL_BUILD OFF CACHE BOOL
   "Build for a bare-metal target.")


Index: compiler-rt/lib/hwasan/CMakeLists.txt
===
--- compiler-rt/lib/hwasan/CMakeLists.txt
+++ compiler-rt/lib/hwasan/CMakeLists.txt
@@ -43,6 +43,11 @@
 set(HWASAN_DEFINITIONS)
 append_list_if(COMPILER_RT_HWASAN_WITH_INTERCEPTORS HWASAN_WITH_INTERCEPTORS=1 HWASAN_DEFINITIONS)
 
+if(FUCHSIA)
+  # Set this explicitly on Fuchsia, otherwise the default value is set to HWASAN_WITH_INTERCEPTORS.
+  list(APPEND HWASAN_DEFINITIONS HWASAN_REPLACE_OPERATORS_NEW_AND_DELETE=1)
+endif()
+
 set(HWASAN_RTL_CFLAGS ${SANITIZER_COMMON_CFLAGS})
 append_rtti_flag(OFF HWASAN_RTL_CFLAGS)
 append_list_if(COMPILER_RT_HAS_FPIC_FLAG -fPIC HWASAN_RTL_CFLAGS)
Index: compiler-rt/CMakeLists.txt
===
--- compiler-rt/CMakeLists.txt
+++ compiler-rt/CMakeLists.txt
@@ -67,8 +67,12 @@
   -D${COMPILER_RT_ASAN_SHADOW_SCALE_DEFINITION})
 endif()
 
-set(COMPILER_RT_HWASAN_WITH_INTERCEPTORS ON CACHE BOOL
-"Enable libc interceptors in HWASan (testing mode)")
+if(FUCHSIA)
+  set(COMPILER_RT_HWASAN_WITH_INTERCEPTORS_DEFAULT OFF)
+else()
+  set(COMPILER_RT_HWASAN_WITH_INTERCEPTORS_DEFAULT ON)
+endif()
+set(COMPILER_RT_HWASAN_WITH_INTERCEPTORS ${COMPILER_RT_HWASAN_WITH_INTERCEPTORS_DEFAULT} CACHE BOOL "Enable libc interceptors in HWASan (testing mode)")
 
 set(COMPILER_RT_BAREMETAL_BUILD OFF CACHE BOOL
   "Build for a bare-metal target.")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D69764: [clang-format] Add East/West Const fixer capability

2021-06-02 Thread David Stone via Phabricator via cfe-commits
davidstone added a comment.

In D69764#2058590 , @rsmith wrote:

> I think that if we are reordering `const`, we should be reordering all 
> //decl-specifier//s -- I'd like to see `int static constexpr unsigned const 
> long inline` reordered to something like `static constexpr inline const 
> unsigned long int` too. Applying this only to `const` seems incomplete to me.

I think it's reasonable for `const` and `volatile` to go together, but I think 
the other specifiers are a different kind of setting because they have 
completely different rules for what they apply to.

  const int * pointer_to_const = nullptr;
  int const * also_pointer_to_const = nullptr;
  int * const const_pointer_to_mutable = nullptr;
  
  constexpr int * constexpr_pointer_to_mutable = nullptr;
  int constexpr * also_constexpr_pointer_to_mutable = nullptr;
  int * constexpr invalid = nullptr;

One of the main reasons people talk about putting `const` on the right is that 
then `const` always applies to the thing immediately to its left (except for 
cases that are weird regardless, like member functions). `constexpr` and other 
specifiers always apply to the top-level thing. Maybe I'm sheltered, but I 
don't see any users asking for the ability to format their code to consistently 
say `int constexpr x;`




Comment at: clang/docs/ClangFormatStyleOptions.rst:1378
 
+**ConstStyle** (``ConstAlignmentStyle``)
+  Different ways to arrange const.

steveire wrote:
> klimek wrote:
> > MyDeveloperDay wrote:
> > > aaron.ballman wrote:
> > > > MyDeveloperDay wrote:
> > > > > klimek wrote:
> > > > > > Personally, I'm somewhat against having 3 different aliases for the 
> > > > > > options. I'd chose one, even though it doesn't make everybody 
> > > > > > happy, and move on. I'm fine with East/West as long as the 
> > > > > > documentation makes it clear what it is.
> > > > > If I have to drop the other options, I think I'd want to go with 
> > > > > East/West const as I feel it has more momentum, just letting people 
> > > > > know before I change the code back (to my original patch ;-) )
> > > > > 
> > > > > https://www.youtube.com/watch?v=gRmI_gsNqcI
> > > > > 
> > > > > {F10954065}
> > > > > 
> > > > @klimek I requested that we do not go with East/West the options and 
> > > > I'm still pretty insistent on it. East/West is a kitschy way to phrase 
> > > > it that I think is somewhat US-centric (where we make a pretty big 
> > > > distinction between the east and west coasts). I do not want to have to 
> > > > mentally map left/right to the less-clear east/west in the config file. 
> > > > Would you be fine if we only had Left/Right instead of East/West? I 
> > > > would be fine with that option, but figured enough people like the cute 
> > > > East/West designation that we might as well support it.
> > > Just for a reference, I'm not from the US and I think east/west still 
> > > translates pretty well. I was happy to support the others. 
> > I'd be fine with only having left/right; my personal feeling is also that 
> > west-const / east-const has kinda become a term of art, though, so I really 
> > don't know which one is "right" :)
> > 
> > Generally, I think this is one of the cases where, given good docs, we're 
> > quickly spending more engineering hours discussing the right solution than 
> > it'll cost aggregated over all future users, under the assumption that 
> > people do not write new configs very often, and the few who will, will 
> > quickly remember.
> > 
> > I'd be fine with only having left/right; my personal feeling is also that 
> > west-const / east-const has kinda become a term of art, though, so I really 
> > don't know which one is "right" :)
> 
> This reminds me of the joke that Americans drive on the "Right" side of the 
> road, and English drive on the "Correct" side. Sort of gives a different 
> meaning to `ConstStyle : Right` and that the alternative is `Wrong` :). Maybe 
> that language ambiguity is why `East`/`West` caught on.
> 
> > people do not write new configs very often
> 
> Agreed. It seems a small number of strong views might influence this enough 
> to make `East`/`West` not be used. What a pity.
I'd also add that the original "joke" was `const west` vs `east const` -- the 
terms put the const where they would apply in the style. This seems to have 
gotten lost in the retelling and now the const is always on the right.

I'd favor `Left` and `Right`. `East` and `West` have "caught on" now, but it's 
caught on among the small group of C++ enthusiasts and still has a sort of 
ingroup joke feel to it. I instinctively know my preferred style is "east" 
because the phrase "east const" feels more natural to me because I've said it 
more. It took me a bit of repetition of thinking "East is right, I put the 
const on the right" to get there.

`East` and `West` also sets a precedent for future options to refer to 
positions as `North` and `So

[PATCH] D103562: [NFC][compiler-rt][hwasan] Refactor hwasan functions

2021-06-02 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan created this revision.
leonardchan added reviewers: mcgrathr, phosek, eugenis, vitalybuka.
leonardchan added a project: Sanitizers.
Herald added a subscriber: dberris.
leonardchan requested review of this revision.
Herald added a subscriber: Sanitizers.

This moves the implementations for `HandleTagMismatch` and 
`__hwasan_tag_mismatch4` from `hwasan_linux.cpp` to `hwasan.cpp` and declares 
them in `hwasan.h`. This way, calls to those functions can be shared with the 
fuchsia implementation without duplicating code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103562

Files:
  compiler-rt/lib/hwasan/hwasan.cpp
  compiler-rt/lib/hwasan/hwasan.h
  compiler-rt/lib/hwasan/hwasan_linux.cpp

Index: compiler-rt/lib/hwasan/hwasan_linux.cpp
===
--- compiler-rt/lib/hwasan/hwasan_linux.cpp
+++ compiler-rt/lib/hwasan/hwasan_linux.cpp
@@ -357,14 +357,6 @@
   return hwasanThreadList().GetThreadByBufferAddress((uptr)R->Next());
 }
 
-struct AccessInfo {
-  uptr addr;
-  uptr size;
-  bool is_store;
-  bool is_load;
-  bool recover;
-};
-
 static AccessInfo GetAccessInfo(siginfo_t *info, ucontext_t *uc) {
   // Access type is passed in a platform dependent way (see below) and encoded
   // as 0xXY, where X&1 is 1 for store, 0 for load, and X&2 is 1 if the error is
@@ -415,28 +407,6 @@
   return AccessInfo{addr, size, is_store, !is_store, recover};
 }
 
-static void HandleTagMismatch(AccessInfo ai, uptr pc, uptr frame,
-  ucontext_t *uc, uptr *registers_frame = nullptr) {
-  InternalMmapVector stack_buffer(1);
-  BufferedStackTrace *stack = stack_buffer.data();
-  stack->Reset();
-  stack->Unwind(pc, frame, uc, common_flags()->fast_unwind_on_fatal);
-
-  // The second stack frame contains the failure __hwasan_check function, as
-  // we have a stack frame for the registers saved in __hwasan_tag_mismatch that
-  // we wish to ignore. This (currently) only occurs on AArch64, as x64
-  // implementations use SIGTRAP to implement the failure, and thus do not go
-  // through the stack saver.
-  if (registers_frame && stack->trace && stack->size > 0) {
-stack->trace++;
-stack->size--;
-  }
-
-  bool fatal = flags()->halt_on_error || !ai.recover;
-  ReportTagMismatch(stack, ai.addr, ai.size, ai.is_store, fatal,
-registers_frame);
-}
-
 static bool HwasanOnSIGTRAP(int signo, siginfo_t *info, ucontext_t *uc) {
   AccessInfo ai = GetAccessInfo(info, uc);
   if (!ai.is_store && !ai.is_load)
@@ -476,20 +446,7 @@
 // rest of the mismatch handling code (C++).
 void __hwasan_tag_mismatch4(uptr addr, uptr access_info, uptr *registers_frame,
 size_t outsize) {
-  __hwasan::AccessInfo ai;
-  ai.is_store = access_info & 0x10;
-  ai.is_load = !ai.is_store;
-  ai.recover = access_info & 0x20;
-  ai.addr = addr;
-  if ((access_info & 0xf) == 0xf)
-ai.size = outsize;
-  else
-ai.size = 1 << (access_info & 0xf);
-
-  __hwasan::HandleTagMismatch(ai, (uptr)__builtin_return_address(0),
-  (uptr)__builtin_frame_address(0), nullptr,
-  registers_frame);
-  __builtin_unreachable();
+  __hwasan::HwasanTagMismatch(addr, access_info, registers_frame, outsize);
 }
 
 #endif // SANITIZER_FREEBSD || SANITIZER_LINUX || SANITIZER_NETBSD
Index: compiler-rt/lib/hwasan/hwasan.h
===
--- compiler-rt/lib/hwasan/hwasan.h
+++ compiler-rt/lib/hwasan/hwasan.h
@@ -145,6 +145,26 @@
 
 void AndroidTestTlsSlot();
 
+// This is a compiler-generated struct that can be shared between hwasan
+// implementations.
+struct AccessInfo {
+  uptr addr;
+  uptr size;
+  bool is_store;
+  bool is_load;
+  bool recover;
+};
+
+// Given access info and frame information, unwind the stack and report the tag
+// mismatch.
+void HandleTagMismatch(AccessInfo ai, uptr pc, uptr frame,
+   void *uc, uptr *registers_frame = nullptr);
+
+// This dispatches to HandleTagMismatch but sets up the AccessInfo, program
+// counter, and frame pointer.
+void HwasanTagMismatch(uptr addr, uptr access_info,
+   uptr *registers_frame, size_t outsize);
+
 }  // namespace __hwasan
 
 #define HWASAN_MALLOC_HOOK(ptr, size)   \
Index: compiler-rt/lib/hwasan/hwasan.cpp
===
--- compiler-rt/lib/hwasan/hwasan.cpp
+++ compiler-rt/lib/hwasan/hwasan.cpp
@@ -177,6 +177,46 @@
 void UpdateMemoryUsage() {}
 #endif
 
+void HandleTagMismatch(AccessInfo ai, uptr pc, uptr frame,
+   void *uc, uptr *registers_frame) {
+  InternalMmapVector stack_buffer(1);
+  BufferedStackTrace *stack = stack_buffer.data();
+  stack->Reset();
+  stack->Unwind(pc, frame, uc, common_flags()->fast_unwind_on_fatal);
+
+  // The second stack frame contains the failure __hwasan_check function, as
+  // we have a stack fra

[PATCH] D103452: [clang] Fix reading long doubles with va_arg on x86_64 mingw

2021-06-02 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4364-4365
 
   // MS x64 ABI requirement: "Any argument that doesn't fit in 8 bytes, or is
   // not 1, 2, 4, or 8 bytes, must be passed by reference."
   if (isAggregateTypeForABI(Ty) || Ty->isMemberPointerType()) {

rnk wrote:
> I wonder if we should make the check more general. There are other cases to 
> consider, like `__int128`. How does GCC pass other large types through 
> varargs? It looks to me like clang passes those indirectly, so we could go 
> ahead and check the type size directly without these aggregate checks.
> 
> Separately, I think `X86_64ABIInfo::EmitMSVAArg` has a bug, it always passes 
> false for indirect, but it should match this code here exactly. Ideally, they 
> would share an implementation.
I'll have a look at what GCC does for `__int128` yeah.

Yes, `X86_64ABIInfo::EmitMSVAArg` is lacking, I've got a separate patch fixing 
that one up to be posted after this one (but this patch fixes an actual issue 
I've run into, the other one is more of a correctness thing).

The open question about that one, is what to do about `long double`. If using 
`__attribute__((ms_abi))` and `__builtin_ms_va_list` on a non-windows platform, 
we don't know if they're meaning to interact with the MS or GNU ABI regarding 
`long double`s.

On one hand, one would mostly be using those constructs to reimplement Windows 
API (i.e. implementing wine or something similar), and in that case, only the 
MS behaviour is of interest. On the other hand, mapping `va_arg(ap, long 
double)` to the GNU case doesn't take anything away for the user either, 
because if you want a MSVC long double, you can just do `va_arg(ap, double)`. 
Then finally, I guess there's no guarantee that the platform where doing that 
even has an exactly matching long double?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103452

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


[PATCH] D102180: [Clang][OpenMP] Emit dependent PreInits before directive.

2021-06-02 Thread Michael Kruse via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG07a6beb40215: [Clang][OpenMP] Emit dependent PreInits before 
directive. (authored by Meinersbur).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102180

Files:
  clang/include/clang/AST/StmtOpenMP.h
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/tile_codegen.cpp
  clang/test/OpenMP/tile_codegen_for_dependent.cpp
  clang/test/OpenMP/tile_codegen_tile_for.cpp

Index: clang/test/OpenMP/tile_codegen_tile_for.cpp
===
--- /dev/null
+++ clang/test/OpenMP/tile_codegen_tile_for.cpp
@@ -0,0 +1,253 @@
+// Check code generation
+// RUN: %clang_cc1 -verify -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=51 -emit-llvm %s -o - | FileCheck %s --check-prefix=IR
+
+// Check same results after serialization round-trip
+// RUN: %clang_cc1 -verify -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=51 -emit-pch -o %t %s
+// RUN: %clang_cc1 -verify -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=51 -include-pch %t -emit-llvm %s -o - | FileCheck %s --check-prefix=IR
+// expected-no-diagnostics
+
+// Account for multiple transformations of a loop before consumed by
+// #pragma omp for.
+
+#ifndef HEADER
+#define HEADER
+
+// placeholder for loop body code.
+extern "C" void body(...) {}
+
+
+// IR-LABEL: @func(
+// IR-NEXT:  [[ENTRY:.*]]:
+// IR-NEXT:%[[START_ADDR:.+]] = alloca i32, align 4
+// IR-NEXT:%[[END_ADDR:.+]] = alloca i32, align 4
+// IR-NEXT:%[[STEP_ADDR:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_IV:.+]] = alloca i32, align 4
+// IR-NEXT:%[[TMP:.+]] = alloca i32, align 4
+// IR-NEXT:%[[I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_1:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_2:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_3:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTFLOOR_0_IV_I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_6:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_8:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_12:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_14:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTFLOOR_0_IV__FLOOR_0_IV_I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_LB:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_UB:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_STRIDE:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_IS_LAST:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTFLOOR_0_IV__FLOOR_0_IV_I18:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTTILE_0_IV__FLOOR_0_IV_I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTTILE_0_IV_I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[TMP0:.+]] = call i32 @__kmpc_global_thread_num(%struct.ident_t* @2)
+// IR-NEXT:store i32 %[[START:.+]], i32* %[[START_ADDR]], align 4
+// IR-NEXT:store i32 %[[END:.+]], i32* %[[END_ADDR]], align 4
+// IR-NEXT:store i32 %[[STEP:.+]], i32* %[[STEP_ADDR]], align 4
+// IR-NEXT:%[[TMP1:.+]] = load i32, i32* %[[START_ADDR]], align 4
+// IR-NEXT:store i32 %[[TMP1]], i32* %[[I]], align 4
+// IR-NEXT:%[[TMP2:.+]] = load i32, i32* %[[START_ADDR]], align 4
+// IR-NEXT:store i32 %[[TMP2]], i32* %[[DOTCAPTURE_EXPR_]], align 4
+// IR-NEXT:%[[TMP3:.+]] = load i32, i32* %[[END_ADDR]], align 4
+// IR-NEXT:store i32 %[[TMP3]], i32* %[[DOTCAPTURE_EXPR_1]], align 4
+// IR-NEXT:%[[TMP4:.+]] = load i32, i32* %[[STEP_ADDR]], align 4
+// IR-NEXT:store i32 %[[TMP4]], i32* %[[DOTCAPTURE_EXPR_2]], align 4
+// IR-NEXT:%[[TMP5:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_1]], align 4
+// IR-NEXT:%[[TMP6:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_]], align 4
+// IR-NEXT:%[[SUB:.+]] = sub i32 %[[TMP5]], %[[TMP6]]
+// IR-NEXT:%[[SUB4:.+]] = sub i32 %[[SUB]], 1
+// IR-NEXT:%[[TMP7:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_2]], align 4
+// IR-NEXT:%[[ADD:.+]] = add i32 %[[SUB4]], %[[TMP7]]
+// IR-NEXT:%[[TMP8:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_2]], align 4
+// IR-NEXT:%[[DIV:.+]] = udiv i32 %[[ADD]], %[[TMP8]]
+// IR-NEXT:%[[SUB5:.+]] = sub i32 %[[DIV]], 1
+// IR-NEXT:store i32 %[[SUB5]], i32* %[[DOTCAPTURE_EXPR_3]], align 4
+// IR-NEXT:store i32 0, i32* %[[DOTFLOOR_0_IV_I]], align 4
+// IR-NEXT:%[[TMP9:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_3]], align 4
+// IR-NEXT:%[[ADD7:.+]] = add i32 %[[TMP9]], 1
+// IR-NEXT:store i32 %[[ADD7]], i32* %[[DOTCAPTURE_EXPR_6]], align 4
+// IR-NEXT:%[[TMP10:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_6]], align 4
+// IR-NEXT:%[[SUB9:.+]] = sub i32 %[[TMP10]], -3
+// IR-NEXT:%[[DIV10:.+]] = udiv i32 %[[SUB9]], 4
+// IR-NEXT:%[[SUB11:.+]] = sub i32 %[[DIV1

[clang] 07a6beb - [Clang][OpenMP] Emit dependent PreInits before directive.

2021-06-02 Thread Michael Kruse via cfe-commits

Author: Michael Kruse
Date: 2021-06-02T16:59:35-05:00
New Revision: 07a6beb402150d25ec7c93a5747520ac2804731d

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

LOG: [Clang][OpenMP] Emit dependent PreInits before directive.

The PreInits of a loop transformation (atm moment only tile) include the 
computation of the trip count. The trip count is needed by any loop-associated 
directives that consumes the transformation-generated loop. Hence, we must 
ensure that the PreInits of consumed loop transformations are emitted with the 
consuming directive.

This is done by addinging the inner loop transformation's PreInits to the outer 
loop-directive's PreInits. The outer loop-directive will consume the de-sugared 
AST such that the inner PreInits are not emitted twice. The PreInits of a loop 
transformation are still emitted directly if its generated loop(s) are not 
associated with another loop-associated directive.

Reviewed By: ABataev

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

Added: 
clang/test/OpenMP/tile_codegen_for_dependent.cpp
clang/test/OpenMP/tile_codegen_tile_for.cpp

Modified: 
clang/include/clang/AST/StmtOpenMP.h
clang/lib/AST/StmtOpenMP.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/tile_codegen.cpp

Removed: 




diff  --git a/clang/include/clang/AST/StmtOpenMP.h 
b/clang/include/clang/AST/StmtOpenMP.h
index 4d14848830fa7..4d6774c1ad280 100644
--- a/clang/include/clang/AST/StmtOpenMP.h
+++ b/clang/include/clang/AST/StmtOpenMP.h
@@ -882,17 +882,45 @@ class OMPLoopBasedDirective : public 
OMPExecutableDirective {
   TryImperfectlyNestedLoops);
   }
 
+  /// Calls the specified callback function for all the loops in \p CurStmt,
+  /// from the outermost to the innermost.
+  static bool doForAllLoops(Stmt *CurStmt, bool TryImperfectlyNestedLoops,
+unsigned NumLoops,
+llvm::function_ref 
Callback,
+llvm::function_ref
+OnTransformationCallback);
+  static bool
+  doForAllLoops(const Stmt *CurStmt, bool TryImperfectlyNestedLoops,
+unsigned NumLoops,
+llvm::function_ref Callback,
+llvm::function_ref
+OnTransformationCallback) {
+auto &&NewCallback = [Callback](unsigned Cnt, Stmt *CurStmt) {
+  return Callback(Cnt, CurStmt);
+};
+auto &&NewTransformCb =
+[OnTransformationCallback](OMPLoopBasedDirective *A) {
+  OnTransformationCallback(A);
+};
+return doForAllLoops(const_cast(CurStmt), 
TryImperfectlyNestedLoops,
+ NumLoops, NewCallback, NewTransformCb);
+  }
+
   /// Calls the specified callback function for all the loops in \p CurStmt,
   /// from the outermost to the innermost.
   static bool
   doForAllLoops(Stmt *CurStmt, bool TryImperfectlyNestedLoops,
 unsigned NumLoops,
-llvm::function_ref Callback);
+llvm::function_ref Callback) {
+auto &&TransformCb = [](OMPLoopBasedDirective *) {};
+return doForAllLoops(CurStmt, TryImperfectlyNestedLoops, NumLoops, 
Callback,
+ TransformCb);
+  }
   static bool
   doForAllLoops(const Stmt *CurStmt, bool TryImperfectlyNestedLoops,
 unsigned NumLoops,
 llvm::function_ref Callback) {
-auto &&NewCallback = [Callback](unsigned Cnt, Stmt *CurStmt) {
+auto &&NewCallback = [Callback](unsigned Cnt, const Stmt *CurStmt) {
   return Callback(Cnt, CurStmt);
 };
 return doForAllLoops(const_cast(CurStmt), 
TryImperfectlyNestedLoops,

diff  --git a/clang/lib/AST/StmtOpenMP.cpp b/clang/lib/AST/StmtOpenMP.cpp
index a9905949d7afa..dbb11e77ac51e 100644
--- a/clang/lib/AST/StmtOpenMP.cpp
+++ b/clang/lib/AST/StmtOpenMP.cpp
@@ -124,11 +124,15 @@ OMPLoopBasedDirective::tryToFindNextInnerLoop(Stmt 
*CurStmt,
 
 bool OMPLoopBasedDirective::doForAllLoops(
 Stmt *CurStmt, bool TryImperfectlyNestedLoops, unsigned NumLoops,
-llvm::function_ref Callback) {
+llvm::function_ref Callback,
+llvm::function_ref
+OnTransformationCallback) {
   CurStmt = CurStmt->IgnoreContainers();
   for (unsigned Cnt = 0; Cnt < NumLoops; ++Cnt) {
-if (auto *Dir = dyn_cast(CurStmt))
+while (auto *Dir = dyn_cast(CurStmt)) {
+  OnTransformationCallback(Dir);
   CurStmt = Dir->getTransformedStmt();
+}
 if (auto *CanonLoop = dyn_cast(CurStmt))
   CurStmt = CanonLoop->getLoopStmt();
 if (Callback(Cnt, CurStmt))

diff  --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp 
b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index 344fa90253ec3..a03cbffc8e8f1 100644
--- a/clang/li

[PATCH] D103555: [Fuchsia] Use libc++abi on Windows in Fuchsia toolchain

2021-06-02 Thread Petr Hosek via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb5dd421a3afa: [Fuchsia] Use libc++abi on Windows in Fuchsia 
toolchain (authored by phosek).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103555

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -83,12 +83,15 @@
   list(APPEND RUNTIME_TARGETS "${target}")
   set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Windows CACHE STRING "")
   set(RUNTIMES_${target}_CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
+  set(RUNTIMES_${target}_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ABI_VERSION 2 CACHE STRING "")
+  set(RUNTIMES_${target}_LIBCXX_CXX_ABI "libcxxabi" CACHE STRING "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_ABI_LINKER_SCRIPT OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
-  set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx" CACHE 
STRING "")
+  set(RUNTIMES_${target}_LIBCXX_NO_VCRUNTIME ON CACHE BOOL "")
+  set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi" 
CACHE STRING "")
 endif()
 
 foreach(target 
aarch64-unknown-linux-gnu;armv7-unknown-linux-gnueabihf;i386-unknown-linux-gnu;x86_64-unknown-linux-gnu)


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -83,12 +83,15 @@
   list(APPEND RUNTIME_TARGETS "${target}")
   set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Windows CACHE STRING "")
   set(RUNTIMES_${target}_CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
+  set(RUNTIMES_${target}_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ABI_VERSION 2 CACHE STRING "")
+  set(RUNTIMES_${target}_LIBCXX_CXX_ABI "libcxxabi" CACHE STRING "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_ABI_LINKER_SCRIPT OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
-  set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx" CACHE STRING "")
+  set(RUNTIMES_${target}_LIBCXX_NO_VCRUNTIME ON CACHE BOOL "")
+  set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi" CACHE STRING "")
 endif()
 
 foreach(target aarch64-unknown-linux-gnu;armv7-unknown-linux-gnueabihf;i386-unknown-linux-gnu;x86_64-unknown-linux-gnu)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] b5dd421 - [Fuchsia] Use libc++abi on Windows in Fuchsia toolchain

2021-06-02 Thread Petr Hosek via cfe-commits

Author: Petr Hosek
Date: 2021-06-02T14:56:27-07:00
New Revision: b5dd421a3afa0290fddf61073274e2a4aa9a

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

LOG: [Fuchsia] Use libc++abi on Windows in Fuchsia toolchain

Don't use vcruntime, this makes our toolchain more hermetic and avoids
some compiler errors we've encountered in compiler-rt.

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

Added: 


Modified: 
clang/cmake/caches/Fuchsia-stage2.cmake

Removed: 




diff  --git a/clang/cmake/caches/Fuchsia-stage2.cmake 
b/clang/cmake/caches/Fuchsia-stage2.cmake
index 7a3853f9e7aa5..982307732327f 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -83,12 +83,15 @@ if(WIN32)
   list(APPEND RUNTIME_TARGETS "${target}")
   set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Windows CACHE STRING "")
   set(RUNTIMES_${target}_CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
+  set(RUNTIMES_${target}_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ABI_VERSION 2 CACHE STRING "")
+  set(RUNTIMES_${target}_LIBCXX_CXX_ABI "libcxxabi" CACHE STRING "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_ABI_LINKER_SCRIPT OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
-  set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx" CACHE 
STRING "")
+  set(RUNTIMES_${target}_LIBCXX_NO_VCRUNTIME ON CACHE BOOL "")
+  set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi" 
CACHE STRING "")
 endif()
 
 foreach(target 
aarch64-unknown-linux-gnu;armv7-unknown-linux-gnueabihf;i386-unknown-linux-gnu;x86_64-unknown-linux-gnu)



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


[PATCH] D103495: [static initializers] Don't put ordered dynamic initializers of static variables into global_ctors

2021-06-02 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment.

In D103495#2794684 , @rnk wrote:

> In D103495#2794667 , @ychen wrote:
>
>> Is there anything preventing us from using the existing priority field to 
>> define the order instead of introducing the order among initializers with 
>> the same priority? If we go with 1., there would be no way to say that "the 
>> order does not matter" right?
>
> The priority is used mainly for inter-object initialization order. It moves 
> the initializer into a `.init_array.N` section, for some number `N`. The 
> programmer may be using existing numbers via 
> `__attribute__((init_priority(N)))`, so it isn't safe for the compiler to use 
> anything other than the default initialization priority.

I see. It makes sense to define the order at IR level then since the order is 
there in `.init_array.N`/`.ctors` section nevertheless.


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

https://reviews.llvm.org/D103495

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


[PATCH] D103184: [AArch64] handle -Wa,-march=

2021-06-02 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked an inline comment as done.
jcai19 added a comment.

In D103184#2793055 , @DavidSpickett 
wrote:

>   $ cat /tmp/test.s
>   irg x0, x0
>   $ aarch64-unknown-linux-gnu-as -march=armv8.5-a+memtag -march=armv8.1-a 
> /tmp/test.s -o /dev/null
>   /tmp/test.s: Assembler messages:
>   /tmp/test.s:1: Error: selected processor does not support `irg x0,x0'
>
> GAS is also taking the last value of `march`. So if we were to follow clang 
> Arm's behaviour, the result would be the same.
>
> Except we don't have to rely on the llvm backend picking the last of 
> "target-feature". Which probably works for architecture versions but for 
> individual extensions I don't think so.
>
> If we make it additive this could happen:
>
>   -march=armv8.5-a+memtag -march=armv8.2-a
>
> Becomes: (mte is the backend name for memtag for silly reasons)
>
>   -target-feature +v8.5-a -target-feature mte -target-feature +v8.2-a
>
> So now instead of getting `v8.2` only, the user now has `v8.2+memtag`. Only 
> if they do `armv8.2-a+nomemtag` will they get what they expect (well, what 
> I'd expect). Which means they'd need to know every previous march value.

Thanks for double checking!  I've changed my implementation since I made that 
comment to take the last value of -Wa,march (and only for assembly files) and 
added more test cases. It should now follows the Arm behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103184

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


[PATCH] D103555: [Fuchsia] Use libc++abi on Windows in Fuchsia toolchain

2021-06-02 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr accepted this revision.
mcgrathr added a comment.

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103555

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


[PATCH] D100308: [clangd] Drop optional on ExternalIndexSpec

2021-06-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9e9ac4138890: [clangd] Drop optional on ExternalIndexSpec 
(authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100308

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/index/ProjectAware.cpp
  clang-tools-extra/clangd/index/ProjectAware.h
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp

Index: clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
@@ -45,8 +45,8 @@
   EXPECT_THAT(match(*Idx, Req), IsEmpty());
 
   Config C;
-  C.Index.External.emplace();
-  C.Index.External->Location = "test";
+  C.Index.External.Kind = Config::ExternalIndexSpec::File;
+  C.Index.External.Location = "test";
   WithContextValue With(Config::Key, std::move(C));
   EXPECT_THAT(match(*Idx, Req), ElementsAre("1"));
   return;
@@ -71,8 +71,8 @@
   EXPECT_EQ(InvocationCount, 0U);
 
   Config C;
-  C.Index.External.emplace();
-  C.Index.External->Location = "test";
+  C.Index.External.Kind = Config::ExternalIndexSpec::File;
+  C.Index.External.Location = "test";
   WithContextValue With(Config::Key, std::move(C));
   match(*Idx, Req);
   // Now it should be created.
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -328,7 +328,7 @@
   ElementsAre(DiagMessage(
   "Remote index may not be specified by untrusted configuration. "
   "Copy this into user config to use it.")));
-  EXPECT_FALSE(Conf.Index.External.hasValue());
+  EXPECT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 }
 
 TEST_F(ConfigCompileTests, ExternalBlockWarnOnMultipleSource) {
@@ -351,11 +351,14 @@
 }
 
 TEST_F(ConfigCompileTests, ExternalBlockDisableWithNone) {
+  compileAndApply();
+  EXPECT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
+
   Fragment::IndexBlock::ExternalBlock External;
   External.IsNone = true;
   Frag.Index.External = std::move(External);
   compileAndApply();
-  EXPECT_FALSE(Conf.Index.External.hasValue());
+  EXPECT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 }
 
 TEST_F(ConfigCompileTests, ExternalBlockErrOnNoSource) {
@@ -406,7 +409,7 @@
   AllOf(DiagMessage("MountPoint must be an absolute path, because this "
 "fragment is not associated with any directory."),
 DiagKind(llvm::SourceMgr::DK_Error;
-  ASSERT_FALSE(Conf.Index.External);
+  EXPECT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 
   auto FooPath = testPath("foo/", llvm::sys::path::Style::posix);
   FooPath = llvm::sys::path::convert_to_slash(FooPath);
@@ -414,22 +417,22 @@
   Frag = GetFrag(testRoot(), "foo/");
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::File);
+  EXPECT_THAT(Conf.Index.External.MountPoint, FooPath);
 
   // None defaults to ".".
   Frag = GetFrag(FooPath, llvm::None);
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::File);
+  EXPECT_THAT(Conf.Index.External.MountPoint, FooPath);
 
   // Without a file, external index is empty.
   Parm.Path = "";
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_FALSE(Conf.Index.External);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 
   // File outside MountPoint, no index.
   auto BazPath = testPath("bar/baz.h", llvm::sys::path::Style::posix);
@@ -438,7 +441,7 @@
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_FALSE(Conf.Index.External);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 
   // File under MountPoint, index should be set.
   BazPath = testPath("foo/baz.h", llvm::sys::path::Style::posix);
@@ -447,8 +450,8 @@
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+  ASSERT_EQ(Conf

[clang-tools-extra] 9e9ac41 - [clangd] Drop optional on ExternalIndexSpec

2021-06-02 Thread Kadir Cetinkaya via cfe-commits

Author: Kadir Cetinkaya
Date: 2021-06-02T23:26:37+02:00
New Revision: 9e9ac4138890425b92d2196344ab59305caa32d7

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

LOG: [clangd] Drop optional on ExternalIndexSpec

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

Added: 


Modified: 
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/index/ProjectAware.cpp
clang-tools-extra/clangd/index/ProjectAware.h
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Config.h 
b/clang-tools-extra/clangd/Config.h
index fe6f4d7fa6e8e..72694788cfbfa 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -70,7 +70,7 @@ struct Config {
   enum class BackgroundPolicy { Build, Skip };
   /// Describes an external index configuration.
   struct ExternalIndexSpec {
-enum { None, File, Server } Kind;
+enum { None, File, Server } Kind = None;
 /// This is one of:
 /// - Address of a clangd-index-server, in the form of "ip:port".
 /// - Absolute path to an index produced by clangd-indexer.
@@ -83,7 +83,7 @@ struct Config {
   struct {
 /// Whether this TU should be indexed.
 BackgroundPolicy Background = BackgroundPolicy::Build;
-llvm::Optional External;
+ExternalIndexSpec External;
   } Index;
 
   /// Controls warnings and errors when parsing code.

diff  --git a/clang-tools-extra/clangd/ConfigCompile.cpp 
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 31beb6ac10cb2..16d66f6154f25 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -376,7 +376,7 @@ struct FragmentCompiler {
 }
 Out.Apply.push_back([Spec(std::move(Spec))](const Params &P, Config &C) {
   if (Spec.Kind == Config::ExternalIndexSpec::None) {
-C.Index.External.reset();
+C.Index.External = Spec;
 return;
   }
   if (P.Path.empty() || !pathStartsWith(Spec.MountPoint, P.Path,

diff  --git a/clang-tools-extra/clangd/index/ProjectAware.cpp 
b/clang-tools-extra/clangd/index/ProjectAware.cpp
index a6cdc86fdc249..3d5430d2b01d8 100644
--- a/clang-tools-extra/clangd/index/ProjectAware.cpp
+++ b/clang-tools-extra/clangd/index/ProjectAware.cpp
@@ -128,9 +128,9 @@ ProjectAwareIndex::indexedFiles() const {
 
 SymbolIndex *ProjectAwareIndex::getIndex() const {
   const auto &C = Config::current();
-  if (!C.Index.External)
+  if (C.Index.External.Kind == Config::ExternalIndexSpec::None)
 return nullptr;
-  const auto &External = *C.Index.External;
+  const auto &External = C.Index.External;
   std::lock_guard Lock(Mu);
   auto Entry = IndexForSpec.try_emplace(External, nullptr);
   if (Entry.second)

diff  --git a/clang-tools-extra/clangd/index/ProjectAware.h 
b/clang-tools-extra/clangd/index/ProjectAware.h
index 888ef3add2ebb..c05f3d307c419 100644
--- a/clang-tools-extra/clangd/index/ProjectAware.h
+++ b/clang-tools-extra/clangd/index/ProjectAware.h
@@ -20,6 +20,7 @@ namespace clangd {
 /// A functor to create an index for an external index specification. Functor
 /// should perform any high latency operation in a separate thread through
 /// AsyncTaskRunner, if set.
+/// Spec is never `None`.
 using IndexFactory = std::function(
 const Config::ExternalIndexSpec &, AsyncTaskRunner *)>;
 

diff  --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp 
b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index f999e0ec2c357..5e7fce8016f12 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -328,7 +328,7 @@ TEST_F(ConfigCompileTests, ExternalServerNeedsTrusted) {
   ElementsAre(DiagMessage(
   "Remote index may not be specified by untrusted configuration. "
   "Copy this into user config to use it.")));
-  EXPECT_FALSE(Conf.Index.External.hasValue());
+  EXPECT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 }
 
 TEST_F(ConfigCompileTests, ExternalBlockWarnOnMultipleSource) {
@@ -351,11 +351,14 @@ TEST_F(ConfigCompileTests, 
ExternalBlockWarnOnMultipleSource) {
 }
 
 TEST_F(ConfigCompileTests, ExternalBlockDisableWithNone) {
+  compileAndApply();
+  EXPECT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
+
   Fragment::IndexBlock::ExternalBlock External;
   External.IsNone = true;
   Frag.Index.External = std::move(External);
   compileAndApply();
-  EXPECT_FALSE(Conf.Index.External.hasValue());
+  EXPECT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 }
 
 TEST_F(ConfigCompileTests, ExternalBlockErrOnNoSource) {
@@ -4

[PATCH] D100308: [clangd] Drop optional on ExternalIndexSpec

2021-06-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 349379.
kadircet added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100308

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/index/ProjectAware.cpp
  clang-tools-extra/clangd/index/ProjectAware.h
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp

Index: clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
@@ -45,8 +45,8 @@
   EXPECT_THAT(match(*Idx, Req), IsEmpty());
 
   Config C;
-  C.Index.External.emplace();
-  C.Index.External->Location = "test";
+  C.Index.External.Kind = Config::ExternalIndexSpec::File;
+  C.Index.External.Location = "test";
   WithContextValue With(Config::Key, std::move(C));
   EXPECT_THAT(match(*Idx, Req), ElementsAre("1"));
   return;
@@ -71,8 +71,8 @@
   EXPECT_EQ(InvocationCount, 0U);
 
   Config C;
-  C.Index.External.emplace();
-  C.Index.External->Location = "test";
+  C.Index.External.Kind = Config::ExternalIndexSpec::File;
+  C.Index.External.Location = "test";
   WithContextValue With(Config::Key, std::move(C));
   match(*Idx, Req);
   // Now it should be created.
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -328,7 +328,7 @@
   ElementsAre(DiagMessage(
   "Remote index may not be specified by untrusted configuration. "
   "Copy this into user config to use it.")));
-  EXPECT_FALSE(Conf.Index.External.hasValue());
+  EXPECT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 }
 
 TEST_F(ConfigCompileTests, ExternalBlockWarnOnMultipleSource) {
@@ -351,11 +351,14 @@
 }
 
 TEST_F(ConfigCompileTests, ExternalBlockDisableWithNone) {
+  compileAndApply();
+  EXPECT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
+
   Fragment::IndexBlock::ExternalBlock External;
   External.IsNone = true;
   Frag.Index.External = std::move(External);
   compileAndApply();
-  EXPECT_FALSE(Conf.Index.External.hasValue());
+  EXPECT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 }
 
 TEST_F(ConfigCompileTests, ExternalBlockErrOnNoSource) {
@@ -406,7 +409,7 @@
   AllOf(DiagMessage("MountPoint must be an absolute path, because this "
 "fragment is not associated with any directory."),
 DiagKind(llvm::SourceMgr::DK_Error;
-  ASSERT_FALSE(Conf.Index.External);
+  EXPECT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 
   auto FooPath = testPath("foo/", llvm::sys::path::Style::posix);
   FooPath = llvm::sys::path::convert_to_slash(FooPath);
@@ -414,22 +417,22 @@
   Frag = GetFrag(testRoot(), "foo/");
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::File);
+  EXPECT_THAT(Conf.Index.External.MountPoint, FooPath);
 
   // None defaults to ".".
   Frag = GetFrag(FooPath, llvm::None);
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::File);
+  EXPECT_THAT(Conf.Index.External.MountPoint, FooPath);
 
   // Without a file, external index is empty.
   Parm.Path = "";
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_FALSE(Conf.Index.External);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 
   // File outside MountPoint, no index.
   auto BazPath = testPath("bar/baz.h", llvm::sys::path::Style::posix);
@@ -438,7 +441,7 @@
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_FALSE(Conf.Index.External);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 
   // File under MountPoint, index should be set.
   BazPath = testPath("foo/baz.h", llvm::sys::path::Style::posix);
@@ -447,8 +450,8 @@
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::File);
+  EXPECT_THAT(Conf.Index.External.MountPoint, FooPath);
 
   // Only matches case-insensitive

[PATCH] D100308: [clangd] Drop optional on ExternalIndexSpec

2021-06-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:555
+  case Config::ExternalIndexSpec::None:
+break;
   case Config::ExternalIndexSpec::Server:

sammccall wrote:
> I think you hit llvm_unreachable here - is this an invariant enforced by the 
> caller, or did you mean to return nullptr?
it is enforced by the caller (i.e. `ProjectAwareIndex::getIndex`) hence 
fallback to unreachable was deliberate.

It was more explicit before as `Generator` takes a reference, also semantically 
it feels wrong for generator to be called with a `None` spec.

Even though we are lucky and can indicate the error by returning null in this 
case, I'd rather keep it as an assertion failure, similar to before. Gonna 
update the documentation on the `IndexFactory` to mention that spec is never 
None though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100308

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


[PATCH] D100308: [clangd] Drop optional on ExternalIndexSpec

2021-06-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 349378.
kadircet marked an inline comment as done.
kadircet added a comment.

Add comment to IndexFactory about pre-condition of spec never being none.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100308

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/index/ProjectAware.cpp
  clang-tools-extra/clangd/index/ProjectAware.h
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp

Index: clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
@@ -45,8 +45,8 @@
   EXPECT_THAT(match(*Idx, Req), IsEmpty());
 
   Config C;
-  C.Index.External.emplace();
-  C.Index.External->Location = "test";
+  C.Index.External.Kind = Config::ExternalIndexSpec::File;
+  C.Index.External.Location = "test";
   WithContextValue With(Config::Key, std::move(C));
   EXPECT_THAT(match(*Idx, Req), ElementsAre("1"));
   return;
@@ -71,8 +71,8 @@
   EXPECT_EQ(InvocationCount, 0U);
 
   Config C;
-  C.Index.External.emplace();
-  C.Index.External->Location = "test";
+  C.Index.External.Kind = Config::ExternalIndexSpec::File;
+  C.Index.External.Location = "test";
   WithContextValue With(Config::Key, std::move(C));
   match(*Idx, Req);
   // Now it should be created.
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -351,11 +351,14 @@
 }
 
 TEST_F(ConfigCompileTests, ExternalBlockDisableWithNone) {
+  compileAndApply();
+  EXPECT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
+
   Fragment::IndexBlock::ExternalBlock External;
   External.IsNone = true;
   Frag.Index.External = std::move(External);
   compileAndApply();
-  EXPECT_FALSE(Conf.Index.External.hasValue());
+  EXPECT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 }
 
 TEST_F(ConfigCompileTests, ExternalBlockErrOnNoSource) {
@@ -406,7 +409,7 @@
   AllOf(DiagMessage("MountPoint must be an absolute path, because this "
 "fragment is not associated with any directory."),
 DiagKind(llvm::SourceMgr::DK_Error;
-  ASSERT_FALSE(Conf.Index.External);
+  EXPECT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 
   auto FooPath = testPath("foo/", llvm::sys::path::Style::posix);
   FooPath = llvm::sys::path::convert_to_slash(FooPath);
@@ -414,22 +417,22 @@
   Frag = GetFrag(testRoot(), "foo/");
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::File);
+  EXPECT_THAT(Conf.Index.External.MountPoint, FooPath);
 
   // None defaults to ".".
   Frag = GetFrag(FooPath, llvm::None);
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::File);
+  EXPECT_THAT(Conf.Index.External.MountPoint, FooPath);
 
   // Without a file, external index is empty.
   Parm.Path = "";
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_FALSE(Conf.Index.External);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 
   // File outside MountPoint, no index.
   auto BazPath = testPath("bar/baz.h", llvm::sys::path::Style::posix);
@@ -438,7 +441,7 @@
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_FALSE(Conf.Index.External);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::None);
 
   // File under MountPoint, index should be set.
   BazPath = testPath("foo/baz.h", llvm::sys::path::Style::posix);
@@ -447,8 +450,8 @@
   Frag = GetFrag("", FooPath.c_str());
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
+  ASSERT_EQ(Conf.Index.External.Kind, Config::ExternalIndexSpec::File);
+  EXPECT_THAT(Conf.Index.External.MountPoint, FooPath);
 
   // Only matches case-insensitively.
   BazPath = testPath("fOo/baz.h", llvm::sys::path::Style::posix);
@@ -461,10 +464,10 @@
   compileAndApply();
   ASSERT_THAT(Diags.Diagnostics, IsEmpty());
 #ifdef CLANGD_PATH_CASE_INSENSITIVE
-  ASSERT_TRUE(Conf.Index.External);
-  EXPECT_THAT(Conf.Index.Exte

[PATCH] D103495: [static initializers] Don't put ordered dynamic initializers of static variables into global_ctors

2021-06-02 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

In D103495#2794667 , @ychen wrote:

> Is there anything preventing us from using the existing priority field to 
> define the order instead of introducing the order among initializers with the 
> same priority? If we go with 1., there would be no way to say that "the order 
> does not matter" right?

The priority is used mainly for inter-object initialization order. It moves the 
initializer into a `.init_array.N` section, for some number `N`. The programmer 
may be using existing numbers via `__attribute__((init_priority(N)))`, so it 
isn't safe for the compiler to use anything other than the default 
initialization priority.


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

https://reviews.llvm.org/D103495

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


[PATCH] D103555: [Fuchsia] Use libc++abi on Windows in Fuchsia toolchain

2021-06-02 Thread Petr Hosek via Phabricator via cfe-commits
phosek created this revision.
phosek added reviewers: mcgrathr, leonardchan, haowei, gulfem.
Herald added a subscriber: mgorny.
phosek requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Don't use vcruntime, this makes our toolchain more hermetic and avoids
some compiler errors we've encountered in compiler-rt.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103555

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -83,12 +83,15 @@
   list(APPEND RUNTIME_TARGETS "${target}")
   set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Windows CACHE STRING "")
   set(RUNTIMES_${target}_CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
+  set(RUNTIMES_${target}_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ABI_VERSION 2 CACHE STRING "")
+  set(RUNTIMES_${target}_LIBCXX_CXX_ABI "libcxxabi" CACHE STRING "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_ABI_LINKER_SCRIPT OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
-  set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx" CACHE 
STRING "")
+  set(RUNTIMES_${target}_LIBCXX_NO_VCRUNTIME ON CACHE BOOL "")
+  set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi" 
CACHE STRING "")
 endif()
 
 foreach(target 
aarch64-unknown-linux-gnu;armv7-unknown-linux-gnueabihf;i386-unknown-linux-gnu;x86_64-unknown-linux-gnu)


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -83,12 +83,15 @@
   list(APPEND RUNTIME_TARGETS "${target}")
   set(RUNTIMES_${target}_CMAKE_SYSTEM_NAME Windows CACHE STRING "")
   set(RUNTIMES_${target}_CMAKE_BUILD_TYPE RelWithDebInfo CACHE STRING "")
+  set(RUNTIMES_${target}_LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ABI_VERSION 2 CACHE STRING "")
+  set(RUNTIMES_${target}_LIBCXX_CXX_ABI "libcxxabi" CACHE STRING "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_FILESYSTEM OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_ABI_LINKER_SCRIPT OFF CACHE BOOL "")
   set(RUNTIMES_${target}_LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
-  set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx" CACHE STRING "")
+  set(RUNTIMES_${target}_LIBCXX_NO_VCRUNTIME ON CACHE BOOL "")
+  set(RUNTIMES_${target}_LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi" CACHE STRING "")
 endif()
 
 foreach(target aarch64-unknown-linux-gnu;armv7-unknown-linux-gnueabihf;i386-unknown-linux-gnu;x86_64-unknown-linux-gnu)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D103495: [static initializers] Don't put ordered dynamic initializers of static variables into global_ctors

2021-06-02 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment.

> The LLVM IR langref says that llvm.global_ctors are called in ascending 
> priority order, and the order between initializers is not defined. That's not 
> very helpful and often doesn't reflect reality. I wonder if we could do two 
> things, perhaps separately:
>
> 1. emit llvm.global_ctors so that they are called in order of appearance in 
> the IR array (is this not already true?)
> 2. define the order of initialization in LangRef

Is there anything preventing us from using the existing priority field to 
define the order instead of introducing the order among initializers with the 
same priority? If we go with 1., there would be no way to say that "the order 
does not matter" right?


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

https://reviews.llvm.org/D103495

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


[PATCH] D103544: [compiler-rt][Fuchsia] Disable interceptors while enabling new/delete replacements

2021-06-02 Thread Petr Hosek via Phabricator via cfe-commits
phosek added inline comments.



Comment at: compiler-rt/lib/hwasan/CMakeLists.txt:45
+if (NOT FUCHSIA)
+  append_list_if(COMPILER_RT_HWASAN_WITH_INTERCEPTORS 
HWASAN_WITH_INTERCEPTORS=1 HWASAN_DEFINITIONS)
+else()

mcgrathr wrote:
> It might be better to force the value of COMPILER_RT_HWASAN_WITH_INTERCEPTORS 
> to OFF instead.
> 
> But I'll leave the cmake details to Petr.
> 
That would be my preference. The patter we usually use is:
```
if(FUCHSIA)
  set(COMPILER_RT_HWASAN_WITH_INTERCEPTORS_DEFAULT OFF)
else()
  set(COMPILER_RT_HWASAN_WITH_INTERCEPTORS_DEFAULT ON)
endif()
set(COMPILER_RT_HWASAN_WITH_INTERCEPTORS 
${COMPILER_RT_HWASAN_WITH_INTERCEPTORS_DEFAULT} CACHE BOOL "Enable libc 
interceptors in HWASan (testing mode)")
```
which would be set in 
https://github.com/llvm/llvm-project/blob/07c2a912ddf1641b969fdbae3418f77c362f67c6/compiler-rt/CMakeLists.txt#L70.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103544

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


[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits
feg208 added a comment.

one remaining


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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


[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 349374.
feg208 marked an inline comment as done.
feg208 added a comment.

Missed a request


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16401,6 +16401,229 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructures) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = true;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"}, // first line\n"
+   "{-1, 93463, \"world\"}, // second line\n"
+   "{ 7, 5,\"!!\"}  // third line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[4] = {\n"
+   "{ 56,23, 21,   \"oh\"}, // first line\n"
+   "{ -1, 93463, 22,   \"my\"}, // second line\n"
+   "{  7, 5,  1, \"goodness\"}  // third line\n"
+   "{234, 5,  1, \"gracious\"}  // fourth line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[3] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[3] = {\n"
+   "{int{56},23, \"hello\"},\n"
+   "{int{-1}, 93463, \"world\"},\n"
+   "{ int{7}, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+  verifyFormat("demo = std::array{\n"
+   "test{56,23, \"hello\"},\n"
+   "test{-1, 93463, \"world\"},\n"
+   "test{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "#if X\n"
+   "{-1, 93463, \"world\"},\n"
+   "#endif\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat(
+  "test demo[] = {\n"
+  "{ 7,23,\n"
+  " \"hello world i am a very long line that really, in any\"\n"
+  " \"just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  \"world\"},\n"
+  "{56, 5, \"!!\"}\n"
+  "};\n",
+  Style);
+  verifyFormat("return GradForUnaryCwise(g, {\n"
+   "{{\"sign\"}, \"Sign\",  "
+   "  {\"x\", \"dy\"}},\n"
+   "{  {\"dx\"},  \"Mul\", {\"dy\""
+   ", \"sign\"}},\n"
+   "});\n",
+   Style);
+
+  Style.ColumnLimit = 0;
+  EXPECT_EQ(
+  "test demo[] = {\n"
+  "{56,23, \"hello world i am a very long line that really, "
+  "in any just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  "
+  " \"world\"},\n"
+  "{ 7, 5,  "
+  "\"!!\"},\n"
+  "};",
+  format("test demo[] = {{56, 23, \"hello world i am a very long line "
+ "that really, in any just world, ought to be s

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits
feg208 updated this revision to Diff 349373.
feg208 marked 2 inline comments as done.
feg208 added a comment.

Captured review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16401,6 +16401,229 @@
getLLVMStyle());
 }
 
+TEST_F(FormatTest, CatchAlignArrayOfStructures) {
+  auto Style = getLLVMStyle();
+  Style.AlignArrayOfStructures = true;
+  Style.AlignConsecutiveAssignments =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  Style.AlignConsecutiveDeclarations =
+  FormatStyle::AlignConsecutiveStyle::ACS_Consecutive;
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"}, // first line\n"
+   "{-1, 93463, \"world\"}, // second line\n"
+   "{ 7, 5,\"!!\"}  // third line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[4] = {\n"
+   "{ 56,23, 21,   \"oh\"}, // first line\n"
+   "{ -1, 93463, 22,   \"my\"}, // second line\n"
+   "{  7, 5,  1, \"goodness\"}  // third line\n"
+   "{234, 5,  1, \"gracious\"}  // fourth line\n"
+   "};\n",
+   Style);
+
+  verifyFormat("struct test demo[3] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[3] = {\n"
+   "{int{56},23, \"hello\"},\n"
+   "{int{-1}, 93463, \"world\"},\n"
+   "{ int{7}, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+  verifyFormat("struct test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "{-1, 93463, \"world\"},\n"
+   "{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+  verifyFormat("demo = std::array{\n"
+   "test{56,23, \"hello\"},\n"
+   "test{-1, 93463, \"world\"},\n"
+   "test{ 7, 5,\"!!\"},\n"
+   "};\n",
+   Style);
+  verifyFormat("test demo[] = {\n"
+   "{56,23, \"hello\"},\n"
+   "#if X\n"
+   "{-1, 93463, \"world\"},\n"
+   "#endif\n"
+   "{ 7, 5,\"!!\"}\n"
+   "};\n",
+   Style);
+
+  verifyFormat(
+  "test demo[] = {\n"
+  "{ 7,23,\n"
+  " \"hello world i am a very long line that really, in any\"\n"
+  " \"just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  \"world\"},\n"
+  "{56, 5, \"!!\"}\n"
+  "};\n",
+  Style);
+  verifyFormat("return GradForUnaryCwise(g, {\n"
+   "{{\"sign\"}, \"Sign\",  "
+   "  {\"x\", \"dy\"}},\n"
+   "{  {\"dx\"},  \"Mul\", {\"dy\""
+   ", \"sign\"}},\n"
+   "});\n",
+   Style);
+
+  Style.ColumnLimit = 0;
+  EXPECT_EQ(
+  "test demo[] = {\n"
+  "{56,23, \"hello world i am a very long line that really, "
+  "in any just world, ought to be split over multiple lines\"},\n"
+  "{-1, 93463,  "
+  " \"world\"},\n"
+  "{ 7, 5,  "
+  "\"!!\"},\n"
+  "};",
+  format("test demo[] = {{56, 23, \"hello world i am a very long line "
+ "that really, in any just world, ought

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-02 Thread Fred Grim via Phabricator via cfe-commits
feg208 marked 6 inline comments as done.
feg208 added a comment.

I rolled up the suggested changes.




Comment at: clang/lib/Format/ContinuationIndenter.cpp:603
   State.Column + Spaces + PPColumnCorrection);
-
   // If "BreakBeforeInheritanceComma" mode, don't break within the inheritance

HazardyKnusperkeks wrote:
> Please don't remove the empty lines.
I am sorry. I was peeling back previous changes and must have overdone it



Comment at: clang/lib/Format/TokenAnnotator.cpp:2657-2659
+if (CurrentToken->is(tok::l_brace)) {
+  ++Depth;
+} else if (CurrentToken->is(tok::r_brace))

HazardyKnusperkeks wrote:
> There are still braces.
blech I thought I got those. Sorry.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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


[PATCH] D103449: [clangd][Protocol] Drop optional from WorkspaceEdit::changes

2021-06-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdc10bf1a4ed0: [clangd][Protocol] Drop optional from 
WorkspaceEdit::changes (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103449

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h


Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -908,7 +908,7 @@
 
 struct WorkspaceEdit {
   /// Holds changes to existing resources.
-  llvm::Optional>> changes;
+  std::map> changes;
 
   /// Note: "documentChanges" is not currently used because currently there is
   /// no support for versioned edits.
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -808,10 +808,8 @@
 }
 
 llvm::json::Value toJSON(const WorkspaceEdit &WE) {
-  if (!WE.changes)
-return llvm::json::Object{};
   llvm::json::Object FileChanges;
-  for (auto &Change : *WE.changes)
+  for (auto &Change : WE.changes)
 FileChanges[Change.first] = llvm::json::Array(Change.second);
   return llvm::json::Object{{"changes", std::move(FileChanges)}};
 }
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -372,8 +372,7 @@
   Action.title = F.Message;
   Action.kind = std::string(CodeAction::QUICKFIX_KIND);
   Action.edit.emplace();
-  Action.edit->changes.emplace();
-  (*Action.edit->changes)[File.uri()] = {F.Edits.begin(), F.Edits.end()};
+  Action.edit->changes[File.uri()] = {F.Edits.begin(), F.Edits.end()};
   return Action;
 }
 
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -748,9 +748,8 @@
   return Reply(std::move(Err));
 
 WorkspaceEdit WE;
-WE.changes.emplace();
 for (const auto &It : R->ApplyEdits) {
-  (*WE.changes)[URI::createFile(It.first()).toString()] =
+  WE.changes[URI::createFile(It.first()).toString()] =
   It.second.asTextEdits();
 }
 // ApplyEdit will take care of calling Reply().
@@ -813,22 +812,20 @@
   if (!Server->getDraft(File))
 return Reply(llvm::make_error(
 "onRename called for non-added file", ErrorCode::InvalidParams));
-  Server->rename(
-  File, Params.position, Params.newName, Opts.Rename,
-  [File, Params, Reply = std::move(Reply),
-   this](llvm::Expected R) mutable {
-if (!R)
-  return Reply(R.takeError());
-if (auto Err = validateEdits(*Server, R->GlobalChanges))
-  return Reply(std::move(Err));
-WorkspaceEdit Result;
-Result.changes.emplace();
-for (const auto &Rep : R->GlobalChanges) {
-  (*Result.changes)[URI::createFile(Rep.first()).toString()] =
-  Rep.second.asTextEdits();
-}
-Reply(Result);
-  });
+  Server->rename(File, Params.position, Params.newName, Opts.Rename,
+ [File, Params, Reply = std::move(Reply),
+  this](llvm::Expected R) mutable {
+   if (!R)
+ return Reply(R.takeError());
+   if (auto Err = validateEdits(*Server, R->GlobalChanges))
+ return Reply(std::move(Err));
+   WorkspaceEdit Result;
+   for (const auto &Rep : R->GlobalChanges) {
+ Result.changes[URI::createFile(Rep.first()).toString()] =
+ Rep.second.asTextEdits();
+   }
+   Reply(Result);
+ });
 }
 
 void ClangdLSPServer::onDocumentDidClose(


Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -908,7 +908,7 @@
 
 struct WorkspaceEdit {
   /// Holds changes to existing resources.
-  llvm::Optional>> changes;
+  std::map> changes;
 
   /// Note: "documentChanges" is not currently used because currently there is
   /// no support for versioned edits.
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -808,10 +808,8 @@
 }
 
 llvm::json::Value toJSON(const WorkspaceEdit &WE) {
-  if (!WE.cha

[clang-tools-extra] dc10bf1 - [clangd][Protocol] Drop optional from WorkspaceEdit::changes

2021-06-02 Thread Kadir Cetinkaya via cfe-commits

Author: Kadir Cetinkaya
Date: 2021-06-02T22:59:18+02:00
New Revision: dc10bf1a4ed0b34b27284b5260ce5c6cc132bd6f

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

LOG: [clangd][Protocol] Drop optional from WorkspaceEdit::changes

This is causing weird code patterns in various places and I can't see
any difference between None and empty change list. Neither in the current use
cases nor in the spec.

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

Added: 


Modified: 
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/Protocol.cpp
clang-tools-extra/clangd/Protocol.h

Removed: 




diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp 
b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index c25195cd338f..f70fd0018cfd 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -748,9 +748,8 @@ void ClangdLSPServer::onCommandApplyTweak(const TweakArgs 
&Args,
   return Reply(std::move(Err));
 
 WorkspaceEdit WE;
-WE.changes.emplace();
 for (const auto &It : R->ApplyEdits) {
-  (*WE.changes)[URI::createFile(It.first()).toString()] =
+  WE.changes[URI::createFile(It.first()).toString()] =
   It.second.asTextEdits();
 }
 // ApplyEdit will take care of calling Reply().
@@ -813,22 +812,20 @@ void ClangdLSPServer::onRename(const RenameParams &Params,
   if (!Server->getDraft(File))
 return Reply(llvm::make_error(
 "onRename called for non-added file", ErrorCode::InvalidParams));
-  Server->rename(
-  File, Params.position, Params.newName, Opts.Rename,
-  [File, Params, Reply = std::move(Reply),
-   this](llvm::Expected R) mutable {
-if (!R)
-  return Reply(R.takeError());
-if (auto Err = validateEdits(*Server, R->GlobalChanges))
-  return Reply(std::move(Err));
-WorkspaceEdit Result;
-Result.changes.emplace();
-for (const auto &Rep : R->GlobalChanges) {
-  (*Result.changes)[URI::createFile(Rep.first()).toString()] =
-  Rep.second.asTextEdits();
-}
-Reply(Result);
-  });
+  Server->rename(File, Params.position, Params.newName, Opts.Rename,
+ [File, Params, Reply = std::move(Reply),
+  this](llvm::Expected R) mutable {
+   if (!R)
+ return Reply(R.takeError());
+   if (auto Err = validateEdits(*Server, R->GlobalChanges))
+ return Reply(std::move(Err));
+   WorkspaceEdit Result;
+   for (const auto &Rep : R->GlobalChanges) {
+ Result.changes[URI::createFile(Rep.first()).toString()] =
+ Rep.second.asTextEdits();
+   }
+   Reply(Result);
+ });
 }
 
 void ClangdLSPServer::onDocumentDidClose(

diff  --git a/clang-tools-extra/clangd/Diagnostics.cpp 
b/clang-tools-extra/clangd/Diagnostics.cpp
index 453b5a644d09..88ec7aaa9b26 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -372,8 +372,7 @@ CodeAction toCodeAction(const Fix &F, const URIForFile 
&File) {
   Action.title = F.Message;
   Action.kind = std::string(CodeAction::QUICKFIX_KIND);
   Action.edit.emplace();
-  Action.edit->changes.emplace();
-  (*Action.edit->changes)[File.uri()] = {F.Edits.begin(), F.Edits.end()};
+  Action.edit->changes[File.uri()] = {F.Edits.begin(), F.Edits.end()};
   return Action;
 }
 

diff  --git a/clang-tools-extra/clangd/Protocol.cpp 
b/clang-tools-extra/clangd/Protocol.cpp
index 06dbdaebb9a4..7c7f41795608 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -808,10 +808,8 @@ llvm::json::Value toJSON(const DocumentSymbol &S) {
 }
 
 llvm::json::Value toJSON(const WorkspaceEdit &WE) {
-  if (!WE.changes)
-return llvm::json::Object{};
   llvm::json::Object FileChanges;
-  for (auto &Change : *WE.changes)
+  for (auto &Change : WE.changes)
 FileChanges[Change.first] = llvm::json::Array(Change.second);
   return llvm::json::Object{{"changes", std::move(FileChanges)}};
 }

diff  --git a/clang-tools-extra/clangd/Protocol.h 
b/clang-tools-extra/clangd/Protocol.h
index 3f6ce566ef2f..4831ea6de750 100644
--- a/clang-tools-extra/clangd/Protocol.h
+++ b/clang-tools-extra/clangd/Protocol.h
@@ -908,7 +908,7 @@ bool fromJSON(const llvm::json::Value &, CodeActionParams 
&, llvm::json::Path);
 
 struct WorkspaceEdit {
   /// Holds changes to existing resources.
-  llvm::Optional>> changes;
+  std::map> changes;
 
   /// Note: "documentChanges" is not currently used because currently there is
   /// no support for v

[PATCH] D103449: [clangd][Protocol] Drop optional from WorkspaceEdit::changes

2021-06-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 349371.
kadircet marked an inline comment as done.
kadircet added a comment.

Get rid of the special case around empty changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103449

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h


Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -908,7 +908,7 @@
 
 struct WorkspaceEdit {
   /// Holds changes to existing resources.
-  llvm::Optional>> changes;
+  std::map> changes;
 
   /// Note: "documentChanges" is not currently used because currently there is
   /// no support for versioned edits.
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -808,10 +808,8 @@
 }
 
 llvm::json::Value toJSON(const WorkspaceEdit &WE) {
-  if (!WE.changes)
-return llvm::json::Object{};
   llvm::json::Object FileChanges;
-  for (auto &Change : *WE.changes)
+  for (auto &Change : WE.changes)
 FileChanges[Change.first] = llvm::json::Array(Change.second);
   return llvm::json::Object{{"changes", std::move(FileChanges)}};
 }
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -372,8 +372,7 @@
   Action.title = F.Message;
   Action.kind = std::string(CodeAction::QUICKFIX_KIND);
   Action.edit.emplace();
-  Action.edit->changes.emplace();
-  (*Action.edit->changes)[File.uri()] = {F.Edits.begin(), F.Edits.end()};
+  Action.edit->changes[File.uri()] = {F.Edits.begin(), F.Edits.end()};
   return Action;
 }
 
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -748,9 +748,8 @@
   return Reply(std::move(Err));
 
 WorkspaceEdit WE;
-WE.changes.emplace();
 for (const auto &It : R->ApplyEdits) {
-  (*WE.changes)[URI::createFile(It.first()).toString()] =
+  WE.changes[URI::createFile(It.first()).toString()] =
   It.second.asTextEdits();
 }
 // ApplyEdit will take care of calling Reply().
@@ -813,22 +812,20 @@
   if (!Server->getDraft(File))
 return Reply(llvm::make_error(
 "onRename called for non-added file", ErrorCode::InvalidParams));
-  Server->rename(
-  File, Params.position, Params.newName, Opts.Rename,
-  [File, Params, Reply = std::move(Reply),
-   this](llvm::Expected R) mutable {
-if (!R)
-  return Reply(R.takeError());
-if (auto Err = validateEdits(*Server, R->GlobalChanges))
-  return Reply(std::move(Err));
-WorkspaceEdit Result;
-Result.changes.emplace();
-for (const auto &Rep : R->GlobalChanges) {
-  (*Result.changes)[URI::createFile(Rep.first()).toString()] =
-  Rep.second.asTextEdits();
-}
-Reply(Result);
-  });
+  Server->rename(File, Params.position, Params.newName, Opts.Rename,
+ [File, Params, Reply = std::move(Reply),
+  this](llvm::Expected R) mutable {
+   if (!R)
+ return Reply(R.takeError());
+   if (auto Err = validateEdits(*Server, R->GlobalChanges))
+ return Reply(std::move(Err));
+   WorkspaceEdit Result;
+   for (const auto &Rep : R->GlobalChanges) {
+ Result.changes[URI::createFile(Rep.first()).toString()] =
+ Rep.second.asTextEdits();
+   }
+   Reply(Result);
+ });
 }
 
 void ClangdLSPServer::onDocumentDidClose(


Index: clang-tools-extra/clangd/Protocol.h
===
--- clang-tools-extra/clangd/Protocol.h
+++ clang-tools-extra/clangd/Protocol.h
@@ -908,7 +908,7 @@
 
 struct WorkspaceEdit {
   /// Holds changes to existing resources.
-  llvm::Optional>> changes;
+  std::map> changes;
 
   /// Note: "documentChanges" is not currently used because currently there is
   /// no support for versioned edits.
Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -808,10 +808,8 @@
 }
 
 llvm::json::Value toJSON(const WorkspaceEdit &WE) {
-  if (!WE.changes)
-return llvm::json::Object{};
   llvm::json::Object FileChanges;
-  for

[PATCH] D103476: [clangd] TUScheduler uses last active file for file-less queries

2021-06-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6c2a4e28f4d1: [clangd] TUScheduler uses last active file for 
file-less queries (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103476

Files:
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -1264,6 +1264,69 @@
   << "association still valid";
 }
 
+TEST_F(TUSchedulerTests, PreservesLastActiveFile) {
+  for (bool Sync : {false, true}) {
+auto Opts = optsForTest();
+if (Sync)
+  Opts.AsyncThreadsCount = 0;
+TUScheduler S(CDB, Opts);
+
+auto CheckNoFileActionsSeesLastActiveFile =
+[&](llvm::StringRef LastActiveFile) {
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+  std::atomic Counter(0);
+  // We only check for run and runQuick as runWithAST and
+  // runWithPreamble is always bound to a file.
+  S.run("run-UsesLastActiveFile", /*Path=*/"", [&] {
+++Counter;
+EXPECT_EQ(LastActiveFile, boundPath());
+  });
+  S.runQuick("runQuick-UsesLastActiveFile", /*Path=*/"", [&] {
+++Counter;
+EXPECT_EQ(LastActiveFile, boundPath());
+  });
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+  EXPECT_EQ(2, Counter.load());
+};
+
+// Check that we see no file initially
+CheckNoFileActionsSeesLastActiveFile("");
+
+// Now check that every action scheduled with a particular file changes the
+// LastActiveFile.
+auto Path = testPath("run.cc");
+S.run(Path, Path, [] {});
+CheckNoFileActionsSeesLastActiveFile(Path);
+
+Path = testPath("runQuick.cc");
+S.runQuick(Path, Path, [] {});
+CheckNoFileActionsSeesLastActiveFile(Path);
+
+Path = testPath("runWithAST.cc");
+S.update(Path, getInputs(Path, ""), WantDiagnostics::No);
+S.runWithAST(Path, Path, [](llvm::Expected Inp) {
+  EXPECT_TRUE(bool(Inp));
+});
+CheckNoFileActionsSeesLastActiveFile(Path);
+
+Path = testPath("runWithPreamble.cc");
+S.update(Path, getInputs(Path, ""), WantDiagnostics::No);
+S.runWithPreamble(
+Path, Path, TUScheduler::Stale,
+[](llvm::Expected Inp) { EXPECT_TRUE(bool(Inp)); });
+CheckNoFileActionsSeesLastActiveFile(Path);
+
+Path = testPath("update.cc");
+S.update(Path, getInputs(Path, ""), WantDiagnostics::No);
+CheckNoFileActionsSeesLastActiveFile(Path);
+
+// An update with the same contents should not change LastActiveFile.
+auto LastActive = Path;
+Path = testPath("runWithAST.cc");
+S.update(Path, getInputs(Path, ""), WantDiagnostics::No);
+CheckNoFileActionsSeesLastActiveFile(LastActive);
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -23,6 +23,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -341,6 +342,9 @@
   // asynchronously.
   llvm::Optional PreambleTasks;
   llvm::Optional WorkerThreads;
+  // Used to create contexts for operations that are not bound to a particular
+  // file (e.g. index queries).
+  std::string LastActiveFile;
 };
 
 } // namespace clangd
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -1506,6 +1506,10 @@
 FD->Contents = Inputs.Contents;
   }
   FD->Worker->update(std::move(Inputs), WantDiags, ContentChanged);
+  // There might be synthetic update requests, don't change the LastActiveFile
+  // in such cases.
+  if (ContentChanged)
+LastActiveFile = File.str();
   return NewFile;
 }
 
@@ -1535,6 +1539,10 @@
 void TUScheduler::runWithSemaphore(llvm::StringRef Name, llvm::StringRef Path,
llvm::unique_function Action,
Semaphore &Sem) {
+  if (Path.empty())
+Path = LastActiveFile;
+  else
+LastActiveFile = Path.str();
   if (!PreambleTasks) {
 WithContext WithProvidedContext(Opts.ContextProvider(Path));
 return Action();
@@ -1559,6 +1567,7 @@
 "trying to get AST for non-added document", ErrorCode::InvalidParams));
 return;
   }
+  LastActiveFile = File.str();

[PATCH] D103452: [clang] Fix reading long doubles with va_arg on x86_64 mingw

2021-06-02 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4364-4365
 
   // MS x64 ABI requirement: "Any argument that doesn't fit in 8 bytes, or is
   // not 1, 2, 4, or 8 bytes, must be passed by reference."
   if (isAggregateTypeForABI(Ty) || Ty->isMemberPointerType()) {

I wonder if we should make the check more general. There are other cases to 
consider, like `__int128`. How does GCC pass other large types through varargs? 
It looks to me like clang passes those indirectly, so we could go ahead and 
check the type size directly without these aggregate checks.

Separately, I think `X86_64ABIInfo::EmitMSVAArg` has a bug, it always passes 
false for indirect, but it should match this code here exactly. Ideally, they 
would share an implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103452

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


[clang-tools-extra] 6c2a4e2 - [clangd] TUScheduler uses last active file for file-less queries

2021-06-02 Thread Kadir Cetinkaya via cfe-commits

Author: Kadir Cetinkaya
Date: 2021-06-02T22:50:24+02:00
New Revision: 6c2a4e28f4d1c0f525c53302c08808c1b4f8073b

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

LOG: [clangd] TUScheduler uses last active file for file-less queries

This enables requests like workspaceSymbols to be dispatched using the
file user was most recently operating on. A replacement for D103179.

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

Added: 


Modified: 
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/TUScheduler.cpp 
b/clang-tools-extra/clangd/TUScheduler.cpp
index 7498598f124fb..09c68a3a250ba 100644
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -1506,6 +1506,10 @@ bool TUScheduler::update(PathRef File, ParseInputs 
Inputs,
 FD->Contents = Inputs.Contents;
   }
   FD->Worker->update(std::move(Inputs), WantDiags, ContentChanged);
+  // There might be synthetic update requests, don't change the LastActiveFile
+  // in such cases.
+  if (ContentChanged)
+LastActiveFile = File.str();
   return NewFile;
 }
 
@@ -1535,6 +1539,10 @@ void TUScheduler::runQuick(llvm::StringRef Name, 
llvm::StringRef Path,
 void TUScheduler::runWithSemaphore(llvm::StringRef Name, llvm::StringRef Path,
llvm::unique_function Action,
Semaphore &Sem) {
+  if (Path.empty())
+Path = LastActiveFile;
+  else
+LastActiveFile = Path.str();
   if (!PreambleTasks) {
 WithContext WithProvidedContext(Opts.ContextProvider(Path));
 return Action();
@@ -1559,6 +1567,7 @@ void TUScheduler::runWithAST(
 "trying to get AST for non-added document", ErrorCode::InvalidParams));
 return;
   }
+  LastActiveFile = File.str();
 
   It->second->Worker->runWithAST(Name, std::move(Action), Invalidation);
 }
@@ -1573,6 +1582,7 @@ void TUScheduler::runWithPreamble(llvm::StringRef Name, 
PathRef File,
 ErrorCode::InvalidParams));
 return;
   }
+  LastActiveFile = File.str();
 
   if (!PreambleTasks) {
 trace::Span Tracer(Name);

diff  --git a/clang-tools-extra/clangd/TUScheduler.h 
b/clang-tools-extra/clangd/TUScheduler.h
index 4731443d3d51b..02b602173ae67 100644
--- a/clang-tools-extra/clangd/TUScheduler.h
+++ b/clang-tools-extra/clangd/TUScheduler.h
@@ -23,6 +23,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -341,6 +342,9 @@ class TUScheduler {
   // asynchronously.
   llvm::Optional PreambleTasks;
   llvm::Optional WorkerThreads;
+  // Used to create contexts for operations that are not bound to a particular
+  // file (e.g. index queries).
+  std::string LastActiveFile;
 };
 
 } // namespace clangd

diff  --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp 
b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
index d68cb3efa3d6d..e15725d5120ba 100644
--- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -1264,6 +1264,69 @@ TEST_F(TUSchedulerTests, IncluderCache) {
   << "association still valid";
 }
 
+TEST_F(TUSchedulerTests, PreservesLastActiveFile) {
+  for (bool Sync : {false, true}) {
+auto Opts = optsForTest();
+if (Sync)
+  Opts.AsyncThreadsCount = 0;
+TUScheduler S(CDB, Opts);
+
+auto CheckNoFileActionsSeesLastActiveFile =
+[&](llvm::StringRef LastActiveFile) {
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+  std::atomic Counter(0);
+  // We only check for run and runQuick as runWithAST and
+  // runWithPreamble is always bound to a file.
+  S.run("run-UsesLastActiveFile", /*Path=*/"", [&] {
+++Counter;
+EXPECT_EQ(LastActiveFile, boundPath());
+  });
+  S.runQuick("runQuick-UsesLastActiveFile", /*Path=*/"", [&] {
+++Counter;
+EXPECT_EQ(LastActiveFile, boundPath());
+  });
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+  EXPECT_EQ(2, Counter.load());
+};
+
+// Check that we see no file initially
+CheckNoFileActionsSeesLastActiveFile("");
+
+// Now check that every action scheduled with a particular file changes the
+// LastActiveFile.
+auto Path = testPath("run.cc");
+S.run(Path, Path, [] {});
+CheckNoFileActionsSeesLastActiveFile(Path);
+
+Path = testPath("runQuick.cc");
+S.runQuick(Path, Path, [] {});
+CheckNoFileActionsSeesLastActiveFile(Path);
+
+Path = testPath("runWithAST.cc");
+S.update(Path, getInputs(

[PATCH] D103527: [Clang][RISCV] Implement vlseg and vlsegff.

2021-06-02 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: clang/include/clang/Basic/riscv_vector.td:807
+  // intrinsic: (ptr, vl)
+  SmallVector Operands = {Ops[NF], Ops[NF + 1]};
+  llvm::Function *F = CGM.getIntrinsic(ID, IntrinsicTypes);

This can be a `llvm::Value Operands[] =` I think?



Comment at: clang/include/clang/Basic/riscv_vector.td:877
+  // intrinsic: (ptr, vl)
+  SmallVector Operands = {Ops[NF], Ops[NF + 2]};
+  Value *NewVL = Ops[NF + 1];

This can be a `llvm::Value Operands[] =` I think?



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:441
   BuiltinStr = "q" + utostr(Scale.getValue()) + BuiltinStr;
+  // Point to vector types. Defined for Zvlsseg load intrinsics.
+  // Zvlsseg load intrinsics have pointer type arguments to store the loaded

Point->Pointer?



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1120
+  }
+  if (HasMaskedOffOperand && NF > 1)
+// Convert

Put curly braces around these if and else bodies since they contain a comment. 
The compiler doesn't need them but its more readable for humans and would be 
consistent with the standards documented here 
https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103527

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


[PATCH] D103179: [clangd] Handle queries without an originating file in ProjectAwareIndex

2021-06-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet abandoned this revision.
kadircet added a comment.

in favor of D103476 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103179

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


[PATCH] D103476: [clangd] TUScheduler uses last active file for file-less queries

2021-06-02 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 349366.
kadircet marked an inline comment as done.
kadircet added a comment.

Keep using `Path` in `runWithSemaphore`, by substituting `LastActiveFile` when 
empty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103476

Files:
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -1264,6 +1264,69 @@
   << "association still valid";
 }
 
+TEST_F(TUSchedulerTests, PreservesLastActiveFile) {
+  for (bool Sync : {false, true}) {
+auto Opts = optsForTest();
+if (Sync)
+  Opts.AsyncThreadsCount = 0;
+TUScheduler S(CDB, Opts);
+
+auto CheckNoFileActionsSeesLastActiveFile =
+[&](llvm::StringRef LastActiveFile) {
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+  std::atomic Counter(0);
+  // We only check for run and runQuick as runWithAST and
+  // runWithPreamble is always bound to a file.
+  S.run("run-UsesLastActiveFile", /*Path=*/"", [&] {
+++Counter;
+EXPECT_EQ(LastActiveFile, boundPath());
+  });
+  S.runQuick("runQuick-UsesLastActiveFile", /*Path=*/"", [&] {
+++Counter;
+EXPECT_EQ(LastActiveFile, boundPath());
+  });
+  ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
+  EXPECT_EQ(2, Counter.load());
+};
+
+// Check that we see no file initially
+CheckNoFileActionsSeesLastActiveFile("");
+
+// Now check that every action scheduled with a particular file changes the
+// LastActiveFile.
+auto Path = testPath("run.cc");
+S.run(Path, Path, [] {});
+CheckNoFileActionsSeesLastActiveFile(Path);
+
+Path = testPath("runQuick.cc");
+S.runQuick(Path, Path, [] {});
+CheckNoFileActionsSeesLastActiveFile(Path);
+
+Path = testPath("runWithAST.cc");
+S.update(Path, getInputs(Path, ""), WantDiagnostics::No);
+S.runWithAST(Path, Path, [](llvm::Expected Inp) {
+  EXPECT_TRUE(bool(Inp));
+});
+CheckNoFileActionsSeesLastActiveFile(Path);
+
+Path = testPath("runWithPreamble.cc");
+S.update(Path, getInputs(Path, ""), WantDiagnostics::No);
+S.runWithPreamble(
+Path, Path, TUScheduler::Stale,
+[](llvm::Expected Inp) { EXPECT_TRUE(bool(Inp)); });
+CheckNoFileActionsSeesLastActiveFile(Path);
+
+Path = testPath("update.cc");
+S.update(Path, getInputs(Path, ""), WantDiagnostics::No);
+CheckNoFileActionsSeesLastActiveFile(Path);
+
+// An update with the same contents should not change LastActiveFile.
+auto LastActive = Path;
+Path = testPath("runWithAST.cc");
+S.update(Path, getInputs(Path, ""), WantDiagnostics::No);
+CheckNoFileActionsSeesLastActiveFile(LastActive);
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/TUScheduler.h
===
--- clang-tools-extra/clangd/TUScheduler.h
+++ clang-tools-extra/clangd/TUScheduler.h
@@ -23,6 +23,7 @@
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -341,6 +342,9 @@
   // asynchronously.
   llvm::Optional PreambleTasks;
   llvm::Optional WorkerThreads;
+  // Used to create contexts for operations that are not bound to a particular
+  // file (e.g. index queries).
+  std::string LastActiveFile;
 };
 
 } // namespace clangd
Index: clang-tools-extra/clangd/TUScheduler.cpp
===
--- clang-tools-extra/clangd/TUScheduler.cpp
+++ clang-tools-extra/clangd/TUScheduler.cpp
@@ -1506,6 +1506,10 @@
 FD->Contents = Inputs.Contents;
   }
   FD->Worker->update(std::move(Inputs), WantDiags, ContentChanged);
+  // There might be synthetic update requests, don't change the LastActiveFile
+  // in such cases.
+  if (ContentChanged)
+LastActiveFile = File.str();
   return NewFile;
 }
 
@@ -1535,6 +1539,10 @@
 void TUScheduler::runWithSemaphore(llvm::StringRef Name, llvm::StringRef Path,
llvm::unique_function Action,
Semaphore &Sem) {
+  if (Path.empty())
+Path = LastActiveFile;
+  else
+LastActiveFile = Path.str();
   if (!PreambleTasks) {
 WithContext WithProvidedContext(Opts.ContextProvider(Path));
 return Action();
@@ -1559,6 +1567,7 @@
 "trying to get AST for non-added document", ErrorCode::InvalidParams));
 return;
   }
+  LastActiveFile = File.str();
 
   It->second->Worker->runWithAST(Name, std::

[PATCH] D102822: [Clang][CodeGen] Set the size of llvm.lifetime to unknown for scalable types.

2021-06-02 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added inline comments.



Comment at: clang/test/CodeGen/RISCV/riscv-v-lifetime.cpp:3
+// REQUIRES: riscv-registered-target
+// RUN: %clang_cc1 -std=c++11 -triple riscv64 -target-feature +experimental-v \
+// RUN:   -emit-llvm -O1 -o - %s | FileCheck %s

Probably best to add -disable-llvm-passes so we don't run the optimizer.



Comment at: clang/test/CodeGen/RISCV/riscv-v-lifetime.cpp:8
+
+template 
+void Foo(T &&);

Are the templates needed? This was enough to fail for me

```
vint32m1_t Baz();   





vint32m1_t Test() { 


  const vint32m1_t &a = Baz();  


  return a; 


}
```

Or do the templates test additional code paths?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102822

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


[PATCH] D103538: [clangd] Run code completion on each token coverd by --check-lines

2021-06-02 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.

Thanks, LGTM.

---

A little bit thinking out loud, was there a particular reason to introduce 
`--check-line` into ClangdMain.cpp rather than Check.cpp?

It feels like we should have --check related flags in Check.cpp instead. That 
way both the signature of `check` stays more reasonable and ClangdMain won't 
accumulate `flag to internal value` logic.




Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:66
+   const ThreadsafeFS &TFS, const ClangdLSPServer::Options &Opts,
+   const bool EnableCodeCompletion);
 

nit: it is not common to have const on params that are copied (especially when 
they are just builtins)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103538

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


[PATCH] D103547: Don't delete the module you're inspecting

2021-06-02 Thread Petr Hosek via Phabricator via cfe-commits
phosek accepted this revision.
phosek added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103547

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


[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-02 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks requested changes to this revision.
HazardyKnusperkeks added a comment.
This revision now requires changes to proceed.

I've added a few comments, and I would like to hear the opinion of others 
regarding the left or right alignment of the elements.




Comment at: clang/lib/Format/ContinuationIndenter.cpp:603
   State.Column + Spaces + PPColumnCorrection);
-
   // If "BreakBeforeInheritanceComma" mode, don't break within the inheritance

Please don't remove the empty lines.



Comment at: clang/lib/Format/TokenAnnotator.cpp:733
+  bool couldBeInStructArrayInitializer() const {
+const auto end = Contexts.rbegin() + 2;
+auto last = Contexts.rbegin();

Capital letter for variables.



Comment at: clang/lib/Format/TokenAnnotator.cpp:733
+  bool couldBeInStructArrayInitializer() const {
+const auto end = Contexts.rbegin() + 2;
+auto last = Contexts.rbegin();

HazardyKnusperkeks wrote:
> Capital letter for variables.
Where does the 2 come from? How do we know that + 2 is always applicable? There 
should be an explanation, and an assert.

On another note, I would prefer `std::next()`.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2657-2659
+if (CurrentToken->is(tok::l_brace)) {
+  ++Depth;
+} else if (CurrentToken->is(tok::r_brace))

There are still braces.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2663
+  CurrentToken = CurrentToken->Next;
+  if (CurrentToken != nullptr) {
+CurrentToken->StartsColumn = true;

I think if you revert the condition and drop the `else` (because of the 
`break`) makes it more readable.



Comment at: clang/lib/Format/TokenAnnotator.h:35
+  LT_VirtualFunctionDecl,
+  LT_ArrayOfStructInitializer
 };

Add a trailing comma, so that future additions do not need to change this line.



Comment at: clang/lib/Format/WhitespaceManager.h:254
+auto NetWidth = InitialSpaces;
+for (auto PrevIter = Start; PrevIter != End; PrevIter++) {
+  // If we broke the line the initial spaces are already

For iterators prefix increment is really better than postfix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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


[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

2021-06-02 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

In D98799#2794546 , @ahatanak wrote:

> Yes, I think I can fix that.

Thanks a lot! If you could follow-up here when that's committed, I can 
follow-up with a fix/improvement for the debug info mangling issue we've been 
discussing here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98799

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


[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

2021-06-02 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

Yes, I think I can fix that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98799

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


[clang] d8e0ae9 - [SYCL] Fix __builtin_sycl_unique_stable_name to work on windows/spir

2021-06-02 Thread Erich Keane via cfe-commits

Author: Erich Keane
Date: 2021-06-02T13:16:14-07:00
New Revision: d8e0ae9a76a62bdc6117630d59bf9967ac9bb4ea

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

LOG: [SYCL] Fix __builtin_sycl_unique_stable_name to work on windows/spir

In the case where the device is an itanium target, and the host is a
windows target, we were getting the names wrong, since in the itanium
case we filter by lambda-signature.

The fix is to always filter by the signature rather than just on
non-windows builds. I considered doing the reverse (that is, checking
the aux-triple), but doing so would result in duplicate lambda mangling
numbers (from linux reusing the same number for different signatures).

Added: 
clang/test/CodeGenSYCL/unique_stable_name_windows_diff.cpp

Modified: 
clang/lib/AST/ASTContext.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index e96f52920521..76f84c728bc6 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -11750,12 +11750,7 @@ unsigned ASTContext::GetSYCLKernelNamingIndex(const 
NamedDecl *ND) {
 
   llvm::SmallVector Decls{Set.begin(), Set.end()};
 
-  // If we are in an itanium situation, the mangling-numbers for a lambda 
depend
-  // on the mangled signature, so sort by that. Only TargetCXXABI::Microsoft
-  // doesn't use the itanium mangler, and just sets the lambda mangling number
-  // incrementally, with no consideration to the signature.
-  if (Target->getCXXABI().getKind() != TargetCXXABI::Microsoft)
-FilterSYCLKernelNamingDecls(RD, Decls);
+  FilterSYCLKernelNamingDecls(RD, Decls);
 
   llvm::sort(Decls, [](const CXXRecordDecl *LHS, const CXXRecordDecl *RHS) {
 return LHS->getLambdaManglingNumber() < RHS->getLambdaManglingNumber();

diff  --git a/clang/test/CodeGenSYCL/unique_stable_name_windows_
diff .cpp b/clang/test/CodeGenSYCL/unique_stable_name_windows_
diff .cpp
new file mode 100644
index ..ecdc1d50abbe
--- /dev/null
+++ b/clang/test/CodeGenSYCL/unique_stable_name_windows_
diff .cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple spir64-unknown-unknown-sycldevice -aux-triple 
x86_64-pc-windows-msvc -fsycl-is-device -disable-llvm-passes -fsycl-is-device 
-emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fsycl-is-device 
-disable-llvm-passes -fsycl-is-device -emit-llvm %s -o - | FileCheck %s
+
+
+template
+__attribute__((sycl_kernel)) void kernel(Func F){
+  F();
+}
+
+template
+__attribute__((sycl_kernel)) void kernel2(Func F){
+  F(1);
+}
+
+template
+__attribute__((sycl_kernel)) void kernel3(Func F){
+  F(1.1);
+}
+
+int main() {
+  int i;
+  double d;
+  float f;
+  auto lambda1 = [](){};
+  auto lambda2 = [](int){};
+  auto lambda3 = [](double){};
+
+  kernel(lambda1);
+  kernel2(lambda2);
+  kernel3(lambda3);
+
+  // Ensure the kernels are named the same between the device and host
+  // invocations.
+  (void)__builtin_sycl_unique_stable_name(decltype(lambda1));
+  (void)__builtin_sycl_unique_stable_name(decltype(lambda2));
+  (void)__builtin_sycl_unique_stable_name(decltype(lambda3));
+
+  // Make sure the following 3 are the same between the host and device 
compile.
+  // Note that these are NOT the same value as eachother, they 
diff er by the
+  // signature.
+  // CHECK: private unnamed_addr constant [22 x i8] c"_ZTSZ4mainEUlvE1_\00"
+  // CHECK: private unnamed_addr constant [22 x i8] c"_ZTSZ4mainEUliE1_\00"
+  // CHECK: private unnamed_addr constant [22 x i8] c"_ZTSZ4mainEUldE1_\00"
+}



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


[PATCH] D103547: Don't delete the module you're inspecting

2021-06-02 Thread Steven Wu via Phabricator via cfe-commits
steven_wu accepted this revision.
steven_wu 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/D103547/new/

https://reviews.llvm.org/D103547

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


[clang] 13659f4 - PR50337, PR50561: Fix determination of whether a template parameter list

2021-06-02 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2021-06-02T13:06:40-07:00
New Revision: 13659f48a1d7814fe893d2383acdc0b0313740a9

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

LOG: PR50337, PR50561: Fix determination of whether a template parameter list
contains constrained parameters.

Added: 


Modified: 
clang/lib/AST/DeclTemplate.cpp
clang/test/SemaTemplate/concepts.cpp

Removed: 




diff  --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index e5b1833a02f2..ec8b00a9eb7d 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -74,7 +74,8 @@ TemplateParameterList::TemplateParameterList(const 
ASTContext& C,
 ->containsUnexpandedParameterPack())
   ContainsUnexpandedParameterPack = true;
   }
-  HasConstrainedParameters = TTP->hasTypeConstraint();
+  if (TTP->hasTypeConstraint())
+HasConstrainedParameters = true;
 } else {
   llvm_unreachable("unexpcted template parameter type");
 }

diff  --git a/clang/test/SemaTemplate/concepts.cpp 
b/clang/test/SemaTemplate/concepts.cpp
index 60c8bc59d33c..c297a75dd82e 100644
--- a/clang/test/SemaTemplate/concepts.cpp
+++ b/clang/test/SemaTemplate/concepts.cpp
@@ -154,3 +154,18 @@ namespace NoConstantFolding {
   template  concept C = &n + 3 - 3 == &n; // expected-error 
{{non-constant expression}} expected-note {{cannot refer to element 3 of 
non-array object}}
   static_assert(C); // expected-note {{while checking}}
 }
+
+namespace PR50337 {
+  template  concept foo = true;
+  template  concept foo2 = foo && true;
+  void f(foo auto, auto);
+  void f(foo2 auto, auto);
+  void g() { f(1, 2); }
+}
+
+namespace PR50561 {
+  template concept C = false;
+  template void f(T, U);
+  template void f(T, U) = delete;
+  void g() { f(0, 0); }
+}



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


[PATCH] D103549: [POC] Put annotation strings into debuginfo.

2021-06-02 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song added a comment.

> As for supporting it in DWARF, probably with a custom attribute 
> (DW_AT_BTF_annotation? (or "LLVM" instead of "BTF" perhaps, I'm not sure)) 
> with a standard form (DW_FORM_strp/strxN/etc - the usual way we emit strings).

This is a good idea. Could you give some pointers where I could add one custom 
attribute? Does llvm currently have any custom attribute?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103549

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


[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

2021-06-02 Thread Gabor Marton via Phabricator via cfe-commits
martong updated this revision to Diff 349352.
martong added a comment.

I am terribly sorry, but I uploaded an unfinished Diff previously, please 
disregard that. So these are the changes:

- Add isEqualTo and simplify members to EquivalenceClass


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103314

Files:
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/find-binop-constraints.cpp

Index: clang/test/Analysis/find-binop-constraints.cpp
===
--- /dev/null
+++ clang/test/Analysis/find-binop-constraints.cpp
@@ -0,0 +1,145 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -verify
+
+void clang_analyzer_eval(bool);
+void clang_analyzer_warnIfReached();
+
+int test_legacy_behavior(int x, int y) {
+  if (y != 0)
+return 0;
+  if (x + y != 0)
+return 0;
+  clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+  return y / (x + y);  // expected-warning{{Division by zero}}
+}
+
+int test_rhs_further_constrained(int x, int y) {
+  if (x + y != 0)
+return 0;
+  if (y != 0)
+return 0;
+  clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_lhs_further_constrained(int x, int y) {
+  if (x + y != 0)
+return 0;
+  if (x != 0)
+return 0;
+  clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_lhs_and_rhs_further_constrained(int x, int y) {
+  if (x % y != 1)
+return 0;
+  if (x != 1)
+return 0;
+  if (y != 2)
+return 0;
+  clang_analyzer_eval(x % y == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 2); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_commutativity(int x, int y) {
+  if (x + y != 0)
+return 0;
+  if (y != 0)
+return 0;
+  clang_analyzer_eval(y + x == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_binop_when_height_is_2_r(int a, int x, int y, int z) {
+  switch (a) {
+  case 1: {
+if (x + y + z != 0)
+  return 0;
+if (z != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(z == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 2: {
+if (x + y + z != 0)
+  return 0;
+if (y != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 3: {
+if (x + y + z != 0)
+  return 0;
+if (x != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 4: {
+if (x + y + z != 0)
+  return 0;
+if (x + y != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 5: {
+if (z != 0)
+  return 0;
+if (x + y + z != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+if (y != 0)
+  return 0;
+clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+break;
+  }
+
+  }
+  return 0;
+}
+
+void test_equivalence_classes_are_updated(int a, int b, int c, int d) {
+  if (a + b != c)
+return;
+  if (a != d)
+return;
+  if (b != 0)
+return;
+  // Keep the symbols and the constraints! alive.
+  (void)(a * b * c * d);
+  clang_analyzer_eval(c == d); // expected-warning{{TRUE}}
+  return;
+}
+
+void test_contradiction(int a, int b, int c, int d) {
+  if (a + b != c)
+return;
+  if (a == c)
+return;
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+
+  // Bring in the contradiction.
+  if (b != 0)
+return;
+  // Keep the symbols and the constraints! alive.
+  (void)(a * b * c * d);
+  clang_analyzer_warnIfReached(); // no-warning, i.e. UNREACHABLE
+  return;
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ADT/FoldingSet.h"
 #include "llvm/AD

[PATCH] D103082: [AArch64][SVE] Improve codegen for dupq SVE ACLE intrinsics

2021-06-02 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

Can we add a few end-to-end tests of bool svdupq with constant operands to 
acle_sve_dupq.c?  The pattern matching to create ptrue seems a bit fragile, so 
I want to make sure we don't break it by accident.




Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:415
+  auto *DupXArg = dyn_cast(DupX->getArgOperand(0));
+  if (!DupXArg || DupXArg->getZExtValue() != 0)
+return None;

isZero(). (Every time getZExtValue() is used somewhere, I have to think about 
whether the value might be wider than 64 bits, so please avoid where possible.)



Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:435
+  // index zero
+  if (!dyn_cast(VecIns->getArgOperand(0)))
+return None;

isa


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103082

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


[PATCH] D103547: Don't delete the module you're inspecting

2021-06-02 Thread Chris Bieneman via Phabricator via cfe-commits
beanz updated this revision to Diff 349350.
beanz added a comment.

Breaking out a separate test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103547

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Modules/clang_module_file_info.m


Index: clang/test/Modules/clang_module_file_info.m
===
--- /dev/null
+++ clang/test/Modules/clang_module_file_info.m
@@ -0,0 +1,16 @@
+
+@import DependsOnModule;
+
+// RUN: rm -rf %t %t-obj
+// RUN: %clang_cc1 -w -Wunused -fmodules -fmodule-format=obj 
-fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t-obj -F 
%S/Inputs -DBLARG -DWIBBLE=WOBBLE -fmodule-feature myfeature %s
+// RUN: %clang -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s
+// RUN: %clang -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s
+
+// This test is just verifying that the clang driver doesn't delete the input
+// file when -module-file-info is passed. We verify this by dumping the module
+// twice subsequently. We have other tests to verify the contents of the module
+// and the tool output (see: module_file_info.m)
+
+// CHECK: Generated by this Clang:
+
+// CHECK: Module name: DependsOnModule
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4883,6 +4883,11 @@
 return "-";
   }
 
+  if (JA.getType() == types::TY_ModuleFile &&
+  C.getArgs().getLastArg(options::OPT_module_file_info)) {
+return "-";
+  }
+
   // Is this the assembly listing for /FA?
   if (JA.getType() == types::TY_PP_Asm &&
   (C.getArgs().hasArg(options::OPT__SLASH_FA) ||


Index: clang/test/Modules/clang_module_file_info.m
===
--- /dev/null
+++ clang/test/Modules/clang_module_file_info.m
@@ -0,0 +1,16 @@
+
+@import DependsOnModule;
+
+// RUN: rm -rf %t %t-obj
+// RUN: %clang_cc1 -w -Wunused -fmodules -fmodule-format=obj -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t-obj -F %S/Inputs -DBLARG -DWIBBLE=WOBBLE -fmodule-feature myfeature %s
+// RUN: %clang -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s
+// RUN: %clang -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s
+
+// This test is just verifying that the clang driver doesn't delete the input
+// file when -module-file-info is passed. We verify this by dumping the module
+// twice subsequently. We have other tests to verify the contents of the module
+// and the tool output (see: module_file_info.m)
+
+// CHECK: Generated by this Clang:
+
+// CHECK: Module name: DependsOnModule
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4883,6 +4883,11 @@
 return "-";
   }
 
+  if (JA.getType() == types::TY_ModuleFile &&
+  C.getArgs().getLastArg(options::OPT_module_file_info)) {
+return "-";
+  }
+
   // Is this the assembly listing for /FA?
   if (JA.getType() == types::TY_PP_Asm &&
   (C.getArgs().hasArg(options::OPT__SLASH_FA) ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D103549: [POC] Put annotation strings into debuginfo.

2021-06-02 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

Generally arbitrary strings are best avoided where possible owing to lack of 
structure, type safety, semantics, etc. But they might be suitable here since 
they're opaque to everything from the frontend to the backend.

As for supporting it in DWARF, probably with a custom attribute 
(`DW_AT_BTF_annotation`? (or "LLVM" instead of "BTF" perhaps, I'm not sure)) 
with a standard form (DW_FORM_strp/strxN/etc - the usual way we emit strings).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103549

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


[clang] 8beaca8 - Remove unused function from a previous iteration of unique-stable-name

2021-06-02 Thread Erich Keane via cfe-commits

Author: Erich Keane
Date: 2021-06-02T12:15:16-07:00
New Revision: 8beaca8c1493f576431c3687329860e918616cd9

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

LOG: Remove unused function from a previous iteration of unique-stable-name

Added: 


Modified: 
clang/lib/Sema/TreeTransform.h

Removed: 




diff  --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 1935bffccb6d..500f72d0cff3 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -2399,12 +2399,6 @@ class TreeTransform {
 return SEHFinallyStmt::Create(getSema().getASTContext(), Loc, Block);
   }
 
-  ExprResult RebuildSYCLUniqueStableNameExpr(SourceLocation OpLoc,
- SourceLocation LParen,
- SourceLocation RParen, Expr *E) {
-return getSema().BuildSYCLUniqueStableNameExpr(OpLoc, LParen, RParen, E);
-  }
-
   ExprResult RebuildSYCLUniqueStableNameExpr(SourceLocation OpLoc,
  SourceLocation LParen,
  SourceLocation RParen,



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


[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

2021-06-02 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

In D98799#2794366 , @ahatanak wrote:

> I don't know know why these fake FunctionDecls are needed, but it looks like 
> it's okay to avoid creating them. I see a few debug info tests failing if 
> `GlobalDecl()` instead of a fake function is passed to `StartFunction`, but 
> it looks like the test check strings should be changed.
>
> AFAICT `CodeGenFunction::GenerateCopyHelperFunction` has been creating these 
> fake FunctionDecls from the beginning and subsequent patches didn't fix it:
>
> https://github.com/llvm/llvm-project/commit/0c74327715af823930cb583490d315f64ef8de4e

Would you be able to make that change, if it's not too much work? (I'm not as 
familiar with the Objective C(++) stuff)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98799

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


[PATCH] D103547: Don't delete the module you're inspecting

2021-06-02 Thread Steven Wu via Phabricator via cfe-commits
steven_wu added a comment.

Can you create a new test for clang Driver instead of rewrite the cc1 test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103547

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


[PATCH] D103543: [compiler-rt][Fuchsia] Support HWASan on Fuchsia

2021-06-02 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr accepted this revision.
mcgrathr added a comment.
This revision is now accepted and ready to land.

I think this may need to be about the last thing to actually land.  It 
shouldn't land before building the runtime reliably works without error, and I 
think we should land any refactoring of the runtime we needed in preparation 
for Fuchsia-specific changes before landing anything Fuchsia-specific to make 
it compile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103543

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


[PATCH] D103549: [POC] Put annotation strings into debuginfo.

2021-06-02 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song created this revision.
yonghong-song added a project: debug-info.
Herald added subscribers: dexonsmith, pengfei, JDevlieghere, hiraditya, 
kristof.beyls, mgorny.
yonghong-song requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This is a Proof-Of-Concept patch and intends to seek suggestions
from llvm community on how to put an attribute with arbitrary
string into the final debuginfo in the object file.

The Use Cases
=

In BPF ecosystem, BTF is the debuginfo used for validation
and additional information.

  https://www.kernel.org/doc/html/latest/bpf/btf.html

Currently, BTF in vmlinux (x86_64, aarch64, etc.) are
generated by using pahole to convert dwarf to BTF and
vmlinux BTF is used to validate bpf program compliance,
e.g., bpf program signature must match kernel function
signature for certain tracing programs. Beyond signature
checking, the following are use cases which will further
help verifier.

1. annotation of "user"/"rcu" etc for function arguments, structure fields and 
global/static variables. Kernel currently uses `address_space` attributes for 
`sparse` tool. But we could like to carry this information to debuginfo. 
Previous attempt https://reviews.llvm.org/D69393 tries to use `address_space` 
which is halted as it needs to touch a lot of other llvm places.
2. annotation of functions. Currently, kernel tries to group them with separate 
logic, e.g., foo() __attribute__("property1", "property2") since the above 
attribute is not supported, kernel has to do some magic like global 
btf_property1: btf type id for foo, ... global btf_property2: btf type id for 
foo, ... this is really error prone as the function definition may be under 
some configs and the `global btf_property1 ...` may not even in the same source 
file as the function. Such a disconnect between function definition and 
function attributes already caused numerous issues.

  We also want to annotate functions with certain pre-conditions (e.g., a 
socket lock has been hold), as bpf programs has started to call kernel 
functions. Such annotations should be really directly applied to the function 
definition to avoid any potential later mismatch issues.
3. annotation of structures, e.g., if somehow this structure fields may have 
been randomized, verifier should know it as it cannot trust debuginfo structure 
layout any more.

Sorry for tense explanation of use cases. The main takeaway
is we want to annotate structure/field/func/argument/variable
with *arbitrary* strings and want such strings to be preserved
in final dwarf (or BTF) output.

An Example
==

In this patch, I hacked clang Frontend to put annotations
in debuginfo and hacked llvm/CodeGen to "output" these
annotations into BTF. The target architecture is x86.
Note that I didn't really output these attributes to BTF yet.
I would like to seek llvm community advise first.

Below is an example to show what the source code looks like.
I am using "annotate" attribute as it accepts arbitrary strings.

  $ cat t1.c
  /* a pointer pointing to user memory */
  #define __user __attribute__((annotate("user")))
  /* a pointer protected by rcu */
  #define __rcu __attribute__((annotate("rcu")))
  /* the struct has some special property */
  #define __special_struct __attribute__((annotate("special_struct")))
  /* sock_lock is held for the function */
  #define __sock_lock_held __attribute((annotate("sock_lock_held")))
  /* the hash table element type is socket */
  #define __special_info __attribute__((annotate("elem_type:socket")))
  
  struct hlist_node;
  struct hlist_head {
struct hlist_node *prev;
struct hlist_node *next;
  } __special_struct;
  struct hlist {
 struct hlist_head head __special_info;
  };
  
  extern void bar(struct hlist *);
  int foo(struct hlist *h,  int *a __user, int *b __rcu) __sock_lock_held {
bar(h);
return *a + *b;
  }
  $ clang --target x86_64 -O2 -c -g t1.c
  TODO (BTF2Debug.cpp): Add func arg 'a' annotation 'user' to .BTF section
  TODO (BTF2Debug.cpp): Add func arg 'b' annotation 'rcu' to .BTF section
  TODO (BTF2Debug.cpp): Add subroutine 'foo' annotation 'sock_lock_held' to 
.BTF section
  TODO (BTF2Debug.cpp): Add field 'head' annotation 'elem_type:socket' to .BTF 
section
  TODO (BTF2Debug.cpp): Add struct 'hlist_head' annotation 'special_struct' to 
.BTF section
  $

What Is Next


First, using "annotate" attribute is not the best choice as I generated
extra globals and IRs. Maybe a different clang specific attribute?

Second, in the above example, I tried to put these attributes in BTF
as I researched and didn't find a way to put these attributes in dwarf.
Do we have a way to put it into dwarf? That works for us too.
Otherwise, we can let x86/arm64 etc. generates BTF (with a flag of course)
which will have these attribute information.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103549

F

[PATCH] D103544: [compiler-rt][Fuchsia] Disable interceptors while enabling new/delete replacements

2021-06-02 Thread Roland McGrath via Phabricator via cfe-commits
mcgrathr added inline comments.



Comment at: compiler-rt/lib/hwasan/CMakeLists.txt:45
+if (NOT FUCHSIA)
+  append_list_if(COMPILER_RT_HWASAN_WITH_INTERCEPTORS 
HWASAN_WITH_INTERCEPTORS=1 HWASAN_DEFINITIONS)
+else()

It might be better to force the value of COMPILER_RT_HWASAN_WITH_INTERCEPTORS 
to OFF instead.

But I'll leave the cmake details to Petr.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103544

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


[PATCH] D98799: [UniqueLinkageName] Use consistent checks when mangling symbo linkage name and debug linkage name.

2021-06-02 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

I don't know know why these fake FunctionDecls are needed, but it looks like 
it's okay to avoid creating them. I see a few debug info tests failing if 
`GlobalDecl()` instead of a fake function is passed to `StartFunction`, but it 
looks like the test check strings should be changed.

AFAICT `CodeGenFunction::GenerateCopyHelperFunction` has been creating these 
fake FunctionDecls from the beginning and subsequent patches didn't fix it:

https://github.com/llvm/llvm-project/commit/0c74327715af823930cb583490d315f64ef8de4e


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98799

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


[PATCH] D103547: Don't delete the module you're inspecting

2021-06-02 Thread Chris Bieneman via Phabricator via cfe-commits
beanz created this revision.
beanz added reviewers: steven_wu, arphaman.
beanz requested review of this revision.
Herald added a project: clang.

Prior to this patch when you used `clang -module-file-info` clang would delete 
the module on completion because the module was treated as an output file.

This fixes the issue so you don't need to invoke cc1 directly to get module 
file information.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103547

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Modules/module_file_info.m


Index: clang/test/Modules/module_file_info.m
===
--- clang/test/Modules/module_file_info.m
+++ clang/test/Modules/module_file_info.m
@@ -7,8 +7,8 @@
 // RUN: %clang_cc1 -module-file-info %t/DependsOnModule.pcm | FileCheck %s 
--check-prefix=RAW
 
 // RUN: %clang_cc1 -w -Wunused -fmodules -fmodule-format=obj 
-fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t-obj -F 
%S/Inputs -DBLARG -DWIBBLE=WOBBLE -fmodule-feature myfeature %s
-// RUN: %clang_cc1 -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s
-// RUN: %clang_cc1 -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s 
--check-prefix=OBJ
+// RUN: %clang -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s
+// RUN: %clang -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s 
--check-prefix=OBJ
 
 // RAW:   Module format: raw
 // OBJ:   Module format: obj
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4883,6 +4883,11 @@
 return "-";
   }
 
+  if (JA.getType() == types::TY_ModuleFile &&
+  C.getArgs().getLastArg(options::OPT_module_file_info)) {
+return "-";
+  }
+
   // Is this the assembly listing for /FA?
   if (JA.getType() == types::TY_PP_Asm &&
   (C.getArgs().hasArg(options::OPT__SLASH_FA) ||


Index: clang/test/Modules/module_file_info.m
===
--- clang/test/Modules/module_file_info.m
+++ clang/test/Modules/module_file_info.m
@@ -7,8 +7,8 @@
 // RUN: %clang_cc1 -module-file-info %t/DependsOnModule.pcm | FileCheck %s --check-prefix=RAW
 
 // RUN: %clang_cc1 -w -Wunused -fmodules -fmodule-format=obj -fimplicit-module-maps -fdisable-module-hash -fmodules-cache-path=%t-obj -F %S/Inputs -DBLARG -DWIBBLE=WOBBLE -fmodule-feature myfeature %s
-// RUN: %clang_cc1 -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s
-// RUN: %clang_cc1 -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s --check-prefix=OBJ
+// RUN: %clang -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s
+// RUN: %clang -module-file-info %t-obj/DependsOnModule.pcm | FileCheck %s --check-prefix=OBJ
 
 // RAW:   Module format: raw
 // OBJ:   Module format: obj
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4883,6 +4883,11 @@
 return "-";
   }
 
+  if (JA.getType() == types::TY_ModuleFile &&
+  C.getArgs().getLastArg(options::OPT_module_file_info)) {
+return "-";
+  }
+
   // Is this the assembly listing for /FA?
   if (JA.getType() == types::TY_PP_Asm &&
   (C.getArgs().hasArg(options::OPT__SLASH_FA) ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D103495: [static initializers] Don't put ordered dynamic initializers of static variables into global_ctors

2021-06-02 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added subscribers: aeubanks, hans, rsmith.
rnk added a comment.

+@rsmith @hans @aeubanks

I think this would be an unfortunate code size and startup time regression for 
Itanium C++ inline variables. The existing code was written under the 
assumption that vague linkage (GVA_DiscardableODR) implies no initialization 
ordering, but I think you've found a valid counterexample.

> specifically when init_array is not used

Can you elaborate on why that is? Here's what I remember, and what I guess is 
happening. ELF has two initializer schemes: .init_array and .ctors. They are 
essentially equivalent, they are both arrays of function pointers, but one is 
called in reverse order and the other is called in forward order. The compiler 
knows which scheme is in use and it is controlled by -fuse-init-array.

The LLVM IR langref says that llvm.global_ctors are called in ascending 
priority order, and the order between initializers is not defined. That's not 
very helpful and often doesn't reflect reality. I wonder if we could do two 
things, perhaps separately:

1. emit llvm.global_ctors so that they are called in order of appearance in the 
IR array (is this not already true?)
2. define the order of initialization in LangRef

With the first change in place, we can be confident that so long as all 
includers of the `StaticsClass` in the test emit both initializers in the 
correct order, no matter which initializers prevail, they will be called in the 
correct order. Of course, this could break users relying on the existing 
ordering, nevermind that it is explicitly documented as undefined.

The second change is only a documentation change, but I would like to find a 
way to justify what LLVM does in GlobalOpt. GlobalOpt can effectively fold a 
dynamic initializer to constant memory in certain cases. That can only ever 
have the effect of making an initializer run earlier. As long as no 
initializers depend on observing uninitialized globals, that should be safe. 
The guarantee that we want to provide is that, for each initializer, all 
initializers prior to it in the global_ctors array will have run when it runs. 
Initializers that appear later may run earlier. Hopefully that covers both the 
usual way that .init_array sections are linked together and the way globalopt 
optimizes dynamic initialization.


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

https://reviews.llvm.org/D103495

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


[PATCH] D103544: [compiler-rt][Fuchsia] Disable interceptors while enabling new/delete replacements

2021-06-02 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan created this revision.
leonardchan added reviewers: phosek, mcgrathr, charco.
leonardchan added a project: Sanitizers.
Herald added subscribers: mgorny, dberris.
leonardchan requested review of this revision.
Herald added a subscriber: Sanitizers.

This disables use of hwasan interceptors which we do not use on Fuchsia. This 
explicitly sets the macro for defining the hwasan versions of `new/delete`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103544

Files:
  compiler-rt/lib/hwasan/CMakeLists.txt


Index: compiler-rt/lib/hwasan/CMakeLists.txt
===
--- compiler-rt/lib/hwasan/CMakeLists.txt
+++ compiler-rt/lib/hwasan/CMakeLists.txt
@@ -41,7 +41,11 @@
   )
 
 set(HWASAN_DEFINITIONS)
-append_list_if(COMPILER_RT_HWASAN_WITH_INTERCEPTORS HWASAN_WITH_INTERCEPTORS=1 
HWASAN_DEFINITIONS)
+if (NOT FUCHSIA)
+  append_list_if(COMPILER_RT_HWASAN_WITH_INTERCEPTORS 
HWASAN_WITH_INTERCEPTORS=1 HWASAN_DEFINITIONS)
+else()
+  list(APPEND HWASAN_DEFINITIONS HWASAN_REPLACE_OPERATORS_NEW_AND_DELETE=1)
+endif()
 
 set(HWASAN_RTL_CFLAGS ${SANITIZER_COMMON_CFLAGS})
 append_rtti_flag(OFF HWASAN_RTL_CFLAGS)


Index: compiler-rt/lib/hwasan/CMakeLists.txt
===
--- compiler-rt/lib/hwasan/CMakeLists.txt
+++ compiler-rt/lib/hwasan/CMakeLists.txt
@@ -41,7 +41,11 @@
   )
 
 set(HWASAN_DEFINITIONS)
-append_list_if(COMPILER_RT_HWASAN_WITH_INTERCEPTORS HWASAN_WITH_INTERCEPTORS=1 HWASAN_DEFINITIONS)
+if (NOT FUCHSIA)
+  append_list_if(COMPILER_RT_HWASAN_WITH_INTERCEPTORS HWASAN_WITH_INTERCEPTORS=1 HWASAN_DEFINITIONS)
+else()
+  list(APPEND HWASAN_DEFINITIONS HWASAN_REPLACE_OPERATORS_NEW_AND_DELETE=1)
+endif()
 
 set(HWASAN_RTL_CFLAGS ${SANITIZER_COMMON_CFLAGS})
 append_rtti_flag(OFF HWASAN_RTL_CFLAGS)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D103543: [compiler-rt][Fuchsia] Support HWASan on Fuchsia

2021-06-02 Thread Leonard Chan via Phabricator via cfe-commits
leonardchan created this revision.
leonardchan added reviewers: phosek, mcgrathr, charco.
leonardchan added a project: Sanitizers.
Herald added subscribers: mgorny, dberris.
leonardchan requested review of this revision.
Herald added a subscriber: Sanitizers.

This allows for hwasan to be built targetting fuchsia.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103543

Files:
  compiler-rt/cmake/config-ix.cmake


Index: compiler-rt/cmake/config-ix.cmake
===
--- compiler-rt/cmake/config-ix.cmake
+++ compiler-rt/cmake/config-ix.cmake
@@ -711,7 +711,7 @@
 endif()
 
 if (COMPILER_RT_HAS_SANITIZER_COMMON AND HWASAN_SUPPORTED_ARCH AND
-OS_NAME MATCHES "Linux|Android")
+OS_NAME MATCHES "Linux|Android|Fuchsia")
   set(COMPILER_RT_HAS_HWASAN TRUE)
 else()
   set(COMPILER_RT_HAS_HWASAN FALSE)


Index: compiler-rt/cmake/config-ix.cmake
===
--- compiler-rt/cmake/config-ix.cmake
+++ compiler-rt/cmake/config-ix.cmake
@@ -711,7 +711,7 @@
 endif()
 
 if (COMPILER_RT_HAS_SANITIZER_COMMON AND HWASAN_SUPPORTED_ARCH AND
-OS_NAME MATCHES "Linux|Android")
+OS_NAME MATCHES "Linux|Android|Fuchsia")
   set(COMPILER_RT_HAS_HWASAN TRUE)
 else()
   set(COMPILER_RT_HAS_HWASAN FALSE)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D99540: [clangd] Preserve diags between tweak enumeration and execution

2021-06-02 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

Sorry I've lost my context - did we decide to move forward with this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99540

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


[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

2021-06-02 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD added a comment.

Important question from @vsavchenko:

> I have two major questions about this implementation:
>
> - Why don't we need an actual check for `IfStmt`? Won't it trigger on `bool 
> unused = !pointer;`? And if so it won't mean **constrained**.
> - Why do we only care about implicit pointer-to-bool conversion? What about 
> situations like `pointer == nullptr`, `NULL != pointer`, 
> `__builtin_expect(pointer, 0)`, etc?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183

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


[PATCH] D100308: [clangd] Drop optional on ExternalIndexSpec

2021-06-02 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: cfe-commits.

Sorry about delay.




Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:555
+  case Config::ExternalIndexSpec::None:
+break;
   case Config::ExternalIndexSpec::Server:

I think you hit llvm_unreachable here - is this an invariant enforced by the 
caller, or did you mean to return nullptr?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100308

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


[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

2021-06-02 Thread Deep Majumder via Phabricator via cfe-commits
RedDocMD updated this revision to Diff 349332.
RedDocMD added a comment.

Moved visitor entirely to SmartPtrChecker.cpp, other refactors, better naming.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtr.h
  clang/lib/StaticAnalyzer/Checkers/SmartPtrChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/test/Analysis/smart-ptr-text-output.cpp

Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -306,10 +306,81 @@
 };
 
 void derefAfterBranchingOnUnknownInnerPtr(std::unique_ptr P) {
-  A *RP = P.get();
+  A *RP = P.get(); // expected-note {{Obtained null inner pointer from 'P'}}
   if (!RP) { // expected-note {{Assuming 'RP' is null}}
 // expected-note@-1 {{Taking true branch}}
 P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
 // expected-note@-1{{Dereference of null smart pointer 'P'}}
   }
 }
+
+void multpleDeclsWithGet(std::unique_ptr P) {
+  A *dummy1 = nullptr, *RP = P.get(), *dummy2; // expected-note {{Obtained null inner pointer from 'P'}}
+  if (!RP) {   // expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+void multipleGetsShouldNotAllHaveNotes(std::unique_ptr P) {
+  A *RP = P.get(); // expected-note {{Obtained null inner pointer from 'P'}}
+  A *dummy1 = P.get();
+  A *dummy2 = P.get();
+  if (!RP) { // expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+void getShouldNotAlwaysLeaveANote() {
+  std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
+  A *a = P.get();
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+}
+
+void getShouldNotLeaveANoteAfterReset(std::unique_ptr P) {
+  A *a = P.get();
+  P.reset(); // expected-note {{Smart pointer 'P' reset using a null value}}
+  P->foo();  // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+ // expected-note@-1{{Dereference of null smart pointer 'P'}}
+}
+
+void getShouldNotLeaveNoteWhenPtrNotUsed(std::unique_ptr P) {
+  A *a = P.get();
+  if (!P) { // expected-note {{Taking true branch}}
+// expected-note@-1 {{Assuming smart pointer 'P' is null}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+void getShouldLeaveANoteWithWhileLoop(std::unique_ptr P) {
+  A *RP = P.get(); // expected-note {{Obtained null inner pointer from 'P'}}
+  while (!RP) {// expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{Loop condition is true.  Entering loop body}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+void getShouldLeaveANoteWithForLoop(std::unique_ptr P) {
+  for (A *RP = P.get(); !RP;) { // expected-note {{Assuming 'RP' is null}}
+// expected-note@-1 {{Loop condition is true.  Entering loop body}}
+// expected-note@-2 {{Obtained null inner pointer from 'P'}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+void getShouldLeaveNoteOnChaining(std::unique_ptr P) {
+  A *praw = P.get(), *other; // expected-note {{Obtained null inner pointer from 'P'}}
+  other = praw;  // expected-note {{Obtained null value here}}
+  if (!other) {  // expected-note {{Assuming 'other' is null}}
+// expected-note@-1 {{Taking true branch}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
\ No newline at end of file
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -76,11 +76,18 @@
   {{"release"}, &SmartPtrModeling::handleRelease},
   {{"swap", 1}, &

[PATCH] D103449: [clangd][Protocol] Drop optional from WorkspaceEdit::changes

2021-06-02 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

changes is in fact optional, indicated by the `?` in `changes?: { [uri: 
DocumentUri]: TextEdit[]; };`.

But the spec requires *some* field to be set, and this is the only one we 
support, so it's not optional in practice for us.




Comment at: clang-tools-extra/clangd/Protocol.cpp:811
 llvm::json::Value toJSON(const WorkspaceEdit &WE) {
-  if (!WE.changes)
+  if (WE.changes.empty())
 return llvm::json::Object{};

I'd suggest deleting this special case.
We no longer have two distinct states of our WorkspaceEdit struct to represent 
`{changes:{}}` and `{}`.

`{changes:{}}` is a well-defined empty edit, even if that never makes sense to 
actually send.
`{}` conforms to the typescript type definition of the spec, but doesn't 
actually define an edit per the spec text.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103449

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


  1   2   >