[PATCH] D125137: [clang-format] Don't remove braces if a 1-statement body would wrap

2022-05-06 Thread Owen Pan via Phabricator via cfe-commits
owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:25374
   verifyFormat("if (a) {\n"
"  b = c >= 0 ? d\n"
" : e;\n"

Jeroen wrote:
> Will it also add braces if they where not there yet?
No, but you can insert braces by setting `InsertBraces` to true and 
`RemoveBracesLLVM` to false.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125137

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


[PATCH] D125137: [clang-format] Don't remove braces if a 1-statement body would wrap

2022-05-06 Thread Jeroen Van Antwerpen via Phabricator via cfe-commits
Jeroen added a comment.

Will it also add braces if they where not there yet?




Comment at: clang/unittests/Format/FormatTest.cpp:25374
   verifyFormat("if (a) {\n"
"  b = c >= 0 ? d\n"
" : e;\n"

Will it also add braces if they where not there yet?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125137

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


[PATCH] D123612: [Driver] Support linking to compiler-rt on AVR

2022-05-06 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/AVR.cpp:430
+  // libgcc distributed with avr-gcc always named 'libgcc.a' even on windows.
+  return (Twine("libclang_rt.") + Component + Arch + ".a").str();
+}

benshi001 wrote:
> This method decides the file name of compiler-rt, it is expected to be 
> 
> `libclang_rt.builtins-avrfamily.a`, such as
> 
> `libclang_rt.builtins-avr51.a`
> `libclang_rt.builtins-avrtiny.a`
> `libclang_rt.builtins-avrxmega3.a`
> 
> 
In `-DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=on` mode this should be 
`lib/clang/$version/lib/$triple/libclang_rt.xxx.a` where there is no 
`-microarch` infix.


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

https://reviews.llvm.org/D123612

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


[PATCH] D124977: [NFC][Clang] Modify expect of fail test or XFAIL because CSKY align is different

2022-05-06 Thread Zixuan Wu via Phabricator via cfe-commits
zixuan-wu added inline comments.



Comment at: clang/test/Sema/builtin-alloca-with-align.c:32
 void test8(void) {
+#if defined(__csky__)
   __builtin_alloca_with_align(sizeof(__INT64_TYPE__), 
__alignof__(__INT64_TYPE__)); // expected-warning {{second argument to 
__builtin_alloca_with_align is supposed to be in bits}}

rengolin wrote:
> This test is platform agnostic, perhaps the extra error could be in a new 
> test, exclusively for csky?
Then I prefer to split the file and add UNSUPPORTED for CSKY in the other file 
which only contains test8


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124977

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


[PATCH] D123612: [Driver] Support linking to compiler-rt on AVR

2022-05-06 Thread Ben Shi via Phabricator via cfe-commits
benshi001 added inline comments.



Comment at: clang/lib/Driver/ToolChains/AVR.cpp:430
+  // libgcc distributed with avr-gcc always named 'libgcc.a' even on windows.
+  return (Twine("libclang_rt.") + Component + Arch + ".a").str();
+}

This method decides the file name of compiler-rt, it is expected to be 

`libclang_rt.builtins-avrfamily.a`, such as

`libclang_rt.builtins-avr51.a`
`libclang_rt.builtins-avrtiny.a`
`libclang_rt.builtins-avrxmega3.a`





