[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

I'd really use some help with the buildbot failures because I can't even figure 
out how to reproduce them:

https://lab.llvm.org/buildbot#builders/169/builds/13146
https://lab.llvm.org/buildbot#builders/70/builds/29330
https://lab.llvm.org/buildbot#builders/37/builds/17758
https://lab.llvm.org/buildbot#builders/77/builds/23089
https://lab.llvm.org/buildbot#builders/19/builds/13338


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137024

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

[PATCH] D136846: [Driver] Add -fsample-profile-use-profi

[Zhang Haoyu via Phabricator via cfe-commits]

HaoyuZhang updated this revision to Diff 472219.
HaoyuZhang added a comment.

1. Modify the DocBrief contents for fsample-profile-use-profi.
2. Add a checking for -fsample-profile-use-profi that this args only allowed 
with -fprofile-sample-use (has a profile).
3. Modify the test case and change the directory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136846

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/pgo-sample-use-profi.c


Index: clang/test/Driver/pgo-sample-use-profi.c
===
--- /dev/null
+++ clang/test/Driver/pgo-sample-use-profi.c
@@ -0,0 +1,5 @@
+// Test if profi flat is enabled in frontend as user-facing feature.
+//
+// RUN: %clang -c -fsample-profile-use-profi 
-fprofile-sample-use=%S/../CodeGen/Inputs/pgo-sample.prof -### %s 2>&1 | 
FileCheck %s
+
+// CHECK: "-mllvm" "-sample-profile-use-profi"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5760,6 +5760,17 @@
 
   Args.AddLastArg(CmdArgs, options::OPT_fclang_abi_compat_EQ);
 
+  if (Args.hasArg(options::OPT_fsample_profile_use_profi)) {
+if (Args.hasArg(options::OPT_fprofile_sample_use_EQ)) {
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back("-sample-profile-use-profi");
+} else {
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << "-fsample-profile-use-profi"
+  << "fprofile-sample-use=";
+}
+  }
+
   // Add runtime flag for PS4/PS5 when PGO, coverage, or sanitizers are 
enabled.
   if (RawTriple.isPS() &&
   !Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1248,6 +1248,13 @@
as cold. Otherwise, treat callsites without profile samples as 
if
we have no profile}]>,
MarshallingInfoFlag>;
+def fsample_profile_use_profi : Flag<["-"], "fsample-profile-use-profi">,
+Flags<[NoXarchOption, CC1Option]>, Group,
+HelpText<"Use profi to infer block and edge counts.">,
+DocBrief<[{Infer block and edge counts. If the profiles have errors or 
missing
+   block caused by sampling, profile inference (profi) can 
converting
+   basic block counts to branch probabilites to fix them by 
extended
+   and re-engineered classic MCMF (min-cost max-flow) approach.}]>;
 def fno_profile_sample_accurate : Flag<["-"], "fno-profile-sample-accurate">,
   Group, Flags<[NoXarchOption]>;
 def fauto_profile : Flag<["-"], "fauto-profile">, Group,


Index: clang/test/Driver/pgo-sample-use-profi.c
===
--- /dev/null
+++ clang/test/Driver/pgo-sample-use-profi.c
@@ -0,0 +1,5 @@
+// Test if profi flat is enabled in frontend as user-facing feature.
+//
+// RUN: %clang -c -fsample-profile-use-profi -fprofile-sample-use=%S/../CodeGen/Inputs/pgo-sample.prof -### %s 2>&1 | FileCheck %s
+
+// CHECK: "-mllvm" "-sample-profile-use-profi"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5760,6 +5760,17 @@
 
   Args.AddLastArg(CmdArgs, options::OPT_fclang_abi_compat_EQ);
 
+  if (Args.hasArg(options::OPT_fsample_profile_use_profi)) {
+if (Args.hasArg(options::OPT_fprofile_sample_use_EQ)) {
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back("-sample-profile-use-profi");
+} else {
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << "-fsample-profile-use-profi"
+  << "fprofile-sample-use=";
+}
+  }
+
   // Add runtime flag for PS4/PS5 when PGO, coverage, or sanitizers are enabled.
   if (RawTriple.isPS() &&
   !Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1248,6 +1248,13 @@
as cold. Otherwise, treat callsites without profile samples as if
we have no profile}]>,
MarshallingInfoFlag>;
+def fsample_profile_use_profi : Flag<["-"], "fsample-profile-use-profi">,
+Flags<[NoXarchOption, CC1Option]>, Group,
+HelpText<"Use profi to infer block and edge counts.">,
+DocBrief<[{Infer block and edge counts. If the profiles have errors or missing
+   block caused by sampling, profile inference (profi) can converting
+   basic block counts to branch

[PATCH] D136846: [Driver] Add -fsample-profile-use-profi

[Zhang Haoyu via Phabricator via cfe-commits]

HaoyuZhang added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1254
+HelpText<"Use profi to infer block and edge counts.">,
+DocBrief<[{Profi - a flow-based profile inference algorithm is an extended
+   and significantly re-engineered classic MCMF (min-cost max-flow)

hans wrote:
> I have to say, just reading this text I don't understand what it does.
> 
> I think a good description would start with "Infer block and edge counts " 
> and then some kind of summary of how it does that.
> 
> I assume profile info is still needed for this (that's the input, right?) 
> That should probably also be explained, and maybe we should warn when using 
> -fsample-profile-use-profi without -fprofile-sample-use?
> 
> 
> My main concern is that there's no documentation for this. How is a user 
> supposed to learn about this feature and how it works? Why can't someone add 
> something to 
> https://clang.llvm.org/docs/UsersManual.html#profile-guided-optimization ? 
> Once that is figured out, describing what the option does will probably be 
> easy.
Sorry for the unclear description of the DocBrief and I have do some 
modification. 

A checking has been added for ensuring that -fsample-profile-use-profi is only 
allowed with fprofile-sample-use. Otherwise, there will be an error.

About the document in above link, do you want me to add some contents about 
using profi after the patch or invite the author of profi?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136846

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

[PATCH] D137059: [Driver] [Modules] Introduce -fsave-std-c++-module-file= to specify the path of the module file (2/2)

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D137059#3898239 , @ChuanqiXu wrote:

> In D137059#3896661 , @dblaikie 
> wrote:
>
>> Could you link to the email/discourse discussion about supporting this mode 
>> (I think you've linked it in other discussions, be good to have it for 
>> reference here & Probably in the other review)? (I'm wondering if we need a 
>> new flag for this, or if it'll be OK to change the driver behavior to always 
>> coalesce the .cppm->.pcm->.o path into a single step, for instance - I 
>> realize this is a somewhat breaking change but may be acceptable given that 
>> modules aren't widely deployed yet)
>
> Done. From my reading, in that discourse discussing, we're not talking about 
> to add the new flags. I add the flag since I don't want the `.pcm` file 
> pollutes the user space accidentally.
>
>> if it'll be OK to change the driver behavior to always coalesce the 
>> .cppm->.pcm->.o path into a single step
>
> I am not sure what you mean. Do you talk about to forbidden the original 
> 2-phase compilation model? If so, I think it is definitely the wrong 
> direction. The 2-phase compilation model should be the correct direction in 
> the long term since it has higher parallelism.

I am not convinced about this second point as motivation for this direction; it 
comes with some significant resource tradeoffs (compared with the proposed 
[near] future version of producing the PCM and the object from one invocation 
of the FE):

- it requires multiple instantiations of the FE
- it blocks the objective of reducing the content of module interfaces (so that 
they only contain the information that pertains to the interface) - since 
requiring source -> pcm, pcm -> object means that the PCM has to contain all 
the information necessary to generate the object.
- in terms of parallelism, the interface PCM has to be generated and 
distributed - the parsing and serialisation has to be complete before the PCM 
can be distributed; that process is the same regardless of whether the FE 
invocation also produces an object.

So, I would suggest that we would move to a single invocation of the compiler 
to produce the PCM and object as the default; if the user has a specific reason 
to want to do the two jobs separately then thay could still do so ( 
-fmodule-only / --precompile ) at the expense of two invocations as now,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137059

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

[PATCH] D127284: [clang-repl] Support statements on global scope in incremental mode.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev updated this revision to Diff 47.
v.g.vassilev marked 4 inline comments as done.
v.g.vassilev added a comment.

Address comments.


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

https://reviews.llvm.org/D127284

Files:
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DeclNodes.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/Template.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ModuleBuilder.cpp
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Interpreter/disambiguate-decl-stmt.cpp
  clang/test/Interpreter/execute-stmts.cpp
  clang/test/Interpreter/stmt-serialization.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp

Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -126,12 +126,7 @@
 
   // FIXME: Add support for wrapping and running statements.
   auto R2 = Interp->Parse("var1++; printf(\"var1 value %d\\n\", var1);");
-  EXPECT_FALSE(!!R2);
-  using ::testing::HasSubstr;
-  EXPECT_THAT(DiagnosticsOS.str(),
-  HasSubstr("error: unknown type name 'var1'"));
-  auto Err = R2.takeError();
-  EXPECT_EQ("Parsing failed.", llvm::toString(std::move(Err)));
+  EXPECT_TRUE(!!R2);
 }
 
 TEST(InterpreterTest, UndoCommand) {
Index: clang/test/Interpreter/stmt-serialization.cpp
===
--- /dev/null
+++ clang/test/Interpreter/stmt-serialization.cpp
@@ -0,0 +1,19 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++20 -fincremental-extensions -fmodules-cache-path=%t \
+// RUN:-x c++ %s -verify
+// expected-no-diagnostics
+
+#pragma clang module build TopLevelStmt
+module TopLevelStmt { module Statements {} }
+#pragma clang module contents
+
+#pragma clang module begin TopLevelStmt.Statements
+extern "C" int printf(const char*,...);
+int i = 0;
+i++;
+#pragma clang module end /*TopLevelStmt.Statements*/
+#pragma clang module endbuild /*TopLevelStmt*/
+
+#pragma clang module import TopLevelStmt.Statements
+
+printf("Value of i is '%d'", i);
Index: clang/test/Interpreter/execute-stmts.cpp
===
--- /dev/null
+++ clang/test/Interpreter/execute-stmts.cpp
@@ -0,0 +1,34 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+// RUN: cat %s | clang-repl -Xcc -Xclang -Xcc  -verify | FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only -verify -fincremental-extensions %s
+
+// expected-no-diagnostics
+
+extern "C" int printf(const char*,...);
+
+// FIXME: Support template instantiation calls.
+//template  T call() { printf("call\n"); return T(); }
+//call();
+ C: call
+
+int i = 1;
+++i;
+printf("i = %d\n", i);
+// CHECK: i = 2
+
+namespace Ns { void f(){ i++; } }
+Ns::f();
+
+void g() { ++i; }
+g();
+::g();
+
+printf("i = %d\n", i);
+// CHECK-NEXT: i = 5
+
+for (; i > 4; --i) printf("i = %d\n", i);
+// CHECK-NEXT: i = 5
+
+int j = i; printf("j = %d\n", j);
+// CHECK-NEXT: j = 4
Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- /dev/null
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -0,0 +1,52 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+// RUN: cat %s | clang-repl -Xcc -Xclang -Xcc -verify | FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only -verify -fincremental-extensions %s
+
+// expected-no-diagnostics
+
+extern "C" int printf(const char*,...);
+
+// Decls which are hard to disambiguate
+
+// FIXME: Support operators.
+// struct S1 { operator int(); };
+// S1::operator int() { return 0; }
+
+
+// Dtors
+using I = int;
+I x = 10;
+x.I::~I();
+x = 20;
+
+// Ctors
+
+// FIXME: Support deduction guide
+// template struct A { A(); A(T); };
+// A() -> A;
+
+struct S2 { S2(); };
+S2::S2() = default;
+
+namespace N { struct S { S(); }; }
+N::S::S() { printf("N::S::S()\n"); }
+N::S s;
+// CHECK: N::S::S()
+
+namespace Ns {namespace Ns { void Ns(); void Fs();}}
+void Ns::Ns::Ns() { printf("void Ns::Ns::Ns()\n"); }
+void Ns::Ns::Fs() {}
+
+Ns::Ns::Fs();
+Ns::Ns::Ns();
+// CHECK-NEXT: void Ns::Ns::Ns()
+
+struct A

[PATCH] D137059: [Driver] [Modules] Introduce -fsave-std-c++-module-file= to specify the path of the module file (2/2)

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D137059#3898463 , @iains wrote:

> In D137059#3898239 , @ChuanqiXu 
> wrote:
>
>> In D137059#3896661 , @dblaikie 
>> wrote:
>>
>>> Could you link to the email/discourse discussion about supporting this mode 
>>> (I think you've linked it in other discussions, be good to have it for 
>>> reference here & Probably in the other review)? (I'm wondering if we need a 
>>> new flag for this, or if it'll be OK to change the driver behavior to 
>>> always coalesce the .cppm->.pcm->.o path into a single step, for instance - 
>>> I realize this is a somewhat breaking change but may be acceptable given 
>>> that modules aren't widely deployed yet)
>>
>> Done. From my reading, in that discourse discussing, we're not talking about 
>> to add the new flags. I add the flag since I don't want the `.pcm` file 
>> pollutes the user space accidentally.
>>
>>> if it'll be OK to change the driver behavior to always coalesce the 
>>> .cppm->.pcm->.o path into a single step
>>
>> I am not sure what you mean. Do you talk about to forbidden the original 
>> 2-phase compilation model? If so, I think it is definitely the wrong 
>> direction. The 2-phase compilation model should be the correct direction in 
>> the long term since it has higher parallelism.
>
> I am not convinced about this second point as motivation for this direction; 
> it comes with some significant resource tradeoffs (compared with the proposed 
> [near] future version of producing the PCM and the object from one invocation 
> of the FE):
>
> - it requires multiple instantiations of the FE
> - it blocks the objective of reducing the content of module interfaces (so 
> that they only contain the information that pertains to the interface) - 
> since requiring source -> pcm, pcm -> object means that the PCM has to 
> contain all the information necessary to generate the object.
> - in terms of parallelism, the interface PCM has to be generated and 
> distributed - the parsing and serialisation has to be complete before the PCM 
> can be distributed; that process is the same regardless of whether the FE 
> invocation also produces an object.
>
> So, I would suggest that we would move to a single invocation of the compiler 
> to produce the PCM and object as the default; if the user has a specific 
> reason to want to do the two jobs separately then thay could still do so ( 
> -fmodule-only / --precompile ) at the expense of two invocations as now,



> (so that they only contain the information that pertains to the interface)

No, we can't do this. It hurts the performance.

> it requires multiple instantiations of the FE

Agreed. But if we care about this, I think it may be best to allow the current 
2 phase compilation model only.  And we forbid the compilation from module unit 
to object files directly. This is cleanest approach.

> in terms of parallelism, the interface PCM has to be generated and 
> distributed - the parsing and serialisation has to be complete before the PCM 
> can be distributed; that process is the same regardless of whether the FE 
> invocation also produces an object.

I think the distribution doesn't matter with parallelism. For parallelism, I 
mean, for the scan-based build systems, the compilation of A must wait until 
the dependent module B compiles to object files, which is significantly worse 
than the 2 phase compilation.

---

> So, I would suggest that we would move to a single invocation of the compiler 
> to produce the PCM and object as the default;

So the question would be where is the destination place? And if we would offer 
an option to allow the user to specify the place? This question is discussed in 
https://reviews.llvm.org/D137058.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137059

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

[PATCH] D121593: [clangd][WIP] Provide clang-include-cleaner

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D121593#3896892 , @Abpostelnicu 
wrote:

> was there any progress on this so far? This would be really useful to be a 
> clang-tidy check.

My impression is that it's actively being worked on: 
https://reviews.llvm.org/search/query/l6_2DSC3T0tJ/#R


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121593

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

[PATCH] D137058: [Driver] [Modules] Support -fsave-std-c++-module-file (1/2)

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D137058#3896999 , @dblaikie wrote:

> @ben.boeckel maybe this'd be the place to discuss the motivation for this 
> feature (picking up from your comment here: 
> https://reviews.llvm.org/D134267#3892629 )
>
> admittedly there is a flag for specifying the filename for Split DWARF too, 
> but it's not a terribly official thing (doesn't have a '-f', '-g', etc prefix)

Although I agreed it would be better to wait for @ben.boeckel to answer the 
question, I still want to say "why we would offer something only if it is 
**terribly** bad". It looks like if it is normally bad, then we won't handle it 
and you need to workaround for it. I feel bad for the strategy.
Why can't we give some small help to make their lives easier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137058

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

[PATCH] D136930: [RISCV] Support -mcpu/mtune=native

[Wang Pengcheng via Phabricator via cfe-commits]

pcwang-thead updated this revision to Diff 472228.
pcwang-thead added a comment.

- Add release note.
- Add tests to `riscv-cpus.c`.
- Add `getRISCVTargetCPU`.
- Make diagnostic more exact for `native`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136930

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Arch/RISCV.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/riscv-cpus.c

Index: clang/test/Driver/riscv-cpus.c
===
--- clang/test/Driver/riscv-cpus.c
+++ clang/test/Driver/riscv-cpus.c
@@ -7,6 +7,10 @@
 // MCPU-ROCKET64: "-nostdsysteminc" "-target-cpu" "rocket-rv64"
 // MCPU-ROCKET64: "-target-feature" "+64bit"
 
+// We cannot check much for -mcpu=native, but it should be replaced by a valid CPU string.
+// RUN: %clang --target=riscv64 -### -c %s 2>&1 -mcpu=native | FileCheck -check-prefix=MCPU-NATIVE %s
+// MCPU-NATIVE-NOT: "-target-cpu" "native"
+
 // RUN: %clang --target=riscv32 -### -c %s 2>&1 -mtune=rocket-rv32 | FileCheck -check-prefix=MTUNE-ROCKET32 %s
 // MTUNE-ROCKET32: "-tune-cpu" "rocket-rv32"
 
@@ -26,6 +30,10 @@
 // RUN: %clang --target=riscv64 -### -c %s 2>&1 -mtune=rocket | FileCheck -check-prefix=MTUNE-ROCKET-64 %s
 // MTUNE-ROCKET-64: "-tune-cpu" "rocket"
 
+// We cannot check much for -mtune=native, but it should be replaced by a valid CPU string.
+// RUN: %clang --target=riscv64 -### -c %s 2>&1 -mtune=native | FileCheck -check-prefix=MTUNE-NATIVE %s
+// MTUNE-NATIVE-NOT: "-tune-cpu" "native"
+
 // mcpu with default march
 // RUN: %clang --target=riscv64 -### -c %s 2>&1 -mcpu=sifive-e20 | FileCheck -check-prefix=MCPU-SIFIVE-E20 %s
 // MCPU-SIFIVE-E20: "-nostdsysteminc" "-target-cpu" "sifive-e20"
@@ -130,10 +138,10 @@
 // Check failed cases
 
 // RUN: %clang --target=riscv32 -### -c %s 2>&1 -mcpu=generic-rv321 | FileCheck -check-prefix=FAIL-MCPU-NAME %s
-// FAIL-MCPU-NAME: error: the clang compiler does not support '-mcpu=generic-rv321'
+// FAIL-MCPU-NAME: error: unsupported argument 'generic-rv321' to option '-mcpu='
 
 // RUN: %clang --target=riscv32 -### -c %s 2>&1 -mcpu=generic-rv32 -march=rv64i | FileCheck -check-prefix=MISMATCH-ARCH %s
-// MISMATCH-ARCH: error: the clang compiler does not support '-mcpu=generic-rv32'
+// MISMATCH-ARCH: error: unsupported argument 'generic-rv32' to option '-mcpu='
 
 // RUN: %clang --target=riscv32 -### -c %s 2>&1 -mcpu=generic-rv64 | FileCheck -check-prefix=MISMATCH-MCPU %s
-// MISMATCH-MCPU: error: the clang compiler does not support '-mcpu=generic-rv64'
+// MISMATCH-MCPU: error: unsupported argument 'generic-rv64' to option '-mcpu='
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -12,6 +12,7 @@
 #include "Arch/M68k.h"
 #include "Arch/Mips.h"
 #include "Arch/PPC.h"
+#include "Arch/RISCV.h"
 #include "Arch/Sparc.h"
 #include "Arch/SystemZ.h"
 #include "Arch/VE.h"
@@ -432,9 +433,7 @@
   return "ck810";
   case llvm::Triple::riscv32:
   case llvm::Triple::riscv64:
-if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
-  return A->getValue();
-return "";
+return riscv::getRISCVTargetCPU(Args, T);
 
   case llvm::Triple::bpfel:
   case llvm::Triple::bpfeb:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2187,7 +2187,10 @@
 
   if (const Arg *A = Args.getLastArg(options::OPT_mtune_EQ)) {
 CmdArgs.push_back("-tune-cpu");
-CmdArgs.push_back(A->getValue());
+if (strcmp(A->getValue(), "native") == 0)
+  CmdArgs.push_back(Args.MakeArgString(llvm::sys::getHostCPUName()));
+else
+  CmdArgs.push_back(A->getValue());
   }
 }
 
Index: clang/lib/Driver/ToolChains/Arch/RISCV.h
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.h
+++ clang/lib/Driver/ToolChains/Arch/RISCV.h
@@ -26,6 +26,8 @@
   const llvm::Triple &Triple);
 StringRef getRISCVArch(const llvm::opt::ArgList &Args,
const llvm::Triple &Triple);
+std::string getRISCVTargetCPU(const llvm::opt::ArgList &Args,
+  const llvm::Triple &Triple);
 } // end namespace riscv
 } // namespace tools
 } // end namespace driver
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -16,6 +16,7 @@
 #include "llvm/ADT/Optional.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/Error.h"
+#include "l

[PATCH] D135937: [X86] Support -march=raptorlake, meteorlake

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe added a comment.

In D135937#3898228 , @FreddyYe wrote:

> For saving capacity of ProcessorSubtypes, gcc decided to not support part of 
> compiler features of these two cpus:
>
>   __builtin_cpu_is("meteorlake")
>   __attribute__((target("arch=raptorlake")))
>   ... some others I don't know.
>
> Gcc's related patch:
> raptorlake 
> 
>  and meteorlake 
> 
> Updated to align with gcc first. Welcome opinions and review!

My opinion is to aligin with gcc first: supported basic -march=xxx only for 
these two cpus and to support the features mentioned above in the future if 
needed. Moreover, these two cpus may have dedicated AdditionalTuning features 
in the future, we can also add at that time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135937

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

[PATCH] D137059: [Driver] [Modules] Introduce -fsave-std-c++-module-file= to specify the path of the module file (2/2)

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D137059#3898482 , @ChuanqiXu wrote:

> In D137059#3898463 , @iains wrote:
>
>> In D137059#3898239 , @ChuanqiXu 
>> wrote:
>>
>>> In D137059#3896661 , @dblaikie 
>>> wrote:
>>>
 Could you link to the email/discourse discussion about supporting this 
 mode (I think you've linked it in other discussions, be good to have it 
 for reference here & Probably in the other review)? (I'm wondering if we 
 need a new flag for this, or if it'll be OK to change the driver behavior 
 to always coalesce the .cppm->.pcm->.o path into a single step, for 
 instance - I realize this is a somewhat breaking change but may be 
 acceptable given that modules aren't widely deployed yet)
>>>
>>> Done. From my reading, in that discourse discussing, we're not talking 
>>> about to add the new flags. I add the flag since I don't want the `.pcm` 
>>> file pollutes the user space accidentally.
>>>
 if it'll be OK to change the driver behavior to always coalesce the 
 .cppm->.pcm->.o path into a single step
>>>
>>> I am not sure what you mean. Do you talk about to forbidden the original 
>>> 2-phase compilation model? If so, I think it is definitely the wrong 
>>> direction. The 2-phase compilation model should be the correct direction in 
>>> the long term since it has higher parallelism.
>>
>> I am not convinced about this second point as motivation for this direction; 
>> it comes with some significant resource tradeoffs (compared with the 
>> proposed [near] future version of producing the PCM and the object from one 
>> invocation of the FE):
>>
>> - it requires multiple instantiations of the FE
>> - it blocks the objective of reducing the content of module interfaces (so 
>> that they only contain the information that pertains to the interface) - 
>> since requiring source -> pcm, pcm -> object means that the PCM has to 
>> contain all the information necessary to generate the object.
>> - in terms of parallelism, the interface PCM has to be generated and 
>> distributed - the parsing and serialisation has to be complete before the 
>> PCM can be distributed; that process is the same regardless of whether the 
>> FE invocation also produces an object.
>>
>> So, I would suggest that we would move to a single invocation of the 
>> compiler to produce the PCM and object as the default; if the user has a 
>> specific reason to want to do the two jobs separately then thay could still 
>> do so ( -fmodule-only / --precompile ) at the expense of two invocations as 
>> now,
>
>
>
>> (so that they only contain the information that pertains to the interface)
>
> No, we can't do this. It hurts the performance.
>
>> it requires multiple instantiations of the FE
>
> Agreed. But if we care about this, I think it may be best to allow the 
> current 2 phase compilation model only.  And we forbid the compilation from 
> module unit to object files directly. This is cleanest approach.
>
>> in terms of parallelism, the interface PCM has to be generated and 
>> distributed - the parsing and serialisation has to be complete before the 
>> PCM can be distributed; that process is the same regardless of whether the 
>> FE invocation also produces an object.
>
> I think the distribution doesn't matter with parallelism. For parallelism, I 
> mean, for the scan-based build systems, the compilation of A must wait until 
> the dependent module B compiles to object files, which is significantly worse 
> than the 2 phase compilation.

Not sure what you mean here;  If there is only one user of a PCM then it does 
not need to be produced (waste of disk space and CPU cycles);
If there are many uses of it (as we might expect in a massively parallel 
distributed build system) then distributing the PCM is important and its 
availability predicates  progress of other builds - from previous discussions 
in WG21 there are users that care very much about the size of distributed 
artefacts.

> ---
>
>> So, I would suggest that we would move to a single invocation of the 
>> compiler to produce the PCM and object as the default;
>
> So the question would be where is the destination place? And if we would 
> offer an option to allow the user to specify the place? This question is 
> discussed in https://reviews.llvm.org/D137058.

Having a mechanism to specify the place for the file is fine by me ( I was only 
commenting on the motivation point for separate pcm and object phases ).

(I think we should move this discussion somewhere else, again - unless it is 
considered a key factor in deciding on this patch, I have no further comments).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137059

___
cfe-commits mailing list

[PATCH] D102693: Do not create LLVM IR `constant`s for objects with dynamic initialisation

[Suyog Sarda via Phabricator via cfe-commits]

ssarda added a comment.
Herald added a project: All.

This patch is causing issue mentioned in 
https://discourse.llvm.org/t/sema-section-type-conflict/66000

@chill can you check once?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102693

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

[PATCH] D137059: [Driver] [Modules] Introduce -fsave-std-c++-module-file= to specify the path of the module file (2/2)

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

> Having a mechanism to specify the place for the file is fine by me ( I was 
> only commenting on the motivation point for separate pcm and object phases ).
>
> (I think we should move this discussion somewhere else, again - unless it is 
> considered a key factor in deciding on this patch, I have no further 
> comments).

Yeah, agreed. Let's avoid the repeating ourselves : )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137059

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

[PATCH] D133757: [clangd] Perform system include extraction inside CommandMangler

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 472235.
nridge added a comment.

Improve handling of config file diagnostics in lit test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133757

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/CompileCommands.h
  clang-tools-extra/clangd/GlobalCompilationDatabase.h
  clang-tools-extra/clangd/QueryDriverDatabase.cpp
  clang-tools-extra/clangd/test/system-include-extractor.test
  clang-tools-extra/clangd/tool/Check.cpp

Index: clang-tools-extra/clangd/tool/Check.cpp
===
--- clang-tools-extra/clangd/tool/Check.cpp
+++ clang-tools-extra/clangd/tool/Check.cpp
@@ -100,9 +100,9 @@
 Config::current().CompileFlags.CDBSearch.FixedCDBPath;
 std::unique_ptr BaseCDB =
 std::make_unique(CDBOpts);
-BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs),
- std::move(BaseCDB));
 auto Mangler = CommandMangler::detect();
+Mangler.SystemIncludeExtractor =
+getSystemIncludeExtractor(llvm::makeArrayRef(Opts.QueryDriverGlobs));
 if (Opts.ResourceDir)
   Mangler.ResourceDir = *Opts.ResourceDir;
 auto CDB = std::make_unique(
Index: clang-tools-extra/clangd/test/system-include-extractor.test
===
--- clang-tools-extra/clangd/test/system-include-extractor.test
+++ clang-tools-extra/clangd/test/system-include-extractor.test
@@ -42,7 +42,9 @@
 # RUN: sed -E -e 's|"file://([A-Z]):/|"file:///\1:/|g' %t.test.1 > %t.test
 
 # Bless the mock driver we've just created so that clangd can execute it.
-# RUN: clangd -lit-test -query-driver="**.test,**.sh" < %t.test | FileCheck -strict-whitespace %t.test
+# Note: include clangd's stderr in the FileCheck input with "2>&1" so that we
+# can match output lines like "ASTWorker building file"
+# RUN: clangd -lit-test -query-driver="**.test,**.sh" < %t.test 2>&1 | FileCheck -strict-whitespace %t.test
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}
 ---
 {
@@ -57,10 +59,25 @@
 }
   }
 }
