[PATCH] D66862: Make lround builtin constexpr (and others)

[Zoe Carver via Phabricator via cfe-commits]

zoecarver created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66862

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/math-builtins.cpp


Index: clang/test/SemaCXX/math-builtins.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/math-builtins.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++11 %s
+
+int main()
+{
+  constexpr float f = 12345.6789;
+
+  static_assert(__builtin_llround(f) == 12346, "");
+  static_assert(__builtin_llroundf(f) == 12346, "");
+
+  static_assert(__builtin_llrint(f) == 12346, "");
+  static_assert(__builtin_llrintf(f) == 12346, "");
+
+  static_assert(__builtin_lrint(f) == 12346, "");
+  static_assert(__builtin_lrintf(f) == 12346, "");
+
+  static_assert(__builtin_lround(f) == 12346, "");
+  static_assert(__builtin_lroundf(f) == 12346, "");
+}
\ No newline at end of file
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -9609,6 +9609,58 @@
 return Success(N == Val.getBitWidth() ? 0 : N + 1, E);
   }
 
+  case Builtin::BIlround:
+  case Builtin::BI__builtin_lround: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToDouble()), E);
+  }
+  case Builtin::BIlroundf:
+  case Builtin::BI__builtin_lroundf: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToFloat()), E);
+  }
+
+  case Builtin::BIllround:
+  case Builtin::BI__builtin_llround: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToDouble()), E);
+  }
+  case Builtin::BIllroundf:
+  case Builtin::BI__builtin_llroundf: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(llround(Val.convertToFloat()), E);
+  }
+
+  case Builtin::BIlrint:
+  case Builtin::BI__builtin_lrint: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToDouble()), E);
+  }
+  case Builtin::BIlrintf:
+  case Builtin::BI__builtin_lrintf: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lrint(Val.convertToFloat()), E);
+  }
+
+  case Builtin::BIllrint:
+  case Builtin::BI__builtin_llrint: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToDouble()), E);
+  }
+  case Builtin::BIllrintf:
+  case Builtin::BI__builtin_llrintf: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(llrint(Val.convertToFloat()), E);
+  }
+
   case Builtin::BI__builtin_fpclassify: {
 APFloat Val(0.0);
 if (!EvaluateFloat(E->getArg(5), Val, Info))


Index: clang/test/SemaCXX/math-builtins.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/math-builtins.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++11 %s
+
+int main()
+{
+  constexpr float f = 12345.6789;
+
+  static_assert(__builtin_llround(f) == 12346, "");
+  static_assert(__builtin_llroundf(f) == 12346, "");
+
+  static_assert(__builtin_llrint(f) == 12346, "");
+  static_assert(__builtin_llrintf(f) == 12346, "");
+
+  static_assert(__builtin_lrint(f) == 12346, "");
+  static_assert(__builtin_lrintf(f) == 12346, "");
+
+  static_assert(__builtin_lround(f) == 12346, "");
+  static_assert(__builtin_lroundf(f) == 12346, "");
+}
\ No newline at end of file
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -9609,6 +9609,58 @@
 return Success(N == Val.getBitWidth() ? 0 : N + 1, E);
   }
 
+  case Builtin::BIlround:
+  case Builtin::BI__builtin_lround: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToDouble()), E);
+  }
+  case Builtin::BIlroundf:
+  case Builtin::BI__builtin_lroundf: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToFloat()), E);
+  }
+
+  case Builtin::BIllround:
+  case Builtin::BI__builtin_llround: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(lround(Val.convertToDouble()), E);
+  }
+  case Builtin::BIllroundf:
+  case Builtin::BI__builtin_llroundf: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   Success(llround(Val.convertToFloat()), E);
+  }
+
+  case Builtin::BIlrint:
+  case Builtin::BI__builtin_lrint: {
+APFloat Val(0.0);
+return EvaluateFloat(E->getArg(0), Val, Info) &&
+   

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

Sorry for the break.
Unfortunately the patch does not apply cleanly anymore. Can you rebase to 
latest trunk?

I surprisingly did not know about `__declspec(selectany)`. I still think having 
the symbol only defined in that loadable module, but not in the statically 
linked library is the cleaner solution than relying on linker cleverness. If 
unused by design, there is no reason why the symbol should be even there. Can 
you convince me otherwise?




Comment at: llvm/include/llvm/Support/Compiler.h:162
 
 // FIXME: Provide this for PE/COFF targets.
 #if (__has_attribute(weak) || LLVM_GNUC_PREREQ(4, 0, 0)) &&
\

With `__declspec(selectany)`, the FIXME should be removed.

However, I think this does not exactly match `__attribute__((__weak__))`: Weak 
symbols can still be undefined whereas `selectany`, if I understand MS's docs 
correctly, requires one valid symbol during the linking phase (Well, we 
currently don't support LLVM dlls anyway). This might be what you want for this 
patch, but classically I'd expect a weak symbol as a symbol that can resolve to 
NULL.

Also, I am not sure mingw/cygwin support it (I'd try out once I can apply the 
patch again).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446



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

[PATCH] D66783: [clang-doc] Use llvm::createStringError and canonicalize error messages

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370155: [clang-doc] Use llvm::createStringError and 
canonicalize error messages (authored by MaskRay, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66783

Files:
  clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp
  clang-tools-extra/trunk/clang-doc/Generators.cpp
  clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/trunk/clang-doc/MDGenerator.cpp
  clang-tools-extra/trunk/clang-doc/Representation.cpp
  clang-tools-extra/trunk/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp

Index: clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
@@ -172,8 +172,8 @@
   llvm::sys::path::native(Root, Path);
   llvm::sys::path::append(Path, RelativePath);
   if (CreateDirectory(Path))
-return llvm::make_error("Unable to create directory.\n",
-   llvm::inconvertibleErrorCode());
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "failed to create directory");
   llvm::sys::path::append(Path, Name + Ext);
   return Path;
 }
Index: clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp
===
--- clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp
+++ clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp
@@ -25,8 +25,8 @@
 
 llvm::Error decodeRecord(Record R, SymbolID , llvm::StringRef Blob) {
   if (R[0] != BitCodeConstants::USRHashSize)
-return llvm::make_error("Incorrect USR size.\n",
-   llvm::inconvertibleErrorCode());
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "incorrect USR size");
 
   // First position in the record is the length of the following array, so we
   // copy the following elements to the field.
@@ -42,8 +42,8 @@
 
 llvm::Error decodeRecord(Record R, int , llvm::StringRef Blob) {
   if (R[0] > INT_MAX)
-return llvm::make_error("Integer too large to parse.\n",
-   llvm::inconvertibleErrorCode());
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "integer too large to parse");
   Field = (int)R[0];
   return llvm::Error::success();
 }
@@ -58,8 +58,8 @@
 Field = (AccessSpecifier)R[0];
 return llvm::Error::success();
   default:
-return llvm::make_error(
-"Invalid value for AccessSpecifier.\n", llvm::inconvertibleErrorCode());
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "invalid value for AccessSpecifier");
   }
 }
 
@@ -73,16 +73,16 @@
 Field = (TagTypeKind)R[0];
 return llvm::Error::success();
   default:
-return llvm::make_error(
-"Invalid value for TagTypeKind.\n", llvm::inconvertibleErrorCode());
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "invalid value for TagTypeKind");
   }
 }
 
 llvm::Error decodeRecord(Record R, llvm::Optional ,
  llvm::StringRef Blob) {
   if (R[0] > INT_MAX)
-return llvm::make_error("Integer too large to parse.\n",
-   llvm::inconvertibleErrorCode());
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "integer too large to parse");
   Field.emplace((int)R[0], Blob, (bool)R[1]);
   return llvm::Error::success();
 }
@@ -97,8 +97,8 @@
 Field = IT;
 return llvm::Error::success();
   }
-  return llvm::make_error("Invalid value for InfoType.\n",
- llvm::inconvertibleErrorCode());
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "invalid value for InfoType");
 }
 
 llvm::Error decodeRecord(Record R, FieldId , llvm::StringRef Blob) {
@@ -113,8 +113,8 @@
 Field = F;
 return llvm::Error::success();
   }
-  return llvm::make_error("Invalid value for FieldId.\n",
- llvm::inconvertibleErrorCode());
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "invalid value for FieldId");
 }
 
 llvm::Error decodeRecord(Record R,
@@ -127,8 +127,8 @@
 llvm::Error decodeRecord(Record R, llvm::SmallVectorImpl ,
  llvm::StringRef Blob) {
   if (R[0] > INT_MAX)
-return llvm::make_error("Integer too large to parse.\n",
- 

[clang-tools-extra] r370155 - [clang-doc] Use llvm::createStringError and canonicalize error messages

[Fangrui Song via cfe-commits]

Author: maskray
Date: Tue Aug 27 19:56:03 2019
New Revision: 370155

URL: http://llvm.org/viewvc/llvm-project?rev=370155=rev
Log:
[clang-doc] Use llvm::createStringError and canonicalize error messages

"Bad block found.\n" -> "bad block found"

The lower cased form with no full stop or newline is more common in LLVM
tools.

Reviewed By: juliehockett

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

Modified:
clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp
clang-tools-extra/trunk/clang-doc/Generators.cpp
clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
clang-tools-extra/trunk/clang-doc/MDGenerator.cpp
clang-tools-extra/trunk/clang-doc/Representation.cpp
clang-tools-extra/trunk/clang-doc/YAMLGenerator.cpp
clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp

Modified: clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp?rev=370155=370154=370155=diff
==
--- clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/BitcodeReader.cpp Tue Aug 27 19:56:03 2019
@@ -25,8 +25,8 @@ llvm::Error decodeRecord(Record R, llvm:
 
 llvm::Error decodeRecord(Record R, SymbolID , llvm::StringRef Blob) {
   if (R[0] != BitCodeConstants::USRHashSize)
-return llvm::make_error("Incorrect USR size.\n",
-   llvm::inconvertibleErrorCode());
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "incorrect USR size");
 
   // First position in the record is the length of the following array, so we
   // copy the following elements to the field.
@@ -42,8 +42,8 @@ llvm::Error decodeRecord(Record R, bool
 
 llvm::Error decodeRecord(Record R, int , llvm::StringRef Blob) {
   if (R[0] > INT_MAX)
-return llvm::make_error("Integer too large to parse.\n",
-   llvm::inconvertibleErrorCode());
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "integer too large to parse");
   Field = (int)R[0];
   return llvm::Error::success();
 }
@@ -58,8 +58,8 @@ llvm::Error decodeRecord(Record R, Acces
 Field = (AccessSpecifier)R[0];
 return llvm::Error::success();
   default:
-return llvm::make_error(
-"Invalid value for AccessSpecifier.\n", 
llvm::inconvertibleErrorCode());
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "invalid value for AccessSpecifier");
   }
 }
 
@@ -73,16 +73,16 @@ llvm::Error decodeRecord(Record R, TagTy
 Field = (TagTypeKind)R[0];
 return llvm::Error::success();
   default:
-return llvm::make_error(
-"Invalid value for TagTypeKind.\n", llvm::inconvertibleErrorCode());
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "invalid value for TagTypeKind");
   }
 }
 
 llvm::Error decodeRecord(Record R, llvm::Optional ,
  llvm::StringRef Blob) {
   if (R[0] > INT_MAX)
-return llvm::make_error("Integer too large to parse.\n",
-   llvm::inconvertibleErrorCode());
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "integer too large to parse");
   Field.emplace((int)R[0], Blob, (bool)R[1]);
   return llvm::Error::success();
 }
@@ -97,8 +97,8 @@ llvm::Error decodeRecord(Record R, InfoT
 Field = IT;
 return llvm::Error::success();
   }
-  return llvm::make_error("Invalid value for InfoType.\n",
- llvm::inconvertibleErrorCode());
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "invalid value for InfoType");
 }
 
 llvm::Error decodeRecord(Record R, FieldId , llvm::StringRef Blob) {
@@ -113,8 +113,8 @@ llvm::Error decodeRecord(Record R, Field
 Field = F;
 return llvm::Error::success();
   }
-  return llvm::make_error("Invalid value for FieldId.\n",
- llvm::inconvertibleErrorCode());
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ "invalid value for FieldId");
 }
 
 llvm::Error decodeRecord(Record R,
@@ -127,8 +127,8 @@ llvm::Error decodeRecord(Record R,
 llvm::Error decodeRecord(Record R, llvm::SmallVectorImpl ,
  llvm::StringRef Blob) {
   if (R[0] > INT_MAX)
-return llvm::make_error("Integer too large to parse.\n",
-   llvm::inconvertibleErrorCode());
+return llvm::createStringError(llvm::inconvertibleErrorCode(),
+   "integer too large to parse");
   Field.emplace_back((int)R[0], Blob, (bool)R[1]);
   return 

[PATCH] D66839: Fix stack address builtin for negative numbers

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

Thanks for the explanation. Should I then not try to "fix" the issue? Or should 
I update sema checking?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66839



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

[PATCH] D66360: Expose constructing a virtual method dispatch through the include/clang/CodeGen APIs.

[Parker Schuh via Phabricator via cfe-commits]

pschuh updated this revision to Diff 217566.
pschuh added a comment.
Herald added a subscriber: mgorny.

Your suggestions of pulling the builder out as an API and exposing CGCallee 
made everything a lot simpler. Unfortunately, I had to split the CGCallee 
header because it threatened to pull EHScopeStack.h and CGValue.h to be public 
(and transitively CodeGenTBAA.h). The old CGCall.h header now just contains the 
Args and ReturnValueSlot.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66360

Files:
  include/clang/CodeGen/Address.h
  include/clang/CodeGen/CGCallee.h
  include/clang/CodeGen/CodeGenABITypes.h
  include/clang/CodeGen/FunctionBuilder.h
  lib/CodeGen/Address.h
  lib/CodeGen/CGBuilder.h
  lib/CodeGen/CGCall.h
  lib/CodeGen/CGCleanup.h
  lib/CodeGen/CGObjCRuntime.h
  lib/CodeGen/CGValue.h
  lib/CodeGen/CMakeLists.txt
  lib/CodeGen/CodeGenABITypes.cpp
  lib/CodeGen/FunctionBuilder.cpp

Index: lib/CodeGen/FunctionBuilder.cpp
===
--- /dev/null
+++ lib/CodeGen/FunctionBuilder.cpp
@@ -0,0 +1,48 @@
+//===--- ModuleBuilder.cpp - Emit LLVM Code from ASTs -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// This exposes a subset of the CodeGenFunction API.
+//
+//===--===//
+
+#include "clang/CodeGen/FunctionBuilder.h"
+#include "CGCXXABI.h"
+#include "CodeGenFunction.h"
+#include "clang/Basic/TargetInfo.h"
+
+using namespace clang;
+using namespace CodeGen;
+
+namespace {
+class FunctionBuilderImpl : public FunctionBuilder {
+ public:
+  CodeGenFunction CGF;
+
+  FunctionBuilderImpl(CodeGenModule )
+  : CGF(CGM, /*suppressNewContext=*/true) {}
+};
+} // namespace
+
+FunctionBuilder::~FunctionBuilder() {}
+
+std::unique_ptr FunctionBuilder::Create(CodeGenModule ) {
+  return std::unique_ptr(new FunctionBuilderImpl(CGM));
+}
+
+CodeGenFunction ::CGF() {
+  return static_cast(this)->CGF;
+}
+
+llvm::IRBuilderBase ::getBuilder() { return CGF().Builder; }
+
+Address FunctionBuilder::emitAdjustThisArgumentForVirtualFunctionCall(
+Address thisAddr, GlobalDecl GD, bool VirtualCall) {
+  return CGF().CGM.getCXXABI().adjustThisArgumentForVirtualFunctionCall(
+  CGF(), GD, thisAddr, VirtualCall);
+}
Index: lib/CodeGen/CodeGenABITypes.cpp
===
--- lib/CodeGen/CodeGenABITypes.cpp
+++ lib/CodeGen/CodeGenABITypes.cpp
@@ -52,6 +52,17 @@
   return CGM.getTypes().arrangeCXXMethodType(RD, FTP, MD);
 }
 
+const CGFunctionInfo &
+CodeGen::arrangeCXXMethodDeclaration(CodeGenModule ,
+ const CXXMethodDecl *MD) {
+  return CGM.getTypes().arrangeCXXMethodDeclaration(MD);
+}
+
+llvm::FunctionType *CodeGen::getFunctionType(CodeGenModule ,
+ const CGFunctionInfo ) {
+  return CGM.getTypes().GetFunctionType(FI);
+}
+
 const CGFunctionInfo &
 CodeGen::arrangeFreeFunctionCall(CodeGenModule ,
  CanQualType returnType,
Index: lib/CodeGen/CMakeLists.txt
===
--- lib/CodeGen/CMakeLists.txt
+++ lib/CodeGen/CMakeLists.txt
@@ -84,6 +84,7 @@
   CodeGenTypes.cpp
   ConstantInitBuilder.cpp
   CoverageMappingGen.cpp
+  FunctionBuilder.cpp
   ItaniumCXXABI.cpp
   MacroPPCallbacks.cpp
   MicrosoftCXXABI.cpp
Index: lib/CodeGen/CGValue.h
===
--- lib/CodeGen/CGValue.h
+++ lib/CodeGen/CGValue.h
@@ -18,7 +18,7 @@
 #include "clang/AST/Type.h"
 #include "llvm/IR/Value.h"
 #include "llvm/IR/Type.h"
-#include "Address.h"
+#include "clang/CodeGen/Address.h"
 #include "CodeGenTBAA.h"
 
 namespace llvm {
Index: lib/CodeGen/CGObjCRuntime.h
===
--- lib/CodeGen/CGObjCRuntime.h
+++ lib/CodeGen/CGObjCRuntime.h
@@ -18,6 +18,7 @@
 #include "CGCall.h"
 #include "CGCleanup.h"
 #include "CGValue.h"
+#include "ABIInfo.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/Basic/IdentifierTable.h" // Selector
 
Index: lib/CodeGen/CGCleanup.h
===
--- lib/CodeGen/CGCleanup.h
+++ lib/CodeGen/CGCleanup.h
@@ -15,7 +15,7 @@
 
 #include "EHScopeStack.h"
 
-#include "Address.h"
+#include "clang/CodeGen/Address.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 
Index: lib/CodeGen/CGCall.h
===
--- lib/CodeGen/CGCall.h
+++ lib/CodeGen/CGCall.h
@@ -19,11 +19,9 @@
 #include "clang/AST/CanonicalType.h"
 #include 

r370151 - Fixup build of clang-interpreter example after change in r370122.

[Douglas Yung via cfe-commits]

Author: dyung
Date: Tue Aug 27 19:13:24 2019
New Revision: 370151

URL: http://llvm.org/viewvc/llvm-project?rev=370151=rev
Log:
Fixup build of clang-interpreter example after change in r370122.

This should fix the build failure on llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast.

Modified:
cfe/trunk/examples/clang-interpreter/main.cpp

Modified: cfe/trunk/examples/clang-interpreter/main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/examples/clang-interpreter/main.cpp?rev=370151=370150=370151=diff
==
--- cfe/trunk/examples/clang-interpreter/main.cpp (original)
+++ cfe/trunk/examples/clang-interpreter/main.cpp Tue Aug 27 19:13:24 2019
@@ -173,11 +173,7 @@ int main(int argc, const char **argv) {
   // Initialize a compiler invocation object from the clang (-cc1) arguments.
   const llvm::opt::ArgStringList  = Cmd.getArguments();
   std::unique_ptr CI(new CompilerInvocation);
-  CompilerInvocation::CreateFromArgs(*CI,
- const_cast(CCArgs.data()),
- const_cast(CCArgs.data()) +
-   CCArgs.size(),
- Diags);
+  CompilerInvocation::CreateFromArgs(*CI, CCArgs, Diags);
 
   // Show the invocation, with -v.
   if (CI->getHeaderSearchOpts().Verbose) {


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

[PATCH] D66668: [OPENMP][Analysis] Add analysis of the map clauses.

[Artem Dergachev via Phabricator via cfe-commits]

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

Yes please! :)


Repository:
  rC Clang

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

https://reviews.llvm.org/D8



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

[PATCH] D66765: [analyzer] (Urgent!) Add 9.0.0. release notes.

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

Szelethus marked 10 inline comments as done.
Szelethus added a comment.

In D66765#1646237 , @NoQ wrote:

> I approve the patch and i don't see anything obvious that we're missing out 
> (@Szelethus, your GSoC isn't on by default back in 9.0, only in master, 
> right?).


Precisely.

> I guess we should really try harder to fill it in with every patch that 
> deserves it.

*judges everyone who lets any important patch through without adding it to the 
release notes from now on*




Comment at: clang/docs/ReleaseNotes.rst:251
+
+  - ``-analyzer-checker-help-alpha``: The list of incomplet and inkorrekt
+checkers not yet advised to be turned on.

NoQ wrote:
> xbolva00 wrote:
> > Please fix this
> I think this was supposed to be a joke but i don't insist^^
Let's reserve jokes for greater depths within the developer docs :^)


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

https://reviews.llvm.org/D66765



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

[PATCH] D66765: [analyzer] (Urgent!) Add 9.0.0. release notes.

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

Szelethus updated this revision to Diff 217564.
Szelethus added a comment.

Fixing inlines!


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

https://reviews.llvm.org/D66765

Files:
  clang/docs/ClangStaticAnalyzer.rst
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -222,10 +222,57 @@
 Static Analyzer
 ---
 
+- Fixed a bug where an incorrect checker name would be displayed for a bug
+  report.`
+
+- New checker: 'security.insecureAPI.DeprecatedOrUnsafeBufferHandling' to 
detect
+  uses of unsafe/deprecated buffer handling functions for C code using the C11
+  standard or newer.
+
+- New checker: 'osx.MIGChecker' to find violations of the Mach Interface
+  Generator calling convention
+
+- New checker: 'optin.osx.OSObjectCStyleCast' to find C-style casts of of XNU
+  libkern OSObjects
+
+- New package: 'apiModeling.llvm' contains modeling checkers to improve the
+  accuracy of reports on LLVM's own codebase.
+
+- The Static Analyzer received a
+  :ref:`developer documentation `.
+
 - The UninitializedObject checker is now considered as stable.
   (moved from the 'alpha.cplusplus' to the 'optin.cplusplus' package)
 
-...
+- New frontend flags: The list of available checkers are now split into 3
+  different frontend flags:
+
+  - ``-analyzer-checker-help``: The list of user-facing, stable checkers.
+
+  - ``-analyzer-checker-help-alpha``: The list of in-development
+checkers not yet advised to be turned on.
+
+  - ``-analyzer-checker-help-developer``: Checkers never meant to be
+enabled/disabled by hand + development checkers.
+
+- New frontend flags: While they have always been around, for the first time,
+  checker and package options are listable:
+
+  - ``-analyzer-checker-option-help``: The list of user-facing, stable checker
+and package options.
+
+  - ``-analyzer-checker-option-help-alpha``: The list of in-development checker
+options not yet advised to be used.
+
+  - ``-analyzer-checker-option-help-developer``: Options never meant to be
+enabled/disabled by hand + development options.
+
+- New frontend flag: ``-analyzer-werror`` to turn analyzer warnings into 
errors.
+
+- Numerous fixes to increase the stability of the experimental cross 
translation
+  unit analysis (CTU).
+
+- CTU now handles virtual functions as well.
 
 .. _release-notes-ubsan:
 
Index: clang/docs/ClangStaticAnalyzer.rst
===
--- clang/docs/ClangStaticAnalyzer.rst
+++ clang/docs/ClangStaticAnalyzer.rst
@@ -2,6 +2,8 @@
 Clang Static Analyzer
 =
 
+.. _clang-static-analyzer-docs:
+
 The Clang Static Analyzer is a source code analysis tool that finds bugs in C, 
C++, and Objective-C programs.
 It implements *path-sensitive*, *inter-procedural analysis* based on *symbolic 
execution* technique.
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -222,10 +222,57 @@
 Static Analyzer
 ---
 
+- Fixed a bug where an incorrect checker name would be displayed for a bug
+  report.`
+
+- New checker: 'security.insecureAPI.DeprecatedOrUnsafeBufferHandling' to detect
+  uses of unsafe/deprecated buffer handling functions for C code using the C11
+  standard or newer.
+
+- New checker: 'osx.MIGChecker' to find violations of the Mach Interface
+  Generator calling convention
+
+- New checker: 'optin.osx.OSObjectCStyleCast' to find C-style casts of of XNU
+  libkern OSObjects
+
+- New package: 'apiModeling.llvm' contains modeling checkers to improve the
+  accuracy of reports on LLVM's own codebase.
+
+- The Static Analyzer received a
+  :ref:`developer documentation `.
+
 - The UninitializedObject checker is now considered as stable.
   (moved from the 'alpha.cplusplus' to the 'optin.cplusplus' package)
 
-...
+- New frontend flags: The list of available checkers are now split into 3
+  different frontend flags:
+
+  - ``-analyzer-checker-help``: The list of user-facing, stable checkers.
+
+  - ``-analyzer-checker-help-alpha``: The list of in-development
+checkers not yet advised to be turned on.
+
+  - ``-analyzer-checker-help-developer``: Checkers never meant to be
+enabled/disabled by hand + development checkers.
+
+- New frontend flags: While they have always been around, for the first time,
+  checker and package options are listable:
+
+  - ``-analyzer-checker-option-help``: The list of user-facing, stable checker
+and package options.
+
+  - ``-analyzer-checker-option-help-alpha``: The list of in-development checker
+options not yet advised to be used.
+
+  - ``-analyzer-checker-option-help-developer``: Options never meant to be
+enabled/disabled by hand + development options.
+
+- New frontend flag: 

[PATCH] D66847: [analyzer] Fix analyzer warnings.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Filed the remaining false positive back on myself as 
https://bugs.llvm.org/show_bug.cgi?id=43135.


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

https://reviews.llvm.org/D66847



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

[PATCH] D66847: [analyzer] Fix analyzer warnings.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D66847#1648209 , @Szelethus wrote:

>   if (isa(D)) {
> const auto *FD = dyn_cast(D); // warn: D is known to be a 
> FunctionDecl, prefer using cast<>
> // ...
>   }
>




In D66847#1648339 , @Szelethus wrote:

>   if (A) {
> const auto *B = dyn_cast_or_null(A); // warn: A is known to 
> be non-null, prefer dyn_cast
>   }
>


Both of these are must-problems. They are hard to solve via symbolic execution 
because all execution paths need to be taken into account simultaneously. Eg.:

  if (isa(D)) {
if (coin)
  D = getDecl(); // Maybe not a FunctionDecl.
  
// It's not enough to know that D is a FunctionDecl assuming coin is false.
const auto *FD = dyn_cast(D); // no-warning:
  }

We could maybe make a syntactic checker or a data flow checker to find these 
anti-patterns. @Charusso also had an idea for a clang-tidy checker that emits 
fixits for code like `dyn_cast(x)->foo()` ("use 'cast' instead").


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

https://reviews.llvm.org/D66847



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

[PATCH] D66601: [Clang][Bundler] Do not require host triple for extracting device bundles

[Sergey Dmitriev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370143: [Clang][Bundler] Do not require host triple for 
extracting device bundles (authored by sdmitriev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66601?vs=217522=217558#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66601

Files:
  cfe/trunk/test/Driver/clang-offload-bundler.c
  cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Index: cfe/trunk/test/Driver/clang-offload-bundler.c
===
--- cfe/trunk/test/Driver/clang-offload-bundler.c
+++ cfe/trunk/test/Driver/clang-offload-bundler.c
@@ -156,14 +156,20 @@
 // RUN: diff %t.i %t.res.i
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=i -targets=openmp-powerpc64le-ibm-linux-gnu -outputs=%t.res.tgt1 -inputs=%t.bundle3.i -unbundle
+// RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: clang-offload-bundler -type=ii -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.res.ii,%t.res.tgt1,%t.res.tgt2 -inputs=%t.bundle3.ii -unbundle
 // RUN: diff %t.ii %t.res.ii
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=ii -targets=openmp-x86_64-pc-linux-gnu -outputs=%t.res.tgt2 -inputs=%t.bundle3.ii -unbundle
+// RUN: diff %t.tgt2 %t.res.tgt2
 // RUN: clang-offload-bundler -type=ll -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.res.ll,%t.res.tgt1,%t.res.tgt2 -inputs=%t.bundle3.ll -unbundle
 // RUN: diff %t.ll %t.res.ll
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=ll -targets=openmp-powerpc64le-ibm-linux-gnu -outputs=%t.res.tgt1 -inputs=%t.bundle3.ll -unbundle
+// RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: clang-offload-bundler -type=s -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.res.s,%t.res.tgt1,%t.res.tgt2 -inputs=%t.bundle3.s -unbundle
 // RUN: diff %t.s %t.res.s
 // RUN: diff %t.tgt1 %t.res.tgt1
@@ -172,6 +178,8 @@
 // RUN: diff %t.s %t.res.s
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=s -targets=openmp-x86_64-pc-linux-gnu -outputs=%t.res.tgt2 -inputs=%t.bundle3.s -unbundle
+// RUN: diff %t.tgt2 %t.res.tgt2
 
 // Check if we can unbundle a file with no magic strings.
 // RUN: clang-offload-bundler -type=s -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.res.s,%t.res.tgt1,%t.res.tgt2 -inputs=%t.s -unbundle
@@ -183,6 +191,10 @@
 // RUN: diff %t.empty %t.res.tgt1
 // RUN: diff %t.empty %t.res.tgt2
 
+// Check that bindler prints an error if given host bundle does not exist in the fat binary.
+// RUN: not clang-offload-bundler -type=s -targets=host-x86_64-xxx-linux-gnu,openmp-powerpc64le-ibm-linux-gnu -outputs=%t.res.s,%t.res.tgt1 -inputs=%t.bundle3.s -unbundle 2>&1 | FileCheck %s --check-prefix CK-NO-HOST-BUNDLE
+// CK-NO-HOST-BUNDLE: error: Can't find bundle for the host target
+
 //
 // Check binary bundle/unbundle. The content that we have before bundling must be the same we have after unbundling.
 //
@@ -194,10 +206,14 @@
 // RUN: diff %t.bc %t.res.bc
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=bc -targets=openmp-powerpc64le-ibm-linux-gnu -outputs=%t.res.tgt1 -inputs=%t.bundle3.bc -unbundle
+// RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: clang-offload-bundler -type=gch -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.res.gch,%t.res.tgt1,%t.res.tgt2 -inputs=%t.bundle3.gch -unbundle
 // RUN: diff %t.ast %t.res.gch
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=gch -targets=openmp-x86_64-pc-linux-gnu -outputs=%t.res.tgt2 -inputs=%t.bundle3.gch -unbundle
+// RUN: diff %t.tgt2 %t.res.tgt2
 // RUN: clang-offload-bundler -type=ast -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.res.ast,%t.res.tgt1,%t.res.tgt2 -inputs=%t.bundle3.ast -unbundle
 // RUN: diff %t.ast %t.res.ast
 // RUN: diff %t.tgt1 %t.res.tgt1
@@ -210,6 +226,8 @@
 // RUN: diff %t.ast %t.res.ast
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=ast -targets=openmp-powerpc64le-ibm-linux-gnu -outputs=%t.res.tgt1 -inputs=%t.bundle3.ast -unbundle
+// RUN: diff %t.tgt1 %t.res.tgt1
 
 // Check if we can unbundle a file with no magic strings.
 // RUN: clang-offload-bundler -type=bc 

[PATCH] D66847: [analyzer] Fix analyzer warnings.

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

Szelethus added a comment.

Another common mistake is this:

  if (A) {
const auto *B = dyn_cast_or_null(A); // warn: A is known to 
be non-null, prefer dyn_cast
  }


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

https://reviews.llvm.org/D66847



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

r370143 - [Clang][Bundler] Do not require host triple for extracting device bundles

[Sergey Dmitriev via cfe-commits]

Author: sdmitriev
Date: Tue Aug 27 18:26:13 2019
New Revision: 370143

URL: http://llvm.org/viewvc/llvm-project?rev=370143=rev
Log:
[Clang][Bundler] Do not require host triple for extracting device bundles

Bundler currently requires host triple to be provided no matter if you are 
performing bundling or unbundling, but for unbundling operation such 
requirement is too restrictive. You may for example want to examine device part 
of the object for a particular offload target, but you have to extract host 
part as well even though you do not need it. Host triple isn't really needed 
for unbundling, so this patch removes that requirement.

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

Modified:
cfe/trunk/test/Driver/clang-offload-bundler.c
cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Modified: cfe/trunk/test/Driver/clang-offload-bundler.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/clang-offload-bundler.c?rev=370143=370142=370143=diff
==
--- cfe/trunk/test/Driver/clang-offload-bundler.c (original)
+++ cfe/trunk/test/Driver/clang-offload-bundler.c Tue Aug 27 18:26:13 2019
@@ -156,14 +156,20 @@
 // RUN: diff %t.i %t.res.i
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=i 
-targets=openmp-powerpc64le-ibm-linux-gnu -outputs=%t.res.tgt1 
-inputs=%t.bundle3.i -unbundle
+// RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: clang-offload-bundler -type=ii 
-targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu
 -outputs=%t.res.ii,%t.res.tgt1,%t.res.tgt2 -inputs=%t.bundle3.ii -unbundle
 // RUN: diff %t.ii %t.res.ii
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=ii -targets=openmp-x86_64-pc-linux-gnu 
-outputs=%t.res.tgt2 -inputs=%t.bundle3.ii -unbundle
+// RUN: diff %t.tgt2 %t.res.tgt2
 // RUN: clang-offload-bundler -type=ll 
-targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu
 -outputs=%t.res.ll,%t.res.tgt1,%t.res.tgt2 -inputs=%t.bundle3.ll -unbundle
 // RUN: diff %t.ll %t.res.ll
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=ll 
-targets=openmp-powerpc64le-ibm-linux-gnu -outputs=%t.res.tgt1 
-inputs=%t.bundle3.ll -unbundle
+// RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: clang-offload-bundler -type=s 
-targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu
 -outputs=%t.res.s,%t.res.tgt1,%t.res.tgt2 -inputs=%t.bundle3.s -unbundle
 // RUN: diff %t.s %t.res.s
 // RUN: diff %t.tgt1 %t.res.tgt1
@@ -172,6 +178,8 @@
 // RUN: diff %t.s %t.res.s
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=s -targets=openmp-x86_64-pc-linux-gnu 
-outputs=%t.res.tgt2 -inputs=%t.bundle3.s -unbundle
+// RUN: diff %t.tgt2 %t.res.tgt2
 
 // Check if we can unbundle a file with no magic strings.
 // RUN: clang-offload-bundler -type=s 
-targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu
 -outputs=%t.res.s,%t.res.tgt1,%t.res.tgt2 -inputs=%t.s -unbundle
@@ -183,6 +191,10 @@
 // RUN: diff %t.empty %t.res.tgt1
 // RUN: diff %t.empty %t.res.tgt2
 
+// Check that bindler prints an error if given host bundle does not exist in 
the fat binary.
+// RUN: not clang-offload-bundler -type=s 
-targets=host-x86_64-xxx-linux-gnu,openmp-powerpc64le-ibm-linux-gnu 
-outputs=%t.res.s,%t.res.tgt1 -inputs=%t.bundle3.s -unbundle 2>&1 | FileCheck 
%s --check-prefix CK-NO-HOST-BUNDLE
+// CK-NO-HOST-BUNDLE: error: Can't find bundle for the host target
+
 //
 // Check binary bundle/unbundle. The content that we have before bundling must 
be the same we have after unbundling.
 //
@@ -194,10 +206,14 @@
 // RUN: diff %t.bc %t.res.bc
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=bc 
-targets=openmp-powerpc64le-ibm-linux-gnu -outputs=%t.res.tgt1 
-inputs=%t.bundle3.bc -unbundle
+// RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: clang-offload-bundler -type=gch 
-targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu
 -outputs=%t.res.gch,%t.res.tgt1,%t.res.tgt2 -inputs=%t.bundle3.gch -unbundle
 // RUN: diff %t.ast %t.res.gch
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=gch -targets=openmp-x86_64-pc-linux-gnu 
-outputs=%t.res.tgt2 -inputs=%t.bundle3.gch -unbundle
+// RUN: diff %t.tgt2 %t.res.tgt2
 // RUN: clang-offload-bundler -type=ast 
-targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu
 -outputs=%t.res.ast,%t.res.tgt1,%t.res.tgt2 -inputs=%t.bundle3.ast -unbundle
 // RUN: diff %t.ast %t.res.ast
 // RUN: diff %t.tgt1 %t.res.tgt1
@@ -210,6 +226,8 @@
 // RUN: diff %t.ast %t.res.ast
 // RUN: diff 

[PATCH] D66856: [Sema] Suppress -Wformat diagnostics for bool types when printed using %hhd

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: aaron.ballman, Nathan-Huckleberry.
Herald added subscribers: ributzka, dexonsmith, jkorous.
Herald added a project: clang.

Instead, emit them under -Wformat-pedantic. This is particularly important for 
us because we have a lot of code that uses `%hhd` to print BOOL values, since 
on macOS BOOL is a typedef for signed char, and we emit fix-its recommending 
`%hhd` to print it. Now that this code is compiled on a target where BOOL is a 
typedef for `_Bool`, we're getting these -Wformat warnings. These aren't all 
that useful.

rdar://problem/54579473 Clang should not warn when using %hhd or %hd format 
specifiers on BOOL values


Repository:
  rC Clang

https://reviews.llvm.org/D66856

Files:
  clang/lib/AST/FormatString.cpp
  clang/test/Sema/format-bool.c


Index: clang/test/Sema/format-bool.c
===
--- /dev/null
+++ clang/test/Sema/format-bool.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -xc %s -verify -DBOOL=_Bool
+// RUN: %clang_cc1 -xc++ %s -verify -DBOOL=bool
+// RUN: %clang_cc1 -xc %s -verify -DBOOL=_Bool -Wformat-pedantic -DPEDANTIC
+// RUN: %clang_cc1 -xc++ %s -verify -DBOOL=bool -Wformat-pedantic -DPEDANTIC
+
+__attribute__((format(__printf__, 1, 2)))
+int p(const char *fmt, ...);
+
+BOOL b;
+
+int main() {
+  p("%d", b);
+  p("%hd", b);
+  p("%hhd", b);
+#ifdef PEDANTIC
+  // expected-warning@-3 {{format specifies type 'short' but the argument has 
type}}
+  // expected-warning@-3 {{format specifies type 'char' but the argument has 
type}}
+#endif
+  p("%u", b);
+  p("%hu", b);
+  p("%hhu", b);
+#ifdef PEDANTIC
+  // expected-warning@-3 {{format specifies type 'unsigned short' but the 
argument has type}}
+  // expected-warning@-3 {{format specifies type 'unsigned char' but the 
argument has type}}
+#endif
+  p("%c", b);
+  p("%f", b); // expected-warning{{format specifies type 'double' but the 
argument has type}}
+  p("%ld", b); // expected-warning{{format specifies type 'long' but the 
argument has type}}
+}
Index: clang/lib/AST/FormatString.cpp
===
--- clang/lib/AST/FormatString.cpp
+++ clang/lib/AST/FormatString.cpp
@@ -360,7 +360,9 @@
   case BuiltinType::UChar:
   case BuiltinType::Char_U:
 return Match;
-}
+  case BuiltinType::Bool:
+return NoMatchPedantic;
+  }
   return NoMatch;
 }
 
@@ -390,6 +392,11 @@
   return NoMatchPedantic;
 return T == C.UnsignedCharTy || T == C.SignedCharTy ? Match
 : NoMatch;
+  case BuiltinType::Bool:
+if (T == C.UnsignedShortTy || T == C.ShortTy ||
+T == C.UnsignedCharTy || T == C.SignedCharTy)
+  return NoMatchPedantic;
+return NoMatch;
   case BuiltinType::Short:
 return T == C.UnsignedShortTy ? Match : NoMatch;
   case BuiltinType::UShort:


Index: clang/test/Sema/format-bool.c
===
--- /dev/null
+++ clang/test/Sema/format-bool.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -xc %s -verify -DBOOL=_Bool
+// RUN: %clang_cc1 -xc++ %s -verify -DBOOL=bool
+// RUN: %clang_cc1 -xc %s -verify -DBOOL=_Bool -Wformat-pedantic -DPEDANTIC
+// RUN: %clang_cc1 -xc++ %s -verify -DBOOL=bool -Wformat-pedantic -DPEDANTIC
+
+__attribute__((format(__printf__, 1, 2)))
+int p(const char *fmt, ...);
+
+BOOL b;
+
+int main() {
+  p("%d", b);
+  p("%hd", b);
+  p("%hhd", b);
+#ifdef PEDANTIC
+  // expected-warning@-3 {{format specifies type 'short' but the argument has type}}
+  // expected-warning@-3 {{format specifies type 'char' but the argument has type}}
+#endif
+  p("%u", b);
+  p("%hu", b);
+  p("%hhu", b);
+#ifdef PEDANTIC
+  // expected-warning@-3 {{format specifies type 'unsigned short' but the argument has type}}
+  // expected-warning@-3 {{format specifies type 'unsigned char' but the argument has type}}
+#endif
+  p("%c", b);
+  p("%f", b); // expected-warning{{format specifies type 'double' but the argument has type}}
+  p("%ld", b); // expected-warning{{format specifies type 'long' but the argument has type}}
+}
Index: clang/lib/AST/FormatString.cpp
===
--- clang/lib/AST/FormatString.cpp
+++ clang/lib/AST/FormatString.cpp
@@ -360,7 +360,9 @@
   case BuiltinType::UChar:
   case BuiltinType::Char_U:
 return Match;
-}
+  case BuiltinType::Bool:
+return NoMatchPedantic;
+  }
   return NoMatch;
 }
 
@@ -390,6 +392,11 @@
   return NoMatchPedantic;
 return T == C.UnsignedCharTy || T == C.SignedCharTy ? Match
 : NoMatch;
+  case BuiltinType::Bool:
+

Re: r370083 - Change the X86 datalayout to add three address spaces for 32 bit signed,

[Vitaly Buka via cfe-commits]

llvm-project/lld/test/ELF/lto/Inputs/drop-debug-info.bc needs to be updated
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/34675/steps/check-lld%20msan/logs/stdio

/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld -shared
/usr/local/google/home/vitalybuka/slow/bbot/llvm-project/lld/test/ELF/lto/Inputs/drop-debug-info.bc
 -disable-verify -o
/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/obj/lld/test/ELF/lto/Output/drop-debug-info.ll.tmp
ld.lld: warning: ignoring debug info with an invalid version (1) in
/usr/local/google/home/vitalybuka/slow/bbot/llvm-project/lld/test/ELF/lto/Inputs/drop-debug-info.bc
ld.lld: ../llvm-project/llvm/lib/CodeGen/MachineFunction.cpp:200: void
llvm::MachineFunction::init(): Assertion
`Target.isCompatibleDataLayout(getDataLayout()) && "Can't create a
MachineFunction using a Module with a " "Target-incompatible DataLayout
attached\n"' failed.
Stack dump:
0. Program arguments:
/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld -shared
/usr/local/google/home/vitalybuka/slow/bbot/llvm-project/lld/test/ELF/lto/Inputs/drop-debug-info.bc
-disable-verify -o
/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/obj/lld/test/ELF/lto/Output/drop-debug-info.ll.tmp

1. Running pass 'Function Pass Manager' on module 'ld-temp.o'.
2. Running pass 'X86 DAG->DAG Instruction Selection' on function '@f'
 #0 0x014e31cd PrintStackTraceSignalHandler(void*)
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x14e31cd)
 #1 0x014e0fce llvm::sys::RunSignalHandlers()
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x14e0fce)
 #2 0x014e3388 SignalHandler(int)
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x14e3388)
 #3 0x7f83527833a0 __restore_rt
(/lib/x86_64-linux-gnu/libpthread.so.0+0x123a0)
 #4 0x7f8351c86cfb raise (/lib/x86_64-linux-gnu/libc.so.6+0x36cfb)
 #5 0x7f8351c718ad abort (/lib/x86_64-linux-gnu/libc.so.6+0x218ad)
 #6 0x7f8351c7177f (/lib/x86_64-linux-gnu/libc.so.6+0x2177f)
 #7 0x7f8351c7f542 (/lib/x86_64-linux-gnu/libc.so.6+0x2f542)
 #8 0x0200c1da llvm::MachineFunction::init()
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x200c1da)
 #9 0x02009082
llvm::MachineModuleInfo::getOrCreateMachineFunction(llvm::Function const&)
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x2009082)
#10 0x02006d36
llvm::MachineFunctionPass::runOnFunction(llvm::Function&)
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x2006d36)
#11 0x01751746 llvm::FPPassManager::runOnFunction(llvm::Function&)
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x1751746)
#12 0x01751a53 llvm::FPPassManager::runOnModule(llvm::Module&)
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x1751a53)
#13 0x0175209c llvm::legacy::PassManagerImpl::run(llvm::Module&)
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x175209c)
#14 0x01b4fe00 (anonymous namespace)::codegen(llvm::lto::Config&,
llvm::TargetMachine*,
std::function > (unsigned int)>,
unsigned int, llvm::Module&)
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x1b4fe00)
#15 0x01b4ce8e llvm::lto::backend(llvm::lto::Config&,
std::function > (unsigned int)>,
unsigned int, std::unique_ptr >, llvm::ModuleSummaryIndex&)
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x1b4ce8e)
#16 0x01b43066
llvm::lto::LTO::runRegularLTO(std::function > (unsigned int)>)
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x1b43066)
#17 0x01b42a21
llvm::lto::LTO::run(std::function > (unsigned int)>,
std::function > (unsigned int)>
(unsigned int, llvm::StringRef)>)
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x1b42a21)
#18 0x012d13e3 lld::elf::BitcodeCompiler::compile()
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x12d13e3)
#19 0x01260ed4 void
lld::elf::LinkerDriver::compileBitcodeFiles >()
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x1260ed4)
#20 0x01255974 void
lld::elf::LinkerDriver::link >(llvm::opt::InputArgList&)
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x1255974)
#21 0x0124570c lld::elf::LinkerDriver::main(llvm::ArrayRef)
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x124570c)
#22 0x01244019 lld::elf::link(llvm::ArrayRef, bool,
llvm::raw_ostream&)
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x1244019)
#23 0x011b781d main
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x11b781d)
#24 0x7f8351c7352b __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x2352b)
#25 0x011b702a _start
(/usr/local/google/home/vitalybuka/slow/bbot/llvm_build/bin/ld.lld+0x11b702a)
Aborted

On Tue, Aug 27, 2019 at 

[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Mmm, i don't know what was that commit meant to fix. Your evaluation looks 
fairly sufficient for turning it on by default. Let's make it an on-by-default 
option and flip the flag back into off-by-default if it turns out to be somehow 
broken.




Comment at: clang/docs/analyzer/checkers.rst:299
+
+This checker has several options which can be set from command line (e.g.
+``-analyzer-config deadcode.DeadStores:WarnForDeadNestedAssignments=true``):

several?



Comment at: clang/test/Analysis/dead-stores.c:81
   extern int *baz();
+#ifdef WARN_FOR_DEAD_NESTED
+  if ((p = baz())) // expected-warning{{Although the value stored}}

Pls consider `-verify=...` for new tests (cf. D60732).


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

https://reviews.llvm.org/D66733



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

r370142 - Revert "Change the X86 datalayout to add three address spaces for 32 bit signed, "

[Vlad Tsyrklevich via cfe-commits]

Author: vlad.tsyrklevich
Date: Tue Aug 27 18:08:54 2019
New Revision: 370142

URL: http://llvm.org/viewvc/llvm-project?rev=370142=rev
Log:
Revert "Change the X86 datalayout to add three address spaces for 32 bit 
signed,"

This reverts commit r370083 because it caused check-lld failures on
sanitizer-x86_64-linux-fast.

Modified:
cfe/trunk/lib/Basic/Targets/OSTargets.h
cfe/trunk/lib/Basic/Targets/X86.h
cfe/trunk/test/CodeGen/Inputs/thinlto-multi-module.ll
cfe/trunk/test/CodeGen/Inputs/thinlto_backend.ll
cfe/trunk/test/CodeGen/Inputs/thinlto_backend_local_name_conflict1.ll
cfe/trunk/test/CodeGen/Inputs/thinlto_backend_local_name_conflict2.ll
cfe/trunk/test/CodeGen/iamcu-abi.c
cfe/trunk/test/CodeGen/target-data.c
cfe/trunk/test/CodeGen/thinlto-diagnostic-handler-remarks-with-hotness.ll
cfe/trunk/test/CodeGen/thinlto-distributed-backend-skip.ll
cfe/trunk/test/CodeGen/thinlto-distributed-cfi-devirt.ll
cfe/trunk/test/CodeGen/thinlto-distributed-cfi.ll
cfe/trunk/test/CodeGen/thinlto-distributed.ll
cfe/trunk/test/CodeGen/thinlto-multi-module.ll
cfe/trunk/test/CodeGen/thinlto_backend.ll
cfe/trunk/test/CodeGen/thinlto_backend_local_name_conflict.ll

Modified: cfe/trunk/lib/Basic/Targets/OSTargets.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/OSTargets.h?rev=370142=370141=370142=diff
==
--- cfe/trunk/lib/Basic/Targets/OSTargets.h (original)
+++ cfe/trunk/lib/Basic/Targets/OSTargets.h Tue Aug 27 18:08:54 2019
@@ -775,11 +775,9 @@ public:
 if (Triple.getArch() == llvm::Triple::arm) {
   // Handled in ARM's setABI().
 } else if (Triple.getArch() == llvm::Triple::x86) {
-  this->resetDataLayout("e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-"
-"i64:64-n8:16:32-S128");
+  this->resetDataLayout("e-m:e-p:32:32-i64:64-n8:16:32-S128");
 } else if (Triple.getArch() == llvm::Triple::x86_64) {
-  this->resetDataLayout("e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-"
-"i64:64-n8:16:32:64-S128");
+  this->resetDataLayout("e-m:e-p:32:32-i64:64-n8:16:32:64-S128");
 } else if (Triple.getArch() == llvm::Triple::mipsel) {
   // Handled on mips' setDataLayout.
 } else {

Modified: cfe/trunk/lib/Basic/Targets/X86.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.h?rev=370142=370141=370142=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.h (original)
+++ cfe/trunk/lib/Basic/Targets/X86.h Tue Aug 27 18:08:54 2019
@@ -340,8 +340,7 @@ public:
 LongDoubleWidth = 96;
 LongDoubleAlign = 32;
 SuitableAlign = 128;
-resetDataLayout("e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-f64:32:64-"
-"f80:32-n8:16:32-S128");
+resetDataLayout("e-m:e-p:32:32-f64:32:64-f80:32-n8:16:32-S128");
 SizeType = UnsignedInt;
 PtrDiffType = SignedInt;
 IntPtrType = SignedInt;
@@ -441,8 +440,7 @@ public:
   UseSignedCharForObjCBool = false;
 SizeType = UnsignedLong;
 IntPtrType = SignedLong;
-resetDataLayout("e-m:o-p:32:32-p270:32:32-p271:32:32-p272:64:64-f64:32:64-"
-"f80:128-n8:16:32-S128");
+resetDataLayout("e-m:o-p:32:32-f64:32:64-f80:128-n8:16:32-S128");
 HasAlignMac68kSupport = true;
   }
 
@@ -467,10 +465,9 @@ public:
 DoubleAlign = LongLongAlign = 64;
 bool IsWinCOFF =
 getTriple().isOSWindows() && getTriple().isOSBinFormatCOFF();
-resetDataLayout(IsWinCOFF ? "e-m:x-p:32:32-p270:32:32-p271:32:32-p272:64:"
-"64-i64:64-f80:32-n8:16:32-a:0:32-S32"
-  : "e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:"
-"64-i64:64-f80:32-n8:16:32-a:0:32-S32");
+resetDataLayout(IsWinCOFF
+? "e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32"
+: "e-m:e-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32");
   }
 };
 
@@ -518,8 +515,7 @@ public:
   : X86_32TargetInfo(Triple, Opts) {
 this->WCharType = TargetInfo::UnsignedShort;
 DoubleAlign = LongLongAlign = 64;
-
resetDataLayout("e-m:x-p:32:32-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:"
-"32-n8:16:32-a:0:32-S32");
+resetDataLayout("e-m:x-p:32:32-i64:64-f80:32-n8:16:32-a:0:32-S32");
   }
 
   void getTargetDefines(const LangOptions ,
@@ -556,8 +552,7 @@ public:
   : X86_32TargetInfo(Triple, Opts) {
 LongDoubleWidth = 64;
 LongDoubleFormat = ::APFloat::IEEEdouble();
-
resetDataLayout("e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-i64:32-f64:"
-"32-f128:32-n8:16:32-a:0:32-S32");
+resetDataLayout("e-m:e-p:32:32-i64:32-f64:32-f128:32-n8:16:32-a:0:32-S32");
 WIntType = UnsignedInt;
   }
 
@@ -616,12 +611,10 @@ public:
 RegParmMax = 6;
 
 // 

r370141 - Fix "commas at the end of enumerator lists are a C99-specific"

[Vitaly Buka via cfe-commits]

Author: vitalybuka
Date: Tue Aug 27 18:04:50 2019
New Revision: 370141

URL: http://llvm.org/viewvc/llvm-project?rev=370141=rev
Log:
Fix "commas at the end of enumerator lists are a C99-specific"

Modified:
cfe/trunk/include/clang-c/Index.h

Modified: cfe/trunk/include/clang-c/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=370141=370140=370141=diff
==
--- cfe/trunk/include/clang-c/Index.h (original)
+++ cfe/trunk/include/clang-c/Index.h Tue Aug 27 18:04:50 2019
@@ -1361,7 +1361,7 @@ enum CXTranslationUnit_Flags {
   /**
* Tells the preprocessor not to skip excluded conditional blocks.
*/
-  CXTranslationUnit_RetainExcludedConditionalBlocks = 0x8000,
+  CXTranslationUnit_RetainExcludedConditionalBlocks = 0x8000
 };
 
 /**


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

[PATCH] D66847: [analyzer] Fix analyzer warnings.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I mean, such code should indeed be refactored to query the custom RTTI only 
once, but given that it's locally obvious that there's no null dereference in 
this code, we should not be emitting a warning on it.


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

https://reviews.llvm.org/D66847



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

[PATCH] D66847: [analyzer] Fix analyzer warnings.

[Artem Dergachev via Phabricator via cfe-commits]

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

In D66847#1648209 , @Szelethus wrote:

> > if the dyn_ part is really necessary here, then you crash with a null 
> > dereference a few lines below because you didn't check the result
>
> Hmm, can we detect things like this?:
>
>   if (isa(D)) {
> const auto *FD = dyn_cast(D); // warn: D is known to be a FunctionDecl, 
> prefer using cast<>
> // ...
>   }
>


We should be detecting things like this as of D66325 
 and D66423  
(we remember which casts were successful and don't re-assume them again), but 
`LocalizationChecker.cpp` turned out to have a false positive of this kind. 
@Charusso, you may want to take a look.




Comment at: clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp:885-888
-  const Decl *D = Call.getDecl();
-  if (D && isa(D)) {
-const FunctionDecl *FD = dyn_cast(D);
-auto formals = FD->parameters();

Note: this was a false positive.


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

https://reviews.llvm.org/D66847



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

[PATCH] D66850: Avoid crash when dumping NULL Type as JSON

[Bert Belder via Phabricator via cfe-commits]

piscisaureus updated this revision to Diff 217545.

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

https://reviews.llvm.org/D66850

Files:
  clang/lib/AST/JSONNodeDumper.cpp


Index: clang/lib/AST/JSONNodeDumper.cpp
===
--- clang/lib/AST/JSONNodeDumper.cpp
+++ clang/lib/AST/JSONNodeDumper.cpp
@@ -66,6 +66,10 @@
 
 void JSONNodeDumper::Visit(const Type *T) {
   JOS.attribute("id", createPointerRepresentation(T));
+
+  if (!T)
+return;
+
   JOS.attribute("kind", (llvm::Twine(T->getTypeClassName()) + "Type").str());
   JOS.attribute("type", createQualType(QualType(T, 0), /*Desugar*/ false));
   attributeOnlyIfTrue("isDependent", T->isDependentType());


Index: clang/lib/AST/JSONNodeDumper.cpp
===
--- clang/lib/AST/JSONNodeDumper.cpp
+++ clang/lib/AST/JSONNodeDumper.cpp
@@ -66,6 +66,10 @@
 
 void JSONNodeDumper::Visit(const Type *T) {
   JOS.attribute("id", createPointerRepresentation(T));
+
+  if (!T)
+return;
+
   JOS.attribute("kind", (llvm::Twine(T->getTypeClassName()) + "Type").str());
   JOS.attribute("type", createQualType(QualType(T, 0), /*Desugar*/ false));
   attributeOnlyIfTrue("isDependent", T->isDependentType());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66847: [analyzer] Fix analyzer warnings.

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

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

> if the dyn_ part is really necessary here, then you crash with a null 
> dereference a few lines below because you didn't check the result

Hmm, can we detect things like this?:

  if (isa(D)) {
const auto *FD = dyn_cast(D); // warn: D is known to be a FunctionDecl, 
prefer using cast<>
// ...
  }


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

https://reviews.llvm.org/D66847



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

[PATCH] D66850: Do not attempt visit child nodes of NULL type

[Bert Belder via Phabricator via cfe-commits]

piscisaureus created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66850

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


Index: clang/include/clang/AST/ASTNodeTraverser.h
===
--- clang/include/clang/AST/ASTNodeTraverser.h
+++ clang/include/clang/AST/ASTNodeTraverser.h
@@ -130,9 +130,9 @@
 
   void Visit(const Type *T) {
 getNodeDelegate().AddChild([=] {
-  getNodeDelegate().Visit(T);
   if (!T)
 return;
+  getNodeDelegate().Visit(T);
   TypeVisitor::Visit(T);
 
   QualType SingleStepDesugar =


Index: clang/include/clang/AST/ASTNodeTraverser.h
===
--- clang/include/clang/AST/ASTNodeTraverser.h
+++ clang/include/clang/AST/ASTNodeTraverser.h
@@ -130,9 +130,9 @@
 
   void Visit(const Type *T) {
 getNodeDelegate().AddChild([=] {
-  getNodeDelegate().Visit(T);
   if (!T)
 return;
+  getNodeDelegate().Visit(T);
   TypeVisitor::Visit(T);
 
   QualType SingleStepDesugar =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370130: [ObjC] Fix type checking for qualified id block 
parameters. (authored by vsapsai, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66831?vs=217500=217538#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66831

Files:
  cfe/trunk/lib/AST/ASTContext.cpp
  cfe/trunk/test/SemaObjC/block-type-safety.m


Index: cfe/trunk/lib/AST/ASTContext.cpp
===
--- cfe/trunk/lib/AST/ASTContext.cpp
+++ cfe/trunk/lib/AST/ASTContext.cpp
@@ -8191,9 +8191,9 @@
   }
 
   if (LHSOPT->isObjCQualifiedIdType() || RHSOPT->isObjCQualifiedIdType())
-return finish(ObjCQualifiedIdTypesAreCompatible(QualType(LHSOPT,0),
-QualType(RHSOPT,0),
-false));
+return finish(ObjCQualifiedIdTypesAreCompatible(
+QualType(BlockReturnType ? LHSOPT : RHSOPT, 0),
+QualType(BlockReturnType ? RHSOPT : LHSOPT, 0), false));
 
   const ObjCInterfaceType* LHS = LHSOPT->getInterfaceType();
   const ObjCInterfaceType* RHS = RHSOPT->getInterfaceType();
Index: cfe/trunk/test/SemaObjC/block-type-safety.m
===
--- cfe/trunk/test/SemaObjC/block-type-safety.m
+++ cfe/trunk/test/SemaObjC/block-type-safety.m
@@ -133,9 +133,20 @@
 @end
 
 int test5() {
+// Returned value is used outside of a block, so error on changing
+// a return type to a more general than expected.
 NSAllArray *(^block)(id);
 id  (^genericBlock)(id);
 genericBlock = block;
+block = genericBlock; // expected-error {{incompatible block pointer types 
assigning to 'NSAllArray *(^)(id)' from 'id (^)(id)'}}
+
+// A parameter is used inside a block, so error on changing a parameter 
type
+// to a more specific than an argument type it will be called with.
+// rdar://problem/52788423
+void (^blockWithParam)(NSAllArray *);
+void (^genericBlockWithParam)(id);
+genericBlockWithParam = blockWithParam; // expected-error {{incompatible 
block pointer types assigning to 'void (^)(id)' from 'void (^)(NSAllArray 
*)'}}
+blockWithParam = genericBlockWithParam;
 return 0;
 }
 


Index: cfe/trunk/lib/AST/ASTContext.cpp
===
--- cfe/trunk/lib/AST/ASTContext.cpp
+++ cfe/trunk/lib/AST/ASTContext.cpp
@@ -8191,9 +8191,9 @@
   }
 
   if (LHSOPT->isObjCQualifiedIdType() || RHSOPT->isObjCQualifiedIdType())
-return finish(ObjCQualifiedIdTypesAreCompatible(QualType(LHSOPT,0),
-QualType(RHSOPT,0),
-false));
+return finish(ObjCQualifiedIdTypesAreCompatible(
+QualType(BlockReturnType ? LHSOPT : RHSOPT, 0),
+QualType(BlockReturnType ? RHSOPT : LHSOPT, 0), false));
 
   const ObjCInterfaceType* LHS = LHSOPT->getInterfaceType();
   const ObjCInterfaceType* RHS = RHSOPT->getInterfaceType();
Index: cfe/trunk/test/SemaObjC/block-type-safety.m
===
--- cfe/trunk/test/SemaObjC/block-type-safety.m
+++ cfe/trunk/test/SemaObjC/block-type-safety.m
@@ -133,9 +133,20 @@
 @end
 
 int test5() {
+// Returned value is used outside of a block, so error on changing
+// a return type to a more general than expected.
 NSAllArray *(^block)(id);
 id  (^genericBlock)(id);
 genericBlock = block;
+block = genericBlock; // expected-error {{incompatible block pointer types assigning to 'NSAllArray *(^)(id)' from 'id (^)(id)'}}
+
+// A parameter is used inside a block, so error on changing a parameter type
+// to a more specific than an argument type it will be called with.
+// rdar://problem/52788423
+void (^blockWithParam)(NSAllArray *);
+void (^genericBlockWithParam)(id);
+genericBlockWithParam = blockWithParam; // expected-error {{incompatible block pointer types assigning to 'void (^)(id)' from 'void (^)(NSAllArray *)'}}
+blockWithParam = genericBlockWithParam;
 return 0;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Thanks for the fast review.


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

https://reviews.llvm.org/D66831



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

r370130 - [ObjC] Fix type checking for qualified id block parameters.

[Volodymyr Sapsai via cfe-commits]

Author: vsapsai
Date: Tue Aug 27 17:25:06 2019
New Revision: 370130

URL: http://llvm.org/viewvc/llvm-project?rev=370130=rev
Log:
[ObjC] Fix type checking for qualified id block parameters.

When checking if block types are compatible, we are checking for
compatibility their return types and parameters' types. As these types
have different variance, we need to check them in different order.

rdar://problem/52788423

Reviewers: erik.pilkington, arphaman

Reviewed By: arphaman

Subscribers: jkorous, dexonsmith, ributzka, cfe-commits

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


Modified:
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/test/SemaObjC/block-type-safety.m

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=370130=370129=370130=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Tue Aug 27 17:25:06 2019
@@ -8191,9 +8191,9 @@ bool ASTContext::canAssignObjCInterfaces
   }
 
   if (LHSOPT->isObjCQualifiedIdType() || RHSOPT->isObjCQualifiedIdType())
-return finish(ObjCQualifiedIdTypesAreCompatible(QualType(LHSOPT,0),
-QualType(RHSOPT,0),
-false));
+return finish(ObjCQualifiedIdTypesAreCompatible(
+QualType(BlockReturnType ? LHSOPT : RHSOPT, 0),
+QualType(BlockReturnType ? RHSOPT : LHSOPT, 0), false));
 
   const ObjCInterfaceType* LHS = LHSOPT->getInterfaceType();
   const ObjCInterfaceType* RHS = RHSOPT->getInterfaceType();

Modified: cfe/trunk/test/SemaObjC/block-type-safety.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/block-type-safety.m?rev=370130=370129=370130=diff
==
--- cfe/trunk/test/SemaObjC/block-type-safety.m (original)
+++ cfe/trunk/test/SemaObjC/block-type-safety.m Tue Aug 27 17:25:06 2019
@@ -133,9 +133,20 @@ int test4 () {
 @end
 
 int test5() {
+// Returned value is used outside of a block, so error on changing
+// a return type to a more general than expected.
 NSAllArray *(^block)(id);
 id  (^genericBlock)(id);
 genericBlock = block;
+block = genericBlock; // expected-error {{incompatible block pointer types 
assigning to 'NSAllArray *(^)(id)' from 'id (^)(id)'}}
+
+// A parameter is used inside a block, so error on changing a parameter 
type
+// to a more specific than an argument type it will be called with.
+// rdar://problem/52788423
+void (^blockWithParam)(NSAllArray *);
+void (^genericBlockWithParam)(id);
+genericBlockWithParam = blockWithParam; // expected-error {{incompatible 
block pointer types assigning to 'void (^)(id)' from 'void (^)(NSAllArray 
*)'}}
+blockWithParam = genericBlockWithParam;
 return 0;
 }
 


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

[PATCH] D66847: [analyzer] Fix analyzer warnings.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 217537.
NoQ edited the summary of this revision.
NoQ added a comment.

Use `auto` in one more place.


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

https://reviews.llvm.org/D66847

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
  clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/cstring-syntax-weird.c
  clang/test/Analysis/cstring-syntax-weird2.c
  clang/test/Analysis/cstring-syntax.c

Index: clang/test/Analysis/cstring-syntax.c
===
--- clang/test/Analysis/cstring-syntax.c
+++ clang/test/Analysis/cstring-syntax.c
@@ -1,7 +1,18 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -analyzer-store=region -Wno-strncat-size -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument -Wno-sizeof-pointer-memaccess -verify %s
-// RUN: %clang_analyze_cc1 -triple armv7-a15-linux -analyzer-checker=unix.cstring.BadSizeArg -analyzer-store=region -Wno-strncat-size -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument -Wno-sizeof-pointer-memaccess -verify %s
-// RUN: %clang_analyze_cc1 -triple aarch64_be-none-linux-gnu -analyzer-checker=unix.cstring.BadSizeArg -analyzer-store=region -Wno-strncat-size -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument -Wno-sizeof-pointer-memaccess -verify %s
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=unix.cstring.BadSizeArg -analyzer-store=region -Wno-strncat-size -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument -Wno-sizeof-pointer-memaccess -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -verify %s\
+// RUN:-Wno-strncat-size -Wno-sizeof-pointer-memaccess \
+// RUN:-Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument
+// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -verify %s\
+// RUN:-Wno-strncat-size -Wno-sizeof-pointer-memaccess \
+// RUN:-Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument\
+// RUN:-triple armv7-a15-linux
+// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -verify %s\
+// RUN:-Wno-strncat-size -Wno-sizeof-pointer-memaccess \
+// RUN:-Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument\
+// RUN:-triple aarch64_be-none-linux-gnu
+// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -verify %s\
+// RUN:-Wno-strncat-size -Wno-sizeof-pointer-memaccess \
+// RUN:-Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument\
+// RUN:-triple i386-apple-darwin10
 
 typedef __SIZE_TYPE__ size_t;
 char  *strncat(char *, const char *, size_t);
Index: clang/test/Analysis/cstring-syntax-weird2.c
===
--- /dev/null
+++ clang/test/Analysis/cstring-syntax-weird2.c
@@ -0,0 +1,17 @@
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=unix.cstring.BadSizeArg \
+// RUN:-verify %s
+
+// expected-no-diagnostics
+
+typedef __SIZE_TYPE__ size_t;
+// The last parameter is normally size_t but the test is about the abnormal
+// situation when it's not a size_t.
+size_t strlcpy(char *, const char *, void (*)());
+
+void foo();
+
+void testWeirdDecls(const char *src) {
+  char dst[10];
+  strlcpy(dst, src, foo); // no-crash
+  strlcpy(dst, src, ); // no-crash
+}
Index: clang/test/Analysis/cstring-syntax-weird.c
===
--- /dev/null
+++ clang/test/Analysis/cstring-syntax-weird.c
@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=unix.cstring.BadSizeArg \
+// RUN:-verify %s
+
+// expected-no-diagnostics
+
+typedef __SIZE_TYPE__ size_t;
+// The last parameter is normally size_t but the test is about the abnormal
+// situation when it's not a size_t.
+size_t strlcpy(char *, const 

[PATCH] D66847: [analyzer] Fix analyzer warnings.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, 
baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, szepet.
Herald added a project: clang.

They ain't gonna fix themselves... well, not yet.

This is an outcome for this summer's GSoC! - 
http://lists.llvm.org/pipermail/cfe-dev/2019-August/063195.html

Found one real crash, added a couple tests:

F9870542: Screen Shot 2019-08-27 at 4.55.04 PM.png 


Addressed 17 style issues that aren't real crashes - mostly misuses of 
`dyn_cast` / `getAs` (i.e., "if the `dyn_` part is really necessary here, then 
you crash with a null dereference a few lines below because you didn't check 
the result"). These changes indeed made the code cleaner and shorter.

Suppressed 4 false positives; i found these assertions/refactorings nice anyway.

That leaves us with only one remaining false positive that isn't really 
actionable; i'll have to investigate.


Repository:
  rC Clang

https://reviews.llvm.org/D66847

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  clang/lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
  clang/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
  clang/lib/StaticAnalyzer/Checkers/IteratorChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocSizeofChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/PointerArithChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/UndefinedAssignmentChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
  clang/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/cstring-syntax-weird.c
  clang/test/Analysis/cstring-syntax-weird2.c
  clang/test/Analysis/cstring-syntax.c

Index: clang/test/Analysis/cstring-syntax.c
===
--- clang/test/Analysis/cstring-syntax.c
+++ clang/test/Analysis/cstring-syntax.c
@@ -1,7 +1,18 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -analyzer-store=region -Wno-strncat-size -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument -Wno-sizeof-pointer-memaccess -verify %s
-// RUN: %clang_analyze_cc1 -triple armv7-a15-linux -analyzer-checker=unix.cstring.BadSizeArg -analyzer-store=region -Wno-strncat-size -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument -Wno-sizeof-pointer-memaccess -verify %s
-// RUN: %clang_analyze_cc1 -triple aarch64_be-none-linux-gnu -analyzer-checker=unix.cstring.BadSizeArg -analyzer-store=region -Wno-strncat-size -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument -Wno-sizeof-pointer-memaccess -verify %s
-// RUN: %clang_analyze_cc1 -triple i386-apple-darwin10 -analyzer-checker=unix.cstring.BadSizeArg -analyzer-store=region -Wno-strncat-size -Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument -Wno-sizeof-pointer-memaccess -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -verify %s\
+// RUN:-Wno-strncat-size -Wno-sizeof-pointer-memaccess \
+// RUN:-Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument
+// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -verify %s\
+// RUN:-Wno-strncat-size -Wno-sizeof-pointer-memaccess \
+// RUN:-Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument\
+// RUN:-triple armv7-a15-linux
+// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -verify %s\
+// RUN:-Wno-strncat-size -Wno-sizeof-pointer-memaccess \
+// RUN:-Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument\
+// RUN:-triple aarch64_be-none-linux-gnu
+// RUN: %clang_analyze_cc1 -analyzer-checker=unix.cstring.BadSizeArg -verify %s\
+// RUN:-Wno-strncat-size -Wno-sizeof-pointer-memaccess \
+// RUN:-Wno-strlcpy-strlcat-size -Wno-sizeof-array-argument\
+// RUN:-triple i386-apple-darwin10
 
 typedef __SIZE_TYPE__ size_t;
 char  *strncat(char *, const char *, size_t);
Index: clang/test/Analysis/cstring-syntax-weird2.c
===
--- /dev/null
+++ clang/test/Analysis/cstring-syntax-weird2.c
@@ -0,0 +1,17 @@
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=unix.cstring.BadSizeArg \
+// RUN:-verify %s
+
+// 

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

[Alexandre Ganea via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL370129: Re-land [clang-scan-deps] Minimizer: Correctly 
handle multi-line content with… (authored by aganea, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D66556?vs=217527=217535#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66556

Files:
  cfe/trunk/.gitattributes
  cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
  cfe/trunk/test/Lexer/minimize_source_to_dependency_directives_invalid_error.c

Index: cfe/trunk/test/Lexer/minimize_source_to_dependency_directives_invalid_error.c
===
--- cfe/trunk/test/Lexer/minimize_source_to_dependency_directives_invalid_error.c
+++ cfe/trunk/test/Lexer/minimize_source_to_dependency_directives_invalid_error.c
@@ -0,0 +1,16 @@
+// Test CF+LF are properly handled along with quoted, multi-line #error
+// RUN: %clang_cc1 -DOTHER -print-dependency-directives-minimized-source %s 2>&1 | FileCheck %s
+
+#ifndef TEST
+#error "message \
+   more message \
+   even more"
+#endif
+
+#ifdef OTHER
+#include 
+#endif
+
+// CHECK:  #ifdef OTHER
+// CHECK-NEXT: #include 
+// CHECK-NEXT: #endif
Index: cfe/trunk/.gitattributes
===
--- cfe/trunk/.gitattributes
+++ cfe/trunk/.gitattributes
@@ -0,0 +1,4 @@
+# Windows line ending tests
+test/Lexer/minimize_source_to_dependency_directives_invalid_error.c text eol=crlf
+test/FixIt/fixit-newline-style.c text eol=crlf
+test/Frontend/system-header-line-directive-ms-lineendings.c text eol=crlf
Index: cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
===
--- cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
+++ cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
@@ -196,15 +196,29 @@
 ++First; // Finish off the string.
 }
 
-static void skipNewline(const char *, const char *End) {
-  assert(isVerticalWhitespace(*First));
-  ++First;
+// Returns the length of EOL, either 0 (no end-of-line), 1 (\n) or 2 (\r\n)
+static unsigned isEOL(const char *First, const char *const End) {
   if (First == End)
-return;
+return 0;
+  if (End - First > 1 && isVerticalWhitespace(First[0]) &&
+  isVerticalWhitespace(First[1]) && First[0] != First[1])
+return 2;
+  return !!isVerticalWhitespace(First[0]);
+}
 
-  // Check for "\n\r" and "\r\n".
-  if (LLVM_UNLIKELY(isVerticalWhitespace(*First) && First[-1] != First[0]))
-++First;
+// Returns the length of the skipped newline
+static unsigned skipNewline(const char *, const char *End) {
+  if (First == End)
+return 0;
+  assert(isVerticalWhitespace(*First));
+  unsigned Len = isEOL(First, End);
+  assert(Len && "expected newline");
+  First += Len;
+  return Len;
+}
+
+static bool wasLineContinuation(const char *First, unsigned EOLLen) {
+  return *(First - (int)EOLLen - 1) == '\\';
 }
 
 static void skipToNewlineRaw(const char *, const char *const End) {
@@ -212,17 +226,21 @@
 if (First == End)
   return;
 
-if (isVerticalWhitespace(*First))
+unsigned Len = isEOL(First, End);
+if (Len)
   return;
 
-while (!isVerticalWhitespace(*First))
+do {
   if (++First == End)
 return;
+  Len = isEOL(First, End);
+} while (!Len);
 
 if (First[-1] != '\\')
   return;
 
-++First; // Keep going...
+First += Len;
+// Keep skipping lines...
   }
 }
 
@@ -277,7 +295,7 @@
 }
 
 static void skipLine(const char *, const char *const End) {
-  do {
+  for (;;) {
 assert(First <= End);
 if (First == End)
   return;
@@ -322,9 +340,10 @@
   return;
 
 // Skip over the newline.
-assert(isVerticalWhitespace(*First));
-skipNewline(First, End);
-  } while (First[-2] == '\\'); // Continue past line-continuations.
+unsigned Len = skipNewline(First, End);
+if (!wasLineContinuation(First, Len)) // Continue past line-continuations.
+  break;
+  }
 }
 
 static void skipDirective(StringRef Name, const char *,
@@ -380,6 +399,8 @@
 // Print out the string.
 if (Last == End || Last == First || Last[-1] != '\\') {
   append(First, reverseOverSpaces(First, Last));
+  First = Last;
+  skipNewline(First, End);
   return;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r370129 - Re-land [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

[Alexandre Ganea via cfe-commits]

Author: aganea
Date: Tue Aug 27 17:04:06 2019
New Revision: 370129

URL: http://llvm.org/viewvc/llvm-project?rev=370129=rev
Log:
Re-land [clang-scan-deps] Minimizer: Correctly handle multi-line content with 
CR+LF line endings

Previously, an #error directive with quoted, multi-line content, along with 
CR+LF line endings wasn't handled correctly.

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

Added:
cfe/trunk/.gitattributes

cfe/trunk/test/Lexer/minimize_source_to_dependency_directives_invalid_error.c   
(contents, props changed)
  - copied, changed from r36, 
cfe/trunk/test/Lexer/minimize_source_to_dependency_directives_invalid_error.c
Modified:
cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp

Added: cfe/trunk/.gitattributes
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/.gitattributes?rev=370129=auto
==
--- cfe/trunk/.gitattributes (added)
+++ cfe/trunk/.gitattributes Tue Aug 27 17:04:06 2019
@@ -0,0 +1,4 @@
+# Windows line ending tests
+test/Lexer/minimize_source_to_dependency_directives_invalid_error.c text 
eol=crlf
+test/FixIt/fixit-newline-style.c text eol=crlf
+test/Frontend/system-header-line-directive-ms-lineendings.c text eol=crlf

Modified: cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp?rev=370129=370128=370129=diff
==
--- cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp (original)
+++ cfe/trunk/lib/Lex/DependencyDirectivesSourceMinimizer.cpp Tue Aug 27 
17:04:06 2019
@@ -196,15 +196,29 @@ static void skipString(const char *
 ++First; // Finish off the string.
 }
 
-static void skipNewline(const char *, const char *End) {
-  assert(isVerticalWhitespace(*First));
-  ++First;
+// Returns the length of EOL, either 0 (no end-of-line), 1 (\n) or 2 (\r\n)
+static unsigned isEOL(const char *First, const char *const End) {
   if (First == End)
-return;
+return 0;
+  if (End - First > 1 && isVerticalWhitespace(First[0]) &&
+  isVerticalWhitespace(First[1]) && First[0] != First[1])
+return 2;
+  return !!isVerticalWhitespace(First[0]);
+}
+
+// Returns the length of the skipped newline
+static unsigned skipNewline(const char *, const char *End) {
+  if (First == End)
+return 0;
+  assert(isVerticalWhitespace(*First));
+  unsigned Len = isEOL(First, End);
+  assert(Len && "expected newline");
+  First += Len;
+  return Len;
+}
 
-  // Check for "\n\r" and "\r\n".
-  if (LLVM_UNLIKELY(isVerticalWhitespace(*First) && First[-1] != First[0]))
-++First;
+static bool wasLineContinuation(const char *First, unsigned EOLLen) {
+  return *(First - (int)EOLLen - 1) == '\\';
 }
 
 static void skipToNewlineRaw(const char *, const char *const End) {
@@ -212,17 +226,21 @@ static void skipToNewlineRaw(const char
 if (First == End)
   return;
 
-if (isVerticalWhitespace(*First))
+unsigned Len = isEOL(First, End);
+if (Len)
   return;
 
-while (!isVerticalWhitespace(*First))
+do {
   if (++First == End)
 return;
+  Len = isEOL(First, End);
+} while (!Len);
 
 if (First[-1] != '\\')
   return;
 
-++First; // Keep going...
+First += Len;
+// Keep skipping lines...
   }
 }
 
@@ -277,7 +295,7 @@ static bool isQuoteCppDigitSeparator(con
 }
 
 static void skipLine(const char *, const char *const End) {
-  do {
+  for (;;) {
 assert(First <= End);
 if (First == End)
   return;
@@ -322,9 +340,10 @@ static void skipLine(const char *,
   return;
 
 // Skip over the newline.
-assert(isVerticalWhitespace(*First));
-skipNewline(First, End);
-  } while (First[-2] == '\\'); // Continue past line-continuations.
+unsigned Len = skipNewline(First, End);
+if (!wasLineContinuation(First, Len)) // Continue past line-continuations.
+  break;
+  }
 }
 
 static void skipDirective(StringRef Name, const char *,
@@ -380,6 +399,8 @@ void Minimizer::printToNewline(const cha
 // Print out the string.
 if (Last == End || Last == First || Last[-1] != '\\') {
   append(First, reverseOverSpaces(First, Last));
+  First = Last;
+  skipNewline(First, End);
   return;
 }
 

Copied: 
cfe/trunk/test/Lexer/minimize_source_to_dependency_directives_invalid_error.c 
(from r36, 
cfe/trunk/test/Lexer/minimize_source_to_dependency_directives_invalid_error.c)
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Lexer/minimize_source_to_dependency_directives_invalid_error.c?p2=cfe/trunk/test/Lexer/minimize_source_to_dependency_directives_invalid_error.c=cfe/trunk/test/Lexer/minimize_source_to_dependency_directives_invalid_error.c=36=370129=370129=diff
==
--- 

[PATCH] D66839: Fix stack address builtin for negative numbers

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

We usually prefer to generate error messages for incorrect parameters to 
builtins in SemaChecking.cpp.

I don't think there's really an infinite loop here.  The parameter to 
__builtin_frame_address is an unsigned integer; values greater than 0x7FFU 
aren't special.  The reason it takes a very long time is that we try to unroll 
the implied loop; generating an object file with four billion load instructions 
takes essentially forever.  We could impose a smaller limit, but it would be 
sort of arbitrary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66839



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

[PATCH] D58497: Clear the KnownModules cache if the preprocessor is going away

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

rsmith added a comment.

Do we need `KnownModules` at all? It seems to serve a very similar purpose to 
the `Modules` string map on `ModuleMap`. (The main difference seems to be that 
`KnownModules` can cache module load failures.) If we can keep only a single 
data structure tracking this, owned by the `ModuleMap` (which also owns the 
modules), that should remove the possibility for dangling module pointers.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58497



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

[PATCH] D66627: [clang-tidy] add checks to bugprone-posix-return

[Jian Cai via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:70
 
+- New :doc:`bugprone-posix-return
+  ` check.

Eugene.Zelenko wrote:
> Check is not new, just modified. Such check should be after new checks.
Is Modified the right keyword for modification? Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66627



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

[PATCH] D66627: [clang-tidy] add checks to bugprone-posix-return

[Jian Cai via Phabricator via cfe-commits]

jcai19 updated this revision to Diff 217532.
jcai19 added a comment.

Fix format and test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66627

Files:
  clang-tools-extra/clang-tidy/bugprone/PosixReturnCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/PosixReturnCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-posix-return.rst
  clang-tools-extra/test/clang-tidy/bugprone-posix-return.cpp

Index: clang-tools-extra/test/clang-tidy/bugprone-posix-return.cpp
===
--- clang-tools-extra/test/clang-tidy/bugprone-posix-return.cpp
+++ clang-tools-extra/test/clang-tidy/bugprone-posix-return.cpp
@@ -9,6 +9,16 @@
 typedef decltype(sizeof(int)) size_t;
 typedef struct __posix_spawn_file_actions* posix_spawn_file_actions_t;
 typedef struct __posix_spawnattr* posix_spawnattr_t;
+# define __CPU_SETSIZE 1024
+# define __NCPUBITS (8 * sizeof (__cpu_mask))
+typedef unsigned long int __cpu_mask;
+typedef struct
+{
+  __cpu_mask __bits[__CPU_SETSIZE / __NCPUBITS];
+} cpu_set_t;
+typedef struct _opaque_pthread_t *__darwin_pthread_t;
+typedef __darwin_pthread_t pthread_t;
+typedef struct pthread_attr_t_ *pthread_attr_t;
 
 extern "C" int posix_fadvise(int fd, off_t offset, off_t len, int advice);
 extern "C" int posix_fallocate(int fd, off_t offset, off_t len);
@@ -23,6 +33,12 @@
  const posix_spawn_file_actions_t *file_actions,
  const posix_spawnattr_t *attrp,
  char *const argv[], char *const envp[]);
+extern "C" int pthread_create(pthread_t *thread, const pthread_attr_t *attr, void *(*start_routine)(void *), void *arg);
+extern "C" int pthread_attr_setaffinity_np(pthread_attr_t *attr, size_t cpusetsize, const cpu_set_t *cpuset);
+extern "C" int pthread_attr_setschedpolicy(pthread_attr_t *attr, int policy);
+extern "C" int pthread_attr_init(pthread_attr_t *attr);
+extern "C" int pthread_yield(void);
+
 
 void warningLessThanZero() {
   if (posix_fadvise(0, 0, 0, 0) < 0) {}
@@ -43,11 +59,38 @@
   if (posix_spawnp(NULL, NULL, NULL, NULL, {NULL}, {NULL}) < 0) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:60: warning:
   // CHECK-FIXES: posix_spawnp(NULL, NULL, NULL, NULL, {NULL}, {NULL}) > 0
+  if (pthread_create(NULL, NULL, NULL, NULL) < 0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:46: warning: the comparison always evaluates to false because pthread_create always returns non-negative values
+  // CHECK-FIXES: pthread_create(NULL, NULL, NULL, NULL) > 0
+  if (pthread_attr_setaffinity_np(NULL, 0, NULL) < 0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:50: warning:
+  // CHECK-FIXES: pthread_attr_setaffinity_np(NULL, 0, NULL) > 0
+  if (pthread_attr_setschedpolicy(NULL, 0) < 0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:44: warning:
+  // CHECK-FIXES: pthread_attr_setschedpolicy(NULL, 0) > 0)
+  if (pthread_attr_init(NULL) < 0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning:
+  // CHECK-FIXES: pthread_attr_init(NULL) > 0
+  if (pthread_yield() < 0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning:
+  // CHECK-FIXES: pthread_yield() > 0
+
 }
 
 void warningAlwaysTrue() {
   if (posix_fadvise(0, 0, 0, 0) >= 0) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: the comparison always evaluates to true because posix_fadvise always returns non-negative values
+  if (pthread_create(NULL, NULL, NULL, NULL) >= 0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:46: warning: the comparison always evaluates to true because pthread_create always returns non-negative values
+  if (pthread_attr_setaffinity_np(NULL, 0, NULL) >= 0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:50: warning:
+  if (pthread_attr_setschedpolicy(NULL, 0) >= 0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:44: warning:
+  if (pthread_attr_init(NULL) >= 0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning:
+  if (pthread_yield() >= 0) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning:
+
 }
 
 void warningEqualsNegative() {
@@ -69,6 +112,15 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:59: warning:
   if (posix_spawnp(NULL, NULL, NULL, NULL, {NULL}, {NULL}) == -1) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:60: warning:
+  if (pthread_create(NULL, NULL, NULL, NULL) == -1) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:46: warning: pthread_create
+  if (pthread_create(NULL, NULL, NULL, NULL) != -1) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
+  if (pthread_create(NULL, NULL, NULL, NULL) <= -1) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
+  if (pthread_create(NULL, NULL, NULL, NULL) < -1) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
+
 }
 
 void WarningWithMacro() {
@@ -85,6 +137,20 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
   if (posix_fadvise(0, 0, 0, 0) < NEGATIVE_ONE) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:33: warning:
+  if (pthread_create(NULL, NULL, NULL, NULL) < ZERO) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:46: warning:
+  // CHECK-FIXES: 

[PATCH] D66601: [Clang][Bundler] Do not require host triple for extracting device bundles

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


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

https://reviews.llvm.org/D66601



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

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I'm not sure what happens, but I see you added .gitattributes. I'd commit it as 
is. Buildbots using svn will keep working. You can check that the monorepo has 
the right line endings afterwards, and try again if not.


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

https://reviews.llvm.org/D66556



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

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

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

dexonsmith added a comment.

In D66556#1648109 , @rnk wrote:

> I'm not sure what happens, but I see you added .gitattributes. I'd commit it 
> as is. Buildbots using svn will keep working. You can check that the monorepo 
> has the right line endings afterwards, and try again if not.


SGTM.


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

https://reviews.llvm.org/D66556



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

[PATCH] D66601: [Clang][Bundler] Do not require host triple for extracting device bundles

[Sergey Dmitriev via Phabricator via cfe-commits]

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



Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:888
+  // treat missing host triple as error if we do unbundling.
+  if ((Unbundle && HostTargetNum > 1) || (!Unbundle && HostTargetNum != 1)) {
 Error = true;

ABataev wrote:
> ABataev wrote:
> > sdmitriev wrote:
> > > sdmitriev wrote:
> > > > sdmitriev wrote:
> > > > > ABataev wrote:
> > > > > > sdmitriev wrote:
> > > > > > > ABataev wrote:
> > > > > > > > I believe,  for unbundling we also must check for `!= 1` rather 
> > > > > > > > than `> 1`. Zero host targets also is not allowed.
> > > > > > > But the whole idea of this change is to remove requirement to 
> > > > > > > provide host triple for unbundling operation. Target bundle(s) 
> > > > > > > can always be extracted without extracting host, so host bundle 
> > > > > > > is optional. Therefore zero host targets should not be considered 
> > > > > > > as error for unbundling.
> > > > > > And why do we need this? I think it would be better to check that 
> > > > > > the requested host triple matches the bundled one using this 
> > > > > > parameter rather than removing it.
> > > > > > And why do we need this?
> > > > > 
> > > > > As I wrote in the summary it is a usability issue. You may for 
> > > > > example want to extract device object for a particular offload target 
> > > > > to examine its contents (symbols, sections, etc..), but currently you 
> > > > > also have to extract host bundle as well even if you do not need it.
> > > > > 
> > > > > > I think it would be better to check that the requested host triple 
> > > > > > matches the bundled one using this parameter rather than removing 
> > > > > > it.
> > > > > 
> > > > > So you suggest to check that host bundle name that exists in the fat 
> > > > > image matches the host bundle name provided it command line if it was 
> > > > > provided? Should it be an error if names do not match?
> > > > > 
> > > > I have updated patch to do error checking if host bundle name was 
> > > > provided in command line.
> > > @ABataev I believe the host bundle name is now being checked as you 
> > > suggested. Can you please confirm that it matches your expectations?
> > Ok, I got the idea of the patch. BTW, will happen if I request the device 
> > code for the triple, which is correct, but bundled container does not have 
> > the device code for this triple?
> What about this?
Based on the comments on lines 775-776 and 801 in ClangOffloadBundler.cpp 
bundler will create empty files for missing device bundles.


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

https://reviews.llvm.org/D66601



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

[PATCH] D66601: [Clang][Bundler] Do not require host triple for extracting device bundles

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:888
+  // treat missing host triple as error if we do unbundling.
+  if ((Unbundle && HostTargetNum > 1) || (!Unbundle && HostTargetNum != 1)) {
 Error = true;

ABataev wrote:
> sdmitriev wrote:
> > sdmitriev wrote:
> > > sdmitriev wrote:
> > > > ABataev wrote:
> > > > > sdmitriev wrote:
> > > > > > ABataev wrote:
> > > > > > > I believe,  for unbundling we also must check for `!= 1` rather 
> > > > > > > than `> 1`. Zero host targets also is not allowed.
> > > > > > But the whole idea of this change is to remove requirement to 
> > > > > > provide host triple for unbundling operation. Target bundle(s) can 
> > > > > > always be extracted without extracting host, so host bundle is 
> > > > > > optional. Therefore zero host targets should not be considered as 
> > > > > > error for unbundling.
> > > > > And why do we need this? I think it would be better to check that the 
> > > > > requested host triple matches the bundled one using this parameter 
> > > > > rather than removing it.
> > > > > And why do we need this?
> > > > 
> > > > As I wrote in the summary it is a usability issue. You may for example 
> > > > want to extract device object for a particular offload target to 
> > > > examine its contents (symbols, sections, etc..), but currently you also 
> > > > have to extract host bundle as well even if you do not need it.
> > > > 
> > > > > I think it would be better to check that the requested host triple 
> > > > > matches the bundled one using this parameter rather than removing it.
> > > > 
> > > > So you suggest to check that host bundle name that exists in the fat 
> > > > image matches the host bundle name provided it command line if it was 
> > > > provided? Should it be an error if names do not match?
> > > > 
> > > I have updated patch to do error checking if host bundle name was 
> > > provided in command line.
> > @ABataev I believe the host bundle name is now being checked as you 
> > suggested. Can you please confirm that it matches your expectations?
> Ok, I got the idea of the patch. BTW, will happen if I request the device 
> code for the triple, which is correct, but bundled container does not have 
> the device code for this triple?
What about this?


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

https://reviews.llvm.org/D66601



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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 217528.
paulkirth added a comment.

Add comment to clarify choice of operands and reasoning about profiling metadata


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66324

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/FixedMetadataKinds.def
  llvm/include/llvm/IR/MDBuilder.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/ThinLTO/X86/lazyload_metadata.ll
  llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,293 @@
+
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; New PM
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.c.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=CORRECT
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:5: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:5: Potential performance regression from use of __builtin_expect(): Annotation was correct on 0.00% of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:5: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:5: Potential performance regression from use of __builtin_expect(): Annotation was correct on 0.00% of profiled executions.
+
+; BOTH-DAG: warning: misexpect-switch.c:26:5: 0.00%
+; BOTH-DAG: remark: misexpect-switch.c:26:5: Potential performance regression from use of __builtin_expect(): Annotation was correct on 0.00% of profiled executions.
+
+; DISABLED-NOT: warning: 

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

[Alexandre Ganea via Phabricator via cfe-commits]

aganea updated this revision to Diff 217527.
aganea added subscribers: jyknight, rnk.
aganea added a comment.

In D66556#1647669 , @arphaman wrote:

> Will the git monorepo handle `svn:eol-style` correctly?


Other files in the repo already use `svn:eol-style`. I'm not sure what happens 
with SVN properties when commits are replicated in git. @jyknight @rnk ? I've 
added `.gitattributes` to ensure these files are checked out from git with the 
proper line ending.


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

https://reviews.llvm.org/D66556

Files:
  .gitattributes
  lib/Lex/DependencyDirectivesSourceMinimizer.cpp
  test/Lexer/minimize_source_to_dependency_directives_invalid_error.c

Index: test/Lexer/minimize_source_to_dependency_directives_invalid_error.c
===
--- test/Lexer/minimize_source_to_dependency_directives_invalid_error.c
+++ test/Lexer/minimize_source_to_dependency_directives_invalid_error.c
@@ -1,5 +1,5 @@
 // Test CF+LF are properly handled along with quoted, multi-line #error
-// RUN: cat %s | unix2dos | %clang_cc1 -DOTHER -print-dependency-directives-minimized-source 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -DOTHER -print-dependency-directives-minimized-source %s 2>&1 | FileCheck %s
 
 #ifndef TEST
 #error "message \
Index: lib/Lex/DependencyDirectivesSourceMinimizer.cpp
===
--- lib/Lex/DependencyDirectivesSourceMinimizer.cpp
+++ lib/Lex/DependencyDirectivesSourceMinimizer.cpp
@@ -196,15 +196,29 @@
 ++First; // Finish off the string.
 }
 
-static void skipNewline(const char *, const char *End) {
-  assert(isVerticalWhitespace(*First));
-  ++First;
+// Returns the length of EOL, either 0 (no end-of-line), 1 (\n) or 2 (\r\n)
+static unsigned isEOL(const char *First, const char *const End) {
   if (First == End)
-return;
+return 0;
+  if (End - First > 1 && isVerticalWhitespace(First[0]) &&
+  isVerticalWhitespace(First[1]) && First[0] != First[1])
+return 2;
+  return !!isVerticalWhitespace(First[0]);
+}
 
-  // Check for "\n\r" and "\r\n".
-  if (LLVM_UNLIKELY(isVerticalWhitespace(*First) && First[-1] != First[0]))
-++First;
+// Returns the length of the skipped newline
+static unsigned skipNewline(const char *, const char *End) {
+  if (First == End)
+return 0;
+  assert(isVerticalWhitespace(*First));
+  unsigned Len = isEOL(First, End);
+  assert(Len && "expected newline");
+  First += Len;
+  return Len;
+}
+
+static bool wasLineContinuation(const char *First, unsigned EOLLen) {
+  return *(First - (int)EOLLen - 1) == '\\';
 }
 
 static void skipToNewlineRaw(const char *, const char *const End) {
@@ -212,17 +226,21 @@
 if (First == End)
   return;
 
-if (isVerticalWhitespace(*First))
+unsigned Len = isEOL(First, End);
+if (Len)
   return;
 
-while (!isVerticalWhitespace(*First))
+do {
   if (++First == End)
 return;
+  Len = isEOL(First, End);
+} while (!Len);
 
 if (First[-1] != '\\')
   return;
 
-++First; // Keep going...
+First += Len;
+// Keep skipping lines...
   }
 }
 
@@ -277,7 +295,7 @@
 }
 
 static void skipLine(const char *, const char *const End) {
-  do {
+  for (;;) {
 assert(First <= End);
 if (First == End)
   return;
@@ -322,9 +340,10 @@
   return;
 
 // Skip over the newline.
-assert(isVerticalWhitespace(*First));
-skipNewline(First, End);
-  } while (First[-2] == '\\'); // Continue past line-continuations.
+unsigned Len = skipNewline(First, End);
+if (!wasLineContinuation(First, Len)) // Continue past line-continuations.
+  break;
+  }
 }
 
 static void skipDirective(StringRef Name, const char *,
@@ -380,6 +399,8 @@
 // Print out the string.
 if (Last == End || Last == First || Last[-1] != '\\') {
   append(First, reverseOverSpaces(First, Last));
+  First = Last;
+  skipNewline(First, End);
   return;
 }
 
Index: .gitattributes
===
--- .gitattributes
+++ .gitattributes
@@ -0,0 +1,4 @@
+# Windows line ending tests
+test/Lexer/minimize_source_to_dependency_directives_invalid_error.c text eol=crlf
+test/FixIt/fixit-newline-style.c text eol=crlf
+test/Frontend/system-header-line-directive-ms-lineendings.c text eol=crlf
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66836: [libc++] Add `__truncating_cast` for safely casting float types to integers

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

Seems like a very useful function. `__max_representable_int_for_float` also 
seems useful. Should this work in C++03? If so there are a few changes that 
need to be made. It would also be great if this could be a `constexpr` (but, 
obviously, not necessary).




Comment at: include/math.h:1556
 
+_LIBCPP_BEGIN_NAMESPACE_STD
+template ::digits > 
numeric_limits<_IntT>::digits)>
+struct __max_representable_int_for_float;

Nit: maybe qualify all the uses of `numeric_limits` and similar?



Comment at: include/math.h:1572
+  static_assert(is_integral<_IntT>::value, "must be an integral type");
+  enum { _Bits = numeric_limits<_IntT>::digits - 
numeric_limits<_FloatT>::digits };
+  static const _IntT value = numeric_limits<_IntT>::max() >> _Bits << _Bits;

What is the enum providing for you? Couldn't this just be `static const int 
_Bits  = ...`?



Comment at: include/math.h:1573
+  enum { _Bits = numeric_limits<_IntT>::digits - 
numeric_limits<_FloatT>::digits };
+  static const _IntT value = numeric_limits<_IntT>::max() >> _Bits << _Bits;
+};

What's the reasoning behind shifting something forward and back? Shouldn't this 
always negate the other operation? 



Comment at: include/math.h:1579
+_IntT __truncating_cast(_RealT __r) _NOEXCEPT {
+  using _Lim = std::numeric_limits<_IntT>;
+  const _IntT _MaxVal = __max_representable_int_for_float<_IntT, 
_RealT>::value;

This will not work before C++11.



Comment at: include/math.h:1582
+  const _RealT __trunc_r = __builtin_trunc(__r);
+  if (__trunc_r >= ::nextafter(static_cast<_RealT>(_MaxVal), INFINITY)) {
+return _Lim::max();

Maybe change `INFINITY` to `std::numeric_limits< _RealT >::infinity()`



Comment at: test/libcxx/numerics/truncating_cast.pass.cpp:10
+// __truncating_cast(RealT)
+
+// Test the conversion function that truncates floating point types to the

Is this supposed to work in C++03? If so, update this test and 
`__truncating_cast`. Otherwise, add an `#if` and a `// UNSUPPORTED: C++98, 
C++03`



Comment at: test/libcxx/numerics/truncating_cast.pass.cpp:25
+  struct {
+double Input;
+IntT Expect;

Maybe test with more than just `double`. `float`, `long double`,  others?



Comment at: test/libcxx/numerics/truncating_cast.pass.cpp:28
+bool IsRepresentable;
+  } TestCases[] = {
+  {0, 0, true},

C++03 will not like this :P


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D66836



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

[PATCH] D66601: [Clang][Bundler] Do not require host triple for extracting device bundles

[Sergey Dmitriev via Phabricator via cfe-commits]

sdmitriev updated this revision to Diff 217522.
sdmitriev marked an inline comment as done.
sdmitriev added a comment.

Added tests for each file type.


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

https://reviews.llvm.org/D66601

Files:
  clang/test/Driver/clang-offload-bundler.c
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -791,8 +791,9 @@
 return false;
   }
 
-  // If we found elements, we emit an error if none of those were for the host.
-  if (!FoundHostBundle) {
+  // If we found elements, we emit an error if none of those were for the host
+  // in case host bundle name was provided in command line.
+  if (!FoundHostBundle && HostInputIndex != ~0u) {
 errs() << "error: Can't find bundle for the host target\n";
 return true;
   }
@@ -895,7 +896,9 @@
 ++Index;
   }
 
-  if (HostTargetNum != 1) {
+  // Host triple is not really needed for unbundling operation, so do not
+  // treat missing host triple as error if we do unbundling.
+  if ((Unbundle && HostTargetNum > 1) || (!Unbundle && HostTargetNum != 1)) {
 Error = true;
 errs() << "error: expecting exactly one host target but got "
<< HostTargetNum << ".\n";
Index: clang/test/Driver/clang-offload-bundler.c
===
--- clang/test/Driver/clang-offload-bundler.c
+++ clang/test/Driver/clang-offload-bundler.c
@@ -156,14 +156,20 @@
 // RUN: diff %t.i %t.res.i
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=i -targets=openmp-powerpc64le-ibm-linux-gnu -outputs=%t.res.tgt1 -inputs=%t.bundle3.i -unbundle
+// RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: clang-offload-bundler -type=ii -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.res.ii,%t.res.tgt1,%t.res.tgt2 -inputs=%t.bundle3.ii -unbundle
 // RUN: diff %t.ii %t.res.ii
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=ii -targets=openmp-x86_64-pc-linux-gnu -outputs=%t.res.tgt2 -inputs=%t.bundle3.ii -unbundle
+// RUN: diff %t.tgt2 %t.res.tgt2
 // RUN: clang-offload-bundler -type=ll -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.res.ll,%t.res.tgt1,%t.res.tgt2 -inputs=%t.bundle3.ll -unbundle
 // RUN: diff %t.ll %t.res.ll
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=ll -targets=openmp-powerpc64le-ibm-linux-gnu -outputs=%t.res.tgt1 -inputs=%t.bundle3.ll -unbundle
+// RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: clang-offload-bundler -type=s -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.res.s,%t.res.tgt1,%t.res.tgt2 -inputs=%t.bundle3.s -unbundle
 // RUN: diff %t.s %t.res.s
 // RUN: diff %t.tgt1 %t.res.tgt1
@@ -172,6 +178,8 @@
 // RUN: diff %t.s %t.res.s
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=s -targets=openmp-x86_64-pc-linux-gnu -outputs=%t.res.tgt2 -inputs=%t.bundle3.s -unbundle
+// RUN: diff %t.tgt2 %t.res.tgt2
 
 // Check if we can unbundle a file with no magic strings.
 // RUN: clang-offload-bundler -type=s -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.res.s,%t.res.tgt1,%t.res.tgt2 -inputs=%t.s -unbundle
@@ -183,6 +191,10 @@
 // RUN: diff %t.empty %t.res.tgt1
 // RUN: diff %t.empty %t.res.tgt2
 
+// Check that bindler prints an error if given host bundle does not exist in the fat binary.
+// RUN: not clang-offload-bundler -type=s -targets=host-x86_64-xxx-linux-gnu,openmp-powerpc64le-ibm-linux-gnu -outputs=%t.res.s,%t.res.tgt1 -inputs=%t.bundle3.s -unbundle 2>&1 | FileCheck %s --check-prefix CK-NO-HOST-BUNDLE
+// CK-NO-HOST-BUNDLE: error: Can't find bundle for the host target
+
 //
 // Check binary bundle/unbundle. The content that we have before bundling must be the same we have after unbundling.
 //
@@ -194,10 +206,14 @@
 // RUN: diff %t.bc %t.res.bc
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=bc -targets=openmp-powerpc64le-ibm-linux-gnu -outputs=%t.res.tgt1 -inputs=%t.bundle3.bc -unbundle
+// RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: clang-offload-bundler -type=gch -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.res.gch,%t.res.tgt1,%t.res.tgt2 -inputs=%t.bundle3.gch -unbundle
 // RUN: diff %t.ast %t.res.gch
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
+// RUN: clang-offload-bundler -type=gch -targets=openmp-x86_64-pc-linux-gnu 

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:150
+// Operand 0 is a string tag "branch_weights"
+if (MDString *Tag = cast(MD->getOperand(0))) {
+  unsigned NOps = MD->getNumOperands();

paulkirth wrote:
> nickdesaulniers wrote:
> > paulkirth wrote:
> > > nickdesaulniers wrote:
> > > > Sorry if I'm going back and forth on this, but it may make sense to 
> > > > check the number of operands first, before accessing any.
> > > No problem. I've reintroduced the check. I was under the impression that 
> > > metadata could not lack a 0th element, but even in that case I should 
> > > have checked for additional elements. 
> > > 
> > > There should always be at least 2 branch weights, otherwise branch 
> > > weights are not necessary, and we should never end up here.
> > > 
> > > Should I add a comment saying that?
> > Sorry, I meant "check the lower bound of the operands first, ..." as `NOps` 
> > might be `0` which would cause `MD->getOperand(0)` to fail, and 
> > `RealWeights` to be size `-1`.  `if (Nops < 1 || Nops > 3) return;`
> > 
> > I think a comment would be helpful, too.
> Thanks for the clarification and example. I think we're on the same page, but 
> let me make sure, since I think I might have muddied the conversation by 
> adding some incomplete context to my reasoning. Here is my reasoning in full, 
> and a proposed comment. Let me know if there is something I'm missing, or 
> that I've misunderstood.
> 
> I think that the check: `if(NOps <3) return;` is the correct semantics here. 
> Operand 0 will always exist if `NOps >=3`.
> In the case of a switch instruction  `NOps > 3` needs to be supported.
> 
> Any profiling data with only less than 2 branch weights implies that the 
> target is completely deterministic, and should not have branch weights 
> associated with it. However, even if that were that happen, we should not 
> emit any misexpect warnings, 
> 
> Proposed comment:
> ```
> // Only emit misexpect diagnostics if at least 2 branch weights are present. 
> // Less than 2 branch weights means that the profiling metadata is incorrect, 
> // is not branch weight metadata, or that the branch is completely 
> deterministic. 
> // In these cases we should not emit any diagnostic related to misexpect.
> ```
Got it, yep that should be good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66324



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

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

[Alex Lorenz via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D66831



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

[PATCH] D66839: Fix stack address builtin for negative numbers

[Zoe Carver via Phabricator via cfe-commits]

zoecarver created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
zoecarver added reviewers: kparzysz, eli.friedman.
zoecarver marked an inline comment as done.
zoecarver added inline comments.



Comment at: clang/test/Sema/builtin-stackaddress.c:10
+// expected-error@+1 {{argument to '__builtin_return_address' must be a 
constant integer}}
+return __builtin_return_address(x);
 }

For some reason `lit` was still erroring here. 


This patch checks that an argument passed to either of the builtins 
`__builtin_frame_address` or `__builtin_return_address` is at least 0. This 
patch resolves the issue where these builtins would cause an infinite loop ( 
25497  ).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66839

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/Sema/builtin-stackaddress.c


Index: clang/test/Sema/builtin-stackaddress.c
===
--- clang/test/Sema/builtin-stackaddress.c
+++ clang/test/Sema/builtin-stackaddress.c
@@ -1,16 +1,30 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 %s
+
 void* a(unsigned x) {
 return __builtin_return_address(0);
 }
 
 void b(unsigned x) {
-return __builtin_return_address(x); // expected-error{{argument to 
'__builtin_return_address' must be a constant integer}}
+// TODO: lit doesn't catch this
+// expected-error@+1 {{argument to '__builtin_return_address' must be a 
constant integer}}
+return __builtin_return_address(x);
 }
 
 void* c(unsigned x) {
+// expected-error@+1 {{argument to '__builtin_return_address' must be at least 
0}}
+return __builtin_return_address(-1);
+}
+
+void* d(unsigned x) {
 return __builtin_frame_address(0);
 }
 
-void d(unsigned x) {
-return __builtin_frame_address(x); // expected-error{{argument to 
'__builtin_frame_address' must be a constant integer}}
+void e(unsigned x) {
+// expected-error@+1 {{argument to '__builtin_frame_address' must be a 
constant integer}}
+return __builtin_frame_address(x);
+}
+
+void* f(unsigned x) {
+// expected-error@+1 {{argument to '__builtin_frame_address' must be at least 
0}}
+return __builtin_frame_address(-1);
 }
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -2568,6 +2568,14 @@
   case Builtin::BI__builtin_return_address: {
 Value *Depth = ConstantEmitter(*this).emitAbstract(E->getArg(0),
getContext().UnsignedIntTy);
+llvm::ConstantInt* DepthConstVal = dyn_cast(Depth);
+if (!DepthConstVal)
+CGM.ErrorUnsupported(E,
+ "argument to '__builtin_return_address' must be a 
constant integer");
+if (DepthConstVal->getSExtValue() < 0)
+CGM.ErrorUnsupported(E,
+ "argument to '__builtin_return_address' must be 
at least 0");
+
 Function *F = CGM.getIntrinsic(Intrinsic::returnaddress);
 return RValue::get(Builder.CreateCall(F, Depth));
   }
@@ -2578,6 +2586,14 @@
   case Builtin::BI__builtin_frame_address: {
 Value *Depth = ConstantEmitter(*this).emitAbstract(E->getArg(0),
getContext().UnsignedIntTy);
+llvm::ConstantInt* DepthConstVal = dyn_cast(Depth);
+if (!DepthConstVal)
+CGM.ErrorUnsupported(E,
+ "argument to '__builtin_frame_address' must be a 
constant integer");
+if (DepthConstVal->getSExtValue() < 0)
+CGM.ErrorUnsupported(E,
+ "argument to '__builtin_frame_address' must be at 
least 0");
+
 Function *F = CGM.getIntrinsic(Intrinsic::frameaddress, AllocaInt8PtrTy);
 return RValue::get(Builder.CreateCall(F, Depth));
   }


Index: clang/test/Sema/builtin-stackaddress.c
===
--- clang/test/Sema/builtin-stackaddress.c
+++ clang/test/Sema/builtin-stackaddress.c
@@ -1,16 +1,30 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 %s
+
 void* a(unsigned x) {
 return __builtin_return_address(0);
 }
 
 void b(unsigned x) {
-return __builtin_return_address(x); // expected-error{{argument to '__builtin_return_address' must be a constant integer}}
+// TODO: lit doesn't catch this
+// expected-error@+1 {{argument to '__builtin_return_address' must be a constant integer}}
+return __builtin_return_address(x);
 }
 
 void* c(unsigned x) {
+// expected-error@+1 {{argument to '__builtin_return_address' must be at least 0}}
+return __builtin_return_address(-1);
+}
+
+void* d(unsigned x) {
 return __builtin_frame_address(0);
 }
 
-void d(unsigned x) {
-return __builtin_frame_address(x); // expected-error{{argument to '__builtin_frame_address' must be a constant integer}}
+void e(unsigned x) {
+// expected-error@+1 

[PATCH] D66839: Fix stack address builtin for negative numbers

[Zoe Carver via Phabricator via cfe-commits]

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



Comment at: clang/test/Sema/builtin-stackaddress.c:10
+// expected-error@+1 {{argument to '__builtin_return_address' must be a 
constant integer}}
+return __builtin_return_address(x);
 }

For some reason `lit` was still erroring here. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66839



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

[PATCH] D66733: [analyzer] Add a checker option to detect nested dead stores

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

steakhal added a comment.

@Szelethus The mispositioned report message was my fault. I used a different 
version of clang for the analysis and to upload the results, which resulted in 
some mispositioned reports.
I've fixed the linked CodeChecker instance.


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

https://reviews.llvm.org/D66733



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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Paul Kirth via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:150
+// Operand 0 is a string tag "branch_weights"
+if (MDString *Tag = cast(MD->getOperand(0))) {
+  unsigned NOps = MD->getNumOperands();

nickdesaulniers wrote:
> paulkirth wrote:
> > nickdesaulniers wrote:
> > > Sorry if I'm going back and forth on this, but it may make sense to check 
> > > the number of operands first, before accessing any.
> > No problem. I've reintroduced the check. I was under the impression that 
> > metadata could not lack a 0th element, but even in that case I should have 
> > checked for additional elements. 
> > 
> > There should always be at least 2 branch weights, otherwise branch weights 
> > are not necessary, and we should never end up here.
> > 
> > Should I add a comment saying that?
> Sorry, I meant "check the lower bound of the operands first, ..." as `NOps` 
> might be `0` which would cause `MD->getOperand(0)` to fail, and `RealWeights` 
> to be size `-1`.  `if (Nops < 1 || Nops > 3) return;`
> 
> I think a comment would be helpful, too.
Thanks for the clarification and example. I think we're on the same page, but 
let me make sure, since I think I might have muddied the conversation by adding 
some incomplete context to my reasoning. Here is my reasoning in full, and a 
proposed comment. Let me know if there is something I'm missing, or that I've 
misunderstood.

I think that the check: `if(NOps <3) return;` is the correct semantics here. 
Operand 0 will always exist if `NOps >=3`.
In the case of a switch instruction  `NOps > 3` needs to be supported.

Any profiling data with only less than 2 branch weights implies that the target 
is completely deterministic, and should not have branch weights associated with 
it. However, even if that were that happen, we should not emit any misexpect 
warnings, 

Proposed comment:
```
// Only emit misexpect diagnostics if at least 2 branch weights are present. 
// Less than 2 branch weights means that the profiling metadata is incorrect, 
// is not branch weight metadata, or that the branch is completely 
deterministic. 
// In these cases we should not emit any diagnostic related to misexpect.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66324



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

[PATCH] D66836: [libc++] Add `__truncating_cast` for safely casting float types to integers

[Eric Fiselier via Phabricator via cfe-commits]

EricWF created this revision.
EricWF added reviewers: mclow.lists, ldionne.
Herald added subscribers: libcxx-commits, dexonsmith, christof.

This is needed anytime we need to clamp an arbitrary floating point value to an 
integer type.


Repository:
  rCXX libc++

https://reviews.llvm.org/D66836

Files:
  include/math.h
  test/libcxx/numerics/truncating_cast.pass.cpp

Index: test/libcxx/numerics/truncating_cast.pass.cpp
===
--- /dev/null
+++ test/libcxx/numerics/truncating_cast.pass.cpp
@@ -0,0 +1,56 @@
+//===--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+// __truncating_cast(RealT)
+
+// Test the conversion function that truncates floating point types to the
+// closes representable value for the specified integer type, or
+// numeric_limits::max()/min() if the value isn't representable.
+
+#include 
+#include 
+#include 
+
+template 
+void test() {
+  using Lim = std::numeric_limits;
+  constexpr bool MaxIsRepresentable = sizeof(IntT) < 8;
+  constexpr bool IsSigned = std::is_signed::value;
+  struct {
+double Input;
+IntT Expect;
+bool IsRepresentable;
+  } TestCases[] = {
+  {0, 0, true},
+  {1, 1, true},
+  {IsSigned ? static_cast(-1) : 0,
+   IsSigned ? static_cast(-1) : 0, true},
+  {Lim::lowest(), Lim::lowest(), true},
+  {static_cast(Lim::max()), Lim::max(), MaxIsRepresentable},
+  {static_cast(Lim::max()) + 1, Lim::max(), false},
+  {static_cast(Lim::max()) + 1024, Lim::max(), false},
+  };
+  for (auto TC : TestCases) {
+auto res = std::__truncating_cast(TC.Input);
+assert(res == TC.Expect);
+if (TC.IsRepresentable) {
+  auto other = static_cast(std::trunc(TC.Input));
+  assert(res == other);
+} else
+  assert(res == Lim::min() || res == Lim::max());
+  }
+}
+
+int main() {
+  test();
+  test();
+  test();
+  test();
+  test();
+  test();
+}
Index: include/math.h
===
--- include/math.h
+++ include/math.h
@@ -1553,6 +1553,41 @@
 typename std::enable_if::value, double>::type
 trunc(_A1 __lcpp_x) _NOEXCEPT {return ::trunc((double)__lcpp_x);}
 