Comment at: clang/lib/Driver/ToolChains/AVR.cpp:433
+
+std::string AVRToolChain::getCompilerRTPath() const {
+  // Return default path appended with "/avr", if it exists.

This method decides the library path of compiler-rt, it is
`$(RESOURCE_DIR)/lib/avr` or `$(RESOURCE_DIR)/lib/`. The resource dir is either 
explicitly specified via user option `-resource-dir` or has default value 
`$(LLVM_INSTALL_DIR)/lib/clang/$(LLVM_VERSION`. such as

`/opt/avr-tool-chain/lib/clang/14.0.1/lib/avr`
`/opt/avr-tool-chain/lib/clang/14.0.1/lib/`




Comment at: clang/lib/Driver/ToolChains/AVR.cpp:444
+ToolChain::RuntimeLibType AVRToolChain::GetDefaultRuntimeLibType() const {
+  return GCCInstallation.isValid() ? ToolChain::RLT_Libgcc
+   : ToolChain::RLT_CompilerRT;

Currently we still use libgcc if `--rtlib` option is not specified.



Comment at: clang/lib/Driver/ToolChains/AVR.cpp:482
+  (RtLib == ToolChain::RLT_Libgcc || RtLib == ToolChain::RLT_CompilerRT) &&
+  "unknown runtime library");
+

Currently we only allow `--rtlib=libgcc` and `--rtlib=compiler-rt`



Comment at: clang/lib/Driver/ToolChains/AVR.cpp:510
+if (RtLib == ToolChain::RLT_Libgcc)
+  CmdArgs.push_back(Args.MakeArgString("-L" + TC.getGCCInstallPath() +
+   "/" + SubPath));

If `--rtlib` is not specified or specified to `libgcc`, then we generate 
`-L$PATH_TO_LIBGCC`



Comment at: clang/lib/Driver/ToolChains/AVR.cpp:542
+// Link to libgcc.
+if (RtLib == ToolChain::RLT_Libgcc)
+  CmdArgs.push_back("-lgcc");

If `--rtlib` is not specified or specified to `libgcc`, then we generate `-lgcc`



Comment at: clang/lib/Driver/ToolChains/AVR.cpp:555
+// Link to compiler-rt.
+if (RtLib == ToolChain::RLT_CompilerRT) {
+  std::string RtLib =

If `--rtlib=compiler-rt` is explicitly specified, we directly put the 
`libclang.builtins-avrxxx.a` as input file, other than 
`-lclang.builtins-avrxxx`, this is a tradition from other platforms, such as 
x86 and arm.


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

https://reviews.llvm.org/D123612

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


[PATCH] D124287: [modules][ODRHash] Compare ODR hashes to detect mismatches in duplicate ObjCInterfaceDecl.

2022-05-06 Thread Volodymyr Sapsai via Phabricator via cfe-commits
vsapsai planned changes to this revision.
vsapsai added inline comments.



Comment at: clang/lib/AST/ODRHash.cpp:528
+
+  //FIXME: Hash other interface-specific elements like protocols, etc.
+

vsapsai wrote:
> jansvoboda11 wrote:
> > Is this important to implement now, or are you fine leaving this be for the 
> > time being?
> When I was posting this patch, I thought it wasn't critical to cover 
> everything immediately. Now I'm thinking about the ways to minimize the 
> uncovered elements. Specifically, I'm thinking about adding support for 
> protocols first and then for interfaces. Protocols aren't as useful as 
> interfaces but they are simpler and can be easier to add.
> 
> And `AddSubDecl` should cover some of the nested decls already, need to add 
> more cases to the test.
I've made good progress at handling anonymous RecordDecl. So think I should add 
handling ivars instead of the lame FIXME.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124287

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


[PATCH] D118355: Add -mmanual-endbr switch to allow manual selection of control-flow protection

2022-05-06 Thread Xiang Zhang via Phabricator via cfe-commits
xiangzhangllvm added inline comments.



Comment at: llvm/lib/Target/X86/X86IndirectBranchTracking.cpp:156-161
   if (needsPrologueENDBR(MF, M)) {
-auto MBB = MF.begin();
-Changed |= addENDBR(*MBB, MBB->begin());
+if (!ManualENDBR || MF.getFunction().doesCfCheck()) {
+  auto MBB = MF.begin();
+  Changed |= addENDBR(*MBB, MBB->begin());
+} else {
+  // When -mmanual-endbr is in effect, the compiler does not

joaomoreira wrote:
> xiangzhangllvm wrote:
> > I am a little puzzle here. Let me copy your patch description here:
> > 
> > ```
> > >When -mmanual-endbr is set, llvm refrains from automatically adding
> > >ENDBR instructions to functions' prologues, which would have been
> > >automatically added by -fcf-protection=branch. Although this works
> > >correctly, missing ENDBR instructions where they are actually needed
> > >could lead to broken binaries, which would fail only in running time.
> > ```
> > I think the purpose of "-mmanual-endbr" should be "Supplementary Way" for 
> > the cases which the CET can't correctly insert endbr in automatic way.
> > Here (in if (needsPrologueENDBR(MF, M)) ) the automatic way will insert the 
> > endbr. So I think the job for "-mmanual-endbr" should be done in parallel 
> > with the old condition. Some like:
> > ```
> > if (ManualENDBR ){
> >   do something
> > }else { // automatic
> >   if (needsPrologueENDBR(MF, M)) {insert endbr}
> >  }
> > }
> > ```
> I don't think the idea of -mmanual-endbr is to have a supplementary way for 
> corner cases where CET misses automatic placement. In my understanding the 
> goal is to set the compiler to not emit ENDBRs unless the attribute cf_check 
> is used, thus providing a way to manually reduce the number of valid targets.
> 
> For reference, here is a link for -mmanual-endbr on gcc, 
> https://gcc.gnu.org/legacy-ml/gcc-patches/2018-12/msg00713.html and here are 
> patches on xen that use the feature (and that also mention this review): 
> https://www.mail-archive.com/xen-devel@lists.xenproject.org/msg114160.html
Thanks! Ok, for  "limit number of ENDBR instructions to reduce program size" 
here the code logic is make sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118355

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


[PATCH] D125149: [Frontend] Flip default of CreateInvocationOptions::ProbePrecompiled to false

2022-05-06 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
sammccall requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is generally a better default for tools other than the compiler, which
shouldn't assume a PCH file on disk is something they can consume.

Preserve the old behavior in places associated with libclang/c-index-test
(including ASTUnit) as there are tests relying on it and most important
consumers are out-of-tree. It's unclear whether the tests are specifically
trying to test this functionality, and what the downstream implications of
removing it are. Hopefully someone more familiar can clean this up in future.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125149

Files:
  clang/include/clang/Frontend/Utils.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/tools/c-index-test/core_main.cpp
  clang/tools/libclang/Indexing.cpp
  clang/unittests/Frontend/UtilsTest.cpp


Index: clang/unittests/Frontend/UtilsTest.cpp
===
--- clang/unittests/Frontend/UtilsTest.cpp
+++ clang/unittests/Frontend/UtilsTest.cpp
@@ -48,20 +48,20 @@
   llvm::IntrusiveRefCntPtr CommandLineDiagsEngine =
   clang::CompilerInstance::createDiagnostics(new DiagnosticOptions, ,
  false);
-  // Default: ProbePrecompiled is true.
+  // Default: ProbePrecompiled=false
   CreateInvocationOptions CIOpts;
   CIOpts.Diags = CommandLineDiagsEngine;
   CIOpts.VFS = FS;
   std::unique_ptr CI = createInvocation(Args, CIOpts);
   ASSERT_TRUE(CI);
-  EXPECT_THAT(CI->getPreprocessorOpts().Includes, ElementsAre());
-  EXPECT_EQ(CI->getPreprocessorOpts().ImplicitPCHInclude, "foo.h.pch");
+  EXPECT_THAT(CI->getPreprocessorOpts().Includes, ElementsAre("foo.h"));
+  EXPECT_EQ(CI->getPreprocessorOpts().ImplicitPCHInclude, "");
 
-  CIOpts.ProbePrecompiled = false;
+  CIOpts.ProbePrecompiled = true;
   CI = createInvocation(Args, CIOpts);
   ASSERT_TRUE(CI);
-  EXPECT_THAT(CI->getPreprocessorOpts().Includes, ElementsAre("foo.h"));
-  EXPECT_EQ(CI->getPreprocessorOpts().ImplicitPCHInclude, "");
+  EXPECT_THAT(CI->getPreprocessorOpts().Includes, ElementsAre());
+  EXPECT_EQ(CI->getPreprocessorOpts().ImplicitPCHInclude, "foo.h.pch");
 }
 
 } // namespace
Index: clang/tools/libclang/Indexing.cpp
===
--- clang/tools/libclang/Indexing.cpp
+++ clang/tools/libclang/Indexing.cpp
@@ -510,6 +510,7 @@
 
   CreateInvocationOptions CIOpts;
   CIOpts.Diags = Diags;
+  CIOpts.ProbePrecompiled = true; // FIXME: historical default. Needed?
   std::shared_ptr CInvok =
   createInvocation(*Args, std::move(CIOpts));
 
Index: clang/tools/c-index-test/core_main.cpp
===
--- clang/tools/c-index-test/core_main.cpp
+++ clang/tools/c-index-test/core_main.cpp
@@ -223,6 +223,7 @@
 Diags(CompilerInstance::createDiagnostics(new DiagnosticOptions));
   CreateInvocationOptions CIOpts;
   CIOpts.Diags = Diags;
+  CIOpts.ProbePrecompiled = true; // FIXME: historical default. Needed?
   auto CInvok = createInvocation(ArgsWithProgName, std::move(CIOpts));
   if (!CInvok)
 return true;
Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -1732,6 +1732,7 @@
 CreateInvocationOptions CIOpts;
 CIOpts.VFS = VFS;
 CIOpts.Diags = Diags;
+CIOpts.ProbePrecompiled = true; // FIXME: historical default. Needed?
 CI = createInvocation(llvm::makeArrayRef(ArgBegin, ArgEnd),
   std::move(CIOpts));
 if (!CI)
Index: clang/include/clang/Frontend/Utils.h
===
--- clang/include/clang/Frontend/Utils.h
+++ clang/include/clang/Frontend/Utils.h
@@ -206,7 +206,7 @@
   /// This is used to replace -include with -include-pch in the cc1 args.
   /// FIXME: ProbePrecompiled=true is a poor, historical default.
   /// It misbehaves if the PCH file is from GCC, has the wrong version, etc.
-  bool ProbePrecompiled = true;
+  bool ProbePrecompiled = false;
   /// If set, the target is populated with the cc1 args produced by the driver.
   /// This may be populated even if createInvocation returns nullptr.
   std::vector *CC1Args = nullptr;


Index: clang/unittests/Frontend/UtilsTest.cpp
===
--- clang/unittests/Frontend/UtilsTest.cpp
+++ clang/unittests/Frontend/UtilsTest.cpp
@@ -48,20 +48,20 @@
   llvm::IntrusiveRefCntPtr CommandLineDiagsEngine =
   clang::CompilerInstance::createDiagnostics(new DiagnosticOptions, ,
  false);
-  // Default: ProbePrecompiled is true.
+  // 

[PATCH] D125082: [CMake] Include llvm-debuginfod-find in Fuchsia toolchain

2022-05-06 Thread Petr Hosek via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG57636c25904e: [CMake] Include llvm-debuginfod-find in 
Fuchsia toolchain (authored by phosek).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125082

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


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -257,6 +257,7 @@
   llvm-bolt
   llvm-cov
   llvm-cxxfilt
+  llvm-debuginfod-find
   llvm-dlltool
   llvm-dwarfdump
   llvm-dwp


Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -257,6 +257,7 @@
   llvm-bolt
   llvm-cov
   llvm-cxxfilt
+  llvm-debuginfod-find
   llvm-dlltool
   llvm-dwarfdump
   llvm-dwp
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 57636c2 - [CMake] Include llvm-debuginfod-find in Fuchsia toolchain

2022-05-06 Thread Petr Hosek via cfe-commits

Author: Petr Hosek
Date: 2022-05-06T16:55:05-07:00
New Revision: 57636c25904e05c6b94370c82e52a89f5baf80f1

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

LOG: [CMake] Include llvm-debuginfod-find in Fuchsia toolchain

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

Added: 


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

Removed: 




diff  --git a/clang/cmake/caches/Fuchsia-stage2.cmake 
b/clang/cmake/caches/Fuchsia-stage2.cmake
index 5c521e75f225a..65fff9ef85610 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -257,6 +257,7 @@ set(LLVM_TOOLCHAIN_TOOLS
   llvm-bolt
   llvm-cov
   llvm-cxxfilt
+  llvm-debuginfod-find
   llvm-dlltool
   llvm-dwarfdump
   llvm-dwp



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


[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

2022-05-06 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added inline comments.



Comment at: clang/lib/CodeGen/CGCUDARuntime.h:58
+  /// Mark the entry as a global variable.
+  OffloadGlobalEntry = 0x0,
+  /// Mark the entry as a managed global variable.

tra wrote:
> jhuber6 wrote:
> > tra wrote:
> > > We're still using the same numeric value for two different kinds of 
> > > entities.
> > > Considering that it's the third round we're making around this point, I'm 
> > > starting to suspect that I may be missing something.
> > > Is there a particular reason kernels and global unmanaged variables have 
> > > to have the same 'kind'?
> > > 
> > > It's possible that I didn't do a good job explaining my enthusiastic 
> > > nitpicking here.
> > > My suggestion to have unified enum for **all** entities we register is 
> > > based on a principle of separation of responsibilities. If we want to 
> > > know what kind of entry we're dealing with, checking the 'kind' field 
> > > should be sufficient. The 'size' field should only indicate the size of 
> > > the entity. Having to consider both kind and size to determine what 
> > > you're dealing with just muddies things and should not be done unless 
> > > there's a good reason for that. E.g. it might be OK if we were short on 
> > > flag bits.
> > > 
> > > 
> > Ah, I see the point you're making now. This is yet another thing that 
> > OpenMP did that I just copied. I wouldn't have implemented it this way but 
> > I figured it would be simpler to keep them similar. I mostly did it this 
> > way because I did some initial tests of registering and accessing CUDA 
> > globals in OpenMP and it required using the same flags for the kernels and 
> > globals. We could change it for CUDA in the future and I could make that 
> > change here if it's valuable. Ideally I would like to rewrite how we do all 
> > this registration with the structs but breaking the ABI makes it 
> > complicated...
> >  I did some initial tests of registering and accessing CUDA globals in 
> > OpenMP and it required using the same flags for the kernels and globals.
> 
> OK. So, there is something that requires this magic. If that's something we 
> must have, then it must be mentioned in the comments around the enum.
> 
> Do you know where I should find the code which needs this? I'm curious what's 
> going on there.
> I wonder if it just checks for "flags==0" and refuses to deal with unknown 
> flags. 
> To think of it, we probably want to put the enum into a common header which 
> defines the `__tgt_offload_entry`.We would not want OpenMP itself to start 
> using the same bits for something else.
Sorry, I should be more specific. The OpenMP offloading runtime currently uses 
a size of zero to indicate a kernel function and the flags have a different 
meaning if it's a kernel. For OpenMP, 0 is a kernel, 1 and 2 are device ctors / 
dtors. I'm not sure why they chose this over just another flag but it's the 
current standard. You can see it used like this here 
https://github.com/llvm/llvm-project/blob/main/openmp/libomptarget/src/omptarget.cpp#L147.
 I'm not sure if there's a good way to wrangle these together now that I think 
about it, considering OpenMP already uses `0x1` to represent `link` OpenMP 
variables so this already collides. But treating the flags different on the 
size is at least consistent with what OpenMP does. It makes it a little hard to 
define one enum for it since we use it two different ways, I'm not a fan of it 
but it's what the current ABI uses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123471

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


[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

2022-05-06 Thread Dávid Bolvanský via Phabricator via cfe-commits
xbolva00 added a comment.

Strong +1.

It would be great to backport it to llvm 14.0.x as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125142

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


[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

2022-05-06 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: clang/lib/CodeGen/CGCUDARuntime.h:58
+  /// Mark the entry as a global variable.
+  OffloadGlobalEntry = 0x0,
+  /// Mark the entry as a managed global variable.

jhuber6 wrote:
> tra wrote:
> > We're still using the same numeric value for two different kinds of 
> > entities.
> > Considering that it's the third round we're making around this point, I'm 
> > starting to suspect that I may be missing something.
> > Is there a particular reason kernels and global unmanaged variables have to 
> > have the same 'kind'?
> > 
> > It's possible that I didn't do a good job explaining my enthusiastic 
> > nitpicking here.
> > My suggestion to have unified enum for **all** entities we register is 
> > based on a principle of separation of responsibilities. If we want to know 
> > what kind of entry we're dealing with, checking the 'kind' field should be 
> > sufficient. The 'size' field should only indicate the size of the entity. 
> > Having to consider both kind and size to determine what you're dealing with 
> > just muddies things and should not be done unless there's a good reason for 
> > that. E.g. it might be OK if we were short on flag bits.
> > 
> > 
> Ah, I see the point you're making now. This is yet another thing that OpenMP 
> did that I just copied. I wouldn't have implemented it this way but I figured 
> it would be simpler to keep them similar. I mostly did it this way because I 
> did some initial tests of registering and accessing CUDA globals in OpenMP 
> and it required using the same flags for the kernels and globals. We could 
> change it for CUDA in the future and I could make that change here if it's 
> valuable. Ideally I would like to rewrite how we do all this registration 
> with the structs but breaking the ABI makes it complicated...
>  I did some initial tests of registering and accessing CUDA globals in OpenMP 
> and it required using the same flags for the kernels and globals.

OK. So, there is something that requires this magic. If that's something we 
must have, then it must be mentioned in the comments around the enum.

Do you know where I should find the code which needs this? I'm curious what's 
going on there.
I wonder if it just checks for "flags==0" and refuses to deal with unknown 
flags. 
To think of it, we probably want to put the enum into a common header which 
defines the `__tgt_offload_entry`.We would not want OpenMP itself to start 
using the same bits for something else.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123471

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


[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

2022-05-06 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added inline comments.



Comment at: clang/lib/CodeGen/CGCUDARuntime.h:58
+  /// Mark the entry as a global variable.
+  OffloadGlobalEntry = 0x0,
+  /// Mark the entry as a managed global variable.

tra wrote:
> We're still using the same numeric value for two different kinds of entities.
> Considering that it's the third round we're making around this point, I'm 
> starting to suspect that I may be missing something.
> Is there a particular reason kernels and global unmanaged variables have to 
> have the same 'kind'?
> 
> It's possible that I didn't do a good job explaining my enthusiastic 
> nitpicking here.
> My suggestion to have unified enum for **all** entities we register is based 
> on a principle of separation of responsibilities. If we want to know what 
> kind of entry we're dealing with, checking the 'kind' field should be 
> sufficient. The 'size' field should only indicate the size of the entity. 
> Having to consider both kind and size to determine what you're dealing with 
> just muddies things and should not be done unless there's a good reason for 
> that. E.g. it might be OK if we were short on flag bits.
> 
> 
Ah, I see the point you're making now. This is yet another thing that OpenMP 
did that I just copied. I wouldn't have implemented it this way but I figured 
it would be simpler to keep them similar. I mostly did it this way because I 
did some initial tests of registering and accessing CUDA globals in OpenMP and 
it required using the same flags for the kernels and globals. We could change 
it for CUDA in the future and I could make that change here if it's valuable. 
Ideally I would like to rewrite how we do all this registration with the 
structs but breaking the ABI makes it complicated...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123471

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


[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

2022-05-06 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: clang/lib/CodeGen/CGCUDARuntime.h:58
+  /// Mark the entry as a global variable.
+  OffloadGlobalEntry = 0x0,
+  /// Mark the entry as a managed global variable.

We're still using the same numeric value for two different kinds of entities.
Considering that it's the third round we're making around this point, I'm 
starting to suspect that I may be missing something.
Is there a particular reason kernels and global unmanaged variables have to 
have the same 'kind'?

It's possible that I didn't do a good job explaining my enthusiastic nitpicking 
here.
My suggestion to have unified enum for **all** entities we register is based on 
a principle of separation of responsibilities. If we want to know what kind of 
entry we're dealing with, checking the 'kind' field should be sufficient. The 
'size' field should only indicate the size of the entity. Having to consider 
both kind and size to determine what you're dealing with just muddies things 
and should not be done unless there's a good reason for that. E.g. it might be 
OK if we were short on flag bits.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123471

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


[PATCH] D122766: [clang] Add the flag -ffile-reproducible

2022-05-06 Thread Alan Zhao via Phabricator via cfe-commits
ayzhao added a comment.

In D122766#3450348 , @dexonsmith 
wrote:

> In D122766#3450298 , @ayzhao wrote:
>
>> So, the general consensus seems to be that we should use backslashes when 
>> targeting Windows.
>>
>> I implemented using only backslashes for Windows; however, 
>> clang/test/SemaCXX/destructor.cpp 
>> 
>>  fails when running on Linux with the following error (among other errors, 
>> but the one below is the most important).
>>
>>   ...
>>   error: 'error' diagnostics seen but not expected:
>> Line 32: 
>> '\\clang\\test\\SemaCXX\\destructor.cpp'
>>  file not found
>>   ...
>>
>> The reason for this is that the test has Clang target windows 
>> 
>>  and the test also has the statement #include __FILE__ 
>> .
>>
>> One way to fix this would be to have every macro that accepts a path 
>> internally convert the path to the build environment's path format, but TBH 
>> I'm not sure. What do you all think?
>
> Wow, `#include __FILE__` is kind of amazing...
>
> It sounds like you're suggesting changing the `#include` directive to start 
> canonicalizing its argument. I'm not a fan of that idea; header search is 
> hard enough to reason about.
>
> I suggest, instead:
>
> - Only canonicalize `__FILE__` for the target platform when there's a 
> command-line flag that suggests it's okay. I suggest adding 
> `-ffile-reproducible`, which could be implied by `-ffile-prefix-map` but also 
> specified separately when `-ffile-prefix-map` isn't being used.
> - When `__FILE__` is being canonicalized, it's okay to fail on `#include 
> __FILE__` if the canonicalized path doesn't work as an argument to `#include` 
> in the current build environment.
>
> This would mean that `-ffile-reproducible` makes `#include __FILE__` 
> non-portable, which would be fine as long as `#include __FILE__` isn't 
> load-bearing for real life usecases. My guess is that it'd be okay?
>
> What do others think?

Patch has been updated to implement `-ffile-reproducible`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122766

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


[PATCH] D122766: [clang] Add the flag -ffile-reproducible

2022-05-06 Thread Alan Zhao via Phabricator via cfe-commits
ayzhao updated this revision to Diff 427774.
ayzhao added a comment.

another missing newline


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122766

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/AST/Expr.cpp
  clang/lib/Basic/LangOptions.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/CodeGenCXX/builtin-source-location.cpp
  clang/test/Preprocessor/file_test.c
  clang/test/Preprocessor/file_test_windows.c

Index: clang/test/Preprocessor/file_test_windows.c
===
--- clang/test/Preprocessor/file_test_windows.c
+++ clang/test/Preprocessor/file_test_windows.c
@@ -1,9 +1,25 @@
 // REQUIRES: system-windows
-// RUN: %clang -E -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL
-// RUN: %clang -E -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ -fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s -check-prefix CHECK-CASE
-// RUN: %clang -E -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -fno-file-reproducible -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ -fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s -check-prefix CHECK-CASE
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+
+// RUN: %clang -E -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s --check-prefix CHECK-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s --check-prefix CHECK-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ -fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s -check-prefix CHECK-CASE-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE-REPRODUCIBLE
+
+// RUN: %clang -E -target x86_64-pc-linux-gnu -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -target x86_64-pc-linux-gnu -ffile-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+
+// Clang defaults to forward slashes for the non-prefix portion of the path even if the build environment is Windows.
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-linux-gnu -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-linux-gnu -ffile-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+
+// RUN: %clang -E -ffile-reproducible -target x86_64-pc-win32 -c -o - %s | FileCheck %s --check-prefix CHECK-WINDOWS-FULL
+// RUN: %clang -E -ffile-reproducible -target x86_64-pc-linux-gnu -c -o - %s | FileCheck %s --check-prefix CHECK-LINUX-FULL
 
 filename: __FILE__
 #include "Inputs/include-file-test/file_test.h"
@@ -27,3 +43,34 @@
 // CHECK-REMOVE: filename: "Inputs/include-file-test/file_test.h"
 // CHECK-REMOVE: basefile: "file_test_windows.c"
 // CHECK-REMOVE-NOT: filename:
+
+// CHECK-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH\\empty\\file_test_windows.c"
+// CHECK-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH\\empty\\Inputs\\include-file-test\\file_test.h"
+// CHECK-REPRODUCIBLE: basefile: "A:\\UNLIKELY_PATH\\empty\\file_test_windows.c"
+// CHECK-REPRODUCIBLE-NOT: filename:
+
+// CHECK-EVIL-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH=empty\\file_test_windows.c"
+// CHECK-EVIL-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH=empty\\Inputs\\include-file-test\\file_test.h"
+// CHECK-EVIL-REPRODUCIBLE: basefile: "A:\\UNLIKELY_PATH=empty\\file_test_windows.c"
+// CHECK-EVIL-REPRODUCIBLE-NOT: filename:
+
+// CHECK-CASE-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH_BASE\\file_test_windows.c"
+// CHECK-CASE-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH_INC\\include-file-test\\file_test.h"
+// CHECK-CASE-REPRODUCIBLE: basefile: "A:\\UNLIKELY_PATH_BASE\\file_test_windows.c"
+// CHECK-CASE-REPRODUCIBLE-NOT: filename:
+
+// CHECK-REMOVE-REPRODUCIBLE: filename: 

[PATCH] D122766: [clang] Add the flag -ffile-reproducible

2022-05-06 Thread Alan Zhao via Phabricator via cfe-commits
ayzhao updated this revision to Diff 427773.
ayzhao added a comment.

missing newline


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122766

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/AST/Expr.cpp
  clang/lib/Basic/LangOptions.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/CodeGenCXX/builtin-source-location.cpp
  clang/test/Preprocessor/file_test.c
  clang/test/Preprocessor/file_test_windows.c

Index: clang/test/Preprocessor/file_test_windows.c
===
--- clang/test/Preprocessor/file_test_windows.c
+++ clang/test/Preprocessor/file_test_windows.c
@@ -1,9 +1,25 @@
 // REQUIRES: system-windows
-// RUN: %clang -E -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL
-// RUN: %clang -E -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ -fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s -check-prefix CHECK-CASE
-// RUN: %clang -E -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -fno-file-reproducible -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ -fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s -check-prefix CHECK-CASE
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+
+// RUN: %clang -E -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s --check-prefix CHECK-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s --check-prefix CHECK-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ -fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s -check-prefix CHECK-CASE-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE-REPRODUCIBLE
+
+// RUN: %clang -E -target x86_64-pc-linux-gnu -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -target x86_64-pc-linux-gnu -ffile-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+
+// Clang defaults to forward slashes for the non-prefix portion of the path even if the build environment is Windows.
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-linux-gnu -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-linux-gnu -ffile-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+
+// RUN: %clang -E -ffile-reproducible -target x86_64-pc-win32 -c -o - %s | FileCheck %s --check-prefix CHECK-WINDOWS-FULL
+// RUN: %clang -E -ffile-reproducible -target x86_64-pc-linux-gnu -c -o - %s | FileCheck %s --check-prefix CHECK-LINUX-FULL
 
 filename: __FILE__
 #include "Inputs/include-file-test/file_test.h"
@@ -27,3 +43,34 @@
 // CHECK-REMOVE: filename: "Inputs/include-file-test/file_test.h"
 // CHECK-REMOVE: basefile: "file_test_windows.c"
 // CHECK-REMOVE-NOT: filename:
+
+// CHECK-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH\\empty\\file_test_windows.c"
+// CHECK-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH\\empty\\Inputs\\include-file-test\\file_test.h"
+// CHECK-REPRODUCIBLE: basefile: "A:\\UNLIKELY_PATH\\empty\\file_test_windows.c"
+// CHECK-REPRODUCIBLE-NOT: filename:
+
+// CHECK-EVIL-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH=empty\\file_test_windows.c"
+// CHECK-EVIL-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH=empty\\Inputs\\include-file-test\\file_test.h"
+// CHECK-EVIL-REPRODUCIBLE: basefile: "A:\\UNLIKELY_PATH=empty\\file_test_windows.c"
+// CHECK-EVIL-REPRODUCIBLE-NOT: filename:
+
+// CHECK-CASE-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH_BASE\\file_test_windows.c"
+// CHECK-CASE-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH_INC\\include-file-test\\file_test.h"
+// CHECK-CASE-REPRODUCIBLE: basefile: "A:\\UNLIKELY_PATH_BASE\\file_test_windows.c"
+// CHECK-CASE-REPRODUCIBLE-NOT: filename:
+
+// CHECK-REMOVE-REPRODUCIBLE: filename: "file_test_windows.c"
+// 

[PATCH] D122766: [clang] Use forward slash as the path separator for Windows in __FILE__, __builtin_FILE(), and std::source_location

2022-05-06 Thread Alan Zhao via Phabricator via cfe-commits
ayzhao updated this revision to Diff 427771.
ayzhao added a comment.

Implement -ffile-reproducible


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122766

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/AST/Expr.cpp
  clang/lib/Basic/LangOptions.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/CodeGenCXX/builtin-source-location.cpp
  clang/test/Preprocessor/file_test.c
  clang/test/Preprocessor/file_test_windows.c

Index: clang/test/Preprocessor/file_test_windows.c
===
--- clang/test/Preprocessor/file_test_windows.c
+++ clang/test/Preprocessor/file_test_windows.c
@@ -1,9 +1,25 @@
 // REQUIRES: system-windows
-// RUN: %clang -E -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL
-// RUN: %clang -E -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ -fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s -check-prefix CHECK-CASE
-// RUN: %clang -E -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -fno-file-reproducible -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ -fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s -check-prefix CHECK-CASE
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+
+// RUN: %clang -E -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s --check-prefix CHECK-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s --check-prefix CHECK-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ -fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s -check-prefix CHECK-CASE-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE-REPRODUCIBLE
+
+// RUN: %clang -E -target x86_64-pc-linux-gnu -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -target x86_64-pc-linux-gnu -ffile-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+
+// Clang defaults to forward slashes for the non-prefix portion of the path even if the build environment is Windows.
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-linux-gnu -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-linux-gnu -ffile-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+
+// RUN: %clang -E -ffile-reproducible -target x86_64-pc-win32 -c -o - %s | FileCheck %s --check-prefix CHECK-WINDOWS-FULL
+// RUN: %clang -E -ffile-reproducible -target x86_64-pc-linux-gnu -c -o - %s | FileCheck %s --check-prefix CHECK-LINUX-FULL
 
 filename: __FILE__
 #include "Inputs/include-file-test/file_test.h"
@@ -27,3 +43,34 @@
 // CHECK-REMOVE: filename: "Inputs/include-file-test/file_test.h"
 // CHECK-REMOVE: basefile: "file_test_windows.c"
 // CHECK-REMOVE-NOT: filename:
+
+// CHECK-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH\\empty\\file_test_windows.c"
+// CHECK-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH\\empty\\Inputs\\include-file-test\\file_test.h"
+// CHECK-REPRODUCIBLE: basefile: "A:\\UNLIKELY_PATH\\empty\\file_test_windows.c"
+// CHECK-REPRODUCIBLE-NOT: filename:
+
+// CHECK-EVIL-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH=empty\\file_test_windows.c"
+// CHECK-EVIL-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH=empty\\Inputs\\include-file-test\\file_test.h"
+// CHECK-EVIL-REPRODUCIBLE: basefile: "A:\\UNLIKELY_PATH=empty\\file_test_windows.c"
+// CHECK-EVIL-REPRODUCIBLE-NOT: filename:
+
+// CHECK-CASE-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH_BASE\\file_test_windows.c"
+// CHECK-CASE-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH_INC\\include-file-test\\file_test.h"
+// CHECK-CASE-REPRODUCIBLE: basefile: "A:\\UNLIKELY_PATH_BASE\\file_test_windows.c"
+// CHECK-CASE-REPRODUCIBLE-NOT: filename:
+
+// CHECK-REMOVE-REPRODUCIBLE: filename: 

[PATCH] D125138: [clang] Add the flag -ffile-reproducible

2022-05-06 Thread Alan Zhao via Phabricator via cfe-commits
ayzhao abandoned this revision.
ayzhao added a comment.

Duplicate of D122766 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125138

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


[PATCH] D125138: [clang] Add the flag -ffile-reproducible

2022-05-06 Thread Alan Zhao via Phabricator via cfe-commits
ayzhao created this revision.
Herald added a project: All.
ayzhao requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

When Clang generates the path prefix (i.e. the path of the directory
where the file is) when generating __FILE__, __builtin_FILE(), and
std::source_location, Clang uses the platform-specific path separator
character of the build environment where Clang _itself_ is built. This
leads to inconsistencies in Chrome builds where Clang running on
non-Windows environments uses the forward slash (/) path separator
while Clang running on Windows builds uses the backslash (\) path
separator. To fix this, we add a flag -ffile-reproducible (and its
inverse, -fno-file-reproducible) to have Clang use the target's
platform-specific file separator character.

Additionally, the existing flags -fmacro-prefix-map and
-ffile-prefix-map now both imply -ffile-reproducible. This can be
overriden by setting -fno-file-reproducible.

[0]: https://crbug.com/1310767


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125138

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/AST/Expr.cpp
  clang/lib/Basic/LangOptions.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/test/CodeGenCXX/builtin-source-location.cpp
  clang/test/Preprocessor/file_test.c
  clang/test/Preprocessor/file_test_windows.c

Index: clang/test/Preprocessor/file_test_windows.c
===
--- clang/test/Preprocessor/file_test_windows.c
+++ clang/test/Preprocessor/file_test_windows.c
@@ -1,9 +1,25 @@
 // REQUIRES: system-windows
-// RUN: %clang -E -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
-// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL
-// RUN: %clang -E -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ -fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s -check-prefix CHECK-CASE
-// RUN: %clang -E -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -fno-file-reproducible -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ -fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s -check-prefix CHECK-CASE
+// RUN: %clang -E -fno-file-reproducible -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+
+// RUN: %clang -E -ffile-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s --check-prefix CHECK-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH\empty -c -o - %s | FileCheck %s --check-prefix CHECK-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p=A:\UNLIKELY_PATH=empty -c -o - %s | FileCheck %s -check-prefix CHECK-EVIL-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p/iNPUTS\=A:\UNLIKELY_PATH_INC\ -fmacro-prefix-map=%p/=A:\UNLIKELY_PATH_BASE\ -c -o - %s | FileCheck %s -check-prefix CHECK-CASE-REPRODUCIBLE
+// RUN: %clang -E -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE-REPRODUCIBLE
+
+// RUN: %clang -E -target x86_64-pc-linux-gnu -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -target x86_64-pc-linux-gnu -ffile-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+
+// Clang defaults to forward slashes for the non-prefix portion of the path even if the build environment is Windows.
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-linux-gnu -fmacro-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+// RUN: %clang -E -fno-file-reproducible -target x86_64-pc-linux-gnu -ffile-prefix-map=%p\= -c -o - %s | FileCheck %s --check-prefix CHECK-REMOVE
+
+// RUN: %clang -E -ffile-reproducible -target x86_64-pc-win32 -c -o - %s | FileCheck %s --check-prefix CHECK-WINDOWS-FULL
+// RUN: %clang -E -ffile-reproducible -target x86_64-pc-linux-gnu -c -o - %s | FileCheck %s --check-prefix CHECK-LINUX-FULL
 
 filename: __FILE__
 #include "Inputs/include-file-test/file_test.h"
@@ -27,3 +43,34 @@
 // CHECK-REMOVE: filename: "Inputs/include-file-test/file_test.h"
 // CHECK-REMOVE: basefile: "file_test_windows.c"
 // CHECK-REMOVE-NOT: filename:
+
+// CHECK-REPRODUCIBLE: filename: "A:\\UNLIKELY_PATH\\empty\\file_test_windows.c"
+// CHECK-REPRODUCIBLE: filename: 

[PATCH] D125137: [clang-format] Don't remove braces if a 1-statement body would wrap

2022-05-06 Thread Owen Pan via Phabricator via cfe-commits
owenpan created this revision.
owenpan added reviewers: curdeius, HazardyKnusperkeks, MyDeveloperDay.
owenpan added a project: clang-format.
Herald added a project: All.
owenpan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Reimplement the `RemoveBracesLLVM` feature which handles a single-statement 
block that would get wrapped.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125137

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -25351,8 +25351,6 @@
"}",
Style);
 
-  // FIXME: See https://github.com/llvm/llvm-project/issues/53543.
-#if 0
   Style.ColumnLimit = 65;
 
   verifyFormat("if (condition) {\n"
@@ -25380,9 +25378,6 @@
"  b = c >= 0 ? d : e;\n"
"}",
Style);
-#endif
-
-  Style.ColumnLimit = 20;
 
   verifyFormat("if (a)\n"
"  b = c > 0 ? d : e;",
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -92,6 +92,7 @@
   void reset();
   void parseFile();
   bool precededByCommentOrPPDirective() const;
+  bool mightFitOnOneLine() const;
   bool parseLevel(bool HasOpeningBrace, bool CanContainBracedList,
   IfStmtKind *IfKind = nullptr,
   TokenType NextLBracesType = TT_Unknown);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -14,6 +14,7 @@
 
 #include "UnwrappedLineParser.h"
 #include "FormatToken.h"
+#include "TokenAnnotator.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
@@ -460,13 +461,56 @@
   return Previous && Previous->is(tok::comment) &&
  (Previous->IsMultiline || Previous->NewlinesBefore > 0);
 }
+
+bool UnwrappedLineParser::mightFitOnOneLine() const {
+  const auto ColumnLimit = Style.ColumnLimit;
+  if (ColumnLimit == 0)
+return true;
+
+  if (Lines.empty())
+return true;
+
+  const auto  = Lines.back();
+  const auto  = PreviousLine.Tokens;
+  assert(!Tokens.empty());
+  const auto *LastToken = Tokens.back().Tok;
+  assert(LastToken);
+  if (!LastToken->isOneOf(tok::semi, tok::comment))
+return true;
+
+  SmallVector SavedTokens;
+  for (const auto  : PreviousLine.Tokens) {
+FormatToken *Tok = new FormatToken;
+Tok->copyFrom(*Token.Tok);
+SavedTokens.push_back(Tok);
+  }
+
+  AnnotatedLine Line(PreviousLine);
+  assert(Line.Last == LastToken);
+
+  TokenAnnotator Annotator(Style, Keywords);
+  Annotator.annotate(Line);
+  Annotator.calculateFormattingInformation(Line);
+
+  const int Length = LastToken->TotalLength;
+
+  int I = 0;
+  for (const auto  : PreviousLine.Tokens) {
+const FormatToken *Tok = SavedTokens[I++];
+Token.Tok->copyFrom(*Tok);
+delete Tok;
+  }
+
+  return Line.Level * Style.IndentWidth + Length <= ColumnLimit;
+}
+
 /// \brief Parses a level, that is ???.
 /// \param HasOpeningBrace If that level is started by an opening brace.
 /// \param CanContainBracedList If the content can contain (at any level) a
 /// braced list.
 /// \param NextLBracesType The type for left brace found in this level.
 /// \returns true if a simple block, or false otherwise. (A simple block has a
-/// single statement.)
+/// single statement that fits on a single line.)
 bool UnwrappedLineParser::parseLevel(bool HasOpeningBrace,
  bool CanContainBracedList,
  IfStmtKind *IfKind,
@@ -533,7 +577,9 @@
 precededByCommentOrPPDirective())
   return false;
 const FormatToken *Next = Tokens->peekNextToken();
-return Next->isNot(tok::comment) || Next->NewlinesBefore > 0;
+if (Next->is(tok::comment) && Next->NewlinesBefore == 0)
+  return false;
+return mightFitOnOneLine();
   }
   nextToken();
   addUnwrappedLine();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

2022-05-06 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 427766.
jhuber6 added a comment.

removing enum


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123471

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CGCUDARuntime.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGenCUDA/offloading-entries.cu

Index: clang/test/CodeGenCUDA/offloading-entries.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/offloading-entries.cu
@@ -0,0 +1,33 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --check-globals
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu \
+// RUN:   --offload-new-driver -emit-llvm -o - -x cuda  %s | FileCheck \
+// RUN:   --check-prefix=HOST %s
+
+#include "Inputs/cuda.h"
+
+//.
+// HOST: @x = internal global i32 undef, align 4
+// HOST: @.omp_offloading.entry_name = internal unnamed_addr constant [8 x i8] c"_Z3foov\00"
+// HOST: @.omp_offloading.entry._Z3foov = weak constant %struct.__tgt_offload_entry { ptr @_Z18__device_stub__foov, ptr @.omp_offloading.entry_name, i64 0, i32 0, i32 0 }, section "cuda_offloading_entries", align 1
+// HOST: @.omp_offloading.entry_name.1 = internal unnamed_addr constant [8 x i8] c"_Z3barv\00"
+// HOST: @.omp_offloading.entry._Z3barv = weak constant %struct.__tgt_offload_entry { ptr @_Z18__device_stub__barv, ptr @.omp_offloading.entry_name.1, i64 0, i32 0, i32 0 }, section "cuda_offloading_entries", align 1
+// HOST: @.omp_offloading.entry_name.2 = internal unnamed_addr constant [2 x i8] c"x\00"
+// HOST: @.omp_offloading.entry.x = weak constant %struct.__tgt_offload_entry { ptr @x, ptr @.omp_offloading.entry_name.2, i64 4, i32 0, i32 0 }, section "cuda_offloading_entries", align 1
+//.
+// HOST-LABEL: @_Z18__device_stub__foov(
+// HOST-NEXT:  entry:
+// HOST-NEXT:[[TMP0:%.*]] = call i32 @cudaLaunch(ptr @_Z18__device_stub__foov)
+// HOST-NEXT:br label [[SETUP_END:%.*]]
+// HOST:   setup.end:
+// HOST-NEXT:ret void
+//
+__global__ void foo() {}
+// HOST-LABEL: @_Z18__device_stub__barv(
+// HOST-NEXT:  entry:
+// HOST-NEXT:[[TMP0:%.*]] = call i32 @cudaLaunch(ptr @_Z18__device_stub__barv)
+// HOST-NEXT:br label [[SETUP_END:%.*]]
+// HOST:   setup.end:
+// HOST-NEXT:ret void
+//
+__global__ void bar() {}
+__device__ int x = 1;
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6084,6 +6084,10 @@
options::OPT_fno_openmp_extensions);
   }
 
+  // Forward the new driver to change offloading code generation.
+  if (Args.hasArg(options::OPT_offload_new_driver))
+CmdArgs.push_back("--offload-new-driver");
+
   SanitizeArgs.addArgs(TC, Args, CmdArgs, InputType);
 
   const XRayArgs  = TC.getXRayArgs();
Index: clang/lib/CodeGen/CGCUDARuntime.h
===
--- clang/lib/CodeGen/CGCUDARuntime.h
+++ clang/lib/CodeGen/CGCUDARuntime.h
@@ -52,6 +52,18 @@
   Texture,  // Builtin texture
 };
 
+/// The kind flag of the global entry.
+enum OffloadVarEntryKindFlag : uint32_t {
+  /// Mark the entry as a global variable.
+  OffloadGlobalEntry = 0x0,
+  /// Mark the entry as a managed global variable.
+  OffloadGlobalManagedEntry = 0x1,
+  /// Mark the entry as a surface variable.
+  OffloadGlobalSurfaceEntry = 0x2,
+  /// Mark the entry as a texture variable.
+  OffloadGlobalTextureEntry = 0x3,
+};
+
   private:
 unsigned Kind : 2;
 unsigned Extern : 1;
Index: clang/lib/CodeGen/CGCUDANV.cpp
===
--- clang/lib/CodeGen/CGCUDANV.cpp
+++ clang/lib/CodeGen/CGCUDANV.cpp
@@ -157,6 +157,8 @@
   llvm::Function *makeModuleDtorFunction();
   /// Transform managed variables for device compilation.
   void transformManagedVars();
+  /// Create offloading entries to register globals in RDC mode.
+  void createOffloadingEntries();
 
 public:
   CGNVCUDARuntime(CodeGenModule );
@@ -210,7 +212,8 @@
 CGNVCUDARuntime::CGNVCUDARuntime(CodeGenModule )
 : CGCUDARuntime(CGM), Context(CGM.getLLVMContext()),
   TheModule(CGM.getModule()),
-  RelocatableDeviceCode(CGM.getLangOpts().GPURelocatableDeviceCode),
+  RelocatableDeviceCode(CGM.getLangOpts().GPURelocatableDeviceCode ||
+CGM.getLangOpts().OffloadingNewDriver),
   DeviceMC(InitDeviceMC(CGM)) {
   CodeGen::CodeGenTypes  = CGM.getTypes();
   ASTContext  = CGM.getContext();
@@ -1107,6 +1110,40 @@
   }
 }
 
+// Creates offloading entries for all the kernels and globals that must be
+// registered. The linker will provide a pointer to this 

[PATCH] D119296: KCFI sanitizer

2022-05-06 Thread Sami Tolvanen via Phabricator via cfe-commits
samitolvanen updated this revision to Diff 427764.
samitolvanen added a comment.

Based on LKML feedback:

- Fixed a bug in `Twine` usage.
- Changed AArch64 to encode register information into the ESR and dropped 
`.kcfi_traps` generation for the arch.
- Changed X86 to generate valid instructions for the type data.
- Dropped the `.kcfi_types` section entirely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119296

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/ControlFlowIntegrity.rst
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/Sanitizers.def
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/test/CodeGen/kcfi.c
  clang/test/Driver/fsanitize.c
  llvm/include/llvm/CodeGen/AsmPrinter.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/lib/Target/X86/X86AsmPrinter.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86MCInstLower.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/test/CodeGen/AArch64/kcfi.ll
  llvm/test/CodeGen/X86/kcfi.ll
  llvm/test/Transforms/InstCombine/kcfi_check.ll

Index: llvm/test/Transforms/InstCombine/kcfi_check.ll
===
--- /dev/null
+++ llvm/test/Transforms/InstCombine/kcfi_check.ll
@@ -0,0 +1,35 @@
+; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+
+define void @f1() #0 prefix i32 10 {
+  ret void
+}
+
+declare void @f2() #0 prefix i32 11
+
+define internal void @f3() {
+  ret void
+}
+
+define void @g(ptr noundef %x) {
+  ; CHECK: call void @llvm.kcfi.check(ptr %x, i32 10)
+  call void @llvm.kcfi.check(ptr %x, i32 10)
+
+  ; CHECK-NOT: call void @llvm.kcfi.check(ptr nonnull @f1, i32 10)
+  ; CHECK: call void @llvm.kcfi.check(ptr nonnull @f1, i32 11)
+  call void @llvm.kcfi.check(ptr nonnull @f1, i32 10)
+  call void @llvm.kcfi.check(ptr nonnull @f1, i32 11)
+
+  ; CHECK: call void @llvm.kcfi.check(ptr nonnull @f2, i32 10)
+  ; CHECK-NOT: call void @llvm.kcfi.check(ptr nonnull @f2, i32 11)
+  call void @llvm.kcfi.check(ptr nonnull @f2, i32 10)
+  call void @llvm.kcfi.check(ptr nonnull @f2, i32 11)
+
+  ; CHECK: call void @llvm.kcfi.check(ptr nonnull @f3, i32 10)
+  call void @llvm.kcfi.check(ptr nonnull @f3, i32 10)
+  ret void
+}
+
+; CHECK: declare void @llvm.kcfi.check(ptr, i32 immarg)
+declare void @llvm.kcfi.check(ptr, i32 immarg)
+
+attributes #0 = { "kcfi" }
Index: llvm/test/CodeGen/X86/kcfi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/kcfi.ll
@@ -0,0 +1,39 @@
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu < %s | FileCheck %s
+; RUN: llc -mtriple=x86_64-unknown-linux-gnu -stop-before=finalize-isel < %s | FileCheck %s --check-prefix=ISEL
+
+; CHECK:   .type __cfi_f1,@function
+; CHECK-LABEL: __cfi_f1:
+; CHECK-NEXT:int3
+; CHECK-NEXT:int3
+; CHECK-NEXT:movl $12345678, %eax
+; CHECK-NEXT:int3
+; CHECK-NEXT:int3
+; CHECK-LABEL: .L__cfi_func_end0:
+; CHECK-NEXT:  .size   __cfi_f1, .L__cfi_func_end0-__cfi_f1
+define void @f1(ptr noundef %x) #0 prefix i32 12345678 {
+
+; CHECK-LABEL: f1:
+; CHECK:   # %bb.0:
+; CHECK: cmpl $12345678, -6(%rdi) # imm = 0xBC614E
+; CHECK-NEXT:je .Ltmp0
+; CHECK-NEXT:  .Ltmp1:
+; CHECK-NEXT:ud2
+; CHECK-NEXT:.section .kcfi_traps,"awo",@progbits,.text
+; CHECK-NEXT:.quad .Ltmp1
+; CHECK-NEXT:.text
+; CHECK-NEXT:  .Ltmp0:
+; CHECK-NEXT:callq *%rdi
+
+; ISEL: name: f1
+; ISEL: body:
+; ISEL: KCFI_CHECK %[[#CALL:]], 12345678, implicit-def dead $eflags
+; ISEL: CALL64r %[[#CALL]], csr_64, implicit $rsp, implicit $ssp, implicit-def $rsp, implicit-def $ssp
+
+  call void @llvm.kcfi.check(ptr %x, i32 12345678)
+  call void %x()
+  ret void
+}
+
+declare void @llvm.kcfi.check(ptr, i32 immarg)
+
+attributes #0 = { "kcfi" }
Index: llvm/test/CodeGen/AArch64/kcfi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/kcfi.ll
@@ -0,0 +1,30 @@
+; RUN: llc -mtriple=aarch64-- < %s | FileCheck %s
+; RUN: llc -mtriple=aarch64-- -stop-before=finalize-isel < %s | FileCheck %s --check-prefix=ISEL
+
+; CHECK:   .word 12345678
+define void @f1(ptr noundef %x) #0 prefix i32 12345678 {
+
+; CHECK-LABEL: f1:
+; CHECK:   // %bb.0:
+; CHECK: ldur w16, [x0, #-4]
+; CHECK-NEXT:movk w17, #24910
+; CHECK-NEXT:movk w17, #188, lsl #16
+; CHECK-NEXT:cmp w16, w17
+; CHECK-NEXT:b.eq .Ltmp0
+; CHECK-NEXT:brk 

[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

2022-05-06 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added inline comments.



Comment at: clang/lib/CodeGen/CGCUDARuntime.h:56-70
+enum OffloadRegionEntryKindFlag : uint32_t {
+  /// Mark the region entry as a kernel.
+  OffloadRegionKernelEntry = 0x0,
+};
+
+/// The kind flag of the global variable entry.
+enum OffloadVarEntryKindFlag : uint32_t {

tra wrote:
> jhuber6 wrote:
> > tra wrote:
> > > We can also fold both enums into one, as we still have the ambiguity of 
> > > what `flags=0` means. 
> > They're selected based on the size, if the size is zero it uses the kernel 
> > flags, otherwise it uses the variable flags. That's how it's done for 
> > OpenMP. I figured keeping the enums separate makes that more clear.
> > They're selected based on the size, if the size is zero it uses the kernel 
> > flags, otherwise it uses the variable flags. 
> 
> Why use two different enums, when one would do? It does not buy us anything 
> other than unnecessary additional complexity.
> 
I mostly copied this from OpenMP, I can merge it into one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123471

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


[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

2022-05-06 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: clang/lib/CodeGen/CGCUDARuntime.h:56-70
+enum OffloadRegionEntryKindFlag : uint32_t {
+  /// Mark the region entry as a kernel.
+  OffloadRegionKernelEntry = 0x0,
+};
+
+/// The kind flag of the global variable entry.
+enum OffloadVarEntryKindFlag : uint32_t {

jhuber6 wrote:
> tra wrote:
> > We can also fold both enums into one, as we still have the ambiguity of 
> > what `flags=0` means. 
> They're selected based on the size, if the size is zero it uses the kernel 
> flags, otherwise it uses the variable flags. That's how it's done for OpenMP. 
> I figured keeping the enums separate makes that more clear.
> They're selected based on the size, if the size is zero it uses the kernel 
> flags, otherwise it uses the variable flags. 

Why use two different enums, when one would do? It does not buy us anything 
other than unnecessary additional complexity.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123471

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


[PATCH] D125064: [clang-format][NFC] Make all TokenAnnotator member functions const

2022-05-06 Thread Owen Pan via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGaf4cf1c6b8ed: [clang-format][NFC] Make all TokenAnnotator 
member functions const (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125064

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h

Index: clang/lib/Format/TokenAnnotator.h
===
--- clang/lib/Format/TokenAnnotator.h
+++ clang/lib/Format/TokenAnnotator.h
@@ -161,43 +161,46 @@
   /// Adapts the indent levels of comment lines to the indent of the
   /// subsequent line.
   // FIXME: Can/should this be done in the UnwrappedLineParser?
-  void setCommentLineLevels(SmallVectorImpl );
+  void setCommentLineLevels(SmallVectorImpl ) const;
 
-  void annotate(AnnotatedLine );
-  void calculateFormattingInformation(AnnotatedLine );
+  void annotate(AnnotatedLine ) const;
+  void calculateFormattingInformation(AnnotatedLine ) const;
 
 private:
   /// Calculate the penalty for splitting before \c Tok.
   unsigned splitPenalty(const AnnotatedLine , const FormatToken ,
-bool InFunctionDecl);
+bool InFunctionDecl) const;
 
   bool spaceRequiredBeforeParens(const FormatToken ) const;
 
   bool spaceRequiredBetween(const AnnotatedLine , const FormatToken ,
-const FormatToken );
+const FormatToken ) const;
 
-  bool spaceRequiredBefore(const AnnotatedLine , const FormatToken );
+  bool spaceRequiredBefore(const AnnotatedLine ,
+   const FormatToken ) const;
 
-  bool mustBreakBefore(const AnnotatedLine , const FormatToken );
+  bool mustBreakBefore(const AnnotatedLine ,
+   const FormatToken ) const;
 
-  bool canBreakBefore(const AnnotatedLine , const FormatToken );
+  bool canBreakBefore(const AnnotatedLine ,
+  const FormatToken ) const;
 
   bool mustBreakForReturnType(const AnnotatedLine ) const;
 
-  void printDebugInfo(const AnnotatedLine );
+  void printDebugInfo(const AnnotatedLine ) const;
 
-  void calculateUnbreakableTailLengths(AnnotatedLine );
+  void calculateUnbreakableTailLengths(AnnotatedLine ) const;
 
-  void calculateArrayInitializerColumnList(AnnotatedLine );
+  void calculateArrayInitializerColumnList(AnnotatedLine ) const;
 
   FormatToken *calculateInitializerColumnList(AnnotatedLine ,
   FormatToken *CurrentToken,
-  unsigned Depth);
+  unsigned Depth) const;
   FormatStyle::PointerAlignmentStyle
-  getTokenReferenceAlignment(const FormatToken );
+  getTokenReferenceAlignment(const FormatToken ) const;
 
-  FormatStyle::PointerAlignmentStyle
-  getTokenPointerOrReferenceAlignment(const FormatToken );
+  FormatStyle::PointerAlignmentStyle getTokenPointerOrReferenceAlignment(
+  const FormatToken ) const;
 
   const FormatStyle 
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2523,7 +2523,7 @@
 } // end anonymous namespace
 
 void TokenAnnotator::setCommentLineLevels(
-SmallVectorImpl ) {
+SmallVectorImpl ) const {
   const AnnotatedLine *NextNonCommentLine = nullptr;
   for (AnnotatedLine *Line : llvm::reverse(Lines)) {
 assert(Line->First);
@@ -2558,7 +2558,7 @@
   return Result;
 }
 
-void TokenAnnotator::annotate(AnnotatedLine ) {
+void TokenAnnotator::annotate(AnnotatedLine ) const {
   for (auto  : Line.Children)
 annotate(*Child);
 
@@ -2725,7 +2725,7 @@
   return false;
 }
 
-void TokenAnnotator::calculateFormattingInformation(AnnotatedLine ) {
+void TokenAnnotator::calculateFormattingInformation(AnnotatedLine ) const {
   for (AnnotatedLine *ChildLine : Line.Children)
 calculateFormattingInformation(*ChildLine);
 
@@ -2845,7 +2845,8 @@
   LLVM_DEBUG({ printDebugInfo(Line); });
 }
 
-void TokenAnnotator::calculateUnbreakableTailLengths(AnnotatedLine ) {
+void TokenAnnotator::calculateUnbreakableTailLengths(
+AnnotatedLine ) const {
   unsigned UnbreakableTailLength = 0;
   FormatToken *Current = Line.Last;
   while (Current) {
@@ -2861,7 +2862,8 @@
   }
 }
 
-void TokenAnnotator::calculateArrayInitializerColumnList(AnnotatedLine ) {
+void TokenAnnotator::calculateArrayInitializerColumnList(
+AnnotatedLine ) const {
   if (Line.First == Line.Last)
 return;
   auto *CurrentToken = Line.First;
@@ -2881,7 +2883,7 @@
 }
 
 FormatToken *TokenAnnotator::calculateInitializerColumnList(
-AnnotatedLine , FormatToken *CurrentToken, unsigned Depth) {
+AnnotatedLine , FormatToken *CurrentToken, unsigned Depth) const {
   while (CurrentToken != nullptr && CurrentToken != 

[clang] af4cf1c - [clang-format][NFC] Make all TokenAnnotator member functions const

2022-05-06 Thread via cfe-commits

Author: owenca
Date: 2022-05-06T14:46:32-07:00
New Revision: af4cf1c6b8ed0d8102fc5e69acdc2fcbbcdaa9a7

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

LOG: [clang-format][NFC] Make all TokenAnnotator member functions const

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

Added: 


Modified: 
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/TokenAnnotator.h

Removed: 




diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 3714b9d3e0fc..8cbe157b1fb2 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2523,7 +2523,7 @@ class ExpressionParser {
 } // end anonymous namespace
 
 void TokenAnnotator::setCommentLineLevels(
-SmallVectorImpl ) {
+SmallVectorImpl ) const {
   const AnnotatedLine *NextNonCommentLine = nullptr;
   for (AnnotatedLine *Line : llvm::reverse(Lines)) {
 assert(Line->First);
@@ -2558,7 +2558,7 @@ static unsigned maxNestingDepth(const AnnotatedLine 
) {
   return Result;
 }
 
-void TokenAnnotator::annotate(AnnotatedLine ) {
+void TokenAnnotator::annotate(AnnotatedLine ) const {
   for (auto  : Line.Children)
 annotate(*Child);
 
@@ -2725,7 +2725,7 @@ bool TokenAnnotator::mustBreakForReturnType(const 
AnnotatedLine ) const {
   return false;
 }
 
-void TokenAnnotator::calculateFormattingInformation(AnnotatedLine ) {
+void TokenAnnotator::calculateFormattingInformation(AnnotatedLine ) const 
{
   for (AnnotatedLine *ChildLine : Line.Children)
 calculateFormattingInformation(*ChildLine);
 
@@ -2845,7 +2845,8 @@ void 
TokenAnnotator::calculateFormattingInformation(AnnotatedLine ) {
   LLVM_DEBUG({ printDebugInfo(Line); });
 }
 
-void TokenAnnotator::calculateUnbreakableTailLengths(AnnotatedLine ) {
+void TokenAnnotator::calculateUnbreakableTailLengths(
+AnnotatedLine ) const {
   unsigned UnbreakableTailLength = 0;
   FormatToken *Current = Line.Last;
   while (Current) {
@@ -2861,7 +2862,8 @@ void 
TokenAnnotator::calculateUnbreakableTailLengths(AnnotatedLine ) {
   }
 }
 
-void TokenAnnotator::calculateArrayInitializerColumnList(AnnotatedLine ) {
+void TokenAnnotator::calculateArrayInitializerColumnList(
+AnnotatedLine ) const {
   if (Line.First == Line.Last)
 return;
   auto *CurrentToken = Line.First;
@@ -2881,7 +2883,7 @@ void 
TokenAnnotator::calculateArrayInitializerColumnList(AnnotatedLine ) {
 }
 
 FormatToken *TokenAnnotator::calculateInitializerColumnList(
-AnnotatedLine , FormatToken *CurrentToken, unsigned Depth) {
+AnnotatedLine , FormatToken *CurrentToken, unsigned Depth) const {
   while (CurrentToken != nullptr && CurrentToken != Line.Last) {
 if (CurrentToken->is(tok::l_brace))
   ++Depth;
@@ -2901,7 +2903,7 @@ FormatToken 
*TokenAnnotator::calculateInitializerColumnList(
 
 unsigned TokenAnnotator::splitPenalty(const AnnotatedLine ,
   const FormatToken ,
-  bool InFunctionDecl) {
+  bool InFunctionDecl) const {
   const FormatToken  = *Tok.Previous;
   const FormatToken  = Tok;
 
@@ -3113,7 +3115,7 @@ bool TokenAnnotator::spaceRequiredBeforeParens(const 
FormatToken ) const {
 
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine ,
   const FormatToken ,
-  const FormatToken ) {
+  const FormatToken ) const {
   if (Left.is(tok::kw_return) &&
   !Right.isOneOf(tok::semi, tok::r_paren, tok::hashhash))
 return true;
@@ -3491,7 +3493,7 @@ bool TokenAnnotator::spaceRequiredBetween(const 
AnnotatedLine ,
 }
 
 bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine ,
- const FormatToken ) {
+ const FormatToken ) const {
   const FormatToken  = *Right.Previous;
 
   // If the token is finalized don't touch it (as it could be in a
@@ -3930,7 +3932,7 @@ static const FormatToken *getFirstNonComment(const 
AnnotatedLine ) {
 }
 
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine ,
- const FormatToken ) {
+ const FormatToken ) const {
   const FormatToken  = *Right.Previous;
   if (Right.NewlinesBefore > 1 && Style.MaxEmptyLinesToKeep > 0)
 return true;
@@ -4310,7 +4312,7 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine 
,
 }
 
 bool TokenAnnotator::canBreakBefore(const AnnotatedLine ,
-const FormatToken ) {
+const FormatToken ) const {
   const FormatToken  = *Right.Previous;
   // Language-specific stuff.
   if 

[PATCH] D124966: Thread safety analysis: Handle compound assignment and ->* overloads

2022-05-06 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:173
   effect.
 
 - ``-Wmisexpect`` warns when the branch weights collected during profiling

Or maybe I should add it here? Not sure why there is a blank line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124966

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


[PATCH] D123676: [clang-format] Fix WhitespaceSensitiveMacros not being honoured when macro closing parenthesis is followed by a newline.

2022-05-06 Thread ksyx via Phabricator via cfe-commits
ksyx accepted this revision.
ksyx added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123676

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


[PATCH] D124966: Thread safety analysis: Handle compound assignment and ->* overloads

2022-05-06 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert updated this revision to Diff 427750.
aaronpuchert added a comment.

Add a release note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124966

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Analysis/ThreadSafety.cpp
  clang/test/SemaCXX/warn-thread-safety-analysis.cpp

Index: clang/test/SemaCXX/warn-thread-safety-analysis.cpp
===
--- clang/test/SemaCXX/warn-thread-safety-analysis.cpp
+++ clang/test/SemaCXX/warn-thread-safety-analysis.cpp
@@ -82,6 +82,9 @@
   T* ptr_;
 };
 
+template
+U& operator->*(const SmartPtr& ptr, U T::*p) { return ptr->*p; }
+
 
 // For testing destructor calls and cleanup.
 class MyString {
@@ -4338,6 +4341,21 @@
 
   void operator()() { }
 
+  Data& operator+=(int);
+  Data& operator-=(int);
+  Data& operator*=(int);
+  Data& operator/=(int);
+  Data& operator%=(int);
+  Data& operator^=(int);
+  Data& operator&=(int);
+  Data& operator|=(int);
+  Data& operator<<=(int);
+  Data& operator>>=(int);
+  Data& operator++();
+  Data& operator++(int);
+  Data& operator--();
+  Data& operator--(int);
+
 private:
   int dat;
 };
@@ -4404,6 +4422,20 @@
   // expected-warning {{reading variable 'datap1_' requires holding mutex 'mu_'}}
 data_ = *datap2_; // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}} \
   // expected-warning {{reading the value pointed to by 'datap2_' requires holding mutex 'mu_'}}
+data_ += 1;   // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+data_ -= 1;   // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+data_ *= 1;   // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+data_ /= 1;   // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+data_ %= 1;   // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+data_ ^= 1;   // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+data_ &= 1;   // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+data_ |= 1;   // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+data_ <<= 1;  // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+data_ >>= 1;  // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+++data_;  // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+data_++;  // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+--data_;  // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
+data_--;  // expected-warning {{writing variable 'data_' requires holding mutex 'mu_' exclusively}}
 
 data_[0] = 0; // expected-warning {{reading variable 'data_' requires holding mutex 'mu_'}}
 (*datap2_)[0] = 0;// expected-warning {{reading the value pointed to by 'datap2_' requires holding mutex 'mu_'}}
@@ -4923,6 +4955,8 @@
   SmartPtr  sp GUARDED_BY(mu1) PT_GUARDED_BY(mu2);
   SmartPtr sq GUARDED_BY(mu1) PT_GUARDED_BY(mu2);
 
+  static constexpr int Cell::*pa = ::a;
+
   void test1() {
 mu1.ReaderLock();
 mu2.Lock();
@@ -4931,6 +4965,7 @@
 if (*sp == 0) doSomething();
 *sp = 0;
 sq->a = 0;
+sq->*pa = 0;
 
 if (sp[0] == 0) doSomething();
 sp[0] = 0;
@@ -4946,6 +4981,7 @@
 if (*sp == 0) doSomething();   // expected-warning {{reading variable 'sp' requires holding mutex 'mu1'}}
 *sp = 0;   // expected-warning {{reading variable 'sp' requires holding mutex 'mu1'}}
 sq->a = 0; // expected-warning {{reading variable 'sq' requires holding mutex 'mu1'}}
+sq->*pa = 0;   // expected-warning {{reading variable 'sq' requires holding mutex 'mu1'}}
 
 if (sp[0] == 0) doSomething();   // expected-warning {{reading variable 'sp' requires holding mutex 'mu1'}}
 sp[0] = 0;   // expected-warning {{reading variable 'sp' requires holding mutex 'mu1'}}
@@ -4962,6 +4998,7 @@
 if (*sp == 0) doSomething();   // expected-warning {{reading the value pointed to by 'sp' requires holding mutex 'mu2'}}
 *sp = 0;   // expected-warning {{reading the value pointed to by 'sp' requires holding mutex 'mu2'}}
 sq->a = 0; // expected-warning {{reading the value pointed to by 'sq' requires holding mutex 'mu2'}}
+sq->*pa = 0;   // 

[PATCH] D124700: [AMDGPU] Add llvm.amdgcn.sched.barrier intrinsic

2022-05-06 Thread Stanislav Mekhanoshin via Phabricator via cfe-commits
rampitec accepted this revision.
rampitec added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124700

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


[PATCH] D124966: Thread safety analysis: Handle compound assignment and ->* overloads

2022-05-06 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added a comment.

In D124966#3497305 , @aaron.ballman 
wrote:

> LGTM! You should probably add a release note for this change so users know 
> about it.

Good point, I'll add a note about compound assignment and increment/decrement. 
I'll leave out `operator->*` though if you don't mind because it's rarely 
overloaded.

> Do we need to update any documentation from this?

I don't think so, we don't really specify what we consider as write and what 
not. It's more of a heuristic anyway, unless at some point we feel comfortable 
enough to do D52395 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124966

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


[PATCH] D124700: [AMDGPU] Add llvm.amdgcn.sched.barrier intrinsic

2022-05-06 Thread Austin Kerbow via Phabricator via cfe-commits
kerbowa updated this revision to Diff 427747.
kerbowa added a comment.
Herald added a subscriber: jsilvanus.

Use i32.
Output hex.
Fix hazard rec tests for pseudo instructions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124700

Files:
  clang/include/clang/Basic/BuiltinsAMDGPU.def
  clang/test/CodeGenOpenCL/builtins-amdgcn.cl
  clang/test/SemaOpenCL/builtins-amdgcn-error.cl
  llvm/include/llvm/IR/IntrinsicsAMDGPU.td
  llvm/lib/Target/AMDGPU/AMDGPUMCInstLower.cpp
  llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
  llvm/lib/Target/AMDGPU/SIInstructions.td
  llvm/lib/Target/AMDGPU/Utils/AMDGPUMemoryUtils.cpp
  llvm/test/CodeGen/AMDGPU/hazard-pseudo-machineinstrs.mir
  llvm/test/CodeGen/AMDGPU/llvm.amdgcn.sched.barrier.ll
  llvm/test/CodeGen/AMDGPU/sched_barrier.mir

Index: llvm/test/CodeGen/AMDGPU/sched_barrier.mir
===
--- /dev/null
+++ llvm/test/CodeGen/AMDGPU/sched_barrier.mir
@@ -0,0 +1,99 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -march=amdgcn -mcpu=gfx908 -run-pass=machine-scheduler -verify-misched -o - %s | FileCheck %s
+
+--- |
+  define amdgpu_kernel void @no_sched_barrier(i32 addrspace(1)* noalias %out, i32 addrspace(1)* noalias %in) { ret void }
+  define amdgpu_kernel void @sched_barrier_0(i32 addrspace(1)* noalias %out, i32 addrspace(1)* noalias %in) { ret void }
+  define amdgpu_kernel void @sched_barrier_1(i32 addrspace(1)* noalias %out, i32 addrspace(1)* noalias %in) { ret void }
+
+  !0 = distinct !{!0}
+  !1 = !{!1, !0}
+...
+
+---
+name: no_sched_barrier
+tracksRegLiveness: true
+body: |
+  bb.0:
+; CHECK-LABEL: name: no_sched_barrier
+; CHECK: [[DEF:%[0-9]+]]:sreg_64 = IMPLICIT_DEF
+; CHECK-NEXT: [[DEF1:%[0-9]+]]:vgpr_32 = IMPLICIT_DEF
+; CHECK-NEXT: [[GLOBAL_LOAD_DWORD_SADDR:%[0-9]+]]:vgpr_32 = GLOBAL_LOAD_DWORD_SADDR [[DEF]], [[DEF1]], 0, 0, implicit $exec :: (load (s32) from %ir.in, !alias.scope !0, addrspace 1)
+; CHECK-NEXT: [[GLOBAL_LOAD_DWORD_SADDR1:%[0-9]+]]:vgpr_32 = GLOBAL_LOAD_DWORD_SADDR [[DEF]], [[DEF1]], 512, 0, implicit $exec :: (load (s32) from %ir.in, !alias.scope !0, addrspace 1)
+; CHECK-NEXT: [[V_MUL_LO_U32_e64_:%[0-9]+]]:vgpr_32 = nsw V_MUL_LO_U32_e64 [[GLOBAL_LOAD_DWORD_SADDR]], [[GLOBAL_LOAD_DWORD_SADDR]], implicit $exec
+; CHECK-NEXT: [[V_MUL_LO_U32_e64_1:%[0-9]+]]:vgpr_32 = nsw V_MUL_LO_U32_e64 [[GLOBAL_LOAD_DWORD_SADDR1]], [[GLOBAL_LOAD_DWORD_SADDR1]], implicit $exec
+; CHECK-NEXT: S_NOP 0
+; CHECK-NEXT: GLOBAL_STORE_DWORD_SADDR [[DEF1]], [[V_MUL_LO_U32_e64_]], [[DEF]], 0, 0, implicit $exec :: (store (s32) into %ir.out, !noalias !0, addrspace 1)
+; CHECK-NEXT: GLOBAL_STORE_DWORD_SADDR [[DEF1]], [[V_MUL_LO_U32_e64_1]], [[DEF]], 512, 0, implicit $exec :: (store (s32) into %ir.out, !noalias !0, addrspace 1)
+; CHECK-NEXT: S_ENDPGM 0
+%0:sreg_64 = IMPLICIT_DEF
+%1:vgpr_32 = IMPLICIT_DEF
+%3:vgpr_32 = GLOBAL_LOAD_DWORD_SADDR %0, %1, 0, 0, implicit $exec :: (load (s32) from %ir.in, !alias.scope !0, addrspace 1)
+%4:vgpr_32 = nsw V_MUL_LO_U32_e64 %3, %3, implicit $exec
+GLOBAL_STORE_DWORD_SADDR %1, %4, %0, 0, 0, implicit $exec :: (store (s32) into %ir.out, !noalias !0, addrspace 1)
+S_NOP 0
+%5:vgpr_32 = GLOBAL_LOAD_DWORD_SADDR %0, %1, 512, 0, implicit $exec :: (load (s32) from %ir.in, !alias.scope !0, addrspace 1)
+%6:vgpr_32 = nsw V_MUL_LO_U32_e64 %5, %5, implicit $exec
+GLOBAL_STORE_DWORD_SADDR %1, %6, %0, 512, 0, implicit $exec :: (store (s32) into %ir.out, !noalias !0, addrspace 1)
+S_ENDPGM 0
+...
+
+---
+name: sched_barrier_0
+tracksRegLiveness: true
+body: |
+  bb.0:
+; CHECK-LABEL: name: sched_barrier_0
+; CHECK: [[DEF:%[0-9]+]]:sreg_64 = IMPLICIT_DEF
+; CHECK-NEXT: [[DEF1:%[0-9]+]]:vgpr_32 = IMPLICIT_DEF
+; CHECK-NEXT: [[GLOBAL_LOAD_DWORD_SADDR:%[0-9]+]]:vgpr_32 = GLOBAL_LOAD_DWORD_SADDR [[DEF]], [[DEF1]], 0, 0, implicit $exec :: (load (s32) from %ir.in, !alias.scope !0, addrspace 1)
+; CHECK-NEXT: [[V_MUL_LO_U32_e64_:%[0-9]+]]:vgpr_32 = nsw V_MUL_LO_U32_e64 [[GLOBAL_LOAD_DWORD_SADDR]], [[GLOBAL_LOAD_DWORD_SADDR]], implicit $exec
+; CHECK-NEXT: GLOBAL_STORE_DWORD_SADDR [[DEF1]], [[V_MUL_LO_U32_e64_]], [[DEF]], 0, 0, implicit $exec :: (store (s32) into %ir.out, !noalias !0, addrspace 1)
+; CHECK-NEXT: S_NOP 0
+; CHECK-NEXT: SCHED_BARRIER 0
+; CHECK-NEXT: [[GLOBAL_LOAD_DWORD_SADDR1:%[0-9]+]]:vgpr_32 = GLOBAL_LOAD_DWORD_SADDR [[DEF]], [[DEF1]], 512, 0, implicit $exec :: (load (s32) from %ir.in, !alias.scope !0, addrspace 1)
+; CHECK-NEXT: [[V_MUL_LO_U32_e64_1:%[0-9]+]]:vgpr_32 = nsw V_MUL_LO_U32_e64 [[GLOBAL_LOAD_DWORD_SADDR1]], [[GLOBAL_LOAD_DWORD_SADDR1]], implicit $exec
+; CHECK-NEXT: GLOBAL_STORE_DWORD_SADDR [[DEF1]], [[V_MUL_LO_U32_e64_1]], [[DEF]], 512, 0, implicit $exec :: (store (s32) into %ir.out, !noalias !0, addrspace 1)
+; 

[clang-tools-extra] 1eb9748 - Fix check-clang-tools target after 7cc8377f2c572a919ecb

2022-05-06 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2022-05-06T23:08:47+02:00
New Revision: 1eb97481ef8b7c8923b29d80b8b018015dd3e27c

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

LOG: Fix check-clang-tools target after 7cc8377f2c572a919ecb

This change was intended to add the tests check-clang and check-clang-pseudo,
but afterwards it was *only* running those tests.
(This was because unlike add_lit_testsuite, add_lit_testsuite*s* does not
get included in umbrella suites).

Added: 


Modified: 
clang-tools-extra/test/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/test/CMakeLists.txt 
b/clang-tools-extra/test/CMakeLists.txt
index 8bce4da23d41..f4c529ee8af2 100644
--- a/clang-tools-extra/test/CMakeLists.txt
+++ b/clang-tools-extra/test/CMakeLists.txt
@@ -96,6 +96,12 @@ if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
   endif()
 endif()
 
+add_lit_testsuite(check-clang-extra "Running clang-tools-extra/test"
+   ${CMAKE_CURRENT_BINARY_DIR}
+   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
+   )
+set_target_properties(check-clang-extra PROPERTIES FOLDER "Clang extra tools' 
tests")
+
 add_lit_testsuites(CLANG-EXTRA ${CMAKE_CURRENT_SOURCE_DIR}
   DEPENDS ${CLANG_TOOLS_TEST_DEPS}
   )



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


[PATCH] D124701: [clang] Honor __attribute__((no_builtin("foo"))) on functions

2022-05-06 Thread Stephen Long via Phabricator via cfe-commits
steplong updated this revision to Diff 427741.
steplong added a comment.

Update patch to accept __attribute__((no_builtin("*")). It adds no-builtins to 
the function attributes, but it still doesn't affect the calls. The calls in 
the function still become builtin calls.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124701

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/no-builtin-2.c


Index: clang/test/CodeGen/no-builtin-2.c
===
--- /dev/null
+++ clang/test/CodeGen/no-builtin-2.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+typedef typeof(sizeof(0)) size_t;
+
+void bar(char *s);
+void *memset(void *s, int c, size_t n);
+void *memcpy(void *d, const void *s, size_t n);
+
+// CHECK: define{{.*}} void @foo1({{.*}}) #[[NO_NOBUILTIN:[0-9]+]]
+// CHECK:   call void @bar
+// CHECK:   call void @llvm.memset
+// CHECK:   call void @llvm.memcpy
+void foo1(char *s, char *d, size_t n)
+{
+bar(s);
+memset(s, 0, n);
+memcpy(d, s, n);
+}
+
+// CHECK: define{{.*}} void @foo2({{.*}}) #[[NOBUILTIN_MEMSET:[0-9]+]]
+// CHECK:   call void @bar
+// CHECK:   {{.*}}call {{.*}} @memset
+// CHECK:   call void @llvm.memcpy
+void foo2(char *s, char *d, size_t n) __attribute__((no_builtin("memset")))
+{
+bar(s);
+memset(s, 1, n);
+memcpy(d, s, n);
+}
+
+// CHECK: define{{.*}} void @foo3({{.*}}) #[[NOBUILTIN_MEMSET_MEMCPY:[0-9]+]]
+// CHECK:   call void @bar
+// CHECK:   {{.*}}call {{.*}} @memset
+// CHECK:   {{.*}}call {{.*}} @memcpy
+void foo3(char *s, char *d, size_t n) __attribute__((no_builtin("memset", 
"memcpy")))
+{
+bar(s);
+memset(s, 2, n);
+memcpy(d, s, n);
+}
+
+// CHECK-NOT: attributes #[[NO_NOBUILTIN]] = {{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK-NOT: attributes #[[NO_NOBUILTIN]] = 
{{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK: attributes #[[NOBUILTIN_MEMSET]] = 
{{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK: attributes #[[NOBUILTIN_MEMSET_MEMCPY]] = 
{{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -1142,7 +1142,8 @@
   if (!S.checkStringLiteralArgumentAttr(AL, I, BuiltinName, ))
 return;
 
-  if (Builtin::Context::isBuiltinFunc(BuiltinName))
+  bool ParsedAttrIsWildCard = BuiltinName == kWildcard;
+  if (Builtin::Context::isBuiltinFunc(BuiltinName) || ParsedAttrIsWildCard)
 AddBuiltinName(BuiltinName);
   else
 S.Diag(LiteralLoc, 
diag::warn_attribute_no_builtin_invalid_builtin_name)
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -5034,6 +5034,7 @@
   const FunctionDecl *FD = cast(GD.getDecl());
 
   if (auto builtinID = FD->getBuiltinID()) {
+std::string AttributeNoBuiltin = "no-builtin-" + FD->getName().str();
 std::string FDInlineName = (FD->getName() + ".inline").str();
 // When directing calling an inline builtin, call it through it's mangled
 // name to make it clear it's not the actual builtin.
@@ -5054,8 +5055,9 @@
 
 // Replaceable builtins provide their own implementation of a builtin. If 
we
 // are in an inline builtin implementation, avoid trivial infinite
-// recursion.
-else
+// recursion. Honor __attribute__((no_builtin("foo"))) on the current
+// function.
+else if (!CGF.CurFn->getAttributes().hasFnAttr(AttributeNoBuiltin))
   return CGCallee::forBuiltin(builtinID, FD);
   }
 


Index: clang/test/CodeGen/no-builtin-2.c
===
--- /dev/null
+++ clang/test/CodeGen/no-builtin-2.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+typedef typeof(sizeof(0)) size_t;
+
+void bar(char *s);
+void *memset(void *s, int c, size_t n);
+void *memcpy(void *d, const void *s, size_t n);
+
+// CHECK: define{{.*}} void @foo1({{.*}}) #[[NO_NOBUILTIN:[0-9]+]]
+// CHECK:   call void @bar
+// CHECK:   call void @llvm.memset
+// CHECK:   call void @llvm.memcpy
+void foo1(char *s, char *d, size_t n)
+{
+bar(s);
+memset(s, 0, n);
+memcpy(d, s, n);
+}
+
+// CHECK: define{{.*}} void @foo2({{.*}}) #[[NOBUILTIN_MEMSET:[0-9]+]]
+// CHECK:   call void @bar
+// CHECK:   {{.*}}call {{.*}} @memset
+// CHECK:   call void @llvm.memcpy
+void foo2(char *s, char *d, size_t n) __attribute__((no_builtin("memset")))
+{
+bar(s);
+memset(s, 1, n);
+memcpy(d, s, n);
+}
+
+// CHECK: define{{.*}} void @foo3({{.*}}) #[[NOBUILTIN_MEMSET_MEMCPY:[0-9]+]]
+// CHECK:   call void @bar
+// CHECK:   {{.*}}call {{.*}} @memset
+// CHECK:   {{.*}}call {{.*}} 

[PATCH] D125050: [OpenMP] Try to Infer target triples using the offloading architecture

2022-05-06 Thread Joseph Huber via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG509b631f84e9: [OpenMP] Try to Infer target triples using the 
offloading architecture (authored by jhuber6).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125050

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Driver.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/openmp-offload-infer.c

Index: clang/test/Driver/openmp-offload-infer.c
===
--- /dev/null
+++ clang/test/Driver/openmp-offload-infer.c
@@ -0,0 +1,50 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+// REQUIRES: amdgpu-registered-target
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp \
+// RUN:  --offload-arch=sm_52 --offload-arch=gfx803 \
+// RUN:  --libomptarget-amdgpu-bc-path=%S/Inputs/hip_dev_lib/libomptarget-amdgpu-gfx803.bc \
+// RUN:  --libomptarget-nvptx-bc-path=%S/Inputs/libomptarget/libomptarget-nvptx-test.bc %s 2>&1 \
+// RUN:   | FileCheck %s
+
+// verify the tools invocations
+// CHECK: "-cc1" "-triple" "x86_64-unknown-linux-gnu"{{.*}}"-emit-llvm-bc"{{.*}}"-x" "c"
+// CHECK: "-cc1" "-triple" "amdgcn-amd-amdhsa" "-aux-triple" "x86_64-unknown-linux-gnu"{{.*}}"-target-cpu" "gfx803"
+// CHECK: "-cc1" "-triple" "nvptx64-nvidia-cuda" "-aux-triple" "x86_64-unknown-linux-gnu"{{.*}}"-target-cpu" "sm_52"
+// CHECK: "-cc1" "-triple" "x86_64-unknown-linux-gnu"{{.*}}"-emit-obj"
+// CHECK: clang-linker-wrapper{{.*}}"--"{{.*}} "-o" "a.out"
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-bindings -fopenmp \
+// RUN: --offload-arch=sm_70 --offload-arch=gfx908:sramecc+:xnack- \
+// RUN: -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-NVIDIA-AMDGPU
+
+// CHECK-NVIDIA-AMDGPU: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[HOST_BC:.+]]"
+// CHECK-NVIDIA-AMDGPU: "amdgcn-amd-amdhsa" - "clang", inputs: ["[[INPUT]]", "[[HOST_BC]]"], output: "[[AMD_BC:.+]]"
+// CHECK-NVIDIA-AMDGPU: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT]]", "[[HOST_BC]]"], output: "[[NVIDIA_PTX:.+]]"
+// CHECK-NVIDIA-AMDGPU: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[NVIDIA_PTX]]"], output: "[[NVIDIA_CUBIN:.+]]"
+// CHECK-NVIDIA-AMDGPU: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[HOST_BC]]", "[[AMD_BC]]", "[[NVIDIA_CUBIN]]"], output: "[[HOST_OBJ:.+]]"
+// CHECK-NVIDIA-AMDGPU: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[HOST_OBJ]]"], output: "a.out"
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-bindings -fopenmp \
+// RUN: --offload-arch=sm_52 --offload-arch=sm_70 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-ARCH-BINDINGS
+
+// CHECK-ARCH-BINDINGS: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[INPUT:.*]]"], output: "[[HOST_BC:.*]]"
+// CHECK-ARCH-BINDINGS: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT]]", "[[HOST_BC]]"], output: "[[DEVICE_BC_SM_52:.*]]"
+// CHECK-ARCH-BINDINGS: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[DEVICE_BC_SM_52]]"], output: "[[DEVICE_OBJ_SM_52:.*]]"
+// CHECK-ARCH-BINDINGS: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT]]", "[[HOST_BC]]"], output: "[[DEVICE_BC_SM_70:.*]]"
+// CHECK-ARCH-BINDINGS: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[DEVICE_BC_SM_70]]"], output: "[[DEVICE_OBJ_SM_70:.*]]"
+// CHECK-ARCH-BINDINGS: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[HOST_BC]]", "[[DEVICE_OBJ_SM_52]]", "[[DEVICE_OBJ_SM_70]]"], output: "[[HOST_OBJ:.*]]"
+// CHECK-ARCH-BINDINGS: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[HOST_OBJ]]"], output: "a.out"
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-bindings -fopenmp \
+// RUN: --offload-arch=sm_70 --offload-arch=gfx908 --offload-arch=native \
+// RUN: -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-FAILED
+
+// CHECK-FAILED: error: failed to deduce triple for target architecture 'native'; specify the triple using '-fopenmp-targets' and '-Xopenmp-target' instead.
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-bindings -fopenmp \
+// RUN: --offload-arch=sm_70 --offload-arch=gfx908 -fno-openmp \
+// RUN: -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-DISABLED
+
+// CHECK-DISABLED-NOT: "nvptx64-nvidia-cuda" - "clang",
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7006,20 +7006,13 @@
   // For all the host OpenMP offloading compile jobs we need to pass the targets
   // information using -fopenmp-targets= option.
   if (JA.isHostOffloading(Action::OFK_OpenMP)) {
-SmallString<128> 

[PATCH] D124721: [OpenMP] Allow compiling multiple target architectures with OpenMP

2022-05-06 Thread Joseph Huber via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8477a0d769a0: [OpenMP] Allow compiling multiple target 
architectures with OpenMP (authored by jhuber6).

Changed prior to commit:
  https://reviews.llvm.org/D124721?vs=426521=427734#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124721

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/test/Driver/amdgpu-openmp-toolchain-new.c
  clang/test/Driver/openmp-offload-gpu-new.c

Index: clang/test/Driver/openmp-offload-gpu-new.c
===
--- clang/test/Driver/openmp-offload-gpu-new.c
+++ clang/test/Driver/openmp-offload-gpu-new.c
@@ -10,6 +10,10 @@
 // RUN:  -Xopenmp-target=nvptx64-nvidia-cuda -march=sm_52 \
 // RUN:  --libomptarget-nvptx-bc-path=%S/Inputs/libomptarget/libomptarget-nvptx-test.bc %s 2>&1 \
 // RUN:   | FileCheck %s
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda \
+// RUN:  --offload-arch=sm_52 \
+// RUN:  --libomptarget-nvptx-bc-path=%S/Inputs/libomptarget/libomptarget-nvptx-test.bc %s 2>&1 \
+// RUN:   | FileCheck %s
 
 // verify the tools invocations
 // CHECK: "-cc1" "-triple" "x86_64-unknown-linux-gnu"{{.*}}"-emit-llvm-bc"{{.*}}"-x" "c"
@@ -40,6 +44,27 @@
 // CHECK-BINDINGS: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[HOST_BC]]", "[[DEVICE_OBJ]]"], output: "[[HOST_OBJ:.*]]"
 // CHECK-BINDINGS: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[HOST_OBJ]]"], output: "a.out"
 
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-bindings -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda --offload-arch=sm_52 --offload-arch=sm_70 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-ARCH-BINDINGS
+// CHECK-ARCH-BINDINGS: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[INPUT:.*]]"], output: "[[HOST_BC:.*]]"
+// CHECK-ARCH-BINDINGS: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT]]", "[[HOST_BC]]"], output: "[[DEVICE_BC_SM_52:.*]]"
+// CHECK-ARCH-BINDINGS: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[DEVICE_BC_SM_52]]"], output: "[[DEVICE_OBJ_SM_52:.*]]"
+// CHECK-ARCH-BINDINGS: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT]]", "[[HOST_BC]]"], output: "[[DEVICE_BC_SM_70:.*]]"
+// CHECK-ARCH-BINDINGS: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[DEVICE_BC_SM_70]]"], output: "[[DEVICE_OBJ_SM_70:.*]]"
+// CHECK-ARCH-BINDINGS: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[HOST_BC]]", "[[DEVICE_OBJ_SM_52]]", "[[DEVICE_OBJ_SM_70]]"], output: "[[HOST_OBJ:.*]]"
+// CHECK-ARCH-BINDINGS: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[HOST_OBJ]]"], output: "a.out"
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-bindings -fopenmp \
+// RUN: -fopenmp-targets=nvptx64-nvidia-cuda,amdgcn-amd-amdhsa -Xopenmp-target=nvptx64-nvidia-cuda --offload-arch=sm_70 \
+// RUN: -fopenmp-targets=nvptx64-nvidia-cuda,amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa --offload-arch=gfx908  \
+// RUN: -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-NVIDIA-AMDGPU
+
+// CHECK-NVIDIA-AMDGPU: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[HOST_BC:.+]]"
+// CHECK-NVIDIA-AMDGPU: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT]]", "[[HOST_BC]]"], output: "[[NVIDIA_PTX:.+]]"
+// CHECK-NVIDIA-AMDGPU: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[NVIDIA_PTX]]"], output: "[[NVIDIA_CUBIN:.+]]"
+// CHECK-NVIDIA-AMDGPU: "amdgcn-amd-amdhsa" - "clang", inputs: ["[[INPUT]]", "[[HOST_BC]]"], output: "[[AMD_BC:.+]]"
+// CHECK-NVIDIA-AMDGPU: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[HOST_BC]]", "[[NVIDIA_CUBIN]]", "[[AMD_BC]]"], output: "[[HOST_OBJ:.+]]"
+// CHECK-NVIDIA-AMDGPU: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[HOST_OBJ]]"], output: "a.out"
+
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -emit-llvm -S -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda -Xopenmp-target=nvptx64-nvidia-cuda -march=sm_52 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-EMIT-LLVM-IR
 // CHECK-EMIT-LLVM-IR: "-cc1"{{.*}}"-triple" "nvptx64-nvidia-cuda"{{.*}}"-emit-llvm"
 
Index: clang/test/Driver/amdgpu-openmp-toolchain-new.c
===
--- clang/test/Driver/amdgpu-openmp-toolchain-new.c
+++ clang/test/Driver/amdgpu-openmp-toolchain-new.c
@@ -3,6 +3,9 @@
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa \
 // RUN:  -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx906 --libomptarget-amdgpu-bc-path=%S/Inputs/hip_dev_lib %s 2>&1 \
 // RUN:   | FileCheck %s
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa \
+// RUN:   

[clang] 8477a0d - [OpenMP] Allow compiling multiple target architectures with OpenMP

2022-05-06 Thread Joseph Huber via cfe-commits

Author: Joseph Huber
Date: 2022-05-06T16:57:16-04:00
New Revision: 8477a0d769a0b877f14e3ec3fde576b6a3b173ce

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

LOG: [OpenMP] Allow compiling multiple target architectures with OpenMP

This patch adds support for OpenMP to use the `--offload-arch` and
`--no-offload-arch` options. Traditionally, OpenMP has only supported
compiling for a single architecture via the `-Xopenmp-target` option.
Now we can pass in a bound architecture and use that if given, otherwise
we default to the value of the `-march` option as before.

Note that this only applies the basic support, the OpenMP target runtime
does not yet know how to choose between multiple architectures.
Additionally other parts of the offloading toolchain (e.g. LTO) require
the `-march` option, these should be worked out later.

Reviewed By: tra

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

Added: 


Modified: 
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
clang/lib/Driver/ToolChains/Cuda.cpp
clang/test/Driver/amdgpu-openmp-toolchain-new.c
clang/test/Driver/openmp-offload-gpu-new.c

Removed: 




diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 5bba896d8c37..b7dd59c45f2d 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -4214,17 +4214,20 @@ void Driver::BuildActions(Compilation , 
DerivedArgList ,
 /// Returns the canonical name for the offloading architecture when using HIP 
or
 /// CUDA.
 static StringRef getCanonicalArchString(Compilation ,
-llvm::opt::DerivedArgList ,
+const llvm::opt::DerivedArgList ,
 StringRef ArchStr,
-Action::OffloadKind Kind) {
-  if (Kind == Action::OFK_Cuda) {
+Action::OffloadKind Kind,
+const ToolChain *TC) {
+  if (Kind == Action::OFK_Cuda ||
+  (Kind == Action::OFK_OpenMP && TC->getTriple().isNVPTX())) {
 CudaArch Arch = StringToCudaArch(ArchStr);
 if (Arch == CudaArch::UNKNOWN || !IsNVIDIAGpuArch(Arch)) {
   C.getDriver().Diag(clang::diag::err_drv_cuda_bad_gpu_arch) << ArchStr;
   return StringRef();
 }
 return Args.MakeArgStringRef(CudaArchToString(Arch));
-  } else if (Kind == Action::OFK_HIP) {
+  } else if (Kind == Action::OFK_HIP ||
+ (Kind == Action::OFK_OpenMP && TC->getTriple().isAMDGPU())) {
 llvm::StringMap Features;
 // getHIPOffloadTargetTriple() is known to return valid value as it has
 // been called successfully in the CreateOffloadingDeviceToolChains().
@@ -4239,7 +4242,8 @@ static StringRef getCanonicalArchString(Compilation ,
 return Args.MakeArgStringRef(
 getCanonicalTargetID(Arch.getValue(), Features));
   }
-  return StringRef();
+  // If the input isn't CUDA or HIP just return the architecture.
+  return ArchStr;
 }
 
 /// Checks if the set offloading architectures does not conflict. Returns the
@@ -4259,12 +4263,8 @@ getConflictOffloadArchCombination(const 
llvm::DenseSet ,
 /// This function returns a set of bound architectures, if there are no bound
 /// architctures we return a set containing only the empty string.
 static llvm::DenseSet
-getOffloadArchs(Compilation , llvm::opt::DerivedArgList ,
-Action::OffloadKind Kind) {
-
-  // If this is OpenMP offloading we don't use a bound architecture.
-  if (Kind == Action::OFK_OpenMP)
-return llvm::DenseSet{StringRef()};
+getOffloadArchs(Compilation , const llvm::opt::DerivedArgList ,
+Action::OffloadKind Kind, const ToolChain *TC) {
 
   // --offload and --offload-arch options are mutually exclusive.
   if (Args.hasArgNoClaim(options::OPT_offload_EQ) &&
@@ -4280,12 +4280,12 @@ getOffloadArchs(Compilation , 
llvm::opt::DerivedArgList ,
   llvm::DenseSet Archs;
   for (auto  : Args) {
 if (Arg->getOption().matches(options::OPT_offload_arch_EQ)) {
-  Archs.insert(getCanonicalArchString(C, Args, Arg->getValue(), Kind));
+  Archs.insert(getCanonicalArchString(C, Args, Arg->getValue(), Kind, TC));
 } else if (Arg->getOption().matches(options::OPT_no_offload_arch_EQ)) {
   if (Arg->getValue() == StringRef("all"))
 Archs.clear();
   else
-Archs.erase(getCanonicalArchString(C, Args, Arg->getValue(), Kind));
+Archs.erase(getCanonicalArchString(C, Args, Arg->getValue(), Kind, 
TC));
 }
   }
 
@@ -4301,6 +4301,11 @@ getOffloadArchs(Compilation , 
llvm::opt::DerivedArgList ,
   Archs.insert(CudaArchToString(CudaArch::CudaDefault));
 else if (Kind == Action::OFK_HIP)
   

[clang] 509b631 - [OpenMP] Try to Infer target triples using the offloading architecture

2022-05-06 Thread Joseph Huber via cfe-commits

Author: Joseph Huber
Date: 2022-05-06T16:57:18-04:00
New Revision: 509b631f84e97e717a675a6ef60ae4728bb11551

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

LOG: [OpenMP] Try to Infer target triples using the offloading architecture

Currently we require the `-fopenmp-targets=` option to specify the
triple to use for the offloading toolchains, and the `-Xopenmp-target=`
option to specify architectures to a specific toolchain. The changes
made in D124721 allowed us to use `--offload-arch=` to specify multiple
target architectures. However, this can become combersome with many
different architectures. This patch introduces functinality that
attempts to deduce the target triple and architectures from the
offloading action. Currently we will deduce known GPU architectures when
only `-fopenmp` is specified.

This required a bit of a hack to cache the deduced architectures,
without this we would've just thrown an error when we tried to look up
the architecture again when generating the job. Normally we require the
user to manually specify the toolchain arguments, but here they would
confict unless we overrode them.

Depends on: D124721

Reviewed By: saiislam

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

Added: 
clang/test/Driver/openmp-offload-infer.c

Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/include/clang/Driver/Driver.h
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index b35693462e33d..e58cfe6c4c566 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -56,6 +56,7 @@ def warn_drv_avr_stdlib_not_linked: Warning<
   "compiler runtime routines will be linked">,
   InGroup;
 def err_drv_cuda_bad_gpu_arch : Error<"unsupported CUDA gpu architecture: %0">;
+def err_drv_offload_bad_gpu_arch : Error<"unsupported %0 gpu architecture: 
%1">;
 def err_drv_no_cuda_installation : Error<
   "cannot find CUDA installation; provide its path via '--cuda-path', or pass "
   "'-nocudainc' to build without CUDA includes">;
@@ -317,6 +318,9 @@ def err_drv_omp_host_target_not_supported : Error<
 def err_drv_expecting_fopenmp_with_fopenmp_targets : Error<
   "'-fopenmp-targets' must be used in conjunction with a '-fopenmp' option "
   "compatible with offloading; e.g., '-fopenmp=libomp' or 
'-fopenmp=libiomp5'">;
+def err_drv_failed_to_deduce_target_from_arch : Error<
+  "failed to deduce triple for target architecture '%0'; specify the triple "
+  "using '-fopenmp-targets' and '-Xopenmp-target' instead.">;
 def err_drv_omp_offload_target_missingbcruntime : Error<
   "no library '%0' found in the default clang lib directory or in LIBRARY_PATH"
   "; use '--libomptarget-%1-bc-path' to specify %1 bitcode library">;

diff  --git a/clang/include/clang/Driver/Driver.h 
b/clang/include/clang/Driver/Driver.h
index eb739ef31d57b..f0f294a669d98 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -300,6 +300,11 @@ class Driver {
   /// stored in it, and will clean them up when torn down.
   mutable llvm::StringMap> ToolChains;
 
+  /// Cache of known offloading architectures for the ToolChain already 
derived.
+  /// This should only be modified when we first initialize the offloading
+  /// toolchains.
+  llvm::DenseMap> 
KnownArchs;
+
 private:
   /// TranslateInputArgs - Create a new derived argument list from the input
   /// arguments, after applying the standard argument translations.
@@ -456,6 +461,13 @@ class Driver {
  const InputTy ,
  Action *HostAction) const;
 
+  /// Returns the set of bound architectures active for this offload kind.
+  /// If there are no bound architctures we return a set containing only the
+  /// empty string.
+  llvm::DenseSet
+  getOffloadArchs(Compilation , const llvm::opt::DerivedArgList ,
+  Action::OffloadKind Kind, const ToolChain *TC) const;
+
   /// Check that the file referenced by Value exists. If it doesn't,
   /// issue a diagnostic and return false.
   /// If TypoCorrect is true and the file does not exist, see if it looks

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index b7dd59c45f2d8..86ef67d2de3ec 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -782,76 +782,117 @@ void 
Driver::CreateOffloadingDeviceToolChains(Compilation ,
   // OpenMP
   //
   // We need to generate an OpenMP toolchain if the user specified targets with
-  // the -fopenmp-targets option.
-  if (Arg *OpenMPTargets =
-  

[PATCH] D124776: [SPIR-V] Allow setting SPIR-V version via target triple

2022-05-06 Thread Ilia Diachkov via Phabricator via cfe-commits
iliya-diyachkov added a comment.

@Anastasia, when are you going to add support for v1.5? We would use it in the 
SPIR-V backend.


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

https://reviews.llvm.org/D124776

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


[PATCH] D124946: [clang] serialize ORIGINAL_PCH_DIR relative to BaseDirectory

2022-05-06 Thread Richard Howell via Phabricator via cfe-commits
rmaz updated this revision to Diff 427731.
rmaz added a comment.

fix for missing temp dir in test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124946

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Modules/relative-original-dir.m


Index: clang/test/Modules/relative-original-dir.m
===
--- /dev/null
+++ clang/test/Modules/relative-original-dir.m
@@ -0,0 +1,7 @@
+// RUN: rm -rf %t/normal-module-map
+// RUN: mkdir -p %t
+// RUN: cp -r %S/Inputs/normal-module-map %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -fmodule-name=libA 
-emit-module %t/normal-module-map/module.map -o 
%t/normal-module-map/outdir/mod.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram 
%t/normal-module-map/outdir/mod.pcm | FileCheck %s
+
+// CHECK:  blob data = 'outdir'
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1461,8 +1461,7 @@
 unsigned AbbrevCode = Stream.EmitAbbrev(std::move(Abbrev));
 
 SmallString<128> OutputPath(OutputFile);
-
-SM.getFileManager().makeAbsolutePath(OutputPath);
+PreparePathForOutput(OutputPath);
 StringRef origDir = llvm::sys::path::parent_path(OutputPath);
 
 RecordData::value_type Record[] = {ORIGINAL_PCH_DIR};
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -2898,6 +2898,7 @@
 
 case ORIGINAL_PCH_DIR:
   F.OriginalDir = std::string(Blob);
+  ResolveImportedPath(F, F.OriginalDir);
   break;
 
 case MODULE_NAME:


Index: clang/test/Modules/relative-original-dir.m
===
--- /dev/null
+++ clang/test/Modules/relative-original-dir.m
@@ -0,0 +1,7 @@
+// RUN: rm -rf %t/normal-module-map
+// RUN: mkdir -p %t
+// RUN: cp -r %S/Inputs/normal-module-map %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -fmodule-name=libA -emit-module %t/normal-module-map/module.map -o %t/normal-module-map/outdir/mod.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram %t/normal-module-map/outdir/mod.pcm | FileCheck %s
+
+// CHECK:  blob data = 'outdir'
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1461,8 +1461,7 @@
 unsigned AbbrevCode = Stream.EmitAbbrev(std::move(Abbrev));
 
 SmallString<128> OutputPath(OutputFile);
-
-SM.getFileManager().makeAbsolutePath(OutputPath);
+PreparePathForOutput(OutputPath);
 StringRef origDir = llvm::sys::path::parent_path(OutputPath);
 
 RecordData::value_type Record[] = {ORIGINAL_PCH_DIR};
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -2898,6 +2898,7 @@
 
 case ORIGINAL_PCH_DIR:
   F.OriginalDir = std::string(Blob);
+  ResolveImportedPath(F, F.OriginalDir);
   break;
 
 case MODULE_NAME:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D123784: [clang][analyzer][ctu] Traverse the ctu CallEnter nodes in reverse order

2022-05-06 Thread Gabor Marton via Phabricator via cfe-commits
martong marked an inline comment as done.
martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/WorkList.cpp:216-218
   // Number of inserted nodes, used to emulate DFS ordering in the priority
   // queue when insertions are equal.
+  long Counter = 0;

steakhal wrote:
> It seems like you are not exploiting the signedness of this variable. And a 
> counter should never be negative anyway.
It is used in the descendant class.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123784

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


[PATCH] D124719: [docs] PCH usage documentation update

2022-05-06 Thread Tom Honermann via Phabricator via cfe-commits
tahonermann closed this revision.
tahonermann added a comment.

Re-closing since this review has already landed.

@akyrtzi, did you see my last comment? If I don't hear back from you, I'll 
submit a new doc change that covers both forms of PCH use.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124719

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


[PATCH] D123784: [clang][analyzer][ctu] Traverse the ctu CallEnter nodes in reverse order

2022-05-06 Thread Gabor Marton via Phabricator via cfe-commits
martong added a comment.

In D123784#3496981 , @xazax.hun wrote:

> This approach fixes the worklist for the second phase. Would it be possible 
> to create a wrapper that reverses the order of any worklist instead of 
> committing to one and hardcode that?

We don't want to reverse the worklist completely. We could reverse if we want 
to, by popping all elements and adding them to another queue which has the 
negated ordering function.

We'd like to have a reversed order only for those nodes that are added to the 
CTUWorklist during the first phase. In all other cases the normal ordering is 
the desired. Why? Because we'd like to keep the original algorithm once we are 
evaluating an inlined foreign function. The only thing we'd like to change is 
that which foreign function to choose for evaluation next. And we'd like that 
to be the one that is closer to the root node in the exploded graph (this way 
the overall path lengths can decrease).

> Would it be possible to create a wrapper that reverses the order of any 
> worklist instead of committing to one and hardcode that?

I don't think this is possible. I mean how could we reverse the DFS, would that 
be a BFS? The set of the visited nodes might change at the moment when we 
choose an other node as next. The only way it could work, if we first collect 
all the nodes that we want to visit. But the graph is being built as we do the 
visitation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123784

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


[PATCH] D114023: [Driver] Pass --fix-cortex-a53-843419 automatically on Fuchsia

2022-05-06 Thread Petr Hosek via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7f0e741db97c: [Driver] Pass --fix-cortex-a53-843419 
automatically on Fuchsia (authored by phosek).
Herald added a subscriber: MaskRay.
Herald added a project: All.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114023

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -41,6 +41,7 @@
 // CHECK: "-pie"
 // CHECK: "--build-id"
 // CHECK: "--hash-style=gnu"
+// CHECK-AARCH64: "--fix-cortex-a53-843419"
 // CHECK: "-dynamic-linker" "ld.so.1"
 // CHECK: Scrt1.o
 // CHECK-NOT: crti.o
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -37,6 +37,8 @@
   static_cast(getToolChain());
   const Driver  = ToolChain.getDriver();
 
+  const llvm::Triple  = ToolChain.getEffectiveTriple();
+
   ArgStringList CmdArgs;
 
   // Silence warning for "clang -g foo.o -o foo"
@@ -84,6 +86,12 @@
 CmdArgs.push_back("--hash-style=gnu");
   }
 
+  if (ToolChain.getArch() == llvm::Triple::aarch64) {
+std::string CPU = getCPUName(D, Args, Triple);
+if (CPU.empty() || CPU == "generic" || CPU == "cortex-a53")
+  CmdArgs.push_back("--fix-cortex-a53-843419");
+  }
+
   CmdArgs.push_back("--eh-frame-hdr");
 
   if (Args.hasArg(options::OPT_static))


Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -41,6 +41,7 @@
 // CHECK: "-pie"
 // CHECK: "--build-id"
 // CHECK: "--hash-style=gnu"
+// CHECK-AARCH64: "--fix-cortex-a53-843419"
 // CHECK: "-dynamic-linker" "ld.so.1"
 // CHECK: Scrt1.o
 // CHECK-NOT: crti.o
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -37,6 +37,8 @@
   static_cast(getToolChain());
   const Driver  = ToolChain.getDriver();
 
+  const llvm::Triple  = ToolChain.getEffectiveTriple();
+
   ArgStringList CmdArgs;
 
   // Silence warning for "clang -g foo.o -o foo"
@@ -84,6 +86,12 @@
 CmdArgs.push_back("--hash-style=gnu");
   }
 
+  if (ToolChain.getArch() == llvm::Triple::aarch64) {
+std::string CPU = getCPUName(D, Args, Triple);
+if (CPU.empty() || CPU == "generic" || CPU == "cortex-a53")
+  CmdArgs.push_back("--fix-cortex-a53-843419");
+  }
+
   CmdArgs.push_back("--eh-frame-hdr");
 
   if (Args.hasArg(options::OPT_static))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 7f0e741 - [Driver] Pass --fix-cortex-a53-843419 automatically on Fuchsia

2022-05-06 Thread Petr Hosek via cfe-commits

Author: Petr Hosek
Date: 2022-05-06T13:27:10-07:00
New Revision: 7f0e741db97c64b4a566d65b878c2e0fe4dabb38

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

LOG: [Driver] Pass --fix-cortex-a53-843419 automatically on Fuchsia

When targeting cortex-a53, set this linker flag rather than relying
on the toolchain users to do it in their build.

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Fuchsia.cpp
clang/test/Driver/fuchsia.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Fuchsia.cpp 
b/clang/lib/Driver/ToolChains/Fuchsia.cpp
index 6fe6f7f82feb..e81c40c74fdb 100644
--- a/clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ b/clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -37,6 +37,8 @@ void fuchsia::Linker::ConstructJob(Compilation , const 
JobAction ,
   static_cast(getToolChain());
   const Driver  = ToolChain.getDriver();
 
+  const llvm::Triple  = ToolChain.getEffectiveTriple();
+
   ArgStringList CmdArgs;
 
   // Silence warning for "clang -g foo.o -o foo"
@@ -84,6 +86,12 @@ void fuchsia::Linker::ConstructJob(Compilation , const 
JobAction ,
 CmdArgs.push_back("--hash-style=gnu");
   }
 
+  if (ToolChain.getArch() == llvm::Triple::aarch64) {
+std::string CPU = getCPUName(D, Args, Triple);
+if (CPU.empty() || CPU == "generic" || CPU == "cortex-a53")
+  CmdArgs.push_back("--fix-cortex-a53-843419");
+  }
+
   CmdArgs.push_back("--eh-frame-hdr");
 
   if (Args.hasArg(options::OPT_static))

diff  --git a/clang/test/Driver/fuchsia.c b/clang/test/Driver/fuchsia.c
index 888f6b1a2c4b..024c029e0e0c 100644
--- a/clang/test/Driver/fuchsia.c
+++ b/clang/test/Driver/fuchsia.c
@@ -41,6 +41,7 @@
 // CHECK: "-pie"
 // CHECK: "--build-id"
 // CHECK: "--hash-style=gnu"
+// CHECK-AARCH64: "--fix-cortex-a53-843419"
 // CHECK: "-dynamic-linker" "ld.so.1"
 // CHECK: Scrt1.o
 // CHECK-NOT: crti.o



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


[PATCH] D123773: [clang][analyzer][ctu] Make CTU a two phase analysis

2022-05-06 Thread Gabor Marton via Phabricator via cfe-commits
martong marked 4 inline comments as done.
martong added a comment.

Thanks for the review Gabor, I'll update soon and hope I could answer your 
questions.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:116
 
+  const bool Foreign = false; // From CTU.
+

xazax.hun wrote:
> I feel that we use different terms for the imported declarations. Sometimes 
> we call them `new`, sometimes `imported`, sometimes `foreign`. In case all of 
> these means the same thing, it would be nice to standardize on a single way 
> of naming. If there is a subtle difference between them, let's document that 
> in a comment. It would be nice if we did not need the comment after the 
> declaration but it would be obvious from the variable name.
Yes, I agree that this should deserver some more explanation. Maybe right above 
this declaration?

So, `new` means that a declaration is **created** newly by the ASTImporter.
`imported` means it has been imported, but not necessarily `new`. Think about 
this case, we import `foo`'s definition.
```
// to.cpp
void bar() {} // from a.h
// from.cpp
void bar() {} // from a.h
void foo() {
  bar();
}
```
Then `foo` will be `new` and `imported`, `bar` will be `imported` and not 
`new`.  
`foreign` basically means `imported` and `new`.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h:154
   llvm::PointerUnion Origin;
+  mutable Optional Foreign; // From CTU.
 

xazax.hun wrote:
> Why do we need this to be mutable? Shouldn't we have this information when 
> the CallEvent is created?
Unfortunately, we get this from the `RuntimeDefinition` which is not available 
during the creation of the `CallEvent`. We use `getRuntimeDefinition()` to 
retrieve the definition and attach that to the already existent `CallEvent`.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h:176
   WorkList *getWorkList() const { return WList.get(); }
+  WorkList *getCTUWorkList() const { return CTUWList.get(); }
 

xazax.hun wrote:
> I think we do not expect the STU WorkList to ever be null, maybe it is time 
> to clean this up and return a reference. 
I'd rather do that independently, so this patch can be focused and kept 
minimal, if you don't mind.
(I mean `WorkList ` would introduce irrelevant scattered changes.)



Comment at: clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:91
+   .Default(None);
+  assert(K.hasValue() && "CTU inlining mode is invalid.");
+  return K.getValue();

xazax.hun wrote:
> The `CTUPhase1InliningMode` is coming from the user, right? Unless we have 
> some validation in place before this code get called, I think it might not be 
> a good idea to assert fail on certain user inputs.
Yeah, I've copied this from below `assert(K.hasValue() && "IPA Mode is 
invalid.");`. Seems like the pattern we have everywhere. I suggest to get rid 
of this wrong pattern independently, but I'd not deviate from the pattern here, 
making it easier to do a bulk update once we do it.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:432
+
+bool ExprEngine::ctuBifurcate(const CallEvent , const Decl *D,
+  NodeBuilder , ExplodedNode *Pred,

xazax.hun wrote:
> What is the meaning if the return value? It looks like the function always 
> returns true.
Nothing. It is being used by `inlineCall` but there are no callers of 
`inlineCall` that would use it because `inlineCall` does return true on all 
paths as well. Good point, this is again a rotten legacy. I am going update 
this soon.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:446
+}
+const bool BState = State->contains(D);
+if (!BState) { // This is the first time we see this foreign function.

xazax.hun wrote:
> So if we see the same foreign function called in multiple contexts, we will 
> only queue one of the contexts for the CTU. Is this the intended design? 
> 
> So if I see:
> ```
> foreign(true);
> foreign(false);
> ```
> 
> The new CTU will only evaluate `foreign(true)` but not `foreign(false)`. 
This is intentional.
```
foreign(true);
foreign(false);
```
The new CTU will evaluate the following paths in the exploded graph:
```
foreign(true); // conservative evaluated
foreign(false); // conservative evaluated
foreign(true); // inlined
foreign(false); // inlined
```
The point is to keep bifurcation to a minimum and avoid the exponential blow up.
So, we will never have a path like this:
```
foreign(true); // conservative evaluated
foreign(false); // inlined
```

Actually, this is the same strategy that we use during the dynamic dispatch of 
C++ virtual calls. See `DynamicDispatchBifurcationMap`.

The conservative evaluation happens in the first phase, the inlining in the 
second 

[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

2022-05-06 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added inline comments.



Comment at: clang/lib/CodeGen/CGCUDARuntime.h:56-70
+enum OffloadRegionEntryKindFlag : uint32_t {
+  /// Mark the region entry as a kernel.
+  OffloadRegionKernelEntry = 0x0,
+};
+
+/// The kind flag of the global variable entry.
+enum OffloadVarEntryKindFlag : uint32_t {

tra wrote:
> We can also fold both enums into one, as we still have the ambiguity of what 
> `flags=0` means. 
They're selected based on the size, if the size is zero it uses the kernel 
flags, otherwise it uses the variable flags. That's how it's done for OpenMP. I 
figured keeping the enums separate makes that more clear.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123471

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


[PATCH] D120489: [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

2022-05-06 Thread Shivam Rajput via Phabricator via cfe-commits
phyBrackets added a comment.

In D120489#3493313 , @martong wrote:

> Maybe it is not too late to update the clang/docs/ReleaseNotes.rst? A new 
> checker is certainly important for the users. Many thanks!

Yeah done! Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120489

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


[clang] bbd0319 - Update ReleaseNotes.rst

2022-05-06 Thread via cfe-commits

Author: Shivam
Date: 2022-05-07T01:20:45+05:30
New Revision: bbd031943a3d1bd72fed362ee3e8456dbb901747

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

LOG: Update ReleaseNotes.rst

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ebc227cf2251..7cdd77b2df3a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -422,7 +422,7 @@ libclang
 Static Analyzer
 ---
 
-- Added a new checker ``alpha.unix.cstring.UninitializedRead `` this will 
check for uninitialized reads
+- Added a new checker ``alpha.unix.cstring.UninitializedRead`` this will check 
for uninitialized reads
   from common memory copy/manipulation functions such as ``memcpy``, 
``mempcpy``, ``memmove``, ``memcmp``, `
   `strcmp``, ``strncmp``, ``strcpy``, ``strlen``, ``strsep`` and many more. 
Although 
   this checker currently is in list of alpha checkers due to a false positive.



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


[PATCH] D124702: [MSVC] Add support for pragma function

2022-05-06 Thread Stephen Long via Phabricator via cfe-commits
steplong updated this revision to Diff 427716.
steplong added a reviewer: aaron.ballman.
steplong added a comment.

Changed it to use SmallSetVector instead of SmallVector. Had to use an 
temporary SmallVector in AddRangeBasedNoBuiltin() because I'm not sure how to 
get a pointer to StringRef. getArrayRef().copy().data() needs an Allocator.

Also, I can't figure out why the test pragma-ms-function-intrinsic.c is failing 
from the pragma not being in file context. The pragmas are not inside a 
function.

Will add Sema and parsing tests in the next update to this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124702

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CodeGen/pragma-ms-function-intrinsic.c

Index: clang/test/CodeGen/pragma-ms-function-intrinsic.c
===
--- /dev/null
+++ clang/test/CodeGen/pragma-ms-function-intrinsic.c
@@ -0,0 +1,78 @@
+// RUN: %clang_cc1 -emit-llvm -fms-extensions -o - %s | FileCheck %s
+
+typedef typeof(sizeof(0)) size_t;
+
+void bar(char *s);
+void *memset(void *s, int c, size_t n);
+void *memcpy(void *d, const void *s, size_t n);
+
+// CHECK: define{{.*}} void @foo1({{.*}}) #[[NO_NOBUILTIN:[0-9]+]]
+// CHECK:   call void @bar
+// CHECK:   call void @llvm.memset
+// CHECK:   call void @llvm.memcpy
+void foo1(char *s, char *d, size_t n)
+{
+bar(s);
+memset(s, 0, n);
+memcpy(d, s, n);
+}
+
+#pragma function(strlen, memset)
+
+// CHECK: define{{.*}} void @foo2({{.*}}) #[[NOBUILTIN_MEMSET:[0-9]+]]
+// CHECK:   call void @bar
+// CHECK:   {{.*}}call {{.*}} @memset
+// CHECK:   call void @llvm.memcpy
+void foo2(char *s, char *d, size_t n)
+{
+bar(s);
+memset(s, 1, n);
+memcpy(d, s, n);
+}
+
+#pragma function(memcpy)
+
+// CHECK: define{{.*}} void @foo3({{.*}}) #[[NOBUILTIN_MEMSET_MEMCPY:[0-9]+]]
+// CHECK:   call void @bar
+// CHECK:   {{.*}}call {{.*}} @memset
+// CHECK:   {{.*}}call {{.*}} @memcpy
+void foo3(char *s, char *d, size_t n)
+{
+bar(s);
+memset(s, 2, n);
+memcpy(d, s, n);
+}
+
+#pragma intrinsic(memset, strlen)
+
+// CHECK: define{{.*}} void @foo4({{.*}}) #[[NOBUILTIN_MEMCPY:[0-9]+]]
+// CHECK:   call void @bar
+// CHECK:   call void @llvm.memset
+// CHECK:   {{.*}}call {{.*}} @memcpy
+void foo4(char *s, char *d, size_t n)
+{
+bar(s);
+memset(s, 3, n);
+memcpy(d, s, n);
+}
+
+#pragma intrinsic(memcpy)
+
+// CHECK: define{{.*}} void @foo5({{.*}}) #[[NO_NOBUILTIN]]
+// CHECK:   call void @bar
+// CHECK:   call void @llvm.memset
+// CHECK:   call void @llvm.memcpy
+void foo5(char *s, char *d, size_t n)
+{
+bar(s);
+memset(s, 4, n);
+memcpy(d, s, n);
+}
+
+// CHECK-NOT: attributes #[[NO_NOBUILTIN]] = {{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK-NOT: attributes #[[NO_NOBUILTIN]] = {{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK: attributes #[[NOBUILTIN_MEMSET]] = {{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK-NOT: attributes #[[NOBUILTIN_MEMSET]] = {{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK: attributes #[[NOBUILTIN_MEMSET_MEMCPY]] = {{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}}
+// CHECK: attributes #[[NOBUILTIN_MEMCPY]] = {{{.*}}"no-builtin-memcpy"{{.*}}}
+// CHECK-NOT: attributes #[[NOBUILTIN_MEMCPY]] = {{{.*}}"no-builtin-memcpy"{{.*}}"no-builtin-memset"{{.*}}}
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10218,10 +10218,12 @@
   // marking the function.
   AddCFAuditedAttribute(NewFD);
 
-  // If this is a function definition, check if we have to apply optnone due to
-  // a pragma.
-  if(D.isFunctionDefinition())
+  // If this is a function definition, check if we have to apply any
+  // attributes (i.e. optnone and no_builtin) due to a pragma.
+  if(D.isFunctionDefinition()) {
 AddRangeBasedOptnone(NewFD);
+AddRangeBasedNoBuiltin(NewFD);
+  }
 
   // If this is the first declaration of an extern C variable, update
   // the map of such variables.
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -1065,6 +1065,27 @@
 OptimizeOffPragmaLocation = PragmaLoc;
 }
 
+void Sema::ActOnPragmaMSIntrinsic(
+SourceLocation Loc, const llvm::SmallSetVector ) {
+  if (!CurContext->getRedeclContext()->isFileContext()) {
+Diag(Loc, diag::err_pragma_intrinsic_function_scope);
+return;
+  }
+
+  for (auto  : Intrinsics)
+MSFunctionNoBuiltins.remove(Intrinsic);
+}
+
+void Sema::ActOnPragmaMSFunction(
+SourceLocation Loc, const llvm::SmallSetVector ) {
+ 

[clang] b390173 - update the doc for the static analyzer checker

2022-05-06 Thread via cfe-commits

Author: Shivam
Date: 2022-05-07T01:19:46+05:30
New Revision: b39017340806ee68b305d5c8330cbc3e1d398a4d

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

LOG: update the doc for the static analyzer checker

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index aa8376eed7f8..ebc227cf2251 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -423,8 +423,8 @@ Static Analyzer
 ---
 
 - Added a new checker ``alpha.unix.cstring.UninitializedRead `` this will 
check for uninitialized reads
-  from common memory copy/manipulation functions such as:
- ``memcpy, mempcpy, memmove, memcmp, strcmp, strncmp, strcpy, strlen, strsep`` 
and many more. Although 
+  from common memory copy/manipulation functions such as ``memcpy``, 
``mempcpy``, ``memmove``, ``memcmp``, `
+  `strcmp``, ``strncmp``, ``strcpy``, ``strlen``, ``strsep`` and many more. 
Although 
   this checker currently is in list of alpha checkers due to a false positive.
 
 .. _release-notes-ubsan:



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


[PATCH] D120727: [libc++] Overhaul how we select the ABI library

2022-05-06 Thread Louis Dionne via Phabricator via cfe-commits
ldionne updated this revision to Diff 427721.
ldionne added a comment.

Rebase onto main


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120727

Files:
  clang/cmake/caches/CrossWinToARMLinux.cmake
  libcxx/CMakeLists.txt
  libcxx/cmake/Modules/HandleLibCXXABI.cmake
  libcxx/docs/BuildingLibcxx.rst
  libcxx/include/CMakeLists.txt
  libcxx/lib/abi/CMakeLists.txt
  libcxx/src/CMakeLists.txt
  libcxx/test/CMakeLists.txt
  libcxx/test/configs/legacy.cfg.in
  libcxx/utils/libcxx/test/config.py

Index: libcxx/utils/libcxx/test/config.py
===
--- libcxx/utils/libcxx/test/config.py
+++ libcxx/utils/libcxx/test/config.py
@@ -412,7 +412,7 @@
 # The compiler normally links in oldnames.lib too, but we've
 # specified -nostdlib above, so we need to specify it manually.
 self.cxx.link_flags += ['-loldnames']
-elif cxx_abi == 'none' or cxx_abi == 'default':
+elif cxx_abi == 'none':
 if self.target_info.is_windows():
 debug_suffix = 'd' if self.debug_build else ''
 self.cxx.link_flags += ['-lmsvcrt%s' % debug_suffix]
Index: libcxx/test/configs/legacy.cfg.in
===
--- libcxx/test/configs/legacy.cfg.in
+++ libcxx/test/configs/legacy.cfg.in
@@ -14,7 +14,7 @@
 config.cxx_library_root = "@LIBCXX_LIBRARY_DIR@"
 config.abi_library_root = "@LIBCXX_CXX_ABI_LIBRARY_PATH@"
 config.enable_shared= @LIBCXX_LINK_TESTS_WITH_SHARED_LIBCXX@
-config.cxx_abi  = "@LIBCXX_CXX_ABI_LIBNAME@"
+config.cxx_abi  = "@LIBCXX_CXXABI_FOR_TESTS@"
 config.configuration_variant= "@LIBCXX_LIT_VARIANT@"
 config.host_triple  = "@LLVM_HOST_TRIPLE@"
 config.sysroot  = "@CMAKE_SYSROOT@"
Index: libcxx/test/CMakeLists.txt
===
--- libcxx/test/CMakeLists.txt
+++ libcxx/test/CMakeLists.txt
@@ -17,7 +17,9 @@
 # The tests shouldn't link to any ABI library when it has been linked into
 # libc++ statically or via a linker script.
 if (LIBCXX_ENABLE_STATIC_ABI_LIBRARY OR LIBCXX_ENABLE_ABI_LINKER_SCRIPT)
-  set(LIBCXX_CXX_ABI_LIBNAME "none")
+  set(LIBCXX_CXXABI_FOR_TESTS "none")
+else()
+  set(LIBCXX_CXXABI_FOR_TESTS "${LIBCXX_CXX_ABI}")
 endif()
 
 # The tests shouldn't link to libunwind if we have a linker script which
Index: libcxx/src/CMakeLists.txt
===
--- libcxx/src/CMakeLists.txt
+++ libcxx/src/CMakeLists.txt
@@ -156,11 +156,6 @@
   set(exclude_from_all EXCLUDE_FROM_ALL)
 endif()
 
-# If LIBCXX_CXX_ABI_LIBRARY_PATH is defined we want to add it to the search path.
-add_link_flags_if(LIBCXX_CXX_ABI_LIBRARY_PATH
-  "${CMAKE_LIBRARY_PATH_FLAG}${LIBCXX_CXX_ABI_LIBRARY_PATH}")
-
-
 if (LIBCXX_GENERATE_COVERAGE AND NOT LIBCXX_COVERAGE_LIBRARY)
   find_compiler_rt_library(profile LIBCXX_COVERAGE_LIBRARY)
 endif()
@@ -197,10 +192,6 @@
 endif()
 
 function(cxx_set_common_defines name)
-  if(LIBCXX_CXX_ABI_HEADER_TARGET)
-add_dependencies(${name} ${LIBCXX_CXX_ABI_HEADER_TARGET})
-  endif()
-
   if (LIBCXX_ENABLE_PARALLEL_ALGORITHMS)
 target_link_libraries(${name} PUBLIC pstl::ParallelSTL)
   endif()
@@ -241,19 +232,18 @@
   # Link against libc++abi
   if (LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY)
 if (APPLE)
-  target_link_libraries(cxx_shared PRIVATE "-Wl,-force_load" "${LIBCXX_CXX_STATIC_ABI_LIBRARY}")
+  target_link_libraries(cxx_shared PRIVATE "-Wl,-force_load" "$")
 else()
-  target_link_libraries(cxx_shared PRIVATE "-Wl,--whole-archive,-Bstatic" "${LIBCXX_CXX_STATIC_ABI_LIBRARY}" "-Wl,-Bdynamic,--no-whole-archive")
+  target_link_libraries(cxx_shared PRIVATE "-Wl,--whole-archive,-Bstatic" "$" "-Wl,-Bdynamic,--no-whole-archive")
 endif()
   else()
-target_link_libraries(cxx_shared PUBLIC "${LIBCXX_CXX_SHARED_ABI_LIBRARY}")
+target_link_libraries(cxx_shared PUBLIC libcxx-abi-shared)
   endif()
 
   # Maybe re-export symbols from libc++abi
   # In particular, we don't re-export the symbols if libc++abi is merged statically
   # into libc++ because in that case there's no dylib to re-export from.
-  if (APPLE AND (LIBCXX_CXX_ABI_LIBNAME STREQUAL "libcxxabi" OR
- LIBCXX_CXX_ABI_LIBNAME STREQUAL "default")
+  if (APPLE AND LIBCXX_CXX_ABI STREQUAL "libcxxabi"
 AND NOT DEFINED LIBCXX_OSX_REEXPORT_LIBCXXABI_SYMBOLS
 AND NOT LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY)
 set(LIBCXX_OSX_REEXPORT_LIBCXXABI_SYMBOLS ON)
@@ -292,7 +282,8 @@
   add_library(cxx_static STATIC ${exclude_from_all} ${LIBCXX_SOURCES} ${LIBCXX_HEADERS})
   target_include_directories(cxx_static PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
   target_link_libraries(cxx_static PUBLIC cxx-headers
- 

[clang] 24e9d90 - Added the brief discription about the new CSA checker.

2022-05-06 Thread via cfe-commits

Author: Shivam
Date: 2022-05-07T01:16:22+05:30
New Revision: 24e9d90e65243fd8674bfc264c1c7d27c3cce67c

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

LOG: Added the brief discription about the new CSA checker.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0a5d3a85834d3..aa8376eed7f8b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -422,7 +422,10 @@ libclang
 Static Analyzer
 ---
 
-- ...
+- Added a new checker ``alpha.unix.cstring.UninitializedRead `` this will 
check for uninitialized reads
+  from common memory copy/manipulation functions such as:
+ ``memcpy, mempcpy, memmove, memcmp, strcmp, strncmp, strcpy, strlen, strsep`` 
and many more. Although 
+  this checker currently is in list of alpha checkers due to a false positive.
 
 .. _release-notes-ubsan:
 



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


[PATCH] D124658: [analyzer] Canonicalize SymIntExpr so the RHS is positive when possible

2022-05-06 Thread Tomasz Kamiński via Phabricator via cfe-commits
tomasz-kaminski-sonarsource added a comment.

Added missing casts in `truncatingToUnsigned` tests.


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

https://reviews.llvm.org/D124658

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


[PATCH] D124658: [analyzer] Canonicalize SymIntExpr so the RHS is positive when possible

2022-05-06 Thread Tomasz Kamiński via Phabricator via cfe-commits
tomasz-kaminski-sonarsource updated this revision to Diff 427717.

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

https://reviews.llvm.org/D124658

Files:
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/additive-op-on-sym-int-expr.c

Index: clang/test/Analysis/additive-op-on-sym-int-expr.c
===
--- clang/test/Analysis/additive-op-on-sym-int-expr.c
+++ clang/test/Analysis/additive-op-on-sym-int-expr.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -triple=x86_64-unknown-linux -analyzer-checker=core,debug.ExprInspection -analyzer-config eagerly-assume=false -verify %s
+// RUN: %clang_analyze_cc1 -triple=x86_64-unknown-linux-gnu -analyzer-checker=core,apiModeling,debug.ExprInspection -analyzer-config eagerly-assume=false -verify %s
 
 void clang_analyzer_dump(int);
 void clang_analyzer_dumpL(long);
@@ -42,17 +42,127 @@
   clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) - 9U }} instead of + 4294967287U
 }
 
+const int intMin = 1 << (sizeof(int) * 8 - 1); // INT_MIN, negation value is not representable
+const long longMin = 1L << (sizeof(long) * 8 - 1); // LONG_MIN, negation value is not representable
+
 void testMin(int i, long l) {
   clang_analyzer_dump(i + (-1));  // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }} instead of + -1
   clang_analyzer_dump(i - (-1));  // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} instead of - -1
   clang_analyzer_dumpL(l + (-1)); // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }} instead of + -1
   clang_analyzer_dumpL(l - (-1)); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} instead of - -1
 
-  int intMin = 1 << (sizeof(int) * 8 - 1); // INT_MIN, negative value is not representable
   // Do not normalize representation if negation would not be representable
-  clang_analyzer_dump(i + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }}
-  clang_analyzer_dump(i - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - -2147483648 }}
+  clang_analyzer_dump(i + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }} no change
+  clang_analyzer_dump(i - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - -2147483648 }} no change
   // Produced value has higher bit with (long) so negation if representable
   clang_analyzer_dumpL(l + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648 }} instead of + -2147483648
   clang_analyzer_dumpL(l - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648 }} instead of - -2147483648
 }
+
+void changingToUnsinged(unsigned u, int i) {
+   unsigned c = u + (unsigned)i;
+   unsigned d = u - (unsigned)i;
+   if (i == -1) {
+ clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1U }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1U }}
+ return;
+   }
+   if (i == intMin) {
+ clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648U }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648U }}
+ return;
+   }
+}
+
+void extendingToSigned(long l, int i) {
+  long c = l + (long)i;
+  long d = l - (long)i;
+  if (i == -1) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }}
+return;
+  }
+  if (i == intMin) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648 }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648 }}
+return;
+  }
+}
+
+void extendingToUnigned(unsigned long ul, int i) {
+  unsigned long c = ul + (unsigned long)i;
+  unsigned long d = ul - (unsigned long)i;
+  if (i == -1) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1U }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1U }}
+return;
+  }
+  if (i == intMin) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648U }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648U }}
+return;
+  }
+}
+
+void truncatingToSigned(int i, long l) {
+  int c = i + (int)l;
+  int d = i - (int)l;
+  if (l == -1L) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }}
+return;
+  }
+  if (l == (long)intMin) { // negation outside of range, no-changes
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }}
+return;
+  }
+  if (l == ((long)intMin - 1L)) { // outside or range, no changes
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483647 }}
+clang_analyzer_dump(d + 0); // 

[PATCH] D124866: [CUDA][HIP] support __noinline__ as keyword

2022-05-06 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

In D124866#3497439 , @tra wrote:

>> CUDA/HIP do not have language spec.
>
> Well. It's not completely true. CUDA programming guide does serve as the 
> de-facto spec for CUDA. It's far from perfect, but it does mention 
> `__noinline__` and `__forceinline__` as function qualifiers: 
> https://docs.nvidia.com/cuda/cuda-c-programming-guide/index.html#noinline-and-forceinline

Thanks for the pointer. I missed that part.

CUDA SDK implements `__noinline__` as attribute `__attribute__((noinline))` 
though. Some requirements may not have diagnostics.


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

https://reviews.llvm.org/D124866

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


[PATCH] D125120: Initial work on parameter info for function pointers

2022-05-06 Thread Visa via Phabricator via cfe-commits
Qwinci created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
Herald added a project: All.
Qwinci requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125120

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaCodeComplete.cpp

Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -3722,6 +3722,7 @@
const PrintingPolicy ,
const FunctionDecl *Function,
const FunctionProtoType *Prototype,
+   FunctionProtoTypeLoc PrototypeLoc,
CodeCompletionBuilder ,
unsigned CurrentArg, unsigned Start = 0,
bool InOptional = false) {
@@ -3743,7 +3744,7 @@
   if (!FirstParameter)
 Opt.AddChunk(CodeCompletionString::CK_Comma);
   // Optional sections are nested.
-  AddOverloadParameterChunks(Context, Policy, Function, Prototype, Opt,
+  AddOverloadParameterChunks(Context, Policy, Function, Prototype, PrototypeLoc, Opt,
  CurrentArg, P, /*InOptional=*/true);
   Result.AddOptionalChunk(Opt.TakeString());
   return;
@@ -3764,6 +3765,14 @@
   if (Param->hasDefaultArg())
 Placeholder += GetDefaultValueString(Param, Context.getSourceManager(),
  Context.getLangOpts());
+} else if (!PrototypeLoc.isNull()) {
+  if (P < PrototypeLoc.getNumParams()) {
+const ParmVarDecl *Param = PrototypeLoc.getParam(P);
+Placeholder = FormatFunctionParameter(Policy, Param);
+if (Param->hasDefaultArg())
+  Placeholder += GetDefaultValueString(Param, Context.getSourceManager(),
+Context.getLangOpts());
+  }
 } else {
   Placeholder = Prototype->getParamType(P).getAsString(Policy);
 }
@@ -3912,7 +3921,7 @@
   if (getKind() == CK_Aggregate)
 AddOverloadAggregateChunks(getAggregate(), Policy, Result, CurrentArg);
   else
-AddOverloadParameterChunks(S.getASTContext(), Policy, FDecl, Proto, Result,
+AddOverloadParameterChunks(S.getASTContext(), Policy, FDecl, Proto, ProtoTypeLoc, Result,
CurrentArg);
   Result.AddChunk(Braced ? CodeCompletionString::CK_RightBrace
  : CodeCompletionString::CK_RightParen);
@@ -5991,6 +6000,16 @@
   return getParamType(SemaRef, Candidates, CurrentArg);
 }
 
+static FunctionProtoTypeLoc GetPrototypeLoc(Expr *Fn) {
+  if (const auto *T = Fn->getType().getTypePtr()->getAs()) {
+const auto *D = T->getDecl();
+if (auto F = D->getTypeSourceInfo()->getTypeLoc().getAs()) {
+  return F;
+}
+  }
+  return FunctionProtoTypeLoc();
+}
+
 QualType Sema::ProduceCallSignatureHelp(Expr *Fn, ArrayRef Args,
 SourceLocation OpenParLoc) {
   Fn = unwrapParenList(Fn);
@@ -6072,6 +6091,8 @@
 } else {
   // Lastly we check whether expression's type is function pointer or
   // function.
+
+  FunctionProtoTypeLoc P = GetPrototypeLoc(NakedFn);
   QualType T = NakedFn->getType();
   if (!T->getPointeeType().isNull())
 T = T->getPointeeType();
@@ -6080,8 +6101,13 @@
 if (!TooManyArguments(FP->getNumParams(),
   ArgsWithoutDependentTypes.size(),
   /*PartialOverloading=*/true) ||
-FP->isVariadic())
-  Results.push_back(ResultCandidate(FP));
+FP->isVariadic()) {
+  if (!P.isNull()) {
+Results.push_back(ResultCandidate(P));
+  } else {
+Results.push_back(ResultCandidate(FP));
+  }
+}
   } else if (auto FT = T->getAs())
 // No prototype and declaration, it may be a K & R style function.
 Results.push_back(ResultCandidate(FT));
Index: clang/lib/Sema/CodeCompleteConsumer.cpp
===
--- clang/lib/Sema/CodeCompleteConsumer.cpp
+++ clang/lib/Sema/CodeCompleteConsumer.cpp
@@ -506,7 +506,8 @@
 
   case CK_FunctionType:
 return Type;
-
+  case CK_FunctionProtoTypeLoc:
+return ProtoTypeLoc.getTypePtr();
   case CK_Template:
   case CK_Aggregate:
 return nullptr;
@@ -515,6 +516,12 @@
   llvm_unreachable("Invalid CandidateKind!");
 }
 
+ const FunctionProtoTypeLoc CodeCompleteConsumer::OverloadCandidate::getFunctionProtoTypeLoc() const {
+   if (Kind == 

[PATCH] D124790: [HLSL] Enable half type for hlsl.

2022-05-06 Thread Xiang Li via Phabricator via cfe-commits
python3kgae updated this revision to Diff 427711.
python3kgae added a comment.

Rebase for fcgl change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124790

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/LangOptions.cpp
  clang/lib/Basic/Targets/DirectX.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/HLSL.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGenHLSL/half.hlsl

Index: clang/test/CodeGenHLSL/half.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/half.hlsl
@@ -0,0 +1,15 @@
+// RUN: %clang_dxc -Tlib_6_7 -fcgl  -Fo - %s | FileCheck %s --check-prefix=FLOAT
+// RUN: %clang_dxc -Tlib_6_7 -enable-16bit-types -fcgl  -Fo - %s | FileCheck %s --check-prefix=HALF
+
+// Make sure use float when not enable-16bit-types.
+// FLOAT:define {{.*}}float @_Z3fooff(float{{[^,]+}}, float{{[^,)]+}})
+// FLOAT-NOT:half
+// FLOAT:ret float %
+
+// Make sure use half when enable-16bit-types.
+// HALF:define {{.*}}half @_Z3fooDhDh(half{{[^,]+}}, half{{[^,)]+}})
+// HALF-NOT:float
+// HALF:ret half %
+half foo(half a, half b) {
+  return a+b;
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1508,7 +1508,13 @@
 << "_Float16";
 Result = Context.Float16Ty;
 break;
-  case DeclSpec::TST_half:Result = Context.HalfTy; break;
+  case DeclSpec::TST_half:
+// For HLSL, when not enable native half type, half will be treat as float.
+if (S.getLangOpts().HLSL && !S.getLangOpts().NativeHalfType)
+  Result = Context.FloatTy;
+else
+  Result = Context.HalfTy;
+break;
   case DeclSpec::TST_BFloat16:
 if (!S.Context.getTargetInfo().hasBFloat16Type())
   S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported)
Index: clang/lib/Driver/ToolChains/HLSL.cpp
===
--- clang/lib/Driver/ToolChains/HLSL.cpp
+++ clang/lib/Driver/ToolChains/HLSL.cpp
@@ -188,5 +188,7 @@
 Opts.getOption(options::OPT_dxil_validator_version),
 DefaultValidatorVer);
   }
+  // FIXME: add validation for enable_16bit_types should be after HLSL 2018 and
+  // shader model 6.2.
   return DAL;
 }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3473,11 +3473,13 @@
   types::ID InputType) {
   const unsigned ForwardedArguments[] = {options::OPT_dxil_validator_version,
  options::OPT_S, options::OPT_emit_llvm,
- options::OPT_disable_llvm_passes};
+ options::OPT_disable_llvm_passes,
+ options::OPT_fnative_half_type};
 
   for (const auto  : ForwardedArguments)
 if (const auto *A = Args.getLastArg(Arg))
   A->renderAsInput(Args, CmdArgs);
+  CmdArgs.push_back("-fallow-half-arguments-and-returns");
 }
 
 static void RenderARCMigrateToolOptions(const Driver , const ArgList ,
Index: clang/lib/Basic/Targets/DirectX.h
===
--- clang/lib/Basic/Targets/DirectX.h
+++ clang/lib/Basic/Targets/DirectX.h
@@ -58,7 +58,7 @@
 resetDataLayout("e-m:e-p:32:32-i1:32-i8:8-i16:16-i32:32-i64:64-f16:16-f32:"
 "32-f64:64-n8:16:32:64");
   }
-
+  bool useFP16ConversionIntrinsics() const override { return false; }
   void getTargetDefines(const LangOptions ,
 MacroBuilder ) const override;
 
Index: clang/lib/Basic/LangOptions.cpp
===
--- clang/lib/Basic/LangOptions.cpp
+++ clang/lib/Basic/LangOptions.cpp
@@ -193,8 +193,8 @@
   // OpenCL, C++ and C2x have bool, true, false keywords.
   Opts.Bool = Opts.OpenCL || Opts.CPlusPlus || Opts.C2x;
 
-  // OpenCL has half keyword
-  Opts.Half = Opts.OpenCL;
+  // OpenCL and HLSL have half keyword
+  Opts.Half = Opts.OpenCL || Opts.HLSL;
 }
 
 FPOptions FPOptions::defaultWithoutTrailingStorage(const LangOptions ) {
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6762,3 +6762,6 @@
   HelpText<"Emit pristine LLVM IR from the frontend by not running any LLVM passes at all."
"Same as -S + -emit-llvm + -disable-llvm-passes.">;
 def fcgl : DXCFlag<"fcgl">, Alias;
+def enable_16bit_types : DXCFlag<"enable-16bit-types">, Alias,
+  HelpText<"Enable 16-bit types and disable min precision types."
+   "Available in HLSL 

[clang-tools-extra] ec34de1 - [clang-tidy][NFC] Fix doc typo for bugprone-unchecked-optional-access

2022-05-06 Thread Yitzhak Mandelbaum via cfe-commits

Author: Yitzhak Mandelbaum
Date: 2022-05-06T19:23:43Z
New Revision: ec34de1bfe5501fd3017ff867bc2481dc052788d

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

LOG: [clang-tidy][NFC] Fix doc typo for bugprone-unchecked-optional-access

Added: 


Modified: 

clang-tools-extra/docs/clang-tidy/checks/bugprone-unchecked-optional-access.rst

Removed: 




diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/bugprone-unchecked-optional-access.rst
 
b/clang-tools-extra/docs/clang-tidy/checks/bugprone-unchecked-optional-access.rst
index 40ca8eeb7ff8..56ce90c726b2 100644
--- 
a/clang-tools-extra/docs/clang-tidy/checks/bugprone-unchecked-optional-access.rst
+++ 
b/clang-tools-extra/docs/clang-tidy/checks/bugprone-unchecked-optional-access.rst
@@ -81,6 +81,8 @@ Rely on invariants of uncommon APIs
 
 The check is unaware of invariants of uncommon APIs. For example:
 
+.. code-block:: c++
+
void f(Foo foo) {
  if (foo.HasProperty("bar")) {
use(*foo.GetProperty("bar")); // unsafe: it is unclear whether 
`foo.GetProperty("bar")` has a value.



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


[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

2022-05-06 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: clang/lib/CodeGen/CGCUDARuntime.h:56-70
+enum OffloadRegionEntryKindFlag : uint32_t {
+  /// Mark the region entry as a kernel.
+  OffloadRegionKernelEntry = 0x0,
+};
+
+/// The kind flag of the global variable entry.
+enum OffloadVarEntryKindFlag : uint32_t {

We can also fold both enums into one, as we still have the ambiguity of what 
`flags=0` means. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123471

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


[PATCH] D124866: [CUDA][HIP] support __noinline__ as keyword

2022-05-06 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

> CUDA/HIP do not have language spec.

Well. It's not completely true. CUDA programming guide does serve as the 
de-facto spec for CUDA. It's far from perfect, but it does mention 
`__noinline__` and `__forceinline__` as function qualifiers: 
https://docs.nvidia.com/cuda/cuda-c-programming-guide/index.html#noinline-and-forceinline


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

https://reviews.llvm.org/D124866

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


[PATCH] D124702: [MSVC] Add support for pragma function

2022-05-06 Thread Stephen Long via Phabricator via cfe-commits
steplong added inline comments.



Comment at: clang/lib/Sema/SemaAttr.cpp:1079
+  MSFunctionNoBuiltins.insert(MSFunctionNoBuiltins.end(),
+  NoBuiltins.begin(), NoBuiltins.end());
+}

hans wrote:
> steplong wrote:
> > hans wrote:
> > > Do we want to avoid duplicates in MSFunctionNoBuiltins? Or maybe it 
> > > doesn't matter?
> > Yea, I didn't think it really mattered. I originally wanted to use a set, 
> > but I needed the strings to be stored in contiguous memory for 
> > NoBuitinAttr::CreateImplicit() in Sema::AddRangeBasedNoBuiltin()
> A set would be nicer conceptually of course.
> 
> How about using an llvm::SmallSetVector. That will solve the problem of 
> duplicates, it will be deterministic, and you can use getArrayRef() to get 
> the values in contiguous memory, or maybe being() and end() will work too.
Hmm do you know where I can find an Allocator? getArrayRef().data() returns a 
pointer to a constant StringRef, but CreateImplicit takes a pointer to a 
StringRef.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124702

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


[PATCH] D121120: [clang-tidy] New check for safe usage of `std::optional` and like types.

2022-05-06 Thread 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 rG7e63a0d479dd: [clang-tidy] New check for safe usage of 
`std::optional` and like types. (authored by ymandel).

Changed prior to commit:
  https://reviews.llvm.org/D121120?vs=427610=427706#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121120

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-unchecked-optional-access.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/types/optional.h
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-unchecked-optional-access.cpp
  
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp

Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -13,6 +13,7 @@
 
 #include "clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/Stmt.h"
@@ -32,10 +33,9 @@
 namespace {
 
 using namespace ::clang::ast_matchers;
-
 using LatticeTransferState = TransferState;
 
-auto optionalClass() {
+DeclarationMatcher optionalClass() {
   return classTemplateSpecializationDecl(
   anyOf(hasName("std::optional"), hasName("std::__optional_storage_base"),
 hasName("__optional_destruct_base"), hasName("absl::optional"),
@@ -230,6 +230,8 @@
   }
 
   // Record that this unwrap is *not* provably safe.
+  // FIXME: include either the name of the optional (if applicable) or a source
+  // range of the access for easier interpretation of the result.
   State.Lattice.getSourceLocations().insert(ObjectExpr->getBeginLoc());
 }
 
@@ -550,6 +552,11 @@
 
 } // namespace
 
+ast_matchers::DeclarationMatcher
+UncheckedOptionalAccessModel::optionalClassDecl() {
+  return optionalClass();
+}
+
 UncheckedOptionalAccessModel::UncheckedOptionalAccessModel(
 ASTContext , UncheckedOptionalAccessModelOptions Options)
 : DataflowAnalysis(
Index: clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
===
--- clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
+++ clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
@@ -49,6 +49,9 @@
   UncheckedOptionalAccessModel(
   ASTContext , UncheckedOptionalAccessModelOptions Options = {});
 
+  /// Returns a matcher for the optional classes covered by this model.
+  static ast_matchers::DeclarationMatcher optionalClassDecl();
+
   static SourceLocationsLattice initialElement() {
 return SourceLocationsLattice();
   }
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unchecked-optional-access.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unchecked-optional-access.cpp
@@ -0,0 +1,112 @@
+// RUN: %check_clang_tidy %s bugprone-unchecked-optional-access %t -- -- -I %S/Inputs/
+
+#include "absl/types/optional.h"
+
+void unchecked_value_access(const absl::optional ) {
+  opt.value();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access]
+}
+
+void unchecked_deref_operator_access(const absl::optional ) {
+  *opt;
+  // CHECK-MESSAGES: :[[@LINE-1]]:4: warning: unchecked access to optional value
+}
+
+struct Foo {
+  void foo() const {}
+};
+
+void unchecked_arrow_operator_access(const absl::optional ) {
+  opt->foo();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value
+}
+
+void checked_access(const absl::optional ) {
+  if (opt.has_value()) {
+opt.value();
+  }
+}
+
+template 
+void function_template_without_user(const absl::optional ) {
+  opt.value(); // no-warning
+}
+
+template 
+void function_template_with_user(const absl::optional ) {
+  opt.value();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value
+}
+
+void function_template_user(const absl::optional ) {
+  // Instantiate the f3 function template so that it gets matched by the check.
+  

[clang] 7e63a0d - [clang-tidy] New check for safe usage of `std::optional` and like types.

2022-05-06 Thread Yitzhak Mandelbaum via cfe-commits

Author: Yitzhak Mandelbaum
Date: 2022-05-06T18:50:36Z
New Revision: 7e63a0d479dd3ccce20de5cddb0f138b537c08bb

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

LOG: [clang-tidy] New check for safe usage of `std::optional` and like types.

This check verifies the safety of access to `std::optional` and related
types (including `absl::optional`). It is based on a corresponding Clang
Dataflow Analysis, which does most of the work. This check merely runs it and
converts its findings into diagnostics.

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

Added: 
clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.h

clang-tools-extra/docs/clang-tidy/checks/bugprone-unchecked-optional-access.rst
clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/types/optional.h

clang-tools-extra/test/clang-tidy/checkers/bugprone-unchecked-optional-access.cpp

Modified: 
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst

clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp 
b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 7b1a2c14ed3c1..2d8bfffc01268 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -63,6 +63,7 @@
 #include "TerminatingContinueCheck.h"
 #include "ThrowKeywordMissingCheck.h"
 #include "TooSmallLoopVariableCheck.h"
+#include "UncheckedOptionalAccessCheck.h"
 #include "UndefinedMemoryManipulationCheck.h"
 #include "UndelegatedConstructorCheck.h"
 #include "UnhandledExceptionAtNewCheck.h"
@@ -185,6 +186,8 @@ class BugproneModule : public ClangTidyModule {
 "bugprone-throw-keyword-missing");
 CheckFactories.registerCheck(
 "bugprone-too-small-loop-variable");
+CheckFactories.registerCheck(
+"bugprone-unchecked-optional-access");
 CheckFactories.registerCheck(
 "bugprone-undefined-memory-manipulation");
 CheckFactories.registerCheck(

diff  --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index b33072ddf01b8..50f6862bc2f44 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -59,6 +59,7 @@ add_clang_library(clangTidyBugproneModule
   TerminatingContinueCheck.cpp
   ThrowKeywordMissingCheck.cpp
   TooSmallLoopVariableCheck.cpp
+  UncheckedOptionalAccessCheck.cpp
   UndefinedMemoryManipulationCheck.cpp
   UndelegatedConstructorCheck.cpp
   UnhandledExceptionAtNewCheck.cpp
@@ -80,6 +81,8 @@ add_clang_library(clangTidyBugproneModule
 clang_target_link_libraries(clangTidyBugproneModule
   PRIVATE
   clangAnalysis
+  clangAnalysisFlowSensitive
+  clangAnalysisFlowSensitiveModels
   clangAST
   clangASTMatchers
   clangBasic

diff  --git 
a/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
new file mode 100644
index 0..ab6b172014324
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
@@ -0,0 +1,108 @@
+//===--- UncheckedOptionalAccessCheck.cpp - clang-tidy 
===//
+//
+// 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
+//
+//===--===//
+
+#include "UncheckedOptionalAccessCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
+#include "clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h"
+#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h"
+#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/Any.h"
+#include 

[PATCH] D124658: [analyzer] Canonicalize SymIntExpr so the RHS is positive when possible

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

@uabelho Could you please apply this change and check if it resolves your 
crashes?
I'm gonna do the same on our side.


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

https://reviews.llvm.org/D124658

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


[PATCH] D123685: [clang][ASTImporter] Add isNewDecl

2022-05-06 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.

Thanks, this looks good to me!




Comment at: clang/include/clang/AST/ASTImporterSharedState.h:43
+  /// Set of the newly created declarations.
+  llvm::DenseSet NewDecls;
+

martong wrote:
> xazax.hun wrote:
> > ASTImporter already has something like `ImportedFromDecls`. Is that not 
> > sufficient to check if a declaration is new?
> > 
> > Is it possible that we may want the "degree" of the imported definition? 
> > I.e., how many hops did we do to import it (is it imported as a result of 
> > evaluating an imported call?).
> What we need here is a list of those declarations that have been **created** 
> and linked into the destination TU by the ASTImporter.
> 
> `ImportedFromDecls` is a mapping of declarations from the "source" context to 
> the "destination" context. It might happen that we do not create a new 
> declaration during the import, however, the mapping is still updated, so next 
> time we can just simply get that from there (it's a cache).
> 
> E.g. during the import of `foo` from `b.cpp` to `a.cpp` we are going to 
> import `X` as well. But during the import of `X` we find the existing 
> definition and then we just simply update the mapping to that. This happens 
> all the time when we include the same header to two different translation 
> units.
> ```
> // a.cpp
> struct X {};
> // b.cpp
> struct X {};
> void foo(X);
> ```
> 
> > Is it possible that we may want the "degree" of the imported definition? 
> > I.e., how many hops did we do to import it
> I am not sure how that would be useful, I mean how and for what could we use 
> that information?
> I am not sure how that would be useful, I mean how and for what could we use 
> that information?

It could be part of the heuristics whether we want something inlined during the 
analysis, but we might not need this information at all. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123685

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


[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

2022-05-06 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 427701.
jhuber6 added a comment.

Changing enum values from a bitfield to simple enumeration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123471

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CGCUDARuntime.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGenCUDA/offloading-entries.cu

Index: clang/test/CodeGenCUDA/offloading-entries.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/offloading-entries.cu
@@ -0,0 +1,33 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --check-globals
+// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu \
+// RUN:   --offload-new-driver -emit-llvm -o - -x cuda  %s | FileCheck \
+// RUN:   --check-prefix=HOST %s
+
+#include "Inputs/cuda.h"
+
+//.
+// HOST: @x = internal global i32 undef, align 4
+// HOST: @.omp_offloading.entry_name = internal unnamed_addr constant [8 x i8] c"_Z3foov\00"
+// HOST: @.omp_offloading.entry._Z3foov = weak constant %struct.__tgt_offload_entry { ptr @_Z18__device_stub__foov, ptr @.omp_offloading.entry_name, i64 0, i32 0, i32 0 }, section "cuda_offloading_entries", align 1
+// HOST: @.omp_offloading.entry_name.1 = internal unnamed_addr constant [8 x i8] c"_Z3barv\00"
+// HOST: @.omp_offloading.entry._Z3barv = weak constant %struct.__tgt_offload_entry { ptr @_Z18__device_stub__barv, ptr @.omp_offloading.entry_name.1, i64 0, i32 0, i32 0 }, section "cuda_offloading_entries", align 1
+// HOST: @.omp_offloading.entry_name.2 = internal unnamed_addr constant [2 x i8] c"x\00"
+// HOST: @.omp_offloading.entry.x = weak constant %struct.__tgt_offload_entry { ptr @x, ptr @.omp_offloading.entry_name.2, i64 4, i32 0, i32 0 }, section "cuda_offloading_entries", align 1
+//.
+// HOST-LABEL: @_Z18__device_stub__foov(
+// HOST-NEXT:  entry:
+// HOST-NEXT:[[TMP0:%.*]] = call i32 @cudaLaunch(ptr @_Z18__device_stub__foov)
+// HOST-NEXT:br label [[SETUP_END:%.*]]
+// HOST:   setup.end:
+// HOST-NEXT:ret void
+//
+__global__ void foo() {}
+// HOST-LABEL: @_Z18__device_stub__barv(
+// HOST-NEXT:  entry:
+// HOST-NEXT:[[TMP0:%.*]] = call i32 @cudaLaunch(ptr @_Z18__device_stub__barv)
+// HOST-NEXT:br label [[SETUP_END:%.*]]
+// HOST:   setup.end:
+// HOST-NEXT:ret void
+//
+__global__ void bar() {}
+__device__ int x = 1;
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6082,6 +6082,10 @@
options::OPT_fno_openmp_extensions);
   }
 
+  // Forward the new driver to change offloading code generation.
+  if (Args.hasArg(options::OPT_offload_new_driver))
+CmdArgs.push_back("--offload-new-driver");
+
   SanitizeArgs.addArgs(TC, Args, CmdArgs, InputType);
 
   const XRayArgs  = TC.getXRayArgs();
Index: clang/lib/CodeGen/CGCUDARuntime.h
===
--- clang/lib/CodeGen/CGCUDARuntime.h
+++ clang/lib/CodeGen/CGCUDARuntime.h
@@ -52,6 +52,24 @@
   Texture,  // Builtin texture
 };
 
+/// The kind flag of the target region entry.
+enum OffloadRegionEntryKindFlag : uint32_t {
+  /// Mark the region entry as a kernel.
+  OffloadRegionKernelEntry = 0x0,
+};
+
+/// The kind flag of the global variable entry.
+enum OffloadVarEntryKindFlag : uint32_t {
+  /// Mark the entry as a global variable.
+  OffloadGlobalVarEntry = 0x0,
+  /// Mark the entry as a managed global variable.
+  OffloadGlobalManagedEntry = 0x1,
+  /// Mark the entry as a surface variable.
+  OffloadGlobalSurfaceEntry = 0x2,
+  /// Mark the entry as a texture variable.
+  OffloadGlobalTextureEntry = 0x3,
+};
+
   private:
 unsigned Kind : 2;
 unsigned Extern : 1;
Index: clang/lib/CodeGen/CGCUDANV.cpp
===
--- clang/lib/CodeGen/CGCUDANV.cpp
+++ clang/lib/CodeGen/CGCUDANV.cpp
@@ -157,6 +157,8 @@
   llvm::Function *makeModuleDtorFunction();
   /// Transform managed variables for device compilation.
   void transformManagedVars();
+  /// Create offloading entries to register globals in RDC mode.
+  void createOffloadingEntries();
 
 public:
   CGNVCUDARuntime(CodeGenModule );
@@ -210,7 +212,8 @@
 CGNVCUDARuntime::CGNVCUDARuntime(CodeGenModule )
 : CGCUDARuntime(CGM), Context(CGM.getLLVMContext()),
   TheModule(CGM.getModule()),
-  RelocatableDeviceCode(CGM.getLangOpts().GPURelocatableDeviceCode),
+  RelocatableDeviceCode(CGM.getLangOpts().GPURelocatableDeviceCode ||
+CGM.getLangOpts().OffloadingNewDriver),
   

[PATCH] D124866: [CUDA][HIP] support __noinline__ as keyword

2022-05-06 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 427700.
yaxunl marked an inline comment as done.
yaxunl added a comment.

added release note and documentation


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

https://reviews.llvm.org/D124866

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/test/CodeGenCUDA/noinline.cu
  clang/test/Lexer/has_extension.cu
  clang/test/SemaCUDA/noinline.cu

Index: clang/test/SemaCUDA/noinline.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/noinline.cu
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=cuda %s
+// RUN: %clang_cc1 -fsyntax-only -verify=pedantic -pedantic %s
+// RUN: %clang_cc1 -fsyntax-only -verify=cpp -x c++ %s
+
+// cuda-no-diagnostics
+
+__noinline__ void fun1() { } // cpp-error {{unknown type name '__noinline__'}}
+// pedantic-warning@-1 {{__noinline__ keyword is a Clang extension for CUDA/HIP}}
+
+__attribute__((noinline)) void fun2() { }
+__attribute__((__noinline__)) void fun3() { }
+[[gnu::__noinline__]] void fun4() { }
Index: clang/test/Lexer/has_extension.cu
===
--- /dev/null
+++ clang/test/Lexer/has_extension.cu
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -E -triple x86_64-linux-gnu %s -o - | FileCheck %s
+
+// CHECK: has_noinline_keyword
+#if __has_extension(cuda_noinline_keyword)
+int has_noinline_keyword();
+#else
+int no_noinine_keyword();
+#endif
Index: clang/test/CodeGenCUDA/noinline.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/noinline.cu
@@ -0,0 +1,30 @@
+// optimization is needed, otherwise by default all functions have noinline.
+
+// RUN: %clang_cc1 -triple nvptx-nvidia-cuda -fcuda-is-device \
+// RUN: -O2 -emit-llvm -o - %s | FileCheck %s
+
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN: -O2 -emit-llvm -o - -x hip %s | FileCheck %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-gnu-linux \
+// RUN: -O2 -emit-llvm -o - %s | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+__noinline__ __device__ __host__ void fun1() {}
+
+__attribute__((noinline)) __device__ __host__ void fun2() {}
+
+__attribute__((__noinline__)) __device__ __host__ void fun3() {}
+
+[[gnu::__noinline__]] __device__ __host__ void fun4() {}
+
+__device__ __host__ void fun5() {}
+
+// CHECK: define{{.*}}@_Z4fun1v{{.*}}#[[ATTR1:[0-9]*]]
+// CHECK: define{{.*}}@_Z4fun2v{{.*}}#[[ATTR1:[0-9]*]]
+// CHECK: define{{.*}}@_Z4fun3v{{.*}}#[[ATTR1:[0-9]*]]
+// CHECK: define{{.*}}@_Z4fun4v{{.*}}#[[ATTR1:[0-9]*]]
+// CHECK: define{{.*}}@_Z4fun5v{{.*}}#[[ATTR2:[0-9]*]]
+// CHECK: attributes #[[ATTR1]] = {{.*}}noinline
+// CHECK-NOT: attributes #[[ATTR2]] = {{.*}}noinline
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -897,6 +897,18 @@
   }
 }
 
+void Parser::ParseCUDAFunctionAttributes(ParsedAttributes ) {
+  while (Tok.is(tok::kw___noinline__)) {
+IdentifierInfo *AttrName = Tok.getIdentifierInfo();
+SourceLocation AttrNameLoc = ConsumeToken();
+attrs.addNew(AttrName, AttrNameLoc, nullptr, AttrNameLoc, nullptr, 0,
+ ParsedAttr::AS_Keyword);
+if (getLangOpts().CUDA) {
+  Diag(Tok, diag::ext_cuda_noinline_keyword);
+}
+  }
+}
+
 void Parser::ParseOpenCLQualifiers(ParsedAttributes ) {
   IdentifierInfo *AttrName = Tok.getIdentifierInfo();
   SourceLocation AttrNameLoc = Tok.getLocation();
@@ -3690,6 +3702,11 @@
   ParseOpenCLKernelAttributes(DS.getAttributes());
   continue;
 
+// CUDA/HIP single token adornments.
+case tok::kw___noinline__:
+  ParseCUDAFunctionAttributes(DS.getAttributes());
+  continue;
+
 // Nullability type specifiers.
 case tok::kw__Nonnull:
 case tok::kw__Nullable:
Index: clang/lib/Basic/IdentifierTable.cpp
===
--- clang/lib/Basic/IdentifierTable.cpp
+++ clang/lib/Basic/IdentifierTable.cpp
@@ -108,6 +108,7 @@
 KEYOPENCLCXX  = 0x40,
 KEYMSCOMPAT   = 0x80,
 KEYSYCL   = 0x100,
+KEYCUDA   = 0x200,
 KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20,
 KEYALL = (0x1ff & ~KEYNOMS18 &
   ~KEYNOOPENCL) // KEYNOMS18 and KEYNOOPENCL are used to exclude.
@@ -158,6 +159,8 @@
 return KS_Future;
   if (LangOpts.isSYCL() && (Flags & KEYSYCL))
 return KS_Enabled;
+  if (LangOpts.CUDA && (Flags & KEYCUDA))
+return KS_Enabled;
   return KS_Disabled;
 }
 
Index: clang/include/clang/Parse/Parser.h

[PATCH] D124866: [CUDA][HIP] support __noinline__ as keyword

2022-05-06 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked an inline comment as done.
yaxunl added inline comments.



Comment at: clang/include/clang/Basic/Features.def:274
+// CUDA/HIP Features
+FEATURE(cuda_noinline_keyword, true)
+

aaron.ballman wrote:
> yaxunl wrote:
> > aaron.ballman wrote:
> > > Do the CUDA or HIP specs define `__noinline__` as a keyword specifically? 
> > > If not, this isn't a `FEATURE`, it's an `EXTENSION` because it's specific 
> > > to Clang, not the language standard.
> > CUDA/HIP do not have language spec. In their programming guide, they do not 
> > define `__noinline__` as a keyword.
> > 
> > Will make it an extension.
> > CUDA/HIP do not have language spec. 
> 
> Then what body of people governs changes to the language? Basically, I'm 
> trying to understand whether this patch meets the community requirements for 
> adding an extension: https://clang.llvm.org/get_involved.html#criteria, 
> specifically #4 (though the rest of the points are worth keeping in mind). I 
> don't want to Clang ending up stepping on toes by defining this extension 
> only to accidentally frustrate the CUDA community.
specific to `__noinline__`, it is largely determined by the existing behaviour 
of CUDA SDK.

The CUDA SDK defines `__noinline__` as a macro `__attribute__((noinline))`. 
However, it is not compatible with some C++ headers which use 
`__attribute__((__noinline__))`.

This patch will not change the usage pattern of `__noinline__`. It is 
equivalent to the original behaviour with the benefit of being compatible with 
C++ headers.


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

https://reviews.llvm.org/D124866

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


[PATCH] D123685: [clang][ASTImporter] Add isNewDecl

2022-05-06 Thread Gabor Marton via Phabricator via cfe-commits
martong marked 4 inline comments as done.
martong added a comment.

@xazax.hun Hey Gabor, thanks a lot for your time and effort for reviewing this 
patch set! 
(And of course thank you for you too @steakhal, but you've already seen most of 
it.)

In D123685#3458329 , @steakhal wrote:

> What happens if the import fails?

Then `Import` returns with an `Error` object.

> Or alternatively the `InitializeImportedDecl(FromD, ToD)` fails?

That function can never fail, that is just setting a few bits:

  void InitializeImportedDecl(Decl *FromD, Decl *ToD) {
ToD->IdentifierNamespace = FromD->IdentifierNamespace;
if (FromD->isUsed())
  ToD->setIsUsed();
if (FromD->isImplicit())
  ToD->setImplicit();
  }




Comment at: clang/include/clang/AST/ASTImporterSharedState.h:43
+  /// Set of the newly created declarations.
+  llvm::DenseSet NewDecls;
+

xazax.hun wrote:
> ASTImporter already has something like `ImportedFromDecls`. Is that not 
> sufficient to check if a declaration is new?
> 
> Is it possible that we may want the "degree" of the imported definition? 
> I.e., how many hops did we do to import it (is it imported as a result of 
> evaluating an imported call?).
What we need here is a list of those declarations that have been **created** 
and linked into the destination TU by the ASTImporter.

`ImportedFromDecls` is a mapping of declarations from the "source" context to 
the "destination" context. It might happen that we do not create a new 
declaration during the import, however, the mapping is still updated, so next 
time we can just simply get that from there (it's a cache).

E.g. during the import of `foo` from `b.cpp` to `a.cpp` we are going to import 
`X` as well. But during the import of `X` we find the existing definition and 
then we just simply update the mapping to that. This happens all the time when 
we include the same header to two different translation units.
```
// a.cpp
struct X {};
// b.cpp
struct X {};
void foo(X);
```

> Is it possible that we may want the "degree" of the imported definition? 
> I.e., how many hops did we do to import it
I am not sure how that would be useful, I mean how and for what could we use 
that information?



Comment at: clang/include/clang/AST/ASTImporterSharedState.h:69-75
   llvm::Optional getImportDeclErrorIfAny(Decl *ToD) const {
 auto Pos = ImportErrors.find(ToD);
 if (Pos != ImportErrors.end())
   return Pos->second;
 else
   return Optional();
   }

steakhal wrote:
> Why does this API accept only non-const pointers?
This is legacy from the old times, internal AST functions all take non-const 
pointers.
However, I agree, we could extend the API with overloads that take a `const`, 
but that should be orthogonal to this change.



Comment at: clang/include/clang/AST/ASTImporterSharedState.h:81
+
+  bool isNewDecl(const Decl *ToD) const { return NewDecls.count(ToD); }
+

xazax.hun wrote:
> I assume this would only be applicable for definitions, so I wonder whether 
> `IsNewDefinition()` would be more descriptive. Or maybe 
> `IsImportedDefinition`? 
In the context of CTU, you are right, we are interested in definitions only. 
However, it would be over-complication to do that distinction at the 
`ASTImporter` level, I think.



Comment at: clang/lib/AST/ASTImporter.cpp:248
   Importer.RegisterImportedDecl(FromD, ToD);
+  Importer.SharedState->setNewDecl(ToD);
   InitializeImportedDecl(FromD, ToD);

xazax.hun wrote:
> Should this be part of `Importer.RegisterImportedDecl`?
No. `RegisterImportedDecl` is being overwritten in some of the descendant lldb 
classes. We want `setNewDecl` called no matter what.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123685

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


[PATCH] D124983: [HLSL] add -fcgl option flag.

2022-05-06 Thread Xiang Li 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 rG3fa5eb4cfc06: [HLSL] add -fcgl option flag. (authored by 
python3kgae).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124983

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/HLSL.cpp
  clang/test/Driver/dxc_fcgl.hlsl


Index: clang/test/Driver/dxc_fcgl.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_fcgl.hlsl
@@ -0,0 +1,5 @@
+// RUN: %clang_dxc -fcgl foo.hlsl -### %s 2>&1 | FileCheck %s
+
+// Make sure fcgl option flag which translated into "-S" "-emit-llvm" 
"-disable-llvm-passes".
+// CHECK:"-S" "-emit-llvm" "-disable-llvm-passes"
+
Index: clang/lib/Driver/ToolChains/HLSL.cpp
===
--- clang/lib/Driver/ToolChains/HLSL.cpp
+++ clang/lib/Driver/ToolChains/HLSL.cpp
@@ -169,6 +169,15 @@
   if (!isLegalValidatorVersion(ValVerStr, getDriver()))
 continue;
 }
+if (A->getOption().getID() == options::OPT_emit_pristine_llvm) {
+  // Translate fcgl into -S -emit-llvm and -disable-llvm-passes.
+  DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_S));
+  DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_emit_llvm));
+  DAL->AddFlagArg(nullptr,
+  Opts.getOption(options::OPT_disable_llvm_passes));
+  A->claim();
+  continue;
+}
 DAL->append(A);
   }
   // Add default validator version if not set.
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3471,7 +3471,9 @@
 
 static void RenderHLSLOptions(const ArgList , ArgStringList ,
   types::ID InputType) {
-  const unsigned ForwardedArguments[] = {options::OPT_dxil_validator_version};
+  const unsigned ForwardedArguments[] = {options::OPT_dxil_validator_version,
+ options::OPT_S, 
options::OPT_emit_llvm,
+ options::OPT_disable_llvm_passes};
 
   for (const auto  : ForwardedArguments)
 if (const auto *A = Args.getLastArg(Arg))
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6732,6 +6732,8 @@
 
 def dxc_Group : OptionGroup<"">, Flags<[DXCOption]>,
   HelpText<"dxc compatibility options">;
+class DXCFlag : Option<["/", "-"], name, KIND_FLAG>,
+  Group, Flags<[DXCOption, NoXarchOption]>;
 class DXCJoinedOrSeparate : Option<["/", "-"], name,
   KIND_JOINED_OR_SEPARATE>, Group, Flags<[DXCOption, 
NoXarchOption]>;
 
@@ -6756,3 +6758,7 @@
  "lib_6_3, lib_6_4, lib_6_5, lib_6_6, lib_6_7, lib_6_x,"
  "ms_6_5, ms_6_6, ms_6_7,"
  "as_6_5, as_6_6, as_6_7">;
+def emit_pristine_llvm : DXCFlag<"emit-pristine-llvm">,
+  HelpText<"Emit pristine LLVM IR from the frontend by not running any LLVM 
passes at all."
+   "Same as -S + -emit-llvm + -disable-llvm-passes.">;
+def fcgl : DXCFlag<"fcgl">, Alias;


Index: clang/test/Driver/dxc_fcgl.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_fcgl.hlsl
@@ -0,0 +1,5 @@
+// RUN: %clang_dxc -fcgl foo.hlsl -### %s 2>&1 | FileCheck %s
+
+// Make sure fcgl option flag which translated into "-S" "-emit-llvm" "-disable-llvm-passes".
+// CHECK:"-S" "-emit-llvm" "-disable-llvm-passes"
+
Index: clang/lib/Driver/ToolChains/HLSL.cpp
===
--- clang/lib/Driver/ToolChains/HLSL.cpp
+++ clang/lib/Driver/ToolChains/HLSL.cpp
@@ -169,6 +169,15 @@
   if (!isLegalValidatorVersion(ValVerStr, getDriver()))
 continue;
 }
+if (A->getOption().getID() == options::OPT_emit_pristine_llvm) {
+  // Translate fcgl into -S -emit-llvm and -disable-llvm-passes.
+  DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_S));
+  DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_emit_llvm));
+  DAL->AddFlagArg(nullptr,
+  Opts.getOption(options::OPT_disable_llvm_passes));
+  A->claim();
+  continue;
+}
 DAL->append(A);
   }
   // Add default validator version if not set.
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3471,7 +3471,9 @@
 
 static void RenderHLSLOptions(const ArgList , ArgStringList ,
   types::ID InputType) {
-  const unsigned ForwardedArguments[] = 

[clang] 3fa5eb4 - [HLSL] add -fcgl option flag.

2022-05-06 Thread via cfe-commits

Author: python3kgae
Date: 2022-05-06T11:42:15-07:00
New Revision: 3fa5eb4cfc065b686c03f912e4414fd00a54d04e

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

LOG: [HLSL] add -fcgl option flag.

fcgl option will make compilation stop after clang codeGen and output the llvm 
ir.
It is added to check clang codeGen output for HLSL.

It will be translated into -S -emit-llvm and -disable-llvm-passes.

Reviewed By: aaron.ballman

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

Added: 
clang/test/Driver/dxc_fcgl.hlsl

Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/HLSL.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index d9a4cb23a0934..ae0048f2a24e6 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6732,6 +6732,8 @@ def _SLASH_ZW : CLJoined<"ZW">;
 
 def dxc_Group : OptionGroup<"">, Flags<[DXCOption]>,
   HelpText<"dxc compatibility options">;
+class DXCFlag : Option<["/", "-"], name, KIND_FLAG>,
+  Group, Flags<[DXCOption, NoXarchOption]>;
 class DXCJoinedOrSeparate : Option<["/", "-"], name,
   KIND_JOINED_OR_SEPARATE>, Group, Flags<[DXCOption, 
NoXarchOption]>;
 
@@ -6756,3 +6758,7 @@ def target_profile : DXCJoinedOrSeparate<"T">, 
MetaVarName<"">,
  "lib_6_3, lib_6_4, lib_6_5, lib_6_6, lib_6_7, lib_6_x,"
  "ms_6_5, ms_6_6, ms_6_7,"
  "as_6_5, as_6_6, as_6_7">;
+def emit_pristine_llvm : DXCFlag<"emit-pristine-llvm">,
+  HelpText<"Emit pristine LLVM IR from the frontend by not running any LLVM 
passes at all."
+   "Same as -S + -emit-llvm + -disable-llvm-passes.">;
+def fcgl : DXCFlag<"fcgl">, Alias;

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index ebe51bcd3368a..33172d8042be9 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3471,7 +3471,9 @@ static void RenderOpenCLOptions(const ArgList , 
ArgStringList ,
 
 static void RenderHLSLOptions(const ArgList , ArgStringList ,
   types::ID InputType) {
-  const unsigned ForwardedArguments[] = {options::OPT_dxil_validator_version};
+  const unsigned ForwardedArguments[] = {options::OPT_dxil_validator_version,
+ options::OPT_S, 
options::OPT_emit_llvm,
+ options::OPT_disable_llvm_passes};
 
   for (const auto  : ForwardedArguments)
 if (const auto *A = Args.getLastArg(Arg))

diff  --git a/clang/lib/Driver/ToolChains/HLSL.cpp 
b/clang/lib/Driver/ToolChains/HLSL.cpp
index 2822e062fcd5c..b62395b168ea4 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -169,6 +169,15 @@ HLSLToolChain::TranslateArgs(const DerivedArgList , 
StringRef BoundArch,
   if (!isLegalValidatorVersion(ValVerStr, getDriver()))
 continue;
 }
+if (A->getOption().getID() == options::OPT_emit_pristine_llvm) {
+  // Translate fcgl into -S -emit-llvm and -disable-llvm-passes.
+  DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_S));
+  DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_emit_llvm));
+  DAL->AddFlagArg(nullptr,
+  Opts.getOption(options::OPT_disable_llvm_passes));
+  A->claim();
+  continue;
+}
 DAL->append(A);
   }
   // Add default validator version if not set.

diff  --git a/clang/test/Driver/dxc_fcgl.hlsl b/clang/test/Driver/dxc_fcgl.hlsl
new file mode 100644
index 0..d3eb2523199ca
--- /dev/null
+++ b/clang/test/Driver/dxc_fcgl.hlsl
@@ -0,0 +1,5 @@
+// RUN: %clang_dxc -fcgl foo.hlsl -### %s 2>&1 | FileCheck %s
+
+// Make sure fcgl option flag which translated into "-S" "-emit-llvm" 
"-disable-llvm-passes".
+// CHECK:"-S" "-emit-llvm" "-disable-llvm-passes"
+



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


[PATCH] D124658: [analyzer] Canonicalize SymIntExpr so the RHS is positive when possible

2022-05-06 Thread Tomasz Kamiński via Phabricator via cfe-commits
tomasz-kaminski-sonarsource updated this revision to Diff 427695.

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

https://reviews.llvm.org/D124658

Files:
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/additive-op-on-sym-int-expr.c

Index: clang/test/Analysis/additive-op-on-sym-int-expr.c
===
--- clang/test/Analysis/additive-op-on-sym-int-expr.c
+++ clang/test/Analysis/additive-op-on-sym-int-expr.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -triple=x86_64-unknown-linux -analyzer-checker=core,debug.ExprInspection -analyzer-config eagerly-assume=false -verify %s
+// RUN: %clang_analyze_cc1 -triple=x86_64-unknown-linux-gnu -analyzer-checker=core,apiModeling,debug.ExprInspection -analyzer-config eagerly-assume=false -verify %s
 
 void clang_analyzer_dump(int);
 void clang_analyzer_dumpL(long);
@@ -42,17 +42,127 @@
   clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) - 9U }} instead of + 4294967287U
 }
 
+const int intMin = 1 << (sizeof(int) * 8 - 1); // INT_MIN, negation value is not representable
+const long longMin = 1L << (sizeof(long) * 8 - 1); // LONG_MIN, negation value is not representable
+
 void testMin(int i, long l) {
   clang_analyzer_dump(i + (-1));  // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }} instead of + -1
   clang_analyzer_dump(i - (-1));  // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} instead of - -1
   clang_analyzer_dumpL(l + (-1)); // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }} instead of + -1
   clang_analyzer_dumpL(l - (-1)); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} instead of - -1
 
-  int intMin = 1 << (sizeof(int) * 8 - 1); // INT_MIN, negative value is not representable
   // Do not normalize representation if negation would not be representable
-  clang_analyzer_dump(i + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }}
-  clang_analyzer_dump(i - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - -2147483648 }}
+  clang_analyzer_dump(i + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }} no change
+  clang_analyzer_dump(i - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - -2147483648 }} no change
   // Produced value has higher bit with (long) so negation if representable
   clang_analyzer_dumpL(l + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648 }} instead of + -2147483648
   clang_analyzer_dumpL(l - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648 }} instead of - -2147483648
 }
+
+void changingToUnsinged(unsigned u, int i) {
+   unsigned c = u + (unsigned)i;
+   unsigned d = u - (unsigned)i;
+   if (i == -1) {
+ clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1U }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1U }}
+ return;
+   }
+   if (i == intMin) {
+ clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648U }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648U }}
+ return;
+   }
+}
+
+void extendingToSigned(long l, int i) {
+  long c = l + (long)i;
+  long d = l - (long)i;
+  if (i == -1) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }}
+return;
+  }
+  if (i == intMin) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648 }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648 }}
+return;
+  }
+}
+
+void extendingToUnigned(unsigned long ul, int i) {
+  unsigned long c = ul + (unsigned long)i;
+  unsigned long d = ul - (unsigned long)i;
+  if (i == -1) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1U }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1U }}
+return;
+  }
+  if (i == intMin) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648U }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648U }}
+return;
+  }
+}
+
+void truncatingToSigned(int i, long l) {
+  int c = i + (int)l;
+  int d = i - (int)l;
+  if (l == -1L) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }}\
+return;
+  }
+  if (l == (long)intMin) { // negation outside of range, no-changes
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }}
+return;
+  }
+  if (l == ((long)intMin - 1L)) { // outside or range, no changes
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483647 }}
+clang_analyzer_dump(d + 0); // 

[PATCH] D124658: [analyzer] Canonicalize SymIntExpr so the RHS is positive when possible

2022-05-06 Thread Tomasz Kamiński via Phabricator via cfe-commits
tomasz-kaminski-sonarsource updated this revision to Diff 427693.
tomasz-kaminski-sonarsource added a comment.

I have removed the assertions and updated the code to handle both extensions of 
reductions of bitwith from RHS to ResultType.
Expanded test, to cover the above scenarios.


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

https://reviews.llvm.org/D124658

Files:
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/additive-op-on-sym-int-expr.c

Index: clang/test/Analysis/additive-op-on-sym-int-expr.c
===
--- clang/test/Analysis/additive-op-on-sym-int-expr.c
+++ clang/test/Analysis/additive-op-on-sym-int-expr.c
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -triple=x86_64-unknown-linux -analyzer-checker=core,debug.ExprInspection -analyzer-config eagerly-assume=false -verify %s
+// RUN: %clang_analyze_cc1 -triple=x86_64-unknown-linux-gnu -analyzer-checker=core,apiModeling,debug.ExprInspection -analyzer-config eagerly-assume=false -verify %s
 
 void clang_analyzer_dump(int);
 void clang_analyzer_dumpL(long);
@@ -42,17 +42,127 @@
   clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) - 9U }} instead of + 4294967287U
 }
 
