[PATCH] D118322: [clangd] Fix a selection tree crash for unmatched-bracket code.

[Haojian Wu via Phabricator via cfe-commits]

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

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118322

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


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -523,6 +523,10 @@
   auto l = [^[[foo = bar]]] { };
 })cpp",
"VarDecl"},
+  {R"cpp(
+/*error-ok*/
+void func() [[{^]])cpp",
+   "CompoundStmt"},
   };
 
   for (const Case &C : Cases) {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -306,6 +306,12 @@
   return SelectionTree::Unselected;
 }
 
+// The eof token is used as a sentinel.
+// In general, source range from an AST node should not claim the eof 
token,
+// but it could occur for unmatched-bracket cases.
+if (ExpandedTokens.back().kind() == tok::eof)
+  ExpandedTokens = ExpandedTokens.drop_back();
+
 SelectionTree::Selection Result = NoTokens;
 while (!ExpandedTokens.empty()) {
   // Take consecutive tokens from the same context together for efficiency.


Index: clang-tools-extra/clangd/unittests/SelectionTests.cpp
===
--- clang-tools-extra/clangd/unittests/SelectionTests.cpp
+++ clang-tools-extra/clangd/unittests/SelectionTests.cpp
@@ -523,6 +523,10 @@
   auto l = [^[[foo = bar]]] { };
 })cpp",
"VarDecl"},
+  {R"cpp(
+/*error-ok*/
+void func() [[{^]])cpp",
+   "CompoundStmt"},
   };
 
   for (const Case &C : Cases) {
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -306,6 +306,12 @@
   return SelectionTree::Unselected;
 }
 
