[PATCH] D27651: [clang-format] Even with AlignConsecutiveDeclarations, PointerAlignment: Right should keep *s and &s to the right

[Marek Kurdej via Phabricator via cfe-commits]

curdeius closed this revision.
curdeius added a comment.

This has been superseded by D103245 . Closing.


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

https://reviews.llvm.org/D27651

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

[clang] 72390f0 - DirectoryWatcher-linux.cpp - add missing implicit MathExtras.h header dependency. NFCI.

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2021-06-06T09:40:20+01:00
New Revision: 72390f0c28dddffc53c20ba0ec765b8de4e05383

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

LOG: DirectoryWatcher-linux.cpp - add missing implicit MathExtras.h header 
dependency. NFCI.

Added: 


Modified: 
clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp

Removed: 




diff  --git a/clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp 
b/clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
index 176d6d6abf33b..963256f268bbb 100644
--- a/clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
+++ b/clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
@@ -14,6 +14,7 @@
 #include "llvm/Support/AlignOf.h"
 #include "llvm/Support/Errno.h"
 #include "llvm/Support/Error.h"
+#include "llvm/Support/MathExtras.h"
 #include "llvm/Support/Path.h"
 #include 
 #include 



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

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

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D101868#2799563 , 
@HazardyKnusperkeks wrote:

> Remains the issue with the alignment, I would like to know @MyDeveloperDay 's 
> opinion on that. Should the values be right aligned, or left aligned? As far 
> as I see all alignment in clang-format is left until now.

I think whatever you choose someone will ask for the opposite, maybe consider 
making it an option now while you are in the code and understand it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101868

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

[PATCH] D103663: [AMDGPU] Add gfx1013 target

[Brendon Cahoon via Phabricator via cfe-commits]

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



Comment at: llvm/docs/AMDGPUUsage.rst:389
 - xnack 
scratch   - *pal-amdpal*
+ ``gfx1013`` ``amdgcn``   dGPU  - cumode  - 
Absolute  - *rocm-amdhsa* *TBA*
+- wavefrontsize64   flat   
   - *pal-amdhsa*

foad wrote:
> Is it dGPU or APU?
> 
> Every other entry with `*TBA*` also has a `TODO::` message
It is APU.



Comment at: llvm/lib/Target/AMDGPU/AMDGPU.td:468
 
+def FeatureGFX10_AEncoding : SubtargetFeature<"gfx10_a-encoding",
+  "GFX10_AEncoding",

foad wrote:
> What is this new encoding? It doesn't seem to be used for anything.
Fixed this. The BVH raytracing instructions use the encoding. 



Comment at: llvm/lib/Target/AMDGPU/AMDGPU.td:1106
   [FeatureGFX10,
FeatureGFX10_BEncoding,
FeatureGFX10_3Insts,

rampitec wrote:
> gfx1030 should now include FeatureGFX10_AEncoding as well. 10_B is an 
> extension above 10_A.
I had added FeatureGFX10_AEncoding as an Implies feature for 
FeatureGFX10_BEncoding in the previous patch.  But, I've changed the patch so 
that it's no longer an Implies feature.



Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.intersect_ray.ll:4
+; RUN: llc -global-isel -march=amdgcn -mcpu=gfx1013 -verify-machineinstrs < %s 
| FileCheck -check-prefix=GCN %s
+; RUN: llc -global-isel -march=amdgcn -mcpu=gfx1012 -verify-machineinstrs < %s 
| FileCheck -check-prefix=GCN %s
 

rampitec wrote:
> rampitec wrote:
> > foad wrote:
> > > This test surely should not pass for gfx1012, since it does not have 
> > > these instructions. And with your patch as written it should fail for 
> > > gfx1013 too, since they are predicated on HasGFX10_BEncoding.
> > > 
> > > @rampitec any idea what is wrong here? Apparently the backend will 
> > > happily generate image_bvh_intersect_ray instructions even for gfx900!
> > Indeed. MIMG_IntersectRay has this:
> > 
> > ```
> >   let SubtargetPredicate = HasGFX10_BEncoding,
> >   AssemblerPredicate = HasGFX10_BEncoding,
> > ```
> > but apparently SubtargetPredicate did not work. It needs to be fixed.
> > 
> > gfx1012 does not have it, gfx1013 does though. That is what GFX10_A 
> > encoding is about, 10_B it has to be replaced with 10_A in BVH and MSAA 
> > load.
> Image lowering and selection is not really done like everything else. For BVH 
> it just lowers intrinsic to opcode. I think the easiest fix is to add to 
> SIISelLowering.cpp where we lower Intrinsic::amdgcn_image_bvh_intersect_ray 
> something like this:
> 
> 
> ```
>   if (!Subtarget->hasGFX10_AEncoding())
> report_fatal_error(
> "requested image instruction is not supported on this GPU");
> ```
I ended up using emitRemovedIntrinsicError, which uses 
DiagnosticInfoUnsupported. This way the failure isn't a crash dump.


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

https://reviews.llvm.org/D103663

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

[PATCH] D103750: [analyzer][WIP] Handle std::make_unique for SmartPtrModeling

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD updated this revision to Diff 350103.
RedDocMD added a comment.

Fixed binding of SVal to variable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103750

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -35,11 +35,19 @@
 using namespace ento;
 
 namespace {
+
+enum class MakeUniqueKind {
+  Regular, // ie, std::make_unique
+  ForOverwrite, // ie, std::make_unique_for_overwrite
+  None // ie, is neither of the above two
+};
+
 class SmartPtrModeling
 : public Checker {
+ check::LiveSymbols, check::Bind> {
 
   bool isBoolConversionMethod(const CallEvent &Call) const;
+  MakeUniqueKind isStdMakeUniqueCall(const CallEvent &Call) const;
 
 public:
   // Whether the checker should model for null dereferences of smart pointers.
@@ -56,6 +64,7 @@
   void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
   const char *Sep) const override;
   void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) const;
+  void checkBind(SVal L, SVal V, const Stmt *S, CheckerContext &C) const;
 
 private:
   void handleReset(const CallEvent &Call, CheckerContext &C) const;
@@ -68,6 +77,7 @@
   bool updateMovedSmartPointers(CheckerContext &C, const MemRegion *ThisRegion,
 const MemRegion *OtherSmartPtrRegion) const;
   void handleBoolConversion(const CallEvent &Call, CheckerContext &C) const;
+  void handleStdMakeUnique(const CallEvent &Call, CheckerContext &C, MakeUniqueKind Kind) const;
 
   using SmartPtrMethodHandlerFn =
   void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
@@ -79,7 +89,21 @@
 };
 } // end of anonymous namespace
 
+class MakeUniqueKindWrapper {
+  const MakeUniqueKind Kind;
+
+public:
+  MakeUniqueKindWrapper(MakeUniqueKind Kind) : Kind(Kind) {}
+  MakeUniqueKind get() const { return Kind; }
+  void Profile(llvm::FoldingSetNodeID &ID) const {
+ID.AddInteger(static_cast(Kind));
+  }
+  bool operator==(const MakeUniqueKind &RHS) const { return Kind == RHS; }
+  bool operator!=(const MakeUniqueKind &RHS) const { return Kind != RHS; }
+};
+
 REGISTER_MAP_WITH_PROGRAMSTATE(TrackedRegionMap, const MemRegion *, SVal)
+REGISTER_LIST_WITH_PROGRAMSTATE(MakeUniqueKindList, MakeUniqueKindWrapper)
 
 // Define the inter-checker API.
 namespace clang {
@@ -175,8 +199,34 @@
   return CD && CD->getConversionType()->isBooleanType();
 }
 
+MakeUniqueKind SmartPtrModeling::isStdMakeUniqueCall(const CallEvent &Call) const {
+  if (Call.getKind() != CallEventKind::CE_Function)
+return MakeUniqueKind::None;
+  const auto *D = Call.getDecl();
+  if (!D)
+return MakeUniqueKind::None;
+  const auto *FTD = llvm::dyn_cast(D);
+  if (!FTD)
+return MakeUniqueKind::None;
+  if (FTD->getDeclName().isIdentifier()) {
+StringRef Name = FTD->getName();
+if (Name == "make_unique")
+  return MakeUniqueKind::Regular;
+else if (Name == "make_unique_for_overwrite")
+  return MakeUniqueKind::ForOverwrite;
+  }
+  return MakeUniqueKind::None;
+}
+
 bool SmartPtrModeling::evalCall(const CallEvent &Call,
 CheckerContext &C) const {
+
+  MakeUniqueKind Kind = isStdMakeUniqueCall(Call);
+  if (Kind != MakeUniqueKind::None) {
+handleStdMakeUnique(Call, C, Kind);
+return true;
+  }
+
   ProgramStateRef State = C.getState();
   if (!smartptr::isStdSmartPtrCall(Call))
 return false;
@@ -272,6 +322,54 @@
   return C.isDifferent();
 }
 