+const int intMin = 1 << (sizeof(int) * 8 - 1); // INT_MIN, negation value is not representable
+const long longMin = 1L << (sizeof(long) * 8 - 1); // LONG_MIN, negation value is not representable
+
 void testMin(int i, long l) {
   clang_analyzer_dump(i + (-1));  // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }} instead of + -1
   clang_analyzer_dump(i - (-1));  // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} instead of - -1
   clang_analyzer_dumpL(l + (-1)); // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }} instead of + -1
   clang_analyzer_dumpL(l - (-1)); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} instead of - -1
 
-  int intMin = 1 << (sizeof(int) * 8 - 1); // INT_MIN, negative value is not representable
   // Do not normalize representation if negation would not be representable
-  clang_analyzer_dump(i + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }}
-  clang_analyzer_dump(i - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - -2147483648 }}
+  clang_analyzer_dump(i + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }} no change
+  clang_analyzer_dump(i - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - -2147483648 }} no change
   // Produced value has higher bit with (long) so negation if representable
   clang_analyzer_dumpL(l + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648 }} instead of + -2147483648
   clang_analyzer_dumpL(l - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648 }} instead of - -2147483648
 }
+
+void changingToUnsinged(unsigned u, int i) {
+   unsigned c = u + (unsigned)i;
+   unsigned d = u - (unsigned)i;
+   if (i == -1) {
+ clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1U }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1U }}
+ return;
+   }
+   if (i == intMin) {
+ clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648U }}
+ clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648U }}
+ return;
+   }
+}
+
+void extendingToSigned(long l, int i) {
+  long c = l + (long)i;
+  long d = l - (long)i;
+  if (i == -1) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }}
+return;
+  }
+  if (i == intMin) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648 }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648 }}
+return;
+  }
+}
+
+void extendingToUnigned(unsigned long ul, int i) {
+  unsigned long c = ul + (unsigned long)i;
+  unsigned long d = ul - (unsigned long)i;
+  if (i == -1) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1U }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1U }}
+return;
+  }
+  if (i == intMin) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648U }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648U }}
+return;
+  }
+}
+
+void truncatingToSigned(int i, long l) {
+  int c = i + (int)l;
+  int d = i - (int)l;
+  if (l == -1L) {
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }}\
+return;
+  }
+  if (l == (long)intMin) { // negation outside of range, no-changes
+clang_analyzer_dump(c + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }}
+clang_analyzer_dump(d + 0); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 

