[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

In D80858#2177104 , @yaxunl wrote:

> In D80858#2171266 , @hliao wrote:
>
>> We may try to solve the issue without RDC firstly, where we don't need to 
>> change that static variable name (if the runtime maintains the device 
>> binaries correctly.) We only need to ensure the linker won't remove their 
>> symbols.
>
> This is essentially externalizing it in linker. However, it may not work.
>
> Consider a static device var was accessed in host code through 
> hipMemCpyToSymbol or hipMemCpyFromSymbol. In device code, this static var may 
> be initialized through host code, or its final value may be read by host code 
> after kernel execution. The existence of these operations mean that this 
> static device variable `is` actually having `external` linkage, instead of 
> `internal` linkage, since it is accessed by external modules. Fail to reflect 
> this truth in IR will cause optimization passes to make incorrect assumptions 
> about this variable and perform incorrect optimizations on it. e.g. the llvm 
> passes can assume the value in this static var never changes, or its final 
> value will not be used, then the llvm passes may simply remove it. Marking it 
> as `used` will not solve the issue, since the llvm passes may still assume 
> its value never changes after initialization, whereas in reality it may be 
> changed by hipMemCpyToSymbol before kernel execution.

That's what I mean by "externalizing" that variable. In fact, `nvcc` doesn't 
the same thing and generate `global` symbol in PTX.

To get file-scope static device variables support, we have to 2 separate issues:

- externalize variable to ensure backend/linker preserve their symbols for 
runtime to query. The major concern is that we'd better check whether a static 
device variable is referenced using host APIs. Otherwise, aggressive IPO 
optimizations may not be able to be applied. As those are file-scope static 
variables, roughly checking their reference on the host side should be doable.
- rename device static variables. Such renaming is only necessary when RDC is 
enabled. Without RDC, there would be conflicting names and renaming is 
unnecessary. For renaming, relying on developer supplied CUID is not reliable 
and we should extend name mangler to include source name, digest, or whatever 
automatically managed by the compiler. If we support static variable access on 
the host side, we should not rely on developer supplied option to function 
correctly. Also, the mangled name should be demangled into a meaningful name if 
possible. I remembered GCC has special support for names from anonymous 
namespaces if they have conflicting names. We may follow the similar scheme.


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

https://reviews.llvm.org/D80858

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

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Michael Liao via Phabricator via cfe-commits]

hliao added a comment.

In D80858#2177159 , @tra wrote:

> It's a good point. Perhaps this is one of the cases where we should *not* 
> follow nvcc.
> We can't have our cake (preserve static behavior) and eat it (treat it as 
> non-static in case something on the host side may decide to use an API which 
> uses symbol names). Something's got to give. While we could make it work in 
> some cases, I don't think we can make it work consistently.
> I think it would be reasonable to restrict APIs that access symbols by name 
> to be applicable to visible symbols only.

In fact, 'nvcc' does that by globalizing that static device variable.


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

https://reviews.llvm.org/D80858

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

[PATCH] D84697: [clangd] Use elog instead of llvm::errs, log instead of llvm::outs

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 281130.
kbobyrev marked 7 inline comments as done.
kbobyrev added a comment.

Use more conservative approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84697

Files:
  clang-tools-extra/clangd/index/remote/server/Server.cpp
  clang-tools-extra/clangd/indexer/IndexerMain.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -657,16 +657,16 @@
   // continuing.
   llvm::SmallString<128> Path(CompileCommandsDir);
   if (std::error_code EC = llvm::sys::fs::make_absolute(Path)) {
-llvm::errs() << "Error while converting the relative path specified by "
-"--compile-commands-dir to an absolute path: "
- << EC.message() << ". The argument will be ignored.\n";
+elog("Error while converting the relative path specified by "
+ "--compile-commands-dir to an absolute path: {0}. The argument "
+ "will be ignored.",
+ EC.message());
   } else {
 CompileCommandsDirPath = std::string(Path.str());
   }
 } else {
-  llvm::errs()
-  << "Path specified by --compile-commands-dir does not exist. The "
- "argument will be ignored.\n";
+  elog("Path specified by --compile-commands-dir does not exist. The "
+   "argument will be ignored.");
 }
   }
 
@@ -750,7 +750,7 @@
   elog("Couldn't determine user config file, not loading");
 }
 std::vector ProviderPointers;
-for (const auto& P : ProviderStack)
+for (const auto &P : ProviderStack)
   ProviderPointers.push_back(P.get());
 Config = config::Provider::combine(std::move(ProviderPointers));
 Opts.ConfigProvider = Config.get();
Index: clang-tools-extra/clangd/indexer/IndexerMain.cpp
===
--- clang-tools-extra/clangd/indexer/IndexerMain.cpp
+++ clang-tools-extra/clangd/indexer/IndexerMain.cpp
@@ -16,6 +16,7 @@
 #include "index/Serialization.h"
 #include "index/Symbol.h"
 #include "index/SymbolCollector.h"
+#include "support/Logger.h"
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Execution.h"
@@ -122,7 +123,7 @@
   std::make_unique(Data),
   clang::tooling::getStripPluginsAdjuster());
   if (Err) {
-llvm::errs() << llvm::toString(std::move(Err)) << "\n";
+clang::clangd::elog("{0}", std::move(Err));
   }
 
   // Emit collected data.
Index: clang-tools-extra/clangd/index/remote/server/Server.cpp
===
--- clang-tools-extra/clangd/index/remote/server/Server.cpp
+++ clang-tools-extra/clangd/index/remote/server/Server.cpp
@@ -39,6 +39,15 @@
 llvm::cl::opt IndexRoot(llvm::cl::desc(""),
  llvm::cl::Positional, llvm::cl::Required);
 
+llvm::cl::opt LogLevel{
+"log",
+llvm::cl::desc("Verbosity of log messages written to stderr"),
+values(clEnumValN(Logger::Error, "error", "Error messages only"),
+   clEnumValN(Logger::Info, "info", "High level execution tracing"),
+   clEnumValN(Logger::Debug, "verbose", "Low level details")),
+llvm::cl::init(Logger::Info),
+};
+
 llvm::cl::opt TraceFile(
 "trace-file",
 llvm::cl::desc("Path to the file where tracer logs will be stored"));
@@ -173,7 +182,7 @@
   Builder.AddListeningPort(ServerAddress, grpc::InsecureServerCredentials());
   Builder.RegisterService(&Service);
   std::unique_ptr Server(Builder.BuildAndStart());
-  llvm::outs() << "Server listening on " << ServerAddress << '\n';
+  log("Server listening on {0}", ServerAddress);
 
   Server->Wait();
 }
@@ -190,8 +199,15 @@
   llvm::cl::ParseCommandLineOptions(argc, argv, Overview);
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
 
+  // Use buffered stream to stderr.
+  llvm::errs().SetBuffered();
+  // Don't flush stdout when logging for performance.
+  llvm::errs().tie(nullptr);
+  clang::clangd::StreamLogger Logger(llvm::errs(), LogLevel);
+  clang::clangd::LoggingSession LoggingSession(Logger);
+
   if (!llvm::sys::path::is_absolute(IndexRoot)) {
-elog("Index root should be an absolute path.");
+llvm::errs() << "Index root should be an absolute path.\n";
 return -1;
   }
 
