[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:315-318
+SVal Val = I->second;
+for (auto si = Val.symbol_begin(), se = Val.symbol_end(); si != se; ++si) {
+  SR.markLive(*si);
+}

Yes, this looks correct, thanks!

In the ideal world we'd have checked that `I->first` is live before marking 
`I->second` live. Unfortunately, currently `checkLiveSymbols` is invoked before 
RegionStore's live symbols detection, so we can't rely on that and it's a bug; 
in order to be correct, RegionStore's live symbols detection and checker's live 
symbols callback should engage in fixpoint iteration.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:494-496
+  OS << "Smart pointer";
+  checkAndPrettyPrintRegion(OS, ThisRegion);
+  OS << " is non-null";

Because these note tags aren't specific to the bug report at hand, they have to 
be marked as //prunable// (i.e., the optional `getNoteTag()`'s parameter 
"`IsPrunable`" should be set to true). That'd reduce bug report clutter by not 
bringing in stack frames that aren't otherwise interesting for the bug report.

Something like this may act as a test (i didn't try):
```lang=c++
struct S {
  std::unique_ptr P;

  void foo() {
if (!P) { // no-note because foo() is pruned
  return;
}
  }

  int bar() {
P = new int(0);
foo();
return 1 / *P; // warning: division by zero
  }

}

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86027

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

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

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

balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:882
+  const QualType SizePtrTy = getPointerTy(SizeTy);
+  const QualType SizePtrRestrictTy = getRestrictTy(SizePtrTy);
 

martong wrote:
> balazske wrote:
> > Another idea: Have a class that handles all the variants (simple, pointer, 
> > const pointer, const restrict pointer, restrict). It can have get functions 
> > that compute the type if not done yet, or get every variant at first time 
> > even if it is later not used (or has no sense).
> > For example create it like `TypeVariants SizeTy{ACtx.getSizeType()};` and 
> > then call `SizeTy.getType()`, `SizeTy.asPtr()`, `SizeTy.asPtrRestrict()`, 
> > ... .
> > ```
> I'd rather keep the current form, because I think it makes it easier to 
> compose types from different base types (and it is similar to the ASTMatchers 
> in this sense).
If I got it correctly, the problem with my approach above is that it has 
separate functions for every special type variant. The current is acceptable 
but looks not fully uniform to me (the type has a `withConst` and for getting a 
pointer or restrict there is a function call). We could have something like a 
"`TypeBuilder`" that has methods `withConst()`, `withRestrict()`, 
`withPointerTo()` (these can return a TypeBuilder too) and finally a 
(omittable) `getType()` that returns `Optional`.
```
const QualType ConstVoidPtrRestrictTy = TypeBuilder{ACtx, 
VoidTy}.withConst().withPointerTo().withRestrict().getType();
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84415

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

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D86465#2235289 , @NoQ wrote:

> If the numbers are confirmed to be as good as what i've sneak-peeked so far, 
> that should be pretty amazing. Also whoa, test coverage!~

I'll add the charts with performance in the next couple of days

> WDYT about moving introducing a generic "`SmallImmutableSet`" in `llvm/ADT` 
> to back your implementation?

Oof, it is not really possible.  First of all, existing `RangeSet` didn't 
really ensure that ranges do not intersect (`add` is/was not checking for 
that), and this is left for the user.  Additionally, it is designed and 
optimized specifically for `RangeSet` queries and operations.  Like `intersect` 
and `contains` in a more generic `Set` sense mean entirely different things.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:88
+  // structure is preferred.
+  using ImplType = llvm::SmallVector;
+

NoQ wrote:
> vsavchenko wrote:
> > grandinj wrote:
> > > Just curious - if they mostly contain 1 or 2 elements, why is N == 4  
> > > here?
> > Essentially, the main factor is **intuition** 😆 
> > The main idea here is that allocations are the single most expensive thing 
> > about ranges, so I want to avoid extra allocations not for 60% of range 
> > sets, but for 90%.  I still should definitely measure performance 
> > difference for different values of N and gather some stats about the 
> > lengths.
> Given that you pass those by pointers, i suspect you don't need to fix the 
> size at all. In fact the small-size of the small vector is, by design, a 
> //runtime// value and you can use `llvm::SmallVectorImpl *` instead, which 
> can point to `SmallVector` of any small-size. This would allow you to 
> allocate small vectors of variable length which you can take advantage of 
> whenever you know the length in advance.
Sounds reasonable, I'll give it a shot!



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:281
+//
+// Shortcut: check that there is even a possibility of the intersection
+//   by checking the two following situations:

NoQ wrote:
> Thanks a lot for adding those comments. These methods were confusing as heck.
😊 



Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:206
+
+  this->checkNegate({{MIN, A}}, {{MIN, MIN}, {D, MAX}});
+  this->checkNegate({{MIN, C}}, {{MIN, MIN}, {B, MAX}});

NoQ wrote:
> Why is it necessary to specify `this->` here?
It is due to shenanigans in test fixtures implementation.  I didn't find a 
better workaround and other tests for other components also use `this->` as far 
as I checked :-(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86465

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

[PATCH] D86065: [SVE] Make ElementCount members private

[David Sherwood via Phabricator via cfe-commits]

david-arm added a comment.

Hi @fpetrogalli, if you don't mind I think I'll stick with Paul's idea for ogt 
because this matches the IR neatly, i.e. "fcmp ogt". Also, for me personally 
it's much simpler and more intuitive.


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

https://reviews.llvm.org/D86065

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

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:119
+
+  return makePersistent(std::move(Result));
+}

NoQ wrote:
> Given that we're certain from the start that the container will be 
> persistent, can we save a copy by directly asking the factory to allocate a 
> fresh container?
> 
> Also this seems to be one of the cases where variable-sized small vectors 
> would make sense.
Surprisingly this is not the case, it is cheaper to do whatever you want on 
small vectors on the stack and check if we already allocated it.
So, it is a deliberate choice to do it this way (you can see it in the comment 
inside of the `makePersistent` function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86465

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

[PATCH] D81930: [AArch64] Add -mmark-bti-property flag.

[Momchil Velikov via Phabricator via cfe-commits]

chill added inline comments.



Comment at: llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.cpp:106
+
+  EmitBTIMarking = MarkBTIProperty.getValue();
 }

No need to the `.getValue()` part.



Comment at: llvm/lib/Target/AArch64/MCTargetDesc/AArch64MCAsmInfo.h:34
   explicit AArch64MCAsmInfoELF(const Triple &T);
+  bool EmitBTIMarking;
 };

Is there a need for this data member? The option value  does not change over 
time, and the option can be defined in `AArch64TargetStreamer.cpp`.


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

https://reviews.llvm.org/D81930

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

[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

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

Szelethus added a comment.

In D86135#2233611 , @martong wrote:

>> The fundamental problem is, we simply can't ask Preprocessor what a macro 
>> expands into without hacking really hard.
>
> Can you summarize what is the exact problem (or give a link to a discussion, 
> etc)? Is it an architectural problem in Clang itself?

I phrased myself poorly. The Preprocessor can tell what a macro usage 
immediately expands into (otherwise this project would have been practically 
impossible to implement), what it struggles to get done is show what a macro 
usage in the source code turns into in the preprocessed translation unit. As is 
often the case, macros may be nested into one another:

  #define MACRO2(ptr) ptr = 0
  #define MACRO1(x) MACRO2(x)
  
  int *a = get();
  MACRO1(a); // Step 1. Expand MACRO1.
 //a.) Retrieve the tokens defined for MACRO1.
 //b.) Resolve parameter x to argument a.
 // Step 2. Expand MACRO2...
  *a = 5;

From this code snippet, should we be interested in what `MACRO1(a)` expands 
into, `Preprocessor` can pretty much only deliver on point 1a.) with any 
convenience. The problem here is that it was simply never engineered to be used 
outside of the preprocessing stage, so much so, that by the time I worked on 
this project about a decade after `Preprocessor`'s inception, I still had to 
turn super trivial methods const. Other than that, the discussions I linked in 
the summary is pretty much all I can offer.

> Could we somehow refactor Clang and the Preprocessor to be usable for us? I 
> mean LLVM and Clang has the mindset to build reusable components, the 
> Preprocessor (and the Sema) should be one of them too, not just the AST.

Working with the `Preprocessor` seems incredibly tedious and error prone -- 
mind that this is almost literally the first thing we implemented in Clang, and 
you can tell. Also, its performance critical, and any meaningful performance 
impact may need strong justification. While it would be beneficial to have the 
knowledge I'm crafting being integrated into the actual class, its hard to 
justify all the effort it would take to do so, especially now this project is 
so far along anyways. If I has to start from scratch, I would try to explore 
other approaches, but might still end up just doing what I did here.

In D86135#2233695 , @xazax.hun wrote:

> Is this related to https://bugs.llvm.org/show_bug.cgi?id=44493?

To some extent, but this patch unfortunately doesn't solve that bug. The 
problem I think is that named variadic macros aren't supported at all.




Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:910
+
+  void injextRange(const ArgTokensTy &Range) {
+TokenRange = Range;

martong wrote:
> typo? `injectRange` ?
Yep.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86135

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

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:882
+  const QualType SizePtrTy = getPointerTy(SizeTy);
+  const QualType SizePtrRestrictTy = getRestrictTy(SizePtrTy);
 

balazske wrote:
> martong wrote:
> > balazske wrote:
> > > Another idea: Have a class that handles all the variants (simple, 
> > > pointer, const pointer, const restrict pointer, restrict). It can have 
> > > get functions that compute the type if not done yet, or get every variant 
> > > at first time even if it is later not used (or has no sense).
> > > For example create it like `TypeVariants SizeTy{ACtx.getSizeType()};` and 
> > > then call `SizeTy.getType()`, `SizeTy.asPtr()`, `SizeTy.asPtrRestrict()`, 
> > > ... .
> > > ```
> > I'd rather keep the current form, because I think it makes it easier to 
> > compose types from different base types (and it is similar to the 
> > ASTMatchers in this sense).
> If I got it correctly, the problem with my approach above is that it has 
> separate functions for every special type variant. The current is acceptable 
> but looks not fully uniform to me (the type has a `withConst` and for getting 
> a pointer or restrict there is a function call). We could have something like 
> a "`TypeBuilder`" that has methods `withConst()`, `withRestrict()`, 
> `withPointerTo()` (these can return a TypeBuilder too) and finally a 
> (omittable) `getType()` that returns `Optional`.
> ```
> const QualType ConstVoidPtrRestrictTy = TypeBuilder{ACtx, 
> VoidTy}.withConst().withPointerTo().withRestrict().getType();
> ```
> 
Yeah, `.withConst()` is making it nonuniform currently, you are right.
What if we had `getConstTy()` that receives an optional? Composability would be 
just perfect in this case :)
```
Optional ConstPthread_attr_tPtrTy = 
getPointerTy(getConstTy(Pthread_attr_tTy));
```
WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84415

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

[PATCH] D86427: Fix some spelling errors

[YangZhihui via Phabricator via cfe-commits]

YangZhihui added a comment.

Thanks, but I don't have commit access


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86427

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

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:88
+  // structure is preferred.
+  using ImplType = llvm::SmallVector;
+

vsavchenko wrote:
> NoQ wrote:
> > vsavchenko wrote:
> > > grandinj wrote:
> > > > Just curious - if they mostly contain 1 or 2 elements, why is N == 4  
> > > > here?
> > > Essentially, the main factor is **intuition** 😆 
> > > The main idea here is that allocations are the single most expensive 
> > > thing about ranges, so I want to avoid extra allocations not for 60% of 
> > > range sets, but for 90%.  I still should definitely measure performance 
> > > difference for different values of N and gather some stats about the 
> > > lengths.
> > Given that you pass those by pointers, i suspect you don't need to fix the 
> > size at all. In fact the small-size of the small vector is, by design, a 
> > //runtime// value and you can use `llvm::SmallVectorImpl *` instead, which 
> > can point to `SmallVector` of any small-size. This would allow you to 
> > allocate small vectors of variable length which you can take advantage of 
> > whenever you know the length in advance.
> Sounds reasonable, I'll give it a shot!
On the second thought, it's not going to be pretty.  I still will need to 
allocate `SmallVector<..., N>`, where `N` is a compile-time constant, so it 
would be a big old switch where I "convert" run-time values into compile-time 
values so-to-speak.

Additionally, the involvement of `SmallVector` doesn't seem to be necessary at 
all because in this setting we don't use the main feature of it that it can 
grow larger than this size onto the heap.  So, we can simply do the same trick 
with `ArrayRef` and `std::array`, but with less ambiguity.

However, one of the benefits of the current approach is "let's allocate as 
little as possible".  It does the operation, checks if we allocated space for a 
container with these contents, and allocates it only if not.  It is MUCH 
cheaper to do the operation and find out that we already have one like this or 
copy it to the newly allocated place than allocating it every time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86465

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

[PATCH] D86427: Fix some spelling errors

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

In D86427#2235574 , @YangZhihui wrote:

> Thanks, but I don't have commit access

I can commit it for you. Can you give me your github email?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86427

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

[PATCH] D86427: Fix some spelling errors

[YangZhihui via Phabricator via cfe-commits]

YangZhihui added a comment.

Thanks,
Full Name:  Yang Zhihui
Email: yangzh.f...@cn.fujitsu.com


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86427

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

[PATCH] D86427: Fix some spelling errors

[YangZhihui via Phabricator via cfe-commits]

YangZhihui added a comment.

how can I add the Author property in the patch 
Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86427

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

[PATCH] D86514: Correctly parse LateParsedTemplates in case of multiple dependent modules

[Vaibhav Garg via Phabricator via cfe-commits]

gargvaibhav64 created this revision.
gargvaibhav64 added reviewers: rsmith, v.g.vassilev.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
gargvaibhav64 requested review of this revision.

While parsing LateParsedTemplates, Clang assumes that the Global DeclID matches 
with the Local DeclID of a Decl. This is not the case when we have multiple 
dependent modules have their own LateParsedTemplate section. In such a case, a 
Local/Global DeclID confusion occurs which leads to improper casting of 
Functions.

This commit creates a MapVector to map the LateParsedTemplate section of each 
Module with their module file and therefore resolving the Global/Local DeclID 
confusion.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86514

Files:
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/Serialization/ASTReader.cpp


Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -3721,6 +3721,9 @@
 
 case LATE_PARSED_TEMPLATE:
   LateParsedTemplates.append(Record.begin(), Record.end());
+  LateParsedTemplatesModulesMap.insert(
+  std::make_pair(&F, std::move(LateParsedTemplates)));
+  LateParsedTemplates.clear();
   break;
 
 case OPTIMIZE_PRAGMA_OPTIONS:
