[PATCH] D69292: Proposal to add -Wtautological-compare to -Wall

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

Sorry I'm late to the party; I've been traveling for 3+ weeks.
I would like to be reassured that the following code will not warn:

  `
  long foo = ...; // some calculation
  if (foo < std::numeric_limits::min() || foo > 
std::numeric_limits::max()) .

This is important for systems where `sizeof(int) == sizeof(long)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69292



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

[clang] 39af723 - Unbreak the clang test suite when hexagon-link is not available

[David Zarzycki via cfe-commits]

Author: David Zarzycki
Date: 2019-11-15T08:47:02+02:00
New Revision: 39af72378dd0481472e267ed7f15da594de0fee3

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

LOG: Unbreak the clang test suite when hexagon-link is not available

All of the other tests are of the form {{hexagon-link|ld}} so this
probably should be too.

Added: 


Modified: 
clang/test/Driver/hexagon-toolchain-elf.c

Removed: 




diff  --git a/clang/test/Driver/hexagon-toolchain-elf.c 
b/clang/test/Driver/hexagon-toolchain-elf.c
index 237242e7cbf1..661e758d931b 100644
--- a/clang/test/Driver/hexagon-toolchain-elf.c
+++ b/clang/test/Driver/hexagon-toolchain-elf.c
@@ -132,7 +132,7 @@
 // RUN:   %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK029 %s
 // CHECK029: "-cc1" {{.*}} "-target-cpu" "hexagonv65"
-// CHECK029: 
hexagon-link{{.*}}/Inputs/hexagon_tree/Tools/bin/../target/hexagon/lib/v65/crt0
+// CHECK029: 
{{hexagon-link|ld}}{{.*}}/Inputs/hexagon_tree/Tools/bin/../target/hexagon/lib/v65/crt0
 
 // 
-
 // Test Linker related args



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