@@ -220,7 +236,7 @@
   std::unique_ptr Index = openIndex(IndexPath);
 
   if (!Index) {
-elog("Failed to open the index.");
+llvm::errs() << "Failed to open the index.\n";
 return -1;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84678: [clang] False line number in a function definition with "void" parameter

[Jaydeep Chauhan via Phabricator via cfe-commits]

Jac1494 added inline comments.



Comment at: clang/test/Sema/void-argument.cpp:1
+// RUN: not %clang_cc1 %s 2>&1 | FileCheck %s
+

riccibruno wrote:
> These kinds of tests are typically done with a `-verify` test. See the other 
> tests in `Sema/` for examples.
we are testing here line number rather than diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84678

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

[PATCH] D84703: [clang codegen][AArch64] Use llvm.aarch64.neon.fcvtzs/u where it's necessary

[Sjoerd Meijer via Phabricator via cfe-commits]

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

Cheers, LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84703

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

[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added reviewers: NoQ, vsavchenko.
Szelethus accepted this revision.
Szelethus added a comment.

Lets make sure that we invite others from the community to participate, here 
and on other patches as well. Otherwise, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84494

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

[PATCH] D80873: [clang][cmake] Force CMAKE_LINKER for multistage build in case of BOOTSTRAP_LLVM_ENABLE_LLD and MSVC

[Kristina Bessonova 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 rGad4ab81dccaa: [clang][cmake] Force CMAKE_LINKER for 
multistage build in case of… (authored by krisb).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80873

Files:
  clang/CMakeLists.txt
  llvm/cmake/modules/HandleLLVMOptions.cmake


Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -261,7 +261,12 @@
   if ( LLVM_USE_LINKER )
 message(FATAL_ERROR "LLVM_ENABLE_LLD and LLVM_USE_LINKER can't be set at 
the same time")
   endif()
-  set(LLVM_USE_LINKER "lld")
+  # In case of MSVC cmake always invokes the linker directly, so the linker
+  # should be specified by CMAKE_LINKER cmake variable instead of by -fuse-ld
+  # compiler option.
+  if ( NOT MSVC )
+set(LLVM_USE_LINKER "lld")
+  endif()
 endif()
 
 if( LLVM_USE_LINKER )
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -753,6 +753,14 @@
 -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/${C_COMPILER}
 -DCMAKE_ASM_COMPILER_ID=Clang)
 
+  # cmake requires CMAKE_LINKER to be specified if the compiler is MSVC-like,
+  # otherwise it defaults the linker to be link.exe.
+  if(BOOTSTRAP_LLVM_ENABLE_LLD)
+if((WIN32 AND NOT BOOTSTRAP_CMAKE_SYSTEM_NAME) OR 
BOOTSTRAP_CMAKE_SYSTEM_NAME STREQUAL "Windows")
+  set(${CLANG_STAGE}_LINKER 
-DCMAKE_LINKER=${LLVM_RUNTIME_OUTPUT_INTDIR}/lld-link${CMAKE_EXECUTABLE_SUFFIX})
+endif()
+  endif()
+
   if(BOOTSTRAP_CMAKE_SYSTEM_NAME)
 set(${CLANG_STAGE}_CONFIG 
-DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config)
 set(${CLANG_STAGE}_TABLEGEN


Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -261,7 +261,12 @@
   if ( LLVM_USE_LINKER )
 message(FATAL_ERROR "LLVM_ENABLE_LLD and LLVM_USE_LINKER can't be set at the same time")
   endif()
-  set(LLVM_USE_LINKER "lld")
+  # In case of MSVC cmake always invokes the linker directly, so the linker
+  # should be specified by CMAKE_LINKER cmake variable instead of by -fuse-ld
+  # compiler option.
+  if ( NOT MSVC )
+set(LLVM_USE_LINKER "lld")
+  endif()
 endif()
 
 if( LLVM_USE_LINKER )
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -753,6 +753,14 @@
 -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/${C_COMPILER}
 -DCMAKE_ASM_COMPILER_ID=Clang)
 
+  # cmake requires CMAKE_LINKER to be specified if the compiler is MSVC-like,
+  # otherwise it defaults the linker to be link.exe.
+  if(BOOTSTRAP_LLVM_ENABLE_LLD)
+if((WIN32 AND NOT BOOTSTRAP_CMAKE_SYSTEM_NAME) OR BOOTSTRAP_CMAKE_SYSTEM_NAME STREQUAL "Windows")
+  set(${CLANG_STAGE}_LINKER -DCMAKE_LINKER=${LLVM_RUNTIME_OUTPUT_INTDIR}/lld-link${CMAKE_EXECUTABLE_SUFFIX})
+endif()
+  endif()
+
   if(BOOTSTRAP_CMAKE_SYSTEM_NAME)
 set(${CLANG_STAGE}_CONFIG -DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config)
 set(${CLANG_STAGE}_TABLEGEN
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] ad4ab81 - [clang][cmake] Force CMAKE_LINKER for multistage build in case of BOOTSTRAP_LLVM_ENABLE_LLD and MSVC

[Kristina Bessonova via cfe-commits]

Author: Kristina Bessonova
Date: 2020-07-28T10:11:52+02:00
New Revision: ad4ab81dccaa72d9b5137433a0923d325ff76135

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

LOG: [clang][cmake] Force CMAKE_LINKER for multistage build in case of 
BOOTSTRAP_LLVM_ENABLE_LLD and MSVC

The issue with LLVM_ENABLE_LLD is that it just passes -fuse-ld=lld
to compiler/linker options which makes sense only for those platforms
where cmake invokes a compiler driver for linking. On Windows (MSVC) cmake
invokes the linker directly and requires CMAKE_LINKER to be specified
otherwise it defaults CMAKE_LINKER to be link.exe.

This patch allows BOOTSTRAP_LLVM_ENABLE_LLD to set CMAKE_LINKER in two cases:
* if building for host Windows,
* if crosscompiling for target Windows.

It also skips adding '-fuse-ld=lld' to make lld-link not warning
about 'unknown argument'.

This fixes build with `clang/cmake/caches/DistributionExample.cmake`
on Windows.

Reviewed By: phosek

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

Added: 


Modified: 
clang/CMakeLists.txt
llvm/cmake/modules/HandleLLVMOptions.cmake

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 1a6a20a271f3..0f08538495fc 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -753,6 +753,14 @@ if (CLANG_ENABLE_BOOTSTRAP)
 -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/${C_COMPILER}
 -DCMAKE_ASM_COMPILER_ID=Clang)
 
+  # cmake requires CMAKE_LINKER to be specified if the compiler is MSVC-like,
+  # otherwise it defaults the linker to be link.exe.
+  if(BOOTSTRAP_LLVM_ENABLE_LLD)
+if((WIN32 AND NOT BOOTSTRAP_CMAKE_SYSTEM_NAME) OR 
BOOTSTRAP_CMAKE_SYSTEM_NAME STREQUAL "Windows")
+  set(${CLANG_STAGE}_LINKER 
-DCMAKE_LINKER=${LLVM_RUNTIME_OUTPUT_INTDIR}/lld-link${CMAKE_EXECUTABLE_SUFFIX})
+endif()
+  endif()
+
   if(BOOTSTRAP_CMAKE_SYSTEM_NAME)
 set(${CLANG_STAGE}_CONFIG 
-DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config)
 set(${CLANG_STAGE}_TABLEGEN

diff  --git a/llvm/cmake/modules/HandleLLVMOptions.cmake 
b/llvm/cmake/modules/HandleLLVMOptions.cmake
index 62dd0ef79cf4..89f7016a7db4 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -261,7 +261,12 @@ if( LLVM_ENABLE_LLD )
   if ( LLVM_USE_LINKER )
 message(FATAL_ERROR "LLVM_ENABLE_LLD and LLVM_USE_LINKER can't be set at 
the same time")
   endif()
-  set(LLVM_USE_LINKER "lld")
+  # In case of MSVC cmake always invokes the linker directly, so the linker
+  # should be specified by CMAKE_LINKER cmake variable instead of by -fuse-ld
+  # compiler option.
+  if ( NOT MSVC )
+set(LLVM_USE_LINKER "lld")
+  endif()
 endif()
 
 if( LLVM_USE_LINKER )



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

[PATCH] D84371: [DFSan] Add efficient fast16labels instrumentation mode.

[Vitaly Buka via Phabricator via cfe-commits]

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

LGTM either way




Comment at: compiler-rt/lib/dfsan/dfsan.cpp:180
 dfsan_label __dfsan_union(dfsan_label l1, dfsan_label l2) {
-  if (flags().fast16labels)
+  if (fast16labels)
 return l1 | l2;

morehouse wrote:
> vitalybuka wrote:
> > isn't better just create new set of callbacks?
> > e.g __dfsan_fast16_union
> > and then we don't need any flags or preinit array initialization
> Should work for `__dfsan_union` and `__dfsan_union_load`, but what about all 
> the other API functions the user can call directly?  We wouldn't be able to 
> warn in those cases.
> Should work for `__dfsan_union` and `__dfsan_union_load`, but what about all 
> the other API functions the user can call directly?  We wouldn't be able to 
> warn in those cases.

Those calls are no-op, it would be nice, but it's not worth it.
You can avoid state variable fast16labels, and avoid preinit_array which time 
to time conflicts with other libs. Which looks more useful then warning.





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84371

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

[PATCH] D83961: [Analyzer] Fix bug report source locations in minimal output.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I believe i clarified above how this checker should work. For testing purposes 
(which is the only reason to ever use the minimal output) i prefer to have it 
at the end of path, because that tells more about the bug path and therefore 
more important to test.

The question i'm asking is more about API contracts of the 
`PathSensitiveBugReport` class over its virtual methods. Right now i don't care 
whether the entire system behaves correctly or not. I want you to explain how 
the change in one component caused a change in another component, so that to 
avoid introducing pairs of mutually cancelling bugs. In other words, this is a 
unit-test question, not an integration-test question.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83961

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

[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp:215
   // The explicit NULL case.
+  // We know that 'location' cannot be non-null.  This is what
+  // we call an "explicit" null dereference.

Maybe "is definitely null" here? Otherwise it can be pretty confusing with a 
double negation.



Comment at: 
clang/test/Analysis/Inputs/expected-plists/conditional-path-notes.c.plist:270
descriptionDereference of null pointer (loaded from 
variable 'x')
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer

I might've missed some discussions on that matter, so please correct me on that.
In my opinion, null pointer dereference is not a memory error.  Null address is 
not a correct memory and never was, so it is a logic error that a special value 
is being interpreted as the pointer to something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84494

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

[PATCH] D80802: [RISCV] Upgrade RVV MC to v0.9.

[Ferran Pallarès Roca via Phabricator via cfe-commits]

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

Aside from the minor nit below this patch LGTM. Thanks a lot for all the 
changes @HsiangKai.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80802

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

[PATCH] D80802: [RISCV] Upgrade RVV MC to v0.9.

[Ferran Pallarès Roca via Phabricator via cfe-commits]

fpallares added inline comments.



Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:542
 defm VMSGT_V : VALU_IV_X_I<"vmsgt", 0b01>;
+}
 

Minor nit: Add comment here (for the other `let RVVConstraint = NoConstraint` 
blocks below as well).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80802

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

[PATCH] D84306: [clang-format][NFC] Be more careful about the layout of FormatToken.

[MyDeveloperDay via Phabricator via cfe-commits]

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

Thank you for the patch, this LGTM, I think this kind of change should help 
reduce memory usage and I feel improves the readability.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84306

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

[PATCH] D84306: [clang-format][NFC] Be more careful about the layout of FormatToken.

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D84306#2177985 , @MyDeveloperDay 
wrote:

> Thank you for the patch, this LGTM, I think this kind of change should help 
> reduce memory usage and I feel improves the readability.

Thanks for your review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84306

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

[PATCH] D84678: [clang] False line number in a function definition with "void" parameter

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



Comment at: clang/test/Sema/void-argument.cpp:1
+// RUN: not %clang_cc1 %s 2>&1 | FileCheck %s
+

Jac1494 wrote:
> riccibruno wrote:
> > These kinds of tests are typically done with a `-verify` test. See the 
> > other tests in `Sema/` for examples.
> we are testing here line number rather than diagnostic.
`-verify` implicitly checks line numbers. Even better: you don't have to 
hard-code the specific line numbers into the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84678

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

[PATCH] D66035: [WebAssembly] WIP: Add support for reference types

[Paulo Matos via Phabricator via cfe-commits]

pmatos updated this revision to Diff 281154.
pmatos added a comment.

Initial implementation of reference types in the WebAssembly backend


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66035

Files:
  .gitlab-ci.yml
  clang/lib/Basic/Targets/WebAssembly.cpp
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  lld/wasm/WriterUtils.cpp
  llvm/include/llvm/BinaryFormat/Wasm.h
  llvm/include/llvm/BinaryFormat/WasmRelocs.def
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/include/llvm/CodeGen/ValueTypes.td
  llvm/include/llvm/MC/MCExpr.h
  llvm/include/llvm/MC/MCSymbolWasm.h
  llvm/include/llvm/Object/Wasm.h
  llvm/include/llvm/Support/MachineValueType.h
  llvm/lib/BinaryFormat/Wasm.cpp
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/ValueTypes.cpp
  llvm/lib/MC/MCExpr.cpp
  llvm/lib/MC/WasmObjectWriter.cpp
  llvm/lib/Object/WasmObjectFile.cpp
  llvm/lib/ObjectYAML/WasmEmitter.cpp
  llvm/lib/ObjectYAML/WasmYAML.cpp
  llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
  llvm/lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyInstPrinter.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyWasmObjectWriter.cpp
  llvm/lib/Target/WebAssembly/WebAssembly.h
  llvm/lib/Target/WebAssembly/WebAssembly.td
  llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyExplicitLocals.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyFastISel.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.h
  llvm/lib/Target/WebAssembly/WebAssemblyInstrCall.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td
  llvm/lib/Target/WebAssembly/WebAssemblyInstrTable.td
  llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyPeephole.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyRegisterInfo.td
  llvm/lib/Target/WebAssembly/WebAssemblySubtarget.h
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/test/CodeGen/WebAssembly/externref.ll
  llvm/test/CodeGen/WebAssembly/reg-argument.mir
  llvm/test/CodeGen/WebAssembly/reg-copy.mir
  llvm/utils/TableGen/CodeGenTarget.cpp

Index: llvm/utils/TableGen/CodeGenTarget.cpp
===
--- llvm/utils/TableGen/CodeGenTarget.cpp
+++ llvm/utils/TableGen/CodeGenTarget.cpp
@@ -196,6 +196,7 @@
   case MVT::iPTRAny:  return "MVT::iPTRAny";
   case MVT::Untyped:  return "MVT::Untyped";
   case MVT::exnref:   return "MVT::exnref";
+  case MVT::externref:   return "MVT::externref";
   default: llvm_unreachable("ILLEGAL VALUE TYPE!");
   }
 }
Index: llvm/test/CodeGen/WebAssembly/reg-copy.mir
===
--- llvm/test/CodeGen/WebAssembly/reg-copy.mir
+++ llvm/test/CodeGen/WebAssembly/reg-copy.mir
@@ -66,3 +66,14 @@
 %0:exnref = COPY %1:exnref
 RETURN implicit-def $arguments
 ...
+---
+name: copy_externref
+# CHECK-LABEL: copy_externref
+body: |
+  ; CHECK-LABEL: bb.0
+  ; CHECK-NEXT: %0:externref = COPY_EXTERNREF %1:externref
+  ; CHECK-NEXT: RETURN
+  bb.0:
+%0:externref = COPY %1:externref
+RETURN implicit-def $arguments
+...
Index: llvm/test/CodeGen/WebAssembly/reg-argument.mir
===
--- llvm/test/CodeGen/WebAssembly/reg-argument.mir
+++ llvm/test/CodeGen/WebAssembly/reg-argument.mir
@@ -57,3 +57,14 @@
 %1:exnref = ARGUMENT_exnref 0, implicit $arguments
 RETURN implicit-def $arguments
 ...
+---
+name: argument_externref
+# CHECK-LABEL: argument_externref
+body: |
+  ; CHECK-LABEL: bb.0:
+  ; CHECK-NEXT: %1:externref = ARGUMENT_externref 0
+  bb.0:
+%0:i32 = CONST_I32 0, implicit-def $arguments
+%1:externref = ARGUMENT_externref 0, implicit $arguments
+RETURN implicit-def $arguments
+...
Index: llvm/test/CodeGen/WebAssembly/externref.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/externref.ll
@@ -0,0 +1,29 @@
+; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -mattr=+reference-types | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128-ni:256"
+target triple = "wasm32-unknown-unknown"
+
+declare i8 addrspace(256)* @test(i8 addrspace(256)*) 
+
+
+; CHECK-LABEL: call_test:
+; CHECK: .functype   call_test (externref) -> (externref)
+define i8 addrspace(256)* @call_tes

[clang] eb10b06 - [clang] Pass the NamedDecl* instead of the DeclarationName into many diagnostics.

[Bruno Ricci via cfe-commits]

Author: Bruno Ricci
Date: 2020-07-28T10:30:35+01:00
New Revision: eb10b065f2a870b425dcc2040b9955e0eee464b4

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

LOG: [clang] Pass the NamedDecl* instead of the DeclarationName into many 
diagnostics.

Background:
---
There are two related argument types which can be sent into a diagnostic to
display the name of an entity: DeclarationName (ak_declarationname) or
NamedDecl* (ak_nameddecl) (there is also ak_identifierinfo for
IdentifierInfo*, but we are not concerned with it here).

A DeclarationName in a diagnostic will just be streamed to the output,
which will directly result in a call to DeclarationName::print.

A NamedDecl* in a diagnostic will also ultimately result in a call to
DeclarationName::print, but with two customisation points along the way:

The first customisation point is NamedDecl::getNameForDiagnostic which is
overloaded by FunctionDecl, ClassTemplateSpecializationDecl and
VarTemplateSpecializationDecl to print the template arguments, if any.

The second customisation point is NamedDecl::printName. By default it just
streams the stored DeclarationName into the output but it can be customised
to provide a user-friendly name for an entity. It is currently overloaded by
DecompositionDecl and MSGuidDecl.

What this patch does:
-
For many diagnostics a DeclarationName is used instead of the NamedDecl*.
This bypasses the two customisation points mentioned above. This patches fix
this for diagnostics in Sema.cpp, SemaCast.cpp, SemaChecking.cpp, SemaDecl.cpp,
SemaDeclAttr.cpp, SemaDecl.cpp, SemaOverload.cpp and SemaStmt.cpp.

I have only modified diagnostics where I could construct a test-case which
demonstrates that the change is appropriate (either with this patch or the next
one).

Reviewed By: erichkeane, aaron.ballman

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

Added: 


Modified: 
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaExpr.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaStmt.cpp
clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.noreturn/p1.cpp
clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p3.cpp
clang/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
clang/test/CXX/temp/temp.param/p15-cxx0x.cpp
clang/test/Modules/module-private.cpp
clang/test/SemaCXX/array-bounds.cpp
clang/test/SemaCXX/attr-unused.cpp
clang/test/SemaCXX/cxx1y-variable-templates_in_class.cpp
clang/test/SemaCXX/default2.cpp
clang/test/SemaCXX/incomplete-call.cpp
clang/test/SemaCXX/references.cpp
clang/test/SemaCXX/return-void.cpp
clang/test/SemaCXX/warn-func-not-needed.cpp
clang/test/SemaCXX/warn-large-by-value-copy.cpp
clang/test/SemaCXX/warn-member-not-needed.cpp
clang/test/SemaCXX/warn-pure-virtual-call-from-ctor-dtor.cpp
clang/test/SemaCXX/warn-pure-virtual-kext.cpp
clang/test/SemaCXX/warn-unused-filescoped.cpp
clang/test/SemaCXX/warn-variable-not-needed.cpp

Removed: 




diff  --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 9c8f3fdcda4a..7415d0d0766b 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1197,7 +1197,7 @@ void Sema::ActOnEndOfTranslationUnit() {
 if (DiagD->isReferenced()) {
   if (isa(DiagD))
 Diag(DiagD->getLocation(), diag::warn_unneeded_member_function)
-  << DiagD->getDeclName();
+<< DiagD;
   else {
 if (FD->getStorageClass() == SC_Static &&
 !FD->isInlineSpecified() &&
@@ -1205,20 +1205,20 @@ void Sema::ActOnEndOfTranslationUnit() {
SourceMgr.getExpansionLoc(FD->getLocation(
   Diag(DiagD->getLocation(),
diag::warn_unneeded_static_internal_decl)
-  << DiagD->getDeclName();
+  << DiagD;
 else
   Diag(DiagD->getLocation(), diag::warn_unneeded_internal_decl)
-   << /*function*/0 << DiagD->getDeclName();
+  << /*function*/ 0 << DiagD;
   }
 } else {
   if (FD->getDescribedFunctionTemplate())
 Diag(DiagD->getLocation(), diag::warn_unused_template)
-  << /*function*/0 << DiagD->getDeclName();
+<< /*function*/ 0 << DiagD;
   else
-Diag(DiagD->getLocation(),
- isa(DiagD) ? diag::warn_unused_member_function
+Diag(DiagD->getLocation(), isa(DiagD)
+   ? diag::warn_unused_member_function
: diag::warn_unused_function)
-

[clang] f5acd11 - [clang-format][NFC] Be more careful about the layout of FormatToken.

[Bruno Ricci via cfe-commits]

Author: Bruno Ricci
Date: 2020-07-28T10:30:28+01:00
New Revision: f5acd11d2c0ea228452aa5ed3abbc2c502009d56

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

LOG: [clang-format][NFC] Be more careful about the layout of FormatToken.

The underlying ABI forces FormatToken to have a lot of padding.

Currently (on x86-64 linux) `sizeof(FormatToken) == 288`. After this patch
`sizeof(FormatToken) == 232`.

No functional changes.

Reviewed By: MyDeveloperDay

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

Added: 


Modified: 
clang/lib/Format/ContinuationIndenter.cpp
clang/lib/Format/Format.cpp
clang/lib/Format/FormatToken.cpp
clang/lib/Format/FormatToken.h
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineFormatter.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/lib/Format/WhitespaceManager.cpp

Removed: 




diff  --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/Format/ContinuationIndenter.cpp
index b1497651a8fe..f3202bcb5bc1 100644
--- a/clang/lib/Format/ContinuationIndenter.cpp
+++ b/clang/lib/Format/ContinuationIndenter.cpp
@@ -284,7 +284,7 @@ bool ContinuationIndenter::canBreak(const LineState &State) 
{
   // The opening "{" of a braced list has to be on the same line as the first
   // element if it is nested in another braced init list or function call.
   if (!Current.MustBreakBefore && Previous.is(tok::l_brace) &&
-  Previous.isNot(TT_DictLiteral) && Previous.BlockKind == BK_BracedInit &&
+  Previous.isNot(TT_DictLiteral) && Previous.is(BK_BracedInit) &&
   Previous.Previous &&
   Previous.Previous->isOneOf(tok::l_brace, tok::l_paren, tok::comma))
 return false;
@@ -501,7 +501,7 @@ bool ContinuationIndenter::mustBreak(const LineState 
&State) {
   // The following could be precomputed as they do not depend on the state.
   // However, as they should take effect only if the UnwrappedLine does not fit
   // into the ColumnLimit, they are checked here in the ContinuationIndenter.
-  if (Style.ColumnLimit != 0 && Previous.BlockKind == BK_Block &&
+  if (Style.ColumnLimit != 0 && Previous.is(BK_Block) &&
   Previous.is(tok::l_brace) && !Current.isOneOf(tok::r_brace, 
tok::comment))
 return true;
 
@@ -627,7 +627,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
   // opening parenthesis. Don't break if it doesn't conserve columns.
   if (Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak &&
   (Previous.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) ||
-   (Previous.is(tok::l_brace) && Previous.BlockKind != BK_Block &&
+   (Previous.is(tok::l_brace) && Previous.isNot(BK_Block) &&
 Style.Cpp11BracedListStyle)) &&
   State.Column > getNewLineColumn(State) &&
   (!Previous.Previous || !Previous.Previous->isOneOf(
@@ -648,7 +648,7 @@ void ContinuationIndenter::addTokenOnCurrentLine(LineState 
&State, bool DryRun,
   if (Style.AlignAfterOpenBracket != FormatStyle::BAS_DontAlign &&
   !State.Stack.back().IsCSharpGenericTypeConstraint &&
   Previous.opensScope() && Previous.isNot(TT_ObjCMethodExpr) &&
-  (Current.isNot(TT_LineComment) || Previous.BlockKind == BK_BracedInit)) {
+  (Current.isNot(TT_LineComment) || Previous.is(BK_BracedInit))) {
 State.Stack.back().Indent = State.Column + Spaces;
 State.Stack.back().IsAligned = true;
   }
@@ -972,7 +972,7 @@ unsigned ContinuationIndenter::getNewLineColumn(const 
LineState &State) {
 return (Style.IndentWidth * State.Line->First->IndentLevel) +
Style.IndentWidth;
 
-  if (NextNonComment->is(tok::l_brace) && NextNonComment->BlockKind == 
BK_Block)
+  if (NextNonComment->is(tok::l_brace) && NextNonComment->is(BK_Block))
 return Current.NestingLevel == 0 ? State.FirstIndent
  : State.Stack.back().Indent;
   if ((Current.isOneOf(tok::r_brace, tok::r_square) ||
@@ -982,8 +982,7 @@ unsigned ContinuationIndenter::getNewLineColumn(const 
LineState &State) {
   State.Stack.size() > 1) {
 if (Current.closesBlockOrBlockTypeList(Style))
   return State.Stack[State.Stack.size() - 2].NestedBlockIndent;
-if (Current.MatchingParen &&
-Current.MatchingParen->BlockKind == BK_BracedInit)
+if (Current.MatchingParen && Current.MatchingParen->is(BK_BracedInit))
   return State.Stack[State.Stack.size() - 2].LastSpace;
 return State.FirstIndent;
   }
@@ -1417,7 +1416,7 @@ void 
ContinuationIndenter::moveStatePastScopeOpener(LineState &State,
   State.Stack.back().IsCSharpGenericTypeConstraint)
 return;
 
-  if (Current.MatchingParen && Current.BlockKind == BK_Block) {
+  if (Current.MatchingParen && Current.is(BK_Block)) {
 moveStateToNewBlock(State);
 retu

[PATCH] D84656: [clang] Pass the NamedDecl* instead of the DeclarationName into many diagnostics.

[Bruno Ricci 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 rGeb10b065f2a8: [clang] Pass the NamedDecl* instead of the 
DeclarationName into many… (authored by riccibruno).

Changed prior to commit:
  https://reviews.llvm.org/D84656?vs=280888&id=281156#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84656

Files:
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.noreturn/p1.cpp
  clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct/p3.cpp
  clang/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
  clang/test/CXX/temp/temp.param/p15-cxx0x.cpp
  clang/test/Modules/module-private.cpp
  clang/test/SemaCXX/array-bounds.cpp
  clang/test/SemaCXX/attr-unused.cpp
  clang/test/SemaCXX/cxx1y-variable-templates_in_class.cpp
  clang/test/SemaCXX/default2.cpp
  clang/test/SemaCXX/incomplete-call.cpp
  clang/test/SemaCXX/references.cpp
  clang/test/SemaCXX/return-void.cpp
  clang/test/SemaCXX/warn-func-not-needed.cpp
  clang/test/SemaCXX/warn-large-by-value-copy.cpp
  clang/test/SemaCXX/warn-member-not-needed.cpp
  clang/test/SemaCXX/warn-pure-virtual-call-from-ctor-dtor.cpp
  clang/test/SemaCXX/warn-pure-virtual-kext.cpp
  clang/test/SemaCXX/warn-unused-filescoped.cpp
  clang/test/SemaCXX/warn-variable-not-needed.cpp

Index: clang/test/SemaCXX/warn-variable-not-needed.cpp
===
--- clang/test/SemaCXX/warn-variable-not-needed.cpp
+++ clang/test/SemaCXX/warn-variable-not-needed.cpp
@@ -5,7 +5,7 @@
 
   namespace {
   template  int abc_template = 0;
-  template <> int abc_template = 0; // expected-warning {{variable 'abc_template' is not needed and will not be emitted}}
+  template <> int abc_template = 0; // expected-warning {{variable 'abc_template' is not needed and will not be emitted}}
   }  // namespace
   template 
   int foo(void) {
Index: clang/test/SemaCXX/warn-unused-filescoped.cpp
===
--- clang/test/SemaCXX/warn-unused-filescoped.cpp
+++ clang/test/SemaCXX/warn-unused-filescoped.cpp
@@ -67,7 +67,7 @@
 
   template 
   void tf() {}  // expected-warning{{unused function template 'tf'}}
-  template <> void tf() {} // expected-warning{{unused function 'tf'}}
+  template <> void tf() {} // expected-warning{{unused function 'tf'}}
 
   struct VS {
 virtual void vm() { }
@@ -102,7 +102,7 @@
 
   template  int vt = 0; // expected-warning {{unused variable template 'vt'}}
   template  int vt = 0;
-  template <> int vt = 0; // expected-warning {{unused variable 'vt'}}
+  template <> int vt = 0; // expected-warning {{unused variable 'vt'}}
 }
 
 namespace PR8841 {
@@ -132,7 +132,7 @@
 namespace rdar8733476 {
 static void foo() {} // expected-warning {{function 'foo' is not needed and will not be emitted}}
 template  static void foo_t() {} // expected-warning {{unused function template 'foo_t'}}
-template <> void foo_t() {} // expected-warning {{function 'foo_t' is not needed and will not be emitted}}
+template <> void foo_t() {} // expected-warning {{function 'foo_t' is not needed and will not be emitted}}
 
 template 
 void bar() {
@@ -157,7 +157,7 @@
   namespace {
   // FIXME: Should be "unused variable template 'var_t'" instead.
   template  const double var_t = 0; // expected-warning {{unused variable 'var_t'}}
-  template <> const double var_t = 0;  // expected-warning {{variable 'var_t' is not needed and will not be emitted}}
+  template <> const double var_t = 0;  // expected-warning {{variable 'var_t' is not needed and will not be emitted}}
   int z = sizeof(var_t);   // expected-warning {{unused variable 'z'}}
   } // namespace
 }
Index: clang/test/SemaCXX/warn-pure-virtual-kext.cpp
===
--- clang/test/SemaCXX/warn-pure-virtual-kext.cpp
+++ clang/test/SemaCXX/warn-pure-virtual-kext.cpp
@@ -10,7 +10,7 @@
 template  struct TA {
   virtual void f() = 0; // expected-note {{'f' declared here}}
 
-  TA() { TA::f(); } // expected-warning {{call to pure virtual member function 'f' has undefined behavior; overrides of 'f' in subclasses are not available in the constructor of 'TA'}} // expected-note {{qualified call to 'TA'::'f' is treated as a virtual call to 'f' due to -fapple-kext}}
+  TA() { TA::f(); } // expected-warning {{call to pure virtual member function 'f' has undefined behavior; overrides of 'f' in subclasses are not available in the constructor of 'TA'}} //

[PATCH] D84306: [clang-format][NFC] Be more careful about the layout of FormatToken.

[Bruno Ricci via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf5acd11d2c0e: [clang-format][NFC] Be more careful about the 
layout of FormatToken. (authored by riccibruno).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84306

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/WhitespaceManager.cpp

Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -49,7 +49,7 @@
   bool IsAligned, bool InPPDirective) {
   if (Tok.Finalized)
 return;
-  Tok.Decision = (Newlines > 0) ? FD_Break : FD_Continue;
+  Tok.setDecision((Newlines > 0) ? FD_Break : FD_Continue);
   Changes.push_back(Change(Tok, /*CreateReplacement=*/true, Tok.WhitespaceRange,
Spaces, StartOfTokenColumn, Newlines, "", "",
IsAligned, InPPDirective && !Tok.IsFirst,
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -472,19 +472,19 @@
   // individual members in a type member list, which would normally
   // trigger BK_Block. In both cases, this must be parsed as an inline
   // braced init.
-  Tok->BlockKind = BK_BracedInit;
+  Tok->setBlockKind(BK_BracedInit);
 else if (PrevTok->is(tok::r_paren))
   // `) { }` can only occur in function or method declarations in JS.
-  Tok->BlockKind = BK_Block;
+  Tok->setBlockKind(BK_Block);
   } else {
-Tok->BlockKind = BK_Unknown;
+Tok->setBlockKind(BK_Unknown);
   }
   LBraceStack.push_back(Tok);
   break;
 case tok::r_brace:
   if (LBraceStack.empty())
 break;
-  if (LBraceStack.back()->BlockKind == BK_Unknown) {
+  if (LBraceStack.back()->is(BK_Unknown)) {
 bool ProbablyBracedList = false;
 if (Style.Language == FormatStyle::LK_Proto) {
   ProbablyBracedList = NextTok->isOneOf(tok::comma, tok::r_square);
@@ -524,11 +524,11 @@
   }
 }
 if (ProbablyBracedList) {
-  Tok->BlockKind = BK_BracedInit;
-  LBraceStack.back()->BlockKind = BK_BracedInit;
+  Tok->setBlockKind(BK_BracedInit);
+  LBraceStack.back()->setBlockKind(BK_BracedInit);
 } else {
-  Tok->BlockKind = BK_Block;
-  LBraceStack.back()->BlockKind = BK_Block;
+  Tok->setBlockKind(BK_Block);
+  LBraceStack.back()->setBlockKind(BK_Block);
 }
   }
   LBraceStack.pop_back();
@@ -545,8 +545,8 @@
 case tok::kw_switch:
 case tok::kw_try:
 case tok::kw___try:
-  if (!LBraceStack.empty() && LBraceStack.back()->BlockKind == BK_Unknown)
-LBraceStack.back()->BlockKind = BK_Block;
+  if (!LBraceStack.empty() && LBraceStack.back()->is(BK_Unknown))
+LBraceStack.back()->setBlockKind(BK_Block);
   break;
 default:
   break;
@@ -557,8 +557,8 @@
 
   // Assume other blocks for all unclosed opening braces.
   for (unsigned i = 0, e = LBraceStack.size(); i != e; ++i) {
-if (LBraceStack[i]->BlockKind == BK_Unknown)
-  LBraceStack[i]->BlockKind = BK_Block;
+if (LBraceStack[i]->is(BK_Unknown))
+  LBraceStack[i]->setBlockKind(BK_Block);
   }
 
   FormatTok = Tokens->setPosition(StoredPosition);
@@ -584,7 +584,7 @@
   assert(FormatTok->isOneOf(tok::l_brace, TT_MacroBlockBegin) &&
  "'{' or macro block token expected");
   const bool MacroBlock = FormatTok->is(TT_MacroBlockBegin);
-  FormatTok->BlockKind = BK_Block;
+  FormatTok->setBlockKind(BK_Block);
 
   size_t PPStartHash = computePPHash();
 
@@ -614,7 +614,7 @@
   if (MacroBlock ? !FormatTok->is(TT_MacroBlockEnd)
  : !FormatTok->is(tok::r_brace)) {
 Line->Level = InitialLevel;
-FormatTok->BlockKind = BK_Block;
+FormatTok->setBlockKind(BK_Block);
 return;
   }
 
@@ -690,7 +690,7 @@
 }
 
 void UnwrappedLineParser::parseChildBlock() {
-  FormatTok->BlockKind = BK_Block;
+  FormatTok->setBlockKind(BK_Block);
   nextToken();
   {
 bool SkipIndent = (Style.Language == FormatStyle::LK_JavaScript &&
@@ -1476,7 +1476,7 @@
 // C# needs this change to ensure that array initialisers and object
 // initialisers are indented the same way.
 if (Style.isCSharp())
-  FormatTok->BlockKind = BK_BracedInit;
+  FormatTok->setBlockKind(BK_BracedInit);
 nextToken();
 parseBra

[PATCH] D84599: [clang-index] Use NamedDecl::getDeclName() instead of NamedDecl::printName in USRGenerator::EmitDeclName

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

The overloads of `NamedDecl::printName` are in D84658 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84599

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

[PATCH] D84494: [Analyzer] Use of BugType in DereferenceChecker (NFC).

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.



Comment at: 
clang/test/Analysis/Inputs/expected-plists/conditional-path-notes.c.plist:270
descriptionDereference of null pointer (loaded from 
variable 'x')
-   categoryLogic error
+   categoryMemory error
typeDereference of null pointer

vsavchenko wrote:
> I might've missed some discussions on that matter, so please correct me on 
> that.
> In my opinion, null pointer dereference is not a memory error.  Null address 
> is not a correct memory and never was, so it is a logic error that a special 
> value is being interpreted as the pointer to something.
I can not decide which is better, "logic error" can be said for almost every 
possible bug. Here the problem is at least related to memory handling. The 
reference of undefined pointer value is "logic error" too (it is known that the 
value was not initialized) but a memory error (try to access invalid or valid 
but wrong address). Probably "pointer handling error" is better?
(One value for bug category is not a good approach, it is never possible to 
classify a bug to exactly one.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84494

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

[PATCH] D80802: [RISCV] Upgrade RVV MC to v0.9.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai added a comment.

Thanks for your review, @fpallares.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80802

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

[PATCH] D79147: Switch to using -debug-info-kind=constructor as default (from =limited)

[Russell Gallop via Phabricator via cfe-commits]

russell.gallop added a comment.

Hi,

It looks like is causing one of the debuginfo-tests: 
llgdb-tests/nrvo-string.cpp to fail, run on Linux. Failure as below. I don't 
think the debuginfo-tests are run on any bot (but probably should be!). I 
bisected the failure back to this change.

Please could you take a look?

Thanks
Russ

  FAIL: debuginfo-tests :: llgdb-tests/nrvo-string.cpp (1 of 1)
   TEST 'debuginfo-tests :: llgdb-tests/nrvo-string.cpp' 
FAILED 
  Script:
  --
  : 'RUN: at line 4';   /home//git/llvm-project/stage1/bin/clang 
--driver-mode=g++ -O0 -fno-exceptions --target=x86_64-unknown-linux-gnu 
/home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp -o 
/home//git/llvm-project/stage1/projects/debuginfo-tests/llgdb-tests/Output/nrvo-string.cpp.tmp.out
 -g
  : 'RUN: at line 5';   
/home//git/llvm-project/debuginfo-tests/llgdb-tests/test_debuginfo.pl 
/home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp 
/home//git/llvm-project/stage1/projects/debuginfo-tests/llgdb-tests/Output/nrvo-string.cpp.tmp.out
  : 'RUN: at line 6';   /home//git/llvm-project/stage1/bin/clang 
--driver-mode=g++ -O1 -fno-exceptions --target=x86_64-unknown-linux-gnu 
/home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp -o 
/home//git/llvm-project/stage1/projects/debuginfo-tests/llgdb-tests/Output/nrvo-string.cpp.tmp.out
 -g
  : 'RUN: at line 7';   
/home//git/llvm-project/debuginfo-tests/llgdb-tests/test_debuginfo.pl 
/home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp 
/home//git/llvm-project/stage1/projects/debuginfo-tests/llgdb-tests/Output/nrvo-string.cpp.tmp.out
  --
  Exit Code: 1
  
  Command Output (stdout):
  --
  Debugger output was:
  Breakpoint 1 at 0x4004f8: file 
/home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp, line 
23.
  Breakpoint 2 at 0x400563: file 
/home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp, line 
39.
  
  Breakpoint 1, get_string () at 
/home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp:23
  23stop();
  $1 = 3
  
  Breakpoint 2, get_string2 () at 
/home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp:39
  39stop();
  
/home//git/llvm-project/stage1/projects/debuginfo-tests/llgdb-tests/Output/nrvo-string.cpp.debugger.script:6:
 Error in sourced command file:
  There is no member named i.
  
  --
  Command Output (stderr):
  --
  
/home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp:52:11:
 error: CHECK: expected string not found in input
  // CHECK: = 5
^
  
/home//git/llvm-project/stage1/projects/debuginfo-tests/llgdb-tests/Output/nrvo-string.cpp.gdb.output:8:1:
 note: scanning from here
  Breakpoint 2, get_string2 () at 
/home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp:39
  ^
  
/home//git/llvm-project/stage1/projects/debuginfo-tests/llgdb-tests/Output/nrvo-string.cpp.gdb.output:8:10:
 note: possible intended match here
  Breakpoint 2, get_string2 () at 
/home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp:39
   ^
  
  Input file: 
/home//git/llvm-project/stage1/projects/debuginfo-tests/llgdb-tests/Output/nrvo-string.cpp.gdb.output
  Check file: 
/home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp
  
  -dump-input=help explains the following input dump.
  
  Full input was:
  <<
  1: Breakpoint 1 at 0x4004f8: file 
/home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp, line 
23.
  2: Breakpoint 2 at 0x400563: file 
/home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp, line 
39.
  3:
  4: Breakpoint 1, get_string () at 
/home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp:23
  5: 23 stop();
  6: $1 = 3
  7:
  8: Breakpoint 2, get_string2 () at 
/home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp:39
  check:52'0 
X
 error: no match found
  check:52'1  ? 
   possible intended match
  9: 39 stop();
  check:52'0 ~~
 10: 
/home//git/llvm-project/stage1/projects/debuginfo-tests/llgdb-tests/Output/nrvo-string.cpp.debugger.script:6:
 Error in sourced command file:
  check:52'0 
~~~
 11: There is no member named i.
  check:52'0 ~~~
  >>
  
  --


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79147

___
cfe-commits mailin

[PATCH] D66035: [WebAssembly] WIP: Add support for reference types

[Paulo Matos via Phabricator via cfe-commits]

pmatos added a comment.

Please ignore my `.gitlab-ci.yml`. That's just an internal change that I got 
uploaded by mistake.
I am looking to see this through and start discussion on this with the goal of 
landing it.

At the moment, for example this test crashes:

  struct {
__attribute__((address_space(256))) char *a[0];
  } b;
  void c() {
for (int d = 0; d < 10; d++)
  b.a[d] = 0;
  }

This picked up from previous work by @vchuravy. I am still surprised by, and 
don't understand the reasoning behind, using a an i8 * for the externref 
representation. If anything I would expect to see i32 *. 
In any case, the test above crashes in loop vectorization in `TargetLowering.h` 
`getValueType` because externref is casted just fine to a pointer type and it 
shouldn't be since `externref` is supposed to be opaque.

I would be keen to hear some comments and suggestions going forward on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66035

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

[clang] b81fd5a - [clang-format][NFC] Fix a Wdocumentation warning in TokenAnnotator.cpp

[Bruno Ricci via cfe-commits]

Author: Bruno Ricci
Date: 2020-07-28T10:58:52+01:00
New Revision: b81fd5aeecd8047ef62348b67cab2cf9a1577d8e

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

LOG: [clang-format][NFC] Fix a Wdocumentation warning in TokenAnnotator.cpp

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index b19fc34bcc80..6cbaf8a30812 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -27,7 +27,7 @@ namespace format {
 namespace {
 
 /// Returns \c true if the token can be used as an identifier in
-/// an Objective-C \c @selector, \c false otherwise.
+/// an Objective-C \c \@selector, \c false otherwise.
 ///
 /// Because getFormattingLangOpts() always lexes source code as
 /// Objective-C++, C++ keywords like \c new and \c delete are



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

[PATCH] D66035: [WebAssembly] WIP: Add support for reference types

[Paulo Matos via Phabricator via cfe-commits]

pmatos added a comment.

There's also this line of work on opaque types that could be potentially 
interested but seems far from being landed: 
https://groups.google.com/g/llvm-dev/c/Dw_DYSXGFto/m/OzzK-CkGAwAJ


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66035

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

[PATCH] D84461: [Concepts] Fix ast dump for immediately declared constraint.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc0bd9fa137c2: [Concepts] Fix ast dump for immediately 
declared constraint. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84461

Files:
  clang/lib/AST/TextNodeDumper.cpp
  clang/test/AST/ast-dump-concepts.cpp


Index: clang/test/AST/ast-dump-concepts.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-concepts.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -std=c++2a -ast-dump 
-ast-dump-filter Foo %s | FileCheck -strict-whitespace %s
+
+// Test with serialization:
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -x c++ -std=c++20 -triple x86_64-unknown-unknown 
-include-pch %t \
+// RUN: -ast-dump-all -ast-dump-filter Foo /dev/null \
+// RUN: | FileCheck --strict-whitespace %s
+
+template 
+concept binary_concept = true;
+
+template 
+struct Foo {
+  // CHECK:  TemplateTypeParmDecl {{.*}} referenced Concept {{.*}} 
'binary_concept'
+  // CHECK-NEXT: |-ConceptSpecializationExpr {{.*}} 'bool'
+  // CHECK-NEXT: `-TemplateArgument {{.*}} type 'int'
+  template  R>
+  Foo(R);
+};
Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -1994,7 +1994,7 @@
   dumpBareDeclRef(TC->getFoundDecl());
   OS << ")";
 }
-Visit(TC->getImmediatelyDeclaredConstraint());
+AddChild([=] { Visit(TC->getImmediatelyDeclaredConstraint()); });
   } else if (D->wasDeclaredWithTypename())
 OS << " typename";
   else


Index: clang/test/AST/ast-dump-concepts.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-concepts.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -std=c++2a -ast-dump -ast-dump-filter Foo %s | FileCheck -strict-whitespace %s
+
+// Test with serialization:
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -x c++ -std=c++20 -triple x86_64-unknown-unknown -include-pch %t \
+// RUN: -ast-dump-all -ast-dump-filter Foo /dev/null \
+// RUN: | FileCheck --strict-whitespace %s
+
+template 
+concept binary_concept = true;
+
+template 
+struct Foo {
+  // CHECK:  TemplateTypeParmDecl {{.*}} referenced Concept {{.*}} 'binary_concept'
+  // CHECK-NEXT: |-ConceptSpecializationExpr {{.*}} 'bool'
+  // CHECK-NEXT: `-TemplateArgument {{.*}} type 'int'
+  template  R>
+  Foo(R);
+};
Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -1994,7 +1994,7 @@
   dumpBareDeclRef(TC->getFoundDecl());
   OS << ")";
 }
-Visit(TC->getImmediatelyDeclaredConstraint());
+AddChild([=] { Visit(TC->getImmediatelyDeclaredConstraint()); });
   } else if (D->wasDeclaredWithTypename())
 OS << " typename";
   else
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c0bd9fa - [Concepts] Fix ast dump for immediately declared constraint.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2020-07-28T12:10:03+02:00
New Revision: c0bd9fa137c28a3ef833b46b7f9770b060275281

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

LOG: [Concepts] Fix ast dump for immediately declared constraint.

Reviewed By: nridge

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

Added: 
clang/test/AST/ast-dump-concepts.cpp

Modified: 
clang/lib/AST/TextNodeDumper.cpp

Removed: 




diff  --git a/clang/lib/AST/TextNodeDumper.cpp 
b/clang/lib/AST/TextNodeDumper.cpp
index 91b984820cd2..5b6c6085e02c 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -1994,7 +1994,7 @@ void TextNodeDumper::VisitTemplateTypeParmDecl(const 
TemplateTypeParmDecl *D) {
   dumpBareDeclRef(TC->getFoundDecl());
   OS << ")";
 }
-Visit(TC->getImmediatelyDeclaredConstraint());
+AddChild([=] { Visit(TC->getImmediatelyDeclaredConstraint()); });
   } else if (D->wasDeclaredWithTypename())
 OS << " typename";
   else

diff  --git a/clang/test/AST/ast-dump-concepts.cpp 
b/clang/test/AST/ast-dump-concepts.cpp
new file mode 100644
index ..530c1baeffa7
--- /dev/null
+++ b/clang/test/AST/ast-dump-concepts.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -std=c++2a -ast-dump 
-ast-dump-filter Foo %s | FileCheck -strict-whitespace %s
+
+// Test with serialization:
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -x c++ -std=c++20 -triple x86_64-unknown-unknown 
-include-pch %t \
+// RUN: -ast-dump-all -ast-dump-filter Foo /dev/null \
+// RUN: | FileCheck --strict-whitespace %s
+
+template 
+concept binary_concept = true;
+
+template 
+struct Foo {
+  // CHECK:  TemplateTypeParmDecl {{.*}} referenced Concept {{.*}} 
'binary_concept'
+  // CHECK-NEXT: |-ConceptSpecializationExpr {{.*}} 'bool'
+  // CHECK-NEXT: `-TemplateArgument {{.*}} type 'int'
+  template  R>
+  Foo(R);
+};



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

[PATCH] D83961: [Analyzer] Fix bug report source locations in minimal output.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

Then the current solution is good, print always end of the bug path.
A change to the bug reporting component was made that caused reported position 
of bugs to change. New is the end of the path, old is the location set by the 
checker (`BugReport::getLocation` value). The location is often same as end of 
the path. The `RefLeakReport` has a special `getLocation` function and there is 
a difference between this location (that is the allocation site) and the end of 
the path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83961

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

[PATCH] D84473: Dump Accumulator

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

Could the llvm-readobj bit be spun off into a separate review, please. As 
things stand it doesn't have any of its own testing, which should be added at 
the same time as adding support in the tool itself. The new option is also 
lacking any documentation in the llvm-readobj and llvm-readelf docs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84473

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

[PATCH] D84602: [MSP430] Expose msp430_builtin calling convention to C code

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1465
+  let Spellings = [Clang<"msp430_builtin">];
+  let Documentation = [Undocumented];
+}

aaron.ballman wrote:
> No new, undocumented attributes, please. Or is this attribute not expected to 
> be used by users? (If it's not to be used by end users, is there a way we can 
> skip exposing the attribute in the frontend and automatically generate the 
> LLVM calling convention when lowering the builtin?)
Initially, it was not exposed and was just emitted according to the [MSP430 
EABI document](https://www.ti.com/lit/an/slaa534a/slaa534a.pdf), Section 6.3, 
as calls to `libgcc`. Now, when porting the `builtins` library of 
`compiler-rt`, I had to be able to
* define **some of** functions as a library function with a special calling 
convention
* declare external functions to be explicitly called from corresponding unit 
tests

So, it is not expected to mimic some existing attribute or be used by end 
users. Frankly speaking, it is possible it can change the meaning when finally 
wiring together all parts of MSP430 compiler-rt port. Ultimately, this is 
expected to be wired into {D84636}.

At the first glance, msp430-gcc just knows these functions by names and sets CC 
accordingly. This could technically solve my problem as well but looks quite 
hackish to me.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1233
 return llvm::dwarf::DW_CC_LLVM_X86RegCall;
+  // TODO case CC_MSP430Builtin:
   }

aaron.ballman wrote:
> This is likely to cause -Werror failures because the switch won't be fully 
> covered. Are you planning to do this TODO as part of this patch, or in a 
> follow up?
> Are you planning to do this TODO as part of this patch, or in a follow up?

It depends on how large that change would be.

I first need to find out how such calling convention identifiers are issued (or 
maybe there already exist one from GCC). I see they are declared inside the 
`llvm/include/llvm/BinaryFormat/Dwarf.def`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84602

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

[PATCH] D84602: [MSP430] Expose msp430_builtin calling convention to C code

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1465
+  let Spellings = [Clang<"msp430_builtin">];
+  let Documentation = [Undocumented];
+}

atrosinenko wrote:
> aaron.ballman wrote:
> > No new, undocumented attributes, please. Or is this attribute not expected 
> > to be used by users? (If it's not to be used by end users, is there a way 
> > we can skip exposing the attribute in the frontend and automatically 
> > generate the LLVM calling convention when lowering the builtin?)
> Initially, it was not exposed and was just emitted according to the [MSP430 
> EABI document](https://www.ti.com/lit/an/slaa534a/slaa534a.pdf), Section 6.3, 
> as calls to `libgcc`. Now, when porting the `builtins` library of 
> `compiler-rt`, I had to be able to
> * define **some of** functions as a library function with a special calling 
> convention
> * declare external functions to be explicitly called from corresponding unit 
> tests
> 
> So, it is not expected to mimic some existing attribute or be used by end 
> users. Frankly speaking, it is possible it can change the meaning when 
> finally wiring together all parts of MSP430 compiler-rt port. Ultimately, 
> this is expected to be wired into {D84636}.
> 
> At the first glance, msp430-gcc just knows these functions by names and sets 
> CC accordingly. This could technically solve my problem as well but looks 
> quite hackish to me.
> At the first glance, msp430-gcc just knows these functions by names and sets 
> CC accordingly. This could technically solve my problem as well but looks 
> quite hackish to me.

I was thinking that MSP430 would have a BuiltinsMSP430.def file in 
include/clang/Basic, and you could specify the attribute on just the builtins 
that matter from within that file. But I notice there is no such file, so 
perhaps that idea won't work as well for you. If you add such a file, I think 
you'd attach your calling convention attributes somewhere near 
`Sema::AddKnownFunctionAttributes()` but you may have to do some work for the 
function type itself.

If we keep the explicit attribute, I'm fine with it being undocumented if users 
aren't supposed to write it themselves, but would appreciate some comments on 
the attribute explaining that.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:1233
 return llvm::dwarf::DW_CC_LLVM_X86RegCall;
+  // TODO case CC_MSP430Builtin:
   }

atrosinenko wrote:
> aaron.ballman wrote:
> > This is likely to cause -Werror failures because the switch won't be fully 
> > covered. Are you planning to do this TODO as part of this patch, or in a 
> > follow up?
> > Are you planning to do this TODO as part of this patch, or in a follow up?
> 
> It depends on how large that change would be.
> 
> I first need to find out how such calling convention identifiers are issued 
> (or maybe there already exist one from GCC). I see they are declared inside 
> the `llvm/include/llvm/BinaryFormat/Dwarf.def`.
> 
I'm not certain how large the change would be (and I definitely don't insist on 
a full implementation), but you should ensure the switch is fully covered so 
that you don't break bots when the patch lands.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84602

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

[PATCH] D84602: [MSP430] Expose msp430_builtin calling convention to C code

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko added a comment.

In D84602#2176584 , @rjmccall wrote:

> Is there only one special calling convention, or is there any chance that 
> different builtin functions would use different conventions?

It depends on how to define "builtin calling convention". Now it is in fact a 
"CC for builtin functions with two 64-bit arguments listed in Section 6.3 of 
EABI document". MSP430 EABI defines one such CC applied to a small subset of 
compiler helper functions, while others use the traditional CC like all other 
functions.

In D84602#2176592 , @aaron.ballman 
wrote:

> To be clear, I also don't have a problem with it, but if users aren't 
> supposed to be writing this attribute themselves and if we can apply the 
> calling convention from markings in a .def file instead, I think it could be 
> a cleaner approach to go that route instead. There's a lot of "ifs" in that 
> sentence though. :-)

Do you mean detecting these functions by their names, like GCC does? If so, 
that trick would replace all the

- D84602: [MSP430] Expose msp430_builtin calling convention to C code 

- D84605: [IR][MSP430] Expose the "msp430_builtin" calling convention to .ll 

- D84636: [RFC] Make the default LibCall implementations from compiler-rt 
builtins library more customizable 

... provided this is considered as a generally suggested solution across the 
codebase, not a hack :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84602

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

[PATCH] D84453: [clang-tidy] Suppress one unittest on macOS.

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D84453#2177665 , @NoQ wrote:

> In D84453#2171548 , @njames93 wrote:
>
>> I was having issues with this test case under macos in D82188 
>> .
>> It would fail for seemingly no apparent reason until I disable a test in a 
>> different translation unit.
>> This made me think there is a subtle bug in the linker used on macos. That 
>> could also explain why asan is having a hard time with this as well. 
>> I got as far as seeing that runCheckOnCode was called, the check was 
>> instantiated but its matchers never got registered effectively meaning it 
>> didn't run.
>
> Damn, i'm very glad you showed up. I can reproduce your problem as well, so 
> ASan is probably not at fault. I'll try to talk to linker folks to see if 
> they can fix this.
>
> I guess i'll still commit this patch so that to unbreak the buildbots but 
> i'll keep an eye on this issue.

If it is an issue with the linking and the issue lies within the system ld 
binary on mac os then we really can't do much here apart from raise an issue 
with apple folks.
Be interesting to see if this bug is present when using lld on mac os, however 
I don't have a mac so its kind of hard for me to test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84453

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

[PATCH] D82898: [clang-tidy] Handled insertion only fixits when determining conflicts.

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 281194.
njames93 added a comment.

Fix new lines in test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82898

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables-conflict.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables-conflict.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables-conflict.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s 
cppcoreguidelines-init-variables,readability-isolate-declaration %t
+
+void foo() {
+  int A, B, C;
+  // CHECK-MESSAGES-DAG: :[[@LINE-1]]:7: warning: variable 'A' is not 
initialized
+  // CHECK-MESSAGES-DAG: :[[@LINE-2]]:10: warning: variable 'B' is not 
initialized
+  // CHECK-MESSAGES-DAG: :[[@LINE-3]]:13: warning: variable 'C' is not 
initialized
+  // CHECK-MESSAGES-DAG: :[[@LINE-4]]:3: warning: multiple declarations in a 
single statement reduces readability
+
+  // Only the isolate declarations fix-it should be applied
+
+  //  CHECK-FIXES: int A;
+  // CHECK-FIXES-NEXT: int B;
+  // CHECK-FIXES-NEXT: int C;
+}
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -590,6 +590,7 @@
 // An event can be either the begin or the end of an interval.
 enum EventType {
   ET_Begin = 1,
+  ET_Insert = 0,
   ET_End = -1,
 };
 
@@ -623,6 +624,8 @@
   //   disallowing the first one.
   if (Type == ET_Begin)
 Priority = std::make_tuple(Begin, Type, -End, -ErrorSize, ErrorId);
+  else if (Type == ET_Insert)
+Priority = std::make_tuple(Begin, Type, -End, ErrorSize, ErrorId);
   else
 Priority = std::make_tuple(End, Type, -Begin, ErrorSize, ErrorId);
 }
@@ -662,19 +665,19 @@
   }
 
   // Build events from error intervals.
-  std::map> FileEvents;
+  llvm::StringMap> FileEvents;
   for (unsigned I = 0; I < ErrorFixes.size(); ++I) {
 for (const auto &FileAndReplace : *ErrorFixes[I].second) {
   for (const auto &Replace : FileAndReplace.second) {
 unsigned Begin = Replace.getOffset();
 unsigned End = Begin + Replace.getLength();
-const std::string &FilePath = std::string(Replace.getFilePath());
-// FIXME: Handle empty intervals, such as those from insertions.
-if (Begin == End)
-  continue;
-auto &Events = FileEvents[FilePath];
-Events.emplace_back(Begin, End, Event::ET_Begin, I, Sizes[I]);
-Events.emplace_back(Begin, End, Event::ET_End, I, Sizes[I]);
+auto &Events = FileEvents[Replace.getFilePath()];
+if (Begin == End) {
+  Events.emplace_back(Begin, End, Event::ET_Insert, I, Sizes[I]);
+} else {
+  Events.emplace_back(Begin, End, Event::ET_Begin, I, Sizes[I]);
+  Events.emplace_back(Begin, End, Event::ET_End, I, Sizes[I]);
+}
   }
 }
   }
@@ -686,6 +689,11 @@
 llvm::sort(Events);
 int OpenIntervals = 0;
 for (const auto &Event : Events) {
+  if (Event.Type == Event::ET_Insert) {
+if (OpenIntervals != 0)
+  Apply[Event.ErrorId] = false;
+continue;
+  }
   if (Event.Type == Event::ET_End)
 --OpenIntervals;
   // This has to be checked after removing the interval from the count if 
it


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables-conflict.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables-conflict.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables,readability-isolate-declaration %t
+
+void foo() {
+  int A, B, C;
+  // CHECK-MESSAGES-DAG: :[[@LINE-1]]:7: warning: variable 'A' is not initialized
+  // CHECK-MESSAGES-DAG: :[[@LINE-2]]:10: warning: variable 'B' is not initialized
+  // CHECK-MESSAGES-DAG: :[[@LINE-3]]:13: warning: variable 'C' is not initialized
+  // CHECK-MESSAGES-DAG: :[[@LINE-4]]:3: warning: multiple declarations in a single statement reduces readability
+
+  // Only the isolate declarations fix-it should be applied
+
+  //  CHECK-FIXES: int A;
+  // CHECK-FIXES-NEXT: int B;
+  // CHECK-FIXES-NEXT: int C;
+}
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -590,6 +590,7 @@
 // An event can be either the b

[PATCH] D84743: [Clang][AMDGCN] Universal device offloading macros header

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam created this revision.
saiislam added reviewers: jdoerfert, ABataev, JonChesterfield.
Herald added subscribers: cfe-commits, Anastasia.
Herald added a project: clang.
Harbormaster failed remote builds in B65994: Diff 281183!
Harbormaster returned this revision to the author for changes because remote 
builds failed.
saiislam updated this revision to Diff 281189.
saiislam added a comment.
saiislam requested review of this revision.
Herald added a subscriber: sstefan1.

Fixed clang-tidy warnings.


This header creates macros  _DEVICE_ARCH and _DEVICE_GPU with values. This
header exists because compiler macros are inconsistent in specifying if a
compiliation is a device pass or a host pass. There is also inconsistency in
how the device architecture and type are specified during a device pass. The
inconsistencies are between OpenMP, CUDA, HIP, and OpenCL. The macro logic
in this header is aware of these inconsistencies and sets useful values for
_DEVICE_ARCH and _DEVICE_GPU during a device compilation. The macros will
not be defined during a host compilation pass. So "#ifndef _DEVICE_ARCH" can
be used by users to imply a host compilation. This header must remain a
preprocessing header only because it is intended to be used by different
languages.

Originally authored by Greg Rodgers (@gregrodgers).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84743

Files:
  clang/lib/Headers/offload_macros.h

Index: clang/lib/Headers/offload_macros.h
===
--- /dev/null
+++ clang/lib/Headers/offload_macros.h
@@ -0,0 +1,118 @@
+//===--- offload_macros.h - Universal _DEVICE Offloading Macros Header ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===---===
+//
+// This header creates macros  _DEVICE_ARCH and _DEVICE_GPU with values. This
+// header exists because compiler macros are inconsistent in specifying if a
+// compiliation is a device pass or a host pass. There is also inconsistency in
+// how the device architecture and type are specified during a device pass. The
+// inconsistencies are between OpenMP, CUDA, HIP, and OpenCL. The macro logic
+// in this header is aware of these inconsistencies and sets useful values for
+// _DEVICE_ARCH and _DEVICE_GPU during a device compilation. The macros will
+// not be defined during a host compilation pass. So "#ifndef _DEVICE_ARCH" can
+// be used by users to imply a host compilation. This header must remain a
+// preprocessing header only because it is intended to be used by different
+// languages.
+//
+//===--===//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_HEADERS_OFFLOAD_MACROS_H
+#define LLVM_CLANG_LIB_HEADERS_OFFLOAD_MACROS_H
+
+#undef _DEVICE_GPU
+#undef _DEVICE_ARCH
+
+#if defined(_OPENMP)
+// OpenMP does not set architecture macros on host pass.
+// So if either set, this is an OpenMP  device pass.
+#if defined(__AMDGCN__) || defined(__NVPTX__)
+#if defined(__AMDGCN__)
+#define _DEVICE_ARCH amdgcn
+// _DEVICE_GPU set below
+#else
+#define _DEVICE_ARCH nvptx64
+#define _DEVICE_GPU __CUDA_ARCH__
+#endif
+#endif
+#elif defined(__CUDA_ARCH__)
+// CUDA sets macros __NVPTX__ on host pass. So use __CUDA_ARCH__
+// to determine if this is device pass.
+#define _DEVICE_ARCH nvptx64
+#define _DEVICE_GPU __CUDA_ARCH__
+#elif defined(__HIP_DEVICE_COMPILE__)
+// HIP sets macros __AMDGCN__ on host pass. So use __HIP_DEVICE_COMPILE__
+// to determine if this is device pass.
+#define _DEVICE_ARCH amdgcn
+// _DEVICE_GPU set below
+#elif defined(__SYCL_DEVICE_ONLY__)
+#if defined(__AMDGCN__)
+#define _DEVICE_ARCH amdgcn
+// _DEVICE_GPU set below
+#else
+#define _DEVICE_ARCH nvptx64
+#define _DEVICE_GPU __CUDA_ARCH__
+#endif
+#elif defined(__OPENCL_C_VERSION__) || defined(__OPENCL_CPP_VERSION__)
+#if defined(__AMDGCN__)
+#define _DEVICE_ARCH amdgcn
+// _DEVICE_GPU set below
+#endif
+#if defined(__NVPTX__)
+#define _DEVICE_ARCH nvptx64
+#define _DEVICE_GPU __CUDA_ARCH__
+#endif
+#endif
+
+#if defined(_DEVICE_ARCH) && (_DEVICE_ARCH == amdgcn)
+// AMD uses binary macros only, so create a value for _DEVICE_GPU
+#if defined(__gfx906__)
+#define _DEVICE_GPU 9060
+#elif defined(__gfx900__)
+#define _DEVICE_GPU 9000
+#elif defined(__gfx601__)
+#define _DEVICE_GPU 6010
+#elif defined(__gfx700__)
+#define _DEVICE_GPU 7000
+#elif defined(__gfx701__)
+#define _DEVICE_GPU 7010
+#elif defined(__gfx702__)
+#define _DEVICE_GPU 7020
+#elif defined(__gfx703__)
+#define _DEVICE_GPU 7030
+#elif defined(__gfx801__)
+#define _DEVICE_GPU 8010
+#elif defined(__gfx802__)
+#define _DEVICE_GPU 8020
+#elif defined(__gfx803__)
+#define _DEVICE_GPU 8030
+#e

[PATCH] D77150: [Analyzer] New Option for ContainerModeling: AggressiveEraseModeling

[Kristóf Umann via Phabricator via cfe-commits]

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

Let me double down -- just because we let some select people know about an 
option, I still don't think it should be user facing. I'm strongly against the 
philosophy that a core-modifying decision, such as configuring a state split 
should be presented on the default interface. Having "smart users" isn't an 
excuse to that either -- the last thing I'd like to see broadcasted is that you 
need to be a genius to use or configure a tool that is suppose to serve you, 
not the other way around.




Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:620-624
+  "AggressiveEraseModeling",
+  "Enables exploration of the past-the-end branch for the "
+  "return value of the erase() method of containers.",
+  "false",
+  Released>

NoQ wrote:
> Szelethus wrote:
> > Hmm. The description isn't really user-friendly, imo, but the option 
> > doesn't sound too user-facing either. I don't think this is an option to be 
> > tinkered with. I think we should make this hidden.
> > 
> > Also, even for developers, I find this a bitconfusing at first. Do we not 
> > enable that by default? Why not? What do we have to gain/lose?
> What is the rule that the user needs to follow in order to avoid the warning? 
> "Always check the return value of `erase()` before use"? If so, a good 
> description would be along the lines of "Warn when the return value of 
> `erase()` is not checked before use; compare that value to end() in order to 
> suppress the warning" (or something more specific).
Please mark this hidden.


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

https://reviews.llvm.org/D77150

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

[PATCH] D84743: [Clang][AMDGCN] Universal device offloading macros header

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a reviewer: arsenm.
JonChesterfield added a comment.
Herald added a subscriber: wdng.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84743

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

[PATCH] D84602: [MSP430] Expose msp430_builtin calling convention to C code

[Anatoly Trosinenko via Phabricator via cfe-commits]

atrosinenko updated this revision to Diff 281198.
atrosinenko added a comment.

Fix some review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84602

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/Specifiers.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/MSP430.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/msp430-cc-builtin.c
  clang/test/Sema/attr-msp430.c
  clang/tools/libclang/CXType.cpp
  llvm/lib/Target/MSP430/MSP430ISelLowering.cpp

Index: llvm/lib/Target/MSP430/MSP430ISelLowering.cpp
===
--- llvm/lib/Target/MSP430/MSP430ISelLowering.cpp
+++ llvm/lib/Target/MSP430/MSP430ISelLowering.cpp
@@ -573,6 +573,7 @@
 report_fatal_error("Unsupported calling convention");
   case CallingConv::C:
   case CallingConv::Fast:
+  case CallingConv::MSP430_BUILTIN:
 return LowerCCCArguments(Chain, CallConv, isVarArg, Ins, dl, DAG, InVals);
   case CallingConv::MSP430_INTR:
 if (Ins.empty())
Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -666,6 +666,7 @@
   TCALLINGCONV(Swift);
   TCALLINGCONV(PreserveMost);
   TCALLINGCONV(PreserveAll);
+case CC_MSP430Builtin: return CXCallingConv_Unexposed;
 case CC_SpirFunction: return CXCallingConv_Unexposed;
 case CC_OpenCLKernel: return CXCallingConv_Unexposed;
   break;
Index: clang/test/Sema/attr-msp430.c
===
--- clang/test/Sema/attr-msp430.c
+++ clang/test/Sema/attr-msp430.c
@@ -11,3 +11,7 @@
 
 __attribute__((interrupt(0))) void f6(void);
 __attribute__((interrupt(63))) void f7(void);
+
+__attribute__((msp430_builtin)) int t2; // expected-warning {{'msp430_builtin' only applies to function types; type here is 'int'}}
+__attribute__((msp430_builtin(1))) void f8(long long a, long long b); // expected-error {{'msp430_builtin' attribute takes no arguments}}
+__attribute__((msp430_builtin)) void f9(long long a, long long b);
Index: clang/test/CodeGen/msp430-cc-builtin.c
===
--- /dev/null
+++ clang/test/CodeGen/msp430-cc-builtin.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple msp430 -emit-llvm < %s | FileCheck %s
+
+__attribute__((msp430_builtin)) int f(long long x, long long y);
+
+__attribute__((msp430_builtin)) int g(long long x, long long y) {
+  return (int)42;
+}
+// CHECK: define cc94 {{(dso_local )?}}i16 @g(i64 %x, i64 %y)
+
+int caller()
+{
+  return f(0, 0);
+// CHECK: call cc94 i16 @f
+}
+
+// CHECK: declare cc94 {{(dso_local )?}}i16 @f(i64, i64)
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -124,7 +124,8 @@
   case ParsedAttr::AT_Pcs: \
   case ParsedAttr::AT_IntelOclBicc:\
   case ParsedAttr::AT_PreserveMost:\
-  case ParsedAttr::AT_PreserveAll
+  case ParsedAttr::AT_PreserveAll: \
+  case ParsedAttr::AT_MSP430Builtin
 
 // Function type attributes.
 #define FUNCTION_TYPE_ATTRS_CASELIST   \
@@ -7248,6 +7249,8 @@
 return createSimpleAttr(Ctx, Attr);
   case ParsedAttr::AT_PreserveAll:
 return createSimpleAttr(Ctx, Attr);
+  case ParsedAttr::AT_MSP430Builtin:
+return createSimpleAttr(Ctx, Attr);
   }
   llvm_unreachable("unexpected attribute kind!");
 }
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4459,6 +4459,9 @@
   case ParsedAttr::AT_PreserveAll:
 D->addAttr(::new (S.Context) PreserveAllAttr(S.Context, AL));
 return;
+  case ParsedAttr::AT_MSP430Builtin:
+D->addAttr(::new (S.Context) MSP430BuiltinAttr(S.Context, AL));
+return;
   default:
 llvm_unreachable("unexpected attribute kind");
   }
@@ -4624,6 +4627,9 @@
   case ParsedAttr::AT_PreserveAll:
 CC = CC_PreserveAll;
 break;
+  case ParsedAttr::AT_MSP430Builtin:
+CC = CC_MSP430Builtin;
+break;
   default: llvm_unreachable("unexpected attribute kind");
   }
 
@@ -7252,6 +7258,7 @@
   case ParsedAttr::AT_PreserveMost:
   case ParsedAttr::AT_PreserveAll:
   case ParsedAttr::AT_AArch64VectorPcs:
+  case ParsedAttr::AT_MSP430Builtin:
 handleCallConvAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Suppress:
Index: clang/lib/CodeGen/CGDebug

[PATCH] D84602: [MSP430] Expose msp430_builtin calling convention to C code

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D84602#2178328 , @atrosinenko wrote:

> In D84602#2176592 , @aaron.ballman 
> wrote:
>
>> To be clear, I also don't have a problem with it, but if users aren't 
>> supposed to be writing this attribute themselves and if we can apply the 
>> calling convention from markings in a .def file instead, I think it could be 
>> a cleaner approach to go that route instead. There's a lot of "ifs" in that 
>> sentence though. :-)
>
> Do you mean detecting these functions by their names, like GCC does? If so, 
> that trick would replace all the
>
> - D84602: [MSP430] Expose msp430_builtin calling convention to C code 
> 
> - D84605: [IR][MSP430] Expose the "msp430_builtin" calling convention to .ll 
> 
> - D84636: [RFC] Make the default LibCall implementations from compiler-rt 
> builtins library more customizable 
>
> ... provided this is considered as a generally suggested solution across the 
> codebase, not a hack :)

I was envisioning specifying the builtins in a .def file like we do with other 
builtins, but allowing that .def file to specify optional calling convention 
information on a per-function basis (like we already support other attributes). 
So there would still be an LLVM attribute to be used, but there would not be a 
user-facing Clang attribute that users could misapply to their own code. So the 
attribute definition here would continue to exist, but the attribute would have 
no spelling associated with it. We do this with some other attributes, like 
`AlignMac68kAttr` or `MaxFieldAlignmentAttr`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84602

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

[PATCH] D82898: [clang-tidy] Handled insertion only fixits when determining conflicts.

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 281200.
njames93 added a comment.

Replace if/else logic with switches


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82898

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables-conflict.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables-conflict.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-init-variables-conflict.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-init-variables,readability-isolate-declaration %t
+
+void foo() {
+  int A, B, C;
+  // CHECK-MESSAGES-DAG: :[[@LINE-1]]:7: warning: variable 'A' is not initialized
+  // CHECK-MESSAGES-DAG: :[[@LINE-2]]:10: warning: variable 'B' is not initialized
+  // CHECK-MESSAGES-DAG: :[[@LINE-3]]:13: warning: variable 'C' is not initialized
+  // CHECK-MESSAGES-DAG: :[[@LINE-4]]:3: warning: multiple declarations in a single statement reduces readability
+
+  // Only the isolate declarations fix-it should be applied
+
+  //  CHECK-FIXES: int A;
+  // CHECK-FIXES-NEXT: int B;
+  // CHECK-FIXES-NEXT: int C;
+}
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -31,6 +31,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Regex.h"
 #include 
@@ -590,6 +591,7 @@
 // An event can be either the begin or the end of an interval.
 enum EventType {
   ET_Begin = 1,
+  ET_Insert = 0,
   ET_End = -1,
 };
 
@@ -621,10 +623,18 @@
   //   one will be processed before, disallowing the second one, and the
   //   end point of the first one will also be processed before,
   //   disallowing the first one.
-  if (Type == ET_Begin)
+  switch (Type) {
+  case ET_Begin:
 Priority = std::make_tuple(Begin, Type, -End, -ErrorSize, ErrorId);
-  else
+return;
+  case ET_Insert:
+Priority = std::make_tuple(Begin, Type, -End, ErrorSize, ErrorId);
+return;
+  case ET_End:
 Priority = std::make_tuple(End, Type, -Begin, ErrorSize, ErrorId);
+return;
+  }
+  llvm_unreachable("Invalid EventType");
 }
 
 bool operator<(const Event &Other) const {
@@ -662,19 +672,19 @@
   }
 
   // Build events from error intervals.
-  std::map> FileEvents;
+  llvm::StringMap> FileEvents;
   for (unsigned I = 0; I < ErrorFixes.size(); ++I) {
 for (const auto &FileAndReplace : *ErrorFixes[I].second) {
   for (const auto &Replace : FileAndReplace.second) {
 unsigned Begin = Replace.getOffset();
 unsigned End = Begin + Replace.getLength();
-const std::string &FilePath = std::string(Replace.getFilePath());
-// FIXME: Handle empty intervals, such as those from insertions.
-if (Begin == End)
-  continue;
-auto &Events = FileEvents[FilePath];
-Events.emplace_back(Begin, End, Event::ET_Begin, I, Sizes[I]);
-Events.emplace_back(Begin, End, Event::ET_End, I, Sizes[I]);
+auto &Events = FileEvents[Replace.getFilePath()];
+if (Begin == End) {
+  Events.emplace_back(Begin, End, Event::ET_Insert, I, Sizes[I]);
+} else {
+  Events.emplace_back(Begin, End, Event::ET_Begin, I, Sizes[I]);
+  Events.emplace_back(Begin, End, Event::ET_End, I, Sizes[I]);
+}
   }
 }
   }
@@ -686,14 +696,21 @@
 llvm::sort(Events);
 int OpenIntervals = 0;
 for (const auto &Event : Events) {
-  if (Event.Type == Event::ET_End)
---OpenIntervals;
-  // This has to be checked after removing the interval from the count if it
-  // is an end event, or before adding it if it is a begin event.
-  if (OpenIntervals != 0)
-Apply[Event.ErrorId] = false;
-  if (Event.Type == Event::ET_Begin)
-++OpenIntervals;
+  switch (Event.Type) {
+  case Event::ET_Begin:
+if (OpenIntervals++ != 0)
+  Apply[Event.ErrorId] = false;
+continue;
+  case Event::ET_Insert:
+if (OpenIntervals != 0)
+  Apply[Event.ErrorId] = false;
+continue;
+  case Event::ET_End:
+if (--OpenIntervals != 0)
+  Apply[Event.ErrorId] = false;
+continue;
+  }
+  llvm_unreachable("Invalid EventType");
 }
 assert(OpenIntervals == 0 && "Amount of begin/end points doesn't match");
   }
___
cfe-

[PATCH] D84678: [clang] False line number in a function definition with "void" parameter

[Jaydeep Chauhan via Phabricator via cfe-commits]

Jac1494 updated this revision to Diff 281202.
Jac1494 added a comment.

Addressed @riccibruno  comments.Thanks


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

https://reviews.llvm.org/D84678

Files:
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/void-argument.cpp


Index: clang/test/Sema/void-argument.cpp
===
--- /dev/null
+++ clang/test/Sema/void-argument.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+void foo(
+void a, // expected-error{{'void' must be the first and only parameter if 
specified}}
+double b,
+int c,
+void d, // expected-error{{'void' must be the first and only parameter if 
specified}}
+int e,
+void f) // expected-error{{'void' must be the first and only parameter if 
specified}}
+{}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5109,7 +5109,7 @@
 // is an incomplete type (C99 6.2.5p19) and function decls cannot
 // have parameters of incomplete type.
 if (FTI.NumParams != 1 || FTI.isVariadic) {
-  S.Diag(DeclType.Loc, diag::err_void_only_param);
+  S.Diag(FTI.Params[i].IdentLoc, diag::err_void_only_param);
   ParamTy = Context.IntTy;
   Param->setType(ParamTy);
 } else if (FTI.Params[i].Ident) {


Index: clang/test/Sema/void-argument.cpp
===
--- /dev/null
+++ clang/test/Sema/void-argument.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+void foo(
+void a, // expected-error{{'void' must be the first and only parameter if specified}}
+double b,
+int c,
+void d, // expected-error{{'void' must be the first and only parameter if specified}}
+int e,
+void f) // expected-error{{'void' must be the first and only parameter if specified}}
+{}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5109,7 +5109,7 @@
 // is an incomplete type (C99 6.2.5p19) and function decls cannot
 // have parameters of incomplete type.
 if (FTI.NumParams != 1 || FTI.isVariadic) {
-  S.Diag(DeclType.Loc, diag::err_void_only_param);
+  S.Diag(FTI.Params[i].IdentLoc, diag::err_void_only_param);
   ParamTy = Context.IntTy;
   Param->setType(ParamTy);
 } else if (FTI.Params[i].Ident) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80917: [OpenMP][AMDGCN] Support OpenMP offloading for AMDGCN architecture - Part 2

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h:66
+  /// for NVPTX.
+  GV_Warp_Size_32,
+  /// The number of bits required to represent the max number of threads in 
warp

What's the point of warp_size_32? It's always set to 32 and seems redundant 
with warp_size



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h:68
+  /// The number of bits required to represent the max number of threads in 
warp
+  GV_Warp_Size_Log2,
+  /// GV_Warp_Size * GV_Slot_Size,

log2 of a compile time constant would be computed at compile time, we don't 
need this field



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h:76
+  /// (~0u >> (GV_Warp_Size - GV_Warp_Size_Log2))
+  GV_Warp_Size_Log2_Mask,
+  // An alternative to the heavy data sharing infrastructure that uses global

As above, this can be computed at compile time, e.g. from that expression


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80917

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

[PATCH] D81865: [clang] Use string tables for static diagnostic descriptions

[Nathan Froyd via Phabricator via cfe-commits]

froydnj added a comment.

In D81865#2176589 , @dblaikie wrote:

> I believe this falls under the (using cppreference ( 
> https://en.cppreference.com/w/cpp/language/union ) , since it's a bit easier 
> to read) UB clause: " It's undefined behavior to read from the member of the 
> union that wasn't most recently written. Many compilers implement, as a 
> non-standard language extension, the ability to read inactive members of a 
> union."
>
> Last member written to was the "StringTable" member, but then it's read from 
> the "String" member, so that'd be UB. Commonly supported, but UB - not sure 
> if we have a general statement that we depend on this behavior in LLVM, even 
> though it's non-standard (but it's possible that we do make such an 
> assumption about the compiler that's building LLVM). It'd be nice to avoid 
> that, though - and not too difficult, I think - I /believe/ it's valid to 
> take a pointer to an object, cast it to char*, compute a pointer to some 
> specific member and then cast it back to the right type and access. But I 
> could be wrong there. @rsmith would be the person to give an authoritative 
> answer.

Thanks for the explanation.  Is the language of "writing" applicable here, 
given that this is a constant blob of storage?  (I suppose the compiler is 
permitted to designate a particular member as having been "written"?)

Would it be more palatable to write:

  struct StaticDiagInfoDescriptionStringTable {
// members as char[] for each diagnostic
  };
  
  const StaticDiagInfoDescriptionStringTable StaticDiagInfoDescriptions = {
// define all the members
  };
  
  ...
  
  struct StaticDiagInfoRec {
...
StringRef getDescription() const {
  size_t MyIndex = this - &StaticDiagInfo[0];
  uint32_t StringOffset = StaticDiagInfoDescriptionOffsets[MyIndex];
  // Defined as a pointer to the first member, and (presumably) there is no 
internal padding.
  const char *StringTable = reinterpret_cast(&StaticDiagInfoDescriptions);
  return StringRef(&StringTable[StringOffset], DescriptionLen);
  };

and then we don't have to care about how the host compiler interprets access to 
different members of unions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81865

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

[PATCH] D66035: [WebAssembly] WIP: Add support for reference types

[Paulo Matos via Phabricator via cfe-commits]

pmatos added a comment.

Related work: https://reviews.llvm.org/D81977


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66035

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

[PATCH] D84182: [OPENMP]Fix PR46012: declare target pointer cannot be accessed in target region.

[Ye Luo via Phabricator via cfe-commits]

ye-luo added a comment.

In D84182#2173578 , @grokos wrote:

> @ABataev:
>
> After this patch was committed, I tried to run the following example:
>
>   #include 
>   
>   int *yptr;
>   
>   int main() {
> int y[10];
> y[1] = 1;
> yptr = &y[0];
>   
> printf("&yptr = %p\n", &yptr);
> printf("&y[0] = %p\n", &y[0]);
>   
> #pragma omp target data map(to: yptr[0:5])
> #pragma omp target
> {
>   printf("y = %d\n", yptr[1]);
>   yptr[1] = 10;
>   printf("y = %d\n", yptr[1]);
> }
>   
> printf("y = %d\n", yptr[1]);
> return 0;
>   }
>
> The arguments clang generates are:
>
>   1) base = &y[0], begin = &yptr, size = 8, type = TARGET_PARAM | TO
>   2) base = &yptr, begin = &y[0], size = 8, type = PTR_AND_OBJ | TO
>
> The second argument is correct, the first argument doesn't make much sense. I 
> believe it should have its base set to &yptr, not &y[0].
> y[0] is not the base for anything, it's only the pointee object.

I hit the same issue and it results incorrect mapping.
https://bugs.llvm.org/show_bug.cgi?id=46868
In addition, only yptr is touched inside the target region. After this patch, 
two mappings instead of one occurs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84182

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

[PATCH] D84182: [OPENMP]Fix PR46012: declare target pointer cannot be accessed in target region.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D84182#2178560 , @ye-luo wrote:

> In D84182#2173578 , @grokos wrote:
>
>> @ABataev:
>>
>> After this patch was committed, I tried to run the following example:
>>
>>   #include 
>>   
>>   int *yptr;
>>   
>>   int main() {
>> int y[10];
>> y[1] = 1;
>> yptr = &y[0];
>>   
>> printf("&yptr = %p\n", &yptr);
>> printf("&y[0] = %p\n", &y[0]);
>>   
>> #pragma omp target data map(to: yptr[0:5])
>> #pragma omp target
>> {
>>   printf("y = %d\n", yptr[1]);
>>   yptr[1] = 10;
>>   printf("y = %d\n", yptr[1]);
>> }
>>   
>> printf("y = %d\n", yptr[1]);
>> return 0;
>>   }
>>
>> The arguments clang generates are:
>>
>>   1) base = &y[0], begin = &yptr, size = 8, type = TARGET_PARAM | TO
>>   2) base = &yptr, begin = &y[0], size = 8, type = PTR_AND_OBJ | TO
>>
>> The second argument is correct, the first argument doesn't make much sense. 
>> I believe it should have its base set to &yptr, not &y[0].
>> y[0] is not the base for anything, it's only the pointee object.
>
> I hit the same issue and it results incorrect mapping.
> https://bugs.llvm.org/show_bug.cgi?id=46868
> In addition, only yptr is touched inside the target region. After this patch, 
> two mappings instead of one occurs.

The fix is almost ready, working on tests update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84182

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

[PATCH] D84554: Use INTERFACE_COMPILE_OPTIONS to disable -Wsuggest-override for any target that links to gtest

[Pavel Labath via Phabricator via cfe-commits]

labath added a comment.

In D84554#2176041 , @logan-5 wrote:

> In D84554#2175012 , @labath wrote:
>
>> Could you elaborate on the lldb issue? I'd like to take a look at that...
>
> It looks like lldb/unittests/TestingSupport/CMakeLists.txt and 
> lldb/unittests/TestingSupport/Symbol/CMakeLists.txt both both manually pull 
> in the googletest/googlemock headers via `include_directories`. It also 
> seems, although I'm not sure, like they don't themselves link to gtest, since 
> simply removing those `include_directories` didn't work.

Thanks. I think I understand the problem now. Those libraries should be linking 
against gtest, but they don't do that because they could get away with not 
doing it. And they add include directories manually because they do not get 
that from linking against gtest. That means we have more things to fix => 
D84748 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84554

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

[clang-tools-extra] 7bae318 - [clang-tidy][NFC] Make OptionsView methods as const where missing

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-07-28T14:52:43+01:00
New Revision: 7bae3188e08746566733148a4ceccdb3cf24e93b

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

LOG: [clang-tidy][NFC] Make OptionsView methods as const where missing

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
clang-tools-extra/clang-tidy/ClangTidyCheck.h

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp 
b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
index c24b8553999c..ffd5bf974ba2 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
@@ -168,10 +168,9 @@ void ClangTidyCheck::OptionsView::store(
   store(Options, LocalName, Value ? StringRef("true") : StringRef("false"));
 }
 
-llvm::Expected
-ClangTidyCheck::OptionsView::getEnumInt(StringRef LocalName,
-ArrayRef Mapping,
-bool CheckGlobal, bool IgnoreCase) {
+llvm::Expected ClangTidyCheck::OptionsView::getEnumInt(
+StringRef LocalName, ArrayRef Mapping, bool CheckGlobal,
+bool IgnoreCase) const {
   auto Iter = CheckGlobal
   ? findPriorityOption(CheckOptions, NamePrefix, LocalName)
   : CheckOptions.find((NamePrefix + LocalName).str());

diff  --git a/clang-tools-extra/clang-tidy/ClangTidyCheck.h 
b/clang-tools-extra/clang-tidy/ClangTidyCheck.h
index 54b725126752..4df8071c841e 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyCheck.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyCheck.h
@@ -330,7 +330,7 @@ class ClangTidyCheck : public 
ast_matchers::MatchFinder::MatchCallback {
 /// supply the mapping required to convert between ``T`` and a string.
 template 
 std::enable_if_t::value, llvm::Expected>
-get(StringRef LocalName, bool IgnoreCase = false) {
+get(StringRef LocalName, bool IgnoreCase = false) const {
   if (llvm::Expected ValueOr =
   getEnumInt(LocalName, typeEraseMapping(), false, IgnoreCase))
 return static_cast(*ValueOr);
@@ -349,7 +349,7 @@ class ClangTidyCheck : public 
ast_matchers::MatchFinder::MatchCallback {
 /// supply the mapping required to convert between ``T`` and a string.
 template 
 std::enable_if_t::value, T>
-get(StringRef LocalName, T Default, bool IgnoreCase = false) {
+get(StringRef LocalName, T Default, bool IgnoreCase = false) const {
   if (auto ValueOr = get(LocalName, IgnoreCase))
 return *ValueOr;
   else
@@ -370,8 +370,7 @@ class ClangTidyCheck : public 
ast_matchers::MatchFinder::MatchCallback {
 /// supply the mapping required to convert between ``T`` and a string.
 template 
 std::enable_if_t::value, llvm::Expected>
-getLocalOrGlobal(StringRef LocalName,
- bool IgnoreCase = false) {
+getLocalOrGlobal(StringRef LocalName, bool IgnoreCase = false) const {
   if (llvm::Expected ValueOr =
   getEnumInt(LocalName, typeEraseMapping(), true, IgnoreCase))
 return static_cast(*ValueOr);
@@ -391,7 +390,8 @@ class ClangTidyCheck : public 
ast_matchers::MatchFinder::MatchCallback {
 /// supply the mapping required to convert between ``T`` and a string.
 template 
 std::enable_if_t::value, T>
-getLocalOrGlobal(StringRef LocalName, T Default, bool IgnoreCase = false) {
+getLocalOrGlobal(StringRef LocalName, T Default,
+ bool IgnoreCase = false) const {
   if (auto ValueOr = getLocalOrGlobal(LocalName, IgnoreCase))
 return *ValueOr;
   else
@@ -420,7 +420,8 @@ class ClangTidyCheck : public 
ast_matchers::MatchFinder::MatchCallback {
 /// supply the mapping required to convert between ``T`` and a string.
 template 
 std::enable_if_t::value>
-store(ClangTidyOptions::OptionMap &Options, StringRef LocalName, T Value) {
+store(ClangTidyOptions::OptionMap &Options, StringRef LocalName,
+  T Value) const {
   ArrayRef> Mapping =
   OptionEnumMapping::getEnumMapping();
   auto Iter = llvm::find_if(
@@ -436,11 +437,11 @@ class ClangTidyCheck : public 
ast_matchers::MatchFinder::MatchCallback {
 
 llvm::Expected getEnumInt(StringRef LocalName,
ArrayRef Mapping,
-   bool CheckGlobal, bool IgnoreCase);
+   bool CheckGlobal, bool IgnoreCase) 
const;
 
 template 
 std::enable_if_t::value, std::vector>
-typeEraseMapping() {
+typeEraseMapping() const {
   ArrayRef> Mapping =
   OptionEnumMapping::getEnumMapping();
   std::vector Result;



___
cfe-commits mailing list
cfe-commit

[PATCH] D84743: [Clang][AMDGCN] Universal device offloading macros header

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added reviewers: tra, MaskRay.
jdoerfert added subscribers: MaskRay, tra.
jdoerfert added a comment.

I like this. I hope this is the start of splitting the `__cuda` headers into 
generic and specific code, right?  @tra @MaskRay any objections on the 
direction?




Comment at: clang/lib/Headers/offload_macros.h:22
+//===--===//
+//===--===//
+

Nit duplicate



Comment at: clang/lib/Headers/offload_macros.h:69
+#endif
+#endif
+

I guess the pattern
```
#if defined(__AMDGCN__)
#define _DEVICE_ARCH amdgcn
// _DEVICE_GPU set below
#endif
#if defined(__NVPTX__)
#define _DEVICE_ARCH nvptx64
#define _DEVICE_GPU __CUDA_ARCH__
#endif
```
is repeating here but it might make sense to lists all the cases one by one 
instead of a single conditional, e.g., `ifdef OPENMP || SYCL || OPENCL || ...`




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84743

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

[PATCH] D84534: [AIX] Static init frontend recovery and backend support

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L marked 5 inline comments as done.
Xiangling_L added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4609
+// their own llvm.global_dtors entry.
+CGM.AddCXXStermFinalizerToGlobalDtor(StermFinalizer, 65535);
+  else

jasonliu wrote:
> Handling template instantiation seems fairly orthogonal to "moving naming 
> logic from frontend to backend". Could we put it in a separate patch (which 
> could be a child of this one)? 
The reason I chose to handle template instantiation and inline variable in this 
patch is that I want to show the scenarios where we have separate 
initialization. And this is also the reason why we want to embed array index 
into sinit/sterm functions. That is, if we move this part into a separate 
patch, I am worried that ppl will feel it's weird to embed index into function 
names.



Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2165
+  emitXXStructor(DL, S.Priority, index, S.Func, isCtor);
+  ++index;
+  continue;

jasonliu wrote:
> Maybe a naive quesiton, in what situation would we have name collision and 
> need `index` as suffix to differentiate?
> Could we just report_fatal_error in those situation?
As far as I know, there are several situations we would have separate 
initialization without considering priority:
1) template specialization
2) inline variable
3) ctor/dtor attribute functions

By embedding the index, we can group those sinit/sterm with same priority 
within current file together.

And as I mentioned above, I chose to handle the former two cases in this patch 
to show why we need to embed the index.

And regarding the third scenario, we've already report_fatal_error on those 
cases.



Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1848
+// beforing emitting them.
+if (isSpecialLLVMGlobalArrayForStaticInit(&G)) {
+  if (GlobalUniqueModuleId.empty()) {

jasonliu wrote:
> This would have conflict with D84363. You might want to rebase it later. 
Thanks for the reminder, I will update the patch accordingly.



Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1935
+  Function *Func = cast(CV);
+  Func->setLinkage(GlobalValue::ExternalLinkage);
+  Func->setName((isCtor ? llvm::Twine("__sinit") : llvm::Twine("__sterm")) +

jasonliu wrote:
> Changing Function name and linkage underneath looks a bit scary. People would 
> have a hard time to map IR to final assembly that gets produced. Have you 
> thought about creating an alias (with the correct linkage and name) to the 
> original function instead?
Good point. I will adjust the implementation here.


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

https://reviews.llvm.org/D84534

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

[PATCH] D84736: [analyzer][RFC] Handle pointer difference of ElementRegion and SymbolicRegion

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

steakhal created this revision.
steakhal added reviewers: NoQ, vsavchenko, martong, baloghadamsoftware, 
ASDenysPetrov, xazax.hun, Szelethus.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, whisperity.
Herald added a project: clang.
Harbormaster failed remote builds in B65978: Diff 281152!
Harbormaster returned this revision to the author for changes because remote 
builds failed.
steakhal requested review of this revision.
steakhal added a comment.

Manually force **request review**, to start a discussion about this.

Although I managed to implement this in a way to pass the tests, but the logic 
became somewhat messy, that's why I stick to this //dummy// patch.
I'm gonna clean that up and update the diff, if you think that such handing of 
memregions are valid.
Sorry for the `cfe-dev` spam, I did not know that I can manually //request 
review//.


Currently, if the analyzer evaluates an expression like `to - from`, it only 
computes reasonable answer if both are representing ElementRegions.

Formally, **for all** memory region `X` and **for all** SVal offset `Y`:
`&Element{SymRegion{X},Y,char} - &SymRegion{X}` => `Y`
The same for the symmetric version, but returning `-Y` instead.

I'm curious why don't we handle the case, when only one of the operands is an 
`ElementRegion` and the other is a plain `SymbolicRegion`.
IMO if the SuperMemoryRegion is the same, we can still return a reasonable 
answer.

In this patch, I suppose an extension to resolve this.
If this seems to be a good idea, I will:

- implement the symmetric version of the check
- add tests


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84736

Files:
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp


Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -1017,6 +1017,21 @@
   }
 }
 
+// Try to get the Index as Nonloc, then cast to ArrayIndexTy.
+// Otherwise return None.
+const auto MaybeNonLocIndexOfElementRegion =
+[this](const ElementRegion *ElemReg) -> Optional {
+  SVal IndexVal = ElemReg->getIndex();
+  Optional Index = IndexVal.getAs();
+  if (!Index)
+return llvm::None;
+  IndexVal = evalCastFromNonLoc(*Index, ArrayIndexTy);
+  Index = IndexVal.getAs();
+  if (!Index)
+return llvm::None;
+  return Index.getValue();
+};
+
 // Handle special cases for when both regions are element regions.
 const ElementRegion *RightER = dyn_cast(RightMR);
 const ElementRegion *LeftER = dyn_cast(LeftMR);
@@ -1027,31 +1042,32 @@
   // give the correct answer.
   if (LeftER->getSuperRegion() == RightER->getSuperRegion() &&
   LeftER->getElementType() == RightER->getElementType()) {
-// Get the left index and cast it to the correct type.
-// If the index is unknown or undefined, bail out here.
-SVal LeftIndexVal = LeftER->getIndex();
-Optional LeftIndex = LeftIndexVal.getAs();
-if (!LeftIndex)
-  return UnknownVal();
-LeftIndexVal = evalCastFromNonLoc(*LeftIndex, ArrayIndexTy);
-LeftIndex = LeftIndexVal.getAs();
-if (!LeftIndex)
-  return UnknownVal();
 
-// Do the same for the right index.
-SVal RightIndexVal = RightER->getIndex();
-Optional RightIndex = RightIndexVal.getAs();
-if (!RightIndex)
-  return UnknownVal();
-RightIndexVal = evalCastFromNonLoc(*RightIndex, ArrayIndexTy);
-RightIndex = RightIndexVal.getAs();
-if (!RightIndex)
+Optional LeftIndex = MaybeNonLocIndexOfElementRegion(LeftER);
+Optional RightIndex = MaybeNonLocIndexOfElementRegion(RightER);
+
+if (!LeftIndex || !RightIndex)
   return UnknownVal();
 
 // Actually perform the operation.
 // evalBinOpNN expects the two indexes to already be the right type.
 return evalBinOpNN(state, op, *LeftIndex, *RightIndex, resultTy);
   }
+} else if (LeftER && isa(RightMR)) {
+  if (LeftER->getSuperRegion() != RightMR)
+return UnknownVal();
+  if (Optional LeftIndex = MaybeNonLocIndexOfElementRegion(LeftER))
+return LeftIndex.getValue();
+  return UnknownVal();
+} else if (RightER && isa(LeftMR)) {
+  if (RightER->getSuperRegion() != LeftMR)
+return UnknownVal();
+  if (Optional RightIndex =
+  MaybeNonLocIndexOfElementRegion(RightER))
+llvm_unreachable(
+"TODO: return the negated value of RightIndex - figure out how 
:D\n"
+"Maybe a unary minus operator would do the job.");
+  return UnknownVal();
 }
 
 // Special handling of the FieldRegions, even with symbolic offsets.


Index: clang/lib/StaticAnalyzer/Core/Simple

[clang] 984cf99 - [clang][NFC] Add some documentation about the use of NamedDecl::getDeclName in diagnostics.

[Bruno Ricci via cfe-commits]

Author: Bruno Ricci
Date: 2020-07-28T15:39:17+01:00
New Revision: 984cf99055a292b3afe4535c013d38914a3da880

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

LOG: [clang][NFC] Add some documentation about the use of 
NamedDecl::getDeclName in diagnostics.

As explained in eb10b065f2a870b425dcc2040b9955e0eee464b4, sending a NamedDecl*
in a diagnostic should generally be preferred over sending the DeclarationName
from getDeclName(). Let's document that.

Added: 


Modified: 
clang/include/clang/AST/Decl.h

Removed: 




diff  --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 28faa2c1fc78..4dd5e14d36e1 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -269,6 +269,19 @@ class NamedDecl : public Decl {
 
   /// Get the actual, stored name of the declaration, which may be a special
   /// name.
+  ///
+  /// Note that generally in diagnostics, the non-null \p NamedDecl* itself
+  /// should be sent into the diagnostic instead of using the result of
+  /// \p getDeclName().
+  ///
+  /// A \p DeclarationName in a diagnostic will just be streamed to the output,
+  /// which will directly result in a call to \p DeclarationName::print.
+  ///
+  /// A \p NamedDecl* in a diagnostic will also ultimately result in a call to
+  /// \p DeclarationName::print, but with two customisation points along the
+  /// way (\p getNameForDiagnostic and \p printName). These are used to print
+  /// the template arguments if any, and to provide a user-friendly name for
+  /// some entities (such as unnamed variables and anonymous records).
   DeclarationName getDeclName() const { return Name; }
 
   /// Set the name of this declaration.



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

[PATCH] D83224: [clangd] Move clang-tidy check modifications into ClangdServer

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D83224#2133457 , @njames93 wrote:

> Do you think there should be a hidden command line option to disable 
> disabling blacklisted checks, purely to prevent hindering attempts to fix 
> these problematic checks.

I would expect those to be fixed and verified with unittests that are using 
TestTU, so the changes in ClangdServer shouldn't really be relevant to them. 
And if developer is trying to test something locally, i think it is better for 
them to just update the blacklist file(they are going to end up doing that 
anyways and perform possibly multiple compiles), so a flag to change blacklists 
behaviour might not be that meaningful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83224

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

[PATCH] D84697: [clangd] Use elog instead of llvm::errs, log instead of llvm::outs

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:202
 
+  // Use buffered stream to stderr.
+  llvm::errs().SetBuffered();

explain why or just delete this comment



Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:202
 
+  // Use buffered stream to stderr.
+  llvm::errs().SetBuffered();

sammccall wrote:
> explain why or just delete this comment
nit: move this down after flag validation?



Comment at: clang-tools-extra/clangd/index/remote/server/Server.cpp:204
+  llvm::errs().SetBuffered();
+  // Don't flush stdout when logging for performance.
+  llvm::errs().tie(nullptr);

I'm not sure performance matters here (if you don't actually write to stdout 
much, it should be free). Rather, this is for correctness: it's not threadsafe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84697

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

[PATCH] D82739: [clangd] Improve heuristic resolution of dependent types in TargetFinder

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:172
 std::vector resolveDependentExprToDecls(const Expr *E) {
   assert(E->isTypeDependent());
   if (const auto *ME = dyn_cast(E)) {

@nridge, the assertion is not true anymore, since we have extended it to 
support possibly-non-dependent expressions (`callExpr`, `MemberExpr`). 

we hit this assert when opening 
`clang/include/clang/ASTMatchers/ASTMatchers.h`. I think we probably need to 
remove it and rename related function names.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82739

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

[PATCH] D83088: Introduce CfgTraits abstraction

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:50
+
+  // Clang's CFG contains nullpointers for unreachable succesors, e.g. when an
+  // if statement's condition is always false, it's 'then' branch is 
represented

Missing space nullpointers, missing s succesors



Comment at: clang/include/clang/Analysis/Analyses/Dominators.h:51
+  // Clang's CFG contains nullpointers for unreachable succesors, e.g. when an
+  // if statement's condition is always false, it's 'then' branch is 
represented
+  // with a nullptr. Account for this in the predecessors / successors

s/it's/its/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83088

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

[PATCH] D84534: [AIX] Static init frontend recovery and backend support

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 281240.
Xiangling_L marked 4 inline comments as done.
Xiangling_L added a comment.

Created alias for sinit and sterm;
Adjusted the testcase accordingly;


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

https://reviews.llvm.org/D84534

Files:
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
  clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
  clang/test/CodeGenCXX/aix-static-init.cpp
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
  llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
  llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
  llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll

Index: llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-non-default-priority.ll
@@ -0,0 +1,10 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 655, void ()* @foo, i8* null }]
+
+define void @foo() {
+  ret void
+}
+
+// CHECK: LLVM ERROR: prioritized sinit and sterm functions are not yet supported
Index: llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll
@@ -0,0 +1,7 @@
+; RUN: not llc -mtriple powerpc-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff < %s 2>&1 | FileCheck %s
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @foo, i8* null }]
+@llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @bar, i8* null }]
+
+// CHECK: LLVM ERROR: cannot produce a unique identifier for this module based on strong external symbols
Index: llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-static-init-default-priority.ll
@@ -0,0 +1,60 @@
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | FileCheck %s
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff < %s | FileCheck %s
+
+@llvm.global_ctors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @init1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @init2, i8* null }]
+@llvm.global_dtors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @destruct1, i8* null }, { i32, void ()*, i8* } { i32 65535, void ()* @destruct2, i8* null }]
+
+define i32 @extFunc() {
+entry:
+  ret i32 3
+}
+
+define internal void @init1() {
+  ret void
+}
+
+define internal void @destruct1() {
+  ret void
+}
+
+define internal void @init2() {
+  ret void
+}
+
+define internal void @destruct2() {
+  ret void
+}
+
+; CHECK:   .lglobl	init1[DS]
+; CHECK:   .lglobl	.init1
+; CHECK:   .csect init1[DS]
+; CHECK: __sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0: # @init1
+; CHECK: .init1:
+; CHECK: .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0:
+; CHECK:   .lglobl	destruct1[DS]
+; CHECK:   .lglobl	.destruct1
+; CHECK:   .csect destruct1[DS]
+; CHECK: __sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0: # @destruct1
+; CHECK: .destruct1:
+; CHECK: .__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0:
+; CHECK:   .lglobl	init2[DS]
+; CHECK:   .lglobl	.init2
+; CHECK:   .csect init2[DS]
+; CHECK: __sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1: # @init2
+; CHECK: .init2:
+; CHECK: .__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1:
+; CHECK:   .lglobl	destruct2[DS]
+; CHECK:   .lglobl	.destruct2
+; CHECK:   .csect destruct2[DS]
+; CHECK: __sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_1: # @destruct2
+; CHECK: .destruct2:
+; CHECK: .__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_1:
+
+; CHECK: 	.globl	__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0
+; CHECK: 	.globl	.__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_0
+; CHECK: 	.globl	__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1
+; CHECK: 	.globl	.__sinit8000_clang_ac404299654d2af7eae71e75c17f7c9b_1
+; CHECK: 	.globl	__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0
+; CHECK: 	.globl	.__sterm8000_clang_ac404299654d2af7eae71e75c17f7c9b_0

[PATCH] D84658: [clang] Overload NamedDecl::printName to provide a user-friendly name for unnamed entities

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

Note that I am planning two improvements as a follow-up to this patch:

- For a parameter `(unnamed variable at {{.*}}:: of type )` -> 
`(unnamed parameter at {{.*}}:: of type )`
- For a lambda:
  - `(unnamed class at {{.*}}::)` -> `(lambda at 
{{.*}}::)`
  - For a captured field in the lambda class: `(unnamed field at 
{{.*}}:: of type )` -> something which refers to the captured 
entity instead of the unnamed field.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84658

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

[PATCH] D83992: [ASTImporter] Add Visitor for TypedefNameDecl's

[Gabor Marton via Phabricator via cfe-commits]

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

LGTM! Thanks for the test!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83992

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

[PATCH] D77150: [Analyzer] New Option for ContainerModeling: AggressiveEraseModeling

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

In D77150#2149870 , @dkrupp wrote:

> Since the analyzer cannot cannot model the size of the containers just yet 
> (or precisely enough), what we are saying with this checker is to "always 
> check the return value of the erase() function before use (for 
> increment/decrement etc.) whether it is not past-end" .
>
> Adam, are you sure that the user would like to enforce such a generic coding 
> rule? Depending on the actual code analyzed, this would require this clumsy 
> check at potentially too many places.

While I agree that realising in the analyser that the user code at hand is 
meant to express that the iterator stepping will not go out of bounds, you have 
to keep in mind, that going out of bounds with an iterator is still UB. In case 
of a `for` loop, it's //always// a dangerous construct to call `erase()` 
inside, unless you, indeed, can explicitly ensure that you won't go out of 
bounds.

Side note: I am surprised there isn't a Tidy check in `bugprone-` that simply 
says //"If you are using `erase()`, use a `while` loop and not a `for` 
loop..."//

I'm not terribly up-to-date with @baloghadamsoftware's patches (nor the innards 
of the analyser), but as a user who'd get this warning, I believe one crucial 
information missing is something like //"note: assuming `erase()` removed the 
the last element, setting `it` to the past-the-end iterator"//

In D77150#2149870 , @dkrupp wrote:

> Oops, I did not mean "request changes". This is just meant to be an 
> additional comment in this discussion.

You can do the //Resign as Reviewer// action to remove your `X` from the patch. 
🙂




Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:620-624
+  "AggressiveEraseModeling",
+  "Enables exploration of the past-the-end branch for the "
+  "return value of the erase() method of containers.",
+  "false",
+  Released>

Szelethus wrote:
> NoQ wrote:
> > Szelethus wrote:
> > > Hmm. The description isn't really user-friendly, imo, but the option 
> > > doesn't sound too user-facing either. I don't think this is an option to 
> > > be tinkered with. I think we should make this hidden.
> > > 
> > > Also, even for developers, I find this a bitconfusing at first. Do we not 
> > > enable that by default? Why not? What do we have to gain/lose?
> > What is the rule that the user needs to follow in order to avoid the 
> > warning? "Always check the return value of `erase()` before use"? If so, a 
> > good description would be along the lines of "Warn when the return value of 
> > `erase()` is not checked before use; compare that value to end() in order 
> > to suppress the warning" (or something more specific).
> Please mark this hidden.
Is marking something hidden still allow for modifying the value from the 
command-line, or via CodeChecker?



Comment at: clang/lib/StaticAnalyzer/Checkers/ContainerModeling.cpp:708-709
+  // end symbol from the container data because the  past-the-end iterator
+  // was also invalidated. The next call to member function `end()` would
+  // create a new one, but we need it now so we create it now.
+  if (!EndSym) {

Is this creation prevented?


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

https://reviews.llvm.org/D77150

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

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLength.h:43
+ ProgramStateRef &State, const Expr *Ex,
+ SVal Buf, bool Hypothetical = false);
+

I do not like that the //get// and //set// (CStringLength) functions are not 
symmetrical. I (and other developers) would think that the get function returns 
a stored value and the set function sets it. The `getCStringLength` is more a 
`computeCStringLength` and additionally may manipulate the `State` too. In this 
form it is usable mostly only for CStringChecker. (A separate function to get 
the value stored in the length map should exist instead of this `Hypothetical` 
thing.)


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

https://reviews.llvm.org/D84316

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

[PATCH] D83992: [ASTImporter] Add Visitor for TypedefNameDecl's

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf761acfb1a73: [ASTImporter] Add Visitor for 
TypedefNameDecl's (authored by vabridgers, committed by einvbri 
).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83992

Files:
  clang/lib/AST/ASTImporterLookupTable.cpp
  clang/test/Analysis/Inputs/ctu-import.c
  clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/ctu-implicit.c


Index: clang/test/Analysis/ctu-implicit.c
===
--- /dev/null
+++ clang/test/Analysis/ctu-implicit.c
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir2
+// RUN: %clang_cc1  \
+// RUN:   -emit-pch -o %t/ctudir2/ctu-import.c.ast %S/Inputs/ctu-import.c
+// RUN: cp %S/Inputs/ctu-import.c.externalDefMap.ast-dump.txt 
%t/ctudir2/externalDefMap.txt
+// RUN: %clang_cc1 -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config  display-ctu-progress=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir2 \
+// RUN:   -verify %s
+
+void clang_analyzer_eval(int);
+
+int testStaticImplicit(void);
+int func(void) {
+  int ret = testStaticImplicit();
+  clang_analyzer_eval(ret == 4); // expected-warning{{TRUE}}
+  return testStaticImplicit();
+}
Index: clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
@@ -0,0 +1 @@
+c:@F@testStaticImplicit ctu-import.c.ast
Index: clang/test/Analysis/Inputs/ctu-import.c
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-import.c
@@ -0,0 +1,15 @@
+
+// Use an internal, implicitly defined type, called by
+// a function imported for CTU. This should not crash.
+int foo(void);
+int foobar(int skip) {
+  __NSConstantString str = {.flags = 1};
+
+  if (str.flags >= 0)
+str.flags = 0;
+  return 4;
+}
+
+int testStaticImplicit(void) {
+  return foobar(3);
+}
Index: clang/lib/AST/ASTImporterLookupTable.cpp
===
--- clang/lib/AST/ASTImporterLookupTable.cpp
+++ clang/lib/AST/ASTImporterLookupTable.cpp
@@ -22,6 +22,20 @@
 struct Builder : RecursiveASTVisitor {
   ASTImporterLookupTable <
   Builder(ASTImporterLookupTable <) : LT(LT) {}
+
+  bool VisitTypedefNameDecl(TypedefNameDecl *D) {
+QualType Ty = D->getUnderlyingType();
+Ty = Ty.getCanonicalType();
+if (const auto *RTy = dyn_cast(Ty)) {
+  LT.add(RTy->getAsRecordDecl());
+  // iterate over the field decls, adding them
+  for (auto *it : RTy->getAsRecordDecl()->fields()) {
+LT.add(it);
+  }
+}
+return true;
+  }
+
   bool VisitNamedDecl(NamedDecl *D) {
 LT.add(D);
 return true;


Index: clang/test/Analysis/ctu-implicit.c
===
--- /dev/null
+++ clang/test/Analysis/ctu-implicit.c
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir2
+// RUN: %clang_cc1  \
+// RUN:   -emit-pch -o %t/ctudir2/ctu-import.c.ast %S/Inputs/ctu-import.c
+// RUN: cp %S/Inputs/ctu-import.c.externalDefMap.ast-dump.txt %t/ctudir2/externalDefMap.txt
+// RUN: %clang_cc1 -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config  display-ctu-progress=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir2 \
+// RUN:   -verify %s
+
+void clang_analyzer_eval(int);
+
+int testStaticImplicit(void);
+int func(void) {
+  int ret = testStaticImplicit();
+  clang_analyzer_eval(ret == 4); // expected-warning{{TRUE}}
+  return testStaticImplicit();
+}
Index: clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
@@ -0,0 +1 @@
+c:@F@testStaticImplicit ctu-import.c.ast
Index: clang/test/Analysis/Inputs/ctu-import.c
===
--- /dev/null
+++ clang/test/Analysis/Inputs/ctu-import.c
@@ -0,0 +1,15 @@
+
+// Use an internal, implicitly defined type, called by
+// a function imported for CTU. This should not crash.
+int foo(void);
+int foobar(int skip) {
+  __NSConstantString str = {.flags = 1};
+
+  if (str.flags >= 0)
+str.flags = 0;
+  return 4;
+}
+
+int testStaticImplicit(void) {
+  return foobar(3);
+}
Index: clang/lib/AST/ASTImporterLookupTable.cpp
===
--- clang/lib/AST/ASTImporterLookupTable.cpp
+++

[clang] f761acf - [ASTImporter] Add Visitor for TypedefNameDecl's

[via cfe-commits]

Author: Vince Bridgers
Date: 2020-07-28T11:52:29-05:00
New Revision: f761acfb1a737d8a631a5e55b58cdb7c2215baad

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

LOG: [ASTImporter] Add Visitor for TypedefNameDecl's

We found a case where Typedef Name Declarations were not being added
correctly when importing builtin types. This exposed the need for a
TypedefNameDecl visitor so these types can be added by RecordDecl and
fields.

This code is covered by the ASTImporterTest cases that use the implicit
struct __NSConstantString_tag definitions.

Thanks to @martong for the debugging assist!

Depends on D83970.

Reviewed By: martong

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

Added: 
clang/test/Analysis/Inputs/ctu-import.c
clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
clang/test/Analysis/ctu-implicit.c

Modified: 
clang/lib/AST/ASTImporterLookupTable.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTImporterLookupTable.cpp 
b/clang/lib/AST/ASTImporterLookupTable.cpp
index 4d6fff8f3419..e17d6082dcdc 100644
--- a/clang/lib/AST/ASTImporterLookupTable.cpp
+++ b/clang/lib/AST/ASTImporterLookupTable.cpp
@@ -22,6 +22,20 @@ namespace {
 struct Builder : RecursiveASTVisitor {
   ASTImporterLookupTable <
   Builder(ASTImporterLookupTable <) : LT(LT) {}
+
+  bool VisitTypedefNameDecl(TypedefNameDecl *D) {
+QualType Ty = D->getUnderlyingType();
+Ty = Ty.getCanonicalType();
+if (const auto *RTy = dyn_cast(Ty)) {
+  LT.add(RTy->getAsRecordDecl());
+  // iterate over the field decls, adding them
+  for (auto *it : RTy->getAsRecordDecl()->fields()) {
+LT.add(it);
+  }
+}
+return true;
+  }
+
   bool VisitNamedDecl(NamedDecl *D) {
 LT.add(D);
 return true;

diff  --git a/clang/test/Analysis/Inputs/ctu-import.c 
b/clang/test/Analysis/Inputs/ctu-import.c
new file mode 100644
index ..6c99a3642797
--- /dev/null
+++ b/clang/test/Analysis/Inputs/ctu-import.c
@@ -0,0 +1,15 @@
+
+// Use an internal, implicitly defined type, called by
+// a function imported for CTU. This should not crash.
+int foo(void);
+int foobar(int skip) {
+  __NSConstantString str = {.flags = 1};
+
+  if (str.flags >= 0)
+str.flags = 0;
+  return 4;
+}
+
+int testStaticImplicit(void) {
+  return foobar(3);
+}

diff  --git 
a/clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt 
b/clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
new file mode 100644
index ..83d3b4ca451e
--- /dev/null
+++ b/clang/test/Analysis/Inputs/ctu-import.c.externalDefMap.ast-dump.txt
@@ -0,0 +1 @@
+c:@F@testStaticImplicit ctu-import.c.ast

diff  --git a/clang/test/Analysis/ctu-implicit.c 
b/clang/test/Analysis/ctu-implicit.c
new file mode 100644
index ..925044845e09
--- /dev/null
+++ b/clang/test/Analysis/ctu-implicit.c
@@ -0,0 +1,20 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir2
+// RUN: %clang_cc1  \
+// RUN:   -emit-pch -o %t/ctudir2/ctu-import.c.ast %S/Inputs/ctu-import.c
+// RUN: cp %S/Inputs/ctu-import.c.externalDefMap.ast-dump.txt 
%t/ctudir2/externalDefMap.txt
+// RUN: %clang_cc1 -analyze \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config  display-ctu-progress=true \
+// RUN:   -analyzer-config ctu-dir=%t/ctudir2 \
+// RUN:   -verify %s
+
+void clang_analyzer_eval(int);
+
+int testStaticImplicit(void);
+int func(void) {
+  int ret = testStaticImplicit();
+  clang_analyzer_eval(ret == 4); // expected-warning{{TRUE}}
+  return testStaticImplicit();
+}



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

[PATCH] D84315: [libTooling] Add a `between` range-selector combinator.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 281268.
ymandel added a comment.

updated comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84315

Files:
  clang/include/clang/Tooling/Transformer/RangeSelector.h
  clang/lib/Tooling/Transformer/Parsing.cpp
  clang/unittests/Tooling/RangeSelectorTest.cpp

Index: clang/unittests/Tooling/RangeSelectorTest.cpp
===
--- clang/unittests/Tooling/RangeSelectorTest.cpp
+++ clang/unittests/Tooling/RangeSelectorTest.cpp
@@ -193,8 +193,33 @@
HasValue(EqualsCharSourceRange(ExpectedAfter)));
 }
 
+TEST(RangeSelectorTest, BetweenOp) {
+  StringRef Code = R"cc(
+int f(int x, int y, int z) { return 3; }
+int g() { return f(3, /* comment */ 7 /* comment */, 9); }
+  )cc";
+  auto Matcher = callExpr(hasArgument(0, expr().bind("a0")),
+  hasArgument(1, expr().bind("a1")));
+  RangeSelector R = between(node("a0"), node("a1"));
+  TestMatch Match = matchCode(Code, Matcher);
+  EXPECT_THAT_EXPECTED(select(R, Match), HasValue(", /* comment */ "));
+}
+
+TEST(RangeSelectorTest, BetweenOpParsed) {
+  StringRef Code = R"cc(
+int f(int x, int y, int z) { return 3; }
+int g() { return f(3, /* comment */ 7 /* comment */, 9); }
+  )cc";
+  auto Matcher = callExpr(hasArgument(0, expr().bind("a0")),
+  hasArgument(1, expr().bind("a1")));
+  auto R = parseRangeSelector(R"rs(between(node("a0"), node("a1")))rs");
+  ASSERT_THAT_EXPECTED(R, llvm::Succeeded());
+  TestMatch Match = matchCode(Code, Matcher);
+  EXPECT_THAT_EXPECTED(select(*R, Match), HasValue(", /* comment */ "));
+}
+
 // Node-id specific version.
-TEST(RangeSelectorTest, RangeOpNodes) {
+TEST(RangeSelectorTest, EncloseOpNodes) {
   StringRef Code = R"cc(
 int f(int x, int y, int z) { return 3; }
 int g() { return f(/* comment */ 3, 7 /* comment */, 9); }
@@ -206,7 +231,7 @@
   EXPECT_THAT_EXPECTED(select(R, Match), HasValue("3, 7"));
 }
 
-TEST(RangeSelectorTest, RangeOpGeneral) {
+TEST(RangeSelectorTest, EncloseOpGeneral) {
   StringRef Code = R"cc(
 int f(int x, int y, int z) { return 3; }
 int g() { return f(/* comment */ 3, 7 /* comment */, 9); }
@@ -218,7 +243,7 @@
   EXPECT_THAT_EXPECTED(select(R, Match), HasValue("3, 7"));
 }
 
-TEST(RangeSelectorTest, RangeOpNodesParsed) {
+TEST(RangeSelectorTest, EncloseOpNodesParsed) {
   StringRef Code = R"cc(
 int f(int x, int y, int z) { return 3; }
 int g() { return f(/* comment */ 3, 7 /* comment */, 9); }
@@ -231,7 +256,7 @@
   EXPECT_THAT_EXPECTED(select(*R, Match), HasValue("3, 7"));
 }
 
-TEST(RangeSelectorTest, RangeOpGeneralParsed) {
+TEST(RangeSelectorTest, EncloseOpGeneralParsed) {
   StringRef Code = R"cc(
 int f(int x, int y, int z) { return 3; }
 int g() { return f(/* comment */ 3, 7 /* comment */, 9); }
Index: clang/lib/Tooling/Transformer/Parsing.cpp
===
--- clang/lib/Tooling/Transformer/Parsing.cpp
+++ clang/lib/Tooling/Transformer/Parsing.cpp
@@ -109,14 +109,14 @@
 static const llvm::StringMap> &
 getBinaryStringSelectors() {
   static const llvm::StringMap> M = {
-  {"encloseNodes", range}};
+  {"encloseNodes", encloseNodes}};
   return M;
 }
 
 static const llvm::StringMap> &
 getBinaryRangeSelectors() {
   static const llvm::StringMap>
-  M = {{"enclose", range}};
+  M = {{"enclose", enclose}, {"between", between}};
   return M;
 }
 
Index: clang/include/clang/Tooling/Transformer/RangeSelector.h
===
--- clang/include/clang/Tooling/Transformer/RangeSelector.h
+++ clang/include/clang/Tooling/Transformer/RangeSelector.h
@@ -56,6 +56,11 @@
 /// * the TokenRange [B,E'] where the token at E' spans the range [E',E).
 RangeSelector after(RangeSelector Selector);
 
+/// Selects the range between `R1` and `R2.
+inline RangeSelector between(RangeSelector R1, RangeSelector R2) {
+  return enclose(after(std::move(R1)), before(std::move(R2)));
+}
+
 /// Selects a node, including trailing semicolon (for non-expression
 /// statements). \p ID is the node's binding in the match result.
 RangeSelector node(std::string ID);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81865: [clang] Use string tables for static diagnostic descriptions

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D81865#2178542 , @froydnj wrote:

> In D81865#2176589 , @dblaikie wrote:
>
>> I believe this falls under the (using cppreference ( 
>> https://en.cppreference.com/w/cpp/language/union ) , since it's a bit easier 
>> to read) UB clause: " It's undefined behavior to read from the member of the 
>> union that wasn't most recently written. Many compilers implement, as a 
>> non-standard language extension, the ability to read inactive members of a 
>> union."
>>
>> Last member written to was the "StringTable" member, but then it's read from 
>> the "String" member, so that'd be UB. Commonly supported, but UB - not sure 
>> if we have a general statement that we depend on this behavior in LLVM, even 
>> though it's non-standard (but it's possible that we do make such an 
>> assumption about the compiler that's building LLVM). It'd be nice to avoid 
>> that, though - and not too difficult, I think - I /believe/ it's valid to 
>> take a pointer to an object, cast it to char*, compute a pointer to some 
>> specific member and then cast it back to the right type and access. But I 
>> could be wrong there. @rsmith would be the person to give an authoritative 
>> answer.
>
> Thanks for the explanation.  Is the language of "writing" applicable here, 
> given that this is a constant blob of storage?  (I suppose the compiler is 
> permitted to designate a particular member as having been "written"?)

I /believe/ it is applicable, though I could be wrong.

Ah, cppreference's example supports that theory at least: `S s = {0x12345678}; 
// initializes the first member, s.n is now the active member` - that the 
initialization itself does set the active member of the union.

(oh, and a later example on cppreference, which is lifted from the C++ spec 
(though not sure exactly which version) says similarly: "Y y = { { 1, 2 } }; // 
OK, y.x is active union member (9.2)")

> Would it be more palatable to write:
>
>   struct StaticDiagInfoDescriptionStringTable {
> // members as char[] for each diagnostic
>   };
>   
>   const StaticDiagInfoDescriptionStringTable StaticDiagInfoDescriptions = {
> // define all the members
>   };
>   
>   ...
>   
>   struct StaticDiagInfoRec {
> ...
> StringRef getDescription() const {
>   size_t MyIndex = this - &StaticDiagInfo[0];
>   uint32_t StringOffset = StaticDiagInfoDescriptionOffsets[MyIndex];
>   // Defined as a pointer to the first member, and (presumably) there is 
> no internal padding.
>   const char *StringTable = reinterpret_cast char*>(&StaticDiagInfoDescriptions);
>   return StringRef(&StringTable[StringOffset], DescriptionLen);
>   };
>
> and then we don't have to care about how the host compiler interprets access 
> to different members of unions?

I think so? I guess that's essentially the point of offsetof.

The comment about padding probably isn't needed, I think? Even if there was 
padding, the StringOffset comes from "offsetof" so it describes the offset 
including any padding involved?

Be great if @rsmith got a chance to weigh in here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81865

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

[PATCH] D84767: [OPENMP]Fix PR46824: Global declare target pointer cannot be accessed in target region.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev created this revision.
ABataev added reviewers: jdoerfert, ye-luo, RaviNarayanaswamy, grokos.
Herald added subscribers: openmp-commits, guansong, yaxunl.
Herald added projects: clang, OpenMP.
ABataev requested review of this revision.
Herald added a subscriber: sstefan1.

Need to map the base pointer for all directives, not only target
data-based ones.
The base pointer is mapped for array sections, array subscript, array
shaping and other array-like constructs with the base pointer. Also,
codegen for use_device_ptr clause was modified to correctly handle
mapping combination of array like constructs + use_device_ptr clause.
The data for use_device_ptr clause is emitted as the last records in the
data mapping array.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84767

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/target_data_codegen.cpp
  clang/test/OpenMP/target_data_use_device_ptr_codegen.cpp
  clang/test/OpenMP/target_map_codegen.cpp
  clang/test/OpenMP/target_update_codegen.cpp
  openmp/libomptarget/src/omptarget.cpp

Index: openmp/libomptarget/src/omptarget.cpp
===
--- openmp/libomptarget/src/omptarget.cpp
+++ openmp/libomptarget/src/omptarget.cpp
@@ -872,14 +872,9 @@
   return OFFLOAD_FAIL;
 }
   }
-} else if (arg_types[i] & OMP_TGT_MAPTYPE_PTR_AND_OBJ) {
-  TgtPtrBegin = Device.getTgtPtrBegin(HstPtrBase, sizeof(void *), IsLast,
-  false, IsHostPtr);
-  TgtBaseOffset = 0; // no offset for ptrs.
-  DP("Obtained target argument " DPxMOD " from host pointer " DPxMOD " to "
- "object " DPxMOD "\n", DPxPTR(TgtPtrBegin), DPxPTR(HstPtrBase),
- DPxPTR(HstPtrBase));
 } else {
+  if (arg_types[i] & OMP_TGT_MAPTYPE_PTR_AND_OBJ)
+HstPtrBase = *reinterpret_cast(HstPtrBase);
   TgtPtrBegin = Device.getTgtPtrBegin(HstPtrBegin, arg_sizes[i], IsLast,
   false, IsHostPtr);
   TgtBaseOffset = (intptr_t)HstPtrBase - (intptr_t)HstPtrBegin;
Index: clang/test/OpenMP/target_update_codegen.cpp
===
--- clang/test/OpenMP/target_update_codegen.cpp
+++ clang/test/OpenMP/target_update_codegen.cpp
@@ -310,18 +310,18 @@
 
 #ifdef CK5
 
-// CK5: [[SIZE00:@.+]] = {{.+}}constant [2 x i[[sz:64|32]]] [i{{64|32}} {{8|4}}, i{{64|32}} 4]
-// CK5: [[MTYPE00:@.+]] = {{.+}}constant [2 x i64] [i64 33, i64 17]
+// CK5: [[SIZE00:@.+]] = {{.+}}constant [1 x i[[sz:64|32]]] [i{{64|32}} 4]
+// CK5: [[MTYPE00:@.+]] = {{.+}}constant [1 x i64] [i64 49]
 
 // CK5-LABEL: lvalue
 void lvalue(int *B, int l, int e) {
 
-  // CK5-DAG: call void @__tgt_target_data_update_mapper(i64 -1, i32 2, i8** [[GEPBP:%.+]], i8** [[GEPP:%.+]], {{.+}}getelementptr {{.+}}[2 x i{{.+}}]* [[SIZE00]], {{.+}}getelementptr {{.+}}[2 x i{{.+}}]* [[MTYPE00]]{{.+}}, i8** null)
+  // CK5-DAG: call void @__tgt_target_data_update_mapper(i64 -1, i32 1, i8** [[GEPBP:%.+]], i8** [[GEPP:%.+]], {{.+}}getelementptr {{.+}}[1 x i{{.+}}]* [[SIZE00]], {{.+}}getelementptr {{.+}}[1 x i{{.+}}]* [[MTYPE00]]{{.+}}, i8** null)
   // CK5-DAG: [[GEPBP]] = getelementptr inbounds {{.+}}[[BP:%[^,]+]]
   // CK5-DAG: [[GEPP]] = getelementptr inbounds {{.+}}[[P:%[^,]+]]
 
-  // CK5-DAG: [[BP0:%.+]] = getelementptr inbounds {{.+}}[[BP]], i{{.+}} 0, i{{.+}} 1
-  // CK5-DAG: [[P0:%.+]] = getelementptr inbounds {{.+}}[[P]], i{{.+}} 0, i{{.+}} 1
+  // CK5-DAG: [[BP0:%.+]] = getelementptr inbounds {{.+}}[[BP]], i{{.+}} 0, i{{.+}} 0
+  // CK5-DAG: [[P0:%.+]] = getelementptr inbounds {{.+}}[[P]], i{{.+}} 0, i{{.+}} 0
   // CK5-DAG: [[BPC0:%.+]] = bitcast i8** [[BP0]] to i32***
   // CK5-DAG: [[PC0:%.+]] = bitcast i8** [[P0]] to i32**
   // CK5-DAG: store i32** [[B_ADDR:%.+]], i32*** [[BPC0]]
@@ -351,18 +351,18 @@
 
 #ifdef CK6
 
-// CK6: [[SIZE00:@.+]] = {{.+}}constant [2 x i[[sz:64|32]]] [i{{64|32}} {{8|4}}, i{{64|32}} 4]
-// CK6: [[MTYPE00:@.+]] = {{.+}}constant [2 x i64] [i64 33, i64 17]
+// CK6: [[SIZE00:@.+]] = {{.+}}constant [1 x i[[sz:64|32]]] [i{{64|32}} 4]
+// CK6: [[MTYPE00:@.+]] = {{.+}}constant [1 x i64] [i64 49]
 
 // CK6-LABEL: lvalue
 void lvalue(int *B, int l, int e) {
 
-  // CK6-DAG: call void @__tgt_target_data_update_mapper(i64 -1, i32 2, i8** [[GEPBP:%.+]], i8** [[GEPP:%.+]], {{.+}}getelementptr {{.+}}[2 x i{{.+}}]* [[SIZE00]], {{.+}}getelementptr {{.+}}[2 x i{{.+}}]* [[MTYPE00]]{{.+}}, i8** null)
+  // CK6-DAG: call void @__tgt_target_data_update_mapper(i64 -1, i32 1, i8** [[GEPBP:%.+]], i8** [[GEPP:%.+]], {{.+}}getelementptr {{.+}}[1 x i{{.+}}]* [[SIZE00]], {{.+}}getelementptr {{.+}}[1 x i{{.+}}]* [[MTYPE00]]{{.+}}, i8** null)
   // CK6-DAG: [[GEPBP]] = getelementptr inbounds {{.+}}[[BP:%[^,]+]]
   // CK6-DAG: [[GEPP]] = getelementptr inbounds {{.+}}[[P:%[^,]+]]
 
-  // CK6-DAG: [[BP0:%.+]] = getelementptr inbounds {{.+}}[[BP]], i{{.+}} 0, i{{.+}} 1
-  // CK6-DAG: [[P0:%.+]] = getelementptr inbounds {{.+}}[[P]], i{{.+}} 0, i{{.+}} 1

[PATCH] D80391: [Driver] Don't make -gsplit-dwarf imply -g2

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Ping...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80391

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

[PATCH] D79147: Switch to using -debug-info-kind=constructor as default (from =limited)

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a subscriber: echristo.
dblaikie added a comment.

In D79147#2178104 , @russell.gallop 
wrote:

> Hi,
>
> It looks like is causing one of the debuginfo-tests: 
> llgdb-tests/nrvo-string.cpp to fail, run on Linux. Failure as below. I don't 
> think the debuginfo-tests are run on any bot (but probably should be!). I 
> bisected the failure back to this change.
>
> Please could you take a look?
>
> Thanks
> Russ
>
>   FAIL: debuginfo-tests :: llgdb-tests/nrvo-string.cpp (1 of 1)
>    TEST 'debuginfo-tests :: llgdb-tests/nrvo-string.cpp' 
> FAILED 
>   Script:
>   --
>   : 'RUN: at line 4';   /home//git/llvm-project/stage1/bin/clang 
> --driver-mode=g++ -O0 -fno-exceptions --target=x86_64-unknown-linux-gnu 
> /home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp -o 
> /home//git/llvm-project/stage1/projects/debuginfo-tests/llgdb-tests/Output/nrvo-string.cpp.tmp.out
>  -g
>   : 'RUN: at line 5';   
> /home//git/llvm-project/debuginfo-tests/llgdb-tests/test_debuginfo.pl 
> /home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp 
> /home//git/llvm-project/stage1/projects/debuginfo-tests/llgdb-tests/Output/nrvo-string.cpp.tmp.out
>   : 'RUN: at line 6';   /home//git/llvm-project/stage1/bin/clang 
> --driver-mode=g++ -O1 -fno-exceptions --target=x86_64-unknown-linux-gnu 
> /home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp -o 
> /home//git/llvm-project/stage1/projects/debuginfo-tests/llgdb-tests/Output/nrvo-string.cpp.tmp.out
>  -g
>   : 'RUN: at line 7';   
> /home//git/llvm-project/debuginfo-tests/llgdb-tests/test_debuginfo.pl 
> /home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp 
> /home//git/llvm-project/stage1/projects/debuginfo-tests/llgdb-tests/Output/nrvo-string.cpp.tmp.out
>   --
>   Exit Code: 1
>   
>   Command Output (stdout):
>   --
>   Debugger output was:
>   Breakpoint 1 at 0x4004f8: file 
> /home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp, 
> line 23.
>   Breakpoint 2 at 0x400563: file 
> /home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp, 
> line 39.
>   
>   Breakpoint 1, get_string () at 
> /home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp:23
>   23stop();
>   $1 = 3
>   
>   Breakpoint 2, get_string2 () at 
> /home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp:39
>   39stop();
>   
> /home//git/llvm-project/stage1/projects/debuginfo-tests/llgdb-tests/Output/nrvo-string.cpp.debugger.script:6:
>  Error in sourced command file:
>   There is no member named i.
>   
>   --
>   Command Output (stderr):
>   --
>   
> /home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp:52:11:
>  error: CHECK: expected string not found in input
>   // CHECK: = 5
> ^
>   
> /home//git/llvm-project/stage1/projects/debuginfo-tests/llgdb-tests/Output/nrvo-string.cpp.gdb.output:8:1:
>  note: scanning from here
>   Breakpoint 2, get_string2 () at 
> /home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp:39
>   ^
>   
> /home//git/llvm-project/stage1/projects/debuginfo-tests/llgdb-tests/Output/nrvo-string.cpp.gdb.output:8:10:
>  note: possible intended match here
>   Breakpoint 2, get_string2 () at 
> /home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp:39
>^
>   
>   Input file: 
> /home//git/llvm-project/stage1/projects/debuginfo-tests/llgdb-tests/Output/nrvo-string.cpp.gdb.output
>   Check file: 
> /home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp
>   
>   -dump-input=help explains the following input dump.
>   
>   Full input was:
>   <<
>   1: Breakpoint 1 at 0x4004f8: file 
> /home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp, 
> line 23.
>   2: Breakpoint 2 at 0x400563: file 
> /home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp, 
> line 39.
>   3:
>   4: Breakpoint 1, get_string () at 
> /home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp:23
>   5: 23 stop();
>   6: $1 = 3
>   7:
>   8: Breakpoint 2, get_string2 () at 
> /home//git/llvm-project/debuginfo-tests/llgdb-tests/nrvo-string.cpp:39
>   check:52'0 
> X
>  error: no match found
>   check:52'1  ?   
>  possible intended match
>   9: 39 stop();
>   check:52'0 ~~
>  10: 
> /home//git/llvm-project/stage1/projects/debuginfo-tests/llgdb-tests/Output/nrvo-string.cpp.debugger.script:6:
>  Error in sourced command file:
>   check:52'0 
> 

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

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

steakhal added a comment.

Thanks for checking this @balazske.




Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLength.h:43
+ ProgramStateRef &State, const Expr *Ex,
+ SVal Buf, bool Hypothetical = false);
+

balazske wrote:
> I do not like that the //get// and //set// (CStringLength) functions are not 
> symmetrical. I (and other developers) would think that the get function 
> returns a stored value and the set function sets it. The `getCStringLength` 
> is more a `computeCStringLength` and additionally may manipulate the `State` 
> too. In this form it is usable mostly only for CStringChecker. (A separate 
> function to get the value stored in the length map should exist instead of 
> this `Hypothetical` thing.)
> [...] get function returns a stored value and the set function sets it.
Certainly a burden to understand. It would be more appealing, but more useful?
The user would have to check and create if necessary regardless. So fusing 
these two functions is more like a feature.
What use case do you think of using only the query function? In other words, 
how can you guarantee that you will find a length for a symbol?

> In this form it is usable mostly only for CStringChecker. (A separate 
> function to get the value stored in the length map should exist instead of 
> this Hypothetical thing.)
You are right. However, I want to focus on splitting parts without modifying 
the already existing API reducing the risk of breaking things.
You should expect such a change in an upcoming patch.


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

https://reviews.llvm.org/D84316

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

[clang] 04a2131 - [libTooling] Add a `between` range-selector combinator.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2020-07-28T17:26:12Z
New Revision: 04a21318b55756d50836f6e40f2d209f18cce417

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

LOG: [libTooling] Add a `between` range-selector combinator.

Adds the `between` combinator and registers it with the parser. As a driveby, 
updates some deprecated names to their current versions.

Reviewed By: gribozavr2

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

Added: 


Modified: 
clang/include/clang/Tooling/Transformer/RangeSelector.h
clang/lib/Tooling/Transformer/Parsing.cpp
clang/unittests/Tooling/RangeSelectorTest.cpp

Removed: 




diff  --git a/clang/include/clang/Tooling/Transformer/RangeSelector.h 
b/clang/include/clang/Tooling/Transformer/RangeSelector.h
index 2807037bc208..e070c0e7e2e6 100644
--- a/clang/include/clang/Tooling/Transformer/RangeSelector.h
+++ b/clang/include/clang/Tooling/Transformer/RangeSelector.h
@@ -56,6 +56,11 @@ RangeSelector before(RangeSelector Selector);
 /// * the TokenRange [B,E'] where the token at E' spans the range [E',E).
 RangeSelector after(RangeSelector Selector);
 
+/// Selects the range between `R1` and `R2.
+inline RangeSelector between(RangeSelector R1, RangeSelector R2) {
+  return enclose(after(std::move(R1)), before(std::move(R2)));
+}
+
 /// Selects a node, including trailing semicolon (for non-expression
 /// statements). \p ID is the node's binding in the match result.
 RangeSelector node(std::string ID);

diff  --git a/clang/lib/Tooling/Transformer/Parsing.cpp 
b/clang/lib/Tooling/Transformer/Parsing.cpp
index 1579115b9313..fb5fd4a800bb 100644
--- a/clang/lib/Tooling/Transformer/Parsing.cpp
+++ b/clang/lib/Tooling/Transformer/Parsing.cpp
@@ -109,14 +109,14 @@ getUnaryRangeSelectors() {
 static const llvm::StringMap> &
 getBinaryStringSelectors() {
   static const llvm::StringMap> M = {
-  {"encloseNodes", range}};
+  {"encloseNodes", encloseNodes}};
   return M;
 }
 
 static const llvm::StringMap> &
 getBinaryRangeSelectors() {
   static const llvm::StringMap>
-  M = {{"enclose", range}};
+  M = {{"enclose", enclose}, {"between", between}};
   return M;
 }
 

diff  --git a/clang/unittests/Tooling/RangeSelectorTest.cpp 
b/clang/unittests/Tooling/RangeSelectorTest.cpp
index e2d7723eab11..64ddee7894eb 100644
--- a/clang/unittests/Tooling/RangeSelectorTest.cpp
+++ b/clang/unittests/Tooling/RangeSelectorTest.cpp
@@ -193,8 +193,33 @@ TEST(RangeSelectorTest, AfterOp) {
HasValue(EqualsCharSourceRange(ExpectedAfter)));
 }
 
+TEST(RangeSelectorTest, BetweenOp) {
+  StringRef Code = R"cc(
+int f(int x, int y, int z) { return 3; }
+int g() { return f(3, /* comment */ 7 /* comment */, 9); }
+  )cc";
+  auto Matcher = callExpr(hasArgument(0, expr().bind("a0")),
+  hasArgument(1, expr().bind("a1")));
+  RangeSelector R = between(node("a0"), node("a1"));
+  TestMatch Match = matchCode(Code, Matcher);
+  EXPECT_THAT_EXPECTED(select(R, Match), HasValue(", /* comment */ "));
+}
+
+TEST(RangeSelectorTest, BetweenOpParsed) {
+  StringRef Code = R"cc(
+int f(int x, int y, int z) { return 3; }
+int g() { return f(3, /* comment */ 7 /* comment */, 9); }
+  )cc";
+  auto Matcher = callExpr(hasArgument(0, expr().bind("a0")),
+  hasArgument(1, expr().bind("a1")));
+  auto R = parseRangeSelector(R"rs(between(node("a0"), node("a1")))rs");
+  ASSERT_THAT_EXPECTED(R, llvm::Succeeded());
+  TestMatch Match = matchCode(Code, Matcher);
+  EXPECT_THAT_EXPECTED(select(*R, Match), HasValue(", /* comment */ "));
+}
+
 // Node-id specific version.
-TEST(RangeSelectorTest, RangeOpNodes) {
+TEST(RangeSelectorTest, EncloseOpNodes) {
   StringRef Code = R"cc(
 int f(int x, int y, int z) { return 3; }
 int g() { return f(/* comment */ 3, 7 /* comment */, 9); }
@@ -206,7 +231,7 @@ TEST(RangeSelectorTest, RangeOpNodes) {
   EXPECT_THAT_EXPECTED(select(R, Match), HasValue("3, 7"));
 }
 
-TEST(RangeSelectorTest, RangeOpGeneral) {
+TEST(RangeSelectorTest, EncloseOpGeneral) {
   StringRef Code = R"cc(
 int f(int x, int y, int z) { return 3; }
 int g() { return f(/* comment */ 3, 7 /* comment */, 9); }
@@ -218,7 +243,7 @@ TEST(RangeSelectorTest, RangeOpGeneral) {
   EXPECT_THAT_EXPECTED(select(R, Match), HasValue("3, 7"));
 }
 
-TEST(RangeSelectorTest, RangeOpNodesParsed) {
+TEST(RangeSelectorTest, EncloseOpNodesParsed) {
   StringRef Code = R"cc(
 int f(int x, int y, int z) { return 3; }
 int g() { return f(/* comment */ 3, 7 /* comment */, 9); }
@@ -231,7 +256,7 @@ TEST(RangeSelectorTest, RangeOpNodesParsed) {
   EXPECT_THAT_EXPECTED(select(*R, Match), HasValue("3, 7"));
 }
 
-TEST(RangeSelectorTest, RangeOpGeneralParsed) {
+TEST(RangeSelectorTest, EncloseOpGeneral

[PATCH] D84315: [libTooling] Add a `between` range-selector combinator.

[Yitzhak Mandelbaum 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 rG04a21318b557: [libTooling] Add a `between` range-selector 
combinator. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84315

Files:
  clang/include/clang/Tooling/Transformer/RangeSelector.h
  clang/lib/Tooling/Transformer/Parsing.cpp
  clang/unittests/Tooling/RangeSelectorTest.cpp

Index: clang/unittests/Tooling/RangeSelectorTest.cpp
===
--- clang/unittests/Tooling/RangeSelectorTest.cpp
+++ clang/unittests/Tooling/RangeSelectorTest.cpp
@@ -193,8 +193,33 @@
HasValue(EqualsCharSourceRange(ExpectedAfter)));
 }
 
+TEST(RangeSelectorTest, BetweenOp) {
+  StringRef Code = R"cc(
+int f(int x, int y, int z) { return 3; }
+int g() { return f(3, /* comment */ 7 /* comment */, 9); }
+  )cc";
+  auto Matcher = callExpr(hasArgument(0, expr().bind("a0")),
+  hasArgument(1, expr().bind("a1")));
+  RangeSelector R = between(node("a0"), node("a1"));
+  TestMatch Match = matchCode(Code, Matcher);
+  EXPECT_THAT_EXPECTED(select(R, Match), HasValue(", /* comment */ "));
+}
+
+TEST(RangeSelectorTest, BetweenOpParsed) {
+  StringRef Code = R"cc(
+int f(int x, int y, int z) { return 3; }
+int g() { return f(3, /* comment */ 7 /* comment */, 9); }
+  )cc";
+  auto Matcher = callExpr(hasArgument(0, expr().bind("a0")),
+  hasArgument(1, expr().bind("a1")));
+  auto R = parseRangeSelector(R"rs(between(node("a0"), node("a1")))rs");
+  ASSERT_THAT_EXPECTED(R, llvm::Succeeded());
+  TestMatch Match = matchCode(Code, Matcher);
+  EXPECT_THAT_EXPECTED(select(*R, Match), HasValue(", /* comment */ "));
+}
+
 // Node-id specific version.
-TEST(RangeSelectorTest, RangeOpNodes) {
+TEST(RangeSelectorTest, EncloseOpNodes) {
   StringRef Code = R"cc(
 int f(int x, int y, int z) { return 3; }
 int g() { return f(/* comment */ 3, 7 /* comment */, 9); }
@@ -206,7 +231,7 @@
   EXPECT_THAT_EXPECTED(select(R, Match), HasValue("3, 7"));
 }
 
-TEST(RangeSelectorTest, RangeOpGeneral) {
+TEST(RangeSelectorTest, EncloseOpGeneral) {
   StringRef Code = R"cc(
 int f(int x, int y, int z) { return 3; }
 int g() { return f(/* comment */ 3, 7 /* comment */, 9); }
@@ -218,7 +243,7 @@
   EXPECT_THAT_EXPECTED(select(R, Match), HasValue("3, 7"));
 }
 
-TEST(RangeSelectorTest, RangeOpNodesParsed) {
+TEST(RangeSelectorTest, EncloseOpNodesParsed) {
   StringRef Code = R"cc(
 int f(int x, int y, int z) { return 3; }
 int g() { return f(/* comment */ 3, 7 /* comment */, 9); }
@@ -231,7 +256,7 @@
   EXPECT_THAT_EXPECTED(select(*R, Match), HasValue("3, 7"));
 }
 
-TEST(RangeSelectorTest, RangeOpGeneralParsed) {
+TEST(RangeSelectorTest, EncloseOpGeneralParsed) {
   StringRef Code = R"cc(
 int f(int x, int y, int z) { return 3; }
 int g() { return f(/* comment */ 3, 7 /* comment */, 9); }
Index: clang/lib/Tooling/Transformer/Parsing.cpp
===
--- clang/lib/Tooling/Transformer/Parsing.cpp
+++ clang/lib/Tooling/Transformer/Parsing.cpp
@@ -109,14 +109,14 @@
 static const llvm::StringMap> &
 getBinaryStringSelectors() {
   static const llvm::StringMap> M = {
-  {"encloseNodes", range}};
+  {"encloseNodes", encloseNodes}};
   return M;
 }
 
 static const llvm::StringMap> &
 getBinaryRangeSelectors() {
   static const llvm::StringMap>
-  M = {{"enclose", range}};
+  M = {{"enclose", enclose}, {"between", between}};
   return M;
 }
 
Index: clang/include/clang/Tooling/Transformer/RangeSelector.h
===
--- clang/include/clang/Tooling/Transformer/RangeSelector.h
+++ clang/include/clang/Tooling/Transformer/RangeSelector.h
@@ -56,6 +56,11 @@
 /// * the TokenRange [B,E'] where the token at E' spans the range [E',E).
 RangeSelector after(RangeSelector Selector);
 
+/// Selects the range between `R1` and `R2.
+inline RangeSelector between(RangeSelector R1, RangeSelector R2) {
+  return enclose(after(std::move(R1)), before(std::move(R2)));
+}
+
 /// Selects a node, including trailing semicolon (for non-expression
 /// statements). \p ID is the node's binding in the match result.
 RangeSelector node(std::string ID);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84774: [NewPM][PassInstrument] Add PrintPass callback to StandardInstrumentations

[Yuanfang Chen via Phabricator via cfe-commits]

ychen created this revision.
ychen added reviewers: asbirlea, aeubanks, hans.
Herald added subscribers: llvm-commits, cfe-commits, dexonsmith, steven_wu, 
zzheng, hiraditya.
Herald added projects: clang, LLVM.
ychen requested review of this revision.

Problem:
Right now, our "Running pass" is not accurate when passes are wrapped in 
adaptor because adaptor is never skipped and a pass could be skipped. The other 
problem is that "Running pass" for a adaptor is before any "Running pass" of 
passes/analyses it depends on. (for example, FunctionToLoopPassAdaptor). So the 
order of printing the not the actual order.

Solution:
Doing things like PassManager::Debuglogging is very intrusive because we need 
to specify Debuglogging whenever adaptor is created. (Actually, right now we're 
not specifying Debuglogging for some sub-PassManagers. Check PassBuilder)

This patch move debug logging for pass as a PassInstrument callback. We could 
be sure that all running passes are logged and in the correct order.

This could also be used to implement hierarchy pass logging in legacy PM. We 
could also move logging of pass manager to this if we want.

The test fixes looks messy. It includes changes:

- Remove PassInstrumentationAnalysis
- Remove PassAdaptor
- If a PassAdaptor is for a real pass, the pass is added
- Pass reorder (to the correct order), related to PassAdaptor
- Add missing passes (due to Debuglogging not passed down)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84774

Files:
  clang/test/CodeGen/thinlto-distributed-newpm.ll
  llvm/include/llvm/IR/PassInstrumentation.h
  llvm/include/llvm/IR/PassManager.h
  llvm/include/llvm/IR/PassManagerImpl.h
  llvm/include/llvm/Passes/StandardInstrumentations.h
  llvm/lib/Analysis/CGSCCPassManager.cpp
  llvm/lib/IR/PassInstrumentation.cpp
  llvm/lib/IR/PassTimingInfo.cpp
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/lib/Transforms/Scalar/LoopPassManager.cpp
  llvm/test/Other/loop-pm-invalidation.ll
  llvm/test/Other/new-pass-manager.ll
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-lto-defaults.ll
  llvm/test/Other/new-pm-pgo.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-prelink-samplepgo-defaults.ll
  llvm/test/Other/pass-pipeline-parsing.ll
  llvm/test/Transforms/LoopRotate/pr35210.ll
  llvm/test/Transforms/LoopUnroll/revisit.ll
  llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
  llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll

Index: llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
===
--- llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
+++ llvm/test/Transforms/SCCP/ipsccp-preserve-analysis.ll
@@ -19,7 +19,6 @@
 ; NEW-PM-NEXT: Invalidating all non-preserved analyses for:
 ; NEW-PM-NEXT: Invalidating all non-preserved analyses for: f1
 ; NEW-PM-NEXT: Invalidating all non-preserved analyses for: f2
-; NEW-PM-NEXT: Running pass: ModuleToFunctionPassAdaptor
 ; NEW-PM-NOT: Running analysis:
 
 ; IR-LABEL: @f1
Index: llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
===
--- llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
+++ llvm/test/Transforms/LoopUnroll/unroll-loop-invalidation.ll
@@ -6,20 +6,19 @@
 ; RUN: opt -S -passes='loop(require),loop-unroll,loop(print-access-info)' -debug-pass-manager < %s 2>&1 | FileCheck %s
 ;
 ; CHECK: Starting llvm::Function pass manager run.
-; CHECK: Running pass: FunctionToLoopPassAdaptor
 ; CHECK: Running analysis: LoopAnalysis
 ; CHECK: Running analysis: InnerAnalysisManagerProxy<
 ; CHECK: Starting Loop pass manager run.
 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
-; CHECK: Running analysis: LoopAccessAnalysis on inner1.header
+; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 2 containing: %inner1.header
 ; CHECK: Finished Loop pass manager run.
 ; CHECK: Starting Loop pass manager run.
 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
-; CHECK: Running analysis: LoopAccessAnalysis on inner2.header
+; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 2 containing: %inner2.header
 ; CHECK: Finished Loop pass manager run.
 ; CHECK: Starting Loop pass manager run.
 ; CHECK: Running pass: RequireAnalysisPass<{{.*}}LoopAccessAnalysis
-; CHECK: Running analysis: LoopAccessAnalysis on outer.header
+; CHECK: Running analysis: LoopAccessAnalysis on Loop at depth 1 containing: %outer.header
 ; CHECK: Finished Loop pass manager run.
 ; CHECK: Running pass: LoopUnrollPass
 ; CHECK: Clearing all analysis results for: inner2.header
@@ -29,16 +28,15 @@
 ; CHECK: Invalidating analysis: LoopAccessAnalysis on inner1.header
 ; CHECK: Invalidating all non-prese

[PATCH] D84457: [OPENMP]Fix PR37671: Privatize local(private) variables in untied tasks.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev updated this revision to Diff 281290.
ABataev added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84457

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/task_codegen.cpp

Index: clang/test/OpenMP/task_codegen.cpp
===
--- clang/test/OpenMP/task_codegen.cpp
+++ clang/test/OpenMP/task_codegen.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix UNTIEDRT
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -emit-pch -o %t %s
 // RUN: %clang_cc1 -fopenmp -x c++ -triple x86_64-apple-darwin10 -include-pch %t -verify %s -emit-llvm -o - | FileCheck %s
 //
@@ -259,7 +259,7 @@
 a = 4;
 c = 5;
   }
-// CHECK: [[ORIG_TASK_PTR:%.+]] = call i8* @__kmpc_omp_task_alloc([[IDENT_T]]* @{{.+}}, i32 [[GTID]], i32 0, i64 40, i64 1, i32 (i32, i8*)* bitcast (i32 (i32, [[KMP_TASK_T]]{{.*}}*)* [[TASK_ENTRY6:@.+]] to i32 (i32, i8*)*))
+// CHECK: [[ORIG_TASK_PTR:%.+]] = call i8* @__kmpc_omp_task_alloc([[IDENT_T]]* @{{.+}}, i32 [[GTID]], i32 0, i64 48, i64 1, i32 (i32, i8*)* bitcast (i32 (i32, [[KMP_TASK_T]]{{.*}}*)* [[TASK_ENTRY6:@.+]] to i32 (i32, i8*)*))
 // CHECK: call i32 @__kmpc_omp_task([[IDENT_T]]* @{{.+}}, i32 [[GTID]], i8* [[ORIG_TASK_PTR]])
 #pragma omp task untied
   {
@@ -296,26 +296,54 @@
 // CHECK: store i32 4, i32* [[A_PTR]]
 
 // CHECK: define internal i32 [[TASK_ENTRY6]](i32 %0, [[KMP_TASK_T]]{{.*}}* noalias %1)
-// CHECK: switch i32 %{{.+}}, label
+// UNTIEDRT: [[S1_ADDR_PTR:%.+]] = alloca %struct.S*,
+// UNTIEDRT: call void (i8*, ...) %{{.+}}(i8* %{{.+}}, %struct.S** [[S1_ADDR_PTR]])
+// UNTIEDRT: [[S1_ADDR:%.+]] = load %struct.S*, %struct.S** [[S1_ADDR_PTR]],
+// CHECK: switch i32 %{{.+}}, label %[[DONE:.+]] [
+
+// CHECK: [[DONE]]:
+// CHECK: br label %[[CLEANUP:[^,]+]]
+
 // CHECK: load i32*, i32** %
 // CHECK: store i32 1, i32* %
 // CHECK: call i32 @__kmpc_omp_task(%
+// UNTIEDRT: br label %[[EXIT:[^,]+]]
 
+// UNTIEDRT: call void [[CONSTR:@.+]](%struct.S* [[S1_ADDR]])
 // CHECK: call i8* @__kmpc_omp_task_alloc(
 // CHECK: call i32 @__kmpc_omp_task(%
 // CHECK: load i32*, i32** %
 // CHECK: store i32 2, i32* %
 // CHECK: call i32 @__kmpc_omp_task(%
+// UNTIEDRT: br label %[[EXIT]]
 
 // CHECK: call i32 @__kmpc_omp_taskyield(%
 // CHECK: load i32*, i32** %
 // CHECK: store i32 3, i32* %
 // CHECK: call i32 @__kmpc_omp_task(%
+// UNTIEDRT: br label %[[EXIT]]
+
+// s1 = S();
+// UNTIEDRT: call void [[CONSTR]](%struct.S* [[TMP:%.+]])
+// UNTIEDRT: [[DST:%.+]] = bitcast %struct.S* [[S1_ADDR]] to i8*
+// UNTIEDRT: [[SRC:%.+]] = bitcast %struct.S* [[TMP]] to i8*
+// UNTIEDRT: call void @llvm.memcpy.{{.+}}(i8* {{.*}}[[DST]], i8* {{.*}}[[SRC]], i64 4, i1 false)
+// UNTIEDRT: call void [[DESTR:@.+]](%struct.S* [[TMP]])
 
 // CHECK: call i32 @__kmpc_omp_taskwait(%
 // CHECK: load i32*, i32** %
 // CHECK: store i32 4, i32* %
 // CHECK: call i32 @__kmpc_omp_task(%
+// UNTIEDRT: br label %[[EXIT]]
+
+// UNTIEDRT: call void [[DESTR]](%struct.S* [[S1_ADDR]])
+// CHECK: br label %[[CLEANUP]]
+
+// CHECK: [[CLEANUP]]:
+// UNTIEDRT: br label %[[EXIT]]
+
+// UNTIEDRT:  [[EXIT]]:
+// UNTIEDRT-NEXT: ret i32 0
 
 struct S1 {
   int a;
Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -21,6 +21,7 @@
 #include "clang/AST/OpenMPClause.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtOpenMP.h"
+#include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Basic/PrettyStackTrace.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
@@ -3785,6 +3786,42 @@
   checkForLastprivateConditionalUpdate(*this, S);
 }
 
+namespace {
+/// Get the list of variables declared in the context of the untied tasks.
+class CheckVarsEscapingUntiedTaskDeclContext final
+: public ConstStmtVisitor {
+  llvm::SmallVector PrivateDecls;
+
+public:
+  explicit CheckVarsEscapingUntiedTaskDeclContext() = default;
+  virtual ~CheckVarsEscapingUntiedTaskDeclContext() = default;
+  void VisitDeclStmt(const DeclStmt *S) {
+if (!S)
+  return;
+// Need to privatize only local vars, static locals can be processed as is.
+for (const Decl *D : S->decls()) {
+  if (const auto *VD = dyn_cast_or_null(D))
+if (VD->hasLocalStorage())
+  PrivateDecls.push_back(VD);
+}
+  }
+  void VisitOMPExecutableDirective(const OMPExecutableDirective *) { return; }
+  void VisitCapturedStmt(const CapturedStmt *) { return; }
+  void VisitLambdaExpr(const LambdaExpr *) { return;

[PATCH] D84774: [NewPM][PassInstrument] Add PrintPass callback to StandardInstrumentations

[Alina Sbirlea via Phabricator via cfe-commits]

asbirlea added a comment.

Does it make sense to have the option to enable seeing where the adaptors are 
run?

Otherwise this looks good, it's nice to have the "Starting" and "Finished" 
match now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84774

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

[PATCH] D84736: [analyzer][RFC] Handle pointer difference of ElementRegion and SymbolicRegion

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

I think adding code snippets that are affected by this patch would make it much 
easier to evaluate the changes and whether this is a good idea or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84736

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

[PATCH] D84774: [NewPM][PassInstrument] Add PrintPass callback to StandardInstrumentations

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D84774#2179410 , @asbirlea wrote:

> Does it make sense to have the option to enable seeing where the adaptors are 
> run?

Adding a static:cl at the top of StandardInstrumentation.cpp should do it. Do 
you want me to add it now or add it when we have use cases?

> Otherwise this looks good, it's nice to have the "Starting" and "Finished" 
> match now.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84774

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

[PATCH] D71726: Let clang atomic builtins fetch add/sub support floating point types

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 281296.
yaxunl added a comment.

Make IEEE single and double type as supported for fp atomics in all targets by 
default. This is based on the assumption that AtomicExpandPass or its ongoing 
work is sufficient to support fp atomics for all targets. This is to facilitate 
middle end and backend end development to support fp atomics.

If a target would like to treat single and double fp atomics as unsupported, it 
can override the default behavior in its own TargetInfo.


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

https://reviews.llvm.org/D71726

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/CodeGen/CGAtomic.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGenCUDA/amdgpu-atomic-ops.cu
  clang/test/CodeGenOpenCL/atomic-ops.cl
  clang/test/Sema/atomic-ops.c
  clang/test/SemaCUDA/amdgpu-atomic-ops.cu
  clang/test/SemaOpenCL/atomic-ops.cl

Index: clang/test/SemaOpenCL/atomic-ops.cl
===
--- clang/test/SemaOpenCL/atomic-ops.cl
+++ clang/test/SemaOpenCL/atomic-ops.cl
@@ -1,10 +1,13 @@
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only -triple=spir64
-// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only -triple=amdgcn-amdhsa-amd-opencl
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify=expected,spir \
+// RUN:   -fsyntax-only -triple=spir64
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -verify -fsyntax-only \
+// RUN:   -triple=amdgcn-amd-amdhsa
 
 // Basic parsing/Sema tests for __opencl_atomic_*
 
 #pragma OPENCL EXTENSION cl_khr_int64_base_atomics : enable
 #pragma OPENCL EXTENSION cl_khr_int64_extended_atomics : enable
+#pragma OPENCL EXTENSION cl_khr_fp16 : enable
 
 typedef __INTPTR_TYPE__ intptr_t;
 typedef int int8 __attribute__((ext_vector_type(8)));
@@ -36,7 +39,7 @@
 
 atomic_int gn;
 void f(atomic_int *i, const atomic_int *ci,
-   atomic_intptr_t *p, atomic_float *d,
+   atomic_intptr_t *p, atomic_float *d, atomic_double *d2, atomic_half *h, // expected-error {{unknown type name 'atomic_half'}}
int *I, const int *CI,
intptr_t *P, float *D, struct S *s1, struct S *s2,
global atomic_int *i_g, local atomic_int *i_l, private atomic_int *i_p,
@@ -70,7 +73,8 @@
 
   __opencl_atomic_fetch_add(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_add(p, 1, memory_order_seq_cst, memory_scope_work_group);
-  __opencl_atomic_fetch_add(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer or pointer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
+  __opencl_atomic_fetch_add(d, 1.0f, memory_order_seq_cst, memory_scope_work_group);
+  __opencl_atomic_fetch_add(d2, 1.0, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_and(i, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_and(p, 1, memory_order_seq_cst, memory_scope_work_group);
   __opencl_atomic_fetch_and(d, 1, memory_order_seq_cst, memory_scope_work_group); // expected-error {{address argument to atomic operation must be a pointer to atomic integer ('__generic atomic_float *' (aka '__generic _Atomic(float) *') invalid)}}
Index: clang/test/SemaCUDA/amdgpu-atomic-ops.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/amdgpu-atomic-ops.cu
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -verify -fsyntax-only -triple=amdgcn-amd-amdhsa \
+// RUN:   -fcuda-is-device -target-cpu gfx906 -fnative-half-type \
+// RUN:   -fnative-half-arguments-and-returns
+
+// REQUIRES: amdgpu-registered-target
+
+#include "Inputs/cuda.h"
+#include 
+
+__device__ _Float16 test_Flot16(_Float16 *p) {
+  return __atomic_fetch_sub(p, 1.0f16, memory_order_relaxed);
+  // expected-error@-1 {{address argument to atomic operation must be a pointer to integer, pointer or supported floating point type ('_Float16 *' invalid)}}
+}
+
+__device__ __fp16 test_fp16(__fp16 *p) {
+  return __atomic_fetch_sub(p, 1.0f16, memory_order_relaxed);
+  // expected-error@-1 {{address argument to atomic operation must be a pointer to integer, pointer or supported floating point type ('__fp16 *' invalid)}}
+}
Index: clang/test/Sema/atomic-ops.c
===
--- clang/test/Sema/atomic-ops.c
+++ clang/test/Sema/atomic-ops.c
@@ -99,7 +99,8 @@
 #define _AS2 __attribute__((address_space(2)))
 
 void f(_Atomic(int) *i, const _Atomic(int) *ci,
-   _Atomic(int*) *p, _Atomic(float) *d,
+   _Atomic(int*) *p, _Atomic(float) *d, _Atomic(double) *d2,
+   _Atomic(long double) *d3,
int *I, const int *CI,
int **P, float *D, struct S *s1, struct S *s2) {
   __c11_atomic_init(I, 5); // expected-error {{pointer to _Atomic}}
@@ -166,13 +167,15 @@
 
   __c11_atomic_fetch_add(i, 1, memory_order_se

[PATCH] D84743: [Clang][AMDGCN] Universal device offloading macros header

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

I'm not sure it's particularly useful, to be honest. CUDA code still needs to 
be compatible with NVCC so it can't be used in portable code like TF or other 
currently used CUDA libraries.
It could be useful internally, though, so I'm fine with it for that purpose.




Comment at: clang/lib/Headers/offload_macros.h:28
+#undef _DEVICE_GPU
+#undef _DEVICE_ARCH
+

The naming seems to conflict with the current notation that GPU `arch` is the 
specific GPU variant. E.g. `gfx900` or `sm_60`.
Perhaps we should use a higher level term. `kind`, `vendor`?



Comment at: clang/lib/Headers/offload_macros.h:33
+// So if either set, this is an OpenMP  device pass.
+#if defined(__AMDGCN__) || defined(__NVPTX__)
+#if defined(__AMDGCN__)

I'd just split it into separate `if` sections for AMDGCN and NVPTX. One less 
nesting level for preprocessor conditionals would be easier to follow.



Comment at: clang/lib/Headers/offload_macros.h:35
+#if defined(__AMDGCN__)
+#define _DEVICE_ARCH amdgcn
+// _DEVICE_GPU set below

What exactly is `amdgcn` and how can it be used in practice? I.e. I can't use 
it in preprocessor conditionals.

I think you need to have numberic constants defined for the different `ARCH` 
variants.



Comment at: clang/lib/Headers/offload_macros.h:37
+// _DEVICE_GPU set below
+#else
+#define _DEVICE_ARCH nvptx64

Please add a comment tracking which conditional this `else` is for. E.g. `// 
__AMDGCN__`



Comment at: clang/lib/Headers/offload_macros.h:38
+#else
+#define _DEVICE_ARCH nvptx64
+#define _DEVICE_GPU __CUDA_ARCH__

Nit -- there's techically 32-bit nvptx, even though it's getting obsolete.



Comment at: clang/lib/Headers/offload_macros.h:71
+
+#if defined(_DEVICE_ARCH) && (_DEVICE_ARCH == amdgcn)
+// AMD uses binary macros only, so create a value for _DEVICE_GPU

This does not work, does it? https://godbolt.org/z/Kn3r4x


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84743

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

[PATCH] D84774: [NewPM][PassInstrument] Add PrintPass callback to StandardInstrumentations

[Alina Sbirlea via Phabricator via cfe-commits]

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

I'm ok if this is added in either this or a follow-up patch.
I think it's useful to have it in the codebase now, with one test to showcase 
it (e.g. do another run line in `new-pass-manager.ll`, with `check-prefixes`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84774

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

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8686
+CodeGenFunction &CGF, MappableExprsHandler::MapCombinedInfoTy 
&CombinedInfo,
+CGOpenMPRuntime::TargetDataInfo &Info, bool SeparateBeginEnd) {
   CodeGenModule &CGM = CGF.CGM;

Can this new flag be encapsulated in `Info`?


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

https://reviews.llvm.org/D84422

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

[PATCH] D84710: [OpenMP][NFC] Consolidate `to` and `from` clause modifiers

[Alexey Bataev via Phabricator via cfe-commits]

ABataev accepted this revision.
ABataev 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/D84710/new/

https://reviews.llvm.org/D84710

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

[clang] 394db22 - Revert "Switch to using -debug-info-kind=constructor as default (from =limited)"

[Amy Huang via cfe-commits]

Author: Amy Huang
Date: 2020-07-28T11:23:59-07:00
New Revision: 394db2259575ef3cac8d3d37836b11eb2373c435

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

LOG: Revert "Switch to using -debug-info-kind=constructor as default (from 
=limited)"

This reverts commit 227db86a1b7dd6f96f7df14890fcd071bc4fe1f5.

Causing debug info errors in google3 LTO builds; also causes a
debuginfo-test failure.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/cl-options.c
clang/test/Driver/clang-g-opts.c
clang/test/Driver/cuda-dwarf-2.cu
clang/test/Driver/debug-options-as.c
clang/test/Driver/debug-options.c
clang/test/Driver/integrated-as.s
clang/test/Driver/myriad-toolchain.c
clang/test/Driver/openmp-offload-gpu.c
clang/test/Driver/split-debug.c
lldb/test/Shell/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index b0de225f8abf..68e4eb0eedda 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -498,7 +498,7 @@ static codegenoptions::DebugInfoKind 
DebugLevelToInfoKind(const Arg &A) {
 return codegenoptions::DebugLineTablesOnly;
   if (A.getOption().matches(options::OPT_gline_directives_only))
 return codegenoptions::DebugDirectivesOnly;
-  return codegenoptions::DebugInfoConstructor;
+  return codegenoptions::LimitedDebugInfo;
 }
 
 static bool mustUseNonLeafFramePointerForTarget(const llvm::Triple &Triple) {
@@ -2383,7 +2383,7 @@ static void CollectArgsForIntegratedAssembler(Compilation 
&C,
   CmdArgs.push_back(Value.data());
 } else {
   RenderDebugEnablingArgs(Args, CmdArgs,
-  codegenoptions::DebugInfoConstructor,
+  codegenoptions::LimitedDebugInfo,
   DwarfVersion, llvm::DebuggerKind::Default);
 }
   } else if (Value.startswith("-mcpu") || Value.startswith("-mfpu") ||
@@ -3656,7 +3656,7 @@ static void RenderDebugOptions(const ToolChain &TC, const 
Driver &D,
   if (const Arg *A =
   Args.getLastArg(options::OPT_g_Group, options::OPT_gsplit_dwarf,
   options::OPT_gsplit_dwarf_EQ)) {
-DebugInfoKind = codegenoptions::DebugInfoConstructor;
+DebugInfoKind = codegenoptions::LimitedDebugInfo;
 
 // If the last option explicitly specified a debug-info level, use it.
 if (checkDebugInfoOption(A, Args, D, TC) &&
@@ -3761,7 +3761,7 @@ static void RenderDebugOptions(const ToolChain &TC, const 
Driver &D,
 if (checkDebugInfoOption(A, Args, D, TC)) {
   if (DebugInfoKind != codegenoptions::DebugLineTablesOnly &&
   DebugInfoKind != codegenoptions::DebugDirectivesOnly) {
-DebugInfoKind = codegenoptions::DebugInfoConstructor;
+DebugInfoKind = codegenoptions::LimitedDebugInfo;
 CmdArgs.push_back("-dwarf-ext-refs");
 CmdArgs.push_back("-fmodule-format=obj");
   }
@@ -3781,9 +3781,7 @@ static void RenderDebugOptions(const ToolChain &TC, const 
Driver &D,
   TC.GetDefaultStandaloneDebug());
   if (const Arg *A = Args.getLastArg(options::OPT_fstandalone_debug))
 (void)checkDebugInfoOption(A, Args, D, TC);
-  if ((DebugInfoKind == codegenoptions::LimitedDebugInfo ||
-   DebugInfoKind == codegenoptions::DebugInfoConstructor) &&
-  NeedFullDebug)
+  if (DebugInfoKind == codegenoptions::LimitedDebugInfo && NeedFullDebug)
 DebugInfoKind = codegenoptions::FullDebugInfo;
 
   if (Args.hasFlag(options::OPT_gembed_source, options::OPT_gno_embed_source,
@@ -6569,7 +6567,7 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID 
InputType,
   options::OPT_gline_tables_only)) {
 *EmitCodeView = true;
 if (DebugInfoArg->getOption().matches(options::OPT__SLASH_Z7))
-  *DebugInfoKind = codegenoptions::DebugInfoConstructor;
+  *DebugInfoKind = codegenoptions::LimitedDebugInfo;
 else
   *DebugInfoKind = codegenoptions::DebugLineTablesOnly;
   } else {
@@ -6866,7 +6864,7 @@ void ClangAs::ConstructJob(Compilation &C, const 
JobAction &JA,
 // the guard for source type, however there is a test which asserts
 // that some assembler invocation receives no -debug-info-kind,
 // and it's not clear whether that test is just overly restrictive.
-DebugInfoKind = (WantDebug ? codegenoptions::DebugInfoConstructor
+DebugInfoKind = (WantDebug ? codegenoptions::LimitedDebugInfo
: codegenoptions::NoDebugInfo);
 // Add the -fdebug-compilation-dir flag if needed.
 addDebugCompDirArg(Args, CmdArgs, C.getDriver().getVFS());

diff  --git a/clang/test/Driver/cl-options.

[PATCH] D83997: [os_log] Improve the way we extend the lifetime of objects passed to __builtin_os_log_format

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D83997#2169745 , @ahatanak wrote:

> The use case for this is a macro in which the call to 
> `__builtin_os_log_format` that writes to the buffer and the call that uses 
> the buffer appear in two different statements. For example:
>
>   __builtin_os_log_format(buf, "%@", getObj());
>   ...
>   use_buffer(buf);
>
> The object returned by the call to `getObj` has to be kept alive until 
> `use_buffer` is called, but currently it gets destructed at the end of the 
> full expression. I think an alternate solution would be to provide users a 
> means to tell ARC optimizer not to move the release call for a local variable 
> past any calls, i.e., something that is stricter than 
> `NS_VALID_UNTIL_END_OF_SCOPE`, but that places more burden on the users.
>
> In the `os_log` macro, the result of the call to `__builtin_os_log_format` is 
> passed directly to the call that uses the buffer, so it doesn't require any 
> lifetime extension as you pointed out.

So are there actually any uses that take advantage of these semantics?  Because 
as I understand it, this builtin exists entirely to support the `os_log` macro. 
 If that macro doesn't need the extension semantics, let's just not do them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83997

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

[PATCH] D84316: [analyzer][NFC] Split CStringChecker to modeling and reporting

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

steakhal added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CStringChecker/CStringLength.h:43
+ ProgramStateRef &State, const Expr *Ex,
+ SVal Buf, bool Hypothetical = false);
+

steakhal wrote:
> balazske wrote:
> > I do not like that the //get// and //set// (CStringLength) functions are 
> > not symmetrical. I (and other developers) would think that the get function 
> > returns a stored value and the set function sets it. The `getCStringLength` 
> > is more a `computeCStringLength` and additionally may manipulate the 
> > `State` too. In this form it is usable mostly only for CStringChecker. (A 
> > separate function to get the value stored in the length map should exist 
> > instead of this `Hypothetical` thing.)
> > [...] get function returns a stored value and the set function sets it.
> Certainly a burden to understand. It would be more appealing, but more useful?
> The user would have to check and create if necessary regardless. So fusing 
> these two functions is more like a feature.
> What use case do you think of using only the query function? In other words, 
> how can you guarantee that you will find a length for a symbol?
> 
> > In this form it is usable mostly only for CStringChecker. (A separate 
> > function to get the value stored in the length map should exist instead of 
> > this Hypothetical thing.)
> You are right. However, I want to focus on splitting parts without modifying 
> the already existing API reducing the risk of breaking things.
> You should expect such a change in an upcoming patch.
On second thought, It probably worth having a cleaner API to a slight 
inconvenience. If he feels like, still can wrap them.
I will investigate it tomorrow.


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

https://reviews.llvm.org/D84316

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

[PATCH] D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2)

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Basic/OpenMPKinds.cpp:73-74
 .Default(OMPC_MOTION_MODIFIER_unknown);
+if (OpenMPVersion < 51 && Type != OMPC_MOTION_MODIFIER_mapper)
+  return OMPC_MOTION_MODIFIER_unknown;
+return Type;

Better to check for the new clauses here:
```
if (OpenMPVersion >= 51 && Type == OMPC_MOTION_MODIFIER_present)

```



Comment at: clang/lib/Parse/ParseOpenMP.cpp:3462
+  // OpenMP 5.1 accepts an optional ',' even if the next character is ':'.
+  // TODO: Is that intentional?
+  if (Tok.is(tok::comma))

`FIXME`. This is a bug.



Comment at: clang/lib/Parse/ParseOpenMP.cpp:3476-3477
 }
+// OpenMP 5.1 permits a ':' even without a preceding modifier.  TODO: Is
+// that intentional?
+if ((!Data.MotionModifiers.empty() || getLangOpts().OpenMP >= 51) &&

`FIXME`. A bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84711

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

[PATCH] D84780: Setting the /bigobj option globally for Windows debug build. https://bugs.llvm.org/show_bug.cgi?id=46733

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam created this revision.
Herald added subscribers: llvm-commits, lldb-commits, cfe-commits, 
AlexeySotkin, msifontes, jurahul, Kayjukh, grosul1, bader, Joonsoo, liufengdb, 
aartbik, lucyrfox, mgester, arpith-jacob, antiagainst, shauheen, jpienaar, 
rriddle, mehdi_amini, hiraditya, mgorny.
Herald added a reviewer: mravishankar.
Herald added a reviewer: antiagainst.
Herald added projects: clang, LLDB, MLIR, LLVM.
zahiraam requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: sstefan1, stephenneuendorffer, nicolasvasilache, 
ormris.

Change-Id: Ie69e3b62ac4e7bd266ed48a76f4cbe9ec8a899d1


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84780

Files:
  clang/lib/ASTMatchers/Dynamic/CMakeLists.txt
  clang/lib/CodeGen/CMakeLists.txt
  clang/lib/Sema/CMakeLists.txt
  clang/unittests/AST/CMakeLists.txt
  clang/unittests/ASTMatchers/CMakeLists.txt
  clang/unittests/Tooling/CMakeLists.txt
  lldb/source/API/CMakeLists.txt
  llvm/cmake/modules/HandleLLVMOptions.cmake
  llvm/lib/Passes/CMakeLists.txt
  mlir/lib/Dialect/SPIRV/CMakeLists.txt

Index: mlir/lib/Dialect/SPIRV/CMakeLists.txt
===
--- mlir/lib/Dialect/SPIRV/CMakeLists.txt
+++ mlir/lib/Dialect/SPIRV/CMakeLists.txt
@@ -1,6 +1,3 @@
-if (MSVC)
-  set_source_files_properties(SPIRVDialect.cpp PROPERTIES COMPILE_FLAGS /bigobj)
-endif()
 
 set(LLVM_TARGET_DEFINITIONS SPIRVCanonicalization.td)
 mlir_tablegen(SPIRVCanonicalization.inc -gen-rewriters)
Index: llvm/lib/Passes/CMakeLists.txt
===
--- llvm/lib/Passes/CMakeLists.txt
+++ llvm/lib/Passes/CMakeLists.txt
@@ -1,7 +1,3 @@
-if (MSVC)
-  set_source_files_properties(PassBuilder.cpp PROPERTIES COMPILE_FLAGS /bigobj)
-endif()
-
 add_llvm_component_library(LLVMPasses
   PassBuilder.cpp
   PassPlugin.cpp
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -468,6 +468,10 @@
   endif()
 endif()
   endif()
+  # By default MSVC has a 2^16 limit on the number of sections in an object file,
+  # but in many objects files need more than that. This flag is to increase the
+  # number of sections.
+  append("/bigobj" CMAKE_CXX_FLAGS)
 endif( MSVC )
 
 # Warnings-as-errors handling for GCC-compatible compilers:
Index: lldb/source/API/CMakeLists.txt
===
--- lldb/source/API/CMakeLists.txt
+++ lldb/source/API/CMakeLists.txt
@@ -126,9 +126,6 @@
   set_property(TARGET liblldb APPEND PROPERTY BUILD_RPATH   "${PYTHON_RPATH}")
 endif()
 
-if (MSVC)
-  set_source_files_properties(SBReproducer.cpp PROPERTIES COMPILE_FLAGS /bigobj)
-endif()
 
 if(lldb_python_wrapper)
   add_dependencies(liblldb swig_wrapper)
Index: clang/unittests/Tooling/CMakeLists.txt
===
--- clang/unittests/Tooling/CMakeLists.txt
+++ clang/unittests/Tooling/CMakeLists.txt
@@ -4,14 +4,6 @@
   Support
   )
 
-# By default MSVC has a 2^16 limit on the number of sections in an object file,
-# and this needs more than that.
-if (MSVC)
-  set_source_files_properties(RecursiveASTVisitorTest.cpp PROPERTIES COMPILE_FLAGS /bigobj)
-  set_source_files_properties(RecursiveASTVisitorTestExprVisitor.cpp PROPERTIES COMPILE_FLAGS /bigobj)
-  set_source_files_properties(RecursiveASTVisitorTests/Callbacks.cpp PROPERTIES COMPILE_FLAGS /bigobj)
-  set_source_files_properties(SourceCodeTest.cpp PROPERTIES COMPILE_FLAGS /bigobj)
-endif()
 
 add_clang_unittest(ToolingTests
   ASTSelectionTest.cpp
Index: clang/unittests/ASTMatchers/CMakeLists.txt
===
--- clang/unittests/ASTMatchers/CMakeLists.txt
+++ clang/unittests/ASTMatchers/CMakeLists.txt
@@ -3,15 +3,6 @@
   Support
   )
 
-# By default MSVC has a 2^16 limit on the number of sections in an object file,
-# and this needs more than that.
-if (MSVC)
-  set_source_files_properties(InternalASTMatchersTest.cpp PROPERTIES COMPILE_FLAGS /bigobj)
-  set_source_files_properties(NodeMatchersTest.cpp PROPERTIES COMPILE_FLAGS /bigobj)
-  set_source_files_properties(NarrowingMatchersTest.cpp PROPERTIES COMPILE_FLAGS /bigobj)
-  set_source_files_properties(ASTTraversalMatchersTest.cpp PROPERTIES COMPILE_FLAGS /bigobj)
-endif()
-
 add_clang_unittest(ASTMatchersTests
   ASTMatchersInternalTest.cpp
   ASTMatchersNodeTest.cpp
Index: clang/unittests/AST/CMakeLists.txt
===
--- clang/unittests/AST/CMakeLists.txt
+++ clang/unittests/AST/CMakeLists.txt
@@ -3,9 +3,6 @@
   Support
   )
 
-if (MSVC)
-  set_source_files_properties(ASTImporterTest.cpp PROPERTIES COMPILE_FLAGS /bigobj)
-endif()
 
 add_clang_unittest(ASTTests
   ASTContextParentMapTest.cpp
Index: cl

[PATCH] D82898: [clang-tidy] Handled insertion only fixits when determining conflicts.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM with a few style nits.




Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:629
 Priority = std::make_tuple(Begin, Type, -End, -ErrorSize, ErrorId);
-  else
+return;
+  case ET_Insert:

I'd drop these `return` statements and the unreachable, below. The switch is 
fully-covered, so we'll get a diagnostic if any new enumerations are added and 
we forget to update this switch.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:703
+  Apply[Event.ErrorId] = false;
+continue;
+  case Event::ET_Insert:

Similar here with `continue` and the unreachable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82898

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

[PATCH] D84781: Use PointerUnion instead of inheritance for alternative clauses in NNS

[Eduardo Caldas via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84781

Files:
  clang/include/clang/Tooling/Syntax/Nodes.h
  clang/include/clang/Tooling/Syntax/Tree.h
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/lib/Tooling/Syntax/Nodes.cpp
  clang/lib/Tooling/Syntax/Tree.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp

Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -987,18 +987,19 @@
 | | |-IdExpression
 | | | |-NestedNameSpecifier
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-n
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-S
 | | | | |-::
-| | | | |-SimpleTemplateNameSpecifier
-| | | | | |-template
-| | | | | |-ST
-| | | | | |-<
-| | | | | |-int
-| | | | | `->
+| | | | |-NameSpecifier
+| | | | | `-SimpleTemplateSpecifier
+| | | | |   |-template
+| | | | |   |-ST
+| | | | |   |-<
+| | | | |   |-int
+| | | | |   `->
 | | | | `-::
 | | | `-UnqualifiedId
 | | |   `-f
@@ -1009,17 +1010,18 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-n
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-S
 | | | | |-::
-| | | | |-SimpleTemplateNameSpecifier
-| | | | | |-ST
-| | | | | |-<
-| | | | | |-int
-| | | | | `->
+| | | | |-NameSpecifier
+| | | | | `-SimpleTemplateSpecifier
+| | | | |   |-ST
+| | | | |   |-<
+| | | | |   |-int
+| | | | |   `->
 | | | | `-::
 | | | `-UnqualifiedId
 | | |   `-f
@@ -1030,13 +1032,14 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-SimpleTemplateNameSpecifier
-| | | | | |-ST
-| | | | | |-<
-| | | | | |-int
-| | | | | `->
+| | | | |-NameSpecifier
+| | | | | `-SimpleTemplateSpecifier
+| | | | |   |-ST
+| | | | |   |-<
+| | | | |   |-int
+| | | | |   `->
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-S
 | | | | `-::
 | | | `-UnqualifiedId
@@ -1051,13 +1054,14 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-SimpleTemplateNameSpecifier
-| | | | | |-ST
-| | | | | |-<
-| | | | | |-int
-| | | | | `->
+| | | | |-NameSpecifier
+| | | | | `-SimpleTemplateSpecifier
+| | | | |   |-ST
+| | | | |   |-<
+| | | | |   |-int
+| | | | |   `->
 | | | | |-::
-| | | | |-IdentifierNameSpecifier
+| | | | |-NameSpecifier
 | | | | | `-S
 | | | | `-::
 | | | |-template
@@ -1115,15 +1119,16 @@
   | |-UnknownExpression
   | | |-IdExpression
   | | | |-NestedNameSpecifier
-  | | | | |-IdentifierNameSpecifier
+  | | | | |-NameSpecifier
   | | | | | `-T
   | | | | |-::
-  | | | | |-SimpleTemplateNameSpecifier
-  | | | | | |-template
-  | | | | | |-U
-  | | | | | |-<
-  | | | | | |-int
-  | | | | | `->
+  | | | | |-NameSpecifier
+  | | | | | `-SimpleTemplateSpecifier
+  | | | | |   |-template
+  | | | | |   |-U
+  | | | | |   |-<
+  | | | | |   |-int
+  | | | | |   `->
   | | | | `-::
   | | | `-UnqualifiedId
   | | |   `-f
@@ -1134,10 +1139,10 @@
   | |-UnknownExpression
   | | |-IdExpression
   | | | |-NestedNameSpecifier
-  | | | | |-IdentifierNameSpecifier
+  | | | | |-NameSpecifier
   | | | | | `-T
   | | | | |-::
-  | | | | |-IdentifierNameSpecifier
+  | | | | |-NameSpecifier
   | | | | | `-U
   | | | | `-::
   | | | `-UnqualifiedId
@@ -1149,7 +1154,7 @@
   | |-UnknownExpression
   | | |-IdExpression
   | | | |-NestedNameSpecifier
-  | | | | |-IdentifierNameSpecifier
+  | | | | |-NameSpecifier
   | | | | | `-T
   | | | | `-::
   | | | |-template
@@ -1216,13 +1221,14 @@
 | |-UnknownExpression
 | | |-IdExpression
 | | | |-NestedNameSpecifier
-| | | | |-DecltypeNameSpecifier
-| | | | | |-decltype
-| | | | | |-(
-| | | | | |-IdExpression
-| | | | | | `-UnqualifiedId
-| | | | | |   `-s
-| | | | | `-)
+| | | | |-NameSpecifier
+| | | | | `-DecltypeSpecifier
+| | | | |   |-decltype
+| | | | |   |-(
+| | | | |   |-IdExpression
+| | | | |   | `-UnqualifiedId
+| | | | |   |   `-s
+| | | | |   `-)
 | | | | `-::
 | | | `-UnqualifiedId
 | | |   `-f
Index: clang/lib/Too

[PATCH] D84658: [clang] Overload NamedDecl::printName to provide a user-friendly name for unnamed entities

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/AST/PrettyPrinter.h:78
 
+  char getOpenDelimiter() const { return MSVCFormatting ? '`' : '('; }
+  char getCloseDelimiter() const { return MSVCFormatting ? '\'' : ')'; }

These names a bit too generic: open/close delimiter for *what* (there's a lot 
of paired delimiters to consider when pretty printing). Perhaps 
`getUnnamedIdentOpenDelimiter()` or something to make it more clear that this 
isn't, say, an attribute delimiter?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84658

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

[PATCH] D66035: [WebAssembly] WIP: Add support for reference types

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

In D66035#2178105 , @pmatos wrote:

> Please ignore my `.gitlab-ci.yml`. That's just an internal change that I got 
> uploaded by mistake.
> I am looking to see this through and start discussion on this with the goal 
> of landing it.

It's great to see this patch being picked up again!

> At the moment, for example this test crashes:
>
>   struct {
> __attribute__((address_space(256))) char *a[0];
>   } b;
>   void c() {
> for (int d = 0; d < 10; d++)
>   b.a[d] = 0;
>   }
>
> This picked up from previous work by @vchuravy. I am still surprised by, and 
> don't understand the reasoning behind, using a an i8 * for the externref 
> representation. If anything I would expect to see i32 *. 
> In any case, the test above crashes in loop vectorization in 
> `TargetLowering.h` `getValueType` because externref is casted just fine to a 
> pointer type and it shouldn't be since `externref` is supposed to be opaque.

I'm not aware of any particular reason to use i8* and I would expect i32* to 
work as well. Certainly once LLVM has opaque pointers, we will want to take 
advantage of those. What is this test case supposed to do? As far as I can 
tell, it's not very meaningful to have reference types in a struct since they 
can't be stored in memory. Is the zero assignment supposed to become a nullref 
assignment?

> I would be keen to hear some comments and suggestions going forward on this.

I am curious about how you plan to use this functionality from frontends going 
forward and what exact use cases you are aiming to support.




Comment at: clang/lib/Basic/Targets/WebAssembly.cpp:43
   .Case("exception-handling", HasExceptionHandling)
+  .Case("reference-types", HasReferenceTypes)
   .Case("bulk-memory", HasBulkMemory)

I would split the target-features changes out into a separate patch that we can 
land right away. There is utility in having the feature flag available to be 
put into the target-features section even if LLVM cannot itself emit reference 
types usage yet.



Comment at: clang/lib/Basic/Targets/WebAssembly.cpp:183
+  HasReferenceTypes = true;
+  resetDataLayout("e-m:e-p:32:32-i64:64-n32:64-S128-ni:256");
+  continue;

It would be good to have a comment about what is different in this data layout 
string. Also, is it really necessary to have a different data layout string 
when reference types are enabled?



Comment at: llvm/include/llvm/CodeGen/ValueTypes.td:169
 def exnref: ValueType<0, 134>;  // WebAssembly's exnref type
+def externref: ValueType<0, 135>;   // WebAssembly's externref type
 def token  : ValueType<0  , 248>;   // TokenTy

Do we need `funcref` here as well, or do we just use `externref` everywhere and 
infer separate `funcref` types later?



Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:4129
+if (Offsets[i] != 0)
+  A = DAG.getNode(ISD::ADD, dl,
+  PtrVT, Ptr,

What is this change for?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66035

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

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

I think Sam's approach is reasonable.

The ability to supply CUID to sub-compilations is useful by itself and should 
probably be split into a separate patch as it's largely independent of the 
externalization of file-scope static vars.


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

https://reviews.llvm.org/D80858

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

[PATCH] D84202: [clang][noderef] Enable -Wnoderef warning for CXX C-style casts

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

> users can still disable the warning on line granularity with pragmas.

This makes me a bit uncomfortable because those pragmas extremely ugly (and not 
easily portable). Also, this will break code for users who were previously 
doing something that was explicitly allowed (but not explicitly documented, at 
least). How often do users need to (legitimately, rather than accidentally) use 
the C-style cast like this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84202

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 281311.
guiand added a comment.

Incorporate C++'s more conservative checks for omitted return values


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/attr-noundef.cpp
  clang/test/CodeGen/indirect-noundef.cpp

Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -o - %s | FileCheck %s
+
+union u1 {
+  int val;
+};
+
+// CHECK: @indirect_callee_int_ptr = global i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = global i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK-LABEL: define noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK-LABEL: define i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) {
+  return a;
+}
+
+int main() {
+  // CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+  indirect_callee_int(0);
+  // CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+  indirect_callee_union((union u1){0});
+
+  indirect_callee_int_ptr = indirect_callee_int;
+  indirect_callee_union_ptr = indirect_callee_union;
+
+  // CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+  indirect_callee_int_ptr(0);
+  // CHECK: call i32 %{{.*}}(i32 %
+  indirect_callee_union_ptr((union u1){});
+
+  return 0;
+}
Index: clang/test/CodeGen/attr-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noundef.cpp
@@ -0,0 +1,152 @@
+// RUN: %clang_cc1 -x c++ -S -emit-llvm %s -o - | FileCheck %s
+
+// Passing structs by value
+// TODO: No structs may currently be marked noundef
+
+namespace check_structs {
+struct Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK: define i32 @{{.*}}ret_trivial
+// CHECK: define void @{{.*}}pass_trivial{{.*}}(i32 %
+
+struct NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef %
+
+struct Huge {
+  int a[1024];
+};
+Huge ret_huge() { return {}; }
+void pass_huge(Huge h) {}
+// CHECK: define void @{{.*}}ret_huge{{.*}}(%"struct.check_structs::Huge"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_huge{{.*}}(%"struct.check_structs::Huge"* noundef byval
+} // namespace check_structs
+
+// Passing unions by value
+// No unions may be marked noundef
+
+namespace check_unions {
+union Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK: define i32 @{{.*}}ret_trivial
+// CHECK: define void @{{.*}}pass_trivial{{.*}}(i32 %
+
+union NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define void @{{.*}}ret_nocopy{{.*}}(%"union.check_unions::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"union.check_unions::NoCopy"* noundef %
+} // namespace check_unions
+
+// Passing `this` pointers
+// `this` pointer must always be defined
+
+namespace check_this {
+struct Object {
+  int data[];
+
+  Object() {
+this->data[0] = 0;
+  }
+  int getData() {
+return this->data[0];
+  }
+  Object *getThis() {
+return this;
+  }
+};
+
+void use_object() {
+  Object obj;
+  obj.getData();
+  obj.getThis();
+}
+// CHECK: define linkonce_odr void @{{.*}}Object{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef i32 @{{.*}}Object{{.*}}getData{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef %"struct.check_this::Object"* @{{.*}}Object{{.*}}getThis{{.*}}(%"struct.check_this::Object"* noundef %
+} // namespace check_this
+
+// Passing vector types
+
+namespace check_vecs {
+typedef int __attribute__((vector_size(12))) i32x3;
+i32x3 ret_vec() {
+  return {};
+}
+void pass_vec(i32x3 v) {
+}
+
+// CHECK: define noundef <3 x i32> @{{.*}}ret_vec{{.*}}()
+// CHECK: define void @{{.*}}pass_vec{{.*}}(<3 x i32> noundef %
+} // namespace check_vecs
+
+// Passing exotic types
+// Function/Array pointers, Function member / Data member pointers, nullptr_t, ExtInt types
+
+namespace check_e

[PATCH] D84774: [NewPM][PassInstrument] Add PrintPass callback to StandardInstrumentations

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks accepted this revision.
aeubanks added a comment.

Thanks for doing this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84774

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

[PATCH] D81083: [Clang] Allow "vector_size" applied to Booleans

[Simon Moll via Phabricator via cfe-commits]

simoll updated this revision to Diff 281322.
simoll added a comment.

- Rebased
- Implement `EmitFromMemory`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81083

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -331,8 +331,7 @@
 typedef __attribute__((ext_vector_type(4))) int vi4;
 const int &reference_to_vec_element = vi4(1).x;
 
-// PR12649
-typedef bool bad __attribute__((__vector_size__(16)));  // expected-error {{invalid vector element type 'bool'}}
+typedef bool good __attribute__((__vector_size__(16)));
 
 namespace Templates {
 template 
@@ -350,9 +349,7 @@
 void Init() {
   const TemplateVectorType::type Works = {};
   const TemplateVectorType::type Works2 = {};
-  // expected-error@#1 {{invalid vector element type 'bool'}}
-  // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
-  const TemplateVectorType::type NoBool = {};
+  const TemplateVectorType::type BoolWorks = {};
   // expected-error@#1 {{invalid vector element type 'int __attribute__((ext_vector_type(4)))' (vector of 4 'int' values)}}
   // expected-note@+1 {{in instantiation of template class 'Templates::TemplateVectorType' requested here}}
   const TemplateVectorType::type NoComplex = {};
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2464,10 +2464,10 @@
 
 QualType Sema::BuildVectorType(QualType CurType, Expr *SizeExpr,
SourceLocation AttrLoc) {
-  // The base type must be integer (not Boolean or enumeration) or float, and
+  // The base type must be boolean or integer (not enumeration) or float, and
   // can't already be a vector.
   if (!CurType->isDependentType() &&
-  (!CurType->isBuiltinType() || CurType->isBooleanType() ||
+  (!CurType->isBuiltinType() ||
(!CurType->isIntegerType() && !CurType->isRealFloatingType( {
 Diag(AttrLoc, diag::err_attribute_invalid_vector_type) << CurType;
 return QualType();
@@ -2496,8 +2496,15 @@
 << SizeExpr->getSourceRange() << "vector";
 return QualType();
   }
-  uint64_t VectorSizeBits = VecSize->getZExtValue() * 8;
-  unsigned TypeSize = static_cast(Context.getTypeSize(CurType));
+
+  uint64_t VecNumElems = VecSize->getZExtValue();
+  uint64_t VectorSizeBits =
+  CurType->isBooleanType()
+  ? VecNumElems
+  : VecNumElems * 8; // FIXME "bitsof(CharUnit)"
+  unsigned TypeSize = CurType->isBooleanType()
+  ? 1
+  : static_cast(Context.getTypeSize(CurType));
 
   if (VectorSizeBits == 0) {
 Diag(AttrLoc, diag::err_attribute_zero_size)
@@ -7557,13 +7564,13 @@
   T = Context.getAdjustedType(T, Wrapped);
 }
 
-/// HandleVectorSizeAttribute - this attribute is only applicable to integral
-/// and float scalars, although arrays, pointers, and function return values are
-/// allowed in conjunction with this construct. Aggregates with this attribute
-/// are invalid, even if they are of the same size as a corresponding scalar.
-/// The raw attribute should contain precisely 1 argument, the vector size for
-/// the variable, measured in bytes. If curType and rawAttr are well formed,
-/// this routine will return a new vector type.
+/// HandleVectorSizeAttribute - this attribute is only applicable to boolean,
+/// integral and float scalars, although arrays, pointers, and function return
+/// values are allowed in conjunction with this construct. Aggregates with this
+/// attribute are invalid, even if they are of the same size as a corresponding
+/// scalar. The raw attribute should contain precisely 1 argument, the vector
+/// size for the variable, measured in bytes. If curType and rawAttr are well
+/// formed, this routine will return a new vector type.
 static void HandleVectorSizeAttr(QualType &CurType, const ParsedAttr &Attr,
  Sema &S) {
   // Check the attribute arguments.
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5946,7 +5946,8 @@
   } else if (LHSVT || RHSVT) {
 ResultType = CheckVectorOperands(
 LHS, RHS, Quest

[PATCH] D84405: CodeGen: Improve generated IR for __builtin_mul_overflow(uint, uint, int)

[Vedant Kumar via Phabricator via cfe-commits]

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

Thanks, looks good to me with a small test addition.




Comment at: clang/test/CodeGen/builtins-overflow.c:123
+  // CHECK: br i1 [[C2]]
+  int r;
+  if (__builtin_mul_overflow(x, y, &r))

vsk wrote:
> Could you add a test for the volatile result case?
I think this is still missing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84405

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

[PATCH] D81678: Introduce noundef attribute at call sites for stricter poison analysis

[Gui Andrade via Phabricator via cfe-commits]

guiand updated this revision to Diff 281323.
guiand edited the summary of this revision.
guiand added a comment.

Fixes regression; allows emitting noundef for non-FunctionDecls as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81678

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/attr-noundef.cpp
  clang/test/CodeGen/indirect-noundef.cpp

Index: clang/test/CodeGen/indirect-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/indirect-noundef.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -x c++ -triple x86_64-unknown-unknown -O0 -emit-llvm -o - %s | FileCheck %s
+
+union u1 {
+  int val;
+};
+
+// CHECK: @indirect_callee_int_ptr = global i32 (i32)*
+int (*indirect_callee_int_ptr)(int);
+// CHECK: @indirect_callee_union_ptr = global i32 (i32)*
+union u1 (*indirect_callee_union_ptr)(union u1);
+
+// CHECK-LABEL: define noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef %
+int indirect_callee_int(int a) { return a; }
+// CHECK-LABEL: define i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+union u1 indirect_callee_union(union u1 a) {
+  return a;
+}
+
+int main() {
+  // CHECK: call noundef i32 @{{.*}}indirect_callee_int{{.*}}(i32 noundef 0)
+  indirect_callee_int(0);
+  // CHECK: call i32 @{{.*}}indirect_callee_union{{.*}}(i32 %
+  indirect_callee_union((union u1){0});
+
+  indirect_callee_int_ptr = indirect_callee_int;
+  indirect_callee_union_ptr = indirect_callee_union;
+
+  // CHECK: call noundef i32 %{{.*}}(i32 noundef 0)
+  indirect_callee_int_ptr(0);
+  // CHECK: call i32 %{{.*}}(i32 %
+  indirect_callee_union_ptr((union u1){});
+
+  return 0;
+}
Index: clang/test/CodeGen/attr-noundef.cpp
===
--- /dev/null
+++ clang/test/CodeGen/attr-noundef.cpp
@@ -0,0 +1,152 @@
+// RUN: %clang_cc1 -x c++ -S -emit-llvm %s -o - | FileCheck %s
+
+// Passing structs by value
+// TODO: No structs may currently be marked noundef
+
+namespace check_structs {
+struct Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK: define i32 @{{.*}}ret_trivial
+// CHECK: define void @{{.*}}pass_trivial{{.*}}(i32 %
+
+struct NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define void @{{.*}}ret_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"struct.check_structs::NoCopy"* noundef %
+
+struct Huge {
+  int a[1024];
+};
+Huge ret_huge() { return {}; }
+void pass_huge(Huge h) {}
+// CHECK: define void @{{.*}}ret_huge{{.*}}(%"struct.check_structs::Huge"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_huge{{.*}}(%"struct.check_structs::Huge"* noundef byval
+} // namespace check_structs
+
+// Passing unions by value
+// No unions may be marked noundef
+
+namespace check_unions {
+union Trivial {
+  int a;
+};
+Trivial ret_trivial() { return {}; }
+void pass_trivial(Trivial e) {}
+// CHECK: define i32 @{{.*}}ret_trivial
+// CHECK: define void @{{.*}}pass_trivial{{.*}}(i32 %
+
+union NoCopy {
+  int a;
+  NoCopy(NoCopy &) = delete;
+};
+NoCopy ret_nocopy() { return {}; }
+void pass_nocopy(NoCopy e) {}
+// CHECK: define void @{{.*}}ret_nocopy{{.*}}(%"union.check_unions::NoCopy"* noalias sret align 4 %
+// CHECK: define void @{{.*}}pass_nocopy{{.*}}(%"union.check_unions::NoCopy"* noundef %
+} // namespace check_unions
+
+// Passing `this` pointers
+// `this` pointer must always be defined
+
+namespace check_this {
+struct Object {
+  int data[];
+
+  Object() {
+this->data[0] = 0;
+  }
+  int getData() {
+return this->data[0];
+  }
+  Object *getThis() {
+return this;
+  }
+};
+
+void use_object() {
+  Object obj;
+  obj.getData();
+  obj.getThis();
+}
+// CHECK: define linkonce_odr void @{{.*}}Object{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef i32 @{{.*}}Object{{.*}}getData{{.*}}(%"struct.check_this::Object"* noundef %
+// CHECK: define linkonce_odr noundef %"struct.check_this::Object"* @{{.*}}Object{{.*}}getThis{{.*}}(%"struct.check_this::Object"* noundef %
+} // namespace check_this
+
+// Passing vector types
+
+namespace check_vecs {
+typedef int __attribute__((vector_size(12))) i32x3;
+i32x3 ret_vec() {
+  return {};
+}
+void pass_vec(i32x3 v) {
+}
+
+// CHECK: define noundef <3 x i32> @{{.*}}ret_vec{{.*}}()
+// CHECK: define void @{{.*}}pass_vec{{.*}}(<3 x i32> noundef %
+} // namespace check_vecs
+
+// Passing exotic types
+// Function/Array pointers, Function member / Data member pointe