+// The eof token is used as a sentinel.
+// In general, source range from an AST node should not claim the eof token,
+// but it could occur for unmatched-bracket cases.
+if (ExpandedTokens.back().kind() == tok::eof)
+  ExpandedTokens = ExpandedTokens.drop_back();
+
 SelectionTree::Selection Result = NoTokens;
 while (!ExpandedTokens.empty()) {
   // Take consecutive tokens from the same context together for efficiency.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117989: [RISCV] Add the passthru operand for RVV nomask binary intrinsics.

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

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

Looks reasonable to me. Thans @khchen!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117989

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

[PATCH] D118225: [RISCV] Decouple Zve* extensions.

[Jianjian Guan via Phabricator via cfe-commits]

jacquesguan added inline comments.



Comment at: llvm/lib/Support/RISCVISAInfo.cpp:747
   // Could not implement Zve* extension and the V extension at the same time.
-  if (HasZve32x && HasV)
+  if (HasZve && HasV)
 return createStringError(

craig.topper wrote:
> Now we need to check if multiple Zve extensions are specified at the same 
> time?
Done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118225

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

[PATCH] D118225: [RISCV] Decouple Zve* extensions.

[Jianjian Guan via Phabricator via cfe-commits]

jacquesguan updated this revision to Diff 403503.
jacquesguan added a comment.

Address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118225

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/test/CodeGen/RISCV/rvv-intrinsics/rvv-error.c
  clang/test/Preprocessor/riscv-target-features.c
  clang/utils/TableGen/RISCVVEmitter.cpp
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/MC/RISCV/attribute-arch.s

Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-arch.s
+++ llvm/test/MC/RISCV/attribute-arch.s
@@ -76,16 +76,16 @@
 # CHECK: attribute  5, "rv32i2p0_zve32x1p0_zvl32b1p0"
 
 .attribute arch, "rv32ifzve32f"
-# CHECK: attribute  5, "rv32i2p0_f2p0_zve32f1p0_zve32x1p0_zvl32b1p0"
+# CHECK: attribute  5, "rv32i2p0_f2p0_zve32f1p0_zvl32b1p0"
 
 .attribute arch, "rv32izve64x"
-# CHECK: attribute  5, "rv32i2p0_zve32x1p0_zve64x1p0_zvl32b1p0_zvl64b1p0"
+# CHECK: attribute  5, "rv32i2p0_zve64x1p0_zvl32b1p0_zvl64b1p0"
 
 .attribute arch, "rv32ifzve64f"
-# CHECK: attribute  5, "rv32i2p0_f2p0_zve32f1p0_zve32x1p0_zve64f1p0_zve64x1p0_zvl32b1p0_zvl64b1p0"
+# CHECK: attribute  5, "rv32i2p0_f2p0_zve64f1p0_zvl32b1p0_zvl64b1p0"
 
 .attribute arch, "rv32ifdzve64d"
-# CHECK: attribute  5, "rv32i2p0_f2p0_d2p0_zve32f1p0_zve32x1p0_zve64d1p0_zve64f1p0_zve64x1p0_zvl32b1p0_zvl64b1p0"
+# CHECK: attribute  5, "rv32i2p0_f2p0_d2p0_zve64d1p0_zvl32b1p0_zvl64b1p0"
 
 ## Experimental extensions require version string to be explicitly specified
 
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -202,18 +202,29 @@
   }
 
   // Vector codegen related methods.
-  bool hasVInstructions() const { return HasStdExtV || HasStdExtZve32x; }
-  bool hasVInstructionsI64() const { return HasStdExtV || HasStdExtZve64x; }
+  bool hasZve() const {
+return HasStdExtZve32x || HasStdExtZve32f || HasStdExtZve64x ||
+   HasStdExtZve64f || HasStdExtZve64d;
+  }
+  bool hasZve64() const {
+return HasStdExtZve64x || HasStdExtZve64f || HasStdExtZve64d;
+  }
+  bool hasZvef() const {
+return HasStdExtZve32f || HasStdExtZve64f || HasStdExtZve64d;
+  }
+  bool hasZved() const { return HasStdExtZve64d; }
+  bool hasVInstructions() const { return HasStdExtV || hasZve(); }
+  bool hasVInstructionsI64() const { return HasStdExtV || hasZve64(); }
   bool hasVInstructionsF16() const {
-return (HasStdExtV || HasStdExtZve32f) && HasStdExtZfh;
+return HasStdExtZfh && (HasStdExtV || hasZvef());
   }
   // FIXME: Consider Zfinx in the future
   bool hasVInstructionsF32() const {
-return HasStdExtV || (HasStdExtZve32f && HasStdExtF);
+return HasStdExtV || (hasZvef() && HasStdExtF);
   }
   // FIXME: Consider Zdinx in the future
   bool hasVInstructionsF64() const {
-return HasStdExtV || (HasStdExtZve64d && HasStdExtD);
+return HasStdExtV || (hasZved() && HasStdExtD);
   }
   // F16 and F64 both require F32.
   bool hasVInstructionsAnyF() const { return hasVInstructionsF32(); }
Index: llvm/lib/Target/RISCV/RISCV.td
===
--- llvm/lib/Target/RISCV/RISCV.td
+++ llvm/lib/Target/RISCV/RISCV.td
@@ -299,24 +299,24 @@
 : SubtargetFeature<"zve32f", "HasStdExtZve32f", "true",
"'Zve32f' (Vector Extensions for Embedded Processors "
"with maximal 32 EEW and F extension)",
-   [FeatureStdExtZve32x]>;
+   [FeatureStdExtZvl32b]>;
 
 def FeatureStdExtZve64x
 : SubtargetFeature<"zve64x", "HasStdExtZve64x", "true",
"'Zve64x' (Vector Extensions for Embedded Processors "
-   "with maximal 64 EEW)", [FeatureStdExtZve32x, FeatureStdExtZvl64b]>;
+   "with maximal 64 EEW)", [FeatureStdExtZvl64b]>;
 
 def FeatureStdExtZve64f
 : SubtargetFeature<"zve64f", "HasStdExtZve64f", "true",
"'Zve64f' (Vector Extensions for Embedded Processors "
"with maximal 64 EEW and F extension)",
-   [FeatureStdExtZve32f, FeatureStdExtZve64x]>;
+   [FeatureStdExtZvl64b]>;
 
 def FeatureStdExtZve64d
 : SubtargetFeature<"zve64d", "HasStdExtZve64d", "true",
"'Zve64d' (Vector Extensions for Embedded Processors "
"with maximal 64 EEW, F and D extension)",
-   [FeatureStdExtZve64f]>;
+   [FeatureStdExtZvl64b]>;
 
 def FeatureStdExtV
 : SubtargetFeature<"v", "HasStdExtV", "true",
Index: llvm/lib/Support/RISCVISAInfo.cpp
==

[PATCH] D118313: [Driver] Remove -fno-experimental-new-pass-manager

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

`rg 'clang_cc1.*fexperimental-new-pass-manager' -l => 41`

I think the CC1 option can be removed separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118313

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

[PATCH] D118313: [Driver] Remove -fno-experimental-new-pass-manager

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D118313#3274709 , @aeubanks wrote:

> can we also remove the -cc1 option?

Some tests will fail if `-DLLVM_ENABLE_NEW_PASS_MANAGER=off` is specified. But 
I see 5 tests have failed with `-DLLVM_ENABLE_NEW_PASS_MANAGER=off`, so perhaps 
we should not worry too much now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118313

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

[PATCH] D118313: [Driver] Remove -fno-experimental-new-pass-manager

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

can we also remove the -cc1 option?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118313

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

[clang] ad39b5b - [NFC] Remove duplicate include

[Jim Lin via cfe-commits]

Author: Jim Lin
Date: 2022-01-27T13:56:13+08:00
New Revision: ad39b5bc59b0e71c86f8cf290ead2d9dd09e5c3e

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

LOG: [NFC] Remove duplicate include

Added: 


Modified: 
clang/lib/Sema/SemaConcept.cpp
clang/unittests/AST/EvaluateAsRValueTest.cpp
clang/unittests/Tooling/Syntax/TokensTest.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 678f1e40e730b..ce99d4848ccaa 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -18,7 +18,6 @@
 #include "clang/Sema/Template.h"
 #include "clang/Sema/Overload.h"
 #include "clang/Sema/Initialization.h"
-#include "clang/Sema/SemaInternal.h"
 #include "clang/AST/ExprConcepts.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/OperatorPrecedence.h"

diff  --git a/clang/unittests/AST/EvaluateAsRValueTest.cpp 
b/clang/unittests/AST/EvaluateAsRValueTest.cpp
index 0475330796d12..bf44136835f21 100644
--- a/clang/unittests/AST/EvaluateAsRValueTest.cpp
+++ b/clang/unittests/AST/EvaluateAsRValueTest.cpp
@@ -11,7 +11,6 @@
 //
 
//===--===//
 
-#include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"

diff  --git a/clang/unittests/Tooling/Syntax/TokensTest.cpp 
b/clang/unittests/Tooling/Syntax/TokensTest.cpp
index e7cb25782133e..22134d9d0cfb0 100644
--- a/clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ b/clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -40,7 +40,6 @@
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Testing/Support/Annotations.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
-#include "gmock/gmock.h"
 #include 
 #include 
 #include 



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

[PATCH] D118010: [clang-tidy] Fix nested namespaces in `readability-static-definition-in-anonymous-namespace` check

[Richard via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG836950c4e602: [clang-tidy] Fix nested namespaces in 
`readability-static-definition-in… (authored by Izaron, committed by 
LegalizeAdulthood).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118010

Files:
  
clang-tools-extra/clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp
  
clang-tools-extra/docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-static-definition-in-anonymous-namespace.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-static-definition-in-anonymous-namespace.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability-static-definition-in-anonymous-namespace.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability-static-definition-in-anonymous-namespace.cpp
@@ -36,6 +36,21 @@
 DEFINE_STATIC_VAR(i);
 // CHECK-FIXES: {{^}}DEFINE_STATIC_VAR(i);
 
+namespace inner {
+int a = 1;
+const int b = 1;
+static int c = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'c' is a static definition in 
anonymous namespace; static is redundant here 
[readability-static-definition-in-anonymous-namespace]
+// CHECK-FIXES: {{^}}int c = 1;
+namespace deep_inner {
+int a = 1;
+const int b = 1;
+static int c = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'c' is a static definition in 
anonymous namespace; static is redundant here 
[readability-static-definition-in-anonymous-namespace]
+// CHECK-FIXES: {{^}}int c = 1;
+} // namespace deep_inner
+} // namespace inner
+
 } // namespace
 
 namespace N {
Index: 
clang-tools-extra/docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst
===
--- 
clang-tools-extra/docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst
+++ 
clang-tools-extra/docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst
@@ -12,7 +12,10 @@
 
   namespace {
 static int a = 1; // Warning.
-static const b = 1; // Warning.
+static const int b = 1; // Warning.
+namespace inner {
+  static int c = 1; // Warning.
+}
   }
 
 The check will apply a fix by removing the redundant ``static`` qualifier.
Index: 
clang-tools-extra/clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp
+++ 
clang-tools-extra/clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp
@@ -17,12 +17,16 @@
 namespace tidy {
 namespace readability {
 
+AST_MATCHER(NamedDecl, isInAnonymousNamespace) {
+  return Node.isInAnonymousNamespace();
+}
+
 void StaticDefinitionInAnonymousNamespaceCheck::registerMatchers(
 MatchFinder *Finder) {
   Finder->addMatcher(
   namedDecl(anyOf(functionDecl(isDefinition(), isStaticStorageClass()),
   varDecl(isDefinition(), isStaticStorageClass())),
-hasParent(namespaceDecl(isAnonymous(
+isInAnonymousNamespace())
   .bind("static-def"),
   this);
 }


Index: clang-tools-extra/test/clang-tidy/checkers/readability-static-definition-in-anonymous-namespace.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-static-definition-in-anonymous-namespace.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-static-definition-in-anonymous-namespace.cpp
@@ -36,6 +36,21 @@
 DEFINE_STATIC_VAR(i);
 // CHECK-FIXES: {{^}}DEFINE_STATIC_VAR(i);
 
+namespace inner {
+int a = 1;
+const int b = 1;
+static int c = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'c' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace]
+// CHECK-FIXES: {{^}}int c = 1;
+namespace deep_inner {
+int a = 1;
+const int b = 1;
+static int c = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'c' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace]
+// CHECK-FIXES: {{^}}int c = 1;
+} // namespace deep_inner
+} // namespace inner
+
 } // namespace
 
 namespace N {
Index: clang-tools-extra/docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst
@@ -12,7 +12,10 @@
 
   namespace {
 static int a = 1; // Warning.
-static const b = 1; 

[clang-tools-extra] 836950c - [clang-tidy] Fix nested namespaces in `readability-static-definition-in-anonymous-namespace` check

[via cfe-commits]

Author: Evgeny Shulgin
Date: 2022-01-26T21:54:17-07:00
New Revision: 836950c4e602548e47851b66a261472f124994fa

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

LOG: [clang-tidy] Fix nested namespaces in 
`readability-static-definition-in-anonymous-namespace` check

The check previously inspected only the immediate parent namespace.
`static` in a named namespace within an unnamed namespace is still
redundant.
We will use `Decl::isInAnonymousNamespace()` method that traverses the
namespaces hierarchy recursively.

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

Added: 


Modified: 

clang-tools-extra/clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp

clang-tools-extra/docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst

clang-tools-extra/test/clang-tidy/checkers/readability-static-definition-in-anonymous-namespace.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp
 
b/clang-tools-extra/clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp
index ef973a1927b07..b76c1837b8019 100644
--- 
a/clang-tools-extra/clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp
+++ 
b/clang-tools-extra/clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp
@@ -17,12 +17,16 @@ namespace clang {
 namespace tidy {
 namespace readability {
 
+AST_MATCHER(NamedDecl, isInAnonymousNamespace) {
+  return Node.isInAnonymousNamespace();
+}
+
 void StaticDefinitionInAnonymousNamespaceCheck::registerMatchers(
 MatchFinder *Finder) {
   Finder->addMatcher(
   namedDecl(anyOf(functionDecl(isDefinition(), isStaticStorageClass()),
   varDecl(isDefinition(), isStaticStorageClass())),
-hasParent(namespaceDecl(isAnonymous(
+isInAnonymousNamespace())
   .bind("static-def"),
   this);
 }

diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst
 
b/clang-tools-extra/docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst
index c1803d4b13052..91d40bd713ee2 100644
--- 
a/clang-tools-extra/docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst
+++ 
b/clang-tools-extra/docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst
@@ -12,7 +12,10 @@ visibility of definitions to a single translation unit.
 
   namespace {
 static int a = 1; // Warning.
-static const b = 1; // Warning.
+static const int b = 1; // Warning.
+namespace inner {
+  static int c = 1; // Warning.
+}
   }
 
 The check will apply a fix by removing the redundant ``static`` qualifier.

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability-static-definition-in-anonymous-namespace.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability-static-definition-in-anonymous-namespace.cpp
index 5c3c8c12d683c..e9938db4f5b83 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability-static-definition-in-anonymous-namespace.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability-static-definition-in-anonymous-namespace.cpp
@@ -36,6 +36,21 @@ DEFINE_STATIC int h = 1;
 DEFINE_STATIC_VAR(i);
 // CHECK-FIXES: {{^}}DEFINE_STATIC_VAR(i);
 
+namespace inner {
+int a = 1;
+const int b = 1;
+static int c = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'c' is a static definition in 
anonymous namespace; static is redundant here 
[readability-static-definition-in-anonymous-namespace]
+// CHECK-FIXES: {{^}}int c = 1;
+namespace deep_inner {
+int a = 1;
+const int b = 1;
+static int c = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'c' is a static definition in 
anonymous namespace; static is redundant here 
[readability-static-definition-in-anonymous-namespace]
+// CHECK-FIXES: {{^}}int c = 1;
+} // namespace deep_inner
+} // namespace inner
+
 } // namespace
 
 namespace N {



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

[PATCH] D118313: [Driver] Remove -fno-experimental-new-pass-manager

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: aeubanks, ctetreau, nikic.
Herald added a subscriber: dang.
MaskRay requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

to give users a final warning that they need to migrate away. They could still
use -flegacy-pass-manager for Clang 14.0.0, but the functionality may not work
for 15.0.0.

-fexperimental-new-pass-manager is a no-op for default builds, so not urgent to
be removed for 14.0.0.

clang/test/Frontend/optimization-remark-with-hotness.c is removed because its
new PM replacement optimization-remark-with-hotness-new-pm.c exists.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118313

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/flegacy-pass-manager.c
  clang/test/Frontend/optimization-remark-with-hotness.c

Index: clang/test/Frontend/optimization-remark-with-hotness.c
===
--- clang/test/Frontend/optimization-remark-with-hotness.c
+++ /dev/null
@@ -1,78 +0,0 @@
-// Generate instrumentation and sampling profile data.
-// RUN: llvm-profdata merge \
-// RUN: %S/Inputs/optimization-remark-with-hotness.proftext \
-// RUN: -o %t.profdata
-// RUN: llvm-profdata merge -sample \
-// RUN: %S/Inputs/optimization-remark-with-hotness-sample.proftext \
-// RUN: -o %t-sample.profdata
-//
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \
-// RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \
-// RUN: -fprofile-instrument-use-path=%t.profdata -Rpass=inline \
-// RUN: -fno-experimental-new-pass-manager \
-// RUN: -Rpass-analysis=inline -Rpass-missed=inline \
-// RUN: -fdiagnostics-show-hotness -verify
-// The clang version of the previous test.
-// RUN: %clang -target x86_64-apple-macosx10.9 %s -c -emit-llvm -o /dev/null \
-// RUN: -fprofile-instr-use=%t.profdata -Rpass=inline \
-// RUN: -fno-experimental-new-pass-manager \
-// RUN: -Rpass-analysis=inline -Rpass-missed=inline \
-// RUN: -fdiagnostics-show-hotness -Xclang -verify
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \
-// RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \
-// RUN: -fprofile-sample-use=%t-sample.profdata -Rpass=inline \
-// RUN: -fno-experimental-new-pass-manager \
-// RUN: -Rpass-analysis=inline -Rpass-missed=inline \
-// RUN: -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=10 \
-// RUN: -verify
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \
-// RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \
-// RUN: -fprofile-instrument-use-path=%t.profdata -Rpass=inline \
-// RUN: -fno-experimental-new-pass-manager \
-// RUN: -Rpass-analysis=inline -Rpass-missed=inline \
-// RUN: -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=10 -verify
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \
-// RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \
-// RUN: -fprofile-instrument-use-path=%t.profdata -Rpass=inline \
-// RUN: -fno-experimental-new-pass-manager \
-// RUN: -Rpass-analysis=inline 2>&1 | FileCheck -check-prefix=HOTNESS_OFF %s
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \
-// RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \
-// RUN: -fprofile-instrument-use-path=%t.profdata -Rpass=inline \
-// RUN: -fno-experimental-new-pass-manager \
-// RUN: -Rpass-analysis=inline -Rno-pass-with-hotness 2>&1 | FileCheck \
-// RUN: -check-prefix=HOTNESS_OFF %s
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \
-// RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \
-// RUN: -fprofile-instrument-use-path=%t.profdata -Rpass=inline \
-// RUN: -Rpass-analysis=inline -fdiagnostics-show-hotness \
-// RUN: -fdiagnostics-hotness-threshold=100 2>&1 \
-// RUN: | FileCheck -allow-empty -check-prefix=THRESHOLD %s
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name \
-// RUN: optimization-remark-with-hotness.c %s -emit-llvm-only \
-// RUN: -Rpass=inline -Rpass-analysis=inline \
-// RUN: -fdiagnostics-show-hotness -fdiagnostics-hotness-threshold=10 2>&1 \
-// RUN: | FileCheck -check-prefix=NO_PGO %s
-
-int foo(int x, int y) __attribute__((always_inline));
-int foo(int x, int y) { return x + y; }
-
-int sum = 0;
-
-void bar(int x) {
-  // HOTNESS_OFF: 'foo' inlined into 'bar'
-  // HOTNESS_OFF-NOT: hotness:
-  // THRESHOLD-NOT: inlined
-  // THRESHOLD-NOT: hotness
-  // NO_PGO: '-fdiagnostics-show-hotness' requires profile-guided optimization information
-  // NO_PGO: '-fdiagnostics-hotness-threshold=' requires profile-guided optimization information
-  // expected-remark@+1 {{'foo' inlined into 'bar' with (cost=always): always inliner at callsite bar:8:10; (hotness:}}
-  sum += foo(x

[clang] ecb5023 - [ObjC] Emit selector load right before msgSend call.

[Ahmed Bougacha via cfe-commits]

Author: Ahmed Bougacha
Date: 2022-01-26T20:52:54-08:00
New Revision: ecb502342cf2b3b4b6dfcd8fe881c596880d1a4a

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

LOG: [ObjC] Emit selector load right before msgSend call.

We currently emit the selector load early, but only because we need
it to compute the signature (so that we know which msgSend variant to
call).  We can prepare the signature with a plain undef, and replace
it with the materialized selector value if (and only if) needed, later.

Concretely, this usually doesn't have an effect, but tests need updating
because we reordered the receiver bitcast and the selector load, which
is always fine.

There is one notable change: with this, when a msgSend needs a
receiver null check, the selector is now loaded in the non-null
block, instead of before the null check.  That should be a mild
improvement.

Added: 


Modified: 
clang/lib/CodeGen/CGObjCMac.cpp
clang/test/CodeGenObjC/arc-foreach.m
clang/test/CodeGenObjC/arc-literals.m
clang/test/CodeGenObjC/arc-loadweakretained-release.m
clang/test/CodeGenObjC/arc-precise-lifetime.m
clang/test/CodeGenObjC/arc-property.m
clang/test/CodeGenObjC/arc-related-result-type.m
clang/test/CodeGenObjC/arc.m
clang/test/CodeGenObjC/class-stubs.m
clang/test/CodeGenObjC/matrix-type-operators.m
clang/test/CodeGenObjC/ns_consume_null_check.m
clang/test/CodeGenObjC/nsvalue-objc-boxable-ios-arc.m
clang/test/CodeGenObjC/nsvalue-objc-boxable-ios.m
clang/test/CodeGenObjC/nsvalue-objc-boxable-mac-arc.m
clang/test/CodeGenObjC/nsvalue-objc-boxable-mac.m
clang/test/CodeGenObjC/objc-container-subscripting-1.m
clang/test/CodeGenObjC/property.m
clang/test/CodeGenObjC/tentative-cfconstantstring.m
clang/test/CodeGenObjCXX/arc-cxx11-member-init.mm
clang/test/CodeGenObjCXX/arc.mm
clang/test/CodeGenObjCXX/message.mm
clang/test/CodeGenObjCXX/property-lvalue-capture.mm
clang/test/CodeGenObjCXX/property-object-reference.mm

Removed: 




diff  --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp
index a11a9ca7d8c0f..e7dba4c8feabe 100644
--- a/clang/lib/CodeGen/CGObjCMac.cpp
+++ b/clang/lib/CodeGen/CGObjCMac.cpp
@@ -2138,16 +2138,7 @@ 
CGObjCCommonMac::EmitMessageSend(CodeGen::CodeGenFunction &CGF,
  const ObjCCommonTypesHelper &ObjCTypes) {
   CodeGenTypes &Types = CGM.getTypes();
   auto selTy = CGF.getContext().getObjCSelType();
-  llvm::Value *SelValue;
-
-  if (Method && Method->isDirectMethod()) {
-// Direct methods will synthesize the proper `_cmd` internally,
-// so just don't bother with setting the `_cmd` argument.
-assert(!IsSuper);
-SelValue = llvm::UndefValue::get(Types.ConvertType(selTy));
-  } else {
-SelValue = GetSelector(CGF, Sel);
-  }
+  llvm::Value *SelValue = llvm::UndefValue::get(Types.ConvertType(selTy));
 
   CallArgList ActualArgs;
   if (!IsSuper)
@@ -2168,10 +2159,15 @@ 
CGObjCCommonMac::EmitMessageSend(CodeGen::CodeGenFunction &CGF,
 canMessageReceiverBeNull(CGF, Method, IsSuper, ClassReceiver, Arg0);
 
   bool RequiresNullCheck = false;
+  bool RequiresSelValue = true;
 
   llvm::FunctionCallee Fn = nullptr;
   if (Method && Method->isDirectMethod()) {
+assert(!IsSuper);
 Fn = GenerateDirectMethod(Method, Method->getClassInterface());
+// Direct methods will synthesize the proper `_cmd` internally,
+// so just don't bother with setting the `_cmd` argument.
+RequiresSelValue = false;
   } else if (CGM.ReturnSlotInterferesWithArgs(MSI.CallInfo)) {
 if (ReceiverCanBeNull) RequiresNullCheck = true;
 Fn = (ObjCABI == 2) ?  ObjCTypes.getSendStretFn2(IsSuper)
@@ -2209,6 +2205,12 @@ 
CGObjCCommonMac::EmitMessageSend(CodeGen::CodeGenFunction &CGF,
 nullReturn.init(CGF, Arg0);
   }
 
+  // If a selector value needs to be passed, emit the load before the call.
+  if (RequiresSelValue) {
+SelValue = GetSelector(CGF, Sel);
+ActualArgs[1] = CallArg(RValue::get(SelValue), selTy);
+  }
+
   llvm::CallBase *CallSite;
   CGCallee Callee = CGCallee::forDirect(BitcastFn);
   RValue rvalue = CGF.EmitCall(MSI.CallInfo, Callee, Return, ActualArgs,

diff  --git a/clang/test/CodeGenObjC/arc-foreach.m 
b/clang/test/CodeGenObjC/arc-foreach.m
index 441b134fc2c03..7b1905a6ff756 100644
--- a/clang/test/CodeGenObjC/arc-foreach.m
+++ b/clang/test/CodeGenObjC/arc-foreach.m
@@ -51,9 +51,9 @@ void test0(NSArray *array) {
 // CHECK-LP64-NEXT: [[SAVED_ARRAY:%.*]] = bitcast i8* [[T2]] to [[ARRAY_T]]*
 
 // Call the enumeration method.
-// CHECK-LP64-NEXT: [[T0:%.*]] = load i8*, i8** @OBJC_SELECTOR_REFERENCES_
-// CHECK-LP64-NEXT: [[T1:%.*]] = bitcast [[ARRAY_T]]* [[SAVED_ARRAY]] to i8*
-// CHECK-LP64-NEXT: [[SIZE:%.*]] = call i6

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

[psigillito via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/FormatToken.h:125
+/// Operators that can follow a C variable.
+static const std::set C_OperatorsFollowingVar = {
+tok::l_square, tok::r_square,

curdeius wrote:
> HazardyKnusperkeks wrote:
> > And maybe choose a different container: 
> > https://llvm.org/docs/ProgrammersManual.html#set-like-containers-std-set-smallset-setvector-etc
> Not done it seems.
> Please rename and use a different type. Maybe `ImmutableSet`? Or just a 
> sorted `std::vector`?
ok updated to sorted std::vector



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:104
+
+auto isAccessModifier = [this, &RootToken]() {
+  bool returnValue{false};

HazardyKnusperkeks wrote:
> In clang-format all lambdas I've seen start with a capital letter.
Updated. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117416

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

[PATCH] D118311: [Clang][ModuleMap] Add conditional parsing via requires block declaration

[Michael Spencer via Phabricator via cfe-commits]

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

Add a new form of requires called a requires block declaration.

A requires block declaration allows conditional inclusion of other
declarations within a module map.

The syntax is the same as a requires-declaration, except that it is
followed by a block. If the feature-list isn’t satisfied, then the
contents of the block is ignored and skipped over. If the feature-list
is satisfied, then the contents of the requires block are parsed as if
the requires block was not present.

This differs from a requires-declaration in that it is not an error to
import a module from within an unsatisfied requires-block-declaration
as long as another declaration of the module exists.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118311

Files:
  clang/docs/Modules.rst
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Basic/Module.h
  clang/lib/Basic/Module.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/test/Modules/requires-block-errors.m
  clang/test/Modules/requires-block.m

Index: clang/test/Modules/requires-block.m
===
--- /dev/null
+++ clang/test/Modules/requires-block.m
@@ -0,0 +1,64 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t/module-cache -fmodules -fimplicit-module-maps -I %t/include %t/A.m -verify
+// RUN: %clang_cc1 -fmodules-cache-path=%t/module-cache -fmodules -fimplicit-module-maps -I %t/include %t/A.mm
+
+//--- include/module.modulemap
+
+requires !cplusplus {
+  module A {
+header "A.h"
+  }
+}
+
+requires cplusplus {
+  module A {
+header "A.h"
+header "A.hpp"
+  }
+}
+
+module B {
+  requires !cplusplus {
+header "B.h"
+  }
+  requires cplusplus {
+header "B.hpp"
+  }
+}
+
+//--- include/A.h
+
+typedef int A_t;
+
+//--- include/A.hpp
+
+using Adrgdrg_t = long;
+
+//--- include/B.h
+
+typedef int B_t;
+
+//--- include/B.hpp
+
+using Bdrgdrg_t = long;
+
+//--- A.m
+@import A;
+A_t a;
+Adrgdrg_t b; // expected-error {{unknown type name 'Adrgdrg_t'}}
+
+//--- A.mm
+@import A;
+A_t a;
+Adrgdrg_t b;
+
+//--- B.m
+@import B;
+B_t a;
+Bdrgdrg_t b; // expected-error {{unknown type name 'Bdrgdrg_t'}}
+
+//--- B.mm
+@import B;
+B_t a; // expected-error {{unknown type name 'B_t'}}
+Bdrgdrg_t b;
Index: clang/test/Modules/requires-block-errors.m
===
--- /dev/null
+++ clang/test/Modules/requires-block-errors.m
@@ -0,0 +1,31 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t/module-cache -fmodules -fimplicit-module-maps -I %t/missing-closing-brace %t/missing-closing-brace.m -verify
+// RUN: %clang_cc1 -fmodules-cache-path=%t/module-cache -fmodules -fimplicit-module-maps -I %t/missing-opening-brace %t/missing-opening-brace.m -verify
+
+//--- missing-closing-brace/module.modulemap
+
+requires !cplusplus {
+  module Pony {
+header "Pony.h"
+  }
+
+//--- missing-closing-brace/Pony.h
+
+//--- missing-closing-brace.m
+// expected-error@module.modulemap:7 {{expected '}'}}
+// expected-note@module.modulemap:2 {{to match this '{'}}
+@import Pony // expected-error {{could not build module 'Pony'}}
+
+//--- missing-opening-brace/module.modulemap
+
+requires !cplusplus
+module Pony {
+  header "Pony.h"
+}
+
+//--- missing-opening-brace/Pony.h
+
+//--- missing-opening-brace.m
+// expected-error@module.modulemap:3 {{expected '{' to start rquires block}}
+@import Pony // expected-error {{could not build module 'Pony'}}
Index: clang/lib/Lex/ModuleMap.cpp
===
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1468,10 +1468,19 @@
 
 using ModuleId = SmallVector, 2>;
 
+struct RequiresFeature {
+  bool RequiredState;
+  std::string Feature;
+};
+using RequiresFeatureList = SmallVector;
+
 bool parseModuleId(ModuleId &Id);
 void parseModuleDecl();
+void parseModuleMembers();
 void parseExternModuleDecl();
-void parseRequiresDecl();
+void parseRequiresFeatureList(RequiresFeatureList &RFL);
+void parseRequiresDecl(bool TopLevel = false);
+void parseRequiresBlockBody(const RequiresFeatureList &RFL);
 void parseHeaderDecl(MMToken::TokenKind, SourceLocation LeadingLoc);
 void parseUmbrellaDirDecl(SourceLocation UmbrellaLoc);
 void parseExportDecl();
@@ -1825,6 +1834,7 @@
 ///
 ///   module-member:
 /// requires-declaration
+/// requires-block-declaration
 /// header-declaration
 /// submodule-declaration
 /// export-declaration
@@ -2030,6 +2040,38 @@
   ActiveModule->ModuleMapIsPrivate)
 diagnosePrivateModules(ExplicitLoc, FrameworkLoc);
 
+  parseModuleMemb

[PATCH] D117935: [AIX][clang] include_next through clang provided float.h

[Hubert Tong via Phabricator via cfe-commits]

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

LGTM with minor comment.




Comment at: clang/test/Headers/Inputs/include/float.h:2
+#pragma once
+#define FLOAT_LOCAL_DEF 1

Minor nit: Suggest to use more explicit name.



Comment at: clang/test/Headers/float-aix.c:6
+
+_Static_assert(FLOAT_LOCAL_DEF, "");




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

https://reviews.llvm.org/D117935

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

[PATCH] D116542: [OpenMP] Add a flag for embedding a file into the module

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 403476.
jhuber6 added a comment.

Forgot to rename file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116542

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/CodeGen/BackendUtil.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/Frontend/embed-object.ll
  llvm/include/llvm/Bitcode/BitcodeWriter.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Bitcode/Writer/CMakeLists.txt

Index: llvm/lib/Bitcode/Writer/CMakeLists.txt
===
--- llvm/lib/Bitcode/Writer/CMakeLists.txt
+++ llvm/lib/Bitcode/Writer/CMakeLists.txt
@@ -11,6 +11,7 @@
   Analysis
   Core
   MC
+  TransformUtils
   Object
   Support
   )
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -69,6 +69,7 @@
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/SHA1.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
 #include 
 #include 
 #include 
@@ -4972,3 +4973,19 @@
   llvm::ConstantArray::get(ATy, UsedArray), "llvm.compiler.used");
   NewUsed->setSection("llvm.metadata");
 }
+
+void llvm::EmbedBufferInModule(llvm::Module &M, llvm::MemoryBufferRef Buf,
+   StringRef SectionName) {
+  ArrayRef ModuleData =
+  ArrayRef(Buf.getBufferStart(), Buf.getBufferSize());
+
+  // Embed the data in the
+  llvm::Constant *ModuleConstant =
+  llvm::ConstantDataArray::get(M.getContext(), ModuleData);
+  llvm::GlobalVariable *GV = new llvm::GlobalVariable(
+  M, ModuleConstant->getType(), true, llvm::GlobalValue::PrivateLinkage,
+  ModuleConstant, "llvm.embedded.object");
+  GV->setSection(SectionName);
+
+  appendToCompilerUsed(M, GV);
+}
Index: llvm/include/llvm/Bitcode/BitcodeWriter.h
===
--- llvm/include/llvm/Bitcode/BitcodeWriter.h
+++ llvm/include/llvm/Bitcode/BitcodeWriter.h
@@ -165,6 +165,11 @@
 bool EmbedCmdline,
 const std::vector &CmdArgs);
 
+  /// Embeds the memory buffer \p Buf into the module \p M as a global using the
+  /// section name \p SectionName.
+  void EmbedBufferInModule(Module &M, MemoryBufferRef Buf,
+   StringRef SectionName);
+
 } // end namespace llvm
 
 #endif // LLVM_BITCODE_BITCODEWRITER_H
Index: clang/test/Frontend/embed-object.ll
===
--- /dev/null
+++ clang/test/Frontend/embed-object.ll
@@ -0,0 +1,13 @@
+; RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm \
+; RUN:-fembed-offload-object=%S/Inputs/empty.h,section -x ir %s -o - \
+; RUN:| FileCheck %s -check-prefix=CHECK
+
+; CHECK: @llvm.embedded.object = private constant [0 x i8] zeroinitializer, section ".llvm.offloading.section"
+; CHECK: @llvm.compiler.used = appending global [2 x i8*] [i8* @x, i8* getelementptr inbounds ([0 x i8], [0 x i8]* @llvm.embedded.object, i32 0, i32 0)], section "llvm.metadata"
+
+@x = private constant i8 1
+@llvm.compiler.used = appending global [1 x i8*] [i8* @x], section "llvm.metadata"
+
+define i32 @foo() {
+  ret i32 0
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -1134,6 +1134,7 @@
 TheModule->setTargetTriple(TargetOpts.Triple);
   }
 
+  EmbedObject(TheModule.get(), CodeGenOpts, Diagnostics);
   EmbedBitcode(TheModule.get(), CodeGenOpts, *MainFile);
 
   LLVMContext &Ctx = TheModule->getContext();
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1750,3 +1750,25 @@
   CGOpts.getEmbedBitcode() != CodeGenOptions::Embed_Bitcode,
   CGOpts.CmdArgs);
 }
+
+void clang::EmbedObject(llvm::Module *M, const CodeGenOptions &CGOpts,
+DiagnosticsEngine &Diags) {
+  if (CGOpts.OffloadObjects.empty())
+return;
+
+  for (StringRef OffloadObject : CGOpts.OffloadObjects) {
+auto FilenameAndSection = OffloadObject.split(',');
+llvm::ErrorOr> ObjectOrErr =
+llvm::MemoryBuffer::getFileOrSTDIN(std::get<0>(FilenameAndSection));
+if (std::error_code EC = ObjectOrErr.getError()) {
+  auto DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
+  "could not open '%0' for embedding");
+  Diags.Report(DiagID) << std::get<0>(FilenameAndSection);
+  return;
+}
+
+SmallString<128> SectionName(
+{

[PATCH] D116542: [OpenMP] Add a flag for embedding a file into the module

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 403468.
jhuber6 added a comment.

clang format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116542

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/CodeGen/BackendUtil.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/Frontend/embed-object.ll
  llvm/include/llvm/Bitcode/BitcodeWriter.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Bitcode/Writer/CMakeLists.txt

Index: llvm/lib/Bitcode/Writer/CMakeLists.txt
===
--- llvm/lib/Bitcode/Writer/CMakeLists.txt
+++ llvm/lib/Bitcode/Writer/CMakeLists.txt
@@ -11,6 +11,7 @@
   Analysis
   Core
   MC
+  TransformUtils
   Object
   Support
   )
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -69,6 +69,7 @@
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/SHA1.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
 #include 
 #include 
 #include 
@@ -4972,3 +4973,19 @@
   llvm::ConstantArray::get(ATy, UsedArray), "llvm.compiler.used");
   NewUsed->setSection("llvm.metadata");
 }
+
+void llvm::EmbedBufferInModule(llvm::Module &M, llvm::MemoryBufferRef Buf,
+   StringRef SectionName) {
+  ArrayRef ModuleData =
+  ArrayRef(Buf.getBufferStart(), Buf.getBufferSize());
+
+  // Embed the data in the
+  llvm::Constant *ModuleConstant =
+  llvm::ConstantDataArray::get(M.getContext(), ModuleData);
+  llvm::GlobalVariable *GV = new llvm::GlobalVariable(
+  M, ModuleConstant->getType(), true, llvm::GlobalValue::PrivateLinkage,
+  ModuleConstant, "llvm.embedded.object");
+  GV->setSection(SectionName);
+
+  appendToCompilerUsed(M, GV);
+}
Index: llvm/include/llvm/Bitcode/BitcodeWriter.h
===
--- llvm/include/llvm/Bitcode/BitcodeWriter.h
+++ llvm/include/llvm/Bitcode/BitcodeWriter.h
@@ -165,6 +165,11 @@
 bool EmbedCmdline,
 const std::vector &CmdArgs);
 
+  /// Embeds the memory buffer \p Buf into the module \p M as a global using the
+  /// section name \p SectionName.
+  void EmbedBufferInModule(Module &M, MemoryBufferRef Buf,
+   StringRef SectionName);
+
 } // end namespace llvm
 
 #endif // LLVM_BITCODE_BITCODEWRITER_H
Index: clang/test/Frontend/embed-object.ll
===
--- /dev/null
+++ clang/test/Frontend/embed-object.ll
@@ -0,0 +1,13 @@
+; RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm \
+; RUN:-fembed-offload-object=%S/Inputs/empty.h,section -x ir %s -o - \
+; RUN:| FileCheck %s -check-prefix=CHECK
+
+; CHECK: @llvm.embedded.object = private constant [0 x i8] zeroinitializer, section ".llvm.offloading.section"
+; CHECK: @llvm.compiler.used = appending global [2 x i8*] [i8* @x, i8* getelementptr inbounds ([0 x i8], [0 x i8]* @llvm.embedded.object, i32 0, i32 0)], section "llvm.metadata"
+
+@x = private constant i8 1
+@llvm.compiler.used = appending global [1 x i8*] [i8* @x], section "llvm.metadata"
+
+define i32 @foo() {
+  ret i32 0
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -1134,6 +1134,7 @@
 TheModule->setTargetTriple(TargetOpts.Triple);
   }
 
+  EmbedBinary(TheModule.get(), CodeGenOpts, Diagnostics);
   EmbedBitcode(TheModule.get(), CodeGenOpts, *MainFile);
 
   LLVMContext &Ctx = TheModule->getContext();
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1745,8 +1745,31 @@
  llvm::MemoryBufferRef Buf) {
   if (CGOpts.getEmbedBitcode() == CodeGenOptions::Embed_Off)
 return;
+
   llvm::EmbedBitcodeInModule(
   *M, Buf, CGOpts.getEmbedBitcode() != CodeGenOptions::Embed_Marker,
   CGOpts.getEmbedBitcode() != CodeGenOptions::Embed_Bitcode,
   CGOpts.CmdArgs);
 }
+
+void clang::EmbedObject(llvm::Module *M, const CodeGenOptions &CGOpts,
+DiagnosticsEngine &Diags) {
+  if (CGOpts.OffloadObjects.empty())
+return;
+
+  for (StringRef OffloadObject : CGOpts.OffloadObjects) {
+auto FilenameAndSection = OffloadObject.split(',');
+llvm::ErrorOr> ObjectOrErr =
+llvm::MemoryBuffer::getFileOrSTDIN(std::get<0>(FilenameAndSection));
+if (std::error_code EC = ObjectOrErr.getError()) {
+  auto DiagID = Diags.getCustomDiagID(DiagnosticsE

[PATCH] D116635: Add warning to detect when calls passing arguments are made to functions without prototypes.

[Dan Liew via Phabricator via cfe-commits]

delcypher added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5529
+def warn_call_function_without_prototype : Warning<
+  "calling function %0 with arguments when function has no prototype">, 
InGroup<
+  DiagGroup<"strict-calls-without-prototype">>, DefaultIgnore;

aaron.ballman wrote:
> delcypher wrote:
> > aaron.ballman wrote:
> > > delcypher wrote:
> > > > aaron.ballman wrote:
> > > > > This diagnostic doesn't tell me what's wrong with the code (and in 
> > > > > fact, there's very possibly nothing wrong with the code whatsoever). 
> > > > > Further, why does calling a function *with no arguments* matter when 
> > > > > it has no prototype? I would imagine this should flag any call to a 
> > > > > function without a prototype given that the function without a 
> > > > > prototype may still expect arguments. e.g.,
> > > > > ```
> > > > > // Header.h
> > > > > int f();
> > > > > 
> > > > > // Source.c
> > > > > int f(a) int a; { ... }
> > > > > 
> > > > > // Caller.c
> > > > > #include "Header.h"
> > > > > 
> > > > > int main() {
> > > > >   return f();
> > > > > }
> > > > > ```
> > > > > I think the diagnostic text should be updated to something more like 
> > > > > `cannot verify %0 is being called with the correct number or 
> > > > > %plural{1:type|:types}1 of arguments because it was declared without 
> > > > > a prototype` (or something along those lines that explains what's 
> > > > > wrong with the code).
> > > > @aaron.ballman  Thanks for the helpful feedback.
> > > > 
> > > > > This diagnostic doesn't tell me what's wrong with the code (and in 
> > > > > fact, there's very possibly nothing wrong with the code whatsoever).
> > > > 
> > > > That's a fair criticism.  I think the diagnostic message you suggest is 
> > > > a lot more helpful so I'll go for something like that.
> > > > 
> > > > > Further, why does calling a function *with no arguments* matter when 
> > > > > it has no prototype?
> > > > 
> > > > The reason was to avoid the warning being noisy. E.g. I didn't the 
> > > > warning to fire in this situation.
> > > > 
> > > > ```
> > > > // Header.h
> > > > int f(); // The user forgot to put `void` between parentheses
> > > > 
> > > > // Source.c
> > > > int f(void) { ... }
> > > > 
> > > > // Caller.c
> > > > #include "Header.h"
> > > > 
> > > > int main() {
> > > >   return f();
> > > > }
> > > > ```
> > > > 
> > > > Forgetting to put `void` in the declaration of `f()` is a pretty common 
> > > > thing to do because a lot of people read `int f()` as declaring a 
> > > > function that takes no arguments (it does in C++ but not in C).
> > > > 
> > > > I don't want the warning to be noisy because I was planning on 
> > > > switching it on by default in open source and in a downstream use-case 
> > > > make it an error.
> > > > 
> > > > How about this as a compromise? Split the warning into two separate 
> > > > warnings
> > > > 
> > > > * `strict-call-without-prototype` -  Warns on calls to functions 
> > > > without a prototype when no arguments are passed. Not enabled by default
> > > > * `call-without-prototype` -Warns on calls to functions without a 
> > > > prototype when arguments are passed.  Enable this by default.
> > > > 
> > > > Alternatively we could enable both by default. That would still allow 
> > > > me to make `call-without-prototype` an error and 
> > > > `strict-call-without-prototype` not be an error for my downstream 
> > > > use-case.
> > > > 
> > > > Thoughts?
> > > > Forgetting to put void in the declaration of f() is a pretty common 
> > > > thing to do because a lot of people read int f() as declaring a 
> > > > function that takes no arguments (it does in C++ but not in C).
> > > 
> > > Yup, and this is exactly why I think we *should* be warning. That is a 
> > > function without a prototype, so the code is very brittle and dangerous 
> > > at the call site. The fact that the call site *currently* is correct 
> > > doesn't mean it's *intentionally* correct. e.g.,
> > > ```
> > > // Header.h
> > > int f(); // No prototype
> > > 
> > > // Source.c
> > > int f(int a, int b) { return 0; } // Has a prototype, no diagnostic
> > > 
> > > // OtherSource.c
> > > #include "Header.h"
> > > 
> > > int main() {
> > >   return f(); // No diagnostic with this patch, but still have the UB.
> > > }
> > > ```
> > > 
> > > > I don't want the warning to be noisy because I was planning on 
> > > > switching it on by default in open source and in a downstream use-case 
> > > > make it an error.
> > > 
> > > Hmmm. Thinking out loud here.
> > > 
> > > Functions without prototypes were standardized in C89 as a deprecated 
> > > feature (C89 3.9.4, 3.9.5). I'd like to get to the point where any code 
> > > that doesn't pass `-ansi` is given a diagnostic (at least in pedantic 
> > > mode outside of system headers) about this deprecation, though I could 
> > > probably be persuaded to keep not diagnose in c89 mode if that'

[PATCH] D118306: [CMake][Fuchsia] Drop 32-bit ios runtimes

[Petr Hosek 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 rG2fcdd685bd02: [CMake][Fuchsia] Drop 32-bit ios runtimes 
(authored by phosek).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118306

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -70,7 +70,7 @@
   set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
   set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
-  set(DARWIN_ios_ARCHS armv7;armv7s;arm64 CACHE STRING "")
+  set(DARWIN_ios_ARCHS arm64 CACHE STRING "")
   set(DARWIN_iossim_ARCHS arm64;x86_64 CACHE STRING "")
   set(DARWIN_osx_ARCHS arm64;x86_64 CACHE STRING "")
 endif()


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -70,7 +70,7 @@
   set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
   set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
-  set(DARWIN_ios_ARCHS armv7;armv7s;arm64 CACHE STRING "")
+  set(DARWIN_ios_ARCHS arm64 CACHE STRING "")
   set(DARWIN_iossim_ARCHS arm64;x86_64 CACHE STRING "")
   set(DARWIN_osx_ARCHS arm64;x86_64 CACHE STRING "")
 endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 2fcdd68 - [CMake][Fuchsia] Drop 32-bit ios runtimes

[Petr Hosek via cfe-commits]

Author: Petr Hosek
Date: 2022-01-26T18:57:21-08:00
New Revision: 2fcdd685bd0213b75f45597b970ea9858a3a9d89

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

LOG: [CMake][Fuchsia] Drop 32-bit ios runtimes

These are no longer supported in the recent Xcode SDK versions.

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

Added: 


Modified: 
clang/cmake/caches/Fuchsia-stage2.cmake

Removed: 




diff  --git a/clang/cmake/caches/Fuchsia-stage2.cmake 
b/clang/cmake/caches/Fuchsia-stage2.cmake
index d998db64d87ab..bc60b0df6d4af 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -70,7 +70,7 @@ if(APPLE)
   set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
   set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
-  set(DARWIN_ios_ARCHS armv7;armv7s;arm64 CACHE STRING "")
+  set(DARWIN_ios_ARCHS arm64 CACHE STRING "")
   set(DARWIN_iossim_ARCHS arm64;x86_64 CACHE STRING "")
   set(DARWIN_osx_ARCHS arm64;x86_64 CACHE STRING "")
 endif()



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

[PATCH] D118306: [CMake][Fuchsia] Drop 32-bit ios runtimes

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr accepted this revision.
mcgrathr 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/D118306/new/

https://reviews.llvm.org/D118306

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

[PATCH] D118306: [CMake][Fuchsia] Drop 32-bit ios runtimes

[Petr Hosek via Phabricator via cfe-commits]

phosek created this revision.
phosek added reviewers: mcgrathr, leonardchan, haowei.
Herald added subscribers: abrachet, mgorny.
phosek requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

These are no longer supported in the recent Xcode SDK versions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118306

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -70,7 +70,7 @@
   set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
   set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
-  set(DARWIN_ios_ARCHS armv7;armv7s;arm64 CACHE STRING "")
+  set(DARWIN_ios_ARCHS arm64 CACHE STRING "")
   set(DARWIN_iossim_ARCHS arm64;x86_64 CACHE STRING "")
   set(DARWIN_osx_ARCHS arm64;x86_64 CACHE STRING "")
 endif()


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -70,7 +70,7 @@
   set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
   set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
   set(LIBCXX_ABI_VERSION 2 CACHE STRING "")
-  set(DARWIN_ios_ARCHS armv7;armv7s;arm64 CACHE STRING "")
+  set(DARWIN_ios_ARCHS arm64 CACHE STRING "")
   set(DARWIN_iossim_ARCHS arm64;x86_64 CACHE STRING "")
   set(DARWIN_osx_ARCHS arm64;x86_64 CACHE STRING "")
 endif()
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117924: [compiler_rt] Add a seperate runtime for Mac Catalyst

[Byoungchan Lee via Phabricator via cfe-commits]

bc-lee added a comment.

It may not be appropriate to add other runtime libraries specifically for Mac 
Catalyst. However, currently `lld` does not allow linking with dynamic 
libraries with different types of build_version_command, whereas `ld64` does 
allow for Mac Catalyst. Considering compatibility with lld, Mac Catalyst’s 
runtime libraries need to be added also.

See my other patch https://reviews.llvm.org/D117925 for more details.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117924

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

[PATCH] D118223: [NFC] [AST] Move isSame* check in ASTReader to ASTContext

[Chuanqi Xu 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 rGf85ee6d56a93: [NFC] [AST] Move isSameEntity into ASTContext 
(authored by ChuanqiXu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118223

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp

Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2945,391 +2945,6 @@
   return LocalOffset + M.GlobalBitOffset;
 }
 
-static bool isSameTemplateParameterList(const ASTContext &C,
-const TemplateParameterList *X,
-const TemplateParameterList *Y);
-static bool isSameEntity(NamedDecl *X, NamedDecl *Y);
-
-/// Determine whether two template parameters are similar enough
-/// that they may be used in declarations of the same template.
-static bool isSameTemplateParameter(const NamedDecl *X,
-const NamedDecl *Y) {
-  if (X->getKind() != Y->getKind())
-return false;
-
-  if (const auto *TX = dyn_cast(X)) {
-const auto *TY = cast(Y);
-if (TX->isParameterPack() != TY->isParameterPack())
-  return false;
-if (TX->hasTypeConstraint() != TY->hasTypeConstraint())
-  return false;
-const TypeConstraint *TXTC = TX->getTypeConstraint();
-const TypeConstraint *TYTC = TY->getTypeConstraint();
-if (!TXTC != !TYTC)
-  return false;
-if (TXTC && TYTC) {
-  auto *NCX = TXTC->getNamedConcept();
-  auto *NCY = TYTC->getNamedConcept();
-  if (!NCX || !NCY || !isSameEntity(NCX, NCY))
-return false;
-  if (TXTC->hasExplicitTemplateArgs() != TYTC->hasExplicitTemplateArgs())
-return false;
-  if (TXTC->hasExplicitTemplateArgs()) {
-const auto *TXTCArgs = TXTC->getTemplateArgsAsWritten();
-const auto *TYTCArgs = TYTC->getTemplateArgsAsWritten();
-if (TXTCArgs->NumTemplateArgs != TYTCArgs->NumTemplateArgs)
-  return false;
-llvm::FoldingSetNodeID XID, YID;
-for (const auto &ArgLoc : TXTCArgs->arguments())
-  ArgLoc.getArgument().Profile(XID, X->getASTContext());
-for (const auto &ArgLoc : TYTCArgs->arguments())
-  ArgLoc.getArgument().Profile(YID, Y->getASTContext());
-if (XID != YID)
-  return false;
-  }
-}
-return true;
-  }
-
-  if (const auto *TX = dyn_cast(X)) {
-const auto *TY = cast(Y);
-return TX->isParameterPack() == TY->isParameterPack() &&
-   TX->getASTContext().hasSameType(TX->getType(), TY->getType());
-  }
-
-  const auto *TX = cast(X);
-  const auto *TY = cast(Y);
-  return TX->isParameterPack() == TY->isParameterPack() &&
- isSameTemplateParameterList(TX->getASTContext(),
- TX->getTemplateParameters(),
- TY->getTemplateParameters());
-}
-
-static NamespaceDecl *getNamespace(const NestedNameSpecifier *X) {
-  if (auto *NS = X->getAsNamespace())
-return NS;
-  if (auto *NAS = X->getAsNamespaceAlias())
-return NAS->getNamespace();
-  return nullptr;
-}
-
-static bool isSameQualifier(const NestedNameSpecifier *X,
-const NestedNameSpecifier *Y) {
-  if (auto *NSX = getNamespace(X)) {
-auto *NSY = getNamespace(Y);
-if (!NSY || NSX->getCanonicalDecl() != NSY->getCanonicalDecl())
-  return false;
-  } else if (X->getKind() != Y->getKind())
-return false;
-
-  // FIXME: For namespaces and types, we're permitted to check that the entity
-  // is named via the same tokens. We should probably do so.
-  switch (X->getKind()) {
-  case NestedNameSpecifier::Identifier:
-if (X->getAsIdentifier() != Y->getAsIdentifier())
-  return false;
-break;
-  case NestedNameSpecifier::Namespace:
-  case NestedNameSpecifier::NamespaceAlias:
-// We've already checked that we named the same namespace.
-break;
-  case NestedNameSpecifier::TypeSpec:
-  case NestedNameSpecifier::TypeSpecWithTemplate:
-if (X->getAsType()->getCanonicalTypeInternal() !=
-Y->getAsType()->getCanonicalTypeInternal())
-  return false;
-break;
-  case NestedNameSpecifier::Global:
-  case NestedNameSpecifier::Super:
-return true;
-  }
-
-  // Recurse into earlier portion of NNS, if any.
-  auto *PX = X->getPrefix();
-  auto *PY = Y->getPrefix();
-  if (PX && PY)
-return isSameQualifier(PX, PY);
-  return !PX && !PY;
-}
-
-/// Determine whether two template parameter lists are similar enough
-/// that they may be used in declarations of the same template.
-static bool isSameTemplateParameterList(const ASTContext &C,
-  

[clang] f85ee6d - [NFC] [AST] Move isSameEntity into ASTContext

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2022-01-27T10:30:50+08:00
New Revision: f85ee6d56a9371ccb8b3a63b6f9ed38ba8d071b7

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

LOG: [NFC] [AST] Move isSameEntity into ASTContext

Currently we are trying to implement the semantics of C++ Modules. A big
challenge would be the ODR checking. Previously we did this in
ASTReader, it would handle the cases like:
```
module;
export module a_module;
import another_module; //  check the ODR consistency here
```
or
```
export module m;
import a_module;
import another_module; // check the ODR consistency here
```

However, it wouldn't handle the case:
```
import another_module; // When we check ODR here, everything looks fine.
```

In the case, the read process is ended. But we need to check the ODR
still. To reuse the facility we do in ASTReader, this patch moves the
corresponding codes into ASTContext. This should be good since there
were facilities like `hasSameTemplateName` and `hasSameType`.

Although the patch is a little bit big, all of the change should be
trivial.

Reviewed By: erichkeane

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

Added: 


Modified: 
clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
clang/lib/Serialization/ASTReaderDecl.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index ed35e73ce4cf9..f39ce14bc82cb 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -2622,6 +2622,18 @@ class ASTContext : public RefCountedBase {
   /// template.
   bool hasSameTemplateName(TemplateName X, TemplateName Y);
 
+  /// Determine whether the two declarations refer to the same entity.
+  bool isSameEntity(NamedDecl *X, NamedDecl *Y);
+
+  /// Determine whether two template parameter lists are similar enough
+  /// that they may be used in declarations of the same template.
+  bool isSameTemplateParameterList(TemplateParameterList *X,
+   TemplateParameterList *Y);
+
+  /// Determine whether two template parameters are similar enough
+  /// that they may be used in declarations of the same template.
+  bool isSameTemplateParameter(NamedDecl *X, NamedDecl *Y);
+
   /// Retrieve the "canonical" template argument.
   ///
   /// The canonical template argument is the simplest template argument

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 9d63724c919a0..8a780250b6d8e 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -6147,6 +6147,376 @@ bool ASTContext::hasSameTemplateName(TemplateName X, 
TemplateName Y) {
   return X.getAsVoidPointer() == Y.getAsVoidPointer();
 }
 
+bool ASTContext::isSameTemplateParameter(NamedDecl *X, NamedDecl *Y) {
+  if (X->getKind() != Y->getKind())
+return false;
+
+  if (auto *TX = dyn_cast(X)) {
+auto *TY = cast(Y);
+if (TX->isParameterPack() != TY->isParameterPack())
+  return false;
+if (TX->hasTypeConstraint() != TY->hasTypeConstraint())
+  return false;
+const TypeConstraint *TXTC = TX->getTypeConstraint();
+const TypeConstraint *TYTC = TY->getTypeConstraint();
+if (!TXTC != !TYTC)
+  return false;
+if (TXTC && TYTC) {
+  auto *NCX = TXTC->getNamedConcept();
+  auto *NCY = TYTC->getNamedConcept();
+  if (!NCX || !NCY || !isSameEntity(NCX, NCY))
+return false;
+  if (TXTC->hasExplicitTemplateArgs() != TYTC->hasExplicitTemplateArgs())
+return false;
+  if (TXTC->hasExplicitTemplateArgs()) {
+auto *TXTCArgs = TXTC->getTemplateArgsAsWritten();
+auto *TYTCArgs = TYTC->getTemplateArgsAsWritten();
+if (TXTCArgs->NumTemplateArgs != TYTCArgs->NumTemplateArgs)
+  return false;
+llvm::FoldingSetNodeID XID, YID;
+for (auto &ArgLoc : TXTCArgs->arguments())
+  ArgLoc.getArgument().Profile(XID, X->getASTContext());
+for (auto &ArgLoc : TYTCArgs->arguments())
+  ArgLoc.getArgument().Profile(YID, Y->getASTContext());
+if (XID != YID)
+  return false;
+  }
+}
+return true;
+  }
+
+  if (auto *TX = dyn_cast(X)) {
+auto *TY = cast(Y);
+return TX->isParameterPack() == TY->isParameterPack() &&
+   TX->getASTContext().hasSameType(TX->getType(), TY->getType());
+  }
+
+  auto *TX = cast(X);
+  auto *TY = cast(Y);
+  return TX->isParameterPack() == TY->isParameterPack() &&
+ isSameTemplateParameterList(TX->getTemplateParameters(),
+ TY->getTemplateParameters());
+}
+
+bool ASTContext::isSameTemplateParameterList(TemplateParameterList *X,
+ TemplateParameterList *Y) {
+  if (X->size() != Y

[PATCH] D118225: [RISCV] Decouple Zve* extensions.

[Jianjian Guan via Phabricator via cfe-commits]

jacquesguan added a comment.

In D118225#3272071 , @eopXD wrote:

> Not sure if this simplifies things. Users and the compiler  can use the macro 
> `__riscv_v_elen` and `__riscv_v_elen_fp` to do things to the vector-related 
> target feature.
>
> Other than that I don't hold any strong objection to this refactoring.

This patch is similar with https://reviews.llvm.org/D117854. I think these five 
Zve* extensions are independent, we do not need to implement Zve32x for Zve64x, 
even all instructions of Zve32x is subset of instructions of Zve64x, same with 
other dependencies.
Current implemention of Zve* with depedency make some unclear. For example, in 
`clang/test/CodeGen/RISCV/rvv-intrinsics/rvv-error.c`, the error message shows 
that the buitin only needs Zve32x or V  extension to be enable. But actually, 
if we just implement Zve64x, these builtins should work as well. These error 
messages is kind of confusing if we make Zve* extensions has depedent 
relationship.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118225

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

[PATCH] D117924: [compiler_rt] Add a seperate runtime for Mac Catalyst

[Dan Liew via Phabricator via cfe-commits]

delcypher added a comment.

@bc-lee Thanks for the patch. While I get what you're trying to do I have 
doubts about being able to accept the patch in its current form. Apple's ASan 
catalyst doesn't work by building a separate dylib, instead it builds the 
macosx dylib in a different way to make it work with catalyst code.

I've made @aralisza a blocking reviewer because she'll need to approve this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117924

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

[PATCH] D117593: [clang-tidy] Change google-explicit-constructor to ignore conversions and operators marked `explicit(false)`

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D117593#3273562 , @aaron.ballman 
wrote:

> Hmmm, this is a rule for checking a specific style guide. 
> https://google.github.io/styleguide/cppguide.html#Implicit_Conversions 
> doesn't say that `explicit(false)` is a reasonable marking. In fact, it says 
> "Implicit conversions can sometimes be necessary and appropriate for types 
> that are designed to be interchangeable, for example when objects of two 
> types are just different representations of the same underlying value. In 
> that case, contact your project leads to request a waiver of this rule."
>
> So I think this behavior needs to be behind a flag so that the default 
> behavior continues to match what the style guide requires (or the style guide 
> should be updated to clarify the behavior of `explicit(false)`).

My understanding of the rule(as a non Googler) was that `explicit(false)` is an 
effective way to disable the rule by explicitly declaring the constructor to be 
implicit. Which is much cleaner than throwing `NOLINT` markers about.
In any case this c++20 syntax is not supported as the current fixit produces 
invalid code, as evidenced in the initial bug report.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117593

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

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

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

rsmith added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:239
 
+- gcc doesn't pack non-packed non-pod members in packed structs. Clang
+  historically did perform such packing. Clang now matches the gcc behavior

"non-packed" here seems a little unclear. Does this mean "GCC doesn't pack 
non-POD members in packed structs unless the packed attribute is also specified 
on the member." ?

(Also I believe the preferred spelling for GCC is capitalized.)



Comment at: clang/docs/ReleaseNotes.rst:242
+  (except on Darwin and PS4). You can switch back to the old ABI behavior with
+  the flag: -fclang-abi-compat=13.0.
+





Comment at: clang/include/clang/Basic/LangOptions.h:185
+/// Attempt to be ABI-compatible with code generated by Clang 13.0.x.
+/// This causes clang to apply to pack non-pod members of packed structs.
+Ver13,





Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1891
+  llvm::Triple Target = Context.getTargetInfo().getTriple();
+  bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() ||
+ Context.getLangOpts().getClangABICompat() <=

`isPOD` is C++ standard specific, and our ABI rule really shouldn't be. Does 
GCC use the C++98 rules here, the C++11 rules, or something else? (Hopefully 
the GCC behavior doesn't change between `-std=c++98` and `-std=c++11`!)

From a quick test, it looks like GCC doesn't pack fields whose types are 
classes with base classes, even if they're trivially-copyable and 
standard-layout, suggesting that it's probably using the C++98 POD rules.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1895
+ Target.isPS4() || Target.isOSDarwin())) ||
+ D->hasAttr();
 

Would it make sense to warn here if `Packed && !FieldPacked`, like GCC does?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117616

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

[clang] 9c62728 - Default to DWARFv4 on Windows

[David Blaikie via cfe-commits]

Author: David Blaikie
Date: 2022-01-26T18:01:07-08:00
New Revision: 9c6272861032f511a23784ce0c5cc8f6ac2f625b

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

LOG: Default to DWARFv4 on Windows

Added: 


Modified: 
clang/lib/Driver/ToolChains/MSVC.h
clang/test/CodeGen/dwarf-version.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/MSVC.h 
b/clang/lib/Driver/ToolChains/MSVC.h
index 8f033de09bf64..c842773996eda 100644
--- a/clang/lib/Driver/ToolChains/MSVC.h
+++ b/clang/lib/Driver/ToolChains/MSVC.h
@@ -69,6 +69,10 @@ class LLVM_LIBRARY_VISIBILITY MSVCToolChain : public 
ToolChain {
 return llvm::DebuggerKind::Default;
   }
 
+  unsigned GetDefaultDwarfVersion() const override {
+return 4;
+  }
+
   enum class SubDirectoryType {
 Bin,
 Include,

diff  --git a/clang/test/CodeGen/dwarf-version.c 
b/clang/test/CodeGen/dwarf-version.c
index b329556ae0d9d..47025e241d13c 100644
--- a/clang/test/CodeGen/dwarf-version.c
+++ b/clang/test/CodeGen/dwarf-version.c
@@ -28,10 +28,10 @@
 // RUN: | FileCheck %s --check-prefixes=NODWARF,CODEVIEW
 // Explicitly request DWARF.
 // RUN: %clang -target i686-pc-windows-msvc -gdwarf -S -emit-llvm -o - %s \
-// RUN: | FileCheck %s --check-prefixes=VER5,NOCODEVIEW
+// RUN: | FileCheck %s --check-prefixes=VER4,NOCODEVIEW
 // Explicitly request both.
 // RUN: %clang -target i686-pc-windows-msvc -gdwarf -gcodeview -S -emit-llvm 
-o - %s \
-// RUN: | FileCheck %s --check-prefixes=VER5,CODEVIEW
+// RUN: | FileCheck %s --check-prefixes=VER4,CODEVIEW
 // RUN: %clang -target powerpc-ibm-aix-xcoff -g -S -emit-llvm -o - %s | \
 // RUN:   FileCheck %s --check-prefix=VER3
 // RUN: %clang -target powerpc-ibm-aix-xcoff -gdwarf-2 -S -emit-llvm -o - %s | 
\



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

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added inline comments.



Comment at: clang/lib/CodeGen/Address.h:70
+unsigned AlignLog = (Pointer.getInt() << 3) | ElementType.getInt();
+return CharUnits::fromQuantity(1UL << AlignLog);
+  }

LegalizeAdulthood wrote:
> This is causing warnings to be emitted:
> ```
> clang\lib\CodeGen\Address.h(69,1): warning C4334: '<<': result of 32-bit 
> shift implicitly converted to 64 bits (was 64-bit shift intended?) 
> [D:\legalize\llvm\llvm-project\build\tools\clang\lib\CodeGen\obj.clangCodeGen.vcxproj]
> ```
Should be fixed by eee97f1617c94b8


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117262

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

[clang] eee97f1 - [clang] Use proper type to left shift after D117262

[Arthur Eubanks via cfe-commits]

Author: Arthur Eubanks
Date: 2022-01-26T17:54:37-08:00
New Revision: eee97f1617c94b88e9f0a964c47dc6003173f818

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

LOG: [clang] Use proper type to left shift after D117262

Causing warnings like
warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits
as reported in D117262.

Added: 


Modified: 
clang/lib/CodeGen/Address.h

Removed: 




diff  --git a/clang/lib/CodeGen/Address.h b/clang/lib/CodeGen/Address.h
index 6a22e567d399a..3ac0f4f0d7e56 100644
--- a/clang/lib/CodeGen/Address.h
+++ b/clang/lib/CodeGen/Address.h
@@ -66,7 +66,7 @@ template  class AddressImpl {
   llvm::Type *getElementType() const { return ElementType.getPointer(); }
   CharUnits getAlignment() const {
 unsigned AlignLog = (Pointer.getInt() << 3) | ElementType.getInt();
-return CharUnits::fromQuantity(1UL << AlignLog);
+return CharUnits::fromQuantity(CharUnits::QuantityType(1) << AlignLog);
   }
 };
 



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

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: clang/lib/CodeGen/Address.h:70
+unsigned AlignLog = (Pointer.getInt() << 3) | ElementType.getInt();
+return CharUnits::fromQuantity(1UL << AlignLog);
+  }

This is causing warnings to be emitted:
```
clang\lib\CodeGen\Address.h(69,1): warning C4334: '<<': result of 32-bit shift 
implicitly converted to 64 bits (was 64-bit shift intended?) 
[D:\legalize\llvm\llvm-project\build\tools\clang\lib\CodeGen\obj.clangCodeGen.vcxproj]
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117262

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

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

I didn't find any mention of Darwin in RecordLayoutBuilder, which surprised me 
- I thought maybe Darwin was using the ClangABICompat to express its ABI 
compatibility requirements (which would sort of be handy rather than having to 
test multiple target features, for those targets that define their ABI by 
Clang's behavior - though I guess even those targets might be willing to take 
some ABI breaks against themselves sometimes (looks like they have, 
historically - looking at the other ClangABICompat values and where/how they're 
tested - I don't see target tests against Darwin and PS4 for all of them)) but 
doesn't look like it.

So - should I be testing Darwin/MacOS/something support differently than I have?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117616

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

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

[David Blaikie via Phabricator via cfe-commits]

dblaikie updated this revision to Diff 403449.
dblaikie added a comment.
Herald added a subscriber: dexonsmith.

Relnotes
Opt out PS4 and Darwin
Add support for -fclang-abi-compat=13


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117616

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/LangOptions.h
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/SemaCXX/class-layout.cpp

Index: clang/test/SemaCXX/class-layout.cpp
===
--- clang/test/SemaCXX/class-layout.cpp
+++ clang/test/SemaCXX/class-layout.cpp
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++98 -Wno-inaccessible-base
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base
+// RUN: %clang_cc1 -triple x86_64-apple-darwin%s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=13
+// RUN: %clang_cc1 -triple x86_64-scei-ps4%s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -DCLANG_ABI_COMPAT=6
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=6 -DCLANG_ABI_COMPAT=6
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=13 -DCLANG_ABI_COMPAT=13
 // expected-no-diagnostics
 
 #define SA(n, p) int a##n[(p) ? 1 : -1]
@@ -604,3 +607,33 @@
 #endif
 #pragma pack(pop)
 }
+
+namespace non_pod {
+struct t1 {
+protected:
+  int a;
+};
+// GCC prints warning: ignoring packed attribute because of unpacked non-POD field 't1 t2::v1'`
+struct t2 {
+  t1 v1;
+} __attribute__((packed));
+#if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 13
+_Static_assert(_Alignof(t1) == 4, "");
+_Static_assert(_Alignof(t2) == 1, "");
+#else
+_Static_assert(_Alignof(t1) == 4, "");
+_Static_assert(_Alignof(t2) == 4, "");
+#endif
+} // namespace non_pod
+
+namespace non_pod_packed {
+struct t1 {
+protected:
+  int a;
+} __attribute__((packed));
+struct t2 {
+  t1 v1;
+} __attribute__((packed));
+_Static_assert(_Alignof(t1) == 1, "");
+_Static_assert(_Alignof(t2) == 1, "");
+} // namespace non_pod_packed
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3559,6 +3559,8 @@
 GenerateArg(Args, OPT_fclang_abi_compat_EQ, "11.0", SA);
   else if (Opts.getClangABICompat() == LangOptions::ClangABI::Ver12)
 GenerateArg(Args, OPT_fclang_abi_compat_EQ, "12.0", SA);
+  else if (Opts.getClangABICompat() == LangOptions::ClangABI::Ver13)
+GenerateArg(Args, OPT_fclang_abi_compat_EQ, "13.0", SA);
 
   if (Opts.getSignReturnAddressScope() ==
   LangOptions::SignReturnAddressScopeKind::All)
@@ -4061,6 +4063,8 @@
 Opts.setClangABICompat(LangOptions::ClangABI::Ver11);
   else if (Major <= 12)
 Opts.setClangABICompat(LangOptions::ClangABI::Ver12);
+  else if (Major <= 13)
+Opts.setClangABICompat(LangOptions::ClangABI::Ver13);
 } else if (Ver != "latest") {
   Diags.Report(diag::err_drv_invalid_value)
   << A->getAsString(Args) << A->getValue();
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1887,7 +1887,12 @@
   UnfilledBitsInLastUnit = 0;
   LastBitfieldStorageUnitSize = 0;
 
-  bool FieldPacked = Packed || D->hasAttr();
+  llvm::Triple Target = Context.getTargetInfo().getTriple();
+  bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() ||
+ Context.getLangOpts().getClangABICompat() <=
+ LangOptions::ClangABI::Ver13 ||
+ Target.isPS4() || Target.isOSDarwin())) ||
+ D->hasAttr();
 
   AlignRequirementKind AlignRequirement = AlignRequirementKind::None;
   CharUnits FieldSize;
Index: clang/include/clang/Basic/LangOptions.h
===
--- clang/include/clang/Basic/LangOptions.h
+++ clang/include/clang/Basic/LangOptions.h
@@ -181,6 +181,10 @@
 /// global-scope inline variables incorrectly.
 Ver12,
 
+/// Attempt to be ABI-compatible with code generated by Clang 13.0.x.
+/// This causes clang to apply to pack non-pod members of packed structs.
+Ver13,
+
 /// Conform to the underlying platform's C and C++ ABIs as closely
 /// as we can.
 Latest
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -236,6 +236,

[PATCH] D114413: [OpenMPIRBuilder] Implement static-chunked workshare-loop schedules.

[Peixin Qiao via Phabricator via cfe-commits]

peixin added a comment.

Can you rebase this? I cannot apply this patch on current main branch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114413

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

[PATCH] D118297: [clang] add Diag -Wasm-volatile for implied volatile asm stmts

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Parse/ParseStmtAsm.cpp:788
+else if (NumOutputs == 0)
+  Diag(Loc, diag::warn_asm_volatile);
+  }