+void SmartPtrModeling::handleStdMakeUnique(const CallEvent &Call,
+CheckerContext &C, MakeUniqueKind Kind) const {
+  assert(Kind != MakeUniqueKind::None && "Call is not to make_unique or make_unique_for_overwrite");
+  ProgramStateRef State = C.getState();
+  State = State->add(Kind);
+  C.addTransition(State);
+}
+
+bool isUniquePtrType(QualType QT) {
+  const auto *T = QT.getTypePtr();
+  if (!T)
+return false;
+  const auto *Decl = T->getAsCXXRecordDecl();
+  if (!Decl || !Decl->getDeclContext()->isStdNamespace())
+return false;
+  const IdentifierInfo *ID = Decl->getIdentifier();
+  if (!ID)
+return false;
+  const StringRef Name = ID->getName();
+  return Name == "unique_ptr";
+}
+
+void SmartPtrModeling::checkBind(SVal L, SVal V, const Stmt *S, CheckerContext &C) const {
+  const auto *TVR = dyn_cast_or_null(L.getAsRegion());
+  if (!TVR)
+return;
+  if (!isUniquePtrType(TVR->getValueType()))
+return;
+  const auto *ThisRegion = dyn_cast(TVR);
+  if (!ThisRegion)
+return;
+
+  ProgramStateRef State = C.getState();
+  auto KindList = State->get();
+  assert(!KindList.isEmpty() && "Expected list to cont

[clang] d466ca0 - [Clang][OpenMP] Add static version of getSingleClause. NFC.

[Michael Kruse via cfe-commits]

Author: Michael Kruse
Date: 2021-06-06T09:17:42-05:00
New Revision: d466ca087aae958d1c0a965c561be07d2cb3e7e2

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

LOG: [Clang][OpenMP] Add static version of getSingleClause. NFC.

The current method getSingleClause requires an instance of 
OMPExecutableDirective to be called. Introduce a static version taking a list 
of clauses as argument instead that can be used during parsing/Sema before any 
OMPExecutableDirective has been created.

This is the same approach as taken for getClausesOfKind for getting more more 
than a single clause of a type which also has a method and static version. NFC 
patch extracted out of D99459 by request.

Reviewed By: ABataev

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

Added: 


Modified: 
clang/include/clang/AST/StmtOpenMP.h

Removed: 




diff  --git a/clang/include/clang/AST/StmtOpenMP.h 
b/clang/include/clang/AST/StmtOpenMP.h
index 4d6774c1ad280..67ba77a5d1b06 100644
--- a/clang/include/clang/AST/StmtOpenMP.h
+++ b/clang/include/clang/AST/StmtOpenMP.h
@@ -460,17 +460,22 @@ class OMPExecutableDirective : public Stmt {
   /// directive). Returns nullptr if no clause of this kind is associated with
   /// the directive.
   template 
-  const SpecificClause *getSingleClause() const {
-auto Clauses = getClausesOfKind();
+  static const SpecificClause *getSingleClause(ArrayRef Clauses) {
+auto ClausesOfKind = getClausesOfKind(Clauses);
 
-if (Clauses.begin() != Clauses.end()) {
-  assert(std::next(Clauses.begin()) == Clauses.end() &&
+if (ClausesOfKind.begin() != ClausesOfKind.end()) {
+  assert(std::next(ClausesOfKind.begin()) == ClausesOfKind.end() &&
  "There are at least 2 clauses of the specified kind");
-  return *Clauses.begin();
+  return *ClausesOfKind.begin();
 }
 return nullptr;
   }
 
+  template 
+  const SpecificClause *getSingleClause() const {
+return getSingleClause(clauses());
+  }
+
   /// Returns true if the current directive has one or more clauses of a
   /// specific kind.
   template 



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

[PATCH] D103665: [Clang][OpenMP] Add static version of getSingleClause. NFC.

[Michael Kruse via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd466ca087aae: [Clang][OpenMP] Add static version of 
getSingleClause. NFC. (authored by Meinersbur).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103665

Files:
  clang/include/clang/AST/StmtOpenMP.h


Index: clang/include/clang/AST/StmtOpenMP.h
===
--- clang/include/clang/AST/StmtOpenMP.h
+++ clang/include/clang/AST/StmtOpenMP.h
@@ -460,17 +460,22 @@
   /// directive). Returns nullptr if no clause of this kind is associated with
   /// the directive.
   template 
-  const SpecificClause *getSingleClause() const {
-auto Clauses = getClausesOfKind();
+  static const SpecificClause *getSingleClause(ArrayRef Clauses) {
+auto ClausesOfKind = getClausesOfKind(Clauses);
 
-if (Clauses.begin() != Clauses.end()) {
-  assert(std::next(Clauses.begin()) == Clauses.end() &&
+if (ClausesOfKind.begin() != ClausesOfKind.end()) {
+  assert(std::next(ClausesOfKind.begin()) == ClausesOfKind.end() &&
  "There are at least 2 clauses of the specified kind");
-  return *Clauses.begin();
+  return *ClausesOfKind.begin();
 }
 return nullptr;
   }
 
+  template 
+  const SpecificClause *getSingleClause() const {
+return getSingleClause(clauses());
+  }
+
   /// Returns true if the current directive has one or more clauses of a
   /// specific kind.
   template 


Index: clang/include/clang/AST/StmtOpenMP.h
===
--- clang/include/clang/AST/StmtOpenMP.h
+++ clang/include/clang/AST/StmtOpenMP.h
@@ -460,17 +460,22 @@
   /// directive). Returns nullptr if no clause of this kind is associated with
   /// the directive.
   template 
-  const SpecificClause *getSingleClause() const {
-auto Clauses = getClausesOfKind();
+  static const SpecificClause *getSingleClause(ArrayRef Clauses) {
+auto ClausesOfKind = getClausesOfKind(Clauses);
 
-if (Clauses.begin() != Clauses.end()) {
-  assert(std::next(Clauses.begin()) == Clauses.end() &&
+if (ClausesOfKind.begin() != ClausesOfKind.end()) {
+  assert(std::next(ClausesOfKind.begin()) == ClausesOfKind.end() &&
  "There are at least 2 clauses of the specified kind");
-  return *Clauses.begin();
+  return *ClausesOfKind.begin();
 }
 return nullptr;
   }
 
+  template 
+  const SpecificClause *getSingleClause() const {
+return getSingleClause(clauses());
+  }
+
   /// Returns true if the current directive has one or more clauses of a
   /// specific kind.
   template 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103750: [analyzer][WIP] Handle std::make_unique for SmartPtrModeling

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD added a comment.

The drawback of the current approach is that we are not using the following 
piece of information:

> std::unique_ptr created from std::make_unique is **not** null (to begin with)

I am not being able to use this info since I don't have access to the raw 
pointer, so cannot create a `SVal` and then constrain the `SVal` to non-null.
Any suggestions @NoQ, @vsavchenko , @xazax.hun, @teemperor?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103750

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

[clang] c41a8fb - [Clang][OpenMP] Refactor checking for mutually exclusive clauses. NFC.

[Michael Kruse via cfe-commits]

Author: Michael Kruse
Date: 2021-06-06T09:49:46-05:00
New Revision: c41a8fbfbb096995367947e5ef7d36501b04d493

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

LOG: [Clang][OpenMP] Refactor checking for mutually exclusive clauses. NFC.

Multiple clauses are mutually exclusive. This patch refactors the functions 
that check for pairs of mutually exclusive clauses into a generalized function 
which also also accepts a list of clause types if which at most one can appear.

NFC patch extracted out of D99459 by request.

Reviewed By: ABataev

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

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 514b3b9ed05fe..aae84bf3b35c3 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -10121,14 +10121,14 @@ 
Sema::ActOnOpenMPParallelSectionsDirective(ArrayRef Clauses,
   DSAStack->getTaskgroupReductionRef(), DSAStack->isCancelRegion());
 }
 
-/// detach and mergeable clauses are mutially exclusive, check for it.
-static bool checkDetachMergeableClauses(Sema &S,
-ArrayRef Clauses) {
+/// Find and diagnose mutually exclusive clause kinds.
+static bool checkMutuallyExclusiveClauses(
+Sema &S, ArrayRef Clauses,
+ArrayRef MutuallyExclusiveClauses) {
   const OMPClause *PrevClause = nullptr;
   bool ErrorFound = false;
   for (const OMPClause *C : Clauses) {
-if (C->getClauseKind() == OMPC_detach ||
-C->getClauseKind() == OMPC_mergeable) {
+if (llvm::is_contained(MutuallyExclusiveClauses, C->getClauseKind())) {
   if (!PrevClause) {
 PrevClause = C;
   } else if (PrevClause->getClauseKind() != C->getClauseKind()) {
@@ -10153,7 +10153,8 @@ StmtResult 
Sema::ActOnOpenMPTaskDirective(ArrayRef Clauses,
   // OpenMP 5.0, 2.10.1 task Construct
   // If a detach clause appears on the directive, then a mergeable clause 
cannot
   // appear on the same directive.
-  if (checkDetachMergeableClauses(*this, Clauses))
+  if (checkMutuallyExclusiveClauses(*this, Clauses,
+{OMPC_detach, OMPC_mergeable}))
 return StmtError();
 
   auto *CS = cast(AStmt);
@@ -11457,28 +11458,6 @@ StmtResult 
Sema::ActOnOpenMPCancelDirective(ArrayRef Clauses,
 CancelRegion);
 }
 
-static bool checkGrainsizeNumTasksClauses(Sema &S,
-  ArrayRef Clauses) {
-  const OMPClause *PrevClause = nullptr;
-  bool ErrorFound = false;
-  for (const OMPClause *C : Clauses) {
-if (C->getClauseKind() == OMPC_grainsize ||
-C->getClauseKind() == OMPC_num_tasks) {
-  if (!PrevClause)
-PrevClause = C;
-  else if (PrevClause->getClauseKind() != C->getClauseKind()) {
-S.Diag(C->getBeginLoc(), diag::err_omp_clauses_mutually_exclusive)
-<< getOpenMPClauseName(C->getClauseKind())
-<< getOpenMPClauseName(PrevClause->getClauseKind());
-S.Diag(PrevClause->getBeginLoc(), diag::note_omp_previous_clause)
-<< getOpenMPClauseName(PrevClause->getClauseKind());
-ErrorFound = true;
-  }
-}
-  }
-  return ErrorFound;
-}
-
 static bool checkReductionClauseWithNogroup(Sema &S,
 ArrayRef Clauses) {
   const OMPClause *ReductionClause = nullptr;
@@ -11529,7 +11508,8 @@ StmtResult Sema::ActOnOpenMPTaskLoopDirective(
   // OpenMP, [2.9.2 taskloop Construct, Restrictions]
   // The grainsize clause and num_tasks clause are mutually exclusive and may
   // not appear on the same taskloop directive.
-  if (checkGrainsizeNumTasksClauses(*this, Clauses))
+  if (checkMutuallyExclusiveClauses(*this, Clauses,
+{OMPC_grainsize, OMPC_num_tasks}))
 return StmtError();
   // OpenMP, [2.9.2 taskloop Construct, Restrictions]
   // If a reduction clause is present on the taskloop directive, the nogroup
@@ -11577,7 +11557,8 @@ StmtResult Sema::ActOnOpenMPTaskLoopSimdDirective(
   // OpenMP, [2.9.2 taskloop Construct, Restrictions]
   // The grainsize clause and num_tasks clause are mutually exclusive and may
   // not appear on the same taskloop directive.
-  if (checkGrainsizeNumTasksClauses(*this, Clauses))
+  if (checkMutuallyExclusiveClauses(*this, Clauses,
+{OMPC_grainsize, OMPC_num_tasks}))
 return StmtError();
   // OpenMP, [2.9.2 taskloop Construct, Restrictions]
   // If a reduction clause is present on the taskloop directive, the nogroup
@@ -11615,7 +11596,8 @@ StmtResult Sema::ActOnOpenMPMasterTaskLoopDirective(
   // OpenMP, [2.9.2 taskloop Construct, Restrictions]
   // The grainsize claus

[PATCH] D103666: [Clang][OpenMP] Refactor checking for mutually exclusive clauses. NFC.

[Michael Kruse via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc41a8fbfbb09: [Clang][OpenMP] Refactor checking for mutually 
exclusive clauses. NFC. (authored by Meinersbur).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103666

Files:
  clang/lib/Sema/SemaOpenMP.cpp

Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -10121,14 +10121,14 @@
   DSAStack->getTaskgroupReductionRef(), DSAStack->isCancelRegion());
 }
 
-/// detach and mergeable clauses are mutially exclusive, check for it.
-static bool checkDetachMergeableClauses(Sema &S,
-ArrayRef Clauses) {
+/// Find and diagnose mutually exclusive clause kinds.
+static bool checkMutuallyExclusiveClauses(
+Sema &S, ArrayRef Clauses,
+ArrayRef MutuallyExclusiveClauses) {
   const OMPClause *PrevClause = nullptr;
   bool ErrorFound = false;
   for (const OMPClause *C : Clauses) {
-if (C->getClauseKind() == OMPC_detach ||
-C->getClauseKind() == OMPC_mergeable) {
+if (llvm::is_contained(MutuallyExclusiveClauses, C->getClauseKind())) {
   if (!PrevClause) {
 PrevClause = C;
   } else if (PrevClause->getClauseKind() != C->getClauseKind()) {
@@ -10153,7 +10153,8 @@
   // OpenMP 5.0, 2.10.1 task Construct
   // If a detach clause appears on the directive, then a mergeable clause cannot
   // appear on the same directive.
-  if (checkDetachMergeableClauses(*this, Clauses))
+  if (checkMutuallyExclusiveClauses(*this, Clauses,
+{OMPC_detach, OMPC_mergeable}))
 return StmtError();
 
   auto *CS = cast(AStmt);
@@ -11457,28 +11458,6 @@
 CancelRegion);
 }
 
-static bool checkGrainsizeNumTasksClauses(Sema &S,
-  ArrayRef Clauses) {
-  const OMPClause *PrevClause = nullptr;
-  bool ErrorFound = false;
-  for (const OMPClause *C : Clauses) {
-if (C->getClauseKind() == OMPC_grainsize ||
-C->getClauseKind() == OMPC_num_tasks) {
-  if (!PrevClause)
-PrevClause = C;
-  else if (PrevClause->getClauseKind() != C->getClauseKind()) {
-S.Diag(C->getBeginLoc(), diag::err_omp_clauses_mutually_exclusive)
-<< getOpenMPClauseName(C->getClauseKind())
-<< getOpenMPClauseName(PrevClause->getClauseKind());
-S.Diag(PrevClause->getBeginLoc(), diag::note_omp_previous_clause)
-<< getOpenMPClauseName(PrevClause->getClauseKind());
-ErrorFound = true;
-  }
-}
-  }
-  return ErrorFound;
-}
-
 static bool checkReductionClauseWithNogroup(Sema &S,
 ArrayRef Clauses) {
   const OMPClause *ReductionClause = nullptr;
@@ -11529,7 +11508,8 @@
   // OpenMP, [2.9.2 taskloop Construct, Restrictions]
   // The grainsize clause and num_tasks clause are mutually exclusive and may
   // not appear on the same taskloop directive.
-  if (checkGrainsizeNumTasksClauses(*this, Clauses))
+  if (checkMutuallyExclusiveClauses(*this, Clauses,
+{OMPC_grainsize, OMPC_num_tasks}))
 return StmtError();
   // OpenMP, [2.9.2 taskloop Construct, Restrictions]
   // If a reduction clause is present on the taskloop directive, the nogroup
@@ -11577,7 +11557,8 @@
   // OpenMP, [2.9.2 taskloop Construct, Restrictions]
   // The grainsize clause and num_tasks clause are mutually exclusive and may
   // not appear on the same taskloop directive.
-  if (checkGrainsizeNumTasksClauses(*this, Clauses))
+  if (checkMutuallyExclusiveClauses(*this, Clauses,
+{OMPC_grainsize, OMPC_num_tasks}))
 return StmtError();
   // OpenMP, [2.9.2 taskloop Construct, Restrictions]
   // If a reduction clause is present on the taskloop directive, the nogroup
@@ -11615,7 +11596,8 @@
   // OpenMP, [2.9.2 taskloop Construct, Restrictions]
   // The grainsize clause and num_tasks clause are mutually exclusive and may
   // not appear on the same taskloop directive.
-  if (checkGrainsizeNumTasksClauses(*this, Clauses))
+  if (checkMutuallyExclusiveClauses(*this, Clauses,
+{OMPC_grainsize, OMPC_num_tasks}))
 return StmtError();
   // OpenMP, [2.9.2 taskloop Construct, Restrictions]
   // If a reduction clause is present on the taskloop directive, the nogroup
@@ -11663,7 +11645,8 @@
   // OpenMP, [2.9.2 taskloop Construct, Restrictions]
   // The grainsize clause and num_tasks clause are mutually exclusive and may
   // not appear on the same taskloop directive.
-  if (checkGrainsizeNumTasksClauses(*this, Clauses))
+  if (checkMutuallyExclusiveClauses(*this, Clauses,
+{OMPC_grainsize, OMPC

[PATCH] D103750: [analyzer][WIP] Handle std::make_unique for SmartPtrModeling

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

xazax.hun added a comment.



> I am not being able to use this info since I don't have access to the raw 
> pointer, so cannot create a `SVal` and then constrain the `SVal` to non-null.
> Any suggestions @NoQ, @vsavchenko , @xazax.hun, @teemperor?

You can always create a new symbol to represent the inner pointer. Something 
like this already happens, when you have a unique_ptr formal parameter and call 
get on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103750

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

[PATCH] D103750: [analyzer][WIP] Handle std::make_unique for SmartPtrModeling

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD added a comment.

In D103750#2801248 , @xazax.hun wrote:

> You can always create a new symbol to represent the inner pointer. Something 
> like this already happens, when you have a unique_ptr formal parameter and 
> call get on it.

The way `handleGet` obtains a new symbol cannot really be used here since we do 
not have an `Expr` for the inner pointer at hand. `handleGet` does the 
following:

  C.getSValBuilder().conjureSymbolVal(
  CallExpr, C.getLocationContext(), Call.getResultType(), 
C.blockCount());

Since we have `CallExpr`, we can easily conjure up an `SVal`. But I don't see 
how I can do it similarly in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103750

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

[PATCH] D103750: [analyzer][WIP] Handle std::make_unique for SmartPtrModeling

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD updated this revision to Diff 350109.
RedDocMD added a comment.
Herald added a reviewer: bollu.

Reformatted code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103750

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  polly/include/polly/CodeGen/IslAst.h
  polly/include/polly/CodeGen/IslNodeBuilder.h
  polly/lib/CodeGen/IslAst.cpp
  polly/lib/CodeGen/IslNodeBuilder.cpp

Index: polly/lib/CodeGen/IslNodeBuilder.cpp
===
--- polly/lib/CodeGen/IslNodeBuilder.cpp
+++ polly/lib/CodeGen/IslNodeBuilder.cpp
@@ -300,12 +300,12 @@
 addReferencesFromStmtSet(Set, &References);
 }
 
-__isl_give isl_union_map *
-IslNodeBuilder::getScheduleForAstNode(__isl_keep isl_ast_node *For) {
-  return IslAstInfo::getSchedule(For);
+isl::union_map
+IslNodeBuilder::getScheduleForAstNode(const isl::ast_node &Node) {
+  return IslAstInfo::getSchedule(Node);
 }
 
-void IslNodeBuilder::getReferencesInSubtree(__isl_keep isl_ast_node *For,
+void IslNodeBuilder::getReferencesInSubtree(const isl::ast_node &For,
 SetVector &Values,
 SetVector &Loops) {
   SetVector SCEVs;
@@ -319,8 +319,7 @@
   for (const auto &I : OutsideLoopIterations)
 Values.insert(cast(I.second)->getValue());
 
-  isl::union_set Schedule =
-  isl::manage(isl_union_map_domain(getScheduleForAstNode(For)));
+  isl::union_set Schedule = getScheduleForAstNode(For).domain();
   addReferencesFromStmtUnionSet(Schedule, References);
 
   for (const SCEV *Expr : SCEVs) {
@@ -476,22 +475,22 @@
   for (int i = 1; i < VectorWidth; i++)
 IVS[i] = Builder.CreateAdd(IVS[i - 1], ValueInc, "p_vector_iv");
 
-  isl_union_map *Schedule = getScheduleForAstNode(For);
-  assert(Schedule && "For statement annotation does not contain its schedule");
+  isl::union_map Schedule = getScheduleForAstNode(isl::manage_copy(For));
+  assert(!Schedule.is_null() &&
+ "For statement annotation does not contain its schedule");
 
   IDToValue[IteratorID] = ValueLB;
 
   switch (isl_ast_node_get_type(Body)) {
   case isl_ast_node_user:
-createUserVector(Body, IVS, isl_id_copy(IteratorID),
- isl_union_map_copy(Schedule));
+createUserVector(Body, IVS, isl_id_copy(IteratorID), Schedule.copy());
 break;
   case isl_ast_node_block: {
 isl_ast_node_list *List = isl_ast_node_block_get_children(Body);
 
 for (int i = 0; i < isl_ast_node_list_n_ast_node(List); ++i)
   createUserVector(isl_ast_node_list_get_ast_node(List, i), IVS,
-   isl_id_copy(IteratorID), isl_union_map_copy(Schedule));
+   isl_id_copy(IteratorID), Schedule.copy());
 
 isl_ast_node_free(Body);
 isl_ast_node_list_free(List);
@@ -504,7 +503,6 @@
 
   IDToValue.erase(IDToValue.find(IteratorID));
   isl_id_free(IteratorID);
-  isl_union_map_free(Schedule);
 
   isl_ast_node_free(For);
   isl_ast_expr_free(Iterator);
@@ -685,7 +683,7 @@
   SetVector SubtreeValues;
   SetVector Loops;
 
-  getReferencesInSubtree(For, SubtreeValues, Loops);
+  getReferencesInSubtree(isl::manage_copy(For), SubtreeValues, Loops);
 
   // Create for all loops we depend on values that contain the current loop
   // iteration. These values are necessary to generate code for SCEVs that
@@ -783,7 +781,7 @@
   bool Vector = PollyVectorizerChoice == VECTORIZER_POLLY;
 
   if (Vector && IslAstInfo::isInnermostParallel(isl::manage_copy(For)) &&
-  !IslAstInfo::isReductionParallel(For)) {
+  !IslAstInfo::isReductionParallel(isl::manage_copy(For))) {
 int VectorWidth = getNumberOfIterations(isl::manage_copy(For));
 if (1 < VectorWidth && VectorWidth <= 16 && !hasPartialAccesses(For)) {
   createForVector(For, VectorWidth);
@@ -791,12 +789,12 @@
 }
   }
 
-  if (IslAstInfo::isExecutedInParallel(For)) {
+  if (IslAstInfo::isExecutedInParallel(isl::manage_copy(For))) {
 createForParallel(For);
 return;
   }
-  bool Parallel =
-  (IslAstInfo::isParallel(For) && !IslAstInfo::isReductionParallel(For));
+  bool Parallel = (IslAstInfo::isParallel(isl::manage_copy(For)) &&
+   !IslAstInfo::isReductionParallel(isl::manage_copy(For)));
   createForSequential(isl::manage(For), Parallel);
 }
 
Index: polly/lib/CodeGen/IslAst.cpp
===
--- polly/lib/CodeGen/IslAst.cpp
+++ polly/lib/CodeGen/IslAst.cpp
@@ -140,7 +140,7 @@
 }
 
 /// Return all broken reductions as a string of clauses (OpenMP style).
-static const std::string getBrokenReductionsStr(__isl_keep isl_ast_node *Node) {
+static const std::string getBrokenReductionsStr(const isl::ast_node &Node) {
   IslAstInfo::MemoryAccessSet *BrokenReductions;
   std::string str;
 
@@ -171,25 +171,26 @@
 static isl_printer *cbPrintFor(__isl_take isl_printer *Printer,
 

[PATCH] D103750: [analyzer][WIP] Handle std::make_unique for SmartPtrModeling

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD updated this revision to Diff 350110.
RedDocMD added a comment.

Fixed git history


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103750

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp

Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -35,11 +35,19 @@
 using namespace ento;
 
 namespace {
+
+enum class MakeUniqueKind {
+  Regular,  // ie, std::make_unique
+  ForOverwrite, // ie, std::make_unique_for_overwrite
+  None  // ie, is neither of the above two
+};
+
 class SmartPtrModeling
 : public Checker {
+ check::LiveSymbols, check::Bind> {
 
   bool isBoolConversionMethod(const CallEvent &Call) const;
+  MakeUniqueKind isStdMakeUniqueCall(const CallEvent &Call) const;
 
 public:
   // Whether the checker should model for null dereferences of smart pointers.
@@ -56,6 +64,7 @@
   void printState(raw_ostream &Out, ProgramStateRef State, const char *NL,
   const char *Sep) const override;
   void checkLiveSymbols(ProgramStateRef State, SymbolReaper &SR) const;
+  void checkBind(SVal L, SVal V, const Stmt *S, CheckerContext &C) const;
 
 private:
   void handleReset(const CallEvent &Call, CheckerContext &C) const;
@@ -68,6 +77,8 @@
   bool updateMovedSmartPointers(CheckerContext &C, const MemRegion *ThisRegion,
 const MemRegion *OtherSmartPtrRegion) const;
   void handleBoolConversion(const CallEvent &Call, CheckerContext &C) const;
+  void handleStdMakeUnique(const CallEvent &Call, CheckerContext &C,
+   MakeUniqueKind Kind) const;
 
   using SmartPtrMethodHandlerFn =
   void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
@@ -79,7 +90,21 @@
 };
 } // end of anonymous namespace
 
+class MakeUniqueKindWrapper {
+  const MakeUniqueKind Kind;
+
+public:
+  MakeUniqueKindWrapper(MakeUniqueKind Kind) : Kind(Kind) {}
+  MakeUniqueKind get() const { return Kind; }
+  void Profile(llvm::FoldingSetNodeID &ID) const {
+ID.AddInteger(static_cast(Kind));
+  }
+  bool operator==(const MakeUniqueKind &RHS) const { return Kind == RHS; }
+  bool operator!=(const MakeUniqueKind &RHS) const { return Kind != RHS; }
+};
+
 REGISTER_MAP_WITH_PROGRAMSTATE(TrackedRegionMap, const MemRegion *, SVal)
+REGISTER_LIST_WITH_PROGRAMSTATE(MakeUniqueKindList, MakeUniqueKindWrapper)
 
 // Define the inter-checker API.
 namespace clang {
@@ -175,8 +200,35 @@
   return CD && CD->getConversionType()->isBooleanType();
 }
 
+MakeUniqueKind
+SmartPtrModeling::isStdMakeUniqueCall(const CallEvent &Call) const {
+  if (Call.getKind() != CallEventKind::CE_Function)
+return MakeUniqueKind::None;
+  const auto *D = Call.getDecl();
+  if (!D)
+return MakeUniqueKind::None;
+  const auto *FTD = llvm::dyn_cast(D);
+  if (!FTD)
+return MakeUniqueKind::None;
+  if (FTD->getDeclName().isIdentifier()) {
+StringRef Name = FTD->getName();
+if (Name == "make_unique")
+  return MakeUniqueKind::Regular;
+else if (Name == "make_unique_for_overwrite")
+  return MakeUniqueKind::ForOverwrite;
+  }
+  return MakeUniqueKind::None;
+}
+
 bool SmartPtrModeling::evalCall(const CallEvent &Call,
 CheckerContext &C) const {
+
+  MakeUniqueKind Kind = isStdMakeUniqueCall(Call);
+  if (Kind != MakeUniqueKind::None) {
+handleStdMakeUnique(Call, C, Kind);
+return true;
+  }
+
   ProgramStateRef State = C.getState();
   if (!smartptr::isStdSmartPtrCall(Call))
 return false;
@@ -272,6 +324,59 @@
   return C.isDifferent();
 }
 
+void SmartPtrModeling::handleStdMakeUnique(const CallEvent &Call,
+   CheckerContext &C,
+   MakeUniqueKind Kind) const {
+  assert(Kind != MakeUniqueKind::None &&
+ "Call is not to make_unique or make_unique_for_overwrite");
+  ProgramStateRef State = C.getState();
+  State = State->add(Kind);
+  C.addTransition(State);
+}
+
+bool isUniquePtrType(QualType QT) {
+  const auto *T = QT.getTypePtr();
+  if (!T)
+return false;
+  const auto *Decl = T->getAsCXXRecordDecl();
+  if (!Decl || !Decl->getDeclContext()->isStdNamespace())
+return false;
+  const IdentifierInfo *ID = Decl->getIdentifier();
+  if (!ID)
+return false;
+  const StringRef Name = ID->getName();
+  return Name == "unique_ptr";
+}
+
+void SmartPtrModeling::checkBind(SVal L, SVal V, const Stmt *S,
+ CheckerContext &C) const {
+  const auto *TVR = dyn_cast_or_null(L.getAsRegion());
+  if (!TVR)
+return;
+  if (!isUniquePtrType(TVR->getValueType()))
+return;
+  const auto *ThisRegion = dyn_cast(TVR);
+  if (!ThisRegion)
+return;
+
+  ProgramS

[PATCH] D103750: [analyzer][WIP] Handle std::make_unique for SmartPtrModeling

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

xazax.hun added a comment.

> Since we have CallExpr, we can easily conjure up an SVal. But I don't see how 
> I can do it similarly in this patch.

You should have a `CallExpr` for `std::make_unique` too. I believe that 
expression is used to determine how the conjured symbol was created (to give it 
an identity). So probably it should be ok to use the `make_unique` call to 
create this symbol (but be careful to create the symbol with the right type).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103750

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

[PATCH] D103771: [clang][msvc] Define _HAS_STATIC_RTTI to 0, when compiling with -fno-rtti

[Markus Böck via Phabricator via cfe-commits]

zero9178 created this revision.
zero9178 added reviewers: rnk, thakis, hans.
zero9178 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When using the -fno-rtti option of the GCC style clang++, using typeid results 
in an error. The MSVC STL however kindly provides a define flag called 
_HAS_STATIC_RTTI, which either enables or disables uses of typeid throughout 
the STL. By default, if undefined, it is set to 1, enabling the use of typeid.

With this patch, _HAS_STATIC_RTTI is set to 0 when -fno-rtti is specified. This 
way various headers of the MSVC STL like functional can be consumed without 
compilation failures.



Some context: I was first made aware of this define in this bug report 
regarding twoPhase lookup: 
https://bugs.chromium.org/p/chromium/issues/detail?id=996675
Back then however there was still a usage of typeid left unguarded, which has 
since been fixed in this commit https://github.com/microsoft/STL/issues/340. 
Since that is November 2019, so a rather recent version of MSVC, this will not 
work in versions prior to this fix. 
For compat one could maybe allow `typeid(void)`, but that is beyond the scope 
of this patch either way I think.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103771

Files:
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Driver/ToolChains/MSVC.h
  clang/test/Driver/msvc-static-rtti.cpp


Index: clang/test/Driver/msvc-static-rtti.cpp
===
--- /dev/null
+++ clang/test/Driver/msvc-static-rtti.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang -target x86_64-pc-windows-msvc -fno-rtti -### %s 2>&1 | 
FileCheck %s -check-prefix STATIC-RTTI-DEF
+// RUN: %clang -target x86_64-pc-windows-msvc -frtti -### %s 2>&1 | FileCheck 
%s -check-prefix STATIC-RTTI-DEF-NOT
+
+// STATIC-RTTI-DEF: -D_HAS_STATIC_RTTI=0
+// STATIC-RTTI-DEF-NOT: -D_HAS_STATIC_RTTI=0
Index: clang/lib/Driver/ToolChains/MSVC.h
===
--- clang/lib/Driver/ToolChains/MSVC.h
+++ clang/lib/Driver/ToolChains/MSVC.h
@@ -122,6 +122,11 @@
 
   bool FoundMSVCInstall() const { return !VCToolChainPath.empty(); }
 
+  void
+  addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const override;
+
 protected:
   void AddSystemIncludeWithSubfolder(const llvm::opt::ArgList &DriverArgs,
  llvm::opt::ArgStringList &CC1Args,
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -1555,3 +1555,11 @@
 
   return DAL;
 }
+
+void MSVCToolChain::addClangTargetOptions(
+const ArgList &DriverArgs, ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const {
+  if (DriverArgs.hasArg(options::OPT_fno_rtti, options::OPT_frtti,
+/*Default=*/false))
+CC1Args.push_back("-D_HAS_STATIC_RTTI=0");
+}


Index: clang/test/Driver/msvc-static-rtti.cpp
===
--- /dev/null
+++ clang/test/Driver/msvc-static-rtti.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang -target x86_64-pc-windows-msvc -fno-rtti -### %s 2>&1 | FileCheck %s -check-prefix STATIC-RTTI-DEF
+// RUN: %clang -target x86_64-pc-windows-msvc -frtti -### %s 2>&1 | FileCheck %s -check-prefix STATIC-RTTI-DEF-NOT
+
+// STATIC-RTTI-DEF: -D_HAS_STATIC_RTTI=0
+// STATIC-RTTI-DEF-NOT: -D_HAS_STATIC_RTTI=0
Index: clang/lib/Driver/ToolChains/MSVC.h
===
--- clang/lib/Driver/ToolChains/MSVC.h
+++ clang/lib/Driver/ToolChains/MSVC.h
@@ -122,6 +122,11 @@
 
   bool FoundMSVCInstall() const { return !VCToolChainPath.empty(); }
 
+  void
+  addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const override;
+
 protected:
   void AddSystemIncludeWithSubfolder(const llvm::opt::ArgList &DriverArgs,
  llvm::opt::ArgStringList &CC1Args,
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -1555,3 +1555,11 @@
 
   return DAL;
 }
+
+void MSVCToolChain::addClangTargetOptions(
+const ArgList &DriverArgs, ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const {
+  if (DriverArgs.hasArg(options::OPT_fno_rtti, options::OPT_frtti,
+/*Default=*/false))
+CC1Args.push_back("-D_HAS_STATIC_RTTI=0");
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.ll

[PATCH] D103772: [clang-cl] Reenable /Zc:twoPhase by default if targetting MSVC 2017 Update 3 or newer

[Markus Böck via Phabricator via cfe-commits]

zero9178 created this revision.
zero9178 added reviewers: rnk, hans, thakis.
Herald added a subscriber: dang.
zero9178 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch effectively relands https://reviews.llvm.org/D66394, which back then 
sadly had to be reverted due to build failures. The summary given in the commit 
is already great, but just to summarize: Since MSVC 2017 Update 3 (or 19.11, so 
beyond the 19.14 minimum that clang-cl supports), two phase template parsing 
had been enabled by default. This patch enables two phase lookup by default in 
clang-cl as well, when the MSVC compatibility version is 19.11 or higher.

The patch previously had to be reverted due to issues when executed with the 
GCC style driver and -fno-rtti as can be seen here 
https://bugs.chromium.org/p/chromium/issues/detail?id=996675. 
https://reviews.llvm.org/D103771, which this patch depends on, implements one 
of the resolutions given in the report, by defining _HAS_STATIC_RTTI. For older 
MSVC STL versions that had a missing guard in the functional header, 
-fno-rtti-data may have to passed instead of -fno-rtti, as soon as 
std::function is instantiated, regardless of whether two phase lookup is 
enabled or not.

Depends on https://reviews.llvm.org/D103771


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103772

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.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
@@ -329,13 +329,19 @@
 
 // We recognize -f[no-]delayed-template-parsing.
 // /Zc:twoPhase[-] has the opposite meaning.
-// RUN: %clang_cl -c -### -- %s 2>&1 | FileCheck -check-prefix=DELAYEDDEFAULT 
%s
-// DELAYEDDEFAULT: "-fdelayed-template-parsing"
-// RUN: %clang_cl -c -fdelayed-template-parsing -### -- %s 2>&1 | FileCheck 
-check-prefix=DELAYEDON %s
-// RUN: %clang_cl -c /Zc:twoPhase- -### -- %s 2>&1 | FileCheck 
-check-prefix=DELAYEDON %s
+// RUN: %clang_cl -c -fmsc-version=1910 -### -- %s 2>&1 | FileCheck 
-check-prefix=OLDDELAYEDDEFAULT %s
+// OLDDELAYEDDEFAULT: "-fdelayed-template-parsing"
+// RUN: %clang_cl -c -fmsc-version=1911 -### -- %s 2>&1 | FileCheck 
-check-prefix=NEWDELAYEDDEFAULT %s
+// NEWDELAYEDDEFAULT-NOT: "-fdelayed-template-parsing"
+// RUN: %clang_cl -c -fmsc-version=1910 -fdelayed-template-parsing -### -- %s 
2>&1 | FileCheck -check-prefix=DELAYEDON %s
+// RUN: %clang_cl -c -fmsc-version=1911 -fdelayed-template-parsing -### -- %s 
2>&1 | FileCheck -check-prefix=DELAYEDON %s
+// RUN: %clang_cl -c -fmsc-version=1910 /Zc:twoPhase- -### -- %s 2>&1 | 
FileCheck -check-prefix=DELAYEDON %s
+// RUN: %clang_cl -c -fmsc-version=1911 /Zc:twoPhase- -### -- %s 2>&1 | 
FileCheck -check-prefix=DELAYEDON %s
 // DELAYEDON: "-fdelayed-template-parsing"
-// RUN: %clang_cl -c -fno-delayed-template-parsing -### -- %s 2>&1 | FileCheck 
-check-prefix=DELAYEDOFF %s
-// RUN: %clang_cl -c /Zc:twoPhase -### -- %s 2>&1 | FileCheck 
-check-prefix=DELAYEDOFF %s
+// RUN: %clang_cl -c -fmsc-version=1910 -fno-delayed-template-parsing -### -- 
%s 2>&1 | FileCheck -check-prefix=DELAYEDOFF %s
+// RUN: %clang_cl -c -fmsc-version=1911 -fno-delayed-template-parsing -### -- 
%s 2>&1 | FileCheck -check-prefix=DELAYEDOFF %s
+// RUN: %clang_cl -c -fmsc-version=1910 /Zc:twoPhase -### -- %s 2>&1 | 
FileCheck -check-prefix=DELAYEDOFF %s
+// RUN: %clang_cl -c -fmsc-version=1911 /Zc:twoPhase -### -- %s 2>&1 | 
FileCheck -check-prefix=DELAYEDOFF %s
 // DELAYEDOFF-NOT: "-fdelayed-template-parsing"
 
 // RUN: %clang_cl -c -### /std:c++latest -- %s 2>&1 | FileCheck -check-prefix 
CHECK-LATEST-CHAR8_T %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6127,12 +6127,14 @@
 !IsWindowsMSVC || IsMSVC2015Compatible))
 CmdArgs.push_back("-fno-threadsafe-statics");
 
-  // -fno-delayed-template-parsing is default, except when targeting MSVC.
-  // Many old Windows SDK versions require this to parse.
-  // FIXME: MSVC introduced /Zc:twoPhase- to disable this behavior in their
-  // compiler. We should be able to disable this by default at some point.
+  // -fno-delayed-template-parsing is default, except when targeting MSVC
+  // earlier than MSVC 2017 15.3 (_MSC_VER 1911). Windows SDK versions
+  // 10.0.15063.0 (Creators Update or Redstone 2) and earlier require this to
+  // parse.
+  bool IsMSVCBefore2017Update3 = !MSVT.empty() && MSVT < VersionTuple(19, 11);
   if (Args.hasFlag(options::OPT_fdelayed_template_parsing,
-   options::OPT_fno_delayed_template_parsing, IsWindowsMSVC))
+   options::OPT_fno_delayed_template_parsing,
+   

[PATCH] D103771: [clang][msvc] Define _HAS_STATIC_RTTI to 0, when compiling with -fno-rtti

[Markus Böck via Phabricator via cfe-commits]

zero9178 updated this revision to Diff 350123.
zero9178 added a comment.

Rebase & add comment in source code explaining the purpose of the define


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

https://reviews.llvm.org/D103771

Files:
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Driver/ToolChains/MSVC.h
  clang/test/Driver/msvc-static-rtti.cpp


Index: clang/test/Driver/msvc-static-rtti.cpp
===
--- /dev/null
+++ clang/test/Driver/msvc-static-rtti.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang -target x86_64-pc-windows-msvc -fno-rtti -### %s 2>&1 | 
FileCheck %s -check-prefix STATIC-RTTI-DEF
+// RUN: %clang -target x86_64-pc-windows-msvc -frtti -### %s 2>&1 | FileCheck 
%s -check-prefix STATIC-RTTI-DEF-NOT
+
+// STATIC-RTTI-DEF: -D_HAS_STATIC_RTTI=0
+// STATIC-RTTI-DEF-NOT: -D_HAS_STATIC_RTTI=0
Index: clang/lib/Driver/ToolChains/MSVC.h
===
--- clang/lib/Driver/ToolChains/MSVC.h
+++ clang/lib/Driver/ToolChains/MSVC.h
@@ -122,6 +122,11 @@
 
   bool FoundMSVCInstall() const { return !VCToolChainPath.empty(); }
 
+  void
+  addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const override;
+
 protected:
   void AddSystemIncludeWithSubfolder(const llvm::opt::ArgList &DriverArgs,
  llvm::opt::ArgStringList &CC1Args,
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -1539,3 +1539,13 @@
 
   return DAL;
 }
+
+void MSVCToolChain::addClangTargetOptions(
+const ArgList &DriverArgs, ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const {
+  // MSVC STL kindly allows removing all usages of typeid by defining
+  // _HAS_STATIC_RTTI to 0. Do so, when compiling with -fno-rtti
+  if (DriverArgs.hasArg(options::OPT_fno_rtti, options::OPT_frtti,
+/*Default=*/false))
+CC1Args.push_back("-D_HAS_STATIC_RTTI=0");
+}


Index: clang/test/Driver/msvc-static-rtti.cpp
===
--- /dev/null
+++ clang/test/Driver/msvc-static-rtti.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang -target x86_64-pc-windows-msvc -fno-rtti -### %s 2>&1 | FileCheck %s -check-prefix STATIC-RTTI-DEF
+// RUN: %clang -target x86_64-pc-windows-msvc -frtti -### %s 2>&1 | FileCheck %s -check-prefix STATIC-RTTI-DEF-NOT
+
+// STATIC-RTTI-DEF: -D_HAS_STATIC_RTTI=0
+// STATIC-RTTI-DEF-NOT: -D_HAS_STATIC_RTTI=0
Index: clang/lib/Driver/ToolChains/MSVC.h
===
--- clang/lib/Driver/ToolChains/MSVC.h
+++ clang/lib/Driver/ToolChains/MSVC.h
@@ -122,6 +122,11 @@
 
   bool FoundMSVCInstall() const { return !VCToolChainPath.empty(); }
 
+  void
+  addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const override;
+
 protected:
   void AddSystemIncludeWithSubfolder(const llvm::opt::ArgList &DriverArgs,
  llvm::opt::ArgStringList &CC1Args,
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -1539,3 +1539,13 @@
 
   return DAL;
 }
+
+void MSVCToolChain::addClangTargetOptions(
+const ArgList &DriverArgs, ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const {
+  // MSVC STL kindly allows removing all usages of typeid by defining
+  // _HAS_STATIC_RTTI to 0. Do so, when compiling with -fno-rtti
+  if (DriverArgs.hasArg(options::OPT_fno_rtti, options::OPT_frtti,
+/*Default=*/false))
+CC1Args.push_back("-D_HAS_STATIC_RTTI=0");
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103772: [clang-cl] Reenable /Zc:twoPhase by default if targetting MSVC 2017 Update 3 or newer

[Markus Böck via Phabricator via cfe-commits]

zero9178 updated this revision to Diff 350125.
zero9178 added a comment.

Rebased onto main


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

https://reviews.llvm.org/D103772

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.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
@@ -329,13 +329,19 @@
 
 // We recognize -f[no-]delayed-template-parsing.
 // /Zc:twoPhase[-] has the opposite meaning.
-// RUN: %clang_cl -c -### -- %s 2>&1 | FileCheck -check-prefix=DELAYEDDEFAULT 
%s
-// DELAYEDDEFAULT: "-fdelayed-template-parsing"
-// RUN: %clang_cl -c -fdelayed-template-parsing -### -- %s 2>&1 | FileCheck 
-check-prefix=DELAYEDON %s
-// RUN: %clang_cl -c /Zc:twoPhase- -### -- %s 2>&1 | FileCheck 
-check-prefix=DELAYEDON %s
+// RUN: %clang_cl -c -fmsc-version=1910 -### -- %s 2>&1 | FileCheck 
-check-prefix=OLDDELAYEDDEFAULT %s
+// OLDDELAYEDDEFAULT: "-fdelayed-template-parsing"
+// RUN: %clang_cl -c -fmsc-version=1911 -### -- %s 2>&1 | FileCheck 
-check-prefix=NEWDELAYEDDEFAULT %s
+// NEWDELAYEDDEFAULT-NOT: "-fdelayed-template-parsing"
+// RUN: %clang_cl -c -fmsc-version=1910 -fdelayed-template-parsing -### -- %s 
2>&1 | FileCheck -check-prefix=DELAYEDON %s
+// RUN: %clang_cl -c -fmsc-version=1911 -fdelayed-template-parsing -### -- %s 
2>&1 | FileCheck -check-prefix=DELAYEDON %s
+// RUN: %clang_cl -c -fmsc-version=1910 /Zc:twoPhase- -### -- %s 2>&1 | 
FileCheck -check-prefix=DELAYEDON %s
+// RUN: %clang_cl -c -fmsc-version=1911 /Zc:twoPhase- -### -- %s 2>&1 | 
FileCheck -check-prefix=DELAYEDON %s
 // DELAYEDON: "-fdelayed-template-parsing"
-// RUN: %clang_cl -c -fno-delayed-template-parsing -### -- %s 2>&1 | FileCheck 
-check-prefix=DELAYEDOFF %s
-// RUN: %clang_cl -c /Zc:twoPhase -### -- %s 2>&1 | FileCheck 
-check-prefix=DELAYEDOFF %s
+// RUN: %clang_cl -c -fmsc-version=1910 -fno-delayed-template-parsing -### -- 
%s 2>&1 | FileCheck -check-prefix=DELAYEDOFF %s
+// RUN: %clang_cl -c -fmsc-version=1911 -fno-delayed-template-parsing -### -- 
%s 2>&1 | FileCheck -check-prefix=DELAYEDOFF %s
+// RUN: %clang_cl -c -fmsc-version=1910 /Zc:twoPhase -### -- %s 2>&1 | 
FileCheck -check-prefix=DELAYEDOFF %s
+// RUN: %clang_cl -c -fmsc-version=1911 /Zc:twoPhase -### -- %s 2>&1 | 
FileCheck -check-prefix=DELAYEDOFF %s
 // DELAYEDOFF-NOT: "-fdelayed-template-parsing"
 
 // RUN: %clang_cl -c -### /std:c++latest -- %s 2>&1 | FileCheck -check-prefix 
CHECK-LATEST-CHAR8_T %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6124,12 +6124,14 @@
 !IsWindowsMSVC || IsMSVC2015Compatible))
 CmdArgs.push_back("-fno-threadsafe-statics");
 
-  // -fno-delayed-template-parsing is default, except when targeting MSVC.
-  // Many old Windows SDK versions require this to parse.
-  // FIXME: MSVC introduced /Zc:twoPhase- to disable this behavior in their
-  // compiler. We should be able to disable this by default at some point.
+  // -fno-delayed-template-parsing is default, except when targeting MSVC
+  // earlier than MSVC 2017 15.3 (_MSC_VER 1911). Windows SDK versions
+  // 10.0.15063.0 (Creators Update or Redstone 2) and earlier require this to
+  // parse.
+  bool IsMSVCBefore2017Update3 = !MSVT.empty() && MSVT < VersionTuple(19, 11);
   if (Args.hasFlag(options::OPT_fdelayed_template_parsing,
-   options::OPT_fno_delayed_template_parsing, IsWindowsMSVC))
+   options::OPT_fno_delayed_template_parsing,
+   IsMSVCBefore2017Update3))
 CmdArgs.push_back("-fdelayed-template-parsing");
 
   // -fgnu-keywords default varies depending on language; only pass if
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6008,10 +6008,10 @@
 def _SLASH_Zc_trigraphs_off : CLFlag<"Zc:trigraphs-">,
   HelpText<"Disable trigraphs (default)">, Alias;
 def _SLASH_Zc_twoPhase : CLFlag<"Zc:twoPhase">,
-  HelpText<"Enable two-phase name lookup in templates">,
+  HelpText<"Enable two-phase name lookup in templates (default)">,
   Alias;
 def _SLASH_Zc_twoPhase_ : CLFlag<"Zc:twoPhase-">,
-  HelpText<"Disable two-phase name lookup in templates (default)">,
+  HelpText<"Disable two-phase name lookup in templates">,
   Alias;
 def _SLASH_Z7 : CLFlag<"Z7">,
   HelpText<"Enable CodeView debug information in object files">;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -119,6 +119,10 @@
 Windows Support
 ---
 
+- clang-cl now def

[PATCH] D103773: [clang-cl] Add /permissive and /permissive-

[Markus Böck via Phabricator via cfe-commits]

zero9178 created this revision.
zero9178 added reviewers: rnk, thakis, hans, mstorsjo.
Herald added a subscriber: dang.
zero9178 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch adds the command line options /permissive and /permissive- to 
clang-cl. These flags are used in MSVC to enable various /Zc language 
conformance options at once. In particular, /permissive is used to enable the 
various non standard behaviour of MSVC, while /permissive- is the opposite.

When either of two command lines are specified they are simply expanded to the 
various underlying /Zc options. In particular when /permissive is passed it 
currently expands to:

- /Zc:twoPhase- (disable two phase lookup)
- -fno-operator-names (disable C++ operator keywords)

/permissive- expands to the opposites of these flags + /Zc:strictStrings 
(/Zc:strictStrings- does not currently exist). In the future, if any more MSVC 
workarounds are ever added they can easily be added to the expansion. One is 
also able to override settings done by permissive. Specifying /permissive- 
/Zc:twoPhase- will apply the settings from permissive minus, but disables two 
phase lookup.

Motivation for this patch was mainly parity with MSVC as well as compatibility 
with Windows SDK headers. The /permissive page from MSVC documents various 
workarounds that have to be done for the Windows SDK headers [1], when MSVC is 
used with /permissive-. In these, Microsoft often recommends simply compiling 
with /permissive for the specified source files. Since some of these also apply 
to clang-cl (which acts like /permissive- by default mostly), and some are 
currently implemented as "hacks" within clang that I'd like to remove, adding 
/permissive and /permissive- to be in full parity with MSVC and Microsofts 
documentation made sense to me.

[1] 
https://docs.microsoft.com/en-us/cpp/build/reference/permissive-standards-conformance?view=msvc-160#windows-header-issues

Depends on https://reviews.llvm.org/D103749 for the -foperator-names CLI option


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103773

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


Index: clang/test/Driver/cl-permissive.c
===
--- /dev/null
+++ clang/test/Driver/cl-permissive.c
@@ -0,0 +1,17 @@
+// Note: %s must be preceded by --, otherwise it may be interpreted as a
+// command-line option, e.g. on Mac where %s is commonly under /Users.
+
+// RUN: %clang_cl /permissive -### -- %s 2>&1 | FileCheck 
-check-prefix=PERMISSIVE %s
+// PERMISSIVE: "-fno-operator-names"
+// PERMISSIVE: "-fdelayed-template-parsing"
+// RUN: %clang_cl /permissive- -### -- %s 2>&1 | FileCheck 
-check-prefix=PERMISSIVE-MINUS %s
+// PERMISSIVE-MINUS-NOT: "-fno-operator-names"
+// PERMISSIVE-MINUS-NOT: "-fdelayed-template-parsing"
+
+// The switches set by permissive may then still be manually enabled or 
disabled
+// RUN: %clang_cl /permissive /Zc:twoPhase -### -- %s 2>&1 | FileCheck 
-check-prefix=PERMISSIVE-OVERWRITE %s
+// PERMISSIVE-OVERWRITE: "-fno-operator-names"
+// PERMISSIVE-OVERWRITE-NOT: "-fdelayed-template-parsing"
+// RUN: %clang_cl /permissive- /Zc:twoPhase- -### -- %s 2>&1 | FileCheck 
-check-prefix=PERMISSIVE-MINUS-OVERWRITE %s
+// PERMISSIVE-MINUS-OVERWRITE-NOT: "-fno-operator-names"
+// PERMISSIVE-MINUS-OVERWRITE: "-fdelayed-template-parsing"
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -1489,6 +1489,20 @@
   DAL.AddJoinedArg(A, Opts.getOption(options::OPT_D), NewVal);
 }
 
+static void TranslatePermissive(Arg *A, llvm::opt::DerivedArgList &DAL,
+const OptTable &Opts) {
+  DAL.AddFlagArg(A, Opts.getOption(options::OPT__SLASH_Zc_twoPhase_));
+  DAL.AddFlagArg(A, Opts.getOption(options::OPT_fno_operator_names));
+  // There is currently no /Zc:strictStrings- in clang-cl
+}
+
+static void TranslatePermissiveMinus(Arg *A, llvm::opt::DerivedArgList &DAL,
+ const OptTable &Opts) {
+  DAL.AddFlagArg(A, Opts.getOption(options::OPT__SLASH_Zc_twoPhase));
+  DAL.AddFlagArg(A, Opts.getOption(options::OPT_foperator_names));
+  DAL.AddFlagArg(A, Opts.getOption(options::OPT__SLASH_Zc_strictStrings));
+}
+
 llvm::opt::DerivedArgList *
 MSVCToolChain::TranslateArgs(const llvm::opt::DerivedArgList &Args,
  StringRef BoundArch,
@@ -1531,6 +1545,12 @@
 } else if (A->getOption().matches(options::OPT_D)) {
   // Translate -Dfoo#bar into -Dfoo=bar.
   TranslateDArg(A, *DAL, Opts);
+} else if (A->getOption().matches(options::OPT__SLASH_permissive)) {
+  // Expand /permissive
+  TranslatePermissive(A, *DAL, Opts);
+} else if (A->getOption().matche

[PATCH] D103771: [clang][msvc] Define _HAS_STATIC_RTTI to 0, when compiling with -fno-rtti

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Some background https://bugs.chromium.org/p/chromium/issues/detail?id=996675 : 
Back then I thought that MSVC switched to /Zc:twoPhase , but that turned out to 
not be true. So we never switched the default.

I think this patch here still makes sense, but I admit I don't have all the 
details paged in. Let's see if rnk or hans have opinions :)


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

https://reviews.llvm.org/D103771

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

[PATCH] D103772: [clang-cl] Reenable /Zc:twoPhase by default if targetting MSVC 2017 Update 3 or newer

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

It was reverted due to build failures, but it didn't reland because it's not 
the default in cl.exe after all. So we shouldn't make it the default in 
clang-cl either.


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

https://reviews.llvm.org/D103772

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

[PATCH] D103773: [clang-cl] Add /permissive and /permissive-

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

I think adding `/permissive-` to make things more conforming is great. The docs 
say "Starting in Visual Studio 2019 version 16.8, the /std:c++latest option 
implicitly sets the /permissive- option." so maybe we should do that too 
(doesn't have to be in this patch).

Since things seem to mostly work without `/permissive` (without the `-`) I'm 
not sure if we should add support for that. It'll make clang-cl less 
conforming, and in practice things seem to be fine as-is (…right?). Part of the 
reason why `/permissive-` doesn't expand to more flags in clang-cl 
(https://docs.microsoft.com/en-us/cpp/build/reference/permissive-standards-conformance?view=msvc-160
 lists a whole bunch more) I imagine is because clang-cl already has the 
stricter defaults there – which it can do because it's a newer compiler that 
needs to support less old code.

Details:

- docs say "You can pass specific /Zc options after /permissive- on the command 
line to override this behavior" – does that work with this approach? We should 
have a test for that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103773

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

[PATCH] D101156: [Clang] Support a user-defined __dso_handle

[Andrew Savonichev via Phabricator via cfe-commits]

asavonic updated this revision to Diff 350134.
asavonic edited the summary of this revision.
asavonic added a comment.

- Used `llvm::TrackingVH` to track Init changes.


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

https://reviews.llvm.org/D101156

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCXX/dso-handle-custom.cpp


Index: clang/test/CodeGenCXX/dso-handle-custom.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dso-handle-custom.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fexceptions %s 
-o - | FileCheck %s --check-prefixes CHECK,CHECK-DEFAULT
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fexceptions %s 
-o - -DHIDDEN | FileCheck %s --check-prefixes CHECK,CHECK-HIDDEN
+
+class A {
+public:
+  ~A();
+} a;
+
+// CHECK-DEFAULT: @__dso_handle = global i8* bitcast (i8** @__dso_handle to 
i8*), align 8
+// CHECK-HIDDEN: @__dso_handle = hidden global i8* bitcast (i8** @__dso_handle 
to i8*), align 8
+// CHECK: define internal void @__cxx_global_var_init()
+// CHECK:   call i32 @__cxa_atexit({{.*}}, {{.*}}, i8* bitcast (i8** 
@__dso_handle to i8*))
+
+#ifdef HIDDEN
+void *__dso_handle __attribute__((__visibility__("hidden"))) = &__dso_handle;
+#else
+void *__dso_handle = &__dso_handle;
+#endif
+
+void use(void *);
+void use_dso_handle() {
+  use(__dso_handle);
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4284,7 +4284,7 @@
   OpenMPRuntime->emitTargetGlobalVariable(D))
 return;
 
-  llvm::Constant *Init = nullptr;
+  llvm::TrackingVH Init;
   bool NeedsGlobalCtor = false;
   bool NeedsGlobalDtor =
   D->needsDestruction(getContext()) == QualType::DK_cxx_destructor;
@@ -4330,9 +4330,8 @@
   } else {
 initializedGlobalDecl = GlobalDecl(D);
 emitter.emplace(*this);
-Init = emitter->tryEmitForInitializer(*InitDecl);
-
-if (!Init) {
+llvm::Constant *Initializer = emitter->tryEmitForInitializer(*InitDecl);
+if (!Initializer) {
   QualType T = InitExpr->getType();
   if (D->getType()->isReferenceType())
 T = D->getType();
@@ -4345,6 +4344,7 @@
 Init = llvm::UndefValue::get(getTypes().ConvertType(T));
   }
 } else {
+  Init = Initializer;
   // We don't need an initializer, so remove the entry for the delayed
   // initializer position (just in case this entry was delayed) if we
   // also don't need to register a destructor.


Index: clang/test/CodeGenCXX/dso-handle-custom.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dso-handle-custom.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fexceptions %s -o - | FileCheck %s --check-prefixes CHECK,CHECK-DEFAULT
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -fexceptions %s -o - -DHIDDEN | FileCheck %s --check-prefixes CHECK,CHECK-HIDDEN
+
+class A {
+public:
+  ~A();
+} a;
+
+// CHECK-DEFAULT: @__dso_handle = global i8* bitcast (i8** @__dso_handle to i8*), align 8
+// CHECK-HIDDEN: @__dso_handle = hidden global i8* bitcast (i8** @__dso_handle to i8*), align 8
+// CHECK: define internal void @__cxx_global_var_init()
+// CHECK:   call i32 @__cxa_atexit({{.*}}, {{.*}}, i8* bitcast (i8** @__dso_handle to i8*))
+
+#ifdef HIDDEN
+void *__dso_handle __attribute__((__visibility__("hidden"))) = &__dso_handle;
+#else
+void *__dso_handle = &__dso_handle;
+#endif
+
+void use(void *);
+void use_dso_handle() {
+  use(__dso_handle);
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4284,7 +4284,7 @@
   OpenMPRuntime->emitTargetGlobalVariable(D))
 return;
 
-  llvm::Constant *Init = nullptr;
+  llvm::TrackingVH Init;
   bool NeedsGlobalCtor = false;
   bool NeedsGlobalDtor =
   D->needsDestruction(getContext()) == QualType::DK_cxx_destructor;
@@ -4330,9 +4330,8 @@
   } else {
 initializedGlobalDecl = GlobalDecl(D);
 emitter.emplace(*this);
-Init = emitter->tryEmitForInitializer(*InitDecl);
-
-if (!Init) {
+llvm::Constant *Initializer = emitter->tryEmitForInitializer(*InitDecl);
+if (!Initializer) {
   QualType T = InitExpr->getType();
   if (D->getType()->isReferenceType())
 T = D->getType();
@@ -4345,6 +4344,7 @@
 Init = llvm::UndefValue::get(getTypes().ConvertType(T));
   }
 } else {
+  Init = Initializer;
   // We don't need an initializer, so remove the entry for the delayed
   // initializer position (just in case this entry was delayed) if we
   // also don't need to register a destructor.
___
cfe-com

[PATCH] D101156: [Clang] Support a user-defined __dso_handle

[Andrew Savonichev via Phabricator via cfe-commits]

asavonic added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4425
+  if (ReplaceInitWithGV)
+Init = llvm::ConstantExpr::getBitCast(GV, GV->getValueType());
+

rjmccall wrote:
> Can we actually do this bitcast for arbitrary initializers?  This seems 
> problematic.
> 
> I think we just need to hold `Init` in an `llvm::TrackingVH` across the call 
> to `GetAddrOfGlobalVar`.
Thanks a lot! `TrackingVH` works great, with a few adjustments because it 
cannot be checked for nullptr.


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

https://reviews.llvm.org/D101156

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

[PATCH] D103777: [X32] Add Triple::isX32(), use it.

[Harald van Dijk via Phabricator via cfe-commits]

hvdijk created this revision.
hvdijk added reviewers: MaskRay, craig.topper.
Herald added subscribers: dexonsmith, pengfei, hiraditya, dschuff.
hvdijk requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

So far, support for x86_64-linux-gnux32 has been handled by explicit
comparisons of Triple.getEnvironment() to GNUX32. This worked as long as
x86_64-linux-gnux32 was the only X32 environment to worry about, but we
now have x86_64-linux-muslx32 as well. To support this, this change adds
an isX32() function and uses it. It replaces all checks for GNUX32 or
MuslX32 by isX32(), except for the following:

- Triple::isGNUEnvironment() and Triple::isMusl() are supposed to treat GNUX32 
and MuslX32 differently.
- computeTargetTriple() needs to be able to transform triples to add or remove 
X32 from the environment and needs to map GNU to GNUX32, and Musl to MuslX32.
- getMultiarchTriple() completely lacks any Musl support and retains the 
explicit check for GNUX32 as it can only return x86_64-linux-gnux32.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103777

Files:
  clang/lib/Basic/Targets/X86.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/linux-cross.cpp
  llvm/include/llvm/ADT/Triple.h
  llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
  llvm/lib/Target/X86/MCTargetDesc/X86MCAsmInfo.cpp
  llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/test/CodeGen/X86/x32-lea-1.ll

Index: llvm/test/CodeGen/X86/x32-lea-1.ll
===
--- llvm/test/CodeGen/X86/x32-lea-1.ll
+++ llvm/test/CodeGen/X86/x32-lea-1.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=x86_64-linux-gnux32 -O0 | FileCheck %s
+; RUN: llc < %s -mtriple=x86_64-linux-muslx32 -O0 | FileCheck %s
 
 define void @foo(i32** %p) {
 ; CHECK-LABEL: foo:
Index: llvm/lib/Target/X86/X86TargetMachine.cpp
===
--- llvm/lib/Target/X86/X86TargetMachine.cpp
+++ llvm/lib/Target/X86/X86TargetMachine.cpp
@@ -110,9 +110,7 @@
 
   Ret += DataLayout::getManglingComponent(TT);
   // X86 and x32 have 32 bit pointers.
-  if ((TT.isArch64Bit() &&
-   (TT.getEnvironment() == Triple::GNUX32 || TT.isOSNaCl())) ||
-  !TT.isArch64Bit())
+  if (!TT.isArch64Bit() || TT.isX32() || TT.isOSNaCl())
 Ret += "-p:32:32";
 
   // Address spaces for 32 bit signed, 32 bit unsigned, and 64 bit pointers.
Index: llvm/lib/Target/X86/X86Subtarget.h
===
--- llvm/lib/Target/X86/X86Subtarget.h
+++ llvm/lib/Target/X86/X86Subtarget.h
@@ -610,14 +610,12 @@
 
   /// Is this x86_64 with the ILP32 programming model (x32 ABI)?
   bool isTarget64BitILP32() const {
-return In64BitMode && (TargetTriple.getEnvironment() == Triple::GNUX32 ||
-   TargetTriple.isOSNaCl());
+return In64BitMode && (TargetTriple.isX32() || TargetTriple.isOSNaCl());
   }
 
   /// Is this x86_64 with the LP64 programming model (standard AMD64, no x32)?
   bool isTarget64BitLP64() const {
-return In64BitMode && (TargetTriple.getEnvironment() != Triple::GNUX32 &&
-   !TargetTriple.isOSNaCl());
+return In64BitMode && (!TargetTriple.isX32() && !TargetTriple.isOSNaCl());
   }
 
   PICStyles::Style getPICStyle() const { return PICStyle; }
Index: llvm/lib/Target/X86/X86RegisterInfo.cpp
===
--- llvm/lib/Target/X86/X86RegisterInfo.cpp
+++ llvm/lib/Target/X86/X86RegisterInfo.cpp
@@ -62,7 +62,7 @@
 // This matches the simplified 32-bit pointer code in the data layout
 // computation.
 // FIXME: Should use the data layout?
-bool Use64BitReg = TT.getEnvironment() != Triple::GNUX32;
+bool Use64BitReg = !TT.isX32();
 StackPtr = Use64BitReg ? X86::RSP : X86::ESP;
 FramePtr = Use64BitReg ? X86::RBP : X86::EBP;
 BasePtr = Use64BitReg ? X86::RBX : X86::EBX;
Index: llvm/lib/Target/X86/X86AsmPrinter.cpp
===
--- llvm/lib/Target/X86/X86AsmPrinter.cpp
+++ llvm/lib/Target/X86/X86AsmPrinter.cpp
@@ -643,8 +643,7 @@
   OutStreamer->SwitchSection(Nt);
 
   // Emitting note header.
-  const int WordSize =
-  TT.isArch64Bit() && TT.getEnvironment() != Triple::GNUX32 ? 8 : 4;
+  const int WordSize = TT.isArch64Bit() && !TT.isX32() ? 8 : 4;
   emitAlignment(WordSize == 4 ? Align(4) : Align(8));
   OutStreamer->emitIntValue(4, 4 /*size*/); // data size for "GNU\0"
   OutStreamer->emitIntValue(8 + WordSize, 4 /*si

[PATCH] D103750: [analyzer][WIP] Handle std::make_unique for SmartPtrModeling

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Tests pls!

Yes I think you should totally do an `evalCall()` here. The function has no 
other side effects apart from making a pointer so it's valuable to fully model 
it so that to avoid unnecessary store invalidations.

In D103750#2801320 , @xazax.hun wrote:

>> Since we have CallExpr, we can easily conjure up an SVal. But I don't see 
>> how I can do it similarly in this patch.
>
> You should have a `CallExpr` for `std::make_unique` too. I believe that 
> expression is used to determine how the conjured symbol was created (to give 
> it an identity). So probably it should be ok to use the `make_unique` call to 
> create this symbol (but be careful to create the symbol with the right type).

That's right, a conjured symbol isn't necessarily the value of its respective 
expression. So you can conjure it and assume that it's non-null. Also consider 
using a //heap// conjured symbol (a-la `getConjuredHeapSymbolVal()`). That 
said, `getConjuredHeapSymbolVal()` doesn't provide an overload for overriding 
the type; there's no reason it shouldn't though, please feel free to extend it.

There's a separate story about `SymbolConjured` being the right class for the 
problem. I'm still in favor of having `SymbolMetadata` instead, so that to 
properly deduplicate symbols across different branches and merge more paths.




Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:204
+MakeUniqueKind
+SmartPtrModeling::isStdMakeUniqueCall(const CallEvent &Call) const {
+  if (Call.getKind() != CallEventKind::CE_Function)

Please use `CallDescription` for most of this stuff.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:337
+
+bool isUniquePtrType(QualType QT) {
+  const auto *T = QT.getTypePtr();

Please merge with `isStdSmartPtrCall`.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:338-341
+  const auto *T = QT.getTypePtr();
+  if (!T)
+return false;
+  const auto *Decl = T->getAsCXXRecordDecl();

`getTypePtr()` is almost never necessary because `QualType` has an overloaded 
`operator->()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103750

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

[PATCH] D98726: [analyzer] Enabling MallocChecker to take up after SmartPtrModelling

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D98726#2798325 , @RedDocMD wrote:

> This is what causes the false suppression. To be more specific, the analyzer 
> tries to follow the logic of the destructor of unique_ptr into the standard 
> library. And since that is in the std namespace, it causes 
> LikelyFalsePositiveSuppressionBRVisitor::finalizeVisitor() to squelch the 
> report. Now there are two problems here:

Which of the two reports are you describing here?

> - Why is there a bug in `unique_ptr.h`? (This is the worse of the two, IMO). 
> I am going to take a look at the standard library code (sigh) and see if 
> that's an actual bug or another false positive.

If the warning is / the bug path ends in `unique_ptr.h` it doesn't mean that 
the bug is in `unique_ptr.h` in particular or in the standard library in 
general. That's the thing with path sensitive analysis: all nodes of the path 
are potentially responsible for the bug. Take away any statement in the middle 
and the entire thing might never happen. This is why path-sensitive checkers 
will most likely never provide fix-it hints: we aren't supposed to know at all 
which part of the path was //the// problem, all we know is that the path in its 
entirety leads to an instance of problematic behavior.

For this reason we're also not massively suppressing all warnings that are 
displayed against the C standard library headers (unlike clang-tidy). In case 
of clang-tidy even if there's a bug in the standard library the user will 
probably be unable to fix it, so it's entirely their job to suppress such 
warnings. In case of the static analyzer the warning in the standard library 
isn't an indication that the standard library needs to be fixed at all; most of 
the time the user can handle it in their code. What we do do, however, is avoid 
analyzing standard library functions as analysis entry points - as it's a good 
indication that the entire bug path (and, therefore, the entire list of things 
to blame) is going to be in the standard library.

We are suppressing warnings against C++ standard library headers though, as 
you've correctly pointed out. This was a judgement call because it looks like 
there are too many false positives among these warnings. We should really 
reconsider this suppression in the future, it could most likely be made much 
more fine-grained.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98726

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

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D103605#2800101 , @vsavchenko 
wrote:

>>> Actually, tracker dies when this happens.  But hey, can you provide a use 
>>> case when the checker really needs to know when this happens?
>>
>> Inlined defensive check suppressions are a major example. Basically null 
>> dereference warnings are suppressed based on properties of the program point 
>> at which the tracker terminates. If we know when this happens, we can 
>> refactor inlined defensive check suppressions into a checker specific 
>> handler.
>
> It seems to me that it is a hard concept to wrap your mind around.  And even 
> if I would tell someone how we suppress null pointer inline checks, I 
> wouldn't use "when the tracker stops" as a description of the event.  It is 
> very vague and tightly coupled with the implementation of tracker stopping, 
> and it is not connected semantically to the problem we are trying to solve.  
> Correct me if I'm wrong, but better description would be track when we 
> constrained the pointer value to null, and if it in the child stack frame, 
> drop it.
> So, maybe instead of very hard to understand "when the tracker stops", we can 
> use "when the constraint changes" or "when the constraint became this".  This 
> is more similar to store handlers in their nature, we find a pair of states 
> where some condition stops holding (man, that can have a generic 
> implementation!), describe it, and call the handler.

Inlined defensive check suppressions are indeed pretty ugly; this problem 
should ideally be solved in a completely different manner.

That said, I strongly disagree that "when the tracker stops" is a 
hard-to-understand concept. It's an extremely natural and intuitive question to 
ask:

//"Where does the value come from?"//

It's fairly reasonable to wonder what happened during analysis that caused us 
to produce this value for the first time, which then traveled down to our bug 
point, and under what circumstances did it happen. Not much precise reasoning 
can be based on this information but it's a very good piece of information to 
drive imprecise reasoning and heuristics, such as (but not limited to) inlined 
defensive check suppressions (where it bears a relatively precise meaning that 
corresponds exactly to "state splits in nested stack frames are intrinsically 
less reliable as they can't ever be derived from presumption of no dead code").

Another example: a value of a top-level parameter variable is much more likely 
to be arbitrary than a return value of an unknown function call (if the user 
wanted pre-conditions they could have added an assert), which in turn is much 
more likely to be arbitrary than a value loaded from a heap region after store 
invalidation from unknown function call (we're 99% sure that the function 
doesn't touch this particular heap region so it's probably just the old value). 
In this sense, I'm pretty tempted to suppress bug paths where two branch 
conditions (ultimately going into opposite directions) are based on different 
values of the same heap variable after different number of invalidations. But 
I'm certainly against doing so for two different top-level parameter values. 
You may say that in these cases the information is most likely available from 
the structure of the symbol (`SymbolRegionValue` vs. `SymbolConjured` vs. 
`SymbolDerived`) but it only confirms my point: origins of values are so 
important that we even carry them as part of the value's identity! Direct 
access to the structure of the symbol for determining origins is also used a 
few times (eg., see call sites of `getOriginRegion()`).

> I want to add something (because I might seem defensive).  I do want to make 
> the world peace, but this series of patchiest is addressing a lot of headache 
> me (and Deep) had because of the way `trackExpressionValue` is organized.
> If you think about it, all these patches just do one thing - extract piece of 
> code into a separate function, class, or abstraction.
> Fundamentally it is all the same.
>
> I think we can discuss future plans, but also try to be realistic about the 
> scope of this refactoring/redesign, since I'm trying to morph existing code 
> into something a little better.

What I'm trying to do here is to see a general direction in which we're moving. 
We have the benefit of being able to predict things. We don't have to be agile; 
our problem is well-defined from the start and will never really change. 
There's a bit of flexibility required to support a variety of different future 
checkers, yes, but that's about it. And then, again, it's also not entirely 
arbitrary: we have a pretty good amount of data on what our checkers typically 
do and it's unlikely to expand drastically in the near future. So we don't 
always need to go for the most flexible solution, but there's often benefit in 
going for a simpler solution that simply covers our predictable needs and 
nothing else

[PATCH] D102148: [clangd] Type hints for variables with 'auto' type

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/InlayHints.cpp:81
+if (auto *AT = D->getType()->getContainedAutoType()) {
+  if (!D->getType()->isDependentType()) {
+addInlayHint(D->getLocation(), InlayHintKind::TypeHint,

nridge wrote:
> sammccall wrote:
> > why this check vs checking whether AT is deduced?
> > (thus fixing the dependent `T` testcase)
> > At first I assumed this was the usual "AutoType in AST doesn't store the 
> > deduced type" problem, but shouldn't it work properly if we're starting at 
> > the variable type?
> Not sure if I'm understanding the suggestion correctly, but if I make the 
> condition `AT->isDeduced()`, I get an unwanted `auto` hint for `var1` in 
> `TypeHints.DependentType` as well (and moreover, the hint for `var2` is also 
> `auto` instead of the desired `T`).
> and moreover, the hint for `var2` is also `auto` instead of the desired `T`

I've looked into this a bit further, and it looks to me like the `T` isn't even 
stored in the AST. The type is just recorded as `auto` (and that's what e.g. 
hover shows too).

I suspect the `auto` is only filled in **after** instantiation (i.e. once the 
initializer expression has a concrete type).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102148

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

[PATCH] D101156: [Clang] Support a user-defined __dso_handle

[John McCall via Phabricator via cfe-commits]

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

Great, LGTM.


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

https://reviews.llvm.org/D101156

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

[PATCH] D103784: [X86] Support __tile_stream_loadd intrinsic for new AMX interface

[Bing Yu via Phabricator via cfe-commits]

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

Adding support for __tile_stream_loadd intrinsic.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103784

Files:
  clang/include/clang/Basic/BuiltinsX86_64.def
  clang/lib/Headers/amxintrin.h
  clang/test/CodeGen/X86/amx_api.c
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/lib/Target/X86/X86ExpandPseudo.cpp
  llvm/lib/Target/X86/X86FastTileConfig.cpp
  llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
  llvm/lib/Target/X86/X86InstrAMX.td
  llvm/lib/Target/X86/X86LowerAMXType.cpp
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/test/CodeGen/X86/AMX/amx-tile-basic.ll

Index: llvm/test/CodeGen/X86/AMX/amx-tile-basic.ll
===
--- llvm/test/CodeGen/X86/AMX/amx-tile-basic.ll
+++ llvm/test/CodeGen/X86/AMX/amx-tile-basic.ll
@@ -23,6 +23,7 @@
 ; CHECK-NEXT:tdpbusd %tmm2, %tmm1, %tmm0
 ; CHECK-NEXT:tdpbuud %tmm2, %tmm1, %tmm0
 ; CHECK-NEXT:tdpbf16ps %tmm2, %tmm1, %tmm0
+; CHECK-NEXT:tileloaddt1 (%rsi,%rdx), %tmm1
 ; CHECK-NEXT:tilestored %tmm0, (%rdi,%rdx)
 ; CHECK-NEXT:tilerelease
 ; CHECK-NEXT:vzeroupper
@@ -35,6 +36,7 @@
   %d2 = call x86_amx @llvm.x86.tdpbusd.internal(i16 8, i16 8, i16 8, x86_amx %d1, x86_amx %a, x86_amx %b)
   %d3 = call x86_amx @llvm.x86.tdpbuud.internal(i16 8, i16 8, i16 8, x86_amx %d2, x86_amx %a, x86_amx %b)
   %d4 = call x86_amx @llvm.x86.tdpbf16ps.internal(i16 8, i16 8, i16 8, x86_amx %d3, x86_amx %a, x86_amx %b)
+  %e = call x86_amx @llvm.x86.tileloaddt164.internal(i16 8, i16 8, i8* %base, i64 %stride)
   call void @llvm.x86.tilestored64.internal(i16 8, i16 8, i8* %pointer, i64 %stride, x86_amx %d4)
 
   ret void
@@ -42,6 +44,7 @@
 
 declare x86_amx @llvm.x86.tilezero.internal(i16, i16)
 declare x86_amx @llvm.x86.tileloadd64.internal(i16, i16, i8*, i64)
+declare x86_amx @llvm.x86.tileloaddt164.internal(i16, i16, i8*, i64)
 declare x86_amx @llvm.x86.tdpbssd.internal(i16, i16, i16, x86_amx, x86_amx, x86_amx)
 declare x86_amx @llvm.x86.tdpbsud.internal(i16, i16, i16, x86_amx, x86_amx, x86_amx)
 declare x86_amx @llvm.x86.tdpbusd.internal(i16, i16, i16, x86_amx, x86_amx, x86_amx)
Index: llvm/lib/Target/X86/X86RegisterInfo.cpp
===
--- llvm/lib/Target/X86/X86RegisterInfo.cpp
+++ llvm/lib/Target/X86/X86RegisterInfo.cpp
@@ -892,6 +892,7 @@
   }
   // We only collect the tile shape that is defined.
   case X86::PTILELOADDV:
+  case X86::PTILELOADDT1V:
   case X86::PTDPBSSDV:
   case X86::PTDPBSUDV:
   case X86::PTDPBUSDV:
Index: llvm/lib/Target/X86/X86LowerAMXType.cpp
===
--- llvm/lib/Target/X86/X86LowerAMXType.cpp
+++ llvm/lib/Target/X86/X86LowerAMXType.cpp
@@ -121,6 +121,7 @@
   default:
 llvm_unreachable("Expect amx intrinsics");
   case Intrinsic::x86_tileloadd64_internal:
+  case Intrinsic::x86_tileloaddt164_internal:
   case Intrinsic::x86_tilestored64_internal: {
 Row = II->getArgOperand(0);
 Col = II->getArgOperand(1);
Index: llvm/lib/Target/X86/X86InstrAMX.td
===
--- llvm/lib/Target/X86/X86InstrAMX.td
+++ llvm/lib/Target/X86/X86InstrAMX.td
@@ -53,6 +53,9 @@
 def PTILELOADDV : PseudoI<(outs TILE:$dst), (ins GR16:$src1,
  GR16:$src2,
  opaquemem:$src3), []>;
+def PTILELOADDT1V : PseudoI<(outs TILE:$dst), (ins GR16:$src1,
+ GR16:$src2,
+ opaquemem:$src3), []>;
 def PTILESTOREDV : PseudoI<(outs), (ins GR16:$src1,
 GR16:$src2, opaquemem:$src3,
 TILE:$src4), []>;
Index: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
===
--- llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
+++ llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
@@ -4617,10 +4617,13 @@
   ReplaceNode(Node, Res);
   return;
 }
-case Intrinsic::x86_tileloadd64_internal: {
+case Intrinsic::x86_tileloadd64_internal:
+case Intrinsic::x86_tileloaddt164_internal: {
   if (!Subtarget->hasAMXTILE())
 break;
-  unsigned Opc = X86::PTILELOADDV;
+  unsigned Opc = IntNo == Intrinsic::x86_tileloadd64_internal
+ ? X86::PTILELOADDV
+ : X86::PTILELOADDT1V;
   // _tile_loadd_internal(row, col, buf, STRIDE)
   SDValue Base = Node->getOperand(4);
   SDValue Scale = getI8Imm(1, dl);
Index: llvm/lib/Target/X86/X86FastTileConfig.cpp
===
--- llvm/lib

[PATCH] D103789: [clangd] Type hints for C++14 return type deduction

[Nathan Ridge via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103789

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


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -478,14 +478,32 @@
 }
 
 TEST(TypeHints, ReturnTypeDeduction) {
-  // FIXME: Not handled yet.
-  // This test is currently here mostly because a naive implementation
-  // might have us print something not super helpful like the function type.
-  assertTypeHints(R"cpp(
-auto func(int x) {
-  return x + 1;
-}
-  )cpp");
+  assertTypeHints(
+  R"cpp(
+$ret1a[[auto]] f1(int x);  // Hint forward declaration too
+$ret1b[[auto]] f1(int x) { return x + 1; }
+
+// Include pointer operators in both location and hint
+// (we could also go the other way).
+int s;
+$ret2[[auto&]] f2() { return s; }
+
+// Do not hint `auto` for trailing return type.
+auto f3() -> int;
+
+// `auto` conversion operator
+struct A {
+  operator $retConv[[auto]]() { return 42; }
+};
+
+// FIXME: Dependent types do not work yet.
+template 
+struct S {
+  auto method() { return T(); }
+};
+  )cpp",
+  ExpectedHint{": int", "ret1a"}, ExpectedHint{": int", "ret1b"},
+  ExpectedHint{": int &", "ret2"}, ExpectedHint{": int", "retConv"});
 }
 
 TEST(TypeHints, DependentType) {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -72,6 +72,23 @@
 return true;
   }
 
+  bool VisitFunctionDecl(FunctionDecl *D) {
+if (auto *AT = D->getReturnType()->getContainedAutoType()) {
+  QualType Deduced = AT->getDeducedType();
+  if (!Deduced.isNull()) {
+SourceRange R = D->getReturnTypeSourceRange();
+// For operator auto(), have to get location of `auto` a different way.
+if (R.isInvalid() && isa(D)) {
+  R = D->getTypeSourceInfo()->getTypeLoc().getBeginLoc();
+}
+addInlayHint(R, InlayHintKind::TypeHint,
+ ": " + D->getReturnType().getAsString(TypeHintPolicy));
+  }
+}
+
+return true;
+  }
+
   bool VisitVarDecl(VarDecl *D) {
 // Do not show hints for the aggregate in a structured binding.
 // In the future, we may show hints for the individual bindings.


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -478,14 +478,32 @@
 }
 
 TEST(TypeHints, ReturnTypeDeduction) {
-  // FIXME: Not handled yet.
-  // This test is currently here mostly because a naive implementation
-  // might have us print something not super helpful like the function type.
-  assertTypeHints(R"cpp(
-auto func(int x) {
-  return x + 1;
-}
-  )cpp");
+  assertTypeHints(
+  R"cpp(
+$ret1a[[auto]] f1(int x);  // Hint forward declaration too
+$ret1b[[auto]] f1(int x) { return x + 1; }
+
+// Include pointer operators in both location and hint
+// (we could also go the other way).
+int s;
+$ret2[[auto&]] f2() { return s; }
+
+// Do not hint `auto` for trailing return type.
+auto f3() -> int;
+
+// `auto` conversion operator
+struct A {
+  operator $retConv[[auto]]() { return 42; }
+};
+
+// FIXME: Dependent types do not work yet.
+template 
+struct S {
+  auto method() { return T(); }
+};
+  )cpp",
+  ExpectedHint{": int", "ret1a"}, ExpectedHint{": int", "ret1b"},
+  ExpectedHint{": int &", "ret2"}, ExpectedHint{": int", "retConv"});
 }
 
 TEST(TypeHints, DependentType) {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -72,6 +72,23 @@
 return true;
   }
 
+  bool VisitFunctionDecl(FunctionDecl *D) {
+if (auto *AT = D->getReturnType()->getContainedAutoType()) {
+  QualType Deduced = AT->getDeducedType();
+  if (!Deduced.isNull()) {
+SourceRange R = D->getReturnTypeSourceRange();
+// For operator auto(), have to get location of `auto` a different way.
+if (R.isInvalid() && isa(D)) {
+  R = D->getTypeSourceInfo()->getTypeLoc().getBeginLoc();
+}
+addInlayHint(R, Inlay