[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

2022-05-06 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added inline comments.



Comment at: clang/lib/CodeGen/CGCUDARuntime.h:58-70
+  OffloadRegionKernelEntry = 0x0,
+};
+
+/// The kind flag of the global variable entry.
+enum OffloadVarEntryKindFlag : uint32_t {
+  /// Mark the entry as a global variable.
+  OffloadGlobalVarEntry = 0x0,

tra wrote:
> I'm a bit puzzled by this arrangement. Are those actually flags (i.e. can be 
> set independently) or are they enumerating specific offload kinds (i.e. only 
> one of these values is intended to be set)?
> 
> I think we want the latter. If that's the case I'd propose to enumerate 
> kernel and data together, so each kind gets a distinct value and is easy to 
> tell when one needs to examine the offload table manually. Right now both 
> kernels and global vars set the flags to 0.
> 
It probably should just be an enumeration. I was tentatively keeping them 
somewhat separate because OpenMP uses different values for these flags, but I 
think keeping this completely compatible is an impossible proposition. If we 
need them to use the same flag we should be able to configure that at some 
point. I will change it to just be a standard enum (I don't handle anything but 
kernels and regular globals in the linker wrapper right now anyway)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123471

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


[PATCH] D124966: Thread safety analysis: Handle compound assignment and ->* overloads

