[PATCH] D71197: llvm premerge: clang format test

[Mikhail Goncharov via Phabricator via cfe-commits]

goncharov added a comment.

Build result: fail - 60594 tests passed, 30 failed and 726 were skipped.

  failed: 
libc++.std/input_output/filesystems/class_directory_entry/directory_entry_cons/path.pass.cpp
  failed: 
libc++.std/input_output/filesystems/class_directory_entry/directory_entry_mods/assign.pass.cpp
  failed: 
libc++.std/input_output/filesystems/class_directory_entry/directory_entry_mods/refresh.pass.cpp
  failed: 
libc++.std/input_output/filesystems/class_directory_entry/directory_entry_mods/replace_filename.pass.cpp
  failed: 
libc++.std/input_output/filesystems/class_directory_entry/directory_entry_obs/file_size.pass.cpp
  failed: 
libc++.std/input_output/filesystems/class_directory_entry/directory_entry_obs/file_type_obs.pass.cpp
  failed: 
libc++.std/input_output/filesystems/class_directory_entry/directory_entry_obs/hard_link_count.pass.cpp
  failed: 
libc++.std/input_output/filesystems/class_directory_entry/directory_entry_obs/last_write_time.pass.cpp
  failed: 
libc++.std/input_output/filesystems/class_directory_iterator/directory_iterator_members/ctor.pass.cpp
  failed: 
libc++.std/input_output/filesystems/class_rec_dir_itr/rec_dir_itr_members/ctor.pass.cpp
  failed: 
libc++.std/input_output/filesystems/class_rec_dir_itr/rec_dir_itr_members/increment.pass.cpp
  failed: 
libc++.std/input_output/filesystems/fs_op_funcs/fs_op_exists/exists.pass.cpp
  failed: 
libc++.std/input_output/filesystems/fs_op_funcs/fs_op_is_block_file/is_block_file.pass.cpp
  failed: 
libc++.std/input_output/filesystems/fs_op_funcs/fs_op_is_char_file/is_character_file.pass.cpp
  failed: 
libc++.std/input_output/filesystems/fs_op_funcs/fs_op_is_directory/is_directory.pass.cpp
  failed: 
libc++.std/input_output/filesystems/fs_op_funcs/fs_op_is_empty/is_empty.pass.cpp
  failed: 
libc++.std/input_output/filesystems/fs_op_funcs/fs_op_is_fifo/is_fifo.pass.cpp
  failed: 
libc++.std/input_output/filesystems/fs_op_funcs/fs_op_is_other/is_other.pass.cpp
  failed: 
libc++.std/input_output/filesystems/fs_op_funcs/fs_op_is_regular_file/is_regular_file.pass.cpp
  failed: 
libc++.std/input_output/filesystems/fs_op_funcs/fs_op_is_socket/is_socket.pass.cpp
  failed: 
libc++.std/input_output/filesystems/fs_op_funcs/fs_op_is_symlink/is_symlink.pass.cpp
  failed: 
libc++.std/input_output/filesystems/fs_op_funcs/fs_op_last_write_time/last_write_time.pass.cpp
  failed: 
libc++.std/input_output/filesystems/fs_op_funcs/fs_op_remove/remove.pass.cpp
  failed: 
libc++.std/input_output/filesystems/fs_op_funcs/fs_op_remove_all/remove_all.pass.cpp
  failed: 
libc++.std/input_output/filesystems/fs_op_funcs/fs_op_status/status.pass.cpp
  failed: 
libc++.std/input_output/filesystems/fs_op_funcs/fs_op_symlink_status/symlink_status.pass.cpp
  failed: 
libc++.std/input_output/filesystems/fs_op_funcs/fs_op_temp_dir_path/temp_directory_path.pass.cpp
  failed: lld.COFF/thinlto-emit-imports.ll
  failed: lld.ELF/lto/thinlto-cant-write-index.ll
  failed: lld.ELF/lto/thinlto-emit-imports.ll


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71197



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

[PATCH] D71197: llvm premerge: clang format test

[Mikhail Goncharov via Phabricator via cfe-commits]

goncharov added a comment.

Build result: SUCCESS -


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71197



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

[PATCH] D71208: CodeGen: Allow annotations on globals in non-zero address space

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D71208#1776633 , @nhaehnle wrote:

> In D71208#1776202 , @rjmccall wrote:
>
> > The questions I'd like to have answered before I can approve this are:
> >
> > - whether there are clients of `@llvm.global.annotations` that will have 
> > problems with non-0 address spaces and
>
>
> Yes, that's where this patch is ultimately coming from. I have some 
> work-in-progress code to generate IRBuilder-using C++ code from LLVM IR, 
> which we're currently using internally (with plans to upstream later next 
> year) for an AMDGPU frontend, where non-0 address spaces are used. Using 
> annotations is the least-invasive way I've found to attach required 
> additional data to LLVM IR generated from C...


My concern is that there's something that's going to blow up or miscompile if 
we start passing in constants that aren't in a regular address space.  Aren't 
there kinds of annotations which get persisted into the emitted code?

>> - whether this will interfere with someday having an invariant that 
>> `addrspacecast` is only used to do legal conversions.
> 
> That could potentially be a problem at some point, but this patch isn't 
> making the situation any worse. Without this patch, Clang just 
> unconditionally crashes (hits an assertion) when there's an annotation on a 
> non-0 address space variable. With this patch, in the worst case the 
> crash/assertion comes later...

Well, it's making it worse in the sense that this global-annotations list would 
become a blocker for tightening the representation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71208



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

[PATCH] D71208: CodeGen: Allow annotations on globals in non-zero address space

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added a comment.

In D71208#1776202 , @rjmccall wrote:

> The questions I'd like to have answered before I can approve this are:
>
> - whether there are clients of `@llvm.global.annotations` that will have 
> problems with non-0 address spaces and


Yes, that's where this patch is ultimately coming from. I have some 
work-in-progress code to generate IRBuilder-using C++ code from LLVM IR, which 
we're currently using internally (with plans to upstream later next year) for 
an AMDGPU frontend, where non-0 address spaces are used. Using annotations is 
the least-invasive way I've found to attach required additional data to LLVM IR 
generated from C...

> - whether this will interfere with someday having an invariant that 
> `addrspacecast` is only used to do legal conversions.

That could potentially be a problem at some point, but this patch isn't making 
the situation any worse. Without this patch, Clang just unconditionally crashes 
(hits an assertion) when there's an annotation on a non-0 address space 
variable. With this patch, in the worst case the crash/assertion comes later...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71208



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

[PATCH] D70856: [Syntax] Build nodes for simple cases of top level declarations

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 60626 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70856



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

[PATCH] D70856: [Syntax] Build nodes for simple cases of top level declarations

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 233004.
ilya-biryukov added a comment.