+_LIBCPP_BEGIN_NAMESPACE_STD
+template ::digits > numeric_limits<_IntT>::digits)>
+struct __max_representable_int_for_float;
+
+template 
+struct __max_representable_int_for_float<_IntT, _FloatT, true> {
+  static_assert(is_floating_point<_FloatT>::value, "must be a floating point type");
+  static_assert(is_integral<_IntT>::value, "must be an integral type");
+  static const _IntT value = numeric_limits<_IntT>::max();
+};
+
+template 
+struct __max_representable_int_for_float<_IntT, _FloatT, false> {
+  static_assert(is_floating_point<_FloatT>::value, "must be a floating point type");
+  static_assert(is_integral<_IntT>::value, "must be an integral type");
+  enum { _Bits = numeric_limits<_IntT>::digits - numeric_limits<_FloatT>::digits };
+  static const _IntT value = numeric_limits<_IntT>::max() >> _Bits << _Bits;
+};
+
+template 
+_LIBCPP_INLINE_VISIBILITY
+_IntT __truncating_cast(_RealT __r) _NOEXCEPT {
+  using _Lim = std::numeric_limits<_IntT>;
+  const _IntT _MaxVal = __max_representable_int_for_float<_IntT, _RealT>::value;
+  const _RealT __trunc_r = __builtin_trunc(__r);
+  if (__trunc_r >= ::nextafter(static_cast<_RealT>(_MaxVal), INFINITY)) {
+return _Lim::max();
+  } else if (__trunc_r <= _Lim::lowest()) {
+return _Lim::min();
+  }
+  return static_cast<_IntT>(__trunc_r);
+}
+_LIBCPP_END_NAMESPACE_STD
+
 } // extern "C++"
 
 #endif // __cplusplus
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r370123 - [preprocessor] Add an opportunity to retain excluded conditional blocks

[Evgeny Mankov via cfe-commits]

Author: emankov
Date: Tue Aug 27 15:15:32 2019
New Revision: 370123

URL: http://llvm.org/viewvc/llvm-project?rev=370123=rev
Log:
[preprocessor] Add an opportunity to retain excluded conditional blocks

It is handy for clang tooling, for instance, in source to source transformation.

Reviewers: vpykhtin (Valery Pykhtin), erichkeane (Erich Keane)

Subscribers: rsmith (Richard Smith), akyrtzi (Argyrios Kyrtzidis)

Tags: #clang

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

Added:
cfe/trunk/test/Index/retain-excluded-conditional-blocks.m
Modified:
cfe/trunk/include/clang-c/Index.h
cfe/trunk/include/clang/Frontend/ASTUnit.h
cfe/trunk/include/clang/Lex/PreprocessorOptions.h
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/tools/c-index-test/c-index-test.c
cfe/trunk/tools/libclang/CIndex.cpp

