[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

2023-05-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

Just an update on this, it works well but some of the cases are not tolerant to 
comments, I'm working my way through those issues


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

https://reviews.llvm.org/D148467

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


[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

2023-05-03 Thread Alexander Shaposhnikov via Phabricator via cfe-commits
alexander-shaposhnikov updated this revision to Diff 518990.
alexander-shaposhnikov added a comment.

Simplify code a little bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

Files:
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/Sema/Template.h
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/SemaTemplate/concepts-friends.cpp
  clang/test/SemaTemplate/concepts-out-of-line-def.cpp
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -816,3 +816,101 @@
 static_assert(Parent::TakesBinary::i == 0);
 }
 
+namespace TemplateInsideNonTemplateClass {
+template concept C = true;
+
+template auto L = [] U>() {};
+
+struct Q {
+  template U> friend constexpr auto decltype(L)::operator()() const;
+};
+
+template 
+concept C1 = false;
+
+struct Foo {
+  template 
+  struct Bar {};
+
+  template 
+requires(C1)
+  struct Bar;
+};
+
+Foo::Bar BarInstance;
+} // namespace TemplateInsideNonTemplateClass
+
+namespace GH61959 {
+template 
+concept C = (sizeof(T0) >= 4);
+
+template
+struct Orig { };
+
+template
+struct Orig {
+  template requires C
+  void f() { }
+
+  template requires true
+  void f() { }
+};
+
+template  struct Mod {};
+
+template 
+struct Mod {
+  template  requires C
+  constexpr static int f() { return 1; }
+
+  template  requires C
+  constexpr static int f() { return 2; }
+};
+
+static_assert(Mod::f() == 1);
+static_assert(Mod::f() == 2);
+
+template
+struct Outer {
+  template
+  struct Inner {};
+
+  template
+  struct Inner {
+template
+void foo()  requires C && C && C{}
+template
+void foo()  requires true{}
+  };
+};
+
+void bar() {
+  Outer::Inner I;
+  I.foo();
+}
+} // namespace GH61959
+
+
+namespace TemplateInsideTemplateInsideTemplate {
+template
+concept C1 = false;
+
+template 
+struct W0 {
+  template 
+  struct W1 {
+template 
+struct F {
+  enum { value = 1 };
+};
+
+template 
+  requires C1
+struct F {
+  enum { value = 2 };
+};
+  };
+};
+
+static_assert(W0<0>::W1<1>::F::value == 1);
+} // TemplateInsideTemplateInsideTemplate
Index: clang/test/SemaTemplate/concepts-out-of-line-def.cpp
===
--- clang/test/SemaTemplate/concepts-out-of-line-def.cpp
+++ clang/test/SemaTemplate/concepts-out-of-line-def.cpp
@@ -127,3 +127,220 @@
 static_assert(S::specialization("str") == SPECIALIZATION_REQUIRES);
 
 } // namespace multiple_template_parameter_lists
+
+static constexpr int CONSTRAINED_METHOD_1 = 1;
+static constexpr int CONSTRAINED_METHOD_2 = 2;
+
+namespace constrained_members {
+
+template 
+struct S {
+  template 
+  static constexpr int constrained_method();
+};
+
+template <>
+template 
+constexpr int S<1>::constrained_method() { return CONSTRAINED_METHOD_1; }
+
+template <>
+template 
+constexpr int S<2>::constrained_method() { return CONSTRAINED_METHOD_2; }
+
+static_assert(S<1>::constrained_method() == CONSTRAINED_METHOD_1);
+static_assert(S<2>::constrained_method() == CONSTRAINED_METHOD_2);
+
+
+template 
+concept ConceptT1T2 = true;
+
+template
+struct S12 {
+  template T4>
+  static constexpr int constrained_method();
+};
+
+template<>
+template T5>
+constexpr int S12::constrained_method() { return CONSTRAINED_METHOD_1; }
+
+template<>
+template T5>
+constexpr int S12::constrained_method() { return CONSTRAINED_METHOD_2; }
+
+static_assert(S12::constrained_method() == CONSTRAINED_METHOD_1);
+static_assert(S12::constrained_method() == CONSTRAINED_METHOD_2);
+
+} // namespace constrained members
+
+namespace constrained_members_of_nested_types {
+
+template 
+struct S {
+  struct Inner0 {
+struct Inner1 {
+  template 
+  static constexpr int constrained_method();
+};
+  };
+};
+
+template <>
+template 
+constexpr int S<1>::Inner0::Inner1::constrained_method() { return CONSTRAINED_METHOD_1; }
+
+template <>
+template 
+constexpr int S<2>::Inner0::Inner1::constrained_method() { return CONSTRAINED_METHOD_2; }
+
+static_assert(S<1>::Inner0::Inner1::constrained_method() == CONSTRAINED_METHOD_1);
+static_assert(S<2>::Inner0::Inner1::constrained_method() == CONSTRAINED_METHOD_2);
+
+
+template 
+concept ConceptT1T2 = true;
+
+template
+struct S12 {
+  struct Inner0 {
+struct Inner1 {
+  template T4>
+  static constexpr int constrained_method();
+};
+  };
+};
+
+template<>
+template T5>
+constexpr int S12::Inner0::Inner1::constrained_method() { return CONSTRAINED_METHOD_1; }
+
+template<>
+template T5>
+constexpr int S12::Inner0::Inner1::constrained_method() { return CONSTRAINED_METHOD_2

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

2023-05-03 Thread Alexander Shaposhnikov via Phabricator via cfe-commits
alexander-shaposhnikov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:133
 }
 
+Response HandlePartialClassTemplateSpec(

HandlePartialClassTemplateSpec is from Erich's diff 
(https://reviews.llvm.org/D147722)



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:344
  Innermost->asArray(), Final);
-
-  const Decl *CurDecl = ND;
+  CurDecl = Response::UseNextDecl(ND).NextDecl;
+  }

this bit is important.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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


[PATCH] D146054: [RISCV] Add --print-supported-extensions and -march=help support

2023-05-03 Thread Kito Cheng via Phabricator via cfe-commits
kito-cheng accepted this revision.
kito-cheng added a comment.

LGTM, consider about the GNU compatibility, I would that has -march=help form 
for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

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


[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

2023-05-03 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

Thanks for the patch, this is a topic we've discussed a couple times in the 
past as well as it has far reaching implications. I believe we're actually in a 
much better situation nowadays.

To summarise some concerns (thanks to @sammccall for useful discussion);

Features


There are certain features that make use of index, ~all of them assume a 
"stale" index already, so this shouldn't effect them as severely.  
The most impacted feature will be code completion, as we only deserialize decls 
needed to parse the main file (until code completion point) from the preamble 
and rely on the index for the rest. So we'll have partial results until 
indexing finishes.  
I believe this is still better than what we've today. As in the presence of a 
global index, we'll still have some results but they'll be stale. Today we're 
not showing any results (well, to be more precise we're showing the identifier 
based ones) until preamble indexing finishes.
Another big concern that doesn't hold today is preamble-changed callbacks. In 
theory clangd notifies clients that a new preamble is available, in case they 
want "fresh" information. Today it's only used for providing semantic 
highlights, which doesn't use index. But in the future, if we let clients fetch 
other information based on this notification, we should make sure freshness of 
preamble index is not required.
There's also going to be some impact on documentations for code completion 
items/hover, but i don't think that's unacceptable either (we can already have 
that as we make use of stale preambles after the first build).

RAM/CPU usage due to extra concurrency
--

This means we'll be running more tasks in parallel, so clangd's peak memory 
usage will increase. Without this patch we've 1 live (not serialized) AST & 1 
serialized AST in memory at peak as all the other tasks would be blocked until 
live AST is destroyed. After this patch we'll also have some tasks that're 
working on the serialized AST. But this isn't something new either, after first 
preamble build we're already hitting this peak usage when using a stale 
preamble and building a new one.
As for extra concurrency, again this is pretty similar to what we do after the 
first preamble build, we're only doing more work if the preamble gets 
invalidated multiple times while indexing is running. Without this patch we 
would only build the "last" version of the preamble, but after this patch we 
can trigger some extra intermediate builds. Since preamble updates are rare, I 
don't think this one is a big concern either.

Safety concerns
---

Taking a closer look at the code, we're already triggering indexing callback 
after preamble serialization is complete. Hence it seems like there are no 
other users of the live AST and other structs needed for indexing. These 
structs are also part of compiler instance as ref-counted objects, hence 
keeping them alive in the indexing task (with a similar approach to Sam's 
patch) should hopefully be safe, as the code that resets/destroys the compiler 
instance isn't putting those into an invalid state (AFAICT).
Only remaining concern is whether clangd itself has any objects that it passes 
into preamble builds with sole-ownership and invalidates/destroys them after 
the build finishes. I don't think that's the case at hindsight either.
Hopefully most of these concerns can also be verified in practice by running 
using an ASAN build of clangd for a while.

Implementation concerns
---

As pointed out, we should store ref-counted objects properly and pass them to 
the preamble callbacks, so that they can be kept-alive by the indexing task. 
This implies some changes to the interface of IndexingCallbacks.
As for AsyncTaskRunner to use, since this indexing task only depends on the 
file-index, which is owned by ClangdServer, I don't think there's any need to 
introduce a new taskrunner into TUScheduler and block its destruction. We can 
just re-use the existing TaskRunner inside parsingcallbacks, in which we run 
stdlib indexing tasks.

Rollout strategy


As mentioned above I think landing the patch after some local testing with an 
ASAN, behind a ClangdServer option is fine. Afterwards we can test out the 
behaviour by gradually turning on this option in our production and once it 
looks safe we can turn it on by default (and just get rid of the option).
Does that strategy works for you folks too?

---

So all in all, I think the direction makes sense, we're trading off some 
"freshness" in certain places, but this tradeoff only effects the behaviour 
until first preamble and in return we get some considerable savings (for a 
large codebase preamble indexing takes ~10 secs @95th%, which is ~13% of 
preamble build latency).
LMK if you've any further concerns/questions and thanks for doing this!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE L

[PATCH] D146054: [RISCV] Add --print-supported-extensions and -march=help support

2023-05-03 Thread Craig Topper via Phabricator via cfe-commits
craig.topper added a comment.

In D146054#4314766 , @kito-cheng 
wrote:

> LGTM, consider about the GNU compatibility, I would that has -march=help form 
> for that.

Are you saying gcc supports march=help? Is that recent?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

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


[PATCH] D146757: [Driver] Enable defining multilib print options independently of flags

2023-05-03 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings updated this revision to Diff 518997.
michaelplatings added a comment.

Chagne code to allow defining multilib flags verbatim


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146757

Files:
  clang/include/clang/Driver/Multilib.h
  clang/lib/Driver/Multilib.cpp
  clang/unittests/Driver/MultilibBuilderTest.cpp
  clang/unittests/Driver/MultilibTest.cpp

Index: clang/unittests/Driver/MultilibTest.cpp
===
--- clang/unittests/Driver/MultilibTest.cpp
+++ clang/unittests/Driver/MultilibTest.cpp
@@ -187,3 +187,18 @@
   EXPECT_EQ("/a", Selection[0].gccSuffix());
   EXPECT_EQ("/b", Selection[1].gccSuffix());
 }
+
+TEST(MultilibBuilder, PrintOptions) {
+  ASSERT_EQ(Multilib::flags_list(), Multilib().getPrintOptions());
+  ASSERT_EQ(Multilib::flags_list({"-x"}),
+Multilib({}, {}, {}, {"+x"}).getPrintOptions());
+  ASSERT_EQ(Multilib::flags_list({"-x"}),
+Multilib({}, {}, {}, {"+x"}, Multilib::PrintOptionsType::PlusFlags)
+.getPrintOptions());
+  ASSERT_EQ(
+  Multilib::flags_list({"-x", "-a", "-c"}),
+  Multilib({}, {}, {}, {"+x", "-y", "+a", "-b", "+c"}).getPrintOptions());
+  ASSERT_EQ(Multilib::flags_list({"-y"}),
+Multilib({}, {}, {}, {"-y"}, Multilib::PrintOptionsType::Verbatim)
+.getPrintOptions());
+}
Index: clang/unittests/Driver/MultilibBuilderTest.cpp
===
--- clang/unittests/Driver/MultilibBuilderTest.cpp
+++ clang/unittests/Driver/MultilibBuilderTest.cpp
@@ -207,3 +207,14 @@
 << "Selection picked " << Selection << " which was not expected ";
   }
 }
+
+TEST(MultilibBuilderTest, PrintOptions) {
+  Multilib M = MultilibBuilder()
+   .flag("+x")
+   .flag("-y")
+   .flag("+a")
+   .flag("-b")
+   .flag("+c")
+   .makeMultilib();
+  ASSERT_EQ(Multilib::flags_list({"-x", "-a", "-c"}), M.getPrintOptions());
+}
Index: clang/lib/Driver/Multilib.cpp
===
--- clang/lib/Driver/Multilib.cpp
+++ clang/lib/Driver/Multilib.cpp
@@ -17,18 +17,17 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/raw_ostream.h"
-#include 
 #include 
-#include 
 
 using namespace clang;
 using namespace driver;
 using namespace llvm::sys;
 
 Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix,
-   StringRef IncludeSuffix, const flags_list &Flags)
+   StringRef IncludeSuffix, const flags_list &Flags,
+   PrintOptionsType PrintOptions)
 : GCCSuffix(GCCSuffix), OSSuffix(OSSuffix), IncludeSuffix(IncludeSuffix),
-  Flags(Flags) {
+  Flags(Flags), PrintOptions(PrintOptions) {
   assert(GCCSuffix.empty() ||
  (StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1));
   assert(OSSuffix.empty() ||
@@ -37,6 +36,28 @@
  (StringRef(IncludeSuffix).front() == '/' && IncludeSuffix.size() > 1));
 }
 
+static Multilib::flags_list
+getPrintOptionsFromPlusFlags(const Multilib::flags_list &Flags) {
+  // Derive print options from flags beginning '+', replacing the first
+  // character with '-'.
+  Multilib::flags_list PrintOptions;
+  for (StringRef Flag : Flags) {
+if (Flag.front() == '+')
+  PrintOptions.push_back(("-" + Flag.substr(1)).str());
+  }
+  return PrintOptions;
+}
+
+Multilib::flags_list Multilib::getPrintOptions() const {
+  switch (PrintOptions) {
+  case PrintOptionsType::Verbatim:
+return Flags;
+  case PrintOptionsType::PlusFlags:
+return getPrintOptionsFromPlusFlags(Flags);
+  }
+  llvm_unreachable("Unknown Multilib::PrintOptionsType");
+}
+
 LLVM_DUMP_METHOD void Multilib::dump() const {
   print(llvm::errs());
 }
@@ -44,14 +65,11 @@
 void Multilib::print(raw_ostream &OS) const {
   if (GCCSuffix.empty())
 OS << ".";
-  else {
+  else
 OS << StringRef(GCCSuffix).drop_front();
-  }
   OS << ";";
-  for (StringRef Flag : Flags) {
-if (Flag.front() == '+')
-  OS << "@" << Flag.substr(1);
-  }
+  for (StringRef Option : getPrintOptions())
+OS << "@" << Option.substr(1);
 }
 
 bool Multilib::operator==(const Multilib &Other) const {
Index: clang/include/clang/Driver/Multilib.h
===
--- clang/include/clang/Driver/Multilib.h
+++ clang/include/clang/Driver/Multilib.h
@@ -31,19 +31,32 @@
 public:
   using flags_list = std::vector;
 
+  /// When -print-multi-lib is used, this defines how the printed options must
+  /// be calculated.
+  enum class PrintOptionsType {
+/// Print Flags verbatim
+Verbatim,
+/// Print flags in the Flags list that begin with '+', but with the initial
+/// '+' replaced with '-'. e.g. "+fno-rtti" becomes "-fno-rtti"
+

[PATCH] D142932: Multilib YAML parsing

2023-05-03 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings updated this revision to Diff 518999.
michaelplatings added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142932

Files:
  clang/include/clang/Driver/Multilib.h
  clang/lib/Driver/Multilib.cpp
  clang/unittests/Driver/MultilibTest.cpp

Index: clang/unittests/Driver/MultilibTest.cpp
===
--- clang/unittests/Driver/MultilibTest.cpp
+++ clang/unittests/Driver/MultilibTest.cpp
@@ -13,6 +13,7 @@
 #include "clang/Driver/Multilib.h"
 #include "../../lib/Driver/ToolChains/CommonArgs.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/Version.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
@@ -202,3 +203,353 @@
 Multilib({}, {}, {}, {"-y"}, Multilib::PrintOptionsType::Verbatim)
 .getPrintOptions());
 }
+
+static void diagnosticCallback(const llvm::SMDiagnostic &D, void *Out) {
+  *reinterpret_cast(Out) = D.getMessage();
+}
+
+static bool parseYaml(MultilibSet &MS, std::string &Diagnostic,
+  const char *Data) {
+  return MS.parseYaml(llvm::MemoryBufferRef(Data, "TEST"), diagnosticCallback,
+  &Diagnostic);
+}
+
+static bool parseYaml(MultilibSet &MS, const char *Data) {
+  return MS.parseYaml(llvm::MemoryBufferRef(Data, "TEST"));
+}
+
+// When updating this version also update MultilibVersionCurrent in Multilib.cpp
+#define YAML_PREAMBLE "MultilibVersion: 1.0\n"
+
+TEST(MultilibTest, ParseInvalid) {
+  std::string Diagnostic;
+
+  MultilibSet MS;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, R"(
+Variants: []
+)"));
+  EXPECT_TRUE(
+  StringRef(Diagnostic).contains("missing required key 'MultilibVersion'"))
+  << Diagnostic;
+
+  // Reject files with a different major version
+  EXPECT_FALSE(parseYaml(MS, Diagnostic,
+ R"(
+MultilibVersion: 2.0
+Variants: []
+)"));
+  EXPECT_TRUE(
+  StringRef(Diagnostic).contains("Multilib version 2.0 is unsupported"))
+  << Diagnostic;
+  EXPECT_FALSE(parseYaml(MS, Diagnostic,
+ R"(
+MultilibVersion: 0.1
+Variants: []
+)"));
+  EXPECT_TRUE(
+  StringRef(Diagnostic).contains("Multilib version 0.1 is unsupported"))
+  << Diagnostic;
+
+  // Reject files with a later minor version
+  EXPECT_FALSE(parseYaml(MS, Diagnostic,
+ R"(
+MultilibVersion: 1.9
+Variants: []
+)"));
+  EXPECT_TRUE(
+  StringRef(Diagnostic).contains("Multilib version 1.9 is unsupported"))
+  << Diagnostic;
+
+  // Accept files with the same major version and the same or earlier minor
+  // version
+  EXPECT_TRUE(parseYaml(MS, Diagnostic, R"(
+MultilibVersion: 1.0
+Variants: []
+)")) << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("missing required key 'Variants'"))
+  << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE R"(
+Variants:
+- Dir: /abc
+  Flags: []
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("paths must be relative"))
+  << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE R"(
+Variants:
+- Flags: []
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("missing required key 'Dir'"))
+  << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE R"(
+Variants:
+- Dir: .
+  Flags: []
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("missing required key 'Flags'"))
+  << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE R"(
+Variants:
+- Dir: .
+  Flags: []
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("missing required key 'Flags'"))
+  << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE R"(
+Variants: []
+FlagMap:
+- Regex: abc
+)"));
+  EXPECT_TRUE(
+  StringRef(Diagnostic)
+  .contains("value required for 'MatchFlags' or 'NoMatchFlags'"))
+  << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE R"(
+Variants: []
+FlagMap:
+- Dir: .
+  Regex: '('
+  Flags: []
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("parentheses not balanced"))
+  << Diagnostic;
+}
+
+TEST(MultilibTest, Parse) {
+  MultilibSet MS;
+  EXPECT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
+Variants:
+- Dir: .
+  Flags: []
+)"));
+  EXPECT_EQ(1U, MS.size());
+  EXPECT_EQ("", MS.begin()->gccSuffix());
+
+  EXPECT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
+Variants:
+- Dir: abc
+  Flags: []
+)"));
+  EXPECT_EQ(1U, MS.size());
+  EXPECT_EQ("/abc", MS.begin()->gccSuffix());
+
+  EXPECT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
+Variants:
+- Dir: pqr
+  Flags: [-mfloat-abi=soft]
+)"));
+  EXPECT_EQ(1U, MS.size());
+  EXPECT_EQ("/pqr", MS.begin()->gccSuffix());
+  EXPECT_EQ(std::vector({"-mfloat-abi=soft"}),
+MS.begin()->flags());
+
+  EXPECT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
+Variants

[PATCH] D142933: Add -print-multi-selection-flags-experimental option

2023-05-03 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings updated this revision to Diff 519000.
michaelplatings added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142933

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.h
  clang/test/Driver/print-multi-selection-flags.c

Index: clang/test/Driver/print-multi-selection-flags.c
===
--- /dev/null
+++ clang/test/Driver/print-multi-selection-flags.c
@@ -0,0 +1,51 @@
+// RUN: %clang -print-multi-selection-flags-experimental --target=aarch64-linux -fc++-abi=itanium -fsanitize=address | FileCheck --check-prefix=CHECK-LINUX %s
+// CHECK-LINUX: --target=aarch64-unknown-linux
+// CHECK-LINUX: -fc++-abi=itanium
+// CHECK-LINUX: -fexceptions
+// CHECK-LINUX: -frtti
+// CHECK-LINUX: -fsanitize=address
+
+// RUN: %clang -print-multi-selection-flags-experimental --target=aarch64-fuchsia -fsanitize=hwaddress | FileCheck --check-prefix=CHECK-FUCHSIA %s
+// CHECK-FUCHSIA: --target=aarch64-unknown-fuchsia
+// CHECK-FUCHSIA: -fsanitize=hwaddress
+
+// RUN: %clang -print-multi-selection-flags-experimental --target=arm-none-eabi -mfloat-abi=soft -fno-exceptions -fno-rtti | FileCheck --check-prefix=CHECK-ARMV4T %s
+// CHECK-ARMV4T: --target=armv4t-none-unknown-eabi
+// CHECK-ARMV4T: -fno-exceptions
+// CHECK-ARMV4T: -fno-rtti
+// CHECK-ARMV4T: -mfloat-abi=soft
+// CHECK-ARMV4T: -mfpu=none
+
+// RUN: %clang -print-multi-selection-flags-experimental --target=armv7em-none-eabi -mfloat-abi=softfp | FileCheck --check-prefix=CHECK-SOFTFP %s
+// CHECK-SOFTFP: --target=thumbv7em-none-unknown-eabi
+// CHECK-SOFTFP: -mfloat-abi=softfp
+// CHECK-SOFTFP: -mfpu=fpv4-sp-d16
+
+// RUN: %clang -print-multi-selection-flags-experimental --target=arm-none-eabihf -march=armv7em -mfpu=fpv5-d16 | FileCheck --check-prefix=CHECK-HARD %s
+// CHECK-HARD: --target=thumbv7em-none-unknown-eabihf
+// CHECK-HARD: -mfloat-abi=hard
+// CHECK-HARD: -mfpu=fpv5-d16
+
+// RUN: %clang -print-multi-selection-flags-experimental --target=arm-none-eabi -mfloat-abi=soft -march=armv8-m.main+nofp | FileCheck --check-prefix=CHECK-V8MMAIN-NOFP %s
+// CHECK-V8MMAIN-NOFP: --target=thumbv8m.main-none-unknown-eabi
+// CHECK-V8MMAIN-NOFP: -mfloat-abi=soft
+// CHECK-V8MMAIN-NOFP: -mfpu=none
+
+// RUN: %clang -print-multi-selection-flags-experimental --target=arm-none-eabi -mfloat-abi=hard -march=armv8.1m.main+mve.fp | FileCheck --check-prefix=CHECK-MVE %s
+// CHECK-MVE: --target=thumbv8.1m.main-none-unknown-eabihf
+// CHECK-MVE: -march=thumbv8.1m.main{{.*}}+mve{{.*}}+mve.fp{{.*}}
+// CHECK-MVE: -mfloat-abi=hard
+// CHECK-MVE: -mfpu=fp-armv8-fullfp16-sp-d16
+
+// RUN: %clang -print-multi-selection-flags-experimental --target=arm-none-eabi -march=armv8.1m.main+mve+nofp | FileCheck --check-prefix=CHECK-MVENOFP %s
+// CHECK-MVENOFP: -march=thumbv8.1m.main{{.*}}+mve{{.*}}
+// CHECK-MVENOFP-NOT: -march=thumbv8.1m.main{{.*}}+mve.fp{{.*}}
+// CHECK-MVENOFP: -mfpu=none
+
+// RUN: %clang -print-multi-selection-flags-experimental --target=aarch64-none-elf -march=armv8-a+lse | FileCheck --check-prefix=CHECK-LSE %s
+// CHECK-LSE: -march=aarch64{{.*}}+lse{{.*}}
+
+// RUN: %clang -print-multi-selection-flags-experimental --target=aarch64-none-elf -march=armv8.5-a+sve+sve2 | FileCheck --check-prefix=CHECK-SVE2 %s
+// RUN: %clang -print-multi-selection-flags-experimental --target=aarch64-none-elf -march=armv9-a| FileCheck --check-prefix=CHECK-SVE2 %s
+// CHECK-SVE2: --target=aarch64-none-unknown-elf
+// CHECK-SVE2: -march=aarch64{{.*}}+simd{{.*}}+sve{{.*}}+sve2{{.*}}
Index: clang/lib/Driver/ToolChains/Arch/ARM.h
===
--- clang/lib/Driver/ToolChains/Arch/ARM.h
+++ clang/lib/Driver/ToolChains/Arch/ARM.h
@@ -63,9 +63,11 @@
 void getARMArchCPUFromArgs(const llvm::opt::ArgList &Args,
llvm::StringRef &Arch, llvm::StringRef &CPU,
bool FromAs = false);
-void getARMTargetFeatures(const Driver &D, const llvm::Triple &Triple,
-  const llvm::opt::ArgList &Args,
-  std::vector &Features, bool ForAS);
+llvm::ARM::FPUKind getARMTargetFeatures(const Driver &D,
+const llvm::Triple &Triple,
+const llvm::opt::ArgList &Args,
+std::vector &Features,
+bool ForAS);
 int getARMSubArchVersionNumber(const llvm::Triple &Triple);
 bool isARMMProfile(const llvm::Triple &Triple);
 bool isARMAProfile(const llvm::Triple &Triple);
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===

[PATCH] D142986: Enable multilib.yaml in the BareMetal ToolChain