+# Look for the "ASTWorker building file" line so that the subsequent diagnostics
+# that are matches are for the C++ source file and not a config file.
+# CHECK: ASTWorker building file
 # CHECK:   "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:   "params": {
 # CHECK-NEXT: "diagnostics": [],
+# CHECK-NEXT: "uri": "file://INPUT_DIR/the-file.cpp",
 ---
 {"jsonrpc":"2.0","id":1,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0","method":"exit"}
+
+# Generate a different compile_commands.json which does not point to the mock driver
+# RUN: echo '[{"directory": "%/t.dir", "command": "gcc the-file.cpp -nostdinc --sysroot /my/sysroot/path -isysroot=/isysroot", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+
+# Generate a clangd config file which points to the mock driver instead
+# RUN: echo 'CompileFlags:' > %t.dir/.clangd
+# RUN: echo '  Compiler: my_driver.sh' >> %t.dir/.clangd
+
+# Run clangd a second time, to make sure it picks up the driver name from the config file
+# Note, we need to pass -enable-config because -lit-test otherwise disables it
+# RUN: clangd -lit-test -enable-config -query-driver="**.test,**.sh" < %t.test 2>&1 | FileCheck -strict-whitespace %t.test
Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -315,24 +315,20 @@
 /// Extracts system includes from a trusted driver by parsing the output of
 /// include search path and appends them to the commands coming from underlying
 /// compilation database.
-class QueryDriverDatabase : public DelegatingCDB {
+class SystemIncludeExtractor {
 public:
-  QueryDriverDatabase(llvm::ArrayRef QueryDriverGlobs,
-  std::unique_ptr Base)
-  : DelegatingCDB(std::move(Base)),
-QueryDriverRegex(convertGlobsToRegex(QueryDriverGlobs)) {}
+  SystemIncludeExtractor(llvm::ArrayRef QueryDriverGlobs)
+  : QueryDriverRegex(convertGlobsToRegex(QueryDriverGlobs)) {}
 
-  llvm::Optional
-  getCompileCommand(PathRef File) const override {
-auto Cmd = DelegatingCDB::getCompileCommand(File);
-if (!Cmd || Cmd->CommandLine.empty())
-  return Cmd;
+  void operator()(tooling::CompileCommand &Cmd, llvm::StringRef File) const {
+if (Cmd.CommandLine.empty())
+  return;
 
 llvm::StringRef Lang;
-for (size_t I = 0, E = Cmd->CommandLine.size(); I < E; ++I) {
-  llvm::StringRef Arg = Cmd->CommandLine[I];
+for (size_t I = 0, E = Cmd.CommandLine.size(); I < E; ++I) {
+  llvm::StringRef Arg = Cmd.CommandLine[I];
   if (Arg == "-x" && I + 1 < E)
-Lang = Cmd->Comm

[PATCH] D133757: [clangd] Perform system include extraction inside CommandMangler

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/test/system-include-extractor.test:82
+
+# Skip past the lack of diagnostics in the workspace and user config files...
+# CHECK2:   "method": "textDocument/publishDiagnostics",

nridge wrote:
> Ugh, this doesn't quite work in that whether we get diagnostics for the 
> **user** config file depends on whether it exists. It happens to on my system 
> but presumably it may not on a buildbot?
> 
> Should I create a user config file in the lit test so it reliably exists (and 
> delete it at the end)? Or is there some FileCheck magic I can employ that 
> would ignore an optional second publishDiagnostics which is not the 
> interesting one?
Ok, I figured out a way to handle this properly:

 * I included clangd's stderr in the FileCheck input so we could match lines 
from stderr like "ASTWorker building file"
 * By first looking for "ASTWorker building file" and then the next 
publishDiagnostics, we can be sure that the diagnostics pertain to the source 
file and not a config file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133757

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

[PATCH] D136796: [llvm-objdump][Offload] Use common offload extraction method

[James Henderson via Phabricator via cfe-commits]

jhenderson added inline comments.



Comment at: llvm/test/tools/llvm-objdump/Offloading/elf_dynamic.test:1
-## Check that we can dump an offloading binary directly.
-# RUN: yaml2obj %S/Inputs/binary.yaml -o %t.bin
-# RUN: llvm-objdump --offloading %t.bin | FileCheck %s --match-full-lines 
--strict-whitespace --implicit-check-not={{.}}
-
 ## Check that we can dump an offloading binary inside of an ELF section.
+# RUN: yaml2obj %S/Inputs/binary.yaml -o %t.bin

Rather than duplicating this and the ET_REL cases, you can use yaml2obj's -D 
option to parameterise the ELF type. Rough code:

```
# RUN: yaml2obj %S/Inputs/binary.yaml -D TYPE=ET_DYN -o %t.so
# RUN: yaml2obj %S/Inputs/binary.yaml -D TYPE=ET_EXEC -o %t.bin
# RUN: yaml2obj %S/Inputs/binary.yaml -D TYPE=ET_REL -o %t.o

...
  Data: ELFDATA2LSB
  Type: [[TYPE]]
...
```



Comment at: llvm/tools/llvm-objdump/OffloadDump.cpp:63
 
-// Print out all the binaries that are contained in this buffer. If we fail
-// to parse a binary before reaching the end of the buffer emit a warning.
-if (Error Err = visitAllBinaries(Binary))
-  reportWarning("while parsing offloading files: " +
-toString(std::move(Err)),
-O.getFileName());
-  }
+  // Print out all the binaries that are contained at this buffer.
+  for (uint64_t I = 0, E = Binaries.size(); I != E; ++I)

Not sure why this was changed.



Comment at: llvm/tools/llvm-objdump/OffloadDump.cpp:76
+
+  // Print out all the binaries that are contained at this buffer.
+  for (uint64_t I = 0, E = Binaries.size(); I != E; ++I)

Should this be "contained in this buffer" rather than "at"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136796

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

[PATCH] D134410: [clang][CodeGen] Add noundef metadata to load instructions (preliminary 1 or 2)

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

In D134410#3895253 , @nlopes wrote:

> In D134410#3895077 , @nikic wrote:
>
>> In D134410#3894995 , @nlopes wrote:
>>
>>> We wanted this patch to make us switch uninitialized loads to poison at 
>>> will, since they become UB. In practice, this helps us fixing bugs in SROA 
>>> and etc without perf degradation.
>>
>> Can you elaborate on this? I don't see how this is necessary for switching 
>> uninitialized loads to poison.
>
> It's not mandatory, it's a simple way of achieving it as !noundef already 
> exists.
>
> We cannot change the default behavior of load as it would break BC.

FWIW, I don't agree with this. It's fine to change load semantics, as long as 
bitcode autoupgrade is handled correctly. I'd go even further and say that at 
least long term, any solution that does not have a plain `load` instruction 
return `poison` for uninitialized memory will be a maintenance mess.

I think the core problem that prevents us from moving in this direction is that 
we have no way to represent a frozen load at all (as `freeze(load)` will 
propagate poison before freezing). If I understand correctly, you're trying to 
solve this by making *all* loads frozen loads, and use `load !noundef` to allow 
dropping the freeze. I think this would be a very bad outcome, especially as 
we're going to lose that `!noundef` on most load speculations, and I don't 
think we want to be introducing freezes when speculating loads (e.g. during 
LICM).

I expect that the path of least resistance is going to be to support bitwise 
poison for load/store/phi/select and promote it to full-value poison on any 
other operation, allowing a freezing load to be expressed as `freeze(load)`.

Please let me know if I completely misunderstood the scheme you have in mind...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134410

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

[PATCH] D136925: [clangd] Index unscoped enums inside classes for code completion

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/index/Serialization.cpp:458
 // data. Later we may want to support some backward compatibility.
-constexpr static uint32_t Version = 17;
+constexpr static uint32_t Version = 18;
 

I don't think indexing new symbols is a **breaking** change to the index 
format, in the sense that an older index cannot be read by a newer clangd.

As such, I don't think it's necessary to increment this version.



Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:2966
   {cls("nx::Clangd1"), cls("ny::Clangd2"), cls("Clangd3"),
-   cls("na::nb::Clangd4")},
+   cls("na::nb::Clangd4"), enmConstant("na::C::Clangd5")},
   Opts);

Hmm, I don't think this type of test actually exercises the 
`isIndexedForCodeCompletion()` codepath (since we're just mocking out the index 
contents, instead of running the actual indexer).

Could we use [this 
type](https://searchfox.org/llvm/source/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp#3341-3359)
 instead?

(Sorry, this is partly my fault for not looking more carefully at what sorts of 
tests are in CodeCompleteTests.cpp before my earlier comment.)



Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:1333
   AllOf(qName("Color2::Yellow"), forCodeCompletion(false)),
+  AllOf(qName("Color3"), forCodeCompletion(true)),
+  AllOf(qName("Color3::Blue"), forCodeCompletion(true)),

nit: place after ns / ns::Black to match the order in the code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136925

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

[PATCH] D136925: [clangd] Index unscoped enums inside classes for code completion

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:2966
   {cls("nx::Clangd1"), cls("ny::Clangd2"), cls("Clangd3"),
-   cls("na::nb::Clangd4")},
+   cls("na::nb::Clangd4"), enmConstant("na::C::Clangd5")},
   Opts);

nridge wrote:
> Hmm, I don't think this type of test actually exercises the 
> `isIndexedForCodeCompletion()` codepath (since we're just mocking out the 
> index contents, instead of running the actual indexer).
> 
> Could we use [this 
> type](https://searchfox.org/llvm/source/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp#3341-3359)
>  instead?
> 
> (Sorry, this is partly my fault for not looking more carefully at what sorts 
> of tests are in CodeCompleteTests.cpp before my earlier comment.)
(Indeed, this test passes even without the change to CodeComplete.cpp.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136925

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

[PATCH] D134410: [clang][CodeGen] Add noundef metadata to load instructions (preliminary 1 or 2)

[Nuno Lopes via Phabricator via cfe-commits]

nlopes added a comment.

In D134410#3898563 , @nikic wrote:

> In D134410#3895253 , @nlopes wrote:
>
>> In D134410#3895077 , @nikic wrote:
>>
>>> In D134410#3894995 , @nlopes 
>>> wrote:
>>>
 We wanted this patch to make us switch uninitialized loads to poison at 
 will, since they become UB. In practice, this helps us fixing bugs in SROA 
 and etc without perf degradation.
>>>
>>> Can you elaborate on this? I don't see how this is necessary for switching 
>>> uninitialized loads to poison.
>>
>> It's not mandatory, it's a simple way of achieving it as !noundef already 
>> exists.
>>
>> We cannot change the default behavior of load as it would break BC.
>
> FWIW, I don't agree with this. It's fine to change load semantics, as long as 
> bitcode autoupgrade is handled correctly. I'd go even further and say that at 
> least long term, any solution that does not have a plain `load` instruction 
> return `poison` for uninitialized memory will be a maintenance mess.
>
> I think the core problem that prevents us from moving in this direction is 
> that we have no way to represent a frozen load at all (as `freeze(load)` will 
> propagate poison before freezing). If I understand correctly, you're trying 
> to solve this by making *all* loads frozen loads, and use `load !noundef` to 
> allow dropping the freeze. I think this would be a very bad outcome, 
> especially as we're going to lose that `!noundef` on most load speculations, 
> and I don't think we want to be introducing freezes when speculating loads 
> (e.g. during LICM).
>
> I expect that the path of least resistance is going to be to support bitwise 
> poison for load/store/phi/select and promote it to full-value poison on any 
> other operation, allowing a freezing load to be expressed as `freeze(load)`.
>
> Please let me know if I completely misunderstood the scheme you have in 
> mind...

You got it right. But the load we propose is not exactly a freezing load. It 
only returns `freeze poison` for uninit memory. It doesn't freeze stored values.
If we introduce a `!uninit_is_poison` flag, we can drop !noundef when hoisting 
and add `!uninit_is_poison` instead (it's implied by !noundef).

The question is what's the default for uninit memory: `freeze poison` or 
`poison`? But I think we need both. And since we need both (unless we add a 
freezing load or bitwise poison), why not keep a behavior closer to the current?
A freezing load is worse as a store+load needs to be forwarded though a freeze, 
as the load is not a NOP anymore.

Bitwise poison would be nice, but I don't know how to make it work. To make it 
work with bit-fields, we would need and/or to propagate poison bit-wise as 
well. But then we will break optimizations that transform between those and 
arithmetic. Then you start wanting that add/mul/etc also propagate poison 
bit-wise and then I don't know how to specify that semantics.  (if you want, I 
can forward you a proposal we wrote a few years ago, but we never managed to 
make sound, so it was never presented in public)

I agree that bit-wise poison for loads sounds appealing (for bit fields, load 
widening, etc), but without support in the rest of the IR, it's not worth much. 
If it becomes value-wise in the 1st operation, then I don't think we gain any 
expressivity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134410

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

[PATCH] D136090: Handle errors in expansion of response files

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

Hello,

I'm wondering if this is really working as expected/intended.

With this patch, if I do

  clang empty.c @atfileesc.txt -dry-run

on an empty file empty.c and atfileesc.txt containing just

  -I @name-with-at/

then I get the following error

  cannot open file: /repo/uabelho/main-github/llvm/atfileesc.txt
  Program aborted due to an unhandled Error:
  cannot open file: /repo/uabelho/main-github/llvm/atfileesc.txt
  PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
and include the crash backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.Program arguments: build-all/bin/clang empty.c @atfileesc.txt -dry-run
   #0 0x02fc9873 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(build-all/bin/clang+0x2fc9873)
   #1 0x02fc758e llvm::sys::RunSignalHandlers() 
(build-all/bin/clang+0x2fc758e)
   #2 0x02fc9bf6 SignalHandler(int) (build-all/bin/clang+0x2fc9bf6)
   #3 0x7f678e7ae630 __restore_rt (/lib64/libpthread.so.0+0xf630)
   #4 0x7f678bef5387 raise (/lib64/libc.so.6+0x36387)
   #5 0x7f678bef6a78 abort (/lib64/libc.so.6+0x37a78)
   #6 0x02f39d00 llvm::Error::fatalUncheckedError() const 
(build-all/bin/clang+0x2f39d00)
   #7 0x009d5e3a clang_main(int, char**) (build-all/bin/clang+0x9d5e3a)
   #8 0x7f678bee1555 __libc_start_main (/lib64/libc.so.6+0x22555)
   #9 0x009d233b _start (build-all/bin/clang+0x9d233b)
  Abort (core dumped)

Note that the file it is complaining about indeed exists:

  -> ls -l /repo/uabelho/main-github/llvm/atfileesc.txt
  -rw-r--r-- 1 uabelho eusers 18 Nov  1 11:15 
/repo/uabelho/main-github/llvm/atfileesc.txt

and as I wrote above it contains

  -> cat /repo/uabelho/main-github/llvm/atfileesc.txt
  -I @name-with-at/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136090

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

[PATCH] D82562: Implement AVX ABI Warning/error

[Jake Li via Phabricator via cfe-commits]

jajadude added a comment.
Herald added subscribers: StephenFan, pengfei.
Herald added a project: All.

   if (Callee->getReturnType()->isVectorType() && 
CGM.getContext().getTypeSize(Callee->getReturnType()) > 128) {
  }

I think this condition will make features like ext_vector_type to be warnings 
too?

e.g. Apple's  provides
`typedef __attribute__((__ext_vector_type__(3),__aligned__(16))) double 
simd_double3;`

It's a vector of 3*64= 192 bits, larger than 128




Comment at: clang/lib/CodeGen/TargetInfo.cpp:2566
+  if (Callee->getReturnType()->isVectorType() &&
+  CGM.getContext().getTypeSize(Callee->getReturnType()) > 128) {
+initFeatureMaps(CGM.getContext(), CallerMap, Caller, CalleeMap, Callee);

I think this condition will make features like 
[ext_vector_type](https://clang.llvm.org/docs/LanguageExtensions.html#vectors-and-extended-vectors)
 to be warnings too?

e.g.  Apple's `` provides
`typedef __attribute__((__ext_vector_type__(3),__aligned__(16))) double 
simd_double3;`

It's a vector of 3*64= 192 bits, larger than 128


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82562

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

[PATCH] D137153: [X86] Support -march=sierraforest, grandridge, graniterapids.

[Freddy, Ye via Phabricator via cfe-commits]

FreddyYe created this revision.
Herald added subscribers: Enna1, pengfei, hiraditya.
Herald added a project: All.
FreddyYe requested review of this revision.
Herald added projects: clang, Sanitizers, LLVM.
Herald added subscribers: llvm-commits, Sanitizers, cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137153

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Basic/Targets/X86.cpp
  clang/test/CodeGen/attr-target-mv.c
  clang/test/CodeGen/target-builtin-noerror.c
  clang/test/Driver/x86-march.c
  clang/test/Misc/target-invalid-cpu-note.c
  clang/test/Preprocessor/predefined-arch-macros.c
  compiler-rt/lib/builtins/cpu_model.c
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Support/X86TargetParser.def
  llvm/include/llvm/Support/X86TargetParser.h
  llvm/lib/Support/Host.cpp
  llvm/lib/Support/X86TargetParser.cpp
  llvm/lib/Target/X86/X86.td
  llvm/test/CodeGen/X86/cpus-intel.ll

Index: llvm/test/CodeGen/X86/cpus-intel.ll
===
--- llvm/test/CodeGen/X86/cpus-intel.ll
+++ llvm/test/CodeGen/X86/cpus-intel.ll
@@ -17,6 +17,9 @@
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=yonah 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=prescott 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=lakemont 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=sierraforest 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=grandridge 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=i686-unknown-unknown -mcpu=graniterapids 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=nocona 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=core2 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
@@ -52,6 +55,9 @@
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=tremont 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=knl 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 ; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=knm 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=sierraforest 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=grandridge 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
+; RUN: llc < %s -o /dev/null -mtriple=x86_64-unknown-unknown -mcpu=graniterapids 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ERROR --allow-empty
 
 define void @foo() {
   ret void
Index: llvm/lib/Target/X86/X86.td
===
--- llvm/lib/Target/X86/X86.td
+++ llvm/lib/Target/X86/X86.td
@@ -943,6 +943,14 @@
   list SPRFeatures =
 !listconcat(ICXFeatures, SPRAdditionalFeatures);
 
+  // Graniterapids
+  list GNRAdditionalFeatures = [FeatureAMXTILE,
+  FeatureAMXINT8,
+  FeatureAMXBF16,
+  FeatureBF16];
+  list GNRFeatures =
+!listconcat(SPRFeatures, GNRAdditionalFeatures);
+
   // Atom
   list AtomFeatures = [FeatureX87,
  FeatureCX8,
@@ -1050,6 +1058,19 @@
   list ADLFeatures =
 !listconcat(TRMFeatures, ADLAdditionalFeatures);
 
+  // Sierraforest
+  list SRFAdditionalFeatures = [FeatureCMPCCXADD,
+  FeatureAVXIFMA,
+  FeatureAVXNECONVERT,
+  FeatureAVXVNNIINT8];
+  list SRFFeatures =
+!listconcat(ADLFeatures, SRFAdditionalFeatures);
+
+  // Grandridge
+  list GRRAdditionalFeatures = [FeatureRAOINT];
+  list GRRFeatures =
+!listconcat(SRFFeatures, GRRAdditionalFeatures);
+
   // Knights Landing
   list KNLFeatures = [FeatureX87,
 FeatureCX8,
@@ -1502,6 +1523,12 @@
 ProcessorFeatures.SPRFeatures, ProcessorFeatures.SPRTuning>;
 def : ProcModel<"alderlake", AlderlakePModel,
 ProcessorFeatures.ADLFeatures, ProcessorFeatures.ADLTuning>;
+def : ProcModel<"sierraforest", SLMModel, ProcessorFeatures.SRFFeatures,
+ProcessorFeatures.TRMTuning>;
+def : ProcModel<"grandridge", SLMModel, ProcessorFeatures.GRRFeatures,
+ 

[PATCH] D137154: Adding nvvm_reflect clang builtin

[Hugh Delaney via Phabricator via cfe-commits]

hdelan created this revision.
Herald added subscribers: mattd, gchakrabarti, asavonic, hiraditya.
Herald added a project: All.
hdelan requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, jholewinski.
Herald added projects: clang, LLVM.

This patch adds __nvvm_reflect as a clang builtin for NVPTX backend. This means
that __nvvm_reflect can be used in source code in order to enable conditional 
compilation
based on compute capability and FTZ properties.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137154

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def
  clang/test/CodeGenCUDA/nvvm-reflect.cu
  llvm/include/llvm/IR/IntrinsicsNVVM.td
  llvm/lib/Target/NVPTX/NVVMReflect.cpp
  llvm/test/CodeGen/NVPTX/nvvm-reflect-opaque.ll
  llvm/test/CodeGen/NVPTX/nvvm-reflect.ll

Index: llvm/test/CodeGen/NVPTX/nvvm-reflect.ll
===
--- llvm/test/CodeGen/NVPTX/nvvm-reflect.ll
+++ llvm/test/CodeGen/NVPTX/nvvm-reflect.ll
@@ -41,7 +41,7 @@
   ret float %ret
 }
 
-declare i32 @llvm.nvvm.reflect.p0i8(i8*)
+declare i32 @llvm.nvvm.reflect(i8*)
 
 ; CHECK-LABEL: define i32 @intrinsic
 define i32 @intrinsic() {
@@ -49,7 +49,7 @@
 ; USE_FTZ_0: ret i32 0
 ; USE_FTZ_1: ret i32 1
   %ptr = tail call i8* @llvm.nvvm.ptr.constant.to.gen.p0i8.p4i8(i8 addrspace(4)* getelementptr inbounds ([11 x i8], [11 x i8] addrspace(4)* @str, i32 0, i32 0))
-  %reflect = tail call i32 @llvm.nvvm.reflect.p0i8(i8* %ptr)
+  %reflect = tail call i32 @llvm.nvvm.reflect(i8* %ptr)
   ret i32 %reflect
 }
 
Index: llvm/test/CodeGen/NVPTX/nvvm-reflect-opaque.ll
===
--- llvm/test/CodeGen/NVPTX/nvvm-reflect-opaque.ll
+++ llvm/test/CodeGen/NVPTX/nvvm-reflect-opaque.ll
@@ -41,7 +41,7 @@
   ret float %ret
 }
 
-declare i32 @llvm.nvvm.reflect.p0i8(ptr)
+declare i32 @llvm.nvvm.reflect(ptr)
 
 ; CHECK-LABEL: define i32 @intrinsic
 define i32 @intrinsic() {
@@ -49,7 +49,7 @@
 ; USE_FTZ_0: ret i32 0
 ; USE_FTZ_1: ret i32 1
   %ptr = tail call ptr @llvm.nvvm.ptr.constant.to.gen.p0i8.p4i8(ptr addrspace(4) @str)
-  %reflect = tail call i32 @llvm.nvvm.reflect.p0i8(ptr %ptr)
+  %reflect = tail call i32 @llvm.nvvm.reflect(ptr %ptr)
   ret i32 %reflect
 }
 
Index: llvm/lib/Target/NVPTX/NVVMReflect.cpp
===
--- llvm/lib/Target/NVPTX/NVVMReflect.cpp
+++ llvm/lib/Target/NVPTX/NVVMReflect.cpp
@@ -40,6 +40,7 @@
 #include 
 #include 
 #define NVVM_REFLECT_FUNCTION "__nvvm_reflect"
+#define NVVM_REFLECT_LLVM_INTRINSIC_NAME "llvm.nvvm.reflect"
 
 using namespace llvm;
 
@@ -119,6 +120,7 @@
   continue;
 Function *Callee = Call->getCalledFunction();
 if (!Callee || (Callee->getName() != NVVM_REFLECT_FUNCTION &&
+Callee->getName() != NVVM_REFLECT_LLVM_INTRINSIC_NAME &&
 Callee->getIntrinsicID() != Intrinsic::nvvm_reflect))
   continue;
 
Index: llvm/include/llvm/IR/IntrinsicsNVVM.td
===
--- llvm/include/llvm/IR/IntrinsicsNVVM.td
+++ llvm/include/llvm/IR/IntrinsicsNVVM.td
@@ -1578,7 +1578,8 @@
 Intrinsic<[], [llvm_anyptr_ty], [], "llvm.nvvm.compiler.warn">;
 
 def int_nvvm_reflect :
-  Intrinsic<[llvm_i32_ty], [llvm_anyptr_ty], [IntrNoMem], "llvm.nvvm.reflect">;
+  Intrinsic<[llvm_i32_ty], [llvm_ptr_ty], [IntrNoMem], "llvm.nvvm.reflect">,
+  ClangBuiltin<"__nvvm_reflect">;
 
 // isspacep.{const, global, local, shared}
 def int_nvvm_isspacep_const
Index: clang/test/CodeGenCUDA/nvvm-reflect.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/nvvm-reflect.cu
@@ -0,0 +1,81 @@
+// REQUIRES: nvptx-registered-target
+
+// Checking to see that __nvvm_reflect resolves to the correct llvm.nvvm.reflect
+// intrinsic
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx64-nvidia-cuda -disable-llvm-passes -S -emit-llvm -x c++ %s -o - | FileCheck %s --check-prefix=NO_NVVM_REFLECT_PASS
+
+// Prepare bitcode file to link with
+// RUN: %clang_cc1 -triple nvptx-unknown-cuda -emit-llvm-bc \
+// RUN:-disable-llvm-passes -o %t.bc %s
+
+// Checking to see if the correct values are substituted for the nvvm_reflect
+// call when llvm passes are enabled.
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx64-nvidia-cuda -target-cpu \
+// RUN:sm_50 -S -o /dev/null %s -mllvm -print-after-all 2>&1 \
+// RUN:| FileCheck %s --check-prefix=ARCH_REFLECT_1
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx64-nvidia-cuda -target-cpu \
+// RUN:sm_52 -S -o /dev/null %s -mllvm -print-after-all 2>&1 \
+// RUN:| FileCheck %s --check-prefix=ARCH_REFLECT_2
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx64-nvidia-cuda -target-cpu \
+// RUN:sm_53 -S -o /dev/null %s -mllvm -print-after-all 2>&1 \
+// RUN:| FileCheck %s --check-prefix=ARCH_REFLECT_3
+// RUN: %clang_cc1 -fcuda-is-device -

[PATCH] D137073: [clang] Fix inline builtin functions of an __asm__ renamed function with symbol prefixes

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: clang/lib/CodeGen/CGExpr.cpp:5020
+std::string AsmName;
+if (auto *A = FD->getAttr()) {
+  StringRef UserLabelPrefix =

rnk wrote:
> I think you can call CGM.getMangledName, but I might be wrong. It's worth 
> doing if straightforward.
Thanks, that seems to work just great, and simplifies things nicely!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137073

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

[PATCH] D137073: [clang] Fix inline builtin functions of an __asm__ renamed function with symbol prefixes

[Martin Storsjö via Phabricator via cfe-commits]

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

Use `CGM.getMangledName()`, simplifying the code instead of complicating it 
further.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137073

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGen/inline-builtin-asm-name.c


Index: clang/test/CodeGen/inline-builtin-asm-name.c
===
--- /dev/null
+++ clang/test/CodeGen/inline-builtin-asm-name.c
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -triple i686-windows-gnu -emit-llvm -o - %s 
-disable-llvm-optzns | FileCheck %s
+
+// CHECK: call i32 @"\01_asm_func_name.inline"
+
+// CHECK: declare dso_local i32 @"\01_asm_func_name"(ptr noundef, i32 noundef, 
ptr noundef, ptr noundef)
+
+// CHECK: define internal i32 @"\01_asm_func_name.inline"
+
+// CHECK: call i32 @__mingw_vsnprintf
+
+// CHECK: declare dso_local i32 @__mingw_vsnprintf
+
+typedef unsigned int size_t;
+
+int __mingw_vsnprintf(char *_DstBuf, size_t _MaxCount, const char *_Format, 
__builtin_va_list _ArgList);
+
+// For the real use case, "_asm_func_name" is actually "___mingw_vsnprintf", 
but it's renamed in the testcase for disambiguation.
+int vsnprintf(char *__stream, size_t __n, const char *__format, 
__builtin_va_list __local_argv) __asm__("_asm_func_name");
+
+extern __inline__ __attribute__((__always_inline__, __gnu_inline__))
+int vsnprintf(char *__stream, size_t __n, const char *__format, 
__builtin_va_list __local_argv)
+{
+  return __mingw_vsnprintf(__stream, __n, __format, __local_argv);
+}
+
+void call(const char* fmt, ...) {
+  char buf[200];
+  __builtin_va_list ap;
+  __builtin_va_start(ap, fmt);
+  vsnprintf(buf, sizeof(buf), fmt, ap);
+  __builtin_va_end(ap);
+}
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -5014,8 +5014,7 @@
 std::string NoBuiltinFD = ("no-builtin-" + FD->getName()).str();
 std::string NoBuiltins = "no-builtins";
 
-auto *A = FD->getAttr();
-StringRef Ident = A ? A->getLabel() : FD->getName();
+StringRef Ident = CGF.CGM.getMangledName(GD);
 std::string FDInlineName = (Ident + ".inline").str();
 
 bool IsPredefinedLibFunction =


Index: clang/test/CodeGen/inline-builtin-asm-name.c
===
--- /dev/null
+++ clang/test/CodeGen/inline-builtin-asm-name.c
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -triple i686-windows-gnu -emit-llvm -o - %s -disable-llvm-optzns | FileCheck %s
+
+// CHECK: call i32 @"\01_asm_func_name.inline"
+
+// CHECK: declare dso_local i32 @"\01_asm_func_name"(ptr noundef, i32 noundef, ptr noundef, ptr noundef)
+
+// CHECK: define internal i32 @"\01_asm_func_name.inline"
+
+// CHECK: call i32 @__mingw_vsnprintf
+
+// CHECK: declare dso_local i32 @__mingw_vsnprintf
+
+typedef unsigned int size_t;
+
+int __mingw_vsnprintf(char *_DstBuf, size_t _MaxCount, const char *_Format, __builtin_va_list _ArgList);
+
+// For the real use case, "_asm_func_name" is actually "___mingw_vsnprintf", but it's renamed in the testcase for disambiguation.
+int vsnprintf(char *__stream, size_t __n, const char *__format, __builtin_va_list __local_argv) __asm__("_asm_func_name");
+
+extern __inline__ __attribute__((__always_inline__, __gnu_inline__))
+int vsnprintf(char *__stream, size_t __n, const char *__format, __builtin_va_list __local_argv)
+{
+  return __mingw_vsnprintf(__stream, __n, __format, __local_argv);
+}
+
+void call(const char* fmt, ...) {
+  char buf[200];
+  __builtin_va_list ap;
+  __builtin_va_start(ap, fmt);
+  vsnprintf(buf, sizeof(buf), fmt, ap);
+  __builtin_va_end(ap);
+}
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -5014,8 +5014,7 @@
 std::string NoBuiltinFD = ("no-builtin-" + FD->getName()).str();
 std::string NoBuiltins = "no-builtins";
 
-auto *A = FD->getAttr();
-StringRef Ident = A ? A->getLabel() : FD->getName();
+StringRef Ident = CGF.CGM.getMangledName(GD);
 std::string FDInlineName = (Ident + ".inline").str();
 
 bool IsPredefinedLibFunction =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137071: [clang][Interp] Implement missing compound assign operators

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/AST/Interp/literals.cpp:553
+  static_assert(IntRem(2, 1) == 0, "");
+  static_assert(IntRem(9, 7) == 2, "");
+

```
static_assert(IntRem(9, 0) == 12, ""); // Not constexpr
static_assert(IntRem(__INT_MIN__, -1) == 12, ""); // Not constexpr
```



Comment at: clang/test/AST/Interp/literals.cpp:572-573
+  static_assert(IntDiv(12, 20) == 0, "");
+  static_assert(IntDiv(2, 1) == 2, "");
+  static_assert(IntDiv(9, 7) == 1, "");
+

Same suggestion for div as for rem



Comment at: clang/test/AST/Interp/literals.cpp:595
+  static_assert(IntMul(2, 1) == 2, "");
+  static_assert(IntMul(9, 7) == 63, "");
 };

An overflow test would be good here.


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

https://reviews.llvm.org/D137071

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

[PATCH] D136790: [Clang][Sema] Add -Wincompatible-function-pointer-types-strict

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8217
+  err_typecheck_convert_incompatible_function_pointer.Text>,
+  InGroup>, 
DefaultIgnore;
 def ext_typecheck_convert_discards_qualifiers : ExtWarn<

samitolvanen wrote:
> aaron.ballman wrote:
> > We don't typically add new off-by-default warnings because we have evidence 
> > that users don't enable them enough to be worth adding them. Is there a way 
> > we can enable this warning by default for CFI compilation units (or when 
> > the cfi sanitizer is enabled) so that it's only off by default for non-CFI 
> > users? I don't think we have any examples of doing this in the code base, 
> > so I believe this would be breaking new ground (and thus is worth thinking 
> > about more, perhaps it's a bad idea for some reason).
> > Is there a way we can enable this warning by default for CFI compilation 
> > units (or when the cfi sanitizer is enabled) so that it's only off by 
> > default for non-CFI users?
> 
> I can look into it, but I'm not sure if there's a convenient way to do that. 
> An alternative to this could be to enable the warning by default, but only 
> actually perform the check if CFI is enabled. This way non-CFI users would 
> never see the warning because this really isn't a concern for them, but CFI 
> users would still get the warning without explicitly enabling it. Or do you 
> think this behavior would be confusing?
Heh, sorry for not being clear, that's actually somewhat along the lines of how 
I envisioned you'd implement it (we don't have a way in tablegen to tie 
`DefaultIgnore` to some language options or a target).

I was thinking the diagnostic would be left as `DefaultIgnore` in the .td file, 
but we'd take note in the driver that CFI was enabled and pass 
`-Wincompatible-function-pointer-types-strict` to the `-cc1` invocation. If the 
user wrote `-Wno-incompatible-function-pointer-types-strict` at the driver 
level, that would come *after* the automatically generated flag for cc1 and 
would still disable the diagnostic in the cc1 invocation (because the last flag 
wins). WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136790

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

[PATCH] D137073: [clang] Fix inline builtin functions of an __asm__ renamed function with symbol prefixes

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added reviewers: serge-sans-paille, nickdesaulniers.
mstorsjo added a comment.

This seems like a followup to D134362 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137073

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

[PATCH] D136796: [llvm-objdump][Offload] Use common offload extraction method

[Joseph Huber via Phabricator via cfe-commits]

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

Addressing comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136796

Files:
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
  llvm/lib/Object/OffloadBinary.cpp
  llvm/test/tools/llvm-objdump/Offloading/binary.test
  llvm/test/tools/llvm-objdump/Offloading/content-failure.test
  llvm/test/tools/llvm-objdump/Offloading/elf.test
  llvm/test/tools/llvm-objdump/Offloading/warning.test
  llvm/tools/llvm-objdump/OffloadDump.cpp

Index: llvm/tools/llvm-objdump/OffloadDump.cpp
===
--- llvm/tools/llvm-objdump/OffloadDump.cpp
+++ llvm/tools/llvm-objdump/OffloadDump.cpp
@@ -10,6 +10,7 @@
 /// This file implements the offloading-specific dumper for llvm-objdump.
 ///
 //===--===//
+
 #include "OffloadDump.h"
 #include "llvm-objdump.h"
 #include "llvm/Object/ELFObjectFile.h"
@@ -46,24 +47,6 @@
  << getOffloadKindName(OB.getOffloadKind()) << "\n";
 }
 
-static Error visitAllBinaries(const OffloadBinary &OB) {
-  uint64_t Offset = 0;
-  uint64_t Index = 0;
-  while (Offset < OB.getMemoryBufferRef().getBufferSize()) {
-MemoryBufferRef Buffer =
-MemoryBufferRef(OB.getData().drop_front(Offset), OB.getFileName());
-auto BinaryOrErr = OffloadBinary::create(Buffer);
-if (!BinaryOrErr)
-  return BinaryOrErr.takeError();
-
-OffloadBinary &Binary = **BinaryOrErr;
-printBinary(Binary, Index++);
-
-Offset += Binary.getSize();
-  }
-  return Error::success();
-}
-
 /// Print the embedded offloading contents of an ObjectFile \p O.
 void llvm::dumpOffloadBinary(const ObjectFile &O) {
   if (!O.isELF()) {
@@ -72,41 +55,25 @@
 return;
   }
 
-  for (ELFSectionRef Sec : O.sections()) {
-if (Sec.getType() != ELF::SHT_LLVM_OFFLOADING)
-  continue;
-
-Expected Contents = Sec.getContents();
-if (!Contents)
-  reportError(Contents.takeError(), O.getFileName());
-
-std::unique_ptr Buffer =
-MemoryBuffer::getMemBuffer(*Contents, O.getFileName(), false);
-if (!isAddrAligned(Align(OffloadBinary::getAlignment()),
-   Buffer->getBufferStart()))
-  Buffer = MemoryBuffer::getMemBufferCopy(Buffer->getBuffer(),
-  Buffer->getBufferIdentifier());
-auto BinaryOrErr = OffloadBinary::create(*Buffer);
-if (!BinaryOrErr)
-  reportError(O.getFileName(), "while extracting offloading files: " +
-   toString(BinaryOrErr.takeError()));
-OffloadBinary &Binary = **BinaryOrErr;
+  SmallVector Binaries;
+  if (Error Err = extractOffloadBinaries(O.getMemoryBufferRef(), Binaries))
+reportError(O.getFileName(), "while extracting offloading files: " +
+ toString(std::move(Err)));
 
-// Print out all the binaries that are contained in this buffer. If we fail
-// to parse a binary before reaching the end of the buffer emit a warning.
-if (Error Err = visitAllBinaries(Binary))
-  reportWarning("while parsing offloading files: " +
-toString(std::move(Err)),
-O.getFileName());
-  }
+  // Print out all the binaries that are contained in this buffer.
+  for (uint64_t I = 0, E = Binaries.size(); I != E; ++I)
+printBinary(*Binaries[I].getBinary(), I);
 }
 
 /// Print the contents of an offload binary file \p OB. This may contain
 /// multiple binaries stored in the same buffer.
 void llvm::dumpOffloadSections(const OffloadBinary &OB) {
-  // Print out all the binaries that are contained at this buffer. If we fail to
-  // parse a binary before reaching the end of the buffer emit a warning.
-  if (Error Err = visitAllBinaries(OB))
-reportWarning("while parsing offloading files: " + toString(std::move(Err)),
-  OB.getFileName());
+  SmallVector Binaries;
+  if (Error Err = extractOffloadBinaries(OB.getMemoryBufferRef(), Binaries))
+reportError(OB.getFileName(), "while extracting offloading files: " +
+  toString(std::move(Err)));
+
+  // Print out all the binaries that are contained in this buffer.
+  for (uint64_t I = 0, E = Binaries.size(); I != E; ++I)
+printBinary(*Binaries[I].getBinary(), I);
 }
Index: llvm/test/tools/llvm-objdump/Offloading/warning.test
===
--- llvm/test/tools/llvm-objdump/Offloading/warning.test
+++ /dev/null
@@ -1,21 +0,0 @@
-## Ensure we give a warning on bad input following good input.
-# RUN: yaml2obj %S/Inputs/binary.yaml -o %t-good.bin
-# RUN: yaml2obj %S/Inputs/malformed.yaml -o %t-bad.bin
-# RUN: cat %t-bad.bin >> %t-good.bin
-# RUN: yaml2obj %s -o %t.elf
-# RUN: llvm-objcopy --update-section .llvm.offloading=%t-good.bin 

[PATCH] D137075: [clang-format] Fix document of AlignTrailingComments

[Yusuke Kadowaki via Phabricator via cfe-commits]

yusuke-kadowaki added a comment.

Thank you.
Please land when you have time.

name: Yusuke Kadowaki
email: yusuke.kadowaki.1...@gmail.com


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137075

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

[PATCH] D136975: [Concepts] Correctly handle failure when checking concepts recursively

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 472268.
erichkeane added a comment.

fix clang format


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

https://reviews.llvm.org/D136975

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaTemplate/concepts-GH53213.cpp
  clang/test/SemaTemplate/concepts-recursive-inst.cpp
  libcxx/DELETE_ME.txt

Index: libcxx/DELETE_ME.txt
===
--- /dev/null
+++ libcxx/DELETE_ME.txt
@@ -0,0 +1 @@
+D136975
Index: clang/test/SemaTemplate/concepts-recursive-inst.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/concepts-recursive-inst.cpp
@@ -0,0 +1,122 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+namespace GH53213 {
+template
+concept c = requires(T t) { f(t); }; // #CDEF
+
+auto f(c auto); // #FDEF
+
+void g() {
+  f(0);
+  // expected-error@-1{{no matching function for call to 'f'}}
+  // expected-note@#FDEF{{constraints not satisfied}}
+  // expected-note@#FDEF{{because 'int' does not satisfy 'c'}}
+  // expected-note@#CDEF{{because 'f(t)' would be invalid: no matching function for call to 'f'}}
+}
+} // namespace GH53213 
+
+namespace GH45736 {
+struct constrained;
+
+template
+  struct type {
+  };
+template
+  constexpr bool f(type) {
+  return true;
+  }
+
+template
+  concept matches = f(type());
+
+
+struct constrained {
+template requires matches
+explicit constrained(U value) {
+}
+};
+
+bool f(constrained const &) {
+return true;
+}
+
+struct outer {
+constrained state;
+};
+
+bool f(outer const & x) {
+return f(x.state);
+}
+} // namespace GH45736
+
+namespace DirectRecursiveCheck {
+template
+concept NotInf = true;
+template
+concept Inf = requires(T& v){ // #INF_REQ
+  {begin(v)}; // #INF_BEGIN_EXPR
+};
+
+void begin(NotInf auto& v){ } // #NOTINF_BEGIN
+// This lookup should fail, since it results in a recursive check.
+// However, this is a 'hard failure'(not a SFINAE failure or constraints
+// violation), so it needs to cause the entire lookup to fail.
+void begin(Inf auto& v){ } // #INF_BEGIN
+
+struct my_range{
+} rng;
+
+void baz() {
+auto it = begin(rng); // #BEGIN_CALL
+// expected-error@#INF_BEGIN {{satisfaction of constraint 'Inf' depends on itself}}
+// expected-note@#INF_BEGIN {{while substituting template arguments into constraint expression here}}
+// expected-note@#INF_BEGIN_EXPR {{while checking constraint satisfaction for template 'begin' required here}}
+// expected-note@#INF_BEGIN_EXPR {{while substituting deduced template arguments into function template 'begin'}}
+// expected-note@#INF_BEGIN_EXPR {{in instantiation of requirement here}}
+// expected-note@#INF_REQ {{while substituting template arguments into constraint expression here}}
+// expected-note@#INF_BEGIN {{while checking the satisfaction of concept 'Inf' requested here}}
+// expected-note@#INF_BEGIN {{while substituting template arguments into constraint expression here}}
+// expected-note@#BEGIN_CALL {{while checking constraint satisfaction for template 'begin' required here}}
+// expected-note@#BEGIN_CALL {{in instantiation of function template specialization}}
+
+// Fallout of the failure is failed lookup, which is necessary to stop odd
+// cascading errors.
+// expected-error@#BEGIN_CALL {{no matching function for call to 'begin'}}
+// expected-note@#NOTINF_BEGIN {{candidate function}}
+// expected-note@#INF_BEGIN{{candidate template ignored: constraints not satisfied}}
+}
+} // namespace DirectRecursiveCheck
+
+namespace GH50891 {
+  template 
+  concept Numeric = requires(T a) { // #NUMERIC
+  foo(a); // #FOO_CALL
+};
+
+  struct Deferred {
+friend void foo(Deferred);
+template  operator TO(); // #OP_TO
+  };
+
+  static_assert(Numeric); // #STATIC_ASSERT
+  // expected-error@#OP_TO {{satisfaction of constraint 'Numeric' depends on itself}}
+  // expected-note@#OP_TO {{while substituting template arguments into constraint expression here}}
+  // FIXME: The following two refer to type-parameter-0-0, it would be nice to
+  // see if we could instead diagnose with the sugared name.
+  // expected-note@#FOO_CALL {{while checking constraint satisfaction for template}}
+  // expected-note@#FOO_CALL {{while substituting deduced template arguments into function template}}
+  // expected-note@#FOO_CALL {{in instantiation of requirement here}}
+  // expected-note@#NUMERIC {{while substituting template arguments into constraint expression here}}
+  // expected-note@#OP_TO {{skipping 2 contexts in backtrace}}
+  // expected-note@#FOO_CALL {{while checking constraint satisfaction for template}}
+  // expected-note@#FOO_CALL {{in instantiation of function template specialization}}
+  // expected-note@#FOO_CALL {{in

[PATCH] D137154: Adding nvvm_reflect clang builtin

[Andrew Savonichev via Phabricator via cfe-commits]

asavonic added inline comments.



Comment at: llvm/include/llvm/IR/IntrinsicsNVVM.td:1581
 def int_nvvm_reflect :
-  Intrinsic<[llvm_i32_ty], [llvm_anyptr_ty], [IntrNoMem], "llvm.nvvm.reflect">;
+  Intrinsic<[llvm_i32_ty], [llvm_ptr_ty], [IntrNoMem], "llvm.nvvm.reflect">,
+  ClangBuiltin<"__nvvm_reflect">;

What is there a reason to change the intrinsic?



Comment at: llvm/lib/Target/NVPTX/NVVMReflect.cpp:123
 if (!Callee || (Callee->getName() != NVVM_REFLECT_FUNCTION &&
+Callee->getName() != NVVM_REFLECT_LLVM_INTRINSIC_NAME &&
 Callee->getIntrinsicID() != Intrinsic::nvvm_reflect))

This seems to be equivalent to `Callee->getIntrinsicID()` call below. Is there 
a case where the intrinsic is not recognized by `getIntrinsicID`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137154

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

[PATCH] D136589: [AArch64] Add support for the Cortex-X3 CPU

[Dave Green via Phabricator via cfe-commits]

dmgreen added a comment.

Thanks. What was wrong with the v9-A features?




Comment at: llvm/unittests/Support/TargetParserTest.cpp:1083
+ AArch64::AEK_SVE | AArch64::AEK_SVE2 |
+ AArch64::AEK_SVE2BITPERM | AArch64::AEK_SB |
+ AArch64::AEK_PROFILE | AArch64::AEK_PERFMON |

Should SSBS be included in here? As far as I understand it is enabled by 
default in 8.5-A. That should mean it's included automatically, but that can be 
said for a lot of the other features here too.
Same for AEK_FLAGM, which I noticed as a difference between A715 and X3.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136589

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

[PATCH] D136554: Implement CWG2631

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:9602-9604
+return Ctx.Context ==
+   ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed ||
+   Ctx.IsCheckingDefaultArgumentOrInitializer;

Hmm, it'd be nice to not name this with the same identifier as the `bool` 
member on line 1333, that surprised me a little bit when I ran into it below.



Comment at: clang/include/clang/Sema/Sema.h:9610
+   "Must be in an expression evaluation context");
+for (auto &Ctx : llvm::reverse(ExprEvalContexts)) {
+  if (Ctx.Context ==





Comment at: clang/include/clang/Sema/Sema.h:9628
+  if (Ctx.Context == ExpressionEvaluationContext::PotentiallyEvaluated &&
+  Ctx.DelayedDefaultInitializationContext.has_value())
+return Ctx.DelayedDefaultInitializationContext;





Comment at: clang/include/clang/Sema/Sema.h:9645-9646
+  if (Ctx.Context == ExpressionEvaluationContext::PotentiallyEvaluated &&
+  !Ctx.DelayedDefaultInitializationContext.has_value() &&
+  Res.has_value())
+break;





Comment at: clang/lib/AST/ExprCXX.cpp:964
+ DeclContext *UsedContext) {
+  size_t Size = totalSizeToAlloc(RewrittenExpr != nullptr ? 1 : 0);
+  auto *Mem = C.Allocate(Size, alignof(CXXDefaultArgExpr));





Comment at: clang/lib/AST/ExprCXX.cpp:1019
+
+  size_t Size = totalSizeToAlloc(RewrittenInitExpr != nullptr ? 1 : 0);
+  auto *Mem = Ctx.Allocate(Size, alignof(CXXDefaultArgExpr));





Comment at: clang/lib/Sema/SemaExpr.cpp:5927
+  bool VisitSourceLocExpr(SourceLocExpr *E) {
+HasImmediateCalls = true;
+return RecursiveASTVisitor::VisitStmt(E);

It may be worth a comment that this relies on the behavior of `VisitLambdaExpr` 
not changing to visit the lambda body. Otherwise, nested invocations would be 
an issue in something like `void func(int a = []{ return 
std::source_location::current(); }());` because we'd claim there is an 
immediate call for that default arg expression when there isn't one.

As it stands, I wonder what the behavior of this code should be:
```
void func(int a =
  ({
int val = std::souce_location::current();
val;
  })
);
```
(Oh, wait, I know, I think we made that code invalid because GNU statement 
expressions in a default argument are horrifying...)

Also, it looks like this code ignores blocks while handling lambdas; shouldn't 
blocks be handled the same way as lambdas?

Can you also add a test for this example:
```
void func(int a =
  (int){ std::source_location::current() }
);
```
and I suppose we also should have a test for:
```
consteval int terrible_idea_dont_write_this_code_please(
  int &out,
  const int (&a)[std::source_location::current()] = { 
std::source_location::current() }
) { out = a[0]; return sizeof(a); }

consteval void func() {
  int a;
  static_assert(terrible_idea_dont_write_this_code_please(a) == a, "hahaha, not 
a chance!");
}
```
which demonstrates just how deeply bizarre WG21's decision is here -- the 
returned value != `out`.

And similar tests for default inits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136554

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

[PATCH] D136872: [OpenMP][OpenMPIRBuilder] Migrate loadOffloadInfoMetadata from clang to OMPIRbuilder

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis updated this revision to Diff 472270.
TIFitis marked an inline comment as done.
TIFitis added a comment.

Fixed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136872

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

Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -4705,6 +4705,54 @@
   getTargetRegionEntryFnName(Name, ParentName, DeviceID, FileID, Line);
 }
 
+/// Loads all the offload entries information from the host IR
+/// metadata.
+void OpenMPIRBuilder::loadOffloadInfoMetadata(
+Module &M, OffloadEntriesInfoManager &OffloadEntriesInfoManager) {
+  // If we are in target mode, load the metadata from the host IR. This code has
+  // to match the metadata creation in createOffloadEntriesAndInfoMetadata().
+
+  NamedMDNode *MD = M.getNamedMetadata(ompOffloadInfoName);
+  if (!MD)
+return;
+
+  for (MDNode *MN : MD->operands()) {
+auto &&GetMDInt = [MN](unsigned Idx) {
+  auto *V = cast(MN->getOperand(Idx));
+  return cast(V->getValue())->getZExtValue();
+};
+
+auto &&GetMDString = [MN](unsigned Idx) {
+  auto *V = cast(MN->getOperand(Idx));
+  return V->getString();
+};
+
+switch (GetMDInt(0)) {
+default:
+  llvm_unreachable("Unexpected metadata!");
+  break;
+case OffloadEntriesInfoManager::OffloadEntryInfo::
+OffloadingEntryInfoTargetRegion: {
+  TargetRegionEntryInfo EntryInfo(/*ParentName=*/GetMDString(3),
+  /*DeviceID=*/GetMDInt(1),
+  /*FileID=*/GetMDInt(2),
+  /*Line=*/GetMDInt(4));
+  OffloadEntriesInfoManager.initializeTargetRegionEntryInfo(
+  EntryInfo, /*Order=*/GetMDInt(5));
+  break;
+}
+case OffloadEntriesInfoManager::OffloadEntryInfo::
+OffloadingEntryInfoDeviceGlobalVar:
+  OffloadEntriesInfoManager.initializeDeviceGlobalVarEntryInfo(
+  /*MangledName=*/GetMDString(1),
+  static_cast(
+  /*Flags=*/GetMDInt(2)),
+  /*Order=*/GetMDInt(3));
+  break;
+}
+  }
+}
+
 bool OffloadEntriesInfoManager::empty() const {
   return OffloadEntriesTargetRegion.empty() &&
  OffloadEntriesDeviceGlobalVar.empty();
Index: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
@@ -1681,6 +1681,19 @@
 BasicBlock *PreInsertBefore,
 BasicBlock *PostInsertBefore,
 const Twine &Name = {});
+  /// OMP Offload Info Metadata name string
+  const std::string ompOffloadInfoName = "omp_offload.info";
+
+  /// Loads all the offload entries information from the host IR
+  /// metadata. This function is only meant to be used with device code
+  /// generation.
+  ///
+  /// \param M Module to load Metadata info from. Module passed maybe
+  /// loaded from bitcode file, i.e, different from OpenMPIRBuilder::M module.
+  /// \param OffloadEntriesInfoManager Initialize Offload Entry information.
+  void
+  loadOffloadInfoMetadata(Module &M,
+  OffloadEntriesInfoManager &OffloadEntriesInfoManager);
 };
 
 /// Data structure to contain the information needed to uniquely identify
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -3171,52 +3171,7 @@
 return;
   }
 
-  llvm::NamedMDNode *MD = ME.get()->getNamedMetadata("omp_offload.info");
-  if (!MD)
-return;
-
-  for (llvm::MDNode *MN : MD->operands()) {
-auto &&GetMDInt = [MN](unsigned Idx) {
-  auto *V = cast(MN->getOperand(Idx));
-  return cast(V->getValue())->getZExtValue();
-};
-
-auto &&GetMDString = [MN](unsigned Idx) {
-  auto *V = cast(MN->getOperand(Idx));
-  return V->getString();
-};
-
-switch (GetMDInt(0)) {
-default:
-  llvm_unreachable("Unexpected metadata!");
-  break;
-case llvm::OffloadEntriesInfoManager::OffloadEntryInfo::
-OffloadingEntryInfoTargetRegion: {
-  assert(CGM.getLangOpts().OpenMPIsDevice && "Initialization of entries is "
- "only required for the "
- "device code generation.");
-  llvm::TargetRegionEntryInfo EntryInfo(/*ParentName=*/GetMDString(3),
-/*Devi

[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Relatedly, we ran into a problem with `clang-cl /showIncludes` not including 
all files in its output when they're linked: 
https://github.com/llvm/llvm-project/issues/58726


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135220

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

[PATCH] D136589: [AArch64] Add support for the Cortex-X3 CPU

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:696
-- Add driver and tuning support for Neoverse V2 via the flag 
``-mcpu=neoverse-v2``.
-  Native detection is also supported via ``-mcpu=native``.
 

What happened with native detection?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136589

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

[PATCH] D136554: Implement CWG2631

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:5927
+  bool VisitSourceLocExpr(SourceLocExpr *E) {
+HasImmediateCalls = true;
+return RecursiveASTVisitor::VisitStmt(E);

aaron.ballman wrote:
> It may be worth a comment that this relies on the behavior of 
> `VisitLambdaExpr` not changing to visit the lambda body. Otherwise, nested 
> invocations would be an issue in something like `void func(int a = []{ return 
> std::source_location::current(); }());` because we'd claim there is an 
> immediate call for that default arg expression when there isn't one.
> 
> As it stands, I wonder what the behavior of this code should be:
> ```
> void func(int a =
>   ({
> int val = std::souce_location::current();
> val;
>   })
> );
> ```
> (Oh, wait, I know, I think we made that code invalid because GNU statement 
> expressions in a default argument are horrifying...)
> 
> Also, it looks like this code ignores blocks while handling lambdas; 
> shouldn't blocks be handled the same way as lambdas?
> 
> Can you also add a test for this example:
> ```
> void func(int a =
>   (int){ std::source_location::current() }
> );
> ```
> and I suppose we also should have a test for:
> ```
> consteval int terrible_idea_dont_write_this_code_please(
>   int &out,
>   const int (&a)[std::source_location::current()] = { 
> std::source_location::current() }
> ) { out = a[0]; return sizeof(a); }
> 
> consteval void func() {
>   int a;
>   static_assert(terrible_idea_dont_write_this_code_please(a) == a, "hahaha, 
> not a chance!");
> }
> ```
> which demonstrates just how deeply bizarre WG21's decision is here -- the 
> returned value != `out`.
> 
> And similar tests for default inits.
Another test case I devised shows we've got a lambda bug in clang, but would be 
worth reasoning about:
```
void func(int a = [b = std::source_location::current()] { return b; }());
```
(this should be valid, but I have no idea what value `a` should have if used as 
a default argument.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136554

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

[PATCH] D136589: [AArch64] Add support for the Cortex-X3 CPU

[Dave Green via Phabricator via cfe-commits]

dmgreen added a subscriber: david-arm.
dmgreen added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:696
-- Add driver and tuning support for Neoverse V2 via the flag 
``-mcpu=neoverse-v2``.
-  Native detection is also supported via ``-mcpu=native``.
 

tschuett wrote:
> What happened with native detection?
I think I prefer the new wording, FWIW, as we add more CPUs. The native 
detection should just be automatically implied.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136589

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

[PATCH] D137154: Adding nvvm_reflect clang builtin

[Hugh Delaney via Phabricator via cfe-commits]

hdelan added inline comments.



Comment at: llvm/include/llvm/IR/IntrinsicsNVVM.td:1581
 def int_nvvm_reflect :
-  Intrinsic<[llvm_i32_ty], [llvm_anyptr_ty], [IntrNoMem], "llvm.nvvm.reflect">;
+  Intrinsic<[llvm_i32_ty], [llvm_ptr_ty], [IntrNoMem], "llvm.nvvm.reflect">,
+  ClangBuiltin<"__nvvm_reflect">;

asavonic wrote:
> What is there a reason to change the intrinsic?
The clang builtin doesn't link up properly with the llvm intrinsic if 
`llvm_anyptr_ty` is used. The `llvm_anyptr_ty` suggests to clang to find a 
general `arg` parameter instead of an `Pointer` which results in a segfault. 
Changing to `llvm_ptr_ty` fixes the issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137154

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

[PATCH] D136846: [Driver] Add -fsample-profile-use-profi

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang/include/clang/Driver/Options.td:1254
+HelpText<"Use profi to infer block and edge counts.">,
+DocBrief<[{Profi - a flow-based profile inference algorithm is an extended
+   and significantly re-engineered classic MCMF (min-cost max-flow)

HaoyuZhang wrote:
> hans wrote:
> > I have to say, just reading this text I don't understand what it does.
> > 
> > I think a good description would start with "Infer block and edge counts " 
> > and then some kind of summary of how it does that.
> > 
> > I assume profile info is still needed for this (that's the input, right?) 
> > That should probably also be explained, and maybe we should warn when using 
> > -fsample-profile-use-profi without -fprofile-sample-use?
> > 
> > 
> > My main concern is that there's no documentation for this. How is a user 
> > supposed to learn about this feature and how it works? Why can't someone 
> > add something to 
> > https://clang.llvm.org/docs/UsersManual.html#profile-guided-optimization ? 
> > Once that is figured out, describing what the option does will probably be 
> > easy.
> Sorry for the unclear description of the DocBrief and I have do some 
> modification. 
> 
> A checking has been added for ensuring that -fsample-profile-use-profi is 
> only allowed with fprofile-sample-use. Otherwise, there will be an error.
> 
> About the document in above link, do you want me to add some contents about 
> using profi after the patch or invite the author of profi?
It would ideally be added in this patch. Inviting the author of profi (I didn't 
realize it was a different person) sounds like a good idea.



Comment at: clang/include/clang/Driver/Options.td:1257
+   basic block counts to branch probabilites to fix them by 
extended
+   and re-engineered classic MCMF (min-cost max-flow) approach.}]>;
 def fno_profile_sample_accurate : Flag<["-"], "fno-profile-sample-accurate">,

Thanks, this is much better!

(nit: s/converting/convert/)



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5763
 
+  if (Args.hasArg(options::OPT_fsample_profile_use_profi)) {
+if (Args.hasArg(options::OPT_fprofile_sample_use_EQ)) {

Could you use `tools::getLastProfileSampleUseArg` instead? It covers all the 
spellings of the option.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5768
+} else {
+  D.Diag(diag::err_drv_argument_only_allowed_with)
+  << "-fsample-profile-use-profi"

I think if you did instead:

```
if (getLastProfileSampleUseArg(Args) && 
Args.hasArg(options::OPT_fsample_profile_use_profi)) {
  CmdArgs.push_back(...
}
```

then clang would warn about the option being unused if profile use is not 
enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136846

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

[PATCH] D136975: [Concepts] Correctly handle failure when checking concepts recursively

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:272
+- Fix a number of recursively-instantiated constraint issues, which would 
possibly
+  result in clang's stack-exhaustion.
+  `Issue 44304 `_





Comment at: clang/include/clang/Sema/Sema.h:7235
+
+  bool SatisfactionStackContains(const llvm::FoldingSetNodeID &ID) {
+return llvm::find(SatisfactionStack, ID) != SatisfactionStack.end();





Comment at: clang/lib/Sema/SemaConcept.cpp:150
+namespace {
+struct SatisfactionStackRAII {
+  Sema &SemaRef;

Er, it'd be nice for this not to shadow the name of the class from `Sema`, 
that's pretty confusing.



Comment at: clang/lib/Sema/SemaConcept.cpp:276-278
+  for (const auto &List : MLTAL)
+for (const auto &TemplateArg : List.Args)
+  TemplateArg.Profile(ID, S.Context);

What are the chances that this `O(N^2)` operation is going to come back to bite 
us in terms of compile time performance?


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

https://reviews.llvm.org/D136975

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

[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

Ok, I think I know what the problem is — these buildbots rely on being able to 
override `LLVM_CONFIG_PATH`. I'm going to see if I can provide backwards 
compatibility with it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137024

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

[PATCH] D136975: [Concepts] Correctly handle failure when checking concepts recursively

[Erich Keane via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaConcept.cpp:150
+namespace {
+struct SatisfactionStackRAII {
+  Sema &SemaRef;

aaron.ballman wrote:
> Er, it'd be nice for this not to shadow the name of the class from `Sema`, 
> that's pretty confusing.
What do you mean?  What name does it shadow?



Comment at: clang/lib/Sema/SemaConcept.cpp:276-278
+  for (const auto &List : MLTAL)
+for (const auto &TemplateArg : List.Args)
+  TemplateArg.Profile(ID, S.Context);

aaron.ballman wrote:
> What are the chances that this `O(N^2)` operation is going to come back to 
> bite us in terms of compile time performance?
I'd hope not too much?  This is just going through the whole list of template 
arguments on this expression, so I think that makes this `O(M*N)`, where M and 
N are limited by the number of template arguments we allow.



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

https://reviews.llvm.org/D136975

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

[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

[Michał Górny via Phabricator via cfe-commits]

mgorny updated this revision to Diff 472285.
mgorny added a comment.

Add a backwards compatibility path that uses `LLVM_CONFIG_PATH` to locate LLVM 
CMake files. I think it's sufficient to use path relative to the specified 
location, and we don't want to actually call it.

Restore the explicit warning when LLVM is not found and the fallback path is 
used.


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

https://reviews.llvm.org/D137024

Files:
  compiler-rt/cmake/Modules/CompilerRTUtils.cmake
  compiler-rt/lib/xray/tests/CMakeLists.txt

Index: compiler-rt/lib/xray/tests/CMakeLists.txt
===
--- compiler-rt/lib/xray/tests/CMakeLists.txt
+++ compiler-rt/lib/xray/tests/CMakeLists.txt
@@ -59,19 +59,26 @@
 LLVM_ENABLE_TERMINFO
 -l${COMPILER_RT_TERMINFO_LIB} XRAY_UNITTEST_LINK_FLAGS)
 
+  # We add the library directories one at a time in our CFLAGS.
+  foreach (DIR ${LLVM_LIBRARY_DIR})
+list(APPEND XRAY_UNITTEST_LINK_FLAGS -L${DIR})
+  endforeach()
+
   if (COMPILER_RT_STANDALONE_BUILD)
-append_list_if(COMPILER_RT_HAS_LLVMXRAY ${LLVM_XRAY_LDFLAGS} XRAY_UNITTEST_LINK_FLAGS)
-append_list_if(COMPILER_RT_HAS_LLVMXRAY ${LLVM_XRAY_LIBLIST} XRAY_UNITTEST_LINK_FLAGS)
-append_list_if(COMPILER_RT_HAS_LLVMTESTINGSUPPORT
-  ${LLVM_TESTINGSUPPORT_LDFLAGS} XRAY_UNITTEST_LINK_FLAGS)
-append_list_if(COMPILER_RT_HAS_LLVMTESTINGSUPPORT
-  ${LLVM_TESTINGSUPPORT_LIBLIST} XRAY_UNITTEST_LINK_FLAGS)
+if (COMPILER_RT_HAS_LLVMXRAY OR COMPILER_RT_HAS_LLVMTESTINGSUPPORT)
+  if (LLVM_LINK_LLVM_DYLIB)
+list(APPEND XRAY_UNITTEST_LINK_FLAGS -lLLVM)
+  endif()
+else()
+  if (COMPILER_RT_HAS_LLVMXRAY)
+list(APPEND XRAY_UNITTEST_LINK_FLAGS -lLLVMXRay)
+  endif()
+  if (COMPILER_RT_HAS_TESTINGSUPPORT)
+list(APPEND XRAY_UNITTEST_LINK_FLAGS -lLLVMTestingSupport)
+  endif()
+  list(APPEND XRAY_UNITTEST_LINK_FLAGS -lLLVMSupport -lLLVMDemangle)
+endif()
   else()
-# We add the library directories one at a time in our CFLAGS.
-foreach (DIR ${LLVM_LIBRARY_DIR})
-  list(APPEND XRAY_UNITTEST_LINK_FLAGS -L${DIR})
-endforeach()
-
 # We also add the actual libraries to link as dependencies.
 list(APPEND XRAY_UNITTEST_LINK_FLAGS -lLLVMXRay -lLLVMSupport -lLLVMDemangle -lLLVMTestingSupport)
   endif()
Index: compiler-rt/cmake/Modules/CompilerRTUtils.cmake
===
--- compiler-rt/cmake/Modules/CompilerRTUtils.cmake
+++ compiler-rt/cmake/Modules/CompilerRTUtils.cmake
@@ -268,14 +268,11 @@
 endfunction()
 
 macro(load_llvm_config)
-  if (NOT LLVM_CONFIG_PATH)
-find_program(LLVM_CONFIG_PATH "llvm-config"
- DOC "Path to llvm-config binary")
-if (NOT LLVM_CONFIG_PATH)
-  message(WARNING "UNSUPPORTED COMPILER-RT CONFIGURATION DETECTED: "
-  "llvm-config not found.\n"
-  "Reconfigure with -DLLVM_CONFIG_PATH=path/to/llvm-config.")
-endif()
+  if (LLVM_CONFIG_PATH AND NOT LLVM_CMAKE_DIR)
+message(WARNING
+  "LLVM_CONFIG_PATH is deprecated, please use LLVM_CMAKE_DIR instead")
+get_filename_component(LLVM_CMAKE_DIR "${LLVM_CONFIG_PATH}" DIRECTORY)
+get_filename_component(LLVM_CMAKE_DIR "${LLVM_CMAKE_DIR}" DIRECTORY)
   endif()
 
   # Compute path to LLVM sources assuming the monorepo layout.
@@ -290,114 +287,37 @@
   "You are not using the monorepo layout. This configuration is DEPRECATED.")
   endif()
 
-  set(FOUND_LLVM_CMAKE_DIR FALSE)
-  if (LLVM_CONFIG_PATH)
-execute_process(
-  COMMAND ${LLVM_CONFIG_PATH} "--obj-root" "--bindir" "--libdir" "--src-root" "--includedir"
-  RESULT_VARIABLE HAD_ERROR
-  OUTPUT_VARIABLE CONFIG_OUTPUT)
-if (HAD_ERROR)
-  message(FATAL_ERROR "llvm-config failed with status ${HAD_ERROR}")
-endif()
-string(REGEX REPLACE "[ \t]*[\r\n]+[ \t]*" ";" CONFIG_OUTPUT ${CONFIG_OUTPUT})
-list(GET CONFIG_OUTPUT 0 BINARY_DIR)
-list(GET CONFIG_OUTPUT 1 TOOLS_BINARY_DIR)
-list(GET CONFIG_OUTPUT 2 LIBRARY_DIR)
-list(GET CONFIG_OUTPUT 3 MAIN_SRC_DIR)
-list(GET CONFIG_OUTPUT 4 INCLUDE_DIR)
-
-set(LLVM_BINARY_DIR ${BINARY_DIR} CACHE PATH "Path to LLVM build tree")
-set(LLVM_LIBRARY_DIR ${LIBRARY_DIR} CACHE PATH "Path to llvm/lib")
-set(LLVM_TOOLS_BINARY_DIR ${TOOLS_BINARY_DIR} CACHE PATH "Path to llvm/bin")
-set(LLVM_INCLUDE_DIR ${INCLUDE_DIR} CACHE PATH "Paths to LLVM headers")
-
-if (NOT EXISTS "${LLVM_MAIN_SRC_DIR_DEFAULT}")
-  # TODO(dliew): Remove this legacy fallback path.
-  message(WARNING
-"Consulting llvm-config for the LLVM source path "
-"as a fallback. This behavior will be removed in the future.")
-  # We don't set `LLVM_MAIN_SRC_DIR` directly to avoid overriding a user
-  # provided CMake cache value.
-  set(LLVM_MAIN_SRC_DIR_DEFAULT "${MAIN_SRC_DIR}")
-  message(STATUS "Using LLVM 

[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

@phosek, could you take another look? The only changes are on top of 
`load_llvm_config`.


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

https://reviews.llvm.org/D137024

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

[PATCH] D136975: [Concepts] Correctly handle failure when checking concepts recursively

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 472286.
erichkeane added a comment.

Fix issues aaron came up with.


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

https://reviews.llvm.org/D136975

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaTemplate/concepts-GH53213.cpp
  clang/test/SemaTemplate/concepts-recursive-inst.cpp

Index: clang/test/SemaTemplate/concepts-recursive-inst.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/concepts-recursive-inst.cpp
@@ -0,0 +1,122 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+namespace GH53213 {
+template
+concept c = requires(T t) { f(t); }; // #CDEF
+
+auto f(c auto); // #FDEF
+
+void g() {
+  f(0);
+  // expected-error@-1{{no matching function for call to 'f'}}
+  // expected-note@#FDEF{{constraints not satisfied}}
+  // expected-note@#FDEF{{because 'int' does not satisfy 'c'}}
+  // expected-note@#CDEF{{because 'f(t)' would be invalid: no matching function for call to 'f'}}
+}
+} // namespace GH53213 
+
+namespace GH45736 {
+struct constrained;
+
+template
+  struct type {
+  };
+template
+  constexpr bool f(type) {
+  return true;
+  }
+
+template
+  concept matches = f(type());
+
+
+struct constrained {
+template requires matches
+explicit constrained(U value) {
+}
+};
+
+bool f(constrained const &) {
+return true;
+}
+
+struct outer {
+constrained state;
+};
+
+bool f(outer const & x) {
+return f(x.state);
+}
+} // namespace GH45736
+
+namespace DirectRecursiveCheck {
+template
+concept NotInf = true;
+template
+concept Inf = requires(T& v){ // #INF_REQ
+  {begin(v)}; // #INF_BEGIN_EXPR
+};
+
+void begin(NotInf auto& v){ } // #NOTINF_BEGIN
+// This lookup should fail, since it results in a recursive check.
+// However, this is a 'hard failure'(not a SFINAE failure or constraints
+// violation), so it needs to cause the entire lookup to fail.
+void begin(Inf auto& v){ } // #INF_BEGIN
+
+struct my_range{
+} rng;
+
+void baz() {
+auto it = begin(rng); // #BEGIN_CALL
+// expected-error@#INF_BEGIN {{satisfaction of constraint 'Inf' depends on itself}}
+// expected-note@#INF_BEGIN {{while substituting template arguments into constraint expression here}}
+// expected-note@#INF_BEGIN_EXPR {{while checking constraint satisfaction for template 'begin' required here}}
+// expected-note@#INF_BEGIN_EXPR {{while substituting deduced template arguments into function template 'begin'}}
+// expected-note@#INF_BEGIN_EXPR {{in instantiation of requirement here}}
+// expected-note@#INF_REQ {{while substituting template arguments into constraint expression here}}
+// expected-note@#INF_BEGIN {{while checking the satisfaction of concept 'Inf' requested here}}
+// expected-note@#INF_BEGIN {{while substituting template arguments into constraint expression here}}
+// expected-note@#BEGIN_CALL {{while checking constraint satisfaction for template 'begin' required here}}
+// expected-note@#BEGIN_CALL {{in instantiation of function template specialization}}
+
+// Fallout of the failure is failed lookup, which is necessary to stop odd
+// cascading errors.
+// expected-error@#BEGIN_CALL {{no matching function for call to 'begin'}}
+// expected-note@#NOTINF_BEGIN {{candidate function}}
+// expected-note@#INF_BEGIN{{candidate template ignored: constraints not satisfied}}
+}
+} // namespace DirectRecursiveCheck
+
+namespace GH50891 {
+  template 
+  concept Numeric = requires(T a) { // #NUMERIC
+  foo(a); // #FOO_CALL
+};
+
+  struct Deferred {
+friend void foo(Deferred);
+template  operator TO(); // #OP_TO
+  };
+
+  static_assert(Numeric); // #STATIC_ASSERT
+  // expected-error@#OP_TO {{satisfaction of constraint 'Numeric' depends on itself}}
+  // expected-note@#OP_TO {{while substituting template arguments into constraint expression here}}
+  // FIXME: The following two refer to type-parameter-0-0, it would be nice to
+  // see if we could instead diagnose with the sugared name.
+  // expected-note@#FOO_CALL {{while checking constraint satisfaction for template}}
+  // expected-note@#FOO_CALL {{while substituting deduced template arguments into function template}}
+  // expected-note@#FOO_CALL {{in instantiation of requirement here}}
+  // expected-note@#NUMERIC {{while substituting template arguments into constraint expression here}}
+  // expected-note@#OP_TO {{skipping 2 contexts in backtrace}}
+  // expected-note@#FOO_CALL {{while checking constraint satisfaction for template}}
+  // expected-note@#FOO_CALL {{in instantiation of function template specialization}}
+  // expected-note@#FOO_CALL {{in instantiation of requirement here}}
+  // expected-note@#NUMERIC {{while substituting template arguments into constraint expression here}}
+  // expected-note@#STATIC

[PATCH] D136975: [Concepts] Correctly handle failure when checking concepts recursively

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaConcept.cpp:150
+namespace {
+struct SatisfactionStackRAII {
+  Sema &SemaRef;

erichkeane wrote:
> aaron.ballman wrote:
> > Er, it'd be nice for this not to shadow the name of the class from `Sema`, 
> > that's pretty confusing.
> What do you mean?  What name does it shadow?
Sema.h:7239 (code you added in this patch).



Comment at: clang/lib/Sema/SemaConcept.cpp:276-278
+  for (const auto &List : MLTAL)
+for (const auto &TemplateArg : List.Args)
+  TemplateArg.Profile(ID, S.Context);

erichkeane wrote:
> aaron.ballman wrote:
> > What are the chances that this `O(N^2)` operation is going to come back to 
> > bite us in terms of compile time performance?
> I'd hope not too much?  This is just going through the whole list of template 
> arguments on this expression, so I think that makes this `O(M*N)`, where M 
> and N are limited by the number of template arguments we allow.
> 
Okay, I was mostly worried about STL headers where there may be a long list of 
template arguments and not a lot of recursion to worry about. We don't really 
memoize whether we've already determined a given constraint is not recursive so 
that we don't have to repeat this work over and over again, but if performance 
is a concern in practice, we could explore that as an option.


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

https://reviews.llvm.org/D136975

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

[PATCH] D137154: Adding nvvm_reflect clang builtin

[Andrew Savonichev via Phabricator via cfe-commits]

asavonic added inline comments.



Comment at: llvm/include/llvm/IR/IntrinsicsNVVM.td:1581
 def int_nvvm_reflect :
-  Intrinsic<[llvm_i32_ty], [llvm_anyptr_ty], [IntrNoMem], "llvm.nvvm.reflect">;
+  Intrinsic<[llvm_i32_ty], [llvm_ptr_ty], [IntrNoMem], "llvm.nvvm.reflect">,
+  ClangBuiltin<"__nvvm_reflect">;

hdelan wrote:
> asavonic wrote:
> > What is there a reason to change the intrinsic?
> The clang builtin doesn't link up properly with the llvm intrinsic if 
> `llvm_anyptr_ty` is used. The `llvm_anyptr_ty` suggests to clang to find a 
> general `arg` parameter instead of an `Pointer` which results in a segfault. 
> Changing to `llvm_ptr_ty` fixes the issue
This sounds like a bug in clang. If clang's default lowering for builtins 
doesn't play well with `llvm_anyptr_ty`, it should be possible to implement it 
with custom lowering code ( in `CodeGenFunction::EmitNVPTXBuiltinExpr`) without 
changing the intrinsic. Some intrinsics with anyptr arguments are already 
implemented with custom lowering (LDG, some atomics).

Changing the intrinsic is problematic for backward compatibility. Overloaded 
intrinsic from old IR will not be recognized, pointers in non-zero address 
space cannot be used as arguments directly (must be addrspacecast to generic 
AS).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137154

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

[PATCH] D136589: [AArch64] Add support for the Cortex-X3 CPU

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:696
-- Add driver and tuning support for Neoverse V2 via the flag 
``-mcpu=neoverse-v2``.
-  Native detection is also supported via ``-mcpu=native``.
 

dmgreen wrote:
> tschuett wrote:
> > What happened with native detection?
> I think I prefer the new wording, FWIW, as we add more CPUs. The native 
> detection should just be automatically implied.
But the word `native` disappeared.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136589

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

[PATCH] D137082: [clang][Interp] Fix dereferencing arrays with no offset applied

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

> There is a difference between a Pointer and a "Pointer to the first element 
> of an array".

I'm pretty confused because this statement is false per the language standard 
(http://eel.is/c++draft/expr.sub#2). Basically, array subscripting works 
through pointer arithmetic, so `&array[0]` and `array` (decayed to a pointer) 
have the same value. Why do we need to offset to get to the first element in 
the interpreter?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137082

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

[PATCH] D136807: [clang][Sema] Fix a clang crash with btf_type_tag

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!




Comment at: clang/docs/ReleaseNotes.rst:261
   `Issue 47136 `_
+- Fix a crash when btf_type_tag attr is applied to the pointee of
+  a function pointer.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136807

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

[PATCH] D137052: [clang-format] Don't skip #else/#elif of #if 0

[sstwcw via Phabricator via cfe-commits]

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

Thanks for fixing the problem I caused.


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

https://reviews.llvm.org/D137052

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

[PATCH] D136936: [clang][Interp] Handle undefined functions better

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeEmitter.cpp:30
   if (!FuncDecl->isDefined(FuncDecl) ||
-  (!FuncDecl->hasBody() && FuncDecl->willHaveBody()))
-return nullptr;
+  (FuncDecl->hasBody() && FuncDecl->willHaveBody()))
+HasBody = false;

`hasBody()` returns `true` if any body in the redeclaration chain already has a 
body, so I'm surprised to see this change -- I don't know if we reset 
`willHaveBody()` when we give the function a body, but the logic here seems 
wrong to me.



Comment at: clang/lib/AST/Interp/Program.cpp:210
 Function *Program::getFunction(const FunctionDecl *F) {
-  F = F->getDefinition();
+  F = F->getCanonicalDecl();
+  assert(F);

Same question here about canonical decl.



Comment at: clang/lib/AST/Interp/Program.h:97
   Function *createFunction(const FunctionDecl *Def, Ts &&... Args) {
+Def = Def->getCanonicalDecl();
 auto *Func = new Function(*this, Def, std::forward(Args)...);

Are you trying to get the `FunctionDecl` which represents the function 
definition, or do you really mean you want the first declaration in the redecl 
chain?


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

https://reviews.llvm.org/D136936

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

[PATCH] D127284: [clang-repl] Support statements on global scope in incremental mode.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev updated this revision to Diff 472224.
v.g.vassilev edited the summary of this revision.
v.g.vassilev added a comment.

Rebase.


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

https://reviews.llvm.org/D127284

Files:
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DeclNodes.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/Template.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ModuleBuilder.cpp
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Interpreter/disambiguate-decl-stmt.cpp
  clang/test/Interpreter/execute-stmts.cpp
  clang/test/Interpreter/stmt-serialization.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp

Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -126,12 +126,7 @@
 
   // FIXME: Add support for wrapping and running statements.
   auto R2 = Interp->Parse("var1++; printf(\"var1 value %d\\n\", var1);");
-  EXPECT_FALSE(!!R2);
-  using ::testing::HasSubstr;
-  EXPECT_THAT(DiagnosticsOS.str(),
-  HasSubstr("error: unknown type name 'var1'"));
-  auto Err = R2.takeError();
-  EXPECT_EQ("Parsing failed.", llvm::toString(std::move(Err)));
+  EXPECT_TRUE(!!R2);
 }
 
 TEST(InterpreterTest, UndoCommand) {
Index: clang/test/Interpreter/stmt-serialization.cpp
===
--- /dev/null
+++ clang/test/Interpreter/stmt-serialization.cpp
@@ -0,0 +1,19 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++20 -fincremental-extensions -fmodules-cache-path=%t \
+// RUN:-x c++ %s -verify
+// expected-no-diagnostics
+
+#pragma clang module build TopLevelStmt
+module TopLevelStmt { module Statements {} }
+#pragma clang module contents
+
+#pragma clang module begin TopLevelStmt.Statements
+extern "C" int printf(const char*,...);
+int i = 0;
+i++;
+#pragma clang module end /*TopLevelStmt.Statements*/
+#pragma clang module endbuild /*TopLevelStmt*/
+
+#pragma clang module import TopLevelStmt.Statements
+
+printf("Value of i is '%d'", i);
Index: clang/test/Interpreter/execute-stmts.cpp
===
--- /dev/null
+++ clang/test/Interpreter/execute-stmts.cpp
@@ -0,0 +1,34 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+// RUN: cat %s | clang-repl -Xcc -Xclang -Xcc  -verify | FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only -verify -fincremental-extensions %s
+
+// expected-no-diagnostics
+
+extern "C" int printf(const char*,...);
+
+// FIXME: Support template instantiation calls.
+//template  T call() { printf("call\n"); return T(); }
+//call();
+ C: call
+
+int i = 1;
+++i;
+printf("i = %d\n", i);
+// CHECK: i = 2
+
+namespace Ns { void f(){ i++; } }
+Ns::f();
+
+void g() { ++i; }
+g();
+::g();
+
+printf("i = %d\n", i);
+// CHECK-NEXT: i = 5
+
+for (; i > 4; --i) printf("i = %d\n", i);
+// CHECK-NEXT: i = 5
+
+int j = i; printf("j = %d\n", j);
+// CHECK-NEXT: j = 4
Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- /dev/null
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -0,0 +1,52 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+// RUN: cat %s | clang-repl -Xcc -Xclang -Xcc -verify | FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only -verify -fincremental-extensions %s
+
+// expected-no-diagnostics
+
+extern "C" int printf(const char*,...);
+
+// Decls which are hard to disambiguate
+
+// FIXME: Support operators.
+// struct S1 { operator int(); };
+// S1::operator int() { return 0; }
+
+
+// Dtors
+using I = int;
+I x = 10;
+x.I::~I();
+x = 20;
+
+// Ctors
+
+// FIXME: Support deduction guide
+// template struct A { A(); A(T); };
+// A() -> A;
+
+struct S2 { S2(); };
+S2::S2() = default;
+
+namespace N { struct S { S(); }; }
+N::S::S() { printf("N::S::S()\n"); }
+N::S s;
+// CHECK: N::S::S()
+
+namespace Ns {namespace Ns { void Ns(); void Fs();}}
+void Ns::Ns::Ns() { printf("void Ns::Ns::Ns()\n"); }
+void Ns::Ns::Fs() {}
+
+Ns::Ns::Fs();
+Ns::Ns::Ns();
+// CHECK-NEXT: void Ns::Ns::Ns()
+
+struct Attrs1 {

[PATCH] D127284: [clang-repl] Support statements on global scope in incremental mode.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.



Comment at: clang/test/Interpreter/disambiguate-decl-stmt.cpp:28-33
+namespace Ns {namespace Ns { void Ns(); void Fs();}}
+void Ns::Ns::Ns() { printf("void Ns::Ns::Ns()\n"); }
+void Ns::Ns::Fs() {}
+
+Ns::Ns::Fs();
+Ns::Ns::Ns();

aaron.ballman wrote:
> v.g.vassilev wrote:
> > aaron.ballman wrote:
> > > This behavior is very confusing to me -- you cannot declare a namespace 
> > > at function scope and you cannot call a function at file scope, so one of 
> > > these two seems like it should not work.
> > > 
> > > (Same kinds of confusion happen elsewhere in the tests.)
> > The mental model has been that we have a mix of statements and declarations 
> > on the global scope. The sequence of statements are only considered to be 
> > as if they have been wrapped in a function body. For example:
> > 
> > ```
> > namespace Ns {namespace Ns { void Ns(); void Fs();}}
> > void Ns::Ns::Ns() { printf("void Ns::Ns::Ns()\n"); }
> > void Ns::Ns::Fs() {}
> > Ns::Ns::Fs();
> > Ns::Ns::Ns();
> > ```
> > 
> > should be semantically equivalent to:
> > 
> > ```
> > namespace Ns {namespace Ns { void Ns(); void Fs();}}
> > void Ns::Ns::Ns() { printf("void Ns::Ns::Ns()\n"); }
> > void Ns::Ns::Fs() {}
> > 
> > void stmt_block1() {
> >   Ns::Ns::Fs();
> >   Ns::Ns::Ns();
> > }
> > auto _ = (stmt_block1(), 1);
> > ```
> > 
> > Does that help in making it less confusing when thinking about it?
> > The mental model has been that we have a mix of statements and declarations 
> > on the global scope.
> 
> I don't understand that mental model because neither C nor C++ allow you to 
> have statements at the global scope. So I'm not certain how to think about 
> statements showing up there (and more importantly, the languages and the 
> compiler wouldn't expect that anyway). For example, consider this:
> ```
> int n = 12;
> int array[n];
> ```
> This is valid at block scope and invalid at global scope: 
> https://godbolt.org/z/8zxn3h79E
> 
> Similarly, consider this code:
> ```
> template 
> void func();
> ```
> This is valid at file scope but invalid at block scope.
> 
> My concern here is that it will be a game of whack-a-mole to identify all of 
> these circumstances and try to get the semantics somewhat close to correct. 
> What is the harm with the REPL always being a full TU, so if the user wants a 
> block scope, they're responsible for writing a function and calling it from 
> `main()`?
That was clarified in the off-list discussion. I will try to summarize it for 
having proper history:

Our new facility teaches the parser to recognize statements when they were not 
declarations. If they were declarations we prefer the language standard. If 
they were valid statements which are spelled out on the global scope we pretend 
they were written in block scope, form a function dedicated definition and 
order it for global initialization.

```
int n = 12;
int array[n];
```

is still invalid on global scope.

```
template 
void func();
```

is untouched. 



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

https://reviews.llvm.org/D127284

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

[PATCH] D137052: [clang-format] Don't skip #else/#elif of #if 0

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Can you also add a test for `#elifdef` and `#elifndef`?


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

https://reviews.llvm.org/D137052

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

[PATCH] D137052: [clang-format] Don't skip #else/#elif of #if 0

[sstwcw via Phabricator via cfe-commits]

sstwcw added a comment.

It just occurred to me that klimek is the one who added all this stuff.  Why 
didn't you include him as a reviewer?


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

https://reviews.llvm.org/D137052

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

[PATCH] D136975: [Concepts] Correctly handle failure when checking concepts recursively

[Erich Keane via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaConcept.cpp:150
+namespace {
+struct SatisfactionStackRAII {
+  Sema &SemaRef;

aaron.ballman wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > Er, it'd be nice for this not to shadow the name of the class from 
> > > `Sema`, that's pretty confusing.
> > What do you mean?  What name does it shadow?
> Sema.h:7239 (code you added in this patch).
Yikes!  Thanks, good catch!



Comment at: clang/lib/Sema/SemaConcept.cpp:276-278
+  for (const auto &List : MLTAL)
+for (const auto &TemplateArg : List.Args)
+  TemplateArg.Profile(ID, S.Context);

aaron.ballman wrote:
> erichkeane wrote:
> > aaron.ballman wrote:
> > > What are the chances that this `O(N^2)` operation is going to come back 
> > > to bite us in terms of compile time performance?
> > I'd hope not too much?  This is just going through the whole list of 
> > template arguments on this expression, so I think that makes this `O(M*N)`, 
> > where M and N are limited by the number of template arguments we allow.
> > 
> Okay, I was mostly worried about STL headers where there may be a long list 
> of template arguments and not a lot of recursion to worry about. We don't 
> really memoize whether we've already determined a given constraint is not 
> recursive so that we don't have to repeat this work over and over again, but 
> if performance is a concern in practice, we could explore that as an option.
I'm somewhat worried there too, but I'd hope that an M*N < ~1000 would be a 
small impact.  We actually DO memoize this, at least on a per instantiation 
basis, as we cache constraint evaluations.


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

https://reviews.llvm.org/D136975

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

[PATCH] D136975: [Concepts] Correctly handle failure when checking concepts recursively

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 472298.
erichkeane marked an inline comment as done.
erichkeane added a comment.

A quick 'quality of life' improvement, otherwise same as before.  Should be 
ready for commit?


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

https://reviews.llvm.org/D136975

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaTemplate/concepts-GH53213.cpp
  clang/test/SemaTemplate/concepts-recursive-inst.cpp

Index: clang/test/SemaTemplate/concepts-recursive-inst.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/concepts-recursive-inst.cpp
@@ -0,0 +1,122 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+namespace GH53213 {
+template
+concept c = requires(T t) { f(t); }; // #CDEF
+
+auto f(c auto); // #FDEF
+
+void g() {
+  f(0);
+  // expected-error@-1{{no matching function for call to 'f'}}
+  // expected-note@#FDEF{{constraints not satisfied}}
+  // expected-note@#FDEF{{because 'int' does not satisfy 'c'}}
+  // expected-note@#CDEF{{because 'f(t)' would be invalid: no matching function for call to 'f'}}
+}
+} // namespace GH53213 
+
+namespace GH45736 {
+struct constrained;
+
+template
+  struct type {
+  };
+template
+  constexpr bool f(type) {
+  return true;
+  }
+
+template
+  concept matches = f(type());
+
+
+struct constrained {
+template requires matches
+explicit constrained(U value) {
+}
+};
+
+bool f(constrained const &) {
+return true;
+}
+
+struct outer {
+constrained state;
+};
+
+bool f(outer const & x) {
+return f(x.state);
+}
+} // namespace GH45736
+
+namespace DirectRecursiveCheck {
+template
+concept NotInf = true;
+template
+concept Inf = requires(T& v){ // #INF_REQ
+  {begin(v)}; // #INF_BEGIN_EXPR
+};
+
+void begin(NotInf auto& v){ } // #NOTINF_BEGIN
+// This lookup should fail, since it results in a recursive check.
+// However, this is a 'hard failure'(not a SFINAE failure or constraints
+// violation), so it needs to cause the entire lookup to fail.
+void begin(Inf auto& v){ } // #INF_BEGIN
+
+struct my_range{
+} rng;
+
+void baz() {
+auto it = begin(rng); // #BEGIN_CALL
+// expected-error@#INF_BEGIN {{satisfaction of constraint 'Inf' depends on itself}}
+// expected-note@#INF_BEGIN {{while substituting template arguments into constraint expression here}}
+// expected-note@#INF_BEGIN_EXPR {{while checking constraint satisfaction for template 'begin' required here}}
+// expected-note@#INF_BEGIN_EXPR {{while substituting deduced template arguments into function template 'begin'}}
+// expected-note@#INF_BEGIN_EXPR {{in instantiation of requirement here}}
+// expected-note@#INF_REQ {{while substituting template arguments into constraint expression here}}
+// expected-note@#INF_BEGIN {{while checking the satisfaction of concept 'Inf' requested here}}
+// expected-note@#INF_BEGIN {{while substituting template arguments into constraint expression here}}
+// expected-note@#BEGIN_CALL {{while checking constraint satisfaction for template 'begin' required here}}
+// expected-note@#BEGIN_CALL {{in instantiation of function template specialization}}
+
+// Fallout of the failure is failed lookup, which is necessary to stop odd
+// cascading errors.
+// expected-error@#BEGIN_CALL {{no matching function for call to 'begin'}}
+// expected-note@#NOTINF_BEGIN {{candidate function}}
+// expected-note@#INF_BEGIN{{candidate template ignored: constraints not satisfied}}
+}
+} // namespace DirectRecursiveCheck
+
+namespace GH50891 {
+  template 
+  concept Numeric = requires(T a) { // #NUMERIC
+  foo(a); // #FOO_CALL
+};
+
+  struct Deferred {
+friend void foo(Deferred);
+template  operator TO(); // #OP_TO
+  };
+
+  static_assert(Numeric); // #STATIC_ASSERT
+  // expected-error@#OP_TO {{satisfaction of constraint 'Numeric' depends on itself}}
+  // expected-note@#OP_TO {{while substituting template arguments into constraint expression here}}
+  // FIXME: The following two refer to type-parameter-0-0, it would be nice to
+  // see if we could instead diagnose with the sugared name.
+  // expected-note@#FOO_CALL {{while checking constraint satisfaction for template}}
+  // expected-note@#FOO_CALL {{while substituting deduced template arguments into function template}}
+  // expected-note@#FOO_CALL {{in instantiation of requirement here}}
+  // expected-note@#NUMERIC {{while substituting template arguments into constraint expression here}}
+  // expected-note@#OP_TO {{skipping 2 contexts in backtrace}}
+  // expected-note@#FOO_CALL {{while checking constraint satisfaction for template}}
+  // expected-note@#FOO_CALL {{in instantiation of function template specialization}}
+  // expected-note@#FOO_CALL {{in instantiation of requirement here}}
+  // expected-note@#

[clang] 0933b8c - Update a stale comment; NFC

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-11-01T10:47:30-04:00
New Revision: 0933b8c72c52dd7f11c6af5f86ce8af01cb194be

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

LOG: Update a stale comment; NFC

This function doesn't return anything these days.

Added: 


Modified: 
clang/lib/Sema/DeclSpec.cpp

Removed: 




diff  --git a/clang/lib/Sema/DeclSpec.cpp b/clang/lib/Sema/DeclSpec.cpp
index dc6a1efdbf746..d59778b5b614b 100644
--- a/clang/lib/Sema/DeclSpec.cpp
+++ b/clang/lib/Sema/DeclSpec.cpp
@@ -1117,9 +1117,8 @@ void DeclSpec::SaveWrittenBuiltinSpecs() {
 }
 
 /// Finish - This does final analysis of the declspec, rejecting things like
-/// "_Imaginary" (lacking an FP type).  This returns a diagnostic to issue or
-/// diag::NUM_DIAGNOSTICS if there is no error.  After calling this method,
-/// DeclSpec is guaranteed self-consistent, even if an error occurred.
+/// "_Imaginary" (lacking an FP type). After calling this method, DeclSpec is
+/// guaranteed to be self-consistent, even if an error occurred.
 void DeclSpec::Finish(Sema &S, const PrintingPolicy &Policy) {
   // Before possibly changing their values, save specs as written.
   SaveWrittenBuiltinSpecs();



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

[PATCH] D137058: [Driver] [Modules] Support -fsave-std-c++-module-file (1/2)

[Ben Boeckel via Phabricator via cfe-commits]

ben.boeckel added a comment.

In D137058#3896999 , @dblaikie wrote:

> Could you expound a bit on why "write the .pcm to the same path as the .o" 
> isn't sufficient? (like Split DWARF does this, for instance - not sure we 
> have lots of other things to extrapolate from (admittedly there is a flag for 
> specifying the filename for Split DWARF too, but it's not a terribly official 
> thing (doesn't have a '-f', '-g', etc prefix))

CMake doesn't support split dwarf (officially; it works, but CMake is just 
ignorant of it), so adapting to existing behavior would be more important 
there. For modules, however, explicit control is just much nicer to provide. It 
means that if heuristics change, CMake doesn't need patches to adapt to the 
logic. I would also vastly prefer that the `.pcm` filename match the *module* 
name, not the source file name. I'll also note that the example here is not 
suitable in the case of `Hello.cxx` and `Hello.cpp` existing at the same time 
(this is why CMake uses `Hello.cxx.o`). So some more control over the `.pcm` 
filename through more than source basename + `.pcm` is required in the general 
case (it is mostly mitigated if it is "replace `.o` with `.pcm`", but this is 
just unnecessary logic to ask anything interacting with Clang to deal with.

It also helps analyzers know that something else is happening explicitly and 
distributed builds to know what to bring back. Plus the other compilers offer 
controls over it; why does Clang have to be different?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137058

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

[PATCH] D136807: [clang][Sema] Fix a clang crash with btf_type_tag

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song updated this revision to Diff 472305.
yonghong-song added a comment.

- improve the bug description in release note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136807

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/attr-btf_type_tag-func-ptr.c


Index: clang/test/CodeGen/attr-btf_type_tag-func-ptr.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-btf_type_tag-func-ptr.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited -S 
-emit-llvm -o - %s | FileCheck %s
+
+struct t {
+ int (__attribute__((btf_type_tag("rcu"))) *f)();
+ int a;
+};
+int foo(struct t *arg) {
+  return arg->a;
+}
+
+// CHECK:  !DIDerivedType(tag: DW_TAG_member, name: "f"
+// CHECK-SAME: baseType: ![[L18:[0-9]+]]
+// CHECK:  ![[L18]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: 
![[#]], size: [[#]], annotations: ![[L21:[0-9]+]])
+// CHECK:  ![[L21]] = !{![[L22:[0-9]+]]}
+// CHECK:  ![[L22]] = !{!"btf_type_tag", !"rcu"}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -6515,6 +6515,9 @@
   CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
 }
 
+while (BTFTagAttributedTypeLoc TL = 
CurrTL.getAs())
+  CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
+
 while (DependentAddressSpaceTypeLoc TL =
CurrTL.getAs()) {
   fillDependentAddressSpaceTypeLoc(TL, D.getTypeObject(i).getAttrs());
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -268,6 +268,8 @@
   functions. `Issue 56154 `_
 - Fix handling of unexpanded packs in template argument expressions.
   `Issue 58679 `_
+- Fix a crash when a ``btf_type_tag`` attribute is applied to the pointee of
+  a function pointer.
 
 Improvements to Clang's diagnostics
 ^^^


Index: clang/test/CodeGen/attr-btf_type_tag-func-ptr.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-btf_type_tag-func-ptr.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited -S -emit-llvm -o - %s | FileCheck %s
+
+struct t {
+ int (__attribute__((btf_type_tag("rcu"))) *f)();
+ int a;
+};
+int foo(struct t *arg) {
+  return arg->a;
+}
+
+// CHECK:  !DIDerivedType(tag: DW_TAG_member, name: "f"
+// CHECK-SAME: baseType: ![[L18:[0-9]+]]
+// CHECK:  ![[L18]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[#]], size: [[#]], annotations: ![[L21:[0-9]+]])
+// CHECK:  ![[L21]] = !{![[L22:[0-9]+]]}
+// CHECK:  ![[L22]] = !{!"btf_type_tag", !"rcu"}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -6515,6 +6515,9 @@
   CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
 }
 
+while (BTFTagAttributedTypeLoc TL = CurrTL.getAs())
+  CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
+
 while (DependentAddressSpaceTypeLoc TL =
CurrTL.getAs()) {
   fillDependentAddressSpaceTypeLoc(TL, D.getTypeObject(i).getAttrs());
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -268,6 +268,8 @@
   functions. `Issue 56154 `_
 - Fix handling of unexpanded packs in template argument expressions.
   `Issue 58679 `_
+- Fix a crash when a ``btf_type_tag`` attribute is applied to the pointee of
+  a function pointer.
 
 Improvements to Clang's diagnostics
 ^^^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135919: [Clang] Correct when Itanium ABI guard variables are set for non-block variables with static or thread storage duration.

[Tom Honermann via Phabricator via cfe-commits]

tahonermann updated this revision to Diff 472306.
tahonermann added a comment.

Rebased.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135919

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
  clang/test/CodeGenCXX/cxx11-thread-local.cpp
  clang/test/CodeGenCXX/cxx1y-variable-template.cpp
  clang/test/CodeGenCXX/global-init.cpp
  clang/test/CodeGenCXX/static-data-member.cpp
  clang/test/OpenMP/threadprivate_codegen.cpp

Index: clang/test/OpenMP/threadprivate_codegen.cpp
===
--- clang/test/OpenMP/threadprivate_codegen.cpp
+++ clang/test/OpenMP/threadprivate_codegen.cpp
@@ -1585,11 +1585,11 @@
 // CHECK1-NEXT:[[GUARD_UNINITIALIZED:%.*]] = icmp eq i8 [[TMP0]], 0
 // CHECK1-NEXT:br i1 [[GUARD_UNINITIALIZED]], label [[INIT_CHECK:%.*]], label [[INIT_END:%.*]]
 // CHECK1:   init.check:
+// CHECK1-NEXT:store i8 1, ptr @_ZGVN2STI2S4E2stE, align 8
 // CHECK1-NEXT:[[TMP1:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB1]])
 // CHECK1-NEXT:call void @__kmpc_threadprivate_register(ptr @[[GLOB1]], ptr @_ZN2STI2S4E2stE, ptr @.__kmpc_global_ctor_..9, ptr null, ptr @.__kmpc_global_dtor_..10)
 // CHECK1-NEXT:call void @_ZN2S4C1Ei(ptr noundef nonnull align 4 dereferenceable(8) @_ZN2STI2S4E2stE, i32 noundef 23)
 // CHECK1-NEXT:[[TMP2:%.*]] = call i32 @__cxa_atexit(ptr @_ZN2S4D1Ev, ptr @_ZN2STI2S4E2stE, ptr @__dso_handle) #[[ATTR3]]
-// CHECK1-NEXT:store i8 1, ptr @_ZGVN2STI2S4E2stE, align 8
 // CHECK1-NEXT:br label [[INIT_END]]
 // CHECK1:   init.end:
 // CHECK1-NEXT:ret void
@@ -2214,11 +2214,11 @@
 // CHECK2-NEXT:[[GUARD_UNINITIALIZED:%.*]] = icmp eq i8 [[TMP0]], 0
 // CHECK2-NEXT:br i1 [[GUARD_UNINITIALIZED]], label [[INIT_CHECK:%.*]], label [[INIT_END:%.*]]
 // CHECK2:   init.check:
+// CHECK2-NEXT:store i8 1, ptr @_ZGVN2STI2S4E2stE, align 8
 // CHECK2-NEXT:[[TMP1:%.*]] = call i32 @__kmpc_global_thread_num(ptr @[[GLOB1]])
 // CHECK2-NEXT:call void @__kmpc_threadprivate_register(ptr @[[GLOB1]], ptr @_ZN2STI2S4E2stE, ptr @.__kmpc_global_ctor_..9, ptr null, ptr @.__kmpc_global_dtor_..10)
 // CHECK2-NEXT:call void @_ZN2S4C1Ei(ptr noundef nonnull align 4 dereferenceable(8) @_ZN2STI2S4E2stE, i32 noundef 23)
 // CHECK2-NEXT:[[TMP2:%.*]] = call i32 @__cxa_atexit(ptr @_ZN2S4D1Ev, ptr @_ZN2STI2S4E2stE, ptr @__dso_handle) #[[ATTR3]]
-// CHECK2-NEXT:store i8 1, ptr @_ZGVN2STI2S4E2stE, align 8
 // CHECK2-NEXT:br label [[INIT_END]]
 // CHECK2:   init.end:
 // CHECK2-NEXT:ret void
@@ -2697,9 +2697,9 @@
 // SIMD1-NEXT:[[GUARD_UNINITIALIZED:%.*]] = icmp eq i8 [[TMP0]], 0
 // SIMD1-NEXT:br i1 [[GUARD_UNINITIALIZED]], label [[INIT_CHECK:%.*]], label [[INIT_END:%.*]]
 // SIMD1:   init.check:
+// SIMD1-NEXT:store i8 1, ptr @_ZGVN2STI2S4E2stE, align 8
 // SIMD1-NEXT:call void @_ZN2S4C1Ei(ptr noundef nonnull align 4 dereferenceable(8) @_ZN2STI2S4E2stE, i32 noundef 23)
 // SIMD1-NEXT:[[TMP1:%.*]] = call i32 @__cxa_atexit(ptr @_ZN2S4D1Ev, ptr @_ZN2STI2S4E2stE, ptr @__dso_handle) #[[ATTR3]]
-// SIMD1-NEXT:store i8 1, ptr @_ZGVN2STI2S4E2stE, align 8
 // SIMD1-NEXT:br label [[INIT_END]]
 // SIMD1:   init.end:
 // SIMD1-NEXT:ret void
@@ -3167,9 +3167,9 @@
 // SIMD2-NEXT:[[GUARD_UNINITIALIZED:%.*]] = icmp eq i8 [[TMP0]], 0, !dbg [[DBG234]]
 // SIMD2-NEXT:br i1 [[GUARD_UNINITIALIZED]], label [[INIT_CHECK:%.*]], label [[INIT_END:%.*]], !dbg [[DBG234]]
 // SIMD2:   init.check:
+// SIMD2-NEXT:store i8 1, ptr @_ZGVN2STI2S4E2stE, align 8, !dbg [[DBG234]]
 // SIMD2-NEXT:call void @_ZN2S4C1Ei(ptr noundef nonnull align 4 dereferenceable(8) @_ZN2STI2S4E2stE, i32 noundef 23), !dbg [[DBG235:![0-9]+]]
 // SIMD2-NEXT:[[TMP1:%.*]] = call i32 @__cxa_atexit(ptr @_ZN2S4D1Ev, ptr @_ZN2STI2S4E2stE, ptr @__dso_handle) #[[ATTR3]], !dbg [[DBG234]]
-// SIMD2-NEXT:store i8 1, ptr @_ZGVN2STI2S4E2stE, align 8, !dbg [[DBG234]]
 // SIMD2-NEXT:br label [[INIT_END]], !dbg [[DBG234]]
 // SIMD2:   init.end:
 // SIMD2-NEXT:ret void, !dbg [[DBG237:![0-9]+]]
@@ -3767,9 +3767,9 @@
 // CHECK-TLS1-NEXT:[[GUARD_UNINITIALIZED:%.*]] = icmp eq i8 [[TMP0]], 0
 // CHECK-TLS1-NEXT:br i1 [[GUARD_UNINITIALIZED]], label [[INIT_CHECK:%.*]], label [[INIT_END:%.*]]
 // CHECK-TLS1:   init.check:
+// CHECK-TLS1-NEXT:store i8 1, ptr @_ZGVN2STI2S4E2stE, align 8
 // CHECK-TLS1-NEXT:call void @_ZN2S4C1Ei(ptr noundef nonnull align 4 dereferenceable(8) @_ZN2STI2S4E2stE, i32 noundef 23)
 // CHECK-TLS1-NEXT:[[TMP1:%.*]] = call i32 @__cxa_thread_atexit(ptr @_ZN2S4D1Ev, ptr @_ZN2STI2S4E2stE, ptr @__dso_handle) #[[ATTR3]]
-// CHECK-TLS1-NEXT:store i8 1, ptr @_ZGVN2STI2S4E2stE, align 8
 // CHECK-TLS1-NEXT:br label [[INIT_END]]
 // CHECK-TLS1:   init.end:
 // CHECK-TLS1-NEXT:ret void
@

[clang] 2f047b5 - [clang][Sema] Fix a clang crash with btf_type_tag

[Yonghong Song via cfe-commits]

Author: Yonghong Song
Date: 2022-11-01T08:07:13-07:00
New Revision: 2f047b56ba0e4830b617999a386f6fea79c16c91

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

LOG: [clang][Sema] Fix a clang crash with btf_type_tag

For the following program,
  $ cat t.c
  struct t {
   int (__attribute__((btf_type_tag("rcu"))) *f)();
   int a;
  };
  int foo(struct t *arg) {
return arg->a;
  }
Compiling with 'clang -g -O2 -S t.c' will cause a failure like below:
  clang: /home/yhs/work/llvm-project/clang/lib/Sema/SemaType.cpp:6391: void 
{anonymous}::DeclaratorLocFiller::VisitParenTypeLoc(clang::ParenTypeLoc):
 Assertion `Chunk.Kind == DeclaratorChunk::Paren' failed.
  PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
and include the crash backtrace, preprocessed source, and associated run script.
  Stack dump:
  ..
  #5 0x7f89e4280ea5 abort (/lib64/libc.so.6+0x21ea5)
  #6 0x7f89e4280d79 _nl_load_domain.cold.0 (/lib64/libc.so.6+0x21d79)
  #7 0x7f89e42a6456 (/lib64/libc.so.6+0x47456)
  #8 0x045c2596 GetTypeSourceInfoForDeclarator((anonymous 
namespace)::TypeProcessingState&, clang::QualType, clang::TypeSourceInfo*) 
SemaType.cpp:0:0
  #9 0x045ccfa5 GetFullTypeForDeclarator((anonymous 
namespace)::TypeProcessingState&, clang::QualType, clang::TypeSourceInfo*) 
SemaType.cpp:0:0
  ..

The reason of the failure is due to the mismatch of TypeLoc and 
D.getTypeObject().Kind. For example,
the TypeLoc is
  BTFTagAttributedType 0x88614e0 'int  btf_type_tag(rcu)()' sugar
  |-ParenType 0x8861480 'int ()' sugar
  | `-FunctionNoProtoType 0x8861450 'int ()' cdecl
  |   `-BuiltinType 0x87fd500 'int'
while corresponding D.getTypeObject().Kind points to DeclaratorChunk::Paren, and
this will cause later assertion.

To fix the issue, similar to AttributedTypeLoc, let us skip 
BTFTagAttributedTypeLoc in
GetTypeSourceInfoForDeclarator().

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

Added: 
clang/test/CodeGen/attr-btf_type_tag-func-ptr.c

Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaType.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f751f96d29e9d..aac52bfc6e8fa 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -268,6 +268,8 @@ Bug Fixes
   functions. `Issue 56154 `_
 - Fix handling of unexpanded packs in template argument expressions.
   `Issue 58679 `_
+- Fix a crash when a ``btf_type_tag`` attribute is applied to the pointee of
+  a function pointer.
 
 Improvements to Clang's diagnostics
 ^^^

diff  --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 9b2a5c7ef8e27..b05e33e855dc0 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -6515,6 +6515,9 @@ GetTypeSourceInfoForDeclarator(TypeProcessingState &State,
   CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
 }
 
+while (BTFTagAttributedTypeLoc TL = 
CurrTL.getAs())
+  CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
+
 while (DependentAddressSpaceTypeLoc TL =
CurrTL.getAs()) {
   fillDependentAddressSpaceTypeLoc(TL, D.getTypeObject(i).getAttrs());

diff  --git a/clang/test/CodeGen/attr-btf_type_tag-func-ptr.c 
b/clang/test/CodeGen/attr-btf_type_tag-func-ptr.c
new file mode 100644
index 0..29ca5f58e4b81
--- /dev/null
+++ b/clang/test/CodeGen/attr-btf_type_tag-func-ptr.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited -S 
-emit-llvm -o - %s | FileCheck %s
+
+struct t {
+ int (__attribute__((btf_type_tag("rcu"))) *f)();
+ int a;
+};
+int foo(struct t *arg) {
+  return arg->a;
+}
+
+// CHECK:  !DIDerivedType(tag: DW_TAG_member, name: "f"
+// CHECK-SAME: baseType: ![[L18:[0-9]+]]
+// CHECK:  ![[L18]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: 
![[#]], size: [[#]], annotations: ![[L21:[0-9]+]])
+// CHECK:  ![[L21]] = !{![[L22:[0-9]+]]}
+// CHECK:  ![[L22]] = !{!"btf_type_tag", !"rcu"}



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

[PATCH] D136807: [clang][Sema] Fix a clang crash with btf_type_tag

[Yonghong Song 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 rG2f047b56ba0e: [clang][Sema] Fix a clang crash with 
btf_type_tag (authored by yonghong-song).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136807

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/attr-btf_type_tag-func-ptr.c


Index: clang/test/CodeGen/attr-btf_type_tag-func-ptr.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-btf_type_tag-func-ptr.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited -S 
-emit-llvm -o - %s | FileCheck %s
+
+struct t {
+ int (__attribute__((btf_type_tag("rcu"))) *f)();
+ int a;
+};
+int foo(struct t *arg) {
+  return arg->a;
+}
+
+// CHECK:  !DIDerivedType(tag: DW_TAG_member, name: "f"
+// CHECK-SAME: baseType: ![[L18:[0-9]+]]
+// CHECK:  ![[L18]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: 
![[#]], size: [[#]], annotations: ![[L21:[0-9]+]])
+// CHECK:  ![[L21]] = !{![[L22:[0-9]+]]}
+// CHECK:  ![[L22]] = !{!"btf_type_tag", !"rcu"}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -6515,6 +6515,9 @@
   CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
 }
 
+while (BTFTagAttributedTypeLoc TL = 
CurrTL.getAs())
+  CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
+
 while (DependentAddressSpaceTypeLoc TL =
CurrTL.getAs()) {
   fillDependentAddressSpaceTypeLoc(TL, D.getTypeObject(i).getAttrs());
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -268,6 +268,8 @@
   functions. `Issue 56154 `_
 - Fix handling of unexpanded packs in template argument expressions.
   `Issue 58679 `_
+- Fix a crash when a ``btf_type_tag`` attribute is applied to the pointee of
+  a function pointer.
 
 Improvements to Clang's diagnostics
 ^^^


Index: clang/test/CodeGen/attr-btf_type_tag-func-ptr.c
===
--- /dev/null
+++ clang/test/CodeGen/attr-btf_type_tag-func-ptr.c
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -triple %itanium_abi_triple -debug-info-kind=limited -S -emit-llvm -o - %s | FileCheck %s
+
+struct t {
+ int (__attribute__((btf_type_tag("rcu"))) *f)();
+ int a;
+};
+int foo(struct t *arg) {
+  return arg->a;
+}
+
+// CHECK:  !DIDerivedType(tag: DW_TAG_member, name: "f"
+// CHECK-SAME: baseType: ![[L18:[0-9]+]]
+// CHECK:  ![[L18]] = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: ![[#]], size: [[#]], annotations: ![[L21:[0-9]+]])
+// CHECK:  ![[L21]] = !{![[L22:[0-9]+]]}
+// CHECK:  ![[L22]] = !{!"btf_type_tag", !"rcu"}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -6515,6 +6515,9 @@
   CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
 }
 
+while (BTFTagAttributedTypeLoc TL = CurrTL.getAs())
+  CurrTL = TL.getNextTypeLoc().getUnqualifiedLoc();
+
 while (DependentAddressSpaceTypeLoc TL =
CurrTL.getAs()) {
   fillDependentAddressSpaceTypeLoc(TL, D.getTypeObject(i).getAttrs());
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -268,6 +268,8 @@
   functions. `Issue 56154 `_
 - Fix handling of unexpanded packs in template argument expressions.
   `Issue 58679 `_
+- Fix a crash when a ``btf_type_tag`` attribute is applied to the pointee of
+  a function pointer.
 
 Improvements to Clang's diagnostics
 ^^^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137154: Adding nvvm_reflect clang builtin

[Hugh Delaney via Phabricator via cfe-commits]

hdelan updated this revision to Diff 472310.
hdelan added a comment.

Removing redundant check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137154

Files:
  llvm/lib/Target/NVPTX/NVVMReflect.cpp


Index: llvm/lib/Target/NVPTX/NVVMReflect.cpp
===
--- llvm/lib/Target/NVPTX/NVVMReflect.cpp
+++ llvm/lib/Target/NVPTX/NVVMReflect.cpp
@@ -40,7 +40,6 @@
 #include 
 #include 
 #define NVVM_REFLECT_FUNCTION "__nvvm_reflect"
-#define NVVM_REFLECT_LLVM_INTRINSIC_NAME "llvm.nvvm.reflect"
 
 using namespace llvm;
 
@@ -120,7 +119,6 @@
   continue;
 Function *Callee = Call->getCalledFunction();
 if (!Callee || (Callee->getName() != NVVM_REFLECT_FUNCTION &&
-Callee->getName() != NVVM_REFLECT_LLVM_INTRINSIC_NAME &&
 Callee->getIntrinsicID() != Intrinsic::nvvm_reflect))
   continue;
 


Index: llvm/lib/Target/NVPTX/NVVMReflect.cpp
===
--- llvm/lib/Target/NVPTX/NVVMReflect.cpp
+++ llvm/lib/Target/NVPTX/NVVMReflect.cpp
@@ -40,7 +40,6 @@
 #include 
 #include 
 #define NVVM_REFLECT_FUNCTION "__nvvm_reflect"
-#define NVVM_REFLECT_LLVM_INTRINSIC_NAME "llvm.nvvm.reflect"
 
 using namespace llvm;
 
@@ -120,7 +119,6 @@
   continue;
 Function *Callee = Call->getCalledFunction();
 if (!Callee || (Callee->getName() != NVVM_REFLECT_FUNCTION &&
-Callee->getName() != NVVM_REFLECT_LLVM_INTRINSIC_NAME &&
 Callee->getIntrinsicID() != Intrinsic::nvvm_reflect))
   continue;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137154: Adding nvvm_reflect clang builtin

[Hugh Delaney via Phabricator via cfe-commits]

hdelan updated this revision to Diff 472311.
hdelan added a comment.

Removing redundant check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137154

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def
  clang/test/CodeGenCUDA/nvvm-reflect.cu
  llvm/include/llvm/IR/IntrinsicsNVVM.td
  llvm/test/CodeGen/NVPTX/nvvm-reflect-opaque.ll
  llvm/test/CodeGen/NVPTX/nvvm-reflect.ll

Index: llvm/test/CodeGen/NVPTX/nvvm-reflect.ll
===
--- llvm/test/CodeGen/NVPTX/nvvm-reflect.ll
+++ llvm/test/CodeGen/NVPTX/nvvm-reflect.ll
@@ -41,7 +41,7 @@
   ret float %ret
 }
 
-declare i32 @llvm.nvvm.reflect.p0i8(i8*)
+declare i32 @llvm.nvvm.reflect(i8*)
 
 ; CHECK-LABEL: define i32 @intrinsic
 define i32 @intrinsic() {
@@ -49,7 +49,7 @@
 ; USE_FTZ_0: ret i32 0
 ; USE_FTZ_1: ret i32 1
   %ptr = tail call i8* @llvm.nvvm.ptr.constant.to.gen.p0i8.p4i8(i8 addrspace(4)* getelementptr inbounds ([11 x i8], [11 x i8] addrspace(4)* @str, i32 0, i32 0))
-  %reflect = tail call i32 @llvm.nvvm.reflect.p0i8(i8* %ptr)
+  %reflect = tail call i32 @llvm.nvvm.reflect(i8* %ptr)
   ret i32 %reflect
 }
 
Index: llvm/test/CodeGen/NVPTX/nvvm-reflect-opaque.ll
===
--- llvm/test/CodeGen/NVPTX/nvvm-reflect-opaque.ll
+++ llvm/test/CodeGen/NVPTX/nvvm-reflect-opaque.ll
@@ -41,7 +41,7 @@
   ret float %ret
 }
 
-declare i32 @llvm.nvvm.reflect.p0i8(ptr)
+declare i32 @llvm.nvvm.reflect(ptr)
 
 ; CHECK-LABEL: define i32 @intrinsic
 define i32 @intrinsic() {
@@ -49,7 +49,7 @@
 ; USE_FTZ_0: ret i32 0
 ; USE_FTZ_1: ret i32 1
   %ptr = tail call ptr @llvm.nvvm.ptr.constant.to.gen.p0i8.p4i8(ptr addrspace(4) @str)
-  %reflect = tail call i32 @llvm.nvvm.reflect.p0i8(ptr %ptr)
+  %reflect = tail call i32 @llvm.nvvm.reflect(ptr %ptr)
   ret i32 %reflect
 }
 
Index: llvm/include/llvm/IR/IntrinsicsNVVM.td
===
--- llvm/include/llvm/IR/IntrinsicsNVVM.td
+++ llvm/include/llvm/IR/IntrinsicsNVVM.td
@@ -1578,7 +1578,8 @@
 Intrinsic<[], [llvm_anyptr_ty], [], "llvm.nvvm.compiler.warn">;
 
 def int_nvvm_reflect :
-  Intrinsic<[llvm_i32_ty], [llvm_anyptr_ty], [IntrNoMem], "llvm.nvvm.reflect">;
+  Intrinsic<[llvm_i32_ty], [llvm_ptr_ty], [IntrNoMem], "llvm.nvvm.reflect">,
+  ClangBuiltin<"__nvvm_reflect">;
 
 // isspacep.{const, global, local, shared}
 def int_nvvm_isspacep_const
Index: clang/test/CodeGenCUDA/nvvm-reflect.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/nvvm-reflect.cu
@@ -0,0 +1,81 @@
+// REQUIRES: nvptx-registered-target
+
+// Checking to see that __nvvm_reflect resolves to the correct llvm.nvvm.reflect
+// intrinsic
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx64-nvidia-cuda -disable-llvm-passes -S -emit-llvm -x c++ %s -o - | FileCheck %s --check-prefix=NO_NVVM_REFLECT_PASS
+
+// Prepare bitcode file to link with
+// RUN: %clang_cc1 -triple nvptx-unknown-cuda -emit-llvm-bc \
+// RUN:-disable-llvm-passes -o %t.bc %s
+
+// Checking to see if the correct values are substituted for the nvvm_reflect
+// call when llvm passes are enabled.
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx64-nvidia-cuda -target-cpu \
+// RUN:sm_50 -S -o /dev/null %s -mllvm -print-after-all 2>&1 \
+// RUN:| FileCheck %s --check-prefix=ARCH_REFLECT_1
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx64-nvidia-cuda -target-cpu \
+// RUN:sm_52 -S -o /dev/null %s -mllvm -print-after-all 2>&1 \
+// RUN:| FileCheck %s --check-prefix=ARCH_REFLECT_2
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx64-nvidia-cuda -target-cpu \
+// RUN:sm_53 -S -o /dev/null %s -mllvm -print-after-all 2>&1 \
+// RUN:| FileCheck %s --check-prefix=ARCH_REFLECT_3
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx64-nvidia-cuda -target-cpu \
+// RUN:sm_60 -S -o /dev/null %s -mllvm -print-after-all 2>&1 \
+// RUN:| FileCheck %s --check-prefix=ARCH_REFLECT_4
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx64-nvidia-cuda -target-cpu \
+// RUN:sm_61 -S -o /dev/null %s -mllvm -print-after-all 2>&1 \
+// RUN:| FileCheck %s --check-prefix=ARCH_REFLECT_5
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx64-nvidia-cuda -target-cpu \
+// RUN:sm_62 -S -o /dev/null %s -mllvm -print-after-all 2>&1 \
+// RUN:| FileCheck %s --check-prefix=ARCH_REFLECT_6
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx64-nvidia-cuda -target-cpu \
+// RUN:sm_70 -S -o /dev/null %s -mllvm -print-after-all 2>&1 \
+// RUN:| FileCheck %s --check-prefix=ARCH_REFLECT_7
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx64-nvidia-cuda -target-cpu \
+// RUN:sm_72 -S -o /dev/null %s -mllvm -print-after-all 2>&1 \
+// RUN:| FileCheck %s --check-prefix=ARCH_REFLECT_8
+// RUN: %clang_cc1 -fcuda-is-device -triple nvptx64-nvidia-cuda -target-cpu \
+// RUN: 

[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:282
+  StringRef FirstComponentName = Path[0].first->getName();
+  if (!getSourceManager().isInSystemHeader(Path[0].second) &&
+  (FirstComponentName == "std" ||

aaron.ballman wrote:
> philnik wrote:
> > aaron.ballman wrote:
> > > philnik wrote:
> > > > aaron.ballman wrote:
> > > > > philnik wrote:
> > > > > > ChuanqiXu wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > erichkeane wrote:
> > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > cor3ntin wrote:
> > > > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > > > std modules should be irreverent with system 
> > > > > > > > > > > > > > > > headers; The intuition of the wording should be 
> > > > > > > > > > > > > > > > that the users can't declare modules like `std` 
> > > > > > > > > > > > > > > > or `std.compat` to avoid possible conflicting. 
> > > > > > > > > > > > > > > > The approach I imaged may be add a new 
> > > > > > > > > > > > > > > > compilation flags (call it `-fstd-modules`) 
> > > > > > > > > > > > > > > > now. And if the compiler found a `std` module 
> > > > > > > > > > > > > > > > declaration without `-fstd-modules`, emit an 
> > > > > > > > > > > > > > > > error.  
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > For now, I think we can skip the check for 
> > > > > > > > > > > > > > > > `-fstd-modules` and add it back when we starts 
> > > > > > > > > > > > > > > > to support std modules actually.
> > > > > > > > > > > > > > > The idea is that standard modules are built from 
> > > > > > > > > > > > > > > system directories... it seems a better heuristic 
> > > > > > > > > > > > > > > than adding a flag for the purpose of 1 
> > > > > > > > > > > > > > > diagnostics ( maybe some other system library 
> > > > > > > > > > > > > > > could in theory export std with no warning, but 
> > > > > > > > > > > > > > > I'm not super worried about that being a concern 
> > > > > > > > > > > > > > > in practice)
> > > > > > > > > > > > > > > The idea is that standard modules are built from 
> > > > > > > > > > > > > > > system directories...
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > This is not true. For example, if someday libc++ 
> > > > > > > > > > > > > > supports std modules, then we need to build the std 
> > > > > > > > > > > > > > modules in our working directory, which is not 
> > > > > > > > > > > > > > system directories. And **ideally**, we would 
> > > > > > > > > > > > > > distribute and install module file in the system 
> > > > > > > > > > > > > > directories. But it is irreverent with the path of 
> > > > > > > > > > > > > > the source file.
> > > > > > > > > > > > > > then we need to build the std modules in our 
> > > > > > > > > > > > > > working directory, which is not system directories.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > `-isystem`, pragmas, and linemarkers are all ways 
> > > > > > > > > > > > > around that -- I don't think we need a feature flag 
> > > > > > > > > > > > > for this, unless I'm misunderstanding something.
> > > > > > > > > > > > Although it may be a little bit nit picker, the module 
> > > > > > > > > > > > unit which declares the std modules won't be header. It 
> > > > > > > > > > > > is a module unit. So it requires we implement std 
> > > > > > > > > > > > modules by wrapping linemarkers around the std modules 
> > > > > > > > > > > > declaration, which looks a little bit overkill.
> > > > > > > > > > > > 
> > > > > > > > > > > > And another point is that maybe we need to introduce 
> > > > > > > > > > > > another feature flags to implement std modules. 
> > > > > > > > > > > > Although I tried to implement std modules within the 
> > > > > > > > > > > > current features, I can't prove we can implement std 
> > > > > > > > > > > > modules in that way in the end of the day.
> > > > > > > > > > > > 
> > > > > > > > > > > > Let me add some more words. The standards require us to 
> > > > > > > > > > > > implement std modules without deprecating the system 
> > > > > > > > > > > > headers. This strategy leads the direction to 
> > > > > > > > > > > > "implement the components in the original headers and 
> > > > > > > > > > > > control their visibility in the std module unit".
> > > > > > > > > > > > It may look like:
> > > > > > > > > > > > 
> > > > > > > > > > > > ```
> > > > > > > > > > > > //--- std.cppm
> > > > > > > > > > > > module;
> > > > > > > > > > > > #include 
> > > > > > > > > > > > ...
> > > > > > > > > > > > export module std;
> > > > > > > > > > > > ```
> > > > > > > > > > > > 
> > > > > > > > > > > > Then how can control the visibility?  In my original 
> > > > > > > > > > > > experiments, I use the style:
> > > > > > > > > > > > 

[PATCH] D136815: [clang][Interp] Unify visiting variable declarations

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

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:282
+  bool isGlobalDecl(const VarDecl *VD) const {
+return !VD->hasLocalStorage() || VD->isConstexpr();
+  }

shafik wrote:
> tbaeder wrote:
> > shafik wrote:
> > > Why not `hasGlobalStorage()`?
> > > 
> > > Also what is the logic behind `isConstexpr()` check?
> > Didn't know `isGlobalStorage()` existed ;)
> > 
> > Constexpr local variables can be handled like global ones, can't they? That 
> > was the logic behind it, nothing else. We can save ourselves the hassle of 
> > local variables in that case.
> I think I am missing a level of logic here. I don't think of constant 
> expressions as needing storage nor do I think of them as global variables.
> 
> So can you take a step back and explain how this fits in the bigger picture?
They don't necessarily need storage in the final executable but we create 
global/local variables for them for bookkeeping, e.g. we need to be able to 
take their address, track the value, etc.


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

https://reviews.llvm.org/D136815

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

[PATCH] D136554: Implement CWG2631

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 472315.
cor3ntin marked 8 inline comments as done.
cor3ntin added a comment.

Address Aaron's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136554

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/CXX/class/class.local/p1-0x.cpp
  clang/test/CodeGenCXX/default-arguments-with-immediate.cpp
  clang/test/PCH/default-argument-with-immediate-calls.cpp
  clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
  clang/test/SemaCXX/source_location.cpp

Index: clang/test/SemaCXX/source_location.cpp
===
--- clang/test/SemaCXX/source_location.cpp
+++ clang/test/SemaCXX/source_location.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s
+// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -DUSE_CONSTEVAL -fexceptions -verify %s
 // expected-no-diagnostics
 
 #define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42)
@@ -8,15 +9,22 @@
 template 
 struct Printer;
 
+#ifdef USE_CONSTEVAL
+#define SOURCE_LOC_EVAL_KIND consteval
+#else
+#define SOURCE_LOC_EVAL_KIND constexpr
+#endif
+
 namespace std {
 class source_location {
   struct __impl;
 
 public:
-  static constexpr source_location current(const __impl *__p = __builtin_source_location()) noexcept {
-source_location __loc;
-__loc.__m_impl = __p;
-return __loc;
+  static SOURCE_LOC_EVAL_KIND source_location
+current(const __impl *__p = __builtin_source_location()) noexcept {
+  source_location __loc;
+  __loc.__m_impl = __p;
+  return __loc;
   }
   constexpr source_location() = default;
   constexpr source_location(source_location const &) = default;
@@ -593,3 +601,32 @@
   }
   static_assert(test());
 }
+
+namespace Lambda {
+#line 8000 "TestLambda.cpp"
+constexpr int nested_lambda(int l = []{
+  return SL::current().line();
+}()) {
+  return l;
+}
+static_assert(nested_lambda() == __LINE__ - 4);
+
+constexpr int lambda_param(int l = [](int l = SL::current().line()) {
+  return l;
+}()) {
+  return l;
+}
+static_assert(lambda_param() == __LINE__);
+
+
+}
+
+constexpr int compound_literal_fun(int a =
+  (int){ std::source_location::current().line() }
+) { return a ;}
+static_assert(compound_literal_fun() == __LINE__);
+
+struct CompoundLiteral {
+  int a = (int){ std::source_location::current().line() };
+};
+static_assert(CompoundLiteral{}.a == __LINE__);
Index: clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2b %s
+
+consteval int undefined();  // expected-note 4 {{declared here}}
+
+void check_lambdas_unused(
+int a = []
+{
+return undefined();  // expected-error {{not a constant expression}} \
+ // expected-note  {{undefined function 'undefined'}}
+}(),
+int b = [](int no_error = undefined()) {
+return no_error;
+}(0),
+int c = [](int defaulted = undefined()) {
+return defaulted;
+}()
+) {}
+
+int check_lambdas_used(
+int b = [](int no_error = undefined()) {
+return no_error;
+}(0),
+int c = [](int defaulted = undefined()) { // expected-error {{not a constant expression}} \
+  // expected-note  {{declared here}} \
+  // expected-note  {{undefined function 'undefined'}}
+return defaulted;
+}(),  // expected-note {{in the default initalizer of 'defaulted'}}
+int d = [](int defaulted = sizeof(undefined())) {
+return defaulted;
+}()
+) {
+return 0;
+}
+
+int test_check_lambdas_used = check_lambdas_used();
+
+struct UnusedInitWithLambda {
+int a = [] {
+return undefined();  // expected-error {{not a constant expression}} \
+ // expected-note  {{undefined function 'undefined'}}
+}();
+// UnusedInitWithLambda is never constructed, so the initializer
+// of b and undefined() are never evaluated.
+int b = [](int no_error = undefined()) {
+return no_error;
+}();
+};
+
+consteval int ub(int n) {
+return 0/n; // expected-note  {{division}}
+}
+
+struct InitWithLambda {
+int b = [](i

[PATCH] D136554: Implement CWG2631

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

@aaron.ballman I tried to address your comment, except that:

- There is a bug with default parameter in blocks so that can't be tested
- Clang does not support default initialization of capture,  and if it did, I 
don't thee how we could support immediate calls in that easily.
- Compound statements in parameters are not supported.




Comment at: clang/include/clang/Sema/Sema.h:1349-1350
+  DeclContext *Context = nullptr;
+
+  bool isValid() const { return Decl != nullptr; }
+};

aaron.ballman wrote:
> This can now be removed as it's unused.
Nice catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136554

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

[PATCH] D137082: [clang][Interp] Fix dereferencing arrays with no offset applied

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

tbaeder added a comment.

In D137082#3899032 , @aaron.ballman 
wrote:

>> There is a difference between a Pointer and a "Pointer to the first element 
>> of an array".
>
> I'm pretty confused because this statement is false per the language standard 
> (http://eel.is/c++draft/expr.sub#2). Basically, array subscripting works 
> through pointer arithmetic, so `&array[0]` and `array` (decayed to a pointer) 
> have the same value. Why do we need to offset to get to the first element in 
> the interpreter?

That's just an implementation detail in the `Pointer` class. For primitive 
arrays, we need the `sizeof(InitMap*)` applied, which happens via `atIndex()`. 
Otherwise, `deref()` will look at the first few bytes of the `InitMap*` pointer.

I've added some documentation about this in https://reviews.llvm.org/D135750 
(and the `MetadataSize` added to `Descriptor` there could be used to clean this 
up I think).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137082

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

[PATCH] D136554: Implement CWG2631

[Corentin Jabot via Phabricator via cfe-commits]

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

Revert code committed accidentally


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136554

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/CXX/class/class.local/p1-0x.cpp
  clang/test/CodeGenCXX/default-arguments-with-immediate.cpp
  clang/test/PCH/default-argument-with-immediate-calls.cpp
  clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
  clang/test/SemaCXX/source_location.cpp

Index: clang/test/SemaCXX/source_location.cpp
===
--- clang/test/SemaCXX/source_location.cpp
+++ clang/test/SemaCXX/source_location.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s
+// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -DUSE_CONSTEVAL -fexceptions -verify %s
 // expected-no-diagnostics
 
 #define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42)
@@ -8,15 +9,22 @@
 template 
 struct Printer;
 
+#ifdef USE_CONSTEVAL
+#define SOURCE_LOC_EVAL_KIND consteval
+#else
+#define SOURCE_LOC_EVAL_KIND constexpr
+#endif
+
 namespace std {
 class source_location {
   struct __impl;
 
 public:
-  static constexpr source_location current(const __impl *__p = __builtin_source_location()) noexcept {
-source_location __loc;
-__loc.__m_impl = __p;
-return __loc;
+  static SOURCE_LOC_EVAL_KIND source_location
+current(const __impl *__p = __builtin_source_location()) noexcept {
+  source_location __loc;
+  __loc.__m_impl = __p;
+  return __loc;
   }
   constexpr source_location() = default;
   constexpr source_location(source_location const &) = default;
@@ -593,3 +601,32 @@
   }
   static_assert(test());
 }
+
+namespace Lambda {
+#line 8000 "TestLambda.cpp"
+constexpr int nested_lambda(int l = []{
+  return SL::current().line();
+}()) {
+  return l;
+}
+static_assert(nested_lambda() == __LINE__ - 4);
+
+constexpr int lambda_param(int l = [](int l = SL::current().line()) {
+  return l;
+}()) {
+  return l;
+}
+static_assert(lambda_param() == __LINE__);
+
+
+}
+
+constexpr int compound_literal_fun(int a =
+  (int){ std::source_location::current().line() }
+) { return a ;}
+static_assert(compound_literal_fun() == __LINE__);
+
+struct CompoundLiteral {
+  int a = (int){ std::source_location::current().line() };
+};
+static_assert(CompoundLiteral{}.a == __LINE__);
Index: clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2b %s
+
+consteval int undefined();  // expected-note 4 {{declared here}}
+
+void check_lambdas_unused(
+int a = []
+{
+return undefined();  // expected-error {{not a constant expression}} \
+ // expected-note  {{undefined function 'undefined'}}
+}(),
+int b = [](int no_error = undefined()) {
+return no_error;
+}(0),
+int c = [](int defaulted = undefined()) {
+return defaulted;
+}()
+) {}
+
+int check_lambdas_used(
+int b = [](int no_error = undefined()) {
+return no_error;
+}(0),
+int c = [](int defaulted = undefined()) { // expected-error {{not a constant expression}} \
+  // expected-note  {{declared here}} \
+  // expected-note  {{undefined function 'undefined'}}
+return defaulted;
+}(),  // expected-note {{in the default initalizer of 'defaulted'}}
+int d = [](int defaulted = sizeof(undefined())) {
+return defaulted;
+}()
+) {
+return 0;
+}
+
+int test_check_lambdas_used = check_lambdas_used();
+
+struct UnusedInitWithLambda {
+int a = [] {
+return undefined();  // expected-error {{not a constant expression}} \
+ // expected-note  {{undefined function 'undefined'}}
+}();
+// UnusedInitWithLambda is never constructed, so the initializer
+// of b and undefined() are never evaluated.
+int b = [](int no_error = undefined()) {
+return no_error;
+}();
+};
+
+consteval int ub(int n) {
+return 0/n; // expected-note  {{division}}
+}
+
+struct InitWithLambda {
+int b = [](int error = undefined()) { // expe

[PATCH] D136554: Implement CWG2631

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

> There is a bug with default parameter in blocks so that can't be tested

Block expressions don't actually support default arguments due to the type 
erasure that is fundamental to how they are implemented. Clang should diagnose 
default arguments in block expressions but doesn't currently 
(https://github.com/llvm/llvm-project/issues/58173).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136554

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

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss updated this revision to Diff 472321.
friss added a comment.

Add version number in cache file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/HeaderSearchOptions.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CMakeLists.txt
  clang/test/Driver/vfsstatcache.c
  clang/test/clang-stat-cache/cache-effects.c
  clang/test/clang-stat-cache/errors.test
  clang/tools/CMakeLists.txt
  clang/tools/clang-stat-cache/CMakeLists.txt
  clang/tools/clang-stat-cache/clang-stat-cache.cpp
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -17,6 +17,7 @@
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 #include 
 #include 
 
@@ -3159,3 +3160,302 @@
 "  DummyFileSystem (RecursiveContents)\n",
 Output);
 }
+
+class StatCacheFileSystemTest : public ::testing::Test {
+public:
+  void SetUp() override {}
+
+  template 
+  void createStatCacheFileSystem(
+  StringRef OutputFile, StringRef BaseDir, bool IsCaseSensitive,
+  IntrusiveRefCntPtr &Result,
+  StringCollection &Filenames,
+  IntrusiveRefCntPtr Lower = new ErrorDummyFileSystem(),
+  uint64_t ValidityToken = 0) {
+vfs::StatCacheFileSystem::StatCacheWriter Generator(
+BaseDir, IsCaseSensitive, ValidityToken);
+std::error_code ErrorCode;
+
+Result.reset();
+
+for (sys::fs::recursive_directory_iterator I(BaseDir, ErrorCode), E;
+ I != E && !ErrorCode; I.increment(ErrorCode)) {
+  Filenames.push_back(I->path());
+  StringRef Path(Filenames.back().c_str());
+  sys::fs::file_status s;
+  status(Path, s);
+  Generator.addEntry(Path, s);
+}
+
+{
+  raw_fd_ostream StatCacheFile(OutputFile, ErrorCode);
+  ASSERT_FALSE(ErrorCode);
+  Generator.writeStatCache(StatCacheFile);
+}
+
+loadCacheFile(OutputFile, ValidityToken, Lower, Result);
+  }
+
+  void loadCacheFile(StringRef OutputFile, uint64_t ExpectedValidityToken,
+ IntrusiveRefCntPtr Lower,
+ IntrusiveRefCntPtr &Result) {
+auto ErrorOrBuffer = MemoryBuffer::getFile(OutputFile);
+EXPECT_TRUE(ErrorOrBuffer);
+StringRef CacheBaseDir;
+bool IsCaseSensitive;
+bool VersionMatch;
+uint64_t FileValidityToken;
+auto E = vfs::StatCacheFileSystem::validateCacheFile(
+(*ErrorOrBuffer)->getMemBufferRef(), CacheBaseDir, IsCaseSensitive,
+FileValidityToken);
+ASSERT_FALSE(E);
+EXPECT_TRUE(VersionMatch);
+EXPECT_EQ(FileValidityToken, ExpectedValidityToken);
+auto ExpectedCache = vfs::StatCacheFileSystem::create(
+std::move(*ErrorOrBuffer), OutputFile, Lower);
+ASSERT_FALSE(ExpectedCache.takeError());
+Result = *ExpectedCache;
+  }
+
+  template 
+  void
+  compareStatCacheToRealFS(IntrusiveRefCntPtr CacheFS,
+   const StringCollection &Files) {
+IntrusiveRefCntPtr RealFS = vfs::getRealFileSystem();
+
+for (auto &File : Files) {
+  auto ErrorOrStatus1 = RealFS->status(File);
+  auto ErrorOrStatus2 = CacheFS->status(File);
+
+  EXPECT_EQ((bool)ErrorOrStatus1, (bool)ErrorOrStatus2);
+  if (!ErrorOrStatus1 || !ErrorOrStatus2)
+continue;
+
+  vfs::Status s1 = *ErrorOrStatus1, s2 = *ErrorOrStatus2;
+  EXPECT_EQ(s1.getName(), s2.getName());
+  EXPECT_EQ(s1.getType(), s2.getType());
+  EXPECT_EQ(s1.getPermissions(), s2.getPermissions());
+  EXPECT_EQ(s1.getLastModificationTime(), s2.getLastModificationTime());
+  EXPECT_EQ(s1.getUniqueID(), s2.getUniqueID());
+  EXPECT_EQ(s1.getUser(), s2.getUser());
+  EXPECT_EQ(s1.getGroup(), s2.getGroup());
+  EXPECT_EQ(s1.getSize(), s2.getSize());
+}
+  }
+};
+
+TEST_F(StatCacheFileSystemTest, Basic) {
+  TempDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  TempDir _a(TestDirectory.path("a"));
+  TempFile _ab(TestDirectory.path("a/b"));
+  TempDir _ac(TestDirectory.path("a/c"));
+  TempFile _acd(TestDirectory.path("a/c/d"), "", "Dummy contents");
+  TempFile _ace(TestDirectory.path("a/c/e"));
+  TempFile _acf(TestDirectory.path("a/c/f"), "", "More dummy contents");
+  TempDir _ag(TestDirectory.path("a/g"));
+  TempFile _agh(TestDirectory.path("a/g/h"));
+
+  StringRef BaseDir(_a.path());
+
+  SmallVector Filenames;
+  IntrusiveRefCntPtr StatCacheFS;
+  createStatCacheFileSystem(TestDirectory.path("stat.cache"), BaseDir,
+/* Is

[PATCH] D137059: [Driver] [Modules] Introduce -fsave-std-c++-module-file= to specify the path of the module file (2/2)

[Ben Boeckel via Phabricator via cfe-commits]

ben.boeckel added a comment.

There is another motivating factor for 1-phase: the build graph is far simpler. 
With 2-phase, CMake will have to write out rules to perform:

- source -> .bmi
- .bmi -> .withbmi.o
- source -> .o

because we do not know if a BMI is needed or not. If it isn't we use the 
latter. If it is, we use the former. Note that this also means we need 2 
different `.o` filenames (as neither `make` nor `ninja` doesn't support 
multiple rules making the same output). This also means that the collator needs 
to generate a response file for the linker to direct which `.o` file to use for 
each TU based on the contents.

Also with 2-phase, it is an open question of whether it actually helps with 
distributed builds (or anywhere process execution and I/O are expensive 
compared to some minimal work unit such as, say, Windows compiling from a 
network drive). Since this is not a bright line, giving the option to say "I 
know that split BMI is better for me in this instance" and "please combine 
here" would be handy (depending on actual real-world perf results on real-world 
projects). Yes, this is a chicken-and-egg cycle :) .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137059

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

[PATCH] D136474: [CodeView][clang] Disable emitting command line into CodeView by default and add flag to enable

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Just to be clear, in any case, this patch here is ready to go as long as it 
doesn't flip the default, yes?

> @thakis do you mind explaining the issues you see and how to replicate them 
> and I can have a look.

See above (`-fmessage-length` gets embedded and is machine-dependent).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136474

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

[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:282
+  StringRef FirstComponentName = Path[0].first->getName();
+  if (!getSourceManager().isInSystemHeader(Path[0].second) &&
+  (FirstComponentName == "std" ||

philnik wrote:
> aaron.ballman wrote:
> > philnik wrote:
> > > aaron.ballman wrote:
> > > > philnik wrote:
> > > > > aaron.ballman wrote:
> > > > > > philnik wrote:
> > > > > > > ChuanqiXu wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > erichkeane wrote:
> > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > > cor3ntin wrote:
> > > > > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > > > > std modules should be irreverent with system 
> > > > > > > > > > > > > > > > > headers; The intuition of the wording should 
> > > > > > > > > > > > > > > > > be that the users can't declare modules like 
> > > > > > > > > > > > > > > > > `std` or `std.compat` to avoid possible 
> > > > > > > > > > > > > > > > > conflicting. The approach I imaged may be add 
> > > > > > > > > > > > > > > > > a new compilation flags (call it 
> > > > > > > > > > > > > > > > > `-fstd-modules`) now. And if the compiler 
> > > > > > > > > > > > > > > > > found a `std` module declaration without 
> > > > > > > > > > > > > > > > > `-fstd-modules`, emit an error.  
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > For now, I think we can skip the check for 
> > > > > > > > > > > > > > > > > `-fstd-modules` and add it back when we 
> > > > > > > > > > > > > > > > > starts to support std modules actually.
> > > > > > > > > > > > > > > > The idea is that standard modules are built 
> > > > > > > > > > > > > > > > from system directories... it seems a better 
> > > > > > > > > > > > > > > > heuristic than adding a flag for the purpose of 
> > > > > > > > > > > > > > > > 1 diagnostics ( maybe some other system library 
> > > > > > > > > > > > > > > > could in theory export std with no warning, but 
> > > > > > > > > > > > > > > > I'm not super worried about that being a 
> > > > > > > > > > > > > > > > concern in practice)
> > > > > > > > > > > > > > > > The idea is that standard modules are built 
> > > > > > > > > > > > > > > > from system directories...
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > This is not true. For example, if someday libc++ 
> > > > > > > > > > > > > > > supports std modules, then we need to build the 
> > > > > > > > > > > > > > > std modules in our working directory, which is 
> > > > > > > > > > > > > > > not system directories. And **ideally**, we would 
> > > > > > > > > > > > > > > distribute and install module file in the system 
> > > > > > > > > > > > > > > directories. But it is irreverent with the path 
> > > > > > > > > > > > > > > of the source file.
> > > > > > > > > > > > > > > then we need to build the std modules in our 
> > > > > > > > > > > > > > > working directory, which is not system 
> > > > > > > > > > > > > > > directories.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > `-isystem`, pragmas, and linemarkers are all ways 
> > > > > > > > > > > > > > around that -- I don't think we need a feature flag 
> > > > > > > > > > > > > > for this, unless I'm misunderstanding something.
> > > > > > > > > > > > > Although it may be a little bit nit picker, the 
> > > > > > > > > > > > > module unit which declares the std modules won't be 
> > > > > > > > > > > > > header. It is a module unit. So it requires we 
> > > > > > > > > > > > > implement std modules by wrapping linemarkers around 
> > > > > > > > > > > > > the std modules declaration, which looks a little bit 
> > > > > > > > > > > > > overkill.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > And another point is that maybe we need to introduce 
> > > > > > > > > > > > > another feature flags to implement std modules. 
> > > > > > > > > > > > > Although I tried to implement std modules within the 
> > > > > > > > > > > > > current features, I can't prove we can implement std 
> > > > > > > > > > > > > modules in that way in the end of the day.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > Let me add some more words. The standards require us 
> > > > > > > > > > > > > to implement std modules without deprecating the 
> > > > > > > > > > > > > system headers. This strategy leads the direction to 
> > > > > > > > > > > > > "implement the components in the original headers and 
> > > > > > > > > > > > > control their visibility in the std module unit".
> > > > > > > > > > > > > It may look like:
> > > > > > > > > > > > > 
> > > > > > > > > > > > > ```
> > > > > > > > > > > > > //--- std.cppm
> > > > > > > > > > > > > module;
> > > > > > > > > > > > > #include 
> > > > > > > > > > > > > ...
> > > > > > > > > > > 

[PATCH] D137172: [Clang] Implement CWG2358 Explicit capture of value

[Corentin Jabot via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137172

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/drs/dr23xx.cpp
  clang/test/CXX/expr/expr.prim/expr.prim.lambda/p13.cpp
  clang/www/cxx_dr_status.html


Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -13956,7 +13956,7 @@
 https://wg21.link/cwg2358";>2358
 CD5
 Explicit capture of value
-Unknown
+Clang 16
   
   
 https://wg21.link/cwg2359";>2359
Index: clang/test/CXX/expr/expr.prim/expr.prim.lambda/p13.cpp
===
--- clang/test/CXX/expr/expr.prim/expr.prim.lambda/p13.cpp
+++ clang/test/CXX/expr/expr.prim/expr.prim.lambda/p13.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -std=c++11 %s -Wunused -Wno-unused-lambda-capture -verify
 
+const int global = 0;
 void f2() {
   int i = 1;
   void g1(int = ([i]{ return i; })()); // expected-error{{lambda expression in 
default argument cannot capture any entity}}
@@ -7,6 +8,8 @@
   void g3(int = ([=]{ return i; })()); // expected-error{{lambda expression in 
default argument cannot capture any entity}}
   void g4(int = ([=]{ return 0; })());
   void g5(int = ([]{ return sizeof i; })());
+  void g6(int = ([x=1, y = global, &z = global]{ return x; })());
+  void g7(int = ([x=i, &y=i]{ return x; })()); // expected-error 2{{default 
argument references local variable 'i' of enclosing function}}
 }
 
 namespace lambda_in_default_args {
Index: clang/test/CXX/drs/dr23xx.cpp
===
--- clang/test/CXX/drs/dr23xx.cpp
+++ clang/test/CXX/drs/dr23xx.cpp
@@ -89,6 +89,16 @@
 #pragma clang __debug dump not_use_2
 }
 
+#if __cplusplus >= 201402L
+namespace dr2358 { // dr2358: 16
+  void f2() {
+int i = 1;
+void g1(int = ([x=1] { return x; }))();  // OK
+void g2(int = ([x=i] { return x; }))();  // expected-error {{default 
argument references local variable 'i' of enclosing function}}
+  }
+}
+#endif
+
 #if __cplusplus >= 201707L
 // Otherwise, if the qualified-id std::tuple_size names a complete class
 // type **with a member value**, the expression std::tuple_size::value shall
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -149,13 +149,19 @@
 }
 
 bool CheckDefaultArgumentVisitor::VisitLambdaExpr(const LambdaExpr *Lambda) {
-  // C++11 [expr.lambda.prim]p13:
-  //   A lambda-expression appearing in a default argument shall not
-  //   implicitly or explicitly capture any entity.
-  if (Lambda->capture_begin() == Lambda->capture_end())
-return false;
-
-  return S.Diag(Lambda->getBeginLoc(), diag::err_lambda_capture_default_arg);
+  // [expr.prim.lambda.capture]/p9
+  // a lambda-expression appearing in a default argument cannot implicitly or 
explicitly capture any local entity.
+  // Such a lambda-expression can still have an init-capture if any 
full-expression in its initializer
+  // satisfies the constraints of an expression appearing in a default 
argument.
+  bool Invalid = false;
+  for(const LambdaCapture& LC : Lambda->captures()) {
+if(!Lambda->isInitCapture(&LC))
+  return S.Diag(LC.getLocation(), diag::err_lambda_capture_default_arg);
+// Init captures are always VarDecl.
+auto *D = cast(LC.getCapturedVar());
+Invalid |= Visit(D->getInit());
+  }
+  return Invalid;
 }
 } // namespace
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -507,6 +507,7 @@
   This means Clang will by default accept code using features from C++17 and
   conforming GNU extensions. Projects incompatible with C++17 can add
   ``-std=gnu++14`` to their build settings to restore the previous behaviour.
+- Implemented DR2358 allowing init captures in lambdas in default arguments.
 
 C++20 Feature Support
 ^


Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -13956,7 +13956,7 @@
 https://wg21.link/cwg2358";>2358
 CD5
 Explicit capture of value
-Unknown
+Clang 16
   
   
 https://wg21.link/cwg2359";>2359
Index: clang/test/CXX/expr/expr.prim/expr.prim.lambda/p13.cpp
===
--- clang/test/CXX/expr/expr.prim/expr.prim.lambda/p13.cpp
+++ clang/test/CXX/expr/expr.prim/expr.prim.lambda/p13.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -std=c++11 %s -Wunused -Wno-

[PATCH] D136554: Implement CWG2631

[Corentin Jabot via Phabricator via cfe-commits]

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

Thanks Tom for the useful info.

I modified the code and comment accordingly.
Ideally we would assert but we can't until 
that other issue is fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136554

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/CXX/class/class.local/p1-0x.cpp
  clang/test/CodeGenCXX/default-arguments-with-immediate.cpp
  clang/test/PCH/default-argument-with-immediate-calls.cpp
  clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
  clang/test/SemaCXX/source_location.cpp

Index: clang/test/SemaCXX/source_location.cpp
===
--- clang/test/SemaCXX/source_location.cpp
+++ clang/test/SemaCXX/source_location.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s
+// RUN: %clang_cc1 -std=c++2a -fcxx-exceptions -DUSE_CONSTEVAL -fexceptions -verify %s
 // expected-no-diagnostics
 
 #define assert(...) ((__VA_ARGS__) ? ((void)0) : throw 42)
@@ -8,15 +9,22 @@
 template 
 struct Printer;
 
+#ifdef USE_CONSTEVAL
+#define SOURCE_LOC_EVAL_KIND consteval
+#else
+#define SOURCE_LOC_EVAL_KIND constexpr
+#endif
+
 namespace std {
 class source_location {
   struct __impl;
 
 public:
-  static constexpr source_location current(const __impl *__p = __builtin_source_location()) noexcept {
-source_location __loc;
-__loc.__m_impl = __p;
-return __loc;
+  static SOURCE_LOC_EVAL_KIND source_location
+current(const __impl *__p = __builtin_source_location()) noexcept {
+  source_location __loc;
+  __loc.__m_impl = __p;
+  return __loc;
   }
   constexpr source_location() = default;
   constexpr source_location(source_location const &) = default;
@@ -593,3 +601,32 @@
   }
   static_assert(test());
 }
+
+namespace Lambda {
+#line 8000 "TestLambda.cpp"
+constexpr int nested_lambda(int l = []{
+  return SL::current().line();
+}()) {
+  return l;
+}
+static_assert(nested_lambda() == __LINE__ - 4);
+
+constexpr int lambda_param(int l = [](int l = SL::current().line()) {
+  return l;
+}()) {
+  return l;
+}
+static_assert(lambda_param() == __LINE__);
+
+
+}
+
+constexpr int compound_literal_fun(int a =
+  (int){ std::source_location::current().line() }
+) { return a ;}
+static_assert(compound_literal_fun() == __LINE__);
+
+struct CompoundLiteral {
+  int a = (int){ std::source_location::current().line() };
+};
+static_assert(CompoundLiteral{}.a == __LINE__);
Index: clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2b %s
+
+consteval int undefined();  // expected-note 4 {{declared here}}
+
+void check_lambdas_unused(
+int a = []
+{
+return undefined();  // expected-error {{not a constant expression}} \
+ // expected-note  {{undefined function 'undefined'}}
+}(),
+int b = [](int no_error = undefined()) {
+return no_error;
+}(0),
+int c = [](int defaulted = undefined()) {
+return defaulted;
+}()
+) {}
+
+int check_lambdas_used(
+int b = [](int no_error = undefined()) {
+return no_error;
+}(0),
+int c = [](int defaulted = undefined()) { // expected-error {{not a constant expression}} \
+  // expected-note  {{declared here}} \
+  // expected-note  {{undefined function 'undefined'}}
+return defaulted;
+}(),  // expected-note {{in the default initalizer of 'defaulted'}}
+int d = [](int defaulted = sizeof(undefined())) {
+return defaulted;
+}()
+) {
+return 0;
+}
+
+int test_check_lambdas_used = check_lambdas_used();
+
+struct UnusedInitWithLambda {
+int a = [] {
+return undefined();  // expected-error {{not a constant expression}} \
+ // expected-note  {{undefined function 'undefined'}}
+}();
+// UnusedInitWithLambda is never constructed, so the initializer
+// of b and undefined() are never evaluated.
+int b = [](int no_error = undefined()) {
+return no_error;
+}();
+};
+
+consteval int ub(int n) {
+return 0/

[PATCH] D137073: [clang] Fix inline builtin functions of an __asm__ renamed function with symbol prefixes

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm, thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137073

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

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Reverse ping. This has been accepted, what is the status of landing this?


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

https://reviews.llvm.org/D123630

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

[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:282
+  StringRef FirstComponentName = Path[0].first->getName();
+  if (!getSourceManager().isInSystemHeader(Path[0].second) &&
+  (FirstComponentName == "std" ||

aaron.ballman wrote:
> philnik wrote:
> > aaron.ballman wrote:
> > > philnik wrote:
> > > > aaron.ballman wrote:
> > > > > philnik wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > philnik wrote:
> > > > > > > > ChuanqiXu wrote:
> > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > erichkeane wrote:
> > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > > > cor3ntin wrote:
> > > > > > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > > > > > std modules should be irreverent with 
> > > > > > > > > > > > > > > > > > system headers; The intuition of the 
> > > > > > > > > > > > > > > > > > wording should be that the users can't 
> > > > > > > > > > > > > > > > > > declare modules like `std` or `std.compat` 
> > > > > > > > > > > > > > > > > > to avoid possible conflicting. The approach 
> > > > > > > > > > > > > > > > > > I imaged may be add a new compilation flags 
> > > > > > > > > > > > > > > > > > (call it `-fstd-modules`) now. And if the 
> > > > > > > > > > > > > > > > > > compiler found a `std` module declaration 
> > > > > > > > > > > > > > > > > > without `-fstd-modules`, emit an error.  
> > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > For now, I think we can skip the check for 
> > > > > > > > > > > > > > > > > > `-fstd-modules` and add it back when we 
> > > > > > > > > > > > > > > > > > starts to support std modules actually.
> > > > > > > > > > > > > > > > > The idea is that standard modules are built 
> > > > > > > > > > > > > > > > > from system directories... it seems a better 
> > > > > > > > > > > > > > > > > heuristic than adding a flag for the purpose 
> > > > > > > > > > > > > > > > > of 1 diagnostics ( maybe some other system 
> > > > > > > > > > > > > > > > > library could in theory export std with no 
> > > > > > > > > > > > > > > > > warning, but I'm not super worried about that 
> > > > > > > > > > > > > > > > > being a concern in practice)
> > > > > > > > > > > > > > > > > The idea is that standard modules are built 
> > > > > > > > > > > > > > > > > from system directories...
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > This is not true. For example, if someday 
> > > > > > > > > > > > > > > > libc++ supports std modules, then we need to 
> > > > > > > > > > > > > > > > build the std modules in our working directory, 
> > > > > > > > > > > > > > > > which is not system directories. And 
> > > > > > > > > > > > > > > > **ideally**, we would distribute and install 
> > > > > > > > > > > > > > > > module file in the system directories. But it 
> > > > > > > > > > > > > > > > is irreverent with the path of the source file.
> > > > > > > > > > > > > > > > then we need to build the std modules in our 
> > > > > > > > > > > > > > > > working directory, which is not system 
> > > > > > > > > > > > > > > > directories.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > `-isystem`, pragmas, and linemarkers are all ways 
> > > > > > > > > > > > > > > around that -- I don't think we need a feature 
> > > > > > > > > > > > > > > flag for this, unless I'm misunderstanding 
> > > > > > > > > > > > > > > something.
> > > > > > > > > > > > > > Although it may be a little bit nit picker, the 
> > > > > > > > > > > > > > module unit which declares the std modules won't be 
> > > > > > > > > > > > > > header. It is a module unit. So it requires we 
> > > > > > > > > > > > > > implement std modules by wrapping linemarkers 
> > > > > > > > > > > > > > around the std modules declaration, which looks a 
> > > > > > > > > > > > > > little bit overkill.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > And another point is that maybe we need to 
> > > > > > > > > > > > > > introduce another feature flags to implement std 
> > > > > > > > > > > > > > modules. Although I tried to implement std modules 
> > > > > > > > > > > > > > within the current features, I can't prove we can 
> > > > > > > > > > > > > > implement std modules in that way in the end of the 
> > > > > > > > > > > > > > day.
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > Let me add some more words. The standards require 
> > > > > > > > > > > > > > us to implement std modules without deprecating the 
> > > > > > > > > > > > > > system headers. This strategy leads the direction 
> > > > > > > > > > > > > > to "implement the components in the original 
> > > > > > > > > > > > > > headers and control their visibility in the std 
> > > > > > > > > > > > > > module unit".
> > > > > > > > 

[PATCH] D137175: [Driver][test] Remove one more obselete REQUIRES: clang-driver

[Jacob Lambert via Phabricator via cfe-commits]

lamb-j created this revision.
Herald added a project: All.
lamb-j requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

These were globally removed by 63fbc77 
 but this 
one was
inadvertently re-added by afcc6ba 
 just 
after that patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137175

Files:
  clang/test/Driver/hip-link-bc-to-bc.hip


Index: clang/test/Driver/hip-link-bc-to-bc.hip
===
--- clang/test/Driver/hip-link-bc-to-bc.hip
+++ clang/test/Driver/hip-link-bc-to-bc.hip
@@ -1,4 +1,4 @@
-// REQUIRES: clang-driver, x86-registered-target, amdgpu-registered-target
+// REQUIRES: x86-registered-target, amdgpu-registered-target
 
 // Check that clang unbundles the two bitcodes and links via llvm-link
 // RUN: touch %T/bundle1.bc


Index: clang/test/Driver/hip-link-bc-to-bc.hip
===
--- clang/test/Driver/hip-link-bc-to-bc.hip
+++ clang/test/Driver/hip-link-bc-to-bc.hip
@@ -1,4 +1,4 @@
-// REQUIRES: clang-driver, x86-registered-target, amdgpu-registered-target
+// REQUIRES: x86-registered-target, amdgpu-registered-target
 
 // Check that clang unbundles the two bitcodes and links via llvm-link
 // RUN: touch %T/bundle1.bc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

In D123630#3899550 , @lenary wrote:

> Reverse ping. This has been accepted, what is the status of landing this?

As far as I can tell this has landed.


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

https://reviews.llvm.org/D123630

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

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

[Sam Elliott via Phabricator via cfe-commits]

lenary added a comment.

Oh, can you close this revision and link it to the landed commit please?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123630

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

[PATCH] D137154: Adding nvvm_reflect clang builtin

[Artem Belevich via Phabricator via cfe-commits]

tra added a subscriber: yaxunl.
tra added a comment.

I don't think it's a good idea. `__nvvm_reflect` is a hack that we should not 
propagate. The only code that currently relies on it is NVIDIA's libdevice 
bitcode and I'd prefer to keep it that way.

Can you elaborate on what motivates this change?

We already have a way to do conditional compilation based on the GPU 
architecture.
If you need the code to know whether FTZ mode is enabled or not, that should be 
exposed on its own. I'm not convinced that it would be a great idea, either. 
LLVM has a lot of ways to control FP code generation (that I'm mostly 
unqualified to comment on) but those options are fine-grained. `__nvvm_reflect` 
would only affect things module-wise and would not always tell you whether FTZ 
instruction variants would be used. E.g. 
llvm/test/CodeGen/NVPTX/math-intrins.ll shows that `ftz` can be controlled per 
function via `"denormal-fp-math-f32" = "preserve-sign"` attribute.

Another aspect of this is that the concept of `ftz` is not unique to NVPTX. I 
believe AMDGPU has `ftz` instruction variants, too. @yaxunl - FYI.

If you need to control what FP instruction variant we generate, you should 
probably use `#pragma clang fp ...` for that and *tell* compiler what you need. 
https://clang.llvm.org/docs/LanguageExtensions.html#extensions-to-specify-floating-point-flags

I do not think clang currently exposes fine-grained details about selected FP 
modes. We do have `__FAST_MATH__`, `__FINITE_MATH_ONLY__`, and 
`__FAST_RELAXED_MATH__`, but that seems to be about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137154

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

[PATCH] D136533: [clang] Fix missing diagnostic of declaration use when accessing TypeDecls through typename access

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In D136533#3896536 , @mizvekov wrote:

> Now that you mention it, looking at the code, I think we don't diagnose an 
> use of a type alias itself, if that is what you mean?
>
> Ie, clang doesn't, GCC does, MSVC doesn't: https://godbolt.org/z/WEo4enjbz
>
> Would fixing that be a problem for libc++?

It might be a problem, but I would argue we should still do it after fixing any 
problematic cases. It seems like Clang's current behavior is broken, as it 
basically ignores the `[[deprecated]]` attribute on aliases?

> Otherwise, typename accesses through the type alias pattern itself should 
> work like from any context, except that if the access happens through a 
> dependent entity, we should be diagnosing it when we instantiate the type 
> alias.
>
> Does what you are talking about fall into any of that? Otherwise, do you have 
> a short example?

Sorry, I'm a bit lost. Let's take it back to the errors we're actually seeing 
with my reproducer above:

  In file included from :1:
  In file included from SDK/usr/include/c++/v1/filesystem:245:
  In file included from 
SDK/usr/include/c++/v1/__filesystem/directory_entry.h:14:
  In file included from SDK/usr/include/c++/v1/__chrono/time_point.h:13:
  In file included from SDK/usr/include/c++/v1/__chrono/duration.h:14:
  In file included from SDK/usr/include/c++/v1/limits:107:
  In file included from SDK/usr/include/c++/v1/type_traits:421:
  In file included from SDK/usr/include/c++/v1/__functional/invoke.h:17:
  In file included from SDK/usr/include/c++/v1/__type_traits/decay.h:13:
  SDK/usr/include/c++/v1/__type_traits/add_pointer.h:26:50: error: 'type' is 
unavailable: introduced in macOS 10.15
  _IsSame::type, void>::value>
   ^
  SDK/usr/include/c++/v1/__type_traits/add_pointer.h:33:39: note: in 
instantiation of default argument for 
'__add_pointer_impl' required here
  {typedef _LIBCPP_NODEBUG typename __add_pointer_impl<_Tp>::type type;};
^~~
  SDK/usr/include/c++/v1/__type_traits/decay.h:44:40: note: in instantiation of 
template class 'std::add_pointer' requested here
typename add_pointer<_Up>::type,
 ^
  SDK/usr/include/c++/v1/__type_traits/decay.h:56:38: note: in instantiation of 
template class 'std::__decay' requested here
  typedef _LIBCPP_NODEBUG typename __decay<_Up, 
__is_referenceable<_Up>::value>::type type;
   ^
  SDK/usr/include/c++/v1/__filesystem/path.h:142:47: note: in instantiation of 
template class 'std::decay' requested here
  template ::type,
^
  SDK/usr/include/c++/v1/__filesystem/path.h:195:31: note: in instantiation of 
default argument for '__is_pathable_char_array' 
required here
bool _IsCharIterT = __is_pathable_char_array<_Tp>::value,
^
  SDK/usr/include/c++/v1/__filesystem/path.h:445:26: note: in instantiation of 
default argument for '__is_pathable' 
required here
typename enable_if<__is_pathable<_SourceOrIter>::value, _Tp>::type;
   ^~~~
  SDK/usr/include/c++/v1/__filesystem/path.h:480:36: note: in instantiation of 
template type alias '_EnableIfPathable' requested here
template  >
 ^
  SDK/usr/include/c++/v1/__filesystem/path.h:482:3: note: in instantiation of 
default argument for 'path' required here
path(const _Source& __src, format = format::auto_format) {
^~
  SDK/usr/include/c++/v1/__filesystem/operations.h:99:75: note: while 
substituting deduced template arguments into function template 'path' [with 
_Source = file_status, $1 = (no value)]
  inline _LIBCPP_HIDE_FROM_ABI bool exists(const path& __p) { return 
exists(__status(__p)); }
^
  SDK/usr/include/c++/v1/__filesystem/file_status.h:28:24: note: 'file_status' 
has been explicitly marked unavailable here
  class _LIBCPP_TYPE_VIS file_status {
 ^

`file_status`, `exists` and `path::path` are all marked as unavailable. Why do 
we diagnose the use of an unavailable type wayyy lower in the stack when we 
instantiate `remove_cv`? I am not seeing what libc++ is doing wrong.

> Otherwise, if current libc++ is fine and building older libc++with newer 
> clang is not supported, is everyone fine with merging this back, as is?
> @thakis ?

See below for what I meant by "unsupported". Another way to think about this 
would be to say that Clang has the burden of keeping a slightly-older libc++ 
compiling. If that's the case, then one could argue that this change should be 
held off until Clang no lon

[PATCH] D136474: [CodeView][clang] Disable emitting command line into CodeView by default and add flag to enable

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks updated this revision to Diff 472351.
aeubanks added a comment.

on by default


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136474

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/debug-info-codeview-buildinfo.c
  clang/test/Driver/gcodeview-command-line.c

Index: clang/test/Driver/gcodeview-command-line.c
===
--- /dev/null
+++ clang/test/Driver/gcodeview-command-line.c
@@ -0,0 +1,19 @@
+// Note: %s must be preceded by --, otherwise it may be interpreted as a
+// command-line option, e.g. on Mac where %s is commonly under /Users.
+
+// ON-NOT: "-gno-codview-commandline"
+// OFF: "-gno-codeview-command-line"
+
+// default
+// RUN: %clang_cl /Z7 -### -- %s 2>&1 | FileCheck -check-prefix=ON %s
+// enabled
+// RUN: %clang_cl /Z7 -gno-codeview-command-line -gcodeview-command-line -### -- %s 2>&1 | FileCheck -check-prefix=ON %s
+// disabled
+// RUN: %clang_cl /Z7 -gcodeview-command-line -gno-codeview-command-line -### -- %s 2>&1 | FileCheck -check-prefix=OFF %s
+
+// enabled, no /Z7
+// RUN: %clang_cl -gcodeview-command-line -### -- %s 2>&1 | FileCheck -check-prefix=ON %s
+
+// GCC-style driver
+// RUN: %clang -g -gcodeview -gno-codeview-command-line -gcodeview-command-line -### -- %s 2>&1 | FileCheck -check-prefix=ON %s
+// RUN: %clang -g -gcodeview -gcodeview-command-line -gno-codeview-command-line -### -- %s 2>&1 | FileCheck -check-prefix=OFF %s
Index: clang/test/CodeGen/debug-info-codeview-buildinfo.c
===
--- clang/test/CodeGen/debug-info-codeview-buildinfo.c
+++ clang/test/CodeGen/debug-info-codeview-buildinfo.c
@@ -1,8 +1,12 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cl --target=i686-windows-msvc /c /Z7 /Fo%t.obj -- %s
 // RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s
+// RUN: %clang_cl -gcodeview-command-line --target=i686-windows-msvc /c /Z7 /Fo%t.obj -- %s
+// RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s
 // RUN: %clang_cl --target=i686-windows-msvc /c /Z7 /Fo%t.obj -fdebug-compilation-dir=. -- %s
 // RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s --check-prefix RELATIVE
+// RUN: %clang_cl -gno-codeview-command-line --target=i686-windows-msvc /c /Z7 /Fo%t.obj -- %s
+// RUN: llvm-pdbutil dump --types %t.obj | FileCheck %s --check-prefix DISABLE
 
 int main(void) { return 42; }
 
@@ -14,13 +18,21 @@
 // CHECK: 0x[[TOOL:.+]] | LF_STRING_ID [size = {{.+}}] ID: , String: [[TOOLVAL:.+[\\/]clang.*]]
 // CHECK: 0x[[CMDLINE:.+]] | LF_STRING_ID [size = {{.+}}] ID: , String: "-cc1
 // CHECK: 0x{{.+}} | LF_BUILDINFO [size = {{.+}}]
-// CHECK:  0x[[PWD]]: `[[PWDVAL]]`
-// CHECK:  0x[[TOOL]]: `[[TOOLVAL]]`
-// CHECK:  0x[[FILEPATH]]: `[[FILEPATHVAL]]`
-// CHECK:  0x[[ZIPDB]]: ``
-// CHECK:  0x[[CMDLINE]]: `"-cc1
+// CHECK-NEXT:  0x[[PWD]]: `[[PWDVAL]]`
+// CHECK-NEXT:  0x[[TOOL]]: `[[TOOLVAL]]`
+// CHECK-NEXT:  0x[[FILEPATH]]: `[[FILEPATHVAL]]`
+// CHECK-NEXT:  0x[[ZIPDB]]: ``
+// CHECK-NEXT:  0x[[CMDLINE]]: `"-cc1
 
 // RELATIVE:   Types (.debug$T)
 // RELATIVE: 
 // RELATIVE: 0x{{.+}} | LF_BUILDINFO [size = {{.+}}]
 // RELATIVE:  0x{{.+}}: `.`
+
+// DISABLE-NOT: cc1
+// DISABLE: 0x{{.+}} | LF_BUILDINFO [size = {{.+}}]
+// DISABLE-NEXT:  0x{{.+}}: `{{.*}}`
+// DISABLE-NEXT:  : ``
+// DISABLE-NEXT:  0x{{.+}}: `{{.*}}`
+// DISABLE-NEXT:  0x{{.+}}: ``
+// DISABLE-NEXT:  : ``
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -4546,8 +4546,10 @@
   }
 
   // Store the command-line for using in the CodeView backend.
-  Res.getCodeGenOpts().Argv0 = Argv0;
-  append_range(Res.getCodeGenOpts().CommandLineArgs, CommandLineArgs);
+  if (Res.getCodeGenOpts().CodeViewCommandLine) {
+Res.getCodeGenOpts().Argv0 = Argv0;
+append_range(Res.getCodeGenOpts().CommandLineArgs, CommandLineArgs);
+  }
 
   FixupInvocation(Res, Diags, Args, DashX);
 
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4315,9 +4315,14 @@
 
   if (EmitCodeView) {
 CmdArgs.push_back("-gcodeview");
+
 Args.addOptInFlag(CmdArgs, options::OPT_gcodeview_ghash,
   options::OPT_gno_codeview_ghash);
+
+Args.addOptOutFlag(CmdArgs, options::OPT_gcodeview_command_line,
+   options::OPT_gno_codeview_

[PATCH] D133289: [C2X] N3007 Type inference for object definitions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for the update to this!




Comment at: clang/lib/Parse/ParseExpr.cpp:1526
+// This is a temporary fix while we don't support C2x 6.5.2.5p4
+if (getLangOpts().C2x && GetLookAheadToken(2).getKind() == tok::l_brace) {
+  Diag(Tok, diag::err_c2x_auto_compound_literal_not_allowed);

I don't think this is quite right; I suspect we're going to need more 
complicated logic here. Consider a case like: `(auto const){ 12 }` or `(auto 
*){ nullptr }` where two tokens ahead is `)` and not `{`. (Note, these type 
specifications can be pretty arbitrarily complex.)

Given that this is a temporary workaround for a lack of support for storage 
class specifiers in compound literals, perhaps another approach is to not try 
to catch uses in compound literals. Instead, we could add tests with FIXME 
comments to demonstrate the behavior we get with compound literals now, and 
when we do that storage class specifier work, we will (hopefully) break those 
tests and come back to make them correct.



Comment at: clang/lib/Parse/ParseExpr.cpp:1530-1531
+}
+  }
+[[fallthrough]];
+

This looks a bit less visually jarring to me (the indentation differences were 
distracting). WDYT?



Comment at: clang/lib/Sema/DeclSpec.cpp:1362
   // type specifier.
   if (S.getLangOpts().CPlusPlus &&
   TypeSpecType == TST_unspecified && StorageClassSpec == SCS_auto) {

to268 wrote:
> Do we need to include C2x just in case?
I don't think so -- we're in a language where the type specifier *is* optional 
for C2x.



Comment at: clang/lib/Sema/DeclSpec.cpp:1370-1372
+  // specifier in a pre-C++11 dialect of C++
+  if ((!S.getLangOpts().CPlusPlus11 && !S.getLangOpts().C2x) &&
+  TypeSpecType == TST_auto)





Comment at: clang/test/C/C2x/n3007.c:13
+  int* pc = &c; // expected-warning {{incompatible pointer types initializing 
'int *' with an expression of type 'unsigned long *'}}
+
+  const int ci = 12;

I'd also like a test for:
```
_Atomic auto i = 12;
_Atomic(auto) j = 12;

_Atomic(int) foo(void);
auto k = foo(); // Do we get _Atomic(int) or just int?
```



Comment at: clang/test/C/C2x/n3007.c:36
+  auto auto_size = sizeof(auto);  // expected-error {{expected expression}}
+  _Alignas(4) auto b[4];  // expected-error {{'b' declared as array of 
'auto'}}
+}

I think this test should be `_Alignas(auto)` right?



Comment at: clang/test/C/C2x/n3007.c:40-41
+void test_arrary(void) {
+  auto a[4];  // expected-error {{'a' declared as array of 'auto'}}
+  auto b[] = {1, 2};  // expected-error {{'b' declared as array of 'auto'}}
+}

I think other array cases we want to test are:
```
auto a = { 1 , 2 }; // Error
auto b = { 1, }; // OK
auto c = (int [3]){ 1, 2, 3 }; // OK
```



Comment at: clang/test/C/C2x/n3007.c:56
+
+  _Static_assert(_Generic(test_char_ptr, const char * : 1, char * : 2) == 2, 
"C is weird");
+}

I'd also like to ensure we reject:
```
typedef auto (*fp)(void);
typedef void (*fp)(auto);

_Generic(0, auto : 1);
```
and we should ensure we properly get integer promotions:
```
short s;
auto a = s;
_Generic(a, int : 1);
```
and a test for use of an explicit pointer declarator (which is UB that we can 
define if we want to):
```
int a;
auto *ptr = &a; // Okay?
auto *ptr = a; // Error
```



Comment at: clang/test/Sema/c2x-auto.c:27
+  auto auto_cl = (auto){13};  // expected-error {{'auto' is not allowed in a 
compound literal}}
+  auto array[] = { 1, 2, 3 }; // expected-error {{'array' declared as array of 
'auto'}}
+}

This one should not be accepted -- the grammar productions for the 
initialization allow for `{0}` and `{0,}` but no more than one element in the 
braces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133289

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

[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

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

dexonsmith added a comment.

In D135220#3870991 , @hans wrote:

> One thought, which I'm not sure is relevant, is that this is only observable 
> for headers which are included more than once, which is rare because normally 
> there are include guards (or pragma once). `isFileMultipleIncludeGuarded` 
> isn't tracked by the FileManager, but otherwise maybe that could have been a 
> useful heuristic: don't merge header files without include guards?

Maybe that heuristic would work a level up, in SourceManager.

- If multiple-include-guarded, create one FileID per FileEntry.
- Else, create one FileID per FileEntryRef.

Although it gets complicated with language features like `#import` in 
Objective-C, where textual inclusion is implicitly multiple-include-guarded. 
Consider a file included both using `#include` (not guarded) and as `#import` 
(guarded).

And I'm not sure we really want to split the FileIDs... that seems like a 
potential performance regression. Instead, I think we just want to track (at 
the use site) how the file was referenced.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135220

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

[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

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

dexonsmith added a comment.

In D135220#3898849 , @hans wrote:

> Relatedly, we ran into a problem with `clang-cl /showIncludes` not including 
> all files in its output when they're linked: 
> https://github.com/llvm/llvm-project/issues/58726

Interestingly, that discussion points out that `-MD` works correctly:

  $ clang -MD -c /tmp/a.cc && cat a.d 
  a.o: /tmp/a.cc /tmp/foo.h /tmp/bar.h

I presume this is because we've pushed FileEntryRef through dependency 
tracking. Just need to push it through more places, I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135220

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

[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:282
+  StringRef FirstComponentName = Path[0].first->getName();
+  if (!getSourceManager().isInSystemHeader(Path[0].second) &&
+  (FirstComponentName == "std" ||

philnik wrote:
> aaron.ballman wrote:
> > philnik wrote:
> > > aaron.ballman wrote:
> > > > philnik wrote:
> > > > > aaron.ballman wrote:
> > > > > > philnik wrote:
> > > > > > > aaron.ballman wrote:
> > > > > > > > philnik wrote:
> > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > erichkeane wrote:
> > > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > > aaron.ballman wrote:
> > > > > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > > > > cor3ntin wrote:
> > > > > > > > > > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > > > > > > > > > std modules should be irreverent with 
> > > > > > > > > > > > > > > > > > > system headers; The intuition of the 
> > > > > > > > > > > > > > > > > > > wording should be that the users can't 
> > > > > > > > > > > > > > > > > > > declare modules like `std` or 
> > > > > > > > > > > > > > > > > > > `std.compat` to avoid possible 
> > > > > > > > > > > > > > > > > > > conflicting. The approach I imaged may be 
> > > > > > > > > > > > > > > > > > > add a new compilation flags (call it 
> > > > > > > > > > > > > > > > > > > `-fstd-modules`) now. And if the compiler 
> > > > > > > > > > > > > > > > > > > found a `std` module declaration without 
> > > > > > > > > > > > > > > > > > > `-fstd-modules`, emit an error.  
> > > > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > > > For now, I think we can skip the check 
> > > > > > > > > > > > > > > > > > > for `-fstd-modules` and add it back when 
> > > > > > > > > > > > > > > > > > > we starts to support std modules actually.
> > > > > > > > > > > > > > > > > > The idea is that standard modules are built 
> > > > > > > > > > > > > > > > > > from system directories... it seems a 
> > > > > > > > > > > > > > > > > > better heuristic than adding a flag for the 
> > > > > > > > > > > > > > > > > > purpose of 1 diagnostics ( maybe some other 
> > > > > > > > > > > > > > > > > > system library could in theory export std 
> > > > > > > > > > > > > > > > > > with no warning, but I'm not super worried 
> > > > > > > > > > > > > > > > > > about that being a concern in practice)
> > > > > > > > > > > > > > > > > > The idea is that standard modules are built 
> > > > > > > > > > > > > > > > > > from system directories...
> > > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > > This is not true. For example, if someday 
> > > > > > > > > > > > > > > > > libc++ supports std modules, then we need to 
> > > > > > > > > > > > > > > > > build the std modules in our working 
> > > > > > > > > > > > > > > > > directory, which is not system directories. 
> > > > > > > > > > > > > > > > > And **ideally**, we would distribute and 
> > > > > > > > > > > > > > > > > install module file in the system 
> > > > > > > > > > > > > > > > > directories. But it is irreverent with the 
> > > > > > > > > > > > > > > > > path of the source file.
> > > > > > > > > > > > > > > > > then we need to build the std modules in our 
> > > > > > > > > > > > > > > > > working directory, which is not system 
> > > > > > > > > > > > > > > > > directories.
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > `-isystem`, pragmas, and linemarkers are all 
> > > > > > > > > > > > > > > > ways around that -- I don't think we need a 
> > > > > > > > > > > > > > > > feature flag for this, unless I'm 
> > > > > > > > > > > > > > > > misunderstanding something.
> > > > > > > > > > > > > > > Although it may be a little bit nit picker, the 
> > > > > > > > > > > > > > > module unit which declares the std modules won't 
> > > > > > > > > > > > > > > be header. It is a module unit. So it requires we 
> > > > > > > > > > > > > > > implement std modules by wrapping linemarkers 
> > > > > > > > > > > > > > > around the std modules declaration, which looks a 
> > > > > > > > > > > > > > > little bit overkill.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > And another point is that maybe we need to 
> > > > > > > > > > > > > > > introduce another feature flags to implement std 
> > > > > > > > > > > > > > > modules. Although I tried to implement std 
> > > > > > > > > > > > > > > modules within the current features, I can't 
> > > > > > > > > > > > > > > prove we can implement std modules in that way in 
> > > > > > > > > > > > > > > the end of the day.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > Let me add some more words. The standards require 
> > > > > > > > > > > > > > > us to implement std modules without deprecating 
> > > > > > > > > > > > > > > the system headers. Thi

[PATCH] D137052: [clang-format] Don't skip #else/#elif of #if 0

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added subscribers: curdeius, djasper, klimek.
MyDeveloperDay added a comment.

In D137052#3899094 , @sstwcw wrote:

> It just occurred to me that klimek is the one who added all this stuff.  Why 
> didn't you include him as a reviewer?

@djasper  and @klimek name is likely all over the code, as they were the 
"founding fathers of clang-format", hopefully they are off doing something new 
and exciting, but those of us here on the reviewers list along with @curdeius 
are really the core clang-format team at this time. Hopefully we are qualified 
enough. (actually @owenpan is likely the most qualified ;-) )

We do our best to stay true to the original designers goals and be good 
custodians of clang-format, but my hope is they are watching over us even from 
afar but we assume they trust us with their baby!

We tend not to ask them to give back more unless they want to.


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

https://reviews.llvm.org/D137052

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

[PATCH] D136975: [Concepts] Correctly handle failure when checking concepts recursively

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

Thanks for the patch. It looks good to me.

About

> Note that we DO need to be careful to make sure we still check
> constraints properly that are caused by a previous constraint, but not
> derived from (such as when a check causes us to check special member
> function generation), so we cannot use the existing logic to see if this
> is being instantiated.

For the `derived from` case, I think we also end up getting the infinite 
recursion? Why do we disable the check for the `derived from` case?


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

https://reviews.llvm.org/D136975

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

[PATCH] D136975: [Concepts] Correctly handle failure when checking concepts recursively

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D136975#3899703 , @ychen wrote:

> Thanks for the patch. It looks good to me.
>
> About
>
>> Note that we DO need to be careful to make sure we still check
>> constraints properly that are caused by a previous constraint, but not
>> derived from (such as when a check causes us to check special member
>> function generation), so we cannot use the existing logic to see if this
>> is being instantiated.
>
> For the `derived from` case, I think we also end up getting the infinite 
> recursion? Why do we disable the check for the `derived from` case?

Can you clarify what you mean?  I'm not sure which test case you're speaking of.


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

https://reviews.llvm.org/D136975

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

[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Frederic Riss via Phabricator via cfe-commits]

friss updated this revision to Diff 472355.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/HeaderSearchOptions.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CMakeLists.txt
  clang/test/Driver/vfsstatcache.c
  clang/test/clang-stat-cache/cache-effects.c
  clang/test/clang-stat-cache/errors.test
  clang/tools/CMakeLists.txt
  clang/tools/clang-stat-cache/CMakeLists.txt
  clang/tools/clang-stat-cache/clang-stat-cache.cpp
  clang/tools/driver/features.json
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -17,6 +17,7 @@
 #include "llvm/Testing/Support/SupportHelpers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 #include 
 #include 
 
@@ -3159,3 +3160,302 @@
 "  DummyFileSystem (RecursiveContents)\n",
 Output);
 }
+
+class StatCacheFileSystemTest : public ::testing::Test {
+public:
+  void SetUp() override {}
+
+  template 
+  void createStatCacheFileSystem(
+  StringRef OutputFile, StringRef BaseDir, bool IsCaseSensitive,
+  IntrusiveRefCntPtr &Result,
+  StringCollection &Filenames,
+  IntrusiveRefCntPtr Lower = new ErrorDummyFileSystem(),
+  uint64_t ValidityToken = 0) {
+vfs::StatCacheFileSystem::StatCacheWriter Generator(
+BaseDir, IsCaseSensitive, ValidityToken);
+std::error_code ErrorCode;
+
+Result.reset();
+
+for (sys::fs::recursive_directory_iterator I(BaseDir, ErrorCode), E;
+ I != E && !ErrorCode; I.increment(ErrorCode)) {
+  Filenames.push_back(I->path());
+  StringRef Path(Filenames.back().c_str());
+  sys::fs::file_status s;
+  status(Path, s);
+  Generator.addEntry(Path, s);
+}
+
+{
+  raw_fd_ostream StatCacheFile(OutputFile, ErrorCode);
+  ASSERT_FALSE(ErrorCode);
+  Generator.writeStatCache(StatCacheFile);
+}
+
+loadCacheFile(OutputFile, ValidityToken, Lower, Result);
+  }
+
+  void loadCacheFile(StringRef OutputFile, uint64_t ExpectedValidityToken,
+ IntrusiveRefCntPtr Lower,
+ IntrusiveRefCntPtr &Result) {
+auto ErrorOrBuffer = MemoryBuffer::getFile(OutputFile);
+EXPECT_TRUE(ErrorOrBuffer);
+StringRef CacheBaseDir;
+bool IsCaseSensitive;
+bool VersionMatch;
+uint64_t FileValidityToken;
+auto E = vfs::StatCacheFileSystem::validateCacheFile(
+(*ErrorOrBuffer)->getMemBufferRef(), CacheBaseDir, IsCaseSensitive,
+FileValidityToken);
+ASSERT_FALSE(E);
+EXPECT_TRUE(VersionMatch);
+EXPECT_EQ(FileValidityToken, ExpectedValidityToken);
+auto ExpectedCache = vfs::StatCacheFileSystem::create(
+std::move(*ErrorOrBuffer), OutputFile, Lower);
+ASSERT_FALSE(ExpectedCache.takeError());
+Result = *ExpectedCache;
+  }
+
+  template 
+  void
+  compareStatCacheToRealFS(IntrusiveRefCntPtr CacheFS,
+   const StringCollection &Files) {
+IntrusiveRefCntPtr RealFS = vfs::getRealFileSystem();
+
+for (auto &File : Files) {
+  auto ErrorOrStatus1 = RealFS->status(File);
+  auto ErrorOrStatus2 = CacheFS->status(File);
+
+  EXPECT_EQ((bool)ErrorOrStatus1, (bool)ErrorOrStatus2);
+  if (!ErrorOrStatus1 || !ErrorOrStatus2)
+continue;
+
+  vfs::Status s1 = *ErrorOrStatus1, s2 = *ErrorOrStatus2;
+  EXPECT_EQ(s1.getName(), s2.getName());
+  EXPECT_EQ(s1.getType(), s2.getType());
+  EXPECT_EQ(s1.getPermissions(), s2.getPermissions());
+  EXPECT_EQ(s1.getLastModificationTime(), s2.getLastModificationTime());
+  EXPECT_EQ(s1.getUniqueID(), s2.getUniqueID());
+  EXPECT_EQ(s1.getUser(), s2.getUser());
+  EXPECT_EQ(s1.getGroup(), s2.getGroup());
+  EXPECT_EQ(s1.getSize(), s2.getSize());
+}
+  }
+};
+
+TEST_F(StatCacheFileSystemTest, Basic) {
+  TempDir TestDirectory("virtual-file-system-test", /*Unique*/ true);
+  TempDir _a(TestDirectory.path("a"));
+  TempFile _ab(TestDirectory.path("a/b"));
+  TempDir _ac(TestDirectory.path("a/c"));
+  TempFile _acd(TestDirectory.path("a/c/d"), "", "Dummy contents");
+  TempFile _ace(TestDirectory.path("a/c/e"));
+  TempFile _acf(TestDirectory.path("a/c/f"), "", "More dummy contents");
+  TempDir _ag(TestDirectory.path("a/g"));
+  TempFile _agh(TestDirectory.path("a/g/h"));
+
+  StringRef BaseDir(_a.path());
+
+  SmallVector Filenames;
+  IntrusiveRefCntPtr StatCacheFS;
+  createStatCacheFileSystem(TestDirectory.path("stat.cache"), BaseDir,
+/* IsCaseSensitive= */ true, 

[PATCH] D137179: [clang][Lex] Header search case insensitivity

[Troy Johnson via Phabricator via cfe-commits]

troyj created this revision.
troyj added a reviewer: bruno.
Herald added a project: All.
troyj requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Correct D135801  to be case insensitive.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137179

Files:
  clang/lib/Lex/HeaderSearch.cpp


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -388,7 +388,9 @@
   break;
 
 // Give earlier keys precedence over identical later keys.
-auto Callback = [&](StringRef Filename) { Index.try_emplace(Filename, i); 
};
+auto Callback = [&](StringRef Filename) {
+  Index.try_emplace(Filename.lower(), i);
+};
 Dir.getHeaderMap()->forEachKey(Callback);
   }
 
@@ -1021,7 +1023,7 @@
 // Handle cold misses of user includes in the presence of many header
 // maps.  We avoid searching perhaps thousands of header maps by
 // jumping directly to the correct one or jumping beyond all of them.
-auto Iter = SearchDirHeaderMapIndex.find(Filename);
+auto Iter = SearchDirHeaderMapIndex.find(Filename.lower());
 if (Iter == SearchDirHeaderMapIndex.end())
   // Not in index => Skip to first SearchDir after initial header maps
   It = search_dir_nth(FirstNonHeaderMapSearchDirIdx);


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -388,7 +388,9 @@
   break;
 
 // Give earlier keys precedence over identical later keys.
-auto Callback = [&](StringRef Filename) { Index.try_emplace(Filename, i); };
+auto Callback = [&](StringRef Filename) {
+  Index.try_emplace(Filename.lower(), i);
+};
 Dir.getHeaderMap()->forEachKey(Callback);
   }
 
@@ -1021,7 +1023,7 @@
 // Handle cold misses of user includes in the presence of many header
 // maps.  We avoid searching perhaps thousands of header maps by
 // jumping directly to the correct one or jumping beyond all of them.
-auto Iter = SearchDirHeaderMapIndex.find(Filename);
+auto Iter = SearchDirHeaderMapIndex.find(Filename.lower());
 if (Iter == SearchDirHeaderMapIndex.end())
   // Not in index => Skip to first SearchDir after initial header maps
   It = search_dir_nth(FirstNonHeaderMapSearchDirIdx);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits