[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Ok, here's the crashing example with `ObjCForCollectionStmt`. It should be 
saved as an `.mm` file and it crashes under pure `--analyze`.

  @interface Item
  // ...
  @end
  
  @interface Collection
  // ...
  @end
  
  typedef void (^Blk)();
  
  struct RAII {
Blk blk;
  
  public:
RAII(Blk blk): blk(blk) {}
~RAII() { blk(); }
  };
  
  void foo(Collection *coll) {
RAII raii(^{});
for (Item *item in coll) {}
  }

The CFG ("allocate a variable, pick the item and put it into that variable, 
execute the body, repeat"):
F12397775: Screen Shot 2020-07-23 at 10.08.02 PM.png 


The interesting part of the ExplodedGraph:
F12397783: Screen Shot 2020-07-23 at 10.11.42 PM.png 


And here's the FIXME that you're looking for:

  ...
  44 /// Generate a node in \p Bldr for an iteration statement using ObjC
  45 /// for-loop iterator.
  46 static void populateObjCForDestinationSet(
  47 ExplodedNodeSet , SValBuilder ,
  48 const ObjCForCollectionStmt *S, const Stmt *elem, SVal elementV,
  49 SymbolManager , const NodeBuilderContext *currBldrCtx,
  50 StmtNodeBuilder , bool hasElements) {
  ...
  56 SVal hasElementsV = svalBuilder.makeTruthVal(hasElements);
  57
  58 // FIXME: S is not an expression. We should not be binding values to 
it.
  59 ProgramStateRef nextState = state->BindExpr(S, LCtx, hasElementsV);
  ...

So, like, the engine is conveniently assigning 0 or 1 to the 
collection-statement in the Environment when the collection is assumed to be 
empty or not.

It's obviously a hack. This shouldn't be in the Environment. This should have 
been a GDM trait attached to the collection. Ideally it should also be modeled, 
i.e. sometimes we do know whether the collection is empty, and it might even be 
modeled occasionally. But in any case this shouldn't be in the Environment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



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

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert updated this revision to Diff 280327.
jdoerfert added a comment.

minor fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83281

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/declare_variant_device_isa_codegen_1.c
  clang/test/OpenMP/declare_variant_messages.c
  llvm/include/llvm/Frontend/OpenMP/OMPContext.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPContext.cpp
  llvm/unittests/Frontend/OpenMPContextTest.cpp

Index: llvm/unittests/Frontend/OpenMPContextTest.cpp
===
--- llvm/unittests/Frontend/OpenMPContextTest.cpp
+++ llvm/unittests/Frontend/OpenMPContextTest.cpp
@@ -38,11 +38,13 @@
 #define OMP_TRAIT_PROPERTY(Enum, TraitSetEnum, TraitSelectorEnum, Str) \
   EXPECT_EQ(TraitProperty::Enum,   \
 getOpenMPContextTraitPropertyKind( \
-TraitSet::TraitSetEnum,\
-getOpenMPContextTraitPropertyName(TraitProperty::Enum)));  \
+TraitSet::TraitSetEnum, TraitSelector::TraitSelectorEnum,  \
+getOpenMPContextTraitPropertyName(TraitProperty::Enum, Str))); \
   EXPECT_EQ(Str, getOpenMPContextTraitPropertyName(\
- getOpenMPContextTraitPropertyKind(TraitSet::TraitSetEnum, \
-   Str))); \
+ getOpenMPContextTraitPropertyKind(\
+ TraitSet::TraitSetEnum,   \
+ TraitSelector::TraitSelectorEnum, Str),   \
+ Str));\
   EXPECT_EQ(TraitSet::TraitSetEnum,\
 getOpenMPContextTraitSetForProperty(TraitProperty::Enum)); \
   EXPECT_EQ(TraitSelector::TraitSelectorEnum,  \
@@ -77,31 +79,31 @@
   EXPECT_TRUE(isVariantApplicableInContext(Empty, DeviceNVPTX));
 
   VariantMatchInfo UserCondFalse;
-  UserCondFalse.addTrait(TraitProperty::user_condition_false);
+  UserCondFalse.addTrait(TraitProperty::user_condition_false, "");
   EXPECT_FALSE(isVariantApplicableInContext(UserCondFalse, HostLinux));
   EXPECT_FALSE(isVariantApplicableInContext(UserCondFalse, DeviceLinux));
   EXPECT_FALSE(isVariantApplicableInContext(UserCondFalse, HostNVPTX));
   EXPECT_FALSE(isVariantApplicableInContext(UserCondFalse, DeviceNVPTX));
 
   VariantMatchInfo DeviceArchArm;
-  DeviceArchArm.addTrait(TraitProperty::device_arch_arm);
+  DeviceArchArm.addTrait(TraitProperty::device_arch_arm, "");
   EXPECT_FALSE(isVariantApplicableInContext(DeviceArchArm, HostLinux));
   EXPECT_FALSE(isVariantApplicableInContext(DeviceArchArm, DeviceLinux));
   EXPECT_FALSE(isVariantApplicableInContext(DeviceArchArm, HostNVPTX));
   EXPECT_FALSE(isVariantApplicableInContext(DeviceArchArm, DeviceNVPTX));
 
   VariantMatchInfo LLVMHostUserCondTrue;
-  LLVMHostUserCondTrue.addTrait(TraitProperty::implementation_vendor_llvm);
-  LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_host);
-  LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_any);
-  LLVMHostUserCondTrue.addTrait(TraitProperty::user_condition_true);
+  LLVMHostUserCondTrue.addTrait(TraitProperty::implementation_vendor_llvm, "");
+  LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_host, "");
+  LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_any, "");
+  LLVMHostUserCondTrue.addTrait(TraitProperty::user_condition_true, "");
   EXPECT_TRUE(isVariantApplicableInContext(LLVMHostUserCondTrue, HostLinux));
   EXPECT_FALSE(isVariantApplicableInContext(LLVMHostUserCondTrue, DeviceLinux));
   EXPECT_TRUE(isVariantApplicableInContext(LLVMHostUserCondTrue, HostNVPTX));
   EXPECT_FALSE(isVariantApplicableInContext(LLVMHostUserCondTrue, DeviceNVPTX));
 
   VariantMatchInfo LLVMHostUserCondTrueCPU = LLVMHostUserCondTrue;
-  LLVMHostUserCondTrueCPU.addTrait(TraitProperty::device_kind_cpu);
+  LLVMHostUserCondTrueCPU.addTrait(TraitProperty::device_kind_cpu, "");
   EXPECT_TRUE(isVariantApplicableInContext(LLVMHostUserCondTrueCPU, HostLinux));
   EXPECT_FALSE(
   isVariantApplicableInContext(LLVMHostUserCondTrueCPU, DeviceLinux));
@@ -111,14 +113,14 @@
   isVariantApplicableInContext(LLVMHostUserCondTrueCPU, DeviceNVPTX));
 
   VariantMatchInfo GPU;
-  GPU.addTrait(TraitProperty::device_kind_gpu);
+  GPU.addTrait(TraitProperty::device_kind_gpu, "");
   

[PATCH] D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff.

[Alina Sbirlea via Phabricator via cfe-commits]

asbirlea updated this revision to Diff 280326.
asbirlea added a comment.

The increase in number of instructions and cycles was caused by reversing the 
order in which the updates are applied by GraphDiff.
I'll look into re-adding some of the original cleanups in a follow-up patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77341

Files:
  llvm/include/llvm/IR/Dominators.h
  llvm/include/llvm/Support/CFGDiff.h
  llvm/include/llvm/Support/GenericDomTree.h
  llvm/include/llvm/Support/GenericDomTreeConstruction.h
  llvm/lib/IR/Dominators.cpp

