[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

[Clement Courbet via Phabricator via cfe-commits]

courbet added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:104
+  castExpr(hasCastKind(CK_LValueToRValue),
+   has(memberExpr(hasDeclaration(fieldDecl(isBitField(,
+  hasParent(

There needs to be some kind of check of the size of the bit field vs the size 
of the integer, because `struct SmallBitfield { unsigned int id : 31; };` 
should warn.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:105
+   has(memberExpr(hasDeclaration(fieldDecl(isBitField(,
+  hasParent(
+  binaryOperator(anyOf(hasOperatorName("<<"), 
hasOperatorName(">>");

What is specific about shifting operators ? What about `x+1` for example ? You 
might have opened a pandora box here: you'll need to tech the check about the 
semantics of all binary operations.

```
BinaryOperator  'int' '+'
  |-ImplicitCastExpr  'int' 
  | `-ImplicitCastExpr  'unsigned int' 
  |   `-MemberExpr  'unsigned int' lvalue bitfield .id 
0x55e144e7eaa0
  | `-DeclRefExpr  'SmallBitfield' lvalue Var 0x55e144e7eb18 'x' 
'SmallBitfield'
  `-IntegerLiteral  'int' 1
```



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:106
+  hasParent(
+  binaryOperator(anyOf(hasOperatorName("<<"), 
hasOperatorName(">>");
+

There also needs to be some check of the constant value of the RHS, because 
`x.id << 30` can actually overflow, so we want to warn in that case.


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

https://reviews.llvm.org/D114105

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

[PATCH] D114142: [clang-format] [PR52527] can join * with /* to form an outside of comment error C4138

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: HazardyKnusperkeks, curdeius.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.

https://bugs.llvm.org/show_bug.cgi?id=52527

The follow patch ensures there is always a space between * and /* to prevent 
transforming

  void foo(* /* comment */)(int bar);

info

  void foo(*/* comment */)(int bar);


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114142

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9439,6 +9439,8 @@
   verifyFormat("void f() { &(*I).first; }");
 
   verifyIndependentOfContext("f(b * /* confusing comment */ ++c);");
+  verifyFormat("f(* /* confusing comment */ foo);");
+  verifyFormat("void (* /*deleter*/)(const Slice &key, void *value)");
   verifyFormat(
   "int *MyValues = {\n"
   "*A, // Operator detection might be confused by the '{'\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3213,6 +3213,12 @@
   };
   if (Right.Tok.getIdentifierInfo() && Left.Tok.getIdentifierInfo())
 return true; // Never ever merge two identifiers.
+
+  // Leave a space between * and /* to avoid C4138 `comment end` found outside
+  // of comment.
+  if (Left.is(tok::star) && Right.is(TT_BlockComment))
+return true;
+
   if (Style.isCpp()) {
 if (Left.is(tok::kw_operator))
   return Right.is(tok::coloncolon);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9439,6 +9439,8 @@
   verifyFormat("void f() { &(*I).first; }");
 
   verifyIndependentOfContext("f(b * /* confusing comment */ ++c);");
+  verifyFormat("f(* /* confusing comment */ foo);");
+  verifyFormat("void (* /*deleter*/)(const Slice &key, void *value)");
   verifyFormat(
   "int *MyValues = {\n"
   "*A, // Operator detection might be confused by the '{'\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3213,6 +3213,12 @@
   };
   if (Right.Tok.getIdentifierInfo() && Left.Tok.getIdentifierInfo())
 return true; // Never ever merge two identifiers.
+
+  // Leave a space between * and /* to avoid C4138 `comment end` found outside
+  // of comment.
+  if (Left.is(tok::star) && Right.is(TT_BlockComment))
+return true;
+
   if (Style.isCpp()) {
 if (Left.is(tok::kw_operator))
   return Right.is(tok::coloncolon);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:87
+  // Reading a small unsigned bitfield member will be wrapped by an implicit
+  // cast to 'int' triggering this checker. But this is known to be safe by the
+  // compiler since it chose 'int' instead of 'unsigned int' as the type of the

(Terminology... 😕)



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:100
+  // `-IntegerLiteral 'int' 1
+  const auto ShiftingWidenedBitfieldValue = castExpr(
+  hasCastKind(CK_IntegralCast), hasType(asString("int")),

`IntWidened`? Just to point out that so far this is the only case we've found 
where the warning is superfluous.


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

https://reviews.llvm.org/D114105

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

[PATCH] D114143: [OpenMP][IRBuilder] Fix createSections

[Shraiysh via Phabricator via cfe-commits]

shraiysh created this revision.
Herald added subscribers: rogfer01, guansong, hiraditya, yaxunl.
shraiysh requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: llvm-commits, cfe-commits, sstefan1.
Herald added projects: clang, LLVM.

Fix for the case when there are no instructions in the entry basic block before 
the call
to `createSections`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114143

Files:
  clang/test/OpenMP/cancel_codegen.cpp
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -3487,6 +3487,37 @@
   EXPECT_TRUE(isSimpleBinaryReduction(Bitcast, FnReductionBB, &Opcode));
 }
 
+TEST_F(OpenMPIRBuilderTest, CreateSectionsSimple) {
+  using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
+  using BodyGenCallbackTy = llvm::OpenMPIRBuilder::StorableBodyGenCallbackTy;
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+  F->setName("func");
+  IRBuilder<> Builder(BB);
+  OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL});
+  llvm::SmallVector SectionCBVector;
+  llvm::SmallVector CaseBBs;
+
+  auto FiniCB = [&](InsertPointTy IP) {};
+  auto SectionCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
+   BasicBlock &FiniBB) {
+Builder.restoreIP(CodeGenIP);
+Builder.CreateBr(&FiniBB);
+  };
+  SectionCBVector.push_back(SectionCB);
+
+  auto PrivCB = [](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
+   llvm::Value &, llvm::Value &Val,
+   llvm::Value *&ReplVal) { return CodeGenIP; };
+  IRBuilder<>::InsertPoint AllocaIP(&F->getEntryBlock(),
+F->getEntryBlock().getFirstInsertionPt());
+  Builder.restoreIP(OMPBuilder.createSections(Loc, AllocaIP, SectionCBVector,
+  PrivCB, FiniCB, false, false));
+  Builder.CreateRetVoid(); // Required at the end of the function
+  EXPECT_NE(F->getEntryBlock().getTerminator(), nullptr);
+  EXPECT_FALSE(verifyModule(*M, &errs()));
+}
+
 TEST_F(OpenMPIRBuilderTest, CreateSections) {
   using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
   using BodyGenCallbackTy = llvm::OpenMPIRBuilder::StorableBodyGenCallbackTy;
@@ -3607,6 +3638,7 @@
 
   ASSERT_EQ(NumBodiesGenerated, 2U);
   ASSERT_EQ(NumFiniCBCalls, 1U);
+  EXPECT_FALSE(verifyModule(*M, &errs()));
 }
 
 TEST_F(OpenMPIRBuilderTest, CreateOffloadMaptypes) {
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -993,6 +993,8 @@
   Value *ST = ConstantInt::get(I32Ty, 1);
   llvm::CanonicalLoopInfo *LoopInfo = createCanonicalLoop(
   Loc, LoopBodyGenCB, LB, UB, ST, true, false, AllocaIP, "section_loop");
+  Builder.SetInsertPoint(AllocaIP.getBlock()->getTerminator());
+  AllocaIP = Builder.saveIP();
   InsertPointTy AfterIP =
   applyStaticWorkshareLoop(Loc.DL, LoopInfo, AllocaIP, true);
   BasicBlock *LoopAfterBB = AfterIP.getBlock();
Index: clang/test/OpenMP/cancel_codegen.cpp
===
--- clang/test/OpenMP/cancel_codegen.cpp
+++ clang/test/OpenMP/cancel_codegen.cpp
@@ -1324,14 +1324,6 @@
 // CHECK3-NEXT:[[RETVAL:%.*]] = alloca i32, align 4
 // CHECK3-NEXT:[[ARGC_ADDR:%.*]] = alloca i32, align 4
 // CHECK3-NEXT:[[ARGV_ADDR:%.*]] = alloca i8**, align 8
-// CHECK3-NEXT:[[P_LASTITER:%.*]] = alloca i32, align 4
-// CHECK3-NEXT:[[P_LOWERBOUND:%.*]] = alloca i32, align 4
-// CHECK3-NEXT:[[P_UPPERBOUND:%.*]] = alloca i32, align 4
-// CHECK3-NEXT:[[P_STRIDE:%.*]] = alloca i32, align 4
-// CHECK3-NEXT:[[P_LASTITER27:%.*]] = alloca i32, align 4
-// CHECK3-NEXT:[[P_LOWERBOUND28:%.*]] = alloca i32, align 4
-// CHECK3-NEXT:[[P_UPPERBOUND29:%.*]] = alloca i32, align 4
-// CHECK3-NEXT:[[P_STRIDE30:%.*]] = alloca i32, align 4
 // CHECK3-NEXT:[[DOTOMP_IV:%.*]] = alloca i32, align 4
 // CHECK3-NEXT:[[TMP:%.*]] = alloca i32, align 4
 // CHECK3-NEXT:[[DOTCAPTURE_EXPR_:%.*]] = alloca i32, align 4
@@ -1348,6 +1340,14 @@
 // CHECK3-NEXT:store i32 [[ARGC]], i32* [[ARGC_ADDR]], align 4
 // CHECK3-NEXT:store i8** [[ARGV]], i8*** [[ARGV_ADDR]], align 8
 // CHECK3-NEXT:[[OMP_GLOBAL_THREAD_NUM:%.*]] = call i32 @__kmpc_global_thread_num(%struct.ident_t* @[[GLOB1:[0-9]+]])
+// CHECK3-NEXT:[[P_LASTITER:%.*]] = alloca i32, align 4
+// CHECK3-NEXT:[[P_LOWERBOUND:%.*]] = alloca i32, align 4
+// CHECK3-NEXT:[[P_UPPERBOUND:%.*]] = alloca i32, align 4
+// CHECK3-NEXT:[[P_STRIDE:%.*]] = alloca i32, align 4
+// CHECK3-NEXT:[[P_LASTITER27:%.*]] = alloca

[clang] b105626 - Fix Windows build after commit 49682f1.

[Douglas Yung via cfe-commits]

Author: Douglas Yung
Date: 2021-11-18T00:23:22-08:00
New Revision: b10562612f2e67bda7813ae9586f0afd37dc3c29

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

LOG: Fix Windows build after commit 49682f1.

Added: 


Modified: 
clang/lib/Driver/ToolChains/SPIRV.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/SPIRV.cpp 
b/clang/lib/Driver/ToolChains/SPIRV.cpp
index 3f942478e2add..16e72d3c733f1 100644
--- a/clang/lib/Driver/ToolChains/SPIRV.cpp
+++ b/clang/lib/Driver/ToolChains/SPIRV.cpp
@@ -12,6 +12,7 @@
 #include "clang/Driver/InputInfo.h"
 #include "clang/Driver/Options.h"
 
+using namespace clang::driver;
 using namespace clang::driver::tools;
 using namespace llvm::opt;
 



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

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 388125.
ChuanqiXu added a comment.

Change includes:

- Add parent for newly created global module fragment.
- Add a test.
- Rename test directory. Suddenly I found that the names of the tests in CXX 
directory are corresponds to the standard one by one. So my name before is not 
good.

@aaron.ballman @urnathan It looks like @rsmith is busy now. Could you please 
help to review this one? Many Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110215

Files:
  clang/include/clang/Basic/Module.h
  clang/include/clang/Lex/ModuleMap.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CXX/module/module.unit/p7/Inputs/CPP.cppm
  clang/test/CXX/module/module.unit/p7/Inputs/h1.h
  clang/test/CXX/module/module.unit/p7/Inputs/h2.h
  clang/test/CXX/module/module.unit/p7/Inputs/h4.h
  clang/test/CXX/module/module.unit/p7/Inputs/h5.h
  clang/test/CXX/module/module.unit/p7/t1.cpp
  clang/test/CXX/module/module.unit/p7/t2.cpp
  clang/test/CXX/module/module.unit/p7/t3.cpp
  clang/test/CXX/module/module.unit/p7/t4.cpp
  clang/test/CXX/module/module.unit/p7/t5.cpp
  clang/test/CXX/module/module.unit/p7/t6.cpp
  clang/test/CodeGenCXX/Inputs/module-extern-C.h
  clang/test/CodeGenCXX/module-extern-C.cpp

Index: clang/test/CodeGenCXX/module-extern-C.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/module-extern-C.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++20 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+
+module;
+
+#include "Inputs/module-extern-C.h"
+
+export module x;
+
+// CHECK: define dso_local void @foo()
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+// CHECK: define dso_local void @bar()
+void bar() {
+  return;
+}
+// CHECK: define dso_local i32 @baz()
+int baz() {
+  return 3;
+}
+// CHECK: define dso_local double @double_func()
+double double_func() {
+  return 5.0;
+}
+}
Index: clang/test/CodeGenCXX/Inputs/module-extern-C.h
===
--- /dev/null
+++ clang/test/CodeGenCXX/Inputs/module-extern-C.h
@@ -0,0 +1,7 @@
+extern "C" void foo();
+extern "C" {
+void bar();
+int baz();
+double double_func();
+}
+extern "C++" class CPP;
Index: clang/test/CXX/module/module.unit/p7/t6.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t6.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -fr %t
+// RUN: mkdir %t
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %S/Inputs/CPP.cppm -I%S/Inputs -o %t/X.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %s -verify
+module;
+#include "Inputs/h2.h"
+export module use;
+import X;
+void printX(CPP *cpp) {
+  cpp->print(); // expected-error {{'CPP' must be defined before it is used}}
+// expected-note@Inputs/CPP.cppm:5 {{definition here is not reachable}}
+}
Index: clang/test/CXX/module/module.unit/p7/t5.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t5.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C++" int a = 5;
Index: clang/test/CXX/module/module.unit/p7/t4.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t4.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C" struct C {
+  int a;
+  int b;
+  double d;
+};
Index: clang/test/CXX/module/module.unit/p7/t3.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t3.cpp
@@ -0,0 +1,7 @@
+// This tests whether the global module would be created when the program don't declare it explicitly.
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+export module x;
+
+extern "C" void foo();
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.unit/p7/t2.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t2.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h2.h"
+
+export module x;
+
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.unit/p7/t1.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t1.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h1.h"
+
+export module x;
+
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+v

[clang-tools-extra] 7aa2ce0 - [NFC][clangd] fix clang-tidy finding on isa_and_nonnull

[Christian Kühnel via cfe-commits]

Author: Christian Kühnel
Date: 2021-11-18T09:16:54Z
New Revision: 7aa2ce0fab3c21cf87c5884f6bf8bdece1b6fe1c

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

LOG: [NFC][clangd] fix clang-tidy finding on isa_and_nonnull

This is a cleanup of the only llvm-prefer-isa-or-dyn-cast-in-conditionals 
finding in the clangd code base. This patch was created by automatically 
applying the fixes from clang-tidy.

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

Added: 


Modified: 
clang-tools-extra/clangd/Selection.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Selection.cpp 
b/clang-tools-extra/clangd/Selection.cpp
index e945f5d79b2c..0b10c7a3a6f9 100644
--- a/clang-tools-extra/clangd/Selection.cpp
+++ b/clang-tools-extra/clangd/Selection.cpp
@@ -500,7 +500,7 @@ class SelectionVisitor : public 
RecursiveASTVisitor {
   //  - those without source range information, we don't record those
   //  - those that can't be stored in DynTypedNode.
   bool TraverseDecl(Decl *X) {
-if (X && isa(X))
+if (llvm::isa_and_nonnull(X))
   return Base::TraverseDecl(X); // Already pushed by constructor.
 // Base::TraverseDecl will suppress children, but not this node itself.
 if (X && X->isImplicit())



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

[PATCH] D113899: [NFC][clangd] fix clang-tidy finding on isa_and_nonnull

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7aa2ce0fab3c: [NFC][clangd] fix clang-tidy finding on 
isa_and_nonnull (authored by kuhnel).

Changed prior to commit:
  https://reviews.llvm.org/D113899?vs=387254&id=388127#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113899

Files:
  clang-tools-extra/clangd/Selection.cpp


Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -500,7 +500,7 @@
   //  - those without source range information, we don't record those
   //  - those that can't be stored in DynTypedNode.
   bool TraverseDecl(Decl *X) {
-if (X && isa(X))
+if (llvm::isa_and_nonnull(X))
   return Base::TraverseDecl(X); // Already pushed by constructor.
 // Base::TraverseDecl will suppress children, but not this node itself.
 if (X && X->isImplicit())


Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -500,7 +500,7 @@
   //  - those without source range information, we don't record those
   //  - those that can't be stored in DynTypedNode.
   bool TraverseDecl(Decl *X) {
-if (X && isa(X))
+if (llvm::isa_and_nonnull(X))
   return Base::TraverseDecl(X); // Already pushed by constructor.
 // Base::TraverseDecl will suppress children, but not this node itself.
 if (X && X->isImplicit())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114116: [clang][ARM] relax -mtp=cp15 for ARMv6 non-thumb cases

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

Apologies had to go on a bit of a dive through the documentation. I've put some 
comments inline and some links to other documents that may help.




Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:150
 
+// The backend does not have support for hard thread pointers when targeting
+// Thumb1.

Will be worth saying `CP15 C13 ThreadID` somewhere to make it easier to look up 
in the documentation.



Comment at: clang/lib/Driver/ToolChains/Arch/ARM.cpp:155
+  llvm::ARM::ArchKind AK = llvm::ARM::parseArch(Triple.getArchName());
+  return Ver >= 7 || AK == llvm::ARM::ArchKind::ARMV6T2 ||
+ (Ver == 6 && Triple.isARM());

Are we restricting based on whether the threadid register is present or whether 
the instructions are available to access the cp15 registers?

If we're going by whether the CPU has the register present then it will be
* A and R profile (not M profile, even the ones that have Thumb2)
* V6K (includes ARM1176 but not ARM1156t2-s which has Thumb-2!) and V7+ (A and 
R profile)

If we're going by the instructions to write to CP15 then it is:
* Arm state (everything)
* Thumb2 (v7 + v6t2)

The above seems to be a blend of the two. Is it worth choosing one form or the 
other? GCC seems to use the latter. I guess using this option falls into the I 
know what I'm doing area that accessing named system registers comes into. If 
the kernel supports it the stricter version may help catch more mistakes though.

The v7 A/R Arm ARM https://developer.arm.com/documentation/ddi0403/ed
has in `D12.7.21 CP15 c13, Context ID support`
``` An ARMv6K implementation requires the Software Thread ID registers 
described in VMSA CP15 c13
register summary, Process, context and thread ID registers on page B3-1474. ```

The Arm 1156-s (the only v6t2 processor) TRM 
https://developer.arm.com/documentation/ddi0338/g/system-control-coprocessor/system-control-processor-registers/c13--process-id-register?lang=en
 which shows only one process ID register under opcode 1 accessed via:
```
MRC p15, 0, , c13, c0, 1 ;Read Process ID Register
```
Whereas the ThreadID register is opcode 3 on CPUs that are v6k and v7.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114116

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

[PATCH] D113250: [clang][driver] Add -fplugin-arg- to pass arguments to plugins

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 388129.
tbaeder added a comment.

I already had a test but forgot to add the file to my local git commit, sorry.


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

https://reviews.llvm.org/D113250

Files:
  clang/docs/ClangPlugins.rst
  clang/docs/ReleaseNotes.rst
  clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Frontend/plugin-driver-args.cpp

Index: clang/test/Frontend/plugin-driver-args.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-driver-args.cpp
@@ -0,0 +1,15 @@
+// Test passing args to plugins via the clang driver and -fplugin-arg
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -fplugin-arg-call_super_plugin-help -fsyntax-only %s 2>&1 | FileCheck %s
+// REQUIRES: plugins, examples
+
+// CHECK: Help text for CallSuper plugin goes here
+
+
+// Check that dashed-args get forwarded like this to the plugin.
+// Dashes cannot be part of the plugin name here
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -fplugin-arg-call_super_plugin-help-long -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix=CHECK-LONG
+// CHECK-LONG: A longer help text describing what the CallSuper plugin does goes here
+
+
+// RUN: %clang -fplugin=%llvmshlibdir/CallSuperAttr%pluginext -fplugin-arg-call_super_plugin-help-long -fsyntax-only %s 2>&1 -### | FileCheck %s --check-prefix=CHECK-CMD
+// CHECK-CMD: "-plugin-arg-call_super_plugin" "help-long"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6660,6 +6660,29 @@
 A->claim();
   }
 
+  // Turn -fplugin-arg-pluginname-key=value into
+  // -plugin-arg-pluginname key=value
+  // GCC has an actual plugin_argument struct with key/value pairs that it
+  // passes to its plugins, but we don't, so just pass it on as-is.
+  //
+  // The syntax for -fplugin-arg- is ambiguous if both plugin name and
+  // argument key are allowed to contain dashes. GCC therefore only
+  // allows dashes in the key. We do the same.
+  for (const Arg *A : Args.filtered(options::OPT_fplugin_arg)) {
+auto ArgValue = StringRef(A->getValue());
+auto FirstDashIndex = ArgValue.find('-');
+
+if (FirstDashIndex == StringRef::npos)
+  continue;
+
+auto Arg = ArgValue.substr(FirstDashIndex + 1);
+auto PluginName = ArgValue.substr(0, FirstDashIndex);
+
+CmdArgs.push_back(Args.MakeArgString(Twine("-plugin-arg-") + PluginName));
+CmdArgs.push_back(Args.MakeArgString(Arg));
+A->claim();
+  }
+
   // Forward -fpass-plugin=name.so to -cc1.
   for (const Arg *A : Args.filtered(options::OPT_fpass_plugin_EQ)) {
 CmdArgs.push_back(
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2504,6 +2504,9 @@
   NegFlag>;
 def fplugin_EQ : Joined<["-"], "fplugin=">, Group, Flags<[NoXarchOption]>, MetaVarName<"">,
   HelpText<"Load the named plugin (dynamic shared object)">;
+def fplugin_arg : Joined<["-"], "fplugin-arg-">,
+  MetaVarName<"-">,
+  HelpText<"Pass  to plugin ">;
 def fpass_plugin_EQ : Joined<["-"], "fpass-plugin=">,
   Group, Flags<[CC1Option]>, MetaVarName<"">,
   HelpText<"Load pass plugin from a dynamic shared object file (only with new pass manager).">,
Index: clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
===
--- clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
+++ clang/examples/CallSuperAttribute/CallSuperAttrInfo.cpp
@@ -145,6 +145,14 @@
 
   bool ParseArgs(const CompilerInstance &CI,
  const std::vector &args) override {
+if (!args.empty()) {
+  if (args[0] == "help") {
+llvm::errs() << "Help text for CallSuper plugin goes here\n";
+  } else if (args[0] == "help-long") {
+llvm::errs() << "A longer help text describing what the CallSuper "
+ << "plugin does goes here\n";
+  }
+}
 return true;
   }
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -62,7 +62,8 @@
 New Compiler Flags
 --
 
-- ...
+- Clang plugin arguments can now be passed through the compiler driver via
+  ``-fplugin-arg-pluginname-arg``, similar to GCC's ``-fplugin-arg``.
 
 Deprecated Compiler Flags
 -
Index: clang/docs/ClangPlugins.rst
===
--- clang/docs/ClangPlugins.rst
+++ clang/docs/ClangPlugins.rst
@@ -125,6 +125,27 @@
 ==
 
 
+Using the compiler driver
+-

[PATCH] D113250: [clang][driver] Add -fplugin-arg- to pass arguments to plugins

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6625
+auto ArgValue = StringRef(A->getValue());
+auto FirstDashIndex = ArgValue.find('-');
+auto Arg = ArgValue.substr(FirstDashIndex + 1);

MaskRay wrote:
> What if FirstDashIndex == std::string::npos?
Things still "work", but the plugin name then gets parsed incorrectly. I've 
decided to skip this case now.


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

https://reviews.llvm.org/D113250

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

[PATCH] D113251: [analyzer][doc] Add user documenation for taint analysis

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 marked 13 inline comments as done.
gamesh411 added inline comments.



Comment at: clang/docs/analyzer/checkers.rst:2351
+
+ clang --analyze ... -Xclang -analyzer-config -Xclang 
alpha.security.taint.TaintPropagation:Config=taint_config.yaml
+

steakhal wrote:
> Per https://reviews.llvm.org/D113004#inline-1078695 we should not advocate 
> users use the `-Xclang` machinery, we should rather refer to it by other 
> tools such as `scan-build`. However, we haven't reached a consensus about 
> this decision yet.
> Consider moving some parts of this doc to the proposed Configuration 
> documentation file - housing the //more// user-facing analyzer options.
Removed the command-line invocation part, and just left a mention to the 
configuration option.



Comment at: clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst:133
+   The value of ``None`` will not consider the arguments that are part of a 
variadic argument list (this option is redundant but can be used to temporarily 
switch off handling of a particular variadic argument option without removing 
the entire variadic entry).
+ - `VariadicIndex` is a number in the range of [0..int_max]. It indicates the 
starting index of the variadic argument in the signature of the function.
+

steakhal wrote:
> It's not exactly for this patch, but we should investigate If we could infer 
> this index from the declaration of the function.
Good idea, this seems like the way forward, I agree.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113251

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

[PATCH] D114120: [clang] Remove CLANG_ROUND_TRIP_CC1_ARGS and always roundtrip in +assert builds

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:615
 // May perform round-trip of command line arguments. By default, the round-trip
 // is enabled if CLANG_ROUND_TRIP_CC1_ARGS was defined during build. This can 
be
 // overwritten at run-time via the "-round-trip-args" and "-no-round-trip-args"

Can you also update this comment that menions `CLANG_ROUND_TRIP_CC1_ARGS`?


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

https://reviews.llvm.org/D114120

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

[PATCH] D112903: [C++20] [Module] Fix bug47116 and implement [module.interface]/p6

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

@aaron.ballman @urnathan gentle ping~


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

https://reviews.llvm.org/D112903

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

[PATCH] D113251: [analyzer][doc] Add user documenation for taint analysis

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 388132.
gamesh411 added a comment.

Fix the review comments of @steakhal


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113251

Files:
  clang/docs/analyzer/checkers.rst
  clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst

Index: clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst
===
--- clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst
+++ clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst
@@ -15,9 +15,9 @@
 Overview
 
 
-Taint analysis works by checking for the occurrence of special events during the symbolic execution of the program.
-Taint analysis defines sources, sinks, and propagation rules. It identifies errors by detecting a flow of information that originates in a taint source, touches a taint sink, and propagates through the program paths via propagation rules.
-A source, sink, or an event that propagates taint is mainly domain-specific knowledge, but there are some built-in defaults provided by :ref:`alpha-security-taint-TaintPropagation`.
+Taint analysis works by checking for the occurrence of special operations during the symbolic execution of the program.
+Taint analysis defines sources, sinks, and propagation rules. It identifies errors by detecting a flow of information that originates from a taint source, reaches a taint sink, and propagates through the program paths via propagation rules.
+A source, sink, or an operation that propagates taint is mainly domain-specific knowledge, but there are some built-in defaults provided by :ref:`alpha-security-taint-TaintPropagation`.
 The checker's documentation also specifies how to provide a custom taint configuration with command-line options.
 
 .. _taint-configuration-example:
@@ -39,6 +39,8 @@
 
   Propagations:
   # sources:
+  # The omission of SrcArgs key indicates unconditional taint propagation,
+  # which is conceptually what a source does.
 # signature:
 # size_t fread(void *ptr, size_t size, size_t nmemb, FILE * stream)
 #
@@ -46,11 +48,13 @@
 # FILE* f = fopen("file.txt");
 # char buf[1024];
 # size_t read = fread(buf, sizeof(buf[0]), sizeof(buf)/sizeof(buf[0]), f);
-# // read and buf is tainted
+# // both read and buf are tainted
 - Name: fread
   DstArgs: [0, -1]
 
   # propagations:
+  # The presence of SrcArgs key indicates conditional taint propagation,
+  # which is conceptually what a propagator does.
 # signature:
 # char *dirname(char *path)
 #
@@ -73,7 +77,7 @@
   Args: [0]
 
 In the example file above, the entries under the `Propagation` key implement the conceptual sources and propagations, and sinks have their dedicated `Sinks` key.
-The user can define program points where the tainted values should be cleansed by listing entries under the `Filters` key.
+The user can define operations (function calls) where the tainted values should be cleansed by listing entries under the `Filters` key.
 Filters model the sanitization of values done by the programmer, and providing these is key to avoiding false-positive findings.
 
 Configuration file syntax and semantics
@@ -86,50 +90,54 @@
  - Propagations
  - Sinks
 
-Under the `Filters` entry, the user can specify a list of events that remove taint (see :ref:`taint-filter-details` for details).
+Under the `Filters` key, the user can specify a list of operations that remove taint (see :ref:`taint-filter-details` for details).
 
-Under the `Propagations` entry, the user can specify a list of events that generate and propagate taint (see :ref:`taint-propagation-details` for details).
-The user can identify taint sources with a `SrcArgs` key in the `Propagation` entry, while propagations have none.
+Under the `Propagations` key, the user can specify a list of operations that introduce and propagate taint (see :ref:`taint-propagation-details` for details).
+The user can mark taint sources with a `SrcArgs` key in the `Propagation` key, while propagations have none.
+The lack of the `SrcArgs` key means unconditional propagation, which is how sources are modeled.
+The semantics of propagations are such, that if any of the source arguments are tainted (specified by indexes in `SrcArgs`) then all of the destination arguments (specified by indexes in `DstArgs`) also become tainted.
 
-Under the `Sinks` entry, the user can specify a list of events where the checker should emit a bug report if taint reaches there (see :ref:`taint-sink-details` for details).
+Under the `Sinks` key, the user can specify a list of operations where the checker should emit a bug report if tainted data reaches it (see :ref:`taint-sink-details` for details).
 
 .. _taint-filter-details:
 
 Filter syntax and semantics
 ###
+
 An entry under `Filters` is a `YAML `_ object with the follo

[PATCH] D114142: [clang-format] [PR52527] can join * with /* to form an outside of comment error C4138

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3219
+  // of comment.
+  if (Left.is(tok::star) && Right.is(TT_BlockComment))
+return true;

Isn't `tok::comment` better than `TT_BlockComment` if a space is also required 
between `*` and `//`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114142

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

[PATCH] D114142: [clang-format] [PR52527] can join * with /* to form an outside of comment error C4138

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3219
+  // of comment.
+  if (Left.is(tok::star) && Right.is(TT_BlockComment))
+return true;

owenpan wrote:
> Isn't `tok::comment` better than `TT_BlockComment` if a space is also 
> required between `*` and `//`?
I guess for formatting as we type, then perhaps that might be better, I just 
couldn't think of a use case for legal code where * would be the last value on 
the line, but perhaps

```
void foo(int *  // this is the first parameter
  ,int second);
```

Let me add that as a test and update the review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114142

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

[clang] 8a4fcfc - Remove non-affecting module maps from PCM files.

[Jan Svoboda via cfe-commits]

Author: Ilya Kuteev
Date: 2021-11-18T11:18:26+01:00
New Revision: 8a4fcfc242a0f12e346c3d6026f2ad8764b08f1e

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

LOG: Remove non-affecting module maps from PCM files.

Problem:
PCM file includes references to all module maps used in compilation which 
created PCM. This problem leads to PCM-rebuilds in distributed compilations as 
some module maps could be missing in isolated compilation. (For example in our 
distributed build system we create a temp folder for every compilation with 
only modules and headers that are needed for that particular command).

Solution:
Add only affecting module map files to a PCM-file.

Reviewed By: rsmith

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

Added: 
clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
clang/test/Modules/add-remove-irrelevant-mobile-map.m
clang/test/SemaCXX/Inputs/compare.modulemap

Modified: 
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Serialization/ASTWriter.cpp
clang/test/SemaCXX/compare-modules-cxx2a.cpp

Removed: 




diff  --git a/clang/include/clang/Serialization/ASTWriter.h 
b/clang/include/clang/Serialization/ASTWriter.h
index ac88cb0a31773..978f6d86ea5c8 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -456,6 +456,9 @@ class ASTWriter : public ASTDeserializationListener,
   std::vector>
   ModuleFileExtensionWriters;
 
+  /// User ModuleMaps skipped when writing control block.
+  std::set SkippedModuleMaps;
+
   /// Retrieve or create a submodule ID for this module.
   unsigned getSubmoduleID(Module *Mod);
 
@@ -475,7 +478,7 @@ class ASTWriter : public ASTDeserializationListener,
   createSignature(StringRef AllBytes, StringRef ASTBlockBytes);
 
   void WriteInputFiles(SourceManager &SourceMgr, HeaderSearchOptions &HSOpts,
-   bool Modules);
+   std::set &AffectingModuleMaps);
   void WriteSourceManagerBlock(SourceManager &SourceMgr,
const Preprocessor &PP);
   void WritePreprocessor(const Preprocessor &PP, bool IsModule);

diff  --git a/clang/lib/Serialization/ASTWriter.cpp 
b/clang/lib/Serialization/ASTWriter.cpp
index ec05ef724cc56..a1972f5c64963 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -161,6 +161,59 @@ static TypeCode getTypeCodeForTypeClass(Type::TypeClass 
id) {
 
 namespace {
 
+std::set GetAllModuleMaps(const HeaderSearch &HS,
+ Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  SmallVector ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, /*WantExternal*/ false);
+if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
+  continue;
+
+for (const auto &KH : HS.findAllModulesForHeader(File)) {
+  if (!KH.getModule())
+continue;
+  ModulesToProcess.push_back(KH.getModule());
+}
+  }
+
+  while (!ModulesToProcess.empty()) {
+auto *CurrentModule = ModulesToProcess.pop_back_val();
+ProcessedModules.insert(CurrentModule);
+
+auto *ModuleMapFile =
+HS.getModuleMap().getModuleMapFileForUniquing(CurrentModule);
+if (!ModuleMapFile) {
+  continue;
+}
+
+ModuleMaps.insert(ModuleMapFile);
+
+for (auto *ImportedModule : (CurrentModule)->Imports) {
+  if (!ImportedModule ||
+  ProcessedModules.find(ImportedModule) != ProcessedModules.end()) {
+continue;
+  }
+  ModulesToProcess.push_back(ImportedModule);
+}
+  }
+
+  return ModuleMaps;
+}
+
 class ASTTypeWriter {
   ASTWriter &Writer;
   ASTWriter::RecordData Record;
@@ -1424,9 +1477,15 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, 
ASTContext &Context,
 Stream.EmitRecordWithBlob(AbbrevCode, Record, origDir);
   }
 
+  std::set AffectingModuleMaps;
+  if (WritingModule) {
+AffectingModuleMaps =
+GetAllModuleMaps(PP.getHeaderSearchInfo(), WritingModule);
+  }
+
   WriteInputFiles(Context.SourceMgr,
   PP.getHeaderSearchInfo().getHeaderSearchOpts(),
-  PP.getLangOpts().Modules);
+  AffectingModuleMaps);
   Stream.ExitBlock();
 }
 
@@ -1444,9 +1503,9 @@ struct InputFil

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

[Jan Svoboda via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8a4fcfc242a0: Remove non-affecting module maps from PCM 
files. (authored by ilyakuteev, committed by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106876

Files:
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
  clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
  clang/test/Modules/add-remove-irrelevant-mobile-map.m
  clang/test/SemaCXX/Inputs/compare.modulemap
  clang/test/SemaCXX/compare-modules-cxx2a.cpp

Index: clang/test/SemaCXX/compare-modules-cxx2a.cpp
===
--- clang/test/SemaCXX/compare-modules-cxx2a.cpp
+++ clang/test/SemaCXX/compare-modules-cxx2a.cpp
@@ -1,15 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -I%S/Inputs %s -fno-modules-error-recovery
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
 
-#pragma clang module build compare
-module compare {
-  explicit module cmp {}
-  explicit module other {}
-}
-#pragma clang module contents
-#pragma clang module begin compare.cmp
-#include "std-compare.h"
-#pragma clang module end
-#pragma clang module endbuild
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -fmodules-cache-path=%t.mcp -I%S/Inputs %s -fno-modules-error-recovery -fmodule-map-file=%S/Inputs/compare.modulemap
 
 struct CC { CC(...); };
 
@@ -24,10 +16,10 @@
 
 // expected-note@std-compare.h:* 2+{{not reachable}}
 
-void b() { void(0 <=> 0); } // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+void b() { void(0 <=> 0); } // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 
 struct B {
-  CC operator<=>(const B&) const = default; // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+  CC operator<=>(const B&) const = default; // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 };
 auto vb = B() <=> B(); // expected-note {{required here}}
 
Index: clang/test/SemaCXX/Inputs/compare.modulemap
===
--- /dev/null
+++ clang/test/SemaCXX/Inputs/compare.modulemap
@@ -0,0 +1,6 @@
+module compare {
+  explicit module cmp {
+header "std-compare.h"
+  }
+  explicit module other {}
+}
Index: clang/test/Modules/add-remove-irrelevant-mobile-map.m
===
--- /dev/null
+++ clang/test/Modules/add-remove-irrelevant-mobile-map.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
+
+// Build without b.modulemap
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap %s -verify
+// RUN: cp %t.mcp/a.pcm %t/a.pcm
+
+// Build with b.modulemap
+// RUN: rm -rf %t.mcp
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap %s -verify
+// RUN: not diff %t.mcp/a.pcm %t/a.pcm
+
+// expected-no-diagnostics
+
+@import a;
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
@@ -0,0 +1 @@
+module b { }
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
@@ -0,0 +1 @@
+module a { }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -161,6 +161,59 @@
 
 namespace {
 
+std::set GetAllModuleMaps(const HeaderSearch &HS,
+ Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  SmallVector ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, /*WantE

[PATCH] D104550: [analyzer] Implement getType for SVal

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D104550#3139239 , @stevewan wrote:

> In D104550#2849582 , @vsavchenko 
> wrote:
>
>> In D104550#2849561 , 
>> @DavidSpickett wrote:
>>
>>> @vsavchenko One of the added tests is failing on our 32 bit Armv7 Thumb 
>>> bot: https://lab.llvm.org/buildbot/#/builders/170/builds/61
>>>
>>>   
>>> /home/tcwg-buildslave/worker/clang-thumbv7-full-2stage/llvm/clang/unittests/StaticAnalyzer/SValTest.cpp:169:
>>>  Failure
>>>   Expected equality of these values:
>>> Context.UnsignedLongTy
>>>   Which is: unsigned long
>>> A.getType(Context)
>>>   Which is: unsigned int
>>>   [  FAILED  ] SValTest.GetLocAsIntType (22 ms)
>>>   [--] 1 test from SValTest (22 ms total)
>>>
>>> A 32/64 bit issue?
>>
>> Hi @DavidSpickett , thanks for looking into this!
>> This patch was almost instantly followed by 
>> https://github.com/llvm/llvm-project/commit/b2842298cebf420ecb3750bf309021a7f37870c1
>>  which fixed the issue.  Please, let me know, if you still see it after that 
>> commit!
>
> Sorry for posting to this slightly aged thread. I'm seeing a similar error of 
> this on 32 bit AIX PPC, where `getUIntPtrType()` returns unsigned long, so 
> the aforementioned follow-on patch no longer applies. The results from 
> `getIntTypeForBitwidth()` seem unreliable in certain cases (e.g. int vs long 
> in ILP32), and I couldn't think of a good way around it. Have you had any 
> future plans on mitigating such problems?

Hi there, what is the exact target triple?
I wonder if it would be possible to create a new unit test case where we pass 
somehow the target triple to `runCheckerOnCode`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104550

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

[PATCH] D114149: [clang-tidy] Fix pr48613: "llvm-header-guard uses a reserved identifier"

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz created this revision.
salman-javed-nz added reviewers: whisperity, hokein, aaron.ballman.
salman-javed-nz added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, rnkovacs, xazax.hun.
salman-javed-nz requested review of this revision.

Fixes https://bugs.llvm.org/show_bug.cgi?id=48613.

llvm-header-guard is suggesting header guards with leading underscores
if the header file path begins with a '/' or similar special character.
Only reserved identifiers should begin with an underscore.

Another problem is that llvm-header-guard does the character
substitution for '/', '.', and '-' only.
There are other characters which are valid in a file path but invalid
in a macro name. Rely on the functions provided in CharInfo.h to
identify those characters.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114149

Files:
  clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
  clang-tools-extra/clang-tidy/utils/HeaderGuard.h
  clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -219,6 +219,25 @@
 "", "/llvm-project/clang-tools-extra/clangd/foo.h",
 StringRef("header is missing header guard")));
 
+  // Characters outside [A-z0-9] are replaced with "_".
+  EXPECT_EQ("#ifndef FOO_BAR_TE_T_A_B_C_H\n"
+"#define FOO_BAR_TE_T_A_B_C_H\n"
+"\n"
+"\n"
+"#endif\n",
+runHeaderGuardCheck("", "include/foo-bar/te$t/a(b)c.h",
+StringRef("header is missing header guard")));
+
+  // Subtitution of characters should not result in a header guard starting
+  // with "_".
+  EXPECT_EQ("#ifndef BAR_H\n"
+"#define BAR_H\n"
+"\n"
+"\n"
+"#endif\n",
+runHeaderGuardCheck("", "include/$bar.h",
+StringRef("header is missing header guard")));
+
 #ifdef WIN32
   // Check interaction with Windows-style path separators (\).
   EXPECT_EQ(
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.h
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.h
@@ -38,6 +38,9 @@
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
Preprocessor *ModuleExpanderPP) override;
 
+  /// Ensure that the provided header guard is a valid identifier.
+  static std::string sanitizeHeaderGuard(StringRef Guard);
+
   /// Returns ``true`` if the check should suggest inserting a trailing comment
   /// on the ``#endif`` of the header guard. It will use the same name as
   /// returned by ``HeaderGuardCheck::getHeaderGuard``.
Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
===
--- clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
+++ clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
@@ -24,6 +24,27 @@
   return std::string(Result.str());
 }
 
+/// Replace invalid characters in the identifier with '_'.
+static std::string replaceInvalidChars(StringRef Identifier) {
+  std::string Fixed{Identifier};
+  for (auto It = Fixed.begin(); It != Fixed.end(); ++It) {
+// Identifier name must start with [A-z].
+if (It == Fixed.begin() && isAsciiIdentifierStart(*It))
+  continue;
+// The subsequent characters can be [A-z0-9].
+if (It > Fixed.begin() && isAsciiIdentifierContinue(*It))
+  continue;
+*It = '_';
+  }
+  return Fixed;
+}
+
+/// Remove all '_' characters at the beginning of the identifier. Only reserved
+/// identifiers are allowed to start with these.
+static StringRef dropLeadingUnderscores(StringRef Identifier) {
+  return Identifier.drop_while([](char c) { return c == '_'; });
+}
+
 namespace {
 class HeaderGuardPPCallbacks : public PPCallbacks {
 public:
@@ -164,6 +185,7 @@
  StringRef CurHeaderGuard,
  std::vector &FixIts) {
 std::string CPPVar = Check->getHeaderGuard(FileName, CurHeaderGuard);
+CPPVar = HeaderGuardCheck::sanitizeHeaderGuard(CPPVar);
 std::string CPPVarUnder = CPPVar + '_';
 
 // Allow a trailing underscore iff we don't have to change the endif comment
@@ -216,6 +238,7 @@
 continue;
 
   std::string CPPVar = Check->getHeaderGuard(FileName);
+  CPPVar = HeaderGuardCheck::sanitizeHeaderGuard(CPPVar);
   std::string CPPVarUnder = CPPVar + '_'; // Allow a trailing underscore.
   // If there's a macro with a name that follows the header guard convention
   // but was not recognized by the preprocessor as a header guard there must
@@ -278,6 +301,11 @@
   P

[PATCH] D114142: [clang-format] [PR52527] can join * with /* to form an outside of comment error C4138

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3219
+  // of comment.
+  if (Left.is(tok::star) && Right.is(TT_BlockComment))
+return true;

MyDeveloperDay wrote:
> owenpan wrote:
> > Isn't `tok::comment` better than `TT_BlockComment` if a space is also 
> > required between `*` and `//`?
> I guess for formatting as we type, then perhaps that might be better, I just 
> couldn't think of a use case for legal code where * would be the last value 
> on the line, but perhaps
> 
> ```
> void foo(int *  // this is the first parameter
>   ,int second);
> ```
> 
> Let me add that as a test and update the review.
Maybe something like:
```
  double term = factor1 * // first factor
factor2;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114142

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

[clang] c772a9b - [clang][modules] NFC: Fix typo in test name

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-11-18T11:57:34+01:00
New Revision: c772a9b6eb1479e042105e4334f0b395fadb1462

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

LOG: [clang][modules] NFC: Fix typo in test name

Added: 
clang/test/Modules/add-remove-irrelevant-module-map.m

Modified: 


Removed: 
clang/test/Modules/add-remove-irrelevant-mobile-map.m



diff  --git a/clang/test/Modules/add-remove-irrelevant-mobile-map.m 
b/clang/test/Modules/add-remove-irrelevant-module-map.m
similarity index 100%
rename from clang/test/Modules/add-remove-irrelevant-mobile-map.m
rename to clang/test/Modules/add-remove-irrelevant-module-map.m



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

[clang] 8e47b83 - [AArch64][ARM] Enablement of Cortex-A710 Support

[Mubashar Ahmad via cfe-commits]

Author: Mubashar Ahmad
Date: 2021-11-18T10:58:05Z
New Revision: 8e47b83ec93de6f92d077240a3665f3a03076957

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

LOG: [AArch64][ARM] Enablement of Cortex-A710 Support

Phabricator review: https://reviews.llvm.org/D113256

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/test/Driver/aarch64-cpus.c
clang/test/Driver/arm-cortex-cpus.c
clang/test/Misc/target-invalid-cpu-note.c
llvm/include/llvm/Support/AArch64TargetParser.def
llvm/include/llvm/Support/ARMTargetParser.def
llvm/lib/Target/AArch64/AArch64.td
llvm/lib/Target/AArch64/AArch64Subtarget.cpp
llvm/lib/Target/AArch64/AArch64Subtarget.h
llvm/lib/Target/ARM/ARM.td
llvm/lib/Target/ARM/ARMSubtarget.cpp
llvm/lib/Target/ARM/ARMSubtarget.h
llvm/unittests/Support/TargetParserTest.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 204ccdfa58e51..104d2e908d809 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -194,6 +194,7 @@ Arm and AArch64 Support in Clang
 - Support has been added for the following processors (command-line 
identifiers in parentheses):
   - Arm Cortex-A510 (``cortex-a510``)
   - Arm Cortex-X2 (``cortex-x2``)
+  - Arm Cortex-A710 (``cortex-A710``)
 
 - The -mtune flag is no longer ignored for AArch64. It is now possible to
   tune code generation for a particular CPU with -mtune without setting any

diff  --git a/clang/test/Driver/aarch64-cpus.c 
b/clang/test/Driver/aarch64-cpus.c
index 1c64e34608377..89cfb7e99a57e 100644
--- a/clang/test/Driver/aarch64-cpus.c
+++ b/clang/test/Driver/aarch64-cpus.c
@@ -413,6 +413,15 @@
 // RUN: %clang -target aarch64 -mcpu=cortex-x2+crypto -### -c %s 2>&1 | 
FileCheck -check-prefix=CORTEX-X2-CRYPTO %s
 // CORTEX-X2-CRYPTO: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" 
"+sm4" "-target-feature" "+sha3" "-target-feature" "+sha2" "-target-feature" 
"+aes"
 
+// RUN: %clang -target aarch64 -mcpu=cortex-a710 -### -c %s 2>&1 | FileCheck 
-check-prefix=CORTEX-A710 %s
+// CORTEX-A710: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" 
"cortex-a710"
+// CORTEX-A710-NOT: "-target-feature" "{{[+-]}}sm4"
+// CORTEX-A710-NOT: "-target-feature" "{{[+-]}}sha3"
+// CORTEX-A710-NOT: "-target-feature" "{{[+-]}}aes"
+// CORTEX-A710-SAME: {{$}}
+// RUN: %clang -target aarch64 -mcpu=cortex-a710+crypto -### -c %s 2>&1 | 
FileCheck -check-prefix=CORTEX-A710-CRYPTO %s
+// CORTEX-A710-CRYPTO: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" 
"-target-feature" "+sm4" "-target-feature" "+sha3" "-target-feature" "+sha2" 
"-target-feature" "+aes"
+
 // RUN: %clang -target aarch64_be -mcpu=cortex-a57 -### -c %s 2>&1 | FileCheck 
-check-prefix=CA57-BE %s
 // RUN: %clang -target aarch64 -mbig-endian -mcpu=cortex-a57 -### -c %s 2>&1 | 
FileCheck -check-prefix=CA57-BE %s
 // RUN: %clang -target aarch64_be -mbig-endian -mcpu=cortex-a57 -### -c %s 
2>&1 | FileCheck -check-prefix=CA57-BE %s

diff  --git a/clang/test/Driver/arm-cortex-cpus.c 
b/clang/test/Driver/arm-cortex-cpus.c
index 68d3ca84f8ee5..8ca7bcd518c95 100644
--- a/clang/test/Driver/arm-cortex-cpus.c
+++ b/clang/test/Driver/arm-cortex-cpus.c
@@ -949,6 +949,17 @@
 // CHECK-CORTEX-A78C-MFPU: "-target-feature" "+sha2"
 // CHECK-CORTEX-A78C-MFPU: "-target-feature" "+aes"
 
+// RUN: %clang -target armv8a-arm-none-eabi -mcpu=cortex-a710 -### -c %s 2>&1 
| FileCheck -check-prefix=CHECK-CORTEX-A710 %s
+// RUN: %clang -target armv8a-arm-none-eabi -mcpu=cortex-a710 
-mfpu=crypto-neon-fp-armv8 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CORTEX-A710-MFPU %s
+// CHECK-CORTEX-A710: "-cc1"{{.*}} "-triple" "armv9a-{{.*}} "-target-cpu" 
"cortex-a710"
+// CHECK-CORTEX-A710-NOT: "-target-feature" "{{[+-]}}sm4"
+// CHECK-CORTEX-A710-NOT: "-target-feature" "{{[+-]}}sha3"
+// CHECK-CORTEX-A710: "-target-feature" "-aes"
+// CHECK-CORTEX-A710-SAME: {{$}}
+// CHECK-CORTEX-A710-MFPU: "-cc1"{{.*}} "-target-feature" "+fp-armv8"
+// CHECK-CORTEX-A710-MFPU: "-target-feature" "+sha2"
+// CHECK-CORTEX-A710-MFPU: "-target-feature" "+aes"
+
 // RUN: %clang -target arm -mcpu=cortex-m23 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CPUV8MBASE %s
 // CHECK-CPUV8MBASE:  "-cc1"{{.*}} "-triple" "thumbv8m.base-
 

diff  --git a/clang/test/Misc/target-invalid-cpu-note.c 
b/clang/test/Misc/target-invalid-cpu-note.c
index 62aabab678172..1b436d8d904f2 100644
--- a/clang/test/Misc/target-invalid-cpu-note.c
+++ b/clang/test/Misc/target-invalid-cpu-note.c
@@ -1,15 +1,15 @@
 // Use CHECK-NEXT instead of multiple CHECK-SAME to ensure we will fail if 
there is anything extra in the output.
 // RUN: not %clang_cc1 -triple armv5--- -target-cpu not-a-cpu -fsyntax-only %s 
2>&1 | FileCheck %s --check-prefix ARM
 // ARM: error: unknown

[PATCH] D113256: [AArch64][ARM] Enablement of Cortex-A710 Support

[Mubashar Ahmad via Phabricator via cfe-commits]

mubashar_ closed this revision.
mubashar_ added a comment.

Landed as: https://reviews.llvm.org/rG8e47b83ec93de6f92d077240a3665f3a03076957


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

https://reviews.llvm.org/D113256

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

[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

[Henry Linjamäki via Phabricator via cfe-commits]

linjamaki updated this revision to Diff 388146.
linjamaki added a comment.

- Adjust `--offload` description: reflect the current state.
- Adjust enum range check in IsAMDGpuArch().
- Make `--offload` and `--offload-arch` options mutually exclusive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

Files:
  clang/include/clang/Basic/Cuda.h
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/Cuda.cpp
  clang/lib/Basic/Targets/NVPTX.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/Inputs/hipspv-dev-lib/a/a.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/b/b.bc
  clang/test/Driver/Inputs/hipspv-dev-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/bin/.hipVersion
  clang/test/Driver/Inputs/hipspv/lib/hip-device-lib/hipspv-spirv64.bc
  clang/test/Driver/Inputs/hipspv/lib/libLLVMHipSpvPasses.so
  clang/test/Driver/Inputs/pass-plugin.so
  clang/test/Driver/hipspv-device-libs.hip
  clang/test/Driver/hipspv-pass-plugin.hip
  clang/test/Driver/hipspv-toolchain-rdc.hip
  clang/test/Driver/hipspv-toolchain.hip
  clang/test/Driver/invalid-offload-options.cpp

Index: clang/test/Driver/invalid-offload-options.cpp
===
--- /dev/null
+++ clang/test/Driver/invalid-offload-options.cpp
@@ -0,0 +1,24 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload= \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=INVALID-TARGET %s
+
+// INVALID-TARGET: error: Invalid or unsupported offload target: '{{.*}}'
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=foo,bar \
+// RUN:   --hip-path=%S/Inputs/hipspv -nogpuinc -nogpulib %s \
+// RUN: 2>&1 | FileCheck --check-prefix=TOO-MANY-TARGETS %s
+
+// TOO-MANY-TARGETS: error: Only one offload target is supported in HIP.
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --offload=amdgcn-amd-amdhsa --offload-arch=gfx900 %s \
+// RUN: 2>&1 | FileCheck --check-prefix=OFFLOAD-ARCH-MIX %s
+
+// OFFLOAD-ARCH-MIX: error: option '--offload-arch' cannot be specified with '--offload'
Index: clang/test/Driver/hipspv-toolchain.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain.hip
@@ -0,0 +1,37 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   --hip-path=%S/Inputs/hipspv -nohipwrapperinc %s \
+// RUN: 2>&1 | FileCheck %s
+
+// CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "spirv64"
+// CHECK-SAME: "-aux-triple" "{{.*}}" "-emit-llvm-bc"
+// CHECK-SAME: "-fcuda-is-device"
+// CHECK-SAME: "-fcuda-allow-variadic-functions"
+// CHECK-SAME: "-mlink-builtin-bitcode" {{".*/hipspv/lib/hip-device-lib/hipspv-spirv64.bc"}}
+// CHECK-SAME: "-isystem" {{".*/hipspv/include"}}
+// CHECK-SAME: "-fhip-new-launch-api"
+// CHECK-SAME: "-o" [[DEV_BC:".*bc"]]
+// CHECK-SAME: "-x" "hip"
+
+// CHECK: {{".*llvm-link"}} [[DEV_BC]] "-o" [[LINK_BC:".*bc"]]
+
+// CHECK: {{".*opt"}} [[LINK_BC]] "-load-pass-plugin"
+// CHECK-SAME: {{".*/hipspv/lib/libLLVMHipSpvPasses.so"}}
+// CHECK-SAME: "-passes=hip-post-link-passes" "-o" [[LOWER_BC:".*bc"]]
+
+// CHECK: {{".*llvm-spirv"}} "--spirv-max-version=1.1" "--spirv-ext=+all"
+// CHECK-SAME: [[LOWER_BC]] "-o" "[[SPIRV_OUT:.*out]]"
+
+// CHECK: {{".*clang-offload-bundler"}} "-type=o" "-bundle-align=4096"
+// CHECK-SAME: "-targets=host-x86_64-unknown-linux,hip-spirv64generic"
+// CHECK-SAME: "-inputs={{.*}},[[SPIRV_OUT]]" "-outputs=[[BUNDLE:.*hipfb]]"
+
+// CHECK: [[CLANG]] "-cc1" "-triple" {{".*"}} "-aux-triple" "spirv64"
+// CHECK-SAME: "-emit-obj"
+// CHECK-SAME: "-fcuda-include-gpubinary" "[[BUNDLE]]"
+// CHECK-SAME: "-o" [[OBJ_HOST:".*o"]] "-x" "hip"
+
+// CHECK: {{".*ld.*"}} {{.*}}[[OBJ_HOST]]
Index: clang/test/Driver/hipspv-toolchain-rdc.hip
===
--- /dev/null
+++ clang/test/Driver/hipspv-toolchain-rdc.hip
@@ -0,0 +1,63 @@
+// REQUIRES: clang-driver
+// REQUIRES: x86-registered-target
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -### -x hip -target x86_64-linux-gnu --offload=spirv64 \
+// RUN:   -fgpu-rdc --hip-path=%S/Inputs/hipspv -nohipwrapperinc \
+// RUN:   %S/Inputs/hip_multiple_inputs/a.cu \
+// RUN:   %S/Inputs/hip_multiple_inputs/b.hip \
+// RUN: 2>&1 | FileCheck %s
+
+// Emit o

[PATCH] D114151: [clang-format] [C++20] [Module] clang-format couldn't recognize partitions

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: HazardyKnusperkeks, curdeius, owenpan, 
ChuanqiXu.
MyDeveloperDay added projects: clang, clang-format.
Herald added subscribers: jrtc27, arichardson.
MyDeveloperDay requested review of this revision.

https://bugs.llvm.org/show_bug.cgi?id=52517

clang-format is butchering modules, this could easily become a barrier to entry 
for modules given clang-formats wide spread use.

Prevent the following from adding spaces around the  `:`  (cf was considering 
the ':' as an InheritanceColon)

  import ;
  import foo:bar;
  import :bar;
  export module foo:bar;
  export import foo:bar;
  export import :bar;
  module foo:bar;

to

  import;
  import foo : bar;
  import : bar;
  export module foo : bar;
  export import foo : bar;
  export import : bar;
  module foo : bar;


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114151

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22397,6 +22397,28 @@
   EXPECT_EQ(Code, format(Code, Style));
 }
 
+TEST_F(FormatTest, Cpp20ModulesSupport) {
+  FormatStyle Style = getLLVMStyle();
+
+  verifyFormat("export import foo:bar;", Style);
+  verifyFormat("export import :bar;", Style);
+  verifyFormat("export module foo:bar;", Style);
+  verifyFormat("export module foo;", Style);
+
+  verifyFormat("import foo:bar;", Style);
+  verifyFormat("import :bar;", Style);
+  verifyFormat("import bar;", Style);
+  verifyFormat("import ;", Style);
+
+  verifyFormat("module foo;", Style);
+  verifyFormat("module foo:bar;", Style);
+
+  verifyFormat("export namespace hi {\n"
+   "const char *sayhi();\n"
+   "}",
+   Style);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1250,6 +1250,21 @@
 addUnwrappedLine();
 return;
   }
+  if (Style.isCpp()) {
+nextToken();
+while (FormatTok) {
+  if (FormatTok->is(tok::colon)) {
+FormatTok->setType(TT_ModulePartitionColon);
+  }
+  if (FormatTok->is(tok::semi)) {
+nextToken();
+break;
+  }
+  nextToken();
+}
+addUnwrappedLine();
+return;
+  }
 }
 if (Style.isCpp() &&
 FormatTok->isOneOf(Keywords.kw_signals, Keywords.kw_qsignals,
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -902,9 +902,13 @@
   break;
 }
   }
-  if (Contexts.back().ColonIsDictLiteral ||
-  Style.Language == FormatStyle::LK_Proto ||
-  Style.Language == FormatStyle::LK_TextProto) {
+  if (Line.First->isOneOf(Keywords.kw_module, Keywords.kw_import) ||
+  Line.First->startsSequence(tok::kw_export, Keywords.kw_module) ||
+  Line.First->startsSequence(tok::kw_export, Keywords.kw_import)) {
+Tok->setType(TT_ModulePartitionColon);
+  } else if (Contexts.back().ColonIsDictLiteral ||
+ Style.Language == FormatStyle::LK_Proto ||
+ Style.Language == FormatStyle::LK_TextProto) {
 Tok->setType(TT_DictLiteral);
 if (Style.Language == FormatStyle::LK_TextProto) {
   if (FormatToken *Previous = Tok->getPreviousNonComment())
@@ -3214,6 +3218,17 @@
   if (Right.Tok.getIdentifierInfo() && Left.Tok.getIdentifierInfo())
 return true; // Never ever merge two identifiers.
   if (Style.isCpp()) {
+// Space between import .
+if (Left.is(Keywords.kw_import) && Right.is(tok::less))
+  return true;
+// No space between import foo:bar but keep a space between import :bar;
+if (Left.is(tok::identifier) && !Left.is(Keywords.kw_import) &&
+Right.is(TT_ModulePartitionColon))
+  return false;
+// No space between :bar;
+if (Left.is(TT_ModulePartitionColon) && Right.is(tok::identifier))
+  return false;
+
 if (Left.is(tok::kw_operator))
   return Right.is(tok::coloncolon);
 if (Right.is(tok::l_brace) && Right.is(BK_BracedInit) &&
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -76,6 +76,7 @@
   TYPE(LineComment)\
   TYPE(MacroBlockBegin)  

[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

[Henry Linjamäki via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/Cuda.h:109
+  // Generic processor model is for testing only.
+  return A >= CudaArch::GFX600 && A <= CudaArch::GFX1035;
 }

yaxunl wrote:
> can we use A < CudaArch::Generic instead? to avoid updating this line each 
> time we add a new gfx arch.
Changed as suggested.



Comment at: clang/include/clang/Driver/Options.td:1136
+def offload_EQ : CommaJoined<["--"], "offload=">, Flags<[NoXarchOption]>,
+  HelpText<"Specify comma-separated list of offloading targets.">;
+

yaxunl wrote:
> linjamaki wrote:
> > tra wrote:
> > > `comma-separated list of offloading targets.` is, unfortunately, somewhat 
> > > ambiguous.
> > > Does it mean "how the offload will be done". I.e. HSA, OpenMP, SPIRV, 
> > > CUDA? 
> > > Or does it mean specific hardware we need to generate the code for? 
> > > The code suggests it's a variant of the former, but the option 
> > > description does not. 
> > > 
> > > E.g. `offload_arch_EQ ` also uses the term "offloading target" but with a 
> > > different meaning.
> > > 
> > I’m not sure how to rephrase the option description to be more clear. In 
> > the [1] the `--offload` option is envisioned to be quite 
> > flexible/expressive - it can take in target triples, offload kinds, 
> > processors, aliases for processor sets, etc.
> > 
> > FYI, I have imagined that the `--offload` option would take in explicit 
> > offload kind and target triple combinations as the basis. For example, 
> > something like this:
> > 
> > 
> > ```
> > --offload=hip-amdgcn-amd-amdhsa,openmp-x86_64-pc-linux-gnu
> > ```
> > 
> > And top of that, there would be predefined strings/shortcuts/aliases that 
> > expand to the basic form. For example, 
> > `--offload=sm_70,openmp-host` could expand to something like:
> > 
> > 
> > ```
> > --offload=cuda-nvptx64-nvidia-cuda,openmp-x86_64-pc-linux-gnu 
> > -Xoffload=cuda-nvptx64-nvidia-cuda -march=sm_70 ...
> > 
> > ```
> > Then there is a feature as discussed in [1] that the offload kind can be 
> > dropped if it can be inferred by other means (e.g. from `-x hip` option). 
> > 
> The description better matches the current implementation.
> 
> By this patch, `--offload=` only supports specifying target triple for HIP 
> and assumes default processor. The description should reflect that.
> 
> In the future, as `--offload=` supports more values, the description may be 
> updated.
> 
> Also, `--offload=` is designed to be mutually exclusive with 
> `--offload-arch=`. Probably we should check and diagnose that.
Thanks for the feedback. The option description has been changed to reflect the 
current state. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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

[PATCH] D112404: [SPIR-V] Add translator tool

[Henry Linjamäki via Phabricator via cfe-commits]

linjamaki added a comment.

Thanks for the help!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112404

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

[PATCH] D114142: [clang-format] [PR52527] can join * with /* to form an outside of comment error C4138

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

These don't seem to be issues

  double term = a *  // first
b;
  
  void foo(int*  // this is the first
   ,
   int second);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114142

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

[PATCH] D114142: [clang-format] [PR52527] can join * with /* to form an outside of comment error C4138

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I'm thinking that those cases are handled else where

as S=1 on those elements (which means space required before)

  M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=double L=6 PPK=2 FakeLParens=2/ 
FakeRParens=0 II=0x1d662dd3990 Text='double'
  M=0 C=1 T=StartOfName S=1 F=0 B=0 BK=0 P=220 Name=identifier L=11 PPK=2 
FakeLParens= FakeRParens=0 II=0x1d662dc9488 Text='term'
  M=0 C=0 T=BinaryOperator S=1 F=0 B=0 BK=0 P=22 Name=equal L=13 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='='
  M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=22 Name=identifier L=15 PPK=2 
FakeLParens=14/ FakeRParens=0 II=0x1d662dc94b8 Text='a'
  M=0 C=0 T=BinaryOperator S=1 F=0 B=0 BK=0 P=34 Name=star L=17 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='*'
  M=0 C=0 T=LineComment S=1 F=0 B=0 BK=0 P=34 Name=comment L=26 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='// first'
  M=1 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=1020 Name=identifier L=106 PPK=2 
FakeLParens= FakeRParens=2 II=0x1d662dc94e8 Text='b'
  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=semi L=107 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=';'



  M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=void L=4 PPK=2 FakeLParens= 
FakeRParens=0 II=0x1d662dd3e28 Text='void'
  M=0 C=1 T=FunctionDeclarationName S=1 F=0 B=0 BK=0 P=80 Name=identifier L=8 
PPK=2 FakeLParens= FakeRParens=0 II=0x1d662dc93f8 Text='foo'
  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=l_paren L=9 PPK=1 FakeLParens= 
FakeRParens=0 II=0x0 Text='('
  M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=140 Name=int L=12 PPK=2 FakeLParens=1/ 
FakeRParens=0 II=0x1d662dd3b40 Text='int'
  M=0 C=1 T=PointerOrReference S=1 F=0 B=0 BK=0 P=230 Name=star L=14 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='*'
  M=0 C=0 T=LineComment S=1 F=0 B=0 BK=0 P=54 Name=comment L=35 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='// this is the first'
  M=1 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=1040 Name=comma L=115 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=','
  M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=41 Name=int L=119 PPK=2 FakeLParens= 
FakeRParens=0 II=0x1d662dd3b40 Text='int'
  M=0 C=1 T=StartOfName S=1 F=0 B=0 BK=0 P=240 Name=identifier L=126 PPK=2 
FakeLParens= FakeRParens=1 II=0x1d662dc9518 Text='second'
  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=43 Name=r_paren L=127 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=')'
  M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=semi L=128 PPK=2 FakeLParens= 
FakeRParens=0 II=0x0 Text=';'


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114142

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

[PATCH] D114151: [clang-format] [C++20] [Module] clang-format couldn't recognize partitions

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

There could be more tests in case we want support c++20 module more. This 
shouldn't be a break issue for this patch since this aims to fix the space in 
partitions only.

  export template<...>;
  export struct {
...
  };
  export int func() {
 // ...
  }
  export using A = B;
  template<...>
  export using A = B;
  export {
  
  };
  export type_name var;

Feel free to ignore this if you want to support them in later patches.

Thanks!




Comment at: clang/docs/ReleaseNotes.rst:231
 
+- Improved Cpp20 Modules support
+





Comment at: clang/unittests/Format/FormatTest.cpp:22406
+  verifyFormat("export module foo:bar;", Style);
+  verifyFormat("export module foo;", Style);
+

The module name could contain dot '.' in the middle. Although it seems like 
wouldn't be broken by clang-format, I think it is better to test it.



Comment at: clang/unittests/Format/FormatTest.cpp:22411
+  verifyFormat("import bar;", Style);
+  verifyFormat("import ;", Style);
+

It lacks a test for `import "header";`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114151

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

[clang] 17ec9d1 - [clang][deps] Don't emit `-fmodule-map-file=`

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-11-18T12:31:24+01:00
New Revision: 17ec9d1f6bc1c64a3a49d16852e6f9d705371433

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

LOG: [clang][deps] Don't emit `-fmodule-map-file=`

During explicit modules build, when all modules are provided via 
`-fmodule-file=` and implicit modules and implicit module maps are 
disabled (`-fno-implicit-modules`, `-fno-implicit-module-maps`), we don't need 
to load the original module map files at all. This patch stops emitting the 
`-fmodule-map-file=` arguments we don't need, saving some compilation time due 
to avoiding parsing such module maps and making the command line shorter.

Reviewed By: bnbarham

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

Added: 


Modified: 
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
clang/test/ClangScanDeps/modules-full.cpp
clang/test/ClangScanDeps/modules-inferred.m
clang/test/ClangScanDeps/modules-pch-common-submodule.c
clang/test/ClangScanDeps/modules-pch-common-via-submodule.c
clang/test/ClangScanDeps/modules-pch.c

Removed: 




diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index e426284a84366..739712baadd06 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -24,8 +24,6 @@ std::vector FullDependencies::getAdditionalArgs(
   ClangModuleDeps, LookupPCMPath, LookupModuleDeps, PCMPaths, ModMapPaths);
   for (const std::string &PCMPath : PCMPaths)
 Ret.push_back("-fmodule-file=" + PCMPath);
-  for (const std::string &ModMapPath : ModMapPaths)
-Ret.push_back("-fmodule-map-file=" + ModMapPath);
 
   return Ret;
 }
@@ -37,10 +35,8 @@ FullDependencies::getAdditionalArgsWithoutModulePaths() 
const {
   "-fno-implicit-module-maps",
   };
 
-  for (const PrebuiltModuleDep &PMD : PrebuiltModuleDeps) {
+  for (const PrebuiltModuleDep &PMD : PrebuiltModuleDeps)
 Args.push_back("-fmodule-file=" + PMD.PCMFile);
-Args.push_back("-fmodule-map-file=" + PMD.ModuleMapFile);
-  }
 
   return Args;
 }

diff  --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp 
b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 0e419ab4e2918..383a850301a1f 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -48,10 +48,8 @@ CompilerInvocation 
ModuleDepCollector::makeInvocationForModuleBuildWithoutPaths(
   CI.getLangOpts()->ImplicitModules = false;
 
   // Report the prebuilt modules this module uses.
-  for (const auto &PrebuiltModule : Deps.PrebuiltModuleDeps) {
+  for (const auto &PrebuiltModule : Deps.PrebuiltModuleDeps)
 CI.getFrontendOpts().ModuleFiles.push_back(PrebuiltModule.PCMFile);
-
CI.getFrontendOpts().ModuleMapFiles.push_back(PrebuiltModule.ModuleMapFile);
-  }
 
   Optimize(CI);
 

diff  --git a/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m 
b/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
index e30e312b6a8da..f9d4d89211e3b 100644
--- a/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
+++ b/clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
@@ -46,7 +46,6 @@
 // CHECK-NEXT: "-fno-implicit-modules"
 // CHECK-NEXT: "-fno-implicit-module-maps"
 // CHECK-NEXT: 
"-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H2]]/header2-{{[A-Z0-9]+}}.pcm"
-// CHECK-NEXT: "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap"
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "file-deps": [
 // CHECK-NEXT: "[[PREFIX]]/modules-fmodule-name-no-module-built.m"

diff  --git a/clang/test/ClangScanDeps/modules-full.cpp 
b/clang/test/ClangScanDeps/modules-full.cpp
index 6841d2ff42c60..d7cdf4cce7ad0 100644
--- a/clang/test/ClangScanDeps/modules-full.cpp
+++ b/clang/test/ClangScanDeps/modules-full.cpp
@@ -46,9 +46,6 @@
 // CHECK-NEXT:   "clang-modulemap-file": 
"[[PREFIX]]/Inputs/module.modulemap",
 // CHECK-NEXT:   "command-line": [
 // CHECK-NEXT: "-cc1"
-// CHECK-NO-ABS-NOT:   "-fmodule-map-file={{.*}}"
-// CHECK-ABS:  "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap"
-// CHECK-CUSTOM:   "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap"
 // CHECK:  "-emit-module"
 // CHECK-NO-ABS-NOT:   "-fmodule-file={{.*}}"
 // CHECK-ABS:  
"-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H2_DINCLUDE]]/header2-{{[A-Z0-9]+}}.pcm"
@@ -111,9 +1

[PATCH] D113473: [clang][deps] Don't emit `-fmodule-map-file=`

[Jan Svoboda via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG17ec9d1f6bc1: [clang][deps] Don't emit 
`-fmodule-map-file=` (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113473

Files:
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/modules-fmodule-name-no-module-built.m
  clang/test/ClangScanDeps/modules-full.cpp
  clang/test/ClangScanDeps/modules-inferred.m
  clang/test/ClangScanDeps/modules-pch-common-submodule.c
  clang/test/ClangScanDeps/modules-pch-common-via-submodule.c
  clang/test/ClangScanDeps/modules-pch.c

Index: clang/test/ClangScanDeps/modules-pch.c
===
--- clang/test/ClangScanDeps/modules-pch.c
+++ clang/test/ClangScanDeps/modules-pch.c
@@ -59,7 +59,6 @@
 // CHECK-PCH-NEXT:   "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
 // CHECK-PCH-NEXT:   "command-line": [
 // CHECK-PCH-NEXT: "-cc1"
-// CHECK-PCH:  "-fmodule-map-file=[[PREFIX]]/module.modulemap"
 // CHECK-PCH:  "-emit-module"
 // CHECK-PCH:  "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON_2]]/ModCommon2-{{.*}}.pcm"
 // CHECK-PCH:  "-fmodules"
@@ -92,10 +91,7 @@
 // CHECK-PCH-NEXT: "-fno-implicit-module-maps",
 // CHECK-PCH-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON_1]]/ModCommon1-{{.*}}.pcm",
 // CHECK-PCH-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON_2]]/ModCommon2-{{.*}}.pcm",
-// CHECK-PCH-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_PCH]]/ModPCH-{{.*}}.pcm",
-// CHECK-PCH-NEXT: "-fmodule-map-file=[[PREFIX]]/module.modulemap",
-// CHECK-PCH-NEXT: "-fmodule-map-file=[[PREFIX]]/module.modulemap",
-// CHECK-PCH-NEXT: "-fmodule-map-file=[[PREFIX]]/module.modulemap"
+// CHECK-PCH-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_PCH]]/ModPCH-{{.*}}.pcm"
 // CHECK-PCH-NEXT:   ],
 // CHECK-PCH-NEXT:   "file-deps": [
 // CHECK-PCH-NEXT: "[[PREFIX]]/pch.h"
@@ -163,8 +159,7 @@
 // CHECK-TU-NEXT:   "command-line": [
 // CHECK-TU-NEXT: "-fno-implicit-modules",
 // CHECK-TU-NEXT: "-fno-implicit-module-maps",
-// CHECK-TU-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_TU]]/ModTU-{{.*}}.pcm",
-// CHECK-TU-NEXT: "-fmodule-map-file=[[PREFIX]]/module.modulemap"
+// CHECK-TU-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_TU]]/ModTU-{{.*}}.pcm"
 // CHECK-TU-NEXT:   ],
 // CHECK-TU-NEXT:   "file-deps": [
 // CHECK-TU-NEXT: "[[PREFIX]]/tu.c",
@@ -203,7 +198,6 @@
 // CHECK-TU-WITH-COMMON-NEXT:   "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
 // CHECK-TU-WITH-COMMON-NEXT:   "command-line": [
 // CHECK-TU-WITH-COMMON-NEXT: "-cc1",
-// CHECK-TU-WITH-COMMON:  "-fmodule-map-file=[[PREFIX]]/module.modulemap"
 // CHECK-TU-WITH-COMMON:  "-emit-module",
 // CHECK-TU-WITH-COMMON:  "-fmodule-file=[[PREFIX]]/build/{{.*}}/ModCommon1-{{.*}}.pcm",
 // CHECK-TU-WITH-COMMON:  "-fmodule-name=ModTUWithCommon",
@@ -230,9 +224,7 @@
 // CHECK-TU-WITH-COMMON-NEXT: "-fno-implicit-modules",
 // CHECK-TU-WITH-COMMON-NEXT: "-fno-implicit-module-maps",
 // CHECK-TU-WITH-COMMON-NEXT: "-fmodule-file=[[PREFIX]]/build/{{.*}}/ModCommon2-{{.*}}.pcm",
-// CHECK-TU-WITH-COMMON-NEXT: "-fmodule-map-file=[[PREFIX]]/module.modulemap"
-// CHECK-TU-WITH-COMMON-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_TU_WITH_COMMON]]/ModTUWithCommon-{{.*}}.pcm",
-// CHECK-TU-WITH-COMMON-NEXT: "-fmodule-map-file=[[PREFIX]]/module.modulemap"
+// CHECK-TU-WITH-COMMON-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_TU_WITH_COMMON]]/ModTUWithCommon-{{.*}}.pcm"
 // CHECK-TU-WITH-COMMON-NEXT:   ],
 // CHECK-TU-WITH-COMMON-NEXT:   "file-deps": [
 // CHECK-TU-WITH-COMMON-NEXT: "[[PREFIX]]/tu_with_common.c",
Index: clang/test/ClangScanDeps/modules-pch-common-via-submodule.c
===
--- clang/test/ClangScanDeps/modules-pch-common-via-submodule.c
+++ clang/test/ClangScanDeps/modules-pch-common-via-submodule.c
@@ -47,7 +47,6 @@
 // CHECK-PCH-NEXT: "-fno-implicit-modules"
 // CHECK-PCH-NEXT: "-fno-implicit-module-maps"
 // CHECK-PCH-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON]]/ModCommon-{{.*}}.pcm"
-// CHECK-PCH-NEXT: "-fmodule-map-file=[[PREFIX]]/module.modulemap"
 // CHECK-PCH-NEXT:   ],
 // CHECK-PCH-NEXT:   "file-deps": [
 // CHECK-PCH-NEXT: "[[PREFIX]]/pch.h"
@@ -112,8 +111,7 @@
 // CHECK-TU-NEXT:   "command-line": [
 // CHECK-TU-NEXT: "-fno-implicit-modules",
 // CHECK-TU-NEXT: "-fno

[clang] 7b67908 - [clang][lex] NFC: Remove unused HeaderFileInfo member

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-11-18T12:33:26+01:00
New Revision: 7b6790850968031fe1c098ed6dcc196ddc547ea5

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

LOG: [clang][lex] NFC: Remove unused HeaderFileInfo member

This patch removes `HeaderFileInfo::isNonDefault`, which is not being used 
anywhere.

Reviewed By: dexonsmith, vsapsai

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

Added: 


Modified: 
clang/include/clang/Lex/HeaderSearch.h

Removed: 




diff  --git a/clang/include/clang/Lex/HeaderSearch.h 
b/clang/include/clang/Lex/HeaderSearch.h
index 1ab1b4f235745..7f5c4c260f556 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -130,13 +130,6 @@ struct HeaderFileInfo {
   /// any.
   const IdentifierInfo *
   getControllingMacro(ExternalPreprocessorSource *External);
-
-  /// Determine whether this is a non-default header file info, e.g.,
-  /// it corresponds to an actual header we've included or tried to include.
-  bool isNonDefault() const {
-return isImport || isPragmaOnce || NumIncludes || ControllingMacro ||
-  ControllingMacroID;
-  }
 };
 
 /// An external source of header file information, which may supply



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

[PATCH] D114092: [clang][lex] NFC: Remove unused HeaderFileInfo member

[Jan Svoboda via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7b6790850968: [clang][lex] NFC: Remove unused HeaderFileInfo 
member (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114092

Files:
  clang/include/clang/Lex/HeaderSearch.h


Index: clang/include/clang/Lex/HeaderSearch.h
===
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -130,13 +130,6 @@
   /// any.
   const IdentifierInfo *
   getControllingMacro(ExternalPreprocessorSource *External);
-
-  /// Determine whether this is a non-default header file info, e.g.,
-  /// it corresponds to an actual header we've included or tried to include.
-  bool isNonDefault() const {
-return isImport || isPragmaOnce || NumIncludes || ControllingMacro ||
-  ControllingMacroID;
-  }
 };
 
 /// An external source of header file information, which may supply


Index: clang/include/clang/Lex/HeaderSearch.h
===
--- clang/include/clang/Lex/HeaderSearch.h
+++ clang/include/clang/Lex/HeaderSearch.h
@@ -130,13 +130,6 @@
   /// any.
   const IdentifierInfo *
   getControllingMacro(ExternalPreprocessorSource *External);
-
-  /// Determine whether this is a non-default header file info, e.g.,
-  /// it corresponds to an actual header we've included or tried to include.
-  bool isNonDefault() const {
-return isImport || isPragmaOnce || NumIncludes || ControllingMacro ||
-  ControllingMacroID;
-  }
 };
 
 /// An external source of header file information, which may supply
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D114093: [clang][lex] Refactor check for the first file include

[Jan Svoboda via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Lex/HeaderSearch.h:445-447
+  bool ShouldEnterIncludeFile(bool &IsFirstIncludeOfFile, Preprocessor &PP,
+  const FileEntry *File, bool isImport,
+  bool ModulesEnabled, Module *M);

vsapsai wrote:
> As we've discussed earlier, the order of parameters is slightly weird. 
> Usually, out-parameters are at the end of the list, though I'm not sure there 
> is a rule about it. Personally, I would move `File` to the front because the 
> method is ShouldEnter**IncludeFile** and keep the order as `File, isImport, 
> PP, ModulesEnabled, M, IsFirstIncludeOfFile` but that's more my personal 
> opinion and I'll leave the final decision to you.
I agree the order is sometimes a bit weird. It seems like the convention is to 
pass "bigger" objects (like `Preprocessor`, `HeaderSearch`, `CompilerInstance` 
etc.) first. I'll move the new out param to the back, but probably won't mess 
with the existing parameters.



Comment at: clang/include/clang/Lex/Preprocessor.h:1370-1371
   /// Emits a diagnostic, doesn't enter the file, and returns true on error.
   bool EnterSourceFile(FileID FID, const DirectoryLookup *Dir,
-   SourceLocation Loc);
+   SourceLocation Loc, bool IsFirstIncludeOfFile = true);
 

vsapsai wrote:
> Not sure we need a default value for `IsFirstIncludeOfFile`. Grepping shows 
> the only other place the method is called is `IncrementalParser::Parse`, so 
> we can provide an explicit value there.
It's also called twice in `Preprocessor::EnterMainSourceFile` where specifying 
`true` would seem redundant. I think I'll keep it as is for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114093

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

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 388151.
ChuanqiXu added a comment.

Format codes.


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

https://reviews.llvm.org/D110215

Files:
  clang/include/clang/Basic/Module.h
  clang/include/clang/Lex/ModuleMap.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CXX/module/module.unit/p7/Inputs/CPP.cppm
  clang/test/CXX/module/module.unit/p7/Inputs/h1.h
  clang/test/CXX/module/module.unit/p7/Inputs/h2.h
  clang/test/CXX/module/module.unit/p7/Inputs/h4.h
  clang/test/CXX/module/module.unit/p7/Inputs/h5.h
  clang/test/CXX/module/module.unit/p7/t1.cpp
  clang/test/CXX/module/module.unit/p7/t2.cpp
  clang/test/CXX/module/module.unit/p7/t3.cpp
  clang/test/CXX/module/module.unit/p7/t4.cpp
  clang/test/CXX/module/module.unit/p7/t5.cpp
  clang/test/CXX/module/module.unit/p7/t6.cpp
  clang/test/CodeGenCXX/Inputs/module-extern-C.h
  clang/test/CodeGenCXX/module-extern-C.cpp

Index: clang/test/CodeGenCXX/module-extern-C.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/module-extern-C.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++20 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+
+module;
+
+#include "Inputs/module-extern-C.h"
+
+export module x;
+
+// CHECK: define dso_local void @foo()
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+// CHECK: define dso_local void @bar()
+void bar() {
+  return;
+}
+// CHECK: define dso_local i32 @baz()
+int baz() {
+  return 3;
+}
+// CHECK: define dso_local double @double_func()
+double double_func() {
+  return 5.0;
+}
+}
Index: clang/test/CodeGenCXX/Inputs/module-extern-C.h
===
--- /dev/null
+++ clang/test/CodeGenCXX/Inputs/module-extern-C.h
@@ -0,0 +1,7 @@
+extern "C" void foo();
+extern "C" {
+void bar();
+int baz();
+double double_func();
+}
+extern "C++" class CPP;
Index: clang/test/CXX/module/module.unit/p7/t6.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t6.cpp
@@ -0,0 +1,12 @@
+// RUN: rm -fr %t
+// RUN: mkdir %t
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %S/Inputs/CPP.cppm -I%S/Inputs -o %t/X.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %s -verify
+module;
+#include "Inputs/h2.h"
+export module use;
+import X;
+void printX(CPP *cpp) {
+  cpp->print(); // expected-error {{'CPP' must be defined before it is used}}
+// expected-note@Inputs/CPP.cppm:5 {{definition here is not reachable}}
+}
Index: clang/test/CXX/module/module.unit/p7/t5.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t5.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C++" int a = 5;
Index: clang/test/CXX/module/module.unit/p7/t4.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t4.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C" struct C {
+  int a;
+  int b;
+  double d;
+};
Index: clang/test/CXX/module/module.unit/p7/t3.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t3.cpp
@@ -0,0 +1,7 @@
+// This tests whether the global module would be created when the program don't declare it explicitly.
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+export module x;
+
+extern "C" void foo();
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.unit/p7/t2.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t2.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h2.h"
+
+export module x;
+
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.unit/p7/t1.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.unit/p7/t1.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h1.h"
+
+export module x;
+
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+void bar() {
+  return;
+}
+int baz() {
+  return 3;
+}
+double double_func() {
+  return 5.0;
+}
+}
+
+extern "C++" {
+void bar_cpp() {
+  return;
+}
+int baz_cpp() {
+  return 3;
+}
+double double_func_cpp() {
+  return 5.0;
+}
+}
Index: clang/test/CXX/module/module.unit/p7/Inputs/h5.h
===
--- /dev/null
+++ clang/test/CXX/modu

[PATCH] D114093: [clang][lex] Refactor check for the first file include

[Jan Svoboda via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
jansvoboda11 marked 2 inline comments as done.
Closed by commit rG197576c40986: [clang][lex] Refactor check for the first file 
include (authored by jansvoboda11).

Changed prior to commit:
  https://reviews.llvm.org/D114093?vs=387948&id=388152#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114093

Files:
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Lexer.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/Lexer.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPLexerChange.cpp

Index: clang/lib/Lex/PPLexerChange.cpp
===
--- clang/lib/Lex/PPLexerChange.cpp
+++ clang/lib/Lex/PPLexerChange.cpp
@@ -67,7 +67,8 @@
 /// EnterSourceFile - Add a source file to the top of the include stack and
 /// start lexing tokens from it instead of the current buffer.
 bool Preprocessor::EnterSourceFile(FileID FID, const DirectoryLookup *CurDir,
-   SourceLocation Loc) {
+   SourceLocation Loc,
+   bool IsFirstIncludeOfFile) {
   assert(!CurTokenLexer && "Cannot #include a file inside a macro!");
   ++NumEnteredSourceFiles;
 
@@ -91,7 +92,8 @@
 CodeCompletionFileLoc.getLocWithOffset(CodeCompletionOffset);
   }
 
-  EnterSourceFileWithLexer(new Lexer(FID, *InputFile, *this), CurDir);
+  EnterSourceFileWithLexer(
+  new Lexer(FID, *InputFile, *this, IsFirstIncludeOfFile), CurDir);
   return false;
 }
 
@@ -377,7 +379,7 @@
   CurPPLexer->MIOpt.GetDefinedMacro()) {
   if (!isMacroDefined(ControllingMacro) &&
   DefinedMacro != ControllingMacro &&
-  HeaderInfo.FirstTimeLexingFile(FE)) {
+  CurLexer->isFirstTimeLexingFile()) {
 
 // If the edit distance between the two macros is more than 50%,
 // DefinedMacro may not be header guard, or can be header guard of
Index: clang/lib/Lex/PPDirectives.cpp
===
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2143,12 +2143,14 @@
   IsImportDecl ||
   IncludeTok.getIdentifierInfo()->getPPKeywordID() == tok::pp_import;
 
+  bool IsFirstIncludeOfFile = false;
+
   // Ask HeaderInfo if we should enter this #include file.  If not, #including
   // this file will have no effect.
   if (Action == Enter && File &&
-  !HeaderInfo.ShouldEnterIncludeFile(*this, &File->getFileEntry(),
- EnterOnce, getLangOpts().Modules,
- SuggestedModule.getModule())) {
+  !HeaderInfo.ShouldEnterIncludeFile(
+  *this, &File->getFileEntry(), EnterOnce, getLangOpts().Modules,
+  SuggestedModule.getModule(), IsFirstIncludeOfFile)) {
 // Even if we've already preprocessed this header once and know that we
 // don't need to see its contents again, we still need to import it if it's
 // modular because we might not have imported it from this submodule before.
@@ -2340,7 +2342,8 @@
   }
 
   // If all is good, enter the new file!
-  if (EnterSourceFile(FID, CurDir, FilenameTok.getLocation()))
+  if (EnterSourceFile(FID, CurDir, FilenameTok.getLocation(),
+  IsFirstIncludeOfFile))
 return {ImportAction::None};
 
   // Determine if we're switching to building a new submodule, and which one.
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -133,10 +133,10 @@
 /// assumes that the associated file buffer and Preprocessor objects will
 /// outlive it, so it doesn't take ownership of either of them.
 Lexer::Lexer(FileID FID, const llvm::MemoryBufferRef &InputFile,
- Preprocessor &PP)
+ Preprocessor &PP, bool IsFirstIncludeOfFile)
 : PreprocessorLexer(&PP, FID),
   FileLoc(PP.getSourceManager().getLocForStartOfFile(FID)),
-  LangOpts(PP.getLangOpts()) {
+  LangOpts(PP.getLangOpts()), IsFirstTimeLexingFile(IsFirstIncludeOfFile) {
   InitLexer(InputFile.getBufferStart(), InputFile.getBufferStart(),
 InputFile.getBufferEnd());
 
@@ -147,8 +147,10 @@
 /// suitable for calls to 'LexFromRawLexer'.  This lexer assumes that the text
 /// range will outlive it, so it doesn't take ownership of it.
 Lexer::Lexer(SourceLocation fileloc, const LangOptions &langOpts,
- const char *BufStart, const char *BufPtr, const char *BufEnd)
-: FileLoc(fileloc), LangOpts(langOpts) {
+ const char *BufStart, const char *BufPtr, const char *BufEnd,
+ bool IsFirstIncludeOfFile)
+: FileLoc(fileloc), LangOpts(langOpts),
+  IsFirstTimeLexingFile(IsFirstIncl

[clang] 197576c - [clang][lex] Refactor check for the first file include

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-11-18T13:01:07+01:00
New Revision: 197576c40986085cbd5250283e1e80a2679c9cf0

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

LOG: [clang][lex] Refactor check for the first file include

This patch refactors the code that checks whether a file has just been included 
for the first time.

The `HeaderSearch::FirstTimeLexingFile` function is removed and the information 
is threaded to the original call site from 
`HeaderSearch::ShouldEnterIncludeFile`. This will make it possible to avoid 
tracking the number of includes in a follow up patch.

Depends on D114092.

Reviewed By: dexonsmith

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

Added: 


Modified: 
clang/include/clang/Lex/HeaderSearch.h
clang/include/clang/Lex/Lexer.h
clang/include/clang/Lex/Preprocessor.h
clang/lib/Lex/HeaderSearch.cpp
clang/lib/Lex/Lexer.cpp
clang/lib/Lex/PPDirectives.cpp
clang/lib/Lex/PPLexerChange.cpp

Removed: 




diff  --git a/clang/include/clang/Lex/HeaderSearch.h 
b/clang/include/clang/Lex/HeaderSearch.h
index 7f5c4c260f55..b3445703f782 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -443,8 +443,8 @@ class HeaderSearch {
   /// \return false if \#including the file will have no effect or true
   /// if we should include it.
   bool ShouldEnterIncludeFile(Preprocessor &PP, const FileEntry *File,
-  bool isImport, bool ModulesEnabled,
-  Module *M);
+  bool isImport, bool ModulesEnabled, Module *M,
+  bool &IsFirstIncludeOfFile);
 
   /// Return whether the specified file is a normal header,
   /// a system header, or a C++ friendly system header.
@@ -489,11 +489,6 @@ class HeaderSearch {
 getFileInfo(File).ControllingMacro = ControllingMacro;
   }
 
-  /// Return true if this is the first time encountering this header.
-  bool FirstTimeLexingFile(const FileEntry *File) {
-return getFileInfo(File).NumIncludes == 1;
-  }
-
   /// Determine whether this file is intended to be safe from
   /// multiple inclusions, e.g., it has \#pragma once or a controlling
   /// macro.

diff  --git a/clang/include/clang/Lex/Lexer.h b/clang/include/clang/Lex/Lexer.h
index 82f494e7c8cf..ba1706b1d13e 100644
--- a/clang/include/clang/Lex/Lexer.h
+++ b/clang/include/clang/Lex/Lexer.h
@@ -128,6 +128,9 @@ class Lexer : public PreprocessorLexer {
 
   bool HasLeadingEmptyMacro;
 
+  /// True if this is the first time we're lexing the input file.
+  bool IsFirstTimeLexingFile;
+
   // NewLinePtr - A pointer to new line character '\n' being lexed. For '\r\n',
   // it also points to '\n.'
   const char *NewLinePtr;
@@ -142,19 +145,22 @@ class Lexer : public PreprocessorLexer {
   /// with the specified preprocessor managing the lexing process.  This lexer
   /// assumes that the associated file buffer and Preprocessor objects will
   /// outlive it, so it doesn't take ownership of either of them.
-  Lexer(FileID FID, const llvm::MemoryBufferRef &InputFile, Preprocessor &PP);
+  Lexer(FileID FID, const llvm::MemoryBufferRef &InputFile, Preprocessor &PP,
+bool IsFirstIncludeOfFile = true);
 
   /// Lexer constructor - Create a new raw lexer object.  This object is only
   /// suitable for calls to 'LexFromRawLexer'.  This lexer assumes that the
   /// text range will outlive it, so it doesn't take ownership of it.
   Lexer(SourceLocation FileLoc, const LangOptions &LangOpts,
-const char *BufStart, const char *BufPtr, const char *BufEnd);
+const char *BufStart, const char *BufPtr, const char *BufEnd,
+bool IsFirstIncludeOfFile = true);
 
   /// Lexer constructor - Create a new raw lexer object.  This object is only
   /// suitable for calls to 'LexFromRawLexer'.  This lexer assumes that the
   /// text range will outlive it, so it doesn't take ownership of it.
   Lexer(FileID FID, const llvm::MemoryBufferRef &FromFile,
-const SourceManager &SM, const LangOptions &LangOpts);
+const SourceManager &SM, const LangOptions &LangOpts,
+bool IsFirstIncludeOfFile = true);
 
   Lexer(const Lexer &) = delete;
   Lexer &operator=(const Lexer &) = delete;
@@ -563,6 +569,9 @@ class Lexer : public PreprocessorLexer {
   static StringRef getIndentationForLine(SourceLocation Loc,
  const SourceManager &SM);
 
+  /// Check if this is the first time we're lexing the input file.
+  bool isFirstTimeLexingFile() const { return IsFirstTimeLexingFile; }
+
 private:
   
//======//
   // Internal implementation interfaces.

diff  --git a/clang/include/clang/Lex/Pr

[PATCH] D110216: [clang] retain type sugar in auto / template argument deduction

[Kai Luo via Phabricator via cfe-commits]

lkail added a comment.

Hi we found regression in our internal tests. It compiles with clang-13.0.0 
https://godbolt.org/z/3abGrcf7o and gcc https://godbolt.org/z/K9oj3Grs1, but 
fails with clang-trunk https://godbolt.org/z/1Tehxa1x9. Is it an intended 
change? If so, do we have to document this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110216

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

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D110215#3139589 , @ChuanqiXu wrote:

> Change includes:
>
> - Add parent for newly created global module fragment.
> - Add a test.
> - Rename test directory. Suddenly I found that the names of the tests in CXX 
> directory are corresponds to the standard one by one. So my name before is 
> not good.
>
> @aaron.ballman @urnathan It looks like @rsmith is busy now. Could you please 
> help to review this one? Many Thanks.

@ChuanqiXu -- I'm happy to take a look (all of your modules patches are still 
on my review queue), but I've been in WG14 meetings all week this week and will 
be on vacation all week next week. Given the complexity and importance of the 
patches, it may be a bit before I can give this a thorough review. (If you get 
approval from other trusted reviewers before I get to anything, please don't 
let me hold any of the patches up!)


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

https://reviews.llvm.org/D110215

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

[PATCH] D114149: [clang-tidy] Fix pr48613: "llvm-header-guard uses a reserved identifier"

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman requested changes to this revision.
aaron.ballman added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp:28
+/// Replace invalid characters in the identifier with '_'.
+static std::string replaceInvalidChars(StringRef Identifier) {
+  std::string Fixed{Identifier};

This fixes some issues but I think it introduces new issues too. The old code 
would accept Unicode characters and pass them along as (valid) identifier 
characters. The new code will replace all the Unicode characters with `_` which 
means some people's header guards may go from useful to `___`. We 
should definitely add test cases for Unicode file names.

Ultimately, I think this functionality will want to use `UnicodeCharSets.h` to 
implement something along the lines of what's done in 
`isAllowedInitiallyIDChar()` and `isAllowedIDChar()` from Lexer.cpp.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114149

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

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114025

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

[PATCH] D113251: [analyzer][doc] Add user documenation for taint analysis

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: clang/docs/analyzer/checkers.rst:2341-2342
+
+Default propagations defined by `GenericTaintChecker`:
+``atoi``, ``atol``, ``atoll``, ``fgetc``, ``fgetln``, ``fgets``, ``fscanf``, 
``sscanf``, ``getc``, ``getc_unlocked``, ``getdelim``, ``getline``, ``getw``, 
``pread``, ``read``, ``strchr``, ``strrchr``, ``tolower``, ``toupper``
+

What does it mean that these are "default propagations"? That taint passes 
through calls to them trivially?



Comment at: clang/docs/analyzer/checkers.rst:2345
+Default sinks defined in `GenericTaintChecker`:
+``printf``, ``setproctitle``, ``system``, ``popen``, ``execl``, ``execle``, 
``execlp``, ``execv``, ``execvp``, ``execvP``, ``execve``, ``dlopen``, 
``memcpy``, ``memmove``, ``strncpy``, ``strndup``, ``malloc``, ``calloc``, 
``alloca``, ``memccpy``, ``realloc``, ``bcopy``
+

`execvp` and `execvP`? What is the capital `P` for? I can't find this overload 
in the POSIX docs.



Comment at: clang/docs/analyzer/checkers.rst:2353
+
+External taint configuration is in `YAML `_ format. The 
taint-related options defined in the config file extend but do not override the 
built-in sources, rules, sinks.
+

Perhaps we should link to the in-house YAML doc 
(http://llvm.org/docs/YamlIO.html#introduction-to-yaml) first, and have the 
user move to other sources from there?



Comment at: clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst:5
+
+Clang Static Analyzer uses taint analysis to detect security-related issues in 
code.
+The backbone of taint analysis in Clang is the `GenericTaintChecker`, which 
the user can access via the :ref:`alpha-security-taint-TaintPropagation` 
checker alias and this checker has a default taint-related configuration.

//The// Clang Static [...]?



Comment at: clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst:5
+
+Clang Static Analyzer uses taint analysis to detect security-related issues in 
code.
+The backbone of taint analysis in Clang is the `GenericTaintChecker`, which 
the user can access via the :ref:`alpha-security-taint-TaintPropagation` 
checker alias and this checker has a default taint-related configuration.

whisperity wrote:
> //The// Clang Static [...]?
uses, or can use?



Comment at: clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst:6
+Clang Static Analyzer uses taint analysis to detect security-related issues in 
code.
+The backbone of taint analysis in Clang is the `GenericTaintChecker`, which 
the user can access via the :ref:`alpha-security-taint-TaintPropagation` 
checker alias and this checker has a default taint-related configuration.
+The checker also provides a configuration interface for extending the default 
settings by providing a configuration file in `YAML `_ 
format.

Clang -> Clang SA / CSA?



Comment at: clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst:6
+Clang Static Analyzer uses taint analysis to detect security-related issues in 
code.
+The backbone of taint analysis in Clang is the `GenericTaintChecker`, which 
the user can access via the :ref:`alpha-security-taint-TaintPropagation` 
checker alias and this checker has a default taint-related configuration.
+The checker also provides a configuration interface for extending the default 
settings by providing a configuration file in `YAML `_ 
format.

whisperity wrote:
> Clang -> Clang SA / CSA?
So the checker has the defaults all the time, or only by enabling the alias can 
I get the defaults?



Comment at: clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst:7
+The backbone of taint analysis in Clang is the `GenericTaintChecker`, which 
the user can access via the :ref:`alpha-security-taint-TaintPropagation` 
checker alias and this checker has a default taint-related configuration.
+The checker also provides a configuration interface for extending the default 
settings by providing a configuration file in `YAML `_ 
format.
+This documentation describes the syntax of the configuration file and gives 
the informal semantics of the configuration options.

Ditto about YAML.



Comment at: clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst:13
+
+.. _taint-configuration-overview
+

I think this wanted to be an anchor? Also see how the syntax highlighting broke 
in the next section.



Comment at: clang/docs/analyzer/user-docs/TaintAnalysisConfiguration.rst:23
+
+.. _taint-configuration-example:
+

AFAIK, RST anchors are global identifiers for the project, so maybe we should 
take care in adding "clang-sa" or some other sort of namespaceing...

(Although this might depend on

[PATCH] D114149: [clang-tidy] Fix pr48613: "llvm-header-guard uses a reserved identifier"

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp:42-46
+/// Remove all '_' characters at the beginning of the identifier. Only reserved
+/// identifiers are allowed to start with these.
+static StringRef dropLeadingUnderscores(StringRef Identifier) {
+  return Identifier.drop_while([](char c) { return c == '_'; });
+}

Is this true? At least in C++ (and perhaps in C) I believe `_foo` is a 
non-reserved identifier, only `__foo` or `_Foo` would be reserved.



Comment at: clang-tools-extra/clang-tidy/utils/HeaderGuard.h:41
 
+  /// Ensure that the provided header guard is a valid identifier.
+  static std::string sanitizeHeaderGuard(StringRef Guard);

valid **non-reserved**


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114149

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

[PATCH] D112024: [clang] diagnose_as attribute for Fortify diagnosing like builtins.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:5973
+``mymemset(n, c, s)`` will diagnose overflows as if it were the call
+``__builtin_memset(s, c, n)``;
+}];

One thing that's not quite clear from the docs is how to handle variadic arg 
builtins like `__builtin_printf` -- how do you specify "the variadic args go 
here"?

Another thing that's unclear is whether you can apply this attribute to a 
member function, and if so, how does that change the indexes given for the 
implicit `this` argument?

We should add test coverage for both of these situations.



Comment at: clang/lib/Sema/SemaChecking.cpp:673
 
-  auto ComputeExplicitObjectSizeArgument =
+  const auto TranslateIndex = [&](unsigned Index) -> Optional {
+if (UseDAIAttr) {

We typically don't use top-level `const` on value types for local 
variables/parameters (only on pointers or references). (same comment applies 
elsewhere)



Comment at: clang/lib/Sema/SemaChecking.cpp:692
   return llvm::None;
-return Result.Val.getInt();
+auto Integer = Result.Val.getInt();
+Integer.setIsUnsigned(true);

Please spell out the type.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1006
+const ParsedAttr &AL) {
+  const auto *DeclFD = dyn_cast_or_null(D);
+  if (!DeclFD)

I don't believe `D` can ever be null.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1014-1018
+  if (Union.is()) {
+return Union.get()->getBeginLoc();
+  } else {
+return Union.get()->Loc;
+  }

Removes curly braces and an `else` after a `return`.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1040
+S.Diag(AL.getLoc(), diag::err_attribute_wrong_number_arguments_for)
+<< AL << AttrFD->getName() << AttrFD->getNumParams();
+return;

Passing in a `NamedDecl` will be automatically quoted by the diagnostics 
engine, while passing in a `StringRef` will not. We prefer syntax to be quoted 
in diagnostics to reduce confusion.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1055-1057
+if (!checkUInt32Argument(S, AL, IndexExpr, Index, I + 1, false)) {
+  return;
+}





Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1061
+  S.Diag(AL.getLoc(), diag::err_attribute_bounds_for_function)
+  << AL << Index << DeclFD->getName() << DeclFD->getNumParams();
+  return;





Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1065-1066
+
+const auto T1 = AttrFD->getParamDecl(I - 1)->getType();
+const auto T2 = DeclFD->getParamDecl(Index - 1)->getType();
+

Please spell out the type as it's not spelled out in the initialization.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:1068-1069
+
+if (T1.getCanonicalType().getUnqualifiedType() !=
+T2.getCanonicalType().getUnqualifiedType()) {
+  S.Diag(IndexExpr->getBeginLoc(), diag::err_attribute_parameter_types)

This may be fine, but it may also be overly restrictive. Consider: 
```
void f(void *dest, char val, size_t n) 
__attribute__((diagnose_as_builtin(__builtin_memset, 1, 2, 3))) {}
```
I think this will get diagnosed because of the type mismatch between `char` and 
`int`, won't it? But notionally, the argument passed to `val` will be promoted 
to `int` anyway, so they're morally equivalent. This would be a good test case 
to add.

FWIW, I prefer erring on the side of being overly restrictive (we can relax 
restrictions more easily than we can add new ones), which is why I think this 
is fine for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112024

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

[PATCH] D114149: [clang-tidy] Fix pr48613: "llvm-header-guard uses a reserved identifier"

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp:42-46
+/// Remove all '_' characters at the beginning of the identifier. Only reserved
+/// identifiers are allowed to start with these.
+static StringRef dropLeadingUnderscores(StringRef Identifier) {
+  return Identifier.drop_while([](char c) { return c == '_'; });
+}

whisperity wrote:
> Is this true? At least in C++ (and perhaps in C) I believe `_foo` is a 
> non-reserved identifier, only `__foo` or `_Foo` would be reserved.
`_foo` is reserved if its an external identifier in both C and C++: 
https://godbolt.org/z/GnG4v33vK

However, the issue here is that header guards are capitalized, so a file name 
like `_foo` still should drop the leading underscore so it doesn't get turned 
into `_FOO` which is always reserved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114149

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

[PATCH] D114120: [clang] Remove CLANG_ROUND_TRIP_CC1_ARGS and always roundtrip in +assert builds

[Nico Weber via Phabricator via cfe-commits]

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



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:615
 // May perform round-trip of command line arguments. By default, the round-trip
 // is enabled if CLANG_ROUND_TRIP_CC1_ARGS was defined during build. This can 
be
 // overwritten at run-time via the "-round-trip-args" and "-no-round-trip-args"

jansvoboda11 wrote:
> Can you also update this comment that menions `CLANG_ROUND_TRIP_CC1_ARGS`?
Done, thanks!


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

https://reviews.llvm.org/D114120

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

[clang] 3950e1b - [clang] Remove CLANG_ROUND_TRIP_CC1_ARGS and always roundtrip in +assert builds

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2021-11-18T08:31:21-05:00
New Revision: 3950e1be8d6ec602fda525ea20340fd3f8e2e919

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

LOG: [clang] Remove CLANG_ROUND_TRIP_CC1_ARGS and always roundtrip in +assert 
builds

This removes the ability to disable roundtripping in assert builds.
(Roundtripping happens by default in assert builds both before and after
this patch.)

The CLANG_ROUND_TRIP_CC1_ARGS was added as an escape hatch 9 months ago
in https://reviews.llvm.org/D97462, with a FIXME to remove it eventually.
It's probably time to remove it.

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

Added: 


Modified: 
clang/CMakeLists.txt
clang/lib/Frontend/CompilerInvocation.cpp

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 5a58d7faa9fa3..6f26e94f31602 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -462,9 +462,6 @@ option(CLANG_ENABLE_STATIC_ANALYZER
 
 option(CLANG_ENABLE_PROTO_FUZZER "Build Clang protobuf fuzzer." OFF)
 
-option(CLANG_ROUND_TRIP_CC1_ARGS
-  "Round-trip command line arguments in -cc1." ${LLVM_ENABLE_ASSERTIONS})
-
 if(NOT CLANG_ENABLE_STATIC_ANALYZER AND CLANG_ENABLE_ARCMT)
   message(FATAL_ERROR "Cannot disable static analyzer while enabling ARCMT or 
Z3")
 endif()
@@ -473,10 +470,6 @@ if(CLANG_ENABLE_ARCMT)
   set(CLANG_ENABLE_OBJC_REWRITER ON)
 endif()
 
-if (CLANG_ROUND_TRIP_CC1_ARGS)
-  add_definitions(-DCLANG_ROUND_TRIP_CC1_ARGS=ON)
-endif()
-
 # Clang version information
 set(CLANG_EXECUTABLE_VERSION
 "${CLANG_VERSION_MAJOR}" CACHE STRING

diff  --git a/clang/lib/Frontend/CompilerInvocation.cpp 
b/clang/lib/Frontend/CompilerInvocation.cpp
index 8d33a59cb9839..c104a6f40e20f 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -612,9 +612,8 @@ using GenerateFn = llvm::function_ref;
 
 // May perform round-trip of command line arguments. By default, the round-trip
-// is enabled if CLANG_ROUND_TRIP_CC1_ARGS was defined during build. This can 
be
-// overwritten at run-time via the "-round-trip-args" and "-no-round-trip-args"
-// command line flags.
+// is enabled in assert builds. This can be overwritten at run-time via the
+// "-round-trip-args" and "-no-round-trip-args" command line flags.
 // During round-trip, the command line arguments are parsed into a dummy
 // instance of CompilerInvocation which is used to generate the command line
 // arguments again. The real CompilerInvocation instance is then created by
@@ -624,8 +623,7 @@ static bool RoundTrip(ParseFn Parse, GenerateFn Generate,
   CompilerInvocation &DummyInvocation,
   ArrayRef CommandLineArgs,
   DiagnosticsEngine &Diags, const char *Argv0) {
-  // FIXME: Switch to '#ifndef NDEBUG' when possible.
-#ifdef CLANG_ROUND_TRIP_CC1_ARGS
+#ifndef NDEBUG
   bool DoRoundTripDefault = true;
 #else
   bool DoRoundTripDefault = false;



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

[PATCH] D114120: [clang] Remove CLANG_ROUND_TRIP_CC1_ARGS and always roundtrip in +assert builds

[Nico Weber via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
thakis marked an inline comment as done.
Closed by commit rG3950e1be8d6e: [clang] Remove CLANG_ROUND_TRIP_CC1_ARGS and 
always roundtrip in +assert builds (authored by thakis).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D114120?vs=388024&id=388166#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114120

Files:
  clang/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -612,9 +612,8 @@
 CompilerInvocation::StringAllocator)>;
 
 // May perform round-trip of command line arguments. By default, the round-trip
-// is enabled if CLANG_ROUND_TRIP_CC1_ARGS was defined during build. This can 
be
-// overwritten at run-time via the "-round-trip-args" and "-no-round-trip-args"
-// command line flags.
+// is enabled in assert builds. This can be overwritten at run-time via the
+// "-round-trip-args" and "-no-round-trip-args" command line flags.
 // During round-trip, the command line arguments are parsed into a dummy
 // instance of CompilerInvocation which is used to generate the command line
 // arguments again. The real CompilerInvocation instance is then created by
@@ -624,8 +623,7 @@
   CompilerInvocation &DummyInvocation,
   ArrayRef CommandLineArgs,
   DiagnosticsEngine &Diags, const char *Argv0) {
-  // FIXME: Switch to '#ifndef NDEBUG' when possible.
-#ifdef CLANG_ROUND_TRIP_CC1_ARGS
+#ifndef NDEBUG
   bool DoRoundTripDefault = true;
 #else
   bool DoRoundTripDefault = false;
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -462,9 +462,6 @@
 
 option(CLANG_ENABLE_PROTO_FUZZER "Build Clang protobuf fuzzer." OFF)
 
-option(CLANG_ROUND_TRIP_CC1_ARGS
-  "Round-trip command line arguments in -cc1." ${LLVM_ENABLE_ASSERTIONS})
-
 if(NOT CLANG_ENABLE_STATIC_ANALYZER AND CLANG_ENABLE_ARCMT)
   message(FATAL_ERROR "Cannot disable static analyzer while enabling ARCMT or 
Z3")
 endif()
@@ -473,10 +470,6 @@
   set(CLANG_ENABLE_OBJC_REWRITER ON)
 endif()
 
-if (CLANG_ROUND_TRIP_CC1_ARGS)
-  add_definitions(-DCLANG_ROUND_TRIP_CC1_ARGS=ON)
-endif()
-
 # Clang version information
 set(CLANG_EXECUTABLE_VERSION
 "${CLANG_VERSION_MAJOR}" CACHE STRING


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -612,9 +612,8 @@
 CompilerInvocation::StringAllocator)>;
 
 // May perform round-trip of command line arguments. By default, the round-trip
-// is enabled if CLANG_ROUND_TRIP_CC1_ARGS was defined during build. This can be
-// overwritten at run-time via the "-round-trip-args" and "-no-round-trip-args"
-// command line flags.
+// is enabled in assert builds. This can be overwritten at run-time via the
+// "-round-trip-args" and "-no-round-trip-args" command line flags.
 // During round-trip, the command line arguments are parsed into a dummy
 // instance of CompilerInvocation which is used to generate the command line
 // arguments again. The real CompilerInvocation instance is then created by
@@ -624,8 +623,7 @@
   CompilerInvocation &DummyInvocation,
   ArrayRef CommandLineArgs,
   DiagnosticsEngine &Diags, const char *Argv0) {
-  // FIXME: Switch to '#ifndef NDEBUG' when possible.
-#ifdef CLANG_ROUND_TRIP_CC1_ARGS
+#ifndef NDEBUG
   bool DoRoundTripDefault = true;
 #else
   bool DoRoundTripDefault = false;
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -462,9 +462,6 @@
 
 option(CLANG_ENABLE_PROTO_FUZZER "Build Clang protobuf fuzzer." OFF)
 
-option(CLANG_ROUND_TRIP_CC1_ARGS
-  "Round-trip command line arguments in -cc1." ${LLVM_ENABLE_ASSERTIONS})
-
 if(NOT CLANG_ENABLE_STATIC_ANALYZER AND CLANG_ENABLE_ARCMT)
   message(FATAL_ERROR "Cannot disable static analyzer while enabling ARCMT or Z3")
 endif()
@@ -473,10 +470,6 @@
   set(CLANG_ENABLE_OBJC_REWRITER ON)
 endif()
 
-if (CLANG_ROUND_TRIP_CC1_ARGS)
-  add_definitions(-DCLANG_ROUND_TRIP_CC1_ARGS=ON)
-endif()
-
 # Clang version information
 set(CLANG_EXECUTABLE_VERSION
 "${CLANG_VERSION_MAJOR}" CACHE STRING
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111318: [clang][clangd] Improve signature help for variadic functions.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz updated this revision to Diff 388167.
adamcz marked an inline comment as done.
adamcz added a comment.

addressed review comments, added new test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111318

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/include/clang/Sema/Overload.h
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CodeCompletion/variadic-template.cpp

Index: clang/test/CodeCompletion/variadic-template.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/variadic-template.cpp
@@ -0,0 +1,18 @@
+template 
+void fun(T x, Args... args) {}
+
+void f() {
+  fun(1, 2, 3, 4);
+  // The results are quite awkward here, but it's the best we can do for now.
+  // Tools, including clangd, can unexpand "args" when showing this to the user.
+  // The important thing is that we provide OVERLOAD signature in all those cases.
+  //
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:7 %s -o - | FileCheck --check-prefix=CHECK-1 %s
+  // CHECK-1: OVERLOAD: [#void#]fun(<#T x#>, Args args...)
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:10 %s -o - | FileCheck --check-prefix=CHECK-2 %s
+  // CHECK-2: OVERLOAD: [#void#]fun(int x)
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:13 %s -o - | FileCheck --check-prefix=CHECK-3 %s
+  // CHECK-3: OVERLOAD: [#void#]fun(int x, int args)
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:16 %s -o - | FileCheck --check-prefix=CHECK-4 %s
+  // CHECK-4: OVERLOAD: [#void#]fun(int x, int args, int args)
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -6456,7 +6456,8 @@
   // parameters is viable only if it has an ellipsis in its parameter
   // list (8.3.5).
   if (TooManyArguments(NumParams, Args.size(), PartialOverloading) &&
-  !Proto->isVariadic()) {
+  !Proto->isVariadic() &&
+  shouldEnforceArgLimit(PartialOverloading, Function)) {
 Candidate.Viable = false;
 Candidate.FailureKind = ovl_fail_too_many_arguments;
 return;
@@ -6946,7 +6947,8 @@
   // parameters is viable only if it has an ellipsis in its parameter
   // list (8.3.5).
   if (TooManyArguments(NumParams, Args.size(), PartialOverloading) &&
-  !Proto->isVariadic()) {
+  !Proto->isVariadic() &&
+  shouldEnforceArgLimit(PartialOverloading, Method)) {
 Candidate.Viable = false;
 Candidate.FailureKind = ovl_fail_too_many_arguments;
 return;
@@ -15242,3 +15244,21 @@
 FunctionDecl *Fn) {
   return FixOverloadedFunctionReference(E.get(), Found, Fn);
 }
+
+bool clang::shouldEnforceArgLimit(bool PartialOverloading,
+  FunctionDecl *Function) {
+  if (!PartialOverloading || !Function)
+return true;
+  if (Function->isVariadic())
+return false;
+  if (const auto *Proto =
+  dyn_cast(Function->getFunctionType()))
+if (Proto->isTemplateVariadic())
+  return false;
+  if (auto *Pattern = Function->getTemplateInstantiationPattern())
+if (const auto *Proto =
+dyn_cast(Pattern->getFunctionType()))
+  if (Proto->isTemplateVariadic())
+return false;
+  return true;
+}
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -5818,7 +5818,8 @@
 if (Candidate.Function) {
   if (Candidate.Function->isDeleted())
 continue;
-  if (!Candidate.Function->isVariadic() &&
+  if (shouldEnforceArgLimit(/*PartialOverloading=*/true,
+Candidate.Function) &&
   Candidate.Function->getNumParams() <= ArgSize &&
   // Having zero args is annoying, normally we don't surface a function
   // with 2 params, if you already have 2 params, because you are
Index: clang/include/clang/Sema/Overload.h
===
--- clang/include/clang/Sema/Overload.h
+++ clang/include/clang/Sema/Overload.h
@@ -1204,6 +1204,20 @@
 return Info;
   }
 
+  // Returns false if signature help is relevant despite number of arguments
+  // exceeding parameters. Specifically, it returns false when
+  // PartialOverloading is true and one of the following:
+  // * Function is variadic
+  // * Function is template variadic
+  // * Function is an instantiation of template variadic function
+  // The last case may seem strange. The idea is that if we added one more
+  // argument, we'd end up with a function similar to Function. Since, in the
+  // context of signature help and/or code completion, we do n

[PATCH] D111318: [clang][clangd] Improve signature help for variadic functions.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz added a comment.

Added a slightly awkward clang (not clangd) test. The output is suboptimial, 
because it's not clangd, but it shows things work as they should.




Comment at: clang-tools-extra/clangd/CodeComplete.cpp:929
+//
+// This only matters for variadic functions/templates, where CurrentArg
+// might be higher than the number of parameters. When that happens, we

sammccall wrote:
> hmm, now that I think about it, should this conversion from arg-index to 
> param-index be happening here, or done already by sema and the param index 
> passed to ProcessOverloadCandidates?
> 
> (The docs say "current arg" but I'm struggling to see how that can really be 
> useful)
Since ProcessOverloadCandidates is called with multiple candidates CurrentParam 
wouldn't make sense - which candidate would that be referring to? What if the 
candidates are:
  void fun1(int x, ...);
  void fun2(int x, int y, ...);
and CurrentArg is 3, that means for the first signature activeParameter = 1, 
while for the second one activeParameter = 2.

Right now we do not return this information, but we should (I left a FIXME and 
intend to follow up, LSP already has the per-signature field).

We could consider moving the CurrentArg out of this function signature and into 
the OverloadCandidate, so it's per candidate in Sema too. Is that what you're 
suggesting? I think the current state of things is fine, but if you think I 
should remove CurrentArg and replace it with a field in OverloadCandidate, let 
me know.




Comment at: clang-tools-extra/clangd/CodeComplete.cpp:945
+}
+SigHelp.activeParameter =
+std::min(SigHelp.activeParameter, NumParams - 1);

sammccall wrote:
> hmm, if foo() is a non-variadic 0-arg function, we set activeParameter to -1.
> Before this patch it's zero which is still weird, but less weird.
Right, that's unintended. Fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111318

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

[PATCH] D111318: [clang][clangd] Improve signature help for variadic functions.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:929
+//
+// This only matters for variadic functions/templates, where CurrentArg
+// might be higher than the number of parameters. When that happens, we

adamcz wrote:
> sammccall wrote:
> > hmm, now that I think about it, should this conversion from arg-index to 
> > param-index be happening here, or done already by sema and the param index 
> > passed to ProcessOverloadCandidates?
> > 
> > (The docs say "current arg" but I'm struggling to see how that can really 
> > be useful)
> Since ProcessOverloadCandidates is called with multiple candidates 
> CurrentParam wouldn't make sense - which candidate would that be referring 
> to? What if the candidates are:
>   void fun1(int x, ...);
>   void fun2(int x, int y, ...);
> and CurrentArg is 3, that means for the first signature activeParameter = 1, 
> while for the second one activeParameter = 2.
> 
> Right now we do not return this information, but we should (I left a FIXME 
> and intend to follow up, LSP already has the per-signature field).
> 
> We could consider moving the CurrentArg out of this function signature and 
> into the OverloadCandidate, so it's per candidate in Sema too. Is that what 
> you're suggesting? I think the current state of things is fine, but if you 
> think I should remove CurrentArg and replace it with a field in 
> OverloadCandidate, let me know.
> 
Whoops, of course you're right, it's more complicated.

In principle, this seems like it does belong on OverloadCandidate. Doing this 
as a field seems like a yak-shave, maybe it makes sense to make 
paramIndexForArg a method on OverloadCandidate though. (And still require 
CurrentArg as input). Up to you, this is fine as it is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111318

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

[PATCH] D114099: Enable `_Float16` type support on X86 without the avx512fp16 flag

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 388179.
zahiraam marked 7 inline comments as done.

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

https://reviews.llvm.org/D114099

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/X86.cpp
  clang/test/CodeGen/X86/Float16-arithmetic.c
  clang/test/CodeGen/X86/avx512fp16-abi.c
  clang/test/CodeGen/X86/avx512fp16-complex.c
  clang/test/CodeGen/X86/fp16-abi.c
  clang/test/CodeGen/X86/fp16-complex.c
  clang/test/Sema/Float16.c
  clang/test/Sema/conversion-target-dep.c
  clang/test/SemaCXX/Float16.cpp

Index: clang/test/SemaCXX/Float16.cpp
===
--- clang/test/SemaCXX/Float16.cpp
+++ clang/test/SemaCXX/Float16.cpp
@@ -1,18 +1,9 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 
-#ifdef HAVE
 // expected-no-diagnostics
-#endif // HAVE
 
-#ifndef HAVE
-// expected-error@+2{{_Float16 is not supported on this target}}
-#endif // !HAVE
 _Float16 f;
-
-#ifndef HAVE
-// expected-error@+2{{invalid suffix 'F16' on floating constant}}
-#endif // !HAVE
 const auto g = 1.1F16;
Index: clang/test/Sema/conversion-target-dep.c
===
--- clang/test/Sema/conversion-target-dep.c
+++ clang/test/Sema/conversion-target-dep.c
@@ -6,7 +6,7 @@
 
 long double ld;
 double d;
-_Float16 f16; // x86-error {{_Float16 is not supported on this target}}
+_Float16 f16;
 
 int main() {
   ld = d; // x86-warning {{implicit conversion increases floating-point precision: 'double' to 'long double'}}
Index: clang/test/Sema/Float16.c
===
--- clang/test/Sema/Float16.c
+++ clang/test/Sema/Float16.c
@@ -1,18 +1,13 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc -target-feature +avx512fp16 %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc -target-feature +avx512fp16 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 
-#ifndef HAVE
-// expected-error@+2{{_Float16 is not supported on this target}}
-#endif // HAVE
 _Float16 f;
 
-#ifdef HAVE
 _Complex _Float16 a;
 void builtin_complex() {
   _Float16 a = 0;
   (void)__builtin_complex(a, a); // expected-error {{'_Complex _Float16' is invalid}}
 }
-#endif
Index: clang/test/CodeGen/X86/fp16-complex.c
===
--- clang/test/CodeGen/X86/fp16-complex.c
+++ clang/test/CodeGen/X86/fp16-complex.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -target-feature +avx512fp16 -o - | FileCheck %s --check-prefix=X86
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -o - | FileCheck %s --check-prefix=X86
 
 _Float16 _Complex add_half_rr(_Float16 a, _Float16 b) {
   // X86-LABEL: @add_half_rr(
Index: clang/test/CodeGen/X86/fp16-abi.c
===
--- clang/test/CodeGen/X86/fp16-abi.c
+++ clang/test/CodeGen/X86/fp16-abi.c
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -triple x86_64-linux -emit-llvm  -target-feature +avx512fp16 < %s | FileCheck %s --check-prefixes=CHECK,CHECK-C
+// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm  < %s | FileCheck %s --check-prefixes=CHECK,CHECK-C
 // RUN: %clang_cc1 -triple x86_64-linux -emit-llvm  -target-feature +avx512fp16 -x c++ -std=c++11 < %s | FileCheck %s --check-prefixes=CHECK,CHECK-CPP
+// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -x c++ -std=c++11 < %s | FileCheck %s --check-prefixes=CHECK,CHECK-CPP
 
 struct half1 {
   _Float16 a;
Index: clang/test/CodeGen/X86/Float16-arithmetic.c
===
--- /dev/null
+++ clang/test/CodeGen/X86/Float16-arithmetic.c
@@ -0,0 +1,73 @@
+// RUN: %clang_cc1 -triple  x86_64-unknown-unknown -emit-llvm  \
+// RUN: < %s  | FileCheck %s --check-prefixes=CHECK
+
+_Float16 add1(_Float16 a, _Float16 b) {
+  // CHECK-LABEL: define{{.*}} half @add1
+  // CHECK: alloca half
+ 

[PATCH] D114099: Enable `_Float16` type support on X86 without the avx512fp16 flag

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:676
 * SPIR
-* X86 (Only available under feature AVX512-FP16)
+* X86
 

Would something like SSE2 and up help understanding?


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

https://reviews.llvm.org/D114099

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

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

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

steakhal added a comment.

Thanks for the valuable feedback.

> D114105#inline-1089282 

Let me clarify my motivation, and why I'm interested in bitfields, conversions, 
and shift expressions.
Bitfields can be quite tricky.
It turns out doing anything useful with a bitfield will eventually result in an 
integral promotion, which mandates that the type must be 'int' if it fits, 
otherwise 'unsigned int' or no integral promotion happens otherwise.
conv.prom/5: 

> A prvalue for an integral bit-field ([class.bit]) can be converted to a 
> prvalue of type **int if int can represent all the values of the bit-field**; 
> otherwise, it can be converted to unsigned int if unsigned int can represent 
> all the values of the bit-field. If the bit-field is larger yet, no integral 
> promotion applies to it. If the bit-field has an enumerated type, it is 
> treated as any other value of that type for promotion purposes.

This behavior really surprises developers, but the context in which it happens 
is mostly binary operators.
At first glance, those reports seem to be false-positives but they are actually 
true-positives - and we are happy catching them.
A recommended workaround to this problem could be simply supplying the proper 
integer literal constant, by adding the `u` unsigned-suffix.
This will turn the `int` operand into `unsigned int`, at which point the binary 
operator will suddenly do the right thing and promote the bitfield expression 
to `unsigned int` as well, and the warning disappears.
This //workaround// works just fine for the regular binary operators, whose 
result type is the promoted type of the operands.

In the expr.compound  section, the shift 
expression is the only one whose return type depends only on the //left// 
operand, making my plotted //workaround// inapplicable for them.
Note that the //compound shift assignment operator// expression doesn't suffer 
from this behavior.
expr.ass/1: 

> [...] The result in all cases is a bit-field if the left operand is a 
> bit-field.

Given that the compiler chose `int` because that would be able to hold the 
value of the underlying bitfield without information loss, I think it makes 
sense to ignore these cases.
Of course, the shift operation might cause issues, but that's I think a 
different topic.
We might be able to do the extra mile and check if the other operand is a 
constant expression and the represented value would cause UB or unspecified 
behavior at the shift operation, but that's not really a conversion issue, what 
this check supposed to look for.

  x.id & 4u; // no-warning: promotion will force x.id to be unsigned
  x.id << 4u; // we still have a warning! the type of the LHS is unaffected by 
the type of the RHS
  (unsigned)x.id << 4; // looks weird, but does the right thing

That being said, I think the matcher should look through paren expressions but 
other than that I don't think there is an issue.
Alternatively, we could say that the users *must* use an explicit cast when 
loading from a bitfield in bitshifts to make their intention clear.
However, it doesn't feel natural and I cannot immediately see how frequently 
those reports would be true-positives.

WDYT? How should I proceed?




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:105
+   has(memberExpr(hasDeclaration(fieldDecl(isBitField(,
+  hasParent(
+  binaryOperator(anyOf(hasOperatorName("<<"), 
hasOperatorName(">>");

courbet wrote:
> What is specific about shifting operators ? What about `x+1` for example ? 
> You might have opened a pandora box here: you'll need to tech the check about 
> the semantics of all binary operations.
> 
> ```
> BinaryOperator  'int' '+'
>   |-ImplicitCastExpr  'int' 
>   | `-ImplicitCastExpr  'unsigned int' 
>   |   `-MemberExpr  'unsigned int' lvalue bitfield .id 
> 0x55e144e7eaa0
>   | `-DeclRefExpr  'SmallBitfield' lvalue Var 0x55e144e7eb18 
> 'x' 'SmallBitfield'
>   `-IntegerLiteral  'int' 1
> ```
> What is specific about shifting operators ? What about `x+1` for example ? 
> You might have opened a pandora box here: you'll need to tech the check about 
> the semantics of all binary operations.
> 
> ```
> BinaryOperator  'int' '+'
>   |-ImplicitCastExpr  'int' 
>   | `-ImplicitCastExpr  'unsigned int' 
>   |   `-MemberExpr  'unsigned int' lvalue bitfield .id 
> 0x55e144e7eaa0
>   | `-DeclRefExpr  'SmallBitfield' lvalue Var 0x55e144e7eb18 
> 'x' 'SmallBitfield'
>   `-IntegerLiteral  'int' 1
> ```




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

https://reviews.llvm.org/D114105

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https:/

[PATCH] D112091: libfuzzer: All building libfuzzer for ARM32

[Matt Morehouse via Phabricator via cfe-commits]

morehouse accepted this revision.
morehouse 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/D112091/new/

https://reviews.llvm.org/D112091

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

[PATCH] D114162: [X86][clang] Enable floating-point type for -mno-x87 option on 32-bits

[Phoebe Wang via Phabricator via cfe-commits]

pengfei created this revision.
pengfei added reviewers: asavonic, erichkeane, nickdesaulniers.
pengfei requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We should match GCC's behavior which allows floating-point type for -mno-x87 
option on 32-bits. https://godbolt.org/z/KrbhfWc9o

The previous block issues have partially been fixed by D112143 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114162

Files:
  clang/lib/Basic/Targets/X86.cpp
  clang/test/Sema/x86-no-x87.cpp
  clang/test/Sema/x86_64-no-x87.cpp

Index: clang/test/Sema/x86_64-no-x87.cpp
===
--- clang/test/Sema/x86_64-no-x87.cpp
+++ /dev/null
@@ -1,145 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -triple x86_64-linux-gnu -target-feature -x87
-// RUN: %clang_cc1 -fsyntax-only -verify %s -triple x86_64-linux-gnu -DNOERROR
-
-#ifdef NOERROR
-// expected-no-diagnostics
-#endif
-
-typedef long double long_double;
-
-// Declaration is fine, unless it is called or defined.
-double decl(long_double x, long_double y);
-
-template 
-T decl_ld_del(T);
-
-// No code is generated for deleted functions
-long_double decl_ld_del(long_double) = delete;
-double decl_ld_del(double) = delete;
-float decl_ld_del(float) = delete;
-
-#ifndef NOERROR
-// expected-error@+4{{'def' requires  'long_double' (aka 'long double') type support, but target 'x86_64-unknown-linux-gnu' does not support it}}
-// expected-note@+3{{'def' defined here}}
-// expected-note@+2{{'x' defined here}}
-#endif
-int def(long_double x) {
-#ifndef NOERROR
-// expected-error@+2{{'x' requires  'long_double' (aka 'long double') type support, but target 'x86_64-unknown-linux-gnu' does not support it}}
-#endif
-  return (int)x;
-}
-
-#ifndef NOERROR
-// expected-note@+3{{'ld_args' defined here}}
-// expected-note@+2{{'ld_args' defined here}}
-#endif
-int ld_args(long_double x, long_double y);
-
-int call1(float x, float y) {
-#ifndef NOERROR
-  // expected-error@+2 2{{'ld_args' requires  'long_double' (aka 'long double') type support, but target 'x86_64-unknown-linux-gnu' does not support it}}
-#endif
-  return ld_args(x, y);
-}
-
-#ifndef NOERROR
-// expected-note@+2{{'ld_ret' defined here}}
-#endif
-long_double ld_ret(double x, double y);
-
-int call2(float x, float y) {
-#ifndef NOERROR
-  // expected-error@+2{{'ld_ret' requires  'long_double' (aka 'long double') type support, but target 'x86_64-unknown-linux-gnu' does not support it}}
-#endif
-  return (int)ld_ret(x, y);
-}
-
-int binop(double x, double y) {
-#ifndef NOERROR
-  // expected-error@+2 2{{expression requires  'long_double' (aka 'long double') type support, but target 'x86_64-unknown-linux-gnu' does not support it}}
-#endif
-  double z = (long_double)x * (long_double)y;
-  return (int)z;
-}
-
-void assign1(long_double *ret, double x) {
-#ifndef NOERROR
-  // expected-error@+2{{expression requires  'long_double' (aka 'long double') type support, but target 'x86_64-unknown-linux-gnu' does not support it}}
-#endif
-  *ret = x;
-}
-
-struct st_long_double1 {
-#ifndef NOERROR
-  // expected-note@+2{{'ld' defined here}}
-#endif
-  long_double ld;
-};
-
-struct st_long_double2 {
-#ifndef NOERROR
-  // expected-note@+2{{'ld' defined here}}
-#endif
-  long_double ld;
-};
-
-struct st_long_double3 {
-#ifndef NOERROR
-  // expected-note@+2{{'ld' defined here}}
-#endif
-  long_double ld;
-};
-
-void assign2() {
-  struct st_long_double1 st;
-#ifndef NOERROR
-  // expected-error@+3{{expression requires  'long_double' (aka 'long double') type support, but target 'x86_64-unknown-linux-gnu' does not support it}}
-  // expected-error@+2{{'ld' requires  'long_double' (aka 'long double') type support, but target 'x86_64-unknown-linux-gnu' does not support it}}
-#endif
-  st.ld = 0.42;
-}
-
-void assign3() {
-  struct st_long_double2 st;
-#ifndef NOERROR
-  // expected-error@+3{{expression requires  'long_double' (aka 'long double') type support, but target 'x86_64-unknown-linux-gnu' does not support it}}
-  // expected-error@+2{{'ld' requires  'long_double' (aka 'long double') type support, but target 'x86_64-unknown-linux-gnu' does not support it}}
-#endif
-  st.ld = 42;
-}
-
-void assign4(double d) {
-  struct st_long_double3 st;
-#ifndef NOERROR
-  // expected-error@+3{{expression requires  'long_double' (aka 'long double') type support, but target 'x86_64-unknown-linux-gnu' does not support it}}
-  // expected-error@+2{{'ld' requires  'long_double' (aka 'long double') type support, but target 'x86_64-unknown-linux-gnu' does not support it}}
-#endif
-  st.ld = d;
-}
-
-void assign5() {
-  // unused variable declaration is fine
-  long_double ld = 0.42;
-}
-
-// Double and Float return type on x86_64 do not use x87 registers
-double d_ret1(float x) {
-  return 0.0;
-}
-
-double d_ret2(float x);
-
-int d_ret3(float x) {
-  return (int)d_ret2(x);
-}
-
-float f_ret1(float x) 

[PATCH] D98895: [X86][clang] Disable long double type for -mno-x87 option

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

@asavonic , I put a patch D114162  to enable 
it on 32-bits. Could you help to review it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98895

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

[PATCH] D114163: Use VersionTuple for parsing versions in Triple. This makes it possible to distinguish between "16" and "16.0" after parsing, which previously was not possible. See also https://gith

[James Farrell via Phabricator via cfe-commits]

jamesfarrell created this revision.
Herald added subscribers: danielkiss, dexonsmith, pengfei, hiraditya.
jamesfarrell requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114163

Files:
  clang/lib/ARCMigrate/ARCMT.cpp
  clang/lib/Basic/Targets/OSTargets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Driver/ToolChains/NetBSD.cpp
  clang/test/Sema/attr-availability-android.c
  clang/test/Sema/attr-availability.c
  clang/test/Sema/availability-guard-format.mm
  clang/test/SemaObjC/attr-availability.m
  clang/test/SemaObjC/property-deprecated-warning.m
  clang/test/SemaObjC/unguarded-availability-maccatalyst.m
  clang/test/SemaObjC/unguarded-availability.m
  llvm/include/llvm/ADT/Triple.h
  llvm/lib/Analysis/TargetLibraryInfo.cpp
  llvm/lib/MC/MCStreamer.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/unittests/ADT/TripleTest.cpp

Index: llvm/unittests/ADT/TripleTest.cpp
===
--- llvm/unittests/ADT/TripleTest.cpp
+++ llvm/unittests/ADT/TripleTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "llvm/ADT/Triple.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/VersionTuple.h"
 #include "gtest/gtest.h"
 
@@ -117,6 +118,18 @@
   EXPECT_EQ(Triple::Linux, T.getOS());
   EXPECT_EQ(Triple::MuslX32, T.getEnvironment());
 
+  T = Triple("arm-unknown-linux-android16");
+  EXPECT_EQ(Triple::arm, T.getArch());
+  EXPECT_EQ(Triple::UnknownVendor, T.getVendor());
+  EXPECT_EQ(Triple::Linux, T.getOS());
+  EXPECT_EQ(Triple::Android, T.getEnvironment());
+
+  T = Triple("aarch64-unknown-linux-android21");
+  EXPECT_EQ(Triple::aarch64, T.getArch());
+  EXPECT_EQ(Triple::UnknownVendor, T.getVendor());
+  EXPECT_EQ(Triple::Linux, T.getOS());
+  EXPECT_EQ(Triple::Android, T.getEnvironment());
+
   // PS4 has two spellings for the vendor.
   T = Triple("x86_64-scei-ps4");
   EXPECT_EQ(Triple::x86_64, T.getArch());
@@ -1261,7 +1274,7 @@
 
 TEST(TripleTest, getOSVersion) {
   Triple T;
-  unsigned Major, Minor, Micro;
+  VersionTuple Version;
 
   T = Triple("i386-apple-darwin9");
   EXPECT_TRUE(T.isMacOSX());
@@ -1269,14 +1282,10 @@
   EXPECT_FALSE(T.isArch16Bit());
   EXPECT_TRUE(T.isArch32Bit());
   EXPECT_FALSE(T.isArch64Bit());
-  T.getMacOSXVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)10, Major);
-  EXPECT_EQ((unsigned)5, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
-  T.getiOSVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)5, Major);
-  EXPECT_EQ((unsigned)0, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
+  T.getMacOSXVersion(Version);
+  EXPECT_EQ(VersionTuple(10, 5), Version);
+  Version = T.getiOSVersion();
+  EXPECT_EQ(VersionTuple(5), Version);
 
   T = Triple("x86_64-apple-darwin9");
   EXPECT_TRUE(T.isMacOSX());
@@ -1284,14 +1293,10 @@
   EXPECT_FALSE(T.isArch16Bit());
   EXPECT_FALSE(T.isArch32Bit());
   EXPECT_TRUE(T.isArch64Bit());
-  T.getMacOSXVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)10, Major);
-  EXPECT_EQ((unsigned)5, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
-  T.getiOSVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)5, Major);
-  EXPECT_EQ((unsigned)0, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
+  T.getMacOSXVersion(Version);
+  EXPECT_EQ(VersionTuple(10, 5), Version);
+  Version = T.getiOSVersion();
+  EXPECT_EQ(VersionTuple(5), Version);
 
   T = Triple("x86_64-apple-macosx");
   EXPECT_TRUE(T.isMacOSX());
@@ -1299,14 +1304,10 @@
   EXPECT_FALSE(T.isArch16Bit());
   EXPECT_FALSE(T.isArch32Bit());
   EXPECT_TRUE(T.isArch64Bit());
-  T.getMacOSXVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)10, Major);
-  EXPECT_EQ((unsigned)4, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
-  T.getiOSVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)5, Major);
-  EXPECT_EQ((unsigned)0, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
+  T.getMacOSXVersion(Version);
+  EXPECT_EQ(VersionTuple(10, 4), Version);
+  Version = T.getiOSVersion();
+  EXPECT_EQ(VersionTuple(5), Version);
 
   T = Triple("x86_64-apple-macosx10.7");
   EXPECT_TRUE(T.isMacOSX());
@@ -1314,14 +1315,10 @@
   EXPECT_FALSE(T.isArch16Bit());
   EXPECT_FALSE(T.isArch32Bit());
   EXPECT_TRUE(T.isArch64Bit());
-  T.getMacOSXVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)10, Major);
-  EXPECT_EQ((unsigned)7, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
-  T.getiOSVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)5, Major);
-  EXPECT_EQ((unsigned)0, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
+  T.getMacOSXVersion(Version);
+  EXPECT_EQ(VersionTuple(10, 7), Version);
+  Version = T.getiOSVersio

[PATCH] D114105: [clang-tidy] Ignore narrowing conversions in case of bitfields

[Clement Courbet via Phabricator via cfe-commits]

courbet added a comment.

Thanks for the details, this explains the motivation well. I think the key 
point is the combination of:

> It turns out doing anything useful with a bitfield will eventually result in 
> an integral promotion, which mandates that the type must be 'int' if it fits
> conv.prom/5: 
>
>> A prvalue for an integral bit-field ([class.bit]) can be converted to a 
>> prvalue of type **int if int can represent all the values of the 
>> bit-field**; otherwise, it can be converted to unsigned int if unsigned int 
>> can represent all the values of the bit-field. If the bit-field is larger 
>> yet, no integral promotion applies to it. If the bit-field has an enumerated 
>> type, it is treated as any other value of that type for promotion purposes.

and

> shift expression is the only one whose return type depends only on the 
> //left// operand

So adding that as a comment above `ShiftingWidenedBitfieldValue` would help.

Maybe also add a note in the comment that we are not interested in whether the 
shift itself might overflow.


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

https://reviews.llvm.org/D114105

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

[PATCH] D114099: Enable `_Float16` type support on X86 without the avx512fp16 flag

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 388189.

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

https://reviews.llvm.org/D114099

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/X86.cpp
  clang/test/CodeGen/X86/Float16-arithmetic.c
  clang/test/CodeGen/X86/avx512fp16-abi.c
  clang/test/CodeGen/X86/avx512fp16-complex.c
  clang/test/CodeGen/X86/fp16-abi.c
  clang/test/CodeGen/X86/fp16-complex.c
  clang/test/Sema/Float16.c
  clang/test/Sema/conversion-target-dep.c
  clang/test/SemaCXX/Float16.cpp

Index: clang/test/SemaCXX/Float16.cpp
===
--- clang/test/SemaCXX/Float16.cpp
+++ clang/test/SemaCXX/Float16.cpp
@@ -1,18 +1,9 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 
-#ifdef HAVE
 // expected-no-diagnostics
-#endif // HAVE
 
-#ifndef HAVE
-// expected-error@+2{{_Float16 is not supported on this target}}
-#endif // !HAVE
 _Float16 f;
-
-#ifndef HAVE
-// expected-error@+2{{invalid suffix 'F16' on floating constant}}
-#endif // !HAVE
 const auto g = 1.1F16;
Index: clang/test/Sema/conversion-target-dep.c
===
--- clang/test/Sema/conversion-target-dep.c
+++ clang/test/Sema/conversion-target-dep.c
@@ -6,7 +6,7 @@
 
 long double ld;
 double d;
-_Float16 f16; // x86-error {{_Float16 is not supported on this target}}
+_Float16 f16;
 
 int main() {
   ld = d; // x86-warning {{implicit conversion increases floating-point precision: 'double' to 'long double'}}
Index: clang/test/Sema/Float16.c
===
--- clang/test/Sema/Float16.c
+++ clang/test/Sema/Float16.c
@@ -1,18 +1,13 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc %s
-// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc -target-feature +avx512fp16 %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s -DHAVE
-// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s -DHAVE
+// RUN: %clang_cc1 -fsyntax-only -verify -triple x86_64-linux-pc -target-feature +avx512fp16 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple spir-unknown-unknown %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 
-#ifndef HAVE
-// expected-error@+2{{_Float16 is not supported on this target}}
-#endif // HAVE
 _Float16 f;
 
-#ifdef HAVE
 _Complex _Float16 a;
 void builtin_complex() {
   _Float16 a = 0;
   (void)__builtin_complex(a, a); // expected-error {{'_Complex _Float16' is invalid}}
 }
-#endif
Index: clang/test/CodeGen/X86/fp16-complex.c
===
--- clang/test/CodeGen/X86/fp16-complex.c
+++ clang/test/CodeGen/X86/fp16-complex.c
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -target-feature +avx512fp16 -o - | FileCheck %s --check-prefix=X86
+// RUN: %clang_cc1 %s -O0 -emit-llvm -triple x86_64-unknown-unknown -o - | FileCheck %s --check-prefix=X86
 
 _Float16 _Complex add_half_rr(_Float16 a, _Float16 b) {
   // X86-LABEL: @add_half_rr(
Index: clang/test/CodeGen/X86/fp16-abi.c
===
--- clang/test/CodeGen/X86/fp16-abi.c
+++ clang/test/CodeGen/X86/fp16-abi.c
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -triple x86_64-linux -emit-llvm  -target-feature +avx512fp16 < %s | FileCheck %s --check-prefixes=CHECK,CHECK-C
+// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm  < %s | FileCheck %s --check-prefixes=CHECK,CHECK-C
 // RUN: %clang_cc1 -triple x86_64-linux -emit-llvm  -target-feature +avx512fp16 -x c++ -std=c++11 < %s | FileCheck %s --check-prefixes=CHECK,CHECK-CPP
+// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -x c++ -std=c++11 < %s | FileCheck %s --check-prefixes=CHECK,CHECK-CPP
 
 struct half1 {
   _Float16 a;
Index: clang/test/CodeGen/X86/Float16-arithmetic.c
===
--- /dev/null
+++ clang/test/CodeGen/X86/Float16-arithmetic.c
@@ -0,0 +1,73 @@
+// RUN: %clang_cc1 -triple  x86_64-unknown-unknown -emit-llvm  \
+// RUN: < %s  | FileCheck %s --check-prefixes=CHECK
+
+_Float16 add1(_Float16 a, _Float16 b) {
+  // CHECK-LABEL: define{{.*}} half @add1
+  // CHECK: alloca half
+  // CHECK: alloca half
+  // CHECK: store h

[clang-tools-extra] 55a7931 - [clang][clangd] Improve signature help for variadic functions.

[Adam Czachorowski via cfe-commits]

Author: Adam Czachorowski
Date: 2021-11-18T15:50:47+01:00
New Revision: 55a79318c60d8a39329195f43bf43b89da9a638e

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

LOG: [clang][clangd] Improve signature help for variadic functions.

This covers both C-style variadic functions and template variadic w/
parameter packs.

Previously we would return no signatures when working with template
variadic functions once activeParameter reached the position of the
parameter pack (except when it was the only param, then we'd still
show it when no arguments were given). With this commit, we now show
signathure help correctly.

Additionally, this commit fixes the activeParameter value in LSP output
of clangd in the presence of variadic functions (both kinds). LSP does
not allow the activeParamter to be higher than the number of parameters
in the active signature. With "..." or parameter pack being just one
argument, for all but first argument passed to "..." we'd report
incorrect activeParameter value. Clients such as VSCode would then treat
it as 0, as suggested in the spec) and highlight the wrong parameter.

In the future, we should add support for per-signature activeParamter
value, which exists in LSP since 3.16.0. This is not part of this
commit.

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

Added: 
clang/test/CodeCompletion/variadic-template.cpp

Modified: 
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang/include/clang/Sema/Overload.h
clang/lib/Sema/SemaCodeComplete.cpp
clang/lib/Sema/SemaOverload.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CodeComplete.cpp 
b/clang-tools-extra/clangd/CodeComplete.cpp
index f033b5ec001d8..ac6c3b077d472 100644
--- a/clang-tools-extra/clangd/CodeComplete.cpp
+++ b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -872,6 +872,28 @@ struct ScoredSignature {
   SignatureQualitySignals Quality;
 };
 
+// Returns the index of the parameter matching argument number "Arg.
+// This is usually just "Arg", except for variadic functions/templates, where
+// "Arg" might be higher than the number of parameters. When that happens, we
+// assume the last parameter is variadic and assume all further args are
+// part of it.
+int paramIndexForArg(const CodeCompleteConsumer::OverloadCandidate &Candidate,
+ int Arg) {
+  int NumParams = 0;
+  if (const auto *F = Candidate.getFunction()) {
+NumParams = F->getNumParams();
+if (F->isVariadic())
+  ++NumParams;
+  } else if (auto *T = Candidate.getFunctionType()) {
+if (auto *Proto = T->getAs()) {
+  NumParams = Proto->getNumParams();
+  if (Proto->isVariadic())
+++NumParams;
+}
+  }
+  return std::min(Arg, std::max(NumParams - 1, 0));
+}
+
 class SignatureHelpCollector final : public CodeCompleteConsumer {
 public:
   SignatureHelpCollector(const clang::CodeCompleteOptions &CodeCompleteOpts,
@@ -902,7 +924,9 @@ class SignatureHelpCollector final : public 
CodeCompleteConsumer {
 SigHelp.activeSignature = 0;
 assert(CurrentArg <= (unsigned)std::numeric_limits::max() &&
"too many arguments");
+
 SigHelp.activeParameter = static_cast(CurrentArg);
+
 for (unsigned I = 0; I < NumCandidates; ++I) {
   OverloadCandidate Candidate = Candidates[I];
   // We want to avoid showing instantiated signatures, because they may be
@@ -912,6 +936,14 @@ class SignatureHelpCollector final : public 
CodeCompleteConsumer {
 if (auto *Pattern = Func->getTemplateInstantiationPattern())
   Candidate = OverloadCandidate(Pattern);
   }
+  if (static_cast(I) == SigHelp.activeSignature) {
+// The activeParameter in LSP relates to the activeSignature. There is
+// another, per-signature field, but we currently do not use it and not
+// all clients might support it.
+// FIXME: Add support for per-signature activeParameter field.
+SigHelp.activeParameter =
+paramIndexForArg(Candidate, SigHelp.activeParameter);
+  }
 
   const auto *CCS = Candidate.CreateSignatureString(
   CurrentArg, S, *Allocator, CCTUInfo, true);

diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp 
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 81dfdcb371fb4..5612a0b17fbba 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -2622,6 +2622,104 @@ TEST(SignatureHelpTest, ConstructorInitializeFields) {
   }
 }
 
+TEST(SignatureHelpTest, Variadic) {
+  const std::string Header = R"cpp(
+void fun(int x, ...) {}
+void test() {)cpp";
+  const std::string ExpectedSig = "fun([[int x]], [[...]]) -> vo

[PATCH] D111318: [clang][clangd] Improve signature help for variadic functions.

[Adam Czachorowski via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG55a79318c60d: [clang][clangd] Improve signature help for 
variadic functions. (authored by adamcz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111318

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/include/clang/Sema/Overload.h
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CodeCompletion/variadic-template.cpp

Index: clang/test/CodeCompletion/variadic-template.cpp
===
--- /dev/null
+++ clang/test/CodeCompletion/variadic-template.cpp
@@ -0,0 +1,18 @@
+template 
+void fun(T x, Args... args) {}
+
+void f() {
+  fun(1, 2, 3, 4);
+  // The results are quite awkward here, but it's the best we can do for now.
+  // Tools, including clangd, can unexpand "args" when showing this to the user.
+  // The important thing is that we provide OVERLOAD signature in all those cases.
+  //
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:7 %s -o - | FileCheck --check-prefix=CHECK-1 %s
+  // CHECK-1: OVERLOAD: [#void#]fun(<#T x#>, Args args...)
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:10 %s -o - | FileCheck --check-prefix=CHECK-2 %s
+  // CHECK-2: OVERLOAD: [#void#]fun(int x)
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:13 %s -o - | FileCheck --check-prefix=CHECK-3 %s
+  // CHECK-3: OVERLOAD: [#void#]fun(int x, int args)
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:16 %s -o - | FileCheck --check-prefix=CHECK-4 %s
+  // CHECK-4: OVERLOAD: [#void#]fun(int x, int args, int args)
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -6456,7 +6456,8 @@
   // parameters is viable only if it has an ellipsis in its parameter
   // list (8.3.5).
   if (TooManyArguments(NumParams, Args.size(), PartialOverloading) &&
-  !Proto->isVariadic()) {
+  !Proto->isVariadic() &&
+  shouldEnforceArgLimit(PartialOverloading, Function)) {
 Candidate.Viable = false;
 Candidate.FailureKind = ovl_fail_too_many_arguments;
 return;
@@ -6946,7 +6947,8 @@
   // parameters is viable only if it has an ellipsis in its parameter
   // list (8.3.5).
   if (TooManyArguments(NumParams, Args.size(), PartialOverloading) &&
-  !Proto->isVariadic()) {
+  !Proto->isVariadic() &&
+  shouldEnforceArgLimit(PartialOverloading, Method)) {
 Candidate.Viable = false;
 Candidate.FailureKind = ovl_fail_too_many_arguments;
 return;
@@ -15242,3 +15244,21 @@
 FunctionDecl *Fn) {
   return FixOverloadedFunctionReference(E.get(), Found, Fn);
 }
+
+bool clang::shouldEnforceArgLimit(bool PartialOverloading,
+  FunctionDecl *Function) {
+  if (!PartialOverloading || !Function)
+return true;
+  if (Function->isVariadic())
+return false;
+  if (const auto *Proto =
+  dyn_cast(Function->getFunctionType()))
+if (Proto->isTemplateVariadic())
+  return false;
+  if (auto *Pattern = Function->getTemplateInstantiationPattern())
+if (const auto *Proto =
+dyn_cast(Pattern->getFunctionType()))
+  if (Proto->isTemplateVariadic())
+return false;
+  return true;
+}
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -5818,7 +5818,8 @@
 if (Candidate.Function) {
   if (Candidate.Function->isDeleted())
 continue;
-  if (!Candidate.Function->isVariadic() &&
+  if (shouldEnforceArgLimit(/*PartialOverloading=*/true,
+Candidate.Function) &&
   Candidate.Function->getNumParams() <= ArgSize &&
   // Having zero args is annoying, normally we don't surface a function
   // with 2 params, if you already have 2 params, because you are
Index: clang/include/clang/Sema/Overload.h
===
--- clang/include/clang/Sema/Overload.h
+++ clang/include/clang/Sema/Overload.h
@@ -1204,6 +1204,20 @@
 return Info;
   }
 
+  // Returns false if signature help is relevant despite number of arguments
+  // exceeding parameters. Specifically, it returns false when
+  // PartialOverloading is true and one of the following:
+  // * Function is variadic
+  // * Function is template variadic
+  // * Function is an instantiation of template variadic function
+  // The last case may seem strange. The idea is that if we added one more
+  // argument, we'd end up with a function similar to Function. Since, in the
+  // context of signatu

[PATCH] D113049: [AIX] Disable tests that fail because of no 64-bit XCOFF object file support

[Jinsong Ji via Phabricator via cfe-commits]

jsji added a comment.

Can we use `UNSUPPORTED` instead of `XFAIL` since it is unsupported?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113049

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

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Do we have a comprehensive list of non-inclusive terms and their inclusive 
correspondent somewhere available?
I mean `master` -> `main`, `white list` -> `inclusive list`, `sanity` -> 
`validation`, ...
I'd assume that we go through that list, and that could give me a clue of how 
many such patches to expect in the future.

Also, I was wondering that in list perhaps we could provide why we consider a 
term non-inclusive. Maybe it is just me but why is `sanity` considered 
non-inclusive?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114025

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

[PATCH] D113049: [AIX] Disable tests that fail because of no 64-bit XCOFF object file support

[Jake Egan via Phabricator via cfe-commits]

Jake-Egan added a comment.

In D113049#3140160 , @jsji wrote:

> Can we use `UNSUPPORTED` instead of `XFAIL` since it is unsupported?

If 64-bit XCOFF object files will be supported in the future, I think it makes 
more sense to use `XFAIL` because these tests will still be run and pass after 
implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113049

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

[PATCH] D113049: [AIX] Disable tests that fail because of no 64-bit XCOFF object file support

[Kai Luo via Phabricator via cfe-commits]

lkail added a comment.

Is there any way to filter these tests out on AIX in `lit.local.cfg`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113049

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

[PATCH] D113538: OpenMP: Start calling setTargetAttributes for generated kernels

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a subscriber: estewart08.
JonChesterfield added a comment.

Is the behaviour change in the above comments intentional? Pointed out by 
@estewart08




Comment at: clang/lib/CodeGen/TargetInfo.cpp:9208
-   FD->hasAttr();
-  if ((IsOpenCLKernel || IsHIPKernel) &&
-  (M.getTriple().getOS() == llvm::Triple::AMDHSA))

Here, we skip the amdgpu-implicitarg-num-bytes and uniform-work-group-size 
assignments if FD is nullptr



Comment at: clang/lib/CodeGen/TargetInfo.cpp:9288
+
+  const bool IsOpenMP = M.getLangOpts().OpenMP && !FD;
+  if ((IsOpenCLKernel || IsHIPKernel || IsOpenMP) &&

Here, we do the amdgpu-implicitarg-num-bytes and uniform-work-group-size 
assignments regardless of whether FD is true or not


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

https://reviews.llvm.org/D113538

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

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA added a comment.

In D114025#3140161 , @martong wrote:

> Do we have a comprehensive list of non-inclusive terms and their inclusive 
> correspondent somewhere available?
> I mean `master` -> `main`, `white list` -> `inclusive list`, `sanity` -> 
> `validation`, ...
> I'd assume that we go through that list, and that could give me a clue of how 
> many such patches to expect in the future.
>
> Also, I was wondering that in list perhaps we could provide why we consider a 
> term non-inclusive. Maybe it is just me but why is `sanity` considered 
> non-inclusive?

I don't know of an official LLVM list, there could be one but I am not certain.

These two sources have been a good place to start for me: 
https://tools.ietf.org/id/draft-knodel-terminology-00.html
https://developers.google.com/style/inclusive-documentation

I have been working on finding replacements for `blacklist`, `whitelist`, and 
`sanity` in Clang/LLVM and @quinnp has been working on replacing `master` when 
possible. That's the extent of our list.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114025

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

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

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

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

Peanut gallery says: I'd recommend //not// taking this particular patch; it 
seems much less "obvious" than the whitelist/inclusionlist type of patch. 
Whereas "whitelist" is just a made-up buzzword that can easily be replaced with 
a different made-up buzzword, "sanity check" is not at all the same as 
"validation test." In many cases, I think "sanity-check" could be reasonably 
replaced with "smoke-test," but (1) this PR doesn't do that, and (2) the phrase 
"smoke-test" is probably //harder// to understand, and (3) this PR currently 
touches many instances of "sanity/insanity/sane/insane" in code comments which 
have nothing to do with testing or checking at all.

"We could do X, but that would be insane" does //not// mean "We could do X, but 
that would not pass validations." It means it would be //stupid//, 
//irrational//, //foolish for obvious reasons//, something along those lines.

I agree that "X is dumb/stupid/insane" is a bad code comment and should be 
improved. It is bad //not just// because it is un-PC but because it is vague 
and therefore not (directly) helpful to the reader. However, you cannot fix 
this kind of comment by just substituting some made-up reason like "it would 
fail validation," because a //lying// comment is //worse// than a vague comment.

Not all of the individual diffs in this PR are bad. But some of them are.




Comment at: clang/lib/Sema/SemaDeclCXX.cpp:9176
   // Don't check the implicit member of the anonymous union type.
-  // This is technically non-conformant, but sanity demands it.
+  // This is technically non-conformant, but validation tests demand it.
   return false;

This comment seems incorrectly translated.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:12260
   // even if hidden by ordinary names, *except* in a dependent context
-  // where it's important for the sanity of two-phase lookup.
+  // where it's important for the validation of two-phase lookup.
   if (!IsInstantiation)

This comment seems incorrectly translated.



Comment at: clang/lib/StaticAnalyzer/Core/Store.cpp:252-255
+  // Verify that we avoid doing the wrong thing in the face of
   // reinterpret_cast.
   if (!regionMatchesCXXRecordType(Derived, Cast->getSubExpr()->getType()))
 return UnknownVal();

This comment seems incorrectly translated. I interpret the writer's intent as, 
"We really don't ever expect to get here from a reinterpret_cast; but if we 
ever do, let's just return something plausible [so that we avoid doing anything 
insane in the following codepath]."

According to my interpretation, this would be better expressed as a simple 
`assert`, and you should file a bug if the assert ever gets hit.

Alternatively, this `if` is necessary for correctness, and the comment should 
simply say something like "We can get here from a reinterpret_cast; in that 
case, return UnknownVal so that [whatever the reason is]."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114025

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

[PATCH] D110549: [HIPSPV][1/4] Refactor HIP tool chain

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110549

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

[PATCH] D113538: OpenMP: Start calling setTargetAttributes for generated kernels

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:9288
+
+  const bool IsOpenMP = M.getLangOpts().OpenMP && !FD;
+  if ((IsOpenCLKernel || IsHIPKernel || IsOpenMP) &&

JonChesterfield wrote:
> Here, we do the amdgpu-implicitarg-num-bytes and uniform-work-group-size 
> assignments regardless of whether FD is true or not
Cancel that, there's a IsOpenMP = ...&& !FD here. Failed to follow the control 
flow.


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

https://reviews.llvm.org/D113538

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

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D114025#3140161 , @martong wrote:

> Do we have a comprehensive list of non-inclusive terms and their inclusive 
> correspondent somewhere available?
> I mean `master` -> `main`, `white list` -> `inclusive list`, `sanity` -> 
> `validation`, ...
> I'd assume that we go through that list, and that could give me a clue of how 
> many such patches to expect in the future.
>
> Also, I was wondering that in list perhaps we could provide why we consider a 
> term non-inclusive. Maybe it is just me but why is `sanity` considered 
> non-inclusive?

How dare you question our world's new overlords.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114025

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

[PATCH] D113049: [AIX] Disable tests that fail because of no 64-bit XCOFF object file support

[Jinsong Ji via Phabricator via cfe-commits]

jsji added a comment.

In D113049#3140165 , @Jake-Egan wrote:

> In D113049#3140160 , @jsji wrote:
>
>> Can we use `UNSUPPORTED` instead of `XFAIL` since it is unsupported?
>
> If 64-bit XCOFF object files will be supported in the future, I think it 
> makes more sense to use `XFAIL` because these tests will still be run and 
> pass after implementation.

We should remove the `UNSUPPORTED` when we enable the 64-bit XCOFF object file 
support . It is a waste of machine time to run them *NOW*, especially 
considering the number of these failing tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113049

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

[PATCH] D113049: [AIX] Disable tests that fail because of no 64-bit XCOFF object file support

[Jinsong Ji via Phabricator via cfe-commits]

jsji added a comment.

In D113049#3140174 , @lkail wrote:

> Is there any way to filter these tests out on AIX in `lit.local.cfg`?

Agree, I would prefer we do something similar to 
https://reviews.llvm.org/rG666accf283311c5110ae4e2e5e4c4b99078eed15#change-NFfZJdfkKBjR
 to exclude the unsupported files for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113049

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

[PATCH] D114163: Use VersionTuple for parsing versions in Triple. This makes it possible to distinguish between "16" and "16.0" after parsing, which previously was not possible.

[James Farrell via Phabricator via cfe-commits]

jamesfarrell updated this revision to Diff 388196.
jamesfarrell retitled this revision from "Use VersionTuple for parsing versions 
in Triple. This makes it possible to distinguish between "16" and "16.0" after 
parsing, which previously was not possible.See also 
https://github.com/android/ndk/issues/1455."; to "Use VersionTuple for parsing 
versions in Triple. This makes it possible to distinguish between "16" and 
"16.0" after parsing, which previously was not possible.".
jamesfarrell edited the summary of this revision.
jamesfarrell added a comment.

Cleanup Triple::getMacOSXVersion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114163

Files:
  clang/lib/ARCMigrate/ARCMT.cpp
  clang/lib/Basic/Targets/OSTargets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Driver/ToolChains/NetBSD.cpp
  clang/test/Sema/attr-availability-android.c
  clang/test/Sema/attr-availability.c
  clang/test/Sema/availability-guard-format.mm
  clang/test/SemaObjC/attr-availability.m
  clang/test/SemaObjC/property-deprecated-warning.m
  clang/test/SemaObjC/unguarded-availability-maccatalyst.m
  clang/test/SemaObjC/unguarded-availability.m
  llvm/include/llvm/ADT/Triple.h
  llvm/lib/Analysis/TargetLibraryInfo.cpp
  llvm/lib/MC/MCStreamer.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/unittests/ADT/TripleTest.cpp

Index: llvm/unittests/ADT/TripleTest.cpp
===
--- llvm/unittests/ADT/TripleTest.cpp
+++ llvm/unittests/ADT/TripleTest.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "llvm/ADT/Triple.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/VersionTuple.h"
 #include "gtest/gtest.h"
 
@@ -117,6 +118,18 @@
   EXPECT_EQ(Triple::Linux, T.getOS());
   EXPECT_EQ(Triple::MuslX32, T.getEnvironment());
 
+  T = Triple("arm-unknown-linux-android16");
+  EXPECT_EQ(Triple::arm, T.getArch());
+  EXPECT_EQ(Triple::UnknownVendor, T.getVendor());
+  EXPECT_EQ(Triple::Linux, T.getOS());
+  EXPECT_EQ(Triple::Android, T.getEnvironment());
+
+  T = Triple("aarch64-unknown-linux-android21");
+  EXPECT_EQ(Triple::aarch64, T.getArch());
+  EXPECT_EQ(Triple::UnknownVendor, T.getVendor());
+  EXPECT_EQ(Triple::Linux, T.getOS());
+  EXPECT_EQ(Triple::Android, T.getEnvironment());
+
   // PS4 has two spellings for the vendor.
   T = Triple("x86_64-scei-ps4");
   EXPECT_EQ(Triple::x86_64, T.getArch());
@@ -1261,7 +1274,7 @@
 
 TEST(TripleTest, getOSVersion) {
   Triple T;
-  unsigned Major, Minor, Micro;
+  VersionTuple Version;
 
   T = Triple("i386-apple-darwin9");
   EXPECT_TRUE(T.isMacOSX());
@@ -1269,14 +1282,10 @@
   EXPECT_FALSE(T.isArch16Bit());
   EXPECT_TRUE(T.isArch32Bit());
   EXPECT_FALSE(T.isArch64Bit());
-  T.getMacOSXVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)10, Major);
-  EXPECT_EQ((unsigned)5, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
-  T.getiOSVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)5, Major);
-  EXPECT_EQ((unsigned)0, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
+  T.getMacOSXVersion(Version);
+  EXPECT_EQ(VersionTuple(10, 5), Version);
+  Version = T.getiOSVersion();
+  EXPECT_EQ(VersionTuple(5), Version);
 
   T = Triple("x86_64-apple-darwin9");
   EXPECT_TRUE(T.isMacOSX());
@@ -1284,14 +1293,10 @@
   EXPECT_FALSE(T.isArch16Bit());
   EXPECT_FALSE(T.isArch32Bit());
   EXPECT_TRUE(T.isArch64Bit());
-  T.getMacOSXVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)10, Major);
-  EXPECT_EQ((unsigned)5, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
-  T.getiOSVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)5, Major);
-  EXPECT_EQ((unsigned)0, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
+  T.getMacOSXVersion(Version);
+  EXPECT_EQ(VersionTuple(10, 5), Version);
+  Version = T.getiOSVersion();
+  EXPECT_EQ(VersionTuple(5), Version);
 
   T = Triple("x86_64-apple-macosx");
   EXPECT_TRUE(T.isMacOSX());
@@ -1299,14 +1304,10 @@
   EXPECT_FALSE(T.isArch16Bit());
   EXPECT_FALSE(T.isArch32Bit());
   EXPECT_TRUE(T.isArch64Bit());
-  T.getMacOSXVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)10, Major);
-  EXPECT_EQ((unsigned)4, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
-  T.getiOSVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)5, Major);
-  EXPECT_EQ((unsigned)0, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
+  T.getMacOSXVersion(Version);
+  EXPECT_EQ(VersionTuple(10, 4), Version);
+  Version = T.getiOSVersion();
+  EXPECT_EQ(VersionTuple(5), Version);
 
   T = Triple("x86_64-apple-macosx10.7");
   EXPECT_TRUE(T.isMacOSX());
@@ -1314,14 +1315,10 @@
   EXPECT_FALSE(T.isArch16Bit());
   EXPECT_FALSE(T.isArch32Bit());
   EXP

[libunwind] 11982ee - [libunwind][AIX] Mark signal_frame.pass.cpp UNSUPPORTED on AIX

[Xing Xue via cfe-commits]

Author: Xing Xue
Date: 2021-11-18T10:24:58-05:00
New Revision: 11982eed2bc818fdbd525e3237a77a3efeb6a497

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

LOG: [libunwind][AIX] Mark signal_frame.pass.cpp UNSUPPORTED on AIX

Summary:
This patch marks libunwind test case signal_frame.pass.cpp as UNSUPPORTED on 
AIX because the AIX assembler does not support CFI directives.

Reviewed by: danielkiss, MaskRay, ldionne, libunwind

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

Added: 


Modified: 
libunwind/test/signal_frame.pass.cpp

Removed: 




diff  --git a/libunwind/test/signal_frame.pass.cpp 
b/libunwind/test/signal_frame.pass.cpp
index cfd4f484a18b7..513eef53bbc6c 100644
--- a/libunwind/test/signal_frame.pass.cpp
+++ b/libunwind/test/signal_frame.pass.cpp
@@ -17,6 +17,10 @@
 
 // UNSUPPORTED: libunwind-arm-ehabi
 
+// The AIX assembler does not support CFI directives, which
+// are necessary to run this test.
+// UNSUPPORTED: target=powerpc{{(64)?}}-ibm-aix
+
 #include 
 #include 
 #include 



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

[PATCH] D114163: Use VersionTuple for parsing versions in Triple. This makes it possible to distinguish between "16" and "16.0" after parsing, which previously was not possible.

[James Farrell via Phabricator via cfe-commits]

jamesfarrell updated this revision to Diff 388198.
jamesfarrell added a comment.

Remove an include that's no longer needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114163

Files:
  clang/lib/ARCMigrate/ARCMT.cpp
  clang/lib/Basic/Targets/OSTargets.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Driver/ToolChains/NetBSD.cpp
  clang/test/Sema/attr-availability-android.c
  clang/test/Sema/attr-availability.c
  clang/test/Sema/availability-guard-format.mm
  clang/test/SemaObjC/attr-availability.m
  clang/test/SemaObjC/property-deprecated-warning.m
  clang/test/SemaObjC/unguarded-availability-maccatalyst.m
  clang/test/SemaObjC/unguarded-availability.m
  llvm/include/llvm/ADT/Triple.h
  llvm/lib/Analysis/TargetLibraryInfo.cpp
  llvm/lib/MC/MCStreamer.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/unittests/ADT/TripleTest.cpp

Index: llvm/unittests/ADT/TripleTest.cpp
===
--- llvm/unittests/ADT/TripleTest.cpp
+++ llvm/unittests/ADT/TripleTest.cpp
@@ -117,6 +117,18 @@
   EXPECT_EQ(Triple::Linux, T.getOS());
   EXPECT_EQ(Triple::MuslX32, T.getEnvironment());
 
+  T = Triple("arm-unknown-linux-android16");
+  EXPECT_EQ(Triple::arm, T.getArch());
+  EXPECT_EQ(Triple::UnknownVendor, T.getVendor());
+  EXPECT_EQ(Triple::Linux, T.getOS());
+  EXPECT_EQ(Triple::Android, T.getEnvironment());
+
+  T = Triple("aarch64-unknown-linux-android21");
+  EXPECT_EQ(Triple::aarch64, T.getArch());
+  EXPECT_EQ(Triple::UnknownVendor, T.getVendor());
+  EXPECT_EQ(Triple::Linux, T.getOS());
+  EXPECT_EQ(Triple::Android, T.getEnvironment());
+
   // PS4 has two spellings for the vendor.
   T = Triple("x86_64-scei-ps4");
   EXPECT_EQ(Triple::x86_64, T.getArch());
@@ -1261,7 +1273,7 @@
 
 TEST(TripleTest, getOSVersion) {
   Triple T;
-  unsigned Major, Minor, Micro;
+  VersionTuple Version;
 
   T = Triple("i386-apple-darwin9");
   EXPECT_TRUE(T.isMacOSX());
@@ -1269,14 +1281,10 @@
   EXPECT_FALSE(T.isArch16Bit());
   EXPECT_TRUE(T.isArch32Bit());
   EXPECT_FALSE(T.isArch64Bit());
-  T.getMacOSXVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)10, Major);
-  EXPECT_EQ((unsigned)5, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
-  T.getiOSVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)5, Major);
-  EXPECT_EQ((unsigned)0, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
+  T.getMacOSXVersion(Version);
+  EXPECT_EQ(VersionTuple(10, 5), Version);
+  Version = T.getiOSVersion();
+  EXPECT_EQ(VersionTuple(5), Version);
 
   T = Triple("x86_64-apple-darwin9");
   EXPECT_TRUE(T.isMacOSX());
@@ -1284,14 +1292,10 @@
   EXPECT_FALSE(T.isArch16Bit());
   EXPECT_FALSE(T.isArch32Bit());
   EXPECT_TRUE(T.isArch64Bit());
-  T.getMacOSXVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)10, Major);
-  EXPECT_EQ((unsigned)5, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
-  T.getiOSVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)5, Major);
-  EXPECT_EQ((unsigned)0, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
+  T.getMacOSXVersion(Version);
+  EXPECT_EQ(VersionTuple(10, 5), Version);
+  Version = T.getiOSVersion();
+  EXPECT_EQ(VersionTuple(5), Version);
 
   T = Triple("x86_64-apple-macosx");
   EXPECT_TRUE(T.isMacOSX());
@@ -1299,14 +1303,10 @@
   EXPECT_FALSE(T.isArch16Bit());
   EXPECT_FALSE(T.isArch32Bit());
   EXPECT_TRUE(T.isArch64Bit());
-  T.getMacOSXVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)10, Major);
-  EXPECT_EQ((unsigned)4, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
-  T.getiOSVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)5, Major);
-  EXPECT_EQ((unsigned)0, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
+  T.getMacOSXVersion(Version);
+  EXPECT_EQ(VersionTuple(10, 4), Version);
+  Version = T.getiOSVersion();
+  EXPECT_EQ(VersionTuple(5), Version);
 
   T = Triple("x86_64-apple-macosx10.7");
   EXPECT_TRUE(T.isMacOSX());
@@ -1314,14 +1314,10 @@
   EXPECT_FALSE(T.isArch16Bit());
   EXPECT_FALSE(T.isArch32Bit());
   EXPECT_TRUE(T.isArch64Bit());
-  T.getMacOSXVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)10, Major);
-  EXPECT_EQ((unsigned)7, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
-  T.getiOSVersion(Major, Minor, Micro);
-  EXPECT_EQ((unsigned)5, Major);
-  EXPECT_EQ((unsigned)0, Minor);
-  EXPECT_EQ((unsigned)0, Micro);
+  T.getMacOSXVersion(Version);
+  EXPECT_EQ(VersionTuple(10, 7), Version);
+  Version = T.getiOSVersion();
+  EXPECT_EQ(VersionTuple(5), Version);
 
   T = Triple("x86_64-apple-macos11.0");
   EXPECT_TRUE(T.isMacOSX());
@@ -1329,10 +1325,8 @@
   EXPECT_FALSE(T.isArch16Bit());
   EXPECT_FALSE(T.isArch32Bit());
   EXPECT_TRUE(T.isArch64Bit());
-  T.getMacOSXVersion(Major, Minor, Mi

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D114025#3140161 , @martong wrote:

> Do we have a comprehensive list of non-inclusive terms and their inclusive 
> correspondent somewhere available?
> I mean `master` -> `main`, `white list` -> `inclusive list`, `sanity` -> 
> `validation`, ...
> I'd assume that we go through that list, and that could give me a clue of how 
> many such patches to expect in the future.
>
> Also, I was wondering that in list perhaps we could provide why we consider a 
> term non-inclusive. Maybe it is just me but why is `sanity` considered 
> non-inclusive?

https://gist.github.com/seanmhanson/fe370c2d8bd2b3228680e38899baf5cc has a 
pretty reasonable explanation about why `sanity` is problematic.

In D114025#3140204 , @lebedev.ri 
wrote:

> How dare you question our world's new overlords.

This is not a particularly constructive comment...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114025

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

[PATCH] D110618: [HIPSPV][2/4] Add HIPSPV tool chain

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

LGTM. I will leave to @tra about -nohipwrapperinc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110618

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

[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

LGTM. I will defer to @tra


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622

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

[PATCH] D113538: OpenMP: Start calling setTargetAttributes for generated kernels

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:9288
+
+  const bool IsOpenMP = M.getLangOpts().OpenMP && !FD;
+  if ((IsOpenCLKernel || IsHIPKernel || IsOpenMP) &&

JonChesterfield wrote:
> JonChesterfield wrote:
> > Here, we do the amdgpu-implicitarg-num-bytes and uniform-work-group-size 
> > assignments regardless of whether FD is true or not
> Cancel that, there's a IsOpenMP = ...&& !FD here. Failed to follow the 
> control flow.
Are we looking for `if (AMDGPU::isKernel(function.getCallingConv())` instead of 
looking at the function attributes? I don't think we want to annotate 
non-kernels with these things


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

https://reviews.llvm.org/D113538

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

[PATCH] D113538: OpenMP: Start calling setTargetAttributes for generated kernels

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:9288
+
+  const bool IsOpenMP = M.getLangOpts().OpenMP && !FD;
+  if ((IsOpenCLKernel || IsHIPKernel || IsOpenMP) &&

JonChesterfield wrote:
> JonChesterfield wrote:
> > JonChesterfield wrote:
> > > Here, we do the amdgpu-implicitarg-num-bytes and uniform-work-group-size 
> > > assignments regardless of whether FD is true or not
> > Cancel that, there's a IsOpenMP = ...&& !FD here. Failed to follow the 
> > control flow.
> Are we looking for `if (AMDGPU::isKernel(function.getCallingConv())` instead 
> of looking at the function attributes? I don't think we want to annotate 
> non-kernels with these things
!FD seems to always be true for openmp kernels because there's no associated 
function. This isn't looking at the IR function calling convention but I 
thought about switching to check that instead, but that's a separate change. 


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

https://reviews.llvm.org/D113538

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

[clang] 26f5643 - [Clang][SVE] Properly enable/disable dependant SVE target features based upon +(no)sve.* options

[Bradley Smith via cfe-commits]

Author: Bradley Smith
Date: 2021-11-18T15:52:28Z
New Revision: 26f56438e3dab44cea4c8f16d4cb16e9424b02c6

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

LOG: [Clang][SVE] Properly enable/disable dependant SVE target features based 
upon +(no)sve.* options

Co-authored-by: Graham Hunter 

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

Added: 
clang/test/Driver/aarch64-implied-sve-features.c

Modified: 
clang/lib/Driver/ToolChains/Arch/AArch64.cpp
clang/test/Driver/aarch64-cpus.c
llvm/include/llvm/Support/AArch64TargetParser.def
llvm/unittests/Support/TargetParserTest.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp 
b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
index b43edbe1b080b..0b60d097b9ca3 100644
--- a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -79,6 +79,25 @@ static bool DecodeAArch64Features(const Driver &D, StringRef 
text,
 else
   return false;
 
+if (Feature == "sve2")
+  Features.push_back("+sve");
+else if (Feature == "sve2-bitperm" || Feature == "sve2-sha3" ||
+ Feature == "sve2-aes" || Feature == "sve2-sm4") {
+  Features.push_back("+sve");
+  Features.push_back("+sve2");
+} else if (Feature == "nosve") {
+  Features.push_back("-sve2");
+  Features.push_back("-sve2-bitperm");
+  Features.push_back("-sve2-sha3");
+  Features.push_back("-sve2-aes");
+  Features.push_back("-sve2-sm4");
+} else if (Feature == "nosve2") {
+  Features.push_back("-sve2-bitperm");
+  Features.push_back("-sve2-sha3");
+  Features.push_back("-sve2-aes");
+  Features.push_back("-sve2-sm4");
+}
+
 // +sve implies +f32mm if the base architecture is v8.6A, v8.7A, v9.1A or
 // v9.2A. It isn't the case in general that sve implies both f64mm and 
f32mm
 if ((ArchKind == llvm::AArch64::ArchKind::ARMV8_6A ||
@@ -130,8 +149,20 @@ getAArch64ArchFeaturesFromMarch(const Driver &D, StringRef 
March,
 
   llvm::AArch64::ArchKind ArchKind = llvm::AArch64::parseArch(Split.first);
   if (ArchKind == llvm::AArch64::ArchKind::INVALID ||
-  !llvm::AArch64::getArchFeatures(ArchKind, Features) ||
-  (Split.second.size() &&
+  !llvm::AArch64::getArchFeatures(ArchKind, Features))
+return false;
+
+  // Enable SVE2 by default on Armv9-A.
+  // It can still be disabled if +nosve2 is present.
+  // We must do this early so that DecodeAArch64Features has the correct state
+  if ((ArchKind == llvm::AArch64::ArchKind::ARMV9A ||
+   ArchKind == llvm::AArch64::ArchKind::ARMV9_1A ||
+   ArchKind == llvm::AArch64::ArchKind::ARMV9_2A)) {
+Features.push_back("+sve");
+Features.push_back("+sve2");
+  }
+
+  if ((Split.second.size() &&
!DecodeAArch64Features(D, Split.second, Features, ArchKind)))
 return false;
 
@@ -419,14 +450,6 @@ void aarch64::getAArch64TargetFeatures(const Driver &D,
   if (Pos != std::end(Features))
 Pos = Features.insert(std::next(Pos), {"+i8mm", "+bf16"});
 
-  // Enable SVE2 by default on Armv9-A.
-  // It can still be disabled if +nosve2 is present.
-  const char *SVE2Archs[] = {"+v9a", "+v9.1a", "+v9.2a"};
-  Pos = std::find_first_of(Features.begin(), Features.end(),
-   std::begin(SVE2Archs), std::end(SVE2Archs));
-  if (Pos != Features.end())
-Features.insert(++Pos, "+sve2");
-
   if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
options::OPT_munaligned_access)) {
 if (A->getOption().matches(options::OPT_mno_unaligned_access))

diff  --git a/clang/test/Driver/aarch64-cpus.c 
b/clang/test/Driver/aarch64-cpus.c
index 89cfb7e99a57e..4a377df99f925 100644
--- a/clang/test/Driver/aarch64-cpus.c
+++ b/clang/test/Driver/aarch64-cpus.c
@@ -809,7 +809,7 @@
 // RUN: %clang -target aarch64 -mlittle-endian -march=armv9-a -### -c %s 2>&1 
| FileCheck -check-prefix=GENERICV9A %s
 // RUN: %clang -target aarch64_be -mlittle-endian -march=armv9a -### -c %s 
2>&1 | FileCheck -check-prefix=GENERICV9A %s
 // RUN: %clang -target aarch64_be -mlittle-endian -march=armv9-a -### -c %s 
2>&1 | FileCheck -check-prefix=GENERICV9A %s
-// GENERICV9A: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic" 
"-target-feature" "+neon" "-target-feature" "+v9a" "-target-feature" "+sve2"
+// GENERICV9A: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-cpu" "generic" 
"-target-feature" "+neon" "-target-feature" "+v9a" "-target-feature" "+sve" 
"-target-feature" "+sve2"
 
 // SVE2 is enabled by default on Armv9-A but it can be disabled
 // RUN: %clang -target aarch64 -march=armv9a+nosve2 -### -c %s 2>&1 | 
FileCheck -check-prefix=GENERICV9A-NOSVE2 %s
@@ -818,7 +818,7 @@
 // RUN: %clang -target aarch64 

[PATCH] D113776: [Clang][SVE] Properly enable/disable dependant SVE target features based upon +(no)sve.* options

[Bradley Smith 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 rG26f56438e3da: [Clang][SVE] Properly enable/disable dependant 
SVE target features based upon +… (authored by bsmith).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113776

Files:
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/test/Driver/aarch64-cpus.c
  clang/test/Driver/aarch64-implied-sve-features.c
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -910,11 +910,11 @@
  AArch64::AEK_SIMD | AArch64::AEK_RAS |
  AArch64::AEK_LSE | AArch64::AEK_RDM |
  AArch64::AEK_RCPC | AArch64::AEK_DOTPROD |
- AArch64::AEK_SVE2 | AArch64::AEK_BF16 |
- AArch64::AEK_I8MM | AArch64::AEK_SVE2BITPERM |
- AArch64::AEK_PAUTH | AArch64::AEK_MTE |
- AArch64::AEK_SSBS | AArch64::AEK_FP16FML |
- AArch64::AEK_SB,
+ AArch64::AEK_BF16 | AArch64::AEK_I8MM |
+ AArch64::AEK_SVE | AArch64::AEK_SVE2 |
+ AArch64::AEK_SVE2BITPERM | AArch64::AEK_PAUTH |
+ AArch64::AEK_MTE | AArch64::AEK_SSBS |
+ AArch64::AEK_FP16FML | AArch64::AEK_SB,
  "9-A"),
 ARMCPUTestParams("cortex-a57", "armv8-a", "crypto-neon-fp-armv8",
  AArch64::AEK_CRC | AArch64::AEK_CRYPTO |
@@ -1030,12 +1030,12 @@
  AArch64::AEK_CRC | AArch64::AEK_FP |
  AArch64::AEK_SIMD | AArch64::AEK_RAS |
  AArch64::AEK_LSE | AArch64::AEK_RDM |
- AArch64::AEK_RCPC | AArch64::AEK_SVE2 |
- AArch64::AEK_DOTPROD | AArch64::AEK_MTE |
- AArch64::AEK_PAUTH | AArch64::AEK_I8MM |
- AArch64::AEK_BF16 | AArch64::AEK_SVE2BITPERM |
- AArch64::AEK_SSBS | AArch64::AEK_SB |
- AArch64::AEK_FP16FML,
+ AArch64::AEK_RCPC | AArch64::AEK_DOTPROD |
+ AArch64::AEK_MTE | AArch64::AEK_PAUTH |
+ AArch64::AEK_I8MM | AArch64::AEK_BF16 |
+ AArch64::AEK_SVE | AArch64::AEK_SVE2 |
+ AArch64::AEK_SVE2BITPERM | AArch64::AEK_SSBS |
+ AArch64::AEK_SB | AArch64::AEK_FP16FML,
  "9-A"),
 ARMCPUTestParams("cyclone", "armv8-a", "crypto-neon-fp-armv8",
  AArch64::AEK_NONE | AArch64::AEK_CRYPTO |
Index: llvm/include/llvm/Support/AArch64TargetParser.def
===
--- llvm/include/llvm/Support/AArch64TargetParser.def
+++ llvm/include/llvm/Support/AArch64TargetParser.def
@@ -145,9 +145,10 @@
 AARCH64_CPU_NAME("cortex-a55", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,
  (AArch64::AEK_FP16 | AArch64::AEK_DOTPROD | AArch64::AEK_RCPC))
 AARCH64_CPU_NAME("cortex-a510", ARMV9A, FK_NEON_FP_ARMV8, false,
- (AArch64::AEK_BF16 | AArch64::AEK_I8MM | AArch64::AEK_SVE2BITPERM |
+ (AArch64::AEK_BF16 | AArch64::AEK_I8MM | AArch64::AEK_SB |
   AArch64::AEK_PAUTH | AArch64::AEK_MTE | AArch64::AEK_SSBS |
-  AArch64::AEK_SB | AArch64::AEK_FP16FML))
+  AArch64::AEK_SVE | AArch64::AEK_SVE2 | AArch64::AEK_SVE2BITPERM |
+  AArch64::AEK_FP16FML))
 AARCH64_CPU_NAME("cortex-a57", ARMV8A, FK_CRYPTO_NEON_FP_ARMV8, false,
  (AArch64::AEK_CRC))
 AARCH64_CPU_NAME("cortex-a65", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,
@@ -188,8 +189,9 @@
   AArch64::AEK_SSBS))
 AARCH64_CPU_NAME("cortex-x2", ARMV9A, FK_NEON_FP_ARMV8, false,
  (AArch64::AEK_MTE | AArch64::AEK_BF16 | AArch64::AEK_I8MM |
-  AArch64::AEK_PAUTH | AArch64::AEK_SSBS | AArch64::AEK_SVE2BITPERM |
-  AArch64::AEK_SB | AArch64::AEK_FP16FML))
+  AArch64::AEK_PAUTH | AArch64::AEK_SSBS | AArch64::AEK_SB |
+  AArch64::AEK_SVE | AArch64::AEK_SVE2 | AArch64::AEK_SVE2BITPERM |
+  AArch64::AEK_FP16FML))
 AARCH64_CPU_NAME("neoverse-e1", ARMV8_2A, FK_CRYPTO_NEON_FP_ARMV8, false,
  (AArch64::AEK_DOTPROD | AArch64::AEK_FP16 | AArch64::AEK_RAS |
 

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Gentle ping.


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

https://reviews.llvm.org/D99797

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

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA updated this revision to Diff 388207.
ZarkoCA edited the summary of this revision.
ZarkoCA added a comment.

- Reword comments based on suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114025

Files:
  clang/include/clang/AST/Redeclarable.h
  clang/include/clang/Analysis/CFG.h
  clang/include/clang/CodeGen/CGFunctionInfo.h
  clang/include/clang/Sema/Lookup.h
  clang/lib/Analysis/BodyFarm.cpp
  clang/lib/Analysis/RetainSummaryManager.cpp
  clang/lib/Basic/DiagnosticIDs.cpp
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/lib/Tooling/Syntax/Tree.cpp

Index: clang/lib/Tooling/Syntax/Tree.cpp
===
--- clang/lib/Tooling/Syntax/Tree.cpp
+++ clang/lib/Tooling/Syntax/Tree.cpp
@@ -126,7 +126,7 @@
   for (auto *N = New; N; N = N->NextSibling) {
 assert(N->Parent == nullptr);
 assert(N->getRole() != NodeRole::Detached && "Roles must be set");
-// FIXME: sanity-check the role.
+// FIXME: validate the role.
   }
 
   auto Reachable = [](Node *From, Node *N) {
Index: clang/lib/StaticAnalyzer/Core/Store.cpp
===
--- clang/lib/StaticAnalyzer/Core/Store.cpp
+++ clang/lib/StaticAnalyzer/Core/Store.cpp
@@ -249,7 +249,7 @@
 }
 
 SVal StoreManager::evalDerivedToBase(SVal Derived, const CastExpr *Cast) {
-  // Sanity check to avoid doing the wrong thing in the face of
+  // Early return to avoid doing the wrong thing in the face of
   // reinterpret_cast.
   if (!regionMatchesCXXRecordType(Derived, Cast->getSubExpr()->getType()))
 return UnknownVal();
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -326,8 +326,8 @@
 }
 Result = InitWithAdjustments;
   } else {
-// We need to create a region no matter what. For sanity, make sure we don't
-// try to stuff a Loc into a non-pointer temporary region.
+// We need to create a region no matter what. Make sure we don't try to
+// stuff a Loc into a non-pointer temporary region.
 assert(!InitValWithAdjustments.getAs() ||
Loc::isLocType(Result->getType()) ||
Result->getType()->isMemberPointerType());
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1670,7 +1670,7 @@
   if (isUnderconstrained(PrevN)) {
 IsSatisfied = true;
 
-// As a sanity check, make sure that the negation of the constraint
+// As a validation check, make sure that the negation of the constraint
 // was infeasible in the current state.  If it is feasible, we somehow
 // missed the transition point.
 assert(!isUnderconstrained(N));
Index: clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -182,8 +182,7 @@
   ProgramStateRef state = C.getState();
 
   if (CE->getNumArgs() < MinArgCount) {
-// The frontend should issue a warning for this case, so this is a sanity
-// check.
+// The frontend should issue a warning for this case, check for that here.
 return;
   } else if (CE->getNumArgs() == MaxArgCount) {
 const Expr *Arg = CE->getArg(CreateModeArgIndex);
@@ -366,7 +365,7 @@
  const unsigned numArgs,
  const unsigned sizeArg,
  const char *fn) const {
-  // Sanity check for the correct number of arguments
+  // Check for the correct number of arguments.
   if (CE->getNumArgs() != numArgs)
 return;
 
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibra

[PATCH] D111863: [libunwind] Add an interface for dynamic .eh_frame registration

[Peter S. Housel via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbab39816085d: [libunwind] Add an interface for dynamic 
.eh_frame registration (authored by housel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111863

Files:
  libunwind/src/DwarfParser.hpp
  libunwind/src/libunwind.cpp
  libunwind/src/libunwind_ext.h

Index: libunwind/src/libunwind_ext.h
===
--- libunwind/src/libunwind_ext.h
+++ libunwind/src/libunwind_ext.h
@@ -51,6 +51,9 @@
 extern void __unw_add_dynamic_fde(unw_word_t fde);
 extern void __unw_remove_dynamic_fde(unw_word_t fde);
 
+extern void __unw_add_dynamic_eh_frame_section(unw_word_t eh_frame_start);
+extern void __unw_remove_dynamic_eh_frame_section(unw_word_t eh_frame_start);
+
 #if defined(_LIBUNWIND_ARM_EHABI)
 extern const uint32_t* decode_eht_entry(const uint32_t*, size_t*, size_t*);
 extern _Unwind_Reason_Code _Unwind_VRS_Interpret(_Unwind_Context *context,
Index: libunwind/src/libunwind.cpp
===
--- libunwind/src/libunwind.cpp
+++ libunwind/src/libunwind.cpp
@@ -292,6 +292,35 @@
   // fde is own mh_group
   DwarfFDECache::removeAllIn((LocalAddressSpace::pint_t)fde);
 }
+
+void __unw_add_dynamic_eh_frame_section(unw_word_t eh_frame_start) {
+  // The eh_frame section start serves as the mh_group
+  unw_word_t mh_group = eh_frame_start;
+  CFI_Parser::CIE_Info cieInfo;
+  CFI_Parser::FDE_Info fdeInfo;
+  auto p = (LocalAddressSpace::pint_t)eh_frame_start;
+  while (true) {
+if (CFI_Parser::decodeFDE(
+LocalAddressSpace::sThisAddressSpace, p, &fdeInfo, &cieInfo,
+true) == NULL) {
+  DwarfFDECache::add((LocalAddressSpace::pint_t)mh_group,
+fdeInfo.pcStart, fdeInfo.pcEnd,
+fdeInfo.fdeStart);
+  p += fdeInfo.fdeLength;
+} else if (CFI_Parser::parseCIE(
+   LocalAddressSpace::sThisAddressSpace, p, &cieInfo) == NULL) {
+  p += cieInfo.cieLength;
+} else
+  return;
+  }
+}
+
+void __unw_remove_dynamic_eh_frame_section(unw_word_t eh_frame_start) {
+  // The eh_frame section start serves as the mh_group
+  DwarfFDECache::removeAllIn(
+  (LocalAddressSpace::pint_t)eh_frame_start);
+}
+
 #endif // defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND)
 #endif // !defined(__USING_SJLJ_EXCEPTIONS__)
 
Index: libunwind/src/DwarfParser.hpp
===
--- libunwind/src/DwarfParser.hpp
+++ libunwind/src/DwarfParser.hpp
@@ -154,7 +154,8 @@
   uintptr_t sectionLength, pint_t fdeHint, FDE_Info *fdeInfo,
   CIE_Info *cieInfo);
   static const char *decodeFDE(A &addressSpace, pint_t fdeStart,
-   FDE_Info *fdeInfo, CIE_Info *cieInfo);
+   FDE_Info *fdeInfo, CIE_Info *cieInfo,
+   bool useCIEInfo = false);
   static bool parseFDEInstructions(A &addressSpace, const FDE_Info &fdeInfo,
const CIE_Info &cieInfo, pint_t upToPC,
int arch, PrologInfo *results);
@@ -162,10 +163,14 @@
   static const char *parseCIE(A &addressSpace, pint_t cie, CIE_Info *cieInfo);
 };
 
-/// Parse a FDE into a CIE_Info and an FDE_Info
+/// Parse a FDE into a CIE_Info and an FDE_Info. If useCIEInfo is
+/// true, treat cieInfo as already-parsed CIE_Info (whose start offset
+/// must match the one specified by the FDE) rather than parsing the
+/// one indicated within the FDE.
 template 
 const char *CFI_Parser::decodeFDE(A &addressSpace, pint_t fdeStart,
- FDE_Info *fdeInfo, CIE_Info *cieInfo) {
+ FDE_Info *fdeInfo, CIE_Info *cieInfo,
+ bool useCIEInfo) {
   pint_t p = fdeStart;
   pint_t cfiLength = (pint_t)addressSpace.get32(p);
   p += 4;
@@ -181,9 +186,14 @@
 return "FDE is really a CIE"; // this is a CIE not an FDE
   pint_t nextCFI = p + cfiLength;
   pint_t cieStart = p - ciePointer;
-  const char *err = parseCIE(addressSpace, cieStart, cieInfo);
-  if (err != NULL)
-return err;
+  if (useCIEInfo) {
+if (cieInfo->cieStart != cieStart)
+  return "CIE start does not match";
+  } else {
+const char *err = parseCIE(addressSpace, cieStart, cieInfo);
+if (err != NULL)
+  return err;
+  }
   p += 4;
   // Parse pc begin and range.
   pint_t pcStart =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] bab3981 - [libunwind] Add an interface for dynamic .eh_frame registration

[Peter S. Housel via cfe-commits]

Author: Peter S. Housel
Date: 2021-11-18T08:06:46-08:00
New Revision: bab39816085d715e52c2131fa249ccd10178764b

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

LOG: [libunwind] Add an interface for dynamic .eh_frame registration

The libgcc runtime library provides __register_frame and
__deregister_frame functions, which can be used by dynamic code
generators to register an .eh_frame section, which contains one or
more Call Frame Information records, each consisting of a Common
Information Entry record followed by one or more Frame Description
Entry records. This libunwind library also provides __register_frame
and __deregister_frame functions, but they are aliases for
__unw_add_dynamic_fde and __unw_remove_dynamic_fde and thus can only
take a single FDE.

This patch adds __unw_add_dynamic_eh_frame_section and
__unw_remove_dynamic_eh_frame_section functions which explicitly use
the .eh_frame format. Clients such as the ORCv2 platform and runtime
can check for these functions and use them if unwinding is being
provided by libunwind, or fall back to __register_frame and
__deregister_frame if unwinding is provided by libgcc.

Reviewed By: lhames

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

Added: 


Modified: 
libunwind/src/DwarfParser.hpp
libunwind/src/libunwind.cpp
libunwind/src/libunwind_ext.h

Removed: 




diff  --git a/libunwind/src/DwarfParser.hpp b/libunwind/src/DwarfParser.hpp
index f9896ad03a113..2153a71c2ec02 100644
--- a/libunwind/src/DwarfParser.hpp
+++ b/libunwind/src/DwarfParser.hpp
@@ -154,7 +154,8 @@ class CFI_Parser {
   uintptr_t sectionLength, pint_t fdeHint, FDE_Info 
*fdeInfo,
   CIE_Info *cieInfo);
   static const char *decodeFDE(A &addressSpace, pint_t fdeStart,
-   FDE_Info *fdeInfo, CIE_Info *cieInfo);
+   FDE_Info *fdeInfo, CIE_Info *cieInfo,
+   bool useCIEInfo = false);
   static bool parseFDEInstructions(A &addressSpace, const FDE_Info &fdeInfo,
const CIE_Info &cieInfo, pint_t upToPC,
int arch, PrologInfo *results);
@@ -162,10 +163,14 @@ class CFI_Parser {
   static const char *parseCIE(A &addressSpace, pint_t cie, CIE_Info *cieInfo);
 };
 
-/// Parse a FDE into a CIE_Info and an FDE_Info
+/// Parse a FDE into a CIE_Info and an FDE_Info. If useCIEInfo is
+/// true, treat cieInfo as already-parsed CIE_Info (whose start offset
+/// must match the one specified by the FDE) rather than parsing the
+/// one indicated within the FDE.
 template 
 const char *CFI_Parser::decodeFDE(A &addressSpace, pint_t fdeStart,
- FDE_Info *fdeInfo, CIE_Info *cieInfo) {
+ FDE_Info *fdeInfo, CIE_Info *cieInfo,
+ bool useCIEInfo) {
   pint_t p = fdeStart;
   pint_t cfiLength = (pint_t)addressSpace.get32(p);
   p += 4;
@@ -181,9 +186,14 @@ const char *CFI_Parser::decodeFDE(A &addressSpace, 
pint_t fdeStart,
 return "FDE is really a CIE"; // this is a CIE not an FDE
   pint_t nextCFI = p + cfiLength;
   pint_t cieStart = p - ciePointer;
-  const char *err = parseCIE(addressSpace, cieStart, cieInfo);
-  if (err != NULL)
-return err;
+  if (useCIEInfo) {
+if (cieInfo->cieStart != cieStart)
+  return "CIE start does not match";
+  } else {
+const char *err = parseCIE(addressSpace, cieStart, cieInfo);
+if (err != NULL)
+  return err;
+  }
   p += 4;
   // Parse pc begin and range.
   pint_t pcStart =

diff  --git a/libunwind/src/libunwind.cpp b/libunwind/src/libunwind.cpp
index 2bf31b182cff2..48750ce670fbf 100644
--- a/libunwind/src/libunwind.cpp
+++ b/libunwind/src/libunwind.cpp
@@ -292,6 +292,35 @@ void __unw_remove_dynamic_fde(unw_word_t fde) {
   // fde is own mh_group
   
DwarfFDECache::removeAllIn((LocalAddressSpace::pint_t)fde);
 }
+
+void __unw_add_dynamic_eh_frame_section(unw_word_t eh_frame_start) {
+  // The eh_frame section start serves as the mh_group
+  unw_word_t mh_group = eh_frame_start;
+  CFI_Parser::CIE_Info cieInfo;
+  CFI_Parser::FDE_Info fdeInfo;
+  auto p = (LocalAddressSpace::pint_t)eh_frame_start;
+  while (true) {
+if (CFI_Parser::decodeFDE(
+LocalAddressSpace::sThisAddressSpace, p, &fdeInfo, &cieInfo,
+true) == NULL) {
+  
DwarfFDECache::add((LocalAddressSpace::pint_t)mh_group,
+fdeInfo.pcStart, fdeInfo.pcEnd,
+fdeInfo.fdeStart);
+  p += fdeInfo.fdeLength;
+} else if (CFI_Parser::parseCIE(
+   LocalAddressSpace::sThisAddressSpace, p, &cieInfo) == NULL) 
{
+  p +

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

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

kuhnel added a comment.

When trying to revert some of the changes, I just noticed there are a couple of 
`if (const auto* ...` in `CodeComplete.cpp` and `AST.cpp` (and maybe other 
files as well). So some folks seem to be using this right now. If we want to be 
consistent, we would have to remove these `const` as well.

I'm not sure what the right way forward would be.

@sammccall  WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113898

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

[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA added a comment.

In D114025#3140192 , @Quuxplusone 
wrote:

> Peanut gallery says: I'd recommend //not// taking this particular patch; it 
> seems much less "obvious" than the whitelist/inclusionlist type of patch. 
> Whereas "whitelist" is just a made-up buzzword that can easily be replaced 
> with a different made-up buzzword, "sanity check" is not at all the same as 
> "validation test." In many cases, I think "sanity-check" could be reasonably 
> replaced with "smoke-test," but (1) this PR doesn't do that, and (2) the 
> phrase "smoke-test" is probably //harder// to understand, and (3) this PR 
> currently touches many instances of "sanity/insanity/sane/insane" in code 
> comments which have nothing to do with testing or checking at all.

I think I understand your point and have tried to address that. Additionally, I 
would also argue that `sanity check/test` also serve as buzzwords and, while we 
may need to take greater care of how we reword things so they convey the 
meaning better, it shouldn't be fine to find replacements for them. Your review 
helps, thanks.

> "We could do X, but that would be insane" does //not// mean "We could do X, 
> but that would not pass validations." It means it would be //stupid//, 
> //irrational//, //foolish for obvious reasons//, something along those lines.
>
> I agree that "X is dumb/stupid/insane" is a bad code comment and should be 
> improved. It is bad //not just// because it is un-PC but because it is vague 
> and therefore not (directly) helpful to the reader. However, you cannot fix 
> this kind of comment by just substituting some made-up reason like "it would 
> fail validation," because a //lying// comment is //worse// than a vague 
> comment.
>
> Not all of the individual  diffs in this PR are bad. But some of them are.

I've tried to address this in the updated diff. However, I think this also 
makes a case that there is a general issue with using `sanity check/test` aside 
from inclusive language.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114025

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

[PATCH] D112923: [clang][deps] Reset some benign codegen options

[Michael Spencer via Phabricator via cfe-commits]

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

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112923

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

[PATCH] D113880: [clang][modules] Infer framework modules in explicit builds

[Michael Spencer via Phabricator via cfe-commits]

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

I think this makes sense to do given that you need to explicitly opt into 
inferred modules anyway. This isn't implicitly finding module maps, as we 
already have a module map with `framework module *`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113880

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

[PATCH] D114099: Enable `_Float16` type support on X86 without the avx512fp16 flag

[John McCall via Phabricator via cfe-commits]

rjmccall requested changes to this revision.
rjmccall added inline comments.
This revision now requires changes to proceed.



Comment at: clang/test/SemaCXX/Float16.cpp:4
+// RUN: %clang_cc1 -fsyntax-only -verify -triple armv7a-linux-gnu %s
+// RUN: %clang_cc1 -fsyntax-only -verify -triple aarch64-linux-gnu %s
 

This test (and Float16.c) should test at least one target that doesn't have 
`_Float16` support, so please just add `-DHAVE` to the x86_64 line and add, I 
dunno, a generic i386 or SPARC line.


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

https://reviews.llvm.org/D114099

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

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:229-233
+// We want to keep the following invariant at all times:
+// ---[ First -->
+// -[ Second --->
+if (First->From() > Second->From())
+  swapIterators(First, FirstEnd, Second, SecondEnd);

martong wrote:
> I am not sure about this, but perhaps we could put this `swapIterators` call 
> right at the beginning of the nested `while` loop (L243). That would 
> eliminate the need to call it again at the end of the second while loop.
I'm afraid, it is not the case. Every loop needs its own `swapIterators`. As 
you can see, `swapIterators` in the nested loop invokes not always because of 
`break;` in the middle. Otherwise, it would invokes each time. But I checked 
your suggestion. It doesn't work.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:243
+// Loop where the invariant holds.
+while (true) {
+  // Skip all enclosed ranges.

martong wrote:
> So, this loop is about to merge conjunct Ranges. The first time when we find 
> disjoint Ranges then we break out. (Or we return once we reach the end of any 
> of the RangeSets.)
> This makes we wonder, if it would be possible to split this `while` loop into 
> a lambda named `mergeConjunctRanges` ?
I'm not in favor of introducing another level of abstraction. It would divide 
the flow/comments and could reduce readability and performance which is crucial 
here.


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

https://reviews.llvm.org/D99797

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

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 388219.
ASDenysPetrov added a comment.

Fixed comments.


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

https://reviews.llvm.org/D99797

Files:
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/unittests/StaticAnalyzer/RangeSetTest.cpp

Index: clang/unittests/StaticAnalyzer/RangeSetTest.cpp
===
--- clang/unittests/StaticAnalyzer/RangeSetTest.cpp
+++ clang/unittests/StaticAnalyzer/RangeSetTest.cpp
@@ -35,12 +35,34 @@
   const RangeSet &Set) {
   return OS << toString(Set);
 }
+// We need it here for better fail diagnostics from gtest.
+LLVM_ATTRIBUTE_UNUSED static std::ostream &operator<<(std::ostream &OS,
+  const Range &R) {
+  return OS << toString(R);
+}
 
 } // namespace ento
 } // namespace clang
 
 namespace {
 
+template  struct TestValues {
+  static constexpr T MIN = std::numeric_limits::min();
+  static constexpr T MAX = std::numeric_limits::max();
+  // MID is a value in the middle of the range
+  // which unary minus does not affect on,
+  // e.g. int8/int32(0), uint8(128), uint32(2147483648).
+  static constexpr T MID =
+  std::is_signed::value ? 0 : ~(static_cast(-1) / static_cast(2));
+  static constexpr T A = MID - (MAX - MID) / 3 * 2;
+  static constexpr T B = MID - (MAX - MID) / 3;
+  static constexpr T C = -B;
+  static constexpr T D = -A;
+
+  static_assert(MIN < A && A < B && B < MID && MID < C && C < D && D < MAX,
+"Values shall be in an ascending order");
+};
+
 template  class RangeSetTest : public testing::Test {
 public:
   // Init block
@@ -55,24 +77,11 @@
   using RawRange = std::pair;
   using RawRangeSet = std::initializer_list;
 
-  static constexpr BaseType getMin() {
-return std::numeric_limits::min();
-  }
-  static constexpr BaseType getMax() {
-return std::numeric_limits::max();
-  }
-  static constexpr BaseType getMid() {
-return isSigned() ? 0 : ~(fromInt(-1) / fromInt(2));
-  }
-
-  static constexpr bool isSigned() { return std::is_signed::value; }
-  static constexpr BaseType fromInt(int X) { return static_cast(X); }
-
-  static llvm::APSInt Base;
   const llvm::APSInt &from(BaseType X) {
-llvm::APSInt Dummy = Base;
-Dummy = X;
-return BVF.getValue(Dummy);
+static llvm::APSInt Base{sizeof(BaseType) * CHAR_BIT,
+ std::is_unsigned::value};
+Base = X;
+return BVF.getValue(Base);
   }
 
   Range from(const RawRange &Init) {
@@ -160,7 +169,7 @@
 
   void checkAdd(RawRangeSet RawLHS, RawRangeSet RawRHS,
 RawRangeSet RawExpected) {
-wrap(&Self::checkAddImpl, RawRHS, RawLHS, RawExpected);
+wrap(&Self::checkAddImpl, RawLHS, RawRHS, RawExpected);
   }
 
   void checkAdd(RawRangeSet RawLHS, BaseType RawRHS, RawRangeSet RawExpected) {
@@ -168,6 +177,29 @@
  RawExpected);
   }
 
+  template 
+  void checkUniteImpl(RangeSet LHS, RHSType RHS, RangeSet Expected) {
+RangeSet Result = F.unite(LHS, RHS);
+EXPECT_EQ(Result, Expected)
+<< "while uniting " << toString(LHS) << " and " << toString(RHS);
+  }
+
+  void checkUnite(RawRangeSet RawLHS, RawRange RawRHS,
+  RawRangeSet RawExpected) {
+wrap(&Self::checkUniteImpl, RawLHS, RawRHS, RawExpected);
+  }
+
+  void checkUnite(RawRangeSet RawLHS, RawRangeSet RawRHS,
+  RawRangeSet RawExpected) {
+wrap(&Self::checkUniteImpl, RawLHS, RawRHS, RawExpected);
+  }
+
+  void checkUnite(RawRangeSet RawLHS, BaseType RawRHS,
+  RawRangeSet RawExpected) {
+wrap(&Self::checkUniteImpl, RawLHS, RawRHS,
+ RawExpected);
+  }
+
   void checkDeleteImpl(const llvm::APSInt &Point, RangeSet From,
RangeSet Expected) {
 RangeSet Result = F.deletePoint(From, Point);
@@ -183,29 +215,19 @@
 
 } // namespace
 
-template 
-llvm::APSInt RangeSetTest::Base{sizeof(BaseType) * 8, !isSigned()};
-
 using IntTypes = ::testing::Types;
 TYPED_TEST_SUITE(RangeSetTest, IntTypes, );
 
 TYPED_TEST(RangeSetTest, RangeSetNegateTest) {
-  // Use next values of the range {MIN, A, B, MID, C, D, MAX}.
-
-  constexpr TypeParam MIN = TestFixture::getMin();
-  constexpr TypeParam MAX = TestFixture::getMax();
-  // MID is a value in the middle of the range
-  // which unary minus does not affect on,
-  // e.g. int8/int32(0), uint8(128), uint32(2147483648).
-  constexpr TypeParam MID = TestFixture::getMid();
-  constexpr TypeParam A = MID - TestFixture::fromInt(42 + 42);
-  constexpr TypeParam B = MID - TestFixture::fromInt(42);
-  constexpr TypeParam C = -B;
-  constexpr TypeParam D = -A;
-
-  static_assert(MIN < A && A < B && B < MID && MID < C && C < D && D < MAX,
-"Values shall be in an ascending order");
+  using TV = Te

[PATCH] D113709: Don't consider `LinkageSpec` when calculating DeclContext `Encloses`

[John McCall via Phabricator via cfe-commits]

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

> We don't properly handle lookup through using declarations when there is a 
> linkage spec in the common chain.

Pedantic note: you mean using *directives*.

At some point, we should probably reconsider whether `extern "C"` ought to be a 
`DeclContext` at all, as opposed to ultimately just setting a bit on the decls 
and providing some way of recovering the source structure.  We don't make 
individual local scopes different DCs, and those probably have more right to be 
one than LSD.

Anyway, this patch seems acceptable.




Comment at: clang/lib/AST/DeclBase.cpp:1215
   for (; DC; DC = DC->getParent())
-if (DC->getPrimaryContext() == this)
+if (DC->getDeclKind() != Decl::LinkageSpec &&
+DC->getPrimaryContext() == this)

`!isa(DC)`, please


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

https://reviews.llvm.org/D113709

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

[PATCH] D99797: [analyzer] Implemented RangeSet::Factory::unite function to handle intersections and adjacency

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Thanks for the update, I am okay with the `.cpp` file, now I continue the 
review with the tests.


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

https://reviews.llvm.org/D99797

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