Modified: cfe/trunk/include/clang-c/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=370123=370122=370123=diff
==
--- cfe/trunk/include/clang-c/Index.h (original)
+++ cfe/trunk/include/clang-c/Index.h Tue Aug 27 15:15:32 2019
@@ -1356,7 +1356,12 @@ enum CXTranslationUnit_Flags {
* the case where these warnings are not of interest, as for an IDE for
* example, which typically shows only the diagnostics in the main file.
*/
-  CXTranslationUnit_IgnoreNonErrorsFromIncludedFiles = 0x4000
+  CXTranslationUnit_IgnoreNonErrorsFromIncludedFiles = 0x4000,
+
+  /**
+   * Tells the preprocessor not to skip excluded conditional blocks.
+   */
+  CXTranslationUnit_RetainExcludedConditionalBlocks = 0x8000,
 };
 
 /**

Modified: cfe/trunk/include/clang/Frontend/ASTUnit.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/ASTUnit.h?rev=370123=370122=370123=diff
==
--- cfe/trunk/include/clang/Frontend/ASTUnit.h (original)
+++ cfe/trunk/include/clang/Frontend/ASTUnit.h Tue Aug 27 15:15:32 2019
@@ -832,6 +832,7 @@ public:
   SkipFunctionBodiesScope::None,
   bool SingleFileParse = false, bool UserFilesAreVolatile = false,
   bool ForSerialization = false,
+  bool RetainExcludedConditionalBlocks = false,
   llvm::Optional ModuleFormat = llvm::None,
   std::unique_ptr *ErrAST = nullptr,
   IntrusiveRefCntPtr VFS = nullptr);

Modified: cfe/trunk/include/clang/Lex/PreprocessorOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/PreprocessorOptions.h?rev=370123=370122=370123=diff
==
--- cfe/trunk/include/clang/Lex/PreprocessorOptions.h (original)
+++ cfe/trunk/include/clang/Lex/PreprocessorOptions.h Tue Aug 27 15:15:32 2019
@@ -142,6 +142,9 @@ public:
   /// compiler invocation and its buffers will be reused.
   bool RetainRemappedFileBuffers = false;
 
+  /// When enabled, excluded conditional blocks retain in the main file.
+  bool RetainExcludedConditionalBlocks = false;
+
   /// The Objective-C++ ARC standard library that we should support,
   /// by providing appropriate definitions to retrofit the standard library
   /// with support for lifetime-qualified pointers.
@@ -201,6 +204,7 @@ public:
 RetainRemappedFileBuffers = true;
 PrecompiledPreambleBytes.first = 0;
 PrecompiledPreambleBytes.second = false;
+RetainExcludedConditionalBlocks = false;
   }
 };
 

Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=370123=370122=370123=diff
==
--- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp Tue Aug 27 15:15:32 2019
@@ -1735,6 +1735,7 @@ ASTUnit *ASTUnit::LoadFromCommandLine(
 bool CacheCodeCompletionResults, bool IncludeBriefCommentsInCodeCompletion,
 bool AllowPCHWithCompilerErrors, SkipFunctionBodiesScope 
SkipFunctionBodies,
 bool SingleFileParse, bool UserFilesAreVolatile, bool ForSerialization,
+bool RetainExcludedConditionalBlocks,
 llvm::Optional ModuleFormat, std::unique_ptr *ErrAST,
 IntrusiveRefCntPtr VFS) {
   assert(Diags.get() && "no DiagnosticsEngine was provided");
@@ -1762,6 +1763,7 @@ ASTUnit *ASTUnit::LoadFromCommandLine(
   PPOpts.RemappedFilesKeepOriginalName = RemappedFilesKeepOriginalName;
   PPOpts.AllowPCHWithCompilerErrors = AllowPCHWithCompilerErrors;
   PPOpts.SingleFileParseMode = SingleFileParse;
+  PPOpts.RetainExcludedConditionalBlocks = RetainExcludedConditionalBlocks;
 
   // Override the resources path.
   CI->getHeaderSearchOpts().ResourceDir = ResourceFilesPath;

Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=370123=370122=370123=diff

[PATCH] D66797: ArrayRef'ized CompilerInvocation::CreateFromArgs

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370122: ArrayRefized 
CompilerInvocation::CreateFromArgs (authored by gribozavr, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66797?vs=217365=217509#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66797

Files:
  cfe/trunk/include/clang/Frontend/CompilerInvocation.h
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
  cfe/trunk/lib/Tooling/Tooling.cpp
  cfe/trunk/tools/arcmt-test/arcmt-test.cpp
  cfe/trunk/tools/clang-import-test/clang-import-test.cpp
  cfe/trunk/tools/driver/cc1_main.cpp
  cfe/trunk/unittests/AST/ExternalASTSourceTest.cpp

Index: cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
===
--- cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -90,9 +90,7 @@
 
   const ArgStringList  = Cmd.getArguments();
   auto CI = std::make_unique();
-  if (!CompilerInvocation::CreateFromArgs(
-  *CI, const_cast(CCArgs.data()),
-  const_cast(CCArgs.data()) + CCArgs.size(), *Diags) &&
+  if (!CompilerInvocation::CreateFromArgs(*CI, CCArgs, *Diags) &&
   !ShouldRecoverOnErorrs)
 return nullptr;
   return CI;
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -3370,8 +3370,7 @@
 }
 
 bool CompilerInvocation::CreateFromArgs(CompilerInvocation ,
-const char *const *ArgBegin,
-const char *const *ArgEnd,
+ArrayRef CommandLineArgs,
 DiagnosticsEngine ) {
   bool Success = true;
 
@@ -3379,9 +3378,8 @@
   std::unique_ptr Opts = createDriverOptTable();
   const unsigned IncludedFlagsBitmask = options::CC1Option;
   unsigned MissingArgIndex, MissingArgCount;
-  InputArgList Args =
-  Opts->ParseArgs(llvm::makeArrayRef(ArgBegin, ArgEnd), MissingArgIndex,
-  MissingArgCount, IncludedFlagsBitmask);
+  InputArgList Args = Opts->ParseArgs(CommandLineArgs, MissingArgIndex,
+  MissingArgCount, IncludedFlagsBitmask);
   LangOptions  = *Res.getLangOpts();
 
   // Check for missing argument error.
Index: cfe/trunk/lib/Tooling/Tooling.cpp
===
--- cfe/trunk/lib/Tooling/Tooling.cpp
+++ cfe/trunk/lib/Tooling/Tooling.cpp
@@ -118,9 +118,7 @@
 DiagnosticsEngine *Diagnostics, const llvm::opt::ArgStringList ) {
   assert(!CC1Args.empty() && "Must at least contain the program name!");
   CompilerInvocation *Invocation = new CompilerInvocation;
-  CompilerInvocation::CreateFromArgs(
-  *Invocation, CC1Args.data() + 1, CC1Args.data() + CC1Args.size(),
-  *Diagnostics);
+  CompilerInvocation::CreateFromArgs(*Invocation, CC1Args, *Diagnostics);
   Invocation->getFrontendOpts().DisableFree = false;
   Invocation->getCodeGenOpts().DisableFree = false;
   return Invocation;
Index: cfe/trunk/unittests/AST/ExternalASTSourceTest.cpp
===
--- cfe/trunk/unittests/AST/ExternalASTSourceTest.cpp
+++ cfe/trunk/unittests/AST/ExternalASTSourceTest.cpp
@@ -53,7 +53,6 @@
   "test.cc", MemoryBuffer::getMemBuffer(FileContents).release());
   const char *Args[] = { "test.cc" };
   CompilerInvocation::CreateFromArgs(*Invocation, Args,
- Args + array_lengthof(Args),
  Compiler.getDiagnostics());
   Compiler.setInvocation(std::move(Invocation));
 
Index: cfe/trunk/tools/driver/cc1_main.cpp
===
--- cfe/trunk/tools/driver/cc1_main.cpp
+++ cfe/trunk/tools/driver/cc1_main.cpp
@@ -213,8 +213,8 @@
   IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
   TextDiagnosticBuffer *DiagsBuffer = new TextDiagnosticBuffer;
   DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagsBuffer);
-  bool Success = CompilerInvocation::CreateFromArgs(
-  Clang->getInvocation(), Argv.begin(), Argv.end(), Diags);
+  bool Success =
+  CompilerInvocation::CreateFromArgs(Clang->getInvocation(), Argv, Diags);
 
   if (Clang->getFrontendOpts().TimeTrace) {
 llvm::timeTraceProfilerInitialize(
Index: cfe/trunk/tools/arcmt-test/arcmt-test.cpp
===
--- cfe/trunk/tools/arcmt-test/arcmt-test.cpp
+++ cfe/trunk/tools/arcmt-test/arcmt-test.cpp
@@ -121,7 +121,7 @@
   }
 
   CompilerInvocation 

r370122 - ArrayRef'ized CompilerInvocation::CreateFromArgs

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Tue Aug 27 15:13:31 2019
New Revision: 370122

URL: http://llvm.org/viewvc/llvm-project?rev=370122=rev
Log:
ArrayRef'ized CompilerInvocation::CreateFromArgs

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Frontend/CompilerInvocation.h
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
cfe/trunk/lib/Tooling/Tooling.cpp
cfe/trunk/tools/arcmt-test/arcmt-test.cpp
cfe/trunk/tools/clang-import-test/clang-import-test.cpp
cfe/trunk/tools/driver/cc1_main.cpp
cfe/trunk/unittests/AST/ExternalASTSourceTest.cpp

Modified: cfe/trunk/include/clang/Frontend/CompilerInvocation.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInvocation.h?rev=370122=370121=370122=diff
==
--- cfe/trunk/include/clang/Frontend/CompilerInvocation.h (original)
+++ cfe/trunk/include/clang/Frontend/CompilerInvocation.h Tue Aug 27 15:13:31 
2019
@@ -21,6 +21,7 @@
 #include "clang/Frontend/PreprocessorOutputOptions.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/ArrayRef.h"
 #include 
 #include 
 
@@ -153,12 +154,8 @@ public:
   /// one of the vaild-to-access (albeit arbitrary) states.
   ///
   /// \param [out] Res - The resulting invocation.
-  /// \param ArgBegin - The first element in the argument vector.
-  /// \param ArgEnd - The last element in the argument vector.
-  /// \param Diags - The diagnostic engine to use for errors.
   static bool CreateFromArgs(CompilerInvocation ,
- const char* const *ArgBegin,
- const char* const *ArgEnd,
+ ArrayRef CommandLineArgs,
  DiagnosticsEngine );
 
   /// Get the directory where the compiler headers

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=370122=370121=370122=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Tue Aug 27 15:13:31 2019
@@ -3370,8 +3370,7 @@ static void ParseTargetArgs(TargetOption
 }
 
 bool CompilerInvocation::CreateFromArgs(CompilerInvocation ,
-const char *const *ArgBegin,
-const char *const *ArgEnd,
+ArrayRef CommandLineArgs,
 DiagnosticsEngine ) {
   bool Success = true;
 
@@ -3379,9 +3378,8 @@ bool CompilerInvocation::CreateFromArgs(
   std::unique_ptr Opts = createDriverOptTable();
   const unsigned IncludedFlagsBitmask = options::CC1Option;
   unsigned MissingArgIndex, MissingArgCount;
-  InputArgList Args =
-  Opts->ParseArgs(llvm::makeArrayRef(ArgBegin, ArgEnd), MissingArgIndex,
-  MissingArgCount, IncludedFlagsBitmask);
+  InputArgList Args = Opts->ParseArgs(CommandLineArgs, MissingArgIndex,
+  MissingArgCount, IncludedFlagsBitmask);
   LangOptions  = *Res.getLangOpts();
 
   // Check for missing argument error.

Modified: cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp?rev=370122=370121=370122=diff
==
--- cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp (original)
+++ cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp Tue Aug 27 
15:13:31 2019
@@ -90,9 +90,7 @@ std::unique_ptr clan
 
   const ArgStringList  = Cmd.getArguments();
   auto CI = std::make_unique();
-  if (!CompilerInvocation::CreateFromArgs(
-  *CI, const_cast(CCArgs.data()),
-  const_cast(CCArgs.data()) + CCArgs.size(), *Diags) &&
+  if (!CompilerInvocation::CreateFromArgs(*CI, CCArgs, *Diags) &&
   !ShouldRecoverOnErorrs)
 return nullptr;
   return CI;

Modified: cfe/trunk/lib/Tooling/Tooling.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Tooling.cpp?rev=370122=370121=370122=diff
==
--- cfe/trunk/lib/Tooling/Tooling.cpp (original)
+++ cfe/trunk/lib/Tooling/Tooling.cpp Tue Aug 27 15:13:31 2019
@@ -118,9 +118,7 @@ CompilerInvocation *newInvocation(
 DiagnosticsEngine *Diagnostics, const llvm::opt::ArgStringList ) {
   assert(!CC1Args.empty() && "Must at least contain the program name!");
   CompilerInvocation *Invocation = new CompilerInvocation;
-  CompilerInvocation::CreateFromArgs(
-  *Invocation, CC1Args.data() + 1, CC1Args.data() + 

[PATCH] D66834: Driver tests: set `--sysroot` to "" to support toolchains with default sysroot

[Sergej Jaskiewicz via Phabricator via cfe-commits]

broadwaylamb created this revision.
broadwaylamb added reviewers: rsmith, sfantao, sepavloff.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When testing clang that has been compiled with `-DDEFAULT_SYSROOT` set to some 
path,
the test `VFSGCCInstallation` would fail, because the search always starts 
either
in the default sysroot (which doesn't exist in the just-created in-memory VFS), 
or in
`${CLANG_BINARY_DIR}/../lib`, while the virtual GCC installation is in
`${CLANG_BINARY_DIR}/../usr/lib`.

Override sysroot to be empty string for the test to succeed.

(Note that a very similar test `VFSGCCInstallationRelativeDir` was passing 
because
the GCC installation was, in fact, in `${CLANG_BINARY_DIR}/../lib`.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66834

Files:
  clang/unittests/Driver/ToolChainTest.cpp


Index: clang/unittests/Driver/ToolChainTest.cpp
===
--- clang/unittests/Driver/ToolChainTest.cpp
+++ clang/unittests/Driver/ToolChainTest.cpp
@@ -60,7 +60,7 @@
 llvm::MemoryBuffer::getMemBuffer("\n"));
 
   std::unique_ptr C(TheDriver.BuildCompilation(
-  {"-fsyntax-only", "--gcc-toolchain=", "foo.cpp"}));
+  {"-fsyntax-only", "--gcc-toolchain=", "--sysroot=", "foo.cpp"}));
   EXPECT_TRUE(C);
 
   std::string S;


Index: clang/unittests/Driver/ToolChainTest.cpp
===
--- clang/unittests/Driver/ToolChainTest.cpp
+++ clang/unittests/Driver/ToolChainTest.cpp
@@ -60,7 +60,7 @@
 llvm::MemoryBuffer::getMemBuffer("\n"));
 
   std::unique_ptr C(TheDriver.BuildCompilation(
-  {"-fsyntax-only", "--gcc-toolchain=", "foo.cpp"}));
+  {"-fsyntax-only", "--gcc-toolchain=", "--sysroot=", "foo.cpp"}));
   EXPECT_TRUE(C);
 
   std::string S;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

In D64931#1647880 , @dyung wrote:

> Hi, this is causing 3 test failures on the PS4 linux bot. The changes may not 
> have been initially flagged because a different issue was causing a build 
> failure which masked the problem. I have bisected the test failures to this 
> change though.
>
> http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/53916
>
> LLVM :: LTO/Resolution/X86/not-prevailing-weak-aliasee.ll
>  LLVM :: ThinLTO/X86/printer.ll
>  lld :: ELF/lto/drop-debug-info.ll


Actually it appears the llvm tests were fixed up in r370105 leaving only the 
LLD test failing.

> Can you take a look?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64931



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

[PATCH] D65694: Properly instantiate a decltype in argument's default initializer

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

rsmith added a comment.

This isn't the right approach: instead of re-instantiating the parameter when 
we encounter a use of it, we should inject the parameters into the local 
instantiation scope before trying to instantiate a default argument 
(`Sema::CheckCXXDefaultArgExpr` should call `addInstantiatedParametersToScope` 
to populate the `LocalInstantiationScope` before calling `SubstInitializer`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65694



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

[PATCH] D66827: Add support for MS qualifiers __ptr32, __ptr64, __sptr, __uptr.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

This change needs a clang CodeGen test to show that we generate IR with 
`__ptr32` / `__ptr64` in the correct places. We should already have some 
semantic tests, but we may need more.




Comment at: clang/lib/AST/MicrosoftMangle.cpp:1874
+case LangAS::ptr32_sptr:
+  Extra.mangleSourceName("_ASPtr32_sptr");
+  break;

Hm, we should actually mangle these as they do. See the FIXME comment above the 
definition of `PointersAre64Bit`. If you look at the uses of PointersAre64Bit, 
you should be able to find the places where you need to check if a pointer type 
is either in the explicit 64-bit address space or in the default address space 
for a 64-bit target. That check sounds like a good helper function.



Comment at: clang/lib/Sema/SemaOverload.cpp:2874
  N->getUnqualifiedType())) {
+  const PointerType *OldTypePtr =
+  dyn_cast(O->getUnqualifiedType());

This probably deserves a comment. It looks like this lets you do this:
```
void foo(int *__ptr64);
void foo(int *p) { } // assume x64 target
```
... without having the compiler think it's creating an overload.

Separately, MSVC doesn't permit `__ptr32/__ptr64` overloads. Is it possible to 
implement that here as well?



Comment at: llvm/test/CodeGen/X86/mixed-ptr-sizes.ll:11
+;   f->p64 = i;
+;   use_foo(f);
+; }

Do you need use_foo? I think `f` is a parameter, so the compiler can't remove 
any stores to it, even without a call to use it. You should be able to simplify 
the test to skip these calls.



Comment at: llvm/test/CodeGen/X86/mixed-ptr-sizes.ll:30
+;
+; $ clang -cc1 -triple x86_64-windows-msvc -fms-extensions -O2 -S t.cpp
+

If you compile this code as plain C code, then it won't have as much name 
mangling, which makes the .ll file more readable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66827



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

[PATCH] D64931: Change X86 datalayout for three address spaces that specify pointer sizes.

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

Hi, this is causing 3 test failures on the PS4 linux bot. The changes may not 
have been initially flagged because a different issue was causing a build 
failure which masked the problem. I have bisected the test failures to this 
change though.

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/53916

LLVM :: LTO/Resolution/X86/not-prevailing-weak-aliasee.ll
LLVM :: ThinLTO/X86/printer.ll
lld :: ELF/lto/drop-debug-info.ll

Can you take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64931



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

[PATCH] D58497: Clear the KnownModules cache if the preprocessor is going away

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

dexonsmith added a comment.
Herald added a subscriber: wuzish.

Is this still a problem?

I'd be interested to understand *why* the preprocessor is going away here.  Is 
the `CompilerInstance` being reused for two different compilations?  Is that 
what we should fix instead?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58497



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

[PATCH] D66598: [Clang][Bundler] Fix for a hang when unbundling fat binary

[Sergey Dmitriev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370115: [Clang][Bundler] Fix for a hang when unbundling fat 
binary (authored by sdmitriev, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66598?vs=217485=217501#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66598

Files:
  cfe/trunk/test/Driver/clang-offload-bundler.c
  cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp


Index: cfe/trunk/test/Driver/clang-offload-bundler.c
===
--- cfe/trunk/test/Driver/clang-offload-bundler.c
+++ cfe/trunk/test/Driver/clang-offload-bundler.c
@@ -221,6 +221,11 @@
 // RUN: diff %t.empty %t.res.tgt1
 // RUN: diff %t.empty %t.res.tgt2
 
+// Check that we do not have to unbundle all available bundles from the fat 
binary.
+// RUN: clang-offload-bundler -type=ast 
-targets=host-%itanium_abi_triple,openmp-x86_64-pc-linux-gnu 
-outputs=%t.res.ast,%t.res.tgt2 -inputs=%t.bundle3.unordered.ast -unbundle
+// RUN: diff %t.ast %t.res.ast
+// RUN: diff %t.tgt2 %t.res.tgt2
+
 //
 // Check object bundle/unbundle. The content should be bundled into an ELF
 // section (we are using a PowerPC little-endian host which uses ELF). We
Index: cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -215,6 +215,7 @@
 
   /// Iterator for the bundle information that is being read.
   StringMap::iterator CurBundleInfo;
+  StringMap::iterator NextBundleInfo;
 
 public:
   BinaryFileHandler() : FileHandler() {}
@@ -284,19 +285,19 @@
   BundlesInfo[Triple] = BundleInfo(Size, Offset);
 }
 // Set the iterator to where we will start to read.
-CurBundleInfo = BundlesInfo.begin();
+CurBundleInfo = BundlesInfo.end();
+NextBundleInfo = BundlesInfo.begin();
   }
 
   StringRef ReadBundleStart(MemoryBuffer ) final {
-if (CurBundleInfo == BundlesInfo.end())
+if (NextBundleInfo == BundlesInfo.end())
   return StringRef();
-
+CurBundleInfo = NextBundleInfo++;
 return CurBundleInfo->first();
   }
 
   void ReadBundleEnd(MemoryBuffer ) final {
 assert(CurBundleInfo != BundlesInfo.end() && "Invalid reader info!");
-++CurBundleInfo;
   }
 
   void ReadBundle(raw_fd_ostream , MemoryBuffer ) final {


Index: cfe/trunk/test/Driver/clang-offload-bundler.c
===
--- cfe/trunk/test/Driver/clang-offload-bundler.c
+++ cfe/trunk/test/Driver/clang-offload-bundler.c
@@ -221,6 +221,11 @@
 // RUN: diff %t.empty %t.res.tgt1
 // RUN: diff %t.empty %t.res.tgt2
 
+// Check that we do not have to unbundle all available bundles from the fat binary.
+// RUN: clang-offload-bundler -type=ast -targets=host-%itanium_abi_triple,openmp-x86_64-pc-linux-gnu -outputs=%t.res.ast,%t.res.tgt2 -inputs=%t.bundle3.unordered.ast -unbundle
+// RUN: diff %t.ast %t.res.ast
+// RUN: diff %t.tgt2 %t.res.tgt2
+
 //
 // Check object bundle/unbundle. The content should be bundled into an ELF
 // section (we are using a PowerPC little-endian host which uses ELF). We
Index: cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -215,6 +215,7 @@
 
   /// Iterator for the bundle information that is being read.
   StringMap::iterator CurBundleInfo;
+  StringMap::iterator NextBundleInfo;
 
 public:
   BinaryFileHandler() : FileHandler() {}
@@ -284,19 +285,19 @@
   BundlesInfo[Triple] = BundleInfo(Size, Offset);
 }
 // Set the iterator to where we will start to read.
-CurBundleInfo = BundlesInfo.begin();
+CurBundleInfo = BundlesInfo.end();
+NextBundleInfo = BundlesInfo.begin();
   }
 
   StringRef ReadBundleStart(MemoryBuffer ) final {
-if (CurBundleInfo == BundlesInfo.end())
+if (NextBundleInfo == BundlesInfo.end())
   return StringRef();
-
+CurBundleInfo = NextBundleInfo++;
 return CurBundleInfo->first();
   }
 
   void ReadBundleEnd(MemoryBuffer ) final {
 assert(CurBundleInfo != BundlesInfo.end() && "Invalid reader info!");
-++CurBundleInfo;
   }
 
   void ReadBundle(raw_fd_ostream , MemoryBuffer ) final {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

[Volodymyr Sapsai via Phabricator via cfe-commits]

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



Comment at: clang/test/SemaObjC/block-type-safety.m:141
 genericBlock = block;
+block = genericBlock; // expected-error {{incompatible block pointer types 
assigning to 'NSAllArray *(^)(id)' from 'id (^)(id)'}}
+

This is an existing error, just added a test for it.



Comment at: clang/test/SemaObjC/block-type-safety.m:148-149
+void (^genericBlockWithParam)(id);
+genericBlockWithParam = blockWithParam; // expected-error {{incompatible 
block pointer types assigning to 'void (^)(id)' from 'void (^)(NSAllArray 
*)'}}
+blockWithParam = genericBlockWithParam;
 return 0;

And this is a new error. Earlier we would emit an error on `blockWithParam = 
genericBlockWithParam` assignment instead.


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

https://reviews.llvm.org/D66831



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

r370116 - [clang][Index][NFC] Move IndexDataConsumer default implementation

[Jan Korous via cfe-commits]

Author: jkorous
Date: Tue Aug 27 14:49:39 2019
New Revision: 370116

URL: http://llvm.org/viewvc/llvm-project?rev=370116=rev
Log:
[clang][Index][NFC] Move IndexDataConsumer default implementation

Modified:
cfe/trunk/include/clang/Index/IndexDataConsumer.h
cfe/trunk/lib/Index/IndexingAction.cpp

Modified: cfe/trunk/include/clang/Index/IndexDataConsumer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Index/IndexDataConsumer.h?rev=370116=370115=370116=diff
==
--- cfe/trunk/include/clang/Index/IndexDataConsumer.h (original)
+++ cfe/trunk/include/clang/Index/IndexDataConsumer.h Tue Aug 27 14:49:39 2019
@@ -32,7 +32,7 @@ public:
 const DeclContext *ContainerDC;
   };
 
-  virtual ~IndexDataConsumer() {}
+  virtual ~IndexDataConsumer() = default;
 
   virtual void initialize(ASTContext ) {}
 
@@ -41,12 +41,16 @@ public:
   /// \returns true to continue indexing, or false to abort.
   virtual bool handleDeclOccurence(const Decl *D, SymbolRoleSet Roles,
ArrayRef Relations,
-   SourceLocation Loc, ASTNodeInfo ASTNode);
+   SourceLocation Loc, ASTNodeInfo ASTNode) {
+return true;
+  }
 
   /// \returns true to continue indexing, or false to abort.
   virtual bool handleMacroOccurence(const IdentifierInfo *Name,
 const MacroInfo *MI, SymbolRoleSet Roles,
-SourceLocation Loc);
+SourceLocation Loc) {
+return true;
+  }
 
   /// \returns true to continue indexing, or false to abort.
   ///
@@ -54,8 +58,10 @@ public:
   /// For "@import MyMod.SubMod", there will be a call for 'MyMod' with the
   /// 'reference' role, and a call for 'SubMod' with the 'declaration' role.
   virtual bool handleModuleOccurence(const ImportDecl *ImportD,
- const Module *Mod,
- SymbolRoleSet Roles, SourceLocation Loc);
+ const Module *Mod, SymbolRoleSet Roles,
+ SourceLocation Loc) {
+return true;
+  }
 
   virtual void finish() {}
 };

Modified: cfe/trunk/lib/Index/IndexingAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/IndexingAction.cpp?rev=370116=370115=370116=diff
==
--- cfe/trunk/lib/Index/IndexingAction.cpp (original)
+++ cfe/trunk/lib/Index/IndexingAction.cpp Tue Aug 27 14:49:39 2019
@@ -21,27 +21,6 @@
 using namespace clang;
 using namespace clang::index;
 
-bool IndexDataConsumer::handleDeclOccurence(const Decl *D, SymbolRoleSet Roles,
-ArrayRef Relations,
-SourceLocation Loc,
-ASTNodeInfo ASTNode) {
-  return true;
-}
-
-bool IndexDataConsumer::handleMacroOccurence(const IdentifierInfo *Name,
- const MacroInfo *MI,
- SymbolRoleSet Roles,
- SourceLocation Loc) {
-  return true;
-}
-
-bool IndexDataConsumer::handleModuleOccurence(const ImportDecl *ImportD,
-  const Module *Mod,
-  SymbolRoleSet Roles,
-  SourceLocation Loc) {
-  return true;
-}
-
 namespace {
 
 class IndexASTConsumer : public ASTConsumer {


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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:150
+// Operand 0 is a string tag "branch_weights"
+if (MDString *Tag = cast(MD->getOperand(0))) {
+  unsigned NOps = MD->getNumOperands();

paulkirth wrote:
> nickdesaulniers wrote:
> > Sorry if I'm going back and forth on this, but it may make sense to check 
> > the number of operands first, before accessing any.
> No problem. I've reintroduced the check. I was under the impression that 
> metadata could not lack a 0th element, but even in that case I should have 
> checked for additional elements. 
> 
> There should always be at least 2 branch weights, otherwise branch weights 
> are not necessary, and we should never end up here.
> 
> Should I add a comment saying that?
Sorry, I meant "check the lower bound of the operands first, ..." as `NOps` 
might be `0` which would cause `MD->getOperand(0)` to fail, and `RealWeights` 
to be size `-1`.  `if (Nops < 1 || Nops > 3) return;`

I think a comment would be helpful, too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66324



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

r370115 - [Clang][Bundler] Fix for a hang when unbundling fat binary

[Sergey Dmitriev via cfe-commits]

Author: sdmitriev
Date: Tue Aug 27 14:47:52 2019
New Revision: 370115

URL: http://llvm.org/viewvc/llvm-project?rev=370115=rev
Log:
[Clang][Bundler] Fix for a hang when unbundling fat binary

clang-offload-bundler tool may hang under certain conditions when it extracts a 
subset of all available device bundles from the fat binary that is handled by 
the BinaryFileHandler. This patch fixes this problem.

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

Modified:
cfe/trunk/test/Driver/clang-offload-bundler.c
cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Modified: cfe/trunk/test/Driver/clang-offload-bundler.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/clang-offload-bundler.c?rev=370115=370114=370115=diff
==
--- cfe/trunk/test/Driver/clang-offload-bundler.c (original)
+++ cfe/trunk/test/Driver/clang-offload-bundler.c Tue Aug 27 14:47:52 2019
@@ -221,6 +221,11 @@
 // RUN: diff %t.empty %t.res.tgt1
 // RUN: diff %t.empty %t.res.tgt2
 
+// Check that we do not have to unbundle all available bundles from the fat 
binary.
+// RUN: clang-offload-bundler -type=ast 
-targets=host-%itanium_abi_triple,openmp-x86_64-pc-linux-gnu 
-outputs=%t.res.ast,%t.res.tgt2 -inputs=%t.bundle3.unordered.ast -unbundle
+// RUN: diff %t.ast %t.res.ast
+// RUN: diff %t.tgt2 %t.res.tgt2
+
 //
 // Check object bundle/unbundle. The content should be bundled into an ELF
 // section (we are using a PowerPC little-endian host which uses ELF). We

Modified: cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp?rev=370115=370114=370115=diff
==
--- cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp (original)
+++ cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp Tue Aug 27 
14:47:52 2019
@@ -215,6 +215,7 @@ class BinaryFileHandler final : public F
 
   /// Iterator for the bundle information that is being read.
   StringMap::iterator CurBundleInfo;
+  StringMap::iterator NextBundleInfo;
 
 public:
   BinaryFileHandler() : FileHandler() {}
@@ -284,19 +285,19 @@ public:
   BundlesInfo[Triple] = BundleInfo(Size, Offset);
 }
 // Set the iterator to where we will start to read.
-CurBundleInfo = BundlesInfo.begin();
+CurBundleInfo = BundlesInfo.end();
+NextBundleInfo = BundlesInfo.begin();
   }
 
   StringRef ReadBundleStart(MemoryBuffer ) final {
-if (CurBundleInfo == BundlesInfo.end())
+if (NextBundleInfo == BundlesInfo.end())
   return StringRef();
-
+CurBundleInfo = NextBundleInfo++;
 return CurBundleInfo->first();
   }
 
   void ReadBundleEnd(MemoryBuffer ) final {
 assert(CurBundleInfo != BundlesInfo.end() && "Invalid reader info!");
-++CurBundleInfo;
   }
 
   void ReadBundle(raw_fd_ostream , MemoryBuffer ) final {


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

[PATCH] D66831: [ObjC] Fix type checking for qualified id block parameters.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
vsapsai added reviewers: erik.pilkington, arphaman.
Herald added subscribers: ributzka, dexonsmith, jkorous.

When checking if block types are compatible, we are checking for
compatibility their return types and parameters' types. As these types
have different variance, we need to check them in different order.

rdar://problem/52788423


https://reviews.llvm.org/D66831

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/SemaObjC/block-type-safety.m


Index: clang/test/SemaObjC/block-type-safety.m
===
--- clang/test/SemaObjC/block-type-safety.m
+++ clang/test/SemaObjC/block-type-safety.m
@@ -133,9 +133,20 @@
 @end
 
 int test5() {
+// Returned value is used outside of a block, so error on changing
+// a return type to a more general than expected.
 NSAllArray *(^block)(id);
 id  (^genericBlock)(id);
 genericBlock = block;
+block = genericBlock; // expected-error {{incompatible block pointer types 
assigning to 'NSAllArray *(^)(id)' from 'id (^)(id)'}}
+
+// A parameter is used inside a block, so error on changing a parameter 
type
+// to a more specific than an argument type it will be called with.
+// rdar://problem/52788423
+void (^blockWithParam)(NSAllArray *);
+void (^genericBlockWithParam)(id);
+genericBlockWithParam = blockWithParam; // expected-error {{incompatible 
block pointer types assigning to 'void (^)(id)' from 'void (^)(NSAllArray 
*)'}}
+blockWithParam = genericBlockWithParam;
 return 0;
 }
 
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -8191,9 +8191,9 @@
   }
 
   if (LHSOPT->isObjCQualifiedIdType() || RHSOPT->isObjCQualifiedIdType())
-return finish(ObjCQualifiedIdTypesAreCompatible(QualType(LHSOPT,0),
-QualType(RHSOPT,0),
-false));
+return finish(ObjCQualifiedIdTypesAreCompatible(
+QualType(BlockReturnType ? LHSOPT : RHSOPT, 0),
+QualType(BlockReturnType ? RHSOPT : LHSOPT, 0), false));
 
   const ObjCInterfaceType* LHS = LHSOPT->getInterfaceType();
   const ObjCInterfaceType* RHS = RHSOPT->getInterfaceType();


Index: clang/test/SemaObjC/block-type-safety.m
===
--- clang/test/SemaObjC/block-type-safety.m
+++ clang/test/SemaObjC/block-type-safety.m
@@ -133,9 +133,20 @@
 @end
 
 int test5() {
+// Returned value is used outside of a block, so error on changing
+// a return type to a more general than expected.
 NSAllArray *(^block)(id);
 id  (^genericBlock)(id);
 genericBlock = block;
+block = genericBlock; // expected-error {{incompatible block pointer types assigning to 'NSAllArray *(^)(id)' from 'id (^)(id)'}}
+
+// A parameter is used inside a block, so error on changing a parameter type
+// to a more specific than an argument type it will be called with.
+// rdar://problem/52788423
+void (^blockWithParam)(NSAllArray *);
+void (^genericBlockWithParam)(id);
+genericBlockWithParam = blockWithParam; // expected-error {{incompatible block pointer types assigning to 'void (^)(id)' from 'void (^)(NSAllArray *)'}}
+blockWithParam = genericBlockWithParam;
 return 0;
 }
 
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -8191,9 +8191,9 @@
   }
 
   if (LHSOPT->isObjCQualifiedIdType() || RHSOPT->isObjCQualifiedIdType())
-return finish(ObjCQualifiedIdTypesAreCompatible(QualType(LHSOPT,0),
-QualType(RHSOPT,0),
-false));
+return finish(ObjCQualifiedIdTypesAreCompatible(
+QualType(BlockReturnType ? LHSOPT : RHSOPT, 0),
+QualType(BlockReturnType ? RHSOPT : LHSOPT, 0), false));
 
   const ObjCInterfaceType* LHS = LHSOPT->getInterfaceType();
   const ObjCInterfaceType* RHS = RHSOPT->getInterfaceType();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

[Paul Kirth via Phabricator via cfe-commits]

paulkirth abandoned this revision.
paulkirth added a comment.

This is being abandoned in favor of D66324 , 
which reimplements this logic completely in the LLVM backend.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65300



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

[PATCH] D65694: Properly instantiate a decltype in argument's default initializer

[Nicholas Allegra via Phabricator via cfe-commits]

comex added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65694



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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Paul Kirth via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:150
+// Operand 0 is a string tag "branch_weights"
+if (MDString *Tag = cast(MD->getOperand(0))) {
+  unsigned NOps = MD->getNumOperands();

nickdesaulniers wrote:
> Sorry if I'm going back and forth on this, but it may make sense to check the 
> number of operands first, before accessing any.
No problem. I've reintroduced the check. I was under the impression that 
metadata could not lack a 0th element, but even in that case I should have 
checked for additional elements. 

There should always be at least 2 branch weights, otherwise branch weights are 
not necessary, and we should never end up here.

Should I add a comment saying that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66324



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

[PATCH] D66830: Consumed checker: various improvements

[Nicholas Allegra via Phabricator via cfe-commits]

comex created this revision.
comex added reviewers: delesley, dblaikie, rsmith.
Herald added subscribers: cfe-commits, kristof.beyls, javed.absar.
Herald added a project: clang.

- Treat arguments to constructor calls the same way as arguments to other calls 
(fixes 42856).  Previously, arguments to constructors were completely ignored – 
even if explicitly marked `param_typestate` or `return_typestate` – except for 
one special case to mark the argument to a move constructor as consumed on 
return.  Even that special case was broken, as it wouldn't apply if a move 
constructor happened to be marked with `return_typestate`.
  - Constructors are still special-cased when determining the default typestate 
for the newly constructed object, unless the constructor is marked 
`return_typestate`.

- Stop ignoring `return_typestate` on rvalue reference parameters.

- When a function takes a consumable object by value, treat that as consuming 
by default, same as with an rvalue reference.

  Note that the object being consumed is always a temporary.  If you write:

  `void a(X); a((X &&)x);`

  ...this first constructs a temporary X by moving from `x`, then calls `a` 
with a pointer to the temporary.  Before and after this change, the move copies 
`x`'s typestate to the temporary and marks `x` itself as consumed.  But before 
this change, if the temporary started out unconsumed (because `x` was 
unconsumed before the move), it would still be unconsumed when its destructor 
was run after the call.  Now it will be considered consumed at that point.

  Note that currently, only parameters explicitly marked `return_typestate` 
have their state-on-return checked on the callee side.  Parameters which are 
implicitly consuming due to being rvalue references – or, after this patch, 
by-value – are checked only on the caller side.  This discrepancy was very 
surprising to me as a user.  I wrote a fix, but in my codebase it broke some 
code that was using `std::forward`.  Therefore, I plan to hold off on 
submitting the fix until a followup patch, which will generalize the current 
`std::move` special case into an attribute that can be applied to any 'cast' 
function like `std::move` and `std::forward`.

- Diagnose if a constructor or a static method is marked `set_typestate`, 
instead of (respectively) ignoring it or crashing.

- Diagnose if a `param_typestate` or `return_typestate` attribute is attached 
to a parameter of non-consumable type.  The wording I came up with is a bit 
fuzzy; suggestions welcome.

- Fix broken check that was trying to `dyn_cast` a `CXXOperatorCallExpr` to a 
sibling class, `CXXMemberCallExpr` (that will never work).  Instead, we need to 
check whether the declaration is a `CXXMethodDecl`.
  - This was harmless in the existing code, for somewhat complicated reasons, 
but caused problems after I refactored `handleCall` to take the arguments as a 
separate parameter.

Some of these changes have the potential to 'break' code (in the sense of 
creating warnings where there were none before) because they check things that 
were previously ignored.

One notable case, which showed up in the existing test code 
(warn-consumed-analysis.cpp), is a copy constructor that takes its argument by 
non-const reference.  Previously, the argument would be ignored and thus 
implicitly treated as leaving the state alone – just like all other constructor 
arguments, except for the argument to a move constructor.  Now it's given the 
usual treatment for non-const reference parameters, which is to mark the 
original object as being in `unknown` state, unless overridden with 
`return_typestate`.  I think the new behavior is more correct, and this isn't 
removing a special case *per se*.  However, since it is a "copy" constructor, 
there might be some argument that the argument should be treated as 
non-consuming instead.

Also, if anyone wants to test this on their code, note that I have a separate 
patch pending to make the CFG more accurate with respect to temporary object 
destructors (D66404 ).


Repository:
  rC Clang

https://reviews.llvm.org/D66830

Files:
  include/clang/Analysis/Analyses/Consumed.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Analysis/Consumed.cpp
  lib/Sema/AnalysisBasedWarnings.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/SemaCXX/warn-consumed-analysis.cpp
  test/SemaCXX/warn-consumed-parsing.cpp

Index: test/SemaCXX/warn-consumed-parsing.cpp
===
--- test/SemaCXX/warn-consumed-parsing.cpp
+++ test/SemaCXX/warn-consumed-parsing.cpp
@@ -62,5 +62,12 @@
   Status {
 };
 
-
+struct CONSUMABLE(unknown) UselessAttrs {
+  static void x() SET_TYPESTATE(consumed); // expected-warning {{consumed analysis attribute is attached to a static method of class 'UselessAttrs'}}
+  UselessAttrs() SET_TYPESTATE(consumed); // expected-warning {{consumed analysis attribute is attached to a constructor of class 

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 217494.
paulkirth added a comment.

Change check for minimum number of metadata operands.

Branch weights must have at least 3 elements:  metadata name, and a list of at 
least 2 branch weights


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66324

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/FixedMetadataKinds.def
  llvm/include/llvm/IR/MDBuilder.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/ThinLTO/X86/lazyload_metadata.ll
  llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,293 @@
+
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; New PM
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.c.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=CORRECT
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:5: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:5: Potential performance regression from use of __builtin_expect(): Annotation was correct on 0.00% of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:5: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:5: Potential performance regression from use of __builtin_expect(): Annotation was correct on 0.00% of profiled executions.
+
+; BOTH-DAG: warning: misexpect-switch.c:26:5: 0.00%
+; BOTH-DAG: remark: misexpect-switch.c:26:5: Potential performance regression from use of __builtin_expect(): 

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 217491.
paulkirth marked 2 inline comments as done.
paulkirth added a comment.

Remove namespace qualifier & insert check for number of arguments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66324

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/FixedMetadataKinds.def
  llvm/include/llvm/IR/MDBuilder.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/ThinLTO/X86/lazyload_metadata.ll
  llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,293 @@
+
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; New PM
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.c.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=CORRECT
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:5: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:5: Potential performance regression from use of __builtin_expect(): Annotation was correct on 0.00% of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:5: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:5: Potential performance regression from use of __builtin_expect(): Annotation was correct on 0.00% of profiled executions.
+
+; BOTH-DAG: warning: misexpect-switch.c:26:5: 0.00%
+; BOTH-DAG: remark: misexpect-switch.c:26:5: Potential performance regression from use of __builtin_expect(): Annotation was correct on 0.00% of profiled 

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

[Macide Celik via Phabricator via cfe-commits]

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

In D66564#1640584 , @Eugene.Zelenko 
wrote:

> Please mention new module and check in Release Notes.





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

https://reviews.llvm.org/D66564



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

[PATCH] D66598: [Clang][Bundler] Fix for a hang when unbundling fat binary

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


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

https://reviews.llvm.org/D66598



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

[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D65300#1647775 , @paulkirth wrote:

> In D65300#1647746 , @lebedev.ri 
> wrote:
>
> > Please can you clarify hat's the current layout of these patches?
> >  Is this patch required, or is it superseded by D66324 
> >  (and thus should be abandoned)? 
> >  I'd like to begin reviewing, but i don't understand where to start.
>
>
> Sorry for the confusion. D66324  should 
> supersede this patch.


Okay, i'll be reviewing that one then.

> It re-implements everything in the backend, and adds support for IR & Sample 
> based profiles, so I think it is safe to ignore this patch for now.



> Is there something I should do to mark/change the patches?

//Please// abandon this patch to make it obvious (Add Action -> Abandon)

> Also, thanks for the feedback.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65300



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

[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

In D65300#1647746 , @lebedev.ri wrote:

> Please can you clarify hat's the current layout of these patches?
>  Is this patch required, or is it superseded by D66324 
>  (and thus should be abandoned)? 
>  I'd like to begin reviewing, but i don't understand where to start.


Sorry for the confusion. D66324  should 
supersede this patch. It re-implements everything in the backend, and adds 
support for IR & Sample based profiles, so I think it is safe to ignore this 
patch for now.

Is there something I should do to mark/change the patches?

Also, thanks for the feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65300



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

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

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

dexonsmith added a comment.

In D66556#1647762 , @dexonsmith wrote:

> In D66556#1646229 , @aganea wrote:
>
> > This failed the build - I've changed `unix2dos` to `svn:eol-style CRLF` 
> > instead.
>
>
> Can we count on `tr` when we have `shell`?  If so, I suggest this instead:
>
>   // REQUIRES: shell
>   // RUN: tr '\n' '\r\n' <%s | %clang_cc1 -DOTHER 
> -print-dependency-directives-minimized-source 2>&1 | FileCheck %s
>


Eh, nevermind, `tr` doesn't handle multi-character replacements.  Better to 
answer Alex's question.


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

https://reviews.llvm.org/D66556



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

[PATCH] D66716: [analyzer] PR43102: Fix an assertion and an out-of-bounds error for diagnostic location construction

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

> Do you think its ever okay to find a last store in a `BlockEdge`?

*In a `BlockEntrance` into an empty block (that has no elements but does have a 
terminator). At least that's what your code is fixing.

Attaching store notes to such program points is most likely not ok. We can 
always make ourselves a `WideningPoint` for that specific purpose, or maybe 
even `PreWidening` / `PostWidening`. Program points are not set in stone, we 
are free to make as many kinds of them as we need (which is why we also have 
tags).

Being able to attach an event note at all to such program point is most likely 
ok. Any program point can potentially represent an interesting event. So this 
is anyway a good change. I'd love to have some more careful testing, maybe a 
unittest (or somehow trick -verify with line breaks), so that to know where 
exactly does the note go in this case (is it the jump from the bottom of the 
loop? is it jump from increment to condition?). This way we'll make sure that 
the implementation is correct.

I'd still want you to figure out why is this widening-specific. Do i understand 
correctly that we're doing widening in an inappropriate moment of time? I'd 
prefer to have this confirmed. Or can we reproduce this crash / incorrect 
behavior / false positive without widening?




Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1805
 // the only thing that leads to is the addition of calls to operator!=.
-if (isa(NB->getTerminator()))
   return nullptr;

Why did this even compile? I thought i deleted these conversions in D61814 >.<



Comment at: clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp:774-775
+
+return PathDiagnosticLocation(
+BE->getBlock()->getTerminatorStmt()->getBeginLoc(), SMng);
   } else if (Optional FE = P.getAs()) {

What exactly is the terminator here in your case? Is it `NullStmt`? Is there 
always a terminator and/or a terminator statement?

What if that's a `BlockEntrance` to the exit-block? (do we even make such 
program points? i hope we don't)

I think this code needs comments in order to explain what picture do we have in 
mind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66716



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

[PATCH] D66556: [clang-scan-deps] Minimizer: Correctly handle multi-line content with CR+LF line endings

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

dexonsmith added a comment.

In D66556#1646229 , @aganea wrote:

> This failed the build - I've changed `unix2dos` to `svn:eol-style CRLF` 
> instead.


Can we count on `tr` when we have `shell`?  If so, I suggest this instead:

  // REQUIRES: shell
  // RUN: tr '\n' '\r\n' <%s | %clang_cc1 -DOTHER 
-print-dependency-directives-minimized-source 2>&1 | FileCheck %s


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

https://reviews.llvm.org/D66556



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

[PATCH] D66598: [Clang][Bundler] Fix for a hang when unbundling fat binary

[Sergey Dmitriev via Phabricator via cfe-commits]

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



Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:295
   return StringRef();
-
+CurBundleInfo = NextBundleInfo++;
 return CurBundleInfo->first();

ABataev wrote:
> Maybe just:
> ```
> const BundleInfo  = *CurBundleInfo;
> std::advance(CurBundleInfo, 1);
> return Bundle.first();
> ```
> instead of all these changes?
It could be done like this if CurBundleInfo was not used in ReadBundle.


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

https://reviews.llvm.org/D66598



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

[PATCH] D66828: [clangd] Add distinct highlightings for static fields and methods

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
nridge added reviewers: hokein, ilya-biryukov, jvikstrom.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66828

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -35,6 +35,7 @@
 std::vector getExpectedTokens(Annotations ) {
   static const std::map KindToString{
   {HighlightingKind::Variable, "Variable"},
+  {HighlightingKind::LocalVariable, "LocalVariable"},
   {HighlightingKind::Parameter, "Parameter"},
   {HighlightingKind::Function, "Function"},
   {HighlightingKind::Class, "Class"},
@@ -42,10 +43,11 @@
   {HighlightingKind::Namespace, "Namespace"},
   {HighlightingKind::EnumConstant, "EnumConstant"},
   {HighlightingKind::Field, "Field"},
+  {HighlightingKind::StaticField, "StaticField"},
   {HighlightingKind::Method, "Method"},
+  {HighlightingKind::StaticMethod, "StaticMethod"},
   {HighlightingKind::TemplateParameter, "TemplateParameter"},
-  {HighlightingKind::Primitive, "Primitive"},
-  {HighlightingKind::LocalVariable, "LocalVariable"}};
+  {HighlightingKind::Primitive, "Primitive"}};
   std::vector ExpectedTokens;
   for (const auto  : KindToString) {
 std::vector Toks = makeHighlightingTokens(
@@ -200,13 +202,15 @@
   struct $Class[[A]] {
 $Primitive[[double]] $Field[[B]];
 $Class[[D]] $Field[[E]];
-static $Primitive[[double]] $Variable[[S]];
+static $Primitive[[double]] $StaticField[[S]];
+static $Primitive[[void]] $StaticMethod[[bar]]() {}
 $Primitive[[void]] $Method[[foo]]() {
   $Field[[B]] = 123;
   this->$Field[[B]] = 156;
   this->$Method[[foo]]();
   $Method[[foo]]();
-  $Variable[[S]] = 90.1;
+  $StaticMethod[[bar]]();
+  $StaticField[[S]] = 90.1;
 }
   };
   $Primitive[[void]] $Function[[foo]]() {
@@ -214,7 +218,7 @@
 $LocalVariable[[AA]].$Field[[B]] += 2;
 $LocalVariable[[AA]].$Method[[foo]]();
 $LocalVariable[[AA]].$Field[[E]].$Field[[C]];
-$Class[[A]]::$Variable[[S]] = 90;
+$Class[[A]]::$StaticField[[S]] = 90;
   }
 )cpp",
   R"cpp(
@@ -629,4 +633,4 @@
 
 } // namespace
 } // namespace clangd
-} // namespace clang
+} // namespace clang
\ No newline at end of file
Index: clang-tools-extra/clangd/test/semantic-highlighting.test
===
--- clang-tools-extra/clangd/test/semantic-highlighting.test
+++ clang-tools-extra/clangd/test/semantic-highlighting.test
@@ -20,9 +20,15 @@
 # CHECK-NEXT:"entity.name.function.method.cpp"
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
+# CHECK-NEXT:"entity.name.function.method.static.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
 # CHECK-NEXT:"variable.other.field.cpp"
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
+# CHECK-NEXT:"variable.other.field.static.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
 # CHECK-NEXT:"entity.name.type.class.cpp"
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
@@ -49,7 +55,7 @@
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
 # CHECK-NEXT:"line": 0,
-# CHECK-NEXT:"tokens": "AAADAAsEAAEAAA=="
+# CHECK-NEXT:"tokens": "AAADAA0EAAEAAA=="
 # CHECK-NEXT:  }
 # CHECK-NEXT:],
 # CHECK-NEXT:"textDocument": {
@@ -64,11 +70,11 @@
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
 # CHECK-NEXT:"line": 0,
-# CHECK-NEXT:"tokens": "AAADAAsEAAEAAA=="
+# CHECK-NEXT:"tokens": "AAADAA0EAAEAAA=="
 # CHECK-NEXT:  }
 # CHECK-NEXT:  {
 # CHECK-NEXT:"line": 1,
-# CHECK-NEXT:"tokens": "AAADAAsEAAEAAA=="
+# CHECK-NEXT:"tokens": "AAADAA0EAAEAAA=="
 # CHECK-NEXT:  }
 # CHECK-NEXT:],
 # CHECK-NEXT:"textDocument": {
@@ -83,7 +89,7 @@
 # CHECK-NEXT:"lines": [
 # CHECK-NEXT:  {
 # CHECK-NEXT:"line": 1,
-# CHECK-NEXT:"tokens": "AAADAAsEAAEAAA=="
+# CHECK-NEXT:"tokens": "AAADAA0EAAEAAA=="
 # CHECK-NEXT:  }
 # CHECK-NEXT:   ],
 # CHECK-NEXT:"textDocument": {
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ 

[PATCH] D66598: [Clang][Bundler] Fix for a hang when unbundling fat binary

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:295
   return StringRef();
-
+CurBundleInfo = NextBundleInfo++;
 return CurBundleInfo->first();

Maybe just:
```
const BundleInfo  = *CurBundleInfo;
std::advance(CurBundleInfo, 1);
return Bundle.first();
```
instead of all these changes?


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

https://reviews.llvm.org/D66598



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

[PATCH] D65300: [clang] [CodeGen] clang-misexpect prototype for compiler warnings

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Please can you clarify hat's the current layout of these patches?
Is this patch required, or is it superseded by D66324 
 (and thus should be abandoned)? 
I'd like to begin reviewing, but i don't understand where to start.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65300



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

[PATCH] D66598: [Clang][Bundler] Fix for a hang when unbundling fat binary

[Sergey Dmitriev via Phabricator via cfe-commits]

sdmitriev updated this revision to Diff 217485.

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

https://reviews.llvm.org/D66598

Files:
  clang/test/Driver/clang-offload-bundler.c
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -215,6 +215,7 @@
 
   /// Iterator for the bundle information that is being read.
   StringMap::iterator CurBundleInfo;
+  StringMap::iterator NextBundleInfo;
 
 public:
   BinaryFileHandler() : FileHandler() {}
@@ -284,19 +285,19 @@
   BundlesInfo[Triple] = BundleInfo(Size, Offset);
 }
 // Set the iterator to where we will start to read.
-CurBundleInfo = BundlesInfo.begin();
+CurBundleInfo = BundlesInfo.end();
+NextBundleInfo = BundlesInfo.begin();
   }
 
   StringRef ReadBundleStart(MemoryBuffer ) final {
-if (CurBundleInfo == BundlesInfo.end())
+if (NextBundleInfo == BundlesInfo.end())
   return StringRef();
-
+CurBundleInfo = NextBundleInfo++;
 return CurBundleInfo->first();
   }
 
   void ReadBundleEnd(MemoryBuffer ) final {
 assert(CurBundleInfo != BundlesInfo.end() && "Invalid reader info!");
-++CurBundleInfo;
   }
 
   void ReadBundle(raw_fd_ostream , MemoryBuffer ) final {
Index: clang/test/Driver/clang-offload-bundler.c
===
--- clang/test/Driver/clang-offload-bundler.c
+++ clang/test/Driver/clang-offload-bundler.c
@@ -221,6 +221,11 @@
 // RUN: diff %t.empty %t.res.tgt1
 // RUN: diff %t.empty %t.res.tgt2
 
+// Check that we do not have to unbundle all available bundles from the fat 
binary.
+// RUN: clang-offload-bundler -type=ast 
-targets=host-%itanium_abi_triple,openmp-x86_64-pc-linux-gnu 
-outputs=%t.res.ast,%t.res.tgt2 -inputs=%t.bundle3.unordered.ast -unbundle
+// RUN: diff %t.ast %t.res.ast
+// RUN: diff %t.tgt2 %t.res.tgt2
+
 //
 // Check object bundle/unbundle. The content should be bundled into an ELF
 // section (we are using a PowerPC little-endian host which uses ELF). We


Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -215,6 +215,7 @@
 
   /// Iterator for the bundle information that is being read.
   StringMap::iterator CurBundleInfo;
+  StringMap::iterator NextBundleInfo;
 
 public:
   BinaryFileHandler() : FileHandler() {}
@@ -284,19 +285,19 @@
   BundlesInfo[Triple] = BundleInfo(Size, Offset);
 }
 // Set the iterator to where we will start to read.
-CurBundleInfo = BundlesInfo.begin();
+CurBundleInfo = BundlesInfo.end();
+NextBundleInfo = BundlesInfo.begin();
   }
 
   StringRef ReadBundleStart(MemoryBuffer ) final {
-if (CurBundleInfo == BundlesInfo.end())
+if (NextBundleInfo == BundlesInfo.end())
   return StringRef();
-
+CurBundleInfo = NextBundleInfo++;
 return CurBundleInfo->first();
   }
 
   void ReadBundleEnd(MemoryBuffer ) final {
 assert(CurBundleInfo != BundlesInfo.end() && "Invalid reader info!");
-++CurBundleInfo;
   }
 
   void ReadBundle(raw_fd_ostream , MemoryBuffer ) final {
Index: clang/test/Driver/clang-offload-bundler.c
===
--- clang/test/Driver/clang-offload-bundler.c
+++ clang/test/Driver/clang-offload-bundler.c
@@ -221,6 +221,11 @@
 // RUN: diff %t.empty %t.res.tgt1
 // RUN: diff %t.empty %t.res.tgt2
 
+// Check that we do not have to unbundle all available bundles from the fat binary.
+// RUN: clang-offload-bundler -type=ast -targets=host-%itanium_abi_triple,openmp-x86_64-pc-linux-gnu -outputs=%t.res.ast,%t.res.tgt2 -inputs=%t.bundle3.unordered.ast -unbundle
+// RUN: diff %t.ast %t.res.ast
+// RUN: diff %t.tgt2 %t.res.tgt2
+
 //
 // Check object bundle/unbundle. The content should be bundled into an ELF
 // section (we are using a PowerPC little-endian host which uses ELF). We
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:150
+// Operand 0 is a string tag "branch_weights"
+if (MDString *Tag = cast(MD->getOperand(0))) {
+  unsigned NOps = MD->getNumOperands();

Sorry if I'm going back and forth on this, but it may make sense to check the 
number of operands first, before accessing any.



Comment at: llvm/lib/Transforms/Utils/MisExpect.cpp:159
+}
+misexpect::verifyMisExpect(, RealWeights, I.getContext());
+  }

do you need the `misexpect::` qualifier here? I think your `using` statement 
above should make this one unnecessary?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66324



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

r370108 - Diagnose _Bool as a C99 extension.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Aug 27 13:33:05 2019
New Revision: 370108

URL: http://llvm.org/viewvc/llvm-project?rev=370108=rev
Log:
Diagnose _Bool as a C99 extension.

Modified:
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/test/Parser/c99.c

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=370108=370107=370108=diff
==
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Tue Aug 27 13:33:05 2019
@@ -3777,6 +3777,9 @@ void Parser::ParseDeclarationSpecifiers(
   break;
 case tok::kw_bool:
 case tok::kw__Bool:
+  if (Tok.is(tok::kw__Bool) && !getLangOpts().C99)
+Diag(Tok, diag::ext_c99_feature) << Tok.getName();
+
   if (Tok.is(tok::kw_bool) &&
   DS.getTypeSpecType() != DeclSpec::TST_unspecified &&
   DS.getStorageClassSpec() == DeclSpec::SCS_typedef) {

Modified: cfe/trunk/test/Parser/c99.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/c99.c?rev=370108=370107=370108=diff
==
--- cfe/trunk/test/Parser/c99.c (original)
+++ cfe/trunk/test/Parser/c99.c Tue Aug 27 13:33:05 2019
@@ -6,3 +6,6 @@ double _Imaginary foo; // ext-warning {{
// expected-error {{imaginary types are not supported}}
 double _Complex bar; // ext-warning {{'_Complex' is a C99 extension}}
 
+#if !defined(__cplusplus)
+_Bool baz; // ext-warning {{'_Bool' is a C99 extension}}
+#endif


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

[PATCH] D66667: Debug Info: Support for DW_AT_export_symbols for anonymous structs

[Shafik Yaghmour via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG528f5da6d862: Debug Info: Support for DW_AT_export_symbols 
for anonymous structs (authored by shafik).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D7?vs=217457=217482#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D7

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/debug-info-export_symbols.cpp


Index: clang/test/CodeGenCXX/debug-info-export_symbols.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-export_symbols.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone -triple 
%itanium_abi_triple %s -o - | FileCheck %s
+
+// CHECK: [[SCOPE:![0-9]+]] = distinct !DICompositeType({{.*}}flags: 
DIFlagTypePassByValue
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, scope: [[SCOPE]]
+// CHECK-SAME:  DIFlagExportSymbols | 
DIFlagTypePassByValue
+struct A {
+ // Anonymous class exports its symbols into A
+ struct {
+ int y;
+ };
+} a;
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3121,7 +3121,8 @@
 
   SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
 
-  // Explicitly record the calling convention for C++ records.
+  // Explicitly record the calling convention and export symbols for C++
+  // records.
   auto Flags = llvm::DINode::FlagZero;
   if (auto CXXRD = dyn_cast(RD)) {
 if (CGM.getCXXABI().getRecordArgABI(CXXRD) == CGCXXABI::RAA_Indirect)
@@ -3132,6 +3133,10 @@
 // Record if a C++ record is non-trivial type.
 if (!CXXRD->isTrivial())
   Flags |= llvm::DINode::FlagNonTrivial;
+
+// Record exports it symbols to the containing structure.
+if (CXXRD->isAnonymousStructOrUnion())
+Flags |= llvm::DINode::FlagExportSymbols;
   }
 
   llvm::DICompositeType *RealDecl = DBuilder.createReplaceableCompositeType(


Index: clang/test/CodeGenCXX/debug-info-export_symbols.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-export_symbols.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone -triple %itanium_abi_triple %s -o - | FileCheck %s
+
+// CHECK: [[SCOPE:![0-9]+]] = distinct !DICompositeType({{.*}}flags: DIFlagTypePassByValue
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, scope: [[SCOPE]]
+// CHECK-SAME:  DIFlagExportSymbols | DIFlagTypePassByValue
+struct A {
+ // Anonymous class exports its symbols into A
+ struct {
+ int y;
+ };
+} a;
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3121,7 +3121,8 @@
 
   SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
 
-  // Explicitly record the calling convention for C++ records.
+  // Explicitly record the calling convention and export symbols for C++
+  // records.
   auto Flags = llvm::DINode::FlagZero;
   if (auto CXXRD = dyn_cast(RD)) {
 if (CGM.getCXXABI().getRecordArgABI(CXXRD) == CGCXXABI::RAA_Indirect)
@@ -3132,6 +3133,10 @@
 // Record if a C++ record is non-trivial type.
 if (!CXXRD->isTrivial())
   Flags |= llvm::DINode::FlagNonTrivial;
+
+// Record exports it symbols to the containing structure.
+if (CXXRD->isAnonymousStructOrUnion())
+Flags |= llvm::DINode::FlagExportSymbols;
   }
 
   llvm::DICompositeType *RealDecl = DBuilder.createReplaceableCompositeType(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66822: Hardware cache line size builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

> BTW, I note that facebook uses 128 bytes for x86

They also use 64 for arm which is interesting (the opposite of this patch).

Also, see this comment in the same snippet:

> We assume a cache line size of 64, so we use a cache line pair size of 128

Which would indicate that 64 is the correct number of bytes for x86. But maybe 
we should double that for `hardware_destructive_interference_size`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66822



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

r370107 - Debug Info: Support for DW_AT_export_symbols for anonymous structs

[Shafik Yaghmour via cfe-commits]

Author: shafik
Date: Tue Aug 27 13:17:35 2019
New Revision: 370107

URL: http://llvm.org/viewvc/llvm-project?rev=370107=rev
Log:
Debug Info: Support for DW_AT_export_symbols for anonymous structs

This implements the DWARF 5 feature described in:

http://dwarfstd.org/ShowIssue.php?issue=141212.1

To support recognizing anonymous structs:

  struct A {
struct { // Anonymous struct
int y;
};
  } a;

This patch adds support in CGDebugInfo::CreateLimitedType(...) for this new 
flag and an accompanying test to verify this feature.

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

Added:
cfe/trunk/test/CodeGenCXX/debug-info-export_symbols.cpp
Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=370107=370106=370107=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Tue Aug 27 13:17:35 2019
@@ -3121,7 +3121,8 @@ llvm::DICompositeType *CGDebugInfo::Crea
 
   SmallString<256> Identifier = getTypeIdentifier(Ty, CGM, TheCU);
 
-  // Explicitly record the calling convention for C++ records.
+  // Explicitly record the calling convention and export symbols for C++
+  // records.
   auto Flags = llvm::DINode::FlagZero;
   if (auto CXXRD = dyn_cast(RD)) {
 if (CGM.getCXXABI().getRecordArgABI(CXXRD) == CGCXXABI::RAA_Indirect)
@@ -3132,6 +3133,10 @@ llvm::DICompositeType *CGDebugInfo::Crea
 // Record if a C++ record is non-trivial type.
 if (!CXXRD->isTrivial())
   Flags |= llvm::DINode::FlagNonTrivial;
+
+// Record exports it symbols to the containing structure.
+if (CXXRD->isAnonymousStructOrUnion())
+Flags |= llvm::DINode::FlagExportSymbols;
   }
 
   llvm::DICompositeType *RealDecl = DBuilder.createReplaceableCompositeType(

Added: cfe/trunk/test/CodeGenCXX/debug-info-export_symbols.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-export_symbols.cpp?rev=370107=auto
==
--- cfe/trunk/test/CodeGenCXX/debug-info-export_symbols.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/debug-info-export_symbols.cpp Tue Aug 27 13:17:35 
2019
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone -triple 
%itanium_abi_triple %s -o - | FileCheck %s
+
+// CHECK: [[SCOPE:![0-9]+]] = distinct !DICompositeType({{.*}}flags: 
DIFlagTypePassByValue
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, scope: [[SCOPE]]
+// CHECK-SAME:  DIFlagExportSymbols | 
DIFlagTypePassByValue
+struct A {
+ // Anonymous class exports its symbols into A
+ struct {
+ int y;
+ };
+} a;


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

[PATCH] D66598: [Clang][Bundler] Fix for a hang when unbundling fat binary

[Sergey Dmitriev via Phabricator via cfe-commits]

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



Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:301
 assert(CurBundleInfo != BundlesInfo.end() && "Invalid reader info!");
 ++CurBundleInfo;
   }

Just noticed that this line now looks redundant and should be removed.


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

https://reviews.llvm.org/D66598



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

[PATCH] D66716: [analyzer] PR43102: Fix an assertion and an out-of-bounds error for diagnostic location construction

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

Szelethus added a comment.

In D66716#1647668 , @NoQ wrote:

> I don't understand. Isn't widening supposed to happen //after we exit the 
> loop//? The loop isn't over yet when the bug is being reported. Why is this 
> problem widening-specific? Given that we also have a weird invalidation of 
> `b`, i suspect that we're doing widening in a wrong moment of time.


Something is totally off here. Do you think its ever okay to find a last store 
in a `BlockEdge`? Should I rather fix this by changing how loop widening works?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66716



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

[PATCH] D66822: Hardware cache line size builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

We should probably tell people never to use this, period. That being said, I 
like your idea of having it be a constant. The only issue would be when, in the 
next few years, people start shipping CPUs with 256-byte-wide cache lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66822



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

[PATCH] D66598: [Clang][Bundler] Fix for a hang when unbundling fat binary

[Sergey Dmitriev via Phabricator via cfe-commits]

sdmitriev added a comment.

Sure. The main unbundling loop looks as follows (see loop on line 745)

  while (!Worklist.empty()) {
StringRef CurTriple = FH.get()->ReadBundleStart(Input);
  
if (CurTriple.empty())
  break;
  
auto Output = Worklist.find(CurTriple);
if (Output == Worklist.end()) {
  continue;
} 
   ...
  }

Here `Worklist` is a collection of bundles that need to be extracted, and `FH` 
is the object implementing `FileHandler` interface. 
`FileHandler::ReadBundleStart()` returns the name of the bundle `FH` currently 
points to. As you can see in the loop above if the name returned by that call 
is not in the set of bundles that need to be extracted, we just call 
`ReadBundleStart` next time assuming that `FileHandler` would advance to the 
next bundle on each `ReadBundleStart` call. That assumption is correct for all 
`FileHandler` implementations except `BinaryFileHandler` which does not advance 
to the next bundle when this method is called. As a result we are going into an 
infinite loop if we need to skip a bundle for the file handled by the 
`BinaryFileHandler` . This patch just fixes this problem in the 
`BinaryFileHandler` implementation.


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

https://reviews.llvm.org/D66598



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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Paul Kirth via Phabricator via cfe-commits]

paulkirth marked 6 inline comments as done.
paulkirth added inline comments.



Comment at: llvm/include/llvm/IR/DiagnosticInfo.h:1009
+public:
+  DiagnosticInfoMisExpect(const Function , const Twine ,
+  const DiagnosticLocation  = DiagnosticLocation(),

nickdesaulniers wrote:
> Is this constructor used anywhere in this patch?
No, sorry, I changed this interface and failed to remove the constructor.



Comment at: llvm/lib/IR/DiagnosticInfo.cpp:378
+ Inst->getDebugLoc()),
+  Msg(Msg) {}
+

nickdesaulniers wrote:
> Why is this constructor defined out of line, when the other is in the header? 
>  Seems like they should either both be in the header, or both be out of line, 
> with the goal being consistency.
I removed the other constructor, which was unused. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66324



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

[PATCH] D66324: clang-misexpect: Profile Guided Validation of Performance Annotations in LLVM

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 217479.
paulkirth added a comment.

Removes unused constructor, reformat code, and remove braces


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66324

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Profile/Inputs/misexpect-branch-nonconst-expect-arg.proftext
  clang/test/Profile/Inputs/misexpect-branch.proftext
  clang/test/Profile/Inputs/misexpect-switch-default-only.proftext
  clang/test/Profile/Inputs/misexpect-switch-default.proftext
  clang/test/Profile/Inputs/misexpect-switch.proftext
  clang/test/Profile/misexpect-branch-cold.c
  clang/test/Profile/misexpect-branch-nonconst-expected-val.c
  clang/test/Profile/misexpect-branch.c
  clang/test/Profile/misexpect-switch-default.c
  clang/test/Profile/misexpect-switch-nonconst.c
  clang/test/Profile/misexpect-switch-only-default-case.c
  clang/test/Profile/misexpect-switch.c
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/FixedMetadataKinds.def
  llvm/include/llvm/IR/MDBuilder.h
  llvm/include/llvm/Transforms/Utils/MisExpect.h
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/lib/IR/MDBuilder.cpp
  llvm/lib/Transforms/IPO/SampleProfile.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/MisExpect.cpp
  llvm/test/ThinLTO/X86/lazyload_metadata.ll
  llvm/test/Transforms/LowerExpectIntrinsic/basic.ll
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-branch.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch-correct.proftext
  llvm/test/Transforms/PGOProfile/Inputs/misexpect-switch.proftext
  llvm/test/Transforms/PGOProfile/misexpect-branch-correct.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch-default.ll
  llvm/test/Transforms/PGOProfile/misexpect-switch.ll

Index: llvm/test/Transforms/PGOProfile/misexpect-switch.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/misexpect-switch.ll
@@ -0,0 +1,293 @@
+
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch.proftext -o %t.profdata
+; RUN: llvm-profdata merge %S/Inputs/misexpect-switch-correct.proftext -o %t.c.profdata
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; New PM
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -S 2>&1 | FileCheck %s --check-prefix=WARNING
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=BOTH
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.profdata -S 2>&1 | FileCheck %s --check-prefix=DISABLED
+
+; RUN: opt < %s -lower-expect -pgo-instr-use -pgo-test-profile-file=%t.c.profdata -S -pgo-warn-misexpect -pass-remarks=misexpect 2>&1 | FileCheck %s --check-prefix=CORRECT
+; RUN: opt < %s -passes="function(lower-expect),pgo-instr-use" -pgo-test-profile-file=%t.c.profdata -pgo-warn-misexpect -pass-remarks=misexpect -S 2>&1 | FileCheck %s --check-prefix=CORRECT
+
+; WARNING-DAG: warning: misexpect-switch.c:26:5: 0.00%
+; WARNING-NOT: remark: misexpect-switch.c:26:5: Potential performance regression from use of __builtin_expect(): Annotation was correct on 0.00% of profiled executions.
+
+; REMARK-NOT: warning: misexpect-switch.c:26:5: 0.00%
+; REMARK-DAG: remark: misexpect-switch.c:26:5: Potential performance regression from use of __builtin_expect(): Annotation was correct on 0.00% of profiled executions.
+
+; BOTH-DAG: warning: misexpect-switch.c:26:5: 0.00%
+; BOTH-DAG: remark: misexpect-switch.c:26:5: Potential performance regression from use of __builtin_expect(): Annotation was correct on 0.00% of profiled executions.
+
+; DISABLED-NOT: warning: 

[PATCH] D66827: Add support for MS qualifiers __ptr32, __ptr64, __sptr, __uptr.

[Amy Huang via Phabricator via cfe-commits]

akhuang created this revision.
akhuang added reviewers: rnk, rsmith.
Herald added subscribers: llvm-commits, cfe-commits, erik.pilkington, 
hiraditya, nhaehnle, jvesely, jholewinski.
Herald added projects: clang, LLVM.

Previously, these qualifiers were being parsed but otherwise ignored.
This change makes it so that an address space is added to specify whether the 
pointer is
32-bit or 64-bit and whether it is sign extended or zero extended.

In the backend, the address space casts are lowered to the corresponding
sign/zero extension or truncation.

The data layout for the address spaces was changed in 
https://reviews.llvm.org/D64931

Related to https://bugs.llvm.org/show_bug.cgi?id=42359


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66827

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/AddressSpaces.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/TCE.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaType.cpp
  llvm/lib/Target/X86/X86.h
  llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/test/CodeGen/X86/mixed-ptr-sizes.ll

Index: llvm/test/CodeGen/X86/mixed-ptr-sizes.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/mixed-ptr-sizes.ll
@@ -0,0 +1,108 @@
+; RUN: llc < %s | FileCheck --check-prefixes=CHECK %s
+
+; Source to regenerate:
+; struct Foo {
+;   int * __ptr32 p32;
+;   int * __ptr64 p64;
+; }
+; void use_foo(Foo *f);
+; void test_sign_ext(Foo *f, int * __ptr32 __sptr i) {
+;   f->p64 = i;
+;   use_foo(f);
+; }
+; void test_zero_ext(Foo *f, int * __ptr32 __uptr i) {
+;   f->p64 = i;
+;   use_foo(f);
+; }
+; void test_trunc(foo *f, int * __ptr64 i) {
+;   f->p32 = i;
+;   use_foo(f);
+; }
+; void test_noop1(foo *f, int * __ptr32 i) {
+;   f->p32 = i;
+;   use_foo(f);
+; }
+; void test_noop2(foo *f, int * __ptr64 i) {
+;   f->p64 = i;
+;   use_foo(f);
+; }
+;
+; $ clang -cc1 -triple x86_64-windows-msvc -fms-extensions -O2 -S t.cpp
+
+; ModuleID = 't.cpp'
+source_filename = "t.cpp"
+target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-windows-msvc"
+
+%struct.Foo = type { i32 addrspace(270)*, i32* }
+declare dso_local void @"?use_foo@@YAXPEAUFoo@@@Z"(%struct.Foo*) local_unnamed_addr #1
+
+; Function Attrs: nounwind
+define dso_local void @"?test_sign_ext@@YAXPEAUFoo@@PEAU?$_ASPtr32_sptr@$$CAH@__clang@@@Z"(%struct.Foo* %f, i32 addrspace(270)* %i) local_unnamed_addr #0 {
+; CHECK-LABEL: {{.*}}test_sign_ext{{.*}}
+; CHECK:   movslq %edx, %rax
+entry:
+  %0 = addrspacecast i32 addrspace(270)* %i to i32*
+  %p64 = getelementptr inbounds %struct.Foo, %struct.Foo* %f, i64 0, i32 1
+  store i32* %0, i32** %p64, align 8, !tbaa !2
+  tail call void @"?use_foo@@YAXPEAUFoo@@@Z"(%struct.Foo* %f) #2
+  ret void
+}
+
+; Function Attrs: nounwind
+define dso_local void @"?test_zero_ext@@YAXPEAUFoo@@PEAU?$_ASPtr32_uptr@$$CAH@__clang@@@Z"(%struct.Foo* %f, i32 addrspace(271)* %i) local_unnamed_addr #0 {
+; CHECK-LABEL: {{.*}}test_zero_ext{{.*}}
+; CHECK:   movl %edx, %eax
+entry:
+  %0 = addrspacecast i32 addrspace(271)* %i to i32*
+  %p64 = getelementptr inbounds %struct.Foo, %struct.Foo* %f, i64 0, i32 1
+  store i32* %0, i32** %p64, align 8, !tbaa !2
+  tail call void @"?use_foo@@YAXPEAUFoo@@@Z"(%struct.Foo* %f) #2
+  ret void
+}
+
+; Function Attrs: nounwind
+define dso_local void @"?test_trunc@@YAXPEAUFoo@@PEAH@Z"(%struct.Foo* %f, i32* %i) local_unnamed_addr #0 {
+; CHECK-LABEL: {{.*}}test_trunc{{.*}}
+; CHECK:   movl %edx, (%rcx)
+entry:
+  %0 = addrspacecast i32* %i to i32 addrspace(270)*
+  %p32 = getelementptr inbounds %struct.Foo, %struct.Foo* %f, i64 0, i32 0
+  store i32 addrspace(270)* %0, i32 addrspace(270)** %p32, align 8, !tbaa !7
+  tail call void @"?use_foo@@YAXPEAUFoo@@@Z"(%struct.Foo* %f) #2
+  ret void
+}
+
+; Function Attrs: nounwind
+define dso_local void @"?test_noop1@@YAXPEAUFoo@@PEAU?$_ASPtr32_sptr@$$CAH@__clang@@@Z"(%struct.Foo* %f, i32 addrspace(270)* %i) local_unnamed_addr #0 {
+; CHECK-LABEL: {{.*}}test_noop1{{.*}}
+; CHECK:   movl %edx, (%rcx)
+entry:
+  %p32 = getelementptr inbounds %struct.Foo, %struct.Foo* %f, i64 0, i32 0
+  store i32 addrspace(270)* %i, i32 addrspace(270)** %p32, align 8, !tbaa !7
+  tail call void @"?use_foo@@YAXPEAUFoo@@@Z"(%struct.Foo* %f) #2
+  ret void
+}
+
+; Function Attrs: nounwind
+define dso_local void @"?test_noop2@@YAXPEAUFoo@@PEAH@Z"(%struct.Foo* %f, i32* %i) local_unnamed_addr #0 {
+; CHECK-LABEL: {{.*}}test_noop2{{.*}}
+; CHECK:   movq %rdx, 8(%rcx)
+entry:
+  %p64 = getelementptr inbounds %struct.Foo, %struct.Foo* %f, i64 0, i32 1
+  store i32* %i, i32** %p64, align 8, 

[PATCH] D66822: Hardware cache line size builtins

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

numbers for cacheline size.

In D66822#1647664 , @zoecarver wrote:

> > Passing-by remark: i'm not sure it is possible to **guarantee** that this 
> > will be always correct and that no ABI break will happen.
>
> You're right. I should have said, "least-likely to cause an ABI break." And I 
> completely agree that there is **no way** to gaurentee this is correct at 
> compile time. `hardware_*_interference_size` certainly has the potential to 
> do more harm than good but, I think that is another discussion.


I don't see why we'd bother to implement this as a builtin, if we're going to 
implement it like this. A much simpler implementation would be to have libc++ 
return 64 for constructive and 128 for destructive, across the board. That'd 
certainly be abi stable, and also correct, at the moment, for architectures 
people generally care about. (And we should tell people to never use these if 
they actually care about it.)

BTW, I note that facebook uses 128 bytes for x86, noting in the source 
:

  Microbenchmarks indicate that pairs of cache lines also see destructive
  interference under heavy use of atomic operations, as observed for atomic
  increment on Sandy Bridge.
  
  We assume a cache line size of 64, so we use a cache line pair size of 128
  to avoid destructive interference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66822



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

[PATCH] D66822: Hardware cache line size builtins

[Zoe Carver via Phabricator via cfe-commits]

zoecarver added a comment.

It may be a good idea only to enable this when `-march=native`. And _maybe_ 
remove the constexpr requirement (though it would make this feature 
significantly less useful it would also make it significantly more accurate).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66822



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

[PATCH] D66723: [clangd] Add a distinct highlighting for local variables

[Nathan Ridge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG86a4a530f435: [clangd] Add a distinct highlighting for local 
variables (authored by nridge).

Changed prior to commit:
  https://reviews.llvm.org/D66723?vs=217065=217476#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66723

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/semantic-highlighting.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -44,7 +44,8 @@
   {HighlightingKind::Field, "Field"},
   {HighlightingKind::Method, "Method"},
   {HighlightingKind::TemplateParameter, "TemplateParameter"},
-  {HighlightingKind::Primitive, "Primitive"}};
+  {HighlightingKind::Primitive, "Primitive"},
+  {HighlightingKind::LocalVariable, "LocalVariable"}};
   std::vector ExpectedTokens;
   for (const auto  : KindToString) {
 std::vector Toks = makeHighlightingTokens(
@@ -103,31 +104,31 @@
 
 TEST(SemanticHighlighting, GetsCorrectTokens) {
   const char *TestCases[] = {
-R"cpp(
+  R"cpp(
   struct $Class[[AS]] {
 $Primitive[[double]] $Field[[SomeMember]];
   };
   struct {
   } $Variable[[S]];
   $Primitive[[void]] $Function[[foo]]($Primitive[[int]] $Parameter[[A]], $Class[[AS]] $Parameter[[As]]) {
-$Primitive[[auto]] $Variable[[VeryLongVariableName]] = 12312;
-$Class[[AS]] $Variable[[AA]];
-$Primitive[[auto]] $Variable[[L]] = $Variable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
-auto $Variable[[FN]] = [ $Variable[[AA]]]($Primitive[[int]] $Parameter[[A]]) -> $Primitive[[void]] {};
-$Variable[[FN]](12312);
+$Primitive[[auto]] $LocalVariable[[VeryLongVariableName]] = 12312;
+$Class[[AS]] $LocalVariable[[AA]];
+$Primitive[[auto]] $LocalVariable[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
+auto $LocalVariable[[FN]] = [ $LocalVariable[[AA]]]($Primitive[[int]] $Parameter[[A]]) -> $Primitive[[void]] {};
+$LocalVariable[[FN]](12312);
   }
 )cpp",
-R"cpp(
+  R"cpp(
   $Primitive[[void]] $Function[[foo]]($Primitive[[int]]);
   $Primitive[[void]] $Function[[Gah]]();
   $Primitive[[void]] $Function[[foo]]() {
-auto $Variable[[Bou]] = $Function[[Gah]];
+auto $LocalVariable[[Bou]] = $Function[[Gah]];
   }
   struct $Class[[A]] {
 $Primitive[[void]] $Method[[abc]]();
   };
 )cpp",
-R"cpp(
+  R"cpp(
   namespace $Namespace[[abc]] {
 template
 struct $Class[[A]] {
@@ -149,12 +150,12 @@
   $Class[[B]]::$Class[[B]]() {}
   $Class[[B]]::~$Class[[B]]() {}
   $Primitive[[void]] $Function[[f]] () {
-$Class[[B]] $Variable[[BB]] = $Class[[B]]();
-$Variable[[BB]].~$Class[[B]]();
+$Class[[B]] $LocalVariable[[BB]] = $Class[[B]]();
+$LocalVariable[[BB]].~$Class[[B]]();
 $Class[[B]]();
   }
 )cpp",
-R"cpp(
+  R"cpp(
   enum class $Enum[[E]] {
 $EnumConstant[[A]],
 $EnumConstant[[B]],
@@ -169,7 +170,7 @@
   $Primitive[[int]] $Variable[[I]] = $EnumConstant[[Hi]];
   $Enum[[E]] $Variable[[L]] = $Enum[[E]]::$EnumConstant[[B]];
 )cpp",
-R"cpp(
+  R"cpp(
   namespace $Namespace[[abc]] {
 namespace {}
 namespace $Namespace[[bcd]] {
@@ -192,7 +193,7 @@
   ::$Namespace[[vwz]]::$Class[[A]] $Variable[[B]];
   ::$Namespace[[abc]]::$Namespace[[bcd]]::$Class[[A]] $Variable[[BB]];
 )cpp",
-R"cpp(
+  R"cpp(
   struct $Class[[D]] {
 $Primitive[[double]] $Field[[C]];
   };
@@ -209,21 +210,21 @@
 }
   };
   $Primitive[[void]] $Function[[foo]]() {
-$Class[[A]] $Variable[[AA]];
-$Variable[[AA]].$Field[[B]] += 2;
-$Variable[[AA]].$Method[[foo]]();
-$Variable[[AA]].$Field[[E]].$Field[[C]];
+$Class[[A]] $LocalVariable[[AA]];
+$LocalVariable[[AA]].$Field[[B]] += 2;
+$LocalVariable[[AA]].$Method[[foo]]();
+$LocalVariable[[AA]].$Field[[E]].$Field[[C]];
 $Class[[A]]::$Variable[[S]] = 90;
   }
 )cpp",
-R"cpp(
+  R"cpp(
   struct $Class[[AA]] {
 $Primitive[[int]] $Field[[A]];
   }
   $Primitive[[int]] $Variable[[B]];
   $Class[[AA]] $Variable[[A]]{$Variable[[B]]};
 )cpp",
-R"cpp(
+  R"cpp(
   namespace $Namespace[[a]] {
 struct $Class[[A]] {};
 typedef $Primitive[[char]] $Primitive[[C]];
@@ -239,7 +240,7 @@