2023-05-03 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings updated this revision to Diff 519001.
michaelplatings added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142986

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/ARM.h
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/test/Driver/baremetal-multilib.yaml
  clang/test/Driver/baremetal.cpp
  clang/test/Driver/lit.local.cfg

Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -1,7 +1,7 @@
 from lit.llvm import llvm_config
 
 config.suffixes = ['.c', '.cpp', '.cppm', '.h', '.m', '.mm', '.S', '.s', '.f90', '.F90', '.f95',
-   '.cu', '.rs', '.cl', '.clcpp', '.hip', '.hipi', '.hlsl']
+   '.cu', '.rs', '.cl', '.clcpp', '.hip', '.hipi', '.hlsl', '.yaml']
 config.substitutions = list(config.substitutions)
 config.substitutions.insert(0,
 ('%clang_cc1',
Index: clang/test/Driver/baremetal.cpp
===
--- clang/test/Driver/baremetal.cpp
+++ clang/test/Driver/baremetal.cpp
@@ -118,9 +118,9 @@
 // Verify that the bare metal driver does not include any host system paths:
 // CHECK-AARCH64-NO-HOST-INC: InstalledDir: [[INSTALLEDDIR:.+]]
 // CHECK-AARCH64-NO-HOST-INC: "-resource-dir" "[[RESOURCE:[^"]+]]"
-// CHECK-AARCH64-NO-HOST-INC-SAME: "-internal-isystem" "[[INSTALLEDDIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+}}aarch64-none-elf{{[/\\]+}}include{{[/\\]+}}c++{{[/\\]+}}v1"
+// CHECK-AARCH64-NO-HOST-INC-SAME: "-internal-isystem" "[[INSTALLEDDIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+[^"]*}}include{{[/\\]+}}c++{{[/\\]+}}v1"
 // CHECK-AARCH64-NO-HOST-INC-SAME: "-internal-isystem" "[[RESOURCE]]{{[/\\]+}}include"
-// CHECK-AARCH64-NO-HOST-INC-SAME: "-internal-isystem" "[[INSTALLEDDIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+}}aarch64-none-elf{{[/\\]+}}include"
+// CHECK-AARCH64-NO-HOST-INC-SAME: "-internal-isystem" "[[INSTALLEDDIR]]{{[/\\]+}}..{{[/\\]+}}lib{{[/\\]+}}clang-runtimes{{[/\\]+[^"]*}}include"
 
 // RUN: %clang %s -### --target=riscv64-unknown-elf -o %t.out -L some/directory/user/asked/for \
 // RUN: --sysroot=%S/Inputs/basic_riscv64_tree/riscv64-unknown-elf 2>&1 \
Index: clang/test/Driver/baremetal-multilib.yaml
===
--- /dev/null
+++ clang/test/Driver/baremetal-multilib.yaml
@@ -0,0 +1,148 @@
+# REQUIRES: shell
+# UNSUPPORTED: system-windows
+
+# RUN: rm -rf %T/baremetal_multilib
+# RUN: mkdir -p %T/baremetal_multilib/bin
+# RUN: mkdir -p %T/baremetal_multilib/lib/clang-runtimes/arm-none-eabi/thumb/v8-m.main/fp/lib
+# RUN: touch %T/baremetal_multilib/lib/clang-runtimes/arm-none-eabi/thumb/v8-m.main/fp/lib/libclang_rt.builtins.a
+# RUN: ln -s %clang %T/baremetal_multilib/bin/clang
+# RUN: ln -s %s %T/baremetal_multilib/lib/clang-runtimes/multilib.yaml
+
+# RUN: %T/baremetal_multilib/bin/clang -no-canonical-prefixes -x c++ %s -### -o %t.out 2>&1 \
+# RUN: --target=thumbv8m.main-none-eabihf --sysroot= \
+# RUN:   | FileCheck -DSYSROOT=%T/baremetal_multilib %s
+# CHECK:  "-cc1" "-triple" "thumbv8m.main-none-unknown-eabihf"
+# CHECK-SAME: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8-m.main/fp/include/c++/v1"
+# CHECK-SAME: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8-m.main/fp/include"
+# CHECK-SAME: "-x" "c++" "{{.*}}baremetal-multilib.yaml"
+# CHECK-NEXT: ld{{(.exe)?}}" "{{.*}}.o" "-Bstatic"
+# CHECK-SAME: "-L[[SYSROOT]]/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8-m.main/fp/lib"
+# CHECK-SAME: "-lc" "-lm" "-lclang_rt.builtins"
+# CHECK-SAME: "-o" "{{.*}}.tmp.out"
+
+# RUN: %T/baremetal_multilib/bin/clang -no-canonical-prefixes -print-multi-directory 2>&1 \
+# RUN: --target=thumbv8m.main-none-eabihf --sysroot= \
+# RUN:   | FileCheck --check-prefix=CHECK-PRINT-MULTI-DIRECTORY %s
+# CHECK-PRINT-MULTI-DIRECTORY: arm-none-eabi/thumb/v8-m.main/fp
+
+# RUN: %T/baremetal_multilib/bin/clang -no-canonical-prefixes -print-multi-lib 2>&1 \
+# RUN: --target=arm-none-eabi --sysroot= \
+# RUN:   | FileCheck --check-prefix=CHECK-PRINT-MULTI-LIB %s
+# CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v6-m/nofp;@-target=thumbv6m-none-unknown-eabi@mfpu=none
+# CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v7-m/nofp;@-target=thumbv7m-none-unknown-eabi@mfpu=none
+# CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v7e-m/nofp;@-target=thumbv7em-none-unknown-eabi@mfpu=none
+# CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v8-m.main/nofp;@-target=thumbv8m.main-none-unknown-eabi@mfpu=none
+# CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v8.1-m.main/nofp/nomve;@-target=thumbv8.1m.main-none-unknown-eabi@mfpu=none
+# CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v7e-m/fpv4_sp_d16;

[PATCH] D143059: [Driver] Enable selecting multiple multilibs

2023-05-03 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings updated this revision to Diff 519002.
michaelplatings added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143059

Files:
  clang/include/clang/Driver/Multilib.h
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Multilib.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/CSKYToolChain.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h
  clang/lib/Driver/ToolChains/Hexagon.cpp
  clang/lib/Driver/ToolChains/Hurd.cpp
  clang/lib/Driver/ToolChains/Linux.cpp
  clang/lib/Driver/ToolChains/MipsLinux.cpp
  clang/lib/Driver/ToolChains/OHOS.cpp
  clang/lib/Driver/ToolChains/RISCVToolchain.cpp
  clang/test/Driver/fuchsia.cpp
  clang/unittests/Driver/MultilibBuilderTest.cpp
  clang/unittests/Driver/MultilibTest.cpp

Index: clang/unittests/Driver/MultilibTest.cpp
===
--- clang/unittests/Driver/MultilibTest.cpp
+++ clang/unittests/Driver/MultilibTest.cpp
@@ -154,18 +154,18 @@
   Multilib("/bar", {}, {}, {"+bar"}),
   });
   Multilib::flags_list Flags1 = {"+foo", "-bar"};
-  Multilib Selection1;
+  llvm::SmallVector Selection1;
   ASSERT_TRUE(MS.select(Flags1, Selection1))
   << "Flag set was {\"+foo\"}, but selection not found";
-  ASSERT_TRUE(Selection1.gccSuffix() == "/foo")
-  << "Selection picked " << Selection1 << " which was not expected";
+  ASSERT_TRUE(Selection1.back().gccSuffix() == "/foo")
+  << "Selection picked " << Selection1.back() << " which was not expected";
 
   Multilib::flags_list Flags2 = {"+foo", "+bar"};
-  Multilib Selection2;
+  llvm::SmallVector Selection2;
   ASSERT_TRUE(MS.select(Flags2, Selection2))
   << "Flag set was {\"+bar\"}, but selection not found";
-  ASSERT_TRUE(Selection2.gccSuffix() == "/bar")
-  << "Selection picked " << Selection2 << " which was not expected";
+  ASSERT_TRUE(Selection2.back().gccSuffix() == "/bar")
+  << "Selection picked " << Selection2.back() << " which was not expected";
 }
 
 TEST(MultilibTest, SelectMultiple) {
@@ -173,17 +173,17 @@
   Multilib("/a", {}, {}, {"x"}),
   Multilib("/b", {}, {}, {"y"}),
   });
-  std::vector Selection;
+  llvm::SmallVector Selection;
 
-  Selection = MS.select({"x"});
+  ASSERT_TRUE(MS.select({"x"}, Selection));
   ASSERT_EQ(1u, Selection.size());
   EXPECT_EQ("/a", Selection[0].gccSuffix());
 
-  Selection = MS.select({"y"});
+  ASSERT_TRUE(MS.select({"y"}, Selection));
   ASSERT_EQ(1u, Selection.size());
   EXPECT_EQ("/b", Selection[0].gccSuffix());
 
-  Selection = MS.select({"y", "x"});
+  ASSERT_TRUE(MS.select({"y", "x"}, Selection));
   ASSERT_EQ(2u, Selection.size());
   EXPECT_EQ("/a", Selection[0].gccSuffix());
   EXPECT_EQ("/b", Selection[1].gccSuffix());
@@ -373,7 +373,7 @@
 
 TEST(MultilibTest, SelectSoft) {
   MultilibSet MS;
-  Multilib Selected;
+  llvm::SmallVector Selected;
   ASSERT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
 Variants:
 - Dir: s
@@ -389,7 +389,7 @@
 
 TEST(MultilibTest, SelectSoftFP) {
   MultilibSet MS;
-  Multilib Selected;
+  llvm::SmallVector Selected;
   ASSERT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
 Variants:
 - Dir: f
@@ -404,7 +404,7 @@
   // If hard float is all that's available then select that only if compiling
   // with hard float.
   MultilibSet MS;
-  Multilib Selected;
+  llvm::SmallVector Selected;
   ASSERT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
 Variants:
 - Dir: h
@@ -417,7 +417,7 @@
 
 TEST(MultilibTest, SelectFloatABI) {
   MultilibSet MS;
-  Multilib Selected;
+  llvm::SmallVector Selected;
   ASSERT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
 Variants:
 - Dir: s
@@ -431,18 +431,18 @@
   MatchFlags: [-mfloat-abi=soft]
 )"));
   MS.select({"-mfloat-abi=soft"}, Selected);
-  EXPECT_EQ("/s", Selected.gccSuffix());
+  EXPECT_EQ("/s", Selected.back().gccSuffix());
   MS.select({"-mfloat-abi=softfp"}, Selected);
-  EXPECT_EQ("/f", Selected.gccSuffix());
+  EXPECT_EQ("/f", Selected.back().gccSuffix());
   MS.select({"-mfloat-abi=hard"}, Selected);
-  EXPECT_EQ("/h", Selected.gccSuffix());
+  EXPECT_EQ("/h", Selected.back().gccSuffix());
 }
 
 TEST(MultilibTest, SelectFloatABIReversed) {
   // If soft is specified after softfp then softfp will never be
   // selected because soft is compatible with softfp and last wins.
   MultilibSet MS;
-  Multilib Selected;
+  llvm::SmallVector Selected;
   ASSERT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
 Variants:
 - Dir: h
@@ -456,11 +456,11 @@
   MatchFlags: [-mfloat-abi=soft]
 )"));
   MS.select({"-mfloat-abi=soft"}, Selected);
-  EXPECT_EQ("/s", Selected.gccSuffix());
+  EXPECT_EQ("/s", Selected.back().gccSuffix());
   MS.select({"-mfloat-abi=softfp"}, Selected);
-  EXPECT_EQ("/s", Selected.gccSuffix());
+  EXPECT_EQ("/s", Selected.back().gccSuffix());
   MS.select({"-mflo

[PATCH] D143075: BareMetal ToolChain multilib layering

2023-05-03 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings updated this revision to Diff 519003.
michaelplatings added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143075

Files:
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/BareMetal.h
  clang/test/Driver/baremetal-multilib.yaml

Index: clang/test/Driver/baremetal-multilib.yaml
===
--- clang/test/Driver/baremetal-multilib.yaml
+++ clang/test/Driver/baremetal-multilib.yaml
@@ -25,6 +25,23 @@
 # RUN:   | FileCheck --check-prefix=CHECK-PRINT-MULTI-DIRECTORY %s
 # CHECK-PRINT-MULTI-DIRECTORY: arm-none-eabi/thumb/v8-m.main/fp
 
+# RUN: %T/baremetal_multilib/bin/clang -no-canonical-prefixes -x c++ %s -### -o %t.out 2>&1 \
+# RUN: --target=thumbv8.1m.main-none-eabihf -fno-exceptions --sysroot= \
+# RUN:   | FileCheck -DSYSROOT=%T/baremetal_multilib --check-prefix=CHECK-LAYERED-MULTILIB %s
+# CHECK-LAYERED-MULTILIB:  "-cc1" "-triple" "thumbv8.1m.main-none-unknown-eabihf"
+# CHECK-LAYERED-MULTILIB-SAME: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8.1-m.main/fp/noexcept/include/c++/v1"
+# CHECK-LAYERED-MULTILIB-SAME: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8.1-m.main/fp/include/c++/v1"
+# CHECK-LAYERED-MULTILIB-SAME: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8.1-m.main/fp/noexcept/include"
+# CHECK-LAYERED-MULTILIB-SAME: "-internal-isystem" "[[SYSROOT]]/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8.1-m.main/fp/include"
+# CHECK-LAYERED-MULTILIB-NEXT: "-L[[SYSROOT]]/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8.1-m.main/fp/noexcept/lib"
+# CHECK-LAYERED-MULTILIB-SAME: "-L[[SYSROOT]]/bin/../lib/clang-runtimes/arm-none-eabi/thumb/v8.1-m.main/fp/lib"
+
+# RUN: %T/baremetal_multilib/bin/clang -no-canonical-prefixes -print-multi-directory 2>&1 \
+# RUN: --target=thumbv8.1m.main-none-eabihf -fno-exceptions --sysroot= \
+# RUN:   | FileCheck --check-prefix=CHECK-LAYERED-PRINT-MULTI-DIRECTORY %s
+# CHECK-LAYERED-PRINT-MULTI-DIRECTORY:  arm-none-eabi/thumb/v8.1-m.main/fp
+# CHECK-LAYERED-PRINT-MULTI-DIRECTORY-NEXT: arm-none-eabi/thumb/v8.1-m.main/fp/noexcept
+
 # RUN: %T/baremetal_multilib/bin/clang -no-canonical-prefixes -print-multi-lib 2>&1 \
 # RUN: --target=arm-none-eabi --sysroot= \
 # RUN:   | FileCheck --check-prefix=CHECK-PRINT-MULTI-LIB %s
@@ -38,6 +55,7 @@
 # CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v8-m.main/fp;@-target=thumbv8m.main-none-unknown-eabihf@mfpu=fpv5-d16
 # CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v8.1-m.main/fp;@-target=thumbv8.1m.main-none-unknown-eabihf@mfpu=fp-armv8-fullfp16-sp-d16
 # CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v8.1-m.main/nofp/mve;@-target=thumbv8.1m.main-none-unknown-eabihf@march=thumbv8.1m.main+mve@mfpu=none
+# CHECK-PRINT-MULTI-LIB: arm-none-eabi/thumb/v8.1-m.main/fp/noexcept;@-target=thumbv8.1m.main-none-unknown-eabihf@mfpu=fp-armv8-fullfp16-sp-d16@fno-exceptions
 
 # RUN: %T/baremetal_multilib/bin/clang -no-canonical-prefixes -x assembler -mexecute-only \
 # RUN: --target=arm-none-eabi --sysroot= %s -c -### 2>&1 \
@@ -107,6 +125,13 @@
 - Dir: arm-none-eabi/thumb/v8.1-m.main/nofp/mve
   Flags: [--target=thumbv8.1m.main-none-unknown-eabihf, -march=thumbv8.1m.main+mve, -mfpu=none]
 
+# A specialisation of v8.1-m.main/fp without exceptions.
+# This layers over the top of the regular v8.1-m.main/fp so it doesn't
+# need to have its own include directory or C library, thereby saving
+# disk space.
+- Dir: arm-none-eabi/thumb/v8.1-m.main/fp/noexcept
+  Flags: [--target=thumbv8.1m.main-none-unknown-eabihf, -mfpu=fp-armv8-fullfp16-sp-d16, -fno-exceptions]
+
 
 # The second section of the file is a map from auto-detected flags
 # to custom flags. The auto-detected flags can be printed out
Index: clang/lib/Driver/ToolChains/BareMetal.h
===
--- clang/lib/Driver/ToolChains/BareMetal.h
+++ clang/lib/Driver/ToolChains/BareMetal.h
@@ -71,6 +71,9 @@
   void AddLinkRuntimeLib(const llvm::opt::ArgList &Args,
  llvm::opt::ArgStringList &CmdArgs) const;
   std::string computeSysRoot() const override;
+
+private:
+  llvm::SmallVector getOrderedMultilibs() const;
 };
 
 } // namespace toolchains
Index: clang/lib/Driver/ToolChains/BareMetal.cpp
===
--- clang/lib/Driver/ToolChains/BareMetal.cpp
+++ clang/lib/Driver/ToolChains/BareMetal.cpp
@@ -103,9 +103,12 @@
   findMultilibs(D, Triple, Args);
   SmallString<128> SysRoot(computeSysRoot());
   if (!SysRoot.empty()) {
-llvm::sys::path::append(SysRoot, "lib");
-getFilePaths().push_back(std::string(SysRoot));
-getLibraryPaths().push_back(std::string(SysRoot));
+for (const Multilib &M : getOrderedMultilibs()) {
+  SmallString<128> Dir(

[PATCH] D143587: [Docs] Multilib design

2023-05-03 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings updated this revision to Diff 519004.
michaelplatings added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143587

Files:
  clang/docs/Multilib.rst
  clang/docs/index.rst

Index: clang/docs/index.rst
===
--- clang/docs/index.rst
+++ clang/docs/index.rst
@@ -100,6 +100,7 @@
CodeOwners
InternalsManual
DriverInternals
+   Multilib
OffloadingDesign
PCHInternals
ItaniumMangleAbiTags
Index: clang/docs/Multilib.rst
===
--- /dev/null
+++ clang/docs/Multilib.rst
@@ -0,0 +1,317 @@
+
+Multilib
+
+
+Introduction
+
+
+This document describes how multilib is implemented in Clang.
+
+What is multilib and why might you care?
+If you're :doc:`cross compiling` then you can't use native
+system headers and libraries. To address this, you can use a combination of
+``--sysroot``, ``-isystem`` and ``-L`` options to point Clang at suitable
+directories for your target.
+However, when there are many possible directories to choose from, it's not
+necessarily obvious which one to pick.
+Multilib allows a toolchain designer to imbue the toolchain with the ability to
+pick a suitable directory automatically, based on the options the user provides
+to Clang. For example, if the user specifies
+``--target=arm-none-eabi -mcpu=cortex-m4`` the toolchain can choose a directory
+containing headers and libraries suitable for Armv7E-M, because it knows that's
+a suitable architecture for Arm Cortex-M4.
+Multilib can also choose between libraries for the same architecture based on
+other options. For example if the user specifies ``-fno-exceptions`` then a
+toolchain could select libraries built without exception support, thereby
+reducing the size of the resulting binary.
+
+Design
+==
+
+Clang supports GCC's ``-print-multi-lib`` and ``-print-multi-directory``
+options. These are described in
+`GCC Developer Options `_.
+
+There are two ways to configure multilib in Clang: hard-coded or via a
+configuration file.
+
+Hard-coded Multilib
+===
+
+The available libraries can be hard-coded in Clang. Typically this is done
+using the ``MultilibBuilder`` interface in
+``clang/include/clang/Driver/MultilibBuilder.h``.
+There are many examples of this in ``lib/Driver/ToolChains/Gnu.cpp``.
+The remainder of this document will not focus on this type of multilib.
+
+EXPERIMENTAL Multilib via configuration file
+
+
+Some Clang toolchains support loading multilib configuration from a
+``multilib.yaml`` configuration file.
+
+A ``multilib.yaml`` configuration file specifies which multilib variants are
+available, their relative location, what compilation options were used to build
+them, and the criteria by which they are selected.
+
+Multilib processing
+===
+
+Clang goes through the following steps to use multilib from a configuration
+file:
+#. Normalize command line options. Clang can accept the same
+   information via different options - for example,
+   ``--target=arm-none-eabi -march=armv7-m`` and
+   ``--target=armv7m-none-eabi`` are equivalent.
+   Clang normalizes the command line before passing them to the multilib system.
+   To see what flags are emitted for a given set of command line options, use
+   the ``-print-multi-selection-flags-experimental`` command line option
+   along with the rest of the options you want to use.
+#. Load ``multilib.yaml`` from sysroot.
+#. Generate additional flags. ``multilib.yaml`` contains a ``FlagMap`` section,
+   which specifies how to generate additional flags based on the flags derived
+   from command line options. Flags are matched using regular expressions.
+   These regular expressions shall use the POSIX extended regular expression
+   syntax.
+#. Match flags against multilib variants. If the generated flags are a superset
+   of the flags specified for a multilib variant then the variant is considered
+   a match.
+   If more than one variant matches then a toolchain may opt to either use only
+   the *last* matching multilib variant, or may use all matching variants,
+   thereby :ref:`layering` them.
+#. Generate ``-isystem`` and ``-L`` options. Iterate in reverse order over
+   the matching multilib variants, and generate ``-isystem`` and ``-L``
+   options based on the multilib variant's directory.
+
+Multilib layering
+=
+
+When Clang selects multilib variants, it may find that more than one variant
+matches.
+
+It is up to the ToolChain subclass to decide what to do in this case.
+There are two options permitted:
+#. Use only the *last* matching multilib variant. This option exists primarily
+   for compatibility with the previous mult

[PATCH] D146757: [Driver] Enable defining multilib flags verbatim

2023-05-03 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings updated this revision to Diff 519005.
michaelplatings added a comment.

Re-run arc diff with clang-format in PATH


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146757

Files:
  clang/include/clang/Driver/Multilib.h
  clang/lib/Driver/Multilib.cpp
  clang/unittests/Driver/MultilibBuilderTest.cpp
  clang/unittests/Driver/MultilibTest.cpp

Index: clang/unittests/Driver/MultilibTest.cpp
===
--- clang/unittests/Driver/MultilibTest.cpp
+++ clang/unittests/Driver/MultilibTest.cpp
@@ -187,3 +187,18 @@
   EXPECT_EQ("/a", Selection[0].gccSuffix());
   EXPECT_EQ("/b", Selection[1].gccSuffix());
 }
+
+TEST(MultilibBuilder, PrintOptions) {
+  ASSERT_EQ(Multilib::flags_list(), Multilib().getPrintOptions());
+  ASSERT_EQ(Multilib::flags_list({"-x"}),
+Multilib({}, {}, {}, {"+x"}).getPrintOptions());
+  ASSERT_EQ(Multilib::flags_list({"-x"}),
+Multilib({}, {}, {}, {"+x"}, Multilib::PrintOptionsType::PlusFlags)
+.getPrintOptions());
+  ASSERT_EQ(
+  Multilib::flags_list({"-x", "-a", "-c"}),
+  Multilib({}, {}, {}, {"+x", "-y", "+a", "-b", "+c"}).getPrintOptions());
+  ASSERT_EQ(Multilib::flags_list({"-y"}),
+Multilib({}, {}, {}, {"-y"}, Multilib::PrintOptionsType::Verbatim)
+.getPrintOptions());
+}
Index: clang/unittests/Driver/MultilibBuilderTest.cpp
===
--- clang/unittests/Driver/MultilibBuilderTest.cpp
+++ clang/unittests/Driver/MultilibBuilderTest.cpp
@@ -207,3 +207,14 @@
 << "Selection picked " << Selection << " which was not expected ";
   }
 }
+
+TEST(MultilibBuilderTest, PrintOptions) {
+  Multilib M = MultilibBuilder()
+   .flag("+x")
+   .flag("-y")
+   .flag("+a")
+   .flag("-b")
+   .flag("+c")
+   .makeMultilib();
+  ASSERT_EQ(Multilib::flags_list({"-x", "-a", "-c"}), M.getPrintOptions());
+}
Index: clang/lib/Driver/Multilib.cpp
===
--- clang/lib/Driver/Multilib.cpp
+++ clang/lib/Driver/Multilib.cpp
@@ -17,18 +17,17 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/raw_ostream.h"
-#include 
 #include 
-#include 
 
 using namespace clang;
 using namespace driver;
 using namespace llvm::sys;
 
 Multilib::Multilib(StringRef GCCSuffix, StringRef OSSuffix,
-   StringRef IncludeSuffix, const flags_list &Flags)
+   StringRef IncludeSuffix, const flags_list &Flags,
+   PrintOptionsType PrintOptions)
 : GCCSuffix(GCCSuffix), OSSuffix(OSSuffix), IncludeSuffix(IncludeSuffix),
-  Flags(Flags) {
+  Flags(Flags), PrintOptions(PrintOptions) {
   assert(GCCSuffix.empty() ||
  (StringRef(GCCSuffix).front() == '/' && GCCSuffix.size() > 1));
   assert(OSSuffix.empty() ||
@@ -37,6 +36,28 @@
  (StringRef(IncludeSuffix).front() == '/' && IncludeSuffix.size() > 1));
 }
 
+static Multilib::flags_list
+getPrintOptionsFromPlusFlags(const Multilib::flags_list &Flags) {
+  // Derive print options from flags beginning '+', replacing the first
+  // character with '-'.
+  Multilib::flags_list PrintOptions;
+  for (StringRef Flag : Flags) {
+if (Flag.front() == '+')
+  PrintOptions.push_back(("-" + Flag.substr(1)).str());
+  }
+  return PrintOptions;
+}
+
+Multilib::flags_list Multilib::getPrintOptions() const {
+  switch (PrintOptions) {
+  case PrintOptionsType::Verbatim:
+return Flags;
+  case PrintOptionsType::PlusFlags:
+return getPrintOptionsFromPlusFlags(Flags);
+  }
+  llvm_unreachable("Unknown Multilib::PrintOptionsType");
+}
+
 LLVM_DUMP_METHOD void Multilib::dump() const {
   print(llvm::errs());
 }
@@ -44,14 +65,11 @@
 void Multilib::print(raw_ostream &OS) const {
   if (GCCSuffix.empty())
 OS << ".";
-  else {
+  else
 OS << StringRef(GCCSuffix).drop_front();
-  }
   OS << ";";
-  for (StringRef Flag : Flags) {
-if (Flag.front() == '+')
-  OS << "@" << Flag.substr(1);
-  }
+  for (StringRef Option : getPrintOptions())
+OS << "@" << Option.substr(1);
 }
 
 bool Multilib::operator==(const Multilib &Other) const {
Index: clang/include/clang/Driver/Multilib.h
===
--- clang/include/clang/Driver/Multilib.h
+++ clang/include/clang/Driver/Multilib.h
@@ -31,19 +31,32 @@
 public:
   using flags_list = std::vector;
 
+  /// When -print-multi-lib is used, this defines how the printed options must
+  /// be calculated.
+  enum class PrintOptionsType {
+/// Print Flags verbatim
+Verbatim,
+/// Print flags in the Flags list that begin with '+', but with the initial
+/// '+' replaced with '-'. e.g. "+fno-rtti" becomes "-fno-rtti"
+PlusFlags,
+

[PATCH] D149643: [clang-format] Correctly limit formatted ranges when specifying qualifier alignment

2023-05-03 Thread Colin Ogilvie via Phabricator via cfe-commits
cogilvie updated this revision to Diff 519007.

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

https://reviews.llvm.org/D149643

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/unittests/Format/QualifierFixerTest.cpp


Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -1343,6 +1343,29 @@
"TemplateType t;", Style);
 }
 
+TEST_F(QualifierFixerTest, Ranges) {
+  FormatStyle Style = getLLVMStyle();
+  Style.QualifierAlignment = FormatStyle::QAS_Custom;
+  Style.QualifierOrder = {"const", "volatile", "type"};
+
+  // Only the first line should be formatted the second should remain as is
+  verifyFormat("template  const Foo f();\n"
+   "template  Foo const f();",
+   "template  Foo const f();\n"
+   "template  Foo const f();",
+   Style, std::vector(1, tooling::Range(0, 36)));
+
+  // Only the middle line should be formatted the first and last should remain
+  // as is
+  verifyFormat("template  Foo const f();\n"
+   "template  const Foo f();\n"
+   "template  Foo const f();",
+   "template  Foo const f();\n"
+   "template  Foo const f();\n"
+   "template  Foo const f();",
+   Style, std::vector(1, tooling::Range(37, 36)));
+}
+
 } // namespace
 } // namespace test
 } // namespace format
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -587,7 +587,7 @@
   assert(QualifierToken != tok::identifier && "Unrecognised Qualifier");
 
   for (AnnotatedLine *Line : AnnotatedLines) {
-if (Line->InPPDirective)
+if (!Line->Affected || Line->InPPDirective)
   continue;
 FormatToken *First = Line->First;
 assert(First);


Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -1343,6 +1343,29 @@
"TemplateType t;", Style);
 }
 
