date:20230503

[PATCH] D149816: [clang][Interp] Implement __builtin_strcmp

2023-05-03 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

  Make our Function class keep a list of parameter offsets so we can
  simply get a parameter by index when evaluating builtin functions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149816

Files:
  clang/lib/AST/Interp/ByteCodeEmitter.cpp
  clang/lib/AST/Interp/Function.cpp
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/lib/AST/Interp/Pointer.h
  clang/test/AST/Interp/builtin-functions.cpp

Index: clang/test/AST/Interp/builtin-functions.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/builtin-functions.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter %s -verify
+// RUN: %clang_cc1 -verify=ref %s -Wno-constant-evaluated
+
+namespace strcmp {
+  constexpr char kFoobar[6] = {'f','o','o','b','a','r'};
+  constexpr char kFoobazfoobar[12] = {'f','o','o','b','a','z','f','o','o','b','a','r'};
+
+  static_assert(__builtin_strcmp("abab", "abab") == 0);
+  static_assert(__builtin_strcmp("abab", "abba") == -1);
+  static_assert(__builtin_strcmp("abab", "abaa") == 1);
+  static_assert(__builtin_strcmp("ababa", "abab") == 1);
+  static_assert(__builtin_strcmp("abab", "ababa") == -1);
+  static_assert(__builtin_strcmp("a\203", "a") == 1);
+  static_assert(__builtin_strcmp("a\203", "a\003") == 1);
+  static_assert(__builtin_strcmp("abab\0banana", "abab") == 0);
+  static_assert(__builtin_strcmp("abab", "abab\0banana") == 0);
+  static_assert(__builtin_strcmp("abab\0banana", "abab\0canada") == 0);
+  static_assert(__builtin_strcmp(0, "abab") == 0); // expected-error {{not an integral constant}} \
+   // expected-note {{dereferenced null}} \
+   // expected-note {{in call to}} \
+   // ref-error {{not an integral constant}} \
+   // ref-note {{dereferenced null}}
+  static_assert(__builtin_strcmp("abab", 0) == 0); // expected-error {{not an integral constant}} \
+   // expected-note {{dereferenced null}} \
+   // expected-note {{in call to}} \
+   // ref-error {{not an integral constant}} \
+   // ref-note {{dereferenced null}}
+
+  static_assert(__builtin_strcmp(kFoobar, kFoobazfoobar) == -1);
+  static_assert(__builtin_strcmp(kFoobar, kFoobazfoobar + 6) == 0); // expected-error {{not an integral constant}} \
+// expected-note {{dereferenced one-past-the-end}} \
+// expected-note {{in call to}} \
+// ref-error {{not an integral constant}} \
+// ref-note {{dereferenced one-past-the-end}}
+}
Index: clang/lib/AST/Interp/Pointer.h
===
--- clang/lib/AST/Interp/Pointer.h
+++ clang/lib/AST/Interp/Pointer.h
@@ -341,7 +341,8 @@
 
   /// Dereferences a primitive element.
   template  T (unsigned I) const {
-return reinterpret_cast(Pointee->rawData())[I];
+assert(I < getNumElems());
+return reinterpret_cast(Pointee->data() + sizeof(InitMap *))[I];
   }
 
   /// Initializes a field.
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -14,15 +14,64 @@
 namespace clang {
 namespace interp {
 
-bool InterpretBuiltin(InterpState , CodePtr , unsigned BuiltinID) {
+template  T getParam(InterpFrame *Frame, unsigned Index) {
+  unsigned Offset = Frame->getFunction()->getParamOffset(Index);
+  return Frame->getParam(Offset);
+}
+
+static bool interp__builtin_strcmp(InterpState , CodePtr OpPC,
+   InterpFrame *Frame) {
+  const Pointer  = getParam(Frame, 0);
+  const Pointer  = getParam(Frame, 1);
+
+  if (!CheckLive(S, OpPC, A, AK_Read) || !CheckLive(S, OpPC, B, AK_Read))
+return false;
+
+  assert(A.getFieldDesc()->isPrimitiveArray());
+  assert(B.getFieldDesc()->isPrimitiveArray());
+
+  unsigned IndexA = A.getIndex();
+  unsigned IndexB = B.getIndex();
+  int32_t Result = 0;
+  for (;; ++IndexA, ++IndexB) {
+const Pointer  = A.atIndex(IndexA);
+const Pointer  = B.atIndex(IndexB);
+if (!CheckRange(S,

[PATCH] D149514: Check if First argument in _builtin_assume_aligned_ is of pointer type

2023-05-03 Thread Timm Bäder via Phabricator via cfe-commits

tbaeder added a comment.

This patch was submitted by a beginner, because 
https://github.com/llvm/llvm-project/issues/62305 has the "good first issue" 
label. From the last few comments, I'm not sure they know how to proceed. Could 
you summarize what the next steps are?


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

https://reviews.llvm.org/D149514

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

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

2023-05-03 Thread Owen Pan via Phabricator via cfe-commits

owenpan added a comment.

Please mark review comments as done after addressing them.




Comment at: clang/unittests/Format/QualifierFixerTest.cpp:1356
+   "template  Foo const f();",
+   Style, std::vector(1, tooling::Range(0, 36)));
+





Comment at: clang/unittests/Format/QualifierFixerTest.cpp:1366
+   "template  Foo const f();",
+   Style, std::vector(1, tooling::Range(37, 36)));
+}




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

https://reviews.llvm.org/D149643

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

[PATCH] D148490: [AIX] use system assembler for assembly files

2023-05-03 Thread ChenZheng via Phabricator via cfe-commits

shchenz added a comment.

gentle ping...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148490

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

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

2023-05-03 Thread M. Zeeshan Siddiqui via Phabricator via cfe-commits

codemzs added a comment.

Thank you @erichkeane for your insightful review. I have addressed the feedback 
from your previous review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149573

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

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

2023-05-03 Thread Owen Pan via Phabricator via cfe-commits

owenpan added inline comments.



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

tahonermann wrote:
> HazardyKnusperkeks wrote:
> > HazardyKnusperkeks wrote:
> > > HazardyKnusperkeks wrote:
> > > > MyDeveloperDay wrote:
> > > > > owenpan wrote:
> > > > > > tahonermann wrote:
> > > > > > > How about using an init capture instead? This will suffice to 
> > > > > > > avoid one of the copies but means that `InsertBraces` doesn't get 
> > > > > > > set until the lambda is invoked. I wouldn't expect that to matter 
> > > > > > > though.
> > > > > > I'm not sure if it's worth the trouble, but if I really had to 
> > > > > > bother, I would do something like the above.
> > > > > Apart from perhaps the unnecessary copying from Expanded -> S.. 
> > > > > doesn't this bascially do the same without us having to remember to 
> > > > > put Expanded back afterwards? I don't see how using Expanded is any 
> > > > > different from using S (other than the copy)
> > > > > 
> > > > > ```
> > > > > if (Style.InsertBraces) {
> > > > >   FormatStyle S = Expanded;
> > > > >   S.InsertBraces = true;
> > > > >   Passes.emplace_back([&](const Environment ) {
> > > > > return BracesInserter(Env, 
> > > > > S).process(/*SkipAnnotation=*/true);
> > > > >   });
> > > > > }
> > > > > ```
> > > > > I'm not sure if it's worth the trouble, but if I really had to 
> > > > > bother, I would do something like the above.
> > > > 
> > > > That wouldn't work, since the pass is only executed afterwards.
> > > > Apart from perhaps the unnecessary copying from Expanded -> S.. doesn't 
> > > > this bascially do the same without us having to remember to put 
> > > > Expanded back afterwards? I don't see how using Expanded is any 
> > > > different from using S (other than the copy)
> > > > 
> > > > ```
> > > > if (Style.InsertBraces) {
> > > >   FormatStyle S = Expanded;
> > > >   S.InsertBraces = true;
> > > >   Passes.emplace_back([&](const Environment ) {
> > > > return BracesInserter(Env, S).process(/*SkipAnnotation=*/true);
> > > >   });
> > > > }
> > > > ```
> > > 
> > > That would have a dangling reference to S and most likely don't work as 
> > > wished.
> > This should work. One could replace the two assignments with a RAII wrapper 
> > which restores the old value.
> Here is another option; it is just the original code but with an init capture 
> that does a move instead of a copy. Coverity shouldn't complain about a move 
> capture (if it does, I'll file a support case with Synopsys).
> > I'm not sure if it's worth the trouble, but if I really had to bother, I 
> > would do something like the above.
> 
> That wouldn't work, since the pass is only executed afterwards.

You're right!



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

owenpan wrote:
> tahonermann wrote:
> > HazardyKnusperkeks wrote:
> > > HazardyKnusperkeks wrote:
> > > > HazardyKnusperkeks wrote:
> > > > > MyDeveloperDay wrote:
> > > > > > owenpan wrote:
> > > > > > > tahonermann wrote:
> > > > > > > > How about using an init capture instead? This will suffice to 
> > > > > > > > avoid one of the copies but means that `InsertBraces` doesn't 
> > > > > > > > get set until the lambda is invoked. I wouldn't expect that to 
> > > > > > > > matter though.
> > > > > > > I'm not sure if it's worth the trouble, but if I really had to 
> > > > > > > bother, I would do something like the above.
> > > > > > Apart from perhaps the unnecessary copying from Expanded -> S.. 
> > > > > > doesn't this bascially do the same without us having to remember to 
> > > > > > put Expanded back afterwards? I don't see how using Expanded is any 
> > > > > > different from using S (other than the copy)
> > > > > > 
> > > > > > ```
> > > > > > if (Style.InsertBraces) {
> > > > > >   FormatStyle S = Expanded;
> > > > > >   S.InsertBraces = true;
> > > > > >   Passes.emplace_back([&](const Environment ) {
> > > > > > return BracesInserter(Env, 
> > > > > > S).process(/*SkipAnnotation=*/true);
> > > > > >   });
> > > > > > }
> > > > > > ```
> > > > > > I'm not sure if it's worth the trouble, but if I really had to 
> > > > > > bother, I would do something like the above.
> > > > > 
> > > > > That wouldn't work, since the pass is only executed afterwards.
> > > > > Apart from perhaps the unnecessary copying from Expanded -> S.. 
> > > > > doesn't this bascially do the same without us having to remember to 
> > > > > put Expanded back afterwards? I don't see how using Expanded is any 
> > > > >

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

2023-05-03 Thread NAKAMURA Takumi via Phabricator via cfe-commits

chapuni added a comment.

Please come in; 
https://discourse.llvm.org/t/rfc-let-mvt-generated-and-restore-mvt-into-llvm-codegen/69547/3


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148767

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

[PATCH] D149809: [Clang][Docs] Fix man page build

2023-05-03 Thread Tom Stellard via Phabricator via cfe-commits

tstellar added a comment.

I think we could remove some of the duplication by making the docs_target 
parameter into a list and passing both docs-clang-html and docs-clang-man to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149809

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

[PATCH] D136436: [Clang][LoongArch] Add register alias handling without `$` prefix

2023-05-03 Thread Lu Weining via Phabricator via cfe-commits

SixWeining added a comment.

In D136436#4314612 , @xen0n wrote:

> Hi, any update on this? Given the `$`-less style has been long established at 
> this time, and present in released GCC versions, it seems we indeed have to 
> follow suit (and later make GCC accept ABI names for FPRs too). It will also 
> make the ClangBuiltLinux work easier.

I tend to adopt @xry111's suggestion that "emulating" GCC's behavior in Clang 
(not in LLVM):

> GCC supports register variable definition like register u64 a0 asm("a0"); but 
> does not support addi.d a0, a0, a1 (because in GCC such a line in inline 
> assembly is passed to GNU as directly).
> Is it possible to "emulate" the behavior in Clang?

And for `FPRs`, I suggest to keep current implementation until GCC support 
non-prefixed names.

This means:

- Non-prefixed `GPRs` names ( and ABI names ) are allowed in register variable 
definition and clobber list.
- Only `$` prefixed register names is allowed in assembler template.
- Do not support non-prefixed names for other registers ( including `FPRs` ) 
anywhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136436

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

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

2023-05-03 Thread Soumi Manna via Phabricator via cfe-commits

Manna updated this revision to Diff 519337.
Manna retitled this revision from "[NFC][Clang]Fix static analyzer tool remarks 
about large copies by value" to "[NFC][Clang]Fix static analyzer tool remarks 
about large copies by values".
Manna edited the summary of this revision.
Manna added a comment.

Thank you for the reviews and feedbacks. I have applied suggestion about init 
capture that does a move instead of a copy.


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

https://reviews.llvm.org/D149647

Files:
  clang/lib/Format/Format.cpp


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -3485,7 +3485,7 @@
 if (Style.InsertBraces) {
   FormatStyle S = Expanded;
   S.InsertBraces = true;
-  Passes.emplace_back([&, S](const Environment ) {
+  Passes.emplace_back([&, S = std::move(S)](const Environment ) {
 return BracesInserter(Env, S).process(/*SkipAnnotation=*/true);
   });
 }
@@ -3493,7 +3493,7 @@
 if (Style.RemoveBracesLLVM) {
   FormatStyle S = Expanded;
   S.RemoveBracesLLVM = true;
-  Passes.emplace_back([&, S](const Environment ) {
+  Passes.emplace_back([&, S = std::move(S)](const Environment ) {
 return BracesRemover(Env, S).process(/*SkipAnnotation=*/true);
   });
 }
@@ -3501,7 +3501,7 @@
 if (Style.RemoveSemicolon) {
   FormatStyle S = Expanded;
   S.RemoveSemicolon = true;
-  Passes.emplace_back([&, S](const Environment ) {
+  Passes.emplace_back([&, S = std::move(S)](const Environment ) {
 return SemiRemover(Env, S).process(/*SkipAnnotation=*/true);
   });
 }


Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -3485,7 +3485,7 @@
 if (Style.InsertBraces) {
   FormatStyle S = Expanded;
   S.InsertBraces = true;
-  Passes.emplace_back([&, S](const Environment ) {
+  Passes.emplace_back([&, S = std::move(S)](const Environment ) {
 return BracesInserter(Env, S).process(/*SkipAnnotation=*/true);
   });
 }
@@ -3493,7 +3493,7 @@
 if (Style.RemoveBracesLLVM) {
   FormatStyle S = Expanded;
   S.RemoveBracesLLVM = true;
-  Passes.emplace_back([&, S](const Environment ) {
+  Passes.emplace_back([&, S = std::move(S)](const Environment ) {
 return BracesRemover(Env, S).process(/*SkipAnnotation=*/true);
   });
 }
@@ -3501,7 +3501,7 @@
 if (Style.RemoveSemicolon) {
   FormatStyle S = Expanded;
   S.RemoveSemicolon = true;
-  Passes.emplace_back([&, S](const Environment ) {
+  Passes.emplace_back([&, S = std::move(S)](const Environment ) {
 return SemiRemover(Env, S).process(/*SkipAnnotation=*/true);
   });
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149809: [Clang][Docs] Fix man page build

2023-05-03 Thread Aiden Grossman via Phabricator via cfe-commits

aidengrossman added reviewers: aaron.ballman, tstellar, rnk.
aidengrossman added a comment.

Posting this as when I was setting up a new dev environment with 
`-DLLVM_ENABLE_SPHINX`, it defaults to having `SPHINX_OUTPUT_HTML` and 
`SPHINX_OUTPUT_MAN` on, and the man build was broken so when I tried to build 
the default target, it failed. This patch fixes the clang man page build.

Not sure this is the ideal approach. I'd really like to remove the redundancy 
between the generated targets reference in `gen_docs_depends` and their values 
as specified in the calls to `gen_rst_files_from_td`, but CMake doesn't really 
seem to have appropriate data structures (a list of tuples or something 
similar) to accomplish this. Very open to suggestions here though since I'm not 
that familiar with CMake.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149809

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

[PATCH] D149809: [Clang][Docs] Fix man page build

2023-05-03 Thread Aiden Grossman via Phabricator via cfe-commits

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

This patch fixes the man page build. It currently doesn't work as
SOURCE_DIR isn't set correctly (just undefined) within the
add_sphinx_target function. This patch also moves around the creation of
targets for autogenerated rst files so that both the man page and html
build can depend upon them as before only the html build depended on
them.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149809

Files:
  clang/docs/CMakeLists.txt


Index: clang/docs/CMakeLists.txt
===
--- clang/docs/CMakeLists.txt
+++ clang/docs/CMakeLists.txt
@@ -90,50 +90,59 @@
 endif()
 endif()
 
-function (gen_rst_file_from_td output_file td_option source docs_target)
+function (gen_rst_file_from_td output_file td_option source)
   if (NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${source}")
 message(FATAL_ERROR "Cannot find source file: ${source} in 
${CMAKE_CURRENT_SOURCE_DIR}")
   endif()
   get_filename_component(TABLEGEN_INCLUDE_DIR 
"${CMAKE_CURRENT_SOURCE_DIR}/${source}" DIRECTORY)
   list(APPEND LLVM_TABLEGEN_FLAGS "-I${TABLEGEN_INCLUDE_DIR}")
   clang_tablegen(${output_file} ${td_option} SOURCE ${source} TARGET 
"gen-${output_file}")
-  add_dependencies(${docs_target} "gen-${output_file}")
+endfunction()
+
+function (gen_docs_depends docs_target)
+  add_dependencies(${docs_target} copy-clang-rst-docs)
+
+  add_dependencies(${docs_target} "gen-AttributeReference.rst")
+  add_dependencies(${docs_target} "gen-DiagnosticsReference.rst")
+  add_dependencies(${docs_target} "gen-ClangCommandLineReference.rst")
 endfunction()
 
 if (LLVM_ENABLE_SPHINX)
   include(AddSphinxTarget)
-  if (SPHINX_FOUND)
+  if (SPHINX_FOUND AND (${SPHINX_OUTPUT_HTML} OR ${SPHINX_OUTPUT_MAN}))
+# Copy rst files to build directory before generating the html
+# documentation.  Some of the rst files are generated, so they
+# only exist in the build directory.  Sphinx needs all files in
+# the same directory in order to generate the html, so we need to
+# copy all the non-gnerated rst files from the source to the build
+# directory before we run sphinx.
+add_custom_target(copy-clang-rst-docs
+  COMMAND "${CMAKE_COMMAND}" -E copy_directory
+  "${CMAKE_CURRENT_SOURCE_DIR}" "${CMAKE_CURRENT_BINARY_DIR}"
+
+  COMMAND "${CMAKE_COMMAND}" -E copy_if_different
+  "${CMAKE_CURRENT_SOURCE_DIR}/../CodeOwners.rst"
+  "${CMAKE_CURRENT_BINARY_DIR}"
+)
+
+# Generated files
+gen_rst_file_from_td(AttributeReference.rst -gen-attr-docs 
../include/clang/Basic/Attr.td)
+gen_rst_file_from_td(DiagnosticsReference.rst -gen-diag-docs 
../include/clang/Basic/Diagnostic.td)
+gen_rst_file_from_td(ClangCommandLineReference.rst -gen-opt-docs 
../include/clang/Driver/ClangOptionDocs.td)
+
 if (${SPHINX_OUTPUT_HTML})
   add_sphinx_target(html clang SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}")
 
-  # Copy rst files to build directory before generating the html
-  # documentation.  Some of the rst files are generated, so they
-  # only exist in the build directory.  Sphinx needs all files in
-  # the same directory in order to generate the html, so we need to
-  # copy all the non-gnerated rst files from the source to the build
-  # directory before we run sphinx.
-  add_custom_target(copy-clang-rst-docs
-COMMAND "${CMAKE_COMMAND}" -E copy_directory
-"${CMAKE_CURRENT_SOURCE_DIR}" "${CMAKE_CURRENT_BINARY_DIR}"
-
-COMMAND "${CMAKE_COMMAND}" -E copy_if_different
-"${CMAKE_CURRENT_SOURCE_DIR}/../CodeOwners.rst"
-"${CMAKE_CURRENT_BINARY_DIR}"
-  )
-  add_dependencies(docs-clang-html copy-clang-rst-docs)
-
   add_custom_command(TARGET docs-clang-html POST_BUILD
 COMMAND "${CMAKE_COMMAND}" -E copy
 "${CMAKE_CURRENT_SOURCE_DIR}/LibASTMatchersReference.html"
 "${CMAKE_CURRENT_BINARY_DIR}/html/LibASTMatchersReference.html")
 
-  # Generated files
-  gen_rst_file_from_td(AttributeReference.rst -gen-attr-docs 
../include/clang/Basic/Attr.td docs-clang-html)
-  gen_rst_file_from_td(DiagnosticsReference.rst -gen-diag-docs 
../include/clang/Basic/Diagnostic.td docs-clang-html)
-  gen_rst_file_from_td(ClangCommandLineReference.rst -gen-opt-docs 
../include/clang/Driver/ClangOptionDocs.td docs-clang-html)
+  gen_docs_depends(docs-clang-html)
 endif()
 if (${SPHINX_OUTPUT_MAN})
-  add_sphinx_target(man clang)
+  add_sphinx_target(man clang SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}")
+  gen_docs_depends(docs-clang-man)
 endif()
   endif()
 endif()


Index: clang/docs/CMakeLists.txt
===
--- clang/docs/CMakeLists.txt
+++ clang/docs/CMakeLists.txt
@@ -90,50 +90,59 @@

[PATCH] D149718: [NFC][Clang] Fix Coverity issues of copy without assign

2023-05-03 Thread Soumi Manna via Phabricator via cfe-commits

Manna updated this revision to Diff 519331.
Manna added a comment.

Fix clang-format errors and typo in comments


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

https://reviews.llvm.org/D149718

Files:
  clang/include/clang/Analysis/BodyFarm.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Analysis/UnsafeBufferUsage.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp


Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -42,6 +42,7 @@
   Code(serialization::STMT_NULL_PTR), AbbrevToUse(0) {}
 
 ASTStmtWriter(const ASTStmtWriter&) = delete;
+ASTStmtWriter =(const ASTStmtWriter &) = delete;
 
 uint64_t Emit() {
   assert(Code != serialization::STMT_NULL_PTR &&
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -762,6 +762,7 @@
 public:
   Strategy() = default;
   Strategy(const Strategy &) = delete; // Let's avoid copies.
+  Strategy =(const Strategy &) = delete;
   Strategy(Strategy &&) = default;
 
   void set(const VarDecl *VD, Kind K) { Map[VD] = K; }
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1783,6 +1783,7 @@
 SemaDiagnosticBuilder(Kind K, SourceLocation Loc, unsigned DiagID,
   const FunctionDecl *Fn, Sema );
 SemaDiagnosticBuilder(SemaDiagnosticBuilder &);
+SemaDiagnosticBuilder =(SemaDiagnosticBuilder &);
 SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default;
 ~SemaDiagnosticBuilder();
 
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -696,6 +696,7 @@
   AttributePool(AttributeFactory ) : Factory(factory) {}
 
   AttributePool(const AttributePool &) = delete;
+  AttributePool =(const AttributePool &) = delete;
 
   ~AttributePool() { Factory.reclaimPool(*this); }
 
@@ -912,6 +913,7 @@
 public:
   ParsedAttributes(AttributeFactory ) : pool(factory) {}
   ParsedAttributes(const ParsedAttributes &) = delete;
+  ParsedAttributes =(const ParsedAttributes &) = delete;
 
   AttributePool () const { return pool; }
 
Index: clang/include/clang/Analysis/BodyFarm.h
===
--- clang/include/clang/Analysis/BodyFarm.h
+++ clang/include/clang/Analysis/BodyFarm.h
@@ -40,6 +40,9 @@
   /// Remove copy constructor to avoid accidental copying.
   BodyFarm(const BodyFarm ) = delete;
 
+  /// Delete copy assignment operator.
+  BodyFarm =(const BodyFarm ) = delete;
+
 private:
   typedef llvm::DenseMap> BodyMap;
 


Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -42,6 +42,7 @@
   Code(serialization::STMT_NULL_PTR), AbbrevToUse(0) {}
 
 ASTStmtWriter(const ASTStmtWriter&) = delete;
+ASTStmtWriter =(const ASTStmtWriter &) = delete;
 
 uint64_t Emit() {
   assert(Code != serialization::STMT_NULL_PTR &&
Index: clang/lib/Analysis/UnsafeBufferUsage.cpp
===
--- clang/lib/Analysis/UnsafeBufferUsage.cpp
+++ clang/lib/Analysis/UnsafeBufferUsage.cpp
@@ -762,6 +762,7 @@
 public:
   Strategy() = default;
   Strategy(const Strategy &) = delete; // Let's avoid copies.
+  Strategy =(const Strategy &) = delete;
   Strategy(Strategy &&) = default;
 
   void set(const VarDecl *VD, Kind K) { Map[VD] = K; }
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1783,6 +1783,7 @@
 SemaDiagnosticBuilder(Kind K, SourceLocation Loc, unsigned DiagID,
   const FunctionDecl *Fn, Sema );
 SemaDiagnosticBuilder(SemaDiagnosticBuilder &);
+SemaDiagnosticBuilder =(SemaDiagnosticBuilder &);
 SemaDiagnosticBuilder(const SemaDiagnosticBuilder &) = default;
 ~SemaDiagnosticBuilder();
 
Index: clang/include/clang/Sema/ParsedAttr.h
===
--- clang/include/clang/Sema/ParsedAttr.h
+++ clang/include/clang/Sema/ParsedAttr.h
@@ -696,6 +696,7 @@
   AttributePool(AttributeFactory ) : Factory(factory) {}
 
   AttributePool(const AttributePool &) = delete;
+  AttributePool =(const AttributePool &) = delete;
 
   ~AttributePool() { Factory.reclaimPool(*this); }
 
@@ -912,6 +913,7 @@
 public:

[PATCH] D149800: [WIP][PGO] Add ability to mark cold functions as optsize/minsize/optnone

2023-05-03 Thread Teresa Johnson via Phabricator via cfe-commits

tejohnson added a subscriber: yamauchi.
tejohnson added a comment.

Previously @yamauchi did a bunch of work related to this called PGSO (Profile 
Guided Size Optimization). See the functions in 
https://github.com/llvm-mirror/llvm/blob/master/lib/Transforms/Utils/SizeOpts.cpp,
 which are used to guide a number of optimizations that can affect code size. 
Do you know why those weren't sufficient?

@davidxl may remember the details, but iirc fully using minsize/optnone for 
cold funcs might have been too big of a hammer performance-wise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149800

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

[PATCH] D148092: [clang][CodeGen] Break up TargetInfo.cpp [4/6]

2023-05-03 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

Is there some alternative way we can write this?  Even if each of the overrides 
is only technically used in one place, it's a repeating pattern, and writing 
the casts inline makes it really hard to read.  (Maybe the helpers can be 
somewhere else?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148092

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

[PATCH] D148093: [clang][CodeGen] Break up TargetInfo.cpp [5/6]

2023-05-03 Thread Eli Friedman via Phabricator via cfe-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148093

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

[PATCH] D146269: MIPS: allow o32 abi with 64bit CPU and 64 abi with 32bit triple

2023-05-03 Thread YunQiang Su via Phabricator via cfe-commits

wzssyqa added a comment.

@MaskRay ping?


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

https://reviews.llvm.org/D146269

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

[PATCH] D146386: [MS ABI] Fix mangling references to declarations.

2023-05-03 Thread Eli Friedman 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 rGcd93532dfc45: [MS ABI] Fix C++ mangling references to 
declarations. (authored by bolshakov-a, committed by efriedma).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146386

Files:
  clang/lib/AST/MicrosoftMangle.cpp
  clang/test/CodeGenCXX/mangle-class-nttp.cpp
  clang/test/CodeGenCXX/mangle-ms-templates.cpp

Index: clang/test/CodeGenCXX/mangle-ms-templates.cpp
===
--- clang/test/CodeGenCXX/mangle-ms-templates.cpp
+++ clang/test/CodeGenCXX/mangle-ms-templates.cpp
@@ -272,7 +272,7 @@
 };
 extern const record inst;
 void recref(type1) {}
-// CHECK: "?recref@@YAXU?$type1@$E?inst@@3Urecord@@B@@@Z"
+// CHECK: "?recref@@YAXU?$type1@$1?inst@@3Urecord@@B@@@Z"
 
 struct _GUID {};
 struct __declspec(uuid("{12345678-1234-1234-1234-1234567890aB}")) uuid;
@@ -286,7 +286,7 @@
 void fun(UUIDType1 a) {}
 // CHECK: "?fun@@YAXU?$UUIDType1@Uuuid@@$1?_GUID_12345678_1234_1234_1234_1234567890ab@@3U__s_GUID@@B@@@Z"
 void fun(UUIDType2 b) {}
-// CHECK: "?fun@@YAXU?$UUIDType2@Uuuid@@$E?_GUID_12345678_1234_1234_1234_1234567890ab@@3U__s_GUID@@B@@@Z"
+// CHECK: "?fun@@YAXU?$UUIDType2@Uuuid@@$1?_GUID_12345678_1234_1234_1234_1234567890ab@@3U__s_GUID@@B@@@Z"
 
 template  struct TypeWithFriendDefinition {
   friend void FunctionDefinedWithInjectedName(TypeWithFriendDefinition) {}
Index: clang/test/CodeGenCXX/mangle-class-nttp.cpp
===
--- clang/test/CodeGenCXX/mangle-class-nttp.cpp
+++ clang/test/CodeGenCXX/mangle-class-nttp.cpp
@@ -15,7 +15,7 @@
 
 int n = 0;
 // CHECK: define weak_odr void @_Z1fIXtl1BadL_Z1nvv(
-// MSABI: define {{.*}} @"??$f@$2UB@@PEBH1?n@@3HAH0AYAXXZ"
+// MSABI: define {{.*}} @"??$f@$2UB@@PEBHE?n@@3HAH0AYAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1BLPKi0ELi1vv(
 // MSABI: define {{.*}} @"??$f@$2UB@@PEBH0A@H00@@@YAXXZ"
@@ -36,15 +36,19 @@
 
 // Pointers to subobjects.
 struct Nested { union { int k; int arr[2]; }; } nested[2];
-struct Derived : A, Nested { int z; } extern derived;
+struct Derived : A, Nested { int z; A a_field; } extern derived;
 // CHECK: define weak_odr void @_Z1fIXtl1BadsoKiL_Z7derivedE16vv
 // MSABI: define {{.*}} void @"??$f@$2UB@@PEBH56E?derived@@3UDerived@@Az@@@H0AYAXXZ"
 template void f();
-// FIXME: We don't know the MS ABI mangling for array subscripting and
-// past-the-end pointers yet.
-#ifndef _WIN32
+// CHECK: define weak_odr void @_Z1fIXtl1BadsoKiL_Z7derivedE20vv
+// MSABI: define {{.*}} void @"??$f@$2UB@@PEBH566E?derived@@3UDerived@@Aa_field@@a@@@H0AYAXXZ"
+template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1BadsoKiL_Z6nestedE_vv
+// MSABI: define {{.*}} void @"??$f@$2UB@@PEBH56CE?nested@@3PAUNested@@A0A@@k@@@H0AYAXXZ"
 template void f();
+// Mangling of pointers to nested array elements and past-the-end pointers
+// is still incorrect in MSVC.
+#ifndef _WIN32
 // CHECK: define weak_odr void @_Z1fIXtl1BadsoKiL_Z6nestedE16_0pvv
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl1BadsoKiL_Z7derivedE8pvv
@@ -59,14 +63,16 @@
 // CHECK: define weak_odr void @_Z1fIXtl2BRsoKiL_Z7derivedE16vv
 // MSABI: define {{.*}} void @"??$f@$2UBR@@AEBH6E?derived@@3UDerived@@Az@YAXXZ"
 template void f();
-// FIXME: We don't know the MS ABI mangling for array subscripting yet.
-#ifndef _WIN32
 // CHECK: define weak_odr void @_Z1fIXtl2BRsoKiL_Z6nestedE_vv
+// MSABI: define {{.*}} void @"??$f@$2UBR@@AEBH6CE?nested@@3PAUNested@@A0A@@k@YAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl2BRsoKiL_Z6nestedE12_0vv
+// MSABI: define {{.*}} void @"??$f@$2UBR@@AEBHC6CE?nested@@3PAUNested@@A00@arr@@00YAXXZ"
 template void f();
 // CHECK: define weak_odr void @_Z1fIXtl2BRsoKiL_Z7derivedE4vv
+// MSABI: define {{.*}} void @"??$f@$2UBR@@AEBH66E?derived@@3UDerived@@AA@@b@YAXXZ"
 template void f();
+#ifndef _WIN32
 // CHECK: define weak_odr void @_Z1fIXtl2BRdecvPKiplcvPcadL_Z7derivedELl16vv
 template void f();
 #endif
@@ -77,42 +83,93 @@
 // CHECK: define weak_odr void @_Z1fIXtl1CadsoKiL_Z7derivedE16vv
 // MSABI: define {{.*}} void @"??$f@$2UC@@PEBH56E?derived@@3UDerived@@Az@@YAXXZ"
 template void f();
-#ifndef _WIN32
 // CHECK: define weak_odr void @_Z1fIXtl1CadsoKiL_Z7derivedE4vv
+// MSABI: define {{.*}} void @"??$f@$2UC@@PEBH566E?derived@@3UDerived@@AA@@b@@YAXXZ"
 template void f();
-#endif
 
 // Pointers to members.
 struct D { const int Derived::*p; int k; };
 template void f() {}
 // CHECK: define weak_odr void @_Z1fIXtl1DLM7DerivedKi0ELi1vv
-// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@H0?0H00@@@YAXXZ"
+// MSABI: define {{.*}} @"??$f@$2UD@@PERDerived@@HNH00@@@YAXXZ"
 template void f();
 //

[clang] cd93532 - [MS ABI] Fix C++ mangling references to declarations.

2023-05-03 Thread Eli Friedman via cfe-commits


Author: Bolshakov
Date: 2023-05-03T18:20:16-07:00
New Revision: cd93532dfc455255cb2fa553090d14aaa52b106b

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

LOG: [MS ABI] Fix C++ mangling references to declarations.

Several issues have been discovered and (hopefully) fixed here:

- Reference NTTPs should be mangled in the same manner as pointer
ones.

-  Pointer fields of class type NTTPs should be treated in the same
manner as reference ones.

- Pointer-to-member fields of class type NTTPs should be treated
differently compared to pointer-to-member NTTPs. Tests on
pointer-to-member-function NTTP class fields added.

- Correct mangling of pointers to anonymous union members.

- A bug in mangling references to subobjects fixed.

- Mangling array subscripts and base class members in references
to subobjects.

Reference NTTP mangling was done back in 2013
in e8fdc06e0dab2e7b98339425dbe369e27e2092a3, and Microsoft might change
mangling algorithm since then. But class type NTTPs are introduced only
in C++20, and the test was written in
b637148ecb62b900872b34eedd78b923bb43c378.
It is strange if the MS ABI had been realy changed, because Microsoft
claims that they maintain ABI stability since VS 2015. I've tested both
on v142 and v143 MSVC toolsets, and they show the same behavior
on the test cases which are changed in this PR. But
pointer-to-member-function NTTP class field mangling has been actually
changed, because it was erroneous in v142, leading to name collisions.

Moreover, pointer-to-member mangling with conversions across class
hierarchy has been enabled.

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

Added: 


Modified: 
clang/lib/AST/MicrosoftMangle.cpp
clang/test/CodeGenCXX/mangle-class-nttp.cpp
clang/test/CodeGenCXX/mangle-ms-templates.cpp

Removed: 




diff  --git a/clang/lib/AST/MicrosoftMangle.cpp 
b/clang/lib/AST/MicrosoftMangle.cpp
index e0fd8abe5e3b8..d70f1a19acbbd 100644
--- a/clang/lib/AST/MicrosoftMangle.cpp
+++ b/clang/lib/AST/MicrosoftMangle.cpp
@@ -29,12 +29,14 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/CRC.h"
 #include "llvm/Support/MD5.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/xxhash.h"
+#include 
 #include 
 
 using namespace clang;
@@ -368,9 +370,13 @@ class MicrosoftCXXNameMangler {
   void mangleVariableEncoding(const VarDecl *VD);
   void mangleMemberDataPointer(const CXXRecordDecl *RD, const ValueDecl *VD,
StringRef Prefix = "$");
+  void mangleMemberDataPointerInClassNTTP(const CXXRecordDecl *,
+  const ValueDecl *);
   void mangleMemberFunctionPointer(const CXXRecordDecl *RD,
const CXXMethodDecl *MD,
StringRef Prefix = "$");
+  void mangleMemberFunctionPointerInClassNTTP(const CXXRecordDecl *RD,
+  const CXXMethodDecl *MD);
   void mangleVirtualMemPtrThunk(const CXXMethodDecl *MD,
 const MethodVFTableLocation );
   void mangleNumber(int64_t Number);
@@ -711,6 +717,28 @@ void 
MicrosoftCXXNameMangler::mangleMemberDataPointer(const CXXRecordDecl *RD,
 mangleNumber(VBTableOffset);
 }
 
+void MicrosoftCXXNameMangler::mangleMemberDataPointerInClassNTTP(
+const CXXRecordDecl *RD, const ValueDecl *VD) {
+  MSInheritanceModel IM = RD->getMSInheritanceModel();
+  //  ::= 
+  //  ::= N
+  //  ::= 8  @  @
+
+  if (IM != MSInheritanceModel::Single && IM != MSInheritanceModel::Multiple)
+return mangleMemberDataPointer(RD, VD, "");
+
+  if (!VD) {
+Out << 'N';
+return;
+  }
+
+  Out << '8';
+  mangleNestedName(VD);
+  Out << '@';
+  mangleUnqualifiedName(VD);
+  Out << '@';
+}
+
 void
 MicrosoftCXXNameMangler::mangleMemberFunctionPointer(const CXXRecordDecl *RD,
  const CXXMethodDecl *MD,
@@ -775,6 +803,34 @@ MicrosoftCXXNameMangler::mangleMemberFunctionPointer(const 
CXXRecordDecl *RD,
 mangleNumber(VBTableOffset);
 }
 
+void MicrosoftCXXNameMangler::mangleMemberFunctionPointerInClassNTTP(
+const CXXRecordDecl *RD, const CXXMethodDecl *MD) {
+  //  ::= 
+  //   ::= N
+  //   ::= E? 
+  //   ::= E?  
+
+  if (!MD) {
+if (RD->getMSInheritanceModel() != MSInheritanceModel::Single)
+  return mangleMemberFunctionPointer(RD, MD, "");
+
+Out << 'N';
+return;
+  }
+
+  Out <<

[PATCH] D134821: [flang][driver] Allow main program to be in an archive

2023-05-03 Thread Peixin Qiao via Phabricator via cfe-commits

peixin added a comment.

> @peixin , wdyt?

@awarzynski Hi Andrzej, sorry for the late reply. I am distracted by several 
internal projects and other things in my life recently (just came back from 
vacation). My boss has not decided to let me continue working on LLVM Flang 
this year, yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134821

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

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

2023-05-03 Thread Johannes Doerfert via Phabricator via cfe-commits

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149666

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

[PATCH] D149802: [clang][modules] Avoid unnecessary writes of .timestamp files

2023-05-03 Thread Jan Svoboda via Phabricator via cfe-commits

jansvoboda11 created this revision.
jansvoboda11 added reviewers: benlangmuir, Bigcheese, gribozavr.
Herald added a subscriber: ributzka.
Herald added a project: All.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Clang currently updates the mtime of .timestamp files on each load of the 
corresponding .pcm file. This is not necessary. In a given build session, Clang 
only needs to write the .timestamp file once, when we first validate the input 
files. This patch makes it so that we only touch the .timestamp file when it's 
older than the build session, alleviating some filesystem contention in 
clang-scan-deps.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149802

Files:
  clang/lib/Serialization/ASTReader.cpp


Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -4502,18 +4502,16 @@
 }
   }
 
-  if (PP.getHeaderSearchInfo()
-  .getHeaderSearchOpts()
-  .ModulesValidateOncePerBuildSession) {
+  HeaderSearchOptions  = PP.getHeaderSearchInfo().getHeaderSearchOpts();
+  if (HSOpts.ModulesValidateOncePerBuildSession) {
 // Now we are certain that the module and all modules it depends on are
-// up to date.  Create or update timestamp files for modules that are
-// located in the module cache (not for PCH files that could be anywhere
-// in the filesystem).
+// up-to-date. For implicitly-built module files, ensure the corresponding
+// timestamp files are up-to-date in this build session.
 for (unsigned I = 0, N = Loaded.size(); I != N; ++I) {
   ImportedModule  = Loaded[I];
-  if (M.Mod->Kind == MK_ImplicitModule) {
+  if (M.Mod->Kind == MK_ImplicitModule &&
+  M.Mod->InputFilesValidationTimestamp < HSOpts.BuildSessionTimestamp)
 updateModuleTimestamp(*M.Mod);
-  }
 }
   }
 


Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -4502,18 +4502,16 @@
 }
   }
 
-  if (PP.getHeaderSearchInfo()
-  .getHeaderSearchOpts()
-  .ModulesValidateOncePerBuildSession) {
+  HeaderSearchOptions  = PP.getHeaderSearchInfo().getHeaderSearchOpts();
+  if (HSOpts.ModulesValidateOncePerBuildSession) {
 // Now we are certain that the module and all modules it depends on are
-// up to date.  Create or update timestamp files for modules that are
-// located in the module cache (not for PCH files that could be anywhere
-// in the filesystem).
+// up-to-date. For implicitly-built module files, ensure the corresponding
+// timestamp files are up-to-date in this build session.
 for (unsigned I = 0, N = Loaded.size(); I != N; ++I) {
   ImportedModule  = Loaded[I];
-  if (M.Mod->Kind == MK_ImplicitModule) {
+  if (M.Mod->Kind == MK_ImplicitModule &&
+  M.Mod->InputFilesValidationTimestamp < HSOpts.BuildSessionTimestamp)
 updateModuleTimestamp(*M.Mod);
-  }
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149800: [WIP][PGO] Add ability to mark cold functions as optsize/minsize/optnone

2023-05-03 Thread Mircea Trofin via Phabricator via cfe-commits

mtrofin added a comment.

high level question, why not have it as a pass that runs after profiles (of 
whatever kind - instrumented or sample-based) are ingested. The pass would 
attribute functions as described.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149800

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

[PATCH] D149800: [WIP][PGO] Add ability to mark cold functions as optsize/minsize/optnone

2023-05-03 Thread Arthur Eubanks via Phabricator via cfe-commits

aeubanks added a comment.

looking for feedback if this makes sense at all


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149800

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

[PATCH] D149800: [WIP][PGO] Add ability to mark cold functions as optsize/minsize/optnone

2023-05-03 Thread Arthur Eubanks via Phabricator via cfe-commits

aeubanks created this revision.
Herald added subscribers: wlei, Enna1, ormris, wenlei, steven_wu, hiraditya.
Herald added a project: All.
aeubanks requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

The performance of cold functions shouldn't matter too much, so if we care 
about binary sizes, add an option to mark cold functions as optsize/minsize for 
binary size, or optnone for compile times [1]. Clang patch will be in a future 
patch

Chrome size (ThinLTO + instrumented PGO):
base:371004392
optsize: 366883296
minsize: 342128928

[1] 
https://discourse.llvm.org/t/rfc-new-feature-proposal-de-optimizing-cold-functions-using-pgo-info/56388


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149800

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Support/PGOOptions.h
  llvm/include/llvm/Transforms/Instrumentation/PGOInstrumentation.h
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/Passes/PassBuilderPipelines.cpp
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Support/PGOOptions.cpp
  llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp
  llvm/test/Transforms/PGOProfile/Inputs/cold-func-attr.proftext
  llvm/test/Transforms/PGOProfile/cold-func-attr.ll
  llvm/tools/opt/NewPMDriver.cpp

Index: llvm/tools/opt/NewPMDriver.cpp
===
--- llvm/tools/opt/NewPMDriver.cpp
+++ llvm/tools/opt/NewPMDriver.cpp
@@ -199,6 +199,18 @@
  cl::desc("Path to the profile remapping file."),
  cl::Hidden);
 
+static cl::opt PGOColdFuncAttr(
+"pgo-cold-func-attr", cl::init(PGOOptions::ColdFuncAttr::None), cl::Hidden,
+cl::desc(
+"Function attribute to apply to cold functions as determined by PGO"),
+cl::values(clEnumValN(PGOOptions::ColdFuncAttr::None, "none", "None"),
+   clEnumValN(PGOOptions::ColdFuncAttr::OptSize, "optsize",
+  "Mark cold functions with optsize."),
+   clEnumValN(PGOOptions::ColdFuncAttr::MinSize, "minsize",
+  "Mark cold functions with minsize."),
+   clEnumValN(PGOOptions::ColdFuncAttr::OptNone, "optnone",
+  "Mark cold functions with optnone.")));
+
 static cl::opt DebugInfoForProfiling(
 "debug-info-for-profiling", cl::init(false), cl::Hidden,
 cl::desc("Emit special debug info to enable PGO profile generation."));
@@ -338,21 +350,23 @@
   std::optional P;
   switch (PGOKindFlag) {
   case InstrGen:
-P = PGOOptions(ProfileFile, "", "", FS, PGOOptions::IRInstr);
+P = PGOOptions(ProfileFile, "", "", FS, PGOOptions::IRInstr,
+   PGOOptions::NoCSAction, PGOColdFuncAttr);
 break;
   case InstrUse:
-P = PGOOptions(ProfileFile, "", ProfileRemappingFile, FS,
-   PGOOptions::IRUse);
+P = PGOOptions(ProfileFile, "", ProfileRemappingFile, FS, PGOOptions::IRUse,
+   PGOOptions::NoCSAction, PGOColdFuncAttr);
 break;
   case SampleUse:
 P = PGOOptions(ProfileFile, "", ProfileRemappingFile, FS,
-   PGOOptions::SampleUse);
+   PGOOptions::SampleUse, PGOOptions::NoCSAction,
+   PGOColdFuncAttr);
 break;
   case NoPGO:
 if (DebugInfoForProfiling || PseudoProbeForProfiling)
   P = PGOOptions("", "", "", nullptr, PGOOptions::NoAction,
- PGOOptions::NoCSAction, DebugInfoForProfiling,
- PseudoProbeForProfiling);
+ PGOOptions::NoCSAction, PGOColdFuncAttr,
+ DebugInfoForProfiling, PseudoProbeForProfiling);
 else
   P = std::nullopt;
   }
@@ -372,7 +386,8 @@
 P->CSProfileGenFile = CSProfileGenFile;
   } else
 P = PGOOptions("", CSProfileGenFile, ProfileRemappingFile, FS,
-   PGOOptions::NoAction, PGOOptions::CSIRInstr);
+   PGOOptions::NoAction, PGOOptions::CSIRInstr,
+   PGOColdFuncAttr);
 } else /* CSPGOKindFlag == CSInstrUse */ {
   if (!P) {
 errs() << "CSInstrUse needs to be together with InstrUse";
Index: llvm/test/Transforms/PGOProfile/cold-func-attr.ll
===
--- /dev/null
+++ llvm/test/Transforms/PGOProfile/cold-func-attr.ll
@@ -0,0 +1,62 @@
+; RUN: llvm-profdata merge %S/Inputs/cold-func-attr.proftext -o %t.profdata
+; RUN: opt < %s -passes=pgo-instr-use -pgo-kind=pgo-instr-use-pipeline -pgo-test-profile-file=%t.profdata -S -pgo-cold-func-attr=none| FileCheck %s --check-prefixes=NONE
+; RUN: opt < %s -passes=pgo-instr-use -pgo-kind=pgo-instr-use-pipeline -pgo-test-profile-file=%t.profdata -S -pgo-cold-func-attr=optsize | FileCheck %s --check-prefixes=OPTSIZE,CHECK
+; RUN: opt < %s -passes=pgo-instr-use -pgo-kind=pgo-instr-use-pipeline -pgo-test-profile-file=%t.profdata -S -pgo-cold-func-attr=minsize |

[PATCH] D125171: [clang-format] Add a new clang-format option AlwaysBreakBeforeFunctionParameters

2023-05-03 Thread jonathan molinatto via Phabricator via cfe-commits

jrmolin updated this revision to Diff 519293.
jrmolin added a comment.

more code review updates. pulled main.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125171

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/ConfigParseTest.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25655,6 +25655,68 @@
   verifyFormat("auto x = 5s .count() == 5;");
 }
 
+TEST_F(FormatTest, BreakBeforeParameterList) {
+  FormatStyle Style = getLLVMStyle();
+  EXPECT_EQ(Style.BreakBeforeFunctionParameters, FormatStyle::FPBS_Leave);
+  Style.BinPackParameters = false;
+
+  // test Leave (basically transparent mode)
+
+  // verify that there is no break by default
+  verifyFormat("int function1();\n" // formatted
+   "int function2(int param1, int param2, int param3);\n"
+   "void function3(int param1, int param2, int param3) {}\n"
+   "int function4(int param1, int param2, int param3);\n",
+   "int function1();\n" // original
+   "int function2(\n"
+   "int param1, int param2, int param3);\n"
+   "void function3(int param1, int param2, int param3) {}\n"
+   "int function4(int param1, int param2, int param3);\n",
+   Style);
+
+  // test Always
+  // verify that there is a break when told to break
+  Style.BreakBeforeFunctionParameters = FormatStyle::FPBS_Always;
+  verifyFormat("int function1(\n"
+   "int param1,\n"
+   "int param2,\n"
+   "int param3);\n"
+   "int function2();\n"
+   "void function3(\n"
+   "int param1,\n"
+   "int param2,\n"
+   "int param3) {}\n"
+   "int function4(\n"
+   "int param1,\n"
+   "int param2,\n"
+   "int param3);\n"
+   "int function5(\n"
+   "int param1,\n"
+   "int param2,\n"
+   "int param3);\n",
+   Style);
+
+  // verify that having no parameters doesn't affect the parentheses
+  verifyFormat("void function1() {}\n", // the formatted part
+   "void function1() {}\n", // the original
+   Style);
+
+  verifyFormat("void function1();\n", // the formatted part
+   "void function1();\n", // the original
+   Style);
+
+  // test Never
+  Style.BreakBeforeFunctionParameters = FormatStyle::FPBS_Never;
+  verifyFormat("int function1();\n" // the formatted part
+   "int function2(int param1, int param2, int param3);\n",
+   "int function1();\n" // the original
+   "int function2(\n"
+   "int param1,\n"
+   "int param2,\n"
+   "int param3);\n",
+   Style);
+}
+
 TEST_F(FormatTest, InterfaceAsClassMemberName) {
   verifyFormat("class Foo {\n"
"  int interface;\n"
Index: clang/unittests/Format/ConfigParseTest.cpp
===
--- clang/unittests/Format/ConfigParseTest.cpp
+++ clang/unittests/Format/ConfigParseTest.cpp
@@ -609,6 +609,13 @@
   CHECK_PARSE("BreakBeforeBraces: Custom", BreakBeforeBraces,
   FormatStyle::BS_Custom);
 
+  CHECK_PARSE("BreakBeforeFunctionParameters: Always",
+  BreakBeforeFunctionParameters, FormatStyle::FPBS_Always);
+  CHECK_PARSE("BreakBeforeFunctionParameters: Leave",
+  BreakBeforeFunctionParameters, FormatStyle::FPBS_Leave);
+  CHECK_PARSE("BreakBeforeFunctionParameters: Never",
+  BreakBeforeFunctionParameters, FormatStyle::FPBS_Never);
+
   Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Never;
   CHECK_PARSE("BraceWrapping:\n"
   "  AfterControlStatement: MultiLine",
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4874,6 +4874,21 @@
 return true;
   }
 
+  // If BreakBeforeFunctionParameters is Always, we want to break before
+  // the next parameter, if there is one. If it is Leave and a newline exists,
+  // make sure we insert one. Otherwise, no newline.
+  if (Left.is(tok::l_paren) && !Right.is(tok::r_paren) && Left.Previous &&
+  Left.Previous->is(TT_FunctionDeclarationName)) {
+switch (Style.BreakBeforeFunctionParameters) {
+case FormatStyle::FPBS_Always:
+  return true;
+case FormatStyle::FPBS_Never:
+

[PATCH] D149796: [clang][Sema][NFC] Move `EnterExpressionEvaluationContext` to its own file

2023-05-03 Thread David Stone via Phabricator via cfe-commits

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

`Sema.h` is huge. This makes a small reduction to it by moving 
`EnterExpressionEvaluationContext` into a new header, since it is an 
independent component.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149796

Files:
  clang/include/clang/Sema/EnterExpressionEvaluationContext.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseInit.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h

Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -19,8 +19,8 @@
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
-#include "clang/AST/ExprConcepts.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/ExprConcepts.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExprOpenMP.h"
 #include "clang/AST/OpenMPClause.h"
@@ -31,6 +31,7 @@
 #include "clang/Basic/DiagnosticParse.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Sema/Designator.h"
+#include "clang/Sema/EnterExpressionEvaluationContext.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Ownership.h"
 #include "clang/Sema/ParsedTemplate.h"
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -22,6 +22,7 @@
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Sema/EnterExpressionEvaluationContext.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/ScopeInfo.h"
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -26,6 +26,7 @@
 #include "clang/Basic/Stack.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Sema/DeclSpec.h"
+#include "clang/Sema/EnterExpressionEvaluationContext.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Sema.h"
Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -10,7 +10,6 @@
 //
 //===--===//
 
-#include "clang/Sema/TemplateDeduction.h"
 #include "TreeTransform.h"
 #include "TypeLocBuilder.h"
 #include "clang/AST/ASTContext.h"
@@ -37,9 +36,11 @@
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
+#include "clang/Sema/EnterExpressionEvaluationContext.h"
 #include "clang/Sema/Ownership.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/Template.h"
+#include "clang/Sema/TemplateDeduction.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/ArrayRef.h"
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -26,6 +26,7 @@
 #include "clang/Basic/Stack.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Sema/DeclSpec.h"
+#include "clang/Sema/EnterExpressionEvaluationContext.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Overload.h"
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -26,6 +26,7 @@
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
+#include "clang/Sema/EnterExpressionEvaluationContext.h"
 #include "clang/Sema/Initialization.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Overload.h"
Index:

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

2023-05-03 Thread Sergei Barannikov via Phabricator via cfe-commits

barannikov88 added a comment.

In D148767#4316464 , @chapuni wrote:

> In D148767#4312692 , @barannikov88 
> wrote:
>
>> Sorry, I don't follow. They may theoretically depend on anything. Why 
>> CodeGen/CodeGenTypes, specifically?
>> If they don't require it, why add it?
>
> I added deps pessimistically, "This depends on CodeGenTypes if LowLevelType.h 
> is included".

Sorry if I was not being clear. Dependencies are harmful and should be avoided 
wherever possible. 
They should not be added "just in case". Now when I want to build just llvm-mc 
I have to build half of the whole stack.
Although D148769  improves the situation, it 
does not justify adding these dependencies.
Your RFC was about dependencies, so I think you understand my concern.
Please try to remove them and only keep the ones that are really necessary.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148767

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

[PATCH] D149791: [clang][deps] NFC: Replace a vector with an array

2023-05-03 Thread David Stone via Phabricator via cfe-commits

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

`ResourceDirectoryCache::findResourceDir` uses a `std::vector` when a 
`std::array` would do. Make this replacement and declare the variable `const`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149791

Files:
  clang/tools/clang-scan-deps/ClangScanDeps.cpp


Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -252,11 +252,9 @@
 if (CachedResourceDir != Cache.end())
   return CachedResourceDir->second;
 
-std::vector PrintResourceDirArgs{ClangBinaryName};
-if (ClangCLMode)
-  PrintResourceDirArgs.push_back("/clang:-print-resource-dir");
-else
-  PrintResourceDirArgs.push_back("-print-resource-dir");
+const std::array PrintResourceDirArgs{
+ClangBinaryName,
+ClangCLMode ? "/clang:-print-resource-dir" : "-print-resource-dir"};
 
 llvm::SmallString<64> OutputFile, ErrorFile;
 llvm::sys::fs::createTemporaryFile("print-resource-dir-output",


Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -252,11 +252,9 @@
 if (CachedResourceDir != Cache.end())
   return CachedResourceDir->second;
 
-std::vector PrintResourceDirArgs{ClangBinaryName};
-if (ClangCLMode)
-  PrintResourceDirArgs.push_back("/clang:-print-resource-dir");
-else
-  PrintResourceDirArgs.push_back("-print-resource-dir");
+const std::array PrintResourceDirArgs{
+ClangBinaryName,
+ClangCLMode ? "/clang:-print-resource-dir" : "-print-resource-dir"};
 
 llvm::SmallString<64> OutputFile, ErrorFile;
 llvm::sys::fs::createTemporaryFile("print-resource-dir-output",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

2023-05-03 Thread NagaChaitanya Vellanki via Phabricator via cfe-commits

chaitanyav updated this revision to Diff 519259.
chaitanyav added a comment.

Rebase with upstream


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Analysis/uninit-vals.c
  clang/test/Sema/integer-overflow.c
  clang/test/Sema/parentheses.c
  clang/test/Sema/parentheses.cpp
  clang/test/SemaCXX/array-bounds.cpp
  clang/test/SemaCXX/integer-overflow.cpp
  libcxx/include/__chrono/duration.h
  libcxx/include/strstream
  libcxx/test/libcxx/algorithms/nth_element_stability.pass.cpp
  libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
  libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
  libcxxabi/src/cxa_personality.cpp

Index: libcxxabi/src/cxa_personality.cpp
===
--- libcxxabi/src/cxa_personality.cpp
+++ libcxxabi/src/cxa_personality.cpp
@@ -718,9 +718,7 @@
 if (actionEntry == 0)
 {
 // Found a cleanup
-results.reason = actions & _UA_SEARCH_PHASE
- ? _URC_CONTINUE_UNWIND
- : _URC_HANDLER_FOUND;
+results.reason = ((actions & _UA_SEARCH_PHASE) ? _URC_CONTINUE_UNWIND : _URC_HANDLER_FOUND);
 return;
 }
 // Convert 1-based byte offset into
@@ -832,9 +830,8 @@
 // End of action list. If this is phase 2 and we have found
 // a cleanup (ttypeIndex=0), return _URC_HANDLER_FOUND;
 // otherwise return _URC_CONTINUE_UNWIND.
-results.reason = hasCleanup && actions & _UA_CLEANUP_PHASE
- ? _URC_HANDLER_FOUND
- : _URC_CONTINUE_UNWIND;
+results.reason =
+(hasCleanup && (actions & _UA_CLEANUP_PHASE)) ? _URC_HANDLER_FOUND : _URC_CONTINUE_UNWIND;
 return;
 }
 // Go to next action
@@ -1243,10 +1240,9 @@
 {
 const __shim_type_info* excpType =
 static_cast(new_exception_header->exceptionType);
-adjustedPtr =
-__getExceptionClass(_exception_header->unwindHeader) == kOurDependentExceptionClass ?
-((__cxa_dependent_exception*)new_exception_header)->primaryException :
-new_exception_header + 1;
+adjustedPtr = (__getExceptionClass(_exception_header->unwindHeader) == kOurDependentExceptionClass)
+  ? ((__cxa_dependent_exception*)new_exception_header)->primaryException
+  : new_exception_header + 1;
 if (!exception_spec_can_catch(ttypeIndex, classInfo, ttypeEncoding,
   excpType, adjustedPtr,
   unwind_exception, base))
Index: libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
===
--- libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
+++ libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
@@ -32,7 +32,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = kSize / 2 - i * (i % 2 ? -1 : 1);
+v[i].value = kSize / 2 - i * ((i % 2) ? -1 : 1);
   }
   std::less comp;
   std::__sort_dispatch(v.begin(), v.end(), comp);
@@ -44,7 +44,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = kSize / 2 - i * (i % 2 ? -1 : 1);
+v[i].value = kSize / 2 - i * ((i % 2) ? -1 : 1);
   }
   auto deterministic_v = deterministic();
   std::sort(v.begin(), v.end());
@@ -62,7 +62,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = kSize / 2 - i * (i % 2 ? -1 : 1);
+v[i].value = kSize / 2 - i * ((i % 2) ? -1 : 1);
   }
   auto snapshot_v = v;
   auto snapshot_custom_v = v;
Index: libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
===
--- libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
+++ libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
@@ -32,7 +32,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = (i % 2 ? 1 : kSize / 2 + i);
+v[i].value = ((i % 2) ? 1 : kSize / 2 + i);
   }
   auto comp = std::less();
   std::__partial_sort_impl(v.begin(), v.begin() + kSize / 2, v.end(), comp);
@@ -44,7 +44,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = (i % 2 ? 1 : kSize / 2 + i);
+v[i].value = ((i % 2) ? 1 : kSize / 2 + i);
   }
   auto deterministic_v = deterministic();

[PATCH] D147717: [C++20][NFC] Claim full support for consteval again

2023-05-03 Thread Christopher Di Bella via Phabricator via cfe-commits

cjdb added a comment.

In D147717#4312066 , @Fznamznon wrote:

> In D147717#4275410 , @cjdb wrote:
>
>> I'm gonna get started on this today!
>
> @cjdb , how is this going? I've seen 
> https://github.com/llvm/llvm-project/issues/62224, but no more issues with 
> consteval label.

I haven't had time to invest in this recently, as I've been gearing up for an 
intern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147717

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

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

2023-05-03 Thread Alexander Shaposhnikov via Phabricator via cfe-commits

alexander-shaposhnikov added a comment.

P.S. Landed. If it survives in the trunk this time - I'll send a follow-up diff 
with the release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

[clang] 3a54022 - [Clang][Sema] Fix comparison of constraint expressions

2023-05-03 Thread Alexander Shaposhnikov via cfe-commits


Author: Alexander Shaposhnikov
Date: 2023-05-03T21:06:12Z
New Revision: 3a540229341e3c8dc6d8ee61309eafaf943ea254

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

LOG: [Clang][Sema] Fix comparison of constraint expressions

This diff switches the approach to comparison of constraint expressions
to the new one based on template args substitution.
It continues the effort to fix our handling of out-of-line definitions
of constrained templates.
This is a recommit of e3b1083e00.

Differential revision: https://reviews.llvm.org/D146178

Added: 


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

Removed: 




diff  --git a/clang/include/clang/AST/DeclTemplate.h 
b/clang/include/clang/AST/DeclTemplate.h
index 3677335fa176f..7cd505218f2b9 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -2309,9 +2309,15 @@ class ClassTemplateDecl : public 
RedeclarableTemplateDecl {
 return static_cast(RedeclarableTemplateDecl::getCommonPtr());
   }
 
+  void setCommonPtr(Common *C) {
+RedeclarableTemplateDecl::Common = C;
+  }
+
 public:
+
   friend class ASTDeclReader;
   friend class ASTDeclWriter;
+  friend class TemplateDeclInstantiator;
 
   /// Load any lazily-loaded specializations from the external source.
   void LoadLazySpecializations() const;

diff  --git a/clang/include/clang/Sema/Template.h 
b/clang/include/clang/Sema/Template.h
index 48e8b78311e12..1de2cc6917b42 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -232,9 +232,21 @@ enum class TemplateSubstitutionKind : char {
 /// Replaces the current 'innermost' level with the provided argument list.
 /// This is useful for type deduction cases where we need to get the entire
 /// list from the AST, but then add the deduced innermost list.
-void replaceInnermostTemplateArguments(ArgList Args) {
-  assert(TemplateArgumentLists.size() > 0 && "Replacing in an empty 
list?");
-  TemplateArgumentLists[0].Args = Args;
+void replaceInnermostTemplateArguments(Decl *AssociatedDecl, ArgList Args) 
{
+  assert((!TemplateArgumentLists.empty() || NumRetainedOuterLevels) &&
+ "Replacing in an empty list?");
+
+  if (!TemplateArgumentLists.empty()) {
+assert((TemplateArgumentLists[0].AssociatedDeclAndFinal.getPointer() ||
+TemplateArgumentLists[0].AssociatedDeclAndFinal.getPointer() ==
+AssociatedDecl) &&
+   "Trying to change incorrect declaration?");
+TemplateArgumentLists[0].Args = Args;
+  } else {
+--NumRetainedOuterLevels;
+TemplateArgumentLists.push_back(
+{{AssociatedDecl, /*Final=*/false}, Args});
+  }
 }
 
 /// Add an outermost level that we are not substituting. We have no

diff  --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 328d66bf33afa..f208cdbd1d87d 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -721,7 +721,7 @@ CalculateTemplateDepthForConstraints(Sema , const 
NamedDecl *ND,
   ND, /*Final=*/false, /*Innermost=*/nullptr, /*RelativeToPrimary=*/true,
   /*Pattern=*/nullptr,
   /*ForConstraintInstantiation=*/true, SkipForSpecialization);
-  return MLTAL.getNumSubstitutedLevels();
+  return MLTAL.getNumLevels();
 }
 
 namespace {
@@ -752,27 +752,44 @@ namespace {
   };
 } // namespace
 
+static const Expr *SubstituteConstraintExpression(Sema , const NamedDecl *ND,
+  const Expr *ConstrExpr) {
+  MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs(
+  ND, /*Final=*/false, /*Innermost=*/nullptr,
+  /*RelativeToPrimary=*/true,
+  /*Pattern=*/nullptr, /*ForConstraintInstantiation=*/true,
+  /*SkipForSpecialization*/ false);
+  if (MLTAL.getNumSubstitutedLevels() == 0)
+return ConstrExpr;
+
+  Sema::SFINAETrap SFINAE(S, /*AccessCheckingSFINAE=*/false);
+  std::optional ThisScope;
+  if (auto *RD = dyn_cast(ND->getDeclContext()))
+ThisScope.emplace(S, const_cast(RD), Qualifiers());
+  ExprResult SubstConstr =
+  S.SubstConstraintExpr(const_cast(ConstrExpr), MLTAL);
+  if (SFINAE.hasErrorOccurred() || !SubstConstr.isUsable())
+return nullptr;
+  return SubstConstr.get();
+}
+
 bool Sema::AreConstraintExpressionsEqual(const

[PATCH] D149182: Remove -Wpacked false positive for non-pod types where the layout isn't directly changed

2023-05-03 Thread David Blaikie via Phabricator via cfe-commits

dblaikie marked an inline comment as done.
dblaikie added a comment.

In D149182#4312202 , @aaron.ballman 
wrote:

> Precommit CI found a valid issue (at least on Debian, the Windows failure 
> appears to be unrelated but it's really hard to tell from that output).

Hmm, right ,yeah, valid test failure - fixed.

> Also, this should have a release note, right?

Happy to add one. (done)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149182

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

[PATCH] D149182: Remove -Wpacked false positive for non-pod types where the layout isn't directly changed

2023-05-03 Thread David Blaikie via Phabricator via cfe-commits

dblaikie updated this revision to Diff 519247.
dblaikie added a comment.

Describe condition in comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149182

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/test/CodeGenCXX/warn-padded-packed.cpp
  clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp

Index: clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
===
--- clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
+++ clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
@@ -6,6 +6,8 @@
 // RUN: -fdump-record-layouts -fsyntax-only -verify -x c++ < %s | \
 // RUN:   FileCheck %s
 
+// expected-no-diagnostics
+
 struct A {
   double d;
 };
@@ -14,7 +16,7 @@
   char x[8];
 };
 
-struct [[gnu::packed]] C : B, A { // expected-warning{{packed attribute is unnecessary for 'C'}}
+struct [[gnu::packed]] C : B, A {
   char x alignas(4)[8];
 };
 
Index: clang/test/CodeGenCXX/warn-padded-packed.cpp
===
--- clang/test/CodeGenCXX/warn-padded-packed.cpp
+++ clang/test/CodeGenCXX/warn-padded-packed.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify %s -emit-llvm-only
+// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify=expected,top %s -emit-llvm-only
+// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify=expected,abi15 -fclang-abi-compat=15 %s -emit-llvm-only
 
 struct S1 {
   char c;
@@ -154,7 +155,7 @@
   char c1;
   short s1;
   char c2;
-  S28_non_pod p1; // expected-warning {{not packing field 'p1' as it is non-POD for the purposes of layout}}
+  S28_non_pod p1; // top-warning {{not packing field 'p1' as it is non-POD for the purposes of layout}}
 } __attribute__((packed));
 
 struct S29_non_pod_align_1 {
@@ -167,6 +168,16 @@
 } __attribute__((packed)); // no warning
 static_assert(alignof(S29) == 1, "");
 
+struct S30 {
+protected:
+ short s;
+} __attribute__((packed)); // no warning
+struct S30_use { // abi15-warning {{packed attribute is unnecessary for 'S30_use'}}
+  char c;
+  S30 u;
+} __attribute__((packed));
+static_assert(sizeof(S30_use) == 3, "");
+
 // The warnings are emitted when the layout of the structs is computed, so we have to use them.
 void f(S1*, S2*, S3*, S4*, S5*, S6*, S7*, S8*, S9*, S10*, S11*, S12*, S13*,
S14*, S15*, S16*, S17*, S18*, S19*, S20*, S21*, S22*, S23*, S24*, S25*,
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2198,11 +2198,19 @@
   << (InBits ? 1 : 0); // (byte|bit)
 }
 
+const auto *CXXRD = dyn_cast(RD);
+
 // Warn if we packed it unnecessarily, when the unpacked alignment is not
 // greater than the one after packing, the size in bits doesn't change and
 // the offset of each field is identical.
+// Unless the type is non-POD (for Clang ABI > 15), where the packed
+// attribute on such a type does allow the type to be packed into other
+// structures that use the packed attribute.
 if (Packed && UnpackedAlignment <= Alignment &&
-UnpackedSizeInBits == getSizeInBits() && !HasPackedField)
+UnpackedSizeInBits == getSizeInBits() && !HasPackedField &&
+(!CXXRD || CXXRD->isPOD() ||
+ Context.getLangOpts().getClangABICompat() <=
+ LangOptions::ClangABI::Ver15))
   Diag(D->getLocation(), diag::warn_unnecessary_packed)
   << Context.getTypeDeclType(RD);
   }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -106,6 +106,12 @@
 - Implemented `DR2397 `_ which allows ``auto`` specifier for pointers
   and reference to arrays.
 
+Warnings
+
+- Address a false positive in ``-Wpacked`` when applied to a non-pod type using
+  Clang ABI >= 15 (fixes `#62353`_,
+  fallout from the non-POD packing ABI fix in LLVM 15)
+
 C Language Changes
 --
 - Support for outputs from asm goto statements along indirect edges has been
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149182: Remove -Wpacked false positive for non-pod types where the layout isn't directly changed

2023-05-03 Thread David Blaikie via Phabricator via cfe-commits

dblaikie updated this revision to Diff 519245.
dblaikie added a comment.

Release note
Restrict change to Clang ABI 16 and above (& test that condition)
Fix AIX test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149182

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/test/CodeGenCXX/warn-padded-packed.cpp
  clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp


Index: clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
===
--- clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
+++ clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
@@ -6,6 +6,8 @@
 // RUN: -fdump-record-layouts -fsyntax-only -verify -x c++ < %s | \
 // RUN:   FileCheck %s
 
+// expected-no-diagnostics
+
 struct A {
   double d;
 };
@@ -14,7 +16,7 @@
   char x[8];
 };
 
-struct [[gnu::packed]] C : B, A { // expected-warning{{packed attribute is 
unnecessary for 'C'}}
+struct [[gnu::packed]] C : B, A {
   char x alignas(4)[8];
 };
 
Index: clang/test/CodeGenCXX/warn-padded-packed.cpp
===
--- clang/test/CodeGenCXX/warn-padded-packed.cpp
+++ clang/test/CodeGenCXX/warn-padded-packed.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify %s 
-emit-llvm-only
+// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked 
-verify=expected,top %s -emit-llvm-only
+// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked 
-verify=expected,abi15 -fclang-abi-compat=15 %s -emit-llvm-only
 
 struct S1 {
   char c;
@@ -154,7 +155,7 @@
   char c1;
   short s1;
   char c2;
-  S28_non_pod p1; // expected-warning {{not packing field 'p1' as it is 
non-POD for the purposes of layout}}
+  S28_non_pod p1; // top-warning {{not packing field 'p1' as it is non-POD for 
the purposes of layout}}
 } __attribute__((packed));
 
 struct S29_non_pod_align_1 {
@@ -167,6 +168,16 @@
 } __attribute__((packed)); // no warning
 static_assert(alignof(S29) == 1, "");
 
+struct S30 {
+protected:
+ short s;
+} __attribute__((packed)); // no warning
+struct S30_use { // abi15-warning {{packed attribute is unnecessary for 
'S30_use'}}
+  char c;
+  S30 u;
+} __attribute__((packed));
+static_assert(sizeof(S30_use) == 3, "");
+
 // The warnings are emitted when the layout of the structs is computed, so we 
have to use them.
 void f(S1*, S2*, S3*, S4*, S5*, S6*, S7*, S8*, S9*, S10*, S11*, S12*, S13*,
S14*, S15*, S16*, S17*, S18*, S19*, S20*, S21*, S22*, S23*, S24*, S25*,
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -2198,11 +2198,16 @@
   << (InBits ? 1 : 0); // (byte|bit)
 }
 
+const auto *CXXRD = dyn_cast(RD);
+
 // Warn if we packed it unnecessarily, when the unpacked alignment is not
 // greater than the one after packing, the size in bits doesn't change and
 // the offset of each field is identical.
 if (Packed && UnpackedAlignment <= Alignment &&
-UnpackedSizeInBits == getSizeInBits() && !HasPackedField)
+UnpackedSizeInBits == getSizeInBits() && !HasPackedField &&
+(!CXXRD || CXXRD->isPOD() ||
+ Context.getLangOpts().getClangABICompat() <=
+ LangOptions::ClangABI::Ver15))
   Diag(D->getLocation(), diag::warn_unnecessary_packed)
   << Context.getTypeDeclType(RD);
   }
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -106,6 +106,12 @@
 - Implemented `DR2397 `_ which allows ``auto`` 
specifier for pointers
   and reference to arrays.
 
+Warnings
+
+- Address a false positive in ``-Wpacked`` when applied to a non-pod type using
+  Clang ABI >= 15 (fixes 
`#62353`_,
+  fallout from the non-POD packing ABI fix in LLVM 15)
+
 C Language Changes
 --
 - Support for outputs from asm goto statements along indirect edges has been


Index: clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
===
--- clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
+++ clang/test/Layout/aix-Wpacked-expecting-diagnostics.cpp
@@ -6,6 +6,8 @@
 // RUN: -fdump-record-layouts -fsyntax-only -verify -x c++ < %s | \
 // RUN:   FileCheck %s
 
+// expected-no-diagnostics
+
 struct A {
   double d;
 };
@@ -14,7 +16,7 @@
   char x[8];
 };
 
-struct [[gnu::packed]] C : B, A { // expected-warning{{packed attribute is unnecessary for 'C'}}
+struct [[gnu::packed]] C : B, A {
   char x alignas(4)[8];
 };
 
Index: clang/test/CodeGenCXX/warn-padded-packed.cpp

[PATCH] D149777: [clang][deps] Teach dep directive scanner about #pragma clang system_header

2023-05-03 Thread Ben Langmuir 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 rG7b492d1be0ea: [clang][deps] Teach dep directive scanner 
about #pragma clang system_header (authored by benlangmuir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149777

Files:
  clang/include/clang/Lex/DependencyDirectivesScanner.h
  clang/lib/Lex/DependencyDirectivesScanner.cpp
  clang/lib/Lex/Lexer.cpp
  clang/unittests/Lex/DependencyDirectivesScannerTest.cpp


Index: clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -90,7 +90,8 @@
"#pragma pop_macro(A)\n"
"#pragma include_alias(, )\n"
"export module m;\n"
-   "import m;\n",
+   "import m;\n"
+   "#pragma clang system_header\n",
Out, Tokens, Directives));
   EXPECT_EQ(pp_define, Directives[0].Kind);
   EXPECT_EQ(pp_undef, Directives[1].Kind);
@@ -113,7 +114,8 @@
   EXPECT_EQ(pp_pragma_include_alias, Directives[18].Kind);
   EXPECT_EQ(cxx_export_module_decl, Directives[19].Kind);
   EXPECT_EQ(cxx_import_decl, Directives[20].Kind);
-  EXPECT_EQ(pp_eof, Directives[21].Kind);
+  EXPECT_EQ(pp_pragma_system_header, Directives[21].Kind);
+  EXPECT_EQ(pp_eof, Directives[22].Kind);
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, EmptyHash) {
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -4480,6 +4480,7 @@
 case pp_pragma_push_macro:
 case pp_pragma_pop_macro:
 case pp_pragma_include_alias:
+case pp_pragma_system_header:
 case pp_include_next:
 case decl_at_import:
 case cxx_module_decl:
Index: clang/lib/Lex/DependencyDirectivesScanner.cpp
===
--- clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -652,9 +652,22 @@
 return false;
   }
 
-  // #pragma clang.
-  if (!isNextIdentifierOrSkipLine("module", First, End))
+  FoundId = tryLexIdentifierOrSkipLine(First, End);
+  if (!FoundId)
 return false;
+  Id = *FoundId;
+
+  // #pragma clang system_header
+  if (Id == "system_header") {
+lexPPDirectiveBody(First, End);
+pushDirective(pp_pragma_system_header);
+return false;
+  }
+
+  if (Id != "module") {
+skipLine(First, End);
+return false;
+  }
 
   // #pragma clang module.
   if (!isNextIdentifierOrSkipLine("import", First, End))
Index: clang/include/clang/Lex/DependencyDirectivesScanner.h
===
--- clang/include/clang/Lex/DependencyDirectivesScanner.h
+++ clang/include/clang/Lex/DependencyDirectivesScanner.h
@@ -68,6 +68,7 @@
   pp_pragma_push_macro,
   pp_pragma_pop_macro,
   pp_pragma_include_alias,
+  pp_pragma_system_header,
   pp_include_next,
   pp_if,
   pp_ifdef,


Index: clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -90,7 +90,8 @@
"#pragma pop_macro(A)\n"
"#pragma include_alias(, )\n"
"export module m;\n"
-   "import m;\n",
+   "import m;\n"
+   "#pragma clang system_header\n",
Out, Tokens, Directives));
   EXPECT_EQ(pp_define, Directives[0].Kind);
   EXPECT_EQ(pp_undef, Directives[1].Kind);
@@ -113,7 +114,8 @@
   EXPECT_EQ(pp_pragma_include_alias, Directives[18].Kind);
   EXPECT_EQ(cxx_export_module_decl, Directives[19].Kind);
   EXPECT_EQ(cxx_import_decl, Directives[20].Kind);
-  EXPECT_EQ(pp_eof, Directives[21].Kind);
+  EXPECT_EQ(pp_pragma_system_header, Directives[21].Kind);
+  EXPECT_EQ(pp_eof, Directives[22].Kind);
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, EmptyHash) {
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -4480,6 +4480,7 @@
 case pp_pragma_push_macro:
 case pp_pragma_pop_macro:
 case pp_pragma_include_alias:
+case pp_pragma_system_header:
 case

[clang] 7b492d1 - [clang][deps] Teach dep directive scanner about #pragma clang system_header

2023-05-03 Thread Ben Langmuir via cfe-commits


Author: Ben Langmuir
Date: 2023-05-03T13:53:21-07:00
New Revision: 7b492d1be0ea5b767cc567ca2792a291242f9973

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

LOG: [clang][deps] Teach dep directive scanner about #pragma clang system_header

This ensures we get the correct FileCharacteristic during scanning. In a
yet-to-be-upstreamed branch this fixes observable failures, but it's
also good to handle this on principle: the FileCharacteristic is a
property of the file that is observable in the scanner, so there is
nothing preventing us from depending on it.

rdar://108627403

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

Added: 


Modified: 
clang/include/clang/Lex/DependencyDirectivesScanner.h
clang/lib/Lex/DependencyDirectivesScanner.cpp
clang/lib/Lex/Lexer.cpp
clang/unittests/Lex/DependencyDirectivesScannerTest.cpp

Removed: 




diff  --git a/clang/include/clang/Lex/DependencyDirectivesScanner.h 
b/clang/include/clang/Lex/DependencyDirectivesScanner.h
index 529b93aa0ffbf..0e115906fbfe5 100644
--- a/clang/include/clang/Lex/DependencyDirectivesScanner.h
+++ b/clang/include/clang/Lex/DependencyDirectivesScanner.h
@@ -68,6 +68,7 @@ enum DirectiveKind : uint8_t {
   pp_pragma_push_macro,
   pp_pragma_pop_macro,
   pp_pragma_include_alias,
+  pp_pragma_system_header,
   pp_include_next,
   pp_if,
   pp_ifdef,

diff  --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp 
b/clang/lib/Lex/DependencyDirectivesScanner.cpp
index 0adbaa36bf7cd..a506b49176302 100644
--- a/clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -652,9 +652,22 @@ bool Scanner::lexPragma(const char *, const char 
*const End) {
 return false;
   }
 
-  // #pragma clang.
-  if (!isNextIdentifierOrSkipLine("module", First, End))
+  FoundId = tryLexIdentifierOrSkipLine(First, End);
+  if (!FoundId)
 return false;
+  Id = *FoundId;
+
+  // #pragma clang system_header
+  if (Id == "system_header") {
+lexPPDirectiveBody(First, End);
+pushDirective(pp_pragma_system_header);
+return false;
+  }
+
+  if (Id != "module") {
+skipLine(First, End);
+return false;
+  }
 
   // #pragma clang module.
   if (!isNextIdentifierOrSkipLine("import", First, End))

diff  --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp
index 53cd43d63ea02..1b00f7cde1fb8 100644
--- a/clang/lib/Lex/Lexer.cpp
+++ b/clang/lib/Lex/Lexer.cpp
@@ -4480,6 +4480,7 @@ bool 
Lexer::LexDependencyDirectiveTokenWhileSkipping(Token ) {
 case pp_pragma_push_macro:
 case pp_pragma_pop_macro:
 case pp_pragma_include_alias:
+case pp_pragma_system_header:
 case pp_include_next:
 case decl_at_import:
 case cxx_module_decl:

diff  --git a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp 
b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
index 5222df9fb9eb2..2f8804784a2e4 100644
--- a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -90,7 +90,8 @@ TEST(MinimizeSourceToDependencyDirectivesTest, AllTokens) {
"#pragma pop_macro(A)\n"
"#pragma include_alias(, )\n"
"export module m;\n"
-   "import m;\n",
+   "import m;\n"
+   "#pragma clang system_header\n",
Out, Tokens, Directives));
   EXPECT_EQ(pp_define, Directives[0].Kind);
   EXPECT_EQ(pp_undef, Directives[1].Kind);
@@ -113,7 +114,8 @@ TEST(MinimizeSourceToDependencyDirectivesTest, AllTokens) {
   EXPECT_EQ(pp_pragma_include_alias, Directives[18].Kind);
   EXPECT_EQ(cxx_export_module_decl, Directives[19].Kind);
   EXPECT_EQ(cxx_import_decl, Directives[20].Kind);
-  EXPECT_EQ(pp_eof, Directives[21].Kind);
+  EXPECT_EQ(pp_pragma_system_header, Directives[21].Kind);
+  EXPECT_EQ(pp_eof, Directives[22].Kind);
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, EmptyHash) {



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

[PATCH] D140538: [Clang][CodeGen] Use poison instead of undef for dummy values in CGExpr [NFC]

2023-05-03 Thread Nikita Popov via Phabricator via cfe-commits

nikic added a comment.

The demangling test shouldn't be modified at all.


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

https://reviews.llvm.org/D140538

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

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

2023-05-03 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

`-march=` `-mcpu=` `-mtune=` are from GCC and they traditionally have many 
opinions on the features.
If GCC is going to have `-march=help`, I think adding `-march=help` to Clang to 
match is fine.
But I'd really like to avoid aliases for non-compatibility reasons. They just 
cause confusion and make users undecided about which one is canonical.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054

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

[PATCH] D148776: [Modules] Move modulemaps to header search directories. NFC intended.

2023-05-03 Thread Volodymyr Sapsai 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 rG24f36a215b4e: [Modules] Move modulemaps to header search 
directories. NFC intended. (authored by vsapsai).

Changed prior to commit:
  https://reviews.llvm.org/D148776?vs=515194=519224#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148776

Files:
  clang/include/clang-c/module.modulemap
  clang/include/clang/module.modulemap
  clang/include/module.modulemap
  lldb/include/lldb/module.modulemap
  lldb/include/module.modulemap
  llvm/CMakeLists.txt
  llvm/include/CMakeLists.txt
  llvm/include/llvm-c/module.modulemap
  llvm/include/llvm/CMakeLists.txt
  llvm/include/llvm/module.extern.modulemap
  llvm/include/llvm/module.install.modulemap
  llvm/include/llvm/module.modulemap
  llvm/include/llvm/module.modulemap.build
  llvm/include/module.extern.modulemap
  llvm/include/module.install.modulemap
  llvm/include/module.modulemap
  llvm/include/module.modulemap.build

Index: llvm/include/module.modulemap.build
===
--- llvm/include/module.modulemap.build
+++ llvm/include/module.modulemap.build
@@ -1,13 +1,13 @@
 // This is copied into the build area for a $src != $build compilation.
 module LLVM_Support_DataTypes {
-  header "Support/DataTypes.h"
+  header "llvm/Support/DataTypes.h"
   export *
 }
 module LLVM_Config_ABI_Breaking {
-  header "Config/abi-breaking.h"
+  header "llvm/Config/abi-breaking.h"
   export *
 }
 module LLVM_Config_Config {
-  header "Config/llvm-config.h"
+  header "llvm/Config/llvm-config.h"
   export *
 }
Index: llvm/include/module.modulemap
===
--- /dev/null
+++ llvm/include/module.modulemap
@@ -0,0 +1,428 @@
+module LLVM_C {
+  umbrella "llvm-c"
+  module * { export * }
+}
+
+module LLVM_Analysis {
+  requires cplusplus
+  umbrella "llvm/Analysis"
+  module * { export * }
+
+  // This is intended for (repeated) textual inclusion.
+  textual header "llvm/Analysis/ScalarFuncs.def"
+  textual header "llvm/Analysis/TargetLibraryInfo.def"
+  textual header "llvm/Analysis/VecFuncs.def"
+}
+
+module LLVM_AsmParser {
+  requires cplusplus
+  umbrella "llvm/AsmParser"
+  module * { export * }
+}
+
+module LLVM_CodeGenTypes {
+  requires cplusplus
+
+  module LLT {
+header "llvm/CodeGen/LowLevelType.h" export *
+  }
+  module MVT {
+header "llvm/CodeGen/MachineValueType.h" export *
+extern module LLVM_Extern_CodeGenTypes_Gen "module.extern.modulemap"
+  }
+}
+
+// A module covering CodeGen/ and Target/. These are intertwined
+// and codependent, and thus notionally form a single module.
+module LLVM_Backend {
+  requires cplusplus
+
+  module CodeGen {
+umbrella "llvm/CodeGen"
+module * { export * }
+
+// Exclude these; they're intended to be included into only a single
+// translation unit (or none) and aren't part of this module.
+exclude header "llvm/CodeGen/LinkAllAsmWriterComponents.h"
+exclude header "llvm/CodeGen/LinkAllCodegenComponents.h"
+
+exclude header "llvm/CodeGen/CodeGenPassBuilder.h"
+
+// These are intended for (repeated) textual inclusion.
+textual header "llvm/CodeGen/DIEValue.def"
+textual header "llvm/CodeGen/MachinePassRegistry.def"
+  }
+}
+
+// FIXME: Make this as a submodule of LLVM_Backend again.
+//Doing so causes a linker error in clang-format.
+module LLVM_Backend_Target {
+  umbrella "llvm/Target"
+  module * { export * }
+}
+
+module LLVM_Bitcode {
+ requires cplusplus
+ umbrella "llvm/Bitcode"
+ module * { export * }
+}
+
+module LLVM_Bitstream {
+ requires cplusplus
+ umbrella "llvm/Bitstream"
+ module * { export * }
+}
+
+module LLVM_BinaryFormat {
+requires cplusplus
+umbrella "llvm/BinaryFormat" module * { export * }
+textual header "llvm/BinaryFormat/Dwarf.def"
+textual header "llvm/BinaryFormat/DXContainerConstants.def"
+textual header "llvm/BinaryFormat/DynamicTags.def"
+textual header "llvm/BinaryFormat/MachO.def"
+textual header "llvm/BinaryFormat/MinidumpConstants.def"
+textual header "llvm/BinaryFormat/Swift.def"
+textual header "llvm/BinaryFormat/ELFRelocs/AArch64.def"
+textual header "llvm/BinaryFormat/ELFRelocs/AMDGPU.def"
+textual header "llvm/BinaryFormat/ELFRelocs/ARM.def"
+textual header "llvm/BinaryFormat/ELFRelocs/ARC.def"
+textual header "llvm/BinaryFormat/ELFRelocs/AVR.def"
+textual header "llvm/BinaryFormat/ELFRelocs/BPF.def"
+textual header "llvm/BinaryFormat/ELFRelocs/CSKY.def"
+textual header "llvm/BinaryFormat/ELFRelocs/Hexagon.def"
+textual header "llvm/BinaryFormat/ELFRelocs/i386.def"
+textual header "llvm/BinaryFormat/ELFRelocs/Lanai.def"
+textual header "llvm/BinaryFormat/ELFRelocs/LoongArch.def"
+textual header

[clang] 24f36a2 - [Modules] Move modulemaps to header search directories. NFC intended.

2023-05-03 Thread Volodymyr Sapsai via cfe-commits


Author: Volodymyr Sapsai
Date: 2023-05-03T13:07:47-07:00
New Revision: 24f36a215b4eabd1d0e4abcce0c9277085d88a96

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

LOG: [Modules] Move modulemaps to header search directories. NFC intended.

In code we use `#include "llvm/Lib/Header.h"` which is located in
"llvm/include/llvm/Lib/Header.h", so we use "llvm/include/" as a header
search path. We should put modulemaps in the same directory and
shouldn't rely on clang to search in immediate subdirectories.

rdar://106677321

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

Added: 
clang/include/module.modulemap
lldb/include/module.modulemap
llvm/include/CMakeLists.txt
llvm/include/module.extern.modulemap
llvm/include/module.install.modulemap
llvm/include/module.modulemap
llvm/include/module.modulemap.build

Modified: 
llvm/CMakeLists.txt
llvm/include/llvm/CMakeLists.txt

Removed: 
clang/include/clang-c/module.modulemap
clang/include/clang/module.modulemap
lldb/include/lldb/module.modulemap
llvm/include/llvm-c/module.modulemap
llvm/include/llvm/module.extern.modulemap
llvm/include/llvm/module.install.modulemap
llvm/include/llvm/module.modulemap
llvm/include/llvm/module.modulemap.build



diff  --git a/clang/include/clang-c/module.modulemap 
b/clang/include/clang-c/module.modulemap
deleted file mode 100644
index 95a59d62344cc..0
--- a/clang/include/clang-c/module.modulemap
+++ /dev/null
@@ -1,4 +0,0 @@
-module Clang_C {
-  umbrella "."
-  module * { export * }
-}

diff  --git a/clang/include/clang/module.modulemap 
b/clang/include/clang/module.modulemap
deleted file mode 100644
index 624045dc4c57b..0
--- a/clang/include/clang/module.modulemap
+++ /dev/null
@@ -1,199 +0,0 @@
-module Clang_Analysis {
-  requires cplusplus
-  umbrella "Analysis"
-
-  textual header "Analysis/Analyses/ThreadSafetyOps.def"
-
-  module * { export * }
-
-  // FIXME: Exclude these headers to avoid pulling all of the AST matchers
-  // library into clang. Due to inline key functions in the headers,
-  // importing the AST matchers library gives a link dependency on the AST
-  // matchers (and thus the AST), which clang-format should not have.
-  exclude header "Analysis/Analyses/ExprMutationAnalyzer.h"
-}
-
-module Clang_AST {
-  requires cplusplus
-  umbrella "AST"
-
-  textual header "AST/BuiltinTypes.def"
-  textual header "AST/CXXRecordDeclDefinitionBits.def"
-  textual header "AST/OperationKinds.def"
-  textual header "AST/TypeLocNodes.def"
-
-  module * { export * }
-}
-
-module Clang_ASTMatchers { requires cplusplus umbrella "ASTMatchers" module * 
{ export * } }
-
-module Clang_Basic {
-  requires cplusplus
-  umbrella "Basic"
-
-  textual header "Basic/AArch64SVEACLETypes.def"
-  textual header "Basic/BuiltinsAArch64.def"
-  textual header "Basic/BuiltinsAMDGPU.def"
-  textual header "Basic/BuiltinsAArch64NeonSVEBridge.def"
-  textual header "Basic/BuiltinsAArch64NeonSVEBridge_cg.def"
-  textual header "Basic/BuiltinsARM.def"
-  textual header "Basic/BuiltinsBPF.def"
-  textual header "Basic/Builtins.def"
-  textual header "Basic/BuiltinHeaders.def"
-  textual header "Basic/BuiltinsHexagon.def"
-  textual header "Basic/BuiltinsHexagonDep.def"
-  textual header "Basic/BuiltinsHexagonMapCustomDep.def"
-  textual header "Basic/BuiltinsLoongArch.def"
-  textual header "Basic/BuiltinsMips.def"
-  textual header "Basic/BuiltinsNEON.def"
-  textual header "Basic/BuiltinsNVPTX.def"
-  textual header "Basic/BuiltinsPPC.def"
-  textual header "Basic/BuiltinsRISCV.def"
-  textual header "Basic/BuiltinsRISCVVector.def"
-  textual header "Basic/BuiltinsSVE.def"
-  textual header "Basic/BuiltinsSystemZ.def"
-  textual header "Basic/BuiltinsVE.def"
-  textual header "Basic/BuiltinsVEVL.gen.def"
-  textual header "Basic/BuiltinsWebAssembly.def"
-  textual header "Basic/BuiltinsX86.def"
-  textual header "Basic/BuiltinsX86_64.def"
-  textual header "Basic/BuiltinsXCore.def"
-  textual header "Basic/CodeGenOptions.def"
-  textual header "Basic/DiagnosticOptions.def"
-  textual header "Basic/Features.def"
-  textual header "Basic/FPOptions.def"
-  textual header "Basic/MSP430Target.def"
-  textual header "Basic/LangOptions.def"
-  textual header "Basic/OpenCLExtensions.def"
-  textual header "Basic/OpenCLImageTypes.def"
-  textual header "Basic/OpenCLExtensionTypes.def"
-  textual header "Basic/OpenMPKinds.def"
-  textual header "Basic/OperatorKinds.def"
-  textual header "Basic/PPCTypes.def"
-  textual header "Basic/RISCVVTypes.def"
-  textual header "Basic/Sanitizers.def"
-  textual header "Basic/TargetCXXABI.def"
-  textual header "Basic/TransformTypeTraits.def"
-  textual header "Basic/TokenKinds.def"
-  textual header

[clang] 3aaf0be - Account for whitespace in the test regex

2023-05-03 Thread Aaron Ballman via cfe-commits


Author: Aaron Ballman
Date: 2023-05-03T16:05:27-04:00
New Revision: 3aaf0bed1c68f0b0052a569437cbdf489ff68b56

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

LOG: Account for whitespace in the test regex

Yet another amendment to 9bb28a18d962e8f6e3fa8f48bd2c6dc183154d26, this
addresses issues found in:
https://lab.llvm.org/buildbot/#/builders/188/builds/29204

Added: 


Modified: 
clang/test/CodeGen/nullptr.c

Removed: 




diff  --git a/clang/test/CodeGen/nullptr.c b/clang/test/CodeGen/nullptr.c
index 88a3a3e8b1ab..7ea951213df0 100644
--- a/clang/test/CodeGen/nullptr.c
+++ b/clang/test/CodeGen/nullptr.c
@@ -53,7 +53,7 @@ void test() {
   // CHECK: store ptr null, ptr %[[nullptr_t_from_int]]
 
   // Calls
-  // CHECK: call void @bool_func(i1 noundef {{(zeroext)?}} false)
+  // CHECK: call void @bool_func(i1 noundef {{(zeroext )?}}false)
   // CHECK: call void @nullptr_func(ptr null)
   // CHECK: call void @nullptr_func(ptr null)
   // CHECK: call void @nullptr_func(ptr null)



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

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

2023-05-03 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D147626#4316535 , @efriedma wrote:

> In D147626#4316212 , @aaron.ballman 
> wrote:
>
>> In D147626#4316190 , @efriedma 
>> wrote:
>>
 If there's not indications of this being disruptive on non-MSVC-compatible 
 targets, then we may still be able to get away with rejecting the 
 extension there.
>>>
>>> If we need to have the codepath anyway, there isn't much harm in allowing 
>>> it on all targets, I think.  There's really only one possible 
>>> interpretation for the construct.
>>
>> You would think, except the GCC extension differs based on C vs C++: 
>> https://godbolt.org/z/E14Yz37To as does the extension in Clang, but 
>> differently than GCC: https://godbolt.org/z/zYznaYPf5 and so we'd also have 
>> to dig into solving that if we wanted to keep GCC compatibility behavior.
>
> I don't see any unions there?  Declaring a flexible array is separate from 
> flexible array initialization.

Sorry, that was a rather bad think-o -- those examples came from GCC's 
documentation of FAM extensions 
(https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html), but you're right, that 
example is not about declarations but initialization. @Fznamznon had an 
example: https://godbolt.org/z/jsWjv7svr

> Actually, despite my saying the interpretation for unions is "obvious", it's 
> actually a little more weird than I thought: `union x { short x[]; }; 
> static_assert(sizeof(x)==2);` compiles with msvc.

Well that's... a bit shorter... than I would have expected that union to be. 
(I'm not apologizing for the pun.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147626

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

[PATCH] D149752: WIP: Debug symlink creation

2023-05-03 Thread Louis Dionne via Phabricator via cfe-commits

ldionne updated this revision to Diff 519218.
ldionne added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Poke CI


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149752

Files:
  clang/foo
  llvm/cmake/modules/LLVMInstallSymlink.cmake


Index: llvm/cmake/modules/LLVMInstallSymlink.cmake
===
--- llvm/cmake/modules/LLVMInstallSymlink.cmake
+++ llvm/cmake/modules/LLVMInstallSymlink.cmake
@@ -17,7 +17,7 @@
   endif()
   set(outdir "${DESTDIR}${outdir}")
 
-  message(STATUS "Creating ${name}")
+  message(STATUS "Creating ${name} with ${link_or_copy}")
 
   execute_process(
 COMMAND "${CMAKE_COMMAND}" -E ${link_or_copy} "${target}" "${name}"


Index: llvm/cmake/modules/LLVMInstallSymlink.cmake
===
--- llvm/cmake/modules/LLVMInstallSymlink.cmake
+++ llvm/cmake/modules/LLVMInstallSymlink.cmake
@@ -17,7 +17,7 @@
   endif()
   set(outdir "${DESTDIR}${outdir}")
 
-  message(STATUS "Creating ${name}")
+  message(STATUS "Creating ${name} with ${link_or_copy}")
 
   execute_process(
 COMMAND "${CMAKE_COMMAND}" -E ${link_or_copy} "${target}" "${name}"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144999: [RFC][MC][MachO]Only emits compact-unwind format for "canonical" personality symbols. For the rest, use DWARFs.

2023-05-03 Thread Vy Nguyen via Phabricator via cfe-commits

oontvoo updated this revision to Diff 519216.
oontvoo added a comment.
Herald added a subscriber: ormris.

- defined equivalent flag in the clang driver


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144999

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/Driver/femit-dwarf-unwind.c
  clang/tools/driver/cc1as_main.cpp
  lld/MachO/UnwindInfoSection.cpp
  lld/test/MachO/Inputs/eh-frame-x86_64-r.o
  lld/test/MachO/compact-unwind-both-local-and-dylib-personality.s
  lld/test/MachO/compact-unwind-generated.test
  lld/test/MachO/compact-unwind-lsda-folding.s
  lld/test/MachO/compact-unwind-stack-ind.s
  lld/test/MachO/compact-unwind.s
  lld/test/MachO/eh-frame-personality-dedup.s
  lld/test/MachO/eh-frame.s
  lld/test/MachO/icf-only-lsda-folded.s
  lld/test/MachO/icf.s
  lld/test/MachO/invalid/compact-unwind-bad-reloc.s
  lld/test/MachO/invalid/compact-unwind-personalities.s
  llvm/include/llvm/MC/MCAsmBackend.h
  llvm/include/llvm/MC/MCContext.h
  llvm/include/llvm/MC/MCTargetOptions.h
  llvm/include/llvm/MC/MCTargetOptionsCommandFlags.h
  llvm/lib/MC/MCAsmBackend.cpp
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCStreamer.cpp
  llvm/lib/MC/MCTargetOptionsCommandFlags.cpp
  llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackendDarwin.h
  llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

Index: llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
===
--- llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
+++ llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
@@ -1350,9 +1350,13 @@
 
   /// Implementation of algorithm to generate the compact unwind encoding
   /// for the CFI instructions.
-  uint32_t
-  generateCompactUnwindEncoding(ArrayRef Instrs) const override {
+  uint32_t generateCompactUnwindEncoding(const MCDwarfFrameInfo *FI,
+ const MCContext *Ctxt) const override {
+ArrayRef Instrs = FI->Instructions;
 if (Instrs.empty()) return 0;
+if (!isDarwinCanonicalPersonality(FI->Personality) &&
+!Ctxt->emitCompactUnwindNonCanonical())
+  return CU::UNWIND_MODE_DWARF;
 
 // Reset the saved registers.
 unsigned SavedRegIdx = 0;
Index: llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackendDarwin.h
===
--- llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackendDarwin.h
+++ llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackendDarwin.h
@@ -11,6 +11,7 @@
 
 #include "ARMAsmBackend.h"
 #include "llvm/BinaryFormat/MachO.h"
+#include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCObjectWriter.h"
 
 namespace llvm {
@@ -32,8 +33,8 @@
 /*Is64Bit=*/false, cantFail(MachO::getCPUType(TT)), Subtype);
   }
 
-  uint32_t generateCompactUnwindEncoding(
-  ArrayRef Instrs) const override;
+  uint32_t generateCompactUnwindEncoding(const MCDwarfFrameInfo *FI,
+ const MCContext *Ctxt) const override;
 };
 } // end namespace llvm
 
Index: llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
===
--- llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
+++ llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
@@ -1108,14 +1108,19 @@
 /// encoded in compact unwind, the method returns UNWIND_ARM_MODE_DWARF which
 /// tells the runtime to fallback and unwind using dwarf.
 uint32_t ARMAsmBackendDarwin::generateCompactUnwindEncoding(
-ArrayRef Instrs) const {
+const MCDwarfFrameInfo *FI, const MCContext *Ctxt) const {
   DEBUG_WITH_TYPE("compact-unwind", llvm::dbgs() << "generateCU()\n");
   // Only armv7k uses CFI based unwinding.
   if (Subtype != MachO::CPU_SUBTYPE_ARM_V7K)
 return 0;
   // No .cfi directives means no frame.
+  ArrayRef Instrs = FI->Instructions;
   if (Instrs.empty())
 return 0;
+  if (!isDarwinCanonicalPersonality(FI->Personality) &&
+  !Ctxt->emitCompactUnwindNonCanonical())
+return CU::UNWIND_ARM_MODE_DWARF;
+
   // Start off assuming CFA is at SP+0.
   unsigned CFARegister = ARM::SP;
   int CFARegisterOffset = 0;
Index: llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
===
--- llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
+++ llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
@@ -564,10 +564,14 @@
   }
 
   /// Generate the compact unwind encoding from the CFI directives.
-  uint32_t generateCompactUnwindEncoding(
- ArrayRef Instrs) const override {
+  uint32_t generateCompactUnwindEncoding(const MCDwarfFrameInfo *FI,
+ const MCContext *Ctxt) const override {
+ArrayRef

[clang] 24f8122 - Speculative fix for 9bb28a18d962e8f6e3fa8f48bd2c6dc183154d26

2023-05-03 Thread Aaron Ballman via cfe-commits


Author: Aaron Ballman
Date: 2023-05-03T15:46:47-04:00
New Revision: 24f81228abaf158460157f26b0b6db2bfe2f05df

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

LOG: Speculative fix for 9bb28a18d962e8f6e3fa8f48bd2c6dc183154d26

Added: 


Modified: 
clang/test/CodeGen/nullptr.c

Removed: 




diff  --git a/clang/test/CodeGen/nullptr.c b/clang/test/CodeGen/nullptr.c
index 796c6414ae45..88a3a3e8b1ab 100644
--- a/clang/test/CodeGen/nullptr.c
+++ b/clang/test/CodeGen/nullptr.c
@@ -53,7 +53,7 @@ void test() {
   // CHECK: store ptr null, ptr %[[nullptr_t_from_int]]
 
   // Calls
-  // CHECK: call void @bool_func(i1 noundef {{zeroext?}} false)
+  // CHECK: call void @bool_func(i1 noundef {{(zeroext)?}} false)
   // CHECK: call void @nullptr_func(ptr null)
   // CHECK: call void @nullptr_func(ptr null)
   // CHECK: call void @nullptr_func(ptr null)



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

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

2023-05-03 Thread Arthur Eubanks via Phabricator via cfe-commits

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

add -verify to module test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146764

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/IgnoreExpr.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/Modules/predefined.cpp
  clang/test/Sema/ms_predefined_expr.cpp

Index: clang/test/Sema/ms_predefined_expr.cpp
===
--- /dev/null
+++ clang/test/Sema/ms_predefined_expr.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
+
+void f() {
+ const char a[] = __FUNCTION__; // expected-warning{{initializing an array from a '__FUNCTION__' predefined identifier is a Microsoft extension}}
+ const char b[] = __FUNCDNAME__; // expected-warning{{initializing an array from a '__FUNCDNAME__' predefined identifier is a Microsoft extension}}
+ const char c[] = __FUNCSIG__; // expected-warning{{initializing an array from a '__FUNCSIG__' predefined identifier is a Microsoft extension}}
+ const char d[] = __func__; // expected-warning{{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
+ const char e[] = __PRETTY_FUNCTION__; // expected-warning{{initializing an array from a '__PRETTY_FUNCTION__' predefined identifier is a Microsoft extension}}
+}
Index: clang/test/Modules/predefined.cpp
===
--- /dev/null
+++ clang/test/Modules/predefined.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -x c++ -std=c++20 -emit-module-interface a.h -o a.pcm -fms-extensions -verify
+// RUN: %clang_cc1 -std=c++20 a.cpp -fmodule-file=A=a.pcm -fms-extensions -fsyntax-only -verify
+
+//--- a.h
+
+// expected-no-diagnostics
+
+export module A;
+
+export template 
+void f() {
+char a[] = __func__;
+}
+
+//--- a.cpp
+
+// expected-warning@a.h:8 {{initializing an array from a '__func__' predefined identifier is a Microsoft extension}}
+
+import A;
+
+void g() {
+f(); // expected-note {{in instantiation of function template specialization 'f' requested here}}
+}
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -593,6 +593,7 @@
   bool HasFunctionName = E->getFunctionName() != nullptr;
   Record.push_back(HasFunctionName);
   Record.push_back(E->getIdentKind()); // FIXME: stable encoding
+  Record.push_back(E->isTransparent());
   Record.AddSourceLocation(E->getLocation());
   if (HasFunctionName)
 Record.AddStmt(E->getFunctionName());
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -582,6 +582,7 @@
   bool HasFunctionName = Record.readInt();
   E->PredefinedExprBits.HasFunctionName = HasFunctionName;
   E->PredefinedExprBits.Kind = Record.readInt();
+  E->PredefinedExprBits.IsTransparent = Record.readInt();
   E->setLocation(readSourceLocation());
   if (HasFunctionName)
 E->setFunctionName(cast(Record.readSubExpr()));
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -173,6 +173,8 @@
   E = GSE->getResultExpr();
 } else if (ChooseExpr *CE = dyn_cast(E)) {
   E = CE->getChosenSubExpr();
+} else if (PredefinedExpr *PE = dyn_cast(E)) {
+  E = PE->getFunctionName();
 } else {
   llvm_unreachable("unexpected expr in string literal init");
 }
@@ -8500,6 +8502,15 @@
 << Init->getSourceRange();
   }
 
+  if (S.getLangOpts().MicrosoftExt && Args.size() == 1 &&
+  isa(Args[0])) {
+// Produce a Microsoft compatibility warning when initializing from a
+// predefined expression since MSVC treats predefined expressions as string
+// literals.
+Expr *Init = Args[0];
+S.Diag(Init->getBeginLoc(), diag::ext_init_from_predefined) << Init;
+  }
+
   // OpenCL v2.0 s6.13.11.1. atomic variables can be initialized in global scope
   QualType ETy = Entity.getType();
   bool HasGlobalAS = ETy.hasAddressSpace() &&
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -3580,7 +3580,8 @@
 }
   }
 
-  return

[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

2023-05-03 Thread 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.

Just some minor nits from me, but LG with those fixed up.




Comment at: clang/include/clang/Basic/DiagnosticGroups.td:230-232
 def CXX20Designator : DiagGroup<"c++20-designator">;
+def CXX2bDefCompCallsNonConstexpr : 
DiagGroup<"c++2b-default-comp-relaxed-constexpr">;
 // Allow -Wno-c99-designator to be used to turn off all warnings on valid C99





Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9425
+  "invokes a non-constexpr comparison function is a C++2b extension">,
+  InGroup;
+def warn_cxx2b_compat_defaulted_comparison_constexpr_mismatch : Warning<

Note, you should coordinate the `2b`->`23` changes with 
https://reviews.llvm.org/D149553



Comment at: clang/www/cxx_status.html:1486
+Clang 17 (Partial)
+   We do not support outside of defaulted special memeber functions 
the change that constexpr functions no
+longer have to be constexpr compatible but rather support a less 
restricted requirements for constexpr

Indentation looks a bit off here.


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

https://reviews.llvm.org/D146090

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

[PATCH] D149776: Re-land "[AMDGPU] Define data layout entries for buffers""

2023-05-03 Thread Krzysztof Drewniak via Phabricator via cfe-commits

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf0415f2a456d: Re-land [AMDGPU] Define data layout 
entries for buffers (authored by krzysz00).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149776

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

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

2023-05-03 Thread Kugan Vivekanandarajah via Phabricator via cfe-commits

kuganv added a comment.

In D148088#4316352 , @kadircet wrote:

> Sorry I was trying to give some brief idea about what it might look like in 
> `Implementation Concerns` section above, but let me give some more details;
>
> I think we can just change the signature for PreambleParsedCallback 
> 
>  to pass along refcounted objects. forgot to mention in the first comment, 
> but we should also change the CanonicalIncludes to be a shared_ptr so that it 
> can outlive the PreambleData. We should invoke the callback inside 
> buildPreamble 
> 
>  after a successful build. Afterwards we should also change the signature for 
> onPreambleAST 
> 
>  to take AST, PP and CanonicalIncludes as ref-counted objects again and 
> `PreambleThread::build` should just forward objects received from 
> `PreambleParsedCallback`. Afterwards inside the 
> UpdateIndexCallbacks::onPreambleAST 
> 
>  we can just invoke indexing on `Tasks` if it's present or synchronously in 
> the absence of it.

Thanks  @kadircet for the details.

> Does that make sense?

It does make sense. Let me work on this and get back to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148088

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

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

2023-05-03 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

In D147626#4316212 , @aaron.ballman 
wrote:

> In D147626#4316190 , @efriedma 
> wrote:
>
>>> If there's not indications of this being disruptive on non-MSVC-compatible 
>>> targets, then we may still be able to get away with rejecting the extension 
>>> there.
>>
>> If we need to have the codepath anyway, there isn't much harm in allowing it 
>> on all targets, I think.  There's really only one possible interpretation 
>> for the construct.
>
> You would think, except the GCC extension differs based on C vs C++: 
> https://godbolt.org/z/E14Yz37To as does the extension in Clang, but 
> differently than GCC: https://godbolt.org/z/zYznaYPf5 and so we'd also have 
> to dig into solving that if we wanted to keep GCC compatibility behavior.

I don't see any unions there?  Declaring a flexible array is separate from 
flexible array initialization.

Actually, despite my saying the interpretation for unions is "obvious", it's 
actually a little more weird than I thought: `union x { short x[]; }; 
static_assert(sizeof(x)==2);` compiles with msvc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147626

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

[PATCH] D141215: [clang-repl] Introduce Value to capture expression results

2023-05-03 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang/include/clang/Interpreter/Value.h:46
+
+#define REPL_BUILTIN_TYPES 
\
+  X(bool, Bool)
\

junaire wrote:
> aaron.ballman wrote:
> > v.g.vassilev wrote:
> > > aaron.ballman wrote:
> > > > v.g.vassilev wrote:
> > > > > aaron.ballman wrote:
> > > > > > Is this expected to be a complete list of builtin types? e.g., 
> > > > > > should this have `char8_t` and `void` and `wchar_t`, etc? Should 
> > > > > > this be including `clang/include/clang/AST/BuiltinTypes.def` 
> > > > > > instead of manually maintaining the list?
> > > > > This is used for various things including storing the bits into a 
> > > > > big-endian agnostic way. For `void` we have a special case in the 
> > > > > Value and we cannot define a union of a `void` type. We can include 
> > > > > the others that you suggest. All the relevant ones are described in 
> > > > > `clang::BuiltinType::getName`.
> > > > > 
> > > > > We cannot use `BuiltinTypes.def` because we want to have a mapping 
> > > > > between the type as written in the language (eg, `bool`, `unsigned`, 
> > > > > etc) and its underlying type name. That mapping is not available in 
> > > > > `BuiltinTypes.def`. Ideally we should extend `BuiltinTypes.def` 
> > > > > somehow but I'd prefer outside of this patch. One of the challenges 
> > > > > is that some of the types depend on the language options (eg. `_Bool` 
> > > > > vs `bool`) and I am not sure this can be resolved by tablegen.
> > > > > 
> > > > > On a broader perspective, the `Value` class is responsible for two 
> > > > > things: (a) get a value from the interpreter to compiled code (see 
> > > > > test case); (b) set a value from compiled code to the interpreter. 
> > > > > The second case is not yet covered (I can open soon another patch). 
> > > > > The major use-case is calling at runtime functions and taking input 
> > > > > parameters from compiled code.
> > > > > 
> > > > > FWIW, we should probably move all of these entities in a separate 
> > > > > namespace. I'd suggest `caas` (compiler-as-a-service) and possibly 
> > > > > rename the `Value` to `InterpreterValue` since `Value` is very 
> > > > > generic and there are already a couple of classes with that name in 
> > > > > llvm and clang. 
> > > > > We can include the others that you suggest. All the relevant ones are 
> > > > > described in clang::BuiltinType::getName.
> > > > 
> > > > Okay, so the plan is to handle all the builtin types (`_BitInt`, 
> > > > `_Complex`, various floating point formats, etc)? Will that be part of 
> > > > this patch or in follow-up work? (My intuition is that we should 
> > > > consider it up front because some of the builtin types are interesting 
> > > > -- like `_BitInt` because it's parameterized, which makes it novel 
> > > > compared to the other types.)
> > > > 
> > > > > We cannot use BuiltinTypes.def because we want to have a mapping 
> > > > > between the type as written in the language (eg, bool, unsigned, etc) 
> > > > > and its underlying type name. That mapping is not available in 
> > > > > BuiltinTypes.def. Ideally we should extend BuiltinTypes.def somehow 
> > > > > but I'd prefer outside of this patch. One of the challenges is that 
> > > > > some of the types depend on the language options (eg. _Bool vs bool) 
> > > > > and I am not sure this can be resolved by tablegen.
> > > > 
> > > > Thanks for the explanation! BuiltinTypes.def works well enough for 
> > > > times when we want to use macros and include the file to generate 
> > > > switch cases and the likes, but you're right that it's not well-suited 
> > > > for this. One thing to consider is whether we should change 
> > > > `BuiltinTypes.def` to be `BuiltinTypes.td` instead and use tablegen to 
> > > > generate the macro/include dance form as well as other output (such as 
> > > > for your needs, that can then consider language options or more complex 
> > > > predicates).
> > > > 
> > > > > FWIW, we should probably move all of these entities in a separate 
> > > > > namespace. I'd suggest caas (compiler-as-a-service) and possibly 
> > > > > rename the Value to InterpreterValue since Value is very generic and 
> > > > > there are already a couple of classes with that name in llvm and 
> > > > > clang.
> > > > 
> > > > I'm not in love with the name `caas` because that's not really a common 
> > > > acronym or abbreviation (and it looks like a typo due to `aa`). 
> > > > However, we already have an `interp` namespace in Clang for one of the 
> > > > other interpreters (constant expression evaluation), so that's not 
> > > > available for use. How about `repl` though?
> > > > 
> > > > As for considering changing the name from `Value` because of how many 
> > > > other `Value` types we have already... that's both a reason to rename 
> > > > and reason not to

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

2023-05-03 Thread Nikita Popov via Phabricator via cfe-commits

nikic added a comment.

In D143624#4316357 , @aeubanks wrote:

> In D143624#4315508 , @nikic wrote:
>
>> In D143624#4315468 , @dmgreen 
>> wrote:
>>
>>> It looks like there is quite a lot more optimization that happens to the 
>>> function being always-inlined (__SSAT) before this change. Through multiple 
>>> rounds of instcombine, almost to the end of the pass pipeline. The new 
>>> version runs a lot less before inlining, only running 
>>> instcombine->simplifycfg and not seeing another instcombine to clean up the 
>>> results. Is that because the AlwaysInlinePass is a module pass and it now 
>>> only runs the passes up to that point?
>>
>> Yes, which is why I personally think this change isn't a good idea. This 
>> essentially breaks our invariant that functions get simplified before they 
>> are inlined. This significantly alters the way alwaysinline functions will 
>> be optimized relative to normally inlined functions.
>
> That invariant shouldn't matter if we're not using heuristics to inline. The 
> normal heuristic-based inliner will still work on simplified callees, but now 
> with the additional benefit of seeing the state of an SCC where there may be 
> alwaysinline calls after the inlinings that must happen having happened.

I agree in the sense that for alwaysinling the simplification doesn't matter 
for cost modelling purposes. However, it can still have other benefits. One is 
that we don't repeat unnecessary work. Any simplification we do before inlining 
doesn't have to be repeated for every (potentially transitive) caller it gets 
inlined into. The other (and in a way, opposite) is that we simplify the 
function before inlining and then again the callee after inlining, which may 
paper over phase ordering issues (or the problem discussed above, which is kind 
of in the same category). Of course, this is not what this is intended for and 
we should endeavor to fix such issues -- but the practical outcome is still 
that you'll probably get worse optimization if you use alwaysinline vs inline 
after this change, which seems kind of unintuitive.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143624

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

[clang] 298d9be - Another fix for 9bb28a18d962e8f6e3fa8f48bd2c6dc183154d26

2023-05-03 Thread Aaron Ballman via cfe-commits


Author: Aaron Ballman
Date: 2023-05-03T15:27:59-04:00
New Revision: 298d9becef655c34731512ff3d41e3fbb3fca155

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

LOG: Another fix for 9bb28a18d962e8f6e3fa8f48bd2c6dc183154d26

This addresses the issue found in:
https://lab.llvm.org/buildbot/#/builders/245/builds/7882

This time, the issue was that not every platform has the same alignment
values, so those were removed from the test case.

Added: 


Modified: 
clang/test/CodeGen/nullptr.c

Removed: 




diff  --git a/clang/test/CodeGen/nullptr.c b/clang/test/CodeGen/nullptr.c
index 59ec0fd1906d..796c6414ae45 100644
--- a/clang/test/CodeGen/nullptr.c
+++ b/clang/test/CodeGen/nullptr.c
@@ -32,25 +32,25 @@ void test() {
   nullptr_func(false);
 
   // Allocation of locals
-  // CHECK: %[[bool_from_nullptr_t:.*]] = alloca i8, align 1
-  // CHECK: %[[nullptr_t_from_nullptr:.*]] = alloca ptr, align 8
-  // CHECK: %[[vp_from_nullptr_t:.*]] = alloca ptr, align 8
-  // CHECK: %[[nullptr_t_from_vp:.*]] = alloca ptr, align 8
-  // CHECK: %[[nullptr_t_from_int:.*]] = alloca ptr, align 8
+  // CHECK: %[[bool_from_nullptr_t:.*]] = alloca i8
+  // CHECK: %[[nullptr_t_from_nullptr:.*]] = alloca ptr
+  // CHECK: %[[vp_from_nullptr_t:.*]] = alloca ptr
+  // CHECK: %[[nullptr_t_from_vp:.*]] = alloca ptr
+  // CHECK: %[[nullptr_t_from_int:.*]] = alloca ptr
 
   // Initialization of locals
-  // CHECK: store i8 0, ptr %[[bool_from_nullptr_t]], align 1
-  // CHECK: store ptr null, ptr %[[nullptr_t_from_nullptr]], align 8
-  // CHECK: store ptr null, ptr %[[vp_from_nullptr_t]], align 8
-  // CHECK: store ptr null, ptr %[[nullptr_t_from_vp]], align 8
-  // CHECK: store ptr null, ptr %[[nullptr_t_from_int]], align 8
+  // CHECK: store i8 0, ptr %[[bool_from_nullptr_t]]
+  // CHECK: store ptr null, ptr %[[nullptr_t_from_nullptr]]
+  // CHECK: store ptr null, ptr %[[vp_from_nullptr_t]]
+  // CHECK: store ptr null, ptr %[[nullptr_t_from_vp]]
+  // CHECK: store ptr null, ptr %[[nullptr_t_from_int]]
 
   // Assignment expressions
-  // CHECK: store i8 0, ptr %[[bool_from_nullptr_t]], align 1
-  // CHECK: store ptr null, ptr %[[nullptr_t_from_nullptr]], align 8
-  // CHECK: store ptr null, ptr %[[vp_from_nullptr_t]], align 8
-  // CHECK: store ptr null, ptr %[[nullptr_t_from_vp]], align 8
-  // CHECK: store ptr null, ptr %[[nullptr_t_from_int]], align 8
+  // CHECK: store i8 0, ptr %[[bool_from_nullptr_t]]
+  // CHECK: store ptr null, ptr %[[nullptr_t_from_nullptr]]
+  // CHECK: store ptr null, ptr %[[vp_from_nullptr_t]]
+  // CHECK: store ptr null, ptr %[[nullptr_t_from_vp]]
+  // CHECK: store ptr null, ptr %[[nullptr_t_from_int]]
 
   // Calls
   // CHECK: call void @bool_func(i1 noundef {{zeroext?}} false)



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

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

2023-05-03 Thread Tom Honermann via Phabricator via cfe-commits

tahonermann added inline comments.



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

HazardyKnusperkeks wrote:
> HazardyKnusperkeks wrote:
> > HazardyKnusperkeks wrote:
> > > MyDeveloperDay wrote:
> > > > owenpan wrote:
> > > > > tahonermann wrote:
> > > > > > How about using an init capture instead? This will suffice to avoid 
> > > > > > one of the copies but means that `InsertBraces` doesn't get set 
> > > > > > until the lambda is invoked. I wouldn't expect that to matter 
> > > > > > though.
> > > > > I'm not sure if it's worth the trouble, but if I really had to 
> > > > > bother, I would do something like the above.
> > > > Apart from perhaps the unnecessary copying from Expanded -> S.. doesn't 
> > > > this bascially do the same without us having to remember to put 
> > > > Expanded back afterwards? I don't see how using Expanded is any 
> > > > different from using S (other than the copy)
> > > > 
> > > > ```
> > > > if (Style.InsertBraces) {
> > > >   FormatStyle S = Expanded;
> > > >   S.InsertBraces = true;
> > > >   Passes.emplace_back([&](const Environment ) {
> > > > return BracesInserter(Env, S).process(/*SkipAnnotation=*/true);
> > > >   });
> > > > }
> > > > ```
> > > > I'm not sure if it's worth the trouble, but if I really had to bother, 
> > > > I would do something like the above.
> > > 
> > > That wouldn't work, since the pass is only executed afterwards.
> > > Apart from perhaps the unnecessary copying from Expanded -> S.. doesn't 
> > > this bascially do the same without us having to remember to put Expanded 
> > > back afterwards? I don't see how using Expanded is any different from 
> > > using S (other than the copy)
> > > 
> > > ```
> > > if (Style.InsertBraces) {
> > >   FormatStyle S = Expanded;
> > >   S.InsertBraces = true;
> > >   Passes.emplace_back([&](const Environment ) {
> > > return BracesInserter(Env, S).process(/*SkipAnnotation=*/true);
> > >   });
> > > }
> > > ```
> > 
> > That would have a dangling reference to S and most likely don't work as 
> > wished.
> This should work. One could replace the two assignments with a RAII wrapper 
> which restores the old value.
Here is another option; it is just the original code but with an init capture 
that does a move instead of a copy. Coverity shouldn't complain about a move 
capture (if it does, I'll file a support case with Synopsys).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149647

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

[PATCH] D149777: [clang][deps] Teach dep directive scanner about #pragma clang system_header

2023-05-03 Thread Ben Langmuir via Phabricator via cfe-commits

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

This ensures we get the correct FileCharacteristic during scanning. In a
yet-to-be-upstreamed branch this fixes observable failures, but it's
also good to handle this on principle: the FileCharacteristic is a
property of the file that is observable in the scanner, so there is
nothing preventing us from depending on it.

rdar://108627403


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149777

Files:
  clang/include/clang/Lex/DependencyDirectivesScanner.h
  clang/lib/Lex/DependencyDirectivesScanner.cpp
  clang/lib/Lex/Lexer.cpp
  clang/unittests/Lex/DependencyDirectivesScannerTest.cpp


Index: clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -90,7 +90,8 @@
"#pragma pop_macro(A)\n"
"#pragma include_alias(, )\n"
"export module m;\n"
-   "import m;\n",
+   "import m;\n"
+   "#pragma clang system_header\n",
Out, Tokens, Directives));
   EXPECT_EQ(pp_define, Directives[0].Kind);
   EXPECT_EQ(pp_undef, Directives[1].Kind);
@@ -113,7 +114,8 @@
   EXPECT_EQ(pp_pragma_include_alias, Directives[18].Kind);
   EXPECT_EQ(cxx_export_module_decl, Directives[19].Kind);
   EXPECT_EQ(cxx_import_decl, Directives[20].Kind);
-  EXPECT_EQ(pp_eof, Directives[21].Kind);
+  EXPECT_EQ(pp_pragma_system_header, Directives[21].Kind);
+  EXPECT_EQ(pp_eof, Directives[22].Kind);
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, EmptyHash) {
Index: clang/lib/Lex/Lexer.cpp
===
--- clang/lib/Lex/Lexer.cpp
+++ clang/lib/Lex/Lexer.cpp
@@ -4480,6 +4480,7 @@
 case pp_pragma_push_macro:
 case pp_pragma_pop_macro:
 case pp_pragma_include_alias:
+case pp_pragma_system_header:
 case pp_include_next:
 case decl_at_import:
 case cxx_module_decl:
Index: clang/lib/Lex/DependencyDirectivesScanner.cpp
===
--- clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -652,9 +652,22 @@
 return false;
   }
 
-  // #pragma clang.
-  if (!isNextIdentifierOrSkipLine("module", First, End))
+  FoundId = tryLexIdentifierOrSkipLine(First, End);
+  if (!FoundId)
 return false;
+  Id = *FoundId;
+
+  // #pragma clang system_header
+  if (Id == "system_header") {
+lexPPDirectiveBody(First, End);
+pushDirective(pp_pragma_system_header);
+return false;
+  }
+
+  if (Id != "module") {
+skipLine(First, End);
+return false;
+  }
 
   // #pragma clang module.
   if (!isNextIdentifierOrSkipLine("import", First, End))
Index: clang/include/clang/Lex/DependencyDirectivesScanner.h
===
--- clang/include/clang/Lex/DependencyDirectivesScanner.h
+++ clang/include/clang/Lex/DependencyDirectivesScanner.h
@@ -68,6 +68,7 @@
   pp_pragma_push_macro,
   pp_pragma_pop_macro,
   pp_pragma_include_alias,
+  pp_pragma_system_header,
   pp_include_next,
   pp_if,
   pp_ifdef,


Index: clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -90,7 +90,8 @@
"#pragma pop_macro(A)\n"
"#pragma include_alias(, )\n"
"export module m;\n"
-   "import m;\n",
+   "import m;\n"
+   "#pragma clang system_header\n",
Out, Tokens, Directives));
   EXPECT_EQ(pp_define, Directives[0].Kind);
   EXPECT_EQ(pp_undef, Directives[1].Kind);
@@ -113,7 +114,8 @@
   EXPECT_EQ(pp_pragma_include_alias, Directives[18].Kind);
   EXPECT_EQ(cxx_export_module_decl, Directives[19].Kind);
   EXPECT_EQ(cxx_import_decl, Directives[20].Kind);
-  EXPECT_EQ(pp_eof, Directives[21].Kind);
+  EXPECT_EQ(pp_pragma_system_header, Directives[21].Kind);
+  EXPECT_EQ(pp_eof, Directives[22].Kind);
 }
 
 TEST(MinimizeSourceToDependencyDirectivesTest, EmptyHash) {
Index: clang/lib/Lex/Lexer.cpp
===

[clang] b210ebe - Fix test bot breakage from 9bb28a18d962e8f6e3fa8f48bd2c6dc183154d26

2023-05-03 Thread Aaron Ballman via cfe-commits


Author: Aaron Ballman
Date: 2023-05-03T15:18:48-04:00
New Revision: b210ebe5c545ab566488d303788b1d12a91fc767

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

LOG: Fix test bot breakage from 9bb28a18d962e8f6e3fa8f48bd2c6dc183154d26

This addresses the issue found by:
https://lab.llvm.org/buildbot/#/builders/197/builds/4783
https://lab.llvm.org/buildbot/#/builders/188/builds/29201

Added: 


Modified: 
clang/test/CodeGen/nullptr.c

Removed: 




diff  --git a/clang/test/CodeGen/nullptr.c b/clang/test/CodeGen/nullptr.c
index 0f541b723339..59ec0fd1906d 100644
--- a/clang/test/CodeGen/nullptr.c
+++ b/clang/test/CodeGen/nullptr.c
@@ -53,7 +53,7 @@ void test() {
   // CHECK: store ptr null, ptr %[[nullptr_t_from_int]], align 8
 
   // Calls
-  // CHECK: call void @bool_func(i1 noundef zeroext false)
+  // CHECK: call void @bool_func(i1 noundef {{zeroext?}} false)
   // CHECK: call void @nullptr_func(ptr null)
   // CHECK: call void @nullptr_func(ptr null)
   // CHECK: call void @nullptr_func(ptr null)



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

[PATCH] D149693: [clang][deps] Make clang-scan-deps write modules in raw format

2023-05-03 Thread Ben Langmuir via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8fe8d69ddf88: [clang][deps] Make clang-scan-deps write 
modules in raw format (authored by benlangmuir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149693

Files:
  clang-tools-extra/clangd/Compiler.cpp
  clang/include/clang/CodeGen/ObjectFilePCHContainerOperations.h
  clang/include/clang/Serialization/PCHContainerOperations.h
  clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Serialization/PCHContainerOperations.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/test/ClangScanDeps/module-format.c
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/Indexing.cpp

Index: clang/tools/libclang/Indexing.cpp
===
--- clang/tools/libclang/Indexing.cpp
+++ clang/tools/libclang/Indexing.cpp
@@ -552,7 +552,7 @@
 
   // Make sure to use the raw module format.
   CInvok->getHeaderSearchOpts().ModuleFormat = std::string(
-  CXXIdx->getPCHContainerOperations()->getRawReader().getFormat());
+  CXXIdx->getPCHContainerOperations()->getRawReader().getFormats().front());
 
   auto Unit = ASTUnit::create(CInvok, Diags, CaptureDiagnostics,
   /*UserFilesAreVolatile=*/true);
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -3964,7 +3964,7 @@
   TUKind, CacheCodeCompletionResults, IncludeBriefCommentsInCodeCompletion,
   /*AllowPCHWithCompilerErrors=*/true, SkipFunctionBodies, SingleFileParse,
   /*UserFilesAreVolatile=*/true, ForSerialization, RetainExcludedCB,
-  CXXIdx->getPCHContainerOperations()->getRawReader().getFormat(),
+  CXXIdx->getPCHContainerOperations()->getRawReader().getFormats().front(),
   ));
 
   // Early failures in LoadFromCommandLine may return with ErrUnit unset.
Index: clang/test/ClangScanDeps/module-format.c
===
--- /dev/null
+++ clang/test/ClangScanDeps/module-format.c
@@ -0,0 +1,64 @@
+// Check that the scanner produces raw ast files, even when builds produce the
+// obj format, and that the scanner can read obj format from PCH and modules
+// imported by PCH.
+
+// Unsupported on AIX because we don't support the requisite "__clangast"
+// section in XCOFF yet.
+// UNSUPPORTED: target={{.*}}-aix{{.*}}
+
+// REQUIRES: shell
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: cp %S/Inputs/modules-pch/* %t
+
+// Scan dependencies of the PCH:
+//
+// RUN: rm -f %t/cdb_pch.json
+// RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-pch/cdb_pch.json > %t/cdb_pch.json
+// RUN: clang-scan-deps -compilation-database %t/cdb_pch.json -format experimental-full \
+// RUN:   -module-files-dir %t/build > %t/result_pch.json
+
+// Explicitly build the PCH:
+//
+// RUN: %deps-to-rsp %t/result_pch.json --module-name=ModCommon1 > %t/mod_common_1.cc1.rsp
+// RUN: %deps-to-rsp %t/result_pch.json --module-name=ModCommon2 > %t/mod_common_2.cc1.rsp
+// RUN: %deps-to-rsp %t/result_pch.json --module-name=ModPCH > %t/mod_pch.cc1.rsp
+// RUN: %deps-to-rsp %t/result_pch.json --tu-index=0 > %t/pch.rsp
+//
+// RUN: %clang @%t/mod_common_1.cc1.rsp
+// RUN: %clang @%t/mod_common_2.cc1.rsp
+// RUN: %clang @%t/mod_pch.cc1.rsp
+// RUN: %clang @%t/pch.rsp
+
+// Scan dependencies of the TU:
+//
+// RUN: rm -f %t/cdb_tu.json
+// RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-pch/cdb_tu.json > %t/cdb_tu.json
+// RUN: clang-scan-deps -compilation-database %t/cdb_tu.json -format experimental-full \
+// RUN:   -module-files-dir %t/build > %t/result_tu.json
+
+// Explicitly build the TU:
+//
+// RUN: %deps-to-rsp %t/result_tu.json --module-name=ModTU > %t/mod_tu.cc1.rsp
+// RUN: %deps-to-rsp %t/result_tu.json --tu-index=0 > %t/tu.rsp
+//
+// RUN: %clang @%t/mod_tu.cc1.rsp
+// RUN: %clang @%t/tu.rsp
+
+// Check the module format for scanner modules:
+//
+// RUN: find %t/cache -name "*.pcm" -exec %clang_cc1 -module-file-info "{}" ";" | FileCheck %s -check-prefix=SCAN
+// SCAN: Module format: raw
+// SCAN: Module format: raw
+// SCAN: Module format: raw
+// SCAN: Module format: raw
+
+// Check the module format for built modules:
+//
+// RUN: find %t/build -name "*.pcm" -exec %clang_cc1 -module-file-info "{}" ";" | FileCheck %s -check-prefix=BUILD
+// BUILD: Module format: obj
+// BUILD: Module format: obj
+// BUILD: Module format: obj
+// BUILD: Module format: obj
+
+// FIXME: check pch format as well; -module-file-info does not work with a PCH
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++

[clang-tools-extra] 8fe8d69 - [clang][deps] Make clang-scan-deps write modules in raw format

2023-05-03 Thread Ben Langmuir via cfe-commits


Author: Ben Langmuir
Date: 2023-05-03T12:07:46-07:00
New Revision: 8fe8d69ddf881db70cb8df31e614a06e633e2c5f

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

LOG: [clang][deps] Make clang-scan-deps write modules in raw format

We have no use for debug info for the scanner modules, and writing raw
ast files speeds up scanning ~15% in some cases. Note that the compile
commands produced by the scanner will still build the obj format (if
requested), and the scanner can *read* obj format pcms, e.g. from a PCH.

rdar://108807592

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

Added: 
clang/test/ClangScanDeps/module-format.c

Modified: 
clang-tools-extra/clangd/Compiler.cpp
clang/include/clang/CodeGen/ObjectFilePCHContainerOperations.h
clang/include/clang/Serialization/PCHContainerOperations.h
clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
clang/lib/Frontend/ASTUnit.cpp
clang/lib/Serialization/PCHContainerOperations.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/tools/libclang/CIndex.cpp
clang/tools/libclang/Indexing.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Compiler.cpp 
b/clang-tools-extra/clangd/Compiler.cpp
index f242db44b4317..7b93353cab1f3 100644
--- a/clang-tools-extra/clangd/Compiler.cpp
+++ b/clang-tools-extra/clangd/Compiler.cpp
@@ -73,7 +73,7 @@ void disableUnsupportedOptions(CompilerInvocation ) {
   // Always default to raw container format as clangd doesn't registry any 
other
   // and clang dies when faced with unknown formats.
   CI.getHeaderSearchOpts().ModuleFormat =
-  PCHContainerOperations().getRawReader().getFormat().str();
+  PCHContainerOperations().getRawReader().getFormats().front().str();
 
   CI.getFrontendOpts().Plugins.clear();
   CI.getFrontendOpts().AddPluginActions.clear();

diff  --git a/clang/include/clang/CodeGen/ObjectFilePCHContainerOperations.h 
b/clang/include/clang/CodeGen/ObjectFilePCHContainerOperations.h
index c13e052149d95..7a02d8725885a 100644
--- a/clang/include/clang/CodeGen/ObjectFilePCHContainerOperations.h
+++ b/clang/include/clang/CodeGen/ObjectFilePCHContainerOperations.h
@@ -32,7 +32,7 @@ class ObjectFilePCHContainerWriter : public 
PCHContainerWriter {
 /// A PCHContainerReader implementation that uses LLVM to
 /// wraps Clang modules inside a COFF, ELF, or Mach-O container.
 class ObjectFilePCHContainerReader : public PCHContainerReader {
-  StringRef getFormat() const override { return "obj"; }
+  ArrayRef getFormats() const override;
 
   /// Returns the serialized AST inside the PCH container Buffer.
   StringRef ExtractPCH(llvm::MemoryBufferRef Buffer) const override;

diff  --git a/clang/include/clang/Serialization/PCHContainerOperations.h 
b/clang/include/clang/Serialization/PCHContainerOperations.h
index 9f9700a418a9c..be10feb5e351c 100644
--- a/clang/include/clang/Serialization/PCHContainerOperations.h
+++ b/clang/include/clang/Serialization/PCHContainerOperations.h
@@ -56,7 +56,7 @@ class PCHContainerReader {
 public:
   virtual ~PCHContainerReader() = 0;
   /// Equivalent to the format passed to -fmodule-format=
-  virtual llvm::StringRef getFormat() const = 0;
+  virtual llvm::ArrayRef getFormats() const = 0;
 
   /// Returns the serialized AST inside the PCH container Buffer.
   virtual llvm::StringRef ExtractPCH(llvm::MemoryBufferRef Buffer) const = 0;
@@ -78,8 +78,7 @@ class RawPCHContainerWriter : public PCHContainerWriter {
 
 /// Implements read operations for a raw pass-through PCH container.
 class RawPCHContainerReader : public PCHContainerReader {
-  llvm::StringRef getFormat() const override { return "raw"; }
-
+  llvm::ArrayRef getFormats() const override;
   /// Simply returns the buffer contained in Buffer.
   llvm::StringRef ExtractPCH(llvm::MemoryBufferRef Buffer) const override;
 };
@@ -87,7 +86,9 @@ class RawPCHContainerReader : public PCHContainerReader {
 /// A registry of PCHContainerWriter and -Reader objects for 
diff erent formats.
 class PCHContainerOperations {
   llvm::StringMap> Writers;
-  llvm::StringMap> Readers;
+  llvm::StringMap Readers;
+  llvm::SmallVector> OwnedReaders;
+
 public:
   /// Automatically registers a RawPCHContainerWriter and
   /// RawPCHContainerReader.
@@ -96,13 +97,17 @@ class PCHContainerOperations {
 Writers[Writer->getFormat()] = std::move(Writer);
   }
   void registerReader(std::unique_ptr Reader) {
-Readers[Reader->getFormat()] = std::move(Reader);
+assert(!Reader->getFormats().empty() &&
+   "PCHContainerReader must handle >=1 format");
+for (llvm::StringRef Fmt : Reader->getFormats())
+  Readers[Fmt] = Reader.get();
+OwnedReaders.push_back(std::move(Reader));
   }
   const PCHContainerWriter *getWriterOrNull(llvm::StringRef

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

2023-05-03 Thread NAKAMURA Takumi via Phabricator via cfe-commits

chapuni added a comment.

In D148767#4312692 , @barannikov88 
wrote:

> Sorry, I don't follow. They may theoretically depend on anything. Why 
> CodeGen/CodeGenTypes, specifically?
> If they don't require it, why add it?

I added deps pessimistically, "This depends on CodeGenTypes if LowLevelType.h 
is included".

In D148767#4315667 , @jobnoorman 
wrote:

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

Sorry for inconvenience. Could you add `CodeGenTypes` in `LINK_COMPONENTS` 
please?

In D148767#4315757 , @vzakhari wrote:

> I can add `CodeGenTypes` link component in 
> `flang/lib/Optimizer/Transforms/CMakeLists.txt`, but I am worried about the 
> comment in `llvm/lib/CodeGen/CMakeLists.txt`:
>
>   # Be careful to append deps on this, since Targets' tablegens depend on 
> this.
>   add_llvm_component_library(LLVMCodeGenTypes
>
> I am not sure whether I need to be careful about adding dependencies onto 
> `LLVMCodeGenTypes` (as I am planning to do) or about adding dependencies for 
> `LLVMCodeGenTypes` target in `llvm/lib/CodeGen/CMakeLists.txt` :)

The latter. Excuse my wrong wording.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148767

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

[PATCH] D149776: Re-land "[AMDGPU] Define data layout entries for buffers""

2023-05-03 Thread Krzysztof Drewniak via Phabricator via cfe-commits

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

Re-land D145441  with data layout upgrade 
code fixed to not break OpenMP.

This reverts commit 3f2fbe92d0f40bcb46db7636db9ec3f7e7899b27 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D149776

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

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

2023-05-03 Thread Aaron Ballman 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 rG9bb28a18d962: [C2x] Update nullptr 
implementation based on CD comments (authored by aaron.ballman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148800

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

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

[clang] 9bb28a1 - [C2x] Update 'nullptr' implementation based on CD comments

2023-05-03 Thread Aaron Ballman via cfe-commits


Author: Aaron Ballman
Date: 2023-05-03T14:50:15-04:00
New Revision: 9bb28a18d962e8f6e3fa8f48bd2c6dc183154d26

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

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

We filed some CD ballot comments which WG14 considered during the
ballot comment resolution meetings in Jan and Feb 2023, and this
updates our implementation based on the decisions reached. Those
decisions were (paraphrased for brevity):

US 9-034 (REJECTED)
  allow (void *)nullptr to be a null pointer constant
US 10-035 (ACCEPTED)
  accept the following code, as in C++:
  void func(nullptr_t); func(0);
US 22-058 (REJECTED)
  accept the following code, as in C++:
  nullptr_t val; (void)(1 ? val : 0); (void)(1 ? nullptr : 0);
US 23-062 (REJECTED)
  reject the following code, as in C++:
  nullptr_t val; bool b1 = val; bool b2 = nullptr;
US 24-061 (ACCEPTED)
  accept the following code, as in C++:
  nullptr_t val; val = 0;
US 21-068 (ACCEPTED)
  accept the following code, as in C++:
  (nullptr_t)nullptr;
GB-071 (ACCEPTED)
  accept the following code, as in C++:
  nullptr_t val; (void)(val == nullptr);

This patch updates the implementation as appropriate, but is primarily
focused around US 10-035, US 24-061, and US 23-062 in terms of
functional changes.

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

Added: 
clang/test/CodeGen/nullptr.c

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

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8d0a9c96a9579..ff391c6cb3b99 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -139,6 +139,25 @@ C2x Feature Support
   removed, as this is no longer a GNU extension but a C2x extension. You can
   use ``-Wno-c2x-extensions`` to silence the extension warning instead.
 
+- Updated the implementation of
+  `WG14 N3042 `_
+  based on decisions reached during the WG14 CD Ballot Resolution meetings held
+  in Jan and Feb 2023. This should complete the implementation of ``nullptr``
+  and ``nullptr_t`` in C. The specific changes are:
+
+  .. code-block:: c
+
+void func(nullptr_t);
+func(0); // Previously required to be rejected, is now accepted.
+func((void *)0); // Previously required to be rejected, is now accepted.
+
+nullptr_t val;
+val = 0; // Previously required to be rejected, is now accepted.
+val = (void *)0; // Previously required to be rejected, is now accepted.
+
+bool b = nullptr; // Was incorrectly rejected by Clang, is now accepted.
+
+
 Non-comprehensive list of changes in this release
 -
 - Clang now saves the address of ABI-indirect function parameters on the stack,

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 8789e4c3cb25f..048f38cb195e5 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -10118,6 +10118,15 @@ Sema::CheckAssignmentConstraints(QualType LHSType, 
ExprResult ,
 return Incompatible;
   }
 
+  // Conversion to nullptr_t (C2x only)
+  if (getLangOpts().C2x && LHSType->isNullPtrType() &&
+  RHS.get()->isNullPointerConstant(Context,
+   Expr::NPC_ValueDependentIsNull)) {
+// null -> nullptr_t
+Kind = CK_NullToPointer;
+return Compatible;
+  }
+
   // Conversions from pointers that are not covered by the above.
   if (isa(RHSType)) {
 // T* -> _Bool
@@ -10335,12 +10344,13 @@ Sema::CheckSingleAssignmentConstraints(QualType 
LHSType, ExprResult ,
   QualType LHSTypeAfterConversion = LHSType.getAtomicUnqualifiedType();
 
   // C99 6.5.16.1p1: the left operand is a pointer and the right is
-  // a null pointer constant.
+  // a null pointer constant or its type is nullptr_t;.
   if ((LHSTypeAfterConversion->isPointerType() ||
LHSTypeAfterConversion->isObjCObjectPointerType() ||
LHSTypeAfterConversion->isBlockPointerType()) &&
-  RHS.get()->isNullPointerConstant(Context,
-   Expr::NPC_ValueDependentIsNull)) {
+  ((getLangOpts().C2x && RHS.get()->getType()->isNullPtrType()) ||
+   RHS.get()->isNullPointerConstant(Context,
+Expr::NPC_ValueDependentIsNull))) {
 if (Diagnose || ConvertRHS) {
   CastKind Kind;
   CXXCastPath Path;
@@ -10351,6 +10361,26 @@ Sema::CheckSingleAssignmentConstraints(QualType 
LHSType, ExprResult ,
 }
 return Compatible;
   }
+  // C2x 6.5.16.1p1: the left operand has type atomic, qualified, or
+  // unqualified bool, and the right

[PATCH] D148924: [clang] Show error if defaulted comparions operator function is volatile or has ref-qualifier &&.

2023-05-03 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added reviewers: hubert.reinterpretcast, shafik, rsmith.
aaron.ballman added a subscriber: rsmith.
aaron.ballman added a comment.

Did you verify that the rest of P2002R1 was implemented correctly? There are a 
lot more changes in that paper than just this tiny bit, so if we're not certain 
this finishes things, we shouldn't mark the status as complete (but if we're 
figuring out what's missing, we should capture those details somewhere so we 
know what's left to do)

https://github.com/llvm/llvm-project/commit/b74a381296eef048911bb22dc4eb2d3598460470
 is what marked the status as partial, but there's no explanation as to why it 
was partial. Perhaps @rsmith remembers what was left to be done there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148924

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

[PATCH] D148654: Modify BoundsSan to improve debuggability

2023-05-03 Thread Oskar Wirga via Phabricator via cfe-commits

oskarwirga updated this revision to Diff 519190.
oskarwirga edited the summary of this revision.
oskarwirga added a comment.

Turns out the lowering of ubsantrap() to a single instruction resulted in 
binaries that did NOT have nonmerged traps, so this is going back to what we 
had before. I also added tests to show that the trap gets preserved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148654

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGen/bounds-checking.c
  llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp

Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -47,6 +47,11 @@
 using namespace clang;
 using namespace CodeGen;
 
+// Experiment to make sanitizers easier to debug
+static llvm::cl::opt ClSanitizeDebugDeoptimization(
+"sanitizer-de-opt-traps", llvm::cl::Optional,
+llvm::cl::desc("Deoptimize traps for sanitizers"), llvm::cl::init(false));
+
 //======//
 //Miscellaneous Helper Methods
 //======//
@@ -3568,22 +3573,16 @@
 
 void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked,
 SanitizerHandler CheckHandlerID) {
-  llvm::BasicBlock *Cont = createBasicBlock("cont");
-
-  // If we're optimizing, collapse all calls to trap down to just one per
-  // check-type per function to save on code size.
-  if (TrapBBs.size() <= CheckHandlerID)
-TrapBBs.resize(CheckHandlerID + 1);
-  llvm::BasicBlock * = TrapBBs[CheckHandlerID];
+  if (ClSanitizeDebugDeoptimization) {
+llvm::BasicBlock *Cont = createBasicBlock("cont");
 
-  if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB) {
-TrapBB = createBasicBlock("trap");
+llvm::BasicBlock *TrapBB = createBasicBlock("trap");
 Builder.CreateCondBr(Checked, Cont, TrapBB);
 EmitBlock(TrapBB);
 
-llvm::CallInst *TrapCall =
-Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::ubsantrap),
-   llvm::ConstantInt::get(CGM.Int8Ty, CheckHandlerID));
+llvm::CallInst *TrapCall = Builder.CreateCall(
+CGM.getIntrinsic(llvm::Intrinsic::ubsantrap),
+llvm::ConstantInt::get(CGM.Int8Ty, TrapBB->getParent()->size()));
 
 if (!CGM.getCodeGenOpts().TrapFuncName.empty()) {
   auto A = llvm::Attribute::get(getLLVMContext(), "trap-func-name",
@@ -3593,16 +3592,45 @@
 TrapCall->setDoesNotReturn();
 TrapCall->setDoesNotThrow();
 Builder.CreateUnreachable();
+
+EmitBlock(Cont);
   } else {
-auto Call = TrapBB->begin();
-assert(isa(Call) && "Expected call in trap BB");
+llvm::BasicBlock *Cont = createBasicBlock("cont");
+
+// If we're optimizing, collapse all calls to trap down to just one per
+// check-type per function to save on code size.
+if (TrapBBs.size() <= CheckHandlerID)
+  TrapBBs.resize(CheckHandlerID + 1);
+llvm::BasicBlock * = TrapBBs[CheckHandlerID];
+
+if (!CGM.getCodeGenOpts().OptimizationLevel || !TrapBB) {
+  TrapBB = createBasicBlock("trap");
+  Builder.CreateCondBr(Checked, Cont, TrapBB);
+  EmitBlock(TrapBB);
+
+  llvm::CallInst *TrapCall = Builder.CreateCall(
+  CGM.getIntrinsic(llvm::Intrinsic::ubsantrap),
+  llvm::ConstantInt::get(CGM.Int8Ty, CheckHandlerID));
+
+  if (!CGM.getCodeGenOpts().TrapFuncName.empty()) {
+auto A = llvm::Attribute::get(getLLVMContext(), "trap-func-name",
+  CGM.getCodeGenOpts().TrapFuncName);
+TrapCall->addFnAttr(A);
+  }
+  TrapCall->setDoesNotReturn();
+  TrapCall->setDoesNotThrow();
+  Builder.CreateUnreachable();
+} else {
+  auto Call = TrapBB->begin();
+  assert(isa(Call) && "Expected call in trap BB");
 
-Call->applyMergedLocation(Call->getDebugLoc(),
-  Builder.getCurrentDebugLocation());
-Builder.CreateCondBr(Checked, Cont, TrapBB);
-  }
+  Call->applyMergedLocation(Call->getDebugLoc(),
+Builder.getCurrentDebugLocation());
+  Builder.CreateCondBr(Checked, Cont, TrapBB);
+}
 
-  EmitBlock(Cont);
+EmitBlock(Cont);
+  }
 }
 
 llvm::CallInst *CodeGenFunction::EmitTrapCall(llvm::Intrinsic::ID IntrID) {
Index: clang/test/CodeGen/bounds-checking.c
===
--- clang/test/CodeGen/bounds-checking.c
+++ clang/test/CodeGen/bounds-checking.c
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -fsanitize=local-bounds -emit-llvm -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
 // RUN: %clang_cc1 -fsanitize=array-bounds -O -fsanitize-trap=array-bounds -emit-llvm -triple x86_64-apple-darwin10 -DNO_DYNAMIC %s -o - | FileCheck %s

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

2023-05-03 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added inline comments.



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

MyDeveloperDay wrote:
> owenpan wrote:
> > tahonermann wrote:
> > > How about using an init capture instead? This will suffice to avoid one 
> > > of the copies but means that `InsertBraces` doesn't get set until the 
> > > lambda is invoked. I wouldn't expect that to matter though.
> > I'm not sure if it's worth the trouble, but if I really had to bother, I 
> > would do something like the above.
> Apart from perhaps the unnecessary copying from Expanded -> S.. doesn't this 
> bascially do the same without us having to remember to put Expanded back 
> afterwards? I don't see how using Expanded is any different from using S 
> (other than the copy)
> 
> ```
> if (Style.InsertBraces) {
>   FormatStyle S = Expanded;
>   S.InsertBraces = true;
>   Passes.emplace_back([&](const Environment ) {
> return BracesInserter(Env, S).process(/*SkipAnnotation=*/true);
>   });
> }
> ```
> I'm not sure if it's worth the trouble, but if I really had to bother, I 
> would do something like the above.

That wouldn't work, since the pass is only executed afterwards.



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

HazardyKnusperkeks wrote:
> MyDeveloperDay wrote:
> > owenpan wrote:
> > > tahonermann wrote:
> > > > How about using an init capture instead? This will suffice to avoid one 
> > > > of the copies but means that `InsertBraces` doesn't get set until the 
> > > > lambda is invoked. I wouldn't expect that to matter though.
> > > I'm not sure if it's worth the trouble, but if I really had to bother, I 
> > > would do something like the above.
> > Apart from perhaps the unnecessary copying from Expanded -> S.. doesn't 
> > this bascially do the same without us having to remember to put Expanded 
> > back afterwards? I don't see how using Expanded is any different from using 
> > S (other than the copy)
> > 
> > ```
> > if (Style.InsertBraces) {
> >   FormatStyle S = Expanded;
> >   S.InsertBraces = true;
> >   Passes.emplace_back([&](const Environment ) {
> > return BracesInserter(Env, S).process(/*SkipAnnotation=*/true);
> >   });
> > }
> > ```
> > I'm not sure if it's worth the trouble, but if I really had to bother, I 
> > would do something like the above.
> 
> That wouldn't work, since the pass is only executed afterwards.
> Apart from perhaps the unnecessary copying from Expanded -> S.. doesn't this 
> bascially do the same without us having to remember to put Expanded back 
> afterwards? I don't see how using Expanded is any different from using S 
> (other than the copy)
> 
> ```
> if (Style.InsertBraces) {
>   FormatStyle S = Expanded;
>   S.InsertBraces = true;
>   Passes.emplace_back([&](const Environment ) {
> return BracesInserter(Env, S).process(/*SkipAnnotation=*/true);
>   });
> }
> ```

That would have a dangling reference to S and most likely don't work as wished.



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

HazardyKnusperkeks wrote:
> HazardyKnusperkeks wrote:
> > MyDeveloperDay wrote:
> > > owenpan wrote:
> > > > tahonermann wrote:
> > > > > How about using an init capture instead? This will suffice to avoid 
> > > > > one of the copies but means that `InsertBraces` doesn't get set until 
> > > > > the lambda is invoked. I wouldn't expect that to matter though.
> > > > I'm not sure if it's worth the trouble, but if I really had to bother, 
> > > > I would do something like the above.
> > > Apart from perhaps the unnecessary copying from Expanded -> S.. doesn't 
> > > this bascially do the same without us having to remember to put Expanded 
> > > back afterwards? I don't see how using Expanded is any different from 
> > > using S (other than the copy)
> > > 
> > > ```
> > > if (Style.InsertBraces) {
> > >   FormatStyle S = Expanded;
> > >   S.InsertBraces = true;
> > >   Passes.emplace_back([&](const Environment ) {
> > > return BracesInserter(Env, S).process(/*SkipAnnotation=*/true);
> > >   });
> > > }
> > > ```
> > > I'm not sure if it's worth the trouble, but if I really had to bother, I 
> > > would do something like the above.
> > 
> > That wouldn't work, since the pass is only executed afterwards.
> > Apart from perhaps the unnecessary copying from

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

2023-05-03 Thread Martin Böhme 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 rG791b0fd02668: [clang][dataflow] Change 
PruneTriviallyFalseEdges for building CFG (authored by kinu, committed by 
mboehme).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149640

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


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


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

[clang] 791b0fd - [clang][dataflow] Change PruneTriviallyFalseEdges for building CFG

2023-05-03 Thread Martin Braenne via cfe-commits


Author: Kinuko Yasuda
Date: 2023-05-03T18:42:15Z
New Revision: 791b0fd02668fd0fcf07788d4e16bb468434f4bf

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

LOG: [clang][dataflow] Change PruneTriviallyFalseEdges for building CFG

Keeping this false could end up with extra iterations on a lot of loops
that aren't real ones (e.g. they could be a do-while-false for macros),
and makes the analyses very slow.

This patch changes the default for
CFG::BuildOptions.PruneTriviallyFalseEdges to true to avoid it.

Reviewed By: ymandel, xazax.hun, gribozavr2

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp 
b/clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
index 6699a0fc9d79e..5520633da68ae 100644
--- a/clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
+++ b/clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
@@ -70,7 +70,7 @@ static llvm::BitVector findReachableBlocks(const CFG ) {
 llvm::Expected
 ControlFlowContext::build(const Decl *D, Stmt , ASTContext ) {
   CFG::BuildOptions Options;
-  Options.PruneTriviallyFalseEdges = false;
+  Options.PruneTriviallyFalseEdges = true;
   Options.AddImplicitDtors = true;
   Options.AddTemporaryDtors = true;
   Options.AddInitializers = true;

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 750d095af451a..c8161c8f40fc9 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2644,7 +2644,7 @@ TEST(TransferTest, VarDeclInDoWhile) {
 void target(int *Foo) {
   do {
 int Bar = *Foo;
-  } while (true);
+  } while (false);
   (void)0;
   /*[[p]]*/
 }
@@ -2675,6 +2675,24 @@ TEST(TransferTest, VarDeclInDoWhile) {
   });
 }
 
+TEST(TransferTest, UnreachableAfterWhileTrue) {
+  std::string Code = R"(
+void target() {
+  while (true) {}
+  (void)0;
+  /*[[p]]*/
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> ,
+ ASTContext ) {
+// The node after the while-true is pruned because it is trivially
+// known to be unreachable.
+ASSERT_TRUE(Results.empty());
+  });
+}
+
 TEST(TransferTest, AggregateInitialization) {
   std::string BracesCode = R"(
 struct A {



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

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

2023-05-03 Thread Peter Kasting via Phabricator via cfe-commits

pkasting added a comment.

Thanks @rsmith; I don't have commit access, so can you land the updated patch 
for me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149713

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

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

2023-05-03 Thread Peter Kasting via Phabricator via cfe-commits

pkasting updated this revision to Diff 519188.
pkasting added a comment.

Update test per review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149713

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/warn-exit-time-destructors.cpp
  clang/test/SemaCXX/warn-global-constructors.cpp


Index: clang/test/SemaCXX/warn-global-constructors.cpp
===
--- clang/test/SemaCXX/warn-global-constructors.cpp
+++ clang/test/SemaCXX/warn-global-constructors.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wglobal-constructors %s -verify
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wglobal-constructors %s 
-verify=expected,cxx11
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -Wglobal-constructors %s 
-verify=expected
 
 int opaque_int();
 
@@ -145,3 +146,22 @@
   const HasUnnamedBitfield nonConstexprConst{1}; // expected-warning {{global 
constructor}}
   HasUnnamedBitfield nonConstexprMutable{1}; // expected-warning {{global 
constructor}}
 }
+
+namespace test7 {
+#if __cplusplus >= 202002L
+#define CPP20_CONSTEXPR constexpr
+#else
+#define CPP20_CONSTEXPR
+#endif
+  struct S {
+CPP20_CONSTEXPR ~S() {}
+  };
+  S s; // cxx11-warning {{global destructor}}
+
+  struct T {
+CPP20_CONSTEXPR ~T() { if (b) {} }
+bool b;
+  };
+  T t; // expected-warning {{global destructor}}
+#undef CPP20_CONSTEXPR
+}
Index: clang/test/SemaCXX/warn-exit-time-destructors.cpp
===
--- clang/test/SemaCXX/warn-exit-time-destructors.cpp
+++ clang/test/SemaCXX/warn-exit-time-destructors.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wexit-time-destructors %s -verify
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wexit-time-destructors %s 
-verify=expected,cxx11
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -Wexit-time-destructors %s 
-verify=expected
 
 namespace test1 {
   struct A { ~A(); };
@@ -48,3 +49,22 @@
 struct A { ~A(); };
 [[clang::no_destroy]] A a; // no warning
 }
+
+namespace test5 {
+#if __cplusplus >= 202002L
+#define CPP20_CONSTEXPR constexpr
+#else
+#define CPP20_CONSTEXPR
+#endif
+  struct S {
+CPP20_CONSTEXPR ~S() {}
+  };
+  S s; // cxx11-warning {{exit-time destructor}}
+
+  struct T {
+CPP20_CONSTEXPR ~T() { if (b) {} }
+bool b;
+  };
+  T t; // expected-warning {{exit-time destructor}}
+#undef CPP20_CONSTEXPR
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -15718,7 +15718,8 @@
 }
   }
 
-  if (!VD->hasGlobalStorage()) return;
+  if (!VD->hasGlobalStorage() || !VD->needsDestruction(Context))
+return;
 
   // Emit warning for non-trivial dtor in global scope (a real global,
   // class-static, function-static).


Index: clang/test/SemaCXX/warn-global-constructors.cpp
===
--- clang/test/SemaCXX/warn-global-constructors.cpp
+++ clang/test/SemaCXX/warn-global-constructors.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wglobal-constructors %s -verify
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wglobal-constructors %s -verify=expected,cxx11
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -Wglobal-constructors %s -verify=expected
 
 int opaque_int();
 
@@ -145,3 +146,22 @@
   const HasUnnamedBitfield nonConstexprConst{1}; // expected-warning {{global constructor}}
   HasUnnamedBitfield nonConstexprMutable{1}; // expected-warning {{global constructor}}
 }
+
+namespace test7 {
+#if __cplusplus >= 202002L
+#define CPP20_CONSTEXPR constexpr
+#else
+#define CPP20_CONSTEXPR
+#endif
+  struct S {
+CPP20_CONSTEXPR ~S() {}
+  };
+  S s; // cxx11-warning {{global destructor}}
+
+  struct T {
+CPP20_CONSTEXPR ~T() { if (b) {} }
+bool b;
+  };
+  T t; // expected-warning {{global destructor}}
+#undef CPP20_CONSTEXPR
+}
Index: clang/test/SemaCXX/warn-exit-time-destructors.cpp
===
--- clang/test/SemaCXX/warn-exit-time-destructors.cpp
+++ clang/test/SemaCXX/warn-exit-time-destructors.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wexit-time-destructors %s -verify
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wexit-time-destructors %s -verify=expected,cxx11
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -Wexit-time-destructors %s -verify=expected
 
 namespace test1 {
   struct A { ~A(); };
@@ -48,3 +49,22 @@
 struct A { ~A(); };
 [[clang::no_destroy]] A a; // no warning
 }
+
+namespace test5 {
+#if __cplusplus >= 202002L
+#define CPP20_CONSTEXPR constexpr
+#else
+#define CPP20_CONSTEXPR
+#endif
+  struct S {
+CPP20_CONSTEXPR ~S() {}
+  };
+  S s; // cxx11-warning {{exit-time destructor}}
+
+  struct T {
+CPP20_CONSTEXPR ~T() { if (b) {} }
+

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

2023-05-03 Thread Arthur Eubanks via Phabricator via cfe-commits

aeubanks added a comment.

In D143624#4315508 , @nikic wrote:

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

That invariant shouldn't matter if we're not using heuristics to inline. The 
normal heuristic-based inliner will still work on simplified callees, but now 
with the additional benefit of seeing the state of an SCC where there may be 
alwaysinline calls after the inlinings that must happen having happened.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143624

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

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

2023-05-03 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet added a comment.

Sorry I was trying to give some brief idea about what it might look like in 
`Implementation Concerns` section above, but let me give some more details;

I think we can just change the signature for PreambleParsedCallback 

 to pass along refcounted objects. forgot to mention in the first comment, but 
we should also change the CanonicalIncludes to be a shared_ptr so that it can 
outlive the PreambleData. We should invoke the callback inside buildPreamble 

 after a successful build. Afterwards we should also change the signature for 
onPreambleAST 

 to take AST, PP and CanonicalIncludes as ref-counted objects again and 
`PreambleThread::build` should just forward objects received from 
`PreambleParsedCallback`. Afterwards inside the 
UpdateIndexCallbacks::onPreambleAST 

 we can just invoke indexing on `Tasks` if it's present or synchronously in the 
absence of it.

Does that make sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148088

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

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

2023-05-03 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D149193#4316293 , @dblaikie wrote:

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

This is a long-known problem that features with auxiliary output files go to 
strange directories (temporary directory, usually `/tmp`).
Among these options, the prominent ones are `-gsplit-dwarf` and `-ftime-trace` 
(`clang -ftime-trace a.c b.c` 
https://github.com/llvm/llvm-project/issues/57285).

By just implementing `-dumpdir `, we don't diverge in "a bunch ways". If GCC 
has fixed their `-o d/a.out` bug, our behavior should match theirs.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149193

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

[PATCH] D149693: [clang][deps] Make clang-scan-deps write modules in raw format

2023-05-03 Thread Michael Spencer via Phabricator via cfe-commits

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

lgtm.


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

https://reviews.llvm.org/D149693

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

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

2023-05-03 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added inline comments.



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

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149647

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

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

2023-05-03 Thread Peter Smith via Phabricator via cfe-commits

peter.smith added a comment.

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

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148665

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

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

2023-05-03 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149647

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

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

2023-05-03 Thread David Blaikie via Phabricator via cfe-commits

dblaikie added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149193

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

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

2023-05-03 Thread Soumi Manna via Phabricator via cfe-commits

Manna added a comment.

Thanks @erichkeane for response.

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

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

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

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

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

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

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

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

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

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

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

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

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

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

In D149647#4316169 , @erichkeane 
wrote:

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149647

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

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

2023-05-03 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D147626#4316190 , @efriedma wrote:

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

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147626

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

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

2023-05-03 Thread Eli Friedman via Phabricator via cfe-commits

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

LGTM


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

https://reviews.llvm.org/D148800

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

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

2023-05-03 Thread Eli Friedman via Phabricator via cfe-commits

efriedma added a comment.

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147626

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

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

2023-05-03 Thread Shafik Yaghmour via Phabricator via cfe-commits

shafik added a subscriber: rsmith.
shafik added inline comments.



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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149149

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

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

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

donat.nagy added a comment.

> It's gonna be bumpy.

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

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149259

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

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

2023-05-03 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added a comment.

In D149647#4316134 , @MyDeveloperDay 
wrote:

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

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149647

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

[PATCH] D149650: Give NullabilityKind a printing operator<

2023-05-03 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D149650#4315211 , @sammccall wrote:

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

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

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

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

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

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149650

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

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

2023-05-03 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149647

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

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

2023-05-03 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

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

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


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

https://reviews.llvm.org/D149643

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

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

2023-05-03 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D147626#4315283 , @Fznamznon wrote:

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

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

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147626

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

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

2023-05-03 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

In D149695#4315194 , @hans wrote:

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

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149695

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

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

2023-05-03 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added subscribers: v.g.vassilev, junaire, rsmith.
aaron.ballman added a comment.

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

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

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

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

Repository:
rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149733

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

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

2023-05-03 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

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

LGTM with the extra test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149713

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

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

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8c22cbea87be: [analyzer] ArrayBoundCheckerV2: suppress false 
positives from ctype macros (authored by donat.nagy).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149460

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


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


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

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

2023-05-03 Thread Richard Smith - zygoloid via Phabricator via cfe-commits

rsmith added a comment.

I think this would be an interesting test:

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

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149713

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

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

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


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

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

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

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

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

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

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

Added: 


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

Removed: 




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

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

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

2023-05-03 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman accepted this revision.
aaron.ballman added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149553

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

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

2023-05-03 Thread Giuliano Belinassi via Phabricator via cfe-commits

giulianobelinassi updated this revision to Diff 519131.
giulianobelinassi added a comment.

Incorporate some of Aron suggestions:

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141714

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

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

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

2023-05-03 Thread Alexander Shaposhnikov via Phabricator via cfe-commits

alexander-shaposhnikov added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

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

2023-05-03 Thread Zahira Ammarguellat via Phabricator via cfe-commits

zahiraam added a comment.

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


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

https://reviews.llvm.org/D146148

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

1 2 >

1 - 100 of 180 matches

Mail list logo