@@ -8386,25 +8389,28 @@
 void ASTReader::ReadLateParsedTemplates(
 llvm::MapVector>
 &LPTMap) {
-  for (unsigned Idx = 0, N = LateParsedTemplates.size(); Idx < N;
-   /* In loop */) {
-FunctionDecl *FD = cast(GetDecl(LateParsedTemplates[Idx++]));
+  for (auto &LPT : LateParsedTemplatesModulesMap) {
+ModuleFile *FMod = LPT.first;
+SmallVector LateParsed(LPT.second);
+for (unsigned Idx = 0, N = LateParsed.size(); Idx < N;
+ /* In loop */) {
+  FunctionDecl *FD =
+  cast(GetLocalDecl(*FMod, LateParsed[Idx++]));
 
-auto LT = std::make_unique();
-LT->D = GetDecl(LateParsedTemplates[Idx++]);
+  auto LT = llvm::make_unique();
+  LT->D = GetLocalDecl(*FMod, LateParsed[Idx++]);
 
-ModuleFile *F = getOwningModuleFile(LT->D);
-assert(F && "No module");
+  ModuleFile *F = getOwningModuleFile(LT->D);
+  assert(F && "No module");
 
-unsigned TokN = LateParsedTemplates[Idx++];
-LT->Toks.reserve(TokN);
-for (unsigned T = 0; T < TokN; ++T)
-  LT->Toks.push_back(ReadToken(*F, LateParsedTemplates, Idx));
+  unsigned TokN = LateParsed[Idx++];
+  LT->Toks.reserve(TokN);
+  for (unsigned T = 0; T < TokN; ++T)
+LT->Toks.push_back(ReadToken(*F, LateParsed, Idx));
 
-LPTMap.insert(std::make_pair(FD, std::move(LT)));
+  LPTMap.insert(std::make_pair(FD, std::move(LT)));
+}
   }
-
-  LateParsedTemplates.clear();
 }
 
 void ASTReader::LoadSelector(Selector Sel) {
Index: clang/include/clang/Serialization/ASTReader.h
===
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -903,6 +903,10 @@
   // A list of late parsed template function data.
   SmallVector LateParsedTemplates;
 
+  // A list of LateParsedTemplates paired with their module files.
+  llvm::MapVector>
+  LateParsedTemplatesModulesMap;
+
   /// The IDs of all decls to be checked for deferred diags.
   ///
   /// Sema tracks these to emit deferred diags.


Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -3721,6 +3721,9 @@
 
 case LATE_PARSED_TEMPLATE:
   LateParsedTemplates.append(Record.begin(), Record.end());
+  LateParsedTemplatesModulesMap.insert(
+  std::make_pair(&F, std::move(LateParsedTemplates)));
+  LateParsedTemplates.clear();
   break;
 
 case OPTIMIZE_PRAGMA_OPTIONS:
@@ -8386,25 +8389,28 @@
 void ASTReader::ReadLateParsedTemplates(
 llvm::MapVector>
 &LPTMap) {
-  for (unsigned Idx = 0, N = LateParsedTemplates.size(); Idx < N;
-   /* In loop */) {
-FunctionDecl *FD = cast(GetDecl(LateParsedTemplates[Idx++]));
+  for (auto &LPT : LateParsedTemplatesModulesMap) {
+ModuleFile *FMod = LPT.first;
+SmallVector LateParsed(LPT.second);
+for (unsigned Idx = 0, N = LateParsed.size(); Idx < N;
+ /* In loop */) {
+  FunctionDecl *FD =
+  cast(GetLocalDecl(*FMod, LateParsed[Idx++]));
 
-auto LT = std::make_unique();
-LT->D = GetDecl(LateParsedTemplates[Idx++]);
+  auto LT = llvm::make_unique();
+  LT->D = GetLocalDecl(*FMod, LateParsed[Idx++]);
 
-ModuleFile *F = getOwningModuleFile(LT->D);
-assert(F && "No module");
+  ModuleFile *F = getOwningModuleFile(LT->D);
+  assert(F && "No module");
 
-unsigned TokN = LateParsedTemplates[Idx++];
-LT->Toks.reserve(TokN);
-for (unsigned T = 0; T < TokN; 

[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

[Balázs Benics via Phabricator via cfe-commits]

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

This preprocessor expansion stuff is definitely not my expertise, nvm here is 
my review.

However, I observed some strange things happening in the test-cases, that's why 
I //request changes//.
I hope that I messed something up at the evaluation of your tests, but please 
have a closer look at them.

There were a few nits, but nothing serious.
I also liked the previous discussion about the reusable components, and I 
understand your decision going this way. I'm fine with that.
Keep up your good work. We need this.




Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:883
 
+/// Wrapper around a Lexer object that can lex tokens one-by-one. Optionally,
+/// one can "inject" a range of tokens into the stream, in which case the next

`Optionally` -> `Additionally`?



Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:892-894
+std::pair LocInfo = SM.getDecomposedLoc(ExpanLoc);
+const llvm::MemoryBuffer *MB = SM.getBuffer(LocInfo.first);
+const char *MacroNameTokenPos = MB->getBufferStart() + LocInfo.second;

It should be more readable if you use `tie`. That way you can give names to the 
parts.



Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:896-898
+RawLexer.reset(new Lexer(SM.getLocForStartOfFile(LocInfo.first), LangOpts,
+ MB->getBufferStart(), MacroNameTokenPos,
+ MB->getBufferEnd()));

I'm always puzzled if I see a naked `new`.
Couldn't we use the assignment operator and `std::make_unique` here?



Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:901
+
+  void next(Token &Result) {
+if (CurrTokenIt == TokenRange.end()) {

I don't like output parameters.
If we really need them, we should at least have a suspicious name.
Unfortunately, I can not come up with anything useful :|



Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:1150-1161
   // Acquire the macro's arguments at the expansion point.
   //
   // The rough idea here is to lex from the first left parentheses to the last
   // right parentheses, and map the macro's parameter to what they will be
   // expanded to. A macro argument may contain several token (like '3 + 4'), so
   // we'll lex until we find a tok::comma or tok::r_paren, at which point we
   // start lexing the next argument or finish.

By //lexing// one might think we use the actual lexer. Should we change this 
comment?



Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:1338-1339
 void TokenPrinter::printToken(const Token &Tok) {
+  // TODO: Handle the case where hash and hashhash occurs right before
+  // __VA_ARGS__.
+

What does //hashhash// mean? I might lack some context though :D



Comment at: clang/test/Analysis/plist-macros-with-expansion.cpp:481
+  i = 0;
+#define DISPATCH(...) PARAMS_RESOLVE_TO_VA_ARGS(__VA_ARGS__);
+

You don't need an ending semicolon here. It will be already there at the 
expansion location.
This way you introduce an empty expression after the macro expansion.
The same happens in all the other cases as well.



Comment at: clang/test/Analysis/plist-macros-with-expansion.cpp:484-486
+  int x = 1;
+  DISPATCH(x, "LF1M healer");
+  (void)(10 / x); // expected-warning{{Division by zero}}

Should we really abuse the division by zero checker here?
Can't we just use an ExprInspection call here?
Maybe it requires a specific BugPath visitor, and that is why we do it this way?



Comment at: clang/test/Analysis/plist-macros-with-expansion.cpp:511
+// CHECK: nameCONCAT_VA_ARGS
+// CHECK-NEXT: expansionvariadicCFunction(x, "You need 
to construct",);x = 0;
+

How did that comma appear there?
https://godbolt.org/z/4En3E5



Comment at: clang/test/Analysis/plist-macros-with-expansion.cpp:517-524
+void stringifyVA_ARGS(void) {
+  int x = 1;
+  STRINGIFIED_VA_ARGS(x, "Additional supply depots required.", 'a', 10);
+  (void)(10 / x); // expected-warning{{Division by zero}}
+}
+
+// CHECK: nameSTRINGIFIED_VA_ARGS

This test case is also bad.
https://godbolt.org/z/89s48K



Comment at: clang/test/Analysis/plist-macros-with-expansion.cpp:526-533
+void stringifyVA_ARGSEmpty(void) {
+  int x = 1;
+  STRINGIFIED_VA_ARGS(x, "Additional supply depots required.");
+  (void)(10 / x); // expected-warning{{Division by zero}}
+}
+
+// CHECK: nameSTRINGIFIED_VA_ARGS

Also. https://godbolt.org/z/8c3dxP


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86135

__

[PATCH] D86514: Correctly parse LateParsedTemplates in case of multiple dependent modules

[Vaibhav Garg via Phabricator via cfe-commits]

gargvaibhav64 updated this revision to Diff 287609.
gargvaibhav64 edited the summary of this revision.
gargvaibhav64 added a comment.

Resolve a typo.


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

https://reviews.llvm.org/D86514

Files:
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/Serialization/ASTReader.cpp


Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -3721,6 +3721,9 @@
 
 case LATE_PARSED_TEMPLATE:
   LateParsedTemplates.append(Record.begin(), Record.end());
+  LateParsedTemplatesModulesMap.insert(
+  std::make_pair(&F, std::move(LateParsedTemplates)));
+  LateParsedTemplates.clear();
   break;
 
 case OPTIMIZE_PRAGMA_OPTIONS:
@@ -8386,25 +8389,28 @@
 void ASTReader::ReadLateParsedTemplates(
 llvm::MapVector>
 &LPTMap) {
-  for (unsigned Idx = 0, N = LateParsedTemplates.size(); Idx < N;
-   /* In loop */) {
-FunctionDecl *FD = cast(GetDecl(LateParsedTemplates[Idx++]));
+  for (auto &LPT : LateParsedTemplatesModulesMap) {
+ModuleFile *FMod = LPT.first;
+SmallVector LateParsed(LPT.second);
+for (unsigned Idx = 0, N = LateParsed.size(); Idx < N;
+ /* In loop */) {
+  FunctionDecl *FD =
+  cast(GetLocalDecl(*FMod, LateParsed[Idx++]));
 
-auto LT = std::make_unique();
-LT->D = GetDecl(LateParsedTemplates[Idx++]);
+  auto LT = std::make_unique();
+  LT->D = GetLocalDecl(*FMod, LateParsed[Idx++]);
 
-ModuleFile *F = getOwningModuleFile(LT->D);
-assert(F && "No module");
+  ModuleFile *F = getOwningModuleFile(LT->D);
+  assert(F && "No module");
 
-unsigned TokN = LateParsedTemplates[Idx++];
-LT->Toks.reserve(TokN);
-for (unsigned T = 0; T < TokN; ++T)
-  LT->Toks.push_back(ReadToken(*F, LateParsedTemplates, Idx));
+  unsigned TokN = LateParsed[Idx++];
+  LT->Toks.reserve(TokN);
+  for (unsigned T = 0; T < TokN; ++T)
+LT->Toks.push_back(ReadToken(*F, LateParsed, Idx));
 
-LPTMap.insert(std::make_pair(FD, std::move(LT)));
+  LPTMap.insert(std::make_pair(FD, std::move(LT)));
+}
   }
-
-  LateParsedTemplates.clear();
 }
 
 void ASTReader::LoadSelector(Selector Sel) {
Index: clang/include/clang/Serialization/ASTReader.h
===
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -903,6 +903,10 @@
   // A list of late parsed template function data.
   SmallVector LateParsedTemplates;
 
+  // A list of LateParsedTemplates paired with their module files.
+  llvm::MapVector>
+  LateParsedTemplatesModulesMap;
+
   /// The IDs of all decls to be checked for deferred diags.
   ///
   /// Sema tracks these to emit deferred diags.


Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -3721,6 +3721,9 @@
 
 case LATE_PARSED_TEMPLATE:
   LateParsedTemplates.append(Record.begin(), Record.end());
+  LateParsedTemplatesModulesMap.insert(
+  std::make_pair(&F, std::move(LateParsedTemplates)));
+  LateParsedTemplates.clear();
   break;
 
 case OPTIMIZE_PRAGMA_OPTIONS:
@@ -8386,25 +8389,28 @@
 void ASTReader::ReadLateParsedTemplates(
 llvm::MapVector>
 &LPTMap) {
-  for (unsigned Idx = 0, N = LateParsedTemplates.size(); Idx < N;
-   /* In loop */) {
-FunctionDecl *FD = cast(GetDecl(LateParsedTemplates[Idx++]));
+  for (auto &LPT : LateParsedTemplatesModulesMap) {
+ModuleFile *FMod = LPT.first;
+SmallVector LateParsed(LPT.second);
+for (unsigned Idx = 0, N = LateParsed.size(); Idx < N;
+ /* In loop */) {
+  FunctionDecl *FD =
+  cast(GetLocalDecl(*FMod, LateParsed[Idx++]));
 
-auto LT = std::make_unique();
-LT->D = GetDecl(LateParsedTemplates[Idx++]);
+  auto LT = std::make_unique();
+  LT->D = GetLocalDecl(*FMod, LateParsed[Idx++]);
 
-ModuleFile *F = getOwningModuleFile(LT->D);
-assert(F && "No module");
+  ModuleFile *F = getOwningModuleFile(LT->D);
+  assert(F && "No module");
 
-unsigned TokN = LateParsedTemplates[Idx++];
-LT->Toks.reserve(TokN);
-for (unsigned T = 0; T < TokN; ++T)
-  LT->Toks.push_back(ReadToken(*F, LateParsedTemplates, Idx));
+  unsigned TokN = LateParsed[Idx++];
+  LT->Toks.reserve(TokN);
+  for (unsigned T = 0; T < TokN; ++T)
+LT->Toks.push_back(ReadToken(*F, LateParsed, Idx));
 
-LPTMap.insert(std::make_pair(FD, std::move(LT)));
+  LPTMap.insert(std::make_pair(FD, std::move(LT)));
+}
   }
-
-  LateParsedTemplates.clear();
 }
 
 void ASTReader::LoadSelector(Selector Sel) {
Index: clang/include/clang/Serialization/ASTReader.h
=

[PATCH] D73376: [analyzer] Add FuchsiaLockChecker and C11LockChecker

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

Szelethus added a comment.
Herald added subscribers: ASDenysPetrov, martong.

Documentation under `clang/docs/analyzer/checkers.rst` seems to be missing. 
Could we get that fixed before 11.0.0 releases?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73376

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

[PATCH] D86293: [analyzer] Add modeling of Eq operator in smart ptr

[Nithin VR via Phabricator via cfe-commits]

vrnithinkumar added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:351
 
+bool SmartPtrModeling::handleEqOp(const CallEvent &Call,
+  CheckerContext &C) const {

vsavchenko wrote:
> xazax.hun wrote:
> > I'd prefer to call this AssignOp to avoid confusion with `==`. While your 
> > naming is correct, I always found this nomenclature confusing.
> IMO it's not a question of preference with this one, `EqOp` is misleading.
Changed to AssignOp



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:384
+
+  const auto *ArgRegionVal = State->get(ArgRegion);
+  if (ArgRegionVal) {

xazax.hun wrote:
> I also find the names of the variables confusing.
> 
> Instead of `ArgRegion` what about `OtherSmartPtrRegion`?
> Instead of `ArgRegionVal` what about `OtherInnerPtr`?
Changed to `OtherSmartPtrRegion ` and `OtherInnerPtr`



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:391
+
+C.addTransition(State, C.getNoteTag([ThisRegion, ArgRegion, IsArgValNull](
+PathSensitiveBugReport &BR,

xazax.hun wrote:
> Adding return after every `addTransition` is a good practive that we should 
> follow.
Added return



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:399
+ArgRegion->printPretty(OS);
+OS << " is null after moved to ";
+ThisRegion->printPretty(OS);

vsavchenko wrote:
> The grammar seems off in this one.  I think it should be smith like "after 
> being moved to" or "after it was moved to".
Changed to "after being moved to"



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:419
+llvm::raw_ostream &OS) {
+  if (&BR.getBugType() != smartptr::getNullDereferenceBugType() ||
+  !BR.isInteresting(ArgRegion))

xazax.hun wrote:
> Isn't this the same as the beginning of the note tag above?
> 
> I wonder if there is a way to deduplicate this code. Not a huge priority 
> though as I do not have an immediate idea for doing this in a clean way.
Yes.
The check for bug type is duplicated across all the note tags.



Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:132
+  std::unique_ptr PToMove; // expected-note {{Default constructed smart 
pointer 'PToMove' is null}}
+  P = std::move(PToMove); // expected-note {{Smart pointer 'P' is null after a 
null value moved from 'PToMove'}}
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' 
[alpha.cplusplus.SmartPtr]}}

vsavchenko wrote:
> NoQ wrote:
> > I suggest: `Null pointer value move-assigned to 'P'`.
> +1
Updated to `Null pointer value move-assigned to 'P'`



Comment at: clang/test/Analysis/smart-ptr-text-output.cpp:139
+  std::unique_ptr P;
+  P = std::move(PToMove); // expected-note {{Smart pointer 'PToMove' is null 
after moved and assigned to 'P'}}
+  PToMove->foo(); // expected-warning {{Dereference of null smart pointer 
'PToMove' [alpha.cplusplus.SmartPtr]}}

NoQ wrote:
> I suggest: `Smart pointer 'PToMove' is null; previous value moved to 'P'`. Or 
> maybe instead keep the note that the move-checker currently emits?
Going with first option "Smart pointer 'PToMove' is null; previous value moved 
to 'P'"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86293

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

[PATCH] D86293: [analyzer] Add modeling of Eq operator in smart ptr

[Nithin VR via Phabricator via cfe-commits]

vrnithinkumar updated this revision to Diff 287612.
vrnithinkumar marked 9 inline comments as done.
vrnithinkumar edited the summary of this revision.
vrnithinkumar added a comment.

- Addressing review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86293

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/test/Analysis/Inputs/system-header-simulator-cxx.h
  clang/test/Analysis/smart-ptr-text-output.cpp
  clang/test/Analysis/smart-ptr.cpp

Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -216,8 +216,7 @@
 std::unique_ptr P;
 std::unique_ptr Q;
 Q = std::move(P);
-// TODO: Fix test with expecting warning after '=' operator overloading modeling.
-Q->foo(); // no-warning
+Q->foo(); // expected-warning {{Dereference of null smart pointer 'Q' [alpha.cplusplus.SmartPtr]}}
   }
 }
 
@@ -276,3 +275,61 @@
 Y->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}}
   }
 }
+
+void derefOnMovedFromValidPtr() {
+  std::unique_ptr PToMove(new A());
+  std::unique_ptr P;
+  P = std::move(PToMove);
+  PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnMovedToNullPtr() {
+  std::unique_ptr PToMove(new A());
+  std::unique_ptr P;
+  P = std::move(PToMove); // No note.
+  P->foo(); // No warning.
+}
+
+void derefOnNullPtrGotMovedFromValidPtr() {
+  std::unique_ptr P(new A());
+  std::unique_ptr PToMove;
+  P = std::move(PToMove);
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnMovedFromUnknownPtr(std::unique_ptr PToMove) {
+  std::unique_ptr P;
+  P = std::move(PToMove);
+  P->foo(); // No warning.
+}
+
+void derefOnMovedUnknownPtr(std::unique_ptr PToMove) {
+  std::unique_ptr P;
+  P = std::move(PToMove);
+  PToMove->foo(); // expected-warning {{Dereference of null smart pointer 'PToMove' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnAssignedNullPtrToNullSmartPtr() {
+  std::unique_ptr P;
+  P = nullptr;
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnAssignedZeroToNullSmartPtr() {
+  std::unique_ptr P(new A());
+  P = 0;
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnAssignedNullToUnknowSmartPtr(std::unique_ptr P) {
+  P = nullptr;
+  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+std::unique_ptr &&returnRValRefOfUniquePtr();
+
+void drefOnAssignedNullFromMethodPtrValidSmartPtr() {
+  std::unique_ptr P(new A());
+  P = returnRValRefOfUniquePtr();
+  P->foo(); // No warning. 
+}
Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -80,7 +80,7 @@
 void derefOnStdSwappedNullPtr() {
   std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
   std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
-  std::swap(P, PNull); // expected-note@Inputs/system-header-simulator-cxx.h:978 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
+  std::swap(P, PNull); // expected-note@Inputs/system-header-simulator-cxx.h:979 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
   // expected-note@-1 {{Calling 'swap'}}
   // expected-note@-2 {{Returning from 'swap'}}
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
@@ -109,14 +109,6 @@
   // expected-note@-1{{Dereference of null smart pointer 'P'}}
 }
 
-void noNoteTagsForNonMatchingBugType() {
-  std::unique_ptr P; // No note.
-  std::unique_ptr P1; // No note.
-  P1 = std::move(P); // expected-note {{Smart pointer 'P' of type 'std::unique_ptr' is reset to null when moved from}}
-  P->foo(); // expected-warning {{Dereference of null smart pointer 'P' of type 'std::unique_ptr' [cplusplus.Move]}}
-  // expected-note@-1 {{Dereference of null smart pointer 'P' of type 'std::unique_ptr'}}
-}
-
 void derefOnRawPtrFromGetOnNullPtr() {
   std::unique_ptr P; // FIXME: add note "Default constructed smart pointer 'P' is null"
   P.get()->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}}
@@ -131,3 +123,50 @@
 void derefOnRawPtrFromGetOnUnknownPtr(std::unique_ptr P) {
   P.get()->foo(); // No warning.
 }
+
+void derefOnMovedFromValidPtr() {
+  std::unique_ptr PToMove(new A());  // expected-note {{Smart pointer 'PToMove' is constructed}}
+  // FIXME: above note should go away once we fix marking region not i

[PATCH] D84932: [builtins] Add more test cases for __div[sdt]f3 LibCalls

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff accepted this revision.
sepavloff 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/D84932/new/

https://reviews.llvm.org/D84932

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

[libunwind] 1c39ffe - [libunwind] Fix warning when building without frameheader cache

[Mikael Holmen via cfe-commits]

Author: Mikael Holmen
Date: 2020-08-25T12:58:39+02:00
New Revision: 1c39ffecd84a5eba54f5fabb433b0192d1dbd3b4

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

LOG: [libunwind] Fix warning when building without frameheader cache

Without the fix the compiler warns with

/data/repo/master/libunwind/src/AddressSpace.hpp:436:44: warning: unused 
parameter 'pinfo_size' [-Wunused-parameter]
size_t pinfo_size, void *data) {
   ^
1 warning generated.

Added: 


Modified: 
libunwind/src/AddressSpace.hpp

Removed: 




diff  --git a/libunwind/src/AddressSpace.hpp b/libunwind/src/AddressSpace.hpp
index b3949b2d27a4..717aa336fd13 100644
--- a/libunwind/src/AddressSpace.hpp
+++ b/libunwind/src/AddressSpace.hpp
@@ -440,6 +440,9 @@ static int findUnwindSectionsByPhdr(struct dl_phdr_info 
*pinfo,
 #if defined(_LIBUNWIND_USE_FRAME_HEADER_CACHE)
   if (ProcessFrameHeaderCache.find(pinfo, pinfo_size, data))
 return 1;
+#else
+  // Avoid warning about unused variable.
+  (void)pinfo_size;
 #endif
 
   Elf_Addr image_base = calculateImageBase(pinfo);



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

[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

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

Szelethus planned changes to this revision.
Szelethus marked an inline comment as done.
Szelethus added a comment.

Thanks! I'll get these fixed.




Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:901
+
+  void next(Token &Result) {
+if (CurrTokenIt == TokenRange.end()) {

steakhal wrote:
> I don't like output parameters.
> If we really need them, we should at least have a suspicious name.
> Unfortunately, I can not come up with anything useful :|
This is intentional, it meant to replicate the `Lexer`'s interface. I would 
prefer to keep this as-is.



Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:1338-1339
 void TokenPrinter::printToken(const Token &Tok) {
+  // TODO: Handle the case where hash and hashhash occurs right before
+  // __VA_ARGS__.
+

steakhal wrote:
> What does //hashhash// mean? I might lack some context though :D
`#` and `##` respectively. The test cases you pointed out as flawed refer to 
this FIXME, though a FIXME in the tests themselves wouldn't hurt.



Comment at: clang/test/Analysis/plist-macros-with-expansion.cpp:481
+  i = 0;
+#define DISPATCH(...) PARAMS_RESOLVE_TO_VA_ARGS(__VA_ARGS__);
+

steakhal wrote:
> You don't need an ending semicolon here. It will be already there at the 
> expansion location.
> This way you introduce an empty expression after the macro expansion.
> The same happens in all the other cases as well.
You are correct, though the point of macro expansion testing is to see whether 
we nailed what the preprocessor is supposed to do -- not whether the code it 
creates makes such sense. In fact, I would argue that most GNU extensions to 
the preprocessor shouldn't be a thing, but we still need to support it.



Comment at: clang/test/Analysis/plist-macros-with-expansion.cpp:484-486
+  int x = 1;
+  DISPATCH(x, "LF1M healer");
+  (void)(10 / x); // expected-warning{{Division by zero}}

steakhal wrote:
> Should we really abuse the division by zero checker here?
> Can't we just use an ExprInspection call here?
> Maybe it requires a specific BugPath visitor, and that is why we do it this 
> way?
We could totally use `ExprInspection` -- but I'd argue that using something 
else isn't an abuse of the specific checker :) Since the entire file is already 
written this way, and would demand changes in the large plist file, I'd prefer 
to keep it this way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86135

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

[PATCH] D86532: (Urgent!) [docs][analyzer] Add documentation for alpha.fuchsia.Lock and alpha.core.C11Lock

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

Szelethus created this revision.
Szelethus added reviewers: xazax.hun, haowei, NoQ, martong, balazske, steakhal.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, ASDenysPetrov, phosek, Charusso, 
gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, 
baloghadamsoftware, whisperity.
Szelethus requested review of this revision.

Exactly what it says on the tin! Needed an extra label for the CTU stuff for 
the release notes, I figured its okay to land that under the same roof.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86532

Files:
  clang/docs/analyzer/checkers.rst
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst


Index: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
===
--- clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
+++ clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
@@ -201,6 +201,8 @@
   ^C
   $
 
+.. _ctu-on-demand:
+
 On-demand analysis
 __
 The analysis produces the necessary AST structure of external TUs during 
analysis. This requires the
Index: clang/docs/analyzer/checkers.rst
===
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -1491,6 +1491,23 @@
 alpha.core
 ^^
 
+.. _alpha-core-C11Lock:
+
+alpha.core.C11Lock
+""
+Similarly to :ref:`alpha.unix.PthreadLock `, checks for
+the locking/unlocking of ``mtx_t`` mutexes.
+
+.. code-block:: cpp
+
+ mtx_t mtx1;
+
+ void bad1(void)
+ {
+   mtx_lock(&mtx1);
+   mtx_lock(&mtx1); // warn: This lock has already been acquired
+ }
+
 .. _alpha-core-CallAndMessageUnInitRefArg:
 
 alpha.core.CallAndMessageUnInitRefArg (C,C++, ObjC)
@@ -1868,6 +1885,26 @@
*P; // warn: dereference of a default constructed smart unique_ptr
  }
 
+alpha.fuchsia
+^
+
+.. _alpha-fuchsia-lock:
+
+alpha.fuchsia.Lock
+""
+Similarly to :ref:`alpha.unix.PthreadLock `, checks for
+the locking/unlocking of fuchsia mutexes.
+
+.. code-block:: cpp
+
+ spin_lock_t mtx1;
+
+ void bad1(void)
+ {
+   spin_lock(&mtx1);
+   spin_lock(&mtx1);   // warn: This lock has already been acquired
+ }
+
 alpha.llvm
 ^^
 


Index: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
===
--- clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
+++ clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
@@ -201,6 +201,8 @@
   ^C
   $
 
+.. _ctu-on-demand:
+
 On-demand analysis
 __
 The analysis produces the necessary AST structure of external TUs during analysis. This requires the
Index: clang/docs/analyzer/checkers.rst
===
--- clang/docs/analyzer/checkers.rst
+++ clang/docs/analyzer/checkers.rst
@@ -1491,6 +1491,23 @@
 alpha.core
 ^^
 
+.. _alpha-core-C11Lock:
+
+alpha.core.C11Lock
+""
+Similarly to :ref:`alpha.unix.PthreadLock `, checks for
+the locking/unlocking of ``mtx_t`` mutexes.
+
+.. code-block:: cpp
+
+ mtx_t mtx1;
+
+ void bad1(void)
+ {
+   mtx_lock(&mtx1);
+   mtx_lock(&mtx1); // warn: This lock has already been acquired
+ }
+
 .. _alpha-core-CallAndMessageUnInitRefArg:
 
 alpha.core.CallAndMessageUnInitRefArg (C,C++, ObjC)
@@ -1868,6 +1885,26 @@
*P; // warn: dereference of a default constructed smart unique_ptr
  }
 
+alpha.fuchsia
+^
+
+.. _alpha-fuchsia-lock:
+
+alpha.fuchsia.Lock
+""
+Similarly to :ref:`alpha.unix.PthreadLock `, checks for
+the locking/unlocking of fuchsia mutexes.
+
+.. code-block:: cpp
+
+ spin_lock_t mtx1;
+
+ void bad1(void)
+ {
+   spin_lock(&mtx1);
+   spin_lock(&mtx1);	// warn: This lock has already been acquired
+ }
+
 alpha.llvm
 ^^
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker] Use Optionals throughout the summary API

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: balazske, vsavchenko.
Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, 
gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, 
szepet, baloghadamsoftware, xazax.hun, whisperity.
Herald added a reviewer: Szelethus.
Herald added a project: clang.
martong requested review of this revision.

By using optionals, we no longer have to check the validity of types that we
get from a lookup. This way, the definition of the summaries have a declarative
form, there are no superflous conditions in the source code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86531

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX-lookup.c

Index: clang/test/Analysis/std-c-library-functions-POSIX-lookup.c
===
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-POSIX-lookup.c
@@ -0,0 +1,20 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -triple i686-unknown-linux 2>&1
+
+// We test here the implementation of our summary API with Optional types. In
+// this TU we do not provide declaration for any of the functions that have
+// summaries. The implementation should be able to handle the nonexistent
+// declarations in a way that the summary is not added to the map. We expect no
+// crashes (i.e. no optionals should be 'dereferenced') and no output.
+
+// Must have at least one call expression to initialize the summary map.
+int bar(void);
+void foo() {
+  bar();
+}
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -314,7 +314,8 @@
   /// The complete list of constraints that defines a single branch.
   typedef std::vector ConstraintSet;
 
-  using ArgTypes = std::vector;
+  using ArgTypes = std::vector>;
+  using RetType = Optional;
 
   // A placeholder type, we use it whenever we do not care about the concrete
   // type in a Signature.
@@ -324,17 +325,37 @@
   // The signature of a function we want to describe with a summary. This is a
   // concessive signature, meaning there may be irrelevant types in the
   // signature which we do not check against a function with concrete types.
-  struct Signature {
-ArgTypes ArgTys;
+  // All types in the spec need to be canonical.
+  class Signature {
+using ArgQualTypes = std::vector;
+ArgQualTypes ArgTys;
 QualType RetTy;
-Signature(ArgTypes ArgTys, QualType RetTy) : ArgTys(ArgTys), RetTy(RetTy) {
-  assertRetTypeSuitableForSignature(RetTy);
-  for (size_t I = 0, E = ArgTys.size(); I != E; ++I) {
-QualType ArgTy = ArgTys[I];
-assertArgTypeSuitableForSignature(ArgTy);
+// True if any component type is not found by lookup.
+bool Invalid = false;
+
+  public:
+// Construct a signature from optional types. If any of the optional types
+// are not set then the signature will be invalid.
+Signature(ArgTypes ArgTys, RetType RetTy) {
+  for (Optional Arg : ArgTys) {
+if (!Arg) {
+  Invalid = true;
+  return;
+} else {
+  assertArgTypeSuitableForSignature(*Arg);
+  this->ArgTys.push_back(*Arg);
+}
+  }
+  if (!RetTy) {
+Invalid = true;
+return;
+  } else {
+assertRetTypeSuitableForSignature(*RetTy);
+this->RetTy = *RetTy;
   }
 }
 
+bool isInvalid() const { return Invalid; }
 bool matches(const FunctionDecl *FD) const;
 
   private:
@@ -388,6 +409,9 @@
   ///   rules for the given parameter's type, those rules are checked once the
   ///   signature is matched.
   class Summary {
+// FIXME Probably the Signature should not be part of the Summary,
+// We can remove once all overload of addToFunctionSummaryMap requires the
+// Signature explicitly given.
 Optional Sign;
 const InvalidationKind InvalidationKd;
 Cases CaseConstraints;
@@ -398,11 +422,13 @@
 const FunctionDecl *FD = nullptr;
 
   public:
-Summary(ArgTypes ArgTys, QualType RetTy, InvalidationKind InvalidationKd)
+Summary(ArgTypes ArgTys, RetType RetTy, InvalidationKind InvalidationKd)
 : Sign(Signature(ArgTys, RetTy)), InvalidationKd(InvalidationKd) {}
 
 Summary(InvalidationKind InvalidationKd) : InvalidationKd(InvalidationKd) {}
 
+// FIXME R

[clang-tools-extra] 4d90ff5 - [clangd] When inserting "using", add "::" in front if that's the style.

[Adam Czachorowski via cfe-commits]

Author: Adam Czachorowski
Date: 2020-08-25T14:07:49+02:00
New Revision: 4d90ff59ac453a67ac692ffdf8242e4cfbd2b34f

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

LOG: [clangd] When inserting "using", add "::" in front if that's the style.

We guess the style based on the existing using declarations. If there
are any and they all start with ::, we add it to the newly added one
too.

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

Added: 


Modified: 
clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
clang-tools-extra/clangd/unittests/TweakTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp 
b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
index 9e64ceeeaead..e4900041671a 100644
--- a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -86,6 +86,13 @@ class UsingFinder : public RecursiveASTVisitor {
   const SourceManager &SM;
 };
 
+bool isFullyQualified(const NestedNameSpecifier *NNS) {
+  if (!NNS)
+return false;
+  return NNS->getKind() == NestedNameSpecifier::Global ||
+ isFullyQualified(NNS->getPrefix());
+}
+
 struct InsertionPointData {
   // Location to insert the "using" statement. If invalid then the statement
   // should not be inserted at all (it already exists).
@@ -94,6 +101,9 @@ struct InsertionPointData {
   // insertion point is anchored to, we may need one or more \n to ensure
   // proper formatting.
   std::string Suffix;
+  // Whether using should be fully qualified, even if what the user typed was
+  // not. This is based on our detection of the local style.
+  bool AlwaysFullyQualify = false;
 };
 
 // Finds the best place to insert the "using" statement. Returns invalid
@@ -118,7 +128,13 @@ findInsertionPoint(const Tweak::Selection &Inputs,
   SM)
   .TraverseAST(Inputs.AST->getASTContext());
 
+  bool AlwaysFullyQualify = true;
   for (auto &U : Usings) {
+// Only "upgrade" to fully qualified is all relevant using decls are fully
+// qualified. Otherwise trust what the user typed.
+if (!isFullyQualified(U->getQualifier()))
+  AlwaysFullyQualify = false;
+
 if (SM.isBeforeInTranslationUnit(Inputs.Cursor, U->getUsingLoc()))
   // "Usings" is sorted, so we're done.
   break;
@@ -137,6 +153,7 @@ findInsertionPoint(const Tweak::Selection &Inputs,
   if (LastUsingLoc.isValid()) {
 InsertionPointData Out;
 Out.Loc = LastUsingLoc;
+Out.AlwaysFullyQualify = AlwaysFullyQualify;
 return Out;
   }
 
@@ -278,6 +295,9 @@ Expected AddUsing::apply(const Selection 
&Inputs) {
 std::string UsingText;
 llvm::raw_string_ostream UsingTextStream(UsingText);
 UsingTextStream << "using ";
+if (InsertionPoint->AlwaysFullyQualify &&
+!isFullyQualified(QualifierToRemove.getNestedNameSpecifier()))
+  UsingTextStream << "::";
 QualifierToRemove.getNestedNameSpecifier()->print(
 UsingTextStream, Inputs.AST->getASTContext().getPrintingPolicy());
 UsingTextStream << Name << ";" << InsertionPoint->Suffix;

diff  --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp 
b/clang-tools-extra/clangd/unittests/TweakTests.cpp
index b4f135b3efe2..5626b7530599 100644
--- a/clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2723,6 +2723,63 @@ namespace foo { void fun(); }
 
 void foo::fun() {
   ff();
+})cpp"},
+// If all other using are fully qualified, add ::
+{R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using ::one::two::ee;
+
+void fun() {
+  one::two::f^f();
+})cpp",
+ R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using ::one::two::ff;using ::one::two::ee;
+
+void fun() {
+  ff();
+})cpp"},
+// Make sure we don't add :: if it's already there
+{R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using ::one::two::ee;
+
+void fun() {
+  ::one::two::f^f();
+})cpp",
+ R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using ::one::two::ff;using ::one::two::ee;
+
+void fun() {
+  ff();
+})cpp"},
+// If even one using doesn't start with ::, do not add it
+{R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using one::two::ee;
+
+void fun() {
+  one::two::f^f();
+})cpp",
+ R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using one::two::ff;using one::two::ee;
+
+void fun() {
+  ff();
 })cpp"}};
   llvm::StringMap EditedFiles;
   for (const auto &Case : Cases) {



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

[PATCH] D86473: [clangd] When inserting "using", add "::" in front if that's the style.

[Adam Czachorowski via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4d90ff59ac45: [clangd] When inserting "using", add 
"::" in front if that's the style. (authored by adamcz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86473

Files:
  clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp

Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -2723,6 +2723,63 @@
 
 void foo::fun() {
   ff();
+})cpp"},
+// If all other using are fully qualified, add ::
+{R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using ::one::two::ee;
+
+void fun() {
+  one::two::f^f();
+})cpp",
+ R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using ::one::two::ff;using ::one::two::ee;
+
+void fun() {
+  ff();
+})cpp"},
+// Make sure we don't add :: if it's already there
+{R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using ::one::two::ee;
+
+void fun() {
+  ::one::two::f^f();
+})cpp",
+ R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using ::one::two::ff;using ::one::two::ee;
+
+void fun() {
+  ff();
+})cpp"},
+// If even one using doesn't start with ::, do not add it
+{R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using one::two::ee;
+
+void fun() {
+  one::two::f^f();
+})cpp",
+ R"cpp(
+#include "test.hpp"
+
+using ::one::two::cc;
+using one::two::ff;using one::two::ee;
+
+void fun() {
+  ff();
 })cpp"}};
   llvm::StringMap EditedFiles;
   for (const auto &Case : Cases) {
Index: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -86,6 +86,13 @@
   const SourceManager &SM;
 };
 
+bool isFullyQualified(const NestedNameSpecifier *NNS) {
+  if (!NNS)
+return false;
+  return NNS->getKind() == NestedNameSpecifier::Global ||
+ isFullyQualified(NNS->getPrefix());
+}
+
 struct InsertionPointData {
   // Location to insert the "using" statement. If invalid then the statement
   // should not be inserted at all (it already exists).
@@ -94,6 +101,9 @@
   // insertion point is anchored to, we may need one or more \n to ensure
   // proper formatting.
   std::string Suffix;
+  // Whether using should be fully qualified, even if what the user typed was
+  // not. This is based on our detection of the local style.
+  bool AlwaysFullyQualify = false;
 };
 
 // Finds the best place to insert the "using" statement. Returns invalid
@@ -118,7 +128,13 @@
   SM)
   .TraverseAST(Inputs.AST->getASTContext());
 
+  bool AlwaysFullyQualify = true;
   for (auto &U : Usings) {
+// Only "upgrade" to fully qualified is all relevant using decls are fully
+// qualified. Otherwise trust what the user typed.
+if (!isFullyQualified(U->getQualifier()))
+  AlwaysFullyQualify = false;
+
 if (SM.isBeforeInTranslationUnit(Inputs.Cursor, U->getUsingLoc()))
   // "Usings" is sorted, so we're done.
   break;
@@ -137,6 +153,7 @@
   if (LastUsingLoc.isValid()) {
 InsertionPointData Out;
 Out.Loc = LastUsingLoc;
+Out.AlwaysFullyQualify = AlwaysFullyQualify;
 return Out;
   }
 
@@ -278,6 +295,9 @@
 std::string UsingText;
 llvm::raw_string_ostream UsingTextStream(UsingText);
 UsingTextStream << "using ";
+if (InsertionPoint->AlwaysFullyQualify &&
+!isFullyQualified(QualifierToRemove.getNestedNameSpecifier()))
+  UsingTextStream << "::";
 QualifierToRemove.getNestedNameSpecifier()->print(
 UsingTextStream, Inputs.AST->getASTContext().getPrintingPolicy());
 UsingTextStream << Name << ";" << InsertionPoint->Suffix;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang/test/Driver/cl-options.c:686
+// vctoolsdir is handled by the driver; just check that we don't error. Pass 
-c because fakedir isn't a real toolchain path
+// RUN: %clang_cl -c -vctoolsdir fakedir -- %s 2>&1
+

zahen wrote:
> zahen wrote:
> > hans wrote:
> > > zahen wrote:
> > > > hans wrote:
> > > > > aganea wrote:
> > > > > > Check that we're not detecting a local installation, and that we 
> > > > > > fallback to the default triple 19.11, ie.
> > > > > > ```
> > > > > > // RUN: %clang_cl ... -vctoolsdir "" -### | FileCheck %s 
> > > > > > --check-prefix VCTOOLSDIR
> > > > > > // VCTOOLSDIR: "-cc1" "-triple" "x86_64-pc-windows-msvc19.11.0"
> > > > > > ```
> > > > > Maybe add a comment explaining the purpose of the test.
> > > > > 
> > > > > And would it be possible to have a test where -vctoolsdir is set to 
> > > > > something, to see that it's picked up correctly?
> > > > What's the expectation on the test boxes?  I can author such a test but 
> > > > it would be very brittle unless we have a spec for how VS should be 
> > > > installed.
> > > I'd suggest passing -vctoolsdir "/foo" and check that some /foo dir is 
> > > getting passed as a system include dir to cc1, and that the link.exe 
> > > invocation is under /foo.
> > > 
> > > I don't think it would be brittle.
> > Clever idea! I'll do it that way and key off the "ignored directory" 
> > warning that's emitted.
> Did further digging and the path test would depend on other environment 
> variables.  If the %INCLUDE% variable is set, as it is by vcvars.bat, those 
> paths are adopted and VCToolChainPath is not used.  I'm hesitant to change 
> that logic without stronger motivation.
> 
> Other ideas for a test that would work run both from a vc command prompt and 
> a vanilla command prompt?  I don't see anything else in the full verbose 
> invocation to key off of:
> 
> 
> ```
> D:\repos\llvm\llvm-project\build\Debug\bin>clang-cl -vctoolsdir "/fake" -v 
> main.cpp
> clang version 12.0.0 (https://github.com/llvm/llvm-project 
> 537f5483fe4e09c39ffd7ac837c00b50dd13d676)
> Target: x86_64-pc-windows-msvc
> Thread model: posix
> InstalledDir: D:\repos\llvm\llvm-project\build\Debug\bin
>  "D:\\repos\\llvm\\llvm-project\\build\\Debug\\bin\\clang-cl.exe" -cc1 
> -triple x86_64-pc-windows-msvc19.11.0 -emit-obj -mrelax-all 
> -mincremental-linker-compatible --mrelax-relocations -disable-free 
> -main-file-name main.cpp -mrelocation-model pic -pic-level 2 
> -mframe-pointer=none -relaxed-aliasing -fmath-errno -fno-rounding-math 
> -mconstructor-aliases -munwind-tables -target-cpu x86-64 -mllvm 
> -x86-asm-syntax=intel -D_MT -flto-visibility-public-std 
> --dependent-lib=libcmt --dependent-lib=oldnames -stack-protector 2 
> -fms-volatile -fdiagnostics-format msvc -v -resource-dir 
> "D:\\repos\\llvm\\llvm-project\\build\\Debug\\lib\\clang\\12.0.0" 
> -internal-isystem 
> "D:\\repos\\llvm\\llvm-project\\build\\Debug\\lib\\clang\\12.0.0\\include" 
> -internal-isystem "C:\\Program Files (x86)\\Microsoft Visual 
> Studio\\2019\\Enterprise\\VC\\Tools\\MSVC\\14.27.29110\\ATLMFC\\include" 
> -internal-isystem "C:\\Program Files (x86)\\Microsoft Visual 
> Studio\\2019\\Enterprise\\VC\\Tools\\MSVC\\14.27.29110\\include" 
> -internal-isystem "C:\\Program Files (x86)\\Windows 
> Kits\\NETFXSDK\\4.8\\include\\um" -internal-isystem "C:\\Program Files 
> (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\ucrt" -internal-isystem 
> "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\shared" 
> -internal-isystem "C:\\Program Files (x86)\\Windows 
> Kits\\10\\include\\10.0.18362.0\\um" -internal-isystem "C:\\Program Files 
> (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\winrt" -internal-isystem 
> "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\cppwinrt" 
> -fdeprecated-macro -fdebug-compilation-dir 
> "D:\\repos\\llvm\\llvm-project\\build\\Debug\\bin" -ferror-limit 19 
> -fmessage-length=121 -fno-use-cxa-atexit -fms-extensions -fms-compatibility 
> -fms-compatibility-version=19.11 -std=c++14 -fdelayed-template-parsing 
> -fcolor-diagnostics -faddrsig -o 
> "C:\\Users\\zahen\\AppData\\Local\\Temp\\main-8f329a.obj" -x c++ main.cpp
> clang -cc1 version 12.0.0 based upon LLVM 12.0.0git default target 
> x86_64-pc-windows-msvc
> #include "..." search starts here:
> #include <...> search starts here:
>  D:\repos\llvm\llvm-project\build\Debug\lib\clang\12.0.0\include
>  C:\Program Files (x86)\Microsoft Visual 
> Studio\2019\Enterprise\VC\Tools\MSVC\14.27.29110\ATLMFC\include
>  C:\Program Files (x86)\Microsoft Visual 
> Studio\2019\Enterprise\VC\Tools\MSVC\14.27.29110\include
>  C:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\include\um
>  C:\Program Files (x86)\Windows Kits\10\include\10.0.18362.0\ucrt
>  C:\Program Files (x86)\Windows Kits\10\include\10.0.18362.0\shared
>  C:\Program Files (x86)\Windows Kits\10\include\10.0.18362.0\um
>  C:\Program Files (x86)\Windows

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

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

Szelethus created this revision.
Szelethus added reviewers: hans, NoQ, vsavchenko, dcoughlin, xazax.hun, 
baloghadamsoftware, martong, balazske, steakhal, Charusso, jkorous, dkrupp, 
gamesh411.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, ASDenysPetrov, phosek, donat.nagy, 
dexonsmith, mikhail.ramalho, a.sidorin, rnkovacs, szepet, whisperity.
Szelethus requested review of this revision.

Late as always, but it seems like not too late :) I tried added everyone who 
contributed to the static analyzer in a meaningful way, but if you did and 
don't find yourself on the reviewer list, please add yourself! I compiled these 
notes from the following list:

  git log llvmorg-11-init..llvmorg-11.0.0-rc2 --oneline -- 
clang/lib/StaticAnalyzer/ clang/include/clang/StaticAnalyzer/

Seems like this is a release to be proud of!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86533

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -440,7 +440,60 @@
 Static Analyzer
 ---
 
-- ...
+- Moved ``PlacementNewChecker`` out of alpha, making it enabled by default.
+
+- New checker: :ref:`alpha.core.C11Lock ` and
+  :ref:`alpha.fuchsia.Lock ` checks for appropriate API
+  locks/unlocks.
+
+- New checker: :ref:`alpha.security.cert.pos.34c `
+  finds calls to the ``putenv`` function which pass a pointer to an automatic
+  variable as the argument.
+
+- New checker: :ref:`webkit.NoUncountedMemberChecker
+  ` to enforce the existence of virtual
+  destructors for all uncounted types used as base classes.
+
+- New checker: :ref:`webkit.NoUncountedMemberChecker
+  ` checks for that raw pointers and 
references
+  to uncounted types can't be used as class members, only ref-counted types.
+
+- New checker: :ref:`alpha.cplusplus.SmartPtr ` check
+  for dereference of null smart pointers.
+
+- Improved the analyzer's understanding of inherited C++ constructors.
+
+- Added support for multi-dimensional variadic arrays in ``core.VLASize``.
+
+- Added a check for insufficient storage in array placement new calls, as well
+  as support for alignment variants to ``cplusplus.PlacementNew``.
+
+- Improve the pre- and post condition modeling of several hundred more standard
+  C functions.
+
+- While still in alpha, ``alpha.unix.PthreadLock``, the iterator and container
+  modeling infrastructure, ``alpha.unix.Stream``, and taint analysis were
+  improved greatly.
+
+- Improved the warning messages of several checkers.
+
+- Fixed a few remaining cases of checkers emmiting reports under the incorrect
+  checker name, and employed a few restrictions to more easily identifiy and
+  avoid such errors.
+
+- Moved several non-reporting checkers (those that model, among other things,
+  standard functions, but emit no diagnostics) to be listed under
+  ``-analyzer-checker-help-developer`` instead of ``-analyzer-checker-help``.
+  Manually enabling or disabling checkers found on this list is not recommended
+  for non-development purposes.
+
+- Added :ref:`on-demand parsing ` capability to cross 
translation
+  unit analysis.
+
+- Numerous fixes for crashes, false positives and false negatives in
+  ``unix.Malloc``, ``osx.cocoa.NSError``, and several other checkers.
+
+- Numerous fixes and improvements for the HTML output.
 
 .. _release-notes-ubsan:
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -440,7 +440,60 @@
 Static Analyzer
 ---
 
-- ...
+- Moved ``PlacementNewChecker`` out of alpha, making it enabled by default.
+
+- New checker: :ref:`alpha.core.C11Lock ` and
+  :ref:`alpha.fuchsia.Lock ` checks for appropriate API
+  locks/unlocks.
+
+- New checker: :ref:`alpha.security.cert.pos.34c `
+  finds calls to the ``putenv`` function which pass a pointer to an automatic
+  variable as the argument.
+
+- New checker: :ref:`webkit.NoUncountedMemberChecker
+  ` to enforce the existence of virtual
+  destructors for all uncounted types used as base classes.
+
+- New checker: :ref:`webkit.NoUncountedMemberChecker
+  ` checks for that raw pointers and references
+  to uncounted types can't be used as class members, only ref-counted types.
+
+- New checker: :ref:`alpha.cplusplus.SmartPtr ` check
+  for dereference of null smart pointers.
+
+- Improved the analyzer's understanding of inherited C++ constructors.
+
+- Added support for multi-dimensional variadic arrays in ``core.VLASize``.
+
+- Added a check for insufficient storage in array placement new calls, as well
+  as support for alignment variants to ``cplusplus.PlacementNew``.
+
+- Improve the pre- and post condition modeling of several hundred more standard
+  C functions.
+
+- While still in alpha, ``alpha.unix.PthreadLock``, the iterator and container

[PATCH] D84415: [analyzer][StdLibraryFunctionsChecker] Add POSIX pthread handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 287640.
martong added a comment.

- Rebase to parent patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84415

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX.c

Index: clang/test/Analysis/std-c-library-functions-POSIX.c
===
--- clang/test/Analysis/std-c-library-functions-POSIX.c
+++ clang/test/Analysis/std-c-library-functions-POSIX.c
@@ -95,6 +95,20 @@
 // CHECK: Loaded summary for: ssize_t send(int sockfd, const void *buf, size_t len, int flags)
 // CHECK: Loaded summary for: int socketpair(int domain, int type, int protocol, int sv[2])
 // CHECK: Loaded summary for: int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags)
+// CHECK: Loaded summary for: int pthread_cond_signal(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_cond_broadcast(pthread_cond_t *cond)
+// CHECK: Loaded summary for: int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg)
+// CHECK: Loaded summary for: int pthread_attr_destroy(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_init(pthread_attr_t *attr)
+// CHECK: Loaded summary for: int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize)
+// CHECK: Loaded summary for: int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize)
+// CHECK: Loaded summary for: int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize)
+// CHECK: Loaded summary for: int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize)
+// CHECK: Loaded summary for: int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr)
+// CHECK: Loaded summary for: int pthread_mutex_destroy(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_lock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_trylock(pthread_mutex_t *mutex)
+// CHECK: Loaded summary for: int pthread_mutex_unlock(pthread_mutex_t *mutex)
 
 long a64l(const char *str64);
 char *l64a(long value);
@@ -227,6 +241,34 @@
 int socketpair(int domain, int type, int protocol, int sv[2]);
 int getnameinfo(const struct sockaddr *restrict sa, socklen_t salen, char *restrict node, socklen_t nodelen, char *restrict service, socklen_t servicelen, int flags);
 
+typedef union {
+  int x;
+} pthread_cond_t;
+int pthread_cond_signal(pthread_cond_t *cond);
+int pthread_cond_broadcast(pthread_cond_t *cond);
+typedef union {
+  int x;
+} pthread_attr_t;
+typedef unsigned long int pthread_t;
+int pthread_create(pthread_t *restrict thread, const pthread_attr_t *restrict attr, void *(*start_routine)(void *), void *restrict arg);
+int pthread_attr_destroy(pthread_attr_t *attr);
+int pthread_attr_init(pthread_attr_t *attr);
+int pthread_attr_getstacksize(const pthread_attr_t *restrict attr, size_t *restrict stacksize);
+int pthread_attr_getguardsize(const pthread_attr_t *restrict attr, size_t *restrict guardsize);
+int pthread_attr_setstacksize(pthread_attr_t *attr, size_t stacksize);
+int pthread_attr_setguardsize(pthread_attr_t *attr, size_t guardsize);
+typedef union {
+  int x;
+} pthread_mutex_t;
+typedef union {
+  int x;
+} pthread_mutexattr_t;
+int pthread_mutex_init(pthread_mutex_t *restrict mutex, const pthread_mutexattr_t *restrict attr);
+int pthread_mutex_destroy(pthread_mutex_t *mutex);
+int pthread_mutex_lock(pthread_mutex_t *mutex);
+int pthread_mutex_trylock(pthread_mutex_t *mutex);
+int pthread_mutex_unlock(pthread_mutex_t *mutex);
+
 // Must have at least one call expression to initialize the summary map.
 int bar(void);
 void foo() {
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -903,6 +903,8 @@
   const QualType ConstWchar_tPtrTy =
   getPointerTy(getConstTy(WCharTy)); // const wchar_t *
   const QualType ConstVoidPtrRestrictTy = getRestrictTy(ConstVoidPtrTy);
+  const QualType SizePtrTy = getPointerTy(SizeTy);
+  const QualType SizePtrRestrictTy = getRestrictTy(SizePtrTy);
 
   const RangeInt IntMax = BVF.getMaxValue(IntTy).getLimitedValue();
   const RangeInt UnsignedIntMax =
@@ -978,6 +980,12 @@
   for (const Summary &S : Summaries)
 operator()(Name, S);
 }
+// Add the same summary for different names with the Signature explicitly
+// given.
+void operator()(std::vector Names, Signature Sign, Summary Sum) {
+  for (StringRef Name : N

[PATCH] D86532: (Urgent!) [docs][analyzer] Add documentation for alpha.fuchsia.Lock and alpha.core.C11Lock

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

Szelethus added a reviewer: hans.
Szelethus added a comment.

We will need to push this to the 11.0.0. release branch as well, sorry for the 
trouble.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86532

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

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

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

Szelethus added a comment.

@vsavchenko I just realized your work for this release is the following list:

  git log llvmorg-11-init..llvmorg-11.0.0-rc2 --oneline -- clang/utils/analyzer/

Is it a correct guess that while your primary audience are the analyzer 
developers, but it wouldn't hurt to mention it in the release notes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86533

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

[PATCH] D86532: (Urgent!) [docs][analyzer] Add documentation for alpha.fuchsia.Lock and alpha.core.C11Lock

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:213
 compile said TU are given in YAML format referred to as `invocation list`, and 
must be passed as an
 analyer-config argument.
 The index, which maps function USR names to source files containing them must 
also be generated by the

While we are here, could you please fix the typo we have here? `analyer-config` 
-> `analyzer-config` (the z is missing).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86532

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

[PATCH] D86097: [OpenMP][AMDGCN] Generate global variables and attributes for AMDGCN

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Reformat the code




Comment at: clang/lib/CodeGen/CGOpenMPRuntime.h:498
   /// far.
+
   class OffloadEntriesInfoManagerTy {

Remove unnecessary formatting changes. 



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.h:2479-2483
+/// Declaration of functions visible in clang::CodeGen namespace, to
+/// be used by target specific specializations of CGOpenMPRuntimeGPU.
+
+FieldDecl *addFieldToRecordDecl(ASTContext &C, DeclContext *DC,
+   QualType FieldTy);

Better to make it a protected member function if you really require it. Plus, 
this function is very small and, I think, you simply create your own copy in 
CGOpenMPRuntimeAMDGCN



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.h:2487
+llvm::GlobalVariable *
+createGlobalStruct(CodeGenModule &CGM, QualType Ty, bool IsConstant,
+   ArrayRef Data, const Twine &Name,

Same here, make it protected or just create a copy, if it is small.



Comment at: clang/lib/CodeGen/CGOpenMPRuntimeAMDGCN.h:29-31
+  int ParallelLevel = 0;
+  int MaxParallelLevel = 0;
+  QualType TgtAttributeStructQTy;

Add comments for all new members



Comment at: clang/lib/CodeGen/CGOpenMPRuntimeAMDGCN.h:97
+/// AMDGCN specific PrePostActionTy implementation
+class AMDGCNPrePostActionTy : public PrePostActionTy {
+int &ParallelLevel;

1. Do you really need to make this class public?
2. `final`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86097

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

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:471
+
+- Improve the pre- and post condition modeling of several hundred more standard
+  C functions.

Umm, this is still an alpha command line option, plus we improved only the 
pre-condition checks.
So, I'd rather say something like:
```
Improve the pre-condition modeling of several functions defined in the POSIX 
standard. This is in alpha currently. To enable, one must explicitly set the 
``ModelPOSIX`` argument of the ``apiModeling.StdCLibraryFunctions`` checker.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86533

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

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

Some grammatical fixes and suggestions, inline. I might have absolutely 
butchered 80-col in the suggestions (thanks Phab for not showing any indication 
of line length...), so make sure you manually reformat the document before 
going forward!




Comment at: clang/docs/ReleaseNotes.rst:446-447
+- New checker: :ref:`alpha.core.C11Lock ` and
+  :ref:`alpha.fuchsia.Lock ` checks for appropriate API
+  locks/unlocks.
+





Comment at: clang/docs/ReleaseNotes.rst:450-451
+- New checker: :ref:`alpha.security.cert.pos.34c `
+  finds calls to the ``putenv`` function which pass a pointer to an automatic
+  variable as the argument.
+





Comment at: clang/docs/ReleaseNotes.rst:458-459
+- New checker: :ref:`webkit.NoUncountedMemberChecker
+  ` checks for that raw pointers and 
references
+  to uncounted types can't be used as class members, only ref-counted types.
+

The phrasing is incredibly convoluted here, let's put the //positive// part of 
the rule that is enforced first.



Comment at: clang/docs/ReleaseNotes.rst:464
+
+- Improved the analyzer's understanding of inherited C++ constructors.
+

This is a core change, right? //How// is this list ordered? Perhaps there 
should be a sort by category, first the core changes, then the individual 
checkers' improvements.



Comment at: clang/docs/ReleaseNotes.rst:471-472
+
+- Improve the pre- and post condition modeling of several hundred more standard
+  C functions.
+

martong wrote:
> Umm, this is still an alpha command line option, plus we improved only the 
> pre-condition checks.
> So, I'd rather say something like:
> ```
> Improve the pre-condition modeling of several functions defined in the POSIX 
> standard. This is in alpha currently. To enable, one must explicitly set the 
> ``ModelPOSIX`` argument of the ``apiModeling.StdCLibraryFunctions`` checker.
> ```
Maybe it's worth mentioning the checker here too, akin to the other list 
elements?



Comment at: clang/docs/ReleaseNotes.rst:480
+
+- Fixed a few remaining cases of checkers emmiting reports under the incorrect
+  checker name, and employed a few restrictions to more easily identifiy and

Typo.



Comment at: clang/docs/ReleaseNotes.rst:480-481
+
+- Fixed a few remaining cases of checkers emmiting reports under the incorrect
+  checker name, and employed a few restrictions to more easily identifiy and
+  avoid such errors.

whisperity wrote:
> Typo.




Comment at: clang/docs/ReleaseNotes.rst:481
+- Fixed a few remaining cases of checkers emmiting reports under the incorrect
+  checker name, and employed a few restrictions to more easily identifiy and
+  avoid such errors.

Typo.



Comment at: clang/docs/ReleaseNotes.rst:487-488
+  ``-analyzer-checker-help-developer`` instead of ``-analyzer-checker-help``.
+  Manually enabling or disabling checkers found on this list is not recommended
+  for non-development purposes.
+





Comment at: clang/docs/ReleaseNotes.rst:490-491
+
+- Added :ref:`on-demand parsing ` capability to cross 
translation
+  unit analysis.
+

What's the proper way of naming this feature, @martong @dkrupp @xazax.hun? Is 
it like this, or should it be in capitals, or should it be 
`cross`**`-`**`translation unit`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86533

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

[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr

[Nithin VR via Phabricator via cfe-commits]

vrnithinkumar updated this revision to Diff 287641.
vrnithinkumar added a comment.

- Making the note tags prunable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86027

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

Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -8,12 +8,13 @@
 void clang_analyzer_warnIfReached();
 void clang_analyzer_numTimesReached();
 void clang_analyzer_eval(bool);
+void clang_analyzer_warnOnDeadSymbol(int *);
 
 void derefAfterMove(std::unique_ptr P) {
   std::unique_ptr Q = std::move(P);
   if (Q)
 clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
-  *Q.get() = 1; // no-warning
+  *Q.get() = 1; // expected-warning {{Dereference of null pointer [core.NullDereference]}}
   if (P)
 clang_analyzer_warnIfReached(); // no-warning
   // TODO: Report a null dereference (instead).
@@ -276,3 +277,69 @@
 Y->foo(); // expected-warning {{Called C++ object pointer is null [core.CallAndMessage]}}
   }
 }
+
+void derefConditionOnNullPtr() {
+  std::unique_ptr P;
+  if (P)
+P->foo(); // No warning.
+  else
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnNotNullPtr() {
+  std::unique_ptr P;
+  if (!P)
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnValidPtr() {
+  std::unique_ptr P(new A());
+  std::unique_ptr PNull;
+  if (P)
+PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefConditionOnNotValidPtr() {
+  std::unique_ptr P(new A());
+  std::unique_ptr PNull;
+  if (!P)
+PNull->foo(); // No warning.
+}
+
+void derefConditionOnUnKnownPtr(std::unique_ptr P) {
+  if (P)
+P->foo(); // No warning.
+  else
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+}
+
+void derefOnValidPtrAfterReset(std::unique_ptr P) {
+  P.reset(new A());
+  if (!P)
+P->foo(); // No warning.
+  else
+P->foo(); // No warning.
+}
+
+void innerPointerSymbolLiveness() {
+  std::unique_ptr P(new int());
+  clang_analyzer_warnOnDeadSymbol(P.get());
+  int *RP = P.release();
+} // expected-warning{{SYMBOL DEAD}}
+
+void boolOpCreatedConjuredSymbolLiveness(std::unique_ptr P) {
+  if (P) {
+int *X = P.get();
+clang_analyzer_warnOnDeadSymbol(X);
+  }
+} // expected-warning{{SYMBOL DEAD}}
+
+void getCreatedConjuredSymbolLiveness(std::unique_ptr P) {
+  int *X = P.get();
+  clang_analyzer_warnOnDeadSymbol(X);
+  int Y;
+  if (!P) {
+Y = *P.get(); // expected-warning {{Dereference of null pointer [core.NullDereference]}}
+// expected-warning@-1 {{SYMBOL DEAD}}
+  }
+}
Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -131,3 +131,112 @@
 void derefOnRawPtrFromGetOnUnknownPtr(std::unique_ptr P) {
   P.get()->foo(); // No warning.
 }
+
+void derefConditionOnNullPtrFalseBranch() {
+  std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
+  if (P) { // expected-note {{Taking false branch}}
+// expected-note@-1{{Smart pointer 'P' is nul}}
+P->foo(); // No warning.
+  } else {
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+void derefConditionOnNullPtrTrueBranch() {
+  std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
+  if (!P) { // expected-note {{Taking true branch}}
+// expected-note@-1{{Smart pointer 'P' is nul}}
+P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'P'}}
+  }
+}
+
+void derefConditionOnValidPtrTrueBranch() {
+  std::unique_ptr P(new A());
+  std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
+  if (P) { // expected-note {{Taking true branch}}
+// expected-note@-1{{Smart pointer 'P' is non-nul}}
+PNull->foo(); // expected-warning {{Dereference of null smart pointer 'PNull' [alpha.cplusplus.SmartPtr]}}
+// expected-note@-1{{Dereference of null smart pointer 'PNull'}}
+  } else {
+PNull->foo(); // No warning
+  }
+}
+
+void derefConditionOnValidPtrFalseBranch() {
+  std::unique_ptr P(new A());
+  std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
+  if (!P) { // exp

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker] Use Optionals throughout the summary API

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/test/Analysis/std-c-library-functions-POSIX-lookup.c:14
+// declarations in a way that the summary is not added to the map. We expect no
+// crashes (i.e. no optionals should be 'dereferenced') and no output.
+

Probably I should use FileCheck with --allow-empty and with CHECK-EMPTY


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86531

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

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:439
 - ...
 
 Static Analyzer

... here.



Comment at: clang/docs/ReleaseNotes.rst:498
 
 .. _release-notes-ubsan:
 

@Szelethus Speaking of labels in the dependency patch D86532, there is no label 
for the CSA changeset...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86533

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

[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr

[Nithin VR via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:494-496
+  OS << "Smart pointer";
+  checkAndPrettyPrintRegion(OS, ThisRegion);
+  OS << " is non-null";

NoQ wrote:
> Because these note tags aren't specific to the bug report at hand, they have 
> to be marked as //prunable// (i.e., the optional `getNoteTag()`'s parameter 
> "`IsPrunable`" should be set to true). That'd reduce bug report clutter by 
> not bringing in stack frames that aren't otherwise interesting for the bug 
> report.
> 
> Something like this may act as a test (i didn't try):
> ```lang=c++
> struct S {
>   std::unique_ptr P;
> 
>   void foo() {
> if (!P) { // no-note because foo() is pruned
>   return;
> }
>   }
> 
>   int bar() {
> P = new int(0);
> foo();
> return 1 / *P; // warning: division by zero
>   }
> 
> }
> 
> ```
Marked these tags as prunable and added the tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86027

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

[PATCH] D86533: (Urgent!) [release][docs][analyzer] Add 11.0.0. release notes

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:490-491
+
+- Added :ref:`on-demand parsing ` capability to cross 
translation
+  unit analysis.
+

whisperity wrote:
> What's the proper way of naming this feature, @martong @dkrupp @xazax.hun? Is 
> it like this, or should it be in capitals, or should it be 
> `cross`**`-`**`translation unit`?
I suppose `cross-translation` is the grammatically correct (?). We tend to 
refer to the feature, however, with all capitals: Cross Translation Unit. This 
way the CTU abbreviation is immediate.
Perhaps we should write then `Cross Translation Unit (CTU) analysis` to be the 
most precise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86533

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

[PATCH] D84932: [builtins] Add more test cases for __div[sdt]f3 LibCalls

[Anatoly Trosinenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb9f49d13fd44: [compiler-rt][builtins] Add more test cases 
for __div[sdt]f3 LibCalls (authored by atrosinenko).

Changed prior to commit:
  https://reviews.llvm.org/D84932?vs=286523&id=287650#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84932

Files:
  compiler-rt/test/builtins/Unit/divdf3_test.c
  compiler-rt/test/builtins/Unit/divsf3_test.c
  compiler-rt/test/builtins/Unit/divtf3_test.c
  compiler-rt/test/builtins/Unit/fp_test.h

Index: compiler-rt/test/builtins/Unit/fp_test.h
===
--- compiler-rt/test/builtins/Unit/fp_test.h
+++ compiler-rt/test/builtins/Unit/fp_test.h
@@ -253,14 +253,29 @@
 return fromRep32(0x7f80U);
 }
 
+static inline float makeNegativeInf32(void)
+{
+return fromRep32(0xff80U);
+}
+
 static inline double makeInf64(void)
 {
 return fromRep64(0x7ff0UL);
 }
 
+static inline double makeNegativeInf64(void)
+{
+return fromRep64(0xfff0UL);
+}
+
 #if __LDBL_MANT_DIG__ == 113
 static inline long double makeInf128(void)
 {
 return fromRep128(0x7fffUL, 0x0UL);
 }
+
+static inline long double makeNegativeInf128(void)
+{
+return fromRep128(0xUL, 0x0UL);
+}
 #endif
Index: compiler-rt/test/builtins/Unit/divtf3_test.c
===
--- compiler-rt/test/builtins/Unit/divtf3_test.c
+++ compiler-rt/test/builtins/Unit/divtf3_test.c
@@ -32,6 +32,8 @@
 int main()
 {
 #if __LDBL_MANT_DIG__ == 113
+// Returned NaNs are assumed to be qNaN by default
+
 // qNaN / any = qNaN
 if (test__divtf3(makeQNaN128(),
  0x1.23456789abcdefp+5L,
@@ -39,17 +41,111 @@
  UINT64_C(0x0)))
 return 1;
 // NaN / any = NaN
-if (test__divtf3(makeNaN128(UINT64_C(0x80003000)),
+if (test__divtf3(makeNaN128(UINT64_C(0x3000)),
  0x1.23456789abcdefp+5L,
  UINT64_C(0x7fff8000),
  UINT64_C(0x0)))
 return 1;
-// inf / any = inf
-if (test__divtf3(makeInf128(),
- 0x1.23456789abcdefp+5L,
+// any / qNaN = qNaN
+if (test__divtf3(0x1.23456789abcdefp+5L,
+ makeQNaN128(),
+ UINT64_C(0x7fff8000),
+ UINT64_C(0x0)))
+return 1;
+// any / NaN = NaN
+if (test__divtf3(0x1.23456789abcdefp+5L,
+ makeNaN128(UINT64_C(0x3000)),
+ UINT64_C(0x7fff8000),
+ UINT64_C(0x0)))
+return 1;
+
+// +Inf / positive = +Inf
+if (test__divtf3(makeInf128(), 3.L,
  UINT64_C(0x7fff),
  UINT64_C(0x0)))
 return 1;
+// +Inf / negative = -Inf
+if (test__divtf3(makeInf128(), -3.L,
+ UINT64_C(0x),
+ UINT64_C(0x0)))
+return 1;
+// -Inf / positive = -Inf
+if (test__divtf3(makeNegativeInf128(), 3.L,
+ UINT64_C(0x),
+ UINT64_C(0x0)))
+return 1;
+// -Inf / negative = +Inf
+if (test__divtf3(makeNegativeInf128(), -3.L,
+ UINT64_C(0x7fff),
+ UINT64_C(0x0)))
+return 1;
+
+// Inf / Inf = NaN
+if (test__divtf3(makeInf128(), makeInf128(),
+ UINT64_C(0x7fff8000),
+ UINT64_C(0x0)))
+return 1;
+// 0.0 / 0.0 = NaN
+if (test__divtf3(+0x0.0p+0L, +0x0.0p+0L,
+ UINT64_C(0x7fff8000),
+ UINT64_C(0x0)))
+return 1;
+// +0.0 / +Inf = +0.0
+if (test__divtf3(+0x0.0p+0L, makeInf128(),
+ UINT64_C(0x0),
+ UINT64_C(0x0)))
+return 1;
+// +Inf / +0.0 = +Inf
+if (test__divtf3(makeInf128(), +0x0.0p+0L,
+ UINT64_C(0x7fff),
+ UINT64_C(0x0)))
+return 1;
+
+// positive / +0.0 = +Inf
+if (test__divtf3(+1.0L, +0x0.0p+0L,
+ UINT64_C(0x7fff),
+ UINT64_C(0x0)))
+return 1;
+// positive / -0.0 = -Inf
+if (test__divtf3(+1.0L, -0x0.0p+0L,
+ UINT64_C(0x),
+ UINT64_C(0x0)))
+return 1;
+// negative / +0.0 = -Inf
+if (test__divtf3(-1.0L, +0x0.0p+0L,
+ UINT64_C(0x),
+ UINT64_C(0x0)))
+return 1;
+// negative / -0.0 = +Inf
+if (test__divtf3(-1.0L, -0x0.0p+0L,
+ UINT64_C(0x7fff),
+ UINT64_C(0x0)))
+return 1;
+
+// 

[PATCH] D84932: [builtins] Add more test cases for __div[sdt]f3 LibCalls

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko added a comment.

Uploaded, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84932

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

[clang] 121a49d - [LiveDebugValues] Add switches for using instr-ref variable locations

[Jeremy Morse via cfe-commits]

Author: Jeremy Morse
Date: 2020-08-25T14:58:48+01:00
New Revision: 121a49d839d79f5a72be3e22a9d156c9e4b219dc

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

LOG: [LiveDebugValues] Add switches for using instr-ref variable locations

This patch adds the -Xclang option
"-fexperimental-debug-variable-locations" and same LLVM CodeGen option,
to pick which variable location tracking solution to use.

Right now all the switch does is pick which LiveDebugValues
implementation to use, the normal VarLoc one or the instruction
referencing one in rGae6f78824031. Over time, the aim is to add fragments
of support in aid of the value-tracking RFC:

  http://lists.llvm.org/pipermail/llvm-dev/2020-February/139440.html

also controlled by this command line switch. That will slowly move
variable locations to be defined by an instruction calculating a value,
and a DBG_INSTR_REF instruction referring to that value. Thus, this is
going to grow into a "use the new kind of variable locations" switch,
rather than just "use the new LiveDebugValues implementation".

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

Added: 
clang/test/Driver/debug-var-experimental-switch.c

Modified: 
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Frontend/CompilerInvocation.cpp
llvm/include/llvm/CodeGen/CommandFlags.h
llvm/include/llvm/Target/TargetOptions.h
llvm/lib/CodeGen/CommandFlags.cpp
llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index cbd9df998e78..0f03373c4e25 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -325,6 +325,9 @@ ENUM_CODEGENOPT(DebuggerTuning, llvm::DebuggerKind, 2,
 /// emitted.
 VALUE_CODEGENOPT(DwarfVersion, 3, 0)
 
+/// Whether to use experimental new variable location tracking.
+CODEGENOPT(ValueTrackingVariableLocations, 1, 0)
+
 /// Whether we should emit CodeView debug information. It's possible to emit
 /// CodeView and DWARF into the same object.
 CODEGENOPT(EmitCodeView, 1, 0)

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 05aa79d06464..5da072ee4cc2 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3870,6 +3870,9 @@ def fdebug_pass_manager : Flag<["-"], 
"fdebug-pass-manager">,
 HelpText<"Prints debug information for the new pass manager">;
 def fno_debug_pass_manager : Flag<["-"], "fno-debug-pass-manager">,
 HelpText<"Disables debug printing for the new pass manager">;
+def fexperimental_debug_variable_locations : Flag<["-"],
+"fexperimental-debug-variable-locations">,
+HelpText<"Use experimental new value-tracking variable locations">;
 // The driver option takes the key as a parameter to the -msign-return-address=
 // and -mbranch-protection= options, but CC1 has a separate option so we
 // don't have to parse the parameter twice.

diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 566affa91b76..093650ac0066 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -520,6 +520,8 @@ static void initTargetOptions(DiagnosticsEngine &Diags,
   Options.EmitAddrsig = CodeGenOpts.Addrsig;
   Options.ForceDwarfFrameSection = CodeGenOpts.ForceDwarfFrameSection;
   Options.EmitCallSiteInfo = CodeGenOpts.EmitCallSiteInfo;
+  Options.ValueTrackingVariableLocations =
+  CodeGenOpts.ValueTrackingVariableLocations;
   Options.XRayOmitFunctionIndex = CodeGenOpts.XRayOmitFunctionIndex;
 
   Options.MCOptions.SplitDwarfFile = CodeGenOpts.SplitDwarfFile;

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 175985bd950a..0313ff4c363b 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -835,6 +835,9 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList 
&Args, InputKind IK,
   llvm::is_contained(DebugEntryValueArchs, T.getArch()))
 Opts.EmitCallSiteInfo = true;
 
+  Opts.ValueTrackingVariableLocations =
+  Args.hasArg(OPT_fexperimental_debug_variable_locations);
+
   Opts.DisableO0ImplyOptNone = Args.hasArg(OPT_disable_O0_optnone);
   Opts.DisableRedZone = Args.hasArg(OPT_disable_red_zone);
   Opts.IndirectTlsSegRefs = Args.hasArg(OPT_mno_tls_direct_seg_refs);

diff  --git a/clang/test/Driver/debug-var-experimental-switch.c 
b/clang/test/Driver/debug-var-experimental-switch.c
new file mode 100644
index ..9c7a782e9e2b
--- /dev/null
+++ b/clang/test/Driver/debug-var-experimental

[PATCH] D83048: [LiveDebugValues] 3/4 Add Xclang and CodeGen options for using instr-ref variable locations

[Jeremy Morse via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG121a49d839d7: [LiveDebugValues] Add switches for using 
instr-ref variable locations (authored by jmorse).

Changed prior to commit:
  https://reviews.llvm.org/D83048?vs=286814&id=287655#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83048

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/debug-var-experimental-switch.c
  llvm/include/llvm/CodeGen/CommandFlags.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp

Index: llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp
===
--- llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp
+++ llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp
@@ -14,6 +14,7 @@
 #include "llvm/CodeGen/Passes.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Pass.h"
+#include "llvm/Target/TargetMachine.h"
 
 /// \file LiveDebugValues.cpp
 ///
@@ -40,7 +41,10 @@
   static char ID;
 
   LiveDebugValues();
-  ~LiveDebugValues() { delete TheImpl; }
+  ~LiveDebugValues() {
+if (TheImpl)
+  delete TheImpl;
+  }
 
   /// Calculate the liveness information for the given machine function.
   bool runOnMachineFunction(MachineFunction &MF) override;
@@ -57,6 +61,7 @@
 
 private:
   LDVImpl *TheImpl;
+  TargetPassConfig *TPC;
 };
 
 char LiveDebugValues::ID = 0;
@@ -69,10 +74,24 @@
 /// Default construct and initialize the pass.
 LiveDebugValues::LiveDebugValues() : MachineFunctionPass(ID) {
   initializeLiveDebugValuesPass(*PassRegistry::getPassRegistry());
-  TheImpl = llvm::makeVarLocBasedLiveDebugValues();
+  TheImpl = nullptr;
 }
 
 bool LiveDebugValues::runOnMachineFunction(MachineFunction &MF) {
-  auto *TPC = getAnalysisIfAvailable();
+  if (!TheImpl) {
+TPC = getAnalysisIfAvailable();
+
+bool InstrRefBased = false;
+if (TPC) {
+  auto &TM = TPC->getTM();
+  InstrRefBased = TM.Options.ValueTrackingVariableLocations;
+}
+
+if (InstrRefBased)
+  TheImpl = llvm::makeInstrRefBasedLiveDebugValues();
+else
+  TheImpl = llvm::makeVarLocBasedLiveDebugValues();
+  }
+
   return TheImpl->ExtendRanges(MF, TPC);
 }
Index: llvm/lib/CodeGen/CommandFlags.cpp
===
--- llvm/lib/CodeGen/CommandFlags.cpp
+++ llvm/lib/CodeGen/CommandFlags.cpp
@@ -85,6 +85,7 @@
 CGOPT(bool, EnableAddrsig)
 CGOPT(bool, EmitCallSiteInfo)
 CGOPT(bool, EnableDebugEntryValues)
+CGOPT(bool, ValueTrackingVariableLocations)
 CGOPT(bool, ForceDwarfFrameSection)
 CGOPT(bool, XRayOmitFunctionIndex)
 
@@ -400,6 +401,12 @@
   cl::init(false));
   CGBINDOPT(EnableDebugEntryValues);
 
+  static cl::opt ValueTrackingVariableLocations(
+  "experimental-debug-variable-locations",
+  cl::desc("Use experimental new value-tracking variable locations"),
+  cl::init(false));
+  CGBINDOPT(ValueTrackingVariableLocations);
+
   static cl::opt ForceDwarfFrameSection(
   "force-dwarf-frame-section",
   cl::desc("Always emit a debug frame section."), cl::init(false));
@@ -475,6 +482,7 @@
   Options.EmitAddrsig = getEnableAddrsig();
   Options.EmitCallSiteInfo = getEmitCallSiteInfo();
   Options.EnableDebugEntryValues = getEnableDebugEntryValues();
+  Options.ValueTrackingVariableLocations = getValueTrackingVariableLocations();
   Options.ForceDwarfFrameSection = getForceDwarfFrameSection();
   Options.XRayOmitFunctionIndex = getXRayOmitFunctionIndex();
 
Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -126,8 +126,8 @@
   EmitStackSizeSection(false), EnableMachineOutliner(false),
   SupportsDefaultOutlining(false), EmitAddrsig(false),
   EmitCallSiteInfo(false), SupportsDebugEntryValues(false),
-  EnableDebugEntryValues(false), ForceDwarfFrameSection(false),
-  XRayOmitFunctionIndex(false),
+  EnableDebugEntryValues(false), ValueTrackingVariableLocations(false),
+  ForceDwarfFrameSection(false), XRayOmitFunctionIndex(false),
   FPDenormalMode(DenormalMode::IEEE, DenormalMode::IEEE) {}
 
 /// DisableFramePointerElim - This returns true if frame pointer elimination
@@ -285,6 +285,11 @@
 /// production.
 bool ShouldEmitDebugEntryValues() const;
 
+// When set to true, use experimental new debug variable location tracking,
+// which seeks to follow the values of variables rather than their location,
+// post isel.
+unsigned ValueTrackingVariableLocations : 1;
+
 /// Emit DWARF debug frame section.
  

[PATCH] D85485: Fix quiet mode in git-clang-format

[Gvald Ike via Phabricator via cfe-commits]

Gvald added a comment.

@serge-sans-paille - the diff is PEP8 compliant now.
Care to commit, as I don't have the right permissions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85485

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

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

- made bool-vector layout target dependent (one byte per bool element on 
Hexagon, one bit everywhere else).
- Added hvx-specific debug info test.
- Updated docs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/ast-print-vector-size-bool.c
  clang/test/CodeGen/debug-info-vector-bool-hvx64.c
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/CodeGen/vector-alignment.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector-bool.cpp
  clang/test/SemaCXX/vector-conditional.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int &reference_to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/test/SemaCXX/vector-conditional.cpp
===
--- clang/test/SemaCXX/vector-conditional.cpp
+++ clang/test/SemaCXX/vector-conditional.cpp
@@ -13,6 +13,7 @@
 using FourFloats = float __attribute__((__vector_size__(16)));
 using TwoDoubles = double __attribute__((__vector_size__(16)));
 using FourDoubles = double __attribute__((__vector_size__(32)));
+using EightBools = bool __attribute__((__vector_size__(2)));
 
 FourShorts four_shorts;
 TwoInts two_ints;
@@ -25,6 +26,8 @@
 FourFloats four_floats;
 TwoDoubles two_doubles;
 FourDoubles four_doubles;
+EightBools eight_bools;
+EightBools other_eight_bools;
 
 enum E {};
 enum class SE {};
@@ -95,6 +98,9 @@
   (void)(four_ints ? four_uints : 3.0f);
   (void)(four_ints ? four_ints : 3.0f);
 
+  // Allow conditional select on bool vectors.
+  (void)(eight_bools ? eight_bools : other_eight_bools);
+
   // When there is a vector and a scalar, conversions must be legal.
   (void)(four_ints ? four_floats : 3); // should work, ints can convert to floats.
   (void)(four_ints ? four_uints : e);  // expected-error {{cannot convert between scalar type 'E' and vector type 'FourUInts'}}
@@ -163,10 +169,10 @@
 void Templates() {
   dependent_cond(two_ints);
   dependent_operand(two_floats);
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
   all_dependent(four_ints, four_uints, four_doubles); // expected-note {{in instantiation of}}
 
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 'unsigned int' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsig

[PATCH] D85934: Enable OpenMP offloading to VE and enable tests for offloading to VE

[Manoel Roemmer via Phabricator via cfe-commits]

manorom updated this revision to Diff 287660.
manorom edited the summary of this revision.
manorom added a comment.

- Move VE specific vars into test run line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85934

Files:
  clang/lib/Frontend/CompilerInvocation.cpp
  openmp/libomptarget/CMakeLists.txt
  openmp/libomptarget/test/api/omp_get_num_devices.c
  openmp/libomptarget/test/api/omp_get_num_devices_with_empty_target.c
  openmp/libomptarget/test/env/omp_target_debug.c
  openmp/libomptarget/test/lit.cfg
  openmp/libomptarget/test/mapping/alloc_fail.c
  openmp/libomptarget/test/mapping/target_implicit_partial_map.c
  openmp/libomptarget/test/offloading/offloading_success.c

Index: openmp/libomptarget/test/offloading/offloading_success.c
===
--- openmp/libomptarget/test/offloading/offloading_success.c
+++ openmp/libomptarget/test/offloading/offloading_success.c
@@ -3,6 +3,7 @@
 // RUN: %libomptarget-compile-run-and-check-powerpc64le-ibm-linux-gnu
 // RUN: %libomptarget-compile-run-and-check-x86_64-pc-linux-gnu
 // RUN: %libomptarget-compile-run-and-check-nvptx64-nvidia-cuda
+// RUN: %libomptarget-compile-ve-unknown-linux-unknown | env VE_LD_LIBRARY_PATH=%host-rtl-dir %libomptarget-run-ve-unknown-linux-unknown | %fcheck-ve-unknown-linux-unknown
 
 #include 
 #include 
Index: openmp/libomptarget/test/mapping/target_implicit_partial_map.c
===
--- openmp/libomptarget/test/mapping/target_implicit_partial_map.c
+++ openmp/libomptarget/test/mapping/target_implicit_partial_map.c
@@ -13,6 +13,10 @@
 // RUN: %libomptarget-compile-x86_64-pc-linux-gnu
 // RUN: %libomptarget-run-x86_64-pc-linux-gnu 2>&1 \
 // RUN: | %fcheck-x86_64-pc-linux-gnu
+
+// RUN: %libomptarget-compile-ve-unknown-linux-unknown
+// RUN: env VE_LD_LIBRARY_PATH=%host-rtl-dir %libomptarget-run-ve-unknown-linux-unknown 2>&1 \
+// RUN: | %fcheck-ve-unknown-linux-unknown
 //
 // END.
 
Index: openmp/libomptarget/test/mapping/alloc_fail.c
===
--- openmp/libomptarget/test/mapping/alloc_fail.c
+++ openmp/libomptarget/test/mapping/alloc_fail.c
@@ -18,6 +18,10 @@
 // RUN: %libomptarget-run-fail-nvptx64-nvidia-cuda 2>&1 \
 // RUN: | %fcheck-nvptx64-nvidia-cuda
 
+// RUN: %libomptarget-compile-ve-unknown-linux-unknown
+// RUN: env VE_LD_LIBRARY_PATH=%host-rtl-dir %libomptarget-run-fail-ve-unknown-linux-unknown 2>&1 \
+// RUN: | %fcheck-ve-unknown-linux-unknown
+
 // CHECK: Libomptarget fatal error 1: failure of target construct while offloading is mandatory
 
 int main() {
Index: openmp/libomptarget/test/lit.cfg
===
--- openmp/libomptarget/test/lit.cfg
+++ openmp/libomptarget/test/lit.cfg
@@ -75,6 +75,11 @@
 append_dynamic_library_path('LIBRARY_PATH', \
 config.omp_host_rtl_directory, ":")
 
+# Target specific flags
+target_flags = {}
+target_flags['ve-unknown-linux-unknown'] = "-Xopenmp-target \"-L " + \
+config.omp_host_rtl_directory + "\""
+
 # substitutions
 # - for targets that exist in the system create the actual command.
 # - for valid targets that do not exist in the system, return false, so that the
@@ -116,9 +121,13 @@
 libomptarget_target, \
 "%not --crash %t-" + libomptarget_target))
 config.substitutions.append(("%clangxx-" + libomptarget_target, \
-"%clangxx %openmp_flags %flags -fopenmp-targets=" + libomptarget_target))
+"%clangxx %openmp_flags %flags-" + libomptarget_target + \
+" -fopenmp-targets=" + libomptarget_target))
 config.substitutions.append(("%clang-" + libomptarget_target, \
-"%clang %openmp_flags %flags -fopenmp-targets=" + libomptarget_target))
+"%clang %openmp_flags %flags-" + libomptarget_target + \
+" -fopenmp-targets=" + libomptarget_target))
+config.substitutions.append(("%flags-" + libomptarget_target, \
+"%flags " + target_flags.get(libomptarget_target, "")))
 config.substitutions.append(("%fcheck-" + libomptarget_target, \
 config.libomptarget_filecheck + " %s"))
 else:
@@ -157,4 +166,5 @@
 config.substitutions.append(("%clang", config.test_c_compiler))
 config.substitutions.append(("%openmp_flags", config.test_openmp_flags))
 config.substitutions.append(("%flags", config.test_flags))
+config.substitutions.append(("%host-rtl-dir", config.omp_host_rtl_directory))
 config.substitutions.append(("%not", config.libomptarget_not))
Index: openmp/libomptarget/test/env/omp_target_debug.c
===
--- openmp/libomptarget/test/env/omp_target_debug.c
+++ openmp/libomptarget/test/env/omp_target_debug.c
@@ -8,6 +8,8 @@
 // RUN: %libomptarget-compile-x86_64-pc-linux-gnu && 

[PATCH] D86065: [SVE] Make ElementCount members private

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added a comment.

In D86065#2235434 , @david-arm wrote:

> Hi @fpetrogalli, if you don't mind I think I'll stick with Paul's idea for 
> ogt because this matches the IR neatly, i.e. "fcmp ogt". Also, for me 
> personally it's much simpler and more intuitive.

I don't mind at all, please follow Paul's suggestion, mine was just an 
alternative. Thanks!


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

https://reviews.llvm.org/D86065

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

[PATCH] D86465: [analyzer][solver] Redesign constraint ranges data structure

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

This is a huge change, I've got pretty tired at the end of the review.
I haven't checked the test-cases too deeply.

I've found only minor typos, clarity enhancement opportunities etc. nothing 
serious.
It was awesome to see how this code can be improved, even becoming 
comprehensible.

Sorry for my nit spam if it needs further discussion - but it looks good to me.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:35-36
 
-  Range(const llvm::APSInt &point)
-  : std::pair(&point, &point) 
{}
+  Range(const llvm::APSInt &Point)
+  : std::pair(&Point, &Point) 
{}
 

I don't think we should inherit from standard type. It should be a private 
member instead.
https://quuxplusone.github.io/blog/2018/12/11/dont-inherit-from-std-types/#never-ever-inherit-from-any-std



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:51
   }
-};
+  void print(raw_ostream &OS) const;
 

We have `dump` methods all over the place. Except this.
I propose to rename this to `dump`.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:53-55
+  // In order to keep non-overlapping ranges sorted, we can compare only From
+  // points.
+  inline bool operator<(const Range &RHS) const { return From() < RHS.From(); }

It's a good practice to define comparison operators as //friend// functions 
inline.
Even if we don't rely on implicit conversions.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:110
+  iterator end() const { return Impl->end(); }
+  size_t size() const { return Impl->size(); }
+

I think it should be `std::size_t`.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:118-123
+/// Create a new set with all ranges from both LHS and RHS.
+/// Possible intersections are not checked here.
+///
+/// Complexity: O(N + M)
+/// where N = size(LHS), M = size(RHS)
+RangeSet add(RangeSet LHS, RangeSet RHS);

What about `merge` or `combine`? I know that `union` is a keyword thus we can 
not use.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:135
+/// @{
+RangeSet getSet(Range Origin);
+RangeSet getSet(const llvm::APSInt &From, const llvm::APSInt &To) {

NoQ wrote:
> "But what about `setGet()`???" - some user, probably :)
Why don't we call this `createSetOf`?
And `createEmptySet`.
I know that we don't create the empty set, but then what does a //factory// if 
not create stuff?



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:149-160
+/// Intersect the given set with the closed range [Lower, Upper].
+///
+/// Unlike the Range type, this range uses modular arithmetic, 
corresponding
+/// to the common treatment of C integer overflow. Thus, if the Lower bound
+/// is greater than the Upper bound, the range is taken to wrap around. 
This
+/// is equivalent to taking the intersection with the two ranges [Min,
+/// Upper] and [Lower, Max], or, alternatively, /removing/ all integers

I think an example would be awesome here.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:170-174
+/// Delete the given point from the range set.
+///
+/// Complexity: O(N)
+/// where N = size(From)
+RangeSet deletePoint(const llvm::APSInt &Point, RangeSet From);

Why does this operation take `O(N)` time? Shouldn't be `O(logN)` instead?



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:190-193
+/// And it makes us to separate the range
+/// like [MIN, N] to [MIN, MIN] U [-N, MAX].
+/// For instance, whole range is {-128..127} and subrange is [-128,-126],
+/// thus [-128,-127,-126,...] negates to [-128,...,126,127].





Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:195-196
+///
+/// Negate restores disrupted ranges on bounds,
+/// e.g. [MIN, B] => [MIN, MIN] U [-B, MAX] => [MIN, B].
+///





Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:203
+  private:
+RangeSet makePersistent(ContainerType &&From);
+ContainerType *construct(ContainerType &&From);

As commented later, this function worth to be documented.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:207-208
+
+RangeSet inte

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Lucas, this seems to have casued https://bugs.llvm.org/show_bug.cgi?id=47001. 
Can you take a look? (I would cc you on the bug, but I couldn't find your email 
in bugzilla.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75169

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

[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

I'm not sure about the //status// of this patch.
If you say that further improvements will be done later and this functionality 
is enough, I'm fine with that.




Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:901
+
+  void next(Token &Result) {
+if (CurrTokenIt == TokenRange.end()) {

Szelethus wrote:
> steakhal wrote:
> > I don't like output parameters.
> > If we really need them, we should at least have a suspicious name.
> > Unfortunately, I can not come up with anything useful :|
> This is intentional, it meant to replicate the `Lexer`'s interface. I would 
> prefer to keep this as-is.
Sure, be it.



Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:1338-1339
 void TokenPrinter::printToken(const Token &Tok) {
+  // TODO: Handle the case where hash and hashhash occurs right before
+  // __VA_ARGS__.
+

Szelethus wrote:
> steakhal wrote:
> > What does //hashhash// mean? I might lack some context though :D
> `#` and `##` respectively. The test cases you pointed out as flawed refer to 
> this FIXME, though a FIXME in the tests themselves wouldn't hurt.
Maybe `HashtagHashtag`?
Or an example would be even better like: `##__VA_ARGS__`



Comment at: clang/test/Analysis/plist-macros-with-expansion.cpp:481
+  i = 0;
+#define DISPATCH(...) PARAMS_RESOLVE_TO_VA_ARGS(__VA_ARGS__);
+

Szelethus wrote:
> steakhal wrote:
> > You don't need an ending semicolon here. It will be already there at the 
> > expansion location.
> > This way you introduce an empty expression after the macro expansion.
> > The same happens in all the other cases as well.
> You are correct, though the point of macro expansion testing is to see 
> whether we nailed what the preprocessor is supposed to do -- not whether the 
> code it creates makes such sense. In fact, I would argue that most GNU 
> extensions to the preprocessor shouldn't be a thing, but we still need to 
> support it.
Oh, now I get it.
I didn't know that this was ann extension lol.



Comment at: clang/test/Analysis/plist-macros-with-expansion.cpp:484-486
+  int x = 1;
+  DISPATCH(x, "LF1M healer");
+  (void)(10 / x); // expected-warning{{Division by zero}}

Szelethus wrote:
> steakhal wrote:
> > Should we really abuse the division by zero checker here?
> > Can't we just use an ExprInspection call here?
> > Maybe it requires a specific BugPath visitor, and that is why we do it this 
> > way?
> We could totally use `ExprInspection` -- but I'd argue that using something 
> else isn't an abuse of the specific checker :) Since the entire file is 
> already written this way, and would demand changes in the large plist file, 
> I'd prefer to keep it this way.
Perfectly fine. I agree with you knowing this. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86135

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

[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 287669.
ASDenysPetrov added a comment.

Added recursive_mutex support.


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

https://reviews.llvm.org/D85984

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
  clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp

Index: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
===
--- /dev/null
+++ clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp
@@ -0,0 +1,207 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.cplusplus.CPlusPlus11Lock -verify %s
+
+namespace std {
+
+struct mutex {
+  void lock();
+  bool try_lock();
+  void unlock();
+};
+
+struct recursive_mutex {
+  void lock();
+  bool try_lock();
+  void unlock();
+};
+
+template 
+struct guard_lock {
+  T &t;
+  guard_lock(T &m) : t(m) {
+t.lock();
+  }
+  ~guard_lock() {
+t.unlock();
+  }
+};
+} // namespace std
+
+std::mutex m1;
+std::mutex m2;
+
+// mutex ok
+
+void m_ok1() {
+  m1.lock(); // no-warning
+}
+
+void m_ok2() {
+  m1.unlock(); // no-warning
+}
+
+void m_ok3() {
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+}
+
+void m_ok4() {
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+}
+
+void m_ok5() {
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+  m2.lock();   // no-warning
+  m2.unlock(); // no-warning
+}
+
+void m_ok6(void) {
+  m1.lock();   // no-warning
+  m2.lock();   // no-warning
+  m2.unlock(); // no-warning
+  m1.unlock(); // no-warning
+}
+
+void m_ok7(void) {
+  if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void m_ok8(void) {
+  m1.unlock();   // no-warning
+  if (m1.try_lock()) // no-warning
+m1.unlock(); // no-warning
+}
+
+void m_ok9(void) {
+  if (!m1.try_lock()) // no-warning
+m1.lock();// no-warning
+  m1.unlock();// no-warning
+}
+
+void m_ok10() {
+  std::guard_lock gl1(m1); // no-warning
+}
+
+// mutex bad
+
+void m_bad1() {
+  m1.lock(); // no-warning
+  m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+void m_bad2() {
+  m1.lock();   // no-warning
+  m1.unlock(); // no-warning
+  m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void m_bad3() {
+  m1.lock();   // no-warning
+  m2.lock();   // no-warning
+  m1.unlock(); // expected-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
+  m2.unlock(); // no-warning
+}
+
+void m_bad5() {
+  while (true)
+m1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void m_bad6() {
+  while (true)
+m1.lock(); // expected-warning{{This lock has already been acquired}}
+}
+
+std::recursive_mutex rm1;
+std::recursive_mutex rm2;
+
+// recursive_mutex ok
+
+void rm_ok1() {
+  rm1.lock(); // no-warning
+}
+
+void rm_ok2() {
+  rm1.unlock(); // no-warning
+}
+
+void rm_ok3() {
+  rm1.lock();   // no-warning
+  rm1.unlock(); // no-warning
+}
+
+void rm_ok4() {
+  rm1.lock();   // no-warning
+  rm1.unlock(); // no-warning
+  rm1.lock();   // no-warning
+  rm1.unlock(); // no-warning
+}
+
+void rm_ok5() {
+  rm1.lock();   // no-warning
+  rm1.unlock(); // no-warning
+  rm2.lock();   // no-warning
+  rm2.unlock(); // no-warning
+}
+
+void rm_ok6(void) {
+  rm1.lock();   // no-warning
+  rm2.lock();   // no-warning
+  rm2.unlock(); // no-warning
+  rm1.unlock(); // no-warning
+}
+
+void rm_ok7(void) {
+  if (rm1.try_lock()) // no-warning
+rm1.unlock(); // no-warning
+}
+
+void rm_ok8(void) {
+  rm1.unlock();   // no-warning
+  if (rm1.try_lock()) // no-warning
+rm1.unlock(); // no-warning
+}
+
+void rm_ok9(void) {
+  if (!rm1.try_lock()) // no-warning
+rm1.lock();// no-warning
+  rm1.unlock();// no-warning
+}
+
+void rm_ok10() {
+  std::guard_lock gl2(rm1); // no-warning
+}
+
+void rm_ok11() {
+  rm1.lock(); // no-warning
+  rm1.lock(); // no-warning
+}
+
+void rm_ok12() {
+  while (true)
+rm1.lock(); // no-warning
+}
+
+// recursive_mutex bad
+
+void rm_bad1() {
+  rm1.lock();   // no-warning
+  rm1.unlock(); // no-warning
+  rm1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
+
+void rm_bad2() {
+  rm1.lock();   // no-warning
+  rm2.lock();   // no-warning
+  rm1.unlock(); // expected-warning {{This was not the most recently acquired lock. Possible lock order reversal}}
+  rm2.unlock(); // no-warning
+}
+
+void rm_bad3() {
+  while (true)
+rm1.unlock(); // expected-warning {{This lock has already been unlocked}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/PthreadLockChecker.cpp
@@ -70,11 +70

[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan added a comment.

In D85324#2234712 , 
@hubert.reinterpretcast wrote:

> In D85324#2233290 , 
> @abhina.sreeskantharajan wrote:
>
>> Thanks Hubert, I fixed the comment.
>
> Got it; I'll look into committing this.

Thanks again!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85324

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

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

[Zachary Henkel via Phabricator via cfe-commits]

zahen updated this revision to Diff 287671.
zahen added a comment.
Herald added a subscriber: ormris.

It felt too invasive to swap the precedence of using vctoolsdir vs %INCLUDE% 
for all clang-cl users so I compromised by skipping the %INCLUDE% check when 
vctoolsdir was passed on the command line.  This way only the users of the new 
flag will see different behavior.

With that change the suggested test for -internal-isystem passes in both 
vanilla and vc command prompts.


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

https://reviews.llvm.org/D85998

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/test/Driver/cl-options.c

Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -682,4 +682,13 @@
 // CLANG-NOT: "--dependent-lib=libcmt"
 // CLANG-NOT: "-vectorize-slp"
 
+// Validate that the default triple is used when run an empty tools dir is specified
+// RUN: %clang_cl -vctoolsdir "" -### -- %s 2>&1 | FileCheck %s --check-prefix VCTOOLSDIR
+// VCTOOLSDIR: "-cc1" "-triple" "x86_64-pc-windows-msvc19.11.0"
+
+// Validate that built-in include paths are based on the supplied path
+// RUN: %clang_cl -vctoolsdir "/fake" -### -- %s 2>&1 | FileCheck %s --check-prefix FAKEDIR
+// FAKEDIR: "-internal-isystem" "/fake\\include"
+// FAKEDIR: "-internal-isystem" "/fake\\atlmfc\\include"
+
 void f() { }
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -66,6 +66,20 @@
 static bool getSystemRegistryString(const char *keyPath, const char *valueName,
 std::string &value, std::string *phValue);
 
+// Check command line arguments to try and find a toolchain.
+static bool
+findVCToolChainViaCommandLine(const ArgList &Args, std::string &Path,
+  MSVCToolChain::ToolsetLayout &VSLayout) {
+  // Don't validate the input; trust the value supplied by the user.
+  // The primary motivation is to prevent unnecessary file and registry access.
+  if (Arg *A = Args.getLastArg(options::OPT__SLASH_vctoolsdir)) {
+Path = A->getValue();
+VSLayout = MSVCToolChain::ToolsetLayout::VS2017OrNewer;
+return true;
+  }
+  return false;
+}
+
 // Check various environment variables to try and find a toolchain.
 static bool findVCToolChainViaEnvironment(std::string &Path,
   MSVCToolChain::ToolsetLayout &VSLayout) {
@@ -747,11 +761,12 @@
   if (getDriver().getInstalledDir() != getDriver().Dir)
 getProgramPaths().push_back(getDriver().Dir);
 
-  // Check the environment first, since that's probably the user telling us
-  // what they want to use.
-  // Failing that, just try to find the newest Visual Studio version we can
-  // and use its default VC toolchain.
-  findVCToolChainViaEnvironment(VCToolChainPath, VSLayout) ||
+  // Check the command line first, that's the user explicitly telling us what to
+  // use. Check the environment next, in case we're being invoked from a VS
+  // command prompt. Failing that, just try to find the newest Visual Studio
+  // version we can and use its default VC toolchain.
+  findVCToolChainViaCommandLine(Args, VCToolChainPath, VSLayout) ||
+  findVCToolChainViaEnvironment(VCToolChainPath, VSLayout) ||
   findVCToolChainViaSetupConfig(VCToolChainPath, VSLayout) ||
   findVCToolChainViaRegistry(VCToolChainPath, VSLayout);
 }
@@ -1263,15 +1278,18 @@
 return;
 
   // Honor %INCLUDE%. It should know essential search paths with vcvarsall.bat.
-  if (llvm::Optional cl_include_dir =
-  llvm::sys::Process::GetEnv("INCLUDE")) {
-SmallVector Dirs;
-StringRef(*cl_include_dir)
-.split(Dirs, ";", /*MaxSplit=*/-1, /*KeepEmpty=*/false);
-for (StringRef Dir : Dirs)
-  addSystemInclude(DriverArgs, CC1Args, Dir);
-if (!Dirs.empty())
-  return;
+  // Skip if the user expressly set a vctoolsdir
+  if (!DriverArgs.getLastArg(options::OPT__SLASH_vctoolsdir)) {
+if (llvm::Optional cl_include_dir =
+llvm::sys::Process::GetEnv("INCLUDE")) {
+  SmallVector Dirs;
+  StringRef(*cl_include_dir)
+  .split(Dirs, ";", /*MaxSplit=*/-1, /*KeepEmpty=*/false);
+  for (StringRef Dir : Dirs)
+addSystemInclude(DriverArgs, CC1Args, Dir);
+  if (!Dirs.empty())
+return;
+}
   }
 
   // When built with access to the proper Windows APIs, try to actually find
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4737,6 +4737,8 @@
 def _SLASH_Tp : CLCompileJoinedOrSeparate<"Tp">,
   HelpText<"Treat  as C++ source file">, MetaVarName<"">;
 def _SLASH_T

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

[Zachary Henkel via Phabricator via cfe-commits]

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



Comment at: clang/test/Driver/cl-options.c:686
+// vctoolsdir is handled by the driver; just check that we don't error. Pass 
-c because fakedir isn't a real toolchain path
+// RUN: %clang_cl -c -vctoolsdir fakedir -- %s 2>&1
+

hans wrote:
> zahen wrote:
> > zahen wrote:
> > > hans wrote:
> > > > zahen wrote:
> > > > > hans wrote:
> > > > > > aganea wrote:
> > > > > > > Check that we're not detecting a local installation, and that we 
> > > > > > > fallback to the default triple 19.11, ie.
> > > > > > > ```
> > > > > > > // RUN: %clang_cl ... -vctoolsdir "" -### | FileCheck %s 
> > > > > > > --check-prefix VCTOOLSDIR
> > > > > > > // VCTOOLSDIR: "-cc1" "-triple" "x86_64-pc-windows-msvc19.11.0"
> > > > > > > ```
> > > > > > Maybe add a comment explaining the purpose of the test.
> > > > > > 
> > > > > > And would it be possible to have a test where -vctoolsdir is set to 
> > > > > > something, to see that it's picked up correctly?
> > > > > What's the expectation on the test boxes?  I can author such a test 
> > > > > but it would be very brittle unless we have a spec for how VS should 
> > > > > be installed.
> > > > I'd suggest passing -vctoolsdir "/foo" and check that some /foo dir is 
> > > > getting passed as a system include dir to cc1, and that the link.exe 
> > > > invocation is under /foo.
> > > > 
> > > > I don't think it would be brittle.
> > > Clever idea! I'll do it that way and key off the "ignored directory" 
> > > warning that's emitted.
> > Did further digging and the path test would depend on other environment 
> > variables.  If the %INCLUDE% variable is set, as it is by vcvars.bat, those 
> > paths are adopted and VCToolChainPath is not used.  I'm hesitant to change 
> > that logic without stronger motivation.
> > 
> > Other ideas for a test that would work run both from a vc command prompt 
> > and a vanilla command prompt?  I don't see anything else in the full 
> > verbose invocation to key off of:
> > 
> > 
> > ```
> > D:\repos\llvm\llvm-project\build\Debug\bin>clang-cl -vctoolsdir "/fake" -v 
> > main.cpp
> > clang version 12.0.0 (https://github.com/llvm/llvm-project 
> > 537f5483fe4e09c39ffd7ac837c00b50dd13d676)
> > Target: x86_64-pc-windows-msvc
> > Thread model: posix
> > InstalledDir: D:\repos\llvm\llvm-project\build\Debug\bin
> >  "D:\\repos\\llvm\\llvm-project\\build\\Debug\\bin\\clang-cl.exe" -cc1 
> > -triple x86_64-pc-windows-msvc19.11.0 -emit-obj -mrelax-all 
> > -mincremental-linker-compatible --mrelax-relocations -disable-free 
> > -main-file-name main.cpp -mrelocation-model pic -pic-level 2 
> > -mframe-pointer=none -relaxed-aliasing -fmath-errno -fno-rounding-math 
> > -mconstructor-aliases -munwind-tables -target-cpu x86-64 -mllvm 
> > -x86-asm-syntax=intel -D_MT -flto-visibility-public-std 
> > --dependent-lib=libcmt --dependent-lib=oldnames -stack-protector 2 
> > -fms-volatile -fdiagnostics-format msvc -v -resource-dir 
> > "D:\\repos\\llvm\\llvm-project\\build\\Debug\\lib\\clang\\12.0.0" 
> > -internal-isystem 
> > "D:\\repos\\llvm\\llvm-project\\build\\Debug\\lib\\clang\\12.0.0\\include" 
> > -internal-isystem "C:\\Program Files (x86)\\Microsoft Visual 
> > Studio\\2019\\Enterprise\\VC\\Tools\\MSVC\\14.27.29110\\ATLMFC\\include" 
> > -internal-isystem "C:\\Program Files (x86)\\Microsoft Visual 
> > Studio\\2019\\Enterprise\\VC\\Tools\\MSVC\\14.27.29110\\include" 
> > -internal-isystem "C:\\Program Files (x86)\\Windows 
> > Kits\\NETFXSDK\\4.8\\include\\um" -internal-isystem "C:\\Program Files 
> > (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\ucrt" -internal-isystem 
> > "C:\\Program Files (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\shared" 
> > -internal-isystem "C:\\Program Files (x86)\\Windows 
> > Kits\\10\\include\\10.0.18362.0\\um" -internal-isystem "C:\\Program Files 
> > (x86)\\Windows Kits\\10\\include\\10.0.18362.0\\winrt" -internal-isystem 
> > "C:\\Program Files (x86)\\Windows 
> > Kits\\10\\include\\10.0.18362.0\\cppwinrt" -fdeprecated-macro 
> > -fdebug-compilation-dir "D:\\repos\\llvm\\llvm-project\\build\\Debug\\bin" 
> > -ferror-limit 19 -fmessage-length=121 -fno-use-cxa-atexit -fms-extensions 
> > -fms-compatibility -fms-compatibility-version=19.11 -std=c++14 
> > -fdelayed-template-parsing -fcolor-diagnostics -faddrsig -o 
> > "C:\\Users\\zahen\\AppData\\Local\\Temp\\main-8f329a.obj" -x c++ main.cpp
> > clang -cc1 version 12.0.0 based upon LLVM 12.0.0git default target 
> > x86_64-pc-windows-msvc
> > #include "..." search starts here:
> > #include <...> search starts here:
> >  D:\repos\llvm\llvm-project\build\Debug\lib\clang\12.0.0\include
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2019\Enterprise\VC\Tools\MSVC\14.27.29110\ATLMFC\include
> >  C:\Program Files (x86)\Microsoft Visual 
> > Studio\2019\Enterprise\VC\Tools\MSVC\14.27.29110\include
> >  C:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\include\um
> >  C:\Progra

[PATCH] D86065: [SVE] Make ElementCount members private

[David Sherwood via Phabricator via cfe-commits]

david-arm updated this revision to Diff 287672.
david-arm added a comment.
Herald added a subscriber: rogfer01.

- Changed comparison function from gt to ogt and added a olt (less than) 
comparison function too.
- Instead of adding the ">>=" operator I've added "/=" instead as I think this 
is more common. In places where ">>= 1" was used we now do "/= 2".
- After rebasing it was necessary to add a "*=" operator too for the Loop 
Vectorizer.


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

https://reviews.llvm.org/D86065

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  llvm/include/llvm/Analysis/TargetTransformInfo.h
  llvm/include/llvm/Analysis/VectorUtils.h
  llvm/include/llvm/CodeGen/ValueTypes.h
  llvm/include/llvm/IR/DataLayout.h
  llvm/include/llvm/IR/DerivedTypes.h
  llvm/include/llvm/IR/Instructions.h
  llvm/include/llvm/Support/MachineValueType.h
  llvm/include/llvm/Support/TypeSize.h
  llvm/lib/Analysis/InstructionSimplify.cpp
  llvm/lib/Analysis/VFABIDemangling.cpp
  llvm/lib/Analysis/ValueTracking.cpp
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/TargetLoweringBase.cpp
  llvm/lib/CodeGen/ValueTypes.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/ConstantFold.cpp
  llvm/lib/IR/Constants.cpp
  llvm/lib/IR/DataLayout.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/IRBuilder.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/IntrinsicInst.cpp
  llvm/lib/IR/Type.cpp
  llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
  llvm/lib/Transforms/Utils/FunctionComparator.cpp
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/lib/Transforms/Vectorize/VPlan.cpp
  llvm/lib/Transforms/Vectorize/VPlan.h
  llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
  llvm/unittests/IR/VectorTypesTest.cpp

Index: llvm/unittests/IR/VectorTypesTest.cpp
===
--- llvm/unittests/IR/VectorTypesTest.cpp
+++ llvm/unittests/IR/VectorTypesTest.cpp
@@ -119,8 +119,8 @@
   EXPECT_EQ(ConvTy->getElementType()->getScalarSizeInBits(), 64U);
 
   EltCnt = V8Int64Ty->getElementCount();
-  EXPECT_EQ(EltCnt.Min, 8U);
-  ASSERT_FALSE(EltCnt.Scalable);
+  EXPECT_EQ(EltCnt.getKnownMinValue(), 8U);
+  ASSERT_FALSE(EltCnt.isScalable());
 }
 
 TEST(VectorTypesTest, Scalable) {
@@ -215,8 +215,8 @@
   EXPECT_EQ(ConvTy->getElementType()->getScalarSizeInBits(), 64U);
 
   EltCnt = ScV8Int64Ty->getElementCount();
-  EXPECT_EQ(EltCnt.Min, 8U);
-  ASSERT_TRUE(EltCnt.Scalable);
+  EXPECT_EQ(EltCnt.getKnownMinValue(), 8U);
+  ASSERT_TRUE(EltCnt.isScalable());
 }
 
 TEST(VectorTypesTest, BaseVectorType) {
@@ -250,7 +250,7 @@
 // test I == J
 VectorType *VI = VTys[I];
 ElementCount ECI = VI->getElementCount();
-EXPECT_EQ(isa(VI), ECI.Scalable);
+EXPECT_EQ(isa(VI), ECI.isScalable());
 
 for (size_t J = I + 1, JEnd = VTys.size(); J < JEnd; ++J) {
   // test I < J
Index: llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
===
--- llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
+++ llvm/unittests/CodeGen/ScalableVectorMVTsTest.cpp
@@ -71,8 +71,8 @@
 
   // Check fields inside llvm::ElementCount
   EltCnt = Vnx4i32.getVectorElementCount();
-  EXPECT_EQ(EltCnt.Min, 4U);
-  ASSERT_TRUE(EltCnt.Scalable);
+  EXPECT_EQ(EltCnt.getKnownMinValue(), 4U);
+  ASSERT_TRUE(EltCnt.isScalable());
 
   // Check that fixed-length vector types aren't scalable.
   EVT V8i32 = EVT::getVectorVT(Ctx, MVT::i32, 8);
@@ -82,8 +82,8 @@
 
   // Check that llvm::ElementCount works for fixed-length types.
   EltCnt = V8i32.getVectorElementCount();
-  EXPECT_EQ(EltCnt.Min, 8U);
-  ASSERT_FALSE(EltCnt.Scalable);
+  EXPECT_EQ(EltCnt.getKnownMinValue(), 8U);
+  ASSERT_FALSE(EltCnt.isScalable());
 }
 
 TEST(ScalableVectorMVTsTest, IRToVTTranslation) {
Index: llvm/lib/Transforms/Vectorize/VPlan.h
===
--- llvm/lib/Transforms/Vectorize/VPlan.h
+++ llvm/lib/Transforms/Vectorize/VPlan.h
@@ -151,14 +151,15 @@
   /// \return True if the map has a scalar entry for \p Key and \p Instance.
   bool hasScalarValue(Value *Key, const VPIteration &Instance) const {
 assert(Instance.Part < UF && "Queried Scalar Part is too large.");
-assert(Instance.Lane < VF.Min && "Queried Scalar Lane is too large.");
-assert(!VF.Scalable && "VF is assumed to be non scalable.");
+assert(Instance.Lane < VF.getKnownMinValue() &&
+   "Queried Scalar Lane is too large.");
+assert(!VF.isScalable() && "VF is assumed to be non scalable.");
 
 if (!hasAnyScalarValue(Key))
   return false;
 const ScalarParts &Entry = ScalarMapStorage.find(Key)->second;
 assert(Entry.size

[PATCH] D86369: [Sema][MSVC] warn at dynamic_cast when /GR- is given

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang/test/SemaCXX/ms_dynamic_cast.cpp:16
+B *b = new D1();
+auto d = dynamic_cast(b); // expected-warning{{should not use 
dynamic_cast with /GR-}}
+}

zequanwu wrote:
> lebedev.ri wrote:
> > I'm not sure it makes sense to talk about MSVC/clang-cl flags when normal 
> > clang is used.
> > Either the diag should only be used in MSVC compat mode,
> > or it should talk about `-fno-rtti` in non-MSVC compat mode.
> I am not sure either... But `-fno-rtti-data` is only used when clang-cl 
> driver translate `/GR-` to `-fno-rtti-data`.
Instead of "should not use" maybe the warning could say something which 
suggests it won't work, like "dynamic_cast will not work since RTTI data is 
disabled" or something like that.

I'm not sure what to do about the name of the option. -fno-rtti-data was added 
to support clang-cl's /GR- option, but it could also be used with regular 
Clang. Maybe the warning doesn't need to mention the name of the flag? Or it 
could mention both? Or maybe it could figure out if it's in clang-cl or regular 
clang mode?

Also, should it also warn about typeid()?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86369

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

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Seems reasonable to me.

I'm still curious why it seems it's not looking for link.exe in the /fake dir 
though.


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

https://reviews.llvm.org/D85998

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

[PATCH] D75169: [ARM] Supporting lowering of half-precision FP arguments and returns in AArch32's backend

[Lucas Prates via Phabricator via cfe-commits]

pratlucas added a comment.

Hi @hans , I'll have a look at it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75169

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

[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker][NFC] Use Optionals throughout the summary API

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 287680.
martong added a comment.

- Use FileCheck in the lit test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86531

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-POSIX-lookup.c

Index: clang/test/Analysis/std-c-library-functions-POSIX-lookup.c
===
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-POSIX-lookup.c
@@ -0,0 +1,22 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -triple i686-unknown-linux 2>&1 | FileCheck %s --allow-empty
+
+// We test here the implementation of our summary API with Optional types. In
+// this TU we do not provide declaration for any of the functions that have
+// summaries. The implementation should be able to handle the nonexistent
+// declarations in a way that the summary is not added to the map. We expect no
+// crashes (i.e. no optionals should be 'dereferenced') and no output.
+
+// Must have at least one call expression to initialize the summary map.
+int bar(void);
+void foo() {
+  bar();
+}
+
+// CHECK-NOT: Loaded summary for:
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -314,7 +314,8 @@
   /// The complete list of constraints that defines a single branch.
   typedef std::vector ConstraintSet;
 
-  using ArgTypes = std::vector;
+  using ArgTypes = std::vector>;
+  using RetType = Optional;
 
   // A placeholder type, we use it whenever we do not care about the concrete
   // type in a Signature.
@@ -324,17 +325,37 @@
   // The signature of a function we want to describe with a summary. This is a
   // concessive signature, meaning there may be irrelevant types in the
   // signature which we do not check against a function with concrete types.
-  struct Signature {
-ArgTypes ArgTys;
+  // All types in the spec need to be canonical.
+  class Signature {
+using ArgQualTypes = std::vector;
+ArgQualTypes ArgTys;
 QualType RetTy;
-Signature(ArgTypes ArgTys, QualType RetTy) : ArgTys(ArgTys), RetTy(RetTy) {
-  assertRetTypeSuitableForSignature(RetTy);
-  for (size_t I = 0, E = ArgTys.size(); I != E; ++I) {
-QualType ArgTy = ArgTys[I];
-assertArgTypeSuitableForSignature(ArgTy);
+// True if any component type is not found by lookup.
+bool Invalid = false;
+
+  public:
+// Construct a signature from optional types. If any of the optional types
+// are not set then the signature will be invalid.
+Signature(ArgTypes ArgTys, RetType RetTy) {
+  for (Optional Arg : ArgTys) {
+if (!Arg) {
+  Invalid = true;
+  return;
+} else {
+  assertArgTypeSuitableForSignature(*Arg);
+  this->ArgTys.push_back(*Arg);
+}
+  }
+  if (!RetTy) {
+Invalid = true;
+return;
+  } else {
+assertRetTypeSuitableForSignature(*RetTy);
+this->RetTy = *RetTy;
   }
 }
 
+bool isInvalid() const { return Invalid; }
 bool matches(const FunctionDecl *FD) const;
 
   private:
@@ -388,6 +409,9 @@
   ///   rules for the given parameter's type, those rules are checked once the
   ///   signature is matched.
   class Summary {
+// FIXME Probably the Signature should not be part of the Summary,
+// We can remove once all overload of addToFunctionSummaryMap requires the
+// Signature explicitly given.
 Optional Sign;
 const InvalidationKind InvalidationKd;
 Cases CaseConstraints;
@@ -398,11 +422,13 @@
 const FunctionDecl *FD = nullptr;
 
   public:
-Summary(ArgTypes ArgTys, QualType RetTy, InvalidationKind InvalidationKd)
+Summary(ArgTypes ArgTys, RetType RetTy, InvalidationKind InvalidationKd)
 : Sign(Signature(ArgTys, RetTy)), InvalidationKd(InvalidationKd) {}
 
 Summary(InvalidationKind InvalidationKd) : InvalidationKd(InvalidationKd) {}
 
+// FIXME Remove, once all overload of addToFunctionSummaryMap requires the
+// Signature explicitly given.
 Summary &setSignature(const Signature &S) {
   Sign = S;
   return *this;
@@ -438,6 +464,13 @@
   return Result;
 }
 
+// FIXME Remove, once all overload of addToFunctionSummaryMap requires the
+// Signature explicitly given.
+bool hasInvalidSi

[PATCH] D86544: [SyntaxTree] Add support for call expressions

[Eduardo Caldas via Phabricator via cfe-commits]

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

Add support for all C++ call expressions, including those coming from 
`operator()`.
A call expression's arguments are inside their own node - `CallArguments` - 
based on the `List` base API.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86544

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -316,12 +316,12 @@
   `-CompoundStatement
 |-'{' OpenParen
 |-ExpressionStatement CompoundStatement_statement
-| |-UnknownExpression ExpressionStatement_expression
-| | |-IdExpression
+| |-CallExpression ExpressionStatement_expression
+| | |-IdExpression CallExpression_callee
 | | | `-UnqualifiedId IdExpression_id
 | | |   `-'test'
-| | |-'('
-| | `-')'
+| | |-'(' OpenParen
+| | `-')' CloseParen
 | `-';'
 |-IfStatement CompoundStatement_statement
 | |-'if' IntroducerKeyword
@@ -330,21 +330,21 @@
 | | `-'1' LiteralToken
 | |-')'
 | |-ExpressionStatement IfStatement_thenStatement
-| | |-UnknownExpression ExpressionStatement_expression
-| | | |-IdExpression
+| | |-CallExpression ExpressionStatement_expression
+| | | |-IdExpression CallExpression_callee
 | | | | `-UnqualifiedId IdExpression_id
 | | | |   `-'test'
-| | | |-'('
-| | | `-')'
+| | | |-'(' OpenParen
+| | | `-')' CloseParen
 | | `-';'
 | |-'else' IfStatement_elseKeyword
 | `-ExpressionStatement IfStatement_elseStatement
-|   |-UnknownExpression ExpressionStatement_expression
-|   | |-IdExpression
+|   |-CallExpression ExpressionStatement_expression
+|   | |-IdExpression CallExpression_callee
 |   | | `-UnqualifiedId IdExpression_id
 |   | |   `-'test'
-|   | |-'('
-|   | `-')'
+|   | |-'(' OpenParen
+|   | `-')' CloseParen
 |   `-';'
 `-'}' CloseParen
 )txt"));
@@ -378,20 +378,21 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-IdExpression
+CallExpression ExpressionStatement_expression
+|-IdExpression CallExpression_callee
 | `-UnqualifiedId IdExpression_id
 |   |-'operator'
 |   `-'+'
-|-'('
-|-IdExpression
-| `-UnqualifiedId IdExpression_id
-|   `-'x'
-|-','
-|-IdExpression
-| `-UnqualifiedId IdExpression_id
-|   `-'x'
-`-')'
+|-'(' OpenParen
+|-CallArguments CallExpression_arguments
+| |-IdExpression List_element
+| | `-UnqualifiedId IdExpression_id
+| |   `-'x'
+| |-',' List_delimiter
+| `-IdExpression List_element
+|   `-UnqualifiedId IdExpression_id
+| `-'x'
+`-')' CloseParen
 )txt"}));
 }
 
@@ -409,8 +410,8 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-MemberExpression
+CallExpression ExpressionStatement_expression
+|-MemberExpression CallExpression_callee
 | |-IdExpression MemberExpression_object
 | | `-UnqualifiedId IdExpression_id
 | |   `-'x'
@@ -419,8 +420,8 @@
 |   `-UnqualifiedId IdExpression_id
 | |-'operator'
 | `-'int'
-|-'('
-`-')'
+|-'(' OpenParen
+`-')' CloseParen
 )txt"}));
 }
 
@@ -436,16 +437,17 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-IdExpression
+CallExpression ExpressionStatement_expression
+|-IdExpression CallExpression_callee
 | `-UnqualifiedId IdExpression_id
 |   |-'operator'
 |   |-'""'
 |   `-'_w'
-|-'('
-|-CharacterLiteralExpression
-| `-''1'' LiteralToken
-`-')'
+|-'(' OpenParen
+|-CallArguments CallExpression_arguments
+| `-CharacterLiteralExpression List_element
+|   `-''1'' LiteralToken
+`-')' CloseParen
 )txt"}));
 }
 
@@ -461,8 +463,8 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-MemberExpression
+CallExpression ExpressionStatement_expression
+|-MemberExpression CallExpression_callee
 | |-IdExpression MemberExpression_object
 | | `-UnqualifiedId IdExpression_id
 | |   `-'x'
@@ -471,8 +473,8 @@
 |   `-UnqualifiedId IdExpression_id
 | |-'~'
 | `-'X'
-|-'('
-`-')'
+|-'(' OpenParen
+`-')' CloseParen
 )txt"}));
 }
 
@@ -492,8 +494,8 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-MemberExpression
+CallExpression ExpressionStatement_expression
+|-MemberExpression CallExpression_callee
 | |-IdExpression MemberExpression_object
 | | `-UnqualifiedId IdExpression_id
 | |   `-'x'
@@ -501,12 +503,12 @@
 | `-IdExpression MemberExpression_member
 |   `-UnqualifiedId IdExpression_id
 | `-'~'
-|-'decltype'
+|-'decltype' OpenParen
 |-'('
 |-'x'
 |-')'
 |-'('
-`-')'
+`-')' CloseParen
 )txt"}));
 }
 
@@ -523,15 +525,15 @@
 }
 )cpp",
   {R

[PATCH] D86135: [analyzer][MacroExpansion] Fix a crash where multiple parameters resolved to __VA_ARGS__

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

Szelethus marked an inline comment as done and an inline comment as not done.
Szelethus added a comment.

I'll get the nits I didn't object to fixed, thats the status you're looking for 
:)




Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:1338-1339
 void TokenPrinter::printToken(const Token &Tok) {
+  // TODO: Handle the case where hash and hashhash occurs right before
+  // __VA_ARGS__.
+

steakhal wrote:
> Szelethus wrote:
> > steakhal wrote:
> > > What does //hashhash// mean? I might lack some context though :D
> > `#` and `##` respectively. The test cases you pointed out as flawed refer 
> > to this FIXME, though a FIXME in the tests themselves wouldn't hurt.
> Maybe `HashtagHashtag`?
> Or an example would be even better like: `##__VA_ARGS__`
If you look a few lines down, you can see that its not up to us to choose this 
one :^) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86135

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

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

[Zachary Henkel via Phabricator via cfe-commits]

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

In D85998#2236401 , @hans wrote:

> I'm still curious why it seems it's not looking for link.exe in the /fake dir 
> though.

clang\lib\Driver\ToolChains\MSVC.cpp:311

FindVisualStudioExecutable 
FilePath = /fake\bin\Hostx64\x64\link.exe 
The test llvm::sys::fs::can_execute(FilePath) fails and the function returns 
"link.exe".  This isn't seen in our environment because //absent path// probes 
are always allowed, it's only //actual file reads// that need to be tracked.


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

https://reviews.llvm.org/D85998

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

[PATCH] D86169: Initial support for letting plugins perform custom parsing of attribute arguments.

[Jonathan Protzenko via Phabricator via cfe-commits]

jonathan.protzenko added a comment.

Thanks for the review!

Regarding Declarator: I briefly mentioned something related in the details 
section... actually maybe you can tell me what's the best approach here. I was 
thinking of just passing in a void* and expecting plugins to do

  if (const Declarator *D = isa_and_nonnull(Context))

This would allow a future-proof API where we can, in the future, pass in more 
classes for the "Context" argument (not just a Declarator), relying on clients 
doing dynamic casts. If I understood 
https://llvm.org/docs/ProgrammersManual.html#isa this `void*` cast would work?

My only hesitation is I'm not sure about the right technical device to use for 
this polymorphism pattern, as I've seen a couple related things in the clang 
code base:

- I've seen a pattern where there's a wrapper class that has a Kind() method 
and then allows downcasting based on the results of the Kind (seems overkill 
here)
- I've also seen references to `opaque_ptr` somewhere with a `get` method... 
(maybe these are helpers to do what I'm doing better?)

If you can confirm the void* is the right way to go (or point me to a suitable 
pattern I can take inspiration from), I'll submit a revised patch, along with 
the extra spelling argument (good point, thanks!).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86169

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

[PATCH] D86169: Initial support for letting plugins perform custom parsing of attribute arguments.

[Jonathan Protzenko via Phabricator via cfe-commits]

jonathan.protzenko added a comment.

I also seem to have broken a bunch of tests, although I'm not sure why. If you 
see an obvious cause, I'm happy to get some pointers, otherwise, I'll just dig 
in an debug as I extend the test suite once the patch stabilizes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86169

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

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

[Zachary Henkel via Phabricator via cfe-commits]

zahen added a comment.

The real reason we don't see it internally is because we use -c for all 
compilation.


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

https://reviews.llvm.org/D85998

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

[PATCH] D86546: [compiler-rt][builtins] Use explicitly-sized integer types for LibCalls

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko created this revision.
atrosinenko added reviewers: efriedma, MaskRay, aykevl, uabelho.
Herald added subscribers: Sanitizers, dberris.
Herald added a project: Sanitizers.
atrosinenko requested review of this revision.

Use s[iu]_int instead of `(unsigned) int` and d[ui]_int instead of
`(unsigned) long long` for LibCall arguments.

Note: the `*vfp` LibCall versions were NOT touched.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86546

Files:
  compiler-rt/lib/builtins/floatsisf.c
  compiler-rt/lib/builtins/floatsitf.c
  compiler-rt/lib/builtins/floatunsisf.c
  compiler-rt/lib/builtins/floatunsitf.c
  compiler-rt/test/builtins/Unit/floatditf_test.c
  compiler-rt/test/builtins/Unit/floatsitf_test.c
  compiler-rt/test/builtins/Unit/floatunditf_test.c
  compiler-rt/test/builtins/Unit/floatunsitf_test.c

Index: compiler-rt/test/builtins/Unit/floatunsitf_test.c
===
--- compiler-rt/test/builtins/Unit/floatunsitf_test.c
+++ compiler-rt/test/builtins/Unit/floatunsitf_test.c
@@ -8,9 +8,9 @@
 
 #include "fp_test.h"
 
-COMPILER_RT_ABI long double __floatunsitf(unsigned int a);
+COMPILER_RT_ABI long double __floatunsitf(su_int a);
 
-int test__floatunsitf(unsigned int a, uint64_t expectedHi, uint64_t expectedLo)
+int test__floatunsitf(su_int a, uint64_t expectedHi, uint64_t expectedLo)
 {
 long double x = __floatunsitf(a);
 int ret = compareResultLD(x, expectedHi, expectedLo);
Index: compiler-rt/test/builtins/Unit/floatunditf_test.c
===
--- compiler-rt/test/builtins/Unit/floatunditf_test.c
+++ compiler-rt/test/builtins/Unit/floatunditf_test.c
@@ -12,9 +12,9 @@
 
 // Returns: long integer converted to long double
 
-COMPILER_RT_ABI long double __floatunditf(unsigned long long a);
+COMPILER_RT_ABI long double __floatunditf(du_int a);
 
-int test__floatunditf(unsigned long long a, uint64_t expectedHi, uint64_t expectedLo)
+int test__floatunditf(du_int a, uint64_t expectedHi, uint64_t expectedLo)
 {
 long double x = __floatunditf(a);
 int ret = compareResultLD(x, expectedHi, expectedLo);
Index: compiler-rt/test/builtins/Unit/floatsitf_test.c
===
--- compiler-rt/test/builtins/Unit/floatsitf_test.c
+++ compiler-rt/test/builtins/Unit/floatsitf_test.c
@@ -8,9 +8,9 @@
 
 #include "fp_test.h"
 
-long COMPILER_RT_ABI double __floatsitf(int a);
+COMPILER_RT_ABI long double __floatsitf(si_int a);
 
-int test__floatsitf(int a, uint64_t expectedHi, uint64_t expectedLo)
+int test__floatsitf(si_int a, uint64_t expectedHi, uint64_t expectedLo)
 {
 long double x = __floatsitf(a);
 int ret = compareResultLD(x, expectedHi, expectedLo);
Index: compiler-rt/test/builtins/Unit/floatditf_test.c
===
--- compiler-rt/test/builtins/Unit/floatditf_test.c
+++ compiler-rt/test/builtins/Unit/floatditf_test.c
@@ -12,9 +12,9 @@
 
 // Returns: long integer converted to long double
 
-COMPILER_RT_ABI long double __floatditf(long long a);
+COMPILER_RT_ABI long double __floatditf(di_int a);
 
-int test__floatditf(long long a, uint64_t expectedHi, uint64_t expectedLo)
+int test__floatditf(di_int a, uint64_t expectedHi, uint64_t expectedLo)
 {
 long double x = __floatditf(a);
 int ret = compareResultLD(x, expectedHi, expectedLo);
Index: compiler-rt/lib/builtins/floatunsitf.c
===
--- compiler-rt/lib/builtins/floatunsitf.c
+++ compiler-rt/lib/builtins/floatunsitf.c
@@ -16,7 +16,7 @@
 #include "fp_lib.h"
 
 #if defined(CRT_HAS_128BIT) && defined(CRT_LDBL_128BIT)
-COMPILER_RT_ABI fp_t __floatunsitf(unsigned int a) {
+COMPILER_RT_ABI fp_t __floatunsitf(su_int a) {
 
   const int aWidth = sizeof a * CHAR_BIT;
 
Index: compiler-rt/lib/builtins/floatunsisf.c
===
--- compiler-rt/lib/builtins/floatunsisf.c
+++ compiler-rt/lib/builtins/floatunsisf.c
@@ -17,7 +17,7 @@
 
 #include "int_lib.h"
 
-COMPILER_RT_ABI fp_t __floatunsisf(unsigned int a) {
+COMPILER_RT_ABI fp_t __floatunsisf(su_int a) {
 
   const int aWidth = sizeof a * CHAR_BIT;
 
Index: compiler-rt/lib/builtins/floatsitf.c
===
--- compiler-rt/lib/builtins/floatsitf.c
+++ compiler-rt/lib/builtins/floatsitf.c
@@ -16,7 +16,7 @@
 #include "fp_lib.h"
 
 #if defined(CRT_HAS_128BIT) && defined(CRT_LDBL_128BIT)
-COMPILER_RT_ABI fp_t __floatsitf(int a) {
+COMPILER_RT_ABI fp_t __floatsitf(si_int a) {
 
   const int aWidth = sizeof a * CHAR_BIT;
 
@@ -26,10 +26,10 @@
 
   // All other cases begin by extracting the sign and absolute value of a
   rep_t sign = 0;
-  unsigned aAbs = (unsigned)a;
+  su_int aAbs = (su_int)a;
   if (a < 0) {
 sign = signBit;
-aAbs = ~(unsigned)a + 1U;
+aAbs = ~(su_int)a + (su_int)1

[PATCH] D86547: [compiler-rt][builtins] Use c[tl]zsi macro instead of __builtin_c[tl]z

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko created this revision.
atrosinenko added reviewers: efriedma, MaskRay, aykevl, uabelho.
Herald added subscribers: Sanitizers, dberris.
Herald added a project: Sanitizers.
atrosinenko requested review of this revision.

`__builtin_c[tl]z` accepts `unsigned int` argument that is not always the
same as uint32_t. For example, `unsigned int` is uint16_t on MSP430.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86547

Files:
  compiler-rt/lib/builtins/floatsisf.c
  compiler-rt/lib/builtins/floatsitf.c
  compiler-rt/lib/builtins/floatunsisf.c
  compiler-rt/lib/builtins/floatunsitf.c
  compiler-rt/lib/builtins/fp_extend.h
  compiler-rt/lib/builtins/udivmoddi4.c


Index: compiler-rt/lib/builtins/udivmoddi4.c
===
--- compiler-rt/lib/builtins/udivmoddi4.c
+++ compiler-rt/lib/builtins/udivmoddi4.c
@@ -82,7 +82,7 @@
 r.s.high = n.s.high & (d.s.high - 1);
 *rem = r.all;
   }
-  return n.s.high >> __builtin_ctz(d.s.high);
+  return n.s.high >> ctzsi(d.s.high);
 }
 // K K
 // ---
@@ -112,7 +112,7 @@
   *rem = n.s.low & (d.s.low - 1);
 if (d.s.low == 1)
   return n.all;
-sr = __builtin_ctz(d.s.low);
+sr = ctzsi(d.s.low);
 q.s.high = n.s.high >> sr;
 q.s.low = (n.s.high << (n_uword_bits - sr)) | (n.s.low >> sr);
 return q.all;
Index: compiler-rt/lib/builtins/fp_extend.h
===
--- compiler-rt/lib/builtins/fp_extend.h
+++ compiler-rt/lib/builtins/fp_extend.h
@@ -33,9 +33,9 @@
   return __builtin_clzl(a);
 #else
   if (a & REP_C(0x))
-return __builtin_clz(a >> 32);
+return clzsi(a >> 32);
   else
-return 32 + __builtin_clz(a & REP_C(0x));
+return 32 + clzsi(a & REP_C(0x));
 #endif
 }
 
Index: compiler-rt/lib/builtins/floatunsitf.c
===
--- compiler-rt/lib/builtins/floatunsitf.c
+++ compiler-rt/lib/builtins/floatunsitf.c
@@ -25,7 +25,7 @@
 return fromRep(0);
 
   // Exponent of (fp_t)a is the width of abs(a).
-  const int exponent = (aWidth - 1) - __builtin_clz(a);
+  const int exponent = (aWidth - 1) - clzsi(a);
   rep_t result;
 
   // Shift a into the significand field and clear the implicit bit.
Index: compiler-rt/lib/builtins/floatunsisf.c
===
--- compiler-rt/lib/builtins/floatunsisf.c
+++ compiler-rt/lib/builtins/floatunsisf.c
@@ -26,7 +26,7 @@
 return fromRep(0);
 
   // Exponent of (fp_t)a is the width of abs(a).
-  const int exponent = (aWidth - 1) - __builtin_clz(a);
+  const int exponent = (aWidth - 1) - clzsi(a);
   rep_t result;
 
   // Shift a into the significand field, rounding if it is a right-shift
Index: compiler-rt/lib/builtins/floatsitf.c
===
--- compiler-rt/lib/builtins/floatsitf.c
+++ compiler-rt/lib/builtins/floatsitf.c
@@ -33,7 +33,7 @@
   }
 
   // Exponent of (fp_t)a is the width of abs(a).
-  const int exponent = (aWidth - 1) - __builtin_clz(aAbs);
+  const int exponent = (aWidth - 1) - clzsi(aAbs);
   rep_t result;
 
   // Shift a into the significand field and clear the implicit bit.
Index: compiler-rt/lib/builtins/floatsisf.c
===
--- compiler-rt/lib/builtins/floatsisf.c
+++ compiler-rt/lib/builtins/floatsisf.c
@@ -33,7 +33,7 @@
   }
 
   // Exponent of (fp_t)a is the width of abs(a).
-  const int exponent = (aWidth - 1) - __builtin_clz(a);
+  const int exponent = (aWidth - 1) - clzsi(a);
   rep_t result;
 
   // Shift a into the significand field, rounding if it is a right-shift


Index: compiler-rt/lib/builtins/udivmoddi4.c
===
--- compiler-rt/lib/builtins/udivmoddi4.c
+++ compiler-rt/lib/builtins/udivmoddi4.c
@@ -82,7 +82,7 @@
 r.s.high = n.s.high & (d.s.high - 1);
 *rem = r.all;
   }
-  return n.s.high >> __builtin_ctz(d.s.high);
+  return n.s.high >> ctzsi(d.s.high);
 }
 // K K
 // ---
@@ -112,7 +112,7 @@
   *rem = n.s.low & (d.s.low - 1);
 if (d.s.low == 1)
   return n.all;
-sr = __builtin_ctz(d.s.low);
+sr = ctzsi(d.s.low);
 q.s.high = n.s.high >> sr;
 q.s.low = (n.s.high << (n_uword_bits - sr)) | (n.s.low >> sr);
 return q.all;
Index: compiler-rt/lib/builtins/fp_extend.h
===
--- compiler-rt/lib/builtins/fp_extend.h
+++ compiler-rt/lib/builtins/fp_extend.h
@@ -33,9 +33,9 @@
   return __builtin_clzl(a);
 #else
   if (a & REP_C(0x))
-return __builtin_clz(a >> 32);
+return clzsi(a >> 32);
   else
-return 32 + __builtin_clz(a & REP_C(0x));
+return 32 + clzsi(a & REP_

[PATCH] D86221: [compiler-rt][builtins] Do not assume int to be at least 32 bit wide

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko abandoned this revision.
atrosinenko added a comment.

Replaced this by D86546: [compiler-rt][builtins] Use explicitly-sized integer 
types for LibCalls  and D86547: 
[compiler-rt][builtins] Use c[tl]zsi macro instead of __builtin_c[tl]z 
. These two patches include everything except 
for factoring out `src_rep_t_clz()` and `rep_clz()` to `clzdi()` for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86221

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

[PATCH] D86376: [HIP] Improve kernel launching latency

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D86376#2234824 , @tra wrote:

> In D86376#2234719 , @yaxunl wrote:
>
>>> This patch appears to be somewhere in the gray area to me. My prior 
>>> experience with CUDA suggests that it will make little to no difference. On 
>>> the other hand, AMD GPUs may be different enough to prove me wrong. Without 
>>> specific evidence, I still can't tell what's the case here.
>>
>> Sorry, the overhead due to `__hipPushConfigure/__hipPopConfigure` is about 
>> 60 us. The typical kernel launching latency is about 500us, therefore the 
>> improvement is around 10%.
>
> 60 *micro seconds* to store/load something from memory? It does not sound 
> right. 0.5 millisecond per kernel launch is also suspiciously high. 
> For CUDA it's ~5us 
> (https://www.hpcs.cs.tsukuba.ac.jp/icpp2019/data/posters/Poster17-abst.pdf). 
> If it does indeed take 60 microseconds to push/pop a O(cacheline) worth of 
> launch config data, the implementation may be doing something wrong. We're 
> talking about O(100) syscalls and that's way too much work for something that 
> simple. What do those calls do?
>
> Can you confirm that the units are indeed microseconds and not nanoseconds?

My previous measurements did not warming up, which caused some one time 
overhead due to device initialization and loading of device binary. With warm 
up, the call of `__hipPushCallConfigure/__hipPopCallConfigure` takes about 19 
us. Based on the trace from rocprofile, the time spent inside these functions 
can be ignored. Most of the time is spent making the calls. These functions 
stay in a shared library, which may be the reason why they take such long time. 
Making them always_inline may get rid of the overhead, however, that would 
require exposing internal data structures.

The kernel launching latency are measured by a simple loop in which a simple 
kernel is launched then hipStreamSynchronize is called. trace is collected by 
rocprofiler and the latency is measured from the end of hipStreamSynchronize to 
the real start of kernel execution. Without this patch, the latency is about 77 
us. With this patch, the latency is about 46 us. The improvement is about 40%. 
The decrement of 31 us is more than 19 us since it also eliminates the overhead 
of kernel stub.

>> I would like to say the motivation of this change is two folds: 1. improve 
>> latency 2. interoperability with C++ programs.
>
> Could you elaborate on the "interoperability with C++ programs"? I don't 
> think I see how this patch helps with that. Or what exactly is the issue with 
> C++ interoperability we have now?

In HIP program, a global symbol is generated in host binary to identify each 
kernel. This symbol is associated with the device kernel by a call of 
hipRegisterFunction in init functions. Each time the kernel needs to be called, 
the associated symbol is passed to hipLaunchKernel. In host code, this symbol 
represents the kernel. Let's call it the kernel symbol. Currently it is the 
kernel stub function, however, it could be any global symbol, as long as it is 
registered with hipRegisterFunction, then hipLaunchKernel can use it to find 
the right kernel and launch it.

In a C/C++ program, a kernel is launched by call of hipLaunchKernel with the 
kernel symbol. Since the kernel symbol is defined in object files generated 
from HIP. For C/C++ program, as long as it declares the kernel symbol as an 
external function or variable which matches the name of the original symbol, 
the linker will resolve to the correct kernel symbol, then the correct kernel 
can be launched.

Here comes the nuance with kernel stub function as the kernel symbol. If you 
still remember, there was a previous patch for HIP to change the kernel stub 
name. rocgdb requires the device stub to have a different name than the real 
kernel, since otherwise it will not be able to break on the real kernel only. 
As a result, the kernel stub now has a prefix `__device_stub_` before mangling.

For example, a kernel `foo` will have a kernel stub with name 
`__device_stub_foo`.

For a C/C++ program to call kernel `foo`, it needs to declare an external 
symbol `__device_stub_foo` then launch it. Of course this is an annoyance for 
C/C++ users, especially this involves mangled names.

However, we cannot change the name of the kernel stub to be the same as the 
kernel, since that will break rocgdb.

Now the solution is to get rid of the kernel stub function. Instead of use 
kernel stub function as kernel symbol, we will emit a global variable as kernel 
symbol. This global variable can have the same name as the kernel, since rocgdb 
will not break on it.


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

https://reviews.llvm.org/D86376

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

[PATCH] D77925: Revert "[TLI] Per-function fveclib for math library used for vectorization"

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

In D77925#2229484 , @mehdi_amini wrote:

> Overall that would likely work for XLA. Something I'd like to mention though 
> in response to:
>
>> The veclib type is also tied to the accepted values for -fveclib, which is a 
>> list of supported lib,
>
> `-fveclib` is a Clang thing, it shouldn't limit what LLVM does. Of course 
> LLVM needs to support Clang, but does not have to inherit the limitation of 
> map 1:1 to Clang UI.
> In particular as a library, it isn't clear why we would make the choice to 
> write LLVM VecLib support this way.

Is there any benefit to keeping a closed list like this in LLVM? If not (and 
presumably clang is checking for valid values of -fveclib), then I think I 
agree with @mehdi_amini. Unless there is an efficiency reason for doing it via 
an enum. It's been awhile since I looked through this code in detail...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77925

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

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

NFC. Make clang-tidy happy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/ast-print-vector-size-bool.c
  clang/test/CodeGen/debug-info-vector-bool-hvx64.c
  clang/test/CodeGen/debug-info-vector-bool.c
  clang/test/CodeGen/vector-alignment.c
  clang/test/SemaCXX/constexpr-vectors.cpp
  clang/test/SemaCXX/vector-bool.cpp
  clang/test/SemaCXX/vector-conditional.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int &reference_to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/test/SemaCXX/vector-conditional.cpp
===
--- clang/test/SemaCXX/vector-conditional.cpp
+++ clang/test/SemaCXX/vector-conditional.cpp
@@ -13,6 +13,7 @@
 using FourFloats = float __attribute__((__vector_size__(16)));
 using TwoDoubles = double __attribute__((__vector_size__(16)));
 using FourDoubles = double __attribute__((__vector_size__(32)));
+using EightBools = bool __attribute__((__vector_size__(2)));
 
 FourShorts four_shorts;
 TwoInts two_ints;
@@ -25,6 +26,8 @@
 FourFloats four_floats;
 TwoDoubles two_doubles;
 FourDoubles four_doubles;
+EightBools eight_bools;
+EightBools other_eight_bools;
 
 enum E {};
 enum class SE {};
@@ -95,6 +98,9 @@
   (void)(four_ints ? four_uints : 3.0f);
   (void)(four_ints ? four_ints : 3.0f);
 
+  // Allow conditional select on bool vectors.
+  (void)(eight_bools ? eight_bools : other_eight_bools);
+
   // When there is a vector and a scalar, conversions must be legal.
   (void)(four_ints ? four_floats : 3); // should work, ints can convert to floats.
   (void)(four_ints ? four_uints : e);  // expected-error {{cannot convert between scalar type 'E' and vector type 'FourUInts'}}
@@ -163,10 +169,10 @@
 void Templates() {
   dependent_cond(two_ints);
   dependent_operand(two_floats);
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(4 * sizeof(double double' (vector of 4 'double' values))}}}
   all_dependent(four_ints, four_uints, four_doubles); // expected-note {{in instantiation of}}
 
-  // expected-error@159 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2 'unsigned int' values))}}}
+  // expected-error@165 {{vector operands to the vector conditional must be the same type ('__attribute__((__vector_size__(4 * sizeof(unsigned int unsigned int' (vector of 4 'unsigned int' values) and '__attribute__((__vector_size__(2 * sizeof(unsigned int unsigned int' (vector of 2

[PATCH] D86544: [SyntaxTree] Add support for call expressions

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas updated this revision to Diff 287694.
eduucaldas added a comment.

- Nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86544

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

Index: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -316,12 +316,12 @@
   `-CompoundStatement
 |-'{' OpenParen
 |-ExpressionStatement CompoundStatement_statement
-| |-UnknownExpression ExpressionStatement_expression
-| | |-IdExpression
+| |-CallExpression ExpressionStatement_expression
+| | |-IdExpression CallExpression_callee
 | | | `-UnqualifiedId IdExpression_id
 | | |   `-'test'
-| | |-'('
-| | `-')'
+| | |-'(' OpenParen
+| | `-')' CloseParen
 | `-';'
 |-IfStatement CompoundStatement_statement
 | |-'if' IntroducerKeyword
@@ -330,21 +330,21 @@
 | | `-'1' LiteralToken
 | |-')'
 | |-ExpressionStatement IfStatement_thenStatement
-| | |-UnknownExpression ExpressionStatement_expression
-| | | |-IdExpression
+| | |-CallExpression ExpressionStatement_expression
+| | | |-IdExpression CallExpression_callee
 | | | | `-UnqualifiedId IdExpression_id
 | | | |   `-'test'
-| | | |-'('
-| | | `-')'
+| | | |-'(' OpenParen
+| | | `-')' CloseParen
 | | `-';'
 | |-'else' IfStatement_elseKeyword
 | `-ExpressionStatement IfStatement_elseStatement
-|   |-UnknownExpression ExpressionStatement_expression
-|   | |-IdExpression
+|   |-CallExpression ExpressionStatement_expression
+|   | |-IdExpression CallExpression_callee
 |   | | `-UnqualifiedId IdExpression_id
 |   | |   `-'test'
-|   | |-'('
-|   | `-')'
+|   | |-'(' OpenParen
+|   | `-')' CloseParen
 |   `-';'
 `-'}' CloseParen
 )txt"));
@@ -378,20 +378,21 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-IdExpression
+CallExpression ExpressionStatement_expression
+|-IdExpression CallExpression_callee
 | `-UnqualifiedId IdExpression_id
 |   |-'operator'
 |   `-'+'
-|-'('
-|-IdExpression
-| `-UnqualifiedId IdExpression_id
-|   `-'x'
-|-','
-|-IdExpression
-| `-UnqualifiedId IdExpression_id
-|   `-'x'
-`-')'
+|-'(' OpenParen
+|-CallArguments CallExpression_arguments
+| |-IdExpression List_element
+| | `-UnqualifiedId IdExpression_id
+| |   `-'x'
+| |-',' List_delimiter
+| `-IdExpression List_element
+|   `-UnqualifiedId IdExpression_id
+| `-'x'
+`-')' CloseParen
 )txt"}));
 }
 
@@ -409,8 +410,8 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-MemberExpression
+CallExpression ExpressionStatement_expression
+|-MemberExpression CallExpression_callee
 | |-IdExpression MemberExpression_object
 | | `-UnqualifiedId IdExpression_id
 | |   `-'x'
@@ -419,8 +420,8 @@
 |   `-UnqualifiedId IdExpression_id
 | |-'operator'
 | `-'int'
-|-'('
-`-')'
+|-'(' OpenParen
+`-')' CloseParen
 )txt"}));
 }
 
@@ -436,16 +437,17 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-IdExpression
+CallExpression ExpressionStatement_expression
+|-IdExpression CallExpression_callee
 | `-UnqualifiedId IdExpression_id
 |   |-'operator'
 |   |-'""'
 |   `-'_w'
-|-'('
-|-CharacterLiteralExpression
-| `-''1'' LiteralToken
-`-')'
+|-'(' OpenParen
+|-CallArguments CallExpression_arguments
+| `-CharacterLiteralExpression List_element
+|   `-''1'' LiteralToken
+`-')' CloseParen
 )txt"}));
 }
 
@@ -461,8 +463,8 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-MemberExpression
+CallExpression ExpressionStatement_expression
+|-MemberExpression CallExpression_callee
 | |-IdExpression MemberExpression_object
 | | `-UnqualifiedId IdExpression_id
 | |   `-'x'
@@ -471,8 +473,8 @@
 |   `-UnqualifiedId IdExpression_id
 | |-'~'
 | `-'X'
-|-'('
-`-')'
+|-'(' OpenParen
+`-')' CloseParen
 )txt"}));
 }
 
@@ -492,8 +494,8 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-MemberExpression
+CallExpression ExpressionStatement_expression
+|-MemberExpression CallExpression_callee
 | |-IdExpression MemberExpression_object
 | | `-UnqualifiedId IdExpression_id
 | |   `-'x'
@@ -501,12 +503,12 @@
 | `-IdExpression MemberExpression_member
 |   `-UnqualifiedId IdExpression_id
 | `-'~'
-|-'decltype'
+|-'decltype' OpenParen
 |-'('
 |-'x'
 |-')'
 |-'('
-`-')'
+`-')' CloseParen
 )txt"}));
 }
 
@@ -523,15 +525,15 @@
 }
 )cpp",
   {R"txt(
-UnknownExpression ExpressionStatement_expression
-|-IdExpression
+CallExpression ExpressionStatement_expression
+|-IdExpression CallExpression_callee
 | `-UnqualifiedId IdExpression_id
 

[PATCH] D86544: [SyntaxTree] Add support for call expressions

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas added a reviewer: gribozavr2.
eduucaldas added inline comments.



Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:506
 | `-'~'
-|-'decltype'
+|-'decltype' OpenParen
 |-'('

This is wrong but it's just because decltype is all wrong



Comment at: clang/unittests/Tooling/Syntax/BuildTreeTest.cpp:2734
+|   |   `-'args'
+|   `-'...'
+`-')' CloseParen

Here there is a divergence between the grammar and the ClangAST. 
According to the [[ https://eel.is/c++draft/expr.post#nt:expression-list | 
grammar ]]  `...` would an optional element of `CallArguments`, but here `...` 
is part of an expression.

Perhaps I have used the wrong kind of `...`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86544

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

[PATCH] D86376: [HIP] Improve kernel launching latency

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D86376#2236501 , @yaxunl wrote:

> My previous measurements did not warming up, which caused some one time 
> overhead due to device initialization and loading of device binary. With warm 
> up, the call of `__hipPushCallConfigure/__hipPopCallConfigure` takes about 19 
> us. Based on the trace from rocprofile, the time spent inside these functions 
> can be ignored. Most of the time is spent making the calls. These functions 
> stay in a shared library, which may be the reason why they take such long 
> time. Making them always_inline may get rid of the overhead, however, that 
> would require exposing internal data structures.

It's still suspiciously high. AFAICT, config/push/pull is just an std::vector 
push/pop. It should not take *that* long.  Few function calls should not lead 
to microseconds of overhead, once linker has resolved the symbol, if they come 
from a shared library.
https://github.com/ROCm-Developer-Tools/HIP/blob/master/vdi/hip_platform.cpp#L590

I wonder if it's the logging facilities that add all this overhead.

> The kernel launching latency are measured by a simple loop in which a simple 
> kernel is launched then hipStreamSynchronize is called. trace is collected by 
> rocprofiler and the latency is measured from the end of hipStreamSynchronize 
> to the real start of kernel execution. Without this patch, the latency is 
> about 77 us. With this patch, the latency is about 46 us. The improvement is 
> about 40%. The decrement of 31 us is more than 19 us since it also eliminates 
> the overhead of kernel stub.

This is rather surprising. A function call by itself does *not* have such high 
overhead. There must be something else. I strongly suspect logging. If you 
remove logging statements from push/pop without changing anything else, how 
does that affect performance?

>>> I would like to say the motivation of this change is two folds: 1. improve 
>>> latency 2. interoperability with C++ programs.
>>
>> Could you elaborate on the "interoperability with C++ programs"? I don't 
>> think I see how this patch helps with that. Or what exactly is the issue 
>> with C++ interoperability we have now?
>
> In HIP program, a global symbol is generated in host binary to identify each 
> kernel. This symbol is associated with the device kernel by a call of 
> hipRegisterFunction in init functions. Each time the kernel needs to be 
> called, the associated symbol is passed to hipLaunchKernel. In host code, 
> this symbol represents the kernel. Let's call it the kernel symbol. Currently 
> it is the kernel stub function, however, it could be any global symbol, as 
> long as it is registered with hipRegisterFunction, then hipLaunchKernel can 
> use it to find the right kernel and launch it.

So far so good, it matches the way CUDA does that.

> In a C/C++ program, a kernel is launched by call of hipLaunchKernel with the 
> kernel symbol.

Do you mean the host-side symbol, registered with the runtime that you've 
described above? Or do you mean that the device-side symbol is somehow visible 
from the host side. I think that's where HIP is different from CUDA.

> Since the kernel symbol is defined in object files generated from HIP. 
> For C/C++ program, as long as it declares the kernel symbol as an external 
> function or variable which matches the name of the original symbol, the 
> linker will resolve to the correct kernel symbol, then the correct kernel can 
> be launched.

The first sentence looks incomplete. It seems to imply that hipLaunchKernel 
uses the device-side kernel symbol and it's the linker which ties host-side 
reference with device-side symbol. If that's the case, then I don't understand 
what purpose is served by hipRegisterFunction. AFAICT, it's not used in this 
scenario at all.

My mental model of kernel launch mechanics looks like this:

- For a kernel foo, there is a host-side symbol (it's the stub for CUDA) with 
the name 'foo' and device-side real kernel 'foo'.
- host side linker has no access to device-side symbols, but we do need to 
associate host and device side 'foo' instances.
- address of host-side foo is registered with runtime to map it to device 
symbol with the name 'foo'
- when a kernel is launched, call site sets up launch config and calls the 
stub, passing it the kernel arguments.
- the stub calls the kernel launch function, and passes host-side foo address 
to the kernel launch function
- launch function finds device-side symbol name via the registration info and 
does device-side address lookup to obtain it's device address
- run device-side function.

In this scenario, the host-side stub for foo is a regular function, which gdb 
can stop on and examine kernel arguments.

How is the process different for HIP? I know that we've changed the stub name 
to avoid debugger confusion about which if the entities corresponds to 'foo'.

> Here comes the nuance with kernel stub function as the kernel 

[PATCH] D85810: [clang] Pass-through remarks options to linker

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

Looks pretty good but I think it can be simplified in one place as I note below.




Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:596
+  // Handle remark diagnostics on screen options: '-Rpass-*'.
+  if (usesRpassOptions(Args))
+renderRpassOptions(Args, CmdArgs);

Is this guard needed? It looks like renderRpassOptions will do the right thing 
if none are specified (won't add anything). Right now you end up checking each 
one a second time if any are enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85810

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

[PATCH] D86558: [OPENMP]Add support for allocate vars in untied tasks.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev created this revision.
ABataev added a reviewer: jdoerfert.
Herald added subscribers: guansong, yaxunl.
Herald added a project: clang.
ABataev requested review of this revision.
Herald added a subscriber: sstefan1.

Local vars, marked with pragma allocate, mustbe allocate by the call of
the runtime function and cannot be allocated as other local variables.
Instead, we allocate a space for the pointer in private record and store
the address, returned by kmpc_alloc call in this pointer.
So, for untied tasks

  #pragma omp task untied
  {
S s;
 #pragma omp allocate(s) allocator(allocator)
s = x;
  }

compiler generates something like this:

  struct task_with_privates {
S *ptr;
  };
  
  void entry(task_with_privates *p) {
S *s = p->s;
switch(partid) {
case 1:
  p->s = (S*)kmpc_alloc();
  kmpc_omp_task();
  br exit;
case 2:
  *s = x;
  kmpc_omp_task();
  br exit;
case 2:
  ~S(s);
  kmpc_free((void*)s);
  br exit;
}
  exit:
  }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86558

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/allocate_codegen.cpp
  clang/test/OpenMP/for_lastprivate_codegen.cpp
  clang/test/OpenMP/for_linear_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen_UDR.cpp
  clang/test/OpenMP/parallel_firstprivate_codegen.cpp
  clang/test/OpenMP/parallel_private_codegen.cpp
  clang/test/OpenMP/task_codegen.cpp

Index: clang/test/OpenMP/task_codegen.cpp
===
--- clang/test/OpenMP/task_codegen.cpp
+++ clang/test/OpenMP/task_codegen.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix UNTIEDRT
-// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
-// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - -DUNTIEDRT | FileCheck %s --check-prefix CHECK --check-prefix UNTIEDRT
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s -DUNTIEDRT
+// RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s --check-prefix CHECK --check-prefix UNTIEDRT
 //
 // RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -fopenmp-enable-irbuilder -x c++ -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fopenmp -fopenmp-enable-irbuilder -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
@@ -14,6 +14,19 @@
 #ifndef HEADER
 #define HEADER
 
+enum omp_allocator_handle_t {
+  omp_null_allocator = 0,
+  omp_default_mem_alloc = 1,
+  omp_large_cap_mem_alloc = 2,
+  omp_const_mem_alloc = 3,
+  omp_high_bw_mem_alloc = 4,
+  omp_low_lat_mem_alloc = 5,
+  omp_cgroup_mem_alloc = 6,
+  omp_pteam_mem_alloc = 7,
+  omp_thread_mem_alloc = 8,
+  KMP_ALLOCATOR_MAX_HANDLE = __UINTPTR_MAX__
+};
+
 // CHECK-DAG: [[IDENT_T:%.+]] = type { i32, i32, i32, i32, i8* }
 // CHECK-DAG: [[STRUCT_SHAREDS:%.+]] = type { i8*, [2 x [[STRUCT_S:%.+]]]* }
 // CHECK-DAG: [[STRUCT_SHAREDS1:%.+]] = type { [2 x [[STRUCT_S:%.+]]]* }
@@ -258,21 +271,26 @@
 a = 4;
 c = 5;
   }
-// CHECK: [[ORIG_TASK_PTR:%.+]] = call i8* @__kmpc_omp_task_alloc([[IDENT_T]]* @{{.+}}, i32 {{%.*}}, i32 0, i64 48, i64 1, i32 (i32, i8*)* bitcast (i32 (i32, [[KMP_TASK_T]]{{.*}}*)* [[TASK_ENTRY6:@.+]] to i32 (i32, i8*)*))
+// CHECK: [[ORIG_TASK_PTR:%.+]] = call i8* @__kmpc_omp_task_alloc([[IDENT_T]]* @{{.+}}, i32 {{%.*}}, i32 0, i64 256, i64 1, i32 (i32, i8*)* bitcast (i32 (i32, [[KMP_TASK_T]]{{.*}}*)* [[TASK_ENTRY6:@.+]] to i32 (i32, i8*)*))
 // CHECK: call i32 @__kmpc_omp_task([[IDENT_T]]* @{{.+}}, i32 {{%.*}}, i8* [[ORIG_TASK_PTR]])
-#pragma omp task untied
+#pragma omp task untied firstprivate(c) allocate(omp_pteam_mem_alloc:c)
   {
-S s1;
+S s1, s2;
+#ifdef UNTIEDRT
+#pragma omp allocate(s2) allocator(omp_pteam_mem_alloc)
+#endif
+s2.a = 0;
 #pragma omp task
-a = 4;
+a = c = 4;
 #pragma omp taskyield
 s1 = S();
+s2.a = 10;
 #pragma omp taskwait
   }
   return a;
 }
 // CHECK: define internal i32 [[TASK_ENTRY1]](i32 %0, [[KMP_TASK_T]]{{.*}}* noalias %1)
-// CHECK: store i32 15, i32* [[A_PTR:@.+]]
+// CHECK: store i32 15, i32* [[A_PTR:@.+]],
 // CHECK: [[A_VAL:%.+]] = load i32, i32* [[A_PTR]]
 // CHECK: [[A_VAL_I8:%.+]] = trunc i32 [[A_VAL]] to i8
 // CHECK: store i8 [[A_VAL_I8]], i8* %{{.+}}
@@ -294,10 +312,13 @@
 // CHECK: define internal i32
 // CHECK: store i32 4, i32* [[A_PTR]]
 
-// CHECK: define internal i32 [[TASK_ENTRY6]](i32 %0, [[KMP_TASK_T]]{{.*}}* noalias %1)
+// CHECK: define internal i32 [[TASK_ENTRY6]](i32 %0, [[KMP_TASK_T]]{{.*}}* noalias %{{.+}})
 // UNTIEDRT: [[S1_ADDR_

[PATCH] D84414: [RISCV] Support Shadow Call Stack

[Z. Zheng via Phabricator via cfe-commits]

zzheng added a comment.

In D84414#2234267 , @lenary wrote:

> Ok, so any compilation units without `-fsanitize=shadow-call-stack` should be 
> compiled with `-ffixed-x18` if you want to link those together. That is 
> reasonable. My question was whether we can ensure that 
> `-fsanitize=shadow-call-stack` can imply `-ffixed-x18` rather than having to 
> pass both options.
>
> It is my understanding that all functions in a CU with 
> `-fsanitize=shadow-call-stack` will get the SCS function attribute, so why 
> can't we use that attribute to work out whether x18 should be reserved, 
> rather than using `-ffixed-x18`? You'll see 
> `RISCVRegisterInfo::getReservedRegs` reserves `fp` and `bp` only if they're 
> needed by the function (which can be based on attributes or other info), 
> rather than using `isRegisterReservedByUser` - which seems cleaner to me.



In D84414#2234327 , @pcc wrote:

> FWIW, on aarch64 we decided to make `-fsanitize=shadow-call-stack` require 
> the x18 reservation (instead of implying it) to try to avoid ABI mismatch 
> problems. That is, it should be safe to mix and match code compiled with and 
> without `-fsanitize=shadow-call-stack`. If we make 
> `-fsanitize=shadow-call-stack` imply the x18 reservation, it makes it more 
> likely that someone will accidentally build and link in incompatible code 
> that does not reserve x18.

Yes, binding '-fsanitize=shadow-call-stack' with '-ffixed-x18' is to ensure 
user don't accidentally link SCS-enabled code with non-SCS code that 
uses/overwrites x18. We can always infer that x18 is reserved when SCS in on.

There's no need to check `RISCVRegisterInfo::getReservedRegs` for x18 by 
looking into function attr. It forms a circular chain: is x18 reserved for SCS? 
<--> SCS is enabled, so x18 should be reserved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414

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

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

[Zachary Henkel via Phabricator via cfe-commits]

zahen added a comment.

Thanks @hans!  I'll need you (or someone else with permission) to land the 
change on my behalf.


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

https://reviews.llvm.org/D85998

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

[PATCH] D86491: [DebugInfo] Move constructor homing case in shouldOmitDefinition.

[Amy Huang via Phabricator via cfe-commits]

akhuang added a comment.

In D86491#2235206 , @MaskRay wrote:

> Note that Harbormaster actually reported the check-clang-codegencxx issues 
> B69369 ..

Definitely my bad; I thought I had run check-clang locally before committing, 
but maybe .. I didn't?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86491

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

[PATCH] D85998: Add clang-cl "vctoolsdir" option to specify the location of the msvc toolchain

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

I'll land tomorrow unless Alexandre beats me to it.


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

https://reviews.llvm.org/D85998

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

[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: rsmith, rjmccall, aaron.ballman.
Mordante added a project: clang.
Herald added a subscriber: cfe-commits.
Mordante requested review of this revision.

Allows the likelihood attributes on label, which are not part of a case 
statement. When a `goto` is used it accepts the attribute placed on the target 
label.

Depends on D85091 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86559

Files:
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
  clang/test/Sema/attr-likelihood.c
  clang/test/SemaCXX/attr-likelihood.cpp

Index: clang/test/SemaCXX/attr-likelihood.cpp
===
--- clang/test/SemaCXX/attr-likelihood.cpp
+++ clang/test/SemaCXX/attr-likelihood.cpp
@@ -115,8 +115,7 @@
   if (x)
 goto lbl;
 
-  // FIXME: allow the attribute on the label
-  [[unlikely]] lbl : // expected-error {{'unlikely' attribute cannot be applied to a declaration}}
+  [[likely]] lbl :
  [[likely]] x = x + 1;
 
   [[likely]]++ x;
Index: clang/test/Sema/attr-likelihood.c
===
--- clang/test/Sema/attr-likelihood.c
+++ clang/test/Sema/attr-likelihood.c
@@ -43,8 +43,7 @@
   if (x)
 goto lbl;
 
-  // FIXME: allow the attribute on the label
-  [[clang::unlikely]] lbl : // expected-error {{'unlikely' attribute cannot be applied to a declaration}}
+  [[clang::unlikely]] lbl :
   [[clang::likely]] x = x + 1;
 
   [[clang::likely]]++ x;
Index: clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
===
--- clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
+++ clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
@@ -125,7 +125,45 @@
 // Make sure the branches aren't optimized away.
 b = true;
   }
+
+  // CHECK: br {{.*}} !prof !8
+  if (b) {
+ goto uend;
+  } else {
+// Make sure the branches aren't optimized away.
+b = true;
+  }
+
+  // CHECK: br {{.*}} !prof !7
+  if (b) {
+ goto lend;
+  } else {
+// Make sure the branches aren't optimized away.
+b = true;
+  }
+
+  // CHECK: br {{.*}} !prof !8
+  if (b) {
+ goto ugcc;
+  } else {
+// Make sure the branches aren't optimized away.
+b = true;
+  }
+
+  // CHECK: br {{.*}} !prof !7
+  if (b) {
+ goto lgcc;
+  } else {
+// Make sure the branches aren't optimized away.
+b = true;
+  }
 end:;
+[[unlikely]] uend:;
+[[likely]] lend:;
+// GCC attributes are handled in a special way, make sure the likelihood
+// attributes aren't ignored.
+[[unlikely]] ugcc: __attribute__((unused));;
+[[likely]] lgcc: __attribute__((unused));;
 }
 
 void ReturnStmt() {
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6410,6 +6410,29 @@
   D->addAttr(::new (S.Context) DeprecatedAttr(S.Context, AL, Str, Replacement));
 }
 
+static bool validateLikelihoodAttr(Sema &S, Decl *D, const ParsedAttr &A) {
+  if (!isa(D)) {
+S.Diag(A.getRange().getBegin(), diag::err_stmt_attribute_invalid_on_decl)
+<< A << D->getBeginLoc();
+return false;
+  }
+
+  if (!S.getLangOpts().CPlusPlus20 && A.isCXX11Attribute() && !A.getScopeName())
+S.Diag(A.getLoc(), diag::ext_cxx20_attr) << A << A.getRange();
+
+  return true;
+}
+
+static void handleLikelyAttr(Sema &S, Decl *D, const ParsedAttr &A) {
+  if (validateLikelihoodAttr(S, D, A))
+D->addAttr(::new (S.Context) LikelyAttr(S.Context, A));
+}
+
+static void handleUnlikelyAttr(Sema &S, Decl *D, const ParsedAttr &A) {
+  if (validateLikelihoodAttr(S, D, A))
+D->addAttr(::new (S.Context) UnlikelyAttr(S.Context, A));
+}
+
 static bool isGlobalVar(const Decl *D) {
   if (const auto *S = dyn_cast(D))
 return S->hasGlobalStorage();
@@ -6943,6 +6966,12 @@
   case ParsedAttr::AT_Deprecated:
 handleDeprecatedAttr(S, D, AL);
 break;
+  case ParsedAttr::AT_Likely:
+handleLikelyAttr(S, D, AL);
+break;
+  case ParsedAttr::AT_Unlikely:
+handleUnlikelyAttr(S, D, AL);
+break;
   case ParsedAttr::AT_Destructor:
 if (S.Context.getTargetInfo().getTriple().isOSAIX())
   llvm::report_fatal_error("'destructor' attribute is not yet supported on AIX");
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -672,17 +672,8 @@
   LikelihoodAttributeFinder(const Stmt *S) { Visit(S); }
 
   void VisitAttributedStmt(const AttributedStmt *S) {
-for (const auto *A : S->getAttrs()) {
-  if (isa(A)) {
-Result = Likely;
-return;
-  }
-  if (isa(A)) {
-Result = Unlikely;
-return;
-  }
-}
-  

[PATCH] D85808: [Remarks][2/2] Expand remarks hotness threshold option support in more tools

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: llvm/lib/Analysis/OptimizationRemarkEmitter.cpp:103
 BFI = &getAnalysis().getBFI();
-  else
+// Get hotness threshold from PSI. This should only happen once.
+if (Context.isDiagnosticsHotnessThresholdSetFromPSI()) {

Can you clarify what you mean by this only happening once?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85808

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

[PATCH] D82582: [SVE] Remove calls to VectorType::getNumElements from clang

[Christopher Tetreault via Phabricator via cfe-commits]

ctetreau marked 8 inline comments as done.
ctetreau added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:1765
   }
   auto *VectorTy = dyn_cast(
   cast(Addr.getPointer()->getType())->getElementType());

c-rhodes wrote:
> cast here
that would be a behavior change because of the dyn_cast.



Comment at: clang/lib/CodeGen/CGExpr.cpp:1799
   llvm::Type *SrcTy = Value->getType();
   auto *VecTy = dyn_cast(SrcTy);
   // Handle vec3 special.

c-rhodes wrote:
> cast here
that would be a behavior change because of the dyn_cast



Comment at: clang/lib/CodeGen/CGExpr.cpp:2214
   if (const VectorType *VTy = Dst.getType()->getAs()) {
 unsigned NumSrcElts = VTy->getNumElements();
 unsigned NumDstElts =

c-rhodes wrote:
> missed one here
this VTy is a clang::VectorType



Comment at: clang/lib/CodeGen/SwiftCallingConv.cpp:321
   // If we have a vector type, split it.
   if (auto vecTy = dyn_cast_or_null(type)) {
 auto eltTy = vecTy->getElementType();

c-rhodes wrote:
> cast here
this would be a behavior change due to dyn_cast


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82582

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

[PATCH] D86562: [OPENMP][NFC]Release notes for OpenMP in clang (11.x).

[Alexey Bataev via Phabricator via cfe-commits]

ABataev created this revision.
ABataev added reviewers: jdoerfert, kkwli0, RaviNarayanaswamy, hfinkel.
Herald added subscribers: jfb, guansong, yaxunl.
Herald added a project: clang.
ABataev requested review of this revision.
Herald added a subscriber: sstefan1.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86562

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -156,7 +156,49 @@
 OpenMP Support in Clang
 ---
 
-- ...
+New features for OpenMP 5.0 were implemented.
+
+- OpenMP 5.0 is the default version supported by the compiler. User can switch
+  to OpenMP 4.5 using ``-fopenmp-version=45`` option.
+
+- Added support for declare variant directive.
+
+- Improved support of math functions for NVPTX target.
+
+- Added support for parallel execution of target regions for NVPTX target.
+
+- Addes support for ``scan`` directives and ``inscan`` modifier in 
``reduction``
+  clauses.
+
+- Added support for ``iterator`` construct.
+
+- Added support for ``depobj`` construct.
+
+- Added support for ``detach`` clauses in task-based directives.
+
+- Added support for array shaping operations.
+
+- Added support for cancellation constructs in ``taskloop`` directives.
+
+- Nonmonotonic modifier is allowed with all schedule kinds.
+
+- Added support for ``task`` and ``default`` modifiers in ``reduction`` 
clauses.
+
+- Added support for strides in array sections.
+
+- Added support for ``use_device_addr`` clause.
+
+- Added support for ``uses_allocators`` clause.
+
+- Added support for ``defaultmap`` clause.
+
+- Added basic support for ``hint`` clause in ``atomic`` directives.
+
+- Added basic support for ``affinity`` clause.
+
+- Added basic support for ``ancestor`` modifier in ``device`` clause.
+
+- Bug fixes and optimizations.
 
 CUDA Support in Clang
 -


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -156,7 +156,49 @@
 OpenMP Support in Clang
 ---
 
-- ...
+New features for OpenMP 5.0 were implemented.
+
+- OpenMP 5.0 is the default version supported by the compiler. User can switch
+  to OpenMP 4.5 using ``-fopenmp-version=45`` option.
+
+- Added support for declare variant directive.
+
+- Improved support of math functions for NVPTX target.
+
+- Added support for parallel execution of target regions for NVPTX target.
+
+- Addes support for ``scan`` directives and ``inscan`` modifier in ``reduction``
+  clauses.
+
+- Added support for ``iterator`` construct.
+
+- Added support for ``depobj`` construct.
+
+- Added support for ``detach`` clauses in task-based directives.
+
+- Added support for array shaping operations.
+
+- Added support for cancellation constructs in ``taskloop`` directives.
+
+- Nonmonotonic modifier is allowed with all schedule kinds.
+
+- Added support for ``task`` and ``default`` modifiers in ``reduction`` clauses.
+
+- Added support for strides in array sections.
+
+- Added support for ``use_device_addr`` clause.
+
+- Added support for ``uses_allocators`` clause.
+
+- Added support for ``defaultmap`` clause.
+
+- Added basic support for ``hint`` clause in ``atomic`` directives.
+
+- Added basic support for ``affinity`` clause.
+
+- Added basic support for ``ancestor`` modifier in ``device`` clause.
+
+- Bug fixes and optimizations.
 
 CUDA Support in Clang
 -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75655: [Docs] Document -lto-whole-program-visibility

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

@pcc ping. I'd like to get something in for LLVM 11 if at all possible. How 
about for now I make the last sentence something like "This is useful in 
situations where it is not safe to specify``-fvisibility=hidden`` at compile 
time, for example when bitcode objects will be consumed by different LTO links 
that don't all have whole program visibility." ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75655

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

[PATCH] D83845: [LTO/WPD] Remove special type test handling for -flto-visibility-public-std

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83845

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

[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

This feels like the wrong approach to me... but I admit that I don't know what 
the "right" approach might be. (I doubt any right approach exists.)

  if (ch == ' ') [[likely]] {
  goto whitespace;  // A
  } else if (ch == '\n' || ch == '\t') [[unlikely]] {
  goto whitespace;  // B
  } else {
  foo();
  }
  [[likely]] whitespace: bar();  // C

It seems like this patch would basically "copy" the `[[likely]]` attribute from 
line C up to lines A and B, where it would reinforce the likelihood of path A 
and (maybe?) "cancel out" the unlikelihood of path B, without actually saying 
anything specifically about the likelihood of label C (which is surely what the 
programmer intended by applying the attribute, right?). OTOH, I can't think of 
any particular optimization that would care about the likelihood of label C. I 
could imagine trying to align label C to a 4-byte boundary or something, but 
that wouldn't be an //optimization// on any platform as far as I know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86559

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

[clang] 01eb123 - [X86] Mention -march=sapphirerapids in the release notes.

[Craig Topper via cfe-commits]

Author: Craig Topper
Date: 2020-08-25T11:57:34-07:00
New Revision: 01eb1233db54454b146cb1e70d6f810ffbc354e5

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

LOG: [X86] Mention -march=sapphirerapids in the release notes.

This was just added in e02d081f2b60b61eb60ef6a49b1a9f907e432d4c.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
llvm/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c036f66d60bf7..e810b92c49271 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -188,6 +188,8 @@ X86 Support in Clang
 - The x86 intrinsics ``__rorb``, ``__rorw``, ``__rord``, ``__rorq`, ``_rotr``,
   ``_rotwr`` and ``_lrotr`` may now be used within constant expressions.
 
+- Support for -march=sapphirerapids was added.
+
 Internal API Changes
 
 

diff  --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index 2a2775f13dd1f..c6b2a3ac9bb57 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -97,6 +97,7 @@ During this release ...
 * The 'mpx' feature was removed from the backend. It had been removed from 
clang
   frontend in 10.0. Mention of the 'mpx' feature in an IR file will print a
   message to stderr, but IR should still compile.
+* Support for -march=sapphirerapids was added.
 
 Changes to the AMDGPU Target
 -



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

[PATCH] D85728: [Analyzer] Support for the new variadic isa<> and isa_and_not_null<> in CastValueChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Hans is pinging us on Bugzilla because this patch is marked as a release 
blocker (and i believe that it indeed is). I think we should land these patches.


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

https://reviews.llvm.org/D85728

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

[PATCH] D85728: [Analyzer] Support for the new variadic isa<> and isa_and_not_null<> in CastValueChecker

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D85728#2237006 , @NoQ wrote:

> Hans is pinging us on Bugzilla because this patch is marked as a release 
> blocker (and i believe that it indeed is). I think we should land these 
> patches.

I believe @baloghadamsoftware is on vacation.
I'm sure he won't be mad if you commit on his behalf.


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

https://reviews.llvm.org/D85728

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

[PATCH] D86491: [DebugInfo] Move constructor homing case in shouldOmitDefinition.

[Amy Huang via Phabricator via cfe-commits]

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

just reopening to update the diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86491

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

[PATCH] D86065: [SVE] Make ElementCount members private

[Christopher Tetreault via Phabricator via cfe-commits]

ctetreau added inline comments.



Comment at: llvm/include/llvm/Support/TypeSize.h:56
 
+  friend bool operator>(const ElementCount &LHS, const ElementCount &RHS) {
+assert(LHS.Scalable == RHS.Scalable &&

fpetrogalli wrote:
> paulwalker-arm wrote:
> > david-arm wrote:
> > > ctetreau wrote:
> > > > fpetrogalli wrote:
> > > > > I think that @ctetreau is right on 
> > > > > https://reviews.llvm.org/D85794#inline-793909. We should not overload 
> > > > > a comparison operator on this class because the set it represent it 
> > > > > cannot be ordered.
> > > > > 
> > > > > Chris suggests an approach of writing a static function that can be 
> > > > > used as a comparison operator,  so that we can make it explicit of 
> > > > > what kind of comparison we  are doing. 
> > > > In C++, it's common to overload the comparison operators for the 
> > > > purposes of being able to std::sort and use ordered sets. Normally, I 
> > > > would be OK with such usages. However, since `ElementCount` is 
> > > > basically a numeric type, and they only have a partial ordering, I 
> > > > think this is dangerous. I'm concerned that this will result in more 
> > > > bugs whereby somebody didn't remember that vectors can be scalable.
> > > > 
> > > > I don't have a strong opinion what the comparator function should be 
> > > > called, but I strongly prefer that it not be a comparison operator.
> > > Hi @ctetreau, yeah I understand. The reason I chose to use operators was 
> > > simply to be consistent with what we have already in TypeSize. Also, we 
> > > have existing "==" and "!=" operators in ElementCount too, although these 
> > > are essentially testing that two ElementCounts are identically the same 
> > > or not, i.e. for 2 given polynomials (a + bx) and (c + dx) we're 
> > > essentially asking if both a==c and b==d.
> > > 
> > > If I introduce a new comparison function, I'll probably keep the asserts 
> > > in for now, but in general we can do better than simply asserting if 
> > > something is scalable or not. For example, we know that (vscale * 4) is 
> > > definitely >= 4 because vscale is at least 1. I'm just not sure if we 
> > > have that need yet.
> > I think we should treat the non-equality comparison functions more like 
> > floating point.  What we don't want is somebody writing !GreaterThan when 
> > they actually mean LessThan.
> > 
> > Perhaps we should name the functions accordingly (i.e. ogt for 
> > OrderedAndGreaterThan).  We will also need matching less than functions 
> > since I can see those being useful when analysing constant insert/extract 
> > element indices which stand a good chance to be a known comparison (with 0 
> > being the most common index).
> > 
> May I suggest the following name scheme? (my 2 c, will not hold the patch for 
> not addressing this comment)
> 
> ```
> static bool [Non]Total(...)
> ```
> 
> with `` being 
> 
> * `LT` -> less than, aka `<`
> * `LToE` -> less than or equal, aka "<="
> * `GT` -> greater than, aka ">"
> * `GToE` -> greater than or equal, aka ">="
> 
> and `Total` , `NonTotal` being the prefix that gives information about the 
> behavior on the value of scalable:
> 
> `Total` -> for example, all scalable ECs are bigger than fixed ECs.
> `NonTotal` -> asserting on `(LHS.Scalable == RHS.Scalable)` before returning 
> `LHS.Min  RHS.Min`.
> 
> Taking it further: it could also be a template on an enumeration that list 
> the type of comparisons?
> 
> ```
> enum CMPType {
>   TotalGT,
>   NonTotalLT,
>   fancy_one
> };
> 
> ...
> 
> template 
> static bool cmp(ElementCount &LHS, ElementCount &RHS );
> 
> ...
> 
> static bool ElementCount::cmp(ElementCount 
> &LHS, ElementCount &RHS ) {
>  /// implementation
> }
> static bool ElementCount::cmp(ElementCount 
> &LHS, ElementCount &RHS ) {
>  /// implementation
> }
> ```
> 
> 
Honestly, I think this is actually worse. My issue is the fact that, from a 
mathematical perspective, `vscale_1 * min_1 < vscale_2 * min_2` is a function 
of `vscale_1` and `vscale_2`. In principle, we can know some ordering 
relationships between certain element counts (such as `vscale * min_1 >= min_2 
= true`), but in general, this function does not make sense. However, an 
`operator<` is useful because it allows you to put an `ElementCount` into an 
ordered set, and it will just work. Renaming the function to `olt` just makes 
it so that you can't put `ElementCount`s into an ordered set, but still implies 
that `ElementCount`s are comparable in general. This will also blow up if you 
actually try to mix fixed width and scalable `ElementCount`s in an ordered set, 
which should work IMO.

Here is what I propose:

1) Add a predicate for establishing an arbitrary ordering. The predicate would 
be completely arbitrary, because it's only useful for establishing an ordering 
for an ordered set or in a sorting algorithm. It could look something like this:

```
static bool orderedBefore(const ElementCount &LHS, const E

[PATCH] D86559: [Sema, CodeGen] Allow [[likely]] and [[unlikely]] on labels

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

In D86559#2236931 , @Quuxplusone wrote:

> It seems like this patch would basically "copy" the `[[likely]]` attribute 
> from line C up to lines A and B, where it would reinforce the likelihood of 
> path A and (maybe?) "cancel out" the unlikelihood of path B, without actually 
> saying anything specifically about the likelihood of label C (which is surely 
> what the programmer intended by applying the attribute, right?).

Yes A is double `[[likely]]` and B is both `[[likely]]` and `[[unlikely]]`.  
Based on http://eel.is/c++draft/dcl.attr.likelihood#:attribute,likely
"The use of the likely attribute is intended to allow implementations to 
optimize for the case where paths of execution including it are arbitrarily 
more likely than any alternative path of execution that does not include such 
an attribute on a statement or label." 
So it seem it's intended to see a goto as a part of a path of execution, since 
it's an unconditional jump.

But I think the standard should be improved:

  if(foo) {
[[likely]][[unlikely]] bar(1); // error: The attribute-token likely shall 
not appear in an
bar(2);  //  attribute-specifier-seq 
that contains the attribute-token unlikely.
  }
  if(foo) {
[[likely]] bar(1); 
[[unlikely]] bar(2);// This contradiction in the `ThenStmt` 
is allowed
  }
  if(foo) {
[[likely]] bar(1);
  } else {
[[likely]] bar(2);   // This contradiction between the 
`ThenStmt` and `ElseStmt` is allowed
  }

So I'll work on a paper to discuss these issues. I already have some notes, but 
I would prefer to get more implementation experience before starting to write.
IMO we should ignore conflicting likelihoods and issue a diagnostic. (For a 
switch multiple `[[likely]]` cases would be allowed, but still no mixing.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86559

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

[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library

[Alexandre Ganea via Phabricator via cfe-commits]

aganea updated this revision to Diff 287747.
aganea marked an inline comment as done.
aganea added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Rebase.
Address @MaskRay's suggestions.
Made `safeLldLink` a public API and made it possible to provide user-defined 
stdout/stderr streams.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70378

Files:
  clang/tools/driver/driver.cpp
  lld/COFF/Driver.cpp
  lld/COFF/Writer.cpp
  lld/COFF/Writer.h
  lld/Common/ErrorHandler.cpp
  lld/ELF/Driver.cpp
  lld/MachO/Driver.cpp
  lld/include/lld/Common/Driver.h
  lld/include/lld/Common/ErrorHandler.h
  lld/lib/Driver/DarwinLdDriver.cpp
  lld/test/COFF/dll.test
  lld/test/COFF/guardcf-lto.ll
  lld/test/COFF/lit.local.cfg
  lld/tools/lld/lld.cpp
  lld/wasm/Driver.cpp
  llvm/include/llvm/Support/CrashRecoveryContext.h
  llvm/include/llvm/Support/Signals.h
  llvm/lib/Support/CrashRecoveryContext.cpp
  llvm/lib/Support/Unix/Signals.inc
  llvm/lib/Support/Windows/Signals.inc

Index: llvm/lib/Support/Windows/Signals.inc
===
--- llvm/lib/Support/Windows/Signals.inc
+++ llvm/lib/Support/Windows/Signals.inc
@@ -868,3 +868,5 @@
  #pragma GCC diagnostic warning "-Wformat"
  #pragma GCC diagnostic warning "-Wformat-extra-args"
 #endif
+
+void sys::UnregisterHandlers() {}
Index: llvm/lib/Support/Unix/Signals.inc
===
--- llvm/lib/Support/Unix/Signals.inc
+++ llvm/lib/Support/Unix/Signals.inc
@@ -331,7 +331,7 @@
 registerHandler(S, SignalKind::IsInfo);
 }
 
-static void UnregisterHandlers() {
+void sys::UnregisterHandlers() {
   // Restore all of the signal handlers to how they were before we showed up.
   for (unsigned i = 0, e = NumRegisteredSignals.load(); i != e; ++i) {
 sigaction(RegisteredSignalInfo[i].SigNo,
@@ -367,7 +367,7 @@
   // crashes when we return and the signal reissues.  This also ensures that if
   // we crash in our signal handler that the program will terminate immediately
   // instead of recursing in the signal handler.
-  UnregisterHandlers();
+  sys::UnregisterHandlers();
 
   // Unmask all potentially blocked kill signals.
   sigset_t SigMask;
Index: llvm/lib/Support/CrashRecoveryContext.cpp
===
--- llvm/lib/Support/CrashRecoveryContext.cpp
+++ llvm/lib/Support/CrashRecoveryContext.cpp
@@ -97,6 +97,13 @@
 
 CrashRecoveryContextCleanup::~CrashRecoveryContextCleanup() {}
 
+CrashRecoveryContext::CrashRecoveryContext() {
+  // On Windows, if abort() was previously triggered (and caught by a previous
+  // CrashRecoveryContext) the Windows CRT removes our installed signal handler,
+  // so we need to install it again.
+  sys::DisableSystemDialogsOnCrash();
+}
+
 CrashRecoveryContext::~CrashRecoveryContext() {
   // Reclaim registered resources.
   CrashRecoveryContextCleanup *i = head;
@@ -370,9 +377,10 @@
   sigaddset(&SigMask, Signal);
   sigprocmask(SIG_UNBLOCK, &SigMask, nullptr);
 
-  // As per convention, -2 indicates a crash or timeout as opposed to failure to
-  // execute (see llvm/include/llvm/Support/Program.h)
-  int RetCode = -2;
+  // Return the same error code as if the program crashed, as mentioned in the
+  // section "Exit Status for Commands":
+  // https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xcu_chap02.html
+  int RetCode = 128 + Signal;
 
   // Don't consider a broken pipe as a crash (see clang/lib/Driver/Driver.cpp)
   if (Signal == SIGPIPE)
Index: llvm/include/llvm/Support/Signals.h
===
--- llvm/include/llvm/Support/Signals.h
+++ llvm/include/llvm/Support/Signals.h
@@ -115,6 +115,8 @@
   /// Context is a system-specific failure context: it is the signal type on
   /// Unix; the ExceptionContext on Windows.
   void CleanupOnSignal(uintptr_t Context);
+
+  void UnregisterHandlers();
 } // End sys namespace
 } // End llvm namespace
 
Index: llvm/include/llvm/Support/CrashRecoveryContext.h
===
--- llvm/include/llvm/Support/CrashRecoveryContext.h
+++ llvm/include/llvm/Support/CrashRecoveryContext.h
@@ -44,11 +44,11 @@
 /// executed in any case, whether crash occurs or not. These actions may be used
 /// to reclaim resources in the case of crash.
 class CrashRecoveryContext {
-  void *Impl;
-  CrashRecoveryContextCleanup *head;
+  void *Impl = nullptr;
+  CrashRecoveryContextCleanup *head = nullptr;
 
 public:
-  CrashRecoveryContext() : Impl(nullptr), head(nullptr) {}
+  CrashRecoveryContext();
   ~CrashRecoveryContext();
 
   /// Register cleanup handler, which is used when the recovery context is
Index: lld/wasm/Driver.cpp
===
--- lld/wasm/Driver.cpp
+++ lld/wasm/Driver.cpp
@@ -8

[PATCH] D84458: [Modules] Improve error message when cannot find parent module for submodule definition.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai updated this revision to Diff 287749.
vsapsai added a comment.

Tweak the error text.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84458

Files:
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/ModuleMap.cpp
  clang/test/Modules/diagnostics.modulemap


Index: clang/test/Modules/diagnostics.modulemap
===
--- clang/test/Modules/diagnostics.modulemap
+++ clang/test/Modules/diagnostics.modulemap
@@ -28,3 +28,9 @@
   header "quux.h" { size 1 mtime 2 }
   header "no_attrs.h" {}
 }
+
+// CHECK: diagnostics.modulemap:[[@LINE+1]]:8: error: no module named 
'unknown' found, parent module must be defined before the submodule
+module unknown.submodule {}
+module known_top_level {}
+// CHECK: diagnostics.modulemap:[[@LINE+1]]:24: error: no module named 
'unknown' in 'known_top_level', parent module must be defined before the 
submodule
+module known_top_level.unknown.submodule {}
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1903,18 +1903,16 @@
 continue;
   }
 
-  if (ActiveModule) {
-Diags.Report(Id[I].second, diag::err_mmap_missing_module_qualified)
-  << Id[I].first
-  << ActiveModule->getTopLevelModule()->getFullModuleName();
-  } else {
-Diags.Report(Id[I].second, diag::err_mmap_expected_module_name);
-  }
+  Diags.Report(Id[I].second, diag::err_mmap_missing_parent_module)
+  << Id[I].first << (ActiveModule != nullptr)
+  << (ActiveModule
+  ? ActiveModule->getTopLevelModule()->getFullModuleName()
+  : "");
   HadError = true;
-  return;
 }
 
-if (ModuleMapFile != Map.getContainingModuleMapFile(TopLevelModule)) {
+if (TopLevelModule &&
+ModuleMapFile != Map.getContainingModuleMapFile(TopLevelModule)) {
   assert(ModuleMapFile != Map.getModuleMapFileForUniquing(TopLevelModule) 
&&
  "submodule defined in same file as 'module *' that allowed its "
  "top-level module");
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===
--- clang/include/clang/Basic/DiagnosticLexKinds.td
+++ clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -695,6 +695,9 @@
   "no module named '%0' visible from '%1'">;
 def err_mmap_missing_module_qualified : Error<
   "no module named '%0' in '%1'">;
+def err_mmap_missing_parent_module: Error<
+  "no module named '%0' %select{found|in '%2'}1, "
+  "parent module must be defined before the submodule">;
 def err_mmap_top_level_inferred_submodule : Error<
   "only submodules and framework modules may be inferred with wildcard 
syntax">;
 def err_mmap_inferred_no_umbrella : Error<


Index: clang/test/Modules/diagnostics.modulemap
===
--- clang/test/Modules/diagnostics.modulemap
+++ clang/test/Modules/diagnostics.modulemap
@@ -28,3 +28,9 @@
   header "quux.h" { size 1 mtime 2 }
   header "no_attrs.h" {}
 }
+
+// CHECK: diagnostics.modulemap:[[@LINE+1]]:8: error: no module named 'unknown' found, parent module must be defined before the submodule
+module unknown.submodule {}
+module known_top_level {}
+// CHECK: diagnostics.modulemap:[[@LINE+1]]:24: error: no module named 'unknown' in 'known_top_level', parent module must be defined before the submodule
+module known_top_level.unknown.submodule {}
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1903,18 +1903,16 @@
 continue;
   }
 
-  if (ActiveModule) {
-Diags.Report(Id[I].second, diag::err_mmap_missing_module_qualified)
-  << Id[I].first
-  << ActiveModule->getTopLevelModule()->getFullModuleName();
-  } else {
-Diags.Report(Id[I].second, diag::err_mmap_expected_module_name);
-  }
+  Diags.Report(Id[I].second, diag::err_mmap_missing_parent_module)
+  << Id[I].first << (ActiveModule != nullptr)
+  << (ActiveModule
+  ? ActiveModule->getTopLevelModule()->getFullModuleName()
+  : "");
   HadError = true;
-  return;
 }
 
-if (ModuleMapFile != Map.getContainingModuleMapFile(TopLevelModule)) {
+if (TopLevelModule &&
+ModuleMapFile != Map.getContainingModuleMapFile(TopLevelModule)) {
   assert(ModuleMapFile != Map.getModuleMapFileForUniquing(TopLevelModule) &&
  "submodule defined in same file as 'module *' that allowed its "
  "top-level module");
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===
--- clang/include/

[PATCH] D86491: [DebugInfo] Move constructor homing case in shouldOmitDefinition.

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 287748.
akhuang added a comment.

Fix errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86491

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp


Index: clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
===
--- clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
+++ clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple 
-debug-info-kind=limited %s -o - | FileCheck %s
 
+// Make sure this still works with constructor homing.
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple 
-debug-info-kind=constructor %s -o - | FileCheck %s
+
 // Run again with -gline-tables-only or -gline-directives-only and verify we 
don't crash.  We won't output
 // type info at all.
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple 
-debug-info-kind=line-tables-only %s -o - | FileCheck %s -check-prefix 
LINES-ONLY
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2260,6 +2260,25 @@
   return false;
 }
 
+static bool canUseCtorHoming(const CXXRecordDecl *RD) {
+  // Constructor homing can be used for classes that have at least one
+  // constructor and have no trivial or constexpr constructors.
+  // Skip this optimization if the class or any of its methods are marked
+  // dllimport.
+  if (RD->isLambda() || RD->hasConstexprNonCopyMoveConstructor() ||
+  isClassOrMethodDLLImport(RD))
+return false;
+
+  if (RD->ctors().empty())
+return false;
+
+  for (const auto *Ctor : RD->ctors())
+if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor())
+  return false;
+
+  return true;
+}
+
 static bool shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
  bool DebugTypeExtRefs, const RecordDecl *RD,
  const LangOptions &LangOpts) {
@@ -2294,23 +2313,6 @@
   !isClassOrMethodDLLImport(CXXDecl))
 return true;
 
-  // In constructor debug mode, only emit debug info for a class when its
-  // constructor is emitted. Skip this optimization if the class or any of
-  // its methods are marked dllimport.
-  //
-  // This applies to classes that don't have any trivial constructors and have
-  // at least one constructor.
-  if (DebugKind == codegenoptions::DebugInfoConstructor &&
-  !CXXDecl->isLambda() && !CXXDecl->hasConstexprNonCopyMoveConstructor() &&
-  !isClassOrMethodDLLImport(CXXDecl)) {
-if (CXXDecl->ctors().empty())
-  return false;
-for (const auto *Ctor : CXXDecl->ctors())
-  if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor())
-return false;
-return true;
-  }
-
   TemplateSpecializationKind Spec = TSK_Undeclared;
   if (const auto *SD = dyn_cast(RD))
 Spec = SD->getSpecializationKind();
@@ -2320,6 +2322,12 @@
   CXXDecl->method_end()))
 return true;
 
+  // In constructor homing mode, only emit complete debug info for a class
+  // when its constructor is emitted.
+  if ((DebugKind == codegenoptions::DebugInfoConstructor) &&
+  canUseCtorHoming(CXXDecl))
+return true;
+
   return false;
 }
 


Index: clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
===
--- clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
+++ clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -debug-info-kind=limited %s -o - | FileCheck %s
 
+// Make sure this still works with constructor homing.
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -debug-info-kind=constructor %s -o - | FileCheck %s
+
 // Run again with -gline-tables-only or -gline-directives-only and verify we don't crash.  We won't output
 // type info at all.
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -debug-info-kind=line-tables-only %s -o - | FileCheck %s -check-prefix LINES-ONLY
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2260,6 +2260,25 @@
   return false;
 }
 
+static bool canUseCtorHoming(const CXXRecordDecl *RD) {
+  // Constructor homing can be used for classes that have at least one
+  // constructor and have no trivial or constexpr constructors.
+  // Skip this optimization if the class or any of its methods are marked
+  // dllimport.
+  if (RD->isLambda() || RD->hasConstexprNonCopyMoveConstructor() ||
+ 

[clang] b1009ee - Reland "[DebugInfo] Move constructor homing case in shouldOmitDefinition."

[Amy Huang via cfe-commits]

Author: Amy Huang
Date: 2020-08-25T12:36:11-07:00
New Revision: b1009ee84fc0242bcebd07889306bf39d9b7170f

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

LOG: Reland "[DebugInfo] Move constructor homing case in shouldOmitDefinition."

For some reason the ctor homing case was before the template
specialization case, and could have returned false too early.
I moved the code out into a separate function to avoid this.

This reverts commit 05777ab941063192b9ccb1775358a83a2700ccc1.

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index e3442ecd4bd5..36bab9c22d43 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2260,6 +2260,25 @@ static bool 
hasExplicitMemberDefinition(CXXRecordDecl::method_iterator I,
   return false;
 }
 
+static bool canUseCtorHoming(const CXXRecordDecl *RD) {
+  // Constructor homing can be used for classes that have at least one
+  // constructor and have no trivial or constexpr constructors.
+  // Skip this optimization if the class or any of its methods are marked
+  // dllimport.
+  if (RD->isLambda() || RD->hasConstexprNonCopyMoveConstructor() ||
+  isClassOrMethodDLLImport(RD))
+return false;
+
+  if (RD->ctors().empty())
+return false;
+
+  for (const auto *Ctor : RD->ctors())
+if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor())
+  return false;
+
+  return true;
+}
+
 static bool shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
  bool DebugTypeExtRefs, const RecordDecl *RD,
  const LangOptions &LangOpts) {
@@ -2294,23 +2313,6 @@ static bool 
shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
   !isClassOrMethodDLLImport(CXXDecl))
 return true;
 
-  // In constructor debug mode, only emit debug info for a class when its
-  // constructor is emitted. Skip this optimization if the class or any of
-  // its methods are marked dllimport.
-  //
-  // This applies to classes that don't have any trivial constructors and have
-  // at least one constructor.
-  if (DebugKind == codegenoptions::DebugInfoConstructor &&
-  !CXXDecl->isLambda() && !CXXDecl->hasConstexprNonCopyMoveConstructor() &&
-  !isClassOrMethodDLLImport(CXXDecl)) {
-if (CXXDecl->ctors().empty())
-  return false;
-for (const auto *Ctor : CXXDecl->ctors())
-  if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor())
-return false;
-return true;
-  }
-
   TemplateSpecializationKind Spec = TSK_Undeclared;
   if (const auto *SD = dyn_cast(RD))
 Spec = SD->getSpecializationKind();
@@ -2320,6 +2322,12 @@ static bool 
shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
   CXXDecl->method_end()))
 return true;
 
+  // In constructor homing mode, only emit complete debug info for a class
+  // when its constructor is emitted.
+  if ((DebugKind == codegenoptions::DebugInfoConstructor) &&
+  canUseCtorHoming(CXXDecl))
+return true;
+
   return false;
 }
 

diff  --git 
a/clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp 
b/clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
index 4e41c4092bf4..b756674f54c4 100644
--- a/clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
+++ b/clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple 
-debug-info-kind=limited %s -o - | FileCheck %s
 
+// Make sure this still works with constructor homing.
+// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple 
-debug-info-kind=constructor %s -o - | FileCheck %s
+
 // Run again with -gline-tables-only or -gline-directives-only and verify we 
don't crash.  We won't output
 // type info at all.
 // RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple 
-debug-info-kind=line-tables-only %s -o - | FileCheck %s -check-prefix 
LINES-ONLY



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

[PATCH] D82582: [SVE] Remove calls to VectorType::getNumElements from clang

[Christopher Tetreault via Phabricator via cfe-commits]

ctetreau updated this revision to Diff 287755.
ctetreau marked 4 inline comments as done.
ctetreau added a comment.

address code review issues, rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82582

Files:
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/SwiftCallingConv.cpp

Index: clang/lib/CodeGen/SwiftCallingConv.cpp
===
--- clang/lib/CodeGen/SwiftCallingConv.cpp
+++ clang/lib/CodeGen/SwiftCallingConv.cpp
@@ -320,9 +320,12 @@
   // If we have a vector type, split it.
   if (auto vecTy = dyn_cast_or_null(type)) {
 auto eltTy = vecTy->getElementType();
-CharUnits eltSize = (end - begin) / vecTy->getNumElements();
+CharUnits eltSize =
+(end - begin) / cast(vecTy)->getNumElements();
 assert(eltSize == getTypeStoreSize(CGM, eltTy));
-for (unsigned i = 0, e = vecTy->getNumElements(); i != e; ++i) {
+for (unsigned i = 0,
+  e = cast(vecTy)->getNumElements();
+ i != e; ++i) {
   addEntry(eltTy, begin, begin + eltSize);
   begin += eltSize;
 }
@@ -674,8 +677,9 @@
 
 bool swiftcall::isLegalVectorType(CodeGenModule &CGM, CharUnits vectorSize,
   llvm::VectorType *vectorTy) {
-  return isLegalVectorType(CGM, vectorSize, vectorTy->getElementType(),
-   vectorTy->getNumElements());
+  return isLegalVectorType(
+  CGM, vectorSize, vectorTy->getElementType(),
+  cast(vectorTy)->getNumElements());
 }
 
 bool swiftcall::isLegalVectorType(CodeGenModule &CGM, CharUnits vectorSize,
@@ -688,7 +692,7 @@
 std::pair
 swiftcall::splitLegalVectorType(CodeGenModule &CGM, CharUnits vectorSize,
 llvm::VectorType *vectorTy) {
-  auto numElts = vectorTy->getNumElements();
+  auto numElts = cast(vectorTy)->getNumElements();
   auto eltTy = vectorTy->getElementType();
 
   // Try to split the vector type in half.
@@ -710,7 +714,7 @@
   }
 
   // Try to split the vector into legal subvectors.
-  auto numElts = origVectorTy->getNumElements();
+  auto numElts = cast(origVectorTy)->getNumElements();
   auto eltTy = origVectorTy->getElementType();
   assert(numElts != 1);
 
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -1320,7 +1320,7 @@
"Splatted expr doesn't match with vector element type?");
 
 // Splat the element across to all elements
-unsigned NumElements = cast(DstTy)->getNumElements();
+unsigned NumElements = cast(DstTy)->getNumElements();
 return Builder.CreateVectorSplat(NumElements, Src, "splat");
   }
 
@@ -1553,12 +1553,12 @@
 Value *RHS = CGF.EmitScalarExpr(E->getExpr(1));
 Value *Mask;
 
-llvm::VectorType *LTy = cast(LHS->getType());
+auto *LTy = cast(LHS->getType());
 unsigned LHSElts = LTy->getNumElements();
 
 Mask = RHS;
 
-llvm::VectorType *MTy = cast(Mask->getType());
+auto *MTy = cast(Mask->getType());
 
 // Mask off the high bits of each shuffle index.
 Value *MaskBits =
@@ -1763,7 +1763,7 @@
 return Visit(E->getInit(0));
   }
 
-  unsigned ResElts = VType->getNumElements();
+  unsigned ResElts = cast(VType)->getNumElements();
 
   // Loop over initializers collecting the Value for each, and remembering
   // whether the source was swizzle (ExtVectorElementExpr).  This will allow
@@ -1787,7 +1787,8 @@
   if (isa(IE)) {
 llvm::ExtractElementInst *EI = cast(Init);
 
-if (EI->getVectorOperandType()->getNumElements() == ResElts) {
+if (cast(EI->getVectorOperandType())
+->getNumElements() == ResElts) {
   llvm::ConstantInt *C = cast(EI->getIndexOperand());
   Value *LHS = nullptr, *RHS = nullptr;
   if (CurIdx == 0) {
@@ -1825,7 +1826,7 @@
   continue;
 }
 
-unsigned InitElts = VVT->getNumElements();
+unsigned InitElts = cast(VVT)->getNumElements();
 
 // If the initializer is an ExtVecEltExpr (a swizzle), and the swizzle's
 // input is the same width as the vector being constructed, generate an
@@ -1834,7 +1835,7 @@
 if (isa(IE)) {
   llvm::ShuffleVectorInst *SVI = cast(Init);
   Value *SVOp = SVI->getOperand(0);
-  llvm::VectorType *OpTy = cast(SVOp->getType());
+  auto *OpTy = cast(SVOp->getType());
 
   if (OpTy->getNumElements() == ResElts) {
 for (unsigned j = 0; j != CurIdx; ++j) {
@@ -2170,7 +2171,7 @@
 llvm::Type *DstTy = ConvertType(DestTy);
 Value *Elt = Visit(const_cast(E));
 // Splat the element across to all elements
-unsigned NumElements = cast(DstTy)->getNumElements();
+unsigned NumElements = cast(DstTy)->getNumElements();
 retur

[clang] 97ccf93 - [SystemZ][z/OS] Add z/OS Target and define macros

[Hubert Tong via cfe-commits]

Author: Abhina Sreeskantharajan
Date: 2020-08-25T15:51:59-04:00
New Revision: 97ccf93b3615ff4c0d5fe116e6a7c7b616d8ec0c

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

LOG: [SystemZ][z/OS] Add z/OS Target and define macros

This patch adds the z/OS target and defines macros as a stepping stone
towards enabling a native build on z/OS.

Reviewed By: hubert.reinterpretcast

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

Added: 
clang/test/Preprocessor/init-zos.c

Modified: 
clang/lib/Basic/Targets.cpp
clang/lib/Basic/Targets/OSTargets.h

Removed: 




diff  --git a/clang/lib/Basic/Targets.cpp b/clang/lib/Basic/Targets.cpp
index 38a388afa534..933b7b342891 100644
--- a/clang/lib/Basic/Targets.cpp
+++ b/clang/lib/Basic/Targets.cpp
@@ -452,6 +452,8 @@ TargetInfo *AllocateTarget(const llvm::Triple &Triple,
 switch (os) {
 case llvm::Triple::Linux:
   return new LinuxTargetInfo(Triple, Opts);
+case llvm::Triple::ZOS:
+  return new ZOSTargetInfo(Triple, Opts);
 default:
   return new SystemZTargetInfo(Triple, Opts);
 }

diff  --git a/clang/lib/Basic/Targets/OSTargets.h 
b/clang/lib/Basic/Targets/OSTargets.h
index a2c0fd42f26d..9c206fc7e6a4 100644
--- a/clang/lib/Basic/Targets/OSTargets.h
+++ b/clang/lib/Basic/Targets/OSTargets.h
@@ -728,6 +728,55 @@ class AIXTargetInfo : public OSTargetInfo {
   bool defaultsToAIXPowerAlignment() const override { return true; }
 };
 
+// z/OS target
+template 
+class LLVM_LIBRARY_VISIBILITY ZOSTargetInfo : public OSTargetInfo {
+protected:
+  void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple,
+MacroBuilder &Builder) const override {
+// FIXME: _LONG_LONG should not be defined under -std=c89.
+Builder.defineMacro("_LONG_LONG");
+Builder.defineMacro("_OPEN_DEFAULT");
+// _UNIX03_WITHDRAWN is required to build libcxx.
+Builder.defineMacro("_UNIX03_WITHDRAWN");
+Builder.defineMacro("__370__");
+Builder.defineMacro("__BFP__");
+// FIXME: __BOOL__ should not be defined under -std=c89.
+Builder.defineMacro("__BOOL__");
+Builder.defineMacro("__LONGNAME__");
+Builder.defineMacro("__MVS__");
+Builder.defineMacro("__THW_370__");
+Builder.defineMacro("__THW_BIG_ENDIAN__");
+Builder.defineMacro("__TOS_390__");
+Builder.defineMacro("__TOS_MVS__");
+Builder.defineMacro("__XPLINK__");
+
+if (this->PointerWidth == 64)
+  Builder.defineMacro("__64BIT__");
+
+if (Opts.CPlusPlus) {
+  Builder.defineMacro("__DLL__");
+  // _XOPEN_SOURCE=600 is required to build libcxx.
+  Builder.defineMacro("_XOPEN_SOURCE", "600");
+}
+
+if (Opts.GNUMode) {
+  Builder.defineMacro("_MI_BUILTIN");
+  Builder.defineMacro("_EXT");
+}
+
+if (Opts.CPlusPlus && Opts.WChar) {
+  // Macro __wchar_t is defined so that the wchar_t data
+  // type is not declared as a typedef in system headers.
+  Builder.defineMacro("__wchar_t");
+}
+  }
+
+public:
+  ZOSTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts)
+  : OSTargetInfo(Triple, Opts) {}
+};
+
 void addWindowsDefines(const llvm::Triple &Triple, const LangOptions &Opts,
MacroBuilder &Builder);
 

diff  --git a/clang/test/Preprocessor/init-zos.c 
b/clang/test/Preprocessor/init-zos.c
new file mode 100644
index ..50c4ed9e539e
--- /dev/null
+++ b/clang/test/Preprocessor/init-zos.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -E -dM -ffreestanding -triple=s390x-none-zos 
-fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix 
S390X-ZOS %s
+// RUN: %clang_cc1 -x c++ -std=gnu++14 -E -dM -ffreestanding 
-triple=s390x-none-zos -fno-signed-char < /dev/null | FileCheck 
-match-full-lines -check-prefix S390X-ZOS -check-prefix S390X-ZOS-GNUXX %s
+
+// S390X-ZOS-GNUXX:#define _EXT 1
+// S390X-ZOS:#define _LONG_LONG 1
+// S390X-ZOS-GNUXX:#define _MI_BUILTIN 1
+// S390X-ZOS:#define _OPEN_DEFAULT 1
+// S390X-ZOS:#define _UNIX03_WITHDRAWN 1
+// S390X-ZOS-GNUXX:#define _XOPEN_SOURCE 600
+// S390X-ZOS:#define __370__ 1
+// S390X-ZOS:#define __64BIT__ 1
+// S390X-ZOS:#define __BFP__ 1
+// S390X-ZOS:#define __BOOL__ 1
+// S390X-ZOS-GNUXX:#define __DLL__ 1
+// S390X-ZOS:#define __LONGNAME__ 1
+// S390X-ZOS:#define __MVS__ 1
+// S390X-ZOS:#define __THW_370__ 1
+// S390X-ZOS:#define __THW_BIG_ENDIAN__ 1
+// S390X-ZOS:#define __TOS_390__ 1
+// S390X-ZOS:#define __TOS_MVS__ 1
+// S390X-ZOS:#define __XPLINK__ 1
+// S390X-ZOS-GNUXX:#define __wchar_t 1



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