2022-05-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM! You should probably add a release note for this change so users know 
about it. (Do we need to update any documentation from this?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124966

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


[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

2022-05-06 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

In D123471#3497224 , @jhuber6 wrote:

> It's a little tough, I chose that format because it's exactly the same as we 
> use with OpenMP with a few different flags. I wish whoever initially designed 
> the struct made the ``reserved`` field 64-bits so it could conceivably hold a 
> pointer to some additional information, but that ship has sailed. I 
> originally chose to have this match the OpenMP struct because it will heavily 
> simplify things if every language uses this same method for registering their 
> globals. I would like to change it, but I'm not sure how well it would be 
> received considering backwards compatibility. I'm not sure what the best path 
> forward is on that front.

If it's exiting format that's already in use, then sticking with it is fine. 
We'll deal with this if/when we'll need to change it.




Comment at: clang/lib/CodeGen/CGCUDARuntime.h:58-70
+  OffloadRegionKernelEntry = 0x0,
+};
+
+/// The kind flag of the global variable entry.
+enum OffloadVarEntryKindFlag : uint32_t {
+  /// Mark the entry as a global variable.
+  OffloadGlobalVarEntry = 0x0,

I'm a bit puzzled by this arrangement. Are those actually flags (i.e. can be 
set independently) or are they enumerating specific offload kinds (i.e. only 
one of these values is intended to be set)?

I think we want the latter. If that's the case I'd propose to enumerate kernel 
and data together, so each kind gets a distinct value and is easy to tell when 
one needs to examine the offload table manually. Right now both kernels and 
global vars set the flags to 0.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123471

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


[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

2022-05-06 Thread Christopher Di Bella via Phabricator via cfe-commits
cjdb added a comment.

Thanks for working on this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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


[PATCH] D117829: [Clang] Add integer mul reduction builtin

2022-05-06 Thread Simon Pilgrim via Phabricator via cfe-commits
RKSimon added a comment.

ping @scanon


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117829

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


[PATCH] D124983: [HLSL] add -fcgl option flag.

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124983

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


[PATCH] D124866: [CUDA][HIP] support __noinline__ as keyword

2022-05-06 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Features.def:274
+// CUDA/HIP Features
+FEATURE(cuda_noinline_keyword, true)
+

yaxunl wrote:
> aaron.ballman wrote:
> > Do the CUDA or HIP specs define `__noinline__` as a keyword specifically? 
> > If not, this isn't a `FEATURE`, it's an `EXTENSION` because it's specific 
> > to Clang, not the language standard.
> CUDA/HIP do not have language spec. In their programming guide, they do not 
> define `__noinline__` as a keyword.
> 
> Will make it an extension.
> CUDA/HIP do not have language spec. 

Then what body of people governs changes to the language? Basically, I'm trying 
to understand whether this patch meets the community requirements for adding an 
extension: https://clang.llvm.org/get_involved.html#criteria, specifically #4 
(though the rest of the points are worth keeping in mind). I don't want to 
Clang ending up stepping on toes by defining this extension only to 
accidentally frustrate the CUDA community.


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

https://reviews.llvm.org/D124866

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


[PATCH] D124866: [CUDA][HIP] support __noinline__ as keyword

2022-05-06 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 427685.
yaxunl marked 4 inline comments as done.
yaxunl added a comment.

revised by Aaron's comments


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

https://reviews.llvm.org/D124866

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/lib/Basic/IdentifierTable.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/test/CodeGenCUDA/noinline.cu
  clang/test/Lexer/has_extension.cu
  clang/test/SemaCUDA/noinline.cu

Index: clang/test/SemaCUDA/noinline.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/noinline.cu
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=cuda %s
+// RUN: %clang_cc1 -fsyntax-only -verify=pedantic -pedantic %s
+// RUN: %clang_cc1 -fsyntax-only -verify=cpp -x c++ %s
+
+// cuda-no-diagnostics
+
+__noinline__ void fun1() { } // cpp-error {{unknown type name '__noinline__'}}
+// pedantic-warning@-1 {{__noinline__ keyword is a Clang extension for CUDA/HIP}}
+
+__attribute__((noinline)) void fun2() { }
+__attribute__((__noinline__)) void fun3() { }
+[[gnu::__noinline__]] void fun4() { }
Index: clang/test/Lexer/has_extension.cu
===
--- /dev/null
+++ clang/test/Lexer/has_extension.cu
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -E -triple x86_64-linux-gnu %s -o - | FileCheck %s
+
+// CHECK: has_noinline_keyword
+#if __has_extension(cuda_noinline_keyword)
+int has_noinline_keyword();
+#else
+int no_noinine_keyword();
+#endif
Index: clang/test/CodeGenCUDA/noinline.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/noinline.cu
@@ -0,0 +1,30 @@
+// optimization is needed, otherwise by default all functions have noinline.
+
+// RUN: %clang_cc1 -triple nvptx-nvidia-cuda -fcuda-is-device \
+// RUN: -O2 -emit-llvm -o - %s | FileCheck %s
+
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -fcuda-is-device \
+// RUN: -O2 -emit-llvm -o - -x hip %s | FileCheck %s
+
+// RUN: %clang_cc1 -triple x86_64-unknown-gnu-linux \
+// RUN: -O2 -emit-llvm -o - %s | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+__noinline__ __device__ __host__ void fun1() {}
+
+__attribute__((noinline)) __device__ __host__ void fun2() {}
+
+__attribute__((__noinline__)) __device__ __host__ void fun3() {}
+
+[[gnu::__noinline__]] __device__ __host__ void fun4() {}
+
+__device__ __host__ void fun5() {}
+
+// CHECK: define{{.*}}@_Z4fun1v{{.*}}#[[ATTR1:[0-9]*]]
+// CHECK: define{{.*}}@_Z4fun2v{{.*}}#[[ATTR1:[0-9]*]]
+// CHECK: define{{.*}}@_Z4fun3v{{.*}}#[[ATTR1:[0-9]*]]
+// CHECK: define{{.*}}@_Z4fun4v{{.*}}#[[ATTR1:[0-9]*]]
+// CHECK: define{{.*}}@_Z4fun5v{{.*}}#[[ATTR2:[0-9]*]]
+// CHECK: attributes #[[ATTR1]] = {{.*}}noinline
+// CHECK-NOT: attributes #[[ATTR2]] = {{.*}}noinline
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -897,6 +897,18 @@
   }
 }
 