- Do not call Builder.getRange() twice for namespaces


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70856

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

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -512,7 +512,190 @@
 | | `-tb
 | `-;
 `-}
-  )txt"}};
+  )txt"},
+  {R"cpp(
+namespace a { namespace b {} }
+namespace a::b {}
+namespace {}
+
+namespace foo = a;
+)cpp",
+   R"txt(
+*: TranslationUnit
+|-NamespaceDefinition
+| |-namespace
+| |-a
+| |-{
+| |-NamespaceDefinition
+| | |-namespace
+| | |-b
+| | |-{
+| | `-}
+| `-}
+|-NamespaceDefinition
+| |-namespace
+| |-a
+| |-::
+| |-b
+| |-{
+| `-}
+|-NamespaceDefinition
+| |-namespace
+| |-{
+| `-}
+`-NamespaceAliasDefinition
+  |-namespace
+  |-foo
+  |-=
+  |-a
+  `-;
+)txt"},
+  {R"cpp(
+namespace ns {}
+using namespace ::ns;
+)cpp",
+   R"txt(
+*: TranslationUnit
+|-NamespaceDefinition
+| |-namespace
+| |-ns
+| |-{
+| `-}
+`-UsingNamespaceDirective
+  |-using
+  |-namespace
+  |-::
+  |-ns
+  `-;
+   )txt"},
+  {R"cpp(
+namespace ns { int a; }
+using ns::a;
+)cpp",
+   R"txt(
+*: TranslationUnit
+|-NamespaceDefinition
+| |-namespace
+| |-ns
+| |-{
+| |-SimpleDeclaration
+| | |-int
+| | |-a
+| | `-;
+| `-}
+`-UsingDeclaration
+  |-using
+  |-ns
+  |-::
+  |-a
+  `-;
+   )txt"},
+  {R"cpp(
+template  struct X {
+  using T::foo;
+  using typename T::bar;
+};
+)cpp",
+   R"txt(
+*: TranslationUnit
+`-UnknownDeclaration
+  |-template
+  |-<
+  |-UnknownDeclaration
+  | |-class
+  | `-T
+  |->
+  |-struct
+  |-X
+  |-{
+  |-UsingDeclaration
+  | |-using
+  | |-T
+  | |-::
+  | |-foo
+  | `-;
+  |-UsingDeclaration
+  | |-using
+  | |-typename
+  | |-T
+  | |-::
+  | |-bar
+  | `-;
+  |-}
+  `-;
+   )txt"},
+  {R"cpp(
+using type = int;
+)cpp",
+   R"txt(
+*: TranslationUnit
+`-TypeAliasDeclaration
+  |-using
+  |-type
+  |-=
+  |-int
+  `-;
+   )txt"},
+  {R"cpp(
+;
+)cpp",
+   R"txt(
+*: TranslationUnit
+`-EmptyDeclaration
+  `-;
+   )txt"},
+  {R"cpp(
+static_assert(true, "message");
+static_assert(true);
+)cpp",
+   R"txt(
+*: TranslationUnit
+|-StaticAssertDeclaration
+| |-static_assert
+| |-(
+| |-UnknownExpression
+| | `-true
+| |-,
+| |-UnknownExpression
+| | `-"message"
+| |-)
+| `-;
+`-StaticAssertDeclaration
+  |-static_assert
+  |-(
+  |-UnknownExpression
+  | `-true
+  |-)
+  `-;
+   )txt"},
+  {R"cpp(
+extern "C" int a;
+extern "C" { int b; int c; }
+)cpp",
+   R"txt(
+*: TranslationUnit
+|-LinkageSpecificationDeclaration
+| |-extern
+| |-"C"
+| `-SimpleDeclaration
+|   |-int
+|   |-a
+|   `-;
+`-LinkageSpecificationDeclaration
+  |-extern
+  |-"C"
+  |-{
+  |-SimpleDeclaration
+  | |-int
+  | |-b
+  | `-;
+  |-SimpleDeclaration
+  | |-int
+  | |-c
+  | `-;
+  `-}
+   )txt"},
+  };
 
   for (const auto  : Cases) {
 SCOPED_TRACE(T.first);
Index: clang/lib/Tooling/Syntax/Nodes.cpp
===
--- clang/lib/Tooling/Syntax/Nodes.cpp
+++ clang/lib/Tooling/Syntax/Nodes.cpp
@@ -50,8 +50,24 @@
 return OS << "CompoundStatement";
   case NodeKind::UnknownDeclaration:
 return OS << "UnknownDeclaration";
+  case NodeKind::EmptyDeclaration:
+return OS << "EmptyDeclaration";
+  case NodeKind::StaticAssertDeclaration:
+return OS << "StaticAssertDeclaration";
+  case NodeKind::LinkageSpecificationDeclaration:
+return OS << "LinkageSpecificationDeclaration";
   case NodeKind::SimpleDeclaration:
 return OS << "SimpleDeclaration";
+  case NodeKind::NamespaceDefinition:
+return OS << "NamespaceDefinition";
+  case NodeKind::NamespaceAliasDefinition:
+return OS << "NamespaceAliasDefinition";
+  case NodeKind::UsingNamespaceDirective:
+return OS << "UsingNamespaceDirective";
+  case NodeKind::UsingDeclaration:
+return OS << "UsingDeclaration";
+  case NodeKind::TypeAliasDeclaration:
+return OS << "TypeAliasDeclaration";
   }
   llvm_unreachable("unknown node kind");
 }
@@ -84,6 +100,10 @@
 return OS << "ExpressionStatement_expression";
   case syntax::NodeRole::CompoundStatement_statement:
 return OS << "CompoundStatement_statement";
+  case syntax::NodeRole::StaticAssertDeclaration_condition:
+return OS << "StaticAssertDeclaration_condition";
+  case syntax::NodeRole::StaticAssertDeclaration_message:
+return OS << "StaticAssertDeclaration_message";
   }
   llvm_unreachable("invalid role");
 }
@@ -216,3 +236,13 @@
   return llvm::cast_or_null(
   

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

[Johannes Doerfert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGeb3e81f43f01: [OpenMP][NFCI] Introduce 
llvm/IR/OpenMPConstants.h (authored by jdoerfert).

Changed prior to commit:
  https://reviews.llvm.org/D69853?vs=232987=233003#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69853

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/StmtOpenMP.h
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/OpenMPKinds.h
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/CMakeLists.txt
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/CMakeLists.txt
  llvm/lib/Frontend/CMakeLists.txt
  llvm/lib/Frontend/LLVMBuild.txt
  llvm/lib/Frontend/OpenMP/CMakeLists.txt
  llvm/lib/Frontend/OpenMP/LLVMBuild.txt
  llvm/lib/Frontend/OpenMP/OMPConstants.cpp
  llvm/lib/LLVMBuild.txt

Index: llvm/lib/Frontend/OpenMP/OMPConstants.cpp
===
--- /dev/null
+++ llvm/lib/Frontend/OpenMP/OMPConstants.cpp
@@ -0,0 +1,34 @@
+//===- OMPConstants.cpp - Helpers related to OpenMP code generation ---===//
+//
+// 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 "llvm/Frontend/OpenMP/OMPConstants.h"
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
+
+using namespace llvm;
+using namespace omp;
+
+Directive llvm::omp::getOpenMPDirectiveKind(StringRef Str) {
+  return llvm::StringSwitch(Str)
+#define OMP_DIRECTIVE(Enum, Str) .Case(Str, Enum)
+#include "llvm/Frontend/OpenMP/OMPKinds.def"
+  .Default(OMPD_unknown);
+}
+
+StringRef llvm::omp::getOpenMPDirectiveName(Directive Kind) {
+  switch (Kind) {
+#define OMP_DIRECTIVE(Enum, Str)   \
+  case Enum:   \
+return Str;
+#include "llvm/Frontend/OpenMP/OMPKinds.def"
+  }
+  llvm_unreachable("Invalid OpenMP directive kind");
+}
Index: llvm/lib/Frontend/OpenMP/LLVMBuild.txt
===
--- llvm/lib/Frontend/OpenMP/LLVMBuild.txt
+++ llvm/lib/Frontend/OpenMP/LLVMBuild.txt
@@ -1,4 +1,4 @@
-;===- ./lib/LLVMBuild.txt --*- Conf -*--===;
+;===- ./lib/Frontend/OpenMP/LLVMBuild.txt --*- Conf -*--===;
 ;
 ; Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 ; See https://llvm.org/LICENSE.txt for license information.
@@ -14,42 +14,8 @@
 ;
 ;======;
 
-[common]
-subdirectories =
- Analysis
- AsmParser
- Bitcode
- Bitstream
- CodeGen
- DebugInfo
- Demangle
- ExecutionEngine
- FuzzMutate
- LineEditor
- Linker
- IR
- IRReader
- LTO
- MC
- MCA
- Object
- BinaryFormat
- ObjectYAML
- Option
- Remarks
- Passes
- ProfileData
- Support
- TableGen
- TextAPI
- Target
- Testing
- ToolDrivers
- Transforms
- WindowsManifest
- XRay
-
 [component_0]
-type = Group
-name = Libraries
-parent = $ROOT
+type = Library
+name = FrontendOpenMP
+parent = Frontend
+required_libraries = Core Support TransformUtils
Index: llvm/lib/Frontend/OpenMP/CMakeLists.txt
===
--- /dev/null
+++ llvm/lib/Frontend/OpenMP/CMakeLists.txt
@@ -0,0 +1,10 @@
+add_llvm_component_library(LLVMFrontendOpenMP
+  OMPConstants.cpp
+
+  ADDITIONAL_HEADER_DIRS
+  ${LLVM_MAIN_INCLUDE_DIR}/llvm/Frontend
+  ${LLVM_MAIN_INCLUDE_DIR}/llvm/Frontend/OpenMP
+
+  DEPENDS
+  intrinsics_gen
+  )
Index: llvm/lib/LLVMBuild.txt
===
--- llvm/lib/LLVMBuild.txt
+++ llvm/lib/LLVMBuild.txt
@@ -24,6 +24,7 @@
  DebugInfo
  Demangle
  ExecutionEngine
+ Frontend
  FuzzMutate
  LineEditor
  Linker
Index: llvm/lib/Frontend/LLVMBuild.txt
===
--- llvm/lib/Frontend/LLVMBuild.txt
+++ llvm/lib/Frontend/LLVMBuild.txt
@@ -1,4 +1,4 @@
-;===- 

[PATCH] D49091: Warn about usage of __has_include/__has_include_next in macro expansions

[Tor Arne Vestbø via Phabricator via cfe-commits]

torarnv added a comment.

https://codereview.qt-project.org/c/qt/qtbase/+/284037 has merged and will be 
part of Qt 5.14.1

I went with treating them all under one strategy:

  #ifndef __has_foo
  #define __has_foo(x) 0
  #endif

This includes `__has_builtin`, `__has_feature`, `__has_attribute`, 
`__has_cpp_attribute`, `__has_include`, and `__has_include_next`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49091



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

[clang] eb3e81f - [OpenMP][NFCI] Introduce llvm/IR/OpenMPConstants.h

[Johannes Doerfert via cfe-commits]

Author: Johannes Doerfert
Date: 2019-12-10T00:10:09-06:00
New Revision: eb3e81f43f019cd90da87169aeff0eaddc4c9ecb

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

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

Summary:
The new OpenMPConstants.h is a location for all OpenMP related constants
(and helpers) to live.

This patch moves the directives there (the enum OpenMPDirectiveKind) and
rewires Clang to use the new location.

Initially part of D69785.

Reviewers: kiranchandramohan, ABataev, RaviNarayanaswamy, gtbercea, grokos, 
sdmitriev, JonChesterfield, hfinkel, fghanim

Subscribers: jholewinski, ppenzin, penzn, llvm-commits, cfe-commits, jfb, 
guansong, bollu, hiraditya, mgorny

Tags: #clang, #llvm

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

Added: 
llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
llvm/lib/Frontend/CMakeLists.txt
llvm/lib/Frontend/LLVMBuild.txt
llvm/lib/Frontend/OpenMP/CMakeLists.txt
llvm/lib/Frontend/OpenMP/LLVMBuild.txt
llvm/lib/Frontend/OpenMP/OMPConstants.cpp

Modified: 
clang/include/clang/AST/OpenMPClause.h
clang/include/clang/AST/StmtOpenMP.h
clang/include/clang/Basic/OpenMPKinds.def
clang/include/clang/Basic/OpenMPKinds.h
clang/lib/AST/CMakeLists.txt
clang/lib/AST/OpenMPClause.cpp
clang/lib/AST/StmtOpenMP.cpp
clang/lib/Basic/OpenMPKinds.cpp
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/Parse/CMakeLists.txt
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/CMakeLists.txt
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/lib/Sema/TreeTransform.h
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
llvm/lib/CMakeLists.txt
llvm/lib/LLVMBuild.txt

Removed: 




diff  --git a/clang/include/clang/AST/OpenMPClause.h 
b/clang/include/clang/AST/OpenMPClause.h
index b2a2035dcb3c..72c638bf6460 100644
--- a/clang/include/clang/AST/OpenMPClause.h
+++ b/clang/include/clang/AST/OpenMPClause.h
@@ -111,7 +111,7 @@ class OMPClauseWithPreInit {
   Stmt *PreInit = nullptr;
 
   /// Region that captures the associated stmt.
-  OpenMPDirectiveKind CaptureRegion = OMPD_unknown;
+  OpenMPDirectiveKind CaptureRegion = llvm::omp::OMPD_unknown;
 
 protected:
   OMPClauseWithPreInit(const OMPClause *This) {
@@ -119,7 +119,9 @@ class OMPClauseWithPreInit {
   }
 
   /// Set pre-initialization statement for the clause.
-  void setPreInitStmt(Stmt *S, OpenMPDirectiveKind ThisRegion = OMPD_unknown) {
+  void
+  setPreInitStmt(Stmt *S,
+ OpenMPDirectiveKind ThisRegion = llvm::omp::OMPD_unknown) {
 PreInit = S;
 CaptureRegion = ThisRegion;
   }
@@ -432,7 +434,7 @@ class OMPIfClause : public OMPClause, public 
OMPClauseWithPreInit {
   SourceLocation ColonLoc;
 
   /// Directive name modifier for the clause.
-  OpenMPDirectiveKind NameModifier = OMPD_unknown;
+  OpenMPDirectiveKind NameModifier = llvm::omp::OMPD_unknown;
 
   /// Name modifier location.
   SourceLocation NameModifierLoc;

diff  --git a/clang/include/clang/AST/StmtOpenMP.h 
b/clang/include/clang/AST/StmtOpenMP.h
index cc10f9e4c48e..65f0afece224 100644
--- a/clang/include/clang/AST/StmtOpenMP.h
+++ b/clang/include/clang/AST/StmtOpenMP.h
@@ -366,8 +366,9 @@ class OMPParallelDirective : public OMPExecutableDirective {
   ///
   OMPParallelDirective(SourceLocation StartLoc, SourceLocation EndLoc,
unsigned NumClauses)
-  : OMPExecutableDirective(this, OMPParallelDirectiveClass, OMPD_parallel,
-   StartLoc, EndLoc, NumClauses, 1),
+  : OMPExecutableDirective(this, OMPParallelDirectiveClass,
+   llvm::omp::OMPD_parallel, StartLoc, EndLoc,
+   NumClauses, 1),
 HasCancel(false) {}
 
   /// Build an empty directive.
@@ -375,9 +376,9 @@ class OMPParallelDirective : public OMPExecutableDirective {
   /// \param NumClauses Number of clauses.
   ///
   explicit OMPParallelDirective(unsigned NumClauses)
-  : OMPExecutableDirective(this, OMPParallelDirectiveClass, OMPD_parallel,
-   SourceLocation(), SourceLocation(), NumClauses,
-   1),
+  : OMPExecutableDirective(this, OMPParallelDirectiveClass,
+   llvm::omp::OMPD_parallel, SourceLocation(),
+   SourceLocation(), NumClauses, 1),
 HasCancel(false) {}
 
   /// Set cancel state.
@@ -1201,8 +1202,8 @@ class OMPSimdDirective : public OMPLoopDirective {
   ///
   

[PATCH] D71049: Remove implicit conversion that promotes half to other larger precision types for fp classification builtins

[Jim Lin via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcefac9dfaac9: Remove implicit conversion that promotes half 
to other larger precision types… (authored by Jim).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71049

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins.c


Index: clang/test/CodeGen/builtins.c
===
--- clang/test/CodeGen/builtins.c
+++ clang/test/CodeGen/builtins.c
@@ -193,8 +193,12 @@
 }
 
 // CHECK-LABEL: define void @test_float_builtins
-void test_float_builtins(float F, double D, long double LD) {
+void test_float_builtins(__fp16 *H, float F, double D, long double LD) {
   volatile int res;
+  res = __builtin_isinf(*H);
+  // CHECK:  call half @llvm.fabs.f16(half
+  // CHECK:  fcmp oeq half {{.*}}, 0xH7C00
+
   res = __builtin_isinf(F);
   // CHECK:  call float @llvm.fabs.f32(float
   // CHECK:  fcmp oeq float {{.*}}, 0x7FF0
@@ -207,6 +211,14 @@
   // CHECK:  call x86_fp80 @llvm.fabs.f80(x86_fp80
   // CHECK:  fcmp oeq x86_fp80 {{.*}}, 0xK7FFF8000
 
+  res = __builtin_isinf_sign(*H);
+  // CHECK:  %[[ABS:.*]] = call half @llvm.fabs.f16(half %[[ARG:.*]])
+  // CHECK:  %[[ISINF:.*]] = fcmp oeq half %[[ABS]], 0xH7C00
+  // CHECK:  %[[BITCAST:.*]] = bitcast half %[[ARG]] to i16
+  // CHECK:  %[[ISNEG:.*]] = icmp slt i16 %[[BITCAST]], 0
+  // CHECK:  %[[SIGN:.*]] = select i1 %[[ISNEG]], i32 -1, i32 1
+  // CHECK:  select i1 %[[ISINF]], i32 %[[SIGN]], i32 0
+
   res = __builtin_isinf_sign(F);
   // CHECK:  %[[ABS:.*]] = call float @llvm.fabs.f32(float %[[ARG:.*]])
   // CHECK:  %[[ISINF:.*]] = fcmp oeq float %[[ABS]], 0x7FF0
@@ -231,6 +243,10 @@
   // CHECK:  %[[SIGN:.*]] = select i1 %[[ISNEG]], i32 -1, i32 1
   // CHECK:  select i1 %[[ISINF]], i32 %[[SIGN]], i32 0
 
+  res = __builtin_isfinite(*H);
+  // CHECK: call half @llvm.fabs.f16(half
+  // CHECK: fcmp one half {{.*}}, 0xH7C00
+
   res = __builtin_isfinite(F);
   // CHECK: call float @llvm.fabs.f32(float
   // CHECK: fcmp one float {{.*}}, 0x7FF0
@@ -239,6 +255,14 @@
   // CHECK: call double @llvm.fabs.f64(double
   // CHECK: fcmp one double {{.*}}, 0x7FF0
 
+  res = __builtin_isnormal(*H);
+  // CHECK: fcmp oeq half
+  // CHECK: call half @llvm.fabs.f16(half
+  // CHECK: fcmp ult half {{.*}}, 0xH7C00
+  // CHECK: fcmp uge half {{.*}}, 0xH0400
+  // CHECK: and i1
+  // CHECK: and i1
+
   res = __builtin_isnormal(F);
   // CHECK: fcmp oeq float
   // CHECK: call float @llvm.fabs.f32(float
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -5808,10 +5808,11 @@
<< OrigArg->getType() << OrigArg->getSourceRange();
 
   // If this is an implicit conversion from float -> float, double, or
-  // long double, remove it.
+  // long double, or half -> half, float, double, or long double, remove it.
   if (ImplicitCastExpr *Cast = dyn_cast(OrigArg)) {
 // Only remove standard FloatCasts, leaving other casts inplace
 if (Cast->getCastKind() == CK_FloatingCast) {
+  bool IgnoreCast = false;
   Expr *CastArg = Cast->getSubExpr();
   if (CastArg->getType()->isSpecificBuiltinType(BuiltinType::Float)) {
 assert(
@@ -5820,6 +5821,19 @@
  Cast->getType()->isSpecificBuiltinType(BuiltinType::LongDouble)) 
&&
 "promotion from float to either float, double, or long double is "
 "the only expected cast here");
+IgnoreCast = true;
+  } else if (CastArg->getType()->isSpecificBuiltinType(BuiltinType::Half)) 
{
+assert(
+(Cast->getType()->isSpecificBuiltinType(BuiltinType::Double) ||
+ Cast->getType()->isSpecificBuiltinType(BuiltinType::Float) ||
+ Cast->getType()->isSpecificBuiltinType(BuiltinType::Half) ||
+ Cast->getType()->isSpecificBuiltinType(BuiltinType::LongDouble)) 
&&
+"promotion from half to either half, float, double, or long double 
"
+"is the only expected cast here");
+IgnoreCast = true;
+  }
+
+  if (IgnoreCast) {
 Cast->setSubExpr(nullptr);
 TheCall->setArg(NumArgs-1, CastArg);
   }


Index: clang/test/CodeGen/builtins.c
===
--- clang/test/CodeGen/builtins.c
+++ clang/test/CodeGen/builtins.c
@@ -193,8 +193,12 @@
 }
 
 // CHECK-LABEL: define void @test_float_builtins
-void test_float_builtins(float F, double D, long double LD) {
+void test_float_builtins(__fp16 *H, float F, double D, long double LD) {
   volatile int res;
+  res = __builtin_isinf(*H);
+  // CHECK:  call half @llvm.fabs.f16(half
+  // CHECK:  fcmp oeq half {{.*}}, 0xH7C00
+
   res = __builtin_isinf(F);
   // CHECK:  call float 

[clang] cefac9d - Remove implicit conversion that promotes half to other larger precision types for fp classification builtins

[Jim Lin via cfe-commits]

Author: Jim Lin
Date: 2019-12-10T13:24:21+08:00
New Revision: cefac9dfaac9c806433ad88cca85bd2f3ba1edad

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

LOG: Remove implicit conversion that promotes half to other larger precision 
types for fp classification builtins

Summary:
It shouldn't promote half to double or any larger precision types for fp 
classification builtins.
Because fp classification builtins would get incorrect result with promoted 
argument.
For example, __builtin_isnormal with a subnormal half value should return 
false, but it is not.
That the subnormal half value is promoted to a normal double value.

Reviewers: aaron.ballman

Reviewed By: aaron.ballman

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/Sema/SemaChecking.cpp
clang/test/CodeGen/builtins.c

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 825e0faa3037..2be9ecec 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5808,10 +5808,11 @@ bool Sema::SemaBuiltinFPClassification(CallExpr 
*TheCall, unsigned NumArgs) {
<< OrigArg->getType() << OrigArg->getSourceRange();
 
   // If this is an implicit conversion from float -> float, double, or
-  // long double, remove it.
+  // long double, or half -> half, float, double, or long double, remove it.
   if (ImplicitCastExpr *Cast = dyn_cast(OrigArg)) {
 // Only remove standard FloatCasts, leaving other casts inplace
 if (Cast->getCastKind() == CK_FloatingCast) {
+  bool IgnoreCast = false;
   Expr *CastArg = Cast->getSubExpr();
   if (CastArg->getType()->isSpecificBuiltinType(BuiltinType::Float)) {
 assert(
@@ -5820,6 +5821,19 @@ bool Sema::SemaBuiltinFPClassification(CallExpr 
*TheCall, unsigned NumArgs) {
  Cast->getType()->isSpecificBuiltinType(BuiltinType::LongDouble)) 
&&
 "promotion from float to either float, double, or long double is "
 "the only expected cast here");
+IgnoreCast = true;
+  } else if (CastArg->getType()->isSpecificBuiltinType(BuiltinType::Half)) 
{
+assert(
+(Cast->getType()->isSpecificBuiltinType(BuiltinType::Double) ||
+ Cast->getType()->isSpecificBuiltinType(BuiltinType::Float) ||
+ Cast->getType()->isSpecificBuiltinType(BuiltinType::Half) ||
+ Cast->getType()->isSpecificBuiltinType(BuiltinType::LongDouble)) 
&&
+"promotion from half to either half, float, double, or long double 
"
+"is the only expected cast here");
+IgnoreCast = true;
+  }
+
+  if (IgnoreCast) {
 Cast->setSubExpr(nullptr);
 TheCall->setArg(NumArgs-1, CastArg);
   }

diff  --git a/clang/test/CodeGen/builtins.c b/clang/test/CodeGen/builtins.c
index 5b482543d2d0..591416d00cc7 100644
--- a/clang/test/CodeGen/builtins.c
+++ b/clang/test/CodeGen/builtins.c
@@ -193,8 +193,12 @@ void test_conditional_bzero() {
 }
 
 // CHECK-LABEL: define void @test_float_builtins
-void test_float_builtins(float F, double D, long double LD) {
+void test_float_builtins(__fp16 *H, float F, double D, long double LD) {
   volatile int res;
+  res = __builtin_isinf(*H);
+  // CHECK:  call half @llvm.fabs.f16(half
+  // CHECK:  fcmp oeq half {{.*}}, 0xH7C00
+
   res = __builtin_isinf(F);
   // CHECK:  call float @llvm.fabs.f32(float
   // CHECK:  fcmp oeq float {{.*}}, 0x7FF0
@@ -207,6 +211,14 @@ void test_float_builtins(float F, double D, long double 
LD) {
   // CHECK:  call x86_fp80 @llvm.fabs.f80(x86_fp80
   // CHECK:  fcmp oeq x86_fp80 {{.*}}, 0xK7FFF8000
 
+  res = __builtin_isinf_sign(*H);
+  // CHECK:  %[[ABS:.*]] = call half @llvm.fabs.f16(half %[[ARG:.*]])
+  // CHECK:  %[[ISINF:.*]] = fcmp oeq half %[[ABS]], 0xH7C00
+  // CHECK:  %[[BITCAST:.*]] = bitcast half %[[ARG]] to i16
+  // CHECK:  %[[ISNEG:.*]] = icmp slt i16 %[[BITCAST]], 0
+  // CHECK:  %[[SIGN:.*]] = select i1 %[[ISNEG]], i32 -1, i32 1
+  // CHECK:  select i1 %[[ISINF]], i32 %[[SIGN]], i32 0
+
   res = __builtin_isinf_sign(F);
   // CHECK:  %[[ABS:.*]] = call float @llvm.fabs.f32(float %[[ARG:.*]])
   // CHECK:  %[[ISINF:.*]] = fcmp oeq float %[[ABS]], 0x7FF0
@@ -231,6 +243,10 @@ void test_float_builtins(float F, double D, long double 
LD) {
   // CHECK:  %[[SIGN:.*]] = select i1 %[[ISNEG]], i32 -1, i32 1
   // CHECK:  select i1 %[[ISINF]], i32 %[[SIGN]], i32 0
 
+  res = __builtin_isfinite(*H);
+  // CHECK: call half @llvm.fabs.f16(half
+  // CHECK: fcmp one half {{.*}}, 0xH7C00
+
   res = __builtin_isfinite(F);
   // CHECK: call float @llvm.fabs.f32(float
   // CHECK: fcmp one float {{.*}}, 0x7FF0

[PATCH] D71240: [clangd] Heuristically resolve dependent method calls

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 232998.
nridge added a comment.

Fix typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71240

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -465,6 +465,39 @@
 
 template 
 struct Fo^o {};
+  )cpp",
+
+  R"cpp(// Heuristic resolution of method
+template 
+struct S {
+  void [[bar]]() {}
+};
+
+template 
+void foo(S arg) {
+  arg.ba^r();
+}
+  )cpp",
+
+  R"cpp(// Heuristic resolution of method via this->
+template 
+struct S {
+  void [[foo]]() {
+this->fo^o();
+  }
+};
+  )cpp",
+
+  R"cpp(// Heuristic resolution of static method
+template 
+struct S {
+  static void [[bar]]() {}
+};
+
+template 
+void foo() {
+  S::ba^r();
+}
   )cpp"};
   for (const char *Test : Tests) {
 Annotations T(Test);
@@ -525,6 +558,21 @@
   Foo abcde$10^("asdf");
   Foo foox2 = Foo$11^("asdf");
 }
+
+template 
+struct S {
+  void $NonstaticOverload1[[bar]](int);
+  void $NonstaticOverload2[[bar]](float);
+
+  static void $StaticOverload1[[baz]](int);
+  static void $StaticOverload2[[baz]](float);
+};
+
+template 
+void dependent_call(S s, U u) {
+  s.ba$12^r(u);
+  S::ba$13^z(u);
+}
   )cpp");
   auto AST = TestTU::withCode(T.code()).build();
   // Ordered assertions are deliberate: we expect a predictable order.
@@ -544,6 +592,15 @@
   ElementsAre(Sym("Foo", T.range("ConstructorLoc";
   EXPECT_THAT(locateSymbolAt(AST, T.point("11")),
   ElementsAre(Sym("Foo", T.range("ConstructorLoc";
+  // These assertions are unordered because the order comes from
+  // CXXRecordDecl::lookupDependentName() which doesn't appear to provide
+  // an order guarantee.
+  EXPECT_THAT(locateSymbolAt(AST, T.point("12")),
+  UnorderedElementsAre(Sym("bar", T.range("NonstaticOverload1")),
+   Sym("bar", T.range("NonstaticOverload2";
+  EXPECT_THAT(locateSymbolAt(AST, T.point("13")),
+  UnorderedElementsAre(Sym("baz", T.range("StaticOverload1")),
+   Sym("baz", T.range("StaticOverload2";
 }
 
 TEST(LocateSymbol, TemplateTypedefs) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -384,8 +384,8 @@
   // different kinds, deduplicate them.
   std::vector Result;
   for (const auto  : References) {
-if (auto Range = getTokenRange(AST.getSourceManager(),
-   AST.getLangOpts(), Ref.Loc)) {
+if (auto Range =
+getTokenRange(AST.getSourceManager(), AST.getLangOpts(), Ref.Loc)) {
   DocumentHighlight DH;
   DH.range = *Range;
   if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Write))
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -79,8 +79,6 @@
 // formally size() is unresolved, but the primary template is a good guess.
 // This affects:
 //  - DependentTemplateSpecializationType,
-//  - DependentScopeMemberExpr
-//  - DependentScopeDeclRefExpr
 //  - DependentNameType
 struct TargetFinder {
   using RelSet = DeclRelationSet;
@@ -212,6 +210,25 @@
 break;
   }
   }
+  void
+  VisitCXXDependentScopeMemberExpr(const CXXDependentScopeMemberExpr *E) {
+const Type *BaseType = E->getBaseType().getTypePtrOrNull();
+if (E->isArrow()) {
+  if (!BaseType || !BaseType->isPointerType()) {
+return;
+  }
+  BaseType = BaseType->getAs()
+ ->getPointeeType()
+ .getTypePtrOrNull();
+}
+addMembersReferencedViaDependentName(BaseType, E->getMember(),
+ /*IsNonstaticMember=*/true);
+  }
+  void VisitDependentScopeDeclRefExpr(const DependentScopeDeclRefExpr *E) {
+addMembersReferencedViaDependentName(E->getQualifier()->getAsType(),
+ E->getDeclName(),
+ /*IsNonstaticMember=*/false);
+  }
   void VisitObjCIvarRefExpr(const ObjCIvarRefExpr *OIRE) {
 

[PATCH] D71240: [clangd] Heuristically resolve dependent method calls

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
nridge added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.
nridge updated this revision to Diff 232996.
nridge added a comment.
nridge edited the summary of this revision.

Add github issue number


The heuristic is to look in the definition of the primary template,
which is what you want in the vast majority of cases.

Fixes https://github.com/clangd/clangd/issues/141


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71240

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -465,6 +465,39 @@
 
 template 
 struct Fo^o {};
+  )cpp",
+
+  R"cpp(// Heuristic resolution of method
+template 
+struct S {
+  void [[bar]]() {}
+};
+
+template 
+void foo(S arg) {
+  arg.ba^r();
+}
+  )cpp",
+
+  R"cpp(// Heuristic resolution of method via this->
+template 
+struct S {
+  void [[foo]]() {
+this->fo^o();
+  }
+};
+  )cpp",
+
+  R"cpp(// Heuristic resolution of static method
+template 
+struct S {
+  static void [[bar]]() {}
+};
+
+template 
+void foo() {
+  S::ba^r();
+}
   )cpp"};
   for (const char *Test : Tests) {
 Annotations T(Test);
@@ -525,6 +558,21 @@
   Foo abcde$10^("asdf");
   Foo foox2 = Foo$11^("asdf");
 }
+
+template 
+struct S {
+  void $NonstaticOverload1[[bar]](int);
+  void $NonstaticOverload2[[bar]](float);
+
+  static void $StaticOverload1[[baz]](int);
+  static void $StaticOverload2[[baz]](int);
+};
+
+template 
+void dependent_call(S s, U u) {
+  s.ba$12^r(u);
+  S::ba$13^z(u);
+}
   )cpp");
   auto AST = TestTU::withCode(T.code()).build();
   // Ordered assertions are deliberate: we expect a predictable order.
@@ -544,6 +592,15 @@
   ElementsAre(Sym("Foo", T.range("ConstructorLoc";
   EXPECT_THAT(locateSymbolAt(AST, T.point("11")),
   ElementsAre(Sym("Foo", T.range("ConstructorLoc";
+  // These assertions are unordered because the order comes from
+  // CXXRecordDecl::lookupDependentName() which doesn't appear to provide
+  // an order guarantee.
+  EXPECT_THAT(locateSymbolAt(AST, T.point("12")),
+  UnorderedElementsAre(Sym("bar", T.range("NonstaticOverload1")),
+   Sym("bar", T.range("NonstaticOverload2";
+  EXPECT_THAT(locateSymbolAt(AST, T.point("13")),
+  UnorderedElementsAre(Sym("baz", T.range("StaticOverload1")),
+   Sym("baz", T.range("StaticOverload2";
 }
 
 TEST(LocateSymbol, TemplateTypedefs) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -384,8 +384,8 @@
   // different kinds, deduplicate them.
   std::vector Result;
   for (const auto  : References) {
-if (auto Range = getTokenRange(AST.getSourceManager(),
-   AST.getLangOpts(), Ref.Loc)) {
+if (auto Range =
+getTokenRange(AST.getSourceManager(), AST.getLangOpts(), Ref.Loc)) {
   DocumentHighlight DH;
   DH.range = *Range;
   if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Write))
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -79,8 +79,6 @@
 // formally size() is unresolved, but the primary template is a good guess.
 // This affects:
 //  - DependentTemplateSpecializationType,
-//  - DependentScopeMemberExpr
-//  - DependentScopeDeclRefExpr
 //  - DependentNameType
 struct TargetFinder {
   using RelSet = DeclRelationSet;
@@ -212,6 +210,25 @@
 break;
   }
   }
+  void
+  VisitCXXDependentScopeMemberExpr(const CXXDependentScopeMemberExpr *E) {
+const Type *BaseType = E->getBaseType().getTypePtrOrNull();
+if (E->isArrow()) {
+  if (!BaseType || !BaseType->isPointerType()) {
+return;
+  }
+  BaseType = BaseType->getAs()
+ ->getPointeeType()
+ .getTypePtrOrNull();
+}
+addMembersReferencedViaDependentName(BaseType, E->getMember(),
+ /*IsNonstaticMember=*/true);
+  }
+  void 

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

[Shiva Chen via Phabricator via cfe-commits]

shiva0217 added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:289
   unsigned CFIIndex = MF.addFrameInst(
   MCCFIInstruction::createDefCfaOffset(nullptr, -StackSize));
   BuildMI(MBB, MBBI, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))

Should the -StackSize be -RealStackSize?



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:667
+.addExternalSymbol(SpillLibCall, RISCVII::MO_CALL)
+.setMIFlag(MachineInstr::FrameSetup);
+

There is a case may trigger an assertion when compile with -O3 -g 
-msave-restore if the libcall has FrameSetup flag.
  int main(int a, char* argv[]) {
exit(0);
return 0;
  }



Comment at: llvm/lib/Target/RISCV/RISCVFrameLowering.cpp:668
+.setMIFlag(MachineInstr::FrameSetup);
+
+// Add registers spilled in libcall as liveins.

GCC will generate stack adjustment and GPR callee saved CFIs for the save 
libcalls. Should we do the same?



Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.h:40
 
+  bool hasReservedSpillSlot(const MachineFunction , unsigned Reg,
+int ) const override;

An alternative of defining hasReservedSpillSlot could be set SaveRegs for the 
registers will be pushed by the libcalls in determineCalleeSaves. So the 
StackSize calculation will take the callee saved registers will be pushed by 
the libcalls into account.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62686



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

[PATCH] D71240: [clangd] Heuristically resolve dependent method calls

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 232996.
nridge added a comment.

Add github issue number


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71240

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -465,6 +465,39 @@
 
 template 
 struct Fo^o {};
+  )cpp",
+
+  R"cpp(// Heuristic resolution of method
+template 
+struct S {
+  void [[bar]]() {}
+};
+
+template 
+void foo(S arg) {
+  arg.ba^r();
+}
+  )cpp",
+
+  R"cpp(// Heuristic resolution of method via this->
+template 
+struct S {
+  void [[foo]]() {
+this->fo^o();
+  }
+};
+  )cpp",
+
+  R"cpp(// Heuristic resolution of static method
+template 
+struct S {
+  static void [[bar]]() {}
+};
+
+template 
+void foo() {
+  S::ba^r();
+}
   )cpp"};
   for (const char *Test : Tests) {
 Annotations T(Test);
@@ -525,6 +558,21 @@
   Foo abcde$10^("asdf");
   Foo foox2 = Foo$11^("asdf");
 }
+
+template 
+struct S {
+  void $NonstaticOverload1[[bar]](int);
+  void $NonstaticOverload2[[bar]](float);
+
+  static void $StaticOverload1[[baz]](int);
+  static void $StaticOverload2[[baz]](int);
+};
+
+template 
+void dependent_call(S s, U u) {
+  s.ba$12^r(u);
+  S::ba$13^z(u);
+}
   )cpp");
   auto AST = TestTU::withCode(T.code()).build();
   // Ordered assertions are deliberate: we expect a predictable order.
@@ -544,6 +592,15 @@
   ElementsAre(Sym("Foo", T.range("ConstructorLoc";
   EXPECT_THAT(locateSymbolAt(AST, T.point("11")),
   ElementsAre(Sym("Foo", T.range("ConstructorLoc";
+  // These assertions are unordered because the order comes from
+  // CXXRecordDecl::lookupDependentName() which doesn't appear to provide
+  // an order guarantee.
+  EXPECT_THAT(locateSymbolAt(AST, T.point("12")),
+  UnorderedElementsAre(Sym("bar", T.range("NonstaticOverload1")),
+   Sym("bar", T.range("NonstaticOverload2";
+  EXPECT_THAT(locateSymbolAt(AST, T.point("13")),
+  UnorderedElementsAre(Sym("baz", T.range("StaticOverload1")),
+   Sym("baz", T.range("StaticOverload2";
 }
 
 TEST(LocateSymbol, TemplateTypedefs) {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -384,8 +384,8 @@
   // different kinds, deduplicate them.
   std::vector Result;
   for (const auto  : References) {
-if (auto Range = getTokenRange(AST.getSourceManager(),
-   AST.getLangOpts(), Ref.Loc)) {
+if (auto Range =
+getTokenRange(AST.getSourceManager(), AST.getLangOpts(), Ref.Loc)) {
   DocumentHighlight DH;
   DH.range = *Range;
   if (Ref.Role & index::SymbolRoleSet(index::SymbolRole::Write))
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -79,8 +79,6 @@
 // formally size() is unresolved, but the primary template is a good guess.
 // This affects:
 //  - DependentTemplateSpecializationType,
-//  - DependentScopeMemberExpr
-//  - DependentScopeDeclRefExpr
 //  - DependentNameType
 struct TargetFinder {
   using RelSet = DeclRelationSet;
@@ -212,6 +210,25 @@
 break;
   }
   }
+  void
+  VisitCXXDependentScopeMemberExpr(const CXXDependentScopeMemberExpr *E) {
+const Type *BaseType = E->getBaseType().getTypePtrOrNull();
+if (E->isArrow()) {
+  if (!BaseType || !BaseType->isPointerType()) {
+return;
+  }
+  BaseType = BaseType->getAs()
+ ->getPointeeType()
+ .getTypePtrOrNull();
+}
+addMembersReferencedViaDependentName(BaseType, E->getMember(),
+ /*IsNonstaticMember=*/true);
+  }
+  void VisitDependentScopeDeclRefExpr(const DependentScopeDeclRefExpr *E) {
+addMembersReferencedViaDependentName(E->getQualifier()->getAsType(),
+ E->getDeclName(),
+ /*IsNonstaticMember=*/false);
+  }
   void VisitObjCIvarRefExpr(const ObjCIvarRefExpr *OIRE) {
 

[clang] ae09dd8 - [Remarks][Driver] Error on -foptimization-record-file with multiple -arch options

[Francis Visoiu Mistrih via cfe-commits]

Author: Francis Visoiu Mistrih
Date: 2019-12-09T20:39:26-08:00
New Revision: ae09dd86a9b7f43543baa92d27c9382099691088

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

LOG: [Remarks][Driver] Error on -foptimization-record-file with multiple -arch 
options

This adds a check for the usage of -foptimization-record-file with
multiple -arch options. This is not permitted since it would require us
to rename the file requested by the user to avoid overwriting it for the
second cc1 invocation.

Added: 


Modified: 
clang/docs/ClangCommandLineReference.rst
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/darwin-opt-record.c

Removed: 




diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index e8d561fae956..7047cd9bb7b3 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -1701,7 +1701,7 @@ Emit OpenMP code only for SIMD-based constructs.
 
 .. option:: -foptimization-record-file=
 
-Specify the file name of any generated YAML optimization record
+Specify the output name of the file containing the optimization remarks. 
Implies -fsave-optimization-record. On Darwin platforms, this cannot be used 
with multiple -arch  options.
 
 .. option:: -foptimize-sibling-calls, -fno-optimize-sibling-calls
 

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 7fb1c846e0b2..b4621a0fcc81 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1415,6 +1415,132 @@ static bool isNoCommonDefault(const llvm::Triple 
) {
   }
 }
 
+static bool shouldEmitRemarks(const ArgList ) {
+  // -fsave-optimization-record enables it.
+  if (Args.hasFlag(options::OPT_fsave_optimization_record,
+   options::OPT_fno_save_optimization_record, false))
+return true;
+
+  // -fsave-optimization-record= enables it as well.
+  if (Args.hasFlag(options::OPT_fsave_optimization_record_EQ,
+   options::OPT_fno_save_optimization_record, false))
+return true;
+
+  // -foptimization-record-file alone enables it too.
+  if (Args.hasFlag(options::OPT_foptimization_record_file_EQ,
+   options::OPT_fno_save_optimization_record, false))
+return true;
+
+  // -foptimization-record-passes alone enables it too.
+  if (Args.hasFlag(options::OPT_foptimization_record_passes_EQ,
+   options::OPT_fno_save_optimization_record, false))
+return true;
+  return false;
+}
+
+static bool hasMultipleInvocations(const llvm::Triple ,
+   const ArgList ) {
+  // Supported only on Darwin where we invoke the compiler multiple times
+  // followed by an invocation to lipo.
+  if (!Triple.isOSDarwin())
+return false;
+  // If more than one "-arch " is specified, we're targeting multiple
+  // architectures resulting in a fat binary.
+  return Args.getAllArgValues(options::OPT_arch).size() > 1;
+}
+
+static bool checkRemarksOptions(const Driver , const ArgList ,
+const llvm::Triple ) {
+  // When enabling remarks, we need to error if:
+  // * The remark file is specified but we're targeting multiple architectures,
+  // which means more than one remark file is being generated.
+  bool hasMultipleInvocations = ::hasMultipleInvocations(Triple, Args);
+  bool hasExplicitOutputFile =
+  Args.getLastArg(options::OPT_foptimization_record_file_EQ);
+  if (hasMultipleInvocations && hasExplicitOutputFile) {
+D.Diag(diag::err_drv_invalid_output_with_multiple_archs)
+<< "-foptimization-record-file";
+return false;
+  }
+  return true;
+}
+
+static void renderRemarksOptions(const ArgList , ArgStringList ,
+ const llvm::Triple ,
+ const InputInfo , const JobAction ) {
+  CmdArgs.push_back("-opt-record-file");
+
+  const Arg *A = Args.getLastArg(options::OPT_foptimization_record_file_EQ);
+  if (A) {
+CmdArgs.push_back(A->getValue());
+  } else {
+bool hasMultipleArchs =
+Triple.isOSDarwin() && // Only supported on Darwin platforms.
+Args.getAllArgValues(options::OPT_arch).size() > 1;
+SmallString<128> F;
+
+if (Args.hasArg(options::OPT_c) || Args.hasArg(options::OPT_S)) {
+  if (Arg *FinalOutput = Args.getLastArg(options::OPT_o))
+F = FinalOutput->getValue();
+}
+
+if (F.empty()) {
+  // Use the input filename.
+  F = llvm::sys::path::stem(Input.getBaseInput());
+
+  // If we're compiling for an offload architecture (i.e. a CUDA device),
+  // we need to make the file name for the device compilation 
diff erent
+  // from the host compilation.
+  if 

[PATCH] D70805: [analyzer] SValHasDescendant: Determine whether the SVal has an SVal descendant

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D70805#1776527 , @NoQ wrote:

> Usually this kind of code is hard to re-use because all use cases require 
> different definitions of "has descendant". We already have at least 3 such 
> definitions floating around and we can't re-use them.
>
> Even in the world of ASTMatchers the actual `hasDescendant` matcher is 
> basically an anti-pattern as you always want to use a matcher for a 
> //specific// parent-child relation instead (cf. 
> http://lists.llvm.org/pipermail/cfe-dev/2019-September/063260.html).


When I encounter the following:

  unsigned Foo = Foo.Bar[Baz]->Qux;
  malloc(strlen(src) + Foo + 1);

At the moment I cannot lookup the `strlen(src)` and nor the `+ 1`. There is no 
way to say the `strlen(src)` which parent is a `malloc()`. I do not think we 
could make the full support of pattern-matching on symbolic level, so at least 
I wanted to see if it is possible. I am even thinking of using the ASTImporter 
to normalize the expression so that in a flatten/serialized form the 
AST-matcher could look for such constructs without any wonky `DeclRefExpr` or 
`ReturnStmt` matching. I believe the latter is the way we could benefit a lot, 
and the former, the current patch needs to be eliminated. For now I cannot 
relax the AST or invent a symbolic-matcher language so I would prefer this 
simple workaround and may we will have better ideas later.


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

https://reviews.llvm.org/D70805



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

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

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 60659 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69853



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

[PATCH] D71155: [analyzer] CERT: StrChecker: 30.c

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2110
+if (const auto *SR = MR->getBaseRegion()->getAs()) {
+  State = State->BindExpr(CE, LCtx, SrcV);
+  C.addTransition(State);

Charusso wrote:
> NoQ wrote:
> > Mmm, that's not a correct return value for these functions. These functions 
> > don't simply pass through their first argument.
> Yes, but we need some index here. It requires a `NonLoc`, so I just randomly 
> picked the first index, but I like the idea of an unknown index. Would we 
> like to introduce `UnknownVal` for indices?
Use the correct region but //conjure the index//.



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2122
+
+  SVal ConjuredV = SVB.getConjuredHeapSymbolVal(CE, LCtx, C.blockCount());
+  SVal ResultV = loc::MemRegionVal(SVB.getRegionManager().getElementRegion(

Charusso wrote:
> NoQ wrote:
> > Why "heap"?
> Well, a string which length is at least 16 characters long is going to be 
> allocated on the heap. I have to conjure the string here to create its 
> element.
o.o
```lang=c++
void foo() {
  // This string is 20 characters long
  // but it's clearly on the stack.
  char str[] = "12345678901234567890";
  // This one is therefore also on the stack.
  char *ptr = strchr(str, '0');
}
```


Repository:
  rC Clang

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

https://reviews.llvm.org/D71155



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

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

[Johannes Doerfert via Phabricator via cfe-commits]

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

Add missing files


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69853

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/StmtOpenMP.h
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/OpenMPKinds.h
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/CMakeLists.txt
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/CMakeLists.txt
  llvm/lib/Frontend/CMakeLists.txt
  llvm/lib/Frontend/LLVMBuild.txt
  llvm/lib/Frontend/OpenMP/CMakeLists.txt
  llvm/lib/Frontend/OpenMP/LLVMBuild.txt
  llvm/lib/Frontend/OpenMP/OMPConstants.cpp
  llvm/lib/LLVMBuild.txt
  llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn

Index: llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn
+++ llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn
@@ -70,7 +70,7 @@
 "LangStandards.cpp",
 "Module.cpp",
 "ObjCRuntime.cpp",
-"OpenMPKinds.cpp",
+"OMPKinds.cpp",
 "OperatorPrecedence.cpp",
 "SanitizerBlacklist.cpp",
 "SanitizerSpecialCaseList.cpp",
Index: llvm/lib/Frontend/OpenMP/OMPConstants.cpp
===
--- /dev/null
+++ llvm/lib/Frontend/OpenMP/OMPConstants.cpp
@@ -0,0 +1,34 @@
+//===- OMPConstants.cpp - Helpers related to OpenMP code generation ---===//
+//
+// 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 "llvm/Frontend/OpenMP/OMPConstants.h"
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
+
+using namespace llvm;
+using namespace omp;
+
+Directive llvm::omp::getOpenMPDirectiveKind(StringRef Str) {
+  return llvm::StringSwitch(Str)
+#define OMP_DIRECTIVE(Enum, Str) .Case(Str, Enum)
+#include "llvm/Frontend/OpenMP/OMPKinds.def"
+  .Default(OMPD_unknown);
+}
+
+StringRef llvm::omp::getOpenMPDirectiveName(Directive Kind) {
+  switch (Kind) {
+#define OMP_DIRECTIVE(Enum, Str)   \
+  case Enum:   \
+return Str;
+#include "llvm/Frontend/OpenMP/OMPKinds.def"
+  }
+  llvm_unreachable("Invalid OpenMP directive kind");
+}
Index: llvm/lib/Frontend/OpenMP/LLVMBuild.txt
===
--- llvm/lib/Frontend/OpenMP/LLVMBuild.txt
+++ llvm/lib/Frontend/OpenMP/LLVMBuild.txt
@@ -1,4 +1,4 @@
-;===- ./lib/LLVMBuild.txt --*- Conf -*--===;
+;===- ./lib/Frontend/OpenMP/LLVMBuild.txt --*- Conf -*--===;
 ;
 ; Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 ; See https://llvm.org/LICENSE.txt for license information.
@@ -14,42 +14,8 @@
 ;
 ;======;
 
-[common]
-subdirectories =
- Analysis
- AsmParser
- Bitcode
- Bitstream
- CodeGen
- DebugInfo
- Demangle
- ExecutionEngine
- FuzzMutate
- LineEditor
- Linker
- IR
- IRReader
- LTO
- MC
- MCA
- Object
- BinaryFormat
- ObjectYAML
- Option
- Remarks
- Passes
- ProfileData
- Support
- TableGen
- TextAPI
- Target
- Testing
- ToolDrivers
- Transforms
- WindowsManifest
- XRay
-
 [component_0]
-type = Group
-name = Libraries
-parent = $ROOT
+type = Library
+name = FrontendOpenMP
+parent = Frontend
+required_libraries = Core Support TransformUtils
Index: llvm/lib/Frontend/OpenMP/CMakeLists.txt
===
--- /dev/null
+++ llvm/lib/Frontend/OpenMP/CMakeLists.txt
@@ -0,0 +1,10 @@
+add_llvm_component_library(LLVMFrontendOpenMP
+  OMPConstants.cpp
+
+  ADDITIONAL_HEADER_DIRS
+  ${LLVM_MAIN_INCLUDE_DIR}/llvm/Frontend
+  ${LLVM_MAIN_INCLUDE_DIR}/llvm/Frontend/OpenMP
+
+  DEPENDS
+  intrinsics_gen
+  )
Index: llvm/lib/LLVMBuild.txt
===
--- llvm/lib/LLVMBuild.txt
+++ 

[PATCH] D70725: [analyzer] Add path notes to FuchsiaHandleCheck

[Artem Dergachev via Phabricator via cfe-commits]

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

Looks great, thanks!~


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

https://reviews.llvm.org/D70725



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

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D71179#1776491 , @ABataev wrote:

> In D71179#1776487 , @hfinkel wrote:
>
> > In D71179#1776467 , @ABataev wrote:
> >
> > > In D71179#1776457 , @jdoerfert 
> > > wrote:
> > >
> > > > > You're doing absolutely the same thing as the original declare 
> > > > > variant implementation.
> > > >
> > > > I don't think so but if you do why do you oppose this approach?
> > > >
> > > > > And I don't think it would be correct to add them as multiversiin 
> > > > > variants to the original function.
> > > >
> > > > Why wouldn't it be correct to pick the version through the overload 
> > > > resolution instead of the code generation?
> > > >  How this could work is already described in the TODO 
> > > > (CodeGenModule.cpp):
> > > >
> > > >   // TODO: We should introduce function aliases for `omp declare 
> > > > variant`
> > > >   //   directives such that we can treat them through the same 
> > > > overload
> > > >   //   resolution scheme (via multi versioning) as `omp begin 
> > > > declare
> > > >   //   variant` functions. For an `omp declare variant(VARIANT) ...`
> > > >   //   that is attached to a BASE function we would create a global 
> > > > alias
> > > >   //   VARIANT = BASE which will participate in the multi version 
> > > > overload
> > > >   //   resolution. If picked, here is no need to emit them 
> > > > explicitly.
> > > >   
> > > >
> > > >
> > > >
> > > >  ---
> > > >
> > > > I still haven't understood why we cannot/should not reuse the existing 
> > > > multi-version support and instead duplicate the logic in some custom 
> > > > scheme.
> > > >  We have this patch that shows how we can reuse the logic in Clang. It 
> > > > works on a per-call basis, so it will work for all context selector 
> > > > (incl. construct).
> > > >  If you think there is something conceptually not working, I'd like to 
> > > > hear about it. However, just saying "it wouldn't be correct" is not 
> > > > sufficient. You need to provide details about the situation, what you 
> > > > think would not work, and why.
> > >
> > >
> > > I explayned already: declare variant cannot be represented as mutiversion 
> > > functiin, for example.
> >
> >
> > @ABataev, can you please elaborate? It's not obvious to me that we cannot 
> > handle the existing declare variant with the same scheme (as @jdoerfert 
> > highlighted above). In general, I believe it's preferable to have one 
> > generic scheme and use it to handle all cases as opposed to continuing to 
> > use a more-limited scheme in addition to the generic scheme.
>
>
> Eaine already. Current version of declare variant cannot be represented as 
> multiversiin functions, because it is not. We have a function that is the 
> alias to other functions with different names. They just are not multiversion 
> functions by definition.


I understand that they have different names. I don't see why we that means that 
they can't be added to the overload set as multi-version candidates if we add 
logic which does exactly that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[PATCH] D71155: [analyzer] CERT: StrChecker: 30.c

[Csaba Dabis via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2109
+  if (const MemRegion *MR = SrcV.getAsRegion()) {
+if (const auto *SR = MR->getBaseRegion()->getAs()) {
+  State = State->BindExpr(CE, LCtx, SrcV);

NoQ wrote:
> Why does it matter whether the region is symbolic or not?
Hm, I think you are right, we could omit that if-statement. I wanted to specify 
to reuse conjured symbols.



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2110
+if (const auto *SR = MR->getBaseRegion()->getAs()) {
+  State = State->BindExpr(CE, LCtx, SrcV);
+  C.addTransition(State);

NoQ wrote:
> Mmm, that's not a correct return value for these functions. These functions 
> don't simply pass through their first argument.
Yes, but we need some index here. It requires a `NonLoc`, so I just randomly 
picked the first index, but I like the idea of an unknown index. Would we like 
to introduce `UnknownVal` for indices?



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2122
+
+  SVal ConjuredV = SVB.getConjuredHeapSymbolVal(CE, LCtx, C.blockCount());
+  SVal ResultV = loc::MemRegionVal(SVB.getRegionManager().getElementRegion(

NoQ wrote:
> Why "heap"?
Well, a string which length is at least 16 characters long is going to be 
allocated on the heap. I have to conjure the string here to create its element.


Repository:
  rC Clang

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

https://reviews.llvm.org/D71155



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

[PATCH] D70805: [analyzer] SValHasDescendant: Determine whether the SVal has an SVal descendant

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Usually this kind of code is hard to re-use because all use cases require 
different definitions of "has descendant". We already have at least 3 such 
definitions floating around and we can't re-use them.

Even in the world of ASTMatchers the actual `hasDescendant` matcher is 
basically an anti-pattern as you always want to use a matcher for a 
//specific// parent-child relation instead (cf. 
http://lists.llvm.org/pipermail/cfe-dev/2019-September/063260.html).


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

https://reviews.llvm.org/D70805



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

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

It still mildly worries me that the attributes aren't truly reusable, as the 
exact meaning of the attribute depends on the domain. In particular, every 
domain is expected to have different approaches to error handling, i.e. a 
function that creates or destroys a handle may fail to, respectively, create or 
destroy the handle, but instead indicate the failure in a domain-specific 
manner, eg. through magical return values or null handle or errno or whatever.

@aaron.ballman, do you think this is a problem? Should we rather go for an 
attribute name that's obviously domain-specific (eg., 
`__attribute__((fuchsia_acquire_handle))`), or is it ok to re-use attributes 
without re-using their exact meaning?


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

https://reviews.llvm.org/D70469



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

[PATCH] D71155: [analyzer] CERT: StrChecker: 30.c

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2109
+  if (const MemRegion *MR = SrcV.getAsRegion()) {
+if (const auto *SR = MR->getBaseRegion()->getAs()) {
+  State = State->BindExpr(CE, LCtx, SrcV);

Why does it matter whether the region is symbolic or not?



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2110
+if (const auto *SR = MR->getBaseRegion()->getAs()) {
+  State = State->BindExpr(CE, LCtx, SrcV);
+  C.addTransition(State);

Mmm, that's not a correct return value for these functions. These functions 
don't simply pass through their first argument.



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2122
+
+  SVal ConjuredV = SVB.getConjuredHeapSymbolVal(CE, LCtx, C.blockCount());
+  SVal ResultV = loc::MemRegionVal(SVB.getRegionManager().getElementRegion(

Why "heap"?


Repository:
  rC Clang

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

https://reviews.llvm.org/D71155



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

[clang-tools-extra] 52b1c94 - Turn off unused variable checking here since we're explicitly adding

[Eric Christopher via cfe-commits]

Author: Eric Christopher
Date: 2019-12-09T19:14:04-08:00
New Revision: 52b1c94a5fe47c8bb5e189bc40bfd50255ce5795

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

LOG: Turn off unused variable checking here since we're explicitly adding
a command line for clang-tidy.

Added: 


Modified: 
clang-tools-extra/test/clang-tidy/checkers/cert-mem57-cpp-cpp17.cpp

Removed: 




diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/cert-mem57-cpp-cpp17.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/cert-mem57-cpp-cpp17.cpp
index d393b229bda1..f26098682c2c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cert-mem57-cpp-cpp17.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cert-mem57-cpp-cpp17.cpp
@@ -1,6 +1,6 @@
 // RUN: %check_clang_tidy %s -std=c++14 cert-mem57-cpp %t
-// RUN: clang-tidy --extra-arg='-std=c++17' --extra-arg='-faligned-allocation' 
-checks='-*,cert-mem57-cpp' --warnings-as-errors='*' %s
-// RUN: clang-tidy --extra-arg='-std=c++2a' --extra-arg='-faligned-allocation' 
-checks='-*,cert-mem57-cpp' --warnings-as-errors='*' %s
+// RUN: clang-tidy --extra-arg='-std=c++17' --extra-arg='-faligned-allocation' 
-checks='-*,cert-mem57-cpp' --extra-arg=-Wno-unused-variable 
--warnings-as-errors='*' %s
+// RUN: clang-tidy --extra-arg='-std=c++2a' --extra-arg='-faligned-allocation' 
-checks='-*,cert-mem57-cpp' --extra-arg=-Wno-unused-variable 
--warnings-as-errors='*' %s
 
 struct alignas(128) Vector {
   char Elems[128];



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

[PATCH] D71152: [analyzer] Keep track of escaped locals.

[Artem Dergachev via Phabricator via cfe-commits]

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

Looks great, thanks! I'd love to see what effect it immediately causes and 
whether there will be any true or false negatives because of this change.

Also i-i-it's not that i insist or something, but somebody will have to add it 
to the exploded graph dumps :)




Comment at: clang/lib/StaticAnalyzer/Core/ProgramState.cpp:48-61
+namespace {
+struct EscapedLocals{};
+} // namespace
+
+template <>
+struct ProgramStateTrait :
+  public ProgramStatePartialTrait> {

xazax.hun wrote:
> xazax.hun wrote:
> > NoQ wrote:
> > > Wait, you are preventing direct access anyway by putting this stuff into 
> > > the .cpp file.
> > > 
> > > In this case i think you can safely use the `REGISTER_...` macros.
> > Yeah, initially I wanted to hide this trait but at this point I have no 
> > idea how to do that. If I get the layering right, ExprEngine is using 
> > ProgramState, and ProgramState should not reference ExprEngine. So if I 
> > want to use a trait in both it should be in ProgramState. The reason why I 
> > also need to touch ProgramState, because the list of invalidated regions 
> > are readily available there. Probably it is possible to somehow get that 
> > list in ExprEngine, but I think that would be way more complicated than the 
> > current solution.
> Well, I realized that ProgramState can access SubEngine, so as long as we 
> extend the abstract interface there we could move the trait to ExprEngine. 
> What do you prefer? 
I very slightly prefer to have it in `ExprEngine` because it's consistent with 
other existing traits: defined in the place in which they're maintained, don't 
show up in the overall definition of the `ProgramState`.


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

https://reviews.llvm.org/D71152



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

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:632
+  /// of some values.
+  ProgramStateRef escapeValue(ProgramStateRef State, ArrayRef Vs,
   PointerEscapeKind K) const;

Dunno, should we rename to `escapeValues()`?



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:614-616
+if (Arg >= FuncDecl->getNumParams())
+  break;
+QualType ParamTy = FuncDecl->getParamDecl(Arg)->getType();

I encourage `Call.parameters()`. This way you won't need to obtain a 
`FuncDecl`. In fact you won't even need it to be a `FunctionDecl`.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:624
+if (const MemRegion *MR = Call.getArgSVal(Arg).getAsRegion())
+  if (!MR->hasStackStorage())
+Escaped.push_back(State->getSVal(MR, Pointee));

Ok, so this patch interacts with D71152 in a non-trivial manner. We should 
re-use the logic that decides whether an escape on bind occurs.


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

https://reviews.llvm.org/D71224



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

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71179#1776487 , @hfinkel wrote:

> In D71179#1776467 , @ABataev wrote:
>
> > In D71179#1776457 , @jdoerfert 
> > wrote:
> >
> > > > You're doing absolutely the same thing as the original declare variant 
> > > > implementation.
> > >
> > > I don't think so but if you do why do you oppose this approach?
> > >
> > > > And I don't think it would be correct to add them as multiversiin 
> > > > variants to the original function.
> > >
> > > Why wouldn't it be correct to pick the version through the overload 
> > > resolution instead of the code generation?
> > >  How this could work is already described in the TODO (CodeGenModule.cpp):
> > >
> > >   // TODO: We should introduce function aliases for `omp declare variant`
> > >   //   directives such that we can treat them through the same 
> > > overload
> > >   //   resolution scheme (via multi versioning) as `omp begin declare
> > >   //   variant` functions. For an `omp declare variant(VARIANT) ...`
> > >   //   that is attached to a BASE function we would create a global 
> > > alias
> > >   //   VARIANT = BASE which will participate in the multi version 
> > > overload
> > >   //   resolution. If picked, here is no need to emit them explicitly.
> > >   
> > >
> > >
> > >
> > >  ---
> > >
> > > I still haven't understood why we cannot/should not reuse the existing 
> > > multi-version support and instead duplicate the logic in some custom 
> > > scheme.
> > >  We have this patch that shows how we can reuse the logic in Clang. It 
> > > works on a per-call basis, so it will work for all context selector 
> > > (incl. construct).
> > >  If you think there is something conceptually not working, I'd like to 
> > > hear about it. However, just saying "it wouldn't be correct" is not 
> > > sufficient. You need to provide details about the situation, what you 
> > > think would not work, and why.
> >
> >
> > I explayned already: declare variant cannot be represented as mutiversion 
> > functiin, for example.
>
>
> @ABataev, can you please elaborate? It's not obvious to me that we cannot 
> handle the existing declare variant with the same scheme (as @jdoerfert 
> highlighted above). In general, I believe it's preferable to have one generic 
> scheme and use it to handle all cases as opposed to continuing to use a 
> more-limited scheme in addition to the generic scheme.


Eaine already. Current version of declare variant cannot be represented as 
multiversiin functions, because it is not. We have a function that is the alias 
to other functions with different names. They just are not multiversion 
functions by definition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

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

[Johannes Doerfert via Phabricator via cfe-commits]

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

Rebase and creation of libFrontendOpenMP


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69853

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/StmtOpenMP.h
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/OpenMPKinds.h
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/CMakeLists.txt
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/CMakeLists.txt
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/CMakeLists.txt
  llvm/lib/Frontend/CMakeLists.txt
  llvm/lib/Frontend/LLVMBuild.txt
  llvm/lib/Frontend/OpenMP/OMPConstants.cpp
  llvm/lib/LLVMBuild.txt
  llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn

Index: llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn
+++ llvm/utils/gn/secondary/clang/lib/Basic/BUILD.gn
@@ -70,7 +70,7 @@
 "LangStandards.cpp",
 "Module.cpp",
 "ObjCRuntime.cpp",
-"OpenMPKinds.cpp",
+"OMPKinds.cpp",
 "OperatorPrecedence.cpp",
 "SanitizerBlacklist.cpp",
 "SanitizerSpecialCaseList.cpp",
Index: llvm/lib/Frontend/OpenMP/OMPConstants.cpp
===
--- /dev/null
+++ llvm/lib/Frontend/OpenMP/OMPConstants.cpp
@@ -0,0 +1,34 @@
+//===- OMPConstants.cpp - Helpers related to OpenMP code generation ---===//
+//
+// 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 "llvm/Frontend/OpenMP/OMPConstants.h"
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
+
+using namespace llvm;
+using namespace omp;
+
+Directive llvm::omp::getOpenMPDirectiveKind(StringRef Str) {
+  return llvm::StringSwitch(Str)
+#define OMP_DIRECTIVE(Enum, Str) .Case(Str, Enum)
+#include "llvm/Frontend/OpenMP/OMPKinds.def"
+  .Default(OMPD_unknown);
+}
+
+StringRef llvm::omp::getOpenMPDirectiveName(Directive Kind) {
+  switch (Kind) {
+#define OMP_DIRECTIVE(Enum, Str)   \
+  case Enum:   \
+return Str;
+#include "llvm/Frontend/OpenMP/OMPKinds.def"
+  }
+  llvm_unreachable("Invalid OpenMP directive kind");
+}
Index: llvm/lib/LLVMBuild.txt
===
--- llvm/lib/LLVMBuild.txt
+++ llvm/lib/LLVMBuild.txt
@@ -24,6 +24,7 @@
  DebugInfo
  Demangle
  ExecutionEngine
+ Frontend
  FuzzMutate
  LineEditor
  Linker
Index: llvm/lib/Frontend/LLVMBuild.txt
===
--- llvm/lib/Frontend/LLVMBuild.txt
+++ llvm/lib/Frontend/LLVMBuild.txt
@@ -1,4 +1,4 @@
-;===- ./lib/LLVMBuild.txt --*- Conf -*--===;
+;===- ./lib/Frontend/LLVMBuild.txt -*- Conf -*--===;
 ;
 ; Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 ; See https://llvm.org/LICENSE.txt for license information.
@@ -15,41 +15,9 @@
 ;======;
 
 [common]
-subdirectories =
- Analysis
- AsmParser
- Bitcode
- Bitstream
- CodeGen
- DebugInfo
- Demangle
- ExecutionEngine
- FuzzMutate
- LineEditor
- Linker
- IR
- IRReader
- LTO
- MC
- MCA
- Object
- BinaryFormat
- ObjectYAML
- Option
- Remarks
- Passes
- ProfileData
- Support
- TableGen
- TextAPI
- Target
- Testing
- ToolDrivers
- Transforms
- WindowsManifest
- XRay
+subdirectories = OpenMP
 
 [component_0]
 type = Group
-name = Libraries
-parent = $ROOT
+name = Frontend
+parent = Libraries
Index: llvm/lib/Frontend/CMakeLists.txt
===
--- /dev/null
+++ llvm/lib/Frontend/CMakeLists.txt
@@ -0,0 +1 @@
+add_subdirectory(OpenMP)
Index: llvm/lib/CMakeLists.txt
===
--- llvm/lib/CMakeLists.txt
+++ llvm/lib/CMakeLists.txt
@@ -8,6 +8,7 @@
 add_subdirectory(BinaryFormat)
 

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

In D71179#1776467 , @ABataev wrote:

> In D71179#1776457 , @jdoerfert wrote:
>
> > > You're doing absolutely the same thing as the original declare variant 
> > > implementation.
> >
> > I don't think so but if you do why do you oppose this approach?
> >
> > > And I don't think it would be correct to add them as multiversiin 
> > > variants to the original function.
> >
> > Why wouldn't it be correct to pick the version through the overload 
> > resolution instead of the code generation?
> >  How this could work is already described in the TODO (CodeGenModule.cpp):
> >
> >   // TODO: We should introduce function aliases for `omp declare variant`
> >   //   directives such that we can treat them through the same overload
> >   //   resolution scheme (via multi versioning) as `omp begin declare
> >   //   variant` functions. For an `omp declare variant(VARIANT) ...`
> >   //   that is attached to a BASE function we would create a global 
> > alias
> >   //   VARIANT = BASE which will participate in the multi version 
> > overload
> >   //   resolution. If picked, here is no need to emit them explicitly.
> >   
> >
> >
> >
> >  ---
> >
> > I still haven't understood why we cannot/should not reuse the existing 
> > multi-version support and instead duplicate the logic in some custom scheme.
> >  We have this patch that shows how we can reuse the logic in Clang. It 
> > works on a per-call basis, so it will work for all context selector (incl. 
> > construct).
> >  If you think there is something conceptually not working, I'd like to hear 
> > about it. However, just saying "it wouldn't be correct" is not sufficient. 
> > You need to provide details about the situation, what you think would not 
> > work, and why.
>
>
> I explayned already: declare variant cannot be represented as mutiversion 
> functiin, for example.


@ABataev, can you please elaborate? It's not obvious to me that we cannot 
handle the existing declare variant with the same scheme (as @jdoerfert 
highlighted above). In general, I believe it's preferable to have one generic 
scheme and use it to handle all cases as opposed to continuing to use a 
more-limited scheme in addition to the generic scheme.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

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

xazax.hun updated this revision to Diff 232982.
xazax.hun added a comment.

- Reuse `ExprEngine::escapeValue`.


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

https://reviews.llvm.org/D71224

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ponter-escape-on-conservative-calls.c

Index: clang/test/Analysis/ponter-escape-on-conservative-calls.c
===
--- /dev/null
+++ clang/test/Analysis/ponter-escape-on-conservative-calls.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=debug.AnalysisOrder -analyzer-config debug.AnalysisOrder:PointerEscape=true -analyzer-config debug.AnalysisOrder:PostCall=true %s 2>&1 | FileCheck %s
+
+
+void f(int *);
+
+int main() {
+int a;
+f();
+return 0;
+}
+
+// CHECK: PostCall
+// CHECK-NEXT: PointerEscape
Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -37,6 +37,7 @@
 // CHECK-NEXT: debug.AnalysisOrder:EndFunction = false
 // CHECK-NEXT: debug.AnalysisOrder:LiveSymbols = false
 // CHECK-NEXT: debug.AnalysisOrder:NewAllocator = false
+// CHECK-NEXT: debug.AnalysisOrder:PointerEscape = false
 // CHECK-NEXT: debug.AnalysisOrder:PostCall = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtArraySubscriptExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXNewExpr = false
@@ -97,4 +98,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 94
+// CHECK-NEXT: num-entries = 95
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -592,9 +592,47 @@
   for (auto I : dstCallEvaluated)
 finishArgumentConstruction(dstArgumentCleanup, I, Call);
 
-  // Finally, run any post-call checks.
-  getCheckerManager().runCheckersForPostCall(Dst, dstArgumentCleanup,
+  ExplodedNodeSet dstPostCall;
+  getCheckerManager().runCheckersForPostCall(dstPostCall, dstArgumentCleanup,
  Call, *this);
+
+  // Escaping symbols conjured during invalidating the regions above.
+  // Note that, for inlined calls the nodes were put back into the worklist,
+  // so we can assume that every node belongs to a conservative call at this
+  // point.
+
+  // Run pointerEscape callback with the newly conjured symbols.
+  SmallVector Escaped;
+  for (auto I : dstPostCall) {
+NodeBuilder B(I, Dst, *currBldrCtx);
+ProgramStateRef State = I->getState();
+Escaped.clear();
+const FunctionDecl *FuncDecl =
+dyn_cast_or_null(Call.getDecl());
+if (FuncDecl) {
+  for (unsigned Arg = 0; Arg < Call.getNumArgs(); ++Arg) {
+if (Arg >= FuncDecl->getNumParams())
+  break;
+QualType ParamTy = FuncDecl->getParamDecl(Arg)->getType();
+if (ParamTy.isNull() ||
+(!ParamTy->isPointerType() && !ParamTy->isReferenceType()))
+  continue;
+QualType Pointee = ParamTy->getPointeeType();
+if (Pointee.isConstQualified() || Pointee->isVoidType())
+  continue;
+if (const MemRegion *MR = Call.getArgSVal(Arg).getAsRegion())
+  if (!MR->hasStackStorage())
+Escaped.push_back(State->getSVal(MR, Pointee));
+  }
+}
+
+State = escapeValue(State, Escaped, PSK_EscapeOnConservativeCall);
+
+if (State == I->getState())
+  Dst.insert(I);
+else
+  B.generateNode(I->getLocation(), State, I);
+  }
 }
 
 ProgramStateRef ExprEngine::bindReturnValue(const CallEvent ,
@@ -670,8 +708,7 @@
 // Conservatively evaluate call by invalidating regions and binding
 // a conjured return value.
 void ExprEngine::conservativeEvalCall(const CallEvent , NodeBuilder ,
-  ExplodedNode *Pred,
-  ProgramStateRef State) {
+  ExplodedNode *Pred, ProgramStateRef State) {
   State = Call.invalidateRegions(currBldrCtx->blockCount(), State);
   State = bindReturnValue(Call, Pred->getLocationContext(), State);
 
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1172,13 +1172,15 @@
   }
 }
 
-ProgramStateRef 

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

> In D69778#1772125 , @dblaikie wrote:
> 
>> I was/am still wondering whether there's a way to coalesce these codepaths 
>> between PCH and the existing modular code generation support?
> 
> 
> In what way could this be united more? The way I understand it, the code 
> paths are pretty much the same, they just need to account for being invoked 
> in different contexts. This patch is tiny compared to what it was originally 
> before I made it reuse all the codegen code.

I guess one aspect is that -building-pch-with-obj seems like it duplicates the 
fmodules-codegen concept (both basically are a flag passed during pcm/pch build 
time that says "I promise to build an object file from this pcm/pch, so rely on 
that assumption when building other things that depend on the pcm/pch) - if I'd 
noticed the -building-pch-with-obj going in I would've made the point then.

But yeah, that's out of scope for this patch, but might be something that'd be 
good to look into to try to merge these features/codepaths more.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69778



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

[PATCH] D71239: [clang][Format] Fix ObjC keywords following try/catch getting split.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

Build result: pass - 60659 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71239



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

[PATCH] D70411: [analyzer] CERT: StrChecker: 31.c

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Ok, so, what should the checker warn on? What should be the intended state 
machine for this checker?

We basically have two possible categories of answers to this question:

- "The checker warns every time it finds an execution path on which unintended 
behavior occurs" - describe unintended behavior (say, "out-of-bounds buffer 
access") and define the checker in such a way that all warnings describe actual 
bugs of this kind (in our example, out-of-bound buffer accesses) as long as the 
rest of the static analyzer works correctly.
- Or, you could say "The checker enforces a certain coding guideline on the 
user" - and then you should describe the guideline in a very short and 
easy-to-understand manner (say, "don't pass the string by pointer anywhere 
before you null-terminate it"), and then make checker check exactly this 
guideline. You may introduce some suppressions for intentional violations of 
the guideline, but the guideline itself should be simple, it should be always 
possible to follow it, and it should sound nice enough for developers to feel 
good about themselves when they follow it.

> - The problem is not the size, but the missing `'\0'` (which you can have 
> multiple of at any point).

The caption of the CERT rule suggests that both of these are problematic.

In D70411#1773703 , @Charusso wrote:

> In D70411#1769786 , @NoQ wrote:
>
> > So i suggest that we make one more step back and agree upon what exactly 
> > are we checking here. I.e., basically, agree on the tests. Because right 
> > now it looks like you're trying to blindly generalize over the examples: 
> > //"The CERT example has a `strcpy()` into a fixed-size buffer, let's warn 
> > on all `strcpy()`s into fixed-size buffers!"//.
>
>
> Just for clarification, I made the entire `MemoryBlockRegion` stuff because I 
> do not care how do you allocate a memory block (even a `string` is counted as 
> that).


Yup, i mean, you can go as far as you want with generalizing over "fixed size 
buffer" in this approach, but what i'm saying is that you're still not 
addressing the whole train of thought about the length of the source string.

In D70411#1773703 , @Charusso wrote:

> > F10967277: ftp.c_8cb07866de61ef56be82135a4d3a5b7e.plist.html 
> > 
> >  The source string is an IP address and port, which has a known limit on 
> > the number of digits it can have.
>
> The known size does not mean that the string is going to be null-terminated.


It does, because `strcpy` is guaranteed to null-terminate as long as it has 
enough storage capacity.

>> For example, in the example titled "Noncompliant Code Example (argv)", it is 
>> explicitly stated that the problem is not only that the buffer is fixed-size 
>> and `strcpy()` is made, but also that the original string //is controlled by 
>> the attacker//. The right analysis to catch such issues is //taint 
>> analysis//. It's a typical taint problem. I honestly believe you should try 
>> to solve it by combining the taint checker with the string checker: "if 
>> string length metadata is tainted (in particular, if the string itself is 
>> tainted) and is potentially larger than the destination buffer size (eg., 
>> unconstrained), warn". With the recent advancements in the development of 
>> the taint checker, i think you can get pretty far this way without 
>> constantly struggling with false positives.
> 
> This checker indeed would require an additional taint analysis, but as we do 
> not have a non-alpha variant, I am fine to call every non-null-terminated 
> string reading an issue (as it is), whatever is the source. That is a 
> generalization, yes, in the best possible manner what we have got today. Also 
> I am not a big fan of relying on non-alpha stuff and I am sure I have defined 
> my checker enough well to catch real issues.

Checks that are part of the generic taint checker are currently in a pretty bad 
shape, but the taint analysis itself is pretty good, and checks that rely on 
taint but aren't part of the generic taint checker are also pretty good. I 
actually believe taint analysis could be enabled by default as soon as somebody 
goes through the broken checks and disables/removes them. If you rely on the 
existing taint analysis infrastructure and make a good check, that'll be 
wonderful and would further encourage us to move taint analysis out of alpha.

> - We heavily need to swap the value-tracking with `NoteTags` to make this 
> useful.

Before i forget, i'm opposed to explaining problems as notes. We should emit a 
warning as soon as we've reached far enough on the execution path to conclude 
that there's a problem.




Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:798
+
+def Str31cChecker : Checker<"31.c">,
+  HelpText<"SEI CERT checker of rules defined 

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71179#1776457 , @jdoerfert wrote:

> > You're doing absolutely the same thing as the original declare variant 
> > implementation.
>
> I don't think so but if you do why do you oppose this approach?
>
> > And I don't think it would be correct to add them as multiversiin variants 
> > to the original function.
>
> Why wouldn't it be correct to pick the version through the overload 
> resolution instead of the code generation?
>  How this could work is already described in the TODO (CodeGenModule.cpp):
>
>   // TODO: We should introduce function aliases for `omp declare variant`
>   //   directives such that we can treat them through the same overload
>   //   resolution scheme (via multi versioning) as `omp begin declare
>   //   variant` functions. For an `omp declare variant(VARIANT) ...`
>   //   that is attached to a BASE function we would create a global alias
>   //   VARIANT = BASE which will participate in the multi version overload
>   //   resolution. If picked, here is no need to emit them explicitly.
>   
>
>
>
>  ---
>
> I still haven't understood why we cannot/should not reuse the existing 
> multi-version support and instead duplicate the logic in some custom scheme.
>  We have this patch that shows how we can reuse the logic in Clang. It works 
> on a per-call basis, so it will work for all context selector (incl. 
> construct).
>  If you think there is something conceptually not working, I'd like to hear 
> about it. However, just saying "it wouldn't be correct" is not sufficient. 
> You need to provide details about the situation, what you think would not 
> work, and why.


I explayned already: declare variant cannot be represented as mutiversion 
functiin, for example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/test/CodeGen/debug-info-extern-basic.c:14-23
+extern int (*foo)(int);
+int test3() {
+  return foo(0);
+}
+
+int test4() {
+  extern int (*foo2)(int);

yonghong-song wrote:
> dblaikie wrote:
> > What do these tests add? What sort of bugs would be caught by these global 
> > function pointer tests that wouldn't be caught by the char tests above them?
> These two additional tests to test extern function pointers. One of bpf 
> program use cases specifically need extern function debuginfo type so I added 
> a bunch. I do agree that this may be too much unnecessary and variable tests 
> should cover this.
> 
> I will remove all these extern function pointer tests including below one, 
> except one like the above test3() which should be enough.
Just to explain my thinking a bit more clearly here:

Testing this feature shouldn't be related to how you want to use the feature, 
but how the feature's designed/what likely places it could have bugs.

If the feature creates types in the same way for every type, and in a way 
that's already well tested for other types - then I don't think it's useful to 
test more than one type here.

(imagine, say, testing code generation for function calls - function calls are 
independent of the expressions that generate their arguments - so it doesn't 
make sense to test "foo(3)" and "foo(1 + 2)", etc - and of course there are 
limits to this sort of "orthogonality" analysis, taken too far you don't get 
enough test coverage, etc - but generally that's what I'm thinking of when I'm 
thinking about test case reduction, is semi-white-box "could I introduce a bug 
that would make one of these tests fail and teh other pass" - if it's pretty 
clear that the two systems are independent, and each independently well tested, 
I'll only test one or two paths through both of the systems)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[PATCH] D71239: [clang][Format] Fix ObjC keywords following try/catch getting split.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese created this revision.
Bigcheese added reviewers: thakis, djasper.
Bigcheese added a project: clang-format.
Herald added a subscriber: dexonsmith.
Herald added a project: clang.

Previously things like:

  int main() {
@try {
} @catch (NSException *e) {
}
  
@try {
} @catch (NSException *e) {
}
  }

Would be formatted like:

  int main() {
@try {
} @catch (NSException *e) {
}
  
@
try {
} @catch (NSException *e) {
}
  }

because `UnwrappedLineParser::parseTryCatch()` would consume the `@` as
part of checking for another `catch` or `finally`. This patch fixes that
by doing a lookahead.

I'm not super happy about the way the lookhead works, but this is the only 
thing that worked.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71239

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTestObjC.cpp


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -191,7 +191,13 @@
"  @throw;\n"
"} @finally {\n"
"  exit(42);\n"
-   "}");
+   "}\n"
+   "@try {\n"
+   "} @catch (NSException *e) {\n"
+   "}\n"
+   "@ /* adena */ try {\n"
+   "} @catch (NSException *e) {\n"
+   "}\n");
   verifyFormat("DEBUG({\n"
"  @try {\n"
"  } @finally {\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1840,16 +1840,27 @@
 --Line->Level;
   }
   while (1) {
-if (FormatTok->is(tok::at))
-  nextToken();
+unsigned StoredPosition = Tokens->getPosition();
+if (FormatTok->is(tok::at)) {
+  // Get next non-comment token.
+  do {
+FormatTok = Tokens->getNextToken();
+  } while (FormatTok->is(tok::comment));
+}
 if (!(FormatTok->isOneOf(tok::kw_catch, Keywords.kw___except,
  tok::kw___finally) ||
   ((Style.Language == FormatStyle::LK_Java ||
 Style.Language == FormatStyle::LK_JavaScript) &&
FormatTok->is(Keywords.kw_finally)) ||
   (FormatTok->Tok.isObjCAtKeyword(tok::objc_catch) ||
-   FormatTok->Tok.isObjCAtKeyword(tok::objc_finally
+   FormatTok->Tok.isObjCAtKeyword(tok::objc_finally {
+  // Go back to before the (optional) @.
+  FormatTok = Tokens->setPosition(StoredPosition);
   break;
+}
+FormatTok = Tokens->setPosition(StoredPosition);
+if (FormatTok->is(tok::at))
+  nextToken();
 nextToken();
 while (FormatTok->isNot(tok::l_brace)) {
   if (FormatTok->is(tok::l_paren)) {


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -191,7 +191,13 @@
"  @throw;\n"
"} @finally {\n"
"  exit(42);\n"
-   "}");
+   "}\n"
+   "@try {\n"
+   "} @catch (NSException *e) {\n"
+   "}\n"
+   "@ /* adena */ try {\n"
+   "} @catch (NSException *e) {\n"
+   "}\n");
   verifyFormat("DEBUG({\n"
"  @try {\n"
"  } @finally {\n"
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1840,16 +1840,27 @@
 --Line->Level;
   }
   while (1) {
-if (FormatTok->is(tok::at))
-  nextToken();
+unsigned StoredPosition = Tokens->getPosition();
+if (FormatTok->is(tok::at)) {
+  // Get next non-comment token.
+  do {
+FormatTok = Tokens->getNextToken();
+  } while (FormatTok->is(tok::comment));
+}
 if (!(FormatTok->isOneOf(tok::kw_catch, Keywords.kw___except,
  tok::kw___finally) ||
   ((Style.Language == FormatStyle::LK_Java ||
 Style.Language == FormatStyle::LK_JavaScript) &&
FormatTok->is(Keywords.kw_finally)) ||
   (FormatTok->Tok.isObjCAtKeyword(tok::objc_catch) ||
-   FormatTok->Tok.isObjCAtKeyword(tok::objc_finally
+   FormatTok->Tok.isObjCAtKeyword(tok::objc_finally {
+  // Go back to before the (optional) @.
+  FormatTok = Tokens->setPosition(StoredPosition);
   break;
+}
+FormatTok = Tokens->setPosition(StoredPosition);
+if (FormatTok->is(tok::at))
+  nextToken();
 nextToken();
 while (FormatTok->isNot(tok::l_brace)) {
   if 

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

> You're doing absolutely the same thing as the original declare variant 
> implementation.

I don't think so but if you do why do you oppose this approach?

> And I don't think it would be correct to add them as multiversiin variants to 
> the original function.

Why wouldn't it be correct to pick the version through the overload resolution 
instead of the code generation?
How this could work is already described in the TODO (CodeGenModule.cpp):

  // TODO: We should introduce function aliases for `omp declare variant`
  //   directives such that we can treat them through the same overload
  //   resolution scheme (via multi versioning) as `omp begin declare
  //   variant` functions. For an `omp declare variant(VARIANT) ...`
  //   that is attached to a BASE function we would create a global alias
  //   VARIANT = BASE which will participate in the multi version overload
  //   resolution. If picked, here is no need to emit them explicitly.



---

I still haven't understood why we cannot/should not reuse the existing 
multi-version support and instead duplicate the logic in some custom scheme.
We have this patch that shows how we can reuse the logic in Clang. It works on 
a per-call basis, so it will work for all context selector (incl. construct).
If you think there is something conceptually not working, I'd like to hear 
about it. However, just saying "it wouldn't be correct" is not sufficient. You 
need to provide details about the situation, what you think would not work, and 
why.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Johannes Doerfert via Phabricator via cfe-commits]

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



Comment at: clang/lib/Headers/__clang_cuda_math_forward_declares.h:41
 __DEVICE__ long abs(long);
 __DEVICE__ long long abs(long long);
-__DEVICE__ double abs(double);

I have to double check what abs declarations where here and which were not. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[clang] 848934c - [c++20] Fix handling of unqualified lookups from a defaulted comparison

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-12-09T17:40:36-08:00
New Revision: 848934c67d484da737b4b92c087bffce069b24ba

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

LOG: [c++20] Fix handling of unqualified lookups from a defaulted comparison
function.

We need to perform unqualified lookups from the context of a defaulted
comparison, but not until we implicitly define the function, at which
point we can't do those lookups any more. So perform the lookup from the
end of the class containing the =default declaration and store the
lookup results on the defaulted function until we synthesize the body.

Added: 
clang/test/PCH/cxx2a-defaulted-comparison.cpp

Modified: 
clang/include/clang/AST/Decl.h
clang/include/clang/AST/DeclBase.h
clang/include/clang/AST/UnresolvedSet.h
clang/include/clang/Sema/Sema.h
clang/include/clang/Sema/Template.h
clang/lib/AST/Decl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaLambda.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/test/CXX/class/class.compare/class.compare.default/p1.cpp

Removed: 




diff  --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 2d1a30657cbd..90e8d19b17fa 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -15,6 +15,7 @@
 
 #include "clang/AST/APValue.h"
 #include "clang/AST/ASTContextAllocate.h"
+#include "clang/AST/DeclAccessPair.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/ExternalASTSource.h"
@@ -1812,13 +1813,37 @@ class FunctionDecl : public DeclaratorDecl,
 TK_DependentFunctionTemplateSpecialization
   };
 
+  /// Stashed information about a defaulted function definition whose body has
+  /// not yet been lazily generated.
+  class DefaultedFunctionInfo final
+  : llvm::TrailingObjects {
+friend TrailingObjects;
+unsigned NumLookups;
+
+  public:
+static DefaultedFunctionInfo *Create(ASTContext ,
+ ArrayRef Lookups);
+/// Get the unqualified lookup results that should be used in this
+/// defaulted function definition.
+ArrayRef getUnqualifiedLookups() const {
+  return {getTrailingObjects(), NumLookups};
+}
+  };
+
 private:
   /// A new[]'d array of pointers to VarDecls for the formal
   /// parameters of this function.  This is null if a prototype or if there are
   /// no formals.
   ParmVarDecl **ParamInfo = nullptr;
 
-  LazyDeclStmtPtr Body;
+  /// The active member of this union is determined by
+  /// FunctionDeclBits.HasDefaultedFunctionInfo.
+  union {
+/// The body of the function.
+LazyDeclStmtPtr Body;
+/// Information about a future defaulted function definition.
+DefaultedFunctionInfo *DefaultedInfo;
+  };
 
   unsigned ODRHash;
 
@@ -2050,17 +2075,25 @@ class FunctionDecl : public DeclaratorDecl,
   /// parser reaches the definition, if called before, this function will 
return
   /// `false`.
   bool isThisDeclarationADefinition() const {
-return isDeletedAsWritten() || isDefaulted() || Body || hasSkippedBody() ||
-   isLateTemplateParsed() || willHaveBody() || hasDefiningAttr();
+return isDeletedAsWritten() || isDefaulted() ||
+   doesThisDeclarationHaveABody() || hasSkippedBody() ||
+   willHaveBody() || hasDefiningAttr();
   }
 
   /// Returns whether this specific declaration of the function has a body.
   bool doesThisDeclarationHaveABody() const {
-return Body || isLateTemplateParsed();
+return (!FunctionDeclBits.HasDefaultedFunctionInfo && Body) ||
+   isLateTemplateParsed();
   }
 
   void setBody(Stmt *B);
-  void setLazyBody(uint64_t Offset) { Body = Offset; }
+  void setLazyBody(uint64_t Offset) {
+FunctionDeclBits.HasDefaultedFunctionInfo = false;
+Body = LazyDeclStmtPtr(Offset);
+  }
+
+  void setDefaultedFunctionInfo(DefaultedFunctionInfo *Info);
+  DefaultedFunctionInfo *getDefaultedFunctionInfo() const;
 
   /// Whether this function is variadic.
   bool isVariadic() const;

diff  --git a/clang/include/clang/AST/DeclBase.h 
b/clang/include/clang/AST/DeclBase.h
index 54cdb84b6f33..91c372585b07 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -1501,10 +1501,9 @@ class DeclContext {
 /// constructor or a destructor.
 uint64_t IsTrivialForCall : 1;
 
-/// Used by CXXMethodDecl
 uint64_t IsDefaulted : 1;
-/// Used by CXXMethodDecl
 uint64_t IsExplicitlyDefaulted : 1;
+uint64_t HasDefaultedFunctionInfo : 1;
 uint64_t HasImplicitReturnZero : 1;
 uint64_t IsLateTemplateParsed 

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

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

xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:603
+  // point.
+  class CollectReachableSymbolsCallback final : public SymbolVisitor {
+ProgramStateRef State;

NoQ wrote:
> xazax.hun wrote:
> > NoQ wrote:
> > > xazax.hun wrote:
> > > > NoQ wrote:
> > > > > WDYT about re-using `ExprEngine::escapeValue()` by changing it to 
> > > > > accept `ArrayRef` instead of a single `SVal`?
> > > > It is not entirely the same. Here we only collect symbols from 
> > > > non-stack regions. There (as far as I understand) we collect all 
> > > > symbols. I could add a flag but at that point I wonder if it is worth 
> > > > the change.
> > > Umm, wait, right, so what are you doing in this callback to begin with? 
> > > The code says "gather all symbols that are encountered as immediate 
> > > values stored in traversed regions". Why not simply "gather all symbols 
> > > that are traversed from parameter regions"?
> > My understanding is the following but correct me if I am wrong:
> > 
> > ```
> > int *getConjuredSymbol();
> > 
> > call(getConjuredSymbol());
> > ```
> > 
> > So we have can talk about two symbols here. One symbol is what was returned 
> > by `getConjuredSymbol` and the other is the pointee, the symbol that we get 
> > when we dereference the result of `getConjuredSymbol`. And my understanding 
> > is that we want to invoke escape for the latter and not the former. 
> > `ExprEngine::escapeValue` invalidates the former but not the latter. The 
> > visitor here invalidates the latter but not the former.  This behavior 
> > solves most of my test cases in FuchsiaHandleChecker. 
> How about `escapeValue(getSVal(getArgSVal()))`? (the type for `getSVal()` 
> could be obtained from the AST).
Hmm, I believe that could work, thanks!


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

https://reviews.llvm.org/D71224



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song updated this revision to Diff 232980.
yonghong-song added a comment.

remove some redundant test cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696

Files:
  clang/include/clang/AST/ASTConsumer.h
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/Targets/BPF.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ModuleBuilder.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/debug-info-extern-basic.c
  clang/test/CodeGen/debug-info-extern-duplicate.c
  clang/test/CodeGen/debug-info-extern-multi.c
  clang/test/CodeGen/debug-info-extern-unused.c
  llvm/include/llvm/IR/DIBuilder.h
  llvm/lib/IR/DIBuilder.cpp
  llvm/lib/IR/DebugInfo.cpp
  llvm/unittests/Transforms/Utils/CloningTest.cpp

Index: llvm/unittests/Transforms/Utils/CloningTest.cpp
===
--- llvm/unittests/Transforms/Utils/CloningTest.cpp
+++ llvm/unittests/Transforms/Utils/CloningTest.cpp
@@ -764,7 +764,7 @@
 
 DBuilder.createGlobalVariableExpression(
 Subprogram, "unattached", "unattached", File, 1,
-DBuilder.createNullPtrType(), false, Expr);
+DBuilder.createNullPtrType(), false, true, Expr);
 
 auto *Entry = BasicBlock::Create(C, "", F);
 IBuilder.SetInsertPoint(Entry);
Index: llvm/lib/IR/DebugInfo.cpp
===
--- llvm/lib/IR/DebugInfo.cpp
+++ llvm/lib/IR/DebugInfo.cpp
@@ -1289,7 +1289,7 @@
   return wrap(unwrap(Builder)->createGlobalVariableExpression(
   unwrapDI(Scope), {Name, NameLen}, {Linkage, LinkLen},
   unwrapDI(File), LineNo, unwrapDI(Ty), LocalToUnit,
-  unwrap(Expr), unwrapDI(Decl),
+  true, unwrap(Expr), unwrapDI(Decl),
   nullptr, AlignInBits));
 }
 
Index: llvm/lib/IR/DIBuilder.cpp
===
--- llvm/lib/IR/DIBuilder.cpp
+++ llvm/lib/IR/DIBuilder.cpp
@@ -640,13 +640,14 @@
 
 DIGlobalVariableExpression *DIBuilder::createGlobalVariableExpression(
 DIScope *Context, StringRef Name, StringRef LinkageName, DIFile *F,
-unsigned LineNumber, DIType *Ty, bool isLocalToUnit, DIExpression *Expr,
+unsigned LineNumber, DIType *Ty, bool isLocalToUnit,
+bool isDefined, DIExpression *Expr,
 MDNode *Decl, MDTuple *templateParams, uint32_t AlignInBits) {
   checkGlobalVariableScope(Context);
 
   auto *GV = DIGlobalVariable::getDistinct(
   VMContext, cast_or_null(Context), Name, LinkageName, F,
-  LineNumber, Ty, isLocalToUnit, true, cast_or_null(Decl),
+  LineNumber, Ty, isLocalToUnit, isDefined, cast_or_null(Decl),
   templateParams, AlignInBits);
   if (!Expr)
 Expr = createExpression();
Index: llvm/include/llvm/IR/DIBuilder.h
===
--- llvm/include/llvm/IR/DIBuilder.h
+++ llvm/include/llvm/IR/DIBuilder.h
@@ -583,7 +583,7 @@
 ///specified)
 DIGlobalVariableExpression *createGlobalVariableExpression(
 DIScope *Context, StringRef Name, StringRef LinkageName, DIFile *File,
-unsigned LineNo, DIType *Ty, bool isLocalToUnit,
+unsigned LineNo, DIType *Ty, bool isLocalToUnit, bool isDefined = true,
 DIExpression *Expr = nullptr, MDNode *Decl = nullptr,
 MDTuple *templateParams = nullptr, uint32_t AlignInBits = 0);
 
Index: clang/test/CodeGen/debug-info-extern-unused.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-unused.c
@@ -0,0 +1,27 @@
+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S -g %s -o - | FileCheck %s
+
+extern char ch;
+int test() {
+  return 0;
+}
+
+int test2() {
+  extern char ch2;
+  return 0;
+}
+
+extern int (*foo)(int);
+int test3() {
+  return 0;
+}
+
+int test4() {
+  extern int (*foo2)(int);
+  return 0;
+}
+
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "ch2"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "foo"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "foo2"
Index: clang/test/CodeGen/debug-info-extern-multi.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-multi.c
@@ -0,0 +1,23 @@
+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S -g %s -o - | FileCheck %s
+
+extern char ch;
+int test() {
+  extern short sh;
+  return ch + sh;
+}
+
+extern char (*foo)(char);
+int test2() {
+  return foo(0) + ch;
+}
+
+// CHECK: distinct !DIGlobalVariable(name: "ch",{{.*}} type: ![[Tch:[0-9]+]], isLocal: false, isDefinition: false
+// CHECK: distinct 

[PATCH] D71041: [analyzer][discussion] Talk about escapes

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

xazax.hun added a comment.

This is the next one: https://reviews.llvm.org/D71224


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71041



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

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:603
+  // point.
+  class CollectReachableSymbolsCallback final : public SymbolVisitor {
+ProgramStateRef State;

xazax.hun wrote:
> NoQ wrote:
> > xazax.hun wrote:
> > > NoQ wrote:
> > > > WDYT about re-using `ExprEngine::escapeValue()` by changing it to 
> > > > accept `ArrayRef` instead of a single `SVal`?
> > > It is not entirely the same. Here we only collect symbols from non-stack 
> > > regions. There (as far as I understand) we collect all symbols. I could 
> > > add a flag but at that point I wonder if it is worth the change.
> > Umm, wait, right, so what are you doing in this callback to begin with? The 
> > code says "gather all symbols that are encountered as immediate values 
> > stored in traversed regions". Why not simply "gather all symbols that are 
> > traversed from parameter regions"?
> My understanding is the following but correct me if I am wrong:
> 
> ```
> int *getConjuredSymbol();
> 
> call(getConjuredSymbol());
> ```
> 
> So we have can talk about two symbols here. One symbol is what was returned 
> by `getConjuredSymbol` and the other is the pointee, the symbol that we get 
> when we dereference the result of `getConjuredSymbol`. And my understanding 
> is that we want to invoke escape for the latter and not the former. 
> `ExprEngine::escapeValue` invalidates the former but not the latter. The 
> visitor here invalidates the latter but not the former.  This behavior solves 
> most of my test cases in FuchsiaHandleChecker. 
How about `escapeValue(getSVal(getArgSVal()))`? (the type for `getSVal()` could 
be obtained from the AST).


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

https://reviews.llvm.org/D71224



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

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

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

xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:648
+  continue;
+State->scanReachableSymbols(Call.getArgSVal(Arg), Scanner);
+  }

NoQ wrote:
> I guess technically, for our own sanity, it's worth it to-rescan the symbols 
> for every node in `dstPostCall`. But i'll be very surprised if they are 
> //actually// going to yield different results for every predecessor node.
I think it is possible. We use the state to get the pointee of some pointers, 
so in case the PostCall splits the state on the value of the outputs it would 
be reasonable to get different results. 


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

https://reviews.llvm.org/D71224



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

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

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

xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:603
+  // point.
+  class CollectReachableSymbolsCallback final : public SymbolVisitor {
+ProgramStateRef State;

NoQ wrote:
> xazax.hun wrote:
> > NoQ wrote:
> > > WDYT about re-using `ExprEngine::escapeValue()` by changing it to accept 
> > > `ArrayRef` instead of a single `SVal`?
> > It is not entirely the same. Here we only collect symbols from non-stack 
> > regions. There (as far as I understand) we collect all symbols. I could add 
> > a flag but at that point I wonder if it is worth the change.
> Umm, wait, right, so what are you doing in this callback to begin with? The 
> code says "gather all symbols that are encountered as immediate values stored 
> in traversed regions". Why not simply "gather all symbols that are traversed 
> from parameter regions"?
My understanding is the following but correct me if I am wrong:

```
int *getConjuredSymbol();

call(getConjuredSymbol());
```

So we have can talk about two symbols here. One symbol is what was returned by 
`getConjuredSymbol` and the other is the pointee, the symbol that we get when 
we dereference the result of `getConjuredSymbol`. And my understanding is that 
we want to invoke escape for the latter and not the former. 
`ExprEngine::escapeValue` invalidates the former but not the latter. The 
visitor here invalidates the latter but not the former.  This behavior solves 
most of my test cases in FuchsiaHandleChecker. 


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

https://reviews.llvm.org/D71224



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

[PATCH] D70520: [WebAssembly] Add new `export_name` clang attribute for controlling wasm export names

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: lld/test/wasm/export-name.ll:20
+; CHECK-NEXT:Exports:
+; CHECK-NEXT:  - Name:memory
+; CHECK-NEXT:Kind:MEMORY

sbc100 wrote:
> dschuff wrote:
> > does this test need to verify that the memory and _start are exported? 
> > seems like just a check for bar would be enough.
> The worry that if I just look for ` Name:bar` it might appear in 
> some other section too mabye?
You could do `CHECK-LABEL: Exports` and then `CHECK: - Name: bar` and then 
`CHECK-LABEL` on the next section?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70520



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

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

[Kan Shengchen via Phabricator via cfe-commits]

skan added a comment.

In D70157#1775481 , @fedor.sergeev 
wrote:

> In D70157#1775016 , @annita.zhang 
> wrote:
>
> > > The point is that we have explicit requirement at the start and we have a 
> > > lowering into 16-byte sequence that we need to be preserved exactly as it 
> > > is.
> > >  Essentially what we need is  a "protection" for this sequence from any 
> > > changes by machinery that generates the binary code.
> > >  How can we protect a particular byte sequence from being changed by this 
> > > branch aligner?
> >
> > No, in general we can't. The current solution is based on assembler to 
> > insert prefix or nop before the cross (or against) boundary branches. It 
> > can only ensure the explicit alignment specified by directive, but not any 
> > implicit alignment. I don't think any fixup based on assembler can do it. 
> > On the other hand, any code sequence after the alignment directive or even 
> > just in a function has some kind of implicit alignment. It's hard for 
> > assembler to tell which implicit alignment to preserve. The preferred way 
> > is to use explicit alignment directive to specify it.
> >
> > For your scenario, a NOP padding is more controllable. NOP padding will be 
> > inserted just before the branch instructions (or macro fusion branch 
> > instructions). So if there's no branches (or macro fusion branches) in your 
> > code sequence, there will be no NOP inserted.
>
>
> What if I insert explicit align(8) right *after* the sequence?


If your insert explicit `.align 8` after the sequence, and the sequence doesn't 
has any branch to be aligned, the current solution won't change the sequence.


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

https://reviews.llvm.org/D70157



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[Yonghong Song via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGen/debug-info-extern-basic.c:14-23
+extern int (*foo)(int);
+int test3() {
+  return foo(0);
+}
+
+int test4() {
+  extern int (*foo2)(int);

dblaikie wrote:
> What do these tests add? What sort of bugs would be caught by these global 
> function pointer tests that wouldn't be caught by the char tests above them?
These two additional tests to test extern function pointers. One of bpf program 
use cases specifically need extern function debuginfo type so I added a bunch. 
I do agree that this may be too much unnecessary and variable tests should 
cover this.

I will remove all these extern function pointer tests including below one, 
except one like the above test3() which should be enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

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

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

I just posted an alternate review (https://reviews.llvm.org/D71238) which 
attempts to carve out a minimum reviewable piece of complexity.  The hope is 
that we can review that one quickly (as there are fewer interacting concerns), 
and then rebase this one (possibly splitting further).

I had previously suggested in review comments that we should reuse the 
infrastructure from .bundle_align_mode.  When I sat down to actually implement 
that, I discovered that the code for that has a bunch of interacting 
assumptions about when fragments are constructed and used vs alignment 
boundaries.  I got a version of this working, but the complexity was worrisome. 
 I now suggest that we should take the rough approach sketched here (a separate 
fragment before the one being aligned), delete the essentially unused bundle 
mode code, and revisit a unified representation if needed for memory density at 
a later time.  (i.e. my previous suggestion wasn't a good one)


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

https://reviews.llvm.org/D70157



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

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:603
+  // point.
+  class CollectReachableSymbolsCallback final : public SymbolVisitor {
+ProgramStateRef State;

xazax.hun wrote:
> NoQ wrote:
> > WDYT about re-using `ExprEngine::escapeValue()` by changing it to accept 
> > `ArrayRef` instead of a single `SVal`?
> It is not entirely the same. Here we only collect symbols from non-stack 
> regions. There (as far as I understand) we collect all symbols. I could add a 
> flag but at that point I wonder if it is worth the change.
Umm, wait, right, so what are you doing in this callback to begin with? The 
code says "gather all symbols that are encountered as immediate values stored 
in traversed regions". Why not simply "gather all symbols that are traversed 
from parameter regions"?



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:648
+  continue;
+State->scanReachableSymbols(Call.getArgSVal(Arg), Scanner);
+  }

I guess technically, for our own sanity, it's worth it to-rescan the symbols 
for every node in `dstPostCall`. But i'll be very surprised if they are 
//actually// going to yield different results for every predecessor node.


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

https://reviews.llvm.org/D71224



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

[PATCH] D65543: [Windows] Autolink with basenames and add libdir to libpath

[Reid Kleckner via Phabricator via cfe-commits]

rnk updated this revision to Diff 232975.
rnk added a comment.

- rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65543

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/test/Driver/cl-options.c
  clang/test/Driver/sanitizer-ld.c
  llvm/cmake/modules/HandleLLVMOptions.cmake

Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -854,6 +854,27 @@
   endif()
 endif()
 
+# When using clang-cl with an instrumentation-based tool, add clang's library
+# resource directory to the library search path. Because cmake invokes the
+# linker directly, it isn't sufficient to pass -fsanitize=* to the linker.
+if (CLANG_CL AND (LLVM_BUILD_INSTRUMENTED OR LLVM_USE_SANITIZER))
+  execute_process(
+COMMAND ${CMAKE_CXX_COMPILER} /clang:-print-resource-dir
+OUTPUT_VARIABLE clang_resource_dir
+ERROR_VARIABLE clang_cl_stderr
+OUTPUT_STRIP_TRAILING_WHITESPACE
+ERROR_STRIP_TRAILING_WHITESPACE
+RESULT_VARIABLE clang_cl_exit_code)
+  if (NOT "${clang_cl_exit_code}" STREQUAL "0")
+message(FATAL_ERROR
+  "Unable to invoke clang-cl to find resource dir: ${clang_cl_stderr}")
+  endif()
+  file(TO_CMAKE_PATH "${clang_resource_dir}" clang_resource_dir)
+  append("/libpath:${clang_resource_dir}/lib/windows"
+CMAKE_EXE_LINKER_FLAGS
+CMAKE_SHARED_LINKER_FLAGS)
+endif()
+
 if(LLVM_PROFDATA_FILE AND EXISTS ${LLVM_PROFDATA_FILE})
   if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang" )
 append("-fprofile-instr-use=\"${LLVM_PROFDATA_FILE}\""
Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -656,16 +656,16 @@
 // RUN: -target x86_64-pc-windows \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CFI-STATS-WIN64 %s
-// CHECK-CFI-STATS-WIN64: "--dependent-lib={{[^"]*}}clang_rt.stats_client-x86_64.lib"
-// CHECK-CFI-STATS-WIN64: "--dependent-lib={{[^"]*}}clang_rt.stats-x86_64.lib"
+// CHECK-CFI-STATS-WIN64: "--dependent-lib=clang_rt.stats_client-x86_64.lib"
+// CHECK-CFI-STATS-WIN64: "--dependent-lib=clang_rt.stats-x86_64.lib"
 // CHECK-CFI-STATS-WIN64: "--linker-option=/include:__sanitizer_stats_register"
 
 // RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \
 // RUN: -target i686-pc-windows \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CFI-STATS-WIN32 %s
-// CHECK-CFI-STATS-WIN32: "--dependent-lib={{[^"]*}}clang_rt.stats_client-i386.lib"
-// CHECK-CFI-STATS-WIN32: "--dependent-lib={{[^"]*}}clang_rt.stats-i386.lib"
+// CHECK-CFI-STATS-WIN32: "--dependent-lib=clang_rt.stats_client-i386.lib"
+// CHECK-CFI-STATS-WIN32: "--dependent-lib=clang_rt.stats-i386.lib"
 // CHECK-CFI-STATS-WIN32: "--linker-option=/include:___sanitizer_stats_register"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -67,11 +67,11 @@
 
 // RUN: %clang_cl -### /FA -fprofile-instr-generate -- %s 2>&1 | FileCheck -check-prefix=CHECK-PROFILE-INSTR-GENERATE %s
 // RUN: %clang_cl -### /FA -fprofile-instr-generate=/tmp/somefile.profraw -- %s 2>&1 | FileCheck -check-prefix=CHECK-PROFILE-INSTR-GENERATE-FILE %s
-// CHECK-PROFILE-INSTR-GENERATE: "-fprofile-instrument=clang" "--dependent-lib={{[^"]*}}clang_rt.profile-{{[^"]*}}.lib"
+// CHECK-PROFILE-INSTR-GENERATE: "-fprofile-instrument=clang" "--dependent-lib=clang_rt.profile-{{[^"]*}}.lib"
 // CHECK-PROFILE-INSTR-GENERATE-FILE: "-fprofile-instrument-path=/tmp/somefile.profraw"
 
 // RUN: %clang_cl -### /FA -fprofile-generate -- %s 2>&1 | FileCheck -check-prefix=CHECK-PROFILE-GENERATE %s
-// CHECK-PROFILE-GENERATE: "-fprofile-instrument=llvm" "--dependent-lib={{[^"]*}}clang_rt.profile-{{[^"]*}}.lib"
+// CHECK-PROFILE-GENERATE: "-fprofile-instrument=llvm" "--dependent-lib=clang_rt.profile-{{[^"]*}}.lib"
 
 // RUN: %clang_cl -### /FA -fprofile-instr-generate -fprofile-instr-use -- %s 2>&1 | FileCheck -check-prefix=CHECK-NO-MIX-GEN-USE %s
 // RUN: %clang_cl -### /FA -fprofile-instr-generate -fprofile-instr-use=file -- %s 2>&1 | FileCheck -check-prefix=CHECK-NO-MIX-GEN-USE %s
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -349,6 +349,16 @@
   

[PATCH] D71201: [ObjC][DWARF] Emit DW_AT_APPLE_objc_direct for methods marked as __attribute__((objc_direct))

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Looks pretty good, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71201



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

[PATCH] D71201: [ObjC][DWARF] Emit DW_AT_APPLE_objc_direct for methods marked as __attribute__((objc_direct))

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:93
 HANDLE_DISP_FLAG((1u << 9), Deleted)
+HANDLE_DISP_FLAG((1u << 10), ObjCDirect)
 

I think it would be good to leave a gap for https://reviews.llvm.org/D68117 
here.
@SouraVX what's the latest plan for representing deleted/defaulted?



Comment at: llvm/test/DebugInfo/X86/objc_direct.ll:47
+@"_OBJC_CLASS_RO_$_Root" = internal global %struct._class_ro_t { i32 2, i32 0, 
i32 0, i8* null, i8* getelementptr inbounds ([5 x i8], [5 x i8]* 
@OBJC_CLASS_NAME_, i32 0, i32 0), %struct.__method_list_t* null, 
%struct._objc_protocol_list* null, %struct._ivar_list_t* null, i8* null, 
%struct._prop_list_t* null }, section "__DATA, __objc_const", align 8
+@"OBJC_LABEL_CLASS_$" = internal global [1 x i8*] [i8* bitcast 
(%struct._class_t* @"OBJC_CLASS_$_Root" to i8*)], section 
"__DATA,__objc_classlist,regular,no_dead_strip", align 8
+@llvm.compiler.used = appending global [2 x i8*] [i8* getelementptr inbounds 
([5 x i8], [5 x i8]* @OBJC_CLASS_NAME_, i32 0, i32 0), i8* bitcast ([1 x i8*]* 
@"OBJC_LABEL_CLASS_$" to i8*)], section "llvm.metadata"

Could we make do in this test without all the ObjC metadata?



Comment at: llvm/test/DebugInfo/X86/objc_direct.ll:81
+
+attributes #0 = { noinline optnone ssp uwtable 
"correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" 
"frame-pointer"="all" "less-precise-fpmad"="false" "min-legal-vector-width"="0" 
"no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" 
"no-signed-zeros-fp-math"="false" "no-trapping-math"="false" 
"stack-protector-buffer-size"="8" "target-cpu"="penryn" 
"target-features"="+cx16,+cx8,+fxsr,+mmx,+sahf,+sse,+sse2,+sse3,+sse4.1,+ssse3,+x87"
 "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind readnone speculatable willreturn }

Please remove all quoted attributes, they cause trouble for maintaining the 
test in the future and usually aren't needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71201



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

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

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

xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:603
+  // point.
+  class CollectReachableSymbolsCallback final : public SymbolVisitor {
+ProgramStateRef State;

NoQ wrote:
> WDYT about re-using `ExprEngine::escapeValue()` by changing it to accept 
> `ArrayRef` instead of a single `SVal`?
It is not entirely the same. Here we only collect symbols from non-stack 
regions. There (as far as I understand) we collect all symbols. I could add a 
flag but at that point I wonder if it is worth the change.


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

https://reviews.llvm.org/D71224



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

[PATCH] D70520: [WebAssembly] Add new `export_name` clang attribute for controlling wasm export names

[Derek Schuff via Phabricator via cfe-commits]

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



Comment at: lld/test/wasm/export-name.ll:20
+; CHECK-NEXT:Exports:
+; CHECK-NEXT:  - Name:memory
+; CHECK-NEXT:Kind:MEMORY

does this test need to verify that the memory and _start are exported? seems 
like just a check for bar would be enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70520



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

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:603
+  // point.
+  class CollectReachableSymbolsCallback final : public SymbolVisitor {
+ProgramStateRef State;

WDYT about re-using `ExprEngine::escapeValue()` by changing it to accept 
`ArrayRef` instead of a single `SVal`?


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

https://reviews.llvm.org/D71224



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

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

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

xazax.hun updated this revision to Diff 232968.
xazax.hun edited the summary of this revision.
xazax.hun added a comment.

- Added a test. More rigorous tests will come in  the FuchsiaHandleChecker.
- Added a new PSK_ entry.


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

https://reviews.llvm.org/D71224

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ponter-escape-on-conservative-calls.c

Index: clang/test/Analysis/ponter-escape-on-conservative-calls.c
===
--- /dev/null
+++ clang/test/Analysis/ponter-escape-on-conservative-calls.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=debug.AnalysisOrder -analyzer-config debug.AnalysisOrder:PointerEscape=true -analyzer-config debug.AnalysisOrder:PostCall=true %s 2>&1 | FileCheck %s
+
+
+void f(int *);
+
+int main() {
+int a;
+f();
+return 0;
+}
+
+// CHECK: PostCall
+// CHECK-NEXT: PointerEscape
Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -37,6 +37,7 @@
 // CHECK-NEXT: debug.AnalysisOrder:EndFunction = false
 // CHECK-NEXT: debug.AnalysisOrder:LiveSymbols = false
 // CHECK-NEXT: debug.AnalysisOrder:NewAllocator = false
+// CHECK-NEXT: debug.AnalysisOrder:PointerEscape = false
 // CHECK-NEXT: debug.AnalysisOrder:PostCall = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtArraySubscriptExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXNewExpr = false
@@ -97,4 +98,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 94
+// CHECK-NEXT: num-entries = 95
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -592,9 +592,71 @@
   for (auto I : dstCallEvaluated)
 finishArgumentConstruction(dstArgumentCleanup, I, Call);
 
-  // Finally, run any post-call checks.
-  getCheckerManager().runCheckersForPostCall(Dst, dstArgumentCleanup,
+  ExplodedNodeSet dstPostCall;
+  getCheckerManager().runCheckersForPostCall(dstPostCall, dstArgumentCleanup,
  Call, *this);
+
+  // Escaping symbols conjured during invalidationg the regions above.
+  // Note that, for inlined calls the nodes were put back into the worklist,
+  // so we can assume that every node belongs to a conservative call at this
+  // point.
+  class CollectReachableSymbolsCallback final : public SymbolVisitor {
+ProgramStateRef State;
+InvalidatedSymbols 
+
+  public:
+explicit CollectReachableSymbolsCallback(ProgramStateRef State,
+ InvalidatedSymbols )
+: State(State), Symbols(Symbols) {}
+
+bool VisitSymbol(SymbolRef Sym) override { return true; }
+bool VisitMemRegion(const MemRegion *MR) override {
+  if (MR->hasStackStorage())
+return false;
+  QualType T;
+  if (const TypedRegion *TR = dyn_cast(MR))
+T = TR->getLocationType();
+  else if (const SymbolicRegion *SR = dyn_cast(MR))
+T = SR->getSymbol()->getType();
+  if (T->isVoidPointerType())
+return false;
+  SVal StoredVal = State->getSVal(MR);
+  if (SymbolRef Sym = StoredVal.getAsSymbol())
+Symbols.insert(Sym);
+  return true;
+}
+  };
+
+  // Run pointerEscape callback with the newly conjured symbols.
+  for (auto I : dstPostCall) {
+NodeBuilder B(I, Dst, *currBldrCtx);
+InvalidatedSymbols Symbols;
+ProgramStateRef State = I->getState();
+CollectReachableSymbolsCallback Scanner(State, Symbols);
+const FunctionDecl *FuncDecl =
+dyn_cast_or_null(Call.getDecl());
+if (FuncDecl) {
+  for (unsigned Arg = 0; Arg < Call.getNumArgs(); ++Arg) {
+if (Arg >= FuncDecl->getNumParams())
+  break;
+QualType ParamTy = FuncDecl->getParamDecl(Arg)->getType();
+if (ParamTy.isNull() ||
+(!ParamTy->isPointerType() && !ParamTy->isReferenceType()))
+  continue;
+if (ParamTy->getPointeeType().isConstQualified())
+  continue;
+State->scanReachableSymbols(Call.getArgSVal(Arg), Scanner);
+  }
+}
+
+State = getCheckerManager().runCheckersForPointerEscape(
+State, Symbols, , PSK_EscapeOnConservativeCall, nullptr);
+
+if (State == I->getState())
+  Dst.insert(I);
+else
+  B.generateNode(I->getLocation(), State, I);
+  }
 }
 
 ProgramStateRef 

[clang] 9c6b7f6 - Revert "[ARM][MVE] Add intrinsics for immediate shifts."

[Eric Christopher via cfe-commits]

Author: Eric Christopher
Date: 2019-12-09T16:47:38-08:00
New Revision: 9c6b7f68b807250e7c3aa01938339fdbd239c4ea

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

LOG: Revert "[ARM][MVE] Add intrinsics for immediate shifts."
and two follow-on commits: one warning fix and one functionality.

As it's breaking at least the lto bot:

http://lab.llvm.org:8011/builders/clang-with-lto-ubuntu/builds/15132/steps/test-stage1-compiler/logs/stdio

This reverts commits:

 8d70f3c933a5b81a87a5ab1af0e3e98ee2cd7c67
 ff4dceef9201c5ae3924e92f6955977f243ac71d
 d97b3e3e65cd77a81b39732af84a1a4229e95091

Added: 


Modified: 
clang/include/clang/Basic/arm_mve.td
clang/include/clang/Basic/arm_mve_defs.td
clang/lib/CodeGen/CGBuiltin.cpp
clang/utils/TableGen/MveEmitter.cpp
llvm/include/llvm/IR/IntrinsicsARM.td
llvm/lib/Target/ARM/ARMInstrMVE.td

Removed: 
clang/test/CodeGen/arm-mve-intrinsics/vector-shift-imm.c
llvm/test/CodeGen/Thumb2/mve-intrinsics/vector-shift-imm.ll



diff  --git a/clang/include/clang/Basic/arm_mve.td 
b/clang/include/clang/Basic/arm_mve.td
index 426d3b5a2f44..f3d3f4124101 100644
--- a/clang/include/clang/Basic/arm_mve.td
+++ b/clang/include/clang/Basic/arm_mve.td
@@ -609,33 +609,6 @@ defm vstrhq: scatter_offset_both;
 defm vstrwq: scatter_offset_both;
 defm vstrdq: scatter_offset_both;
 
-multiclass PredicatedImmediateVectorShift<
-Immediate immtype, string predIntrName, list unsignedFlag = []> {
-  foreach predIntr = [IRInt] in {
-def _m_n: Intrinsic;
-def _x_n: Intrinsic;
-  }
-}
-
-let params = T.Int in {
-  def vshlq_n: Intrinsic;
-  defm vshlq: PredicatedImmediateVectorShift;
-
-  let pnt = PNT_NType in {
-def vshrq_n: Intrinsic;
-defm vshrq: PredicatedImmediateVectorShift;
-  }
-}
-
 // Base class for the scalar shift intrinsics.
 class ScalarShift:
   Intrinsic 
{

diff  --git a/clang/include/clang/Basic/arm_mve_defs.td 
b/clang/include/clang/Basic/arm_mve_defs.td
index 6bc9b35f0fc4..1d72cc45796c 100644
--- a/clang/include/clang/Basic/arm_mve_defs.td
+++ b/clang/include/clang/Basic/arm_mve_defs.td
@@ -66,10 +66,6 @@ def xor: IRBuilder<"CreateXor">;
 def sub: IRBuilder<"CreateSub">;
 def shl: IRBuilder<"CreateShl">;
 def lshr: IRBuilder<"CreateLShr">;
-def immshr: CGHelperFn<"MVEImmediateShr"> {
-  let special_params = [IRBuilderIntParam<1, "unsigned">,
-IRBuilderIntParam<2, "bool">];
-}
 def fadd: IRBuilder<"CreateFAdd">;
 def fmul: IRBuilder<"CreateFMul">;
 def fsub: IRBuilder<"CreateFSub">;
@@ -322,8 +318,8 @@ def imm_simd_vmvn : Immediate {
 //
 // imm_0toNm1 is the same but with the range offset by 1, i.e. 0 to N-1
 // inclusive.
-def imm_1toN : Immediate>;
-def imm_0toNm1 : Immediate>;
+def imm_1toN : Immediate>;
+def imm_0toNm1 : Immediate>;
 
 // imm_lane has to be the index of a vector lane in the main vector type, i.e
 // it can range from 0 to (128 / size of scalar)-1 inclusive. (e.g. vgetq_lane)

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 890019ac51c2..7447a5841599 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -6802,14 +6802,6 @@ Value *CodeGenFunction::EmitARMBuiltinExpr(unsigned 
BuiltinID,
   }
 }
 
-template
-static Integer GetIntegerConstantValue(const Expr *E, ASTContext ) {
-  llvm::APSInt IntVal;
-  assert(E->isIntegerConstantExpr(IntVal, Context) &&
- "Sema should have checked this was a constant");
-  return IntVal.getExtValue();
-}
-
 static llvm::Value *SignOrZeroExtend(CGBuilderTy , llvm::Value *V,
  llvm::Type *T, bool Unsigned) {
   // Helper function called by Tablegen-constructed ARM MVE builtin codegen,
@@ -6817,27 +6809,6 @@ static llvm::Value *SignOrZeroExtend(CGBuilderTy 
, llvm::Value *V,
   return Unsigned ? Builder.CreateZExt(V, T) : Builder.CreateSExt(V, T);
 }
 
-static llvm::Value *MVEImmediateShr(CGBuilderTy , llvm::Value *V,
-uint32_t Shift, bool Unsigned) {
-  // MVE helper function for integer shift right. This must handle signed vs
-  // unsigned, and also deal specially with the case where the shift count is
-  // equal to the lane size. In LLVM IR, an LShr with that parameter would be
-  // undefined behavior, but in MVE it's legal, so we must convert it to code
-  // that is not undefined in IR.
-  unsigned LaneBits =
-  V->getType()->getVectorElementType()->getPrimitiveSizeInBits();
-  if (Shift == LaneBits) {
-// An unsigned shift of the full lane size always generates zero, so we can
-// simply emit a zero vector. A signed shift of the full lane size does the
-// same thing as shifting by one bit fewer.
-if (Unsigned)
-  return llvm::Constant::getNullValue(V->getType());
-

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[David Blaikie via Phabricator via cfe-commits]

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

In D70696#1774931 , @Jac1494 wrote:

> For some real life case like below we need debuginfo for declaration of 
> global extern variable .
>
> $cat shlib.c
>  int var;
>  int test()
>  { return var++; }
>
> $cat test.c
>  extern int test();
>  extern int var;
>  int main()
>  { var++; printf("%d\n",test()); }
>
> If we debug above case with gdb it is not giving types of variable var.
>  Because of no variable DIE is there in executable.
>
> (gdb) b main
>  Breakpoint 1 at 0x40063c: file test.c, line 4.
>  (gdb) pt var
>  type = 


Sounds like this is if you build shlib without debug info?

If you build shlib with debug info, it seems to be fine:

  $ clang-tot -shared -o libshlib.so shlib.c -fpic -g
  $ clang-tot test.c -lshlib -g -L.
  $ LD_LIBRARY_PATH=. gdb ./a.out
  (gdb) start
  Temporary breakpoint 1, main () at test.c:4
  4   { var++; printf("%d\n",test()); }
  (gdb) pt var
  type = int
  (gdb) 

This is the same as if the code were compiled statically & one test.c was 
compiled with debug info but shlib.c was compiled without debug info - the 
shared-library-ness doesn't change this situation.

-fstandalone-debug is the flag to use if parts of the program are built without 
debug info. Yes, that could be used to add global variable declaration to the 
DWARF & currently isn't - but I think that's a sufficiently nuanced/separate 
feature built on top of the work in this patch that it should be done 
separately, rather than trying to have all the design discussion for that in 
this code review.

This looks fine to me - I think a lot of the testing probably isn't testing 
"interesting" cases, but up to you.




Comment at: clang/test/CodeGen/debug-info-extern-basic.c:14-23
+extern int (*foo)(int);
+int test3() {
+  return foo(0);
+}
+
+int test4() {
+  extern int (*foo2)(int);

What do these tests add? What sort of bugs would be caught by these global 
function pointer tests that wouldn't be caught by the char tests above them?



Comment at: clang/test/CodeGen/debug-info-extern-duplicate.c:4-38
+extern char ch;
+extern char ch;
+int test() {
+  return ch;
+}
+
+extern char ch2;

Similarly - are these covering novel/distinct codepaths from the other tests? 
Are there codepaths that are different for multiple declarations?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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

[PATCH] D71225: [OpenMP][WIP] atomic update only evaluate once for expression having side effect

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71225#1776113 , @jdoerfert wrote:

> In D71225#1776105 , @cchen wrote:
>
> > Oops, accidentally remove my own comment. I'm not sure why `iarr[foo(), 
> > foo(), 0]` violate the rule since it will be evaluated as `iarr[0]`, and 
> > the counter `foo()` modified is also in the same location for both left 
> > hand side and right hand side.
>
>
> It's not a violation but it taps into the unspecified behavior, which in 
> combination with side effects seems to be worth a warning.
>
> So my reasoning is:
>
> //x := `iarr[foo(), foo(), 0]`//
>
> which is fine on its own. Using it in the atomic update of the form,
>
> // x = x + 1//
>
> also fine.
>
> However, the standard says the number of times //x// is evaluated is 
> unspecified. 
>  Thus, transforming the above to
>
> // x += 1 //
>
> should be valid. So should be,
>
> // t =  *t = *t + 1 //
>
> and even
>
> // x = (x, x, x, x+1) //


Agree, missed that he used commas instead of array expressions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71225



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

[PATCH] D70701: Fix more VFS tests on Windows

[Adrian McCarthy via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1431
 
-if (IsRootEntry && !sys::path::is_absolute(Name)) {
-  assert(NameValueNode && "Name presence should be checked earlier");

rnk wrote:
> amccarth wrote:
> > rnk wrote:
> > > Is there a way to unit test this? I see some existing tests in 
> > > llvm/unittests/Support/VirtualFileSystemTest.cpp.
> > > 
> > > I looked at the yaml files in the VFS tests this fixes, and I see entries 
> > > like this:
> > > { 'name': '/tests', 'type': 'directory', ... },
> > > { 'name': '/indirect-vfs-root-files', 'type': 'directory', ... },
> > > { 'name': 
> > > 'C:/src/llvm-project/clang/test/VFS/Inputs/Broken.framework', 'type': 
> > > 'directory', ... },
> > > { 'name': 
> > > 'C:/src/llvm-project/build/tools/clang/test/VFS/Output/vfsroot-with-overlay.c.tmp/cache',
> > >  'type': 'directory', ... },
> > > 
> > > So it makes sense to me that we need to handle both kinds of absolute 
> > > path.
> > > Is there a way to unit test this?
> > 
> > What do you mean by "this"?  The `/tests` and `/indirect-vfs-root-files` 
> > entries in that yaml are the ones causing several tests to fail without 
> > this fix, so I take it that this is already being tested.  But perhaps you 
> > meant testing something more specific?
> > What do you mean by "this"? 
> I guess what I meant was, can you unit test the whole change in case there 
> are behavior differences here not covered by the clang tests?
This change causes no regressions in those llvm unit tests 
(`llvm/unittests/Support/VirtualFileSystemTest.cpp`).  They all still pass.

But thanks for pointing those out, because my subsequent changes do seem to 
make a difference.


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

https://reviews.llvm.org/D70701



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

[clang] 9803178 - Avoid Attr.h includes, CodeGen edition

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2019-12-09T16:17:18-08:00
New Revision: 9803178a78c1858b0ac868c2cdf402cec5a10db9

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

LOG: Avoid Attr.h includes, CodeGen edition

This saves around 20 includes of Attr.h. Not much.

Added: 


Modified: 
clang/include/clang/CodeGen/CGFunctionInfo.h
clang/lib/CodeGen/CGBlocks.cpp
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CGCXX.cpp
clang/lib/CodeGen/CGCXXABI.cpp
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGClass.cpp
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGDecl.cpp
clang/lib/CodeGen/CGDeclCXX.cpp
clang/lib/CodeGen/CGExprAgg.cpp
clang/lib/CodeGen/CGExprConstant.cpp
clang/lib/CodeGen/CGExprScalar.cpp
clang/lib/CodeGen/CGObjC.cpp
clang/lib/CodeGen/CGObjCGNU.cpp
clang/lib/CodeGen/CGObjCMac.cpp
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
clang/lib/CodeGen/CGStmt.cpp
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/CodeGen/CGVTables.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/CodeGen/CodeGenModule.h
clang/lib/CodeGen/ItaniumCXXABI.cpp
clang/lib/CodeGen/MicrosoftCXXABI.cpp
clang/lib/CodeGen/SanitizerMetadata.cpp
clang/lib/CodeGen/TargetInfo.cpp

Removed: 




diff  --git a/clang/include/clang/CodeGen/CGFunctionInfo.h 
b/clang/include/clang/CodeGen/CGFunctionInfo.h
index 5069d9af42a3..65a1061f12a8 100644
--- a/clang/include/clang/CodeGen/CGFunctionInfo.h
+++ b/clang/include/clang/CodeGen/CGFunctionInfo.h
@@ -15,7 +15,6 @@
 #ifndef LLVM_CLANG_CODEGEN_CGFUNCTIONINFO_H
 #define LLVM_CLANG_CODEGEN_CGFUNCTIONINFO_H
 
-#include "clang/AST/Attr.h"
 #include "clang/AST/CanonicalType.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/Decl.h"

diff  --git a/clang/lib/CodeGen/CGBlocks.cpp b/clang/lib/CodeGen/CGBlocks.cpp
index 6a1a73955319..b0295ce69bff 100644
--- a/clang/lib/CodeGen/CGBlocks.cpp
+++ b/clang/lib/CodeGen/CGBlocks.cpp
@@ -19,6 +19,7 @@
 #include "CodeGenModule.h"
 #include "ConstantEmitter.h"
 #include "TargetInfo.h"
+#include "clang/AST/Attr.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/CodeGen/ConstantInitBuilder.h"
 #include "llvm/ADT/SmallSet.h"

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 7b5d691d26cf..890019ac51c2 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -20,6 +20,7 @@
 #include "PatternInit.h"
 #include "TargetInfo.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/OSLog.h"
 #include "clang/Basic/TargetBuiltins.h"

diff  --git a/clang/lib/CodeGen/CGCXX.cpp b/clang/lib/CodeGen/CGCXX.cpp
index 7e5fe0fd6b1d..1928e0df3809 100644
--- a/clang/lib/CodeGen/CGCXX.cpp
+++ b/clang/lib/CodeGen/CGCXX.cpp
@@ -12,10 +12,11 @@
 
 // We might split this into multiple files if it gets too unwieldy
 
-#include "CodeGenModule.h"
 #include "CGCXXABI.h"
 #include "CodeGenFunction.h"
+#include "CodeGenModule.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"

diff  --git a/clang/lib/CodeGen/CGCXXABI.cpp b/clang/lib/CodeGen/CGCXXABI.cpp
index 23dae2b61d04..7ada4032b3ee 100644
--- a/clang/lib/CodeGen/CGCXXABI.cpp
+++ b/clang/lib/CodeGen/CGCXXABI.cpp
@@ -13,6 +13,7 @@
 
 #include "CGCXXABI.h"
 #include "CGCleanup.h"
+#include "clang/AST/Attr.h"
 
 using namespace clang;
 using namespace CodeGen;

diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 657c9260e6e6..4cf709be7429 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -19,6 +19,7 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "TargetInfo.h"
+#include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
@@ -28,7 +29,6 @@
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "clang/CodeGen/SwiftCallingConv.h"
 #include "llvm/ADT/StringExtras.h"
-#include "llvm/Transforms/Utils/Local.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/CallingConv.h"
@@ -36,6 +36,7 @@
 #include "llvm/IR/InlineAsm.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Intrinsics.h"
+#include "llvm/Transforms/Utils/Local.h"
 using namespace clang;
 using namespace CodeGen;
 

diff  --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index 64c4d3e423fd..3f3825b76275 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -16,6 +16,7 @@
 #include "CGRecordLayout.h"
 #include 

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71179#1776108 , @jdoerfert wrote:

> In D71179#1776046 , @ABataev wrote:
>
> > In D71179#1776034 , @jdoerfert 
> > wrote:
> >
> > > > Not always. If we see that the context selector does not match, we can 
> > > > skip everything between begin/end. It means exactly what I said - 
> > > > multiversioning is needed only for `construct` because all other traits 
> > > > can be easily resolved at the compile time. Generally speaking, there 
> > > > are 2 kinds of traits - global traits (like `vendor`, `kind`, `isa`, 
> > > > etc.), which can be resolved completely statically and do not need 
> > > > multiversioning, and local traits, like `construct`, which depend on 
> > > > the OpenMP directives and require something similar to the 
> > > > multiversioning.
> > >
> > > The case where the code is skipped is easy, sure. However, if we "could 
> > > easily resolve" the other case, we could have implemented an #ifdef 
> > > solution for `math.h/cmath`. This was not the case and still is not. We 
> > > basically populate the namespace with multiple versions of the same 
> > > function (with the same name) and then select the appropriate one for 
> > > each call site.
> > >
> > > Instead of trying to argue why this is not needed for some cases, could 
> > > you argue why we should have multiple schemes to resolve all types of 
> > > variants? It seems you inherently assume the codegen patching scheme 
> > > implemented right now is useful even if we need something else to 
> > > complement it. I don't think so, thus there is little reason for me to 
> > > distinguish between the types of variants that need multi-version support 
> > > ant the types that can be implemented with multi-versions but don't need 
> > > it.
> >
> >
> > Because each particular problem requires its own solution and it is always 
> > a bad idea to use the microscope to hammer the nails.
>
>
> While I see where you are coming from, I disagree. We have a generic 
> framework available that we already need to use in some cases, there is no 
> harm in using it for all cases. It would be different if we wouldn't need the 
> generic framework at all, but that is not the case. All I ask is to literally 
> share existing code, no additional complexity needed. Your suggestion will 
> complicate the setup, duplicate logic, and make it overall harder to maintain 
> and compose in the future. If you still disagree, please provide some 
> arguments (and details) why we would benefit from your setup.


I have different opinion. You can reuse existing codegen for declare variant 
functions with global context selectors only. You just need to iterate through 
all the variants and choose the best one.
That's why you don't need the dispatching in your scheme. You're doing 
absolutely the same thing as the original declare variant implementation.

We cannot use multiversioning for the original declare variant construct since 
there is no multiversioning at all. We have a single function with many 
different aliasing functions, having different names. They are completely 
different functions. And I don't think it would be correct to add them as 
multiversiin variants to the original function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[PATCH] D70627: [AST] Split out Attrs.h for table generated *Attr classes

[Reid Kleckner via Phabricator via cfe-commits]

rnk abandoned this revision.
rnk added a comment.

I think this isn't necessary. I think I got most of the value of this in 
rG60573ae6fe509b618dc6a2c5c55d921bccd77608 
, and we 
don't need two AttrBase.h / Attr.h headers in the long run. This is how many 
files depend on Attr.h now:

  $ ninja -t deps | grep 'AST.Attr\.h' | wc -l
  683

I think most deps are through Sema.h and ASTMatchers.h:

  $ ninja -t deps | grep 'Sema.Sema\.h' | wc -l
  102
  $ ninja -t deps | grep 'ASTMatchers.ASTMatchers\.h' | wc -l
  391


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70627



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

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

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

xazax.hun updated this revision to Diff 232955.
xazax.hun added a comment.

- Do not track explicitly if a call was conservative.


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

https://reviews.llvm.org/D71224

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

Index: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -10,6 +10,7 @@
 //
 //===--===//
 
+#include "clang/AST/Type.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "PrettyStackTraceLocationContext.h"
 #include "clang/AST/CXXInheritance.h"
@@ -18,6 +19,8 @@
 #include "clang/Analysis/ConstructionContext.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/Store.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Support/SaveAndRestore.h"
@@ -576,15 +579,14 @@
 
   // Run any pre-call checks using the generic call interface.
   ExplodedNodeSet dstPreVisit;
-  getCheckerManager().runCheckersForPreCall(dstPreVisit, Pred,
-Call, *this);
+  getCheckerManager().runCheckersForPreCall(dstPreVisit, Pred, Call, *this);
 
   // Actually evaluate the function call.  We try each of the checkers
   // to see if the can evaluate the function call, and get a callback at
   // defaultEvalCall if all of them fail.
   ExplodedNodeSet dstCallEvaluated;
-  getCheckerManager().runCheckersForEvalCall(dstCallEvaluated, dstPreVisit,
- Call, *this);
+  getCheckerManager().runCheckersForEvalCall(
+  dstCallEvaluated, dstPreVisit, Call, *this);
 
   // If there were other constructors called for object-type arguments
   // of this call, clean them up.
@@ -592,9 +594,72 @@
   for (auto I : dstCallEvaluated)
 finishArgumentConstruction(dstArgumentCleanup, I, Call);
 
-  // Finally, run any post-call checks.
-  getCheckerManager().runCheckersForPostCall(Dst, dstArgumentCleanup,
+  ExplodedNodeSet dstPostCall;
+  getCheckerManager().runCheckersForPostCall(dstPostCall, dstArgumentCleanup,
  Call, *this);
+
+  // Escaping symbols conjured during invalidationg the regions above.
+  // Note that, for inlined calls the nodes were put back into the worklist,
+  // so we can assume that every node belongs to a conservative call at this
+  // point.
+  class CollectReachableSymbolsCallback final : public SymbolVisitor {
+ProgramStateRef State;
+InvalidatedSymbols 
+
+  public:
+explicit CollectReachableSymbolsCallback(ProgramStateRef State,
+ InvalidatedSymbols )
+: State(State), Symbols(Symbols) {}
+
+bool VisitSymbol(SymbolRef Sym) override { return true; }
+bool VisitMemRegion(const MemRegion *MR) override {
+  if (MR->hasStackStorage())
+return false;
+  QualType T;
+  if (const TypedRegion *TR = dyn_cast(MR))
+T = TR->getLocationType();
+  else if (const SymbolicRegion *SR = dyn_cast(MR))
+T = SR->getSymbol()->getType();
+  if (T->isVoidPointerType())
+return false;
+  SVal StoredVal = State->getSVal(MR);
+  if (SymbolRef Sym = StoredVal.getAsSymbol())
+Symbols.insert(Sym);
+  return true;
+}
+  };
+
+  // Run pointerEscape callback with the newly conjured symbols.
+  for (auto I : dstPostCall) {
+NodeBuilder B(I, Dst, *currBldrCtx);
+InvalidatedSymbols Symbols;
+ProgramStateRef State = I->getState();
+CollectReachableSymbolsCallback Scanner(State, Symbols);
+const FunctionDecl *FuncDecl =
+dyn_cast_or_null(Call.getDecl());
+if (FuncDecl) {
+  for (unsigned Arg = 0; Arg < Call.getNumArgs(); ++Arg) {
+if (Arg >= FuncDecl->getNumParams())
+  break;
+QualType ParamTy = FuncDecl->getParamDecl(Arg)->getType();
+if (ParamTy.isNull() ||
+(!ParamTy->isPointerType() && !ParamTy->isReferenceType()))
+  continue;
+if (ParamTy->getPointeeType().isConstQualified())
+  continue;
+State->scanReachableSymbols(Call.getArgSVal(Arg), Scanner);
+  }
+}
+
+// TODO: the PSK is a lie.
+State = getCheckerManager().runCheckersForPointerEscape(
+State, Symbols, , PSK_DirectEscapeOnCall, nullptr);
+
+if (State != I->getState())
+  B.generateNode(I->getLocation(), State, I);
+else
+  Dst.insert(I);
+  }
 }
 
 ProgramStateRef ExprEngine::bindReturnValue(const CallEvent ,
@@ -644,8 +709,8 @@
 ITraits.setTrait(TargetR,
 

[PATCH] D71208: CodeGen: Allow annotations on globals in non-zero address space

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

The questions I'd like to have answered before I can approve this are:

- whether there are clients of `@llvm.global.annotations` that will have 
problems with non-0 address spaces and
- whether this will interfere with someday having an invariant that 
`addrspacecast` is only used to do legal conversions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71208



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

[PATCH] D69764: [clang-format] Add Left/Right Const (East/West , Before/After) fixer capability

[Ilya Mirsky via Phabricator via cfe-commits]

ilya added a comment.

In D69764#1775842 , @MyDeveloperDay 
wrote:

> In D69764#1775835 , @ilya wrote:
>
> > In D69764#1732235 , @aaron.ballman 
> > wrote:
> >
> > > I like the functionality, but am slightly opposed to using "east/west" 
> > > terminology -- that's not a ubiquitous phrase and it takes a bit of 
> > > thinking before it makes sense. I think "left/right" is likely to be more 
> > > universally understood.
> > >
> > > Also, should this apply to other qualifiers like `volatile` or 
> > > `restrict`? If so, the name `ConstStyle` should probably be 
> > > `CVQualifierStyle` or something else.
> >
> >
> > +1.
> >  In addition to `volatile` and `restrict`, in my organization we'd also be 
> > interested in applying this to address space qualifiers (custom keywords 
> > added in our downstream fork). Can this be generalized to accept a map of 
> > qualifier keywords and its desired orientation?
>
>
> Could you give me an example of what you mean?


As described in section 5 of the embedded C standard ISO/IEC J TC1 SC22 WG14 
N1169 , our 
downstream llvm fork supports several address spaces, let's denote them `__as1` 
and `__as2`. These address space qualifiers behave similarly to `const`, so the 
following are semantically equal:

  __as1 int foo;
  int __as1 bar;

So for us it would be nice to be able to specify alignment options for a 
dynamic list of custom qualifiers. Something like the following:

  QualifierStyle:
const: Right
volatile:  Right
__as1: Right
__as2: Right

But I don't know if this can be (easily) supported in a .clang-format file, 
since the style options are defined as (static) enums. I realize my proposal 
might be out of the scope of this patch, but I wanted to get some opinions from 
the community.


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

https://reviews.llvm.org/D69764



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

[PATCH] D71224: [analyzer][WIP] Escape symbols stored into specific region after a conservative evalcall.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I think you don't need to smuggle `WasConservative` all the way up. It's 
implied that if the evaluation was not conservative, then the respective 
`ExplodedNodeSet` is going to be empty, as all nodes will be put directly into 
the worklist instead. Eg., `checkPostCall` isn't going to be invoked 
immediately after `inlineCall`, but only after `enqueueEndOfFunction`.




Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:650
+
+// TODO: the PSK is a lie.
+State = getCheckerManager().runCheckersForPointerEscape(

xazax.hun wrote:
> How much do we care about the escape kind? For each symbol we need to check 
> if it was directly passed to the callee. It is not too bad I guess, but I was 
> wondering.
Dunno, just introduce a new `PSK_` item and use it here. It isn't supposed to 
be per-symbol, it's just to notify checkers that we're in this new post-call 
invocation for out-parameters, so that they could opt out of the whole callback.


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

https://reviews.llvm.org/D71224



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

[PATCH] D71224: [analyzer][WIP] Escape symbols stored into specific region after a conservative evalcall.

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

xazax.hun marked 3 inline comments as done.
xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:696
   // And make the result node.
   Bldr.generateNode(Call.getProgramPoint(), State, Pred);
 }

NoQ wrote:
> xazax.hun wrote:
> > After some offline conversation it is very likely that we want to move the 
> > `runCheckersForPointerEscape` here. 
> > 
> > The main question is, how should we get all the data?
> > 
> > We should know about:
> > * What regions are output params.
> > * What regions are considered escaped.
> > * What regions have traits that prevents escaping.
> > 
> > Is there anything else?
> > What regions are output params.
> 
> That should be obvious from the AST. Like, parameters of non-const 
> pointer/reference types.
> 
> > What regions are considered escaped.
> 
> Output parameter regions (as `TopLevelInvalidated`) and whatever's reachable 
> from them.
> 
> > What regions have traits that prevents escaping.
> 
> Currently the only trait that affects escaping (as opposed to invalidation) 
> is `TK_SuppressEscape` and it is never applied to out-parameters.
> After some offline conversation it is very likely that we want to move the 
> runCheckersForPointerEscape here.

Nope, we do not want to move it here. We want the pointerEscape to happen AFTER 
postCall callbacks.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:650
+
+// TODO: the PSK is a lie.
+State = getCheckerManager().runCheckersForPointerEscape(

How much do we care about the escape kind? For each symbol we need to check if 
it was directly passed to the callee. It is not too bad I guess, but I was 
wondering.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:1151
+  // TODO: it is a path sensitive property if a call was inlined.
+  return false;
 }

Hmm. Should I introduce a very short lived trait or do we have a better way to 
deal with stuff like this? A short lived map? 


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

https://reviews.llvm.org/D71224



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

[PATCH] D71224: [analyzer][WIP] Escape symbols stored into specific region after a conservative evalcall.

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

xazax.hun updated this revision to Diff 232948.
xazax.hun added a comment.

- Prototype a new approach.


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

https://reviews.llvm.org/D71224

Files:
  clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/lib/StaticAnalyzer/Core/CheckerManager.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

Index: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -18,6 +18,8 @@
 #include "clang/Analysis/ConstructionContext.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState_Fwd.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/Store.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Support/SaveAndRestore.h"
@@ -576,15 +578,14 @@
 
   // Run any pre-call checks using the generic call interface.
   ExplodedNodeSet dstPreVisit;
-  getCheckerManager().runCheckersForPreCall(dstPreVisit, Pred,
-Call, *this);
+  getCheckerManager().runCheckersForPreCall(dstPreVisit, Pred, Call, *this);
 
   // Actually evaluate the function call.  We try each of the checkers
   // to see if the can evaluate the function call, and get a callback at
   // defaultEvalCall if all of them fail.
   ExplodedNodeSet dstCallEvaluated;
-  getCheckerManager().runCheckersForEvalCall(dstCallEvaluated, dstPreVisit,
- Call, *this);
+  bool WasConservative = getCheckerManager().runCheckersForEvalCall(
+  dstCallEvaluated, dstPreVisit, Call, *this);
 
   // If there were other constructors called for object-type arguments
   // of this call, clean them up.
@@ -593,8 +594,68 @@
 finishArgumentConstruction(dstArgumentCleanup, I, Call);
 
   // Finally, run any post-call checks.
-  getCheckerManager().runCheckersForPostCall(Dst, dstArgumentCleanup,
+  if (!WasConservative) {
+getCheckerManager().runCheckersForPostCall(Dst, dstArgumentCleanup, Call,
+   *this);
+return;
+  }
+
+  // Escaping symbols conjured during invalidationg the regions above.
+  class CollectReachableSymbolsCallback final : public SymbolVisitor {
+ProgramStateRef State;
+InvalidatedSymbols 
+
+  public:
+explicit CollectReachableSymbolsCallback(ProgramStateRef State,
+ InvalidatedSymbols )
+: State(State), Symbols(Symbols) {}
+
+bool VisitSymbol(SymbolRef Sym) override { return true; }
+bool VisitMemRegion(const MemRegion *MR) override {
+  if (MR->hasStackStorage())
+return false;
+  SVal StoredVal = State->getSVal(MR);
+  if (SymbolRef Sym = StoredVal.getAsSymbol())
+Symbols.insert(Sym);
+  return true;
+}
+  };
+
+  ExplodedNodeSet dstPostCall;
+  getCheckerManager().runCheckersForPostCall(dstPostCall, dstArgumentCleanup,
  Call, *this);
+
+  // Run pointerEscape callback with the newly conjured symbols.
+  for (auto I : dstPostCall) {
+NodeBuilder B(I, Dst, *currBldrCtx);
+InvalidatedSymbols Symbols;
+ProgramStateRef State = I->getState();
+CollectReachableSymbolsCallback Scanner(State, Symbols);
+const FunctionDecl *FuncDecl =
+dyn_cast_or_null(Call.getDecl());
+if (FuncDecl) {
+  for (unsigned Arg = 0; Arg < Call.getNumArgs(); ++Arg) {
+if (Arg >= FuncDecl->getNumParams())
+  break;
+QualType ParamTy = FuncDecl->getParamDecl(Arg)->getType();
+if (ParamTy.isNull() ||
+(!ParamTy->isPointerType() && !ParamTy->isReferenceType()))
+  continue;
+if (ParamTy->getPointeeType().isConstQualified())
+  continue;
+State->scanReachableSymbols(Call.getArgSVal(Arg), Scanner);
+  }
+}
+
+// TODO: the PSK is a lie.
+State = getCheckerManager().runCheckersForPointerEscape(
+State, Symbols, , PSK_DirectEscapeOnCall, nullptr);
+
+if (State != I->getState())
+  B.generateNode(I->getLocation(), State, I);
+else
+  Dst.insert(I);
+  }
 }
 
 ProgramStateRef ExprEngine::bindReturnValue(const CallEvent ,
@@ -644,8 +705,8 @@
 ITraits.setTrait(TargetR,
 RegionAndSymbolInvalidationTraits::TK_DoNotInvalidateSuperRegion);
 State = State->invalidateRegions(TargetR, E, Count, LCtx,
- /* CausesPointerEscape=*/false, nullptr,
- , );
+ /* CausesPointerEscape=*/false,
+ nullptr, , );
 
 R = 

[clang] e6e6e34 - [c++20] Defaulted comparison support for array members.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-12-09T14:54:06-08:00
New Revision: e6e6e34b95cfe03275943fde0db259cc7d57f4ad

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

LOG: [c++20] Defaulted comparison support for array members.

Added: 


Modified: 
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/class/class.compare/class.compare.default/p5.cpp
clang/test/CXX/class/class.compare/class.eq/p2.cpp
clang/test/CXX/class/class.compare/class.eq/p3.cpp
clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
clang/test/CXX/class/class.compare/class.spaceship/p3.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 28159b9c0812..1ee6ee5dcf12 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -7442,6 +7442,7 @@ class DefaultedComparisonSynthesizer
 StmtListResult, StmtResult,
 std::pair> {
   SourceLocation Loc;
+  unsigned ArrayDepth = 0;
 
 public:
   using Base = DefaultedComparisonVisitor;
@@ -7467,29 +7468,56 @@ class DefaultedComparisonSynthesizer
 case DefaultedComparisonKind::None:
   llvm_unreachable("not a defaulted comparison");
 
-case DefaultedComparisonKind::Equal:
+case DefaultedComparisonKind::Equal: {
   // C++2a [class.eq]p3:
   //   [...] compar[e] the corresponding elements [...] until the first
   //   index i where xi == yi yields [...] false. If no such index exists,
   //   V is true. Otherwise, V is false.
   //
   // Join the comparisons with '&&'s and return the result. Use a right
-  // fold because that short-circuits more naturally.
-  for (Stmt *EAsStmt : llvm::reverse(Stmts.Stmts)) {
-Expr *E = cast(EAsStmt);
-if (RetVal.isUnset()) {
-  RetVal = E;
+  // fold (traversing the conditions right-to-left), because that
+  // short-circuits more naturally.
+  auto OldStmts = std::move(Stmts.Stmts);
+  Stmts.Stmts.clear();
+  ExprResult CmpSoFar;
+  // Finish a particular comparison chain.
+  auto FinishCmp = [&] {
+if (Expr *Prior = CmpSoFar.get()) {
+  // Convert the last expression to 'return ...;'
+  if (RetVal.isUnset() && Stmts.Stmts.empty())
+RetVal = CmpSoFar;
+  // Convert any prior comparison to 'if (!(...)) return false;'
+  else if (Stmts.add(buildIfNotCondReturnFalse(Prior)))
+return true;
+  CmpSoFar = ExprResult();
+}
+return false;
+  };
+  for (Stmt *EAsStmt : llvm::reverse(OldStmts)) {
+Expr *E = dyn_cast(EAsStmt);
+if (!E) {
+  // Found an array comparison.
+  if (FinishCmp() || Stmts.add(EAsStmt))
+return StmtError();
+  continue;
+}
+
+if (CmpSoFar.isUnset()) {
+  CmpSoFar = E;
   continue;
 }
-RetVal = S.CreateBuiltinBinOp(Loc, BO_LAnd, E, RetVal.get());
-if (RetVal.isInvalid())
+CmpSoFar = S.CreateBuiltinBinOp(Loc, BO_LAnd, E, CmpSoFar.get());
+if (CmpSoFar.isInvalid())
   return StmtError();
   }
+  if (FinishCmp())
+return StmtError();
+  std::reverse(Stmts.Stmts.begin(), Stmts.Stmts.end());
   //   If no such index exists, V is true.
   if (RetVal.isUnset())
 RetVal = S.ActOnCXXBoolLiteral(Loc, tok::kw_true);
-  Stmts.Stmts.clear();
   break;
+}
 
 case DefaultedComparisonKind::ThreeWay: {
   // Per C++2a [class.spaceship]p3, as a fallback add:
@@ -7579,7 +7607,100 @@ class DefaultedComparisonSynthesizer
   // FIXME: When expanding a subobject, register a note in the code synthesis
   // stack to say which subobject we're comparing.
 
-  // FIXME: Build a loop for an array subobject.
+  StmtResult buildIfNotCondReturnFalse(ExprResult Cond) {
+if (Cond.isInvalid())
+  return StmtError();
+
+ExprResult NotCond = S.CreateBuiltinUnaryOp(Loc, UO_LNot, Cond.get());
+if (NotCond.isInvalid())
+  return StmtError();
+
+ExprResult False = S.ActOnCXXBoolLiteral(Loc, tok::kw_false);
+assert(!False.isInvalid() && "should never fail");
+StmtResult ReturnFalse = S.BuildReturnStmt(Loc, False.get());
+if (ReturnFalse.isInvalid())
+  return StmtError();
+
+return S.ActOnIfStmt(Loc, false, nullptr,
+ S.ActOnCondition(nullptr, Loc, NotCond.get(),
+  Sema::ConditionKind::Boolean),
+ ReturnFalse.get(), SourceLocation(), nullptr);
+  }
+
+  StmtResult visitSubobjectArray(QualType Type, llvm::APInt Size,
+ ExprPair Subobj) {
+QualType SizeType = 

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

In D71091#1776009 , @liuliu wrote:

> No problem! I have a custom patch that isn't as nice but get the job done. 
> Let me know when you put new diff out and I can test it on our codebase again.


https://reviews.llvm.org/D71226/new/ is the incremental patch as this one is 
already merged


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71091



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

[PATCH] D71225: [OpenMP][WIP] atomic update only evaluate once for expression having side effect

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

@jdoerfert , thanks for your explanation, it's really clear and helpful. I'll 
add a warning message in another patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71225



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

[PATCH] D71225: [OpenMP][WIP] atomic update only evaluate once for expression having side effect

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D71225#1776105 , @cchen wrote:

> Oops, accidentally remove my own comment. I'm not sure why `iarr[foo(), 
> foo(), 0]` violate the rule since it will be evaluated as `iarr[0]`, and the 
> counter `foo()` modified is also in the same location for both left hand side 
> and right hand side.


It's not a violation but it taps into the unspecified behavior, which in 
combination with side effects seems to be worth a warning.

So my reasoning is:

//x := `iarr[foo(), foo(), 0]`//

which is fine on its own. Using it in the atomic update of the form,

// x = x + 1//

also fine.

However, the standard says the number of times //x// is evaluated is 
unspecified. 
Thus, transforming the above to

// x += 1 //

should be valid. So should be,

// t =  *t = *t + 1 //

and even

// x = (x, x, x, x+1) //


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71225



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

[PATCH] D71226: Also synthesize _cmd and self for properties

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1646bb866433: Also synthesize _cmd and self for properties 
(authored by MadCoder, committed by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D71226?vs=232928=232944#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71226

Files:
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/CodeGenObjC/direct-method.m


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -190,3 +190,14 @@
   // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty2]"
   return [r getInt] + [r intProperty] + [r intProperty2];
 }
+
+__attribute__((objc_root_class))
+@interface RootDeclOnly
+@property(direct, readonly) int intProperty;
+@end
+
+int useRootDeclOnly(RootDeclOnly *r) {
+  // CHECK-LABEL: define i32 @useRootDeclOnly
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[RootDeclOnly 
intProperty]"
+  return [r intProperty];
+}
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -2498,6 +2498,9 @@
 // A user declared getter will be synthesize when @synthesize of
 // the property with the same name is seen in the @implementation
 GetterMethod->setPropertyAccessor(true);
+
+  GetterMethod->createImplicitParams(Context,
+ GetterMethod->getClassInterface());
   property->setGetterMethodDecl(GetterMethod);
 
   // Skip setter if property is read-only.
@@ -2569,6 +2572,9 @@
   // A user declared setter will be synthesize when @synthesize of
   // the property with the same name is seen in the @implementation
   SetterMethod->setPropertyAccessor(true);
+
+SetterMethod->createImplicitParams(Context,
+   SetterMethod->getClassInterface());
 property->setSetterMethodDecl(SetterMethod);
   }
   // Add any synthesized methods to the global pool. This allows us to


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -190,3 +190,14 @@
   // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty2]"
   return [r getInt] + [r intProperty] + [r intProperty2];
 }
+
+__attribute__((objc_root_class))
+@interface RootDeclOnly
+@property(direct, readonly) int intProperty;
+@end
+
+int useRootDeclOnly(RootDeclOnly *r) {
+  // CHECK-LABEL: define i32 @useRootDeclOnly
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[RootDeclOnly intProperty]"
+  return [r intProperty];
+}
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -2498,6 +2498,9 @@
 // A user declared getter will be synthesize when @synthesize of
 // the property with the same name is seen in the @implementation
 GetterMethod->setPropertyAccessor(true);
+
+  GetterMethod->createImplicitParams(Context,
+ GetterMethod->getClassInterface());
   property->setGetterMethodDecl(GetterMethod);
 
   // Skip setter if property is read-only.
@@ -2569,6 +2572,9 @@
   // A user declared setter will be synthesize when @synthesize of
   // the property with the same name is seen in the @implementation
   SetterMethod->setPropertyAccessor(true);
+
+SetterMethod->createImplicitParams(Context,
+   SetterMethod->getClassInterface());
 property->setSetterMethodDecl(SetterMethod);
   }
   // Add any synthesized methods to the global pool. This allows us to
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 1646bb8 - Also synthesize _cmd and self for properties

[Alex Lorenz via cfe-commits]

Author: Pierre Habouzit
Date: 2019-12-09T14:30:01-08:00
New Revision: 1646bb86643326db6e220291d5c71c8d616f66fb

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

LOG: Also synthesize _cmd and self for properties

Patch by: Pierre Habouzit

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

Added: 


Modified: 
clang/lib/Sema/SemaObjCProperty.cpp
clang/test/CodeGenObjC/direct-method.m

Removed: 




diff  --git a/clang/lib/Sema/SemaObjCProperty.cpp 
b/clang/lib/Sema/SemaObjCProperty.cpp
index 2d91ea1f5fd0..f6717f4cbe5e 100644
--- a/clang/lib/Sema/SemaObjCProperty.cpp
+++ b/clang/lib/Sema/SemaObjCProperty.cpp
@@ -2498,6 +2498,9 @@ void Sema::ProcessPropertyDecl(ObjCPropertyDecl 
*property) {
 // A user declared getter will be synthesize when @synthesize of
 // the property with the same name is seen in the @implementation
 GetterMethod->setPropertyAccessor(true);
+
+  GetterMethod->createImplicitParams(Context,
+ GetterMethod->getClassInterface());
   property->setGetterMethodDecl(GetterMethod);
 
   // Skip setter if property is read-only.
@@ -2569,6 +2572,9 @@ void Sema::ProcessPropertyDecl(ObjCPropertyDecl 
*property) {
   // A user declared setter will be synthesize when @synthesize of
   // the property with the same name is seen in the @implementation
   SetterMethod->setPropertyAccessor(true);
+
+SetterMethod->createImplicitParams(Context,
+   SetterMethod->getClassInterface());
 property->setSetterMethodDecl(SetterMethod);
   }
   // Add any synthesized methods to the global pool. This allows us to

diff  --git a/clang/test/CodeGenObjC/direct-method.m 
b/clang/test/CodeGenObjC/direct-method.m
index eb5a52eb11a2..373bd22a84cd 100644
--- a/clang/test/CodeGenObjC/direct-method.m
+++ b/clang/test/CodeGenObjC/direct-method.m
@@ -190,3 +190,14 @@ int useRoot(Root *r) {
   // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty2]"
   return [r getInt] + [r intProperty] + [r intProperty2];
 }
+
+__attribute__((objc_root_class))
+@interface RootDeclOnly
+@property(direct, readonly) int intProperty;
+@end
+
+int useRootDeclOnly(RootDeclOnly *r) {
+  // CHECK-LABEL: define i32 @useRootDeclOnly
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[RootDeclOnly 
intProperty]"
+  return [r intProperty];
+}



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

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D71179#1776046 , @ABataev wrote:

> In D71179#1776034 , @jdoerfert wrote:
>
> > > Not always. If we see that the context selector does not match, we can 
> > > skip everything between begin/end. It means exactly what I said - 
> > > multiversioning is needed only for `construct` because all other traits 
> > > can be easily resolved at the compile time. Generally speaking, there are 
> > > 2 kinds of traits - global traits (like `vendor`, `kind`, `isa`, etc.), 
> > > which can be resolved completely statically and do not need 
> > > multiversioning, and local traits, like `construct`, which depend on the 
> > > OpenMP directives and require something similar to the multiversioning.
> >
> > The case where the code is skipped is easy, sure. However, if we "could 
> > easily resolve" the other case, we could have implemented an #ifdef 
> > solution for `math.h/cmath`. This was not the case and still is not. We 
> > basically populate the namespace with multiple versions of the same 
> > function (with the same name) and then select the appropriate one for each 
> > call site.
> >
> > Instead of trying to argue why this is not needed for some cases, could you 
> > argue why we should have multiple schemes to resolve all types of variants? 
> > It seems you inherently assume the codegen patching scheme implemented 
> > right now is useful even if we need something else to complement it. I 
> > don't think so, thus there is little reason for me to distinguish between 
> > the types of variants that need multi-version support ant the types that 
> > can be implemented with multi-versions but don't need it.
>
>
> Because each particular problem requires its own solution and it is always a 
> bad idea to use the microscope to hammer the nails.


While I see where you are coming from, I disagree. We have a generic framework 
available that we already need to use in some cases, there is no harm in using 
it for all cases. It would be different if we wouldn't need the generic 
framework at all, but that is not the case. All I ask is to literally share 
existing code, no additional complexity needed. Your suggestion will complicate 
the setup, duplicate logic, and make it overall harder to maintain and compose 
in the future. If you still disagree, please provide some arguments (and 
details) why we would benefit from your setup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[PATCH] D71225: [OpenMP][WIP] atomic update only evaluate once for expression having side effect

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71225



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

[PATCH] D71225: [OpenMP][WIP] atomic update only evaluate once for expression having side effect

[Chi Chun Chen via Phabricator via cfe-commits]

cchen added a comment.

Oops, accidentally remove my own comment. I'm not sure why `iarr[foo(), foo(), 
0]` violate the rule since it will be evaluated as `iarr[0]`, and the counter 
`foo()` modified is also in the same location for both left hand side and right 
hand side.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71225



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

[PATCH] D71225: [OpenMP][WIP] atomic update only evaluate once for expression having side effect

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

> • For forms that allow multiple occurrences of x, the number of times that x 
> is evaluated is unspecified.

x here is `iarr[foo(), foo(), 0]`, if I'm not mistaken. Assuming not, any 
multiple of 2 is a fine amount of evaluations for `foo`.

I'd vote for a warning here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71225



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

[PATCH] D71141: [Wdocumentation] Use C2x/C++14 deprecated attribute

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang/test/Sema/warn-documentation-fixits.c:27
+
+// CHECK: fix-it:"{{.*}}":{7:1-7:1}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{11:1-11:1}:"[[ATTRIBUTE]] "

Mordante wrote:
> gribozavr2 wrote:
> > "[[ATTRIBUTE]]"?
> > 
> > I expected either "[[deprecated]]" or "ATTRIBUTE".
> > 
> > Oh, these are FileCheck's regex brackets, which make it just a literal 
> > "ATTRIBUTE"... I think it would be clearer without the brackets :)
> These are not simple regex bracket, but they are substitution blocks [1]. 
> These are defined by the RUN scrips to select between the 
> `__attribute__((deprecated)) ` and `[[deprecated]]` replacements.
> 
> [1] 
> https://llvm.org/docs/CommandGuide/FileCheck.html#filecheck-string-substitution-blocks
Oh, it is FileCheck's -D flag... sorry I misread it. Please carry on.


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

https://reviews.llvm.org/D71141



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

[PATCH] D71224: [analyzer][WIP] Escape symbols stored into specific region after a conservative evalcall.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D71224#1776052 , @xazax.hun wrote:

> Also, it looks like for example `CStringChecker` calls 
> `ProgramState::InvalidateRegions` directly when modeling function calls. So 
> it looks like if checkers are using evalCall, pointerEscape will not be 
> triggered automatically. I am not sure if that would be a bug or feature :D


I'd say it's a feature, because `evalCall` is the official way for checkers to 
opt out of babysitting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71224



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

[PATCH] D71140: [Wdocumentation] Properly place deprecated attribute

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang/test/Sema/warn-documentation-fixits.cpp:4
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -Wdocumentation 
-Wdocumentation-pedantic -fcomment-block-commands=foobar 
-fdiagnostics-parseable-fixits %s 2>&1 | FileCheck 
--check-prefixes=CHECK,CHECK14 %s
 
 // expected-warning@+1 {{parameter 'ZZ' not found in the function 
declaration}} expected-note@+1 {{did you mean 'a'?}}

Mordante wrote:
> I added one here, is this what you wanted or one for the RUN at line 1?
This looks good. We're testing the default language mode like every other test, 
and c++11 and c++14 specifically because there's special logic for them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71140



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

[PATCH] D71226: Also synthesize _cmd and self for properties

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Alternatively, we could make IRGen not depend on those parameters being set 
just to call a function, which seems reasonable since the parameter variables 
are logically internal to the implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71226



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

[PATCH] D71226: Also synthesize _cmd and self for properties

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

In D71226#1776038 , @rjmccall wrote:

> It'd be nice if we didn't have to synthesize parameters for all declarations 
> when we're just handling an `@interface`, but we can improve that in 
> follow-ups.


yeah ideally I'd want to do it lazily when we emit the direct call, but at this 
point the Decl is const and that was a huge change to make.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71226



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

[PATCH] D71224: [analyzer][WIP] Escape symbols stored into specific region after a conservative evalcall.

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

xazax.hun added a comment.

Also, it looks like for example `CStringChecker` calls 
`ProgramState::InvalidateRegions` directly when modeling function calls. So it 
looks like if checkers are using evalCall, pointerEscape will not be triggered 
automatically. I am not sure if that would be a bug or feature :D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71224



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

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D71179#1776034 , @jdoerfert wrote:

> > Not always. If we see that the context selector does not match, we can skip 
> > everything between begin/end. It means exactly what I said - 
> > multiversioning is needed only for `construct` because all other traits can 
> > be easily resolved at the compile time. Generally speaking, there are 2 
> > kinds of traits - global traits (like `vendor`, `kind`, `isa`, etc.), which 
> > can be resolved completely statically and do not need multiversioning, and 
> > local traits, like `construct`, which depend on the OpenMP directives and 
> > require something similar to the multiversioning.
>
> The case where the code is skipped is easy, sure. However, if we "could 
> easily resolve" the other case, we could have implemented an #ifdef solution 
> for `math.h/cmath`. This was not the case and still is not. We basically 
> populate the namespace with multiple versions of the same function (with the 
> same name) and then select the appropriate one for each call site.
>
> Instead of trying to argue why this is not needed for some cases, could you 
> argue why we should have multiple schemes to resolve all types of variants? 
> It seems you inherently assume the codegen patching scheme implemented right 
> now is useful even if we need something else to complement it. I don't think 
> so, thus there is little reason for me to distinguish between the types of 
> variants that need multi-version support ant the types that can be 
> implemented with multi-versions but don't need it.


Because each particular problem requires its own solution and it is always a 
bad idea to use the microscope to hammer the nails.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

> Not always. If we see that the context selector does not match, we can skip 
> everything between begin/end. It means exactly what I said - multiversioning 
> is needed only for `construct` because all other traits can be easily 
> resolved at the compile time. Generally speaking, there are 2 kinds of traits 
> - global traits (like `vendor`, `kind`, `isa`, etc.), which can be resolved 
> completely statically and do not need multiversioning, and local traits, like 
> `construct`, which depend on the OpenMP directives and require something 
> similar to the multiversioning.

The case where the code is skipped is easy, sure. However, if we "could easily 
resolve" the other case, we could have implemented an #ifdef solution for 
`math.h/cmath`. This was not the case and still is not. We basically populate 
the namespace with multiple versions of the same function (with the same name) 
and then select the appropriate one for each call site.

Instead of trying to argue why this is not needed for some cases, could you 
argue why we should have multiple schemes to resolve all types of variants? It 
seems you inherently assume the codegen patching scheme implemented right now 
is useful even if we need something else to complement it. I don't think so, 
thus there is little reason for me to distinguish between the types of variants 
that need multi-version support ant the types that can be implemented with 
multi-versions but don't need it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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

[PATCH] D71224: [analyzer][WIP] Escape symbols stored into specific region after a conservative evalcall.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:696
   // And make the result node.
   Bldr.generateNode(Call.getProgramPoint(), State, Pred);
 }

xazax.hun wrote:
> After some offline conversation it is very likely that we want to move the 
> `runCheckersForPointerEscape` here. 
> 
> The main question is, how should we get all the data?
> 
> We should know about:
> * What regions are output params.
> * What regions are considered escaped.
> * What regions have traits that prevents escaping.
> 
> Is there anything else?
> What regions are output params.

That should be obvious from the AST. Like, parameters of non-const 
pointer/reference types.

> What regions are considered escaped.

Output parameter regions (as `TopLevelInvalidated`) and whatever's reachable 
from them.

> What regions have traits that prevents escaping.

Currently the only trait that affects escaping (as opposed to invalidation) is 
`TK_SuppressEscape` and it is never applied to out-parameters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71224



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

[PATCH] D71225: [OpenMP][WIP] atomic update only evaluate once for expression having side effect

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

It is against the standard, I assume. According to standard, `During the 
execution of an atomic region, multiple syntactic occurrences of x must 
designate the same storage location.`. In your case, this is not so. I assume 
it would be better to implement the diagnostic here that the code is not OpenMP 
compliant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71225



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

[PATCH] D71226: Also synthesize _cmd and self for properties

[Alex Lorenz via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71226



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

[PATCH] D71226: Also synthesize _cmd and self for properties

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

It'd be nice if we didn't have to synthesize parameters for all declarations 
when we're just handling an `@interface`, but we can improve that in follow-ups.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71226



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

[PATCH] D71227: [cuda][hip] Fix function overload resolution in the global initiailizer.

[Michael Liao via Phabricator via cfe-commits]

hliao updated this revision to Diff 232938.
hliao added a comment.

refine again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71227

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCUDA/function-overload.cu
  clang/test/SemaCUDA/global-initializers-host.cu
  clang/test/SemaCUDA/hip-pinned-shadow.cu

Index: clang/test/SemaCUDA/hip-pinned-shadow.cu
===
--- clang/test/SemaCUDA/hip-pinned-shadow.cu
+++ clang/test/SemaCUDA/hip-pinned-shadow.cu
@@ -13,13 +13,19 @@
 
 template 
 struct texture : public textureReference {
+// expected-note@-1{{candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 0 were provided}}
+// expected-note@-2{{candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 0 were provided}}
+// expected-note@-3{{candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 0 were provided}}
+// expected-note@-4{{candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 0 were provided}}
 texture() { a = 1; }
+// expected-note@-1{{candidate constructor not viable: call to __host__ function from __device__ function}}
+// expected-note@-2{{candidate constructor not viable: call to __host__ function from __device__ function}}
 };
 
 __hip_pinned_shadow__ texture tex;
 __device__ __hip_pinned_shadow__ texture tex2; // expected-error{{'hip_pinned_shadow' and 'device' attributes are not compatible}}
-// expected-error@-1{{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables}}
-// expected-note@-2{{conflicting attribute is here}}
+// expected-note@-1{{conflicting attribute is here}}
+// expected-error@-2{{no matching constructor for initialization of 'texture'}}
 __constant__ __hip_pinned_shadow__ texture tex3; // expected-error{{'hip_pinned_shadow' and 'constant' attributes are not compatible}}
-  // expected-error@-1{{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables}}
-  // expected-note@-2{{conflicting attribute is here}}
+  // expected-note@-1{{conflicting attribute is here}}
+  // expected-error@-2{{no matching constructor for initialization of 'texture'}}
Index: clang/test/SemaCUDA/global-initializers-host.cu
===
--- clang/test/SemaCUDA/global-initializers-host.cu
+++ clang/test/SemaCUDA/global-initializers-host.cu
@@ -6,12 +6,14 @@
 // module initializer.
 
 struct S {
+  // expected-note@-1 {{candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 0 were provided}}
+  // expected-note@-2 {{candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 0 were provided}}
   __device__ S() {}
-  // expected-note@-1 {{'S' declared here}}
+  // expected-note@-1 {{candidate constructor not viable: call to __device__ function from __host__ function}}
 };
 
 S s;
-// expected-error@-1 {{reference to __device__ function 'S' in global initializer}}
+// expected-error@-1 {{no matching constructor for initialization of 'S'}}
 
 struct T {
   __host__ __device__ T() {}
@@ -19,14 +21,17 @@
 T t;  // No error, this is OK.
 
 struct U {
+  // expected-note@-1 {{candidate constructor (the implicit copy constructor) not viable: no known conversion from 'int' to 'const U' for 1st argument}}
+  // expected-note@-2 {{candidate constructor (the implicit move constructor) not viable: no known conversion from 'int' to 'U' for 1st argument}}
   __host__ U() {}
+  // expected-note@-1 {{candidate constructor not viable: requires 0 arguments, but 1 was provided}}
   __device__ U(int) {}
-  // expected-note@-1 {{'U' declared here}}
+  // expected-note@-1 {{candidate constructor not viable: call to __device__ function from __host__ function}}
 };
 U u(42);
-// expected-error@-1 {{reference to __device__ function 'U' in global initializer}}
+// expected-error@-1 {{no matching constructor for initialization of 'U'}}
 
 __device__ int device_fn() { return 42; }
-// expected-note@-1 {{'device_fn' declared here}}
+// expected-note@-1 {{candidate function not viable: call to __device__ function from 

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Liu Liu via Phabricator via cfe-commits]

liuliu added a comment.

No problem! I have a custom patch that isn't as nice but get the job done. Let 
me know when you put new diff out and I can test it on our codebase again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71091



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

[PATCH] D71226: Also synthesize _cmd and self for properties

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

@liuliu that fixes your test case (which I reproduced in the CG test)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71226



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

[PATCH] D71227: [cuda][hip] Fix function overload resolution in the global initiailizer.

[Michael Liao via Phabricator via cfe-commits]

hliao created this revision.
hliao added reviewers: tra, jlebar, yaxunl.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
hliao updated this revision to Diff 232933.
hliao added a comment.
hliao edited the summary of this revision.

refine commit message


- As global initializers are not under any function body, they needs to look 
into the current variable being initialized. That is not addressed in the 
current CUDA/HIP overloadable function resolution and ignore target checking. 
That may result in wrong candidate to be considered as illustrated in the newly 
added test case.
- In this patch, a non-local varialble stack is introduced to keep trace the 
current non-local variable being initialized so that initialization function 
could be inspected for the target preference.
- Besides newly added tests, existing tests are refined as the current 
implementation adds extra check on global initializers to ensure no device 
functions are used.  As the target match checking is enabled in this patch, 
such check is only necessary for CUDA device global variables. They are not 
allowed to be non-trivially inialized. As HIP starts to support non-trivial 
initialization of device initialization, such target matching check is 
mandatory to be enforced.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71227

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCUDA/function-overload.cu
  clang/test/SemaCUDA/global-initializers-host.cu
  clang/test/SemaCUDA/hip-pinned-shadow.cu

Index: clang/test/SemaCUDA/hip-pinned-shadow.cu
===
--- clang/test/SemaCUDA/hip-pinned-shadow.cu
+++ clang/test/SemaCUDA/hip-pinned-shadow.cu
@@ -13,13 +13,19 @@
 
 template 
 struct texture : public textureReference {
+// expected-note@-1{{candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 0 were provided}}
+// expected-note@-2{{candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 0 were provided}}
+// expected-note@-3{{candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 0 were provided}}
+// expected-note@-4{{candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 0 were provided}}
 texture() { a = 1; }
+// expected-note@-1{{candidate constructor not viable: call to __host__ function from __device__ function}}
+// expected-note@-2{{candidate constructor not viable: call to __host__ function from __device__ function}}
 };
 
 __hip_pinned_shadow__ texture tex;
 __device__ __hip_pinned_shadow__ texture tex2; // expected-error{{'hip_pinned_shadow' and 'device' attributes are not compatible}}
-// expected-error@-1{{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables}}
-// expected-note@-2{{conflicting attribute is here}}
+// expected-note@-1{{conflicting attribute is here}}
+// expected-error@-2{{no matching constructor for initialization of 'texture'}}
 __constant__ __hip_pinned_shadow__ texture tex3; // expected-error{{'hip_pinned_shadow' and 'constant' attributes are not compatible}}
-  // expected-error@-1{{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables}}
-  // expected-note@-2{{conflicting attribute is here}}
+  // expected-note@-1{{conflicting attribute is here}}
+  // expected-error@-2{{no matching constructor for initialization of 'texture'}}
Index: clang/test/SemaCUDA/global-initializers-host.cu
===
--- clang/test/SemaCUDA/global-initializers-host.cu
+++ clang/test/SemaCUDA/global-initializers-host.cu
@@ -6,12 +6,14 @@
 // module initializer.
 
 struct S {
+  // expected-note@-1 {{candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 0 were provided}}
+  // expected-note@-2 {{candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 0 were provided}}
   __device__ S() {}
-  // expected-note@-1 {{'S' declared here}}
+  // expected-note@-1 {{candidate constructor not viable: call to __device__ function from __host__ function}}
 };
 
 S s;
-// expected-error@-1 {{reference to __device__ function 'S' in global initializer}}
+// 

[PATCH] D71225: [OpenMP][WIP] atomic update only evaluate once for expression having side effect

[Chi Chun Chen via Phabricator via cfe-commits]

cchen created this revision.
cchen added a reviewer: ABataev.
Herald added subscribers: cfe-commits, jfb, guansong.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.
cchen edited the summary of this revision.
cchen added a project: OpenMP.
cchen added subscribers: dreachem, sandoval.

This patch is for pointing out that we might need to check if the
expression in atomic update having any side effect. In the past we
always only evaluate an expression once, for example:

  int cnt = 0;
  int foo() {
return ++cnt;
  }
  
  void bar() {
int iarr[100];
iarr[foo(), foo(), 0]  = iarr[foo(), foo(), 0] + 1;
  }

For now, the result of cnt is 2, however, we probably want cnt to be 4
or we might want to tell user that we only evaluate the expression once
as diagnosis message.

Actually, `atomic capture` have the exact same issue, but the purpose of
this patch to expose the issue. After the discussion, I might send another
patch to fix the issue.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71225

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/atomic_update_codegen.cpp


Index: clang/test/OpenMP/atomic_update_codegen.cpp
===
--- clang/test/OpenMP/atomic_update_codegen.cpp
+++ clang/test/OpenMP/atomic_update_codegen.cpp
@@ -27,6 +27,12 @@
 _Complex int civ, cix;
 _Complex float cfv, cfx;
 _Complex double cdv, cdx;
+int iarr[100];
+int cnt = 0;
+
+int foo() {
+  return cnt++;
+}
 
 typedef int int4 __attribute__((__vector_size__(16)));
 int4 int4x;
@@ -900,6 +906,17 @@
 // CHECK: call{{.*}} @__kmpc_flush(
 #pragma omp atomic seq_cst
   rix = dv / rix;
+
+#pragma omp atomic update
+  iarr[foo(), 0]  = iarr[foo(), 0] + 1;
+// CHECK: call i32 @foo()
+// CHECK: call i32 @foo()
+// CHECK: [[ZERO:%.+]] = atomicrmw add i32* [[ARRAY_IDX:.+]], i32 1 monotonic
+
+#pragma omp atomic update
+  iarr[foo(), foo(), 0]  = iarr[foo(), foo(), 0] + 1;
+// CHECK-COUNT-4: call i32 @foo()
+// CHECK: [[ZERO:%.+]] = atomicrmw add i32* [[ARRAY_IDX:.+]], i32 1 monotonic
   return 0;
 }
 
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -3946,6 +3946,30 @@
   return Res;
 }
 
+static void traverseBinOp(CodeGenFunction , const Expr *E) {
+  if (auto *BinOp = dyn_cast(E)) {
+traverseBinOp(CGF, BinOp->getLHS());
+traverseBinOp(CGF, BinOp->getRHS());
+  } else {
+if (auto *Call = dyn_cast(E)) {
+  CGF.EmitCallExpr(Call);
+}
+  }
+}
+
+static void handleArraySubscriptSideEffect(CodeGenFunction ,
+   const Expr *X) {
+  // If X is a ArraySubscript and the expression has side effect, then we'll
+  // have wrong result since we only evaluate the expression once.
+  if (X->getStmtClass() == Expr::ArraySubscriptExprClass) {
+auto E = cast(X);
+if (E->getLHS() != E->getIdx()) {
+  assert(E->getRHS() == E->getIdx() && "index was neither LHS nor RHS");
+  traverseBinOp(CGF, E->getIdx());
+}
+  }
+}
+
 static void emitOMPAtomicUpdateExpr(CodeGenFunction , bool IsSeqCst,
 const Expr *X, const Expr *E,
 const Expr *UE, bool IsXLHSInRHSPart,
@@ -3960,6 +3984,7 @@
   // x = x binop expr; -> xrval binop expr
   // x = expr Op x; - > expr binop xrval;
   assert(X->isLValue() && "X of 'omp atomic update' is not lvalue");
+  handleArraySubscriptSideEffect(CGF, X);
   LValue XLValue = CGF.EmitLValue(X);
   RValue ExprRValue = CGF.EmitAnyExpr(E);
   llvm::AtomicOrdering AO = IsSeqCst


Index: clang/test/OpenMP/atomic_update_codegen.cpp
===
--- clang/test/OpenMP/atomic_update_codegen.cpp
+++ clang/test/OpenMP/atomic_update_codegen.cpp
@@ -27,6 +27,12 @@
 _Complex int civ, cix;
 _Complex float cfv, cfx;
 _Complex double cdv, cdx;
+int iarr[100];
+int cnt = 0;
+
+int foo() {
+  return cnt++;
+}
 
 typedef int int4 __attribute__((__vector_size__(16)));
 int4 int4x;
@@ -900,6 +906,17 @@
 // CHECK: call{{.*}} @__kmpc_flush(
 #pragma omp atomic seq_cst
   rix = dv / rix;
+
+#pragma omp atomic update
+  iarr[foo(), 0]  = iarr[foo(), 0] + 1;
+// CHECK: call i32 @foo()
+// CHECK: call i32 @foo()
+// CHECK: [[ZERO:%.+]] = atomicrmw add i32* [[ARRAY_IDX:.+]], i32 1 monotonic
+
+#pragma omp atomic update
+  iarr[foo(), foo(), 0]  = iarr[foo(), foo(), 0] + 1;
+// CHECK-COUNT-4: call i32 @foo()
+// CHECK: [[ZERO:%.+]] = atomicrmw add i32* [[ARRAY_IDX:.+]], i32 1 monotonic
   return 0;
 }
 
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -3946,6 +3946,30 @@
   return Res;
 }
 
+static void traverseBinOp(CodeGenFunction , const Expr *E) {
+  if (auto *BinOp =