Index: llvm/lib/IR/Dominators.cpp
===
--- llvm/lib/IR/Dominators.cpp
+++ llvm/lib/IR/Dominators.cpp
@@ -90,9 +90,10 @@
 DomTreeBuilder::BBPostDomTree , BasicBlock *From, BasicBlock *To);
 
 template void llvm::DomTreeBuilder::ApplyUpdates(
-DomTreeBuilder::BBDomTree , DomTreeBuilder::BBUpdates);
+DomTreeBuilder::BBDomTree , DomTreeBuilder::BBDomTreeGraphDiff &);
 template void llvm::DomTreeBuilder::ApplyUpdates(
-DomTreeBuilder::BBPostDomTree , DomTreeBuilder::BBUpdates);
+DomTreeBuilder::BBPostDomTree ,
+DomTreeBuilder::BBPostDomTreeGraphDiff &);
 
 template bool llvm::DomTreeBuilder::Verify(
 const DomTreeBuilder::BBDomTree ,
Index: llvm/include/llvm/Support/GenericDomTreeConstruction.h
===
--- llvm/include/llvm/Support/GenericDomTreeConstruction.h
+++ llvm/include/llvm/Support/GenericDomTreeConstruction.h
@@ -58,6 +58,7 @@
   using TreeNodePtr = DomTreeNodeBase *;
   using RootsT = decltype(DomTreeT::Roots);
   static constexpr bool IsPostDom = DomTreeT::IsPostDominator;
+  using GraphDiffT = GraphDiff;
 
   // Information record used by Semi-NCA during tree construction.
   struct InfoRec {
@@ -77,28 +78,27 @@
   using UpdateT = typename DomTreeT::UpdateType;
   using UpdateKind = typename DomTreeT::UpdateKind;
   struct BatchUpdateInfo {
-SmallVector Updates;
-using NodePtrAndKind = PointerIntPair;
-
-// In order to be able to walk a CFG that is out of sync with the CFG
-// DominatorTree last knew about, use the list of updates to reconstruct
-// previous CFG versions of the current CFG. For each node, we store a set
-// of its virtually added/deleted future successors and predecessors.
-// Note that these children are from the future relative to what the
-// DominatorTree knows about -- using them to gets us some snapshot of the
-// CFG from the past (relative to the state of the CFG).
-DenseMap> FutureSuccessors;
-DenseMap> FuturePredecessors;
+// Note: Updates inside PreViewCFG are aleady legalized.
+BatchUpdateInfo(GraphDiffT )
+: PreViewCFG(PreViewCFG),
+  NumLegalized(PreViewCFG.getNumLegalizedUpdates()) {}
+
 // Remembers if the whole tree was recalculated at some point during the
 // current batch update.
 bool IsRecalculated = false;
+GraphDiffT 
+const size_t NumLegalized;
   };
 
   BatchUpdateInfo *BatchUpdates;
   using BatchUpdatePtr = BatchUpdateInfo *;
+  std::unique_ptr EmptyGD;
 
   // If BUI is a nullptr, then there's no batch update in progress.
-  SemiNCAInfo(BatchUpdatePtr BUI) : BatchUpdates(BUI) {}
+  SemiNCAInfo(BatchUpdatePtr BUI) : BatchUpdates(BUI) {
+if (!BatchUpdates)
+  EmptyGD = std::make_unique();
+  }
 
   void clear() {
 NumToNode = {nullptr}; // Restore to initial state with a dummy start node.
@@ -107,8 +107,7 @@
 // in progress, we need this information to continue it.
   }
 
-  template 
-  struct ChildrenGetter {
+  template  struct ChildrenGetter {
 using ResultTy = SmallVector;
 
 static ResultTy Get(NodePtr N, std::integral_constant) {
@@ -121,50 +120,16 @@
   return ResultTy(IChildren.begin(), IChildren.end());
 }
 
-using Tag = std::integral_constant;
+using Tag = std::integral_constant;
 
 // The function below is the core part of the batch updater. It allows the
 // Depth Based Search algorithm to perform incremental updates in lockstep
 // with updates to the CFG. We emulated lockstep CFG updates by getting its
 // next snapshots by reverse-applying future updates.
 static ResultTy Get(NodePtr N, BatchUpdatePtr BUI) {
-  ResultTy Res = Get(N, Tag());
-  // If there's no batch update in progress, simply return node's children.
-  if (!BUI) return Res;
-
-  // CFG children are actually its *most current* children, and we have to
-  // reverse-apply the future updates to get the node's children at the
-  // point in time the update was performed.
-  auto  = (Inverse != IsPostDom) ? BUI->FuturePredecessors
-: BUI->FutureSuccessors;
-  auto FCIt = FutureChildren.find(N);
-  if (FCIt == FutureChildren.end()) return Res;
-
-  for (auto ChildAndKind 

[clang] 70e7aa4 - [AST][FPEnv] Keep FP options in trailing storage of CallExpr

[Serge Pavlov via cfe-commits]

Author: Serge Pavlov
Date: 2020-07-24T12:04:19+07:00
New Revision: 70e7aa4a4ed36a034c43b249d0842f4f273b44e1

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

LOG: [AST][FPEnv] Keep FP options in trailing storage of CallExpr

This change allow a CallExpr to have optional FPOptionsOverride object,
stored in trailing storage. The implementaion is made similar to the way
used in BinaryOperator.

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

Added: 
clang/test/AST/ast-dump-fpfeatures.cpp

Modified: 
clang/include/clang/AST/Expr.h
clang/include/clang/AST/ExprCXX.h
clang/include/clang/AST/Stmt.h
clang/include/clang/AST/TextNodeDumper.h
clang/lib/AST/ASTImporter.cpp
clang/lib/AST/Expr.cpp
clang/lib/AST/ExprCXX.cpp
clang/lib/AST/TextNodeDumper.cpp
clang/lib/Analysis/BodyFarm.cpp
clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
clang/lib/Frontend/Rewrite/RewriteObjC.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterStmt.cpp

Removed: 




diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 24bface15d3e..96db7bc3be29 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -2272,12 +2272,12 @@ class UnaryOperator final
   /// Is FPFeatures in Trailing Storage?
   bool hasStoredFPFeatures() const { return UnaryOperatorBits.HasFPFeatures; }
 
-protected:
-  /// Get FPFeatures from trailing storage
+  /// Get FPFeatures from trailing storage.
   FPOptionsOverride getStoredFPFeatures() const {
 return getTrailingFPFeatures();
   }
 
+protected:
   /// Set FPFeatures in trailing storage, used only by Serialization
   void setStoredFPFeatures(FPOptionsOverride F) { getTrailingFPFeatures() = F; 
}
 
@@ -2787,6 +2787,8 @@ class CallExpr : public Expr {
   //
   // * An array of getNumArgs() "Stmt *" for the argument expressions.
   //
+  // * An optional of type FPOptionsOverride.
+  //
   // Note that we store the offset in bytes from the this pointer to the start
   // of the trailing objects. It would be perfectly possible to compute it
   // based on the dynamic kind of the CallExpr. However 1.) we have plenty of
@@ -2808,6 +2810,15 @@ class CallExpr : public Expr {
   /// this pointer to the trailing objects.
   static unsigned offsetToTrailingObjects(StmtClass SC);
 
+  unsigned getSizeOfTrailingStmts() const {
+return (1 + getNumPreArgs() + getNumArgs()) * sizeof(Stmt *);
+  }
+
+  size_t getOffsetOfTrailingFPFeatures() const {
+assert(hasStoredFPFeatures());
+return CallExprBits.OffsetToTrailingObjects + getSizeOfTrailingStmts();
+  }
+
 public:
   enum class ADLCallKind : bool { NotADL, UsesADL };
   static constexpr ADLCallKind NotADL = ADLCallKind::NotADL;
@@ -2818,16 +2829,19 @@ class CallExpr : public Expr {
   /// allocated for the trailing objects.
   CallExpr(StmtClass SC, Expr *Fn, ArrayRef PreArgs,
ArrayRef Args, QualType Ty, ExprValueKind VK,
-   SourceLocation RParenLoc, unsigned MinNumArgs, ADLCallKind UsesADL);
+   SourceLocation RParenLoc, FPOptionsOverride FPFeatures,
+   unsigned MinNumArgs, ADLCallKind UsesADL);
 
   /// Build an empty call expression, for deserialization.
   CallExpr(StmtClass SC, unsigned NumPreArgs, unsigned NumArgs,
-   EmptyShell Empty);
+   bool hasFPFeatures, EmptyShell Empty);
 
   /// Return the size in bytes needed for the trailing objects.
   /// Used by the derived classes to allocate the right amount of storage.
-  static unsigned sizeOfTrailingObjects(unsigned NumPreArgs, unsigned NumArgs) 
{
-return (1 + NumPreArgs + NumArgs) * sizeof(Stmt *);
+  static unsigned sizeOfTrailingObjects(unsigned NumPreArgs, unsigned NumArgs,
+bool HasFPFeatures) {
+return (1 + NumPreArgs + NumArgs) * sizeof(Stmt *) +
+   HasFPFeatures * sizeof(FPOptionsOverride);
   }
 
   Stmt *getPreArg(unsigned I) {
@@ -2845,22 +2859,43 @@ class CallExpr : public Expr {
 
   unsigned getNumPreArgs() const { return CallExprBits.NumPreArgs; }
 
+  /// Return a pointer to the trailing FPOptions
+  FPOptionsOverride *getTrailingFPFeatures() {
+assert(hasStoredFPFeatures());
+return reinterpret_cast(
+reinterpret_cast(this) + CallExprBits.OffsetToTrailingObjects +
+getSizeOfTrailingStmts());
+  }
+  const FPOptionsOverride *getTrailingFPFeatures() const {
+assert(hasStoredFPFeatures());
+return reinterpret_cast(
+reinterpret_cast(this) +
+CallExprBits.OffsetToTrailingObjects + 

[PATCH] D84343: [AST] Keep FP options in trailing storage of CallExpr

[Serge Pavlov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG70e7aa4a4ed3: [AST][FPEnv] Keep FP options in trailing 
storage of CallExpr (authored by sepavloff).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84343

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/BodyFarm.cpp
  clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  clang/lib/Frontend/Rewrite/RewriteObjC.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/AST/ast-dump-fpfeatures.cpp

Index: clang/test/AST/ast-dump-fpfeatures.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-fpfeatures.cpp
@@ -0,0 +1,37 @@
+// Test without serialization:
+// RUN: %clang_cc1 -fsyntax-only -triple x86_64-pc-linux -std=c++11 -ast-dump %s \
+// RUN: | FileCheck --strict-whitespace %s
+
+// Test with serialization:
+// RUN: %clang_cc1 -triple x86_64-pc-linux -emit-pch -o %t %s
+// RUN: %clang_cc1 -x c++ -triple x86_64-pc-linux -include-pch %t -ast-dump-all /dev/null \
+// RUN: | sed -e "s/ //" -e "s/ imported//" \
+// RUN: | FileCheck --strict-whitespace %s
+
+float func_01(float x);
+
+template 
+T func_02(T x) {
+#pragma STDC FP_CONTRACT ON
+  return func_01(x);
+}
+
+float func_03(float x) {
+#pragma STDC FP_CONTRACT OFF
+  return func_02(x);
+}
+
+// CHECK:  FunctionTemplateDecl {{.*}} func_02
+// CHECK:FunctionDecl {{.*}} func_02 'float (float)'
+// CHECK-NEXT: TemplateArgument type 'float'
+// CHECK-NEXT:   BuiltinType {{.*}} 'float'
+// CHECK-NEXT: ParmVarDecl {{.*}} x 'float'
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT:   ReturnStmt
+// CHECK-NEXT: CallExpr {{.*}} FPContractMode=1
+
+// CHECK:  FunctionDecl {{.*}} func_03 'float (float)'
+// CHECK-NEXT:   ParmVarDecl {{.*}} x 'float'
+// CHECK-NEXT: CompoundStmt
+// CHECK-NEXT:   ReturnStmt
+// CHECK-NEXT: CallExpr {{.*}} FPContractMode=0
\ No newline at end of file
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -848,12 +848,15 @@
 void ASTStmtWriter::VisitCallExpr(CallExpr *E) {
   VisitExpr(E);
   Record.push_back(E->getNumArgs());
+  Record.push_back(E->hasStoredFPFeatures());
   Record.AddSourceLocation(E->getRParenLoc());
   Record.AddStmt(E->getCallee());
   for (CallExpr::arg_iterator Arg = E->arg_begin(), ArgEnd = E->arg_end();
Arg != ArgEnd; ++Arg)
 Record.AddStmt(*Arg);
   Record.push_back(static_cast(E->getADLCallKind()));
+  if (E->hasStoredFPFeatures())
+Record.push_back(E->getFPFeatures().getAsOpaqueInt());
   Code = serialization::EXPR_CALL;
 }
 
@@ -1550,7 +1553,6 @@
   VisitCallExpr(E);
   Record.push_back(E->getOperator());
   Record.AddSourceRange(E->Range);
-  Record.push_back(E->getFPFeatures().getAsOpaqueInt());
   Code = serialization::EXPR_CXX_OPERATOR_CALL;
 }
 
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -991,12 +991,15 @@
 void ASTStmtReader::VisitCallExpr(CallExpr *E) {
   VisitExpr(E);
   unsigned NumArgs = Record.readInt();
+  bool HasFPFeatures = Record.readInt();
   assert((NumArgs == E->getNumArgs()) && "Wrong NumArgs!");
   E->setRParenLoc(readSourceLocation());
   E->setCallee(Record.readSubExpr());
   for (unsigned I = 0; I != NumArgs; ++I)
 E->setArg(I, Record.readSubExpr());
   E->setADLCallKind(static_cast(Record.readInt()));
+  if (HasFPFeatures)
+E->setStoredFPFeatures(FPOptionsOverride(Record.readInt()));
 }
 
 void ASTStmtReader::VisitCXXMemberCallExpr(CXXMemberCallExpr *E) {
@@ -1662,7 +1665,6 @@
   VisitCallExpr(E);
   E->CXXOperatorCallExprBits.OperatorKind = Record.readInt();
   E->Range = Record.readSourceRange();
-  E->setFPFeatures(FPOptionsOverride(Record.readInt()));
 }
 
 void ASTStmtReader::VisitCXXRewrittenBinaryOperator(
@@ -2977,7 +2979,8 @@
 
 case EXPR_CALL:
   S = CallExpr::CreateEmpty(
-  Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields], Empty);
+  Context, /*NumArgs=*/Record[ASTStmtReader::NumExprFields],
+  /*HasFPFeatures=*/Record[ASTStmtReader::NumExprFields + 1], Empty);
   break;
 
 case EXPR_RECOVERY:
@@ -3585,12 +3588,14 @@
 
 case 

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D84316#2168730 , @steakhal wrote:

> In D84316#2168462 , @NoQ wrote:
>
> > Such separation also looks amazing because ultimately the first part can be 
> > made checker-inspecific (i.e., a reusable half-baked trait that can be 
> > instantiated multiple times to track various things regardless of their 
> > meaning).
>
>
> Could you elaborate on the latter part? (instantiated multiple times...)


Imagine something like re-using the state trait implementation between 
`MallocChecker` and `StreamChecker` because they both model "resources that can 
be deallocated twice or leaked" - regardless of the specific nature of these 
resources. These checkers can implement their own API modeling maps, escape 
rules, warning messages, maybe model additional aspects of their problems, but 
fundamentally they're solving the same problem: finding leaks and overreleases 
of resources. This problem should ideally be solved once. This is why i 
advocate for abstract, generalized, "half-baked" state trait boilerplate 
implementations that can be re-used across checkers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84316



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

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D84316#2168730 , @steakhal wrote:

> I wanted to have a separate class for bookkeeping while minimalizing the 
> necessary changes.
>  What do you think would be the best way to organize this separation?
>
> Few notes:
>
> - Checkers are implemented in the anonymous namespace, so only the given file 
> has access to them.
> - I wanted to separate the bookkeeping logic from the reporting/function 
> modeling logic in different files.
> - I like the fact that after the change the CStringChecker implements only 
> the evalCall checker callback.
>
> Let me know if I misunderstood something.


Mmm, none of these benefits sound like they outweigh confusing the cost of 
users with a new checker flag that can't even be used in any sensible way.

If you want separate files, just put the checker into a header and include it 
from multiple cpp files. A few checkers already do that - RetainCountChecker, 
MPIChecker, UninitializedObjectChecker. There's nothing fundamental about 
keeping checkers in an anonymous namespace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84316



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

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

In D80858#2170821 , @tra wrote:

> In D80858#2170781 , @hliao wrote:
>
> > The problem is not whether we have solution to tell them but when we need 
> > to add that. Not all `static` device variables need to be visible to the 
> > host side. `Externalizing` them adds the overhead for the linker and may 
> > pose additional restrictions on aggressive optimizations. Do we have to 
> > support every ambiguous usage in the burden of the compiler change?
>
>
> It's a multi-part problem. 
>  The key request here is, IIUIC, to make TU-local variables visible within 
> the TU on both host and the device. 
>  If and how it should be implemented is up for the discusstion. 
>  For me the request is reasonable and it would be good to have it as it would 
> make the code behave according to general expectations of how TU-local 
> variables behave -- "if it's in the same source file, it should be accessible 
> from the functions in that file".


We may try to solve the issue without RDC firstly, where we don't need to 
change that static variable name (if the runtime maintains the device binaries 
correctly.) We only need to ensure the linker won't remove their symbols.

> I'm mostly discussing the 'how' part and we obviously don't have all the 
> answers yet.
> 
> I agree, that adding the 'handles' for all non-visible variables is 
> suboptimal and it may prove to be too big of a burden. That said, it does not 
> look like an outright showstopper either. Ideally we only need those handles 
> for the items that are being referred to by the host. Most likely most of 
> them will not be. The problem is that it's the host-side compilation which 
> knows which symbols will be needed, but it's the device-side compilation 
> where we would have to generate aliases and, generally speaking, device-side 
> compilation may not be able to ever tell which symbols will be needed by the 
> host. I don't have a good solution for that. Ideally we need to have exact 
> same AST on both sides and we can't guarantee that with the current 
> implementation of HD functions and the use of preprocessor macros.
> 
>>> 
>>> 
 Also, in the context of CUDA runtime/driver API, if a device variable is 
 addressable on the host side through the runtime API, it should be 
 addressable through the driver API as well. However, the naming will be a 
 big challenge.
>>> 
>>> I'm not convinced that I completely agree with this assertion. It's 
>>> reasonable for visible symbols (though even then there's a question of how 
>>> do you figure out a properly mangled name for that symbol), but expecting 
>>> it to work for non-visible symbols would push it too far, IMO.
>> 
>> Won't this patch just makes invisible variables "visible"?
> 
> Yes, but there will be no name conflicts, so for all practical purposes they 
> don't change the end result of linking.




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

https://reviews.llvm.org/D80858



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

[PATCH] D84371: [DFSan] Add efficient fast16labels instrumentation mode.

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added inline comments.



Comment at: compiler-rt/lib/dfsan/dfsan.cpp:180
 dfsan_label __dfsan_union(dfsan_label l1, dfsan_label l2) {
-  if (flags().fast16labels)
+  if (fast16labels)
 return l1 | l2;

isn't better just create new set of callbacks?
e.g __dfsan_fast16_union
and then we don't need any flags or preinit array initialization



Comment at: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:810
+
+  DFSanUseFast16LabelsFn = Mod->getOrInsertFunction("dfsan_use_fast16labels",
+DFSanUseFast16LabelsFnTy);

DFSanUseFast16LabelsFnTy and DFSanUseFast16LabelsFn is used one in one place
I'd make them local variables.  in if (ClFast16Labels) of runOnModule


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84371



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

[PATCH] D84482: [Fuchsia] Use -z dead-reloc-in-nonalloc=*=0 for Fuchsia targets

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr created this revision.
mcgrathr added reviewers: phosek, leonardchan.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84482

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp


Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -53,6 +53,13 @@
   CmdArgs.push_back("-z");
   CmdArgs.push_back("now");
 
+  // 0 is never a valid code address for Fuchsia (user or kernel), so using the
+  // traditional 0 value for references to discarded code is always fine on
+  // Fuchsia.  The new defaults of -1 and such in lld confuse some older tools
+  // for no benefit to Fuchsia.
+  CmdArgs.push_back("-z");
+  CmdArgs.push_back("dead-reloc-in-nonalloc=*=0");
+
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   if (llvm::sys::path::filename(Exec).equals_lower("ld.lld") ||
   llvm::sys::path::stem(Exec).equals_lower("ld.lld")) {


Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -53,6 +53,13 @@
   CmdArgs.push_back("-z");
   CmdArgs.push_back("now");
 
+  // 0 is never a valid code address for Fuchsia (user or kernel), so using the
+  // traditional 0 value for references to discarded code is always fine on
+  // Fuchsia.  The new defaults of -1 and such in lld confuse some older tools
+  // for no benefit to Fuchsia.
+  CmdArgs.push_back("-z");
+  CmdArgs.push_back("dead-reloc-in-nonalloc=*=0");
+
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());
   if (llvm::sys::path::filename(Exec).equals_lower("ld.lld") ||
   llvm::sys::path::stem(Exec).equals_lower("ld.lld")) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84371: [DFSan] Add efficient fast16labels instrumentation mode.

[Kostya Serebryany via Phabricator via cfe-commits]

kcc added a comment.

Yep, cool.
LGTM from me, but please get another pair if eyes (Vitaly?)




Comment at: clang/docs/DataFlowSanitizer.rst:182
+less CPU and code size overhead.  To use fast16labels instrumentation, you'll
+need to specify `-fsanitize=dataflow -mllvm -fast-16-labels` in your compile 
and
+link commands and use a modified API for creating and managing labels.

maybe -dataflow-fast-16-labels?



Comment at: clang/docs/DataFlowSanitizer.rst:201
+  int main(void) {
+int i = 1;
+int j = 2;

perhaps unconfuse the test a bit by using other constants 
  i = 100;
  j = 200;
  k = 300;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84371



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

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

[Petr Hosek via Phabricator via cfe-commits]

phosek updated this revision to Diff 280305.

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

https://reviews.llvm.org/D79219

Files:
  clang/CMakeLists.txt
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/CMakeLists.txt
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp

Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-  HAVE_LIBZ
   HAVE_OCAMLOPT
   HAVE_OCAML_OUNIT
   LLVM_ENABLE_DIA_SDK
   LLVM_ENABLE_FFI
   LLVM_ENABLE_THREADS
+  LLVM_ENABLE_ZLIB
   LLVM_INCLUDE_GO_TESTS
   LLVM_LIBXML2_ENABLED
   LLVM_LINK_LLVM_DYLIB
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -17,13 +17,13 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_ZLIB_H
+#if LLVM_ENABLE_ZLIB
 #include 
 #endif
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 static Error createError(StringRef Err) {
   return make_error(Err, inconvertibleErrorCode());
 }
Index: llvm/lib/Support/CRC.cpp
===
--- llvm/lib/Support/CRC.cpp
+++ llvm/lib/Support/CRC.cpp
@@ -25,7 +25,7 @@
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 0 || !HAVE_ZLIB_H
+#if !LLVM_ENABLE_ZLIB
 
 static const uint32_t CRCTable[256] = {
 0x, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f,
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -1,7 +1,7 @@
-set(system_libs)
-if ( LLVM_ENABLE_ZLIB AND HAVE_LIBZ )
-  set(system_libs ${system_libs} ${ZLIB_LIBRARIES})
+if(LLVM_ENABLE_ZLIB)
+  set(imported_libs ZLIB::ZLIB)
 endif()
+
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
@@ -194,10 +194,32 @@
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support
   ${Backtrace_INCLUDE_DIRS}
-  LINK_LIBS ${system_libs} ${delayload_flags} ${Z3_LINK_FILES}
+  LINK_LIBS ${system_libs} ${imported_libs} ${delayload_flags} ${Z3_LINK_FILES}
   )
 
-set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${system_libs}")
+set(llvm_system_libs ${system_libs})
+
+if(LLVM_ENABLE_ZLIB)
+  string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
+  get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION_${build_type})
+  if(NOT zlib_library)
+get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION)
+  endif()
+  get_filename_component(zlib_library ${zlib_library} NAME)
+  if(CMAKE_STATIC_LIBRARY_PREFIX AND CMAKE_STATIC_LIBRARY_SUFFIX AND
+  zlib_library MATCHES "^${CMAKE_STATIC_LIBRARY_PREFIX}.*${CMAKE_STATIC_LIBRARY_SUFFIX}$")
+STRING(REGEX REPLACE "^${CMAKE_STATIC_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+STRING(REGEX REPLACE "${CMAKE_STATIC_LIBRARY_SUFFIX}$" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_SHARED_LIBRARY_PREFIX AND CMAKE_SHARED_LIBRARY_SUFFIX AND
+  zlib_library MATCHES "^${CMAKE_SHARED_LIBRARY_PREFIX}.*${CMAKE_SHARED_LIBRARY_SUFFIX}$")
+STRING(REGEX REPLACE "^${CMAKE_SHARED_LIBRARY_PREFIX}" "" 

[PATCH] D84371: [DFSan] Add efficient fast16labels instrumentation mode.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 280304.
morehouse added a comment.

- Fix libfuzzer dataflow tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84371

Files:
  clang/docs/DataFlowSanitizer.rst
  compiler-rt/lib/dfsan/dfsan.cpp
  compiler-rt/lib/dfsan/dfsan_flags.inc
  compiler-rt/lib/dfsan/done_abilist.txt
  compiler-rt/lib/fuzzer/FuzzerDataFlowTrace.cpp
  compiler-rt/lib/fuzzer/dataflow/DataFlow.cpp
  compiler-rt/test/dfsan/fast16labels.c
  compiler-rt/test/fuzzer/dataflow.test
  compiler-rt/test/fuzzer/only-some-bytes-fork.test
  compiler-rt/test/fuzzer/only-some-bytes.test
  llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
  llvm/test/Instrumentation/DataFlowSanitizer/fast16labels.ll

Index: llvm/test/Instrumentation/DataFlowSanitizer/fast16labels.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/DataFlowSanitizer/fast16labels.ll
@@ -0,0 +1,100 @@
+; Test that fast-16-labels mode uses inline ORs rather than calling
+; __dfsan_union or __dfsan_union_load.
+; RUN: opt < %s -dfsan -fast-16-labels -S | FileCheck %s --implicit-check-not="call{{.*}}__dfsan_union"
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i8 @add(i8 %a, i8 %b) {
+  ; CHECK-LABEL: define i8 @"dfs$add"
+  ; CHECK-DAG: %[[ALABEL:.*]] = load{{.*}}__dfsan_arg_tls, i64 0, i64 0
+  ; CHECK-DAG: %[[BLABEL:.*]] = load{{.*}}__dfsan_arg_tls, i64 0, i64 1
+  ; CHECK: %[[ADDLABEL:.*]] = or i16 %[[ALABEL]], %[[BLABEL]]
+  ; CHECK: add i8
+  ; CHECK: store i16 %[[ADDLABEL]], i16* @__dfsan_retval_tls
+  ; CHECK: ret i8
+  %c = add i8 %a, %b
+  ret i8 %c
+}
+
+define i8 @load8(i8* %p) {
+  ; CHECK-LABEL: define i8 @"dfs$load8"
+  ; CHECK: load i16, i16*
+  ; CHECK: ptrtoint i8* {{.*}} to i64
+  ; CHECK: and i64
+  ; CHECK: mul i64
+  ; CHECK: inttoptr i64
+  ; CHECK: load i16, i16*
+  ; CHECK: or i16
+  ; CHECK: load i8, i8*
+  ; CHECK: store i16 {{.*}} @__dfsan_retval_tls
+  ; CHECK: ret i8
+
+  %a = load i8, i8* %p
+  ret i8 %a
+}
+
+define i16 @load16(i16* %p) {
+  ; CHECK-LABEL: define i16 @"dfs$load16"
+  ; CHECK: ptrtoint i16*
+  ; CHECK: and i64
+  ; CHECK: mul i64
+  ; CHECK: inttoptr i64 {{.*}} i16*
+  ; CHECK: getelementptr i16
+  ; CHECK: load i16, i16*
+  ; CHECK: load i16, i16*
+  ; CHECK: or i16
+  ; CHECK: or i16
+  ; CHECK: load i16, i16*
+  ; CHECK: store {{.*}} @__dfsan_retval_tls
+  ; CHECK: ret i16
+
+  %a = load i16, i16* %p
+  ret i16 %a
+}
+
+define i32 @load32(i32* %p) {
+  ; CHECK-LABEL: define i32 @"dfs$load32"
+  ; CHECK: ptrtoint i32*
+  ; CHECK: and i64
+  ; CHECK: mul i64
+  ; CHECK: inttoptr i64 {{.*}} i16*
+  ; CHECK: bitcast i16* {{.*}} i64*
+  ; CHECK: load i64, i64*
+  ; CHECK: lshr i64 {{.*}}, 32
+  ; CHECK: or i64
+  ; CHECK: lshr i64 {{.*}}, 16
+  ; CHECK: or i64
+  ; CHECK: trunc i64 {{.*}} i16
+  ; CHECK: or i16
+  ; CHECK: load i32, i32*
+  ; CHECK: store i16 {{.*}} @__dfsan_retval_tls
+  ; CHECK: ret i32
+
+  %a = load i32, i32* %p
+  ret i32 %a
+}
+
+define i64 @load64(i64* %p) {
+  ; CHECK-LABEL: define i64 @"dfs$load64"
+  ; CHECK: ptrtoint i64*
+  ; CHECK: and i64
+  ; CHECK: mul i64
+  ; CHECK: inttoptr i64 {{.*}} i16*
+  ; CHECK: bitcast i16* {{.*}} i64*
+  ; CHECK: load i64, i64*
+  ; CHECK: getelementptr i64, i64* {{.*}}, i64 1
+  ; CHECK: load i64, i64*
+  ; CHECK: or i64
+  ; CHECK: lshr i64 {{.*}}, 32
+  ; CHECK: or i64
+  ; CHECK: lshr i64 {{.*}}, 16
+  ; CHECK: or i64
+  ; CHECK: trunc i64 {{.*}} i16
+  ; CHECK: or i16
+  ; CHECK: load i64, i64*
+  ; CHECK: store i16 {{.*}} @__dfsan_retval_tls
+  ; CHECK: ret i64
+
+  %a = load i64, i64* %p
+  ret i64 %a
+}
Index: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
@@ -91,6 +91,7 @@
 #include "llvm/Transforms/Instrumentation.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/Local.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
 #include 
 #include 
 #include 
@@ -176,6 +177,14 @@
 cl::desc("Insert calls to __dfsan_*_callback functions on data events."),
 cl::Hidden, cl::init(false));
 
+// Use a distinct bit for each base label, enabling faster unions with less
+// instrumentation.  Limits the max number of base labels to 16.
+static cl::opt ClFast16Labels(
+"fast-16-labels",
+cl::desc("Use more efficient instrumentation, limiting the number of "
+ "labels to 16."),
+cl::Hidden, cl::init(false));
+
 static StringRef GetGlobalTypeString(const GlobalValue ) {
   // Types of GlobalVariables are always pointer types.
   Type *GType = 

[PATCH] D83972: Modify ImportDefiniton for ObjCInterfaceDecl so that we always the ImportDeclContext one we start the definition

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik updated this revision to Diff 280291.
shafik added a comment.

Updating diff since the parent landed.


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

https://reviews.llvm.org/D83972

Files:
  clang/lib/AST/ASTImporter.cpp
  lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py


Index: lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py
===
--- lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py
+++ lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py
@@ -14,9 +14,8 @@
 lldbutil.run_to_source_breakpoint(self, "// break here", 
lldb.SBFileSpec("main.m"))
 
 self.expect_expr("chb->hb->field1", result_type="unsigned int", 
result_value="0")
-
-## FIXME field2 should have a value of 1
-self.expect("expr chb->hb->field2", matching=False, substrs = ["= 1"]) 
# this must happen second
+## This should happen second
+self.expect_expr("chb->hb->field2", result_type="unsigned int", 
result_value="1")
 
 self.expect_expr("hb2->field1", result_type="unsigned int", 
result_value="10")
 self.expect_expr("hb2->field2", result_type="unsigned int", 
result_value="3")
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -4758,11 +4758,10 @@
   return ToImplOrErr.takeError();
   }
 
-  if (shouldForceImportDeclContext(Kind)) {
-// Import all of the members of this class.
-if (Error Err = ImportDeclContext(From, /*ForceImport=*/true))
-  return Err;
-  }
+  // Import all of the members of this class.
+  if (Error Err = ImportDeclContext(From, /*ForceImport=*/true))
+return Err;
+
   return Error::success();
 }
 


Index: lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py
===
--- lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py
+++ lldb/test/API/lang/objc/bitfield_ivars/TestBitfieldIvars.py
@@ -14,9 +14,8 @@
 lldbutil.run_to_source_breakpoint(self, "// break here", lldb.SBFileSpec("main.m"))
 
 self.expect_expr("chb->hb->field1", result_type="unsigned int", result_value="0")
-
-## FIXME field2 should have a value of 1
-self.expect("expr chb->hb->field2", matching=False, substrs = ["= 1"]) # this must happen second
+## This should happen second
+self.expect_expr("chb->hb->field2", result_type="unsigned int", result_value="1")
 
 self.expect_expr("hb2->field1", result_type="unsigned int", result_value="10")
 self.expect_expr("hb2->field2", result_type="unsigned int", result_value="3")
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -4758,11 +4758,10 @@
   return ToImplOrErr.takeError();
   }
 
-  if (shouldForceImportDeclContext(Kind)) {
-// Import all of the members of this class.
-if (Error Err = ImportDeclContext(From, /*ForceImport=*/true))
-  return Err;
-  }
+  // Import all of the members of this class.
+  if (Error Err = ImportDeclContext(From, /*ForceImport=*/true))
+return Err;
+
   return Error::success();
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84476: Make hip math headers easier to use from C

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield created this revision.
JonChesterfield added a reviewer: yaxunl.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Make hip math headers easier to use from C

Motivation is a step towards using the hip math headers to implement math.h
for openmp, which needs to work with C as well as C++. NFC for C++ code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84476

Files:
  clang/lib/Headers/__clang_hip_libdevice_declares.h
  clang/lib/Headers/__clang_hip_math.h


Index: clang/lib/Headers/__clang_hip_math.h
===
--- clang/lib/Headers/__clang_hip_math.h
+++ clang/lib/Headers/__clang_hip_math.h
@@ -95,8 +95,10 @@
 }
 
 // BEGIN FLOAT
+#ifdef _cplusplus
 __DEVICE__
 inline float abs(float __x) { return __ocml_fabs_f32(__x); }
+#endif
 __DEVICE__
 inline float acosf(float __x) { return __ocml_acos_f32(__x); }
 __DEVICE__
@@ -251,7 +253,7 @@
   uint32_t sign : 1;
 } bits;
 
-static_assert(sizeof(float) == sizeof(ieee_float), "");
+static_assert(sizeof(float) == sizeof(struct ieee_float), "");
   } __tmp;
 
   __tmp.bits.sign = 0u;
@@ -553,8 +555,10 @@
 // END FLOAT
 
 // BEGIN DOUBLE
+#ifdef _cplusplus
 __DEVICE__
 inline double abs(double __x) { return __ocml_fabs_f64(__x); }
+#endif
 __DEVICE__
 inline double acos(double __x) { return __ocml_acos_f64(__x); }
 __DEVICE__
@@ -712,7 +716,7 @@
   uint32_t exponent : 11;
   uint32_t sign : 1;
 } bits;
-static_assert(sizeof(double) == sizeof(ieee_double), "");
+static_assert(sizeof(double) == sizeof(struct ieee_double), "");
   } __tmp;
 
   __tmp.bits.sign = 0u;
@@ -1178,6 +1182,7 @@
   return std::max(__arg1, __arg2);
 }
 
+#ifdef _cplusplus
 __DEVICE__
 inline float pow(float __base, int __iexp) { return powif(__base, __iexp); }
 
@@ -1188,6 +1193,7 @@
 inline _Float16 pow(_Float16 __base, int __iexp) {
   return __ocml_pown_f16(__base, __iexp);
 }
+#endif
 
 #pragma pop_macro("__DEF_FUN1")
 #pragma pop_macro("__DEF_FUN2")
Index: clang/lib/Headers/__clang_hip_libdevice_declares.h
===
--- clang/lib/Headers/__clang_hip_libdevice_declares.h
+++ clang/lib/Headers/__clang_hip_libdevice_declares.h
@@ -10,7 +10,9 @@
 #ifndef __CLANG_HIP_LIBDEVICE_DECLARES_H__
 #define __CLANG_HIP_LIBDEVICE_DECLARES_H__
 
+#ifdef __cplusplus
 extern "C" {
+#endif
 
 // BEGIN FLOAT
 __device__ __attribute__((const)) float __ocml_acos_f32(float);
@@ -316,7 +318,7 @@
 __device__ inline __2f16
 __llvm_amdgcn_rcp_2f16(__2f16 __x) // Not currently exposed by ROCDL.
 {
-  return __2f16{__llvm_amdgcn_rcp_f16(__x.x), __llvm_amdgcn_rcp_f16(__x.y)};
+  return (__2f16){__llvm_amdgcn_rcp_f16(__x.x), __llvm_amdgcn_rcp_f16(__x.y)};
 }
 __device__ __attribute__((const)) __2f16 __ocml_rint_2f16(__2f16);
 __device__ __attribute__((const)) __2f16 __ocml_rsqrt_2f16(__2f16);
@@ -325,6 +327,8 @@
 __device__ __attribute__((const)) __2f16 __ocml_trunc_2f16(__2f16);
 __device__ __attribute__((const)) __2f16 __ocml_pown_2f16(__2f16, __2i16);
 
+#ifdef __cplusplus
 } // extern "C"
+#endif
 
 #endif // __CLANG_HIP_LIBDEVICE_DECLARES_H__


Index: clang/lib/Headers/__clang_hip_math.h
===
--- clang/lib/Headers/__clang_hip_math.h
+++ clang/lib/Headers/__clang_hip_math.h
@@ -95,8 +95,10 @@
 }
 
 // BEGIN FLOAT
+#ifdef _cplusplus
 __DEVICE__
 inline float abs(float __x) { return __ocml_fabs_f32(__x); }
+#endif
 __DEVICE__
 inline float acosf(float __x) { return __ocml_acos_f32(__x); }
 __DEVICE__
@@ -251,7 +253,7 @@
   uint32_t sign : 1;
 } bits;
 
-static_assert(sizeof(float) == sizeof(ieee_float), "");
+static_assert(sizeof(float) == sizeof(struct ieee_float), "");
   } __tmp;
 
   __tmp.bits.sign = 0u;
@@ -553,8 +555,10 @@
 // END FLOAT
 
 // BEGIN DOUBLE
+#ifdef _cplusplus
 __DEVICE__
 inline double abs(double __x) { return __ocml_fabs_f64(__x); }
+#endif
 __DEVICE__
 inline double acos(double __x) { return __ocml_acos_f64(__x); }
 __DEVICE__
@@ -712,7 +716,7 @@
   uint32_t exponent : 11;
   uint32_t sign : 1;
 } bits;
-static_assert(sizeof(double) == sizeof(ieee_double), "");
+static_assert(sizeof(double) == sizeof(struct ieee_double), "");
   } __tmp;
 
   __tmp.bits.sign = 0u;
@@ -1178,6 +1182,7 @@
   return std::max(__arg1, __arg2);
 }
 
+#ifdef _cplusplus
 __DEVICE__
 inline float pow(float __base, int __iexp) { return powif(__base, __iexp); }
 
@@ -1188,6 +1193,7 @@
 inline _Float16 pow(_Float16 __base, int __iexp) {
   return __ocml_pown_f16(__base, __iexp);
 }
+#endif
 
 #pragma pop_macro("__DEF_FUN1")
 #pragma pop_macro("__DEF_FUN2")
Index: clang/lib/Headers/__clang_hip_libdevice_declares.h
===
--- clang/lib/Headers/__clang_hip_libdevice_declares.h
+++ 

[PATCH] D84473: Dump Accumulator

[Mircea Trofin via Phabricator via cfe-commits]

mtrofin added inline comments.



Comment at: llvm/include/llvm/Analysis/DumpAccumulator.h:43
+//
+// Note that ThinLTO is not supported yet.
+//

I'd clarify that transferring the data from pre-link thinlto to post-link isn't 
supported, but one could use this in post-link, correct?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84473



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

[PATCH] D84371: [DFSan] Add efficient fast16labels instrumentation mode.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added a subscriber: Dor1s.
morehouse added a comment.

In D84371#2168367 , @kcc wrote:

> In what cases do we still call __dfsan_union?


We still call `__dfsan_union_load` when we load sizes greater than 2 bytes but 
not divisible by 4.  I actually am not sure what instruction sequence would hit 
this case, but it was already in DFSan so I left it there.

`__dfsan_union_load` then calls `__dfsan_union`.




Comment at: compiler-rt/lib/dfsan/dfsan.cpp:173
+// Configures the DFSan runtime to use fast16labels mode.
+extern "C" SANITIZER_INTERFACE_ATTRIBUTE void dfsan_use_fast16labels() {
+  use_fast16labels = true;

kcc wrote:
> I wonder if we can get away w/o these flags.
> Just assume that fast16mode works only when the fast16mode instrumentation is 
> enabled. 
> (remember, fast16mode is currently experimental, we are free to change its 
> behavior) 
SGTM.  I only know of @Dor1s using it at the moment.



Comment at: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:1079
+   cast(DFSanUseFast16LabelsFn.getCallee()),
+   "__dfsan_fast16labels_preinit");
+UseFast16LabelsInit->setSection(".preinit_array");

vitalybuka wrote:
> what we are going to do in __dfsan_fast16labels_preinit?
This global is simply a pointer to `dfsan_use_fast16labels()` so that it runs 
during preinit.  `dfsan_use_fast16labels` just enables fast16labels mode in the 
runtime.