+void Parser::ParseCUDAFunctionAttributes(ParsedAttributes ) {
+  while (Tok.is(tok::kw___noinline__)) {
+IdentifierInfo *AttrName = Tok.getIdentifierInfo();
+SourceLocation AttrNameLoc = ConsumeToken();
+attrs.addNew(AttrName, AttrNameLoc, nullptr, AttrNameLoc, nullptr, 0,
+ ParsedAttr::AS_Keyword);
+if (getLangOpts().CUDA) {
+  Diag(Tok, diag::ext_cuda_noinline_keyword);
+}
+  }
+}
+
 void Parser::ParseOpenCLQualifiers(ParsedAttributes ) {
   IdentifierInfo *AttrName = Tok.getIdentifierInfo();
   SourceLocation AttrNameLoc = Tok.getLocation();
@@ -3690,6 +3702,11 @@
   ParseOpenCLKernelAttributes(DS.getAttributes());
   continue;
 
+// CUDA/HIP single token adornments.
+case tok::kw___noinline__:
+  ParseCUDAFunctionAttributes(DS.getAttributes());
+  continue;
+
 // Nullability type specifiers.
 case tok::kw__Nonnull:
 case tok::kw__Nullable:
Index: clang/lib/Basic/IdentifierTable.cpp
===
--- clang/lib/Basic/IdentifierTable.cpp
+++ clang/lib/Basic/IdentifierTable.cpp
@@ -108,6 +108,7 @@
 KEYOPENCLCXX  = 0x40,
 KEYMSCOMPAT   = 0x80,
 KEYSYCL   = 0x100,
+KEYCUDA   = 0x200,
 KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20,
 KEYALL = (0x1ff & ~KEYNOMS18 &
   ~KEYNOOPENCL) // KEYNOMS18 and KEYNOOPENCL are used to exclude.
@@ -158,6 +159,8 @@
 return KS_Future;
   if (LangOpts.isSYCL() && (Flags & KEYSYCL))
 return KS_Enabled;
+  if (LangOpts.CUDA && (Flags & KEYCUDA))
+return KS_Enabled;
   return KS_Disabled;
 }
 
Index: clang/include/clang/Parse/Parser.h
===
--- 

[PATCH] D125103: [clangd] Speed up a slow sleeping testcase.

2022-05-06 Thread Sam McCall 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 rGc468635b7dfc: [clangd] Speed up a slow sleeping testcase. 
(authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125103

Files:
  clang-tools-extra/clangd/support/Threading.cpp
  clang-tools-extra/clangd/support/Threading.h
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -247,28 +247,31 @@
 }
 
 TEST_F(TUSchedulerTests, Debounce) {
-  std::atomic CallbackCount(0);
-  {
-auto Opts = optsForTest();
-Opts.UpdateDebounce = DebouncePolicy::fixed(std::chrono::seconds(1));
-TUScheduler S(CDB, Opts, captureDiags());
-// FIXME: we could probably use timeouts lower than 1 second here.
-auto Path = testPath("foo.cpp");
-updateWithDiags(S, Path, "auto (debounced)", WantDiagnostics::Auto,
-[&](std::vector) {
-  ADD_FAILURE()
-  << "auto should have been debounced and canceled";
-});
-std::this_thread::sleep_for(std::chrono::milliseconds(200));
-updateWithDiags(S, Path, "auto (timed out)", WantDiagnostics::Auto,
-[&](std::vector) { ++CallbackCount; });
-std::this_thread::sleep_for(std::chrono::seconds(2));
-updateWithDiags(S, Path, "auto (shut down)", WantDiagnostics::Auto,
-[&](std::vector) { ++CallbackCount; });
-
-ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
-  }
-  EXPECT_EQ(2, CallbackCount);
+  auto Opts = optsForTest();
+  Opts.UpdateDebounce = DebouncePolicy::fixed(std::chrono::milliseconds(500));
+  TUScheduler S(CDB, Opts, captureDiags());
+  auto Path = testPath("foo.cpp");
+  // Issue a write that's going to be debounced away.
+  updateWithDiags(S, Path, "auto (debounced)", WantDiagnostics::Auto,
+  [&](std::vector) {
+ADD_FAILURE()
+<< "auto should have been debounced and canceled";
+  });
+  // Sleep a bit to verify that it's really debounce that's holding diagnostics.
+  std::this_thread::sleep_for(std::chrono::milliseconds(50));
+
+  // Issue another write, this time we'll wait for its diagnostics.
+  Notification N;
+  updateWithDiags(S, Path, "auto (timed out)", WantDiagnostics::Auto,
+  [&](std::vector) { N.notify(); });
+  EXPECT_TRUE(N.wait(timeoutSeconds(1)));
+
+  // Once we start shutting down the TUScheduler, this one becomes a dead write.
+  updateWithDiags(S, Path, "auto (discarded)", WantDiagnostics::Auto,
+  [&](std::vector) {
+ADD_FAILURE()
+<< "auto should have been discarded (dead write)";
+  });
 }
 
 TEST_F(TUSchedulerTests, Cancellation) {
Index: clang-tools-extra/clangd/support/Threading.h
===
--- clang-tools-extra/clangd/support/Threading.h
+++ clang-tools-extra/clangd/support/Threading.h
@@ -24,20 +24,6 @@
 namespace clang {
 namespace clangd {
 
-/// A threadsafe flag that is initially clear.
-class Notification {
-public:
-  // Sets the flag. No-op if already set.
-  void notify();
-  // Blocks until flag is set.
-  void wait() const;
-
-private:
-  bool Notified = false;
-  mutable std::condition_variable CV;
-  mutable std::mutex Mu;
-};
-
 /// Limits the number of threads that can acquire the lock at the same time.
 class Semaphore {
 public:
@@ -100,6 +86,21 @@
   return true;
 }
 
+/// A threadsafe flag that is initially clear.
+class Notification {
+public:
+  // Sets the flag. No-op if already set.
+  void notify();
+  // Blocks until flag is set.
+  void wait() const { (void)wait(Deadline::infinity()); }
+  LLVM_NODISCARD bool wait(Deadline D) const;
+
+private:
+  bool Notified = false;
+  mutable std::condition_variable CV;
+  mutable std::mutex Mu;
+};
+
 /// Runs tasks on separate (detached) threads and wait for all tasks to finish.
 /// Objects that need to spawn threads can own an AsyncTaskRunner to ensure they
 /// all complete on destruction.
Index: clang-tools-extra/clangd/support/Threading.cpp
===
--- clang-tools-extra/clangd/support/Threading.cpp
+++ clang-tools-extra/clangd/support/Threading.cpp
@@ -34,9 +34,9 @@
   }
 }
 
-void Notification::wait() const {
+bool Notification::wait(Deadline D) const {
   std::unique_lock Lock(Mu);
-  CV.wait(Lock, [this] { return Notified; });
+  return clangd::wait(Lock, CV, D, [&] { return Notified; });
 }
 
 Semaphore::Semaphore(std::size_t MaxLocks) : 

[clang-tools-extra] c468635 - [clangd] Speed up a slow sleeping testcase.

2022-05-06 Thread Sam McCall via cfe-commits

Author: Sam McCall
Date: 2022-05-06T20:12:17+02:00
New Revision: c468635b7dfcec302f7688f5dcde91913524e355

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

LOG: [clangd] Speed up a slow sleeping testcase.

This testcase runs slowly due to 3.2s of sleeps = 2 + 1 + 0.2s.
After this patch it has 0.55s only.

Reduced by:
 - observed that the last test was bogus: we were sleeping until the queue was
   idle, effectively just a second copy of the first test. This avoids 1s sleep.
 - when waiting for debounce, sleep only until test passes, not for enough
   time to be safe (in practice was 2x debounce time, now 1x debounce time)
 - scaling delays down by a factor of 2 (note: factor of 10 caused bot failures)

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

Added: 


Modified: 
clang-tools-extra/clangd/support/Threading.cpp
clang-tools-extra/clangd/support/Threading.h
clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/support/Threading.cpp 
b/clang-tools-extra/clangd/support/Threading.cpp
index f7a511d62315..06d00c32d57d 100644
--- a/clang-tools-extra/clangd/support/Threading.cpp
+++ b/clang-tools-extra/clangd/support/Threading.cpp
@@ -34,9 +34,9 @@ void Notification::notify() {
   }
 }
 
-void Notification::wait() const {
+bool Notification::wait(Deadline D) const {
   std::unique_lock Lock(Mu);
-  CV.wait(Lock, [this] { return Notified; });
+  return clangd::wait(Lock, CV, D, [&] { return Notified; });
 }
 
 Semaphore::Semaphore(std::size_t MaxLocks) : FreeSlots(MaxLocks) {}

diff  --git a/clang-tools-extra/clangd/support/Threading.h 
b/clang-tools-extra/clangd/support/Threading.h
index 7f4ef6c0b1cb..7d5cd0dafba4 100644
--- a/clang-tools-extra/clangd/support/Threading.h
+++ b/clang-tools-extra/clangd/support/Threading.h
@@ -24,20 +24,6 @@
 namespace clang {
 namespace clangd {
 
-/// A threadsafe flag that is initially clear.
-class Notification {
-public:
-  // Sets the flag. No-op if already set.
-  void notify();
-  // Blocks until flag is set.
-  void wait() const;
-
-private:
-  bool Notified = false;
-  mutable std::condition_variable CV;
-  mutable std::mutex Mu;
-};
-
 /// Limits the number of threads that can acquire the lock at the same time.
 class Semaphore {
 public:
@@ -100,6 +86,21 @@ LLVM_NODISCARD bool wait(std::unique_lock ,
   return true;
 }
 
+/// A threadsafe flag that is initially clear.
+class Notification {
+public:
+  // Sets the flag. No-op if already set.
+  void notify();
+  // Blocks until flag is set.
+  void wait() const { (void)wait(Deadline::infinity()); }
+  LLVM_NODISCARD bool wait(Deadline D) const;
+
+private:
+  bool Notified = false;
+  mutable std::condition_variable CV;
+  mutable std::mutex Mu;
+};
+
 /// Runs tasks on separate (detached) threads and wait for all tasks to finish.
 /// Objects that need to spawn threads can own an AsyncTaskRunner to ensure 
they
 /// all complete on destruction.

diff  --git a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp 
b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
index 919f69c37840..76f4cbafc830 100644
--- a/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -247,28 +247,31 @@ TEST_F(TUSchedulerTests, WantDiagnostics) {
 }
 
 TEST_F(TUSchedulerTests, Debounce) {
-  std::atomic CallbackCount(0);
-  {
-auto Opts = optsForTest();
-Opts.UpdateDebounce = DebouncePolicy::fixed(std::chrono::seconds(1));
-TUScheduler S(CDB, Opts, captureDiags());
-// FIXME: we could probably use timeouts lower than 1 second here.
-auto Path = testPath("foo.cpp");
-updateWithDiags(S, Path, "auto (debounced)", WantDiagnostics::Auto,
-[&](std::vector) {
-  ADD_FAILURE()
-  << "auto should have been debounced and canceled";
-});
-std::this_thread::sleep_for(std::chrono::milliseconds(200));
-updateWithDiags(S, Path, "auto (timed out)", WantDiagnostics::Auto,
-[&](std::vector) { ++CallbackCount; });
-std::this_thread::sleep_for(std::chrono::seconds(2));
-updateWithDiags(S, Path, "auto (shut down)", WantDiagnostics::Auto,
-[&](std::vector) { ++CallbackCount; });
-
-ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
-  }
-  EXPECT_EQ(2, CallbackCount);
+  auto Opts = optsForTest();
+  Opts.UpdateDebounce = DebouncePolicy::fixed(std::chrono::milliseconds(500));
+  TUScheduler S(CDB, Opts, captureDiags());
+  auto Path = testPath("foo.cpp");
+  // Issue a write that's going to be debounced away.
+  updateWithDiags(S, Path, "auto (debounced)", WantDiagnostics::Auto,
+  [&](std::vector) {
+ 

[PATCH] D124866: [CUDA][HIP] support __noinline__ as keyword

2022-05-06 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked 4 inline comments as done.
yaxunl added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1778-1779
 def NoInline : DeclOrStmtAttr {
-  let Spellings = [GCC<"noinline">, CXX11<"clang", "noinline">,
+  let Spellings = [Keyword<"__noinline__">, GCC<"noinline">, CXX11<"clang", 
"noinline">,
C2x<"clang", "noinline">, Declspec<"noinline">];
   let Accessors = [Accessor<"isClangNoInline", [CXX11<"clang", "noinline">,

aaron.ballman wrote:
> 
will do



Comment at: clang/include/clang/Basic/Features.def:274
+// CUDA/HIP Features
+FEATURE(cuda_noinline_keyword, true)
+

aaron.ballman wrote:
> Do the CUDA or HIP specs define `__noinline__` as a keyword specifically? If 
> not, this isn't a `FEATURE`, it's an `EXTENSION` because it's specific to 
> Clang, not the language standard.
CUDA/HIP do not have language spec. In their programming guide, they do not 
define `__noinline__` as a keyword.

Will make it an extension.



Comment at: clang/lib/Parse/ParseDecl.cpp:902
+  while (Tok.is(tok::kw___noinline__)) {
+IdentifierInfo *AttrName = Tok.getIdentifierInfo();
+SourceLocation AttrNameLoc = ConsumeToken();

aaron.ballman wrote:
> I think we should we be issuing a pedantic "this is a clang extension" 
> warning here, WDYT?
will do



Comment at: clang/test/SemaCUDA/noinline.cu:8
+__attribute__((noinline)) void fun2() { }
+__attribute__((__noinline__)) void fun3() { }

aaron.ballman wrote:
> I think there should also be a test like:
> ```
> [[gnu::__noinline__]] void fun4() {}
> ```
> to verify that the double square bracket syntax also correctly handles this 
> being a keyword now (I expect the test to pass).
will do


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

https://reviews.llvm.org/D124866

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


[PATCH] D125103: [clangd] Speed up a slow sleeping testcase.

2022-05-06 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

In D125103#3497140 , @ilya-biryukov 
wrote:

> Many thanks for improving the running time, this slow test has bugged me 
> since the day I first ran it.
> I'd even be happy to remove it completely. It's slow, possibly flaky, so IMO 
> the corresponding check does not carry its weight.

I agree in general, unfortunately this test is really valuable:

- it tests something critical
- it tests something tricky
- it's the only test of that functionality
- it's prevented real bugs in the past
- I can't see how to replace it with reasonable effort
- it doesn't seem to be flaky at all in practice => hard to justify an 
unreasonable effort

So I want to keep it, and I promise to feel bad about it.
(I think this is the only ridiculous sleeping test we have in clangd...)




Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:272
+  [&](std::vector) {
+ADD_FAILURE()
+<< "auto should have been discarded (dead write)";

ilya-biryukov wrote:
> This is potentially a race in the test that we did not have before, right?
> There is probably much less work in the main test thread, but we probably can 
> still see the failure if we are unlucky with how the OS schedules us.
> 
> My stance is it's better to have no tests for certain behaviours than flaky 
> tests.
> But if you find it useful let's try to run it and see whether my suspicions 
> are unfounded, don't want to block the change on it.
Yes-ish - it's the same thing as the race on the first updateWithDiags. In both 
cases we're hoping that the next event happens within 450/500ms so the debounce 
doesn't expire.

AFAIK this hasn't been flaky at all in practice (albeit at 1000ms), so until we 
have a better test (see above) I'd like to keep it :-(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125103

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


[PATCH] D124382: [Clang] Recognize target address space in superset calculation

2022-05-06 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: clang/include/clang/AST/Type.h:508
+  static_cast(LangAS::FirstTargetAddressSpace));
+// When dealing with target AS return true if:
+// * A is equal to B, or

jchlanda wrote:
> tra wrote:
> > Is the code above intended to ensure that both A and B are target AS at 
> > this point?
> > If so, then it could be simplified to something like this:
> > ```
> > if (ASMap) {
> >   if (!isTargetAddressSpace(A))
> > A = getLangASFromTargetAS((*ASMap)[static_cast(A)]);
> >   if (!isTargetAddressSpace(B))
> > B = getLangASFromTargetAS((*ASMap)[static_cast(B)]);
> > 
> >   Generic = 
> > getLangASFromTargetAS((*ASMap)[static_cast(LangAS::opencl_generic)])
> >   Constant = 
> > getLangASFromTargetAS((*ASMap)[static_cast(LangAS::opencl_constant)]);
> > 
> >   // ... proceed inferring whether A is superset of B in target AS.
> >   return;
> > }
> > assert (isTargetAddressSpace(A) && isTargetAddressSpace(B));
> > ```
> Yes at the end of AS map accesses all address spaces have to be expressed in 
> therms of HW values, but I want it to happen only in the case of mixed AS 
> (Language and HW). I will add assert and use helpers, like you suggested in 
> the snippet, but would like to keep the `^` condition.
> would like to keep the ^ condition.

OK. Adding a comment explaining what's going on would be helpful here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124382

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


[PATCH] D125109: [clangd] Rewrite TweakTesting helpers to avoid reparsing the same code. NFC

2022-05-06 Thread Sam McCall via Phabricator via cfe-commits
sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: usaxena95, kadircet, arphaman.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang-tools-extra.

Previously the EXPECT_AVAILABLE macros would rebuild the code at each marked
point, by expanding the cases textually.
There were often lots, and it's nice to have lots!

This reduces total unittest time by ~10% on my machine.
I did have to sacrifice a little apply() coverage in AddUsingTests (was calling
expandCases directly, which was otherwise unused), but we have
EXPECT_AVAILABLE tests covering that, I don't think there's real risk here.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125109

Files:
  clang-tools-extra/clangd/unittests/tweaks/AddUsingTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
  clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h

Index: clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
===
--- clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
+++ clang-tools-extra/clangd/unittests/tweaks/TweakTesting.h
@@ -9,9 +9,11 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TWEAKS_TWEAKTESTING_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TWEAKS_TWEAKTESTING_H
 
+#include "ParsedAST.h"
 #include "index/Index.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
@@ -89,14 +91,13 @@
   std::string apply(llvm::StringRef MarkedCode,
 llvm::StringMap *EditedFiles = nullptr) const;
 
-  // Accepts a code snippet with many ranges (or points) marked, and returns a
-  // list of snippets with one range marked each.
-  // Primarily used from EXPECT_AVAILABLE/EXPECT_UNAVAILABLE macro.
-  static std::vector expandCases(llvm::StringRef MarkedCode);
-
-  // Returns a matcher that accepts marked code snippets where the tweak is
-  // available at the marked range.
-  ::testing::Matcher isAvailable() const;
+  // Helpers for EXPECT_AVAILABLE/EXPECT_UNAVAILABLE macros.
+  using WrappedAST = std::pair;
+  WrappedAST build(llvm::StringRef) const;
+  bool isAvailable(WrappedAST &, llvm::Annotations::Range) const;
+  // Return code re-decorated with a single point/range.
+  static std::string decorate(llvm::StringRef, unsigned);
+  static std::string decorate(llvm::StringRef, llvm::Annotations::Range);
 };
 
 MATCHER_P2(FileWithContents, FileName, Contents, "") {
@@ -109,18 +110,18 @@
 TweakID##Test() : TweakTest(#TweakID) {}   \
   }
 
-#define EXPECT_AVAILABLE(MarkedCode)   \
-  do { \
-for (const auto  : expandCases(MarkedCode))   \
-  EXPECT_THAT(Case, ::clang::clangd::TweakTest::isAvailable());\
-  } while (0)
-
-#define EXPECT_UNAVAILABLE(MarkedCode) \
+#define EXPECT_AVAILABLE_(MarkedCode, Available)   \
   do { \
-for (const auto  : expandCases(MarkedCode))   \
-  EXPECT_THAT(Case,\
-  ::testing::Not(::clang::clangd::TweakTest::isAvailable()));  \
+llvm::Annotations A{llvm::StringRef(MarkedCode)};  \
+auto AST = build(A.code());\
+assert(!A.points().empty() || !A.ranges().empty());\
+for (const auto  : A.points())   \
+  EXPECT_EQ(Available, isAvailable(AST, {P, P})) << decorate(A.code(), P); \
+for (const auto  : A.ranges())   \
+  EXPECT_EQ(Available, isAvailable(AST, R)) << decorate(A.code(), R);  \
   } while (0)
+#define EXPECT_AVAILABLE(MarkedCode) EXPECT_AVAILABLE_(MarkedCode, true)
+#define EXPECT_UNAVAILABLE(MarkedCode) EXPECT_AVAILABLE_(MarkedCode, false)
 
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/TweakTesting.cpp
@@ -8,7 +8,6 @@
 
 #include "TweakTesting.h"
 
-#include "Annotations.h"
 #include "SourceCode.h"
 #include "TestTU.h"
 #include "refactor/Tweak.h"
@@ -50,31 +49,25 @@
   return Outer;
 }
 
-std::pair rangeOrPoint(const Annotations ) {
-  Range SelectionRng;
+llvm::Annotations::Range rangeOrPoint(const llvm::Annotations ) {
   if 

[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

2022-05-06 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

In D123471#3497169 , @tra wrote:

>> Type struct __tgt_offload_entry {
>>
>>   void*addr;  // Pointer to the offload entry info.
>>   // (function or global)
>>   char*name;  // Name of the function or global.
>>   size_t  size;   // Size of the entry info (0 if it a function).
>>   int32_t flags;
>>   int32_t reserved;
>>
>> };
>
> One thing you need to consider is that this introduces a new ABI. 
> This structure may change over time and we will need to be able to deal with 
> libraries compiled with potentially different version of clang which may use 
> a different format for the entries.
> I think we may need some sort of version stamp.
> We could use the section name for this purpose and rename it when we change 
> the struct format, but that would be a bit more fragile as it's easier to 
> forget to update the name if/when the struct format changes.
> Also, format mismatch would looks like offload section is missing, which 
> would need special handling when we diagnose the problem to distinguish 
> incompatible offload table from the missing offload table.

It's a little tough, I chose that format because it's exactly the same as we 
use with OpenMP with a few different flags. I wish whoever initially designed 
the struct made the ``reserved`` field 64-bits so it could conceivably hold a 
pointer to some additional information, but that ship has sailed. I originally 
chose to have this match the OpenMP struct because it will heavily simplify 
things if every language uses this same method for registering their globals. I 
would like to change it, but I'm not sure how well it would be received 
considering backwards compatibility. I'm not sure what the best path forward is 
on that front.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123471

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


[PATCH] D124866: [CUDA][HIP] support __noinline__ as keyword

2022-05-06 Thread Artem Belevich via Phabricator via cfe-commits
tra added a reviewer: rsmith.
tra added a subscriber: rsmith.
tra added a comment.

> I don't know how language extensions come about in CUDA or HIP -- is there an 
> appropriate standards body (or something similar) that's aware of this 
> extension and supports it?

Summoning @rsmith for his language lawyer expertise.


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

https://reviews.llvm.org/D124866

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


[PATCH] D125050: [OpenMP] Try to Infer target triples using the offloading architecture

2022-05-06 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:789
+  (C.getInputArgs().hasArg(options::OPT_fopenmp_targets_EQ) ||
+   C.getInputArgs().hasArg(options::OPT_offload_arch_EQ));
+  if (IsOpenMPOffloading) {

yaxunl wrote:
> probably should be
> 
> C.getInputArgs().hasArg(options::OPT_offload_arch_EQ) && !IsHIP && !IsCUDA
> 
> otherwise a HIP program may get both HIP offloading and OpenMP offloading at 
> the same time. Currently it is not supported.
I do that below, line 815. The logic states that we only take 
`--offloading-arch` to imply OpenMP offloading if it's not currently CUDA or 
HIP. I'm pretty sure there's an existing test that covers this as it used to 
fail one of those HIP-interop tests until I added `!IsHIP && !IsCUDA` below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125050

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


[PATCH] D123471: [CUDA] Create offloading entries when using the new driver

2022-05-06 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

> Type struct __tgt_offload_entry {
>
>   void*addr;  // Pointer to the offload entry info.
>   // (function or global)
>   char*name;  // Name of the function or global.
>   size_t  size;   // Size of the entry info (0 if it a function).
>   int32_t flags;
>   int32_t reserved;
>
> };

One thing you need to consider is that this introduces a new ABI. 
This structure may change over time and we will need to be able to deal with 
libraries compiled with potentially different version of clang which may use a 
different format for the entries.
I think we may need some sort of version stamp.
We could use the section name for this purpose and rename it when we change the 
struct format, but that would be a bit more fragile as it's easier to forget to 
update the name if/when the struct format changes.
Also, format mismatch would looks like offload section is missing, which would 
need special handling when we diagnose the problem to distinguish incompatible 
offload table from the missing offload table.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123471

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


[PATCH] D125050: [OpenMP] Try to Infer target triples using the offloading architecture

2022-05-06 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment.

could you please add a test to 
https://github.com/llvm/llvm-project/blob/main/clang/test/Driver/hip-offload-arch.hip

to make sure when -fopenmp is specified, no openmp offloading is performed? 
Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125050

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


[PATCH] D125094: [ARM][Thumb] Command-line option to ensure AAPCS compliant Frame Records

2022-05-06 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

We probably want testcases for accessing various parts of the stack frame with 
FP stored in r11; for example, can we correctly access arguments passed on the 
stack if there's stack realignment?




Comment at: llvm/lib/Target/ARM/Thumb1FrameLowering.cpp:242
+  // Find last push instruction for GPRCS2 - spilling of high registers
+  // (r8-r11) could consist of multiple tPUSH and tMOVr instructions.
+  while (true) {

It seems sort of fragile to assume the entry block doesn't contain any tPUSH 
instructions; we can use them in places other than the prologue.  Can we use 
the FrameSetup flag?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125094

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


[PATCH] D125050: [OpenMP] Try to Infer target triples using the offloading architecture

2022-05-06 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:789
+  (C.getInputArgs().hasArg(options::OPT_fopenmp_targets_EQ) ||
+   C.getInputArgs().hasArg(options::OPT_offload_arch_EQ));
+  if (IsOpenMPOffloading) {

probably should be

C.getInputArgs().hasArg(options::OPT_offload_arch_EQ) && !IsHIP && !IsCUDA

otherwise a HIP program may get both HIP offloading and OpenMP offloading at 
the same time. Currently it is not supported.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125050

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


[PATCH] D125103: [clangd] Speed up a slow sleeping testcase.

2022-05-06 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

Many thanks for improving the running time, this slow test has bugged me since 
the day I first ran it.
I'd even be happy to remove it completely. It's slow, possibly flaky, so IMO 
the corresponding check does not carry its weight.

I'm not sure if we introduced a new race condition there that will cause flaky 
test, see the comment.
Otherwise LGTM.




Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:272
+  [&](std::vector) {
+ADD_FAILURE()
+<< "auto should have been discarded (dead write)";

This is potentially a race in the test that we did not have before, right?
There is probably much less work in the main test thread, but we probably can 
still see the failure if we are unlucky with how the OS schedules us.

My stance is it's better to have no tests for certain behaviours than flaky 
tests.
But if you find it useful let's try to run it and see whether my suspicions are 
unfounded, don't want to block the change on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125103

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


[PATCH] D125092: [OpenMP] Add basic support for properly handling static libraries

2022-05-06 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added a comment.

In D125092#3497033 , @saiislam wrote:

>> Ideally we could just put this on the linker itself, but nvlink doesn't seem 
>> to support .a files.
>
> Yes, nvlink does not support archives. So we used a wrapper to extract cubin 
> files from the archive and pass them to nvlink. Please see, Clang Nvlink 
> Wrapper .

Yeah, it seems we'll need to do a manual work-around later to handle the 
following case:

  // foo.o
  void foo() { return; }

  //libfoo.a
  void foo() { return; }

  clang foo.o libfoo.a // no conflict, symbol foo isn't undefined in foo.o so 
libfoo.a is never loaded

  nvlink foo.cubin libfoo.cubin // conflict, both are loaded and linked together

This is a little bit of an edge-case so I just went for the basic support here, 
since AMD just uses `lld` we could just pass in `libfoo.a` and it should handle 
it correctly for us. Add it to the list of Nvidia problems we need to work 
around externally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125092

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


[PATCH] D125050: [OpenMP] Try to Infer target triples using the offloading architecture

2022-05-06 Thread Saiyedul Islam via Phabricator via cfe-commits
saiislam accepted this revision.
saiislam added a comment.
This revision is now accepted and ready to land.

Thanks!
LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125050

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


  1   2   3   >