[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 229448.
jdoerfert marked 5 inline comments as done.
jdoerfert added a comment.

Addressed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70258

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMPIRBuilder.h
  llvm/lib/Frontend/OpenMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMPIRBuilder.cpp
@@ -228,7 +228,8 @@
   // If we are in a cancellable parallel region, barriers are cancellation
   // points.
   // TODO: Check why we would force simple calls or to ignore the cancel flag.
-  bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+  bool UseCancelBarrier =
+  !ForceSimpleCall && isLastFinalizationInfoCancellable(OMPD_parallel);
 
   Value *Result = Builder.CreateCall(
   getOrCreateRuntimeFunction(UseCancelBarrier ? OMPRTL___kmpc_cancel_barrier
@@ -248,13 +249,14 @@
 Builder.CreateCondBr(Cmp, NonCancellationBlock, CancellationBlock,
  /* TODO weight */ nullptr, nullptr);
 
-Builder.SetInsertPoint(NonCancellationBlock);
-assert(CancellationBlock->getParent() == BB->getParent() &&
-   "Unexpected cancellation block parent!");
+// From the cancellation block we finalize all variables and go to the
+// post finalization block that is known to the FiniCB callback.
+Builder.SetInsertPoint(CancellationBlock);
+auto  = FinalizationStack.back();
+FI.FiniCB(Builder.saveIP());
 
-// TODO: This is a workaround for now, we always reset the cancellation
-// block until we manage it ourselves here.
-CancellationBlock = nullptr;
+// The continuation block is where code generation continues.
+Builder.SetInsertPoint(NonCancellationBlock);
   }
 
   return Builder.saveIP();
Index: llvm/include/llvm/Frontend/OpenMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMPIRBuilder.h
@@ -37,12 +37,44 @@
   /// Add attributes known for \p FnID to \p Fn.
   void addAttributes(omp::RuntimeFunction FnID, Function );
 
-  /// Set the cancellation block to \p CBB.
-  void setCancellationBlock(BasicBlock *CBB) { CancellationBlock = CBB; }
-
   /// Type used throughout for insertion points.
   using InsertPointTy = IRBuilder<>::InsertPoint;
 
+  /// Callback type for variable finalization (think destructors).
+  ///
+  /// \param CodeGenIP is the insertion point at which the finalization code
+  ///  should be placed.
+  ///
+  /// A finalize callback knows about all objects that need finalization, e.g.
+  /// destruction, when the scope of the currently generated construct is left
+  /// at the time, and location, the callback is invoked.
+  using FinalizeCallbackTy = function_ref;
+
+  struct FinalizationInfo {
+/// The finalization callback provided by the last in-flight invocation of
+/// Create for the directive of kind DK.
+FinalizeCallbackTy FiniCB;
+
+/// The directive kind of the innermost directive that has an associated
+/// region which might require finalization when it is left.
+omp::Directive DK;
+
+/// Flag to indicate if the directive is cancellable.
+bool IsCancellable;
+  };
+
+  /// Push a finalization callback on the finalization stack.
+  ///
+  /// NOTE: Temporary solution until Clang CG is gone.
+  void pushFinalizationCB(FinalizationInfo &) {
+FinalizationStack.emplace_back(std::move(FI));
+  }
+
+  /// Pop the last finalization callback from the finalization stack.
+  ///
+  /// NOTE: Temporary solution until Clang CG is gone.
+  void popFinalizationCB() { FinalizationStack.pop_back(); }
+
   /// Description of a LLVM-IR insertion point (IP) and a debug/source location
   /// (filename, line, column, ...).
   struct LocationDescription {
@@ -112,6 +144,19 @@
 omp::Directive DK, bool ForceSimpleCall,
 bool CheckCancelFlag);
 
+  /// The finalization stack made up of finalize callbacks currently in-flight,
+  /// wrapped into FinalizationInfo objects that reference also the finalization
+  /// target block and the kind of cancellable directive.
+  SmallVector FinalizationStack;
+
+  /// Return true if the last entry in the finalization stack is of kind \p DK
+  /// and cancellable.
+  bool isLastFinalizationInfoCancellable(omp::Directive DK) {
+return !FinalizationStack.empty() &&
+   FinalizationStack.back().IsCancellable &&
+   FinalizationStack.back().DK == DK;
+  }
+
   /// Return the current thread ID.
   ///
   /// \param Ident The ident (ident_t*) describing the query origin.
@@ -123,9 +168,6 @@
   /// The LLVM-IR Builder used to create IR.
   

[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:1499-1509
+OMPBuilder->pushFinalizationCB(std::move(FI));
+  }
+
   CGOpenMPOutlinedRegionInfo CGInfo(*CS, ThreadIDVar, CodeGen, InnermostKind,
 HasCancel, OutlinedHelperName);
   CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(CGF, );
+  llvm::Function *Fn = CGF.GenerateOpenMPCapturedStmtFunction(*CS);

ABataev wrote:
> Again, need RAII
There is no appropriate scope to put it in.



Comment at: llvm/include/llvm/Frontend/OpenMPIRBuilder.h:69-71
+  void pushFinalizationCB(FinalizationInfo &) {
+FinalizationStack.emplace_back(std::move(FI));
+  }

ABataev wrote:
> Maybe `push...` for const reference and `emplace...` for variadic template 
> (just like in standard library)?
That would defeat the purpose of using move semantic. I can make it a push but 
we will then have to copy and not move.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70258



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

[PATCH] D70290: [OpenMP] Use the OpenMPIRBuilder for "omp parallel"

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added reviewers: kiranchandramohan, ABataev, RaviNarayanaswamy, 
gtbercea, grokos, sdmitriev, JonChesterfield, hfinkel, fghanim.
Herald added subscribers: guansong, bollu.
Herald added a project: clang.

NOTE: This patch needs tests and is missing privatization functionality (see
the TODO). Basic testing was done and any other functionality should be
in place.

This allows to use the OpenMPIRBuilder for parallel regions. Code was
extracted from D61953  and adapted to work 
with the new version (D70109 ).

All but one feature should be supported. An update of this patch will
provide test coverage and privatization other than shared.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70290

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp

Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -15,10 +15,11 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "TargetInfo.h"
+#include "clang/AST/DeclOpenMP.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtOpenMP.h"
-#include "clang/AST/DeclOpenMP.h"
 #include "clang/Basic/PrettyStackTrace.h"
+#include "llvm/Frontend/OpenMPIRBuilder.h"
 using namespace clang;
 using namespace CodeGen;
 using namespace llvm::omp;
@@ -1316,6 +1317,86 @@
  llvm::SmallVectorImpl &) {}
 
 void CodeGenFunction::EmitOMPParallelDirective(const OMPParallelDirective ) {
+
+  llvm::OpenMPIRBuilder *OMPBuilder = CGM.getOpenMPIRBuilder();
+  if (OMPBuilder) {
+// Check if we have any if clause associated with the directive.
+llvm::Value *IfCond = nullptr;
+if (const auto *C = S.getSingleClause())
+  IfCond = EmitScalarExpr(C->getCondition(),
+  /*IgnoreResultAssign=*/true);
+
+llvm::Value *NumThreads = nullptr;
+if (const auto *NumThreadsClause = S.getSingleClause())
+  NumThreads = EmitScalarExpr(NumThreadsClause->getNumThreads(),
+  /*IgnoreResultAssign=*/true);
+
+ProcBindKind ProcBind = OMP_PB_default;
+if (const auto *ProcBindClause = S.getSingleClause())
+  ProcBind = ProcBindClause->getProcBindKind();
+
+using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
+
+// The cleanup callback that finalizes all variabels at the given location,
+// thus calls destructors etc.
+auto FiniCB = [this](InsertPointTy IP) {
+  CGBuilderTy::InsertPointGuard IPG(Builder);
+  assert(IP.getBlock()->end() != IP.getPoint() &&
+ "OpenMP IR Builder should cause terminated block!");
+  llvm::BasicBlock *IPBB = IP.getBlock();
+  llvm::BasicBlock *DestBB = IPBB->splitBasicBlock(IP.getPoint());
+  IPBB->getTerminator()->eraseFromParent();
+  Builder.SetInsertPoint(IPBB);
+  CodeGenFunction::JumpDest Dest = getJumpDestInCurrentScope(DestBB);
+  EmitBranchThroughCleanup(Dest);
+};
+
+// Privatization callback that performs appropriate action for
+// shared/private/firstprivate/lastprivate/copyin/... variables.
+//
+// TODO: This defaults to shared right now.
+auto PrivCB = [](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
+ llvm::Value , llvm::Value *) {
+  // The next line is appropriate only for variables (Val) with the
+  // data-sharing attribute "shared".
+  ReplVal = 
+
+  return CodeGenIP;
+};
+
+const CapturedStmt *CS = S.getCapturedStmt(OMPD_parallel);
+const Stmt *ParallelRegionBodyStmt = CS->getCapturedStmt();
+
+auto BodyGenCB = [ParallelRegionBodyStmt,
+  this](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
+llvm::BasicBlock ) {
+  auto OldAllocaIP = AllocaInsertPt;
+  AllocaInsertPt = &*AllocaIP.getPoint();
+
+  auto OldReturnBlock = ReturnBlock;
+  ReturnBlock = getJumpDestInCurrentScope();
+
+  llvm::BasicBlock *CodeGenIPBB = CodeGenIP.getBlock();
+  CodeGenIPBB->splitBasicBlock(CodeGenIP.getPoint());
+  llvm::Instruction *CodeGenIPBBTI = CodeGenIPBB->getTerminator();
+  CodeGenIPBBTI->removeFromParent();
+
+  Builder.SetInsertPoint(CodeGenIPBB);
+
+  EmitStmt(ParallelRegionBodyStmt);
+
+  Builder.Insert(CodeGenIPBBTI);
+
+  AllocaInsertPt = OldAllocaIP;
+  ReturnBlock = OldReturnBlock;
+};
+
+Builder.restoreIP(OMPBuilder->CreateParallel(Builder, BodyGenCB, PrivCB,
+ FiniCB, IfCond, NumThreads,
+ ProcBind, S.hasCancel()));
+return;
+  }
+
   // Emit parallel region as a standalone region.
   auto & = [](CodeGenFunction , PrePostActionTy ) {
 Action.Enter(CGF);
___
cfe-commits mailing list

[PATCH] D69876: Allow output constraints on "asm goto"

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Friendly ping. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876



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

[PATCH] D70289: [OpenMP][NFCI] Use the libFrontend ProcBindKind in Clang

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added reviewers: kiranchandramohan, ABataev, RaviNarayanaswamy, 
gtbercea, grokos, sdmitriev, JonChesterfield, hfinkel, fghanim.
Herald added subscribers: cfe-commits, guansong, bollu, jholewinski.
Herald added a project: clang.

This removes the OpenMPProcBindClauseKind enum in favor of
llvm::omp::ProcBindKind which lives in OpenMPConstants.h and was
introduced in D70109 .

No change in behavior is expected.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70289

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -6679,7 +6679,7 @@
 }
 
 void OMPClauseWriter::VisitOMPProcBindClause(OMPProcBindClause *C) {
-  Record.push_back(C->getProcBindKind());
+  Record.push_back(unsigned(C->getProcBindKind()));
   Record.AddSourceLocation(C->getLParenLoc());
   Record.AddSourceLocation(C->getProcBindKindKwLoc());
 }
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -12548,8 +12548,7 @@
 }
 
 void OMPClauseReader::VisitOMPProcBindClause(OMPProcBindClause *C) {
-  C->setProcBindKind(
-   static_cast(Record.readInt()));
+  C->setProcBindKind(static_cast(Record.readInt()));
   C->setLParenLoc(Record.readSourceLocation());
   C->setProcBindKindKwLoc(Record.readSourceLocation());
 }
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -1615,7 +1615,7 @@
   ///
   /// By default, performs semantic analysis to build the new OpenMP clause.
   /// Subclasses may override this routine to provide different behavior.
-  OMPClause *RebuildOMPProcBindClause(OpenMPProcBindClauseKind Kind,
+  OMPClause *RebuildOMPProcBindClause(ProcBindKind Kind,
   SourceLocation KindKwLoc,
   SourceLocation StartLoc,
   SourceLocation LParenLoc,
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -11466,9 +11466,8 @@
  ArgumentLoc, StartLoc, LParenLoc, EndLoc);
 break;
   case OMPC_proc_bind:
-Res = ActOnOpenMPProcBindClause(
-static_cast(Argument), ArgumentLoc, StartLoc,
-LParenLoc, EndLoc);
+Res = ActOnOpenMPProcBindClause(static_cast(Argument),
+ArgumentLoc, StartLoc, LParenLoc, EndLoc);
 break;
   case OMPC_atomic_default_mem_order:
 Res = ActOnOpenMPAtomicDefaultMemOrderClause(
@@ -11588,15 +11587,16 @@
   OMPDefaultClause(Kind, KindKwLoc, StartLoc, LParenLoc, EndLoc);
 }
 
-OMPClause *Sema::ActOnOpenMPProcBindClause(OpenMPProcBindClauseKind Kind,
+OMPClause *Sema::ActOnOpenMPProcBindClause(ProcBindKind Kind,
SourceLocation KindKwLoc,
SourceLocation StartLoc,
SourceLocation LParenLoc,
SourceLocation EndLoc) {
-  if (Kind == OMPC_PROC_BIND_unknown) {
+  if (Kind == OMP_PB_unknown) {
 Diag(KindKwLoc, diag::err_omp_unexpected_clause_value)
-<< getListOfPossibleValues(OMPC_proc_bind, /*First=*/0,
-   /*Last=*/OMPC_PROC_BIND_unknown)
+<< getListOfPossibleValues(OMPC_proc_bind,
+   /*First=*/unsigned(OMP_PB_master),
+   /*Last=*/unsigned(OMP_PB_spread))
 << getOpenMPClauseName(OMPC_proc_bind);
 return nullptr;
   }
Index: clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
===
--- clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
+++ clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
@@ -212,7 +212,7 @@
   /// Emit call to void __kmpc_push_proc_bind(ident_t *loc, kmp_int32
   /// global_tid, int proc_bind) to generate code for 'proc_bind' clause.
   virtual void emitProcBindClause(CodeGenFunction ,
-  

[PATCH] D70157: Align branches within 32-Byte boundary

[annita.zhang via Phabricator via cfe-commits]

annita.zhang added a comment.

In D70157#1744243 , @chandlerc wrote:

> Thanks for sending this out!
>
> I've made a minor comment on the flag structure, but my bigger question would 
> be: can you summarize the performance hit and code size hit you've seen 
> across some benchmarks? SPEC and the LLVM test suite would be nice, and maybe 
> code size alone for some large binary like Chrome, Firefox, or Safari?
>
> I'd be particularly interested in comparing the performance & code size hit 
> incurred by the suggested "fused,jcc,jmp" set compared to all (adding call, 
> ret, and indirect). If there is a big drop in either, I'd love to know which 
> of these incurs the large drop.
>
> Thanks,
> -Chandler


We are working on the performance and code size evaluation. Will update once 
it's available.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70157



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

[PATCH] D69853: [OpenMP][NFCI] Introduce llvm/IR/OpenMPConstants.h

[Francesco Petrogalli via Phabricator via cfe-commits]

fpetrogalli added inline comments.



Comment at: clang/include/clang/Basic/OpenMPKinds.h:23
 /// OpenMP directives.
-enum OpenMPDirectiveKind {
-#define OPENMP_DIRECTIVE(Name) \
-  OMPD_##Name,
-#define OPENMP_DIRECTIVE_EXT(Name, Str) \
-  OMPD_##Name,
-#include "clang/Basic/OpenMPKinds.def"
-  OMPD_unknown
-};
+using OpenMPDirectiveKind = llvm::omp::Directive;
 

I am not a fan of `using` in header files in general contexts. It's mostly a 
stylistic preference. Why not use `llvm::omp::Directive` everywhere? It is not 
much longer than the substitution...



Comment at: clang/lib/Basic/OpenMPKinds.cpp:384
 OpenMPClauseKind CKind) {
-  assert(DKind <= OMPD_unknown);
+  assert(unsigned(DKind) <= unsigned(OMPD_unknown));
   assert(CKind <= OMPC_unknown);

I really wish we would not have to add all these casts. They make the code 
ugly. I prefer `enum class` over `enum`, but if it results in this because most 
of the code uses enums, maybe is worth using just `enum`.



Comment at: clang/lib/Serialization/ASTWriter.cpp:6625
 void OMPClauseWriter::VisitOMPClauseWithPreInit(OMPClauseWithPreInit *C) {
-  Record.push_back(C->getCaptureRegion());
+  Record.push_back(uint64_t(C->getCaptureRegion()));
   Record.AddStmt(C->getPreInitStmt());

hum... is this really necessary? Why not make the type of the elements of 
`Record` the same as the type returned by `C->getCaptureregion()`?



Comment at: llvm/include/llvm/Frontend/OpenMPConstants.h:29-34
+/// Make the enum values available in the llvm::omp namespace. This allows us 
to
+/// write something like OMPD_parallel if we have a `using namespace omp`. At
+/// the same time we do not loose the strong type guarantees of the enum class,
+/// that is we cannot pass an unsigned as Directive without an explicit cast.
+#define OMP_DIRECTIVE(Enum, ...) constexpr auto Enum = omp::Directive::Enum;
+#include "llvm/Frontend/OpenMPKinds.def"

Is this really necessary? What's wrong with writing `Directive::OMPD_parallel` 
when `using namespace omp`?

Also, isn't the information provided by `OMPD` a duplicate of `omp::Directive`? 
In my mind the acronym expands to **O**pen**MP** **D**irective...

Isn't the following more concise?

```
omp::Directive::parallel
omp::Directive::declare_simd
omp::Directive::whatever
```

IMHO, less is better :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69853



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

[PATCH] D70280: Remove Support/Options.h, it is unused

[Chris Bieneman via Phabricator via cfe-commits]

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

Yea. Unfortunately the migration stalled out because there was no way to make 
the registration method work with the new pass manager.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70280



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

[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc added a comment.

> ! In D70144#1745583 , @JonasToth 
> wrote:
> 
>> ! In D70144#1745532 , 
>> @malcolm.parsons wrote:
> 
> Should `clang::format::cleanupAroundReplacements()` handle this instead?

My initial attempt did not go well. I thought perhaps adding:

  cleanupLeft(Line->First, tok::semi, tok::semi);

to clang/lib/Format/Format.cpp:1491 might do the trick. Stepping through that 
in the debugger shows that `cleanupPair` iterates over tokens on affected lines 
over the affected range. But after the newly added `default` token and 
subsequent `semi` token comes a nullptr - I could not see how to peek past the 
`default;` to see what else is on the line.

There's a comment admitting that this needs some architectural work:

  // FIXME: in the current implementation the granularity of affected range
  // is an annotated line. However, this is not sufficient. Furthermore,
  // redundant code introduced by replacements does not necessarily
  // intercept with ranges of replacements that result in the redundancy.
  // To determine if some redundant code is actually introduced by
  // replacements(e.g. deletions), we need to come up with a more
  // sophisticated way of computing affected ranges.

Agreed. Even if we could see all the tokens on a line, it seems the test case I 
added to this patch at clang-tidy/checkers/modernize-use-equals-default.cpp:50, 
where the redundant semicolon is on the next line, could never be addressed 
given the current architecture. Changing the TokenAnalyzer::analyze() interface 
would affect JavaScriptRequoter, ObjCHeaderStyleGuesser, 
JavaScriptImportSorter, Formatter, and others.

Thoughts on how to proceed?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D70144



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

[PATCH] D70284: Constant strings emitted when `-fno-constant-cfstrings` is passed should allow dead stripping

[Ben D. Jones via Phabricator via cfe-commits]

bendjones updated this revision to Diff 229441.
bendjones added a comment.

Added `"objc_arc_inert"` and updated 
`clang/test/CodeGenObjC/ns-constant-strings.m` tests to make sure the section 
is emitted as we want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70284

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/ns-constant-strings.m


Index: clang/test/CodeGenObjC/ns-constant-strings.m
===
--- clang/test/CodeGenObjC/ns-constant-strings.m
+++ clang/test/CodeGenObjC/ns-constant-strings.m
@@ -33,7 +33,14 @@
 // CHECK-NONFRAGILE: @"OBJC_CLASS_$_NSConstantString" = external global
 
 // CHECK-FRAGILE: @.str = private unnamed_addr constant [6 x i8] c"MyApp\00"
+// CHECK-FRAGILE: @_unnamed_nsstring_ = private constant 
%struct.__builtin_NSString { i32* getelementptr inbounds ([0 x i32], [0 x i32]* 
@_NSConstantStringClassReference, i32 0, i32 0), i8* getelementptr inbounds ([6 
x i8], [6 x i8]* @.str, i32 0, i32 0), i32 5 }, section 
"__OBJC,__cstring_object,regular"
 // CHECK-FRAGILE: @.str.1 = private unnamed_addr constant [7 x i8] c"MyApp1\00"
+// CHECK-FRAGILE: @_unnamed_nsstring_.2 = private constant 
%struct.__builtin_NSString { i32* getelementptr inbounds ([0 x i32], [0 x i32]* 
@_NSConstantStringClassReference, i32 0, i32 0), i8* getelementptr inbounds ([7 
x i8], [7 x i8]* @.str.1, i32 0, i32 0), i32 6 }, section 
"__OBJC,__cstring_object,regular"
 
 // CHECK-NONFRAGILE: @.str = private unnamed_addr constant [6 x i8] c"MyApp\00"
+// CHECK-NONFRAGILE: @_unnamed_nsstring_ = private constant 
%struct.__builtin_NSString { i32* bitcast (%struct._class_t* 
@"OBJC_CLASS_$_NSConstantString" to i32*), i8* getelementptr inbounds ([6 x 
i8], [6 x i8]* @.str, i32 0, i32 0), i32 5 }, section 
"__DATA,__objc_stringobj,regular"
 // CHECK-NONFRAGILE: @.str.1 = private unnamed_addr constant [7 x i8] 
c"MyApp1\00"
+// CHECK-NONFRAGILE: @_unnamed_nsstring_.2 = private constant 
%struct.__builtin_NSString { i32* bitcast (%struct._class_t* 
@"OBJC_CLASS_$_NSConstantString" to i32*), i8* getelementptr inbounds ([7 x 
i8], [7 x i8]* @.str.1, i32 0, i32 0), i32 6 }, section 
"__DATA,__objc_stringobj,regular"
+
+// CHECK-FRAGILE: attributes #0 = { "objc_arc_inert" }
+// CHECK-NONFRAGILE: attributes #0 = { "objc_arc_inert" }
\ No newline at end of file
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -1978,6 +1978,7 @@
   return V;
 }
 
+// This is only used when `-fno-constant-cfstrings` is given
 ConstantAddress
 CGObjCCommonMac::GenerateConstantNSString(const StringLiteral *Literal) {
   unsigned StringLength = 0;
@@ -2029,13 +2030,12 @@
   GV = Fields.finishAndCreateGlobal("_unnamed_nsstring_", Alignment,
 /*constant*/ true,
 llvm::GlobalVariable::PrivateLinkage);
-  const char *NSStringSection = 
"__OBJC,__cstring_object,regular,no_dead_strip";
-  const char *NSStringNonFragileABISection =
-  "__DATA,__objc_stringobj,regular,no_dead_strip";
-  // FIXME. Fix section.
+  const char *NSStringSection = "__OBJC,__cstring_object,regular";
+  const char *NSStringNonFragileABISection = "__DATA,__objc_stringobj,regular";
   GV->setSection(CGM.getLangOpts().ObjCRuntime.isNonFragile()
  ? NSStringNonFragileABISection
  : NSStringSection);
+  GV->addAttribute("objc_arc_inert");
   Entry.second = GV;
 
   return ConstantAddress(GV, Alignment);


Index: clang/test/CodeGenObjC/ns-constant-strings.m
===
--- clang/test/CodeGenObjC/ns-constant-strings.m
+++ clang/test/CodeGenObjC/ns-constant-strings.m
@@ -33,7 +33,14 @@
 // CHECK-NONFRAGILE: @"OBJC_CLASS_$_NSConstantString" = external global
 
 // CHECK-FRAGILE: @.str = private unnamed_addr constant [6 x i8] c"MyApp\00"
+// CHECK-FRAGILE: @_unnamed_nsstring_ = private constant %struct.__builtin_NSString { i32* getelementptr inbounds ([0 x i32], [0 x i32]* @_NSConstantStringClassReference, i32 0, i32 0), i8* getelementptr inbounds ([6 x i8], [6 x i8]* @.str, i32 0, i32 0), i32 5 }, section "__OBJC,__cstring_object,regular"
 // CHECK-FRAGILE: @.str.1 = private unnamed_addr constant [7 x i8] c"MyApp1\00"
+// CHECK-FRAGILE: @_unnamed_nsstring_.2 = private constant %struct.__builtin_NSString { i32* getelementptr inbounds ([0 x i32], [0 x i32]* @_NSConstantStringClassReference, i32 0, i32 0), i8* getelementptr inbounds ([7 x i8], [7 x i8]* @.str.1, i32 0, i32 0), i32 6 }, section "__OBJC,__cstring_object,regular"
 
 // CHECK-NONFRAGILE: @.str = private unnamed_addr constant [6 x i8] c"MyApp\00"
+// CHECK-NONFRAGILE: @_unnamed_nsstring_ = private constant %struct.__builtin_NSString { i32* bitcast (%struct._class_t* 

[PATCH] D70158: [analyzer] Fix Objective-C accessor body farms after D68108.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 229437.
NoQ added a comment.

I discovered another problem related to this patch, which turned out to be a 
crash (serious!), so here's an update.

We're crashing on roughly the following idiom:

  @interface I : NSObject
  @property NSObject *o;
  @end
  ​
  @implementation I
  @end
  ​
  @interface J : I
  @end
  
  @implementation J
  @synthesize o;
  @end
  
  void foo(I *i) {
i.o; // crash
  }

In this code we're using `@synthesize` to synthesize a property that was 
already declared in our superclass, without re-declaring it in our class.

Here's the respective AST:

  ObjCInterfaceDecl 0x115653598  line:3:12 I
  |-super ObjCInterface 0x7fa75fb54f78 'NSObject'
  |-ObjCImplementation 0x1156538c8 'I'
  |-ObjCPropertyDecl 0x1156536c8  col:21 o 'NSObject *' 
assign readwrite atomic unsafe_unretained
  |-ObjCMethodDecl 0x115653748  col:21 implicit - o 'NSObject *'
  `-ObjCMethodDecl 0x1156537d0  col:21 implicit - setO: 'void'
`-ParmVarDecl 0x115653858  col:21 o 'NSObject *'
  ObjCImplementationDecl 0x1156538c8  line:7:17 I
  |-ObjCInterface 0x115653598 'I'
  |-ObjCIvarDecl 0x115653950  col:21 implicit _o 'NSObject *' 
synthesize private
  `-ObjCPropertyImplDecl 0x1156539b0 <, col:21>  o 
synthesize
|-ObjCProperty 0x1156536c8 'o'
`-ObjCIvar 0x115653950 '_o' 'NSObject *'
  ObjCInterfaceDecl 0x115653bc8  line:10:12 J
  |-super ObjCInterface 0x115653598 'I'
  `-ObjCImplementation 0x115653ce0 'J'
  ObjCImplementationDecl 0x115653ce0  line:13:17 J
  |-ObjCInterface 0x115653bc8 'J'
  |-ObjCIvarDecl 0x115653d68  col:13 o 'NSObject *' synthesize 
private
  `-ObjCPropertyImplDecl 0x115653dc8  col:13 o synthesize
|-ObjCProperty 0x1156536c8 'o'
`-ObjCIvar 0x115653d68 'o' 'NSObject *'

- In the interface `I` there is an `ObjCPropertyDecl` for `o` and a pair of 
accessors. Nothing new here.
- In the implementation `I` there's an `ObjCIvarDecl` for `_o`. Nothing new 
here either.
- In the interface `J` there's basically nothing apart from the inheritance 
from `I`. In particular, there's no `ObjCPropertyDecl`.
- In the implementation `J` there's:
  - An `ObjCPropertyImplDecl` for `o`
  - An `ObjCIvarDecl` for `o` which seems to be the new ivar backing the 
synthesized property (btw why not `_o`?), and
  - A pair of accessor stubs (introduced in D68108 
).

So, basically, 2 pairs of accessors, 2 ivars, but only one property.

This roughly corresponds to the run-time behavior of this code: you can still 
access the old ivar with the ivar syntax from a method in `I`, but you cannot 
access the old ivar with property syntax even in a method of `I` - it'll 
redirect you to the new ivar instead.

In particular, invoking `ObjCMethodDecl::findPropertyDecl()` on the new stab 
causes a crash, because it's //a property accessor that is not backed by a 
property decl//.

I changed the code to try to avoid relying on the existence of a property decl 
for synthesized accessor stubs, instead attempting a brute-force lookup by name 
first.

The question still stands about how exactly do we want the 
`ObjCMethodDecl::findPropertyDecl()` method to behave in this scenario (as 
opposed to crashing). I could easily modify it to try to find the old property 
decl in the superclass, but it isn't particularly helpful for me, because it 
wouldn't allow me to find the ivar i'm looking for. I would prefer it to give 
me the `ObjCPropertyImplDecl` but it is unfortunately not an 
`ObjCPropertyDecl`. It looks like a similar problem has also arised here 
, do we want a more 
principled solution?

P.S.

> Moving that code from the static analyzer into Sema may be a good idea though.

Note that the analyzer's body farm doesn't intend to be precise; instead, it's 
expected to be a simplified AST that's easy for the static analyzer to 
understand. For example, the current body farm ignores //atomicity// because 
the static analyzer doesn't care about mutli-threaded environments; even if the 
body farm is improved, the analyzer will need to be separately taught to 
understand that more complicated AST, but the body farm was our way to avoid 
teaching the analyzer about the more complicated ASTs to begin with ("atomics 
are just functions, let's farm simpler bodies for them").


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

https://reviews.llvm.org/D70158

Files:
  clang/lib/Analysis/BodyFarm.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/Analysis/Inputs/expected-plists/nullability-notes.m.plist
  clang/test/Analysis/nullability-notes.m
  clang/test/Analysis/properties.m

Index: clang/test/Analysis/properties.m
===
--- clang/test/Analysis/properties.m
+++ clang/test/Analysis/properties.m
@@ -3,6 +3,8 @@
 
 void clang_analyzer_eval(int);
 
+#define nil ((id)0)
+
 typedef const void * CFTypeRef;
 extern CFTypeRef 

[clang] 3466ceb - Add a test to cover structural match for recursive data types

[Bruno Cardoso Lopes via cfe-commits]

Author: Bruno Cardoso Lopes
Date: 2019-11-14T18:32:27-08:00
New Revision: 3466cebe94ba461b296bb1314e76118cc2823dfb

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

LOG: Add a test to cover structural match for recursive data types

This didn't use to work prior to r370639, now that this is supported
add a testcase to prevent regressions.

rdar://problem/53602368

Added: 
clang/test/Modules/Inputs/rec-types/a.h
clang/test/Modules/Inputs/rec-types/b.h
clang/test/Modules/Inputs/rec-types/c.h
clang/test/Modules/Inputs/rec-types/module.modulemap
clang/test/Modules/structural-equivalent-recursive-types.c

Modified: 


Removed: 




diff  --git a/clang/test/Modules/Inputs/rec-types/a.h 
b/clang/test/Modules/Inputs/rec-types/a.h
new file mode 100644
index ..8e88ea7c5042
--- /dev/null
+++ b/clang/test/Modules/Inputs/rec-types/a.h
@@ -0,0 +1,2 @@
+// rec-types/a.h
+#include "b.h"

diff  --git a/clang/test/Modules/Inputs/rec-types/b.h 
b/clang/test/Modules/Inputs/rec-types/b.h
new file mode 100644
index ..05bbd99dd580
--- /dev/null
+++ b/clang/test/Modules/Inputs/rec-types/b.h
@@ -0,0 +1,2 @@
+// rec-types/b.h
+#include "c.h"

diff  --git a/clang/test/Modules/Inputs/rec-types/c.h 
b/clang/test/Modules/Inputs/rec-types/c.h
new file mode 100644
index ..fd2eb5a259b0
--- /dev/null
+++ b/clang/test/Modules/Inputs/rec-types/c.h
@@ -0,0 +1,7 @@
+struct some_descriptor
+{
+  // commenting line above make this struct work
+  void *(*thunk)(struct some_descriptor *);
+  unsigned long key;
+};
+

diff  --git a/clang/test/Modules/Inputs/rec-types/module.modulemap 
b/clang/test/Modules/Inputs/rec-types/module.modulemap
new file mode 100644
index ..680ed71ac9f2
--- /dev/null
+++ b/clang/test/Modules/Inputs/rec-types/module.modulemap
@@ -0,0 +1,9 @@
+module a {
+  header "a.h"
+  // Hide content by not re-exporting module b.
+}
+
+module b {
+  header "b.h"
+  export *
+}

diff  --git a/clang/test/Modules/structural-equivalent-recursive-types.c 
b/clang/test/Modules/structural-equivalent-recursive-types.c
new file mode 100644
index ..bb3ec7b499bb
--- /dev/null
+++ b/clang/test/Modules/structural-equivalent-recursive-types.c
@@ -0,0 +1,7 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t 
-I%S/Inputs/rec-types -fsyntax-only %s -verify
+#include "a.h"
+#include "c.h"
+void foo(struct some_descriptor *st) { (void)st->thunk; }
+
+// expected-no-diagnostics



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

[PATCH] D70285: Wrap C APIs with pragmas enforcing -Werror=strict-prototypes

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

dexonsmith added a comment.

Note that we have C files including these headers in llvm/tools/llvm-c-test and 
clang/tools/c-index-test, and I tested that dropping a `(void)` to just `()` 
triggers the error when compiling those.


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

https://reviews.llvm.org/D70285



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

[PATCH] D70284: Constant strings emitted when `-fno-constant-cfstrings` is passed should allow dead stripping

[Ben D. Jones via Phabricator via cfe-commits]

bendjones added a comment.

In D70284#1746853 , @dexonsmith wrote:

> The code looks right, but can you add a test for this in CodeGenObjC?  Also, 
> when you re-upload the patch with the tests, please use `-U999` to 
> provide full context in the diff.


Will do


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70284



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

[PATCH] D70284: Constant strings emitted when `-fno-constant-cfstrings` is passed should allow dead stripping

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

dexonsmith added a comment.

The code looks right, but can you add a test for this in CodeGenObjC?  Also, 
when you re-upload the patch with the tests, please use `-U999` to provide 
full context in the diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70284



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

[PATCH] D70285: Wrap C APIs with pragmas enforcing -Werror=strict-prototypes

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

dexonsmith created this revision.
dexonsmith added reviewers: aaron.ballman, Bigcheese, jkorous-apple.
Herald added subscribers: ributzka, arphaman, steven_wu, hiraditya, mehdi_amini.
Herald added a reviewer: deadalnix.
Herald added a project: LLVM.

Force `-Werror=strict-prototypes` so that C API tests fail to compile if
we add a non-prototype declaration.  This should help avoid regressions
like bddecba4b333f7772029b4937d2c34f9f2fda6ca was fixing.


https://reviews.llvm.org/D70285

Files:
  clang/include/clang-c/BuildSystem.h
  clang/include/clang-c/CXCompilationDatabase.h
  clang/include/clang-c/CXErrorCode.h
  clang/include/clang-c/CXString.h
  clang/include/clang-c/Documentation.h
  clang/include/clang-c/ExternC.h
  clang/include/clang-c/FatalErrorHandler.h
  clang/include/clang-c/Index.h
  clang/include/clang-c/Platform.h
  llvm/include/llvm-c/Analysis.h
  llvm/include/llvm-c/BitReader.h
  llvm/include/llvm-c/BitWriter.h
  llvm/include/llvm-c/Comdat.h
  llvm/include/llvm-c/Core.h
  llvm/include/llvm-c/DebugInfo.h
  llvm/include/llvm-c/Disassembler.h
  llvm/include/llvm-c/Error.h
  llvm/include/llvm-c/ErrorHandling.h
  llvm/include/llvm-c/ExecutionEngine.h
  llvm/include/llvm-c/ExternC.h
  llvm/include/llvm-c/IRReader.h
  llvm/include/llvm-c/Initialization.h
  llvm/include/llvm-c/LinkTimeOptimizer.h
  llvm/include/llvm-c/Linker.h
  llvm/include/llvm-c/Object.h
  llvm/include/llvm-c/OrcBindings.h
  llvm/include/llvm-c/Remarks.h
  llvm/include/llvm-c/Support.h
  llvm/include/llvm-c/Target.h
  llvm/include/llvm-c/TargetMachine.h
  llvm/include/llvm-c/Transforms/AggressiveInstCombine.h
  llvm/include/llvm-c/Transforms/Coroutines.h
  llvm/include/llvm-c/Transforms/IPO.h
  llvm/include/llvm-c/Transforms/InstCombine.h
  llvm/include/llvm-c/Transforms/PassManagerBuilder.h
  llvm/include/llvm-c/Transforms/Scalar.h
  llvm/include/llvm-c/Transforms/Utils.h
  llvm/include/llvm-c/Transforms/Vectorize.h
  llvm/include/llvm-c/Types.h
  llvm/include/llvm-c/lto.h

Index: llvm/include/llvm-c/lto.h
===
--- llvm/include/llvm-c/lto.h
+++ llvm/include/llvm-c/lto.h
@@ -16,6 +16,8 @@
 #ifndef LLVM_C_LTO_H
 #define LLVM_C_LTO_H
 
+#include "llvm-c/ExternC.h"
+
 #ifdef __cplusplus
 #include 
 #else
@@ -98,9 +100,7 @@
 /** opaque reference to a thin code generator */
 typedef struct LLVMOpaqueThinLTOCodeGenerator *thinlto_code_gen_t;
 
-#ifdef __cplusplus
-extern "C" {
-#endif
+LLVM_C_EXTERN_C_BEGIN
 
 /**
  * Returns a printable string.
@@ -900,8 +900,6 @@
  * @} // endgroup LLVMCTLTO_CACHING
  */
 
-#ifdef __cplusplus
-}
-#endif
+LLVM_C_EXTERN_C_END
 
 #endif /* LLVM_C_LTO_H */
Index: llvm/include/llvm-c/Types.h
===
--- llvm/include/llvm-c/Types.h
+++ llvm/include/llvm-c/Types.h
@@ -15,10 +15,9 @@
 #define LLVM_C_TYPES_H
 
 #include "llvm-c/DataTypes.h"
+#include "llvm-c/ExternC.h"
 
-#ifdef __cplusplus
-extern "C" {
-#endif
+LLVM_C_EXTERN_C_BEGIN
 
 /**
  * @defgroup LLVMCSupportTypes Types and Enumerations
@@ -172,8 +171,6 @@
  * @}
  */
 
-#ifdef __cplusplus
-}
-#endif
+LLVM_C_EXTERN_C_END
 
 #endif
Index: llvm/include/llvm-c/Transforms/Vectorize.h
===
--- llvm/include/llvm-c/Transforms/Vectorize.h
+++ llvm/include/llvm-c/Transforms/Vectorize.h
@@ -20,11 +20,10 @@
 #ifndef LLVM_C_TRANSFORMS_VECTORIZE_H
 #define LLVM_C_TRANSFORMS_VECTORIZE_H
 
+#include "llvm-c/ExternC.h"
 #include "llvm-c/Types.h"
 
-#ifdef __cplusplus
-extern "C" {
-#endif
+LLVM_C_EXTERN_C_BEGIN
 
 /**
  * @defgroup LLVMCTransformsVectorize Vectorization transformations
@@ -43,8 +42,6 @@
  * @}
  */
 
-#ifdef __cplusplus
-}
-#endif /* defined(__cplusplus) */
+LLVM_C_EXTERN_C_END
 
 #endif
Index: llvm/include/llvm-c/Transforms/Utils.h
===
--- llvm/include/llvm-c/Transforms/Utils.h
+++ llvm/include/llvm-c/Transforms/Utils.h
@@ -19,11 +19,10 @@
 #ifndef LLVM_C_TRANSFORMS_UTILS_H
 #define LLVM_C_TRANSFORMS_UTILS_H
 
+#include "llvm-c/ExternC.h"
 #include "llvm-c/Types.h"
 
-#ifdef __cplusplus
-extern "C" {
-#endif
+LLVM_C_EXTERN_C_BEGIN
 
 /**
  * @defgroup LLVMCTransformsUtils Transformation Utilities
@@ -45,9 +44,7 @@
  * @}
  */
 
-#ifdef __cplusplus
-}
-#endif /* defined(__cplusplus) */
+LLVM_C_EXTERN_C_END
 
 #endif
 
Index: llvm/include/llvm-c/Transforms/Scalar.h
===
--- llvm/include/llvm-c/Transforms/Scalar.h
+++ llvm/include/llvm-c/Transforms/Scalar.h
@@ -19,11 +19,10 @@
 #ifndef LLVM_C_TRANSFORMS_SCALAR_H
 #define LLVM_C_TRANSFORMS_SCALAR_H
 
+#include "llvm-c/ExternC.h"
 #include "llvm-c/Types.h"
 
-#ifdef __cplusplus
-extern "C" {
-#endif
+LLVM_C_EXTERN_C_BEGIN
 
 /**
  * @defgroup LLVMCTransformsScalar Scalar transformations
@@ -166,8 +165,6 @@
  * @}
  */
 
-#ifdef __cplusplus
-}
-#endif /* defined(__cplusplus) */

[PATCH] D70157: Align branches within 32-Byte boundary

[Xiang Zhang via Phabricator via cfe-commits]

xiangzhangllvm added a comment.

In D70157#1746793 , @MaskRay wrote:

> On x86, the preferred function alignment is 16 
> (https://github.com/llvm/llvm-project/blob/arcpatch-D70157/llvm/lib/Target/X86/X86ISelLowering.cpp#L1893),
>  which is the default function alignment in text sections. If the 
> cross-boundary decision is made with alignment=32 
> (--x86-align-branch-boundary=32) in mind, and the section alignment is still 
> 16 (not increased to 32 or higher), the linker may place the section at an 
> address which equals 16 modulo 32, the section contents will thus shift by 
> 16. The instructions that do not cross the boundary in the object files may 
> cross the boundary in the linker output. Have you considered increasing the 
> section alignment to 32?
>
> Shall we default to -mbranches-within-32B-boundaries if the specified -march= 
> or -mtune= may be affected by the erratum?


Hi Fangrui, Here will set the section alignment to 32, 
llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:658


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70157



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

[PATCH] D70284: Constant strings emitted when `-fno-constant-cfstrings` is passed should allow dead stripping

[Ben D. Jones via Phabricator via cfe-commits]

bendjones created this revision.
bendjones added reviewers: erik.pilkington, ahatanak.
bendjones created this object with edit policy "Administrators".
bendjones added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith.

Part of rdar://56643852 that makes all possible constant string code paths emit 
without `no_dead_strip` so that dead stripping is allowed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70284

Files:
  clang/lib/CodeGen/CGObjCMac.cpp


Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -1978,6 +1978,7 @@
   return V;
 }
 
+// This is only used when `-fno-constant-cfstrings` is given
 ConstantAddress
 CGObjCCommonMac::GenerateConstantNSString(const StringLiteral *Literal) {
   unsigned StringLength = 0;
@@ -2029,10 +2030,8 @@
   GV = Fields.finishAndCreateGlobal("_unnamed_nsstring_", Alignment,
 /*constant*/ true,
 llvm::GlobalVariable::PrivateLinkage);
-  const char *NSStringSection = 
"__OBJC,__cstring_object,regular,no_dead_strip";
-  const char *NSStringNonFragileABISection =
-  "__DATA,__objc_stringobj,regular,no_dead_strip";
-  // FIXME. Fix section.
+  const char *NSStringSection = "__OBJC,__cstring_object,regular";
+  const char *NSStringNonFragileABISection = "__DATA,__objc_stringobj,regular";
   GV->setSection(CGM.getLangOpts().ObjCRuntime.isNonFragile()
  ? NSStringNonFragileABISection
  : NSStringSection);


Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -1978,6 +1978,7 @@
   return V;
 }
 
+// This is only used when `-fno-constant-cfstrings` is given
 ConstantAddress
 CGObjCCommonMac::GenerateConstantNSString(const StringLiteral *Literal) {
   unsigned StringLength = 0;
@@ -2029,10 +2030,8 @@
   GV = Fields.finishAndCreateGlobal("_unnamed_nsstring_", Alignment,
 /*constant*/ true,
 llvm::GlobalVariable::PrivateLinkage);
-  const char *NSStringSection = "__OBJC,__cstring_object,regular,no_dead_strip";
-  const char *NSStringNonFragileABISection =
-  "__DATA,__objc_stringobj,regular,no_dead_strip";
-  // FIXME. Fix section.
+  const char *NSStringSection = "__OBJC,__cstring_object,regular";
+  const char *NSStringNonFragileABISection = "__DATA,__objc_stringobj,regular";
   GV->setSection(CGM.getLangOpts().ObjCRuntime.isNonFragile()
  ? NSStringNonFragileABISection
  : NSStringSection);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70157: Align branches within 32-Byte boundary

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:173
+
+  bool needAlignBranch() const;
+  bool needAlignJcc() const;

We can generalize these functions with a function that takes a parameter.



Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:411
+  cast(*Operand.getExpr()).getSymbol().getName();
+  if (SymbolName.contains("__tls_get_addr"))
+return true;

Check 64bit, then use exact comparison with either `___tls_get_addr` or 
`__tls_get_addr`



Comment at: llvm/test/MC/X86/i386-align-branch-1a.s:1
+# RUN: llvm-mc -filetype=obj -triple i386-unknown-unknown 
--x86-align-branch-boundary=32 --x86-align-branch=fused-jcc-jmp  
--x86-align-branch-prefix-size=5 %s | llvm-objdump -d  - | FileCheck %s
+# RUN: llvm-mc -filetype=obj -triple i386-unknown-unknown 
--x86-branches-within-32B-boundaries %s | llvm-objdump -d  - | FileCheck %s

MaskRay wrote:
> If you want to test that `--x86-branches-within-32B-boundaries` expands to 
> the 3 other `--x86-*` options.
> 
> ```
> ... -o %t
> ... -o %t2
> cmp %t %t2
> FileCheck --input-file %t
> ```
We need a file-level comment describing the purpose of the test.

What do `1a`, `2a`, and `5a` mean? If we can find appropriate, descriptive 
names, use them. Don't simply copy the binutils tests. If tests exist for each 
of the used instruction prefix, write a comment to make that clear.



Comment at: llvm/test/MC/X86/i386-align-branch-6a.s:10
+# CHECK:2: 89 e5   movl%esp, %ebp
+# CHECK:4: 90  nop
+# CHECK:5: 90  nop

Use something like `# CHECK-COUNT-20:90  nop` (address is intentionally 
omitted) to skip a large number of NOPs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70157



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

[PATCH] D70219: Make `-fmodule-file==` apply to .pcm file compilations

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Looks reasonable to me, but I don't know a great deal about the 
PrebuiltModuleFiles & so I'd like @rsmith to at least glance at/approve this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D70219



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

[PATCH] D70157: Align branches within 32-Byte boundary

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

On x86, the preferred function alignment is 16 
(https://github.com/llvm/llvm-project/blob/arcpatch-D70157/llvm/lib/Target/X86/X86ISelLowering.cpp#L1893),
 which is the default function alignment in text sections. If the 
cross-boundary decision is made with alignment=32 
(--x86-align-branch-boundary=32) in mind, and the section alignment is still 16 
(not increased to 32 or higher), the linker may place the section at an address 
which equals 16 modulo 32, the section contents will thus shift by 16. The 
instructions that do not cross the boundary in the object files may cross the 
boundary in the linker output. Have you considered increasing the section 
alignment to 32?

Shall we default to -mbranches-within-32B-boundaries if the specified -march= 
or -mtune= may be affected by the erratum?




Comment at: llvm/test/MC/X86/i386-align-branch-1a.s:1
+# RUN: llvm-mc -filetype=obj -triple i386-unknown-unknown 
--x86-align-branch-boundary=32 --x86-align-branch=fused-jcc-jmp  
--x86-align-branch-prefix-size=5 %s | llvm-objdump -d  - | FileCheck %s
+# RUN: llvm-mc -filetype=obj -triple i386-unknown-unknown 
--x86-branches-within-32B-boundaries %s | llvm-objdump -d  - | FileCheck %s

If you want to test that `--x86-branches-within-32B-boundaries` expands to the 
3 other `--x86-*` options.

```
... -o %t
... -o %t2
cmp %t %t2
FileCheck --input-file %t
```



Comment at: llvm/test/MC/X86/i386-align-branch-1a.s:9
+# CHECK:  foo:
+# CHECK:0: 65 65 65 a3 01 00 00 00 movl%eax, %gs:1
+# CHECK:8: 55  pushl   %ebp

Use `CHECK-NEXT:` (see https://llvm.org/docs/CommandGuide/FileCheck.html)

Add comments where REX prefixes or NOPs are used.



Comment at: llvm/test/MC/X86/i386-align-branch-1a.s:9
+# CHECK:  foo:
+# CHECK:0: 65 65 65 a3 01 00 00 00 movl%eax, %gs:1
+# CHECK:8: 55  pushl   %ebp

MaskRay wrote:
> Use `CHECK-NEXT:` (see https://llvm.org/docs/CommandGuide/FileCheck.html)
> 
> Add comments where REX prefixes or NOPs are used.
Don't mix tabs with spaces. The columns do not align in an editor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70157



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

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Hi vingeldal,

thanks for you contribution and welcome to LLVM :)
If you have any questions or the like, feel free to ask!

Best Regards, Jonas


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70265



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

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:20
+namespace {
+AST_MATCHER(VarDecl, isLocalVarDecl) { return Node.isLocalVarDecl(); }
+} // namespace

This matcher already exists either as global matcher or in some other check. 
please refactor it out.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp:39
+  << MatchedDecl;
+  diag(MatchedDecl->getLocation(), "insert 'const '", DiagnosticIDs::Note)
+  << FixItHint::CreateInsertion(MatchedDecl->getLocation(), "const ");

`const` fixing is a complicated issue. I would rather not do this now, as there 
is currently a patch in flight that does the general case.
If you really want it, there is a util for that in `clang-tidy/util/` that is 
able to do some const-fixing.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:97
 
+- New :doc:`cppcoreguidelines-avoid-non-const-global-variables
+  ` 
check.

Please describe the check with one sentence and sync that with the check 
documentation.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.rst:6
+
+FIXME: Describe what patterns does the check detect and why. Give examples.

Please adjust the documentation here as well :)



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp:3
+
+int nonConstVariable = 0;
+// CHECK-MESSAGES:  warning: variable 'nonConstVariable' is non-const and 
global [cppcoreguidelines-avoid-non-const-global-variables]

Please add test-cases for pointers and references of builtins (`int` and the 
like) and classes/structs/enums/unions, function pointers and member 
function/data pointers.
We need to consider template variables as well (i think a c++14 feature).

I am not sure about the `consteval` support in clang for c++20, but for each 
feature from a newer standard its better to have a additional test-file that 
will use this standard.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70265



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

[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In D70144#1745881 , @poelmanc wrote:

> In D70144#1745583 , @JonasToth wrote:
>
> > In D70144#1745532 , 
> > @malcolm.parsons wrote:
> >
> > > Should `clang::format::cleanupAroundReplacements()` handle this instead?
> >
> >
> > Of yes. Totally forgot that. That would probably the most elegant way :)
>
>
> Interesting, so is the advantage of that approach that //any// fixit 
> Replacement or Insertion that ends with a semicolon would have it removed if 
> a semicolon already immediately follows it? That makes sense - one less thing 
> for individual //check// developers to worry about.
>
> I can take a look at doing that.


Thinking about that: The cleanup should be called in general after clang-tidy 
runs. So it is weird, that it does not catch it.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D70144



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

[PATCH] D51567: CMake: Consolidate gtest detection code

[Tom Stellard via Phabricator via cfe-commits]

tstellar abandoned this revision.
tstellar added a comment.
Herald added a project: clang.

Dropping this since it is more Fedora specific.


Repository:
  rC Clang

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

https://reviews.llvm.org/D51567



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

[PATCH] D70157: Align branches within 32-Byte boundary

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: llvm/lib/MC/MCFragment.cpp:435
+OS << "\n   ";
+OS << "Subtype:" << MF->SubKind << "Size: " << MF->getSize();
+break;

```
error: use of overloaded operator '<<' is ambiguous (with opera
nd types 'llvm::raw_ostream' and 'llvm::MCMachineDependentFragment::SubType')
```

It seems you haven't defined `raw_ostream <<(raw_ostream , 
MCMachineDependentFragment::SubType)`. I believe you also missed a space before 
`Size: `.



Comment at: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp:88
+SmallVector BranchTypes;
+StringRef(Val).split(BranchTypes, '-', -1, false);
+for (auto BranchType : BranchTypes) {

skan wrote:
> craig.topper wrote:
> > skan wrote:
> > > craig.topper wrote:
> > > > skan wrote:
> > > > > chandlerc wrote:
> > > > > > I feel like a comma-separated list would be a bit more clear...
> > > > > We can't use comma-separated list because we need pass the option 
> > > > > with flto.  
> > > > > "-Wl,-plugin-opt=--x86-align-branch-boundary=32,--plugin-opt=-x86-align-branch=fused,jcc,jmp,--plugin-opt=-x86-align-branch-prefix-size=5"
> > > > >  would cause a compile fail because "jcc" was recognized as another 
> > > > > option rather than a part of option "-x86-align-branch=fused,jcc,jmp" 
> > > > Isn't there some way to nest quotes into the part of after -plugin-opt= 
> > > > ?
> > > I tried to use --plugin-opt=-x86-align-branch="fused,jcc,jmp", it didn't 
> > > work.
> > What if you put —plugin-opt=“-x86-align-branch=fused,jcc,jmp”
> It doesn't work too.
Both gcc and clang just split the -Wl argument by comma. There is no escape 
character. For reference, 
https://sourceware.org/ml/binutils/2019-11/msg00173.html is the GNU as patch on 
the binutils side.

Have you considered `+` plus? I think it may be more intuitive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70157



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

[PATCH] D70119: Improve gen_ast_dump_json_test.py

[Alexander Richardson via Phabricator via cfe-commits]

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



Comment at: clang/test/AST/gen_ast_dump_json_test.py:3
 
+from __future__ import print_function
 from collections import OrderedDict

aaron.ballman wrote:
> Is this needed? I don't see anything using `print_function`.
Thanks for reviewing!

Despite the mismatched name, all python 3 print features depend on this. For 
example the `sep=` and function like syntax. Python2 prints a tuple for paren 
expressions instead of treating print as a function.



Comment at: clang/test/AST/gen_ast_dump_json_test.py:11
 import re
+import tempfile
 import subprocess

aaron.ballman wrote:
> Can you keep this in alphabetical order?
Will fix before committing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70119



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

[PATCH] D70280: Remove Support/Options.h, it is unused

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.
rnk added a reviewer: beanz.
Herald added subscribers: cfe-commits, hiraditya, mgorny.
Herald added a reviewer: jfb.
Herald added projects: clang, LLVM.

It was added in 2014 in 732e0aa9fb84f1 with one use in Scalarizer.cpp.
That one use was then removed when porting to the new pass manager in
2018 in b6f76002d9158628e78.

While the RFC and the desire to get off of static initializers for
cl::opt all still stand, this code is now dead, and I think we should
delete this code until someone is ready to do the migration.

This change depends on a large mechanical cleanup of including
CommandLine.h, StringMap.h, and raw_ostream.h which I'll push shortly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70280

Files:
  clang/tools/clang-scan-deps/ClangScanDeps.cpp
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/Support/Options.h
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/DebugCounter.cpp
  llvm/lib/Support/Options.cpp
  llvm/lib/Support/Signals.cpp
  llvm/lib/Transforms/Scalar/Scalarizer.cpp

Index: llvm/lib/Transforms/Scalar/Scalarizer.cpp
===
--- llvm/lib/Transforms/Scalar/Scalarizer.cpp
+++ llvm/lib/Transforms/Scalar/Scalarizer.cpp
@@ -37,8 +37,8 @@
 #include "llvm/InitializePasses.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/MathExtras.h"
-#include "llvm/Support/Options.h"
 #include "llvm/Transforms/Scalar.h"
 #include 
 #include 
Index: llvm/lib/Support/Signals.cpp
===
--- llvm/lib/Support/Signals.cpp
+++ llvm/lib/Support/Signals.cpp
@@ -15,19 +15,19 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Config/llvm-config.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/Format.h"
-#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/FormatAdapters.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/ManagedStatic.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Mutex.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/raw_ostream.h"
-#include "llvm/Support/Options.h"
 #include 
 
 //===--===//
Index: llvm/lib/Support/Options.cpp
===
--- llvm/lib/Support/Options.cpp
+++ /dev/null
@@ -1,32 +0,0 @@
-//===- llvm/Support/Options.cpp - Debug options support -*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-//
-// This file implements the helper objects for defining debug options using the
-// new API built on cl::opt, but not requiring the use of static globals.
-//
-//===--===//
-
-#include "llvm/Support/Options.h"
-#include "llvm/Support/ManagedStatic.h"
-
-using namespace llvm;
-
-OptionRegistry::~OptionRegistry() {
-  for (auto IT = Options.begin(); IT != Options.end(); ++IT)
-delete IT->second;
-}
-
-void OptionRegistry::addOption(void *Key, cl::Option *O) {
-  assert(Options.find(Key) == Options.end() &&
- "Argument with this key already registerd");
-  Options.insert(std::make_pair(Key, O));
-}
-
-static ManagedStatic OR;
-
-OptionRegistry ::instance() { return *OR; }
Index: llvm/lib/Support/DebugCounter.cpp
===
--- llvm/lib/Support/DebugCounter.cpp
+++ llvm/lib/Support/DebugCounter.cpp
@@ -2,7 +2,6 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/ManagedStatic.h"
-#include "llvm/Support/Options.h"
 
 using namespace llvm;
 
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -116,7 +116,6 @@
   MD5.cpp
   NativeFormatting.cpp
   Optional.cpp
-  Options.cpp
   Parallel.cpp
   PluginLoader.cpp
   PrettyStackTrace.cpp
Index: llvm/include/llvm/Support/Options.h
===
--- llvm/include/llvm/Support/Options.h
+++ /dev/null
@@ -1,119 +0,0 @@
-//===- llvm/Support/Options.h - Debug options support ---*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH 

[PATCH] D68578: [HIP] Fix device stub name

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1099-1108
+auto *FD = const_cast((ND));
+if (auto *TD = cast(FD)->getPrimaryTemplate())
+  FD = TD->getTemplatedDecl();
+auto OldDeclName = FD->getDeclName();
+auto NewNameStr = std::string("__device_stub__") + 
OldDeclName.getAsString();
+auto *NewId = (NewNameStr);
+auto NewDeclName = DeclarationName(NewId);

yaxunl wrote:
> tra wrote:
> > yaxunl wrote:
> > > tra wrote:
> > > > On one hand I like this patch variant much better than the one that 
> > > > changed the mangling itself.
> > > > On the other hand this code appears to reply on implementation details. 
> > > > I.e. we're setting new name on `FD` which may or may not be the same as 
> > > > `ND`, but we're always passing `ND` to `getMangledNameImpl()`. 
> > > > 
> > > > Perhaps we could implement name-tweaking as another `MultiVersionKind` 
> > > > which we already plumb into getMangledNameImpl() and which allows 
> > > > changing the name for target attributes & features.
> > > > 
> > > > 
> > > The mangled name of an instantiated template function does not depends on 
> > > its own name, but on the template. If we do not want to depend on this 
> > > implementation detail, it seems I have to clone the template and 
> > > instantiate from the clone.
> > > 
> > > MultiVersion does not help us here since it only appends .postfix to 
> > > mangled name. The obstacle we are facing is how to change the unmangled 
> > > name.
> > > The mangled name of an instantiated template function does not depends on 
> > > its own name, but on the template. If we do not want to depend on this 
> > > implementation detail, it seems I have to clone the template and 
> > > instantiate from the clone.
> > 
> > That would be putting more effort into working around the fact that 
> > `getMangledNameImpl()` doesa not provide a good API to change the name the 
> > way you need to. *That's* what needs to be addressed, IMO.
> > 
> > >MultiVersion does not help us here since it only appends .postfix to 
> > >mangled name. The obstacle we are facing is how to change the unmangled 
> > >name.
> > 
> > *Some* existing implementations append to the mangled name, but we can do 
> > other manipulations there, too. The string with the mangled name originates 
> > in `getMangledNameImpl` and we could do more than just append to it. We do 
> > not have to use the `MultiVersion` for that, either. E.g. we prepend 
> > `__regcall3__` to the names of functions with `CC_X86RegCall` calling 
> > convention. We could do something similar for the kernel stub, I wonder if 
> > we could just generate a unique name and be done with that?
> > 
> > Hmm. Unique name probably would not do if, let's say, a kernel is defined 
> > in one TU, but we need to call it from another TU. So, whichever way we 
> > change the name of the stub, it will need to be the same everywhere. 
> > You may want to add a test verifying that launching of declaration-only 
> > kernels uses the right name.
> > 
> > Consistency of name mangling means that we do need to include regular 
> > C++-mangled information. Which means we need to do the name tweaking deeper 
> > down. How about using calling conventions? It's been suggested in the past 
> > that a lot of shenanigans around kernel launches could/should be done as a 
> > different calling convention. One of the things affected by the calling 
> > convention is mangling and we can add prefix there: 
> > https://github.com/llvm/llvm-project/blob/master/clang/lib/AST/Mangle.cpp#L164
> > 
> > We could tag host-side kernel with 'kernel call' calling convention on the 
> > host side and then plumb prefixing to be done similar to `__regcall3__`.
> > 
> > If that works that may be a useful improvement overall. For instance, we 
> > may no longer need to stash a `it's a kernel` flag among attributes and it 
> > would probably be useful for other things (e.g enforcing address space 
> > requirements for kernel pointer arguments).
> > 
> will add a test for decl only kernel. At least for the current implementation 
> I see it works. A decl of stub function with expected name is emitted and can 
> be called.
> 
> About calling conv. I've tried implementing `__global__` as a calling conv 
> before. The issue is that it is part of type system and clang enforces type 
> checking for that. e.g. you cannot assign it to an ordinary function pointer 
> unless that function pointer is also declared with the same calling 
> convention. This will cause lots of type mismatching issues. In CUDA 
> language, `__global__` is not part of type system since it is just an 
> attribute.
> 
> We could introduce a calling conv for stub, but probably we can only use use 
> it when we mangle the stub function.
> 
> 
OK. I'm fresh out of ideas.

We should add some sort of assert to make sure that the mangled name does have 
the prefix we intended to add. Also a TODO to 

[PATCH] D19031: [clang-format] Flexible line endings

[Stephan T. Lavavej via Phabricator via cfe-commits]

STL_MSFT added a comment.

MSVC's STL currently uses CRLF (DOS) line endings, not LF (Unix). I wrote a 
validator, 
https://github.com/microsoft/STL/blob/58bb49d63d92e7a0346a05af29816aeea6b4cf0f/tools/validate/validate.cpp
 , to detect LF files, mixed line endings (LF and CRLF in the same file), 
damaged endings (CR only), and enforcing one newline at the end of every file, 
because clang-format doesn't currently handle those whitespace issues.


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

https://reviews.llvm.org/D19031



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

[PATCH] D68578: [HIP] Fix device stub name

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1099-1108
+auto *FD = const_cast((ND));
+if (auto *TD = cast(FD)->getPrimaryTemplate())
+  FD = TD->getTemplatedDecl();
+auto OldDeclName = FD->getDeclName();
+auto NewNameStr = std::string("__device_stub__") + 
OldDeclName.getAsString();
+auto *NewId = (NewNameStr);
+auto NewDeclName = DeclarationName(NewId);

tra wrote:
> yaxunl wrote:
> > tra wrote:
> > > On one hand I like this patch variant much better than the one that 
> > > changed the mangling itself.
> > > On the other hand this code appears to reply on implementation details. 
> > > I.e. we're setting new name on `FD` which may or may not be the same as 
> > > `ND`, but we're always passing `ND` to `getMangledNameImpl()`. 
> > > 
> > > Perhaps we could implement name-tweaking as another `MultiVersionKind` 
> > > which we already plumb into getMangledNameImpl() and which allows 
> > > changing the name for target attributes & features.
> > > 
> > > 
> > The mangled name of an instantiated template function does not depends on 
> > its own name, but on the template. If we do not want to depend on this 
> > implementation detail, it seems I have to clone the template and 
> > instantiate from the clone.
> > 
> > MultiVersion does not help us here since it only appends .postfix to 
> > mangled name. The obstacle we are facing is how to change the unmangled 
> > name.
> > The mangled name of an instantiated template function does not depends on 
> > its own name, but on the template. If we do not want to depend on this 
> > implementation detail, it seems I have to clone the template and 
> > instantiate from the clone.
> 
> That would be putting more effort into working around the fact that 
> `getMangledNameImpl()` doesa not provide a good API to change the name the 
> way you need to. *That's* what needs to be addressed, IMO.
> 
> >MultiVersion does not help us here since it only appends .postfix to mangled 
> >name. The obstacle we are facing is how to change the unmangled name.
> 
> *Some* existing implementations append to the mangled name, but we can do 
> other manipulations there, too. The string with the mangled name originates 
> in `getMangledNameImpl` and we could do more than just append to it. We do 
> not have to use the `MultiVersion` for that, either. E.g. we prepend 
> `__regcall3__` to the names of functions with `CC_X86RegCall` calling 
> convention. We could do something similar for the kernel stub, I wonder if we 
> could just generate a unique name and be done with that?
> 
> Hmm. Unique name probably would not do if, let's say, a kernel is defined in 
> one TU, but we need to call it from another TU. So, whichever way we change 
> the name of the stub, it will need to be the same everywhere. 
> You may want to add a test verifying that launching of declaration-only 
> kernels uses the right name.
> 
> Consistency of name mangling means that we do need to include regular 
> C++-mangled information. Which means we need to do the name tweaking deeper 
> down. How about using calling conventions? It's been suggested in the past 
> that a lot of shenanigans around kernel launches could/should be done as a 
> different calling convention. One of the things affected by the calling 
> convention is mangling and we can add prefix there: 
> https://github.com/llvm/llvm-project/blob/master/clang/lib/AST/Mangle.cpp#L164
> 
> We could tag host-side kernel with 'kernel call' calling convention on the 
> host side and then plumb prefixing to be done similar to `__regcall3__`.
> 
> If that works that may be a useful improvement overall. For instance, we may 
> no longer need to stash a `it's a kernel` flag among attributes and it would 
> probably be useful for other things (e.g enforcing address space requirements 
> for kernel pointer arguments).
> 
will add a test for decl only kernel. At least for the current implementation I 
see it works. A decl of stub function with expected name is emitted and can be 
called.

About calling conv. I've tried implementing `__global__` as a calling conv 
before. The issue is that it is part of type system and clang enforces type 
checking for that. e.g. you cannot assign it to an ordinary function pointer 
unless that function pointer is also declared with the same calling convention. 
This will cause lots of type mismatching issues. In CUDA language, `__global__` 
is not part of type system since it is just an attribute.

We could introduce a calling conv for stub, but probably we can only use use it 
when we mangle the stub function.




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

https://reviews.llvm.org/D68578



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

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added inline comments.



Comment at: clang/lib/Sema/SemaExprObjC.cpp:1129
+  }
+}
+

rjmccall wrote:
> Is this a good idea to do for arbitrary diagnostics?  I feel like saying 
> "direct" in the note when that has nothing to do with the conflict could be a 
> little misleading.
fair enough. some of the mismatches will be because of directness so it can 
help in that instance but I can see how it's confusing.

fixed, and the fact that tests didn't need any updating probably proves you 
right ;)


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

https://reviews.llvm.org/D69991



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

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 229410.
MadCoder marked 2 inline comments as done.
MadCoder added a comment.

reverted the hunk about "direct methods" note.


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

https://reviews.llvm.org/D69991

Files:
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/ObjCRuntime.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclObjC.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/CodeGenObjC/direct-method.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/method-direct-properties.m
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct.m
@@ -0,0 +1,148 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+@protocol Proto
+- (void)protoMethod;  // expected-note {{previous declaration is here}}
++ (void)classProtoMethod; // expected-note {{previous declaration is here}}
+@end
+
+@protocol ProtoDirectFail
+- (void)protoMethod __attribute__((objc_direct));  // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
++ (void)classProtoMethod __attribute__((objc_direct)); // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
+@end
+
+__attribute__((objc_root_class))
+@interface Root
+- (void)rootRegular;  // expected-note {{previous declaration is here}}
++ (void)classRootRegular; // expected-note {{previous declaration is here}}
+- (void)rootDirect __attribute__((objc_direct));  // expected-note {{previous declaration is here}};
++ (void)classRootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
+- (void)otherRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherRootDirect' declared here}}
++ (void)otherClassRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherClassRootDirect' declared here}}
+- (void)notDirectInIface; // expected-note {{previous declaration is here}}
++ (void)classNotDirectInIface;// expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct_members))
+@interface Root ()
+- (void)rootExtensionDirect;  // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect; // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct_members))
+@interface Root(Direct)
+- (void)rootCategoryDirect;  // expected-note {{previous declaration is here}}
++ (void)classRootCategoryDirect; // expected-note {{previous declaration is here}}
+@end
+
+@interface Root ()
+- (void)rootExtensionRegular;   // expected-note {{previous declaration is here}}
++ (void)classRootExtensionRegular;  // expected-note {{previous declaration is here}}
+- (void)rootExtensionDirect2 __attribute__((objc_direct));  // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface Root (Direct2)
+- (void)rootCategoryRegular;   // expected-note {{previous declaration is here}}
++ (void)classRootCategoryRegular;  // expected-note {{previous declaration is here}}
+- (void)rootCategoryDirect2 __attribute__((objc_direct));  // expected-note {{previous declaration is here}}
++ (void)classRootCategoryDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_root_class, objc_direct_members)) // expected-error {{'objc_direct_members' attribute only applies to Objective-C implementation declarations and Objective-C containers}}
+@interface SubDirectFail : Root
+- (instancetype)init;
+@end
+
+@interface Sub : Root 
+/* invalid overrides with directs */
+- (void)rootRegular __attribute__((objc_direct));   // expected-error {{methods that override superclass methods cannot be direct}}
++ (void)classRootRegular __attribute__((objc_direct));  // expected-error {{methods that override superclass methods cannot be direct}}
+- 

[PATCH] D70133: [ARM, MVE] Add intrinsics for 'administrative' vector operations.

[Dave Green via Phabricator via cfe-commits]

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

Thanks. That, er, makes it slightly simpler to review.

LGTM




Comment at: clang/test/CodeGen/arm-mve-intrinsics/admin.c:1671
+//
+float16x8_t test_vuninitializedq_polymorphic_f16(float16x8_t (*funcptr)(void))
+{

simon_tatham wrote:
> dmgreen wrote:
> > I see. This is the "non-evaluating" part. It looks odd from a C perspective.
> Slightly, although it's no more odd than `sizeof` or `__typeof` for only 
> using the type of its argument and not actually evaluating it. But it's what 
> ACLE specifies!
> 
> (I think, rather like I just mentioned in D70088, that the use case is for 
> polymorphism within something like a C++ template – if you have the vector 
> type as a template parameter, this allows you to generate an uninit value of 
> the same vector type reasonably easily, without having to write your own 
> system of C++ template specializations that select the right explicitly 
> suffixed intrinsic.)
It's more like a macro than a function call. Makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70133



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

[PATCH] D70274: [clang][IFS] Driver pipeline change for clang-ifs: generate interface stubs after standard pipeline.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi created this revision.
plotfi added a reviewer: compnerd.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Up until now, clang interface stubs has replaced the standard PP -> C -> BE -> 
ASM -> LNK pipeline. With this change, it will happen in conjunction with it. 
So what when you build your code you will get an a.out or lib.so as well as an 
interface stub file.

TODO: add another test or two.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70274

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/InterfaceStubs.cpp
  clang/lib/Driver/Types.cpp
  clang/test/InterfaceStubs/driver-test.c
  clang/test/InterfaceStubs/windows.cpp

Index: clang/test/InterfaceStubs/windows.cpp
===
--- clang/test/InterfaceStubs/windows.cpp
+++ clang/test/InterfaceStubs/windows.cpp
@@ -1,6 +1,7 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -triple x86_64-windows-msvc -o - %s -emit-interface-stubs | FileCheck -check-prefix=CHECK-CC1 %s
-// RUN: %clang -target x86_64-windows-msvc -o - %s -emit-interface-stubs -emit-merged-ifs | FileCheck -check-prefix=CHECK-IFS %s
+// RUN: %clang -target x86_64-windows-msvc -o - %s -emit-interface-stubs -emit-merged-ifs -c | FileCheck -check-prefix=CHECK-IFS %s
+// note: -c is added here to prevent clang from invoking link.exe
 
 // CHECK-CC1: Symbols:
 // CHECK-CC1-NEXT: ?helloWindowsMsvc@@YAHXZ
Index: clang/test/InterfaceStubs/driver-test.c
===
--- clang/test/InterfaceStubs/driver-test.c
+++ clang/test/InterfaceStubs/driver-test.c
@@ -1,11 +1,13 @@
 // REQUIRES: x86-registered-target
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -x c -o %t1.so -emit-interface-stubs %s %S/object.c %S/weak.cpp && \
-// RUN: llvm-nm %t1.so 2>&1 | FileCheck --check-prefix=CHECK-IFS %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -x c -o %t1 -emit-interface-stubs %s %S/object.c %S/weak.cpp
+// RUN: llvm-nm %t1   2>&1 | FileCheck %s
+// RUN: llvm-nm %t1.ifso 2>&1 | FileCheck %s
 
-// CHECK-IFS-DAG: data
-// CHECK-IFS-DAG: foo
-// CHECK-IFS-DAG: strongFunc
-// CHECK-IFS-DAG: weakFunc
+// CHECK-DAG: data
+// CHECK-DAG: foo
+// CHECK-DAG: strongFunc
+// CHECK-DAG: weakFunc
 
 int foo(int bar) { return 42 + 1844; }
+int main() { return foo(23); }
Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -330,22 +330,6 @@
 llvm::copy_if(PhaseList, std::back_inserter(P),
   [](phases::ID Phase) { return Phase <= phases::Precompile; });
 
-  // Treat Interface Stubs like its own compilation mode.
-  else if (DAL.getLastArg(options::OPT_emit_interface_stubs)) {
-llvm::SmallVector IfsModePhaseList;
-llvm::SmallVector  = PhaseList;
-phases::ID LastPhase = phases::IfsMerge;
-if (Id != types::TY_IFS) {
-  if (DAL.hasArg(options::OPT_c))
-LastPhase = phases::Compile;
-  PL = IfsModePhaseList;
-  types::getCompilationPhases(types::TY_IFS_CPP, PL);
-}
-llvm::copy_if(PL, std::back_inserter(P), [&](phases::ID Phase) {
-  return Phase <= LastPhase;
-});
-  }
-
   // -{fsyntax-only,-analyze,emit-ast} only run up to the compiler.
   else if (DAL.getLastArg(options::OPT_fsyntax_only) ||
DAL.getLastArg(options::OPT_print_supported_cpus) ||
Index: clang/lib/Driver/ToolChains/InterfaceStubs.cpp
===
--- clang/lib/Driver/ToolChains/InterfaceStubs.cpp
+++ clang/lib/Driver/ToolChains/InterfaceStubs.cpp
@@ -21,11 +21,29 @@
   std::string Merger = getToolChain().GetProgramPath(getShortName());
   llvm::opt::ArgStringList CmdArgs;
   CmdArgs.push_back("-action");
-  CmdArgs.push_back(Args.getLastArg(options::OPT_emit_merged_ifs)
-? "write-ifs"
-: "write-bin");
+  const bool WriteBin = !Args.getLastArg(options::OPT_emit_merged_ifs);
+  CmdArgs.push_back(WriteBin ? "write-bin" : "write-ifs");
   CmdArgs.push_back("-o");
-  CmdArgs.push_back(Output.getFilename());
+
+  std::string OutputFilename = Output.getFilename();
+
+  // Normally we want to write to a side-car file ending in ".ifso" so for
+  // example if `clang -emit-interface-stubs -shared -o libhello.so` were
+  // invoked then we would like to get libhello.so and libhello.ifso. If the
+  // stdout stream is given as the output file (ie `-o -`), that is the one
+  // exception where we will just append to the same filestream as the normal
+  // output.
+  if (OutputFilename != "-") {
+std::string DllFileExt = ".so";
+size_t TrimedSize = OutputFilename.size() - DllFileExt.size();
+if (OutputFilename.size() > DllFileExt.size() &&
+std::equal(OutputFilename.begin() + TrimedSize, OutputFilename.end(),
+   DllFileExt.begin()))
+  OutputFilename = 

[PATCH] D70257: [BPF] Restrict preserve_access_index attribute to C only

[Yonghong Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdd16b3fe2559: [BPF] Restrict preserve_access_index attribute 
to C only (authored by yonghong-song).

Changed prior to commit:
  https://reviews.llvm.org/D70257?vs=229355=229406#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70257

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/Sema/bpf-attr-preserve-access-index.cpp


Index: clang/test/Sema/bpf-attr-preserve-access-index.cpp
===
--- /dev/null
+++ clang/test/Sema/bpf-attr-preserve-access-index.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -x c++ -triple bpf-pc-linux-gnu -dwarf-version=4 
-fsyntax-only -verify %s
+
+#define __reloc__ __attribute__((preserve_access_index))
+
+struct t1 {
+  int a;
+  int b[4];
+  int c:1;
+} __reloc__; // expected-warning {{'preserve_access_index' attribute ignored}}
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -3408,10 +3408,6 @@
   if (!ArrayBase || !CGF.getDebugInfo())
 return false;
 
-  const auto *ImplicitCast = dyn_cast(ArrayBase);
-  if (!ImplicitCast)
-return false;
-
   // Only support base as either a MemberExpr or DeclRefExpr.
   // DeclRefExpr to cover cases like:
   //struct s { int a; int b[10]; };
@@ -3419,39 +3415,24 @@
   //p[1].a
   // p[1] will generate a DeclRefExpr and p[1].a is a MemberExpr.
   // p->b[5] is a MemberExpr example.
-  const Expr *E = ImplicitCast->getSubExpr();
-  const auto *MemberCast = dyn_cast(E);
-  if (MemberCast)
-return MemberCast->getMemberDecl()->hasAttr();
-
-  const auto *DeclRefCast = dyn_cast(E);
-  if (DeclRefCast) {
-const VarDecl *VarDef = dyn_cast(DeclRefCast->getDecl());
+  const Expr *E = ArrayBase->IgnoreImpCasts();
+  if (const auto *ME = dyn_cast(E))
+return ME->getMemberDecl()->hasAttr();
+
+  if (const auto *DRE = dyn_cast(E)) {
+const auto *VarDef = dyn_cast(DRE->getDecl());
 if (!VarDef)
   return false;
 
-const auto *PtrT = dyn_cast(VarDef->getType().getTypePtr());
+const auto *PtrT = VarDef->getType()->getAs();
 if (!PtrT)
   return false;
-const auto *PointeeT = PtrT->getPointeeType().getTypePtr();
-
-// Peel off typedef's
-const auto *TypedefT = dyn_cast(PointeeT);
-while (TypedefT) {
-  PointeeT = TypedefT->desugar().getTypePtr();
-  TypedefT = dyn_cast(PointeeT);
-}
-
-// Not a typedef any more, it should be an elaborated type.
-const auto ElaborateT = dyn_cast(PointeeT);
-if (!ElaborateT)
-  return false;
 
-const auto *RecT = 
dyn_cast(ElaborateT->desugar().getTypePtr());
-if (!RecT)
-  return false;
-
-return RecT->getDecl()->hasAttr();
+const auto *PointeeT = PtrT->getPointeeType()
+ ->getUnqualifiedDesugaredType();
+if (const auto *RecT = dyn_cast(PointeeT))
+  return RecT->getDecl()->hasAttr();
+return false;
   }
 
   return false;
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1584,6 +1584,7 @@
   let Spellings = [Clang<"preserve_access_index">];
   let Subjects = SubjectList<[Record], ErrorDiag>;
   let Documentation = [BPFPreserveAccessIndexDocs];
+  let LangOpts = [COnly];
 }
 
 def WebAssemblyImportModule : InheritableAttr,


Index: clang/test/Sema/bpf-attr-preserve-access-index.cpp
===
--- /dev/null
+++ clang/test/Sema/bpf-attr-preserve-access-index.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -x c++ -triple bpf-pc-linux-gnu -dwarf-version=4 -fsyntax-only -verify %s
+
+#define __reloc__ __attribute__((preserve_access_index))
+
+struct t1 {
+  int a;
+  int b[4];
+  int c:1;
+} __reloc__; // expected-warning {{'preserve_access_index' attribute ignored}}
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -3408,10 +3408,6 @@
   if (!ArrayBase || !CGF.getDebugInfo())
 return false;
 
-  const auto *ImplicitCast = dyn_cast(ArrayBase);
-  if (!ImplicitCast)
-return false;
-
   // Only support base as either a MemberExpr or DeclRefExpr.
   // DeclRefExpr to cover cases like:
   //struct s { int a; int b[10]; };
@@ -3419,39 +3415,24 @@
   //p[1].a
   // p[1] will generate a DeclRefExpr and p[1].a is a MemberExpr.
   // p->b[5] is a MemberExpr example.
-  const Expr *E = ImplicitCast->getSubExpr();
-  const auto *MemberCast = dyn_cast(E);
-  if (MemberCast)
-return MemberCast->getMemberDecl()->hasAttr();
-
-  const auto *DeclRefCast = dyn_cast(E);
-  if 

[PATCH] D70257: [BPF] Restrict preserve_access_index attribute to C only

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song marked an inline comment as done.
yonghong-song added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:3431
 
-const auto *RecT = 
dyn_cast(ElaborateT->desugar().getTypePtr());
-if (!RecT)
-  return false;
-
-return RecT->getDecl()->hasAttr();
+const auto *PointeeT = PtrT->getPointeeType().getTypePtr()
+ ->getUnqualifiedDesugaredType();

aaron.ballman wrote:
> You can drop the `getTypePtr()` here and just use the magic `->` overload.
Thanks! This indeed better:
```
+const auto *PointeeT = PtrT->getPointeeType()
+ ->getUnqualifiedDesugaredType();
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70257



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

[clang] dd16b3f - [BPF] Restrict preserve_access_index attribute to C only

[Yonghong Song via cfe-commits]

Author: Yonghong Song
Date: 2019-11-14T14:14:59-08:00
New Revision: dd16b3fe2559789adcdd7d4d0bfe2796897877a3

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

LOG: [BPF] Restrict preserve_access_index attribute to C only

This patch is a follow-up for commit 4e2ce228ae79
  [BPF] Add preserve_access_index attribute for record definition
to restrict attribute for C only. A new test case is added
to check for this restriction.

Additional code polishing is done based on
Aaron Ballman's suggestion in https://reviews.llvm.org/D69759/new/.

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

Added: 
clang/test/Sema/bpf-attr-preserve-access-index.cpp

Modified: 
clang/include/clang/Basic/Attr.td
clang/lib/CodeGen/CGExpr.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index 91140c0ddf17..ce7ad2ceaac2 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1584,6 +1584,7 @@ def BPFPreserveAccessIndex : InheritableAttr,
   let Spellings = [Clang<"preserve_access_index">];
   let Subjects = SubjectList<[Record], ErrorDiag>;
   let Documentation = [BPFPreserveAccessIndexDocs];
+  let LangOpts = [COnly];
 }
 
 def WebAssemblyImportModule : InheritableAttr,

diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index e32db8c56df9..384e8f72a619 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -3408,10 +3408,6 @@ static bool IsPreserveAIArrayBase(CodeGenFunction , 
const Expr *ArrayBase) {
   if (!ArrayBase || !CGF.getDebugInfo())
 return false;
 
-  const auto *ImplicitCast = dyn_cast(ArrayBase);
-  if (!ImplicitCast)
-return false;
-
   // Only support base as either a MemberExpr or DeclRefExpr.
   // DeclRefExpr to cover cases like:
   //struct s { int a; int b[10]; };
@@ -3419,39 +3415,24 @@ static bool IsPreserveAIArrayBase(CodeGenFunction , 
const Expr *ArrayBase) {
   //p[1].a
   // p[1] will generate a DeclRefExpr and p[1].a is a MemberExpr.
   // p->b[5] is a MemberExpr example.
-  const Expr *E = ImplicitCast->getSubExpr();
-  const auto *MemberCast = dyn_cast(E);
-  if (MemberCast)
-return MemberCast->getMemberDecl()->hasAttr();
-
-  const auto *DeclRefCast = dyn_cast(E);
-  if (DeclRefCast) {
-const VarDecl *VarDef = dyn_cast(DeclRefCast->getDecl());
+  const Expr *E = ArrayBase->IgnoreImpCasts();
+  if (const auto *ME = dyn_cast(E))
+return ME->getMemberDecl()->hasAttr();
+
+  if (const auto *DRE = dyn_cast(E)) {
+const auto *VarDef = dyn_cast(DRE->getDecl());
 if (!VarDef)
   return false;
 
-const auto *PtrT = dyn_cast(VarDef->getType().getTypePtr());
+const auto *PtrT = VarDef->getType()->getAs();
 if (!PtrT)
   return false;
-const auto *PointeeT = PtrT->getPointeeType().getTypePtr();
-
-// Peel off typedef's
-const auto *TypedefT = dyn_cast(PointeeT);
-while (TypedefT) {
-  PointeeT = TypedefT->desugar().getTypePtr();
-  TypedefT = dyn_cast(PointeeT);
-}
-
-// Not a typedef any more, it should be an elaborated type.
-const auto ElaborateT = dyn_cast(PointeeT);
-if (!ElaborateT)
-  return false;
 
-const auto *RecT = 
dyn_cast(ElaborateT->desugar().getTypePtr());
-if (!RecT)
-  return false;
-
-return RecT->getDecl()->hasAttr();
+const auto *PointeeT = PtrT->getPointeeType()
+ ->getUnqualifiedDesugaredType();
+if (const auto *RecT = dyn_cast(PointeeT))
+  return RecT->getDecl()->hasAttr();
+return false;
   }
 
   return false;

diff  --git a/clang/test/Sema/bpf-attr-preserve-access-index.cpp 
b/clang/test/Sema/bpf-attr-preserve-access-index.cpp
new file mode 100644
index ..4a88ec2eda3c
--- /dev/null
+++ b/clang/test/Sema/bpf-attr-preserve-access-index.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -x c++ -triple bpf-pc-linux-gnu -dwarf-version=4 
-fsyntax-only -verify %s
+
+#define __reloc__ __attribute__((preserve_access_index))
+
+struct t1 {
+  int a;
+  int b[4];
+  int c:1;
+} __reloc__; // expected-warning {{'preserve_access_index' attribute ignored}}



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

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Chi Chun Chen via Phabricator via cfe-commits]

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



Comment at: clang/test/OpenMP/target_defaultmap_codegen.cpp:1538-1546
+void declare_target_link(float A)
+{
+  // CK26: [[AADDR:%.+]] = alloca float*
+  // CK26: store float* [[A:%.+]], float** [[AADDR]]
+  // CK26: [[ZERO:%.+]] = load float*, float** [[AADDR]]
+  // CK26: [[ONE:%.+]] = load float, float* [[ZERO]]
+#pragma omp target defaultmap(none:aggregate) map(A)

ABataev wrote:
> What does this test check? It is not the test for declare target link. Also, 
> A is calar and defaultmap is for aggregates.
My bad, I forgot to update from target to(Vector) to target link(Vector). And 
my intention for the aggregate is for Vector[1024] not for A. The confusion is 
due to my negligence, I'll fix it right away. Thanks for your passion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/test/OpenMP/target_defaultmap_codegen.cpp:1538-1546
+void declare_target_link(float A)
+{
+  // CK26: [[AADDR:%.+]] = alloca float*
+  // CK26: store float* [[A:%.+]], float** [[AADDR]]
+  // CK26: [[ZERO:%.+]] = load float*, float** [[AADDR]]
+  // CK26: [[ONE:%.+]] = load float, float* [[ZERO]]
+#pragma omp target defaultmap(none:aggregate) map(A)

What does this test check? It is not the test for declare target link. Also, A 
is calar and defaultmap is for aggregates.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D70101: [X86] Fix the implementation of __readcr3/__writecr3 to work in 64-bit mode

[Craig Topper via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3cec2a17de74: [X86] Fix the implementation of 
__readcr3/__writecr3 to work in 64-bit mode (authored by craig.topper).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70101

Files:
  clang/lib/Headers/intrin.h
  clang/test/Headers/ms-intrin.cpp


Index: clang/test/Headers/ms-intrin.cpp
===
--- clang/test/Headers/ms-intrin.cpp
+++ clang/test/Headers/ms-intrin.cpp
@@ -56,12 +56,8 @@
   __nop();
   __readmsr(0);
 
-  // FIXME: Call these in 64-bit too once the intrinsics have been fixed to
-  // work there, PR19301
-#ifndef _M_X64
   __readcr3();
   __writecr3(0);
-#endif
 
 #ifdef _M_ARM
   __dmb(_ARM_BARRIER_ISHST);
Index: clang/lib/Headers/intrin.h
===
--- clang/lib/Headers/intrin.h
+++ clang/lib/Headers/intrin.h
@@ -36,6 +36,12 @@
 /* Define the default attributes for the functions in this file. */
 #define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__))
 
+#if __x86_64__
+#define __LPTRINT_TYPE__ __int64
+#else
+#define __LPTRINT_TYPE__ long
+#endif
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -94,8 +100,7 @@
 void __outwordstring(unsigned short, unsigned short *, unsigned long);
 unsigned long __readcr0(void);
 unsigned long __readcr2(void);
-static __inline__
-unsigned long __readcr3(void);
+unsigned __LPTRINT_TYPE__ __readcr3(void);
 unsigned long __readcr4(void);
 unsigned long __readcr8(void);
 unsigned int __readdr(unsigned int);
@@ -132,7 +137,7 @@
 void __wbinvd(void);
 void __writecr0(unsigned int);
 static __inline__
-void __writecr3(unsigned int);
+void __writecr3(unsigned __INTPTR_TYPE__);
 void __writecr4(unsigned int);
 void __writecr8(unsigned int);
 void __writedr(unsigned int, unsigned int);
@@ -565,24 +570,26 @@
   __asm__ ("rdmsr" : "=d"(__edx), "=a"(__eax) : "c"(__register));
   return (((unsigned __int64)__edx) << 32) | (unsigned __int64)__eax;
 }
+#endif
 
-static __inline__ unsigned long __DEFAULT_FN_ATTRS
+static __inline__ unsigned __LPTRINT_TYPE__ __DEFAULT_FN_ATTRS
 __readcr3(void) {
-  unsigned long __cr3_val;
-  __asm__ __volatile__ ("mov %%cr3, %0" : "=q"(__cr3_val) : : "memory");
+  unsigned __LPTRINT_TYPE__ __cr3_val;
+  __asm__ __volatile__ ("mov %%cr3, %0" : "=r"(__cr3_val) : : "memory");
   return __cr3_val;
 }
 
 static __inline__ void __DEFAULT_FN_ATTRS
-__writecr3(unsigned int __cr3_val) {
-  __asm__ ("mov %0, %%cr3" : : "q"(__cr3_val) : "memory");
+__writecr3(unsigned __INTPTR_TYPE__ __cr3_val) {
+  __asm__ ("mov %0, %%cr3" : : "r"(__cr3_val) : "memory");
 }
-#endif
 
 #ifdef __cplusplus
 }
 #endif
 
+#undef __LPTRINT_TYPE__
+
 #undef __DEFAULT_FN_ATTRS
 
 #endif /* __INTRIN_H */


Index: clang/test/Headers/ms-intrin.cpp
===
--- clang/test/Headers/ms-intrin.cpp
+++ clang/test/Headers/ms-intrin.cpp
@@ -56,12 +56,8 @@
   __nop();
   __readmsr(0);
 
-  // FIXME: Call these in 64-bit too once the intrinsics have been fixed to
-  // work there, PR19301
-#ifndef _M_X64
   __readcr3();
   __writecr3(0);
-#endif
 
 #ifdef _M_ARM
   __dmb(_ARM_BARRIER_ISHST);
Index: clang/lib/Headers/intrin.h
===
--- clang/lib/Headers/intrin.h
+++ clang/lib/Headers/intrin.h
@@ -36,6 +36,12 @@
 /* Define the default attributes for the functions in this file. */
 #define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__))
 
+#if __x86_64__
+#define __LPTRINT_TYPE__ __int64
+#else
+#define __LPTRINT_TYPE__ long
+#endif
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -94,8 +100,7 @@
 void __outwordstring(unsigned short, unsigned short *, unsigned long);
 unsigned long __readcr0(void);
 unsigned long __readcr2(void);
-static __inline__
-unsigned long __readcr3(void);
+unsigned __LPTRINT_TYPE__ __readcr3(void);
 unsigned long __readcr4(void);
 unsigned long __readcr8(void);
 unsigned int __readdr(unsigned int);
@@ -132,7 +137,7 @@
 void __wbinvd(void);
 void __writecr0(unsigned int);
 static __inline__
-void __writecr3(unsigned int);
+void __writecr3(unsigned __INTPTR_TYPE__);
 void __writecr4(unsigned int);
 void __writecr8(unsigned int);
 void __writedr(unsigned int, unsigned int);
@@ -565,24 +570,26 @@
   __asm__ ("rdmsr" : "=d"(__edx), "=a"(__eax) : "c"(__register));
   return (((unsigned __int64)__edx) << 32) | (unsigned __int64)__eax;
 }
+#endif
 
-static __inline__ unsigned long __DEFAULT_FN_ATTRS
+static __inline__ unsigned __LPTRINT_TYPE__ __DEFAULT_FN_ATTRS
 __readcr3(void) {
-  unsigned long __cr3_val;
-  __asm__ __volatile__ ("mov %%cr3, %0" : "=q"(__cr3_val) : : "memory");
+  unsigned __LPTRINT_TYPE__ __cr3_val;
+  __asm__ __volatile__ ("mov %%cr3, %0" : "=r"(__cr3_val) : : "memory");
   return 

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc marked an inline comment as not done.
poelmanc added a comment.

In D67460#1737529 , @aaron.ballman 
wrote:

> I'm a bit worried that this manual parsing technique will need fixing again 
> in the future, but I think this is at least a reasonable incremental 
> improvement.


Thanks and I agree. Your comments encouraged me to take another stab at 
improving things. See D70270 , a whole new 
approach that removes manual parsing in favor of AST processing and 
successfully converts many more cases from `typedef` to `using`.

I didn't want to clobber this patch with that one in case any fatal flaws are 
discovered with the new approach. We can commit this one first - getting 
improved handling of commas and getting additional test cases - and treat 
D70270  as that next incremental improvement. 
Or we can keep this on hold until D70270  is 
worked out.

Thanks again for all your feedback and help!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D67460



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

[clang] 3cec2a1 - [X86] Fix the implementation of __readcr3/__writecr3 to work in 64-bit mode

[Craig Topper via cfe-commits]

Author: Craig Topper
Date: 2019-11-14T13:21:36-08:00
New Revision: 3cec2a17de744900401c83aedb442e2acc1f23f8

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

LOG: [X86] Fix the implementation of __readcr3/__writecr3 to work in 64-bit mode

We need to use a 64-bit type in 64-bit mode so a 64-bit register
will get used in the generated assembly. I've also changed the
constraints to just use "r" intead of "q". "q" forces to a only
an a/b/c/d register in 32-bit mode, but I see no reason that
would matter here.

Fixes Nico's note in PR19301 over 4 years ago.

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

Added: 


Modified: 
clang/lib/Headers/intrin.h
clang/test/Headers/ms-intrin.cpp

Removed: 




diff  --git a/clang/lib/Headers/intrin.h b/clang/lib/Headers/intrin.h
index 9786ba147fca..a73a576bc7a3 100644
--- a/clang/lib/Headers/intrin.h
+++ b/clang/lib/Headers/intrin.h
@@ -36,6 +36,12 @@
 /* Define the default attributes for the functions in this file. */
 #define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__))
 
+#if __x86_64__
+#define __LPTRINT_TYPE__ __int64
+#else
+#define __LPTRINT_TYPE__ long
+#endif
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -94,8 +100,7 @@ void __outword(unsigned short, unsigned short);
 void __outwordstring(unsigned short, unsigned short *, unsigned long);
 unsigned long __readcr0(void);
 unsigned long __readcr2(void);
-static __inline__
-unsigned long __readcr3(void);
+unsigned __LPTRINT_TYPE__ __readcr3(void);
 unsigned long __readcr4(void);
 unsigned long __readcr8(void);
 unsigned int __readdr(unsigned int);
@@ -132,7 +137,7 @@ void __vmx_vmptrst(unsigned __int64 *);
 void __wbinvd(void);
 void __writecr0(unsigned int);
 static __inline__
-void __writecr3(unsigned int);
+void __writecr3(unsigned __INTPTR_TYPE__);
 void __writecr4(unsigned int);
 void __writecr8(unsigned int);
 void __writedr(unsigned int, unsigned int);
@@ -565,24 +570,26 @@ __readmsr(unsigned long __register) {
   __asm__ ("rdmsr" : "=d"(__edx), "=a"(__eax) : "c"(__register));
   return (((unsigned __int64)__edx) << 32) | (unsigned __int64)__eax;
 }
+#endif
 
-static __inline__ unsigned long __DEFAULT_FN_ATTRS
+static __inline__ unsigned __LPTRINT_TYPE__ __DEFAULT_FN_ATTRS
 __readcr3(void) {
-  unsigned long __cr3_val;
-  __asm__ __volatile__ ("mov %%cr3, %0" : "=q"(__cr3_val) : : "memory");
+  unsigned __LPTRINT_TYPE__ __cr3_val;
+  __asm__ __volatile__ ("mov %%cr3, %0" : "=r"(__cr3_val) : : "memory");
   return __cr3_val;
 }
 
 static __inline__ void __DEFAULT_FN_ATTRS
-__writecr3(unsigned int __cr3_val) {
-  __asm__ ("mov %0, %%cr3" : : "q"(__cr3_val) : "memory");
+__writecr3(unsigned __INTPTR_TYPE__ __cr3_val) {
+  __asm__ ("mov %0, %%cr3" : : "r"(__cr3_val) : "memory");
 }
-#endif
 
 #ifdef __cplusplus
 }
 #endif
 
+#undef __LPTRINT_TYPE__
+
 #undef __DEFAULT_FN_ATTRS
 
 #endif /* __INTRIN_H */

diff  --git a/clang/test/Headers/ms-intrin.cpp 
b/clang/test/Headers/ms-intrin.cpp
index 18bb79820378..d3b6a1a278da 100644
--- a/clang/test/Headers/ms-intrin.cpp
+++ b/clang/test/Headers/ms-intrin.cpp
@@ -56,12 +56,8 @@ void f() {
   __nop();
   __readmsr(0);
 
-  // FIXME: Call these in 64-bit too once the intrinsics have been fixed to
-  // work there, PR19301
-#ifndef _M_X64
   __readcr3();
   __writecr3(0);
-#endif
 
 #ifdef _M_ARM
   __dmb(_ARM_BARRIER_ISHST);



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

[PATCH] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc created this revision.
poelmanc added reviewers: alexfh, aaron.ballman.
poelmanc added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, mgehre.
Herald added a project: clang.

In D67460#1737529 , @aaron.ballman 
wrote:

> I'm a bit worried that this manual parsing technique will need fixing again 
> in the future


That concern plus prior comments and help from `clang -ast-dump` led me to 
develop this new approach based entirely on the AST that completely eliminates 
manual parsing. It now handles `typedef`s that include comma-separated multiple 
types, and handles embedded `struct` definitions, which previously could not be 
automatically converted.

For example, with this patch `modernize-use-using` now can convert:

  typedef struct { int a; } R_t, *R_p;

to:

  using R_t = struct { int a; }; using R_p = R_t*;

`-ast-dump` showed that the `CXXRecordDecl` definitions and multiple 
`TypedefDecl`s come consecutively in the tree, so `check()` stores information 
between calls to determine when it is receiving a second or additional 
`TypedefDecl` within a single `typedef`, or when the current `TypedefDecl` 
refers to an embedded `CXXRecordDecl` like a `struct`.

This patch is independent of D67460  except 
for borrowing its extended set of tests. All those tests pass, plus several 
that previously remained as `typedef` are now modernized to use `using`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D70270

Files:
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-using.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
  clang/include/clang/Basic/SourceLocation.h

Index: clang/include/clang/Basic/SourceLocation.h
===
--- clang/include/clang/Basic/SourceLocation.h
+++ clang/include/clang/Basic/SourceLocation.h
@@ -192,6 +192,18 @@
   return LHS.getRawEncoding() < RHS.getRawEncoding();
 }
 
+inline bool operator>(const SourceLocation , const SourceLocation ) {
+  return LHS.getRawEncoding() > RHS.getRawEncoding();
+}
+
+inline bool operator<=(const SourceLocation , const SourceLocation ) {
+  return !(LHS > RHS);
+}
+
+inline bool operator>=(const SourceLocation , const SourceLocation ) {
+  return !(LHS < RHS);
+}
+
 /// A trivial tuple used to represent a source range.
 class SourceRange {
   SourceLocation B;
@@ -219,6 +231,11 @@
 return B != X.B || E != X.E;
   }
 
+  // Returns true iff other is wholly contained within this range.
+  bool contains(const SourceRange ) const {
+return B <= other.B && E >= other.E;
+  }
+
   void print(raw_ostream , const SourceManager ) const;
   std::string printToString(const SourceManager ) const;
   void dump(const SourceManager ) const;
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -84,7 +84,9 @@
 
 typedef int bla1, bla2, bla3;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef int bla1, bla2, bla3;
+// CHECK-MESSAGES: :[[@LINE-2]]:17: warning: use 'using' instead of 'typedef'
+// CHECK-MESSAGES: :[[@LINE-3]]:23: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using bla1 = int; using bla2 = int; using bla3 = int;
 
 #define CODE typedef int INT
 
@@ -136,16 +138,16 @@
 
 typedef struct Q1 { int a; } S1;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef struct Q1 { int a; } S1;
+// CHECK-FIXES: using S1 = struct Q1 { int a; };
 typedef struct { int b; } S2;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef struct { int b; } S2;
+// CHECK-FIXES: using S2 = struct { int b; };
 struct Q2 { int c; } typedef S3;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: struct Q2 { int c; } typedef S3;
+// CHECK-FIXES: using S3 = struct Q2 { int c; };
 struct { int d; } typedef S4;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: struct { int d; } typedef S4;
+// CHECK-FIXES: using S4 = struct { int d; };
 
 namespace my_space {
   class my_cclass {};
@@ -183,3 +185,77 @@
   void f() override { super::f(); }
 };
 }
+
+template 
+class TwoArgTemplate {
+  typedef TwoArgTemplate self;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
+  // CHECK-FIXES: using self = TwoArgTemplate;
+};
+
+template 
+struct S {};
+
+typedef S<(0 > 0), int> S_t, *S_p;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: 

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc added a comment.

In D70165#1745530 , @alexfh wrote:

> While I have no objections against this patch, I wonder whether someone had a 
> chance to ask GCC developers about this? Is it a conscious choice to suggest 
> `override` when `final` is present? What's the argument for doing so?


Thanks, someone should ask them as I believe this issue extends beyond 
clang-tidy: code with functions marked `final` //cannot// satisfy both `gcc 
-Wsuggest-override` and `clang -Winconsistent-missing-override`; `gcc` demands 
`override final` and `clang` demands just `final`.

Even if `clang` and `gcc` find a common ground, people will be stuck with 
current versions for quite a while, so this clang-tidy patch should prove 
helpful.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D70165



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

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese created this revision.
Herald added subscribers: cfe-commits, tschuett, dexonsmith, mgrang.
Herald added a project: clang.
Bigcheese added reviewers: arphaman, kousikk.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70268

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/Inputs/modules_cdb.json
  clang/test/ClangScanDeps/modules-full.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -13,6 +13,7 @@
 #include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
 #include "clang/Tooling/JSONCompilationDatabase.h"
 #include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/Options.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
@@ -130,9 +131,10 @@
 /// based on the result.
 ///
 /// \returns True on error.
-static bool handleDependencyToolResult(const std::string ,
-   llvm::Expected ,
-   SharedStream , SharedStream ) {
+static bool
+handleMakeDependencyToolResult(const std::string ,
+   llvm::Expected ,
+   SharedStream , SharedStream ) {
   if (!MaybeFile) {
 llvm::handleAllErrors(
 MaybeFile.takeError(), [, ](llvm::StringError ) {
@@ -147,6 +149,141 @@
   return false;
 }
 
+static llvm::json::Array toJSONSorted(const llvm::StringSet<> ) {
+  std::vector Strings;
+  for (auto & : Set)
+Strings.push_back(I.getKey());
+  std::sort(Strings.begin(), Strings.end());
+  return llvm::json::Array(Strings);
+}
+
+// Thread safe.
+class FullDeps {
+public:
+  void mergeDeps(StringRef Input, FullDependencies FD, size_t InputIndex) {
+InputDeps ID;
+ID.FileName = Input;
+ID.ContextHash = std::move(FD.ContextHash);
+ID.FileDeps = std::move(FD.DirectFileDependencies);
+ID.ModuleDeps = std::move(FD.DirectModuleDependencies);
+ID.OutputPaths = std::move(FD.OutputPaths);
+
+std::unique_lock ul(Lock);
+for (ModuleDeps  : FD.ClangModuleDeps) {
+  auto I = Modules.find({MD.ContextHash, MD.ModuleName, 0});
+  if (I != Modules.end()) {
+I->first.InputIndex = std::min(I->first.InputIndex, InputIndex);
+continue;
+  }
+  Modules.insert(
+  I, {{MD.ContextHash, MD.ModuleName, InputIndex}, std::move(MD)});
+}
+
+Inputs.push_back(std::move(ID));
+  }
+
+  void printFullOutput(raw_ostream ) {
+// Sort the modules by name to get a deterministic order.
+std::vector ModuleNames;
+for (auto & : Modules)
+  ModuleNames.push_back(M.first);
+std::sort(ModuleNames.begin(), ModuleNames.end(),
+  [](const ContextModulePair , const ContextModulePair ) {
+return std::tie(A.ModuleName, A.InputIndex) <
+   std::tie(B.ModuleName, B.InputIndex);
+  });
+
+std::sort(Inputs.begin(), Inputs.end(),
+  [](const InputDeps , const InputDeps ) {
+return std::tie(A.FileName, A.OutputPaths) <
+   std::tie(B.FileName, B.OutputPaths);
+  });
+
+using namespace llvm::json;
+
+Array OutModules;
+for (auto & : ModuleNames) {
+  auto  = Modules[ModName];
+  Object O{
+  {"name", MD.ModuleName},
+  {"context-hash", MD.ContextHash},
+  {"file-deps", toJSONSorted(MD.FileDeps)},
+  {"clang-module-deps", toJSONSorted(MD.ClangModuleDeps)},
+  {"clang-modulemap-file", MD.ClangModuleMapFile},
+  };
+  OutModules.push_back(std::move(O));
+}
+
+Array TUs;
+for (auto & : Inputs) {
+  Object O{
+  {"input-file", I.FileName},
+  {"clang-context-hash", I.ContextHash},
+  {"file-deps", I.FileDeps},
+  {"clang-module-deps", I.ModuleDeps},
+  {"output-files", I.OutputPaths},
+  };
+  TUs.push_back(std::move(O));
+}
+
+Object Output{
+{"modules", std::move(OutModules)},
+{"translation-units", std::move(TUs)},
+};
+
+OS << llvm::formatv("{0:2}\n", Value(std::move(Output)));
+  }
+
+private:
+  struct ContextModulePair {
+std::string ContextHash;
+std::string ModuleName;
+mutable size_t InputIndex;
+
+bool operator==(const ContextModulePair ) const {
+  return ContextHash == Other.ContextHash && ModuleName == Other.ModuleName;
+}
+  };
+
+  struct ContextModulePairHasher {
+std::size_t 

[PATCH] D62686: [RISCV] Add support for save/restore of callee-saved registers via libcalls

[Pengxuan Zheng via Phabricator via cfe-commits]

pzheng added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:278
   // Add CFI directives for callee-saved registers.
-  const std::vector  = MFI.getCalleeSavedInfo();
-  // Iterate over list of callee-saved registers and emit .cfi_restore
-  // directives.
-  for (const auto  : CSI) {
-Register Reg = Entry.getReg();
-unsigned CFIIndex = MF.addFrameInst(MCCFIInstruction::createRestore(
-nullptr, RI->getDwarfRegNum(Reg, true)));
-BuildMI(MBB, MBBI, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
-.addCFIIndex(CFIIndex);
+  if (!CSI.empty()) {
+// Iterate over list of callee-saved registers and emit .cfi_restore

lewis-revill wrote:
> lenary wrote:
> > Nit: You shouldn't need this `if` statement - the for loop just won't 
> > execute if CSI is empty, surely.
> Good catch, thanks!
Any chance this comment from @lenary is missed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62686



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

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D70172#1745998 , @rnk wrote:

> Are we sure using both Itanium and MS C++ ABIs at the same time is really the 
> best way forward here? What are the constraints on CUDA that require the 
> Itanium ABI? I'm sure there are real reasons you can't just use the MS ABI as 
> is, but I'm curious what they are. Was there some RFC or design showing that 
> this is the right way forward?
>
> I wonder if it would be more productive to add new, more expansive 
> attributes, similar to `__attribute__((ms_struct))`, that tag class or 
> function decls as MS or Itanium C++ ABI. CUDA could then leverage this as 
> needed, and it would be much easier to construct test cases for MS/Itanium 
> interop. This is an expansion in scope, but it seems like it could be 
> generally useful, and if we're already going to enter the crazy world of 
> multiple C++ ABIs in a single TU, we might as well bite the bullet and do it 
> in a way that isn't specific to CUDA.


We are not using Itanium ABI when we do host compilation of CUDA/HIP on 
windows. During the host compilation on windows only MS C++ ABI is used.

This issue is not due to mixing MS ABI with Itanium ABI.

This issue arises from the delayed diagnostics for CUDA/HIP. Basically we do 
not want to emit certain diagnostics (e.g. error in inline assembly code) in 
`__host__` `__device__` functions to avoid clutter. We only want to emit such 
diagnostics once we are certain these functions will be emitted in IR.

To implement this, clang maintains a call graph. For each reference to a 
function, clang checks the current context. If it is evaluating context and it 
is a function, clang assumes the referenced function is callee and its context 
is the caller. Clang checks if the caller is known to be emitted (if it has 
body and external linkage). If not, clang adds this caller/callee pair to the 
call graph. If the caller is known to be emitted, clang will check if the 
callee is known to be emitted. If so, do nothing. If the callee is not known to 
be emitted, clang will eliminate it and all its callee from the call graph, and 
emits the delayed diagnostics associated with them.

You can see a caller is added to the call graph only if it is not known to be 
emitted. Therefore clang has an assert that if a callee is known to be emitted, 
it should not be in the call graph.

On windows, when vtable is known to be emitted for a class, clang does a body 
check for dtor of the class. It makes the dtor as the context, then checks the 
dtor. I think it is to emulate the situation that a deleting dtor is calling a 
normal dtor. This happens if the dtor is not defined since otherwise the dtor 
has already been checked. Since dtor is not defined yet, it is not known to be 
emitted and put into call graph. Later on, if the dtor is defined, it will be 
checked again. This time it is known to be emitted, then clang finds that it is 
in the call graph, then the assert fails.

So the issue is that clang incorrectly assume the dtor is not known to be 
emitted in the first check and put it in the call graph. To fix that, a map is 
added to Sema to tell clang that it is checking a deleting dtor which is 
supposed to be emitted even if it is not defined.


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

https://reviews.llvm.org/D70172



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

[PATCH] D70257: [BPF] Restrict preserve_access_index attribute to C only

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a small nit. Thank you!




Comment at: clang/lib/CodeGen/CGExpr.cpp:3431
 
-const auto *RecT = 
dyn_cast(ElaborateT->desugar().getTypePtr());
-if (!RecT)
-  return false;
-
-return RecT->getDecl()->hasAttr();
+const auto *PointeeT = PtrT->getPointeeType().getTypePtr()
+ ->getUnqualifiedDesugaredType();

You can drop the `getTypePtr()` here and just use the magic `->` overload.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70257



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

[PATCH] D69759: [BPF] Add preserve_access_index attribute for record definition

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D69759#1746192 , @yonghong-song 
wrote:

> Sorry about the committing. I thought all the review questions are addressed.


No worries, this happens sometimes. :-)

> Definitely will be more patient next time until getting review confirmation 
> and
>  also adhering to llvm review policy.

Thanks!

> Thanks for the detailed review! I have addressed all of your review comments 
> in patch
>  https://reviews.llvm.org/D70257. I have added you as the reviewer.

Fantastic, thank you!




Comment at: clang/lib/CodeGen/CGExpr.cpp:3440-3443
+while (TypedefT) {
+  PointeeT = TypedefT->desugar().getTypePtr();
+  TypedefT = dyn_cast(PointeeT);
+}

yonghong-song wrote:
> aaron.ballman wrote:
> > Is there a reason you only want to strip off typedefs, and not other forms 
> > of sugar? e.g., why not call `getUnqualifiedDesugaredType()`?
> Actually, ElaboratedT is also a sugar type, the above 
> getUnqualifiedDesugaredType() will remove it too.
Yes, but you just strip off that sugar anyway, so that's why I suggested this. 
It seems you got the gist of my suggestion in the other review though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69759



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

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/SemaExprObjC.cpp:1129
+  }
+}
+

Is this a good idea to do for arbitrary diagnostics?  I feel like saying 
"direct" in the note when that has nothing to do with the conflict could be a 
little misleading.


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

https://reviews.llvm.org/D69991



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

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Tests for codegen of the declare target to|link vars with the new defaultmap 
clauses?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D70101: [X86] Fix the implementation of __readcr3/__writecr3 to work in 64-bit mode

[Reid Kleckner via Phabricator via cfe-commits]

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

Lgtm


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

https://reviews.llvm.org/D70101



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

[PATCH] D69564: Include the mangled name in -ast-dump=json

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69564



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

[PATCH] D70119: Improve gen_ast_dump_json_test.py

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from some nits. Thank you for this!




Comment at: clang/test/AST/gen_ast_dump_json_test.py:3
 
+from __future__ import print_function
 from collections import OrderedDict

Is this needed? I don't see anything using `print_function`.



Comment at: clang/test/AST/gen_ast_dump_json_test.py:11
 import re
+import tempfile
 import subprocess

Can you keep this in alphabetical order?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70119



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

[PATCH] D70101: [X86] Fix the implementation of __readcr3/__writecr3 to work in 64-bit mode

[Craig Topper via Phabricator via cfe-commits]

craig.topper updated this revision to Diff 229372.
craig.topper added a comment.

Add __LPTRINT_TYPE__ and use that to qualify the readcr3. Use __INTPTR_TYPE__ 
for writecr3


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

https://reviews.llvm.org/D70101

Files:
  clang/lib/Headers/intrin.h
  clang/test/Headers/ms-intrin.cpp


Index: clang/test/Headers/ms-intrin.cpp
===
--- clang/test/Headers/ms-intrin.cpp
+++ clang/test/Headers/ms-intrin.cpp
@@ -56,12 +56,8 @@
   __nop();
   __readmsr(0);
 
-  // FIXME: Call these in 64-bit too once the intrinsics have been fixed to
-  // work there, PR19301
-#ifndef _M_X64
   __readcr3();
   __writecr3(0);
-#endif
 
 #ifdef _M_ARM
   __dmb(_ARM_BARRIER_ISHST);
Index: clang/lib/Headers/intrin.h
===
--- clang/lib/Headers/intrin.h
+++ clang/lib/Headers/intrin.h
@@ -36,6 +36,12 @@
 /* Define the default attributes for the functions in this file. */
 #define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__))
 
+#if __x86_64__
+#define __LPTRINT_TYPE__ __int64
+#else
+#define __LPTRINT_TYPE__ long
+#endif
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -94,8 +100,7 @@
 void __outwordstring(unsigned short, unsigned short *, unsigned long);
 unsigned long __readcr0(void);
 unsigned long __readcr2(void);
-static __inline__
-unsigned long __readcr3(void);
+unsigned __LPTRINT_TYPE__ __readcr3(void);
 unsigned long __readcr4(void);
 unsigned long __readcr8(void);
 unsigned int __readdr(unsigned int);
@@ -132,7 +137,7 @@
 void __wbinvd(void);
 void __writecr0(unsigned int);
 static __inline__
-void __writecr3(unsigned int);
+void __writecr3(unsigned __INTPTR_TYPE__);
 void __writecr4(unsigned int);
 void __writecr8(unsigned int);
 void __writedr(unsigned int, unsigned int);
@@ -565,24 +570,26 @@
   __asm__ ("rdmsr" : "=d"(__edx), "=a"(__eax) : "c"(__register));
   return (((unsigned __int64)__edx) << 32) | (unsigned __int64)__eax;
 }
+#endif
 
-static __inline__ unsigned long __DEFAULT_FN_ATTRS
+static __inline__ unsigned __LPTRINT_TYPE__ __DEFAULT_FN_ATTRS
 __readcr3(void) {
-  unsigned long __cr3_val;
-  __asm__ __volatile__ ("mov %%cr3, %0" : "=q"(__cr3_val) : : "memory");
+  unsigned __LPTRINT_TYPE__ __cr3_val;
+  __asm__ __volatile__ ("mov %%cr3, %0" : "=r"(__cr3_val) : : "memory");
   return __cr3_val;
 }
 
 static __inline__ void __DEFAULT_FN_ATTRS
-__writecr3(unsigned int __cr3_val) {
-  __asm__ ("mov %0, %%cr3" : : "q"(__cr3_val) : "memory");
+__writecr3(unsigned __INTPTR_TYPE__ __cr3_val) {
+  __asm__ ("mov %0, %%cr3" : : "r"(__cr3_val) : "memory");
 }
-#endif
 
 #ifdef __cplusplus
 }
 #endif
 
+#undef __LPTRINT_TYPE__
+
 #undef __DEFAULT_FN_ATTRS
 
 #endif /* __INTRIN_H */


Index: clang/test/Headers/ms-intrin.cpp
===
--- clang/test/Headers/ms-intrin.cpp
+++ clang/test/Headers/ms-intrin.cpp
@@ -56,12 +56,8 @@
   __nop();
   __readmsr(0);
 
-  // FIXME: Call these in 64-bit too once the intrinsics have been fixed to
-  // work there, PR19301
-#ifndef _M_X64
   __readcr3();
   __writecr3(0);
-#endif
 
 #ifdef _M_ARM
   __dmb(_ARM_BARRIER_ISHST);
Index: clang/lib/Headers/intrin.h
===
--- clang/lib/Headers/intrin.h
+++ clang/lib/Headers/intrin.h
@@ -36,6 +36,12 @@
 /* Define the default attributes for the functions in this file. */
 #define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__))
 
+#if __x86_64__
+#define __LPTRINT_TYPE__ __int64
+#else
+#define __LPTRINT_TYPE__ long
+#endif
+
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -94,8 +100,7 @@
 void __outwordstring(unsigned short, unsigned short *, unsigned long);
 unsigned long __readcr0(void);
 unsigned long __readcr2(void);
-static __inline__
-unsigned long __readcr3(void);
+unsigned __LPTRINT_TYPE__ __readcr3(void);
 unsigned long __readcr4(void);
 unsigned long __readcr8(void);
 unsigned int __readdr(unsigned int);
@@ -132,7 +137,7 @@
 void __wbinvd(void);
 void __writecr0(unsigned int);
 static __inline__
-void __writecr3(unsigned int);
+void __writecr3(unsigned __INTPTR_TYPE__);
 void __writecr4(unsigned int);
 void __writecr8(unsigned int);
 void __writedr(unsigned int, unsigned int);
@@ -565,24 +570,26 @@
   __asm__ ("rdmsr" : "=d"(__edx), "=a"(__eax) : "c"(__register));
   return (((unsigned __int64)__edx) << 32) | (unsigned __int64)__eax;
 }
+#endif
 
-static __inline__ unsigned long __DEFAULT_FN_ATTRS
+static __inline__ unsigned __LPTRINT_TYPE__ __DEFAULT_FN_ATTRS
 __readcr3(void) {
-  unsigned long __cr3_val;
-  __asm__ __volatile__ ("mov %%cr3, %0" : "=q"(__cr3_val) : : "memory");
+  unsigned __LPTRINT_TYPE__ __cr3_val;
+  __asm__ __volatile__ ("mov %%cr3, %0" : "=r"(__cr3_val) : : "memory");
   return __cr3_val;
 }
 
 static __inline__ void __DEFAULT_FN_ATTRS
-__writecr3(unsigned int __cr3_val) {
-  

[PATCH] D70265: [clang-tidy] Add clang tidy check I.2 to cppcoreguidelines

[Kim Viggedal via Phabricator via cfe-commits]

vingeldal created this revision.
vingeldal added reviewers: JonasToth, aaron.ballman.
Herald added subscribers: kbarton, xazax.hun, mgorny, nemanjai.
Herald added a project: clang.

Cpp Core Guideline I.2, a.k.a "Avoid non-const global variables"
For detailed documentation, see:
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#i2-avoid-non-const-global-variables


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70265

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-non-const-global-variables.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-non-const-global-variables %t
+
+int nonConstVariable = 0;
+// CHECK-MESSAGES:  warning: variable 'nonConstVariable' is non-const and global [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: int const nonConstVariable = 0;
+
+namespace namespace_name {
+int nonConstNamespaceVariable = 0;
+// CHECK-MESSAGES: warning: variable 'nonConstNamespaceVariable' is non-const and global [cppcoreguidelines-avoid-non-const-global-variables]
+// CHECK-FIXES: int const nonConstNamespaceVariable = 0;
+} // namespace namespace_name
+
+const int constVariable = 0;
+
+constexpr int constexprVariable = 0;
+
+int function() {
+  int nonConstReturnValue = 0;
+  return nonConstReturnValue;
+}
+
+namespace {
+int nonConstAnonymousNamespaceVariable = 0;
+}
+
+class Class {
+  int nonConstMemberVariable = 0;
+};
+
+int main() {
+  for (int i = 0; i < 10; ++i) {
+int nonConstLoopVariable = 42;
+nonConstVariable = nonConstLoopVariable + i;
+  }
+}
\ No newline at end of file
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -194,6 +194,7 @@
cppcoreguidelines-avoid-c-arrays (redirects to modernize-avoid-c-arrays) 
cppcoreguidelines-avoid-goto
cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) 
+   cppcoreguidelines-avoid-non-const-global-variables
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) 
cppcoreguidelines-explicit-virtual-functions (redirects to modernize-use-override) 
cppcoreguidelines-init-variables
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-non-const-global-variables.rst
@@ -0,0 +1,6 @@
+.. title:: clang-tidy - cppcoreguidelines-avoid-non-const-global-variables
+
+cppcoreguidelines-avoid-non-const-global-variables
+==
+
+FIXME: Describe what patterns does the check detect and why. Give examples.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -94,6 +94,9 @@
   Without the null terminator it can result in undefined behaviour when the
   string is read.
 
+- New :doc:`cppcoreguidelines-avoid-non-const-global-variables
+  ` check.
+
 - New alias :doc:`cert-pos44-c
   ` to
   :doc:`bugprone-bad-signal-to-kill-thread
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -15,6 +15,7 @@
 #include "../modernize/UseOverrideCheck.h"
 #include "../readability/MagicNumbersCheck.h"
 #include "AvoidGotoCheck.h"
+#include "AvoidNonConstGlobalVariablesCheck.h"
 #include "InitVariablesCheck.h"
 #include "InterfacesGlobalInitCheck.h"
 #include "MacroUsageCheck.h"
@@ -48,6 +49,8 @@
 "cppcoreguidelines-avoid-goto");
 CheckFactories.registerCheck(
 "cppcoreguidelines-avoid-magic-numbers");
+

[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:1483
+// OpenMPIRBuilder is asked to construct a parallel (or similar) construct.
+auto FiniCB = [&](llvm::OpenMPIRBuilder::InsertPointTy IP) {
+  assert(IP.getBlock()->end() == IP.getPoint() &&

Better to not capture all the object implicitly by reference, provide explicit 
list



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:1486
+ "Clang CG should cause non-terminated block!");
+  auto OldIP = CGF.Builder.saveIP();
+  CGF.Builder.restoreIP(IP);

Use `InsertPointGuard` where possible rather than use `saveIP/restoreIP` 
explicitly



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:1490
+  CGF.getOMPCancelDestination(OMPD_parallel);
+  assert(Dest.getBlock());
+  CGF.EmitBranchThroughCleanup(Dest);

Add a message for the assert.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:1499-1509
+OMPBuilder->pushFinalizationCB(std::move(FI));
+  }
+
   CGOpenMPOutlinedRegionInfo CGInfo(*CS, ThreadIDVar, CodeGen, InnermostKind,
 HasCancel, OutlinedHelperName);
   CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(CGF, );
+  llvm::Function *Fn = CGF.GenerateOpenMPCapturedStmtFunction(*CS);

Again, need RAII



Comment at: llvm/include/llvm/Frontend/OpenMPIRBuilder.h:69-71
+  void pushFinalizationCB(FinalizationInfo &) {
+FinalizationStack.emplace_back(std::move(FI));
+  }

Maybe `push...` for const reference and `emplace...` for variadic template 
(just like in standard library)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70258



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

[PATCH] D68401: [Hexagon] Fix clang driver to parse -mcpu/-mvXX and -march properly.

[Sumanth Gundapaneni via Phabricator via cfe-commits]

sgundapa closed this revision.
sgundapa added a comment.

I forgot to update the Differential Revision in the commit.


Repository:
  rC Clang

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

https://reviews.llvm.org/D68401



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

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:3919
+in particular, this means that it cannot override a superclass method or 
satisfy
+a protocol requirement.
+

rjmccall wrote:
> Please add a new paragraph here:
> 
>   Because a direct method cannot be overridden, it is an error to perform
>   a ``super`` message send of one.
> 
> And you should test that.  (I noticed this because you had an 
> `assert(!IsSuper);` in IRGen, which was both a good idea and also begging for 
> a justification. :))
hah turns out I actually need to implement the Sema check for this :D



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:4087
+
+ReceiverCanBeNull = isWeakLinkedClass(OID);
+  }

rjmccall wrote:
> The assumption here is that a direct class method can never be invoked on a 
> nullable value, like a `Class`.  I think that's true, but it's worth making 
> that explicit in a comment.
Hmm actually I think that it's _not_ true. as in I'm not disallowing it today 
but I should be. I need to figure out how to do that, messaging a `Class` 
should be as disallowed as messaging `id`.

but right now it's not.


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

https://reviews.llvm.org/D69991



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

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 229366.
MadCoder marked 7 inline comments as done.
MadCoder added a comment.

Updated for the new round of comments, with added tests and Sema checks errors 
for:

- messaging super
- messaging a nullable Class expression


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

https://reviews.llvm.org/D69991

Files:
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/ObjCRuntime.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclObjC.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/CodeGenObjC/direct-method.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/method-direct-properties.m
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct.m
@@ -0,0 +1,148 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+@protocol Proto
+- (void)protoMethod;  // expected-note {{previous declaration is here}}
++ (void)classProtoMethod; // expected-note {{previous declaration is here}}
+@end
+
+@protocol ProtoDirectFail
+- (void)protoMethod __attribute__((objc_direct));  // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
++ (void)classProtoMethod __attribute__((objc_direct)); // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
+@end
+
+__attribute__((objc_root_class))
+@interface Root
+- (void)rootRegular;  // expected-note {{previous declaration is here}}
++ (void)classRootRegular; // expected-note {{previous declaration is here}}
+- (void)rootDirect __attribute__((objc_direct));  // expected-note {{previous declaration is here}};
++ (void)classRootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
+- (void)otherRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherRootDirect' declared here}}
++ (void)otherClassRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherClassRootDirect' declared here}}
+- (void)notDirectInIface; // expected-note {{previous declaration is here}}
++ (void)classNotDirectInIface;// expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct_members))
+@interface Root ()
+- (void)rootExtensionDirect;  // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect; // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct_members))
+@interface Root(Direct)
+- (void)rootCategoryDirect;  // expected-note {{previous declaration is here}}
++ (void)classRootCategoryDirect; // expected-note {{previous declaration is here}}
+@end
+
+@interface Root ()
+- (void)rootExtensionRegular;   // expected-note {{previous declaration is here}}
++ (void)classRootExtensionRegular;  // expected-note {{previous declaration is here}}
+- (void)rootExtensionDirect2 __attribute__((objc_direct));  // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface Root (Direct2)
+- (void)rootCategoryRegular;   // expected-note {{previous declaration is here}}
++ (void)classRootCategoryRegular;  // expected-note {{previous declaration is here}}
+- (void)rootCategoryDirect2 __attribute__((objc_direct));  // expected-note {{previous declaration is here}}
++ (void)classRootCategoryDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_root_class, objc_direct_members)) // expected-error {{'objc_direct_members' attribute only applies to Objective-C implementation declarations and Objective-C containers}}
+@interface SubDirectFail : Root
+- (instancetype)init;
+@end
+
+@interface Sub : Root 
+/* invalid overrides with directs */
+- (void)rootRegular __attribute__((objc_direct));   // expected-error {{methods that override superclass methods cannot be direct}}
++ (void)classRootRegular 

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: llvm/include/llvm/IR/DIBuilder.h:243
+ unsigned LineNo, DIScope *Context,
+ uint32_t AlignInBits = 0);
 

awpandey wrote:
> aprantl wrote:
> > This should be `Optional` AlignInBits. Even better perhaps 
> > `llvm::Align` but perhaps that's too restrictive.
> Yes @aprantl  this should be of `Optional`  type but changing this 
> will require change in the `DIDerivedType::get` macro and this macro is used 
> by many other functions. Please correct me if I am wrong. ??
> 
> Current functionality of the `createTypeDef` is like the default value of the 
> `alignInBits`  will be `0`. Consider below comment in `DIBuilder.cpp`.
Updating `DIDerivedType::get` to use Optional would also be a good change, but 
for this patch I'd be happy if at least the DIBuilder interface is using 
optional and then treats None as `0` in the implemenation. We can optionally do 
another patch to change the interface of `DIDerivedType::get`


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

https://reviews.llvm.org/D70111



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

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Johannes Doerfert via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:228
+   getOrCreateThreadID(getOrCreateIdent(SrcLocStr))};
+  bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+  Value *Result = Builder.CreateCall(

ABataev wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > ABataev wrote:
> > > > jdoerfert wrote:
> > > > > ABataev wrote:
> > > > > > ABataev wrote:
> > > > > > > ABataev wrote:
> > > > > > > > jdoerfert wrote:
> > > > > > > > > ABataev wrote:
> > > > > > > > > > jdoerfert wrote:
> > > > > > > > > > > ABataev wrote:
> > > > > > > > > > > > jdoerfert wrote:
> > > > > > > > > > > > > ABataev wrote:
> > > > > > > > > > > > > > jdoerfert wrote:
> > > > > > > > > > > > > > > ABataev wrote:
> > > > > > > > > > > > > > > > jdoerfert wrote:
> > > > > > > > > > > > > > > > > ABataev wrote:
> > > > > > > > > > > > > > > > > > Maybe add an assert when the cancellation 
> > > > > > > > > > > > > > > > > > version is requested but the cancellation 
> > > > > > > > > > > > > > > > > > block is not set? Instead of the generating 
> > > > > > > > > > > > > > > > > > simple version of barrier.
> > > > > > > > > > > > > > > > > The interface doesn't work that way as we do 
> > > > > > > > > > > > > > > > > not know here if the cancellation was 
> > > > > > > > > > > > > > > > > requested except if the block was set. That 
> > > > > > > > > > > > > > > > > is basically the flag (and I expect it to 
> > > > > > > > > > > > > > > > > continue to be that way).
> > > > > > > > > > > > > > > > Maybe instead of `ForceSimpleBarrier` add a 
> > > > > > > > > > > > > > > > flag `EmitCancelBarrier` and if it set to true, 
> > > > > > > > > > > > > > > > always emit cancel barrier, otherwise always 
> > > > > > > > > > > > > > > > emit simple barrier? And add an assertion for 
> > > > > > > > > > > > > > > > non-set cancellation block or even accept it as 
> > > > > > > > > > > > > > > > a parameter here.
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > Also, what if we have inner exception handling 
> > > > > > > > > > > > > > > > in the region? Will you handle the cleanup 
> > > > > > > > > > > > > > > > correctly in case of the cancelation barrier?
> > > > > > > > > > > > > > > > Maybe instead of ForceSimpleBarrier add a flag 
> > > > > > > > > > > > > > > > EmitCancelBarrier and if it set to true, always 
> > > > > > > > > > > > > > > > emit cancel barrier, otherwise always emit 
> > > > > > > > > > > > > > > > simple barrier? And add an assertion for 
> > > > > > > > > > > > > > > > non-set cancellation block or even accept it as 
> > > > > > > > > > > > > > > > a parameter here.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > What is the difference in moving some of the 
> > > > > > > > > > > > > > > boolean logic to the caller? Also, we have test 
> > > > > > > > > > > > > > > to verify we get cancellation barriers if we need 
> > > > > > > > > > > > > > > them, both unit tests and clang lit tests.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > Also, what if we have inner exception handling 
> > > > > > > > > > > > > > > > in the region? Will you handle the cleanup 
> > > > > > > > > > > > > > > > correctly in case of the cancelation barrier?
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > I think so. Right now through the code in clang 
> > > > > > > > > > > > > > > that does the set up of the cancellation block, 
> > > > > > > > > > > > > > > later through callbacks but we only need that for 
> > > > > > > > > > > > > > > regions where we actually go out of some scope, 
> > > > > > > > > > > > > > > e.g., parallel.
> > > > > > > > > > > > > > 1. I'm just thinking about future users of thus 
> > > > > > > > > > > > > > interface. It woild be good if we could provide 
> > > > > > > > > > > > > > safe interface for all the users, not only clang.
> > > > > > > > > > > > > > 2. Exit out of the OpenMP region is not allowed. 
> > > > > > > > > > > > > > But you may have inner try...catch or just simple 
> > > > > > > > > > > > > > compound statement with local vars that require 
> > > > > > > > > > > > > > constructors/destructors. And the cancellation 
> > > > > > > > > > > > > > barrier may exit out of these regions. And you need 
> > > > > > > > > > > > > > to call all required destructors. You'd better to 
> > > > > > > > > > > > > > think about it now, not later.
> > > > > > > > > > > > > > 2. [...] You'd better to think about it now, not 
> > > > > > > > > > > > > > later.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > First, I do think about it now and I hope this was 
> > > > > > > > > > > > > not an insinuation to suggest otherwise.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > > 1. I'm just thinking about future users of thus 
> > > > > > > > > > > > > > interface. It woild be good if 

[PATCH] D69759: [BPF] Add preserve_access_index attribute for record definition

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song marked 7 inline comments as done.
yonghong-song added a comment.

Sorry about the committing. I thought all the review questions are addressed.
Definitely will be more patient next time until getting review confirmation and
also adhering to llvm review policy.
Thanks for the detailed review! I have addressed all of your review comments in 
patch
https://reviews.llvm.org/D70257. I have added you as the reviewer.




Comment at: clang/include/clang/Basic/Attr.td:1584
+ TargetSpecificAttr  {
+  let Spellings = [Clang<"preserve_access_index">];
+  let Subjects = SubjectList<[Record], ErrorDiag>;

aaron.ballman wrote:
> If this attribute is only supported in C mode, then this attribute should 
> have: `let LangOpts = [COnly];` in it.
Yes, added.



Comment at: clang/lib/CodeGen/CGExpr.cpp:3440-3443
+while (TypedefT) {
+  PointeeT = TypedefT->desugar().getTypePtr();
+  TypedefT = dyn_cast(PointeeT);
+}

aaron.ballman wrote:
> Is there a reason you only want to strip off typedefs, and not other forms of 
> sugar? e.g., why not call `getUnqualifiedDesugaredType()`?
Actually, ElaboratedT is also a sugar type, the above 
getUnqualifiedDesugaredType() will remove it too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69759



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

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:2953-2954
+(!(isOpenMPTargetExecutionDirective(DKind) && Res &&
+   (*Res == OMPDeclareTargetDeclAttr::MT_Link ||
+*Res == OMPDeclareTargetDeclAttr::MT_To {
+  // Only check for data-mapping attribute and is_device_ptr here

cchen wrote:
> ABataev wrote:
> > ABataev wrote:
> > > Why comparing the value of `*Res` here if just comparing against all 
> > > possible values? This condition is always `true` if `Res` is true. I 
> > > don't think we need to map variables with `to` mapping.
> > Do we need to map `declare target to` vars at all?
> I don't understand this since spec says that 
> ```
> each variable referenced in the construct that does not have a predetermined 
> data-sharing attribute and does not appear in a to or link clause on a 
> declare target directive must be listed in a data-mapping attribute clause, a 
> data-sharing attribute clause (including a data-sharing attribute clause on a 
> combined construct where target is one of the constituent constructs), or an 
> is_device_ptr clause.
> ```
> So, I think I should check both to and link.
> Or are you saying that to clause is impossible to happen here since we have a 
> "Skip internally declared static variables" check so that if there is a to 
> clause, it will return before hitting Line 2952. Therefore, we should only 
> check for link clause?
Missed `!` in the condition. Why we check for 
`!isOpenMPTargetExecutionDirective(DKind)`



Comment at: clang/lib/Sema/SemaOpenMP.cpp:2952
+VarsWithInheritedDSA.count(VD) == 0 &&
+(!(isOpenMPTargetExecutionDirective(DKind) && Res))) {
+  // Only check for data-mapping attribute and is_device_ptr here

Why extra parens in the condition?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D70257: [BPF] Restrict preserve_access_index attribute to C only

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song created this revision.
yonghong-song added a reviewer: aaron.ballman.
Herald added subscribers: cfe-commits, arphaman.
Herald added a project: clang.
yonghong-song retitled this revision from "[BPF] Restrict preserve_access_index 
to C only" to "[BPF] Restrict preserve_access_index attribute to C only".

This patch is a follow-up for commit 4e2ce228ae79 


  [BPF] Add preserve_access_index attribute for record definition

to restrict attribute for C only. A new test case is added
to check for this restriction.

Additional code polishing is done based on
Aaron Ballman's suggestion in https://reviews.llvm.org/D69759/new/.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70257

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/Sema/bpf-attr-preserve-access-index.cpp


Index: clang/test/Sema/bpf-attr-preserve-access-index.cpp
===
--- /dev/null
+++ clang/test/Sema/bpf-attr-preserve-access-index.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -x c++ -triple bpf-pc-linux-gnu -dwarf-version=4 
-fsyntax-only -verify %s
+
+#define __reloc__ __attribute__((preserve_access_index))
+
+struct t1 {
+  int a;
+  int b[4];
+  int c:1;
+} __reloc__; // expected-warning {{'preserve_access_index' attribute ignored}}
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -3408,10 +3408,6 @@
   if (!ArrayBase || !CGF.getDebugInfo())
 return false;
 
-  const auto *ImplicitCast = dyn_cast(ArrayBase);
-  if (!ImplicitCast)
-return false;
-
   // Only support base as either a MemberExpr or DeclRefExpr.
   // DeclRefExpr to cover cases like:
   //struct s { int a; int b[10]; };
@@ -3419,39 +3415,24 @@
   //p[1].a
   // p[1] will generate a DeclRefExpr and p[1].a is a MemberExpr.
   // p->b[5] is a MemberExpr example.
-  const Expr *E = ImplicitCast->getSubExpr();
-  const auto *MemberCast = dyn_cast(E);
-  if (MemberCast)
-return MemberCast->getMemberDecl()->hasAttr();
-
-  const auto *DeclRefCast = dyn_cast(E);
-  if (DeclRefCast) {
-const VarDecl *VarDef = dyn_cast(DeclRefCast->getDecl());
+  const Expr *E = ArrayBase->IgnoreImpCasts();
+  if (const auto *ME = dyn_cast(E))
+return ME->getMemberDecl()->hasAttr();
+
+  if (const auto *DRE = dyn_cast(E)) {
+const auto *VarDef = dyn_cast(DRE->getDecl());
 if (!VarDef)
   return false;
 
-const auto *PtrT = dyn_cast(VarDef->getType().getTypePtr());
+const auto *PtrT = VarDef->getType()->getAs();
 if (!PtrT)
   return false;
-const auto *PointeeT = PtrT->getPointeeType().getTypePtr();
-
-// Peel off typedef's
-const auto *TypedefT = dyn_cast(PointeeT);
-while (TypedefT) {
-  PointeeT = TypedefT->desugar().getTypePtr();
-  TypedefT = dyn_cast(PointeeT);
-}
-
-// Not a typedef any more, it should be an elaborated type.
-const auto ElaborateT = dyn_cast(PointeeT);
-if (!ElaborateT)
-  return false;
 
-const auto *RecT = 
dyn_cast(ElaborateT->desugar().getTypePtr());
-if (!RecT)
-  return false;
-
-return RecT->getDecl()->hasAttr();
+const auto *PointeeT = PtrT->getPointeeType().getTypePtr()
+ ->getUnqualifiedDesugaredType();
+if (const auto *RecT = dyn_cast(PointeeT))
+  return RecT->getDecl()->hasAttr();
+return false;
   }
 
   return false;
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1584,6 +1584,7 @@
   let Spellings = [Clang<"preserve_access_index">];
   let Subjects = SubjectList<[Record], ErrorDiag>;
   let Documentation = [BPFPreserveAccessIndexDocs];
+  let LangOpts = [COnly];
 }
 
 def WebAssemblyImportModule : InheritableAttr,


Index: clang/test/Sema/bpf-attr-preserve-access-index.cpp
===
--- /dev/null
+++ clang/test/Sema/bpf-attr-preserve-access-index.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -x c++ -triple bpf-pc-linux-gnu -dwarf-version=4 -fsyntax-only -verify %s
+
+#define __reloc__ __attribute__((preserve_access_index))
+
+struct t1 {
+  int a;
+  int b[4];
+  int c:1;
+} __reloc__; // expected-warning {{'preserve_access_index' attribute ignored}}
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -3408,10 +3408,6 @@
   if (!ArrayBase || !CGF.getDebugInfo())
 return false;
 
-  const auto *ImplicitCast = dyn_cast(ArrayBase);
-  if (!ImplicitCast)
-return false;
-
   // Only support base as either a MemberExpr or DeclRefExpr.
   // DeclRefExpr to cover cases like:
   //

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Johannes Doerfert via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:200
+OpenMPIRBuilder::InsertPointTy
+OpenMPIRBuilder::emitBarrierImpl(const LocationDescription , Directive 
Kind,
+ bool ForceSimpleCall, bool CheckCancelFlag) {

ABataev wrote:
> Maybe split emission ща `__kmpc_barrier` and `__kmpc_cancel_barrier` 
> functions into 2 independent functions fo the frontends? Rather than rely on 
> the boolean flags?
The frontend doesn't know necessarily when a barrier is issued if it is a 
cancellable, arguably it shouldn't need to know at all

I copied the flags from clang but I will look into removing them eventually 
(and to add a TODO in the meantime).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69785



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

[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert created this revision.
jdoerfert added reviewers: kiranchandramohan, ABataev, RaviNarayanaswamy, 
gtbercea, grokos, sdmitriev, JonChesterfield, hfinkel, fghanim.
Herald added subscribers: cfe-commits, guansong, bollu, hiraditya.
Herald added projects: clang, LLVM.

As a permanent and generic solution to the problem of variable
finalization (destructors, lastprivate, ...), this patch introduces the
finalization stack. The objects on the stack describe (1) the
(structured) regions the OpenMP-IR-Builder is currently constructing,
(2) if these are cancellable, and (3) the callback that will perform the
finalization (=cleanup) when necessary.

As the finalization can be necessary multiple times, at different source
locations, the callback takes the position at which code is currently
generated. This position will also encode the destination of the "region
exit" block *iff* the finalization call was issues for a region
generated by the OpenMPIRBuilder. For regions generated through the old
Clang OpenMP code geneneration, the "region exit" is determined by Clang
inside the finalization call instead (see getOMPCancelDestination).

As a first user, the parallel + cancel barrier interaction is changed.
In contrast to the temporary solution before, the barrier generation in
Clang does not need to be aware of the "CancelDestination" block.
Instead, the finalization callback is and, as described above, later
even that one does not need to be.

D70109  will be updated to use this scheme.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70258

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMPIRBuilder.h
  llvm/lib/Frontend/OpenMPIRBuilder.cpp

Index: llvm/lib/Frontend/OpenMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMPIRBuilder.cpp
@@ -228,7 +228,8 @@
   // If we are in a cancellable parallel region, barriers are cancellation
   // points.
   // TODO: Check why we would force simple calls or to ignore the cancel flag.
-  bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+  bool UseCancelBarrier =
+  !ForceSimpleCall && isLastFinalizationInfoCancellable(OMPD_parallel);
 
   Value *Result = Builder.CreateCall(
   getOrCreateRuntimeFunction(UseCancelBarrier ? OMPRTL___kmpc_cancel_barrier
@@ -248,13 +249,14 @@
 Builder.CreateCondBr(Cmp, NonCancellationBlock, CancellationBlock,
  /* TODO weight */ nullptr, nullptr);
 
-Builder.SetInsertPoint(NonCancellationBlock);
-assert(CancellationBlock->getParent() == BB->getParent() &&
-   "Unexpected cancellation block parent!");
+// From the cancellation block we finalize all variables and go to the
+// post finalization block that is known to the FiniCB callback.
+Builder.SetInsertPoint(CancellationBlock);
+auto  = FinalizationStack.back();
+FI.FiniCB(Builder.saveIP());
 
-// TODO: This is a workaround for now, we always reset the cancellation
-// block until we manage it ourselves here.
-CancellationBlock = nullptr;
+// The continuation block is where code generation continues.
+Builder.SetInsertPoint(NonCancellationBlock);
   }
 
   return Builder.saveIP();
Index: llvm/include/llvm/Frontend/OpenMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMPIRBuilder.h
@@ -37,12 +37,44 @@
   /// Add attributes known for \p FnID to \p Fn.
   void addAttributes(omp::RuntimeFunction FnID, Function );
 
-  /// Set the cancellation block to \p CBB.
-  void setCancellationBlock(BasicBlock *CBB) { CancellationBlock = CBB; }
-
   /// Type used throughout for insertion points.
   using InsertPointTy = IRBuilder<>::InsertPoint;
 
+  /// Callback type for variable finalization (think destructors).
+  ///
+  /// \param CodeGenIP is the insertion point at which the finalization code
+  ///  should be placed.
+  ///
+  /// A finalize callback knows about all objects that need finalization, e.g.
+  /// destruction, when the scope of the currently generated construct is left
+  /// at the time, and location, the callback is invoked.
+  using FinalizeCallbackTy = function_ref;
+
+  struct FinalizationInfo {
+/// The finalization callback provided by the last in-flight invocation of
+/// Create for the directive of kind DK.
+FinalizeCallbackTy FiniCB;
+
+/// The directive kind of the innermost directive that has an associated
+/// region which might require finalization when it is left.
+omp::Directive DK;
+
+/// Flag to indicate if the directive is cancellable.
+bool IsCancellable;
+  };
+
+  /// Push a finalization callback on the finalization stack.
+  ///
+  /// NOTE: Temporary solution until Clang CG is gone.
+  void 

[clang] 9fcf4f3 - [Hexagon] Fix clang driver to parse -mcpu/-mvXX and -march properly.

[Sumanth Gundapaneni via cfe-commits]

Author: Sumanth Gundapaneni
Date: 2019-11-14T12:59:15-06:00
New Revision: 9fcf4f372c7e08b7ee64a202cc09860a17da8152

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

LOG: [Hexagon] Fix clang driver to parse -mcpu/-mvXX and -march properly.

Before this patch if we pass "-mcpu=hexagonv65 -march=hexagon" in this order,
the driver fails to figure out the correct cpu version. This patch fixed this
issue.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Hexagon.cpp
clang/test/Driver/hexagon-toolchain-elf.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Hexagon.cpp 
b/clang/lib/Driver/ToolChains/Hexagon.cpp
index 96cc084e2821..4a735a2a1d59 100644
--- a/clang/lib/Driver/ToolChains/Hexagon.cpp
+++ b/clang/lib/Driver/ToolChains/Hexagon.cpp
@@ -574,7 +574,7 @@ const StringRef HexagonToolChain::GetDefaultCPU() {
 
 const StringRef HexagonToolChain::GetTargetCPUVersion(const ArgList ) {
   Arg *CpuArg = nullptr;
-  if (Arg *A = Args.getLastArg(options::OPT_mcpu_EQ, options::OPT_march_EQ))
+  if (Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
 CpuArg = A;
 
   StringRef CPU = CpuArg ? CpuArg->getValue() : GetDefaultCPU();

diff  --git a/clang/test/Driver/hexagon-toolchain-elf.c 
b/clang/test/Driver/hexagon-toolchain-elf.c
index a7eeca0fdb12..237242e7cbf1 100644
--- a/clang/test/Driver/hexagon-toolchain-elf.c
+++ b/clang/test/Driver/hexagon-toolchain-elf.c
@@ -121,6 +121,19 @@
 // CHECK028-NOT: "-ffp-contract=fast"
 // CHECK028: {{hexagon-link|ld}}
 
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mcpu=hexagonv65 -march=hexagon\
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK029 %s
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN:   -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN:   -mv65 -march=hexagon\
+// RUN:   %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK029 %s
+// CHECK029: "-cc1" {{.*}} "-target-cpu" "hexagonv65"
+// CHECK029: 
hexagon-link{{.*}}/Inputs/hexagon_tree/Tools/bin/../target/hexagon/lib/v65/crt0
+
 // 
-
 // Test Linker related args
 // 
-



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

[PATCH] D69970: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

In D69970#1746103 , @dblaikie wrote:

> In D69970#1745038 , @vsk wrote:
>
> > Don't emit declaration subprograms for functions with reserved names. There 
> > may not be much benefit from referencing these functions from call site 
> > tags (e.g. instead of relying on call site info to work out the arguments 
> > passed to __asan_memcpy, the asan runtime could log those arguments 
> > instead).
> >
> > This fixes a verifier failure:
> >
> >   inlinable function call in a function with debug info must have a !dbg 
> > location
> > %134 = call i8* @__asan_memcpy(i8* %agg.tmp2.sroa.0.0..sroa_cast10.i, 
> > i8* %agg.tmp.sroa.0.i.0..sroa_cast, i64 32)
> >
> >
> > @aprantl / @djtodoro, mind taking another look?
>
>
> (my very vague take on this, admittedly having not followed this work in 
> detail, is that this sounds pretty heuristical & not like it's a 
> principled/general solution? But I don't really know much about all this & 
> will leave actual approvals/disapprovals to the other folks who've been 
> reviewing this stuff (& admittedly, as the person who introduced the 
> "inlinable call with no debug location in debug-having function" 
> assertion/failure, it is a bit contextual as to whether a certain call is 
> inlinable, etc))


*nod*, yeah I agree, but haven't come up with a better alternative. One option 
I considered was to play whac-a-mole and just teach ExpandICmps/ASan/others to 
emit builtin calls with line 0 locations attached. But I'm not sure that the 
extra line 0 locations would really help (maybe they'd actually get in the way 
of some reasonable location fallthrough), so the cleaner option seemed to be to 
not emit the declaration subprograms in the first place.


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

https://reviews.llvm.org/D69970



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

[PATCH] D69330: [AST] Add RecoveryExpr to retain expressions on semantic errors

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

I would prefer that we have dedicated tests for them rather than scattering the 
tests throughout the existing test suite (for example, consider adding 
`-Wno-unused` to the tests producing "result unused" warnings and adding a 
dedicated test).




Comment at: clang/include/clang/Sema/Sema.h:3568-3569
+  /// Corrects typos inside \p SubExprs.
+  ExprResult ActOnSemanticError(SourceLocation Begin, SourceLocation End,
+ArrayRef SubExprs);
+

Generally the `ActOn*` interface is invoked by the parser in response to 
syntactic constructs, so `ActOnSemanticError` is a surprising function name. 
Maybe `CreateRecoveryExpr` would be better. Is that too narrow for the intended 
semantics of this function?



Comment at: clang/lib/AST/TextNodeDumper.cpp:736-739
+void TextNodeDumper::VisitRecoveryExpr(const RecoveryExpr *Node) {
+  OS << " "
+ << "";
+}

You shouldn't need to print anything here. The class name is automatically 
printed for you.



Comment at: clang/lib/Sema/SemaExceptionSpec.cpp:1287
 // FIXME: Can any of the above throw?  If so, when?
 return CT_Cannot;
 

Should we return `CT_Dependent` for `RecoveryExprClass`, since we model it as 
being dependent?



Comment at: clang/lib/Sema/SemaExpr.cpp:17978
+  }
+  return RecoveryExpr::Create(Context, Begin, End, NoTypos);
+}

We shouldn't create these in code synthesis contexts (eg, during SFINAE checks 
in template instantiations).



Comment at: clang/lib/Sema/TreeTransform.h:9470
+ExprResult TreeTransform::TransformRecoveryExpr(RecoveryExpr *E) {
+  return E;
+}

We should either transform the subexpressions or just return `ExprError()` 
here. With this approach, we can violate AST invariants (eg, by having the same 
`Expr` reachable through two different code paths in the same function body, or 
by retaining `DeclRefExpr`s referring to declarations from the wrong context, 
etc).



Comment at: clang/test/SemaCXX/builtin-operator-new-delete.cpp:94
   void *p = __builtin_operator_new(42, std::align_val_t(2)); // expected-error 
{{call to '__builtin_operator_new' selects non-usual allocation function}}
-  __builtin_operator_delete(p, std::align_val_t(2)); // expected-error 
{{call to '__builtin_operator_delete' selects non-usual deallocation function}}
+  __builtin_operator_delete((void*)0, std::align_val_t(2)); // 
expected-error {{call to '__builtin_operator_delete' selects non-usual 
deallocation function}}
 #endif

What happens to the original testcase here? I wouldn't expect the invalid call 
expression in the initializer of `p` to affect whether we diagnose uses of `p`.

(Nit: remove spaces to realign the comments here.)



Comment at: clang/test/SemaOpenCLCXX/address-space-references.cl:15
+  bar(1) // expected-error{{binding reference of type 'const __global unsigned 
int' to value of type 'int' changes address space}} \
+ // expected-error{{expected ';' after expression}}
 }

Consider just adding the missing semicolon, so that this test is only testing 
the thing it aims to test.



Comment at: clang/test/SemaTemplate/instantiate-init.cpp:102
 (void)sizeof(array_lengthof(Description::data));
-
+// expected-warning@+1{{expression result unused}}
 sizeof(array_lengthof( // expected-error{{no matching function for call to 
'array_lengthof'}}

Add a cast-to-`void`, like on the previous line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69330



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

[PATCH] D69922: [OpenMP] Use the OpenMP-IR-Builder

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3489
+OMPRegionInfo->hasCancel()) {
+  auto IP = CGF.Builder.saveIP();
+  llvm::BasicBlock *ExitBB =

Also, you need to introduce/use RAII that saves/restores insertion point 
automatically.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:3492
+  CGF.createBasicBlock(".cancel.exit", IP.getBlock()->getParent());
+  OMPBuilder->setCancellationBlock(ExitBB);
+  CGF.Builder.SetInsertPoint(ExitBB);

Maybe, instead of saving the state, pass the pointer to the cancel block as a 
parameter to the `CreateBarrier` function?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69922



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

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:200
+OpenMPIRBuilder::InsertPointTy
+OpenMPIRBuilder::emitBarrierImpl(const LocationDescription , Directive 
Kind,
+ bool ForceSimpleCall, bool CheckCancelFlag) {

Maybe split emission ща `__kmpc_barrier` and `__kmpc_cancel_barrier` functions 
into 2 independent functions fo the frontends? Rather than rely on the boolean 
flags?



Comment at: llvm/lib/Frontend/OpenMPIRBuilder.cpp:228
+   getOrCreateThreadID(getOrCreateIdent(SrcLocStr))};
+  bool UseCancelBarrier = !ForceSimpleCall && CancellationBlock;
+  Value *Result = Builder.CreateCall(

ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > jdoerfert wrote:
> > > > ABataev wrote:
> > > > > ABataev wrote:
> > > > > > ABataev wrote:
> > > > > > > jdoerfert wrote:
> > > > > > > > ABataev wrote:
> > > > > > > > > jdoerfert wrote:
> > > > > > > > > > ABataev wrote:
> > > > > > > > > > > jdoerfert wrote:
> > > > > > > > > > > > ABataev wrote:
> > > > > > > > > > > > > jdoerfert wrote:
> > > > > > > > > > > > > > ABataev wrote:
> > > > > > > > > > > > > > > jdoerfert wrote:
> > > > > > > > > > > > > > > > ABataev wrote:
> > > > > > > > > > > > > > > > > Maybe add an assert when the cancellation 
> > > > > > > > > > > > > > > > > version is requested but the cancellation 
> > > > > > > > > > > > > > > > > block is not set? Instead of the generating 
> > > > > > > > > > > > > > > > > simple version of barrier.
> > > > > > > > > > > > > > > > The interface doesn't work that way as we do 
> > > > > > > > > > > > > > > > not know here if the cancellation was requested 
> > > > > > > > > > > > > > > > except if the block was set. That is basically 
> > > > > > > > > > > > > > > > the flag (and I expect it to continue to be 
> > > > > > > > > > > > > > > > that way).
> > > > > > > > > > > > > > > Maybe instead of `ForceSimpleBarrier` add a flag 
> > > > > > > > > > > > > > > `EmitCancelBarrier` and if it set to true, always 
> > > > > > > > > > > > > > > emit cancel barrier, otherwise always emit simple 
> > > > > > > > > > > > > > > barrier? And add an assertion for non-set 
> > > > > > > > > > > > > > > cancellation block or even accept it as a 
> > > > > > > > > > > > > > > parameter here.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > Also, what if we have inner exception handling in 
> > > > > > > > > > > > > > > the region? Will you handle the cleanup correctly 
> > > > > > > > > > > > > > > in case of the cancelation barrier?
> > > > > > > > > > > > > > > Maybe instead of ForceSimpleBarrier add a flag 
> > > > > > > > > > > > > > > EmitCancelBarrier and if it set to true, always 
> > > > > > > > > > > > > > > emit cancel barrier, otherwise always emit simple 
> > > > > > > > > > > > > > > barrier? And add an assertion for non-set 
> > > > > > > > > > > > > > > cancellation block or even accept it as a 
> > > > > > > > > > > > > > > parameter here.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > What is the difference in moving some of the 
> > > > > > > > > > > > > > boolean logic to the caller? Also, we have test to 
> > > > > > > > > > > > > > verify we get cancellation barriers if we need 
> > > > > > > > > > > > > > them, both unit tests and clang lit tests.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > Also, what if we have inner exception handling in 
> > > > > > > > > > > > > > > the region? Will you handle the cleanup correctly 
> > > > > > > > > > > > > > > in case of the cancelation barrier?
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > I think so. Right now through the code in clang 
> > > > > > > > > > > > > > that does the set up of the cancellation block, 
> > > > > > > > > > > > > > later through callbacks but we only need that for 
> > > > > > > > > > > > > > regions where we actually go out of some scope, 
> > > > > > > > > > > > > > e.g., parallel.
> > > > > > > > > > > > > 1. I'm just thinking about future users of thus 
> > > > > > > > > > > > > interface. It woild be good if we could provide safe 
> > > > > > > > > > > > > interface for all the users, not only clang.
> > > > > > > > > > > > > 2. Exit out of the OpenMP region is not allowed. But 
> > > > > > > > > > > > > you may have inner try...catch or just simple 
> > > > > > > > > > > > > compound statement with local vars that require 
> > > > > > > > > > > > > constructors/destructors. And the cancellation 
> > > > > > > > > > > > > barrier may exit out of these regions. And you need 
> > > > > > > > > > > > > to call all required destructors. You'd better to 
> > > > > > > > > > > > > think about it now, not later.
> > > > > > > > > > > > > 2. [...] You'd better to think about it now, not 
> > > > > > > > > > > > > later.
> > > > > > > > > > > > 
> > > > > > > > > > > > First, I do think about it now and I 

[PATCH] D70249: [clang-format] Fixed edge-case with `Style.SpacesInSquareBrackets` with trailing bare `&` lambda capture

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4ee70e00b509: [clang-format] Fixed edge-case with 
SpacesInSquareBrackets with trailing bare… (authored by mitchell-stellar).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70249

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10638,6 +10638,7 @@
   verifyFormat("return [ i, args... ] {};", Spaces);
   verifyFormat("int foo = [  ]() {};", Spaces);
   verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ & ]() {};", Spaces);
   verifyFormat("int foo = [ =,  ]() {};", Spaces);
   verifyFormat("int foo = [ , = ]() {};", Spaces);
 }
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2996,14 +2996,18 @@
 return true;
   }
   if (Left.is(TT_UnaryOperator)) {
-// The alternative operators for ~ and ! are "compl" and "not".
-// If they are used instead, we do not want to combine them with
-// the token to the right, unless that is a left paren.
 if (!Right.is(tok::l_paren)) {
+  // The alternative operators for ~ and ! are "compl" and "not".
+  // If they are used instead, we do not want to combine them with
+  // the token to the right, unless that is a left paren.
   if (Left.is(tok::exclaim) && Left.TokenText == "not")
 return true;
   if (Left.is(tok::tilde) && Left.TokenText == "compl")
 return true;
+  // Lambda captures allow for a lone &, so "&]" needs to be properly
+  // handled.
+  if (Left.is(tok::amp) && Right.is(tok::r_square))
+return Style.SpacesInSquareBrackets;
 }
 return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
Right.is(TT_BinaryOperator);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10638,6 +10638,7 @@
   verifyFormat("return [ i, args... ] {};", Spaces);
   verifyFormat("int foo = [  ]() {};", Spaces);
   verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ & ]() {};", Spaces);
   verifyFormat("int foo = [ =,  ]() {};", Spaces);
   verifyFormat("int foo = [ , = ]() {};", Spaces);
 }
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2996,14 +2996,18 @@
 return true;
   }
   if (Left.is(TT_UnaryOperator)) {
-// The alternative operators for ~ and ! are "compl" and "not".
-// If they are used instead, we do not want to combine them with
-// the token to the right, unless that is a left paren.
 if (!Right.is(tok::l_paren)) {
+  // The alternative operators for ~ and ! are "compl" and "not".
+  // If they are used instead, we do not want to combine them with
+  // the token to the right, unless that is a left paren.
   if (Left.is(tok::exclaim) && Left.TokenText == "not")
 return true;
   if (Left.is(tok::tilde) && Left.TokenText == "compl")
 return true;
+  // Lambda captures allow for a lone &, so "&]" needs to be properly
+  // handled.
+  if (Left.is(tok::amp) && Right.is(tok::r_square))
+return Style.SpacesInSquareBrackets;
 }
 return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
Right.is(TT_BinaryOperator);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69959: [C-index] Fix test when using Debug target & MSVC STL

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm

Related to this discussion, I noticed `to_vector` in STLExtras.h:
https://github.com/llvm/llvm-project/blob/e2369fd197d9e/llvm/include/llvm/ADT/STLExtras.h#L1329
I guess that would be the most idiomatic way to do this if we needed it.


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

https://reviews.llvm.org/D69959



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

[PATCH] D70101: [X86] Fix the implementation of __readcr3/__writecr3 to work in 64-bit mode

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D70101#1741565 , @craig.topper 
wrote:

> So in order to match the long and int behavior, I need two different macros 
> to replace the argument? I think __INTPTR_TYPE__ will return int on 32-bit 
> targets so I can't use that for the long case. But maybe for the __int64?


Right, we can use `__INTPTR_TYPE__` for all `__writecr*` functions (they use 
int or long long), and our own macro (`__LPTRINT_TYPE__`?) for `__readcr*`. 
Make sure to undef it so it doesn't leak out of the header, similar to 
`__DEFAULT_FN_ATTRS`.


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

https://reviews.llvm.org/D70101



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

[PATCH] D69970: [CGDebugInfo] Emit subprograms for decls when AT_tail_call is understood (reland with fixes)

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D69970#1745038 , @vsk wrote:

> Don't emit declaration subprograms for functions with reserved names. There 
> may not be much benefit from referencing these functions from call site tags 
> (e.g. instead of relying on call site info to work out the arguments passed 
> to __asan_memcpy, the asan runtime could log those arguments instead).
>
> This fixes a verifier failure:
>
>   inlinable function call in a function with debug info must have a !dbg 
> location
> %134 = call i8* @__asan_memcpy(i8* %agg.tmp2.sroa.0.0..sroa_cast10.i, i8* 
> %agg.tmp.sroa.0.i.0..sroa_cast, i64 32)
>
>
> @aprantl / @djtodoro, mind taking another look?


(my very vague take on this, admittedly having not followed this work in 
detail, is that this sounds pretty heuristical & not like it's a 
principled/general solution? But I don't really know much about all this & will 
leave actual approvals/disapprovals to the other folks who've been reviewing 
this stuff (& admittedly, as the person who introduced the "inlinable call with 
no debug location in debug-having function" assertion/failure, it is a bit 
contextual as to whether a certain call is inlinable, etc))


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

https://reviews.llvm.org/D69970



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

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Chi Chun Chen via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaOpenMP.cpp:2953-2954
+(!(isOpenMPTargetExecutionDirective(DKind) && Res &&
+   (*Res == OMPDeclareTargetDeclAttr::MT_Link ||
+*Res == OMPDeclareTargetDeclAttr::MT_To {
+  // Only check for data-mapping attribute and is_device_ptr here

ABataev wrote:
> ABataev wrote:
> > Why comparing the value of `*Res` here if just comparing against all 
> > possible values? This condition is always `true` if `Res` is true. I don't 
> > think we need to map variables with `to` mapping.
> Do we need to map `declare target to` vars at all?
I don't understand this since spec says that 
```
each variable referenced in the construct that does not have a predetermined 
data-sharing attribute and does not appear in a to or link clause on a declare 
target directive must be listed in a data-mapping attribute clause, a 
data-sharing attribute clause (including a data-sharing attribute clause on a 
combined construct where target is one of the constituent constructs), or an 
is_device_ptr clause.
```
So, I think I should check both to and link.
Or are you saying that to clause is impossible to happen here since we have a 
"Skip internally declared static variables" check so that if there is a to 
clause, it will return before hitting Line 2952. Therefore, we should only 
check for link clause?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[clang] 4ee70e0 - [clang-format] Fixed edge-case with SpacesInSquareBrackets with trailing bare "&" lambda capture.

[Mitchell Balan via cfe-commits]

Author: Mitchell Balan
Date: 2019-11-14T13:24:50-05:00
New Revision: 4ee70e00b509fe26bac4196df76dc7c6153f1206

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

LOG: [clang-format] Fixed edge-case with SpacesInSquareBrackets with trailing 
bare "&" lambda capture.

Summary:
Lambda captures allow for a lone `&` capture, so `&]` needs to be properly 
handled.

`int foo = [& ]() {}` is fixed to give `int foo = [ & ]() {}`

Reviewers: MyDeveloperDay

Reviewed by: MyDeveloperDay

Subscribers: cfe-commits

Tags: #clang, #clang-format

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index d3c9dd56f97c..c5ed49e9a409 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2996,14 +2996,18 @@ bool TokenAnnotator::spaceRequiredBefore(const 
AnnotatedLine ,
 return true;
   }
   if (Left.is(TT_UnaryOperator)) {
-// The alternative operators for ~ and ! are "compl" and "not".
-// If they are used instead, we do not want to combine them with
-// the token to the right, unless that is a left paren.
 if (!Right.is(tok::l_paren)) {
+  // The alternative operators for ~ and ! are "compl" and "not".
+  // If they are used instead, we do not want to combine them with
+  // the token to the right, unless that is a left paren.
   if (Left.is(tok::exclaim) && Left.TokenText == "not")
 return true;
   if (Left.is(tok::tilde) && Left.TokenText == "compl")
 return true;
+  // Lambda captures allow for a lone &, so "&]" needs to be properly
+  // handled.
+  if (Left.is(tok::amp) && Right.is(tok::r_square))
+return Style.SpacesInSquareBrackets;
 }
 return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
Right.is(TT_BinaryOperator);

diff  --git a/clang/unittests/Format/FormatTest.cpp 
b/clang/unittests/Format/FormatTest.cpp
index 01154e2c2f40..b8a73621c779 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -10638,6 +10638,7 @@ TEST_F(FormatTest, ConfigurableSpacesInSquareBrackets) {
   verifyFormat("return [ i, args... ] {};", Spaces);
   verifyFormat("int foo = [  ]() {};", Spaces);
   verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ & ]() {};", Spaces);
   verifyFormat("int foo = [ =,  ]() {};", Spaces);
   verifyFormat("int foo = [ , = ]() {};", Spaces);
 }



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

[PATCH] D70256: [FPEnv] clang support for constrained FP builtins

[Kevin P. Neal via Phabricator via cfe-commits]

kpn created this revision.
kpn added reviewers: rjmccall, dblaikie.
Herald added projects: clang, LLVM.
Herald added a subscriber: llvm-commits.

Currently if constrained fp math is requested clang is still emitting calls to 
the non-constrained versions of math builtins. This patch corrects that for 
calls that are not target-specific.

I really wanted to keep constrained/non-constrained decisions in the IRBuilder, 
but that causes complications. This version stays close to that ideal without 
the complications.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70256

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/constrained-math-builtins.c
  llvm/include/llvm/IR/IRBuilder.h

Index: llvm/include/llvm/IR/IRBuilder.h
===
--- llvm/include/llvm/IR/IRBuilder.h
+++ llvm/include/llvm/IR/IRBuilder.h
@@ -2096,6 +2096,8 @@
 case Intrinsic::experimental_constrained_fpext:
 case Intrinsic::experimental_constrained_fptoui:
 case Intrinsic::experimental_constrained_fptosi:
+case Intrinsic::experimental_constrained_lround:
+case Intrinsic::experimental_constrained_llround:
   C = CreateIntrinsic(ID, {DestTy, V->getType()}, {V, ExceptV}, nullptr,
   Name);
   break;
@@ -2306,6 +2308,37 @@
 Args, OpBundles, Name, FPMathTag);
   }
 
+  // Deprecated [opaque pointer types]
+  CallInst *CreateConstrainedFPCall(
+  Value *Callee, ArrayRef Args, const Twine  = "",
+  Optional Rounding = None,
+  Optional Except = None) {
+llvm::SmallVector UseArgs;
+
+for (auto *OneArg : Args)
+  UseArgs.push_back(OneArg);
+Function *F = cast(Callee);
+switch (F->getIntrinsicID()) {
+default:
+  UseArgs.push_back(getConstrainedFPRounding(Rounding));
+  break;
+case Intrinsic::experimental_constrained_fpext:
+case Intrinsic::experimental_constrained_fptoui:
+case Intrinsic::experimental_constrained_fptosi:
+case Intrinsic::experimental_constrained_lround:
+case Intrinsic::experimental_constrained_llround:
+  // No rounding metadata for these intrinsics.
+  break;
+}
+UseArgs.push_back(getConstrainedFPExcept(Except));
+
+CallInst *C = CreateCall(
+cast(Callee->getType()->getPointerElementType()), Callee,
+UseArgs, Name);
+setConstrainedFPCallAttr(C);
+return C;
+  }
+
   Value *CreateSelect(Value *C, Value *True, Value *False,
   const Twine  = "", Instruction *MDFrom = nullptr) {
 if (auto *CC = dyn_cast(C))
Index: clang/test/CodeGen/constrained-math-builtins.c
===
--- /dev/null
+++ clang/test/CodeGen/constrained-math-builtins.c
@@ -0,0 +1,150 @@
+// RUN: %clang_cc1 -triple x86_64-linux -ffp-exception-behavior=strict -w -S -o - -emit-llvm %s | FileCheck %s
+
+// Test codegen of constrained math builtins.
+
+void foo(double *d, float f, float *fp, long double *l, int *i, const char *c) {
+  f = __builtin_fmod(f,f);f = __builtin_fmodf(f,f);   f =  __builtin_fmodl(f,f);
+
+// CHECK: declare double @llvm.experimental.constrained.frem.f64(double, double, metadata, metadata)
+// CHECK: declare float @llvm.experimental.constrained.frem.f32(float, float, metadata, metadata)
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.frem.f80(x86_fp80, x86_fp80, metadata, metadata)
+
+  __builtin_pow(f,f);__builtin_powf(f,f);   __builtin_powl(f,f);
+
+// CHECK: declare double @llvm.experimental.constrained.pow.f64(double, double, metadata, metadata)
+// CHECK: declare float @llvm.experimental.constrained.pow.f32(float, float, metadata, metadata)
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.pow.f80(x86_fp80, x86_fp80, metadata, metadata)
+
+  __builtin_powi(f,f);__builtin_powif(f,f);   __builtin_powil(f,f);
+
+// CHECK: declare double @llvm.experimental.constrained.powi.f64(double, i32, metadata, metadata)
+// CHECK: declare float @llvm.experimental.constrained.powi.f32(float, i32, metadata, metadata)
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.powi.f80(x86_fp80, i32, metadata, metadata)
+
+  __builtin_ceil(f);   __builtin_ceilf(f);  __builtin_ceill(f);
+
+// CHECK: declare double @llvm.experimental.constrained.ceil.f64(double, metadata, metadata)
+// CHECK: declare float @llvm.experimental.constrained.ceil.f32(float, metadata, metadata)
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.ceil.f80(x86_fp80, metadata, metadata)
+
+  __builtin_cos(f);__builtin_cosf(f);   __builtin_cosl(f); 
+
+// CHECK: declare double @llvm.experimental.constrained.cos.f64(double, metadata, metadata)
+// CHECK: declare float @llvm.experimental.constrained.cos.f32(float, metadata, metadata)
+// CHECK: declare x86_fp80 @llvm.experimental.constrained.cos.f80(x86_fp80, metadata, metadata)
+
+  __builtin_exp(f);__builtin_expf(f);   

[PATCH] D19031: [clang-format] Flexible line endings

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a subscriber: STL_MSFT.
MyDeveloperDay added a comment.

I suspect this patch might need a  rebase, but I personally don't see anything 
wrong with it. I think this could really help.

I'm pretty sure the Unix line ending requirement is in almost every major style 
guide (I saw @STL_MSFT  mention that they standardize on \r\n too) so I think 
it meets the criteria. (I know i'll personally use it as I live in a 
cross-platform shop and users forgetting to configure their Visual Studio is a 
major pain!)

If you're happy to rebase it, I'm happy to review it again.


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

https://reviews.llvm.org/D19031



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

[PATCH] D69979: clang: Guess at some platform FTZ/DAZ default settings

[Sanjay Patel via Phabricator via cfe-commits]

spatel added a comment.

In D69979#1740294 , @arsenm wrote:

> In D69979#1738099 , @craig.topper 
> wrote:
>
> > I checked Redhat 7.4 that's on the server I'm using for work. And I had a 
> > coworker check his Ubuntu 18.04 system with this program. And both systems 
> > printed 1f80 as the value of MXCSR which shows FTZ and DAZ are both 0. Are 
> > you seeing something different?
> >
> >   #include 
> >   #include 
> >  
> >   int main() {
> > int csr = _mm_getcsr();
> > printf("%x\n", csr);
> > return 0;
> >   }
> >
>
>
> I see the value as 1f80. However the test program I wrote suggests the 
> default is to flush (and what the comments in bug 34994 suggest?):


Is the test program attached somewhere? 
Bug 34994 (https://bugs.llvm.org/show_bug.cgi?id=34994) was limited to changing 
cases where we are running in some kind of loose-FP environment (otherwise, we 
would not be generating a sqrt estimate sequence at all). In the default 
(IEEE-compliant) environment, x86 would use a full-precision sqrt instruction 
or make a call to libm sqrt.


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

https://reviews.llvm.org/D69979



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

[PATCH] D70253: [AArch64][SVE2] Implement remaining SVE2 floating-point intrinsics

[Kerry McLaughlin via Phabricator via cfe-commits]

kmclaughlin created this revision.
kmclaughlin added reviewers: huntergr, sdesmalen, dancgr.
Herald added subscribers: hiraditya, kristof.beyls, tschuett.
Herald added a project: LLVM.

Adds the following intrinsics:

- faddp
- fmaxp, fminp, fmaxnmp & fminnmp
- fmlalb, fmlalt, fmlslb & fmlslt
- flogb


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70253

Files:
  llvm/include/llvm/IR/IntrinsicsAArch64.td
  llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
  llvm/lib/Target/AArch64/SVEInstrFormats.td
  llvm/test/CodeGen/AArch64/sve2-intrinsics-fp-int-binary-logarithm.ll
  llvm/test/CodeGen/AArch64/sve2-intrinsics-fp-widening-mul-acc.ll
  llvm/test/CodeGen/AArch64/sve2-intrinsics-non-widening-pairwise-arith.ll

Index: llvm/test/CodeGen/AArch64/sve2-intrinsics-non-widening-pairwise-arith.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/sve2-intrinsics-non-widening-pairwise-arith.ll
@@ -0,0 +1,191 @@
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve2 < %s | FileCheck %s
+
+;
+; FADDP
+;
+
+define  @faddp_f16( %pg,  %a,  %b) {
+; CHECK-LABEL: faddp_f16:
+; CHECK: faddp z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.faddp.nxv8f16( %pg,
+   %a,
+   %b)
+  ret  %out
+}
+
+define  @faddp_f32( %pg,  %a,  %b) {
+; CHECK-LABEL: faddp_f32:
+; CHECK: faddp z0.s, p0/m, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.faddp.nxv4f32( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+define  @faddp_f64( %pg,  %a,  %b) {
+; CHECK-LABEL: faddp_f64:
+; CHECK: faddp z0.d, p0/m, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.faddp.nxv2f64( %pg,
+ %a,
+ %b)
+  ret  %out
+}
+
+;
+; FMAXP
+;
+
+define  @fmaxp_f16( %pg,  %a,  %b) {
+; CHECK-LABEL: fmaxp_f16:
+; CHECK: fmaxp z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fmaxp.nxv8f16( %pg,
+   %a,
+   %b)
+  ret  %out
+}
+
+define  @fmaxp_f32( %pg,  %a,  %b) {
+; CHECK-LABEL: fmaxp_f32:
+; CHECK: fmaxp z0.s, p0/m, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fmaxp.nxv4f32( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+define  @fmaxp_f64( %pg,  %a,  %b) {
+; CHECK-LABEL: fmaxp_f64:
+; CHECK: fmaxp z0.d, p0/m, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fmaxp.nxv2f64( %pg,
+ %a,
+ %b)
+  ret  %out
+}
+
+;
+; FMAXNMP
+;
+
+define  @fmaxnmp_f16( %pg,  %a,  %b) {
+; CHECK-LABEL: fmaxnmp_f16:
+; CHECK: fmaxnmp z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fmaxnmp.nxv8f16( %pg,
+ %a,
+ %b)
+  ret  %out
+}
+
+define  @fmaxnmp_f32( %pg,  %a,  %b) {
+; CHECK-LABEL: fmaxnmp_f32:
+; CHECK: fmaxnmp z0.s, p0/m, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fmaxnmp.nxv4f32( %pg,
+  %a,
+  %b)
+  ret  %out
+}
+
+define  @fmaxnmp_f64( %pg,  %a,  %b) {
+; CHECK-LABEL: fmaxnmp_f64:
+; CHECK: fmaxnmp z0.d, p0/m, z0.d, z1.d
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fmaxnmp.nxv2f64( %pg,
+   %a,
+   %b)
+  ret  %out
+}
+
+;
+; FMINP
+;
+
+define  @fminp_f16( %pg,  %a,  %b) {
+; CHECK-LABEL: fminp_f16:
+; CHECK: fminp z0.h, p0/m, z0.h, z1.h
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fminp.nxv8f16( %pg,
+   %a,
+   %b)
+  ret  %out
+}
+
+define  @fminp_f32( %pg,  %a,  %b) {
+; CHECK-LABEL: fminp_f32:
+; CHECK: fminp z0.s, p0/m, z0.s, z1.s
+; CHECK-NEXT: ret
+  %out = call  @llvm.aarch64.sve.fminp.nxv4f32( %pg,
+%a,
+%b)
+  ret  %out
+}
+
+define  @fminp_f64( %pg,  %a,  %b) {
+; CHECK-LABEL: fminp_f64:
+; CHECK: 

[PATCH] D70196: [clang-include-fixer] Skip .rc files when finding symbols

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe2369fd197d9: [clang-include-fixer] Skip .rc files when 
finding symbols (authored by rnk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70196

Files:
  
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py


Index: 
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
===
--- 
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
+++ 
clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
@@ -89,6 +89,9 @@
   database = json.load(open(os.path.join(build_path, db_path)))
   files = [entry['file'] for entry in database]
 
+  # Filter out .rc files on Windows. CMake includes them for some reason.
+  files = [f for f in files if not f.endswith('.rc')]
+
   max_task = args.j
   if max_task == 0:
 max_task = multiprocessing.cpu_count()


Index: clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
===
--- clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
+++ clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
@@ -89,6 +89,9 @@
   database = json.load(open(os.path.join(build_path, db_path)))
   files = [entry['file'] for entry in database]
 
+  # Filter out .rc files on Windows. CMake includes them for some reason.
+  files = [f for f in files if not f.endswith('.rc')]
+
   max_task = args.j
   if max_task == 0:
 max_task = multiprocessing.cpu_count()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70041: register cuda language activation event and activate for .cuh files

[Paul Taylor via Phabricator via cfe-commits]

ptaylor added inline comments.



Comment at: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts:92
+  // *not* load itself automatically on '.cu' and '.cuh' files.
+  const cudaFilePatterns: {scheme: string, pattern: string}[] = [
+{scheme : 'file', pattern : '**/*.{cu}'},

hokein wrote:
> I think we could simplify the code, we could move this part to the 
> package.json under the `contributes` umbrella, something like
> 
> ```
>   "contributes": {
> "languages": [
> {
> "id": "cuda",
> "filenamePatterns": [
> "**/*.{cu}",
> "**/*.{cuh}",
> ],
> }
> ]
> ```
> 
> then in the extension.ts, we only need an entry `{ scheme: 'file', language: 
> 'cuda' }` when initailizing the clientOptions.
Yes that's an option. I chose this approach to stay consistent with the current 
behavior, because `vscode-clangd` can do CUDA intellisense and refactoring, it 
doesn't contribute the CUDA grammar definitions for syntax highlighting.

The VSCode docs aren't clear on whether multiple extensions can contribute 
separate features for the same language, or what happens when two plugins both 
contribute languages with the same `languageId`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70041



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

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

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

dexonsmith added inline comments.



Comment at: clang/test/CodeGenObjC/direct-method.m:25
+  // CHECK-NEXT: store %0* %self, %0** %self.addr,
+  // CHECK-NEXT: store i8* %_cmd, i8** %_cmd.addr,
+

rjmccall wrote:
> The IR names of local values aren't emitted in release builds of the 
> compiler, so you need to use FileCheck variables for these allocas the same 
> way that you do for RETVAL and SELF.
That’s worth double-checking.  I think that has been fixed, such that whether 
names are dropped is controlled by a cc1 flag (which the driver sets by default 
in release builds).


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

https://reviews.llvm.org/D69991



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

[PATCH] D70249: [clang-format] Fixed edge-case with `Style.SpacesInSquareBrackets` with trailing bare `&` lambda capture

[MyDeveloperDay via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

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

https://reviews.llvm.org/D70249



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

[PATCH] D70196: [clang-include-fixer] Skip .rc files when finding symbols

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

This is a single .rc entry:

  {
"directory": "C:/src/llvm-project/build",
"command": "C:\\PROGRA~2\\WI3CF2~1\\10\\bin\\100183~1.0\\x64\\rc.exe 
-DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE 
-D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_
  DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_HAS_EXCEPTIONS=0 
-D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__
  STDC_LIMIT_MACROS -DRC_FILE_VERSION=\\\"10.0.0git\\\" 
-DRC_INTERNAL_NAME=\\\"llvm-tblgen\\\" -DRC_PRODUCT_NAME=\\\"LLVM\\\" 
-DRC_PRODUCT_VERSION=\\\"10.0.0git\\\" -DRC_VERS
  ION_FIELD_1=10 -DRC_VERSION_FIELD_2=0 -DRC_VERSION_FIELD_3=0 
-DRC_VERSION_FIELD_4=0 -IC:\\src\\llvm-project\\build\\utils\\TableGen 
-IC:\\src\\llvm-project\\llvm\\utils\\Ta
  bleGen -IC:\\src\\llvm-project\\build\\include 
-IC:\\src\\llvm-project\\llvm\\include  /DWIN32   -UNDEBUG /nologo 
/foutils\\TableGen\\CMakeFiles\\llvm-tblgen.dir\\__\\__\\r
  esources\\windows_version_resource.rc.res 
C:\\src\\llvm-project\\llvm\\resources\\windows_version_resource.rc",
"file": "C:/src/llvm-project/llvm/resources/windows_version_resource.rc"
  },

There is one for every binary. I think .rc files can include headers to get 
things like struct sizes and macros. My understanding is that it's an old fork 
of the Visual C compiler, so it is in some sense a "compilation". I suppose if 
you wanted to write a global renaming tool for a macro, you'd need to know 
about uses in .rc files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70196



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

[clang-tools-extra] e2369fd - [clang-include-fixer] Skip .rc files when finding symbols

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2019-11-14T10:04:35-08:00
New Revision: e2369fd197d9ed9916bf78b2c8f6d7b8e0d66691

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

LOG: [clang-include-fixer] Skip .rc files when finding symbols

Summary:
For some reason CMake includes entries for .rc files, but
find-all-symbols handles them improperly.

See PR43993

Reviewers: sammccall, bkramer

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 

clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py

Removed: 




diff  --git 
a/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
 
b/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
index 5e9dde723435..02b100bd849b 100755
--- 
a/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
+++ 
b/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/run-find-all-symbols.py
@@ -89,6 +89,9 @@ def main():
   database = json.load(open(os.path.join(build_path, db_path)))
   files = [entry['file'] for entry in database]
 
+  # Filter out .rc files on Windows. CMake includes them for some reason.
+  files = [f for f in files if not f.endswith('.rc')]
+
   max_task = args.j
   if max_task == 0:
 max_task = multiprocessing.cpu_count()



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

[clang] 399e29d - [OPENMP]Add assignment operator in UDR test, NFC.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2019-11-14T13:01:16-05:00
New Revision: 399e29ddc600a2d91e08e7029e7dade3581c9820

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

LOG: [OPENMP]Add assignment operator in UDR test, NFC.

Add assignment operator in the test to check that even if the operator
was declare explicitly, the constructor is called in the user-defined
reduction initializer anyway.

Added: 


Modified: 
clang/test/OpenMP/for_reduction_codegen_UDR.cpp

Removed: 




diff  --git a/clang/test/OpenMP/for_reduction_codegen_UDR.cpp 
b/clang/test/OpenMP/for_reduction_codegen_UDR.cpp
index f3816de5f5bf..6d8223c724f5 100644
--- a/clang/test/OpenMP/for_reduction_codegen_UDR.cpp
+++ b/clang/test/OpenMP/for_reduction_codegen_UDR.cpp
@@ -38,6 +38,7 @@ struct S : public BaseS, public BaseS1 {
   T f;
   S(T a) : f(a + g) {}
   S() : f(g) {}
+  S& operator=(const S&);
   ~S() {}
 };
 void red(BaseS1&, const BaseS1&);



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

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Are we sure using both Itanium and MS C++ ABIs at the same time is really the 
best way forward here? What are the constraints on CUDA that require the 
Itanium ABI? I'm sure there are real reasons you can't just use the MS ABI as 
is, but I'm curious what they are. Was there some RFC or design showing that 
this is the right way forward?

I wonder if it would be more productive to add new, more expansive attributes, 
similar to `__attribute__((ms_struct))`, that tag class or function decls as MS 
or Itanium C++ ABI. CUDA could then leverage this as needed, and it would be 
much easier to construct test cases for MS/Itanium interop. This is an 
expansion in scope, but it seems like it could be generally useful, and if 
we're already going to enter the crazy world of multiple C++ ABIs in a single 
TU, we might as well bite the bullet and do it in a way that isn't specific to 
CUDA.


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

https://reviews.llvm.org/D70172



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

[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

I would suggest to split this patch into 2 part: the first one is NFC and just 
makes the ompiler to use the new interfaces and the second with the new 
functionality for mappers.




Comment at: include/clang/AST/OpenMPClause.h:4298
+  /// use_device_ptr and is_device_ptr.
+  bool hasMapper;
+

Not in Camel format



Comment at: include/clang/AST/OpenMPClause.h:4543
   /// of the class.
-  ArrayRef getUDMapperRefs() const {
-return llvm::makeArrayRef(
-static_cast(this)->template getTrailingObjects() +
+  ArrayRef getUDMapperRefs() const {
+assert(hasMapper &&

Why changed to `const Expr *` here? I believe, `ArrayRef` makes it constant 
automatically.



Comment at: include/clang/AST/OpenMPClause.h:4596
+// Whether this clause is possible to have user-defined mappers associated.
+bool hasMapper;
+

Not in Camel format



Comment at: include/clang/AST/OpenMPClause.h:4599
+// The user-defined mapper associated with the current declaration.
+ArrayRef::iterator MapperCur;
+

No need for const in `ArrayRef`



Comment at: include/clang/AST/OpenMPClause.h:4621
 ArrayRef CumulativeListSizes,
-MappableExprComponentListRef Components)
+MappableExprComponentListRef Components, bool hasMapper,
+ArrayRef Mappers)

Not in Camel format



Comment at: include/clang/AST/OpenMPClause.h:4743
+getComponentsRef(), hasMapper,
+hasMapper ? getUDMapperRefs() : ArrayRef());
   }

`ArrayRef()`->`llvm::None`?



Comment at: include/clang/AST/OpenMPClause.h:4750
+ getComponentsRef().end()),
+hasMapper, ArrayRef());
   }

`ArrayRef()`->`llvm::None`



Comment at: include/clang/AST/OpenMPClause.h:4763
+getComponentListSizesRef(), getComponentsRef(), hasMapper,
+hasMapper ? getUDMapperRefs() : ArrayRef());
   }

Same here



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:742
   // Call to void __tgt_target_data_end(int64_t device_id, int32_t arg_num,
-  // void** args_base, void **args, size_t *arg_sizes, int64_t *arg_types);
+  // void** args_base, void **args, int64_t *arg_sizes, int64_t *arg_types);
   OMPRTL__tgt_target_data_end,

Better to fix it in a separate patch



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:755-794
+  // Call to int32_t __tgt_target_mapper(int64_t device_id, void *host_ptr,
+  // int32_t arg_num, void** args_base, void **args, int64_t *arg_sizes, 
int64_t
+  // *arg_types, void **arg_mappers);
+  OMPRTL__tgt_target_mapper,
+  // Call to int32_t __tgt_target_nowait_mapper(int64_t device_id, void
+  // *host_ptr, int32_t arg_num, void** args_base, void **args, int64_t
+  // *arg_sizes, int64_t *arg_types, void **arg_mappers);

I believe these new functions replace completely the old one, so old functions 
must be removed



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:2746
+break;
+  }
   case OMPRTL__tgt_mapper_num_components: {

Same here, remove processing for old functions.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7388
 
-  llvm::Value *getExprTypeSize(const Expr *E) const {
+  llvm::Value *getExprTypeSize(const Expr *E, bool hasMapper) const {
 QualType ExprTy = E->getType().getCanonicalType();

CamelCase



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7388
 
-  llvm::Value *getExprTypeSize(const Expr *E) const {
+  llvm::Value *getExprTypeSize(const Expr *E, bool hasMapper) const {
 QualType ExprTy = E->getType().getCanonicalType();

ABataev wrote:
> CamelCase
Seems to me, with mapper you're changing the contract of this function. It is 
supposed to return the size in bytes, with Mapper it returns just number of 
elements. Bad decision. Better to convert size in bytes to number of elements 
in place where you need it.



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7411
+if (hasMapper)
+  return CGF.Builder.getInt64(1);
+else

Why do we return `1` here?



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7412
+  return CGF.Builder.getInt64(1);
+else
+  return CGF.getTypeSize(BaseTy);

No need for `else` here



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7431-7434
+  return CGF.Builder.getInt64(1);
+else
+  return ElemSize;
+  }

Same here



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:7445
+  return LengthVal;
+else
+  return CGF.Builder.CreateNUWMul(LengthVal, ElemSize);

No need for `else` here

[PATCH] D69759: [BPF] Add preserve_access_index attribute for record definition

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Why did this get committed when there were still outstanding review questions 
that were not answered?

In D69759#1739935 , @yonghong-song 
wrote:

> Regarding to this one "This is missing a bunch of C++ tests that show what 
> happens in the case of inheritance, template instantiations, etc."
>  Could you give me some example tests which I can take a look in order to add 
> support. FYI, BPF backend publicly only supports C language,
>  (a restrict C for example __thread keyword is not supported). I guess 
> template instantiations does not apply here? Or we can still test
>  template instantiation since we are at clang stage?


If the attribute only makes sense in C mode, it should not be accepted in C++ 
mode. However, it currently is accepted in C++ mode which is why I was asking 
for tests demonstrating how it should behave.

In D69759#1742264 , @yonghong-song 
wrote:

> @aaron.ballman just ping, could you let me know if you have any further 
> comments? Thanks!


I was out all last week at wg21 meetings and a short vacation, so that's why 
the delay in review. FWIW, we usually give a week before pinging a review.




Comment at: clang/include/clang/Basic/Attr.td:1584
+ TargetSpecificAttr  {
+  let Spellings = [Clang<"preserve_access_index">];
+  let Subjects = SubjectList<[Record], ErrorDiag>;

If this attribute is only supported in C mode, then this attribute should have: 
`let LangOpts = [COnly];` in it.



Comment at: clang/lib/CodeGen/CGExpr.cpp:3411-3422
+  const auto *ImplicitCast = dyn_cast(ArrayBase);
+  if (!ImplicitCast)
+return false;
+
+  // Only support base as either a MemberExpr or DeclRefExpr.
+  // DeclRefExpr to cover cases like:
+  //struct s { int a; int b[10]; };

Is there a reason to do this as opposed to `IgnoreImpCasts()`?



Comment at: clang/lib/CodeGen/CGExpr.cpp:3423-3424
+  const Expr *E = ImplicitCast->getSubExpr();
+  const auto *MemberCast = dyn_cast(E);
+  if (MemberCast)
+return MemberCast->getMemberDecl()->hasAttr();

```
if (const auto *ME = dyn_cast(E))
  return ...
```




Comment at: clang/lib/CodeGen/CGExpr.cpp:3427-3428
+
+  const auto *DeclRefCast = dyn_cast(E);
+  if (DeclRefCast) {
+const VarDecl *VarDef = dyn_cast(DeclRefCast->getDecl());

Similar here -- you can sink the variable into the `if`.



Comment at: clang/lib/CodeGen/CGExpr.cpp:3433
+
+const auto *PtrT = dyn_cast(VarDef->getType().getTypePtr());
+if (!PtrT)

`const auto *PtrT = VarDef->getType()->getAs();`



Comment at: clang/lib/CodeGen/CGExpr.cpp:3440-3443
+while (TypedefT) {
+  PointeeT = TypedefT->desugar().getTypePtr();
+  TypedefT = dyn_cast(PointeeT);
+}

Is there a reason you only want to strip off typedefs, and not other forms of 
sugar? e.g., why not call `getUnqualifiedDesugaredType()`?



Comment at: clang/lib/CodeGen/CGExpr.cpp:3446
+// Not a typedef any more, it should be an elaborated type.
+const auto ElaborateT = dyn_cast(PointeeT);
+if (!ElaborateT)

`const auto *`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69759



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

[PATCH] D69204: [OpenMP 5.0] - Extend defaultmap

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:2953-2954
+(!(isOpenMPTargetExecutionDirective(DKind) && Res &&
+   (*Res == OMPDeclareTargetDeclAttr::MT_Link ||
+*Res == OMPDeclareTargetDeclAttr::MT_To {
+  // Only check for data-mapping attribute and is_device_ptr here

ABataev wrote:
> Why comparing the value of `*Res` here if just comparing against all possible 
> values? This condition is always `true` if `Res` is true. I don't think we 
> need to map variables with `to` mapping.
Do we need to map `declare target to` vars at all?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:2846
+// 1. the implicit behavior for aggregate is tofrom
+// 2. it's a declare target link/to
+if (IsAggregateOrDeclareTarget) {

Is this still true for `decalare target to` vars?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69204



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

[PATCH] D69825: [Clang][Driver] Bypass cc1 process and re-enter driver main

[Alexandre Ganea via Phabricator via cfe-commits]

aganea updated this revision to Diff 229333.
aganea marked an inline comment as done.
aganea added a comment.

Call into `ExecuteCC1Tool()` directly, as suggested by @hans
Add more comments.
Remove `pThis` in `CrashRecoveryContext.cpp:RunSafely()` as suggested by 
@zturner


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

https://reviews.llvm.org/D69825

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Job.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Job.cpp
  clang/tools/driver/driver.cpp
  llvm/include/llvm/Support/CrashRecoveryContext.h
  llvm/lib/Support/CrashRecoveryContext.cpp
  llvm/lib/Support/Unix/Signals.inc
  llvm/lib/Support/Windows/Signals.inc

Index: llvm/lib/Support/Windows/Signals.inc
===
--- llvm/lib/Support/Windows/Signals.inc
+++ llvm/lib/Support/Windows/Signals.inc
@@ -187,7 +187,7 @@
 using namespace llvm;
 
 // Forward declare.
-static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep);
+LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep);
 static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType);
 
 // The function to call if ctrl-c is pressed.
@@ -521,10 +521,13 @@
 extern "C" VOID WINAPI RtlCaptureContext(PCONTEXT ContextRecord);
 #endif
 
-void llvm::sys::PrintStackTrace(raw_ostream ) {
-  STACKFRAME64 StackFrame = {};
-  CONTEXT Context = {};
-  ::RtlCaptureContext();
+static void LocalPrintStackTrace(raw_ostream , PCONTEXT C) {
+  STACKFRAME64 StackFrame{};
+  CONTEXT Context{};
+  if (!C) {
+::RtlCaptureContext();
+C = 
+  }
 #if defined(_M_X64)
   StackFrame.AddrPC.Offset = Context.Rip;
   StackFrame.AddrStack.Offset = Context.Rsp;
@@ -546,9 +549,12 @@
   StackFrame.AddrStack.Mode = AddrModeFlat;
   StackFrame.AddrFrame.Mode = AddrModeFlat;
   PrintStackTraceForThread(OS, GetCurrentProcess(), GetCurrentThread(),
-   StackFrame, );
+   StackFrame, C);
 }
 
+void llvm::sys::PrintStackTrace(raw_ostream ) {
+  LocalPrintStackTrace(OS, nullptr);
+}
 
 void llvm::sys::SetInterruptFunction(void (*IF)()) {
   RegisterHandler();
@@ -785,7 +791,7 @@
   return std::error_code();
 }
 
-static LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) {
+LONG WINAPI LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) {
   Cleanup();
 
   // We'll automatically write a Minidump file here to help diagnose
@@ -803,42 +809,9 @@
<< "\n";
   }
 
-  // Initialize the STACKFRAME structure.
-  STACKFRAME64 StackFrame = {};
-
-#if defined(_M_X64)
-  StackFrame.AddrPC.Offset = ep->ContextRecord->Rip;
-  StackFrame.AddrPC.Mode = AddrModeFlat;
-  StackFrame.AddrStack.Offset = ep->ContextRecord->Rsp;
-  StackFrame.AddrStack.Mode = AddrModeFlat;
-  StackFrame.AddrFrame.Offset = ep->ContextRecord->Rbp;
-  StackFrame.AddrFrame.Mode = AddrModeFlat;
-#elif defined(_M_IX86)
-  StackFrame.AddrPC.Offset = ep->ContextRecord->Eip;
-  StackFrame.AddrPC.Mode = AddrModeFlat;
-  StackFrame.AddrStack.Offset = ep->ContextRecord->Esp;
-  StackFrame.AddrStack.Mode = AddrModeFlat;
-  StackFrame.AddrFrame.Offset = ep->ContextRecord->Ebp;
-  StackFrame.AddrFrame.Mode = AddrModeFlat;
-#elif defined(_M_ARM64) || defined(_M_ARM)
-  StackFrame.AddrPC.Offset = ep->ContextRecord->Pc;
-  StackFrame.AddrPC.Mode = AddrModeFlat;
-  StackFrame.AddrStack.Offset = ep->ContextRecord->Sp;
-  StackFrame.AddrStack.Mode = AddrModeFlat;
-#if defined(_M_ARM64)
-  StackFrame.AddrFrame.Offset = ep->ContextRecord->Fp;
-#else
-  StackFrame.AddrFrame.Offset = ep->ContextRecord->R11;
-#endif
-  StackFrame.AddrFrame.Mode = AddrModeFlat;
-#endif
-
-  HANDLE hProcess = GetCurrentProcess();
-  HANDLE hThread = GetCurrentThread();
-  PrintStackTraceForThread(llvm::errs(), hProcess, hThread, StackFrame,
-   ep->ContextRecord);
+  LocalPrintStackTrace(llvm::errs(), ep ? ep->ContextRecord : nullptr);
 
-  _exit(ep->ExceptionRecord->ExceptionCode);
+  return EXCEPTION_EXECUTE_HANDLER;
 }
 
 static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType) {
Index: llvm/lib/Support/Unix/Signals.inc
===
--- llvm/lib/Support/Unix/Signals.inc
+++ llvm/lib/Support/Unix/Signals.inc
@@ -79,7 +79,7 @@
 
 using namespace llvm;
 
-static RETSIGTYPE SignalHandler(int Sig);  // defined below.
+RETSIGTYPE SignalHandler(int Sig);  // defined below.
 static RETSIGTYPE InfoSignalHandler(int Sig);  // defined below.
 
 using SignalHandlerFunctionType = void (*)();
@@ -341,7 +341,7 @@
 }
 
 // The signal handler that runs.
-static RETSIGTYPE SignalHandler(int Sig) {
+RETSIGTYPE SignalHandler(int Sig) {
   // Restore the signal behavior to default, so that the program actually
   // crashes when we return and the signal reissues.  This also ensures that if
   // we crash in our signal handler that the program will terminate immediately
@@ -356,8 +356,8 @@
   {
 

[PATCH] D68578: [HIP] Fix device stub name

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1099-1108
+auto *FD = const_cast((ND));
+if (auto *TD = cast(FD)->getPrimaryTemplate())
+  FD = TD->getTemplatedDecl();
+auto OldDeclName = FD->getDeclName();
+auto NewNameStr = std::string("__device_stub__") + 
OldDeclName.getAsString();
+auto *NewId = (NewNameStr);
+auto NewDeclName = DeclarationName(NewId);

yaxunl wrote:
> tra wrote:
> > On one hand I like this patch variant much better than the one that changed 
> > the mangling itself.
> > On the other hand this code appears to reply on implementation details. 
> > I.e. we're setting new name on `FD` which may or may not be the same as 
> > `ND`, but we're always passing `ND` to `getMangledNameImpl()`. 
> > 
> > Perhaps we could implement name-tweaking as another `MultiVersionKind` 
> > which we already plumb into getMangledNameImpl() and which allows changing 
> > the name for target attributes & features.
> > 
> > 
> The mangled name of an instantiated template function does not depends on its 
> own name, but on the template. If we do not want to depend on this 
> implementation detail, it seems I have to clone the template and instantiate 
> from the clone.
> 
> MultiVersion does not help us here since it only appends .postfix to mangled 
> name. The obstacle we are facing is how to change the unmangled name.
> The mangled name of an instantiated template function does not depends on its 
> own name, but on the template. If we do not want to depend on this 
> implementation detail, it seems I have to clone the template and instantiate 
> from the clone.

That would be putting more effort into working around the fact that 
`getMangledNameImpl()` doesa not provide a good API to change the name the way 
you need to. *That's* what needs to be addressed, IMO.

>MultiVersion does not help us here since it only appends .postfix to mangled 
>name. The obstacle we are facing is how to change the unmangled name.

*Some* existing implementations append to the mangled name, but we can do other 
manipulations there, too. The string with the mangled name originates in 
`getMangledNameImpl` and we could do more than just append to it. We do not 
have to use the `MultiVersion` for that, either. E.g. we prepend `__regcall3__` 
to the names of functions with `CC_X86RegCall` calling convention. We could do 
something similar for the kernel stub, I wonder if we could just generate a 
unique name and be done with that?

Hmm. Unique name probably would not do if, let's say, a kernel is defined in 
one TU, but we need to call it from another TU. So, whichever way we change the 
name of the stub, it will need to be the same everywhere. 
You may want to add a test verifying that launching of declaration-only kernels 
uses the right name.

Consistency of name mangling means that we do need to include regular 
C++-mangled information. Which means we need to do the name tweaking deeper 
down. How about using calling conventions? It's been suggested in the past that 
a lot of shenanigans around kernel launches could/should be done as a different 
calling convention. One of the things affected by the calling convention is 
mangling and we can add prefix there: 
https://github.com/llvm/llvm-project/blob/master/clang/lib/AST/Mangle.cpp#L164

We could tag host-side kernel with 'kernel call' calling convention on the host 
side and then plumb prefixing to be done similar to `__regcall3__`.

If that works that may be a useful improvement overall. For instance, we may no 
longer need to stash a `it's a kernel` flag among attributes and it would 
probably be useful for other things (e.g enforcing address space requirements 
for kernel pointer arguments).



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

https://reviews.llvm.org/D68578



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

[PATCH] D70249: [clang-format] Fixed edge-case with `Style.SpacesInSquareBrackets` with trailing bare `&` lambda capture

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar created this revision.
mitchell-stellar added a reviewer: MyDeveloperDay.
mitchell-stellar added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Lambda captures allow for a lone `&` capture, so `&]` needs to be properly 
handled.


Repository:
  rC Clang

https://reviews.llvm.org/D70249

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10638,6 +10638,7 @@
   verifyFormat("return [ i, args... ] {};", Spaces);
   verifyFormat("int foo = [  ]() {};", Spaces);
   verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ & ]() {};", Spaces);
   verifyFormat("int foo = [ =,  ]() {};", Spaces);
   verifyFormat("int foo = [ , = ]() {};", Spaces);
 }
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2996,14 +2996,18 @@
 return true;
   }
   if (Left.is(TT_UnaryOperator)) {
-// The alternative operators for ~ and ! are "compl" and "not".
-// If they are used instead, we do not want to combine them with
-// the token to the right, unless that is a left paren.
 if (!Right.is(tok::l_paren)) {
+  // The alternative operators for ~ and ! are "compl" and "not".
+  // If they are used instead, we do not want to combine them with
+  // the token to the right, unless that is a left paren.
   if (Left.is(tok::exclaim) && Left.TokenText == "not")
 return true;
   if (Left.is(tok::tilde) && Left.TokenText == "compl")
 return true;
+  // Lambda captures allow for a lone &, so "&]" needs to be properly
+  // handled.
+  if (Left.is(tok::amp) && Right.is(tok::r_square))
+return Style.SpacesInSquareBrackets;
 }
 return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
Right.is(TT_BinaryOperator);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10638,6 +10638,7 @@
   verifyFormat("return [ i, args... ] {};", Spaces);
   verifyFormat("int foo = [  ]() {};", Spaces);
   verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ & ]() {};", Spaces);
   verifyFormat("int foo = [ =,  ]() {};", Spaces);
   verifyFormat("int foo = [ , = ]() {};", Spaces);
 }
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2996,14 +2996,18 @@
 return true;
   }
   if (Left.is(TT_UnaryOperator)) {
-// The alternative operators for ~ and ! are "compl" and "not".
-// If they are used instead, we do not want to combine them with
-// the token to the right, unless that is a left paren.
 if (!Right.is(tok::l_paren)) {
+  // The alternative operators for ~ and ! are "compl" and "not".
+  // If they are used instead, we do not want to combine them with
+  // the token to the right, unless that is a left paren.
   if (Left.is(tok::exclaim) && Left.TokenText == "not")
 return true;
   if (Left.is(tok::tilde) && Left.TokenText == "compl")
 return true;
+  // Lambda captures allow for a lone &, so "&]" needs to be properly
+  // handled.
+  if (Left.is(tok::amp) && Right.is(tok::r_square))
+return Style.SpacesInSquareBrackets;
 }
 return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
Right.is(TT_BinaryOperator);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68578: [HIP] Fix device stub name

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1099-1108
+auto *FD = const_cast((ND));
+if (auto *TD = cast(FD)->getPrimaryTemplate())
+  FD = TD->getTemplatedDecl();
+auto OldDeclName = FD->getDeclName();
+auto NewNameStr = std::string("__device_stub__") + 
OldDeclName.getAsString();
+auto *NewId = (NewNameStr);
+auto NewDeclName = DeclarationName(NewId);

tra wrote:
> On one hand I like this patch variant much better than the one that changed 
> the mangling itself.
> On the other hand this code appears to reply on implementation details. I.e. 
> we're setting new name on `FD` which may or may not be the same as `ND`, but 
> we're always passing `ND` to `getMangledNameImpl()`. 
> 
> Perhaps we could implement name-tweaking as another `MultiVersionKind` which 
> we already plumb into getMangledNameImpl() and which allows changing the name 
> for target attributes & features.
> 
> 
The mangled name of an instantiated template function does not depends on its 
own name, but on the template. If we do not want to depend on this 
implementation detail, it seems I have to clone the template and instantiate 
from the clone.

MultiVersion does not help us here since it only appends .postfix to mangled 
name. The obstacle we are facing is how to change the unmangled name.


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

https://reviews.llvm.org/D68578



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