[PATCH] D78443: [Local] Update getOrEnforceKnownAlignment/getKnownAlignment to use Align/MaybeAlign.

[Craig Topper via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG68b2e507e4fd: [Local] Update 
getOrEnforceKnownAlignment/getKnownAlignment to use… (authored by craig.topper).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78443

Files:
  clang/lib/CodeGen/CGCall.cpp
  llvm/include/llvm/Transforms/Utils/Local.h
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/lib/Transforms/Utils/Local.cpp
  llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

Index: llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
===
--- llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
+++ llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
@@ -1015,11 +1015,11 @@
  vectorizeStoreChain(Chains.second, InstructionsProcessed);
 }
 
-unsigned NewAlign = getOrEnforceKnownAlignment(S0->getPointerOperand(),
-   StackAdjustedAlignment,
-   DL, S0, nullptr, );
-if (NewAlign >= Alignment.value())
-  Alignment = Align(NewAlign);
+Align NewAlign = getOrEnforceKnownAlignment(S0->getPointerOperand(),
+Align(StackAdjustedAlignment),
+DL, S0, nullptr, );
+if (NewAlign >= Alignment)
+  Alignment = NewAlign;
 else
   return false;
   }
@@ -1160,10 +1160,11 @@
  vectorizeLoadChain(Chains.second, InstructionsProcessed);
 }
 
-unsigned NewAlign = getOrEnforceKnownAlignment(
-  L0->getPointerOperand(), StackAdjustedAlignment, DL, L0, nullptr, );
-if (NewAlign >= Alignment.value())
-  Alignment = Align(NewAlign);
+Align NewAlign = getOrEnforceKnownAlignment(L0->getPointerOperand(),
+Align(StackAdjustedAlignment),
+DL, L0, nullptr, );
+if (NewAlign >= Alignment)
+  Alignment = NewAlign;
 else
   return false;
   }
Index: llvm/lib/Transforms/Utils/Local.cpp
===
--- llvm/lib/Transforms/Utils/Local.cpp
+++ llvm/lib/Transforms/Utils/Local.cpp
@@ -1156,9 +1156,8 @@
 /// often possible though. If alignment is important, a more reliable approach
 /// is to simply align all global variables and allocation instructions to
 /// their preferred alignment from the beginning.
-static unsigned enforceKnownAlignment(Value *V, unsigned Alignment,
-  unsigned PrefAlign,
-  const DataLayout ) {
+static Align enforceKnownAlignment(Value *V, Align Alignment, Align PrefAlign,
+   const DataLayout ) {
   assert(PrefAlign > Alignment);
 
   V = V->stripPointerCasts();
@@ -1170,21 +1169,21 @@
 // stripPointerCasts recurses through infinite layers of bitcasts,
 // while computeKnownBits is not allowed to traverse more than 6
 // levels.
-Alignment = std::max(AI->getAlignment(), Alignment);
+Alignment = max(AI->getAlign(), Alignment);
 if (PrefAlign <= Alignment)
   return Alignment;
 
 // If the preferred alignment is greater than the natural stack alignment
 // then don't round up. This avoids dynamic stack realignment.
-if (DL.exceedsNaturalStackAlignment(Align(PrefAlign)))
+if (DL.exceedsNaturalStackAlignment(PrefAlign))
   return Alignment;
-AI->setAlignment(Align(PrefAlign));
+AI->setAlignment(PrefAlign);
 return PrefAlign;
   }
 
   if (auto *GO = dyn_cast(V)) {
 // TODO: as above, this shouldn't be necessary.
-Alignment = std::max(GO->getAlignment(), Alignment);
+Alignment = max(GO->getAlign(), Alignment);
 if (PrefAlign <= Alignment)
   return Alignment;
 
@@ -1195,18 +1194,18 @@
 if (!GO->canIncreaseAlignment())
   return Alignment;
 
-GO->setAlignment(Align(PrefAlign));
+GO->setAlignment(PrefAlign);
 return PrefAlign;
   }
 
   return Alignment;
 }
 
-unsigned llvm::getOrEnforceKnownAlignment(Value *V, unsigned PrefAlign,
-  const DataLayout ,
-  const Instruction *CxtI,
-  AssumptionCache *AC,
-  const DominatorTree *DT) {
+Align llvm::getOrEnforceKnownAlignment(Value *V, MaybeAlign PrefAlign,
+   const DataLayout ,
+   const Instruction *CxtI,
+   AssumptionCache *AC,
+   

[PATCH] D78534: [libclang] Install both libclang.a and libclang.so when LIBCLANG_BUILD_STATIC=ON

[Han Zhu via Phabricator via cfe-commits]

zhuhan0 created this revision.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78534

Files:
  clang/cmake/modules/AddClang.cmake


Index: clang/cmake/modules/AddClang.cmake
===
--- clang/cmake/modules/AddClang.cmake
+++ clang/cmake/modules/AddClang.cmake
@@ -99,38 +99,40 @@
   endif()
   llvm_add_library(${name} ${LIBTYPE} ${ARG_UNPARSED_ARGUMENTS} ${srcs})
 
-  if(TARGET ${name})
-target_link_libraries(${name} INTERFACE ${LLVM_COMMON_LIBS})
+  foreach(lib ${name} ${name}_static)
+if(TARGET ${lib})
+  target_link_libraries(${lib} INTERFACE ${LLVM_COMMON_LIBS})
 
-if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ARG_INSTALL_WITH_TOOLCHAIN)
-  set(export_to_clangtargets)
-  if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
-  "clang-libraries" IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
-  NOT LLVM_DISTRIBUTION_COMPONENTS)
-set(export_to_clangtargets EXPORT ClangTargets)
-set_property(GLOBAL PROPERTY CLANG_HAS_EXPORTS True)
-  endif()
+  if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ARG_INSTALL_WITH_TOOLCHAIN)
+set(export_to_clangtargets)
+if(${lib} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
+"clang-libraries" IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
+NOT LLVM_DISTRIBUTION_COMPONENTS)
+  set(export_to_clangtargets EXPORT ClangTargets)
+  set_property(GLOBAL PROPERTY CLANG_HAS_EXPORTS True)
+endif()
 
-  install(TARGETS ${name}
-COMPONENT ${name}
-${export_to_clangtargets}
-LIBRARY DESTINATION lib${LLVM_LIBDIR_SUFFIX}
-ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX}
-RUNTIME DESTINATION bin)
+install(TARGETS ${lib}
+  COMPONENT ${lib}
+  ${export_to_clangtargets}
+  LIBRARY DESTINATION lib${LLVM_LIBDIR_SUFFIX}
+  ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX}
+  RUNTIME DESTINATION bin)
 
-  if (NOT LLVM_ENABLE_IDE)
-add_llvm_install_targets(install-${name}
- DEPENDS ${name}
- COMPONENT ${name})
-  endif()
+if (NOT LLVM_ENABLE_IDE)
+  add_llvm_install_targets(install-${lib}
+  DEPENDS ${lib}
+  COMPONENT ${lib})
+endif()
 
-  set_property(GLOBAL APPEND PROPERTY CLANG_LIBS ${name})
+set_property(GLOBAL APPEND PROPERTY CLANG_LIBS ${lib})
+  endif()
+  set_property(GLOBAL APPEND PROPERTY CLANG_EXPORTS ${lib})
+else()
+  # Add empty "phony" target
+  add_custom_target(${lib})
 endif()
-set_property(GLOBAL APPEND PROPERTY CLANG_EXPORTS ${name})
-  else()
-# Add empty "phony" target
-add_custom_target(${name})
-  endif()
+  endforeach()
 
   set_target_properties(${name} PROPERTIES FOLDER "Clang libraries")
   set_clang_windows_version_resource_properties(${name})


Index: clang/cmake/modules/AddClang.cmake
===
--- clang/cmake/modules/AddClang.cmake
+++ clang/cmake/modules/AddClang.cmake
@@ -99,38 +99,40 @@
   endif()
   llvm_add_library(${name} ${LIBTYPE} ${ARG_UNPARSED_ARGUMENTS} ${srcs})
 
-  if(TARGET ${name})
-target_link_libraries(${name} INTERFACE ${LLVM_COMMON_LIBS})
+  foreach(lib ${name} ${name}_static)
+if(TARGET ${lib})
+  target_link_libraries(${lib} INTERFACE ${LLVM_COMMON_LIBS})
 
-if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ARG_INSTALL_WITH_TOOLCHAIN)
-  set(export_to_clangtargets)
-  if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
-  "clang-libraries" IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
-  NOT LLVM_DISTRIBUTION_COMPONENTS)
-set(export_to_clangtargets EXPORT ClangTargets)
-set_property(GLOBAL PROPERTY CLANG_HAS_EXPORTS True)
-  endif()
+  if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ARG_INSTALL_WITH_TOOLCHAIN)
+set(export_to_clangtargets)
+if(${lib} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
+"clang-libraries" IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
+NOT LLVM_DISTRIBUTION_COMPONENTS)
+  set(export_to_clangtargets EXPORT ClangTargets)
+  set_property(GLOBAL PROPERTY CLANG_HAS_EXPORTS True)
+endif()
 
-  install(TARGETS ${name}
-COMPONENT ${name}
-${export_to_clangtargets}
-LIBRARY DESTINATION lib${LLVM_LIBDIR_SUFFIX}
-ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX}
-RUNTIME DESTINATION bin)
+install(TARGETS ${lib}
+  COMPONENT ${lib}
+  ${export_to_clangtargets}
+  LIBRARY DESTINATION lib${LLVM_LIBDIR_SUFFIX}
+  ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX}
+  RUNTIME DESTINATION bin)
 