+TEST_F(QualifierFixerTest, Ranges) {
+  FormatStyle Style = getLLVMStyle();
+  Style.QualifierAlignment = FormatStyle::QAS_Custom;
+  Style.QualifierOrder = {"const", "volatile", "type"};
+
+  // Only the first line should be formatted the second should remain as is
+  verifyFormat("template  const Foo f();\n"
+   "template  Foo const f();",
+   "template  Foo const f();\n"
+   "template  Foo const f();",
+   Style, std::vector(1, tooling::Range(0, 36)));
+
+  // Only the middle line should be formatted the first and last should remain
+  // as is
+  verifyFormat("template  Foo const f();\n"
+   "template  const Foo f();\n"
+   "template  Foo const f();",
+   "template  Foo const f();\n"
+   "template  Foo const f();\n"
+   "template  Foo const f();",
+   Style, std::vector(1, tooling::Range(37, 36)));
+}
+
 } // namespace
 } // namespace test
 } // namespace format
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -587,7 +587,7 @@
   assert(QualifierToken != tok::identifier && "Unrecognised Qualifier");
 
   for (AnnotatedLine *Line : AnnotatedLines) {
-if (Line->InPPDirective)
+if (!Line->Affected || Line->InPPDirective)
   continue;
 FormatToken *First = Line->First;
 assert(First);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D149643: [clang-format] Correctly limit formatted ranges when specifying qualifier alignment

2023-05-03 Thread Colin Ogilvie via Phabricator via cfe-commits
cogilvie updated this revision to Diff 519008.

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

https://reviews.llvm.org/D149643

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/unittests/Format/QualifierFixerTest.cpp


Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -1343,6 +1343,29 @@
"TemplateType t;", Style);
 }
 
+TEST_F(QualifierFixerTest, Ranges) {
+  FormatStyle Style = getLLVMStyle();
+  Style.QualifierAlignment = FormatStyle::QAS_Custom;
+  Style.QualifierOrder = {"const", "volatile", "type"};
+
+  // Only the first line should be formatted; the second should remain as is.
+  verifyFormat("template  const Foo f();\n"
+   "template  Foo const f();",
+   "template  Foo const f();\n"
+   "template  Foo const f();",
+   Style, std::vector(1, tooling::Range(0, 36)));
+
+  // Only the middle line should be formatted; the first and last should remain
+  // as is.
+  verifyFormat("template  Foo const f();\n"
+   "template  const Foo f();\n"
+   "template  Foo const f();",
+   "template  Foo const f();\n"
+   "template  Foo const f();\n"
+   "template  Foo const f();",
+   Style, std::vector(1, tooling::Range(37, 36)));
+}
+
 } // namespace
 } // namespace test
 } // namespace format
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -587,7 +587,7 @@
   assert(QualifierToken != tok::identifier && "Unrecognised Qualifier");
 
   for (AnnotatedLine *Line : AnnotatedLines) {
-if (Line->InPPDirective)
+if (!Line->Affected || Line->InPPDirective)
   continue;
 FormatToken *First = Line->First;
 assert(First);


Index: clang/unittests/Format/QualifierFixerTest.cpp
===
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -1343,6 +1343,29 @@
"TemplateType t;", Style);
 }
 
+TEST_F(QualifierFixerTest, Ranges) {
+  FormatStyle Style = getLLVMStyle();
+  Style.QualifierAlignment = FormatStyle::QAS_Custom;
+  Style.QualifierOrder = {"const", "volatile", "type"};
+
+  // Only the first line should be formatted; the second should remain as is.
+  verifyFormat("template  const Foo f();\n"
+   "template  Foo const f();",
+   "template  Foo const f();\n"
+   "template  Foo const f();",
+   Style, std::vector(1, tooling::Range(0, 36)));
+
+  // Only the middle line should be formatted; the first and last should remain
+  // as is.
+  verifyFormat("template  Foo const f();\n"
+   "template  const Foo f();\n"
+   "template  Foo const f();",
+   "template  Foo const f();\n"
+   "template  Foo const f();\n"
+   "template  Foo const f();",
+   Style, std::vector(1, tooling::Range(37, 36)));
+}
+
 } // namespace
 } // namespace test
 } // namespace format
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -587,7 +587,7 @@
   assert(QualifierToken != tok::identifier && "Unrecognised Qualifier");
 
   for (AnnotatedLine *Line : AnnotatedLines) {
-if (Line->InPPDirective)
+if (!Line->Affected || Line->InPPDirective)
   continue;
 FormatToken *First = Line->First;
 assert(First);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] c68e92d - Fix MSVC "not all control paths return a value" warning. NFC.

2023-05-03 Thread Simon Pilgrim via cfe-commits

Author: Simon Pilgrim
Date: 2023-05-03T10:23:41+01:00
New Revision: c68e92d941723702810093161be4834f3ca68372

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

LOG: Fix MSVC "not all control paths return a value" warning. NFC.

Added: 


Modified: 
clang/lib/Sema/SemaRISCVVectorLookup.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaRISCVVectorLookup.cpp 
b/clang/lib/Sema/SemaRISCVVectorLookup.cpp
index f2cfa7db5c8d..981ab5f13716 100644
--- a/clang/lib/Sema/SemaRISCVVectorLookup.cpp
+++ b/clang/lib/Sema/SemaRISCVVectorLookup.cpp
@@ -87,6 +87,7 @@ ProtoSeq2ArrayRef(IntrinsicKind K, uint16_t Index, uint8_t 
Length) {
   case IntrinsicKind::SIFIVE_VECTOR:
 return ArrayRef(&RVSiFiveVectorSignatureTable[Index], Length);
   }