`Loc` is not correct if `volatile` is not the first asm-qualifier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118297

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

[PATCH] D118297: [clang] add Diag -Wasm-volatile for implied volatile asm stmts

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
Herald added subscribers: frasercrmck, luismarques, apazos, sameer.abuasal, 
pengfei, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, 
rogfer01, edward-jones, zzheng, jrtc27, niosHD, sabuasal, sunfish, simoncook, 
johnrusso, rbar, asb, dylanmckay.
nickdesaulniers requested review of this revision.
Herald added subscribers: Sanitizers, cfe-commits, pcwang-thead, aheejin.
Herald added projects: clang, Sanitizers.

According to the GCC docs on inline asm
https://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Volatile

> asm statements that have no output operands and asm goto statements,
> are implicitly volatile.

We tend to see the volatile qualifier tacked on to asm statements "just
to be safe" when in reality it makes no difference in IR for `asm goto`
statements or output-less `asm`.

Really the `volatile` asm-qualifier exists only to signal that an asm
statement should not be DCE'd (when it has outputs but they are unused),
CSE'd, or LICM'd. It is not a general compiler barrier.

Create a new warning, `-Wasm-volatile` and warn in either cases.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118297

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/Headers/x86gprintrin.h
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/test/CodeGen/X86/amx_inline_asm.c
  clang/test/CodeGen/X86/x86-64-inline-asm.c
  clang/test/CodeGen/X86/x86_32-inline-asm.c
  clang/test/CodeGen/avr-unsupported-inline-asm-constraints.c
  clang/test/PCH/asm.h
  clang/test/Parser/asm-goto.c
  clang/test/Parser/asm-goto.cpp
  clang/test/Parser/asm-qualifiers.c
  clang/test/Parser/cxx-stmt.cpp
  clang/test/Sema/arm-asm.c
  clang/test/Sema/arm64-inline-asm.c
  clang/test/Sema/asm-goto.cpp
  clang/test/Sema/inline-asm-validate-riscv.c
  clang/test/Sema/inline-asm-validate-x86.c
  clang/test/Sema/riscv-asm.c
  compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h
  compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang_x86.h
  compiler-rt/lib/sanitizer_common/sanitizer_common.h
  compiler-rt/lib/sanitizer_common/tests/sanitizer_test_utils.h
  compiler-rt/lib/scudo/standalone/common.h
  compiler-rt/lib/scudo/standalone/tests/wrappers_cpp_test.cpp

