[PATCH] D108119: Wiring of standard library indexing into clangd.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(First just the bug that I suspect is blocking you from testing this, design 
feedback to come)




Comment at: clang-tools-extra/clangd/ClangdServer.cpp:184
+}
+AddIndex(SLIndex->get());
+  }

this pointer will dangle after the `if` block finishes (the local `auto` owns 
the index)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108119

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

[PATCH] D107450: [clang-tidy] Fix wrong FIxIt in performance-move-const-arg

[gehry via Phabricator via cfe-commits]

Sockke updated this revision to Diff 366815.
Sockke added a comment.

update! Fixed some diagnostic descriptions.


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

https://reviews.llvm.org/D107450

Files:
  clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
  clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.h
  clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp
@@ -246,3 +246,27 @@
   };
   f(MoveSemantics());
 }
+
+void showInt(int &&) {}
+void testInt() {
+  int a = 10;
+  showInt(std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect; consider changing showInt's parameter from 'int &&' to 'int'
+}
+template 
+void forwardToShowInt(T &&t) {
+  showInt(static_cast(t));
+}
+void testTemplate() {
+  int a = 10;
+  forwardToShowInt(std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: std::move of the variable 'a' of the trivially-copyable type 'int' has no effect
+}
+
+struct Tmp {};
+void showTmp(Tmp &&) {}
+void testTmp() {
+  Tmp t;
+  showTmp(std::move(t));
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: std::move of the variable 't' of the trivially-copyable type 'Tmp' has no effect
+}
Index: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.h
===
--- clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.h
+++ clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MOVECONSTANTARGUMENTCHECK_H
 
 #include "../ClangTidyCheck.h"
+#include "llvm/ADT/DenseSet.h"
 
 namespace clang {
 namespace tidy {
@@ -36,6 +37,7 @@
 
 private:
   const bool CheckTriviallyCopyableMove;
+  llvm::DenseSet HasCheckedMoveSet;
 };
 
 } // namespace performance
Index: clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
===
--- clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
+++ clang-tools-extra/clang-tidy/performance/MoveConstArgCheck.cpp
@@ -50,7 +50,9 @@
   Finder->addMatcher(
   invocation(forEachArgumentWithParam(
  MoveCallMatcher,
- parmVarDecl(hasType(references(isConstQualified())
+ parmVarDecl(anyOf(hasType(references(isConstQualified())),
+   hasType(rValueReferenceType(
+ .bind("invocation-parm")))
   .bind("receiving-expr"),
   this);
 }
@@ -58,6 +60,15 @@
 void MoveConstArgCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *CallMove = Result.Nodes.getNodeAs("call-move");
   const auto *ReceivingExpr = Result.Nodes.getNodeAs("receiving-expr");
+  const auto *InvocationParm =
+  Result.Nodes.getNodeAs("invocation-parm");
+
+  if (!ReceivingExpr && HasCheckedMoveSet.contains(CallMove))
+return;
+
+  if (ReceivingExpr)
+HasCheckedMoveSet.insert(CallMove);
+
   const Expr *Arg = CallMove->getArg(0);
   SourceManager &SM = Result.Context->getSourceManager();
 
@@ -90,20 +101,52 @@
   return;
 
 bool IsVariable = isa(Arg);
+bool IsRValueReferenceArg = false;
+bool IsTemplateInstantiation = false;
+StringRef FuncName;
+QualType ParmType;
 const auto *Var =
 IsVariable ? dyn_cast(Arg)->getDecl() : nullptr;
-auto Diag = diag(FileMoveRange.getBegin(),
- "std::move of the %select{|const }0"
- "%select{expression|variable %4}1 "
- "%select{|of the trivially-copyable type %5 }2"
- "has no effect; remove std::move()"
- "%select{| or make the variable non-const}3")
-<< IsConstArg << IsVariable << IsTriviallyCopyable
-<< (IsConstArg && IsVariable && !IsTriviallyCopyable) << Var
-<< Arg->getType();
+
+if (ReceivingExpr &&
+InvocationParm->getOriginalType()->isRValueReferenceType() &&
+!ReceivingExpr->getType()->isRecordType() && Arg->isLValue()) {
+  IsRValueReferenceArg = true;
+  if (Arg->getType()->isBuiltinType()) {
+const auto *ReceivingCallExpr = dyn_cast(ReceivingExpr);
+if (!ReceivingCallExpr)
+  return;
+IsTemplateInstantiation =
+ReceivingCallExpr->getDirectCallee()->isTemplateInstantiation();
+FuncName = ReceivingCallExpr->getDirectCallee()->getName();
+ParmType = InvocationParm->getOriginalType();
+  }
+}
+
+auto Diag =
+diag(FileMoveRange.getBegin(),
+ 

[PATCH] D108173: [WIP][clang] Add a cmake flag for choosing to build clang-repl

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

Rather than hardcoding this only for `clang-repl`, I'd prefer to use a more 
generic approach.

We already have `LLVM_DISTRIBUTION_COMPONENTS` so one possibility would be to 
set a lit feature for every component. For example, if 
`LLVM_DISTRIBUTION_COMPONENTS=foo;bar` we would set 
`foo-distribution-component` and `bar-distribution-component`. The challenge is 
handling the default case where you want to build and test everything. We might 
be able to use CMake properties for that purpose to get the list of all tools 
and define the corresponding features.

Alternative would be to iterate over the `CLANG_TEST_DEPS` list and check if 
each entry is in `LLVM_DISTRIBUTION_COMPONENTS`, and if so add it to the list 
of dependencies and set a feature for it. If `LLVM_DISTRIBUTION_COMPONENTS` is 
empty, then include everything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108173

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

[PATCH] D105426: [clangd] IncludeCleaner as a library: Find all references to symbols in the file

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Remove findReferencedFiles (to be introduced in the next patch) to preserve
proposed granularity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105426

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -0,0 +1,136 @@
+//===--- IncludeCleanerTests.cpp *- C++ -*-===//
+//
+// 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 "Annotations.h"
+#include "IncludeCleaner.h"
+#include "TestTU.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(IWYU, ReferencedLocations) {
+  struct TestCase {
+std::string HeaderCode;
+std::string MainCode;
+  };
+  TestCase Cases[] = {
+  // DeclRefExpr
+  {
+  "int ^x();",
+  "int y = x();",
+  },
+  // RecordDecl
+  {
+  "class ^X;",
+  "X *y;",
+  },
+  // TypedefType and UsingDecls
+  {
+  "using ^Integer = int;",
+  "Integer x;",
+  },
+  {
+  "namespace ns { struct ^X; struct ^X {}; }",
+  "using ns::X;",
+  },
+  {
+  "namespace ns { struct X; struct X {}; }",
+  "using namespace ns;",
+  },
+  {
+  "struct ^A {}; using B = A; using ^C = B;",
+  "C a;",
+  },
+  {
+  "typedef bool ^Y; template  struct ^X {};",
+  "X x;",
+  },
+  {
+  "struct Foo; struct ^Foo{}; typedef Foo ^Bar;",
+  "Bar b;",
+  },
+  // MemberExpr
+  {
+  "struct ^X{int ^a;}; X ^foo();",
+  "int y = foo().a;",
+  },
+  // Expr (type is traversed)
+  {
+  "class ^X{}; X ^foo();",
+  "auto bar() { return foo(); }",
+  },
+  // Redecls
+  {
+  "class ^X; class ^X{}; class ^X;",
+  "X *y;",
+  },
+  // Constructor
+  {
+  "struct ^X { ^X(int) {} int ^foo(); };",
+  "auto x = X(42); auto y = x.foo();",
+  },
+  // Static function
+  {
+  "struct ^X { static bool ^foo(); }; bool X::^foo() {}",
+  "auto b = X::foo();",
+  },
+  // TemplateRecordDecl
+  {
+  "template  class ^X;",
+  "X *y;",
+  },
+  // Type name not spelled out in code
+  {
+  "class ^X{}; X ^getX();",
+  "auto x = getX();",
+  },
+  // Enums
+  {
+  "enum ^Color { ^Red = 42, Green = 9000};",
+  "int MyColor = Red;",
+  },
+  {
+  "struct ^X { enum ^Language { ^CXX = 42, Python = 9000}; };",
+  "int Lang = X::CXX;",
+  },
+  {
+  // When a type is resolved via a using declaration, the
+  // UsingShadowDecl is not referenced in the AST.
+  // Compare to TypedefType, or DeclRefExpr::getFoundDecl().
+  // ^
+  "namespace ns { class ^X; }; using ns::X;",
+  "X *y;",
+  }};
+  for (const TestCase &T : Cases) {
+TestTU TU;
+TU.Code = T.MainCode;
+Annotations Header(T.HeaderCode);
+TU.HeaderCode = Header.code().str();
+auto AST = TU.build();
+
+std::vector Points;
+for (const auto &Loc : findReferencedLocations(AST)) {
+  if (AST.getSourceManager().getBufferName(Loc).endswith(
+  TU.HeaderFilename)) {
+Points.push_back(offsetToPosition(
+TU.HeaderCode, AST.getSourceManager().getFileOffset(Loc)));
+  }
+}
+llvm::sort(Points);
+
+EXPECT_EQ(Points, Header.points()) << T.HeaderCode << "\n---\n"
+   << T.MainCode;
+  }
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -58,6 +58,7 @@
   HeadersTests.cpp
   HeaderSourceSwitchTests.cpp
   HoverTests.cpp
+  IncludeCleanerTests.cpp
   IndexActionTests.cpp
   IndexTests.cpp
   InlayHintTests.cpp
Index: clang-tools-extra/clangd/IncludeCleaner.h

[PATCH] D105426: [clangd] IncludeCleaner as a library: Find all references to symbols in the file

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Fix tests name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105426

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -0,0 +1,136 @@
+//===--- IncludeCleanerTests.cpp *- C++ -*-===//
+//
+// 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 "Annotations.h"
+#include "IncludeCleaner.h"
+#include "TestTU.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(IncludeCleaner, ReferencedLocations) {
+  struct TestCase {
+std::string HeaderCode;
+std::string MainCode;
+  };
+  TestCase Cases[] = {
+  // DeclRefExpr
+  {
+  "int ^x();",
+  "int y = x();",
+  },
+  // RecordDecl
+  {
+  "class ^X;",
+  "X *y;",
+  },
+  // TypedefType and UsingDecls
+  {
+  "using ^Integer = int;",
+  "Integer x;",
+  },
+  {
+  "namespace ns { struct ^X; struct ^X {}; }",
+  "using ns::X;",
+  },
+  {
+  "namespace ns { struct X; struct X {}; }",
+  "using namespace ns;",
+  },
+  {
+  "struct ^A {}; using B = A; using ^C = B;",
+  "C a;",
+  },
+  {
+  "typedef bool ^Y; template  struct ^X {};",
+  "X x;",
+  },
+  {
+  "struct Foo; struct ^Foo{}; typedef Foo ^Bar;",
+  "Bar b;",
+  },
+  // MemberExpr
+  {
+  "struct ^X{int ^a;}; X ^foo();",
+  "int y = foo().a;",
+  },
+  // Expr (type is traversed)
+  {
+  "class ^X{}; X ^foo();",
+  "auto bar() { return foo(); }",
+  },
+  // Redecls
+  {
+  "class ^X; class ^X{}; class ^X;",
+  "X *y;",
+  },
+  // Constructor
+  {
+  "struct ^X { ^X(int) {} int ^foo(); };",
+  "auto x = X(42); auto y = x.foo();",
+  },
+  // Static function
+  {
+  "struct ^X { static bool ^foo(); }; bool X::^foo() {}",
+  "auto b = X::foo();",
+  },
+  // TemplateRecordDecl
+  {
+  "template  class ^X;",
+  "X *y;",
+  },
+  // Type name not spelled out in code
+  {
+  "class ^X{}; X ^getX();",
+  "auto x = getX();",
+  },
+  // Enums
+  {
+  "enum ^Color { ^Red = 42, Green = 9000};",
+  "int MyColor = Red;",
+  },
+  {
+  "struct ^X { enum ^Language { ^CXX = 42, Python = 9000}; };",
+  "int Lang = X::CXX;",
+  },
+  {
+  // When a type is resolved via a using declaration, the
+  // UsingShadowDecl is not referenced in the AST.
+  // Compare to TypedefType, or DeclRefExpr::getFoundDecl().
+  // ^
+  "namespace ns { class ^X; }; using ns::X;",
+  "X *y;",
+  }};
+  for (const TestCase &T : Cases) {
+TestTU TU;
+TU.Code = T.MainCode;
+Annotations Header(T.HeaderCode);
+TU.HeaderCode = Header.code().str();
+auto AST = TU.build();
+
+std::vector Points;
+for (const auto &Loc : findReferencedLocations(AST)) {
+  if (AST.getSourceManager().getBufferName(Loc).endswith(
+  TU.HeaderFilename)) {
+Points.push_back(offsetToPosition(
+TU.HeaderCode, AST.getSourceManager().getFileOffset(Loc)));
+  }
+}
+llvm::sort(Points);
+
+EXPECT_EQ(Points, Header.points()) << T.HeaderCode << "\n---\n"
+   << T.MainCode;
+  }
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -58,6 +58,7 @@
   HeadersTests.cpp
   HeaderSourceSwitchTests.cpp
   HoverTests.cpp
+  IncludeCleanerTests.cpp
   IndexActionTests.cpp
   IndexTests.cpp
   InlayHintTests.cpp
Index: clang-tools-extra/clangd/IncludeCleaner.h
===
--- /dev/null
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -0,0 

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

[Yang Haonan via Phabricator via cfe-commits]

haonanya updated this revision to Diff 366829.
haonanya added a comment.

Add the new builtins to clang/lib/Sema/OpenCLBuiltins.td.


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

https://reviews.llvm.org/D106343

Files:
  clang/lib/Headers/opencl-c-base.h
  clang/lib/Headers/opencl-c.h
  clang/lib/Sema/OpenCLBuiltins.td
  clang/test/Headers/opencl-c-header.cl
  clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl

Index: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -7,6 +7,8 @@
 // RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CLC++ -fdeclare-opencl-builtins -DNO_HEADER
 // RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CLC++ -fdeclare-opencl-builtins -finclude-default-header
 // RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CL2.0 -fdeclare-opencl-builtins -finclude-default-header -cl-ext=-cl_khr_fp64 -DNO_FP64
+// RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CL3.0 -fdeclare-opencl-builtins -DNO_HEADER
+// RUN: %clang_cc1 %s -triple spir -verify -pedantic -Wconversion -Werror -fsyntax-only -cl-std=CL3.0 -fdeclare-opencl-builtins -finclude-default-header
 
 // Test the -fdeclare-opencl-builtins option.  This is not a completeness
 // test, so it should not test for all builtins defined by OpenCL.  Instead
@@ -122,6 +124,46 @@
 }
 #endif
 
+#if !defined(NO_HEADER) && __OPENCL_C_VERSION__ == 300
+// Check added atomic_fetch_ functions by cl_ext_float_atomics
+// extension can be called
+void test_atomic_fetch_with_address_space(volatile __generic atomic_float *a_float,
+  volatile __generic atomic_double *a_double,
+  volatile __local atomic_float *a_float_local,
+  volatile __local atomic_double *a_double_local,
+  volatile __global atomic_float *a_float_global,
+  volatile __global atomic_double *a_double_global) {
+  float f1, resf1;
+  double d1, resd1;
+
+  resf1 = atomic_fetch_min(a_float, f1);
+  resf1 = atomic_fetch_max(a_float, f1);
+  resf1 = atomic_fetch_add(a_float, f1);
+  resf1 = atomic_fetch_sub(a_float, f1);
+  resf1 = atomic_fetch_min_explicit(a_float_local, f1, memory_order_seq_cst);
+  resf1 = atomic_fetch_max_explicit(a_float_local, f1, memory_order_seq_cst);
+  resf1 = atomic_fetch_add_explicit(a_float_local, f1, memory_order_seq_cst);
+  resf1 = atomic_fetch_sub_explicit(a_float_local, f1, memory_order_seq_cst);
+  resf1 = atomic_fetch_min_explicit(a_float_global, f1, memory_order_seq_cst, memory_scope_work_group);
+  resf1 = atomic_fetch_max_explicit(a_float_global, f1, memory_order_seq_cst, memory_scope_work_group);
+  resf1 = atomic_fetch_add_explicit(a_float_global, f1, memory_order_seq_cst, memory_scope_work_group);
+  resf1 = atomic_fetch_sub_explicit(a_float_global, f1, memory_order_seq_cst, memory_scope_work_group);
+
+  resd1 = atomic_fetch_min(a_double, d1);
+  resd1 = atomic_fetch_max(a_double, d1);
+  resd1 = atomic_fetch_add(a_double, d1);
+  resd1 = atomic_fetch_sub(a_double, d1);
+  resd1 = atomic_fetch_min_explicit(a_double_local, d1, memory_order_seq_cst);
+  resd1 = atomic_fetch_max_explicit(a_double_local, d1, memory_order_seq_cst);
+  resd1 = atomic_fetch_add_explicit(a_double_local, d1, memory_order_seq_cst);
+  resd1 = atomic_fetch_sub_explicit(a_double_local, d1, memory_order_seq_cst);
+  resd1 = atomic_fetch_min_explicit(a_double_global, d1, memory_order_seq_cst, memory_scope_work_group);
+  resd1 = atomic_fetch_max_explicit(a_double_global, d1, memory_order_seq_cst, memory_scope_work_group);
+  resd1 = atomic_fetch_add_explicit(a_double_global, d1, memory_order_seq_cst, memory_scope_work_group);
+  resd1 = atomic_fetch_sub_explicit(a_double_global, d1, memory_order_seq_cst, memory_scope_work_group);
+}
+#endif // !defined(NO_HEADER) && __OPENCL_C_VERSION__ == 300
+
 // Test old atomic overloaded with generic address space in C++ for OpenCL.
 #if __OPENCL_C_VERSION__ >= 200
 void test_legacy_atomics_cpp(__generic volatile unsigned int *a) {
Index: clang/test/Headers/opencl-c-header.cl
===
--- clang/test/Headers/opencl-c-header.cl
+++ clang/test/Headers/opencl-c-header.cl
@@ -135,6 +135,36 @@
 #if __opencl_c_integer_dot_product_input_4x8bit_packed != 1
 #error "Incorrectly defined __opencl_c_integer_dot_product_input_4x8bit_packed"
 #endif
+#if __opencl_c_ext_fp16_global_atomic_load_store != 1
+#error "Incorrectly defined __opencl_c_ext_fp16_global_atomic_load_store"
+#endif
+#if __opencl_c_ext_fp16_local_atomic_load_store

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

[Yang Haonan via Phabricator via cfe-commits]

haonanya added a comment.

Hi, Anastasia and svenvh. 
Sorry for late reply. I have updated patch per your comments.
Many thanks for your comments.


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

https://reviews.llvm.org/D106343

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

[PATCH] D107366: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

[Valeriy Savchenko via Phabricator via cfe-commits]

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

Now it looks good!  Thanks again!


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

https://reviews.llvm.org/D107366

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

[PATCH] D105426: [clangd] IncludeCleaner as a library: Find all references to symbols in the file

[Kirill Bobyrev via Phabricator via cfe-commits]

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

[clangd] IncludeCleaner: Mark used headers.

Follow-up on D105426 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105426

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -0,0 +1,136 @@
+//===--- IncludeCleanerTests.cpp *- C++ -*-===//
+//
+// 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 "Annotations.h"
+#include "IncludeCleaner.h"
+#include "TestTU.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(IncludeCleaner, ReferencedLocations) {
+  struct TestCase {
+std::string HeaderCode;
+std::string MainCode;
+  };
+  TestCase Cases[] = {
+  // DeclRefExpr
+  {
+  "int ^x();",
+  "int y = x();",
+  },
+  // RecordDecl
+  {
+  "class ^X;",
+  "X *y;",
+  },
+  // TypedefType and UsingDecls
+  {
+  "using ^Integer = int;",
+  "Integer x;",
+  },
+  {
+  "namespace ns { struct ^X; struct ^X {}; }",
+  "using ns::X;",
+  },
+  {
+  "namespace ns { struct X; struct X {}; }",
+  "using namespace ns;",
+  },
+  {
+  "struct ^A {}; using B = A; using ^C = B;",
+  "C a;",
+  },
+  {
+  "typedef bool ^Y; template  struct ^X {};",
+  "X x;",
+  },
+  {
+  "struct Foo; struct ^Foo{}; typedef Foo ^Bar;",
+  "Bar b;",
+  },
+  // MemberExpr
+  {
+  "struct ^X{int ^a;}; X ^foo();",
+  "int y = foo().a;",
+  },
+  // Expr (type is traversed)
+  {
+  "class ^X{}; X ^foo();",
+  "auto bar() { return foo(); }",
+  },
+  // Redecls
+  {
+  "class ^X; class ^X{}; class ^X;",
+  "X *y;",
+  },
+  // Constructor
+  {
+  "struct ^X { ^X(int) {} int ^foo(); };",
+  "auto x = X(42); auto y = x.foo();",
+  },
+  // Static function
+  {
+  "struct ^X { static bool ^foo(); }; bool X::^foo() {}",
+  "auto b = X::foo();",
+  },
+  // TemplateRecordDecl
+  {
+  "template  class ^X;",
+  "X *y;",
+  },
+  // Type name not spelled out in code
+  {
+  "class ^X{}; X ^getX();",
+  "auto x = getX();",
+  },
+  // Enums
+  {
+  "enum ^Color { ^Red = 42, Green = 9000};",
+  "int MyColor = Red;",
+  },
+  {
+  "struct ^X { enum ^Language { ^CXX = 42, Python = 9000}; };",
+  "int Lang = X::CXX;",
+  },
+  {
+  // When a type is resolved via a using declaration, the
+  // UsingShadowDecl is not referenced in the AST.
+  // Compare to TypedefType, or DeclRefExpr::getFoundDecl().
+  // ^
+  "namespace ns { class ^X; }; using ns::X;",
+  "X *y;",
+  }};
+  for (const TestCase &T : Cases) {
+TestTU TU;
+TU.Code = T.MainCode;
+Annotations Header(T.HeaderCode);
+TU.HeaderCode = Header.code().str();
+auto AST = TU.build();
+
+std::vector Points;
+for (const auto &Loc : findReferencedLocations(AST)) {
+  if (AST.getSourceManager().getBufferName(Loc).endswith(
+  TU.HeaderFilename)) {
+Points.push_back(offsetToPosition(
+TU.HeaderCode, AST.getSourceManager().getFileOffset(Loc)));
+  }
+}
+llvm::sort(Points);
+
+EXPECT_EQ(Points, Header.points()) << T.HeaderCode << "\n---\n"
+   << T.MainCode;
+  }
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -58,6 +58,7 @@
   HeadersTests.cpp
   HeaderSourceSwitchTests.cpp
   HoverTests.cpp
+  IncludeCleanerTests.cpp
   Inde

[PATCH] D105426: [clangd] IncludeCleaner as a library: Find all references to symbols in the file

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Push back to the origin.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105426

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -0,0 +1,136 @@
+//===--- IncludeCleanerTests.cpp *- C++ -*-===//
+//
+// 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 "Annotations.h"
+#include "IncludeCleaner.h"
+#include "TestTU.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(IncludeCleaner, ReferencedLocations) {
+  struct TestCase {
+std::string HeaderCode;
+std::string MainCode;
+  };
+  TestCase Cases[] = {
+  // DeclRefExpr
+  {
+  "int ^x();",
+  "int y = x();",
+  },
+  // RecordDecl
+  {
+  "class ^X;",
+  "X *y;",
+  },
+  // TypedefType and UsingDecls
+  {
+  "using ^Integer = int;",
+  "Integer x;",
+  },
+  {
+  "namespace ns { struct ^X; struct ^X {}; }",
+  "using ns::X;",
+  },
+  {
+  "namespace ns { struct X; struct X {}; }",
+  "using namespace ns;",
+  },
+  {
+  "struct ^A {}; using B = A; using ^C = B;",
+  "C a;",
+  },
+  {
+  "typedef bool ^Y; template  struct ^X {};",
+  "X x;",
+  },
+  {
+  "struct Foo; struct ^Foo{}; typedef Foo ^Bar;",
+  "Bar b;",
+  },
+  // MemberExpr
+  {
+  "struct ^X{int ^a;}; X ^foo();",
+  "int y = foo().a;",
+  },
+  // Expr (type is traversed)
+  {
+  "class ^X{}; X ^foo();",
+  "auto bar() { return foo(); }",
+  },
+  // Redecls
+  {
+  "class ^X; class ^X{}; class ^X;",
+  "X *y;",
+  },
+  // Constructor
+  {
+  "struct ^X { ^X(int) {} int ^foo(); };",
+  "auto x = X(42); auto y = x.foo();",
+  },
+  // Static function
+  {
+  "struct ^X { static bool ^foo(); }; bool X::^foo() {}",
+  "auto b = X::foo();",
+  },
+  // TemplateRecordDecl
+  {
+  "template  class ^X;",
+  "X *y;",
+  },
+  // Type name not spelled out in code
+  {
+  "class ^X{}; X ^getX();",
+  "auto x = getX();",
+  },
+  // Enums
+  {
+  "enum ^Color { ^Red = 42, Green = 9000};",
+  "int MyColor = Red;",
+  },
+  {
+  "struct ^X { enum ^Language { ^CXX = 42, Python = 9000}; };",
+  "int Lang = X::CXX;",
+  },
+  {
+  // When a type is resolved via a using declaration, the
+  // UsingShadowDecl is not referenced in the AST.
+  // Compare to TypedefType, or DeclRefExpr::getFoundDecl().
+  // ^
+  "namespace ns { class ^X; }; using ns::X;",
+  "X *y;",
+  }};
+  for (const TestCase &T : Cases) {
+TestTU TU;
+TU.Code = T.MainCode;
+Annotations Header(T.HeaderCode);
+TU.HeaderCode = Header.code().str();
+auto AST = TU.build();
+
+std::vector Points;
+for (const auto &Loc : findReferencedLocations(AST)) {
+  if (AST.getSourceManager().getBufferName(Loc).endswith(
+  TU.HeaderFilename)) {
+Points.push_back(offsetToPosition(
+TU.HeaderCode, AST.getSourceManager().getFileOffset(Loc)));
+  }
+}
+llvm::sort(Points);
+
+EXPECT_EQ(Points, Header.points()) << T.HeaderCode << "\n---\n"
+   << T.MainCode;
+  }
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -58,6 +58,7 @@
   HeadersTests.cpp
   HeaderSourceSwitchTests.cpp
   HoverTests.cpp
+  IncludeCleanerTests.cpp
   IndexActionTests.cpp
   IndexTests.cpp
   InlayHintTests.cpp
Index: clang-tools-extra/clangd/IncludeCleaner.h
===
--- /dev/null
+++ clang-tools-extra/clangd/IncludeCleaner.h

[PATCH] D106876: Remove non-affecting module maps from PCM files.

[Ilya Kuteev via Phabricator via cfe-commits]

ilyakuteev added a comment.

Ping


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

https://reviews.llvm.org/D106876

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

[PATCH] D108194: [clangd] IncludeCleaner: Mark used headers

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman, mgrang.
kbobyrev requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Follow-p on D105426 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108194

Files:
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h

Index: clang-tools-extra/clangd/ParsedAST.h
===
--- clang-tools-extra/clangd/ParsedAST.h
+++ clang-tools-extra/clangd/ParsedAST.h
@@ -116,6 +116,8 @@
 return Resolver.get();
   }
 
+  void computeUsedIncludes();
+
 private:
   ParsedAST(llvm::StringRef Version,
 std::shared_ptr Preamble,
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -18,6 +18,7 @@
 #include "Features.h"
 #include "Headers.h"
 #include "HeuristicResolver.h"
+#include "IncludeCleaner.h"
 #include "IncludeFixer.h"
 #include "Preamble.h"
 #include "SourceCode.h"
@@ -601,5 +602,25 @@
 return llvm::None;
   return llvm::StringRef(Preamble->Version);
 }
+
+void ParsedAST::computeUsedIncludes() {
+  const auto &SM = getSourceManager();
+
+  auto Refs = findReferencedLocations(*this);
+  auto ReferencedFileIDs = findReferencedFiles(Refs, getSourceManager());
+  std::vector ReferencedFilenames;
+  ReferencedFilenames.reserve(ReferencedFileIDs.size());
+  for (FileID FID : ReferencedFileIDs) {
+const FileEntry *FE = SM.getFileEntryForID(FID);
+if (!FE) {
+  elog("Missing FE for {0}", SM.getComposedLoc(FID, 0).printToString(SM));
+  continue;
+}
+ReferencedFilenames.push_back(SM.getFileEntryForID(FID)->getName());
+  }
+  Includes.markUsed(SM.getFileEntryForID(SM.getMainFileID())->getName(),
+ReferencedFilenames, directlyReferencedFiles);
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/IncludeCleaner.h
===
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -46,6 +46,22 @@
 /// - err on the side of reporting all possible locations
 ReferencedLocations findReferencedLocations(ParsedAST &AST);
 
+/// Retrieves IDs of all files containing SourceLocations from \p Locs. Those
+/// locations could be within macro expansions and are not resolved to their
+/// spelling locations.
+llvm::DenseSet findReferencedFiles(const ReferencedLocations &Locs,
+   const SourceManager &SM);
+
+inline llvm::DenseMap
+directlyReferencedFiles(const IncludeStructure::AbstractIncludeGraph &Graph,
+const llvm::DenseSet &Referenced,
+unsigned EntryPoint) {
+  llvm::DenseMap Result;
+  for (unsigned Inclusion : Graph.lookup(EntryPoint))
+Result.try_emplace(Inclusion, Referenced.contains(Inclusion));
+  return Result;
+}
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -137,5 +137,24 @@
   return Result;
 }
 
+llvm::DenseSet
+findReferencedFiles(const llvm::DenseSet &Locs,
+const SourceManager &SM) {
+  std::vector Sorted{Locs.begin(), Locs.end()};
+  // Group by FileID.
+  llvm::sort(Sorted);
+  ReferencedFiles Result(SM);
+  for (auto It = Sorted.begin(); It < Sorted.end();) {
+FileID FID = SM.getFileID(*It);
+Result.add(FID, *It);
+// Cheaply skip over all the other locations from the same FileID.
+// This avoids lots of redundant Loc->File lookups for the same file.
+do
+  ++It;
+while (It != Sorted.end() && SM.isInFileID(*It, FID));
+  }
+  return std::move(Result.Files);
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Headers.h
===
--- clang-tools-extra/clangd/Headers.h
+++ clang-tools-extra/clangd/Headers.h
@@ -58,6 +58,7 @@
   unsigned HashOffset = 0; // Byte offset from start of file to #.
   int HashLine = 0;// Line number containing the directive, 0-indexed.
   SrcMgr::CharacteristicKind FileKind = SrcMgr::C_User;
+  bool Used = false; // Contains symbols used in the main file.
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Inclusion &);
 bool operator==(const Inclusion &LHS, const Inclusion &RHS);
@@ -129,6 +130,22 @@
  

[PATCH] D108194: [clangd] IncludeCleaner: Mark used headers

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Nit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108194

Files:
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h

Index: clang-tools-extra/clangd/ParsedAST.h
===
--- clang-tools-extra/clangd/ParsedAST.h
+++ clang-tools-extra/clangd/ParsedAST.h
@@ -116,6 +116,8 @@
 return Resolver.get();
   }
 
+  void computeUsedIncludes();
+
 private:
   ParsedAST(llvm::StringRef Version,
 std::shared_ptr Preamble,
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -18,6 +18,7 @@
 #include "Features.h"
 #include "Headers.h"
 #include "HeuristicResolver.h"
+#include "IncludeCleaner.h"
 #include "IncludeFixer.h"
 #include "Preamble.h"
 #include "SourceCode.h"
@@ -601,5 +602,25 @@
 return llvm::None;
   return llvm::StringRef(Preamble->Version);
 }
+
+void ParsedAST::computeUsedIncludes() {
+  const auto &SM = getSourceManager();
+
+  auto Refs = findReferencedLocations(*this);
+  auto ReferencedFileIDs = findReferencedFiles(Refs, SM);
+  std::vector ReferencedFilenames;
+  ReferencedFilenames.reserve(ReferencedFileIDs.size());
+  for (FileID FID : ReferencedFileIDs) {
+const FileEntry *FE = SM.getFileEntryForID(FID);
+if (!FE) {
+  elog("Missing FE for {0}", SM.getComposedLoc(FID, 0).printToString(SM));
+  continue;
+}
+ReferencedFilenames.push_back(SM.getFileEntryForID(FID)->getName());
+  }
+  Includes.markUsed(SM.getFileEntryForID(SM.getMainFileID())->getName(),
+ReferencedFilenames, directlyReferencedFiles);
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/IncludeCleaner.h
===
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -46,6 +46,22 @@
 /// - err on the side of reporting all possible locations
 ReferencedLocations findReferencedLocations(ParsedAST &AST);
 
+/// Retrieves IDs of all files containing SourceLocations from \p Locs. Those
+/// locations could be within macro expansions and are not resolved to their
+/// spelling locations.
+llvm::DenseSet findReferencedFiles(const ReferencedLocations &Locs,
+   const SourceManager &SM);
+
+inline llvm::DenseMap
+directlyReferencedFiles(const IncludeStructure::AbstractIncludeGraph &Graph,
+const llvm::DenseSet &Referenced,
+unsigned EntryPoint) {
+  llvm::DenseMap Result;
+  for (unsigned Inclusion : Graph.lookup(EntryPoint))
+Result.try_emplace(Inclusion, Referenced.contains(Inclusion));
+  return Result;
+}
+
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -137,5 +137,24 @@
   return Result;
 }
 
+llvm::DenseSet
+findReferencedFiles(const llvm::DenseSet &Locs,
+const SourceManager &SM) {
+  std::vector Sorted{Locs.begin(), Locs.end()};
+  // Group by FileID.
+  llvm::sort(Sorted);
+  ReferencedFiles Result(SM);
+  for (auto It = Sorted.begin(); It < Sorted.end();) {
+FileID FID = SM.getFileID(*It);
+Result.add(FID, *It);
+// Cheaply skip over all the other locations from the same FileID.
+// This avoids lots of redundant Loc->File lookups for the same file.
+do
+  ++It;
+while (It != Sorted.end() && SM.isInFileID(*It, FID));
+  }
+  return std::move(Result.Files);
+}
+
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Headers.h
===
--- clang-tools-extra/clangd/Headers.h
+++ clang-tools-extra/clangd/Headers.h
@@ -58,6 +58,7 @@
   unsigned HashOffset = 0; // Byte offset from start of file to #.
   int HashLine = 0;// Line number containing the directive, 0-indexed.
   SrcMgr::CharacteristicKind FileKind = SrcMgr::C_User;
+  bool Used = false; // Contains symbols used in the main file.
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Inclusion &);
 bool operator==(const Inclusion &LHS, const Inclusion &RHS);
@@ -129,6 +130,22 @@
  llvm::StringRef IncludedName,
  llvm::StringRef IncludedRealName);
 
+  // Classifying the main-file includes as "used" or "unused" is subtle
+  // (consider transitive include

[PATCH] D108194: [clangd] IncludeCleaner: Mark used headers

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev planned changes to this revision.
kbobyrev added a comment.

Need to add some basic tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108194

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

[PATCH] D106343: [OpenCL] Support cl_ext_float_atomics

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

Thanks for the update!  I have a few points to improve the patch.




Comment at: clang/lib/Sema/OpenCLBuiltins.td:1107
+// The functionality added by cl_ext_float_atomics extension
+let MinVersion = CL30 in {
+  foreach TypePair =

Do we really need to guard these additions behind OpenCL 3.0?  The spec mentions
> The functionality added by this extension uses the OpenCL C 2.0 atomic syntax 
> and hence requires OpenCL 2.0 or newer.
(same applies to the opencl-h.c changes of course)



Comment at: clang/lib/Sema/OpenCLBuiltins.td:1112
+  foreach AddressSpaceiKind = [GenericAS, GlobalAS, LocalAS] in {
+  def:
+Builtin<"atomic_fetch_" #ModOp, [

The feature macros seem to be missing.  See `FuncExtOpenCLCPipes` for an 
example how to do that.



Comment at: clang/lib/Sema/OpenCLBuiltins.td:1132-1135
+  foreach TypePair =
+  [[AtomicFloat, Float, Float], [AtomicDouble, Double, Double]] in {
+foreach ModOp = ["max", "min"] in {
+  foreach AddressSpaceiKind = [GenericAS, GlobalAS, LocalAS] in {

This can be merged into the preceeding `foreach` parts I think?



Comment at: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl:139
+
+  resf1 = atomic_fetch_min(a_float, f1);
+  resf1 = atomic_fetch_max(a_float, f1);

As mentioned in the comment on lines 13-17, this test is not meant to be 
exhaustive.  So you don't have to test every overload, checking one or two 
builtins should suffice.


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

https://reviews.llvm.org/D106343

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

[PATCH] D106721: [AArch64] Implemnt MSVC __mulh and __umulh builtins and corresponding IR level intrinsics

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 366839.
mstorsjo added a comment.

Reimplemented the builtins by expanding to the corresponding i128 
multiplication in IR, just like the corresponding existing `__mulh` and 
`__umulh` builtins on x86.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106721

Files:
  clang/include/clang/Basic/BuiltinsAArch64.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/intrin.h
  clang/test/CodeGen/arm64-microsoft-intrinsics.c


Index: clang/test/CodeGen/arm64-microsoft-intrinsics.c
===
--- clang/test/CodeGen/arm64-microsoft-intrinsics.c
+++ clang/test/CodeGen/arm64-microsoft-intrinsics.c
@@ -81,6 +81,28 @@
 // CHECK-MSVC: fence syncscope("singlethread")
 // CHECK-LINUX: error: implicit declaration of function '_ReadWriteBarrier'
 
+long long check_mulh(long long a, long long b) {
+  return __mulh(a, b);
+}
+
+// CHECK-MSVC: %[[ARG1:.*]] = sext i64 {{.*}} to i128
+// CHECK-MSVC: %[[ARG2:.*]] = sext i64 {{.*}} to i128
+// CHECK-MSVC: %[[PROD:.*]] = mul nsw i128 %[[ARG1]], %[[ARG2]]
+// CHECK-MSVC: %[[HIGH:.*]] = ashr i128 %[[PROD]], 64
+// CHECK-MSVC: %[[RES:.*]] = trunc i128 %[[HIGH]] to i64
+// CHECK-LINUX: error: implicit declaration of function '__mulh'
+
+unsigned long long check_umulh(unsigned long long a, unsigned long long b) {
+  return __umulh(a, b);
+}
+
+// CHECK-MSVC: %[[ARG1:.*]] = zext i64 {{.*}} to i128
+// CHECK-MSVC: %[[ARG2:.*]] = zext i64 {{.*}} to i128
+// CHECK-MSVC: %[[PROD:.*]] = mul nuw i128 %[[ARG1]], %[[ARG2]]
+// CHECK-MSVC: %[[HIGH:.*]] = lshr i128 %[[PROD]], 64
+// CHECK-MSVC: %[[RES:.*]] = trunc i128 %[[HIGH]] to i64
+// CHECK-LINUX: error: implicit declaration of function '__umulh'
+
 unsigned __int64 check__getReg() {
   unsigned volatile __int64 reg;
   reg = __getReg(18);
Index: clang/lib/Headers/intrin.h
===
--- clang/lib/Headers/intrin.h
+++ clang/lib/Headers/intrin.h
@@ -574,6 +574,9 @@
 unsigned short __cdecl _byteswap_ushort(unsigned short val);
 unsigned long __cdecl _byteswap_ulong (unsigned long val);
 unsigned __int64 __cdecl _byteswap_uint64(unsigned __int64 val);
+
+__int64 __mulh(__int64 __a, __int64 __b);
+unsigned __int64 __umulh(unsigned __int64 __a, unsigned __int64 __b);
 #endif
 
 
/**\
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -9712,6 +9712,29 @@
 return Builder.CreateCall(F);
   }
 
+  if (BuiltinID == AArch64::BI__mulh || BuiltinID == AArch64::BI__umulh) {
+llvm::Type *ResType = ConvertType(E->getType());
+llvm::Type *Int128Ty = llvm::IntegerType::get(getLLVMContext(), 128);
+
+bool IsSigned = BuiltinID == AArch64::BI__mulh;
+Value *LHS =
+Builder.CreateIntCast(EmitScalarExpr(E->getArg(0)), Int128Ty, 
IsSigned);
+Value *RHS =
+Builder.CreateIntCast(EmitScalarExpr(E->getArg(1)), Int128Ty, 
IsSigned);
+
+Value *MulResult, *HigherBits;
+if (IsSigned) {
+  MulResult = Builder.CreateNSWMul(LHS, RHS);
+  HigherBits = Builder.CreateAShr(MulResult, 64);
+} else {
+  MulResult = Builder.CreateNUWMul(LHS, RHS);
+  HigherBits = Builder.CreateLShr(MulResult, 64);
+}
+HigherBits = Builder.CreateIntCast(HigherBits, ResType, IsSigned);
+
+return HigherBits;
+  }
+
   // Handle MSVC intrinsics before argument evaluation to prevent double
   // evaluation.
   if (Optional MsvcIntId = translateAarch64ToMsvcIntrin(BuiltinID))
Index: clang/include/clang/Basic/BuiltinsAArch64.def
===
--- clang/include/clang/Basic/BuiltinsAArch64.def
+++ clang/include/clang/Basic/BuiltinsAArch64.def
@@ -243,6 +243,9 @@
 TARGET_HEADER_BUILTIN(_WriteStatusReg, "viLLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_AddressOfReturnAddress, "v*", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 
+TARGET_HEADER_BUILTIN(__mulh,  "SLLiSLLiSLLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(__umulh, "ULLiULLiULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
+
 #undef BUILTIN
 #undef LANGBUILTIN
 #undef TARGET_HEADER_BUILTIN


Index: clang/test/CodeGen/arm64-microsoft-intrinsics.c
===
--- clang/test/CodeGen/arm64-microsoft-intrinsics.c
+++ clang/test/CodeGen/arm64-microsoft-intrinsics.c
@@ -81,6 +81,28 @@
 // CHECK-MSVC: fence syncscope("singlethread")
 // CHECK-LINUX: error: implicit declaration of function '_ReadWriteBarrier'
 
+long long check_mulh(long long a, long long b) {
+  return __mulh(a, b);
+}
+
+// CHECK-MSVC: %[[ARG1:.*]] = sext i64 {{.*}} to i128
+// CHECK-MSVC: %[[ARG2:.*]] = sext i64 {{.*}} to i1

[PATCH] D105268: [X86] AVX512FP16 instructions enabling 5/6

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added inline comments.



Comment at: clang/lib/Headers/avx512fp16intrin.h:2373
+  (__v32hf)(__m512h)(A), (__v32hf)(__m512h)(B), (__v32hf)(__m512h)(C), 
\
+  (__mmask32)-1, (int)(R))
+

Add outer brackets to all the defines to prevent precedence issues:
```
#define _mm512_fmadd_round_ph(A, B, C, R)  \
 ((__m512h) __builtin_ia32_vfmaddph512_mask(   \
  (__v32hf)(__m512h)(A), (__v32hf)(__m512h)(B), (__v32hf)(__m512h)(C), \
  (__mmask32)-1, (int)(R)))
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105268

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

[PATCH] D105269: [X86] AVX512FP16 instructions enabling 6/6

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added inline comments.



Comment at: clang/lib/Headers/avx512fp16intrin.h:2900
+  (__v4sf)(__m128h)(C), (__v4sf)(__m128h)(A), (__v4sf)(__m128h)(B),
\
+  (__mmask8)-1, (int)(R))
+

Outer brackets


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105269

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

[PATCH] D108029: [clang][Codegen] Introduce the disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 366845.
glider added a comment.

Address Aaron's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/attr-no-sanitize-coverage.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp

Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -943,6 +943,7 @@
   // Those attributes should be safe to propagate to the extracted function.
   case Attribute::AlwaysInline:
   case Attribute::Cold:
+  case Attribute::DisableSanitizerInstrumentation:
   case Attribute::Hot:
   case Attribute::NoRecurse:
   case Attribute::InlineHint:
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -628,6 +628,8 @@
 return bitc::ATTR_KIND_IN_ALLOCA;
   case Attribute::Cold:
 return bitc::ATTR_KIND_COLD;
+  case Attribute::DisableSanitizerInstrumentation:
+return bitc::ATTR_DISABLE_SANITIZER_INSTRUMENTATION;
   case Attribute::Hot:
 return bitc::ATTR_KIND_HOT;
   case Attribute::ElementType:
Index: llvm/include/llvm/IR/Attributes.td
===
--- llvm/include/llvm/IR/Attributes.td
+++ llvm/include/llvm/IR/Attributes.td
@@ -86,6 +86,9 @@
 def DereferenceableOrNull : IntAttr<"dereferenceable_or_null",
 [ParamAttr, RetAttr]>;
 
+/// Do not instrument function with sanitizers.
+def DisableSanitizerInstrumentation: EnumAttr<"disable_sanitizer_instrumentation", [FnAttr]>;
+
 /// Provide pointer element type to intrinsic.
 def ElementType : TypeAttr<"elementtype", [ParamAttr]>;
 
Index: llvm/include/llvm/Bitcode/LLVMBitCodes.h
===
--- llvm/include/llvm/Bitcode/LLVMBitCodes.h
+++ llvm/include/llvm/Bitcode/LLVMBitCodes.h
@@ -671,6 +671,7 @@
   ATTR_KIND_SWIFT_ASYNC = 75,
   ATTR_KIND_NO_SANITIZE_COVERAGE = 76,
   ATTR_KIND_ELEMENTTYPE = 77,
+  ATTR_DISABLE_SANITIZER_INSTRUMENTATION = 78,
 };
 
 enum ComdatSelectionKindCodes {
Index: llvm/include/llvm/AsmParser/LLToken.h
===
--- llvm/include/llvm/AsmParser/LLToken.h
+++ llvm/include/llvm/AsmParser/LLToken.h
@@ -190,6 +190,7 @@
   kw_convergent,
   kw_dereferenceable,
   kw_dereferenceable_or_null,
+  kw_disable_sanitizer_instrumentation,
   kw_elementtype,
   kw_inaccessiblememonly,
   kw_inaccessiblemem_or_argmemonly,
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -56,6 +56,7 @@
 // CHECK-NEXT: DLLExport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: DLLImport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: Destructor (SubjectMatchRule_function)
+// CHECK-NEXT: DisableSanitizerInstrumentation (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: DisableTailCalls (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: EnableIf (SubjectMatchRule_function)
 // CHECK-NEXT: EnforceTCB (SubjectMatchRule_function)
Index: clang/test/CodeGen/attr-no-sanitize-coverage.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-no-sanitize-coverage.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -debug-info-kind=limited %s -emit-llvm -o - | FileCheck %s
+
+void t1() __attribute__((disable_sanitizer_instrumentation)) {
+}
+// CHECK: disable_sanitizer_instrumentation
+// CHECK-NEXT: void @t1
+
+// CHECK-NOT: disable_sanitizer_instrumentation
+// CHECK: void @t2
+void t2() {
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2286,6 +2286,10 @@
   /// instrumented with __cyg_profile_func_* calls
   bool ShouldInstrumentFunction();
 
+  /// ShouldSkipSa

[PATCH] D108029: [clang][Codegen] Introduce the disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:2945
+  let Spellings = [Clang<"disable_sanitizer_instrumentation">];
+  let Subjects = SubjectList<[Function]>;
+  let Documentation = [DisableSanitizerInstrumentationDocs];

aaron.ballman wrote:
> Do we want this to also appertain to `GlobalVar` and `ObjCMethod` so it's got 
> the same subjects as `no_sanitize`?
Good catch, thank you. When adding the new attribute to ASan, we probably want 
it to mimic the behavior of `no_sanitize("address")`



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:526-528
+  if (CurFuncDecl->hasAttr())
+return true;
+  return false;

aaron.ballman wrote:
> 
Right :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

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

[PATCH] D108138: [SimplifyCFG] Remove switch statements before vectorization

[David Sherwood via Phabricator via cfe-commits]

david-arm added a comment.

In D108138#2947229 , @lebedev.ri 
wrote:

> I'm not sure i'm sold on this, even though i'm aware that selects hurt 
> vectorization.
> How does this Simplify the CFG? I think it would be best to teach LV selects,
> or at worst do this in LV itself.

Hi @lebedev.ri, I'm under the impression that the vectoriser has a policy of 
never making scalar transformations so I doubt it would be acceptable to do 
this in the vectoriser pass. I think the only realistic alternative is to teach 
LV how to vectorise switch statements and create the vector compares and 
selects directly in the code, or scalarise them in the vector loop with 
creation of new blocks. @fhahn and @craig.topper do you have any thoughts on 
this or preference?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108138

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

[PATCH] D108034: [SPIR-V] Add SPIR-V builtin functions and types

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Can you elaborate the overall design flow please? Particularly, it is not clear 
why we need to duplicate OpenCL types and builtins at source level for 
SPIR-V... Is there any way to avoid this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108034

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

[PATCH] D108138: [SimplifyCFG] Remove switch statements before vectorization

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

The alternative to teaching looprotate/LV about switches is to make swiches 
non-canonical in the first half of the pipeline, before LV.
That is, don't form them, and aggressively expand any and all existing switches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108138

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

[PATCH] D108138: [SimplifyCFG] Remove switch statements before vectorization

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

> I'm under the impression that the vectoriser has a policy of never making 
> scalar transformations

I'm not sure what you mean. I've not looked into the details, but it could 
presumably be done as some sort of VPlan transformation, possibly in the 
constructions of vplans to treat switches like multiple icmp's/branches?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108138

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

[PATCH] D108138: [SimplifyCFG] Remove switch statements before vectorization

[David Sherwood via Phabricator via cfe-commits]

david-arm added a comment.

In D108138#2948975 , @dmgreen wrote:

>> I'm under the impression that the vectoriser has a policy of never making 
>> scalar transformations
>
> I'm not sure what you mean. I've not looked into the details, but it could 
> presumably be done as some sort of VPlan transformation, possibly in the 
> constructions of vplans to treat switches like multiple icmp's/branches?

Hi @dmgreen, I just meant that if LV makes a scalar transformation prior to 
legality/cost-model checks, then for some reason we don't vectorise, we then 
end up with a changed scalar body without any vectorisation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108138

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

[PATCH] D106891: [Remarks] Emit optimization remarks for atomics generating CAS loop

[Anshil Gandhi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf22ba5187350: [Remarks] Emit optimization remarks for 
atomics generating CAS loop (authored by gandhi21299).

Changed prior to commit:
  https://reviews.llvm.org/D106891?vs=366683&id=366735#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

Files:
  clang/test/CodeGenCUDA/atomics-remarks-gfx90a.cu
  clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll

Index: llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
@@ -0,0 +1,103 @@
+; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs --pass-remarks=atomic-expand \
+; RUN:  %s -o - 2>&1 | FileCheck %s --check-prefix=GFX90A-CAS
+
+; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at system memory scope
+; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at agent memory scope
+; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at workgroup memory scope
+; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at wavefront memory scope
+; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at singlethread memory scope
+; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at one-as memory scope
+; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at agent-one-as memory scope
+; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at workgroup-one-as memory scope
+; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at wavefront-one-as memory scope
+; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at singlethread-one-as memory scope
+
+; GFX90A-CAS-LABEL: atomic_add_cas:
+; GFX90A-CAS: flat_atomic_cmpswap v3, v[0:1], v[4:5] glc
+; GFX90A-CAS: s_cbranch_execnz
+define dso_local void @atomic_add_cas(float* %p, float %q) {
+entry:
+  %ret = atomicrmw fadd float* %p, float %q monotonic, align 4
+  ret void
+}
+
+; GFX90A-CAS-LABEL: atomic_add_cas_agent:
+; GFX90A-CAS: flat_atomic_cmpswap v3, v[0:1], v[4:5] glc
+; GFX90A-CAS: s_cbranch_execnz
+define dso_local void @atomic_add_cas_agent(float* %p, float %q) {
+entry:
+  %ret = atomicrmw fadd float* %p, float %q syncscope("agent") monotonic, align 4
+  ret void
+}
+
+; GFX90A-CAS-LABEL: atomic_add_cas_workgroup:
+; GFX90A-CAS: flat_atomic_cmpswap v3, v[0:1], v[4:5] glc
+; GFX90A-CAS: s_cbranch_execnz
+define dso_local void @atomic_add_cas_workgroup(float* %p, float %q) {
+entry:
+  %ret = atomicrmw fadd float* %p, float %q syncscope("workgroup") monotonic, align 4
+  ret void
+}
+
+; GFX90A-CAS-LABEL: atomic_add_cas_wavefront:
+; GFX90A-CAS: flat_atomic_cmpswap v3, v[0:1], v[4:5] glc
+; GFX90A-CAS: s_cbranch_execnz
+define dso_local void @atomic_add_cas_wavefront(float* %p, float %q) {
+entry:
+  %ret = atomicrmw fadd float* %p, float %q syncscope("wavefront") monotonic, align 4
+  ret void
+}
+
+; GFX90A-CAS-LABEL: atomic_add_cas_singlethread:
+; GFX90A-CAS: flat_atomic_cmpswap v3, v[0:1], v[4:5] glc
+; GFX90A-CAS: s_cbranch_execnz
+define dso_local void @atomic_add_cas_singlethread(float* %p, float %q) {
+entry:
+  %ret = atomicrmw fadd float* %p, float %q syncscope("singlethread") monotonic, align 4
+  ret void
+}
+
+; GFX90A-CAS-LABEL: atomic_add_cas_one_as:
+; GFX90A-CAS: flat_atomic_cmpswap v3, v[0:1], v[4:5] glc
+; GFX90A-CAS: s_cbranch_execnz
+define dso_local void @atomic_add_cas_one_as(float* %p, float %q) {
+entry:
+  %ret = atomicrmw fadd float* %p, float %q syncscope("one-as") monotonic, align 4
+  ret void
+}
+
+; GFX90A-CAS-LABEL: atomic_add_cas_agent_one_as:
+; GFX90A-CAS: flat_atomic_cmpswap v3, v[0:1], v[4:5] glc
+; GFX90A-CAS: s_cbranch_execnz
+define dso_local void @atomic_add_cas_agent_one_as(float* %p, float %q) {
+entry:
+  %ret = atomicrmw fadd float* %p, float %q syncscope("agent-one-as") monotonic, align 4
+  ret void
+}
+
+; GFX90A-CAS-LABEL: atomic_add_cas_workgroup_one_as:
+; GFX90A-CAS: flat_atomic_cmpswap v3, v[0:1], v[4:5] glc
+; GFX90A-CAS: s_cbranch_execnz
+define dso_local void @atomic_add_cas_workgroup_one_as(float* %p, float %q) {
+entry:
+  %ret = atomicrmw fadd float* %p, float %q syncscope("workgroup-one-as") monotonic, align 4
+  ret void
+}
+
+; GFX90A-CAS-LABEL: atomic_add_cas_wavefront_one_as:
+; GFX90A-CAS: flat_atomic_cmpswap v3, v[0:1], v[4:5] glc
+; GFX90A-CAS: s_cbranch_execnz
+define dso_local void @atomic_add_cas_wavefront_one_as(float* %p, float %q) {
+entry:
+  %ret = atomicrmw fadd float* %p, float %q syncscope("wavefront-one-as") monotonic, align 4
+  ret void
+}
+
+; G

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 366751.
gandhi21299 added a comment.

- added more tests
- addressed feedback from reviewer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

Files:
  clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.h
  llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll

Index: llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
===
--- llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
+++ llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
@@ -1,6 +1,9 @@
 ; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs --pass-remarks=atomic-expand \
 ; RUN:  %s -o - 2>&1 | FileCheck %s --check-prefix=GFX90A-CAS
 
+; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs --pass-remarks=si-lower \
+; RUN:  %s -o - 2>&1 | FileCheck %s --check-prefix=GFX90A-HW
+
 ; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at system memory scope
 ; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at agent memory scope
 ; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at workgroup memory scope
@@ -100,4 +103,97 @@
 entry:
   %ret = atomicrmw fadd float* %p, float %q syncscope("singlethread-one-as") monotonic, align 4
   ret void
-}
\ No newline at end of file
+}
+
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope system due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread-one-as due to an unsafe request.
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw:
+; GFX90A-HW:ds_add_f64 v2, v[0:1]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw(double addrspace(3)* %ptr) #0 {
+main_body:
+  %ret = atomicrmw fadd double addrspace(3)* %ptr, double 4.0 seq_cst
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_agent:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_agent(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wg:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wg(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("workgroup") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wavefront:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wavefront(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("wavefront") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_single_thread:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_single_thread(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("singlethread") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_aoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_aoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent-one-as") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wgoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wgoa(float addrspace(1)* %ptr, float %val) #0 {
+main_bod

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 366752.
gandhi21299 added a comment.

- corrected an argument in AtomicExpand pass


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

Files:
  clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.h
  llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll

Index: llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
===
--- llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
+++ llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
@@ -1,6 +1,9 @@
 ; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs --pass-remarks=atomic-expand \
 ; RUN:  %s -o - 2>&1 | FileCheck %s --check-prefix=GFX90A-CAS
 
+; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs --pass-remarks=si-lower \
+; RUN:  %s -o - 2>&1 | FileCheck %s --check-prefix=GFX90A-HW
+
 ; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at system memory scope
 ; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at agent memory scope
 ; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at workgroup memory scope
@@ -100,4 +103,97 @@
 entry:
   %ret = atomicrmw fadd float* %p, float %q syncscope("singlethread-one-as") monotonic, align 4
   ret void
-}
\ No newline at end of file
+}
+
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope system due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread-one-as due to an unsafe request.
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw:
+; GFX90A-HW:ds_add_f64 v2, v[0:1]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw(double addrspace(3)* %ptr) #0 {
+main_body:
+  %ret = atomicrmw fadd double addrspace(3)* %ptr, double 4.0 seq_cst
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_agent:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_agent(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wg:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wg(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("workgroup") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wavefront:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wavefront(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("wavefront") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_single_thread:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_single_thread(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("singlethread") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_aoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_aoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent-one-as") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wgoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wgoa(float addrspace(1)* %

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl:9
 
+// RUN: %clang_cc1 -cl-std=CL2.0 -O0 -triple=amdgcn-amd-amdhsa -target-cpu 
gfx90a \
+// RUN: -Rpass=si-lower -munsafe-fp-atomics %s -S -o - 2>&1 | \

You are compiling 2 functions with 2 different sets of options. Essentially it 
is unclear what are you checking because either half skips half of the remarks. 
Either compile a single function differently or make 2 different tests.



Comment at: clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl:13
+
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -O0 -triple=amdgcn-amd-amdhsa -target-cpu 
gfx90a \
+// RUN: -Rpass=si-lower -S -emit-llvm -o - 2>&1 | \

This line does not have -munsafe-fp-atomics option...



Comment at: clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl:60
+// GFX90A-HW-LABEL: @atomic_unsafe_hw
+// GFX90A-HW:   atomicrmw fadd float addrspace(1)* %{{.*}}, float %{{.*}} 
syncscope("workgroup-one-as") monotonic, align 4
+// GFX90A-HW:   atomicrmw fadd float addrspace(1)* %{{.*}}, float %{{.*}} 
syncscope("agent-one-as") monotonic, align 4

... therefor these checks must fail.



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12195
 
+  if (!fpModeMatchesGlobalFPAtomicMode(RMW))
+return reportUnsafeHWInst(RMW, AtomicExpansionKind::None);

This is wrong. Condition is inverted and essentially tests should fail. Make 
sure you can pass testing before posting a diff.



Comment at: llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll:108
+
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at 
memory scope system due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at 
memory scope agent due to an unsafe request.

Does it print a function name before the diagnostics? Label checks would be 
useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

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

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

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



Comment at: clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl:9
 
+// RUN: %clang_cc1 -cl-std=CL2.0 -O0 -triple=amdgcn-amd-amdhsa -target-cpu 
gfx90a \
+// RUN: -Rpass=si-lower -munsafe-fp-atomics %s -S -o - 2>&1 | \

rampitec wrote:
> You are compiling 2 functions with 2 different sets of options. Essentially 
> it is unclear what are you checking because either half skips half of the 
> remarks. Either compile a single function differently or make 2 different 
> tests.
I will create 2 seperate tests



Comment at: llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll:108
+
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at 
memory scope system due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at 
memory scope agent due to an unsafe request.

rampitec wrote:
> Does it print a function name before the diagnostics? Label checks would be 
> useful.
Nope, it does not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

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

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 366765.
gandhi21299 marked 2 inline comments as done.
gandhi21299 added a comment.

- split the OpenCL test into two for brevity
- fixed a mistake in SIISelLowering as pointed out by reviewer
- added the missing -munsafe-fp-atomics flag


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

Files:
  clang/test/CodeGenOpenCL/atomics-cas-remarks-gfx90a.cl
  clang/test/CodeGenOpenCL/atomics-remarks-gfx90a.cl
  clang/test/CodeGenOpenCL/atomics-unsafe-hw-remarks-gfx90a.cl
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.h
  llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll

Index: llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
===
--- llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
+++ llvm/test/CodeGen/AMDGPU/atomics-remarks-gfx90a.ll
@@ -1,6 +1,9 @@
 ; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs --pass-remarks=atomic-expand \
 ; RUN:  %s -o - 2>&1 | FileCheck %s --check-prefix=GFX90A-CAS
 
+; RUN: llc -march=amdgcn -mcpu=gfx90a -verify-machineinstrs --pass-remarks=si-lower \
+; RUN:  %s -o - 2>&1 | FileCheck %s --check-prefix=GFX90A-HW
+
 ; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at system memory scope
 ; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at agent memory scope
 ; GFX90A-CAS: A compare and swap loop was generated for an atomic fadd operation at workgroup memory scope
@@ -101,3 +104,96 @@
   %ret = atomicrmw fadd float* %p, float %q syncscope("singlethread-one-as") monotonic, align 4
   ret void
 }
+
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope system due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope agent-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope workgroup-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope wavefront-one-as due to an unsafe request.
+; GFX90A-HW: Hardware instruction generated for atomic fadd operation at memory scope singlethread-one-as due to an unsafe request.
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw:
+; GFX90A-HW:ds_add_f64 v2, v[0:1]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw(double addrspace(3)* %ptr) #0 {
+main_body:
+  %ret = atomicrmw fadd double addrspace(3)* %ptr, double 4.0 seq_cst
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_agent:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_agent(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wg:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wg(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("workgroup") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_wavefront:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_wavefront(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("wavefront") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_single_thread:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_single_thread(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("singlethread") monotonic, align 4
+  ret void
+}
+
+; GFX90A-HW-LABEL: atomic_add_unsafe_hw_aoa:
+; GFX90A-HW:global_atomic_add_f32 v0, v1, s[2:3]
+; GFX90A-HW:s_endpgm
+define amdgpu_kernel void @atomic_add_unsafe_hw_aoa(float addrspace(1)* %ptr, float %val) #0 {
+main_body:
+  %ret = atomicrmw fadd float addrspace(1)* %ptr, float %val syncscope("agent-one-as") mon

[PATCH] D108138: [SimplifyCFG] Remove switch statements before vectorization

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

In D108138#2948995 , @david-arm wrote:

> In D108138#2948975 , @dmgreen wrote:
>
>>> I'm under the impression that the vectoriser has a policy of never making 
>>> scalar transformations
>>
>> I'm not sure what you mean. I've not looked into the details, but it could 
>> presumably be done as some sort of VPlan transformation, possibly in the 
>> constructions of vplans to treat switches like multiple icmp's/branches?
>
> Hi @dmgreen, I just meant that if LV makes a scalar transformation prior to 
> legality/cost-model checks, then for some reason we don't vectorise, we then 
> end up with a changed scalar body without any vectorisation.

Oh yeah, that makes sense. I was wondering if we could teach VPlan to treat 
them as ICmp/Br without having to actually transform the IR, just doing it as 
part of constructing the VPlan.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108138

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

[PATCH] D108199: [msan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider created this revision.
glider added reviewers: eugenis, melver.
Herald added a subscriber: hiraditya.
glider requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Unlike __attribute__((no_sanitize("memory"))), this one will cause MSan
to skip the entire function during instrumentation.

Depends on https://reviews.llvm.org/D108029


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108199

Files:
  clang/docs/MemorySanitizer.rst
  clang/test/CodeGen/sanitize-memory-disable.c
  llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp

Index: llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -5330,6 +5330,9 @@
   if (!CompileKernel && F.getName() == kMsanModuleCtorName)
 return false;
 
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+return false;
+
   MemorySanitizerVisitor Visitor(F, *this, TLI);
 
   // Clear out readonly/readnone attributes.
Index: clang/test/CodeGen/sanitize-memory-disable.c
===
--- /dev/null
+++ clang/test/CodeGen/sanitize-memory-disable.c
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck -check-prefixes CHECK,WITHOUT %s
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=memory | FileCheck -check-prefixes CHECK,MSAN %s
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=kernel-memory | FileCheck -check-prefixes CHECK,KMSAN %s
+
+// Instrumented function.
+// MSan uses memset(addr, -1, size) to poison allocas and stores shadow of the return value in
+// __msan_retval_tls. KMSAN uses __msan_poison_alloca() to poison allocas and calls
+// __msan_get_context_state() at function prologue to access the task context struct (including the
+// shadow of the return value).
+//
+// CHECK: @instrumented1
+// KMSAN: __msan_get_context_state
+// WITHOUT-NOT: __msan_poison_alloca
+// WITHOUT-NOT: @llvm.memset
+// MSAN: @llvm.memset{{.*}}({{.*}}, i8 -1
+// KMSAN: __msan_poison_alloca
+// WITHOUT-NOT: __msan_retval_tls
+// MSAN: __msan_retval_tls
+// CHECK: ret i32
+int instrumented1(int *a) {
+  volatile char buf[8];
+  return *a;
+}
+
+// Function with no_sanitize("memory")/no_sanitize("kernel-memory"): no shadow propagation, but
+// unpoisons memory to prevent false positives.
+// MSan uses memset(addr, 0, size) to unpoison locals, KMSAN uses __msan_unpoison_alloca(). Both
+// tools still access the retval shadow to write 0 to it.
+//
+// CHECK: @no_false_positives1
+// KMSAN: __msan_get_context_state
+// WITHOUT-NOT: __msan_unpoison_alloca
+// WITHOUT-NOT: @llvm.memset
+// MSAN: @llvm.memset{{.*}}({{.*}}, i8 0
+// KMSAN: __msan_unpoison_alloca
+// WITHOUT-NOT: __msan_retval_tls
+// MSAN: __msan_retval_tls
+// CHECK: ret i32
+__attribute__((no_sanitize("memory")))
+__attribute__((no_sanitize("kernel-memory")))
+int no_false_positives1(int *a) {
+  volatile char buf[8];
+  return *a;
+}
+
+// Function with disable_sanitizer_instrumentation: no instrumentation at all.
+//
+// CHECK: @no_instrumentation1
+// KMSAN-NOT: __msan_get_context_state
+// WITHOUT-NOT: __msan_poison_alloca
+// WITHOUT-NOT: @llvm.memset
+// MSAN-NOT: @llvm.memset{{.*}}({{.*}}, i8 0
+// KMSAN-NOT: __msan_unpoison_alloca
+// WITHOUT-NOT: __msan_retval_tls
+// MSAN-NOT: __msan_retval_tls
+// CHECK: ret i32
+__attribute__((disable_sanitizer_instrumentation))
+int no_instrumentation1(int *a) {
+  volatile char buf[8];
+  return *a;
+}
+
Index: clang/docs/MemorySanitizer.rst
===
--- clang/docs/MemorySanitizer.rst
+++ clang/docs/MemorySanitizer.rst
@@ -85,6 +85,15 @@
 avoid false positives.  This attribute may not be supported by other compilers,
 so we suggest to use it together with ``__has_feature(memory_sanitizer)``.
 
+``__attribute__((disable_sanitizer_instrumentation))``
+
+
+The ``disable_sanitizer_instrumentation`` attribute can be applied to a certain
+function to prevent all kinds of instrumentation. This attribute overrides
+``no_sanitize("memory")`` and may introduce false positives, so it should
+be used with care, e.g. when the user wants to ensure critical code does not
+have unexpected side effects.
+
 Ignorelist
 --
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108119: Wiring of standard library indexing into clangd.

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel updated this revision to Diff 366856.
kuhnel marked an inline comment as done.
kuhnel added a comment.

fixed lifecycle of StdLibIndex variable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108119

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -484,6 +484,18 @@
 init(true),
 };
 
+opt IndexStandardLibrary{
+"index-standard-library",
+cat(Features),
+desc("Background index should always include the STL headers even if \n"
+ "not used at the moment. This will enable code completion and \n"
+ "include fixer."
+ "WARNING: This option is experimental only, and will be removed "
+ "eventually. Don't rely on it"),
+init(false),
+Hidden,
+};
+
 std::function getMemoryCleanupFunction() {
   if (!EnableMallocTrim)
 return nullptr;
@@ -912,6 +924,7 @@
   if (ForceOffsetEncoding != OffsetEncoding::UnsupportedEncoding)
 Opts.Encoding = ForceOffsetEncoding;
 
+  Opts.IndexStandardLibrary = IndexStandardLibrary;
   if (CheckFile.getNumOccurrences()) {
 llvm::SmallString<256> Path;
 if (auto Error =
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -24,6 +24,7 @@
 #include "index/Background.h"
 #include "index/FileIndex.h"
 #include "index/Index.h"
+#include "index/StdLib.h"
 #include "refactor/Rename.h"
 #include "refactor/Tweak.h"
 #include "support/Cancellation.h"
@@ -167,7 +168,11 @@
 FeatureModuleSet *FeatureModules = nullptr;
 
 explicit operator TUScheduler::Options() const;
+
+// Automatically index the standard library - experimental feature
+bool IndexStandardLibrary = false;
   };
+
   // Sensible default options for use in tests.
   // Features like indexing must be enabled if desired.
   static Options optsForTest();
@@ -403,6 +408,8 @@
   std::unique_ptr BackgroundIdx;
   // Storage for merged views of the various indexes.
   std::vector> MergedIdx;
+  // If present, the index of the standard library.
+  std::unique_ptr StandardLibraryIdx;
 
   // When set, provides clang-tidy options for a specific file.
   TidyProviderRef ClangTidyProvider;
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -27,6 +27,7 @@
 #include "index/CanonicalIncludes.h"
 #include "index/FileIndex.h"
 #include "index/Merge.h"
+#include "index/StdLib.h"
 #include "refactor/Rename.h"
 #include "refactor/Tweak.h"
 #include "support/Logger.h"
@@ -175,6 +176,16 @@
   this->Index = Idx;
 }
   };
+  if (Opts.IndexStandardLibrary) {
+auto SLIndex = indexStandardLibrary(TFS);
+if (!SLIndex) {
+  elog("Unable to build standard library index. Not using it.");
+  elog(toString(SLIndex.takeError()).c_str());
+} else {
+  StandardLibraryIdx = std::move(SLIndex.get());
+  AddIndex(StandardLibraryIdx.get());
+}
+  }
   if (Opts.StaticIndex)
 AddIndex(Opts.StaticIndex);
   if (Opts.BackgroundIndex) {


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -484,6 +484,18 @@
 init(true),
 };
 
+opt IndexStandardLibrary{
+"index-standard-library",
+cat(Features),
+desc("Background index should always include the STL headers even if \n"
+ "not used at the moment. This will enable code completion and \n"
+ "include fixer."
+ "WARNING: This option is experimental only, and will be removed "
+ "eventually. Don't rely on it"),
+init(false),
+Hidden,
+};
+
 std::function getMemoryCleanupFunction() {
   if (!EnableMallocTrim)
 return nullptr;
@@ -912,6 +924,7 @@
   if (ForceOffsetEncoding != OffsetEncoding::UnsupportedEncoding)
 Opts.Encoding = ForceOffsetEncoding;
 
+  Opts.IndexStandardLibrary = IndexStandardLibrary;
   if (CheckFile.getNumOccurrences()) {
 llvm::SmallString<256> Path;
 if (auto Error =
Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -24,6 +24,7 @@
 #include "index/Background.h"
 #include "index/FileIndex.h"
 #include "index/Index.h"
+#include "index/StdLib.h"
 #include "refactor/Rename.h"
 #include "refactor

[PATCH] D108119: Wiring of standard library indexing into clangd.

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:180
+  if (Opts.IndexStandardLibrary) {
+auto SLIndex = indexStandardLibrary(TFS);
+if (!SLIndex || !SLIndex->get()) {

I'm not sure if returning a Expected makes sense here. I suppose logging 
could be done from StandardLibraryIndex just as well.




Comment at: clang-tools-extra/clangd/ClangdServer.cpp:184
+}
+AddIndex(SLIndex->get());
+  }

sammccall wrote:
> this pointer will dangle after the `if` block finishes (the local `auto` owns 
> the index)
Ah yes, you're right. The other indexes are kept in member variables...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108119

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

[PATCH] D108138: [SimplifyCFG] Remove switch statements before vectorization

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

xbolva00 added subscribers: nikic, xbolva00.
xbolva00 added a comment.

Also check @nikic’s https://reviews.llvm.org/D95296


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108138

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

[PATCH] D108202: [tsan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider created this revision.
glider added a reviewer: melver.
Herald added subscribers: jfb, hiraditya.
glider requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Unlike __attribute__((no_sanitize("thread"))), this one will cause TSan
to skip the entire function during instrumentation.

Depends on https://reviews.llvm.org/D108029


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108202

Files:
  clang/docs/ThreadSanitizer.rst
  clang/test/CodeGen/sanitize-thread-disable.c
  llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp


Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
@@ -563,6 +563,12 @@
   // all.
   if (F.hasFnAttribute(Attribute::Naked))
 return false;
+
+  // __attribute__(disable_sanitizer_instrumentation) prevents all kinds of
+  // instrumentation.
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+return false;
+
   initialize(*F.getParent());
   SmallVector AllLoadsAndStores;
   SmallVector LocalLoadsAndStores;
Index: clang/test/CodeGen/sanitize-thread-disable.c
===
--- /dev/null
+++ clang/test/CodeGen/sanitize-thread-disable.c
@@ -0,0 +1,57 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck -check-prefixes 
CHECK,WITHOUT %s
+// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=thread | FileCheck 
-check-prefixes CHECK,TSAN %s
+
+#include 
+
+// Instrumented function.
+// TSan inserts calls to __tsan_func_entry() and __tsan_func_exit() to 
prologue/epilogue.
+// Non-atomic loads are instrumented with __tsan_readXXX(), atomic loads - with
+// __tsan_atomicXXX_load().
+//
+// CHECK: @instrumented1
+// TSAN: call void @__tsan_func_entry
+// WITHOUT-NOT: call void @__tsan_func_entry
+// TSAN: call void @__tsan_read4
+// WITHOUT-NOT: call void @__tsan_read4
+// TSAN: call i32 @__tsan_atomic32_load
+// WITHOUT-NOT: call i32 @__tsan_atomic32_load
+// TSAN: call void @__tsan_func_exit
+// WITHOUT-NOT: call void @__tsan_func_exit
+// CHECK: ret i32
+int instrumented1(int *a, _Atomic int *b) {
+  return *a + atomic_load(b);
+}
+
+// Function with no_sanitize("thread").
+// TSan only inserts instrumentation necessary to prevent false positives: 
calls are inserted for
+// function entry/exit and atomics, but not plain memory accesses.
+//
+// CHECK: @no_false_positives1
+// TSAN: call void @__tsan_func_entry
+// WITHOUT-NOT: call void @__tsan_func_entry
+// TSAN-NOT: call void @__tsan_read4
+// WITHOUT-NOT: call void @__tsan_read4
+// TSAN: call i32 @__tsan_atomic32_load
+// WITHOUT-NOT: call i32 @__tsan_atomic32_load
+// TSAN: call void @__tsan_func_exit
+// WITHOUT-NOT: call void @__tsan_func_exit
+// CHECK: ret i32
+__attribute__((no_sanitize("thread"))) int no_false_positives1(int *a, _Atomic 
int *b) {
+  return *a + atomic_load(b);
+}
+
+// Function with disable_sanitizer_instrumentation: no instrumentation at all.
+//
+// CHECK: @no_instrumentation1
+// TSAN-NOT: call void @__tsan_func_entry
+// WITHOUT-NOT: call void @__tsan_func_entry
+// TSAN-NOT: call void @__tsan_read4
+// WITHOUT-NOT: call void @__tsan_read4
+// TSAN-NOT: call i32 @__tsan_atomic32_load
+// WITHOUT-NOT: call i32 @__tsan_atomic32_load
+// TSAN-NOT: call void @__tsan_func_exit
+// WITHOUT-NOT: call void @__tsan_func_exit
+// CHECK: ret i32
+__attribute__((disable_sanitizer_instrumentation)) int no_instrumentation1(int 
*a, _Atomic int *b) {
+  return *a + atomic_load(b);
+}
Index: clang/docs/ThreadSanitizer.rst
===
--- clang/docs/ThreadSanitizer.rst
+++ clang/docs/ThreadSanitizer.rst
@@ -100,6 +100,15 @@
 traces.  This attribute may not be supported by other compilers, so we suggest
 to use it together with ``__has_feature(thread_sanitizer)``.
 
+``__attribute__((disable_sanitizer_instrumentation))``
+
+
+The ``disable_sanitizer_instrumentation`` attribute can be applied to a certain
+function to prevent all kinds of instrumentation. This attribute overrides
+``no_sanitize("thread")`` and may introduce false positives, so it should
+be used with care, e.g. when the user wants to ensure critical code does not
+have unexpected side effects.
+
 Ignorelist
 --
 


Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
@@ -563,6 +563,12 @@
   // all.
   if (F.hasFnAttribute(Attribute::Naked))
 return false;
+
+  // __attribute__(disable_sanitizer_instrumentation) prevents all kinds of
+  // instrumentation.
+  if (F.hasFnAttrib

[PATCH] D104975: Implement P1949

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 366860.
cor3ntin marked an inline comment as done.
cor3ntin added a comment.

Fix Aaron's comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104975

Files:
  clang/include/clang/Basic/CharInfo.h
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/Lexer.cpp
  clang/lib/Lex/UnicodeCharSets.h
  clang/test/CXX/drs/dr1xx.cpp
  clang/test/CXX/drs/dr2xx.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/FixIt/fixit-unicode.c
  clang/test/Lexer/unicode.c
  clang/test/Parser/cxx11-user-defined-literals.cpp
  clang/test/Preprocessor/assembler-with-cpp.c
  clang/test/Preprocessor/ucn-allowed-chars.c
  clang/test/Preprocessor/utf8-allowed-chars.c
  clang/www/cxx_dr_status.html
  clang/www/cxx_status.html
  clang/www/make_cxx_dr_status
  llvm/include/llvm/Support/UnicodeCharRanges.h

Index: llvm/include/llvm/Support/UnicodeCharRanges.h
===
--- llvm/include/llvm/Support/UnicodeCharRanges.h
+++ llvm/include/llvm/Support/UnicodeCharRanges.h
@@ -62,6 +62,14 @@
   /// Returns true if the character set contains the Unicode code point
   /// \p C.
   bool contains(uint32_t C) const {
+if (C < 127) {
+  for (const auto &R : Ranges) {
+if (R.Lower <= C && R.Upper >= C)
+  return true;
+if (R.Lower > C)
+  return false;
+  }
+}
 return std::binary_search(Ranges.begin(), Ranges.end(), C);
   }
 
Index: clang/www/make_cxx_dr_status
===
--- clang/www/make_cxx_dr_status
+++ clang/www/make_cxx_dr_status
@@ -135,12 +135,16 @@
 avail_style = ' class="na"'
   elif status.startswith('sup '):
 dup = status.split(' ', 1)[1]
-avail = 'Superseded by %s' % (dup, dup)
-try:
-  _, avail_style = availability(int(dup))
-except:
-  print >>sys.stderr, "issue %s marked as sup %s" % (issue, dup)
-  avail_style = ' class="none"'
+if dup.startswith('P'):
+  avail = 'Superseded by https://wg21.link/%s";>%s' % (dup, dup)
+  avail_style = ' class="na"'
+else:
+  avail = 'Superseded by %s' % (dup, dup)
+  try:
+_, avail_style = availability(int(dup))
+  except:
+print >>sys.stderr, "issue %s marked as sup %s" % (issue, dup)
+avail_style = ' class="none"'
   elif status.startswith('dup '):
 dup = int(status.split(' ', 1)[1])
 avail = 'Duplicate of %s' % (dup, dup)
Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1314,7 +1314,7 @@
 
   C++ identifier syntax using UAX 31
   https://wg21.link/P1949R7";>P1949R7
-  No
+  Clang 14
 
 
   Mixed string literal concatenation
Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -824,7 +824,7 @@
 https://wg21.link/cwg131";>131
 TC1
 Typo in Lao characters
-Yes
+Superseded by P1949
   
   
 https://wg21.link/cwg132";>132
@@ -1527,7 +1527,7 @@
 https://wg21.link/cwg248";>248
 C++11
 Identifier characters
-Yes (C++11 onwards)
+Superseded by P1949
   
   
 https://wg21.link/cwg249";>249
@@ -4020,7 +4020,7 @@
 https://wg21.link/cwg663";>663
 CD1
 Valid Cyrillic identifier characters
-Yes (C++11 onwards)
+Superseded by P1949
   
   
 https://wg21.link/cwg664";>664
Index: clang/test/Preprocessor/utf8-allowed-chars.c
===
--- clang/test/Preprocessor/utf8-allowed-chars.c
+++ clang/test/Preprocessor/utf8-allowed-chars.c
@@ -18,51 +18,41 @@
 
 // Identifier initial characters
 extern char ๐; // C++03, C11, C++11
-extern char ̀; // disallowed initially in C11/C++11, always in C99/C++03
-
-
-
-
+extern char ̀; // disallowed initially in C11/C++, always in C99
 
 
 
 
 #if __cplusplus
-# if __cplusplus >= 201103L
-// C++11
-// expected-warning@9 {{using this character in an identifier is incompatible with C++98}}
-// expected-warning@10 {{using this character in an identifier is incompatible with C++98}}
-// expected-error@13 {{non-ASCII characters are not allowed outside of literals and identifiers}}
-// expected-warning@14 {{using this character in an identifier is incompatible with C++98}}
-// expected-error@21 {{expected unqualified-id}}
+// expected-error@11 {{not allowed in an identifier}}
+// expected-error@13 {{not allowed in an identifier}}
+// expected-error@20 {{character  not allowed at the start of an identifier}}
+// expected-error@21 {{character  not allowed at the start of an identifier}}
+// expected-warning@20 {{declaration does not declare anything}}
+// expected-warning@21 {{declaration does not declare any

[PATCH] D104975: Implement P1949

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a commenting nit that I managed to miss last time (sorry about 
that). Btw, would you like to apply for commit privileges now that you've 
gotten a few patches under your belt 
(https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access)?




Comment at: clang/lib/Lex/Lexer.cpp:1661-1663
+// We got a unicode codepoint that is neither a space nor a
+// a valid identifier part
+// Carry on as if the codepoint was valid for recovery purposes

Oops, I missed this one last time!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104975

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

[PATCH] D108038: [C++4OpenCL] C++ for OpenCL version 2021 introduced to command line

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks

Let's add a reference to RFC in the description: 
https://lists.llvm.org/pipermail/cfe-dev/2021-August/068593.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108038

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

[PATCH] D104975: Implement P1949

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 366865.
cor3ntin added a comment.

Fix comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104975

Files:
  clang/include/clang/Basic/CharInfo.h
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/lib/Lex/Lexer.cpp
  clang/lib/Lex/UnicodeCharSets.h
  clang/test/CXX/drs/dr1xx.cpp
  clang/test/CXX/drs/dr2xx.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/FixIt/fixit-unicode.c
  clang/test/Lexer/unicode.c
  clang/test/Parser/cxx11-user-defined-literals.cpp
  clang/test/Preprocessor/assembler-with-cpp.c
  clang/test/Preprocessor/ucn-allowed-chars.c
  clang/test/Preprocessor/utf8-allowed-chars.c
  clang/www/cxx_dr_status.html
  clang/www/cxx_status.html
  clang/www/make_cxx_dr_status
  llvm/include/llvm/Support/UnicodeCharRanges.h

Index: llvm/include/llvm/Support/UnicodeCharRanges.h
===
--- llvm/include/llvm/Support/UnicodeCharRanges.h
+++ llvm/include/llvm/Support/UnicodeCharRanges.h
@@ -62,6 +62,14 @@
   /// Returns true if the character set contains the Unicode code point
   /// \p C.
   bool contains(uint32_t C) const {
+if (C < 127) {
+  for (const auto &R : Ranges) {
+if (R.Lower <= C && R.Upper >= C)
+  return true;
+if (R.Lower > C)
+  return false;
+  }
+}
 return std::binary_search(Ranges.begin(), Ranges.end(), C);
   }
 
Index: clang/www/make_cxx_dr_status
===
--- clang/www/make_cxx_dr_status
+++ clang/www/make_cxx_dr_status
@@ -135,12 +135,16 @@
 avail_style = ' class="na"'
   elif status.startswith('sup '):
 dup = status.split(' ', 1)[1]
-avail = 'Superseded by %s' % (dup, dup)
-try:
-  _, avail_style = availability(int(dup))
-except:
-  print >>sys.stderr, "issue %s marked as sup %s" % (issue, dup)
-  avail_style = ' class="none"'
+if dup.startswith('P'):
+  avail = 'Superseded by https://wg21.link/%s";>%s' % (dup, dup)
+  avail_style = ' class="na"'
+else:
+  avail = 'Superseded by %s' % (dup, dup)
+  try:
+_, avail_style = availability(int(dup))
+  except:
+print >>sys.stderr, "issue %s marked as sup %s" % (issue, dup)
+avail_style = ' class="none"'
   elif status.startswith('dup '):
 dup = int(status.split(' ', 1)[1])
 avail = 'Duplicate of %s' % (dup, dup)
Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1314,7 +1314,7 @@
 
   C++ identifier syntax using UAX 31
   https://wg21.link/P1949R7";>P1949R7
-  No
+  Clang 14
 
 
   Mixed string literal concatenation
Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -824,7 +824,7 @@
 https://wg21.link/cwg131";>131
 TC1
 Typo in Lao characters
-Yes
+Superseded by P1949
   
   
 https://wg21.link/cwg132";>132
@@ -1527,7 +1527,7 @@
 https://wg21.link/cwg248";>248
 C++11
 Identifier characters
-Yes (C++11 onwards)
+Superseded by P1949
   
   
 https://wg21.link/cwg249";>249
@@ -4020,7 +4020,7 @@
 https://wg21.link/cwg663";>663
 CD1
 Valid Cyrillic identifier characters
-Yes (C++11 onwards)
+Superseded by P1949
   
   
 https://wg21.link/cwg664";>664
Index: clang/test/Preprocessor/utf8-allowed-chars.c
===
--- clang/test/Preprocessor/utf8-allowed-chars.c
+++ clang/test/Preprocessor/utf8-allowed-chars.c
@@ -18,51 +18,41 @@
 
 // Identifier initial characters
 extern char ๐; // C++03, C11, C++11
-extern char ̀; // disallowed initially in C11/C++11, always in C99/C++03
-
-
-
-
+extern char ̀; // disallowed initially in C11/C++, always in C99
 
 
 
 
 #if __cplusplus
-# if __cplusplus >= 201103L
-// C++11
-// expected-warning@9 {{using this character in an identifier is incompatible with C++98}}
-// expected-warning@10 {{using this character in an identifier is incompatible with C++98}}
-// expected-error@13 {{non-ASCII characters are not allowed outside of literals and identifiers}}
-// expected-warning@14 {{using this character in an identifier is incompatible with C++98}}
-// expected-error@21 {{expected unqualified-id}}
+// expected-error@11 {{not allowed in an identifier}}
+// expected-error@13 {{not allowed in an identifier}}
+// expected-error@20 {{character  not allowed at the start of an identifier}}
+// expected-error@21 {{character  not allowed at the start of an identifier}}
+// expected-warning@20 {{declaration does not declare anything}}
+// expected-warning@21 {{declaration does not declare anything}}
 
-# else
-// C++03
-// expected-error@9 {{

[PATCH] D108113: [C++4OpenCL] Enable -cl-std flag clc++21 as a shorthand for clc++2021

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a subscriber: svenvh.
Anastasia added a comment.

@svenvh I would like to discuss it further with Sven whether it is a good 
direction or not?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108113

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

[PATCH] D107963: [OpenCL] Fix as_type(vec3) invalid store creation

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4789
-
-if (!CGF.CGM.getCodeGenOpts().PreserveVec3Type) {
-  Src = createCastsForTypeOfSameSize(Builder, CGF.CGM.getDataLayout(), Src,

While I agree with this fix and it obviously looks incorrect, I wonder if the 
original intent was to condition the previous statement instead so that we 
avoid converting to size 4 at all? Although I have a feeling we are entering 
the behavior that is not documented anywhere. In the spec I can see this:


```
When the operand and result type contain a different number of elements, the 
result shall be implementation-defined except if the operand is a 4-component 
vector and the result is a 3-component vector. In this case, the bits in the 
operand shall be returned directly without modification as the new type. 
```

but it seems to cover the inverse conversion?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107963

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

[PATCH] D108119: Wiring of standard library indexing into clangd.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:180
+  if (Opts.IndexStandardLibrary) {
+auto SLIndex = indexStandardLibrary(TFS);
+if (!SLIndex) {

A couple of issues with indexing here and placing it in the stack:

1. it's synchronous and blocks all other operations, we shouldn't do that. This 
is a latency sensitive path: the user just opened the first file in their 
editor. Our usual trick here is using a SwapIndex as a placeholder and loading 
asynchronously, but...

2. there's no way to disable it or use a different configuration for e.g. C 
files, or according to config.

I think ultimately the triggering logic will want to live in TUScheduler so it 
can be sensitive to CompilerInvocation (and thus LangOpts) as well as config. 
Essentially it will wrap ParseInputs.Index after calling 
buildCompilerInvocation.

That's more complicated to get right, and if you want to delay it and still be 
able to use this locally it makes sense. Some FIXME comments should describe 
how this needs to be moved before the feature is usable by end-users though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108119

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

[PATCH] D108113: [C++4OpenCL] Enable -cl-std flag clc++21 as a shorthand for clc++2021

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

In D108113#2949141 , @Anastasia wrote:

> @svenvh I would like to discuss it further with Sven whether it is a good 
> direction or not?

The underlying motivation for adding those aliases seems to be missing from the 
description.

Personally, I do not see the point of adding aliases to save two characters 
(you only save the "20" part of "clc++2021"), while adding potential confusion 
("is the language called //C++ for OpenCL 2021// or //C++ for OpenCL 21//, or 
maybe it has something to do with //OpenCL 2.1//?").  Also they are being added 
as deprecated aliases, suggesting they might be removed at some point in the 
future again?

I'd much rather see one canonical way of specifying the language version, 
instead of adding multiple aliases to do the same thing.  But perhaps there is 
a good reason that I am not aware of?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108113

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

[PATCH] D108038: [C++4OpenCL] C++ for OpenCL version 2021 introduced to command line

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh accepted this revision.
svenvh added a comment.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108038

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

[PATCH] D105426: [clangd] IncludeCleaner as a library: Find all references to symbols in the file

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:65
+  bool VisitExpr(Expr *E) {
+TraverseType(E->getType());
+return true;

I just realized this uses "T" if any expression has type "T*" even if never 
dereferenced, this is probably a gross overestimate. Nevertheless I guess fine 
for now.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:101
+
+// Given a set of referenced FileIDs, determines all the potentially-referenced
+// files and macros by traversing expansion/spelling locations of macro IDs.

this struct is not used in this patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105426

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

[PATCH] D106030: [Clang] add support for error+warning fn attrs

[Aaron Ballman via Phabricator via cfe-commits]

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

Clang parts LGTM, thank you for this!




Comment at: clang/test/Sema/attr-error.c:31-32
+
+__attribute__((error("foo"))) int bad5(void);   // expected-error {{'error' 
and 'warning' attributes are not compatible}}
+__attribute__((warning("foo"))) int bad5(void); // expected-note {{conflicting 
attribute is here}}
+

nickdesaulniers wrote:
> aaron.ballman wrote:
> > nickdesaulniers wrote:
> > > aaron.ballman wrote:
> > > > I think the diagnostic order is backwards here. The first declaration 
> > > > is where I'd expect the note and the second declaration is where I'd 
> > > > expect the error. (The idea is: the first declaration adds an attribute 
> > > > to the decl, so the redeclaration is what introduces the conflict and 
> > > > so that's where the error should live.) As an example: 
> > > > https://godbolt.org/z/bjGTWxYvh
> > > I'm not sure how to reconcile this. Looking at `bad4` above (different 
> > > case than `bad5`), the diagnostic we get is:
> > > ```
> > > /tmp/y.c:1:30: error: 'warning' and 'error' attributes are not compatible
> > > __attribute__((error("foo"), warning("foo")))
> > >  ^
> > > /tmp/y.c:1:16: note: conflicting attribute is here
> > > __attribute__((error("foo"), warning("foo")))
> > >^
> > > 1 error generated.
> > > ```
> > > which looks correct to me. If I flip the order these are diagnosed in, 
> > > that fixes `bad5` but if I flip around the ordering:
> > > ```
> > > --- a/clang/lib/Sema/SemaDeclAttr.cpp
> > > +++ b/clang/lib/Sema/SemaDeclAttr.cpp
> > > -  Diag(CI.getLoc(), diag::err_attributes_are_not_compatible) << CI 
> > > << EA;
> > > -  Diag(EA->getLocation(), diag::note_conflicting_attribute);
> > > +  Diag(EA->getLocation(), diag::err_attributes_are_not_compatible)
> > > +  << CI << EA;
> > > +  Diag(CI.getLoc(), diag::note_conflicting_attribute);
> > > ```
> > > Then `bad4` doesn't look quite correct.
> > > ```
> > > /tmp/y.c:1:16: error: 'warning' and 'error' attributes are not compatible
> > > __attribute__((error("foo"), warning("foo")))
> > >^
> > > /tmp/y.c:1:30: note: conflicting attribute is here
> > > __attribute__((error("foo"), warning("foo")))
> > >  ^
> > > ```
> > > Perhaps in the callers of `handleErrorAttr` I'm confusing which `Decl` is 
> > > the "new" vs the "old?"
> > > which looks correct to me.
> > 
> > That also looks correct to me; the second attribute is the diagnostic and 
> > the first attribute is the note.
> > 
> > > Perhaps in the callers of handleErrorAttr I'm confusing which Decl is the 
> > > "new" vs the "old?"
> > 
> > Huh, this is rather strange. The logic you're using is basically the same 
> > as `checkAttrMutualExclusion()`, just with extra work to figure out which 
> > kind of attribute you're dealing with. I'm not seeing what's causing the 
> > order to be different when I reason my way through the code. Perhaps in the 
> > `bad4()` case, are we somehow processing the attributes in reverse order?
> > 
> > FWIW, at the end of the day, we can live with the diagnostic in either 
> > situation, even if they're a bit strange. I think the merge behavior (two 
> > different decls) is more important to get right because that's going to be 
> > the far more common scenario to see the diagnostic in. If it turns out that 
> > it's a systemic issue (or just gnarly to figure out), it won't block this 
> > review. We can always improve the behavior in a follow-up.
> I suspect that `DiagnoseMutualExclusions` (via tablegen) would be able to 
> catch this, had we distinct `Attr`s for warning and error; example tablegen 
> rule: `def : MutualExclusions<[Hot, Cold]>;`.
I suspect you're correct. Perhaps the logic in ClangAttrEmitter.cpp is already 
accounting for some oddity here that we're not. The way the generated mutual 
exclusions look are:
```
  if (const auto *Second = dyn_cast(A)) {
if (const auto *First = D->getAttr()) {
  S.Diag(First->getLocation(), diag::err_attributes_are_not_compatible) << 
First << Second;
  S.Diag(Second->getLocation(), diag::note_conflicting_attribute);
  return false;
}
return true;
  }
```
Compared with your code, this is different. In your code, `CI` is analogous to 
`Second` above, and `EA` is analogous to `First`. You diagnose at the location 
of `First` but pass in `<< Second (CI) << First (EA)` as the arguments.
```
ErrorAttr *Sema::mergeErrorAttr(Decl *D, const AttributeCommonInfo &CI,
StringRef NewUserDiagnostic) {
  if (const auto *EA = D->getAttr()) {
std::string NewAttr = CI.getNormalizedFullName();
assert((NewAttr == "error" || NewAttr == "warning") &&
   "unexpected normalized full name");
bool Match = (EA->isError() && NewAttr == "error") ||
 (EA->i

[PATCH] D105268: [X86] AVX512FP16 instructions enabling 5/6

[Pengfei Wang via Phabricator via cfe-commits]

pengfei updated this revision to Diff 366873.
pengfei added a comment.

Rebased.
Add extra parentheses for macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105268

Files:
  clang/include/clang/Basic/BuiltinsX86.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/avx512fp16intrin.h
  clang/lib/Headers/avx512vlfp16intrin.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/X86/avx512fp16-builtins.c
  clang/test/CodeGen/X86/avx512vlfp16-builtins.c
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86InstrAVX512.td
  llvm/lib/Target/X86/X86InstrFMA3Info.cpp
  llvm/lib/Target/X86/X86InstrFoldTables.cpp
  llvm/lib/Target/X86/X86InstrFormats.td
  llvm/lib/Target/X86/X86InstrInfo.cpp
  llvm/lib/Target/X86/X86IntrinsicsInfo.h
  llvm/test/CodeGen/X86/avx512fp16-fma-commute.ll
  llvm/test/CodeGen/X86/avx512fp16-fma-intrinsics.ll
  llvm/test/CodeGen/X86/avx512fp16vl-fma-intrinsics.ll
  llvm/test/CodeGen/X86/fp-strict-scalar-fp16.ll
  llvm/test/CodeGen/X86/stack-folding-fp-avx512fp16-fma.ll
  llvm/test/CodeGen/X86/stack-folding-fp-avx512fp16vl-fma.ll
  llvm/test/CodeGen/X86/vec-strict-128-fp16.ll
  llvm/test/CodeGen/X86/vec-strict-256-fp16.ll
  llvm/test/CodeGen/X86/vec-strict-512-fp16.ll
  llvm/test/MC/Disassembler/X86/avx512fp16.txt
  llvm/test/MC/Disassembler/X86/avx512fp16vl.txt
  llvm/test/MC/X86/avx512fp16.s
  llvm/test/MC/X86/avx512fp16vl.s
  llvm/test/MC/X86/intel-syntax-avx512fp16.s
  llvm/test/MC/X86/intel-syntax-avx512fp16vl.s

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

[PATCH] D105268: [X86] AVX512FP16 instructions enabling 5/6

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: clang/lib/Headers/avx512fp16intrin.h:2373
+  (__v32hf)(__m512h)(A), (__v32hf)(__m512h)(B), (__v32hf)(__m512h)(C), 
\
+  (__mmask32)-1, (int)(R))
+

RKSimon wrote:
> Add outer brackets to all the defines to prevent precedence issues:
> ```
> #define _mm512_fmadd_round_ph(A, B, C, R) 
>  \
>  ((__m512h) __builtin_ia32_vfmaddph512_mask(  
>  \
>   (__v32hf)(__m512h)(A), (__v32hf)(__m512h)(B), (__v32hf)(__m512h)(C),
>  \
>   (__mmask32)-1, (int)(R)))
> ```
Thanks Simon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105268

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

[PATCH] D107963: [OpenCL] Fix as_type(vec3) invalid store creation

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4789
-
-if (!CGF.CGM.getCodeGenOpts().PreserveVec3Type) {
-  Src = createCastsForTypeOfSameSize(Builder, CGF.CGM.getDataLayout(), Src,

Anastasia wrote:
> While I agree with this fix and it obviously looks incorrect, I wonder if the 
> original intent was to condition the previous statement instead so that we 
> avoid converting to size 4 at all? Although I have a feeling we are entering 
> the behavior that is not documented anywhere. In the spec I can see this:
> 
> 
> ```
> When the operand and result type contain a different number of elements, the 
> result shall be implementation-defined except if the operand is a 4-component 
> vector and the result is a 3-component vector. In this case, the bits in the 
> operand shall be returned directly without modification as the new type. 
> ```
> 
> but it seems to cover the inverse conversion?
Yeah I have a similar fix for the inverse case (which is further down in this 
function) in my local branch.

I did try to extend the guard to also cover the `ConvertVec3AndVec4` call, but 
that also led to invalid StoreInst creation.  Since I wasn't sure about the 
intent of the conditioning on `PreserveVec3Type` here, I didn't investigate 
further.

I was hoping @jaykang10 (who added this in D30810) might have some insight into 
why the guard was here in the first place.  But it has been over 4 years since 
that was committed, so there might not be a ready answer.  Either way, I'll 
hold off committing this for a few more days.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107963

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

[PATCH] D105177: [clangd] Implemented indexing of standard library

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/FS.h:77
 
+/// Get a virtual root node for the filesystem depending on the OS
+inline const llvm::StringLiteral virtualRoot() {

"node" doesn't mean anything here.



Comment at: clang-tools-extra/clangd/FS.h:79
+inline const llvm::StringLiteral virtualRoot() {
+#ifdef _WIN32
+  return "\\";

this should be defined out-of-line unless it's performance-critical for some 
reason.

Conditional compilation in inline bodies is a magnet for ODR violations. 
`_WIN32` is probably fine but no reason to scare the reader :-)



Comment at: clang-tools-extra/clangd/FS.h:80
+#ifdef _WIN32
+  return "\\";
+#else

This is a (drive-)relative path, we have various places we need absolute paths 
and may want to reuse this there. Does `C:\virtual` work?



Comment at: clang-tools-extra/clangd/FS.h:80
+#ifdef _WIN32
+  return "\\";
+#else

sammccall wrote:
> This is a (drive-)relative path, we have various places we need absolute 
> paths and may want to reuse this there. Does `C:\virtual` work?
as mentioned this should ideally include some clue that it's a virtual path



Comment at: clang-tools-extra/clangd/index/StdLib.cpp:40
+const ThreadsafeFS &TFS, StandardLibraryVersion LibraryVersion)
+: VirtualUmbrellaHeaderFileName(virtualRoot().str() + 
"UmbrellaHeader.hpp"),
+  TFS(TFS), LibraryVersion(LibraryVersion) {}

llvm::sys::path::append



Comment at: clang-tools-extra/clangd/index/StdLib.cpp:40
+const ThreadsafeFS &TFS, StandardLibraryVersion LibraryVersion)
+: VirtualUmbrellaHeaderFileName(virtualRoot().str() + 
"UmbrellaHeader.hpp"),
+  TFS(TFS), LibraryVersion(LibraryVersion) {}

sammccall wrote:
> llvm::sys::path::append
don't use .hpp and rely on the driver picking the right language & version, 
this won't generalize. Instead, insert the flags "-xc++-header" and 
"-std=c++14" based on the StandardLibraryVersion



Comment at: clang-tools-extra/clangd/index/StdLib.cpp:48
+  // bild the string only once and cache the results in `*Once`.
+  static std::string Once = [] {
+std::vector Headers;

the static variable must be a `string*` rather than `string` to avoid global 
destructors.



Comment at: clang-tools-extra/clangd/index/StdLib.cpp:72
+  Inputs.TFS = &TFS;
+  // TODO: can we get a real compile command from somewhere?
+  Inputs.CompileCommand.Directory = virtualRoot().str();

I'm not sure what this means, I don't think there's anything better to do here.



Comment at: clang-tools-extra/clangd/index/StdLib.cpp:86
+  HeaderSources, VirtualUmbrellaHeaderFileName,
+  /*RequiresNullTerminator=*/false);
+  assert(Buffer && Buffer->getBufferSize() > 0);

why false? the default (true) is what clang's parser needs



Comment at: clang-tools-extra/clangd/index/StdLib.cpp:97
+
+  // we only care about the symbols, so not storing the other attributes
+  auto Action = createStaticIndexingAction(

pass the default nullptr instead of a no-op lambda, it allows the indexer to 
skip work



Comment at: clang-tools-extra/clangd/index/StdLib.cpp:110
+
+  assert(IndexSlabs.Symbols && "Symbols must be set.");
+

this assertion message doesn't say anything vs the assertion itself, either 
drop it or say why instead



Comment at: clang-tools-extra/clangd/index/StdLib.h:34
+// FIXME: add feature to detect this version somehow (magically).
+enum StandardLibraryVersion { cxx14 = 0 };
+

nit: enum class to avoid polluting namespace.

llvm style capitalizes variables: `CXX14`



Comment at: clang-tools-extra/clangd/index/StdLib.h:34
+// FIXME: add feature to detect this version somehow (magically).
+enum StandardLibraryVersion { cxx14 = 0 };
+

sammccall wrote:
> nit: enum class to avoid polluting namespace.
> 
> llvm style capitalizes variables: `CXX14`
nit: "variant" rather than version to avoid confusion with language version?
(since this will cover c also)



Comment at: clang-tools-extra/clangd/index/StdLib.h:36
+
+// external interface for getting a standard library index.
+Expected>

The comment should be aimed at users of this module, not implementers of it :-)
This is the main API comment...



Comment at: clang-tools-extra/clangd/index/StdLib.h:37
+// external interface for getting a standard library index.
+Expected>
+indexStandardLibrary(const ThreadsafeFS &TFS,

I think I agree with your comment elsewhere that it's sufficient to return 
unique_ptr, indicate it might be null, and log errors.



Comment at: clang-tools-extra/clangd/inde

[PATCH] D104285: [analyzer][AST] Retrieve value by direct index from list initialization of constant array declaration.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@aaron.ballman
Ok, I got your concerns. As I can see we shall only reason about objects within 
the bounds. Otherwise, we shall return `UndefinedVal`.
E.g.:

  int arr[2][5];
  int* ptr1= (int*)arr; // Valid indexing for `ptr` is in range [0,4].
  int* ptr2 = &arr[0][0]; // Same as above.
  ptr1[4]; // Valid object.
  ptr2[5]; // Out of bound. UB. UndefinedVal.

Would it be correct?


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

https://reviews.llvm.org/D104285

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

[PATCH] D108029: [clang][Codegen] Introduce the disable_sanitizer_instrumentation attribute

[Aaron Ballman via Phabricator via cfe-commits]

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

Clang bits LGTM with a few minor documentation nits.




Comment at: clang/include/clang/Basic/AttrDocs.td:2598-2599
+  let Content = [{
+Use the ``disable_sanitizer_instrumentation`` attribute on a function to
+specify that no sanitizer instrumentation should be applied.
+

function, Objective-C method, or global variable.



Comment at: clang/include/clang/Basic/AttrDocs.td:2601-2602
+
+This is not the same as ``__attribute__((no_sanitize(...)))``, which depending
+on the tool may still insert instrumentation to prevent false positive reports.
+  }];




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

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

[clang] ef198cd - [SVE] Remove usage of getMaxVScale for AArch64, in favour of IR Attribute

[Dylan Fleming via cfe-commits]

Author: Dylan Fleming
Date: 2021-08-17T14:42:47+01:00
New Revision: ef198cd99e6bac3a2e87adb6c8a18fb461056fa6

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

LOG: [SVE] Remove usage of getMaxVScale for AArch64, in favour of IR Attribute

Removed AArch64 usage of the getMaxVScale interface, replacing it with
the vscale_range(min, max) IR Attribute.

Reviewed By: paulwalker-arm

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

Added: 


Modified: 
clang/include/clang/Basic/TargetInfo.h
clang/lib/Basic/Targets/AArch64.cpp
clang/lib/Basic/Targets/AArch64.h
clang/lib/CodeGen/CodeGenFunction.cpp
clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c
llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
llvm/test/Analysis/CostModel/AArch64/sve-gather.ll
llvm/test/Analysis/CostModel/AArch64/sve-scatter.ll
llvm/test/Transforms/LoopVectorize/AArch64/first-order-recurrence.ll
llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll
llvm/test/Transforms/LoopVectorize/AArch64/scalable-vectorization.ll
llvm/test/Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll
llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll

Removed: 




diff  --git a/clang/include/clang/Basic/TargetInfo.h 
b/clang/include/clang/Basic/TargetInfo.h
index c7a57a7dba9a8..21289b0dfd04c 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -871,6 +871,11 @@ class TargetInfo : public virtual TransferrableTargetInfo,
   /// across the current set of primary and secondary targets.
   virtual ArrayRef getTargetBuiltins() const = 0;
 
+  /// Returns target-specific min and max values VScale_Range.
+  virtual Optional>
+  getVScaleRange(const LangOptions &LangOpts) const {
+return None;
+  }
   /// The __builtin_clz* and __builtin_ctz* built-in
   /// functions are specified to have undefined results for zero inputs, but
   /// on targets that support these operations in a way that provides

diff  --git a/clang/lib/Basic/Targets/AArch64.cpp 
b/clang/lib/Basic/Targets/AArch64.cpp
index e163ebfa2348b..2b5bf34a7b23f 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -424,6 +424,17 @@ ArrayRef 
AArch64TargetInfo::getTargetBuiltins() const {
  Builtin::FirstTSBuiltin);
 }
 
+Optional>
+AArch64TargetInfo::getVScaleRange(const LangOptions &LangOpts) const {
+  if (LangOpts.ArmSveVectorBits) {
+unsigned VScale = LangOpts.ArmSveVectorBits / 128;
+return std::pair(VScale, VScale);
+  }
+  if (hasFeature("sve"))
+return std::pair(0, 16);
+  return None;
+}
+
 bool AArch64TargetInfo::hasFeature(StringRef Feature) const {
   return Feature == "aarch64" || Feature == "arm64" || Feature == "arm" ||
  (Feature == "neon" && (FPU & NeonMode)) ||

diff  --git a/clang/lib/Basic/Targets/AArch64.h 
b/clang/lib/Basic/Targets/AArch64.h
index 46882a808336b..12830348fb453 100644
--- a/clang/lib/Basic/Targets/AArch64.h
+++ b/clang/lib/Basic/Targets/AArch64.h
@@ -96,6 +96,9 @@ class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public 
TargetInfo {
 
   ArrayRef getTargetBuiltins() const override;
 
+  Optional>
+  getVScaleRange(const LangOptions &LangOpts) const override;
+
   bool hasFeature(StringRef Feature) const override;
   bool handleTargetFeatures(std::vector &Features,
 DiagnosticsEngine &Diags) override;

diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index f5eed8572daa3..dca42045325df 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -484,11 +484,13 @@ void CodeGenFunction::FinishFunction(SourceLocation 
EndLoc) {
   //function.
   CurFn->addFnAttr("min-legal-vector-width", llvm::utostr(LargestVectorWidth));
 
-  // Add vscale attribute if appropriate.
-  if (getLangOpts().ArmSveVectorBits) {
-unsigned VScale = getLangOpts().ArmSveVectorBits / 128;
-CurFn->addFnAttr(llvm::Attribute::getWithVScaleRangeArgs(getLLVMContext(),
- VScale, VScale));
+  // Add vscale_range attribute if appropriate.
+  Optional> VScaleRange =
+  getContext().getTargetInfo().getVScaleRange(getLangOpts());
+

[PATCH] D106277: [SVE] Remove the interface for getMaxVScale in favour of the IR attributes

[Dylan Fleming via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGef198cd99e6b: [SVE] Remove usage of getMaxVScale for 
AArch64, in favour of IR Attribute (authored by DylanFleming-arm).

Changed prior to commit:
  https://reviews.llvm.org/D106277?vs=366001&id=366885#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106277

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/arm-sve-vector-bits-vscale-range.c
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
  llvm/lib/Target/AArch64/AArch64TargetTransformInfo.h
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/test/Analysis/CostModel/AArch64/sve-gather.ll
  llvm/test/Analysis/CostModel/AArch64/sve-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/first-order-recurrence.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-strict-fadd.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vectorization.ll
  llvm/test/Transforms/LoopVectorize/AArch64/scalable-vf-hint.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-cond-inv-loads.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-gather-scatter.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-inv-store.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
  llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll

Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-widen-phi.ll
@@ -12,7 +12,7 @@
 ; that we can use gather instructions with the correct offsets, taking
 ; vscale into account.
 
-define void @widen_ptr_phi_unrolled(i32* noalias nocapture %a, i32* noalias nocapture %b, i32* nocapture readonly %c, i64 %n) {
+define void @widen_ptr_phi_unrolled(i32* noalias nocapture %a, i32* noalias nocapture %b, i32* nocapture readonly %c, i64 %n) #0 {
 ; CHECK-LABEL: @widen_ptr_phi_unrolled(
 ; CHECK:   vector.body:
 ; CHECK-NEXT:[[POINTER_PHI:%.*]] = phi i32* [ %c, %vector.ph ], [ %[[PTR_IND:.*]], %vector.body ]
@@ -122,7 +122,7 @@
 ; because it is stored to memory.
 ;
 
-define i32 @pointer_iv_mixed(i32* noalias %a, i32** noalias %b, i64 %n) {
+define i32 @pointer_iv_mixed(i32* noalias %a, i32** noalias %b, i64 %n) #0 {
 ; CHECK-LABEL: @pointer_iv_mixed(
 ; CHECK: vector.body
 ; CHECK:   %[[IDX:.*]] = phi i64 [ 0, %vector.ph ], [ %{{.*}}, %vector.body ]
@@ -170,7 +170,7 @@
   ret i32 %tmp5
 }
 
-
+attributes #0 = { vscale_range(0, 16) }
 !0 = distinct !{!0, !1, !2, !3, !4, !5}
 !1 = !{!"llvm.loop.mustprogress"}
 !2 = !{!"llvm.loop.vectorize.width", i32 4}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-strict-fadd-cost.ll
@@ -49,7 +49,7 @@
   ret double %add
 }
 
-attributes #0 = { "target-features"="+sve" }
+attributes #0 = { "target-features"="+sve" vscale_range(0, 16) }
 
 !0 = distinct !{!0, !1}
 !1 = !{!"llvm.loop.vectorize.scalable.enable", i1 true}
Index: llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
===
--- llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
+++ llvm/test/Transforms/LoopVectorize/AArch64/sve-large-strides.ll
@@ -1,6 +1,6 @@
 ; RUN: opt -mtriple aarch64-linux-gnu -mattr=+sve -loop-vectorize -scalable-vectorization=on -dce -instcombine -S <%s | FileCheck %s
 
-define void @stride7_i32(i32* noalias nocapture %dst, i64 %n) {
+define void @stride7_i32(i32* noalias nocapture %dst, i64 %n) #0 {
 ; CHECK-LABEL: @stride7_i32(
 ; CHECK:  vector.body
 ; CHECK:%[[VEC_IND:.*]] = phi  [ %{{.*}}, %vector.ph ], [ %{{.*}}, %vector.body ]
@@ -27,7 +27,7 @@
   ret void
 }
 
-define void @stride7_f64(double* noalias nocapture %dst, i64 %n) {
+define void @stride7_f64(double* noalias nocapture %dst, i64 %n) #0 {
 ; CHECK-LABEL: @stride7_f64(
 ; CHECK:  vector.body
 ; CHECK:%[[VEC_IND:.*]] = phi  [ %{{.*}}, %vector.ph ], [ %{{.*}}, %vector.body ]
@@ -55,7 +55,7 @@
 }
 
 
-define void @cond_stride7_f64(double* noalias nocapture %dst, i64* noalias nocapture readonly %cond, i64 %n) {
+define void @cond_stride7_f64(double* noalias nocapture %dst, i64* noalias nocapture readonly %cond, i64 %n) #0 {
 ; CHECK-LABEL: @cond_stride7_f64(
 ; CHECK:  vector.body
 ; CHECK:%[[MASK:.*]] = icmp ne 
@@ -90,7 +90,7 @@
   ret void
 }
 
-
+attributes #0 = { vscale_

[PATCH] D107430: [OMPIRBuilder] Add ordered directive to OMPIRBuilder

[Peixin Qiao via Phabricator via cfe-commits]

peixin added a comment.

@Meinersbur  Thanks for your review and suggestions.

> While manually editing `ordered_codegen.cpp` gives nicer results, re-running 
> `update_cc_test_checks.py` would be less work. Your manual changes would be 
> also be dropped the next time somebody runs update_cc_test_checks.py and 
> commits.

Thanks for the notice. I followed the other OMPIRBuilder PRs in order to make 
review work easily. It should do not matter my manual changes are dropped the 
next time.

> The code seems derived from @fghanim single/master/etc implementation, would 
> be nice if they could have a look.

Yes. He is in the reviewers list.

> The non-OMPBuilder code emits an outlined `__captured_stmt`, but not with 
> OMPBuilder enabled. I assume this is due to the missing `finatlize` call.

Sorry about the misguide. It is not due to missing `finalize` call. My last 
version of patch only implements the code for `ordered threads`. So I use it 
for `ordered simd` test. The non-OMPIRBuilder code emits the outlined function 
call for `ordered simd`, while emits the inlined statements for `ordered 
threads`. Now the non-OMPIRBuilder code and OMPIRBuilder code generate the 
similar IRs.




Comment at: clang/lib/Sema/SemaOpenMP.cpp:5323-5325
+Range = AssertSuccess(Actions.BuildBinOp(
+nullptr, {}, BO_Add, Range,
+Actions.ActOnIntegerConstant(SourceLocation(), 1).get()));

Meinersbur wrote:
> Haven't really understood why this is necessary, but this new version LGTM.
This is the problem of doing operation ++(Expr A - Expr B), which should be 
replaced with (Expr A - Expr B + "1"). To understand why it is not supportedin 
clang sema, you may need to look at the function stack calls, which I listed as 
follows:

```
SemaOpenMP.cpp: BuildUnaryOp(…Expr…)->
SemaExpr.cpp: 
BuildUnaryOp()->CreateBuiltinUnaryOp()->CheckIncrementDecrementOperand() 
->CheckForModifiableLvalue() {
  Expr::isModifiableLvalueResult IsLV = E->isModifiableLvalue(S.Context, &Loc);
  case Expr::MLV_ClassTemporary:
  DiagID = diag::err_typecheck_expression_not_modifiable_lvalue;
}
```
The root reason is that the temporary expression (Expr A - Expr B) is not 
modifiable LValue. I revised the commit message. Please take a look at it and 
check if it is ok for you.



Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:2154
+  Builder.restoreIP(
+  OMPBuilder.createOrdered(Builder, BodyGenCB, FiniCB, false));
+

Meinersbur wrote:
> Did you intend to pass IsThreads=true. Currently it is failing because no 
> `__kmpc_ordered` is emitted.
Yes. Thanks for pointing out the problem.



Comment at: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp:2156-2157
+
+  Value *EntryBBTI = EntryBB->getTerminator();
+  EXPECT_EQ(EntryBBTI, nullptr);
+

Meinersbur wrote:
> Consider emitting a terminator, call `finalize()` and `verifyModule`.
Why do you want me to emit the terminator? If it is because you think the 
outlined captured function is not generated due to finalize call, there is no 
need. Discussed above. Sorry about the misguide.


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

https://reviews.llvm.org/D107430

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

[PATCH] D108029: [clang][Codegen] Introduce the disable_sanitizer_instrumentation attribute

[Marco Elver via Phabricator via cfe-commits]

melver added inline comments.



Comment at: llvm/include/llvm/IR/Attributes.td:90
+/// Do not instrument function with sanitizers.
+def DisableSanitizerInstrumentation: 
EnumAttr<"disable_sanitizer_instrumentation", [FnAttr]>;
+

There's this long-tail of changes required for adding new keywords to the IR. 
Have a look at https://reviews.llvm.org/D102772 -- things that I see currently 
missing are various tests etc.



Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:631
 return bitc::ATTR_KIND_COLD;
+  case Attribute::DisableSanitizerInstrumentation:
+return bitc::ATTR_DISABLE_SANITIZER_INSTRUMENTATION;

There's also BitcodeReader, which needs something similar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

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

[PATCH] D108029: [clang][Codegen] Introduce the disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider updated this revision to Diff 366887.
glider added a comment.

Fix the docs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/attr-no-sanitize-coverage.c
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  llvm/include/llvm/AsmParser/LLToken.h
  llvm/include/llvm/Bitcode/LLVMBitCodes.h
  llvm/include/llvm/IR/Attributes.td
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Transforms/Utils/CodeExtractor.cpp

Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -943,6 +943,7 @@
   // Those attributes should be safe to propagate to the extracted function.
   case Attribute::AlwaysInline:
   case Attribute::Cold:
+  case Attribute::DisableSanitizerInstrumentation:
   case Attribute::Hot:
   case Attribute::NoRecurse:
   case Attribute::InlineHint:
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -628,6 +628,8 @@
 return bitc::ATTR_KIND_IN_ALLOCA;
   case Attribute::Cold:
 return bitc::ATTR_KIND_COLD;
+  case Attribute::DisableSanitizerInstrumentation:
+return bitc::ATTR_DISABLE_SANITIZER_INSTRUMENTATION;
   case Attribute::Hot:
 return bitc::ATTR_KIND_HOT;
   case Attribute::ElementType:
Index: llvm/include/llvm/IR/Attributes.td
===
--- llvm/include/llvm/IR/Attributes.td
+++ llvm/include/llvm/IR/Attributes.td
@@ -86,6 +86,9 @@
 def DereferenceableOrNull : IntAttr<"dereferenceable_or_null",
 [ParamAttr, RetAttr]>;
 
+/// Do not instrument function with sanitizers.
+def DisableSanitizerInstrumentation: EnumAttr<"disable_sanitizer_instrumentation", [FnAttr]>;
+
 /// Provide pointer element type to intrinsic.
 def ElementType : TypeAttr<"elementtype", [ParamAttr]>;
 
Index: llvm/include/llvm/Bitcode/LLVMBitCodes.h
===
--- llvm/include/llvm/Bitcode/LLVMBitCodes.h
+++ llvm/include/llvm/Bitcode/LLVMBitCodes.h
@@ -671,6 +671,7 @@
   ATTR_KIND_SWIFT_ASYNC = 75,
   ATTR_KIND_NO_SANITIZE_COVERAGE = 76,
   ATTR_KIND_ELEMENTTYPE = 77,
+  ATTR_DISABLE_SANITIZER_INSTRUMENTATION = 78,
 };
 
 enum ComdatSelectionKindCodes {
Index: llvm/include/llvm/AsmParser/LLToken.h
===
--- llvm/include/llvm/AsmParser/LLToken.h
+++ llvm/include/llvm/AsmParser/LLToken.h
@@ -190,6 +190,7 @@
   kw_convergent,
   kw_dereferenceable,
   kw_dereferenceable_or_null,
+  kw_disable_sanitizer_instrumentation,
   kw_elementtype,
   kw_inaccessiblememonly,
   kw_inaccessiblemem_or_argmemonly,
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -56,6 +56,7 @@
 // CHECK-NEXT: DLLExport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: DLLImport (SubjectMatchRule_function, SubjectMatchRule_variable, SubjectMatchRule_record, SubjectMatchRule_objc_interface)
 // CHECK-NEXT: Destructor (SubjectMatchRule_function)
+// CHECK-NEXT: DisableSanitizerInstrumentation (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: DisableTailCalls (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: EnableIf (SubjectMatchRule_function)
 // CHECK-NEXT: EnforceTCB (SubjectMatchRule_function)
Index: clang/test/CodeGen/attr-no-sanitize-coverage.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-no-sanitize-coverage.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -debug-info-kind=limited %s -emit-llvm -o - | FileCheck %s
+
+void t1() __attribute__((disable_sanitizer_instrumentation)) {
+}
+// CHECK: disable_sanitizer_instrumentation
+// CHECK-NEXT: void @t1
+
+// CHECK-NOT: disable_sanitizer_instrumentation
+// CHECK: void @t2
+void t2() {
+}
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2286,6 +2286,10 @@
   /// instrumented with __cyg_profile_func_* calls
   bool ShouldInstrumentFunction();
 
+  /// ShouldSkipSanitizerInstr

[PATCH] D108029: [clang][Codegen] Introduce the disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

glider added inline comments.



Comment at: llvm/include/llvm/IR/Attributes.td:90
+/// Do not instrument function with sanitizers.
+def DisableSanitizerInstrumentation: 
EnumAttr<"disable_sanitizer_instrumentation", [FnAttr]>;
+

melver wrote:
> There's this long-tail of changes required for adding new keywords to the IR. 
> Have a look at https://reviews.llvm.org/D102772 -- things that I see 
> currently missing are various tests etc.
It's ridiculous that we have so many handwritten files that list all the 
attributes (all those llvm.vim etc)
But I'll definitely need to update BitcodeReader and the tests. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108029

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

[PATCH] D105267: [X86] AVX512FP16 instructions enabling 4/6

[LuoYuanke via Phabricator via cfe-commits]

LuoYuanke added inline comments.



Comment at: clang/include/clang/Basic/BuiltinsX86.def:1897
+
+TARGET_BUILTIN(__builtin_ia32_rndscaleph_128_mask, "V8xV8xIiV8xUc", 
"ncV:128:", "avx512fp16,avx512vl")
+TARGET_BUILTIN(__builtin_ia32_rndscaleph_256_mask, "V16xV16xIiV16xUs", 
"ncV:256:", "avx512fp16,avx512vl")

The naming convention is not consistent. Rename it to rndscaleph128?



Comment at: clang/include/clang/Basic/BuiltinsX86.def:1898
+TARGET_BUILTIN(__builtin_ia32_rndscaleph_128_mask, "V8xV8xIiV8xUc", 
"ncV:128:", "avx512fp16,avx512vl")
+TARGET_BUILTIN(__builtin_ia32_rndscaleph_256_mask, "V16xV16xIiV16xUs", 
"ncV:256:", "avx512fp16,avx512vl")
+TARGET_BUILTIN(__builtin_ia32_rndscaleph_mask, "V32xV32xIiV32xUiIi", 
"ncV:512:", "avx512fp16")

Ditto.



Comment at: clang/include/clang/Basic/BuiltinsX86.def:1899
+TARGET_BUILTIN(__builtin_ia32_rndscaleph_256_mask, "V16xV16xIiV16xUs", 
"ncV:256:", "avx512fp16,avx512vl")
+TARGET_BUILTIN(__builtin_ia32_rndscaleph_mask, "V32xV32xIiV32xUiIi", 
"ncV:512:", "avx512fp16")
+TARGET_BUILTIN(__builtin_ia32_reduceph128_mask, "V8xV8xIiV8xUc", "ncV:128:", 
"avx512fp16,avx512vl")

rndscaleph512?



Comment at: clang/include/clang/Basic/BuiltinsX86.def:1906
+TARGET_BUILTIN(__builtin_ia32_getmantsh_round_mask, "V8xV8xV8xIiV8xUcIi", 
"ncV:128:", "avx512fp16")
+TARGET_BUILTIN(__builtin_ia32_getexpsh128_round_mask, "V8xV8xV8xV8xUcIi", 
"ncV:128:", "avx512fp16")
+TARGET_BUILTIN(__builtin_ia32_scalefsh_round_mask, "V8xV8xV8xV8xUcIi", 
"ncV:128:", "avx512fp16")

The name convention is not consistent for scalar version. getexpsh?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105267

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

[PATCH] D108132: Add implicit map for a list item appears in a reduction clause.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 updated this revision to Diff 366894.
jyu2 added a comment.
Herald added a project: OpenMP.
Herald added a subscriber: openmp-commits.

Fix format and add new runtime test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108132

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/OpenMP/reduction_implicit_map.cpp
  openmp/libomptarget/test/mapping/reduction_implicit_map.cpp

Index: openmp/libomptarget/test/mapping/reduction_implicit_map.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/reduction_implicit_map.cpp
@@ -0,0 +1,28 @@
+// RUN: %libomptarget-compilexx-run-and-check-generic
+
+// amdgcn does not have printf definition
+// XFAIL: amdgcn-amd-amdhsa
+
+#include 
+
+void sum(int* input, int size, int* output)
+{
+#pragma omp target teams distribute parallel for reduction(+:output[0]) \
+ map(to:input[0:size])
+  for (int i = 0; i < size; i++)
+output[0] += input[i];
+}
+int main()
+{
+  const int size = 100;
+  int *array = new int[size];
+  int result = 0;
+  for (int i = 0; i < size; i++)
+array[i] = i + 1;
+  sum(array, size, &result);
+  // CHECK: Result=5050
+  printf("Result=%d\n", result);
+  delete[] array;
+  return 0;
+}
+
Index: clang/test/OpenMP/reduction_implicit_map.cpp
===
--- /dev/null
+++ clang/test/OpenMP/reduction_implicit_map.cpp
@@ -0,0 +1,119 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-cuda-mode -x c++ \
+// RUN:  -triple powerpc64le-unknown-unknown -DCUDA \
+// RUN:  -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o \
+// RUN:  %t-ppc-host.bc
+
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-cuda-mode -x c++ \
+// RUN:  -triple nvptx64-unknown-unknown -DCUA \
+// RUN:  -fopenmp-targets=nvptx64-nvidia-cuda -DCUDA -emit-llvm %s \
+// RUN:  -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc \
+// RUN:  -o - | FileCheck %s --check-prefix CHECK
+
+// RUN: %clang_cc1 -verify -fopenmp -x c++ \
+// RUN:   -triple powerpc64le-unknown-unknown -DDIAG\
+// RUN:   -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm \
+// RUN:   %s -o - | FileCheck  %s \
+// RUN:   --check-prefix=CHECK1
+
+// RUN: %clang_cc1 -verify -fopenmp -x c++ \
+// RUN:   -triple i386-unknown-unknown \
+// RUN:   -fopenmp-targets=i386-pc-linux-gnu -emit-llvm \
+// RUN:   %s -o - | FileCheck  %s \
+// RUN:   --check-prefix=CHECK2
+
+// expected-no-diagnostics
+
+#if defined(CUDA)
+int foo(int n) {
+  double *e;
+  //no error and no implicit map generated for e[:1]
+  #pragma omp target parallel reduction(+: e[:1])
+*e=10;
+  ;
+  return 0;
+}
+// CHECK-NOT @.offload_maptypes
+// CHECK: call void @__kmpc_nvptx_end_reduce_nowait(
+#elif defined(DIAG)
+class S2 {
+  mutable int a;
+public:
+  S2():a(0) { }
+  S2(S2 &s2):a(s2.a) { }
+  S2 &operator +(S2 &s);
+};
+int bar() {
+ S2 o[5];
+  //no warnig "copyable and not guaranteed to be mapped correctly" and
+  //implicit map generated.
+#pragma omp target reduction(+:o[0])
+  for (int i = 0; i < 10; i++);
+  double b[10][10][10];
+  //no error no implicit map generated, the map for b is generated but not
+  //for b[0:2][2:4][1].
+#pragma omp target parallel for reduction(task, +: b[0:2][2:4][1])
+  for (long long i = 0; i < 10; ++i);
+  return 0;
+}
+// map for variable o
+// CHECK1: offload_sizes = private unnamed_addr constant [1 x i64] [i64 20]
+// CHECK1: offload_maptypes = private unnamed_addr constant [1 x i64] [i64 547]
+// map for b:
+// CHECK1: @.offload_sizes.1 = private unnamed_addr constant [1 x i64] [i64 8000]
+// CHECK1: @.offload_maptypes.2 = private unnamed_addr constant [1 x i64] [i64 547]
+#else
+// generate implicit map for array elements or array sections in reduction
+// clause. In following case: the implicit map is generate for output[0]
+// with map size 4 and output[:3] with map size 12.
+void sum(int* input, int size, int* output)
+{
+#pragma omp target teams distribute parallel for reduction(+: output[0]) \
+ map(to: input [0:size])
+  for (int i = 0; i < size; i++)
+output[0] += input[i];
+#pragma omp target teams distribute parallel for reduction(+: output[:3])  \
+ map(to: input [0:size])
+  for (int i = 0; i < size; i++)
+output[0] += input[i];
+  int a[10];
+#pragma omp target parallel reduction(+: a[:2])
+  for (int i = 0; i < size; i++)
+;
+#pragma omp target parallel reduction(+: a[3])
+  for (int i = 0; i < size; i++)
+;
+}
+//CHECK2: @.offload_sizes = private unnamed_addr constant [2 x i64] [i64 4, i64 8]
+//CHECK2: @.offload_maptypes.10 = private unnamed_addr constant [2 x i64] [i64 800, i64 547]
+//CHECK2: @.offload_sizes.13 = private unnamed_addr constant [

[PATCH] D104285: [analyzer][AST] Retrieve value by direct index from list initialization of constant array declaration.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D104285#2949273 , @ASDenysPetrov 
wrote:

> @aaron.ballman
> Ok, I got your concerns.

Thanks for sticking with me!

> As I can see we shall only reason about objects within the bounds. Otherwise, 
> we shall return `UndefinedVal`.
> E.g.:
>
>   int arr[2][5];
>   int* ptr1= (int*)arr; // Valid indexing for `ptr` is in range [0,4].
>   int* ptr2 = &arr[0][0]; // Same as above.
>   ptr1[4]; // Valid object.
>   ptr2[5]; // Out of bound. UB. UndefinedVal.
>
> Would it be correct?

I believe so, yes (with a caveat below). I also believe this holds (reversing 
the pointer bases):

  ptr2[4]; // valid object
  ptr1[5]; // out of bounds

I've been staring at the C standard for a while, and I think the situation is 
also UB in C. As with C++, the array subscript operators are rewritten to be 
pointer arithmetic using addition (6.5.2.1p2). Additive operators says 
(6.5.6p9) in part: `... If both the pointer operand and the result point to 
elements of the same array object, or one past the last element of the array 
object, the evaluation shall not produce an overflow; otherwise the behavior is 
undefined. If the result points to one past the last element of the array 
object, it shall not be used as the operand of a unary * operator that is 
evaluated.` I believe we run afoul of "the same array object" and "one past the 
last element" clauses because multidimensional arrays are defined to be arrays 
of arrays (6.7.6.2).

Complicating matters somewhat, I would also say that your use of `[5]` is not 
technically out of bounds, but is a one-past-the-end that's then dereferenced 
as part of the subscript rewriting. So it's technically fine to form the 
pointer to the one-past-the-end element, but it's not okay to dereference it. 
That matters for things like:

  int arr[2][5] = {0};
  const int* ptr2 = &arr[0][0];
  const int* end = ptr2 + 5;
  
  for (; ptr2 < end; ++ptr2) {
int whatever = *ptr2;
  }

where `end` is fine because it's never dereferenced. This distinction may 
matter to the static analyzer because a one-past-the-end pointer is valid for 
performing arithmetic on, but an out-of-bounds pointer is not.


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

https://reviews.llvm.org/D104285

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

[PATCH] D108132: Add implicit map for a list item appears in a reduction clause.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:5825
+  for (Expr *E : RC->varlists())
+if (!dyn_cast(E))
+  ImplicitExprs.emplace_back(E);

`isa`. Also, what if this is a `MemberExpr`?



Comment at: clang/lib/Sema/SemaOpenMP.cpp:18475-18476
+  !QTy.isTriviallyCopyableType(SemaRef.Context)) {
+if (NoDiagnose)
+  return true;
 SemaRef.Diag(SL, diag::warn_omp_non_trivial_type_mapped) << QTy << SR;

It still might be good to emit the warning if the reduction type is mappable. 
Also, need to extend this check in general and avoid emitting it if the user 
defined mapper for the type is provided (in a separate patch, not here).



Comment at: clang/lib/Sema/SemaOpenMP.cpp:19435
 SemaRef, SimpleExpr, CurComponents, CKind, DSAS->getCurrentDirective(),
-/*NoDiagnose=*/false);
+/*NoDiagnose=*/NoDiagnose);
 if (!BE)

You can remove `/*NoDiagnose=*/` here.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:19481-19482
 if (VD && DSAS->isThreadPrivate(VD)) {
+  if (NoDiagnose)
+continue;
   DSAStackTy::DSAVarData DVar = DSAS->getTopDSA(VD, /*FromParent=*/false);

Hmm, should not we still diagnose this case?



Comment at: openmp/libomptarget/test/mapping/reduction_implicit_map.cpp:4
+// amdgcn does not have printf definition
+// XFAIL: amdgcn-amd-amdhsa
+

Can we make UNSUPPORTED instead of XFAIL?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108132

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

[PATCH] D107450: [clang-tidy] Fix wrong FIxIt in performance-move-const-arg

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

Quuxplusone added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/performance-move-const-arg.cpp:263
+  forwardToShowInt(std::move(a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: std::move of the variable 'a' 
of the trivially-copyable type 'int' has no effect; consider changing 
forwardToShowInt's parameter from 'int'&& to 'int'&
+}

Quuxplusone wrote:
> `forwardToShowInt` takes `T&&`, not `int&&`, and so it can't be changed in 
> the way the diagnostic is suggesting. I think the right answer here is not to 
> emit the diagnostic at all, when the offending function is a template.
> 
> (Relatively minor nits, all moot because this diagnostic should not be 
> emitted at all: `'int'&&` should be `'int&&'`, `trivially copyable` should 
> not be hyphenated, and `int&` is a strictly worse suggestion than either 
> `const int&` or plain `int`.  IMVHO it would be reasonable to land a very 
> trivial patch to remove the hyphen from `trivially copyable` and update the 
> tests, and then rebase this PR on top of that.)
Okay, I have no //complaints// about the diagnostics being tested by these 
tests anymore.
I am //puzzled// about why the tool is suggesting to rewrite `int&&` as `int`, 
but not suggesting to rewrite `Tmp&&` as `Tmp`; those cases should be exactly 
isomorphic, right?
But the goal of this PR is now to //eliminate wrong fixits//, and I believe 
you've succeeded at that.
Btw, I'm not qualified to review the actual code change in 
`MoveConstArgCheck.cpp`; I'll let someone else do that.


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

https://reviews.llvm.org/D107450

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

[PATCH] D107963: [OpenCL] Fix as_type(vec3) invalid store creation

[JinGu Kang via Phabricator via cfe-commits]

jaykang10 added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4789
-
-if (!CGF.CGM.getCodeGenOpts().PreserveVec3Type) {
-  Src = createCastsForTypeOfSameSize(Builder, CGF.CGM.getDataLayout(), Src,

svenvh wrote:
> Anastasia wrote:
> > While I agree with this fix and it obviously looks incorrect, I wonder if 
> > the original intent was to condition the previous statement instead so that 
> > we avoid converting to size 4 at all? Although I have a feeling we are 
> > entering the behavior that is not documented anywhere. In the spec I can 
> > see this:
> > 
> > 
> > ```
> > When the operand and result type contain a different number of elements, 
> > the result shall be implementation-defined except if the operand is a 
> > 4-component vector and the result is a 3-component vector. In this case, 
> > the bits in the operand shall be returned directly without modification as 
> > the new type. 
> > ```
> > 
> > but it seems to cover the inverse conversion?
> Yeah I have a similar fix for the inverse case (which is further down in this 
> function) in my local branch.
> 
> I did try to extend the guard to also cover the `ConvertVec3AndVec4` call, 
> but that also led to invalid StoreInst creation.  Since I wasn't sure about 
> the intent of the conditioning on `PreserveVec3Type` here, I didn't 
> investigate further.
> 
> I was hoping @jaykang10 (who added this in D30810) might have some insight 
> into why the guard was here in the first place.  But it has been over 4 years 
> since that was committed, so there might not be a ready answer.  Either way, 
> I'll hold off committing this for a few more days.
I am sorry for late response. I has not been feeling well.

As far as I remember, the goal was to avoid bitcast and keep load or store with 
vec3 type on IR level. I guess I did not consider the conversion from vec3 type 
to scalar type and vice versa.

I guess this guard was to avoid the bitcast. It could be wrong for scalar type. 
If you check the scalar type in the guard, it could be good to keep existing 
behavior for vector type.

Additionally, you could also want to change below code for conversion from 
non-vec3 to vec3.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107963

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

[PATCH] D105167: [analyzer] Fix HTML report deduplication.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov accepted this revision.
ASDenysPetrov added a comment.

Nice work! Unfortunately I'm not able to run tests on my Windows env, but I've 
run you tests files manually. It works for me.

P.S. BTW, is there any workarounds to make current tests supported on Windows? 
I know there is //REQUIRES// instruction 
(https://llvm.org/docs/TestingGuide.html#constraining-test-execution) but I 
didn't a sufficient description of it.




Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:286-287
+  // but the stable report filename is still more verbose.
+  // We should rename the option ("verbose" filename?) but it will break
+  // people's workflows.
+  if (DiagOpts.ShouldWriteStableReportFilename) {

vsavchenko wrote:
> Can we make a mirror for this option and mark the other one as deprecated?
Nice idea. I'm in favor of a mirorring.



Comment at: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp:300
+
+  filename << StringRef(getIssueHash(D, PP)).substr(0, 6).str() << ".html";
+  llvm::sys::path::append(ResultPath, Directory, filename.str());

Do you think 6 trimmed characters are enough to avoid collisions?


Repository:
  rC Clang

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

https://reviews.llvm.org/D105167

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

[PATCH] D108211: Emit sizeof/alignof values as notes when a static_assert fails

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: courbet, Quuxplusone, aaron.ballman.
arichardson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Trying to debug a static_assert(sizeof(foo) == ...) failure can be rather
awkward since there is no easy way to print the value of the sizeof() at
compile-time without another compiler diagnostic involving an array size.
This patch emits additional notes when a static_assert fails for any
UnaryExprOrTypeTraitExpr expression inside the static_assert.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108211

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp
  clang/test/Parser/objc-static-assert.mm
  clang/test/Sema/static-assert.c
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/static-assert-cxx17.cpp
  clang/test/SemaCXX/static-assert.cpp
  clang/test/SemaTemplate/overload-candidates.cpp

Index: clang/test/SemaTemplate/overload-candidates.cpp
===
--- clang/test/SemaTemplate/overload-candidates.cpp
+++ clang/test/SemaTemplate/overload-candidates.cpp
@@ -62,6 +62,7 @@
 
 template struct NonTemplateFunction {
   typename boost::enable_if::type f(); // expected-error{{failed requirement 'sizeof(char) == 4'; 'enable_if' cannot be used to disable this declaration}}
+  // expected-note@-1{{with 'sizeof(char)' equal to 1}}
 };
 NonTemplateFunction NTFC; // expected-note{{here}}
 
Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -21,8 +21,10 @@
 T<1> t1; // expected-note {{in instantiation of template class 'T<1>' requested here}}
 T<2> t2;
 
-template struct S {
-static_assert(sizeof(T) > sizeof(char), "Type not big enough!"); // expected-error {{static_assert failed due to requirement 'sizeof(char) > sizeof(char)' "Type not big enough!"}}
+template  struct S {
+  static_assert(sizeof(T) > sizeof(char), "Type not big enough!"); // expected-error {{static_assert failed due to requirement 'sizeof(char) > sizeof(char)' "Type not big enough!"}}
+   // expected-note@-1{{with 'sizeof(char)' equal to 1}}
+   // expected-note@-2{{with 'sizeof(char)' equal to 1}}
 };
 
 S s1; // expected-note {{in instantiation of template class 'S' requested here}}
@@ -199,3 +201,18 @@
 constexpr NotBool constexprNotBool;
 static_assert(notBool, "message");  // expected-error {{value of type 'struct NotBool' is not contextually convertible to 'bool'}}
 static_assert(constexprNotBool, "message"); // expected-error {{value of type 'const NotBool' is not contextually convertible to 'bool'}}
+
+struct IntAndPointer {
+  int i;
+  void *p;
+};
+static_assert(sizeof(IntAndPointer) == 4, "message");
+// expected-error@-1{{static_assert failed due to requirement 'sizeof(IntAndPointer) == 4' "message"}}
+// expected-note@-2{{with 'sizeof(IntAndPointer)' equal to 16}}
+static_assert(alignof(IntAndPointer) == 4, "message");
+// expected-error@-1{{static_assert failed due to requirement 'alignof(IntAndPointer) == 4' "message"}}
+// expected-note@-2{{with 'alignof(IntAndPointer)' equal to 8}}
+static_assert(alignof(IntAndPointer) == sizeof(IntAndPointer), "message");
+// expected-error@-1{{static_assert failed due to requirement 'alignof(IntAndPointer) == sizeof(IntAndPointer)' "message"}}
+// expected-note@-2{{with 'alignof(IntAndPointer)' equal to 8}}
+// expected-note@-3{{with 'sizeof(IntAndPointer)' equal to 16}}
Index: clang/test/SemaCXX/static-assert-cxx17.cpp
===
--- clang/test/SemaCXX/static-assert-cxx17.cpp
+++ clang/test/SemaCXX/static-assert-cxx17.cpp
@@ -89,6 +89,7 @@
   // expected-error@-1{{static_assert failed due to requirement 'int(0)'}}
   static_assert(sizeof(X) == 0);
   // expected-error@-1{{static_assert failed due to requirement 'sizeof(X) == 0'}}
+  // expected-note@-2{{with 'sizeof(X)' equal to 8}}
   static_assert((const X *)nullptr);
   // expected-error@-1{{static_assert failed due to requirement '(const X *)nullptr'}}
   static_assert(static_cast *>(nullptr));
@@ -97,6 +98,7 @@
   // expected-error@-1{{static_assert failed due to requirement '(X const[0]){} == nullptr'}}
   static_assert(sizeof(X().X::~X())>) == 0);
   // expected-error@-1{{static_assert failed due to requirement 'sizeof(X) == 0'}}
+  // expected-note@-2{{with 'sizeof(X)' equal to 8}}
   static_assert(constexpr_return_false());
   // expected-error@-1{{static_assert failed due to requirement 'constexpr_return_false()'}}
 }
Index: c

[PATCH] D108212: Emit offsetof values as notes when a static_assert fails

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: courbet, Quuxplusone, aaron.ballman.
arichardson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Trying to debug a static_assert(offset(foo, field) == ...) failure can be
rather awkward since there is no easy way to print the value at compile
time without another compiler diagnostic involving an array size.
This builds upon the sizeof()/alignof() printing and extends it to handle
OffsetOfExpr.

Depends on D108211 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108212

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaCXX/static-assert.cpp


Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -216,3 +216,10 @@
 // expected-error@-1{{static_assert failed due to requirement 
'alignof(IntAndPointer) == sizeof(IntAndPointer)' "message"}}
 // expected-note@-2{{with 'alignof(IntAndPointer)' equal to 8}}
 // expected-note@-3{{with 'sizeof(IntAndPointer)' equal to 16}}
+#define offsetof(s, f) __builtin_offsetof(s, f)
+static_assert(__builtin_offsetof(IntAndPointer, p) == -1, "message");
+// expected-error@-1{{static_assert failed due to requirement 
'__builtin_offsetof(IntAndPointer, p) == -1' "message"}}
+// expected-note@-2{{with '__builtin_offsetof(IntAndPointer, p)' equal to 8}}
+static_assert(offsetof(IntAndPointer, i) == -1, "message");
+// expected-error@-1{{static_assert failed due to requirement 
'__builtin_offsetof(IntAndPointer, i) == -1' "message"}}
+// expected-note@-2{{with '__builtin_offsetof(IntAndPointer, i)' equal to 0}}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -3577,10 +3577,10 @@
   explicit FailedBooleanConditionPrinterHelper(
   const PrintingPolicy &P, Sema &S,
   SmallVectorImpl &Notes)
-  : Policy(P), S(S), Notes(Notes) {}
+  : Policy(P), SemaRef(S), Notes(Notes) {}
 
-  bool handledStmt(Stmt *E, raw_ostream &OS) override {
-const auto *DR = dyn_cast(E);
+  bool handledStmt(Stmt *S, raw_ostream &OS) override {
+const auto *DR = dyn_cast(S);
 if (DR && DR->getQualifier()) {
   // If this is a qualified name, expand the template arguments in nested
   // qualifiers.
@@ -3595,18 +3595,20 @@
 IV->getSpecializedTemplate()->getTemplateParameters());
   }
   return true;
-} else if (auto *UE = dyn_cast(E)) {
+} else if (isa(S) || isa(S)) {
+  Expr *E = cast(S);
   Expr::EvalResult Result;
-  if (UE->EvaluateAsConstantExpr(Result, S.Context) && Result.Val.isInt()) 
{
+  if (E->EvaluateAsConstantExpr(Result, SemaRef.Context) &&
+  Result.Val.isInt()) {
 std::string ExprStr;
 llvm::raw_string_ostream ExprStream(ExprStr);
-UE->printPretty(ExprStream, nullptr, Policy);
+E->printPretty(ExprStream, nullptr, Policy);
 ExprStream.flush();
 Notes.push_back(PartialDiagnosticAt(
-UE->getExprLoc(),
-S.PDiag(diag::note_static_assert_requirement_context)
+E->getExprLoc(),
+SemaRef.PDiag(diag::note_static_assert_requirement_context)
 << ExprStr << toString(Result.Val.getInt(), 10)
-<< UE->getSourceRange()));
+<< E->getSourceRange()));
   }
 }
 return false;
@@ -3614,7 +3616,7 @@
 
 private:
   const PrintingPolicy Policy;
-  Sema &S;
+  Sema &SemaRef;
   SmallVectorImpl &Notes;
 };
 


Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -216,3 +216,10 @@
 // expected-error@-1{{static_assert failed due to requirement 'alignof(IntAndPointer) == sizeof(IntAndPointer)' "message"}}
 // expected-note@-2{{with 'alignof(IntAndPointer)' equal to 8}}
 // expected-note@-3{{with 'sizeof(IntAndPointer)' equal to 16}}
+#define offsetof(s, f) __builtin_offsetof(s, f)
+static_assert(__builtin_offsetof(IntAndPointer, p) == -1, "message");
+// expected-error@-1{{static_assert failed due to requirement '__builtin_offsetof(IntAndPointer, p) == -1' "message"}}
+// expected-note@-2{{with '__builtin_offsetof(IntAndPointer, p)' equal to 8}}
+static_assert(offsetof(IntAndPointer, i) == -1, "message");
+// expected-error@-1{{static_assert failed due to requirement '__builtin_offsetof(IntAndPointer, i) == -1' "message"}}
+// expected-note@-2{{with '__builtin_offsetof(IntAndPointer, i)' equal to 0}}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -3577,10 +3577,10 @

[PATCH] D105972: Fix __attribute__((annotate("")) with non-zero globals AS

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 366911.
arichardson added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105972

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenTypeCache.h
  clang/test/CodeGen/annotations-global.c


Index: clang/test/CodeGen/annotations-global.c
===
--- clang/test/CodeGen/annotations-global.c
+++ clang/test/CodeGen/annotations-global.c
@@ -4,6 +4,7 @@
 // RUN: FileCheck --check-prefix=BAR %s < %t1
 // RUN: FileCheck --check-prefix=FOOS %s < %t1
 // RUN: FileCheck --check-prefix=ADDRSPACE %s < %t1
+// RUN: %clang_cc1 %s -triple r600 -emit-llvm -o - | FileCheck %s 
--check-prefix AS1-GLOBALS
 // END.
 
 static __attribute((annotate("sfoo_0"))) __attribute((annotate("sfoo_1"))) 
char sfoo;
@@ -45,3 +46,8 @@
 
 // ADDRSPACE: target triple
 // ADDRSPACE: @llvm.global.annotations = appending global {{.*}} addrspacecast 
(i8 addrspace(1)* @addrspace1_var to i8*), {{.*}}
+
+// AS1-GLOBALS: target datalayout = "{{.+}}-A5-G1"
+// AS1-GLOBALS: @llvm.global.annotations = appending addrspace(1) global [11 x 
{ i8 addrspace(1)*, i8 addrspace(1)*, i8 addrspace(1)*, i32, i8 addrspace(1)* }]
+// AS1-GLOBALS-SAME: { i8 addrspace(1)* @a.bar,
+// AS1-GLOBALS-SAME: { i8 addrspace(1)* @addrspace1_var,
Index: clang/lib/CodeGen/CodeGenTypeCache.h
===
--- clang/lib/CodeGen/CodeGenTypeCache.h
+++ clang/lib/CodeGen/CodeGenTypeCache.h
@@ -69,6 +69,12 @@
 llvm::PointerType *AllocaInt8PtrTy;
   };
 
+  /// void* in default globals address space
+  union {
+llvm::PointerType *GlobalsVoidPtrTy;
+llvm::PointerType *GlobalsInt8PtrTy;
+  };
+
   /// The size and alignment of the builtin C type 'int'.  This comes
   /// up enough in various ABI lowering tasks to be worth pre-computing.
   union {
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -130,8 +130,9 @@
 C.getTargetInfo().getMaxPointerWidth());
   Int8PtrTy = Int8Ty->getPointerTo(0);
   Int8PtrPtrTy = Int8PtrTy->getPointerTo(0);
-  AllocaInt8PtrTy = Int8Ty->getPointerTo(
-  M.getDataLayout().getAllocaAddrSpace());
+  const llvm::DataLayout &DL = M.getDataLayout();
+  AllocaInt8PtrTy = Int8Ty->getPointerTo(DL.getAllocaAddrSpace());
+  GlobalsInt8PtrTy = Int8Ty->getPointerTo(DL.getDefaultGlobalsAddressSpace());
   ASTAllocaAddressSpace = getTargetCodeGenInfo().getASTAllocaAddressSpace();
 
   RuntimeCC = getTargetCodeGenInfo().getABIInfo().getRuntimeCC();
@@ -2531,7 +2532,7 @@
 llvm::Constant *CodeGenModule::EmitAnnotationArgs(const AnnotateAttr *Attr) {
   ArrayRef Exprs = {Attr->args_begin(), Attr->args_size()};
   if (Exprs.empty())
-return llvm::ConstantPointerNull::get(Int8PtrTy);
+return llvm::ConstantPointerNull::get(GlobalsInt8PtrTy);
 
   llvm::FoldingSetNodeID ID;
   for (Expr *E : Exprs) {
@@ -2555,7 +2556,7 @@
   ".args");
   GV->setSection(AnnotationSection);
   GV->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
-  auto *Bitcasted = llvm::ConstantExpr::getBitCast(GV, Int8PtrTy);
+  auto *Bitcasted = llvm::ConstantExpr::getBitCast(GV, GlobalsInt8PtrTy);
 
   Lookup = Bitcasted;
   return Bitcasted;
@@ -2570,17 +2571,19 @@
  *LineNoCst = EmitAnnotationLineNo(L),
  *Args = EmitAnnotationArgs(AA);
 
-  llvm::Constant *ASZeroGV = GV;
-  if (GV->getAddressSpace() != 0) {
-ASZeroGV = llvm::ConstantExpr::getAddrSpaceCast(
-   GV, GV->getValueType()->getPointerTo(0));
+  llvm::Constant *GVInGlobalsAS = GV;
+  if (GV->getAddressSpace() !=
+  getDataLayout().getDefaultGlobalsAddressSpace()) {
+GVInGlobalsAS = llvm::ConstantExpr::getAddrSpaceCast(
+GV, GV->getValueType()->getPointerTo(
+getDataLayout().getDefaultGlobalsAddressSpace()));
   }
 
   // Create the ConstantStruct for the global annotation.
   llvm::Constant *Fields[] = {
-  llvm::ConstantExpr::getBitCast(ASZeroGV, Int8PtrTy),
-  llvm::ConstantExpr::getBitCast(AnnoGV, Int8PtrTy),
-  llvm::ConstantExpr::getBitCast(UnitGV, Int8PtrTy),
+  llvm::ConstantExpr::getBitCast(GVInGlobalsAS, GlobalsInt8PtrTy),
+  llvm::ConstantExpr::getBitCast(AnnoGV, GlobalsInt8PtrTy),
+  llvm::ConstantExpr::getBitCast(UnitGV, GlobalsInt8PtrTy),
   LineNoCst,
   Args,
   };


Index: clang/test/CodeGen/annotations-global.c
===
--- clang/test/CodeGen/annotations-global.c
+++ clang/test/CodeGen/annotations-global.c
@@ -4,6 +4,7 @@
 // RUN: FileCheck --check-prefix=BAR %s < %t1
 // RUN: FileCheck --check-prefix=FOOS %s < %t1
 // RUN: FileCheck --check-prefix=ADDRSPACE %s < %t1
+// RU

[PATCH] D107477: [Clang][AST][NFC] Resolve FIXME: Make CXXRecordDecl *Record const.

[Simon Pilgrim via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D107477

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

[PATCH] D104285: [analyzer][AST] Retrieve value by direct index from list initialization of constant array declaration.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@aaron.ballman
Let me speak some thoughts. Consider next:

  int arr[2][5];
  int *ptr1 = &arr[0][0];
  int *ptr2 = &arr[1][0];

The Standard tells that `ptr1[5]` is UB and `ptr2[0]` is a valid object. In 
practice `ptr1` and `ptr2` usually are equal. But the Standard does not 
garantee them to be equal and this depends on a particular implementation. So 
we should rely on that there might be a compiler such that creates every 
subarray disjointed. I think this is an exact excerpt from what our arguing 
actually starts from.


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

https://reviews.llvm.org/D104285

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

[PATCH] D106615: [Clang][LLVM] generate btf_tag annotations for DIComposite types

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

@dblaikie ping. Could you help take a look at the patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106615

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

[clang] 9dabacd - [analyzer] Adjust JS code of analyzer's HTML report for IE support.

[Denys Petrov via cfe-commits]

Author: Denys Petrov
Date: 2021-08-17T19:32:34+03:00
New Revision: 9dabacd09fdd52b5995546794290651c477d3885

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

LOG: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

Summary: Change and replace some functions which IE does not support. This 
patch is made as a continuation of D92928 revision. Also improve hot keys 
behavior.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp 
b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
index 3ee12c0bdf65..c90046ffb413 100644
--- a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -1289,15 +1289,32 @@ var findNum = function() {
 return out;
 };
 
+var classListAdd = function(el, theClass) {
+  if(!el.className.baseVal)
+el.className += " " + theClass;
+  else
+el.className.baseVal += " " + theClass;
+};
+
+var classListRemove = function(el, theClass) {
+  var className = (!el.className.baseVal) ?
+  el.className : el.className.baseVal;
+className = className.replace(" " + theClass, "");
+  if(!el.className.baseVal)
+el.className = className;
+  else
+el.className.baseVal = className;
+};
+
 var scrollTo = function(el) {
 querySelectorAllArray(".selected").forEach(function(s) {
-s.classList.remove("selected");
+  classListRemove(s, "selected");
 });
-el.classList.add("selected");
+classListAdd(el, "selected");
 window.scrollBy(0, el.getBoundingClientRect().top -
 (window.innerHeight / 2));
 highlightArrowsForSelectedEvent();
-}
+};
 
 var move = function(num, up, numItems) {
   if (num == 1 && up || num == numItems - 1 && !up) {
@@ -1332,9 +1349,11 @@ window.addEventListener("keydown", function (event) {
   if (event.defaultPrevented) {
 return;
   }
-  if (event.key == "j") {
+  // key 'j'
+  if (event.keyCode == 74) {
 navigateTo(/*up=*/false);
-  } else if (event.key == "k") {
+  // key 'k'
+  } else if (event.keyCode == 75) {
 navigateTo(/*up=*/true);
   } else {
 return;
@@ -1350,8 +1369,11 @@ StringRef 
HTMLDiagnostics::generateArrowDrawingJavascript() {
 

[PATCH] D107366: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

[Denys Petrov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9dabacd09fdd: [analyzer] Adjust JS code of analyzer's 
HTML report for IE support. (authored by ASDenysPetrov).

Changed prior to commit:
  https://reviews.llvm.org/D107366?vs=364418&id=366923#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107366

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


Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -1289,15 +1289,32 @@
 return out;
 };
 
+var classListAdd = function(el, theClass) {
+  if(!el.className.baseVal)
+el.className += " " + theClass;
+  else
+el.className.baseVal += " " + theClass;
+};
+
+var classListRemove = function(el, theClass) {
+  var className = (!el.className.baseVal) ?
+  el.className : el.className.baseVal;
+className = className.replace(" " + theClass, "");
+  if(!el.className.baseVal)
+el.className = className;
+  else
+el.className.baseVal = className;
+};
+
 var scrollTo = function(el) {
 querySelectorAllArray(".selected").forEach(function(s) {
-s.classList.remove("selected");
+  classListRemove(s, "selected");
 });
-el.classList.add("selected");
+classListAdd(el, "selected");
 window.scrollBy(0, el.getBoundingClientRect().top -
 (window.innerHeight / 2));
 highlightArrowsForSelectedEvent();
-}
+};
 
 var move = function(num, up, numItems) {
   if (num == 1 && up || num == numItems - 1 && !up) {
@@ -1332,9 +1349,11 @@
   if (event.defaultPrevented) {
 return;
   }
-  if (event.key == "j") {
+  // key 'j'
+  if (event.keyCode == 74) {
 navigateTo(/*up=*/false);
-  } else if (event.key == "k") {
+  // key 'k'
+  } else if (event.keyCode == 75) {
 navigateTo(/*up=*/true);
   } else {
 return;
@@ -1350,8 +1369,11 @@
 

[PATCH] D108215: [clang][deps] NFC: Move `ResourceDirectoryCache`

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, arphaman.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The `ResourceDirectoryCache` is useful not only in the `clang-scan-deps` CLI 
tool, but also in the downstream libclang API. This NFC patch moves it to 
`DependencyScanningService.h`. to make it reusable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108215

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningService.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -14,16 +14,12 @@
 #include "clang/Tooling/JSONCompilationDatabase.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/Twine.h"
-#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/JSON.h"
-#include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/Threading.h"
 #include 
-#include 
 
 using namespace clang;
 using namespace tooling::dependencies;
@@ -44,69 +40,6 @@
   raw_ostream &OS;
 };
 
-class ResourceDirectoryCache {
-public:
-  /// findResourceDir finds the resource directory relative to the clang
-  /// compiler being used in Args, by running it with "-print-resource-dir"
-  /// option and cache the results for reuse. \returns resource directory path
-  /// associated with the given invocation command or empty string if the
-  /// compiler path is NOT an absolute path.
-  StringRef findResourceDir(const tooling::CommandLineArguments &Args,
-bool ClangCLMode) {
-if (Args.size() < 1)
-  return "";
-
-const std::string &ClangBinaryPath = Args[0];
-if (!llvm::sys::path::is_absolute(ClangBinaryPath))
-  return "";
-
-const std::string &ClangBinaryName =
-std::string(llvm::sys::path::filename(ClangBinaryPath));
-
-std::unique_lock LockGuard(CacheLock);
-const auto &CachedResourceDir = Cache.find(ClangBinaryPath);
-if (CachedResourceDir != Cache.end())
-  return CachedResourceDir->second;
-
-std::vector PrintResourceDirArgs{ClangBinaryName};
-if (ClangCLMode)
-  PrintResourceDirArgs.push_back("/clang:-print-resource-dir");
-else
-  PrintResourceDirArgs.push_back("-print-resource-dir");
-
-llvm::SmallString<64> OutputFile, ErrorFile;
-llvm::sys::fs::createTemporaryFile("print-resource-dir-output",
-   "" /*no-suffix*/, OutputFile);
-llvm::sys::fs::createTemporaryFile("print-resource-dir-error",
-   "" /*no-suffix*/, ErrorFile);
-llvm::FileRemover OutputRemover(OutputFile.c_str());
-llvm::FileRemover ErrorRemover(ErrorFile.c_str());
-llvm::Optional Redirects[] = {
-{""}, // Stdin
-OutputFile.str(),
-ErrorFile.str(),
-};
-if (const int RC = llvm::sys::ExecuteAndWait(
-ClangBinaryPath, PrintResourceDirArgs, {}, Redirects)) {
-  auto ErrorBuf = llvm::MemoryBuffer::getFile(ErrorFile.c_str());
-  llvm::errs() << ErrorBuf.get()->getBuffer();
-  return "";
-}
-
-auto OutputBuf = llvm::MemoryBuffer::getFile(OutputFile.c_str());
-if (!OutputBuf)
-  return "";
-StringRef Output = OutputBuf.get()->getBuffer().rtrim('\n');
-
-Cache[ClangBinaryPath] = Output.str();
-return Cache[ClangBinaryPath];
-  }
-
-private:
-  std::map Cache;
-  std::mutex CacheLock;
-};
-
 llvm::cl::opt Help("h", llvm::cl::desc("Alias for -help"),
  llvm::cl::Hidden);
 
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningService.cpp
@@ -8,11 +8,65 @@
 
 #include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
 #include "llvm/Support/TargetSelect.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/Program.h"
 
 using namespace clang;
 using namespace tooling;
 using namespace dependencies;
 
+StringRef
+ResourceDirectoryCache::findResourceDir(const CommandLineArguments &Args,
+bool ClangCLMode) {
+  if (Args.size() < 1)
+return "";
+
+  const std::string &ClangBinaryPath = Args[0];
+  if (!llvm::sys::path::is_absolute(ClangBinaryPath))
+return "";
+
+  const std::string &ClangBinaryName =
+  std::string(llvm::sys::path::filename(ClangBinaryPath));
+
+  std::unique_lock LockGuard(CacheLock);

[PATCH] D107963: [OpenCL] Fix as_type(vec3) invalid store creation

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:4789
-
-if (!CGF.CGM.getCodeGenOpts().PreserveVec3Type) {
-  Src = createCastsForTypeOfSameSize(Builder, CGF.CGM.getDataLayout(), Src,

jaykang10 wrote:
> svenvh wrote:
> > Anastasia wrote:
> > > While I agree with this fix and it obviously looks incorrect, I wonder if 
> > > the original intent was to condition the previous statement instead so 
> > > that we avoid converting to size 4 at all? Although I have a feeling we 
> > > are entering the behavior that is not documented anywhere. In the spec I 
> > > can see this:
> > > 
> > > 
> > > ```
> > > When the operand and result type contain a different number of elements, 
> > > the result shall be implementation-defined except if the operand is a 
> > > 4-component vector and the result is a 3-component vector. In this case, 
> > > the bits in the operand shall be returned directly without modification 
> > > as the new type. 
> > > ```
> > > 
> > > but it seems to cover the inverse conversion?
> > Yeah I have a similar fix for the inverse case (which is further down in 
> > this function) in my local branch.
> > 
> > I did try to extend the guard to also cover the `ConvertVec3AndVec4` call, 
> > but that also led to invalid StoreInst creation.  Since I wasn't sure about 
> > the intent of the conditioning on `PreserveVec3Type` here, I didn't 
> > investigate further.
> > 
> > I was hoping @jaykang10 (who added this in D30810) might have some insight 
> > into why the guard was here in the first place.  But it has been over 4 
> > years since that was committed, so there might not be a ready answer.  
> > Either way, I'll hold off committing this for a few more days.
> I am sorry for late response. I has not been feeling well.
> 
> As far as I remember, the goal was to avoid bitcast and keep load or store 
> with vec3 type on IR level. I guess I did not consider the conversion from 
> vec3 type to scalar type and vice versa.
> 
> I guess this guard was to avoid the bitcast. It could be wrong for scalar 
> type. If you check the scalar type in the guard, it could be good to keep 
> existing behavior for vector type.
> 
> Additionally, you could also want to change below code for conversion from 
> non-vec3 to vec3.
No worries, thanks for replying!

> the goal was to avoid bitcast and keep load or store with vec3 type on IR 
> level.

I think that is already achieved by the changes in CGExpr.cpp from your 
previous commit.  But here in CGExprScalar.cpp we are handling the case where 
we have to convert away to non-vec3 (because `NumElementsDst != 3`) and we do 
this conversion unconditionally already.  I don't see why we would not want to 
emit the bitcast because it is needed for correctness.

> It could be wrong for scalar type.

The problem that my patch fixes is not limited to scalar types: it also occurs 
for e.g. `float3` to `double2`.  Perhaps I should add that test case too?

> If you check the scalar type in the guard, it could be good to keep existing 
> behavior for vector type.

My patch does not make a difference to any of the pre-existing tests in 
`preserve_vec3.cl`.  Do you have a specific case that is not covered by the 
test, but for which you want to preserve the behavior?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107963

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

[PATCH] D107775: [Clang][AST] Resolve FIXME: Remove ObjCObjectPointer from isSpecifierType

[Alf via Phabricator via cfe-commits]

gAlfonso-bit updated this revision to Diff 366931.

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

https://reviews.llvm.org/D107775

Files:
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp


Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -303,7 +303,6 @@
   SaveAndRestore PrevPHIsEmpty(HasEmptyPlaceHolder);
 
   // Print qualifiers as appropriate.
-
   bool CanPrefixQualifiers = false;
   bool NeedARCStrongQualifier = false;
   CanPrefixQualifiers = canPrefixQualifiers(T, NeedARCStrongQualifier);
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2785,7 +2785,6 @@
   case DependentTemplateSpecialization:
   case ObjCInterface:
   case ObjCObject:
-  case ObjCObjectPointer: // FIXME: object pointers aren't really specifiers
 return true;
   default:
 return false;
Index: clang/lib/AST/DeclPrinter.cpp
===
--- clang/lib/AST/DeclPrinter.cpp
+++ clang/lib/AST/DeclPrinter.cpp
@@ -153,11 +153,14 @@
   while (!BaseType->isSpecifierType()) {
 if (const PointerType *PTy = BaseType->getAs())
   BaseType = PTy->getPointeeType();
+else if (const ObjCObjectPointerType *OPT =
+ BaseType->getAs())
+  BaseType = OPT->getPointeeType();
 else if (const BlockPointerType *BPy = BaseType->getAs())
   BaseType = BPy->getPointeeType();
-else if (const ArrayType* ATy = dyn_cast(BaseType))
+else if (const ArrayType *ATy = dyn_cast(BaseType))
   BaseType = ATy->getElementType();
-else if (const FunctionType* FTy = BaseType->getAs())
+else if (const FunctionType *FTy = BaseType->getAs())
   BaseType = FTy->getReturnType();
 else if (const VectorType *VTy = BaseType->getAs())
   BaseType = VTy->getElementType();


Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -303,7 +303,6 @@
   SaveAndRestore PrevPHIsEmpty(HasEmptyPlaceHolder);
 
   // Print qualifiers as appropriate.
-
   bool CanPrefixQualifiers = false;
   bool NeedARCStrongQualifier = false;
   CanPrefixQualifiers = canPrefixQualifiers(T, NeedARCStrongQualifier);
Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2785,7 +2785,6 @@
   case DependentTemplateSpecialization:
   case ObjCInterface:
   case ObjCObject:
-  case ObjCObjectPointer: // FIXME: object pointers aren't really specifiers
 return true;
   default:
 return false;
Index: clang/lib/AST/DeclPrinter.cpp
===
--- clang/lib/AST/DeclPrinter.cpp
+++ clang/lib/AST/DeclPrinter.cpp
@@ -153,11 +153,14 @@
   while (!BaseType->isSpecifierType()) {
 if (const PointerType *PTy = BaseType->getAs())
   BaseType = PTy->getPointeeType();
+else if (const ObjCObjectPointerType *OPT =
+ BaseType->getAs())
+  BaseType = OPT->getPointeeType();
 else if (const BlockPointerType *BPy = BaseType->getAs())
   BaseType = BPy->getPointeeType();
-else if (const ArrayType* ATy = dyn_cast(BaseType))
+else if (const ArrayType *ATy = dyn_cast(BaseType))
   BaseType = ATy->getElementType();
-else if (const FunctionType* FTy = BaseType->getAs())
+else if (const FunctionType *FTy = BaseType->getAs())
   BaseType = FTy->getReturnType();
 else if (const VectorType *VTy = BaseType->getAs())
   BaseType = VTy->getElementType();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108083: [PATCH 1/8] [sanitizer] Add hexagon support to sanitizer-common

[Vitaly Buka via Phabricator via cfe-commits]

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

Please correct me if I am wrong, but LLVM does not use [PATCH *] tags for 
series. Please remove them from title and git patches.
You can link dependent patches into Phabricator "stack" above, but most 
extracted patches are orthogonal, and can be summited in any order.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108083

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

[PATCH] D108083: [PATCH 1/8] [sanitizer] Add hexagon support to sanitizer-common

[Vitaly Buka via Phabricator via cfe-commits]

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

Sorry, approved accidentally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108083

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

[PATCH] D104285: [analyzer][AST] Retrieve value by direct index from list initialization of constant array declaration.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D104285#2949638 , @ASDenysPetrov 
wrote:

> @aaron.ballman
> Let me speak some thoughts. Consider next:
>
>   int arr[2][5];
>   int *ptr1 = &arr[0][0];
>   int *ptr2 = &arr[1][0];
>
> The Standard tells that `ptr1[5]` is UB and `ptr2[0]` is a valid object.

Agreed.

> In practice `ptr1` and `ptr2` usually are equal.

Do you mean `&ptr1[5]` and `&ptr2[0]`? If so, I agree they are usually going to 
have the same pointer address at runtime.

> But the Standard does not garantee them to be equal and this depends on a 
> particular implementation. So we should rely on that there might be a 
> compiler such that creates every subarray disjointed. I think this is an 
> exact excerpt from what our arguing actually starts from.

I don't think that compilers will create a disjointed multidimensional array, 
as that would waste space at runtime. However, I do think that *optimizers* are 
getting much smarter about UB situations, saying "that can't happen", and 
basing decisions on it. For example, this touches on pointer provenance which 
is an open area of discussion in LLVM that's still being hammered out (it also 
relates to the C `restrict` keyword). In a provenance world, the pointer has 
more information than just its address; it also knows from where the pointer 
was derived, so you can tell (in the backend) that `&ptr1[5]` and `&ptr2[0]` 
point to *different* objects even if the pointer values are identical. So while 
the runtime layout of the array object may *allow* for these sort of type 
shenanigans with the most obvious implementation strategies for 
multidimensional arrays, the programming language's object model does not allow 
for them and optimizers may do unexpected things.


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

https://reviews.llvm.org/D104285

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

[PATCH] D107887: scan-build-py: Force the opening in utf-8

[Daniel via Phabricator via cfe-commits]

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

Seems reasonable to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107887

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

[PATCH] D107078: [analyzer] Catch leaking stack addresses via stack variables

[Gabor Marton via Phabricator via cfe-commits]

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

It looks good to me, my concerns are addressed. But please wait for @NoQ 's 
approval.


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

https://reviews.llvm.org/D107078

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

[PATCH] D107887: scan-build-py: Force the opening in utf-8

[Petr Hosek via Phabricator via cfe-commits]

phosek accepted this revision.
phosek added a comment.

LGTM




Comment at: clang/tools/scan-build-py/lib/libscanbuild/report.py:420
 
-with open(filename) as handler:
+with open(filename, encoding="utf-8") as handler:
 for line in handler.readlines():

The rest of this file uses `'` rather than `"` for strings, can we keep it 
consistent?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107887

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

[PATCH] D108083: [PATCH 1/8] [sanitizer] Add hexagon support to sanitizer-common

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

In D108083#2948056 , @bcain wrote:

> In D108083#2947914 , @vitalybuka 
> wrote:
>
>> Do we have public bot for hexagon?
>
> I think we have one intended to for checking hexagon-unknown-elf and perhaps 
> not one for hexagon-unknown-linux-musl. I believe that only the Linux target 
> uses the compiler-rt libraries, though.
>
> We can help create a bot for hexagon-unknown-linux-musl. What's the process?

Thank you. https://llvm.org/docs/HowToAddABuilder.html




Comment at: compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp:162
+#elif SANITIZER_LINUX && defined(__hexagon__)
+#include "sanitizer_syscall_linux_hexagon.inc"
 #else

bcain wrote:
> Most of the feedback from `clang-format` checks don't seem to really be the 
> format we use here.  We should probably apply a more specific `.clang-format` 
> file if these aren't useful.
clang-format pre-merge check uses .clang-format files.
If you apply it it locally make sure to use -style=file.

Actually this place is ugly. Current .clang-format will indent when the rest of 
the block is not.
So up to you, keep it as-is, or clang-format entire block 152-165 (ideally in a 
separate patch)



Comment at: compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cpp:25
   return pc + 8;
-#elif defined(__powerpc__) || defined(__arm__) || defined(__aarch64__)
+#elif defined(__powerpc__) || defined(__arm__) || defined(__aarch64__) \
+  || defined(__hexagon__)

but please fix ones like this which do not require significant reformatting of 
unchanged code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108083

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

[PATCH] D108199: [msan] Add support for disable_sanitizer_instrumentation attribute

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis accepted this revision.
eugenis 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/D108199/new/

https://reviews.llvm.org/D108199

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

[PATCH] D107887: scan-build-py: Force the opening in utf-8

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru updated this revision to Diff 366937.
sylvestre.ledru added a comment.

Replace " => '


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107887

Files:
  clang/tools/scan-build-py/lib/libscanbuild/report.py


Index: clang/tools/scan-build-py/lib/libscanbuild/report.py
===
--- clang/tools/scan-build-py/lib/libscanbuild/report.py
+++ clang/tools/scan-build-py/lib/libscanbuild/report.py
@@ -417,7 +417,7 @@
 'bug_path_length': 1
 }
 
-with open(filename) as handler:
+with open(filename, encoding='utf-8') as handler:
 for line in handler.readlines():
 # do not read the file further
 if endsign.match(line):


Index: clang/tools/scan-build-py/lib/libscanbuild/report.py
===
--- clang/tools/scan-build-py/lib/libscanbuild/report.py
+++ clang/tools/scan-build-py/lib/libscanbuild/report.py
@@ -417,7 +417,7 @@
 'bug_path_length': 1
 }
 
-with open(filename) as handler:
+with open(filename, encoding='utf-8') as handler:
 for line in handler.readlines():
 # do not read the file further
 if endsign.match(line):
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108218: Use installed llvm-lit.py instead of lit.py PR-51072

[Yaron Keren via Phabricator via cfe-commits]

yaron.keren created this revision.
yaron.keren requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108218

Files:
  clang/test/utils/update_cc_test_checks/lit.local.cfg


Index: clang/test/utils/update_cc_test_checks/lit.local.cfg
===
--- clang/test/utils/update_cc_test_checks/lit.local.cfg
+++ clang/test/utils/update_cc_test_checks/lit.local.cfg
@@ -1,4 +1,5 @@
 import os
+import glob
 
 import lit.util
 
@@ -19,7 +20,9 @@
 script_path = os.path.join(config.llvm_src_root, 'utils',
'update_cc_test_checks.py')
 assert os.path.isfile(script_path)
-lit = config.llvm_external_lit if config.llvm_external_lit else 
shell_quote(os.path.join(config.llvm_src_root, 'utils', 'lit', 'lit.py'))
+# Windows: llvm-lit.py, Linux: llvm-lit
+llvm_lit = glob.glob(os.path.join(config.llvm_tools_dir, 'llvm-lit*'))[0]
+lit = config.llvm_external_lit if config.llvm_external_lit else 
shell_quote(llvm_lit)
 python = shell_quote(config.python_executable)
 config.substitutions.append(
 ('%update_cc_test_checks', "%s %s %s" % (


Index: clang/test/utils/update_cc_test_checks/lit.local.cfg
===
--- clang/test/utils/update_cc_test_checks/lit.local.cfg
+++ clang/test/utils/update_cc_test_checks/lit.local.cfg
@@ -1,4 +1,5 @@
 import os
+import glob
 
 import lit.util
 
@@ -19,7 +20,9 @@
 script_path = os.path.join(config.llvm_src_root, 'utils',
'update_cc_test_checks.py')
 assert os.path.isfile(script_path)
-lit = config.llvm_external_lit if config.llvm_external_lit else shell_quote(os.path.join(config.llvm_src_root, 'utils', 'lit', 'lit.py'))
+# Windows: llvm-lit.py, Linux: llvm-lit
+llvm_lit = glob.glob(os.path.join(config.llvm_tools_dir, 'llvm-lit*'))[0]
+lit = config.llvm_external_lit if config.llvm_external_lit else shell_quote(llvm_lit)
 python = shell_quote(config.python_executable)
 config.substitutions.append(
 ('%update_cc_test_checks', "%s %s %s" % (
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 70b06fe - scan-build-py: Force the opening in utf-8

[Sylvestre Ledru via cfe-commits]

Author: Sylvestre Ledru
Date: 2021-08-17T19:19:39+02:00
New Revision: 70b06fe8a1862f5b9a0ef4e5e9098c1e457cf275

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

LOG: scan-build-py: Force the opening in utf-8

It fails on ubuntu bionic otherwise with:
```
scan-build-py-14: Run 'scan-view 
/tmp/scan-build-2021-08-09-09-14-36-765350-nx9s888s' to examine bug reports.
scan-build-py-14: Internal error.
Traceback (most recent call last):
  File "/usr/lib/llvm-14/lib/libscanbuild/__init__.py", line 125, in wrapper
return function(*args, **kwargs)
  File "/usr/lib/llvm-14/lib/libscanbuild/analyze.py", line 72, in scan_build
number_of_bugs = document(args)
  File "/usr/lib/llvm-14/lib/libscanbuild/report.py", line 35, in document
for bug in read_bugs(args.output, html_reports_available):
  File "/usr/lib/llvm-14/lib/libscanbuild/report.py", line 282, in read_bugs
for bug in parser(bug_file):
  File "/usr/lib/llvm-14/lib/libscanbuild/report.py", line 421, in 
parse_bug_html
for line in handler.readlines():
  File "/usr/lib/python3.6/encodings/ascii.py", line 26, in decode
return codecs.ascii_decode(input, self.errors)[0]
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 3360: 
ordinal not in range(128)
scan-build-py-14: Please run this command again and turn on verbose mode (add 
'-' as argument).
```

I guess it is caused by a problem in Python 3.6

Reviewed By: phosek, isthismyaccount

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

Added: 


Modified: 
clang/tools/scan-build-py/lib/libscanbuild/report.py

Removed: 




diff  --git a/clang/tools/scan-build-py/lib/libscanbuild/report.py 
b/clang/tools/scan-build-py/lib/libscanbuild/report.py
index 729b25e6350f..0962b636a921 100644
--- a/clang/tools/scan-build-py/lib/libscanbuild/report.py
+++ b/clang/tools/scan-build-py/lib/libscanbuild/report.py
@@ -417,7 +417,7 @@ def parse_bug_html(filename):
 'bug_path_length': 1
 }
 
-with open(filename) as handler:
+with open(filename, encoding='utf-8') as handler:
 for line in handler.readlines():
 # do not read the file further
 if endsign.match(line):



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

[PATCH] D108218: Use installed llvm-lit.py instead of lit.py PR-51072

[Yaron Keren via Phabricator via cfe-commits]

yaron.keren abandoned this revision.
yaron.keren added a comment.

opened by mistake


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108218

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

[PATCH] D107887: scan-build-py: Force the opening in utf-8

[Sylvestre Ledru via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG70b06fe8a186: scan-build-py: Force the opening in utf-8 
(authored by sylvestre.ledru).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107887

Files:
  clang/tools/scan-build-py/lib/libscanbuild/report.py


Index: clang/tools/scan-build-py/lib/libscanbuild/report.py
===
--- clang/tools/scan-build-py/lib/libscanbuild/report.py
+++ clang/tools/scan-build-py/lib/libscanbuild/report.py
@@ -417,7 +417,7 @@
 'bug_path_length': 1
 }
 
-with open(filename) as handler:
+with open(filename, encoding='utf-8') as handler:
 for line in handler.readlines():
 # do not read the file further
 if endsign.match(line):


Index: clang/tools/scan-build-py/lib/libscanbuild/report.py
===
--- clang/tools/scan-build-py/lib/libscanbuild/report.py
+++ clang/tools/scan-build-py/lib/libscanbuild/report.py
@@ -417,7 +417,7 @@
 'bug_path_length': 1
 }
 
-with open(filename) as handler:
+with open(filename, encoding='utf-8') as handler:
 for line in handler.readlines():
 # do not read the file further
 if endsign.match(line):
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108085: Use installed llvm-lit.py instead of lit.py PR-51072

[Yaron Keren via Phabricator via cfe-commits]

yaron.keren updated this revision to Diff 366941.
yaron.keren added a comment.

Switched to Linux repo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108085

Files:
  clang/test/utils/update_cc_test_checks/lit.local.cfg


Index: clang/test/utils/update_cc_test_checks/lit.local.cfg
===
--- clang/test/utils/update_cc_test_checks/lit.local.cfg
+++ clang/test/utils/update_cc_test_checks/lit.local.cfg
@@ -1,4 +1,5 @@
 import os
+import glob
 
 import lit.util
 
@@ -19,7 +20,9 @@
 script_path = os.path.join(config.llvm_src_root, 'utils',
'update_cc_test_checks.py')
 assert os.path.isfile(script_path)
-lit = config.llvm_external_lit if config.llvm_external_lit else 
shell_quote(os.path.join(config.llvm_src_root, 'utils', 'lit', 'lit.py'))
+# Windows: llvm-lit.py, Linux: llvm-lit
+llvm_lit = glob.glob(os.path.join(config.llvm_tools_dir, 'llvm-lit*'))[0]
+lit = config.llvm_external_lit if config.llvm_external_lit else 
shell_quote(llvm_lit)
 python = shell_quote(config.python_executable)
 config.substitutions.append(
 ('%update_cc_test_checks', "%s %s %s" % (


Index: clang/test/utils/update_cc_test_checks/lit.local.cfg
===
--- clang/test/utils/update_cc_test_checks/lit.local.cfg
+++ clang/test/utils/update_cc_test_checks/lit.local.cfg
@@ -1,4 +1,5 @@
 import os
+import glob
 
 import lit.util
 
@@ -19,7 +20,9 @@
 script_path = os.path.join(config.llvm_src_root, 'utils',
'update_cc_test_checks.py')
 assert os.path.isfile(script_path)
-lit = config.llvm_external_lit if config.llvm_external_lit else shell_quote(os.path.join(config.llvm_src_root, 'utils', 'lit', 'lit.py'))
+# Windows: llvm-lit.py, Linux: llvm-lit
+llvm_lit = glob.glob(os.path.join(config.llvm_tools_dir, 'llvm-lit*'))[0]
+lit = config.llvm_external_lit if config.llvm_external_lit else shell_quote(llvm_lit)
 python = shell_quote(config.python_executable)
 config.substitutions.append(
 ('%update_cc_test_checks', "%s %s %s" % (
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108132: Add implicit map for a list item appears in a reduction clause.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 updated this revision to Diff 366945.
jyu2 edited the summary of this revision.
jyu2 added a comment.

Address Alex's comments.  Thanks Alex.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108132

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/OpenMP/reduction_implicit_map.cpp
  openmp/libomptarget/test/mapping/reduction_implicit_map.cpp

Index: openmp/libomptarget/test/mapping/reduction_implicit_map.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/reduction_implicit_map.cpp
@@ -0,0 +1,28 @@
+// RUN: %libomptarget-compilexx-run-and-check-generic
+
+// amdgcn does not have printf definition
+// UNSUPPORTED: amdgcn-amd-amdhsa
+
+#include 
+
+void sum(int* input, int size, int* output)
+{
+#pragma omp target teams distribute parallel for reduction(+:output[0]) \
+ map(to:input[0:size])
+  for (int i = 0; i < size; i++)
+output[0] += input[i];
+}
+int main()
+{
+  const int size = 100;
+  int *array = new int[size];
+  int result = 0;
+  for (int i = 0; i < size; i++)
+array[i] = i + 1;
+  sum(array, size, &result);
+  // CHECK: Result=5050
+  printf("Result=%d\n", result);
+  delete[] array;
+  return 0;
+}
+
Index: clang/test/OpenMP/reduction_implicit_map.cpp
===
--- /dev/null
+++ clang/test/OpenMP/reduction_implicit_map.cpp
@@ -0,0 +1,122 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-cuda-mode -x c++ \
+// RUN:  -triple powerpc64le-unknown-unknown -DCUDA \
+// RUN:  -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o \
+// RUN:  %t-ppc-host.bc
+
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-cuda-mode -x c++ \
+// RUN:  -triple nvptx64-unknown-unknown -DCUA \
+// RUN:  -fopenmp-targets=nvptx64-nvidia-cuda -DCUDA -emit-llvm %s \
+// RUN:  -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc \
+// RUN:  -o - | FileCheck %s --check-prefix CHECK
+
+// RUN: %clang_cc1 -verify -fopenmp -x c++ \
+// RUN:   -triple powerpc64le-unknown-unknown -DDIAG\
+// RUN:   -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm \
+// RUN:   %s -o - | FileCheck  %s \
+// RUN:   --check-prefix=CHECK1
+
+// RUN: %clang_cc1 -verify -fopenmp -x c++ \
+// RUN:   -triple i386-unknown-unknown \
+// RUN:   -fopenmp-targets=i386-pc-linux-gnu -emit-llvm \
+// RUN:   %s -o - | FileCheck  %s \
+// RUN:   --check-prefix=CHECK2
+
+
+#if defined(CUDA)
+// expected-no-diagnostics
+
+int foo(int n) {
+  double *e;
+  //no error and no implicit map generated for e[:1]
+  #pragma omp target parallel reduction(+: e[:1])
+*e=10;
+  ;
+  return 0;
+}
+// CHECK-NOT @.offload_maptypes
+// CHECK: call void @__kmpc_nvptx_end_reduce_nowait(
+#elif defined(DIAG)
+class S2 {
+  mutable int a;
+public:
+  S2():a(0) { }
+  S2(S2 &s2):a(s2.a) { }
+  S2 &operator +(S2 &s);
+};
+int bar() {
+ S2 o[5];
+  //warnig "copyable and not guaranteed to be mapped correctly" and
+  //implicit map generated.
+#pragma omp target parallel reduction(+:o[0]) //expected-warning {{Type 'S2' is not trivially copyable and not guaranteed to be mapped correctly}}
+  for (int i = 0; i < 10; i++);
+  double b[10][10][10];
+  //no error no implicit map generated, the map for b is generated but not
+  //for b[0:2][2:4][1].
+#pragma omp target parallel for reduction(task, +: b[0:2][2:4][1])
+  for (long long i = 0; i < 10; ++i);
+  return 0;
+}
+// map for variable o
+// CHECK1: offload_sizes = private unnamed_addr constant [1 x i64] [i64 4]
+// CHECK1: offload_maptypes = private unnamed_addr constant [1 x i64] [i64 547]
+// map for b:
+// CHECK1: @.offload_sizes{{.*}} = private unnamed_addr constant [1 x i64] [i64 8000]
+// CHECK1: @.offload_maptypes{{.*}} = private unnamed_addr constant [1 x i64] [i64 547]
+#else
+// expected-no-diagnostics
+
+// generate implicit map for array elements or array sections in reduction
+// clause. In following case: the implicit map is generate for output[0]
+// with map size 4 and output[:3] with map size 12.
+void sum(int* input, int size, int* output)
+{
+#pragma omp target teams distribute parallel for reduction(+: output[0]) \
+ map(to: input [0:size])
+  for (int i = 0; i < size; i++)
+output[0] += input[i];
+#pragma omp target teams distribute parallel for reduction(+: output[:3])  \
+ map(to: input [0:size])
+  for (int i = 0; i < size; i++)
+output[0] += input[i];
+  int a[10];
+#pragma omp target parallel reduction(+: a[:2])
+  for (int i = 0; i < size; i++)
+;
+#pragma omp target parallel reduction(+: a[3])
+  for (int i = 0; i < size; i++)
+;
+}
+//CHECK2: @.offload_sizes = private unnamed_addr constant [2 x i64] [i64 4, i64 8]
+//CHECK2: @.offload_maptypes.10 = p

[PATCH] D108150: [Remarks] [AMDGPU] Emit optimization remarks for atomics generating hardware instructions

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108150

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

[PATCH] D108083: [PATCH 1/8] [sanitizer] Add hexagon support to sanitizer-common

[Brian Cain via Phabricator via cfe-commits]

bcain added a comment.

In D108083#2949815 , @vitalybuka 
wrote:

>> We can help create a bot for hexagon-unknown-linux-musl. What's the process?
>
> Thank you. https://llvm.org/docs/HowToAddABuilder.html

Oh I think I misunderstood:

> Volunteers can provide their build machines to work as build workers to 
> public LLVM Buildbot.

I was prepared to write / test / enable a build recipe but I don't have access 
to a public system that can be added as a worker.  But I can work with our team 
to get the ball rolling, it just doesn't seem like it would be a quick process.




Comment at: compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp:162
+#elif SANITIZER_LINUX && defined(__hexagon__)
+#include "sanitizer_syscall_linux_hexagon.inc"
 #else

vitalybuka wrote:
> bcain wrote:
> > Most of the feedback from `clang-format` checks don't seem to really be the 
> > format we use here.  We should probably apply a more specific 
> > `.clang-format` file if these aren't useful.
> clang-format pre-merge check uses .clang-format files.
> If you apply it it locally make sure to use -style=file.
> 
> Actually this place is ugly. Current .clang-format will indent when the rest 
> of the block is not.
> So up to you, keep it as-is, or clang-format entire block 152-165 (ideally in 
> a separate patch)
I did apply clang-format-diff via pre-commit hook and I don't believe it made 
significant changes, but this is after that formatting.  I prefer to keep it 
as-is because the style near here seems to match.



Comment at: compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cpp:25
   return pc + 8;
-#elif defined(__powerpc__) || defined(__arm__) || defined(__aarch64__)
+#elif defined(__powerpc__) || defined(__arm__) || defined(__aarch64__) \
+  || defined(__hexagon__)

vitalybuka wrote:
> but please fix ones like this which do not require significant reformatting 
> of unchanged code.
Ok that's no problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108083

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

[PATCH] D107850: [asan] Implemented custom calling convention similar to the one used by HWASan for X86.

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

This can be split into 3 patches in the following order:

1. x86
2. Instrumentation
3. clang

Each of them needs tests, Intrtumentation changes are not tested.




Comment at: llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h:150
 
+namespace AsanAccessInfo {
+

It's not how enums described here 
https://llvm.org/docs/CodingStandards.html#id43
Also it's common to use enum class nowerdays.


However this one does not need to be enum and just "constexpr size_t"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107850

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

[PATCH] D108083: [PATCH 1/8] [sanitizer] Add hexagon support to sanitizer-common

[Brian Cain via Phabricator via cfe-commits]

bcain added inline comments.



Comment at: compiler-rt/lib/sanitizer_common/sanitizer_stacktrace.cpp:25
   return pc + 8;
-#elif defined(__powerpc__) || defined(__arm__) || defined(__aarch64__)
+#elif defined(__powerpc__) || defined(__arm__) || defined(__aarch64__) \
+  || defined(__hexagon__)

bcain wrote:
> vitalybuka wrote:
> > but please fix ones like this which do not require significant reformatting 
> > of unchanged code.
> Ok that's no problem.
>> but please fix ones like this which do not require significant reformatting 
>> of unchanged code.
> Ok that's no problem.

But it's a little odd -- following the format would pull the `or` operator onto 
the previous line, which would deviate from the style used in the rest of the 
file.

Still happy to make the change, it just doesn't seem like it fits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108083

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

[PATCH] D108215: [clang][deps] NFC: Move `ResourceDirectoryCache`

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

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

I don't think this should be reused. If I'm reading the code correctly, this is 
just a super heavyweight (but cached) call to `Driver::GetResourcesPath()`. It 
should probably be deleted instead.

I suggest instead:

1. Use `Driver::GetResourcesPath()` directly in the libclang application.
2. Figure out if it's safe to do that here as well, and if so, delete this and 
use it here.

Regarding "is it safe?", the question is whether we expect the 
`clang-scan-deps` to be built from the same sources as the `clang` it's 
scanning for. I think we do. We're relying on that already for the results of 
the scan to be correct.




Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:422-423
 if (!HasResourceDir) {
   StringRef ResourceDir =
   ResourceDirCache.findResourceDir(Args, ClangCLMode);
   if (!ResourceDir.empty()) {

This is likely much faster, since it avoids a lock:
```
lang=c++
assert(!Args.empty());
std::string ResourceDir = Driver::GetResourcePath(Args[0]);
```
But if the string operations turn out to be expensive we could add caching on 
top.

Unless, is it supported/expected for `clang-scan-deps` to be from a different 
toolchain than `clang`?



Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:424-427
   if (!ResourceDir.empty()) {
 AdjustedArgs.push_back("-resource-dir");
 AdjustedArgs.push_back(std::string(ResourceDir));
   }

I assume this is for performance (to avoid doing the filesystem search in each 
invocation)? If so, please add a comment explaining that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108215

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

[PATCH] D108223: gn build: Build libclang.so on ELF platforms.

[Peter Collingbourne via Phabricator via cfe-commits]

pcc created this revision.
pcc added a reviewer: thakis.
pcc requested review of this revision.
Herald added a project: LLVM.

This requires changing the ELF build to enable -fPIC, consistent
with other platforms.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108223

Files:
  llvm/utils/gn/build/BUILD.gn
  llvm/utils/gn/secondary/clang/tools/libclang/BUILD.gn


Index: llvm/utils/gn/secondary/clang/tools/libclang/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/tools/libclang/BUILD.gn
+++ llvm/utils/gn/secondary/clang/tools/libclang/BUILD.gn
@@ -5,33 +5,24 @@
 # This build file is just enough to get check-clang to pass, it's missing
 # several things from the CMake build:
 # - a build target copying the Python bindings
-# - the GN linux build always builds without -fPIC (as if LLVM_ENABLE_PIC=OFF
-#   in the CMake build), so libclang is always a static library on linux
 # - the GN build doesn't have LIBCLANG_BUILD_STATIC
 
-libclang_target_type = "shared_library"
-if (host_os != "win" && host_os != "mac") {
-  # ELF targets need -fPIC to build shared libs but they aren't on by default.
-  # For now, make libclang a static lib there.
-  libclang_target_type = "static_library"
-} else {
-  action("linker_script_to_exports") {
-script = "linker-script-to-export-list.py"
-inputs = [ "libclang.map" ]
-outputs = [ "$target_gen_dir/libclang.exports" ]
-args = [
-  rebase_path(inputs[0], root_build_dir),
-  rebase_path(outputs[0], root_build_dir),
-]
-  }
+action("linker_script_to_exports") {
+  script = "linker-script-to-export-list.py"
+  inputs = [ "libclang.map" ]
+  outputs = [ "$target_gen_dir/libclang.exports" ]
+  args = [
+rebase_path(inputs[0], root_build_dir),
+rebase_path(outputs[0], root_build_dir),
+  ]
+}
 
-  symbol_exports("exports") {
-deps = [ ":linker_script_to_exports" ]
-exports_file = "$target_gen_dir/libclang.exports"
-  }
+symbol_exports("exports") {
+  deps = [ ":linker_script_to_exports" ]
+  exports_file = "$target_gen_dir/libclang.exports"
 }
 
-target(libclang_target_type, "libclang") {
+shared_library("libclang") {
   configs += [ "//llvm/utils/gn/build:clang_code" ]
   deps = [
 "//clang/include/clang/Config",
@@ -48,14 +39,17 @@
 "//llvm/lib/Support",
 "//llvm/lib/Target:TargetsToBuild",
   ]
+  if (current_os == "win" || current_os == "mac") {
+deps += [ ":exports" ]
+  } else {
+inputs = [ "libclang.map" ]
+ldflags =
+[ "-Wl,--version-script," + rebase_path(inputs[0], root_build_dir) ]
+  }
   if (clang_enable_arcmt) {
 deps += [ "//clang/lib/ARCMigrate" ]
   }
 
-  if (libclang_target_type == "shared_library") {
-deps += [ ":exports" ]
-  }
-
   defines = []
 
   if (host_os == "win") {
Index: llvm/utils/gn/build/BUILD.gn
===
--- llvm/utils/gn/build/BUILD.gn
+++ llvm/utils/gn/build/BUILD.gn
@@ -377,6 +377,9 @@
 "//llvm/include",
 "$root_gen_dir/llvm/include",
   ]
+  if (current_os != "win") {
+cflags = [ "-fPIC" ]
+  }
 }
 
 config("lld_code") {


Index: llvm/utils/gn/secondary/clang/tools/libclang/BUILD.gn
===
--- llvm/utils/gn/secondary/clang/tools/libclang/BUILD.gn
+++ llvm/utils/gn/secondary/clang/tools/libclang/BUILD.gn
@@ -5,33 +5,24 @@
 # This build file is just enough to get check-clang to pass, it's missing
 # several things from the CMake build:
 # - a build target copying the Python bindings
-# - the GN linux build always builds without -fPIC (as if LLVM_ENABLE_PIC=OFF
-#   in the CMake build), so libclang is always a static library on linux
 # - the GN build doesn't have LIBCLANG_BUILD_STATIC
 
-libclang_target_type = "shared_library"
-if (host_os != "win" && host_os != "mac") {
-  # ELF targets need -fPIC to build shared libs but they aren't on by default.
-  # For now, make libclang a static lib there.
-  libclang_target_type = "static_library"
-} else {
-  action("linker_script_to_exports") {
-script = "linker-script-to-export-list.py"
-inputs = [ "libclang.map" ]
-outputs = [ "$target_gen_dir/libclang.exports" ]
-args = [
-  rebase_path(inputs[0], root_build_dir),
-  rebase_path(outputs[0], root_build_dir),
-]
-  }
+action("linker_script_to_exports") {
+  script = "linker-script-to-export-list.py"
+  inputs = [ "libclang.map" ]
+  outputs = [ "$target_gen_dir/libclang.exports" ]
+  args = [
+rebase_path(inputs[0], root_build_dir),
+rebase_path(outputs[0], root_build_dir),
+  ]
+}
 
-  symbol_exports("exports") {
-deps = [ ":linker_script_to_exports" ]
-exports_file = "$target_gen_dir/libclang.exports"
-  }
+symbol_exports("exports") {
+  deps = [ ":linker_script_to_exports" ]
+  exports_file = "$target_gen_dir/libclang.exports"
 }
 
-target(libclang_target_type, "libclang") {
+shared_library("libclang") {
   configs +=

[PATCH] D108132: Add implicit map for a list item appears in a reduction clause.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 marked 4 inline comments as not done.
jyu2 added a comment.






Comment at: clang/lib/Sema/SemaOpenMP.cpp:5825
+  for (Expr *E : RC->varlists())
+if (!dyn_cast(E))
+  ImplicitExprs.emplace_back(E);

ABataev wrote:
> `isa`. Also, what if this is a `MemberExpr`?
Yes isa.   Changed.  Thanks.

For reduction MemeberExpr is not allowed, it is only allowed variable name, 
array element or array section.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:18475-18476
+  !QTy.isTriviallyCopyableType(SemaRef.Context)) {
+if (NoDiagnose)
+  return true;
 SemaRef.Diag(SL, diag::warn_omp_non_trivial_type_mapped) << QTy << SR;

ABataev wrote:
> It still might be good to emit the warning if the reduction type is mappable. 
> Also, need to extend this check in general and avoid emitting it if the user 
> defined mapper for the type is provided (in a separate patch, not here).
Thanks.  Make sense, in this case, implicit map will be add, so warning should 
be emit. Changed.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:19435
 SemaRef, SimpleExpr, CurComponents, CKind, DSAS->getCurrentDirective(),
-/*NoDiagnose=*/false);
+/*NoDiagnose=*/NoDiagnose);
 if (!BE)

ABataev wrote:
> You can remove `/*NoDiagnose=*/` here.
Changed.  Thanks.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:19481-19482
 if (VD && DSAS->isThreadPrivate(VD)) {
+  if (NoDiagnose)
+continue;
   DSAStackTy::DSAVarData DVar = DSAS->getTopDSA(VD, /*FromParent=*/false);

ABataev wrote:
> Hmm, should not we still diagnose this case?
The rule is skip the error as well as skip adding implicit map clause.  So that 
we don't get regression for old code.



Comment at: openmp/libomptarget/test/mapping/reduction_implicit_map.cpp:4
+// amdgcn does not have printf definition
+// XFAIL: amdgcn-amd-amdhsa
+

ABataev wrote:
> Can we make UNSUPPORTED instead of XFAIL?
changed.  Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108132

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