-  if (NOT LLVM_ENABLE_IDE)
-add_llvm_install_targets(install-${name}
-   

[clang] 68b2e50 - [Local] Update getOrEnforceKnownAlignment/getKnownAlignment to use Align/MaybeAlign.

[Craig Topper via cfe-commits]

Author: Craig Topper
Date: 2020-04-20T21:31:44-07:00
New Revision: 68b2e507e4fdf2776e568e95d27ce1ff54097476

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

LOG: [Local] Update getOrEnforceKnownAlignment/getKnownAlignment to use 
Align/MaybeAlign.

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

Added: 


Modified: 
clang/lib/CodeGen/CGCall.cpp
llvm/include/llvm/Transforms/Utils/Local.h
llvm/lib/CodeGen/CodeGenPrepare.cpp
llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
llvm/lib/Transforms/Utils/InlineFunction.cpp
llvm/lib/Transforms/Utils/Local.cpp
llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 3c44632dfd60..b3fda6bd4ea3 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -4177,8 +4177,8 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo 
,
 bool NeedCopy = false;
 
 if (Addr.getAlignment() < Align &&
-llvm::getOrEnforceKnownAlignment(V, Align.getQuantity(), *TD) <
-Align.getQuantity()) {
+llvm::getOrEnforceKnownAlignment(V, Align.getAsAlign(), *TD) <
+Align.getAsAlign()) {
   NeedCopy = true;
 } else if (I->hasLValue()) {
   auto LV = I->getKnownLValue();

diff  --git a/llvm/include/llvm/Transforms/Utils/Local.h 
b/llvm/include/llvm/Transforms/Utils/Local.h
index c8afb5c3f304..c193f9c95d16 100644
--- a/llvm/include/llvm/Transforms/Utils/Local.h
+++ b/llvm/include/llvm/Transforms/Utils/Local.h
@@ -267,18 +267,18 @@ AllocaInst *DemotePHIToStack(PHINode *P, Instruction 
*AllocaPoint = nullptr);
 /// so if alignment is important, a more reliable approach is to simply align
 /// all global variables and allocation instructions to their preferred
 /// alignment from the beginning.
-unsigned getOrEnforceKnownAlignment(Value *V, unsigned PrefAlign,
-const DataLayout ,
-const Instruction *CxtI = nullptr,
-AssumptionCache *AC = nullptr,
-const DominatorTree *DT = nullptr);
+Align getOrEnforceKnownAlignment(Value *V, MaybeAlign PrefAlign,
+ const DataLayout ,
+ const Instruction *CxtI = nullptr,
+ AssumptionCache *AC = nullptr,
+ const DominatorTree *DT = nullptr);
 
 /// Try to infer an alignment for the specified pointer.
-inline unsigned getKnownAlignment(Value *V, const DataLayout ,
-  const Instruction *CxtI = nullptr,
-  AssumptionCache *AC = nullptr,
-  const DominatorTree *DT = nullptr) {
-  return getOrEnforceKnownAlignment(V, 0, DL, CxtI, AC, DT);
+inline Align getKnownAlignment(Value *V, const DataLayout ,
+   const Instruction *CxtI = nullptr,
+   AssumptionCache *AC = nullptr,
+   const DominatorTree *DT = nullptr) {
+  return getOrEnforceKnownAlignment(V, MaybeAlign(), DL, CxtI, AC, DT);
 }
 
 /// Create a call that matches the invoke \p II in terms of arguments,

diff  --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp 
b/llvm/lib/CodeGen/CodeGenPrepare.cpp
index d6a216f9f12c..d11d74a4a079 100644
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
@@ -1939,12 +1939,14 @@ bool CodeGenPrepare::optimizeCallInst(CallInst *CI, 
bool ) {
 // If this is a memcpy (or similar) then we may be able to improve the
 // alignment
 if (MemIntrinsic *MI = dyn_cast(CI)) {
-  unsigned DestAlign = getKnownAlignment(MI->getDest(), *DL);
-  if (DestAlign > MI->getDestAlignment())
+  Align DestAlign = getKnownAlignment(MI->getDest(), *DL);
+  MaybeAlign MIDestAlign = MI->getDestAlign();
+  if (!MIDestAlign || DestAlign > *MIDestAlign)
 MI->setDestAlignment(DestAlign);
   if (MemTransferInst *MTI = dyn_cast(MI)) {
-unsigned SrcAlign = getKnownAlignment(MTI->getSource(), *DL);
-if (SrcAlign > MTI->getSourceAlignment())
+MaybeAlign MTISrcAlign = MTI->getSourceAlign();
+Align SrcAlign = getKnownAlignment(MTI->getSource(), *DL);
+if (!MTISrcAlign || SrcAlign > *MTISrcAlign)
   MTI->setSourceAlignment(SrcAlign);
   }
 }

diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp 
b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
index 

[clang] 4b03dd7 - PR45534: don't ignore unmodeled side-effects when constant-evaluating a call to __builtin_constant_p.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-04-20T21:23:35-07:00
New Revision: 4b03dd7b849e8f5068dc8d72c6eab724c22a2805

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

LOG: PR45534: don't ignore unmodeled side-effects when constant-evaluating a 
call to __builtin_constant_p.

Such side-effects should result in the call evaluating to 'false', even
if we can still determine what value the argument expression will
evaluate to.

Added: 


Modified: 
clang/lib/AST/ExprConstant.cpp
clang/test/SemaCXX/builtin-constant-p.cpp

Removed: 




diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 8bc7a1128e7a..ad61221a6a91 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -10438,7 +10438,7 @@ static bool EvaluateBuiltinConstantP(EvalInfo , 
const Expr *Arg) {
   ArgType->isAnyComplexType() || ArgType->isPointerType() ||
   ArgType->isNullPtrType()) {
 APValue V;
-if (!::EvaluateAsRValue(Info, Arg, V)) {
+if (!::EvaluateAsRValue(Info, Arg, V) || Info.EvalStatus.HasSideEffects) {
   Fold.keepDiagnostics();
   return false;
 }

diff  --git a/clang/test/SemaCXX/builtin-constant-p.cpp 
b/clang/test/SemaCXX/builtin-constant-p.cpp
index f70676d250e0..1b8cf455ef27 100644
--- a/clang/test/SemaCXX/builtin-constant-p.cpp
+++ b/clang/test/SemaCXX/builtin-constant-p.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++17 -verify %s
+// RUN: %clang_cc1 -std=c++20 -verify %s
 
 using intptr_t = __INTPTR_TYPE__;
 
@@ -135,3 +136,33 @@ static_assert(mutate6(true) == 10);
 // not being a pointer to the start of a string literal.
 namespace std { struct type_info; }
 static_assert(__builtin_constant_p((int)));
+
+void mutate_as_side_effect() {
+  int a;
+  static_assert(!__builtin_constant_p(((void)++a, 1)));
+}
+
+namespace dtor_side_effect {
+  struct A {
+constexpr A() {}
+~A();
+  };
+  static_assert(!__builtin_constant_p((A{}, 123)));
+}
+
+#if __cplusplus >= 202002L
+namespace constexpr_dtor {
+  struct A {
+int *p;
+constexpr ~A() { *p = 0; }
+  };
+  struct Q { int n; constexpr int *get() { return  } };
+  static_assert(!__builtin_constant_p((A{}, 123)));
+  // FIXME: We should probably accept this. GCC does.
+  // However, GCC appears to do so by running the destructors at the end of the
+  // enclosing full-expression, which seems broken; running them at the end of
+  // the evaluation of the __builtin_constant_p argument would be more
+  // defensible.
+  static_assert(!__builtin_constant_p((A{Q().get()}, 123)));
+}
+#endif



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

[clang] e128f71 - PR45535: Check for variables with non-trivial destruction when

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-04-20T21:23:35-07:00
New Revision: e128f710ea871bab5ed14b1944caa935ed61b003

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

LOG: PR45535: Check for variables with non-trivial destruction when
determining whether a statement expression has side-effects.

Added: 


Modified: 
clang/lib/AST/Expr.cpp
clang/test/CodeGenCXX/builtin-constant-p.cpp

Removed: 




diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index f108b49ceac1..bb27f40994dc 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -3270,6 +3270,26 @@ namespace {
 
 bool hasSideEffects() const { return HasSideEffects; }
 
+void VisitDecl(const Decl *D) {
+  if (!D)
+return;
+
+  // We assume the caller checks subexpressions (eg, the initializer, VLA
+  // bounds) for side-effects on our behalf.
+  if (auto *VD = dyn_cast(D)) {
+// Registering a destructor is a side-effect.
+if (IncludePossibleEffects && VD->isThisDeclarationADefinition() &&
+VD->needsDestruction(Context))
+  HasSideEffects = true;
+  }
+}
+
+void VisitDeclStmt(const DeclStmt *DS) {
+  for (auto *D : DS->decls())
+VisitDecl(D);
+  Inherited::VisitDeclStmt(DS);
+}
+
 void VisitExpr(const Expr *E) {
   if (!HasSideEffects &&
   E->HasSideEffects(Context, IncludePossibleEffects))

diff  --git a/clang/test/CodeGenCXX/builtin-constant-p.cpp 
b/clang/test/CodeGenCXX/builtin-constant-p.cpp
index 6d853e8a6828..866faa5ec976 100644
--- a/clang/test/CodeGenCXX/builtin-constant-p.cpp
+++ b/clang/test/CodeGenCXX/builtin-constant-p.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple=x86_64-linux-gnu -emit-llvm -o - %s
+// RUN: %clang_cc1 -triple=x86_64-linux-gnu -emit-llvm -o - %s | FileCheck %s
 
 // Don't crash if the argument to __builtin_constant_p isn't scalar.
 template 
@@ -22,3 +22,27 @@ class numeric {
 bool bcp() {
   return is_constant(numeric(1));
 }
+
+// PR45535
+struct with_dtor {
+  ~with_dtor();
+};
+// CHECK: define {{.*}}bcp_stmt_expr_1
+bool bcp_stmt_expr_1() {
+  // CHECK-NOT: call {{.*}}with_dtorD
+  return __builtin_constant_p(({with_dtor wd; 123;}));
+}
+
+int do_not_call();
+// CHECK: define {{.*}}bcp_stmt_expr_2
+bool bcp_stmt_expr_2(int n) {
+  // CHECK-NOT: call {{.*}}do_not_call
+  return __builtin_constant_p(({
+// This has a side-effect due to the VLA bound, so CodeGen should fold it
+// to false.
+typedef int arr[do_not_call()];
+n;
+  }));
+  // CHECK-NOT: }
+  // CHECK: ret i1 false
+}



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

[PATCH] D78443: [Local] Update getOrEnforceKnownAlignment/getKnownAlignment to use Align/MaybeAlign.

[Eli Friedman via Phabricator via cfe-commits]

efriedma accepted this revision.
efriedma 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/D78443/new/

https://reviews.llvm.org/D78443



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

[PATCH] D78443: [Local] Update getOrEnforceKnownAlignment/getKnownAlignment to use Align/MaybeAlign.

[Craig Topper via Phabricator via cfe-commits]

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

Use getAsAlign


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78443

Files:
  clang/lib/CodeGen/CGCall.cpp
  llvm/include/llvm/Transforms/Utils/Local.h
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/lib/Transforms/Utils/Local.cpp
  llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

Index: llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
===
--- llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
+++ llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
@@ -1015,11 +1015,11 @@
  vectorizeStoreChain(Chains.second, InstructionsProcessed);
 }
 
-unsigned NewAlign = getOrEnforceKnownAlignment(S0->getPointerOperand(),
-   StackAdjustedAlignment,
-   DL, S0, nullptr, );
-if (NewAlign >= Alignment.value())
-  Alignment = Align(NewAlign);
+Align NewAlign = getOrEnforceKnownAlignment(S0->getPointerOperand(),
+Align(StackAdjustedAlignment),
+DL, S0, nullptr, );
+if (NewAlign >= Alignment)
+  Alignment = NewAlign;
 else
   return false;
   }
@@ -1160,10 +1160,11 @@
  vectorizeLoadChain(Chains.second, InstructionsProcessed);
 }
 
-unsigned NewAlign = getOrEnforceKnownAlignment(
-  L0->getPointerOperand(), StackAdjustedAlignment, DL, L0, nullptr, );
-if (NewAlign >= Alignment.value())
-  Alignment = Align(NewAlign);
+Align NewAlign = getOrEnforceKnownAlignment(L0->getPointerOperand(),
+Align(StackAdjustedAlignment),
+DL, L0, nullptr, );
+if (NewAlign >= Alignment)
+  Alignment = NewAlign;
 else
   return false;
   }
Index: llvm/lib/Transforms/Utils/Local.cpp
===
--- llvm/lib/Transforms/Utils/Local.cpp
+++ llvm/lib/Transforms/Utils/Local.cpp
@@ -1156,9 +1156,8 @@
 /// often possible though. If alignment is important, a more reliable approach
 /// is to simply align all global variables and allocation instructions to
 /// their preferred alignment from the beginning.
-static unsigned enforceKnownAlignment(Value *V, unsigned Alignment,
-  unsigned PrefAlign,
-  const DataLayout ) {
+static Align enforceKnownAlignment(Value *V, Align Alignment, Align PrefAlign,
+   const DataLayout ) {
   assert(PrefAlign > Alignment);
 
   V = V->stripPointerCasts();
@@ -1170,21 +1169,21 @@
 // stripPointerCasts recurses through infinite layers of bitcasts,
 // while computeKnownBits is not allowed to traverse more than 6
 // levels.
-Alignment = std::max(AI->getAlignment(), Alignment);
+Alignment = max(AI->getAlign(), Alignment);
 if (PrefAlign <= Alignment)
   return Alignment;
 
 // If the preferred alignment is greater than the natural stack alignment
 // then don't round up. This avoids dynamic stack realignment.
-if (DL.exceedsNaturalStackAlignment(Align(PrefAlign)))
+if (DL.exceedsNaturalStackAlignment(PrefAlign))
   return Alignment;
-AI->setAlignment(Align(PrefAlign));
+AI->setAlignment(PrefAlign);
 return PrefAlign;
   }
 
   if (auto *GO = dyn_cast(V)) {
 // TODO: as above, this shouldn't be necessary.
-Alignment = std::max(GO->getAlignment(), Alignment);
+Alignment = max(GO->getAlign(), Alignment);
 if (PrefAlign <= Alignment)
   return Alignment;
 
@@ -1195,18 +1194,18 @@
 if (!GO->canIncreaseAlignment())
   return Alignment;
 
-GO->setAlignment(Align(PrefAlign));
+GO->setAlignment(PrefAlign);
 return PrefAlign;
   }
 
   return Alignment;
 }
 
-unsigned llvm::getOrEnforceKnownAlignment(Value *V, unsigned PrefAlign,
-  const DataLayout ,
-  const Instruction *CxtI,
-  AssumptionCache *AC,
-  const DominatorTree *DT) {
+Align llvm::getOrEnforceKnownAlignment(Value *V, MaybeAlign PrefAlign,
+   const DataLayout ,
+   const Instruction *CxtI,
+   AssumptionCache *AC,
+   const DominatorTree *DT) {
   assert(V->getType()->isPointerTy() &&
  

[PATCH] D78252: [AArch64] FMLA/FMLS patterns improvement.

[Pavel Iliin via Phabricator via cfe-commits]

ilinpv updated this revision to Diff 258865.
ilinpv added a comment.

Patterns corrected, vector_extract tests added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78252

Files:
  clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics-constrained.c
  llvm/lib/Target/AArch64/AArch64InstrFormats.td
  llvm/test/CodeGen/AArch64/fp16_intrinsic_lane.ll

Index: llvm/test/CodeGen/AArch64/fp16_intrinsic_lane.ll
===
--- llvm/test/CodeGen/AArch64/fp16_intrinsic_lane.ll
+++ llvm/test/CodeGen/AArch64/fp16_intrinsic_lane.ll
@@ -14,8 +14,7 @@
 ; CHECK-NEXT:.cfi_startproc
 ; CHECK-NEXT:  // %bb.0: // %entry
 ; CHECK-NEXT:// kill: def $d2 killed $d2 def $q2
-; CHECK-NEXT:dup v2.4h, v2.h[0]
-; CHECK-NEXT:fmla v0.4h, v2.4h, v1.4h
+; CHECK-NEXT:fmla v0.4h, v1.4h, v2.h[0]
 ; CHECK-NEXT:ret
 entry:
   %lane1 = shufflevector <4 x half> %c, <4 x half> undef, <4 x i32> zeroinitializer
@@ -29,8 +28,7 @@
 ; CHECK-NEXT:.cfi_startproc
 ; CHECK-NEXT:  // %bb.0: // %entry
 ; CHECK-NEXT:// kill: def $d2 killed $d2 def $q2
-; CHECK-NEXT:dup v2.8h, v2.h[0]
-; CHECK-NEXT:fmla v0.8h, v2.8h, v1.8h
+; CHECK-NEXT:fmla v0.8h, v1.8h, v2.h[0]
 ; CHECK-NEXT:ret
 entry:
   %lane1 = shufflevector <4 x half> %c, <4 x half> undef, <8 x i32> zeroinitializer
@@ -43,8 +41,7 @@
 ; CHECK:   .Lt_vfma_laneq_f16$local:
 ; CHECK-NEXT:.cfi_startproc
 ; CHECK-NEXT:  // %bb.0: // %entry
-; CHECK-NEXT:dup v2.4h, v2.h[0]
-; CHECK-NEXT:fmla v0.4h, v1.4h, v2.4h
+; CHECK-NEXT:fmla v0.4h, v1.4h, v2.h[0]
 ; CHECK-NEXT:ret
 entry:
   %lane1 = shufflevector <8 x half> %c, <8 x half> undef, <4 x i32> zeroinitializer
@@ -57,8 +54,7 @@
 ; CHECK:   .Lt_vfmaq_laneq_f16$local:
 ; CHECK-NEXT:.cfi_startproc
 ; CHECK-NEXT:  // %bb.0: // %entry
-; CHECK-NEXT:dup v2.8h, v2.h[0]
-; CHECK-NEXT:fmla v0.8h, v1.8h, v2.8h
+; CHECK-NEXT:fmla v0.8h, v1.8h, v2.h[0]
 ; CHECK-NEXT:ret
 entry:
   %lane1 = shufflevector <8 x half> %c, <8 x half> undef, <8 x i32> zeroinitializer
@@ -72,8 +68,7 @@
 ; CHECK-NEXT:.cfi_startproc
 ; CHECK-NEXT:  // %bb.0: // %entry
 ; CHECK-NEXT:// kill: def $h2 killed $h2 def $q2
-; CHECK-NEXT:dup v2.4h, v2.h[0]
-; CHECK-NEXT:fmla v0.4h, v2.4h, v1.4h
+; CHECK-NEXT:fmla v0.4h, v1.4h, v2.h[0]
 ; CHECK-NEXT:ret
 entry:
   %vecinit = insertelement <4 x half> undef, half %c, i32 0
@@ -88,8 +83,7 @@
 ; CHECK-NEXT:.cfi_startproc
 ; CHECK-NEXT:  // %bb.0: // %entry
 ; CHECK-NEXT:// kill: def $h2 killed $h2 def $q2
-; CHECK-NEXT:dup v2.8h, v2.h[0]
-; CHECK-NEXT:fmla v0.8h, v2.8h, v1.8h
+; CHECK-NEXT:fmla v0.8h, v1.8h, v2.h[0]
 ; CHECK-NEXT:ret
 entry:
   %vecinit = insertelement <8 x half> undef, half %c, i32 0
@@ -104,7 +98,7 @@
 ; CHECK-NEXT:.cfi_startproc
 ; CHECK-NEXT:  // %bb.0: // %entry
 ; CHECK-NEXT:// kill: def $d2 killed $d2 def $q2
-; CHECK-NEXT:fmadd h0, h1, h2, h0
+; CHECK-NEXT:fmla h0, h1, v2.h[0]
 ; CHECK-NEXT:ret
 entry:
   %extract = extractelement <4 x half> %c, i32 0
@@ -117,7 +111,7 @@
 ; CHECK:   .Lt_vfmah_laneq_f16$local:
 ; CHECK-NEXT:.cfi_startproc
 ; CHECK-NEXT:  // %bb.0: // %entry
-; CHECK-NEXT:fmadd h0, h1, h2, h0
+; CHECK-NEXT:fmla h0, h1, v2.h[0]
 ; CHECK-NEXT:ret
 entry:
   %extract = extractelement <8 x half> %c, i32 0
@@ -131,9 +125,7 @@
 ; CHECK-NEXT:.cfi_startproc
 ; CHECK-NEXT:  // %bb.0: // %entry
 ; CHECK-NEXT:// kill: def $d2 killed $d2 def $q2
-; CHECK-NEXT:fneg v1.4h, v1.4h
-; CHECK-NEXT:dup v2.4h, v2.h[0]
-; CHECK-NEXT:fmla v0.4h, v2.4h, v1.4h
+; CHECK-NEXT:fmls v0.4h, v1.4h, v2.h[0]
 ; CHECK-NEXT:ret
 entry:
   %sub = fsub <4 x half> , %b
@@ -148,9 +140,7 @@
 ; CHECK-NEXT:.cfi_startproc
 ; CHECK-NEXT:  // %bb.0: // %entry
 ; CHECK-NEXT:// kill: def $d2 killed $d2 def $q2
-; CHECK-NEXT:fneg v1.8h, v1.8h
-; CHECK-NEXT:dup v2.8h, v2.h[0]
-; CHECK-NEXT:fmla v0.8h, v2.8h, v1.8h
+; CHECK-NEXT:fmls v0.8h, v1.8h, v2.h[0]
 ; CHECK-NEXT:ret
 entry:
   %sub = fsub <8 x half> , %b
@@ -164,8 +154,7 @@
 ; CHECK:   .Lt_vfms_laneq_f16$local:
 ; CHECK-NEXT:.cfi_startproc
 ; CHECK-NEXT:  // %bb.0: // %entry
-; CHECK-NEXT:dup v2.4h, v2.h[0]
-; CHECK-NEXT:fmls v0.4h, v2.4h, v1.4h
+; CHECK-NEXT:fmls v0.4h, v1.4h, v2.h[0]
 ; CHECK-NEXT:ret
 entry:
   %sub = fsub <4 x half> , %b
@@ -179,8 +168,7 @@
 ; CHECK:   .Lt_vfmsq_laneq_f16$local:
 ; CHECK-NEXT:.cfi_startproc
 ; CHECK-NEXT:  // %bb.0: // %entry
-; CHECK-NEXT:dup v2.8h, v2.h[0]
-; CHECK-NEXT:fmls v0.8h, v2.8h, v1.8h
+; CHECK-NEXT:fmls v0.8h, v1.8h, v2.h[0]
 ; CHECK-NEXT:ret
 entry:
   %sub = fsub <8 x half> , %b
@@ -195,9 +183,7 @@
 ; CHECK-NEXT:.cfi_startproc
 ; CHECK-NEXT:  // %bb.0: // %entry
 ; CHECK-NEXT:// kill: def $h2 killed $h2 def $q2
-; CHECK-NEXT:fneg 

[PATCH] D77168: Add a flag to debug automatic variable initialization

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

We've rolled out `-ftrivial-auto-var-init=pattern` to all of Fuchsia several 
months ago. In my experience, having the `pragma` would have been really 
valuable during the rollout stage when we've often commented out portion of 
code while narrowing down the issue, which is basically a strawman version of 
the `pragma`. Similarly, the variable byte pattern would have also been a great 
timesaver.

I don't think we ever had a use case for the proposed flag, which doesn't mean 
that it's not useful, but I don't know how to use it effectively. Specifically, 
every time I was debugging an uninitialized variable issue, I'd know which file 
and often even which portion of the file the bug is in. In our build, we always 
apply flags to the entire target, not individual files, so to use the proposed 
flag, I'd have to modify the build to extract a single file into a separate 
target in order to apply the flag only to that file which seems comparatively 
intrusive to inserting the `pragma`. Is that something you could comment on? Do 
you have an existing experience of using this flag in your build? What's your 
strategy for using it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77168



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

[PATCH] D78443: [Local] Update getOrEnforceKnownAlignment/getKnownAlignment to use Align/MaybeAlign.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:4181
+llvm::getOrEnforceKnownAlignment(V,
+ llvm::Align(Align.getQuantity()),
+ *TD) < Align.getQuantity()) {

`Align.getAsAlign()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78443



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

[PATCH] D78521: [clangd] Extend dexp to support remote index

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev marked 5 inline comments as done.
kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:14
 
+#include "Index.grpc.pb.h"
 #include "SourceCode.h"

sammccall wrote:
> this include and the stuff depending on it needs to be ifdef'd
Ah, right, I forgot about this. Was present in the previous patch where I had 
custom definitions, but forgot about it for some reason.



Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:40
+RemoteMode("remote",
+   llvm::cl::desc("Connect to  remote index"));
+

@sammccall do you know why this opt is not being set for some reason? When I 
try to run `dexp --help` it is displayed in the possible arguments and when I 
try to pass anything invalid for boolean flags this also fails, but the value 
is not changed regardless of the values I put here :( Must be something simple 
I've missed.



Comment at: clang-tools-extra/clangd/index/remote/CMakeLists.txt:1
-generate_grpc_protos(RemoteIndexProtos "Index.proto")
+generate_grpc_protos(RemoteIndexProtos "Index.proto" RemoteProtosLocation)
 

sammccall wrote:
> why is this extra param needed?
Because this needs to be included both here and for `dexp` binary.



Comment at: clang-tools-extra/clangd/index/remote/Index.proto:16
+
+  rpc FuzzyFind(FuzzyFindRequest) returns (FuzzyFindReply) {}
+}

sammccall wrote:
> should also return a stream. has_more is annoying, but you can keep it in the 
> message and set it only on the last element of the stream.
Is this an idiomatic way of using gRPC + Protobuf? Seems counter-intuitive to 
me since repeated stream of symbols in the message + single `has_more` seems 
quite logical.



Comment at: clang-tools-extra/clangd/index/remote/Index.proto:37
+message FuzzyFindReply {
+  // FIXME(kirillbobyrev): Convert to Symbol.
+  repeated string symbols = 1;

sammccall wrote:
> confused... why not use Symbol now?
Couldn't put `Symbol`s into `FuzzyFindReply` for some reason. Clangd behaves 
really weird with Protobuf inclusions for me... Need to figure out why that 
happens, but might be me doing something wrong.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78521



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

[PATCH] D78491: Avoid relying on address space zero default parameter in llvm/IR

[Alexander Richardson via Phabricator via cfe-commits]

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

In D78491#1992618 , @aykevl wrote:

> I can't give an LGTM but at least from my perspective I very much welcome 
> this change. I am still hitting problems with non-zero address spaces on AVR. 
> One nit in an inline comment.


In case it helps, our changes can be seen here: 
https://github.com/CTSRD-CHERI/llvm-project. However, we made many of those 
changes before there was a program address space. For us all pointers are in 
AS200, so we initially had a getDefaultAS() helper and use that in cases where 
it should be program/globals address space. That is one of the things I intend 
to fix while upstreaming.

> Do you plan on eventually removing `LLVM_NO_IMPLICIT_ADDRESS_SPACE` when the 
> transition is finished? If so, I think it's worth documenting this somewhere 
> - perhaps at llvm/lib/IR/CMakeLists.txt.

Yes, ultimately the goal is to remove the default parameters. It might also 
make sense to switch the define from an opt-out 
`LLVM_NO_IMPLICIT_ADDRESS_SPACE` to and opt-in 
`LLVM_NEED_IMPLICIT_ADDRESS_SPACE_ZERO` for individual directories/projects 
that have not been ported to the explicit APIs yet.




Comment at: llvm/include/llvm/IR/DataLayout.h:356
   /// Layout pointer alignment
-  Align getPointerABIAlignment(unsigned AS) const;
 

aykevl wrote:
> There is no default address space in this declaration?
Good catch, I did not mean to change this line. Copy-paste error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78491



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

[PATCH] D78521: [clangd] Extend dexp to support remote index

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp:14
 
+#include "Index.grpc.pb.h"
 #include "SourceCode.h"

this include and the stuff depending on it needs to be ifdef'd



Comment at: clang-tools-extra/clangd/index/remote/CMakeLists.txt:1
-generate_grpc_protos(RemoteIndexProtos "Index.proto")
+generate_grpc_protos(RemoteIndexProtos "Index.proto" RemoteProtosLocation)
 

why is this extra param needed?



Comment at: clang-tools-extra/clangd/index/remote/CMakeLists.txt:9
+add_definitions(-DGOOGLE_PROTOBUF_NO_RTTI=1)
+add_clang_library(clangDaemonRemoteIndex
+  Index.cpp

let's avoid propagating the clangDaemon name any further. clangdRemoteIndex? or 
clangdRemoteIndexClient?



Comment at: clang-tools-extra/clangd/index/remote/Index.h:21
+
+class IndexClient : public SymbolIndex {
+public:

the class doesn't need to be exposed here, as the SymbolIndex interface seems 
to do everything we need. Just expose a factory?

Actually, I think that means we can expose this header whether grpc is enabled 
or not. If no grpc, then we just link in an implementation of the factory that 
always returns an error. WDYT?



Comment at: clang-tools-extra/clangd/index/remote/Index.proto:16
+
+  rpc FuzzyFind(FuzzyFindRequest) returns (FuzzyFindReply) {}
+}

should also return a stream. has_more is annoying, but you can keep it in the 
message and set it only on the last element of the stream.



Comment at: clang-tools-extra/clangd/index/remote/Index.proto:37
+message FuzzyFindReply {
+  // FIXME(kirillbobyrev): Convert to Symbol.
+  repeated string symbols = 1;

confused... why not use Symbol now?



Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:44
 
+clangd::FuzzyFindRequest deserialize(const FuzzyFindRequest *Request) {
+  clangd::FuzzyFindRequest Result;

move all the conversions into a file, marshalling.cpp or whatever?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78521



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

[PATCH] D78443: [Local] Update getOrEnforceKnownAlignment/getKnownAlignment to use Align/MaybeAlign.

[Craig Topper via Phabricator via cfe-commits]

craig.topper updated this revision to Diff 258859.
craig.topper added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix for clang usage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78443

Files:
  clang/lib/CodeGen/CGCall.cpp
  llvm/include/llvm/Transforms/Utils/Local.h
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
  llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
  llvm/lib/Transforms/Utils/InlineFunction.cpp
  llvm/lib/Transforms/Utils/Local.cpp
  llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp

Index: llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
===
--- llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
+++ llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
@@ -1015,11 +1015,11 @@
  vectorizeStoreChain(Chains.second, InstructionsProcessed);
 }
 
-unsigned NewAlign = getOrEnforceKnownAlignment(S0->getPointerOperand(),
-   StackAdjustedAlignment,
-   DL, S0, nullptr, );
-if (NewAlign >= Alignment.value())
-  Alignment = Align(NewAlign);
+Align NewAlign = getOrEnforceKnownAlignment(S0->getPointerOperand(),
+Align(StackAdjustedAlignment),
+DL, S0, nullptr, );
+if (NewAlign >= Alignment)
+  Alignment = NewAlign;
 else
   return false;
   }
@@ -1160,10 +1160,11 @@
  vectorizeLoadChain(Chains.second, InstructionsProcessed);
 }
 
-unsigned NewAlign = getOrEnforceKnownAlignment(
-  L0->getPointerOperand(), StackAdjustedAlignment, DL, L0, nullptr, );
-if (NewAlign >= Alignment.value())
-  Alignment = Align(NewAlign);
+Align NewAlign = getOrEnforceKnownAlignment(L0->getPointerOperand(),
+Align(StackAdjustedAlignment),
+DL, L0, nullptr, );
+if (NewAlign >= Alignment)
+  Alignment = NewAlign;
 else
   return false;
   }
Index: llvm/lib/Transforms/Utils/Local.cpp
===
--- llvm/lib/Transforms/Utils/Local.cpp
+++ llvm/lib/Transforms/Utils/Local.cpp
@@ -1156,9 +1156,8 @@
 /// often possible though. If alignment is important, a more reliable approach
 /// is to simply align all global variables and allocation instructions to
 /// their preferred alignment from the beginning.
-static unsigned enforceKnownAlignment(Value *V, unsigned Alignment,
-  unsigned PrefAlign,
-  const DataLayout ) {
+static Align enforceKnownAlignment(Value *V, Align Alignment, Align PrefAlign,
+   const DataLayout ) {
   assert(PrefAlign > Alignment);
 
   V = V->stripPointerCasts();
@@ -1170,21 +1169,21 @@
 // stripPointerCasts recurses through infinite layers of bitcasts,
 // while computeKnownBits is not allowed to traverse more than 6
 // levels.
-Alignment = std::max(AI->getAlignment(), Alignment);
+Alignment = max(AI->getAlign(), Alignment);
 if (PrefAlign <= Alignment)
   return Alignment;
 
 // If the preferred alignment is greater than the natural stack alignment
 // then don't round up. This avoids dynamic stack realignment.
-if (DL.exceedsNaturalStackAlignment(Align(PrefAlign)))
+if (DL.exceedsNaturalStackAlignment(PrefAlign))
   return Alignment;
-AI->setAlignment(Align(PrefAlign));
+AI->setAlignment(PrefAlign);
 return PrefAlign;
   }
 
   if (auto *GO = dyn_cast(V)) {
 // TODO: as above, this shouldn't be necessary.
-Alignment = std::max(GO->getAlignment(), Alignment);
+Alignment = max(GO->getAlign(), Alignment);
 if (PrefAlign <= Alignment)
   return Alignment;
 
@@ -1195,18 +1194,18 @@
 if (!GO->canIncreaseAlignment())
   return Alignment;
 
-GO->setAlignment(Align(PrefAlign));
+GO->setAlignment(PrefAlign);
 return PrefAlign;
   }
 
   return Alignment;
 }
 
-unsigned llvm::getOrEnforceKnownAlignment(Value *V, unsigned PrefAlign,
-  const DataLayout ,
-  const Instruction *CxtI,
-  AssumptionCache *AC,
-  const DominatorTree *DT) {
+Align llvm::getOrEnforceKnownAlignment(Value *V, MaybeAlign PrefAlign,
+   const DataLayout ,
+   const Instruction *CxtI,
+   AssumptionCache *AC,
+

[PATCH] D78521: [clangd] Extend dexp to support remote index

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 258861.
kbobyrev added a comment.

Fix build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78521

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/Serialization.h
  clang-tools-extra/clangd/index/YAMLSerialization.cpp
  clang-tools-extra/clangd/index/dex/dexp/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
  clang-tools-extra/clangd/index/remote/CMakeLists.txt
  clang-tools-extra/clangd/index/remote/Index.cpp
  clang-tools-extra/clangd/index/remote/Index.h
  clang-tools-extra/clangd/index/remote/Index.proto
  clang-tools-extra/clangd/index/remote/client/CMakeLists.txt
  clang-tools-extra/clangd/index/remote/client/Client.cpp
  clang-tools-extra/clangd/index/remote/server/Server.cpp
  llvm/cmake/modules/FindGRPC.cmake

Index: llvm/cmake/modules/FindGRPC.cmake
===
--- llvm/cmake/modules/FindGRPC.cmake
+++ llvm/cmake/modules/FindGRPC.cmake
@@ -23,11 +23,11 @@
   find_program(PROTOC protoc)
 endif()
 
-# Proto headers are generated in ${CMAKE_CURRENT_BINARY_DIR}.
+# Proto headers are generated in ${GeneratedFilesLocation}.
 # Libraries that use these headers should adjust the include path.
 # FIXME(kirillbobyrev): Allow optional generation of gRPC code and give callers
 # control over it via additional parameters.
-function(generate_grpc_protos LibraryName ProtoFile)
+function(generate_grpc_protos LibraryName ProtoFile GeneratedFilesLocation)
   get_filename_component(ProtoSourceAbsolutePath "${CMAKE_CURRENT_SOURCE_DIR}/${ProtoFile}" ABSOLUTE)
   get_filename_component(ProtoSourcePath ${ProtoSourceAbsolutePath} PATH)
 
@@ -35,6 +35,7 @@
   set(GeneratedProtoHeader "${CMAKE_CURRENT_BINARY_DIR}/Index.pb.h")
   set(GeneratedGRPCSource "${CMAKE_CURRENT_BINARY_DIR}/Index.grpc.pb.cc")
   set(GeneratedGRPCHeader "${CMAKE_CURRENT_BINARY_DIR}/Index.grpc.pb.h")
+  set(${GeneratedFilesLocation} ${CMAKE_CURRENT_BINARY_DIR} PARENT_SCOPE)
   add_custom_command(
 OUTPUT "${GeneratedProtoSource}" "${GeneratedProtoHeader}" "${GeneratedGRPCSource}" "${GeneratedGRPCHeader}"
 COMMAND ${PROTOC}
Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -23,6 +23,7 @@
 
 namespace clang {
 namespace clangd {
+namespace remote {
 namespace {
 
 static const std::string Overview = R"(
@@ -40,6 +41,22 @@
   return loadIndex(Index, /*UseIndex=*/true);
 }
 
+clangd::FuzzyFindRequest deserialize(const FuzzyFindRequest *Request) {
+  clangd::FuzzyFindRequest Result;
+  Result.Query = Request->query();
+  for (const auto  : Request->scopes())
+Result.Scopes.push_back(Scope);
+  Result.AnyScope = Request->any_scope();
+  if (Request->limit())
+Result.Limit = Request->limit();
+  Result.RestrictForCodeCompletion = Request->resricted_for_code_completion();
+  for (const auto  : Request->proximity_paths())
+Result.ProximityPaths.push_back(Path);
+  for (const auto  : Request->preferred_types())
+Result.ProximityPaths.push_back(Type);
+  return Result;
+}
+
 class RemoteIndexServer final : public remote::Index::Service {
 public:
   RemoteIndexServer(std::unique_ptr Index)
@@ -47,24 +64,35 @@
 
 private:
   grpc::Status Lookup(grpc::ServerContext *Context,
-  const remote::LookupRequest *Request,
-  grpc::ServerWriter *Reply) override {
-llvm::outs() << "Lookup of symbol with ID " << Request->id() << '\n';
-LookupRequest Req;
-auto SID = SymbolID::fromStr(Request->id());
-if (!SID) {
-  llvm::outs() << llvm::toString(SID.takeError()) << "\n";
-  return grpc::Status::CANCELLED;
+  const LookupRequest *Request,
+  grpc::ServerWriter *Reply) override {
+clangd::LookupRequest Req;
+for (const auto  : Request->id()) {
+  const std::string ConstantString = ID;
+  auto SID = SymbolID::fromStr(StringRef(ConstantString));
+  if (!SID)
+return grpc::Status::CANCELLED;
+  Req.IDs.insert(*SID);
 }
-Req.IDs.insert(*SID);
-Index->lookup(Req, [&](const Symbol ) {
-  remote::LookupReply NextSymbol;
-  NextSymbol.set_symbol_yaml(toYAML(Sym));
+Index->lookup(Req, [&](const clangd::Symbol ) {
+  remote::Symbol NextSymbol;
+  NextSymbol.set_yaml_serializatiton(toYAML(Sym));
   Reply->Write(NextSymbol);
 });
 return grpc::Status::OK;
   }
 
+  grpc::Status FuzzyFind(grpc::ServerContext *Context,
+ const FuzzyFindRequest *Request,
+ FuzzyFindReply *Reply) override {
+const auto Req = deserialize(Request);
+bool HasMore = Index->fuzzyFind(Req, [&](const clangd::Symbol ) {
+  

[PATCH] D76801: [AST] Print a> without extra spaces in C++11 or later.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D76801#1991904 , @labath wrote:

> David's example does work with gdb without -Wl,--gdb-index (the member 
> variable is shown), presumably due to the aforementioned fuzzy matching. 
> However, it does not work if gdb-index is enabled,  nor with lldb (as lldb is 
> always very index-oriented and assumes equality everywhere). That is 
> definitely not ideal, though I'm not sure that means about this patch. This 
> is definitely not the only difference in the formatting of DW_AT_names of 
> templates. For example, `template operator<<(T, T)` will come out 
> as `operator<< ` with gcc, but as `operator<<` with clang (with or 
> without this patch).
>  OTOH, differences in type names are more likely to cause problems than is 
> the case for functions/operators.


That is concerning. Any idea if that's only with lld's gdb-indexx 
implementation, or also gold's? This isn't the only naming divergence between 
GCC and Clang, though, so if gdb-index doesn't support any divergence, that's a 
problem... (I think non-type template parameters diverge too, perhaps? well, 
GCC 6 used to print "foo<3u>" (for an unsigned non-type template parameter) and 
clang/more recent GCC versions just use "foo<3>" now, for instance - enums, GCC 
uses foo<(EnumType)0> whereas Clang uses foo if it's named and 
foo<3> if it's unnamed))

Differences in function names could manifest as differences in type names - due 
to local types nested inside functions, but they're less likely/highly unlikely 
to have a decl/def resolution between CUs, so maybe nested types are less 
problematic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76801



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

[PATCH] D68049: Propeller: Clang options for basic block sections

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

rsmith added a comment.

Please add documentation for the new flags.




Comment at: clang/include/clang/Driver/Options.td:1974
 def fno_function_sections : Flag<["-"], "fno-function-sections">, 
Group;
+def fbasicblock_sections_EQ : Joined<["-"], "fbasicblock-sections=">, 
Group, Flags<[CC1Option, CC1AsOption]>,
+  HelpText<"Place each function's basic blocks in unique sections (ELF Only) : 
all | labels | none | ">;

I would prefer to spell this `basic-block` rather than `basicblock`, but I 
don't feel strongly about it.



Comment at: clang/include/clang/Driver/Options.td:1975
+def fbasicblock_sections_EQ : Joined<["-"], "fbasicblock-sections=">, 
Group, Flags<[CC1Option, CC1AsOption]>,
+  HelpText<"Place each function's basic blocks in unique sections (ELF Only) : 
all | labels | none | ">;
 def fdata_sections : Flag <["-"], "fdata-sections">, Group,

It's not great to use the same argument as both one of three specific strings 
and as an arbitrary filename. This not only prevents using those three names as 
the file name, it also means adding any more specific strings is a 
backwards-incompatible change. Can you find some way to tweak this to avoid 
that problem?



Comment at: clang/include/clang/Driver/Options.td:1990-1994
+def funique_bb_section_names : Flag <["-"], "funique-bb-section-names">,
+  Group, Flags<[CC1Option]>,
+  HelpText<"Use unique names for basic block sections (ELF Only)">;
+def fno_unique_bb_section_names : Flag <["-"], "fno-unique-bb-section-names">,
+  Group;

I don't like the inconsistency of using `bb` here and `basicblock` / 
`basic-block` above. Would spelling this out fully 
(`-funique-basic-block-section-names`) be OK?



Comment at: clang/lib/CodeGen/BackendUtil.cpp:486-501
+  Options.BBSections =
+  llvm::StringSwitch(CodeGenOpts.BBSections)
+  .Case("all", llvm::BasicBlockSection::All)
+  .Case("labels", llvm::BasicBlockSection::Labels)
+  .Case("none", llvm::BasicBlockSection::None)
+  .Default(llvm::BasicBlockSection::List);
+

I don't like doing the parsing here. But... this is consistent with what the 
other options above do, so I'm OK with keep it like this for consistency. (If 
someone wants to clean this up we can do that separately for all these enum 
options.)


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

https://reviews.llvm.org/D68049



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

[PATCH] D78495: [nfc] Accept addrspacecast allocas in InitTempAlloca

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield abandoned this revision.
JonChesterfield added a comment.

No problem. This isn't on the live path - the function is mostly called from 
openmp codegen and clang doesn't target openmp/amdgcn just yet. I'll roll this 
change into the codegen patch to enable that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78495



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

[PATCH] D78521: [clangd] Extend dexp to support remote index

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov, mgorny.
Herald added a project: clang.

- Make it possible for dexp to use remote server
- Remove clangd-index-client (since it's now merged into dexp)
- Implement `clangd::remote::IndexClient` that is derived from `SymbolIndex`
- Slightly improve CMake infrastructure

This is still WIP due to a couple of FIXMEs I need to resolve in this patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78521

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/Serialization.h
  clang-tools-extra/clangd/index/YAMLSerialization.cpp
  clang-tools-extra/clangd/index/dex/dexp/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/dexp/Dexp.cpp
  clang-tools-extra/clangd/index/remote/CMakeLists.txt
  clang-tools-extra/clangd/index/remote/Index.cpp
  clang-tools-extra/clangd/index/remote/Index.h
  clang-tools-extra/clangd/index/remote/Index.proto
  clang-tools-extra/clangd/index/remote/client/CMakeLists.txt
  clang-tools-extra/clangd/index/remote/client/Client.cpp
  clang-tools-extra/clangd/index/remote/server/Server.cpp
  llvm/cmake/modules/FindGRPC.cmake

Index: llvm/cmake/modules/FindGRPC.cmake
===
--- llvm/cmake/modules/FindGRPC.cmake
+++ llvm/cmake/modules/FindGRPC.cmake
@@ -23,11 +23,11 @@
   find_program(PROTOC protoc)
 endif()
 
-# Proto headers are generated in ${CMAKE_CURRENT_BINARY_DIR}.
+# Proto headers are generated in ${GeneratedFilesLocation}.
 # Libraries that use these headers should adjust the include path.
 # FIXME(kirillbobyrev): Allow optional generation of gRPC code and give callers
 # control over it via additional parameters.
-function(generate_grpc_protos LibraryName ProtoFile)
+function(generate_grpc_protos LibraryName ProtoFile GeneratedFilesLocation)
   get_filename_component(ProtoSourceAbsolutePath "${CMAKE_CURRENT_SOURCE_DIR}/${ProtoFile}" ABSOLUTE)
   get_filename_component(ProtoSourcePath ${ProtoSourceAbsolutePath} PATH)
 
@@ -35,6 +35,7 @@
   set(GeneratedProtoHeader "${CMAKE_CURRENT_BINARY_DIR}/Index.pb.h")
   set(GeneratedGRPCSource "${CMAKE_CURRENT_BINARY_DIR}/Index.grpc.pb.cc")
   set(GeneratedGRPCHeader "${CMAKE_CURRENT_BINARY_DIR}/Index.grpc.pb.h")
+  set(${GeneratedFilesLocation} ${CMAKE_CURRENT_BINARY_DIR} PARENT_SCOPE)
   add_custom_command(
 OUTPUT "${GeneratedProtoSource}" "${GeneratedProtoHeader}" "${GeneratedGRPCSource}" "${GeneratedGRPCHeader}"
 COMMAND ${PROTOC}
Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -23,6 +23,7 @@
 
 namespace clang {
 namespace clangd {
+namespace remote {
 namespace {
 
 static const std::string Overview = R"(
@@ -40,6 +41,22 @@
   return loadIndex(Index, /*UseIndex=*/true);
 }
 
+clangd::FuzzyFindRequest deserialize(const FuzzyFindRequest *Request) {
+  clangd::FuzzyFindRequest Result;
+  Result.Query = Request->query();
+  for (const auto  : Request->scopes())
+Result.Scopes.push_back(Scope);
+  Result.AnyScope = Request->any_scope();
+  if (Request->limit())
+Result.Limit = Request->limit();
+  Result.RestrictForCodeCompletion = Request->resricted_for_code_completion();
+  for (const auto  : Request->proximity_paths())
+Result.ProximityPaths.push_back(Path);
+  for (const auto  : Request->preferred_types())
+Result.ProximityPaths.push_back(Type);
+  return Result;
+}
+
 class RemoteIndexServer final : public remote::Index::Service {
 public:
   RemoteIndexServer(std::unique_ptr Index)
@@ -47,24 +64,35 @@
 
 private:
   grpc::Status Lookup(grpc::ServerContext *Context,
-  const remote::LookupRequest *Request,
-  grpc::ServerWriter *Reply) override {
-llvm::outs() << "Lookup of symbol with ID " << Request->id() << '\n';
-LookupRequest Req;
-auto SID = SymbolID::fromStr(Request->id());
-if (!SID) {
-  llvm::outs() << llvm::toString(SID.takeError()) << "\n";
-  return grpc::Status::CANCELLED;
+  const LookupRequest *Request,
+  grpc::ServerWriter *Reply) override {
+clangd::LookupRequest Req;
+for (const auto  : Request->id()) {
+  const std::string ConstantString = ID;
+  auto SID = SymbolID::fromStr(StringRef(ConstantString));
+  if (!SID)
+return grpc::Status::CANCELLED;
+  Req.IDs.insert(*SID);
 }
-Req.IDs.insert(*SID);
-Index->lookup(Req, [&](const Symbol ) {
-  remote::LookupReply NextSymbol;
-  NextSymbol.set_symbol_yaml(toYAML(Sym));
+Index->lookup(Req, [&](const clangd::Symbol ) {
+  remote::Symbol NextSymbol;
+  NextSymbol.set_yaml_serializatiton(toYAML(Sym));
  

[PATCH] D77392: [WIP][clangd] Make signatureHelp work with stale preambles

[Sam McCall via Phabricator via cfe-commits]

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

Keep missing the "ship it" button...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77392



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

[PATCH] D77392: [WIP][clangd] Make signatureHelp work with stale preambles

[Sam McCall via Phabricator via cfe-commits]

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

Great stuff!




Comment at: clang-tools-extra/clangd/Preamble.cpp:281
+  // We are only interested in newly added includes.
+  llvm::StringSet<> ExistingIncludes;
+  for (const auto  : Preamble.LexedIncludes)

kadircet wrote:
> sammccall wrote:
> > Why not a DenseSet>?
> > (The copies probably don't matter, but I think it'd be a bit clearer and 
> > more typesafe)
> PPKeywordKind didn't have a DenseMapInfo, adding one. It already has two 
> invalid enum values.
Ugh, I thought enums had those implicitly. The need for two invalid values is 
really annoying, it's probably why we don't have more implicit ones.

I wonder whether it's feasible (technically and legally) to replace 
DenseHashMap with a fork of absl::flat_hash_map. I'm pretty sure it's faster, 
and it doesn't have these weird API requirements.



Comment at: clang-tools-extra/clangd/Preamble.cpp:128
+std::vector
+scanPreambleIncludes(llvm::StringRef Contents,
+ llvm::IntrusiveRefCntPtr VFS,

The error-handling paths here just return {}, same as an empty preamble.

Results:
if both baseline & current fail, then we'll generate no patch --> fine
if baseline is ok and current fails, we'll consider all headers, we'll consider 
all headers removed and generate no new includes --> fine for now 
(added-includes-only model)
if baseline fails and current is OK, we'll create a patch that adds all the 
headers --> really slow, we'd be better off creating an empty patch

clarity and debuggability:
behaviors are implicit and silent. I think we should write out these three 
cases explicitly in the code, and log them at least at `-log/verbose`. These 
should be rare conditions I think. (If they're not, we should be able to track 
down & handle the common causes I think)

I think this function should probably return Expected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77392



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

[PATCH] D78513: [hip] Claim builtin type `__float128` supported if the host target supports it.

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

In D78513#1993115 , @yaxunl wrote:

> Currently if instructions of float128 get to amdgpu backend, are we going to 
> crash?


As `Float128Format` is re-defined as `double`, there won't be any issue in the 
backend. But, it won't function as the developer expects. That's quite similar 
to `long double` in clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78513



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

[PATCH] D77776: [Driver] Default to libc++ on FreeBSD

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

I don't like the fact that this only changes one of the users of 
`getTriple().getOSMajorVersion()`. Could you add a new member function such as

  void FreeBSD::getMajorVersion() const {
unsigned Major = getTriple().getOSMajorVersion();
if (Major == 0)
   return 10; 
return Major
  }

and replace all uses of `getTriple().getOSMajorVersion()` with 
`getMajorVersion()`.
We could also use the host version instead of 10?

  +#ifdef __FreeBSD__
  +   return __FreeBSD_version / 10;
  +#else
  +   return 10;
  +#endif


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D6



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

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 258834.
zoecarver marked 2 inline comments as done.
zoecarver added a comment.

- Rebase
- Fix based on review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67052

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/remove_cv.cpp
  clang/test/SemaCXX/remove_reference.cpp

Index: clang/test/SemaCXX/remove_reference.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/remove_reference.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++11 %s
+
+template
+struct test_remove_ref_trait
+{
+  typedef __remove_reference(T) type;
+};
+
+template
+struct test_remove_ref
+{
+  static const bool value = __is_same(typename test_remove_ref_trait::type, T)  &&
+__is_same(typename test_remove_ref_trait::type, T) &&
+__is_same(typename test_remove_ref_trait::type, T);
+};
+
+struct Foo { };
+
+template
+struct Bar { };
+
+template
+class Baz { };
+
+class Biz;
+
+void x() { }
+
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref>::value, "");
+static_assert(test_remove_ref>::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+
Index: clang/test/SemaCXX/remove_cv.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/remove_cv.cpp
@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 -std=c++11 %s
+
+template
+struct remove_const
+{
+  typedef __remove_const(T) type;
+};
+
+template
+struct remove_volatile
+{
+  typedef __remove_volatile(T) type;
+};
+
+template
+struct remove_cv
+{
+  typedef __remove_cv(T) type;
+};
+
+template
+struct test
+{
+  static const bool value =
+__is_same(typename remove_const::type, T) &&
+__is_same(typename remove_const::type, volatile T) &&
+__is_same(typename remove_const::type, T) &&
+
+__is_same(typename remove_volatile::type, T) &&
+__is_same(typename remove_volatile::type, const T) &&
+__is_same(typename remove_volatile::type, T) &&
+
+__is_same(typename remove_cv::type, T) &&
+__is_same(typename remove_cv::type, T) &&
+__is_same(typename remove_cv::type, T) &&
+__is_same(typename remove_cv::type, T);
+};
+
+struct Foo { };
+
+template
+struct Bar { };
+
+template
+class Baz { };
+
+class Biz;
+
+void x() { }
+
+static_assert(test::value, "");
+static_assert(test::value, "");
+static_assert(test::value, "");
+static_assert(test::value, "");
+static_assert(test>::value, "");
+static_assert(test>::value, "");
+static_assert(test::value, "");
+static_assert(test::value, "");
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1253,6 +1253,24 @@
   return OpenCLAccessAttr::Keyword_read_only;
 }
 
+static UnaryTransformType::UTTKind
+TSTToUnaryTransformType(DeclSpec::TST SwitchTST) {
+  switch (SwitchTST) {
+  case TST_removeCV:
+return UnaryTransformType::RemoveCV;
+  case TST_removeConst:
+return UnaryTransformType::RemoveConst;
+  case TST_removeVolatile:
+return UnaryTransformType::RemoveVolatile;
+  case TST_removeReferenceType:
+return UnaryTransformType::RemoveReferenceType;
+  case TST_underlyingType:
+return UnaryTransformType::EnumUnderlyingType;
+  default:
+llvm_unreachable("Cannot map TST to unary transform type");
+  }
+}
+
 static QualType ConvertConstrainedAutoDeclSpecToType(Sema , DeclSpec ,
  AutoTypeKeyword AutoKW) {
   assert(DS.isConstrainedAuto());
@@ -1605,16 +1623,23 @@
 break;
   }
   case DeclSpec::TST_underlyingType:
+  case DeclSpec::TST_removeReferenceType:
+  case DeclSpec::TST_removeCV:
+  case DeclSpec::TST_removeConst:
+  case DeclSpec::TST_removeVolatile: {
 Result = S.GetTypeFromParser(DS.getRepAsType());
-assert(!Result.isNull() && "Didn't get a type for __underlying_type?");
+assert(!Result.isNull() &&
+   "Type transform builtin 

[PATCH] D77456: [clangd] Parse `foo` in documentation comments and render as code.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D77456#1991484 , @sammccall wrote:

> No, I'm complaining about the space before the period in
>
>   Tests primality of `p` .
>
>
> and the plaintext rendering too
>
>   Tests primality of p .
>


Ah I see, yes this looks annoying, and it is only because the next block starts 
with a punctuation :/

> Two main objections to the idea of "raw":
> 
> - we're going to emit arbitrary, potentially malformed markdown into the 
> markdown stream, which can have arbitrary effects/glitches. I'd rather the 
> emitter always emits valid markup and thus can't lose track of the context.
> - this assumes the input is markdown. I want `\c foo` to also render as 
> code-font `foo` in markdown and as backtick-foo in plaintext. So what do we 
> do there, generate markdown as a string and then emit it as a raw chunk? What 
> a mess.
> 
>   I really do think what we want is a chunk with semantics "emphasized code" 
> that renders as a code span in markdown and as backtick-delimited text in 
> plaintext. Thus the proposal to put an emphasis bit on the code chunk. WDYT?

Sorry I wasn't clear on my comment. I was also suggesting putting some markdown 
generated by us while parsing the documentation into this raw field, which 
would be rendered as-is. In case we would like to perform this for other 
markers like `*`.
So going with an emphasis-only solution is also OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77456



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

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver updated this revision to Diff 258836.
zoecarver added a comment.

- Format using clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67052

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaTemplateVariadic.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/remove_cv.cpp
  clang/test/SemaCXX/remove_reference.cpp

Index: clang/test/SemaCXX/remove_reference.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/remove_reference.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -std=c++11 %s
+
+template
+struct test_remove_ref_trait
+{
+  typedef __remove_reference(T) type;
+};
+
+template
+struct test_remove_ref
+{
+  static const bool value = __is_same(typename test_remove_ref_trait::type, T)  &&
+__is_same(typename test_remove_ref_trait::type, T) &&
+__is_same(typename test_remove_ref_trait::type, T);
+};
+
+struct Foo { };
+
+template
+struct Bar { };
+
+template
+class Baz { };
+
+class Biz;
+
+void x() { }
+
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref>::value, "");
+static_assert(test_remove_ref>::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+static_assert(test_remove_ref::value, "");
+
Index: clang/test/SemaCXX/remove_cv.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/remove_cv.cpp
@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 -std=c++11 %s
+
+template
+struct remove_const
+{
+  typedef __remove_const(T) type;
+};
+
+template
+struct remove_volatile
+{
+  typedef __remove_volatile(T) type;
+};
+
+template
+struct remove_cv
+{
+  typedef __remove_cv(T) type;
+};
+
+template
+struct test
+{
+  static const bool value =
+__is_same(typename remove_const::type, T) &&
+__is_same(typename remove_const::type, volatile T) &&
+__is_same(typename remove_const::type, T) &&
+
+__is_same(typename remove_volatile::type, T) &&
+__is_same(typename remove_volatile::type, const T) &&
+__is_same(typename remove_volatile::type, T) &&
+
+__is_same(typename remove_cv::type, T) &&
+__is_same(typename remove_cv::type, T) &&
+__is_same(typename remove_cv::type, T) &&
+__is_same(typename remove_cv::type, T);
+};
+
+struct Foo { };
+
+template
+struct Bar { };
+
+template
+class Baz { };
+
+class Biz;
+
+void x() { }
+
+static_assert(test::value, "");
+static_assert(test::value, "");
+static_assert(test::value, "");
+static_assert(test::value, "");
+static_assert(test>::value, "");
+static_assert(test>::value, "");
+static_assert(test::value, "");
+static_assert(test::value, "");
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1253,6 +1253,24 @@
   return OpenCLAccessAttr::Keyword_read_only;
 }
 
+static UnaryTransformType::UTTKind
+TSTToUnaryTransformType(DeclSpec::TST SwitchTST) {
+  switch (SwitchTST) {
+  case TST_removeCV:
+return UnaryTransformType::RemoveCV;
+  case TST_removeConst:
+return UnaryTransformType::RemoveConst;
+  case TST_removeVolatile:
+return UnaryTransformType::RemoveVolatile;
+  case TST_removeReferenceType:
+return UnaryTransformType::RemoveReferenceType;
+  case TST_underlyingType:
+return UnaryTransformType::EnumUnderlyingType;
+  default:
+llvm_unreachable("Cannot map TST to unary transform type");
+  }
+}
+
 static QualType ConvertConstrainedAutoDeclSpecToType(Sema , DeclSpec ,
  AutoTypeKeyword AutoKW) {
   assert(DS.isConstrainedAuto());
@@ -1605,16 +1623,23 @@
 break;
   }
   case DeclSpec::TST_underlyingType:
+  case DeclSpec::TST_removeReferenceType:
+  case DeclSpec::TST_removeCV:
+  case DeclSpec::TST_removeConst:
+  case DeclSpec::TST_removeVolatile: {
 Result = S.GetTypeFromParser(DS.getRepAsType());
-assert(!Result.isNull() && "Didn't get a type for __underlying_type?");
+assert(!Result.isNull() &&
+   "Type transform builtin may not have received a type.");
 Result = 

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

ping


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

https://reviews.llvm.org/D77954



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

[PATCH] D77392: [WIP][clangd] Make signatureHelp work with stale preambles

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 258833.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Address comments
- Don't store LexedInclude in PreambleData, expose the contents in 
PrecompiledPreamble instead.
- Introduce DenseMapInfo for PPKeywordKind


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77392

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Preamble.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/PreambleTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang/include/clang/Basic/TokenKinds.h
  clang/include/clang/Frontend/PrecompiledPreamble.h

Index: clang/include/clang/Frontend/PrecompiledPreamble.h
===
--- clang/include/clang/Frontend/PrecompiledPreamble.h
+++ clang/include/clang/Frontend/PrecompiledPreamble.h
@@ -16,6 +16,7 @@
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/AlignOf.h"
 #include "llvm/Support/MD5.h"
 #include 
@@ -94,6 +95,11 @@
   /// be used for logging and debugging purposes only.
   std::size_t getSize() const;
 
+  /// Returned string is not null-terminated.
+  llvm::StringRef getContents() const {
+return {PreambleBytes.data(), PreambleBytes.size()};
+  }
+
   /// Check whether PrecompiledPreamble can be reused for the new contents(\p
   /// MainFileBuffer) of the main file.
   bool CanReuse(const CompilerInvocation ,
Index: clang/include/clang/Basic/TokenKinds.h
===
--- clang/include/clang/Basic/TokenKinds.h
+++ clang/include/clang/Basic/TokenKinds.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_CLANG_BASIC_TOKENKINDS_H
 #define LLVM_CLANG_BASIC_TOKENKINDS_H
 
+#include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/Support/Compiler.h"
 
 namespace clang {
@@ -95,7 +96,25 @@
 /// Return true if this is an annotation token representing a pragma.
 bool isPragmaAnnotation(TokenKind K);
 
-}  // end namespace tok
-}  // end namespace clang
+} // end namespace tok
+} // end namespace clang
+
+namespace llvm {
+template <> struct DenseMapInfo {
+  static inline clang::tok::PPKeywordKind getEmptyKey() {
+return clang::tok::PPKeywordKind::pp_not_keyword;
+  }
+  static inline clang::tok::PPKeywordKind getTombstoneKey() {
+return clang::tok::PPKeywordKind::NUM_PP_KEYWORDS;
+  }
+  static unsigned getHashValue(const clang::tok::PPKeywordKind ) {
+return static_cast(Val);
+  }
+  static bool isEqual(const clang::tok::PPKeywordKind ,
+  const clang::tok::PPKeywordKind ) {
+return LHS == RHS;
+  }
+};
+} // namespace llvm
 
 #endif
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -246,66 +246,6 @@
   EXPECT_EQ(2, CallbackCount);
 }
 
-static std::vector includes(const PreambleData *Preamble) {
-  std::vector Result;
-  if (Preamble)
-for (const auto  : Preamble->Includes.MainFileIncludes)
-  Result.push_back(Inclusion.Written);
-  return Result;
-}
-
-TEST_F(TUSchedulerTests, PreambleConsistency) {
-  std::atomic CallbackCount(0);
-  {
-Notification InconsistentReadDone; // Must live longest.
-TUScheduler S(CDB, optsForTest());
-auto Path = testPath("foo.cpp");
-// Schedule two updates (A, B) and two preamble reads (stale, consistent).
-// The stale read should see A, and the consistent read should see B.
-// (We recognize the preambles by their included files).
-auto Inputs = getInputs(Path, "#include ");
-Inputs.Version = "A";
-updateWithCallback(S, Path, Inputs, WantDiagnostics::Yes, [&]() {
-  // This callback runs in between the two preamble updates.
-
-  // This blocks update B, preventing it from winning the race
-  // against the stale read.
-  // If the first read was instead consistent, this would deadlock.
-  InconsistentReadDone.wait();
-  // This delays update B, preventing it from winning a race
-  // against the consistent read. The consistent read sees B
-  // only because it waits for it.
-  // If the second read was stale, it would usually see A.
-  std::this_thread::sleep_for(std::chrono::milliseconds(100));
-});
-Inputs.Contents = "#include ";
-Inputs.Version = "B";
-S.update(Path, Inputs, WantDiagnostics::Yes);
-
-S.runWithPreamble("StaleRead", Path, TUScheduler::Stale,
-   

[PATCH] D77392: [WIP][clangd] Make signatureHelp work with stale preambles

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked 15 inline comments as done.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/Preamble.cpp:278
+  // This shouldn't coincide with any real file name.
+  PP.PatchFileName = llvm::formatv("{0}_preamble_patch.h", FileName);
+

sammccall wrote:
> I'm slightly nervous about incorporating the filename itself, not sure why 
> but it feels unneccesary.
> WDYT about "dir/__preamble_patch__.h"?
it was done to get rid of path manipulations, but not that important.



Comment at: clang-tools-extra/clangd/Preamble.cpp:281
+  // We are only interested in newly added includes.
+  llvm::StringSet<> ExistingIncludes;
+  for (const auto  : Preamble.LexedIncludes)

sammccall wrote:
> Why not a DenseSet>?
> (The copies probably don't matter, but I think it'd be a bit clearer and more 
> typesafe)
PPKeywordKind didn't have a DenseMapInfo, adding one. It already has two 
invalid enum values.



Comment at: clang-tools-extra/clangd/Preamble.h:74
+  /// ones in disabled regions.
+  std::vector LexedIncludes;
 };

sammccall wrote:
> Since computing these inclusions seems to be cheap (you've disabled all the 
> IO), would it be clearer and more flexible to store the preamble text as a 
> string and compute both the baseline & new includes at the same time?
> I'm thinking if other preamble constructs (macros?) are added, needing to put 
> more data structures in PreambleData, where if you store the text only those 
> can be private details.
> 
> (In fact the preamble text is already stored in PrecompiledPreamble, you 
> could just expose it there)
> 
> 
since a preamble can be used with multiple preamble patches i wanted to reduce 
lexing overhead. I suppose we can address that later if it shows up in the 
traces. exposing the contents in PrecompiledPreamble.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77392



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

[PATCH] D67052: Add reference type transformation builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver marked 6 inline comments as done.
zoecarver added a comment.

@EricWF and @miscco thanks for the review comments. Sorry for the delay, I 
forgot about this patch. All your comments have been addressed/fixed.




Comment at: clang/lib/AST/TypePrinter.cpp:1020
 
   switch (T->getUTTKind()) {
 case UnaryTransformType::EnumUnderlyingType:

miscco wrote:
> Couldn't we use `TextNodeDumper::VisitUnaryTransformType` to dump the string 
> and then simplify it to a single case.
> 
> Given the possibility that maybe some day one might add the `add_foo` builtins
> Couldn't we use TextNodeDumper::VisitUnaryTransformType to dump the string 
> and then simplify it to a single case.

Good call. Will do.

> Given the possibility that maybe some day one might add the add_foo builtins

I had a patch that added those builtins but, we decided that there was no real 
reason for them to exist.




Comment at: clang/lib/Sema/SemaType.cpp:1624
   }
+  case DeclSpec::TST_removeReferenceType:
+  case DeclSpec::TST_removeCV:

EricWF wrote:
> Why can't we share the implementation with `TST_underlyingType`?
Yep, fixed. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67052



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

[PATCH] D78513: [hip] Claim builtin type `__float128` supported if the host target supports it.

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Currently if instructions of float128 get to amdgpu backend, are we going to 
crash?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78513



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

[PATCH] D78441: Delete NaCl support

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

@jfb thanks for the heads-up.
I replied on the mailing list thread.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78441



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

[PATCH] D78511: [Driver][doc] Document option -mtune as a no-op. NFC.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer updated this revision to Diff 258823.
SjoerdMeijer added a comment.

Cheers, that's probably what I wanted to say.


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

https://reviews.llvm.org/D78511

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2710,7 +2710,8 @@
 def module_file_info : Flag<["-"], "module-file-info">, 
Flags<[DriverOption,CC1Option]>, Group,
   HelpText<"Provide information about a particular module file">;
 def mthumb : Flag<["-"], "mthumb">, Group;
-def mtune_EQ : Joined<["-"], "mtune=">, Group;
+def mtune_EQ : Joined<["-"], "mtune=">, Group,
+  HelpText<"Accepted for compatibility with GCC. Currently has no effect.">;
 def multi__module : Flag<["-"], "multi_module">;
 def multiply__defined__unused : Separate<["-"], "multiply_defined_unused">;
 def multiply__defined : Separate<["-"], "multiply_defined">;
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -2735,6 +2735,8 @@
 .. option:: -mtune=
 .. program:: clang
 
+Accepted for compatibility with GCC. Currently has no effect.
+
 .. option:: -mtvos-version-min=, -mappletvos-version-min=
 
 .. option:: -municode


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2710,7 +2710,8 @@
 def module_file_info : Flag<["-"], "module-file-info">, Flags<[DriverOption,CC1Option]>, Group,
   HelpText<"Provide information about a particular module file">;
 def mthumb : Flag<["-"], "mthumb">, Group;
-def mtune_EQ : Joined<["-"], "mtune=">, Group;
+def mtune_EQ : Joined<["-"], "mtune=">, Group,
+  HelpText<"Accepted for compatibility with GCC. Currently has no effect.">;
 def multi__module : Flag<["-"], "multi_module">;
 def multiply__defined__unused : Separate<["-"], "multiply_defined_unused">;
 def multiply__defined : Separate<["-"], "multiply_defined">;
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -2735,6 +2735,8 @@
 .. option:: -mtune=
 .. program:: clang
 
+Accepted for compatibility with GCC. Currently has no effect.
+
 .. option:: -mtvos-version-min=, -mappletvos-version-min=
 
 .. option:: -municode
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78513: [hip] Claim builtin type `__float128` supported if the host target supports it.

[Michael Liao via Phabricator via cfe-commits]

hliao created this revision.
hliao added reviewers: tra, yaxunl.
Herald added subscribers: cfe-commits, kerbowa, nhaehnle, jvesely.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78513

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/test/SemaCUDA/amdgpu-f128.cu


Index: clang/test/SemaCUDA/amdgpu-f128.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/amdgpu-f128.cu
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -aux-triple 
x86_64-unknown-linux-gnu -fcuda-is-device -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+typedef __float128 f128_t;
Index: clang/lib/Basic/Targets/AMDGPU.cpp
===
--- clang/lib/Basic/Targets/AMDGPU.cpp
+++ clang/lib/Basic/Targets/AMDGPU.cpp
@@ -363,4 +363,17 @@
   copyAuxTarget(Aux);
   LongDoubleFormat = SaveLongDoubleFormat;
   Float128Format = SaveFloat128Format;
+  // For certain builtin types support on the host target, claim they are
+  // support to pass the compilation of the host code during the device-side
+  // compilation.
+  // FIXME: As the side effect, we also accept `__float128` uses in the device
+  // code. To rejct these builtin types supported in the host target but not in
+  // the device target, one approach would support `device_builtin` attribute
+  // so that we could tell the device builtin types from the host ones. The
+  // also solves the different representations of the same builtin type, such
+  // as `size_t` in the MSVC environment.
+  if (Aux->hasFloat128Type()) {
+HasFloat128 = true;
+Float128Format = DoubleFormat;
+  }
 }


Index: clang/test/SemaCUDA/amdgpu-f128.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/amdgpu-f128.cu
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -aux-triple x86_64-unknown-linux-gnu -fcuda-is-device -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+typedef __float128 f128_t;
Index: clang/lib/Basic/Targets/AMDGPU.cpp
===
--- clang/lib/Basic/Targets/AMDGPU.cpp
+++ clang/lib/Basic/Targets/AMDGPU.cpp
@@ -363,4 +363,17 @@
   copyAuxTarget(Aux);
   LongDoubleFormat = SaveLongDoubleFormat;
   Float128Format = SaveFloat128Format;
+  // For certain builtin types support on the host target, claim they are
+  // support to pass the compilation of the host code during the device-side
+  // compilation.
+  // FIXME: As the side effect, we also accept `__float128` uses in the device
+  // code. To rejct these builtin types supported in the host target but not in
+  // the device target, one approach would support `device_builtin` attribute
+  // so that we could tell the device builtin types from the host ones. The
+  // also solves the different representations of the same builtin type, such
+  // as `size_t` in the MSVC environment.
+  if (Aux->hasFloat128Type()) {
+HasFloat128 = true;
+Float128Format = DoubleFormat;
+  }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78511: [Driver][doc] Document option -mtune as a no-op. NFC.

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

Quuxplusone added inline comments.



Comment at: clang/docs/ClangCommandLineReference.rst:2738
 
+Accepts any value for compatability reasons with GCC, thus not performing any 
CPU type specific tuning.
+

"Accepts any value, for compatibility with GCC. Does not perform any 
CPU-specific tuning."

In email I said:
> there is no practical difference, but there may be a psychological 
> difference, between saying "Clang permanently treats -mtune as a no-op" 
> versus "Clang has a remarkably low //quality of implementation// for -mtune, 
> and has no immediate plans to either improve or regress it."

To me, this patch's wording implies that Clang is promising to keep -mtune as a 
no-op forever. If Bob puts `-mtune=supercalifragilistic` in his Makefile today, 
and then in the future Clang starts emitting an "unknown target" error, will 
Bob point to this documentation as evidence that Clang promised to accept 
`-mtune=supercalifragilistic` as a no-op?

So I would rather say:

"Accepted for compatibility with GCC. Currently has no effect."


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

https://reviews.llvm.org/D78511



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

[PATCH] D78066: [SemaObjC] Forbid storing an unboxed integer literal in an NSNumber

[Erik Pilkington via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG85cca945b4c9: [SemaObjC] Forbid storing an unboxed integer 
literal in an NSNumber (authored by erik.pilkington).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78066

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaObjC/objc-literal-fixit.m

Index: clang/test/SemaObjC/objc-literal-fixit.m
===
--- /dev/null
+++ clang/test/SemaObjC/objc-literal-fixit.m
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.10 %s -verify=c,expected
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.10 %s -xobjective-c++ -verify=cxx,expected
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.10 %s -fobjc-arc  -verify=c,arc,expected
+
+typedef signed char BOOL;
+#define YES __objc_yes
+#define NO __objc_no
+
+@interface NSNumber
++(instancetype)numberWithChar:(char)value;
++(instancetype)numberWithInt:(int)value;
++(instancetype)numberWithDouble:(double)value;
++(instancetype)numberWithBool:(BOOL)value;
+@end
+
+void test() {
+  NSNumber *n = YES; // expected-error{{numeric literal must be prefixed by '@'}}
+  NSNumber *n1 = 1; // expected-error{{numeric literal must be prefixed by '@'}}
+
+  NSNumber *n2 = NO; // c-warning{{expression which evaluates to zero treated as a null pointer constant}}
+ // cxx-error@-1{{numeric literal must be prefixed by '@'}}
+  NSNumber *n3 = 0;
+  NSNumber *n4 = 0.0; // expected-error{{numeric literal must be prefixed by '@'}}
+
+  NSNumber *n5 = '\0'; // c-warning{{expression which evaluates to zero treated as a null pointer constant}}
+   // cxx-error@-1{{numeric literal must be prefixed by '@'}}
+
+
+  NSNumber *n6 = '1'; // expected-error{{numeric literal must be prefixed by '@'}}
+
+  int i;
+  NSNumber *n7 = i; // c-warning{{incompatible integer to pointer conversion}}
+// arc-error@-1{{implicit conversion of 'int' to 'NSNumber *' is disallowed with ARC}}
+// cxx-error@-2{{cannot initialize a variable of type 'NSNumber *' with an lvalue of type 'int'}}
+
+  id n8 = 1; // c-warning{{incompatible integer to pointer conversion}}
+ // arc-error@-1{{implicit conversion of 'int' to 'id' is disallowed with ARC}}
+ // cxx-error@-2{{cannot initialize a variable of type 'id' with an rvalue of type 'int'}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -5625,7 +5625,7 @@
   if (S.CheckObjCBridgeRelatedConversions(Initializer->getBeginLoc(),
   DestType, Initializer->getType(),
   Initializer) ||
-  S.ConversionToObjCStringLiteralCheck(DestType, Initializer))
+  S.CheckConversionToObjCLiteral(DestType, Initializer))
 Args[0] = Initializer;
 }
 if (!isa(Initializer))
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -4363,7 +4363,7 @@
   // to 'NSString *', instead of falling through to report a "bridge cast"
   // diagnostic.
   if (castACTC == ACTC_retainable && exprACTC == ACTC_none &&
-  ConversionToObjCStringLiteralCheck(castType, castExpr, Diagnose))
+  CheckConversionToObjCLiteral(castType, castExpr, Diagnose))
 return ACR_error;
 
   // Do not issue "bridge cast" diagnostic when implicit casting
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -9283,7 +9283,7 @@
 if (getLangOpts().ObjC &&
 (CheckObjCBridgeRelatedConversions(E->getBeginLoc(), LHSType,
E->getType(), E, Diagnose) ||
- ConversionToObjCStringLiteralCheck(LHSType, E, Diagnose))) {
+ CheckConversionToObjCLiteral(LHSType, E, Diagnose))) {
   if (!Diagnose)
 return Incompatible;
   // Replace the expression with a corrected version and continue so we
@@ -15228,21 +15228,15 @@
   SourceLocExpr(Context, Kind, BuiltinLoc, RPLoc, ParentContext);
 }
 
-bool Sema::ConversionToObjCStringLiteralCheck(QualType DstType, Expr *,
-  bool Diagnose) {
+bool Sema::CheckConversionToObjCLiteral(QualType DstType, Expr *,
+bool Diagnose) {
   if (!getLangOpts().ObjC)
 return false;
 
   const ObjCObjectPointerType *PT = 

[PATCH] D78511: [Driver][doc] Document option -mtune as a no-op. NFC.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer created this revision.
SjoerdMeijer added a reviewer: dblaikie.

This is a doc change documenting that option -mtune does not perform any CPU 
type specific tuning but exists for compatability reasons with GCC.


https://reviews.llvm.org/D78511

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Driver/Options.td


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2710,7 +2710,9 @@
 def module_file_info : Flag<["-"], "module-file-info">, 
Flags<[DriverOption,CC1Option]>, Group,
   HelpText<"Provide information about a particular module file">;
 def mthumb : Flag<["-"], "mthumb">, Group;
-def mtune_EQ : Joined<["-"], "mtune=">, Group;
+def mtune_EQ : Joined<["-"], "mtune=">, Group,
+  HelpText<"Accept any  for compatability reasons with GCC, thus not "
+   "performing any CPU type specific tuning">;
 def multi__module : Flag<["-"], "multi_module">;
 def multiply__defined__unused : Separate<["-"], "multiply_defined_unused">;
 def multiply__defined : Separate<["-"], "multiply_defined">;
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -2735,6 +2735,8 @@
 .. option:: -mtune=
 .. program:: clang
 
+Accepts any value for compatability reasons with GCC, thus not performing any 
CPU type specific tuning.
+
 .. option:: -mtvos-version-min=, -mappletvos-version-min=
 
 .. option:: -municode


Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2710,7 +2710,9 @@
 def module_file_info : Flag<["-"], "module-file-info">, Flags<[DriverOption,CC1Option]>, Group,
   HelpText<"Provide information about a particular module file">;
 def mthumb : Flag<["-"], "mthumb">, Group;
-def mtune_EQ : Joined<["-"], "mtune=">, Group;
+def mtune_EQ : Joined<["-"], "mtune=">, Group,
+  HelpText<"Accept any  for compatability reasons with GCC, thus not "
+   "performing any CPU type specific tuning">;
 def multi__module : Flag<["-"], "multi_module">;
 def multiply__defined__unused : Separate<["-"], "multiply_defined_unused">;
 def multiply__defined : Separate<["-"], "multiply_defined">;
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -2735,6 +2735,8 @@
 .. option:: -mtune=
 .. program:: clang
 
+Accepts any value for compatability reasons with GCC, thus not performing any CPU type specific tuning.
+
 .. option:: -mtvos-version-min=, -mappletvos-version-min=
 
 .. option:: -municode
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78338: [clangd] Enable diagnostic fixes within macro argument expansions.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6529b0c48aab: [clangd] Enable diagnostic fixes within macro 
argument expansions. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D78338?vs=258202=258812#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78338

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -104,6 +104,7 @@
   // Check we report correct ranges, including various edge-cases.
   Annotations Test(R"cpp(
 // error-ok
+#define ID(X) X
 namespace test{};
 void $decl[[foo]]();
 class T{$explicit[[]]$constructor[[T]](int a);};
@@ -116,6 +117,7 @@
   struct Foo { int x; }; Foo a;
   a.$nomember[[y]];
   test::$nomembernamespace[[test]];
+  $macro[[ID($macroarg[[fod]])]]();
 }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
@@ -144,6 +146,10 @@
   Diag(Test.range("nomember"), "no member named 'y' in 'Foo'"),
   Diag(Test.range("nomembernamespace"),
"no member named 'test' in namespace 'test'"),
+  AllOf(Diag(Test.range("macro"),
+ "use of undeclared identifier 'fod'; did you mean 
'foo'?"),
+WithFix(Fix(Test.range("macroarg"), "foo",
+"change 'fod' to 'foo'"))),
   // We make sure here that the entire token is highlighted
   AllOf(Diag(Test.range("constructor"),
  "single-argument constructors must be marked explicit to "
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -556,10 +556,23 @@
 if (!InsideMainFile)
   return false;
 
+// Copy as we may modify the ranges.
+auto FixIts = Info.getFixItHints().vec();
 llvm::SmallVector Edits;
-for (auto  : Info.getFixItHints()) {
-  // Follow clang's behavior, don't apply FixIt to the code in macros,
-  // we are less certain it is the right fix.
+for (auto  : FixIts) {
+  // Allow fixits within a single macro-arg expansion to be applied.
+  // This can be incorrect if the argument is expanded multiple times in
+  // different contexts. Hopefully this is rare!
+  if (FixIt.RemoveRange.getBegin().isMacroID() &&
+  FixIt.RemoveRange.getEnd().isMacroID() &&
+  SM.getFileID(FixIt.RemoveRange.getBegin()) ==
+  SM.getFileID(FixIt.RemoveRange.getEnd())) {
+FixIt.RemoveRange = CharSourceRange(
+{SM.getTopMacroCallerLoc(FixIt.RemoveRange.getBegin()),
+ SM.getTopMacroCallerLoc(FixIt.RemoveRange.getEnd())},
+FixIt.RemoveRange.isTokenRange());
+  }
+  // Otherwise, follow clang's behavior: no fixits in macros.
   if (FixIt.RemoveRange.getBegin().isMacroID() ||
   FixIt.RemoveRange.getEnd().isMacroID())
 return false;
@@ -570,8 +583,8 @@
 
 llvm::SmallString<64> Message;
 // If requested and possible, create a message like "change 'foo' to 
'bar'".
-if (SyntheticMessage && Info.getNumFixItHints() == 1) {
-  const auto  = Info.getFixItHint(0);
+if (SyntheticMessage && FixIts.size() == 1) {
+  const auto  = FixIts.front();
   bool Invalid = false;
   llvm::StringRef Remove =
   Lexer::getSourceText(FixIt.RemoveRange, SM, *LangOpts, );


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -104,6 +104,7 @@
   // Check we report correct ranges, including various edge-cases.
   Annotations Test(R"cpp(
 // error-ok
+#define ID(X) X
 namespace test{};
 void $decl[[foo]]();
 class T{$explicit[[]]$constructor[[T]](int a);};
@@ -116,6 +117,7 @@
   struct Foo { int x; }; Foo a;
   a.$nomember[[y]];
   test::$nomembernamespace[[test]];
+  $macro[[ID($macroarg[[fod]])]]();
 }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
@@ -144,6 +146,10 @@
   Diag(Test.range("nomember"), "no member named 'y' in 'Foo'"),
   Diag(Test.range("nomembernamespace"),
"no member named 'test' in namespace 'test'"),
+  AllOf(Diag(Test.range("macro"),
+ "use of undeclared identifier 'fod'; did you mean 'foo'?"),
+WithFix(Fix(Test.range("macroarg"), "foo",
+"change 

[PATCH] D78495: [nfc] Accept addrspacecast allocas in InitTempAlloca

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

You're not testing an assertion, you're testing that code is generated 
correctly for some file on amdgcn.  Just write an ordinary IR-generation test 
that currently crashes and this test fixes.

This is not an NFC change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78495



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

[clang] 85cca94 - [SemaObjC] Forbid storing an unboxed integer literal in an NSNumber

[Erik Pilkington via cfe-commits]

Author: Erik Pilkington
Date: 2020-04-20T15:22:51-04:00
New Revision: 85cca945b4c93ecfd3f3bb7a7b2d5fa4104c3560

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

LOG: [SemaObjC] Forbid storing an unboxed integer literal in an NSNumber

This fixes a common mistake (the 3 should be @3): NSNumber *n = 3. This extends
an existing check for NSString. Also, this only errs if the initializer isn't a
null pointer constant, so NSNumber *n = 0; continues to work. rdar://47029572

Differential revision: https://reviews.llvm.org/D78066

Added: 
clang/test/SemaObjC/objc-literal-fixit.m

Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaExprObjC.cpp
clang/lib/Sema/SemaInit.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a64e313bf271..014ee1c2f2d7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2945,7 +2945,7 @@ def warn_objc_literal_comparison : Warning<
   "a numeric literal|a boxed expression|}0 has undefined behavior">,
   InGroup;
 def err_missing_atsign_prefix : Error<
-  "string literal must be prefixed by '@' ">;
+  "%select{string|numeric}0 literal must be prefixed by '@'">;
 def warn_objc_string_literal_comparison : Warning<
   "direct comparison of a string literal has undefined behavior">,
   InGroup;

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index af58b0ec4e82..5cd75b176761 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9455,8 +9455,8 @@ class Sema final {
  QualType DestType, QualType SrcType,
  Expr *, bool Diagnose = true);
 
-  bool ConversionToObjCStringLiteralCheck(QualType DstType, Expr *,
-  bool Diagnose = true);
+  bool CheckConversionToObjCLiteral(QualType DstType, Expr *,
+bool Diagnose = true);
 
   bool checkInitMethod(ObjCMethodDecl *method, QualType receiverTypeIfCall);
 

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index fbb5d4b05bbf..75d80c0cc45c 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -9283,7 +9283,7 @@ Sema::CheckSingleAssignmentConstraints(QualType LHSType, 
ExprResult ,
 if (getLangOpts().ObjC &&
 (CheckObjCBridgeRelatedConversions(E->getBeginLoc(), LHSType,
E->getType(), E, Diagnose) ||
- ConversionToObjCStringLiteralCheck(LHSType, E, Diagnose))) {
+ CheckConversionToObjCLiteral(LHSType, E, Diagnose))) {
   if (!Diagnose)
 return Incompatible;
   // Replace the expression with a corrected version and continue so we
@@ -15228,21 +15228,15 @@ ExprResult 
Sema::BuildSourceLocExpr(SourceLocExpr::IdentKind Kind,
   SourceLocExpr(Context, Kind, BuiltinLoc, RPLoc, ParentContext);
 }
 
-bool Sema::ConversionToObjCStringLiteralCheck(QualType DstType, Expr *,
-  bool Diagnose) {
+bool Sema::CheckConversionToObjCLiteral(QualType DstType, Expr *,
+bool Diagnose) {
   if (!getLangOpts().ObjC)
 return false;
 
   const ObjCObjectPointerType *PT = DstType->getAs();
   if (!PT)
 return false;
-
-  if (!PT->isObjCIdType()) {
-// Check if the destination is the 'NSString' interface.
-const ObjCInterfaceDecl *ID = PT->getInterfaceDecl();
-if (!ID || !ID->getIdentifier()->isStr("NSString"))
-  return false;
-  }
+  const ObjCInterfaceDecl *ID = PT->getInterfaceDecl();
 
   // Ignore any parens, implicit casts (should only be
   // array-to-pointer decays), and not-so-opaque values.  The last is
@@ -15252,15 +15246,41 @@ bool 
Sema::ConversionToObjCStringLiteralCheck(QualType DstType, Expr *,
 if (OV->getSourceExpr())
   SrcExpr = OV->getSourceExpr()->IgnoreParenImpCasts();
 
-  StringLiteral *SL = dyn_cast(SrcExpr);
-  if (!SL || !SL->isAscii())
-return false;
-  if (Diagnose) {
-Diag(SL->getBeginLoc(), diag::err_missing_atsign_prefix)
-<< FixItHint::CreateInsertion(SL->getBeginLoc(), "@");
-Exp = BuildObjCStringLiteral(SL->getBeginLoc(), SL).get();
+  if (auto *SL = dyn_cast(SrcExpr)) {
+if (!PT->isObjCIdType() &&
+!(ID && ID->getIdentifier()->isStr("NSString")))
+  return false;
+if (!SL->isAscii())
+  return false;
+
+if (Diagnose) {
+  Diag(SL->getBeginLoc(), diag::err_missing_atsign_prefix)
+  << /*string*/0 << 

[clang-tools-extra] 6529b0c - [clangd] Enable diagnostic fixes within macro argument expansions.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-04-20T21:18:31+02:00
New Revision: 6529b0c48aab83bdada1d21a79da13b0971d1aec

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

LOG: [clangd] Enable diagnostic fixes within macro argument expansions.

Summary: This seems like a pretty safe case, and common enough to be useful.

Reviewers: hokein

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, 
cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Diagnostics.cpp 
b/clang-tools-extra/clangd/Diagnostics.cpp
index 32f6a9bf776c..d72c2bd68ce8 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -556,10 +556,23 @@ void 
StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
 if (!InsideMainFile)
   return false;
 
+// Copy as we may modify the ranges.
+auto FixIts = Info.getFixItHints().vec();
 llvm::SmallVector Edits;
-for (auto  : Info.getFixItHints()) {
-  // Follow clang's behavior, don't apply FixIt to the code in macros,
-  // we are less certain it is the right fix.
+for (auto  : FixIts) {
+  // Allow fixits within a single macro-arg expansion to be applied.
+  // This can be incorrect if the argument is expanded multiple times in
+  // 
diff erent contexts. Hopefully this is rare!
+  if (FixIt.RemoveRange.getBegin().isMacroID() &&
+  FixIt.RemoveRange.getEnd().isMacroID() &&
+  SM.getFileID(FixIt.RemoveRange.getBegin()) ==
+  SM.getFileID(FixIt.RemoveRange.getEnd())) {
+FixIt.RemoveRange = CharSourceRange(
+{SM.getTopMacroCallerLoc(FixIt.RemoveRange.getBegin()),
+ SM.getTopMacroCallerLoc(FixIt.RemoveRange.getEnd())},
+FixIt.RemoveRange.isTokenRange());
+  }
+  // Otherwise, follow clang's behavior: no fixits in macros.
   if (FixIt.RemoveRange.getBegin().isMacroID() ||
   FixIt.RemoveRange.getEnd().isMacroID())
 return false;
@@ -570,8 +583,8 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level 
DiagLevel,
 
 llvm::SmallString<64> Message;
 // If requested and possible, create a message like "change 'foo' to 
'bar'".
-if (SyntheticMessage && Info.getNumFixItHints() == 1) {
-  const auto  = Info.getFixItHint(0);
+if (SyntheticMessage && FixIts.size() == 1) {
+  const auto  = FixIts.front();
   bool Invalid = false;
   llvm::StringRef Remove =
   Lexer::getSourceText(FixIt.RemoveRange, SM, *LangOpts, );

diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp 
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index f64f8e9901a9..026e145ed65d 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -104,6 +104,7 @@ TEST(DiagnosticsTest, DiagnosticRanges) {
   // Check we report correct ranges, including various edge-cases.
   Annotations Test(R"cpp(
 // error-ok
+#define ID(X) X
 namespace test{};
 void $decl[[foo]]();
 class T{$explicit[[]]$constructor[[T]](int a);};
@@ -116,6 +117,7 @@ o]]();
   struct Foo { int x; }; Foo a;
   a.$nomember[[y]];
   test::$nomembernamespace[[test]];
+  $macro[[ID($macroarg[[fod]])]]();
 }
   )cpp");
   auto TU = TestTU::withCode(Test.code());
@@ -144,6 +146,10 @@ o]]();
   Diag(Test.range("nomember"), "no member named 'y' in 'Foo'"),
   Diag(Test.range("nomembernamespace"),
"no member named 'test' in namespace 'test'"),
+  AllOf(Diag(Test.range("macro"),
+ "use of undeclared identifier 'fod'; did you mean 
'foo'?"),
+WithFix(Fix(Test.range("macroarg"), "foo",
+"change 'fod' to 'foo'"))),
   // We make sure here that the entire token is highlighted
   AllOf(Diag(Test.range("constructor"),
  "single-argument constructors must be marked explicit to "



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

[PATCH] D78501: Add a facility to get system cache directory and use it in clangd

[Vojtěch Štěpančík via Phabricator via cfe-commits]

VojtechStep updated this revision to Diff 258805.
VojtechStep edited the summary of this revision.
VojtechStep added a comment.

@sammccall I think I fixed all raised issues. Let me know if there is anything 
else I can do, otherwise feel free to commit it.

Also Note to self: Once this is pushed to Arch packages we should add Clangd to 
the Supported list of XDG Base Dir Spec support on Arch Wiki 
.


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

https://reviews.llvm.org/D78501

Files:
  clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
  llvm/include/llvm/Support/Path.h
  llvm/lib/Support/Unix/Path.inc
  llvm/lib/Support/Windows/Path.inc
  llvm/unittests/Support/Path.cpp

Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -358,6 +358,122 @@
 }
 #endif
 
+TEST(Support, CacheDirectoryWithEnv) {
+  std::string Expected;
+  // The value of the environment variable is set to a non-standard
+  // location for the test, so we have to store the original value
+  // even if it's not set.
+  // Use an std::string to copy the data
+#ifdef _WIN32
+  Optional OriginalStorage;
+  if (wchar_t const *path = ::_wgetenv(L"XDG_CACHE_HOME")) {
+OriginalStorage = path;
+  }
+
+  ::_wputenv("XDG_CACHE_HOME", L"C:\\xdg\\cache");
+
+  if (wchar_t const *localAppData = ::_wgetenv(L"LOCALAPPDATA")) {
+auto pathLen = ::wcslen(path);
+ArrayRef ref{reinterpret_cast(path),
+   pathLen * sizeof(wchar_t)};
+convertUTF16ToUTF8String(ref, Expected);
+  }
+
+  if (!Expected.empty()) {
+SmallString<128> CacheDir;
+auto status = path::cache_directory(CacheDir);
+EXPECT_TRUE(status);
+EXPECT_EQ(Expected, CacheDir);
+  }
+
+  if (OriginalStorage.hasValue()) {
+::_wputenv("XDG_CACHE_HOME", OriginalStorage->c_str());
+  } else {
+::_wputenv("XDG_CACHE_HOME", nullptr);
+  }
+#else
+  Optional OriginalStorage;
+  if (char const *path = ::getenv("XDG_CACHE_HOME")) {
+OriginalStorage = path;
+  }
+
+  Expected = "/xdg/cache";
+  ::setenv("XDG_CACHE_HOME", Expected.c_str(), 1);
+  EXPECT_EQ(Expected, std::string(::getenv("XDG_CACHE_HOME")));
+
+  SmallString<128> CacheDir;
+  auto status = path::cache_directory(CacheDir);
+  EXPECT_TRUE(status);
+  EXPECT_EQ(Expected, CacheDir);
+
+  if (OriginalStorage.hasValue()) {
+::setenv("XDG_CACHE_HOME", OriginalStorage->c_str(), 1);
+  } else {
+::unsetenv("XDG_CACHE_HOME");
+  }
+#endif
+}
+
+TEST(Support, CacheDirectoryNoEnv) {
+  std::string Expected;
+#ifdef _WIN32
+  Optional OriginalStorage;
+  if (wchar_t const *path = ::_wgetenv(L"XDG_CACHE_HOME")) {
+OriginalStorage = path;
+  }
+
+  ::_wputenv("XDG_CACHE_HOME", nullptr);
+  EXPECT_EQ(nullptr, ::_wgetenv("XDG_CACHE_HOME"));
+
+  if (wchar_t const *localAppData = ::_wgetenv(L"LOCALAPPDATA")) {
+auto pathLen = ::wcslen(path);
+ArrayRef ref {reinterpret_cast(path),
+   pathLen * sizeof(wchar_t)};
+convertUTF16ToUTF8String(ref, Expected);
+  }
+
+  // LocalAppData should always be set, but better safe than sorry
+  if (!Expected.empty()) {
+SmallString<128> CacheDir;
+auto status = path::cache_directory(CacheDir);
+EXPECT_TRUE(status);
+EXPECT_EQ(Expected, CacheDir);
+  }
+
+  if (OriginalStorage.hasValue()) {
+::_wputenv("XDG_CACHE_HOME", OriginalStorage->c_str());
+  } else {
+::_wputenv("XDG_CACHE_HOME", nullptr);
+  }
+#else
+  Optional OriginalStorage;
+  if (char const *path = ::getenv("XDG_CACHE_HOME")) {
+OriginalStorage = path;
+  }
+
+  ::unsetenv("XDG_CACHE_HOME");
+  EXPECT_EQ(nullptr, ::getenv("XDG_CACHE_HOME"));
+
+  SmallString<128> HomeDir;
+
+  auto home_status = path::home_directory(HomeDir);
+  EXPECT_TRUE(home_status);
+  path::append(HomeDir, ".cache");
+  Expected = HomeDir.str().str();
+
+  SmallString<128> CacheDir;
+  auto status = path::cache_directory(CacheDir);
+  EXPECT_TRUE(status);
+  EXPECT_EQ(Expected, CacheDir);
+
+  if (OriginalStorage.hasValue()) {
+::setenv("XDG_CACHE_HOME", OriginalStorage->c_str(), 1);
+  } else {
+::unsetenv("XDG_CACHE_HOME");
+  }
+#endif
+}
+
 TEST(Support, TempDirectory) {
   SmallString<32> TempDir;
   path::system_temp_directory(false, TempDir);
Index: llvm/lib/Support/Windows/Path.inc
===
--- llvm/lib/Support/Windows/Path.inc
+++ llvm/lib/Support/Windows/Path.inc
@@ -1372,6 +1372,10 @@
   return getKnownFolderPath(FOLDERID_Profile, result);
 }
 
+bool cache_directory(SmallVectorImpl ) {
+  return getKnownFolderPath(FOLDERID_LocalAppData, result);
+}
+
 static bool getTempDirEnvVar(const wchar_t *Var, SmallVectorImpl ) {
   SmallVector Buf;
   size_t Size = 1024;
Index: llvm/lib/Support/Unix/Path.inc

[PATCH] D78509: [AArch64][SVE] Add addressing mode for contiguous loads & stores

[Francesco Petrogalli via Phabricator via cfe-commits]

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

I think I made a mess with the Actions for this review! I mean to accept it, 
not to enforce the nit comments!

Francesco


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78509



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

[PATCH] D77776: [Driver] Default to libc++ on FreeBSD

[Ed Maste via Phabricator via cfe-commits]

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

Looks ok to me


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D6



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

[PATCH] D77776: [Driver] Default to libc++ on FreeBSD

[Ed Maste via Phabricator via cfe-commits]

emaste added a comment.

In D6#1981271 , @jbeich wrote:

> - Limit the scope to `-stdlib`
>
> In D6#1972543 , @dim wrote:
>
> > the lowest supported version, which is currently 10
>
>
> Where is this defined? Does someone build LLVM master on FreeBSD < 11.3?


Not certain, but there are a number of FreeBSD consumers still shipping 
products based on FreeBSD 10 who might be trying to use newer toolchains. I 
don't think we need to be too concerned about those cases but if it's trivial 
to avoid breaking them we might as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D6



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

[PATCH] D77156: [CMAKE] Plumb include_directories() into tablegen()

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.
Herald added a subscriber: frgossen.

I'm not sure this is actually the right approach. Adding the directory's 
`INCLUDE_DIRECTORIES` option also adds any path added with 
`include_directories`, which includes system paths and other locations that 
shouldn't ever have tablegen files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77156



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

[PATCH] D78509: [AArch64][SVE] Add addressing mode for contiguous loads & stores

[Francesco Petrogalli via Phabricator via cfe-commits]

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

Hi @kmclaughlin , thank you for working on this!

The patch LGTM, but please consider renaming the multiclass for the non 
faulting loads like I suggested. The rest of the feedback is mostly cosmetic 
changes!

Francesco




Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:1617
 
+  multiclass ld1nf {
+// scalar + immediate (mul vl)

The instruction mnemonic is `ldfn1*`, maybe we should call this multiclass 
`ldnf1` instead of `ld1nf`?



Comment at: 
llvm/test/CodeGen/AArch64/sve-intrinsics-ld1-addressing-mode-reg-reg.ll:7
+
+define  @ld1b_i8( %pg, i8* %a, i64 
%offset) {
+; CHECK-LABEL: ld1b_i8

Nit: I think you should rename the variable `%offset` to `%index` all through 
this tests, as usually we intend offset to represent bytes, while index is for 
indexes of arrays, which are scaled.



Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-ld1.ll:1
 ; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
 

For the sake of consistency, I think it is worth splitting this test into two 
tests. One that tests the pattern for the value of offset of 0 (you can leave 
the current name for the file), and one that tests all valid and invalid values 
for the reg+imm addressing mode, and call the test 
`sve-intrinsics-ld1-addressing-mode-reg-imm.ll`



Comment at: 
llvm/test/CodeGen/AArch64/sve-intrinsics-st1-addressing-mode-reg-reg.ll:7
+
+define void @st1b_i8( %data,  %pred, i8* 
%a, i64 %offset) {
+; CHECK-LABEL: st1b_i8:

Nit: same here. For the sake of consistency with other tests, I think you could 
rename `%offset` to `%index`.



Comment at: llvm/test/CodeGen/AArch64/sve-intrinsics-st1.ll:1
 ; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
 

Nit: Same for this test, I think it is worth splitting for the sake of 
consistency with other tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78509



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

[PATCH] D77229: [Analyzer][WIP] Avoid handling of LazyCompundVals in IteratorModeling

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 258798.
baloghadamsoftware added a comment.

I commented out lines which prevent creation of `StackFrameContext` for 
non-inlined functions. Now everything works regarding the iterator checkers. I 
also simplified things and created new methods for `CallEvent`: 
`getArgObject()` and `getReturnObject()`.

Two tests fail now: `temporaries.cpp` emits `TRUE` in lines `894` and `918` (I 
wonder whether they are correct) and `explain-svals.cpp` emits `lazily frozen 
compound value of parameter ''` in line `96` which is surely incorrect.

The main problem remains to solve is what we already discussed: either to find 
always the same `Decl` or create a new kind of region for arguments.


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

https://reviews.llvm.org/D77229

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/InvalidatedIteratorChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/Iterator.cpp
  clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/IteratorRangeChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MismatchedIteratorChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/STLAlgorithmModeling.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  clang/test/Analysis/container-modeling.cpp
  clang/test/Analysis/iterator-modeling.cpp

Index: clang/test/Analysis/iterator-modeling.cpp
===
--- clang/test/Analysis/iterator-modeling.cpp
+++ clang/test/Analysis/iterator-modeling.cpp
@@ -1862,7 +1862,7 @@
 void clang_analyzer_printState();
 
 void print_state(std::vector ) {
-  const auto i0 = V.cbegin();
+  auto i0 = V.cbegin();
   clang_analyzer_printState();
 
 // CHECK:  "checker_messages": [
@@ -1871,7 +1871,8 @@
 // CHECK-NEXT: "i0 : Valid ; Container == SymRegion{reg_$[[#]] & V>} ; Offset == conj_$[[#]]{long, LC[[#]], S[[#]], #[[#]]}"
 // CHECK-NEXT:   ]}
 
-  const auto i1 = V.cend();
+  ++i0;
+  auto i1 = V.cend();
   clang_analyzer_printState();
   
 // CHECK:  "checker_messages": [
@@ -1879,4 +1880,6 @@
 // CHECK-NEXT: "Iterator Positions :",
 // CHECK-NEXT: "i1 : Valid ; Container == SymRegion{reg_$[[#]] & V>} ; Offset == conj_$[[#]]{long, LC[[#]], S[[#]], #[[#]]}"
 // CHECK-NEXT:   ]}
+
+  --i1;
 }
Index: clang/test/Analysis/container-modeling.cpp
===
--- clang/test/Analysis/container-modeling.cpp
+++ clang/test/Analysis/container-modeling.cpp
@@ -17,7 +17,7 @@
 void clang_analyzer_warnIfReached();
 
 void begin(const std::vector ) {
-  V.begin();
+  const auto i0 = V.begin();
 
   clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
   clang_analyzer_express(clang_analyzer_container_begin(V)); // expected-warning{{$V.begin()}}
@@ -25,7 +25,7 @@
 }
 
 void end(const std::vector ) {
-  V.end();
+  const auto i0 = V.end();
 
   clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
   clang_analyzer_express(clang_analyzer_container_end(V)); // expected-warning{{$V.end()}}
@@ -41,10 +41,10 @@
 // Move
 
 void move_assignment(std::vector , std::vector ) {
-  V1.cbegin();
-  V1.cend();
-  V2.cbegin();
-  V2.cend();
+  const auto i0 = V1.cbegin();
+  const auto i1 = V1.cend();
+  const auto i2 = V2.cbegin();
+  const auto i3 = V2.cend();
   long B1 = clang_analyzer_container_begin(V1);
   long E1 = clang_analyzer_container_end(V1);
   long B2 = clang_analyzer_container_begin(V2);
@@ -70,8 +70,8 @@
 void clang_analyzer_dump(void*);
 
 void push_back(std::vector , int n) {
-  V.cbegin();
-  V.cend();
+  const auto i0 = V.cbegin();
+  const auto i1 = V.cend();
 
   clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
@@ -90,8 +90,8 @@
 /// past-the-end position of the container is incremented).
 
 void emplace_back(std::vector , int n) {
-  V.cbegin();
-  V.cend();
+  const auto i0 = V.cbegin();
+  const auto i1 = V.cend();
 
   clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
@@ -110,8 +110,8 @@
 /// past-the-end position of the container is decremented).
 
 void pop_back(std::vector , int n) {
-  V.cbegin();
-  V.cend();
+  const auto i0 = V.cbegin();
+  const auto i1 = V.cend();
 
   clang_analyzer_denote(clang_analyzer_container_begin(V), "$V.begin()");
   clang_analyzer_denote(clang_analyzer_container_end(V), "$V.end()");
@@ -131,8 +131,8 @@
 /// position of the container is decremented).
 
 void push_front(std::list 

[PATCH] D78390: [dfsan] Add "DataFlow" option to LLVM_USE_SANITIZER

[Zola Bridges via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0f12480bd13a: [dfsan] Add DataFlow option to 
LLVM_USE_SANITIZER (authored by zbrid).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78390

Files:
  clang/docs/DataFlowSanitizer.rst
  libcxx/CMakeLists.txt
  libcxx/utils/libcxx/test/config.py
  llvm/cmake/modules/HandleLLVMOptions.cmake
  llvm/docs/CMake.rst


Index: llvm/docs/CMake.rst
===
--- llvm/docs/CMake.rst
+++ llvm/docs/CMake.rst
@@ -422,7 +422,7 @@
 **LLVM_USE_SANITIZER**:STRING
   Define the sanitizer used to build LLVM binaries and tests. Possible values
   are ``Address``, ``Memory``, ``MemoryWithOrigins``, ``Undefined``, 
``Thread``,
-  and ``Address;Undefined``. Defaults to empty string.
+  ``DataFlow``, and ``Address;Undefined``. Defaults to empty string.
 
 **LLVM_ENABLE_LTO**:STRING
   Add ``-flto`` or ``-flto=`` flags to the compile and link command
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -728,6 +728,8 @@
 elseif (LLVM_USE_SANITIZER STREQUAL "Thread")
   append_common_sanitizer_flags()
   append("-fsanitize=thread" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+elseif (LLVM_USE_SANITIZER STREQUAL "DataFlow")
+  append("-fsanitize=dataflow" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 elseif (LLVM_USE_SANITIZER STREQUAL "Address;Undefined" OR
 LLVM_USE_SANITIZER STREQUAL "Undefined;Address")
   append_common_sanitizer_flags()
Index: libcxx/utils/libcxx/test/config.py
===
--- libcxx/utils/libcxx/test/config.py
+++ libcxx/utils/libcxx/test/config.py
@@ -907,6 +907,8 @@
 self.cxx.flags += ['-fsanitize=thread']
 self.config.available_features.add('tsan')
 self.config.available_features.add('sanitizer-new-delete')
+elif san == 'DataFlow':
+self.cxx.flags += ['-fsanitize=dataflow']
 else:
 self.lit_config.fatal('unsupported value for '
   'use_sanitizer: {0}'.format(san))
Index: libcxx/CMakeLists.txt
===
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -682,6 +682,8 @@
   append_flags(SANITIZER_FLAGS "-fsanitize=address,undefined 
-fno-sanitize=vptr,function -fno-sanitize-recover=all")
 elseif (USE_SANITIZER STREQUAL "Thread")
   append_flags(SANITIZER_FLAGS -fsanitize=thread)
+elseif (USE_SANITIZER STREQUAL "DataFlow")
+  append_flags(SANITIZER_FLAGS -fsanitize=dataflow)
 else()
   message(WARNING "Unsupported value of LLVM_USE_SANITIZER: 
${USE_SANITIZER}")
 endif()
Index: clang/docs/DataFlowSanitizer.rst
===
--- clang/docs/DataFlowSanitizer.rst
+++ clang/docs/DataFlowSanitizer.rst
@@ -20,6 +20,31 @@
 dynamic data flow analysis framework to be used by clients to help
 detect application-specific issues within their own code.
 
+How to build libc++ with DFSan
+==
+
+DFSan requires either all of your code to be instrumented or for uninstrumented
+functions to be listed as``uninstrumented`` in the `ABI list`_.
+
+If you'd like to have instrumented libc++ functions, then you need to build it
+with DFSan instrumentation from source. Here is an example of how to build
+libc++ and the libc++ ABI with data flow sanitizer instrumentation.
+
+.. code-block:: console
+  cd libcxx-build
+
+  # An example using ninja
+  cmake -GNinja path/to/llvm-project/llvm \
+-DCMAKE_C_COMPILER=clang \
+-DCMAKE_CXX_COMPILER=clang++ \
+-DLLVM_USE_SANITIZER="DataFlow" \
+-DLLVM_ENABLE_LIBCXX=ON \
+-DLLVM_ENABLE_PROJECTS="libcxx;libcxxabi"
+
+  ninja cxx cxxabi
+
+Note: Ensure you are building with a sufficiently new version of Clang.
+
 Usage
 =
 
@@ -33,6 +58,8 @@
 For further information about each function, please refer to the header
 file.
 
+.. _ABI list:
+
 ABI List
 
 


Index: llvm/docs/CMake.rst
===
--- llvm/docs/CMake.rst
+++ llvm/docs/CMake.rst
@@ -422,7 +422,7 @@
 **LLVM_USE_SANITIZER**:STRING
   Define the sanitizer used to build LLVM binaries and tests. Possible values
   are ``Address``, ``Memory``, ``MemoryWithOrigins``, ``Undefined``, ``Thread``,
-  and ``Address;Undefined``. Defaults to empty string.
+  ``DataFlow``, and ``Address;Undefined``. Defaults to empty string.
 
 **LLVM_ENABLE_LTO**:STRING
   Add ``-flto`` or ``-flto=`` flags to the compile and link command
Index: llvm/cmake/modules/HandleLLVMOptions.cmake

[PATCH] D78508: [Clang] Allow long as size_t printf argument on 32-bit Windows platforms.

[Jacek Caban via Phabricator via cfe-commits]

jacek created this revision.
jacek added a reviewer: clang.
Herald added subscribers: cfe-commits, mstorsjo.
Herald added a project: clang.

On 32-bit Windows platform, int and long is mixed all over the place in 
platform headers. The most notable example is SIZE_T, which is unsigned long, 
unlike size_t, which is unsigned int. %I, %z and %j formats do the right thing 
for those types, but clang issues a warning, leaving no good way to print 
SIZE_T (other than disabling warnings or adding useless casts).

GCC supports only %I for mingw targets, but allows both int and long to be used.

This is causing problems when using clang to build Wine PE files.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78508

Files:
  clang/lib/AST/FormatString.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/test/Sema/format-strings-ms.c


Index: clang/test/Sema/format-strings-ms.c
===
--- clang/test/Sema/format-strings-ms.c
+++ clang/test/Sema/format-strings-ms.c
@@ -21,17 +21,35 @@
 void signed_test() {
   short val = 30;
   printf("val = %I64d\n", val); // expected-warning{{format specifies type 
'__int64' (aka 'long long') but the argument has type 'short'}}
+  printf("val = %Id\n", val);
   long long bigval = 30;
   printf("val = %I32d\n", bigval); // expected-warning{{format specifies type 
'__int32' (aka 'int') but the argument has type 'long long'}}
   printf("val = %Id\n", bigval); // expected-warning{{format specifies type 
'__int32' (aka 'int') but the argument has type 'long long'}}
+  int ival = 30;
+  printf("%Id", ival);
+  printf("%zd", ival);
+  printf("%td", ival);
+  long lval = 30;
+  printf("%Id", lval);
+  printf("%zd", lval);
+  printf("%td", lval);
 }
 
 void unsigned_test() {
   unsigned short val = 30;
   printf("val = %I64u\n", val); // expected-warning{{format specifies type 
'unsigned __int64' (aka 'unsigned long long') but the argument has type 
'unsigned short'}}
+  printf("val = %Iu\n", val);
   unsigned long long bigval = 30;
   printf("val = %I32u\n", bigval); // expected-warning{{format specifies type 
'unsigned __int32' (aka 'unsigned int') but the argument has type 'unsigned 
long long'}}
   printf("val = %Iu\n", bigval); // expected-warning{{format specifies type 
'unsigned __int32' (aka 'unsigned int') but the argument has type 'unsigned 
long long'}}
+  unsigned int ival = 30;
+  printf("%Iu", ival);
+  printf("%zu", ival);
+  printf("%tu", ival);
+  unsigned long lval = 30;
+  printf("%Iu", lval);
+  printf("%zu", lval);
+  printf("%tu", lval);
 }
 
 void w_test(wchar_t c, wchar_t *s) {
Index: clang/lib/AST/PrintfFormatString.cpp
===
--- clang/lib/AST/PrintfFormatString.cpp
+++ clang/lib/AST/PrintfFormatString.cpp
@@ -527,8 +527,8 @@
 return ArgType::makeSizeT(ArgType(Ctx.getSignedSizeType(), "ssize_t"));
   case LengthModifier::AsInt3264:
 return Ctx.getTargetInfo().getTriple().isArch64Bit()
-   ? ArgType(Ctx.LongLongTy, "__int64")
-   : ArgType(Ctx.IntTy, "__int32");
+   ? ArgType::makeSizeT(ArgType(Ctx.LongLongTy, "__int64"))
+   : ArgType::makeSizeT(ArgType(Ctx.IntTy, "__int32"));
   case LengthModifier::AsPtrDiff:
 return ArgType::makePtrdiffT(
 ArgType(Ctx.getPointerDiffType(), "ptrdiff_t"));
@@ -562,8 +562,10 @@
 return ArgType::makeSizeT(ArgType(Ctx.getSizeType(), "size_t"));
   case LengthModifier::AsInt3264:
 return Ctx.getTargetInfo().getTriple().isArch64Bit()
-   ? ArgType(Ctx.UnsignedLongLongTy, "unsigned __int64")
-   : ArgType(Ctx.UnsignedIntTy, "unsigned __int32");
+   ? ArgType::makeSizeT(
+ ArgType(Ctx.UnsignedLongLongTy, "unsigned __int64"))
+   : ArgType::makeSizeT(
+ ArgType(Ctx.UnsignedIntTy, "unsigned __int32"));
   case LengthModifier::AsPtrDiff:
 return ArgType::makePtrdiffT(
 ArgType(Ctx.getUnsignedPointerDiffType(), "unsigned ptrdiff_t"));
Index: clang/lib/AST/FormatString.cpp
===
--- clang/lib/AST/FormatString.cpp
+++ clang/lib/AST/FormatString.cpp
@@ -401,8 +401,21 @@
   case BuiltinType::UInt:
 return T == C.IntTy ? Match : NoMatch;
   case BuiltinType::Long:
+// Win32 platform mixes long and int for size_t-like purposes.
+// ssize_t is int, but platform type SSIZE_T is long.
+if ((isSizeT() || isPtrdiffT()) &&
+C.getTargetInfo().getTriple().isOSMSVCRT() &&
+C.getTargetInfo().getTriple().isArch32Bit())
+  return Match;
 return T == C.UnsignedLongTy ? Match : NoMatch;
   case BuiltinType::ULong:
+// Win32 platform mixes long and int for size_t-like 

[PATCH] D78509: [AArch64][SVE] Add addressing mode for contiguous loads & stores

[Kerry McLaughlin via Phabricator via cfe-commits]

kmclaughlin created this revision.
kmclaughlin added reviewers: sdesmalen, fpetrogalli, efriedma.
Herald added subscribers: danielkiss, psnobl, rkruppe, hiraditya, 
kristof.beyls, tschuett.
Herald added a reviewer: rengolin.
Herald added a project: LLVM.

This patch adds the register + register addressing mode for
SVE contiguous load and store intrinsics (LD1 & ST1)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78509

Files:
  llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td
  llvm/test/CodeGen/AArch64/sve-intrinsics-ld1-addressing-mode-reg-reg.ll
  llvm/test/CodeGen/AArch64/sve-intrinsics-ld1.ll
  llvm/test/CodeGen/AArch64/sve-intrinsics-st1-addressing-mode-reg-reg.ll
  llvm/test/CodeGen/AArch64/sve-intrinsics-st1.ll

Index: llvm/test/CodeGen/AArch64/sve-intrinsics-st1.ll
===
--- llvm/test/CodeGen/AArch64/sve-intrinsics-st1.ll
+++ llvm/test/CodeGen/AArch64/sve-intrinsics-st1.ll
@@ -83,8 +83,7 @@
 define void @st1b_out_of_upper_bound( %data,  %pg, i8* %a) {
 ; CHECK-LABEL: st1b_out_of_upper_bound:
 ; CHECK: rdvl x[[OFFSET:[0-9]+]], #8
-; CHECK: add x[[BASE:[0-9]+]], x0, x[[OFFSET]]
-; CHECK: st1b { z0.b }, p0, [x[[BASE]]]
+; CHECK: st1b { z0.b }, p0, [x0, x[[OFFSET]]]
 ; CHECK-NEXT: ret
   %base_scalable = bitcast i8* %a to *
   %base = getelementptr , * %base_scalable, i64 8
@@ -96,8 +95,7 @@
 define void @st1b_out_of_lower_bound( %data,  %pg, i8* %a) {
 ; CHECK-LABEL: st1b_out_of_lower_bound:
 ; CHECK: rdvl x[[OFFSET:[0-9]+]], #-9
-; CHECK: add x[[BASE:[0-9]+]], x0, x[[OFFSET]]
-; CHECK: st1b { z0.b }, p0, [x[[BASE]]]
+; CHECK: st1b { z0.b }, p0, [x0, x[[OFFSET]]]
 ; CHECK-NEXT: ret
   %base_scalable = bitcast i8* %a to *
   %base = getelementptr , * %base_scalable, i64 -9
Index: llvm/test/CodeGen/AArch64/sve-intrinsics-st1-addressing-mode-reg-reg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/sve-intrinsics-st1-addressing-mode-reg-reg.ll
@@ -0,0 +1,184 @@
+; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s
+
+;
+; ST1B
+;
+
+define void @st1b_i8( %data,  %pred, i8* %a, i64 %offset) {
+; CHECK-LABEL: st1b_i8:
+; CHECK: st1b { z0.b }, p0, [x0, x1]
+; CHECK-NEXT: ret
+  %base = getelementptr i8, i8* %a, i64 %offset
+  call void @llvm.aarch64.sve.st1.nxv16i8( %data,
+   %pred,
+  i8* %base)
+  ret void
+}
+
+
+
+define void @st1b_h( %data,  %pred, i8* %a, i64 %offset) {
+; CHECK-LABEL: st1b_h:
+; CHECK: st1b { z0.h }, p0, [x0, x1]
+; CHECK-NEXT: ret
+  %base = getelementptr i8, i8* %a, i64 %offset
+  %trunc = trunc  %data to 
+  call void @llvm.aarch64.sve.st1.nxv8i8( %trunc,
+  %pred,
+ i8* %base)
+  ret void
+}
+
+define void @st1b_s( %data,  %pred, i8* %a, i64 %offset) {
+; CHECK-LABEL: st1b_s:
+; CHECK: st1b { z0.s }, p0, [x0, x1]
+; CHECK-NEXT: ret
+  %base = getelementptr i8, i8* %a, i64 %offset
+  %trunc = trunc  %data to 
+  call void @llvm.aarch64.sve.st1.nxv4i8( %trunc,
+  %pred,
+ i8* %base)
+  ret void
+}
+
+define void @st1b_d( %data,  %pred, i8* %a, i64 %offset) {
+; CHECK-LABEL: st1b_d:
+; CHECK: st1b { z0.d }, p0, [x0, x1]
+; CHECK-NEXT: ret
+  %base = getelementptr i8, i8* %a, i64 %offset
+  %trunc = trunc  %data to 
+  call void @llvm.aarch64.sve.st1.nxv2i8( %trunc,
+  %pred,
+ i8* %base)
+  ret void
+}
+
+;
+; ST1H
+;
+
+define void @st1h_i16( %data,  %pred, i16* %a, i64 %offset) {
+; CHECK-LABEL: st1h_i16:
+; CHECK: st1h { z0.h }, p0, [x0, x1, lsl #1]
+; CHECK-NEXT: ret
+  %base = getelementptr i16, i16* %a, i64 %offset
+  call void @llvm.aarch64.sve.st1.nxv8i16( %data,
+   %pred,
+  i16* %base)
+  ret void
+}
+
+define void @st1h_f16( %data,  %pred, half* %a, i64 %offset) {
+; CHECK-LABEL: st1h_f16:
+; CHECK: st1h { z0.h }, p0, [x0, x1, lsl #1]
+; CHECK-NEXT: ret
+  %base = getelementptr half, half* %a, i64 %offset
+  call void @llvm.aarch64.sve.st1.nxv8f16( %data,
+   %pred,
+  half* %base)
+  ret void
+}
+
+define void @st1h_s( %data,  %pred, i16* %addr) {
+; CHECK-LABEL: st1h_s:
+; CHECK: st1h { z0.s }, p0, [x0]
+; CHECK-NEXT: ret
+  %trunc = trunc  %data to 
+  call void @llvm.aarch64.sve.st1.nxv4i16( %trunc,
+  %pred,
+ i16* %addr)
+  ret void
+}
+
+define void @st1h_d( %data,  %pred, i16* %a, i64 %offset) {
+; CHECK-LABEL: st1h_d:
+; CHECK: st1h { z0.d }, p0, [x0, x1, lsl #1]
+; CHECK-NEXT: ret
+  %base = getelementptr i16, i16* %a, i64 %offset
+  %trunc = trunc  

[PATCH] D78390: [dfsan] Add "DataFlow" option to LLVM_USE_SANITIZER

[Zola Bridges via Phabricator via cfe-commits]

zbrid added a comment.

Bug: https://bugs.llvm.org/show_bug.cgi?id=45621


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78390



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

[PATCH] D78390: [dfsan] Add "DataFlow" option to LLVM_USE_SANITIZER

[Zola Bridges via Phabricator via cfe-commits]

zbrid updated this revision to Diff 258789.
zbrid added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78390

Files:
  clang/docs/DataFlowSanitizer.rst
  libcxx/CMakeLists.txt
  libcxx/utils/libcxx/test/config.py
  llvm/cmake/modules/HandleLLVMOptions.cmake
  llvm/docs/CMake.rst


Index: llvm/docs/CMake.rst
===
--- llvm/docs/CMake.rst
+++ llvm/docs/CMake.rst
@@ -422,7 +422,7 @@
 **LLVM_USE_SANITIZER**:STRING
   Define the sanitizer used to build LLVM binaries and tests. Possible values
   are ``Address``, ``Memory``, ``MemoryWithOrigins``, ``Undefined``, 
``Thread``,
-  and ``Address;Undefined``. Defaults to empty string.
+  ``DataFlow``, and ``Address;Undefined``. Defaults to empty string.
 
 **LLVM_ENABLE_LTO**:STRING
   Add ``-flto`` or ``-flto=`` flags to the compile and link command
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -728,6 +728,8 @@
 elseif (LLVM_USE_SANITIZER STREQUAL "Thread")
   append_common_sanitizer_flags()
   append("-fsanitize=thread" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+elseif (LLVM_USE_SANITIZER STREQUAL "DataFlow")
+  append("-fsanitize=dataflow" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 elseif (LLVM_USE_SANITIZER STREQUAL "Address;Undefined" OR
 LLVM_USE_SANITIZER STREQUAL "Undefined;Address")
   append_common_sanitizer_flags()
Index: libcxx/utils/libcxx/test/config.py
===
--- libcxx/utils/libcxx/test/config.py
+++ libcxx/utils/libcxx/test/config.py
@@ -907,6 +907,8 @@
 self.cxx.flags += ['-fsanitize=thread']
 self.config.available_features.add('tsan')
 self.config.available_features.add('sanitizer-new-delete')
+elif san == 'DataFlow':
+self.cxx.flags += ['-fsanitize=dataflow']
 else:
 self.lit_config.fatal('unsupported value for '
   'use_sanitizer: {0}'.format(san))
Index: libcxx/CMakeLists.txt
===
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -682,6 +682,8 @@
   append_flags(SANITIZER_FLAGS "-fsanitize=address,undefined 
-fno-sanitize=vptr,function -fno-sanitize-recover=all")
 elseif (USE_SANITIZER STREQUAL "Thread")
   append_flags(SANITIZER_FLAGS -fsanitize=thread)
+elseif (USE_SANITIZER STREQUAL "DataFlow")
+  append_flags(SANITIZER_FLAGS -fsanitize=dataflow)
 else()
   message(WARNING "Unsupported value of LLVM_USE_SANITIZER: 
${USE_SANITIZER}")
 endif()
Index: clang/docs/DataFlowSanitizer.rst
===
--- clang/docs/DataFlowSanitizer.rst
+++ clang/docs/DataFlowSanitizer.rst
@@ -20,6 +20,31 @@
 dynamic data flow analysis framework to be used by clients to help
 detect application-specific issues within their own code.
 
+How to build libc++ with DFSan
+==
+
+DFSan requires either all of your code to be instrumented or for uninstrumented
+functions to be listed as``uninstrumented`` in the `ABI list`_.
+
+If you'd like to have instrumented libc++ functions, then you need to build it
+with DFSan instrumentation from source. Here is an example of how to build
+libc++ and the libc++ ABI with data flow sanitizer instrumentation.
+
+.. code-block:: console
+  cd libcxx-build
+
+  # An example using ninja
+  cmake -GNinja path/to/llvm-project/llvm \
+-DCMAKE_C_COMPILER=clang \
+-DCMAKE_CXX_COMPILER=clang++ \
+-DLLVM_USE_SANITIZER="DataFlow" \
+-DLLVM_ENABLE_LIBCXX=ON \
+-DLLVM_ENABLE_PROJECTS="libcxx;libcxxabi"
+
+  ninja cxx cxxabi
+
+Note: Ensure you are building with a sufficiently new version of Clang.
+
 Usage
 =
 
@@ -33,6 +58,8 @@
 For further information about each function, please refer to the header
 file.
 
+.. _ABI list:
+
 ABI List
 
 


Index: llvm/docs/CMake.rst
===
--- llvm/docs/CMake.rst
+++ llvm/docs/CMake.rst
@@ -422,7 +422,7 @@
 **LLVM_USE_SANITIZER**:STRING
   Define the sanitizer used to build LLVM binaries and tests. Possible values
   are ``Address``, ``Memory``, ``MemoryWithOrigins``, ``Undefined``, ``Thread``,
-  and ``Address;Undefined``. Defaults to empty string.
+  ``DataFlow``, and ``Address;Undefined``. Defaults to empty string.
 
 **LLVM_ENABLE_LTO**:STRING
   Add ``-flto`` or ``-flto=`` flags to the compile and link command
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ 

[clang] 9b2ab41 - Revert "[MS] Fix assert handling enum forward decls in hasVisibleDefinition"

[Rumeet Dhindsa via cfe-commits]

Author: Rumeet Dhindsa
Date: 2020-04-20T10:40:27-07:00
New Revision: 9b2ab41037f45ad92ab4e850591093ffc45d3e10

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

LOG: Revert "[MS] Fix assert handling enum forward decls in 
hasVisibleDefinition"

This reverts commit e62dc1f6252c1dcdcc2a64e8e3b07a32412e9d89.

Reverting as per discussion with the patch author.
This patch causes module import error, but there was no intended
behavior change for code that does not use Microsoft extensions.

Added: 


Modified: 
clang/lib/Sema/SemaType.cpp

Removed: 
clang/test/Modules/Inputs/ms-enums/A.h
clang/test/Modules/Inputs/ms-enums/B.h
clang/test/Modules/Inputs/ms-enums/module.map
clang/test/Modules/ms-enums.cpp



diff  --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 4ecd36209e5b..075c30f88b3f 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -8120,10 +8120,10 @@ bool Sema::hasVisibleDefinition(NamedDecl *D, NamedDecl 
**Suggested,
   } else if (auto *ED = dyn_cast(D)) {
 if (auto *Pattern = ED->getTemplateInstantiationPattern())
   ED = Pattern;
-if (OnlyNeedComplete && !ED->getIntegerType().isNull()) {
-  // If the enum has an integer type, it may have been forward declared.
-  // Since we're only looking for a complete type (not a definition), any
-  // visible declaration of it will do.
+if (OnlyNeedComplete && ED->isFixed()) {
+  // If the enum has a fixed underlying type, and we're only looking for a
+  // complete type (not a definition), any visible declaration of it will
+  // do.
   *Suggested = nullptr;
   for (auto *Redecl : ED->redecls()) {
 if (isVisible(Redecl))

diff  --git a/clang/test/Modules/Inputs/ms-enums/A.h 
b/clang/test/Modules/Inputs/ms-enums/A.h
deleted file mode 100644
index 168445221c03..
--- a/clang/test/Modules/Inputs/ms-enums/A.h
+++ /dev/null
@@ -1 +0,0 @@
-enum fwd_enum;

diff  --git a/clang/test/Modules/Inputs/ms-enums/B.h 
b/clang/test/Modules/Inputs/ms-enums/B.h
deleted file mode 100644
index 7a13ba4d72d4..
--- a/clang/test/Modules/Inputs/ms-enums/B.h
+++ /dev/null
@@ -1 +0,0 @@
-#include "A.h"

diff  --git a/clang/test/Modules/Inputs/ms-enums/module.map 
b/clang/test/Modules/Inputs/ms-enums/module.map
deleted file mode 100644
index d9aed01430c4..
--- a/clang/test/Modules/Inputs/ms-enums/module.map
+++ /dev/null
@@ -1,2 +0,0 @@
-module A { header "A.h" }
-module B { header "B.h" }

diff  --git a/clang/test/Modules/ms-enums.cpp b/clang/test/Modules/ms-enums.cpp
deleted file mode 100644
index b3a377c6fa63..
--- a/clang/test/Modules/ms-enums.cpp
+++ /dev/null
@@ -1,12 +0,0 @@
-// RUN: rm -rf %t
-// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions 
-fms-compatibility -x c++ -std=c++20 -fmodules-cache-path=%t -fmodules 
-fimplicit-module-maps -I %S/Inputs/ms-enums %s -verify 
-fno-modules-error-recovery
-
-#include "B.h"
-// expected-note@A.h:1 {{previous declaration is here}}
-// expected-note@A.h:1 2 {{previous definition is here}}
-
-fwd_enum gv_enum; // expected-error {{must be imported}}
-
-struct Foo {
-  enum fwd_enum enum_field; // expected-error 2 {{must be imported}}
-};



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

[clang] 0f12480 - [dfsan] Add "DataFlow" option to LLVM_USE_SANITIZER

[Zola Bridges via cfe-commits]

Author: Zola Bridges
Date: 2020-04-20T10:30:52-07:00
New Revision: 0f12480bd13ad2d08b6ef3d64388dd8a12e1a503

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

LOG: [dfsan] Add "DataFlow" option to LLVM_USE_SANITIZER

Summary:
This patch add the dataflow option to LLVM_USE_SANITIZER and documents
it.

Tested via check-cxx (wip to fix the errors).

Reviewers: morehouse, #libc!

Subscribers: mgorny, cfe-commits, libcxx-commits

Tags: #clang, #libc

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

Added: 


Modified: 
clang/docs/DataFlowSanitizer.rst
libcxx/CMakeLists.txt
libcxx/utils/libcxx/test/config.py
llvm/cmake/modules/HandleLLVMOptions.cmake
llvm/docs/CMake.rst

Removed: 




diff  --git a/clang/docs/DataFlowSanitizer.rst 
b/clang/docs/DataFlowSanitizer.rst
index e0e9d74efde9..cc9b8e6e1699 100644
--- a/clang/docs/DataFlowSanitizer.rst
+++ b/clang/docs/DataFlowSanitizer.rst
@@ -20,6 +20,31 @@ specific class of bugs on its own.  Instead, it provides a 
generic
 dynamic data flow analysis framework to be used by clients to help
 detect application-specific issues within their own code.
 
+How to build libc++ with DFSan
+==
+
+DFSan requires either all of your code to be instrumented or for uninstrumented
+functions to be listed as``uninstrumented`` in the `ABI list`_.
+
+If you'd like to have instrumented libc++ functions, then you need to build it
+with DFSan instrumentation from source. Here is an example of how to build
+libc++ and the libc++ ABI with data flow sanitizer instrumentation.
+
+.. code-block:: console
+  cd libcxx-build
+
+  # An example using ninja
+  cmake -GNinja path/to/llvm-project/llvm \
+-DCMAKE_C_COMPILER=clang \
+-DCMAKE_CXX_COMPILER=clang++ \
+-DLLVM_USE_SANITIZER="DataFlow" \
+-DLLVM_ENABLE_LIBCXX=ON \
+-DLLVM_ENABLE_PROJECTS="libcxx;libcxxabi"
+
+  ninja cxx cxxabi
+
+Note: Ensure you are building with a sufficiently new version of Clang.
+
 Usage
 =
 
@@ -33,6 +58,8 @@ The APIs are defined in the header file 
``sanitizer/dfsan_interface.h``.
 For further information about each function, please refer to the header
 file.
 
+.. _ABI list:
+
 ABI List
 
 

diff  --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index cab3f7b14036..b05cad79d76a 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -682,6 +682,8 @@ function(get_sanitizer_flags OUT_VAR  USE_SANITIZER)
   append_flags(SANITIZER_FLAGS "-fsanitize=address,undefined 
-fno-sanitize=vptr,function -fno-sanitize-recover=all")
 elseif (USE_SANITIZER STREQUAL "Thread")
   append_flags(SANITIZER_FLAGS -fsanitize=thread)
+elseif (USE_SANITIZER STREQUAL "DataFlow")
+  append_flags(SANITIZER_FLAGS -fsanitize=dataflow)
 else()
   message(WARNING "Unsupported value of LLVM_USE_SANITIZER: 
${USE_SANITIZER}")
 endif()

diff  --git a/libcxx/utils/libcxx/test/config.py 
b/libcxx/utils/libcxx/test/config.py
index c31a47fb4f5d..ce77ec8c71c9 100644
--- a/libcxx/utils/libcxx/test/config.py
+++ b/libcxx/utils/libcxx/test/config.py
@@ -907,6 +907,8 @@ def add_ubsan():
 self.cxx.flags += ['-fsanitize=thread']
 self.config.available_features.add('tsan')
 self.config.available_features.add('sanitizer-new-delete')
+elif san == 'DataFlow':
+self.cxx.flags += ['-fsanitize=dataflow']
 else:
 self.lit_config.fatal('unsupported value for '
   'use_sanitizer: {0}'.format(san))

diff  --git a/llvm/cmake/modules/HandleLLVMOptions.cmake 
b/llvm/cmake/modules/HandleLLVMOptions.cmake
index 0c5f4e08aaba..91133d00782d 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -728,6 +728,8 @@ if(LLVM_USE_SANITIZER)
 elseif (LLVM_USE_SANITIZER STREQUAL "Thread")
   append_common_sanitizer_flags()
   append("-fsanitize=thread" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+elseif (LLVM_USE_SANITIZER STREQUAL "DataFlow")
+  append("-fsanitize=dataflow" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
 elseif (LLVM_USE_SANITIZER STREQUAL "Address;Undefined" OR
 LLVM_USE_SANITIZER STREQUAL "Undefined;Address")
   append_common_sanitizer_flags()

diff  --git a/llvm/docs/CMake.rst b/llvm/docs/CMake.rst
index 32d2ebdfc2c2..b8686b6d786e 100644
--- a/llvm/docs/CMake.rst
+++ b/llvm/docs/CMake.rst
@@ -422,7 +422,7 @@ LLVM-specific variables
 **LLVM_USE_SANITIZER**:STRING
   Define the sanitizer used to build LLVM binaries and tests. Possible values
   are ``Address``, ``Memory``, ``MemoryWithOrigins``, ``Undefined``, 
``Thread``,
-  and ``Address;Undefined``. Defaults to empty string.
+  ``DataFlow``, and 

[PATCH] D78390: [dfsan] Add "DataFlow" option to LLVM_USE_SANITIZER

[Zola Bridges via Phabricator via cfe-commits]

zbrid added a comment.

Gonna land this and file a bug for the failing tests. The tests shouldn't block 
anyone upstream since I'm only adding a build mode. I'll do some more digging 
into those failures in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78390



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

[PATCH] D78506: [NFC] Common up TargetCodeGenInfo for all PowerPC target.

[Jason Liu via Phabricator via cfe-commits]

jasonliu added a comment.

To reviewers:
Although the patch is showing changes on various ABIInfo classes. But those 
classes are actually untouched.
This patch only touches PowerPC TargetCodeGenInfo related classes.


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

https://reviews.llvm.org/D78506



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

[PATCH] D78286: [analyzer] Track runtime types represented by Obj-C Class objects

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:198
+  // 'self' variable of the current class method.
+  if (ReceiverSVal == Message.getSelfSVal()) {
+// In this case, we should return the type of the enclosing class

NoQ wrote:
> vsavchenko wrote:
> > NoQ wrote:
> > > vsavchenko wrote:
> > > > NoQ wrote:
> > > > > I believe this is pretty much always the case. At least whenever 
> > > > > `getInstanceReceiver()` is available. Another exception seem to be 
> > > > > when `ReceiverSVal` is an `UnknownVal` (in this case `self` is going 
> > > > > to be `SymbolRegionValue` because it's never set in the Store), but 
> > > > > that's it. I inferred this by looking at 
> > > > > `ObjCMethodCall::getInitialStackFrameContents()`.
> > > > > 
> > > > > I think we should have used `getSelfSVal()` to begin with.
> > > > > I believe this is pretty much always the case.
> > > > 
> > > > I didn't quite get what you mean by that
> > > > 
> > > > 
> > > What i'm trying to say is that `C.getSVal(RecE)` and 
> > > `Message.getSelfSVal()` and `Message.getReceiverSVal()` are basically the 
> > > same `SVal`. It shouldn't be necessary to check both or check whether 
> > > they're the same; you must have meant to check for something else, 
> > > probably something purely syntactic.
> > > 
> > > 
> > > 
> > > > I inferred this by looking at 
> > > > ObjCMethodCall::getInitialStackFrameContents().
> > > 
> > > Wait, so it's only true for inlined methods. For non-inlined methods 
> > > `getSelfSVal()` will be unknown :/
> > Yeah, that might be a bit extraneous to do it with `SVal`s, but this code 
> > for sure does its job (it is definitely not a redundant check). 
> > `getSelfSVal()` returns receiver of the function //containing// the call 
> > and not the call itself. So, it does check if we the receiver of the 
> > message is `self`.
> > 
> > I changed it to this way of doing things because it is consistent with how 
> > the same thing is done in `getRuntimeDefinition`.
> > `getSelfSVal()` returns receiver of the function containing the call and 
> > not the call itself
> 
>  looks broken to me.
Let's rename `getSelfSVal()` so that it screamed that it's the callee's self as 
opposed to the caller's self, and explain in a long comment why do we even care 
about the caller's self. I.e., that we heuristically assume that if a class 
method jumps into another class method on the same class object, it's going to 
be devirtualized to the same class. Which isn't always the case, hence !Precise.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78286



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

[PATCH] D73290: [PowerPC] Add clang -msvr4-struct-return for 32-bit ELF

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai accepted this revision.
nemanjai added a comment.

Aside from a couple of minor nits that shouldn't require another review, LGTM.




Comment at: clang/docs/ClangCommandLineReference.rst:2631
+
+Override the default ABI for 32-bit targets to return small structs in
+registers, as in the System V ABI (1995).

Can you specify that "small" means 8 bytes or smaller?



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4378
+const llvm::Triple , const CodeGenOptions ) {
+  assert(Triple.getArch() == llvm::Triple::ppc);
+

Please add text to the assert to help anyone who ends up tripping it. Perhaps:
```
assert(Triple.getArch() == llvm::Triple::ppc &&
   "Invalid triple for a 32-bit PowerPC target");
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73290



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

[PATCH] D74387: [SYCL] Defer __float128 type usage diagnostics

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

Okay, seems like OpenMP needs unsupported types diagnosing as well. I'm trying 
to adapt this patch for OpenMP, but it doesn't work out of the box because it 
diagnoses memcpy like operations, so with the current patch the code like this 
will cause diagnostics:

   struct T {
 char a;
 __float128 f;
 char c;
 T() : a(12), f(15) {}
  }
  
  #pragma omp declare target
  T a = T();
  #pragma omp end declare target

It happens because member initialization in the constructor is still usage of 
`f` field which is marked unavailable because of type. I'm not sure that it is 
possible to understand how the unavailable declaration is used in the place 
where diagnostic about usage of unavailable declaration is actually emitted, so 
I will probably need some other place/solution for it.

@jdoerfert , could you please help to understand how the diagnostic should work 
for OpenMP cases? Or you probably have some spec/requirements for it?
Understanding what exactly is needed will help with the implementation, I guess.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74387



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

[PATCH] D78506: [NFC] Common up TargetCodeGenInfo for all PowerPC target.

[Jason Liu via Phabricator via cfe-commits]

jasonliu created this revision.
jasonliu added reviewers: sfertile, nemanjai, hubert.reinterpretcast, 
cebowleratibm.
Herald added subscribers: steven.zhang, shchenz.
jasonliu added a reviewer: PowerPC.
jasonliu added a comment.

To reviewers:
Although the patch is showing changes on various ABIInfo classes. But those 
classes are actually untouched.
This patch only touches PowerPC TargetCodeGenInfo related classes.


There is little differences across all power pc related target for 
TargetCodeGenInfo implementation.
The biggest difference is the different ABIInfo classes it takes. 
But we don't necessarily need so many TargetCodeGenInfo for different PowerPC 
architect. 
So it might be beneficial to common them up as one PPCTargetCodeGenInfo.


https://reviews.llvm.org/D78506

Files:
  clang/lib/CodeGen/TargetInfo.cpp

Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -4172,8 +4172,9 @@
   /*allowHigherAlign*/ false);
 }
 
-// PowerPC-32
+// PowerPC
 namespace {
+// PowerPC-32
 /// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information.
 class PPC32_SVR4_ABIInfo : public DefaultABIInfo {
   bool IsSoftFloatABI;
@@ -4188,10 +4189,136 @@
 QualType Ty) const override;
 };
 
-class PPC32TargetCodeGenInfo : public TargetCodeGenInfo {
+// PowerPC-64
+/// PPC64_SVR4_ABIInfo - The 64-bit PowerPC ELF (SVR4) ABI information.
+class PPC64_SVR4_ABIInfo : public SwiftABIInfo {
+public:
+  enum ABIKind {
+ELFv1 = 0,
+ELFv2
+  };
+
+private:
+  static const unsigned GPRBits = 64;
+  ABIKind Kind;
+  bool HasQPX;
+  bool IsSoftFloatABI;
+
+  // A vector of float or double will be promoted to <4 x f32> or <4 x f64> and
+  // will be passed in a QPX register.
+  bool IsQPXVectorTy(const Type *Ty) const {
+if (!HasQPX)
+  return false;
+
+if (const VectorType *VT = Ty->getAs()) {
+  unsigned NumElements = VT->getNumElements();
+  if (NumElements == 1)
+return false;
+
+  if (VT->getElementType()->isSpecificBuiltinType(BuiltinType::Double)) {
+if (getContext().getTypeSize(Ty) <= 256)
+  return true;
+  } else if (VT->getElementType()->
+   isSpecificBuiltinType(BuiltinType::Float)) {
+if (getContext().getTypeSize(Ty) <= 128)
+  return true;
+  }
+}
+
+return false;
+  }
+
+  bool IsQPXVectorTy(QualType Ty) const {
+return IsQPXVectorTy(Ty.getTypePtr());
+  }
+
+public:
+  PPC64_SVR4_ABIInfo(CodeGen::CodeGenTypes , ABIKind Kind, bool HasQPX,
+ bool SoftFloatABI)
+  : SwiftABIInfo(CGT), Kind(Kind), HasQPX(HasQPX),
+IsSoftFloatABI(SoftFloatABI) {}
+
+  bool isPromotableTypeForABI(QualType Ty) const;
+  CharUnits getParamTypeAlignment(QualType Ty) const;
+
+  ABIArgInfo classifyReturnType(QualType RetTy) const;
+  ABIArgInfo classifyArgumentType(QualType Ty) const;
+
+  bool isHomogeneousAggregateBaseType(QualType Ty) const override;
+  bool isHomogeneousAggregateSmallEnough(const Type *Ty,
+ uint64_t Members) const override;
+
+  // TODO: We can add more logic to computeInfo to improve performance.
+  // Example: For aggregate arguments that fit in a register, we could
+  // use getDirectInReg (as is done below for structs containing a single
+  // floating-point value) to avoid pushing them to memory on function
+  // entry.  This would require changing the logic in PPCISelLowering
+  // when lowering the parameters in the caller and args in the callee.
+  void computeInfo(CGFunctionInfo ) const override {
+if (!getCXXABI().classifyReturnType(FI))
+  FI.getReturnInfo() = classifyReturnType(FI.getReturnType());
+for (auto  : FI.arguments()) {
+  // We rely on the default argument classification for the most part.
+  // One exception:  An aggregate containing a single floating-point
+  // or vector item must be passed in a register if one is available.
+  const Type *T = isSingleElementStruct(I.type, getContext());
+  if (T) {
+const BuiltinType *BT = T->getAs();
+if (IsQPXVectorTy(T) ||
+(T->isVectorType() && getContext().getTypeSize(T) == 128) ||
+(BT && BT->isFloatingPoint())) {
+  QualType QT(T, 0);
+  I.info = ABIArgInfo::getDirectInReg(CGT.ConvertType(QT));
+  continue;
+}
+  }
+  I.info = classifyArgumentType(I.type);
+}
+  }
+
+  Address EmitVAArg(CodeGenFunction , Address VAListAddr,
+QualType Ty) const override;
+
+  bool shouldPassIndirectlyForSwift(ArrayRef scalars,
+bool asReturnValue) const override {
+return occupiesMoreThan(CGT, scalars, /*total*/ 4);
+  }
+
+  bool isSwiftErrorInRegister() const override {
+return false;
+  }
+};
+
+class PPCTargetCodeGenInfo : public 

[PATCH] D78491: Avoid relying on address space zero default parameter in llvm/IR

[Ayke via Phabricator via cfe-commits]

aykevl added a comment.

I can't give an LGTM but at least from my perspective I very much welcome this 
change. I am still hitting problems with non-zero address spaces on AVR. One 
nit in an inline comment.

Do you plan on eventually removing `LLVM_NO_IMPLICIT_ADDRESS_SPACE` when the 
transition is finished? If so, I think it's worth documenting this somewhere - 
perhaps at llvm/lib/IR/CMakeLists.txt.




Comment at: llvm/include/llvm/IR/DataLayout.h:356
   /// Layout pointer alignment
-  Align getPointerABIAlignment(unsigned AS) const;
 

There is no default address space in this declaration?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78491



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

[PATCH] D77221: [AVR] Rework MCU family detection to support more AVR MCUs

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Patch generally looks good, but I agree that it could use some basic tests.  
You don't need to do specifically test all 20 million releases, but make sure 
you cover the first and last in the array as well as something in the middle, 
just to make sure your scans are working right.




Comment at: llvm/include/llvm/Support/AVRTargetParser.h:27
+  int Number;
+};
+

I think the relationship would be clearer if this type were named 
`MCUFamilyInfo`.

More generally, these types and fields could use some comments, like 
"Information about a specific AVR microcontroller release" or "Information 
about a family of AVR microcontrollers" or "Some sort of number that goes 
somewhere and has some kind of meaning, maybe it's reported by hardware or 
something, I dunno".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77221



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

[PATCH] D78000: [ASTImporter] Fix handling of not defined FromRecord in ImportContext(...)

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

Beside Gabors comment I think I'm happy with this. Thanks!




Comment at: clang/unittests/AST/ASTImporterTest.cpp:5939
+
+/// An ExternalASTSource that keeps track of the tags is completed.
+struct SourceWithCompletedTagList : clang::ExternalASTSource {

"is completed" -> "it completed"



Comment at: clang/unittests/AST/ASTImporterTest.cpp:5954
+   SmallVectorImpl ) override {
+DC->setHasExternalLexicalStorage(true);
+  }

You can remove this as you changed the check in the ASTImporter to only check 
for the existence of an ExternalASTSource. 


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

https://reviews.llvm.org/D78000



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

[PATCH] D77735: [SveEmitter] Implement builtins for gathers/scatters

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7451
+  }
+  llvm_unreachable("Unknown MemEltType");
+}

SjoerdMeijer wrote:
> nit: to be consistent, do this in the default clause?
Doing that would lead to 

  warning: default label in switch which covers all enumeration values 
[-Wcovered-switch-default]


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77735



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

[PATCH] D78501: Add a facility to get system cache directory and use it in clangd

[Sam McCall via Phabricator via cfe-commits]

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

Thanks! Looks good with a few nits.

Once you're done, let me know if I should land this for you (after a few 
patches landed this way you can apply for commit access: 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access)

You can add "Fixes https://github.com/clangd/clangd/issues/341; to the 
description to close the associated bug.

Note to self: maybe we should add this to the release notes so people can 
delete ~/.clangd.




Comment at: llvm/include/llvm/Support/Path.h:372
+/// Get the directory where installed packages should put their
+/// machine-local cache.
+///

I'd add "e.g. $XDG_CACHE_HOME" to this comment. (No need to spell out the 
fallback or windows behavior, but this gives a bit more of a hint)



Comment at: llvm/include/llvm/Support/Path.h:375
+/// @param result Holds the resulting path name.
+/// @result True if the appropriate directory was found.
+bool cache_directory(SmallVectorImpl );

This kind of implies you check that it exists (which it may not, particularly 
on the ~/.cache fallback). Avoiding IO seems like the right thing, but I'd 
mention in the function description that the returned path may not exist yet.



Comment at: llvm/lib/Support/Unix/Path.inc:1142
+bool cache_directory(SmallVectorImpl ) {
+  char *RequestedDir = getenv("XDG_CACHE_HOME");
+  if (RequestedDir) {

nit: suggest const char* to signal we're not doing anything bad with this 
terrifying API :-)



Comment at: llvm/lib/Support/Unix/Path.inc:1143
+  char *RequestedDir = getenv("XDG_CACHE_HOME");
+  if (RequestedDir) {
+result.clear();

nit: llvm style is to inline this `if (char * RequestedDir = ...)`



Comment at: llvm/unittests/Support/Path.cpp:363
+
+  std::string expected;
+

nit: Expected
(uppercase for vars in llvm style. The style in Path.h/.inc is very old/mixed 
up and you've done a great job being consistent, but at least for local vars in 
the cpp file we should follow the guidelines)



Comment at: llvm/unittests/Support/Path.cpp:426
+#ifdef _WIN32
+  std::wstring OriginalStorage;
+  if (wchar_t const *path = ::_wgetenv(L"XDG_CACHE_HOME")) {

nit: use an llvm::Optional so we correctly distinguish unset vs 
empty?, and elsewhere

(I guess we should extract a RAII environment-restorer for this file, but not 
going to ask you to boil that ocean :-))



Comment at: llvm/unittests/Support/Path.cpp:464
+  SmallString<128> HomeDir;
+  if (path::home_directory(HomeDir)) {
+path::append(HomeDir, ".cache");

you can just ASSERT_TRUE, this should always succeed in our test setups I think.



Comment at: llvm/unittests/Support/Path.cpp:469
+
+  if (!expected.empty()) {
+SmallString<128> CacheDir;

Why are we testing this twice, once conditionally?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78501



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

[PATCH] D78501: Add a facility to get system cache directory and use it in clangd

[Vojtěch Štěpančík via Phabricator via cfe-commits]

VojtechStep created this revision.
VojtechStep added reviewers: sammccall, chandlerc, Bigcheese.
VojtechStep added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, usaxena95, ormris, kadircet, arphaman, 
dexonsmith, jkorous, MaskRay, ilya-biryukov, hiraditya.
Herald added a project: clang.

This patch adds a function that is similar to 
`llvm::sys::path::home_directory`, but provides access to the system cache 
directory.

For Windows, that is %LOCALAPPDATA%, and applications should put their files 
under %LOCALAPPDATA%\Organization\Product\.

For *nixes, it adheres to the XDG Base Directory Specification, so it first 
looks at the XDG_CACHE_HOME environment variable and falls back to ~/.cache/.

Subsequently, the Clangd Index storage leverages this new API to put index 
files somewhere else than the users home directory.

Previous discussion is here 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78501

Files:
  clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
  llvm/include/llvm/Support/Path.h
  llvm/lib/Support/Unix/Path.inc
  llvm/lib/Support/Windows/Path.inc
  llvm/unittests/Support/Path.cpp

Index: llvm/unittests/Support/Path.cpp
===
--- llvm/unittests/Support/Path.cpp
+++ llvm/unittests/Support/Path.cpp
@@ -358,6 +358,136 @@
 }
 #endif
 
+TEST(Support, CacheDirectoryWithEnv) {
+
+  std::string expected;
+
+  // The value of the environment variable is set to a non-standard
+  // location for the test, so we have to store the original value
+  // even if it's not set.
+  // Use an std::string to copy the data
+
+#ifdef _WIN32
+  std::wstring OriginalStorage;
+  if (wchar_t const *path = ::_wgetenv(L"XDG_CACHE_HOME")) {
+OriginalStorage = path;
+  }
+
+  ::_wputenv("XDG_CACHE_HOME", L"C:\\xdg\\cache");
+
+  if (wchar_t const *localAppData = ::_wgetenv(L"LOCALAPPDATA")) {
+auto pathLen = ::wcslen(path);
+ArrayRef ref{reinterpret_cast(path),
+   pathLen * sizeof(wchar_t)};
+convertUTF16ToUTF8String(ref, expected);
+  }
+
+  // LocalAppData should always be set, but better safe than sorry
+  if (!expected.empty()) {
+SmallString<128> CacheDir;
+auto status = path::cache_directory(CacheDir);
+EXPECT_TRUE(status);
+EXPECT_EQ(expected, CacheDir);
+  }
+
+  if (!OriginalStorage.empty()) {
+::_wputenv("XDG_CACHE_HOME", OriginalStorage.c_str());
+  } else {
+::_wputenv("XDG_CACHE_HOME", nullptr);
+  }
+#else
+  std::string OriginalStorage;
+  if (char const *path = ::getenv("XDG_CACHE_HOME")) {
+OriginalStorage = path;
+  }
+
+  expected = "/xdg/cache";
+  ::setenv("XDG_CACHE_HOME", expected.c_str(), 1);
+  EXPECT_EQ(expected, std::string(::getenv("XDG_CACHE_HOME")));
+
+  {
+SmallString<128> CacheDir;
+auto status = path::cache_directory(CacheDir);
+EXPECT_TRUE(status);
+EXPECT_EQ(expected, CacheDir);
+  }
+
+  if (!OriginalStorage.empty()) {
+::setenv("XDG_CACHE_HOME", OriginalStorage.c_str(), 1);
+  } else {
+::unsetenv("XDG_CACHE_HOME");
+  }
+#endif
+}
+
+TEST(Support, CacheDirectoryNoEnv) {
+  std::string expected;
+#ifdef _WIN32
+  std::wstring OriginalStorage;
+  if (wchar_t const *path = ::_wgetenv(L"XDG_CACHE_HOME")) {
+OriginalStorage = path;
+  }
+
+  ::_wputenv("XDG_CACHE_HOME", nullptr);
+  EXPECT_EQ(nullptr, ::_wgetenv("XDG_CACHE_HOME"));
+
+  if (wchar_t const *localAppData = ::_wgetenv(L"LOCALAPPDATA")) {
+auto pathLen = ::wcslen(path);
+ArrayRef ref{reinterpret_cast(path),
+   pathLen * sizeof(wchar_t)};
+convertUTF16ToUTF8String(ref, expected);
+  }
+
+  // LocalAppData should always be set, but better safe than sorry
+  if (!expected.empty()) {
+SmallString<128> CacheDir;
+auto status = path::cache_directory(CacheDir);
+EXPECT_TRUE(status);
+EXPECT_EQ(expected, CacheDir);
+  }
+
+  if (!OriginalStorage.empty()) {
+::_wputenv("XDG_CACHE_HOME", OriginalStorage.c_str());
+  } else {
+::_wputenv("XDG_CACHE_HOME", nullptr);
+  }
+#else
+  std::string OriginalStorage;
+  if (char const *path = ::getenv("XDG_CACHE_HOME")) {
+OriginalStorage = path;
+  }
+
+  ::unsetenv("XDG_CACHE_HOME");
+  EXPECT_EQ(nullptr, ::getenv("XDG_CACHE_HOME"));
+
+  SmallString<128> HomeDir;
+  if (path::home_directory(HomeDir)) {
+path::append(HomeDir, ".cache");
+expected = std::string(HomeDir.str());
+  }
+
+  if (!expected.empty()) {
+SmallString<128> CacheDir;
+auto status = path::cache_directory(CacheDir);
+EXPECT_TRUE(status);
+EXPECT_EQ(expected, CacheDir);
+  }
+
+  {
+SmallString<128> CacheDir;
+auto status = path::cache_directory(CacheDir);
+EXPECT_TRUE(status);
+EXPECT_EQ(expected, CacheDir);
+  }
+
+  if (!OriginalStorage.empty()) {
+::setenv("XDG_CACHE_HOME", OriginalStorage.c_str(), 1);
+  } else {
+::unsetenv("XDG_CACHE_HOME");
+  }
+#endif
+}
+
 

[PATCH] D78100: [AST] Suppress the spammy "attempt to use a deleted fucntion" diagnostic.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

OK LG without changes




Comment at: clang/lib/Sema/SemaDecl.cpp:12565
+  auto RecoveryExpr =
+  CreateRecoveryExpr(Var->getLocation(), Var->getEndLoc(), {});
+  if (RecoveryExpr.get())

hokein wrote:
> sammccall wrote:
> > This seems like it's going to claim some actual tokens, when the thing it 
> > represents doesn't cover any tokens.
> > 
> > I think both start/end source locations should be invalid.
> actually, I think it is still valuable to set the var location to 
> recovery-expr.
> ```
> Foo [[foo]]; // if there is a valid default ctor, we have a CtorExpr which 
> has the `foo` range; otherwise there is a recoveryExpr with the same range.
> ```
Yeah, invalid source range is scary too. Let's go with this and see if things 
break. I think more likely it'll be a mild annoyance like the range of 
CXXConstructExpr.

(In a perfect world maybe this would have a location but no range, or a 
SourceRange would have half-open semantics and could represent a point).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78100



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

[PATCH] D78366: [Preamble] Allow recursive inclusion of header-guarded mainfile.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGee12edcb7642: [Preamble] Allow recursive inclusion of 
header-guarded mainfile. (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78366

Files:
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang/lib/Lex/PPDirectives.cpp

Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -1940,19 +1940,6 @@
 return {ImportAction::None};
   }
 
-  // Check for circular inclusion of the main file.
-  // We can't generate a consistent preamble with regard to the conditional
-  // stack if the main file is included again as due to the preamble bounds
-  // some directives (e.g. #endif of a header guard) will never be seen.
-  // Since this will lead to confusing errors, avoid the inclusion.
-  if (File && PreambleConditionalStack.isRecording() &&
-  SourceMgr.translateFile(>getFileEntry()) ==
-  SourceMgr.getMainFileID()) {
-Diag(FilenameTok.getLocation(),
- diag::err_pp_including_mainfile_in_preamble);
-return {ImportAction::None};
-  }
-
   // Should we enter the source file? Set to Skip if either the source file is
   // known to have no effect beyond its effect on module visibility -- that is,
   // if it's got an include guard that is already defined, set to Import if it
@@ -2070,6 +2057,19 @@
 Action = (SuggestedModule && !getLangOpts().CompilingPCH) ? Import : Skip;
   }
 
+  // Check for circular inclusion of the main file.
+  // We can't generate a consistent preamble with regard to the conditional
+  // stack if the main file is included again as due to the preamble bounds
+  // some directives (e.g. #endif of a header guard) will never be seen.
+  // Since this will lead to confusing errors, avoid the inclusion.
+  if (Action == Enter && File && PreambleConditionalStack.isRecording() &&
+  SourceMgr.translateFile(>getFileEntry()) ==
+  SourceMgr.getMainFileID()) {
+Diag(FilenameTok.getLocation(),
+ diag::err_pp_including_mainfile_in_preamble);
+return {ImportAction::None};
+  }
+
   if (Callbacks && !IsImportDecl) {
 // Notify the callback object that we've seen an inclusion directive.
 // FIXME: Use a different callback for a pp-import?
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -33,6 +33,7 @@
 using ::testing::Field;
 using ::testing::IsEmpty;
 using ::testing::Pair;
+using testing::SizeIs;
 using ::testing::UnorderedElementsAre;
 
 ::testing::Matcher WithFix(::testing::Matcher FixMatcher) {
@@ -378,6 +379,47 @@
   ElementsAre(Diag(Test.range(), "use of undeclared identifier 'b'")));
 }
 
+// Recursive main-file include is diagnosed, and doesn't crash.
+TEST(DiagnosticsTest, RecursivePreamble) {
+  auto TU = TestTU::withCode(R"cpp(
+#include "foo.h" // error-ok
+int symbol;
+  )cpp");
+  TU.Filename = "foo.h";
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  ElementsAre(DiagName("pp_including_mainfile_in_preamble")));
+  EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
+}
+
+// Recursive main-file include with #pragma once guard is OK.
+TEST(DiagnosticsTest, RecursivePreamblePragmaOnce) {
+  auto TU = TestTU::withCode(R"cpp(
+#pragma once
+#include "foo.h"
+int symbol;
+  )cpp");
+  TU.Filename = "foo.h";
+  EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty());
+  EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
+}
+
+// Recursive main-file include with #ifndef guard should be OK.
+// However, it's not yet recognized (incomplete at end of preamble).
+TEST(DiagnosticsTest, RecursivePreambleIfndefGuard) {
+  auto TU = TestTU::withCode(R"cpp(
+#ifndef FOO
+#define FOO
+#include "foo.h" // error-ok
+int symbol;
+#endif
+  )cpp");
+  TU.Filename = "foo.h";
+  // FIXME: should be no errors here.
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  ElementsAre(DiagName("pp_including_mainfile_in_preamble")));
+  EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
+}
+
 TEST(DiagnosticsTest, InsideMacros) {
   Annotations Test(R"cpp(
 #define TEN 10
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78338: [clangd] Enable diagnostic fixes within macro argument expansions.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:563
+for (auto  : FixIts) {
+  // Allow fixits within a single macro-arg expansion to be applied.
+  if (FixIt.RemoveRange.getBegin().isMacroID() &&

hokein wrote:
> sammccall wrote:
> > hokein wrote:
> > > I feel a bit nervous about this (even it is for macro-arg expansion 
> > > only), as macro is very tricky.
> > > 
> > > I think we may result in invalid code after applying the fixits in some 
> > > cases:
> > > 
> > > 1) if the fix is to remove an unused variable (interestingly, clang 
> > > doesn't provide fixit to remove an unused variable, but for unused lambda 
> > > capture, it does)
> > > 
> > > ```
> > > #define LA(arg1, arg2) [arg1, arg2] { return arg2;}
> > > void test1(int x, int y) {
> > >   LA(x, y); // after fixit, it would become LA(, y)? or LA(y)
> > > }
> > > ```
> > > 
> > > 2) if the fix is to add some characters to the macro argument, e.g. 
> > > adding a dereference `*`, the semantic of the code after macro expansion 
> > > maybe changed.
> > > 
> > > ```
> > > void f1(int );
> > > void f2(int *a) {
> > >f1(a); // clang will emits a diagnostic with a fixit adding preceding 
> > > a `*` to a.
> > > }
> > > ```
> > > 
> > > maybe we should be more conservative? just whitelist some diagnostics? 
> > > fixing typos seems pretty safe.
> > your `test1` example doesn't trigger this case because the fix has to 
> > delete a comma that's provided by the macro body - this patch doesn't 
> > change behavior.
> > 
> > To construct an example that follows this schema:
> > ```
> > struct S { S(int *x); };
> > int *x;
> > S s1(*x); // fixit -> S s1(x);
> > #define CONCAT(a,b) a b
> > S s2(CONCAT(*, x)); // fixit -> S s2(CONCAT(, x));
> > ```
> > 
> > The fixed code compiles fine and addresses the error in the expected way. 
> > It may not be *idiomatic*, but this is also a pathological case. I think 
> > it's at least as good to offer the fix in this case, and certainly it's not 
> > a good reason to drop support for others..
> > 
> > ---
> > 
> > > void f1(int );
> > 
> > I can't follow this example, there are no macros?
> > Why would the insertion change semantics?
> > 
> > ---
> > 
> > > maybe we should be more conservative? just whitelist some diagnostics? 
> > > fixing typos seems pretty safe.
> > 
> > I think this leaves a lot of value on the table - we've been conservative 
> > so far.
> > The problem with whitelists is they're incomplete and outdated (e.g. we 
> > have a whitelist for include fixer that's very incomplete, and I haven't 
> > managed to get around to fixing it, and neither has anyone else).
> > So I think we should use this (or a blacklist) only if we can show this 
> > plausibly causes real problems.
> > 
> > (To put this another way: by being too aggressive we'll get more feedback, 
> > by being more conservative we'll continue to get none)
> > 
> > your test1 example doesn't trigger this case because the fix has to delete 
> > a comma that's provided by the macro body - this patch doesn't change 
> > behavior.
> 
> ah, you are right.
> 
> > I can't follow this example, there are no macros?
> > Why would the insertion change semantics?
> 
> sorry, the example was incomplete, the case is like
> 
> ```
> int f1(int );
> #define ABC(x) *x + f1(x);
> void f2(int *a) {
>   ABC(a); // fixit -> ABC(*a), incorrect for the `*x` in macro body.
> }
> ```
> 
> if the macro argument is being used in multiple places of the macro body, it 
> probably leads to problems. I suspect this is common in practice, we should 
> not allow fixit in this case.
> 
> 
> >  think this leaves a lot of value on the table - we've been conservative so 
> > far.
> 
> fair enough.
> 
> if the macro argument is being used in multiple places of the macro body, it 
> probably leads to problems. I suspect this is common in practice, we should 
> not allow fixit in this case.

This is definitely true.

As discussed offline, this is mitigated by:
 - stringified expansions are common, but don't really count for this purpose
 - fix will often make sense for all occurrences (we can't detect this), in 
which case not offering it is **worse**
 - remaining cases where fix makes sense for one case and not others probably 
aren't that numerous
 - behaviour in this case is to apply the fix and diagnose the resulting error 
from the other expansion, which isn't that terrible (as far as macro diagnostic 
experience goes)

Agreed to do it anyway and wait for feedback, I'll document this though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78338



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

[PATCH] D77591: [SveEmitter] Explicitly merge with zero/undef

[Sander de Smalen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9986b3de26d3: [SveEmitter] Explicitly merge with zero/undef 
(authored by sdesmalen).

Changed prior to commit:
  https://reviews.llvm.org/D77591?vs=255501=258760#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77591

Files:
  clang/include/clang/Basic/arm_sve.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abs.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_neg.c

Index: clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_neg.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_neg.c
@@ -0,0 +1,197 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall -emit-llvm -o - %s | FileCheck %s
+
+#include 
+
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
+#endif
+
+svint8_t test_svneg_s8_z(svbool_t pg, svint8_t op)
+{
+  // CHECK-LABEL: test_svneg_s8_z
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv16i8( zeroinitializer,  %pg,  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s8,_z,)(pg, op);
+}
+
+svint16_t test_svneg_s16_z(svbool_t pg, svint16_t op)
+{
+  // CHECK-LABEL: test_svneg_s16_z
+  // CHECK: %[[PG:.*]] = call  @llvm.aarch64.sve.convert.from.svbool.nxv8i1( %pg)
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv8i16( zeroinitializer,  %[[PG]],  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s16,_z,)(pg, op);
+}
+
+svint32_t test_svneg_s32_z(svbool_t pg, svint32_t op)
+{
+  // CHECK-LABEL: test_svneg_s32_z
+  // CHECK: %[[PG:.*]] = call  @llvm.aarch64.sve.convert.from.svbool.nxv4i1( %pg)
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv4i32( zeroinitializer,  %[[PG]],  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s32,_z,)(pg, op);
+}
+
+svint64_t test_svneg_s64_z(svbool_t pg, svint64_t op)
+{
+  // CHECK-LABEL: test_svneg_s64_z
+  // CHECK: %[[PG:.*]] = call  @llvm.aarch64.sve.convert.from.svbool.nxv2i1( %pg)
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv2i64( zeroinitializer,  %[[PG]],  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s64,_z,)(pg, op);
+}
+
+svint8_t test_svneg_s8_m(svint8_t inactive, svbool_t pg, svint8_t op)
+{
+  // CHECK-LABEL: test_svneg_s8_m
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv16i8( %inactive,  %pg,  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s8,_m,)(inactive, pg, op);
+}
+
+svint16_t test_svneg_s16_m(svint16_t inactive, svbool_t pg, svint16_t op)
+{
+  // CHECK-LABEL: test_svneg_s16_m
+  // CHECK: %[[PG:.*]] = call  @llvm.aarch64.sve.convert.from.svbool.nxv8i1( %pg)
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv8i16( %inactive,  %[[PG]],  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s16,_m,)(inactive, pg, op);
+}
+
+svint32_t test_svneg_s32_m(svint32_t inactive, svbool_t pg, svint32_t op)
+{
+  // CHECK-LABEL: test_svneg_s32_m
+  // CHECK: %[[PG:.*]] = call  @llvm.aarch64.sve.convert.from.svbool.nxv4i1( %pg)
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv4i32( %inactive,  %[[PG]],  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s32,_m,)(inactive, pg, op);
+}
+
+svint64_t test_svneg_s64_m(svint64_t inactive, svbool_t pg, svint64_t op)
+{
+  // CHECK-LABEL: test_svneg_s64_m
+  // CHECK: %[[PG:.*]] = call  @llvm.aarch64.sve.convert.from.svbool.nxv2i1( %pg)
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv2i64( %inactive,  %[[PG]],  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s64,_m,)(inactive, pg, op);
+}
+
+svint8_t test_svneg_s8_x(svbool_t pg, svint8_t op)
+{
+  // CHECK-LABEL: test_svneg_s8_x
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv16i8( undef,  %pg,  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s8,_x,)(pg, op);
+}
+
+svint16_t test_svneg_s16_x(svbool_t pg, svint16_t op)
+{
+  // CHECK-LABEL: test_svneg_s16_x
+  // CHECK: %[[PG:.*]] = call  @llvm.aarch64.sve.convert.from.svbool.nxv8i1( %pg)
+  // CHECK: %[[INTRINSIC:.*]] = call  @llvm.aarch64.sve.neg.nxv8i16( undef,  %[[PG]],  %op)
+  // CHECK: ret  %[[INTRINSIC]]
+  return SVE_ACLE_FUNC(svneg,_s16,_x,)(pg, op);
+}
+
+svint32_t test_svneg_s32_x(svbool_t pg, svint32_t op)
+{
+  // 

[PATCH] D78495: [nfc] Accept addrspacecast allocas in InitTempAlloca

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

In D78495#1992404 , @arsenm wrote:

> Needs test?


I'm not sure how to write said test. How do we normally hit asserts from the 
clang test suite?

This fires a lot in the openmp on amdgcn downstream branch, but I'm happy 
carrying this as a local patch until the rest of the clang change can be put up 
for review if preferred.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78495



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

[PATCH] D78495: [nfc] Accept addrspacecast allocas in InitTempAlloca

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

Needs test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78495



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

[PATCH] D77735: [SveEmitter] Implement builtins for gathers/scatters

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7601
+  llvm::Type *SrcDataTy = getSVEType(TypeFlags);
+  llvm::Type *OverloadedTy = llvm::VectorType::get(
+  SVEBuiltinMemEltTy(TypeFlags), SrcDataTy->getVectorElementCount());

nit: This can use `auto`, which would make OverloadedTy a `llvm::VectorType`



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7602
+  llvm::Type *OverloadedTy = llvm::VectorType::get(
+  SVEBuiltinMemEltTy(TypeFlags), SrcDataTy->getVectorElementCount());
+

Just be aware that `getVectorElementCount` has been removed from Type in 
D77278, so you'll need to cast to `VectorType` and use `getElementCount` 
instead.
(fyi - in D77596 I've made `getSVEType` actually return a VectorType so that 
cast wouldn't be needed)



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7635
+  // the actual type being stored. Cast accordingly.
+  Ops[1] = EmitSVEPredicateCast(Ops[1], cast(OverloadedTy));
+

You can remove the `cast` if you make the variable use `auto`.
(these comments apply equally to EmitSVEGatherLoad)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77735



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

[clang] ee12edc - [Preamble] Allow recursive inclusion of header-guarded mainfile.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-04-20T17:28:42+02:00
New Revision: ee12edcb76423c78b55cdddae2edfe45cbb2ccd6

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

LOG: [Preamble] Allow recursive inclusion of header-guarded mainfile.

Summary:
This is guaranteed to be a no-op without the preamble, so should be a
no-op with it too.

Partially fixes https://github.com/clangd/clangd/issues/337
This doesn't yet work for #ifndef guards, which are not recognized in preambles.
see D78038

I can't for the life of me work out how to test this outside clangd.
The original reentrant preamble diagnostic was untested, I added a test
to clangd for that too.

Reviewers: kadircet

Subscribers: ilya-biryukov, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang/lib/Lex/PPDirectives.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp 
b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
index 00dd1f864813..f64f8e9901a9 100644
--- a/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -33,6 +33,7 @@ using ::testing::ElementsAre;
 using ::testing::Field;
 using ::testing::IsEmpty;
 using ::testing::Pair;
+using testing::SizeIs;
 using ::testing::UnorderedElementsAre;
 
 ::testing::Matcher WithFix(::testing::Matcher FixMatcher) {
@@ -378,6 +379,47 @@ TEST(DiagnosticsTest, Preprocessor) {
   ElementsAre(Diag(Test.range(), "use of undeclared identifier 'b'")));
 }
 
+// Recursive main-file include is diagnosed, and doesn't crash.
+TEST(DiagnosticsTest, RecursivePreamble) {
+  auto TU = TestTU::withCode(R"cpp(
+#include "foo.h" // error-ok
+int symbol;
+  )cpp");
+  TU.Filename = "foo.h";
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  ElementsAre(DiagName("pp_including_mainfile_in_preamble")));
+  EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
+}
+
+// Recursive main-file include with #pragma once guard is OK.
+TEST(DiagnosticsTest, RecursivePreamblePragmaOnce) {
+  auto TU = TestTU::withCode(R"cpp(
+#pragma once
+#include "foo.h"
+int symbol;
+  )cpp");
+  TU.Filename = "foo.h";
+  EXPECT_THAT(TU.build().getDiagnostics(), IsEmpty());
+  EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
+}
+
+// Recursive main-file include with #ifndef guard should be OK.
+// However, it's not yet recognized (incomplete at end of preamble).
+TEST(DiagnosticsTest, RecursivePreambleIfndefGuard) {
+  auto TU = TestTU::withCode(R"cpp(
+#ifndef FOO
+#define FOO
+#include "foo.h" // error-ok
+int symbol;
+#endif
+  )cpp");
+  TU.Filename = "foo.h";
+  // FIXME: should be no errors here.
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  ElementsAre(DiagName("pp_including_mainfile_in_preamble")));
+  EXPECT_THAT(TU.build().getLocalTopLevelDecls(), SizeIs(1));
+}
+
 TEST(DiagnosticsTest, InsideMacros) {
   Annotations Test(R"cpp(
 #define TEN 10

diff  --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 660c4a5e089d..d6b6f5695b6c 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -1940,19 +1940,6 @@ Preprocessor::ImportAction 
Preprocessor::HandleHeaderIncludeOrImport(
 return {ImportAction::None};
   }
 
-  // Check for circular inclusion of the main file.
-  // We can't generate a consistent preamble with regard to the conditional
-  // stack if the main file is included again as due to the preamble bounds
-  // some directives (e.g. #endif of a header guard) will never be seen.
-  // Since this will lead to confusing errors, avoid the inclusion.
-  if (File && PreambleConditionalStack.isRecording() &&
-  SourceMgr.translateFile(>getFileEntry()) ==
-  SourceMgr.getMainFileID()) {
-Diag(FilenameTok.getLocation(),
- diag::err_pp_including_mainfile_in_preamble);
-return {ImportAction::None};
-  }
-
   // Should we enter the source file? Set to Skip if either the source file is
   // known to have no effect beyond its effect on module visibility -- that is,
   // if it's got an include guard that is already defined, set to Import if it
@@ -2070,6 +2057,19 @@ Preprocessor::ImportAction 
Preprocessor::HandleHeaderIncludeOrImport(
 Action = (SuggestedModule && !getLangOpts().CompilingPCH) ? Import : Skip;
   }
 
+  // Check for circular inclusion of the main file.
+  // We can't generate a consistent preamble with regard to the conditional
+  // stack if the main file is included again as due to the preamble bounds
+  // some directives (e.g. #endif of a header guard) will never be seen.
+  // Since 

[clang] 9986b3d - [SveEmitter] Explicitly merge with zero/undef

[Sander de Smalen via cfe-commits]

Author: Sander de Smalen
Date: 2020-04-20T16:26:20+01:00
New Revision: 9986b3de26d31be26d978194333c44e82873f3ff

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

LOG: [SveEmitter] Explicitly merge with zero/undef

Builtins that have the merge type MergeAnyExp or MergeZeroExp,
merge into a 'undef' or 'zero' vector respectively, which enables the
_x and _z behaviour for unary operations.

This patch also adds builtins for svabs and svneg.

Reviewers: SjoerdMeijer, efriedma, rovka

Reviewed By: efriedma

Tags: #clang

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

Added: 
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abs.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_neg.c

Modified: 
clang/include/clang/Basic/arm_sve.td
clang/lib/CodeGen/CGBuiltin.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/arm_sve.td 
b/clang/include/clang/Basic/arm_sve.td
index 84f03e60b51f..6f665d0c6716 100644
--- a/clang/include/clang/Basic/arm_sve.td
+++ b/clang/include/clang/Basic/arm_sve.td
@@ -296,6 +296,18 @@ def SVSTNT1 : MInst<"svstnt1[_{d}]", "vPpd", 
"csilUcUsUiUlhfd", [IsStore], MemEl
 // Store one vector, with no truncation, non-temporal (scalar base, VL 
displacement)
 def SVSTNT1_VNUM : MInst<"svstnt1_vnum[_{d}]", "vPpld", "csilUcUsUiUlhfd", 
[IsStore], MemEltTyDefault, "aarch64_sve_stnt1">;
 
+
+// Integer arithmetic
+
+multiclass SInstZPZ flags=[]> {
+  def _M : SInst;
+  def _X : SInst;
+  def _Z : SInst;
+}
+
+defm SVABS : SInstZPZ<"svabs", "csil", "aarch64_sve_abs">;
+defm SVNEG : SInstZPZ<"svneg", "csil", "aarch64_sve_neg">;
+
 

 // Permutations and selection
 def SVEXT: SInst<"svext[_{d}]",   "dddi", "csilUcUsUiUlhfd", 
MergeNone, "aarch64_sve_ext", [], [ImmCheck<2, ImmCheckExtract, 1>]>;
@@ -318,6 +330,10 @@ def SVQSHLU_M  : SInst<"svqshlu[_n_{d}]", "uPdi", "csil",  
   MergeOp1,  "aa
 
 

 // Floating-point arithmetic
+
+defm SVABS_F : SInstZPZ<"svabs", "hfd", "aarch64_sve_fabs">;
+defm SVNEG_F : SInstZPZ<"svneg", "hfd", "aarch64_sve_fneg">;
+
 def SVTMAD  : SInst<"svtmad[_{d}]",  "dddi", "hfd", MergeNone, 
"aarch64_sve_ftmad_x", [], [ImmCheck<2, ImmCheck0_7>]>;
 
 def SVMLA_LANE  : SInst<"svmla_lane[_{d}]",  "i",  "hfd", MergeNone, 
"aarch64_sve_fmla_lane", [], [ImmCheck<3, ImmCheckLaneIndex, 2>]>;

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 96c7c9ed2d7b..df45fef9d6c1 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -7591,6 +7591,18 @@ Value *CodeGenFunction::EmitSVEMaskedStore(const 
CallExpr *E,
   return Builder.CreateCall(F, {Val, Predicate, BasePtr});
 }
 
+static void InsertExplicitZeroOperand(CGBuilderTy , llvm::Type *Ty,
+  SmallVectorImpl ) {
+  auto *SplatZero = Constant::getNullValue(Ty);
+  Ops.insert(Ops.begin(), SplatZero);
+}
+
+static void InsertExplicitUndefOperand(CGBuilderTy , llvm::Type *Ty,
+   SmallVectorImpl ) {
+  auto *SplatUndef = UndefValue::get(Ty);
+  Ops.insert(Ops.begin(), SplatUndef);
+}
+
 Value *CodeGenFunction::EmitAArch64SVEBuiltinExpr(unsigned BuiltinID,
   const CallExpr *E) {
   // Find out if any arguments are required to be integer constant expressions.
@@ -7630,6 +7642,12 @@ Value 
*CodeGenFunction::EmitAArch64SVEBuiltinExpr(unsigned BuiltinID,
   else if (Builtin->LLVMIntrinsic != 0) {
 llvm::Type* OverloadedTy = getSVEType(TypeFlags);
 
+if (TypeFlags.getMergeType() == SVETypeFlags::MergeZeroExp)
+  InsertExplicitZeroOperand(Builder, Ty, Ops);
+
+if (TypeFlags.getMergeType() == SVETypeFlags::MergeAnyExp)
+  InsertExplicitUndefOperand(Builder, Ty, Ops);
+
 // Predicates must match the main datatype.
 for (unsigned i = 0, e = Ops.size(); i != e; ++i) {
   if (auto PredTy = dyn_cast(Ops[i]->getType()))

diff  --git a/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abs.c 
b/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abs.c
new file mode 100644
index ..2db01ff7d64c
--- /dev/null
+++ b/clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abs.c
@@ -0,0 +1,197 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -triple aarch64-none-linux-gnu 
-target-feature +sve -fallow-half-arguments-and-returns -S -O1 -Werror -Wall 
-emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -DSVE_OVERLOADED_FORMS -triple 
aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns 
-S -O1 

[PATCH] D72035: [analyzer][NFC] Use CallEvent checker callback in GenericTaintChecker

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

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



Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:113
  const CheckerContext ) {
-  const FunctionDecl *FDecl = C.getCalleeDecl(CE);
+  assert(Call.getDecl());
+  const FunctionDecl *FDecl = Call.getDecl()->getAsFunction();

NoQ wrote:
> rG878194414107e94600de31a11be09a347fb2598b!
Nice catch! Thank you for the fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72035



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

[PATCH] D78495: [nfc] Accept addrspacecast allocas in InitTempAlloca

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield created this revision.
JonChesterfield added reviewers: rjmccall, aaron.ballman, ABataev, jdoerfert, 
arsenm.
Herald added subscribers: cfe-commits, wdng.
Herald added a project: clang.

[nfc] Accept addrspacecast allocas in InitTempAlloca
Changes the precondition to be slightly more permissive. Useful for amdgcn where
allocas are created with a cast to an address space.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78495

Files:
  clang/lib/CodeGen/CGExpr.cpp


Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -125,8 +125,12 @@
 }
 
 void CodeGenFunction::InitTempAlloca(Address Var, llvm::Value *Init) {
-  assert(isa(Var.getPointer()));
-  auto *Store = new llvm::StoreInst(Init, Var.getPointer());
+  auto *Alloca = Var.getPointer();
+  assert(isa(Alloca) ||
+ (isa(Alloca) &&
+  isa(
+  cast(Alloca)->getPointerOperand(;
+  auto *Store = new llvm::StoreInst(Init, Alloca);
   Store->setAlignment(Var.getAlignment().getAsAlign());
   llvm::BasicBlock *Block = AllocaInsertPt->getParent();
   Block->getInstList().insertAfter(AllocaInsertPt->getIterator(), Store);


Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -125,8 +125,12 @@
 }
 
 void CodeGenFunction::InitTempAlloca(Address Var, llvm::Value *Init) {
-  assert(isa(Var.getPointer()));
-  auto *Store = new llvm::StoreInst(Init, Var.getPointer());
+  auto *Alloca = Var.getPointer();
+  assert(isa(Alloca) ||
+ (isa(Alloca) &&
+  isa(
+  cast(Alloca)->getPointerOperand(;
+  auto *Store = new llvm::StoreInst(Init, Alloca);
   Store->setAlignment(Var.getAlignment().getAsAlign());
   llvm::BasicBlock *Block = AllocaInsertPt->getParent();
   Block->getInstList().insertAfter(AllocaInsertPt->getIterator(), Store);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77594: [SveEmitter] Add support for _n form builtins

[Sjoerd Meijer via Phabricator via cfe-commits]

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

Looks reasonable to me




Comment at: clang/utils/TableGen/SveEmitter.cpp:212
+  bool hasSplat() const {
+return Proto.find_first_of("ajfrKLR") != std::string::npos;
+  }

"ajfrKLR" -> bingo ;-)

This probably makes sense, but who knows :-)
Not even sure if a comment makes things better here...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77594



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

[PATCH] D76680: [SveEmitter] Add immediate checks for lanes and complex imms

[Sander de Smalen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sdesmalen marked an inline comment as done.
Closed by commit rGfc6453974980: [SveEmitter] Add immediate checks for lanes 
and complex imms (authored by sdesmalen).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76680

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/arm_sve.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmla.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dot.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_mla.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_cadd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_cmla.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_dot.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_mla.c
  clang/utils/TableGen/SveEmitter.cpp

Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -456,9 +456,19 @@
 Bitwidth = ElementBitwidth;
 NumVectors = 0;
 break;
+  case 'e':
+Signed = false;
+ElementBitwidth /= 2;
+break;
   case 'h':
 ElementBitwidth /= 2;
 break;
+  case 'q':
+ElementBitwidth /= 4;
+break;
+  case 'o':
+ElementBitwidth *= 4;
+break;
   case 'P':
 Signed = true;
 Float = false;
Index: clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_mla.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_mla.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
+#endif
+
+#include 
+
+svfloat16_t test_svmla_lane_f16(svfloat16_t op1, svfloat16_t op2, svfloat16_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 7]}}
+  return SVE_ACLE_FUNC(svmla_lane,_f16,,)(op1, op2, op3, 8);
+}
+
+svfloat32_t test_svmla_lane_f32(svfloat32_t op1, svfloat32_t op2, svfloat32_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 3]}}
+  return SVE_ACLE_FUNC(svmla_lane,_f32,,)(op1, op2, op3, -1);
+}
+
+svfloat64_t test_svmla_lane_f64(svfloat64_t op1, svfloat64_t op2, svfloat64_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 1]}}
+  return SVE_ACLE_FUNC(svmla_lane,_f64,,)(op1, op2, op3, 2);
+}
Index: clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_dot.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_dot.c
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
+#endif
+
+#include 
+
+svint32_t test_svdot_lane_s32(svint32_t op1, svint8_t op2, svint8_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 3]}}
+  return SVE_ACLE_FUNC(svdot_lane,_s32,,)(op1, op2, op3, -1);
+}
+
+svint32_t test_svdot_lane_s32_1(svint32_t op1, svint8_t op2, svint8_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 3]}}
+  return SVE_ACLE_FUNC(svdot_lane,_s32,,)(op1, op2, op3, 4);
+}
+
+svint64_t test_svdot_lane_s64(svint64_t op1, svint16_t op2, svint16_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 1]}}
+  return SVE_ACLE_FUNC(svdot_lane,_s64,,)(op1, op2, op3, -1);
+}
+
+svint64_t test_svdot_lane_s64_1(svint64_t op1, svint16_t op2, svint16_t op3)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 1]}}
+  return SVE_ACLE_FUNC(svdot_lane,_s64,,)(op1, op2, op3, 2);
+}
+
+svuint32_t test_svdot_lane_u32(svuint32_t op1, svuint8_t op2, svuint8_t op3)
+{
+  // expected-error-re@+1 {{argument value 

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

[Digger via Phabricator via cfe-commits]

DiggerLin updated this revision to Diff 258732.
DiggerLin marked 2 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76932

Files:
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/test/Driver/aix-as.c
  llvm/include/llvm/MC/MCAsmInfo.h
  llvm/include/llvm/MC/MCDirectives.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/MC/MCAsmInfoXCOFF.cpp
  llvm/lib/MC/MCAsmStreamer.cpp
  llvm/lib/MC/MCXCOFFStreamer.cpp
  llvm/lib/MC/XCOFFObjectWriter.cpp
  llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
  llvm/test/CodeGen/PowerPC/aix-LinkOnceAnyLinkage.ll
  llvm/test/CodeGen/PowerPC/aix-LinkOnceODRLinkage.ll
  llvm/test/CodeGen/PowerPC/aix-WeakODRLinkage.ll
  llvm/test/CodeGen/PowerPC/aix-extern-weak.ll
  llvm/test/CodeGen/PowerPC/aix-extern.ll
  llvm/test/CodeGen/PowerPC/aix-reference-func-addr-const.ll
  llvm/test/CodeGen/PowerPC/aix-weak.ll
  llvm/test/CodeGen/PowerPC/aix-xcoff-reloc.ll

Index: llvm/test/CodeGen/PowerPC/aix-xcoff-reloc.ll
===
--- llvm/test/CodeGen/PowerPC/aix-xcoff-reloc.ll
+++ llvm/test/CodeGen/PowerPC/aix-xcoff-reloc.ll
@@ -36,7 +36,7 @@
 ; OBJ-NEXT:   NumberOfSections: 2
 ; OBJ-NEXT:   TimeStamp: None (0x0)
 ; OBJ-NEXT:   SymbolTableOffset: 0x13C
-; OBJ-NEXT:   SymbolTableEntries: 24
+; OBJ-NEXT:   SymbolTableEntries: 26
 ; OBJ-NEXT:   OptionalHeaderSize: 0x0
 ; OBJ-NEXT:   Flags: 0x0
 ; OBJ-NEXT: }