Index: compiler-rt/lib/scudo/standalone/tests/wrappers_cpp_test.cpp
===
--- compiler-rt/lib/scudo/standalone/tests/wrappers_cpp_test.cpp
+++ compiler-rt/lib/scudo/standalone/tests/wrappers_cpp_test.cpp
@@ -159,7 +159,7 @@
   char *P = new char[1UL << SizeLog];
   EXPECT_NE(P, nullptr);
   // Make sure this value is not optimized away.
-  asm volatile("" : : "r,m"(P) : "memory");
+  asm("" : : "r,m"(P) : "memory");
   delete[] P;
 }
   }
@@ -175,7 +175,7 @@
 char *P = new char[1UL << SizeLog];
 EXPECT_NE(P, nullptr);
 // Make sure this value is not optimized away.
-asm volatile("" : : "r,m"(P) : "memory");
+asm("" : : "r,m"(P) : "memory");
 // Make sure we can touch all of the allocation.
 memset(P, 0x32, 1U << SizeLog);
 // EXPECT_LE(1U << SizeLog, malloc_usable_size(ptr));
Index: compiler-rt/lib/scudo/standalone/common.h
===
--- compiler-rt/lib/scudo/standalone/common.h
+++ compiler-rt/lib/scudo/standalone/common.h
@@ -103,15 +103,15 @@
 
 inline void yieldProcessor(u8 Count) {
 #if defined(__i386__) || defined(__x86_64__)
-  __asm__ __volatile__("" ::: "memory");
+  __asm__("" ::: "memory");
   for (u8 I = 0; I < Count; I++)
-__asm__ __volatile__("pause");
+__asm__("pause");
 #elif defined(__aarch64__) || defined(__arm__)
-  __asm__ __volatile__("" ::: "memory");
+  __asm__("" ::: "memory");
   for (u8 I = 0; I < Count; I++)
-__asm__ __volatile__("yield");
+__asm__("yield");
 #endif
-  __asm__ __volatile__("" ::: "memory");
+  __asm__("" ::: "memory");
 }
 
 // Platform specific functions.
Index: compiler-rt/lib/sanitizer_common/tests/sanitizer_test_utils.h
===
--- compiler-rt/lib/sanitizer_common/tests/sanitizer_test_utils.h
+++ compiler-rt/lib/sanitizer_common/tests/sanitizer_test_utils.h
@@ -63,10 +63,10 @@
 #  define SANITIZER_WORDSIZE 32
 #endif
 
-// Make the compiler thinks that something is going on there.
+// Make the compiler think that something is going on there.
 inline void break_optimization(void *arg) {
 #if !defined(_WIN32) || defined(__clang__)
-  __asm__ __volatile__("" : : "r" (arg) : "memory");
+  __asm__("" : : "r"(arg) : "memory");
 #endif
 }
 
Index: compiler-rt/lib/sanitizer_common/sanitizer_common.h
===
--- compiler-rt/lib/sanitizer_common/sanitizer_common.h
+++ compiler-rt/lib/sanitizer_common/sanitizer_common.h
@@ -944,7 +944,7 @@
 #if defined(_MS

[PATCH] D117809: [clang] Add an extract-api driver option

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

@morehouse should be fixed in 98fa46f870e4 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117809

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

[clang] 98fa46f - [FIX][clang] Fix unused private field in ExtractAPIVisitor

[Zixu Wang via cfe-commits]

Author: Zixu Wang
Date: 2022-01-26T16:24:04-08:00
New Revision: 98fa46f870e402f225b60ab5d02487c36d79632b

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

LOG: [FIX][clang] Fix unused private field in ExtractAPIVisitor

Fix a build failure where an unused private field in ExtractAPIVisitor
triggered a warning turned into error.

Added: 


Modified: 
clang/lib/Frontend/ExtractAPIConsumer.cpp

Removed: 




diff  --git a/clang/lib/Frontend/ExtractAPIConsumer.cpp 
b/clang/lib/Frontend/ExtractAPIConsumer.cpp
index 92a0385c96cc0..cdf67f3c327aa 100644
--- a/clang/lib/Frontend/ExtractAPIConsumer.cpp
+++ b/clang/lib/Frontend/ExtractAPIConsumer.cpp
@@ -9,21 +9,14 @@ using namespace clang;
 namespace {
 class ExtractAPIVisitor : public RecursiveASTVisitor {
 public:
-  explicit ExtractAPIVisitor(ASTContext *Context) : Context(Context) {}
-
   bool VisitNamedDecl(NamedDecl *Decl) {
 llvm::outs() << Decl->getName() << "\n";
 return true;
   }
-
-private:
-  ASTContext *Context;
 };
 
 class ExtractAPIConsumer : public ASTConsumer {
 public:
-  explicit ExtractAPIConsumer(ASTContext *Context) : Visitor(Context) {}
-
   void HandleTranslationUnit(ASTContext &Context) override {
 Visitor.TraverseDecl(Context.getTranslationUnitDecl());
   }
@@ -35,5 +28,5 @@ class ExtractAPIConsumer : public ASTConsumer {
 
 std::unique_ptr
 ExtractAPIAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
-  return std::make_unique(&CI.getASTContext());
+  return std::make_unique();
 }



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

[PATCH] D117809: [clang] Add an extract-api driver option

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

On it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117809

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

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Daniele Castagna via Phabricator via cfe-commits]

dcastagna added a comment.

@yaxunl Are you OK landing this change as it is, without the check for OS and 
environment in getHIPOffloadTargetTriple?
We can follow up with patch that adds checks for in OS and environment in 
Triple.cpp. Is that what you meant?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117137

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

[PATCH] D118104: Make run-clang-tidy.py print the configured checks correctly

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py:163
 
 def run_tidy(args, tmpdir, build_path, queue, lock, failed_files):
   """Takes filenames out of queue and runs clang-tidy on them."""

LegalizeAdulthood wrote:
> Am I missing something, or is this function now unused?
Nope, still used in line 301.


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

https://reviews.llvm.org/D118104

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

[PATCH] D116543: [OpenMP] Embed device files into the host IR

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 403413.
jhuber6 added a comment.

Updating after changing flag in D116542 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116543

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/openmp-offload-gpu.c


Index: clang/test/Driver/openmp-offload-gpu.c
===
--- clang/test/Driver/openmp-offload-gpu.c
+++ clang/test/Driver/openmp-offload-gpu.c
@@ -353,3 +353,10 @@
 // NEW_DRIVER: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: 
["[[DEVICE_ASM]]"], output: "[[DEVICE_OBJ:.+]]" 
 // NEW_DRIVER: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[HOST_BC]]", 
"[[DEVICE_OBJ]]"], output: "[[HOST_OBJ:.+]]" 
 // NEW_DRIVER: "x86_64-unknown-linux-gnu" - "[[LINKER:.+]]", inputs: 
["[[HOST_OBJ]]"], output: "openmp-offload-gpu"
+
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda 
-Xopenmp-target=nvptx64-nvida-cuda -march=sm_70 \
+// RUN:  
--libomptarget-nvptx-bc-path=%S/Inputs/libomptarget/libomptarget-new-nvptx-test.bc
 \
+// RUN:  -fopenmp-new-driver -no-canonical-prefixes %s -o 
openmp-offload-gpu 2>&1 \
+// RUN:   | FileCheck -check-prefix=NEW_DRIVER_EMBEDDING %s
+
+// NEW_DRIVER_EMBEDDING: 
-fembed-offload-object=[[CUBIN:.*\.cubin]],nvptx64-nvidia-cuda.sm_70
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4365,9 +4365,9 @@
   IsHeaderModulePrecompile ? HeaderModuleInput : Inputs[0];
 
   InputInfoList ModuleHeaderInputs;
+  InputInfoList OpenMPHostInputs;
   const InputInfo *CudaDeviceInput = nullptr;
   const InputInfo *OpenMPDeviceInput = nullptr;
-  const InputInfo *OpenMPHostInput = nullptr;
   for (const InputInfo &I : Inputs) {
 if (&I == &Input) {
   // This is the primary input.
@@ -4384,8 +4384,8 @@
   CudaDeviceInput = &I;
 } else if (IsOpenMPDevice && !OpenMPDeviceInput) {
   OpenMPDeviceInput = &I;
-} else if (IsOpenMPHost && !OpenMPHostInput) {
-  OpenMPHostInput = &I;
+} else if (IsOpenMPHost) {
+  OpenMPHostInputs.push_back(I);
 } else {
   llvm_unreachable("unexpectedly given multiple inputs");
 }
@@ -6891,6 +6891,24 @@
 }
   }
 
+  // Host-side OpenMP offloading recieves the device object files and embeds it
+  // in a named section including the associated target triple and 
architecture.
+  if (IsOpenMPHost && !OpenMPHostInputs.empty()) {
+auto InputFile = OpenMPHostInputs.begin();
+auto OpenMPTCs = C.getOffloadToolChains();
+for (auto TI = OpenMPTCs.first, TE = OpenMPTCs.second; TI != TE;
+ ++TI, ++InputFile) {
+  const ToolChain *TC = TI->second;
+  const ArgList &TCArgs = C.getArgsForToolChain(TC, "", 
Action::OFK_OpenMP);
+  StringRef File =
+  C.getArgs().MakeArgString(TC->getInputFilename(*InputFile));
+
+  CmdArgs.push_back(Args.MakeArgString(
+  "-fembed-offload-object=" + File + "," + TC->getTripleString() + "." 
+
+  TCArgs.getLastArgValue(options::OPT_march_EQ)));
+}
+  }
+
   if (Triple.isAMDGPU()) {
 handleAMDGPUCodeObjectVersionOptions(D, Args, CmdArgs);
 


Index: clang/test/Driver/openmp-offload-gpu.c
===
--- clang/test/Driver/openmp-offload-gpu.c
+++ clang/test/Driver/openmp-offload-gpu.c
@@ -353,3 +353,10 @@
 // NEW_DRIVER: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[DEVICE_ASM]]"], output: "[[DEVICE_OBJ:.+]]" 
 // NEW_DRIVER: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[HOST_BC]]", "[[DEVICE_OBJ]]"], output: "[[HOST_OBJ:.+]]" 
 // NEW_DRIVER: "x86_64-unknown-linux-gnu" - "[[LINKER:.+]]", inputs: ["[[HOST_OBJ]]"], output: "openmp-offload-gpu"
+
+// RUN:   %clang -### -fopenmp=libomp -fopenmp-targets=nvptx64-nvidia-cuda -Xopenmp-target=nvptx64-nvida-cuda -march=sm_70 \
+// RUN:  --libomptarget-nvptx-bc-path=%S/Inputs/libomptarget/libomptarget-new-nvptx-test.bc \
+// RUN:  -fopenmp-new-driver -no-canonical-prefixes %s -o openmp-offload-gpu 2>&1 \
+// RUN:   | FileCheck -check-prefix=NEW_DRIVER_EMBEDDING %s
+
+// NEW_DRIVER_EMBEDDING: -fembed-offload-object=[[CUBIN:.*\.cubin]],nvptx64-nvidia-cuda.sm_70
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4365,9 +4365,9 @@
   IsHeaderModulePrecompile ? HeaderModuleInput : Inputs[0];
 
   InputInfoList ModuleHeaderInputs;
+  InputInfoList OpenMPHostInputs;
   const InputInfo *CudaDeviceInput = nullptr;
   const InputInfo *OpenMPDeviceInput = nullptr;
-  const InputInfo *OpenMPHostInput = nullptr;
   for (const InputInfo &I : Inputs) {
 if 

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D117137#3273330 , @tra wrote:

> In D117137#3269365 , @yaxunl wrote:
>
>> Does that mean only "spirv{64}-unknown-unknown" is acceptable, or 
>> "spirv{64}-amd-unknown-unknown" is also acceptable?
>
> My point is that `unknown` part of the triple is a catch-all for `anything, 
> including something invalid` and should not be used for positive checks.
> If we do not care about those parts of the triple ( accepting invalid triple 
> would imply it), then we should not check those parts at all. 
> Otherwise it leads to a weird inconsistency -- invalid triple like 
> `spirv64-foo-bar`is accepted, but an equally nonsensical triple like 
> `spir64-suse-whatever`which happens to use a known vendor or OS parts is not.
>
> The bottom line is that if there's currently no known use of the 
> vendor/OS/env parts of the triple, then we should not be checking them. 
> If we do want to accept specific triple, then appropriate enums should be 
> used/added.

I get your point. `TT.getVendor() == llvm::Triple::UnknownVendor`  and 
`TT.getOS() == llvm::Triple::UnknownOS` checks the processed vendor/OS string 
instead of the original string, which could be misleading.

Since SPIRV backend requires OS and environment to be unknown. It seems we'd 
better check the original OS and environment string in the Triple by splitting 
the triple by `-` and taking the 3rd and 4th element 
(https://github.com/llvm/llvm-project/blob/main/llvm/lib/Support/Triple.cpp#L795).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117137

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

[PATCH] D116542: [OpenMP] Add a flag for embedding a file into the module

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: llvm/lib/Bitcode/Writer/CMakeLists.txt:14
   MC
+  TransformUtils
   Object

I'm not sure if it's worth linking TransformUtils just for 
`appendToCompilerUsed` I can copy the implementation locally if not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116542

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

[PATCH] D116542: [OpenMP] Add a flag for embedding a file into the module

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 403404.
jhuber6 added a comment.
Herald added a subscriber: mgorny.

Updating approach, use a vector of string pairs now. Multiple files are simply
passed multiple times. Will add filename to the offloading section name laterf,
as similar sections could be merged when performing a relocatable link.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116542

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/CodeGen/BackendUtil.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/Frontend/embed-object.ll
  llvm/include/llvm/Bitcode/BitcodeWriter.h
  llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
  llvm/lib/Bitcode/Writer/CMakeLists.txt

Index: llvm/lib/Bitcode/Writer/CMakeLists.txt
===
--- llvm/lib/Bitcode/Writer/CMakeLists.txt
+++ llvm/lib/Bitcode/Writer/CMakeLists.txt
@@ -11,6 +11,7 @@
   Analysis
   Core
   MC
+  TransformUtils
   Object
   Support
   )
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -69,6 +69,7 @@
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/SHA1.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Transforms/Utils/ModuleUtils.h"
 #include 
 #include 
 #include 
@@ -4972,3 +4973,18 @@
   llvm::ConstantArray::get(ATy, UsedArray), "llvm.compiler.used");
   NewUsed->setSection("llvm.metadata");
 }
+
+void llvm::EmbedObjectInModule(llvm::Module &M, llvm::MemoryBufferRef Buf,
+   StringRef SectionName) {
+  ArrayRef ModuleData = ArrayRef(Buf.getBufferStart(), Buf.getBufferSize());
+
+  // Embed the data in the
+  llvm::Constant *ModuleConstant =
+  llvm::ConstantDataArray::get(M.getContext(), ModuleData);
+  llvm::GlobalVariable *GV = new llvm::GlobalVariable(
+  M, ModuleConstant->getType(), true, llvm::GlobalValue::PrivateLinkage,
+  ModuleConstant, "llvm.embedded.object");
+  GV->setSection(SectionName);
+
+  appendToCompilerUsed(M, GV);
+}
Index: llvm/include/llvm/Bitcode/BitcodeWriter.h
===
--- llvm/include/llvm/Bitcode/BitcodeWriter.h
+++ llvm/include/llvm/Bitcode/BitcodeWriter.h
@@ -165,6 +165,11 @@
 bool EmbedCmdline,
 const std::vector &CmdArgs);
 
+  /// Embeds the memory buffer \p Buf into the module \p M as a global using the
+  /// section name \p SectionName.
+  void EmbedObjectInModule(Module &M, MemoryBufferRef Buf,
+   StringRef SectionName);
+
 } // end namespace llvm
 
 #endif // LLVM_BITCODE_BITCODEWRITER_H
Index: clang/test/Frontend/embed-object.ll
===
--- /dev/null
+++ clang/test/Frontend/embed-object.ll
@@ -0,0 +1,13 @@
+; RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm \
+; RUN:-fembed-offload-object=%S/Inputs/empty.h,section -x ir %s -o - \
+; RUN:| FileCheck %s -check-prefix=CHECK
+
+; CHECK: @llvm.embedded.object = private constant [0 x i8] zeroinitializer, section ".llvm.offloading.section"
+; CHECK: @llvm.compiler.used = appending global [2 x i8*] [i8* @x, i8* getelementptr inbounds ([0 x i8], [0 x i8]* @llvm.embedded.object, i32 0, i32 0)], section "llvm.metadata"
+
+@x = private constant i8 1
+@llvm.compiler.used = appending global [1 x i8*] [i8* @x], section "llvm.metadata"
+
+define i32 @foo() {
+  ret i32 0
+}
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -1134,6 +1134,7 @@
 TheModule->setTargetTriple(TargetOpts.Triple);
   }
 
+  EmbedBinary(TheModule.get(), CodeGenOpts, Diagnostics);
   EmbedBitcode(TheModule.get(), CodeGenOpts, *MainFile);
 
   LLVMContext &Ctx = TheModule->getContext();
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -1745,8 +1745,31 @@
  llvm::MemoryBufferRef Buf) {
   if (CGOpts.getEmbedBitcode() == CodeGenOptions::Embed_Off)
 return;
+
   llvm::EmbedBitcodeInModule(
   *M, Buf, CGOpts.getEmbedBitcode() != CodeGenOptions::Embed_Marker,
   CGOpts.getEmbedBitcode() != CodeGenOptions::Embed_Bitcode,
   CGOpts.CmdArgs);
 }
