[PATCH] D115103: Leak Sanitizer port to Windows

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: compiler-rt/lib/lsan/lsan.h:20
+#elif SANITIZER_WINDOWS
+#  include "lsan_win.h"
 #endif

vitalybuka wrote:
> MyDeveloperDay wrote:
> > clang-format?
> @clemenswasser  Can you please a separate tiny patch which clang-format 
> nearby lines
Sorry, is there a reason we can't follow the current style in this patch? sorry 
did I miss something?  Its not about fighting clang-format its about ensuring 
we follow the style from the lines around it which are following the 
clang-format style. I don't need you to clang-format the whole file (although 
that would be great because this directory has very low clang-format status)

{F20932130, size=full}
https://clang.llvm.org/docs/ClangFormattedStatus.html

All I'm asking is that we don't indent the preprocessor directives to a non 
LLVM style.

i.e.

```
#include "lsan_win.h"
#if !SANITIZER_WINDOWS
#endif
```

vs

```
#  include "lsan_win.h"
#  if !SANITIZER_WINDOWS
#  endif
```


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

https://reviews.llvm.org/D115103

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

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D115149#3175068 , @NoQ wrote:

>> It can happen if the `Loc` was perfectly constrained to a concrete
>> value (`nonloc::ConcreteInt`)
>
> This shouldn't happen. It should be `loc::ConcreteInt` which is, well, a 
> `Loc`.

I see your point and thank you for taking a look. Perhaps there is a problem in 
`SimpleSValBuilder::simplifySVal` which is responsible to use the constraint 
and build up the `ConcreteInt`. Especially, `VisitSymbolData` and `getConst` is 
worth to continue with the investigation, we might have the wrong type 
associated to the symbol:

  if (Const)
return Loc::isLocType(Sym->getType()) ? (SVal)SVB.makeIntLocVal(*Const)
  : (SVal)SVB.makeIntVal(*Const);

We are going to further investigate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115149

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

[PATCH] D115103: Leak Sanitizer port to Windows

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a subscriber: kuhnel.
MyDeveloperDay added inline comments.



Comment at: compiler-rt/lib/lsan/lsan_common.h:48
+#elif SANITIZER_NETBSD || SANITIZER_FUCHSIA || SANITIZER_WINDOWS
+#  define CAN_SANITIZE_LEAKS 1
 #else

vitalybuka wrote:
> MyDeveloperDay wrote:
> > you probably don't want to change this do you?
> I am fine like this, fighting clang-format is not useful
> however it would be nice if you clang-format relevant block in a separate 
> patch then then rebase this patch on top of it
it might not seem useful to individuals,  but I think its the guidance, and a 
consistent style really help new people to understand what's going on.

https://llvm.org/docs/Contributing.html#how-to-submit-a-patch

I'm a little surprise the "llvm-premerge-checks" didn't highlight this @kuhnel 




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

https://reviews.llvm.org/D115103

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

[PATCH] D114938: [Analyzer][NFCi] SValBuilder: Simlify a SymExpr to the absolute simplest form

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D114938#3174331 , @steakhal wrote:

> You mentioned in the summary that there are different places where 
> simplification-like machinary kicks in, which hindered the test case 
> synthesis. What places did you refer to exactly?

These are the places where we call `SimpleSValBuilder::simplifySVal`.

1. EquivalenceClass::simplify
2. SimpleSValBuilder::evalBinOpNN
3. SimpleSValBuilder::getKnownValue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114938

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

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 392297.
simoll added a comment.

Formatting / Cleanup


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115045

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h
  clang/lib/Driver/ToolChains/BareMetal.h
  clang/lib/Driver/ToolChains/Fuchsia.h
  clang/lib/Driver/ToolChains/Hexagon.h
  clang/lib/Driver/ToolChains/MipsLinux.h
  clang/lib/Driver/ToolChains/Solaris.h
  clang/lib/Driver/ToolChains/WebAssembly.h
  clang/test/Driver/ve-toolchain.c
  clang/test/Driver/ve-toolchain.cpp

Index: clang/test/Driver/ve-toolchain.cpp
===
--- clang/test/Driver/ve-toolchain.cpp
+++ clang/test/Driver/ve-toolchain.cpp
@@ -110,10 +110,10 @@
 /// Checking -fintegrated-as
 
 // RUN: %clangxx -### -target ve-unknown-linux-gnu \
-// RUN: -x assembler -fuse-ld=ld %s 2>&1 | \
+// RUN: -x assembler %s 2>&1 | \
 // RUN:FileCheck -check-prefix=AS %s
 // RUN: %clangxx -### -target ve-unknown-linux-gnu \
-// RUN: -fno-integrated-as -x assembler -fuse-ld=ld %s 2>&1 | \
+// RUN: -fno-integrated-as -x assembler %s 2>&1 | \
 // RUN:FileCheck -check-prefix=NAS %s
 
 // AS: clang{{.*}} "-cc1as"
@@ -131,7 +131,6 @@
 
 // RUN: %clangxx -### -target ve-unknown-linux-gnu \
 // RUN: --sysroot %S/Inputs/basic_ve_tree \
-// RUN: -fuse-ld=ld \
 // RUN: -resource-dir=%S/Inputs/basic_ve_tree/resource_dir \
 // RUN: --stdlib=c++ %s 2>&1 | FileCheck -check-prefix=DEF %s
 
Index: clang/test/Driver/ve-toolchain.c
===
--- clang/test/Driver/ve-toolchain.c
+++ clang/test/Driver/ve-toolchain.c
@@ -61,10 +61,10 @@
 /// Checking -fintegrated-as
 
 // RUN: %clang -### -target ve \
-// RUN:-x assembler -fuse-ld=ld %s 2>&1 | \
+// RUN:-x assembler %s 2>&1 | \
 // RUN:FileCheck -check-prefix=AS %s
 // RUN: %clang -### -target ve \
-// RUN:-fno-integrated-as -fuse-ld=ld -x assembler %s 2>&1 | \
+// RUN:-fno-integrated-as -x assembler %s 2>&1 | \
 // RUN:FileCheck -check-prefix=NAS %s
 
 // AS: clang{{.*}} "-cc1as"
@@ -83,7 +83,6 @@
 // RUN: %clang -### -target ve-unknown-linux-gnu \
 // RUN: --sysroot %S/Inputs/basic_ve_tree \
 // RUN: -resource-dir=%S/Inputs/basic_ve_tree/resource_dir \
-// RUN: -fuse-ld=ld \
 // RUN: %s 2>&1 | FileCheck -check-prefix=DEF %s
 
 // DEF:  clang{{.*}}" "-cc1"
Index: clang/lib/Driver/ToolChains/WebAssembly.h
===
--- clang/lib/Driver/ToolChains/WebAssembly.h
+++ clang/lib/Driver/ToolChains/WebAssembly.h
@@ -67,7 +67,9 @@
llvm::opt::ArgStringList &CmdArgs) const override;
   SanitizerMask getSupportedSanitizers() const override;
 
-  const char *getDefaultLinker() const override { return "wasm-ld"; }
+  const char *getDefaultLinker() const override {
+return getConfiguredDefaultLinker("wasm-ld");
+  }
 
   Tool *buildLinker() const override;
 
Index: clang/lib/Driver/ToolChains/Solaris.h
===
--- clang/lib/Driver/ToolChains/Solaris.h
+++ clang/lib/Driver/ToolChains/Solaris.h
@@ -67,7 +67,7 @@
 
   const char *getDefaultLinker() const override {
 // clang currently uses Solaris ld-only options.
-return "/usr/bin/ld";
+return getConfiguredDefaultLinker("/usr/bin/ld");
   }
 
 protected:
Index: clang/lib/Driver/ToolChains/MipsLinux.h
===
--- clang/lib/Driver/ToolChains/MipsLinux.h
+++ clang/lib/Driver/ToolChains/MipsLinux.h
@@ -49,7 +49,7 @@
   }
 
   const char *getDefaultLinker() const override {
-return "ld.lld";
+return getConfiguredDefaultLinker("ld.lld");
   }
 
 private:
Index: clang/lib/Driver/ToolChains/Hexagon.h
===
--- clang/lib/Driver/ToolChains/Hexagon.h
+++ clang/lib/Driver/ToolChains/Hexagon.h
@@ -85,7 +85,8 @@
  llvm::opt::ArgStringList &CC1Args) const override;
 
   const char *getDefaultLinker() const override {
-return getTriple().isMusl() ? "ld.lld" : "hexagon-link";
+return getConfiguredDefaultLinker(getTriple().isMusl() ? "ld.lld"
+   : "hexagon-link");
   }
 
   CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList &Args) const override;
Index: clang/lib/Driver/ToolChains/Fuchsia.h
===
--- clang/lib/Driver/ToolChains/Fuchsia.h
+++ clang/lib/Driver/ToolChains/Fuchsia.h
@@ -91,7 +91,7 @@
llvm::opt::ArgStringList &CmdArgs) const override;
 
   const char *getDefaultLinker() const override {
-return "ld.lld";
+return getConfiguredDefaul

[PATCH] D114439: [Annotation] Allow parameter pack expansions in annotate attribute

[Steffen Larsen via Phabricator via cfe-commits]

steffenlarsen added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:724
+  "attribute %0 does not support pack expansion in the last argument">;
+def err_attribute_parm_pack_last_argument_only : Error<
+  "pack expansion in attributes is restricted to only the last argument">;

erichkeane wrote:
> I don't really see why this is required?  I would think the 722 would be all 
> you would need.
Intention was to make a distinction between the two cases, since we had the 
granularity to do so. I.e. `clang::annotate` would never use 722 as it 
explicitly states that the attribute doesn't support pack expansion (in the 
last argument) which is untrue for `clang::annotate`, but if a user was to do 
pack expansion on the first argument of `clang::annotate` they would get this 
error telling them that the given argument cannot accept the pack expansion.



Comment at: clang/lib/Parse/ParseDecl.cpp:450
+  // Pack expansion is only allowed in the last attribute argument.
+  if (ArgExprs.size() + 1 < attributeNumberOfArguments(*AttrName)) {
+Diag(Tok.getLocation(),

erichkeane wrote:
> I don't think this should be diagnosed here, and I don't think it is 'right'. 
>  I think the ClangAttrEmitter should ensure that the VariadicExprArgument 
> needs to be the 'last' thing, but I think that REALLY means "supports a pack 
> anywhere inside of it".
> 
> See my test examples below, I don't think this parsing is sufficient for that.
That is also the intention here. All it checks is that we are in or beyond the 
last argument of the attribute. For example, `clang::annotate` will return 2 
from `attributeNumberOfArguments` as `VariadicExprArgument` only counts as a 
single argument. Thus, any pack expansion expressions parsed after the first 
will be accepted. I do agree though that the error message may be a little 
confusing for users.

I will add the suggested tests and rethink the diagnostics.


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

https://reviews.llvm.org/D114439

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

[PATCH] D115219: [C++20] [Coroutines] Mark coroutine done if unhandled_exception throws

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 392302.
ChuanqiXu edited the summary of this revision.
ChuanqiXu added a comment.

Use `llvm.coro.end(frame, /*InUnwindPath=*/true)` instead of new coroutine 
intrinsics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115219

Files:
  llvm/lib/Transforms/Coroutines/CoroSplit.cpp
  llvm/test/Transforms/Coroutines/coro-split-eh-00.ll
  llvm/test/Transforms/Coroutines/coro-split-eh-01.ll


Index: llvm/test/Transforms/Coroutines/coro-split-eh-01.ll
===
--- llvm/test/Transforms/Coroutines/coro-split-eh-01.ll
+++ llvm/test/Transforms/Coroutines/coro-split-eh-01.ll
@@ -61,6 +61,9 @@
 ; CHECK:  lpad:
 ; CHECK-NEXT:   %tok = cleanuppad within none []
 ; CHECK-NEXT:   call void @print(i32 2)
+; Checks that the coroutine would be marked as done if it exits in unwinding 
path.
+; CHECK-NEXT:   %[[RESUME_ADDR:.+]] = getelementptr inbounds %[[FRAME_TY:.+]], 
%[[FRAME_TY]]* %FramePtr, i32 0, i32 0
+; CHECK-NEXT:   store void (%[[FRAME_TY]]*)* null, void (%[[FRAME_TY]]*)** 
%[[RESUME_ADDR]], align 8
 ; CHECK-NEXT:   cleanupret from %tok unwind to caller
 
 declare i8* @llvm.coro.free(token, i8*)
Index: llvm/test/Transforms/Coroutines/coro-split-eh-00.ll
===
--- llvm/test/Transforms/Coroutines/coro-split-eh-00.ll
+++ llvm/test/Transforms/Coroutines/coro-split-eh-00.ll
@@ -67,6 +67,9 @@
 ; CHECK-NEXT:  %lpval = landingpad { i8*, i32 }
 ; CHECK-NEXT: cleanup
 ; CHECK-NEXT:  call void @print(i32 2)
+; Checks that the coroutine would be marked as done if it exits in unwinding 
path.
+; CHECK-NEXT:  %[[RESUME_ADDR:.+]] = getelementptr inbounds 
%[[FRAME_TY:.+]], %[[FRAME_TY]]* %FramePtr, i32 0, i32 0
+; CHECK-NEXT:  store void (%[[FRAME_TY]]*)* null, void (%[[FRAME_TY]]*)** 
%[[RESUME_ADDR]], align 8
 ; CHECK-NEXT:  resume { i8*, i32 } %lpval
 
 declare i8* @llvm.coro.free(token, i8*)
Index: llvm/lib/Transforms/Coroutines/CoroSplit.cpp
===
--- llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -288,10 +288,24 @@
 
   switch (Shape.ABI) {
   // In switch-lowering, this does nothing in the main function.
-  case coro::ABI::Switch:
+  case coro::ABI::Switch: {
+// In C++'s specificatin, the corotine should be marked as done
+// if the coroutine leaves in unwinding path.
+//
+// FIXME: We should refactor this once there is other language
+// which uses Switch-Resumed style other than C++.
+//
+// Done state is represented by storing zero in ResumeFnAddr.
+auto *GepIndex = Builder.CreateStructGEP(
+Shape.FrameTy, FramePtr, coro::Shape::SwitchFieldIndex::Resume,
+"ResumeFn.addr");
+auto *NullPtr = ConstantPointerNull::get(cast(
+Shape.FrameTy->getTypeAtIndex(coro::Shape::SwitchFieldIndex::Resume)));
+Builder.CreateStore(NullPtr, GepIndex);
 if (!InResume)
   return;
 break;
+  }
   // In async lowering this does nothing.
   case coro::ABI::Async:
 break;


Index: llvm/test/Transforms/Coroutines/coro-split-eh-01.ll
===
--- llvm/test/Transforms/Coroutines/coro-split-eh-01.ll
+++ llvm/test/Transforms/Coroutines/coro-split-eh-01.ll
@@ -61,6 +61,9 @@
 ; CHECK:  lpad:
 ; CHECK-NEXT:   %tok = cleanuppad within none []
 ; CHECK-NEXT:   call void @print(i32 2)
+; Checks that the coroutine would be marked as done if it exits in unwinding path.
+; CHECK-NEXT:   %[[RESUME_ADDR:.+]] = getelementptr inbounds %[[FRAME_TY:.+]], %[[FRAME_TY]]* %FramePtr, i32 0, i32 0
+; CHECK-NEXT:   store void (%[[FRAME_TY]]*)* null, void (%[[FRAME_TY]]*)** %[[RESUME_ADDR]], align 8
 ; CHECK-NEXT:   cleanupret from %tok unwind to caller
 
 declare i8* @llvm.coro.free(token, i8*)
Index: llvm/test/Transforms/Coroutines/coro-split-eh-00.ll
===
--- llvm/test/Transforms/Coroutines/coro-split-eh-00.ll
+++ llvm/test/Transforms/Coroutines/coro-split-eh-00.ll
@@ -67,6 +67,9 @@
 ; CHECK-NEXT:  %lpval = landingpad { i8*, i32 }
 ; CHECK-NEXT: cleanup
 ; CHECK-NEXT:  call void @print(i32 2)
+; Checks that the coroutine would be marked as done if it exits in unwinding path.
+; CHECK-NEXT:  %[[RESUME_ADDR:.+]] = getelementptr inbounds %[[FRAME_TY:.+]], %[[FRAME_TY]]* %FramePtr, i32 0, i32 0
+; CHECK-NEXT:  store void (%[[FRAME_TY]]*)* null, void (%[[FRAME_TY]]*)** %[[RESUME_ADDR]], align 8
 ; CHECK-NEXT:  resume { i8*, i32 } %lpval
 
 declare i8* @llvm.coro.free(token, i8*)
Index: llvm/lib/Transforms/Coroutines/CoroSplit.cpp
===
--- llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ llvm/lib/Tra

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

I don't think the updated logic is correct. For example, in the case of Fuchsia 
we don't want to take `CLANG_DEFAULT_LINKER` into account, that's why we 
override `getDefaultLinker`. I assume it's the same for other toolchains that 
override `getDefaultLinker`.

The issue that affected AMDGPU bots is that `StringRef UseLinker = A ? 
A->getValue() : ""` is now going to evaluate to an empty string unless 
`-fuse-ld=` is set, and we'll take the `UseLinker.empty() || UseLinker == "ld"` 
branch, where `const char *DefaultLinker = getDefaultLinker()` is going to 
return `"lld"` because AMDGPU bot sets `-DCLANG_DEFAULT_LINKER=lld` in their 
build. That's not a valid name, the valid name is `ld.lld`. Prior to your 
patch, we would take the `!llvm::sys::path::is_absolute(UseLinker)` branch and 
construct the correct linker name by appending `CLANG_DEFAULT_LINKER` to 
'"ld."`.

I'd argue that your originally patch is actually the behavior we want. Rather, 
we shouldn't consider `-DCLANG_DEFAULT_LINKER=lld` as a valid value. Instead 
AMDGPU bot should use `-DCLANG_DEFAULT_LINKER=ld.lld`. Even better would be to 
introduce a second CMake variable so we can control the default value for 
`-fuse-ld=` and for `--ld-path` options separately. Right now it's not clear 
which of the two is controlled by `-DCLANG_DEFAULT_LINKER=` (that's because 
`-DCLANG_DEFAULT_LINKER=` actually predates the `--ld-path` option).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115045

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

[PATCH] D115219: [C++20] [Coroutines] Mark coroutine done if unhandled_exception throws

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D115219#3175582 , @rjmccall wrote:

> I agree that you shouldn't call `suspend`, but doesn't `coro.end` have the 
> behavior of marking the coroutine done?  Should we just be calling `coro.end` 
> on this path?

@rjmccall great insight! `coro.end` wouldn't marking the coroutine done 
previously. But its place is perfect to do so. I have added the codes to mark 
the coroutine as done for `coro.end` in the unwind path. And I have checked the 
behavior. The exception happened earlier wouldn't run into the path of coro.end 
exists. So the behavior is satisfying. The only defect might be that the 
behavior is C++'s semantic. Although, there might be a problem if there is 
another language also uses switch-resumed ABI one day. I think the current 
approach is much better at least it has and would generate less code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115219

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

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In D115045#3175648 , @phosek wrote:

> I don't think the updated logic is correct. For example, in the case of 
> Fuchsia we don't want to take `CLANG_DEFAULT_LINKER` into account, that's why 
> we override `getDefaultLinker`. I assume it's the same for other toolchains 
> that override `getDefaultLinker`.
>
> The issue that affected AMDGPU bots is that `StringRef UseLinker = A ? 
> A->getValue() : ""` is now going to evaluate to an empty string unless 
> `-fuse-ld=` is set, and we'll take the `UseLinker.empty() || UseLinker == 
> "ld"` branch, where `const char *DefaultLinker = getDefaultLinker()` is going 
> to return `"lld"` because AMDGPU bot sets `-DCLANG_DEFAULT_LINKER=lld` in 
> their build. That's not a valid name, the valid name is `ld.lld`. Prior to 
> your patch, we would take the `!llvm::sys::path::is_absolute(UseLinker)` 
> branch and construct the correct linker name by appending 
> `CLANG_DEFAULT_LINKER` to '"ld."`.
>
> I'd argue that your originally patch is actually the behavior we want. 
> Rather, we shouldn't consider `-DCLANG_DEFAULT_LINKER=lld` as a valid value. 
> Instead AMDGPU bot should use `-DCLANG_DEFAULT_LINKER=ld.lld`. Even better 
> would be to introduce a second CMake variable so we can control the default 
> value for `-fuse-ld=` and for `--ld-path` options separately. Right now it's 
> not clear which of the two is controlled by `-DCLANG_DEFAULT_LINKER=` (that's 
> because `-DCLANG_DEFAULT_LINKER=` actually predates the `--ld-path` option).

I forgot to mention, I don't think your updated logic is actually going to 
address the failure we've seen on the AMDGPU bot because the test failure 
they're seeing is not in the AMDGPU target, it's in the 
`x86_64-unknown-linux-gnu` target and that's still going to follow the control 
flow I described above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115045

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

[PATCH] D115225: [X86][MS-InlineAsm] Make the constraint *m to be simple place holder

[Phoebe Wang via Phabricator via cfe-commits]

pengfei created this revision.
pengfei added reviewers: skan, xiangzhangllvm, craig.topper, coby.
Herald added a subscriber: hiraditya.
pengfei requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

D113096  solved the "undefined reference to 
xxx" issue by adding
constraint *m for the global var. But it has strong side effect due to
the symbol in the assembly being replace with constraint variable.
This leads to some lowering fails. https://godbolt.org/z/h3nWoerPe

This patch fix the problem by use the constraint *m as place holder
rather than real constraint. It has negligible effect for the existing
code generation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115225

Files:
  clang/test/CodeGen/X86/ms_fmul.c
  clang/test/CodeGen/ms-inline-asm-functions.c
  clang/test/CodeGen/ms-inline-asm-static-variable.c
  clang/test/CodeGen/ms-inline-asm-variables.c
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/test/CodeGen/X86/ms-inline-asm-array.ll

Index: llvm/test/CodeGen/X86/ms-inline-asm-array.ll
===
--- llvm/test/CodeGen/X86/ms-inline-asm-array.ll
+++ llvm/test/CodeGen/X86/ms-inline-asm-array.ll
@@ -5,7 +5,7 @@
 ; CHECK: movl%edx, arr(,%rdx,4)
 define dso_local i32 @main() #0 {
 entry:
-  call void asm sideeffect inteldialect "mov dword ptr $0[rdx * $$4],edx", "=*m,~{dirflag},~{fpsr},~{flags}"([10 x i32]* @arr) #1, !srcloc !4
+  call void asm sideeffect inteldialect "mov dword ptr arr[rdx * $$4],edx", "=*m,~{dirflag},~{fpsr},~{flags}"([10 x i32]* @arr) #1, !srcloc !4
   ret i32 0
 }
 
Index: llvm/lib/Target/X86/AsmParser/X86Operand.h
===
--- llvm/lib/Target/X86/AsmParser/X86Operand.h
+++ llvm/lib/Target/X86/AsmParser/X86Operand.h
@@ -285,6 +285,13 @@
 
   bool isOffsetOfLocal() const override { return isImm() && Imm.LocalRef; }
 
+  bool isMemPlaceholder(const MCInstrDesc &Desc) const override {
+// Add more restrictions to avoid the use of global symbols. This helps
+// with reducing the code size.
+return !Desc.isRematerializable() && !Desc.isCall() && isMem() &&
+   !Mem.BaseReg && !Mem.IndexReg;
+  }
+
   bool needAddressOf() const override { return AddressOf; }
 
   bool isMem() const override { return Kind == Memory; }
Index: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
===
--- llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
@@ -2551,8 +2551,6 @@
   StringRef ErrMsg;
   unsigned BaseReg = SM.getBaseReg();
   unsigned IndexReg = SM.getIndexReg();
-  if (IndexReg && BaseReg == X86::RIP)
-BaseReg = 0;
   unsigned Scale = SM.getScale();
   if (!PtrInOperand)
 Size = SM.getElementSize() << 3;
Index: llvm/lib/MC/MCParser/AsmParser.cpp
===
--- llvm/lib/MC/MCParser/AsmParser.cpp
+++ llvm/lib/MC/MCParser/AsmParser.cpp
@@ -6012,12 +6012,13 @@
 
   bool isOutput = (i == 1) && Desc.mayStore();
   SMLoc Start = SMLoc::getFromPointer(SymName.data());
+  int64_t Size = Operand.isMemPlaceholder(Desc) ? 0 : SymName.size();
   if (isOutput) {
 ++InputIdx;
 OutputDecls.push_back(OpDecl);
 OutputDeclsAddressOf.push_back(Operand.needAddressOf());
 OutputConstraints.push_back(("=" + Constraint).str());
-AsmStrRewrites.emplace_back(AOK_Output, Start, SymName.size());
+AsmStrRewrites.emplace_back(AOK_Output, Start, Size);
   } else {
 InputDecls.push_back(OpDecl);
 InputDeclsAddressOf.push_back(Operand.needAddressOf());
@@ -6025,7 +6026,7 @@
 if (Desc.OpInfo[i - 1].isBranchTarget())
   AsmStrRewrites.emplace_back(AOK_CallInput, Start, SymName.size());
 else
-  AsmStrRewrites.emplace_back(AOK_Input, Start, SymName.size());
+  AsmStrRewrites.emplace_back(AOK_Input, Start, Size);
   }
 }
 
@@ -6140,13 +6141,17 @@
   OS << Ctx.getAsmInfo()->getPrivateLabelPrefix() << AR.Label;
   break;
 case AOK_Input:
-  OS << '$' << InputIdx++;
+  if (AR.Len)
+OS << '$' << InputIdx;
+  ++InputIdx;
   break;
 case AOK_CallInput:
   OS << "${" << InputIdx++ << ":P}";
   break;
 case AOK_Output:
-  OS << '$' << OutputIdx++;
+  if (AR.Len)
+OS << '$' << OutputIdx;
+  ++OutputIdx;
   break;
 case AOK_SizeDirective:
   switch (AR.Val) {
Index: llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
===
--- llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
+++ llvm/include/llvm/MC/

[clang] 978431e - [Analyzer] SValBuilder: Simlify a SymExpr to the absolute simplest form

[Gabor Marton via cfe-commits]

Author: Gabor Marton
Date: 2021-12-07T10:02:32+01:00
New Revision: 978431e80b6155878d8d5b4fc7a67c90af317c01

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

LOG: [Analyzer] SValBuilder: Simlify a SymExpr to the absolute simplest form

Move the SymExpr simplification fixpoint logic into SValBuilder.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp 
b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
index 23c67c64f9752..74403a160b8e0 100644
--- a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -2191,42 +2191,6 @@ LLVM_NODISCARD ProgramStateRef reAssume(ProgramStateRef 
State,
  Constraint->getMaxValue(), true);
 }
 
-// Simplify the given symbol with the help of the SValBuilder. In
-// SValBuilder::symplifySval, we traverse the symbol tree and query the
-// constraint values for the sub-trees and if a value is a constant we do the
-// constant folding. Compound symbols might collapse to simpler symbol tree
-// that is still possible to further simplify. Thus, we do the simplification 
on
-// a new symbol tree until we reach the simplest form, i.e. the fixpoint.
-//
-// Consider the following symbol `(b * b) * b * b` which has this tree:
-//   *
-//  / \
-// *   b
-///  \
-//   /b
-// (b * b)
-// Now, if the `b * b == 1` new constraint is added then during the first
-// iteration we have the following transformations:
-//   *  *
-//  / \/ \
-// *   b -->  b   b
-///  \
-//   /b
-//  1
-// We need another iteration to reach the final result `1`.
-LLVM_NODISCARD
-static SVal simplifyUntilFixpoint(SValBuilder &SVB, ProgramStateRef State,
-  const SymbolRef Sym) {
-  SVal Val = SVB.makeSymbolVal(Sym);
-  SVal SimplifiedVal = SVB.simplifySVal(State, Val);
-  // Do the simplification until we can.
-  while (SimplifiedVal != Val) {
-Val = SimplifiedVal;
-SimplifiedVal = SVB.simplifySVal(State, Val);
-  }
-  return SimplifiedVal;
-}
-
 // Iterate over all symbols and try to simplify them. Once a symbol is
 // simplified then we check if we can merge the simplified symbol's equivalence
 // class to this class. This way, we simplify not just the symbols but the
@@ -2238,8 +2202,7 @@ EquivalenceClass::simplify(SValBuilder &SVB, 
RangeSet::Factory &F,
   SymbolSet ClassMembers = Class.getClassMembers(State);
   for (const SymbolRef &MemberSym : ClassMembers) {
 
-const SVal SimplifiedMemberVal =
-simplifyUntilFixpoint(SVB, State, MemberSym);
+const SVal SimplifiedMemberVal = simplifyToSVal(State, MemberSym);
 const SymbolRef SimplifiedMemberSym = SimplifiedMemberVal.getAsSymbol();
 
 // The symbol is collapsed to a constant, check if the current State is

diff  --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp 
b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 4ca35dd06ae58..dad8a7b3caae0 100644
--- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -21,6 +21,35 @@ using namespace ento;
 
 namespace {
 class SimpleSValBuilder : public SValBuilder {
+
+  // With one `simplifySValOnce` call, a compound symbols might collapse to
+  // simpler symbol tree that is still possible to further simplify. Thus, we
+  // do the simplification on a new symbol tree until we reach the simplest
+  // form, i.e. the fixpoint.
+  // Consider the following symbol `(b * b) * b * b` which has this tree:
+  //   *
+  //  / \
+  // *   b
+  ///  \
+  //   /b
+  // (b * b)
+  // Now, if the `b * b == 1` new constraint is added then during the first
+  // iteration we have the following transformations:
+  //   *  *
+  //  / \/ \
+  // *   b -->  b   b
+  ///  \
+  //   /b
+  //  1
+  // We need another iteration to reach the final result `1`.
+  SVal simplifyUntilFixpoint(ProgramStateRef State, SVal Val);
+
+  // Recursively descends into symbolic expressions and replaces symbols
+  // with their known values (in the sense of the getKnownValue() method).
+  // We traverse the symbol tree and query the constraint values for the
+  // sub-trees and if a value is a constant we do the constant folding.
+  SVal simplifySValOnce(ProgramStateRef State, SVal V);
+
 public:
   SimpleSValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
 ProgramStateMa

[PATCH] D114938: [Analyzer][NFCi] SValBuilder: Simlify a SymExpr to the absolute simplest form

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG978431e80b61: [Analyzer] SValBuilder: Simlify a SymExpr to 
the absolute simplest form (authored by martong).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114938

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp

Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -21,6 +21,35 @@
 
 namespace {
 class SimpleSValBuilder : public SValBuilder {
+
+  // With one `simplifySValOnce` call, a compound symbols might collapse to
+  // simpler symbol tree that is still possible to further simplify. Thus, we
+  // do the simplification on a new symbol tree until we reach the simplest
+  // form, i.e. the fixpoint.
+  // Consider the following symbol `(b * b) * b * b` which has this tree:
+  //   *
+  //  / \
+  // *   b
+  ///  \
+  //   /b
+  // (b * b)
+  // Now, if the `b * b == 1` new constraint is added then during the first
+  // iteration we have the following transformations:
+  //   *  *
+  //  / \/ \
+  // *   b -->  b   b
+  ///  \
+  //   /b
+  //  1
+  // We need another iteration to reach the final result `1`.
+  SVal simplifyUntilFixpoint(ProgramStateRef State, SVal Val);
+
+  // Recursively descends into symbolic expressions and replaces symbols
+  // with their known values (in the sense of the getKnownValue() method).
+  // We traverse the symbol tree and query the constraint values for the
+  // sub-trees and if a value is a constant we do the constant folding.
+  SVal simplifySValOnce(ProgramStateRef State, SVal V);
+
 public:
   SimpleSValBuilder(llvm::BumpPtrAllocator &alloc, ASTContext &context,
 ProgramStateManager &stateMgr)
@@ -40,8 +69,6 @@
   ///  (integer) value, that value is returned. Otherwise, returns NULL.
   const llvm::APSInt *getKnownValue(ProgramStateRef state, SVal V) override;
 
-  /// Recursively descends into symbolic expressions and replaces symbols
-  /// with their known values (in the sense of the getKnownValue() method).
   SVal simplifySVal(ProgramStateRef State, SVal V) override;
 
   SVal MakeSymIntVal(const SymExpr *LHS, BinaryOperator::Opcode op,
@@ -1105,7 +1132,20 @@
   return nullptr;
 }
 
+SVal SimpleSValBuilder::simplifyUntilFixpoint(ProgramStateRef State, SVal Val) {
+  SVal SimplifiedVal = simplifySValOnce(State, Val);
+  while (SimplifiedVal != Val) {
+Val = SimplifiedVal;
+SimplifiedVal = simplifySValOnce(State, Val);
+  }
+  return SimplifiedVal;
+}
+
 SVal SimpleSValBuilder::simplifySVal(ProgramStateRef State, SVal V) {
+  return simplifyUntilFixpoint(State, V);
+}
+
+SVal SimpleSValBuilder::simplifySValOnce(ProgramStateRef State, SVal V) {
   // For now, this function tries to constant-fold symbols inside a
   // nonloc::SymbolVal, and does nothing else. More simplifications should
   // be possible, such as constant-folding an index in an ElementRegion.
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -2191,42 +2191,6 @@
  Constraint->getMaxValue(), true);
 }
 
-// Simplify the given symbol with the help of the SValBuilder. In
-// SValBuilder::symplifySval, we traverse the symbol tree and query the
-// constraint values for the sub-trees and if a value is a constant we do the
-// constant folding. Compound symbols might collapse to simpler symbol tree
-// that is still possible to further simplify. Thus, we do the simplification on
-// a new symbol tree until we reach the simplest form, i.e. the fixpoint.
-//
-// Consider the following symbol `(b * b) * b * b` which has this tree:
-//   *
-//  / \
-// *   b
-///  \
-//   /b
-// (b * b)
-// Now, if the `b * b == 1` new constraint is added then during the first
-// iteration we have the following transformations:
-//   *  *
-//  / \/ \
-// *   b -->  b   b
-///  \
-//   /b
-//  1
-// We need another iteration to reach the final result `1`.
-LLVM_NODISCARD
-static SVal simplifyUntilFixpoint(SValBuilder &SVB, ProgramStateRef State,
-  const SymbolRef Sym) {
-  SVal Val = SVB.makeSymbolVal(Sym);
-  SVal SimplifiedVal = SVB.simplifySVal(State, Val);
-  // Do the simplification until we can.
-  while (SimplifiedVal != Val) {
-Val = SimplifiedVal;
-SimplifiedVal = SVB.simplifySVal(State, Val);
-  }
-  return Simplif

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D115045#3175648 , @phosek wrote:

> Rather, we shouldn't consider `-DCLANG_DEFAULT_LINKER=lld` as a valid value. 
> Instead AMDGPU bot should use `-DCLANG_DEFAULT_LINKER=ld.lld`.

I think this would be a kinda disruptive change; I (and others) specify it as 
`-DCLANG_DEFAULT_LINKER=lld` so far: 
https://github.com/mstorsjo/llvm-mingw/blob/master/build-llvm.sh#L162 And other 
projects too: 
https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-clang/PKGBUILD#L204


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115045

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

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

In D115045#3175760 , @mstorsjo wrote:

> In D115045#3175648 , @phosek wrote:
>
>> Rather, we shouldn't consider `-DCLANG_DEFAULT_LINKER=lld` as a valid value. 
>> Instead AMDGPU bot should use `-DCLANG_DEFAULT_LINKER=ld.lld`.
>
> I think this would be a kinda disruptive change; I (and others) specify it as 
> `-DCLANG_DEFAULT_LINKER=lld` so far: 
> https://github.com/mstorsjo/llvm-mingw/blob/master/build-llvm.sh#L162 And 
> other projects too: 
> https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-clang/PKGBUILD#L204

In that case, perhaps we should consider `-DCLANG_DEFAULT_LINKER=` to set the 
default value for `-fuse-ld` only and have another variable for `--ld-path`? 
Open question is if `getDefaultLinker()` should return the name of the linker 
or the filename. I'd argue for the former which would be consistent with 
`-DCLANG_DEFAULT_LINKER=`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115045

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

[clang] 698584f - [IR] Remove unbounded as possible value for vscale_range minimum

[Cullen Rhodes via cfe-commits]

Author: Cullen Rhodes
Date: 2021-12-07T09:52:21Z
New Revision: 698584f89b8f8bd7f6c2d2cd61efb5548857da2a

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

LOG: [IR] Remove unbounded as possible value for vscale_range minimum

The default for min is changed to 1. The behaviour of -mvscale-{min,max}
in Clang is also changed such that 16 is the max vscale when targeting
SVE and no max is specified.

Reviewed By: sdesmalen, paulwalker-arm

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

Added: 
clang/test/Frontend/aarch64-vscale-min.c

Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/include/clang/Driver/Options.td
clang/lib/Basic/Targets/AArch64.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c
llvm/docs/LangRef.rst
llvm/include/llvm/IR/Attributes.h
llvm/lib/IR/Attributes.cpp
llvm/lib/IR/Verifier.cpp
llvm/test/Analysis/CostModel/AArch64/sve-gather.ll
llvm/test/Analysis/CostModel/AArch64/sve-scatter.ll
llvm/test/Bitcode/attributes.ll
llvm/test/Transforms/InstCombine/icmp-vscale.ll
llvm/test/Transforms/InstCombine/vscale_sext_and_zext.ll
llvm/test/Transforms/InstCombine/vscale_trunc.ll
llvm/test/Transforms/LoopVectorize/AArch64/first-order-recurrence.ll
llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll
llvm/test/Transforms/LoopVectorize/AArch64/scalable-vectorization.ll
llvm/test/Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll
llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
llvm/test/Verifier/vscale_range.ll

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index 2f50918b527be..3638d07cc3b35 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -608,6 +608,8 @@ def err_cc1_round_trip_ok_then_fail : Error<
   "generated arguments parse failed in round-trip">;
 def err_cc1_round_trip_mismatch : Error<
   "generated arguments do not match in round-trip">;
+def err_cc1_unbounded_vscale_min : Error<
+  "minimum vscale must be an unsigned integer greater than 0">;
 
 def err_drv_ssp_missing_offset_argument : Error<
   "'%0' is used without '-mstack-protector-guard-offset', and there is no 
default">;

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 9d5f021102a5e..e9c3ece3d1634 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3357,8 +3357,7 @@ def msve_vector_bits_EQ : Joined<["-"], 
"msve-vector-bits=">, Group,
+  HelpText<"Specify the vscale minimum. Defaults to \"1\". (AArch64 only)">,
   MarshallingInfoInt>;
 def mvscale_max_EQ : Joined<["-"], "mvscale-max=">,
   Group, Flags<[NoXarchOption,CC1Option]>,

diff  --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index 4d403ae1809d4..0212889811486 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -474,10 +474,12 @@ ArrayRef 
AArch64TargetInfo::getTargetBuiltins() const {
 Optional>
 AArch64TargetInfo::getVScaleRange(const LangOptions &LangOpts) const {
   if (LangOpts.VScaleMin || LangOpts.VScaleMax)
-return std::pair(LangOpts.VScaleMin,
- LangOpts.VScaleMax);
+return std::pair(
+LangOpts.VScaleMin ? LangOpts.VScaleMin : 1, LangOpts.VScaleMax);
+
   if (hasFeature("sve"))
-return std::pair(0, 16);
+return std::pair(1, 16);
+
   return None;
 }
 

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index c104a6f40e20f..f760eb284e75f 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4123,6 +4123,13 @@ bool CompilerInvocation::ParseLangArgs(LangOptions 
&Opts, ArgList &Args,
 {std::string(Split.first), std::string(Split.second)});
   }
 
+  // Error if -mvscale-min is unbounded.
+  if (Arg *A = Args.getLastArg(options::OPT_mvscale_min_EQ)) {
+unsigned VScaleMin;
+if (StringRef(A->getValue()).getAsInteger(10, VScaleMin) || VScaleMin == 0)
+  Diags.Report(diag::err_cc1_unbounded_vscale_min);
+  }
+
   return Diags.getNumErrors() == NumErrorsBefore;
 }
 

diff  --git a/clang/test/CodeGen/arm-sve-vector-bits-vsca

[PATCH] D113294: [IR] Remove unbounded as possible value for vscale_range minimum

[Cullen Rhodes 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 rG698584f89b8f: [IR] Remove unbounded as possible value for 
vscale_range minimum (authored by c-rhodes).

Changed prior to commit:
  https://reviews.llvm.org/D113294?vs=392002&id=392312#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113294

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c
  clang/test/Frontend/aarch64-vscale-min.c
  llvm/docs/LangRef.rst
  llvm/include/llvm/IR/Attributes.h
  llvm/lib/IR/Attributes.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Analysis/CostModel/AArch64/sve-gather.ll
  llvm/test/Analysis/CostModel/AArch64/sve-scatter.ll
  llvm/test/Bitcode/attributes.ll
  llvm/test/Transforms/InstCombine/icmp-vscale.ll
  llvm/test/Transforms/InstCombine/vscale_sext_and_zext.ll
  llvm/test/Transforms/InstCombine/vscale_trunc.ll
  llvm/test/Transforms/LoopVectorize/AArch64/first-order-recurrence.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vectorization.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
  llvm/test/Verifier/vscale_range.ll

Index: llvm/test/Verifier/vscale_range.ll
===
--- llvm/test/Verifier/vscale_range.ll
+++ llvm/test/Verifier/vscale_range.ll
@@ -1,4 +1,7 @@
 ; RUN: not llvm-as %s -o /dev/null 2>&1 | FileCheck %s
 
+; CHECK: 'vscale_range' minimum must be greater than 0
+declare i8* @a(i32*) vscale_range(0, 1)
+
 ; CHECK: 'vscale_range' minimum cannot be greater than maximum
 declare i8* @b(i32*) vscale_range(8, 1)
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
@@ -210,7 +210,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
@@ -53,7 +53,7 @@
   ret double %add
 }
 
-attributes #0 = { "target-features"="+sve" vscale_range(0, 16) }
+attributes #0 = { "target-features"="+sve" vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1}
 !1 = !{!"llvm.loop.vectorize.scalable.enable", i1 true}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
@@ -90,7 +90,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
@@ -59,7 +59,7 @@
   ret void
 }
 
-attributes #0 = { "target-features"="+neon,+sve" vscale_range(0, 16) }
+attributes #0 = { "target-features"="+neon,+sve" vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
@@ -153,7 +153,7 @@
   ret void
 }
 
-attributes #0 = { vscale_range(0, 16) }
+attributes #0 = { vscale_range(1, 16) }
 
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
===

[PATCH] D114394: Compile-time computation of string attribute hashes

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

> Frontends not written in C++ will always be going through the 
> AttributeKey::get() API, which will be slower than the initial state (I think 
> -- we still have to calculate the hash dynamically, but now we also need to 
> intern an AttributeKey by performing an actual hash table lookup.)

I ran the following benchmark

  #include "llvm-c/Core.h"
  
  int main() {
LLVMContextRef ctx = LLVMContextCreate();
LLVMAttributeRef Key = LLVMCreateStringAttribute(ctx, "mykey", 5, 
"myvalue", 7);
unsigned size;
for(int i = 0; i <1; ++i)
for(int j = 0; j <1; ++j)
  LLVMGetStringAttributeValue(Key, &size);
return size;
  }

linking dynamically with libLLVMCore.so , the compiler doesn't optimize away 
the loop content.

Before the patch: 0.39s , ~ 2.5G instructions
With the patch: 0.30s, ~2.3G instructions

Why is that so? A side effect of the patch is that srings are memoized into a 
StringPool, so less allocation etc as soon as we lookup the attribute more than 
once.


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

https://reviews.llvm.org/D114394

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

[PATCH] D115228: [Clang] Do not check if we are in a discared context in non-immediate contexts

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin created this revision.
cor3ntin requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This fixes in a regression introduced by 6eeda06c1 
.

When deducing the return type of nested function calls, only the
return type of the outermost expression should be ignored.

Instead of assuming all contextes nested in a discared statements
are themselves discarded, only assume that in immediate contexts.

Similarly, only consider contextes immediately in an immediate or
discarded statement as being themselves immediate.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115228

Files:
  clang/include/clang/Sema/Sema.h
  clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp


Index: clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
===
--- clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
+++ clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
@@ -159,4 +159,19 @@
   surprise: {}
   }
 }
+
+namespace deduced_return_type_in_discareded_statement {
+
+  template 
+  auto a(const T& t) {
+return t;
+  }
+
+  void f() {
+  if constexpr (false) {
+  a(a(0));
+  }
+  }
+}
+
 #endif
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1324,12 +1324,12 @@
 
 bool isImmediateFunctionContext() const {
   return Context == ExpressionEvaluationContext::ImmediateFunctionContext 
||
- InImmediateFunctionContext;
+ (Context == ExpressionEvaluationContext::DiscardedStatement && 
InImmediateFunctionContext);
 }
 
 bool isDiscardedStatementContext() const {
-  return Context == ExpressionEvaluationContext::DiscardedStatement ||
- InDiscardedStatement;
+  return Context  == ExpressionEvaluationContext::DiscardedStatement ||
+ (Context == ExpressionEvaluationContext::ImmediateFunctionContext 
&& InDiscardedStatement);
 }
   };
 


Index: clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
===
--- clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
+++ clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
@@ -159,4 +159,19 @@
   surprise: {}
   }
 }
+
+namespace deduced_return_type_in_discareded_statement {
+
+  template 
+  auto a(const T& t) {
+return t;
+  }
+
+  void f() {
+  if constexpr (false) {
+  a(a(0));
+  }
+  }
+}
+
 #endif
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1324,12 +1324,12 @@
 
 bool isImmediateFunctionContext() const {
   return Context == ExpressionEvaluationContext::ImmediateFunctionContext ||
- InImmediateFunctionContext;
+ (Context == ExpressionEvaluationContext::DiscardedStatement && InImmediateFunctionContext);
 }
 
 bool isDiscardedStatementContext() const {
-  return Context == ExpressionEvaluationContext::DiscardedStatement ||
- InDiscardedStatement;
+  return Context  == ExpressionEvaluationContext::DiscardedStatement ||
+ (Context == ExpressionEvaluationContext::ImmediateFunctionContext && InDiscardedStatement);
 }
   };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115228: [Clang] Do not check if we are in a discared context in non-immediate contexts

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 392319.
cor3ntin added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115228

Files:
  clang/include/clang/Sema/Sema.h
  clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp


Index: clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
===
--- clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
+++ clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
@@ -159,4 +159,19 @@
   surprise: {}
   }
 }
+
+namespace deduced_return_type_in_discareded_statement {
+
+template 
+auto a(const T &t) {
+  return t;
+}
+
+void f() {
+  if constexpr (false) {
+a(a(0));
+  }
+}
+} // namespace deduced_return_type_in_discareded_statement
+
 #endif
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1324,12 +1324,15 @@
 
 bool isImmediateFunctionContext() const {
   return Context == ExpressionEvaluationContext::ImmediateFunctionContext 
||
- InImmediateFunctionContext;
+ (Context == ExpressionEvaluationContext::DiscardedStatement &&
+  InImmediateFunctionContext);
 }
 
 bool isDiscardedStatementContext() const {
   return Context == ExpressionEvaluationContext::DiscardedStatement ||
- InDiscardedStatement;
+ (Context ==
+  ExpressionEvaluationContext::ImmediateFunctionContext &&
+  InDiscardedStatement);
 }
   };
 


Index: clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
===
--- clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
+++ clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
@@ -159,4 +159,19 @@
   surprise: {}
   }
 }
+
+namespace deduced_return_type_in_discareded_statement {
+
+template 
+auto a(const T &t) {
+  return t;
+}
+
+void f() {
+  if constexpr (false) {
+a(a(0));
+  }
+}
+} // namespace deduced_return_type_in_discareded_statement
+
 #endif
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1324,12 +1324,15 @@
 
 bool isImmediateFunctionContext() const {
   return Context == ExpressionEvaluationContext::ImmediateFunctionContext ||
- InImmediateFunctionContext;
+ (Context == ExpressionEvaluationContext::DiscardedStatement &&
+  InImmediateFunctionContext);
 }
 
 bool isDiscardedStatementContext() const {
   return Context == ExpressionEvaluationContext::DiscardedStatement ||
- InDiscardedStatement;
+ (Context ==
+  ExpressionEvaluationContext::ImmediateFunctionContext &&
+  InDiscardedStatement);
 }
   };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115222: [Coroutines] Remove unused coroutine builtin/intrinsics llvm.coro.param (NFC-ish)

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D115222#3175577 , @rjmccall wrote:

> I imagine Gor hoped for this optimization to be implemented someday, assuming 
> it's still allowed by the language specification.  Maybe you would be 
> interested in pursuing that?  It sounds like it's really just (1) emitting 
> the intrinsic in the frontend and then (2) checking if the copied parameter 
> variable is actually used after reaching a suspend point, other than in ways 
> that wouldn't happen if the intrinsic returned false.

To my knowledge of language specification, the optimization is not allowed now. 
Both the construction and destruction is required now. But I am indeed 
interested to implement this. Since I know that many implementation of C++ 
features comes before they become standard. So I am wondering if this could be 
a proposal demo to the language specification : )
BTW, I am still prefer to remove the intrinsic to avoid misunderstanding. Or is 
It sufficient to add a TODO/FIXME marker in the Documentation? How do you think 
about it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115222

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

[PATCH] D115231: [Clang] Add __builtin_reduce_xor

[Jun Zhang via Phabricator via cfe-commits]

junaire created this revision.
junaire requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch implements __builtin_reduce_xor as specified in D111529 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115231

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-reduction-math.c
  clang/test/Sema/builtins-reduction-math.c


Index: clang/test/Sema/builtins-reduction-math.c
===
--- clang/test/Sema/builtins-reduction-math.c
+++ clang/test/Sema/builtins-reduction-math.c
@@ -35,3 +35,17 @@
   i = __builtin_reduce_min(i);
   // expected-error@-1 {{1st argument must be a vector type (was 'int')}}
 }
+
+void test_builtin_reduce_xor(int i, float4 v, int3 iv) {
+  struct Foo s = __builtin_reduce_xor(iv);
+  // expected-error@-1 {{initializing 'struct Foo' with an expression of 
incompatible type 'int'}}
+
+  i = __builtin_reduce_xor(v, v);
+  // expected-error@-1 {{too many arguments to function call, expected 1, have 
2}}
+
+  i = __builtin_reduce_xor();
+  // expected-error@-1 {{too few arguments to function call, expected 1, have 
0}}
+
+  i = __builtin_reduce_xor(i);
+  // expected-error@-1 {{1st argument must be a vector type (was 'int')}}
+}
Index: clang/test/CodeGen/builtins-reduction-math.c
===
--- clang/test/CodeGen/builtins-reduction-math.c
+++ clang/test/CodeGen/builtins-reduction-math.c
@@ -57,3 +57,14 @@
   const si8 cvi1 = vi1;
   unsigned long long r5 = __builtin_reduce_min(cvi1);
 }
+
+void test_builtin_reduce_xor(float4 vf1, si8 vi1, u4 vu1) {
+
+  // CHECK:  [[VI1:%.+]] = load <8 x i16>, <8 x i16>* %vi1.addr, align 16
+  // CHECK-NEXT: call i16 @llvm.vector.reduce.xor.v8i16(<8 x i16> [[VI1]])
+  short r2 = __builtin_reduce_xor(vi1);
+
+  // CHECK:  [[VU1:%.+]] = load <4 x i32>, <4 x i32>* %vu1.addr, align 16
+  // CHECK-NEXT: call i32 @llvm.vector.reduce.xor.v4i32(<4 x i32> [[VU1]])
+  unsigned r3 = __builtin_reduce_xor(vu1);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2109,6 +2109,7 @@
 break;
   case Builtin::BI__builtin_reduce_max:
   case Builtin::BI__builtin_reduce_min:
+  case Builtin::BI__builtin_reduce_xor:
 if (SemaBuiltinReduceMath(TheCall))
   return ExprError();
 break;
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -3204,6 +3204,13 @@
 return RValue::get(Result);
   }
 
+  case Builtin::BI__builtin_reduce_xor: {
+Value *Op0 = EmitScalarExpr(E->getArg(0));
+Value *Result = Builder.CreateUnaryIntrinsic(
+llvm::Intrinsic::vector_reduce_xor, Op0, nullptr, "rdx.xor");
+return RValue::get(Result);
+  }
+
   case Builtin::BI__builtin_matrix_transpose: {
 const auto *MatrixTy = 
E->getArg(0)->getType()->getAs();
 Value *MatValue = EmitScalarExpr(E->getArg(0));
Index: clang/include/clang/Basic/Builtins.def
===
--- clang/include/clang/Basic/Builtins.def
+++ clang/include/clang/Basic/Builtins.def
@@ -648,6 +648,7 @@
 BUILTIN(__builtin_elementwise_min, "v.", "nct")
 BUILTIN(__builtin_reduce_max, "v.", "nct")
 BUILTIN(__builtin_reduce_min, "v.", "nct")
+BUILTIN(__builtin_reduce_xor, "v.", "nct")
 
 BUILTIN(__builtin_matrix_transpose, "v.", "nFt")
 BUILTIN(__builtin_matrix_column_major_load, "v.", "nFt")


Index: clang/test/Sema/builtins-reduction-math.c
===
--- clang/test/Sema/builtins-reduction-math.c
+++ clang/test/Sema/builtins-reduction-math.c
@@ -35,3 +35,17 @@
   i = __builtin_reduce_min(i);
   // expected-error@-1 {{1st argument must be a vector type (was 'int')}}
 }
+
+void test_builtin_reduce_xor(int i, float4 v, int3 iv) {
+  struct Foo s = __builtin_reduce_xor(iv);
+  // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'int'}}
+
+  i = __builtin_reduce_xor(v, v);
+  // expected-error@-1 {{too many arguments to function call, expected 1, have 2}}
+
+  i = __builtin_reduce_xor();
+  // expected-error@-1 {{too few arguments to function call, expected 1, have 0}}
+
+  i = __builtin_reduce_xor(i);
+  // expected-error@-1 {{1st argument must be a vector type (was 'int')}}
+}
Index: clang/test/CodeGen/builtins-reduction-math.c
===
--- clang/test/CodeGen/builtins-reduction-math.c
+++ clang/test/CodeGen/builtins-reduction-math.c
@@ -57,3 +57,14 @@
   const si8 cvi1 = vi1;
   unsigned long long r5 = __builtin_reduce_

[PATCH] D115045: [Clang] Ignore CLANG_DEFAULT_LINKER for custom-linker toolchains

[Simon Moll via Phabricator via cfe-commits]

simoll added a comment.

In D115045#3175648 , @phosek wrote:

> I don't think the updated logic is correct. For example, in the case of 
> Fuchsia we don't want to take `CLANG_DEFAULT_LINKER` into account, that's why 
> we override `getDefaultLinker`. I assume it's the same for other toolchains 
> that override `getDefaultLinker`.

You do. With the logic as before this patch, if `CLANG_DEFAULT_LINKER` is an 
absolute path,  your build overrides the default linker on Fuchsia (and 
elsewhere).

> The issue that affected AMDGPU bots is that `StringRef UseLinker = A ? 
> A->getValue() : ""` is now going to evaluate to an empty string unless 
> `-fuse-ld=` is set, and we'll take the `UseLinker.empty() || UseLinker == 
> "ld"` branch, where `const char *DefaultLinker = getDefaultLinker()` is going 
> to return `"lld"` because AMDGPU bot sets `-DCLANG_DEFAULT_LINKER=lld` in 
> their build. That's not a valid name, the valid name is `ld.lld`. Prior to 
> your patch, we would take the `!llvm::sys::path::is_absolute(UseLinker)` 
> branch and construct the correct linker name by appending 
> `CLANG_DEFAULT_LINKER` to '"ld."`.

Right, the updated patch does not emulate the old behavior here. What makes a 
linker name valid?

> I'd argue that your originally patch is actually the behavior we want. 
> Rather, we shouldn't consider `-DCLANG_DEFAULT_LINKER=lld` as a valid value. 
> Instead AMDGPU bot should use `-DCLANG_DEFAULT_LINKER=ld.lld`. Even better 
> would be to introduce a second CMake variable so we can control the default 
> value for `-fuse-ld=` and for `--ld-path` options separately. Right now it's 
> not clear which of the two is controlled by `-DCLANG_DEFAULT_LINKER=` (that's 
> because `-DCLANG_DEFAULT_LINKER=` actually predates the `--ld-path` option).

The interplay of `CLANG_DEFAULT_LINKER` , `-fuse-ld` and `--ld-path` is really 
a mess. How do we proceed here?
To be honest, it is hard to change anything about these flags as any semantic 
change may result in breakage on some (maybe downstream) toolchain and build 
configuration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115045

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

[PATCH] D112091: libfuzzer: All building libfuzzer for ARM32

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

Since this change we've had check-2 timeouts on our 2 stage v7 bots 
https://lab.llvm.org/buildbot/#/builders/190/builds/513 (we have one building 
thumb and one Arm)

These bots run on v7 hardware as opposed to some of our others that actually 
run on v8 hardware, those seem fine. I see you already disabled some tests so 
I'm going to see if I can find the specific tests that are hanging and do the 
same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112091

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

[PATCH] D115232: [clangd] WIP various stdlib indexing stuff

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman, mgrang, javed.absar, 
mgorny.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

To be split


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115232

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/FS.cpp
  clang-tools-extra/clangd/FS.h
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/FileIndex.h
  clang-tools-extra/clangd/index/StdLib.cpp
  clang-tools-extra/clangd/index/StdLib.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolCollector.h
  clang-tools-extra/clangd/index/SymbolOrigin.cpp
  clang-tools-extra/clangd/index/SymbolOrigin.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/StdLibIndexTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -1121,7 +1121,8 @@
   public:
 BlockPreambleThread(llvm::StringRef BlockVersion, Notification &N)
 : BlockVersion(BlockVersion), N(N) {}
-void onPreambleAST(PathRef Path, llvm::StringRef Version, ASTContext &Ctx,
+void onPreambleAST(PathRef Path, llvm::StringRef Version,
+   const CompilerInvocation &, ASTContext &Ctx,
std::shared_ptr PP,
const CanonicalIncludes &) override {
   if (Version == BlockVersion)
Index: clang-tools-extra/clangd/unittests/StdLibIndexTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/StdLibIndexTests.cpp
@@ -0,0 +1,101 @@
+//===-- StdLibIndexTests.cpp *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Compiler.h"
+#include "Config.h"
+#include "TestFS.h"
+#include "index/StdLib.h"
+#include "clang/Basic/LangStandard.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace testing;
+
+namespace clang {
+namespace clangd {
+namespace {
+
+/// check that the generated header sources contains some usual standard library
+/// headers
+TEST(StdLibIndexTests, generateUmbrellaHeader) {
+  auto CXX = generateUmbrellaHeaders(LangStandard::lang_cxx14).str();
+  EXPECT_THAT(CXX, HasSubstr("#include "));
+  EXPECT_THAT(CXX, HasSubstr("#include "));
+  EXPECT_THAT(CXX, Not(HasSubstr("#include ")));
+
+  auto C = generateUmbrellaHeaders(LangStandard::lang_c11).str();
+  EXPECT_THAT(C, Not(HasSubstr("#include ")));
+  EXPECT_THAT(C, Not(HasSubstr("#include ")));
+  EXPECT_THAT(C, HasSubstr("#include "));
+}
+
+MATCHER_P(Named, Name, "") { return arg.Name == Name; }
+
+/// build the index and check if it contains the right symbols
+TEST(StdLibIndexTests, buildIndex) {
+  MockFS FS;
+  // TODO: maybe find a way to use a local libcxx for testing if that is
+  //   available on the machine
+  std::string HeaderMock = R"CPP(
+  #if __cplusplus >= 201703L
+int func17();
+  #elif __cplusplus >= 201402L
+int func14();
+  #else
+bool func98();
+  #endif
+  int __implementation_details();
+  )CPP";
+  StdlibIndexSpec Spec;
+  Spec.LangStd = LangStandard::lang_cxx14;
+  auto Symbols = indexUmbrellaHeaders(HeaderMock, FS, Spec);
+
+  EXPECT_THAT(Symbols, ElementsAre(Named("func14")));
+}
+
+CompilerInvocation parse(std::vector Flags) {
+  ParseInputs Inputs;
+  Inputs.CompileCommand.Filename = testPath(Flags.back());
+  Inputs.CompileCommand.Directory = testRoot();
+  Inputs.CompileCommand.CommandLine = std::move(Flags);
+  MockFS FS;
+  Inputs.TFS = &FS;
+  IgnoreDiagnostics IgnoreDiags;
+  auto Result = buildCompilerInvocation(Inputs, IgnoreDiags);
+  EXPECT_TRUE(Result);
+  return *Result;
+}
+
+TEST(StdLibIndexTests, Detect) {
+  {
+Config Cfg;
+Cfg.Index.StandardLibrary = false;
+WithContextValue WithCfg(Config::Key, std::move(Cfg));
+EXPECT_FALSE(StdlibIndexSpec::detect(parse({"

[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

[Alexey Bader via Phabricator via cfe-commits]

bader added a comment.

In D110622#3174113 , @tra wrote:

> The patch looks OK for the time being. That said, I do have concerns that we 
> may be organically growing something that will be troublesome to deal with 
> long-term.
>
> TBH, I still can't quite make sense of where/how SPIR-V fits in the 
> offloading nomenclature.
>
> Right now we have multiple levels of offloading-related control points.
>
> - offload targets, specified by --offload-arch. Determines the ISA of the GPU 
> binary we produce.
> - offload mechanism: OpenMP, CUDA runtime, HSA. Determines how we 
> compile/pack/launch the GPU binaries.
> - front-end: CUDA/HIP/ C/C++ w/ OpenMP.
> - Driver: Determines compilation pipeline to glue everything together,
>
> SPIR-V in these patches appears to be wearing multiple hats. 
> It changes compilation pipeline, it changes offload mechanism and it changes 
> offload targets.

From my POV, SPIR-V is "the ISA of GPU binary we produce" and we might need 
some changes at offloading-related control points:

- offload mechanism: none of listed "offload mechanisms" (i.e. OpenMP, CUDA 
runtime, HSA) is able to handle SPIR-V natively. On the other hand, I'm not 
sure if there is a need in additional changes for all "offloading mechanisms". 
E.g. Intel's compiler implements OpenMP-offload to SPIR-V target using OpenMP 
runtime plug-in lowering OpenMP runtime calls to OpenCL/Level Zero. OpenCL and 
Level Zero  runtimes are 
able to compile and launch SPIR-V binaries.
- front-end: if we compare SPIR to other ISAs, they change compilation pipeline 
as well (e.g. add new built-ins to expose ISA, add CodeGen library changes to 
emit ISA specific metadata, etc.). AMDGPU ISA 
 or NVIDIA 
 GPU 
 ISA changed front-end/compilation 
pipeline as well. Do you refer to some other non-ISA specific changes? BTW, 
shameless plug, the patch adding built-in functions and types for SPIR-V ISA is 
under review here - https://reviews.llvm.org/D108034.
- Driver: front-end compiler doesn't support SPIR-V format yet (i.e. SPIR-V 
requires special encoding different from LLVM bitcode) , so Driver hooks up 
LLVM->SPIR-V translator tool to produce SPIR-V binary.

> To further complicate things, it appears to be a derivative of the HIP 
> compilation. I can't tell if it's an implementation detail at the moment, or 
> whether it will become a more generic offload mechanism that would be 
> expected to be used by other front- and back-ends. E.g. can we potentially 
> compile CUDA code to target SPIR-V? Can OpenMP offload to SPIR-V?

Intel's compiler compiles OpenMP offload and SYCL to SPIR-V, so we definitely 
would like to target SPIR-V using other front-ends.

> So, the question is -- what's the right way to specify something like this in 
> a consistent manner? 
> `--offload` option proposed here does not seem to be a good fit. It was 
> intended as a more flexible way to create a single `-cc1` sub-compilation and 
> we're doing quite a bit more here.

Does `--offload-arch=spirv*` fit better here? If I understand the goal of this 
patch correctly, it tries to provide controls for changing offload target for 
HIP application from default (AMDGCN) to SPIR-V.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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

[PATCH] D114522: [clangd] Add desugared type to hover

[Sam McCall via Phabricator via cfe-commits]

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

Thank you! (& Sorry for the delay, have been sick)
LG with the understanding we may have to iterate on behavior/configurability.

As noted below I'd suggest either disabling aka for params, or reverting to the 
previous version, but I don't feel strongly.




Comment at: clang-tools-extra/clangd/Hover.cpp:1255
   if (P.Type)
-Output.push_back(*P.Type);
+OS << llvm::to_string(*P.Type);
   if (P.Name)

this can just be << *P.Type, skipping the temporary string



Comment at: clang-tools-extra/clangd/Hover.cpp:1271
 OS << " = " << *P.Default;
+  if (P.Type && P.Type->AKA)
+OS << llvm::formatv(" (aka {0})", *P.Type->AKA);

kadircet wrote:
> sammccall wrote:
> > Hmm, this seems to render as:
> > 
> > ```std::string Foo = "" (aka basic_string)```
> > 
> > it's not *completely* clear what the aka refers to.
> > I don't have a better solution, though.
> > @kadircet any opinions here?
> i think it's attached to the type of the parameter, not it's name nor the 
> initializer-expr. so I think we should move this closer to the type (i.e. 
> `std::string (aka basic_sting) Foo = ""`, basically 
> `llvm::toString(P.Type)` ?)
> i think it's attached to the type of the parameter

This is logically correct but I think it's harder to read. This puts text in 
the middle of code, in a way that doesn't seem obvious to me: parens mean 
several things in C++ and it may be hard to recognize this means none of them.

Worst case is we have function types: `word(*)() (aka long(*)()) x = nullptr`

It also disrupts the reading flow in the case where the aka is *not* needed for 
understanding.
I think overall the previous version was better, though not great.

I'm tempted to say we should scope down this patch further until we have a 
better feel for how it behaves, i.e. exclude param types from aka for now. 
Param types are less obviously useful to disambiguate than result types. (e.g. 
because in most cases you can hover over the input expression).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114522

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

[PATCH] D114522: [clangd] Add desugared type to hover

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D114522#3156167 , @kadircet wrote:

> Have a list of user-configurable FQNs that AKA printing would be disabled for 
> (and provide a good set of defaults inside clangd for STL)

BTW this seems hard to implement well without invasive changes to the desugar 
logic.

Nontrivial case: the list is `{std::string}`, the type is `std::string*`.

Pathological case: the list is `{std::string}`, and the type is 
`pair`. Desugar yields `pair*, long 
long>`, desired output is `pair`.
desugarForDiagnostic needs to exclude the first TP from desugaring based on it 
matching a string, but we don't stringify all the intermediate types.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114522

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

[PATCH] D115149: [analyzer][solver] Fix assertion on (NonLoc, Op, Loc) expressions

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:463
+  if (const Optional RV = rhs.getAs()) {
+const auto IsCommutative = [](BinaryOperatorKind Op) {
+  return Op == BO_Mul || Op == BO_Add || Op == BO_And || Op == BO_Xor ||

IMO it should be in a family of `BinaryOperator::is..` methods.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115149

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

[PATCH] D112091: libfuzzer: All building libfuzzer for ARM32

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

Done in https://reviews.llvm.org/rG6bfbb89e96fa.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112091

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

[PATCH] D115107: [clang][clangd] Desugar array type.

[Sam McCall via Phabricator via cfe-commits]

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

Nice!




Comment at: clang/lib/AST/ASTDiagnostic.cpp:134
 
+if (const auto *CAT = dyn_cast(Ty)) {
+  QualType ElementTy =

this doesn't directly express the idea "we handle all array types". You *could* 
write:

```
if (const auto *AT = dyn_cast(Ty)) {
 QualType ElementTy = desugar(...);
 switch(AT->getTypeClass()) {
case Type::ConstantArray:
  QT = Context.getConstantArrayType(...);
  break;
...
default:
  llvm_unreachable("Unhandled array type");
 }
 break;
}
```

Up to you whether you think this is clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115107

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

[PATCH] D115235: [clang][dataflow] Implement a basic algorithm for dataflow analysis

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev created this revision.
sgatev added reviewers: ymandel, NoQ, xazax.hun, gribozavr2.
Herald added subscribers: rnkovacs, mgorny.
sgatev requested review of this revision.
Herald added a project: clang.

This is part of the implementation of the dataflow analysis framework.
See "[RFC] A dataflow analysis framework for Clang AST" on cfe-dev.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115235

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/DataflowWorklist.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -0,0 +1,144 @@
+//===- unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+
+using namespace clang;
+using namespace dataflow;
+
+using ::testing::NotNull;
+
+template 
+class AnalysisCallback : public ast_matchers::MatchFinder::MatchCallback {
+public:
+  void run(const ast_matchers::MatchFinder::MatchResult &Result) override {
+const auto *Func = Result.Nodes.getNodeAs("func");
+ASSERT_THAT(Func, NotNull());
+
+Stmt *Body = Func->getBody();
+ASSERT_THAT(Body, NotNull());
+
+CFG::BuildOptions Options;
+Options.AddImplicitDtors = true;
+Options.AddTemporaryDtors = true;
+Options.setAllAlwaysAdd();
+
+std::unique_ptr Cfg =
+CFG::buildCFG(nullptr, Body, Result.Context, Options);
+ASSERT_THAT(Cfg, NotNull());
+
+AnalysisT Analysis(*Result.Context);
+Environment Env;
+BlockStates = runDataflowAnalysis(*Cfg, Analysis, Env);
+  }
+
+  std::vector<
+  llvm::Optional>>
+  BlockStates;
+};
+
+template 
+std::vector>>
+runAnalysis(const char *Code) {
+  std::unique_ptr AST =
+  tooling::buildASTFromCodeWithArgs(Code, {"-std=c++11"});
+
+  AnalysisCallback Callback;
+  ast_matchers::MatchFinder Finder;
+  Finder.addMatcher(
+  ast_matchers::functionDecl(ast_matchers::hasName("target")).bind("func"),
+  &Callback);
+  Finder.matchAST(AST->getASTContext());
+
+  return Callback.BlockStates;
+}
+
+class NoopLattice {
+public:
+  bool operator==(const NoopLattice &) const { return true; }
+
+  LatticeJoinEffect join(const NoopLattice &) {
+return LatticeJoinEffect::Unchanged;
+  }
+};
+
+class NoopAnalysis : public DataflowAnalysis {
+public:
+  NoopAnalysis(ASTContext &Context)
+  : DataflowAnalysis(Context) {}
+
+  static NoopLattice initialElement() { return {}; }
+
+  NoopLattice transfer(const Stmt *S, const NoopLattice &E, Environment &Env) {
+return {};
+  }
+};
+
+TEST(DataflowAnalysisTest, NoopAnalysis) {
+  auto BlockStates = runAnalysis(R"(
+void target() {}
+  )");
+  EXPECT_EQ(BlockStates.size(), 2u);
+  EXPECT_TRUE(BlockStates[0].hasValue());
+  EXPECT_TRUE(BlockStates[1].hasValue());
+}
+
+struct NonConvergingLattice {
+  int State;
+
+  bool operator==(const NonConvergingLattice &Other) const {
+return State == Other.State;
+  }
+
+  LatticeJoinEffect join(const NonConvergingLattice &Other) {
+if (Other.State == 0)
+  return LatticeJoinEffect::Unchanged;
+State += Other.State;
+return LatticeJoinEffect::Changed;
+  }
+};
+
+class NonConvergingAnalysis
+: public DataflowAnalysis {
+public:
+  explicit NonConvergingAnalysis(ASTContext &Context)
+  : DataflowAnalysis(Context) {
+  }
+
+  static NonConvergingLattice initialElement() { return {0}; }
+
+  NonConvergingLattice transfer(const Stmt *Stmt,
+const NonConvergingLattice &Element,
+Environment &Environment) {
+return {Element.State + 1};
+  }
+};
+
+TEST(DataflowAnalysisTest, NonConvergingAnalysis) {
+  auto BlockStates = runAnalysis(R"(
+void target() {
+  while(true) {}
+}
+  )");
+  EXPECT_EQ(BlockStates.size(), 4u);
+  EXPECT_FALSE(BlockStates[0].hasValue());
+  EXPECT_TRUE

[PATCH] D114665: [clangd] Make a.k.a printing configurable.

[Sam McCall via Phabricator via cfe-commits]

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

(Sorry about all the boilerplate for adding config, I think I should probably 
add some tablegen magic to cover everything except Config.h)




Comment at: clang-tools-extra/clangd/ConfigFragment.h:271
+  /// Describes hover preferences.
+  struct HoverBlock {
+/// Whether hover show a.k.a type.

One question is whether the setting should control hover specifically, or 
whether it covers "in places we print types" more generally. But it doesn't 
seem likely we'll make this configurable for diagnostics, and I don't have 
other examples. Most of our settings are per-feature. So I think this is right 
as it is.



Comment at: clang-tools-extra/clangd/ConfigFragment.h:273
+/// Whether hover show a.k.a type.
+llvm::Optional> AKAPrint;
+  };

I'd call this ShowAKA

(verb before its object; "show" is a little less jargony)



Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:1
 //===-- HoverTests.cpp 
===//
 //

we should also add at least one test with it disabled


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114665

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

[PATCH] D115108: [clangd] Print type for VarTemplateDecl in hover.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall 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/D115108/new/

https://reviews.llvm.org/D115108

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

[PATCH] D114439: [Annotation] Allow parameter pack expansions in annotate attribute

[Steffen Larsen via Phabricator via cfe-commits]

steffenlarsen updated this revision to Diff 392340.
steffenlarsen added a comment.

The new revision does the following:

1. Revisits the diagnostics. Now parameter packs will cause the same error on 
attributes that do not support parameter packs in arguments, while attributes 
that support it will cause an error if parameter packs are used in the 
incorrect argument, specifying which argument should be the earliest for it.
2. Adds additional tests to ensure that variadic arguments supporting packs 
also allow pack expansion intermingled with other arguments.
3. Adds assertions to attribute generation to check that attributes only ever 
support packs in the last variadic argument and that there are no other 
variadic arguments if they do.


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

https://reviews.llvm.org/D114439

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/Parser/cxx0x-attributes.cpp
  clang/test/SemaTemplate/attributes.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -1164,10 +1164,12 @@
   };
 
   class VariadicExprArgument : public VariadicArgument {
+bool AllowPack = false;
+
   public:
 VariadicExprArgument(const Record &Arg, StringRef Attr)
-  : VariadicArgument(Arg, Attr, "Expr *")
-{}
+: VariadicArgument(Arg, Attr, "Expr *"),
+  AllowPack(Arg.getValueAsBit("AllowPack")) {}
 
 void writeASTVisitorTraversal(raw_ostream &OS) const override {
   OS << "  {\n";
@@ -2264,6 +2266,47 @@
   OS << "#endif // CLANG_ATTR_THIS_ISA_IDENTIFIER_ARG_LIST\n\n";
 }
 
+static void emitClangAttrNumArgs(RecordKeeper &Records, raw_ostream &OS) {
+  OS << "#if defined(CLANG_ATTR_NUM_ARGS)\n";
+  std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
+  for (const auto *A : Attrs) {
+std::vector Args = A->getValueAsListOfDefs("Args");
+forEachUniqueSpelling(*A, [&](const FlattenedSpelling &S) {
+  OS << ".Case(\"" << S.name() << "\", " << Args.size() << ")\n";
+});
+  }
+  OS << "#endif // CLANG_ATTR_NUM_ARGS\n\n";
+}
+
+static bool argSupportsParmPack(const Record *Arg) {
+  return !Arg->getSuperClasses().empty() &&
+ llvm::StringSwitch(
+ Arg->getSuperClasses().back().first->getName())
+ .Case("VariadicExprArgument", true)
+ .Default(false) &&
+ Arg->getValueAsBit("AllowPack");
+}
+
+static void emitClangAttrLastArgParmPackSupport(RecordKeeper &Records,
+raw_ostream &OS) {
+  OS << "#if defined(CLANG_ATTR_LAST_ARG_PARM_PACK_SUPPORT)\n";
+  std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
+  for (const auto *A : Attrs) {
+std::vector Args = A->getValueAsListOfDefs("Args");
+bool LastArgSupportsParmPack =
+!Args.empty() && argSupportsParmPack(Args.back());
+forEachUniqueSpelling(*A, [&](const FlattenedSpelling &S) {
+  OS << ".Case(\"" << S.name() << "\", " << LastArgSupportsParmPack
+ << ")\n";
+});
+  }
+  OS << "#endif // CLANG_ATTR_LAST_ARG_PARM_PACK_SUPPORT\n\n";
+}
+
+static bool isArgVariadic(const Record &R, StringRef AttrName) {
+  return createArgument(R, AttrName)->isVariadic();
+}
+
 static void emitAttributes(RecordKeeper &Records, raw_ostream &OS,
bool Header) {
   std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
@@ -2320,6 +2363,18 @@
 std::vector> Args;
 Args.reserve(ArgRecords.size());
 
+if (!ArgRecords.empty()) {
+  const bool LastArgSupportsPack = argSupportsParmPack(ArgRecords.back());
+  for (size_t I = 0; I < ArgRecords.size() - 1; ++I) {
+assert((!LastArgSupportsPack ||
+!isArgVariadic(*ArgRecords[I], R.getName())) &&
+   "Attributes supporting packs can only have the last "
+   "argument as variadic");
+assert(!argSupportsParmPack(ArgRecords[I]) &&
+   "Only the last argument can allow packs");
+  }
+}
+
 bool HasOptArg = false;
 bool HasFakeArg = false;
 for (const auto *ArgRecord : ArgRecords) {
@@ -3382,10 +3437,6 @@
   }
 }
 
-static bool isArgVariadic(const Record &R, StringRef AttrName) {
-  return createArgument(R, AttrName)->isVariadic();
-}
-
 static void emitArgInfo(const Record &R, raw_ostream &OS) {
   // This function will count the number of arguments specified for the
   // attribute and emit the number of required arguments followed by the
@@ -4219,6 +4270,8 @@
   emitClangAttrIdentifierArgList(Records, OS);
   emitClangAttrVariadicIdentifierArgList(Records, OS);
   emitClangAttrThisIsaIdentifierArgList(Records, OS);
+  emitClangAttrNumArgs(Records, O

[PATCH] D115235: [clang][dataflow] Implement a basic algorithm for dataflow analysis

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev updated this revision to Diff 392346.
sgatev added a comment.

Minor changes to parameter names.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115235

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/DataflowWorklist.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -0,0 +1,143 @@
+//===- unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+
+using namespace clang;
+using namespace dataflow;
+
+using ::testing::NotNull;
+
+template 
+class AnalysisCallback : public ast_matchers::MatchFinder::MatchCallback {
+public:
+  void run(const ast_matchers::MatchFinder::MatchResult &Result) override {
+const auto *Func = Result.Nodes.getNodeAs("func");
+ASSERT_THAT(Func, NotNull());
+
+Stmt *Body = Func->getBody();
+ASSERT_THAT(Body, NotNull());
+
+CFG::BuildOptions Options;
+Options.AddImplicitDtors = true;
+Options.AddTemporaryDtors = true;
+Options.setAllAlwaysAdd();
+
+std::unique_ptr Cfg =
+CFG::buildCFG(nullptr, Body, Result.Context, Options);
+ASSERT_THAT(Cfg, NotNull());
+
+AnalysisT Analysis(*Result.Context);
+Environment Env;
+BlockStates = runDataflowAnalysis(*Cfg, Analysis, Env);
+  }
+
+  std::vector<
+  llvm::Optional>>
+  BlockStates;
+};
+
+template 
+std::vector>>
+runAnalysis(const char *Code) {
+  std::unique_ptr AST =
+  tooling::buildASTFromCodeWithArgs(Code, {"-std=c++11"});
+
+  AnalysisCallback Callback;
+  ast_matchers::MatchFinder Finder;
+  Finder.addMatcher(
+  ast_matchers::functionDecl(ast_matchers::hasName("target")).bind("func"),
+  &Callback);
+  Finder.matchAST(AST->getASTContext());
+
+  return Callback.BlockStates;
+}
+
+class NoopLattice {
+public:
+  bool operator==(const NoopLattice &) const { return true; }
+
+  LatticeJoinEffect join(const NoopLattice &) {
+return LatticeJoinEffect::Unchanged;
+  }
+};
+
+class NoopAnalysis : public DataflowAnalysis {
+public:
+  NoopAnalysis(ASTContext &Context)
+  : DataflowAnalysis(Context) {}
+
+  static NoopLattice initialElement() { return {}; }
+
+  NoopLattice transfer(const Stmt *S, const NoopLattice &E, Environment &Env) {
+return {};
+  }
+};
+
+TEST(DataflowAnalysisTest, NoopAnalysis) {
+  auto BlockStates = runAnalysis(R"(
+void target() {}
+  )");
+  EXPECT_EQ(BlockStates.size(), 2u);
+  EXPECT_TRUE(BlockStates[0].hasValue());
+  EXPECT_TRUE(BlockStates[1].hasValue());
+}
+
+struct NonConvergingLattice {
+  int State;
+
+  bool operator==(const NonConvergingLattice &Other) const {
+return State == Other.State;
+  }
+
+  LatticeJoinEffect join(const NonConvergingLattice &Other) {
+if (Other.State == 0)
+  return LatticeJoinEffect::Unchanged;
+State += Other.State;
+return LatticeJoinEffect::Changed;
+  }
+};
+
+class NonConvergingAnalysis
+: public DataflowAnalysis {
+public:
+  explicit NonConvergingAnalysis(ASTContext &Context)
+  : DataflowAnalysis(Context) {
+  }
+
+  static NonConvergingLattice initialElement() { return {0}; }
+
+  NonConvergingLattice transfer(const Stmt *S, const NonConvergingLattice &E,
+Environment &Env) {
+return {E.State + 1};
+  }
+};
+
+TEST(DataflowAnalysisTest, NonConvergingAnalysis) {
+  auto BlockStates = runAnalysis(R"(
+void target() {
+  while(true) {}
+}
+  )");
+  EXPECT_EQ(BlockStates.size(), 4u);
+  EXPECT_FALSE(BlockStates[0].hasValue());
+  EXPECT_TRUE(BlockStates[1].hasValue());
+  EXPECT_TRUE(BlockStates[2].hasValue());
+  EXPECT_TRUE(BlockStates[3].hasValue());
+}
Index: clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
==

[PATCH] D114439: [Annotation] Allow parameter pack expansions in annotate attribute

[Steffen Larsen via Phabricator via cfe-commits]

steffenlarsen added inline comments.



Comment at: clang/test/Parser/cxx0x-attributes.cpp:268
+  void faz [[clang::annotate("B", (Is + ...))]] (); // expected-warning {{pack 
fold expression is a C++17 extension}}
+  void foz [[clang::annotate("C", Is...)]] ();
 }

erichkeane wrote:
> what about:
> void foz [[clang::annotate("D", Is)]] ();
> 
> I would expect that to error.
> 
> Another test I'd like to see:
> 
> void foz[[clang::annotate("E", 1, 2, 3, Is...)]]
> 
> Also, I don't see why if THAT works, that:
> void foz[[clang::annotate("E", 1, Is..., 2, 3)]]
> 
> shouldn't be allowed as well.
> what about:
> void foz [[clang::annotate("D", Is)]] ();
>
> I would expect that to error.

So would I, but it seems that the parser and annotate is more than happy to 
consume this example, even in the current state of Clang: 
https://godbolt.org/z/rx64EKeeE
I am not sure as to why, but seeing as it is a preexisting bug I don't know if 
it makes sense to include it in the scope of this patch.

>Another test I'd like to see:
>
> void foz[[clang::annotate("E", 1, 2, 3, Is...)]]
> 
> Also, I don't see why if THAT works, that:
> void foz[[clang::annotate("E", 1, Is..., 2, 3)]]
> 
> shouldn't be allowed as well.

They seem to work as expected. I have added these cases.



Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:1166
 
   class VariadicExprArgument : public VariadicArgument {
+bool AllowPack = false;

erichkeane wrote:
> The rule of 'only the last argument is allowed to support a pack' should be 
> in the attribute emitter.
> The rule of 'only the last argument is allowed to support a pack' should be 
> in the attribute emitter.

I have added some assertions around the area where attributes are generate, as 
that carries more surrounding information. Is that approximately what you had 
in mind?


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

https://reviews.llvm.org/D114439

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

[clang] 7d5315f - Fix Sphinx formatting in release notes

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2021-12-07T07:56:40-05:00
New Revision: 7d5315fc4c4836dbc51e471f2544e8cdec452405

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

LOG: Fix Sphinx formatting in release notes

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5e6be24839af6..5ca1e1e2a8898 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -184,11 +184,11 @@ ABI Changes in Clang
 
 
 - The ``_ExtInt(N)`` extension has been standardized in C23 as ``_BitInt(N)``.
-The mangling of this type in C++ has accordingly changed: under the Microsoft
-ABI it is now mangled using the ``_BitInt`` spelling, and under the Itanium ABI
-it is now mangled using a dedicated production. Note: the ABI for 
``_BitInt(N)``
-is still in the process of being stabilized, so this type should not yet be
-used in interfaces that require ABI stability.
+  The mangling of this type in C++ has accordingly changed: under the Microsoft
+  ABI it is now mangled using the ``_BitInt`` spelling, and under the Itanium 
ABI
+  it is now mangled using a dedicated production. Note: the ABI for 
``_BitInt(N)``
+  is still in the process of being stabilized, so this type should not yet be
+  used in interfaces that require ABI stability.
 
 OpenMP Support in Clang
 ---



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

[PATCH] D115243: [clangd] Extend SymbolOrigin, stop serializing it

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: wenlei, usaxena95, arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

New values:

- Split Dynamic into Open/Preamble
- Add Background (previously was just Unknown)
- Soon: stdlib index

This requires extending to 16 bits, which fits within the padding of Symbol.
Unfortunately we're also *serializing* SymbolOrigin as a fixed 8 bits.

Stop serializing SymbolOrigin:

- conceptually, the source is whoever indexes or *deserializes* a symbol
- deserialization takes SymbolOrigin as a parameter and stamps it on each sym
- this is a breaking format change


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115243

Files:
  clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/index/Serialization.h
  clang-tools-extra/clangd/index/SymbolOrigin.cpp
  clang-tools-extra/clangd/index/SymbolOrigin.h
  clang-tools-extra/clangd/index/YAMLSerialization.cpp
  clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
  clang-tools-extra/clangd/test/index-serialization/Inputs/sample.idx
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
  clang-tools-extra/clangd/unittests/SerializationTests.cpp

Index: clang-tools-extra/clangd/unittests/SerializationTests.cpp
===
--- clang-tools-extra/clangd/unittests/SerializationTests.cpp
+++ clang-tools-extra/clangd/unittests/SerializationTests.cpp
@@ -49,7 +49,6 @@
   End:
 Line: 1
 Column: 1
-Origin:128
 Flags:129
 Documentation:'Foo doc'
 ReturnType:'int'
@@ -121,6 +120,10 @@
   return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
 }
 
+auto readIndexFile(llvm::StringRef Text) {
+  return readIndexFile(Text, SymbolOrigin::Static);
+}
+
 TEST(SerializationTest, NoCrashOnEmptyYAML) {
   EXPECT_TRUE(bool(readIndexFile("")));
 }
@@ -143,7 +146,7 @@
   EXPECT_EQ(Sym1.Documentation, "Foo doc");
   EXPECT_EQ(Sym1.ReturnType, "int");
   EXPECT_EQ(StringRef(Sym1.CanonicalDeclaration.FileURI), "file:///path/foo.h");
-  EXPECT_EQ(Sym1.Origin, static_cast(1 << 7));
+  EXPECT_EQ(Sym1.Origin, SymbolOrigin::Static);
   EXPECT_EQ(static_cast(Sym1.Flags), 129);
   EXPECT_TRUE(Sym1.Flags & Symbol::IndexedForCodeCompletion);
   EXPECT_FALSE(Sym1.Flags & Symbol::Deprecated);
Index: clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/BackgroundIndexTests.cpp
@@ -73,7 +73,8 @@
 if (Storage.find(ShardIdentifier) == Storage.end()) {
   return nullptr;
 }
-auto IndexFile = readIndexFile(Storage[ShardIdentifier]);
+auto IndexFile =
+readIndexFile(Storage[ShardIdentifier], SymbolOrigin::Background);
 if (!IndexFile) {
   ADD_FAILURE() << "Error while reading " << ShardIdentifier << ':'
 << IndexFile.takeError();
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -586,7 +586,7 @@
 auto NewIndex = std::make_unique(std::make_unique());
 auto IndexLoadTask = [File = External.Location,
   PlaceHolder = NewIndex.get()] {
-  if (auto Idx = loadIndex(File, /*UseDex=*/true))
+  if (auto Idx = loadIndex(File, SymbolOrigin::Static, /*UseDex=*/true))
 PlaceHolder->reset(std::move(Idx));
 };
 if (Tasks) {
Index: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
===
--- clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
+++ clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
@@ -334,7 +334,8 @@
 }
 
 // Auto-detects input format when parsing
-auto IndexIn = clang::clangd::readIndexFile(Buffer->get()->getBuffer());
+auto IndexIn = clang::clangd::readIndexFile(Buffer->get()->getBuffer(),
+SymbolOrigin::Static);
 if (!IndexIn) {
   llvm::errs() << llvm::toString(IndexIn.takeError()) << "\n";
   return;
@@ -374,7 +375,7 @@
   return Index.startswith("remote:")
  ? remote::getClient(Index.drop_front(strlen("remote:")),
  ProjectRoot)
- : loadIndex(Index, /*UseDex=*/true);
+ : loadIndex(Index, SymbolOrigin::Static, /*UseDex=*/true);
 }
 
 bool runCommand(std::string Request, const SymbolIndex &Index) {
Index: clang-tools-extra/clangd/index/YAMLSerialization.cpp
==

[PATCH] D113995: [clangd] Dex Trigrams: Improve query trigram generation

[Sam McCall via Phabricator via cfe-commits]

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

Sorry about the delay!




Comment at: clang-tools-extra/clangd/index/dex/Trigram.cpp:46
   // not available then 0 is stored.
-  std::vector> Next(LowercaseIdentifier.size());
+  llvm::SmallVector> Next(LowercaseIdentifier.size());
   unsigned NextTail = 0, NextHead = 0;

the default size of this vector is 6 - if we care about the optimization we 
probably want to double it or so?

(Roles above is fine, it's 40)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113995

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

[PATCH] D114782: [X86][clang] Emit diagnostic for float and double when we have features -x87 and -sse on 64-bits

[Andrew Savonichev via Phabricator via cfe-commits]

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

LGTM. The new diagnostic is not generic, but it is probably fine as long as 
x86_64 is the only target that needs this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114782

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

[PATCH] D114885: Use VersionTuple for parsing versions in Triple, fixing issues that caused the original change to be reverted. This makes it possible to distinguish between "16" and "16.0" after pars

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D114885#3174152 , @jamesfarrell 
wrote:

> In D114885#3174074 , @thakis wrote:
>
>> Oh, it's already reverted, apologies. We'll know if that fixed that bot in 
>> 15 min or so then. Something to keep in mind for relanding though :)
>
> Yeah, I'm currently trying to build and test on my Mac laptop. It's not ARM 
> tho, so if the test passes there I'm not sure what my next steps are to land 
> this change.

It seems to also fail on other arm bots. Chances are it'll fail locally on 
intel machines if you run cmake with 
`-DLLVM_DEFAULT_TARGET_TRIPLE=arm64-apple-darwin` (or some other arm triple). 
Does that help? (I haven't tried it in this case, but most of the time that's 
sufficient to repro arm-host issues on non-host machines.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114885

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

[PATCH] D114439: [Annotation] Allow parameter pack expansions in annotate attribute

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Parse/ParseDecl.cpp:447
 Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression()));
+
+if (Tok.is(tok::ellipsis)) {

So I was thinking about this overnight... I wonder if this logic is inverted 
here?  Aaron can better answer, but I wonder if we should be instead detecting 
when we are on the 'last' parameter and it is one of these 
`VariadicExprArgument` things (that accept a pack), then dropping the parser 
into a loop of:

while (Tok.is(tok::comma)){
  Tok.Consume();
  ArgExpr = CorrectTypos(ParseAssignmentExpr(...));
}
// finally, consume the closing paren.

WDYT?


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

https://reviews.llvm.org/D114439

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

[PATCH] D115248: [clang] Fix Bug 28101

[PoYao Chang via Phabricator via cfe-commits]

rZhBoYao created this revision.
rZhBoYao added reviewers: rsmith, aaron.ballman, erichkeane, v.g.vassilev, rnk.
rZhBoYao added a project: clang.
rZhBoYao requested review of this revision.
Herald added a subscriber: cfe-commits.

This should fix Bug 28101  where 
the culprit line is parsed as `FieldDecl 0x2136240  col:6 
'T':'T'`(why not `A` tho?).

I observed that GCC(trunk on godbolt.org) would issue: `:6:6: error: 
field 'int A::A' with same name as class`, which corresponds to clang's 
`diag::err_member_name_of_class`. But as the dumped AST shows, there is no name 
on the FieldDecl, and then I found clang would issue 
`diag::err_expected_member_name_or_semi` on `int (double) {};`, which resembles 
`T (A < T >) {};` so I went with this error.

I am ready to be corrected as I'm new to clang and this differs from GCC's 
behavior. Any guidance and comments would be appreciated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115248

Files:
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -15440,6 +15440,11 @@
 break;
   }
 }
+if (LLVM_UNLIKELY(!Pattern))
+  for (const auto *FD : ClassPattern->fields())
+if (Diags.hasErrorOccurred() && !FD->getIdentifier()) {
+  return ExprError();
+}
 assert(Pattern && "We must have set the Pattern!");
 
 if (!Pattern->hasInClassInitializer() ||
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -6205,6 +6205,11 @@
 // is exited (and the declarator has been parsed).
 DeclScopeObj.EnterDeclaratorScope();
 }
+if (D.hasName() && !D.getIdentifier())
+  Diag(getMissingDeclaratorIdLoc(D, Tok.getLocation()),
+   diag::err_expected_member_name_or_semi)
+  << (D.getDeclSpec().isEmpty() ? SourceRange()
+: D.getDeclSpec().getSourceRange());
   } else if (D.mayOmitIdentifier()) {
 // This could be something simple like "int" (in which case the declarator
 // portion is empty), if an abstract-declarator is allowed.


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -15440,6 +15440,11 @@
 break;
   }
 }
+if (LLVM_UNLIKELY(!Pattern))
+  for (const auto *FD : ClassPattern->fields())
+if (Diags.hasErrorOccurred() && !FD->getIdentifier()) {
+  return ExprError();
+}
 assert(Pattern && "We must have set the Pattern!");
 
 if (!Pattern->hasInClassInitializer() ||
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -6205,6 +6205,11 @@
 // is exited (and the declarator has been parsed).
 DeclScopeObj.EnterDeclaratorScope();
 }
+if (D.hasName() && !D.getIdentifier())
+  Diag(getMissingDeclaratorIdLoc(D, Tok.getLocation()),
+   diag::err_expected_member_name_or_semi)
+  << (D.getDeclSpec().isEmpty() ? SourceRange()
+: D.getDeclSpec().getSourceRange());
   } else if (D.mayOmitIdentifier()) {
 // This could be something simple like "int" (in which case the declarator
 // portion is empty), if an abstract-declarator is allowed.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114483: [SYCL] Add support for sycl_special_class attribute

[Victor Lomuller via Phabricator via cfe-commits]

Naghasan added a comment.

Just a few comments




Comment at: clang/include/clang/Basic/AttrDocs.td:411-413
+The ``__attribute__((sycl_special_class))`` attribute is used in SYCL
+headers to indicate that a class or a struct needs additional implementation 
when
+it is passed from host to device.

I think this is a bit hard to follow if you don't know the SYCL kernel lowering 
process. Small suggestion to try to make this more self contained.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:8058
+  case ParsedAttr::AT_SYCLSpecialClass:
+handleSimpleAttribute(S, D, AL);
+break;

Don't we need to have a validation for at least the `__init` function ? The doc 
says it is mandatory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114483

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

[PATCH] D115103: Leak Sanitizer port to Windows

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

oh! gosh I'm sorry I didn't realize that `lsan` has this non standard LLVM 
style .clang-format, my apologies!  D100238: [sanitizer] Set 
IndentPPDirectives: AfterHash in .clang-format 


kind of surprised TBH, doesn't feel the indentation happens enough to warrant 
it, but that's up to you its not my backyard.

  $ grep "#if" *.cpp | wc -l
  461
  llvm-project/compiler-rt/lib/sanitizer_common
  $ grep "# if" *.cpp | wc -l
  27


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

https://reviews.llvm.org/D115103

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

[PATCH] D113995: [clangd] Dex Trigrams: Improve query trigram generation

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 392380.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

No problem, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113995

Files:
  clang-tools-extra/clangd/index/dex/Trigram.cpp
  clang-tools-extra/clangd/unittests/DexTests.cpp

Index: clang-tools-extra/clangd/unittests/DexTests.cpp
===
--- clang-tools-extra/clangd/unittests/DexTests.cpp
+++ clang-tools-extra/clangd/unittests/DexTests.cpp
@@ -386,30 +386,35 @@
   trigramsAre({"c", "cl", "cla", "lan", "ang", "ngd"}));
 
   EXPECT_THAT(identifierTrigramTokens("abc_def"),
-  trigramsAre({"a", "ab", "ad", "abc", "abd", "ade", "bcd", "bde",
-   "cde", "def"}));
+  trigramsAre({"a", "d", "ab", "ad", "de", "abc", "abd", "ade",
+   "bcd", "bde", "cde", "def"}));
 
   EXPECT_THAT(identifierTrigramTokens("a_b_c_d_e_"),
-  trigramsAre({"a", "a_", "ab", "abc", "bcd", "cde"}));
+  trigramsAre({"a", "b", "ab", "bc", "abc", "bcd", "cde"}));
 
   EXPECT_THAT(identifierTrigramTokens("unique_ptr"),
-  trigramsAre({"u", "un", "up", "uni", "unp", "upt", "niq", "nip",
-   "npt", "iqu", "iqp", "ipt", "que", "qup", "qpt",
-   "uep", "ept", "ptr"}));
+  trigramsAre({"u",   "p",   "un",  "up",  "pt",  "uni", "unp",
+   "upt", "niq", "nip", "npt", "iqu", "iqp", "ipt",
+   "que", "qup", "qpt", "uep", "ept", "ptr"}));
 
-  EXPECT_THAT(
-  identifierTrigramTokens("TUDecl"),
-  trigramsAre({"t", "tu", "td", "tud", "tde", "ude", "dec", "ecl"}));
+  EXPECT_THAT(identifierTrigramTokens("TUDecl"),
+  trigramsAre({"t", "d", "tu", "td", "de", "tud", "tde", "ude",
+   "dec", "ecl"}));
 
   EXPECT_THAT(identifierTrigramTokens("IsOK"),
-  trigramsAre({"i", "is", "io", "iso", "iok", "sok"}));
+  trigramsAre({"i", "o", "is", "ok", "io", "iso", "iok", "sok"}));
 
-  EXPECT_THAT(
-  identifierTrigramTokens("abc_defGhij__klm"),
-  trigramsAre({"a",   "ab",  "ad",  "abc", "abd", "ade", "adg", "bcd",
-   "bde", "bdg", "cde", "cdg", "def", "deg", "dgh", "dgk",
-   "efg", "egh", "egk", "fgh", "fgk", "ghi", "ghk", "gkl",
-   "hij", "hik", "hkl", "ijk", "ikl", "jkl", "klm"}));
+  EXPECT_THAT(identifierTrigramTokens("_pb"),
+  trigramsAre({"_", "_p", "p", "pb"}));
+  EXPECT_THAT(identifierTrigramTokens("__pb"),
+  trigramsAre({"_", "_p", "p", "pb"}));
+
+  EXPECT_THAT(identifierTrigramTokens("abc_defGhij__klm"),
+  trigramsAre({"a",   "d",   "ab",  "ad",  "dg",  "de",  "abc",
+   "abd", "ade", "adg", "bcd", "bde", "bdg", "cde",
+   "cdg", "def", "deg", "dgh", "dgk", "efg", "egh",
+   "egk", "fgh", "fgk", "ghi", "ghk", "gkl", "hij",
+   "hik", "hkl", "ijk", "ikl", "jkl", "klm"}));
 }
 
 TEST(DexTrigrams, QueryTrigrams) {
@@ -419,8 +424,16 @@
 
   EXPECT_THAT(generateQueryTrigrams(""), trigramsAre({}));
   EXPECT_THAT(generateQueryTrigrams("_"), trigramsAre({"_"}));
-  EXPECT_THAT(generateQueryTrigrams("__"), trigramsAre({"__"}));
-  EXPECT_THAT(generateQueryTrigrams("___"), trigramsAre({}));
+  EXPECT_THAT(generateQueryTrigrams("__"), trigramsAre({"_"}));
+  EXPECT_THAT(generateQueryTrigrams("___"), trigramsAre({"_"}));
+
+  EXPECT_THAT(generateQueryTrigrams("m_"), trigramsAre({"m"}));
+
+  EXPECT_THAT(generateQueryTrigrams("p_b"), trigramsAre({"pb"}));
+  EXPECT_THAT(generateQueryTrigrams("pb_"), trigramsAre({"pb"}));
+  EXPECT_THAT(generateQueryTrigrams("_p"), trigramsAre({"_p"}));
+  EXPECT_THAT(generateQueryTrigrams("_pb_"), trigramsAre({"pb"}));
+  EXPECT_THAT(generateQueryTrigrams("__pb"), trigramsAre({"pb"}));
 
   EXPECT_THAT(generateQueryTrigrams("X86"), trigramsAre({"x86"}));
 
@@ -525,25 +538,45 @@
 }
 
 TEST(DexTest, ShortQuery) {
-  auto I = Dex::build(generateSymbols({"OneTwoThreeFour"}), RefSlab(),
+  auto I = Dex::build(generateSymbols({"_OneTwoFourSix"}), RefSlab(),
   RelationSlab());
   FuzzyFindRequest Req;
   Req.AnyScope = true;
   bool Incomplete;
 
-  EXPECT_THAT(match(*I, Req, &Incomplete), ElementsAre("OneTwoThreeFour"));
+  EXPECT_THAT(match(*I, Req, &Incomplete), ElementsAre("_OneTwoFourSix"));
   EXPECT_FALSE(Incomplete) << "Empty string is not a short query";
 
-  Req.Query = "t";
-  EXPECT_THAT(match(*I, Req, &Incomplete), ElementsAre());
-  EXPECT_TRUE(Incomplete) << "Short queries have different semantics";
+  Req.Query = "o";
+  EXPECT_THAT(match(*I, Req, &Incomplete), ElementsAre("_OneTwoFourSix"));
+  EXPECT_TRUE(Incomplete) << "Using first head as unigram";

[PATCH] D114421: [asan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 392382.
glider added a comment.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

Addressed the comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114421

Files:
  clang/docs/AddressSanitizer.rst
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/test/CodeGen/address-safety-attr-flavors.cpp
  clang/test/CodeGen/asan-globals.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  
llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll

Index: llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll
@@ -0,0 +1,47 @@
+; This test checks that we are not instrumenting sanitizer code.
+; RUN: opt < %s -passes='asan-pipeline' -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function with sanitize_address is instrumented.
+; Function Attrs: nounwind uwtable
+define void @instr_sa(i32* %a) sanitize_address {
+entry:
+  %tmp1 = load i32, i32* %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, i32* %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @instr_sa
+; CHECK: call void @__asan_report_load
+
+
+; Function with disable_sanitizer_instrumentation is not instrumented.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi(i32* %a) disable_sanitizer_instrumentation {
+entry:
+  %tmp1 = load i32, i32* %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, i32* %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi
+; CHECK-NOT: call void @__asan_report_load
+
+
+; disable_sanitizer_instrumentation takes precedence over sanitize_address.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi_sa(i32* %a) disable_sanitizer_instrumentation sanitize_address {
+entry:
+  %tmp1 = load i32, i32* %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, i32* %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi_sa
+; CHECK-NOT: call void @__asan_report_load
+
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -2889,6 +2889,9 @@
   // Leave if the function doesn't need instrumentation.
   if (!F.hasFnAttribute(Attribute::SanitizeAddress)) return FunctionModified;
 
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+return FunctionModified;
+
   LLVM_DEBUG(dbgs() << "ASAN instrumenting:\n" << F << "\n");
 
   initializeCallbacks(*F.getParent());
Index: clang/test/CodeGen/asan-globals.cpp
===
--- clang/test/CodeGen/asan-globals.cpp
+++ clang/test/CodeGen/asan-globals.cpp
@@ -10,6 +10,7 @@
 int global;
 int dyn_init_global = global;
 int __attribute__((no_sanitize("address"))) attributed_global;
+int __attribute__((disable_sanitizer_instrumentation)) disable_instrumentation_global;
 int blacklisted_global;
 
 int __attribute__((section("__DATA, __common"))) sectioned_global; // KASAN - ignore globals in a section
@@ -50,30 +51,32 @@
 // UWTABLE: attributes #[[#ATTR]] = { nounwind uwtable }
 // UWTABLE: ![[#]] = !{i32 7, !"uwtable", i32 1}
 
-// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[DISABLE_INSTR_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
 // CHECK: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], !"extra_global", i1 false, i1 false}
 // CHECK: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
 // CHECK: ![[GLOBAL]] = !{{{.*}} ![[GLOBAL_LOC:[0-9]+]], !"global", i1 false, i1 false}
 // CHECK: ![[GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 10, i32 5}
 // CHECK: ![[DYN_INIT_GLOBAL]] = !{{{.*}} ![[DYN_INIT_LOC:[0-9]+]], !"dyn_init_global", i1 true, i1 false}
 // CHECK: ![[DYN_INIT_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 11, i32 5}
-// CHECK: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
-// CHECK: ![[BLACKLISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
+// CHECK: ![[ATTR_GLOBAL]] = !{{{.*@attributed_global.*}}, null, null, i1 false, i1 true}
+// CHECK: ![[DISABLE_INSTR_GLOBAL]] = !{{

TCE target nonconforming definition of long long and intmax_t

[Aaron Ballman via cfe-commits]

Hello! I was digging around in stdint.h to do some implementation work
on C2x and I noticed that the TCE target seems to be nonconforming.

In C17, the implementation limits for intmax_t and uintmax_t are
specified by 7.20.2.5 as:

— minimum value of greatest-width signed integer type
INTMAX_MIN -(2^63 - 1)
— maximum value of greatest-width signed integer type
INTMAX_MAX 2^63 - 1
— maximum value of greatest-width unsigned integer type
UINTMAX_MAX 2^64 - 1

Similarly, the implementation limits for long long and unsigned long
long are specified by 5.2.4.2.1p1:

minimum value for an object of type long long int
LLONG_MIN -9223372036854775807 // -(2^63 - 1)
— maximum value for an object of type long long int
LLONG_MAX +9223372036854775807 // 2^63 - 1
— maximum value for an object of type unsigned long long int
ULLONG_MAX 18446744073709551615 // 2^64 - 1

However, the TCE target appears to set these to 32-bit limits, not
64-bit limits:

https://github.com/llvm/llvm-project/blob/2ab513cd3e0648806db7ed1f8170ad4a3d4e7749/clang/lib/Basic/Targets/TCE.h#L61

Is this target still being maintained? If so, what should be done
here? (I can file a bug report about this once we have a bug database
that can accept new content so we don't lose track of this.)

Thanks!

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

[PATCH] D114421: [asan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 392389.
glider added a comment.

Updated asan-globals.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114421

Files:
  clang/docs/AddressSanitizer.rst
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/test/CodeGen/address-safety-attr-flavors.cpp
  clang/test/CodeGen/asan-globals.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  
llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll

Index: llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll
@@ -0,0 +1,47 @@
+; This test checks that we are not instrumenting sanitizer code.
+; RUN: opt < %s -passes='asan-pipeline' -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function with sanitize_address is instrumented.
+; Function Attrs: nounwind uwtable
+define void @instr_sa(i32* %a) sanitize_address {
+entry:
+  %tmp1 = load i32, i32* %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, i32* %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @instr_sa
+; CHECK: call void @__asan_report_load
+
+
+; Function with disable_sanitizer_instrumentation is not instrumented.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi(i32* %a) disable_sanitizer_instrumentation {
+entry:
+  %tmp1 = load i32, i32* %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, i32* %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi
+; CHECK-NOT: call void @__asan_report_load
+
+
+; disable_sanitizer_instrumentation takes precedence over sanitize_address.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi_sa(i32* %a) disable_sanitizer_instrumentation sanitize_address {
+entry:
+  %tmp1 = load i32, i32* %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, i32* %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi_sa
+; CHECK-NOT: call void @__asan_report_load
+
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -2889,6 +2889,9 @@
   // Leave if the function doesn't need instrumentation.
   if (!F.hasFnAttribute(Attribute::SanitizeAddress)) return FunctionModified;
 
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+return FunctionModified;
+
   LLVM_DEBUG(dbgs() << "ASAN instrumenting:\n" << F << "\n");
 
   initializeCallbacks(*F.getParent());
Index: clang/test/CodeGen/asan-globals.cpp
===
--- clang/test/CodeGen/asan-globals.cpp
+++ clang/test/CodeGen/asan-globals.cpp
@@ -10,6 +10,7 @@
 int global;
 int dyn_init_global = global;
 int __attribute__((no_sanitize("address"))) attributed_global;
+int __attribute__((disable_sanitizer_instrumentation)) disable_instrumentation_global;
 int blacklisted_global;
 
 int __attribute__((section("__DATA, __common"))) sectioned_global; // KASAN - ignore globals in a section
@@ -50,31 +51,33 @@
 // UWTABLE: attributes #[[#ATTR]] = { nounwind uwtable }
 // UWTABLE: ![[#]] = !{i32 7, !"uwtable", i32 1}
 
-// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[DISABLE_INSTR_GLOBAL:[0-9]+]], ![[BLACKLISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
 // CHECK: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], !"extra_global", i1 false, i1 false}
 // CHECK: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
 // CHECK: ![[GLOBAL]] = !{{{.*}} ![[GLOBAL_LOC:[0-9]+]], !"global", i1 false, i1 false}
 // CHECK: ![[GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 10, i32 5}
 // CHECK: ![[DYN_INIT_GLOBAL]] = !{{{.*}} ![[DYN_INIT_LOC:[0-9]+]], !"dyn_init_global", i1 true, i1 false}
 // CHECK: ![[DYN_INIT_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 11, i32 5}
-// CHECK: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
-// CHECK: ![[BLACKLISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
+// CHECK: ![[ATTR_GLOBAL]] = !{{{.*@attributed_global.*}}, null, null, i1 false, i1 true}
+// CHECK: ![[DISABLE_INSTR_GLOBAL]] = !{{{.*@disable_instrumentation_global.*}}, null, null, i1 false, i1 true}
+// CHEC

[PATCH] D114421: [asan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider added a comment.

> Should we have in AddressSanitizer.cpp the following for consistency with 
> other sanitizers?

Good catch, without that disable_sanitizer_instrumentation could not override 
sanitize_address.
I fixed this and added a test.




Comment at: clang/test/CodeGen/asan-globals.cpp:62
 // CHECK: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
+// CHECK: ![[DISABLE_INSTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
 // CHECK: ![[BLACKLISTED_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}

eugenis wrote:
> Does this test rely on the metadata being in the same order as the global 
> declarations in the source?  Feels brittle and hard to understand - could you 
> check if the regexps could be made more precise?
`llvm.asan.globals` above actually look as follows: `!llvm.asan.globals = !{!0, 
!2, !4, !6, !7, !8, !9, !11, !13, !15}`

, so the placeholder variables (e.g. `ATTR_GLOBAL` and `DISABLE_INSTR_GLOBAL`) 
actually match just the metadata node numbers.
I replaced the `{{.*}}` with variable/file names where applicable, but we'll 
still depend on the order of globals.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114421

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

[clang-tools-extra] 976a74d - [clangd] Dex Trigrams: Improve query trigram generation

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2021-12-07T15:46:13+01:00
New Revision: 976a74d7d2dbd19670614f603caf490cca892fdc

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

LOG: [clangd] Dex Trigrams: Improve query trigram generation

These are the trigrams for queries right now:

- "va" -> {Trigram("va")}
- "va_" -> {} (empty)

This is suboptimal since the resulting query will discard the query information
and return all symbols, some of which will be later be scored expensively
(fuzzy matching score). This is related to
https://github.com/clangd/clangd/issues/39 but does not fix it. Accidentally,
because of that incorrect behavior, when user types "tok::va" there are no
results (the issue is that `tok::kw___builtin_va_arg` does not have "va" token)
but when "tok::va_" is typed, expected result (`tok::kw___builtin_va_arg`)
shows up by accident. This is because the dex query transformer will only
lookup symbols within the `tok::` namespace. There won't be many, so the
returned results will contain symbol we need; this symbol will be filtered out
by the expensive checks and that will be displayed in the editor.

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/index/dex/Trigram.cpp
clang-tools-extra/clangd/unittests/DexTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/index/dex/Trigram.cpp 
b/clang-tools-extra/clangd/index/dex/Trigram.cpp
index fddca8a953209..e6f42f67c50e7 100644
--- a/clang-tools-extra/clangd/index/dex/Trigram.cpp
+++ b/clang-tools-extra/clangd/index/dex/Trigram.cpp
@@ -12,10 +12,14 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
 #include 
+#include 
 #include 
 #include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -25,21 +29,22 @@ namespace dex {
 template 
 static void identifierTrigrams(llvm::StringRef Identifier, Func Out) {
   // Apply fuzzy matching text segmentation.
-  std::vector Roles(Identifier.size());
+  llvm::SmallVector Roles(Identifier.size());
   calculateRoles(Identifier,
  llvm::makeMutableArrayRef(Roles.data(), Identifier.size()));
 
   std::string LowercaseIdentifier = Identifier.lower();
 
   // For each character, store indices of the characters to which fuzzy 
matching
-  // algorithm can jump. There are 3 possible variants:
+  // algorithm can jump. There are 2 possible variants:
   //
   // * Next Tail - next character from the same segment
   // * Next Head - front character of the next segment
   //
   // Next stores tuples of three indices in the presented order, if a variant 
is
   // not available then 0 is stored.
-  std::vector> Next(LowercaseIdentifier.size());
+  llvm::SmallVector, 12> Next(
+  LowercaseIdentifier.size());
   unsigned NextTail = 0, NextHead = 0;
   for (int I = LowercaseIdentifier.size() - 1; I >= 0; --I) {
 Next[I] = {{NextTail, NextHead}};
@@ -51,14 +56,14 @@ static void identifierTrigrams(llvm::StringRef Identifier, 
Func Out) {
 
   // Iterate through valid sequences of three characters Fuzzy Matcher can
   // process.
-  for (size_t I = 0; I < LowercaseIdentifier.size(); ++I) {
+  for (unsigned I = 0; I < LowercaseIdentifier.size(); ++I) {
 // Skip delimiters.
 if (Roles[I] != Head && Roles[I] != Tail)
   continue;
-for (const unsigned J : Next[I]) {
+for (unsigned J : Next[I]) {
   if (J == 0)
 continue;
-  for (const unsigned K : Next[J]) {
+  for (unsigned K : Next[J]) {
 if (K == 0)
   continue;
 Out(Trigram(LowercaseIdentifier[I], LowercaseIdentifier[J],
@@ -66,16 +71,29 @@ static void identifierTrigrams(llvm::StringRef Identifier, 
Func Out) {
   }
 }
   }
-  // Emit short-query trigrams: FooBar -> f, fo, fb.
-  if (!LowercaseIdentifier.empty())
-Out(Trigram(LowercaseIdentifier[0]));
-  if (LowercaseIdentifier.size() >= 2)
-Out(Trigram(LowercaseIdentifier[0], LowercaseIdentifier[1]));
-  for (size_t I = 1; I < LowercaseIdentifier.size(); ++I)
-if (Roles[I] == Head) {
-  Out(Trigram(LowercaseIdentifier[0], LowercaseIdentifier[I]));
+  // Short queries semantics are 
diff erent. When the user dosn't type enough
+  // symbols to form trigrams, we still want to serve meaningful results. To
+  // achieve that, we form incomplete trigrams (bi- and unigrams) for the
+  // identifiers and also generate short trigrams on the query side from what
+  // is available. We allow a small number of short trigram types in order to
+  // prevent excessive memory usage and increase the quality of the search.
+  // Only the first few symbols are allowed to be use

[PATCH] D113995: [clangd] Dex Trigrams: Improve query trigram generation

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG976a74d7d2db: [clangd] Dex Trigrams: Improve query trigram 
generation (authored by kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113995

Files:
  clang-tools-extra/clangd/index/dex/Trigram.cpp
  clang-tools-extra/clangd/unittests/DexTests.cpp

Index: clang-tools-extra/clangd/unittests/DexTests.cpp
===
--- clang-tools-extra/clangd/unittests/DexTests.cpp
+++ clang-tools-extra/clangd/unittests/DexTests.cpp
@@ -386,30 +386,35 @@
   trigramsAre({"c", "cl", "cla", "lan", "ang", "ngd"}));
 
   EXPECT_THAT(identifierTrigramTokens("abc_def"),
-  trigramsAre({"a", "ab", "ad", "abc", "abd", "ade", "bcd", "bde",
-   "cde", "def"}));
+  trigramsAre({"a", "d", "ab", "ad", "de", "abc", "abd", "ade",
+   "bcd", "bde", "cde", "def"}));
 
   EXPECT_THAT(identifierTrigramTokens("a_b_c_d_e_"),
-  trigramsAre({"a", "a_", "ab", "abc", "bcd", "cde"}));
+  trigramsAre({"a", "b", "ab", "bc", "abc", "bcd", "cde"}));
 
   EXPECT_THAT(identifierTrigramTokens("unique_ptr"),
-  trigramsAre({"u", "un", "up", "uni", "unp", "upt", "niq", "nip",
-   "npt", "iqu", "iqp", "ipt", "que", "qup", "qpt",
-   "uep", "ept", "ptr"}));
+  trigramsAre({"u",   "p",   "un",  "up",  "pt",  "uni", "unp",
+   "upt", "niq", "nip", "npt", "iqu", "iqp", "ipt",
+   "que", "qup", "qpt", "uep", "ept", "ptr"}));
 
-  EXPECT_THAT(
-  identifierTrigramTokens("TUDecl"),
-  trigramsAre({"t", "tu", "td", "tud", "tde", "ude", "dec", "ecl"}));
+  EXPECT_THAT(identifierTrigramTokens("TUDecl"),
+  trigramsAre({"t", "d", "tu", "td", "de", "tud", "tde", "ude",
+   "dec", "ecl"}));
 
   EXPECT_THAT(identifierTrigramTokens("IsOK"),
-  trigramsAre({"i", "is", "io", "iso", "iok", "sok"}));
+  trigramsAre({"i", "o", "is", "ok", "io", "iso", "iok", "sok"}));
 
-  EXPECT_THAT(
-  identifierTrigramTokens("abc_defGhij__klm"),
-  trigramsAre({"a",   "ab",  "ad",  "abc", "abd", "ade", "adg", "bcd",
-   "bde", "bdg", "cde", "cdg", "def", "deg", "dgh", "dgk",
-   "efg", "egh", "egk", "fgh", "fgk", "ghi", "ghk", "gkl",
-   "hij", "hik", "hkl", "ijk", "ikl", "jkl", "klm"}));
+  EXPECT_THAT(identifierTrigramTokens("_pb"),
+  trigramsAre({"_", "_p", "p", "pb"}));
+  EXPECT_THAT(identifierTrigramTokens("__pb"),
+  trigramsAre({"_", "_p", "p", "pb"}));
+
+  EXPECT_THAT(identifierTrigramTokens("abc_defGhij__klm"),
+  trigramsAre({"a",   "d",   "ab",  "ad",  "dg",  "de",  "abc",
+   "abd", "ade", "adg", "bcd", "bde", "bdg", "cde",
+   "cdg", "def", "deg", "dgh", "dgk", "efg", "egh",
+   "egk", "fgh", "fgk", "ghi", "ghk", "gkl", "hij",
+   "hik", "hkl", "ijk", "ikl", "jkl", "klm"}));
 }
 
 TEST(DexTrigrams, QueryTrigrams) {
@@ -419,8 +424,16 @@
 
   EXPECT_THAT(generateQueryTrigrams(""), trigramsAre({}));
   EXPECT_THAT(generateQueryTrigrams("_"), trigramsAre({"_"}));
-  EXPECT_THAT(generateQueryTrigrams("__"), trigramsAre({"__"}));
-  EXPECT_THAT(generateQueryTrigrams("___"), trigramsAre({}));
+  EXPECT_THAT(generateQueryTrigrams("__"), trigramsAre({"_"}));
+  EXPECT_THAT(generateQueryTrigrams("___"), trigramsAre({"_"}));
+
+  EXPECT_THAT(generateQueryTrigrams("m_"), trigramsAre({"m"}));
+
+  EXPECT_THAT(generateQueryTrigrams("p_b"), trigramsAre({"pb"}));
+  EXPECT_THAT(generateQueryTrigrams("pb_"), trigramsAre({"pb"}));
+  EXPECT_THAT(generateQueryTrigrams("_p"), trigramsAre({"_p"}));
+  EXPECT_THAT(generateQueryTrigrams("_pb_"), trigramsAre({"pb"}));
+  EXPECT_THAT(generateQueryTrigrams("__pb"), trigramsAre({"pb"}));
 
   EXPECT_THAT(generateQueryTrigrams("X86"), trigramsAre({"x86"}));
 
@@ -525,25 +538,45 @@
 }
 
 TEST(DexTest, ShortQuery) {
-  auto I = Dex::build(generateSymbols({"OneTwoThreeFour"}), RefSlab(),
+  auto I = Dex::build(generateSymbols({"_OneTwoFourSix"}), RefSlab(),
   RelationSlab());
   FuzzyFindRequest Req;
   Req.AnyScope = true;
   bool Incomplete;
 
-  EXPECT_THAT(match(*I, Req, &Incomplete), ElementsAre("OneTwoThreeFour"));
+  EXPECT_THAT(match(*I, Req, &Incomplete), ElementsAre("_OneTwoFourSix"));
   EXPECT_FALSE(Incomplete) << "Empty string is not a short query";
 
-  Req.Query = "t";
-  EXPECT_THAT(match(*I, Req, &Incomplete), ElementsAre());
-  EXPECT_TRUE(Incomplete) << "Short queries have different semantics";
+  Req.Query = "o";
+  EXPECT_THAT(match(*I, Req, &Incomplete), ElementsAre("_OneTwoFourSix"));
+  EXPECT_TRU

[PATCH] D114564: Fix the use of -fno-approx-func along with -Ofast or -ffast-math

[Masoud Ataei via Phabricator via cfe-commits]

masoud.ataei added a comment.

Gentle reminder for reviewers. -- This PR is ready for review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114564

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

[PATCH] D115248: [clang] Fix Bug 28101

[PoYao Chang via Phabricator via cfe-commits]

rZhBoYao updated this revision to Diff 392399.

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

https://reviews.llvm.org/D115248

Files:
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -15440,6 +15440,11 @@
 break;
   }
 }
+if (LLVM_UNLIKELY(!Pattern))
+  for (const auto *FD : ClassPattern->fields())
+if (Diags.hasErrorOccurred() && !FD->getIdentifier()) {
+  return ExprError();
+}
 assert(Pattern && "We must have set the Pattern!");
 
 if (!Pattern->hasInClassInitializer() ||
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -6205,6 +6205,12 @@
 // is exited (and the declarator has been parsed).
 DeclScopeObj.EnterDeclaratorScope();
 }
+if (D.getContext() == DeclaratorContext::Member && D.hasName() &&
+!D.getIdentifier())
+  Diag(getMissingDeclaratorIdLoc(D, Tok.getLocation()),
+   diag::err_expected_member_name_or_semi)
+  << (D.getDeclSpec().isEmpty() ? SourceRange()
+: D.getDeclSpec().getSourceRange());
   } else if (D.mayOmitIdentifier()) {
 // This could be something simple like "int" (in which case the declarator
 // portion is empty), if an abstract-declarator is allowed.


Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -15440,6 +15440,11 @@
 break;
   }
 }
+if (LLVM_UNLIKELY(!Pattern))
+  for (const auto *FD : ClassPattern->fields())
+if (Diags.hasErrorOccurred() && !FD->getIdentifier()) {
+  return ExprError();
+}
 assert(Pattern && "We must have set the Pattern!");
 
 if (!Pattern->hasInClassInitializer() ||
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -6205,6 +6205,12 @@
 // is exited (and the declarator has been parsed).
 DeclScopeObj.EnterDeclaratorScope();
 }
+if (D.getContext() == DeclaratorContext::Member && D.hasName() &&
+!D.getIdentifier())
+  Diag(getMissingDeclaratorIdLoc(D, Tok.getLocation()),
+   diag::err_expected_member_name_or_semi)
+  << (D.getDeclSpec().isEmpty() ? SourceRange()
+: D.getDeclSpec().getSourceRange());
   } else if (D.mayOmitIdentifier()) {
 // This could be something simple like "int" (in which case the declarator
 // portion is empty), if an abstract-declarator is allowed.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115231: [Clang] Add __builtin_reduce_xor

[Florian Hahn via Phabricator via cfe-commits]

fhahn added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:2113
+  case Builtin::BI__builtin_reduce_xor:
 if (SemaBuiltinReduceMath(TheCall))
   return ExprError();

I think `reduce_xor` is only specified for integer types, so I think we need an 
extra check here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115231

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

[PATCH] D115248: [clang] Fix Bug 28101

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Can you add some tests please?  Additionally, we don't typically use 
LLVM_UNLIKELY like you did there.


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

https://reviews.llvm.org/D115248

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

[PATCH] D115103: Leak Sanitizer port to Windows

[Clemens Wasser via Phabricator via cfe-commits]

clemenswasser added inline comments.



Comment at: compiler-rt/lib/lsan/lsan.h:20
+#elif SANITIZER_WINDOWS
+#  include "lsan_win.h"
 #endif

MyDeveloperDay wrote:
> clang-format?
Yes, this is caused by clang-format. What should I do about it?



Comment at: compiler-rt/lib/lsan/lsan.h:20
+#elif SANITIZER_WINDOWS
+#  include "lsan_win.h"
 #endif

vitalybuka wrote:
> clemenswasser wrote:
> > MyDeveloperDay wrote:
> > > clang-format?
> > Yes, this is caused by clang-format. What should I do about it?
> @clemenswasser  Can you please a separate tiny patch which clang-format 
> nearby lines
@vitalybuka Should I create a patch, where I format the whole file or just 
these `#include`s?


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

https://reviews.llvm.org/D115103

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

[PATCH] D114688: [Clang] Add __builtin_elementwise_ceil

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

Thanks for the update, it looks like it should be in line with @aaron.ballman's 
suggestions!

I think it might be good to split off the refactoring of 
`SemaBuiltinElementwiseMathOneArg`-> `PrepareBuiltinElementwiseMathOneArgCall` 
and the update for `BI__builtin_elementwise_abs` into a separate, non 
functional change (NFC).




Comment at: clang/lib/Sema/SemaChecking.cpp:2101
 
-  case Builtin::BI__builtin_elementwise_abs:
-if (SemaBuiltinElementwiseMathOneArg(TheCall))
+  // __builtin_elementwise_abs estricts the element type to signed integers or
+  // floating point types only.

nit: typo estricts -> restricts? 



Comment at: clang/lib/Sema/SemaChecking.cpp:2121
+
+  // __builtin_elementwise_abs estricts the element type to floating point
+  // types only.

nit: typo `estricts` -> `restricts`? It should also say `_ceil` instead of 
`_abs`, right?



Comment at: clang/lib/Sema/SemaChecking.cpp:16739
 
-  QualType EltTy = TyA;
-  if (auto *VecTy = EltTy->getAs())
-EltTy = VecTy->getElementType();
-  if (EltTy->isUnsignedIntegerType())
-return Diag(ArgLoc, diag::err_builtin_invalid_arg_type)
-   << 1 << /*signed integer or float ty*/ 3 << TyA;
+  if (checkMathBuiltinElementType(*this, A.get()->getBeginLoc(), TyA))
+return true;

w


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

https://reviews.llvm.org/D114688

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

[PATCH] D115249: Clang-Tidy implicit bool conversion check use upercase suffixes

[Kilian Traub via Phabricator via cfe-commits]

kili created this revision.
Herald added a subscriber: carlosgalvezp.
kili requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Make the type suffix of the implicit bool conversion check upercase. So that 
the result does not fail the readability uppercase literall suffix check.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115249

Files:
  clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp


Index: clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
@@ -44,10 +44,10 @@
  ASTContext &Context) {
   switch (CastExprKind) {
   case CK_IntegralToBoolean:
-return Type->isUnsignedIntegerType() ? "0u" : "0";
+return Type->isUnsignedIntegerType() ? "0U" : "0";
 
   case CK_FloatingToBoolean:
-return Context.hasSameType(Type, Context.FloatTy) ? "0.0f" : "0.0";
+return Context.hasSameType(Type, Context.FloatTy) ? "0.0F" : "0.0";
 
   case CK_PointerToBoolean:
   case CK_MemberPointerToBoolean: // Fall-through on purpose.
@@ -206,13 +206,13 @@
 
   if (DestType->isFloatingType()) {
 if (Context.hasSameType(DestType, Context.FloatTy)) {
-  return BoolLiteral->getValue() ? "1.0f" : "0.0f";
+  return BoolLiteral->getValue() ? "1.0F" : "0.0F";
 }
 return BoolLiteral->getValue() ? "1.0" : "0.0";
   }
 
   if (DestType->isUnsignedIntegerType()) {
-return BoolLiteral->getValue() ? "1u" : "0u";
+return BoolLiteral->getValue() ? "1U" : "0U";
   }
   return BoolLiteral->getValue() ? "1" : "0";
 }


Index: clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
@@ -44,10 +44,10 @@
  ASTContext &Context) {
   switch (CastExprKind) {
   case CK_IntegralToBoolean:
-return Type->isUnsignedIntegerType() ? "0u" : "0";
+return Type->isUnsignedIntegerType() ? "0U" : "0";
 
   case CK_FloatingToBoolean:
-return Context.hasSameType(Type, Context.FloatTy) ? "0.0f" : "0.0";
+return Context.hasSameType(Type, Context.FloatTy) ? "0.0F" : "0.0";
 
   case CK_PointerToBoolean:
   case CK_MemberPointerToBoolean: // Fall-through on purpose.
@@ -206,13 +206,13 @@
 
   if (DestType->isFloatingType()) {
 if (Context.hasSameType(DestType, Context.FloatTy)) {
-  return BoolLiteral->getValue() ? "1.0f" : "0.0f";
+  return BoolLiteral->getValue() ? "1.0F" : "0.0F";
 }
 return BoolLiteral->getValue() ? "1.0" : "0.0";
   }
 
   if (DestType->isUnsignedIntegerType()) {
-return BoolLiteral->getValue() ? "1u" : "0u";
+return BoolLiteral->getValue() ? "1U" : "0U";
   }
   return BoolLiteral->getValue() ? "1" : "0";
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115250: switched to emulated TLV on macOS before 10.7

[Kirill A. Korinsky via Phabricator via cfe-commits]

catap created this revision.
catap added a reviewer: chandlerc.
catap added projects: clang, LLVM.
Herald added subscribers: dexonsmith, JDevlieghere.
catap requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits.

The first version of macOS which had TLV inside DYLD was macOS 10.7 and 
llvm/clang can't compile code:

  __thread int x;
  
  void use_x() {
  x = 7;
  }

on macOS before 10.7 as:

.section__TEXT,__text,regular,pure_instructions
.macosx_version_min 10, 6
.globl  _use_x  ## -- Begin function use_x
.p2align4, 0x90
  _use_x: ## @use_x
.cfi_startproc
pushq   %rbp
.cfi_def_cfa_offset 16
.cfi_offset %rbp, -16
movq%rsp, %rbp
.cfi_def_cfa_register %rbp
leaq___emutls_v.x(%rip), %rdi
callq   ___emutls_get_address
movl$7, (%rax)
popq%rbp
retq
.cfi_endproc
  ## -- End function
.section__DATA,__data
.globl  ___emutls_v.x   ## @__emutls_v.x
.p2align3
  ___emutls_v.x:
.quad   4   ## 0x4
.quad   4   ## 0x4
.quad   0
.quad   0
  
  .subsections_via_symbols

Which allows me to build rust for macOS 10.6 for example.

This changes effectively reverts an assumption that was introduced at least at 
25 Jun 2010 in commit 
https://github.com/llvm/llvm-project/commit/17c7b89054639ad3cd86f917ad11a50377c8de17

I guess it isn't the only place, but it shown how deep ago it was.

This patch was tested on all MacPorts users for quite a while since it was 
firstly introduced on 13 Dec 2018 at commit 
https://github.com/macports/macports-ports/commit/5949e6264728ac7f74625c9eaf5b41d2852d07e6

Co-authored-by: Ken Cunningham 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115250

Files:
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  llvm/include/llvm/ADT/Triple.h


Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -839,7 +839,7 @@
 
   /// Tests whether the target uses emulated TLS as default.
   bool hasDefaultEmulatedTLS() const {
-return isAndroid() || isOSOpenBSD() || isWindowsCygwinEnvironment();
+return isAndroid() || isOSOpenBSD() || isWindowsCygwinEnvironment() || 
isMacOSXVersionLT(10, 7);
   }
 
   /// Tests whether the target uses -data-sections as default.
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2580,7 +2580,7 @@
   const char *Name = "__cxa_atexit";
   if (TLS) {
 const llvm::Triple &T = CGF.getTarget().getTriple();
-Name = T.isOSDarwin() ?  "_tlv_atexit" : "__cxa_thread_atexit";
+Name = (T.isOSDarwin() && !T.isMacOSXVersionLT(10, 7)) ?  "_tlv_atexit" : 
"__cxa_thread_atexit";
   }
 
   // We're assuming that the destructor function is something we can
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -91,7 +91,7 @@
 this->TLSSupported = false;
 
 if (Triple.isMacOSX())
-  this->TLSSupported = !Triple.isMacOSXVersionLT(10, 7);
+  this->TLSSupported = !Triple.isMacOSXVersionLT(10, 4);
 else if (Triple.isiOS()) {
   // 64-bit iOS supported it from 8 onwards, 32-bit device from 9 onwards,
   // 32-bit simulator from 10 onwards.


Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -839,7 +839,7 @@
 
   /// Tests whether the target uses emulated TLS as default.
   bool hasDefaultEmulatedTLS() const {
-return isAndroid() || isOSOpenBSD() || isWindowsCygwinEnvironment();
+return isAndroid() || isOSOpenBSD() || isWindowsCygwinEnvironment() || isMacOSXVersionLT(10, 7);
   }
 
   /// Tests whether the target uses -data-sections as default.
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2580,7 +2580,7 @@
   const char *Name = "__cxa_atexit";
   if (TLS) {
 const llvm::Triple &T = CGF.getTarget().getTriple();
-Name = T.isOSDarwin() ?  "_tlv_atexit" : "__cxa_thread_atexit";
+Name = (T.isOSDarwin() && !T.isMacOSXVersionLT(10, 7)) ?  "_tlv_atexit" : "__cxa_thread_atexit";
   }
 
   // We're assuming that the destructor function is something we can
Index: clang/lib/Basic/Targets/OSTargets.h
===

[PATCH] D114688: [Clang] Add __builtin_elementwise_ceil

[Jun Zhang via Phabricator via cfe-commits]

junaire updated this revision to Diff 392405.
junaire added a comment.

Fix typos


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

https://reviews.llvm.org/D114688

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-elementwise-math.c
  clang/test/Sema/builtins-elementwise-math.c
  clang/test/SemaCXX/builtins-elementwise-math.cpp

Index: clang/test/SemaCXX/builtins-elementwise-math.cpp
===
--- clang/test/SemaCXX/builtins-elementwise-math.cpp
+++ clang/test/SemaCXX/builtins-elementwise-math.cpp
@@ -36,3 +36,10 @@
   static_assert(!is_const::value);
   static_assert(!is_const::value);
 }
+
+void test_builtin_elementwise_ceil() {
+  const float a = 42.0;
+  float b = 42.3;
+  static_assert(!is_const::value);
+  static_assert(!is_const::value);
+}
Index: clang/test/Sema/builtins-elementwise-math.c
===
--- clang/test/Sema/builtins-elementwise-math.c
+++ clang/test/Sema/builtins-elementwise-math.c
@@ -135,3 +135,24 @@
   c1 = __builtin_elementwise_min(c1, c2);
   // expected-error@-1 {{1st argument must be a vector, integer or floating point type (was '_Complex float')}}
 }
+
+void test_builtin_elementwise_ceil(int i, float f, double d, float4 v, int3 iv, unsigned u, unsigned4 uv) {
+
+  struct Foo s = __builtin_elementwise_ceil(f);
+  // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'float'}}
+
+  i = __builtin_elementwise_ceil();
+  // expected-error@-1 {{too few arguments to function call, expected 1, have 0}}
+
+  i = __builtin_elementwise_ceil(i);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'int')}}
+
+  i = __builtin_elementwise_ceil(f, f);
+  // expected-error@-1 {{too many arguments to function call, expected 1, have 2}}
+
+  u = __builtin_elementwise_ceil(u);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned int')}}
+
+  uv = __builtin_elementwise_ceil(uv);
+  // expected-error@-1 {{1st argument must be a floating point type (was 'unsigned4' (vector of 4 'unsigned int' values))}}
+}
Index: clang/test/CodeGen/builtins-elementwise-math.c
===
--- clang/test/CodeGen/builtins-elementwise-math.c
+++ clang/test/CodeGen/builtins-elementwise-math.c
@@ -189,3 +189,20 @@
   // CHECK-NEXT: call i32 @llvm.smin.i32(i32 [[IAS1]], i32 [[B]])
   int_as_one = __builtin_elementwise_min(int_as_one, b);
 }
+
+void test_builtin_elementwise_ceil(float f1, float f2, double d1, double d2,
+   float4 vf1, float4 vf2, si8 vi1, si8 vi2,
+   long long int i1, long long int i2, short si) {
+  // CHECK-LABEL: define void @test_builtin_elementwise_ceil(
+  // CHECK:  [[F1:%.+]] = load float, float* %f1.addr, align 4
+  // CHECK-NEXT:  call float @llvm.ceil.f32(float [[F1]])
+  f2 = __builtin_elementwise_ceil(f1);
+
+  // CHECK:  [[D1:%.+]] = load double, double* %d1.addr, align 8
+  // CHECK-NEXT: call double @llvm.ceil.f64(double [[D1]])
+  d2 = __builtin_elementwise_ceil(d1);
+
+  // CHECK:  [[VF1:%.+]] = load <4 x float>, <4 x float>* %vf1.addr, align 16
+  // CHECK-NEXT: call <4 x float> @llvm.ceil.v4f32(<4 x float> [[VF1]])
+  vf2 = __builtin_elementwise_ceil(vf1);
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2098,10 +2098,47 @@
 break;
   }
 
-  case Builtin::BI__builtin_elementwise_abs:
-if (SemaBuiltinElementwiseMathOneArg(TheCall))
+  // __builtin_elementwise_abs restricts the element type to signed integers or
+  // floating point types only.
+  case Builtin::BI__builtin_elementwise_abs: {
+if (PrepareBuiltinElementwiseMathOneArgCall(TheCall))
   return ExprError();
+
+QualType ArgTy = TheCall->getArg(0)->getType();
+QualType EltTy = ArgTy;
+
+if (auto *VecTy = EltTy->getAs())
+  EltTy = VecTy->getElementType();
+if (EltTy->isUnsignedIntegerType()) {
+  Diag(TheCall->getArg(0)->getBeginLoc(),
+   diag::err_builtin_invalid_arg_type)
+  << 1 << /* signed integer or float ty*/ 3 << ArgTy;
+  return ExprError();
+}
+break;
+  }
+
+  // __builtin_elementwise_ceil restricts the element type to floating point
+  // types only.
+  case Builtin::BI__builtin_elementwise_ceil: {
+if (PrepareBuiltinElementwiseMathOneArgCall(TheCall))
+  return ExprError();
+
+QualType ArgTy = TheCall->getArg(0)->getType();
+QualType EltTy = ArgTy;
+
+if (auto *VecTy = EltTy->getAs())
+  EltTy = VecTy->getElementType();
+if (!EltTy->isFloatingType()) {
+  D

[PATCH] D115252: Add note about inlining dllimport functions to the attribute docs

[Hans Wennborg via Phabricator via cfe-commits]

hans created this revision.
hans added a reviewer: rnk.
Herald added a reviewer: aaron.ballman.
hans requested review of this revision.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115252

Files:
  clang/include/clang/Basic/AttrDocs.td


Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -187,6 +187,10 @@
 are imported from external modules. See the dllimport_ documentation on MSDN
 for more information.
 
+Note that a dllimport function may still be inlined, if its definition is
+available and it doesn't reference any non-dllimport functions or global
+variables.
+
 .. _dllimport: https://msdn.microsoft.com/en-us/library/3y1sfaz2.aspx
   }];
 }


Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -187,6 +187,10 @@
 are imported from external modules. See the dllimport_ documentation on MSDN
 for more information.
 
+Note that a dllimport function may still be inlined, if its definition is
+available and it doesn't reference any non-dllimport functions or global
+variables.
+
 .. _dllimport: https://msdn.microsoft.com/en-us/library/3y1sfaz2.aspx
   }];
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D115253: [C2x] Support the *_WIDTH macros in limits.h and stdint.h

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, jfb, jyknight, erichkeane.
Herald added subscribers: mstorsjo, fedor.sergeev, dschuff.
aaron.ballman requested review of this revision.
Herald added a subscriber: aheejin.
Herald added a project: clang.

This completes the implementation of WG14 N2412 
(http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2412.pdf), which standardizes 
C on a twos complement representation for integer types. The only work that 
remained there was to define the correct macros in the standard headers, which 
this patch does.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115253

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Headers/limits.h
  clang/lib/Headers/stdint.h
  clang/test/Headers/limits.cpp
  clang/test/Headers/stdint.c
  clang/test/Preprocessor/init-aarch64.c
  clang/test/Preprocessor/init.c
  clang/www/c_status.html

Index: clang/www/c_status.html
===
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -715,11 +715,7 @@
 
   Two's complement sign representation
   http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2412.pdf";>N2412
-  
-Partial
-  Lacking width macros in limits.h and stdint.h
-
-  
+  Clang 14
 
 
   Adding the u8 character prefix
Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -1519,6 +1519,7 @@
 // WEBASSEMBLY-NEXT:#define __ATOMIC_RELEASE 3
 // WEBASSEMBLY-NEXT:#define __ATOMIC_SEQ_CST 5
 // WEBASSEMBLY-NEXT:#define __BIGGEST_ALIGNMENT__ 16
+// WEBASSEMBLY-NEXT:#define __BOOL_WIDTH__ 8
 // WEBASSEMBLY-NEXT:#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
 // WEBASSEMBLY-NEXT:#define __CHAR16_TYPE__ unsigned short
 // WEBASSEMBLY-NEXT:#define __CHAR32_TYPE__ unsigned int
@@ -1608,21 +1609,25 @@
 // WEBASSEMBLY-NEXT:#define __INT16_FMTi__ "hi"
 // WEBASSEMBLY-NEXT:#define __INT16_MAX__ 32767
 // WEBASSEMBLY-NEXT:#define __INT16_TYPE__ short
+// WEBASSEMBLY-NEXT:#define __INT16_WIDTH__ 16
 // WEBASSEMBLY-NEXT:#define __INT32_C_SUFFIX__
 // WEBASSEMBLY-NEXT:#define __INT32_FMTd__ "d"
 // WEBASSEMBLY-NEXT:#define __INT32_FMTi__ "i"
 // WEBASSEMBLY-NEXT:#define __INT32_MAX__ 2147483647
 // WEBASSEMBLY-NEXT:#define __INT32_TYPE__ int
+// WEBASSEMBLY-NEXT:#define __INT32_WIDTH__ 32
 // WEBASSEMBLY-NEXT:#define __INT64_C_SUFFIX__ LL
 // WEBASSEMBLY-NEXT:#define __INT64_FMTd__ "lld"
 // WEBASSEMBLY-NEXT:#define __INT64_FMTi__ "lli"
 // WEBASSEMBLY-NEXT:#define __INT64_MAX__ 9223372036854775807LL
 // WEBASSEMBLY-NEXT:#define __INT64_TYPE__ long long int
+// WEBASSEMBLY-NEXT:#define __INT64_WIDTH__ 64
 // WEBASSEMBLY-NEXT:#define __INT8_C_SUFFIX__
 // WEBASSEMBLY-NEXT:#define __INT8_FMTd__ "hhd"
 // WEBASSEMBLY-NEXT:#define __INT8_FMTi__ "hhi"
 // WEBASSEMBLY-NEXT:#define __INT8_MAX__ 127
 // WEBASSEMBLY-NEXT:#define __INT8_TYPE__ signed char
+// WEBASSEMBLY-NEXT:#define __INT8_WIDTH__ 8
 // WEBASSEMBLY-NEXT:#define __INTMAX_C_SUFFIX__ LL
 // WEBASSEMBLY-NEXT:#define __INTMAX_FMTd__ "lld"
 // WEBASSEMBLY-NEXT:#define __INTMAX_FMTi__ "lli"
@@ -1640,34 +1645,42 @@
 // WEBASSEMBLY-NEXT:#define __INT_FAST16_FMTi__ "hi"
 // WEBASSEMBLY-NEXT:#define __INT_FAST16_MAX__ 32767
 // WEBASSEMBLY-NEXT:#define __INT_FAST16_TYPE__ short
+// WEBASSEMBLY-NEXT:#define __INT_FAST16_WIDTH__ 16
 // WEBASSEMBLY-NEXT:#define __INT_FAST32_FMTd__ "d"
 // WEBASSEMBLY-NEXT:#define __INT_FAST32_FMTi__ "i"
 // WEBASSEMBLY-NEXT:#define __INT_FAST32_MAX__ 2147483647
 // WEBASSEMBLY-NEXT:#define __INT_FAST32_TYPE__ int
+// WEBASSEMBLY-NEXT:#define __INT_FAST32_WIDTH__ 32
 // WEBASSEMBLY-NEXT:#define __INT_FAST64_FMTd__ "lld"
 // WEBASSEMBLY-NEXT:#define __INT_FAST64_FMTi__ "lli"
 // WEBASSEMBLY-NEXT:#define __INT_FAST64_MAX__ 9223372036854775807LL
 // WEBASSEMBLY-NEXT:#define __INT_FAST64_TYPE__ long long int
+// WEBASSEMBLY-NEXT:#define __INT_FAST64_WIDTH__ 64
 // WEBASSEMBLY-NEXT:#define __INT_FAST8_FMTd__ "hhd"
 // WEBASSEMBLY-NEXT:#define __INT_FAST8_FMTi__ "hhi"
 // WEBASSEMBLY-NEXT:#define __INT_FAST8_MAX__ 127
 // WEBASSEMBLY-NEXT:#define __INT_FAST8_TYPE__ signed char
+// WEBASSEMBLY-NEXT:#define __INT_FAST8_WIDTH__ 8
 // WEBASSEMBLY-NEXT:#define __INT_LEAST16_FMTd__ "hd"
 // WEBASSEMBLY-NEXT:#define __INT_LEAST16_FMTi__ "hi"
 // WEBASSEMBLY-NEXT:#define __INT_LEAST16_MAX__ 32767
 // WEBASSEMBLY-NEXT:#define __INT_LEAST16_TYPE__ short
+// WEBASSEMBLY-NEXT:#define __INT_LEAST16_WIDTH__ 16
 // WEBASSEMBLY-NEXT:#define __INT_LEAST32_FMTd__ "d"
 // WEBASSEMBLY-NEXT:#define __INT_LEAST32_FMTi__ "i"
 // WEBASSEMBLY-NEXT:#define __INT_LEAST32_MAX__ 2147483647
 // WEBASSEMBLY-NEXT:#define __INT_LEAST32_TYPE__ int
+// WEBASSEMBLY-NEXT:#define __INT_LEAST32_WIDTH__ 32
 // WEBASSEMBLY-NEXT:#define __INT_LEAST64_FMTd__ "lld"
 // WEBASSEMBLY-NEXT:#define 

[PATCH] D115252: Add note about inlining dllimport functions to the attribute docs

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115252

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

[PATCH] D115254: Revert "Revert "Use VersionTuple for parsing versions in Triple, fixing issues that caused the original change to be reverted. This makes it possible to distinguish between "16" and "

[James Farrell via Phabricator via cfe-commits]

jamesfarrell created this revision.
Herald added subscribers: dexonsmith, pengfei, hiraditya.
jamesfarrell requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This reverts commit 63a6348cad6caccf285c1661bc60d8ba5a40c972 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115254

Files:
  clang/lib/ARCMigrate/ARCMT.cpp
  clang/lib/Basic/Targets/OSTargets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Driver/ToolChains/NetBSD.cpp
  clang/test/Sema/attr-availability-android.c
  clang/test/Sema/attr-availability.c
  clang/test/Sema/availability-guard-format.mm
  clang/test/SemaObjC/attr-availability.m
  clang/test/SemaObjC/property-deprecated-warning.m
  clang/test/SemaObjC/unguarded-availability-maccatalyst.m
  clang/test/SemaObjC/unguarded-availability.m
  llvm/include/llvm/ADT/Triple.h
  llvm/lib/Analysis/TargetLibraryInfo.cpp
  llvm/lib/MC/MCStreamer.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/unittests/ADT/TripleTest.cpp
  llvm/unittests/Support/Host.cpp

Index: llvm/unittests/Support/Host.cpp
===
--- llvm/unittests/Support/Host.cpp
+++ llvm/unittests/Support/Host.cpp
@@ -366,7 +366,6 @@
   }
 }
 
-#if defined(__APPLE__) || defined(_AIX)
 static bool runAndGetCommandOutput(
 const char *ExePath, ArrayRef argv,
 std::unique_ptr &Buffer, off_t &Size) {
@@ -406,12 +405,9 @@
   // disabled.
   (void) runAndGetCommandOutput;
 }
-#endif
 
-#if defined(__APPLE__)
 TEST_F(HostTest, getMacOSHostVersion) {
-  using namespace llvm::sys;
-  llvm::Triple HostTriple(getProcessTriple());
+  llvm::Triple HostTriple(llvm::sys::getProcessTriple());
   if (!HostTriple.isMacOSX())
 return;
 
@@ -420,34 +416,32 @@
   std::unique_ptr Buffer;
   off_t Size;
   ASSERT_EQ(runAndGetCommandOutput(SwVersPath, argv, Buffer, Size), true);
-  StringRef SystemVersion(Buffer.get(), Size);
+  StringRef SystemVersionStr = StringRef(Buffer.get(), Size).rtrim();
 
   // Ensure that the two versions match.
-  unsigned SystemMajor, SystemMinor, SystemMicro;
-  ASSERT_EQ(llvm::Triple((Twine("x86_64-apple-macos") + SystemVersion))
-.getMacOSXVersion(SystemMajor, SystemMinor, SystemMicro),
+  VersionTuple SystemVersion;
+  ASSERT_EQ(llvm::Triple((Twine("x86_64-apple-macos") + SystemVersionStr))
+.getMacOSXVersion(SystemVersion),
 true);
-  unsigned HostMajor, HostMinor, HostMicro;
-  ASSERT_EQ(HostTriple.getMacOSXVersion(HostMajor, HostMinor, HostMicro), true);
+  VersionTuple HostVersion;
+  ASSERT_EQ(HostTriple.getMacOSXVersion(HostVersion), true);
 
-  if (SystemMajor > 10) {
+  if (SystemVersion.getMajor() > 10) {
 // Don't compare the 'Minor' and 'Micro' versions, as they're always '0' for
 // the 'Darwin' triples on 11.x.
-ASSERT_EQ(SystemMajor, HostMajor);
+ASSERT_EQ(SystemVersion.getMajor(), HostVersion.getMajor());
   } else {
 // Don't compare the 'Micro' version, as it's always '0' for the 'Darwin'
 // triples.
-ASSERT_EQ(std::tie(SystemMajor, SystemMinor), std::tie(HostMajor, HostMinor));
+ASSERT_EQ(SystemVersion.getMajor(), HostVersion.getMajor());
+ASSERT_EQ(SystemVersion.getMinor(), HostVersion.getMinor());
   }
 }
-#endif
 
-#if defined(_AIX)
 TEST_F(HostTest, AIXVersionDetect) {
-  using namespace llvm::sys;
-
-  llvm::Triple HostTriple(getProcessTriple());
-  ASSERT_EQ(HostTriple.getOS(), Triple::AIX);
+  llvm::Triple HostTriple(llvm::sys::getProcessTriple());
+  if (HostTriple.getOS() != Triple::AIX)
+return;
 
   llvm::Triple ConfiguredHostTriple(LLVM_HOST_TRIPLE);
   ASSERT_EQ(ConfiguredHostTriple.getOS(), Triple::AIX);
@@ -457,22 +451,21 @@
   std::unique_ptr Buffer;
   off_t Size;
   ASSERT_EQ(runAndGetCommandOutput(ExePath, argv, Buffer, Size), true);
-  StringRef SystemVersion(Buffer.get(), Size);
+  StringRef SystemVersionStr = StringRef(Buffer.get(), Size).rtrim();
 
-  unsigned SystemMajor, SystemMinor, SystemMicro;
-  llvm::Triple((Twine("powerpc-ibm-aix") + SystemVersion))
-  .getOSVersion(SystemMajor, SystemMinor, SystemMicro);
+  VersionTuple SystemVersion =
+  llvm::Triple((Twine("powerpc-ibm-aix") + SystemVersionStr))
+  .getOSVersion();
 
   // Ensure that the host triple version (major) and release (minor) numbers,
   // unless explicitly configured, match with those of the current system.
   if (!ConfiguredHostTriple.getOSMajorVersion()) {
-unsigned HostMajor, HostMinor, HostMicro;
-HostTriple.getOSVersion(HostMajor, HostMinor, HostMicro);
-ASSERT_EQ(std::tie(SystemMajor, SystemMinor),
-  

[PATCH] D115254: Revert "Revert "Use VersionTuple for parsing versions in Triple, fixing issues that caused the original change to be reverted. This makes it possible to distinguish between "16" and "

[James Farrell via Phabricator via cfe-commits]

jamesfarrell added a comment.

Only change from previous attempt is to call rtrim() on the output of the 
commands in the unit tests, since the new version parsing code fails if there 
are leftover characters like \n.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115254

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

[PATCH] D114688: [Clang] Add __builtin_elementwise_ceil

[Aaron Ballman via Phabricator via cfe-commits]

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

In D114688#3176433 , @fhahn wrote:

> I think it might be good to split off the refactoring of 
> `SemaBuiltinElementwiseMathOneArg`-> 
> `PrepareBuiltinElementwiseMathOneArgCall` and the update for 
> `BI__builtin_elementwise_abs` into a separate, non functional change (NFC).

FWIW, I would not be opposed to that, but I don't insist either. LGTM!


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

https://reviews.llvm.org/D114688

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

[PATCH] D114564: Fix the use of -fno-approx-func along with -Ofast or -ffast-math

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman edited reviewers, added: joerg, rjmccall; removed: aaron.ballman.
aaron.ballman added a comment.

Removing myself as a reviewer because this is just far enough outside my 
expertise to feel uncomfortable reviewing it, but added a few trusted folks who 
have expressed opinions in this area before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114564

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

[PATCH] D115103: Leak Sanitizer port to Windows

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: compiler-rt/lib/lsan/lsan.h:20
+#elif SANITIZER_WINDOWS
+#  include "lsan_win.h"
 #endif

MyDeveloperDay wrote:
> clemenswasser wrote:
> > vitalybuka wrote:
> > > clemenswasser wrote:
> > > > MyDeveloperDay wrote:
> > > > > clang-format?
> > > > Yes, this is caused by clang-format. What should I do about it?
> > > @clemenswasser  Can you please a separate tiny patch which clang-format 
> > > nearby lines
> > @vitalybuka Should I create a patch, where I format the whole file or just 
> > these `#include`s?
> Sorry, is there a reason we can't follow the current style in this patch? 
> sorry did I miss something?  Its not about fighting clang-format its about 
> ensuring we follow the style from the lines around it which are following the 
> clang-format style. I don't need you to clang-format the whole file (although 
> that would be great because this directory has very low clang-format status)
> 
> {F20932130, size=full}
> https://clang.llvm.org/docs/ClangFormattedStatus.html
> 
> All I'm asking is that we don't indent the preprocessor directives to a non 
> LLVM style.
> 
> i.e.
> 
> ```
> #include "lsan_win.h"
> #if !SANITIZER_WINDOWS
> #endif
> ```
> 
> vs
> 
> ```
> #  include "lsan_win.h"
> #  if !SANITIZER_WINDOWS
> #  endif
> ```
You can ignore my comment, whilst I think the lsan .clang-format file is 
incorrect in terms of matching the LLVM style it is correct from the lsan 
subprojects point of view. I see now that you are doing what is in the local 
.clang-format file and its the rest of the file is incorrect. (you would handle 
that separately or not at all)

The fact that the .clang-format file doesn't follow the LLVM style is 
unfortunate, but as I expressed thats upto you all. 


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

https://reviews.llvm.org/D115103

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

[PATCH] D115253: [C2x] Support the *_WIDTH macros in limits.h and stdint.h

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

A few nits, but not nearly 'expert' enough to approve without giving everyone 
else some time to look this over.




Comment at: clang/lib/Frontend/InitPreprocessor.cpp:260
   DefineFmt(Prefix + Twine(TypeWidth), Ty, TI, Builder);
+  DefineTypeWidth(Prefix + Twine(TypeWidth) + "_WIDTH__", Ty, TI, Builder);
 }

Minor-est of nits:  It would seem that TypeSize and TypeWidth should go 
together, just because of how similar they are.  Is there a reason to not put 
this line above the Fmt?



Comment at: clang/test/Headers/limits.cpp:33
 
-const bool char_is_signed = (char)-1 < (char)0;
-_Static_assert(CHAR_MIN == (char_is_signed ? -CHAR_MAX-1 : 0), "");
-_Static_assert(CHAR_MAX == (char_is_signed ? -(CHAR_MIN+1) : (char)~0ULL), "");
+_Static_assert(CHAR_MIN == (((char)-1 < (char)0) ? -CHAR_MAX-1 : 0), "");
+_Static_assert(CHAR_MAX == (((char)-1 < (char)0) ? -(CHAR_MIN+1) : 
(char)~0ULL), "");

Is this change related?  Not sure I get what the change here is.



Comment at: clang/test/Headers/limits.cpp:62
+/* None of these are defined. */
+int BOOL_WIDTH, CHAR_WIDTH, SCHAR_WIDTH, UCHAR_WIDTH, USHRT_WIDTH, SHRT_WIDTH,
+UINT_WIDTH, INT_WIDTH, ULONG_WIDTH, LONG_WIDTH, ULLONG_WIDTH, LLONG_WIDTH;

Hmm... this is somewhat clever... perhaps overly so?  Can you improve the 
comment on 61?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115253

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

[PATCH] D115248: [clang] Fix Bug 28101

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

FWIW, this appears to cause quite a few failures in precommit CI that should be 
addressed as well: 
https://buildkite.com/llvm-project/premerge-checks/builds/68803#45a9b858-fa66-4a4b-8c4a-20c2cef820e5

  Failed Tests (11):
Clang :: CXX/basic/basic.lookup/basic.lookup.qual/class.qual/p2.cpp
Clang :: CXX/drs/dr3xx.cpp
Clang :: CXX/special/class.ctor/p1.cpp
Clang :: CXX/temp/temp.deduct.guide/p1.cpp
Clang :: CodeGenObjCXX/block-in-template-inst.mm
Clang :: Parser/explicit-bool.cpp
Clang :: SemaCXX/constant-expression-cxx11.cpp
Clang :: SemaCXX/constructor.cpp
Clang :: SemaCXX/conversion-function.cpp
Clang :: SemaCXX/cxx2a-compat.cpp
Clang :: SemaCXX/destructor.cpp


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

https://reviews.llvm.org/D115248

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

[PATCH] D115253: [C2x] Support the *_WIDTH macros in limits.h and stdint.h

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked 2 inline comments as done.
aaron.ballman added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:260
   DefineFmt(Prefix + Twine(TypeWidth), Ty, TI, Builder);
+  DefineTypeWidth(Prefix + Twine(TypeWidth) + "_WIDTH__", Ty, TI, Builder);
 }

erichkeane wrote:
> Minor-est of nits:  It would seem that TypeSize and TypeWidth should go 
> together, just because of how similar they are.  Is there a reason to not put 
> this line above the Fmt?
No reason not to move it, so move it I shall.



Comment at: clang/test/Headers/limits.cpp:33
 
-const bool char_is_signed = (char)-1 < (char)0;
-_Static_assert(CHAR_MIN == (char_is_signed ? -CHAR_MAX-1 : 0), "");
-_Static_assert(CHAR_MAX == (char_is_signed ? -(CHAR_MIN+1) : (char)~0ULL), "");
+_Static_assert(CHAR_MIN == (((char)-1 < (char)0) ? -CHAR_MAX-1 : 0), "");
+_Static_assert(CHAR_MAX == (((char)-1 < (char)0) ? -(CHAR_MIN+1) : 
(char)~0ULL), "");

erichkeane wrote:
> Is this change related?  Not sure I get what the change here is.
The original code would only compile in C++ mode; once I added a C RUN line, it 
failed because it wasn't a valid constant expression in C. This change makes it 
a valid constant expression in both C and C++ mode.



Comment at: clang/test/Headers/limits.cpp:62
+/* None of these are defined. */
+int BOOL_WIDTH, CHAR_WIDTH, SCHAR_WIDTH, UCHAR_WIDTH, USHRT_WIDTH, SHRT_WIDTH,
+UINT_WIDTH, INT_WIDTH, ULONG_WIDTH, LONG_WIDTH, ULLONG_WIDTH, LLONG_WIDTH;

erichkeane wrote:
> Hmm... this is somewhat clever... perhaps overly so?  Can you improve the 
> comment on 61?
FWIW, this follows the same pattern as what's on line 41. Do you have a 
suggested improvement (I can make it in both spots)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115253

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

[PATCH] D115253: [C2x] Support the *_WIDTH macros in limits.h and stdint.h

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 392424.
aaron.ballman marked 2 inline comments as done.
aaron.ballman added a comment.

Updated based on review feedback.


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

https://reviews.llvm.org/D115253

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Headers/limits.h
  clang/lib/Headers/stdint.h
  clang/test/Headers/limits.cpp
  clang/test/Headers/stdint.c
  clang/test/Preprocessor/init-aarch64.c
  clang/test/Preprocessor/init.c
  clang/www/c_status.html

Index: clang/www/c_status.html
===
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -715,11 +715,7 @@
 
   Two's complement sign representation
   http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2412.pdf";>N2412
-  
-Partial
-  Lacking width macros in limits.h and stdint.h
-
-  
+  Clang 14
 
 
   Adding the u8 character prefix
Index: clang/test/Preprocessor/init.c
===
--- clang/test/Preprocessor/init.c
+++ clang/test/Preprocessor/init.c
@@ -1519,6 +1519,7 @@
 // WEBASSEMBLY-NEXT:#define __ATOMIC_RELEASE 3
 // WEBASSEMBLY-NEXT:#define __ATOMIC_SEQ_CST 5
 // WEBASSEMBLY-NEXT:#define __BIGGEST_ALIGNMENT__ 16
+// WEBASSEMBLY-NEXT:#define __BOOL_WIDTH__ 8
 // WEBASSEMBLY-NEXT:#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
 // WEBASSEMBLY-NEXT:#define __CHAR16_TYPE__ unsigned short
 // WEBASSEMBLY-NEXT:#define __CHAR32_TYPE__ unsigned int
@@ -1608,21 +1609,25 @@
 // WEBASSEMBLY-NEXT:#define __INT16_FMTi__ "hi"
 // WEBASSEMBLY-NEXT:#define __INT16_MAX__ 32767
 // WEBASSEMBLY-NEXT:#define __INT16_TYPE__ short
+// WEBASSEMBLY-NEXT:#define __INT16_WIDTH__ 16
 // WEBASSEMBLY-NEXT:#define __INT32_C_SUFFIX__
 // WEBASSEMBLY-NEXT:#define __INT32_FMTd__ "d"
 // WEBASSEMBLY-NEXT:#define __INT32_FMTi__ "i"
 // WEBASSEMBLY-NEXT:#define __INT32_MAX__ 2147483647
 // WEBASSEMBLY-NEXT:#define __INT32_TYPE__ int
+// WEBASSEMBLY-NEXT:#define __INT32_WIDTH__ 32
 // WEBASSEMBLY-NEXT:#define __INT64_C_SUFFIX__ LL
 // WEBASSEMBLY-NEXT:#define __INT64_FMTd__ "lld"
 // WEBASSEMBLY-NEXT:#define __INT64_FMTi__ "lli"
 // WEBASSEMBLY-NEXT:#define __INT64_MAX__ 9223372036854775807LL
 // WEBASSEMBLY-NEXT:#define __INT64_TYPE__ long long int
+// WEBASSEMBLY-NEXT:#define __INT64_WIDTH__ 64
 // WEBASSEMBLY-NEXT:#define __INT8_C_SUFFIX__
 // WEBASSEMBLY-NEXT:#define __INT8_FMTd__ "hhd"
 // WEBASSEMBLY-NEXT:#define __INT8_FMTi__ "hhi"
 // WEBASSEMBLY-NEXT:#define __INT8_MAX__ 127
 // WEBASSEMBLY-NEXT:#define __INT8_TYPE__ signed char
+// WEBASSEMBLY-NEXT:#define __INT8_WIDTH__ 8
 // WEBASSEMBLY-NEXT:#define __INTMAX_C_SUFFIX__ LL
 // WEBASSEMBLY-NEXT:#define __INTMAX_FMTd__ "lld"
 // WEBASSEMBLY-NEXT:#define __INTMAX_FMTi__ "lli"
@@ -1640,34 +1645,42 @@
 // WEBASSEMBLY-NEXT:#define __INT_FAST16_FMTi__ "hi"
 // WEBASSEMBLY-NEXT:#define __INT_FAST16_MAX__ 32767
 // WEBASSEMBLY-NEXT:#define __INT_FAST16_TYPE__ short
+// WEBASSEMBLY-NEXT:#define __INT_FAST16_WIDTH__ 16
 // WEBASSEMBLY-NEXT:#define __INT_FAST32_FMTd__ "d"
 // WEBASSEMBLY-NEXT:#define __INT_FAST32_FMTi__ "i"
 // WEBASSEMBLY-NEXT:#define __INT_FAST32_MAX__ 2147483647
 // WEBASSEMBLY-NEXT:#define __INT_FAST32_TYPE__ int
+// WEBASSEMBLY-NEXT:#define __INT_FAST32_WIDTH__ 32
 // WEBASSEMBLY-NEXT:#define __INT_FAST64_FMTd__ "lld"
 // WEBASSEMBLY-NEXT:#define __INT_FAST64_FMTi__ "lli"
 // WEBASSEMBLY-NEXT:#define __INT_FAST64_MAX__ 9223372036854775807LL
 // WEBASSEMBLY-NEXT:#define __INT_FAST64_TYPE__ long long int
+// WEBASSEMBLY-NEXT:#define __INT_FAST64_WIDTH__ 64
 // WEBASSEMBLY-NEXT:#define __INT_FAST8_FMTd__ "hhd"
 // WEBASSEMBLY-NEXT:#define __INT_FAST8_FMTi__ "hhi"
 // WEBASSEMBLY-NEXT:#define __INT_FAST8_MAX__ 127
 // WEBASSEMBLY-NEXT:#define __INT_FAST8_TYPE__ signed char
+// WEBASSEMBLY-NEXT:#define __INT_FAST8_WIDTH__ 8
 // WEBASSEMBLY-NEXT:#define __INT_LEAST16_FMTd__ "hd"
 // WEBASSEMBLY-NEXT:#define __INT_LEAST16_FMTi__ "hi"
 // WEBASSEMBLY-NEXT:#define __INT_LEAST16_MAX__ 32767
 // WEBASSEMBLY-NEXT:#define __INT_LEAST16_TYPE__ short
+// WEBASSEMBLY-NEXT:#define __INT_LEAST16_WIDTH__ 16
 // WEBASSEMBLY-NEXT:#define __INT_LEAST32_FMTd__ "d"
 // WEBASSEMBLY-NEXT:#define __INT_LEAST32_FMTi__ "i"
 // WEBASSEMBLY-NEXT:#define __INT_LEAST32_MAX__ 2147483647
 // WEBASSEMBLY-NEXT:#define __INT_LEAST32_TYPE__ int
+// WEBASSEMBLY-NEXT:#define __INT_LEAST32_WIDTH__ 32
 // WEBASSEMBLY-NEXT:#define __INT_LEAST64_FMTd__ "lld"
 // WEBASSEMBLY-NEXT:#define __INT_LEAST64_FMTi__ "lli"
 // WEBASSEMBLY-NEXT:#define __INT_LEAST64_MAX__ 9223372036854775807LL
 // WEBASSEMBLY-NEXT:#define __INT_LEAST64_TYPE__ long long int
+// WEBASSEMBLY-NEXT:#define __INT_LEAST64_WIDTH__ 64
 // WEBASSEMBLY-NEXT:#define __INT_LEAST8_FMTd__ "hhd"
 // WEBASSEMBLY-NEXT:#define __INT_LEAST8_FMTi__ "hhi"
 // WEBASSEMBLY-NEXT:#define __INT_LEAST8_MAX__ 127
 // WEBASSEM

[PATCH] D115235: [clang][dataflow] Implement a basic algorithm for dataflow analysis

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

LGTM but deferring approval to @xazax.hun .




Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:30
+public:
+  void run(const ast_matchers::MatchFinder::MatchResult &Result) override {
+const auto *Func = Result.Nodes.getNodeAs("func");

Add `assert(BlockStates.empty());` ?



Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:58
+std::vector>>
+runAnalysis(const char *Code) {
+  std::unique_ptr AST =

Please prefer StringRef to `const char *` if possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115235

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

[PATCH] D115253: [C2x] Support the *_WIDTH macros in limits.h and stdint.h

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/test/Headers/limits.cpp:33
 
-const bool char_is_signed = (char)-1 < (char)0;
-_Static_assert(CHAR_MIN == (char_is_signed ? -CHAR_MAX-1 : 0), "");
-_Static_assert(CHAR_MAX == (char_is_signed ? -(CHAR_MIN+1) : (char)~0ULL), "");
+_Static_assert(CHAR_MIN == (((char)-1 < (char)0) ? -CHAR_MAX-1 : 0), "");
+_Static_assert(CHAR_MAX == (((char)-1 < (char)0) ? -(CHAR_MIN+1) : 
(char)~0ULL), "");

aaron.ballman wrote:
> erichkeane wrote:
> > Is this change related?  Not sure I get what the change here is.
> The original code would only compile in C++ mode; once I added a C RUN line, 
> it failed because it wasn't a valid constant expression in C. This change 
> makes it a valid constant expression in both C and C++ mode.
Ah, thanks for the clarification!



Comment at: clang/test/Headers/limits.cpp:62
+/* None of these are defined. */
+int BOOL_WIDTH, CHAR_WIDTH, SCHAR_WIDTH, UCHAR_WIDTH, USHRT_WIDTH, SHRT_WIDTH,
+UINT_WIDTH, INT_WIDTH, ULONG_WIDTH, LONG_WIDTH, ULLONG_WIDTH, LLONG_WIDTH;

aaron.ballman wrote:
> erichkeane wrote:
> > Hmm... this is somewhat clever... perhaps overly so?  Can you improve the 
> > comment on 61?
> FWIW, this follows the same pattern as what's on line 41. Do you have a 
> suggested improvement (I can make it in both spots)?
Hrmph, so prior art here :(  It annoys me a touch that it ONLY works to ensure 
that they are not defined to something that would be an illegal identifier (so 
accidentally only defining INT_WIDTH and not UINT_WIDTH wouldn't be caught).

BUT I don't have a better idea other than some silliness of 
'#ifdef BOOL_WIDTH
 #error
#endif`

And I'm not sure my relatively minor concerns are worth the effort.


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

https://reviews.llvm.org/D115253

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

[PATCH] D111639: [Sema] check PseudoObject when rebuilding CXXOperatorCallExpr in template instantiation

[Bruno De Fraine via Phabricator via cfe-commits]

brunodf updated this revision to Diff 392429.
brunodf added a comment.

New version of patch to address problems detected by buildbots.

(Will describe in more detail in follow-up comment.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111639

Files:
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/D111639.cpp
  clang/test/SemaCXX/PR51855.cpp
  clang/test/SemaObjCXX/instantiate-property-access.mm

Index: clang/test/SemaObjCXX/instantiate-property-access.mm
===
--- clang/test/SemaObjCXX/instantiate-property-access.mm
+++ clang/test/SemaObjCXX/instantiate-property-access.mm
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -DDEPENDENT -verify %s
 // expected-no-diagnostics
 
 class C {};
@@ -9,6 +10,10 @@
 
 C operator += (C c1, C c2);
 
+C operator++(C c1);
+
+bool operator!(C c1);
+
 enum TextureType { TextureType3D  };
 
 @interface Texture
@@ -16,9 +21,13 @@
 @property  C c;
 @end
 
-template  class Framebuffer {
+template  class Framebuffer {
 public:
-  Texture **color_attachment;  
+#ifdef DEPENDENT
+  T **color_attachment;
+#else
+  Texture **color_attachment;
+#endif
   Framebuffer();
 };
 
@@ -28,8 +37,15 @@
   (void)(color_attachment[0].c == color_attachment[0].c);
   (void)(color_attachment[0].c == 1);
   (void)(1 == color_attachment[0].c);
+  (void)(!color_attachment[0].textureType);
+  ++color_attachment[0].textureType;
+  (void)(!color_attachment[0].c);
 }
 
 void foo() {
+#ifdef DEPENDENT
+  Framebuffer();
+#else
   Framebuffer();
+#endif
 }
Index: clang/test/SemaCXX/PR51855.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/PR51855.cpp
@@ -0,0 +1,71 @@
+// RUN: %clang_cc1 -S -triple %itanium_abi_triple -fms-extensions -emit-llvm %s -o - | FileCheck %s
+
+struct F {};
+
+F operator*=(F &lhs, int rhs);
+
+F operator++(F &lhs);
+
+struct S {
+  short _m;
+  S(short _m) : _m(_m) {}
+
+  void putM(short rhs) { _m = rhs; }
+  short getM() { return _m; }
+
+  __declspec(property(get = getM, put = putM)) short theData;
+};
+
+int test1a(int i) {
+  S tmp(i);
+  tmp.theData *= 2;
+  return tmp.theData;
+}
+
+// CHECK-LABEL: define {{.*}} @_Z6test1ai(
+// CHECK: call {{.*}} @_ZN1SC1Es(
+// CHECK: call {{.*}} @_ZN1S4getMEv(
+// CHECK: call {{.*}} @_ZN1S4putMEs(
+// CHECK: call {{.*}} @_ZN1S4getMEv(
+
+template 
+int test1b(int i) {
+  T tmp(i);
+  tmp.theData *= 2;
+  return tmp.theData;
+}
+
+template int test1b(int);
+
+// CHECK-LABEL: define {{.*}} @_Z6test1bI1SEii(
+// CHECK: call {{.*}} @_ZN1SC1Es(
+// CHECK: call {{.*}} @_ZN1S4getMEv(
+// CHECK: call {{.*}} @_ZN1S4putMEs(
+// CHECK: call {{.*}} @_ZN1S4getMEv(
+
+int test2a(int i) {
+  S tmp(i);
+  ++tmp.theData;
+  return tmp.theData;
+}
+
+// CHECK-LABEL: define {{.*}} i32 @_Z6test2ai(
+// CHECK: call {{.*}} @_ZN1SC1Es(
+// CHECK: call {{.*}} @_ZN1S4getMEv(
+// CHECK: call {{.*}} @_ZN1S4putMEs(
+// CHECK: call {{.*}} @_ZN1S4getMEv(
+
+template 
+int test2b(int i) {
+  T tmp(i);
+  ++tmp.theData;
+  return tmp.theData;
+}
+
+template int test2b(int);
+
+// CHECK-LABEL: define {{.*}} i32 @_Z6test2bI1SEii(
+// CHECK: call {{.*}} @_ZN1SC1Es(
+// CHECK: call {{.*}} @_ZN1S4getMEv(
+// CHECK: call {{.*}} @_ZN1S4putMEs(
+// CHECK: call {{.*}} @_ZN1S4getMEv(
Index: clang/test/SemaCXX/D111639.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/D111639.cpp
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+// Test case for an issue that appeared while developing D111639:
+//
+// An earlier version of the diff unexpectedly caused compilation of
+// llvm/Support/AllocatorBase.h and llvm/Support/Allocator.h to fail in
+// multistage buildbots.  This is a reduced version of the construct that
+// caused the problem.
+
+using size_t = __SIZE_TYPE__;
+
+struct X {
+  int i;
+};
+
+X operator&(X, X); // binary operator& needed to fill (non-member) overload set of unary operator&
+
+template 
+class Base {
+public:
+  void *Alloc(size_t Size) {
+static_assert(
+static_cast(&Base::Alloc) != static_cast(&Derived::Alloc),
+"must override");
+return static_cast(this)->Alloc(Size);
+  }
+  template 
+  T *Alloc() {
+return static_cast(Alloc(sizeof(T)));
+  }
+};
+
+class Sub : public Base {
+public:
+  void *Alloc(int);
+  void *Alloc(size_t Size) {
+return Alloc((int)Size);
+  }
+  using Base::Alloc;
+};
+
+Sub s;
+
+void *test() {
+  return s.Alloc((size_t)123);
+}
+
+char *test2() {
+  return s.Alloc();
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -14630,18 +14630,37 @@
   Expr *Callee = OrigCallee->IgnoreParenCasts();
   bool isPostIncDec = Second

[PATCH] D114522: [clangd] Add desugared type to hover

[liu hui via Phabricator via cfe-commits]

lh123 added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:1271
 OS << " = " << *P.Default;
+  if (P.Type && P.Type->AKA)
+OS << llvm::formatv(" (aka {0})", *P.Type->AKA);

sammccall wrote:
> kadircet wrote:
> > sammccall wrote:
> > > Hmm, this seems to render as:
> > > 
> > > ```std::string Foo = "" (aka basic_string)```
> > > 
> > > it's not *completely* clear what the aka refers to.
> > > I don't have a better solution, though.
> > > @kadircet any opinions here?
> > i think it's attached to the type of the parameter, not it's name nor the 
> > initializer-expr. so I think we should move this closer to the type (i.e. 
> > `std::string (aka basic_sting) Foo = ""`, basically 
> > `llvm::toString(P.Type)` ?)
> > i think it's attached to the type of the parameter
> 
> This is logically correct but I think it's harder to read. This puts text in 
> the middle of code, in a way that doesn't seem obvious to me: parens mean 
> several things in C++ and it may be hard to recognize this means none of them.
> 
> Worst case is we have function types: `word(*)() (aka long(*)()) x = nullptr`
> 
> It also disrupts the reading flow in the case where the aka is *not* needed 
> for understanding.
> I think overall the previous version was better, though not great.
> 
> I'm tempted to say we should scope down this patch further until we have a 
> better feel for how it behaves, i.e. exclude param types from aka for now. 
> Param types are less obviously useful to disambiguate than result types. 
> (e.g. because in most cases you can hover over the input expression).
> I'm tempted to say we should scope down this patch further until we have a 
> better feel for how it behaves, i.e. exclude param types from aka for now. 
> Param types are less obviously useful to disambiguate than result types. 
> (e.g. because in most cases you can hover over the input expression).

I‘d like to keep the `a.k.a` type for parameter. because: 
1. sometime we pass `literal (or null pointer)` as parameter, but clang doesn't 
support hover on literal. eg:
```
using mint = int;
void foo(mint *);
void code() {
foo(nullptr); // hover over foo, we can get 'mint * (aka int *)'
}
```
2. It may be useful when making function calls, although it does not work well 
when the function is overloaded.**(add a.k.a type to signature help?)** eg:
```
using mint = int;
void foo(mint *);
void code() {
foo(); // hover on foo, we can get 'mint * (aka int *)'
}
```

>  I think overall the previous version was better, though not great.

Agree with that, it seems that putting a.k.a in the middle of the code makes 
the hover look bad based on my recent use


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114522

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

[clang] a18632a - Add diagnostic groups for attribute extensions

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2021-12-07T11:49:53-05:00
New Revision: a18632adc884397e0cd7f27bdb8ea3a822b63000

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

LOG: Add diagnostic groups for attribute extensions

Some users have a need to control attribute extension diagnostics
independent of other extension diagnostics. Consider something like use
of [[nodiscard]] within C++11:
```
[[nodiscard]]
int f();
```
If compiled with -Wc++17-extensions enabled, this will produce warning:
use of the 'nodiscard' attribute is a C++17 extension. This diagnostic
is correct -- using [[nodiscard]] in C++11 mode is a C++17 extension.
And the behavior of __has_cpp_attribute(nodiscard) is also correct --
we support [[nodiscard]] in C++11 mode as a conforming extension. But
this makes use of -Werror or -pedantic-errors` builds more onerous.

This patch adds diagnostic groups for attribute extensions so that
users can selectively disable attribute extension diagnostics. I
believe this is preferable to requiring users to specify additional
flags because it means -Wc++17-extensions continues to be the way we
enable all C++17-related extension diagnostics. It would be quite easy
for someone to use that flag thinking they're protected from some
portability issues without realizing it skipped attribute extensions if
we went the other way.

This addresses PR33518.

Added: 
clang/test/SemaCXX/attr-extension-diags.cpp

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5ca1e1e2a8898..6b0ce5bfd595a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -53,6 +53,11 @@ Improvements to Clang's diagnostics
 
 - -Wbitwise-instead-of-logical (part of -Wbool-operation) warns about use of 
bitwise operators with boolean operands which have side effects.
 
+- Added diagnostic groups to control diagnostics for attribute extensions by
+  adding groups ``-Wc++N-attribute-extensions`` (where ``N`` is the standard
+  release being diagnosed against). These new groups are automatically implied
+  when passing ``-Wc++N-extensions``. Resolves PR33518.
+
 Non-comprehensive list of changes in this release
 -
 

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 22cd9344d1935..c0642efaee4eb 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1046,6 +1046,13 @@ def : DiagGroup<"unused-local-typedefs", 
[UnusedLocalTypedef]>;
 def NonGCC : DiagGroup<"non-gcc",
 [SignCompare, Conversion, LiteralRange]>;
 
+def CXX14Attrs : DiagGroup<"c++14-attribute-extensions">;
+def CXX17Attrs : DiagGroup<"c++17-attribute-extensions">;
+def CXX20Attrs : DiagGroup<"c++20-attribute-extensions">;
+def FutureAttrs : DiagGroup<"future-attribute-extensions", [CXX14Attrs,
+CXX17Attrs,
+CXX20Attrs]>;
+
 // A warning group for warnings about using C++11 features as extensions in
 // earlier C++ versions.
 def CXX11 : DiagGroup<"c++11-extensions", [CXX11ExtraSemi, 
CXX11InlineNamespace,
@@ -1053,15 +1060,15 @@ def CXX11 : DiagGroup<"c++11-extensions", 
[CXX11ExtraSemi, CXX11InlineNamespace,
 
 // A warning group for warnings about using C++14 features as extensions in
 // earlier C++ versions.
-def CXX14 : DiagGroup<"c++14-extensions", [CXX14BinaryLiteral]>;
+def CXX14 : DiagGroup<"c++14-extensions", [CXX14BinaryLiteral, CXX14Attrs]>;
 
 // A warning group for warnings about using C++17 features as extensions in
 // earlier C++ versions.
-def CXX17 : DiagGroup<"c++17-extensions">;
+def CXX17 : DiagGroup<"c++17-extensions", [CXX17Attrs]>;
 
 // A warning group for warnings about using C++20 features as extensions in
 // earlier C++ versions.
-def CXX20 : DiagGroup<"c++20-extensions", [CXX20Designator]>;
+def CXX20 : DiagGroup<"c++20-extensions", [CXX20Designator, CXX20Attrs]>;
 
 // A warning group for warnings about using C++2b features as extensions in
 // earlier C++ versions.

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 640286d684e86..2ab99f5f4c9e2 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8626,11 +8626,11 @@ def warn_unused_volatile : Warning<
   InGroup>;
 
 def ext_cxx14_attr : Extension<
-  "use of the %0 attribute is a C++14 extension">, InGroup;
+  "use of the %0 attribute is a C++14 extension">

[PATCH] D113115: Add diagnostic groups for attribute extensions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

I've committed (with a release note) in 
a18632adc884397e0cd7f27bdb8ea3a822b63000 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113115

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

[PATCH] D112616: Fix crash on invalid code involving late parsed inline methods

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D112616#3106436 , @aaron.ballman 
wrote:

> Ping

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112616

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

[PATCH] D112616: Fix crash on invalid code involving late parsed inline methods

[Erich Keane via Phabricator via cfe-commits]

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

I'm about as confident as _I_ could be on this one, so between that and the 
time, I think my LGTM now carries a bit of weight.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112616

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

[PATCH] D114665: [clangd] Make a.k.a printing configurable.

[liu hui via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/ConfigFragment.h:271
+  /// Describes hover preferences.
+  struct HoverBlock {
+/// Whether hover show a.k.a type.

sammccall wrote:
> One question is whether the setting should control hover specifically, or 
> whether it covers "in places we print types" more generally. But it doesn't 
> seem likely we'll make this configurable for diagnostics, and I don't have 
> other examples. Most of our settings are per-feature. So I think this is 
> right as it is.
In the future, the AKA type can also be displayed in the signature help, but I 
don't know the best place for this option.（for now, it should be fine to put 
this setting in hover）


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114665

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

[PATCH] D115228: [Clang] Do not check if we are in a discared context in non-immediate contexts

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM me, I'll land shortly on your behalf.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115228

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

[clang] 2334314 - Do not check if we are in a discared context in non-immediate contexts

[Aaron Ballman via cfe-commits]

Author: Corentin Jabot
Date: 2021-12-07T12:13:35-05:00
New Revision: 2334314550724812bb94e36c6b5df7d665bdbd9d

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

LOG: Do not check if we are in a discared context in non-immediate contexts

This fixes in a regression introduced by 6eeda06c1.

When deducing the return type of nested function calls, only the
return type of the outermost expression should be ignored.

Instead of assuming all contextes nested in a discared statements
are themselves discarded, only assume that in immediate contexts.

Similarly, only consider contextes immediately in an immediate or
discarded statement as being themselves immediate.

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 40c0b549968dc..8182d25295c5f 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1324,12 +1324,15 @@ class Sema final {
 
 bool isImmediateFunctionContext() const {
   return Context == ExpressionEvaluationContext::ImmediateFunctionContext 
||
- InImmediateFunctionContext;
+ (Context == ExpressionEvaluationContext::DiscardedStatement &&
+  InImmediateFunctionContext);
 }
 
 bool isDiscardedStatementContext() const {
   return Context == ExpressionEvaluationContext::DiscardedStatement ||
- InDiscardedStatement;
+ (Context ==
+  ExpressionEvaluationContext::ImmediateFunctionContext &&
+  InDiscardedStatement);
 }
   };
 

diff  --git a/clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp 
b/clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
index ed61119dc252a..72a61874ed038 100644
--- a/clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
+++ b/clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
@@ -159,4 +159,19 @@ a:  if constexpr(sizeof(n) == 4) // expected-error 
{{redefinition}} expected-not
   surprise: {}
   }
 }
+
+namespace deduced_return_type_in_discareded_statement {
+
+template 
+auto a(const T &t) {
+  return t;
+}
+
+void f() {
+  if constexpr (false) {
+a(a(0));
+  }
+}
+} // namespace deduced_return_type_in_discareded_statement
+
 #endif



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

[PATCH] D115228: [Clang] Do not check if we are in a discared context in non-immediate contexts

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Thanks for the quick fix! I've landed on your behalf in 
2334314550724812bb94e36c6b5df7d665bdbd9d 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115228

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

[PATCH] D115199: [WIP][X86][AMX] Support amxpreserve attribute in clang.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:2895
 
+def AMXPreserve : InheritableAttr, TargetSpecificAttr {
+  let Spellings = [GCC<"amxpreserve">, Declspec<"amxpreserve">];

Inherited on what? This attribute needs a `Subjects` list, I suspect, or it 
needs to not be using a `SimpleHandler`.



Comment at: clang/include/clang/Basic/Attr.td:2896
+def AMXPreserve : InheritableAttr, TargetSpecificAttr {
+  let Spellings = [GCC<"amxpreserve">, Declspec<"amxpreserve">];
+  let Documentation = [Undocumented];

Does GCC support this attribute? Or MSVC? I don't see any evidence that either 
compiler supports this, so I think this should be one spelling: `Clang`.



Comment at: clang/include/clang/Basic/Attr.td:2897
+  let Spellings = [GCC<"amxpreserve">, Declspec<"amxpreserve">];
+  let Documentation = [Undocumented];
+  let SimpleHandler = 1;

No new, undocumented attributes please.



Comment at: clang/lib/Sema/SemaType.cpp:7517
 
+  if (attr.getKind() == ParsedAttr::AT_AMXPreserve) {
+if (S.CheckAttrTarget(attr))

Why are there type system changes for a declaration attribute?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115199

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

[PATCH] D115108: [clangd] Print type for VarTemplateDecl in hover.

[liu hui via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG51dc466642c5: [clangd] Print type for VarTemplateDecl in 
hover. (authored by lh123).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115108

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -901,6 +901,35 @@
  HI.Kind = index::SymbolKind::Unknown;
  HI.Type = "int[10]";
  HI.Value = "{1}";
+   }},
+  {// Var template decl
+   R"cpp(
+  using m_int = int;
+
+  template  m_int ^[[arr]][Size];
+ )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "arr";
+ HI.Kind = index::SymbolKind::Variable;
+ HI.Type = "m_int[Size]";
+ HI.NamespaceScope = "";
+ HI.Definition = "template  m_int arr[Size]";
+ HI.TemplateParameters = {{{"int"}, {"Size"}, llvm::None}};
+   }},
+  {// Var template decl specialization
+   R"cpp(
+  using m_int = int;
+
+  template  m_int arr[Size];
+
+  template <> m_int ^[[arr]]<4>[4];
+ )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "arr<4>";
+ HI.Kind = index::SymbolKind::Variable;
+ HI.Type = "m_int[4]";
+ HI.NamespaceScope = "";
+ HI.Definition = "m_int arr[4]";
}}};
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Code);
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -594,6 +594,8 @@
 HI.Type = TTP->wasDeclaredWithTypename() ? "typename" : "class";
   else if (const auto *TTP = dyn_cast(D))
 HI.Type = printType(TTP, PP);
+  else if (const auto *VT = dyn_cast(D))
+HI.Type = printType(VT->getTemplatedDecl()->getType(), PP);
 
   // Fill in value with evaluated initializer if possible.
   if (const auto *Var = dyn_cast(D)) {


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -901,6 +901,35 @@
  HI.Kind = index::SymbolKind::Unknown;
  HI.Type = "int[10]";
  HI.Value = "{1}";
+   }},
+  {// Var template decl
+   R"cpp(
+  using m_int = int;
+
+  template  m_int ^[[arr]][Size];
+ )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "arr";
+ HI.Kind = index::SymbolKind::Variable;
+ HI.Type = "m_int[Size]";
+ HI.NamespaceScope = "";
+ HI.Definition = "template  m_int arr[Size]";
+ HI.TemplateParameters = {{{"int"}, {"Size"}, llvm::None}};
+   }},
+  {// Var template decl specialization
+   R"cpp(
+  using m_int = int;
+
+  template  m_int arr[Size];
+
+  template <> m_int ^[[arr]]<4>[4];
+ )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "arr<4>";
+ HI.Kind = index::SymbolKind::Variable;
+ HI.Type = "m_int[4]";
+ HI.NamespaceScope = "";
+ HI.Definition = "m_int arr[4]";
}}};
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Code);
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -594,6 +594,8 @@
 HI.Type = TTP->wasDeclaredWithTypename() ? "typename" : "class";
   else if (const auto *TTP = dyn_cast(D))
 HI.Type = printType(TTP, PP);
+  else if (const auto *VT = dyn_cast(D))
+HI.Type = printType(VT->getTemplatedDecl()->getType(), PP);
 
   // Fill in value with evaluated initializer if possible.
   if (const auto *Var = dyn_cast(D)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 51dc466 - [clangd] Print type for VarTemplateDecl in hover.

[via cfe-commits]

Author: lh123
Date: 2021-12-08T01:20:14+08:00
New Revision: 51dc466642c5566c289468b269a8c69b0e447720

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

LOG: [clangd] Print type for VarTemplateDecl in hover.

Print type for VarTemplateDecl in hover.

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Hover.cpp 
b/clang-tools-extra/clangd/Hover.cpp
index 76e9f4d243b9..16a7d5832581 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -594,6 +594,8 @@ HoverInfo getHoverContents(const NamedDecl *D, const 
PrintingPolicy &PP,
 HI.Type = TTP->wasDeclaredWithTypename() ? "typename" : "class";
   else if (const auto *TTP = dyn_cast(D))
 HI.Type = printType(TTP, PP);
+  else if (const auto *VT = dyn_cast(D))
+HI.Type = printType(VT->getTemplatedDecl()->getType(), PP);
 
   // Fill in value with evaluated initializer if possible.
   if (const auto *Var = dyn_cast(D)) {

diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp 
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 0d71dcbf8a93..5376750b25b4 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -901,6 +901,35 @@ class Foo {})cpp";
  HI.Kind = index::SymbolKind::Unknown;
  HI.Type = "int[10]";
  HI.Value = "{1}";
+   }},
+  {// Var template decl
+   R"cpp(
+  using m_int = int;
+
+  template  m_int ^[[arr]][Size];
+ )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "arr";
+ HI.Kind = index::SymbolKind::Variable;
+ HI.Type = "m_int[Size]";
+ HI.NamespaceScope = "";
+ HI.Definition = "template  m_int arr[Size]";
+ HI.TemplateParameters = {{{"int"}, {"Size"}, llvm::None}};
+   }},
+  {// Var template decl specialization
+   R"cpp(
+  using m_int = int;
+
+  template  m_int arr[Size];
+
+  template <> m_int ^[[arr]]<4>[4];
+ )cpp",
+   [](HoverInfo &HI) {
+ HI.Name = "arr<4>";
+ HI.Kind = index::SymbolKind::Variable;
+ HI.Type = "m_int[4]";
+ HI.NamespaceScope = "";
+ HI.Definition = "m_int arr[4]";
}}};
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Code);



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

[PATCH] D115243: [clangd] Extend SymbolOrigin, stop serializing it

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

I think we also need to update `index/remote/Server.cpp` && `FileSymbols` (and 
`FileIndex` too).

Regarding updates to `loadIndex`, I actually think it makes sense for that 
index the always retrieve symbols as `Static` origin, then whoever makes use of 
that (we always have either a `FileSymbol` or a complete remote-index 
marshaling on top) should overwrite the origin while either building the final 
merged index or during marshaling. That would get rid of lots of extra plumbing.
WDYT?




Comment at: clang-tools-extra/clangd/index/SymbolOrigin.cpp:17
 return OS << "unknown";
-  constexpr static char Sigils[] = "ADSMIR67";
+  constexpr static char Sigils[] = "AOSMIR67BP012345";
   for (unsigned I = 0; I < sizeof(Sigils); ++I)

`s/P/9` && `s/6/P`



Comment at: clang-tools-extra/clangd/index/SymbolOrigin.h:26
+  Static = 1 << 2, // From a static, externally-built index.
   Merge = 1 << 3,  // A non-trivial index merge was performed.
   Identifier = 1 << 4, // Raw identifiers in file.

do we think this still provides a meaningful signal ?
It only provides value only when multiple indices of same type was involved in 
providing the result (and only that single type of indices). I suppose after 
this patch it can only happen for `SM` (as there are certain remote-index 
implementations that mark themselves as static) or `RM` (well this is not 
possible today, but some day we might have a stack of remote-indices).
As soon as there's a different type of index is involved `M` no longer has any 
meanings, as it's clear that there was some sort of merge happening (or am I 
missing something obvious?)

---

Before this patch situation is a little bit different since `FileIndex` just 
said `D` for both main file and preamble, but that's changing.



Comment at: clang-tools-extra/clangd/index/SymbolOrigin.h:30
+  Preamble = 1 << 6,   // From the dynamic index of preambles.
+   // 7 reserved
+  Background = 1 << 8, // From the automatic project index.

thanks for remembering that! :D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115243

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

[PATCH] D114394: Compile-time computation of string attribute hashes

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

*comment removed, I've been doing more detailed benchmark that imply a rework 
of the patch*


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

https://reviews.llvm.org/D114394

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

[PATCH] D111100: enable plugins for clang-tidy

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D00#3162293 , @vtjnash wrote:

> There is clearly some more work to do to get the cmake file to be correct, 
> but was hoping to check this looked like the direction you thought looked 
> right for adding this test, since there isn't an obvious example to follow.

My familiarity with plugins is pretty limited (I'm on Windows, where plugins 
don't work at all), but this looks like a reasonable direction to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D00

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

[PATCH] D115140: [ARM][clang] Option b-key must not affect __ARM_FEATURE_PAC_DEFAULT

[Ties Stuij via Phabricator via cfe-commits]

stuij added a comment.

@danielkiss Yes that needs to be addressed, but we are doing that in another 
patch that will (hopefully) cover all permutations of architecture and branch 
protection values on both the cmdline and as function attributes.

So this patch is just about not having logic in the ARM code that shouldn't be 
there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115140

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

[PATCH] D114665: [clangd] Make a.k.a printing configurable.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/ConfigFragment.h:271
+  /// Describes hover preferences.
+  struct HoverBlock {
+/// Whether hover show a.k.a type.

lh123 wrote:
> sammccall wrote:
> > One question is whether the setting should control hover specifically, or 
> > whether it covers "in places we print types" more generally. But it doesn't 
> > seem likely we'll make this configurable for diagnostics, and I don't have 
> > other examples. Most of our settings are per-feature. So I think this is 
> > right as it is.
> In the future, the AKA type can also be displayed in the signature help, but 
> I don't know the best place for this option.（for now, it should be fine to 
> put this setting in hover）
we've got a `Style` section actually, which might be more suitable for 
extensibility but I am also afraid of pushing ourselves into a corner by 
putting too much meaning into a boolean flag.

maybe we should just go with a command line flag until we figure out what to do 
here (as it's less invasive)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114665

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

[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

>> So, the question is -- what's the right way to specify something like this 
>> in a consistent manner? 
>> `--offload` option proposed here does not seem to be a good fit. It was 
>> intended as a more flexible way to create a single `-cc1` sub-compilation 
>> and we're doing quite a bit more here.
>
> Does `--offload-arch=spirv*` fit better here? If I understand the goal of 
> this patch correctly, it tries to provide controls for changing offload 
> target for HIP application from default (AMDGCN) to SPIR-V.

`--offload-arch=` only accepts GPU arch which is translated to processor option 
(-mcpu= or -march=) in clang -cc1. spirv is a target triple which is not 
suitable for `--offload-arch=`.

`--offload=` is supposed to cover both target triple and processor with some 
flexibility. If only target triple is specified, it assumes default processor. 
If only processor is specified, it deduces target triple. It also allows both 
triple and processor. In this case, `--offload=spirv` translates to -triple 
spirv -mcpu=generic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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

[PATCH] D114957: [AMDGPU] Change llvm.amdgcn.image.bvh.intersect.ray to take vec3 args

[Jay Foad via Phabricator via cfe-commits]

foad abandoned this revision.
foad added a comment.

Abandoned in favour of D115032 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114957

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

[PATCH] D115235: [clang][dataflow] Implement a basic algorithm for dataflow analysis

[Gábor Horváth via Phabricator via cfe-commits]

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

This patch looks good to me, most of my comments are things to consider in 
follow-up patches.




Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:32
+TypeErasedDataflowAnalysisState computeBlockInputState(
+std::vector> &BlockStates,
+const CFGBlock &Block, const Environment &InitEnv,

Alternatively, if we have a bottom, `forall e in lattice join(e, bottom) == e`, 
so we do not really need optionals and can express nodes that do not contribute 
to the state using the bottom value. (Or using top, depending on the 
orientation for our lattices, unfortunately the literature is using both 
orientations which is really confusing.)

I'm fine with the current approach, but I'd love to see a function level 
comment about `nullopt`s representing nodes that were not yet evaluated.



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:35
+TypeErasedDataflowAnalysis &Analysis) {
+  TypeErasedDataflowAnalysisState State = {Analysis.typeErasedInitialElement(),
+   InitEnv};

I did not catch this earlier, but I wonder if we should pass the block in to 
`typeErasedInitialElement`. There are some analysis where the initial element 
might be different for certain nodes. E.g. here is the algorithms for computing 
dominators from wikipedia:
```
 // dominator of the start node is the start itself
 Dom(n0) = {n0}
 // for all other nodes, set all nodes as the dominators
 for each n in N - {n0}
 Dom(n) = N;
 // iteratively eliminate nodes that are not dominators
 while changes in any Dom(n)
 for each n in N - {n0}:
 Dom(n) = {n} union with intersection over Dom(p) for all p in pred(n)
```

Here the initial analysis state for the entry node differs from the initial 
state for the rest of the nodes. 



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:69
+  for (const CFGElement &Element : Block) {
+const llvm::Optional Stmt = Element.getAs();
+if (!Stmt.hasValue())

I think this is fine for now, but in general, `CFGStmt` is probably not the 
only kind of `CFGElement` that we want to evaluate.

E.g., `CFGImplicitDtor` or `CFGLifetimeEnds` might also be interesting in the 
future if we want to find certain problems, like dereferencing an invalid 
pointer. I'd love to see a FIXME.



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:104
+  // limit the number of iterations.
+  // FIXME: Consider making the maximum number of iterations configurable.
+  unsigned Iterations = 0;

I'd strongly suggest setting up some statistics in the future: 
https://llvm.org/docs/ProgrammersManual.html#the-statistic-class-stats-option

Having some counters, like how many function timed out, what is the average 
iteration count and so on could be very handy to monitor the performance of the 
analysis before and after some changes (both for client analyses or for the 
framework itself).



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:134
+Worklist.enqueueSuccessors(Block);
+  }
+

If a block still has `None` as its state at this point, it is unreachable in 
the CFG. One option is to ignore them, as we do at the moment and it is 
completely fine and should not change this patch. But for some analysis in the 
future, we might want to check dead code as well (it might be dead for the 
current platform due to some preprocessor defines but alive for some other). 
E.g., it would also be nice to diagnose a null dereference in a block that was 
not reached. 



Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:37-40
+CFG::BuildOptions Options;
+Options.AddImplicitDtors = true;
+Options.AddTemporaryDtors = true;
+Options.setAllAlwaysAdd();

I wonder if the dataflow analysis framework should provide a factory that that 
returns a `CFG::BuildOptions` which is a good default for most clients. Can be 
in a follow-up patch, or completely ignored. Or maybe even a convenience 
function running the analysis on a function definition without the user having 
to know about the CFG.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115235

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

[PATCH] D115254: Revert "Revert "Use VersionTuple for parsing versions in Triple, fixing issues that caused the original change to be reverted. This makes it possible to distinguish between "16" and "

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D115254#3176564 , @jamesfarrell 
wrote:

> Only change from previous attempt is to call rtrim() on the output of the 
> commands in the unit tests, since the new version parsing code fails if there 
> are leftover characters like \n.

My only comment is on the commit message, which ideally would be a bit easier 
to get information from:

- the summary line is very very long; can you cut it down, and put more in the 
body?
- it'd be great to link directly to the commit that's being reapplied as well 
(both the most recent and the original)

E.g., something like this, but feel free to reword / add more info / etc:

  Reapply "Use VersionTuple for parsing versions in Triple" (v3)
  
  Revert SHA1-1, reapplying SHA1-2 after fixing unit tests. The only change is
  to call rtrim() on the output of the commands in the unit tests, since the new
  version parsing code fails if there are leftover characters like `\n`.
  
  The original commit message (from SHA1-3) follows:
  
  Use VersionTuple for parsing versions in Triple. This makes it possible to 
distinguish between "16" and "16.0" after parsing, which previously was not 
possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115254

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

[PATCH] D114908: [clang] Don't call inheritDefaultTemplateArguments() on CXXDeductionGuideDecl's template parameters

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114908

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

[PATCH] D115103: Leak Sanitizer port to Windows

[Clemens Wasser via Phabricator via cfe-commits]

clemenswasser updated this revision to Diff 392444.
clemenswasser added a comment.

Split this patch up into multiple smaller ones:

- D115186 
- D115204 
- D115262 


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

https://reviews.llvm.org/D115103

Files:
  clang/lib/Driver/ToolChains/MSVC.cpp
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/lsan/CMakeLists.txt
  compiler-rt/lib/lsan/lsan.h
  compiler-rt/lib/lsan/lsan_common.cpp
  compiler-rt/lib/lsan/lsan_common.h
  compiler-rt/lib/lsan/lsan_common_win.cpp
  compiler-rt/lib/lsan/lsan_win.cpp
  compiler-rt/lib/lsan/lsan_win.h
  compiler-rt/test/lsan/lit.common.cfg.py

Index: compiler-rt/test/lsan/lit.common.cfg.py
===
--- compiler-rt/test/lsan/lit.common.cfg.py
+++ compiler-rt/test/lsan/lit.common.cfg.py
@@ -79,7 +79,8 @@
 supported_linux = (not config.android) and config.host_os == 'Linux' and config.host_arch in ['aarch64', 'x86_64', 'ppc64', 'ppc64le', 'mips64', 'riscv64', 'arm', 'armhf', 'armv7l', 's390x']
 supported_darwin = config.host_os == 'Darwin' and config.target_arch in ['x86_64']
 supported_netbsd = config.host_os == 'NetBSD' and config.target_arch in ['x86_64', 'i386']
-if not (supported_android or supported_linux or supported_darwin or supported_netbsd):
+supported_windows = config.host_os == 'Windows'
+if not (supported_android or supported_linux or supported_darwin or supported_netbsd or supported_windows):
   config.unsupported = True
 
 # Don't support Thumb due to broken fast unwinder
Index: compiler-rt/lib/lsan/lsan_win.h
===
--- /dev/null
+++ compiler-rt/lib/lsan/lsan_win.h
@@ -0,0 +1,41 @@
+//=-- lsan_win.h -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===-===//
+//
+// This file is a part of LeakSanitizer.
+// Standalone LSan RTL code common to Windows systems.
+//
+//===-===//
+
+#ifndef LSAN_WINDOWS_H
+#define LSAN_WINDOWS_H
+
+#include "lsan_thread.h"
+#include "sanitizer_common/sanitizer_platform.h"
+
+#if !SANITIZER_WINDOWS
+#  error "lsan_win.h is used only on Windows systems (SANITIZER_WINDOWS)"
+#endif
+
+namespace __sanitizer {
+struct DTLS;
+}
+
+namespace __lsan {
+
+class ThreadContext final : public ThreadContextLsanBase {
+ public:
+  explicit ThreadContext(int tid);
+  void OnStarted(void *arg) override;
+};
+
+void ThreadStart(u32 tid, tid_t os_id,
+ ThreadType thread_type = ThreadType::Regular);
+
+}  // namespace __lsan
+
+#endif  // LSAN_WINDOWS_H
Index: compiler-rt/lib/lsan/lsan_win.cpp
===
--- /dev/null
+++ compiler-rt/lib/lsan/lsan_win.cpp
@@ -0,0 +1,106 @@
+//=-- lsan_win.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===-===//
+//
+// This file is a part of LeakSanitizer.
+// Standalone LSan RTL code common to POSIX-like systems.
+//
+//===-===//
+
+#include "sanitizer_common/sanitizer_platform.h"
+
+#if SANITIZER_WINDOWS
+#  include "lsan.h"
+#  include "lsan_allocator.h"
+#  include "sanitizer_common/sanitizer_stacktrace.h"
+#  include "sanitizer_common/sanitizer_tls_get_addr.h"
+
+namespace __lsan {
+
+ThreadContext::ThreadContext(int tid) : ThreadContextLsanBase(tid) {}
+
+struct OnStartedArgs {
+  uptr stack_begin;
+  uptr stack_end;
+  uptr cache_begin;
+  uptr cache_end;
+  uptr tls_begin;
+  uptr tls_end;
+};
+
+void ThreadContext::OnStarted(void *arg) {
+  auto args = reinterpret_cast(arg);
+  stack_begin_ = args->stack_begin;
+  stack_end_ = args->stack_end;
+  cache_begin_ = args->cache_begin;
+  cache_end_ = args->cache_end;
+}
+
+void ThreadStart(u32 tid, tid_t os_id, ThreadType thread_type) {
+  OnStartedArgs args;
+  uptr stack_size = 0;
+  uptr tls_size = 0;
+  GetThreadStackAndTls(tid == kMainTid, &args.stack_begin, &stack_size,
+   &args.tls_begin, &tls_size);
+  args.stack_end = args.stack_begin + stack_size;
+  args.tls_end = args.tls_begin + tls_size;
+  GetAllocatorCacheRange(&args.cache_begin, &args.cache_end);
+  ThreadContextLsanBase::ThreadStart(tid, os_id, thread_type, &args);
+}
+
+bool GetThreadRangesLocked(tid_t