Comment at: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:1339
+IRB.CreateAlignedLoad(IRB.getInt64Ty(), WideAddr, ShadowAlign);
+for (uint64_t Ofs = 64 / DFS.ShadowWidthBits; Ofs != Size;
+ Ofs += 64 / DFS.ShadowWidthBits) {

vitalybuka wrote:
> have you considered a loop?
> 
I think we should avoid loop overhead at least for the common case here  
(loading 8-16 bytes of shadow).

I think it's also possible to hit this case with vector instructions, but 
vanilla DFSan currently doesn't use a loop in that case either.  If we want to 
use a loop for vector loads, I think we should do it in a different patch.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84371



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

[PATCH] D84371: [DFSan] Add efficient fast16labels instrumentation mode.

[Matt Morehouse via Phabricator via cfe-commits]

morehouse updated this revision to Diff 280286.
morehouse marked 6 inline comments as done.
morehouse added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- Add documentation.
- Remove fast16labels runtime flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84371

Files:
  clang/docs/DataFlowSanitizer.rst
  compiler-rt/lib/dfsan/dfsan.cpp
  compiler-rt/lib/dfsan/dfsan_flags.inc
  compiler-rt/lib/dfsan/done_abilist.txt
  compiler-rt/test/dfsan/fast16labels.c
  llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
  llvm/test/Instrumentation/DataFlowSanitizer/fast16labels.ll

Index: llvm/test/Instrumentation/DataFlowSanitizer/fast16labels.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/DataFlowSanitizer/fast16labels.ll
@@ -0,0 +1,100 @@
+; Test that fast-16-labels mode uses inline ORs rather than calling
+; __dfsan_union or __dfsan_union_load.
+; RUN: opt < %s -dfsan -fast-16-labels -S | FileCheck %s --implicit-check-not="call{{.*}}__dfsan_union"
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i8 @add(i8 %a, i8 %b) {
+  ; CHECK-LABEL: define i8 @"dfs$add"
+  ; CHECK-DAG: %[[ALABEL:.*]] = load{{.*}}__dfsan_arg_tls, i64 0, i64 0
+  ; CHECK-DAG: %[[BLABEL:.*]] = load{{.*}}__dfsan_arg_tls, i64 0, i64 1
+  ; CHECK: %[[ADDLABEL:.*]] = or i16 %[[ALABEL]], %[[BLABEL]]
+  ; CHECK: add i8
+  ; CHECK: store i16 %[[ADDLABEL]], i16* @__dfsan_retval_tls
+  ; CHECK: ret i8
+  %c = add i8 %a, %b
+  ret i8 %c
+}
+
+define i8 @load8(i8* %p) {
+  ; CHECK-LABEL: define i8 @"dfs$load8"
+  ; CHECK: load i16, i16*
+  ; CHECK: ptrtoint i8* {{.*}} to i64
+  ; CHECK: and i64
+  ; CHECK: mul i64
+  ; CHECK: inttoptr i64
+  ; CHECK: load i16, i16*
+  ; CHECK: or i16
+  ; CHECK: load i8, i8*
+  ; CHECK: store i16 {{.*}} @__dfsan_retval_tls
+  ; CHECK: ret i8
+
+  %a = load i8, i8* %p
+  ret i8 %a
+}
+
+define i16 @load16(i16* %p) {
+  ; CHECK-LABEL: define i16 @"dfs$load16"
+  ; CHECK: ptrtoint i16*
+  ; CHECK: and i64
+  ; CHECK: mul i64
+  ; CHECK: inttoptr i64 {{.*}} i16*
+  ; CHECK: getelementptr i16
+  ; CHECK: load i16, i16*
+  ; CHECK: load i16, i16*
+  ; CHECK: or i16
+  ; CHECK: or i16
+  ; CHECK: load i16, i16*
+  ; CHECK: store {{.*}} @__dfsan_retval_tls
+  ; CHECK: ret i16
+
+  %a = load i16, i16* %p
+  ret i16 %a
+}
+
+define i32 @load32(i32* %p) {
+  ; CHECK-LABEL: define i32 @"dfs$load32"
+  ; CHECK: ptrtoint i32*
+  ; CHECK: and i64
+  ; CHECK: mul i64
+  ; CHECK: inttoptr i64 {{.*}} i16*
+  ; CHECK: bitcast i16* {{.*}} i64*
+  ; CHECK: load i64, i64*
+  ; CHECK: lshr i64 {{.*}}, 32
+  ; CHECK: or i64
+  ; CHECK: lshr i64 {{.*}}, 16
+  ; CHECK: or i64
+  ; CHECK: trunc i64 {{.*}} i16
+  ; CHECK: or i16
+  ; CHECK: load i32, i32*
+  ; CHECK: store i16 {{.*}} @__dfsan_retval_tls
+  ; CHECK: ret i32
+
+  %a = load i32, i32* %p
+  ret i32 %a
+}
+
+define i64 @load64(i64* %p) {
+  ; CHECK-LABEL: define i64 @"dfs$load64"
+  ; CHECK: ptrtoint i64*
+  ; CHECK: and i64
+  ; CHECK: mul i64
+  ; CHECK: inttoptr i64 {{.*}} i16*
+  ; CHECK: bitcast i16* {{.*}} i64*
+  ; CHECK: load i64, i64*
+  ; CHECK: getelementptr i64, i64* {{.*}}, i64 1
+  ; CHECK: load i64, i64*
+  ; CHECK: or i64
+  ; CHECK: lshr i64 {{.*}}, 32
+  ; CHECK: or i64
+  ; CHECK: lshr i64 {{.*}}, 16
+  ; CHECK: or i64
+  ; CHECK: trunc i64 {{.*}} i16
+  ; CHECK: or i16
+  ; CHECK: load i64, i64*
+  ; CHECK: store i16 {{.*}} @__dfsan_retval_tls
+  ; CHECK: ret i64
+
+  %a = load i64, i64* %p
+  ret i64 %a
+}
Index: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
@@ -91,6 +91,7 @@
 #include "llvm/Transforms/Instrumentation.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
 #include "llvm/Transforms/Utils/Local.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
 #include 
 #include 
 #include 
@@ -176,6 +177,14 @@
 cl::desc("Insert calls to __dfsan_*_callback functions on data events."),
 cl::Hidden, cl::init(false));
 
+// Use a distinct bit for each base label, enabling faster unions with less
+// instrumentation.  Limits the max number of base labels to 16.
+static cl::opt ClFast16Labels(
+"fast-16-labels",
+cl::desc("Use more efficient instrumentation, limiting the number of "
+ "labels to 16."),
+cl::Hidden, cl::init(false));
+
 static StringRef GetGlobalTypeString(const GlobalValue ) {
   // Types of GlobalVariables are always pointer types.
   Type *GType = G.getValueType();
@@ -359,6 +368,7 @@
   FunctionType *DFSanVarargWrapperFnTy;
   FunctionType 

[PATCH] D84473: Dump Accumulator

[Kazu Hirata via Phabricator via cfe-commits]

kazu created this revision.
kazu added reviewers: davidxl, mtrofin, wmi.
kazu added a project: LLVM.
Herald added subscribers: cfe-commits, rupprecht, MaskRay, hiraditya, mgorny.
Herald added a reviewer: jhenderson.
Herald added a project: clang.

This patch adds a utility called DumpAccumulator.

DumpAccumulator allows you to dump arbitrary text messages into a special
section called .llvm_dump.  The linker then concatenates these messages into
the identically named section in the final executable.

This utility makes it easy to collect information from optimization
passes of interest in a build environment that caches compilation.

See llvm/include/llvm/Analysis/DumpAccumulator.h for usage.

The original implementation is from Mircea Trofin, and I generalized it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84473

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Analysis/DumpAccumulator.h
  llvm/include/llvm/InitializePasses.h
  llvm/lib/Analysis/CMakeLists.txt
  llvm/lib/Analysis/DumpAccumulator.cpp
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/PassRegistry.def
  llvm/test/Other/new-pm-defaults.ll
  llvm/tools/llvm-readobj/ObjDumper.cpp
  llvm/tools/llvm-readobj/ObjDumper.h
  llvm/tools/llvm-readobj/llvm-readobj.cpp

Index: llvm/tools/llvm-readobj/llvm-readobj.cpp
===
--- llvm/tools/llvm-readobj/llvm-readobj.cpp
+++ llvm/tools/llvm-readobj/llvm-readobj.cpp
@@ -362,6 +362,10 @@
 clEnumVal(GNU, "GNU readelf style")),
  cl::init(LLVM));
 
+  // --llvm-dump
+  cl::opt LLVMDumpSection("llvm-dump", cl::desc("Print .llvm-dump"),
+cl::init(false));
+
   cl::extrahelp
   HelpResponse("\nPass @FILE as argument to read options from FILE.\n");
 } // namespace opts
@@ -487,6 +491,8 @@
 Dumper->printSymbols(opts::Symbols, opts::DynamicSymbols);
   if (!opts::StringDump.empty())
 Dumper->printSectionsAsString(Obj, opts::StringDump);
+  if (opts::LLVMDumpSection)
+Dumper->printLLVMDumpSection(Obj);
   if (!opts::HexDump.empty())
 Dumper->printSectionsAsHex(Obj, opts::HexDump);
   if (opts::HashTable)
Index: llvm/tools/llvm-readobj/ObjDumper.h
===
--- llvm/tools/llvm-readobj/ObjDumper.h
+++ llvm/tools/llvm-readobj/ObjDumper.h
@@ -100,6 +100,7 @@
 
   void printSectionsAsString(const object::ObjectFile *Obj,
  ArrayRef Sections);
+  void printLLVMDumpSection(const object::ObjectFile *Obj);
   void printSectionsAsHex(const object::ObjectFile *Obj,
   ArrayRef Sections);
 
Index: llvm/tools/llvm-readobj/ObjDumper.cpp
===
--- llvm/tools/llvm-readobj/ObjDumper.cpp
+++ llvm/tools/llvm-readobj/ObjDumper.cpp
@@ -15,8 +15,10 @@
 #include "Error.h"
 #include "llvm-readobj.h"
 #include "llvm/Object/ObjectFile.h"
+#include "llvm/Support/Compression.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/LEB128.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -80,6 +82,45 @@
   return Ret;
 }
 
+void ObjDumper::printLLVMDumpSection(const object::ObjectFile *Obj) {
+  auto Sections = getSectionRefsByNameOrIndex(Obj, {".llvm_dump"});
+  if (Sections.size() != 1) {
+reportWarning(createError(".llvm_dump section not found"),
+  Obj->getFileName());
+return;
+  }
+  auto Section = Sections[0];
+  StringRef SectionContent =
+  unwrapOrError(Obj->getFileName(), Section.getContents());
+
+  StringRef Current = SectionContent;
+  unsigned Read = 0;
+  auto ReadNumber = [&]() {
+uint64_t V =
+decodeULEB128(reinterpret_cast(Current.data()), );
+Current = Current.substr(Read);
+return V;
+  };
+  while (Current.size() > 0) {
+uint64_t UncompressedSize = ReadNumber();
+std::unique_ptr Uncompressed(new char[UncompressedSize]);
+uint64_t CompressedSize = ReadNumber();
+if (CompressedSize > Current.size()) {
+  reportWarning(
+  createError(
+  "Expecting a larger compressed buffer than available data"),
+  Obj->getFileName());
+}
+if (auto E =
+zlib::uncompress(Current, Uncompressed.get(), UncompressedSize)) {
+  reportWarning(createError("Error decompressing"), Obj->getFileName());
+}
+Current = Current.substr(CompressedSize);
+StringRef Message(Uncompressed.get(), UncompressedSize);
+W.startLine() << Message << "\n";
+  }
+}
+
 void ObjDumper::printSectionsAsString(const object::ObjectFile *Obj,
   ArrayRef Sections) {
   bool First = true;
Index: llvm/test/Other/new-pm-defaults.ll
===
--- 

[PATCH] D83592: [Coverage] Add comment to skipped regions

[Zequan Wu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Lex/Preprocessor.cpp:973
+  if ((LexLevel == 0 || PreprocessToken) &&
+  !Result.getFlag(Token::IsReinjected)) {
+if (LexLevel == 0)

@hans , can you take a look on this part? I saw `TokenCount` was introduced for 
a warning `-Wmax-tokens`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83592



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

[PATCH] D83592: [Coverage] Add comment to skipped regions

[Zequan Wu via Phabricator via cfe-commits]

zequanwu updated this revision to Diff 280283.
zequanwu edited the summary of this revision.
zequanwu added a comment.

In `Preprocessor.cpp`, don't increment `TokenCount` if `LexLevel == 0`.

I tests this with chromium crypto_unittests. It doesn't track comments regions.

I don't why when using the cmake option `-DLLVM_BUILD_INSTRUMENTED_COVERAGE=On` 
to test coverage for clang itself, it does tracking comments.
Also, with that option on, `llvm-cov` crashes at 
assertion:`llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp:578: virtual 
Expected (anonymous 
namespace)::VersionedCovMapFuncRecordReader::readCoverageHeader(const char *, const char *, 
BinaryCoverageReader::DecompressedData &) [Version = llvm::coverage::Version4, 
IntPtrT = unsigned long, Endian = llvm::support::little]: Assertion 
`(CovMapVersion)CovHeader->getVersion() == Version' failed.`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83592

Files:
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/lib/CodeGen/CoverageMappingGen.h
  clang/lib/Lex/Preprocessor.cpp
  clang/test/CoverageMapping/break.c
  clang/test/CoverageMapping/builtinmacro.c
  clang/test/CoverageMapping/classtemplate.cpp
  clang/test/CoverageMapping/comment-in-macro.c
  clang/test/CoverageMapping/continue.c
  clang/test/CoverageMapping/coroutine.cpp
  clang/test/CoverageMapping/deferred-region.cpp
  clang/test/CoverageMapping/if.cpp
  clang/test/CoverageMapping/includehell.cpp
  clang/test/CoverageMapping/label.cpp
  clang/test/CoverageMapping/logical.cpp
  clang/test/CoverageMapping/loops.cpp
  clang/test/CoverageMapping/macro-expressions.cpp
  clang/test/CoverageMapping/macroparams2.c
  clang/test/CoverageMapping/macros.c
  clang/test/CoverageMapping/macroscopes.cpp
  clang/test/CoverageMapping/moremacros.c
  clang/test/CoverageMapping/objc.m
  clang/test/CoverageMapping/pr32679.cpp
  clang/test/CoverageMapping/preprocessor.c
  clang/test/CoverageMapping/return.c
  clang/test/CoverageMapping/switch.cpp
  clang/test/CoverageMapping/switchmacro.c
  clang/test/CoverageMapping/test.c
  clang/test/CoverageMapping/trycatch.cpp
  clang/test/CoverageMapping/unreachable-macro.c
  clang/test/CoverageMapping/while.c
  clang/test/lit.cfg.py
  compiler-rt/test/profile/Inputs/instrprof-comdat.h
  compiler-rt/test/profile/coverage_comments.cpp
  compiler-rt/test/profile/instrprof-set-file-object-merging.c

Index: compiler-rt/test/profile/instrprof-set-file-object-merging.c
===
--- compiler-rt/test/profile/instrprof-set-file-object-merging.c
+++ compiler-rt/test/profile/instrprof-set-file-object-merging.c
@@ -34,7 +34,7 @@
 // CHECK:   17|  2|
 // CHECK:   18|  2|  FILE *F = fopen(argv[1], "r+b");
 // CHECK:   19|  2|  if (!F) {
-// CHECK:   20|  1|// File might not exist, try opening with truncation
+// CHECK:   20|   |// File might not exist, try opening with truncation
 // CHECK:   21|  1|F = fopen(argv[1], "w+b");
 // CHECK:   22|  1|  }
 // CHECK:   23|  2|  __llvm_profile_set_file_object(F, 1);
Index: compiler-rt/test/profile/coverage_comments.cpp
===
--- /dev/null
+++ compiler-rt/test/profile/coverage_comments.cpp
@@ -0,0 +1,71 @@
+// RUN: %clangxx_profgen -fcoverage-mapping -Wno-comment -o %t %s
+// RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t
+// RUN: llvm-profdata merge -o %t.profdata %t.profraw
+// RUN: llvm-cov show %t -instr-profile %t.profdata -path-equivalence=/tmp,%S 2>&1 | FileCheck %s
+
+int main() {   // CHECK:   [[# @LINE]]| 1|int main() {
+/* comment */ int x = 0;   // CHECK-NEXT:  [[# @LINE]]| 1|
+int y = 0; /* comment */   // CHECK-NEXT:  [[# @LINE]]| 1|
+int z = 0; // comment  // CHECK-NEXT:  [[# @LINE]]| 1|
+// comment // CHECK-NEXT:  [[# @LINE]]|  |
+   // CHECK-NEXT:  [[# @LINE]]|  |
+x = 0; /*  // CHECK-NEXT:  [[# @LINE]]| 1|
+comment// CHECK-NEXT:  [[# @LINE]]|  |
+*/ // CHECK-NEXT:  [[# @LINE]]|  |
+   // CHECK-NEXT:  [[# @LINE]]|  |
+/* // CHECK-NEXT:  [[# @LINE]]|  |
+comment// CHECK-NEXT:  [[# @LINE]]|  |
+*/ x = 0;  // CHECK-NEXT:  [[# @LINE]]| 1|
+   // CHECK-NEXT:  [[# @LINE]]|  |
+/* comment */  // CHECK-NEXT:  [[# @LINE]]|  |
+// comment // CHECK-NEXT:  [[# @LINE]]|  |
+/* comment */  // CHECK-NEXT:  [[# @LINE]]|  |
+z =  

[PATCH] D83494: [libFuzzer] Link libFuzzer's own interceptors when other compiler runtimes are not linked.

[Dokyung Song via Phabricator via cfe-commits]

dokyungs added a comment.

In D83494#2170838 , @dmajor wrote:

> After this commit, several of our builds are failing with 
> `FuzzerInterceptors.cpp:30:10: fatal error: 
> 'sanitizer/common_interface_defs.h' file not found`. This is odd because the 
> file certainly seems like it exists. Is there a missing include path 
> somewhere, perhaps?


There seems to be a missing `include_directories` in CMakeLists.txt. I will 
send out a patch for review shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83494



___
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

[Lingda Li via Phabricator via cfe-commits]

lildmh added a comment.

Can you check if https://reviews.llvm.org/D84470 fixes the problem as this test 
doesn't work on my machine? If yes, please accept and upstream it since I don't 
have the permission.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67833



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

[PATCH] D83553: [PATCH 3/4][Sema][AArch64] Add codegen for arm_sve_vector_bits attribute

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:152
+  Align, Name,
+  /*ArraySize=*/nullptr, Alloca);
 

c-rhodes wrote:
> efriedma wrote:
> > Do we need to bitcast the result of CreateTempAlloca to a pointer to the 
> > array type?  I'm concerned that we might miss a bitcast if the source code 
> > uses the address of the variable.
> > Do we need to bitcast the result of CreateTempAlloca to a pointer to the 
> > array type? I'm concerned that we might miss a bitcast if the source code 
> > uses the address of the variable.
> 
> You were right, I've spent some time investigating this. The current 
> implementation crashes on:
> ```fixed_int32_t global;
> fixed_int32_t address_of_global() {
>   fixed_int32_t *global_ptr;
>   global_ptr = 
>   return *global_ptr;
> }```
> 
> the reason being `global` is represented as an `ArrayType` whereas the 
> pointer `global_ptr` is scalable:
> 
> ```@global = global [4 x i32] zeroinitializer, align 16
> %global_ptr = alloca *, align 8```
> 
> so when storing the address of `global` to `global_ptr` the store it tries to 
> create causes a crash:
> 
> `store [4 x i32]* @global, ** %global_ptr, align 8`
> 
> I tried your suggestion to bitcast to alloca to the array type in 
> `CreateMemTemp` but found for that example it isn't called, it's created by a 
> call to `CreateTempAlloca` in CGDecl.cpp (`EmitAutoVarAlloca`). 
> `CreateTempAlloca` takes an `llvm::Type *Ty` so it's not as straightforward 
> as doing a bitcast there, although I found it could be done in 
> `EmitAutoVarAlloca` but it means having to handle this is two places I'm 
> aware of and potentially others I haven't hit. In this case as well it also 
> required looking through the pointer to see if the pointee was a VLST then 
> doing a bitcast.
> 
> I've also experimented with representing allocas as fixed-length arrays to 
> see if that will make it any easier and it does simplify the patch a little. 
> It does require handling `PointerType` in `ConvertTypeForMem` however as we 
> do for `ConstantArray`, same issue I mentioned in response to your other 
> comment about removing that.
> 
> I planning to update the patch with that implementation but I've just found 
> another issue:
> 
> ```fixed_int32_t arr[3];
> fixed_int32_t *z() {
>   fixed_int32_t *array_ptr;
>   array_ptr = [0];
>   return array_ptr;
> }```
> 
> trying to create a store:
> `store [4 x i32]* %0, ** %retval, align 8`
> 
> although this is done in CGStmt.cpp as it's for a retval so it looks like a 
> bitcast could also be required there.
I think a `fixed_int32_t *` needs to be converted to `[4 x i32]*`, for the sake 
of consistency... but see also my other comment.



Comment at: clang/lib/CodeGen/CodeGenTypes.cpp:151
+  return llvm::ArrayType::get(*MemTy, A->getSize().getZExtValue());
+  }
+

c-rhodes wrote:
> efriedma wrote:
> > I think the default handling for constant arrays should do the right thing, 
> > now that we've changed the default behavior of ConvertTypeForMem.
> > I think the default handling for constant arrays should do the right thing, 
> > now that we've changed the default behavior of ConvertTypeForMem.
> 
> `ConvertType` looks at the canonical type so the type attribute is lost.
That sounds like a bug in the AST: since isVLST() affects the semantics of the 
type, it needs to be part of the canonical type. Otherwise you're going to be 
finding bugs all over in both Sema and CodeGen.


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

https://reviews.llvm.org/D83553



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

[PATCH] D83494: [libFuzzer] Link libFuzzer's own interceptors when other compiler runtimes are not linked.

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

After this commit, several of our builds are failing with 
`FuzzerInterceptors.cpp:30:10: fatal error: 'sanitizer/common_interface_defs.h' 
file not found`. This is odd because the file certainly seems like it exists. 
Is there a missing include path somewhere, perhaps?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83494



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

[clang] 38c71b7 - Revert "[CMake] Simplify CMake handling for zlib"

[Petr Hosek via cfe-commits]

Author: Petr Hosek
Date: 2020-07-23T15:12:42-07:00
New Revision: 38c71b7c85cac4c8de39329ff3ed682e63f61525

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

LOG: Revert "[CMake] Simplify CMake handling for zlib"

This reverts commit 1d09ecf36175f7910ffedd6d497c07b5c74c22fb since
it breaks sanitizer bots.

Added: 


Modified: 
clang/CMakeLists.txt
clang/test/CMakeLists.txt
clang/test/lit.site.cfg.py.in
compiler-rt/test/lit.common.configured.in
lld/CMakeLists.txt
lld/test/CMakeLists.txt
lld/test/lit.site.cfg.py.in
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
llvm/cmake/config-ix.cmake
llvm/include/llvm/Config/config.h.cmake
llvm/lib/Support/CMakeLists.txt
llvm/lib/Support/CRC.cpp
llvm/lib/Support/Compression.cpp
llvm/test/CMakeLists.txt
llvm/test/lit.site.cfg.py.in
llvm/unittests/Support/CompressionTest.cpp

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 1c4c22b1aaad..1a6a20a271f3 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -114,10 +114,6 @@ if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR )
   option(CLANG_ENABLE_BOOTSTRAP "Generate the clang bootstrap target" OFF)
   option(LLVM_ENABLE_LIBXML2 "Use libxml2 if available." ON)
 
-  if(LLVM_ENABLE_ZLIB)
-find_package(ZLIB)
-  endif()
-
   include(AddLLVM)
   include(TableGen)
   include(HandleLLVMOptions)

diff  --git a/clang/test/CMakeLists.txt b/clang/test/CMakeLists.txt
index 334a90498d0d..38bbc5be90d5 100644
--- a/clang/test/CMakeLists.txt
+++ b/clang/test/CMakeLists.txt
@@ -9,6 +9,15 @@ endif ()
 
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} CLANG_TOOLS_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
 
+if(CLANG_BUILT_STANDALONE)
+  # Set HAVE_LIBZ according to recorded LLVM_ENABLE_ZLIB value. This
+  # value is forced to 0 if zlib was not found, so it is fine to use it
+  # instead of HAVE_LIBZ (not recorded).
+  if(LLVM_ENABLE_ZLIB)
+set(HAVE_LIBZ 1)
+  endif()
+endif()
+
 llvm_canonicalize_cmake_booleans(
   CLANG_BUILD_EXAMPLES
   CLANG_ENABLE_ARCMT
@@ -16,7 +25,7 @@ llvm_canonicalize_cmake_booleans(
   CLANG_SPAWN_CC1
   ENABLE_BACKTRACES
   ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER
-  LLVM_ENABLE_ZLIB
+  HAVE_LIBZ
   LLVM_ENABLE_PER_TARGET_RUNTIME_DIR
   LLVM_ENABLE_PLUGINS
   LLVM_ENABLE_THREADS)

diff  --git a/clang/test/lit.site.cfg.py.in b/clang/test/lit.site.cfg.py.in
index 286ea06d798c..d9b5b2f2592e 100644
--- a/clang/test/lit.site.cfg.py.in
+++ b/clang/test/lit.site.cfg.py.in
@@ -16,7 +16,7 @@ config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.host_cxx = "@CMAKE_CXX_COMPILER@"
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @LLVM_ENABLE_ZLIB@
+config.have_zlib = @HAVE_LIBZ@
 config.clang_arcmt = @CLANG_ENABLE_ARCMT@
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@

diff  --git a/compiler-rt/test/lit.common.configured.in 
b/compiler-rt/test/lit.common.configured.in
index 000bf9b98470..1f746c067b84 100644
--- a/compiler-rt/test/lit.common.configured.in
+++ b/compiler-rt/test/lit.common.configured.in
@@ -57,7 +57,7 @@ elif config.android:
 else:
   set_default("target_suffix", "-%s" % config.target_arch)
 
-set_default("have_zlib", "@LLVM_ENABLE_ZLIB@")
+set_default("have_zlib", "@HAVE_LIBZ@")
 set_default("libcxx_used", "@LLVM_LIBCXX_USED@")
 
 # LLVM tools dir can be passed in lit parameters, so try to

diff  --git a/lld/CMakeLists.txt b/lld/CMakeLists.txt
index bcfc2c6270b3..e9bd1bd29c5c 100644
--- a/lld/CMakeLists.txt
+++ b/lld/CMakeLists.txt
@@ -51,10 +51,6 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)
   find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR} 
NO_DEFAULT_PATH)
 
-  if(LLVM_ENABLE_ZLIB)
-find_package(ZLIB)
-  endif()
-
   include(AddLLVM)
   include(TableGen)
   include(HandleLLVMOptions)

diff  --git a/lld/test/CMakeLists.txt b/lld/test/CMakeLists.txt
index e7d113330739..4fbd2534b5a9 100644
--- a/lld/test/CMakeLists.txt
+++ b/lld/test/CMakeLists.txt
@@ -4,8 +4,17 @@ set(LLVM_BUILD_MODE "%(build_mode)s")
 set(LLVM_TOOLS_DIR "${LLVM_TOOLS_BINARY_DIR}/%(build_config)s")
 set(LLVM_LIBS_DIR 
"${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/%(build_config)s")
 
+if(LLD_BUILT_STANDALONE)
+  # Set HAVE_LIBZ according to recorded LLVM_ENABLE_ZLIB value. This
+  # value is forced to 0 if zlib was not found, so it is fine to use it
+  # instead of HAVE_LIBZ (not recorded).
+  if(LLVM_ENABLE_ZLIB)
+set(HAVE_LIBZ 1)
+  endif()
+endif()
+
 

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D80858#2170781 , @hliao wrote:

> The problem is not whether we have solution to tell them but when we need to 
> add that. Not all `static` device variables need to be visible to the host 
> side. `Externalizing` them adds the overhead for the linker and may pose 
> additional restrictions on aggressive optimizations. Do we have to support 
> every ambiguous usage in the burden of the compiler change?


It's a multi-part problem. 
The key request here is, IIUIC, to make TU-local variables visible within the 
TU on both host and the device. 
If and how it should be implemented is up for the discusstion. 
For me the request is reasonable and it would be good to have it as it would 
make the code behave according to general expectations of how TU-local 
variables behave -- "if it's in the same source file, it should be accessible 
from the functions in that file".

I'm mostly discussing the 'how' part and we obviously don't have all the 
answers yet.

I agree, that adding the 'handles' for all non-visible variables is suboptimal 
and it may prove to be too big of a burden. That said, it does not look like an 
outright showstopper either. Ideally we only need those handles for the items 
that are being referred to by the host. Most likely most of them will not be. 
The problem is that it's the host-side compilation which knows which symbols 
will be needed, but it's the device-side compilation where we would have to 
generate aliases and, generally speaking, device-side compilation may not be 
able to ever tell which symbols will be needed by the host. I don't have a good 
solution for that. Ideally we need to have exact same AST on both sides and we 
can't guarantee that with the current implementation of HD functions and the 
use of preprocessor macros.

>> 
>> 
>>> Also, in the context of CUDA runtime/driver API, if a device variable is 
>>> addressable on the host side through the runtime API, it should be 
>>> addressable through the driver API as well. However, the naming will be a 
>>> big challenge.
>> 
>> I'm not convinced that I completely agree with this assertion. It's 
>> reasonable for visible symbols (though even then there's a question of how 
>> do you figure out a properly mangled name for that symbol), but expecting it 
>> to work for non-visible symbols would push it too far, IMO.
> 
> Won't this patch just makes invisible variables "visible"?

Yes, but there will be no name conflicts, so for all practical purposes they 
don't change the end result of linking.


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

https://reviews.llvm.org/D80858



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

[PATCH] D82999: [CodeGen] Check the cleanup flag before destructing lifetime-extended temporaries created in conditional expressions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I agree, that can be done separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82999



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

[PATCH] D83325: [Sema] Be more thorough when unpacking the AS-qualified pointee for a pointer conversion.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

`removeAddrSpaceQualType` should guarantee that it removes the address space 
qualifier; you shouldn't need to do something special here.  That means it 
needs to iteratively desugar and collect qualifiers as long as the type is 
still address-space-qualified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83325



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

[PATCH] D79219: [CMake] Simplify CMake handling for zlib

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1d09ecf36175: [CMake] Simplify CMake handling for zlib 
(authored by phosek).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79219

Files:
  clang/CMakeLists.txt
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  compiler-rt/test/lit.common.configured.in
  lld/CMakeLists.txt
  lld/test/CMakeLists.txt
  lld/test/lit.site.cfg.py.in
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/cmake/config-ix.cmake
  llvm/include/llvm/Config/config.h.cmake
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/CRC.cpp
  llvm/lib/Support/Compression.cpp
  llvm/test/CMakeLists.txt
  llvm/test/lit.site.cfg.py.in
  llvm/unittests/Support/CompressionTest.cpp

Index: llvm/unittests/Support/CompressionTest.cpp
===
--- llvm/unittests/Support/CompressionTest.cpp
+++ llvm/unittests/Support/CompressionTest.cpp
@@ -21,7 +21,7 @@
 
 namespace {
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 
 void TestZlibCompression(StringRef Input, int Level) {
   SmallString<32> Compressed;
Index: llvm/test/lit.site.cfg.py.in
===
--- llvm/test/lit.site.cfg.py.in
+++ llvm/test/lit.site.cfg.py.in
@@ -33,7 +33,7 @@
 config.host_ldflags = '@HOST_LDFLAGS@'
 config.llvm_use_intel_jitevents = @LLVM_USE_INTEL_JITEVENTS@
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.have_libxar = @HAVE_LIBXAR@
 config.have_dia_sdk = @LLVM_ENABLE_DIA_SDK@
 config.enable_ffi = @LLVM_ENABLE_FFI@
Index: llvm/test/CMakeLists.txt
===
--- llvm/test/CMakeLists.txt
+++ llvm/test/CMakeLists.txt
@@ -1,12 +1,12 @@
 llvm_canonicalize_cmake_booleans(
   BUILD_SHARED_LIBS
   HAVE_LIBXAR
-  HAVE_LIBZ
   HAVE_OCAMLOPT
   HAVE_OCAML_OUNIT
   LLVM_ENABLE_DIA_SDK
   LLVM_ENABLE_FFI
   LLVM_ENABLE_THREADS
+  LLVM_ENABLE_ZLIB
   LLVM_INCLUDE_GO_TESTS
   LLVM_LIBXML2_ENABLED
   LLVM_LINK_LLVM_DYLIB
Index: llvm/lib/Support/Compression.cpp
===
--- llvm/lib/Support/Compression.cpp
+++ llvm/lib/Support/Compression.cpp
@@ -17,13 +17,13 @@
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_ZLIB_H
+#if LLVM_ENABLE_ZLIB
 #include 
 #endif
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 1 && HAVE_LIBZ
+#if LLVM_ENABLE_ZLIB
 static Error createError(StringRef Err) {
   return make_error(Err, inconvertibleErrorCode());
 }
Index: llvm/lib/Support/CRC.cpp
===
--- llvm/lib/Support/CRC.cpp
+++ llvm/lib/Support/CRC.cpp
@@ -25,7 +25,7 @@
 
 using namespace llvm;
 
-#if LLVM_ENABLE_ZLIB == 0 || !HAVE_ZLIB_H
+#if !LLVM_ENABLE_ZLIB
 
 static const uint32_t CRCTable[256] = {
 0x, 0x77073096, 0xee0e612c, 0x990951ba, 0x076dc419, 0x706af48f,
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -1,7 +1,7 @@
-set(system_libs)
-if ( LLVM_ENABLE_ZLIB AND HAVE_LIBZ )
-  set(system_libs ${system_libs} ${ZLIB_LIBRARIES})
+if(LLVM_ENABLE_ZLIB)
+  set(imported_libs ZLIB::ZLIB)
 endif()
+
 if( MSVC OR MINGW )
   # libuuid required for FOLDERID_Profile usage in lib/Support/Windows/Path.inc.
   # advapi32 required for CryptAcquireContextW in lib/Support/Windows/Path.inc.
@@ -194,10 +194,34 @@
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support
   ${Backtrace_INCLUDE_DIRS}
-  LINK_LIBS ${system_libs} ${delayload_flags} ${Z3_LINK_FILES}
+  LINK_LIBS ${system_libs} ${imported_libs} ${delayload_flags} ${Z3_LINK_FILES}
   )
 
-set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${system_libs}")
+set(llvm_system_libs ${system_libs})
+
+if(LLVM_ENABLE_ZLIB)
+  string(TOUPPER ${CMAKE_BUILD_TYPE} build_type)
+  get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION_${build_type})
+  if(NOT zlib_library)
+get_property(zlib_library TARGET ZLIB::ZLIB PROPERTY LOCATION)
+  endif()
+  get_filename_component(zlib_library ${zlib_library} NAME)
+  if(CMAKE_STATIC_LIBRARY_PREFIX AND zlib_library MATCHES "^${CMAKE_STATIC_LIBRARY_PREFIX}.*")
+STRING(REGEX REPLACE "^${CMAKE_STATIC_LIBRARY_PREFIX}" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_STATIC_LIBRARY_SUFFIX AND zlib_library MATCHES ".*${CMAKE_STATIC_LIBRARY_SUFFIX}$")
+STRING(REGEX REPLACE "${CMAKE_STATIC_LIBRARY_SUFFIX}$" "" zlib_library ${zlib_library})
+  endif()
+  if(CMAKE_SHARED_LIBRARY_PREFIX AND 

[clang] 1d09ecf - [CMake] Simplify CMake handling for zlib

[Petr Hosek via cfe-commits]

Author: Petr Hosek
Date: 2020-07-23T14:47:25-07:00
New Revision: 1d09ecf36175f7910ffedd6d497c07b5c74c22fb

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

LOG: [CMake] Simplify CMake handling for zlib

Rather than handling zlib handling manually, use find_package from CMake
to find zlib properly. Use this to normalize the LLVM_ENABLE_ZLIB,
HAVE_ZLIB, HAVE_ZLIB_H. Furthermore, require zlib if LLVM_ENABLE_ZLIB is
set to YES, which requires the distributor to explicitly select whether
zlib is enabled or not. This simplifies the CMake handling and usage in
the rest of the tooling.

This is a reland of abb0075 with all followup changes and fixes that
should address issues that were reported in PR44780.

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

Added: 


Modified: 
clang/CMakeLists.txt
clang/test/CMakeLists.txt
clang/test/lit.site.cfg.py.in
compiler-rt/test/lit.common.configured.in
lld/CMakeLists.txt
lld/test/CMakeLists.txt
lld/test/lit.site.cfg.py.in
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
llvm/cmake/config-ix.cmake
llvm/include/llvm/Config/config.h.cmake
llvm/lib/Support/CMakeLists.txt
llvm/lib/Support/CRC.cpp
llvm/lib/Support/Compression.cpp
llvm/test/CMakeLists.txt
llvm/test/lit.site.cfg.py.in
llvm/unittests/Support/CompressionTest.cpp

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 1a6a20a271f3..1c4c22b1aaad 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -114,6 +114,10 @@ if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR )
   option(CLANG_ENABLE_BOOTSTRAP "Generate the clang bootstrap target" OFF)
   option(LLVM_ENABLE_LIBXML2 "Use libxml2 if available." ON)
 
+  if(LLVM_ENABLE_ZLIB)
+find_package(ZLIB)
+  endif()
+
   include(AddLLVM)
   include(TableGen)
   include(HandleLLVMOptions)

diff  --git a/clang/test/CMakeLists.txt b/clang/test/CMakeLists.txt
index 38bbc5be90d5..334a90498d0d 100644
--- a/clang/test/CMakeLists.txt
+++ b/clang/test/CMakeLists.txt
@@ -9,15 +9,6 @@ endif ()
 
 string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} CLANG_TOOLS_DIR 
${LLVM_RUNTIME_OUTPUT_INTDIR})
 
-if(CLANG_BUILT_STANDALONE)
-  # Set HAVE_LIBZ according to recorded LLVM_ENABLE_ZLIB value. This
-  # value is forced to 0 if zlib was not found, so it is fine to use it
-  # instead of HAVE_LIBZ (not recorded).
-  if(LLVM_ENABLE_ZLIB)
-set(HAVE_LIBZ 1)
-  endif()
-endif()
-
 llvm_canonicalize_cmake_booleans(
   CLANG_BUILD_EXAMPLES
   CLANG_ENABLE_ARCMT
@@ -25,7 +16,7 @@ llvm_canonicalize_cmake_booleans(
   CLANG_SPAWN_CC1
   ENABLE_BACKTRACES
   ENABLE_EXPERIMENTAL_NEW_PASS_MANAGER
-  HAVE_LIBZ
+  LLVM_ENABLE_ZLIB
   LLVM_ENABLE_PER_TARGET_RUNTIME_DIR
   LLVM_ENABLE_PLUGINS
   LLVM_ENABLE_THREADS)

diff  --git a/clang/test/lit.site.cfg.py.in b/clang/test/lit.site.cfg.py.in
index d9b5b2f2592e..286ea06d798c 100644
--- a/clang/test/lit.site.cfg.py.in
+++ b/clang/test/lit.site.cfg.py.in
@@ -16,7 +16,7 @@ config.host_triple = "@LLVM_HOST_TRIPLE@"
 config.target_triple = "@TARGET_TRIPLE@"
 config.host_cxx = "@CMAKE_CXX_COMPILER@"
 config.llvm_use_sanitizer = "@LLVM_USE_SANITIZER@"
-config.have_zlib = @HAVE_LIBZ@
+config.have_zlib = @LLVM_ENABLE_ZLIB@
 config.clang_arcmt = @CLANG_ENABLE_ARCMT@
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@

diff  --git a/compiler-rt/test/lit.common.configured.in 
b/compiler-rt/test/lit.common.configured.in
index 1f746c067b84..000bf9b98470 100644
--- a/compiler-rt/test/lit.common.configured.in
+++ b/compiler-rt/test/lit.common.configured.in
@@ -57,7 +57,7 @@ elif config.android:
 else:
   set_default("target_suffix", "-%s" % config.target_arch)
 
-set_default("have_zlib", "@HAVE_LIBZ@")
+set_default("have_zlib", "@LLVM_ENABLE_ZLIB@")
 set_default("libcxx_used", "@LLVM_LIBCXX_USED@")
 
 # LLVM tools dir can be passed in lit parameters, so try to

diff  --git a/lld/CMakeLists.txt b/lld/CMakeLists.txt
index e9bd1bd29c5c..bcfc2c6270b3 100644
--- a/lld/CMakeLists.txt
+++ b/lld/CMakeLists.txt
@@ -51,6 +51,10 @@ if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)
   find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR} 
NO_DEFAULT_PATH)
 
+  if(LLVM_ENABLE_ZLIB)
+find_package(ZLIB)
+  endif()
+
   include(AddLLVM)
   include(TableGen)
   include(HandleLLVMOptions)

diff  --git a/lld/test/CMakeLists.txt b/lld/test/CMakeLists.txt
index 4fbd2534b5a9..e7d113330739 100644
--- a/lld/test/CMakeLists.txt
+++ b/lld/test/CMakeLists.txt
@@ -4,17 +4,8 @@ set(LLVM_BUILD_MODE 

[PATCH] D84343: [AST] Keep FP options in trailing storage of CallExpr

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall 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/D84343/new/

https://reviews.llvm.org/D84343



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

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

In D80858#2170668 , @tra wrote:

> In D80858#2170604 , @hliao wrote:
>
> > In D80858#2170547 , @tra wrote:
> >
> > > Would it work if we generate a globally unique visible aliases for the 
> > > static vars and use the alias' name to register device-side entities 
> > > without changing their visibility?
> >
> >
> > We still need to define how a `static` device variable should be visible on 
> > the host side.
>
>
> As far as the host is concerned, if a 'static' variable is within the same TU 
> it (or, rather, its shadow) should be accessible.
>
> > How that behaves in the context of relocatable code generation as well as 
> > anonymous namespaces.
>
> AFAICT it would just work -- each TU only sees static things within that TU 
> on both host and device sides. Visible module-unique aliases would allow 
> host-side binary to get address of  an otherwise non-visible variables. I 
> expect that will work with RDC -- because we didn't change the visibility of 
> the symbols themselves, they behave according to the regular linking rules. 
> The aliases are also globally unique, so they should present no issues either.


The problem is not whether we have solution to tell them but when we need to 
add that. Not all `static` device variables need to be visible to the host 
side. `Externalizing` them adds the overhead for the linker and may pose 
additional restrictions on aggressive optimizations. Do we have to support 
every ambiguous usage in the burden of the compiler change?

> 
> 
>> Also, in the context of CUDA runtime/driver API, if a device variable is 
>> addressable on the host side through the runtime API, it should be 
>> addressable through the driver API as well. However, the naming will be a 
>> big challenge.
> 
> I'm not convinced that I completely agree with this assertion. It's 
> reasonable for visible symbols (though even then there's a question of how do 
> you figure out a properly mangled name for that symbol), but expecting it to 
> work for non-visible symbols would push it too far, IMO.

Won't this patch just makes invisible variables "visible"?

> It will also have issues with RDC as the name resulution will become 
> ambiguous. Compiler does have necessary info to resolve that ambiguity, 
> runtime support library, too, but the driver does not. We'd have to provide 
> the driver with the additional info to let it map host-side symbol info to 
> its device counterpart. It's doable, but I don't think it's particularly 
> useful in practice. If something needs to be accessed via the driver API, it 
> would be reasonable to expect it to be a public symbol.




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

https://reviews.llvm.org/D80858



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

[PATCH] D83731: Add Debug Info Size to Symbol Status

[Yifan Shen via Phabricator via cfe-commits]

aelitashen updated this revision to Diff 280261.
aelitashen added a comment.

Fix run_test to cover every system case and attributes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83731

Files:
  clang/tools/clang-format/git-clang-format
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
  lldb/test/API/tools/lldb-vscode/module/Makefile
  lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
  lldb/tools/lldb-vscode/JSONUtils.cpp

Index: lldb/tools/lldb-vscode/JSONUtils.cpp
===
--- lldb/tools/lldb-vscode/JSONUtils.cpp
+++ lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -7,6 +7,8 @@
 //===--===//
 
 #include 
+#include 
+#include 
 
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/FormatAdapters.h"
@@ -327,6 +329,50 @@
   return llvm::json::Value(std::move(object));
 }
 