+  llvm_unreachable("Unhandled IntrinsicKind");
 }
 
 static QualType RVVType2Qual(ASTContext &Context, const RVVType *Type) {



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


[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

2023-05-03 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision.
kadircet added reviewers: aaron.ballman, ilya-biryukov.
Herald added a subscriber: arphaman.
Herald added a project: All.
kadircet requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

FunctionDecls can be created with null types (D124351 
 added such a new
code path), to be filled in later. But parsing can stop before
completing the Decl (e.g. if code completion
point is reached).
Unfortunately most of the methods in FunctionDecl and its derived
classes assume a complete decl and don't perform null-checks.
Since we're not encountring crashes in the wild along other code paths
today introducing extra checks into quite a lot of places didn't feel
right (due to extra complexity && run time checks).
I believe another alternative would be to change Parser & Sema to never
create decls with invalid types, but I can't really see an easy way of
doing that, as most of the pieces are structured around filling that
information as parsing proceeds.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149733

Files:
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/lib/Index/USRGeneration.cpp


Index: clang/lib/Index/USRGeneration.cpp
===
--- clang/lib/Index/USRGeneration.cpp
+++ clang/lib/Index/USRGeneration.cpp
@@ -226,6 +226,11 @@
   if (ShouldGenerateLocation(D) && GenLoc(D, /*IncludeOffset=*/isLocal(D)))
 return;
 
+  if (D->getType().isNull()) {
+IgnoreResults = true;
+return;
+  }
+
   const unsigned StartSize = Buf.size();
   VisitDeclContext(D->getDeclContext());
   if (Buf.size() == StartSize)
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -4002,6 +4002,19 @@
   EXPECT_EQ(Second.activeParameter, 1);
 }
 
+TEST(CompletionTest, DoNotCrash) {
+  llvm::StringLiteral Cases[] = {
+  R"cpp(
+template  struct Foo {};
+auto a = [x(3)](Foo<^>){};
+)cpp",
+  };
+  for (auto Case : Cases) {
+SCOPED_TRACE(Case);
+auto Completions = completions(Case);
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang


Index: clang/lib/Index/USRGeneration.cpp
===
--- clang/lib/Index/USRGeneration.cpp
+++ clang/lib/Index/USRGeneration.cpp
@@ -226,6 +226,11 @@
   if (ShouldGenerateLocation(D) && GenLoc(D, /*IncludeOffset=*/isLocal(D)))
 return;
 
+  if (D->getType().isNull()) {
+IgnoreResults = true;
+return;
+  }
+
   const unsigned StartSize = Buf.size();
   VisitDeclContext(D->getDeclContext());
   if (Buf.size() == StartSize)
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -4002,6 +4002,19 @@
   EXPECT_EQ(Second.activeParameter, 1);
 }
 
+TEST(CompletionTest, DoNotCrash) {
+  llvm::StringLiteral Cases[] = {
+  R"cpp(
+template  struct Foo {};
+auto a = [x(3)](Foo<^>){};
+)cpp",
+  };
+  for (auto Case : Cases) {
+SCOPED_TRACE(Case);
+auto Completions = completions(Case);
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D146634: [clang][USR] Prevent crashes when parameter lists have nulls

2023-05-03 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet abandoned this revision.
kadircet added a comment.

in favor of D149733 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146634

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


[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

2023-05-03 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a subscriber: erichkeane.
ilya-biryukov added a comment.

It's true that `FunctionType` having a null type does break a lot of even its 
own methods,
e.g. even something as simple as `isVariadic` will dereference a null pointer 
as it uses `getType()->getAs()`.

I am not entirely sure what the contract there should be either.

- Is it the client code of `FunctionDecl` itself that's responsible for 
checking the null type or methods of `FunctionDecl`?
- Would it be easier to avoid creating the functions altogether? (I remember 
seeing some legitimate use-cases for null types there, but I wonder if those 
could be modelled differently).

@aaron.ballman, @erichkeane what is Clang's stance on those? How should we 
proceed here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149733

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


[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

2023-05-03 Thread Stephan Bergmann via Phabricator via cfe-commits
sberg added a comment.

Since this commit (plus its follow-up 
https://github.com/llvm/llvm-project/commit/ce861ec782ae3f41807b61e855512aaccf3c2149
 "[Clang][Sema] Add a temporary workaround in SemaConcept.cpp", to avoid 
`clang/lib/AST/ExprConstant.cpp:15332: bool 
clang::Expr::EvaluateAsConstantExpr(EvalResult &, const ASTContext &, 
ConstantExprKind) const: Assertion `!isValueDependent() && "Expression 
evaluator can't be called on a dependent expression."' failed` in 
`-DLLVM_ENABLE_ASSERTIONS=ON` builds):

  $ cat test.cc
  #include 
  #include 
  #include 
  std::vector> v1;
  std::vector> v2;
  void f() {
v2.insert(v2.end(), std::make_move_iterator(v1.begin()), 
std::make_move_iterator(v1.end()));
  }

  $ clang++ -std=c++20 -fsyntax-only test.cc
  In file included from test.cc:1:
  In file included from 
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/memory:66:
  In file included from 
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_tempbuf.h:61:
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_construct.h:115:4:
 error: no matching function for call to 'construct_at'
std::construct_at(__p, std::forward<_Args>(__args)...);
^
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_uninitialized.h:120:11:
 note: in instantiation of function template specialization 
'std::_Construct, std::unique_ptr &>' requested here
  std::_Construct(std::__addressof(*__cur), *__first);
   ^
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_uninitialized.h:371:14:
 note: in instantiation of function template specialization 
'std::__do_uninit_copy *>, 
std::unique_ptr *>' requested here
  return std::__do_uninit_copy(__first, __last, __result);
  ^
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_uninitialized.h:384:19:
 note: in instantiation of function template specialization 
'std::__uninitialized_copy_a *>, 
std::unique_ptr *, std::unique_ptr>' requested here
return std::__uninitialized_copy_a(_GLIBCXX_MAKE_MOVE_ITERATOR(__first),
^
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/vector.tcc:766:12:
 note: in instantiation of function template specialization 
'std::__uninitialized_move_a *, std::unique_ptr *, 
std::allocator>>' requested here
  std::__uninitialized_move_a(this->_M_impl._M_finish - __n,
   ^
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_vector.h:1481:4:
 note: in instantiation of function template specialization 
'std::vector>::_M_range_insert
 *, std::vector' requested here
_M_range_insert(begin() + __offset, __first, __last,
^
  test.cc:7:6: note: in instantiation of function template specialization 
'std::vector>::insert
 *, std::vector>>>, void>' requested here
v2.insert(v2.end(), std::make_move_iterator(v1.begin()), 
std::make_move_iterator(v1.end()));
   ^
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_construct.h:94:5:
 note: candidate template ignored: substitution failure [with _Tp = 
std::unique_ptr, _Args =  &>]: call to deleted 
constructor of 'std::unique_ptr'
  construct_at(_Tp* __location, _Args&&... __args)
  ^
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_construct.h:119:25:
 error: call to deleted constructor of 'std::unique_ptr'
::new((void*)__p) _Tp(std::forward<_Args>(__args)...);
  ^   ~~~
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/unique_ptr.h:522:7:
 note: 'unique_ptr' has been explicitly marked deleted here
unique_ptr(const unique_ptr&) = delete;
^
  In file included from test.cc:1:
  In file included from 
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/memory:69:
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_uninitialized.h:120:6:
 error: no matching function for call to '_Construct'
  std::_Construct(std::__addressof(*__cur), *__first);
  ^~~
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/stl_uninitialized.h:371:14:
 note: in instantiation of function template specialization 
'std::__do_uninit_copy
 *, std::vector>>>, std::unique_ptr *>' requested here
  return std::__do_uninit_copy(__first, __last, __result);
  ^
  
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/vector.tcc:781:12:
 note: in instantiation of function template specialization 
'std::__uninitialized_copy_a
 *, std::vector>>>, std::unique_ptr *, 
std::unique_ptr>' requested here
  std::__uninitialized_copy_a(__mid, __last,
   ^
  
/usr/lib/gcc/x86_64-redhat-linux/

[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

2023-05-03 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

FWIW, 
https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/Decl.cpp#L2987 is 
the assertion that suggests "null type is fine for functiondecls", but as 
pointed out pretty much all of the methods assume it's non-null && most of the 
users also don't check for validness before use.
so my 2cents, in the past this intermediate state of a functiondecl wasn't 
observable outside, but overtime that "constraint" got violated in various 
places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149733

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


[PATCH] D148066: [RISCV] Add Smaia and Ssaia extensions support

2023-05-03 Thread Alex Bradbury via Phabricator via cfe-commits
asb added a comment.

Just noting for posterity that we discussed this at last week's RISC-V sync-up 
call and I think the tentative conclusion (there weren't particularly strong 
views) was that experiments CSRs should really be gated by 
-menable-experimental-extensions, with the CSR names gated so as to avoid the 
issues mentioned here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148066

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


[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

2023-05-03 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits
kuganv added a comment.

Thanks @kadircet , @sammccall and @ilya-biryukov for the super detailed 
explanation. As mentioned, we will test clangd with an ASAN build to test.

I also would like to get your feedback on the implementation.  After we claim 
the AST as per @sammccall's patch into LiveAST, we seem to have at least two 
options for delayed indexing.

Option 1:

- Change onPreambleAST to get LiveAST to pass it to PreambleThread
- In PreambleThread::build, as part of the scope exit

  if (!ReusedPreamble)
 // Index Preamble

Option 2:

- Change onPreambleAST to get LiveAST as value
- Start an Indexing thread
- TUScheduler can manage another thread for indexing similar to how it does 
preamble and AST thread.

Have you had any thoughts about this. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148088

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


[PATCH] D149640: [clang][dataflow] Change PruneTriviallyFalseEdges for building CFG

2023-05-03 Thread Martin Böhme via Phabricator via cfe-commits
mboehme added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:2690
+// `after_loop` is pruned by PruneTriviallyFalseEdges, so we only 
should
+// have `in_loop`.
+ASSERT_TRUE(Results.empty());

This comment looks stale?



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:2673
-
-EXPECT_EQ(BarVal, FooPointeeVal);
   });

kinu wrote:
> mboehme wrote:
> > It's unfortuante that all of these checks have gone away. I think the test 
> > was actually trying to test something.
> > 
> > I'd suggest checking the environment at two different places:
> > 
> > ```
> > void target(int *Foo) {
> >   do {
> > int Bar = *Foo;
> > // [[in_loop]]
> >   } while (true);
> >   (void)0;
> >   // [[after_loop]]
> > }
> > ```
> > 
> > You can keep the existing checks for the `in_loop` environment and verify 
> > that `Results` doesn't actually contain an environment for `after_loop`.
> Wdyt if we change this to exercise `do { } while (false)` instead (with the 
> checks that we already have), and add a simple while (true) {}?
Thanks, good idea!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149640

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


[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

2023-05-03 Thread Stephan Bergmann via Phabricator via cfe-commits
sberg added a comment.

In D146178#4314999 , @sberg wrote:

> Since this commit...

ah, already taken care of with 
https://github.com/llvm/llvm-project/commit/3e850a6eea5277082a0b7b701754c86530d25c40
 "Revert '[Clang][Sema] Fix comparison of constraint expressions'"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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


[PATCH] D149737: [clang][ExtractAPI] Add semicolon to function declaration fragments

2023-05-03 Thread NagaChaitanya Vellanki via Phabricator via cfe-commits
chaitanyav created this revision.
chaitanyav added a reviewer: dang.
Herald added a reviewer: ributzka.
Herald added a project: All.
chaitanyav requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add missing semicolon at the end of function declarations to fragments


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149737

Files:
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/test/ExtractAPI/availability.c
  clang/test/ExtractAPI/global_record.c
  clang/test/ExtractAPI/global_record_multifile.c
  clang/test/ExtractAPI/macro_undefined.c

Index: clang/test/ExtractAPI/macro_undefined.c
===
--- clang/test/ExtractAPI/macro_undefined.c
+++ clang/test/ExtractAPI/macro_undefined.c
@@ -67,7 +67,7 @@
 },
 {
   "kind": "text",
-  "spelling": "()"
+  "spelling": "();"
 }
   ],
   "functionSignature": {
@@ -173,7 +173,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ");"
 }
   ],
   "functionSignature": {
Index: clang/test/ExtractAPI/global_record_multifile.c
===
--- clang/test/ExtractAPI/global_record_multifile.c
+++ clang/test/ExtractAPI/global_record_multifile.c
@@ -191,7 +191,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ");"
 }
   ],
   "docComment": {
Index: clang/test/ExtractAPI/global_record.c
===
--- clang/test/ExtractAPI/global_record.c
+++ clang/test/ExtractAPI/global_record.c
@@ -189,7 +189,7 @@
 },
 {
   "kind": "text",
-  "spelling": ")"
+  "spelling": ");"
 }
   ],
   "docComment": {
Index: clang/test/ExtractAPI/availability.c
===
--- clang/test/ExtractAPI/availability.c
+++ clang/test/ExtractAPI/availability.c
@@ -76,7 +76,7 @@
 },
 {
   "kind": "text",
-  "spelling": "()"
+  "spelling": "();"
 }
   ],
   "functionSignature": {
@@ -150,7 +150,7 @@
 },
 {
   "kind": "text",
-  "spelling": "()"
+  "spelling": "();"
 }
   ],
   "functionSignature": {
@@ -234,7 +234,7 @@
 },
 {
   "kind": "text",
-  "spelling": "()"
+  "spelling": "();"
 }
   ],
   "functionSignature": {
@@ -334,7 +334,7 @@
 },
 {
   "kind": "text",
-  "spelling": "()"
+  "spelling": "();"
 }
   ],
   "functionSignature": {
@@ -416,7 +416,7 @@
 },
 {
   "kind": "text",
-  "spelling": "()"
+  "spelling": "();"
 }
   ],
   "functionSignature": {
Index: clang/lib/ExtractAPI/DeclarationFragments.cpp
===
--- clang/lib/ExtractAPI/DeclarationFragments.cpp
+++ clang/lib/ExtractAPI/DeclarationFragments.cpp
@@ -457,7 +457,7 @@
   Fragments.append(")", DeclarationFragments::FragmentKind::Text);
 
   // FIXME: Handle exception specifiers: throw, noexcept
-  return Fragments;
+  return Fragments.append(";", DeclarationFragments::FragmentKind::Text);
 }
 
 DeclarationFragments DeclarationFragmentsBuilder::getFragmentsForEnumConstant(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D149640: [clang][dataflow] Change PruneTriviallyFalseEdges for building CFG

2023-05-03 Thread Kinuko Yasuda via Phabricator via cfe-commits
kinu updated this revision to Diff 519029.
kinu added a comment.

Updated the stale comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149640

Files:
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2643,7 +2643,7 @@
 void target(int *Foo) {
   do {
 int Bar = *Foo;
-  } while (true);
+  } while (false);
   (void)0;
   /*[[p]]*/
 }
@@ -2674,6 +2674,24 @@
   });
 }
 
+TEST(TransferTest, UnreachableAfterWhileTrue) {
+  std::string Code = R"(
+void target() {
+  while (true) {}
+  (void)0;
+  /*[[p]]*/
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+// The node after the while-true is pruned because it is trivially
+// known to be unreachable.
+ASSERT_TRUE(Results.empty());
+  });
+}
+
 TEST(TransferTest, AggregateInitialization) {
   std::string BracesCode = R"(
 struct A {
Index: clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
+++ clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
@@ -70,7 +70,7 @@
 llvm::Expected
 ControlFlowContext::build(const Decl *D, Stmt &S, ASTContext &C) {
   CFG::BuildOptions Options;
-  Options.PruneTriviallyFalseEdges = false;
+  Options.PruneTriviallyFalseEdges = true;
   Options.AddImplicitDtors = true;
   Options.AddTemporaryDtors = true;
   Options.AddInitializers = true;


Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2643,7 +2643,7 @@
 void target(int *Foo) {
   do {
 int Bar = *Foo;
-  } while (true);
+  } while (false);
   (void)0;
   /*[[p]]*/
 }
@@ -2674,6 +2674,24 @@
   });
 }
 
+TEST(TransferTest, UnreachableAfterWhileTrue) {
+  std::string Code = R"(
+void target() {
+  while (true) {}
+  (void)0;
+  /*[[p]]*/
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+// The node after the while-true is pruned because it is trivially
+// known to be unreachable.
+ASSERT_TRUE(Results.empty());
+  });
+}
+
 TEST(TransferTest, AggregateInitialization) {
   std::string BracesCode = R"(
 struct A {
Index: clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
+++ clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
@@ -70,7 +70,7 @@
 llvm::Expected
 ControlFlowContext::build(const Decl *D, Stmt &S, ASTContext &C) {
   CFG::BuildOptions Options;
-  Options.PruneTriviallyFalseEdges = false;
+  Options.PruneTriviallyFalseEdges = true;
   Options.AddImplicitDtors = true;
   Options.AddTemporaryDtors = true;
   Options.AddInitializers = true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D149640: [clang][dataflow] Change PruneTriviallyFalseEdges for building CFG

2023-05-03 Thread Kinuko Yasuda via Phabricator via cfe-commits
kinu marked an inline comment as done.
kinu added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:2690
+// `after_loop` is pruned by PruneTriviallyFalseEdges, so we only 
should
+// have `in_loop`.
+ASSERT_TRUE(Results.empty());

mboehme wrote:
> This comment looks stale?
Good catch, updated!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149640

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


[PATCH] D149013: [clang][Interp] Check pointers when accessing base class

2023-05-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added subscribers: hubert.reinterpretcast, rsmith.
aaron.ballman added inline comments.



Comment at: clang/test/AST/Interp/records.cpp:509-512
+  constexpr A *a2 = &b + 1; // expected-error {{must be initialized by a 
constant expression}} \
+// expected-note {{cannot access base class of 
pointer past the end of object}} \
+// ref-error {{must be initialized by a constant 
expression}} \
+// ref-note {{cannot access base class of pointer 
past the end of object}}

tbaeder wrote:
> cor3ntin wrote:
> > aaron.ballman wrote:
> > > I may have jumped the gun on accepting this, actually. Forming the 
> > > pointer to `&b + 1` is fine, but evaluating it by dereferencing it would 
> > > be UB. e.g., http://eel.is/c++draft/expr.const#13.3
> > We probably want tests that ensure `&b+2` is invalid in all cases
> It's being evaluated because the base class is accessed, isn't it? if `b` was 
> of type `A*`, we would not emit a diagnostic (see line 506 above).
I'm not seeing the access to the base class -- we're forming a pointer, not 
dereferencing it.

https://godbolt.org/z/vxThqczeo

I think this is an existing Clang bug. CC @hubert.reinterpretcast @rsmith for 
additional opinions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149013

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


[PATCH] D149149: [clang][Interp] Check one-past-the-end pointers in GetPtrField

2023-05-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/test/AST/Interp/records.cpp:500
 
 namespace PointerArith {
   struct A {};

shafik wrote:
> aaron.ballman wrote:
> > Neat! For the tests in this namespace, Clang and ICC agree, GCC and MSVC 
> > agree, and users get to cry: https://godbolt.org/z/7EWWrY5z6
> Yeah, unfortunate but I am pretty sure clang is correct on both of those.
I think Clang is correct on one of those, but not both of them. I think Clang 
is correct to reject `constexpr const int *pn = &(&b + 1)->n;` as that involves 
dereferencing the one-past-the-end pointer, but I think Clang is wrong to 
reject `constexpr A *a2 = &b + 1;` as there's no dereference there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149149

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


[PATCH] D149695: MS inline asm: remove obsolete code adding AOK_SizeDirective (e.g. dword ptr)

2023-05-03 Thread Hans Wennborg via Phabricator via cfe-commits
hans added a comment.

> The AOK_SizeDirective part from 5b37c181291210bedfbb7a6af5d51229f3652ef0
> (2014-08) seems unneeded nowadays (the root cause has likely been fixed
> elsewhere).

Would it be possible to confirm when/if the size directive here became 
unnecessary?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149695

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


[PATCH] D146764: [clang] Make predefined expressions string literals under -fms-extensions

2023-05-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D146764#4313905 , @aeubanks wrote:

> added the warning, not sure if this needs more tests for testing the 
> interaction between `-pedantic`, `-Wmicrosoft`, 
> `-Wmicrosoft-init-from-predefined` or if that's already assumed to work

That's generally assumed to work, really the only question is whether we want 
`Extension` (only warned in `-pedantic`) or `ExtWarn` (warned by default). I 
think most of our Microsoft extension warnings are `ExtWarn` and it seems 
defensible for this to be on by default, but it'd be nice to know just how 
chatty this is on real world code bases too. I have the sneaking suspicion this 
code pattern doesn't come up often enough for this to be "too chatty", but if 
we find it triggers on popular libraries we might want to consider dialing it 
back to just `Extension`.




Comment at: clang/test/Modules/predefined.cpp:6
+// RUN: %clang_cc1 -x c++ -std=c++20 -emit-module-interface a.h -o a.pcm 
-fms-extensions
+// RUN: %clang_cc1 -std=c++20 a.cpp -fmodule-file=A=a.pcm -fms-extensions 
-fsyntax-only
+

aeubanks wrote:
> aaron.ballman wrote:
> > Er, should we have a `-verify` on this as well as `// 
> > expected-no-diagnostics`?
> isn't that tested by the Sema test?
Not quite. The Sema test does test a lot of the functionality, but this is 
testing whether we import the AST node properly and can still use it, so it's 
testing a different code path for generating the AST nodes used for doing 
semantic checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146764

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


[PATCH] D149650: Give NullabilityKind a printing operator<

2023-05-03 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a reviewer: aaron.ballman.
sammccall added a comment.

In D149650#4312389 , @aaron.ballman 
wrote:

> I guess I'm not seeing much motivation for this change. We already have 
> `clang::getNullabilitySpelling()` and `const StreamingDiagnostic 
> &clang::operator<<(const StreamingDiagnostic &DB, DiagNullabilityKind 
> nullability)` and now we're adding a third way to get this information. If 
> this is just for debug/testing purposes, can we use existing debug formatters 
> to convert the enumeration value into the enumerator name instead?

We're using NullabilityKind in our dataflow-based null-safety clang-tidy check 
 that we hope to 
upstream when it works.
(The idea is to use `_Nullable` and `_Nonnull` annotations on API boundaries to 
gradually type pointers, and to provide a verification clang-tidy check and 
libraries to infer annotations for existing code. The actual clang-tidy check 
wrapper isn't in that repo yet, but it's trivial).

It would be convenient if e.g. EXPECT_THAT(someVectorOfNK, 
ElementsAre(Nullable, Unspecified)) 

 printed something useful when it fails, if we could write OS << NK and get 
Unspecified rather than OS << getSpelling(NK) and get _Unspecified_Nullability, 
etc.
This doesn't concern clang core (ha, there are no unit tests...) but there's no 
reasonable way to do this downstream without using a different type.

> `StreamingDiagnostic &clang::operator<<`

This actually wants the user-visible spelling, so I think it can just use 
getSpelling... if I make that change we're back to just two implementations :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149650

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


cfe-commits@lists.llvm.org

2023-05-03 Thread Jens Massberg via Phabricator via cfe-commits
massberg updated this revision to Diff 519042.
massberg marked an inline comment as done.
massberg added a comment.

Fixed typo, added release note and updated cxx_status.html.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148924

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
  clang/test/CodeGenCXX/virtual-compare.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1005,7 +1005,7 @@
   

 https://wg21.link/p2002r1";>P2002R1
-Partial
+Clang 17
   
   
 https://wg21.link/p2085r0";>P2085R0
Index: clang/test/CodeGenCXX/virtual-compare.cpp
===
--- clang/test/CodeGenCXX/virtual-compare.cpp
+++ clang/test/CodeGenCXX/virtual-compare.cpp
@@ -21,8 +21,6 @@
   virtual std::strong_ordering operator<=>(const A &) const & = default;
   // CHECK-SAME: @_ZN1A1gEv
   virtual void g();
-  // CHECK-SAME: @_ZNKO1AssERKS_
-  virtual std::strong_ordering operator<=>(const A &) const && = default;
   // CHECK-SAME: @_ZN1A1hEv
   virtual void h();
 
@@ -35,9 +33,6 @@
 
   // CHECK-SAME: @_ZNKR1AeqERKS_
   // implicit virtual A &operator==(const A&) const & = default;
-
-  // CHECK-SAME: @_ZNKO1AeqERKS_
-  // implicit virtual A &operator==(const A&) const && = default;
 };
 
 // For Y:
Index: clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
===
--- clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
+++ clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
@@ -15,7 +15,8 @@
 
   bool operator<(const A&) const;
   bool operator<=(const A&) const = default;
-  bool operator==(const A&) const volatile && = default; // surprisingly, OK
+  bool operator==(const A&) const && = default; // expected-error {{ref-qualifier '&&' is not allowed on a defaulted comparison operators}}
+  bool operator>=(const A&) const volatile = default; // expected-error {{defaulted comparison function must not be volatile}}
   bool operator<=>(const A&) = default; // expected-error {{defaulted member three-way comparison operator must be const-qualified}}
   bool operator>=(const B&) const = default; // expected-error-re {{invalid parameter type for defaulted relational comparison operator; found 'const B &', expected 'const A &'{{$
   static bool operator>(const B&) = default; // expected-error {{overloaded 'operator>' cannot be a static member function}}
@@ -185,6 +186,11 @@
 friend bool operator==(const S6&, const S6&) = default; // expected-error {{because it was already declared outside}}
 };
 
+struct S7 {
+  bool operator==(S7 const &) const &&;
+};
+bool S7::operator==(S7 const &) const && = default; // expected-error {{ref-qualifier '&&' is not allowed on a defaulted comparison operators}}
+
 enum e {};
 bool operator==(e, int) = default; // expected-error{{expected class or reference to a constant class}}
 
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -8579,8 +8579,8 @@
   // C++2a [class.compare.default]p1:
   //   A defaulted comparison operator function for some class C shall be a
   //   non-template function declared in the member-specification of C that is
-  //-- a non-static const member of C having one parameter of type
-  //   const C&, or
+  //-- a non-static const non-volatile member of C having one parameter of type
+  //   const C& and either no ref-qualifier or the ref-qualifier &, or
   //-- a friend of C having two parameters of type const C& or two
   //   parameters of type C.
 
@@ -8590,6 +8590,17 @@
 auto *MD = cast(FD);
 assert(!MD->isStatic() && "comparison function cannot be a static member");
 
+if (MD->getRefQualifier() == RQ_RValue) {
+  Diag(MD->getLocation(), diag::err_ref_qualifier_comparison_operator);
+
+  // Remove the ref qualifier to recover.
+  const auto *FPT = MD->getType()->castAs();
+  FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+  EPI.RefQualifier = RQ_None;
+  MD->setType(Context.getFunctionType(FPT->getReturnType(),
+  FPT->getParamTypes(), EPI));
+}
+
 // If we're out-of-class, this is the class we're comparing.
 if (!RD)
   RD = MD->getParent();
@@ -8612,6 +8623,17 @@
   MD->setType(Context.getFunctionType(FPT->getReturnType(),
   FPT->getParamTypes(), EPI));
 }
+
+if (MD->isVolatile()) {
+  Diag(M

[PATCH] D149650: Give NullabilityKind a printing operator<

2023-05-03 Thread Sam McCall via Phabricator via cfe-commits
sammccall updated this revision to Diff 519046.
sammccall added a comment.

Make diagnostic-printing use the (other) existing function, come out neutral!

(This adds quotes to some diagnostic, but their omission seems to be an error)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149650

Files:
  clang/include/clang/Basic/Specifiers.h
  clang/lib/Basic/Diagnostic.cpp
  clang/lib/Basic/IdentifierTable.cpp
  clang/test/SemaObjC/nullable-result.m

Index: clang/test/SemaObjC/nullable-result.m
===
--- clang/test/SemaObjC/nullable-result.m
+++ clang/test/SemaObjC/nullable-result.m
@@ -25,9 +25,9 @@
 }
 
 void test_dup(void) {
-  id _Nullable_result _Nullable_result a; // expected-warning {{duplicate nullability specifier _Nullable_result}}
-  id _Nullable _Nullable_result b; // expected-error{{nullability specifier _Nullable_result conflicts with existing specifier '_Nullable'}}
-  id _Nullable_result _Nonnull c; // expected-error{{nullability specifier '_Nonnull' conflicts with existing specifier _Nullable_result}}
+  id _Nullable_result _Nullable_result a; // expected-warning {{duplicate nullability specifier '_Nullable_result'}}
+  id _Nullable _Nullable_result b; // expected-error{{nullability specifier '_Nullable_result' conflicts with existing specifier '_Nullable'}}
+  id _Nullable_result _Nonnull c; // expected-error{{nullability specifier '_Nonnull' conflicts with existing specifier '_Nullable_result'}}
 }
 
 @interface NoContextSensitive
Index: clang/lib/Basic/IdentifierTable.cpp
===
--- clang/lib/Basic/IdentifierTable.cpp
+++ clang/lib/Basic/IdentifierTable.cpp
@@ -849,6 +849,20 @@
   llvm_unreachable("Unknown nullability kind.");
 }
 
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, NullabilityKind NK) {
+  switch (NK) {
+  case NullabilityKind::NonNull:
+return OS << "NonNull";
+  case NullabilityKind::Nullable:
+return OS << "Nullable";
+  case NullabilityKind::NullableResult:
+return OS << "NullableResult";
+  case NullabilityKind::Unspecified:
+return OS << "Unspecified";
+  }
+  llvm_unreachable("Unknown nullability kind.");
+}
+
 diag::kind
 IdentifierTable::getFutureCompatDiagKind(const IdentifierInfo &II,
  const LangOptions &LangOpts) {
Index: clang/lib/Basic/Diagnostic.cpp
===
--- clang/lib/Basic/Diagnostic.cpp
+++ clang/lib/Basic/Diagnostic.cpp
@@ -43,28 +43,12 @@
 
 const StreamingDiagnostic &clang::operator<<(const StreamingDiagnostic &DB,
  DiagNullabilityKind nullability) {
-  StringRef string;
-  switch (nullability.first) {
-  case NullabilityKind::NonNull:
-string = nullability.second ? "'nonnull'" : "'_Nonnull'";
-break;
-
-  case NullabilityKind::Nullable:
-string = nullability.second ? "'nullable'" : "'_Nullable'";
-break;
-
-  case NullabilityKind::Unspecified:
-string = nullability.second ? "'null_unspecified'" : "'_Null_unspecified'";
-break;
-
-  case NullabilityKind::NullableResult:
-assert(!nullability.second &&
-   "_Nullable_result isn't supported as context-sensitive keyword");
-string = "_Nullable_result";
-break;
-  }
-
-  DB.AddString(string);
+  DB.AddString(
+  ("'" +
+   getNullabilitySpelling(nullability.first,
+  /*isContextSensitive=*/nullability.second) +
+   "'")
+  .str());
   return DB;
 }
 
Index: clang/include/clang/Basic/Specifiers.h
===
--- clang/include/clang/Basic/Specifiers.h
+++ clang/include/clang/Basic/Specifiers.h
@@ -19,6 +19,9 @@
 #include "llvm/Support/DataTypes.h"
 #include "llvm/Support/ErrorHandling.h"
 
+namespace llvm {
+class raw_ostream;
+} // namespace llvm
 namespace clang {
 
   /// Define the meaning of possible values of the kind in ExplicitSpecifier.
@@ -333,6 +336,8 @@
 // parameters are assumed to only get null on error.
 NullableResult,
   };
+  /// Prints human-readable debug representation.
+  llvm::raw_ostream &operator<<(llvm::raw_ostream&, NullabilityKind);
 
   /// Return true if \p L has a weaker nullability annotation than \p R. The
   /// ordering is: Unspecified < Nullable < NonNull.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 22e2db6 - [clang] Reject flexible array member in a union in C++

2023-05-03 Thread Mariya Podchishchaeva via cfe-commits

Author: Mariya Podchishchaeva
Date: 2023-05-03T08:54:35-04:00
New Revision: 22e2db6010b029ebd4c6d3d1fd30224d8b3109ef

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

LOG: [clang] Reject flexible array member in a union in C++

It was rejected in C, and in a strange way accepted in C++. However, the
support was never properly tested and fully implemented, so just reject
it in C++ mode as well.

This change also fixes crash on attempt to initialize union with flexible
array member. Due to missing check on union, there was a null expression
added to init list that caused crash later.

Fixes https://github.com/llvm/llvm-project/issues/61746

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaInit.cpp
clang/test/Layout/aix-power-alignment-typedef.cpp
clang/test/Sema/MicrosoftExtensions.c
clang/test/Sema/init.c
clang/test/SemaCXX/flexible-array-test.cpp
clang/test/SemaCXX/gnu-flags.cpp
clang/test/SemaObjCXX/flexible-array.mm

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8d0a9c96a9579..416d816796514 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -55,6 +55,7 @@ C++ Specific Potentially Breaking Changes
 -
 - Clang won't search for coroutine_traits in std::experimental namespace any 
more.
   Clang will only search for std::coroutine_traits for coroutines then.
+- Clang now rejects unions containing a flexible array member.
 
 ABI Changes in This Version
 ---

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 0ee43fb8837a1..4313948515752 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -267,7 +267,6 @@ def ExtraSemi : DiagGroup<"extra-semi", 
[CXX98CompatExtraSemi,
  CXX11ExtraSemi]>;
 
 def GNUFlexibleArrayInitializer : DiagGroup<"gnu-flexible-array-initializer">;
-def GNUFlexibleArrayUnionMember : DiagGroup<"gnu-flexible-array-union-member">;
 def GNUFoldingConstant : DiagGroup<"gnu-folding-constant">;
 def FormatInsufficientArgs : DiagGroup<"format-insufficient-args">;
 def FormatExtraArgs : DiagGroup<"format-extra-args">;
@@ -1137,7 +1136,7 @@ def GNU : DiagGroup<"gnu", [GNUAlignofExpression, 
GNUAnonymousStruct,
 GNUConditionalOmittedOperand, GNUDesignator,
 GNUEmptyStruct,
 VLAExtension, GNUFlexibleArrayInitializer,
-GNUFlexibleArrayUnionMember, GNUFoldingConstant,
+GNUFoldingConstant,
 GNUImaginaryConstant, GNUIncludeNext,
 GNULabelsAsValue, GNULineMarker, 
GNUNullPointerArithmetic,
 GNUOffsetofExtensions, GNUPointerArith,

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 17585752edf8e..c3cb6740a5131 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6232,6 +6232,9 @@ def ext_variable_sized_type_in_struct : ExtWarn<
 
 def ext_c99_flexible_array_member : Extension<
   "flexible array members are a C99 feature">, InGroup;
+// Flexible array members in unions are not supported, but union case is still
+// present in the diagnostic so it matches TagTypeKind enum and can be emitted
+// with Diag(...) << ... << SomeTagDecl->getTagKind().
 def err_flexible_array_virtual_base : Error<
   "flexible array member %0 not allowed in "
   "%select{struct|interface|union|class|enum}1 which has a virtual base 
class">;
@@ -6254,15 +6257,10 @@ def ext_flexible_array_empty_aggregate_ms : Extension<
   InGroup;
 def err_flexible_array_union : Error<
   "flexible array member %0 in a union is not allowed">;
-def ext_flexible_array_union_ms : Extension<
-  "flexible array member %0 in a union is a Microsoft extension">,
-  InGroup;
 def ext_flexible_array_empty_aggregate_gnu : Extension<
   "flexible array member %0 in otherwise empty "
   "%select{struct|interface|union|class|enum}1 is a GNU extension">,
   InGroup;
-def ext_flexible_array_union_gnu : Extension<
-  "flexible array member %0 in a union is a GNU extension">, 
InGroup;
 
 def err_flexible_array_not_at_end : Error<
   "flexible array member %0 with type %1 is not at the end of"

diff  --git a/clang/lib/Sema/Se

[PATCH] D147626: [clang] Reject flexible array member in a union in C++

2023-05-03 Thread Mariya Podchishchaeva via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG22e2db6010b0: [clang] Reject flexible array member in a 
union in C++ (authored by Fznamznon).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147626

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/test/Layout/aix-power-alignment-typedef.cpp
  clang/test/Sema/MicrosoftExtensions.c
  clang/test/Sema/init.c
  clang/test/SemaCXX/flexible-array-test.cpp
  clang/test/SemaCXX/gnu-flags.cpp
  clang/test/SemaObjCXX/flexible-array.mm

Index: clang/test/SemaObjCXX/flexible-array.mm
===
--- clang/test/SemaObjCXX/flexible-array.mm
+++ clang/test/SemaObjCXX/flexible-array.mm
@@ -4,7 +4,7 @@
 
 union VariableSizeUnion {
   int s;
-  char c[];
+  char c[]; //expected-error {{flexible array member 'c' in a union is not allowed}}
 };
 
 @interface LastUnionIvar {
Index: clang/test/SemaCXX/gnu-flags.cpp
===
--- clang/test/SemaCXX/gnu-flags.cpp
+++ clang/test/SemaCXX/gnu-flags.cpp
@@ -8,34 +8,27 @@
 
 // RUN: %clang_cc1 -fsyntax-only -verify %s -DALL -Wno-gnu \
 // RUN:   -Wgnu-anonymous-struct -Wredeclared-class-member \
-// RUN:   -Wgnu-flexible-array-union-member -Wgnu-folding-constant \
-// RUN:   -Wgnu-empty-struct
+// RUN:   -Wgnu-folding-constant -Wgnu-empty-struct
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s -DALL -Wno-gnu \
 // RUN:   -Wgnu-anonymous-struct -Wredeclared-class-member \
-// RUN:   -Wgnu-flexible-array-union-member -Wgnu-folding-constant \
-// RUN:   -Wgnu-empty-struct
+// RUN:   -Wgnu-folding-constant -Wgnu-empty-struct
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s -DALL -Wno-gnu \
 // RUN:   -Wgnu-anonymous-struct -Wredeclared-class-member \
-// RUN:   -Wgnu-flexible-array-union-member -Wgnu-folding-constant \
-// RUN:   -Wgnu-empty-struct
+// RUN:   -Wgnu-folding-constant -Wgnu-empty-struct
 
 // RUN: %clang_cc1 -fsyntax-only -verify %s -DNONE -Wgnu \
 // RUN:   -Wno-gnu-anonymous-struct -Wno-redeclared-class-member \
-// RUN:   -Wno-gnu-flexible-array-union-member -Wno-gnu-folding-constant \
-// RUN:   -Wno-gnu-empty-struct
+// RUN:   -Wno-gnu-folding-constant -Wno-gnu-empty-struct
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s -DNONE -Wgnu \
 // RUN:   -Wno-gnu-anonymous-struct -Wno-redeclared-class-member \
-// RUN:   -Wno-gnu-flexible-array-union-member -Wno-gnu-folding-constant \
-// RUN:   -Wno-gnu-empty-struct
+// RUN:   -Wno-gnu-folding-constant -Wno-gnu-empty-struct
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s -DNONE -Wgnu \
 // RUN:   -Wno-gnu-anonymous-struct -Wno-redeclared-class-member \
-// RUN:   -Wno-gnu-flexible-array-union-member -Wno-gnu-folding-constant \
-// RUN:   -Wno-gnu-empty-struct
+// RUN:   -Wno-gnu-folding-constant -Wno-gnu-empty-struct
 
 // Additional disabled tests:
 // %clang_cc1 -fsyntax-only -verify %s -DANONYMOUSSTRUCT -Wno-gnu -Wgnu-anonymous-struct
 // %clang_cc1 -fsyntax-only -verify %s -DREDECLAREDCLASSMEMBER -Wno-gnu -Wredeclared-class-member
-// %clang_cc1 -fsyntax-only -verify %s -DFLEXIBLEARRAYUNIONMEMBER -Wno-gnu -Wgnu-flexible-array-union-member
 // %clang_cc1 -fsyntax-only -verify %s -DFOLDINGCONSTANT -Wno-gnu -Wgnu-folding-constant
 // %clang_cc1 -fsyntax-only -verify %s -DEMPTYSTRUCT -Wno-gnu -Wgnu-empty-struct
 
@@ -70,19 +63,6 @@
   };
 }
 
-
-#if ALL || FLEXIBLEARRAYUNIONMEMBER
-// expected-warning@+6 {{flexible array member 'c1' in a union is a GNU extension}}
-#endif
-
-struct faum {
-   int l;
-   union {
-   int c1[];
-   };
-};
-
-
 #if (ALL || FOLDINGCONSTANT) && (__cplusplus <= 199711L) // C++03 or earlier modes
 // expected-warning@+4 {{in-class initializer for static data member is not a constant expression; folding it to a constant is a GNU extension}}
 #endif
Index: clang/test/SemaCXX/flexible-array-test.cpp
===
--- clang/test/SemaCXX/flexible-array-test.cpp
+++ clang/test/SemaCXX/flexible-array-test.cpp
@@ -16,7 +16,7 @@
 
 struct Rec {
   union { // expected-warning-re {{variable sized type '{{.*}}' not at the end of a struct or class is a GNU extension}}
-int u0[];
+int u0[]; // expected-error {{flexible array member 'u0' in a union is not allowed}}
   };
   int x;
 } rec;
@@ -63,7 +63,7 @@
 
 union B {
   int s;
-  char c[];
+  char c[]; // expected-error {{flexible array member 'c' in a union is not allowed}}
 };
 
 class C {
Index: clang/test/Sema/init.c
===
--- clang/test/Sema/init.c
+++ clang/test/Sema/init.c
@@ -164,3 +164,6 @@
 
 typedef struct { uintptr_t x : 2; } StructWithBitfield;
 StructWith

[PATCH] D148370: [Clang][Flang][OpenMP] Add loadOffloadInfoMetadata and createOffloadEntriesAndInfoMetadata into OMPIRBuilder's finalize and initialize

2023-05-03 Thread Andrew Gozillon via Phabricator via cfe-commits
agozillon updated this revision to Diff 519048.
agozillon added a comment.

- Move hostIRFilePath initialize invocation to ModuleTranslation.cpp to respect 
TargetOp patch
- Apply reviewer feedback
- Tidy up missed style guidelines i sillily forgot about


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148370

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/CMakeLists.txt
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
  mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

Index: mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
===
--- mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -1259,19 +1259,23 @@
 llvm::OpenMPIRBuilder *ModuleTranslation::getOpenMPBuilder() {
   if (!ompBuilder) {
 ompBuilder = std::make_unique(*llvmModule);
-ompBuilder->initialize();
 
 bool isDevice = false;
+llvm::StringRef hostIRFilePath = "";
 if (auto offloadMod =
-dyn_cast(mlirModule))
+dyn_cast(mlirModule)) {
   isDevice = offloadMod.getIsDevice();
+  hostIRFilePath = offloadMod.getHostIRFilePath();
+}
+
+ompBuilder->initialize(hostIRFilePath);
 
 // TODO: set the flags when available
-llvm::OpenMPIRBuilderConfig Config(
+llvm::OpenMPIRBuilderConfig config(
 isDevice, /* IsTargetCodegen */ false,
 /* HasRequiresUnifiedSharedMemory */ false,
 /* OpenMPOffloadMandatory */ false);
-ompBuilder->setConfig(Config);
+ompBuilder->setConfig(config);
   }
   return ompBuilder.get();
 }
Index: mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
===
--- mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
+++ mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
@@ -15,6 +15,7 @@
 #define MLIR_TARGET_LLVMIR_MODULETRANSLATION_H
 
 #include "mlir/Dialect/LLVMIR/LLVMInterfaces.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/IR/Operation.h"
 #include "mlir/IR/SymbolTable.h"
 #include "mlir/IR/Value.h"
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -21,6 +21,7 @@
 #include "llvm/Analysis/OptimizationRemarkEmitter.h"
 #include "llvm/Analysis/ScalarEvolution.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
+#include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfoMetadata.h"
@@ -445,7 +446,29 @@
   return Fn;
 }
 
-void OpenMPIRBuilder::initialize() { initializeTypes(M); }
+void OpenMPIRBuilder::initialize(StringRef HostFilePath) {
+  initializeTypes(M);
+
+  if (HostFilePath.empty())
+return;
+
+  auto Buf = MemoryBuffer::getFile(HostFilePath);
+  if (auto Err = Buf.getError())
+report_fatal_error(("error opening host file from host file path inside of "
+"OpenMPIRBuilder: " +
+Err.message())
+   .c_str());
+
+  LLVMContext Ctx;
+  auto M = expectedToErrorOrAndEmitErrors(
+  Ctx, parseBitcodeFile(Buf.get()->getMemBufferRef(), Ctx));
+  if (auto Err = M.getError())
+report_fatal_error(
+("error parsing host file inside of OpenMPIRBuilder: " + Err.message())
+.c_str());
+
+  loadOffloadInfoMetadata(*M.get());
+}
 
 void OpenMPIRBuilder::finalize(Function *Fn) {
   SmallPtrSet ParallelRegionBlockSet;
@@ -534,6 +557,17 @@
 
   // Remove work items that have been completed.
   OutlineInfos = std::move(DeferredOutlines);
+
+  OpenMPIRBuilder::EmitMetadataErrorReportFunctionTy &&ErrorReportFn =
+  [](OpenMPIRBuilder::EmitMetadataErrorKind Kind,
+ const TargetRegionEntryInfo &EntryInfo) -> void {
+errs() << "Error of kind: " << Kind
+   << " when emitting offload entries and metadata during "
+  "OMPIRBuilder finalization \n";
+  };
+
+  if (!OffloadInfoManager.empty())
+createOffloadEntriesAndInfoMetadata(ErrorReportFn);
 }
 
 OpenMPIRBuilder::~OpenMPIRBuilder() {
Index: llvm/lib/Frontend/OpenMP/CMakeLists.txt
===
--- llvm/lib/Frontend/OpenMP/CMakeLists.txt
+++ llvm/lib/Frontend/OpenMP/CMakeLists.txt
@@ -19,4 +19,5 @@
   Analysis
   MC
   Scalar
+  BitReader
   )
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -418,8 +418,14 @@
 
   /// Initialize the internal state, this will put structures types and
   

[PATCH] D148370: [Clang][Flang][OpenMP] Add loadOffloadInfoMetadata and createOffloadEntriesAndInfoMetadata into OMPIRBuilder's finalize and initialize

2023-05-03 Thread Andrew Gozillon via Phabricator via cfe-commits
agozillon updated this revision to Diff 519050.
agozillon marked an inline comment as done.
agozillon added a comment.

- Remove unneccessary OMPIRBuilder scope as well


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148370

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/CMakeLists.txt
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
  mlir/lib/Target/LLVMIR/ModuleTranslation.cpp

Index: mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
===
--- mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -1259,19 +1259,23 @@
 llvm::OpenMPIRBuilder *ModuleTranslation::getOpenMPBuilder() {
   if (!ompBuilder) {
 ompBuilder = std::make_unique(*llvmModule);
-ompBuilder->initialize();
 
 bool isDevice = false;
+llvm::StringRef hostIRFilePath = "";
 if (auto offloadMod =
-dyn_cast(mlirModule))
+dyn_cast(mlirModule)) {
   isDevice = offloadMod.getIsDevice();
+  hostIRFilePath = offloadMod.getHostIRFilePath();
+}
+
+ompBuilder->initialize(hostIRFilePath);
 
 // TODO: set the flags when available
-llvm::OpenMPIRBuilderConfig Config(
+llvm::OpenMPIRBuilderConfig config(
 isDevice, /* IsTargetCodegen */ false,
 /* HasRequiresUnifiedSharedMemory */ false,
 /* OpenMPOffloadMandatory */ false);
-ompBuilder->setConfig(Config);
+ompBuilder->setConfig(config);
   }
   return ompBuilder.get();
 }
Index: mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
===
--- mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
+++ mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
@@ -15,6 +15,7 @@
 #define MLIR_TARGET_LLVMIR_MODULETRANSLATION_H
 
 #include "mlir/Dialect/LLVMIR/LLVMInterfaces.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/IR/Operation.h"
 #include "mlir/IR/SymbolTable.h"
 #include "mlir/IR/Value.h"
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -21,6 +21,7 @@
 #include "llvm/Analysis/OptimizationRemarkEmitter.h"
 #include "llvm/Analysis/ScalarEvolution.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
+#include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/IR/CFG.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DebugInfoMetadata.h"
@@ -445,7 +446,29 @@
   return Fn;
 }
 
-void OpenMPIRBuilder::initialize() { initializeTypes(M); }
+void OpenMPIRBuilder::initialize(StringRef HostFilePath) {
+  initializeTypes(M);
+
+  if (HostFilePath.empty())
+return;
+
+  auto Buf = MemoryBuffer::getFile(HostFilePath);
+  if (auto Err = Buf.getError())
+report_fatal_error(("error opening host file from host file path inside of "
+"OpenMPIRBuilder: " +
+Err.message())
+   .c_str());
+
+  LLVMContext Ctx;
+  auto M = expectedToErrorOrAndEmitErrors(
+  Ctx, parseBitcodeFile(Buf.get()->getMemBufferRef(), Ctx));
+  if (auto Err = M.getError())
+report_fatal_error(
+("error parsing host file inside of OpenMPIRBuilder: " + Err.message())
+.c_str());
+
+  loadOffloadInfoMetadata(*M.get());
+}
 
 void OpenMPIRBuilder::finalize(Function *Fn) {
   SmallPtrSet ParallelRegionBlockSet;
@@ -534,6 +557,17 @@
 
   // Remove work items that have been completed.
   OutlineInfos = std::move(DeferredOutlines);
+
+  EmitMetadataErrorReportFunctionTy &&ErrorReportFn =
+  [](EmitMetadataErrorKind Kind,
+ const TargetRegionEntryInfo &EntryInfo) -> void {
+errs() << "Error of kind: " << Kind
+   << " when emitting offload entries and metadata during "
+  "OMPIRBuilder finalization \n";
+  };
+
+  if (!OffloadInfoManager.empty())
+createOffloadEntriesAndInfoMetadata(ErrorReportFn);
 }
 
 OpenMPIRBuilder::~OpenMPIRBuilder() {
Index: llvm/lib/Frontend/OpenMP/CMakeLists.txt
===
--- llvm/lib/Frontend/OpenMP/CMakeLists.txt
+++ llvm/lib/Frontend/OpenMP/CMakeLists.txt
@@ -19,4 +19,5 @@
   Analysis
   MC
   Scalar
+  BitReader
   )
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -418,8 +418,14 @@
 
   /// Initialize the internal state, this will put structures types and
   /// potentially other helpers into the underlying module. Must be called
-  /// before any other method and only once!
-

[PATCH] D148370: [Clang][Flang][OpenMP] Add loadOffloadInfoMetadata and createOffloadEntriesAndInfoMetadata into OMPIRBuilder's finalize and initialize

2023-05-03 Thread Andrew Gozillon via Phabricator via cfe-commits
agozillon added a comment.

Updated to fix the style issues that were present and pointed out by @jdoerfert 
thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148370

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


[PATCH] D149744: [clang][dataflow][NFC] Eliminate unnecessary helper `stripReference()`.

2023-05-03 Thread Martin Böhme via Phabricator via cfe-commits
mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

`QualType::getNonReferenceType()` does the same thing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149744

Files:
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp


Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -276,12 +276,6 @@
   return nullptr;
 }
 
-/// If `Type` is a reference type, returns the type of its pointee. Otherwise,
-/// returns `Type` itself.
-QualType stripReference(QualType Type) {
-  return Type->isReferenceType() ? Type->getPointeeType() : Type;
-}
-
 /// Returns true if and only if `Type` is an optional type.
 bool isOptionalType(QualType Type) {
   if (!Type->isRecordType())
@@ -339,7 +333,7 @@
 return &ValueLoc;
   }
 
-  auto Ty = stripReference(Q);
+  auto Ty = Q.getNonReferenceType();
   auto *ValueVal = Env.createValue(Ty);
   if (ValueVal == nullptr)
 return nullptr;
@@ -493,11 +487,13 @@
   assert(F.getTemplateSpecializationArgs() != nullptr);
   assert(F.getTemplateSpecializationArgs()->size() > 0);
 
-  const int TemplateParamOptionalWrappersCount = countOptionalWrappers(
-  *MatchRes.Context,
-  stripReference(F.getTemplateSpecializationArgs()->get(0).getAsType()));
-  const int ArgTypeOptionalWrappersCount =
-  countOptionalWrappers(*MatchRes.Context, stripReference(E.getType()));
+  const int TemplateParamOptionalWrappersCount =
+  countOptionalWrappers(*MatchRes.Context, 
F.getTemplateSpecializationArgs()
+   ->get(0)
+   .getAsType()
+   .getNonReferenceType());
+  const int ArgTypeOptionalWrappersCount = countOptionalWrappers(
+  *MatchRes.Context, E.getType().getNonReferenceType());
 
   // Check if this is a constructor/assignment call for `optional` with
   // argument of type `U` such that `T` is constructible from `U`.


Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -276,12 +276,6 @@
   return nullptr;
 }
 
-/// If `Type` is a reference type, returns the type of its pointee. Otherwise,
-/// returns `Type` itself.
-QualType stripReference(QualType Type) {
-  return Type->isReferenceType() ? Type->getPointeeType() : Type;
-}
-
 /// Returns true if and only if `Type` is an optional type.
 bool isOptionalType(QualType Type) {
   if (!Type->isRecordType())
@@ -339,7 +333,7 @@
 return &ValueLoc;
   }
 
-  auto Ty = stripReference(Q);
+  auto Ty = Q.getNonReferenceType();
   auto *ValueVal = Env.createValue(Ty);
   if (ValueVal == nullptr)
 return nullptr;
@@ -493,11 +487,13 @@
   assert(F.getTemplateSpecializationArgs() != nullptr);
   assert(F.getTemplateSpecializationArgs()->size() > 0);
 
-  const int TemplateParamOptionalWrappersCount = countOptionalWrappers(
-  *MatchRes.Context,
-  stripReference(F.getTemplateSpecializationArgs()->get(0).getAsType()));
-  const int ArgTypeOptionalWrappersCount =
-  countOptionalWrappers(*MatchRes.Context, stripReference(E.getType()));
+  const int TemplateParamOptionalWrappersCount =
+  countOptionalWrappers(*MatchRes.Context, F.getTemplateSpecializationArgs()
+   ->get(0)
+   .getAsType()
+   .getNonReferenceType());
+  const int ArgTypeOptionalWrappersCount = countOptionalWrappers(
+  *MatchRes.Context, E.getType().getNonReferenceType());
 
   // Check if this is a constructor/assignment call for `optional` with
   // argument of type `U` such that `T` is constructible from `U`.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 7178ee1 - Revert "[clang] Reject flexible array member in a union in C++"

2023-05-03 Thread Mariya Podchishchaeva via cfe-commits

Author: Mariya Podchishchaeva
Date: 2023-05-03T09:25:03-04:00
New Revision: 7178ee190235bd5b6cc7c71d3ccc061d4b12656b

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

LOG: Revert "[clang] Reject flexible array member in a union in C++"

This reverts commit 22e2db6010b029ebd4c6d3d1fd30224d8b3109ef.

Broke buildbots on Windows. It seems standard headers on Windows contain
flexible array members in unions

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaInit.cpp
clang/test/Layout/aix-power-alignment-typedef.cpp
clang/test/Sema/MicrosoftExtensions.c
clang/test/Sema/init.c
clang/test/SemaCXX/flexible-array-test.cpp
clang/test/SemaCXX/gnu-flags.cpp
clang/test/SemaObjCXX/flexible-array.mm

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 416d816796514..8d0a9c96a9579 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -55,7 +55,6 @@ C++ Specific Potentially Breaking Changes
 -
 - Clang won't search for coroutine_traits in std::experimental namespace any 
more.
   Clang will only search for std::coroutine_traits for coroutines then.
-- Clang now rejects unions containing a flexible array member.
 
 ABI Changes in This Version
 ---

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 4313948515752..0ee43fb8837a1 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -267,6 +267,7 @@ def ExtraSemi : DiagGroup<"extra-semi", 
[CXX98CompatExtraSemi,
  CXX11ExtraSemi]>;
 
 def GNUFlexibleArrayInitializer : DiagGroup<"gnu-flexible-array-initializer">;
+def GNUFlexibleArrayUnionMember : DiagGroup<"gnu-flexible-array-union-member">;
 def GNUFoldingConstant : DiagGroup<"gnu-folding-constant">;
 def FormatInsufficientArgs : DiagGroup<"format-insufficient-args">;
 def FormatExtraArgs : DiagGroup<"format-extra-args">;
@@ -1136,7 +1137,7 @@ def GNU : DiagGroup<"gnu", [GNUAlignofExpression, 
GNUAnonymousStruct,
 GNUConditionalOmittedOperand, GNUDesignator,
 GNUEmptyStruct,
 VLAExtension, GNUFlexibleArrayInitializer,
-GNUFoldingConstant,
+GNUFlexibleArrayUnionMember, GNUFoldingConstant,
 GNUImaginaryConstant, GNUIncludeNext,
 GNULabelsAsValue, GNULineMarker, 
GNUNullPointerArithmetic,
 GNUOffsetofExtensions, GNUPointerArith,

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c3cb6740a5131..17585752edf8e 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6232,9 +6232,6 @@ def ext_variable_sized_type_in_struct : ExtWarn<
 
 def ext_c99_flexible_array_member : Extension<
   "flexible array members are a C99 feature">, InGroup;
-// Flexible array members in unions are not supported, but union case is still
-// present in the diagnostic so it matches TagTypeKind enum and can be emitted
-// with Diag(...) << ... << SomeTagDecl->getTagKind().
 def err_flexible_array_virtual_base : Error<
   "flexible array member %0 not allowed in "
   "%select{struct|interface|union|class|enum}1 which has a virtual base 
class">;
@@ -6257,10 +6254,15 @@ def ext_flexible_array_empty_aggregate_ms : Extension<
   InGroup;
 def err_flexible_array_union : Error<
   "flexible array member %0 in a union is not allowed">;
+def ext_flexible_array_union_ms : Extension<
+  "flexible array member %0 in a union is a Microsoft extension">,
+  InGroup;
 def ext_flexible_array_empty_aggregate_gnu : Extension<
   "flexible array member %0 in otherwise empty "
   "%select{struct|interface|union|class|enum}1 is a GNU extension">,
   InGroup;
+def ext_flexible_array_union_gnu : Extension<
+  "flexible array member %0 in a union is a GNU extension">, 
InGroup;
 
 def err_flexible_array_not_at_end : Error<
   "flexible array member %0 with type %1 is not at the end of"

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index a5bab074d08e3..4cdf2982b99d5 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -18690,7 +18690,8 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, 
Decl *EnclosingDecl,
   if (Record) {
 // Flexible array member.
 // Micr

[PATCH] D147626: [clang] Reject flexible array member in a union in C++

2023-05-03 Thread Mariya Podchishchaeva via Phabricator via cfe-commits
Fznamznon added a comment.

I've got an error from buildbot on Windows:

  C:\Program Files (x86)\Windows 
Kits\10\include\10.0.19041.0\um\winioctl.h:13404:51: error: flexible array 
member 'Lev1Depends' in a union is not allowed
  
  STORAGE_QUERY_DEPENDENT_VOLUME_LEV1_ENTRY Lev1Depends[];

It feels like Windows headers contain flexible array members in unions. We 
probably can't just always reject them on Windows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147626

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


[PATCH] D149666: [OpenMP][OMPIRBuilder] Migrate MapCombinedInfoTy from Clang to OpenMPIRBuilder

2023-05-03 Thread Akash Banerjee via Phabricator via cfe-commits
TIFitis updated this revision to Diff 519054.
TIFitis added a comment.

Changed name from llvm::OpenMPIRBuilder::MapCombinedInfoTy to 
llvm::OpenMPIRBuilder::MapInfosTy, and changed to struct instead of class.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149666

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h

Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -1445,6 +1445,49 @@
 bool separateBeginEndCalls() { return SeparateBeginEndCalls; }
   };
 
+  using MapValuesArrayTy = SmallVector;
+  using MapFlagsArrayTy = SmallVector;
+  using MapNamesArrayTy = SmallVector;
+  using MapDimArrayTy = SmallVector;
+  using MapNonContiguousArrayTy = SmallVector;
+
+  /// This structure contains combined information generated for mappable
+  /// clauses, including base pointers, pointers, sizes, map types, user-defined
+  /// mappers, and non-contiguous information.
+  struct MapInfosTy {
+struct StructNonContiguousInfo {
+  bool IsNonContiguous = false;
+  MapDimArrayTy Dims;
+  MapNonContiguousArrayTy Offsets;
+  MapNonContiguousArrayTy Counts;
+  MapNonContiguousArrayTy Strides;
+};
+MapValuesArrayTy BasePointers;
+MapValuesArrayTy Pointers;
+MapValuesArrayTy Sizes;
+MapFlagsArrayTy Types;
+MapNamesArrayTy Names;
+StructNonContiguousInfo NonContigInfo;
+
+/// Append arrays in \a CurInfo.
+void append(MapInfosTy &CurInfo) {
+  BasePointers.append(CurInfo.BasePointers.begin(),
+  CurInfo.BasePointers.end());
+  Pointers.append(CurInfo.Pointers.begin(), CurInfo.Pointers.end());
+  Sizes.append(CurInfo.Sizes.begin(), CurInfo.Sizes.end());
+  Types.append(CurInfo.Types.begin(), CurInfo.Types.end());
+  Names.append(CurInfo.Names.begin(), CurInfo.Names.end());
+  NonContigInfo.Dims.append(CurInfo.NonContigInfo.Dims.begin(),
+CurInfo.NonContigInfo.Dims.end());
+  NonContigInfo.Offsets.append(CurInfo.NonContigInfo.Offsets.begin(),
+   CurInfo.NonContigInfo.Offsets.end());
+  NonContigInfo.Counts.append(CurInfo.NonContigInfo.Counts.begin(),
+  CurInfo.NonContigInfo.Counts.end());
+  NonContigInfo.Strides.append(CurInfo.NonContigInfo.Strides.begin(),
+   CurInfo.NonContigInfo.Strides.end());
+}
+  };
+
   /// Emit the arguments to be passed to the runtime library based on the
   /// arrays of base pointers, pointers, sizes, map types, and mappers.  If
   /// ForEndCall, emit map types to be passed for the end of the region instead
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -6831,67 +6831,30 @@
 const Expr *getMapExpr() const { return MapExpr; }
   };
 
-  /// Class that associates information with a base pointer to be passed to the
-  /// runtime library.
-  class BasePointerInfo {
-/// The base pointer.
-llvm::Value *Ptr = nullptr;
-/// The base declaration that refers to this device pointer, or null if
-/// there is none.
-const ValueDecl *DevPtrDecl = nullptr;
-
-  public:
-BasePointerInfo(llvm::Value *Ptr, const ValueDecl *DevPtrDecl = nullptr)
-: Ptr(Ptr), DevPtrDecl(DevPtrDecl) {}
-llvm::Value *operator*() const { return Ptr; }
-const ValueDecl *getDevicePtrDecl() const { return DevPtrDecl; }
-void setDevicePtrDecl(const ValueDecl *D) { DevPtrDecl = D; }
-  };
-
+  using MapBaseValuesArrayTy = llvm::OpenMPIRBuilder::MapValuesArrayTy;
+  using MapValuesArrayTy = llvm::OpenMPIRBuilder::MapValuesArrayTy;
+  using MapFlagsArrayTy = llvm::OpenMPIRBuilder::MapFlagsArrayTy;
+  using MapDimArrayTy = llvm::OpenMPIRBuilder::MapDimArrayTy;
+  using MapNonContiguousArrayTy =
+  llvm::OpenMPIRBuilder::MapNonContiguousArrayTy;
   using MapExprsArrayTy = SmallVector;
-  using MapBaseValuesArrayTy = SmallVector;
-  using MapValuesArrayTy = SmallVector;
-  using MapFlagsArrayTy = SmallVector;
-  using MapMappersArrayTy = SmallVector;
-  using MapDimArrayTy = SmallVector;
-  using MapNonContiguousArrayTy = SmallVector;
+  using MapValueDeclsArrayTy = SmallVector;
 
   /// This structure contains combined information generated for mappable
   /// clauses, including base pointers, pointers, sizes, map types, user-defined
   /// mappers, and non-contiguous information.
-  struct MapCombinedInfoTy {
-struct StructNonContiguousInfo {
-  bool IsNonContiguous = false;
-  MapDimArrayTy Dims;
-  MapNonContiguousArrayTy Offsets;
-  MapNonContiguousArrayT

[PATCH] D149573: [Clang][C++23] Implement core language changes from P1467R9 extended floating-point types and standard names and introduce Bfloat16 arithmetic type.

2023-05-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

Others definitely need to review this, but i'm about happy with it.




Comment at: clang/include/clang/Basic/LangOptions.def:468
 
+LANGOPT(ExperimentalNewItaniumMangling, 1, 0, "experimental new Itanium 
mangling")
+

codemzs wrote:
> erichkeane wrote:
> > I'm not sure I like the implications or maintainability of this.  This is 
> > something that is likely to get out of hand too quickly.  We probably need 
> > to just figure out what the itanium mangling is GOING to be (perhaps 
> > @rjmccall has some  insight?), and just implement that.
> > 
> > Also, this isn't really a 'Language Option', so much as a codegen option.
> Thank you for your valuable feedback. Based on your suggestion and 
> considering that GCC has already implemented the mangling as `DF16b`, I agree 
> that if we have reasonable confidence in the direction the mangling will 
> take, it would be better to implement it directly instead of guarding it with 
> an experimental flag. Therefore, I will be removing the flag. I initially 
> added the flag since @tahonermann was on board with implementing the bf16 
> arithmetic type, assuming the mangling was finalized.
> 
> However, I understand your concerns and would appreciate further input from 
> both @tahonermann and @rjmccall on this matter. My intention is to avoid 
> stalling the progress of this change due to mangling finalization.
> 
> I'm open to further discussion and collaboration to ensure we make the right 
> decision for our project while maintaining the momentum of the review process.
Thanks, I think this is a good direction for this patch to take.



Comment at: clang/lib/AST/ASTContext.cpp:135
+CXX23FloatingPointConversionRankMap = {
+{{{FloatingRankCompareResult::FRCR_Equal,
+   FloatingRankCompareResult::FRCR_Unordered,

Just a suggestion here, feel free to toss it, but I suspect some comments to 
explain the 'grid' a bit are helpful.

Basically, split it up into the 5 groups, and comment which is which.



Comment at: clang/test/CodeGenCXX/cxx2b-fp-ext-std-names-p1467r9.cpp:5
+// CHECK-LABEL: @_Z1fDF16b(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[V_ADDR:%.*]] = alloca bfloat, align 2

codemzs wrote:
> erichkeane wrote:
> > `entry` isn't a stable name here, so you shouldn't use this label.
> I have removed the "entry" label as you suggested.
> 
> Just to let you know, this label was automatically generated by the script 
> `utils/update_cc_test_checks.py`, which is used to update the expected output 
> of the test cases. I noticed that this label is present in other tests in the 
> codebase as well.
> 
> Out of curiosity, why is this label not considered stable?
Urgh, I kinda hate that script, it always gives such difficult to read tests...

My understanding is that when compiled with 'strip symbol names', no names are 
included that aren't required for ABI purposes.  So labels all switch to %0, 
param names are removed entirely/etc.

Though, I guess I haven't seen that bot complain in a while, so perhaps it 
disappeared...



Comment at: clang/test/Sema/cxx2b-fp-ext-std-names-p1467r9.cpp:9
+//CHECK:  |-VarDecl {{.*}} f16_val_6 '_Float16' cinit
+//CHECK-NEXT: | `-FloatingLiteral {{.*}} '_Float16' 1.00e+00
+_Float16 f16_val_7 = static_cast<_Float16>(1.0bf16); // expected-error 
{{static_cast from 'BF16' to '_Float16' is not allowed}}

codemzs wrote:
> erichkeane wrote:
> > How we emit the result of the floats is inconsistent, at least in IR, so 
> > I'm not sure how stable it'll be here.
> > 
> > Also, don't use the `|-` and `| \`-`` in the check-lines, those get messy 
> > when something changes.  
> I've observed similar floating-point representations in other tests, but I 
> acknowledge your concerns about potential inconsistencies and stability. To 
> address this, would replacing them with `{{.*}}` be a suitable solution? I'm 
> also considering dividing the test into two parts: one for error checks in 
> the current location and another for AST checks under `test/AST`. Would this 
> resolve your concerns about the use of `|-` and `| ` characters? Furthermore, 
> I'd like to clarify whether your comment about avoiding `|-` applies 
> specifically to tests in the `test/Sema` directory or more generally. My 
> intention is to seek clarification and ensure the test code adheres to best 
> practices.
I looked it up too, and I don't see this being a problem in our AST output as 
it is with our LLVM output, so perhaps I'm incorrect here.  Feel free to leave 
them alone as is...

As far as the `|-` characters: I'd suggest just removing them entirely.  Just 
left-align so it'd be :

```
VarDecl {{.*}} f16_val_6 '_Float16' cinit
FloatingLiteral {{.*}} '_Float16' 1.00e+00
```

It'll still pass, and not be sensitive to the AST-dump's way of outputting.


CHANGES SINCE LAST ACTION
  https://re

[PATCH] D149666: [OpenMP][OMPIRBuilder] Migrate MapCombinedInfoTy from Clang to OpenMPIRBuilder

2023-05-03 Thread Akash Banerjee via Phabricator via cfe-commits
TIFitis marked 2 inline comments as done.
TIFitis added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:6848
+  class MapCombinedInfoTy : public llvm::OpenMPIRBuilder::MapCombinedInfoTy {
+  public:
 MapExprsArrayTy Exprs;

jdoerfert wrote:
> TIFitis wrote:
> > jdoerfert wrote:
> > > Not sure why you made it a class with public, but I guess it doesn't 
> > > matter.
> > > Do we really want to use the same name though? I would add "Base" or sth 
> > > to the llvm class.
> > The clang code, and eventually the OMPIRBuilder code accesses the members 
> > directly so they need to be public.
> > 
> > I have kept it the same name in similar fashion to the TargetDataInfo type 
> > that also got migrated. From OMPBuilder's perspective the type is 
> > sufficient and when using it from MLIR I don't think we would need the 
> > extra data structures that clang uses.
> > 
> > Let me know what you think, I can change it to a different name as you 
> > suggested.
> Two structures with the same name is not a good idea. Rename one.
> 
> struct A === class A + public,
> so why did you move from struct A to class A + public?
I have changed the OMPIRBuilder type name to MapInfosTy. Is that okay?

I see what you mean, I have changed it to struct.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:6872
   NonContigInfo.Strides.append(CurInfo.NonContigInfo.Strides.begin(),
 CurInfo.NonContigInfo.Strides.end());
 }

jdoerfert wrote:
> TIFitis wrote:
> > jdoerfert wrote:
> > > We should use the base append function, no?
> > The base append function is missing append for the members we introduced 
> > here like `Exprs` and `Mappers`.
> I understand that. I did not say the base function is sufficient, I said we 
> should use it.
> 
Updated to use the base append method.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149666

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


[PATCH] D149694: [Clang] Update warning on some designator initializer cases involving unions

2023-05-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

I think this looks right, and is consistent with guidance from the bug report.


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

https://reviews.llvm.org/D149694

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


[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

2023-05-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

Missing context in the diff, but the changes since commit are all sensible to 
me.  If this fixes everything we know about, I'm OK with it.




Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:344
  Innermost->asArray(), Final);
-
-  const Decl *CurDecl = ND;
+  CurDecl = Response::UseNextDecl(ND).NextDecl;
+  }

alexander-shaposhnikov wrote:
> this bit is important.
Oh yeah, that makes a lot of sense.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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


[PATCH] D133863: [RISCV] Add MC support of RISCV zcmt Extension

2023-05-03 Thread Xinlong Wu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9f0d725744aa: [RISCV] Add MC support of RISCV zcmt Extension 
(authored by VincentWu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133863

Files:
  clang/test/Preprocessor/riscv-target-features.c
  llvm/docs/RISCVUsage.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/RISCVFeatures.td
  llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
  llvm/lib/Target/RISCV/RISCVSchedRocket.td
  llvm/lib/Target/RISCV/RISCVSchedSiFive7.td
  llvm/lib/Target/RISCV/RISCVSystemOperands.td
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/rv32zcmt-invalid.s
  llvm/test/MC/RISCV/rv32zcmt-valid.s
  llvm/test/MC/RISCV/rvzcmt-user-csr-name.s

Index: llvm/test/MC/RISCV/rvzcmt-user-csr-name.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rvzcmt-user-csr-name.s
@@ -0,0 +1,29 @@
+# RUN: llvm-mc %s -triple=riscv32 -riscv-no-aliases -mattr=+experimental-zcmt -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-INST,CHECK-ENC %s
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+experimental-zcmt < %s \
+# RUN: | llvm-objdump -d --mattr=+experimental-zcmt - \
+# RUN: | FileCheck -check-prefix=CHECK-INST-ALIAS %s
+#
+# RUN: llvm-mc %s -triple=riscv64 -riscv-no-aliases -mattr=+experimental-zcmt -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-INST,CHECK-ENC %s
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+experimental-zcmt < %s \
+# RUN: | llvm-objdump -d --mattr=+experimental-zcmt - \
+# RUN: | FileCheck -check-prefix=CHECK-INST-ALIAS %s
+
+##
+# Jump Vector Table CSR
+##
+
+# jvt
+# name
+# CHECK-INST: csrrs t1, jvt, zero
+# CHECK-ENC:  encoding: [0x73,0x23,0x70,0x01]
+# CHECK-INST-ALIAS: csrr t1, jvt
+# uimm12
+# CHECK-INST: csrrs t2, jvt, zero
+# CHECK-ENC:  encoding: [0xf3,0x23,0x70,0x01]
+# CHECK-INST-ALIAS: csrr t2, jvt
+# name
+csrrs t1, jvt, zero
+# uimm12
+csrrs t2, 0x017, zero
Index: llvm/test/MC/RISCV/rv32zcmt-valid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rv32zcmt-valid.s
@@ -0,0 +1,39 @@
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+experimental-zcmt\
+# RUN:  -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv32 -mattr=+experimental-zcmt\
+# RUN:  -mattr=m < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zcmt\
+# RUN:  -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefixes=CHECK-OBJ,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+experimental-zcmt\
+# RUN:  -riscv-no-aliases -show-encoding \
+# RUN: | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
+# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+experimental-zcmt\
+# RUN:  -mattr=m < %s \
+# RUN: | llvm-objdump --mattr=+experimental-zcmt\
+# RUN:  -M no-aliases -d -r - \
+# RUN: | FileCheck --check-prefixes=CHECK-OBJ,CHECK-ASM-AND-OBJ %s
+#
+# RUN: not llvm-mc -triple riscv32 \
+# RUN: -riscv-no-aliases -show-encoding < %s 2>&1 \
+# RUN: | FileCheck -check-prefixes=CHECK-NO-EXT %s
+# RUN: not llvm-mc -triple riscv64 \
+# RUN: -riscv-no-aliases -show-encoding < %s 2>&1 \
+# RUN: | FileCheck -check-prefixes=CHECK-NO-EXT %s
+
+# CHECK-ASM-AND-OBJ: cm.jt 1
+# CHECK-ASM: encoding: [0x06,0xa0]
+# CHECK-NO-EXT: error: instruction requires the following: 'Zcmt' (table jump instuctions for code-size reduction){{$}}
+cm.jt 1
+
+# CHECK-ASM: cm.jalt 1
+# CHECK-OBJ: cm.jt 1
+# CHECK-ASM: encoding: [0x06,0xa0]
+# CHECK-NO-EXT: error: instruction requires the following: 'Zcmt' (table jump instuctions for code-size reduction){{$}}
+cm.jalt 1
+
+# CHECK-ASM-AND-OBJ: cm.jalt 32
+# CHECK-ASM: encoding: [0x82,0xa0]
+# CHECK-NO-EXT: error: instruction requires the following: 'Zcmt' (table jump instuctions for code-size reduction){{$}}
+cm.jalt 32
Index: llvm/test/MC/RISCV/rv32zcmt-invalid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rv32zcmt-invalid.s
@@ -0,0 +1,10 @@
+# RUN: not llvm-mc -triple=riscv32 -mattr=+experimental-zcmt -riscv-no-aliases -show-encoding < %s 2>&1 \
+# RUN: | FileCheck -check-prefixes=CHECK-ERROR %s
+# RUN: not llvm-mc -triple=riscv64 -mattr=+experimental-zcmt -riscv-no-aliases -show-encoding < %s 2>&1 \
+# RUN: | FileCheck -check-prefixes=CHECK-ERROR %s
+
+# CHECK-ERROR: error: immediate must be an integer in the range [0, 31]
+cm.jt 64
+
+# CHECK-ERROR: error: immediate must be an integer in the range [0, 255]
+cm.jalt 256
Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-

[clang] 9f0d725 - [RISCV] Add MC support of RISCV zcmt Extension

2023-05-03 Thread via cfe-commits

Author: WuXinlong
Date: 2023-05-03T22:06:37+08:00
New Revision: 9f0d725744aa2f36395fdccd85a2f21b960ff661

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

LOG: [RISCV] Add MC support of RISCV zcmt Extension

This patch add the instructions of zcmt extension.
[[ https://github.com/riscv/riscv-code-size-reduction/releases/tag/v1.0.0-RC5.7 
| spac is here ]]
Which includes two instructions (cm.jt&cm.jalt) and a CSR Reg JVT

co-author: @Scott Egerton

Reviewed By: kito-cheng, craig.topper

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

Added: 
llvm/test/MC/RISCV/rv32zcmt-invalid.s
llvm/test/MC/RISCV/rv32zcmt-valid.s
llvm/test/MC/RISCV/rvzcmt-user-csr-name.s

Modified: 
clang/test/Preprocessor/riscv-target-features.c
llvm/docs/RISCVUsage.rst
llvm/lib/Support/RISCVISAInfo.cpp
llvm/lib/Target/RISCV/RISCVFeatures.td
llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
llvm/lib/Target/RISCV/RISCVSchedRocket.td
llvm/lib/Target/RISCV/RISCVSchedSiFive7.td
llvm/lib/Target/RISCV/RISCVSystemOperands.td
llvm/test/CodeGen/RISCV/attributes.ll
llvm/test/MC/RISCV/attribute-arch.s

Removed: 




diff  --git a/clang/test/Preprocessor/riscv-target-features.c 
b/clang/test/Preprocessor/riscv-target-features.c
index 59830012f0eb2..2dadbe2c420db 100644
--- a/clang/test/Preprocessor/riscv-target-features.c
+++ b/clang/test/Preprocessor/riscv-target-features.c
@@ -48,6 +48,7 @@
 // CHECK-NOT: __riscv_zcb {{.*$}}
 // CHECK-NOT: __riscv_zcd {{.*$}}
 // CHECK-NOT: __riscv_zcf {{.*$}}
+// CHECK-NOT: __riscv_zcmt {{.*$}}
 // CHECK-NOT: __riscv_h {{.*$}}
 // CHECK-NOT: __riscv_zvbb {{.*$}}
 // CHECK-NOT: __riscv_zvbc {{.*$}}
@@ -511,6 +512,13 @@
 // RUN: -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-ZCF-EXT %s
 // CHECK-ZCF-EXT: __riscv_zcf 100{{$}}
 
+// RUN: %clang -target riscv32 -march=rv32izcmt1p0 
-menable-experimental-extensions \
+// RUN: -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-ZCMT-EXT %s
+// RUN: %clang -target riscv64 -march=rv64izcmt1p0 
-menable-experimental-extensions \
+// RUN: -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-ZCMT-EXT %s
+// CHECK-ZCMT-EXT: __riscv_zca 100{{$}}
+// CHECK-ZCMT-EXT: __riscv_zcmt 100{{$}}
+
 // RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32izicsr2p0 -x c -E 
-dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-ZICSR-EXT %s
 // RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64izicsr2p0 -x c -E 
-dM %s \

diff  --git a/llvm/docs/RISCVUsage.rst b/llvm/docs/RISCVUsage.rst
index 5ec8be1a0fafa..c76f45c7c3811 100644
--- a/llvm/docs/RISCVUsage.rst
+++ b/llvm/docs/RISCVUsage.rst
@@ -195,6 +195,9 @@ The primary goal of experimental support is to assist in 
the process of ratifica
 ``experimental-zcf``
   LLVM implements the `1.0.1 draft specification 
`__.
 
+``experimental-zcmt``
+  LLVM implements the `1.0.1 draft specification 
`_.
+
 ``experimental-zfa``
   LLVM implements the `0.2 draft specification 
`__.
 

diff  --git a/llvm/lib/Support/RISCVISAInfo.cpp 
b/llvm/lib/Support/RISCVISAInfo.cpp
index 2014dfdb67ec2..d25dc2b0381d1 100644
--- a/llvm/lib/Support/RISCVISAInfo.cpp
+++ b/llvm/lib/Support/RISCVISAInfo.cpp
@@ -144,6 +144,7 @@ static const RISCVSupportedExtension 
SupportedExperimentalExtensions[] = {
 {"zcb", RISCVExtensionVersion{1, 0}},
 {"zcd", RISCVExtensionVersion{1, 0}},
 {"zcf", RISCVExtensionVersion{1, 0}},
+{"zcmt", RISCVExtensionVersion{1, 0}},
 {"zfa", RISCVExtensionVersion{0, 2}},
 {"zicond", RISCVExtensionVersion{1, 0}},
 {"zvfh", RISCVExtensionVersion{0, 1}},
@@ -851,6 +852,7 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool 
EnableExperimentalExtension,
 }
 
 Error RISCVISAInfo::checkDependency() {
+  bool HasC = Exts.count("c") != 0;
   bool HasD = Exts.count("d") != 0;
   bool HasF = Exts.count("f") != 0;
   bool HasZfinx = Exts.count("zfinx") != 0;
@@ -859,6 +861,8 @@ Error RISCVISAInfo::checkDependency() {
   bool HasZve32f = Exts.count("zve32f") != 0;
   bool HasZve64d = Exts.count("zve64d") != 0;
   bool HasZvl = MinVLen != 0;
+  bool HasZcmt = Exts.count("zcmt") != 0;
+  bool HasZcd = Exts.count("zcd") != 0;
 
   if (HasF && HasZfinx)
 return createStringError(errc::invalid_argument,
@@ -908,6 +912,16 @@ Error RISCVISAInfo::checkDependency() {
 errc::invalid_argument,
 "'zvknhb' requires 'v' or 'zve64*' extension to also be specified");
 
+  if (HasZcmt && HasD && HasC)
+return createStringError(
+errc::invalid_argument,
+"'zcmt' is 

[PATCH] D147875: [clang][Diagnostics] Show line numbers when printing code snippets

2023-05-03 Thread Piotr Zegar via Phabricator via cfe-commits
PiotrZSL added a comment.

Clang-tidy related changes looks fine.


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

https://reviews.llvm.org/D147875

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


[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

2023-05-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

Hmm... So I don't see us being able to change how we create `FunctionDecl`s, 
the way we are doing it with a 'build up to the final thing' is about all we 
can do.  However, it should never escape its 'creation' functions without its 
type set.

IF I were to design this from scratch, I probably would have designed a 
`FunctionDeclBuilder` type thing that modeled a `FunctionDecl`, but asserted 
when being `finalized` (to get the `FunctionDecl` from it) if it wasn't 
complete.  However, we don't really have anything that models that in this 
codebase, and I'm not sure we really want to add something like that...

I don't recall the case of a Null type being valid for functions (perhaps Aaron 
does?  Or perhaps its an error condition?).  But otherwise, I would expect 
`FunctionDecl` to have a type as soon as it escapes the function responsible 
for 'making' it (that is, the one that calls the ::Create and is filling in 
everything).  If that patch added a path that doesn't set the return type, 
perhaps we should just fix that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149733

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


[PATCH] D149110: [HIP] Detect HIP for Ubuntu, Mint, Gentoo, etc.

2023-05-03 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl accepted this revision.
yaxunl added a comment.

LGTM. Thanks. I can help commit this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149110

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


[PATCH] D149460: [analyzer] ArrayBoundCheckerV2: suppress false positives from ctype macros

2023-05-03 Thread Balázs Benics via Phabricator via cfe-commits
steakhal accepted this revision.
steakhal 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/D149460/new/

https://reviews.llvm.org/D149460

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


[PATCH] D149259: [analyzer][NFC] Use std::optional instead of custom "empty" state

2023-05-03 Thread Donát Nagy via Phabricator via cfe-commits
donat.nagy added a comment.

@steakhal Could you review this change when you have some time for it?

I'm planning to stabilize and de-alpha this checker with a series of 
incremental changes; and it'd be good have this commit and the unrelated tweak 
https://reviews.llvm.org/D149460 merged before I start working on the next 
steps. My plans for the near future:

- Replace the prototype-level error reporting with real, detailed, useful 
messages. Implementing this may require some code reorganization to preserve 
and pass around the details information.
- Perform an early return when there is no real indexing (there are no 
ElementRegion layers in the load operation).
- Reconsider the handling of multidimensional arrays: the current code (in fact 
the function `computeOffset` that I'm refactoring there) says that `arr[0][10]` 
is a valid way to index `int arr[4][5]` which not what the users would expect 
(in this simple case, even clang-tidy knows that "array index 10 is past the 
end of the array", but ArrayBoundsV2 implements complex logic to compare the 
index with the whole memory region allocated for the array). I'm leaning 
towards switching to "normal" behavior here.
- Report situations like `struct foo {int field;} arr[5]; arr[10].field = 123;` 
-- the current implementation doesn't "see" these because this is writing a 
FieldRegion (which happens to be inside an ElementRegion, but the checker 
doesn't look for that that).

I'm also open to feedback/suggestions connected to these plans. It's likely 
that the code added by the current NFC commit will be rewritten by the followup 
changes, but I think it's still useful to merge it, because it documents the 
switch from the old implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149259

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


[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

2023-05-03 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

In D149733#4315360 , @erichkeane 
wrote:

> I don't recall the case of a Null type being valid for functions (perhaps 
> Aaron does?  Or perhaps its an error condition?).  But otherwise, I would 
> expect `FunctionDecl` to have a type as soon as it escapes the function 
> responsible for 'making' it (that is, the one that calls the ::Create and is 
> filling in everything).

I guess that's where the issue comes from. In theory 
`Parser::ParseLambdaExpressionAfterIntroducer` calls both 
`Sema::ActOnLambdaExpressionAfterIntroducer` (which creates incomplete method 
decl) and `Sema::ActOnStartOfLambdaDefinition` (which populates type info), so 
once we exit the parse function the methoddecl should have a valid type.
But the way clang implements code completion breaks that guarantee, as parser 
can hand out those semi-complete declarations to code completion consumers if 
code completion point is reached while parsing the tokens in between.
More importantly I don't know if this "variant" holds in all the places that 
creates a function decl (even in this example we can actually take an early 
exit 

 but we at least mark the decl invalid before doing so).

> If that patch added a path that doesn't set the return type, perhaps we 
> should just fix that?

it's not just the return type, it's the full type for the function, see here 

 (line 919 of clang/lib/Sema/SemaLambda.cpp in case the link doesn't work).
due to above mentioned reasons, i don't think there's anything wrong with that 
patch in particular (either it's fine or we've a bunch more cases that are 
wrong and hard to fix due to the first reason you mentioned), it just separated 
introduction and finalisation of a decl. 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDeclCXX.cpp#L14469
 is another example (and we've got bunch more).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149733

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


[PATCH] D143624: Inlining: Run the legacy AlwaysInliner before the regular inliner.

2023-05-03 Thread Dave Green via Phabricator via cfe-commits
dmgreen added a comment.

It looks like there is quite a lot more optimization that happens to the 
function being always-inlined (__SSAT) before this change. Through multiple 
rounds of instcombine, almost to the end of the pass pipeline. The new version 
runs a lot less before inlining, only running instcombine->simplifycfg and not 
seeing another instcombine to clean up the results. Is that because the 
AlwaysInlinePass is a module pass and it now only runs the passes up to that 
point?

It does look like there might be a chance to undo the transform, as opposed to 
prevent the transform that blocks it. Something like 
https://alive2.llvm.org/ce/z/qHtPqz seems to happen at at least one point. 
Might that be more preferable?

There are some other changes I#m seeing though, from the same function inlined 
into different routine. This one for example seems to be not longer applying to 
canonicalizeClampLike, so the ssat doesn't get created. 
https://godbolt.org/z/qMW44qfz4. That doesn't seem to be easily undoable 
without knowing the value is positive though 
https://alive2.llvm.org/ce/z/v9YdaK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143624

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


[PATCH] D143624: Inlining: Run the legacy AlwaysInliner before the regular inliner.

2023-05-03 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment.

In D143624#4315468 , @dmgreen wrote:

> It looks like there is quite a lot more optimization that happens to the 
> function being always-inlined (__SSAT) before this change. Through multiple 
> rounds of instcombine, almost to the end of the pass pipeline. The new 
> version runs a lot less before inlining, only running 
> instcombine->simplifycfg and not seeing another instcombine to clean up the 
> results. Is that because the AlwaysInlinePass is a module pass and it now 
> only runs the passes up to that point?

Yes, which is why I personally think this change isn't a good idea. This 
essentially breaks our invariant that functions get simplified before they are 
inlined. This significantly alters the way alwaysinline functions will be 
optimized relative to normally inlined functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143624

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


[PATCH] D143624: Inlining: Run the legacy AlwaysInliner before the regular inliner.

2023-05-03 Thread Mircea Trofin via Phabricator via cfe-commits
mtrofin added a comment.

In D143624#4315508 , @nikic wrote:

> In D143624#4315468 , @dmgreen wrote:
>
>> It looks like there is quite a lot more optimization that happens to the 
>> function being always-inlined (__SSAT) before this change. Through multiple 
>> rounds of instcombine, almost to the end of the pass pipeline. The new 
>> version runs a lot less before inlining, only running 
>> instcombine->simplifycfg and not seeing another instcombine to clean up the 
>> results. Is that because the AlwaysInlinePass is a module pass and it now 
>> only runs the passes up to that point?
>
> Yes, which is why I personally think this change isn't a good idea. This 
> essentially breaks our invariant that functions get simplified before they 
> are inlined. This significantly alters the way alwaysinline functions will be 
> optimized relative to normally inlined functions.

(Nitpicking just on the invariant part) Not sure if that's always the 
invariant, because we could be inlining a call site in a SCC where both caller 
and callee are in that same SCC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143624

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


[PATCH] D145441: [AMDGPU] Define data layout entries for buffers

2023-05-03 Thread Krzysztof Drewniak 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 rGf9c1ede2543b: [AMDGPU] Define data layout entries for 
buffers (authored by krzysz00).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145441

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/test/CodeGen/target-data.c
  clang/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl
  llvm/docs/AMDGPUUsage.rst
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/Target/AMDGPU/AMDGPU.h
  llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.cpp
  llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
  llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.f32-no-rtn.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.f32-rtn.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.f64.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.v2f16-no-rtn.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.v2f16-rtn.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-non-integral-address-spaces.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.atomic.dim.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.dim.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.2d.d16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.2d.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.2darraymsaa.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.3d.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.d.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.g16.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.g16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.store.2d.d16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.image.atomic.dim.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.add.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.cmpswap.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.fadd-with-ret.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.fadd.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.load.format.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.load.format.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.store.format.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.store.format.f32.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.store.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.load.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.store.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.store.i8.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.store.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.add.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.cmpswap.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.fadd-with-ret.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.fadd.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.load.format.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.load.format.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.load.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.store.format.f16.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.store.format.f32.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.store.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.tbuffer.load.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.tbuffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.image.load.1d.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.image.sample.1d.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.raw.buffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.struct.buffer.load.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.struct.buffer.store.ll
  llvm/test/CodeGen/AMDGPU/addrspacecast-captured.ll
  llvm/test/CodeGen/AMDGPU/annotate-kernel-features-hsa.ll
  llvm/test/CodeGen/AMDGPU/buffer-atomic-fadd.f32-no-rtn.ll
  llvm/test/CodeGen/AMDGPU/buffer-atomic-fadd.f32-rtn.ll
  llvm/test/CodeGen/AMDGPU/buffer-atomic-fadd.f64.ll
  llvm/test/CodeGen/AMDGPU/buffer-atomic-fadd.v2f16-no-r

[PATCH] D148381: [WIP][Clang] Add element_count attribute

2023-05-03 Thread Kees Cook via Phabricator via cfe-commits
kees added a comment.

This is great! It passes my own testing for the sanitizer too. I'm looking 
forward to __bdos support. :)

FWIW, similar progress is being made on the GCC side:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108896


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148381

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


[PATCH] D149737: [clang][ExtractAPI] Add semicolon to function declaration fragments

2023-05-03 Thread NagaChaitanya Vellanki via Phabricator via cfe-commits
chaitanyav added a comment.

@dang @ributzka I missed adding semicolons to function declaration fragments in 
this commit 
https://github.com/llvm/llvm-project/commit/afce10c5b60fada1db369d3770f4389da7ef30ef
 . Please review this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149737

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


[PATCH] D148822: [clang][BFloat] Avoid redefining bfloat16_t in arm_neon.h

2023-05-03 Thread Dimitry Andric 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 rGdb492316399a: [clang][BFloat] Avoid redefining bfloat16_t in 
arm_neon.h (authored by dim).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148822

Files:
  clang/utils/TableGen/NeonEmitter.cpp
  clang/utils/TableGen/SveEmitter.cpp


Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -1103,7 +1103,6 @@
   OS << "typedef __SVBFloat16_t svbfloat16_t;\n";
 
   OS << "#include \n";
-  OS << "typedef __bf16 bfloat16_t;\n";
 
   OS << "typedef __SVFloat32_t svfloat32_t;\n";
   OS << "typedef __SVFloat64_t svfloat64_t;\n";
Index: clang/utils/TableGen/NeonEmitter.cpp
===
--- clang/utils/TableGen/NeonEmitter.cpp
+++ clang/utils/TableGen/NeonEmitter.cpp
@@ -2353,7 +2353,6 @@
   OS << "#include \n\n";
 
   OS << "#include \n";
-  OS << "typedef __bf16 bfloat16_t;\n";
 
   // Emit NEON-specific scalar typedefs.
   OS << "typedef float float32_t;\n";


Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -1103,7 +1103,6 @@
   OS << "typedef __SVBFloat16_t svbfloat16_t;\n";
 
   OS << "#include \n";
-  OS << "typedef __bf16 bfloat16_t;\n";
 
   OS << "typedef __SVFloat32_t svfloat32_t;\n";
   OS << "typedef __SVFloat64_t svfloat64_t;\n";
Index: clang/utils/TableGen/NeonEmitter.cpp
===
--- clang/utils/TableGen/NeonEmitter.cpp
+++ clang/utils/TableGen/NeonEmitter.cpp
@@ -2353,7 +2353,6 @@
   OS << "#include \n\n";
 
   OS << "#include \n";
-  OS << "typedef __bf16 bfloat16_t;\n";
 
   // Emit NEON-specific scalar typedefs.
   OS << "typedef float float32_t;\n";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] db49231 - [clang][BFloat] Avoid redefining bfloat16_t in arm_neon.h

2023-05-03 Thread Dimitry Andric via cfe-commits

Author: Dimitry Andric
Date: 2023-05-03T17:54:58+02:00
New Revision: db492316399a0edc26788265c7fce78c63a0f838

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

LOG: [clang][BFloat] Avoid redefining bfloat16_t in arm_neon.h

As of https://reviews.llvm.org/D79708, clang-tblgen generates `arm_neon.h`,
`arm_sve.h` and `arm_bf16.h`, and all those generated files will contain a
typedef of `bfloat16_t`. However, `arm_neon.h` and `arm_sve.h` include
`arm_bf16.h` immediately before their own typedef:

#include 
typedef __bf16 bfloat16_t;

With a recent version of clang (I used 16.0.1) this results in warnings:

/usr/lib/clang/16/include/arm_neon.h:38:16: error: redefinition of typedef 
'bfloat16_t' is a C11 feature [-Werror,-Wtypedef-redefinition]

Since `arm_bf16.h` is very likely supposed to be the one true place where
`bfloat16_t` is defined, I propose to delete the duplicate typedefs from the
generated `arm_neon.h` and `arm_sve.h`.

Reviewed By: sdesmalen, simonbutcher

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

Added: 


Modified: 
clang/utils/TableGen/NeonEmitter.cpp
clang/utils/TableGen/SveEmitter.cpp

Removed: 




diff  --git a/clang/utils/TableGen/NeonEmitter.cpp 
b/clang/utils/TableGen/NeonEmitter.cpp
index 51bb774c6da99..6ef5790731a6b 100644
--- a/clang/utils/TableGen/NeonEmitter.cpp
+++ b/clang/utils/TableGen/NeonEmitter.cpp
@@ -2353,7 +2353,6 @@ void NeonEmitter::run(raw_ostream &OS) {
   OS << "#include \n\n";
 
   OS << "#include \n";
-  OS << "typedef __bf16 bfloat16_t;\n";
 
   // Emit NEON-specific scalar typedefs.
   OS << "typedef float float32_t;\n";

diff  --git a/clang/utils/TableGen/SveEmitter.cpp 
b/clang/utils/TableGen/SveEmitter.cpp
index bc50cbad4b541..d5d3f5fe558a8 100644
--- a/clang/utils/TableGen/SveEmitter.cpp
+++ b/clang/utils/TableGen/SveEmitter.cpp
@@ -1103,7 +1103,6 @@ void SVEEmitter::createHeader(raw_ostream &OS) {
   OS << "typedef __SVBFloat16_t svbfloat16_t;\n";
 
   OS << "#include \n";
-  OS << "typedef __bf16 bfloat16_t;\n";
 
   OS << "typedef __SVFloat32_t svfloat32_t;\n";
   OS << "typedef __SVFloat64_t svfloat64_t;\n";



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


[PATCH] D148767: Restore CodeGen/LowLevelType from `Support`

2023-05-03 Thread Job Noorman via Phabricator via cfe-commits
jobnoorman added a comment.

Hi, this seems to have broken my bolt+debug+shared build. I don't think there 
are build bots for this configuration but you can reproduce it like this:

  cmake -G Ninja -DCMAKE_BUILD_TYPE="Debug" \
 -DLLVM_ENABLE_PROJECTS="clang;lld;bolt" \
 -DBUILD_SHARED_LIBS=True \
 -DLLVM_BUILD_TESTS=True \
 -DLLVM_CCACHE_BUILD=ON \
 -DLLVM_ENABLE_LLD=True \
 -DLLVM_TARGETS_TO_BUILD="X86;RISCV;AArch64" \
 -DLLVM_APPEND_VC_REV=False ../llvm

And this is the linker error:

  ld.lld: error: undefined symbol: llvm::LLT::print(llvm::raw_ostream&) const
  >>> referenced by LowLevelType.h:269 
(/.../llvm-project/llvm/include/llvm/CodeGen/LowLevelType.h:269)
  >>>   
tools/bolt/lib/Passes/CMakeFiles/LLVMBOLTPasses.dir/AsmDump.cpp.o:(llvm::LLT::dump()
 const)
  collect2: error: ld returned 1 exit status


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148767

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


[PATCH] D149259: [analyzer][NFC] Use std::optional instead of custom "empty" state

2023-05-03 Thread Balázs Benics via Phabricator via cfe-commits
steakhal added a comment.

In D149259#4315393 , @donat.nagy 
wrote:

> @steakhal Could you review this change when you have some time for it?

I didn't forget about this one. It's in progress, but it's difficult to squeeze 
this in. The ETA is probably tomorrow.

> I'm planning to stabilize and de-alpha this checker with a series of 
> incremental changes; and it'd be good have this commit and the unrelated 
> tweak https://reviews.llvm.org/D149460 merged before I start working on the 
> next steps.

Good luck with moving this out. It's gonna be bumpy.

> My plans for the near future:
>
> - Replace the prototype-level error reporting with real, detailed, useful 
> messages. Implementing this may require some code reorganization to preserve 
> and pass around the details information.

IMO this is the most important step, yes.

> - Perform an early return when there is no real indexing (there are no 
> ElementRegion layers in the load operation).
> - Reconsider the handling of multidimensional arrays: the current code (in 
> fact the function `computeOffset` that I'm refactoring there) says that 
> `arr[0][10]` is a valid way to index `int arr[4][5]` which not what the users 
> would expect (in this simple case, even clang-tidy knows that "array index 10 
> is past the end of the array", but ArrayBoundsV2 implements complex logic to 
> compare the index with the whole memory region allocated for the array). I'm 
> leaning towards switching to "normal" behavior here.

In fact, I believe it's UB to index an `int arr[4][5]` multi-dimensional array 
like `arr[0][10]`.

> - Report situations like `struct foo {int field;} arr[5]; arr[10].field = 
> 123;` -- the current implementation doesn't "see" these because this is 
> writing a FieldRegion (which happens to be inside an ElementRegion, but the 
> checker doesn't look for that that).

+1

> I'm also open to feedback/suggestions connected to these plans. It's likely 
> that the code added by the current NFC commit will be rewritten by the 
> followup changes, but I think it's still useful to merge it, because it 
> documents the switch from the old implementation.

I'm looking forward to these change. Count me for the reviews.
Also, note that this is in production here, so it will take me time to verify 
the patches. So, I'd probably prefer to have a patch stack, and do the 
evaluation for some selected patches - basically bundling the stack into a few 
(1-2) checkpoints.
It would mean that the reviews would be done for each revision, but the 
verification (prior to landing anything) would be done at the selected stages.
WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149259

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


[PATCH] D145441: [AMDGPU] Define data layout entries for buffers

2023-05-03 Thread Jan-Patrick Lehr via Phabricator via cfe-commits
jplehr added a comment.

Hey Krzysztof,
I believe this broke the openmp offload buildbot 
https://lab.llvm.org/buildbot/#/builders/193/builds/30576


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145441

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


[PATCH] D149758: Revert "[AMDGPU] Define data layout entries for buffers"

2023-05-03 Thread Krzysztof Drewniak via Phabricator via cfe-commits
krzysz00 created this revision.
Herald added subscribers: kosarev, jeroen.dobbelaere, foad, wenlei, okura, 
kuter, kerbowa, arphaman, zzheng, hiraditya, tpr, dstuttard, yaxunl, jvesely, 
kzhuravl, arsenm, MatzeB.
Herald added a project: All.
krzysz00 requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, pcwang-thead, wdng.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: sstefan1.
Herald added projects: clang, LLVM.

This reverts commit f9c1ede2543b37fabe9f2d8f8fed5073c475d850 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149758

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/test/CodeGen/target-data.c
  clang/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl
  llvm/docs/AMDGPUUsage.rst
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/Target/AMDGPU/AMDGPU.h
  llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.cpp
  llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
  llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.f32-no-rtn.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.f32-rtn.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.f64.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.v2f16-no-rtn.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.v2f16-rtn.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-non-integral-address-spaces.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.atomic.dim.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.dim.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.2d.d16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.2d.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.2darraymsaa.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.3d.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.d.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.g16.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.g16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.store.2d.d16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.image.atomic.dim.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.add.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.cmpswap.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.fadd-with-ret.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.fadd.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.load.format.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.load.format.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.store.format.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.store.format.f32.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.store.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.load.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.store.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.store.i8.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.store.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.add.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.cmpswap.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.fadd-with-ret.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.fadd.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.load.format.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.load.format.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.load.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.store.format.f16.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.store.format.f32.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.store.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.tbuffer.load.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.tbuffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.image.load.1d.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.image.sample.1d.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.raw.buffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.struct.buffer.load.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.struct.buffer.store.ll
  llvm/test/CodeGen/AMDGPU/addrspacecast

[PATCH] D149758: Revert "[AMDGPU] Define data layout entries for buffers"

2023-05-03 Thread Krzysztof Drewniak via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3f2fbe92d0f4: Revert "[AMDGPU] Define data layout 
entries for buffers" (authored by krzysz00).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149758

Files:
  clang/lib/Basic/Targets/AMDGPU.cpp
  clang/test/CodeGen/target-data.c
  clang/test/CodeGenOpenCL/amdgpu-env-amdgcn.cl
  llvm/docs/AMDGPUUsage.rst
  llvm/lib/IR/AutoUpgrade.cpp
  llvm/lib/Target/AMDGPU/AMDGPU.h
  llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.cpp
  llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
  llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.f32-no-rtn.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.f32-rtn.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.f64.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.v2f16-no-rtn.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/buffer-atomic-fadd.v2f16-rtn.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-non-integral-address-spaces.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.atomic.dim.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.dim.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.2d.d16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.2d.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.2darraymsaa.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.load.3d.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.d.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.g16.a16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.sample.g16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/legalize-llvm.amdgcn.image.store.2d.d16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.image.atomic.dim.mir
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.add.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.cmpswap.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.fadd-with-ret.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.atomic.fadd.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.load.format.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.load.format.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.store.format.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.store.format.f32.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.buffer.store.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.load.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.store.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.store.i8.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.raw.tbuffer.store.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.add.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.cmpswap.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.fadd-with-ret.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.atomic.fadd.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.load.format.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.load.format.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.load.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.store.format.f16.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.store.format.f32.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.buffer.store.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.tbuffer.load.f16.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.struct.tbuffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.image.load.1d.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.image.sample.1d.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.raw.buffer.load.ll
  llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.struct.buffer.load.ll
  
llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-amdgcn.struct.buffer.store.ll
  llvm/test/CodeGen/AMDGPU/addrspacecast-captured.ll
  llvm/test/CodeGen/AMDGPU/annotate-kernel-features-hsa.ll
  llvm/test/CodeGen/AMDGPU/buffer-atomic-fadd.f32-no-rtn.ll
  llvm/test/CodeGen/AMDGPU/buffer-atomic-fadd.f32-rtn.ll
  llvm/test/CodeG

[PATCH] D148800: [C2x] Update 'nullptr' implementation based on CD comments

2023-05-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 519102.
aaron.ballman added a comment.

Updated based on review feedback:

- Changed CK_Noop to CK_NullToPointer to model the same as in C++
- Added codegen tests
- Added a constant expression test
- Fixed some comments in the test file


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

https://reviews.llvm.org/D148800

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExpr.cpp
  clang/test/C/C2x/n3042.c
  clang/test/CodeGen/nullptr.c
  clang/test/Sema/nullptr.c
  clang/www/c_status.html

Index: clang/www/c_status.html
===
--- clang/www/c_status.html
+++ clang/www/c_status.html
@@ -1215,13 +1215,7 @@
 
   Introduce the nullptr constant
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3042.htm";>N3042
-  
-Partial
-  Parts of the implementation may be incorrect until WG14 has completed NB comment
-  resolution for incompatibilities with C++ that were discovered. The major use cases
-  and usage patterns should work well, though.
-
-  
+  Clang 17
 
 
   Memory layout of unions
Index: clang/test/Sema/nullptr.c
===
--- clang/test/Sema/nullptr.c
+++ clang/test/Sema/nullptr.c
@@ -16,8 +16,8 @@
   p = null;
   int *pi = nullptr;
   pi = null;
-  null = 0; // expected-error {{assigning to 'nullptr_t' from incompatible type 'int'}}
-  bool b = nullptr; // expected-error {{initializing 'bool' with an expression of incompatible type 'nullptr_t'}}
+  null = 0;
+  bool b = nullptr;
 
   // Can't convert nullptr to integral implicitly.
   uintptr_t i = nullptr; // expected-error-re {{initializing 'uintptr_t' (aka '{{.*}}') with an expression of incompatible type 'nullptr_t'}}
@@ -77,6 +77,9 @@
 
 static_assert(sizeof(nullptr_t) == sizeof(void*), "");
 
+static_assert(!nullptr, "");
+static_assert(!(bool){nullptr}, "");
+
 static_assert(!(nullptr < nullptr), ""); // expected-error {{invalid operands to binary expression}}
 static_assert(!(nullptr > nullptr), ""); // expected-error {{invalid operands to binary expression}}
 static_assert(  nullptr <= nullptr, ""); // expected-error {{invalid operands to binary expression}}
Index: clang/test/CodeGen/nullptr.c
===
--- /dev/null
+++ clang/test/CodeGen/nullptr.c
@@ -0,0 +1,63 @@
+// RUN: %clang_cc1 -S %s -std=c2x -emit-llvm -o - | FileCheck %s
+
+// Test that null <-> nullptr_t conversions work as expected.
+typedef typeof(nullptr) nullptr_t;
+
+nullptr_t nullptr_t_val;
+
+void bool_func(bool);
+void nullptr_func(nullptr_t);
+
+void test() {
+  // Test initialization
+  bool bool_from_nullptr_t = nullptr_t_val;
+  nullptr_t nullptr_t_from_nullptr = nullptr;
+  void *vp_from_nullptr_t = nullptr_t_val;
+  nullptr_t nullptr_t_from_vp = (void *)0;
+  nullptr_t nullptr_t_from_int = 0;
+
+  // Test assignment
+  bool_from_nullptr_t = nullptr_t_val;
+  nullptr_t_from_nullptr = nullptr;
+  vp_from_nullptr_t = nullptr_t_val;
+  nullptr_t_from_vp = (void *)0;
+  nullptr_t_from_int = 0;
+
+  // Test calls
+  bool_func(nullptr_t_from_nullptr);
+  nullptr_func(nullptr_t_from_nullptr);
+  nullptr_func(0);
+  nullptr_func((void *)0);
+  nullptr_func(nullptr);
+  nullptr_func(false);
+
+  // Allocation of locals
+  // CHECK: %[[bool_from_nullptr_t:.*]] = alloca i8, align 1
+  // CHECK: %[[nullptr_t_from_nullptr:.*]] = alloca ptr, align 8
+  // CHECK: %[[vp_from_nullptr_t:.*]] = alloca ptr, align 8
+  // CHECK: %[[nullptr_t_from_vp:.*]] = alloca ptr, align 8
+  // CHECK: %[[nullptr_t_from_int:.*]] = alloca ptr, align 8
+
+  // Initialization of locals
+  // CHECK: store i8 0, ptr %[[bool_from_nullptr_t]], align 1
+  // CHECK: store ptr null, ptr %[[nullptr_t_from_nullptr]], align 8
+  // CHECK: store ptr null, ptr %[[vp_from_nullptr_t]], align 8
+  // CHECK: store ptr null, ptr %[[nullptr_t_from_vp]], align 8
+  // CHECK: store ptr null, ptr %[[nullptr_t_from_int]], align 8
+
+  // Assignment expressions
+  // CHECK: store i8 0, ptr %[[bool_from_nullptr_t]], align 1
+  // CHECK: store ptr null, ptr %[[nullptr_t_from_nullptr]], align 8
+  // CHECK: store ptr null, ptr %[[vp_from_nullptr_t]], align 8
+  // CHECK: store ptr null, ptr %[[nullptr_t_from_vp]], align 8
+  // CHECK: store ptr null, ptr %[[nullptr_t_from_int]], align 8
+
+  // Calls
+  // CHECK: call void @bool_func(i1 noundef zeroext false)
+  // CHECK: call void @nullptr_func(ptr null)
+  // CHECK: call void @nullptr_func(ptr null)
+  // CHECK: call void @nullptr_func(ptr null)
+  // CHECK: call void @nullptr_func(ptr null)
+  // CHECK: call void @nullptr_func(ptr null)
+}
+
Index: clang/test/C/C2x/n3042.c
===
--- clang/test/C/C2x/n3042.c
+++ clang/test/C/C2x/n3042.c
@@ -1,10 +1,7 @@
 // RUN: %clang_cc1 -verify -ffreestanding -Wno-un

[PATCH] D141714: Fix ast print of variables with attributes

2023-05-03 Thread Giuliano Belinassi via Phabricator via cfe-commits
giulianobelinassi added a comment.

Hi. See inline answers. I will send the new version in a few minutes.




Comment at: clang/lib/AST/DeclPrinter.cpp:52-58
+enum AttrPrintLoc {
+  SIDE_NONE = 0,
+  SIDE_LEFT = 1,
+  SIDE_MIDDLE = 2,
+  SIDE_RIGHT = 4,
+  SIDE_ANY = SIDE_LEFT | SIDE_MIDDLE | SIDE_RIGHT,
+};

aaron.ballman wrote:
> aaron.ballman wrote:
> > 
> I think we should use an `enum class` just so we don't steal these 
> identifiers at the global scope within this file, WDYT?
Unfortunately that would result in necessary auxiliary code to do an bitwise 
'&' operation, so I don't think this is a good idea. Although I've explicitly 
now access those constants by using the AttrPrintLoc:: keyword rather than 
directly referencing the constant directly.



Comment at: clang/lib/AST/DeclPrinter.cpp:272
+  // to classify each of them.
+  if (A->isCXX11Attribute()) {
+// C++11 onwards attributes can not be placed on left side. Output on

aaron.ballman wrote:
> This should also handle C2x attributes, so I'd use 
> `isStandardAttributeSyntax()` instead.
Done.



Comment at: clang/lib/AST/DeclPrinter.cpp:279
+attrloc = Right;
+  } else if (kind == attr::DiagnoseIf) {
+// DiagnoseIf should be print on the right side because it may refer to

aaron.ballman wrote:
> There are other attributes for which this is true as well, like `enable_if`, 
> thread safety annotations, etc.
I have expanded this to enable_if, as it is trivial. The thread safety 
annotations is not that trivial and seems to not trigger any test failure. So I 
think I will leave this to a next patch that properly parses the attributes to 
find references to symbols declared in function argument.



Comment at: clang/test/Analysis/blocks.mm:81
 // ANALYZER-NEXT:   2: [B1.1] (CXXConstructExpr, [B1.3], 
StructWithCopyConstructor)
-// CHECK-NEXT:   3: StructWithCopyConstructor s(5) 
__attribute__((blocks("byref")));
+// CHECK-NEXT:   3: StructWithCopyConstructor s 
__attribute__((blocks("byref")))(5);
 // CHECK-NEXT:   4: ^{ }

aaron.ballman wrote:
> I can't quite tell if this change is good, bad, or just different.
This indeed doesn't look good, but for what it is worth it is still corretly 
accepted by clang and gcc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141714

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


[PATCH] D148767: Restore CodeGen/LowLevelType from `Support`

2023-05-03 Thread Slava Zakharin via Phabricator via cfe-commits
vzakhari added a comment.

In D148767#4315667 , @jobnoorman 
wrote:

> Hi, this seems to have broken my bolt+debug+shared build. I don't think there 
> are build bots for this configuration but you can reproduce it like this:

Same with flang+debug+shared build:

  ld.lld: error: undefined symbol: llvm::LLT::print(llvm::raw_ostream&) const
  >>> referenced by LowLevelType.h:269 
(/llvm-project/llvm/include/llvm/CodeGen/LowLevelType.h:269)
  >>>   
CMakeFiles/obj.FIRTransforms.dir/SimplifyIntrinsics.cpp.o:(llvm::LLT::dump() 
const)

I can add `CodeGenTypes` link component in 
`flang/lib/Optimizer/Transforms/CMakeLists.txt`, but I am worried about the 
comment in `llvm/lib/CodeGen/CMakeLists.txt`:

  # Be careful to append deps on this, since Targets' tablegens depend on this.
  add_llvm_component_library(LLVMCodeGenTypes

I am not sure whether I need to be careful about adding dependencies onto 
`LLVMCodeGenTypes` (as I am planning to do) or about adding dependencies for 
`LLVMCodeGenTypes` target in `llvm/lib/CodeGen/CMakeLists.txt` :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148767

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


[PATCH] D149553: [clang] Use -std=c++23 instead of -std=c++2b

2023-05-03 Thread Mark de Wever via Phabricator via cfe-commits
Mordante marked 3 inline comments as done.
Mordante added a comment.

In D149553#4313211 , @aaron.ballman 
wrote:

> In D149553#4312788 , @Mordante 
> wrote:
>
>> In D149553#4310478 , 
>> @aaron.ballman wrote:
>>
>>> Thank you for working on this! The Clang changes are mostly all good, but I 
>>> think we should hold off on changing the value of `__cplusplus` for the 
>>> moment as it's not set in stone in the standard yet (I thought it was 
>>> though, so I'm checking with the editor).
>>
>> I noticed the same and I was wondering whether I should contact the editor. 
>> I noticed some approved papers also have not been merged yet. (These papers 
>> are still open issues in the paper repository.)
>>
>> I'll wait a few days to see whether the draft gets updated otherwise I 
>> revert the macro change.
>>
>> I would be very surprised if the macro gets a different value, that's why I 
>> already bumped it.
>
> I heard back from the editor yesterday and he said that the macro would be 
> replaced with an appropriate value for the DIS, and that he hopes to ensure 
> that value is published in the next meeting mailing (which would be roughly 
> May 15). So I think we can either wait until that mailing comes out and see 
> if it has a concrete value to land these changes, or we can land everything 
> but the macro value changes and deal with that in a follow up (this might be 
> easier due to rebasing woes given how large this patch is).

Yes I created a new patch for that part. That patch is a lot easier to rebase 
than this one.




Comment at: clang/lib/Frontend/InitPreprocessor.cpp:455
+if (LangOpts.CPlusPlus23)
+  Builder.defineMacro("__cplusplus", "202302L");
 //  [C++20] The integer literal 202002L.

aaron.ballman wrote:
> I think this might still be premature. I noticed that the draft C++ standard 
> does not have this value set yet: 
> https://github.com/cplusplus/draft/blob/main/source/config.tex#L6
> 
> I've emailed the editor to find out if that's a mistake (I am pretty sure 
> we're sending this off to PDTS at some point shortly).
I've reverted this and similar parts. They are now in D149761. I will update 
that patch once editor made a release with the updated `__cplusplus` macro.



Comment at: clang/test/Parser/cxx2b-label.cpp:1
-// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx2b -std=c++2b 
-Wpre-c++2b-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx23 -std=c++23 
-Wpre-c++23-compat %s
 // RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx20 -std=c++20 %s

ldionne wrote:
> We could also consider renaming those files.
I could do that but rather in a follow-up there are still files named like 
`clang/test/Parser/cxx0x-lambda-expressions.cpp` too. I'm not sure how much the 
Clang devs want to remove these code names.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149553

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


[PATCH] D146148: Float_t and double_t types shouldn't be modified by #pragma clang fp eval_method

2023-05-03 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 519121.

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

https://reviews.llvm.org/D146148

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Basic/TokenKinds.h
  clang/include/clang/Lex/Token.h
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/Lex/Lexer.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/abi-check-1.cpp
  clang/test/Sema/abi-check-2.cpp
  clang/test/Sema/abi-check-3.cpp
  clang/test/Sema/attr-only-in-default-eval.cpp

Index: clang/test/Sema/attr-only-in-default-eval.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-only-in-default-eval.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+typedef float float_t [[clang::available_only_in_default_eval_method]];
+using double_t __attribute__((available_only_in_default_eval_method)) = double;
+
+// expected-error@+1{{'available_only_in_default_eval_method' attribute only applies to typedefs}}
+class  __attribute__((available_only_in_default_eval_method)) C1 {
+};
+// expected-error@+1{{'available_only_in_default_eval_method' attribute only applies to typedefs}}
+class  [[clang::available_only_in_default_eval_method]] C2 {
+};
+
+// expected-error@+1{{'available_only_in_default_eval_method' attribute only applies to typedefs}}
+struct [[clang::available_only_in_default_eval_method]] S1;
+// expected-error@+1{{'available_only_in_default_eval_method' attribute only applies to typedefs}}
+struct __attribute__((available_only_in_default_eval_method)) S2;
+
+// expected-error@+1{{'available_only_in_default_eval_method' attribute only applies to typedefs}}
+void __attribute__((available_only_in_default_eval_method)) foo();
+// expected-error@+1{{'available_only_in_default_eval_method' attribute cannot be applied to types}}
+void [[clang::available_only_in_default_eval_method]] goo();
+// expected-error@+1{{'available_only_in_default_eval_method' attribute cannot be applied to types}}
+void bar() [[clang::available_only_in_default_eval_method]];
+// expected-error@+1{{'available_only_in_default_eval_method' attribute only applies to typedefs}}
+void barz() __attribute__((available_only_in_default_eval_method));
+
Index: clang/test/Sema/abi-check-3.cpp
===
--- /dev/null
+++ clang/test/Sema/abi-check-3.cpp
@@ -0,0 +1,86 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -DNOERROR %s
+// RUN: %clang_cc1 -fsyntax-only -verify -ffp-eval-method=extended -DNOERROR %s
+
+// RUN: %clang_cc1 -verify -fsyntax-only -ffp-eval-method=source %s
+// RUN: %clang_cc1 -verify -fsyntax-only -ffp-eval-method=double %s
+
+
+#ifdef NOERROR
+// expected-no-diagnostics
+typedef float float_t;
+typedef double double_t;
+#else
+typedef float float_t; //expected-error 9 {{cannot use type 'float_t' within '#pragma clang fp eval_method'; type is set according to the default eval method for the translation unit}}
+
+typedef double double_t; //expected-error 9 {{cannot use type 'double_t' within '#pragma clang fp eval_method'; type is set according to the default eval method for the translation unit}}
+#endif
+
+float foo1() {
+#pragma clang fp eval_method(extended)
+  float a;
+  double b;
+  return a - b;
+}
+
+float foo2() {
+#pragma clang fp eval_method(extended)
+  float_t a; 
+  double_t b; 
+  return a - b;
+}
+
+void foo3() {
+#pragma clang fp eval_method(extended)
+  char buff[sizeof(float_t)];
+  char bufd[sizeof(double_t)];
+  buff[1] = bufd[2];
+}
+
+float foo4() {
+#pragma clang fp eval_method(extended)
+  typedef float_t FT;
+  typedef double_t DT;
+  FT a;
+  DT b;
+  return a - b;
+}
+
+int foo5() {
+#pragma clang fp eval_method(extended)
+  int t = _Generic( 1.0L, float_t:1, default:0);
+  int v = _Generic( 1.0L, double_t:1, default:0);
+  return t;
+}
+
+void foo6() {
+#pragma clang fp eval_method(extended)
+  auto resf = [](float_t f) { return f; };
+  auto resd = [](double_t g) { return g; };
+}
+
+void foo7() {
+#pragma clang fp eval_method(extended)
+  float f = (float_t)1; 
+  double d = (double_t)2; 
+}
+
+void foo8() {
+#pragma clang fp eval_method(extended)
+  using Ft = float_t;
+  using Dt = double_t;
+  Ft a;
+  Dt b;
+}
+
+void foo9() {
+#pragma clang fp eval_method(extended)
+  float c1 = (float_t)12;
+  double c2 = (double_t)13;
+}
+
+float foo10() {
+#pragma clang fp eval_method(extended)
+  extern float_t f;
+  extern double_t g;
+  return f-g;
+}
Index: clang/test/Sema/abi-check-2.cpp
===
--- /dev/null
+++ clang/test/Sema/abi-check-2.cpp
@@ -0,0 +1,85 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -DNOERROR %s
+// RUN

[PATCH] D146148: Float_t and double_t types shouldn't be modified by #pragma clang fp eval_method

2023-05-03 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added a comment.

(Sorry for taking so long to get to it!) Thanks.


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

https://reviews.llvm.org/D146148

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


[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

2023-05-03 Thread Alexander Shaposhnikov via Phabricator via cfe-commits
alexander-shaposhnikov added a comment.

@erichkeane - thanks, then I'm going to give it another try.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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


[PATCH] D141714: Fix ast print of variables with attributes

2023-05-03 Thread Giuliano Belinassi via Phabricator via cfe-commits
giulianobelinassi updated this revision to Diff 519131.
giulianobelinassi added a comment.

Incorporate some of Aron suggestions:

- Replace isDeclspecAttribute with isStandardAttributeSyntax
- Avoid multiple calls to getAsFunction
- Use AttrPrintLoc:: instead of referencing the value directly
- Also output enable_if on the right side of decl.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141714

Files:
  clang/lib/AST/DeclPrinter.cpp
  clang/test/AST/ast-print-attr-knr.c
  clang/test/AST/ast-print-attr.c
  clang/test/AST/ast-print-pragmas.cpp
  clang/test/Analysis/blocks.mm
  clang/test/OpenMP/assumes_codegen.cpp
  clang/test/OpenMP/assumes_print.cpp
  clang/test/OpenMP/assumes_template_print.cpp
  clang/test/OpenMP/declare_simd_ast_print.cpp
  clang/test/Sema/attr-print.c
  clang/test/SemaCXX/attr-print.cpp
  clang/test/SemaCXX/cxx11-attr-print.cpp

Index: clang/test/SemaCXX/cxx11-attr-print.cpp
===
--- clang/test/SemaCXX/cxx11-attr-print.cpp
+++ clang/test/SemaCXX/cxx11-attr-print.cpp
@@ -3,8 +3,7 @@
 // CHECK: int x __attribute__((aligned(4)));
 int x __attribute__((aligned(4)));
 
-// FIXME: Print this at a valid location for a __declspec attr.
-// CHECK: int y __declspec(align(4));
+// CHECK: __declspec(align(4)) int y;
 __declspec(align(4)) int y;
 
 // CHECK: int z {{\[}}[gnu::aligned(4)]];
@@ -65,7 +64,7 @@
 
 // CHECK: int m __attribute__((aligned(4
 // CHECK: int n alignas(4
-// CHECK: static int f() __attribute__((pure))
+// CHECK: __attribute__((pure)) static int f()
 // CHECK: static int g() {{\[}}[gnu::pure]]
 template  struct S {
   __attribute__((aligned(4))) int m;
@@ -80,7 +79,7 @@
 
 // CHECK: int m __attribute__((aligned(4
 // CHECK: int n alignas(4
-// CHECK: static int f() __attribute__((pure))
+// CHECK: __attribute__((pure)) static int f()
 // CHECK: static int g() {{\[}}[gnu::pure]]
 template struct S;
 
Index: clang/test/SemaCXX/attr-print.cpp
===
--- clang/test/SemaCXX/attr-print.cpp
+++ clang/test/SemaCXX/attr-print.cpp
@@ -3,8 +3,7 @@
 // CHECK: int x __attribute__((aligned(4)));
 int x __attribute__((aligned(4)));
 
-// FIXME: Print this at a valid location for a __declspec attr.
-// CHECK: int y __declspec(align(4));
+// CHECK: __declspec(align(4)) int y;
 __declspec(align(4)) int y;
 
 // CHECK: void foo() __attribute__((const));
Index: clang/test/Sema/attr-print.c
===
--- clang/test/Sema/attr-print.c
+++ clang/test/Sema/attr-print.c
@@ -3,8 +3,7 @@
 // CHECK: int x __attribute__((aligned(4)));
 int x __attribute__((aligned(4)));
 
-// FIXME: Print this at a valid location for a __declspec attr.
-// CHECK: int y __declspec(align(4));
+// CHECK: __declspec(align(4)) int y;
 __declspec(align(4)) int y;
 
 // CHECK: short arr[3] __attribute__((aligned));
Index: clang/test/OpenMP/declare_simd_ast_print.cpp
===
--- clang/test/OpenMP/declare_simd_ast_print.cpp
+++ clang/test/OpenMP/declare_simd_ast_print.cpp
@@ -60,11 +60,11 @@
 
 class VV {
   // CHECK: #pragma omp declare simd uniform(this, a) linear(val(b): a)
-  // CHECK-NEXT: int add(int a, int b) __attribute__((cold)){
+  // CHECK-NEXT:  __attribute__((cold)) int add(int a, int b) {
   // CHECK-NEXT: return a + b;
   // CHECK-NEXT: }
   #pragma omp declare simd uniform(this, a) linear(val(b): a)
-  int add(int a, int b) __attribute__((cold)) { return a + b; }
+  __attribute__((cold)) int add(int a, int b) { return a + b; }
 
   // CHECK: #pragma omp declare simd aligned(b: 4) aligned(a) linear(ref(b): 4) linear(val(this)) linear(val(a))
   // CHECK-NEXT: float taddpf(float *a, float *&b) {
Index: clang/test/OpenMP/assumes_template_print.cpp
===
--- clang/test/OpenMP/assumes_template_print.cpp
+++ clang/test/OpenMP/assumes_template_print.cpp
@@ -17,7 +17,7 @@
 struct S {
   int a;
 // CHECK: template  struct S {
-// CHECK: void foo() __attribute__((assume("ompx_global_assumption"))) {
+// CHECK: __attribute__((assume("ompx_global_assumption"))) void foo() {
   void foo() {
 #pragma omp parallel
 {}
@@ -25,15 +25,15 @@
 };
 
 // CHECK: template<> struct S {
-// CHECK: void foo() __attribute__((assume("ompx_global_assumption"))) {
+// CHECK: __attribute__((assume("ompx_global_assumption"))) void foo() {
 
 #pragma omp begin assumes no_openmp
-// CHECK: void S_with_assumes_no_call() __attribute__((assume("omp_no_openmp"))) __attribute__((assume("ompx_global_assumption"))) {
+// CHECK: __attribute__((assume("omp_no_openmp"))) __attribute__((assume("ompx_global_assumption"))) void S_with_assumes_no_call() {
 void S_with_assumes_no_call() {
   S s;
   s.a = 0;
 }
-// CHECK: void 

[PATCH] D149553: [clang] Use -std=c++23 instead of -std=c++2b

2023-05-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM! I did not spot any other concerns beyond the macro value one. Thank you 
for working on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149553

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


[clang] 8c22cbe - [analyzer] ArrayBoundCheckerV2: suppress false positives from ctype macros

2023-05-03 Thread Donát Nagy via cfe-commits

Author: Donát Nagy
Date: 2023-05-03T18:52:27+02:00
New Revision: 8c22cbea87beb74da3dc5891c40cdf574cd5fe56

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

LOG: [analyzer] ArrayBoundCheckerV2: suppress false positives from ctype macros

The checker alpha.security.ArrayBoundV2 created bug reports in
situations when the (tainted) result of fgetc() or getchar() was passed
to one of the isX() macros from ctype.h.

This is a common input handling pattern (within the limited toolbox of
the C language) and several open source projects contained code where it
led to false positive reports; so this commit suppresses ArrayBoundV2
reports generated within the isX() macros.

Note that here even true positive reports would be difficult to
understand, as they'd refer to the implementation details of these
macros.

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
clang/test/Analysis/taint-generic.c

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
index a476613b6dcc7..6d0cc505756b8 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -42,8 +42,10 @@ class ArrayBoundCheckerV2 :
   void reportTaintOOB(CheckerContext &C, ProgramStateRef errorState,
   SVal TaintedSVal) const;
 
+  static bool isFromCtypeMacro(const Stmt *S, ASTContext &AC);
+
 public:
-  void checkLocation(SVal l, bool isLoad, const Stmt*S,
+  void checkLocation(SVal l, bool isLoad, const Stmt *S,
  CheckerContext &C) const;
 };
 
@@ -155,6 +157,15 @@ void ArrayBoundCheckerV2::checkLocation(SVal location, 
bool isLoad,
   // memory access is within the extent of the base region.  Since we
   // have some flexibility in defining the base region, we can achieve
   // various levels of conservatism in our buffer overflow checking.
+
+  // The header ctype.h (from e.g. glibc) implements the isX() macros as
+  //   #define isX(arg) (LOOKUP_TABLE[arg] & BITMASK_FOR_X)
+  // and incomplete analysis of these leads to false positives. As even
+  // accurate reports would be confusing for the users, just disable reports
+  // from these macros:
+  if (isFromCtypeMacro(LoadS, checkerContext.getASTContext()))
+return;
+
   ProgramStateRef state = checkerContext.getState();
 
   SValBuilder &svalBuilder = checkerContext.getSValBuilder();
@@ -267,6 +278,25 @@ void ArrayBoundCheckerV2::reportOOB(CheckerContext 
&checkerContext,
   checkerContext.emitReport(std::move(BR));
 }
 
+bool ArrayBoundCheckerV2::isFromCtypeMacro(const Stmt *S, ASTContext &ACtx) {
+  SourceLocation Loc = S->getBeginLoc();
+  if (!Loc.isMacroID())
+return false;
+
+  StringRef MacroName = Lexer::getImmediateMacroName(
+  Loc, ACtx.getSourceManager(), ACtx.getLangOpts());
+
+  if (MacroName.size() < 7 || MacroName[0] != 'i' || MacroName[1] != 's')
+return false;
+
+  return ((MacroName == "isalnum") || (MacroName == "isalpha") ||
+  (MacroName == "isblank") || (MacroName == "isdigit") ||
+  (MacroName == "isgraph") || (MacroName == "islower") ||
+  (MacroName == "isnctrl") || (MacroName == "isprint") ||
+  (MacroName == "ispunct") || (MacroName == "isspace") ||
+  (MacroName == "isupper") || (MacroName == "isxdigit"));
+}
+
 #ifndef NDEBUG
 LLVM_DUMP_METHOD void RegionRawOffsetV2::dump() const {
   dumpToStream(llvm::errs());

diff  --git a/clang/test/Analysis/taint-generic.c 
b/clang/test/Analysis/taint-generic.c
index 626e01e39d158..62e1f570b6622 100644
--- a/clang/test/Analysis/taint-generic.c
+++ b/clang/test/Analysis/taint-generic.c
@@ -156,6 +156,28 @@ void bufferGetchar(int x) {
   Buffer[m] = 1;  //expected-warning {{Out of bound memory access (index is 
tainted)}}
 }
 
+extern const unsigned short int **__ctype_b_loc (void);
+enum { _ISdigit = 2048 };
+# define isdigit(c) ((*__ctype_b_loc ())[(int) (c)] & (unsigned short int) 
_ISdigit)
+
+int isdigitImplFalsePositive(void) {
+  // If this code no longer produces a bug report, then consider removing the
+  // special case that disables buffer overflow reports coming from the isX
+  // macros in ctypes.h.
+  int c = getchar();
+  return ((*__ctype_b_loc ())[(int) (c)] & (unsigned short int) _ISdigit);
+  //expected-warning@-1 {{Out of bound memory access (index is tainted)}}
+}
+
+int isdigitSuppressed(void) {
+  // Same code as above, but reports are suppressed based on macro name:
+  int c = getchar();
+  return isdigit(c); //no-warning
+}
+
+// Some later tests use isdigit as a function, so we need to undef it:
+#undef isdig

[PATCH] D149713: [Sema] Avoid emitting warnings for constant destruction.

2023-05-03 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

I think this would be an interesting test:

  struct S {
/*not constexpr*/ S();
constexpr ~S() {}
  };
  S s; // no warning
  
  struct T {
/*not constexpr*/ T();
constexpr ~T() { if (b) {} }
bool b;
  };
  T t; // expected-warning {{exit-time destructor}}

(For a non-`constexpr` variable with a `constexpr` destructor, our behaviour 
depends on whether the destructor call is a constant expression.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149713

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


[PATCH] D149460: [analyzer] ArrayBoundCheckerV2: suppress false positives from ctype macros

2023-05-03 Thread Donát Nagy via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8c22cbea87be: [analyzer] ArrayBoundCheckerV2: suppress false 
positives from ctype macros (authored by donat.nagy).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149460

Files:
  clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
  clang/test/Analysis/taint-generic.c


Index: clang/test/Analysis/taint-generic.c
===
--- clang/test/Analysis/taint-generic.c
+++ clang/test/Analysis/taint-generic.c
@@ -156,6 +156,28 @@
   Buffer[m] = 1;  //expected-warning {{Out of bound memory access (index is 
tainted)}}
 }
 
+extern const unsigned short int **__ctype_b_loc (void);
+enum { _ISdigit = 2048 };
+# define isdigit(c) ((*__ctype_b_loc ())[(int) (c)] & (unsigned short int) 
_ISdigit)
+
+int isdigitImplFalsePositive(void) {
+  // If this code no longer produces a bug report, then consider removing the
+  // special case that disables buffer overflow reports coming from the isX
+  // macros in ctypes.h.
+  int c = getchar();
+  return ((*__ctype_b_loc ())[(int) (c)] & (unsigned short int) _ISdigit);
+  //expected-warning@-1 {{Out of bound memory access (index is tainted)}}
+}
+
+int isdigitSuppressed(void) {
+  // Same code as above, but reports are suppressed based on macro name:
+  int c = getchar();
+  return isdigit(c); //no-warning
+}
+
+// Some later tests use isdigit as a function, so we need to undef it:
+#undef isdigit
+
 void testUncontrolledFormatString(char **p) {
   char s[80];
   fscanf(stdin, "%s", s);
Index: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -42,8 +42,10 @@
   void reportTaintOOB(CheckerContext &C, ProgramStateRef errorState,
   SVal TaintedSVal) const;
 
+  static bool isFromCtypeMacro(const Stmt *S, ASTContext &AC);
+
 public:
-  void checkLocation(SVal l, bool isLoad, const Stmt*S,
+  void checkLocation(SVal l, bool isLoad, const Stmt *S,
  CheckerContext &C) const;
 };
 
@@ -155,6 +157,15 @@
   // memory access is within the extent of the base region.  Since we
   // have some flexibility in defining the base region, we can achieve
   // various levels of conservatism in our buffer overflow checking.
+
+  // The header ctype.h (from e.g. glibc) implements the isX() macros as
+  //   #define isX(arg) (LOOKUP_TABLE[arg] & BITMASK_FOR_X)
+  // and incomplete analysis of these leads to false positives. As even
+  // accurate reports would be confusing for the users, just disable reports
+  // from these macros:
+  if (isFromCtypeMacro(LoadS, checkerContext.getASTContext()))
+return;
+
   ProgramStateRef state = checkerContext.getState();
 
   SValBuilder &svalBuilder = checkerContext.getSValBuilder();
@@ -267,6 +278,25 @@
   checkerContext.emitReport(std::move(BR));
 }
 
+bool ArrayBoundCheckerV2::isFromCtypeMacro(const Stmt *S, ASTContext &ACtx) {
+  SourceLocation Loc = S->getBeginLoc();
+  if (!Loc.isMacroID())
+return false;
+
+  StringRef MacroName = Lexer::getImmediateMacroName(
+  Loc, ACtx.getSourceManager(), ACtx.getLangOpts());
+
+  if (MacroName.size() < 7 || MacroName[0] != 'i' || MacroName[1] != 's')
+return false;
+
+  return ((MacroName == "isalnum") || (MacroName == "isalpha") ||
+  (MacroName == "isblank") || (MacroName == "isdigit") ||
+  (MacroName == "isgraph") || (MacroName == "islower") ||
+  (MacroName == "isnctrl") || (MacroName == "isprint") ||
+  (MacroName == "ispunct") || (MacroName == "isspace") ||
+  (MacroName == "isupper") || (MacroName == "isxdigit"));
+}
+
 #ifndef NDEBUG
 LLVM_DUMP_METHOD void RegionRawOffsetV2::dump() const {
   dumpToStream(llvm::errs());


Index: clang/test/Analysis/taint-generic.c
===
--- clang/test/Analysis/taint-generic.c
+++ clang/test/Analysis/taint-generic.c
@@ -156,6 +156,28 @@
   Buffer[m] = 1;  //expected-warning {{Out of bound memory access (index is tainted)}}
 }
 
+extern const unsigned short int **__ctype_b_loc (void);
+enum { _ISdigit = 2048 };
+# define isdigit(c) ((*__ctype_b_loc ())[(int) (c)] & (unsigned short int) _ISdigit)
+
+int isdigitImplFalsePositive(void) {
+  // If this code no longer produces a bug report, then consider removing the
+  // special case that disables buffer overflow reports coming from the isX
+  // macros in ctypes.h.
+  int c = getchar();
+  return ((*__ctype_b_loc ())[(int) (c)] & (unsigned short int) _ISdigit);
+  //expected-warning@-1 {{Out of bound memory access (index is tainted)}}
+}
+
+int isdigitSuppressed(void) {
+  // Same code as above, but reports are supp

[PATCH] D149713: [Sema] Avoid emitting warnings for constant destruction.

2023-05-03 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

LGTM with the extra test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149713

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


[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

2023-05-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added subscribers: v.g.vassilev, junaire, rsmith.
aaron.ballman added a comment.

Adding in some of the clang-repl folks because they are likely to run into this 
as well, and adding Richard in case he has more historical context.

What I understand of the historical context here is that we've assumed very 
early on that inspecting state of partially-constructed AST nodes only happens 
when debugging (through things like calls to `dump()`, etc). So it was assumed 
that having a partially constructed object which is updated as we continue 
parsing and semantic analysis is fine because the only way to break the 
invariant is from the debugger. However, over time, we've invalidated that 
assumption more and more (REPL, libclang, etc) and now we're paying the price.

I think that refactoring the way we construct AST nodes so that they're not in 
a partially-constructed state by the time we've called `Create()` is a 
nonstarter; I think we've got too much reliance built up on that pattern. For 
example, we do `CreateEmpty()` + building up state as a matter of course when 
deserializing the AST for PCH or modules.  However, I'm not keen on us playing 
whack-a-mole with the kinds of checks from this review. For starters, that's 
going to have a long-tail that makes it hard to know if we've ever gotten to 
the bottom of the issue. But also, each one of these checks is basically 
useless for the primary way in which Clang is consumed (as a compiler), so this 
increases compile times for limited benefit to "most" users. In this particular 
case, we may be fine as this is limited to libclang and so it shouldn't add 
overhead for the common path, but I suspect we're going to find cases that need 
fixing in more common areas of the compiler that could be more troublesome.

I wish I had a good answer for how to address this, but I don't have one off 
the top of my head. About the closest I can think of is to use a bit on the 
declaration to say "I am being worked on still" and add a `Finalize()` method 
to say "I am no longer being worked on" and perform sanity checks and a getter 
method to tell us if the AST node is still under construction or not. That 
gives us a generic way to quickly test "should we be inspecting this further" 
in circumstances where it matters and there is precedent with how we handle 
parsing declaration specifiers 
(https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Sema/DeclSpec.h#L844),
 but this isn't all that much better than ad hoc tests like checking it the 
type is null or not (so I'm not suggesting this is the best way to solve the 
problem, just spitballing ideas).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149733

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


[PATCH] D149695: MS inline asm: remove obsolete code adding AOK_SizeDirective (e.g. dword ptr)

2023-05-03 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

In D149695#4315194 , @hans wrote:

>> The AOK_SizeDirective part from 5b37c181291210bedfbb7a6af5d51229f3652ef0
>> (2014-08) seems unneeded nowadays (the root cause has likely been fixed
>> elsewhere).
>
> Would it be possible to confirm when/if the size directive here became 
> unnecessary?

I compared the assembled output from C test files. They are identical. (I know 
it would be best to prove this by fully understanding the logic, but that 
appears difficult...)

> The generated object files for CodeGen/ms-inline-asm-functions.c, 
> CodeGen/ms-inline-asm-functions.c, and CodeGenCXX/ms-inline-asm-fields.cpp 
> are unchanged with just this patch.
>
> When D149579  is subsequently applied, the 
> FIXME part of kptr in CodeGen/ms-inline-asm-functions.c will be fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149695

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


[PATCH] D147626: [clang] Reject flexible array member in a union in C++

2023-05-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D147626#4315283 , @Fznamznon wrote:

> I've got an error from buildbot on Windows:
>
>   C:\Program Files (x86)\Windows 
> Kits\10\include\10.0.19041.0\um\winioctl.h:13404:51: error: flexible array 
> member 'Lev1Depends' in a union is not allowed
>   
>   STORAGE_QUERY_DEPENDENT_VOLUME_LEV1_ENTRY Lev1Depends[];
>
> It feels like Windows headers contain flexible array members in unions. We 
> probably can't just always reject them on Windows.

Thank you for the quick revert -- yeah, this means we have to make this work 
for MSVC compatibility, unfortunately. Any indication we're going to hit 
something similar with GCC compatibility in a glibc (or other system) header?

I think we'll have to back off the hard stance of always rejecting this because 
I think we'll need this to work in `-fms-compatibility` mode. If there's not 
indications of this being disruptive on non-MSVC-compatible targets, then we 
may still be able to get away with rejecting the extension there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147626

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


[PATCH] D149643: [clang-format] Correctly limit formatted ranges when specifying qualifier alignment

2023-05-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

Thanks for the patch...this tells me people are starting to use this feature in 
anger!! i.e. your formatting via git-clang-format (which is brave!) ;-) which 
means you have the code modifying features perhaps on my default...

LGTM, if you will need someone to land this for you we'll need your name and 
email address, if not please wait for @owenpan


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

https://reviews.llvm.org/D149643

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


[PATCH] D149647: [NFC][Clang]Fix static analyzer tool remarks about large copies by value

2023-05-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

Can you link to the Coverity issue so we can see what it was complaining about?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149647

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


[PATCH] D149650: Give NullabilityKind a printing operator<

2023-05-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D149650#4315211 , @sammccall wrote:

> In D149650#4312389 , @aaron.ballman 
> wrote:
>
>> I guess I'm not seeing much motivation for this change. We already have 
>> `clang::getNullabilitySpelling()` and `const StreamingDiagnostic 
>> &clang::operator<<(const StreamingDiagnostic &DB, DiagNullabilityKind 
>> nullability)` and now we're adding a third way to get this information. If 
>> this is just for debug/testing purposes, can we use existing debug 
>> formatters to convert the enumeration value into the enumerator name instead?
>
> We're using NullabilityKind in our dataflow-based null-safety clang-tidy 
> check  that we hope 
> to upstream when it works.

Ah, good to know! I think it would be reasonable to add this functionality once 
there's some agreement on adding the clang-tidy check.

> (The idea is to use `_Nullable` and `_Nonnull` annotations on API boundaries 
> to gradually type pointers, and to provide a verification clang-tidy check 
> and libraries to infer annotations for existing code. The actual clang-tidy 
> check wrapper isn't in that repo yet, but it's trivial).
>
> It would be convenient if e.g. EXPECT_THAT(someVectorOfNK, 
> ElementsAre(Nullable, Unspecified)) 
> 
>  printed something useful when it fails, if we could write OS << NK and get 
> Unspecified rather than OS << getSpelling(NK) and get 
> _Unspecified_Nullability, etc.
> This doesn't concern clang core (ha, there are no unit tests...) but there's 
> no reasonable way to do this downstream without using a different type.

Hmmm, but does this need to be added to `Specifiers.h` instead of being kept 
packaged up with clang-tidy? For things like AST matchers, we usually ask that 
the matcher remain local to the clang-tidy check unless the same matcher is 
needed in multiple checks. This feels somewhat similar, despite it not being 
related to AST matching -- if the only need for this functionality exists out 
of the clang tree, can we keep it out of the clang tree until we find more 
needs?

>> `StreamingDiagnostic &clang::operator<<`
>
> This actually wants the user-visible spelling, so I think it can just use 
> getSpelling... if I make that change we're back to just two implementations 
> :-)

Oh, I really like those changes! Feel free to land that as an NFC change if 
you'd like (the addition of quotes is a bug fix, but not really a functional 
change).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149650

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


[PATCH] D149647: [NFC][Clang]Fix static analyzer tool remarks about large copies by value

2023-05-03 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

In D149647#4316134 , @MyDeveloperDay 
wrote:

> Can you link to the Coverity issue so we can see what it was complaining 
> about?

Unfortunately this is from an internal-run of Coverity that Intel is running.  
I'd be shocked if it doesn't appear on the open-source/LLVM version though?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149647

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


[PATCH] D149259: [analyzer][NFC] Use std::optional instead of custom "empty" state

2023-05-03 Thread Donát Nagy via Phabricator via cfe-commits
donat.nagy added a comment.

> It's gonna be bumpy.

Yup :) but it's not impossible...

> Also, note that this is in production here, so it will take me time to verify 
> the patches. So, I'd probably prefer to have a patch stack, and do the 
> evaluation for some selected patches - basically bundling the stack into a 
> few (1-2) checkpoints.
> It would mean that the reviews would be done for each revision, but the 
> verification (prior to landing anything) would be done at the selected stages.
> WDYT?

It would be effective to adopt a workflow where the individual commits are 
quickly reviewed for design directions and visible code quality issues (to 
avoid dead ends and keep the rebasing on a manageable level); but the complete 
verification and the upstreaming is done later, in bundles (to avoid wasting 
time on repeated verifications). Feel free to give "Seems fine for now; start 
working on the next commit; this will be verified and merged later." reviews 
after handling the bulk of obvious issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149259

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


[PATCH] D149149: [clang][Interp] Check one-past-the-end pointers in GetPtrField

2023-05-03 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a subscriber: rsmith.
shafik added inline comments.



Comment at: clang/test/AST/Interp/records.cpp:509
 
   constexpr A *a2 = &b + 1; // expected-error {{must be initialized by a 
constant expression}} \
 // expected-note {{cannot access base class of 
pointer past the end of object}} \

@rsmith it looks like you added this diagnostic a while ago see ce1ec5e1f5620 
did you mean it to apply to this case? The rules have changed a lot since then 
so perhaps this was invalid but now is good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149149

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


[PATCH] D147626: [clang] Reject flexible array member in a union in C++

2023-05-03 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

> If there's not indications of this being disruptive on non-MSVC-compatible 
> targets, then we may still be able to get away with rejecting the extension 
> there.

If we need to have the codepath anyway, there isn't much harm in allowing it on 
all targets, I think.  There's really only one possible interpretation for the 
construct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147626

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


[PATCH] D148800: [C2x] Update 'nullptr' implementation based on CD comments

2023-05-03 Thread Eli Friedman via Phabricator via cfe-commits
efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D148800

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


[PATCH] D147626: [clang] Reject flexible array member in a union in C++

2023-05-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D147626#4316190 , @efriedma wrote:

>> If there's not indications of this being disruptive on non-MSVC-compatible 
>> targets, then we may still be able to get away with rejecting the extension 
>> there.
>
> If we need to have the codepath anyway, there isn't much harm in allowing it 
> on all targets, I think.  There's really only one possible interpretation for 
> the construct.

You would think, except the GCC extension differs based on C vs C++: 
https://godbolt.org/z/E14Yz37To as does the extension in Clang, but differently 
than GCC: https://godbolt.org/z/zYznaYPf5 and so we'd also have to dig into 
solving that if we wanted to keep GCC compatibility behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147626

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


[PATCH] D149647: [NFC][Clang]Fix static analyzer tool remarks about large copies by value

2023-05-03 Thread Soumi Manna via Phabricator via cfe-commits
Manna added a comment.

Thanks @erichkeane for response.

@MyDeveloperDay, Our internal Coverity link won't work without login. 
This is what Coverity was complaining about.

Big parameter passed by value
Copying large values is inefficient, consider passing by reference; Low, 
medium, and high size thresholds for detection can be adjusted.

1. In clang::​format::​internal::​reformat(clang::​format::​FormatStyle const 
&, llvm::​StringRef, llvm::​ArrayRef, unsigned int, 
unsigned int, unsigned int, llvm::​StringRef, 
clang::​format::​FormattingAttemptStatus 
*)::​[lambda(clang::​format::​Environment const &) (instance 5)]::​operator 
()(clang::​format::​Environment const &): A very large function call parameter 
exceeding the high threshold is passed by value. (CWE-398)

if (Style.RemoveSemicolon) {
3499  FormatStyle S = Expanded;
3500  S.RemoveSemicolon = true;

pass_by_value: Capturing variable S of type clang::format::FormatStyle (size 
572 bytes) by value, which exceeds the high threshold of 512 bytes.

3501  Passes.emplace_back([&, S](const Environment &Env) {
3502return SemiRemover(Env, S).process(/*SkipAnnotation=*/true);
3503  });
3504}

2. In clang::​format::​internal::​reformat(clang::​format::​FormatStyle const 
&, llvm::​StringRef, llvm::​ArrayRef, unsigned int, 
unsigned int, unsigned int, llvm::​StringRef, 
clang::​format::​FormattingAttemptStatus 
*)::​[lambda(clang::​format::​Environment const &) (instance 4)]::​operator 
()(clang::​format::​Environment const &): A very large function call parameter 
exceeding the high threshold is passed by value. (CWE-398)

if (Style.RemoveBracesLLVM) {
3491  FormatStyle S = Expanded;
3492  S.RemoveBracesLLVM = true;

pass_by_value: Capturing variable S of type clang::format::FormatStyle (size 
572 bytes) by value, which exceeds the high threshold of 512 bytes.

3493  Passes.emplace_back([&, S](const Environment &Env) {
3494return BracesRemover(Env, S).process(/*SkipAnnotation=*/true);
3495  });
3496}

3. In clang::​format::​internal::​reformat(clang::​format::​FormatStyle const 
&, llvm::​StringRef, llvm::​ArrayRef, unsigned int, 
unsigned int, unsigned int, llvm::​StringRef, 
clang::​format::​FormattingAttemptStatus 
*)::​[lambda(clang::​format::​Environment const &) (instance 3)]::​operator 
()(clang::​format::​Environment const &): A very large function call parameter 
exceeding the high threshold is passed by value. (CWE-398)

if (Style.InsertBraces) {
3483  FormatStyle S = Expanded;
3484  S.InsertBraces = true;

pass_by_value: Capturing variable S of type clang::format::FormatStyle (size 
572 bytes) by value, which exceeds the high threshold of 512 bytes.

3485  Passes.emplace_back([&, S](const Environment &Env) {
3486return BracesInserter(Env, S).process(/*SkipAnnotation=*/true);
3487  });
3488}

In D149647#4316169 , @erichkeane 
wrote:

> In D149647#4316134 , 
> @MyDeveloperDay wrote:
>
>> Can you link to the Coverity issue so we can see what it was complaining 
>> about?
>
> Unfortunately this is from an internal-run of Coverity that Intel is running. 
>  I'd be shocked if it doesn't appear on the open-source/LLVM version though?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149647

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


[PATCH] D149193: [Driver] Add -dumpdir and change -gsplit-dwarf .dwo names for linking

2023-05-03 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

I guess my main question is: What's the motivation for implementing this? Do 
you have a need/use for this? (it doesn't seem to be motivated by GCC 
compatibility - as discussed, looks like we're diverging in a bunch of ways 
from their behavior and the argument made that these are "developer" flags, so 
not a stable/compatible interface used across both compilers)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149193

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


[PATCH] D149647: [NFC][Clang]Fix static analyzer tool remarks about large copies by value

2023-05-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

ok! I'm not a massive lambda expert, but aren't we passing by  const reference 
e.g. `const FormatStyle &Style`, can someone give me a lession as to why it 
thinks its by value? maybe I'm looking at the wrong "pass by"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149647

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


[PATCH] D148665: Change -fsanitize=function to place two words before the function entry

2023-05-03 Thread Peter Smith via Phabricator via cfe-commits
peter.smith added a comment.

My apologies for not responding. If I've got this right there are 4 related 
patches:
D148573  (approved)
D148785  Use type hashes rather than RTTI 
D148827  support C
D148665  (this one)

My initial impressions is that this makes -fsanitize=function look more like 
-fsanitize=kcfi which makes it accessible from C and available to more targets. 
Is there anything that we lose in the process of making these changes? For 
example I would expect RTTI to have more information available than a type 
hash, although this might not make any functional difference.

I'll try and look over the next few days and leave some comments, apologies a 
bit busy at work at the moment so I can't promise anything speedy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148665

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


[PATCH] D149647: [NFC][Clang]Fix static analyzer tool remarks about large copies by value

2023-05-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/Format.cpp:3486-3489
+  Expanded.InsertBraces = true;
+  Passes.emplace_back([&](const Environment &Env) {
+return BracesInserter(Env, Expanded).process(/*SkipAnnotation=*/true);
   });

owenpan wrote:
> tahonermann wrote:
> > How about using an init capture instead? This will suffice to avoid one of 
> > the copies but means that `InsertBraces` doesn't get set until the lambda 
> > is invoked. I wouldn't expect that to matter though.
> I'm not sure if it's worth the trouble, but if I really had to bother, I 
> would do something like the above.
Apart from perhaps the unnecessary copying from Expanded -> S.. doesn't this 
bascially do the same without us having to remember to put Expanded back 
afterwards? I don't see how using Expanded is any different from using S (other 
than the copy)

```
if (Style.InsertBraces) {
  FormatStyle S = Expanded;
  S.InsertBraces = true;
  Passes.emplace_back([&](const Environment &Env) {
return BracesInserter(Env, S).process(/*SkipAnnotation=*/true);
  });
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149647

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


  1   2   >