+
+void clang::EmbedBinary(llvm::Module *M, const CodeGenOptions &CGOpts,
+DiagnosticsEngine &Diags) {
+  if (CGOpts.OffloadObjects.empty())
+return;
+
+  for (StringRef OffloadObject : CGOpts.OffloadObjects) {
+auto FilenameAndSection = Offl

[PATCH] D114639: Raise the minimum Visual Studio version to VS2019

[Stella Stamenova via Phabricator via cfe-commits]

stella.stamenova added a comment.

In D114639#3273831 , @RKSimon wrote:

> In D114639#3273761 , 
> @stella.stamenova wrote:
>
>> Can we update the lldb instructions as part of this change to also point to 
>> VS2019? Alternatively, I can send a separate change for that.
>
> I'll add it to this patch - are people happy for me to get this in before the 
> 14.x branch?

Sounds good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114639

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

[PATCH] D114639: Raise the minimum Visual Studio version to VS2019

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

In D114639#3273761 , 
@stella.stamenova wrote:

> Can we update the lldb instructions as part of this change to also point to 
> VS2019? Alternatively, I can send a separate change for that.

I'll add it to this patch - are people happy for me to get this in before the 
14.x branch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114639

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

[PATCH] D117809: [clang] Add an extract-api driver option

[Matt Morehouse via Phabricator via cfe-commits]

morehouse added inline comments.



Comment at: clang/lib/Frontend/ExtractAPIConsumer.cpp:20
+private:
+  ASTContext *Context;
+};

`ASTContext *Context` is unused, and causing buildbot failures:  
https://lab.llvm.org/buildbot/#/builders/77/builds/13645

```
/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/clang/lib/Frontend/ExtractAPIConsumer.cpp:20:15:
 error: private field 'Context' is not used [-Werror,-Wunused-private-field]
  ASTContext *Context;
  ^
1 error generated.
```

Please fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117809

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

[PATCH] D118104: Make run-clang-tidy.py print the configured checks correctly

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: clang-tools-extra/clang-tidy/tool/run-clang-tidy.py:163
 
 def run_tidy(args, tmpdir, build_path, queue, lock, failed_files):
   """Takes filenames out of queue and runs clang-tidy on them."""

Am I missing something, or is this function now unused?


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

https://reviews.llvm.org/D118104

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

[PATCH] D114639: Raise the minimum Visual Studio version to VS2019

[Stella Stamenova via Phabricator via cfe-commits]

stella.stamenova added a comment.

Can we update the lldb instructions as part of this change to also point to 
VS2019? Alternatively, I can send a separate change for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114639

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

[clang] 6bc20eb - [cc1as] Remove -Wa, --compress-debug-sections=zlib-gnu

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2022-01-26T13:28:51-08:00
New Revision: 6bc20eb13470514c67e9ee5bbd515c2197ce8ea8

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

LOG: [cc1as] Remove -Wa,--compress-debug-sections=zlib-gnu

It's obsoleted and unlikely used. See D117744.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Gnu.cpp
clang/test/Driver/compress-noias.c
clang/tools/driver/cc1as_main.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index fbb1af4dbe22b..7a9570a686f4b 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -694,7 +694,7 @@ void tools::gnutools::Assembler::ConstructJob(Compilation 
&C,
   CmdArgs.push_back("--compress-debug-sections");
 } else {
   StringRef Value = A->getValue();
-  if (Value == "none" || Value == "zlib" || Value == "zlib-gnu") {
+  if (Value == "none" || Value == "zlib") {
 CmdArgs.push_back(
 Args.MakeArgString("--compress-debug-sections=" + Twine(Value)));
   } else {

diff  --git a/clang/test/Driver/compress-noias.c 
b/clang/test/Driver/compress-noias.c
index 40ec80acd761e..d0d7ef34e70b8 100644
--- a/clang/test/Driver/compress-noias.c
+++ b/clang/test/Driver/compress-noias.c
@@ -27,10 +27,6 @@
 // RUN: %clang -### -target i686-unknown-linux-gnu -fno-integrated-as -gz=zlib 
-c %s 2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_ZLIB %s
 // CHECK-OPT_GZ_EQ_ZLIB: "--compress-debug-sections=zlib"
 
-// RUN: %clang -### -target i686-unknown-linux-gnu -fno-integrated-as 
-gz=zlib-gnu -x assembler -c %s 2>&1 | FileCheck -check-prefix 
CHECK-OPT_GZ_EQ_ZLIB_GNU %s
-// RUN: %clang -### -target i686-unknown-linux-gnu -fno-integrated-as 
-gz=zlib-gnu -c %s 2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_ZLIB_GNU %s
-// CHECK-OPT_GZ_EQ_ZLIB_GNU: "--compress-debug-sections=zlib-gnu"
-
 // RUN: %clang -### -target i686-unknown-linux-gnu -fno-integrated-as 
-gz=invalid -x assembler -c %s 2>&1 | FileCheck -check-prefix 
CHECK-OPT_GZ_EQ_INVALID %s
 // RUN: %clang -### -target i686-unknown-linux-gnu -fno-integrated-as 
-gz=invalid -c %s 2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_INVALID %s
 // CHECK-OPT_GZ_EQ_INVALID: error: unsupported argument 'invalid' to option 
'gz='

diff  --git a/clang/tools/driver/cc1as_main.cpp 
b/clang/tools/driver/cc1as_main.cpp
index db3288d752815..6459d1534b39e 100644
--- a/clang/tools/driver/cc1as_main.cpp
+++ b/clang/tools/driver/cc1as_main.cpp
@@ -228,7 +228,6 @@ bool 
AssemblerInvocation::CreateFromArgs(AssemblerInvocation &Opts,
 llvm::StringSwitch(A->getValue())
 .Case("none", llvm::DebugCompressionType::None)
 .Case("zlib", llvm::DebugCompressionType::Z)
-.Case("zlib-gnu", llvm::DebugCompressionType::GNU)
 .Default(llvm::DebugCompressionType::None);
   }
 



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

[PATCH] D118010: [clang-tidy] Fix nested namespaces in `readability-static-definition-in-anonymous-namespace` check

[Evgeny Shulgin via Phabricator via cfe-commits]

Izaron added a comment.

In D118010#3273716 , 
@LegalizeAdulthood wrote:

> If there is an open github issue on this, please put "Fixes #" in the 
> commit description before pushing.

Thanks! I didn't file a github issue. Just searched on github - no one else did 
it either.

I don't have commit access, so this review could be commited by someone else if 
that's okay.
If someone is willing to do it, kindly please do it on my behalf =) My name is 
`Evgeny Shulgin`, email `izaronpl...@gmail.com`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118010

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

[PATCH] D117744: [Driver] Remove obsoleted -gz=zlib-gnu

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG35d15222c05d: [Driver] Remove obsoleted -gz=zlib-gnu 
(authored by MaskRay).

Changed prior to commit:
  https://reviews.llvm.org/D117744?vs=401498&id=403395#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117744

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/amdgcn-gz-options.cl
  clang/test/Driver/compress.c


Index: clang/test/Driver/compress.c
===
--- clang/test/Driver/compress.c
+++ clang/test/Driver/compress.c
@@ -29,11 +29,6 @@
 // CHECK-OPT_GZ_EQ_ZLIB: {{.* "-cc1(as)?".* "--compress-debug-sections=zlib"}}
 // CHECK-OPT_GZ_EQ_ZLIB: "--compress-debug-sections=zlib"
 
-// RUN: %clang -### -target x86_64-unknown-linux-gnu -gz=zlib-gnu -x assembler 
%s 2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_ZLIB_GNU %s
-// RUN: %clang -### -target x86_64-unknown-linux-gnu -gz=zlib-gnu %s 2>&1 | 
FileCheck -check-prefix CHECK-OPT_GZ_EQ_ZLIB_GNU %s
-// CHECK-OPT_GZ_EQ_ZLIB_GNU: {{.* "-cc1(as)?".* 
"--compress-debug-sections=zlib-gnu"}}
-// CHECK-OPT_GZ_EQ_ZLIB_GNU: "--compress-debug-sections=zlib-gnu"
-
 // RUN: %clang -### -fintegrated-as -gz=invalid -x assembler -c %s 2>&1 | 
FileCheck -check-prefix CHECK-OPT_GZ_EQ_INVALID %s
 // RUN: %clang -### -fintegrated-as -gz=invalid -c %s 2>&1 | FileCheck 
-check-prefix CHECK-OPT_GZ_EQ_INVALID %s
 // CHECK-OPT_GZ_EQ_INVALID: error: unsupported argument 'invalid' to option 
'gz='
Index: clang/test/Driver/amdgcn-gz-options.cl
===
--- clang/test/Driver/amdgcn-gz-options.cl
+++ clang/test/Driver/amdgcn-gz-options.cl
@@ -9,8 +9,3 @@
 // RUN: %clang -### -target amdgcn-amd-amdhsa -gz=zlib %s 2>&1 | FileCheck 
-check-prefix CHECK-OPT_GZ_EQ_ZLIB %s
 // CHECK-OPT_GZ_EQ_ZLIB: {{.* "-cc1(as)?".* "--compress-debug-sections=zlib"}}
 // CHECK-OPT_GZ_EQ_ZLIB: "--compress-debug-sections=zlib"
-
-// RUN: %clang -### -target amdgcn-amd-amdhsa -gz=zlib-gnu -x assembler %s 
2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_ZLIB_GNU %s
-// RUN: %clang -### -target amdgcn-amd-amdhsa -gz=zlib-gnu %s 2>&1 | FileCheck 
-check-prefix CHECK-OPT_GZ_EQ_ZLIB_GNU %s
-// CHECK-OPT_GZ_EQ_ZLIB_GNU: {{.* "-cc1(as)?".* 
"--compress-debug-sections=zlib-gnu"}}
-// CHECK-OPT_GZ_EQ_ZLIB_GNU: "--compress-debug-sections=zlib-gnu"
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -286,13 +286,13 @@
 const ToolChain &TC, const llvm::opt::ArgList &Args,
 llvm::opt::ArgStringList &CmdArgs) {
   // GNU ld supports --compress-debug-sections=none|zlib|zlib-gnu|zlib-gabi
-  // whereas zlib is an alias to zlib-gabi. Therefore -gz=none|zlib|zlib-gnu
-  // are translated to --compress-debug-sections=none|zlib|zlib-gnu.
-  // -gz is not translated since ld --compress-debug-sections option requires 
an
+  // whereas zlib is an alias to zlib-gabi and zlib-gnu is obsoleted. Therefore
+  // -gz=none|zlib are translated to --compress-debug-sections=none|zlib. -gz
+  // is not translated since ld --compress-debug-sections option requires an
   // argument.
   if (const Arg *A = Args.getLastArg(options::OPT_gz_EQ)) {
 StringRef V = A->getValue();
-if (V == "none" || V == "zlib" || V == "zlib-gnu")
+if (V == "none" || V == "zlib")
   CmdArgs.push_back(Args.MakeArgString("--compress-debug-sections=" + V));
 else
   TC.getDriver().Diag(diag::err_drv_unsupported_option_argument)
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1117,7 +1117,7 @@
 StringRef Value = A->getValue();
 if (Value == "none") {
   CmdArgs.push_back("--compress-debug-sections=none");
-} else if (Value == "zlib" || Value == "zlib-gnu") {
+} else if (Value == "zlib") {
   if (llvm::zlib::isAvailable()) {
 CmdArgs.push_back(
 Args.MakeArgString("--compress-debug-sections=" + Twine(Value)));
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4974,8 +4974,8 @@
   HelpText<"The string to embed in the .LLVM.command.line section.">,
   MarshallingInfoString>;
 def compress_debug_sections_EQ : Joined<["-", "--"], 
"compress-debug-sections=">,
-HelpText<"DWARF debug sections compression type">, 
Values<"none,zlib,zlib-gnu">,
-NormalizedValuesScope<"llvm::DebugCompressionType">, 
NormalizedValues<["None", "Z", "GNU"]>,
+Hel

[clang] 35d1522 - [Driver] Remove obsoleted -gz=zlib-gnu

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2022-01-26T13:26:51-08:00
New Revision: 35d15222c05d3409076e170e4851fd3d58a64e35

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

LOG: [Driver] Remove obsoleted -gz=zlib-gnu

GCC added -gz=zlib-gnu in 2014 for -gz meaning change (.zdebug =>
SHF_COMPRESSED) and the legacy zlib-gnu hasn't gain adoption.

According to Debian Code Search (`gz=zlib-gnu`), no project uses -gz=zlib-gnu
(valgrind has a configure to use -gz=zlib). Any possible -gz=zlib-gnu user can
switch to -gz smoothly (supported by integrated assemblers for many years;
binutils 2.26).

Reviewed By: dblaikie

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

Added: 


Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/test/Driver/amdgcn-gz-options.cl
clang/test/Driver/compress.c

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index a57787a4f4c38..b3de12e8c7b52 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -4974,8 +4974,8 @@ def record_command_line : Separate<["-"], 
"record-command-line">,
   HelpText<"The string to embed in the .LLVM.command.line section.">,
   MarshallingInfoString>;
 def compress_debug_sections_EQ : Joined<["-", "--"], 
"compress-debug-sections=">,
-HelpText<"DWARF debug sections compression type">, 
Values<"none,zlib,zlib-gnu">,
-NormalizedValuesScope<"llvm::DebugCompressionType">, 
NormalizedValues<["None", "Z", "GNU"]>,
+HelpText<"DWARF debug sections compression type">, Values<"none,zlib">,
+NormalizedValuesScope<"llvm::DebugCompressionType">, 
NormalizedValues<["None", "Z"]>,
 MarshallingInfoEnum, "None">;
 def compress_debug_sections : Flag<["-", "--"], "compress-debug-sections">,
   Alias, AliasArgs<["zlib"]>;

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index ea83551845070..4386e395bc6ce 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1117,7 +1117,7 @@ static void RenderDebugInfoCompressionArgs(const ArgList 
&Args,
 StringRef Value = A->getValue();
 if (Value == "none") {
   CmdArgs.push_back("--compress-debug-sections=none");
-} else if (Value == "zlib" || Value == "zlib-gnu") {
+} else if (Value == "zlib") {
   if (llvm::zlib::isAvailable()) {
 CmdArgs.push_back(
 Args.MakeArgString("--compress-debug-sections=" + Twine(Value)));

diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp 
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 5711998f90434..1d30090ca21c2 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -286,13 +286,13 @@ void tools::addLinkerCompressDebugSectionsOption(
 const ToolChain &TC, const llvm::opt::ArgList &Args,
 llvm::opt::ArgStringList &CmdArgs) {
   // GNU ld supports --compress-debug-sections=none|zlib|zlib-gnu|zlib-gabi
-  // whereas zlib is an alias to zlib-gabi. Therefore -gz=none|zlib|zlib-gnu
-  // are translated to --compress-debug-sections=none|zlib|zlib-gnu.
-  // -gz is not translated since ld --compress-debug-sections option requires 
an
+  // whereas zlib is an alias to zlib-gabi and zlib-gnu is obsoleted. Therefore
+  // -gz=none|zlib are translated to --compress-debug-sections=none|zlib. -gz
+  // is not translated since ld --compress-debug-sections option requires an
   // argument.
   if (const Arg *A = Args.getLastArg(options::OPT_gz_EQ)) {
 StringRef V = A->getValue();
-if (V == "none" || V == "zlib" || V == "zlib-gnu")
+if (V == "none" || V == "zlib")
   CmdArgs.push_back(Args.MakeArgString("--compress-debug-sections=" + V));
 else
   TC.getDriver().Diag(diag::err_drv_unsupported_option_argument)

diff  --git a/clang/test/Driver/amdgcn-gz-options.cl 
b/clang/test/Driver/amdgcn-gz-options.cl
index 40fe9cfcc50df..0d09a714b0326 100644
--- a/clang/test/Driver/amdgcn-gz-options.cl
+++ b/clang/test/Driver/amdgcn-gz-options.cl
@@ -9,8 +9,3 @@
 // RUN: %clang -### -target amdgcn-amd-amdhsa -gz=zlib %s 2>&1 | FileCheck 
-check-prefix CHECK-OPT_GZ_EQ_ZLIB %s
 // CHECK-OPT_GZ_EQ_ZLIB: {{.* "-cc1(as)?".* "--compress-debug-sections=zlib"}}
 // CHECK-OPT_GZ_EQ_ZLIB: "--compress-debug-sections=zlib"
-
-// RUN: %clang -### -target amdgcn-amd-amdhsa -gz=zlib-gnu -x assembler %s 
2>&1 | FileCheck -check-prefix CHECK-OPT_GZ_EQ_ZLIB_GNU %s
-// RUN: %clang -### -target amdgcn-amd-amdhsa -gz=zlib-gnu %s 2>&1 | FileCheck 
-check-prefix CHECK-OPT_GZ_EQ_ZLIB_GNU %s
-// CHECK-OPT_GZ_EQ_ZLIB_GNU: {{.* "-cc1(as)?".* 
"--compress-debug-sections=zlib-gnu"}}
-// CHECK-OPT_GZ_EQ_ZLI

[PATCH] D56303: [clang-tidy] Recognize labelled statements when simplifying boolean exprs

[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.

In D56303#3273694 , @LegalizeAdulthood 
wrote:

> In D56303#3272711 , @aaron.ballman 
> wrote:
>
>> The rule of thumb is: if you have significant NFC changes,
>> do them first and push the changes (often doesn't even
>> require a review because it's NFC, but that depends on
>> what comfort level you have about the changes being NFC),
>> and then do the functional changes on top of the improved
>> code.
>
> This is reasonable, but is it written down anywhere so that people
> don't have to through this trial by fire every time? `:)`

Yup, in about the most obtuse place you could image: 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access. We could 
probably stand to update https://llvm.org/docs/DeveloperPolicy.html to be more 
explicit about this and document it in a much more obvious place. (I agree with 
the complaint about us doing a poor job of writing these kinds of expectations 
down.)

Oops, forgot to accept the last time I said I thought this looked good. Please 
still give a day or two before landing in case @JonasToth or @alexfh have 
further comments or concerns.


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

https://reviews.llvm.org/D56303

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

[PATCH] D118010: [clang-tidy] Fix nested namespaces in `readability-static-definition-in-anonymous-namespace` check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added a comment.

If there is an open github issue on this, please put "Fixes #" in the 
commit description before pushing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118010

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

[PATCH] D118010: [clang-tidy] Fix nested namespaces in `readability-static-definition-in-anonymous-namespace` check

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood accepted this revision.
LegalizeAdulthood 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/D118010/new/

https://reviews.llvm.org/D118010

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

[PATCH] D56303: [clang-tidy] Recognize labelled statements when simplifying boolean exprs

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood updated this revision to Diff 403390.
LegalizeAdulthood added a comment.

Update from review comments


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

https://reviews.llvm.org/D56303

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprMatchers.h
  
clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-case.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
  clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
  clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/ReadabilityModuleTest.cpp
@@ -1,6 +1,9 @@
+#include "../../clang/unittests/ASTMatchers/ASTMatchersTest.h"
 #include "ClangTidyTest.h"
 #include "readability/BracesAroundStatementsCheck.h"
 #include "readability/NamespaceCommentCheck.h"
+#include "readability/SimplifyBooleanExprMatchers.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -9,6 +12,123 @@
 
 using readability::BracesAroundStatementsCheck;
 using readability::NamespaceCommentCheck;
+using namespace ast_matchers;
+
+TEST_P(ASTMatchersTest, HasCaseSubstatement) {
+  EXPECT_TRUE(matches(
+  "void f() { switch (1) { case 1: return; break; default: break; } }",
+  traverse(TK_AsIs, caseStmt(hasSubstatement(returnStmt());
+}
+
+TEST_P(ASTMatchersTest, HasDefaultSubstatement) {
+  EXPECT_TRUE(matches(
+  "void f() { switch (1) { case 1: return; break; default: break; } }",
+  traverse(TK_AsIs, defaultStmt(hasSubstatement(breakStmt());
+}
+
+TEST_P(ASTMatchersTest, HasLabelSubstatement) {
+  EXPECT_TRUE(
+  matches("void f() { while (1) { bar: break; foo: return; } }",
+  traverse(TK_AsIs, labelStmt(hasSubstatement(breakStmt());
+}
+
+TEST_P(ASTMatchersTest, HasSubstatementSequenceSimple) {
+  const char *Text = "int f() { int x = 5; if (x < 0) return 1; return 0; }";
+  EXPECT_TRUE(matches(
+  Text, compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt();
+  EXPECT_FALSE(matches(
+  Text, compoundStmt(hasSubstatementSequence(ifStmt(), labelStmt();
+  EXPECT_FALSE(matches(
+  Text, compoundStmt(hasSubstatementSequence(returnStmt(), ifStmt();
+  EXPECT_FALSE(matches(
+  Text, compoundStmt(hasSubstatementSequence(switchStmt(), labelStmt();
+}
+
+TEST_P(ASTMatchersTest, HasSubstatementSequenceAlmost) {
+  const char *Text = R"code(
+int f() {
+  int x = 5;
+  if (x < 10)
+;
+  if (x < 0)
+return 1;
+  return 0;
+}
+)code";
+  EXPECT_TRUE(matches(
+  Text, compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt();
+  EXPECT_TRUE(
+  matches(Text, compoundStmt(hasSubstatementSequence(ifStmt(), ifStmt();
+}
+
+TEST_P(ASTMatchersTest, HasSubstatementSequenceComplex) {
+  const char *Text = R"code(
+int f() {
+  int x = 5;
+  if (x < 10)
+x -= 10;
+  if (x < 0)
+return 1;
+  return 0;
+}
+)code";
+  EXPECT_TRUE(matches(
+  Text, compoundStmt(hasSubstatementSequence(ifStmt(), returnStmt();
+  EXPECT_FALSE(
+  matches(Text, compoundStmt(hasSubstatementSequence(ifStmt(), expr();
+}
+
+TEST_P(ASTMatchersTest, HasSubstatementSequenceExpression) {
+  const char *Text = R"code(
+int f() {
+  return ({ int x = 5;
+  int result;
+  if (x < 10)
+x -= 10;
+  if (x < 0)
+result = 1;
+  else
+result = 0;
+  result;
+});
+  }
+)code";
+  EXPECT_TRUE(
+  matches(Text, stmtExpr(hasSubstatementSequence(ifStmt(), expr();
+  EXPECT_FALSE(
+  matches(Text, stmtExpr(hasSubstatementSequence(ifStmt(), returnStmt();
+}
+
+// Copied from ASTMatchersTests
+static std::vector allTestClangConfigs() {
+  std::vector all_configs;
+  for (TestLanguage lang : {Lang_C89, Lang_C99, Lang_CXX03, Lang_CXX11,
+Lang_CXX14, Lang_CXX17, Lang_CXX20}) {
+TestClangConfig config;
+config.Language = lang;
+
+// Use an unknown-unknown triple so we don't instantiate the full system
+// toolchain.  On Linux, instantiating the toolchain involves stat'ing
+// large portions of /usr/lib, and this slows down not only this test, but
+// all other tests, via contention in the kernel.
+//
+// FIXME: This is a hack to work around the fact that there's no way to do
+// the equivalent of runToolOnCodeWithArgs without instantiating a full
+// Driver.  We should consider having a function, at least for tests, that
+// invokes cc1.
+config.Target = "i386-unknown-unknown";
+all_configs.push_back(config);
+
+// Windows target is interestin

[PATCH] D56303: [clang-tidy] Recognize labelled statements when simplifying boolean exprs

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added a comment.

In D56303#3272711 , @aaron.ballman 
wrote:

> The rule of thumb is: if you have significant NFC changes,
> do them first and push the changes (often doesn't even
> require a review because it's NFC, but that depends on
> what comfort level you have about the changes being NFC),
> and then do the functional changes on top of the improved
> code.

This is reasonable, but is it written down anywhere so that people
don't have to through this trial by fire every time? `:)`


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

https://reviews.llvm.org/D56303

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

[PATCH] D117939: [clang-tidy] Add more documentation about check development

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood updated this revision to Diff 403386.
LegalizeAdulthood added a comment.

Update from review comments


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

https://reviews.llvm.org/D117939

Files:
  clang-tools-extra/docs/clang-tidy/Contributing.rst

Index: clang-tools-extra/docs/clang-tidy/Contributing.rst
===
--- clang-tools-extra/docs/clang-tidy/Contributing.rst
+++ clang-tools-extra/docs/clang-tidy/Contributing.rst
@@ -22,6 +22,8 @@
 check, it will create the check, update the CMake file and create a test;
   * ``rename_check.py`` does what the script name suggests, renames an existing
 check;
+  * :program:`pp-trace` logs method calls on `PPCallbacks` for a source file
+and is invaluable in understanding the preprocessor mechanism;
   * :program:`clang-query` is invaluable for interactive prototyping of AST
 matchers and exploration of the Clang AST;
   * `clang-check`_ with the ``-ast-dump`` (and optionally ``-ast-dump-filter``)
@@ -70,6 +72,14 @@
 .. _Using Clang Tools: https://clang.llvm.org/docs/ClangTools.html
 .. _How To Setup Clang Tooling For LLVM: https://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
 
+When you `configure the CMake build `_,
+make sure that you enable the ``clang`` and ``clang-tools-extra`` projects to
+build :program:`clang-tidy`.
+Because your new check will have associated documentation, you will also want to install
+`Sphinx `_ and enable it in the CMake configuration.
+To save build time of the core Clang libraries you may want to only enable the ``X86``
+target in the CMake configuration.
+
 
 The Directory Structure
 ---
@@ -215,11 +225,215 @@
 and `clang-tidy/google/ExplicitConstructorCheck.cpp
 `_).
 
+If you need to interact with macros or preprocessor directives, you will want to
+override the method ``registerPPCallbacks``.  The ``add_new_check.py`` script
+does not generate an override for this method in the starting point for your
+new check.
+
+If your check applies only under a specific set of language options, be sure
+to override the method ``isLanguageVersionSupported`` to reflect that.
+
+Check development tips
+--
+
+Writing your first check can be a daunting task, particularly if you are unfamiliar
+with the LLVM and Clang code bases.  Here are some suggestions for orienting yourself
+in the codebase and working on your check incrementally.
+
+Guide to useful documentation
+^
+
+Many of the support classes created for LLVM are used by Clang, such as `StringRef
+`_
+and `SmallVector `_.
+These and other commonly used classes are described in the `Important and useful LLVM APIs
+`_ and
+`Picking the Right Data Structure for the Task
+`_
+sections of the `LLVM Programmer's Manual
+`_.  You don't need to memorize all the
+details of these classes; the generated `doxygen documentation `_
+has everything if you need it.  In the header `LLVM/ADT/STLExtras.h
+`_ you'll find useful versions of the STL
+algorithms that operate on LLVM containers, such as `llvm::all_of
+`_.
+
+Clang is implemented on top of LLVM and introduces its own set of classes that you
+will interact with while writing your check.  When a check issues diagnostics and
+fix-its, these are associated with locations in the source code.  Source code locations,
+source files, ranges of source locations and the `SourceManager
+`_ class provide
+the mechanisms for describing such locations.  These and
+other topics are described in the `"Clang" CFE Internals Manual
+`_.  Whereas the doxygen generated
+documentation serves as a reference to the internals of Clang, this document serves
+as a guide to other developers.  Topics in that manual of interest to a check developer
+are:
+
+- `The Clang "Basic" Library
+  `_ for
+  information about diagnostics, fix-it hints and source locations.
+- `The Lexer and Preprocessor Library
+  `_
+  for information about tokens, l

[PATCH] D117939: [clang-tidy] Add more documentation about check development

[Richard via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:389-390
+- Test your check under both Windows and Linux environments.
+- Watch out for high false positive rates; ideally there should be none, but a
+  small number may be tolerable.  High false positive rates prevent adoption.
+- Consider configurable options for your check to deal with false positives.

ymandel wrote:
> aaron.ballman wrote:
> > LegalizeAdulthood wrote:
> > > ymandel wrote:
> > > > Is it worth giving a rule-of-thumb? Personally I'd go with < 10%, all 
> > > > things being equal.  A check for a serious bug may reasonably have a 
> > > > higher false positive rate, and trivial checks might not justify *any* 
> > > > false positives. But, if neither of these apply, I'd recommend 10% as 
> > > > the default.
> > > I'm OK with rule-of-thumb 10% advice.
> > FWIW, I think 10% is pretty arbitrary and I'd rather not see us try to nail 
> > it down to a concrete number. In practical terms, it really depends on the 
> > check.
> > 
> > Also, clang-tidy is where we put things with a "high" false positive rate 
> > already, so this statement has implications on what an acceptable false 
> > positive rate is for Clang or the static analyzer.
> > 
> > How about something along these lines:
> > ```
> > - Watch out for high false positive rates. Ideally, a check would have no 
> > false positives, but given that matching against an AST is not control- or 
> > data flow-sensitive, a number of false positives are expected. The higher 
> > the false positive rate, the less likely the check will be adopted in 
> > practice. Mechanisms should be put in place to help the user manage false 
> > positives.
> > - There are two primary mechanisms for managing false positives: supporting 
> > a code pattern which allows the programmer to silence the diagnostic in an 
> > ad hoc manner and check configuration options to control the behavior of 
> > the check.
> > - Consider supporting a code pattern to allow the programmer to silence the 
> > diagnostic whenever such a code pattern can clearly express the 
> > programmer's intent. For example, allowing an explicit cast to `void` to 
> > silence an unused variable diagnostic.
> > - Consider adding check configuration options to allow the user to opt into 
> > more aggressive checking behavior without burdening users for the common 
> > high-confidence cases.
> > ```
> > (or something along those lines). The basic idea I have there is: false 
> > positives are expected, try to keep them to a minimum, and here are the two 
> > most common ways code reviewers will ask you to handle false positives when 
> > they're a concern.
> Strongly agree. 10% has served as well in practice for the threshold at which 
> we disable/fix checks, but it's definitely arbitrary. I much prefer your 
> suggested approach.
Yeah, I wasn't a fan of a magic number style piece of advice.  I like the 
reworded suggestion better.


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

https://reviews.llvm.org/D117939

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

[PATCH] D118220: [clang-format] Correctly format lambdas with variadic template parameters.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

Haha, and I want to have a few things fixed before release 14! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118220

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

[PATCH] D117593: [clang-tidy] Change google-explicit-constructor to ignore conversions and operators marked `explicit(false)`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Hmmm, this is a rule for checking a specific style guide. 
https://google.github.io/styleguide/cppguide.html#Implicit_Conversions doesn't 
say that `explicit(false)` is a reasonable marking. In fact, it says "Implicit 
conversions can sometimes be necessary and appropriate for types that are 
designed to be interchangeable, for example when objects of two types are just 
different representations of the same underlying value. In that case, contact 
your project leads to request a waiver of this rule."

So I think this behavior needs to be behind a flag so that the default behavior 
continues to match what the style guide requires (or the style guide should be 
updated to clarify the behavior of `explicit(false)`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117593

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

[PATCH] D118220: [clang-format] Correctly format lambdas with variadic template parameters.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D118220#3273416 , 
@HazardyKnusperkeks wrote:

> You guys have an enormous speed currently.

I'm just trying to keep up with @curdeius, but my real work keeps getting in 
the way, but I'm loving the momentum and trying to get back most days to at 
least help move things forward!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118220

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

[PATCH] D117744: [Driver] Remove obsoleted -gz=zlib-gnu

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D117744#3273521 , @dblaikie wrote:

> In D117744#3273481 , @MaskRay wrote:
>
>> In D117744#3273463 , @dblaikie 
>> wrote:
>>
>>> I don't especially feel that removing low-cost features like this is a 
>>> great idea (the benefit to cleaning up a small amount of code compared to 
>>> the (albeit small) risk of breaking users doesn't seem high value to me), 
>>> but sure - give it a go.
>>
>> Thanks:)
>>
>> This is to ensure that there are no producers: then we can do something more 
>> ambitious, like removing assembler support:)
>>
>> Driver change is a good start because if someone has justifiable complaint, 
>> the change can be relatively easily reversed without churn to the underlying 
>> implementation.
>>
>> It will also make future removal of .zdebug from ld.lld justifiable.
>
> Yeah, I appreciate the desire, but it doesn't seem especially burdensome to 
> support (since we'll still have support for the SHF_COMPRESSED style and it's 
> not a lot of code on top of that to support the zdebug style) so I don't see 
> as much value in making these changes, but that's OK.

The issue with `.zdebug` is the section name. When iterating over input 
sections (which may cost ld.lld 0.4% time, the ratio will go up if we support 
parallel symbol table initialization), not needing to scan section names 
(different places from the section metadata) will improve a bit.
Yeah, it's saving on some very minor stuff, I know.

> Oh, one thing: would probably be good to make two separate commits, one for 
> -gz and one for the --compress-debug-sections

OK


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117744

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

[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 403370.
zahiraam marked an inline comment as done.

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

https://reviews.llvm.org/D117569

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/CodeGenCXX/PR19955.cpp
  clang/test/CodeGenCXX/dllimport.cpp
  clang/test/SemaCXX/PR19955.cpp
  clang/test/SemaCXX/dllimport-constexpr.cpp

Index: clang/test/SemaCXX/dllimport-constexpr.cpp
===
--- clang/test/SemaCXX/dllimport-constexpr.cpp
+++ clang/test/SemaCXX/dllimport-constexpr.cpp
@@ -40,7 +40,6 @@
 // constexpr initialization doesn't work for dllimport things.
 // expected-error@+1{{must be initialized by a constant expression}}
 constexpr void (*constexpr_import_func)() = &imported_func;
-// expected-error@+1{{must be initialized by a constant expression}}
 constexpr int *constexpr_import_int = &imported_int;
 // expected-error@+1{{must be initialized by a constant expression}}
 constexpr void (Foo::*constexpr_memptr)() = &Foo::imported_method;
@@ -60,3 +59,14 @@
   // expected-note@+1 {{requested here}}
   StaticConstexpr::g_fp();
 }
+
+void foo() {
+  extern int __declspec(dllimport) dll_import_int;
+  constexpr int& dll_import_constexpr_ref = dll_import_int;
+}
+
+extern int __declspec(dllimport) dll_import_int;
+constexpr int& dll_import_constexpr_ref = dll_import_int;
+int goo() {
+  return dll_import_constexpr_ref;
+}
Index: clang/test/SemaCXX/PR19955.cpp
===
--- clang/test/SemaCXX/PR19955.cpp
+++ clang/test/SemaCXX/PR19955.cpp
@@ -2,7 +2,7 @@
 // RUN: %clang_cc1 -triple i686-mingw32 -verify -std=c++11 %s
 
 extern int __attribute__((dllimport)) var;
-constexpr int *varp = &var; // expected-error {{must be initialized by a constant expression}}
+constexpr int *varp = &var;
 
 extern __attribute__((dllimport)) void fun();
 constexpr void (*funp)(void) = &fun; // expected-error {{must be initialized by a constant expression}}
Index: clang/test/CodeGenCXX/dllimport.cpp
===
--- clang/test/CodeGenCXX/dllimport.cpp
+++ clang/test/CodeGenCXX/dllimport.cpp
@@ -98,8 +98,8 @@
 USE(inlineStaticLocalsFunc);
 
 // The address of a dllimport global cannot be used in constant initialization.
-// M32-DAG: @"?arr@?1??initializationFunc@@YAPAHXZ@4QBQAHB" = internal global [1 x i32*] zeroinitializer
-// GNU-DAG: @_ZZ18initializationFuncvE3arr = internal global [1 x i32*] zeroinitializer
+// M32-DAG: @"?arr@?1??initializationFunc@@YAPAHXZ@4QBQAHB" = internal constant [1 x i32*] [i32* @"?ExternGlobalDecl@@3HA"]
+// GNU-DAG: @_ZZ18initializationFuncvE3arr = internal constant [1 x i32*] [i32* @ExternGlobalDecl]
 int *initializationFunc() {
   static int *const arr[] = {&ExternGlobalDecl};
   return arr[0];
Index: clang/test/CodeGenCXX/PR19955.cpp
===
--- clang/test/CodeGenCXX/PR19955.cpp
+++ clang/test/CodeGenCXX/PR19955.cpp
@@ -6,20 +6,16 @@
 
 extern int *varp;
 int *varp = &var;
-// CHECK-DAG: @"?varp@@3PAHA" = dso_local global i32* null
-// X64-DAG: @"?varp@@3PEAHEA" = dso_local global i32* null
+// CHECK-DAG: @"?var@@3HA" = external dllimport global i32
+// CHECK-DAG: @"?varp@@3PAHA" = dso_local global i32* @"?var@@3HA"
+// X64-DAG: @"?var@@3HA" = external dllimport global i32, align 4
+// X64-DAG: @"?varp@@3PEAHEA" = dso_local global i32* @"?var@@3HA"
 
 extern void (*funp)();
 void (*funp)() = &fun;
 // CHECK-DAG: @"?funp@@3P6AXXZA" = dso_local global void ()* null
 // X64-DAG: @"?funp@@3P6AXXZEA" = dso_local global void ()* null
 
-// CHECK-LABEL: @"??__Evarp@@YAXXZ"
-// CHECK-DAG: store i32* @"?var@@3HA", i32** @"?varp@@3PAHA"
-
-// X64-LABEL: @"??__Evarp@@YAXXZ"
-// X64-DAG: store i32* @"?var@@3HA", i32** @"?varp@@3PEAHEA"
-
 // CHECK-LABEL: @"??__Efunp@@YAXXZ"()
 // CHECK-DAG: store void ()* @"?fun@@YAXXZ", void ()** @"?funp@@3P6AXXZA"
 
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -2211,9 +2211,12 @@
 // FIXME: Diagnostic!
 return false;
 
-  // A dllimport variable never acts like a constant, unless we're
+  // A dllimport/dllexport variable never acts like a constant, unless we're
   // evaluating a value for use only in name mangling.
-  if (!isForManglingOnly(Kind) && Var->hasAttr())
+  if (Info.getLangOpts().C99 && !isForManglingOnly(Kind) &&
+  Var->hasAttr() ||
+  Info.getLangOpts().CPlusPlus && !isForManglingOnly(Kind) &&
+  Var->hasAttr())
 // FIXME: Diagnostic!
 return false;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117744: [Driver] Remove obsoleted -gz=zlib-gnu

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D117744#3273481 , @MaskRay wrote:

> In D117744#3273463 , @dblaikie 
> wrote:
>
>> I don't especially feel that removing low-cost features like this is a great 
>> idea (the benefit to cleaning up a small amount of code compared to the 
>> (albeit small) risk of breaking users doesn't seem high value to me), but 
>> sure - give it a go.
>
> Thanks:)
>
> This is to ensure that there are no producers: then we can do something more 
> ambitious, like removing assembler support:)
>
> Driver change is a good start because if someone has justifiable complaint, 
> the change can be relatively easily reversed without churn to the underlying 
> implementation.
>
> It will also make future removal of .zdebug from ld.lld justifiable.

Yeah, I appreciate the desire, but it doesn't seem especially burdensome to 
support (since we'll still have support for the SHF_COMPRESSED style and it's 
not a lot of code on top of that to support the zdebug style) so I don't see as 
much value in making these changes, but that's OK.

Oh, one thing: would probably be good to make two separate commits, one for -gz 
and one for the --compress-debug-sections


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117744

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

[PATCH] D117744: [Driver] Remove obsoleted -gz=zlib-gnu

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D117744#3273463 , @dblaikie wrote:

> I don't especially feel that removing low-cost features like this is a great 
> idea (the benefit to cleaning up a small amount of code compared to the 
> (albeit small) risk of breaking users doesn't seem high value to me), but 
> sure - give it a go.

Thanks:)

This is to ensure that there are no producers: then we can do something more 
ambitious, like removing assembler support:)

Driver change is a good start because if someone has justifiable complaint, the 
change can be relatively easily reversed without churn to the underlying 
implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117744

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

[PATCH] D117744: [Driver] Remove obsoleted -gz=zlib-gnu

[David Blaikie via Phabricator via cfe-commits]

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

I don't especially feel that removing low-cost features like this is a great 
idea (the benefit to cleaning up a small amount of code compared to the (albeit 
small) risk of breaking users doesn't seem high value to me), but sure - give 
it a go.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117744

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

[PATCH] D117056: [clangd] Properly compute framework-style include spelling

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 403361.
dgoldman marked an inline comment as done.
dgoldman added a comment.

Formatting fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117056

Files:
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp

Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -663,6 +663,83 @@
   QName("Cat::meow"), QName("Cat::pur")));
 }
 
+TEST_F(SymbolCollectorTest, ObjCFrameworkIncludeHeader) {
+  CollectorOpts.CollectIncludePath = true;
+  auto FrameworksPath = testPath("Frameworks/");
+  std::string FrameworkHeader = R"(
+__attribute((objc_root_class))
+@interface NSObject
+@end
+  )";
+  InMemoryFileSystem->addFile(
+  testPath("Frameworks/Foundation.framework/Headers/NSObject.h"), 0,
+  llvm::MemoryBuffer::getMemBuffer(FrameworkHeader));
+  std::string PrivateFrameworkHeader = R"(
+#import 
+
+@interface PrivateClass : NSObject
+@end
+  )";
+  InMemoryFileSystem->addFile(
+  testPath("Frameworks/Foundation.framework/Headers/NSObject.h"), 0,
+  llvm::MemoryBuffer::getMemBuffer(FrameworkHeader));
+  InMemoryFileSystem->addFile(
+  testPath(
+  "Frameworks/Foundation.framework/PrivateHeaders/NSObject+Private.h"),
+  0, llvm::MemoryBuffer::getMemBuffer(PrivateFrameworkHeader));
+
+  std::string Header = R"(
+#import 
+#import 
+
+@interface Container : NSObject
+@end
+  )";
+  std::string Main = "";
+  TestFileName = testPath("test.m");
+  runSymbolCollector(Header, Main, {"-F", FrameworksPath, "-xobjective-c++"});
+  EXPECT_THAT(
+  Symbols,
+  UnorderedElementsAre(
+  AllOf(QName("NSObject"), IncludeHeader("\"Foundation/NSObject.h\"")),
+  AllOf(QName("PrivateClass"),
+IncludeHeader("\"Foundation/NSObject+Private.h\"")),
+  AllOf(QName("Container";
+
+  // After adding the umbrella headers, we should use that spelling instead.
+  std::string UmbrellaHeader = R"(
+#import 
+  )";
+  InMemoryFileSystem->addFile(
+  testPath("Frameworks/Foundation.framework/Headers/Foundation.h"), 0,
+  llvm::MemoryBuffer::getMemBuffer(UmbrellaHeader));
+  std::string PrivateUmbrellaHeader = R"(
+#import 
+  )";
+  InMemoryFileSystem->addFile(
+  testPath("Frameworks/Foundation.framework/PrivateHeaders/"
+   "Foundation_Private.h"),
+  0, llvm::MemoryBuffer::getMemBuffer(PrivateUmbrellaHeader));
+  runSymbolCollector(Header, Main, {"-F", FrameworksPath, "-xobjective-c++"});
+  EXPECT_THAT(Symbols,
+  UnorderedElementsAre(
+  AllOf(QName("NSObject"),
+IncludeHeader("\"Foundation/Foundation.h\"")),
+  AllOf(QName("PrivateClass"),
+IncludeHeader("\"Foundation/Foundation_Private.h\"")),
+  AllOf(QName("Container";
+
+  runSymbolCollector(Header, Main,
+ {"-iframework", FrameworksPath, "-xobjective-c++"});
+  EXPECT_THAT(
+  Symbols,
+  UnorderedElementsAre(
+  AllOf(QName("NSObject"), IncludeHeader("")),
+  AllOf(QName("PrivateClass"),
+IncludeHeader("")),
+  AllOf(QName("Container";
+}
+
 TEST_F(SymbolCollectorTest, Locations) {
   Annotations Header(R"cpp(
 // Declared in header, defined in main.
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -183,6 +183,13 @@
 // including filename normalization, URI conversion etc.
 // Expensive checks are cached internally.
 class SymbolCollector::HeaderFileURICache {
+  struct FrameworkUmbrellaSpelling {
+// Spelling for the public umbrella header, e.g. 
+llvm::Optional PublicHeader;
+// Spelling for the private umbrella header, e.g.
+// 
+llvm::Optional PrivateHeader;
+  };
   // Weird double-indirect access to PP, which might not be ready yet when
   // HeaderFiles is created but will be by the time it's used.
   // (IndexDataConsumer::setPreprocessor can happen before or after initialize)
@@ -193,6 +200,9 @@
   llvm::DenseMap CacheFEToURI;
   llvm::StringMap CachePathToURI;
   llvm::DenseMap CacheFIDToInclude;
+  llvm::StringMap CachePathToFrameworkSpelling;
+  llvm::StringMap
+  CacheFrameworkToUmbrellaHeaderSpelling;
 
 public:
   HeaderFileURICache(Preprocessor *&PP, const SourceManager &SM,
@@ -249,6 +259,125 @@
 return R.first->second;
   }
 
+  struct FrameworkHeaderPath {
+// Path to the framework directory containin

[clang] 6a953d9 - [clang] Fix -Wsubobject-linkage after D117262

[Arthur Eubanks via cfe-commits]

Author: Arthur Eubanks
Date: 2022-01-26T11:43:44-08:00
New Revision: 6a953d931c4de6f4b5bdc5466d15da62ede00b95

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

LOG: [clang] Fix -Wsubobject-linkage after D117262

/home/buildbot/llvm-avr-linux/llvm-avr-linux/llvm/clang/lib/CodeGen/Address.h:76:7:
 warning: 'clang::CodeGen::Address' has a field 'clang::CodeGen::Address::A' 
whose type uses the anonymous namespace [-Wsubobject-linkage]

https://lab.llvm.org/buildbot/#/builders/112/builds/12047

Added: 


Modified: 
clang/lib/CodeGen/Address.h

Removed: 




diff  --git a/clang/lib/CodeGen/Address.h b/clang/lib/CodeGen/Address.h
index 06b82e404cce..6a22e567d399 100644
--- a/clang/lib/CodeGen/Address.h
+++ b/clang/lib/CodeGen/Address.h
@@ -22,7 +22,6 @@
 namespace clang {
 namespace CodeGen {
 
-namespace {
 // We try to save some space by using 6 bits over two PointerIntPairs to store
 // the alignment. However, some arches don't support 3 bits in a PointerIntPair
 // so we fallback to storing the alignment separately.
@@ -70,7 +69,6 @@ template  class AddressImpl {
 return CharUnits::fromQuantity(1UL << AlignLog);
   }
 };
-} // namespace
 
 /// An aligned address.
 class Address {



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

[PATCH] D117569: Constexpr not supported with __declspec(dllimport).

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 403354.

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

https://reviews.llvm.org/D117569

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/CodeGenCXX/PR19955.cpp
  clang/test/CodeGenCXX/dllimport.cpp
  clang/test/SemaCXX/PR19955.cpp
  clang/test/SemaCXX/dllimport-constexpr.cpp


Index: clang/test/SemaCXX/dllimport-constexpr.cpp
===
--- clang/test/SemaCXX/dllimport-constexpr.cpp
+++ clang/test/SemaCXX/dllimport-constexpr.cpp
@@ -40,7 +40,6 @@
 // constexpr initialization doesn't work for dllimport things.
 // expected-error@+1{{must be initialized by a constant expression}}
 constexpr void (*constexpr_import_func)() = &imported_func;
-// expected-error@+1{{must be initialized by a constant expression}}
 constexpr int *constexpr_import_int = &imported_int;
 // expected-error@+1{{must be initialized by a constant expression}}
 constexpr void (Foo::*constexpr_memptr)() = &Foo::imported_method;
@@ -60,3 +59,8 @@
   // expected-note@+1 {{requested here}}
   StaticConstexpr::g_fp();
 }
+
+void foo() {
+  extern int __declspec(dllimport) dll_import_int;
+  constexpr int& dll_import_constexpr_ref = dll_import_int;
+}
Index: clang/test/SemaCXX/PR19955.cpp
===
--- clang/test/SemaCXX/PR19955.cpp
+++ clang/test/SemaCXX/PR19955.cpp
@@ -2,7 +2,7 @@
 // RUN: %clang_cc1 -triple i686-mingw32 -verify -std=c++11 %s
 
 extern int __attribute__((dllimport)) var;
-constexpr int *varp = &var; // expected-error {{must be initialized by a 
constant expression}}
+constexpr int *varp = &var;
 
 extern __attribute__((dllimport)) void fun();
 constexpr void (*funp)(void) = &fun; // expected-error {{must be initialized 
by a constant expression}}
Index: clang/test/CodeGenCXX/dllimport.cpp
===
--- clang/test/CodeGenCXX/dllimport.cpp
+++ clang/test/CodeGenCXX/dllimport.cpp
@@ -98,8 +98,8 @@
 USE(inlineStaticLocalsFunc);
 
 // The address of a dllimport global cannot be used in constant initialization.
-// M32-DAG: @"?arr@?1??initializationFunc@@YAPAHXZ@4QBQAHB" = internal global 
[1 x i32*] zeroinitializer
-// GNU-DAG: @_ZZ18initializationFuncvE3arr = internal global [1 x i32*] 
zeroinitializer
+// M32-DAG: @"?arr@?1??initializationFunc@@YAPAHXZ@4QBQAHB" = internal 
constant [1 x i32*] [i32* @"?ExternGlobalDecl@@3HA"]
+// GNU-DAG: @_ZZ18initializationFuncvE3arr = internal constant [1 x i32*] 
[i32* @ExternGlobalDecl]
 int *initializationFunc() {
   static int *const arr[] = {&ExternGlobalDecl};
   return arr[0];
Index: clang/test/CodeGenCXX/PR19955.cpp
===
--- clang/test/CodeGenCXX/PR19955.cpp
+++ clang/test/CodeGenCXX/PR19955.cpp
@@ -6,20 +6,16 @@
 
 extern int *varp;
 int *varp = &var;
-// CHECK-DAG: @"?varp@@3PAHA" = dso_local global i32* null
-// X64-DAG: @"?varp@@3PEAHEA" = dso_local global i32* null
+// CHECK-DAG: @"?var@@3HA" = external dllimport global i32
+// CHECK-DAG: @"?varp@@3PAHA" = dso_local global i32* @"?var@@3HA"
+// X64-DAG: @"?var@@3HA" = external dllimport global i32, align 4
+// X64-DAG: @"?varp@@3PEAHEA" = dso_local global i32* @"?var@@3HA"
 
 extern void (*funp)();
 void (*funp)() = &fun;
 // CHECK-DAG: @"?funp@@3P6AXXZA" = dso_local global void ()* null
 // X64-DAG: @"?funp@@3P6AXXZEA" = dso_local global void ()* null
 
-// CHECK-LABEL: @"??__Evarp@@YAXXZ"
-// CHECK-DAG: store i32* @"?var@@3HA", i32** @"?varp@@3PAHA"
-
-// X64-LABEL: @"??__Evarp@@YAXXZ"
-// X64-DAG: store i32* @"?var@@3HA", i32** @"?varp@@3PEAHEA"
-
 // CHECK-LABEL: @"??__Efunp@@YAXXZ"()
 // CHECK-DAG: store void ()* @"?fun@@YAXXZ", void ()** @"?funp@@3P6AXXZA"
 
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -2213,7 +2213,10 @@
 
   // A dllimport variable never acts like a constant, unless we're
   // evaluating a value for use only in name mangling.
-  if (!isForManglingOnly(Kind) && Var->hasAttr())
+  if (Info.getLangOpts().C99 && !isForManglingOnly(Kind) &&
+  Var->hasAttr() ||
+  Info.getLangOpts().CPlusPlus && !isForManglingOnly(Kind) &&
+  !Var->hasAttr())
 // FIXME: Diagnostic!
 return false;
 }


Index: clang/test/SemaCXX/dllimport-constexpr.cpp
===
--- clang/test/SemaCXX/dllimport-constexpr.cpp
+++ clang/test/SemaCXX/dllimport-constexpr.cpp
@@ -40,7 +40,6 @@
 // constexpr initialization doesn't work for dllimport things.
 // expected-error@+1{{must be initialized by a constant expression}}
 constexpr void (*constexpr_import_func)() = &imported_func;
-// expected-error@+1{{must be initialized by a constant expression}}
 constexpr int *constexpr_import_int = &importe

[PATCH] D118220: [clang-format] Correctly format lambdas with variadic template parameters.

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

You guys have an enormous speed currently.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118220

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

[PATCH] D118084: [CUDA, NVPTX] Pass byval aggregates directly

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D118084#3272154 , @lebedev.ri 
wrote:

> My last idea was about allowing splitting
>
>   struct {
> int a;
> int b[2];
>   } a;
>
> into
>
>   // not in a struct anymore!
>   int a;
>   int b[2]

This looks like it's a somewhat different problem.

In my case this is what bites me: https://godbolt.org/z/x9bWjePGY
It's a variant of this issue: https://github.com/llvm/llvm-project/issues/51734

I have a WIP patch that converts a GEP with a dynamic index with a known range 
of values into a series of comparisons and fixed-index GEPs. I guess I'll need 
to get it sorted out first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118084

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

[PATCH] D116753: Default to -fno-math-errno for musl too

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.

Looks great!

Thanks for telling me that GCC `overloads -fno-math-errno to mean that malloc 
does not set errno`. This looks really scary. Clang doesn't have the problem:)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116753

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

[PATCH] D117809: [clang] Add an extract-api driver option

[Zixu Wang via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb1d946cbf780: [clang] Add an extract-api driver option 
(authored by zixuw).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117809

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Frontend/FrontendActions.h
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/Types.cpp
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/ExtractAPIConsumer.cpp
  clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  clang/test/Driver/extract-api.c

Index: clang/test/Driver/extract-api.c
===
--- /dev/null
+++ clang/test/Driver/extract-api.c
@@ -0,0 +1,16 @@
+// RUN: %clang -target x86_64-unknown-unknown -ccc-print-phases -extract-api %s 2> %t
+// RUN: echo 'END' >> %t
+// RUN: FileCheck -check-prefix EXTRACT-API-PHASES -input-file %t %s
+
+// EXTRACT-API-PHASES: 0: input,
+// EXTRACT-API-PHASES: , c
+// EXTRACT-API-PHASES: 1: preprocessor, {0}, cpp-output
+// EXTRACT-API-PHASES: 2: compiler, {1}, api-information
+// EXTRACT-API-PHASES-NOT: 3:
+// EXTRACT-API-PHASES: END
+
+// FIXME: Check for the dummy output now to verify that the custom action was executed.
+// RUN: %clang -extract-api %s | FileCheck -check-prefix DUMMY-OUTPUT %s
+
+void dummy_function();
+// DUMMY-OUTPUT: dummy_function
Index: clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -57,6 +57,8 @@
   case EmitLLVMOnly:   return std::make_unique();
   case EmitCodeGenOnly:return std::make_unique();
   case EmitObj:return std::make_unique();
+  case ExtractAPI:
+return std::make_unique();
   case FixIt:  return std::make_unique();
   case GenerateModule:
 return std::make_unique();
Index: clang/lib/Frontend/ExtractAPIConsumer.cpp
===
--- /dev/null
+++ clang/lib/Frontend/ExtractAPIConsumer.cpp
@@ -0,0 +1,39 @@
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/ASTConsumers.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendActions.h"
+
+using namespace clang;
+
+namespace {
+class ExtractAPIVisitor : public RecursiveASTVisitor {
+public:
+  explicit ExtractAPIVisitor(ASTContext *Context) : Context(Context) {}
+
+  bool VisitNamedDecl(NamedDecl *Decl) {
+llvm::outs() << Decl->getName() << "\n";
+return true;
+  }
+
+private:
+  ASTContext *Context;
+};
+
+class ExtractAPIConsumer : public ASTConsumer {
+public:
+  explicit ExtractAPIConsumer(ASTContext *Context) : Visitor(Context) {}
+
+  void HandleTranslationUnit(ASTContext &Context) override {
+Visitor.TraverseDecl(Context.getTranslationUnitDecl());
+  }
+
+private:
+  ExtractAPIVisitor Visitor;
+};
+} // namespace
+
+std::unique_ptr
+ExtractAPIAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
+  return std::make_unique(&CI.getASTContext());
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2408,6 +2408,7 @@
   {frontend::EmitCodeGenOnly, OPT_emit_codegen_only},
   {frontend::EmitCodeGenOnly, OPT_emit_codegen_only},
   {frontend::EmitObj, OPT_emit_obj},
+  {frontend::ExtractAPI, OPT_extract_api},
 
   {frontend::FixIt, OPT_fixit_EQ},
   {frontend::FixIt, OPT_fixit},
@@ -4147,6 +4148,7 @@
   case frontend::EmitLLVMOnly:
   case frontend::EmitCodeGenOnly:
   case frontend::EmitObj:
+  case frontend::ExtractAPI:
   case frontend::FixIt:
   case frontend::GenerateModule:
   case frontend::GenerateModuleInterface:
Index: clang/lib/Frontend/CMakeLists.txt
===
--- clang/lib/Frontend/CMakeLists.txt
+++ clang/lib/Frontend/CMakeLists.txt
@@ -20,6 +20,7 @@
   DependencyFile.cpp
   DependencyGraph.cpp
   DiagnosticRenderer.cpp
+  ExtractAPIConsumer.cpp
   FrontendAction.cpp
   FrontendActions.cpp
   FrontendOptions.cpp
Index: clang/lib/Driver/Types.cpp
===
--- clang/lib/Driver/Types.cpp
+++ clang/lib/Driver/Types.cpp
@@ -143,6 +143,7 @@
   case TY_CXXModule: case TY_PP_CXXModule:
   case TY_AST: case TY_ModuleFile: case TY_PCH:
   case TY_LLVM_IR: case TY_LLVM_BC:
+  case TY_API_INFO:
 return true;
   }
 }
In

[clang] b1d946c - [clang] Add an extract-api driver option

[Zixu Wang via cfe-commits]

Author: Zixu Wang
Date: 2022-01-26T11:31:12-08:00
New Revision: b1d946cbf780f1769b3a3a39ce68e462a181869e

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

LOG: [clang] Add an extract-api driver option

This is the initial commit for the clang-extract-api RFC

Add a new driver option `-extract-api` and associate it with a dummy
(for now) frontend action to set up the initial structure for
incremental works.

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

Added: 
clang/lib/Frontend/ExtractAPIConsumer.cpp
clang/test/Driver/extract-api.c

Modified: 
clang/include/clang/Driver/Options.td
clang/include/clang/Driver/Types.def
clang/include/clang/Frontend/FrontendActions.h
clang/include/clang/Frontend/FrontendOptions.h
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/Types.cpp
clang/lib/Frontend/CMakeLists.txt
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 49ceebcb51cf5..a57787a4f4c38 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1080,6 +1080,8 @@ def end_no_unused_arguments : Flag<["--"], 
"end-no-unused-arguments">, Flags<[Co
   HelpText<"Start emitting warnings for unused driver arguments">;
 def interface_stub_version_EQ : JoinedOrSeparate<["-"], 
"interface-stub-version=">, Flags<[CC1Option]>;
 def exported__symbols__list : Separate<["-"], "exported_symbols_list">;
+def extract_api : Flag<["-"], "extract-api">, Flags<[CC1Option]>, 
Group,
+  HelpText<"Extract API information">;
 def e : JoinedOrSeparate<["-"], "e">, Flags<[LinkerInput]>, Group;
 def fmax_tokens_EQ : Joined<["-"], "fmax-tokens=">, Group, 
Flags<[CC1Option]>,
   HelpText<"Max total number of preprocessed tokens for -Wmax-tokens.">,

diff  --git a/clang/include/clang/Driver/Types.def 
b/clang/include/clang/Driver/Types.def
index 997eea445c225..7adf59ca5c992 100644
--- a/clang/include/clang/Driver/Types.def
+++ b/clang/include/clang/Driver/Types.def
@@ -100,4 +100,5 @@ TYPE("dSYM", dSYM, INVALID, 
"dSYM",   phases
 TYPE("dependencies", Dependencies, INVALID, "d",  
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("cuda-fatbin",  CUDA_FATBIN,  INVALID, "fatbin", 
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("hip-fatbin",   HIP_FATBIN,   INVALID, "hipfb",  
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
+TYPE("api-information",  API_INFO, INVALID, "json",   
phases::Compile)
 TYPE("none", Nothing,  INVALID, nullptr,  
phases::Compile, phases::Backend, phases::Assemble, phases::Link)

diff  --git a/clang/include/clang/Frontend/FrontendActions.h 
b/clang/include/clang/Frontend/FrontendActions.h
index 8bd6509559f70..8eceb81723d23 100644
--- a/clang/include/clang/Frontend/FrontendActions.h
+++ b/clang/include/clang/Frontend/FrontendActions.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_FRONTEND_FRONTENDACTIONS_H
 
 #include "clang/Frontend/FrontendAction.h"
+#include 
 #include 
 #include 
 
@@ -270,6 +271,12 @@ class PrintDependencyDirectivesSourceMinimizerAction : 
public FrontendAction {
   bool usesPreprocessorOnly() const override { return true; }
 };
 
+class ExtractAPIAction : public ASTFrontendAction {
+protected:
+  std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
+ StringRef InFile) override;
+};
+
 
//===--===//
 // Preprocessor Actions
 
//===--===//

diff  --git a/clang/include/clang/Frontend/FrontendOptions.h 
b/clang/include/clang/Frontend/FrontendOptions.h
index 1d9d89a28c6c4..7ce8076a3ee41 100644
--- a/clang/include/clang/Frontend/FrontendOptions.h
+++ b/clang/include/clang/Frontend/FrontendOptions.h
@@ -75,6 +75,9 @@ enum ActionKind {
   /// Emit a .o file.
   EmitObj,
 
+  // Extract API information
+  ExtractAPI,
+
   /// Parse and apply any fixits to the source.
   FixIt,
 

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index c7314e11c7865..2e4ebc10e9ba3 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -62,6 +62,7 @@
 #include "clang/Driver/SanitizerArgs.h"
 #include "clang/Driver/Tool.h"
 #include "clang/Driver/ToolChain.h"
+#include "clang/Driver/Types.h"
 #include "llvm/ADT/ArrayRef.h"
 #

[PATCH] D116541: [OpenMP] Introduce new flag to change offloading driver pipeline

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/include/clang/Driver/Driver.h:45
+class Command;
+class Compilation;
+class JobList;

JonChesterfield wrote:
> This looks like it should be a breaking change - InputInfo is no longer 
> forward declared. Would it be reasonable to keep the forward declaration and 
> put the typedef between class statements and the LTOKind enum?
We can't forward declare a struct and use it by-value in a container, I would 
need to change it to a pointer. It's doable, but I don't think it's ideal. I'm 
not sure why this would break anything, the forward declarations simply were 
there to avoid including more files here. I could be wrong however.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116541

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

[PATCH] D117830: [HeaderSearch] Track framework name in LookupFile

[Cyndy Ishida via Phabricator via cfe-commits]

cishida added inline comments.



Comment at: clang/lib/Lex/HeaderSearch.cpp:1043
 
 // If this file is found in a header map and uses the framework style of
 // includes, then this header is part of a framework we're building.

dgoldman wrote:
> cishida wrote:
> > sammccall wrote:
> > > The comment here explicitly say "in a header map and". Unfortunately it 
> > > doesn't say why, and I don't know enough to guess.
> > nit/reminder: This should be updated now 
> Updated, LMK if you have any suggestions on the wording here.
LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117830

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

[PATCH] D117137: [Driver] Add CUDA support for --offload param

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

In D117137#3269365 , @yaxunl wrote:

> Does that mean only "spirv{64}-unknown-unknown" is acceptable, or 
> "spirv{64}-amd-unknown-unknown" is also acceptable?

My point is that `unknown` part of the triple is a catch-all for `anything, 
including something invalid` and should not be used for positive checks.
If we do not care about those parts of the triple ( accepting invalid triple 
would imply it), then we should not check those parts at all. 
Otherwise it leads to a weird inconsistency -- invalid triple like 
`spirv64-foo-bar`is accepted, but an equally nonsensical triple like 
`spir64-suse-whatever`which happens to use a known vendor or OS parts is not.

The bottom line is that if there's currently no known use of the vendor/OS/env 
parts of the triple, then we should not be checking them. 
If we do want to accept specific triple, then appropriate enums should be 
used/added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117137

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

[PATCH] D117830: [HeaderSearch] Track framework name in LookupFile

[David Goldman via Phabricator via cfe-commits]

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



Comment at: clang/lib/Lex/HeaderSearch.cpp:1043
 
 // If this file is found in a header map and uses the framework style of
 // includes, then this header is part of a framework we're building.

cishida wrote:
> sammccall wrote:
> > The comment here explicitly say "in a header map and". Unfortunately it 
> > doesn't say why, and I don't know enough to guess.
> nit/reminder: This should be updated now 
Updated, LMK if you have any suggestions on the wording here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117830

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

[PATCH] D117830: [HeaderSearch] Track framework name in LookupFile

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 403340.
dgoldman added a comment.

Update comment + run clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117830

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp


Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -187,6 +187,30 @@
 "Sub/Sub.h");
 }
 
+TEST_F(HeaderSearchTest, HeaderFrameworkLookup) {
+  std::string HeaderPath = "/tmp/Frameworks/Foo.framework/Headers/Foo.h";
+  addSystemFrameworkSearchDir("/tmp/Frameworks");
+  VFS->addFile(
+  HeaderPath, 0, llvm::MemoryBuffer::getMemBufferCopy("", HeaderPath),
+  /*User=*/None, /*Group=*/None, llvm::sys::fs::file_type::regular_file);
+
+  bool IsFrameworkFound = false;
+  const DirectoryLookup *CurDir = nullptr;
+  auto FoundFile = Search.LookupFile(
+  "Foo/Foo.h", SourceLocation(), /*isAngled=*/true, /*FromDir=*/nullptr,
+  CurDir, /*Includers=*/{}, /*SearchPath=*/nullptr,
+  /*RelativePath=*/nullptr, /*RequestingModule=*/nullptr,
+  /*SuggestedModule=*/nullptr, /*IsMapped=*/nullptr, &IsFrameworkFound);
+
+  EXPECT_TRUE(FoundFile.hasValue());
+  EXPECT_TRUE(IsFrameworkFound);
+  auto &FE = FoundFile.getValue();
+  auto FI = Search.getExistingFileInfo(FE);
+  EXPECT_TRUE(FI);
+  EXPECT_TRUE(FI->IsValid);
+  EXPECT_EQ(FI->Framework.str(), "Foo");
+}
+
 // Helper struct with null terminator character to make MemoryBuffer happy.
 template 
 struct NullTerminatedFile : public FileTy {
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1040,8 +1040,9 @@
   }
 }
 
-// If this file is found in a header map and uses the framework style of
-// includes, then this header is part of a framework we're building.
+// Set the `Framework` info if this file is in a header map with framework
+// style include spelling or found in a framework dir. The header map case
+// is possible when building frameworks which use header maps.
 if (CurDir->isHeaderMap() && isAngled) {
   size_t SlashPos = Filename.find('/');
   if (SlashPos != StringRef::npos)
@@ -1049,6 +1050,11 @@
 getUniqueFrameworkName(StringRef(Filename.begin(), SlashPos));
   if (CurDir->isIndexHeaderMap())
 HFI.IndexHeaderMapHeader = 1;
+} else if (CurDir->isFramework()) {
+  size_t SlashPos = Filename.find('/');
+  if (SlashPos != StringRef::npos)
+HFI.Framework =
+getUniqueFrameworkName(StringRef(Filename.begin(), SlashPos));
 }
 
 if (checkMSVCHeaderSearch(Diags, MSFE ? &MSFE->getFileEntry() : nullptr,


Index: clang/unittests/Lex/HeaderSearchTest.cpp
===
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -187,6 +187,30 @@
 "Sub/Sub.h");
 }
 
+TEST_F(HeaderSearchTest, HeaderFrameworkLookup) {
+  std::string HeaderPath = "/tmp/Frameworks/Foo.framework/Headers/Foo.h";
+  addSystemFrameworkSearchDir("/tmp/Frameworks");
+  VFS->addFile(
+  HeaderPath, 0, llvm::MemoryBuffer::getMemBufferCopy("", HeaderPath),
+  /*User=*/None, /*Group=*/None, llvm::sys::fs::file_type::regular_file);
+
+  bool IsFrameworkFound = false;
+  const DirectoryLookup *CurDir = nullptr;
+  auto FoundFile = Search.LookupFile(
+  "Foo/Foo.h", SourceLocation(), /*isAngled=*/true, /*FromDir=*/nullptr,
+  CurDir, /*Includers=*/{}, /*SearchPath=*/nullptr,
+  /*RelativePath=*/nullptr, /*RequestingModule=*/nullptr,
+  /*SuggestedModule=*/nullptr, /*IsMapped=*/nullptr, &IsFrameworkFound);
+
+  EXPECT_TRUE(FoundFile.hasValue());
+  EXPECT_TRUE(IsFrameworkFound);
+  auto &FE = FoundFile.getValue();
+  auto FI = Search.getExistingFileInfo(FE);
+  EXPECT_TRUE(FI);
+  EXPECT_TRUE(FI->IsValid);
+  EXPECT_EQ(FI->Framework.str(), "Foo");
+}
+
 // Helper struct with null terminator character to make MemoryBuffer happy.
 template 
 struct NullTerminatedFile : public FileTy {
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -1040,8 +1040,9 @@
   }
 }
 
-// If this file is found in a header map and uses the framework style of
-// includes, then this header is part of a framework we're building.
+// Set the `Framework` info if this file is in a header map with framework
+// style include spelling or found in a framework dir. The header map case
+// is possible when building frameworks which use header maps.
 if (CurDir->isHeaderMap() && isAngled) {
   size_t SlashPos = Fil

[PATCH] D117600: [CGCall] Annotate operator new with inaccessiblememonly if AssumeSaneOperatorNew is on

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

xbolva00 added a comment.

Test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117600

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

[PATCH] D117600: [CGCall] Annotate operator new with inaccessiblememonly if AssumeSaneOperatorNew is on

[Bryce Wilson via Phabricator via cfe-commits]

Bryce-MW added inline comments.



Comment at: clang/lib/CodeGen/CGCall.cpp:2071
   RetAttrs.addAttribute(llvm::Attribute::NoAlias);
+  FuncAttrs.addAttribute("inaccessiblememonly");
+}

nikic wrote:
> This should be `Attribute::InacccessibleMemOnly`. It's not a string attribute.
Thanks, I tried to look for a list but I didn't have any luck (and my IDE was 
having trouble loading the CMake project so I couldn't jump to definition on 
another attribute).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117600

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

[PATCH] D117600: [CGCall] Annotate operator new with inaccessiblememonly if AssumeSaneOperatorNew is on

[Bryce Wilson via Phabricator via cfe-commits]

Bryce-MW updated this revision to Diff 403339.
Bryce-MW marked an inline comment as done.
Bryce-MW added a comment.

- Use proper attribute


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117600

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp


Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2062,11 +2062,14 @@
   AddAttributesFromFunctionProtoType(
   getContext(), FuncAttrs, Fn->getType()->getAs());
   if (AttrOnCallSite && Fn->isReplaceableGlobalAllocationFunction()) {
-// A sane operator new returns a non-aliasing pointer.
+// A sane operator new returns a non-aliasing pointer and is
+// inaccessiblememonly
 auto Kind = Fn->getDeclName().getCXXOverloadedOperator();
 if (getCodeGenOpts().AssumeSaneOperatorNew &&
-(Kind == OO_New || Kind == OO_Array_New))
+(Kind == OO_New || Kind == OO_Array_New)) {
   RetAttrs.addAttribute(llvm::Attribute::NoAlias);
+  FuncAttrs.addAttribute(llvm::Attribute::InacccessibleMemOnly);
+}
   }
   const CXXMethodDecl *MD = dyn_cast(Fn);
   const bool IsVirtualCall = MD && MD->isVirtual();
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2256,7 +2256,7 @@
 def fno_asm : Flag<["-"], "fno-asm">, Group;
 def fno_asynchronous_unwind_tables : Flag<["-"], 
"fno-asynchronous-unwind-tables">, Group;
 def fno_assume_sane_operator_new : Flag<["-"], 
"fno-assume-sane-operator-new">, Group,
-  HelpText<"Don't assume that C++'s global operator new can't alias any 
pointer">,
+  HelpText<"Don't assume that C++'s global operator new can't alias any 
pointer or access accessible memory">,
   Flags<[CC1Option]>, 
MarshallingInfoNegativeFlag>;
 def fno_builtin : Flag<["-"], "fno-builtin">, Group, 
Flags<[CC1Option, CoreOption]>,
   HelpText<"Disable implicit builtin knowledge of functions">;


Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2062,11 +2062,14 @@
   AddAttributesFromFunctionProtoType(
   getContext(), FuncAttrs, Fn->getType()->getAs());
   if (AttrOnCallSite && Fn->isReplaceableGlobalAllocationFunction()) {
-// A sane operator new returns a non-aliasing pointer.
+// A sane operator new returns a non-aliasing pointer and is
+// inaccessiblememonly
 auto Kind = Fn->getDeclName().getCXXOverloadedOperator();
 if (getCodeGenOpts().AssumeSaneOperatorNew &&
-(Kind == OO_New || Kind == OO_Array_New))
+(Kind == OO_New || Kind == OO_Array_New)) {
   RetAttrs.addAttribute(llvm::Attribute::NoAlias);
+  FuncAttrs.addAttribute(llvm::Attribute::InacccessibleMemOnly);
+}
   }
   const CXXMethodDecl *MD = dyn_cast(Fn);
   const bool IsVirtualCall = MD && MD->isVirtual();
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2256,7 +2256,7 @@
 def fno_asm : Flag<["-"], "fno-asm">, Group;
 def fno_asynchronous_unwind_tables : Flag<["-"], "fno-asynchronous-unwind-tables">, Group;
 def fno_assume_sane_operator_new : Flag<["-"], "fno-assume-sane-operator-new">, Group,
-  HelpText<"Don't assume that C++'s global operator new can't alias any pointer">,
+  HelpText<"Don't assume that C++'s global operator new can't alias any pointer or access accessible memory">,
   Flags<[CC1Option]>, MarshallingInfoNegativeFlag>;
 def fno_builtin : Flag<["-"], "fno-builtin">, Group, Flags<[CC1Option, CoreOption]>,
   HelpText<"Disable implicit builtin knowledge of functions">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117744: [Driver] Remove obsoleted -gz=zlib-gnu

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Ping:)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117744

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

[PATCH] D117939: [clang-tidy] Add more documentation about check development

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.



Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:362
+Private custom matchers are a good example of auxiliary support code for your 
check
+that is best tested with a unit test.  It will be easier to test your matchers 
or
+other support classes by writing a unit test than by writing a ``FileCheck`` 
integration

aaron.ballman wrote:
> LegalizeAdulthood wrote:
> > If I change the wording from "is best tested with a unit test" to "can be 
> > tested with a unit test",
> > would that alleviate the concern?  I want to encourage appropriate testing 
> > and unit testing
> > complex helper code is better than integration testing helper code.
> > 
> > I find it easier to have confidence in private matchers if they are unit 
> > tested and I've recently
> > had a few situations where I had to write relatively complex helper 
> > functions to analyze raw
> > text that I felt would have been better tested with a unit test than an 
> > integration test.
> > If I change the wording from "is best tested with a unit test" to "can be 
> > tested with a unit test", would that alleviate the concern?
> 
> I largely addresses mine -- saying it can be done is great, saying it should 
> be done is what gave me pause.
+1



Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:389-390
+- Test your check under both Windows and Linux environments.
+- Watch out for high false positive rates; ideally there should be none, but a
+  small number may be tolerable.  High false positive rates prevent adoption.
+- Consider configurable options for your check to deal with false positives.

aaron.ballman wrote:
> LegalizeAdulthood wrote:
> > ymandel wrote:
> > > Is it worth giving a rule-of-thumb? Personally I'd go with < 10%, all 
> > > things being equal.  A check for a serious bug may reasonably have a 
> > > higher false positive rate, and trivial checks might not justify *any* 
> > > false positives. But, if neither of these apply, I'd recommend 10% as the 
> > > default.
> > I'm OK with rule-of-thumb 10% advice.
> FWIW, I think 10% is pretty arbitrary and I'd rather not see us try to nail 
> it down to a concrete number. In practical terms, it really depends on the 
> check.
> 
> Also, clang-tidy is where we put things with a "high" false positive rate 
> already, so this statement has implications on what an acceptable false 
> positive rate is for Clang or the static analyzer.
> 
> How about something along these lines:
> ```
> - Watch out for high false positive rates. Ideally, a check would have no 
> false positives, but given that matching against an AST is not control- or 
> data flow-sensitive, a number of false positives are expected. The higher the 
> false positive rate, the less likely the check will be adopted in practice. 
> Mechanisms should be put in place to help the user manage false positives.
> - There are two primary mechanisms for managing false positives: supporting a 
> code pattern which allows the programmer to silence the diagnostic in an ad 
> hoc manner and check configuration options to control the behavior of the 
> check.
> - Consider supporting a code pattern to allow the programmer to silence the 
> diagnostic whenever such a code pattern can clearly express the programmer's 
> intent. For example, allowing an explicit cast to `void` to silence an unused 
> variable diagnostic.
> - Consider adding check configuration options to allow the user to opt into 
> more aggressive checking behavior without burdening users for the common 
> high-confidence cases.
> ```
> (or something along those lines). The basic idea I have there is: false 
> positives are expected, try to keep them to a minimum, and here are the two 
> most common ways code reviewers will ask you to handle false positives when 
> they're a concern.
Strongly agree. 10% has served as well in practice for the threshold at which 
we disable/fix checks, but it's definitely arbitrary. I much prefer your 
suggested approach.


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

https://reviews.llvm.org/D117939

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

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

[Arthur Eubanks 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 rGb1613f05ae0c: [NFC] Store Address's alignment into 
PointerIntPairs (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117262

Files:
  clang/lib/CodeGen/Address.h

Index: clang/lib/CodeGen/Address.h
===
--- clang/lib/CodeGen/Address.h
+++ clang/lib/CodeGen/Address.h
@@ -14,30 +14,79 @@
 #ifndef LLVM_CLANG_LIB_CODEGEN_ADDRESS_H
 #define LLVM_CLANG_LIB_CODEGEN_ADDRESS_H
 
-#include "llvm/IR/Constants.h"
 #include "clang/AST/CharUnits.h"
+#include "llvm/ADT/PointerIntPair.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/Support/MathExtras.h"
 
 namespace clang {
 namespace CodeGen {
 
-/// An aligned address.
-class Address {
+namespace {
+// We try to save some space by using 6 bits over two PointerIntPairs to store
+// the alignment. However, some arches don't support 3 bits in a PointerIntPair
+// so we fallback to storing the alignment separately.
+template = 8> class AddressImpl {};
+
+template  class AddressImpl {
   llvm::Value *Pointer;
   llvm::Type *ElementType;
   CharUnits Alignment;
 
+public:
+  AddressImpl(llvm::Value *Pointer, llvm::Type *ElementType,
+  CharUnits Alignment)
+  : Pointer(Pointer), ElementType(ElementType), Alignment(Alignment) {}
+  llvm::Value *getPointer() const { return Pointer; }
+  llvm::Type *getElementType() const { return ElementType; }
+  CharUnits getAlignment() const { return Alignment; }
+};
+
+template  class AddressImpl {
+  // Int portion stores upper 3 bits of the log of the alignment.
+  llvm::PointerIntPair Pointer;
+  // Int portion stores lower 3 bits of the log of the alignment.
+  llvm::PointerIntPair ElementType;
+
+public:
+  AddressImpl(llvm::Value *Pointer, llvm::Type *ElementType,
+  CharUnits Alignment)
+  : Pointer(Pointer), ElementType(ElementType) {
+if (Alignment.isZero())
+  return;
+// Currently the max supported alignment is much less than 1 << 63 and is
+// guaranteed to be a power of 2, so we can store the log of the alignment
+// into 6 bits.
+assert(Alignment.isPowerOfTwo() && "Alignment cannot be zero");
+auto AlignLog = llvm::Log2_64(Alignment.getQuantity());
+assert(AlignLog < (1 << 6) && "cannot fit alignment into 6 bits");
+this->Pointer.setInt(AlignLog >> 3);
+this->ElementType.setInt(AlignLog & 7);
+  }
+  llvm::Value *getPointer() const { return Pointer.getPointer(); }
+  llvm::Type *getElementType() const { return ElementType.getPointer(); }
+  CharUnits getAlignment() const {
+unsigned AlignLog = (Pointer.getInt() << 3) | ElementType.getInt();
+return CharUnits::fromQuantity(1UL << AlignLog);
+  }
+};
+} // namespace
+
+/// An aligned address.
+class Address {
+  AddressImpl A;
+
 protected:
-  Address(std::nullptr_t) : Pointer(nullptr), ElementType(nullptr) {}
+  Address(std::nullptr_t) : A(nullptr, nullptr, CharUnits::Zero()) {}
 
 public:
-  Address(llvm::Value *pointer, llvm::Type *elementType, CharUnits alignment)
-  : Pointer(pointer), ElementType(elementType), Alignment(alignment) {
-assert(pointer != nullptr && "Pointer cannot be null");
-assert(elementType != nullptr && "Element type cannot be null");
-assert(llvm::cast(pointer->getType())
-   ->isOpaqueOrPointeeTypeMatches(elementType) &&
+  Address(llvm::Value *Pointer, llvm::Type *ElementType, CharUnits Alignment)
+  : A(Pointer, ElementType, Alignment) {
+assert(Pointer != nullptr && "Pointer cannot be null");
+assert(ElementType != nullptr && "Element type cannot be null");
+assert(llvm::cast(Pointer->getType())
+   ->isOpaqueOrPointeeTypeMatches(ElementType) &&
"Incorrect pointer element type");
-assert(!alignment.isZero() && "Alignment cannot be zero");
   }
 
   // Deprecated: Use constructor with explicit element type instead.
@@ -46,11 +95,11 @@
 Alignment) {}
 
   static Address invalid() { return Address(nullptr); }
-  bool isValid() const { return Pointer != nullptr; }
+  bool isValid() const { return A.getPointer() != nullptr; }
 
   llvm::Value *getPointer() const {
 assert(isValid());
-return Pointer;
+return A.getPointer();
   }
 
   /// Return the type of the pointer value.
@@ -61,7 +110,7 @@
   /// Return the type of the values stored in this address.
   llvm::Type *getElementType() const {
 assert(isValid());
-return ElementType;
+return A.getElementType();
   }
 
   /// Return the address space that this address resides in.
@@ -77,19 +126,19 @@
   /// Return the alignment of this pointer.
   CharUnits getAlignment() const {
 assert(isValid());
-return Alignment;
+return A.getAlignment();
   }
 
   /// Return address with different po

[clang] b1613f0 - [NFC] Store Address's alignment into PointerIntPairs

[Arthur Eubanks via cfe-commits]

Author: Arthur Eubanks
Date: 2022-01-26T10:35:28-08:00
New Revision: b1613f05ae0ce4efc6b6475ea4459957ebcb0150

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

LOG: [NFC] Store Address's alignment into PointerIntPairs

This mitigates the extra memory caused by D115725.

On 32-bit arches where we only have 2 bits per PointerIntPair we fall
back to simply storing alignment separately.

Reviewed By: rnk, nikic

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

Added: 


Modified: 
clang/lib/CodeGen/Address.h

Removed: 




diff  --git a/clang/lib/CodeGen/Address.h b/clang/lib/CodeGen/Address.h
index 37c20291c0e80..06b82e404cce4 100644
--- a/clang/lib/CodeGen/Address.h
+++ b/clang/lib/CodeGen/Address.h
@@ -14,30 +14,79 @@
 #ifndef LLVM_CLANG_LIB_CODEGEN_ADDRESS_H
 #define LLVM_CLANG_LIB_CODEGEN_ADDRESS_H
 
-#include "llvm/IR/Constants.h"
 #include "clang/AST/CharUnits.h"
+#include "llvm/ADT/PointerIntPair.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/Support/MathExtras.h"
 
 namespace clang {
 namespace CodeGen {
 
-/// An aligned address.
-class Address {
+namespace {
+// We try to save some space by using 6 bits over two PointerIntPairs to store
+// the alignment. However, some arches don't support 3 bits in a PointerIntPair
+// so we fallback to storing the alignment separately.
+template = 8> class AddressImpl {};
+
+template  class AddressImpl {
   llvm::Value *Pointer;
   llvm::Type *ElementType;
   CharUnits Alignment;
 
+public:
+  AddressImpl(llvm::Value *Pointer, llvm::Type *ElementType,
+  CharUnits Alignment)
+  : Pointer(Pointer), ElementType(ElementType), Alignment(Alignment) {}
+  llvm::Value *getPointer() const { return Pointer; }
+  llvm::Type *getElementType() const { return ElementType; }
+  CharUnits getAlignment() const { return Alignment; }
+};
+
+template  class AddressImpl {
+  // Int portion stores upper 3 bits of the log of the alignment.
+  llvm::PointerIntPair Pointer;
+  // Int portion stores lower 3 bits of the log of the alignment.
+  llvm::PointerIntPair ElementType;
+
+public:
+  AddressImpl(llvm::Value *Pointer, llvm::Type *ElementType,
+  CharUnits Alignment)
+  : Pointer(Pointer), ElementType(ElementType) {
+if (Alignment.isZero())
+  return;
+// Currently the max supported alignment is much less than 1 << 63 and is
+// guaranteed to be a power of 2, so we can store the log of the alignment
+// into 6 bits.
+assert(Alignment.isPowerOfTwo() && "Alignment cannot be zero");
+auto AlignLog = llvm::Log2_64(Alignment.getQuantity());
+assert(AlignLog < (1 << 6) && "cannot fit alignment into 6 bits");
+this->Pointer.setInt(AlignLog >> 3);
+this->ElementType.setInt(AlignLog & 7);
+  }
+  llvm::Value *getPointer() const { return Pointer.getPointer(); }
+  llvm::Type *getElementType() const { return ElementType.getPointer(); }
+  CharUnits getAlignment() const {
+unsigned AlignLog = (Pointer.getInt() << 3) | ElementType.getInt();
+return CharUnits::fromQuantity(1UL << AlignLog);
+  }
+};
+} // namespace
+
+/// An aligned address.
+class Address {
+  AddressImpl A;
+
 protected:
-  Address(std::nullptr_t) : Pointer(nullptr), ElementType(nullptr) {}
+  Address(std::nullptr_t) : A(nullptr, nullptr, CharUnits::Zero()) {}
 
 public:
-  Address(llvm::Value *pointer, llvm::Type *elementType, CharUnits alignment)
-  : Pointer(pointer), ElementType(elementType), Alignment(alignment) {
-assert(pointer != nullptr && "Pointer cannot be null");
-assert(elementType != nullptr && "Element type cannot be null");
-assert(llvm::cast(pointer->getType())
-   ->isOpaqueOrPointeeTypeMatches(elementType) &&
+  Address(llvm::Value *Pointer, llvm::Type *ElementType, CharUnits Alignment)
+  : A(Pointer, ElementType, Alignment) {
+assert(Pointer != nullptr && "Pointer cannot be null");
+assert(ElementType != nullptr && "Element type cannot be null");
+assert(llvm::cast(Pointer->getType())
+   ->isOpaqueOrPointeeTypeMatches(ElementType) &&
"Incorrect pointer element type");
-assert(!alignment.isZero() && "Alignment cannot be zero");
   }
 
   // Deprecated: Use constructor with explicit element type instead.
@@ -46,11 +95,11 @@ class Address {
 Alignment) {}
 
   static Address invalid() { return Address(nullptr); }
-  bool isValid() const { return Pointer != nullptr; }
+  bool isValid() const { return A.getPointer() != nullptr; }
 
   llvm::Value *getPointer() const {
 assert(isValid());
-return Pointer;
+return A.getPointer();
   }
 
   /// Return the type of the pointer value.
@@ -61,7 +110,7 @@ class Address {
   /// Return the type of the values stored in this address.
   llvm::Type *getElem

[PATCH] D117929: [XRay] Add support for RISCV

[Ashwin Poduval via Phabricator via cfe-commits]

ashwin98 updated this revision to Diff 403331.
ashwin98 added a comment.

Made changes to handle lint issues.


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

https://reviews.llvm.org/D117929

Files:
  clang/lib/Driver/XRayArgs.cpp
  compiler-rt/cmake/Modules/AllSupportedArchDefs.cmake
  compiler-rt/lib/xray/CMakeLists.txt
  compiler-rt/lib/xray/xray_interface.cpp
  compiler-rt/lib/xray/xray_riscv.cpp
  compiler-rt/lib/xray/xray_trampoline_riscv32.S
  compiler-rt/lib/xray/xray_trampoline_riscv64.S
  compiler-rt/lib/xray/xray_tsc.h
  llvm/lib/CodeGen/XRayInstrumentation.cpp
  llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll

Index: llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/xray-attribute-instrumentation.ll
@@ -0,0 +1,70 @@
+; RUN: llc -mtriple=riscv32-unknown-elf -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK %s
+; RUN: llc -mtriple=riscv32-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK %s
+; RUN: llc -mtriple=riscv64-unknown-elf -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+; RUN: llc -mtriple=riscv64-unknown-linux-gnu -mattr=+d -verify-machineinstrs < %s | FileCheck --check-prefix=CHECK --check-prefix=CHECK-RISCV64 %s
+
+define i32 @foo() nounwind "function-instrument"="xray-always" {
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_0:
+; CHECK-NEXT:   j .Ltmp0
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-LABEL:  .Ltmp0:
+  ret i32 0
+; CHECK:.p2align 2
+; CHECK-LABEL:  .Lxray_sled_1:
+; CHECK-NEXT:   j .Ltmp1
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-NEXT:   nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-RISCV64:nop
+; CHECK-LABEL:  .Ltmp1:
+; CHECK-NEXT:   ret
+}
+; CHECK:.section xray_instr_map,"ao",@progbits,foo
+; CHECK-LABEL:  .Lxray_sleds_start0:
+; CHECK:.Lxray_sled_0-.Ltmp2
+; CHECK:.Lxray_sled_1-.Ltmp3
+; CHECK-LABEL:  .Lxray_sleds_end0:
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -187,6 +187,9 @@
   unsigned getMaxInterleaveFactor() const {
 return hasVInstructions() ? MaxInterleaveFactor : 1;
   }
+  // Add XRay support - needs double precision floats at present and does not
+  // support compressed instructions
+  bool isXRaySupported() const override { return hasStdExtD() && !hasStdExtC(); }
 
 protected:
   // GlobalISel related APIs.
Index: llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
===
--- llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
+++ llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
@@ -15,6 +15,7 @@
 #include "MCTargetDesc/RISCVMCExpr.h"
 #include "MCTargetDesc/RISCVTargetStreamer.h"
 #include "RISCV.h"
+#include "RISCVSubtarget.h"
 #include "RISCVTargetMachine.h"
 #include "TargetInfo/RISCVTargetInfo.h"
 #include "llvm/ADT/Statistic.h"
@@ -25,10 +26,12 @@
 #include "llvm/CodeGen/MachineModuleInfo.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCInst.h"
+#include "llvm/MC/MCInstBuilder.h"
 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 using namespace llvm;
 
 #define DEBUG_TYPE "asm-printer"
@@ -65,11 +68,18 @@
 return LowerRISCVMach

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.

lgtm

BTW, as a follow-on cleanup, which I may attempt, I think we could convert this 
alignment from CharUnits to llvm::Align. The vast majority of the 
`getAlignment` callers feed it to CGBuilder::CreateAlignedLoad/Store:
http://llvm-cs.pcc.me.uk/tools/clang/lib/CodeGen/Address.h/rgetAlignment
Those call IRBuilder::CreateAlignedLoad/Store, which accept MaybeAlign.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117262

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

[PATCH] D116542: [OpenMP] Add a flag for embedding a file into the module

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/include/clang/Basic/CodeGenOptions.h:279
 
+  /// List of file passed with -fembed-offload-binary option to embed
+  /// device-side offloading binaries in the host object file.

JonChesterfield wrote:
> This is unclear. List in what sense? It's a std::string, which suggests it's 
> a freeform argument that gets parsed somewhere else.
> 
> The relation between the two strings is also unclear. Are the lists meant to 
> be the same length, implying a one to one mapping? If there is a strong 
> relation between the two we can probably remove a bunch of error handling by 
> taking one argument instead of two.
> 
> Perhaps the variable should be something like a vector section>>?
It's a list of comma separated values, but you're right. This should be parsed 
out when we handle the flags.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1757
+
+  assert(BinaryFilenames.size() == BinarySections.size() &&
+ "Different number of filenames and section names in embedding");

JonChesterfield wrote:
> Definitely don't want to assert on invalid commandline argument
Will remove.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1774
+  SectionName += ".";
+  SectionName += *BinarySection;
+}

JonChesterfield wrote:
> This looks lossy - if two files use the same section name, they'll end up 
> appended in an order that is probably an implementation quirk of llvm-link, 
> and I think we've thrown away the filename info so can't get back to where we 
> were.
> 
> Would .llvm.offloading.filename be a reasonable name for each section, with 
> either error on duplicates or warning + discard?
We only care about the sections per-file right. When I extract these in the 
`linker-wrapper` I simply look at each file's sections, and put them into a 
list of device inputs, we don't need them to be unique as long as there aren't 
multiple in the same file.



Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4982
+  Type *UsedElementType = Type::getInt8Ty(M.getContext())->getPointerTo(0);
+  GlobalVariable *Used = collectUsedGlobalVariables(M, UsedGlobals, true);
+  for (auto *GV : UsedGlobals) {

JonChesterfield wrote:
> I think I've written some handling very like this in an LDS pass that I meant 
> to factor out for reuse but haven't got around to - we're removing a value 
> from compiler.used?
This handling of the compiler.used variable was copied from the implementation 
of `-fembed-bitcode` above. I wasn't sure about their methodology but I figured 
it worked for them. I can tidy it up to be more straightforward.



Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4994
+
+  // Embed the data in the
+  llvm::Constant *ModuleConstant =

JonChesterfield wrote:
> missing end of comment
Will fix.



Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:5003
+  // sections after linking.
+  GV->setAlignment(Align(1));
+  UsedArray.push_back(

JonChesterfield wrote:
> Is this necessary? 1 seems a likely default for a uint8_t array
Will remove.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116542

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

[PATCH] D116542: [OpenMP] Add a flag for embedding a file into the module

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4934
 assert(Old->hasOneUse() &&
"llvm.embedded.module can only be used once in llvm.compiler.used");
 GV->takeName(Old);

here's an assert that this function only handled one at a time



Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4968
 
   // Recreate llvm.compiler.used.
   ArrayType *ATy = ArrayType::get(UsedElementType, UsedArray.size());

This is probably better written in terms of a call to a function like 
removeFromUsedList, which exists in AMDGPULowerModuleLDSPass at present because 
I forgot to move it and failed to find an existing implementation. Perhaps 
something that should be next to llvm::collectUsedGlobalVariables in Module.cpp?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116542

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

[PATCH] D117616: GCC ABI Compatibility: Preserve alignment of non-pod members in packed structs

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a subscriber: Xiangling_L.
dblaikie added a comment.

@Xiangling_L - I see you made an ABI change for AIX in 
rG05ad8e942996f36cc694478542ccd84aa5bbb80f 
 - any 
idea if this new ABI break/fix is relevant to AIX? Would you want to stay with 
the current Clang layout (like Apple and Sony are) or would you want the fix to 
be compatible with GCC?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117616

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

[PATCH] D116542: [OpenMP] Add a flag for embedding a file into the module

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4891
   GlobalVariable *Used = collectUsedGlobalVariables(M, UsedGlobals, true);
   for (auto *GV : UsedGlobals) {
 if (GV->getName() != "llvm.embedded.module" &&

This seems to be where the code was copied from. Looks broken here too, unless 
there's some constraint which means exactly one things is embedded at a time - 
which might be the case given the .module suffix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116542

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

[PATCH] D116542: [OpenMP] Add a flag for embedding a file into the module

[Jon Chesterfield via Phabricator via cfe-commits]

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

I don't think this works for multiple files. The test only tries one and misses 
all the parsing handling.




Comment at: clang/include/clang/Basic/CodeGenOptions.h:279
 
+  /// List of file passed with -fembed-offload-binary option to embed
+  /// device-side offloading binaries in the host object file.

This is unclear. List in what sense? It's a std::string, which suggests it's a 
freeform argument that gets parsed somewhere else.

The relation between the two strings is also unclear. Are the lists meant to be 
the same length, implying a one to one mapping? If there is a strong relation 
between the two we can probably remove a bunch of error handling by taking one 
argument instead of two.

Perhaps the variable should be something like a vector>?



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1757
+
+  assert(BinaryFilenames.size() == BinarySections.size() &&
+ "Different number of filenames and section names in embedding");

Definitely don't want to assert on invalid commandline argument



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1774
+  SectionName += ".";
+  SectionName += *BinarySection;
+}

This looks lossy - if two files use the same section name, they'll end up 
appended in an order that is probably an implementation quirk of llvm-link, and 
I think we've thrown away the filename info so can't get back to where we were.

Would .llvm.offloading.filename be a reasonable name for each section, with 
either error on duplicates or warning + discard?



Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4982
+  Type *UsedElementType = Type::getInt8Ty(M.getContext())->getPointerTo(0);
+  GlobalVariable *Used = collectUsedGlobalVariables(M, UsedGlobals, true);
+  for (auto *GV : UsedGlobals) {

I think I've written some handling very like this in an LDS pass that I meant 
to factor out for reuse but haven't got around to - we're removing a value from 
compiler.used?



Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4983
+  GlobalVariable *Used = collectUsedGlobalVariables(M, UsedGlobals, true);
+  for (auto *GV : UsedGlobals) {
+if (!GV->getName().startswith("llvm.embedded.object"))

This removes all variables with that prefix



Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4994
+
+  // Embed the data in the
+  llvm::Constant *ModuleConstant =

missing end of comment



Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:5003
+  // sections after linking.
+  GV->setAlignment(Align(1));
+  UsedArray.push_back(

Is this necessary? 1 seems a likely default for a uint8_t array



Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:5007
+
+  // Recreate llvm.compiler.used.
+  ArrayType *ATy = ArrayType::get(UsedElementType, UsedArray.size());

And this pushes one global back into that array

So this looks like it'll mark the last embedded file as .used and none of the 
others


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116542

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

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

[Nikita Popov via Phabricator via cfe-commits]

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

LG from my side


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117262

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

[PATCH] D117262: [NFC] Store Address's alignment into PointerIntPairs

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added inline comments.



Comment at: clang/lib/CodeGen/Address.h:29
+// so we fallback to storing the alignment separately.
+template = 8> class AddressImpl {};
+

nikic wrote:
> Why do we need the extra T parameter?
without it we end up instantiating `AddressImpl` unconditionally (because 
all template parameters are known?) which static_asserts in 32 bit builds


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117262

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