[PATCH] D67031: [Clang][Bundler] Error reporting improvements [NFC]

[Sergey Dmitriev via Phabricator via cfe-commits]

sdmitriev created this revision.
sdmitriev added a reviewer: ABataev.
Herald added a reviewer: alexshap.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D67031

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
@@ -25,23 +25,17 @@
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
-#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/StringSaver.h"
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
+#include "llvm/Support/WithColor.h"
+#include "llvm/Support/raw_ostream.h"
 
 using namespace llvm;
 using namespace llvm::object;
@@ -98,6 +92,9 @@
 /// Path to the current binary.
 static std::string BundlerExecutable;
 
+/// Saved argv[0].
+static StringRef Argv0;
+
 /// Obtain the offload kind and real machine triple out of the target
 /// information specified by the user.
 static void getOffloadKindAndTriple(StringRef Target, StringRef ,
@@ -113,6 +110,15 @@
   return OffloadKind == "host";
 }
 
+/// Error reporting functions.
+static void reportError(Error E) {
+  logAllUnhandledErrors(std::move(E), WithColor::error(errs(), Argv0));
+}
+
+static void reportFileError(StringRef File, std::error_code EC) {
+  reportError(createFileError(File, EC));
+}
+
 /// Generic file handler interface.
 class FileHandler {
 public:
@@ -146,7 +152,6 @@
 
   /// Write the marker that closes a bundle for the triple \a TargetTriple to \a
   /// OS. Return true if any error was found.
-
   virtual bool WriteBundleEnd(raw_fd_ostream , StringRef TargetTriple) = 0;
 
   /// Write the bundle from \a Input into \a OS.
@@ -327,7 +332,7 @@
 
 unsigned Idx = 0;
 for (auto  : TargetNames) {
-  MemoryBuffer  = *Inputs[Idx++].get();
+  MemoryBuffer  = *Inputs[Idx++];
   // Bundle offset.
   Write8byteIntegerToBuffer(OS, HeaderSize);
   // Size of the bundle (adds to the next bundle's offset)
@@ -467,7 +472,8 @@
 ErrorOr Objcopy = sys::findProgramByName(
 "llvm-objcopy", sys::path::parent_path(BundlerExecutable));
 if (!Objcopy) {
-  errs() << "error: unable to find 'llvm-objcopy' in path.\n";
+  reportError(createStringError(Objcopy.getError(),
+"unable to find 'llvm-objcopy' in path."));
   return true;
 }
 
@@ -489,13 +495,14 @@
 // If the user asked for the commands to be printed out, we do that instead
 // of executing it.
 if (PrintExternalCommands) {
-  errs() << "\"" << Objcopy.get() << "\"";
+  errs() << "\"" << *Objcopy << "\"";
   for (StringRef Arg : drop_begin(ObjcopyArgs, 1))
 errs() << " \"" << Arg << "\"";
   errs() << "\n";
 } else {
-  if (sys::ExecuteAndWait(Objcopy.get(), ObjcopyArgs)) {
-errs() << "error: llvm-objcopy tool failed.\n";
+  if (sys::ExecuteAndWait(*Objcopy, ObjcopyArgs)) {
+reportError(createStringError(inconvertibleErrorCode(),
+  "'llvm-objcopy' tool failed."));
 return true;
   }
 }
@@ -622,13 +629,11 @@
   // We only support regular object files. If this is not an object file,
   // default to the binary handler. The handler will be owned by the client of
   // this function.
-  std::unique_ptr Obj(
-  dyn_cast(BinaryOrErr.get().release()));
-
-  if (!Obj)
-return new BinaryFileHandler();
-
-  return new ObjectFileHandler(std::move(Obj));
+  if (auto *Obj = dyn_cast(BinaryOrErr->get())) {
+  BinaryOrErr->release();
+  return new ObjectFileHandler(std::unique_ptr(Obj));
+  }
+  return new BinaryFileHandler();
 }
 
 /// Return an appropriate handler given the input files and options.
@@ -650,7 +655,10 @@
   if (FilesType == "ast")
 return new BinaryFileHandler();
 
-  errs() << "error: invalid file type specified.\n";
+  reportError(
+  createStringError(errc::invalid_argument,
+"'" + FilesType + "': invalid file type specified."));
+
   return nullptr;
 }
 
@@ -662,7 +670,7 @@
   raw_fd_ostream OutputFile(OutputFileNames.front(), EC, sys::fs::OF_None);
 
   if (EC) {
-errs() << "error: Can't open file " << OutputFileNames.front() << ".\n";
+reportFileError(OutputFileNames.front(), EC);
 return true;
   }
 
@@ -675,31 +683,31 @@
 

[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 218200.
paulkirth added a comment.

Add clang and LLVM tests for __builtin_unpredictable


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/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-unpredictable.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-stripped.ll
  llvm/test/Transforms/PGOProfile/misexpect-branch-unpredictable.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 the llvm.expect intrinsic: 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 the llvm.expect intrinsic: 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 

[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 218198.
paulkirth added a comment.

Add inline checks for misexpect tags to LowerExpectIntrinstic test


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/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-stripped.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 the llvm.expect intrinsic: 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 the llvm.expect intrinsic: 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 the llvm.expect intrinsic: Annotation was correct on 0.00% of profiled executions.
+
+; DISABLED-NOT: warning: 

[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 218197.
paulkirth added a comment.

Update LowerExpectIntrinsic/basic.ll to generate misexpect metadata for a switch


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/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-stripped.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 the llvm.expect intrinsic: 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 the llvm.expect intrinsic: 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 the llvm.expect intrinsic: Annotation was correct on 0.00% of 

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

[Paul Kirth via Phabricator via cfe-commits]

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



Comment at: llvm/test/Transforms/LowerExpectIntrinsic/basic.ll:141
   %tobool = icmp ne i64 %expval, 0
-; CHECK: !prof !1
+; CHECK: !prof !2
 ; CHECK-NOT: @llvm.expect

lebedev.ri wrote:
> Here and elsewhere:
> is `!prof !1` no longer present?
The misexpect metadata tags take those slots.

`br i1 %tobool, label %if.then, label %if.end, !prof !0, !misexpect !1`

Unless my understanding of how the tags work is flawed, I think this is the 
expected behavior



Comment at: llvm/test/Transforms/LowerExpectIntrinsic/basic.ll:292-293
+; CHECK: !3 = !{!"misexpect", i64 1, i64 2000, i64 1}
+; CHECK: !4 = !{!"branch_weights", i32 1, i32 2000, i32 1}
+; CHECK: !5 = !{!"branch_weights", i32 2000, i32 1, i32 1}

lebedev.ri wrote:
> Should there be a FIXME, are some `misexpect` missing here?
This is happenstance unfortunately.

```
; CHECK: !4 = !{!"branch_weights", i32 1, i32 2000, i32 1}
; CHECK: !5 = !{!"branch_weights", i32 2000, i32 1, i32 1}
```

In both of these cases the expected index is still 0 or 1, so the metadata tags 
are deduplicated. 

If we want to check further, then we need to modify the test to select an index 
greater than 1.


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 added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3449
 
+  if (Diags.isIgnored(diag::warn_profile_data_misexpect, SourceLocation()))
+Res.FrontendOpts.LLVMArgs.push_back("-pgo-warn-misexpect");

lebedev.ri wrote:
> Err, this works? I'd expect `!` to be missing here..
I ran check-clang and check-llvm. Are there more tests I should be running?



Comment at: llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp:78-87
+  SI.setMetadata(
+  LLVMContext::MD_misexpect,
+  MDBuilder(CI->getContext())
+  .createMisExpect(Index, LikelyBranchWeight, UnlikelyBranchWeight));
+
+  SI.setCondition(ArgValue);
+  misexpect::checkClangInstrumentation(SI);

lebedev.ri wrote:
> paulkirth wrote:
> > lebedev.ri wrote:
> > > Why can't `LLVMContext::MD_prof` be reused?
> > It didn't seem appropriate to me, but that can be changed if it is the 
> > right thing to do. 
> > 
> > However, I don't see code elsewhere that loops over `MD_prof` metadata 
> > checking its tag. I see lots of examples that check if the tag matches, and 
> > then exits if it doesn't match. 
> > 
> > If I added "misexpect" as a new `MD_prof`tag, then code in places like 
> > `extractProfTotalWeight(...)` in IR/Metadata.cpp would have to be updated. 
> > I'm not sure how pervasive that pattern is, but I did  want to avoid 
> > introducing systemic changes like that.
> > 
> > https://github.com/llvm/llvm-project/blob/9976a5bc1db5a240378a61a68c0e182060bcb554/llvm/lib/IR/Metadata.cpp#L1336
> That's not exactly what i was asking.
> I'm specifically talking about how `"misexpect"` and `"branch_weigths"` carry 
> basically the same data,
> are set basically in the same place always, with the difference that 
> `"misexpect"` also has the switch case index.
> This is both somewhat bloaty, and is prone to getting out of sync.
> 
> I have two questions:
> 1. Can `"misexpect"` metadata simply not be invented, but can existing 
> `LLVMContext::MD_misexpect` simply be used?
> 2. Can `"misexpect"` be changed to not store the weights, but a reference to 
> the `LLVMContext::MD_misexpect` metadata that will be emitted anyway?
> 
Sorry for the misunderstanding. 

I think what you are suggesting is to recreate the misexpect metadata from the 
MD_prof "branch_weights metadata. Is that right?

I think that may be possible, but it will add complexity. While the metadata 
approach is very straightforward. I think there is value in keeping this 
simple, and then iterating on it. 

Also, LowerExpectIntrinsic.cpp has a comment near the top suggesting that the 
LikelyBranchWeight and UnlikelyBranchWeights be hoisted to a common space. This 
would significantly shrink the size of the misexpect metadata if it were 
accessible directly. However, that is a change that should happen separately 
from this patch, and might require further discussion to find the correct 
shared space.

Ultimately, however, I do not see a way to completely remove some kind of 
metadata tag, as we only need to perform misexpect checks when the intrinsic 
was used -- which should be a small minority of cases. If there is another way, 
then I am happy to hear it, but again, maybe that should be another patch.





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] D67026: Introduce a DirectoryEntryRef that stores both a reference and an accessed name to the directory entry

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
arphaman marked an inline comment as done.
Closed by commit rL370562: Introduce a DirectoryEntryRef that stores both a 
reference and an (authored by arphaman, committed by ).
Herald added subscribers: llvm-commits, jsji, MaskRay, kbarton, nemanjai.
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D67026?vs=218181=218196#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67026

Files:
  cfe/trunk/include/clang/Basic/FileManager.h
  cfe/trunk/include/clang/Lex/DirectoryLookup.h
  cfe/trunk/lib/Basic/FileManager.cpp
  cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
  cfe/trunk/lib/Lex/HeaderSearch.cpp
  cfe/trunk/lib/Lex/PPDirectives.cpp
  cfe/trunk/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json
  cfe/trunk/test/ClangScanDeps/subframework_header_dir_symlink.m
  cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
  cfe/trunk/unittests/Lex/PPCallbacksTest.cpp

Index: cfe/trunk/lib/Lex/PPDirectives.cpp
===
--- cfe/trunk/lib/Lex/PPDirectives.cpp
+++ cfe/trunk/lib/Lex/PPDirectives.cpp
@@ -1695,7 +1695,7 @@
 // Give the clients a chance to recover.
 SmallString<128> RecoveryPath;
 if (Callbacks->FileNotFound(Filename, RecoveryPath)) {
-  if (auto DE = FileMgr.getDirectory(RecoveryPath)) {
+  if (auto DE = FileMgr.getOptionalDirectoryRef(RecoveryPath)) {
 // Add the recovery path to the list of search paths.
 DirectoryLookup DL(*DE, SrcMgr::C_User, false);
 HeaderInfo.AddSearchPath(DL, isAngled);
Index: cfe/trunk/lib/Lex/HeaderSearch.cpp
===
--- cfe/trunk/lib/Lex/HeaderSearch.cpp
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp
@@ -296,6 +296,7 @@
 /// getName - Return the directory or filename corresponding to this lookup
 /// object.
 StringRef DirectoryLookup::getName() const {
+  // FIXME: Use the name from \c DirectoryEntryRef.
   if (isNormalDir())
 return getDir()->getName();
   if (isFramework())
@@ -496,7 +497,7 @@
 
   // FrameworkName = "/System/Library/Frameworks/"
   SmallString<1024> FrameworkName;
-  FrameworkName += getFrameworkDir()->getName();
+  FrameworkName += getFrameworkDirRef()->getName();
   if (FrameworkName.empty() || FrameworkName.back() != '/')
 FrameworkName.push_back('/');
 
Index: cfe/trunk/lib/Basic/FileManager.cpp
===
--- cfe/trunk/lib/Basic/FileManager.cpp
+++ cfe/trunk/lib/Basic/FileManager.cpp
@@ -107,16 +107,8 @@
   addAncestorsAsVirtualDirs(DirName);
 }
 
-/// Converts a llvm::ErrorOr to an llvm::ErrorOr by promoting
-/// the address of the inner reference to a pointer or by propagating the error.
-template 
-static llvm::ErrorOr promoteInnerReference(llvm::ErrorOr value) {
-  if (value) return &*value;
-  return value.getError();
-}
-
-llvm::ErrorOr
-FileManager::getDirectory(StringRef DirName, bool CacheFailure) {
+llvm::Expected
+FileManager::getDirectoryRef(StringRef DirName, bool CacheFailure) {
   // stat doesn't like trailing separators except for root directory.
   // At least, on Win32 MSVCRT, stat() cannot strip trailing '/'.
   // (though it can strip '\\')
@@ -141,8 +133,11 @@
   // contains both virtual and real directories.
   auto SeenDirInsertResult =
   SeenDirEntries.insert({DirName, std::errc::no_such_file_or_directory});
-  if (!SeenDirInsertResult.second)
-return promoteInnerReference(SeenDirInsertResult.first->second);
+  if (!SeenDirInsertResult.second) {
+if (SeenDirInsertResult.first->second)
+  return DirectoryEntryRef(&*SeenDirInsertResult.first);
+return llvm::errorCodeToError(SeenDirInsertResult.first->second.getError());
+  }
 
   // We've not seen this before. Fill it in.
   ++NumDirCacheMisses;
@@ -163,7 +158,7 @@
   NamedDirEnt.second = statError;
 else
   SeenDirEntries.erase(DirName);
-return statError;
+return llvm::errorCodeToError(statError);
   }
 
   // It exists.  See if we have already opened a directory with the
@@ -179,7 +174,15 @@
 UDE.Name  = InterndDirName;
   }
 
-  return 
+  return DirectoryEntryRef();
+}
+
+llvm::ErrorOr
+FileManager::getDirectory(StringRef DirName, bool CacheFailure) {
+  auto Result = getDirectoryRef(DirName, CacheFailure);
+  if (Result)
+return >getDirEntry();
+  return llvm::errorToErrorCode(Result.takeError());
 }
 
 llvm::ErrorOr
Index: cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
===
--- cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
+++ cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
@@ -148,7 +148,7 @@
   }
 
   // If the directory exists, add it.
-  if (auto DE = FM.getDirectory(MappedPathStr)) {
+  if (auto DE = FM.getOptionalDirectoryRef(MappedPathStr)) {
 IncludePath.push_back(

[PATCH] D67026: Introduce a DirectoryEntryRef that stores both a reference and an accessed name to the directory entry

[Alex Lorenz via Phabricator via cfe-commits]

arphaman marked 4 inline comments as done.
arphaman added inline comments.



Comment at: clang/lib/Lex/HeaderSearch.cpp:299
 StringRef DirectoryLookup::getName() const {
+  // FIXME: Use the name from \c DirectoryEntryRef.
   if (isNormalDir())

dexonsmith wrote:
> Did you mean to leave this behind for a future commit?  Or did you miss 
> fixing this?
This is for a future commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67026



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

r370562 - Introduce a DirectoryEntryRef that stores both a reference and an

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Fri Aug 30 18:26:04 2019
New Revision: 370562

URL: http://llvm.org/viewvc/llvm-project?rev=370562=rev
Log:
Introduce a DirectoryEntryRef that stores both a reference and an
accessed name to the directory entry

This commit introduces a parallel API that returns a DirectoryEntryRef
to the FileManager, similar to the parallel FileEntryRef API. All
uses will have to be update in follow-up patches. The immediate use of the new 
API in this
patch fixes the issue where a file manager was reused in clang-scan-deps,
but reported an different file path whenever a framework lookup was done 
through a symlink.

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

Modified:
cfe/trunk/include/clang/Basic/FileManager.h
cfe/trunk/include/clang/Lex/DirectoryLookup.h
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json
cfe/trunk/test/ClangScanDeps/subframework_header_dir_symlink.m
cfe/trunk/unittests/Lex/HeaderSearchTest.cpp
cfe/trunk/unittests/Lex/PPCallbacksTest.cpp

Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=370562=370561=370562=diff
==
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Fri Aug 30 18:26:04 2019
@@ -45,12 +45,31 @@ class FileSystemStatCache;
 class DirectoryEntry {
   friend class FileManager;
 
+  // FIXME: We should not be storing a directory entry name here.
   StringRef Name; // Name of the directory.
 
 public:
   StringRef getName() const { return Name; }
 };
 
+/// A reference to a \c DirectoryEntry  that includes the name of the directory
+/// as it was accessed by the FileManager's client.
+class DirectoryEntryRef {
+public:
+  const DirectoryEntry () const { return *Entry->getValue(); }
+
+  StringRef getName() const { return Entry->getKey(); }
+
+private:
+  friend class FileManager;
+
+  DirectoryEntryRef(
+  llvm::StringMapEntry> *Entry)
+  : Entry(Entry) {}
+
+  const llvm::StringMapEntry> *Entry;
+};
+
 /// Cached information about one file (either on disk
 /// or in the virtual file system).
 ///
@@ -259,6 +278,29 @@ public:
   /// virtual).
   ///
   /// This returns a \c std::error_code if there was an error reading the
+  /// directory. On success, returns the reference to the directory entry
+  /// together with the exact path that was used to access a file by a
+  /// particular call to getDirectoryRef.
+  ///
+  /// \param CacheFailure If true and the file does not exist, we'll cache
+  /// the failure to find this file.
+  llvm::Expected getDirectoryRef(StringRef DirName,
+bool CacheFailure = true);
+
+  /// Get a \c DirectoryEntryRef if it exists, without doing anything on error.
+  llvm::Optional
+  getOptionalDirectoryRef(StringRef DirName, bool CacheFailure = true) {
+return llvm::expectedToOptional(getDirectoryRef(DirName, CacheFailure));
+  }
+
+  /// Lookup, cache, and verify the specified directory (real or
+  /// virtual).
+  ///
+  /// This function is deprecated and will be removed at some point in the
+  /// future, new clients should use
+  ///  \c getDirectoryRef.
+  ///
+  /// This returns a \c std::error_code if there was an error reading the
   /// directory. If there is no error, the DirectoryEntry is guaranteed to be
   /// non-NULL.
   ///

Modified: cfe/trunk/include/clang/Lex/DirectoryLookup.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/DirectoryLookup.h?rev=370562=370561=370562=diff
==
--- cfe/trunk/include/clang/Lex/DirectoryLookup.h (original)
+++ cfe/trunk/include/clang/Lex/DirectoryLookup.h Fri Aug 30 18:26:04 2019
@@ -36,14 +36,17 @@ public:
 LT_HeaderMap
   };
 private:
-  union {  // This union is discriminated by isHeaderMap.
+  union DLU { // This union is discriminated by isHeaderMap.
 /// Dir - This is the actual directory that we're referring to for a normal
 /// directory or a framework.
-const DirectoryEntry *Dir;
+DirectoryEntryRef Dir;
 
 /// Map - This is the HeaderMap if this is a headermap lookup.
 ///
 const HeaderMap *Map;
+
+DLU(DirectoryEntryRef Dir) : Dir(Dir) {}
+DLU(const HeaderMap *Map) : Map(Map) {}
   } u;
 
   /// DirCharacteristic - The type of directory this is: this is an instance of
@@ -62,24 +65,18 @@ private:
   unsigned SearchedAllModuleMaps : 1;
 
 public:
-  /// DirectoryLookup ctor - Note that this ctor *does not take ownership* of
-  /// 'dir'.
-  DirectoryLookup(const DirectoryEntry *dir, SrcMgr::CharacteristicKind DT,
+  /// This ctor *does not take 

r370558 - [c++20] Add support for designated direct-list-initialization syntax.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Fri Aug 30 18:00:37 2019
New Revision: 370558

URL: http://llvm.org/viewvc/llvm-project?rev=370558=rev
Log:
[c++20] Add support for designated direct-list-initialization syntax.

This completes the implementation of P0329R4.

Added:
cfe/trunk/test/Parser/cxx2a-designated-init.cpp
Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Parse/ParseInit.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/www/cxx_status.html

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=370558=370557=370558=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Fri Aug 30 18:00:37 2019
@@ -4831,6 +4831,10 @@ public:
   SourceLocation getEqualOrColonLoc() const { return EqualOrColonLoc; }
   void setEqualOrColonLoc(SourceLocation L) { EqualOrColonLoc = L; }
 
+  /// Whether this designated initializer should result in 
direct-initialization
+  /// of the designated subobject (eg, '{.foo{1, 2, 3}}').
+  bool isDirectInit() const { return EqualOrColonLoc.isInvalid(); }
+
   /// Determines whether this designated initializer used the
   /// deprecated GNU syntax for designated initializers.
   bool usesGNUSyntax() const { return GNUSyntax; }

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=370558=370557=370558=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Fri Aug 30 18:00:37 2019
@@ -4644,7 +4644,7 @@ public:
SourceLocation RBraceLoc);
 
   ExprResult ActOnDesignatedInitializer(Designation ,
-SourceLocation Loc,
+SourceLocation EqualOrColonLoc,
 bool GNUSyntax,
 ExprResult Init);
 

Modified: cfe/trunk/lib/Parse/ParseInit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseInit.cpp?rev=370558=370557=370558=diff
==
--- cfe/trunk/lib/Parse/ParseInit.cpp (original)
+++ cfe/trunk/lib/Parse/ParseInit.cpp Fri Aug 30 18:00:37 2019
@@ -116,6 +116,8 @@ static void CheckArrayDesignatorSyntax(P
 /// ParseInitializerWithPotentialDesignator - Parse the 'initializer' 
production
 /// checking to see if the token stream starts with a designator.
 ///
+/// C99:
+///
 ///   designation:
 /// designator-list '='
 /// [GNU]   array-designator
@@ -133,6 +135,21 @@ static void CheckArrayDesignatorSyntax(P
 /// '[' constant-expression ']'
 /// [GNU]   '[' constant-expression '...' constant-expression ']'
 ///
+/// C++20:
+///
+///   designated-initializer-list:
+/// designated-initializer-clause
+/// designated-initializer-list ',' designated-initializer-clause
+///
+///   designated-initializer-clause:
+/// designator brace-or-equal-initializer
+///
+///   designator:
+/// '.' identifier
+///
+/// We allow the C99 syntax extensions in C++20, but do not allow the C++20
+/// extension (a braced-init-list after the designator with no '=') in C99.
+///
 /// NOTE: [OBC] allows '[ objc-receiver objc-message-args ]' as an
 /// initializer (because it is an expression).  We need to consider this case
 /// when parsing array designators.
@@ -365,6 +382,14 @@ ExprResult Parser::ParseInitializerWithP
   ParseInitializer());
   }
 
+  // Handle a C++20 braced designated initialization, which results in
+  // direct-list-initialization of the aggregate element. We allow this as an
+  // extension from C++11 onwards (when direct-list-initialization was added).
+  if (Tok.is(tok::l_brace) && getLangOpts().CPlusPlus11) {
+return Actions.ActOnDesignatedInitializer(Desig, SourceLocation(), false,
+  ParseBraceInitializer());
+  }
+
   // We read some number of designators and found something that isn't an = or
   // an initializer.  If we have exactly one array designator, this
   // is the GNU 'designation: array-designator' extension.  Otherwise, it is a

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=370558=370557=370558=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Aug 30 18:00:37 2019
@@ -2367,6 +2367,29 @@ InitListChecker::CheckDesignatedInitiali
 bool FinishSubobjectInit,
 

[PATCH] D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added a comment.

Yeah I think that can be a more generalized solution too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66983



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

[PATCH] D67030: ContentCache: Simplify by always owning the MemoryBuffer

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

dexonsmith created this revision.
dexonsmith added reviewers: arphaman, Bigcheese, rsmith.
Herald added a subscriber: ributzka.

This changes ContentCache::Buffer to use `std::unique_ptr`
instead of the PointerIntPair.  It drops the (mostly unused) `DoNotFree`
bit in favour of creating a new, non-owning MemoryBuffer instance.


https://reviews.llvm.org/D67030

Files:
  clang/include/clang/Basic/SourceManager.h
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp

Index: lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
===
--- lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
+++ lldb/source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
@@ -151,7 +151,7 @@
   clang::SourceManager SM(diags, file_mgr);
   auto buf = llvm::MemoryBuffer::getMemBuffer(full_source);
 
-  FileID FID = SM.createFileID(clang::SourceManager::Unowned, buf.get());
+  FileID FID = SM.createFileID(buf->getMemBufferRef());
 
   // Let's just enable the latest ObjC and C++ which should get most tokens
   // right.
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -336,8 +336,12 @@
 
 // Override the contents of the "from" file with the contents of
 // the "to" file.
-SourceMgr.overrideFileContents(FromFile, RB.second,
-   InitOpts.RetainRemappedFileBuffers);
+if (InitOpts.RetainRemappedFileBuffers)
+  SourceMgr.overrideFileContents(FromFile, *RB.second);
+else
+  SourceMgr.overrideFileContents(
+  FromFile, std::unique_ptr(
+const_cast(RB.second)));
   }
 
   // Remap files in the source manager (with other files).
Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -49,28 +49,22 @@
 // SourceManager Helper Classes
 //===--===//
 
-ContentCache::~ContentCache() {
-  if (shouldFreeBuffer())
-delete Buffer.getPointer();
-}
-
 /// getSizeBytesMapped - Returns the number of bytes actually mapped for this
 /// ContentCache. This can be 0 if the MemBuffer was not actually expanded.
 unsigned ContentCache::getSizeBytesMapped() const {
-  return Buffer.getPointer() ? Buffer.getPointer()->getBufferSize() : 0;
+  return Buffer ? Buffer->getBufferSize() : 0;
 }
 
 /// Returns the kind of memory used to back the memory buffer for
 /// this content cache.  This is used for performance analysis.
 llvm::MemoryBuffer::BufferKind ContentCache::getMemoryBufferKind() const {
-  assert(Buffer.getPointer());
+  assert(Buffer);
 
   // Should be unreachable, but keep for sanity.
-  if (!Buffer.getPointer())
+  if (!Buffer)
 return llvm::MemoryBuffer::MemoryBuffer_Malloc;
 
-  const llvm::MemoryBuffer *buf = Buffer.getPointer();
-  return buf->getBufferKind();
+  return Buffer->getBufferKind();
 }
 
 /// getSize - Returns the size of the content encapsulated by this ContentCache.
@@ -78,23 +72,8 @@
 ///  scratch buffer.  If the ContentCache encapsulates a source file, that
 ///  file is not lazily brought in from disk to satisfy this query.
 unsigned ContentCache::getSize() const {
-  return Buffer.getPointer() ? (unsigned) Buffer.getPointer()->getBufferSize()
- : (unsigned) ContentsEntry->getSize();
-}
-
-void ContentCache::replaceBuffer(const llvm::MemoryBuffer *B, bool DoNotFree) {
-  IsBufferInvalid = false;
-
-  if (B && B == Buffer.getPointer()) {
-assert(0 && "Replacing with the same buffer");
-Buffer.setInt(DoNotFree? DoNotFreeFlag : 0);
-return;
-  }
-
-  if (shouldFreeBuffer())
-delete Buffer.getPointer();
-  Buffer.setPointer(B);
-  Buffer.setInt((B && DoNotFree) ? DoNotFreeFlag : 0);
+  return Buffer ? (unsigned)Buffer->getBufferSize()
+: (unsigned)ContentsEntry->getSize();
 }
 
 llvm::Optional
@@ -102,10 +81,10 @@
 SourceLocation Loc) const {
   // Lazily create the Buffer for ContentCaches that wrap files.  If we already
   // computed it, just return what we have.
-  if (isBufferInvalid())
+  if (IsBufferInvalid)
 return None;
-  if (auto *B = Buffer.getPointer())
-return B->getMemBufferRef();
+  if (Buffer)
+return Buffer->getMemBufferRef();
   if (!ContentsEntry)
 return None;
 
@@ -147,7 +126,7 @@
 return None;
   }
 
-  Buffer.setPointer(BufferOrError->release());
+  Buffer = std::move(*BufferOrError);
 
   // Check that the file's size is the same as in the file entry (which may
   // have come from a stat cache).
@@ -166,7 +145,7 @@
   // If the buffer is valid, check to see if it has a UTF Byte Order Mark
   

[PATCH] D67028: Use musttail for variadic method thunks when possible

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Do we have test coverage for a variadic, covariant thunk for a function without 
a definition?  I don't think there's any way for us to actually emit that, but 
we should make sure the error message is right.

I'm a little concerned that using musttail thunks with the Itanium ABI will 
flush out bugs that have been hiding because we have less test coverage on 
Windows. But it's probably the right thing to do.




Comment at: clang/lib/CodeGen/CGVTables.cpp:206
+  AdjustedThisPtr = Builder.CreateBitCast(AdjustedThisPtr,
+  ThisStore->getOperand(0)->getType());
   ThisStore->setOperand(0, AdjustedThisPtr);

This is fine, I guess, but how is it related?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67028



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

[PATCH] D67029: SourceManager: Prefer Optional over MemoryBuffer*

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

dexonsmith created this revision.
dexonsmith added reviewers: arphaman, Bigcheese.
Herald added subscribers: llvm-commits, ributzka, kadircet, jkorous, hiraditya.
Herald added a reviewer: martong.
Herald added a reviewer: shafik.
Herald added a project: LLVM.

Change the APIs in SourceManager returning `const MemoryBuffer*` to
return `Optional` instead, and remove the `bool
*Invalid` flag from same.  This removes the need to create dummy
invalid buffers in SrcMgr::ContentCache and clarifies buffer ownership.

A few APIs changed, and a few were added.

- SrcMgr::ContentCache::getBuffer returns Optional.
- SrcMgr::ContentCache::getBufferIdentifier returns Optional.
- SrcMgr::ContentCache::getBufferSize returns Optional.
- SourceManager::getMemoryBufferForFile returns Optional.
- SourceManager::getBuffer returns Optional.
- SourceManager::getBufferOrFake returns MemoryBufferRef, replacing None with 
fake data.
- SourceManager::getBufferIdentifier returns Optional.
- SourceManager::getBufferIdentifierOrEmpty returns StringRef, replacing None 
with `""`.
- SourceManager::getBufferDataOrNone returns Optional

SourceManager::getBufferData did not change to return
Optional because its usage is too pervasive.  A possible
follow-up would be to add SourceManager::getBufferDataOrFake (to match
the other APIs) and move components over but I'm not sure it's urgent.

Threading MemoryBufferRef (instead of `const MemoryBuffer*`) through had
a few small side effects:

- FrontendInputFile now stores a MemoryBufferRef.
- SourceManager::createFileID now takes a MemoryBufferRef.
- llvm::line_iterator now uses an Optional.
- MemoryBufferRef::operator== now exists and compares pointer identity for 
comparing Optional.


https://reviews.llvm.org/D67029

Files:
  clang-tools-extra/clang-tidy/bugprone/SuspiciousSemicolonCheck.cpp
  clang-tools-extra/clang-tidy/misc/StaticAssertCheck.cpp
  clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
  clang-tools-extra/clangd/ClangdUnit.cpp
  clang-tools-extra/clangd/Format.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/modularize/PreprocessorTracker.cpp
  clang/include/clang/Basic/SourceManager.h
  clang/include/clang/Frontend/FrontendAction.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/include/clang/Frontend/PrecompiledPreamble.h
  clang/include/clang/Lex/Lexer.h
  clang/lib/ARCMigrate/Transforms.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/Basic/Diagnostic.cpp
  clang/lib/Basic/SourceLocation.cpp
  clang/lib/Basic/SourceManager.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp
  clang/lib/Frontend/Rewrite/HTMLPrint.cpp
  clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
  clang/lib/Frontend/Rewrite/RewriteMacros.cpp
  clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  clang/lib/Frontend/Rewrite/RewriteObjC.cpp
  clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
  clang/lib/Lex/Lexer.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Lex/PPLexerChange.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Rewrite/HTMLRewrite.cpp
  clang/lib/Rewrite/TokenRewriter.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/IssueHash.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/tools/clang-diff/ClangDiff.cpp
  clang/tools/clang-import-test/clang-import-test.cpp
  clang/tools/libclang/CIndex.cpp
  llvm/include/llvm/Support/LineIterator.h
  llvm/include/llvm/Support/MemoryBuffer.h
  llvm/lib/Support/LineIterator.cpp

Index: llvm/lib/Support/LineIterator.cpp
===
--- llvm/lib/Support/LineIterator.cpp
+++ llvm/lib/Support/LineIterator.cpp
@@ -31,27 +31,37 @@
   return false;
 }
 
+line_iterator::line_iterator(const MemoryBufferRef , bool SkipBlanks,
+ char CommentMarker)
+: line_iterator(StringRef(Buffer.getBufferStart(), Buffer.getBufferSize()),
+SkipBlanks, CommentMarker) {}
+
 line_iterator::line_iterator(const MemoryBuffer , bool SkipBlanks,
  char CommentMarker)
-: Buffer(Buffer.getBufferSize() ?  : nullptr),
+: line_iterator(StringRef(Buffer.getBufferStart(), Buffer.getBufferSize()),
+SkipBlanks, CommentMarker) {}
+
+line_iterator::line_iterator(StringRef Buffer, bool SkipBlanks,
+ char CommentMarker)
+: Buffer(Buffer.empty() ? Optional(None) : Buffer),
   

[PATCH] D67023: Diagnose use of ATOMIC_VAR_INIT

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

Is atomic initialization now correct in all modes (including C++) without this 
macro? I don’t think we should diagnose until such a time because some code 
uses to macro to be portably correct. IIRC we only ended up fixing C++ in 20 
with Nico’s paper (after Olivier and I failed repeatedly to do so.


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

https://reviews.llvm.org/D67023



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

[PATCH] D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

@sunfish That sounds like a useful mechanism to me. I'd be happy to work on 
that next week. Is the consensus that we should not merge this change and 
instead pursue that idea?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66983



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

[PATCH] D67020: [WebAssembly] Add SIMD QFMA/QFMS

[Thomas Lively via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370556: [WebAssembly] Add SIMD QFMA/QFMS (authored by 
tlively, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D67020?vs=218167=218193#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D67020

Files:
  cfe/trunk/include/clang/Basic/BuiltinsWebAssembly.def
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/test/CodeGen/builtins-wasm.c
  llvm/trunk/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/trunk/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
  llvm/trunk/test/CodeGen/WebAssembly/simd-intrinsics.ll
  llvm/trunk/test/MC/WebAssembly/simd-encodings.s

Index: llvm/trunk/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
===
--- llvm/trunk/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
+++ llvm/trunk/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
@@ -732,3 +732,24 @@
   )
 ) in
 def : Pat<(t1 (bitconvert (t2 V128:$v))), (t1 V128:$v)>;
+
+//===--===//
+// Quasi-Fused Multiply- Add and Subtract (QFMA/QFMS)
+//===--===//
+multiclass SIMDQFM baseInst> {
+  defm QFMA_#vec_t :
+SIMD_I<(outs V128:$dst), (ins V128:$a, V128:$b, V128:$c),
+   (outs), (ins),
+   [(set (vec_t V128:$dst),
+ (int_wasm_qfma (vec_t V128:$a), (vec_t V128:$b), (vec_t V128:$c)))],
+   vec#".qfma\t$dst, $a, $b, $c", vec#".qfma", baseInst>;
+  defm QFMS_#vec_t :
+SIMD_I<(outs V128:$dst), (ins V128:$a, V128:$b, V128:$c),
+   (outs), (ins),
+   [(set (vec_t V128:$dst),
+ (int_wasm_qfms (vec_t V128:$a), (vec_t V128:$b), (vec_t V128:$c)))],
+   vec#".qfms\t$dst, $a, $b, $c", vec#".qfms", !add(baseInst, 1)>;
+}
+
+defm "" : SIMDQFM;
+defm "" : SIMDQFM;
Index: llvm/trunk/include/llvm/IR/IntrinsicsWebAssembly.td
===
--- llvm/trunk/include/llvm/IR/IntrinsicsWebAssembly.td
+++ llvm/trunk/include/llvm/IR/IntrinsicsWebAssembly.td
@@ -109,6 +109,14 @@
   Intrinsic<[llvm_i32_ty],
 [llvm_anyvector_ty],
 [IntrNoMem, IntrSpeculatable]>;
+def int_wasm_qfma :
+  Intrinsic<[llvm_anyvector_ty],
+[LLVMMatchType<0>, LLVMMatchType<0>, LLVMMatchType<0>],
+[IntrNoMem, IntrSpeculatable]>;
+def int_wasm_qfms :
+  Intrinsic<[llvm_anyvector_ty],
+[LLVMMatchType<0>, LLVMMatchType<0>, LLVMMatchType<0>],
+[IntrNoMem, IntrSpeculatable]>;
 
 //===--===//
 // Bulk memory intrinsics
Index: llvm/trunk/test/CodeGen/WebAssembly/simd-intrinsics.ll
===
--- llvm/trunk/test/CodeGen/WebAssembly/simd-intrinsics.ll
+++ llvm/trunk/test/CodeGen/WebAssembly/simd-intrinsics.ll
@@ -290,11 +290,35 @@
 declare <4 x float> @llvm.wasm.bitselect.v4f32(<4 x float>, <4 x float>, <4 x float>)
 define <4 x float> @bitselect_v4f32(<4 x float> %v1, <4 x float> %v2, <4 x float> %c) {
   %a = call <4 x float> @llvm.wasm.bitselect.v4f32(
- <4 x float> %v1, <4 x float> %v2, <4 x float> %c
+<4 x float> %v1, <4 x float> %v2, <4 x float> %c
   )
   ret <4 x float> %a
 }
 
+; CHECK-LABEL: qfma_v4f32:
+; SIMD128-NEXT: .functype qfma_v4f32 (v128, v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: f32x4.qfma $push[[R:[0-9]+]]=, $0, $1, $2{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <4 x float> @llvm.wasm.qfma.v4f32(<4 x float>, <4 x float>, <4 x float>)
+define <4 x float> @qfma_v4f32(<4 x float> %a, <4 x float> %b, <4 x float> %c) {
+  %v = call <4 x float> @llvm.wasm.qfma.v4f32(
+<4 x float> %a, <4 x float> %b, <4 x float> %c
+  )
+  ret <4 x float> %v
+}
+
+; CHECK-LABEL: qfms_v4f32:
+; SIMD128-NEXT: .functype qfms_v4f32 (v128, v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: f32x4.qfms $push[[R:[0-9]+]]=, $0, $1, $2{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <4 x float> @llvm.wasm.qfms.v4f32(<4 x float>, <4 x float>, <4 x float>)
+define <4 x float> @qfms_v4f32(<4 x float> %a, <4 x float> %b, <4 x float> %c) {
+  %v = call <4 x float> @llvm.wasm.qfms.v4f32(
+<4 x float> %a, <4 x float> %b, <4 x float> %c
+  )
+  ret <4 x float> %v
+}
+
 ; ==
 ; 2 x f64
 ; ==
@@ -309,3 +333,27 @@
   )
   ret <2 x double> %a
 }
+
+; CHECK-LABEL: qfma_v2f64:
+; SIMD128-NEXT: .functype qfma_v2f64 (v128, v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: f64x2.qfma $push[[R:[0-9]+]]=, $0, $1, $2{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <2 x double> @llvm.wasm.qfma.v2f64(<2 x double>, <2 x double>, <2 x double>)
+define <2 x double> 

r370556 - [WebAssembly] Add SIMD QFMA/QFMS

[Thomas Lively via cfe-commits]

Author: tlively
Date: Fri Aug 30 17:12:29 2019
New Revision: 370556

URL: http://llvm.org/viewvc/llvm-project?rev=370556=rev
Log:
[WebAssembly] Add SIMD QFMA/QFMS

Summary:
Adds clang builtins and LLVM intrinsics for these experimental
instructions. They are not implemented in engines yet, but that is ok
because the user must opt into using them by calling the builtins.

Reviewers: aheejin, dschuff

Reviewed By: aheejin

Subscribers: sbc100, jgravelle-google, hiraditya, sunfish, cfe-commits, 
llvm-commits

Tags: #clang, #llvm

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

Modified:
cfe/trunk/include/clang/Basic/BuiltinsWebAssembly.def
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/builtins-wasm.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsWebAssembly.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsWebAssembly.def?rev=370556=370555=370556=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsWebAssembly.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsWebAssembly.def Fri Aug 30 17:12:29 
2019
@@ -108,6 +108,11 @@ TARGET_BUILTIN(__builtin_wasm_max_f64x2,
 TARGET_BUILTIN(__builtin_wasm_sqrt_f32x4, "V4fV4f", "nc", 
"unimplemented-simd128")
 TARGET_BUILTIN(__builtin_wasm_sqrt_f64x2, "V2dV2d", "nc", 
"unimplemented-simd128")
 
+TARGET_BUILTIN(__builtin_wasm_qfma_f32x4, "V4fV4fV4fV4f", "nc", "simd128")
+TARGET_BUILTIN(__builtin_wasm_qfms_f32x4, "V4fV4fV4fV4f", "nc", "simd128")
+TARGET_BUILTIN(__builtin_wasm_qfma_f64x2, "V2dV2dV2dV2d", "nc", 
"unimplemented-simd128")
+TARGET_BUILTIN(__builtin_wasm_qfms_f64x2, "V2dV2dV2dV2d", "nc", 
"unimplemented-simd128")
+
 TARGET_BUILTIN(__builtin_wasm_trunc_saturate_s_i32x4_f32x4, "V4iV4f", "nc", 
"simd128")
 TARGET_BUILTIN(__builtin_wasm_trunc_saturate_u_i32x4_f32x4, "V4iV4f", "nc", 
"simd128")
 TARGET_BUILTIN(__builtin_wasm_trunc_saturate_s_i64x2_f64x2, "V2LLiV2d", "nc", 
"unimplemented-simd128")

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=370556=370555=370556=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Fri Aug 30 17:12:29 2019
@@ -14173,7 +14173,29 @@ Value *CodeGenFunction::EmitWebAssemblyB
 Function *Callee = CGM.getIntrinsic(Intrinsic::sqrt, Vec->getType());
 return Builder.CreateCall(Callee, {Vec});
   }
-
+  case WebAssembly::BI__builtin_wasm_qfma_f32x4:
+  case WebAssembly::BI__builtin_wasm_qfms_f32x4:
+  case WebAssembly::BI__builtin_wasm_qfma_f64x2:
+  case WebAssembly::BI__builtin_wasm_qfms_f64x2: {
+Value *A = EmitScalarExpr(E->getArg(0));
+Value *B = EmitScalarExpr(E->getArg(1));
+Value *C = EmitScalarExpr(E->getArg(2));
+unsigned IntNo;
+switch (BuiltinID) {
+case WebAssembly::BI__builtin_wasm_qfma_f32x4:
+case WebAssembly::BI__builtin_wasm_qfma_f64x2:
+  IntNo = Intrinsic::wasm_qfma;
+  break;
+case WebAssembly::BI__builtin_wasm_qfms_f32x4:
+case WebAssembly::BI__builtin_wasm_qfms_f64x2:
+  IntNo = Intrinsic::wasm_qfms;
+  break;
+default:
+  llvm_unreachable("unexpected builtin ID");
+}
+Function *Callee = CGM.getIntrinsic(IntNo, A->getType());
+return Builder.CreateCall(Callee, {A, B, C});
+  }
   default:
 return nullptr;
   }

Modified: cfe/trunk/test/CodeGen/builtins-wasm.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins-wasm.c?rev=370556=370555=370556=diff
==
--- cfe/trunk/test/CodeGen/builtins-wasm.c (original)
+++ cfe/trunk/test/CodeGen/builtins-wasm.c Fri Aug 30 17:12:29 2019
@@ -412,6 +412,34 @@ f64x2 sqrt_f64x2(f64x2 x) {
   // WEBASSEMBLY: ret
 }
 
+f32x4 qfma_f32x4(f32x4 a, f32x4 b, f32x4 c) {
+  return __builtin_wasm_qfma_f32x4(a, b, c);
+  // WEBASSEMBLY: call <4 x float> @llvm.wasm.qfma.v4f32(
+  // WEBASSEMBLY-SAME: <4 x float> %a, <4 x float> %b, <4 x float> %c)
+  // WEBASSEMBLY-NEXT: ret
+}
+
+f32x4 qfms_f32x4(f32x4 a, f32x4 b, f32x4 c) {
+  return __builtin_wasm_qfms_f32x4(a, b, c);
+  // WEBASSEMBLY: call <4 x float> @llvm.wasm.qfms.v4f32(
+  // WEBASSEMBLY-SAME: <4 x float> %a, <4 x float> %b, <4 x float> %c)
+  // WEBASSEMBLY-NEXT: ret
+}
+
+f64x2 qfma_f64x2(f64x2 a, f64x2 b, f64x2 c) {
+  return __builtin_wasm_qfma_f64x2(a, b, c);
+  // WEBASSEMBLY: call <2 x double> @llvm.wasm.qfma.v2f64(
+  // WEBASSEMBLY-SAME: <2 x double> %a, <2 x double> %b, <2 x double> %c)
+  // WEBASSEMBLY-NEXT: ret
+}
+
+f64x2 qfms_f64x2(f64x2 a, f64x2 b, f64x2 c) {
+  return __builtin_wasm_qfms_f64x2(a, b, c);
+  // WEBASSEMBLY: call <2 x double> @llvm.wasm.qfms.v2f64(
+  // WEBASSEMBLY-SAME: <2 x double> %a, <2 x double> %b, <2 x double> %c)
+  // WEBASSEMBLY-NEXT: ret
+}
+
 i32x4 

[PATCH] D67020: [WebAssembly] Add SIMD QFMA/QFMS

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

In D67020#1653403 , @aheejin wrote:

> Should we handle these too?
>
> - There seems to be a generic intrinsic for fused multiply-add: 
> https://github.com/llvm/llvm-project/blob/d21a3e41a4cfd52e3c5c9341f0b5ce8a173198bf/llvm/include/llvm/Target/GenericOpcodes.td#L612-L619
> - There are FMA related nodes in ISDOpcode.h, such as
>   - 
> https://github.com/llvm/llvm-project/blob/1ea909270c6d9ad612a46c2a15c13acc4f6d623a/llvm/include/llvm/CodeGen/ISDOpcodes.h#L332-L337
>   - 
> https://github.com/llvm/llvm-project/blob/1ea909270c6d9ad612a46c2a15c13acc4f6d623a/llvm/include/llvm/CodeGen/ISDOpcodes.h#L294


Yes, eventually. For now I want the FMA to be explicitly opt-in, and I don't 
care about the autovectorizer generating it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67020



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

[PATCH] D67028: Use musttail for variadic method thunks when possible

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.
rnk added reviewers: rsmith, hans, efriedma.
Herald added a subscriber: fedor.sergeev.
Herald added a project: clang.

This avoids cloning variadic virtual methods when the target supports
musttail and the return type is not covariant. I think we never
implemented this previously because it doesn't handle the covariant
case. But, in the MS ABI, there are some cases where vtable thunks must
be emitted even when the variadic method defintion is not available, so
it looks like we need to implement this. Do it for both ABIs, since it's
a nice size improvement and simplification for Itanium.

Fixes PR43173.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67028

Files:
  clang/lib/CodeGen/CGVTables.cpp
  clang/test/CodeGenCXX/linetable-virtual-variadic.cpp
  clang/test/CodeGenCXX/thunks-variadic-covariant.cpp
  clang/test/CodeGenCXX/thunks-variadic.cpp
  clang/test/CodeGenCXX/thunks.cpp

Index: clang/test/CodeGenCXX/thunks.cpp
===
--- clang/test/CodeGenCXX/thunks.cpp
+++ clang/test/CodeGenCXX/thunks.cpp
@@ -1,6 +1,7 @@
-// RUN: %clang_cc1 %s -triple=x86_64-pc-linux-gnu -munwind-tables -emit-llvm -o - | FileCheck --check-prefix=CHECK --check-prefix=CHECK-NONOPT %s
-// RUN: %clang_cc1 %s -triple=x86_64-pc-linux-gnu -debug-info-kind=standalone -dwarf-version=5 -munwind-tables -emit-llvm -o - | FileCheck --check-prefix=CHECK --check-prefix=CHECK-NONOPT --check-prefix=CHECK-DBG %s
-// RUN: %clang_cc1 %s -triple=x86_64-pc-linux-gnu -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes | FileCheck --check-prefix=CHECK --check-prefix=CHECK-OPT %s
+// Sparc64 is used because AArch64 and X86_64 would both use musttail.
+// RUN: %clang_cc1 %s -triple=sparc64-pc-linux-gnu -munwind-tables -emit-llvm -o - | FileCheck --check-prefix=CHECK --check-prefix=CHECK-NONOPT %s
+// RUN: %clang_cc1 %s -triple=sparc64-pc-linux-gnu -debug-info-kind=standalone -dwarf-version=5 -munwind-tables -emit-llvm -o - | FileCheck --check-prefix=CHECK --check-prefix=CHECK-NONOPT --check-prefix=CHECK-DBG %s
+// RUN: %clang_cc1 %s -triple=sparc64-pc-linux-gnu -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes | FileCheck --check-prefix=CHECK --check-prefix=CHECK-OPT %s
 
 namespace Test1 {
 
Index: clang/test/CodeGenCXX/thunks-variadic.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/thunks-variadic.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 %s -triple=x86_64-windows-msvc -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes | FileCheck --check-prefix=MSVC %s
+// RUN: %clang_cc1 %s -triple=x86_64-windows-gnu -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes | FileCheck --check-prefix=ITANIUM %s
+
+namespace test1 {
+class a {
+  virtual void b(const char *, ...);
+};
+class c {
+  virtual void b(const char *, ...);
+};
+class e : c, a {
+  void b(const char *, ...) override;
+} d;
+
+// MSVC-LABEL: define linkonce_odr dso_local void @"?b@e@test1@@G7EAAXPEBDZZ"
+// MSVC-SAME: (%"class.test1::e"* %this, i8* %[[ARG:[^,]+]], ...)
+// MSVC: getelementptr i8, i8* %{{.*}}, i32 -8
+// MSVC: musttail call void (%"class.test1::e"*, i8*, ...) @"?b@e@test1@@EEAAXPEBDZZ"(%"class.test1::e"* %{{.*}}, i8* %[[ARG]], ...)
+
+// ITANIUM-LABEL: define available_externally dso_local void @_ZThn8_N5test11e1bEPKcz
+// ITANIUM-SAME: (%"class.test1::e"* %this, i8* %{{.*}}, ...)
+// ITANIUM: getelementptr inbounds i8, i8* %{{.*}}, i64 -8
+// ITANIUM: musttail call void (%"class.test1::e"*, i8*, ...) @_ZN5test11e1bEPKcz(%"class.test1::e"* %{{.*}}, i8* %{{.*}}, ...)
+}
Index: clang/test/CodeGenCXX/thunks-variadic-covariant.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/thunks-variadic-covariant.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 %s -triple=x86_64-windows-msvc -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes | FileCheck --check-prefix=MSVC %s
+// RUN: %clang_cc1 %s -triple=x86_64-windows-gnu -munwind-tables -emit-llvm -o - -O1 -disable-llvm-passes | FileCheck --check-prefix=ITANIUM %s
+
+struct A {
+  virtual A *f(int x, ...);
+};
+struct B {
+  virtual B *f(int x, ...);
+};
+struct C : A, B {
+  virtual void c();
+  virtual C *f(int x, ...);
+};
+C *makeC();
+C *C::f(int x, ...) { return makeC(); }
+C c;
+
+// MSVC-LABEL: define dso_local %struct.C* @"?f@C@@UEAAPEAU1@HZZ"
+// MSVC-SAME: (%struct.C* %this, i32 %x, ...)
+// MSVC: call %struct.C* @"?makeC@@YAPEAUC@@XZ"()
+
+// MSVC-LABEL: define weak_odr dso_local %struct.C* @"?f@C@@W7EAAPEAUB@@HZZ"
+// MSVC-SAME: (%struct.C* %this, i32 %x, ...)
+// MSVC: call %struct.C* @"?makeC@@YAPEAUC@@XZ"()
+// MSVC: getelementptr inbounds i8, i8* {{.*}}, i32 8
+// MSVC: ret %struct.C* %7
+
+// MSVC-LABEL: define linkonce_odr dso_local %struct.C* @"?f@C@@W7EAAPEAU1@HZZ"
+// MSVC-SAME: (%struct.C* %this, i32 %x, ...)
+// MSVC: getelementptr i8, i8* %{{[^,]*}}, i32 -8
+// MSVC: 

r370555 - [c++20] Disallow template argument deduction from a braced-init-list

[Richard Smith via cfe-commits]

Author: rsmith
Date: Fri Aug 30 17:05:50 2019
New Revision: 370555

URL: http://llvm.org/viewvc/llvm-project?rev=370555=rev
Log:
[c++20] Disallow template argument deduction from a braced-init-list
containing designators. The C++20 wording doesn't actually say what
happens in this case, but treating this as a non-deduced context seems
like the most natural behavior.

(We might want to consider deducing through array designators as an
extension in the future, but will need to be careful to deduce the array
bound properly if we do so. That's not permitted herein.)

Modified:
cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp
cfe/trunk/test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp
cfe/trunk/test/SemaTemplate/deduction.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp?rev=370555=370554=370555=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp Fri Aug 30 17:05:50 2019
@@ -3702,6 +3702,12 @@ static Sema::TemplateDeductionResult Ded
 return Sema::TDK_Success;
   }
 
+  // Resolving a core issue: a braced-init-list containing any designators is
+  // a non-deduced context.
+  for (Expr *E : ILE->inits())
+if (isa(E))
+  return Sema::TDK_Success;
+
   // Deduction only needs to be done for dependent types.
   if (ElTy->isDependentType()) {
 for (Expr *E : ILE->inits()) {
@@ -4509,6 +4515,12 @@ Sema::DeduceAutoType(TypeLoc Type, Expr
 if (!Type.getType().getNonReferenceType()->getAs())
   return DAR_Failed;
 
+// Resolving a core issue: a braced-init-list containing any designators is
+// a non-deduced context.
+for (Expr *E : InitList->inits())
+  if (isa(E))
+return DAR_Failed;
+
 SourceRange DeducedFromInitRange;
 for (unsigned i = 0, e = InitList->getNumInits(); i < e; ++i) {
   Expr *Init = InitList->getInit(i);

Modified: cfe/trunk/test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp?rev=370555=370554=370555=diff
==
--- cfe/trunk/test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp (original)
+++ cfe/trunk/test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp Fri Aug 30 
17:05:50 2019
@@ -29,9 +29,9 @@ namespace std {
 typedef const _E* iterator;
 typedef const _E* const_iterator;
 
-initializer_list() : __begin_(nullptr), __size_(0) {}
+constexpr initializer_list() : __begin_(nullptr), __size_(0) {}
 
-size_tsize()  const {return __size_;}
+constexpr size_tsize()  const {return __size_;}
 const _E* begin() const {return __begin_;}
 const _E* end()   const {return __begin_ + __size_;}
   };
@@ -354,3 +354,14 @@ namespace no_conversion_after_auto_list_
   struct Y { using T = std::initializer_list(*)(); operator T(); };
   auto (*y)() = { Y() }; // expected-error {{from initializer list}}
 }
+
+namespace designated_init {
+  constexpr auto a = {.a = 1, .b = 2}; // expected-error {{cannot deduce}}
+  constexpr auto b = {[0] = 1, [4] = 2}; // expected-error {{cannot deduce}} 
expected-warning {{C99}}
+  constexpr auto c = {1, [4] = 2}; // expected-error {{cannot deduce}} 
expected-warning 2{{C99}} expected-note {{here}}
+  constexpr auto d = {1, [0] = 2}; // expected-error {{cannot deduce}} 
expected-warning 2{{C99}} expected-note {{here}}
+
+  // If we ever start accepting the above, these assertions should pass.
+  static_assert(c.size() == 5, "");
+  static_assert(d.size() == 1, "");
+}

Modified: cfe/trunk/test/SemaTemplate/deduction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/deduction.cpp?rev=370555=370554=370555=diff
==
--- cfe/trunk/test/SemaTemplate/deduction.cpp (original)
+++ cfe/trunk/test/SemaTemplate/deduction.cpp Fri Aug 30 17:05:50 2019
@@ -539,3 +539,10 @@ namespace dependent_list_deduction {
 #endif
   }
 }
+
+namespace designators {
+  template constexpr int f(T (&&)[N]) { return N; } // 
expected-note 2{{couldn't infer template argument 'T'}}
+  static_assert(f({1, 2, [20] = 3}) == 3, ""); // expected-error {{no matching 
function}} expected-warning 2{{C99}} expected-note {{}}
+
+  static_assert(f({.a = 1, .b = 2}) == 3, ""); // expected-error {{no matching 
function}}
+}


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

[PATCH] D67020: [WebAssembly] Add SIMD QFMA/QFMS

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added a comment.

Should we handle these too?

- There seems to be a generic intrinsic for fused multiply-add: 
https://github.com/llvm/llvm-project/blob/d21a3e41a4cfd52e3c5c9341f0b5ce8a173198bf/llvm/include/llvm/Target/GenericOpcodes.td#L612-L619
- There are FMA related nodes in ISDOpcode.h, such as
  - 
https://github.com/llvm/llvm-project/blob/1ea909270c6d9ad612a46c2a15c13acc4f6d623a/llvm/include/llvm/CodeGen/ISDOpcodes.h#L332-L337
  - 
https://github.com/llvm/llvm-project/blob/1ea909270c6d9ad612a46c2a15c13acc4f6d623a/llvm/include/llvm/CodeGen/ISDOpcodes.h#L294


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67020



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

[PATCH] D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

In D66983#1651981 , @craig.topper 
wrote:

> DAG combine is supposed to check with TargetLowering::isShuffleMaskLegal.


In @tlively's example, it is DAGCombine, and it does check isShuffleMaskLegal. 
However for wasm, it appears that's not enough -- in wasm, all shuffle masks 
are legal, because you can do any shuffle in a single wasm instruction. This 
makes it tricky, because the user here is aiming for an x86-like cost model, 
but the LLVM wasm backend doesn't have any x86-specific knowledge, so it just 
tells DAGCombine to form any shuffle it sees fit.

I wonder if it would make sense to introduce a counterpart to 
isShuffleMaskLegal, which instead of returning a bool returned a cost value. 
And then, we could teach the wasm backend about certain shuffle patterns which 
are known to be fast across multiple architectures. Then we could teach 
DAGComine to check whether the new shuffle it wants to create is actually 
cheaper than the one it's replacing. Thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66983



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

[PATCH] D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

Link to DAGCombiner.cpp code: 
https://github.com/llvm/llvm-project/blob/802aab5/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp#L19014


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66983



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

[PATCH] D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added a comment.

In D66983#1653226 , @tlively wrote:

> The code that does the undesirable optimization is around 
> llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:18776.


Now line 18776 is a blank line :) You can take a permalink from the 
https://github.com/llvm/llvm-project repo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66983



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

[PATCH] D67020: [WebAssembly] Add SIMD QFMA/QFMS

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

https://github.com/WebAssembly/simd/pull/79/files

They need benchmarking data before they can be merged into the proposal, and 
they need to be supported in the toolchain for us to get good benchmarking data.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67020



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

[PATCH] D67026: Introduce a DirectoryEntryRef that stores both a reference and an accessed name to the directory entry

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

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

A few minor comments.  Assuming the `FIXME` I point out was intentionally left 
for later, this LGTM.




Comment at: clang/include/clang/Basic/FileManager.h:59
+public:
+  const DirectoryEntry *getDirEntry() const { return &(*Entry->getValue()); }
+

Should this return `const Directory&` because it's guaranteed to be valid.



Comment at: clang/include/clang/Lex/DirectoryLookup.h:68-69
 public:
   /// DirectoryLookup ctor - Note that this ctor *does not take ownership* of
   /// 'dir'.
+  DirectoryLookup(DirectoryEntryRef Dir, SrcMgr::CharacteristicKind DT,

Since you're modifying this constructor it might be nice to cleanup the comment 
style (drop the `Directory ctor -` prefix).  Up to you.



Comment at: clang/lib/Lex/HeaderSearch.cpp:299
 StringRef DirectoryLookup::getName() const {
+  // FIXME: Use the name from \c DirectoryEntryRef.
   if (isNormalDir())

Did you mean to leave this behind for a future commit?  Or did you miss fixing 
this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67026



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

[PATCH] D67026: Introduce a DirectoryEntryRef that stores both a reference and an accessed name to the directory entry

[Alex Lorenz via Phabricator via cfe-commits]

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

This patch is similar to the FileEntryRef patch, in that it introduces a 
parallel API to get the directory entry, without replacing all uses. The 
immediate use in the patch fixes the issue where a file manager was reused in 
clang-scan-deps, but reported an different file path whenever a framework 
lookup was done through a symlink.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67026

Files:
  clang/include/clang/Basic/FileManager.h
  clang/include/clang/Lex/DirectoryLookup.h
  clang/lib/Basic/FileManager.cpp
  clang/lib/Frontend/InitHeaderSearch.cpp
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json
  clang/test/ClangScanDeps/subframework_header_dir_symlink.m

Index: clang/test/ClangScanDeps/subframework_header_dir_symlink.m
===
--- clang/test/ClangScanDeps/subframework_header_dir_symlink.m
+++ clang/test/ClangScanDeps/subframework_header_dir_symlink.m
@@ -10,9 +10,8 @@
 // RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/subframework_header_dir_symlink_cdb.json > %t.cdb
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1  -reuse-filemanager=0 | \
 // RUN:   FileCheck %s
-// FIXME: Make this work when the filemanager is reused:
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -reuse-filemanager=1 | \
-// RUN:   not FileCheck %s
+// RUN:   FileCheck %s
 
 #ifndef EMPTY
 #include "Framework/Framework.h"
Index: clang/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json
===
--- clang/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json
+++ clang/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json
@@ -6,7 +6,7 @@
 },
 {
   "directory": "DIR",
-  "command": "clang -E DIR/subframework_header_dir_symlink2.m -FInputs/frameworks_symlink -iframework Inputs/frameworks",
+  "command": "clang -E DIR/subframework_header_dir_symlink2.m -FInputs/frameworks -iframework Inputs/frameworks_symlink",
   "file": "DIR/subframework_header_dir_symlink2.m"
 }
 ]
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -1695,7 +1695,7 @@
 // Give the clients a chance to recover.
 SmallString<128> RecoveryPath;
 if (Callbacks->FileNotFound(Filename, RecoveryPath)) {
-  if (auto DE = FileMgr.getDirectory(RecoveryPath)) {
+  if (auto DE = FileMgr.getOptionalDirectoryRef(RecoveryPath)) {
 // Add the recovery path to the list of search paths.
 DirectoryLookup DL(*DE, SrcMgr::C_User, false);
 HeaderInfo.AddSearchPath(DL, isAngled);
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -296,6 +296,7 @@
 /// getName - Return the directory or filename corresponding to this lookup
 /// object.
 StringRef DirectoryLookup::getName() const {
+  // FIXME: Use the name from \c DirectoryEntryRef.
   if (isNormalDir())
 return getDir()->getName();
   if (isFramework())
@@ -496,7 +497,7 @@
 
   // FrameworkName = "/System/Library/Frameworks/"
   SmallString<1024> FrameworkName;
-  FrameworkName += getFrameworkDir()->getName();
+  FrameworkName += getFrameworkDirRef()->getName();
   if (FrameworkName.empty() || FrameworkName.back() != '/')
 FrameworkName.push_back('/');
 
Index: clang/lib/Frontend/InitHeaderSearch.cpp
===
--- clang/lib/Frontend/InitHeaderSearch.cpp
+++ clang/lib/Frontend/InitHeaderSearch.cpp
@@ -148,7 +148,7 @@
   }
 
   // If the directory exists, add it.
-  if (auto DE = FM.getDirectory(MappedPathStr)) {
+  if (auto DE = FM.getOptionalDirectoryRef(MappedPathStr)) {
 IncludePath.push_back(
   std::make_pair(Group, DirectoryLookup(*DE, Type, isFramework)));
 return true;
Index: clang/lib/Basic/FileManager.cpp
===
--- clang/lib/Basic/FileManager.cpp
+++ clang/lib/Basic/FileManager.cpp
@@ -107,16 +107,8 @@
   addAncestorsAsVirtualDirs(DirName);
 }
 
-/// Converts a llvm::ErrorOr to an llvm::ErrorOr by promoting
-/// the address of the inner reference to a pointer or by propagating the error.
-template 
-static llvm::ErrorOr promoteInnerReference(llvm::ErrorOr value) {
-  if (value) return &*value;
-  return value.getError();
-}
-
-llvm::ErrorOr
-FileManager::getDirectory(StringRef DirName, bool CacheFailure) {
+llvm::Expected
+FileManager::getDirectoryRef(StringRef DirName, bool CacheFailure) {
   // stat doesn't like trailing separators except for root 

[PATCH] D66710: ASTReader: Bypass overridden files when reading PCHs

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

dexonsmith closed this revision.
dexonsmith marked an inline comment as done.
dexonsmith added a comment.

r370546.


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

https://reviews.llvm.org/D66710



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

r370546 - ASTReader: Bypass overridden files when reading PCHs

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

Author: dexonsmith
Date: Fri Aug 30 15:59:25 2019
New Revision: 370546

URL: http://llvm.org/viewvc/llvm-project?rev=370546=rev
Log:
ASTReader: Bypass overridden files when reading PCHs

If contents of a file that is part of a PCM are overridden when reading
it, but weren't overridden when the PCM was being built, the ASTReader
will emit an error.  Now it creates a separate FileEntry for recovery,
bypassing the overridden content instead of discarding it.  The
pre-existing testcase clang/test/PCH/remap-file-from-pch.cpp confirms
that the new recovery method works correctly.

This resolves a long-standing FIXME to avoid hypothetically invalidating
another precompiled module that's already using the overridden contents.

This also removes ContentCache-related API that would be unsafe to use
across `CompilerInstance`s in an implicit modules build.  This helps to
unblock us sinking it from SourceManager into FileManager in the future,
which would allow us to delete `InMemoryModuleCache`.

https://reviews.llvm.org/D66710

Modified:
cfe/trunk/include/clang/Basic/FileManager.h
cfe/trunk/include/clang/Basic/SourceManager.h
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/lib/Basic/SourceManager.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/unittests/Basic/FileManagerTest.cpp

Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=370546=370545=370546=diff
==
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Fri Aug 30 15:59:25 2019
@@ -116,6 +116,8 @@ public:
 
   const StringRef getName() const { return Name; }
 
+  bool isValid() const { return Entry->isValid(); }
+
   const FileEntry () const { return *Entry; }
 
   off_t getSize() const { return Entry->getSize(); }
@@ -128,6 +130,13 @@ public:
 
   time_t getModificationTime() const { return Entry->getModificationTime(); }
 
+  friend bool operator==(const FileEntryRef , const FileEntryRef ) {
+return LHS.Entry == RHS.Entry && LHS.Name == RHS.Name;
+  }
+  friend bool operator!=(const FileEntryRef , const FileEntryRef ) {
+return !(LHS == RHS);
+  }
+
 private:
   StringRef Name;
   const FileEntry *Entry;
@@ -158,6 +167,10 @@ class FileManager : public RefCountedBas
   /// The virtual files that we have allocated.
   SmallVector, 4> VirtualFileEntries;
 
+  /// A set of files that bypass the maps and uniquing.  They can have
+  /// conflicting filenames.
+  SmallVector, 0> BypassFileEntries;
+
   /// A cache that maps paths to directory entries (either real or
   /// virtual) we have looked up, or an error that occurred when we looked up
   /// the directory.
@@ -314,6 +327,16 @@ public:
   const FileEntry *getVirtualFile(StringRef Filename, off_t Size,
   time_t ModificationTime);
 
+  /// Retrieve a FileEntry that bypasses VFE, which is expected to be a virtual
+  /// file entry, to access the real file.  The returned FileEntry will have
+  /// the same filename as FE but a different identity and its own stat.
+  ///
+  /// This should be used only for rare error recovery paths because it
+  /// bypasses all mapping and uniquing, blindly creating a new FileEntry.
+  /// There is no attempt to deduplicate these; if you bypass the same file
+  /// twice, you get two new file entries.
+  llvm::Optional getBypassFile(FileEntryRef VFE);
+
   /// Open the specified file as a MemoryBuffer, returning a new
   /// MemoryBuffer if successful, otherwise returning null.
   llvm::ErrorOr>
@@ -353,11 +376,6 @@ public:
   void GetUniqueIDMapping(
 SmallVectorImpl ) const;
 
-  /// Modifies the size and modification time of a previously created
-  /// FileEntry. Use with caution.
-  static void modifyFileEntry(FileEntry *File, off_t Size,
-  time_t ModificationTime);
-
   /// Retrieve the canonical name for a given directory.
   ///
   /// This is a very expensive operation, despite its results being cached,

Modified: cfe/trunk/include/clang/Basic/SourceManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/SourceManager.h?rev=370546=370545=370546=diff
==
--- cfe/trunk/include/clang/Basic/SourceManager.h (original)
+++ cfe/trunk/include/clang/Basic/SourceManager.h Fri Aug 30 15:59:25 2019
@@ -952,11 +952,12 @@ public:
 return false;
   }
 
-  /// Disable overridding the contents of a file, previously enabled
-  /// with #overrideFileContents.
+  /// Bypass the overridden contents of a file.  This creates a new FileEntry
+  /// and initializes the content cache for it.  Returns nullptr if there is no
+  /// such file in the filesystem.
   ///
   /// This should be called before parsing has begun.
-  void 

[PATCH] D67025: Add .inl as valid C++ header type

[Wasim Abbas via Phabricator via cfe-commits]

abbaswasim created this revision.
Herald added subscribers: cfe-commits, kadircet, ilya-biryukov.
Herald added a project: clang.

`clangd` doesn't consider `.inl` a valid C++ source 
(https://github.com/clangd/clangd/issues/16#issuecomment-512942589) this pull 
request adds support for that.

Until the `export` keyword is supported by compilers mentioned 
https://stackoverflow.com/questions/5416872/using-export-keyword-with-templates 
a common way of keeping header files cleaner is to move template definitions 
into `.inl` files. This seems to be a common practice. Some evidence of it 
being commonly used for this purpose.

https://www.randygaul.net/2015/01/31/c-keyword-inline-and-inl-files/
https://drake.mit.edu/cxx_inl.html
https://www.codeproject.com/Articles/48575/How-to-define-a-template-class-in-a-h-file-and-imp
https://stackoverflow.com/questions/31949731/very-confused-on-template-inl-file
even brdf-loader https://github.com/rgl-epfl/brdf-loader

Some other ways of separating definitions is 
https://github.com/RobotLocomotion/drake/issues/3141 where they say you should 
use .inl or .impl or -inl.h or -impl.h and this might be ok but then things 
like projectile can't find it because that only works with extensions.

One caveat of this is, the above methods of using `.inl` files means its not a 
complete translation unit so `clangd` still can't fully compile it so one has 
to add `#include "header.hpp"` at the top of `.inl` files to make it work but 
thats not a big deal.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67025

Files:
  clang/lib/Driver/Types.cpp


Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -237,6 +237,7 @@
.Case("cp", TY_CXX)
.Case("cu", TY_CUDA)
.Case("hh", TY_CXXHeader)
+   .Case("inl", TY_CXXHeader)
.Case("ii", TY_PP_CXX)
.Case("ll", TY_LLVM_IR)
.Case("mi", TY_PP_ObjC)


Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -237,6 +237,7 @@
.Case("cp", TY_CXX)
.Case("cu", TY_CUDA)
.Case("hh", TY_CXXHeader)
+   .Case("inl", TY_CXXHeader)
.Case("ii", TY_PP_CXX)
.Case("ll", TY_LLVM_IR)
.Case("mi", TY_PP_ObjC)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

[Richard Smith - zygoloid 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 rL370544: [c++20] Implement semantic restrictions for C++20 
designated (authored by rsmith, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59754?vs=195611=218179#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59754

Files:
  cfe/trunk/include/clang/Basic/DiagnosticGroups.td
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/include/clang/Sema/Sema.h
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/lib/Sema/SemaInit.cpp
  cfe/trunk/lib/Sema/SemaOverload.cpp
  cfe/trunk/lib/Sema/TreeTransform.h
  cfe/trunk/test/Analysis/globals.cpp
  cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp
  cfe/trunk/test/CodeGenObjCXX/designated-initializers.mm
  cfe/trunk/test/PCH/cxx1y-default-initializer.cpp
  cfe/trunk/test/Parser/cxx0x-lambda-expressions.cpp
  cfe/trunk/test/Parser/objc-init.m
  cfe/trunk/test/Sema/designated-initializers.c
  cfe/trunk/test/Sema/static-assert.c
  cfe/trunk/test/SemaCXX/aggregate-initialization.cpp
  cfe/trunk/test/SemaCXX/c99.cpp
  cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
  cfe/trunk/test/SemaCXX/constexpr-printing.cpp
  cfe/trunk/test/SemaCXX/cxx0x-initializer-constructor.cpp
  cfe/trunk/test/SemaCXX/cxx2a-initializer-aggregates.cpp
  cfe/trunk/test/SemaCXX/decltype.cpp
  cfe/trunk/test/SemaCXX/designated-initializers-base-class.cpp
  cfe/trunk/test/SemaCXX/designated-initializers.cpp
  cfe/trunk/test/SemaCXX/eval-crashes.cpp
  cfe/trunk/test/SemaCXX/member-init.cpp
  cfe/trunk/test/SemaObjCXX/message.mm
  cfe/trunk/test/SemaTemplate/instantiate-c99.cpp
  cfe/trunk/test/SemaTemplate/instantiate-init.cpp

Index: cfe/trunk/include/clang/Sema/Sema.h
===
--- cfe/trunk/include/clang/Sema/Sema.h
+++ cfe/trunk/include/clang/Sema/Sema.h
@@ -2775,6 +2775,9 @@
  Expr *Value,
  bool AllowNRVO = true);
 
+  bool CanPerformAggregateInitializationForOverloadResolution(
+  const InitializedEntity , InitListExpr *From);
+
   bool CanPerformCopyInitialization(const InitializedEntity ,
 ExprResult Init);
   ExprResult PerformCopyInitialization(const InitializedEntity ,
@@ -4636,6 +4639,10 @@
MultiExprArg InitArgList,
SourceLocation RBraceLoc);
 
+  ExprResult BuildInitList(SourceLocation LBraceLoc,
+   MultiExprArg InitArgList,
+   SourceLocation RBraceLoc);
+
   ExprResult ActOnDesignatedInitializer(Designation ,
 SourceLocation Loc,
 bool GNUSyntax,
Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -154,7 +154,7 @@
 
 // C99 Designated Initializers
 def ext_designated_init : Extension<
-  "designated initializers are a C99 feature">, InGroup;
+  "designated initializers are a C99 feature">, InGroup;
 def err_array_designator_negative : Error<
   "array designator value '%0' is negative">;
 def err_array_designator_empty_range : Error<
@@ -173,15 +173,17 @@
 def note_field_designator_found : Note<"field designator refers here">;
 def err_designator_for_scalar_init : Error<
   "designator in initializer for scalar type %0">;
-def warn_subobject_initializer_overrides : Warning<
-  "subobject initialization overrides initialization of other fields "
-  "within its enclosing subobject">, InGroup;
 def warn_initializer_overrides : Warning<
-  "initializer overrides prior initialization of this subobject">,
-  InGroup;
+  "initializer %select{partially |}0overrides prior initialization of "
+  "this subobject">, InGroup;
+def ext_initializer_overrides : ExtWarn,
+  InGroup, SFINAEFailure;
+def err_initializer_overrides_destructed : Error<
+  "initializer would partially override prior initialization of object of "
+  "type %1 with non-trivial destruction">;
 def note_previous_initializer : Note<
   "previous initialization %select{|with side effects }0is here"
-  "%select{| (side effects may not occur at run time)}0">;
+  "%select{| (side effects will not occur at run time)}0">;
 def err_designator_into_flexible_array_member : Error<
   "designator into flexible array member subobject">;
 def note_flexible_array_member : Note<
@@ -189,6 +191,28 @@
 def ext_flexible_array_init : Extension<
   "flexible array initialization is a GNU extension">, InGroup;
 
+// C++20 designated initializers
+def 

[PATCH] D66834: Driver tests: set `--sysroot=""` to support clang with `DEFAULT_SYSROOT`

[Sergej Jaskiewicz via Phabricator via cfe-commits]

broadwaylamb added a comment.

In D66834#1652756 , @compnerd wrote:

> I think that this is pretty easy to forget.  Fortunately, last argument wins. 
>  Why not sink this into the `%clang` substitution in lit?  That ensures that 
> we run with an empty sysroot and then when the test needs to adjust the 
> sysroot, it can do so explicitly.


This totally makes sense.

I've just tried to do it, but unfortunately some tests are failing, for 
example, `Driver/cc1-response-files.c`. The problem is that `%clang` is 
expanded to `/path/to/clang --sysroot=`, but the succeeding flags (such as 
`-cc1`) may be incompatible with `--sysroot`.

Or am I missing something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66834



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

r370544 - [c++20] Implement semantic restrictions for C++20 designated

[Richard Smith via cfe-commits]

Author: rsmith
Date: Fri Aug 30 15:52:55 2019
New Revision: 370544

URL: http://llvm.org/viewvc/llvm-project?rev=370544=rev
Log:
[c++20] Implement semantic restrictions for C++20 designated
initializers.

This has some interesting interactions with our existing extensions to
support C99 designated initializers as an extension in C++. Those are
resolved as follows:

 * We continue to permit the full breadth of C99 designated initializers
   in C++, with the exception that we disallow a partial overwrite of an
   initializer with a non-trivially-destructible type. (Full overwrite
   is OK, because we won't run the first initializer at all.)

 * The C99 extensions are disallowed in SFINAE contexts and during
   overload resolution, where they could change the meaning of valid
   programs.

 * C++20 disallows reordering of initializers. We only check for that for
   the simple cases that the C++20 rules permit (designators of the form
   '.field_name =' and continue to allow reordering in other cases).
   It would be nice to improve this behavior in future.

 * All C99 designated initializer extensions produce a warning by
   default in C++20 mode. People are going to learn the C++ rules based
   on what Clang diagnoses, so it's important we diagnose these properly
   by default.

 * In C++ <= 17, we apply the C++20 rules rather than the C99 rules, and
   so still diagnose C99 extensions as described above. We continue to
   accept designated C++20-compatible initializers in C++ <= 17 silently
   by default (but naturally still reject under -pedantic-errors).

This is not a complete implementation of P0329R4. In particular, that
paper introduces new non-C99-compatible syntax { .field { init } }, and
we do not support that yet.

This is based on a previous patch by Don Hinton, though I've made
substantial changes when addressing the above interactions.

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/test/Analysis/globals.cpp
cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp
cfe/trunk/test/CodeGenObjCXX/designated-initializers.mm
cfe/trunk/test/PCH/cxx1y-default-initializer.cpp
cfe/trunk/test/Parser/cxx0x-lambda-expressions.cpp
cfe/trunk/test/Parser/objc-init.m
cfe/trunk/test/Sema/designated-initializers.c
cfe/trunk/test/Sema/static-assert.c
cfe/trunk/test/SemaCXX/aggregate-initialization.cpp
cfe/trunk/test/SemaCXX/c99.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx11.cpp
cfe/trunk/test/SemaCXX/constexpr-printing.cpp
cfe/trunk/test/SemaCXX/cxx0x-initializer-constructor.cpp
cfe/trunk/test/SemaCXX/cxx2a-initializer-aggregates.cpp
cfe/trunk/test/SemaCXX/decltype.cpp
cfe/trunk/test/SemaCXX/designated-initializers-base-class.cpp
cfe/trunk/test/SemaCXX/designated-initializers.cpp
cfe/trunk/test/SemaCXX/eval-crashes.cpp
cfe/trunk/test/SemaCXX/member-init.cpp
cfe/trunk/test/SemaObjCXX/message.mm
cfe/trunk/test/SemaTemplate/instantiate-c99.cpp
cfe/trunk/test/SemaTemplate/instantiate-init.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=370544=370543=370544=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Fri Aug 30 15:52:55 2019
@@ -100,7 +100,6 @@ def GNUComplexInteger : DiagGroup<"gnu-c
 def GNUConditionalOmittedOperand : 
DiagGroup<"gnu-conditional-omitted-operand">;
 def ConfigMacros : DiagGroup<"config-macros">;
 def : DiagGroup<"ctor-dtor-privacy">;
-def GNUDesignator : DiagGroup<"gnu-designator">;
 def GNUStringLiteralOperatorTemplate :
   DiagGroup<"gnu-string-literal-operator-template">;
 def UndefinedVarTemplate : DiagGroup<"undefined-var-template">;
@@ -146,6 +145,12 @@ def Deprecated : DiagGroup<"deprecated",
   DeprecatedWritableStr]>,
  DiagCategory<"Deprecations">;
 
+def CXX2aDesignator : DiagGroup<"c++20-designator">;
+// Allow -Wno-c99-designator to be used to turn off all warnings on valid C99
+// designators (including the warning controlled by -Wc++20-designator).
+def C99Designator : DiagGroup<"c99-designator", [CXX2aDesignator]>;
+def GNUDesignator : DiagGroup<"gnu-designator">;
+
 def DynamicExceptionSpec
 : DiagGroup<"dynamic-exception-spec", [DeprecatedDynamicExceptionSpec]>;
 
@@ -896,7 +901,7 @@ def CXX17 : DiagGroup<"c++17-extensions"
 
 // A warning group for warnings about using C++2a features as extensions in
 // earlier C++ versions.
-def CXX2a : 

[PATCH] D67023: Diagnose use of ATOMIC_VAR_INIT

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked an inline comment as done.
aaron.ballman added inline comments.



Comment at: lib/Lex/PPMacroExpansion.cpp:523
+  llvm::sys::path::filename(getSourceManager().getFilename(
+  M.getLocalDirective()->getLocation())) == "stdatomic.h") {
+Diag(Identifier, diag::warn_pp_macro_deprecated)

rsmith wrote:
> This assumes there is a local directive for the macro. There need not be (it 
> could be imported from a module). We should presumably check to see if all 
> active macro directives are from that header instead.
I'm not familiar with how that's done -- is there some existing code doing 
something similar I should model off of? Also, what would the test case look 
like for that scenario?


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

https://reviews.llvm.org/D67023



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

[PATCH] D67023: Diagnose use of ATOMIC_VAR_INIT

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

rsmith added inline comments.



Comment at: lib/Lex/PPMacroExpansion.cpp:523
+  llvm::sys::path::filename(getSourceManager().getFilename(
+  M.getLocalDirective()->getLocation())) == "stdatomic.h") {
+Diag(Identifier, diag::warn_pp_macro_deprecated)

This assumes there is a local directive for the macro. There need not be (it 
could be imported from a module). We should presumably check to see if all 
active macro directives are from that header instead.


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

https://reviews.llvm.org/D67023



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

[PATCH] D66572: [analyzer] NFC: BugReporter Separation Ep.I.

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:75
 /// individual bug reports.
 class BugReport : public llvm::ilist_node {
 public:

NoQ wrote:
> gribozavr wrote:
> > Szelethus wrote:
> > > Shouldn't we make this an abstract class?
> > I'm not sure that intrusive linked list is the right data structure for the 
> > job. I'd personally put bug reports into a vector and make a custom data 
> > structure if a vector becomes a performance problem.
> > Shouldn't we make this an abstract class?
> 
> Mmm, yeah, moved virtual methods from `BugReport` to `BasicBugReport` 
> whenever `PathSensitiveBugReport` immediately overrides them and it looks 
> much better now!
> 
> > I'm not sure that intrusive linked list is the right data structure for the 
> > job.
> 
> Me neither, but it seems to be orthogonal to this patch, and this patch is 
> already huge, so i'll do this in a follow-up patch, ok?
D67024


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

https://reviews.llvm.org/D66572



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

[PATCH] D66999: [ASTImporter][NFC] Add comments to code where we cannot use HandleNameConflict

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:3454
 << FoundField->getType();
-
+  // This case is not handled with HandleNameConflict, because by the time
+  // when we reach here, the enclosing class is already imported. If there

Do we have an example that hits this case?



Comment at: clang/lib/AST/ASTImporter.cpp:5262
+  // template.
+  // FIXME Perhaps return with a different error?
   return make_error(ImportError::NameConflict);

I believe this is ill-formed no diagnostic required. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66999



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

[PATCH] D67024: [analyzer] NFC: Replace intrusive list of bug reports with a vector of pointers.

[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, dylanmckay.
Herald added a project: clang.

They are said to be more efficient and they're definitely less brain-damaging. 
Suggested  by 
@gribozavr.


Repository:
  rC Clang

https://reviews.llvm.org/D67024

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -3004,8 +3004,8 @@
 llvm::make_range(BR.EQClasses_begin(), BR.EQClasses_end());
 
 for (const auto  : EQClasses) {
-  for (const BugReport  : EQ) {
-const auto *PR = dyn_cast();
+  for (const auto  : EQ) {
+const auto *PR = dyn_cast(I.get());
 if (!PR)
   continue;
 const ExplodedNode *EN = PR->getErrorNode();
@@ -3135,7 +3135,7 @@
 // Iterate through the reports and get their nodes.
 for (BugReporter::EQClasses_iterator
EI = BR.EQClasses_begin(), EE = BR.EQClasses_end(); EI != EE; ++EI) {
-  const auto *R = dyn_cast(&*EI->begin());
+  const auto *R = dyn_cast(EI->begin()->get());
   if (!R)
 continue;
   const auto *N = const_cast(R->getErrorNode());
Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2800,17 +2800,17 @@
 
 BugReport *PathSensitiveBugReporter::findReportInEquivalenceClass(
 BugReportEquivClass , SmallVectorImpl ) {
-  BugReportEquivClass::iterator I = EQ.begin(), E = EQ.end();
+  auto I = EQ.begin(), E = EQ.end();
   assert(I != E);
-  const BugType& BT = I->getBugType();
+  const BugType& BT = (*I)->getBugType();
 
   // If we don't need to suppress any of the nodes because they are
   // post-dominated by a sink, simply add all the nodes in the equivalence class
   // to 'Nodes'.  Any of the reports will serve as a "representative" report.
   if (!BT.isSuppressOnSink()) {
-BugReport *R = &*I;
-for (auto  : EQ) {
-  if (auto *PR = dyn_cast()) {
+BugReport *R = I->get();
+for (auto  : EQ) {
+  if (auto *PR = dyn_cast(J.get())) {
 R = PR;
 bugReports.push_back(PR);
   }
@@ -2827,7 +2827,7 @@
   BugReport *exampleReport = nullptr;
 
   for (; I != E; ++I) {
-auto *R = dyn_cast(&*I);
+auto *R = dyn_cast(I->get());
 if (!R)
   continue;
 
Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
===
--- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -72,7 +72,7 @@
 
 /// This class provides an interface through which checkers can create
 /// individual bug reports.
-class BugReport : public llvm::ilist_node {
+class BugReport {
 public:
   using ranges_iterator = const SourceRange *;
   using NoteList = SmallVector, 4>;
@@ -454,11 +454,18 @@
 class BugReportEquivClass : public llvm::FoldingSetNode {
   friend class BugReporter;
 
+public:
+  typedef llvm::SmallVector, 4> ReportList;
+  using iterator = ReportList::iterator;
+  using const_iterator = ReportList::const_iterator;
+
+
+private:
   /// List of *owned* BugReport objects.
-  llvm::ilist Reports;
+  ReportList Reports;
 
-  void AddReport(std::unique_ptr R) {
-Reports.push_back(R.release());
+  void AddReport(std::unique_ptr &) {
+Reports.push_back(std::move(R));
   }
 
 public:
@@ -466,12 +473,9 @@
 
   void Profile(llvm::FoldingSetNodeID& ID) const {
 assert(!Reports.empty());
-Reports.front().Profile(ID);
+Reports.front()->Profile(ID);
   }
 
-  using iterator = llvm::ilist::iterator;
-  using const_iterator = llvm::ilist::const_iterator;
-
   iterator begin() { return Reports.begin(); }
   iterator end() { return Reports.end(); }
 
@@ -563,7 +567,7 @@
   virtual BugReport *
   findReportInEquivalenceClass(BugReportEquivClass ,
SmallVectorImpl ) {
-return &*eqClass.begin();
+return eqClass.begin()->get();
   }
 
 protected:
___
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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Missing tests for `__builtin_unpredictable()`.




Comment at: clang/lib/CodeGen/CodeGenAction.cpp:626
+  unsigned Line, Column;
+  bool BadDebugInfo = false;
+  FullSourceLoc Loc =

paulkirth wrote:
> lebedev.ri wrote:
> > This should be `BadDebugInfoLoc` (in `getBestLocationFromDebugLoc()` too)
> That should be in a separate patch, right?
Yeah, let's keep that for later.



Comment at: clang/lib/CodeGen/CodeGenAction.cpp:369
 void UnsupportedDiagHandler(const llvm::DiagnosticInfoUnsupported );
+void MisExpectDiagHandler(const llvm::DiagnosticInfoMisExpect );
 /// Specialized handlers for optimization remarks.

Comment missing



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3449
 
+  if (Diags.isIgnored(diag::warn_profile_data_misexpect, SourceLocation()))
+Res.FrontendOpts.LLVMArgs.push_back("-pgo-warn-misexpect");

Err, this works? I'd expect `!` to be missing here..



Comment at: llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp:78-87
+  SI.setMetadata(
+  LLVMContext::MD_misexpect,
+  MDBuilder(CI->getContext())
+  .createMisExpect(Index, LikelyBranchWeight, UnlikelyBranchWeight));
+
+  SI.setCondition(ArgValue);
+  misexpect::checkClangInstrumentation(SI);

paulkirth wrote:
> lebedev.ri wrote:
> > Why can't `LLVMContext::MD_prof` be reused?
> It didn't seem appropriate to me, but that can be changed if it is the right 
> thing to do. 
> 
> However, I don't see code elsewhere that loops over `MD_prof` metadata 
> checking its tag. I see lots of examples that check if the tag matches, and 
> then exits if it doesn't match. 
> 
> If I added "misexpect" as a new `MD_prof`tag, then code in places like 
> `extractProfTotalWeight(...)` in IR/Metadata.cpp would have to be updated. 
> I'm not sure how pervasive that pattern is, but I did  want to avoid 
> introducing systemic changes like that.
> 
> https://github.com/llvm/llvm-project/blob/9976a5bc1db5a240378a61a68c0e182060bcb554/llvm/lib/IR/Metadata.cpp#L1336
That's not exactly what i was asking.
I'm specifically talking about how `"misexpect"` and `"branch_weigths"` carry 
basically the same data,
are set basically in the same place always, with the difference that 
`"misexpect"` also has the switch case index.
This is both somewhat bloaty, and is prone to getting out of sync.

I have two questions:
1. Can `"misexpect"` metadata simply not be invented, but can existing 
`LLVMContext::MD_misexpect` simply be used?
2. Can `"misexpect"` be changed to not store the weights, but a reference to 
the `LLVMContext::MD_misexpect` metadata that will be emitted anyway?




Comment at: llvm/test/Transforms/LowerExpectIntrinsic/basic.ll:141
   %tobool = icmp ne i64 %expval, 0
-; CHECK: !prof !1
+; CHECK: !prof !2
 ; CHECK-NOT: @llvm.expect

Here and elsewhere:
is `!prof !1` no longer present?



Comment at: llvm/test/Transforms/LowerExpectIntrinsic/basic.ll:292-293
+; CHECK: !3 = !{!"misexpect", i64 1, i64 2000, i64 1}
+; CHECK: !4 = !{!"branch_weights", i32 1, i32 2000, i32 1}
+; CHECK: !5 = !{!"branch_weights", i32 2000, i32 1, i32 1}

Should there be a FIXME, are some `misexpect` missing here?


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] D65239: [analyzer] RangeConstraintManager: Apply constraint ranges of bitwise operations

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:488-489
+static ProgramStateRef applyBitwiseRanges(ProgramStateRef State,
+  BasicValueFactory ,
+  RangeSet::Factory , RangeSet RS,
+  SymbolRef Sym) {

I recommend making this method non-static, so that not to have to pass BV and F 
around.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:510
+  const SymExpr *CurrentSym = SIE->getLHS();
+  if (const RangeSet *CurrentRS = Constraints.lookup(CurrentSym)) {
+const RangeSet NewRS = assumeNonZero(BV, F, CurrentSym, *CurrentRS);

If there's no current range in the map, it doesn't mean that there's no current 
range at all. Instead it means that the symbol is completely unconstrained and 
its range is equal to the whole domain of the type's possible values. You 
should use `RangeConstraintManager::getRange()` instead to retrieve the 
"default" range for the symbol. It would also always exist, so no need to check.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:517
+if (!NewRS.isEmpty()) {
+  State = State->set(CurrentSym, NewRS);
+} else {

Aaand at this point you might as well call `applyBitwiseRanges()` recursively. 
Or even call `assume()` recursively. This will be the better way to implement 
your loop idea, because now it's gonna be correct on all recursion depth levels.


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

https://reviews.llvm.org/D65239



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

[PATCH] D67023: Diagnose use of ATOMIC_VAR_INIT

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, jyknight, joerg.
Herald added a subscriber: jfb.

The `ATOMIC_VAR_INIT` was deprecated in C17 (C17 7.31.8p2), largely because it 
is fundamentally broken. It has already been proposed to be removed from C2x 
(http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2390.pdf). There is no 
replacement for the macro because the macro is unnecessary for implementations 
to get initialization correct (plain assignment suffices).

This patch adds a diagnostic when expanding the `ATOMIC_VAR_INIT` macro that 
comes from stdatomic.h in all C modes. This will help users to recognize that 
use of this macro is strongly discouraged.


https://reviews.llvm.org/D67023

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Lex/PPMacroExpansion.cpp
  test/Preprocessor/Inputs/atomic_var_init/stdatomic.h
  test/Preprocessor/atomic_var_init.c


Index: test/Preprocessor/atomic_var_init.c
===
--- test/Preprocessor/atomic_var_init.c
+++ test/Preprocessor/atomic_var_init.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -Wdeprecated -isystem 
Inputs/atomic_var_init
+
+// Do not diagnose if the macro is not defined in stdatomic.h because it could
+// be a user-defined macro instead then.
+#define ATOMIC_VAR_INIT(x) (x)
+_Atomic int a = ATOMIC_VAR_INIT(12);
+#undef ATOMIC_VAR_INIT
+
+// Diagnose use of the macro from stdatomic.h
+#include 
+_Atomic int i = ATOMIC_VAR_INIT(12); // expected-warning {{'ATOMIC_VAR_INIT' 
is deprecated}}
+
+#define FOO(x) ATOMIC_VAR_INIT(x)
+_Atomic int j = FOO(12); // expected-warning {{'ATOMIC_VAR_INIT' is 
deprecated}}
+
+// Do not diagnose use in conditional inclusion directives.
+#ifdef ATOMIC_VAR_INIT
+#elif defined(ATOMIC_VAR_INIT)
+#endif
+
Index: test/Preprocessor/Inputs/atomic_var_init/stdatomic.h
===
--- test/Preprocessor/Inputs/atomic_var_init/stdatomic.h
+++ test/Preprocessor/Inputs/atomic_var_init/stdatomic.h
@@ -0,0 +1 @@
+#define ATOMIC_VAR_INIT(x) (x)
Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ lib/Lex/PPMacroExpansion.cpp
@@ -513,6 +513,18 @@
   // Notice that this macro has been used.
   markMacroAsUsed(MI);
 
+  // C17 7.31.8p2
+  // If the macro is named ATOMIC_VAR_INIT and it came from the stdatomic.h
+  // header file, diagnose it as being deprecated. This macro does not exist
+  // in C++, so do not trigger a deprecation warning in that language mode.
+  if (!getLangOpts().CPlusPlus &&
+  Identifier.getIdentifierInfo()->isStr("ATOMIC_VAR_INIT") &&
+  llvm::sys::path::filename(getSourceManager().getFilename(
+  M.getLocalDirective()->getLocation())) == "stdatomic.h") {
+Diag(Identifier, diag::warn_pp_macro_deprecated)
+<< Identifier.getIdentifierInfo();
+  }
+
   // Remember where the token is expanded.
   SourceLocation ExpandLoc = Identifier.getLocation();
   SourceRange ExpansionRange(ExpandLoc, ExpansionEnd);
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -326,6 +326,8 @@
 def warn_pp_objc_macro_redef_ignored : Warning<
   "ignoring redefinition of Objective-C qualifier macro">,
   InGroup>;
+def warn_pp_macro_deprecated : Warning<"%0 is deprecated">,
+  InGroup;
 
 def pp_invalid_string_literal : Warning<
   "invalid string literal, ignoring final '\\'">;


Index: test/Preprocessor/atomic_var_init.c
===
--- test/Preprocessor/atomic_var_init.c
+++ test/Preprocessor/atomic_var_init.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -Wdeprecated -isystem Inputs/atomic_var_init
+
+// Do not diagnose if the macro is not defined in stdatomic.h because it could
+// be a user-defined macro instead then.
+#define ATOMIC_VAR_INIT(x) (x)
+_Atomic int a = ATOMIC_VAR_INIT(12);
+#undef ATOMIC_VAR_INIT
+
+// Diagnose use of the macro from stdatomic.h
+#include 
+_Atomic int i = ATOMIC_VAR_INIT(12); // expected-warning {{'ATOMIC_VAR_INIT' is deprecated}}
+
+#define FOO(x) ATOMIC_VAR_INIT(x)
+_Atomic int j = FOO(12); // expected-warning {{'ATOMIC_VAR_INIT' is deprecated}}
+
+// Do not diagnose use in conditional inclusion directives.
+#ifdef ATOMIC_VAR_INIT
+#elif defined(ATOMIC_VAR_INIT)
+#endif
+
Index: test/Preprocessor/Inputs/atomic_var_init/stdatomic.h
===
--- test/Preprocessor/Inputs/atomic_var_init/stdatomic.h
+++ test/Preprocessor/Inputs/atomic_var_init/stdatomic.h
@@ -0,0 +1 @@
+#define ATOMIC_VAR_INIT(x) (x)
Index: lib/Lex/PPMacroExpansion.cpp
===
--- lib/Lex/PPMacroExpansion.cpp
+++ 

[PATCH] D66714: [analyzer] Don't run the analyzer for -analyzer-list-enabled-checkers

[Artem Dergachev via Phabricator via cfe-commits]

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

You can probably write a test for it via `-analyzer-display-progress`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66714



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

[PATCH] D59637: [analyzer] Use the custom propagation rules and sinks in GenericTaintChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I have a few immediate style issues but otherwise it looks good :)




Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:237
 
-const char GenericTaintChecker::MsgUncontrolledFormatString[] =
+const llvm::StringLiteral GenericTaintChecker::MsgUncontrolledFormatString =
 "Untrusted data is used as a format string "

`StringLiteral`s need to always be declared as `constexpr`.



Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:251
+const llvm::StringLiteral GenericTaintChecker::MsgCustomSink =
+"Untrusted data is passed to a user defined sink";
+;

It should be "user-defined" because it's a single adjective.

I recommend against using the word "sink" in user-facing messages because it's 
too jargony. Do we have a better word for this?



Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:252
+"Untrusted data is passed to a user defined sink";
+;
 } // end of anonymous namespace

This semicolon looks unnecessary.



Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:595-596
+  StringRef Name = C.getCalleeName(C.getCalleeDecl(CE));
+  llvm::errs() << "Skip out of bound SrcArg: " << Name << ":" << ArgNum
+   << '\n';
+  continue;

These debug prints should not land.



Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:840
+
+  const GenericTaintChecker::ArgVector  = It->getValue();
+  for (unsigned ArgNum : Args) {

The `GenericTaintChecker::` qualifier is unnecessary.


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

https://reviews.llvm.org/D59637



___
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

[Petr Hosek via Phabricator via cfe-commits]

phosek accepted this revision.
phosek added a comment.

In D66324#1653198 , @paulkirth wrote:

> In D66324#1652364 , @lebedev.ri 
> wrote:
>
> > Trying to start reviewing this.
> >  The llvm side of the patch is self contained; clang patch should be split 
> > into a dependent review.
>
>
> Looking at this, is it necessary to split up the patch? That will loose a lot 
> of the previous comment/context, and I'd prefer to land this as a single 
> change.  Doing so has the benefit that if my patch needs to be reverted, it 
> can be done all together.  For example, if only the llvm patch is reverted, 
> then it will likely cause other problems and make triage needlessly painful.


I'd actually prefer to land this as a single change, it makes it easier to 
revert and look at the diff later.


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] D59637: [analyzer] Use the custom propagation rules and sinks in GenericTaintChecker

[Borsik Gábor via Phabricator via cfe-commits]

boga95 added a comment.

Should I do anything or it is ready?


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

https://reviews.llvm.org/D59637



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

[PATCH] D66983: [WebAssembly] Add wasm-specific vector shuffle builtin and intrinsic

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

The context for this CL is 
https://github.com/emscripten-core/emscripten/issues/9340. The code that does 
the undesirable optimization is around 
llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:18776. I think it is a reasonable 
assumption that what you put in is what you get out, especially if you're 
trying to trigger particular WebAssembly engine behavior that the backend 
should not really be reasoning about. Now perhaps users should not be reasoning 
about it either, but given that they are I think this patch is reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66983



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

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

> IIRC, when we rolled out `-Wstrict-prototypes` we explicitly excluded this 
> case since it hit a lot of code without finding bugs.

I'm not a historian, but I believe this was suggested by @rsmith in 
https://bugs.llvm.org/show_bug.cgi?id=20796#c8, and I think we agreed in IRC 
yesterday that this should in fact warn. As I wrote in the commit message, the 
standard isn't terribly clear on what a prototype is, but we already treat it 
as non-prototype and that's very likely correct.

There are no mentions of too many false positives in the bug report.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66919



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

[PATCH] D64146: [Clang Interpreter] Initial patch for the constexpr interpreter

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

Seems like this patch introduced a cyclic dependency between Basic and AST when 
building with LLVM_ENABLE_MODULES=On:

  While building module 'Clang_AST' imported from 
llvm/lldb/include/lldb/Symbol/ClangUtil.h:14:
  While building module 'Clang_Basic' imported from 
llvm/llvm/../clang/include/clang/AST/Stmt.h:18:
  In file included from :73:
  clang/include/clang/Basic/OptionalDiagnostic.h:17:10: fatal error: cyclic 
dependency in module 'Clang_AST': Clang_AST -> Clang_Basic -> Clang_AST
  #include "clang/AST/APValue.h"


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64146



___
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 218171.
paulkirth added a comment.

Fix comment for misexpect option


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/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-stripped.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 the llvm.expect intrinsic: 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 the llvm.expect intrinsic: 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 the llvm.expect intrinsic: Annotation was correct on 0.00% of profiled executions.
+
+; DISABLED-NOT: warning: misexpect-switch.c:26:5: 0.00%
+; 

[PATCH] D67019: [analyzer] pr43179: CallDescription: Check number of parameters as well as number of arguments.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

For the actual variadic functions i'm thinking of something like 
`{CDF_Variadic, "fprintf", 2}` which will match all calls to variadic functions 
named "fprintf" that have //exactly// two non-variadic parameters and //at 
least// two arguments on the call site [which occupy those non-variadic 
parameters]. I'm not implementing it yet because we don't need it yet.

I'm not sure if that's the best syntax. I didn't think too deeply about it 
because i'm waiting for more examples to generalize upon.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67019



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

[PATCH] D67020: [WebAssembly] Add SIMD QFMA/QFMS

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added a comment.

Could you point to the spec of these instructions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67020



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

r370535 - Revert [Clang Interpreter] Initial patch for the constexpr interpreter

[Nandor Licker via cfe-commits]

Author: nand
Date: Fri Aug 30 14:32:00 2019
New Revision: 370535

URL: http://llvm.org/viewvc/llvm-project?rev=370535=rev
Log:
Revert [Clang Interpreter] Initial patch for the constexpr interpreter

This reverts r370531 (git commit d4c1002e0ab50f6891cdd2f5bd3a8f3a3584)

Removed:
cfe/trunk/docs/ConstantInterpreter.rst
cfe/trunk/include/clang/Basic/OptionalDiagnostic.h
cfe/trunk/lib/AST/Interp/
cfe/trunk/test/AST/Interp/
cfe/trunk/utils/TableGen/ClangOpcodesEmitter.cpp
Modified:
cfe/trunk/docs/index.rst
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/CMakeLists.txt
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp
cfe/trunk/test/SemaCXX/constexpr-many-arguments.cpp
cfe/trunk/test/SemaCXX/shift.cpp
cfe/trunk/utils/TableGen/CMakeLists.txt
cfe/trunk/utils/TableGen/TableGen.cpp
cfe/trunk/utils/TableGen/TableGenBackends.h

Removed: cfe/trunk/docs/ConstantInterpreter.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ConstantInterpreter.rst?rev=370534=auto
==
--- cfe/trunk/docs/ConstantInterpreter.rst (original)
+++ cfe/trunk/docs/ConstantInterpreter.rst (removed)
@@ -1,194 +0,0 @@
-
-Constant Interpreter
-
-
-.. contents::
-   :local:
-
-Introduction
-
-
-The constexpr interpreter aims to replace the existing tree evaluator in 
clang, improving performance on constructs which are executed inefficiently by 
the evaluator. The interpreter is activated using the following flags:
-
-* ``-fexperimental-new-constant-interpreter`` enables the interpreter, falling 
back to the evaluator for unsupported features
-* ``-fforce-experimental-new-constant-interpreter`` forces the use of the 
interpreter, bailing out if an unsupported feature is encountered
-
-Bytecode Compilation
-
-
-Bytecode compilation is handled in ``ByteCodeStmtGen.h`` for statements and 
``ByteCodeExprGen.h`` for expressions. The compiler has two different backends: 
one to generate bytecode for functions (``ByteCodeEmitter``) and one to 
directly evaluate expressions as they are compiled, without generating bytecode 
(``EvalEmitter``). All functions are compiled to bytecode, while toplevel 
expressions used in constant contexts are directly evaluated since the bytecode 
would never be reused. This mechanism aims to pave the way towards replacing 
the evaluator, improving its performance on functions and loops, while being 
just as fast on single-use toplevel expressions.
-
-The interpreter relies on stack-based, strongly-typed opcodes. The glue logic 
between the code generator, along with the enumeration and description of 
opcodes, can be found in ``Opcodes.td``. The opcodes are implemented as generic 
template methods in ``Interp.h`` and instantiated with the relevant primitive 
types by the interpreter loop or by the evaluating emitter.
-
-Primitive Types

-
-* ``PT_{U|S}int{8|16|32|64}``
-
-  Signed or unsigned integers of a specific bit width, implemented using the 
```Integral``` type.
-
-* ``PT_{U|S}intFP``
-
-  Signed or unsigned integers of an arbitrary, but fixed width used to 
implement
-  integral types which are required by the target, but are not supported by 
the host.
-  Under the hood, they rely on APValue. The ``Integral`` specialisation for 
these
-  types is required by opcodes to share an implementation with fixed integrals.
-
-* ``PT_Bool``
-
-  Representation for boolean types, essentially a 1-bit unsigned ``Integral``.
-
-* ``PT_RealFP``
-
-  Arbitrary, but fixed precision floating point numbers. Could be specialised 
in
-  the future similarly to integers in order to improve floating point 
performance.
-
-* ``PT_Ptr``
-
-  Pointer type, defined in ``"Pointer.h"``.
-
-* ``PT_FnPtr``
-
-  Function pointer type, can also be a null function pointer. Defined in 
``"Pointer.h"``.
-
-* ``PT_MemPtr``
-
-  Member pointer type, can also be a null member pointer. Defined in 
``"Pointer.h"``
-
-Composite types

-
-The interpreter distinguishes two kinds of composite types: arrays and 
records. Unions are represented as records, except a single field can be marked 
as active. The contents of inactive fields are kept until they
-are reactivated and overwritten.
-
-
-Bytecode Execution
-==
-
-Bytecode is executed using a stack-based interpreter. The execution context 
consists of an ``InterpStack``, along with a chain of ``InterpFrame`` objects 
storing the call frames. Frames are built by call instructions and destroyed by 
return instructions. 

[PATCH] D66988: [NewPM][Sancov] Make Sancov a Module Pass instead of 2 Passes

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

*ping* This patch also seems to fix an issue with mismatched PC table and 
inline counters:

  ./out/Fuzz2/region_deserialize

   
  INFO: Seed: 264880470
  INFO: Loaded 1 modules   (170004 inline 8-bit counters): 170004 [0x2259c90, 
0x22834a4), 
  INFO: Loaded 1 PC tables (144267 PCs): 144267 [0x22834a8,0x24b6d58)
  ERROR: The size of coverage PC tables does not match the
  number of instrumented PCs. This might be a compiler bug,
  please contact the libFuzzer developers.
  Also check https://bugs.llvm.org/show_bug.cgi?id=34636
  for possible workarounds (tl;dr: don't use the old GNU ld)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66988



___
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 218168.
paulkirth added a comment.

Remove extra include from CodeGenFunction.cpp


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/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-stripped.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 the llvm.expect intrinsic: 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 the llvm.expect intrinsic: 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 the llvm.expect intrinsic: Annotation was correct on 0.00% of profiled executions.
+
+; DISABLED-NOT: 

[PATCH] D67020: [WebAssembly] Add SIMD QFMA/QFMS

[Thomas Lively via Phabricator via cfe-commits]

tlively created this revision.
tlively added reviewers: aheejin, dschuff.
Herald added subscribers: llvm-commits, cfe-commits, sunfish, hiraditya, 
jgravelle-google, sbc100.
Herald added projects: clang, LLVM.

Adds clang builtins and LLVM intrinsics for these experimental
instructions. They are not implemented in engines yet, but that is ok
because the user must opt into using them by calling the builtins.

Depends on D66983 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67020

Files:
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-wasm.c
  llvm/include/llvm/IR/IntrinsicsWebAssembly.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
  llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
  llvm/test/MC/WebAssembly/simd-encodings.s

Index: llvm/test/MC/WebAssembly/simd-encodings.s
===
--- llvm/test/MC/WebAssembly/simd-encodings.s
+++ llvm/test/MC/WebAssembly/simd-encodings.s
@@ -382,6 +382,12 @@
 # CHECK: f32x4.sqrt # encoding: [0xfd,0x97,0x01]
 f32x4.sqrt
 
+# CHECK: f32x4.qfma # encoding: [0xfd,0x98,0x01]
+f32x4.qfma
+
+# CHECK: f32x4.qfms # encoding: [0xfd,0x99,0x01]
+f32x4.qfms
+
 # CHECK: f32x4.add # encoding: [0xfd,0x9a,0x01]
 f32x4.add
 
@@ -409,6 +415,12 @@
 # CHECK: f64x2.sqrt # encoding: [0xfd,0xa2,0x01]
 f64x2.sqrt
 
+# CHECK: f64x2.qfma # encoding: [0xfd,0xa3,0x01]
+f64x2.qfma
+
+# CHECK: f64x2.qfms # encoding: [0xfd,0xa4,0x01]
+f64x2.qfms
+
 # CHECK: f64x2.add # encoding: [0xfd,0xa5,0x01]
 f64x2.add
 
Index: llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
===
--- llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
+++ llvm/test/CodeGen/WebAssembly/simd-intrinsics.ll
@@ -320,11 +320,35 @@
 declare <4 x float> @llvm.wasm.bitselect.v4f32(<4 x float>, <4 x float>, <4 x float>)
 define <4 x float> @bitselect_v4f32(<4 x float> %v1, <4 x float> %v2, <4 x float> %c) {
   %a = call <4 x float> @llvm.wasm.bitselect.v4f32(
- <4 x float> %v1, <4 x float> %v2, <4 x float> %c
+<4 x float> %v1, <4 x float> %v2, <4 x float> %c
   )
   ret <4 x float> %a
 }
 
+; CHECK-LABEL: qfma_v4f32:
+; SIMD128-NEXT: .functype qfma_v4f32 (v128, v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: f32x4.qfma $push[[R:[0-9]+]]=, $0, $1, $2{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <4 x float> @llvm.wasm.qfma.v4f32(<4 x float>, <4 x float>, <4 x float>)
+define <4 x float> @qfma_v4f32(<4 x float> %a, <4 x float> %b, <4 x float> %c) {
+  %v = call <4 x float> @llvm.wasm.qfma.v4f32(
+<4 x float> %a, <4 x float> %b, <4 x float> %c
+  )
+  ret <4 x float> %v
+}
+
+; CHECK-LABEL: qfms_v4f32:
+; SIMD128-NEXT: .functype qfms_v4f32 (v128, v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: f32x4.qfms $push[[R:[0-9]+]]=, $0, $1, $2{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <4 x float> @llvm.wasm.qfms.v4f32(<4 x float>, <4 x float>, <4 x float>)
+define <4 x float> @qfms_v4f32(<4 x float> %a, <4 x float> %b, <4 x float> %c) {
+  %v = call <4 x float> @llvm.wasm.qfms.v4f32(
+<4 x float> %a, <4 x float> %b, <4 x float> %c
+  )
+  ret <4 x float> %v
+}
+
 ; ==
 ; 2 x f64
 ; ==
@@ -339,3 +363,27 @@
   )
   ret <2 x double> %a
 }
+
+; CHECK-LABEL: qfma_v2f64:
+; SIMD128-NEXT: .functype qfma_v2f64 (v128, v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: f64x2.qfma $push[[R:[0-9]+]]=, $0, $1, $2{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <2 x double> @llvm.wasm.qfma.v2f64(<2 x double>, <2 x double>, <2 x double>)
+define <2 x double> @qfma_v2f64(<2 x double> %a, <2 x double> %b, <2 x double> %c) {
+  %v = call <2 x double> @llvm.wasm.qfma.v2f64(
+<2 x double> %a, <2 x double> %b, <2 x double> %c
+  )
+  ret <2 x double> %v
+}
+
+; CHECK-LABEL: qfms_v2f64:
+; SIMD128-NEXT: .functype qfms_v2f64 (v128, v128, v128) -> (v128){{$}}
+; SIMD128-NEXT: f64x2.qfms $push[[R:[0-9]+]]=, $0, $1, $2{{$}}
+; SIMD128-NEXT: return $pop[[R]]{{$}}
+declare <2 x double> @llvm.wasm.qfms.v2f64(<2 x double>, <2 x double>, <2 x double>)
+define <2 x double> @qfms_v2f64(<2 x double> %a, <2 x double> %b, <2 x double> %c) {
+  %v = call <2 x double> @llvm.wasm.qfms.v2f64(
+<2 x double> %a, <2 x double> %b, <2 x double> %c
+  )
+  ret <2 x double> %v
+}
Index: llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
===
--- llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
+++ llvm/lib/Target/WebAssembly/WebAssemblyInstrSIMD.td
@@ -732,3 +732,24 @@
   )
 ) in
 def : Pat<(t1 (bitconvert (t2 V128:$v))), (t1 V128:$v)>;
+
+//===--===//
+// Quasi-Fused 

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

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

In D66324#1652364 , @lebedev.ri wrote:

> Trying to start reviewing this.
>  The llvm side of the patch is self contained; clang patch should be split 
> into a dependent review.


Looking at this, is it necessary to split up the patch? That will loose a lot 
of the previous comment/context, and I'd prefer to land this as a single 
change.  Doing so has the benefit that if my patch needs to be reverted, it can 
be done all together.  For example, if only the llvm patch is reverted, then it 
will likely cause other problems and make triage needlessly painful.


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] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

We had a discussion on IRC yesterday and @aaron.ballman pointed out that the 
K syntax has been considered "obsolescent" (= deprecated) since C89, 30 years 
ago. He added that there are thoughts within the standards committee about 
removing the syntax entirely from C2x.

In D66919#1653078 , @dexonsmith wrote:

> I would suggest `-Werror`-by-default since the compiler knows it's incorrect 
> code.


Do we have any precedence for a warning that's treated as error by default? I 
haven't seen that before, that's why I'm asking. Even `-Wreturn-type` isn't 
treated as error by default.

> All I'm suggesting is that the compiler knows that the call is wrong, so it 
> should error (by-default).

K allows weird stuff, so I'm really not sure it's necessarily wrong. To my 
knowledge the `...` varargs syntax was only introduced with prototypes, and the 
K version of printf was "implicitly vararg". Meaning that the additional 
parameters weren't mentioned in the function header at all, and scraped from 
the stack using the `va_*` macros as it's done today.

Also by that argument we shouldn't emit `-Wstrict-prototypes` at all on 
definitions, because clearly the compiler can also do type checking then. In 
fact, we actually do that to some extent: instead of default-promoting the call 
arguments, we default-promote the parameter types and build a 
`FunctionProtoType`, so that calls work as if there had been a proper 
prototype. This has the odd side effect of causing different behavior depending 
on whether the compiler sees the definition of a function, as @aaron.ballman 
noted. The example we looked at was

  int f();
  int g() { return f(1.0); }
  int f(x) int x; {}
  int h() { return f(1.0); }

One might think that we emit the same code for `g` and `h`, but we don't!

> After adding that, my feeling is that diagnosing a missing prototype on a 
> defining declaration would be a style nitpick that might fit better in 
> clang-tidy.

It's not a stylistic issue, prototype and non-prototype declarations are 
semantically very different. C has **no notion of argument checking for 
non-prototypes** at all. The `identifier-list` of a K definition is 
essentially private.

Of course we can impose our own semantics for non-prototypes, but we can also 
just nudge people into using standard syntax that has been available for many 
decades that guarantees them proper argument checks, both regarding number and 
types of arguments.

> IIRC, when we rolled out `-Wstrict-prototypes` we explicitly excluded this 
> case since it hit a lot of code without finding bugs.

The GCC people didn't, so it can't be that bad. Also we already warn on block 
definitions with zero parameters for some reason. Lastly, the fix is very easy: 
just add `void`. With some `-fixit` magic it might just be a matter of running 
Clang over the entire code base once and you're done with it.

We don't only warn on actual bugs, but also on using deprecated language 
features and generally about problematic code. As I pointed out, 
**zero-parameter K definitions are problematic even if we error out when they 
are called with the wrong number of parameters**, because such definitions 
might be inline parts of an interface that's used with other compilers that 
don't have such checks.

> If you do pursue this please use a distinct `-W` flag so it can be turned off 
> without losing the benefits of `-Wstrict-prototypes`.

The warning is not on by default, and not enabled by `-Wall` or `-Wextra`, so 
this would serve the small sliver of developers who researched long enough to 
find this flag, decided to activate it only with Clang and not GCC, and then 
for some reason don't want it to do what it promises in its name. If the 
warning calls itself "strict", then that's what it should be.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66919



___
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 218164.
paulkirth added a comment.

Address Review items

- minimize debug info in llvm tests
- sanitize paths in debug info
- use more complete checks in test  for LowerExpectIntrisic
- remove references to __builtin_expect from backend code
  - update test code for new diagnostic messages
- rename checkClangInstrumentation to checkFrontendInstrumentation
- revert whitespace removal in CodeGenFunction.cpp
- use more idomatic check when adding diagnostic to the backend


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-stripped.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 the llvm.expect intrinsic: Annotation was correct on 0.00% of profiled executions.
+
+; REMARK-NOT: warning: 

[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.

[David Gatwood via Phabricator via cfe-commits]

dgatwood added a comment.

Whoops.  I was expecting to get emailed about review comments and never got 
anything, so I thought it was just not getting reviewed.  I'll work on this 
again.  Sorry about that.  :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65917



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

[PATCH] D66505: Make add_new_check.py's insertion of registerCheck<> match the sort order

[Daniel Sanders via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370527: Make add_new_check.pys insertion of 
registerCheck match the sort order (authored by dsanders, committed by 
).
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D66505?vs=218137=218163#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66505

Files:
  clang-tools-extra/trunk/clang-tidy/add_new_check.py


Index: clang-tools-extra/trunk/clang-tidy/add_new_check.py
===
--- clang-tools-extra/trunk/clang-tidy/add_new_check.py
+++ clang-tools-extra/trunk/clang-tidy/add_new_check.py
@@ -165,31 +165,50 @@
 header_added = False
 header_found = False
 check_added = False
+check_fq_name = module + '-' + check_name
 check_decl = ('CheckFactories.registerCheck<' + check_name_camel +
-  '>(\n"' + module + '-' + check_name + '");\n')
+  '>(\n"' + check_fq_name + '");\n')
 
-for line in lines:
-  if not header_added:
-match = re.search('#include "(.*)"', line)
-if match:
-  header_found = True
-  if match.group(1) > check_name_camel:
+lines = iter(lines)
+try:
+  while True:
+line = lines.next()
+if not header_added:
+  match = re.search('#include "(.*)"', line)
+  if match:
+header_found = True
+if match.group(1) > check_name_camel:
+  header_added = True
+  f.write('#include "' + check_name_camel + '.h"\n')
+  elif header_found:
 header_added = True
 f.write('#include "' + check_name_camel + '.h"\n')
-elif header_found:
-  header_added = True
-  f.write('#include "' + check_name_camel + '.h"\n')
-
-  if not check_added:
-if line.strip() == '}':
-  check_added = True
-  f.write(check_decl)
-else:
-  match = re.search('registerCheck<(.*)>', line)
-  if match and match.group(1) > check_name_camel:
+
+if not check_added:
+  if line.strip() == '}':
 check_added = True
 f.write(check_decl)
-  f.write(line)
+  else:
+match = re.search('registerCheck<(.*)> *\( *(?:"([^"]*)")?', line)
+prev_line = None
+if match:
+  current_check_name = match.group(2)
+  if current_check_name is None:
+# If we didn't find the check name on this line, look on the
+# next one.
+prev_line = line
+line = lines.next()
+match = re.search(' *"([^"]*)"', line)
+if match:
+  current_check_name = match.group(1)
+  if current_check_name > check_fq_name:
+check_added = True
+f.write(check_decl)
+  if prev_line:
+f.write(prev_line)
+f.write(line)
+except StopIteration:
+  pass
 
 
 # Adds a release notes entry.


Index: clang-tools-extra/trunk/clang-tidy/add_new_check.py
===
--- clang-tools-extra/trunk/clang-tidy/add_new_check.py
+++ clang-tools-extra/trunk/clang-tidy/add_new_check.py
@@ -165,31 +165,50 @@
 header_added = False
 header_found = False
 check_added = False
+check_fq_name = module + '-' + check_name
 check_decl = ('CheckFactories.registerCheck<' + check_name_camel +
-  '>(\n"' + module + '-' + check_name + '");\n')
+  '>(\n"' + check_fq_name + '");\n')
 
-for line in lines:
-  if not header_added:
-match = re.search('#include "(.*)"', line)
-if match:
-  header_found = True
-  if match.group(1) > check_name_camel:
+lines = iter(lines)
+try:
+  while True:
+line = lines.next()
+if not header_added:
+  match = re.search('#include "(.*)"', line)
+  if match:
+header_found = True
+if match.group(1) > check_name_camel:
+  header_added = True
+  f.write('#include "' + check_name_camel + '.h"\n')
+  elif header_found:
 header_added = True
 f.write('#include "' + check_name_camel + '.h"\n')
-elif header_found:
-  header_added = True
-  f.write('#include "' + check_name_camel + '.h"\n')
-
-  if not check_added:
-if line.strip() == '}':
-  check_added = True
-  f.write(check_decl)
-else:
-  match = re.search('registerCheck<(.*)>', line)
-  if match and match.group(1) > check_name_camel:
+
+if not check_added:
+  if line.strip() == '}':
 check_added = True
 

[PATCH] D67019: [analyzer] pr43179: CallDescription: Check number of parameters as well as number of arguments.

[Csaba Dabis via Phabricator via cfe-commits]

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

I like the isolation of this kind of stuff. Thanks you!


Repository:
  rC Clang

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

https://reviews.llvm.org/D67019



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

[clang-tools-extra] r370527 - Make add_new_check.py's insertion of registerCheck<> match the sort order

[Daniel Sanders via cfe-commits]

Author: dsanders
Date: Fri Aug 30 13:47:02 2019
New Revision: 370527

URL: http://llvm.org/viewvc/llvm-project?rev=370527=rev
Log:
Make add_new_check.py's insertion of registerCheck<> match the sort order

Summary:
Following on from review comments in D65919 about the ordering
of the registerCheck<> calls. Sort based on the check name which might
be on the line after the registerCheck<>

Reviewers: aaron.ballman

Subscribers: cfe-commits, llvm-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clang-tidy/add_new_check.py

Modified: clang-tools-extra/trunk/clang-tidy/add_new_check.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/add_new_check.py?rev=370527=370526=370527=diff
==
--- clang-tools-extra/trunk/clang-tidy/add_new_check.py (original)
+++ clang-tools-extra/trunk/clang-tidy/add_new_check.py Fri Aug 30 13:47:02 2019
@@ -165,31 +165,50 @@ def adapt_module(module_path, module, ch
 header_added = False
 header_found = False
 check_added = False
+check_fq_name = module + '-' + check_name
 check_decl = ('CheckFactories.registerCheck<' + check_name_camel +
-  '>(\n"' + module + '-' + check_name + '");\n')
+  '>(\n"' + check_fq_name + '");\n')
 
-for line in lines:
-  if not header_added:
-match = re.search('#include "(.*)"', line)
-if match:
-  header_found = True
-  if match.group(1) > check_name_camel:
+lines = iter(lines)
+try:
+  while True:
+line = lines.next()
+if not header_added:
+  match = re.search('#include "(.*)"', line)
+  if match:
+header_found = True
+if match.group(1) > check_name_camel:
+  header_added = True
+  f.write('#include "' + check_name_camel + '.h"\n')
+  elif header_found:
 header_added = True
 f.write('#include "' + check_name_camel + '.h"\n')
-elif header_found:
-  header_added = True
-  f.write('#include "' + check_name_camel + '.h"\n')
 
-  if not check_added:
-if line.strip() == '}':
-  check_added = True
-  f.write(check_decl)
-else:
-  match = re.search('registerCheck<(.*)>', line)
-  if match and match.group(1) > check_name_camel:
+if not check_added:
+  if line.strip() == '}':
 check_added = True
 f.write(check_decl)
-  f.write(line)
+  else:
+match = re.search('registerCheck<(.*)> *\( *(?:"([^"]*)")?', line)
+prev_line = None
+if match:
+  current_check_name = match.group(2)
+  if current_check_name is None:
+# If we didn't find the check name on this line, look on the
+# next one.
+prev_line = line
+line = lines.next()
+match = re.search(' *"([^"]*)"', line)
+if match:
+  current_check_name = match.group(1)
+  if current_check_name > check_fq_name:
+check_added = True
+f.write(check_decl)
+  if prev_line:
+f.write(prev_line)
+f.write(line)
+except StopIteration:
+  pass
 
 
 # Adds a release notes entry.


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

[PATCH] D67019: [analyzer] pr43179: CallDescription: Check number of parameters as well as number of arguments.

[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.
NoQ retitled this revision from "[analyzer] CallDescription: Check number of 
parameters as well as number of arguments." to "[analyzer] pr43179: 
CallDescription: Check number of parameters as well as number of arguments.".
NoQ edited the summary of this revision.

This is https://bugs.llvm.org/show_bug.cgi?id=43179.

Most functions that our checkers react upon are not C-style variadic functions, 
and therefore they have as many actual arguments as they have formal parameters.

However, it's not impossible to define a variadic function with the same name. 
This will crash any checker that relies on `CallDescription` to check the 
number of arguments but then silently assumes that the number of parameters is 
the same.

Change `CallDescription` to check both the number of arguments and the number 
of parameters by default.

If we're intentionally trying to match variadic functions, allow specifying 
arguments and parameters separately (possibly omitting any of them). For now we 
only have one `CallDescription` which would make use of those, namely 
`__builtin_va_start` itself.


Repository:
  rC Clang

https://reviews.llvm.org/D67019

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/Analysis/cast-value-weird.cpp


Index: clang/test/Analysis/cast-value-weird.cpp
===
--- /dev/null
+++ clang/test/Analysis/cast-value-weird.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,apiModeling -verify %s
+
+// expected-no-diagnostics
+
+namespace llvm {
+template 
+void cast(...);
+void a() { cast(int()); } // no-crash
+} // namespace llvm
Index: clang/lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- clang/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ clang/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -368,7 +368,8 @@
 
   if (CD.Flags & CDF_MaybeBuiltin) {
 return CheckerContext::isCLibraryFunction(FD, CD.getFunctionName()) &&
-   (!CD.RequiredArgs || CD.RequiredArgs <= getNumArgs());
+   (!CD.RequiredArgs || CD.RequiredArgs <= getNumArgs()) &&
+   (!CD.RequiredParams || CD.RequiredParams <= parameters().size());
   }
 
   if (!CD.IsLookupDone) {
@@ -407,7 +408,8 @@
   return false;
   }
 
-  return (!CD.RequiredArgs || CD.RequiredArgs == getNumArgs());
+  return (!CD.RequiredArgs || CD.RequiredArgs == getNumArgs()) &&
+ (!CD.RequiredParams || CD.RequiredParams == parameters().size());
 }
 
 SVal CallEvent::getArgSVal(unsigned Index) const {
Index: clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ValistChecker.cpp
@@ -116,7 +116,9 @@
 // vswprintf is the wide version of vsnprintf,
 // vsprintf has no wide version
 {{"vswscanf", 3}, 2}};
-const CallDescription ValistChecker::VaStart("__builtin_va_start", 2),
+
+const CallDescription
+ValistChecker::VaStart("__builtin_va_start", /*Args=*/2, /*Params=*/1),
 ValistChecker::VaCopy("__builtin_va_copy", 2),
 ValistChecker::VaEnd("__builtin_va_end", 1);
 } // end anonymous namespace
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
@@ -1064,8 +1064,19 @@
   // e.g. "{a, b}" represent the qualified names, like "a::b".
   std::vector QualifiedName;
   Optional RequiredArgs;
+  Optional RequiredParams;
   int Flags;
 
+  // A constructor helper.
+  static Optional readRequiredParams(Optional RequiredArgs,
+ Optional RequiredParams) {
+if (RequiredParams)
+  return RequiredParams;
+if (RequiredArgs)
+  return static_cast(*RequiredArgs);
+return None;
+  }
+
 public:
   /// Constructs a CallDescription object.
   ///
@@ -1078,14 +1089,17 @@
   /// call. Omit this parameter to match every occurrence of call with a given
   /// name regardless the number of arguments.
   CallDescription(int Flags, ArrayRef QualifiedName,
-  Optional RequiredArgs = None)
+  Optional RequiredArgs = None,
+  Optional RequiredParams = None)
   : QualifiedName(QualifiedName), RequiredArgs(RequiredArgs),
+RequiredParams(readRequiredParams(RequiredArgs, RequiredParams)),
 Flags(Flags) {}

[PATCH] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

[Daniel Sanders via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
dsanders marked an inline comment as done.
Closed by commit rL370512: [clang-tidy] Add llvm-prefer-register-over-unsigned 
to clang-tidy (authored by dsanders, committed by ).
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D65919?vs=218015=218156#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65919

Files:
  clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/trunk/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp
  clang-tools-extra/trunk/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/llvm-prefer-register-over-unsigned.rst
  clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp
  
clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned2.cpp
  
clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned3.cpp

Index: clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned3.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned3.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned3.cpp
@@ -0,0 +1,33 @@
+// RUN: %check_clang_tidy %s llvm-prefer-register-over-unsigned %t
+
+namespace llvm { };
+
+// This class shouldn't trigger it despite the similarity as it's not inside the llvm namespace
+class Register {
+public:
+  operator unsigned();
+};
+
+Register getReg();
+
+void do_nothing_1() {
+  unsigned Reg1 = getReg();
+  // CHECK-FIXES: do_nothing_1()
+  // CHECK-FIXES-NEXT: unsigned Reg1 = getReg();
+}
+
+void do_nothing_2() {
+  using namespace llvm;
+  unsigned Reg2 = getReg();
+  // CHECK-FIXES: do_nothing_2()
+  // CHECK-FIXES-NEXT: using namespace llvm;
+  // CHECK-FIXES-NEXT: unsigned Reg2 = getReg();
+}
+
+namespace llvm {
+void do_nothing_3() {
+  unsigned Reg3 = getReg();
+  // CHECK-FIXES: do_nothing_3()
+  // CHECK-FIXES-NEXT: unsigned Reg3 = getReg();
+}
+} // end namespace llvm
Index: clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp
@@ -0,0 +1,143 @@
+// RUN: %check_clang_tidy %s llvm-prefer-register-over-unsigned %t
+
+namespace llvm {
+class Register {
+public:
+  operator unsigned();
+
+  unsigned Reg;
+};
+
+// This class shouldn't trigger it despite the similarity.
+class RegisterLike {
+public:
+  operator unsigned();
+
+  unsigned Reg;
+};
+} // end namespace llvm
+
+llvm::Register getReg();
+llvm::RegisterLike getRegLike();
+
+void apply_1() {
+  unsigned Reg1 = getReg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'Reg1' declared as 'unsigned int'; use 'llvm::Register' instead [llvm-prefer-register-over-unsigned]
+  // CHECK-FIXES: apply_1()
+  // CHECK-FIXES-NEXT: llvm::Register Reg1 = getReg();
+}
+
+void apply_2() {
+  using namespace llvm;
+  unsigned Reg2 = getReg();
+  // FIXME: Function-scoped UsingDirectiveDecl's don't appear to be in the
+  //DeclContext for the function so we can't elide the llvm:: in this
+  //case. Fortunately, it doesn't actually occur in the LLVM code base.
+  // CHECK-MESSAGES: :[[@LINE-4]]:12: warning: variable 'Reg2' declared as 'unsigned int'; use 'llvm::Register' instead [llvm-prefer-register-over-unsigned]
+  // CHECK-FIXES: apply_2()
+  // CHECK-FIXES-NEXT: using namespace llvm;
+  // CHECK-FIXES-NEXT: llvm::Register Reg2 = getReg();
+}
+
+namespace llvm {
+void apply_3() {
+  unsigned Reg3 = getReg();
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: variable 'Reg3' declared as 'unsigned int'; use 'Register' instead [llvm-prefer-register-over-unsigned]
+  // CHECK-FIXES: apply_3()
+  // CHECK-FIXES-NEXT: Register Reg3 = getReg();
+}
+} // end namespace llvm
+
+void done_1() {
+  llvm::Register Reg1 = getReg();
+  // CHECK-FIXES: done_1()
+  // CHECK-FIXES-NEXT: llvm::Register Reg1 = getReg();
+}
+
+void done_2() {
+  using namespace llvm;
+  Register Reg2 = getReg();
+  // CHECK-FIXES: done_2()
+  // CHECK-FIXES-NEXT: using namespace llvm;
+  // CHECK-FIXES-NEXT: Register Reg2 = getReg();
+}
+
+namespace llvm {
+void done_3() {
+  Register Reg3 = getReg();
+  // CHECK-FIXES: done_3()
+  // CHECK-FIXES-NEXT: Register Reg3 = getReg();
+}
+} // end namespace llvm
+
+void do_nothing_1() {
+  unsigned Reg1 = getRegLike();
+  // CHECK-FIXES: do_nothing_1()
+  // CHECK-FIXES-NEXT: unsigned Reg1 = getRegLike();
+}
+
+void do_nothing_2() {
+  using 

[PATCH] D65919: [clang-tidy] Add llvm-prefer-register-over-unsigned check and apply it to LLVM

[Daniel Sanders via Phabricator via cfe-commits]

dsanders added a comment.

r370512


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65919



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

[clang-tools-extra] r370512 - [clang-tidy] Add llvm-prefer-register-over-unsigned to clang-tidy

[Daniel Sanders via cfe-commits]

Author: dsanders
Date: Fri Aug 30 13:01:59 2019
New Revision: 370512

URL: http://llvm.org/viewvc/llvm-project?rev=370512=rev
Log:
[clang-tidy] Add llvm-prefer-register-over-unsigned to clang-tidy

Summary:
This clang-tidy check is looking for unsigned integer variables whose 
initializer
starts with an implicit cast from llvm::Register and changes the type of the
variable to llvm::Register (dropping the llvm:: where possible).

Reviewers: arsenm, bogner

Subscribers: jholewinski, MatzeB, qcolombet, dschuff, jyknight, dylanmckay, 
sdardis, nemanjai, jvesely, wdng, nhaehnle, mgorny, sbc100, jgravelle-google, 
kristof.beyls, hiraditya, aheejin, kbarton, fedor.sergeev, javed.absar, asb, 
rbar, johnrusso, simoncook, apazos, sabuasal, niosHD, jrtc27, MaskRay, zzheng, 
edward-jones, atanasyan, rogfer01, MartinMosbeck, brucehoult, the_o, tpr, PkmX, 
jocewei, jsji, Petar.Avramovic, asbirlea, Jim, s.egerton, cfe-commits, 
llvm-commits

Tags: #clang, #llvm

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

Added:
clang-tools-extra/trunk/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp
clang-tools-extra/trunk/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/llvm-prefer-register-over-unsigned.rst

clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned.cpp

clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned2.cpp

clang-tools-extra/trunk/test/clang-tidy/llvm-prefer-register-over-unsigned3.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt?rev=370512=370511=370512=diff
==
--- clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/llvm/CMakeLists.txt Fri Aug 30 13:01:59 
2019
@@ -5,6 +5,7 @@ add_clang_library(clangTidyLLVMModule
   IncludeOrderCheck.cpp
   LLVMTidyModule.cpp
   PreferIsaOrDynCastInConditionalsCheck.cpp
+  PreferRegisterOverUnsignedCheck.cpp
   TwineLocalCheck.cpp
 
   LINK_LIBS

Modified: clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp?rev=370512=370511=370512=diff
==
--- clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/llvm/LLVMTidyModule.cpp Fri Aug 30 
13:01:59 2019
@@ -13,6 +13,7 @@
 #include "HeaderGuardCheck.h"
 #include "IncludeOrderCheck.h"
 #include "PreferIsaOrDynCastInConditionalsCheck.h"
+#include "PreferRegisterOverUnsignedCheck.h"
 #include "TwineLocalCheck.h"
 
 namespace clang {
@@ -28,6 +29,8 @@ public:
 "llvm-namespace-comment");
 CheckFactories.registerCheck(
 "llvm-prefer-isa-or-dyn-cast-in-conditionals");
+CheckFactories.registerCheck(
+"llvm-prefer-register-over-unsigned");
 CheckFactories.registerCheck("llvm-twine-local");
   }
 };

Added: 
clang-tools-extra/trunk/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp?rev=370512=auto
==
--- clang-tools-extra/trunk/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp 
(added)
+++ clang-tools-extra/trunk/clang-tidy/llvm/PreferRegisterOverUnsignedCheck.cpp 
Fri Aug 30 13:01:59 2019
@@ -0,0 +1,64 @@
+//===--- PreferRegisterOverUnsignedCheck.cpp - clang-tidy 
-===//
+//
+// 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
+//
+//===--===//
+
+#include "PreferRegisterOverUnsignedCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace llvm_check {
+
+void PreferRegisterOverUnsignedCheck::registerMatchers(MatchFinder *Finder) {
+  auto RegisterClassMatch = hasType(
+  cxxRecordDecl(hasName("::llvm::Register")).bind("registerClassDecl"));
+
+  Finder->addMatcher(
+  valueDecl(allOf(
+  hasType(qualType(isUnsignedInteger()).bind("varType")),
+  varDecl(hasInitializer(exprWithCleanups(has(implicitCastExpr(has(
+  cxxMemberCallExpr(allOf(on(RegisterClassMatch),
+  

[PATCH] D64480: [ASTImporter] Added visibility context check for TypedefNameDecl.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: lib/AST/ASTImporter.cpp:949
+return Importer.GetFromTU(Found) == From->getTranslationUnitDecl();
+  return From->isInAnonymousNamespace() == Found->isInAnonymousNamespace();
+}

I am not sure what case this covers? Can you elaborate? I see the case the 
condition above is catching.



Comment at: unittests/AST/ASTImporterVisibilityTest.cpp:348
+::testing::Values(
+std::make_tuple(ExternTypedef, ExternTypedef, ExpectLink),
+std::make_tuple(ExternTypedef, AnonTypedef, ExpectNotLink),

Perhaps `ExpectLink` would be better as `ExpectLinkedDeclChain` and the same 
for `ExpectNotLink`.

It was actually confusing at first because link is an overload term.



Comment at: unittests/AST/ASTImporterVisibilityTest.cpp:352
+std::make_tuple(AnonTypedef, AnonTypedef, ExpectNotLink))), );
+INSTANTIATE_TEST_CASE_P(
+ParameterizedTests, ImportTypeAliasVisibility,

We need another set of tests for typedefs and using e.g. `ExternTypedef` and 
`ExternUsing` since they are equivalent. 


Repository:
  rC Clang

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

https://reviews.llvm.org/D64480



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

[PATCH] D65744: [PR42707][OpenCL] Fix addr space deduction for auto

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D65744#1652355 , @Anastasia wrote:

> I don't think this is likely to change. Are you suggesting to move the 
> deduction logic for pointee of pointers, references and block pointers into 
> ASTContext helper that creates a pointer/reference/block pointer type?


No.  I'm suggesting that the deduction logic should be much more 
straightforward, just some sort of "is the type non-dependent and lacking a 
qualifier", and it should be applied in the two basic places we build these 
types in Sema, i.e. in the type-from-declarator logic and in the 
`Build{Pointer,Reference}Type` logic.  Instead we have something very elaborate 
that apparently recursively looks through pointer types and is contingent on 
the exact spelling, e.g. trying to find `auto` types, which seems both brittle 
and unnecessary.




Comment at: include/clang/AST/Type.h:6509
+  return isa(CanonicalType);
+}
+

Hmm.  So this method, confusingly, will not return true for a deduced `auto`, 
unless the deduced type is itself an undeduced `auto` (which I'm not sure can 
happen).  I think it at least needs a different name; `isUndeducedAutoType()` 
would be okay if the latter case is not possible.  But it might be better if we 
can just define away the need for the method entirely.



Comment at: lib/Sema/SemaType.cpp:7441
+  // the initializing expression type during the type deduction.
+  (T->isAutoType() && IsPointee) || (IsAutoPointee) ||
   // OpenCL spec v2.0 s6.9.b:

Anastasia wrote:
> mantognini wrote:
> > mantognini wrote:
> > > Shouldn't the parentheses around `IsAutoPointee` be removed for style 
> > > consistency?
> > With the `if` statement introduced above, `IsAutoPointee` can be true only 
> > in C++ mode. Could it be an issue to not guard `(T->isAutoType() && 
> > IsPointee)` for non-C++ mode? (I guess not, but I couldn't convince myself.)
> I think `TreeTransforms` will only be used in C++ mode. But also `isAutoType` 
> should only be true in C++ mode. So I think we should be fine.
I don't think `TreeTransform` is expected to be C++-only, but I agree that 
`isAutoType` is good enough.


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

https://reviews.llvm.org/D65744



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

[PATCH] D66919: Warn about zero-parameter K definitions in -Wstrict-prototypes

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

dexonsmith added a comment.

> In D66919#1650174 , @dexonsmith 
> wrote:
> 
>> We could carve out a `-W` flag (if it doesn't already exist) that warns if 
>> you incorrectly pass parameters to a function whose definition has no 
>> prototype
> 
> 
> The warning exists, but there is no flag apparently.
> 
>   void f() {}
>   void g() {
>   f(0);
>   }
> 
> 
> spits out
> 
>   test.c:3:8: warning: too many arguments in call to 'f'
>   f(0);
>   ~  ^

Great; then I think we should add a flag.  I would suggest `-Werror`-by-default 
since the compiler knows it's incorrect code.

In D66919#1651473 , @aaronpuchert 
wrote:

> "Meaning" is a difficult term. What we know is that this is a K 
> definition, and does not define a prototype. So one would expect 
> `-Wstrict-prototypes` to warn about this.


All I'm suggesting is that the compiler knows that the call is wrong, so it 
should error (by-default).

After adding that, my feeling is that diagnosing a missing prototype on a 
defining declaration would be a style nitpick that might fit better in 
clang-tidy.  IIRC, when we rolled out `-Wstrict-prototypes` we explicitly 
excluded this case since it hit a lot of code without finding bugs.

But at the same time, I don't want to block this.  If you do pursue this please 
use a distinct `-W` flag so it can be turned off without losing the benefits of 
`-Wstrict-prototypes`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66919



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

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:54-55
+const SourceLocation Loc,
+const Expr *SubLHS = nullptr,
+const Expr *SubRHS = nullptr);
+

I'd appreciate some comments on what these arguments should be when non-null.



Comment at: lib/Sema/SemaExpr.cpp:11031-11032
   // Do not diagnose macros.
-  if (Loc.isMacroID())
+  if (Loc.isMacroID() || XorLHS.get()->getBeginLoc().isMacroID() ||
+  XorRHS.get()->getBeginLoc().isMacroID())
 return;

I would appreciate it if this patch didn't also change the behavior for macros. 
That seems like a larger discussion that can happen in a separate patch.



Comment at: lib/Sema/SemaExpr.cpp:11052
+  Negative = (Opc == UO_Minus);
+  ExplicitPlus = (Opc == UO_Plus);
 } else {

`!Negative` (we already verified above that it's either the + or - operator, so 
if it's not one, it's the other.)?



Comment at: lib/Sema/SemaExpr.cpp:11067
+
+  // Do not diagnose 2 ^ 64, but allow special case (2 ^ 64) - 1.
+  if (SubLHS && SubRHS && (LeftSideValue != 2 || RightSideValue != 64))

This comment explains what the code does, but not why it does it. Given that 
we're adding special cases, I think more comments here explaining why this is 
valuable would be appreciated.



Comment at: lib/Sema/SemaExpr.cpp:11073
   LHSInt->getBeginLoc(), S.getLocForEndOfToken(RHSInt->getLocation()));
-  llvm::StringRef ExprStr =
+  std::string ExprStr =
   Lexer::getSourceText(ExprRange, S.getSourceManager(), S.getLangOpts());

Why is this type changing?



Comment at: lib/Sema/SemaExpr.cpp:11078
   CharSourceRange::getCharRange(Loc, S.getLocForEndOfToken(Loc));
-  llvm::StringRef XorStr =
+  std::string XorStr =
   Lexer::getSourceText(XorRange, S.getSourceManager(), S.getLangOpts());

Same question here.



Comment at: lib/Sema/SemaExpr.cpp:11130
+  ? "ULLONG_MAX"
+  : "-1LL";
+  ExprRange = CharSourceRange::getCharRange(

The two branches are not equivalent. What about `~0ULL` instead?


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

https://reviews.llvm.org/D66397



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

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D66397#1652771 , @xbolva00 wrote:

> >> tbh, I would appreciate if you would leave the definition of 
> >> diagnoseXorMisusedAsPow() where it is and add a forward declare of the 
> >> function earlier in the file.
>
> I uploaded the patch almost 2 weeks ago and nobody complained so I thought it 
> is okay. Uploaded, fixed.


I appreciate it -- this made it much easier to spot the actual changes in the 
code.


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

https://reviews.llvm.org/D66397



___
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 updated this revision to Diff 218149.
EricWF marked an inline comment as done.
EricWF added a comment.

Address review comments.

- Document that NaN isn't a supported input.
- Fix spelling mistake.


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

https://reviews.llvm.org/D66836

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

Index: test/libcxx/numerics/clamp_to_integral.pass.cpp
===
--- /dev/null
+++ test/libcxx/numerics/clamp_to_integral.pass.cpp
@@ -0,0 +1,90 @@
+//===--===//
+//
+// 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
+//
+//===--===//
+
+// __clamp_to_integral(RealT)
+
+// Test the conversion function that truncates floating point types to the
+// closest representable value for the specified integer type, or
+// numeric_limits::max()/min() if the value isn't representable.
+
+#include 
+#include 
+#include 
+
+template 
+void test() {
+  typedef std::numeric_limits Lim;
+  const bool MaxIsRepresentable = sizeof(IntT) < 8;
+  const bool IsSigned = std::is_signed::value;
+  struct TestCase {
+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},
+  {nextafter(static_cast(Lim::max()), INFINITY), Lim::max(), false},
+  };
+  for (TestCase TC : TestCases) {
+auto res = std::__clamp_to_integral(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());
+  }
+}
+
+template 
+void test_float() {
+  typedef std::numeric_limits Lim;
+  const bool MaxIsRepresentable = sizeof(IntT) < 4;
+  ((void)MaxIsRepresentable);
+  const bool IsSigned = std::is_signed::value;
+  struct TestCase {
+float 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 },
+   {nextafter(static_cast(Lim::max()), INFINITY), Lim::max(), false},
+  };
+  for (TestCase TC : TestCases) {
+auto res = std::__clamp_to_integral(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();
+  test_float();
+  test_float();
+  test_float();
+}
Index: include/math.h
===
--- include/math.h
+++ include/math.h
@@ -1553,6 +1553,40 @@
 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),
+int _Bits = (numeric_limits<_IntT>::digits - numeric_limits<_FloatT>::digits)>
+_LIBCPP_INLINE_VISIBILITY
+_LIBCPP_CONSTEXPR _IntT __max_representable_int_for_float() _NOEXCEPT {
+  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_assert(numeric_limits<_FloatT>::radix == 2, "FloatT has incorrect radix");
+  static_assert(_IsSame<_FloatT, float>::value || _IsSame<_FloatT, double>::value
+   || _IsSame<_FloatT,long double>::value, "unsupported floating point type");
+  return _FloatBigger ? numeric_limits<_IntT>::max() :  (numeric_limits<_IntT>::max() >> _Bits << _Bits);
+}
+
+// Convert a floating point number to the specified integral type after
+// clamping to the integral types representable range.
+//
+// The behavior is undefined if `__r` is NaN.
+template 
+_LIBCPP_INLINE_VISIBILITY
+_IntT __clamp_to_integral(_RealT __r) _NOEXCEPT {
+  using _Lim = std::numeric_limits<_IntT>;
+  const _IntT _MaxVal = std::__max_representable_int_for_float<_IntT, _RealT>();
+  if (__r >= ::nextafter(static_cast<_RealT>(_MaxVal), INFINITY)) {
+return _Lim::max();
+  } else if (__r <= _Lim::lowest()) {
+return _Lim::min();
+  }
+  return static_cast<_IntT>(__r);
+}
+

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

[Alexey Bataev via Phabricator via cfe-commits]

ABataev accepted this revision.
ABataev added a comment.

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66559



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

[PATCH] D66505: Make add_new_check.py's insertion of registerCheck<> match the sort order

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thank you for working on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66505



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

[PATCH] D66706: [Wdocumentation] fixes an assertion failure with typedefed function and block pointer

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang/lib/AST/Comment.cpp:151
+static bool getFunctionTypeLoc(TypeLoc TL, FunctionTypeLoc ,
+   bool testTypedefTypeLoc = false) {
   TypeLoc PrevTL;

Mordante wrote:
> gribozavr wrote:
> > Mordante wrote:
> > > gribozavr wrote:
> > > > Why is the new functionality turned off sometimes? It seems to me that 
> > > > we always want to look through typedefs.
> > > Setting `testTypedefTypeLoc` to `true` breaks a unit test in 
> > > `test/Sema/warn-documentation.cpp:358`
> > > 
> > > ```
> > > typedef int (*test_not_function_like_typedef1)(int aaa);
> > > 
> > > // expected-warning@+1 {{'\param' command used in a comment that is not 
> > > attached to a function declaration}}
> > > /// \param aaa Meow.  
> > >   
> > > typedef test_not_function_like_typedef1 test_not_function_like_typedef2;
> > > ```
> > > and its corresponding test using a `using` instead of `typedef`. This has 
> > > been introduced in:
> > > ```
> > > commit 49fdf8d3f5e114e6f8b49fde29a30b0eaaa3c5dd
> > > Author: Dmitri Gribenko 
> > > Date:   Sat Sep 15 21:13:36 2012 +
> > > 
> > > Comment parsing: don't treat typedef to a typedef to a function as a
> > > 'function-like' type that can be annotated with \param.
> > > 
> > > Thanks to Eli Friedman for noticing!
> > > 
> > > llvm-svn: 163985
> > > 
> > > ```
> > > I'm not sure whether or not we should allow this typedef documentation. I 
> > > just tested with Doxygen. It doesn't complain and shows the parameter 
> > > documentation for `test_not_function_like_typedef2`. So on that basis we 
> > > could remove this `expected-warning` and `testTypedefTypeLoc`.
> > > 
> > > What do you think?
> > Thanks for the explanation. I can't find the context for that decision, and 
> > the commit description does not explain the reason either.
> > 
> > It is really a policy decision -- when introducing a comment for function 
> > return value and arguments, should the declaration include the said return 
> > value and arguments, or can they be visible through a typedef?
> > 
> > Thinking about it, my inclination is to say that comments for the return 
> > value and arguments should be present on the declaration that introduces 
> > them. Otherwise, it is breaking an abstraction barrier.
> > 
> > WDYT?
> (I just noticed I forgot to submit this comment.)
> 
> I tend to agree. However Doxygen accepts the code, but Doxygen doesn't seem 
> to validate whether parameters exist.
> So I wonder whether we do our users a service by warning about code that 
> Doxygen properly handles. On that basis I think we can allow it.
> 
> We could add an extra warning in the pedantic group and let the user decide 
> whether or not she thinks it warrants a warning.
> 
> I think that would be the best solution: accept the comment but allow the 
> user to opt-in on a warning.
Doxygen and C++ both allow a lot of dubious stuff. That can't be a reason to 
avoid producing a warning -- we would have no warnings if that was the bar.

I think what we should discuss is, what is the use case for adding a `\returns` 
command in this position -- and how is documenting parameters not a use case. 
If there is a valid use case, how common it is in real world.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66706



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

[PATCH] D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature

[Kristina Bessonova via Phabricator via cfe-commits]

krisb added a comment.

I see. But this doesn't seem like a valid case, does it? Shouldn't we somehow 
diagnose it to not to silently ignore user-specified options?


Repository:
  rC Clang

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

https://reviews.llvm.org/D66018



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

[PATCH] D66505: Make add_new_check.py's insertion of registerCheck<> match the sort order

[Daniel Sanders via Phabricator via cfe-commits]

dsanders added a comment.

Missed one of the commands from my history that I used to verify it:

  ./add_new_check.py llvm i 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66505



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

[PATCH] D66505: Make add_new_check.py's insertion of registerCheck<> match the sort order

[Daniel Sanders via Phabricator via cfe-commits]

dsanders updated this revision to Diff 218137.
dsanders marked an inline comment as done.
dsanders added a comment.

- Full stop at end of comment
- last_line -> prev_line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66505

Files:
  clang-tools-extra/clang-tidy/add_new_check.py


Index: clang-tools-extra/clang-tidy/add_new_check.py
===
--- clang-tools-extra/clang-tidy/add_new_check.py
+++ clang-tools-extra/clang-tidy/add_new_check.py
@@ -165,31 +165,50 @@
 header_added = False
 header_found = False
 check_added = False
+check_fq_name = module + '-' + check_name
 check_decl = ('CheckFactories.registerCheck<' + check_name_camel +
-  '>(\n"' + module + '-' + check_name + '");\n')
-
-for line in lines:
-  if not header_added:
-match = re.search('#include "(.*)"', line)
-if match:
-  header_found = True
-  if match.group(1) > check_name_camel:
+  '>(\n"' + check_fq_name + '");\n')
+
+lines = iter(lines)
+try:
+  while True:
+line = lines.next()
+if not header_added:
+  match = re.search('#include "(.*)"', line)
+  if match:
+header_found = True
+if match.group(1) > check_name_camel:
+  header_added = True
+  f.write('#include "' + check_name_camel + '.h"\n')
+  elif header_found:
 header_added = True
 f.write('#include "' + check_name_camel + '.h"\n')
-elif header_found:
-  header_added = True
-  f.write('#include "' + check_name_camel + '.h"\n')
-
-  if not check_added:
-if line.strip() == '}':
-  check_added = True
-  f.write(check_decl)
-else:
-  match = re.search('registerCheck<(.*)>', line)
-  if match and match.group(1) > check_name_camel:
+
+if not check_added:
+  if line.strip() == '}':
 check_added = True
 f.write(check_decl)
-  f.write(line)
+  else:
+match = re.search('registerCheck<(.*)> *\( *(?:"([^"]*)")?', line)
+prev_line = None
+if match:
+  current_check_name = match.group(2)
+  if current_check_name is None:
+# If we didn't find the check name on this line, look on the
+# next one.
+prev_line = line
+line = lines.next()
+match = re.search(' *"([^"]*)"', line)
+if match:
+  current_check_name = match.group(1)
+  if current_check_name > check_fq_name:
+check_added = True
+f.write(check_decl)
+  if prev_line:
+f.write(prev_line)
+f.write(line)
+except StopIteration:
+  pass
 
 
 # Adds a release notes entry.


Index: clang-tools-extra/clang-tidy/add_new_check.py
===
--- clang-tools-extra/clang-tidy/add_new_check.py
+++ clang-tools-extra/clang-tidy/add_new_check.py
@@ -165,31 +165,50 @@
 header_added = False
 header_found = False
 check_added = False
+check_fq_name = module + '-' + check_name
 check_decl = ('CheckFactories.registerCheck<' + check_name_camel +
-  '>(\n"' + module + '-' + check_name + '");\n')
-
-for line in lines:
-  if not header_added:
-match = re.search('#include "(.*)"', line)
-if match:
-  header_found = True
-  if match.group(1) > check_name_camel:
+  '>(\n"' + check_fq_name + '");\n')
+
+lines = iter(lines)
+try:
+  while True:
+line = lines.next()
+if not header_added:
+  match = re.search('#include "(.*)"', line)
+  if match:
+header_found = True
+if match.group(1) > check_name_camel:
+  header_added = True
+  f.write('#include "' + check_name_camel + '.h"\n')
+  elif header_found:
 header_added = True
 f.write('#include "' + check_name_camel + '.h"\n')
-elif header_found:
-  header_added = True
-  f.write('#include "' + check_name_camel + '.h"\n')
-
-  if not check_added:
-if line.strip() == '}':
-  check_added = True
-  f.write(check_decl)
-else:
-  match = re.search('registerCheck<(.*)>', line)
-  if match and match.group(1) > check_name_camel:
+
+if not check_added:
+  if line.strip() == '}':
 check_added = True
 f.write(check_decl)
-  f.write(line)
+  else:
+match = re.search('registerCheck<(.*)> *\( *(?:"([^"]*)")?', line)
+prev_line = None
+if 

[PATCH] D66505: Make add_new_check.py's insertion of registerCheck<> match the sort order

[Daniel Sanders via Phabricator via cfe-commits]

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

In D66505#1652420 , @alexfh wrote:

> Mostly LG, if you've verified that this works. A couple of comments below.


I verified it using

  ./add_new_check.py llvm prefer-register-over-unsigned
  ./add_new_check.py llvm zzz
  ./add_new_check.py llvm nb
  ./add_new_check.py llvm iz
  ./add_new_check.py llvm a

which is enough to cover every insertion point in the LLVM module




Comment at: clang-tools-extra/clang-tidy/add_new_check.py:192
+  else:
+match = re.search('registerCheck<(.*)> *\( *(?:"([^"]*)")?', line)
+last_line = None

alexfh wrote:
> Are there actually any registerCheck calls with spaces between the `>` and 
> the `(`? Should we clang-format these instead of supporting them in the 
> script?
> Are there actually any registerCheck calls with spaces between the > and the 
> (?

No, it was just trivial to be robust against it in case it does somehow happen. 
Likewise for the whitespace after the '(' but before the '"' or newline.

> Should we clang-format these instead of supporting them in the script?

I think the script should stick to inserting the new code rather than 
correcting existing mistakes that somehow crept in. I do agree that people 
should generally clang-format their patches before commit though which will 
prevent it creeping in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66505



___
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 a comment.

Thanks for the review.

I'm working on splitting up the patch now.

I should be able to address most of your comments, but I left some inline 
comments for clarification/discussion.




Comment at: clang/lib/CodeGen/CodeGenAction.cpp:626
+  unsigned Line, Column;
+  bool BadDebugInfo = false;
+  FullSourceLoc Loc =

lebedev.ri wrote:
> This should be `BadDebugInfoLoc` (in `getBestLocationFromDebugLoc()` too)
That should be in a separate patch, right?



Comment at: llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp:78-87
+  SI.setMetadata(
+  LLVMContext::MD_misexpect,
+  MDBuilder(CI->getContext())
+  .createMisExpect(Index, LikelyBranchWeight, UnlikelyBranchWeight));
+
+  SI.setCondition(ArgValue);
+  misexpect::checkClangInstrumentation(SI);

lebedev.ri wrote:
> Why can't `LLVMContext::MD_prof` be reused?
It didn't seem appropriate to me, but that can be changed if it is the right 
thing to do. 

However, I don't see code elsewhere that loops over `MD_prof` metadata checking 
its tag. I see lots of examples that check if the tag matches, and then exits 
if it doesn't match. 

If I added "misexpect" as a new `MD_prof`tag, then code in places like 
`extractProfTotalWeight(...)` in IR/Metadata.cpp would have to be updated. I'm 
not sure how pervasive that pattern is, but I did  want to avoid introducing 
systemic changes like that.

https://github.com/llvm/llvm-project/blob/9976a5bc1db5a240378a61a68c0e182060bcb554/llvm/lib/IR/Metadata.cpp#L1336



Comment at: llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp:82
 
-  SI.setCondition(ArgValue);
   return true;

lebedev.ri wrote:
> Why do you need to move this line?
I did this so that the condition is available to check when performing 
validation, and the metadata won't be replaced by the likely/unlikely values 
until after I've finished the validation. Having the condition available in 
when performing the check provides for better diagnostics output.  


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] D66808: [ConstExprPreter] Full patch of the interpreter.

[Nandor Licker via Phabricator via cfe-commits]

nand updated this revision to Diff 218131.
nand added a comment.

Added more features


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66808

Files:
  clang/docs/ConstantInterpreter.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/OptionalDiagnostic.h
  clang/include/clang/Driver/Options.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/Interp/Block.cpp
  clang/lib/AST/Interp/Block.h
  clang/lib/AST/Interp/Builtin.cpp
  clang/lib/AST/Interp/Builtin.h
  clang/lib/AST/Interp/ByteCodeEmitter.cpp
  clang/lib/AST/Interp/ByteCodeEmitter.h
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeGenError.cpp
  clang/lib/AST/Interp/ByteCodeGenError.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.h
  clang/lib/AST/Interp/CMakeLists.txt
  clang/lib/AST/Interp/Context.cpp
  clang/lib/AST/Interp/Context.h
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Descriptor.h
  clang/lib/AST/Interp/Disasm.cpp
  clang/lib/AST/Interp/EvalEmitter.cpp
  clang/lib/AST/Interp/EvalEmitter.h
  clang/lib/AST/Interp/Frame.cpp
  clang/lib/AST/Interp/Frame.h
  clang/lib/AST/Interp/Function.cpp
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/HeapUtils.cpp
  clang/lib/AST/Interp/HeapUtils.h
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpFrame.cpp
  clang/lib/AST/Interp/InterpFrame.h
  clang/lib/AST/Interp/InterpStack.cpp
  clang/lib/AST/Interp/InterpStack.h
  clang/lib/AST/Interp/InterpState.cpp
  clang/lib/AST/Interp/InterpState.h
  clang/lib/AST/Interp/Opcode.h
  clang/lib/AST/Interp/Opcodes.td
  clang/lib/AST/Interp/Pointer.cpp
  clang/lib/AST/Interp/Pointer.h
  clang/lib/AST/Interp/Program.cpp
  clang/lib/AST/Interp/Program.h
  clang/lib/AST/Interp/Real.h
  clang/lib/AST/Interp/Record.cpp
  clang/lib/AST/Interp/Record.h
  clang/lib/AST/Interp/Source.cpp
  clang/lib/AST/Interp/Source.h
  clang/lib/AST/Interp/State.cpp
  clang/lib/AST/Interp/State.h
  clang/lib/AST/Interp/Type.cpp
  clang/lib/AST/Interp/Type.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/AST/Interp/alignof.cpp
  clang/test/AST/Interp/arg_struct.cpp
  clang/test/AST/Interp/array.cpp
  clang/test/AST/Interp/array_in_array.cpp
  clang/test/AST/Interp/array_loop_init.cpp
  clang/test/AST/Interp/bitfield.cpp
  clang/test/AST/Interp/boolean.cpp
  clang/test/AST/Interp/bound_member_ptr.cpp
  clang/test/AST/Interp/call.cpp
  clang/test/AST/Interp/case_range.cpp
  clang/test/AST/Interp/cast.cpp
  clang/test/AST/Interp/compound_assign.cpp
  clang/test/AST/Interp/cond.cpp
  clang/test/AST/Interp/default_cons.cpp
  clang/test/AST/Interp/designated_init.cpp
  clang/test/AST/Interp/double_to_int.cpp
  clang/test/AST/Interp/dummy.cpp
  clang/test/AST/Interp/enum.cpp
  clang/test/AST/Interp/extern.cpp
  clang/test/AST/Interp/float.cpp
  clang/test/AST/Interp/flow.cpp
  clang/test/AST/Interp/for_range.cpp
  clang/test/AST/Interp/funcptr.cpp
  clang/test/AST/Interp/global.cpp
  clang/test/AST/Interp/inheritance.cpp
  clang/test/AST/Interp/init.cpp
  clang/test/AST/Interp/init_list_int.cpp
  clang/test/AST/Interp/init_list_ptr.cpp
  clang/test/AST/Interp/int128.cpp
  clang/test/AST/Interp/int64.cpp
  clang/test/AST/Interp/lifetime.cpp
  clang/test/AST/Interp/locals.cpp
  clang/test/AST/Interp/logical.cpp
  clang/test/AST/Interp/loop.cpp
  clang/test/AST/Interp/member_fn.cpp
  clang/test/AST/Interp/member_fn_virtual.cpp
  clang/test/AST/Interp/member_ptr.cpp
  clang/test/AST/Interp/member_ptr_ambiguity.cpp
  clang/test/AST/Interp/member_ptr_derived.cpp
  clang/test/AST/Interp/member_ptr_explicit_path.cpp
  clang/test/AST/Interp/member_ptr_missing.cpp
  clang/test/AST/Interp/member_ptr_print.cpp
  clang/test/AST/Interp/member_ptr_wrong_path.cpp
  clang/test/AST/Interp/method.cpp
  clang/test/AST/Interp/null_method.cpp
  clang/test/AST/Interp/nullptr.cpp
  clang/test/AST/Interp/nullptr_struct.cpp
  clang/test/AST/Interp/operator.cpp
  clang/test/AST/Interp/overflow.cpp
  clang/test/AST/Interp/past_end.cpp
  clang/test/AST/Interp/ptr.cpp
  clang/test/AST/Interp/ptr_derived_cast.cpp
  clang/test/AST/Interp/ptrdiff.cpp
  clang/test/AST/Interp/ptrdiff_overflow.cpp
  clang/test/AST/Interp/pure_virt_ind.cpp
  clang/test/AST/Interp/read_only.cpp
  clang/test/AST/Interp/ref_field.cpp
  clang/test/AST/Interp/relocation.cpp
  clang/test/AST/Interp/return.cpp
  clang/test/AST/Interp/rvo.cpp
  clang/test/AST/Interp/scoping.cpp
  clang/test/AST/Interp/selfref.cpp
  clang/test/AST/Interp/set_field.cpp
  clang/test/AST/Interp/shift.cpp
  clang/test/AST/Interp/sizeof.cpp
  

[PATCH] D67010: [Modules] Move search paths from control block to unhashed control block

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno created this revision.
bruno added reviewers: dexonsmith, arphaman, rsmith.
Herald added subscribers: cfe-commits, ributzka, jkorous.
Herald added a project: clang.

Header search paths are currently ignored for the purpose of computing
`getModuleHash()` during a module build. This means that it doesn't
matter how much one changes header search paths in different clang
invocations, it will always use the same PCM.

This is not really correct but has been mostly "working" ever since.
However, when clang started signing the CONTROL_BLOCK section of the
PCM, which is where the header search paths live in the PCMs, we started
to get unexpected rebuilds when using implicit modules and `signature
mismatch` errors when those are rebuilt in the context of PCHs.

It's inconsistent that we ignore header search paths when selecting a
PCM to build/reuse but consider it as part of the signature of the PCM.
This patch proposes to move serialization of header search paths to
the UNHASHED_CONTROL_BLOCK instead, as we currently do for diagnostics.

Disconsidering the header search paths as part of the signature
shouldn't make it worse for correctness because if the header search
path would change anything, that would change the INPUT_FILE, which
*still is* part of the CONTROL_BLOCK, and therefore will trigger a
module rebuild.

Follow ups from this that might be interesting:

- Introduce -fmodules-strict-header-search, which does consider search paths in 
`getModuleHash()`. This would provide users a way out when they hit bugs 
related to header search paths, but with the downside of bigger module caches 
(in case those paths change often in their builds).
- Optimize the serialization of such paths to only consider the ones that 
actually contribute to the INPUT_LIST. This has the nice effect that a module 
only knows about the paths that are actually relevant to it, increase 
reusability.
- Add warnings or remarks when the header search paths in the PCM don't match 
the ones in the compiler.

There are no testcases for this change, as it would require using
llvm-bcanalyzer in clang tests, which we don't currently do.

rdar://problem/45567811


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67010

Files:
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1327,6 +1327,7 @@
   BLOCK(UNHASHED_CONTROL_BLOCK);
   RECORD(SIGNATURE);
   RECORD(DIAGNOSTIC_OPTIONS);
+  RECORD(HEADER_SEARCH_PATHS);
   RECORD(DIAG_PRAGMA_MAPPINGS);
 
 #undef RECORD
@@ -1446,6 +1447,29 @@
   // are generally transient files and will almost always be overridden.
   Stream.EmitRecord(DIAGNOSTIC_OPTIONS, Record);
 
+  // Header search paths.
+  Record.clear();
+  const HeaderSearchOptions 
+= PP.getHeaderSearchInfo().getHeaderSearchOpts();
+
+  // Include entries.
+  Record.push_back(HSOpts.UserEntries.size());
+  for (unsigned I = 0, N = HSOpts.UserEntries.size(); I != N; ++I) {
+const HeaderSearchOptions::Entry  = HSOpts.UserEntries[I];
+AddString(Entry.Path, Record);
+Record.push_back(static_cast(Entry.Group));
+Record.push_back(Entry.IsFramework);
+Record.push_back(Entry.IgnoreSysRoot);
+  }
+
+  // System header prefixes.
+  Record.push_back(HSOpts.SystemHeaderPrefixes.size());
+  for (unsigned I = 0, N = HSOpts.SystemHeaderPrefixes.size(); I != N; ++I) {
+AddString(HSOpts.SystemHeaderPrefixes[I].Prefix, Record);
+Record.push_back(HSOpts.SystemHeaderPrefixes[I].IsSystemHeader);
+  }
+  Stream.EmitRecord(HEADER_SEARCH_PATHS, Record);
+
   // Write out the diagnostic/pragma mappings.
   WritePragmaDiagnosticMappings(Diags, /* isModule = */ WritingModule);
 
@@ -1649,25 +1673,8 @@
   Record.clear();
   const HeaderSearchOptions 
 = PP.getHeaderSearchInfo().getHeaderSearchOpts();
-  AddString(HSOpts.Sysroot, Record);
-
-  // Include entries.
-  Record.push_back(HSOpts.UserEntries.size());
-  for (unsigned I = 0, N = HSOpts.UserEntries.size(); I != N; ++I) {
-const HeaderSearchOptions::Entry  = HSOpts.UserEntries[I];
-AddString(Entry.Path, Record);
-Record.push_back(static_cast(Entry.Group));
-Record.push_back(Entry.IsFramework);
-Record.push_back(Entry.IgnoreSysRoot);
-  }
-
-  // System header prefixes.
-  Record.push_back(HSOpts.SystemHeaderPrefixes.size());
-  for (unsigned I = 0, N = HSOpts.SystemHeaderPrefixes.size(); I != N; ++I) {
-AddString(HSOpts.SystemHeaderPrefixes[I].Prefix, Record);
-Record.push_back(HSOpts.SystemHeaderPrefixes[I].IsSystemHeader);
-  }
 
+  AddString(HSOpts.Sysroot, Record);
   AddString(HSOpts.ResourceDir, Record);
   AddString(HSOpts.ModuleCachePath, Record);
   AddString(HSOpts.ModuleUserBuildPath, Record);

r370493 - [clang-scan-deps] NFC, remove outdated implementation comment

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Fri Aug 30 10:34:22 2019
New Revision: 370493

URL: http://llvm.org/viewvc/llvm-project?rev=370493=rev
Log:
[clang-scan-deps] NFC, remove outdated implementation comment

There's no need to purge symlinked entries in the FileManager,
as the new FileEntryRef API allows us to compute dependencies more
accurately when the FileManager is reused.

Modified:
cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Modified: cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp?rev=370493=370492=370493=diff
==
--- cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp 
(original)
+++ cfe/trunk/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp Fri 
Aug 30 10:34:22 2019
@@ -87,7 +87,6 @@ public:
 
 // Use the dependency scanning optimized file system if we can.
 if (DepFS) {
-  // FIXME: Purge the symlink entries from the stat cache in the FM.
   const CompilerInvocation  = Compiler.getInvocation();
   // Add any filenames that were explicity passed in the build settings and
   // that might be opened, as we want to ensure we don't run source


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

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:5423
+   SemaRef.Diag(DefaultLoc, diag::err_omp_loop_not_canonical_cond)
+   << IneqCondIsCanonical << LCDecl;
 return true;

I would not suggest to rely on the conversion of bool values to integer, better 
to use `IneqCondIsCanonical ? 0 : 1`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66559



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

[PATCH] D66710: ASTReader: Bypass overridden files when reading PCHs

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno accepted this revision.
bruno added a comment.

LGTM!


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

https://reviews.llvm.org/D66710



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

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

[Chi Chun Chen via Phabricator via cfe-commits]

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

I'm not able to `arc land` my patch since I updated the patch for a minor 
refactor and the state is "not approve" now. Do I wait for the approval for the 
new patch so that I can `arc land` or I don't need to do `arc land` myself?
Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66559



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

[PATCH] D66706: [Wdocumentation] fixes an assertion failure with typedefed function and block pointer

[Mark de Wever via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/Comment.cpp:151
+static bool getFunctionTypeLoc(TypeLoc TL, FunctionTypeLoc ,
+   bool testTypedefTypeLoc = false) {
   TypeLoc PrevTL;

gribozavr wrote:
> Mordante wrote:
> > gribozavr wrote:
> > > Why is the new functionality turned off sometimes? It seems to me that we 
> > > always want to look through typedefs.
> > Setting `testTypedefTypeLoc` to `true` breaks a unit test in 
> > `test/Sema/warn-documentation.cpp:358`
> > 
> > ```
> > typedef int (*test_not_function_like_typedef1)(int aaa);
> > 
> > // expected-warning@+1 {{'\param' command used in a comment that is not 
> > attached to a function declaration}}
> > /// \param aaa Meow.
> > 
> > typedef test_not_function_like_typedef1 test_not_function_like_typedef2;
> > ```
> > and its corresponding test using a `using` instead of `typedef`. This has 
> > been introduced in:
> > ```
> > commit 49fdf8d3f5e114e6f8b49fde29a30b0eaaa3c5dd
> > Author: Dmitri Gribenko 
> > Date:   Sat Sep 15 21:13:36 2012 +
> > 
> > Comment parsing: don't treat typedef to a typedef to a function as a
> > 'function-like' type that can be annotated with \param.
> > 
> > Thanks to Eli Friedman for noticing!
> > 
> > llvm-svn: 163985
> > 
> > ```
> > I'm not sure whether or not we should allow this typedef documentation. I 
> > just tested with Doxygen. It doesn't complain and shows the parameter 
> > documentation for `test_not_function_like_typedef2`. So on that basis we 
> > could remove this `expected-warning` and `testTypedefTypeLoc`.
> > 
> > What do you think?
> Thanks for the explanation. I can't find the context for that decision, and 
> the commit description does not explain the reason either.
> 
> It is really a policy decision -- when introducing a comment for function 
> return value and arguments, should the declaration include the said return 
> value and arguments, or can they be visible through a typedef?
> 
> Thinking about it, my inclination is to say that comments for the return 
> value and arguments should be present on the declaration that introduces 
> them. Otherwise, it is breaking an abstraction barrier.
> 
> WDYT?
(I just noticed I forgot to submit this comment.)

I tend to agree. However Doxygen accepts the code, but Doxygen doesn't seem to 
validate whether parameters exist.
So I wonder whether we do our users a service by warning about code that 
Doxygen properly handles. On that basis I think we can allow it.

We could add an extra warning in the pedantic group and let the user decide 
whether or not she thinks it warrants a warning.

I think that would be the best solution: accept the comment but allow the user 
to opt-in on a warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66706



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

[PATCH] D66989: FileManager: Remove ShouldCloseOpenFile argument from getBufferForFile, NFC

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

dexonsmith closed this revision.
dexonsmith added a comment.

r370488


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

https://reviews.llvm.org/D66989



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

r370488 - FileManager: Remove ShouldCloseOpenFile argument from getBufferForFile, NFC

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

Author: dexonsmith
Date: Fri Aug 30 09:56:26 2019
New Revision: 370488

URL: http://llvm.org/viewvc/llvm-project?rev=370488=rev
Log:
FileManager: Remove ShouldCloseOpenFile argument from getBufferForFile, NFC

Remove this dead code.  We always close it.

Modified:
cfe/trunk/include/clang/Basic/FileManager.h
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/lib/Serialization/ModuleManager.cpp

Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=370488=370487=370488=diff
==
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Fri Aug 30 09:56:26 2019
@@ -317,8 +317,7 @@ public:
   /// Open the specified file as a MemoryBuffer, returning a new
   /// MemoryBuffer if successful, otherwise returning null.
   llvm::ErrorOr>
-  getBufferForFile(const FileEntry *Entry, bool isVolatile = false,
-   bool ShouldCloseOpenFile = true);
+  getBufferForFile(const FileEntry *Entry, bool isVolatile = false);
   llvm::ErrorOr>
   getBufferForFile(StringRef Filename, bool isVolatile = false) {
 return getBufferForFileImpl(Filename, /*FileSize=*/-1, isVolatile);

Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=370488=370487=370488=diff
==
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Fri Aug 30 09:56:26 2019
@@ -426,8 +426,7 @@ void FileManager::fillRealPathName(FileE
 }
 
 llvm::ErrorOr>
-FileManager::getBufferForFile(const FileEntry *Entry, bool isVolatile,
-  bool ShouldCloseOpenFile) {
+FileManager::getBufferForFile(const FileEntry *Entry, bool isVolatile) {
   uint64_t FileSize = Entry->getSize();
   // If there's a high enough chance that the file have changed since we
   // got its size, force a stat before opening it.
@@ -440,10 +439,7 @@ FileManager::getBufferForFile(const File
 auto Result =
 Entry->File->getBuffer(Filename, FileSize,
/*RequiresNullTerminator=*/true, isVolatile);
-// FIXME: we need a set of APIs that can make guarantees about whether a
-// FileEntry is open or not.
-if (ShouldCloseOpenFile)
-  Entry->closeFile();
+Entry->closeFile();
 return Result;
   }
 

Modified: cfe/trunk/lib/Serialization/ModuleManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ModuleManager.cpp?rev=370488=370487=370488=diff
==
--- cfe/trunk/lib/Serialization/ModuleManager.cpp (original)
+++ cfe/trunk/lib/Serialization/ModuleManager.cpp Fri Aug 30 09:56:26 2019
@@ -185,9 +185,7 @@ ModuleManager::addModule(StringRef FileN
   Buf = llvm::MemoryBuffer::getSTDIN();
 } else {
   // Get a buffer of the file and close the file descriptor when done.
-  Buf = FileMgr.getBufferForFile(NewModule->File,
- /*isVolatile=*/false,
- /*ShouldClose=*/true);
+  Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/false);
 }
 
 if (!Buf) {


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

[PATCH] D66989: FileManager: Remove ShouldCloseOpenFile argument from getBufferForFile, NFC

[Michael Spencer via Phabricator via cfe-commits]

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

lgtm


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

https://reviews.llvm.org/D66989



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

[PATCH] D66559: [OPENMP] Update the diagnosis message for canonical loop form

[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, thanks for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66559



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

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

>> tbh, I would appreciate if you would leave the definition of 
>> diagnoseXorMisusedAsPow() where it is and add a forward declare of the 
>> function earlier in the file.

I uploaded the patch almost 2 weeks ago and nobody complained so I thought it 
is okay. Uploaded, fixed.


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

https://reviews.llvm.org/D66397



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

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 updated this revision to Diff 218115.
xbolva00 added a comment.

Fixed review comment.


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

https://reviews.llvm.org/D66397

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/SemaCXX/warn-xor-as-pow.cpp

Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -,7 +,7 @@
   "result of '%0' is %1; did you mean '%2'?">,
   InGroup;
 def note_xor_used_as_pow_silence : Note<
-  "replace expression with '%0' to silence this warning">;
+  "replace expression with '%0' %select{|or use 'xor' instead of '^' }1to silence this warning">;
 
 def warn_null_pointer_compare : Warning<
 "comparison of %select{address of|function|array}0 '%1' %select{not |}2"
Index: test/SemaCXX/warn-xor-as-pow.cpp
===
--- test/SemaCXX/warn-xor-as-pow.cpp
+++ test/SemaCXX/warn-xor-as-pow.cpp
@@ -10,8 +10,10 @@
 #define XOR(x, y) (x ^ y)
 #define TWO 2
 #define TEN 10
+#define IOP 64
 #define TWO_ULL 2ULL
 #define EPSILON 10 ^ -300
+#define ALPHA_OFFSET 3
 
 #define flexor 7
 
@@ -21,7 +23,7 @@
 constexpr long long operator"" _0b(unsigned long long v) { return v; }
 constexpr long long operator"" _0X(unsigned long long v) { return v; }
 #else
-#define xor^ // iso646.h
+#define xor ^ // iso646.h
 #endif
 
 void test(unsigned a, unsigned b) {
@@ -32,25 +34,25 @@
   res = 2 ^ -1;
   res = 2 ^ 0; // expected-warning {{result of '2 ^ 0' is 2; did you mean '1 << 0' (1)?}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1"
-   // expected-note@-2 {{replace expression with '0x2 ^ 0' to silence this warning}}
+   // expected-note@-2 {{replace expression with '0x2 ^ 0' or use 'xor' instead of '^' to silence this warning}}
   res = 2 ^ 1; // expected-warning {{result of '2 ^ 1' is 3; did you mean '1 << 1' (2)?}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1 << 1"
-   // expected-note@-2 {{replace expression with '0x2 ^ 1' to silence this warning}}
+   // expected-note@-2 {{replace expression with '0x2 ^ 1' or use 'xor' instead of '^' to silence this warning}}
   res = 2 ^ 2; // expected-warning {{result of '2 ^ 2' is 0; did you mean '1 << 2' (4)?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1 << 2"
-  // expected-note@-2 {{replace expression with '0x2 ^ 2' to silence this warning}}
+  // expected-note@-2 {{replace expression with '0x2 ^ 2' or use 'xor' instead of '^' to silence this warning}}
   res = 2 ^ 8; // expected-warning {{result of '2 ^ 8' is 10; did you mean '1 << 8' (256)?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:14}:"1 << 8"
-  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
-  res = TWO ^ 8; // expected-warning {{result of 'TWO ^ 8' is 10; did you mean '1 << 8' (256)?}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1 << 8"
-  // expected-note@-2 {{replace expression with '0x2 ^ 8' to silence this warning}}
+  // expected-note@-2 {{replace expression with '0x2 ^ 8' or use 'xor' instead of '^' to silence this warning}}
+  res = 2 ^ +8; // expected-warning {{result of '2 ^ +8' is 10; did you mean '1 << +8' (256)?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1 << +8"
+  // expected-note@-2 {{replace expression with '0x2 ^ +8' or use 'xor' instead of '^' to silence this warning}}
+  res = TWO ^ 8;
   res = 2 ^ 16; // expected-warning {{result of '2 ^ 16' is 18; did you mean '1 << 16' (65536)?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1 << 16"
-  // expected-note@-2 {{replace expression with '0x2 ^ 16' to silence this warning}}
-  res = 2 ^ TEN; // expected-warning {{result of '2 ^ TEN' is 8; did you mean '1 << TEN' (1024)?}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:16}:"1 << TEN"
-  // expected-note@-2 {{replace expression with '0x2 ^ TEN' to silence this warning}}
+  // expected-note@-2 {{replace expression with '0x2 ^ 16' or use 'xor' instead of '^' to silence this warning}}
+  res = 2 ^ TEN;
+  res = res + (2 ^ ALPHA_OFFSET);
   res = 0x2 ^ 16;
   res = 2 xor 16;
 
@@ -69,31 +71,50 @@
   res = TWO_ULL ^ 16;
   res = 2 ^ 32; // expected-warning {{result of '2 ^ 32' is 34; did you mean '1LL << 32'?}}
   // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:15}:"1LL << 32"
-  // expected-note@-2 {{replace expression with '0x2 ^ 32' to silence this warning}}
-  res = 2 ^ 64;
+  // expected-note@-2 {{replace expression with '0x2 ^ 32' or use 'xor' instead of '^' to silence this warning}}
+  res = (2 ^ 64) - 1; // expected-warning {{result of '2 ^ 64' is 66; did you mean '-1LL'?}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:21}:"-1LL"
+  // expected-note@-2 

[PATCH] D66834: Driver tests: set `--sysroot=""` to support clang with `DEFAULT_SYSROOT`

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added a comment.

I think that this is pretty easy to forget.  Fortunately, last argument wins.  
Why not sink this into the `%clang` substitution in lit?  That ensures that we 
run with an empty sysroot and then when the test needs to adjust the sysroot, 
it can do so explicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66834



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

[PATCH] D66995: [clangd] Add highlighting for macro expansions.

[Johan Vikström via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL370482: [clangd] Add highlighting for macro expansions. 
(authored by jvikstrom, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66995?vs=218089=218112#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66995

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

Index: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
@@ -24,16 +24,20 @@
 class HighlightingTokenCollector
 : public RecursiveASTVisitor {
   std::vector Tokens;
-  ASTContext 
-  const SourceManager 
+  ParsedAST 
 
 public:
-  HighlightingTokenCollector(ParsedAST )
-  : Ctx(AST.getASTContext()), SM(AST.getSourceManager()) {}
+  HighlightingTokenCollector(ParsedAST ) : AST(AST) {}
 
   std::vector collectTokens() {
 Tokens.clear();
-TraverseAST(Ctx);
+TraverseAST(AST.getASTContext());
+// Add highlightings for macro expansions as they are not traversed by the
+// visitor.
+// FIXME: Should add highlighting to the macro definitions as well. But this
+// information is not collected in ParsedAST right now.
+for (const SourceLocation  : AST.getMainFileExpansions())
+  addToken(L, HighlightingKind::Macro);
 // Initializer lists can give duplicates of tokens, therefore all tokens
 // must be deduplicated.
 llvm::sort(Tokens);
@@ -264,6 +268,7 @@
   }
 
   void addToken(SourceLocation Loc, HighlightingKind Kind) {
+const auto  = AST.getSourceManager();
 if (Loc.isMacroID()) {
   // Only intereseted in highlighting arguments in macros (DEF_X(arg)).
   if (!SM.isMacroArgExpansion(Loc))
@@ -279,7 +284,7 @@
 if (!isInsideMainFile(Loc, SM))
   return;
 
-auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
+auto R = getTokenRange(SM, AST.getASTContext().getLangOpts(), Loc);
 if (!R) {
   // R should always have a value, if it doesn't something is very wrong.
   elog("Tried to add semantic token with an invalid range");
@@ -466,6 +471,8 @@
 return "entity.name.type.template.cpp";
   case HighlightingKind::Primitive:
 return "storage.type.primitive.cpp";
+  case HighlightingKind::Macro:
+return "entity.name.function.preprocessor.cpp";
   case HighlightingKind::NumKinds:
 llvm_unreachable("must not pass NumKinds to the function");
   }
Index: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
@@ -47,7 +47,8 @@
   {HighlightingKind::Method, "Method"},
   {HighlightingKind::StaticMethod, "StaticMethod"},
   {HighlightingKind::TemplateParameter, "TemplateParameter"},
-  {HighlightingKind::Primitive, "Primitive"}};
+  {HighlightingKind::Primitive, "Primitive"},
+  {HighlightingKind::Macro, "Macro"}};
   std::vector ExpectedTokens;
   for (const auto  : KindToString) {
 std::vector Toks = makeHighlightingTokens(
@@ -391,9 +392,9 @@
   R"cpp(
   #define DEF_MULTIPLE(X) namespace X { class X { int X; }; }
   #define DEF_CLASS(T) class T {};
-  DEF_MULTIPLE(XYZ);
-  DEF_MULTIPLE(XYZW);
-  DEF_CLASS($Class[[A]])
+  $Macro[[DEF_MULTIPLE]](XYZ);
+  $Macro[[DEF_MULTIPLE]](XYZW);
+  $Macro[[DEF_CLASS]]($Class[[A]])
   #define MACRO_CONCAT(X, V, T) T foo##X = V
   #define DEF_VAR(X, V) int X = V
   #define DEF_VAR_T(T, X, V) T X = V
@@ -404,26 +405,27 @@
   #define SOME_NAME_SET variable2 = 123
   #define INC_VAR(X) X += 2
   $Primitive[[void]] $Function[[foo]]() {
-DEF_VAR($LocalVariable[[X]],  123);
-DEF_VAR_REV(908, $LocalVariable[[XY]]);
-$Primitive[[int]] CPY( $LocalVariable[[XX]] );
-DEF_VAR_TYPE($Class[[A]], $LocalVariable[[AA]]);
-$Primitive[[double]] SOME_NAME;
-$Primitive[[int]] SOME_NAME_SET;
+$Macro[[DEF_VAR]]($LocalVariable[[X]],  123);
+$Macro[[DEF_VAR_REV]](908, $LocalVariable[[XY]]);
+$Primitive[[int]] $Macro[[CPY]]( $LocalVariable[[XX]] );
+$Macro[[DEF_VAR_TYPE]]($Class[[A]], $LocalVariable[[AA]]);
+$Primitive[[double]] $Macro[[SOME_NAME]];
+$Primitive[[int]] $Macro[[SOME_NAME_SET]];
 $LocalVariable[[variable]] = 20.1;
-MACRO_CONCAT(var, 2, $Primitive[[float]]);
- 

[PATCH] D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature

[Diogo N. Sampaio via Phabricator via cfe-commits]

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

`clang -### -target arm-arm-none-eabit -march=armv8-m.main+crypto` did not show 
+sha2 or +aes. After the patch it does.
I believe that is not expected, as in ARM.td `crypto` is not applied for any M 
profile. And Arm®v8-M Architecture Reference Manual does not reference these 
extensions neither.




Comment at: lib/Driver/ToolChains/Arch/ARM.cpp:485
+
+  if (llvm::ARM::parseArchVersion(ArchSuffix) >= 8) {
+auto CryptoIt =

` && (llvm::ARM::parseArchProfile(Arch) == llvm::ARM::ProfileKind::A)`



Repository:
  rC Clang

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

https://reviews.llvm.org/D66018



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

[clang-tools-extra] r370482 - [clangd] Add highlighting for macro expansions.

[Johan Vikstrom via cfe-commits]

Author: jvikstrom
Date: Fri Aug 30 08:47:27 2019
New Revision: 370482

URL: http://llvm.org/viewvc/llvm-project?rev=370482=rev
Log:
[clangd] Add highlighting for macro expansions.

Summary: https://github.com/clangd/clangd/issues/134

Reviewers: hokein, ilya-biryukov

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

Tags: #clang

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

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

Modified: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp?rev=370482=370481=370482=diff
==
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp Fri Aug 30 08:47:27 
2019
@@ -24,16 +24,20 @@ namespace {
 class HighlightingTokenCollector
 : public RecursiveASTVisitor {
   std::vector Tokens;
-  ASTContext 
-  const SourceManager 
+  ParsedAST 
 
 public:
-  HighlightingTokenCollector(ParsedAST )
-  : Ctx(AST.getASTContext()), SM(AST.getSourceManager()) {}
+  HighlightingTokenCollector(ParsedAST ) : AST(AST) {}
 
   std::vector collectTokens() {
 Tokens.clear();
-TraverseAST(Ctx);
+TraverseAST(AST.getASTContext());
+// Add highlightings for macro expansions as they are not traversed by the
+// visitor.
+// FIXME: Should add highlighting to the macro definitions as well. But 
this
+// information is not collected in ParsedAST right now.
+for (const SourceLocation  : AST.getMainFileExpansions())
+  addToken(L, HighlightingKind::Macro);
 // Initializer lists can give duplicates of tokens, therefore all tokens
 // must be deduplicated.
 llvm::sort(Tokens);
@@ -264,6 +268,7 @@ private:
   }
 
   void addToken(SourceLocation Loc, HighlightingKind Kind) {
+const auto  = AST.getSourceManager();
 if (Loc.isMacroID()) {
   // Only intereseted in highlighting arguments in macros (DEF_X(arg)).
   if (!SM.isMacroArgExpansion(Loc))
@@ -279,7 +284,7 @@ private:
 if (!isInsideMainFile(Loc, SM))
   return;
 
-auto R = getTokenRange(SM, Ctx.getLangOpts(), Loc);
+auto R = getTokenRange(SM, AST.getASTContext().getLangOpts(), Loc);
 if (!R) {
   // R should always have a value, if it doesn't something is very wrong.
   elog("Tried to add semantic token with an invalid range");
@@ -466,6 +471,8 @@ llvm::StringRef toTextMateScope(Highligh
 return "entity.name.type.template.cpp";
   case HighlightingKind::Primitive:
 return "storage.type.primitive.cpp";
+  case HighlightingKind::Macro:
+return "entity.name.function.preprocessor.cpp";
   case HighlightingKind::NumKinds:
 llvm_unreachable("must not pass NumKinds to the function");
   }

Modified: clang-tools-extra/trunk/clangd/SemanticHighlighting.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SemanticHighlighting.h?rev=370482=370481=370482=diff
==
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.h (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.h Fri Aug 30 08:47:27 
2019
@@ -38,6 +38,7 @@ enum class HighlightingKind {
   Namespace,
   TemplateParameter,
   Primitive,
+  Macro,
 
   NumKinds,
 };

Modified: clang-tools-extra/trunk/clangd/test/semantic-highlighting.test
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/semantic-highlighting.test?rev=370482=370481=370482=diff
==
--- clang-tools-extra/trunk/clangd/test/semantic-highlighting.test (original)
+++ clang-tools-extra/trunk/clangd/test/semantic-highlighting.test Fri Aug 30 
08:47:27 2019
@@ -45,6 +45,9 @@
 # CHECK-NEXT:  ],
 # CHECK-NEXT:  [
 # CHECK-NEXT:"storage.type.primitive.cpp"
+# CHECK-NEXT:  ],
+# CHECK-NEXT:  [
+# CHECK-NEXT:"entity.name.function.preprocessor.cpp"
 # CHECK-NEXT:  ]
 # CHECK-NEXT:]
 # CHECK-NEXT:  },

Modified: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp?rev=370482=370481=370482=diff
==
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp 
(original)
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp Fri 
Aug 30 08:47:27 2019
@@ -47,7 +47,8 @@ std::vector getExpect
   {HighlightingKind::Method, "Method"},
  

r370481 - Revert [Clang Interpreter] Initial patch for the constexpr interpreter

[Nandor Licker via cfe-commits]

Author: nand
Date: Fri Aug 30 08:41:45 2019
New Revision: 370481

URL: http://llvm.org/viewvc/llvm-project?rev=370481=rev
Log:
Revert [Clang Interpreter] Initial patch for the constexpr interpreter

This reverts r370476 (git commit a5590950549719d0d9ea69ed164b0c8c0f4e02e6)

Removed:
cfe/trunk/docs/ConstantInterpreter.rst
cfe/trunk/include/clang/Basic/OptionalDiagnostic.h
cfe/trunk/lib/AST/Interp/
cfe/trunk/test/AST/Interp/
cfe/trunk/utils/TableGen/ClangOpcodesEmitter.cpp
Modified:
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/include/clang/Basic/DiagnosticASTKinds.td
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/CMakeLists.txt
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/SemaCXX/constant-expression-cxx2a.cpp
cfe/trunk/test/SemaCXX/constexpr-many-arguments.cpp
cfe/trunk/test/SemaCXX/shift.cpp
cfe/trunk/utils/TableGen/CMakeLists.txt
cfe/trunk/utils/TableGen/TableGen.cpp
cfe/trunk/utils/TableGen/TableGenBackends.h

Removed: cfe/trunk/docs/ConstantInterpreter.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ConstantInterpreter.rst?rev=370480=auto
==
--- cfe/trunk/docs/ConstantInterpreter.rst (original)
+++ cfe/trunk/docs/ConstantInterpreter.rst (removed)
@@ -1,194 +0,0 @@
-
-Constant Interpreter
-
-
-.. contents::
-   :local:
-
-Introduction
-
-
-The constexpr interpreter aims to replace the existing tree evaluator in 
clang, improving performance on constructs which are executed inefficiently by 
the evaluator. The interpreter is activated using the following flags:
-
-* ``-fexperimental-new-constant-interpreter`` enables the interpreter, falling 
back to the evaluator for unsupported features
-* ``-fforce-experimental-new-constant-interpreter`` forces the use of the 
interpreter, bailing out if an unsupported feature is encountered
-
-Bytecode Compilation
-
-
-Bytecode compilation is handled in ``ByteCodeStmtGen.h`` for statements and 
``ByteCodeExprGen.h`` for expressions. The compiler has two different backends: 
one to generate bytecode for functions (``ByteCodeEmitter``) and one to 
directly evaluate expressions as they are compiled, without generating bytecode 
(``EvalEmitter``). All functions are compiled to bytecode, while toplevel 
expressions used in constant contexts are directly evaluated since the bytecode 
would never be reused. This mechanism aims to pave the way towards replacing 
the evaluator, improving its performance on functions and loops, while being 
just as fast on single-use toplevel expressions.
-
-The interpreter relies on stack-based, strongly-typed opcodes. The glue logic 
between the code generator, along with the enumeration and description of 
opcodes, can be found in ``Opcodes.td``. The opcodes are implemented as generic 
template methods in ``Interp.h`` and instantiated with the relevant primitive 
types by the interpreter loop or by the evaluating emitter.
-
-Primitive Types

-
-* ``PT_{U|S}int{8|16|32|64}``
-
-  Signed or unsigned integers of a specific bit width, implemented using the 
```Integral``` type.
-
-* ``PT_{U|S}intFP``
-
-  Signed or unsigned integers of an arbitrary, but fixed width used to 
implement
-  integral types which are required by the target, but are not supported by 
the host.
-  Under the hood, they rely on APValue. The ``Integral`` specialisation for 
these
-  types is required by opcodes to share an implementation with fixed integrals.
-
-* ``PT_Bool``
-
-  Representation for boolean types, essentially a 1-bit unsigned ``Integral``.
-
-* ``PT_RealFP``
-
-  Arbitrary, but fixed precision floating point numbers. Could be specialised 
in
-  the future similarly to integers in order to improve floating point 
performance.
-
-* ``PT_Ptr``
-
-  Pointer type, defined in ``"Pointer.h"``.
-
-* ``PT_FnPtr``
-
-  Function pointer type, can also be a null function pointer. Defined in 
``"Pointer.h"``.
-
-* ``PT_MemPtr``
-
-  Member pointer type, can also be a null member pointer. Defined in 
``"Pointer.h"``
-
-Composite types

-
-The interpreter distinguishes two kinds of composite types: arrays and 
records. Unions are represented as records, except a single field can be marked 
as active. The contents of inactive fields are kept until they
-are reactivated and overwritten.
-
-
-Bytecode Execution
-==
-
-Bytecode is executed using a stack-based interpreter. The execution context 
consists of an ``InterpStack``, along with a chain of ``InterpFrame`` objects 
storing the call frames. Frames are built by call instructions and destroyed by 
return instructions. They perform one allocation to 

[PATCH] D59922: [Attributor] Deduce "no-capture" argument attribute

[Johannes Doerfert via Phabricator via cfe-commits]

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

Update tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59922

Files:
  llvm/include/llvm/Transforms/IPO/Attributor.h
  llvm/lib/Transforms/IPO/Attributor.cpp
  llvm/test/Transforms/FunctionAttrs/align.ll
  llvm/test/Transforms/FunctionAttrs/arg_nocapture.ll
  llvm/test/Transforms/FunctionAttrs/arg_returned.ll
  llvm/test/Transforms/FunctionAttrs/dereferenceable.ll
  llvm/test/Transforms/FunctionAttrs/internal-noalias.ll
  llvm/test/Transforms/FunctionAttrs/liveness.ll
  llvm/test/Transforms/FunctionAttrs/noalias_returned.ll
  llvm/test/Transforms/FunctionAttrs/nocapture.ll
  llvm/test/Transforms/FunctionAttrs/nonnull.ll
  llvm/test/Transforms/FunctionAttrs/nosync.ll
  llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll

Index: llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll
===
--- llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll
+++ llvm/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll
@@ -102,7 +102,7 @@
 }
 
 ; CHECK: Function Attrs: nofree norecurse nosync nounwind
-; CHECK-NEXT: define i32* @external_sink_ret2_nrw(i32* readnone %n0, i32* nocapture readonly %r0, i32* returned %w0)
+; CHECK-NEXT: define i32* @external_sink_ret2_nrw(i32* readnone %n0, i32* nocapture readonly %r0, i32* returned "no-capture-maybe-returned" %w0)
 define i32* @external_sink_ret2_nrw(i32* %n0, i32* %r0, i32* %w0) {
 entry:
   %tobool = icmp ne i32* %n0, null
Index: llvm/test/Transforms/FunctionAttrs/nosync.ll
===
--- llvm/test/Transforms/FunctionAttrs/nosync.ll
+++ llvm/test/Transforms/FunctionAttrs/nosync.ll
@@ -28,7 +28,7 @@
 ; FNATTR: Function Attrs: norecurse nounwind optsize readnone ssp uwtable
 ; FNATTR-NEXT: define nonnull i32* @foo(%struct.ST* readnone %s)
 ; ATTRIBUTOR: Function Attrs: nofree nosync nounwind optsize readnone ssp uwtable
-; ATTRIBUTOR-NEXT: define nonnull i32* @foo(%struct.ST* %s)
+; ATTRIBUTOR-NEXT: define nonnull i32* @foo(%struct.ST* "no-capture-maybe-returned" %s)
 define i32* @foo(%struct.ST* %s) nounwind uwtable readnone optsize ssp {
 entry:
   %arrayidx = getelementptr inbounds %struct.ST, %struct.ST* %s, i64 1, i32 2, i32 1, i64 5, i64 13
@@ -186,7 +186,7 @@
 ; FNATTR: Function Attrs: nofree noinline nounwind uwtable
 ; FNATTR-NEXT: define i32 @scc1(i32* %0)
 ; ATTRIBUTOR: Function Attrs: nofree noinline noreturn nosync nounwind uwtable
-; ATTRIBUTOR-NEXT: define i32 @scc1(i32* %0)
+; ATTRIBUTOR-NEXT: define i32 @scc1(i32* nocapture %0)
 define i32 @scc1(i32* %0) noinline nounwind uwtable {
   tail call void @scc2(i32* %0);
   %val = tail call i32 @volatile_load(i32* %0);
@@ -196,7 +196,7 @@
 ; FNATTR: Function Attrs: nofree noinline nounwind uwtable
 ; FNATTR-NEXT: define void @scc2(i32* %0)
 ; ATTRIBUTOR: Function Attrs: nofree noinline noreturn nosync nounwind uwtable
-; ATTRIBUTOR-NEXT: define void @scc2(i32* %0)
+; ATTRIBUTOR-NEXT: define void @scc2(i32* nocapture %0)
 define void @scc2(i32* %0) noinline nounwind uwtable {
   tail call i32 @scc1(i32* %0);
   ret void;
@@ -224,7 +224,7 @@
 ; FNATTR: Function Attrs: nofree norecurse nounwind
 ; FNATTR-NEXT: define void @foo1(i32* nocapture %0, %"struct.std::atomic"* nocapture %1)
 ; ATTRIBUTOR-NOT: nosync
-; ATTRIBUTOR: define void @foo1(i32* %0, %"struct.std::atomic"* %1)
+; ATTRIBUTOR: define void @foo1(i32* nocapture %0, %"struct.std::atomic"* nocapture %1)
 define void @foo1(i32* %0, %"struct.std::atomic"* %1) {
   store i32 100, i32* %0, align 4
   fence release
@@ -236,7 +236,7 @@
 ; FNATTR: Function Attrs: nofree norecurse nounwind
 ; FNATTR-NEXT: define void @bar(i32* nocapture readnone %0, %"struct.std::atomic"* nocapture readonly %1)
 ; ATTRIBUTOR-NOT: nosync
-; ATTRIBUTOR: define void @bar(i32* %0, %"struct.std::atomic"* %1)
+; ATTRIBUTOR: define void @bar(i32* nocapture %0, %"struct.std::atomic"* nocapture %1)
 define void @bar(i32* %0, %"struct.std::atomic"* %1) {
   %3 = getelementptr inbounds %"struct.std::atomic", %"struct.std::atomic"* %1, i64 0, i32 0, i32 0
   br label %4
@@ -256,7 +256,7 @@
 ; FNATTR: Function Attrs: nofree norecurse nounwind
 ; FNATTR-NEXT: define void @foo1_singlethread(i32* nocapture %0, %"struct.std::atomic"* nocapture %1)
 ; ATTRIBUTOR: Function Attrs: nofree nosync
-; ATTRIBUTOR: define void @foo1_singlethread(i32* %0, %"struct.std::atomic"* %1)
+; ATTRIBUTOR: define void @foo1_singlethread(i32* nocapture %0, %"struct.std::atomic"* nocapture %1)
 define void @foo1_singlethread(i32* %0, %"struct.std::atomic"* %1) {
   store i32 100, i32* %0, align 4
   fence syncscope("singlethread") release
@@ -268,7 +268,7 @@
 ; FNATTR: Function Attrs: nofree norecurse nounwind
 ; FNATTR-NEXT: define void @bar_singlethread(i32* nocapture