@@ -86,7 +86,7 @@
 ; RELOC-NEXT:   }
 ; RELOC-NEXT:   Relocation {
 ; RELOC-NEXT: Virtual Address: 0x1A
-; RELOC-NEXT: Symbol: globalA (20)
+; RELOC-NEXT: Symbol: globalA (22)
 ; RELOC-NEXT: IsSigned: No
 ; RELOC-NEXT: FixupBitValue: 0
 ; RELOC-NEXT: Length: 16
@@ -94,7 +94,7 @@
 ; RELOC-NEXT:   }
 ; RELOC-NEXT:   Relocation {
 ; RELOC-NEXT: Virtual Address: 0x1E
-; RELOC-NEXT: Symbol: globalB (22)
+; RELOC-NEXT: Symbol: globalB (24)
 ; RELOC-NEXT: IsSigned: No
 ; RELOC-NEXT: FixupBitValue: 0
 ; RELOC-NEXT: Length: 16
@@ -104,7 +104,7 @@
 ; RELOC-NEXT: Section (index: 2) .data {
 ; RELOC-NEXT: Relocation {
 ; RELOC-NEXT:   Virtual Address: 0x70
-; RELOC-NEXT:   Symbol: arr (12)
+; RELOC-NEXT:   Symbol: arr (14)
 ; RELOC-NEXT:   IsSigned: No
 ; RELOC-NEXT:   FixupBitValue: 0
 ; RELOC-NEXT:   Length: 32
@@ -112,7 +112,7 @@
 ; RELOC-NEXT: }
 ; RELOC-NEXT: Relocation {
 ; RELOC-NEXT:   Virtual Address: 0x74
-; RELOC-NEXT:   Symbol: .foo (4)
+; RELOC-NEXT:   Symbol: .foo (6)
 ; RELOC-NEXT:   IsSigned: No
 ; RELOC-NEXT:   FixupBitValue: 0
 ; RELOC-NEXT:   Length: 32
@@ -120,7 +120,7 @@
 ; RELOC-NEXT: }
 ; RELOC-NEXT: Relocation {
 ; RELOC-NEXT:   Virtual Address: 0x78
-; RELOC-NEXT:   Symbol: TOC (18)
+; RELOC-NEXT:   Symbol: TOC (20)
 ; RELOC-NEXT:   IsSigned: No
 ; RELOC-NEXT:   FixupBitValue: 0
 ; RELOC-NEXT:   Length: 32
@@ -128,7 +128,7 @@
 ; RELOC-NEXT: }
 ; RELOC-NEXT: Relocation {
 ; RELOC-NEXT:   Virtual Address: 0x80
-; RELOC-NEXT:   Symbol: globalA (8)
+; RELOC-NEXT:   Symbol: globalA (10)
 ; RELOC-NEXT:   IsSigned: No
 ; RELOC-NEXT:   FixupBitValue: 0
 ; RELOC-NEXT:   Length: 32
@@ -136,7 +136,7 @@
 ; RELOC-NEXT: }
 ; RELOC-NEXT: Relocation {
 ; RELOC-NEXT:   Virtual Address: 0x84
-; RELOC-NEXT:   Symbol: globalB (10)
+; RELOC-NEXT:   Symbol: globalB (12)
 ; RELOC-NEXT:   IsSigned: No
 ; RELOC-NEXT:   FixupBitValue: 0
 ; RELOC-NEXT:   Length: 32
@@ -168,6 +168,26 @@
 ; SYM-NEXT:   }
 ; SYM-NEXT:   Symbol {
 ; SYM-NEXT: Index: 2
+; SYM-NEXT: Name: bar
+; SYM-NEXT: Value (RelocatableAddress): 0x0
+; SYM-NEXT: Section: N_UNDEF
+; SYM-NEXT: Type: 0x0
+; SYM-NEXT: StorageClass: C_EXT (0x2)
+; SYM-NEXT: NumberOfAuxEntries: 1
+; SYM-NEXT: CSECT Auxiliary Entry {
+; SYM-NEXT:   Index: 3
+; SYM-NEXT:   SectionLen: 0
+; SYM-NEXT:   ParameterHashIndex: 0x0
+; SYM-NEXT:   TypeChkSectNum: 0x0
+; SYM-NEXT:   SymbolAlignmentLog2: 0
+; SYM-NEXT:   SymbolType: XTY_ER (0x0)
+; SYM-NEXT:   StorageMappingClass: XMC_DS (0xA)
+; SYM-NEXT:   StabInfoIndex: 0x0
+; SYM-NEXT:   StabSectNum: 0x0
+; SYM-NEXT: }
+; SYM-NEXT:   }
+; SYM-NEXT:   Symbol {
+; SYM-NEXT: Index: 4
 ; SYM-NEXT: Name: .text
 ; SYM-NEXT: Value (RelocatableAddress): 0x0
 ; SYM-NEXT: Section: .text
@@ -175,7 +195,7 @@
 ; SYM-NEXT: StorageClass: C_HIDEXT (0x6B)
 ; SYM-NEXT: NumberOfAuxEntries: 1
 ; SYM-NEXT: CSECT Auxiliary Entry {
-; SYM-NEXT:   Index: 3
+; SYM-NEXT:   Index: 5
 ; SYM-NEXT:   SectionLen: 64
 ; SYM-NEXT:   ParameterHashIndex: 0x0
 ; SYM-NEXT:   TypeChkSectNum: 0x0
@@ -187,7 +207,7 @@
 ; SYM-NEXT: }
 ; SYM-NEXT:   }
 ; SYM-NEXT:   Symbol {
-; SYM-NEXT: Index: 4
+; SYM-NEXT: Index: 6
 ; SYM-NEXT: Name: .foo
 ; SYM-NEXT: Value (RelocatableAddress): 0x0
 ; SYM-NEXT: Section: .text
@@ -195,8 +215,8 @@
 ; SYM-NEXT: StorageClass: 

[PATCH] D78286: [analyzer] Track runtime types represented by Obj-C Class objects

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp:198
+  // 'self' variable of the current class method.
+  if (ReceiverSVal == Message.getSelfSVal()) {
+// In this case, we should return the type of the enclosing class

vsavchenko wrote:
> NoQ wrote:
> > vsavchenko wrote:
> > > NoQ wrote:
> > > > I believe this is pretty much always the case. At least whenever 
> > > > `getInstanceReceiver()` is available. Another exception seem to be when 
> > > > `ReceiverSVal` is an `UnknownVal` (in this case `self` is going to be 
> > > > `SymbolRegionValue` because it's never set in the Store), but that's 
> > > > it. I inferred this by looking at 
> > > > `ObjCMethodCall::getInitialStackFrameContents()`.
> > > > 
> > > > I think we should have used `getSelfSVal()` to begin with.
> > > > I believe this is pretty much always the case.
> > > 
> > > I didn't quite get what you mean by that
> > > 
> > > 
> > What i'm trying to say is that `C.getSVal(RecE)` and 
> > `Message.getSelfSVal()` and `Message.getReceiverSVal()` are basically the 
> > same `SVal`. It shouldn't be necessary to check both or check whether 
> > they're the same; you must have meant to check for something else, probably 
> > something purely syntactic.
> > 
> > 
> > 
> > > I inferred this by looking at 
> > > ObjCMethodCall::getInitialStackFrameContents().
> > 
> > Wait, so it's only true for inlined methods. For non-inlined methods 
> > `getSelfSVal()` will be unknown :/
> Yeah, that might be a bit extraneous to do it with `SVal`s, but this code for 
> sure does its job (it is definitely not a redundant check). `getSelfSVal()` 
> returns receiver of the function //containing// the call and not the call 
> itself. So, it does check if we the receiver of the message is `self`.
> 
> I changed it to this way of doing things because it is consistent with how 
> the same thing is done in `getRuntimeDefinition`.
> `getSelfSVal()` returns receiver of the function containing the call and not 
> the call itself

 looks broken to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78286



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

[PATCH] D78491: Avoid relying on address space zero default parameter in llvm/IR

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: aykevl, dylanmckay, arsenm, bjope, theraven, 
jrtc27.
Herald added subscribers: cfe-commits, hiraditya, mgorny, wdng.
Herald added a reviewer: ctetreau.
Herald added a project: clang.

APIs such as PointerType::getUnqual/Type::getPointerTo() can result in
pointers being created in address space zero even though this is not
correct for all targets. We have been bitten by this issue many times our
CHERI fork: we use address space 200 for both globals and functions.
Creating a pointer in address space zero will generally result instruction
selection failures or in some cases code being generated that triggers
traps at run time. To avoid these problems, I've remove the many instances
of `unsigned AS = 0` function parameters to ensure that pointers get created
in the right address space.

Keeping these changes out-of-tree results in compilation failures almost
every time I do an upstream merge. It would be very beneficial for us to
have these changes upstreamed, and should help targets such as AVR than
use a non-zero program address space.
I've seen some recent commits for AVR that fix the same AS=0 issues that
we have out-of-tree (e.g. 215dc2e203341f7d1edc4c4a191b048af4ace43d 
).
I think making such bugs a compilation failure should be beneficial for
everyone even if it means typing a few more characters to get a pointer type.

This change allows for gradual migration: we can add a single CMake line
to each directory that should no longer compile with implicit address
space zero: `add_definitions(-DLLVM_NO_IMPLICIT_ADDRESS_SPACE=1)`. In this
patch I've added the definition to lib/IR and I will follow up with further
cleanups.

Depends on D70947 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78491

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.h
  llvm/include/llvm/Analysis/TargetTransformInfo.h
  llvm/include/llvm/CodeGen/MachineMemOperand.h
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/IR/DataLayout.h
  llvm/include/llvm/IR/DerivedTypes.h
  llvm/include/llvm/IR/Function.h
  llvm/include/llvm/IR/GlobalObject.h
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/IR/Type.h
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/IR/CMakeLists.txt
  llvm/lib/IR/Constants.cpp
  llvm/lib/IR/ConstantsContext.h
  llvm/lib/IR/Core.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/IR/InlineAsm.cpp
  llvm/lib/IR/Instructions.cpp
  llvm/lib/IR/Mangler.cpp

Index: llvm/lib/IR/Mangler.cpp
===
--- llvm/lib/IR/Mangler.cpp
+++ llvm/lib/IR/Mangler.cpp
@@ -101,7 +101,7 @@
 if (AI->hasByValOrInAllocaAttr())
   Ty = cast(Ty)->getElementType();
 // Size should be aligned to pointer size.
-unsigned PtrSize = DL.getPointerSize();
+unsigned PtrSize = DL.getPointerSize(0);
 ArgWords += alignTo(DL.getTypeAllocSize(Ty), PtrSize);
   }
 
Index: llvm/lib/IR/Instructions.cpp
===
--- llvm/lib/IR/Instructions.cpp
+++ llvm/lib/IR/Instructions.cpp
@@ -616,12 +616,14 @@
   // Create the call to Malloc.
   BasicBlock *BB = InsertBefore ? InsertBefore->getParent() : InsertAtEnd;
   Module *M = BB->getParent()->getParent();
-  Type *BPTy = Type::getInt8PtrTy(BB->getContext());
+  Type *BPTy = Type::getInt8PtrTy(
+  BB->getContext(), M->getDataLayout().getDefaultGlobalsAddressSpace());
   FunctionCallee MallocFunc = MallocF;
   if (!MallocFunc)
 // prototype malloc as "void *malloc(size_t)"
 MallocFunc = M->getOrInsertFunction("malloc", BPTy, IntPtrTy);
-  PointerType *AllocPtrType = PointerType::getUnqual(AllocTy);
+  PointerType *AllocPtrType = AllocTy->getPointerTo(
+  MallocFunc.getFunctionType()->getReturnType()->getPointerAddressSpace());
   CallInst *MCall = nullptr;
   Instruction *Result = nullptr;
   if (InsertBefore) {
@@ -712,7 +714,8 @@
   Module *M = BB->getParent()->getParent();
 
   Type *VoidTy = Type::getVoidTy(M->getContext());
-  Type *IntPtrTy = Type::getInt8PtrTy(M->getContext());
+  Type *IntPtrTy = Type::getInt8PtrTy(
+  M->getContext(), Source->getType()->getPointerAddressSpace());
   // prototype free as "void free(void*)"
   FunctionCallee FreeFunc = M->getOrInsertFunction("free", VoidTy, IntPtrTy);
   CallInst *Result = nullptr;
Index: llvm/lib/IR/InlineAsm.cpp
===
--- llvm/lib/IR/InlineAsm.cpp
+++ llvm/lib/IR/InlineAsm.cpp
@@ -30,7 +30,8 @@
 InlineAsm::InlineAsm(FunctionType *FTy, const std::string ,
  const std::string , bool hasSideEffects,
  bool isAlignStack, AsmDialect asmDialect)
-: Value(PointerType::getUnqual(FTy), Value::InlineAsmVal),
+: Value(PointerType::get(FTy, FTy->getPointerAddressSpace()),
+

[clang] fc64539 - [SveEmitter] Add immediate checks for lanes and complex imms

[Sander de Smalen via cfe-commits]

Author: Sander de Smalen
Date: 2020-04-20T15:10:54+01:00
New Revision: fc645397498037ccb7df230a07e9a8762aaf8c8f

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

LOG: [SveEmitter] Add immediate checks for lanes and complex imms

Adds another bunch of of intrinsics that take immediates with
varying ranges based, some being a complex rotation immediate
which are a set of allowed immediates rather than a range.

svmla_lane:   lane immediate ranging 0..(128/(1*sizeinbits(elt)) - 1)
svcmla_lane:  lane immediate ranging 0..(128/(2*sizeinbits(elt)) - 1)
svdot_lane:   lane immediate ranging 0..(128/(4*sizeinbits(elt)) - 1)
svcadd:   complex rotate immediate [90, 270]
svcmla:
svcmla_lane:  complex rotate immediate [0, 90, 180, 270]

Reviewers: efriedma, SjoerdMeijer, rovka

Reviewed By: efriedma

Tags: #clang

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

Added: 
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmla.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dot.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_mla.c
clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_cadd.c
clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_cmla.c
clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_dot.c
clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_mla.c

Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/arm_sve.td
clang/lib/Sema/SemaChecking.cpp
clang/utils/TableGen/SveEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 97ad1a6c7920..a64e313bf271 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9244,6 +9244,10 @@ def err_argument_not_shifted_byte : Error<
   "argument should be an 8-bit value shifted by a multiple of 8 bits">;
 def err_argument_not_shifted_byte_or_xxff : Error<
   "argument should be an 8-bit value shifted by a multiple of 8 bits, or in 
the form 0x??FF">;
+def err_rotation_argument_to_cadd
+: Error<"argument should be the value 90 or 270">;
+def err_rotation_argument_to_cmla
+: Error<"argument should be the value 0, 90, 180 or 270">;
 def warn_neon_vector_initializer_non_portable : Warning<
   "vector initializers are not compatible with NEON intrinsics in big endian "
   "mode">, InGroup>;

diff  --git a/clang/include/clang/Basic/arm_sve.td 
b/clang/include/clang/Basic/arm_sve.td
index 9fe4715e4ea1..84f03e60b51f 100644
--- a/clang/include/clang/Basic/arm_sve.td
+++ b/clang/include/clang/Basic/arm_sve.td
@@ -62,7 +62,10 @@
 // d: default
 // c: const pointer type
 // P: predicate type
+// e: 1/2 width unsigned elements, 2x element count
 // h: 1/2 width elements, 2x element count
+// q: 1/4 width elements, 4x element count
+// o: 4x width elements, 1/4 element count
 //
 // i: constant uint64_t
 //
@@ -164,6 +167,11 @@ def ImmCheckShiftRight  : ImmCheckType<3>;  // 
1..sizeinbits(elt)
 def ImmCheckShiftRightNarrow: ImmCheckType<4>;  // 1..sizeinbits(elt)/2
 def ImmCheckShiftLeft   : ImmCheckType<5>;  // 0..(sizeinbits(elt) - 1)
 def ImmCheck0_7 : ImmCheckType<6>;  // 0..7
+def ImmCheckLaneIndex   : ImmCheckType<7>;  // 
0..(128/(1*sizeinbits(elt)) - 1)
+def ImmCheckLaneIndexCompRotate : ImmCheckType<8>;  // 
0..(128/(2*sizeinbits(elt)) - 1)
+def ImmCheckLaneIndexDot: ImmCheckType<9>;  // 
0..(128/(4*sizeinbits(elt)) - 1)
+def ImmCheckComplexRot90_270: ImmCheckType<10>; // [90,270]
+def ImmCheckComplexRotAll90 : ImmCheckType<11>; // [0, 90, 180,270]
 
 class ImmCheck {
   int Arg = arg;
@@ -312,7 +320,19 @@ def SVQSHLU_M  : SInst<"svqshlu[_n_{d}]", "uPdi", "csil",  
   MergeOp1,  "aa
 // Floating-point arithmetic
 def SVTMAD  : SInst<"svtmad[_{d}]",  "dddi", "hfd", MergeNone, 
"aarch64_sve_ftmad_x", [], [ImmCheck<2, ImmCheck0_7>]>;
 
+def SVMLA_LANE  : SInst<"svmla_lane[_{d}]",  "i",  "hfd", MergeNone, 
"aarch64_sve_fmla_lane", [], [ImmCheck<3, ImmCheckLaneIndex, 2>]>;
+def SVCMLA_LANE : SInst<"svcmla_lane[_{d}]", "ii", "hf",  MergeNone, 
"aarch64_sve_fcmla_lane", [], [ImmCheck<3, ImmCheckLaneIndexCompRotate, 2>,
+   
 ImmCheck<4, ImmCheckComplexRotAll90>]>;
+
+def SVCADD_M : SInst<"svcadd[_{d}]", "dPddi",  "hfd", MergeOp1,  
"aarch64_sve_fcadd", [], [ImmCheck<3, ImmCheckComplexRot90_270>]>;
+def SVCMLA_M : SInst<"svcmla[_{d}]", "dPdddi", "hfd", MergeOp1,  
"aarch64_sve_fcmla", [], [ImmCheck<4, ImmCheckComplexRotAll90>]>;
+
 

[PATCH] D78100: [AST] Suppress the spammy "attempt to use a deleted fucntion" diagnostic.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaDecl.cpp:12565
+  auto RecoveryExpr =
+  CreateRecoveryExpr(Var->getLocation(), Var->getEndLoc(), {});
+  if (RecoveryExpr.get())

sammccall wrote:
> This seems like it's going to claim some actual tokens, when the thing it 
> represents doesn't cover any tokens.
> 
> I think both start/end source locations should be invalid.
actually, I think it is still valuable to set the var location to recovery-expr.
```
Foo [[foo]]; // if there is a valid default ctor, we have a CtorExpr which has 
the `foo` range; otherwise there is a recoveryExpr with the same range.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78100



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

[PATCH] D77594: [SveEmitter] Add support for _n form builtins

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77594



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

[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

[Sander de Smalen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG515020c091e7: [SveEmitter] Add more immediate operand 
checks. (authored by sdesmalen).

Changed prior to commit:
  https://reviews.llvm.org/D76679?vs=252246=258724#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76679

Files:
  clang/include/clang/Basic/arm_sve.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_asrd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ext.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_tmad.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_asrd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_ext.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_tmad.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_qshlu.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_shrnb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_qshlu.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_shrnb.c
  clang/utils/TableGen/SveEmitter.cpp

Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -456,6 +456,9 @@
 Bitwidth = ElementBitwidth;
 NumVectors = 0;
 break;
+  case 'h':
+ElementBitwidth /= 2;
+break;
   case 'P':
 Signed = true;
 Float = false;
@@ -463,6 +466,11 @@
 Bitwidth = 16;
 ElementBitwidth = 1;
 break;
+  case 'u':
+Predicate = false;
+Signed = false;
+Float = false;
+break;
   case 'i':
 Predicate = false;
 Float = false;
Index: clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_shrnb.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_shrnb.c
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE2 -triple aarch64-none-linux-gnu -target-feature +sve2 -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE2 -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve2 -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
+#endif
+
+#include 
+
+svint8_t test_svshrnb_n_s16(svint16_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [1, 8]}}
+  return SVE_ACLE_FUNC(svshrnb,_n_s16,,)(op1, 0);
+}
+
+svint16_t test_svshrnb_n_s32(svint32_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [1, 16]}}
+  return SVE_ACLE_FUNC(svshrnb,_n_s32,,)(op1, 0);
+}
+
+svint32_t test_svshrnb_n_s64(svint64_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [1, 32]}}
+  return SVE_ACLE_FUNC(svshrnb,_n_s64,,)(op1, 0);
+}
+
+svuint8_t test_svshrnb_n_u16(svuint16_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [1, 8]}}
+  return SVE_ACLE_FUNC(svshrnb,_n_u16,,)(op1, 0);
+}
+
+svuint16_t test_svshrnb_n_u32(svuint32_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [1, 16]}}
+  return SVE_ACLE_FUNC(svshrnb,_n_u32,,)(op1, 0);
+}
+
+svuint32_t test_svshrnb_n_u64(svuint64_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [1, 32]}}
+  return SVE_ACLE_FUNC(svshrnb,_n_u64,,)(op1, 0);
+}
Index: clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_qshlu.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_qshlu.c
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE2 -triple aarch64-none-linux-gnu -target-feature +sve2 -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE2 -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve2 -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
+#endif
+
+#include 
+
+svuint8_t test_svqshlu_n_s8_m(svbool_t pg, svint8_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 7]}}
+  return SVE_ACLE_FUNC(svqshlu,_n_s8,_m,)(pg, op1, -1);
+}
+
+svuint16_t test_svqshlu_n_s16_m(svbool_t pg, svint16_t 

[PATCH] D78280: [Analyzer][StreamChecker] Track streams that were not found to be opened.

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

Szelethus added a comment.

The high level idea seems great after surveying the analyzer for similar 
issues, but I might need to think about this a bit longer. @baloghadamsoftware, 
IteratorChecker needs to solve similar problems, right? Do you have any input 
on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78280



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

[PATCH] D78401: [SveEmitter] IsInsertOp1SVALL and builtins for svqdec[bhwd] and svqinc[bhwd]

[Sjoerd Meijer via Phabricator via cfe-commits]

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

Looks reasonable




Comment at: clang/include/clang/Basic/arm_sve.td:530
+class sat_type { string U = u; string T = t; }
+def SIGNED_BYTE : sat_type<"",  "c">;
+def SIGNED_HALF : sat_type<"",  "s">;

nit: just wondering if all these defs should be all capitals.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7684
 
+if (TypeFlags.isInsertOp1SVALL())
+  Ops.insert([1], Builder.getInt32(31));

would this be the most appropriate place to add the useful sentence from the 
description: 

"Some ACLE builtins leave out the argument to specify the predicate
pattern, which is expected to be expanded to an SV_ALL pattern."

because that's what 31 is, right?



Comment at: clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_qdecb.c:7
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3

nit: used,unused -> "used/unused", or "used, unused"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78401



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

[PATCH] D78338: [clangd] Enable diagnostic fixes within macro argument expansions.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/Diagnostics.cpp:563
+for (auto  : FixIts) {
+  // Allow fixits within a single macro-arg expansion to be applied.
+  if (FixIt.RemoveRange.getBegin().isMacroID() &&

sammccall wrote:
> hokein wrote:
> > I feel a bit nervous about this (even it is for macro-arg expansion only), 
> > as macro is very tricky.
> > 
> > I think we may result in invalid code after applying the fixits in some 
> > cases:
> > 
> > 1) if the fix is to remove an unused variable (interestingly, clang doesn't 
> > provide fixit to remove an unused variable, but for unused lambda capture, 
> > it does)
> > 
> > ```
> > #define LA(arg1, arg2) [arg1, arg2] { return arg2;}
> > void test1(int x, int y) {
> >   LA(x, y); // after fixit, it would become LA(, y)? or LA(y)
> > }
> > ```
> > 
> > 2) if the fix is to add some characters to the macro argument, e.g. adding 
> > a dereference `*`, the semantic of the code after macro expansion maybe 
> > changed.
> > 
> > ```
> > void f1(int );
> > void f2(int *a) {
> >f1(a); // clang will emits a diagnostic with a fixit adding preceding a 
> > `*` to a.
> > }
> > ```
> > 
> > maybe we should be more conservative? just whitelist some diagnostics? 
> > fixing typos seems pretty safe.
> your `test1` example doesn't trigger this case because the fix has to delete 
> a comma that's provided by the macro body - this patch doesn't change 
> behavior.
> 
> To construct an example that follows this schema:
> ```
> struct S { S(int *x); };
> int *x;
> S s1(*x); // fixit -> S s1(x);
> #define CONCAT(a,b) a b
> S s2(CONCAT(*, x)); // fixit -> S s2(CONCAT(, x));
> ```
> 
> The fixed code compiles fine and addresses the error in the expected way. It 
> may not be *idiomatic*, but this is also a pathological case. I think it's at 
> least as good to offer the fix in this case, and certainly it's not a good 
> reason to drop support for others..
> 
> ---
> 
> > void f1(int );
> 
> I can't follow this example, there are no macros?
> Why would the insertion change semantics?
> 
> ---
> 
> > maybe we should be more conservative? just whitelist some diagnostics? 
> > fixing typos seems pretty safe.
> 
> I think this leaves a lot of value on the table - we've been conservative so 
> far.
> The problem with whitelists is they're incomplete and outdated (e.g. we have 
> a whitelist for include fixer that's very incomplete, and I haven't managed 
> to get around to fixing it, and neither has anyone else).
> So I think we should use this (or a blacklist) only if we can show this 
> plausibly causes real problems.
> 
> (To put this another way: by being too aggressive we'll get more feedback, by 
> being more conservative we'll continue to get none)
> 
> your test1 example doesn't trigger this case because the fix has to delete a 
> comma that's provided by the macro body - this patch doesn't change behavior.

ah, you are right.

> I can't follow this example, there are no macros?
> Why would the insertion change semantics?

sorry, the example was incomplete, the case is like

```
int f1(int );
#define ABC(x) *x + f1(x);
void f2(int *a) {
  ABC(a); // fixit -> ABC(*a), incorrect for the `*x` in macro body.
}
```

if the macro argument is being used in multiple places of the macro body, it 
probably leads to problems. I suspect this is common in practice, we should not 
allow fixit in this case.


>  think this leaves a lot of value on the table - we've been conservative so 
> far.

fair enough.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78338



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

[clang] 515020c - [SveEmitter] Add more immediate operand checks.

[Sander de Smalen via cfe-commits]

Author: Sander de Smalen
Date: 2020-04-20T14:41:58+01:00
New Revision: 515020c091e74723ee0876229890d71a8aa79702

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

LOG: [SveEmitter] Add more immediate operand checks.

This patch adds a number of intrinsics that take immediates with
varying ranges based on the element size one of the operands.

svext:   immediate ranging 0 to (2048/sizeinbits(elt) - 1)
svasrd:  immediate ranging 1..sizeinbits(elt)
svqshlu: immediate ranging 1..sizeinbits(elt)/2
ftmad:   immediate ranging 0..(sizeinbits(elt) - 1)

Reviewers: efriedma, SjoerdMeijer, rovka, rengolin

Reviewed By: SjoerdMeijer

Tags: #clang

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

Added: 
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_asrd.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ext.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_tmad.c
clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_asrd.c
clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_ext.c
clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_tmad.c
clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_qshlu.c
clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_shrnb.c
clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_qshlu.c
clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_shrnb.c

Modified: 
clang/include/clang/Basic/arm_sve.td
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/Sema/SemaChecking.cpp
clang/utils/TableGen/SveEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/arm_sve.td 
b/clang/include/clang/Basic/arm_sve.td
index 75fd3ca499d0..9fe4715e4ea1 100644
--- a/clang/include/clang/Basic/arm_sve.td
+++ b/clang/include/clang/Basic/arm_sve.td
@@ -58,9 +58,11 @@
 // ---
 // prototype: return (arg, arg, ...)
 //
+// u: vector of unsigned integers
 // d: default
 // c: const pointer type
 // P: predicate type
+// h: 1/2 width elements, 2x element count
 //
 // i: constant uint64_t
 //
@@ -157,14 +159,18 @@ class ImmCheckType {
 }
 def ImmCheck0_31: ImmCheckType<0>;  // 0..31 (used for e.g. 
predicate patterns)
 def ImmCheck1_16: ImmCheckType<1>;  // 1..16
+def ImmCheckExtract : ImmCheckType<2>;  // 
0..(2048/sizeinbits(elt) - 1)
+def ImmCheckShiftRight  : ImmCheckType<3>;  // 1..sizeinbits(elt)
+def ImmCheckShiftRightNarrow: ImmCheckType<4>;  // 1..sizeinbits(elt)/2
+def ImmCheckShiftLeft   : ImmCheckType<5>;  // 0..(sizeinbits(elt) - 1)
+def ImmCheck0_7 : ImmCheckType<6>;  // 0..7
 
 class ImmCheck {
   int Arg = arg;
-   int EltSizeArg = eltSizeArg;
+  int EltSizeArg = eltSizeArg;
   ImmCheckType Kind = kind;
 }
 
-// Every intrinsic subclasses Inst.
 class Inst ft, list ch, MemEltType met> {
   string Name = n;
@@ -282,6 +288,30 @@ def SVSTNT1 : MInst<"svstnt1[_{d}]", "vPpd", 
"csilUcUsUiUlhfd", [IsStore], MemEl
 // Store one vector, with no truncation, non-temporal (scalar base, VL 
displacement)
 def SVSTNT1_VNUM : MInst<"svstnt1_vnum[_{d}]", "vPpld", "csilUcUsUiUlhfd", 
[IsStore], MemEltTyDefault, "aarch64_sve_stnt1">;
 
+
+// Permutations and selection
+def SVEXT: SInst<"svext[_{d}]",   "dddi", "csilUcUsUiUlhfd", 
MergeNone, "aarch64_sve_ext", [], [ImmCheck<2, ImmCheckExtract, 1>]>;
+
+
+// Shifts
+def SVASRD_M : SInst<"svasrd[_n_{d}]", "dPdi", "csil",MergeOp1,  
"aarch64_sve_asrd", [], [ImmCheck<2, ImmCheckShiftRight, 1>]>;
+
+
+// SVE2 - Narrowing DSP operations
+let ArchGuard = "defined(__ARM_FEATURE_SVE2)" in {
+def SVSHRNB  : SInst<"svshrnb[_n_{d}]","hdi",  "silUsUiUl", MergeNone, 
"aarch64_sve_shrnb", [], [ImmCheck<1, ImmCheckShiftRightNarrow, 0>]>;
+}
+
+
+// SVE2 - Uniform DSP operations
+let ArchGuard = "defined(__ARM_FEATURE_SVE2)" in {
+def SVQSHLU_M  : SInst<"svqshlu[_n_{d}]", "uPdi", "csil", MergeOp1,  
"aarch64_sve_sqshlu", [], [ImmCheck<2, ImmCheckShiftLeft,  1>]>;
+}
+
+
+// Floating-point arithmetic
+def SVTMAD  : SInst<"svtmad[_{d}]",  "dddi", "hfd", MergeNone, 
"aarch64_sve_ftmad_x", [], [ImmCheck<2, ImmCheck0_7>]>;
+
 

 // Saturating scalar arithmetic
 def SVQDECH_S : SInst<"svqdech_pat[_{d}]",   "ddIi", "s", MergeNone, 

[PATCH] D76929: [AArch64][SVE] Add SVE intrinsic for LD1RQ

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:11622
+  if (VT.isFloatingPoint())
+Load = DAG.getNode(ISD::BITCAST, DL, VT, Load);
+

kmclaughlin wrote:
> sdesmalen wrote:
> > I'd expect this to then use `Load.getValue(0)` ?
> I think this will have the same effect, as `Load` just returns a single value
`SDValue LoadChain = SDValue(Load.getNode(), 1);` suggests that `Load` has two 
return values, the result of the load, and the Chain.



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

https://reviews.llvm.org/D76929



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

[PATCH] D78487: Explicitly move from llvm::json Array/Object to Value

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb36b889a3b81: Explicitly move from llvm::json Array/Object 
to Value (authored by MForster, committed by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78487

Files:
  clang-tools-extra/clangd/Protocol.cpp


Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1002,7 +1002,7 @@
 Result.push_back(Tok.tokenModifiers);
   }
   assert(Result.size() == SemanticTokenEncodingSize * Toks.size());
-  return Result;
+  return std::move(Result);
 }
 
 bool operator==(const SemanticToken , const SemanticToken ) {
@@ -1030,7 +1030,7 @@
 Result["edits"] = *TE.edits;
   if (TE.tokens)
 Result["data"] = encodeTokens(*TE.tokens);
-  return Result;
+  return std::move(Result);
 }
 
 bool fromJSON(const llvm::json::Value , SemanticTokensParams ) {


Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1002,7 +1002,7 @@
 Result.push_back(Tok.tokenModifiers);
   }
   assert(Result.size() == SemanticTokenEncodingSize * Toks.size());
-  return Result;
+  return std::move(Result);
 }
 
 bool operator==(const SemanticToken , const SemanticToken ) {
@@ -1030,7 +1030,7 @@
 Result["edits"] = *TE.edits;
   if (TE.tokens)
 Result["data"] = encodeTokens(*TE.tokens);
-  return Result;
+  return std::move(Result);
 }
 
 bool fromJSON(const llvm::json::Value , SemanticTokensParams ) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72035: [analyzer][NFC] Use CallEvent checker callback in GenericTaintChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.
Herald added a subscriber: ASDenysPetrov.



Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:113
  const CheckerContext ) {
-  const FunctionDecl *FDecl = C.getCalleeDecl(CE);
+  assert(Call.getDecl());
+  const FunctionDecl *FDecl = Call.getDecl()->getAsFunction();

rG878194414107e94600de31a11be09a347fb2598b!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72035



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

[PATCH] D78487: Explicitly move from llvm::json Array/Object to Value

[Sam McCall via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78487



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

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

[Oliver Stannard (Linaro) via Phabricator via cfe-commits]

ostannard added a reviewer: kristof.beyls.
ostannard added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76291



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

[PATCH] D77845: [analyzer][CallAndMessage][NFC] Change old callbacks to rely on CallEvent

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

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



Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:113-114
  const CheckerContext ) {
-  assert(Call.getDecl());
+  if (!Call.getDecl())
+return None;
   const FunctionDecl *FDecl = Call.getDecl()->getAsFunction();

Well, rG878194414107e94600de31a11be09a347fb2598b got this :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77845



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

[PATCH] D77735: [SveEmitter] Implement builtins for gathers/scatters

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added a reviewer: efriedma.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

This is a big patch, but looks reasonable to me.




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7451
+  }
+  llvm_unreachable("Unknown MemEltType");
+}

nit: to be consistent, do this in the default clause?



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7458
+llvm_unreachable("Invalid SVETypeFlag!");
+
+  case SVETypeFlags::EltTyInt8:

nit: no need for the newlines here and also below?



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7555
+
+  // From ACLE we always get . This might be incompatible with
+  // the actual type being loaded. Cast accordingly.

nit: can you rephrase this comment a bit I.e. the "From ACLE we always get ..." 
is a bit confusing I think. You want to say that this is how the ACLE defines 
this, but the IR looks different. You can also specify which bit is different, 
because that was not immediately obvious to me. 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77735



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

[clang-tools-extra] b36b889 - Explicitly move from llvm::json Array/Object to Value

[Sam McCall via cfe-commits]

Author: Michael Forster
Date: 2020-04-20T15:18:52+02:00
New Revision: b36b889a3b81b893c220c1858d16493152b36852

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

LOG: Explicitly move from llvm::json Array/Object to Value

Summary: The implicit conversion fails under Clang 3.8.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/Protocol.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Protocol.cpp 
b/clang-tools-extra/clangd/Protocol.cpp
index 5756e3b02871..b0c7ce2acb33 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -1002,7 +1002,7 @@ static llvm::json::Value 
encodeTokens(llvm::ArrayRef Toks) {
 Result.push_back(Tok.tokenModifiers);
   }
   assert(Result.size() == SemanticTokenEncodingSize * Toks.size());
-  return Result;
+  return std::move(Result);
 }
 
 bool operator==(const SemanticToken , const SemanticToken ) {
@@ -1030,7 +1030,7 @@ llvm::json::Value toJSON(const SemanticTokensOrEdits ) 
{
 Result["edits"] = *TE.edits;
   if (TE.tokens)
 Result["data"] = encodeTokens(*TE.tokens);
-  return Result;
+  return std::move(Result);
 }
 
 bool fromJSON(const llvm::json::Value , SemanticTokensParams ) {



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

[PATCH] D78487: Explicitly move from llvm::json Array/Object to Value

[Michael Forster via Phabricator via cfe-commits]

MForster created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous.
Herald added a project: clang.
MForster retitled this revision from "Explicitly move from llvm::json Array to 
Value" to "Explicitly move from llvm::json Array/Object to Value".
MForster added a reviewer: sammccall.

The implicit conversion fails under Clang 3.8.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78487

Files:
  clang-tools-extra/clangd/Protocol.cpp


Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1002,7 +1002,7 @@
 Result.push_back(Tok.tokenModifiers);
   }
   assert(Result.size() == SemanticTokenEncodingSize * Toks.size());
-  return Result;
+  return std::move(Result);
 }
 
 bool operator==(const SemanticToken , const SemanticToken ) {
@@ -1030,7 +1030,7 @@
 Result["edits"] = *TE.edits;
   if (TE.tokens)
 Result["data"] = encodeTokens(*TE.tokens);
-  return Result;
+  return std::move(Result);
 }
 
 bool fromJSON(const llvm::json::Value , SemanticTokensParams ) {


Index: clang-tools-extra/clangd/Protocol.cpp
===
--- clang-tools-extra/clangd/Protocol.cpp
+++ clang-tools-extra/clangd/Protocol.cpp
@@ -1002,7 +1002,7 @@
 Result.push_back(Tok.tokenModifiers);
   }
   assert(Result.size() == SemanticTokenEncodingSize * Toks.size());
-  return Result;
+  return std::move(Result);
 }
 
 bool operator==(const SemanticToken , const SemanticToken ) {
@@ -1030,7 +1030,7 @@
 Result["edits"] = *TE.edits;
   if (TE.tokens)
 Result["data"] = encodeTokens(*TE.tokens);
-  return Result;
+  return std::move(Result);
 }
 
 bool fromJSON(const llvm::json::Value , SemanticTokensParams ) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits