[PATCH] D132209: [Clang][OpenMP] Make copyin clause on combined and composite construct work

2022-08-22 Thread Yuichiro Utsumi via Phabricator via cfe-commits
yutsumi updated this revision to Diff 454394.
yutsumi added a comment.

add a clang unit test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132209

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/parallel_copyin_combined_codegen.c
  openmp/runtime/test/parallel/omp_parallel_copyin_combined.c

Index: openmp/runtime/test/parallel/omp_parallel_copyin_combined.c
===
--- /dev/null
+++ openmp/runtime/test/parallel/omp_parallel_copyin_combined.c
@@ -0,0 +1,110 @@
+// RUN: %libomp-compile-and-run
+#include "omp_testsuite.h"
+
+#define N 100
+
+int x1, x2, x3, x4, x5;
+#pragma omp threadprivate(x1, x2, x3, x4, x5)
+
+int test_omp_parallel_copyin() {
+  int a[N];
+  x1 = 1;
+
+#pragma omp parallel copyin(x1)
+#pragma omp for
+  for (int i = 0; i < N; i++)
+a[i] = i + x1;
+
+  int sum = 0;
+
+  for (int i = 0; i < N; i++)
+sum += a[i];
+
+  return (sum == ((99 + 2 * x1) * 100) / 2);
+}
+
+int test_omp_parallel_for_copyin() {
+  int a[N];
+  x2 = 2;
+
+#pragma omp parallel for copyin(x2)
+  for (int i = 0; i < N; i++)
+a[i] = i + x2;
+
+  int sum = 0;
+
+  for (int i = 0; i < N; i++)
+sum += a[i];
+
+  return (sum == ((99 + 2 * x2) * 100) / 2);
+}
+
+int test_omp_parallel_for_simd_copyin() {
+  int a[N];
+  x3 = 3;
+
+#pragma omp parallel for simd copyin(x3)
+  for (int i = 0; i < N; i++)
+a[i] = i + x3;
+
+  int sum = 0;
+
+  for (int i = 0; i < N; i++)
+sum += a[i];
+
+  return (sum == ((99 + 2 * x3) * 100) / 2);
+}
+
+int test_omp_parallel_sections_copyin() {
+  int a = 0;
+  int b = 0;
+  x4 = 4;
+
+#pragma omp parallel sections copyin(x4)
+  {
+#pragma omp section
+{ a = x4; }
+
+#pragma omp section
+{ b = x4; }
+  }
+
+  return (a + b == x4 * 2);
+}
+
+int test_omp_parallel_master_copyin() {
+  int a[N];
+  x5 = 5;
+
+#pragma omp parallel master copyin(x5)
+  for (int i = 0; i < N; i++)
+a[i] = i + x5;
+
+  int sum = 0;
+
+  for (int i = 0; i < N; i++)
+sum += a[i];
+
+  return (sum == ((99 + 2 * x5) * 100) / 2);
+}
+
+int main() {
+  int num_failed = 0;
+
+  if (!test_omp_parallel_copyin())
+num_failed++;
+
+  if (!test_omp_parallel_for_copyin())
+num_failed++;
+
+  if (!test_omp_parallel_for_simd_copyin())
+num_failed++;
+
+  if (!test_omp_parallel_sections_copyin())
+num_failed++;
+
+  if (!test_omp_parallel_master_copyin())
+num_failed++;
+
+  return num_failed;
+}
Index: clang/test/OpenMP/parallel_copyin_combined_codegen.c
===
--- /dev/null
+++ clang/test/OpenMP/parallel_copyin_combined_codegen.c
@@ -0,0 +1,532 @@
+// RUN: %clang_cc1 -no-opaque-pointers -verify -fopenmp -x c -triple x86_64-unknown-linux -emit-llvm %s -o - | FileCheck %s
+// expected-no-diagnostics
+
+#define N 100
+
+int x;
+#pragma omp threadprivate(x)
+
+void test_omp_parallel_copyin(int *a) {
+  x = 1;
+
+#pragma omp parallel copyin(x)
+#pragma omp for
+  for (int i = 0; i < N; i++)
+a[i] = i + x;
+}
+
+void test_omp_parallel_for_copyin(int *a) {
+  x = 2;
+
+#pragma omp parallel for copyin(x)
+  for (int i = 0; i < N; i++)
+a[i] = i + x;
+}
+
+void test_omp_parallel_for_simd_copyin(int *a) {
+  x = 3;
+
+#pragma omp parallel for simd copyin(x)
+  for (int i = 0; i < N; i++)
+a[i] = i + x;
+}
+
+void test_omp_parallel_sections_copyin(int *a, int *b) {
+  x = 4;
+
+#pragma omp parallel sections copyin(x)
+  {
+#pragma omp section
+{ *a = x; }
+
+#pragma omp section
+{ *b = x; }
+  }
+}
+
+void test_omp_parallel_master_copyin(int *a) {
+  x = 5;
+
+#pragma omp parallel master copyin(x)
+  for (int i = 0; i < N; i++)
+a[i] = i + x;
+}
+
+// CHECK-LABEL: define {{[^@]+}}@test_omp_parallel_copyin
+// CHECK-SAME: (i32* noundef [[A:%.*]]) #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   [[A_ADDR:%.*]] = alloca i32*, align 8
+// CHECK-NEXT:   store i32* [[A]], i32** [[A_ADDR]], align 8
+// CHECK-NEXT:   [[TMP0:%.*]] = call i32* @llvm.threadlocal.address.p0i32(i32* @x)
+// CHECK-NEXT:   store i32 1, i32* [[TMP0]], align 4
+// CHECK-NEXT:   [[TMP1:%.*]] = call i32* @llvm.threadlocal.address.p0i32(i32* @x)
+// CHECK-NEXT:   call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* @[[GLOB3:[0-9]+]], i32 2, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, i32**, i32*)* @.omp_outlined. to void (i32*, i32*, ...)*), i32** [[A_ADDR]], i32* [[TMP1]])
+// CHECK-NEXT:   ret void
+//
+// CHECK-LABEL: define {{[^@]+}}@.omp_outlined.
+// CHECK-SAME: (i32* noalias noundef [[DOTGLOBAL_TID_:%.*]], i32* noalias noundef [[DOTBOUND_TID_:%.*]], i32** noundef nonnull align 8 dereferenceable(8) [[A:%.*]], i32* noundef nonnull align 4 dereferenceable(4) [[X:%.*]]) #[[ATTR1:[0-9]+]] {
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   [[DOTGLOBAL_TID__ADDR:%.*]] = alloca i32*, align 8
+// CHECK-NEXT:   [[DOTBOUND_TI

[PATCH] D131685: [clang][AST] RecursiveASTVisitor should visit owned TagDecl of friend type.

2022-08-22 Thread Balázs Kéri via Phabricator via cfe-commits
balazske updated this revision to Diff 454408.
balazske marked 4 inline comments as done.
balazske added a comment.

- Removed FIXME comment
- Updated test: check AST exactly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131685

Files:
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/unittests/AST/ASTContextParentMapTest.cpp


Index: clang/unittests/AST/ASTContextParentMapTest.cpp
===
--- clang/unittests/AST/ASTContextParentMapTest.cpp
+++ clang/unittests/AST/ASTContextParentMapTest.cpp
@@ -119,5 +119,34 @@
   Lang_CXX11));
 }
 
+TEST(GetParents, FriendTypeLoc) {
+  auto AST = tooling::buildASTFromCode("struct A { friend struct Fr; };"
+   "struct B { friend struct Fr; };"
+   "struct Fr;");
+  auto &Ctx = AST->getASTContext();
+  auto &TU = *Ctx.getTranslationUnitDecl();
+  auto &A = *TU.lookup(&Ctx.Idents.get("A")).front();
+  auto &B = *TU.lookup(&Ctx.Idents.get("B")).front();
+  auto &FrA = *cast(*++(cast(A).decls_begin()));
+  auto &FrB = *cast(*++(cast(B).decls_begin()));
+  TypeLoc FrALoc = FrA.getFriendType()->getTypeLoc();
+  TypeLoc FrBLoc = FrB.getFriendType()->getTypeLoc();
+  TagDecl *FrATagDecl =
+  FrALoc.getTypePtr()->getAs()->getOwnedTagDecl();
+  TagDecl *FrBTagDecl =
+  FrBLoc.getTypePtr()->getAs()->getOwnedTagDecl();
+
+  EXPECT_THAT(Ctx.getParents(A), ElementsAre(DynTypedNode::create(TU)));
+  EXPECT_THAT(Ctx.getParents(B), ElementsAre(DynTypedNode::create(TU)));
+  EXPECT_THAT(Ctx.getParents(FrA), ElementsAre(DynTypedNode::create(A)));
+  EXPECT_THAT(Ctx.getParents(FrB), ElementsAre(DynTypedNode::create(B)));
+  EXPECT_THAT(Ctx.getParents(FrALoc), ElementsAre(DynTypedNode::create(FrA)));
+  EXPECT_THAT(Ctx.getParents(FrBLoc), ElementsAre(DynTypedNode::create(FrB)));
+  EXPECT_TRUE(FrATagDecl);
+  EXPECT_FALSE(FrBTagDecl);
+  EXPECT_THAT(Ctx.getParents(*FrATagDecl),
+  ElementsAre(DynTypedNode::create(FrA)));
+}
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -1549,10 +1549,15 @@
 
 DEF_TRAVERSE_DECL(FriendDecl, {
   // Friend is either decl or a type.
-  if (D->getFriendType())
+  if (D->getFriendType()) {
 TRY_TO(TraverseTypeLoc(D->getFriendType()->getTypeLoc()));
-  else
+// Traverse any CXXRecordDecl owned by this type, since
+// it will not be in the parent context:
+if (auto *ET = D->getFriendType()->getType()->getAs())
+  TRY_TO(TraverseDecl(ET->getOwnedTagDecl()));
+  } else {
 TRY_TO(TraverseDecl(D->getFriendDecl()));
+  }
 })
 
 DEF_TRAVERSE_DECL(FriendTemplateDecl, {


Index: clang/unittests/AST/ASTContextParentMapTest.cpp
===
--- clang/unittests/AST/ASTContextParentMapTest.cpp
+++ clang/unittests/AST/ASTContextParentMapTest.cpp
@@ -119,5 +119,34 @@
   Lang_CXX11));
 }
 
+TEST(GetParents, FriendTypeLoc) {
+  auto AST = tooling::buildASTFromCode("struct A { friend struct Fr; };"
+   "struct B { friend struct Fr; };"
+   "struct Fr;");
+  auto &Ctx = AST->getASTContext();
+  auto &TU = *Ctx.getTranslationUnitDecl();
+  auto &A = *TU.lookup(&Ctx.Idents.get("A")).front();
+  auto &B = *TU.lookup(&Ctx.Idents.get("B")).front();
+  auto &FrA = *cast(*++(cast(A).decls_begin()));
+  auto &FrB = *cast(*++(cast(B).decls_begin()));
+  TypeLoc FrALoc = FrA.getFriendType()->getTypeLoc();
+  TypeLoc FrBLoc = FrB.getFriendType()->getTypeLoc();
+  TagDecl *FrATagDecl =
+  FrALoc.getTypePtr()->getAs()->getOwnedTagDecl();
+  TagDecl *FrBTagDecl =
+  FrBLoc.getTypePtr()->getAs()->getOwnedTagDecl();
+
+  EXPECT_THAT(Ctx.getParents(A), ElementsAre(DynTypedNode::create(TU)));
+  EXPECT_THAT(Ctx.getParents(B), ElementsAre(DynTypedNode::create(TU)));
+  EXPECT_THAT(Ctx.getParents(FrA), ElementsAre(DynTypedNode::create(A)));
+  EXPECT_THAT(Ctx.getParents(FrB), ElementsAre(DynTypedNode::create(B)));
+  EXPECT_THAT(Ctx.getParents(FrALoc), ElementsAre(DynTypedNode::create(FrA)));
+  EXPECT_THAT(Ctx.getParents(FrBLoc), ElementsAre(DynTypedNode::create(FrB)));
+  EXPECT_TRUE(FrATagDecl);
+  EXPECT_FALSE(FrBTagDecl);
+  EXPECT_THAT(Ctx.getParents(*FrATagDecl),
+  ElementsAre(DynTypedNode::create(FrA)));
+}
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -1549,10 +1549,15 @@
 
 DEF_TRAVERSE_DECL(FriendDecl, {
  

[PATCH] D132316: [CMake] `${LLVM_BINARY_DIR}/lib(${LLVM_LIBDIR_SUFFIX})?` -> `${LLVM_LIBRARY_DIR}`

2022-08-22 Thread Sebastian Neubauer via Phabricator via cfe-commits
sebastian-ne added inline comments.



Comment at: llvm/tools/llvm-shlib/CMakeLists.txt:89
 
-  set(LLVM_EXPORTED_SYMBOL_FILE ${LLVM_BINARY_DIR}/libllvm-c.exports)
 

The lib here is not used as a folder, so this should probably not be replaced?
It should probably include CMAKE_CFG_INTDIR though (i.e. 
`${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/libllvm-c.exports`), to be consistent 
with other places, though this only affects the ninja multi-config build. I’m 
not sure if that works at all and it’s outside of the scope of this patch 
anyway.



Comment at: llvm/tools/llvm-shlib/CMakeLists.txt:91
 
   set(LIB_DIR ${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib${LLVM_LIBDIR_SUFFIX})
   set(LIB_NAME ${LIB_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}LLVM)

Should this be `set(LIB_DIR ${LLVM_LIBRARY_DIR})`?



Comment at: llvm/tools/llvm-shlib/CMakeLists.txt:139
   foreach(lib ${LIB_NAMES})
 list(APPEND FULL_LIB_NAMES 
${LLVM_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib/${lib}.lib)
   endforeach()

This should probably use LLVM_LIBRARY_DIR as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132316

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


[clang-tools-extra] 3c2cb8e - [clangd] Disable IncludeCleaner for ObjC

2022-08-22 Thread Kadir Cetinkaya via cfe-commits

Author: Kadir Cetinkaya
Date: 2022-08-22T11:28:06+02:00
New Revision: 3c2cb8e2f09b4ed48c83bda2551de82f2ded1d1d

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

LOG: [clangd] Disable IncludeCleaner for ObjC

Added: 


Modified: 
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp 
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 0bdf32c6d69dd..4e5758547a12e 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -476,6 +476,9 @@ std::vector issueUnusedIncludesDiagnostics(ParsedAST 
&AST,
   Cfg.Diagnostics.SuppressAll ||
   Cfg.Diagnostics.Suppress.contains("unused-includes"))
 return {};
+  // Interaction is only polished for C/CPP.
+  if (AST.getLangOpts().ObjC)
+return {};
   trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics");
   std::vector Result;
   std::string FileName =

diff  --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp 
b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index d79f1219511f8..9449952b10e80 100644
--- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -7,10 +7,12 @@
 
//===--===//
 
 #include "Annotations.h"
+#include "Config.h"
 #include "IncludeCleaner.h"
 #include "SourceCode.h"
 #include "TestFS.h"
 #include "TestTU.h"
+#include "support/Context.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
@@ -599,6 +601,27 @@ TEST(IncludeCleaner, IWYUPragmaExport) {
   EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
 }
 
+TEST(IncludeCleaner, NoDiagsForObjC) {
+  TestTU TU;
+  TU.Code = R"cpp(
+#include "foo.h"
+
+void bar() {}
+)cpp";
+  TU.AdditionalFiles["foo.h"] = R"cpp(
+#ifndef FOO_H
+#define FOO_H
+
+#endif
+  )cpp";
+  TU.ExtraArgs.emplace_back("-xobjective-c");
+
+  Config Cfg;
+  Cfg.Diagnostics.UnusedIncludes = Config::Strict;
+  WithContextValue Ctx(Config::Key, std::move(Cfg));
+  ParsedAST AST = TU.build();
+  EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
+}
 } // namespace
 } // namespace clangd
 } // namespace clang



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


[PATCH] D131665: [CMake] Support passing arguments to build tool (bootstrap).

2022-08-22 Thread Russell Gallop via Phabricator via cfe-commits
russell.gallop resigned from this revision.
russell.gallop added a comment.

Resign as reviewer as I work with Carlos (and am not familiar enough with the 
details of this area).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131665

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


[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-22 Thread Stanislav Gatev via Phabricator via cfe-commits
sgatev added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:103
+  /// code being analysed.
+  virtual void transferCFGElement(const CFGElement *Element, Lattice &L,
+  Environment &Env) {}

Any reason not to call this one `transfer` too?



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:157
 
+// Holds data structures required for running dataflow analysis.
+struct AnalysisContext {

Let's start struct and member comments with `///`.



Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:184
 ///   All predecessors of `Block` except those with loop back edges must have
-///   already been transferred. States in `BlockStates` that are set to
+///   already been transferred. States in `TC.BlockStates` that are set to
 ///   `llvm::None` represent basic blocks that are not evaluated yet.





Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:312
+/// Transfers `State` by evaluating each element in the `Block` based on the
+/// `TC.Analysis` specified.
+///





Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:314
+///
+/// Built in transfer functions (if the option for `ApplyBuiltinTransfer` is 
set
+/// by the analysis) will be applied to the element before evaluation by the





Comment at: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:328-340
+  switch (Element.getKind()) {
+  case CFGElement::Statement: {
+builtinTransfer(Element.castAs(), State, AC);
+break;
+  }
+  case CFGElement::Initializer: {
+builtinTransfer(Element.castAs(), State);

The level of abstraction for built-in and user code is different in this 
function which makes it hard to follow. Let's move this in a 
`builtinTransfer(const CFGElement &, TypeErasedDataflowAnalysisState &)` 
function that dispatches to one of the other built-in transfer functions. This 
way `transferCFGBlock` won't mix implementation details with the high level 
algorithm.



Comment at: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:341-347
+}
+// User-provided analysis
+AC.Analysis.transferTypeErased(&Element, State.Lattice, State.Env);
+// Post processing
+if (PostVisitCFG) {
+  PostVisitCFG(Element, State);
+}





Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:164-170
+std::function VerifyResults, ArrayRef 
Args,
+const tooling::FileContentMappings &VirtualMappedFiles = {}) {
+
+  std::function
+  PostVisitCFG = nullptr;
+  if (PostVisitStmt) {




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131614

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


[PATCH] D121779: [RISCV] Add zihintntl compressed instructions

2022-08-22 Thread Cao Shun via Phabricator via cfe-commits
alextsao1999 updated this revision to Diff 454433.
alextsao1999 added a comment.

Fix check no c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121779

Files:
  llvm/lib/Target/RISCV/RISCVInstrInfoC.td
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/rv32zihintntl-invalid.s
  llvm/test/MC/RISCV/rv32zihintntlc-invalid.s
  llvm/test/MC/RISCV/rv32zihintntlc-valid.s

Index: llvm/test/MC/RISCV/rv32zihintntlc-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rv32zihintntlc-valid.s
@@ -0,0 +1,52 @@
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+experimental-zihintntl,+c -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+experimental-zihintntl,+c -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+experimental-zihintntl,+c < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zihintntl,+c -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+experimental-zihintntl,+c < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zihintntl,+c -d -r - \
+# RUN: | FileCheck --check-prefix=CHECK-ASM-AND-OBJ %s
+# RUN: not llvm-mc %s -triple=riscv32 -mattr=+experimental-zihintntl 2>&1 | FileCheck -check-prefix=CHECK-NO-C %s
+# RUN: not llvm-mc %s -triple=riscv64 -mattr=+experimental-zihintntl 2>&1 | FileCheck -check-prefix=CHECK-NO-C %s
+
+# CHECK-ASM-AND-OBJ: ntl.p1
+# CHECK-ASM: encoding: [0x33,0x00,0x20,0x00]
+ntl.p1
+
+# CHECK-ASM-AND-OBJ: ntl.pall
+# CHECK-ASM: encoding: [0x33,0x00,0x30,0x00]
+ntl.pall
+
+# CHECK-ASM-AND-OBJ: ntl.s1
+# CHECK-ASM: encoding: [0x33,0x00,0x40,0x00]
+ntl.s1
+
+# CHECK-ASM-AND-OBJ: ntl.all
+# CHECK-ASM: encoding: [0x33,0x00,0x50,0x00]
+ntl.all
+
+# CHECK-ASM-AND-OBJ: c.ntl.p1
+# CHECK-ASM: encoding: [0x0a,0x90]
+# CHECK-NO-C: error: instruction requires the following: 'C' (Compressed Instructions)
+# CHECK-NO-C-NEXT: c.ntl.p1
+c.ntl.p1
+
+# CHECK-ASM-AND-OBJ: c.ntl.pall
+# CHECK-ASM: encoding: [0x0e,0x90]
+# CHECK-NO-C: error: instruction requires the following: 'C' (Compressed Instructions)
+# CHECK-NO-C-NEXT: c.ntl.pall
+c.ntl.pall
+
+# CHECK-ASM-AND-OBJ: c.ntl.s1
+# CHECK-ASM: encoding: [0x12,0x90]
+# CHECK-NO-C: error: instruction requires the following: 'C' (Compressed Instructions)
+# CHECK-NO-C-NEXT: c.ntl.s1
+c.ntl.s1
+
+# CHECK-ASM-AND-OBJ: c.ntl.all
+# CHECK-ASM: encoding: [0x16,0x90]
+# CHECK-NO-C: error: instruction requires the following: 'C' (Compressed Instructions)
+# CHECK-NO-C-NEXT: c.ntl.all
+c.ntl.all
Index: llvm/test/MC/RISCV/rv32zihintntlc-invalid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rv32zihintntlc-invalid.s
@@ -0,0 +1,13 @@
+# RUN: not llvm-mc -triple riscv32 -mattr=+experimental-zihintntl,+c < %s 2>&1 | FileCheck %s
+# RUN: not llvm-mc -triple riscv64 -mattr=+experimental-zihintntl,+c < %s 2>&1 | FileCheck %s
+
+c.ntl.p1 1 # CHECK: :[[@LINE]]:10: error: invalid operand for instruction
+c.ntl.pall 2 # CHECK: :[[@LINE]]:12: error: invalid operand for instruction
+c.ntl.s1 3 # CHECK: :[[@LINE]]:10: error: invalid operand for instruction
+c.ntl.all 4 # CHECK: :[[@LINE]]:11: error: invalid operand for instruction
+
+c.ntl.p1 t0, t1 # CHECK: :[[@LINE]]:10: error: invalid operand for instruction
+c.ntl.pall t0, t1 # CHECK: :[[@LINE]]:12: error: invalid operand for instruction
+c.ntl.s1 t0, t1 # CHECK: :[[@LINE]]:10: error: invalid operand for instruction
+c.ntl.all t0, t1 # CHECK: :[[@LINE]]:11: error: invalid operand for instruction
+
Index: llvm/test/MC/RISCV/rv32zihintntl-invalid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rv32zihintntl-invalid.s
@@ -0,0 +1,13 @@
+# RUN: not llvm-mc -triple riscv32 -mattr=+experimental-zihintntl < %s 2>&1 | FileCheck %s
+# RUN: not llvm-mc -triple riscv64 -mattr=+experimental-zihintntl < %s 2>&1 | FileCheck %s
+
+ntl.p1 1 # CHECK: :[[@LINE]]:8: error: invalid operand for instruction
+ntl.pall 2 # CHECK: :[[@LINE]]:10: error: invalid operand for instruction
+ntl.s1 3 # CHECK: :[[@LINE]]:8: error: invalid operand for instruction
+ntl.all 4 # CHECK: :[[@LINE]]:9: error: invalid operand for instruction
+
+ntl.p1 t0, t1 # CHECK: :[[@LINE]]:8: error: invalid operand for instruction
+ntl.pall t0, t1 # CHECK: :[[@LINE]]:10: error: invalid operand for instruction
+ntl.s1 t0, t1 # CHECK: :[[@LINE]]:8: error: invalid operand for instruction
+ntl.all t0, t1 # CHECK: :[[@LINE]]:9: error: invalid operand for instruction
+
Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-arch.s
+++ llvm/test/MC/RISCV/attribute-arch.s
@@ -185,6 +185,12 @@

[PATCH] D121779: [RISCV] Add zihintntl compressed instructions

2022-08-22 Thread Cao Shun via Phabricator via cfe-commits
alextsao1999 marked an inline comment as done.
alextsao1999 added inline comments.



Comment at: llvm/test/MC/RISCV/rv32zihintntlc-valid.s:42
+# CHECK-ASM: encoding: [0x16,0x90]
+c.ntl.all

kito-cheng wrote:
> Could you add an invalid check for `c.ntl` instruction to make sure they 
> can't use without `C`? 
> e.g. `-mattr=+experimental-zihintntl` but no `+c`.
Done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121779

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


[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-08-22 Thread dongjunduo via Phabricator via cfe-commits
dongjunduo updated this revision to Diff 454439.
dongjunduo added a comment.

Restyle code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/check-time-trace.cpp
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -256,17 +256,10 @@
   llvm::TimerGroup::clearAll();
 
   if (llvm::timeTraceProfilerEnabled()) {
-SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
-llvm::sys::path::replace_extension(Path, "json");
-if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
-  // replace the suffix to '.json' directly
-  SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
-  if (llvm::sys::fs::is_directory(TracePath))
-llvm::sys::path::append(TracePath, llvm::sys::path::filename(Path));
-  Path.assign(TracePath);
-}
+SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
+assert(!TracePath.empty() && "`-ftime-trace=` is empty");
 if (auto profilerOutput = Clang->createOutputFile(
-Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
+TracePath.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
 /*useTemporary=*/false)) {
   llvm::timeTraceProfilerWrite(*profilerOutput);
   profilerOutput.reset();
Index: clang/test/Driver/check-time-trace.cpp
===
--- clang/test/Driver/check-time-trace.cpp
+++ clang/test/Driver/check-time-trace.cpp
@@ -1,3 +1,9 @@
+// RUN: rm -rf %T/exe && mkdir %T/exe
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s -###
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s
+// RUN: cat %T/exe/check-time-trace*.json \
+// RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
 // RUN: %clangxx -S -ftime-trace -ftime-trace-granularity=0 -o %T/check-time-trace %s
 // RUN: cat %T/check-time-trace.json \
 // RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4658,6 +4658,104 @@
   llvm_unreachable("invalid phase in ConstructPhaseAction");
 }
 
+// Infer data storing path of the options `-ftime-trace`, `-ftime-trace=`
+void InferTimeTracePath(Compilation &C) {
+  bool TimeTrace = C.getArgs().getLastArg(options::OPT_ftime_trace) != nullptr;
+  bool TimeTraceFile =
+  C.getArgs().getLastArg(options::OPT_ftime_trace_EQ) != nullptr;
+  // Whether `-ftime-trace` or `-ftime-trace=` are specified
+  if (!TimeTrace && !TimeTraceFile) return;
+
+  // If `-ftime-trace=` is specified, TracePath is the .
+  // Else if there is a linking job, TracePath is the parent path of .exe,
+  // then the OutputFile's name may be appended to it.
+  // Else, TracePath is "",
+  // then the full OutputFile's path may be appended to it.
+  SmallString<128> TracePath("");
+
+  if (TimeTraceFile) {
+TracePath = SmallString<128>(
+C.getArgs().getLastArg(options::OPT_ftime_trace_EQ)->getValue());
+  } else {
+// Get linking executable file's parent path as TracePath's parent path,
+// default is ".". Filename may be determined and added into TracePath then.
+//
+// e.g. executable file's path: /usr/local/a.out
+//  its parent's path:  /usr/local
+for (auto &J : C.getJobs()) {
+  if (J.getSource().getKind() == Action::LinkJobClass &&
+  !J.getOutputFilenames().empty()) {
+auto OutputFilePath = SmallString<128>(J.getOutputFilenames()[0].c_str());
+if (llvm::sys::path::has_parent_path(OutputFilePath)) {
+  TracePath = llvm::sys::path::parent_path(OutputFilePath);
+} else {
+  TracePath = SmallString<128>(".");
+}
+break;
+  }
+}
+  }
+
+  // Add or replace -ftime-trace` to the correct one to all clang jobs
+  for (auto &J : C.getJobs()) {
+if (J.getSource().getKind() == Action::AssembleJobClass ||
+J.getSource().getKind() == Action::BackendJobClass ||
+J.getSource().getKind() == Action::CompileJobClass) {
+  SmallString<128> TracePathReal = TracePath;
+  SmallString<128> OutputPath(J.getOutputFilenames()[0].c_str());
+  std::string arg = std::string("-ftime-trace=");
+  if (!TimeTraceFile) {
+if (TracePathReal.empty()) {
+  // /xxx/yyy.o => /xxx/yyy.json
+  llvm::sys::path::replace_extension(OutputPath, "json");
+  arg += st

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-08-22 Thread dongjunduo via Phabricator via cfe-commits
dongjunduo marked 7 inline comments as done.
dongjunduo added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4674
+for (auto &J : C.getJobs()) {
+  if (J.getSource().getKind() == Action::LinkJobClass &&
+  !J.getOutputFilenames().empty()) {

jamieschmeiser wrote:
> Can you have a link job without an output filename?  If not, then just have 
> an assert for !empty.  Again, the negative test and continue might be easier 
> to understand.
The output filename should not be empty. 

If the "-o" is not specified, the output filename may be "a.out".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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


[PATCH] D130255: [Clang][LoongArch] Add initial LoongArch target and driver support

2022-08-22 Thread Lu Weining via Phabricator via cfe-commits
SixWeining updated this revision to Diff 454446.
SixWeining added a comment.

Use `-fsyntax-only`; `-check-prefix` to `--check-prefix`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130255

Files:
  clang/lib/Basic/CMakeLists.txt
  clang/lib/Basic/Targets.cpp
  clang/lib/Basic/Targets/LoongArch.cpp
  clang/lib/Basic/Targets/LoongArch.h
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/ToolChains/Arch/LoongArch.cpp
  clang/lib/Driver/ToolChains/Arch/LoongArch.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/Clang.h
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/test/Driver/Inputs/multilib_loongarch_linux_sdk/bin/.keep
  clang/test/Driver/Inputs/multilib_loongarch_linux_sdk/include/.keep
  
clang/test/Driver/Inputs/multilib_loongarch_linux_sdk/lib/gcc/loongarch64-unknown-linux-gnu/12.1.0/base/lp64d/crtbegin.o
  
clang/test/Driver/Inputs/multilib_loongarch_linux_sdk/lib/gcc/loongarch64-unknown-linux-gnu/12.1.0/base/lp64f/crtbegin.o
  
clang/test/Driver/Inputs/multilib_loongarch_linux_sdk/lib/gcc/loongarch64-unknown-linux-gnu/12.1.0/base/lp64s/crtbegin.o
  
clang/test/Driver/Inputs/multilib_loongarch_linux_sdk/lib/gcc/loongarch64-unknown-linux-gnu/12.1.0/crtbegin.o
  
clang/test/Driver/Inputs/multilib_loongarch_linux_sdk/loongarch64-unknown-linux-gnu/bin/ld
  
clang/test/Driver/Inputs/multilib_loongarch_linux_sdk/loongarch64-unknown-linux-gnu/lib/.keep
  
clang/test/Driver/Inputs/multilib_loongarch_linux_sdk/loongarch64-unknown-linux-gnu/lib64/.keep
  clang/test/Driver/Inputs/multilib_loongarch_linux_sdk/sysroot/usr/lib/.keep
  clang/test/Driver/Inputs/multilib_loongarch_linux_sdk/sysroot/usr/lib64/.keep
  clang/test/Driver/frame-pointer.c
  clang/test/Driver/loongarch-abi-error.c
  clang/test/Driver/loongarch-abi.c
  clang/test/Driver/loongarch-toolchain.c
  clang/test/Preprocessor/init-loongarch.c

Index: clang/test/Preprocessor/init-loongarch.c
===
--- /dev/null
+++ clang/test/Preprocessor/init-loongarch.c
@@ -0,0 +1,641 @@
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple loongarch32 /dev/null \
+// RUN:   | FileCheck --match-full-lines --check-prefix=LA32 %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple loongarch32-unknown-linux /dev/null \
+// RUN:   | FileCheck --match-full-lines --check-prefixes=LA32,LA32-LINUX %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple loongarch32 \
+// RUN: -fforce-enable-int128 /dev/null | FileCheck --match-full-lines \
+// RUN: --check-prefixes=LA32,LA32-INT128 %s
+
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple loongarch64 /dev/null \
+// RUN:   | FileCheck --match-full-lines --check-prefix=LA64 %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple loongarch64-unknown-linux /dev/null \
+// RUN:   | FileCheck --match-full-lines --check-prefixes=LA64,LA64-LINUX %s
+// RUN: %clang_cc1 -E -dM -ffreestanding -fgnuc-version=4.2.1 -triple loongarch64 \
+// RUN: -fforce-enable-int128 /dev/null | FileCheck --match-full-lines \
+// RUN: --check-prefixes=LA64,LA64-INT128 %s
+
+ Note that common macros are tested in init.c, such as __VERSION__. So they're not listed here.
+
+// LA32: #define _ILP32 1
+// LA32: #define __ATOMIC_ACQUIRE 2
+// LA32-NEXT: #define __ATOMIC_ACQ_REL 4
+// LA32-NEXT: #define __ATOMIC_CONSUME 1
+// LA32-NEXT: #define __ATOMIC_RELAXED 0
+// LA32-NEXT: #define __ATOMIC_RELEASE 3
+// LA32-NEXT: #define __ATOMIC_SEQ_CST 5
+// LA32: #define __BIGGEST_ALIGNMENT__ 16
+// LA32: #define __BITINT_MAXWIDTH__ 128
+// LA32: #define __BOOL_WIDTH__ 8
+// LA32: #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
+// LA32: #define __CHAR16_TYPE__ unsigned short
+// LA32: #define __CHAR32_TYPE__ unsigned int
+// LA32: #define __CHAR_BIT__ 8
+// LA32: #define __CLANG_ATOMIC_BOOL_LOCK_FREE 1
+// LA32: #define __CLANG_ATOMIC_CHAR16_T_LOCK_FREE 1
+// LA32: #define __CLANG_ATOMIC_CHAR32_T_LOCK_FREE 1
+// LA32: #define __CLANG_ATOMIC_CHAR_LOCK_FREE 1
+// LA32: #define __CLANG_ATOMIC_INT_LOCK_FREE 1
+// LA32: #define __CLANG_ATOMIC_LLONG_LOCK_FREE 1
+// LA32: #define __CLANG_ATOMIC_LONG_LOCK_FREE 1
+// LA32: #define __CLANG_ATOMIC_POINTER_LOCK_FREE 1
+// LA32: #define __CLANG_ATOMIC_SHORT_LOCK_FREE 1
+// LA32: #define __CLANG_ATOMIC_WCHAR_T_LOCK_FREE 1
+// LA32: #define __DBL_DECIMAL_DIG__ 17
+// LA32: #define __DBL_DENORM_MIN__ 4.9406564584124654e-324
+// LA32: #define __DBL_DIG__ 15
+// LA32: #define __DBL_EPSILON__ 2.2204460492503131e-16
+// LA32: #define __DBL_HAS_DENORM__ 1
+// LA32: #define __DBL_HAS_INFINITY__ 1
+// LA32: #define __DBL_HAS_QUIET_NAN__ 1
+// LA32: #define __DBL_MANT_DIG__ 53
+// LA32: #define __DBL_MAX_10_EXP__ 308
+// LA32: #define __DBL_MAX_EXP__ 1024
+// LA32: #define __DBL_MAX__ 1.7976931348623157e+308
+// LA32: #define __DBL_MIN_10_EX

[clang] 3c48263 - [analyzer] Remove pattern matching of lambda capture initializers

2022-08-22 Thread via cfe-commits

Author: isuckatcs
Date: 2022-08-22T13:00:31+02:00
New Revision: 3c482632e64c1a3e4967db325562f2e353513abc

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

LOG: [analyzer] Remove pattern matching of lambda capture initializers

Prior to this patch we handled lambda captures based on their
initializer expression, which resulted in pattern matching. With
C++17 copy elision the initializer expression can be anything,
and this approach proved to be fragile and a source of crashes.
This patch removes pattern matching and only checks whether the
object is under construction or not.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/test/Analysis/lambdas.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index d1b7ef7806ba7..c3f7719537789 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -1147,23 +1147,12 @@ void ExprEngine::VisitLambdaExpr(const LambdaExpr *LE, 
ExplodedNode *Pred,
 
   assert(InitExpr && "Capture missing initialization expression");
 
-  if (const auto AILE = dyn_cast(InitExpr)) {
-// If the AILE initializes a POD array, we need to keep it as the
-// InitExpr.
-if (dyn_cast(AILE->getSubExpr()))
-  InitExpr = AILE->getSubExpr();
-  }
-
-  // With C++17 copy elision this can happen.
-  if (const auto *FC = dyn_cast(InitExpr))
-InitExpr = FC->getSubExpr();
-
-  assert(InitExpr &&
- "Extracted capture initialization expression is missing");
-
-  if (dyn_cast(InitExpr)) {
-InitVal = *getObjectUnderConstruction(State, {LE, Idx}, LocCtxt);
-InitVal = State->getSVal(InitVal.getAsRegion());
+  // With C++17 copy elision the InitExpr can be anything, so instead of
+  // pattern matching all cases, we simple check if the current field is
+  // under construction or not, regardless what it's InitExpr is.
+  if (const auto OUC =
+  getObjectUnderConstruction(State, {LE, Idx}, LocCtxt)) {
+InitVal = State->getSVal(OUC->getAsRegion());
 
 State = finishObjectConstruction(State, {LE, Idx}, LocCtxt);
   } else

diff  --git a/clang/test/Analysis/lambdas.cpp b/clang/test/Analysis/lambdas.cpp
index 7a6d35e0d6b21..b1d9784dcc0bf 100644
--- a/clang/test/Analysis/lambdas.cpp
+++ b/clang/test/Analysis/lambdas.cpp
@@ -3,6 +3,8 @@
 // RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,debug.DumpCFG 
-analyzer-config inline-lambdas=true %s > %t 2>&1
 // RUN: FileCheck --input-file=%t %s
 
+#include "Inputs/system-header-simulator-cxx.h"
+
 void clang_analyzer_warnIfReached();
 void clang_analyzer_eval(int);
 
@@ -217,6 +219,13 @@ void testCopyElidedObjectCaptured(int x) {
   }();
 }
 
+static auto MakeUniquePtr() { return std::make_unique>(); }
+
+void testCopyElidedUniquePtr() {
+  [uniquePtr = MakeUniquePtr()] {}();
+  clang_analyzer_warnIfReached(); // expected-warning{{TRUE}}
+}
+
 #endif
 
 // Test inline defensive checks



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


[PATCH] D131944: [analyzer] Remove pattern matching of lambda capture initializers

2022-08-22 Thread Domján Dániel via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3c482632e64c: [analyzer] Remove pattern matching of lambda 
capture initializers (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131944

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/test/Analysis/lambdas.cpp


Index: clang/test/Analysis/lambdas.cpp
===
--- clang/test/Analysis/lambdas.cpp
+++ clang/test/Analysis/lambdas.cpp
@@ -3,6 +3,8 @@
 // RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,debug.DumpCFG 
-analyzer-config inline-lambdas=true %s > %t 2>&1
 // RUN: FileCheck --input-file=%t %s
 
+#include "Inputs/system-header-simulator-cxx.h"
+
 void clang_analyzer_warnIfReached();
 void clang_analyzer_eval(int);
 
@@ -217,6 +219,13 @@
   }();
 }
 
+static auto MakeUniquePtr() { return std::make_unique>(); }
+
+void testCopyElidedUniquePtr() {
+  [uniquePtr = MakeUniquePtr()] {}();
+  clang_analyzer_warnIfReached(); // expected-warning{{TRUE}}
+}
+
 #endif
 
 // Test inline defensive checks
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -1147,23 +1147,12 @@
 
   assert(InitExpr && "Capture missing initialization expression");
 
-  if (const auto AILE = dyn_cast(InitExpr)) {
-// If the AILE initializes a POD array, we need to keep it as the
-// InitExpr.
-if (dyn_cast(AILE->getSubExpr()))
-  InitExpr = AILE->getSubExpr();
-  }
-
-  // With C++17 copy elision this can happen.
-  if (const auto *FC = dyn_cast(InitExpr))
-InitExpr = FC->getSubExpr();
-
-  assert(InitExpr &&
- "Extracted capture initialization expression is missing");
-
-  if (dyn_cast(InitExpr)) {
-InitVal = *getObjectUnderConstruction(State, {LE, Idx}, LocCtxt);
-InitVal = State->getSVal(InitVal.getAsRegion());
+  // With C++17 copy elision the InitExpr can be anything, so instead of
+  // pattern matching all cases, we simple check if the current field is
+  // under construction or not, regardless what it's InitExpr is.
+  if (const auto OUC =
+  getObjectUnderConstruction(State, {LE, Idx}, LocCtxt)) {
+InitVal = State->getSVal(OUC->getAsRegion());
 
 State = finishObjectConstruction(State, {LE, Idx}, LocCtxt);
   } else


Index: clang/test/Analysis/lambdas.cpp
===
--- clang/test/Analysis/lambdas.cpp
+++ clang/test/Analysis/lambdas.cpp
@@ -3,6 +3,8 @@
 // RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,debug.DumpCFG -analyzer-config inline-lambdas=true %s > %t 2>&1
 // RUN: FileCheck --input-file=%t %s
 
+#include "Inputs/system-header-simulator-cxx.h"
+
 void clang_analyzer_warnIfReached();
 void clang_analyzer_eval(int);
 
@@ -217,6 +219,13 @@
   }();
 }
 
+static auto MakeUniquePtr() { return std::make_unique>(); }
+
+void testCopyElidedUniquePtr() {
+  [uniquePtr = MakeUniquePtr()] {}();
+  clang_analyzer_warnIfReached(); // expected-warning{{TRUE}}
+}
+
 #endif
 
 // Test inline defensive checks
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -1147,23 +1147,12 @@
 
   assert(InitExpr && "Capture missing initialization expression");
 
-  if (const auto AILE = dyn_cast(InitExpr)) {
-// If the AILE initializes a POD array, we need to keep it as the
-// InitExpr.
-if (dyn_cast(AILE->getSubExpr()))
-  InitExpr = AILE->getSubExpr();
-  }
-
-  // With C++17 copy elision this can happen.
-  if (const auto *FC = dyn_cast(InitExpr))
-InitExpr = FC->getSubExpr();
-
-  assert(InitExpr &&
- "Extracted capture initialization expression is missing");
-
-  if (dyn_cast(InitExpr)) {
-InitVal = *getObjectUnderConstruction(State, {LE, Idx}, LocCtxt);
-InitVal = State->getSVal(InitVal.getAsRegion());
+  // With C++17 copy elision the InitExpr can be anything, so instead of
+  // pattern matching all cases, we simple check if the current field is
+  // under construction or not, regardless what it's InitExpr is.
+  if (const auto OUC =
+  getObjectUnderConstruction(State, {LE, Idx}, LocCtxt)) {
+InitVal = State->getSVal(OUC->getAsRegion());
 
 State = finishObjectConstruction(State, {LE, Idx}, LocCtxt);
   } else
___
cfe-commits mailing list

[PATCH] D131533: [Flang][Driver] Enable PIC in the frontend

2022-08-22 Thread Andrzej Warzynski via Phabricator via cfe-commits
awarzynski accepted this revision.
awarzynski added a comment.

Thanks for all the updates and for working on this! I'm not an expert in the 
semantics of `-fpie`/`-fpic`/`-mrelocation-model`, but this basically 
replicates the logic in Clang and I am not aware of any good reasons for Flang 
to diverge from that. This looks good to me.

I've left a few minor comments inline and would appreciate if you could address 
them before landing this. Thanks again for taking this on!




Comment at: clang/include/clang/Driver/Options.td:6320-6325
+def pic_level : Separate<["-"], "pic-level">,
+  HelpText<"Value for __PIC__">,
+  MarshallingInfoInt>;
+def pic_is_pie : Flag<["-"], "pic-is-pie">,
+  HelpText<"File is for a position independent executable">,
+  MarshallingInfoFlag>;

MaskRay wrote:
> awarzynski wrote:
> > awarzynski wrote:
> > > These are code-gen options to me. While originally located under 
> > > "Language Options", I think that it would make more sense to move them 
> > > near "CodeGen Options" instead (e.g. near `mrelocation_model`). @MaskRay 
> > > any thoughts?
> > Turns out that in Clang these options are indeed [[ 
> > https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/LangOptions.def#L199
> >  | LangOptions ]]. That's a bit confusing to me, but oh well.
> clang/lib/Frontend/InitPreprocessor.cpp defines `__PIC__`.. IIUC the file 
> does not know CodeGenOptions, so LangOptions isn't a bad choice.
I think that we all agree that these options should remain somewhere withing 
the "Language Options" block, i.e. below:

```
//===--===//
// Language Options
//===--===//
```

As previously, you can use a `let` statement there:
```
let Flags = [CC1Option, FC1Option, NoDriverOption] in {
def pic_level : Separate<["-"], "pic-level">,
  HelpText<"Value for __PIC__">,
  MarshallingInfoInt>;
def pic_is_pie : Flag<["-"], "pic-is-pie">,
  HelpText<"File is for a position independent executable">,
  MarshallingInfoFlag>;

} // let Flags = [CC1Option, FC1Option, NoDriverOption]
```



Comment at: clang/include/clang/Driver/Options.td:5245
+
+} // let Flags = [CC1Option, CC1AsOption, NoDriverOption]
+





Comment at: flang/test/Driver/pic-flags.f90:1
-! Verify that in contrast to Clang, Flang does not default to generating 
position independent executables/code
 




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

https://reviews.llvm.org/D131533

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


[PATCH] D131665: [CMake] Support passing arguments to build tool (bootstrap).

2022-08-22 Thread Andrew Ng via Phabricator via cfe-commits
andrewng resigned from this revision.
andrewng added a comment.

Resign as reviewer as I also work with Carlos and helped to author this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131665

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


[clang] c81bf94 - [analyzer] Handling non-POD multidimensional arrays in ArrayInitLoopExpr

2022-08-22 Thread via cfe-commits

Author: isuckatcs
Date: 2022-08-22T13:53:53+02:00
New Revision: c81bf940c77ee9439eb1f63011e7ecf0e07c56d9

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

LOG: [analyzer] Handling non-POD multidimensional arrays in ArrayInitLoopExpr

This patch makes it possible for lambdas, implicit copy/move ctors
and structured bindings to handle non-POD multidimensional arrays.

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

Added: 


Modified: 
clang/include/clang/AST/ASTContext.h
clang/include/clang/Analysis/CFG.h
clang/lib/AST/ASTContext.cpp
clang/lib/Analysis/CFG.cpp
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
clang/test/Analysis/array-init-loop.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index a01a6aa4bd7e4..2d533b49114b6 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -2731,6 +2731,10 @@ class ASTContext : public RefCountedBase {
   /// Return number of constant array elements.
   uint64_t getConstantArrayElementCount(const ConstantArrayType *CA) const;
 
+  /// Return number of elements initialized in an ArrayInitLoopExpr.
+  uint64_t
+  getArrayInitLoopExprElementCount(const ArrayInitLoopExpr *AILE) const;
+
   /// Perform adjustment on the parameter type of a function.
   ///
   /// This routine adjusts the given parameter type @p T to the actual

diff  --git a/clang/include/clang/Analysis/CFG.h 
b/clang/include/clang/Analysis/CFG.h
index 4f16a6361950e..9212276a3f691 100644
--- a/clang/include/clang/Analysis/CFG.h
+++ b/clang/include/clang/Analysis/CFG.h
@@ -1466,6 +1466,8 @@ class CFG {
   llvm::DenseMap SyntheticDeclStmts;
 };
 
+Expr *extractElementInitializerFromNestedAILE(const ArrayInitLoopExpr *AILE);
+
 } // namespace clang
 
 
//===--===//

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index a4c6a6951279d..8cc70fdc75d15 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -6905,6 +6905,21 @@ ASTContext::getConstantArrayElementCount(const 
ConstantArrayType *CA)  const {
   return ElementCount;
 }
 
+uint64_t ASTContext::getArrayInitLoopExprElementCount(
+const ArrayInitLoopExpr *AILE) const {
+  if (!AILE)
+return 0;
+
+  uint64_t ElementCount = 1;
+
+  do {
+ElementCount *= AILE->getArraySize().getZExtValue();
+AILE = dyn_cast(AILE->getSubExpr());
+  } while (AILE);
+
+  return ElementCount;
+}
+
 /// getFloatingRank - Return a relative rank for floating point types.
 /// This routine will assert if passed a built-in type that isn't a float.
 static FloatingRank getFloatingRank(QualType T) {

diff  --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 31655e43e8993..6131256d4a0d0 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -1332,6 +1332,18 @@ class CFGBuilder {
 
 } // namespace
 
+Expr *
+clang::extractElementInitializerFromNestedAILE(const ArrayInitLoopExpr *AILE) {
+  if (!AILE)
+return nullptr;
+
+  Expr *AILEInit = AILE->getSubExpr();
+  while (const auto *E = dyn_cast(AILEInit))
+AILEInit = E->getSubExpr();
+
+  return AILEInit;
+}
+
 inline bool AddStmtChoice::alwaysAdd(CFGBuilder &builder,
  const Stmt *stmt) const {
   return builder.alwaysAdd(stmt) || kind == AlwaysAdd;
@@ -1706,11 +1718,12 @@ CFGBlock *CFGBuilder::addInitializer(CXXCtorInitializer 
*I) {
   if (Init) {
 // If the initializer is an ArrayInitLoopExpr, we want to extract the
 // initializer, that's used for each element.
-const auto *AILE = dyn_cast_or_null(Init);
+auto *AILEInit = extractElementInitializerFromNestedAILE(
+dyn_cast(Init));
 
 findConstructionContexts(
 ConstructionContextLayer::create(cfg->getBumpVectorContext(), I),
-AILE ? AILE->getSubExpr() : Init);
+AILEInit ? AILEInit : Init);
 
 if (HasTemporaries) {
   // For expression with temporaries go directly to subexpression to omit
@@ -3415,11 +3428,12 @@ CFGBlock *CFGBuilder::VisitLambdaExpr(LambdaExpr *E, 
AddStmtChoice asc) {
 if (Expr *Init = *it) {
   // If the initializer is an ArrayInitLoopExpr, we want to extract the
   // initializer, that's used for each element.
-  const auto *AILE = dyn_cast_or_null(Init);
+  auto *AILEInit = extractElementInitializerFromNestedAILE(
+  dyn_cast(Init));
 
   findConstructionContexts(ConstructionContextLayer::create(
cfg->getBumpVectorContext(), {E, Idx}),
-   AILE ? AILE->getSubExpr() : Init);
+ 

[PATCH] D131840: [analyzer] Handling non-POD multidimensional arrays in ArrayInitLoopExpr

2022-08-22 Thread Domján Dániel via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc81bf940c77e: [analyzer] Handling non-POD multidimensional 
arrays in ArrayInitLoopExpr (authored by isuckatcs).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131840

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Analysis/CFG.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Analysis/CFG.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/test/Analysis/array-init-loop.cpp

Index: clang/test/Analysis/array-init-loop.cpp
===
--- clang/test/Analysis/array-init-loop.cpp
+++ clang/test/Analysis/array-init-loop.cpp
@@ -223,3 +223,86 @@
   clang_analyzer_eval(moved.arr[2].i == 5); // expected-warning{{TRUE}}
   clang_analyzer_eval(moved.arr[3].i == 6); // expected-warning{{TRUE}}
 }
+
+//Note: This is the only solution I could find to check the values without 
+// crashing clang. For more details on the crash see Issue #57135.
+void lambda_capture_multi_array() {
+  S3 arr[2][2] = {1,2,3,4};
+
+  {
+int x = [arr] { return arr[0][0].i; }();
+clang_analyzer_eval(x == 1); // expected-warning{{TRUE}}
+  }
+
+  {
+int x = [arr] { return arr[0][1].i; }();
+clang_analyzer_eval(x == 2); // expected-warning{{TRUE}}
+  }
+
+  {
+int x = [arr] { return arr[1][0].i; }();
+clang_analyzer_eval(x == 3); // expected-warning{{TRUE}}
+  }
+
+  {
+int x = [arr] { return arr[1][1].i; }();
+clang_analyzer_eval(x == 4); // expected-warning{{TRUE}}
+  }
+}
+
+// This struct will force constructor inlining in MultiWrapper.
+struct UserDefinedCtor {
+  int i;
+  UserDefinedCtor() {}
+  UserDefinedCtor(const UserDefinedCtor ©) {
+int j = 1;
+i = copy.i;
+  }
+};
+
+struct MultiWrapper {
+  UserDefinedCtor arr[2][2];
+};
+
+void copy_ctor_multi() {
+  MultiWrapper MW;
+
+  MW.arr[0][0].i = 0;
+  MW.arr[0][1].i = 1;
+  MW.arr[1][0].i = 2;
+  MW.arr[1][1].i = 3;
+
+  MultiWrapper MWCopy = MW;
+  
+  clang_analyzer_eval(MWCopy.arr[0][0].i == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(MWCopy.arr[0][1].i == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(MWCopy.arr[1][0].i == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(MWCopy.arr[1][1].i == 3); // expected-warning{{TRUE}}
+} 
+
+void move_ctor_multi() {
+  MultiWrapper MW;
+
+  MW.arr[0][0].i = 0;
+  MW.arr[0][1].i = 1;
+  MW.arr[1][0].i = 2;
+  MW.arr[1][1].i = 3;
+
+  MultiWrapper MWMove = (MultiWrapper &&) MW;
+  
+  clang_analyzer_eval(MWMove.arr[0][0].i == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(MWMove.arr[0][1].i == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(MWMove.arr[1][0].i == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(MWMove.arr[1][1].i == 3); // expected-warning{{TRUE}}
+} 
+
+void structured_binding_multi() {
+  S3 arr[2][2] = {1,2,3,4};
+
+  auto [a,b] = arr;
+
+  clang_analyzer_eval(a[0].i == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(a[1].i == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(b[0].i == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(b[1].i == 4); // expected-warning{{TRUE}}
+}
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -524,16 +524,32 @@
   //   | `-DeclRefExpr
   //   `-ArrayInitIndexExpr
   //
+  // The resulting expression for a multidimensional array.
+  // ArrayInitLoopExpr  <-- we're here
+  // |-OpaqueValueExpr
+  // | `-DeclRefExpr<-- match this
+  // `-ArrayInitLoopExpr
+  //   |-OpaqueValueExpr
+  //   | `-ArraySubscriptExpr
+  //   |   |-ImplicitCastExpr
+  //   |   | `-OpaqueValueExpr
+  //   |   |   `-DeclRefExpr
+  //   |   `-ArrayInitIndexExpr
+  //   `-CXXConstructExpr <-- extract this
+  // ` ...
+
+  const auto *OVESrc = AILE->getCommonExpr()->getSourceExpr();
+
   // HACK: There is no way we can put the index of the array element into the
   // CFG unless we unroll the loop, so we manually select and bind the required
   // parameter to the environment.
-  const auto *CE = cast(AILE->getSubExpr());
-  const auto *OVESrc = AILE->getCommonExpr()->getSourceExpr();
+  const auto *CE =
+  cast(extractElementInitializerFromNestedAILE(AILE));
 
   SVal Base = UnknownVal();
   if (const auto *ME = dyn_cast(OVESrc))
 Base = State->getSVal(ME, LCtx);
-  else if (const auto *DRE = cast(OVESrc))
+  else if (const auto *DRE = dyn_cast(OVESrc))
 Base = State->getLValue(cast(DRE->getDecl()), LCtx);
   else
 llvm_unreachable("ArrayInitLoopExpr contains unexpected source expression");
@@

[PATCH] D132209: [Clang][OpenMP] Make copyin clause on combined and composite construct work

2022-08-22 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132209

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


[PATCH] D127293: [clang-tidy] Ignore other members in a union if any member of it is initialized in cppcoreguidelines-pro-type-member-init

2022-08-22 Thread gehry via Phabricator via cfe-commits
Sockke added a comment.

Friendly ping @LegalizeAdulthood.


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

https://reviews.llvm.org/D127293

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


[PATCH] D132324: [RFC] Remove support for building libc++ with `LLVM_ENABLE_PROJECTS`

2022-08-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D132324#3737976 , @Ericson2314 
wrote:

> Ah I see email about sphinx jobs defined out of tree :/ I will take a look at 
> that, see if it is easy to fix.

This still hasn't been fixed, so none of the documentation is being updated on 
the website: https://lab.llvm.org/buildbot/#/builders/89/builds/31614 -- if 
it's not easy, please revert until we have a path forward.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132324

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


[PATCH] D132290: [clang-tidy] Skip unions in use-equals-default

2022-08-22 Thread Alexander Shaposhnikov via Phabricator via cfe-commits
alexander-shaposhnikov updated this revision to Diff 454470.
alexander-shaposhnikov added a comment.

Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132290

Files:
  clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
@@ -33,6 +33,15 @@
   ~NE() { f(); }
 };
 
+// Skip unions.
+union NU {
+  NU() {}
+  // CHECK-FIXES: NU() {}
+  ~NU() {}
+  // CHECK-FIXES: ~NU() {}
+  NE Field;
+};
+
 // Initializer or arguments.
 class IA {
 public:
Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
@@ -32,6 +32,18 @@
   int Field;
 };
 
+// Skip unions.
+union NU {
+  NU(const NU &Other) : Field(Other.Field) {}
+  // CHECK-FIXES: NU(const NU &Other) :
+  NU &operator=(const NU &Other) {
+Field = Other.Field;
+return *this;
+  }
+  // CHECK-FIXES: NU &operator=(const NU &Other) {
+  IL Field;
+};
+
 // Wrong type.
 struct WT {
   WT(const IL &Other) {}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -127,6 +127,11 @@
   ``push_front`` on STL-style containers and replacing them with ``emplace``
   or ``emplace_front``.
 
+- Improved `modernize-use-equals-default 
`_ check.
+
+  The check now skips unions since in this case a default constructor with 
empty body
+  is not equivalent to the explicitly defaulted one.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -217,12 +217,17 @@
 }
 
 void UseEqualsDefaultCheck::registerMatchers(MatchFinder *Finder) {
+  // Skip unions since constructors with empty bodies behave differently
+  // in comparison with structs/classes.
+
   // Destructor.
-  Finder->addMatcher(cxxDestructorDecl(isDefinition()).bind(SpecialFunction),
+  
Finder->addMatcher(cxxDestructorDecl(unless(hasParent(recordDecl(isUnion(,
+   isDefinition())
+ .bind(SpecialFunction),
  this);
   Finder->addMatcher(
   cxxConstructorDecl(
-  isDefinition(),
+  unless(hasParent(recordDecl(isUnion(, isDefinition(),
   anyOf(
   // Default constructor.
   allOf(unless(hasAnyConstructorInitializer(isWritten())),
@@ -237,7 +242,8 @@
   this);
   // Copy-assignment operator.
   Finder->addMatcher(
-  cxxMethodDecl(isDefinition(), isCopyAssignmentOperator(),
+  cxxMethodDecl(unless(hasParent(recordDecl(isUnion(, isDefinition(),
+isCopyAssignmentOperator(),
 // isCopyAssignmentOperator() allows the parameter to be
 // passed by value, and in this case it cannot be
 // defaulted.


Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default.cpp
@@ -33,6 +33,15 @@
   ~NE() { f(); }
 };
 
+// Skip unions.
+union NU {
+  NU() {}
+  // CHECK-FIXES: NU() {}
+  ~NU() {}
+  // CHECK-FIXES: ~NU() {}
+  NE Field;
+};
+
 // Initializer or arguments.
 class IA {
 public:
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-equals-default-copy.cpp
@@ -32,6 +32,18 @@
   int Field;
 };
 
+// Skip unions.
+union NU {
+  NU(const NU &Other) : Field(Other.Field) {}
+  // CHECK-FIXES: NU(const NU &Other) :
+  NU &operator=(const NU &Other) {
+Field = Other.Field;
+return *this;
+  }
+  // CHECK-FIXES:

[PATCH] D53847: [C++2a] P0634r3: Down with typename!

2022-08-22 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

In D53847#3735758 , @erichkeane wrote:

> In D53847#3735738 , @ilya-biryukov 
> wrote:
>
>> In D53847#3735704 , @erichkeane 
>> wrote:
>>
>>> Note that this would also let us mark P2324 
>>>  as complete as well.  @ilya-biryukov : 
>>> Since there is no response, I suspect the answer here is someone should 
>>> take this over.  Were you going to?
>>
>> I could, but also busy with other things and happy to give this one away.
>> Would you be interested to take it over instead?
>
> Also very busy with other things, but was sorta looking into closing the 
> partial P2324 .  I haven't dug into this one 
> much in a while, but I was sort of wondering if this is a candidate for being 
> 'split up' into smaller patches/handle 1 case at a time. If i have time next 
> week, I might just start looking into making this work for single-examples, 
> rather than everything.

Welp, spoke too soon, my concepts stuff got reverted again, so I'll be 
workingon that for a while now, so I wont' have time to jump on this today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D53847

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


[PATCH] D132372: [X86][AVX512FP16] Add the missing const modifiers. NFCI

2022-08-22 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei created this revision.
pengfei added reviewers: craig.topper, yubing, LuoYuanke, RKSimon, skan, 
FreddyYe.
Herald added a subscriber: StephenFan.
Herald added a project: All.
pengfei requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch fixes lit fails after D132342 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132372

Files:
  clang/include/clang/Basic/BuiltinsX86.def
  clang/lib/Headers/avx512fp16intrin.h


Index: clang/lib/Headers/avx512fp16intrin.h
===
--- clang/lib/Headers/avx512fp16intrin.h
+++ clang/lib/Headers/avx512fp16intrin.h
@@ -829,7 +829,7 @@
   struct __mm_load_sh_struct {
 _Float16 __u;
   } __attribute__((__packed__, __may_alias__));
-  _Float16 __u = ((struct __mm_load_sh_struct *)__dp)->__u;
+  _Float16 __u = ((const struct __mm_load_sh_struct *)__dp)->__u;
   return (__m128h){__u, 0, 0, 0, 0, 0, 0, 0};
 }
 
@@ -838,13 +838,13 @@
   __m128h src = (__v8hf)__builtin_shufflevector(
   (__v8hf)__W, (__v8hf)_mm_setzero_ph(), 0, 8, 8, 8, 8, 8, 8, 8);
 
-  return (__m128h)__builtin_ia32_loadsh128_mask((__v8hf *)__A, src, __U & 1);
+  return (__m128h)__builtin_ia32_loadsh128_mask((const __v8hf *)__A, src, __U 
& 1);
 }
 
 static __inline__ __m128h __DEFAULT_FN_ATTRS128
 _mm_maskz_load_sh(__mmask8 __U, const void *__A) {
   return (__m128h)__builtin_ia32_loadsh128_mask(
-  (__v8hf *)__A, (__v8hf)_mm_setzero_ph(), __U & 1);
+  (const __v8hf *)__A, (__v8hf)_mm_setzero_ph(), __U & 1);
 }
 
 static __inline__ __m512h __DEFAULT_FN_ATTRS512
Index: clang/include/clang/Basic/BuiltinsX86.def
===
--- clang/include/clang/Basic/BuiltinsX86.def
+++ clang/include/clang/Basic/BuiltinsX86.def
@@ -1791,7 +1791,7 @@
 TARGET_BUILTIN(__builtin_ia32_cmpph256_mask, "UsV16xV16xIiUs", "ncV:256:", 
"avx512fp16,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_cmpph128_mask, "UcV8xV8xIiUc", "ncV:128:", 
"avx512fp16,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_cmpsh_mask, "UcV8xV8xIiUcIi", "ncV:128:", 
"avx512fp16")
-TARGET_BUILTIN(__builtin_ia32_loadsh128_mask, "V8xV8x*V8xUc", "nV:128:", 
"avx512fp16")
+TARGET_BUILTIN(__builtin_ia32_loadsh128_mask, "V8xV8xC*V8xUc", "nV:128:", 
"avx512fp16")
 TARGET_BUILTIN(__builtin_ia32_storesh128_mask, "vV8x*V8xUc", "nV:128:", 
"avx512fp16")
 
 TARGET_BUILTIN(__builtin_ia32_rcpph128_mask, "V8xV8xV8xUc", "ncV:128:", 
"avx512fp16,avx512vl")


Index: clang/lib/Headers/avx512fp16intrin.h
===
--- clang/lib/Headers/avx512fp16intrin.h
+++ clang/lib/Headers/avx512fp16intrin.h
@@ -829,7 +829,7 @@
   struct __mm_load_sh_struct {
 _Float16 __u;
   } __attribute__((__packed__, __may_alias__));
-  _Float16 __u = ((struct __mm_load_sh_struct *)__dp)->__u;
+  _Float16 __u = ((const struct __mm_load_sh_struct *)__dp)->__u;
   return (__m128h){__u, 0, 0, 0, 0, 0, 0, 0};
 }
 
@@ -838,13 +838,13 @@
   __m128h src = (__v8hf)__builtin_shufflevector(
   (__v8hf)__W, (__v8hf)_mm_setzero_ph(), 0, 8, 8, 8, 8, 8, 8, 8);
 
-  return (__m128h)__builtin_ia32_loadsh128_mask((__v8hf *)__A, src, __U & 1);
+  return (__m128h)__builtin_ia32_loadsh128_mask((const __v8hf *)__A, src, __U & 1);
 }
 
 static __inline__ __m128h __DEFAULT_FN_ATTRS128
 _mm_maskz_load_sh(__mmask8 __U, const void *__A) {
   return (__m128h)__builtin_ia32_loadsh128_mask(
-  (__v8hf *)__A, (__v8hf)_mm_setzero_ph(), __U & 1);
+  (const __v8hf *)__A, (__v8hf)_mm_setzero_ph(), __U & 1);
 }
 
 static __inline__ __m512h __DEFAULT_FN_ATTRS512
Index: clang/include/clang/Basic/BuiltinsX86.def
===
--- clang/include/clang/Basic/BuiltinsX86.def
+++ clang/include/clang/Basic/BuiltinsX86.def
@@ -1791,7 +1791,7 @@
 TARGET_BUILTIN(__builtin_ia32_cmpph256_mask, "UsV16xV16xIiUs", "ncV:256:", "avx512fp16,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_cmpph128_mask, "UcV8xV8xIiUc", "ncV:128:", "avx512fp16,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_cmpsh_mask, "UcV8xV8xIiUcIi", "ncV:128:", "avx512fp16")
-TARGET_BUILTIN(__builtin_ia32_loadsh128_mask, "V8xV8x*V8xUc", "nV:128:", "avx512fp16")
+TARGET_BUILTIN(__builtin_ia32_loadsh128_mask, "V8xV8xC*V8xUc", "nV:128:", "avx512fp16")
 TARGET_BUILTIN(__builtin_ia32_storesh128_mask, "vV8x*V8xUc", "nV:128:", "avx512fp16")
 
 TARGET_BUILTIN(__builtin_ia32_rcpph128_mask, "V8xV8xV8xUc", "ncV:128:", "avx512fp16,avx512vl")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D132342: [X86][AVX512FP16] Relax limitation to AVX512FP16 intrinsics. NFCI

2022-08-22 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei updated this revision to Diff 454475.
pengfei added a comment.

Rebased on D132372 .

> Should the const change be a separate patch? They feel unrelated.

Done. The change results in lit test fails.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132342

Files:
  clang/lib/Headers/avx512fp16intrin.h
  clang/lib/Headers/avx512vlfp16intrin.h
  clang/lib/Headers/immintrin.h


Index: clang/lib/Headers/immintrin.h
===
--- clang/lib/Headers/immintrin.h
+++ clang/lib/Headers/immintrin.h
@@ -214,17 +214,13 @@
 #include 
 #endif
 
-/*
- * FIXME: _Float16 type is legal only when HW support float16 operation.
- * We use __AVX512FP16__ to identify if float16 is supported or not, so
- * when float16 is not supported, the related header is not included.
- *
- */
-#if defined(__AVX512FP16__)
+#if !(defined(_MSC_VER) || defined(__SCE__)) || __has_feature(modules) ||  
\
+defined(__AVX512FP16__)
 #include 
 #endif
 
-#if defined(__AVX512FP16__) && defined(__AVX512VL__)
+#if !(defined(_MSC_VER) || defined(__SCE__)) || __has_feature(modules) ||  
\
+(defined(__AVX512VL__) && defined(__AVX512FP16__))
 #include 
 #endif
 
Index: clang/lib/Headers/avx512vlfp16intrin.h
===
--- clang/lib/Headers/avx512vlfp16intrin.h
+++ clang/lib/Headers/avx512vlfp16intrin.h
@@ -11,6 +11,8 @@
 "Never use  directly; include  instead."
 #endif
 
+#ifdef __SSE2__
+
 #ifndef __AVX512VLFP16INTRIN_H
 #define __AVX512VLFP16INTRIN_H
 
@@ -2066,3 +2068,4 @@
 #undef __DEFAULT_FN_ATTRS256
 
 #endif
+#endif
Index: clang/lib/Headers/avx512fp16intrin.h
===
--- clang/lib/Headers/avx512fp16intrin.h
+++ clang/lib/Headers/avx512fp16intrin.h
@@ -10,6 +10,8 @@
 #error "Never use  directly; include  
instead."
 #endif
 
+#ifdef __SSE2__
+
 #ifndef __AVX512FP16INTRIN_H
 #define __AVX512FP16INTRIN_H
 
@@ -3347,3 +3349,4 @@
 #undef __DEFAULT_FN_ATTRS512
 
 #endif
+#endif


Index: clang/lib/Headers/immintrin.h
===
--- clang/lib/Headers/immintrin.h
+++ clang/lib/Headers/immintrin.h
@@ -214,17 +214,13 @@
 #include 
 #endif
 
-/*
- * FIXME: _Float16 type is legal only when HW support float16 operation.
- * We use __AVX512FP16__ to identify if float16 is supported or not, so
- * when float16 is not supported, the related header is not included.
- *
- */
-#if defined(__AVX512FP16__)
+#if !(defined(_MSC_VER) || defined(__SCE__)) || __has_feature(modules) ||  \
+defined(__AVX512FP16__)
 #include 
 #endif
 
-#if defined(__AVX512FP16__) && defined(__AVX512VL__)
+#if !(defined(_MSC_VER) || defined(__SCE__)) || __has_feature(modules) ||  \
+(defined(__AVX512VL__) && defined(__AVX512FP16__))
 #include 
 #endif
 
Index: clang/lib/Headers/avx512vlfp16intrin.h
===
--- clang/lib/Headers/avx512vlfp16intrin.h
+++ clang/lib/Headers/avx512vlfp16intrin.h
@@ -11,6 +11,8 @@
 "Never use  directly; include  instead."
 #endif
 
+#ifdef __SSE2__
+
 #ifndef __AVX512VLFP16INTRIN_H
 #define __AVX512VLFP16INTRIN_H
 
@@ -2066,3 +2068,4 @@
 #undef __DEFAULT_FN_ATTRS256
 
 #endif
+#endif
Index: clang/lib/Headers/avx512fp16intrin.h
===
--- clang/lib/Headers/avx512fp16intrin.h
+++ clang/lib/Headers/avx512fp16intrin.h
@@ -10,6 +10,8 @@
 #error "Never use  directly; include  instead."
 #endif
 
+#ifdef __SSE2__
+
 #ifndef __AVX512FP16INTRIN_H
 #define __AVX512FP16INTRIN_H
 
@@ -3347,3 +3349,4 @@
 #undef __DEFAULT_FN_ATTRS512
 
 #endif
+#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D126183: Implement soft reset of the diagnostics engine.

2022-08-22 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment.

FYI, this change caused warnings when built with GCC:

  [1/1] Building CXX object 
tools/clang/unittests/Basic/CMakeFiles/BasicTests.dir/DiagnosticTest.cpp.o
  ../tools/clang/unittests/Basic/DiagnosticTest.cpp:17:6: warning: ‘void 
clang::DiagnosticsTestHelper(clang::DiagnosticsEngine&)’ has not been declared 
within ‘clang’
 17 | void clang::DiagnosticsTestHelper(DiagnosticsEngine &diag) {
|  ^
  In file included from ../tools/clang/unittests/Basic/DiagnosticTest.cpp:9:
  ../tools/clang/include/clang/Basic/Diagnostic.h:548:15: note: only here as a 
‘friend’
548 |   friend void DiagnosticsTestHelper(DiagnosticsEngine &);
|   ^




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126183

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


[PATCH] D132372: [X86][AVX512FP16] Add the missing const modifiers. NFCI

2022-08-22 Thread 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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132372

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


[PATCH] D132141: [X86] Emulate _rdrand64_step with two rdrand32 if it is 32bit

2022-08-22 Thread Simon Pilgrim via Phabricator via cfe-commits
RKSimon added inline comments.



Comment at: clang/lib/Headers/immintrin.h:301
+  unsigned long long tmp;
+  if (__builtin_ia32_rdrand32_step((unsigned int *)&tmp) &
+  __builtin_ia32_rdrand32_step(((unsigned int *)&tmp) + 1)) {

RKSimon wrote:
> craig.topper wrote:
> > craig.topper wrote:
> > > Should `&` be `&&`?
> > Can we avoid the pointer cast here? Use two unsigned ints and manually 
> > concatenate them to a 64-bit value.
> +1
> ```
> unsigned int lo, hi;
> if (__builtin_ia32_rdrand32_step(&lo) &&
> __builtin_ia32_rdrand32_step(&hi)) {
>   *p = ((unsigned long)hi << 32) | lo;
>   return 1;
> }
> ```
Are there any sideeffects that we might encounter by not always performing both 
__builtin_ia32_rdrand32_step calls?
```
  unsigned int __lo, __hi;
  int __res_lo = __builtin_ia32_rdrand32_step(&__lo);
  int __res_hi = __builtin_ia32_rdrand32_step(&__hi);
  if (__res_lo && __res_hi) {
*__p = ((unsigned long long)__hi << 32) | (unsigned long long)__lo;
return 1;
  } else {
*__p = 0;
return 0;
  }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132141

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


[PATCH] D131853: [clangd] Add doxygen parsing for Hover

2022-08-22 Thread Tom Praschan via Phabricator via cfe-commits
tom-anders added reviewers: sammccall, nridge.
tom-anders added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131853

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


[PATCH] D131990: [DRAFT][WebAssembly] Do not support `[[clang::musttail]]` by default

2022-08-22 Thread Brian Osman via Phabricator via cfe-commits
brianosman added a comment.

I agree that  the front-end error is preferable. In addition to front-end vs. 
back-end distinction being important -- it's also ignorable (if someone wants 
to do that).
Is the goal to also make __has_cpp_attribute(clang::musttail) resolve to 0? I 
think that's the ideal outcome - it lets users rely on documented ("standard") 
behavior to make decisions about which attributes to use. As it stands, I have 
to write my code like this:

  #if __has_cpp_attribute(clang::musttail) && !defined(__EMSCRIPTEN__)
...

That's brittle, and easy to overlook or forget to update if/when the WASM 
engines and LLVM code-gen do support musttail in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131990

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


[PATCH] D132141: [X86] Emulate _rdrand64_step with two rdrand32 if it is 32bit

2022-08-22 Thread Bing Yu via Phabricator via cfe-commits
yubing added inline comments.



Comment at: clang/lib/Headers/immintrin.h:301
+  unsigned long long tmp;
+  if (__builtin_ia32_rdrand32_step((unsigned int *)&tmp) &
+  __builtin_ia32_rdrand32_step(((unsigned int *)&tmp) + 1)) {

RKSimon wrote:
> RKSimon wrote:
> > craig.topper wrote:
> > > craig.topper wrote:
> > > > Should `&` be `&&`?
> > > Can we avoid the pointer cast here? Use two unsigned ints and manually 
> > > concatenate them to a 64-bit value.
> > +1
> > ```
> > unsigned int lo, hi;
> > if (__builtin_ia32_rdrand32_step(&lo) &&
> > __builtin_ia32_rdrand32_step(&hi)) {
> >   *p = ((unsigned long)hi << 32) | lo;
> >   return 1;
> > }
> > ```
> Are there any sideeffects that we might encounter by not always performing 
> both __builtin_ia32_rdrand32_step calls?
> ```
>   unsigned int __lo, __hi;
>   int __res_lo = __builtin_ia32_rdrand32_step(&__lo);
>   int __res_hi = __builtin_ia32_rdrand32_step(&__hi);
>   if (__res_lo && __res_hi) {
> *__p = ((unsigned long long)__hi << 32) | (unsigned long long)__lo;
> return 1;
>   } else {
> *__p = 0;
> return 0;
>   }
> ```
however, if the first rdrand32 failed, then we don't need to execute the second 
one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132141

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


[clang] e2ab606 - Add N2562 to the C status page

2022-08-22 Thread Aaron Ballman via cfe-commits

Author: Aaron Ballman
Date: 2022-08-22T09:46:12-04:00
New Revision: e2ab6061a976400df672842dc2af76b527fa421a

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

LOG: Add N2562 to the C status page

It was originally left off thinking the paper only impacts the C
standard library, but Clang supports diagnostics for incorrect use of
a format specifier, so this paper has some frontend impacts as well.

Added: 


Modified: 
clang/www/c_status.html

Removed: 




diff  --git a/clang/www/c_status.html b/clang/www/c_status.html
index bae3ed5d4195b..5eefdda4cc45b 100644
--- a/clang/www/c_status.html
+++ b/clang/www/c_status.html
@@ -816,6 +816,17 @@ C2x implementation status
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2607.pdf";>N2607
   Partial
 
+
+  Unclear type relationship between a format specifier and its 
argument
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf";>N2562
+  
+Partial
+  Clang supports diagnostics checking format specifier validity, but
+  does not yet account for all of the changes in this paper, especially
+  regarding length modifiers like h and hh.
+
+  
+
 
 
   String functions for freestanding implementations



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


[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

2022-08-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Thanks for working on this @nickdesaulniers! I think we actually want to go a 
slightly different direction than this and disable the diagnostics entirely. 
Basically, we should be make sure the format specifier diagnostics are 
accounting for the clarifications in 
https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf. So the `h` and `hh` 
modifiers would not even be pedantic warnings in this case.

This should also have a release note associated with it and if you think you've 
completed support for N2562, the clang/www/c_status.html page should update the 
`Partial` markings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132266

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


[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-22 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment.

In D113107#3736325 , @rjmccall wrote:

> In D113107#3736312 , @zahiraam 
> wrote:
>
>> This is a reduced test case from the codegen/complex-strictfp.c
>>
>> _Complex double g1, g2;
>> double D;
>>
>> void foo(void) {
>>
>>   g1 = g1 + D;
>>
>> }
>>
>> The issue is that we are calling in VisitBinAssign the function  
>> EmitUnpromotion (since promotionTy is null). This creates 2 constrained 
>> (-ffp-exception-behavior=maytrap used) fptrunc instructions. The verifier 
>> for Intrinsic::experimental_constrained_fptrunc fails at this check:
>> https://github.com/intel/llvm/blob/sycl/llvm/lib/IR/Verifier.cpp#L5885
>> Is the call for EmitUnpromotion at the right place? Or should VisitBinAssign 
>> be treated the same way than the operator of HANDLEBINOP? Since in this case 
>> we are in the unpromoted path, we don't really need to add those fptrunc? 
>> Any thoughts?
>
> If `VisitBinAssign` isn't opting to promoted emission (by calling 
> `EmitPromoted...`), it shouldn't have to call `EmitUnpromotion`; it should be 
> able to rely on getting appropriate values for the type of its operands.  
> That's what I mean by a strong postcondition.  So yeah, if you added a call 
> to `EmitUnpromotion` there as a workaround at some point, you should take it 
> out.  If that breaks tests, it's because some emitter is leaking promoted 
> values out of the normal emission path, and we need to fix that emitter.

Fixed codegen issue and LIT test issue mentioned by @rjmccall.
I need second pair of eyes to check the resulting IR, @pengfei  can you please 
help with that?
Do we want to add the promoted path for VistiBinAssign in this patch or can we 
add it in a subsequent patch? 
Thanks.


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

https://reviews.llvm.org/D113107

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


[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-08-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677
 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L");
-Builder.defineMacro("__cpp_concepts", "201907L");
+Builder.defineMacro("__cpp_concepts", "202002L");
 Builder.defineMacro("__cpp_conditional_explicit", "201806L");

cor3ntin wrote:
> royjacobson wrote:
> > aaron.ballman wrote:
> > > Does any of the not-yet-implemented bits (including from the DRs) impact 
> > > the ability to use conditionally trivial special member functions? If so, 
> > > we might want to be careful about aggressively bumping this value. (It's 
> > > more palatable for us to come back and bump the value later than it is 
> > > for us to claim we implement something fully when we know we don't -- the 
> > > goal of the feature test macros is so that users don't have to resort to 
> > > compiler version checks, which is what users have to use when they fall 
> > > into that "not fully implemented" space.)
> > I don't think they're very significant, and the benefits of enabling it 
> > seem large enough for me - for example, std::expected works with libstdc++ 
> > and passes their unit tests but is gated by this macro.
> > 
> > We still have a non-trivial amount of concept bugs to go over, but I 
> > support enabling this.
> > 
> I think it's better to be conservative, It's the lesser of two not great 
> options.
> I'm hoping we can get to fix the issues in the clang 16 cycle , but in the 
> meantime we should not claim conformance if we are not, in fact, conforming.
> We still have a non-trivial amount of concept bugs to go over, but I support 
> enabling this.

I think we should specify the updated macro value only after we think concepts 
have no known major issues with them. (Unknown issues happen and there's not 
much we can do about them, this is more that we shouldn't specify that we 
conform to a particular feature test macro when we knowingly still don't have 
it right yet.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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


[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

2022-08-22 Thread YingChi Long via Phabricator via cfe-commits
inclyc added a comment.

In D132266#3739513 , @aaron.ballman 
wrote:

> Thanks for working on this @nickdesaulniers! I think we actually want to go a 
> slightly different direction than this and disable the diagnostics entirely. 
> Basically, we should be make sure the format specifier diagnostics are 
> accounting for the clarifications in 
> https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf. So the `h` and 
> `hh` modifiers would not even be pedantic warnings in this case.
>
> This should also have a release note associated with it and if you think 
> you've completed support for N2562, the clang/www/c_status.html page should 
> update the `Partial` markings.

Sorry, I have some questions about this clarification agreement (currently 
working on N2562). The length modifier `hh` says in the standard that it should 
point to `signed char` or `unsigned char`, and if an `int` parameter is passed, 
why shouldn't we give such a warning? (even if it's pedantic somehow)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132266

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


[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-08-22 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677
 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L");
-Builder.defineMacro("__cpp_concepts", "201907L");
+Builder.defineMacro("__cpp_concepts", "202002L");
 Builder.defineMacro("__cpp_conditional_explicit", "201806L");

aaron.ballman wrote:
> cor3ntin wrote:
> > royjacobson wrote:
> > > aaron.ballman wrote:
> > > > Does any of the not-yet-implemented bits (including from the DRs) 
> > > > impact the ability to use conditionally trivial special member 
> > > > functions? If so, we might want to be careful about aggressively 
> > > > bumping this value. (It's more palatable for us to come back and bump 
> > > > the value later than it is for us to claim we implement something fully 
> > > > when we know we don't -- the goal of the feature test macros is so that 
> > > > users don't have to resort to compiler version checks, which is what 
> > > > users have to use when they fall into that "not fully implemented" 
> > > > space.)
> > > I don't think they're very significant, and the benefits of enabling it 
> > > seem large enough for me - for example, std::expected works with 
> > > libstdc++ and passes their unit tests but is gated by this macro.
> > > 
> > > We still have a non-trivial amount of concept bugs to go over, but I 
> > > support enabling this.
> > > 
> > I think it's better to be conservative, It's the lesser of two not great 
> > options.
> > I'm hoping we can get to fix the issues in the clang 16 cycle , but in the 
> > meantime we should not claim conformance if we are not, in fact, conforming.
> > We still have a non-trivial amount of concept bugs to go over, but I 
> > support enabling this.
> 
> I think we should specify the updated macro value only after we think 
> concepts have no known major issues with them. (Unknown issues happen and 
> there's not much we can do about them, this is more that we shouldn't specify 
> that we conform to a particular feature test macro when we knowingly still 
> don't have it right yet.)
Yeah, I don't think we can set this until we can at least compile a basic 
libstdc++ ranges use, which the deferred instantiation still holds up.  That 
would be my 'bare minimum' test for whether we can set that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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


[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-08-22 Thread Jamie Schmeiser via Phabricator via cfe-commits
jamieschmeiser added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4667
+  // Whether `-ftime-trace` or `-ftime-trace=` are specified
+  if (!TimeTrace && !TimeTraceFile) return;
+

The return should be on the next line.  Did you run this through clang-format?  
Is it okay with this on the same line?



Comment at: clang/lib/Driver/Driver.cpp:4674
+for (auto &J : C.getJobs()) {
+  if (J.getSource().getKind() == Action::LinkJobClass &&
+  !J.getOutputFilenames().empty()) {

dongjunduo wrote:
> jamieschmeiser wrote:
> > Can you have a link job without an output filename?  If not, then just have 
> > an assert for !empty.  Again, the negative test and continue might be 
> > easier to understand.
> The output filename should not be empty. 
> 
> If the "-o" is not specified, the output filename may be "a.out".
Yes.  If the output filename should not be empty, then assert that rather than 
testing it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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


[PATCH] D132141: [X86] Emulate _rdrand64_step with two rdrand32 if it is 32bit

2022-08-22 Thread Simon Pilgrim via Phabricator via cfe-commits
RKSimon added inline comments.



Comment at: clang/lib/Headers/immintrin.h:301
+  unsigned long long tmp;
+  if (__builtin_ia32_rdrand32_step((unsigned int *)&tmp) &
+  __builtin_ia32_rdrand32_step(((unsigned int *)&tmp) + 1)) {

yubing wrote:
> RKSimon wrote:
> > RKSimon wrote:
> > > craig.topper wrote:
> > > > craig.topper wrote:
> > > > > Should `&` be `&&`?
> > > > Can we avoid the pointer cast here? Use two unsigned ints and manually 
> > > > concatenate them to a 64-bit value.
> > > +1
> > > ```
> > > unsigned int lo, hi;
> > > if (__builtin_ia32_rdrand32_step(&lo) &&
> > > __builtin_ia32_rdrand32_step(&hi)) {
> > >   *p = ((unsigned long)hi << 32) | lo;
> > >   return 1;
> > > }
> > > ```
> > Are there any sideeffects that we might encounter by not always performing 
> > both __builtin_ia32_rdrand32_step calls?
> > ```
> >   unsigned int __lo, __hi;
> >   int __res_lo = __builtin_ia32_rdrand32_step(&__lo);
> >   int __res_hi = __builtin_ia32_rdrand32_step(&__hi);
> >   if (__res_lo && __res_hi) {
> > *__p = ((unsigned long long)__hi << 32) | (unsigned long long)__lo;
> > return 1;
> >   } else {
> > *__p = 0;
> > return 0;
> >   }
> > ```
> however, if the first rdrand32 failed, then we don't need to execute the 
> second one.
I understand that - but given randomizers are often used for sensitive 
applications (crypto) - my question was whether not always calling this twice 
was going to affect things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132141

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


[PATCH] D132302: [clang] Add support for __attribute__((guard(nocf)))

2022-08-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

There should be some explicit test coverage that show the new attribute 
spelling(s) work when spelled directly rather than when relying on predefined 
macros.




Comment at: clang/include/clang/Basic/Attr.td:3496
   // we might also want to support __declspec(guard(suppress)).
-  let Spellings = [Declspec<"guard">];
+  let Spellings = [Declspec<"guard">, GCC<"guard">];
   let Subjects = SubjectList<[Function]>;

I don't see any evidence that this is supported by GCC: 
https://godbolt.org/z/bEPv4E7ab

I think this should be using `Clang` instead of `GCC` so that it works as both 
`[[clang::guard(nocf)]]` and `__attribute__((guard(nocf)))`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132302

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


[clang] df23fc4 - [X86][AVX512FP16] Add the missing const modifiers. NFCI

2022-08-22 Thread Phoebe Wang via cfe-commits

Author: Phoebe Wang
Date: 2022-08-22T22:07:42+08:00
New Revision: df23fc4f7fe7b9210217cbd37c409fc76d4ff136

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

LOG: [X86][AVX512FP16] Add the missing const modifiers. NFCI

This patch fixes lit fails after D132342.

Reviewed By: RKSimon

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

Added: 


Modified: 
clang/include/clang/Basic/BuiltinsX86.def
clang/lib/Headers/avx512fp16intrin.h

Removed: 




diff  --git a/clang/include/clang/Basic/BuiltinsX86.def 
b/clang/include/clang/Basic/BuiltinsX86.def
index 6bf35c340c2d1..ad8509e6124d4 100644
--- a/clang/include/clang/Basic/BuiltinsX86.def
+++ b/clang/include/clang/Basic/BuiltinsX86.def
@@ -1791,7 +1791,7 @@ TARGET_BUILTIN(__builtin_ia32_cmpph512_mask, 
"UiV32xV32xIiUiIi", "ncV:512:", "av
 TARGET_BUILTIN(__builtin_ia32_cmpph256_mask, "UsV16xV16xIiUs", "ncV:256:", 
"avx512fp16,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_cmpph128_mask, "UcV8xV8xIiUc", "ncV:128:", 
"avx512fp16,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_cmpsh_mask, "UcV8xV8xIiUcIi", "ncV:128:", 
"avx512fp16")
-TARGET_BUILTIN(__builtin_ia32_loadsh128_mask, "V8xV8x*V8xUc", "nV:128:", 
"avx512fp16")
+TARGET_BUILTIN(__builtin_ia32_loadsh128_mask, "V8xV8xC*V8xUc", "nV:128:", 
"avx512fp16")
 TARGET_BUILTIN(__builtin_ia32_storesh128_mask, "vV8x*V8xUc", "nV:128:", 
"avx512fp16")
 
 TARGET_BUILTIN(__builtin_ia32_rcpph128_mask, "V8xV8xV8xUc", "ncV:128:", 
"avx512fp16,avx512vl")

diff  --git a/clang/lib/Headers/avx512fp16intrin.h 
b/clang/lib/Headers/avx512fp16intrin.h
index 99409a31b32bd..2e4ca353ff692 100644
--- a/clang/lib/Headers/avx512fp16intrin.h
+++ b/clang/lib/Headers/avx512fp16intrin.h
@@ -829,7 +829,7 @@ static __inline__ __m128h __DEFAULT_FN_ATTRS128 
_mm_load_sh(void const *__dp) {
   struct __mm_load_sh_struct {
 _Float16 __u;
   } __attribute__((__packed__, __may_alias__));
-  _Float16 __u = ((struct __mm_load_sh_struct *)__dp)->__u;
+  _Float16 __u = ((const struct __mm_load_sh_struct *)__dp)->__u;
   return (__m128h){__u, 0, 0, 0, 0, 0, 0, 0};
 }
 
@@ -838,13 +838,13 @@ _mm_mask_load_sh(__m128h __W, __mmask8 __U, const void 
*__A) {
   __m128h src = (__v8hf)__builtin_shufflevector(
   (__v8hf)__W, (__v8hf)_mm_setzero_ph(), 0, 8, 8, 8, 8, 8, 8, 8);
 
-  return (__m128h)__builtin_ia32_loadsh128_mask((__v8hf *)__A, src, __U & 1);
+  return (__m128h)__builtin_ia32_loadsh128_mask((const __v8hf *)__A, src, __U 
& 1);
 }
 
 static __inline__ __m128h __DEFAULT_FN_ATTRS128
 _mm_maskz_load_sh(__mmask8 __U, const void *__A) {
   return (__m128h)__builtin_ia32_loadsh128_mask(
-  (__v8hf *)__A, (__v8hf)_mm_setzero_ph(), __U & 1);
+  (const __v8hf *)__A, (__v8hf)_mm_setzero_ph(), __U & 1);
 }
 
 static __inline__ __m512h __DEFAULT_FN_ATTRS512



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


[PATCH] D132372: [X86][AVX512FP16] Add the missing const modifiers. NFCI

2022-08-22 Thread Phoebe Wang 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 rGdf23fc4f7fe7: [X86][AVX512FP16] Add the missing const 
modifiers. NFCI (authored by pengfei).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132372

Files:
  clang/include/clang/Basic/BuiltinsX86.def
  clang/lib/Headers/avx512fp16intrin.h


Index: clang/lib/Headers/avx512fp16intrin.h
===
--- clang/lib/Headers/avx512fp16intrin.h
+++ clang/lib/Headers/avx512fp16intrin.h
@@ -829,7 +829,7 @@
   struct __mm_load_sh_struct {
 _Float16 __u;
   } __attribute__((__packed__, __may_alias__));
-  _Float16 __u = ((struct __mm_load_sh_struct *)__dp)->__u;
+  _Float16 __u = ((const struct __mm_load_sh_struct *)__dp)->__u;
   return (__m128h){__u, 0, 0, 0, 0, 0, 0, 0};
 }
 
@@ -838,13 +838,13 @@
   __m128h src = (__v8hf)__builtin_shufflevector(
   (__v8hf)__W, (__v8hf)_mm_setzero_ph(), 0, 8, 8, 8, 8, 8, 8, 8);
 
-  return (__m128h)__builtin_ia32_loadsh128_mask((__v8hf *)__A, src, __U & 1);
+  return (__m128h)__builtin_ia32_loadsh128_mask((const __v8hf *)__A, src, __U 
& 1);
 }
 
 static __inline__ __m128h __DEFAULT_FN_ATTRS128
 _mm_maskz_load_sh(__mmask8 __U, const void *__A) {
   return (__m128h)__builtin_ia32_loadsh128_mask(
-  (__v8hf *)__A, (__v8hf)_mm_setzero_ph(), __U & 1);
+  (const __v8hf *)__A, (__v8hf)_mm_setzero_ph(), __U & 1);
 }
 
 static __inline__ __m512h __DEFAULT_FN_ATTRS512
Index: clang/include/clang/Basic/BuiltinsX86.def
===
--- clang/include/clang/Basic/BuiltinsX86.def
+++ clang/include/clang/Basic/BuiltinsX86.def
@@ -1791,7 +1791,7 @@
 TARGET_BUILTIN(__builtin_ia32_cmpph256_mask, "UsV16xV16xIiUs", "ncV:256:", 
"avx512fp16,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_cmpph128_mask, "UcV8xV8xIiUc", "ncV:128:", 
"avx512fp16,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_cmpsh_mask, "UcV8xV8xIiUcIi", "ncV:128:", 
"avx512fp16")
-TARGET_BUILTIN(__builtin_ia32_loadsh128_mask, "V8xV8x*V8xUc", "nV:128:", 
"avx512fp16")
+TARGET_BUILTIN(__builtin_ia32_loadsh128_mask, "V8xV8xC*V8xUc", "nV:128:", 
"avx512fp16")
 TARGET_BUILTIN(__builtin_ia32_storesh128_mask, "vV8x*V8xUc", "nV:128:", 
"avx512fp16")
 
 TARGET_BUILTIN(__builtin_ia32_rcpph128_mask, "V8xV8xV8xUc", "ncV:128:", 
"avx512fp16,avx512vl")


Index: clang/lib/Headers/avx512fp16intrin.h
===
--- clang/lib/Headers/avx512fp16intrin.h
+++ clang/lib/Headers/avx512fp16intrin.h
@@ -829,7 +829,7 @@
   struct __mm_load_sh_struct {
 _Float16 __u;
   } __attribute__((__packed__, __may_alias__));
-  _Float16 __u = ((struct __mm_load_sh_struct *)__dp)->__u;
+  _Float16 __u = ((const struct __mm_load_sh_struct *)__dp)->__u;
   return (__m128h){__u, 0, 0, 0, 0, 0, 0, 0};
 }
 
@@ -838,13 +838,13 @@
   __m128h src = (__v8hf)__builtin_shufflevector(
   (__v8hf)__W, (__v8hf)_mm_setzero_ph(), 0, 8, 8, 8, 8, 8, 8, 8);
 
-  return (__m128h)__builtin_ia32_loadsh128_mask((__v8hf *)__A, src, __U & 1);
+  return (__m128h)__builtin_ia32_loadsh128_mask((const __v8hf *)__A, src, __U & 1);
 }
 
 static __inline__ __m128h __DEFAULT_FN_ATTRS128
 _mm_maskz_load_sh(__mmask8 __U, const void *__A) {
   return (__m128h)__builtin_ia32_loadsh128_mask(
-  (__v8hf *)__A, (__v8hf)_mm_setzero_ph(), __U & 1);
+  (const __v8hf *)__A, (__v8hf)_mm_setzero_ph(), __U & 1);
 }
 
 static __inline__ __m512h __DEFAULT_FN_ATTRS512
Index: clang/include/clang/Basic/BuiltinsX86.def
===
--- clang/include/clang/Basic/BuiltinsX86.def
+++ clang/include/clang/Basic/BuiltinsX86.def
@@ -1791,7 +1791,7 @@
 TARGET_BUILTIN(__builtin_ia32_cmpph256_mask, "UsV16xV16xIiUs", "ncV:256:", "avx512fp16,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_cmpph128_mask, "UcV8xV8xIiUc", "ncV:128:", "avx512fp16,avx512vl")
 TARGET_BUILTIN(__builtin_ia32_cmpsh_mask, "UcV8xV8xIiUcIi", "ncV:128:", "avx512fp16")
-TARGET_BUILTIN(__builtin_ia32_loadsh128_mask, "V8xV8x*V8xUc", "nV:128:", "avx512fp16")
+TARGET_BUILTIN(__builtin_ia32_loadsh128_mask, "V8xV8xC*V8xUc", "nV:128:", "avx512fp16")
 TARGET_BUILTIN(__builtin_ia32_storesh128_mask, "vV8x*V8xUc", "nV:128:", "avx512fp16")
 
 TARGET_BUILTIN(__builtin_ia32_rcpph128_mask, "V8xV8xV8xUc", "ncV:128:", "avx512fp16,avx512vl")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D132372: [X86][AVX512FP16] Add the missing const modifiers. NFCI

2022-08-22 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei added a comment.

Thanks @RKSimon


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132372

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


[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

2022-08-22 Thread YingChi Long via Phabricator via cfe-commits
inclyc added a comment.

N2562.pdf:

> Modify 7.21.6.2p12:
> ...
>  Unless a length modifier is specified, t~~T~~he corresponding argument shall 
> be a pointer to int ~~signed integer~~.

Does this clarification statement mean that `hhd` should not be considered to 
correspond to `int`, but a `signed char`? If so, could this exactly imply that 
we have unmatched argument type? (i.e. `int` <--x--> `hhd` (signed char) )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132266

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


[PATCH] D132285: [LoongArch] Implement ABI lowering

2022-08-22 Thread Xi Ruoyao via Phabricator via cfe-commits
xry111 added a comment.

In D132285#3737855 , @SixWeining 
wrote:

> Ignore zero-width bit fields and add a test.

Thanks!  There is a GCC test (https://gcc.gnu.org/r12-7951) which can be used 
to test if there is any ABI incompatibility between GCC and Clang, but I guess 
it's not possible to run it at the early development stage of Clang.  Maybe we 
can run it later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132285

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


[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

2022-08-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D132266#3739542 , @inclyc wrote:

> In D132266#3739513 , @aaron.ballman 
> wrote:
>
>> Thanks for working on this @nickdesaulniers! I think we actually want to go 
>> a slightly different direction than this and disable the diagnostics 
>> entirely. Basically, we should be make sure the format specifier diagnostics 
>> are accounting for the clarifications in 
>> https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf. So the `h` and 
>> `hh` modifiers would not even be pedantic warnings in this case.
>>
>> This should also have a release note associated with it and if you think 
>> you've completed support for N2562, the clang/www/c_status.html page should 
>> update the `Partial` markings.
>
> Sorry, I have some questions about this clarification agreement (currently 
> working on N2562). The length modifier `hh` says in the standard that it 
> should point to `signed char` or `unsigned char`, and if an `int` parameter 
> is passed, why shouldn't we give such a warning? (even if it's pedantic 
> somehow)

That's for the `fscanf` functionality, which is different than the `fprintf` 
functionality that's being addressed here. We should be diagnosing the `fscanf` 
type mismatch cases, per 7.23.6.2p12: "In the following, the type of the 
corresponding argument for a conversion specifier shall be a pointer to a type 
determined by the length modifiers, if any, or specified by the conversion 
specifier."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132266

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


[PATCH] D132342: [X86][AVX512FP16] Relax limitation to AVX512FP16 intrinsics. NFCI

2022-08-22 Thread Simon Pilgrim via Phabricator via cfe-commits
RKSimon added inline comments.



Comment at: clang/lib/Headers/avx512fp16intrin.h:13
 
+#ifdef __SSE2__
+

Doesn't this have to be the general case like in other places in the headers?
```
#if !(defined(_MSC_VER) || defined(__SCE__)) || __has_feature(modules) ||  \
defined(__SSE2__)
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132342

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


[clang] 210a419 - [docs] Adjust the example command line in DebuggingCoroutines.rst

2022-08-22 Thread Chuanqi Xu via cfe-commits

Author: Chuanqi Xu
Date: 2022-08-22T22:15:47+08:00
New Revision: 210a4197b4d73388030c63aea6dc51dde3e9166e

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

LOG: [docs] Adjust the example command line in DebuggingCoroutines.rst

The original commandline example was not correct in some environments.
Adjust the example to avoid any misunderstanding.

Added: 


Modified: 
clang/docs/DebuggingCoroutines.rst

Removed: 




diff  --git a/clang/docs/DebuggingCoroutines.rst 
b/clang/docs/DebuggingCoroutines.rst
index e6942a4be6dbf..828822f52bd61 100644
--- a/clang/docs/DebuggingCoroutines.rst
+++ b/clang/docs/DebuggingCoroutines.rst
@@ -147,8 +147,10 @@ printing the details of the coroutine frame from an 
address is also possible:
   (gdb) # Get the linkage name for the coroutine
   (gdb) x 0x4019e0
   0x4019e0 <_ZL9coro_taski>:  0xe5894855
+  (gdb) # Turn off the demangler temporarily to avoid the debugger 
misunderstanding the name.
+  (gdb) set demangle-style none
   (gdb) # The coroutine frame type is 'linkage_name.coro_frame_ty'
-  (gdb) print  (_ZL9coro_taski.coro_frame_ty)*(0x418eb0)
+  (gdb) print  ('_ZL9coro_taski.coro_frame_ty')*(0x418eb0)
   $2 = {__resume_fn = 0x4019e0 , __destroy_fn = 0x402000 
, __promise = {...}, ...}
 
 The above is possible because:



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


[PATCH] D132074: [OpenMP] Add option to assert no nested OpenMP parallelism on the GPU

2022-08-22 Thread Carlo Bertolli via Phabricator via cfe-commits
carlo.bertolli added a comment.

This looks good, but what happens when the user accidentally adds a nested 
parallel when this option is turned on? Do we get serial (correct) execution?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132074

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


[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

2022-08-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D132266#3739567 , @inclyc wrote:

> N2562.pdf:
>
>> Modify 7.21.6.2p12:
>> ...
>>  Unless a length modifier is specified, t~~T~~he corresponding argument 
>> shall be a pointer to int ~~signed integer~~.
>
> Does this clarification statement mean that `hhd` should not be considered to 
> correspond to `int`, but a `signed char`?

Correct.

> If so, could this exactly imply that we have unmatched argument type? (i.e. 
> `int` <--x--> `hhd` (signed char) )

Here's how that's intended to be read:

  int i;
  signed char sc;
  unsigned char uc;
  char c;
  
  scanf("%hhd, %hhd, %hhd, %hhd", &i, &sc, &uc, &c);

The first format specifier has a type mismatch: expected `signed char *` or 
`unsigned char *`, got `int *`.
The second format specifier is correct: expected `signed char *` or `unsigned 
char *`, got `signed char *`.
The third format specifier is correct: expected `signed char *` or `unsigned 
char *`, got `unsigned char *`.
The fourth format specifier is questionable (and I'll file an NB comment to 
clarify this) but I think intended to be correct: expected `signed char *` or 
`unsigned char *`, got `char *`. Pedantically, this is incorrect per 6.2.5 
making `char` special.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132266

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


[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

2022-08-22 Thread YingChi Long via Phabricator via cfe-commits
inclyc added a comment.

What I'm confusing is
Which of the following two explanations is the exact meaning of `hhd`?

1. consumes a 32-bit signed integer, then truncates it *inside* printf
2. consumes an 8-bit signed integer

If it is the case 1, we should not emit this warning, but N2562 said that it 
still seems to be 2 ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132266

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


[PATCH] D132074: [OpenMP] Add option to assert no nested OpenMP parallelism on the GPU

2022-08-22 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

In D132074#3739593 , @carlo.bertolli 
wrote:

> This looks good, but what happens when the user accidentally adds a nested 
> parallel when this option is turned on? Do we get serial (correct) execution?

With the code as it is, it will simply ignore the level and continue executing. 
This will probably be broken as we don't adjust some of the other state 
variables for this. The flag more-so asserts that nested parallelism will not 
work in any capacity than we reduce them to a single thread. We could 
potentially make it work, but it would be more complicated. There's an 
assertion if nested parallelism is attempted while being disabled, but this 
requires the user checking the assertions via `-fopenmp-debug`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132074

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


[PATCH] D132074: [OpenMP] Add option to assert no nested OpenMP parallelism on the GPU

2022-08-22 Thread Carlo Bertolli via Phabricator via cfe-commits
carlo.bertolli accepted this revision.
carlo.bertolli added a comment.
This revision is now accepted and ready to land.

Thanks for explaining. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132074

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


[PATCH] D132377: [clang][dataflow] Add `SetupTest` parameter for `AnalysisInputs`.

2022-08-22 Thread weiyi via Phabricator via cfe-commits
wyt created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a project: All.
wyt requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Moves the work required for retrieving annotation states into the `SetupTest` 
and `PostVisitCFG` callback to avoid having to run a separate pass over the CFG 
after analysis has completed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132377

Files:
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -56,28 +56,6 @@
 
 namespace test {
 
-/// Arguments for building the dataflow analysis.
-template  struct AnalysisInputs {
-  /// Input code that is analyzed.
-  llvm::StringRef Code;
-  /// The body of the function which matches this matcher is analyzed.
-  ast_matchers::internal::Matcher TargetFuncMatcher;
-  /// The analysis to be run is constructed with this function that takes as
-  /// argument the AST generated from the code being analyzed and the
-  /// initial state from which the analysis starts with.
-  std::function MakeAnalysis;
-  /// If provided, this function is applied on each CFG element after the
-  /// analysis has been run.
-  std::function
-  PostVisitCFG = nullptr;
-
-  /// Options for building the AST context.
-  ArrayRef ASTBuildArgs = {};
-  /// Options for building the AST context.
-  const tooling::FileContentMappings &ASTBuildVirtualMappedFiles = {};
-};
-
 /// Contains data structures required and produced by a dataflow analysis run.
 struct AnalysisOutputs {
   /// Input code that is analyzed. Points within the code may be marked with
@@ -106,60 +84,43 @@
   llvm::ArrayRef> BlockStates;
 };
 
+/// Arguments for building the dataflow analysis.
+template  struct AnalysisInputs {
+  /// Input code that is analyzed.
+  llvm::StringRef Code;
+  /// The body of the function which matches this matcher is analyzed.
+  ast_matchers::internal::Matcher TargetFuncMatcher;
+  /// The analysis to be run is constructed with this function that takes as
+  /// argument the AST generated from the code being analyzed and the
+  /// initial state from which the analysis starts with.
+  std::function MakeAnalysis;
+  /// If provided, this function is applied on each CFG element after the
+  /// analysis has been run.
+  std::function
+  PostVisitCFG = nullptr;
+  /// If provided, this function is executed immediately before running the
+  /// dataflow analysis to allow for additional setup. All fields in the
+  /// `AnalysisOutputs` argument will be initialized except for the
+  /// `BlockStates` field which is only computed later during the analysis.
+  std::function SetupTest = nullptr;
+
+  /// Options for building the AST context.
+  ArrayRef ASTBuildArgs = {};
+  /// Options for building the AST context.
+  const tooling::FileContentMappings &ASTBuildVirtualMappedFiles = {};
+};
+
 /// Returns assertions based on annotations that are present after statements in
 /// `AnnotatedCode`.
 llvm::Expected>
 buildStatementToAnnotationMapping(const FunctionDecl *Func,
   llvm::Annotations AnnotatedCode);
 
-/// Returns line numbers and content of the annotations in `AO.Code`.
+/// Returns line numbers and content of the annotations in `AnnotatedCode`.
 llvm::DenseMap
-getAnnotationLinesAndContent(AnalysisOutputs &AO);
-
-// FIXME: Return a string map instead of a vector of pairs.
-//
-/// Returns the analysis states at each annotated statement in `AO.Code`.
-template 
-llvm::Expected>>>
-getAnnotationStates(AnalysisOutputs &AO) {
-  using StateT = DataflowAnalysisState;
-  // FIXME: Extend to annotations on non-statement constructs.
-  // Get annotated statements.
-  llvm::Expected>
-  MaybeStmtToAnnotations =
-  buildStatementToAnnotationMapping(AO.Target, AO.Code);
-  if (!MaybeStmtToAnnotations)
-return MaybeStmtToAnnotations.takeError();
-  auto &StmtToAnnotations = *MaybeStmtToAnnotations;
-
-  // Compute a map from statement annotations to the state computed
-  // for the program point immediately after the annotated statement.
-  std::vector> Results;
-  for (const CFGBlock *Block : AO.CFCtx.getCFG()) {
-// Skip blocks that were not evaluated.
-if (!AO.BlockStates[Block->getBlockID()])
-  continue;
-
-transferBlock(
-AO.CFCtx, AO.BlockStates, *Block, AO.InitEnv, AO.Analysis,
-[&Results,
- &StmtToAnnotations](const clang::CFGElement &Element,
- const TypeErasedDataflowAnalysisState &State) {
-  auto Stmt = Element.getAs();
-  if (!Stmt)
-return;
-  auto It = StmtToAnnotations.find(Stmt->getStmt(

[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

2022-08-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D132266#3739600 , @inclyc wrote:

> What I'm confusing is
> Which of the following two explanations is the exact meaning of `hhd`?
>
> 1. consumes a 32-bit signed integer, then truncates it *inside* printf
> 2. consumes an 8-bit signed integer
>
> If it is the case 1, we should not emit this warning, but N2562 said that it 
> still seems to be 2 ?

There's some confusion happening that we should clarify first. The sections 
you've been quoting so far are for the behavior of `fscanf` not `fprintf` and 
the requirements are different.

For `fprintf`, `hhd` means that `fprintf` consumes an `int`-sized object by 
calling effectively `signed char value = (signed char)va_arg(list, int);`.
For `fscanf`, `hhd` means that `fscanf` consumes a pointer in which to store 
the converted data by effectively calling `signed char *ptr = (signed char 
*)va_arg(list, signed char *); *ptr = value;`

This patch is currently handling only the `fprintf` cases and is not addressing 
the `fscanf` ones.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132266

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


[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-08-22 Thread dongjunduo via Phabricator via cfe-commits
dongjunduo updated this revision to Diff 454501.
dongjunduo added a comment.

Restyle code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/check-time-trace.cpp
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -256,17 +256,10 @@
   llvm::TimerGroup::clearAll();
 
   if (llvm::timeTraceProfilerEnabled()) {
-SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
-llvm::sys::path::replace_extension(Path, "json");
-if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
-  // replace the suffix to '.json' directly
-  SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
-  if (llvm::sys::fs::is_directory(TracePath))
-llvm::sys::path::append(TracePath, llvm::sys::path::filename(Path));
-  Path.assign(TracePath);
-}
+SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
+assert(!TracePath.empty() && "`-ftime-trace=` is empty");
 if (auto profilerOutput = Clang->createOutputFile(
-Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
+TracePath.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
 /*useTemporary=*/false)) {
   llvm::timeTraceProfilerWrite(*profilerOutput);
   profilerOutput.reset();
Index: clang/test/Driver/check-time-trace.cpp
===
--- clang/test/Driver/check-time-trace.cpp
+++ clang/test/Driver/check-time-trace.cpp
@@ -1,3 +1,9 @@
+// RUN: rm -rf %T/exe && mkdir %T/exe
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s -###
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s
+// RUN: cat %T/exe/check-time-trace*.json \
+// RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
 // RUN: %clangxx -S -ftime-trace -ftime-trace-granularity=0 -o %T/check-time-trace %s
 // RUN: cat %T/check-time-trace.json \
 // RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4658,6 +4658,107 @@
   llvm_unreachable("invalid phase in ConstructPhaseAction");
 }
 
+// Infer data storing path of the options `-ftime-trace`, `-ftime-trace=`
+void InferTimeTracePath(Compilation &C) {
+  bool TimeTrace = C.getArgs().getLastArg(options::OPT_ftime_trace) != nullptr;
+  bool TimeTraceFile =
+  C.getArgs().getLastArg(options::OPT_ftime_trace_EQ) != nullptr;
+  // Whether `-ftime-trace` or `-ftime-trace=` are specified
+  if (!TimeTrace && !TimeTraceFile)
+return;
+
+  // If `-ftime-trace=` is specified, TracePath is the .
+  // Else if there is a linking job, TracePath is the parent path of .exe,
+  // then the OutputFile's name may be appended to it.
+  // Else, TracePath is "",
+  // then the full OutputFile's path may be appended to it.
+  SmallString<128> TracePath("");
+
+  if (TimeTraceFile) {
+TracePath = SmallString<128>(
+C.getArgs().getLastArg(options::OPT_ftime_trace_EQ)->getValue());
+  } else {
+// Get linking executable file's parent path as TracePath's parent path,
+// default is ".". Filename may be determined and added into TracePath then.
+//
+// e.g. executable file's path: /usr/local/a.out
+//  its parent's path:  /usr/local
+for (auto &J : C.getJobs()) {
+  if (J.getSource().getKind() == Action::LinkJobClass) {
+assert(!J.getOutputFilenames().empty() &&
+   "linking output filename is empty");
+auto OutputFilePath =
+SmallString<128>(J.getOutputFilenames()[0].c_str());
+if (llvm::sys::path::has_parent_path(OutputFilePath)) {
+  TracePath = llvm::sys::path::parent_path(OutputFilePath);
+} else {
+  TracePath = SmallString<128>(".");
+}
+break;
+  }
+}
+  }
+
+  // Add or replace -ftime-trace` to the correct one to all clang jobs
+  for (auto &J : C.getJobs()) {
+if (J.getSource().getKind() == Action::AssembleJobClass ||
+J.getSource().getKind() == Action::BackendJobClass ||
+J.getSource().getKind() == Action::CompileJobClass) {
+  SmallString<128> TracePathReal = TracePath;
+  SmallString<128> OutputPath(J.getOutputFilenames()[0].c_str());
+  std::string arg = std::string("-ftime-trace=");
+  if (!TimeTraceFile) {
+if (TracePathReal.empty()) {
+  // /xxx/yyy.o => /xxx/yyy.json
+  

[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

2022-08-22 Thread YingChi Long via Phabricator via cfe-commits
inclyc added a comment.

In D132266#3739618 , @aaron.ballman 
wrote:

> In D132266#3739600 , @inclyc wrote:
>
>> What I'm confusing is
>> Which of the following two explanations is the exact meaning of `hhd`?
>>
>> 1. consumes a 32-bit signed integer, then truncates it *inside* printf
>> 2. consumes an 8-bit signed integer
>>
>> If it is the case 1, we should not emit this warning, but N2562 said that it 
>> still seems to be 2 ?
>
> There's some confusion happening that we should clarify first. The sections 
> you've been quoting so far are for the behavior of `fscanf` not `fprintf` and 
> the requirements are different.
>
> For `fprintf`, `hhd` means that `fprintf` consumes an `int`-sized object by 
> calling effectively `signed char value = (signed char)va_arg(list, int);`.
> For `fscanf`, `hhd` means that `fscanf` consumes a pointer in which to store 
> the converted data by effectively calling `signed char *ptr = (signed char 
> *)va_arg(list, signed char *); *ptr = value;`
>
> This patch is currently handling only the `fprintf` cases and is not 
> addressing the `fscanf` ones.

Thanks! This completely solved my doubts. I mistakenly thought that `hhd` also 
corresponds to `signed char` in printf now. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132266

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


[PATCH] D132260: [pseudo] Eliminate a false parse of structured binding declaration.

2022-08-22 Thread 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/pseudo/lib/cxx/CXX.cpp:164
 
+bool isStructuredBinding(const ForestNode* N) {
+  assert(N->symbol() == Symbol::decl_specifier_seq);

nit: specifiesStructuredBinding?
(because the structured binding itself is something bigger)



Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:166
+  assert(N->symbol() == Symbol::decl_specifier_seq);
+  for (const auto &Child : N->descendants()) {
+if (Child.kind() == ForestNode::Terminal) {

If you're going to iterate over tokens, it seems more direct (and cheaper) to 
do it directly:
`for (const Tok& : P.Tokens.tokens(RHS[0].startTokenIndex(), 
RHS[1].startTokenIndex()))`



Comment at: clang-tools-extra/pseudo/test/cxx/structured-binding.cpp:1
+
+// RUN: clang-pseudo -grammar=cxx -source=%s --start-symbol=statement-seq 
--print-forest | FileCheck %s

nit: drop blank line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132260

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


[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-08-22 Thread dongjunduo via Phabricator via cfe-commits
dongjunduo marked 2 inline comments as done.
dongjunduo added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4667
+  // Whether `-ftime-trace` or `-ftime-trace=` are specified
+  if (!TimeTrace && !TimeTraceFile) return;
+

jamieschmeiser wrote:
> The return should be on the next line.  Did you run this through 
> clang-format?  Is it okay with this on the same line?
Yeah. I always use 'clang-format -i xxx.cpp' to check the code style, but some 
old codes may be restyled to a new format which I haven't modified. 

So I restore the code but forget to check the new added code then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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


[PATCH] D132324: [RFC] Remove support for building libc++ with `LLVM_ENABLE_PROJECTS`

2022-08-22 Thread John Ericson via Phabricator via cfe-commits
Ericson2314 added a comment.

I did a "soft revert" in rG952f90b72b3546d6b6b038d410f07ce520c59b48 
 which 
makes it a non-fatal error so everything keeps on working, but I can do a real 
revert too if that is preferred.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132324

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


[PATCH] D132324: [RFC] Remove support for building libc++ with `LLVM_ENABLE_PROJECTS`

2022-08-22 Thread John Ericson via Phabricator via cfe-commits
Ericson2314 added a comment.

https://reviews.llvm.org/rZORG3a209ca6c1b9 This s what I had started doing. but 
I am not sure it is a very good solution. I think I might write a discourse 
post with my thoughts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132324

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


[PATCH] D132342: [X86][AVX512FP16] Relax limitation to AVX512FP16 intrinsics. NFCI

2022-08-22 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei added inline comments.



Comment at: clang/lib/Headers/avx512fp16intrin.h:13
 
+#ifdef __SSE2__
+

RKSimon wrote:
> Doesn't this have to be the general case like in other places in the headers?
> ```
> #if !(defined(_MSC_VER) || defined(__SCE__)) || __has_feature(modules) || 
>  \
> defined(__SSE2__)
> ```
I don't think so. `!(defined(_MSC_VER) || defined(__SCE__)) || 
__has_feature(modules) || defined(__AVX512FP16__)` has already been checked in 
immintrin.h

So if we are preprocessing here, the above condition must be true. Since 
`AVX512FP16` implies `SSE2`, the new condition is constant ture.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132342

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


[PATCH] D132379: [Support] Class for response file expansion

2022-08-22 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff created this revision.
sepavloff added reviewers: rjmccall, kadircet, jackoalan, thakis, rnk.
Herald added a subscriber: hiraditya.
Herald added a project: All.
sepavloff requested review of this revision.
Herald added subscribers: llvm-commits, MaskRay.
Herald added projects: clang, LLVM.

Functions that implement expansion of response and config files depend
on many options, which are passes as arguments. Extending the expansion
requires new options, it in turn causes changing calls in various places
making them even more bulky.

This change introduces a class ExpansionContext, which represents set of
options that control the expansion. Its methods implements expansion of
responce files including config files. It makes extending the expansion
easier.

No functional changes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132379

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
  clang/tools/driver/driver.cpp
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp
  llvm/unittests/Support/CommandLineTest.cpp

Index: llvm/unittests/Support/CommandLineTest.cpp
===
--- llvm/unittests/Support/CommandLineTest.cpp
+++ llvm/unittests/Support/CommandLineTest.cpp
@@ -870,9 +870,9 @@
   // Expand response files.
   llvm::BumpPtrAllocator A;
   llvm::StringSaver Saver(A);
-  ASSERT_TRUE(llvm::cl::ExpandResponseFiles(
-  Saver, llvm::cl::TokenizeGNUCommandLine, Argv, false, true, false,
-  /*CurrentDir=*/StringRef(TestRoot), FS));
+  llvm::cl::ExpansionContext ECtx(Saver, llvm::cl::TokenizeGNUCommandLine, &FS);
+  ECtx.setCurrentDir(TestRoot).setRelativeNames(true);
+  ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
   EXPECT_THAT(Argv, testing::Pointwise(
 StringEquality(),
 {"test/test", "-flag_1", "-option_1", "-option_2",
@@ -933,9 +933,9 @@
 #else
   cl::TokenizerCallback Tokenizer = cl::TokenizeGNUCommandLine;
 #endif
-  ASSERT_FALSE(
-  cl::ExpandResponseFiles(Saver, Tokenizer, Argv, false, false, false,
-  /*CurrentDir=*/llvm::StringRef(TestRoot), FS));
+  llvm::cl::ExpansionContext ECtx(Saver, Tokenizer, &FS);
+  ECtx.setCurrentDir(TestRoot);
+  ASSERT_FALSE(ECtx.expandResponseFiles(Argv));
 
   EXPECT_THAT(Argv,
   testing::Pointwise(StringEquality(),
@@ -972,9 +972,9 @@
 
   BumpPtrAllocator A;
   StringSaver Saver(A);
-  ASSERT_FALSE(cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv,
-   false, false, false,
-   /*CurrentDir=*/StringRef(TestRoot), FS));
+  llvm::cl::ExpansionContext ECtx(Saver, cl::TokenizeGNUCommandLine, &FS);
+  ECtx.setCurrentDir(TestRoot);
+  ASSERT_FALSE(ECtx.expandResponseFiles(Argv));
 
   // ASSERT instead of EXPECT to prevent potential out-of-bounds access.
   ASSERT_EQ(Argv.size(), 1 + NON_RSP_AT_ARGS + 2);
@@ -1007,9 +1007,9 @@
 
   BumpPtrAllocator A;
   StringSaver Saver(A);
-  ASSERT_TRUE(cl::ExpandResponseFiles(Saver, cl::TokenizeGNUCommandLine, Argv,
-  false, true, false,
-  /*CurrentDir=*/StringRef(TestRoot), FS));
+  llvm::cl::ExpansionContext ECtx(Saver, cl::TokenizeGNUCommandLine, &FS);
+  ECtx.setCurrentDir(TestRoot).setRelativeNames(true);
+  ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
   EXPECT_THAT(Argv,
   testing::Pointwise(StringEquality(), {"test/test", "-flag"}));
 }
@@ -1027,9 +1027,9 @@
   SmallVector Argv = {"clang", "@eols.rsp"};
   BumpPtrAllocator A;
   StringSaver Saver(A);
-  ASSERT_TRUE(cl::ExpandResponseFiles(Saver, cl::TokenizeWindowsCommandLine,
-  Argv, true, true, false,
-  /*CurrentDir=*/StringRef(TestRoot), FS));
+  llvm::cl::ExpansionContext ECtx(Saver, cl::TokenizeWindowsCommandLine, &FS);
+  ECtx.setCurrentDir(TestRoot).setMarkEOLs(true).setRelativeNames(true);
+  ASSERT_TRUE(ECtx.expandResponseFiles(Argv));
   const char *Expected[] = {"clang", "-Xclang", "-Wno-whatever", nullptr,
 "input.cpp"};
   ASSERT_EQ(array_lengthof(Expected), Argv.size());
@@ -1126,7 +1126,8 @@
 
   llvm::BumpPtrAllocator A;
   llvm::StringSaver Saver(A);
-  bool Result = llvm::cl::readConfigFile(ConfigFile.path(), Saver, Argv);
+  llvm::cl::ExpansionContext ECtx(Saver);
+  bool Result = ECtx.readConfigFile(ConfigFile.path(), Argv);
 
   EXPECT_TRUE(Result);
   EXPECT_EQ(Argv.size(), 13U);
Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -1153,15 +1153,12 @@
 }
 
 // FName must be an absolute path.
-static llvm::Error ExpandResponseFile(StringRef FName, StringSaver &Saver,
-   

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

2022-08-22 Thread Felipe de Azevedo Piovezan via Phabricator via cfe-commits
fdeazeve added a comment.

I believe this patch might have broken LLDB bots 
https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/46269/

I've confirmed that d2d77e050b32 
 is good 
while e9ef45635b77 
 (this 
commit) breaks some tests.

To repro, a build of llvm+lldb+libcxx+libcxxapi and

  # inside build dir
  /bin/llvm-lit -v 
tools/lldb/test/API/lang/objc/modules-objc-property/TestModulesObjCProperty.py

Alternatively, running the `check-lldb` target should do it. But there is one 
other unrelated failure that might show up in that target


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116203

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


[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

2022-08-22 Thread Felipe de Azevedo Piovezan via Phabricator via cfe-commits
fdeazeve added a comment.

Forgot to add the failure message:

  /usr/include/c++/v1/experimental/propagate_const:138:11: error: no template 
named 'remove_reference_t'; did you mean 'remove_reference'?
typedef remove_reference_t())> element_type;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116203

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


[PATCH] D132295: [clang-format] Change heuristic for locating lambda template arguments

2022-08-22 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel updated this revision to Diff 454513.
rymiel added a comment.

Remove trailing newlines in added format tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132295

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -756,18 +756,85 @@
 
   Tokens = annotate("[]() -> auto {}");
   ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
   EXPECT_TOKEN(Tokens[4], tok::arrow, TT_LambdaArrow);
   EXPECT_TOKEN(Tokens[6], tok::l_brace, TT_LambdaLBrace);
 
   Tokens = annotate("[]() -> auto & {}");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
   EXPECT_TOKEN(Tokens[4], tok::arrow, TT_LambdaArrow);
   EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
 
   Tokens = annotate("[]() -> auto * {}");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
   EXPECT_TOKEN(Tokens[4], tok::arrow, TT_LambdaArrow);
   EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[] {}");
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[] noexcept {}");
+  ASSERT_EQ(Tokens.size(), 6u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[3], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[] -> auto {}");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::arrow, TT_LambdaArrow);
+  EXPECT_TOKEN(Tokens[4], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  () {}");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[8], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  {}");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[6], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  () {}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[9], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  {}");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  () {}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[9], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  {}");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  () {}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[9], tok::l_brace, TT_LambdaLBrace);
+
+  Tokens = annotate("[]  {}");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::l_square, TT_LambdaLSquare);
+  EXPECT_TOKEN(Tokens[2], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[7], tok::l_brace, TT_LambdaLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -21626,6 +21626,18 @@
"g();\n"
"  }\n"
"};\n");
+  verifyFormat("auto L = [](T...) {\n"
+   "  {\n"
+   "f();\n"
+   "g();\n"
+   "  }\n"
+   "};");
+  verifyFormat("auto L = [](T...) {\n"
+   "  {\n"
+   "f();\n"
+   "g();\n"
+   "  }\n"
+   "};");
 
   // Multiple lambdas in the same parentheses change indentation rules. These
   // lambdas are forced to start on new lines.
Index: clang/li

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

2022-08-22 Thread Mark de Wever via Phabricator via cfe-commits
Mordante added a subscriber: aaron.ballman.
Mordante added a comment.

In D126907#3738383 , @erichkeane 
wrote:

> There was a test I dealt with previously where a ton of the header files were 
> run with modules enabled (and an auto generated files), so I'm shocked to 
> find there is MORE differences here.  @ldionne : This is another example 
> where trying to test libcxx is particularly difficult to do apparently.

I disagree; testing libcxx is easy. Unfortunately what's missing is good 
documentation explaining how to test different configurations. A similar libc++ 
related issue came up in another Clang review recently where the libc++ CI 
setup was unclear. Afterwards I had a talk with @aaron.ballman and I agreed to 
improve the documentation. While improving the documentation I noticed some 
issues with our CI setup. So before publishing documentation that doesn't 
reflect reality, I first wanted to fix these issues. While testing my fixes I 
ran into the build failure due to this patch. So now we're at a full circle.

With the revert of this patch it's possible to fix the CI issues for libc++ and 
I will continue with improving the documentation.

Note that this specific issue isn't directly detected in the libc++ CI. This 
would require a full clang build and testing with modules. That's not done, 
full builds are only tested without modules. If wanted I supply a patch how to 
test this configuration in the libc++ CI.

If you need further assistance feel free to reach out to me or other libc++ 
developers, the easiest way to reach us is #libcxx on Discord.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126907

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


[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-22 Thread Phoebe Wang via Phabricator via cfe-commits
pengfei added inline comments.



Comment at: clang/test/CodeGen/X86/Float16-arithmetic.c:42
 _Float16 add2(_Float16 a, _Float16 b, _Float16 c) {
   return a + b + c;
 }

Missing the same ternary operation test in complex tests?



Comment at: clang/test/CodeGen/X86/Float16-arithmetic.c:207
+// CHECK-NEXT:[[EXT:%.*]] = fpext half [[TMP0]] to float
+// CHECK-NEXT:store float [[EXT]], ptr [[RETVAL]], align 2
+// CHECK-NEXT:[[TMP1:%.*]] = load half, ptr [[RETVAL]], align 2

Not sure if we need a fptrunc and store the half value. The following tests 
have the same problem.



Comment at: clang/test/CodeGen/X86/Float16-arithmetic.c:369-370
+// CHECK-NEXT:[[ADD_R:%.*]] = fadd float [[EXT]], [[FNEG]]
+// CHECK-NEXT:store float [[ADD_R]], ptr [[RETVAL]], align 2
+// CHECK-NEXT:[[TMP1:%.*]] = load half, ptr [[RETVAL]], align 2
+// CHECK-NEXT:ret half [[TMP1]]

The operations don't look correct. The following tests have the same problem.



Comment at: clang/test/CodeGen/X86/Float16-complex.c:2
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown 
-target-feature +avx512fp16 -o - | FileCheck %s --check-prefixes=AVX
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -o - | 
FileCheck %s --check-prefixes=X86

AVX/X86 are not good names. Maybe NATIVE/PROMOTE?



Comment at: clang/test/CodeGen/volatile-1.c:195
   ci=ci=ci;
-  // CHECK-NEXT: [[T:%.*]] = load volatile [[INT]], [[INT]]* getelementptr 
inbounds ([[CINT]], [[CINT]]* @ci, i32 0, i32 1)
+  // CHECK: [[T:%.*]] = load volatile [[INT]], [[INT]]* getelementptr inbounds 
([[CINT]], [[CINT]]* @ci, i32 0, i32 1)
   // CHECK-NEXT: store volatile [[INT]] [[T]], [[INT]]* getelementptr inbounds 
([[CINT]], [[CINT]]* @ci, i32 0, i32 1)

What's happening here? Better to add a comment if it is intended.



Comment at: clang/test/CodeGenCXX/volatile-1.cpp:241
   __imag ci = __imag ci = __imag ci;
-  // CHECK-NEXT: [[T:%.*]] = load volatile [[INT]], [[INT]]* getelementptr 
inbounds ([[CINT]], [[CINT]]* @ci, i32 0, i32 1)
+  // CHECK: [[T:%.*]] = load volatile [[INT]], [[INT]]* getelementptr inbounds 
([[CINT]], [[CINT]]* @ci, i32 0, i32 1)
   // CHECK-NEXT: store volatile [[INT]] [[T]], [[INT]]* getelementptr inbounds 
([[CINT]], [[CINT]]* @ci, i32 0, i32 1)

ditto.


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

https://reviews.llvm.org/D113107

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


[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-22 Thread weiyi via Phabricator via cfe-commits
wyt updated this revision to Diff 454523.
wyt marked 8 inline comments as done.
wyt added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131614

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -71,14 +71,25 @@
   std::vector> &BlockStates;
 };
 
+// FIXME: Rename to `checkDataflow` after usages of the overload that applies to
+// `CFGStmt` have been replaced.
+//
+/// Runs dataflow analysis (specified from `MakeAnalysis`) and the
+/// `PostVisitCFG` function (if provided) on the body of the function that
+/// matches `TargetFuncMatcher` in code snippet `Code`. `VerifyResults` checks
+/// that the results from the analysis are correct.
+///
+/// Requirements:
+///
+///  `AnalysisT` contains a type `Lattice`.
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,
 ast_matchers::internal::Matcher TargetFuncMatcher,
 std::function MakeAnalysis,
-std::function
-PostVisitStmt,
+PostVisitCFG,
 std::function VerifyResults, ArrayRef Args,
 const tooling::FileContentMappings &VirtualMappedFiles = {}) {
   llvm::Annotations AnnotatedCode(Code);
@@ -112,13 +123,14 @@
   Environment Env(DACtx, *F);
   auto Analysis = MakeAnalysis(Context, Env);
 
-  std::function
-  PostVisitStmtClosure = nullptr;
-  if (PostVisitStmt != nullptr) {
-PostVisitStmtClosure = [&PostVisitStmt, &Context](
-   const CFGStmt &Stmt,
-   const TypeErasedDataflowAnalysisState &State) {
-  PostVisitStmt(Context, Stmt, State);
+  std::function
+  PostVisitCFGClosure = nullptr;
+  if (PostVisitCFG) {
+PostVisitCFGClosure = [&PostVisitCFG, &Context](
+  const CFGElement &Element,
+  const TypeErasedDataflowAnalysisState &State) {
+  PostVisitCFG(Context, Element, State);
 };
   }
 
@@ -130,7 +142,7 @@
 
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env,
-   PostVisitStmtClosure);
+   PostVisitCFGClosure);
   if (!MaybeBlockStates)
 return MaybeBlockStates.takeError();
   auto &BlockStates = *MaybeBlockStates;
@@ -141,6 +153,32 @@
   return llvm::Error::success();
 }
 
+template 
+llvm::Error checkDataflow(
+llvm::StringRef Code,
+ast_matchers::internal::Matcher TargetFuncMatcher,
+std::function MakeAnalysis,
+std::function
+PostVisitStmt,
+std::function VerifyResults, ArrayRef Args,
+const tooling::FileContentMappings &VirtualMappedFiles = {}) {
+  std::function
+  PostVisitCFG = nullptr;
+  if (PostVisitStmt) {
+PostVisitCFG =
+[&PostVisitStmt](ASTContext &Context, const CFGElement &Element,
+ const TypeErasedDataflowAnalysisState &State) {
+  if (auto Stmt = Element.getAs()) {
+PostVisitStmt(Context, *Stmt, State);
+  }
+};
+  }
+  return checkDataflowOnCFG(Code, TargetFuncMatcher, MakeAnalysis, PostVisitCFG,
+VerifyResults, Args, VirtualMappedFiles);
+}
+
 // Runs dataflow on the body of the function that matches `TargetFuncMatcher` in
 // code snippet `Code`. Requires: `AnalysisT` contains a type `Lattice`.
 template 
@@ -157,9 +195,9 @@
 const tooling::FileContentMappings &VirtualMappedFiles = {}) {
   using StateT = DataflowAnalysisState;
 
-  return checkDataflow(
+  return checkDataflowOnCFG(
   Code, std::move(TargetFuncMatcher), std::move(MakeAnalysis),
-  /*PostVisitStmt=*/nullptr,
+  /*PostVisitCFG=*/nullptr,
   [&VerifyResults](AnalysisData AnalysisData) {
 if (AnalysisData.BlockStates.empty()) {
   VerifyResults({}, AnalysisData.ASTCtx);
@@ -180,9 +218,13 @@
   AnalysisData.CFCtx, AnalysisData.BlockStates, *Block,
   AnalysisData.Env, AnalysisData.Analysis,
   [&Results,
-   &Annotations](const clang::CFGStmt &Stmt,
+   &Annotations](const clang::CFGElement &Element,
  const TypeErasedDataflowAnalysisState &State) {
-auto It = Annotations.find(Stmt.getStmt());
+// FIXME: Extend testing annotations to non statement constructs
+auto Stmt = Element.getAs();
+if (!Stmt)
+ 

[PATCH] D132147: [clang][dataflow] Refactor `TestingSupport.h`

2022-08-22 Thread weiyi via Phabricator via cfe-commits
wyt updated this revision to Diff 454525.
wyt added a comment.

Propagate change from parent patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132147

Files:
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1240,43 +1240,45 @@
 /*IgnoreSmartPointerDereference=*/true};
 std::vector Diagnostics;
 llvm::Error Error = checkDataflow(
-SourceCode, FuncMatcher,
-[Options](ASTContext &Ctx, Environment &) {
-  return UncheckedOptionalAccessModel(Ctx, Options);
+/*AI:AnalysisInputs=*/{
+.Code = SourceCode,
+.TargetFuncMatcher = FuncMatcher,
+.MakeAnalysis =
+[Options](ASTContext &Ctx, Environment &) {
+  return UncheckedOptionalAccessModel(Ctx, Options);
+},
+.PostVisitCFG =
+[&Diagnostics,
+ Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
+ASTContext &Ctx, const CFGElement &Elt,
+const TypeErasedDataflowAnalysisState &State) mutable {
+  auto Stmt = Elt.getAs();
+  if (!Stmt) {
+return;
+  }
+  auto StmtDiagnostics =
+  Diagnoser.diagnose(Ctx, Stmt->getStmt(), State.Env);
+  llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
+},
+.ASTBuildArgs = {"-fsyntax-only", "-std=c++17",
+ "-Wno-undefined-inline"},
+.ASTBuildVirtualMappedFiles = FileContents,
 },
-[&Diagnostics, Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
-ASTContext &Ctx, const CFGStmt &Stmt,
-const TypeErasedDataflowAnalysisState &State) mutable {
-  auto StmtDiagnostics =
-  Diagnoser.diagnose(Ctx, Stmt.getStmt(), State.Env);
-  llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
-},
-[&Diagnostics](AnalysisData AnalysisData) {
-  auto &SrcMgr = AnalysisData.ASTCtx.getSourceManager();
-
+/*VerifyResults=*/[&Diagnostics](llvm::DenseMap
+ &Annotations,
+ AnalysisOutputs &AO) {
   llvm::DenseSet AnnotationLines;
-  for (const auto &Pair : AnalysisData.Annotations) {
-auto *Stmt = Pair.getFirst();
-AnnotationLines.insert(
-SrcMgr.getPresumedLineNumber(Stmt->getBeginLoc()));
-// We add both the begin and end locations, so that if the
-// statement spans multiple lines then the test will fail.
-//
-// FIXME: Going forward, we should change this to instead just
-// get the single line number from the annotation itself, rather
-// than looking at the statement it's attached to.
-AnnotationLines.insert(
-SrcMgr.getPresumedLineNumber(Stmt->getEndLoc()));
+  for (const auto &[Line, _] : Annotations) {
+AnnotationLines.insert(Line);
   }
-
+  auto &SrcMgr = AO.ASTCtx.getSourceManager();
   llvm::DenseSet DiagnosticLines;
   for (SourceLocation &Loc : Diagnostics) {
 DiagnosticLines.insert(SrcMgr.getPresumedLineNumber(Loc));
   }
 
   EXPECT_THAT(DiagnosticLines, ContainerEq(AnnotationLines));
-},
-{"-fsyntax-only", "-std=c++17", "-Wno-undefined-inline"}, FileContents);
+});
 if (Error)
   FAIL() << llvm::toString(std::move(Error));
   }
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -56,47 +56,130 @@
 
 namespace test {
 
-// Returns assertions based on annotations that are present after statements in
-// `AnnotatedCode`.
-llvm::Expected>
-buildStatementToAnnotationMapping(const FunctionDecl *Func,
-  llvm::Annotations AnnotatedCode);
+/// Arguments for building the dataflow analysis.
+template  struct AnalysisInputs {
+  /// Input code that is analyzed.
+  llvm::StringRef Code;
+  /// The body of the function which matches this matcher is analyzed.
+  ast_matchers::internal::Matcher TargetFuncMatcher;
+  /// T

[PATCH] D132377: [clang][dataflow] Add `SetupTest` parameter for `AnalysisInputs`.

2022-08-22 Thread weiyi via Phabricator via cfe-commits
wyt updated this revision to Diff 454527.
wyt added a comment.
Herald added a reviewer: NoQ.

Propagate change from parent patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132377

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -56,28 +56,6 @@
 
 namespace test {
 
-/// Arguments for building the dataflow analysis.
-template  struct AnalysisInputs {
-  /// Input code that is analyzed.
-  llvm::StringRef Code;
-  /// The body of the function which matches this matcher is analyzed.
-  ast_matchers::internal::Matcher TargetFuncMatcher;
-  /// The analysis to be run is constructed with this function that takes as
-  /// argument the AST generated from the code being analyzed and the
-  /// initial state from which the analysis starts with.
-  std::function MakeAnalysis;
-  /// If provided, this function is applied on each CFG element after the
-  /// analysis has been run.
-  std::function
-  PostVisitCFG = nullptr;
-
-  /// Options for building the AST context.
-  ArrayRef ASTBuildArgs = {};
-  /// Options for building the AST context.
-  const tooling::FileContentMappings &ASTBuildVirtualMappedFiles = {};
-};
-
 /// Contains data structures required and produced by a dataflow analysis run.
 struct AnalysisOutputs {
   /// Input code that is analyzed. Points within the code may be marked with
@@ -106,60 +84,43 @@
   llvm::ArrayRef> BlockStates;
 };
 
+/// Arguments for building the dataflow analysis.
+template  struct AnalysisInputs {
+  /// Input code that is analyzed.
+  llvm::StringRef Code;
+  /// The body of the function which matches this matcher is analyzed.
+  ast_matchers::internal::Matcher TargetFuncMatcher;
+  /// The analysis to be run is constructed with this function that takes as
+  /// argument the AST generated from the code being analyzed and the
+  /// initial state from which the analysis starts with.
+  std::function MakeAnalysis;
+  /// If provided, this function is applied on each CFG element after the
+  /// analysis has been run.
+  std::function
+  PostVisitCFG = nullptr;
+  /// If provided, this function is executed immediately before running the
+  /// dataflow analysis to allow for additional setup. All fields in the
+  /// `AnalysisOutputs` argument will be initialized except for the
+  /// `BlockStates` field which is only computed later during the analysis.
+  std::function SetupTest = nullptr;
+
+  /// Options for building the AST context.
+  ArrayRef ASTBuildArgs = {};
+  /// Options for building the AST context.
+  const tooling::FileContentMappings &ASTBuildVirtualMappedFiles = {};
+};
+
 /// Returns assertions based on annotations that are present after statements in
 /// `AnnotatedCode`.
 llvm::Expected>
 buildStatementToAnnotationMapping(const FunctionDecl *Func,
   llvm::Annotations AnnotatedCode);
 
-/// Returns line numbers and content of the annotations in `AO.Code`.
+/// Returns line numbers and content of the annotations in `AnnotatedCode`.
 llvm::DenseMap
-getAnnotationLinesAndContent(AnalysisOutputs &AO);
-
-// FIXME: Return a string map instead of a vector of pairs.
-//
-/// Returns the analysis states at each annotated statement in `AO.Code`.
-template 
-llvm::Expected>>>
-getAnnotationStates(AnalysisOutputs &AO) {
-  using StateT = DataflowAnalysisState;
-  // FIXME: Extend to annotations on non-statement constructs.
-  // Get annotated statements.
-  llvm::Expected>
-  MaybeStmtToAnnotations =
-  buildStatementToAnnotationMapping(AO.Target, AO.Code);
-  if (!MaybeStmtToAnnotations)
-return MaybeStmtToAnnotations.takeError();
-  auto &StmtToAnnotations = *MaybeStmtToAnnotations;
-
-  // Compute a map from statement annotations to the state computed
-  // for the program point immediately after the annotated statement.
-  std::vector> Results;
-  for (const CFGBlock *Block : AO.CFCtx.getCFG()) {
-// Skip blocks that were not evaluated.
-if (!AO.BlockStates[Block->getBlockID()])
-  continue;
-
-transferBlock(
-AO.CFCtx, AO.BlockStates, *Block, AO.InitEnv, AO.Analysis,
-[&Results,
- &StmtToAnnotations](const clang::CFGElement &Element,
- const TypeErasedDataflowAnalysisState &State) {
-  auto Stmt = Element.getAs();
-  if (!Stmt)
-return;
-  auto It = StmtToAnnotations.find(Stmt->getStmt());
-  if (It == StmtToAnnotations.end())
-return;
-  auto *Lattice =
-  llvm::any_cast(&State.Latt

[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

2022-08-22 Thread Felipe de Azevedo Piovezan via Phabricator via cfe-commits
fdeazeve added a comment.

It seems like non-lldb bots are also failing, if it makes it easier to 
reproduce:
 
https://green.lab.llvm.org/green/view/Clang/job/clang-stage1-RA/30837/consoleFull#-54352964249ba4694-19c4-4d7e-bec5-911270d8a58c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116203

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


[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

2022-08-22 Thread weiyi via Phabricator via cfe-commits
wyt updated this revision to Diff 454532.
wyt added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131614

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -71,14 +71,25 @@
   std::vector> &BlockStates;
 };
 
+// FIXME: Rename to `checkDataflow` after usages of the overload that applies to
+// `CFGStmt` have been replaced.
+//
+/// Runs dataflow analysis (specified from `MakeAnalysis`) and the
+/// `PostVisitCFG` function (if provided) on the body of the function that
+/// matches `TargetFuncMatcher` in code snippet `Code`. `VerifyResults` checks
+/// that the results from the analysis are correct.
+///
+/// Requirements:
+///
+///  `AnalysisT` contains a type `Lattice`.
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,
 ast_matchers::internal::Matcher TargetFuncMatcher,
 std::function MakeAnalysis,
-std::function
-PostVisitStmt,
+PostVisitCFG,
 std::function VerifyResults, ArrayRef Args,
 const tooling::FileContentMappings &VirtualMappedFiles = {}) {
   llvm::Annotations AnnotatedCode(Code);
@@ -112,13 +123,14 @@
   Environment Env(DACtx, *F);
   auto Analysis = MakeAnalysis(Context, Env);
 
-  std::function
-  PostVisitStmtClosure = nullptr;
-  if (PostVisitStmt != nullptr) {
-PostVisitStmtClosure = [&PostVisitStmt, &Context](
-   const CFGStmt &Stmt,
-   const TypeErasedDataflowAnalysisState &State) {
-  PostVisitStmt(Context, Stmt, State);
+  std::function
+  PostVisitCFGClosure = nullptr;
+  if (PostVisitCFG) {
+PostVisitCFGClosure = [&PostVisitCFG, &Context](
+  const CFGElement &Element,
+  const TypeErasedDataflowAnalysisState &State) {
+  PostVisitCFG(Context, Element, State);
 };
   }
 
@@ -130,7 +142,7 @@
 
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env,
-   PostVisitStmtClosure);
+   PostVisitCFGClosure);
   if (!MaybeBlockStates)
 return MaybeBlockStates.takeError();
   auto &BlockStates = *MaybeBlockStates;
@@ -141,6 +153,32 @@
   return llvm::Error::success();
 }
 
+template 
+llvm::Error checkDataflow(
+llvm::StringRef Code,
+ast_matchers::internal::Matcher TargetFuncMatcher,
+std::function MakeAnalysis,
+std::function
+PostVisitStmt,
+std::function VerifyResults, ArrayRef Args,
+const tooling::FileContentMappings &VirtualMappedFiles = {}) {
+  std::function
+  PostVisitCFG = nullptr;
+  if (PostVisitStmt) {
+PostVisitCFG =
+[&PostVisitStmt](ASTContext &Context, const CFGElement &Element,
+ const TypeErasedDataflowAnalysisState &State) {
+  if (auto Stmt = Element.getAs()) {
+PostVisitStmt(Context, *Stmt, State);
+  }
+};
+  }
+  return checkDataflowOnCFG(Code, TargetFuncMatcher, MakeAnalysis, PostVisitCFG,
+VerifyResults, Args, VirtualMappedFiles);
+}
+
 // Runs dataflow on the body of the function that matches `TargetFuncMatcher` in
 // code snippet `Code`. Requires: `AnalysisT` contains a type `Lattice`.
 template 
@@ -157,9 +195,9 @@
 const tooling::FileContentMappings &VirtualMappedFiles = {}) {
   using StateT = DataflowAnalysisState;
 
-  return checkDataflow(
+  return checkDataflowOnCFG(
   Code, std::move(TargetFuncMatcher), std::move(MakeAnalysis),
-  /*PostVisitStmt=*/nullptr,
+  /*PostVisitCFG=*/nullptr,
   [&VerifyResults](AnalysisData AnalysisData) {
 if (AnalysisData.BlockStates.empty()) {
   VerifyResults({}, AnalysisData.ASTCtx);
@@ -180,9 +218,13 @@
   AnalysisData.CFCtx, AnalysisData.BlockStates, *Block,
   AnalysisData.Env, AnalysisData.Analysis,
   [&Results,
-   &Annotations](const clang::CFGStmt &Stmt,
+   &Annotations](const clang::CFGElement &Element,
  const TypeErasedDataflowAnalysisState &State) {
-auto It = Annotations.find(Stmt.getStmt());
+// FIXME: Extend testing annotations to non statement constructs
+auto Stmt = Element.getAs();
+if (!Stmt)
+  return;
+auto I

[PATCH] D132324: [RFC] Remove support for building libc++ with `LLVM_ENABLE_PROJECTS`

2022-08-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D132324#3739646 , @Ericson2314 
wrote:

> I did a "soft revert" in rG952f90b72b3546d6b6b038d410f07ce520c59b48 
>  which 
> makes it a non-fatal error so everything keeps on working, but I can do a 
> real revert too if that is preferred.

It doesn't seem to have helped the sphinx publish bot: 
https://lab.llvm.org/buildbot/#/builders/89/builds/31676

  CMake Error at 
/home/buildbot/as-worker-4/publish-sphinx-docs/llvm-project/libcxxabi/CMakeLists.txt:138
 (message):
LIBCXXABI_LIBCXX_INCLUDES= is not a valid directory.  Please provide the
path to where the libc++ headers have been installed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132324

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


[PATCH] D132258: clang/apple: Infer simulator env from -mios-simulator-version-min flag

2022-08-22 Thread Hans Wennborg via Phabricator via cfe-commits
hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

lgtm since it matches xcode clang

For the commit message, perhaps the "(Xcode's clang already behaves like this," 
part should be moved further down, probably to just after "This patch makes it 
so that", since the "already behaves like this" should refer to the new 
behaviour.


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

https://reviews.llvm.org/D132258

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


[PATCH] D131616: [clang][dataflow] Generalise match switch utility to other AST types and add a `CFGMatchSwitch` which currently handles `CFGStmt` and `CFGInitializer`.

2022-08-22 Thread weiyi via Phabricator via cfe-commits
wyt updated this revision to Diff 454534.
wyt added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131616

Files:
  clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h
  clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
  clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
  clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
@@ -5,12 +5,6 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===--===//
-//
-//  This file defines a simplistic version of Constant Propagation as an example
-//  of a forward, monotonic dataflow analysis. The analysis tracks all
-//  variables in the scope, but lacks escape analysis.
-//
-//===--===//
 
 #include "clang/Analysis/FlowSensitive/MatchSwitch.h"
 #include "TestingSupport.h"
Index: clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
===
--- clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
+++ clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
@@ -4,6 +4,7 @@
   )
 
 add_clang_unittest(ClangAnalysisFlowSensitiveTests
+  CFGMatchSwitchTest.cpp
   ChromiumCheckModelTest.cpp
   DataflowAnalysisContextTest.cpp
   DataflowEnvironmentTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
@@ -0,0 +1,132 @@
+//===- unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp ===//
+//
+// 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 "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/StringRef.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace dataflow;
+using namespace ast_matchers;
+
+namespace {
+// State for tracking the number of matches on each kind of CFGElement by the
+// CFGMatchSwitch. Currently only tracks CFGStmt and CFGInitializer.
+struct CFGElementMatches {
+  unsigned StmtMatches = 0;
+  unsigned InitializerMatches = 0;
+};
+
+// Returns a match switch that counts the number of local variables
+// (singly-declared) and fields initialized to the integer literal 42.
+auto buildCFGMatchSwitch() {
+  return CFGMatchSwitchBuilder()
+  .CaseOfCFGStmt(
+  declStmt(hasSingleDecl(
+  varDecl(hasInitializer(integerLiteral(equals(42)),
+  [](const DeclStmt *, const MatchFinder::MatchResult &,
+ CFGElementMatches &Counter) { Counter.StmtMatches++; })
+  .CaseOfCFGInit(
+  cxxCtorInitializer(withInitializer(integerLiteral(equals(42,
+  [](const CXXCtorInitializer *, const MatchFinder::MatchResult &,
+ CFGElementMatches &Counter) { Counter.InitializerMatches++; })
+  .Build();
+}
+
+// Runs the match switch `MS` on the control flow graph generated from `Code`,
+// tracking information in state `S`. For simplicity, this test utility is
+// restricted to CFGs with a single control flow block (excluding entry and
+// exit blocks) - generated by `Code` with sequential flow (i.e. no branching).
+//
+// Requirements:
+//
+//  `Code` must contain a function named `f`, the body of this function will be
+//  used to generate the CFG.
+template 
+void applySwitchToCode(CFGMatchSwitch &MS, State &S,
+   llvm::StringRef Code) {
+  auto Unit = tooling::buildASTFromCodeWithArgs(Code, {"-Wno-unused-value"});
+  auto &Ctx = Unit->getASTContext();
+  const auto *F = selectFirst(
+  "f", match(functionDecl(isDefinition(), hasName("f")).bind("f"), Ctx));
+
+  CFG::BuildOptions BO;
+  BO.AddInitializers = true;
+
+  auto CFG = CFG::buildCFG(F, F->getBody(), &Ctx, BO);
+  auto CFGBlock = *CFG->getEntry().succ_begin();
+  for (auto &Elt : CFGBlock->Elements) {
+MS(Elt, Ctx, S);
+  }
+}
+
+TEST(CFGMatchSwitchTest, NoInitial

[PATCH] D132147: [clang][dataflow] Refactor `TestingSupport.h`

2022-08-22 Thread weiyi via Phabricator via cfe-commits
wyt updated this revision to Diff 454535.
wyt added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132147

Files:
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1240,43 +1240,45 @@
 /*IgnoreSmartPointerDereference=*/true};
 std::vector Diagnostics;
 llvm::Error Error = checkDataflow(
-SourceCode, FuncMatcher,
-[Options](ASTContext &Ctx, Environment &) {
-  return UncheckedOptionalAccessModel(Ctx, Options);
+/*AI:AnalysisInputs=*/{
+.Code = SourceCode,
+.TargetFuncMatcher = FuncMatcher,
+.MakeAnalysis =
+[Options](ASTContext &Ctx, Environment &) {
+  return UncheckedOptionalAccessModel(Ctx, Options);
+},
+.PostVisitCFG =
+[&Diagnostics,
+ Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
+ASTContext &Ctx, const CFGElement &Elt,
+const TypeErasedDataflowAnalysisState &State) mutable {
+  auto Stmt = Elt.getAs();
+  if (!Stmt) {
+return;
+  }
+  auto StmtDiagnostics =
+  Diagnoser.diagnose(Ctx, Stmt->getStmt(), State.Env);
+  llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
+},
+.ASTBuildArgs = {"-fsyntax-only", "-std=c++17",
+ "-Wno-undefined-inline"},
+.ASTBuildVirtualMappedFiles = FileContents,
 },
-[&Diagnostics, Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
-ASTContext &Ctx, const CFGStmt &Stmt,
-const TypeErasedDataflowAnalysisState &State) mutable {
-  auto StmtDiagnostics =
-  Diagnoser.diagnose(Ctx, Stmt.getStmt(), State.Env);
-  llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
-},
-[&Diagnostics](AnalysisData AnalysisData) {
-  auto &SrcMgr = AnalysisData.ASTCtx.getSourceManager();
-
+/*VerifyResults=*/[&Diagnostics](llvm::DenseMap
+ &Annotations,
+ AnalysisOutputs &AO) {
   llvm::DenseSet AnnotationLines;
-  for (const auto &Pair : AnalysisData.Annotations) {
-auto *Stmt = Pair.getFirst();
-AnnotationLines.insert(
-SrcMgr.getPresumedLineNumber(Stmt->getBeginLoc()));
-// We add both the begin and end locations, so that if the
-// statement spans multiple lines then the test will fail.
-//
-// FIXME: Going forward, we should change this to instead just
-// get the single line number from the annotation itself, rather
-// than looking at the statement it's attached to.
-AnnotationLines.insert(
-SrcMgr.getPresumedLineNumber(Stmt->getEndLoc()));
+  for (const auto &[Line, _] : Annotations) {
+AnnotationLines.insert(Line);
   }
-
+  auto &SrcMgr = AO.ASTCtx.getSourceManager();
   llvm::DenseSet DiagnosticLines;
   for (SourceLocation &Loc : Diagnostics) {
 DiagnosticLines.insert(SrcMgr.getPresumedLineNumber(Loc));
   }
 
   EXPECT_THAT(DiagnosticLines, ContainerEq(AnnotationLines));
-},
-{"-fsyntax-only", "-std=c++17", "-Wno-undefined-inline"}, FileContents);
+});
 if (Error)
   FAIL() << llvm::toString(std::move(Error));
   }
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -56,47 +56,130 @@
 
 namespace test {
 
-// Returns assertions based on annotations that are present after statements in
-// `AnnotatedCode`.
-llvm::Expected>
-buildStatementToAnnotationMapping(const FunctionDecl *Func,
-  llvm::Annotations AnnotatedCode);
+/// Arguments for building the dataflow analysis.
+template  struct AnalysisInputs {
+  /// Input code that is analyzed.
+  llvm::StringRef Code;
+  /// The body of the function which matches this matcher is analyzed.
+  ast_matchers::internal::Matcher TargetFuncMatcher;
+  /// The analysis to be run is con

[PATCH] D132388: [pseudo] Fix HeadsPartition is not initialized correctly.

2022-08-22 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: All.
hokein requested review of this revision.
Herald added a subscriber: alextsao1999.
Herald added a project: clang-tools-extra.

The bug was that if we recover from the token 0, we will make the
Heads empty (Line646), which results no recovery being applied.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132388

Files:
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/unittests/GLRTest.cpp


Index: clang-tools-extra/pseudo/unittests/GLRTest.cpp
===
--- clang-tools-extra/pseudo/unittests/GLRTest.cpp
+++ clang-tools-extra/pseudo/unittests/GLRTest.cpp
@@ -652,6 +652,31 @@
 "[  1, end) └─word := \n");
 }
 
+TEST_F(GLRTest, RecoveryFromStartOfInput) {
+  build(R"bnf(
+_ := start [recover=Fallback] EOF
+
+start := IDENTIFIER
+  )bnf");
+  TestLang.Table = LRTable::buildSLR(TestLang.G);
+  bool fallback_recovered = false;
+  auto fallback = [&](Token::Index Start, const TokenStream & Code) {
+fallback_recovered = true;
+return Code.tokens().size();
+  };
+  TestLang.RecoveryStrategies.try_emplace(
+  extensionID("Fallback"),
+  fallback);
+  clang::LangOptions LOptions;
+  TokenStream Tokens = cook(lex("?", LOptions), LOptions);
+
+  const ForestNode &Parsed =
+  glrParse({Tokens, Arena, GSStack}, id("start"), TestLang);
+  EXPECT_TRUE(fallback_recovered);
+  EXPECT_EQ(Parsed.dumpRecursive(TestLang.G),
+"[  0, end) start := \n");
+}
+
 TEST_F(GLRTest, RepeatedRecovery) {
   // We require multiple steps of recovery at eof and then a reduction in order
   // to successfully parse.
Index: clang-tools-extra/pseudo/lib/GLR.cpp
===
--- clang-tools-extra/pseudo/lib/GLR.cpp
+++ clang-tools-extra/pseudo/lib/GLR.cpp
@@ -31,7 +31,6 @@
   const TokenStream &Tokens,
   const Language &Lang) {
   assert(Strategy != 0);
-  assert(Begin > 0);
   if (auto S = Lang.RecoveryStrategies.lookup(Strategy))
 return S(Begin, Tokens);
   return Token::Invalid;
@@ -614,7 +613,7 @@
   // Invariant: Heads is partitioned by source: {shifted | reduced}.
   // HeadsPartition is the index of the first head formed by reduction.
   // We use this to discard and recreate the reduced heads during recovery.
-  unsigned HeadsPartition = 0;
+  unsigned HeadsPartition = Heads.size();
   std::vector NextHeads;
   auto MaybeGC = [&, Roots(std::vector{}), I(0u)]() mutable 
{
 assert(NextHeads.empty() && "Running GC at the wrong time!");


Index: clang-tools-extra/pseudo/unittests/GLRTest.cpp
===
--- clang-tools-extra/pseudo/unittests/GLRTest.cpp
+++ clang-tools-extra/pseudo/unittests/GLRTest.cpp
@@ -652,6 +652,31 @@
 "[  1, end) └─word := \n");
 }
 
+TEST_F(GLRTest, RecoveryFromStartOfInput) {
+  build(R"bnf(
+_ := start [recover=Fallback] EOF
+
+start := IDENTIFIER
+  )bnf");
+  TestLang.Table = LRTable::buildSLR(TestLang.G);
+  bool fallback_recovered = false;
+  auto fallback = [&](Token::Index Start, const TokenStream & Code) {
+fallback_recovered = true;
+return Code.tokens().size();
+  };
+  TestLang.RecoveryStrategies.try_emplace(
+  extensionID("Fallback"),
+  fallback);
+  clang::LangOptions LOptions;
+  TokenStream Tokens = cook(lex("?", LOptions), LOptions);
+
+  const ForestNode &Parsed =
+  glrParse({Tokens, Arena, GSStack}, id("start"), TestLang);
+  EXPECT_TRUE(fallback_recovered);
+  EXPECT_EQ(Parsed.dumpRecursive(TestLang.G),
+"[  0, end) start := \n");
+}
+
 TEST_F(GLRTest, RepeatedRecovery) {
   // We require multiple steps of recovery at eof and then a reduction in order
   // to successfully parse.
Index: clang-tools-extra/pseudo/lib/GLR.cpp
===
--- clang-tools-extra/pseudo/lib/GLR.cpp
+++ clang-tools-extra/pseudo/lib/GLR.cpp
@@ -31,7 +31,6 @@
   const TokenStream &Tokens,
   const Language &Lang) {
   assert(Strategy != 0);
-  assert(Begin > 0);
   if (auto S = Lang.RecoveryStrategies.lookup(Strategy))
 return S(Begin, Tokens);
   return Token::Invalid;
@@ -614,7 +613,7 @@
   // Invariant: Heads is partitioned by source: {shifted | reduced}.
   // HeadsPartition is the index of the first head formed by reduction.
   // We use this to discard and recreate the reduced heads during recovery.
-  unsigned HeadsPartition = 0;
+  unsigned HeadsPartition = Heads.size();
   std::vector NextHeads;
   auto MaybeGC = [&, Roots(std::vector{}), I(0u)]() mutable {
 assert(NextHeads.empty() && "Running GC at the wrong time!");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

2022-08-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D126907#3739750 , @Mordante wrote:

> In D126907#3738383 , @erichkeane 
> wrote:
>
>> There was a test I dealt with previously where a ton of the header files 
>> were run with modules enabled (and an auto generated files), so I'm shocked 
>> to find there is MORE differences here.  @ldionne : This is another example 
>> where trying to test libcxx is particularly difficult to do apparently.
>
> I disagree; testing libcxx is easy.

FWIW, that's not been my experience. I can't even get libcxx to build for me on 
Windows with Visual Studio cmake integration, let alone test it. These are the 
results I got when I tried again today from the instructions from the website:

  CMake Error in F:/source/llvm-project/libcxx/benchmarks/CMakeLists.txt:
No known features for CXX compiler
  
"Clang"
  
version 16.0.0.
  
  
  CMake Generate step failed.  Build files cannot be regenerated correctly.
  FAILED: runtimes/runtimes-stamps/runtimes-configure 

That said, I recall at one point I was able to get libcxx to build for me, but 
then I ran into issues getting the tests to run (and we document this on the 
website: "Building with Visual Studio currently does not permit running 
tests."). The result is that I stopped trying to build or test libcxx locally 
ages ago and I rely entirely on the bots to tell me if something's gone wrong.

> Unfortunately what's missing is good documentation explaining how to test 
> different configurations. A similar libc++ related issue came up in another 
> Clang review recently where the libc++ CI setup was unclear. Afterwards I had 
> a talk with @aaron.ballman and I agreed to improve the documentation. While 
> improving the documentation I noticed some issues with our CI setup. So 
> before publishing documentation that doesn't reflect reality, I first wanted 
> to fix these issues. While testing my fixes I ran into the build failure due 
> to this patch. So now we're at a full circle.

And I greatly appreciate the improvements that have been going on here!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126907

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


[PATCH] D132388: [pseudo] Fix HeadsPartition is not initialized correctly.

2022-08-22 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132388

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


[PATCH] D132377: [clang][dataflow] Add `SetupTest` parameter for `AnalysisInputs`.

2022-08-22 Thread weiyi via Phabricator via cfe-commits
wyt updated this revision to Diff 454537.
wyt added a comment.

Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132377

Files:
  clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -56,28 +56,6 @@
 
 namespace test {
 
-/// Arguments for building the dataflow analysis.
-template  struct AnalysisInputs {
-  /// Input code that is analyzed.
-  llvm::StringRef Code;
-  /// The body of the function which matches this matcher is analyzed.
-  ast_matchers::internal::Matcher TargetFuncMatcher;
-  /// The analysis to be run is constructed with this function that takes as
-  /// argument the AST generated from the code being analyzed and the
-  /// initial state from which the analysis starts with.
-  std::function MakeAnalysis;
-  /// If provided, this function is applied on each CFG element after the
-  /// analysis has been run.
-  std::function
-  PostVisitCFG = nullptr;
-
-  /// Options for building the AST context.
-  ArrayRef ASTBuildArgs = {};
-  /// Options for building the AST context.
-  const tooling::FileContentMappings &ASTBuildVirtualMappedFiles = {};
-};
-
 /// Contains data structures required and produced by a dataflow analysis run.
 struct AnalysisOutputs {
   /// Input code that is analyzed. Points within the code may be marked with
@@ -106,60 +84,43 @@
   llvm::ArrayRef> BlockStates;
 };
 
+/// Arguments for building the dataflow analysis.
+template  struct AnalysisInputs {
+  /// Input code that is analyzed.
+  llvm::StringRef Code;
+  /// The body of the function which matches this matcher is analyzed.
+  ast_matchers::internal::Matcher TargetFuncMatcher;
+  /// The analysis to be run is constructed with this function that takes as
+  /// argument the AST generated from the code being analyzed and the
+  /// initial state from which the analysis starts with.
+  std::function MakeAnalysis;
+  /// If provided, this function is applied on each CFG element after the
+  /// analysis has been run.
+  std::function
+  PostVisitCFG = nullptr;
+  /// If provided, this function is executed immediately before running the
+  /// dataflow analysis to allow for additional setup. All fields in the
+  /// `AnalysisOutputs` argument will be initialized except for the
+  /// `BlockStates` field which is only computed later during the analysis.
+  std::function SetupTest = nullptr;
+
+  /// Options for building the AST context.
+  ArrayRef ASTBuildArgs = {};
+  /// Options for building the AST context.
+  const tooling::FileContentMappings &ASTBuildVirtualMappedFiles = {};
+};
+
 /// Returns assertions based on annotations that are present after statements in
 /// `AnnotatedCode`.
 llvm::Expected>
 buildStatementToAnnotationMapping(const FunctionDecl *Func,
   llvm::Annotations AnnotatedCode);
 
-/// Returns line numbers and content of the annotations in `AO.Code`.
+/// Returns line numbers and content of the annotations in `AnnotatedCode`.
 llvm::DenseMap
-getAnnotationLinesAndContent(AnalysisOutputs &AO);
-
-// FIXME: Return a string map instead of a vector of pairs.
-//
-/// Returns the analysis states at each annotated statement in `AO.Code`.
-template 
-llvm::Expected>>>
-getAnnotationStates(AnalysisOutputs &AO) {
-  using StateT = DataflowAnalysisState;
-  // FIXME: Extend to annotations on non-statement constructs.
-  // Get annotated statements.
-  llvm::Expected>
-  MaybeStmtToAnnotations =
-  buildStatementToAnnotationMapping(AO.Target, AO.Code);
-  if (!MaybeStmtToAnnotations)
-return MaybeStmtToAnnotations.takeError();
-  auto &StmtToAnnotations = *MaybeStmtToAnnotations;
-
-  // Compute a map from statement annotations to the state computed
-  // for the program point immediately after the annotated statement.
-  std::vector> Results;
-  for (const CFGBlock *Block : AO.CFCtx.getCFG()) {
-// Skip blocks that were not evaluated.
-if (!AO.BlockStates[Block->getBlockID()])
-  continue;
-
-transferBlock(
-AO.CFCtx, AO.BlockStates, *Block, AO.InitEnv, AO.Analysis,
-[&Results,
- &StmtToAnnotations](const clang::CFGElement &Element,
- const TypeErasedDataflowAnalysisState &State) {
-  auto Stmt = Element.getAs();
-  if (!Stmt)
-return;
-  auto It = StmtToAnnotations.find(Stmt->getStmt());
-  if (It == StmtToAnnotations.end())
-return;
-  auto *Lattice =
-  llvm::any_cast(&State.Lattice.Value);
-  Results.emplace_back(It->second, StateT{*Lattice, State.Env});
-});
-  }
-
-  return Results;

[PATCH] D132258: clang/apple: Infer simulator env from -mios-simulator-version-min flag

2022-08-22 Thread Nico Weber via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3affbae5300d: clang/apple: Infer simulator env from 
-mios-simulator-version-min= flag (authored by thakis).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132258

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-ld.c
  clang/test/Driver/darwin-version.c


Index: clang/test/Driver/darwin-version.c
===
--- clang/test/Driver/darwin-version.c
+++ clang/test/Driver/darwin-version.c
@@ -48,6 +48,10 @@
 // RUN: FileCheck --check-prefix=CHECK-VERSION-IOS10 %s
 // CHECK-VERSION-IOS10: x86_64-apple-ios11.0.0-simulator
 
+// RUN: %clang -target arm64-apple-darwin -mios-simulator-version-min=11.0 -c 
-### %s 2>&1 | \
+// RUN: FileCheck --check-prefix=CHECK-VERSION-IOS10-ARM64 %s
+// CHECK-VERSION-IOS10-ARM64: arm64-apple-ios11.0.0-simulator
+
 // RUN: %clang -target arm64-apple-ios11.1 -c -### %s 2>&1 | \
 // RUN: FileCheck --check-prefix=CHECK-VERSION-IOS11 %s
 // CHECK-VERSION-IOS11: arm64-apple-ios11.1.0
Index: clang/test/Driver/darwin-ld.c
===
--- clang/test/Driver/darwin-ld.c
+++ clang/test/Driver/darwin-ld.c
@@ -67,6 +67,10 @@
 // LINK_IOSSIM_3_0-NOT: -lbundle1.o
 // LINK_IOSSIM_3_0: -lSystem
 
+// RUN: %clang -target arm64-apple-darwin -fuse-ld= -mlinker-version=700 -### 
-arch arm64 -mios-simulator-version-min=15.0 %t.o 2>&1 | FileCheck 
-check-prefix=LINK_IOSSIM_ARM64 %s
+
+// LINK_IOSSIM_ARM64: "-platform_version" "ios-simulator" "15.0.0" "15.0.0"
+
 // RUN: %clang -target i386-apple-darwin9 -### -fpie %t.o 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_EXPLICIT_PIE %s < %t.log
 //
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1655,9 +1655,12 @@
 Result.setEnvironment(Environment, OSVersion, SDKInfo);
 return Result;
   }
-  static DarwinPlatform createOSVersionArg(DarwinPlatformKind Platform,
-   Arg *A) {
-return DarwinPlatform(OSVersionArg, Platform, A);
+  static DarwinPlatform createOSVersionArg(DarwinPlatformKind Platform, Arg *A,
+   bool IsSimulator) {
+DarwinPlatform Result{OSVersionArg, Platform, A};
+if (IsSimulator)
+  Result.Environment = DarwinEnvironmentKind::Simulator;
+return Result;
   }
   static DarwinPlatform createDeploymentTargetEnv(DarwinPlatformKind Platform,
   StringRef EnvVarName,
@@ -1752,23 +1755,33 @@
  : TvOSVersion ? TvOSVersion : WatchOSVersion)
  ->getAsString(Args);
 }
-return DarwinPlatform::createOSVersionArg(Darwin::MacOS, macOSVersion);
+return DarwinPlatform::createOSVersionArg(Darwin::MacOS, macOSVersion,
+  /*IsImulator=*/false);
   } else if (iOSVersion) {
 if (TvOSVersion || WatchOSVersion) {
   TheDriver.Diag(diag::err_drv_argument_not_allowed_with)
   << iOSVersion->getAsString(Args)
   << (TvOSVersion ? TvOSVersion : WatchOSVersion)->getAsString(Args);
 }
-return DarwinPlatform::createOSVersionArg(Darwin::IPhoneOS, iOSVersion);
+return DarwinPlatform::createOSVersionArg(
+Darwin::IPhoneOS, iOSVersion,
+iOSVersion->getOption().getID() ==
+options::OPT_mios_simulator_version_min_EQ);
   } else if (TvOSVersion) {
 if (WatchOSVersion) {
   TheDriver.Diag(diag::err_drv_argument_not_allowed_with)
   << TvOSVersion->getAsString(Args)
   << WatchOSVersion->getAsString(Args);
 }
-return DarwinPlatform::createOSVersionArg(Darwin::TvOS, TvOSVersion);
+return DarwinPlatform::createOSVersionArg(
+Darwin::TvOS, TvOSVersion,
+TvOSVersion->getOption().getID() ==
+options::OPT_mtvos_simulator_version_min_EQ);
   } else if (WatchOSVersion)
-return DarwinPlatform::createOSVersionArg(Darwin::WatchOS, WatchOSVersion);
+return DarwinPlatform::createOSVersionArg(
+Darwin::WatchOS, WatchOSVersion,
+WatchOSVersion->getOption().getID() ==
+options::OPT_mwatchos_simulator_version_min_EQ);
   return None;
 }
 
@@ -2228,6 +2241,7 @@
 
   DarwinEnvironmentKind Environment = OSTarget->getEnvironment();
   // Recognize iOS targets with an x86 architecture as the iOS simulator.
+  // FIXME: Remove this.
   if (Environment == NativeEnvironment && Platform != MacOS &&
   Platform != DriverKit && OSTarget->canInferSimulatorFromArch() &&
   getTriple().isX86())


Index: clang/test/Driver/darwin-version.c

[clang] 3affbae - clang/apple: Infer simulator env from -mios-simulator-version-min= flag

2022-08-22 Thread Nico Weber via cfe-commits

Author: Nico Weber
Date: 2022-08-22T12:19:34-04:00
New Revision: 3affbae5300dcdf753c954a65ab6a96fcf65c483

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

LOG: clang/apple: Infer simulator env from -mios-simulator-version-min= flag

Before this patch, open-source clang would consider
`-target x86_64-apple-darwin -mios-simulator-version-min=11.0` as
targeting the iOS simulator, due to the mios flag informing it
that we want to target iOS, and logic in the driver then realizing
that x86 iOS builds must be the simulator.

However, for `-target arm64-apple-darwin -mios-simulator-version-min=11.0`
that didn't work and clang thought that it's building for actual iOS,
and not for the simulator.

Due to this, building compiler-rt for arm64 iossim would lead to
all .o files in RTSanitizerCommonSymbolizer.iossim.dir being built
for iOS instead of for iOS simulator, and clang would ask ld64 to
link for iOS, but using the iPhoneSimulator sysroot. This would then
lead to many warnings from ld64 looking like:

ld: warning: building for iOS, but linking in .tbd file
(.../iPhoneSimulator.sdk/usr/lib/libc++abi.tbd) built for iOS Simulator

Worse, with ld64.lld, this diagnostic is currently an error instead
of a warning.

This patch makes it so that the presence of -mios-simulator-version-min=
now informs clang that we're building for simulator. That way, all the
.o files are built for simulator, the linker is informed that we're
building for simulator, and everything Just Works.

(Xcode's clang already behaves like this, so this makes open-source clang
match Xcode clang.)

We can now likely remove the hack to treat non-mac darwin x86 as
simulator, but doing that feels slightly risky, so I'm leaving that
for a follow-up patch.

(This patch is made necessary by the existence of arm64 macs.)

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Darwin.cpp
clang/test/Driver/darwin-ld.c
clang/test/Driver/darwin-version.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Darwin.cpp 
b/clang/lib/Driver/ToolChains/Darwin.cpp
index 7e3fc625d8c85..2a581a17bd099 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -1655,9 +1655,12 @@ struct DarwinPlatform {
 Result.setEnvironment(Environment, OSVersion, SDKInfo);
 return Result;
   }
-  static DarwinPlatform createOSVersionArg(DarwinPlatformKind Platform,
-   Arg *A) {
-return DarwinPlatform(OSVersionArg, Platform, A);
+  static DarwinPlatform createOSVersionArg(DarwinPlatformKind Platform, Arg *A,
+   bool IsSimulator) {
+DarwinPlatform Result{OSVersionArg, Platform, A};
+if (IsSimulator)
+  Result.Environment = DarwinEnvironmentKind::Simulator;
+return Result;
   }
   static DarwinPlatform createDeploymentTargetEnv(DarwinPlatformKind Platform,
   StringRef EnvVarName,
@@ -1752,23 +1755,33 @@ getDeploymentTargetFromOSVersionArg(DerivedArgList 
&Args,
  : TvOSVersion ? TvOSVersion : WatchOSVersion)
  ->getAsString(Args);
 }
-return DarwinPlatform::createOSVersionArg(Darwin::MacOS, macOSVersion);
+return DarwinPlatform::createOSVersionArg(Darwin::MacOS, macOSVersion,
+  /*IsImulator=*/false);
   } else if (iOSVersion) {
 if (TvOSVersion || WatchOSVersion) {
   TheDriver.Diag(diag::err_drv_argument_not_allowed_with)
   << iOSVersion->getAsString(Args)
   << (TvOSVersion ? TvOSVersion : WatchOSVersion)->getAsString(Args);
 }
-return DarwinPlatform::createOSVersionArg(Darwin::IPhoneOS, iOSVersion);
+return DarwinPlatform::createOSVersionArg(
+Darwin::IPhoneOS, iOSVersion,
+iOSVersion->getOption().getID() ==
+options::OPT_mios_simulator_version_min_EQ);
   } else if (TvOSVersion) {
 if (WatchOSVersion) {
   TheDriver.Diag(diag::err_drv_argument_not_allowed_with)
   << TvOSVersion->getAsString(Args)
   << WatchOSVersion->getAsString(Args);
 }
-return DarwinPlatform::createOSVersionArg(Darwin::TvOS, TvOSVersion);
+return DarwinPlatform::createOSVersionArg(
+Darwin::TvOS, TvOSVersion,
+TvOSVersion->getOption().getID() ==
+options::OPT_mtvos_simulator_version_min_EQ);
   } else if (WatchOSVersion)
-return DarwinPlatform::createOSVersionArg(Darwin::WatchOS, WatchOSVersion);
+return DarwinPlatform::createOSVersionArg(
+Darwin::WatchOS, WatchOSVersion,
+WatchOSVersion->getOption().getID() ==
+options::OPT_mwatchos_simulator_version_m

[PATCH] D132389: [pseudo] Apply the EOF fallback error-recovery strategy by default.

2022-08-22 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: All.
hokein requested review of this revision.
Herald added a subscriber: alextsao1999.
Herald added a project: clang-tools-extra.

Address 2 FIXMEs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D132389

Files:
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/lib/cxx/CXX.cpp
  clang-tools-extra/pseudo/lib/grammar/GrammarBNF.cpp
  clang-tools-extra/pseudo/unittests/GLRTest.cpp

Index: clang-tools-extra/pseudo/unittests/GLRTest.cpp
===
--- clang-tools-extra/pseudo/unittests/GLRTest.cpp
+++ clang-tools-extra/pseudo/unittests/GLRTest.cpp
@@ -654,27 +654,24 @@
 
 TEST_F(GLRTest, RecoveryFromStartOfInput) {
   build(R"bnf(
-_ := start [recover=Fallback] EOF
+_ := start EOF
 
 start := IDENTIFIER
   )bnf");
   TestLang.Table = LRTable::buildSLR(TestLang.G);
   bool fallback_recovered = false;
-  auto fallback = [&](Token::Index Start, const TokenStream & Code) {
+  auto fallback = [&](Token::Index Start, const TokenStream &Code) {
 fallback_recovered = true;
 return Code.tokens().size();
   };
-  TestLang.RecoveryStrategies.try_emplace(
-  extensionID("Fallback"),
-  fallback);
+  TestLang.RecoveryStrategies.try_emplace(extensionID("Eof"), fallback);
   clang::LangOptions LOptions;
   TokenStream Tokens = cook(lex("?", LOptions), LOptions);
 
   const ForestNode &Parsed =
   glrParse({Tokens, Arena, GSStack}, id("start"), TestLang);
   EXPECT_TRUE(fallback_recovered);
-  EXPECT_EQ(Parsed.dumpRecursive(TestLang.G),
-"[  0, end) start := \n");
+  EXPECT_EQ(Parsed.dumpRecursive(TestLang.G), "[  0, end) start := \n");
 }
 
 TEST_F(GLRTest, RepeatedRecovery) {
@@ -756,6 +753,10 @@
 "[  0, end) └─IDENTIFIER := tok[0]\n");
 
   Input = "notest";
+  TestLang.RecoveryStrategies.try_emplace(
+  extensionID("Eof"), [](Token::Index Begin, const TokenStream &Code) {
+return Code.tokens().size();
+  });
   const TokenStream &Failed = cook(lex(Input, LOptions), LOptions);
   EXPECT_EQ(glrParse({Failed, Arena, GSStack}, id("start"), TestLang)
 .dumpRecursive(TestLang.G),
Index: clang-tools-extra/pseudo/lib/grammar/GrammarBNF.cpp
===
--- clang-tools-extra/pseudo/lib/grammar/GrammarBNF.cpp
+++ clang-tools-extra/pseudo/lib/grammar/GrammarBNF.cpp
@@ -76,6 +76,8 @@
 });
 // Add an empty string for the corresponding sentinel unset attribute.
 T->AttributeValues.push_back("");
+// Add the fallback EOF error recovery strategy.
+UniqueAttributeValues.insert("Eof");
 UniqueAttributeValues.erase("");
 for (llvm::StringRef Name : UniqueAttributeValues) {
   T->AttributeValues.emplace_back();
@@ -269,6 +271,10 @@
 }
   }
 }
+if (Spec.Target == StartSymbol) {
+  R.Recovery = LookupExtensionID("Eof");
+  R.RecoveryIndex = 0;
+}
   }
 
   // Inlines all _opt symbols.
Index: clang-tools-extra/pseudo/lib/cxx/CXX.cpp
===
--- clang-tools-extra/pseudo/lib/cxx/CXX.cpp
+++ clang-tools-extra/pseudo/lib/cxx/CXX.cpp
@@ -389,9 +389,14 @@
   return Token::Invalid;
 }
 
+Token::Index recoverEof(Token::Index, const TokenStream &Tokens) {
+  return Tokens.tokens().size();
+}
+
 llvm::DenseMap buildRecoveryStrategies() {
   return {
   {Extension::Brackets, recoverBrackets},
+  {Extension::Eof, recoverEof},
   };
 }
 
Index: clang-tools-extra/pseudo/lib/GLR.cpp
===
--- clang-tools-extra/pseudo/lib/GLR.cpp
+++ clang-tools-extra/pseudo/lib/GLR.cpp
@@ -648,10 +648,7 @@
   Reduce(Heads, /*allow all reductions*/ tokenSymbol(tok::unknown));
 
   glrRecover(Heads, I, Params, Lang, NextHeads);
-  if (NextHeads.empty())
-// FIXME: Ensure the `_ := start-symbol` rules have a fallback
-// error-recovery strategy attached. Then this condition can't happen.
-return Params.Forest.createOpaque(StartSymbol, /*Token::Index=*/0);
+  assert(!NextHeads.empty() &&  "no fallback recovery strategy!" );
 } else
   ++I;
 
@@ -685,11 +682,9 @@
 }
 return Result;
   };
-  if (auto *Result = SearchForAccept(Heads))
-return *Result;
-  // We failed to parse the input, returning an opaque forest node for recovery.
-  // FIXME: as above, we can add fallback error handling so this is impossible.
-  return Params.Forest.createOpaque(StartSymbol, /*Token::Index=*/0);
+  auto *Result = SearchForAccept(Heads);
+  assert(Result);
+  return *Result;
 }
 
 void glrReduce(std::vector &Heads, SymbolID Lookahead,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

2022-08-22 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

In D126907#3739750 , @Mordante wrote:

> In D126907#3738383 , @erichkeane 
> wrote:
>
>> There was a test I dealt with previously where a ton of the header files 
>> were run with modules enabled (and an auto generated files), so I'm shocked 
>> to find there is MORE differences here.  @ldionne : This is another example 
>> where trying to test libcxx is particularly difficult to do apparently.
>
> I disagree; testing libcxx is easy. Unfortunately what's missing is good 
> documentation explaining how to test different configurations. A similar 
> libc++ related issue came up in another Clang review recently where the 
> libc++ CI setup was unclear. Afterwards I had a talk with @aaron.ballman and 
> I agreed to improve the documentation. While improving the documentation I 
> noticed some issues with our CI setup. So before publishing documentation 
> that doesn't reflect reality, I first wanted to fix these issues. While 
> testing my fixes I ran into the build failure due to this patch. So now we're 
> at a full circle.
>
> With the revert of this patch it's possible to fix the CI issues for libc++ 
> and I will continue with improving the documentation.
>
> Note that this specific issue isn't directly detected in the libc++ CI. This 
> would require a full clang build and testing with modules. That's not done, 
> full builds are only tested without modules. If wanted I supply a patch how 
> to test this configuration in the libc++ CI.
>
> If you need further assistance feel free to reach out to me or other libc++ 
> developers, the easiest way to reach us is #libcxx on Discord.

This is the 3rd time some non-obvious-how-to-run libcxx test failure has caused 
a revert request to this patch, which is obviously getting frustrating. 
Additionally, in none of the 3 cases was I alerted automatically.

I appreciate that there are actually ways to RUN these tests on my machine 
(Aaron is not as lucky, see above), but the lack of a "test all the libcxx 
things" flag is shocking.  It seems that I'd need at least 2 separate CMake 
directories to test these configurations?  That seems shocking to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126907

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


[PATCH] D132342: [X86][AVX512FP16] Relax limitation to AVX512FP16 intrinsics. NFCI

2022-08-22 Thread Simon Pilgrim via Phabricator via cfe-commits
RKSimon added inline comments.



Comment at: clang/lib/Headers/avx512fp16intrin.h:13
 
+#ifdef __SSE2__
+

pengfei wrote:
> RKSimon wrote:
> > Doesn't this have to be the general case like in other places in the 
> > headers?
> > ```
> > #if !(defined(_MSC_VER) || defined(__SCE__)) || __has_feature(modules) ||   
> >\
> > defined(__SSE2__)
> > ```
> I don't think so. `!(defined(_MSC_VER) || defined(__SCE__)) || 
> __has_feature(modules) || defined(__AVX512FP16__)` has already been checked 
> in immintrin.h
> 
> So if we are preprocessing here, the above condition must be true. Since 
> `AVX512FP16` implies `SSE2`, the new condition is constant ture.
got it - cheers


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132342

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


[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

2022-08-22 Thread YingChi Long via Phabricator via cfe-commits
inclyc added a comment.

If some one use `%hhd` with an unmatched type argument. Should we emit diagnose 
like

  format specifies type 'int' but the argument has type 'WhateverType'

instead of

  format specifies type 'char' but the argument has type 'WhateverType'

?

It's seems that there are many regression tests about this. Should we keep the 
legacy behaviour?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132266

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


[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

2022-08-22 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment.

In D116203#3739856 , @fdeazeve wrote:

> It seems like non-lldb bots are also failing, if it makes it easier to 
> reproduce:
>  
> https://green.lab.llvm.org/green/view/Clang/job/clang-stage1-RA/30837/consoleFull#-54352964249ba4694-19c4-4d7e-bec5-911270d8a58c

Thanks for flagging this. Based on the fact it only seems to be 
`remove_reference_t`, I'm wondering if this is an issue with D131732 
 (which was merged immediately after D116203 
).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116203

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


[PATCH] D131665: [CMake] Support passing arguments to build tool (bootstrap).

2022-08-22 Thread Petr Hosek via Phabricator via cfe-commits
phosek accepted this revision.
phosek 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/D131665/new/

https://reviews.llvm.org/D131665

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


[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

2022-08-22 Thread Felipe de Azevedo Piovezan via Phabricator via cfe-commits
fdeazeve added a comment.

In D116203#3740015 , @cjdb wrote:

> In D116203#3739856 , @fdeazeve 
> wrote:
>
>> It seems like non-lldb bots are also failing, if it makes it easier to 
>> reproduce:
>>  
>> https://green.lab.llvm.org/green/view/Clang/job/clang-stage1-RA/30837/consoleFull#-54352964249ba4694-19c4-4d7e-bec5-911270d8a58c
>
> Thanks for flagging this. Based on the fact it only seems to be 
> `remove_reference_t`, I'm wondering if this is an issue with D131732 
>  (which was merged immediately after 
> D116203 ).

I'm not sure, to expand on my previous message, I tested these two commits 
which appear one right after the other in the log:

- e9ef45635b77 
 (14 
hou..) cjdb@g.. [clang] adds unary type transformations as compiler built-ins
- d2d77e050b32 
 (15 
hou..) Ting.W.. [PowerPC][Coroutines] Add tail-call check with call information 
for coroutines

The tests pass on `d2d77e050b32` and fail on `e9ef45635b77`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116203

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


[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

2022-08-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D132266#3740013 , @inclyc wrote:

> If some one use `%hhd` with an unmatched type argument. Should we emit 
> diagnose like
>
>   format specifies type 'int' but the argument has type 'WhateverType'
>
> instead of
>
>   format specifies type 'char' but the argument has type 'WhateverType'
>
> ?
>
> It's seems that there are many regression tests about this. Should we keep 
> the legacy behaviour?

I think we want the diagnostic to identify the target type from the format 
specifier along with the actual and promoted type (if relevant) of the argument 
that mismatches. e.g.,

  float f = 1.0f;
  printf("%hd", f);

I would expect this to say something like `format specifies type 'short' but 
the argument has type 'double' (promoted from 'float')`.

This way users know 1) what the type of the source argument is without hunting 
for it, 2) what the target type of the format specifier is without hunting down 
what the specifiers mean, 3) what the actual mismatching type is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132266

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


[PATCH] D132379: [Support] Class for response file expansion

2022-08-22 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

Can you say what you're trying to do?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132379

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


[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-08-22 Thread Whitney Tsang via Phabricator via cfe-commits
Whitney added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4663
+void InferTimeTracePath(Compilation &C) {
+  bool TimeTrace = C.getArgs().getLastArg(options::OPT_ftime_trace) != nullptr;
+  bool TimeTraceFile =

Usually bool variable has name that starts with `Has` or something similar, 
e.g., `HasTimeTraceFile`.



Comment at: clang/lib/Driver/Driver.cpp:4702
+
+  // Add or replace -ftime-trace` to the correct one to all clang jobs
+  for (auto &J : C.getJobs()) {

Do you mean Add or replace the modified `-ftime-trace=` to all clang jobs?



Comment at: clang/lib/Driver/Driver.cpp:4739
+
+  // replace `-ftime-trace=`
+  auto &JArgs = J.getArguments();

should we also replace `-ftime-trace`?



Comment at: clang/test/Driver/check-time-trace.cpp:4
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o 
%T/exe/check-time-trace %s
+// RUN: cat %T/exe/check-time-trace*.json \
+// RUN:   | %python -c 'import json, sys; 
json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \

what may be between `check-time-trace` and `.json`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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


[PATCH] D132286: [clang][Interp] Implement function calls

2022-08-22 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:603
+  if (const auto *FuncDecl = dyn_cast_or_null(Callee)) {
+Function *Func = P.getFunction(FuncDecl);
+





Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:614
+
+if (Optional T = classify(E->getType())) {
+  // Put arguments on the stack.

Should this be using `CallExpr::getCallReturnType()` instead? (with test cases 
demonstrating the difference between `getType()` and this call)



Comment at: clang/lib/AST/Interp/Interp.cpp:409-411
 }
+
 bool Interpret(InterpState &S, APValue &Result) {

Spurious whitespace change?



Comment at: clang/lib/AST/Interp/Interp.cpp:60
+  S.Current =
+  new InterpFrame(S, const_cast(Func), S.Current, PC, {});
+

tbaeder wrote:
> This patch adds two `const_cast`s. They aren't strictly necessary, since the 
> `InterpFrame` doesn't need to modify the function. I have a local NFC patch 
> to add `const` to a few places to make this work.
Thanks! FWIW, because this is an entirely new interface for the compiler, I'm 
hoping we can force some better const correctness in this part of the code base.



Comment at: clang/lib/AST/Interp/InterpFrame.cpp:174
 
+  assert(Args);
   // Copy the initial value.

Can you add a message to this assert so folks know what broke when you got 
here. Also, what about functions with no arguments?



Comment at: clang/lib/AST/Interp/PrimType.h:85
   case Name: { using T = PrimConv::T; B; break; }
+
 #define TYPE_SWITCH(Expr, B)   
\

Spurious whitespace change?



Comment at: clang/test/AST/Interp/functions.cpp:1-2
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++14 -verify 
%s
+// RUN: %clang_cc1 -std=c++14 -verify=ref %s
+

I don't think we need to limit the testing to just c++14



Comment at: clang/test/AST/Interp/functions.cpp:8-9
+template constexpr T identity(T t) { return t; }
+static_assert(identity(true), "");
+static_assert(identity(true), "");
+static_assert(!identity(false), "");

Why testing twice?



Comment at: clang/test/AST/Interp/functions.cpp:48
+}
+static_assert(sub(10, 8, 2) == 0, "");

It's a bit fuzzy where to draw the line on test coverage as this example is 
partially about function calls and partially about reference tracking. But I 
think we need more complex tests than what we've got so far.
```
constexpr void func(int &ref) {
  ref = 12;
}
constexpr int do_it() {
  int mutate_me = 0;
  func(mutate_me);
  return mutate_me;
}
static_assert(do_it() == 12, "");

constexpr int &get_ref(int &i) {
  return i;
}
constexpr int do_it_again() {
  int i = 0;
  get_ref(i) = 12;
  return i;
}
static_assert(do_it_again() == 12, "");
```

(Also, while working on this interpreter, please keep in mind that C is also 
looking to add support for constant expressions; C2x will have `constexpr` 
object definitions and we expect to widen that functionality for C2y. But this 
means you should keep in mind the crazy things that C can do too, like:
```
int oh_god_this_hurts(struct { int a; } s) {
  return s.a;
}
```
which could someday get a `constexpr` slapped in front of it.)

Speaking of C, remember that we also have C extensions, like using VM types, 
that should have test coverage:
```
constexpr int func(int n, int array[static n], int m) {
  return array[m];
}

constexpr int do_it() {
  int array[] = { 0, 1, 2, 3, 4 };
  return func(5, array, 1);
}

static_assert(do_it() == 1, "");
```


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

https://reviews.llvm.org/D132286

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


[PATCH] D132266: [Clang][SemaChecking] move %hh and %h -Wformat warnings to -Wformat-pedantic

2022-08-22 Thread YingChi Long via Phabricator via cfe-commits
inclyc added a comment.

> format specifies type 'short' but the argument has type 'double' (promoted 
> from 'float')

I'm not sure about this. I'm curious about we just consider any integer with 
width less than or equal to `int` may be specified by `%hhd` ? (Because of 
default argument promotions). For example, we may expect `%hhd` could accept 
`int` `short` and `char`. And give diagnostics like

  format specifies integers with width less than or equal to 'int', but the 
argument has type 'double' (promoted from 'float')
  
  note: {info about why %hhd consumes argument like this}



> fwprintf shall behave as if it uses va_arg with a type argument naming the 
> type resulting from
> applying the default argument promotions to the type corresponding to the 
> conversion specification and
> then converting the result of the va_arg expansion to the type corresponding 
> to the conversion
> specification.



-

OpenCL seems to defined a series of length modifiers for vectors, Is it more 
reasonable to consider `hh` as fixed length (char) only? Because vectors are 
passed without argument promotions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132266

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


[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-08-22 Thread Roy Jacobson via Phabricator via cfe-commits
royjacobson added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677
 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L");
-Builder.defineMacro("__cpp_concepts", "201907L");
+Builder.defineMacro("__cpp_concepts", "202002L");
 Builder.defineMacro("__cpp_conditional_explicit", "201806L");

erichkeane wrote:
> aaron.ballman wrote:
> > cor3ntin wrote:
> > > royjacobson wrote:
> > > > aaron.ballman wrote:
> > > > > Does any of the not-yet-implemented bits (including from the DRs) 
> > > > > impact the ability to use conditionally trivial special member 
> > > > > functions? If so, we might want to be careful about aggressively 
> > > > > bumping this value. (It's more palatable for us to come back and bump 
> > > > > the value later than it is for us to claim we implement something 
> > > > > fully when we know we don't -- the goal of the feature test macros is 
> > > > > so that users don't have to resort to compiler version checks, which 
> > > > > is what users have to use when they fall into that "not fully 
> > > > > implemented" space.)
> > > > I don't think they're very significant, and the benefits of enabling it 
> > > > seem large enough for me - for example, std::expected works with 
> > > > libstdc++ and passes their unit tests but is gated by this macro.
> > > > 
> > > > We still have a non-trivial amount of concept bugs to go over, but I 
> > > > support enabling this.
> > > > 
> > > I think it's better to be conservative, It's the lesser of two not great 
> > > options.
> > > I'm hoping we can get to fix the issues in the clang 16 cycle , but in 
> > > the meantime we should not claim conformance if we are not, in fact, 
> > > conforming.
> > > We still have a non-trivial amount of concept bugs to go over, but I 
> > > support enabling this.
> > 
> > I think we should specify the updated macro value only after we think 
> > concepts have no known major issues with them. (Unknown issues happen and 
> > there's not much we can do about them, this is more that we shouldn't 
> > specify that we conform to a particular feature test macro when we 
> > knowingly still don't have it right yet.)
> Yeah, I don't think we can set this until we can at least compile a basic 
> libstdc++ ranges use, which the deferred instantiation still holds up.  That 
> would be my 'bare minimum' test for whether we can set that.
But we're already defining the `__cpp_concepts` macro, even with the current 
known bugs. The version bump, introduced by 
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2493r0.html is 
specifically about the conditionally trivial SMFs paper.

We can decide that we want the version bump to mean 'no more known concept bugs 
in clang' instead. But that would extend the meaning of the macro and would be 
confusing to users who want to rely on the documented, intended meaning of the 
version.

Also I think telling library writers 'this feature works now, but we didn't set 
its feature test macro' will make them use more compiler version checks, not 
less.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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


[PATCH] D132302: [clang] Add support for __attribute__((guard(nocf)))

2022-08-22 Thread Alvin Wong via Phabricator via cfe-commits
alvinhochun updated this revision to Diff 454559.
alvinhochun edited the summary of this revision.
alvinhochun added a comment.

Applied suggestions from Aaron. Thanks for the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132302

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/CodeGen/guard_nocf.c
  clang/test/CodeGenCXX/guard_nocf.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/attr-guard_nocf.c

Index: clang/test/Sema/attr-guard_nocf.c
===
--- clang/test/Sema/attr-guard_nocf.c
+++ clang/test/Sema/attr-guard_nocf.c
@@ -1,10 +1,20 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -fsyntax-only %s
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -verify -std=c++11 -fsyntax-only -x c++ %s
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -verify -std=c++11 -fsyntax-only -x c++ %s
+
+// The x86_64-w64-windows-gnu version tests mingw target, which relies on
+// __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
 // Function definition.
 __declspec(guard(nocf)) void testGuardNoCF(void) { // no warning
 }
 
+// Function definition using GNU-style attribute without relying on the
+// __declspec define for mingw.
+__attribute__((guard(nocf))) void testGNUStyleGuardNoCF(void) { // no warning
+}
+
 // Can not be used on variable, parameter, or function pointer declarations.
 int __declspec(guard(nocf)) i;  // expected-warning {{'guard' attribute only applies to functions}}
 void testGuardNoCFFuncParam(double __declspec(guard(nocf)) i) {}// expected-warning {{'guard' attribute only applies to functions}}
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
@@ -26,6 +26,7 @@
 // CHECK-NEXT: BuiltinAlias (SubjectMatchRule_function)
 // CHECK-NEXT: CFAuditedTransfer (SubjectMatchRule_function)
 // CHECK-NEXT: CFConsumed (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: CFGuard (SubjectMatchRule_function)
 // CHECK-NEXT: CFICanonicalJumpTable (SubjectMatchRule_function)
 // CHECK-NEXT: CFUnknownTransfer (SubjectMatchRule_function)
 // CHECK-NEXT: CPUDispatch (SubjectMatchRule_function)
Index: clang/test/CodeGenCXX/guard_nocf.cpp
===
--- clang/test/CodeGenCXX/guard_nocf.cpp
+++ clang/test/CodeGenCXX/guard_nocf.cpp
@@ -1,4 +1,8 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -std=c++11 -emit-llvm -O2 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -std=c++11 -emit-llvm -O2 -o - %s | FileCheck %s
+
+// The x86_64-w64-windows-gnu version tests mingw target, which relies on
+// __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
 void target_func();
 void (*func_ptr)() = &target_func;
@@ -10,6 +14,23 @@
 // CHECK-LABEL: nocf0
 // CHECK: call{{.*}}[[NOCF:#[0-9]+]]
 
+// Explicitly test using the GNU-style attribute without relying on the
+// __declspec define for mingw.
+// The "guard_nocf" attribute must be added.
+__attribute__((guard(nocf))) void nocf0_gnu_style() {
+  (*func_ptr)();
+}
+// CHECK-LABEL: nocf0_gnu_style
+// CHECK: call{{.*}}[[NOCF:#[0-9]+]]
+
+// Test using the c++-style attribute.
+// The "guard_nocf" attribute must be added.
+[[clang::guard(nocf)]] void nocf0_cxx_style() {
+  (*func_ptr)();
+}
+// CHECK-LABEL: nocf0_cxx_style
+// CHECK: call{{.*}}[[NOCF:#[0-9]+]]
+
 // The "guard_nocf" attribute must *not* be added.
 void cf0() {
   (*func_ptr)();
Index: clang/test/CodeGen/guard_nocf.c
===
--- clang/test/CodeGen/guard_nocf.c
+++ clang/test/CodeGen/guard_nocf.c
@@ -1,4 +1,8 @@
 // RUN: %clang_cc1 -triple %ms_abi_triple -fms-extensions -emit-llvm -O2 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-w64-windows-gnu -emit-llvm -O2 -o - %s | FileCheck %s
+
+// The x86_64-w64-windows-gnu version tests mingw target, which relies on
+// __declspec(...) being defined as __attribute__((...)) by compiler built-in.
 
 void target_func(void);
 void (*func_ptr)(void) = &target_func;
@@ -10,6 +14,15 @@
 // CHECK-LABEL: nocf0
 // CHECK: call{{.*}}[[NOCF:#[0-9]+]]
 
+// Explicitly test using the GNU-style attribute without relying on the
+// __declspec define for mingw.
+// The "guard_nocf" attribute must be added.
+__attribute__((guard(nocf))) void nocf0_gnu_style(void) {
+  (*func_ptr)();
+}
+// CHECK-LABEL: nocf0_gnu_style
+// CHECK: call{{.*}}[[NOCF:#[0-9]+]]
+
 // The "guard_nocf" attribute must *not* be added.
 void c

[PATCH] D113107: Support of expression granularity for _Float16.

2022-08-22 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam marked an inline comment as done.
zahiraam added inline comments.



Comment at: clang/test/CodeGen/X86/Float16-arithmetic.c:207
+// CHECK-NEXT:[[EXT:%.*]] = fpext half [[TMP0]] to float
+// CHECK-NEXT:store float [[EXT]], ptr [[RETVAL]], align 2
+// CHECK-NEXT:[[TMP1:%.*]] = load half, ptr [[RETVAL]], align 2

pengfei wrote:
> Not sure if we need a fptrunc and store the half value. The following tests 
> have the same problem.
I think that's what we want?
// CHECK-LABEL: @RealOp(
// CHECK-NEXT:  entry:
// CHECK-NEXT:[[A_ADDR:%.*]] = alloca half, align 2
// CHECK-NEXT:store half [[A:%.*]], ptr [[A_ADDR]], align 2
// CHECK-NEXT:[[TMP0:%.*]] = load half, ptr [[A_ADDR]], align 2
// CHECK-NEXT:[[EXT:%.*]] = fpext half [[TMP0]] to float
// CHECK-NEXT:[[UNPROMOTION:%.*]] = fptrunc float [[EXT]] to half
// CHECK-NEXT:ret half [[UNPROMOTION]]

Do you agree? If this is correct, I will make the change the other operators.


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

https://reviews.llvm.org/D113107

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


[PATCH] D132379: [Support] Class for response file expansion

2022-08-22 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added a comment.

In D132379#3740102 , @thakis wrote:

> Can you say what you're trying to do?

There is an intention to extend algorithm of included files search. In 
particular it requires passing down paths where config files may be searched 
for. It could solve the problem discussed in 
https://discourse.llvm.org/t/rfc-adding-a-default-file-location-to-config-file-support/63606,
 there are also other users that need such feature.

If config files are used to tune compiler for a platform which has variants, it 
is convenient to extract common platform settings into separate config file, 
which could be included by config files of particular variants. Now such 
solution does not work because included config files are searched differently 
than the file in --config option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132379

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


[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-08-22 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/Frontend/InitPreprocessor.cpp:677
 //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L");
-Builder.defineMacro("__cpp_concepts", "201907L");
+Builder.defineMacro("__cpp_concepts", "202002L");
 Builder.defineMacro("__cpp_conditional_explicit", "201806L");

royjacobson wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > cor3ntin wrote:
> > > > royjacobson wrote:
> > > > > aaron.ballman wrote:
> > > > > > Does any of the not-yet-implemented bits (including from the DRs) 
> > > > > > impact the ability to use conditionally trivial special member 
> > > > > > functions? If so, we might want to be careful about aggressively 
> > > > > > bumping this value. (It's more palatable for us to come back and 
> > > > > > bump the value later than it is for us to claim we implement 
> > > > > > something fully when we know we don't -- the goal of the feature 
> > > > > > test macros is so that users don't have to resort to compiler 
> > > > > > version checks, which is what users have to use when they fall into 
> > > > > > that "not fully implemented" space.)
> > > > > I don't think they're very significant, and the benefits of enabling 
> > > > > it seem large enough for me - for example, std::expected works with 
> > > > > libstdc++ and passes their unit tests but is gated by this macro.
> > > > > 
> > > > > We still have a non-trivial amount of concept bugs to go over, but I 
> > > > > support enabling this.
> > > > > 
> > > > I think it's better to be conservative, It's the lesser of two not 
> > > > great options.
> > > > I'm hoping we can get to fix the issues in the clang 16 cycle , but in 
> > > > the meantime we should not claim conformance if we are not, in fact, 
> > > > conforming.
> > > > We still have a non-trivial amount of concept bugs to go over, but I 
> > > > support enabling this.
> > > 
> > > I think we should specify the updated macro value only after we think 
> > > concepts have no known major issues with them. (Unknown issues happen and 
> > > there's not much we can do about them, this is more that we shouldn't 
> > > specify that we conform to a particular feature test macro when we 
> > > knowingly still don't have it right yet.)
> > Yeah, I don't think we can set this until we can at least compile a basic 
> > libstdc++ ranges use, which the deferred instantiation still holds up.  
> > That would be my 'bare minimum' test for whether we can set that.
> But we're already defining the `__cpp_concepts` macro, even with the current 
> known bugs. The version bump, introduced by 
> https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2493r0.html is 
> specifically about the conditionally trivial SMFs paper.
> 
> We can decide that we want the version bump to mean 'no more known concept 
> bugs in clang' instead. But that would extend the meaning of the macro and 
> would be confusing to users who want to rely on the documented, intended 
> meaning of the version.
> 
> Also I think telling library writers 'this feature works now, but we didn't 
> set its feature test macro' will make them use more compiler version checks, 
> not less.
The feature test macros aren't supposed to mean "I support the feature from the 
paper that updated it".  They are intended to mean "I support the feature from 
the paper that updated it AND everything before it".

I don't believe we need to be bug-free, but something as extreme as "we can't 
compile a large number of uses of concepts because we don't implement a central 
design consideration" (which, btw, was clarified in Core after the 2019 date 
IIRC) means we shouldn't be updating this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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


[PATCH] D132398: Allow static consteval member functions to be used in const expressions.

2022-08-22 Thread Utkarsh Saxena via Phabricator via cfe-commits
usaxena95 created this revision.
Herald added a project: All.
usaxena95 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/D132398

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -908,3 +908,15 @@
   return testDefaultArgForParam() + testDefaultArgForParam((E)1);
 }
 }
+
+namespace GH56246 {
+struct S {
+static consteval int hello() { return 1;}
+consteval int baz() const { return this->hello();}
+// Previously these two failed because of the use of `this` in the 
constant expressions.
+int foo() const { return this->hello(); }
+constexpr int bar() const { return this->hello();}
+} s;
+static_assert(s.bar() == 1);
+static_assert(s.baz() == 1);
+}
\ No newline at end of file
Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2356,15 +2356,19 @@
   template void f1(T t) {
 constexpr int k = t.size();
   }
-  template void f2(const T &t) { // expected-note 2{{declared 
here}}
-constexpr int k = t.size(); // expected-error 2{{constant}} expected-note 
2{{function parameter 't' with unknown value cannot be used in a constant 
expression}}
+  template void f2_for_non_static(const T &t) { // expected-note 
{{declared here}}
+constexpr int k = t.size(); // expected-error {{constant}} expected-note 
{{function parameter 't' with unknown value cannot be used in a constant 
expression}}
+  }
+  template void f2_for_static(const T &t) {
+constexpr int k = t.size();
+static_assert(k==3, "");
   }
   template void f3(const T &t) {
 constexpr int k = T::size();
   }
   void g(array<3> a) {
 f1(a);
-f2(a); // expected-note {{instantiation of}}
+f2_for_static(a);
 f3(a);
   }
 
@@ -2373,7 +2377,7 @@
   };
   void h(array_nonstatic<3> a) {
 f1(a);
-f2(a); // expected-note {{instantiation of}}
+f2_for_non_static(a); // expected-note {{instantiation of}}
   }
 }
 
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -43,7 +43,9 @@
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/CurrentSourceLocExprScope.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/OSLog.h"
 #include "clang/AST/OptionalDiagnostic.h"
 #include "clang/AST/RecordLayout.h"
@@ -8449,7 +8451,8 @@
   // Handle static member functions.
   if (const CXXMethodDecl *MD = dyn_cast(E->getMemberDecl())) {
 if (MD->isStatic()) {
-  VisitIgnoredBaseExpression(E->getBase());
+  if(Info.checkingPotentialConstantExpression())
+VisitIgnoredBaseExpression(E->getBase());
   return Success(MD);
 }
   }


Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -908,3 +908,15 @@
   return testDefaultArgForParam() + testDefaultArgForParam((E)1);
 }
 }
+
+namespace GH56246 {
+struct S {
+static consteval int hello() { return 1;}
+consteval int baz() const { return this->hello();}
+// Previously these two failed because of the use of `this` in the constant expressions.
+int foo() const { return this->hello(); }
+constexpr int bar() const { return this->hello();}
+} s;
+static_assert(s.bar() == 1);
+static_assert(s.baz() == 1);
+}
\ No newline at end of file
Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2356,15 +2356,19 @@
   template void f1(T t) {
 constexpr int k = t.size();
   }
-  template void f2(const T &t) { // expected-note 2{{declared here}}
-constexpr int k = t.size(); // expected-error 2{{constant}} expected-note 2{{function parameter 't' with unknown value cannot be used in a constant expression}}
+  template void f2_for_non_static(const T &t) { // expected-note {{declared here}}
+constexpr int k = t.size(); // expected-error {{constant}} expected-note {{function parameter 't' with unknown value cannot be used in a constant expression}}
+  }
+  template void f2_for_static(const T &t) {
+constexpr int k = t.size();
+static_assert(k==3, "");
   }
   template void f3(const T &t) {
 conste

  1   2   3   >