+static uint64_t GetDebugInfoSizeInSection(lldb::SBSection section) {
+  uint64_t debug_info_size = 0;
+  llvm::StringRef section_name(section.GetName());
+  if (section_name.startswith(".debug") || section_name.startswith("__debug") ||
+  section_name.startswith(".apple") || section_name.startswith("__apple"))
+debug_info_size += section.GetFileByteSize();
+  size_t num_sub_sections = section.GetNumSubSections();
+  for (size_t i = 0; i < num_sub_sections; i++) {
+debug_info_size +=
+GetDebugInfoSizeInSection(section.GetSubSectionAtIndex(i));
+  }
+  return debug_info_size;
+}
+
+static uint64_t GetDebugInfoSize(lldb::SBModule module) {
+  uint64_t debug_info_size = 0;
+  size_t num_sections = module.GetNumSections();
+  for (size_t i = 0; i < num_sections; i++) {
+debug_info_size += GetDebugInfoSizeInSection(module.GetSectionAtIndex(i));
+  }
+  return debug_info_size;
+}
+
+static std::string ConvertDebugInfoSizeToString(uint64_t debug_info) {
+  std::ostringstream oss;
+  oss << " (";
+  oss << std::fixed << std::setprecision(1);
+
+  if (debug_info < 1024) {
+oss << debug_info << "B";
+  } else if (debug_info < 1024 * 1024) {
+double kb = double(debug_info) / 1024.0;
+oss << kb << "KB";
+  } else if (debug_info < 1024 * 1024 * 1024) {
+double mb = double(debug_info) / (1024.0 * 1024.0);
+oss << mb << "MB";
+  } else {
+double gb = double(debug_info) / (1024.0 * 1024.0 * 1024.0);
+oss << gb << "GB";
+;
+  }
+  oss << ")";
+  return oss.str();
+}
 llvm::json::Value CreateModule(lldb::SBModule ) {
   llvm::json::Object object;
   if (!module.IsValid())
@@ -339,9 +385,15 @@
   std::string module_path(module_path_arr);
   object.try_emplace("path", module_path);
   if (module.GetNumCompileUnits() > 0) {
-object.try_emplace("symbolStatus", "Symbols loaded.");
+std::string symbol_str = "Symbols loaded.";
+uint64_t debug_info = GetDebugInfoSize(module);
+if (debug_info > 0) {
+  symbol_str += ConvertDebugInfoSizeToString(debug_info);
+}
+object.try_emplace("symbolStatus", symbol_str);
 char symbol_path_arr[PATH_MAX];
-module.GetSymbolFileSpec().GetPath(symbol_path_arr, sizeof(symbol_path_arr));
+module.GetSymbolFileSpec().GetPath(symbol_path_arr,
+   sizeof(symbol_path_arr));
 std::string symbol_path(symbol_path_arr);
 object.try_emplace("symbolFilePath", symbol_path);
   } else {
@@ -352,8 +404,9 @@
   object.try_emplace("addressRange", loaded_addr);
   std::string version_str;
   uint32_t version_nums[3];
-  uint32_t num_versions = module.GetVersion(version_nums, sizeof(version_nums)/sizeof(uint32_t));
-  for (uint32_t i=0; ihttps://llvm.org/LICENSE.txt for license information.
-# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-#
-#======#
-
-r"""
-clang-format git integration
-
-
-This file provides a clang-format integration for git. Put it somewhere in your
-path and ensure that it is executable. Then, "git clang-format" will invoke
-clang-format on the changes in current files or a specific commit.
-
-For further details, run:
-git clang-format -h
-
-Requires Python 2.7 or Python 3
-"""
-
-from __future__ import absolute_import, division, print_function
-import argparse
-import collections
-import contextlib
-import errno
-import os
-import re
-import subprocess
-import sys
-
-usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
-
-desc = '''
-If zero or one commits are given, run clang-format on all lines that differ
-between the working directory and , which defaults to HEAD.  Changes are
-only applied to the working directory.
-
-If two commits are given (requires --diff), run clang-format on all lines in the
-second  that differ from the first .
-
-The following git-config settings set the default of the corresponding option:
-  

[PATCH] D84364: [CUDA][HIP] Defer all diagnostics for host device functions

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D84364#2170244 , @tra wrote:

> I'm going to try the patch on our CUDA code and see how it fares. Stay tuned.


The patch works well enough to build TensorFlow which is a good sign that 
existing working code should be OK -- we're making HD checks less strict, so 
already working code should not be affected.

The remaining concern is whether we're going to unintentionally allow something 
we should not have. Let me see if I can come up with some examples, in addition 
to the `hdf_not_called()`one I've commented on in the test file.




Comment at: clang/test/SemaCUDA/deferred-all.cu:12-18
+// Check no diagnostics for this function since it is never
+// called.
+
+inline __host__ __device__ void hdf_not_called() {
+  callee(1);
+  bad_line
+}

I think we do need diagnostics in this scenario, regardless of whether the 
function is called or not.
We would have emitted it for a regular inline function and I believe it should 
be the case here, too.



Comment at: clang/test/SemaCUDA/deferred-all.cu:44
+
+struct A { int x; typedef int type; };
+struct B { int x; };

Nit: I'd rename `type` -> `isA` which would make its use in sfinae() more 
obvious.


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

https://reviews.llvm.org/D84364



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

[PATCH] D81385: Fix libdl linking for libclang in standalone mode

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa41af6e41e6f: [clang] Fix libdl linking for libclang in 
standalone mode (authored by thieta, committed by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81385

Files:
  clang/tools/libclang/CMakeLists.txt


Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -68,7 +68,12 @@
 
 if (HAVE_LIBDL)
   list(APPEND LIBS ${CMAKE_DL_LIBS})
-endif()
+elseif (CLANG_BUILT_STANDALONE)
+  find_library(DL_LIBRARY_PATH dl)
+  if (DL_LIBRARY_PATH)
+list(APPEND LIBS dl)
+  endif ()
+endif ()
 
 option(LIBCLANG_BUILD_STATIC
   "Build libclang as a static library (in addition to a shared one)" OFF)


Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -68,7 +68,12 @@
 
 if (HAVE_LIBDL)
   list(APPEND LIBS ${CMAKE_DL_LIBS})
-endif()
+elseif (CLANG_BUILT_STANDALONE)
+  find_library(DL_LIBRARY_PATH dl)
+  if (DL_LIBRARY_PATH)
+list(APPEND LIBS dl)
+  endif ()
+endif ()
 
 option(LIBCLANG_BUILD_STATIC
   "Build libclang as a static library (in addition to a shared one)" OFF)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a41af6e - [clang] Fix libdl linking for libclang in standalone mode

[Martin Storsjö via cfe-commits]

Author: Tobias Hieta
Date: 2020-07-24T00:10:22+03:00
New Revision: a41af6e41e6fcf3e7030feaf24057cbe8291b748

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

LOG: [clang] Fix libdl linking for libclang in standalone mode

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

Added: 


Modified: 
clang/tools/libclang/CMakeLists.txt

Removed: 




diff  --git a/clang/tools/libclang/CMakeLists.txt 
b/clang/tools/libclang/CMakeLists.txt
index 9b34682cc49b..a4077140acee 100644
--- a/clang/tools/libclang/CMakeLists.txt
+++ b/clang/tools/libclang/CMakeLists.txt
@@ -68,7 +68,12 @@ endif ()
 
 if (HAVE_LIBDL)
   list(APPEND LIBS ${CMAKE_DL_LIBS})
-endif()
+elseif (CLANG_BUILT_STANDALONE)
+  find_library(DL_LIBRARY_PATH dl)
+  if (DL_LIBRARY_PATH)
+list(APPEND LIBS dl)
+  endif ()
+endif ()
 
 option(LIBCLANG_BUILD_STATIC
   "Build libclang as a static library (in addition to a shared one)" OFF)



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

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

[Christopher Tetreault via Phabricator via cfe-commits]

ctetreau updated this revision to Diff 280254.
ctetreau added a comment.

address code review issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82582

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

Index: clang/lib/CodeGen/SwiftCallingConv.cpp
===
--- clang/lib/CodeGen/SwiftCallingConv.cpp
+++ clang/lib/CodeGen/SwiftCallingConv.cpp
@@ -320,9 +320,12 @@
   // If we have a vector type, split it.
   if (auto vecTy = dyn_cast_or_null(type)) {
 auto eltTy = vecTy->getElementType();
-CharUnits eltSize = (end - begin) / vecTy->getNumElements();
+CharUnits eltSize =
+(end - begin) / cast(vecTy)->getNumElements();
 assert(eltSize == getTypeStoreSize(CGM, eltTy));
-for (unsigned i = 0, e = vecTy->getNumElements(); i != e; ++i) {
+for (unsigned i = 0,
+  e = cast(vecTy)->getNumElements();
+ i != e; ++i) {
   addEntry(eltTy, begin, begin + eltSize);
   begin += eltSize;
 }
@@ -674,8 +677,9 @@
 
 bool swiftcall::isLegalVectorType(CodeGenModule , CharUnits vectorSize,
   llvm::VectorType *vectorTy) {
-  return isLegalVectorType(CGM, vectorSize, vectorTy->getElementType(),
-   vectorTy->getNumElements());
+  return isLegalVectorType(
+  CGM, vectorSize, vectorTy->getElementType(),
+  cast(vectorTy)->getNumElements());
 }
 
 bool swiftcall::isLegalVectorType(CodeGenModule , CharUnits vectorSize,
@@ -688,7 +692,7 @@
 std::pair
 swiftcall::splitLegalVectorType(CodeGenModule , CharUnits vectorSize,
 llvm::VectorType *vectorTy) {
-  auto numElts = vectorTy->getNumElements();
+  auto numElts = cast(vectorTy)->getNumElements();
   auto eltTy = vectorTy->getElementType();
 
   // Try to split the vector type in half.
@@ -710,7 +714,7 @@
   }
 
   // Try to split the vector into legal subvectors.
-  auto numElts = origVectorTy->getNumElements();
+  auto numElts = cast(origVectorTy)->getNumElements();
   auto eltTy = origVectorTy->getElementType();
   assert(numElts != 1);
 
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -1323,7 +1323,7 @@
"Splatted expr doesn't match with vector element type?");
 
 // Splat the element across to all elements
-unsigned NumElements = cast(DstTy)->getNumElements();
+unsigned NumElements = cast(DstTy)->getNumElements();
 return Builder.CreateVectorSplat(NumElements, Src, "splat");
   }
 
@@ -1630,7 +1630,7 @@
 Value *RHS = CGF.EmitScalarExpr(E->getExpr(1));
 Value *Mask;
 
-llvm::VectorType *LTy = cast(LHS->getType());
+auto *LTy = cast(LHS->getType());
 unsigned LHSElts = LTy->getNumElements();
 
 Mask = RHS;
@@ -1648,10 +1648,12 @@
 //   n = extract mask i
 //   x = extract val n
 //   newv = insert newv, x, i
-auto *RTy = llvm::FixedVectorType::get(LTy->getElementType(),
-   MTy->getNumElements());
+auto *RTy = llvm::FixedVectorType::get(
+LTy->getElementType(),
+cast(MTy)->getNumElements());
 Value* NewV = llvm::UndefValue::get(RTy);
-for (unsigned i = 0, e = MTy->getNumElements(); i != e; ++i) {
+for (unsigned i = 0, e = cast(MTy)->getNumElements();
+ i != e; ++i) {
   Value *IIndx = llvm::ConstantInt::get(CGF.SizeTy, i);
   Value *Indx = Builder.CreateExtractElement(Mask, IIndx, "shuf_idx");
 
@@ -1840,7 +1842,7 @@
 return Visit(E->getInit(0));
   }
 
-  unsigned ResElts = VType->getNumElements();
+  unsigned ResElts = cast(VType)->getNumElements();
 
   // Loop over initializers collecting the Value for each, and remembering
   // whether the source was swizzle (ExtVectorElementExpr).  This will allow
@@ -1864,7 +1866,8 @@
   if (isa(IE)) {
 llvm::ExtractElementInst *EI = cast(Init);
 
-if (EI->getVectorOperandType()->getNumElements() == ResElts) {
+if (cast(EI->getVectorOperandType())
+->getNumElements() == ResElts) {
   llvm::ConstantInt *C = cast(EI->getIndexOperand());
   Value *LHS = nullptr, *RHS = nullptr;
   if (CurIdx == 0) {
@@ -1902,7 +1905,7 @@
   continue;
 }
 
-unsigned InitElts = VVT->getNumElements();
+unsigned InitElts = cast(VVT)->getNumElements();
 
 // If the initializer is an ExtVecEltExpr (a swizzle), and the swizzle's
 // input is the same width as the vector being constructed, generate an
@@ -1911,7 +1914,7 @@
 if (isa(IE)) {
   llvm::ShuffleVectorInst *SVI = cast(Init);
   Value *SVOp = SVI->getOperand(0);
-  

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

[Ravi Narayanaswamy via Phabricator via cfe-commits]

RaviNarayanaswamy added a comment.

In D84422#2170667 , @grokos wrote:

> So is the test case that motivated this patch illegal OpenMP code?
>
>   #pragma omp target enter data map(alloc:i)
>   #pragma omp target data map(present, alloc: i)
>   {
> #pragma omp target exit data map(delete:i) // you cannot delete that 
> object in the scope, illegal code?
>   } // fails presence check here
>   ```Sent mail to OpenMP to clarify.





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84422



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

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D80858#2170604 , @hliao wrote:

> In D80858#2170547 , @tra wrote:
>
> > Would it work if we generate a globally unique visible aliases for the 
> > static vars and use the alias' name to register device-side entities 
> > without changing their visibility?
>
>
> We still need to define how a `static` device variable should be visible on 
> the host side.


As far as the host is concerned, if a 'static' variable is within the same TU 
it (or, rather, its shadow) should be accessible.

> How that behaves in the context of relocatable code generation as well as 
> anonymous namespaces.

AFAICT it would just work -- each TU only sees static things within that TU on 
both host and device sides. Visible module-unique aliases would allow host-side 
binary to get address of  an otherwise non-visible variables. I expect that 
will work with RDC -- because we didn't change the visibility of the symbols 
themselves, they behave according to the regular linking rules. The aliases are 
also globally unique, so they should present no issues either.

> Also, in the context of CUDA runtime/driver API, if a device variable is 
> addressable on the host side through the runtime API, it should be 
> addressable through the driver API as well. However, the naming will be a big 
> challenge.

I'm not convinced that I completely agree with this assertion. It's reasonable 
for visible symbols (though even then there's a question of how do you figure 
out a properly mangled name for that symbol), but expecting it to work for 
non-visible symbols would push it too far, IMO.
It will also have issues with RDC as the name resulution will become ambiguous. 
Compiler does have necessary info to resolve that ambiguity, runtime support 
library, too, but the driver does not. We'd have to provide the driver with the 
additional info to let it map host-side symbol info to its device counterpart. 
It's doable, but I don't think it's particularly useful in practice. If 
something needs to be accessed via the driver API, it would be reasonable to 
expect it to be a public symbol.


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

https://reviews.llvm.org/D80858



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

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

[George Rokos via Phabricator via cfe-commits]

grokos added a comment.

So is the test case that motivated this patch illegal OpenMP code?

  #pragma omp target enter data map(alloc:i)
  #pragma omp target data map(present, alloc: i)
  {
#pragma omp target exit data map(delete:i)
  } // fails presence check here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84422



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

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

[Ravi Narayanaswamy via Phabricator via cfe-commits]

RaviNarayanaswamy added a comment.

In D84422#2170285 , @grokos wrote:

> What confuses me about this interpretation of the standard is the 
> inconsistency at `data exit`. So if we have an explicit `omp target exit data 
> map(present...)` then we should respect the "present" semantics, whereas when 
> we have a scoped data exit:
>
>   #pragma omp target data map(present,...)
>   {
> ...
>   } // implicit "exit data" here
>
>
> then "present" should be ignored.
>
> I agree that the paragraph from the standard leaves little room for other 
> interpretations, I'd just like to point out that it looks inconsistent - at 
> least to me.


When you  use present on a  variable on a scoped target data region,  you  
cannot delete that object in the scope.  I would say this  is a test case 
error.  It should still be present on exit, checking for that is maybe redundant


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84422



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

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

rsmith added a comment.

These new builtins should ideally support constant evaluation if possible.




Comment at: clang/docs/LanguageExtensions.rst:2439-2440
 
+Clang provides versions of the following functions which are overloaded based 
on
+the pointer parameter types:
+

This is missing some important details:

- What does the size parameter mean? Is it number of bytes or number of 
elements? If it's number of bytes, what happens if it's not a multiple of the 
element size, particularly in the `_Atomic` case?
- What does the value parameter to `memset` mean? Is it splatted to the element 
width? Does it specify a complete element value?
- For `_Atomic`, what memory order is used?
- For `volatile`, what access size / type is used? Do we want to make any 
promises?
- Are the loads and stores typed or untyped? (In particular, do we annotate 
with TBAA metadata?)
- Do we guarantee to copy the object representation or only the value 
representation? (Do we preserve the values of padding bits in the source, and 
initialize padding bits in the destination?)

You should also document whether constant evaluation of these builtins is 
supported.



Comment at: clang/docs/LanguageExtensions.rst:2446-2448
+These overloads support destinations and sources which are a mix of
+``volatile``, ``_Atomic``, ``restrict``, ``__unaligned``, and use non-default
+address spaces. These can be used as building blocks for different facitilies:

Mixing those qualifiers doesn't seem like it will work in many cases: we don't 
allow mixing `volatile` and `_Atomic` (though I'm not sure why; LLVM supports 
volatile atomic operations), and presumably we shouldn't allow mixing 
`__unaligned` and `_Atomic` (although I don't see any tests for that, and maybe 
we should just outright disallow combining `_Atomic` with `__unaligned` in 
general).



Comment at: clang/include/clang/Basic/Builtins.def:471
 
 // Random GCC builtins
 BUILTIN(__builtin_constant_p, "i.", "nctu")

Are these really GCC builtins?



Comment at: clang/include/clang/Basic/Builtins.def:1487
 
 // Clang builtins (not available in GCC).
 BUILTIN(__builtin_addressof, "v*v&", "nct")

The new builtins probably belong in this section of the file instead.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7961-7963
-def err_atomic_op_needs_trivial_copy : Error<
-  "address argument to atomic operation must be a pointer to a "
-  "trivially-copyable type (%0 invalid)">;

I'd prefer to keep this diagnostic separate, since it communicates more 
information than `err_argument_needs_trivial_copy` does: specifically that we 
need a trivial copy because we're performing an atomic operation.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8926-8934
+def err_const_arg : Error<"argument must be non-const, got %0">;
+
+def err_argument_needs_trivial_copy : Error<"address argument must be a 
pointer to a trivially-copyable type (%0 invalid)">;
+  
+def err_atomic_volatile_unsupported : Error<"mixing _Atomic and volatile 
qualifiers is unsupported (%select{%1|%1 and %2}0 cannot have both _Atomic and 
volatile)">;
+  
+def err_atomic_sizes_must_match : Error<"_Atomic sizes must match, %0 is %1 
bytes and %2 is %3 bytes">;

Please format these diagnostics consistently with the rest of the file: line 
break after `Error<`, wrap to 80 columns, don't leave blank lines between 
individual diagnostics in a group of related diagnostics.



Comment at: clang/lib/Sema/SemaChecking.cpp:1277-1278
 CallExpr *Call) {
-  if (Call->getNumArgs() != 1) {
-S.Diag(Call->getBeginLoc(), diag::err_opencl_builtin_to_addr_arg_num)
-<< Call->getDirectCallee() << Call->getSourceRange();
+  if (checkArgCount(S, Call, 1))
 return true;
 

There are a bunch of places in this file that do manual argument count checking 
and could use `checkArgCount` instead (search for `err_typecheck_call_too_` to 
find them). If you want to clean this up, please do so in a separate change.



Comment at: clang/lib/Sema/SemaChecking.cpp:5510-5512
+  if ((DstValTy->isAtomicType() || SrcValTy->isAtomicType()) &&
+  (!DstValTy.getUnqualifiedType()->isVoidType() &&
+   !SrcValTy.getUnqualifiedType()->isVoidType()) &&

Do we need this constraint?

- If one side is atomic and the other is not, then we can do all of the 
operations with the atomic width.
- If both sides are atomic, then one side is 2^N times the size of the other; 
we can do 2^N operations on one side for each operation on the other side.

Maybe the second case is not worth the effort, but permitting (for example) a 
memcpy from an `_Atomic int*` to a `char*` seems useful and there doesn't seem 
to be a good reason to disallow it.

[PATCH] D84461: [Concepts] Fix ast dump for immediately declared constraint.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: rsmith.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84461

Files:
  clang/lib/AST/TextNodeDumper.cpp
  clang/test/AST/ast-dump-concepts.cpp


Index: clang/test/AST/ast-dump-concepts.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-concepts.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -std=c++2a -ast-dump 
-ast-dump-filter Foo %s | FileCheck -strict-whitespace %s
+
+template 
+concept not_same_as = true;
+
+template 
+struct Foo {
+  // CHECK:  TemplateTypeParmDecl {{.*}} referenced Concept {{.*}} 
'not_same_as'
+  // CHECK-NEXT: |-ConceptSpecializationExpr {{.*}} 'bool'
+  // CHECK-NEXT: `-TemplateArgument {{.*}} type 'int'
+  template  R>
+  Foo(R) requires(true);
+};
Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -1981,7 +1981,7 @@
   dumpBareDeclRef(TC->getFoundDecl());
   OS << ")";
 }
-Visit(TC->getImmediatelyDeclaredConstraint());
+AddChild([=] { Visit(TC->getImmediatelyDeclaredConstraint()); });
   } else if (D->wasDeclaredWithTypename())
 OS << " typename";
   else


Index: clang/test/AST/ast-dump-concepts.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-concepts.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -std=c++2a -ast-dump -ast-dump-filter Foo %s | FileCheck -strict-whitespace %s
+
+template 
+concept not_same_as = true;
+
+template 
+struct Foo {
+  // CHECK:  TemplateTypeParmDecl {{.*}} referenced Concept {{.*}} 'not_same_as'
+  // CHECK-NEXT: |-ConceptSpecializationExpr {{.*}} 'bool'
+  // CHECK-NEXT: `-TemplateArgument {{.*}} type 'int'
+  template  R>
+  Foo(R) requires(true);
+};
Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -1981,7 +1981,7 @@
   dumpBareDeclRef(TC->getFoundDecl());
   OS << ")";
 }
-Visit(TC->getImmediatelyDeclaredConstraint());
+AddChild([=] { Visit(TC->getImmediatelyDeclaredConstraint()); });
   } else if (D->wasDeclaredWithTypename())
 OS << " typename";
   else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

In D80858#2170547 , @tra wrote:

> Would it work if we generate a globally unique visible aliases for the static 
> vars and use the alias' name to register device-side entities without 
> changing their visibility?


We still need to define how a `static` device variable should be visible on the 
host side. How that behaves in the context of relocatable code generation as 
well as anonymous namespaces. Also, in the context of CUDA runtime/driver API, 
if a device variable is addressable on the host side through the runtime API, 
it should be addressable through the driver API as well. However, the naming 
will be a big challenge.


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

https://reviews.llvm.org/D80858



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

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

In D80858#2170533 , @yaxunl wrote:

> In D80858#2170328 , @hliao wrote:
>
> > In D80858#2169534 , @yaxunl wrote:
> >
> > > Another reason is that we need to support it in rdc mode, where different 
> > > TU can have static var with the same name.
> >
> >
> > That's an issue of our current RDC support through LLVM IR instead of 
> > native code. The name conflicts are introduced as we link all TUs into a 
> > single module at IR level. The frontend should not be changed to support 
> > that.
>
>
> I am not sure if ISA level linking would help for this. My understanding is 
> that the linker will rename the internal symbols having the same name from 
> different objects. It does not matter if it is llvm-link or native linker. 
> Once they are renamed we can no longer find them by name.
>
> I don't see a reason why we cannot support accessing static device variable 
> in FE. We have requests for this feature from different users.


the register API also has a module ID parameter. With that, the runtime should 
be able to establish the map from the pointer of that host stub variable to its 
device module and the name within that module. For non-RDC case, that's already 
enough for the runtime to find the correct device variable.

But, as `static` internal linkage is defined, it makes that identifier only 
visible to a single TU and not visible across TU. To be host, I think we still 
need to define that behaves in the context of CUDA or HIP as it obviously 
breaks the definition by making that `static` visible between host and device 
TUs in the restrict definition.


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

https://reviews.llvm.org/D80858



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

[PATCH] D84409: [libTooling] Add an `EditGenerator` that applies a rule throughout a bound node.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 accepted this revision.
gribozavr2 added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:337
+// refer to nodes bound by the calling rule. `Rule` is not applied to the node
+// itself.
+EditGenerator rewriteDescendants(std::string NodeId, RewriteRule Rule);

Please use three slashes for doc comments.



Comment at: clang/lib/Tooling/Transformer/RewriteRule.cpp:194
+for (auto  : transformer::detail::buildMatchers(Rule))
+  MF->addMatcher(forEachDescendantDynamically(std::move(Nodes), 
Matcher),
+ this);

Wouldn't this line move the same value multiple times?



Comment at: clang/lib/Tooling/Transformer/RewriteRule.cpp:208
+}
+// FIXME: some combinators legitimately return no changes.
+if (Transformations->empty()) {

The fix seems trivial -- removing the if statement. Why not do that?



Comment at: clang/unittests/Tooling/TransformerTest.cpp:422
+  std::string Input =
+  "int f(int x) { int y = x; { int z = x * x; } return x; }";
+  std::string Expected =

These tests do not demonstrate that the rewrite is only applied to the 
descendants of the specified AST node -- would be good to test that.



Comment at: clang/unittests/Tooling/TransformerTest.cpp:1135
+
+TEST_F(TransformerTest, RewriteDescendantsUnboundNode) {
+  std::string Input =

Any particular reason to add these tests at the end of the file instead of 
grouped together with the rest of the tests for `rewriteDescendants` above?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84409



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

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

Would it work if we generate a globally unique visible aliases for the static 
vars and use the alias' name to register device-side entities without changing 
their visibility?


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

https://reviews.llvm.org/D80858



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

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

I don't know if the OpenMP committee has any documented rationale for this 
behavior.  I can say that the OpenACC committee is considering the same 
semantics.  However, the issues to consider are not identical.  For example, 
OpenACC has a separate structured reference counter, meaning it should be 
impossible for such data not to be present at the exit of a data construct 
unless you've shut down the runtime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84422



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

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D80858#2170328 , @hliao wrote:

> In D80858#2169534 , @yaxunl wrote:
>
> > Another reason is that we need to support it in rdc mode, where different 
> > TU can have static var with the same name.
>
>
> That's an issue of our current RDC support through LLVM IR instead of native 
> code. The name conflicts are introduced as we link all TUs into a single 
> module at IR level. The frontend should not be changed to support that.


I am not sure if ISA level linking would help for this. My understanding is 
that the linker will rename the internal symbols having the same name from 
different objects. It does not matter if it is llvm-link or native linker. Once 
they are renamed we can no longer find them by name.

I don't see a reason why we cannot support accessing static device variable in 
FE. We have requests for this feature from different users.


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

https://reviews.llvm.org/D80858



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

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

[Volodymyr Sapsai via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:700
+  "no module named '%0' %select{found|in '%2'}1, "
+  "expected parent module to be defined before the submodule">;
 def err_mmap_top_level_inferred_submodule : Error<

Not sure about this second part of the message ("expected..."). Wanted to make 
it more actionable but not entirely happy with the wording. Any suggestions 
including removal are welcome.



Comment at: clang/lib/Lex/ModuleMap.cpp:1911
-  } else {
-Diags.Report(Id[I].second, diag::err_mmap_expected_module_name);
-  }

Both of `err_mmap_missing_module_qualified` and `err_mmap_expected_module_name` 
are still used in other places.



Comment at: clang/lib/Lex/ModuleMap.cpp:1914
   HadError = true;
-  return;
 }

I've removed this `return` because otherwise you get spurious "expected module 
declaration" errors for valid module definition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84458



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

[PATCH] D82598: [analyzer][Liveness][NFC] Get rid of statement liveness, because such a thing doesn't exist

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

xazax.hun added a comment.

In D82598#2164363 , @Szelethus wrote:

> Actually, what I said initially is true:
>
> > [...] the only non-expression statements it **queried** are 
> > ObjCForCollectionStmts [...]


Btw, sorry. I somehow misunderstood the snippet and tought that the test fails 
after the change.  I am also ok with dropping this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82598



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

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

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
vsapsai added reviewers: rsmith, bruno, Bigcheese.
Herald added subscribers: ributzka, dexonsmith, jkorous.
Herald added a project: clang.

Before the change the diagnostic for

  module unknown.submodule {}

was "error: expected module name" which is incorrect and misleading
because both "unknown" and "submodule" are valid module names.

We already have a better error message when a parent module is a
submodule itself and is missing. Make the error for a missing top-level
module more like the one for a submodule.

rdar://problem/64424407


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84458

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


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


Index: clang/test/Modules/diagnostics.modulemap
===
--- clang/test/Modules/diagnostics.modulemap
+++ clang/test/Modules/diagnostics.modulemap
@@ -28,3 +28,9 @@
   header "quux.h" { size 1 mtime 2 }
   header "no_attrs.h" {}
 }
+
+// CHECK: diagnostics.modulemap:[[@LINE+1]]:8: error: no module named 'unknown' found, expected parent module to be defined before the submodule
+module unknown.submodule {}
+module known_top_level {}
+// CHECK: diagnostics.modulemap:[[@LINE+1]]:24: error: no module named 'unknown' in 'known_top_level', expected parent module to be defined before the submodule
+module known_top_level.unknown.submodule {}
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1903,18 +1903,16 @@
 continue;
   }
 
-  if (ActiveModule) {
-Diags.Report(Id[I].second, diag::err_mmap_missing_module_qualified)
-  << Id[I].first
-  << ActiveModule->getTopLevelModule()->getFullModuleName();
-  } else {
-Diags.Report(Id[I].second, diag::err_mmap_expected_module_name);
-  }
+  Diags.Report(Id[I].second, diag::err_mmap_missing_parent_module)
+  << Id[I].first << (ActiveModule != nullptr)
+  << (ActiveModule
+  ? ActiveModule->getTopLevelModule()->getFullModuleName()
+  : "");
   HadError = true;
-  return;
 }
 
-if (ModuleMapFile != 

[PATCH] D83909: [OPENMP] Fix PR46730: Fix compiler crash on taskloop over constructible loop counters.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev updated this revision to Diff 280231.
ABataev added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83909

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/taskloop_codegen.cpp


Index: clang/test/OpenMP/taskloop_codegen.cpp
===
--- clang/test/OpenMP/taskloop_codegen.cpp
+++ clang/test/OpenMP/taskloop_codegen.cpp
@@ -229,4 +229,20 @@
 // CHECK: br label %
 // CHECK: ret i32 0
 
+class St {
+public:
+  operator int();
+  St +=(int);
+};
+
+// CHECK-LABEL: taskloop_with_class
+void taskloop_with_class() {
+  St s1;
+  // CHECK: [[TD:%.+]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* 
@{{.+}}, i32 [[GTID:%.+]], i32 1, i64 88, i64 8, i32 (i32, i8*)* bitcast (i32 
(i32, [[TD_TYPE:%.+]]*)* @{{.+}} to i32 (i32, i8*)*))
+  // CHECK: call void @__kmpc_taskloop(%struct.ident_t* @{{.+}}, i32 [[GTID]], 
i8* [[TD]], i32 1, i64* %{{.+}}, i64* %{{.+}}, i64 %{{.+}}, i32 1, i32 0, i64 
0, i8* bitcast (void ([[TD_TYPE]]*, [[TD_TYPE]]*, i32)* @{{.+}} to i8*))
+#pragma omp taskloop
+  for (St s = St(); s < s1; s += 1) {
+  }
+}
+
 #endif
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -2244,7 +2244,11 @@
   [](OpenMPDirectiveKind K) { return isOpenMPTaskingDirective(K); },
   Level)) {
 bool IsTriviallyCopyable =
-D->getType().getNonReferenceType().isTriviallyCopyableType(Context);
+D->getType().getNonReferenceType().isTriviallyCopyableType(Context) &&
+!D->getType()
+ .getNonReferenceType()
+ .getCanonicalType()
+ ->getAsCXXRecordDecl();
 OpenMPDirectiveKind DKind = DSAStack->getDirective(Level);
 SmallVector CaptureRegions;
 getOpenMPCaptureRegions(CaptureRegions, DKind);


Index: clang/test/OpenMP/taskloop_codegen.cpp
===
--- clang/test/OpenMP/taskloop_codegen.cpp
+++ clang/test/OpenMP/taskloop_codegen.cpp
@@ -229,4 +229,20 @@
 // CHECK: br label %
 // CHECK: ret i32 0
 
+class St {
+public:
+  operator int();
+  St +=(int);
+};
+
+// CHECK-LABEL: taskloop_with_class
+void taskloop_with_class() {
+  St s1;
+  // CHECK: [[TD:%.+]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* @{{.+}}, i32 [[GTID:%.+]], i32 1, i64 88, i64 8, i32 (i32, i8*)* bitcast (i32 (i32, [[TD_TYPE:%.+]]*)* @{{.+}} to i32 (i32, i8*)*))
+  // CHECK: call void @__kmpc_taskloop(%struct.ident_t* @{{.+}}, i32 [[GTID]], i8* [[TD]], i32 1, i64* %{{.+}}, i64* %{{.+}}, i64 %{{.+}}, i32 1, i32 0, i64 0, i8* bitcast (void ([[TD_TYPE]]*, [[TD_TYPE]]*, i32)* @{{.+}} to i8*))
+#pragma omp taskloop
+  for (St s = St(); s < s1; s += 1) {
+  }
+}
+
 #endif
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -2244,7 +2244,11 @@
   [](OpenMPDirectiveKind K) { return isOpenMPTaskingDirective(K); },
   Level)) {
 bool IsTriviallyCopyable =
-D->getType().getNonReferenceType().isTriviallyCopyableType(Context);
+D->getType().getNonReferenceType().isTriviallyCopyableType(Context) &&
+!D->getType()
+ .getNonReferenceType()
+ .getCanonicalType()
+ ->getAsCXXRecordDecl();
 OpenMPDirectiveKind DKind = DSAStack->getDirective(Level);
 SmallVector CaptureRegions;
 getOpenMPCaptureRegions(CaptureRegions, DKind);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: llvm/test/CodeGen/RISCV/shadowcallstack.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32-unknown-elf -mattr=+reserve-x18 
-verify-machineinstrs < %s \
+; RUN: | FileCheck %s --check-prefix=RV32

As I said before, please just use `-mtriple=riscv32`. The `-unknown-elf` is 
implied, irrelevant and wastes space, so all the OS-independent CodeGen tests 
just specify the CPU.



Comment at: llvm/test/CodeGen/RISCV/shadowcallstack.ll:3
+; RUN: llc -mtriple=riscv32-unknown-elf -mattr=+reserve-x18 
-verify-machineinstrs < %s \
+; RUN: | FileCheck %s --check-prefix=RV32
+

Two extra spaces to indent the | is the predominant style.



Comment at: llvm/test/CodeGen/RISCV/shadowcallstack.ll:4
+; RUN: | FileCheck %s --check-prefix=RV32
+
+; RUN: llc -mtriple=riscv64-unknown-elf -mattr=+reserve-x18 
-verify-machineinstrs < %s \

Delete this blank line.



Comment at: llvm/test/CodeGen/RISCV/shadowcallstack.ll:12
+; RV32-NEXT:ret
+; RV32-NOT: x18
+;

pcc wrote:
> Shouldn't it be looking for `s2` since that's how `x18` is spelled in 
> assembly?
The -NOTs shouldn't even exist, this isn't how you use 
`update_llc_test_checks.py`. But yes, by default that's how it'll be printed 
unless you disable printing aliases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414



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

[PATCH] D84341: Implement __builtin_eh_return_data_regno for SystemZ

[Slavomír Kučera via Phabricator via cfe-commits]

slavek-kucera added a comment.

I addressed the code formatting. As I don't have commit access, could you 
commit the change for me?


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

https://reviews.llvm.org/D84341



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

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

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added a comment.

> Any callee-saved register can be used.

In fact, any register may be used, as long as it cannot be used to pass 
arguments or return values. It may be better to select a temporary register, as 
`x18` is on aarch64 when not being used as a platform register, so that 
problems are noticed more easily (i.e. more likely to be clobbered by 
incompatible code).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414



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

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

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: llvm/test/CodeGen/RISCV/shadowcallstack.ll:12
+; RV32-NEXT:ret
+; RV32-NOT: x18
+;

Shouldn't it be looking for `s2` since that's how `x18` is spelled in assembly?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414



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

[PATCH] D84455: [Concepts] Fix a deserialization crash.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

btw, `getTypeConstraint` and `hasTypeConstraint` APIs are quite easy to be 
misused (there is another similar bug in getAssociatedConstraints 
),
 I think we need to refine them (rename or so) to make them less confusing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84455



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

[PATCH] D83731: Add Debug Info Size to Symbol Status

[Greg Clayton via Phabricator via cfe-commits]

clayborg added inline comments.



Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:55-61
+active_modules = self.vscode.get_active_modules()
+program_module = active_modules[program_basename]
+self.assertEqual(program_basename, program_module['name'])
+self.assertEqual(program, program_module['path'])
+self.assertIn('symbolFilePath', program_module)
+self.assertEqual(symbols_path, program_module['symbolFilePath'])
+self.assertIn('addressRange', program_module)

aelitashen wrote:
> clayborg wrote:
> > Unindent everything after the self.waitUntil()? Otherwise we are not 
> > testing the program, symbolFilePath and addressRange on symbols with size.
> When unindenting these codes, the dsym test for darwin fails as the symbol 
> paths don't match. One is using dysm path, one is using a.out.path.
Then we need to fix this function so that it does work. One thing to note in 
the dSYM case: dSYM files are bundles which means it is a directory that 
contains a file within it. So you might specify "/tmp/a.out.dSYM" as the path, 
but end up with "/tmp/a.out.dSYM/Context/Resources/DWARF/a.out" as the symbol 
path. So you could switch the symbol file path to use assertIn:

```
self.assertIn(symbols_path, program_module['symbolFilePath'])
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83731



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

[PATCH] D84457: [OPENMP]Fix PR37671: Privatize local(private) variables in untied tasks.

[Alexey Bataev via Phabricator via cfe-commits]

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

In untied tasks, need to allocate the space for local variales, declared
in task region, when the memory for task data is allocated. THe function
can be interrupted and we can exit from the function in untied task
switch. Need to keep the state of the local variables in this case.
Also, the compiler should not call cleanup when exiting in untied task
switch until the real exit out of the declaration scope is met during
 execution.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84457

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/task_codegen.cpp

Index: clang/test/OpenMP/task_codegen.cpp
===
--- clang/test/OpenMP/task_codegen.cpp
+++ clang/test/OpenMP/task_codegen.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix UNTIEDRT
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
 //
@@ -259,7 +259,7 @@
 a = 4;
 c = 5;
   }
-// CHECK: [[ORIG_TASK_PTR:%.+]] = call i8* @__kmpc_omp_task_alloc([[IDENT_T]]* @{{.+}}, i32 [[GTID]], i32 0, i64 40, i64 1, i32 (i32, i8*)* bitcast (i32 (i32, [[KMP_TASK_T]]{{.*}}*)* [[TASK_ENTRY6:@.+]] to i32 (i32, i8*)*))
+// CHECK: [[ORIG_TASK_PTR:%.+]] = call i8* @__kmpc_omp_task_alloc([[IDENT_T]]* @{{.+}}, i32 [[GTID]], i32 0, i64 48, i64 1, i32 (i32, i8*)* bitcast (i32 (i32, [[KMP_TASK_T]]{{.*}}*)* [[TASK_ENTRY6:@.+]] to i32 (i32, i8*)*))
 // CHECK: call i32 @__kmpc_omp_task([[IDENT_T]]* @{{.+}}, i32 [[GTID]], i8* [[ORIG_TASK_PTR]])
 #pragma omp task untied
   {
@@ -296,26 +296,54 @@
 // CHECK: store i32 4, i32* [[A_PTR]]
 
 // CHECK: define internal i32 [[TASK_ENTRY6]](i32 %0, [[KMP_TASK_T]]{{.*}}* noalias %1)
-// CHECK: switch i32 %{{.+}}, label
+// UNTIEDRT: [[S1_ADDR_PTR:%.+]] = alloca %struct.S*,
+// UNTIEDRT: call void (i8*, ...) %{{.+}}(i8* %{{.+}}, %struct.S** [[S1_ADDR_PTR]])
+// UNTIEDRT: [[S1_ADDR:%.+]] = load %struct.S*, %struct.S** [[S1_ADDR_PTR]],
+// CHECK: switch i32 %{{.+}}, label %[[DONE:.+]] [
+
+// CHECK: [[DONE]]:
+// CHECK: br label %[[CLEANUP:[^,]+]]
+
 // CHECK: load i32*, i32** %
 // CHECK: store i32 1, i32* %
 // CHECK: call i32 @__kmpc_omp_task(%
+// UNTIEDRT: br label %[[EXIT:[^,]+]]
 
+// UNTIEDRT: call void [[CONSTR:@.+]](%struct.S* [[S1_ADDR]])
 // CHECK: call i8* @__kmpc_omp_task_alloc(
 // CHECK: call i32 @__kmpc_omp_task(%
 // CHECK: load i32*, i32** %
 // CHECK: store i32 2, i32* %
 // CHECK: call i32 @__kmpc_omp_task(%
+// UNTIEDRT: br label %[[EXIT]]
 
 // CHECK: call i32 @__kmpc_omp_taskyield(%
 // CHECK: load i32*, i32** %
 // CHECK: store i32 3, i32* %
 // CHECK: call i32 @__kmpc_omp_task(%
+// UNTIEDRT: br label %[[EXIT]]
+
+// s1 = S();
+// UNTIEDRT: call void [[CONSTR]](%struct.S* [[TMP:%.+]])
+// UNTIEDRT: [[DST:%.+]] = bitcast %struct.S* [[S1_ADDR]] to i8*
+// UNTIEDRT: [[SRC:%.+]] = bitcast %struct.S* [[TMP]] to i8*
+// UNTIEDRT: call void @llvm.memcpy.{{.+}}(i8* {{.*}}[[DST]], i8* {{.*}}[[SRC]], i64 4, i1 false)
+// UNTIEDRT: call void [[DESTR:@.+]](%struct.S* [[TMP]])
 
 // CHECK: call i32 @__kmpc_omp_taskwait(%
 // CHECK: load i32*, i32** %
 // CHECK: store i32 4, i32* %
 // CHECK: call i32 @__kmpc_omp_task(%
+// UNTIEDRT: br label %[[EXIT]]
+
+// UNTIEDRT: call void [[DESTR]](%struct.S* [[S1_ADDR]])
+// CHECK: br label %[[CLEANUP]]
+
+// CHECK: [[CLEANUP]]:
+// UNTIEDRT: br label %[[EXIT]]
+
+// UNTIEDRT:  [[EXIT]]:
+// UNTIEDRT-NEXT: ret i32 0
 
 struct S1 {
   int a;
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/OpenMPClause.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Basic/PrettyStackTrace.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
@@ -3785,6 +3786,42 @@
   checkForLastprivateConditionalUpdate(*this, S);
 }
 
+namespace {
+/// Get the list of variables declared in the context of the untied tasks.
+class CheckVarsEscapingUntiedTaskDeclContext final
+: public ConstStmtVisitor {
+  llvm::SmallVector PrivateDecls;
+
+public:
+  explicit CheckVarsEscapingUntiedTaskDeclContext() = default;
+  virtual ~CheckVarsEscapingUntiedTaskDeclContext() = default;
+  void VisitDeclStmt(const DeclStmt *S) {
+if (!S)
+  return;
+

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D80858#2170311 , @hliao wrote:

> > The problem is that even though the static variable is registered through 
> > `__hipRigisterVariable`, the runtime still relies on looking up symbol name 
> > to get the address of the device variable. That's why we need to 
> > externalize the static variable.
>
> If so, the runtime should be fixed as the variable name. I remembered I fixed 
> the global one so that each one is uniquely identified by module id plus the 
> name. For runtime APIs, all host-side references to device variables should 
> look through the host stub variables instead of its name. If runtime or API 
> doesn't follow that, we should fix them instead of asking the compiler to do 
> the favor.


For runtime APIs, we do reference device variables through host stub variable 
instead of its name. However, how does runtime implements that internally?

In host compilation, clang emits call of `__hipRegisterVariable(shadow_var, 
device_var_name)` in init functions.

runtime builds a map of each shadow var to a device var name. then runtime 
looks up device var name in code object to get the real address of a device var.

Note: `__hipRegisterVariable` does not really associate a shadow var with the 
address of a device var, since in host compilation there is no way to know the 
address of a device var. It only associates a shadow var with the name of a 
device var.

So eventually, runtime still needs to look up the device var symbol in code 
objects. Since ROCm runtime does not look up local symbols, it cannot find the 
static device var in code objects, unless we externalize it.


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

https://reviews.llvm.org/D80858



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

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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:33-37
+  std::vector  = MF.getFrameInfo().getCalleeSavedInfo();
+  if (std::none_of(CSI.begin(), CSI.end(), [](CalleeSavedInfo ) {
+return CSR.getReg() == RISCV::X1;
+  }))
+return;

jrtc27 wrote:
> ```
> if (find(CSI, RISCV::X1) == CSI.end())
>   return;
> ```
> (using `llvm::find` as a convenient wrapper around `std::find`, ie shorthand 
> for `std::find(CSI.begin(), CSI.end(), RISCV::X1)`). Though personally I'd 
> prefer to see X1 come from `RI.getRARegister()` rather than be hard-coded; 
> other functions in this file already hard-code it, but in our CHERI fork we 
> need to replace RISCV::X1 with RISCV::C1 everywhere so have changed those. 
> Having said that, CHERI renders a shadow call stack unnecessary, so I don't 
> particularly care if it's broken there, personally. But I still think it's 
> nicer code.
`!llvm::is_contained(CSI, RISCV::X1)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414



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

[PATCH] D83731: Add Debug Info Size to Symbol Status

[Yifan Shen via Phabricator via cfe-commits]

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



Comment at: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py:55-61
+active_modules = self.vscode.get_active_modules()
+program_module = active_modules[program_basename]
+self.assertEqual(program_basename, program_module['name'])
+self.assertEqual(program, program_module['path'])
+self.assertIn('symbolFilePath', program_module)
+self.assertEqual(symbols_path, program_module['symbolFilePath'])
+self.assertIn('addressRange', program_module)

clayborg wrote:
> Unindent everything after the self.waitUntil()? Otherwise we are not testing 
> the program, symbolFilePath and addressRange on symbols with size.
When unindenting these codes, the dsym test for darwin fails as the symbol 
paths don't match. One is using dysm path, one is using a.out.path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83731



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

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

[Jessica Clarke via Phabricator via cfe-commits]

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

1. Please use local variables with meaningful names for `RISCV::Xn` rather than 
inlining them everywhere and making it harder at a glance to work out what's 
going on without knowing what the ABI register names are.

2. Please make a `RISCVABI::getSCSPReg` or similar function to avoid 
hard-coding X18 in a bunch of places.

3. I'm not convinced a callee-saved register makes any more sense than anything 
else. Whatever you pick, compatibility only works one way round. If foo uses an 
SCS and calls bar that doesn't, then being callee saved helps you, but if bar 
calls foo then (a) foo will try to dereference SCSP (to store then later load), 
likely faulting, perhaps instead "just" clobbering arbitrary memory (b) if foo 
manages to return back to bar without faulting then bar would expect the 
register to have been saved, but it hasn't, an ABI violation. If you use a 
caller-saved register instead then bar can call foo just fine, but foo can't 
call bar as it'll clobber its SCSP. Reserving any register like this is a 
fundamental ABI incompatibility, so picking x18 because it's callee-saved is 
meaningless, and picking it to avoid duplicating 6 lines of code (or fewer, if 
the existing 6 lines are refactored) isn't a good reason either. It's 
ultimately arbitrary, but it should be at least be based on some kind of valid 
reasoning if such reasoning exists. X18 may or may not be a right answer.

  (One might initially think that this incompatibility is fine if you're 
building an executable with SCSP that calls other libraries without SCSP, but 
that breaks down as soon as callbacks exist, as well as more niche things like 
symbol preemption.)




Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:33-37
+  std::vector  = MF.getFrameInfo().getCalleeSavedInfo();
+  if (std::none_of(CSI.begin(), CSI.end(), [](CalleeSavedInfo ) {
+return CSR.getReg() == RISCV::X1;
+  }))
+return;

```
if (find(CSI, RISCV::X1) == CSI.end())
  return;
```
(using `llvm::find` as a convenient wrapper around `std::find`, ie shorthand 
for `std::find(CSI.begin(), CSI.end(), RISCV::X1)`). Though personally I'd 
prefer to see X1 come from `RI.getRARegister()` rather than be hard-coded; 
other functions in this file already hard-code it, but in our CHERI fork we 
need to replace RISCV::X1 with RISCV::C1 everywhere so have changed those. 
Having said that, CHERI renders a shadow call stack unnecessary, so I don't 
particularly care if it's broken there, personally. But I still think it's 
nicer code.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:54
+  // addi s2, s2, 4
+  BuildMI(MBB, MI, DL, TII->get(RISCV::SW))
+  .addReg(RISCV::X1)

This is wrong for RV64.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:69-73
+  std::vector  = MF.getFrameInfo().getCalleeSavedInfo();
+  if (std::none_of(CSI.begin(), CSI.end(), [](CalleeSavedInfo ) {
+return CSR.getReg() == RISCV::X1;
+  }))
+return;

As above.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:94
+  .addImm(-SlotSize);
+  BuildMI(MBB, MI, DL, TII->get(RISCV::LW))
+  .addReg(RISCV::X1, RegState::Define)

Also wrong for RV64.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414



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

[PATCH] D84455: [Concepts] Fix a deserialization crash.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: rsmith.
Herald added a subscriber: kristof.beyls.
Herald added a project: clang.

`TemplateTypeParmDecl::hasTypeConstraint` is not a safe guard for
checking `TemplateTypeParmDecl::getTypeConstraint()` result is null.

in somecases (e.g. implicit deduction guide templates synthesized from the
constructor), hasTypeConstraint returns false, and getTypeConstraint
returns a nullptr.

Fix https://bugs.llvm.org/show_bug.cgi?id=46790


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84455

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/test/PCH/cxx2a-constraints-crash.cpp


Index: clang/test/PCH/cxx2a-constraints-crash.cpp
===
--- /dev/null
+++ clang/test/PCH/cxx2a-constraints-crash.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -std=c++2a -emit-pch %s -o %t
+// RUN: %clang_cc1 -std=c++2a -include-pch %t -verify %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template 
+concept not_same_as = true;
+
+template 
+struct subrange {
+  template  R>
+  subrange(R) requires(Kind == 0);
+
+  template  R>
+  subrange(R) requires(Kind != 0);
+};
+
+template 
+subrange(R) -> subrange<42>;
+
+int main() {
+  int c;
+  subrange s(c);
+}
+
+#endif
\ No newline at end of file
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2909,9 +2909,11 @@
   return false;
 if (TX->hasTypeConstraint() != TY->hasTypeConstraint())
   return false;
-if (TX->hasTypeConstraint()) {
-  const TypeConstraint *TXTC = TX->getTypeConstraint();
-  const TypeConstraint *TYTC = TY->getTypeConstraint();
+const TypeConstraint *TXTC = TX->getTypeConstraint();
+const TypeConstraint *TYTC = TY->getTypeConstraint();
+if (!TXTC != !TYTC)
+  return false;
+if (TXTC && TYTC) {
   if (TXTC->getNamedConcept() != TYTC->getNamedConcept())
 return false;
   if (TXTC->hasExplicitTemplateArgs() != TYTC->hasExplicitTemplateArgs())


Index: clang/test/PCH/cxx2a-constraints-crash.cpp
===
--- /dev/null
+++ clang/test/PCH/cxx2a-constraints-crash.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -std=c++2a -emit-pch %s -o %t
+// RUN: %clang_cc1 -std=c++2a -include-pch %t -verify %s
+
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template 
+concept not_same_as = true;
+
+template 
+struct subrange {
+  template  R>
+  subrange(R) requires(Kind == 0);
+
+  template  R>
+  subrange(R) requires(Kind != 0);
+};
+
+template 
+subrange(R) -> subrange<42>;
+
+int main() {
+  int c;
+  subrange s(c);
+}
+
+#endif
\ No newline at end of file
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2909,9 +2909,11 @@
   return false;
 if (TX->hasTypeConstraint() != TY->hasTypeConstraint())
   return false;
-if (TX->hasTypeConstraint()) {
-  const TypeConstraint *TXTC = TX->getTypeConstraint();
-  const TypeConstraint *TYTC = TY->getTypeConstraint();
+const TypeConstraint *TXTC = TX->getTypeConstraint();
+const TypeConstraint *TYTC = TY->getTypeConstraint();
+if (!TXTC != !TYTC)
+  return false;
+if (TXTC && TYTC) {
   if (TXTC->getNamedConcept() != TYTC->getNamedConcept())
 return false;
   if (TXTC->hasExplicitTemplateArgs() != TYTC->hasExplicitTemplateArgs())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83915: [PowerPC] Remove QPX/A2Q BGQ/BGP support

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

Thanks for working on this. Please, however, take another pass of the test 
cases (especially those that are not in the PowerPC directory).  Many of those 
should not be deleted, please change triple instead. They're testing general 
functionality.




Comment at: 
llvm/test/Transforms/DeadStoreElimination/combined-partial-overwrites.ll:1
-; RUN: opt -S -dse -enable-dse-partial-store-merging=false < %s | FileCheck %s
-target datalayout = "E-m:e-i64:64-n32:64"

This test should not be deleted. Change the triple.



Comment at: llvm/test/Transforms/EntryExitInstrumenter/mcount.ll:1
-; RUN: opt 
-passes="function(ee-instrument),cgscc(inline),function(post-inline-ee-instrument)"
 -S < %s | FileCheck %s
-

This test should not be deleted. It's testing general functionality. The triple 
should be changed.



Comment at: openmp/runtime/src/kmp_platform.h:71
-#undef KMP_OS_CNK
-#define KMP_OS_CNK 1
-#endif

Was this not used anywhere? Or is there more to delete?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83915



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

[PATCH] D84348: WIP: Add complete id-expression support to syntax trees

[Eduardo Caldas via Phabricator via cfe-commits]

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

Update API to new nested-name-specifier grammar rule


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84348

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

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -988,13 +988,13 @@
 | | | |-NestedNameSpecifier
 | | | | |-GlobalNameSpecifier
 | | | | | `-::
-| | | | |-NamespaceNameSpecifier
+| | | | |-IdentifierNameSpecifier
 | | | | | |-n
 | | | | | `-::
-| | | | |-TypeNameSpecifier
+| | | | |-IdentifierNameSpecifier
 | | | | | |-S
 | | | | | `-::
-| | | | `-TypeWithTemplateNameSpecifier
+| | | | `-SimpleTemplateNameSpecifier
 | | | |   |-template
 | | | |   |-TS
 | | | |   |-<
@@ -1010,13 +1010,13 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-NamespaceNameSpecifier
+| | | | |-IdentifierNameSpecifier
 | | | | | |-n
 | | | | | `-::
-| | | | |-TypeNameSpecifier
+| | | | |-IdentifierNameSpecifier
 | | | | | |-S
 | | | | | `-::
-| | | | `-TypeNameSpecifier
+| | | | `-SimpleTemplateNameSpecifier
 | | | |   |-TS
 | | | |   |-<
 | | | |   |-int
@@ -1031,13 +1031,13 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-TypeNameSpecifier
+| | | | |-SimpleTemplateNameSpecifier
 | | | | | |-TS
 | | | | | |-<
 | | | | | |-int
 | | | | | |->
 | | | | | `-::
-| | | | `-TypeNameSpecifier
+| | | | `-IdentifierNameSpecifier
 | | | |   |-S
 | | | |   `-::
 | | | `-UnqualifiedId
@@ -1052,13 +1052,13 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-TypeNameSpecifier
+| | | | |-SimpleTemplateNameSpecifier
 | | | | | |-TS
 | | | | | |-<
 | | | | | |-int
 | | | | | |->
 | | | | | `-::
-| | | | `-TypeNameSpecifier
+| | | | `-IdentifierNameSpecifier
 | | | |   |-S
 | | | |   `-::
 | | | |-template
@@ -1116,10 +1116,10 @@
   | |-UnknownExpression
   | | |-IdExpression
   | | | |-NestedNameSpecifier
-  | | | | |-TypeNameSpecifier
+  | | | | |-IdentifierNameSpecifier
   | | | | | |-T
   | | | | | `-::
-  | | | | `-TypeWithTemplateNameSpecifier
+  | | | | `-SimpleTemplateNameSpecifier
   | | | |   |-template
   | | | |   |-U
   | | | |   |-<
@@ -1135,7 +1135,7 @@
   | |-UnknownExpression
   | | |-IdExpression
   | | | |-NestedNameSpecifier
-  | | | | |-TypeNameSpecifier
+  | | | | |-IdentifierNameSpecifier
   | | | | | |-T
   | | | | | `-::
   | | | | `-IdentifierNameSpecifier
@@ -1150,7 +1150,7 @@
   | |-UnknownExpression
   | | |-IdExpression
   | | | |-NestedNameSpecifier
-  | | | | `-TypeNameSpecifier
+  | | | | `-IdentifierNameSpecifier
   | | | |   |-T
   | | | |   `-::
   | | | |-template
@@ -1217,7 +1217,7 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | `-TypeNameSpecifier
+| | | | `-DecltypeNameSpecifier
 | | | |   |-decltype
 | | | |   |-(
 | | | |   |-IdExpression
Index: clang/lib/Tooling/Syntax/Nodes.cpp
===
--- clang/lib/Tooling/Syntax/Nodes.cpp
+++ clang/lib/Tooling/Syntax/Nodes.cpp
@@ -118,16 +118,12 @@
 return OS << "MemberPointer";
   case NodeKind::GlobalNameSpecifier:
 return OS << "GlobalNameSpecifier";
-  case NodeKind::NamespaceNameSpecifier:
-return OS << "NamespaceNameSpecifier";
-  case NodeKind::TypeNameSpecifier:
-return OS << "TypeNameSpecifier";
-  case NodeKind::UnknownNameSpecifier:
-return OS << "UnknownNameSpecifier";
+  case NodeKind::DecltypeNameSpecifier:
+return OS << "DecltypeNameSpecifier";
   case NodeKind::IdentifierNameSpecifier:
 return OS << "IdentifierNameSpecifier";
-  case NodeKind::TypeWithTemplateNameSpecifier:
-return OS << "TypeWithTemplateNameSpecifier";
+  case NodeKind::SimpleTemplateNameSpecifier:
+return OS << "SimpleTemplateNameSpecifier";
   case NodeKind::NestedNameSpecifier:
 return OS << "NestedNameSpecifier";
   }
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -744,23 +744,37 @@
 return true;
   }
 
-  syntax::NameSpecifier *
-  BuildNameSpecifier(NestedNameSpecifier::SpecifierKind K) {
-

[PATCH] D84453: [clang-tidy] Suppress one unittest under ASan.

[Valeriy Savchenko via Phabricator via cfe-commits]

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

LGTM, thanks for taking care of this!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D84453



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

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

[Z. Zheng via Phabricator via cfe-commits]

zzheng added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:95
+  .addReg(RISCV::X18)
+  .addImm(0);
+}

apazos wrote:
> There are thee things to observe here and other reviewers might have some 
> additional comments:
> 
> - RISC-V does not have a reserved platform register like AAch64. The patch 
> uses one of the RISC-V callee saved registers, x18, which happens to coincide 
> with AArch64's register. It is possible to select another register, and 
> additional checks for the flag combo "-fsanitize=shadow-call-stack 
> -ffixed-" will have to be added.
> 
> - The  return address is saved on both the SCS (whose location is 
> protected/hidden) and also in the regular stack. But the return from a 
> function uses the value saved on SCS. The understanding is that not saving it 
> in the regular stack can impact debugging.
> 
> - The SCS is ascending, while the regular stack, by RISC-V convention, is 
> descending. The SCS is not used for passing parameters between calls like the 
> regular stack, so it seems to be ok. But this can be changed too. AArch64 's 
> SCS is also ascending.
Thanks for the clarification, Ana.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414



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

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

[Z. Zheng via Phabricator via cfe-commits]

zzheng updated this revision to Diff 280210.
zzheng marked 3 inline comments as done.
zzheng edited the summary of this revision.
zzheng added a comment.

Using 'BLOCKED' now.

clang-formated RISCVFrameLowering.cpp

updated style of test/CodeGen/RISCV/shadowcallstack.ll


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414

Files:
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/test/CodeGen/shadowcallstack-attr.c
  clang/test/Driver/sanitizer-ld.c
  llvm/lib/Target/RISCV/RISCVFrameLowering.cpp
  llvm/test/CodeGen/RISCV/shadowcallstack.ll

Index: llvm/test/CodeGen/RISCV/shadowcallstack.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/shadowcallstack.ll
@@ -0,0 +1,179 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv32-unknown-elf -mattr=+reserve-x18 -verify-machineinstrs < %s \
+; RUN: | FileCheck %s --check-prefix=RV32
+
+; RUN: llc -mtriple=riscv64-unknown-elf -mattr=+reserve-x18 -verify-machineinstrs < %s \
+; RUN: | FileCheck %s --check-prefix=RV64
+
+define void @f1() shadowcallstack {
+; RV32-LABEL: f1:
+; RV32:   # %bb.0:
+; RV32-NEXT:ret
+; RV32-NOT: x18
+;
+; RV64-LABEL: f1:
+; RV64:   # %bb.0:
+; RV64-NEXT:ret
+; RV64-NOT: x18
+  ret void
+}
+
+declare void @foo()
+
+define void @f2() shadowcallstack {
+; RV32-LABEL: f2:
+; RV32:   # %bb.0:
+; RV32-NEXT:tail foo
+; RV32-NOT: x18
+;
+; RV64-LABEL: f2:
+; RV64:   # %bb.0:
+; RV64-NEXT:tail foo
+; RV64-NOT: x18
+  tail call void @foo()
+  ret void
+}
+
+declare i32 @bar()
+
+define i32 @f3() shadowcallstack {
+; RV32-LABEL: f3:
+; RV32:   # %bb.0:
+; RV32-NEXT:sw ra, 0(s2)
+; RV32-NEXT:addi s2, s2, 4
+; RV32-NEXT:addi sp, sp, -16
+; RV32-NEXT:.cfi_def_cfa_offset 16
+; RV32-NEXT:sw ra, 12(sp)
+; RV32-NEXT:.cfi_offset ra, -4
+; RV32-NEXT:call bar
+; RV32-NEXT:lw ra, 12(sp)
+; RV32-NEXT:addi sp, sp, 16
+; RV32-NEXT:addi s2, s2, -4
+; RV32-NEXT:lw ra, 0(s2)
+; RV32-NEXT:ret
+;
+; RV64-LABEL: f3:
+; RV64:   # %bb.0:
+; RV64-NEXT:sw ra, 0(s2)
+; RV64-NEXT:addi s2, s2, 8
+; RV64-NEXT:addi sp, sp, -16
+; RV64-NEXT:.cfi_def_cfa_offset 16
+; RV64-NEXT:sd ra, 8(sp)
+; RV64-NEXT:.cfi_offset ra, -8
+; RV64-NEXT:call bar
+; RV64-NEXT:ld ra, 8(sp)
+; RV64-NEXT:addi sp, sp, 16
+; RV64-NEXT:addi s2, s2, -8
+; RV64-NEXT:lw ra, 0(s2)
+; RV64-NEXT:ret
+  %res = call i32 @bar()
+  %res1 = add i32 %res, 1
+  ret i32 %res
+}
+
+define i32 @f4() shadowcallstack {
+; RV32-LABEL: f4:
+; RV32:   # %bb.0:
+; RV32-NEXT:sw ra, 0(s2)
+; RV32-NEXT:addi s2, s2, 4
+; RV32-NEXT:addi sp, sp, -16
+; RV32-NEXT:.cfi_def_cfa_offset 16
+; RV32-NEXT:sw ra, 12(sp)
+; RV32-NEXT:sw s0, 8(sp)
+; RV32-NEXT:sw s1, 4(sp)
+; RV32-NEXT:sw s3, 0(sp)
+; RV32-NEXT:.cfi_offset ra, -4
+; RV32-NEXT:.cfi_offset s0, -8
+; RV32-NEXT:.cfi_offset s1, -12
+; RV32-NEXT:.cfi_offset s3, -16
+; RV32-NEXT:call bar
+; RV32-NEXT:mv s3, a0
+; RV32-NEXT:call bar
+; RV32-NEXT:mv s1, a0
+; RV32-NEXT:call bar
+; RV32-NEXT:mv s0, a0
+; RV32-NEXT:call bar
+; RV32-NEXT:add a1, s3, s1
+; RV32-NEXT:add a0, s0, a0
+; RV32-NEXT:add a0, a1, a0
+; RV32-NEXT:lw s3, 0(sp)
+; RV32-NEXT:lw s1, 4(sp)
+; RV32-NEXT:lw s0, 8(sp)
+; RV32-NEXT:lw ra, 12(sp)
+; RV32-NEXT:addi sp, sp, 16
+; RV32-NEXT:addi s2, s2, -4
+; RV32-NEXT:lw ra, 0(s2)
+; RV32-NEXT:ret
+;
+; RV64-LABEL: f4:
+; RV64:   # %bb.0:
+; RV64-NEXT:sw ra, 0(s2)
+; RV64-NEXT:addi s2, s2, 8
+; RV64-NEXT:addi sp, sp, -32
+; RV64-NEXT:.cfi_def_cfa_offset 32
+; RV64-NEXT:sd ra, 24(sp)
+; RV64-NEXT:sd s0, 16(sp)
+; RV64-NEXT:sd s1, 8(sp)
+; RV64-NEXT:sd s3, 0(sp)
+; RV64-NEXT:.cfi_offset ra, -8
+; RV64-NEXT:.cfi_offset s0, -16
+; RV64-NEXT:.cfi_offset s1, -24
+; RV64-NEXT:.cfi_offset s3, -32
+; RV64-NEXT:call bar
+; RV64-NEXT:mv s3, a0
+; RV64-NEXT:call bar
+; RV64-NEXT:mv s1, a0
+; RV64-NEXT:call bar
+; RV64-NEXT:mv s0, a0
+; RV64-NEXT:call bar
+; RV64-NEXT:add a1, s3, s1
+; RV64-NEXT:add a0, s0, a0
+; RV64-NEXT:addw a0, a1, a0
+; RV64-NEXT:ld s3, 0(sp)
+; RV64-NEXT:ld s1, 8(sp)
+; RV64-NEXT:ld s0, 16(sp)
+; RV64-NEXT:ld ra, 24(sp)
+; RV64-NEXT:addi sp, sp, 32
+; RV64-NEXT:addi s2, s2, -8
+; RV64-NEXT:lw ra, 0(s2)
+; RV64-NEXT:ret
+  %res1 = call i32 @bar()
+  %res2 = call i32 @bar()
+  %res3 = call i32 @bar()
+  %res4 = call i32 @bar()
+  %res12 = add i32 %res1, %res2
+  %res34 = add i32 %res3, %res4
+  %res1234 = add i32 %res12, %res34
+  ret i32 %res1234
+}
+
+define i32 @f5() shadowcallstack nounwind {
+; RV32-LABEL: f5:
+; RV32:   # %bb.0:
+; 

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

In D80858#2169534 , @yaxunl wrote:

> Another reason is that we need to support it in rdc mode, where different TU 
> can have static var with the same name.


That's an issue of our current RDC support through LLVM IR instead of native 
code. The name conflicts are introduced as we link all TUs into a single module 
at IR level. The frontend should not be changed to support that.


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

https://reviews.llvm.org/D80858



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

[PATCH] D84453: [clang-tidy] Suppress one unittest under ASan.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: alexfh, gribozavr, dcoughlin, delcypher, yln, kubamracek, 
vsavchenko.
Herald added subscribers: martong, mgehre, Charusso, xazax.hun.

It's failing on macOS under AddressSanitizer for no obvious reason - i.e., no 
report is produced, just the code isn't working correctly. Might possibly be a 
bug in ASan but I want to silence an internal buildbot before a deeper 
investigation. I didn't try platforms other than macOS.

The failure can be reproduced somewhere around 
rG0b2a92224630f6e177d091b8391cfa943764aba5 
 by 
self-hosting. The unittest has to be compiled as ` -DCMAKE_BUILD_TYPE=Release 
-DLLVM_USE_SANITIZER="Address"` in order to reproduce the issue 
(`RelWithDebInfo` works too, but not `Debug`).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D84453

Files:
  clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp


Index: 
clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
@@ -21,6 +21,9 @@
   }
 };
 
+// FIXME: This test currently fails under ASan without any actual ASan report.
+// It's as if runCheckOnCode() is not invoked at all. Must be a bug.
+#if !defined(__has_feature) || !__has_feature(address_sanitizer)
 TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
   std::vector Errors;
   runCheckOnCode("int a;", );
@@ -28,6 +31,7 @@
   EXPECT_EQ("type specifier", Errors[0].Message.Message);
   EXPECT_EQ("variable", Errors[1].Message.Message);
 }
+#endif
 
 } // namespace test
 } // namespace tidy


Index: clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ClangTidyDiagnosticConsumerTest.cpp
@@ -21,6 +21,9 @@
   }
 };
 
+// FIXME: This test currently fails under ASan without any actual ASan report.
+// It's as if runCheckOnCode() is not invoked at all. Must be a bug.
+#if !defined(__has_feature) || !__has_feature(address_sanitizer)
 TEST(ClangTidyDiagnosticConsumer, SortsErrors) {
   std::vector Errors;
   runCheckOnCode("int a;", );
@@ -28,6 +31,7 @@
   EXPECT_EQ("type specifier", Errors[0].Message.Message);
   EXPECT_EQ("variable", Errors[1].Message.Message);
 }
+#endif
 
 } // namespace test
 } // namespace tidy
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

In D80858#2169399 , @yaxunl wrote:

> In D80858#2169295 , @hliao wrote:
>
> > I don't that's proper way to support file-scope static device variables. As 
> > we discuss APIs like cudaMemcpyToSymol, that's a runtime API instead of 
> > driver API. The later needs to specify the module (or code object) id in 
> > addition to a symbol name and won't have the conflict issues. For the 
> > runtime API, all named device variables (static or not) are identified at 
> > the host side by their host stub variables. That stub variable is used to 
> > register the corresponding device variables associated with a module id to 
> > unique identify that variables across all TUs. As long as we look up device 
> > variables using their host stub variable pointers, they are uniquely 
> > identified already. The runtime implementation needs to find the module id 
> > and the variable symbol from the pointer of its host stub variable. It's 
> > not the compiler job to fabricate name uniquely across TUs.
>
>
> The problem is that even though the static variable is registered through 
> `__hipRigisterVariable`, the runtime still relies on looking up symbol name 
> to get the address of the device variable. That's why we need to externalize 
> the static variable.


If so, the runtime should be fixed as the variable name. I remembered I fixed 
the global one so that each one is uniquely identified by module id plus the 
name. For runtime APIs, all host-side references to device variables should 
look through the host stub variables instead of its name. If runtime or API 
doesn't follow that, we should fix them instead of asking the compiler to do 
the favor.


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

https://reviews.llvm.org/D80858



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

[PATCH] D84356: [AIX] remove -u from the clang when invoke aix as assembler

[Jason Liu via Phabricator via cfe-commits]

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

Thanks. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84356



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

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

[George Rokos via Phabricator via cfe-commits]

grokos added a comment.

What confuses me about this interpretation of the standard is the inconsistency 
at `data exit`. So if we have an explicit `omp target exit data 
map(present...)` then we should respect the "present" semantics, whereas when 
we have a scoped data exit:

  #pragma omp target data map(present,...)
  {
...
  } // implicit "exit data" here

then "present" should be ignored.

I agree that the paragraph from the standard leaves little room for other 
interpretations, I'd just like to point out that it looks inconsistent - at 
least to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84422



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

>>> Do you think it'd be useful to have different guarantees for different 
>>> operands?  I guess it could come up, but it'd be a whole lot of extra 
>>> complexity that I can't imagine we'd ever support.
>> 
>> You mean, if `element_size` is passed then you get different guarantees?
> 
> No, sorry, I mean different guarantees for the different pointer operands.  
> In principle, we could allow you to say that the memcpy has to be done with 
> 4-byte accesses from the source and 2-byte accesses to the destination.  
> That's implementable but a lot of work.

Gotcha. Yeah I think it's useful as a niche thing, and if IR supports that for 
say `volatile` then we can honor lopsided `volatile` overloads (instead of 
treating the entire thing as `volatile`). I hadn't really thought about 
lopsided access sizes (since it fell out of `_Atomic`). Maybe it's useful? I 
was just banning unequal sizes before because it seemed like a mistake to copy 
to/from different types. If we wanted to support it, I suppose we could add 
another optional parameter, so I'm OK not doing it now, and adding later if 
useful.

Alright, I'll update the patch as discussed, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D79279#2170187 , @jfb wrote:

> In D79279#2170157 , @rjmccall wrote:
>
> > I think the argument is treated as if it were 1 if not given.  That's all 
> > that ordinary memcpy formally guarantees, which seems to work fine 
> > (semantically, if not performance-wise) for pretty much everything today.
>
>
> I'm not sure that's true: consider a `memcpy` implementation which copies 
> some bytes twice (at different access size, there's an overlap because 
> somehow it's more efficient). That would probably violate the programmer's 
> expectations, and I don't think `volatile` nor atomic `memcpy` allow this 
> (but regular `memcpy` does).


Yes, that's true, if you need an only-accessed-once guarantee, that's above and 
beyond just a minimum access size.  I agree that `volatile` would need to make 
this guarantee.

>> Do you think it'd be useful to have different guarantees for different 
>> operands?  I guess it could come up, but it'd be a whole lot of extra 
>> complexity that I can't imagine we'd ever support.
> 
> You mean, if `element_size` is passed then you get different guarantees?

No, sorry, I mean different guarantees for the different pointer operands.  In 
principle, we could allow you to say that the memcpy has to be done with 4-byte 
accesses from the source and 2-byte accesses to the destination.  That's 
implementable but a lot of work.

>> If one of the arguments is `volatile`, arguably the minimum access width (if 
>> given) needs to be exact.  If we don't support that right now, it's okay to 
>> make it an error, which is basically you've already done with the `_Atomic 
>> volatile` diagnostic.
> 
> Agreed. `volatile` with size makes a lot of sense, and the IR version of it, 
> once created, ought to not be able to widen accesses. `volatile` without a 
> size specified makes sense too, because you just want a single read and a 
> single write, don't care about tearing.

Right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279



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

[PATCH] D84364: [CUDA][HIP] Defer all diagnostics for host device functions

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

I'm going to try the patch on our CUDA code and see how it fares. Stay tuned.


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

https://reviews.llvm.org/D84364



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

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

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

Szelethus added a comment.

You could do it in the code, but if the modeling wouldn't be present from 
CStringModeling, CStringChecker wouldn't work properly. So you should make it a 
strong dependency, just as you did in this patch. My comment was mainly a 
response to @NoQ :)




Comment at: clang/lib/StaticAnalyzer/Checkers/CStringLength.cpp:175-181
+void ento::registerCStringModeling(CheckerManager ) {
+  Mgr.registerChecker();
+}
+
+bool ento::shouldRegisterCStringModeling(const CheckerManager &) {
+  return true;
+}

steakhal wrote:
> Szelethus wrote:
> > We traditionally put these on the bottom of the file -- I don't think this 
> > would upset the structure too much :)
> I wanted to place the class definition as close as possible to the 
> registration function.
> I can move this though.
Yeah, I see what you were going for, but I'd prefer to keep it down there still.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84316



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

LLVM buildmaster is back to normal work

[Galina Kistanova via cfe-commits]

Hello everyone,

LLVM buildmaster is back to normal work, sorry for inconvenience.
Last night updates caused some distortions unfortunately.

Thanks

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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D79279#2170157 , @rjmccall wrote:

> I think the argument is treated as if it were 1 if not given.  That's all 
> that ordinary memcpy formally guarantees, which seems to work fine 
> (semantically, if not performance-wise) for pretty much everything today.


I'm not sure that's true: consider a `memcpy` implementation which copies some 
bytes twice (at different access size, there's an overlap because somehow it's 
more efficient). That would probably violate the programmer's expectations, and 
I don't think `volatile` nor atomic `memcpy` allow this (but regular `memcpy` 
does).

> I don't think you need any restrictions on element size.  It's probably 
> sensible to require the pointers to be dynamically aligned to a multiple of 
> the access width, but I don't think you can enforce that statically.

Agreed, if we're given a short and told to copy 4 bytes at a time then UBSan 
could find the constraint violation on alignment, but generally the only way we 
can diagnose is if the parameter is `__unaligned` (because there you're 
explicitly telling me it's not aligned, and the constraint is that it has to 
be).

> And of course the length needs to be a multiple of the access size.

Yeah.

> Do you think it'd be useful to have different guarantees for different 
> operands?  I guess it could come up, but it'd be a whole lot of extra 
> complexity that I can't imagine we'd ever support.

You mean, if `element_size` is passed then you get different guarantees? I 
think that's what makes sense: if you're asking for atomic `memcpy` then you 
get guarantees. If you're asking for `volatile` `mempcy` then you get others. 
That's why overloading (and multiple parameters) can be confusing, but at the 
same time I think it's better than having the combinatorial number of named 
functions instead.

> If one of the arguments is `volatile`, arguably the minimum access width (if 
> given) needs to be exact.  If we don't support that right now, it's okay to 
> make it an error, which is basically you've already done with the `_Atomic 
> volatile` diagnostic.

Agreed. `volatile` with size makes a lot of sense, and the IR version of it, 
once created, ought to not be able to widen accesses. `volatile` without a size 
specified makes sense too, because you just want a single read and a single 
write, don't care about tearing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279



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

[PATCH] D84405: CodeGen: Improve generated IR for __builtin_mul_overflow(uint, uint, int)

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

In D84405#2170110 , @vsk wrote:

> How were you able to show that the specialized IRGen is equivalent to the 
> generic kind? I tried doing this with direct inspection 
> (https://godbolt.org/z/o5WEn3), but wasn't able to convince myself. In the 
> past I used a test driver to compare the before/after compiler output -- you 
> might find that useful 
> (https://gist.github.com/vedantk/3eb9c88f82e5c32f2e590555b4af5081).


I compared with what gcc was generating and it looked similar (although my x86 
assembly knowledge is not great), but I will give your test driver a try, 
thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84405



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

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

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

steakhal marked 4 inline comments as done.
steakhal added a comment.

In D84316#2169195 , @Szelethus wrote:

> [...] you really cant make CStringChecker work without CStringModeling


How should I fuse the `CStringModeling` and the `CStringChecker` together while 
splitting them up?

I mean, that would be the best if the `CStringChecker` would focus on modeling 
the related cstring functions while letting the `CStringModeling` do the 
bookkeeping.
I see some contradiction here.




Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:429
 
 def CStringModeling : Checker<"CStringModeling">,
+  HelpText<"Responsible for essential modeling of cstring lengths. "

Szelethus wrote:
> What other aspects of c strings needs to be modeled? Is it only length? If 
> so, how about we rename the checker to `CStringLengthModeling`?
For now I think the cstring length is enough.
I'm not sure if we will want to have other properties as well.
You are probably right.



Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:495
   ]>,
-  Dependencies<[CStringModeling]>,
+  Dependencies<[CStringChecker]>,
   Documentation,

Szelethus wrote:
> I dug around a bit, and found this commit as to why this was needed: 
> rGe56167e8f87acf87a9de3d383752e18a738cf056. So this dependency is appropriate.
Interesting.
I was just doing a search & replace though :)



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:74
 enum class ConcatFnKind { none = 0, strcat = 1, strlcat = 2 };
-class CStringChecker : public Checker< eval::Call,
- check::PreStmt,
- check::LiveSymbols,
- check::DeadSymbols,
- check::RegionChanges
- > {
+class CStringChecker : public Checker {
   mutable std::unique_ptr BT_Null, BT_Bounds, BT_Overlap,

Szelethus wrote:
> This is somewhat orthogonal to the patch, but shouldn't precondition 
> violations be reported at `preCall`?
That is the current behavior.
We should consider in the future using `preCall` if we refactor so relentlessly.



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringLength.cpp:175-181
+void ento::registerCStringModeling(CheckerManager ) {
+  Mgr.registerChecker();
+}
+
+bool ento::shouldRegisterCStringModeling(const CheckerManager &) {
+  return true;
+}

Szelethus wrote:
> We traditionally put these on the bottom of the file -- I don't think this 
> would upset the structure too much :)
I wanted to place the class definition as close as possible to the registration 
function.
I can move this though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84316



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I think the argument is treated as if it were 1 if not given.  That's all that 
ordinary memcpy formally guarantees, which seems to work fine (semantically, if 
not performance-wise) for pretty much everything today.  I don't think you need 
any restrictions on element size.  It's probably sensible to require the 
pointers to be dynamically aligned to a multiple of the access width, but I 
don't think you can enforce that statically.  And of course the length needs to 
be a multiple of the access size.

Do you think it'd be useful to have different guarantees for different 
operands?  I guess it could come up, but it'd be a whole lot of extra 
complexity that I can't imagine we'd ever support.

If one of the arguments is `volatile`, arguably the minimum access width (if 
given) needs to be exact.  If we don't support that right now, it's okay to 
make it an error, which is basically you've already done with the `_Atomic 
volatile` diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D79279#2170095 , @rjmccall wrote:

> I don't think any of these should allow _Atomic unless we're going to give it 
> some sort of consistent atomic semantics (which is hard to imagine being 
> useful), and I think you should just take an extra argument of the minimum 
> access width on all of them uniformly if you think that's important.  
> Builtins can have optional arguments.


OK so: `__builtin_memcpy_overloaded` which overloads on `volatile`, `restrict`, 
`__unaligned`, and address spaces, but not on `_Atomic` qualifiers. Optionally, 
a 4th integer parameter can be provided to represent `element_size`. If 
provided, this becomes an unordered atomic memcpy with element size equal to or 
greater than the provided `element_size`. That value must be a power of two, 
and must be lock-free (what we call maximum atomic inline width in target 
info). If provided, then `__unaligned` is invalid, and `volatile` ought to be 
valid but is currently unsupported because IR can't do atomic+`volatile` memcpy 
(it would be useful, say for shared memory, but Patches Welcome).

Do you think there should be any relationship at all between dst/src pointee 
type's size and `element_size`? i.e. if I copy `short*` using an element size 
of 1 byte, is that OK? It seems like larger element sizes is always OK, but 
smaller might be a programmer error? If that's what they wanted, they could 
have done `(char*)my_short`. Or is this trying to be too helpful?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279



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

[PATCH] D84405: CodeGen: Improve generated IR for __builtin_mul_overflow(uint, uint, int)

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

How were you able to show that the specialized IRGen is equivalent to the 
generic kind? I tried doing this with direct inspection 
(https://godbolt.org/z/o5WEn3), but wasn't able to convince myself. In the past 
I used a test driver to compare the before/after compiler output -- you might 
find that useful 
(https://gist.github.com/vedantk/3eb9c88f82e5c32f2e590555b4af5081).




Comment at: clang/test/CodeGen/builtins-overflow.c:123
+  // CHECK: br i1 [[C2]]
+  int r;
+  if (__builtin_mul_overflow(x, y, ))

Could you add a test for the volatile result case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84405



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

[PATCH] D84029: [clang][Driver] Default to /usr/bin/ld on Solaris

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Can you add a test to `test/Driver/solaris-ld.c`?

> However, if someone forgets this, it depends on the user's PATH whether or 
> not clang finds the correct linker, which doesn't make for a good user 
> experience.

Not very sure about this. The last resort of GetProgramPath is `PATH`. On 
FreeBSD and Linux, this is how `/usr/bin/ld` gets selected. PATH can affect 
`ld` choice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84029



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I don't think any of these should allow _Atomic unless we're going to give it 
some sort of consistent atomic semantics (which is hard to imagine being 
useful), and I think you should just take an extra argument of the minimum 
access width on all of them uniformly if you think that's important.  Builtins 
can have optional arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279



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

[PATCH] D75044: [AArch64] __builtin_return_address for PAuth.

[Momchil Velikov via Phabricator via cfe-commits]

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

LGTM. Thanks!


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

https://reviews.llvm.org/D75044



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

> My point is that this has nothing to do with the ordinary semantics of 
> `_Atomic`.  You're basically just looking at the word "atomic" and saying 
> that, hey, a minimum access size is sortof related to atomicity.
> 
> If you want this to be able to control the minimum access size, you should 
> allow that to be passed in as an optional argument instead.

OK so it sounds like you're suggesting *two* versions of the overloaded 
builtins:

1. `__builtin_memcpy_overloaded` which overloads on `volatile`, `restrict`, 
`__unaligned`, and address spaces, but **not** on `_Atomic` qualifiers.
2. `__builtin_atomic_memcpy_overloaded` which overloads on `volatile` (but 
unsupported for now), `restrict`, and address spaces, but **not** on `_Atomic` 
qualifiers (because it's implicit), and **not** on `__unaligned` because that's 
a constraint. This takes an extra "element size" parameter, which we hope folks 
don't confuse with the size parameter (I'd expect a template or macro wrapper 
to hide that extra parameter when actually using the builtin).

Of course, that's two versions for each of `memcpy`, `memmove`, `memset`, and 
any other `*mem` that we decide to add to this list of overloadable functions.

Is that correct?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279



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

[PATCH] D83553: [PATCH 3/4][Sema][AArch64] Add codegen for arm_sve_vector_bits attribute

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes marked an inline comment as done.
c-rhodes added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:152
+  Align, Name,
+  /*ArraySize=*/nullptr, Alloca);
 

efriedma wrote:
> Do we need to bitcast the result of CreateTempAlloca to a pointer to the 
> array type?  I'm concerned that we might miss a bitcast if the source code 
> uses the address of the variable.
> Do we need to bitcast the result of CreateTempAlloca to a pointer to the 
> array type? I'm concerned that we might miss a bitcast if the source code 
> uses the address of the variable.

You were right, I've spent some time investigating this. The current 
implementation crashes on:
```fixed_int32_t global;
fixed_int32_t address_of_global() {
  fixed_int32_t *global_ptr;
  global_ptr = 
  return *global_ptr;
}```

the reason being `global` is represented as an `ArrayType` whereas the pointer 
`global_ptr` is scalable:

```@global = global [4 x i32] zeroinitializer, align 16
%global_ptr = alloca *, align 8```

so when storing the address of `global` to `global_ptr` the store it tries to 
create causes a crash:

`store [4 x i32]* @global, ** %global_ptr, align 8`

I tried your suggestion to bitcast to alloca to the array type in 
`CreateMemTemp` but found for that example it isn't called, it's created by a 
call to `CreateTempAlloca` in CGDecl.cpp (`EmitAutoVarAlloca`). 
`CreateTempAlloca` takes an `llvm::Type *Ty` so it's not as straightforward as 
doing a bitcast there, although I found it could be done in `EmitAutoVarAlloca` 
but it means having to handle this is two places I'm aware of and potentially 
others I haven't hit. In this case as well it also required looking through the 
pointer to see if the pointee was a VLST then doing a bitcast.

I've also experimented with representing allocas as fixed-length arrays to see 
if that will make it any easier and it does simplify the patch a little. It 
does require handling `PointerType` in `ConvertTypeForMem` however as we do for 
`ConstantArray`, same issue I mentioned in response to your other comment about 
removing that.

I planning to update the patch with that implementation but I've just found 
another issue:

```fixed_int32_t arr[3];
fixed_int32_t *z() {
  fixed_int32_t *array_ptr;
  array_ptr = [0];
  return array_ptr;
}```

trying to create a store:
`store [4 x i32]* %0, ** %retval, align 8`

although this is done in CGStmt.cpp as it's for a retval so it looks like a 
bitcast could also be required there.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3985
+else
+  Init = EmitNullConstant(D->getType());
   } else {

efriedma wrote:
> EmitNullConstant should just do the right thing, I think, now that we've 
> changed the default behavior of ConvertTypeForMem.
> EmitNullConstant should just do the right thing, I think, now that we've 
> changed the default behavior of ConvertTypeForMem.

Good spot, these changes can be removed



Comment at: clang/lib/CodeGen/CodeGenTypes.cpp:151
+  return llvm::ArrayType::get(*MemTy, A->getSize().getZExtValue());
+  }
+

efriedma wrote:
> I think the default handling for constant arrays should do the right thing, 
> now that we've changed the default behavior of ConvertTypeForMem.
> I think the default handling for constant arrays should do the right thing, 
> now that we've changed the default behavior of ConvertTypeForMem.

`ConvertType` looks at the canonical type so the type attribute is lost.


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

https://reviews.llvm.org/D83553



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

[PATCH] D79279: Add overloaded versions of builtin mem* functions

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D79279#2169522 , @jfb wrote:

> In D79279#2168649 , @rjmccall wrote:
>
> > In D79279#2168533 , @jfb wrote:
> >
> > > In D79279#2168479 , @rjmccall 
> > > wrote:
> > >
> > > > Is there a need for an atomic memcpy at all?  Why is it useful to allow 
> > > > this operation to take on "atomic" semantics — which aren't actually 
> > > > atomic because the loads and stores to elements are torn — with 
> > > > hardcoded memory ordering and somewhat arbitrary rules about what the 
> > > > atomic size is?
> > >
> > >
> > > Hans lays out a rationale for usefulness in his paper, but what I've 
> > > implemented is more useful: it's *unordered* so you can fence as you 
> > > desire around it, yet it guarantees a minimum memory access size based on 
> > > the pointer parameters. For example, copying an atomic `int` will be 4 
> > > byte operations which are single-copy-atomic, but the accesses from one 
> > > `int` to the next aren't performed in any guaranteed order (or observable 
> > > in any guaranteed order either). I talked about this with him a while ago 
> > > but IIRC he wasn't sure about implementation among other things, so when 
> > > you asked me to widen my original `volatile`-only `memcpy` to also do 
> > > other qualifiers, I realized that it was a neat way to do atomic as well 
> > > as other qualifiers. I've talked to a few SG1 folks about this, and I 
> > > believe (for other reasons too) it's where the design will end up for 
> > > Hans' paper.
> >
> >
> > I can see the usefulness of this operation, but it seems like a odd 
> > semantic mismatch for what is basically just a memcpy where one of the 
> > pointers happens to have `_Atomic` type, like you're shoe-horning it into 
> > this builtin just to avoid declaring a different one.
>
>
> I'm following the discussion we had here regarding overloading 
> :
>
> >> There are other qualifiers that can meaningfully contribute to the 
> >> operation here besides volatile, such as restrict and (more importantly) 
> >> address spaces. And again, for the copy operations these might differ 
> >> between the two pointer types.
> >> 
> >> In both cases, I’d say that the logical design is to allow the pointers to 
> >> be to arbitrarily-qualified types. We can then propagate that information 
> >> from the builtin into the LLVM intrinsic call as best as we’re allowed. So 
> >> I think you should make builtins called something like 
> >> __builtin_overloaded_memcpy (name to be decided) and just have their 
> >> semantics be type-directed.
> > 
> > Ah yes, I’d like to hear what others think of this. I hadn’t thought about 
> > it before you brought it up, and it sounds like a good idea.
>
> As you noted earlier, for `memcpy` you probably want to express differences 
> in destination and source qualification, even if today IR can't express e.g. 
> `volatile` source and non-`volatile` destination. You were talking about 
> `volatile`, but this applies to the entire combination of dst+src qualified 
> with zero-to-five `volatile`, `_Atomic`, `__unaligned`, `restrict`, and 
> address space. Pulling the entire combination space out into different 
> functions would create way too many functions. Right now the implementation 
> has a few limitations: it treats both dst and src as `volatile` if either 
> are, it can't do `_Atomic` with `volatile` so we diagnose, and it ignores 
> `restrict`.  Otherwise it supports all combinations.


My point is that this has nothing to do with the ordinary semantics of 
`_Atomic`.  You're basically just looking at the word "atomic" and saying that, 
hey, a minimum access size is sortof related to atomicity.

If you want this to be able to control the minimum access size, you should 
allow that to be passed in as an optional argument instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79279



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

[PATCH] D84310: [libTooling] Add assorted `EditGenerator` combinators.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:125
+/// not bound, then no edits are produced.
+inline EditGenerator ifBound(std::string ID, ASTEdit TrueEdit) {
+  return ifBound(std::move(ID), edit(std::move(TrueEdit)), noEdits());

asoffer wrote:
> I don't know about LLVM style preferences here, but a default argument to the 
> example above seems reasonable too.
I'd like that, but don't see how to get the types to work out, "noEdits" is a 
generator, but FalseEdit branch is an `ASTEdit`. did you have something in mind?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84310



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

[PATCH] D79744: clang: Use byref for aggregate kernel arguments

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D79744#2165929 , @rjmccall wrote:

> Arguably we should add this attribute to all indirect arguments.  I can 
> understand not wanting to update all the test cases, but you could probably 
> avoid adding a new IndirectByRef kind of ABIArgInfo by treating kernels 
> specially in ConstructAttributeList.
>
> Or, sorry, I forget — is this semantically necessary because the argument is 
> to constant memory and the callee has to copy it to form the mutable local?  
> If so, I think (1) the above statement about theoretically using `byref` on 
> all arguments still applies and (2) we do need a new ABIArgInfo kind, but we 
> should name it something like IndirectAliased.


Yes, it's semantically needed to insert the copy from constant memory. I 
originally interpreted a copy as necessary if the indirect addrspace did not 
match the stack address space, which is a sort of roundabout way of achieving 
the same thing


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

https://reviews.llvm.org/D79744



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

[PATCH] D84310: [libTooling] Add assorted `EditGenerator` combinators.

[Andy Soffer via Phabricator via cfe-commits]

asoffer accepted this revision.
asoffer added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/include/clang/Tooling/Transformer/RewriteRule.h:125
+/// not bound, then no edits are produced.
+inline EditGenerator ifBound(std::string ID, ASTEdit TrueEdit) {
+  return ifBound(std::move(ID), edit(std::move(TrueEdit)), noEdits());

I don't know about LLVM style preferences here, but a default argument to the 
example above seems reasonable too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84310



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

[PATCH] D83997: [os_log] Improve the way we extend the lifetime of objects passed to __builtin_os_log_format

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

The use case for this is a macro in which the call to `__builtin_os_log_format` 
that writes to the buffer and the call that uses the buffer appear in two 
different statements. For example:

  __builtin_os_log_format(buf, "%@", getObj());
  ...
  use_buffer(buf);

The object returned by the call to `getObj` has to be kept alive until 
`use_buffer` is called, but currently it gets destructed at the end of the full 
expression. I think an alternate solution would be to provide users a means to 
tell ARC optimizer not to move the release call for a local variable past any 
calls, i.e., something that is stricter than `NS_VALID_UNTIL_END_OF_SCOPE`, but 
that places more burden on the users.

In the `os_log` macro, the result of the call to `__builtin_os_log_format` is 
passed directly to the call that uses the buffer, so it doesn't require any 
lifetime extension as you pointed out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83997



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

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

[Joel E. Denny via Phabricator via cfe-commits]

jdenny created this revision.
jdenny added reviewers: grokos, ABataev, jdoerfert.
Herald added subscribers: llvm-commits, openmp-commits, cfe-commits, sstefan1, 
guansong, yaxunl.
Herald added projects: clang, OpenMP, LLVM.

Without this patch, the following example fails but shouldn't
according to my read of OpenMP TR8:

  #pragma omp target enter data map(alloc:i)
  #pragma omp target data map(present, alloc: i)
  {
#pragma omp target exit data map(delete:i)
  } // fails presence check here

OpenMP TR8 sec. 2.22.7.1 "map Clause", p. 321, L23-26 states:

> If the map clause appears on a target, target data, target enter
>  data or target exit data construct with a present map-type-modifier
>  then on entry to the region if the corresponding list item does not 
>  appear in the device data environment an error occurs and the 
>  program terminates.

I see no corresponding statement about the exit from a region.  Thus,
the `present` modifier should:

1. Check for presence upon entry into a `target exit data` construct.
2. Should not check for presence upon exit from a `target data` region, as in 
the above example.

The problem is that Clang calls the same set of
`__tgt_target_data_end_*` functions for these two cases, making them
indistinguishable in the runtime where the presence check is
implemented.  To fix that, this patch changes Clang to generate calls
to a new set of runtime functions, `__tgt_target_exit_data_*`, for the 
case of `target exit data`.

For symmetry, this patch makes a similar change for `target enter
data`, but that change isn't required for the above fix.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84422

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/declare_mapper_codegen.cpp
  clang/test/OpenMP/target_enter_data_codegen.cpp
  clang/test/OpenMP/target_enter_data_depend_codegen.cpp
  clang/test/OpenMP/target_exit_data_codegen.cpp
  clang/test/OpenMP/target_exit_data_depend_codegen.cpp
  clang/test/OpenMP/target_map_member_expr_array_section_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  openmp/libomptarget/include/omptarget.h
  openmp/libomptarget/src/exports
  openmp/libomptarget/src/interface.cpp
  openmp/libomptarget/src/omptarget.cpp
  openmp/libomptarget/src/private.h
  openmp/libomptarget/test/mapping/present/target_data_at_exit.c

Index: openmp/libomptarget/test/mapping/present/target_data_at_exit.c
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/present/target_data_at_exit.c
@@ -0,0 +1,37 @@
+// RUN: %libomptarget-compile-aarch64-unknown-linux-gnu -fopenmp-version=51
+// RUN: %libomptarget-run-aarch64-unknown-linux-gnu 2>&1 \
+// RUN: | %fcheck-aarch64-unknown-linux-gnu
+
+// RUN: %libomptarget-compile-powerpc64-ibm-linux-gnu -fopenmp-version=51
+// RUN: %libomptarget-run-powerpc64-ibm-linux-gnu 2>&1 \
+// RUN: | %fcheck-powerpc64-ibm-linux-gnu
+
+// RUN: %libomptarget-compile-powerpc64le-ibm-linux-gnu -fopenmp-version=51
+// RUN: %libomptarget-run-powerpc64le-ibm-linux-gnu 2>&1 \
+// RUN: | %fcheck-powerpc64le-ibm-linux-gnu
+
+// RUN: %libomptarget-compile-x86_64-pc-linux-gnu -fopenmp-version=51
+// RUN: %libomptarget-run-x86_64-pc-linux-gnu 2>&1 \
+// RUN: | %fcheck-x86_64-pc-linux-gnu
+
+#include 
+
+int main() {
+  int i;
+
+#pragma omp target enter data map(alloc:i)
+
+  // i isn't present at the end of the target data region, but the "present"
+  // modifier is only checked at the beginning of a region.
+#pragma omp target data map(present, alloc: i)
+  {
+#pragma omp target exit data map(delete:i)
+  }
+
+  // CHECK-NOT: Libomptarget
+  // CHECK: success
+  // CHECK-NOT: Libomptarget
+  fprintf(stderr, "success\n");
+
+  return 0;
+}
Index: openmp/libomptarget/src/private.h
===
--- openmp/libomptarget/src/private.h
+++ openmp/libomptarget/src/private.h
@@ -24,8 +24,8 @@
 
 extern int target_data_end(DeviceTy , int32_t arg_num, void **args_base,
void **args, int64_t *arg_sizes, int64_t *arg_types,
-   void **arg_mappers,
-   __tgt_async_info *async_info_ptr);
+   void **arg_mappers, __tgt_async_info *async_info_ptr,
+   bool for_exit_data);
 
 extern int target_data_update(DeviceTy , int32_t arg_num,
   void **args_base, void **args,
Index: openmp/libomptarget/src/omptarget.cpp
===
--- openmp/libomptarget/src/omptarget.cpp
+++ openmp/libomptarget/src/omptarget.cpp
@@ -421,10 +421,30 @@
   return OFFLOAD_SUCCESS;
 }
 
+static int target_data_end_not_for_exit_data(DeviceTy , int32_t arg_num,
+ void **args_base, void **args,
+ int64_t *arg_sizes,
+

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

[Ana Pazos via Phabricator via cfe-commits]

apazos added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:95
+  .addReg(RISCV::X18)
+  .addImm(0);
+}

There are thee things to observe here and other reviewers might have some 
additional comments:

- RISC-V does not have a reserved platform register like AAch64. The patch uses 
one of the RISC-V callee saved registers, x18, which happens to coincide with 
AArch64's register. It is possible to select another register, and additional 
checks for the flag combo "-fsanitize=shadow-call-stack -ffixed-" will have 
to be added.

- The  return address is saved on both the SCS (whose location is 
protected/hidden) and also in the regular stack. But the return from a function 
uses the value saved on SCS. The understanding is that not saving it in the 
regular stack can impact debugging.

- The SCS is ascending, while the regular stack, by RISC-V convention, is 
descending. The SCS is not used for passing parameters between calls like the 
regular stack, so it seems to be ok. But this can be changed too. AArch64 's 
SCS is also ascending.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84414



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

[PATCH] D83494: [libFuzzer] Link libFuzzer's own interceptors when other compiler runtimes are not linked.

[Dokyung Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
dokyungs marked an inline comment as done.
Closed by commit rG831ae45e3dc6: Recommit [libFuzzer] Link 
libFuzzers own interceptors when other compiler… (authored by dokyungs).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83494

Files:
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  compiler-rt/lib/fuzzer/CMakeLists.txt
  compiler-rt/lib/fuzzer/FuzzerInterceptors.cpp
  compiler-rt/test/fuzzer/CustomAllocator.cpp
  compiler-rt/test/fuzzer/custom-allocator.test
  compiler-rt/test/fuzzer/memcmp.test
  compiler-rt/test/fuzzer/memcmp64.test
  compiler-rt/test/fuzzer/strcmp.test
  compiler-rt/test/fuzzer/strncmp.test
  compiler-rt/test/fuzzer/strstr.test

Index: compiler-rt/test/fuzzer/strstr.test
===
--- compiler-rt/test/fuzzer/strstr.test
+++ compiler-rt/test/fuzzer/strstr.test
@@ -1,5 +1,11 @@
 UNSUPPORTED: freebsd
 RUN: %cpp_compiler %S/StrstrTest.cpp -o %t-StrstrTest
 RUN: not %run %t-StrstrTest   -seed=1 -runs=200   2>&1 | FileCheck %s
-CHECK: BINGO
 
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-strstr %S/StrstrTest.cpp -o %t-NoAsanStrstrTest
+RUN: not %run %t-NoAsanStrstrTest -seed=1 -runs=200   2>&1 | FileCheck %s
+
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc -fno-builtin-strstr %S/CustomAllocator.cpp %S/StrstrTest.cpp -o %t-NoAsanCustomAllocatorStrstrTest
+RUN: not %run %t-NoAsanCustomAllocatorStrstrTest -seed=1 -runs=200   2>&1 | FileCheck %s
+
+CHECK: BINGO
Index: compiler-rt/test/fuzzer/strncmp.test
===
--- compiler-rt/test/fuzzer/strncmp.test
+++ compiler-rt/test/fuzzer/strncmp.test
@@ -1,5 +1,11 @@
 UNSUPPORTED: freebsd
 RUN: %cpp_compiler %S/StrncmpTest.cpp -o %t-StrncmpTest
 RUN: not %run %t-StrncmpTest  -seed=2 -runs=1000   2>&1 | FileCheck %s
-CHECK: BINGO
 
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-strncmp %S/StrncmpTest.cpp -o %t-NoAsanStrncmpTest
+RUN: not %run %t-NoAsanStrncmpTest-seed=2 -runs=1000   2>&1 | FileCheck %s
+
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc -fno-builtin-strncmp %S/CustomAllocator.cpp %S/StrncmpTest.cpp -o %t-NoAsanCustomAllocatorStrncmpTest
+RUN: not %run %t-NoAsanCustomAllocatorStrncmpTest -seed=2 -runs=1000   2>&1 | FileCheck %s
+
+CHECK: BINGO
Index: compiler-rt/test/fuzzer/strcmp.test
===
--- compiler-rt/test/fuzzer/strcmp.test
+++ compiler-rt/test/fuzzer/strcmp.test
@@ -1,5 +1,11 @@
 UNSUPPORTED: freebsd
 RUN: %cpp_compiler %S/StrcmpTest.cpp -o %t-StrcmpTest
 RUN: not %run %t-StrcmpTest   -seed=1 -runs=200   2>&1 | FileCheck %s
-CHECK: BINGO
 
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-strcmp %S/StrcmpTest.cpp -o %t-NoAsanStrcmpTest
+RUN: not %run %t-NoAsanStrcmpTest -seed=1 -runs=200   2>&1 | FileCheck %s
+
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc -fno-builtin-strcmp %S/CustomAllocator.cpp %S/StrcmpTest.cpp -o %t-NoAsanCustomAllocatorStrcmpTest
+RUN: not %run %t-NoAsanCustomAllocatorStrcmpTest -seed=1 -runs=200   2>&1 | FileCheck %s
+
+CHECK: BINGO
Index: compiler-rt/test/fuzzer/memcmp64.test
===
--- compiler-rt/test/fuzzer/memcmp64.test
+++ compiler-rt/test/fuzzer/memcmp64.test
@@ -1,4 +1,8 @@
 UNSUPPORTED: freebsd
 RUN: %cpp_compiler %S/Memcmp64BytesTest.cpp -o %t-Memcmp64BytesTest
 RUN: not %run %t-Memcmp64BytesTest-seed=1 -runs=100   2>&1 | FileCheck %s
+
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-memcmp %S/Memcmp64BytesTest.cpp -o %t-NoAsanMemcmp64BytesTest
+RUN: not %run %t-NoAsanMemcmp64BytesTest  -seed=1 -runs=100   2>&1 | FileCheck %s
+
 CHECK: BINGO
Index: compiler-rt/test/fuzzer/memcmp.test
===
--- compiler-rt/test/fuzzer/memcmp.test
+++ compiler-rt/test/fuzzer/memcmp.test
@@ -1,4 +1,11 @@
 UNSUPPORTED: freebsd
 RUN: %cpp_compiler %S/MemcmpTest.cpp -o %t-MemcmpTest
 RUN: not %run %t-MemcmpTest   -seed=1 -runs=1000   2>&1 | FileCheck %s
+
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-memcmp %S/MemcmpTest.cpp -o %t-NoAsanMemcmpTest
+RUN: not %run %t-NoAsanMemcmpTest -seed=1 -runs=1000   2>&1 | FileCheck %s
+
+RUN: %cpp_compiler -fno-sanitize=address -fno-builtin-calloc -fno-builtin-memcmp %S/CustomAllocator.cpp %S/MemcmpTest.cpp -o %t-NoAsanCustomAllocatorMemcmpTest
+RUN: not %run %t-NoAsanCustomAllocatorMemcmpTest -seed=1 -runs=1000   2>&1 | FileCheck %s
+
 CHECK: BINGO
Index: compiler-rt/test/fuzzer/custom-allocator.test