[PATCH] D133662: [Clang] WIP: Change -ftime-trace storing path and support multiple compilation jobs

2022-09-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clang/include/clang/Driver/Compilation.h:134
+
+  llvm::SmallDenseMap TimeTraceFiles;
+

I don't think we should use a non-zero inline element for these members.
They are, in 99.9%  cases, empty. We should optimize for the member size by 
making the number of inline element to 0.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133662

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


[PATCH] D133662: [Clang] WIP: Change -ftime-trace storing path and support multiple compilation jobs

2022-09-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment.

At a glance the approach looks more elegant to me, but I haven't studied this 
carefully yet. Also thanks for checking how to handle offloading.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133662

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


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

2022-09-13 Thread Guillot Tony via Phabricator via cfe-commits
to268 marked 6 inline comments as done.
to268 added a comment.

In D133289#3784042 , @aaron.ballman 
wrote:

>   auto auto k = 12; // 99% this is intended to be accepted; first `auto` is 
> the storage class specifier, second `auto` is a redundant storage class 
> specifier

It seems that specifying `auto` twice isn't supported at the time being

  auto auto test = 12; // error: cannot combine with previous 'auto' 
declaration specifier




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133289

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


[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-09-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision.
MaskRay added a comment.
This revision now requires changes to proceed.

Sorry, we should compare this approach with D133662 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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


[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

2022-09-13 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Driver/Driver.cpp:4516
+  bool HasTimeTrace = C.getArgs().hasArg(options::OPT_ftime_trace);
+  bool HasTimeTraceFile = C.getArgs().hasArg(options::OPT_ftime_trace_EQ);
+  // Whether `-ftime-trace` or `-ftime-trace=` are specified

Change this variable to use `getLastArg(options::OPT_ftime_trace_EQ)` instead.

The convention is to use getLastArg if both hasArg and getLastArg are needed.



Comment at: clang/lib/Driver/Driver.cpp:4517
+  bool HasTimeTraceFile = C.getArgs().hasArg(options::OPT_ftime_trace_EQ);
+  // Whether `-ftime-trace` or `-ftime-trace=` are specified
+  if (!HasTimeTrace && !HasTimeTraceFile)

Delete the comment. Don't add comment which just repeats what simply code does.



Comment at: clang/lib/Driver/Driver.cpp:4526
+  // then the full OutputFile's path may be appended to it.
+  SmallString<128> TracePath("");
+  if (HasTimeTraceFile) {





Comment at: clang/lib/Driver/Driver.cpp:4528
+  if (HasTimeTraceFile) {
+TracePath = SmallString<128>(
+C.getArgs().getLastArg(options::OPT_ftime_trace_EQ)->getValue());

Delete `SmallString<128>` => `TracePath = ...`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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


[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2022-09-13 Thread John Ericson via Phabricator via cfe-commits
Ericson2314 added inline comments.



Comment at: cmake/Modules/GNUBinaryDirs.cmake:3
+  get_filename_component(CMAKE_LIBDIR_BASENAME "${CMAKE_INSTALL_LIBDIR}" NAME)
+endif()
+

compnerd wrote:
> Should this perhaps be moved further down near the usage?
Sure



Comment at: cmake/Modules/GNUBinaryDirs.cmake:6
+if (NOT DEFINED CMAKE_BINARY_BINDIR)
+  set(CMAKE_BINARY_BINDIR "${CMAKE_BINARY_BINDIR}/bin")
+endif()

compnerd wrote:
> arichardson wrote:
> > I find this name a bit confusing, maybe something like `CMAKE_BUILD_BINDIR` 
> > (and the same for _LIBDIR/_INCLUDEDIR would be less surprising? That way 
> > it's clear that we are referring to binaries/libraries/includes in the 
> > build output (and it mirrors CMAKE_INSTALL_INCLUDEDIR, etc.) 
> I think you mean `${CMAKE_BINARY_DIR}/bin` as this is in a block where 
> `CMAKE_BINARY_BINDIR` is undefined.
@compnerd oops thanks.

@arichardson `CMAKE_BINARY_*` is the already existing naming convention of 
built-in variables. I agree it is not the best, but I don't want to buck the 
standard.



Comment at: cmake/Modules/GNUBinaryDirs.cmake:10
+if (NOT DEFINED CMAKE_BINARY_INCLUDEDIR)
+  set(CMAKE_BINARY_INCLUDEDIR "${CMAKE_BINARY_DIR}/inc")
+endif()

compnerd wrote:
> Is the default spelling for include not `include` not `inc`?
Oops yes it is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132608

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


[PATCH] D132608: [CMake] Clean up CMake binary dir handling

2022-09-13 Thread John Ericson via Phabricator via cfe-commits
Ericson2314 updated this revision to Diff 459985.
Ericson2314 marked 4 inline comments as done.
Ericson2314 added a comment.

Rebase, avoid sed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132608

Files:
  bolt/CMakeLists.txt
  clang/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  cmake/Modules/GNUBinaryDirs.cmake
  cmake/Modules/LLVMLibdirSuffix.cmake
  cmake/Modules/LLVMSetIntDirs.cmake
  compiler-rt/CMakeLists.txt
  compiler-rt/cmake/base-config-ix.cmake
  flang/CMakeLists.txt
  libc/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/docs/BuildingLibcxx.rst
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/docs/BuildingLibunwind.rst
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/cmake/modules/LLDBStandalone.cmake
  llvm/CMakeLists.txt
  mlir/CMakeLists.txt
  openmp/CMakeLists.txt
  polly/CMakeLists.txt

Index: polly/CMakeLists.txt
===
--- polly/CMakeLists.txt
+++ polly/CMakeLists.txt
@@ -1,12 +1,28 @@
 # Check if this is a in tree build.
+
+set(POLLY_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+set(POLLY_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+
 if (NOT DEFINED LLVM_MAIN_SRC_DIR)
   project(Polly)
   cmake_minimum_required(VERSION 3.13.4)
   set(POLLY_STANDALONE_BUILD TRUE)
 endif()
 
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+  set(LLVM_COMMON_CMAKE_UTILS ${POLLY_SOURCE_DIR}/../cmake)
+endif()
+
+list(INSERT CMAKE_MODULE_PATH 0
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
+
+# Must go before the first `include(GNUInstallDirs)`.
+include(LLVMLibdirSuffix)
+
 # Must go below project(..)
 include(GNUInstallDirs)
+include(GNUBinaryDirs)
 
 if(POLLY_STANDALONE_BUILD)
   # Where is LLVM installed?
@@ -48,18 +64,10 @@
   set(POLLY_GTEST_AVAIL 1)
 endif ()
 
-set(POLLY_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
-set(POLLY_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
-
-if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
-  set(LLVM_COMMON_CMAKE_UTILS ${POLLY_SOURCE_DIR}/../cmake)
-endif()
-
 # Make sure that our source directory is on the current cmake module path so that
 # we can include cmake files from this directory.
 list(INSERT CMAKE_MODULE_PATH 0
   "${POLLY_SOURCE_DIR}/cmake"
-  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
   )
 
 include("polly_macros")
Index: openmp/CMakeLists.txt
===
--- openmp/CMakeLists.txt
+++ openmp/CMakeLists.txt
@@ -1,12 +1,5 @@
 cmake_minimum_required(VERSION 3.13.4)
 
-set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
-
-# Add path for custom modules
-list(INSERT CMAKE_MODULE_PATH 0
-  "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
-  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
-  )
 
 # llvm/runtimes/ will set OPENMP_STANDALONE_BUILD.
 if (OPENMP_STANDALONE_BUILD OR "${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
@@ -14,6 +7,18 @@
   project(openmp C CXX)
 endif()
 
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+  set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
+endif()
+
+# Add path for custom modules
+list(INSERT CMAKE_MODULE_PATH 0
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
+
+# Must go before the first `include(GNUInstallDirs)`.
+include(LLVMLibdirSuffix)
+
 # Must go below project(..)
 include(GNUInstallDirs)
 
@@ -51,6 +56,11 @@
   endif()
 endif()
 
+# Add path for custom modules
+list(INSERT CMAKE_MODULE_PATH 0
+  "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
+  )
+
 # Check and set up common compiler flags.
 include(config-ix)
 include(HandleOpenMPOptions)
Index: mlir/CMakeLists.txt
===
--- mlir/CMakeLists.txt
+++ mlir/CMakeLists.txt
@@ -1,13 +1,28 @@
 # MLIR project.
 
+set(MLIR_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+set(MLIR_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+
 # Check if MLIR is built as a standalone project.
 if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   project(mlir)
   set(MLIR_STANDALONE_BUILD TRUE)
 endif()
 
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+  set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
+endif()
+
+list(INSERT CMAKE_MODULE_PATH 0
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
+
+# Must go before the first `include(GNUInstallDirs)`.
+include(LLVMLibdirSuffix)
+
 # Must go below project(..)
 include(GNUInstallDirs)
+include(GNUBinaryDirs)
 
 if(MLIR_STANDALONE_BUILD)
   cmake_minimum_required(VERSION 3.13.4)
@@ -36,12 +51,10 @@
 "Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')")
 mark_as_advanced(MLIR_TOOLS_INSTALL_DIR)
 
-set(MLIR_MAIN_SRC_DIR ${CMAKE_CURRENT_SOURCE_DIR}  )
+set(MLIR_MAIN_SRC_DIR ${MLIR_SOURCE_DIR}   )
 set(MLIR_MAIN_INCLUDE_DIR ${MLIR_MAIN_SRC_DIR}/include )
 
-set(MLIR_SOURCE_DIR  ${CMAKE_CURRENT_SOURCE_DIR})
-set(MLIR_BINARY_DIR  ${CMAKE_CURRENT_BINARY_DIR})
-set(MLIR_INCLUDE_DIR ${CMAKE_CURRENT_BINARY_DIR}/include)
+set(MLIR_INCLUDE_DIR 

[PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited, part 2

2022-09-13 Thread John Ericson via Phabricator via cfe-commits
Ericson2314 added a comment.

I think I fixed the issue: I was confusing `/lib` stemming from lib subdirs in 
the source and `/lib` stemming from building things in a lib directory that 
will later be installed also to a lib directory. It is just the latter case 
that uses the suffix, and so just the latter case will be so `sed`ed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132316

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


[clang] f712c01 - [HLSL]Add -O and -Od option for dxc mode.

2022-09-13 Thread Xiang Li via cfe-commits

Author: Xiang Li
Date: 2022-09-13T21:26:18-07:00
New Revision: f712c0131f3b2d7b95ff4e21e8050943c01565d6

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

LOG: [HLSL]Add -O and -Od option for dxc mode.

Two new dxc mode options -O and -Od are added for dxc mode.
-O is just alias of existing cc1 -O option.
-Od will be lowered into -O0 and -dxc-opt-disable.

-dxc-opt-disable is cc1 option added to for build ShaderFlags.

Reviewed By: beanz

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

Added: 
clang/test/CodeGenHLSL/disable_opt.hlsl
clang/test/Driver/dxc_O.hlsl

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

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index d6a4ad40f502..3931a2e9272b 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -6402,6 +6402,9 @@ def cl_ignored_Group : OptionGroup<"">,
 class CLFlag : Option<["/", "-"], name, KIND_FLAG>,
   Group, Flags<[CLOption, NoXarchOption]>;
 
+class CLDXCFlag : Option<["/", "-"], name, KIND_FLAG>,
+  Group, Flags<[CLDXCOption, NoXarchOption]>;
+
 class CLCompileFlag : Option<["/", "-"], name, KIND_FLAG>,
   Group, Flags<[CLOption, NoXarchOption]>;
 
@@ -6411,6 +6414,9 @@ class CLIgnoredFlag : Option<["/", "-"], 
name, KIND_FLAG>,
 class CLJoined : Option<["/", "-"], name, KIND_JOINED>,
   Group, Flags<[CLOption, NoXarchOption]>;
 
+class CLDXCJoined : Option<["/", "-"], name, KIND_JOINED>,
+  Group, Flags<[CLDXCOption, NoXarchOption]>;
+
 class CLCompileJoined : Option<["/", "-"], name, KIND_JOINED>,
   Group, Flags<[CLOption, NoXarchOption]>;
 
@@ -6507,7 +6513,7 @@ def _SLASH_J : CLFlag<"J">, HelpText<"Make char type 
unsigned">,
 
 // The _SLASH_O option handles all the /O flags, but we also provide separate
 // aliased options to provide separate help messages.
-def _SLASH_O : CLJoined<"O">,
+def _SLASH_O : CLDXCJoined<"O">,
   HelpText<"Set multiple /O flags at once; e.g. '/O2y-' for '/O2 /Oy-'">,
   MetaVarName<"">;
 def : CLFlag<"O1">, Alias<_SLASH_O>, AliasArgs<["1"]>,
@@ -6520,7 +6526,7 @@ def : CLFlag<"Ob1">, Alias<_SLASH_O>, AliasArgs<["b1"]>,
   HelpText<"Only inline functions explicitly or implicitly marked inline">;
 def : CLFlag<"Ob2">, Alias<_SLASH_O>, AliasArgs<["b2"]>,
   HelpText<"Inline functions as deemed beneficial by the compiler">;
-def : CLFlag<"Od">, Alias<_SLASH_O>, AliasArgs<["d"]>,
+def : CLDXCFlag<"Od">, Alias<_SLASH_O>, AliasArgs<["d"]>,
   HelpText<"Disable optimization">;
 def : CLFlag<"Og">, Alias<_SLASH_O>, AliasArgs<["g"]>,
   HelpText<"No effect">;

diff  --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp 
b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 591d0c43f182..8e0088784e54 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -46,6 +46,11 @@ void addDxilValVersion(StringRef ValVersionStr, llvm::Module 
) {
   auto *DXILValMD = M.getOrInsertNamedMetadata(DXILValKey);
   DXILValMD->addOperand(Val);
 }
+void addDisableOptimizations(llvm::Module ) {
+  StringRef Key = "dx.disable_optimizations";
+  M.addModuleFlag(llvm::Module::ModFlagBehavior::Override, Key, 1);
+}
+
 } // namespace
 
 void CGHLSLRuntime::finishCodeGen() {
@@ -56,6 +61,8 @@ void CGHLSLRuntime::finishCodeGen() {
 addDxilValVersion(TargetOpts.DxilValidatorVersion, M);
 
   generateGlobalCtorDtorCalls();
+  if (CGM.getCodeGenOpts().OptimizationLevel == 0)
+addDisableOptimizations(M);
 }
 
 void CGHLSLRuntime::annotateHLSLResource(const VarDecl *D, GlobalVariable *GV) 
{

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 45a71be97acd..a4ff80f275d7 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -3520,6 +3520,7 @@ static void RenderHLSLOptions(const ArgList , 
ArgStringList ,
  options::OPT_D,
  options::OPT_I,
  options::OPT_S,
+ options::OPT_O,
  options::OPT_emit_llvm,
  options::OPT_emit_obj,
  options::OPT_disable_llvm_passes,

diff  --git a/clang/lib/Driver/ToolChains/HLSL.cpp 
b/clang/lib/Driver/ToolChains/HLSL.cpp
index e3952361df7f..a80fea2e3efe 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -164,6 +164,18 @@ HLSLToolChain::TranslateArgs(const DerivedArgList , 
StringRef BoundArch,

[PATCH] D128845: [HLSL]Add -O and -Od option for dxc mode.

2022-09-13 Thread Xiang Li via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf712c0131f3b: [HLSL]Add -O and -Od option for dxc mode. 
(authored by python3kgae).

Changed prior to commit:
  https://reviews.llvm.org/D128845?vs=459883=459977#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128845

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/HLSL.cpp
  clang/test/CodeGenHLSL/disable_opt.hlsl
  clang/test/Driver/dxc_O.hlsl
  clang/test/Driver/dxc_fcgl.hlsl

Index: clang/test/Driver/dxc_fcgl.hlsl
===
--- clang/test/Driver/dxc_fcgl.hlsl
+++ clang/test/Driver/dxc_fcgl.hlsl
@@ -1,5 +1,6 @@
 // RUN: %clang_dxc -fcgl -T lib_6_7 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"
+// CHECK:"-S"
+// CHECK-SAME:"-emit-llvm" "-disable-llvm-passes"
 
Index: clang/test/Driver/dxc_O.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_O.hlsl
@@ -0,0 +1,18 @@
+// RUN: %clang_dxc -T lib_6_7  foo.hlsl -### %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -T lib_6_7 -Od foo.hlsl -### %s 2>&1 | FileCheck %s --check-prefix=Od
+// RUN: %clang_dxc -T lib_6_7 -O0 foo.hlsl -### %s 2>&1 | FileCheck %s --check-prefix=O0
+// RUN: %clang_dxc -T lib_6_7 -O1 foo.hlsl -### %s 2>&1 | FileCheck %s --check-prefix=O1
+// RUN: %clang_dxc -T lib_6_7 -O2 foo.hlsl -### %s 2>&1 | FileCheck %s --check-prefix=O2
+// RUN: %clang_dxc -T lib_6_7 -O3 foo.hlsl -### %s 2>&1 | FileCheck %s --check-prefix=O3
+
+// Make sure default is O3.
+// CHECK: "-O3"
+
+// Make sure Od/O0 option flag which translated into "-O0".
+// Od: "-O0"
+// O0: "-O0"
+
+// Make sure O1/O2/O3 is send to cc1.
+// O1: "-O1"
+// O2: "-O2"
+// O3: "-O3"
Index: clang/test/CodeGenHLSL/disable_opt.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/disable_opt.hlsl
@@ -0,0 +1,12 @@
+// RUN: %clang -cc1 -S -triple dxil-pc-shadermodel6.3-library -O0 -emit-llvm -xhlsl  -o - %s | FileCheck %s
+// RUN: %clang -cc1 -S -triple dxil-pc-shadermodel6.3-library -O3 -emit-llvm -xhlsl  -o - %s | FileCheck %s --check-prefix=OPT
+
+// CHECK:!"dx.disable_optimizations", i32 1}
+
+// OPT-NOT:"dx.disable_optimizations"
+
+float bar(float a, float b);
+
+float foo(float a, float b) {
+  return bar(a, b);
+}
Index: clang/lib/Driver/ToolChains/HLSL.cpp
===
--- clang/lib/Driver/ToolChains/HLSL.cpp
+++ clang/lib/Driver/ToolChains/HLSL.cpp
@@ -164,6 +164,18 @@
   A->claim();
   continue;
 }
+if (A->getOption().getID() == options::OPT__SLASH_O) {
+  StringRef OStr = A->getValue();
+  if (OStr == "d") {
+DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_O0));
+A->claim();
+continue;
+  } else {
+DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_O), OStr);
+A->claim();
+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));
@@ -192,6 +204,9 @@
 Opts.getOption(options::OPT_dxil_validator_version),
 DefaultValidatorVer);
   }
+  if (!DAL->hasArg(options::OPT_O_Group)) {
+DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_O), "3");
+  }
   // 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
@@ -3520,6 +3520,7 @@
  options::OPT_D,
  options::OPT_I,
  options::OPT_S,
+ options::OPT_O,
  options::OPT_emit_llvm,
  options::OPT_emit_obj,
  options::OPT_disable_llvm_passes,
Index: clang/lib/CodeGen/CGHLSLRuntime.cpp
===
--- clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -46,6 +46,11 @@
   auto *DXILValMD = M.getOrInsertNamedMetadata(DXILValKey);
   DXILValMD->addOperand(Val);
 }
+void addDisableOptimizations(llvm::Module ) {
+  StringRef Key = "dx.disable_optimizations";
+  

[PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited, part 2

2022-09-13 Thread John Ericson via Phabricator via cfe-commits
Ericson2314 updated this revision to Diff 459975.
Ericson2314 added a comment.

Require `LLVM_LIBDIR_SUFFIX` in the sed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132316

Files:
  clang/cmake/modules/CMakeLists.txt
  flang/cmake/modules/CMakeLists.txt
  lld/cmake/modules/CMakeLists.txt
  lldb/cmake/modules/LLDBConfig.cmake
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/CMakeLists.txt
  mlir/cmake/modules/CMakeLists.txt
  polly/cmake/CMakeLists.txt
  polly/test/CMakeLists.txt

Index: polly/test/CMakeLists.txt
===
--- polly/test/CMakeLists.txt
+++ polly/test/CMakeLists.txt
@@ -46,7 +46,7 @@
 
 set(LLVM_BINARY_DIR "${LLVM_BINARY_DIR}")
 set(LLVM_TOOLS_DIR "${LLVM_TOOLS_BINARY_DIR}")
-set(LLVM_LIBS_DIR "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}")
+set(LLVM_LIBS_DIR "${LLVM_LIBRARY_DIR}")
 if (CMAKE_LIBRARY_OUTPUT_DIRECTORY)
   set(POLLY_LIB_DIR ${CMAKE_LIBRARY_OUTPUT_DIRECTORY})
 else()
Index: polly/cmake/CMakeLists.txt
===
--- polly/cmake/CMakeLists.txt
+++ polly/cmake/CMakeLists.txt
@@ -12,7 +12,7 @@
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
   "Path for CMake subdirectory for LLVM (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/llvm')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
+set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
 
 if (CMAKE_CONFIGURATION_TYPES)
   set(POLLY_EXPORTS_FILE_NAME "PollyExports-$>.cmake")
Index: mlir/cmake/modules/CMakeLists.txt
===
--- mlir/cmake/modules/CMakeLists.txt
+++ mlir/cmake/modules/CMakeLists.txt
@@ -15,7 +15,7 @@
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
   "Path for CMake subdirectory for LLVM (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/llvm')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
+set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
 
 get_property(MLIR_EXPORTS GLOBAL PROPERTY MLIR_EXPORTS)
 export(TARGETS ${MLIR_EXPORTS} FILE ${mlir_cmake_builddir}/MLIRTargets.cmake)
Index: llvm/cmake/modules/CMakeLists.txt
===
--- llvm/cmake/modules/CMakeLists.txt
+++ llvm/cmake/modules/CMakeLists.txt
@@ -3,7 +3,7 @@
 include(FindPrefixFromConfig)
 
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
+set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
 
 # First for users who use an installed LLVM, create the LLVMExports.cmake file.
 set(LLVM_EXPORTS_FILE ${llvm_cmake_builddir}/LLVMExports.cmake)
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -1093,7 +1093,7 @@
   message(FATAL_ERROR "LLVM_INSTALL_PACKAGE_DIR must be defined and writable. GEN_CONFIG should only be passe when building LLVM proper.")
   endif()
   # LLVM_INSTALL_PACKAGE_DIR might be absolute, so don't reuse below.
-  set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
+  set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
   file(WRITE
   "${llvm_cmake_builddir}/LLVMConfigExtensions.cmake"
   "set(LLVM_STATIC_EXTENSIONS ${LLVM_STATIC_EXTENSIONS})")
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -940,9 +940,9 @@
   )
 
 # They are not referenced. See set_output_directory().
-set( CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/bin )
-set( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
-set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX} )
+set( CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LLVM_TOOLS_BINARY_DIR} )
+set( CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LLVM_LIBRARY_DIR} )
+set( CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LLVM_LIBRARY_DIR} )
 
 if(LLVM_INCLUDE_TESTS)
   include(GetErrcMessages)
Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -276,7 +276,7 @@
   # could be and pick the first that exists.
   foreach(CANDIDATE "${Clang_DIR}/../.." "${LLVM_DIR}" "${LLVM_LIBRARY_DIRS}"
 "${LLVM_BUILD_LIBRARY_DIR}"
-"${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}")
+"${LLVM_LIBRARY_DIR}")
 # Build 

[PATCH] D132316: [CMake] Avoid `LLVM_BINARY_DIR` when other more specific variable are better-suited

2022-09-13 Thread John Ericson via Phabricator via cfe-commits
Ericson2314 updated this revision to Diff 459968.
Ericson2314 added a comment.

Rebase on top of D133828 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132316

Files:
  bolt/lib/Target/AArch64/CMakeLists.txt
  bolt/lib/Target/X86/CMakeLists.txt
  bolt/unittests/Core/CMakeLists.txt
  clang/cmake/modules/CMakeLists.txt
  clang/lib/Tooling/CMakeLists.txt
  flang/cmake/modules/CMakeLists.txt
  lld/cmake/modules/CMakeLists.txt
  lldb/cmake/modules/LLDBConfig.cmake
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/tools/llvm-exegesis/lib/AArch64/CMakeLists.txt
  llvm/tools/llvm-exegesis/lib/Mips/CMakeLists.txt
  llvm/tools/llvm-exegesis/lib/PowerPC/CMakeLists.txt
  llvm/tools/llvm-exegesis/lib/X86/CMakeLists.txt
  llvm/unittests/Target/ARM/CMakeLists.txt
  llvm/unittests/Target/DirectX/CMakeLists.txt
  llvm/unittests/tools/llvm-exegesis/AArch64/CMakeLists.txt
  llvm/unittests/tools/llvm-exegesis/ARM/CMakeLists.txt
  llvm/unittests/tools/llvm-exegesis/Mips/CMakeLists.txt
  llvm/unittests/tools/llvm-exegesis/PowerPC/CMakeLists.txt
  llvm/unittests/tools/llvm-exegesis/X86/CMakeLists.txt
  llvm/unittests/tools/llvm-mca/X86/CMakeLists.txt
  mlir/cmake/modules/CMakeLists.txt
  polly/cmake/CMakeLists.txt
  polly/test/CMakeLists.txt

Index: polly/test/CMakeLists.txt
===
--- polly/test/CMakeLists.txt
+++ polly/test/CMakeLists.txt
@@ -46,7 +46,7 @@
 
 set(LLVM_BINARY_DIR "${LLVM_BINARY_DIR}")
 set(LLVM_TOOLS_DIR "${LLVM_TOOLS_BINARY_DIR}")
-set(LLVM_LIBS_DIR "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}")
+set(LLVM_LIBS_DIR "${LLVM_LIBRARY_DIR}")
 if (CMAKE_LIBRARY_OUTPUT_DIRECTORY)
   set(POLLY_LIB_DIR ${CMAKE_LIBRARY_OUTPUT_DIRECTORY})
 else()
Index: polly/cmake/CMakeLists.txt
===
--- polly/cmake/CMakeLists.txt
+++ polly/cmake/CMakeLists.txt
@@ -12,7 +12,7 @@
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
   "Path for CMake subdirectory for LLVM (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/llvm')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
+set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
 
 if (CMAKE_CONFIGURATION_TYPES)
   set(POLLY_EXPORTS_FILE_NAME "PollyExports-$>.cmake")
Index: mlir/cmake/modules/CMakeLists.txt
===
--- mlir/cmake/modules/CMakeLists.txt
+++ mlir/cmake/modules/CMakeLists.txt
@@ -15,7 +15,7 @@
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
   "Path for CMake subdirectory for LLVM (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/llvm')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(llvm_cmake_builddir "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm")
+set(llvm_cmake_builddir "${LLVM_LIBRARY_DIR}/cmake/llvm")
 
 get_property(MLIR_EXPORTS GLOBAL PROPERTY MLIR_EXPORTS)
 export(TARGETS ${MLIR_EXPORTS} FILE ${mlir_cmake_builddir}/MLIRTargets.cmake)
Index: llvm/unittests/tools/llvm-mca/X86/CMakeLists.txt
===
--- llvm/unittests/tools/llvm-mca/X86/CMakeLists.txt
+++ llvm/unittests/tools/llvm-mca/X86/CMakeLists.txt
@@ -1,6 +1,6 @@
 add_llvm_mca_unittest_includes(
   ${LLVM_MAIN_SRC_DIR}/lib/Target/X86
-  ${LLVM_BINARY_DIR}/lib/Target/X86
+  ${LLVM_LIBRARY_DIR}/Target/X86
   )
 
 add_llvm_mca_unittest_sources(
Index: llvm/unittests/tools/llvm-exegesis/X86/CMakeLists.txt
===
--- llvm/unittests/tools/llvm-exegesis/X86/CMakeLists.txt
+++ llvm/unittests/tools/llvm-exegesis/X86/CMakeLists.txt
@@ -1,6 +1,6 @@
 add_llvm_exegesis_unittest_includes(
   ${LLVM_MAIN_SRC_DIR}/lib/Target/X86
-  ${LLVM_BINARY_DIR}/lib/Target/X86
+  ${LLVM_LIBRARY_DIR}/Target/X86
   ${LLVM_MAIN_SRC_DIR}/tools/llvm-exegesis/lib
   )
 
Index: llvm/unittests/tools/llvm-exegesis/PowerPC/CMakeLists.txt
===
--- llvm/unittests/tools/llvm-exegesis/PowerPC/CMakeLists.txt
+++ llvm/unittests/tools/llvm-exegesis/PowerPC/CMakeLists.txt
@@ -1,6 +1,6 @@
 add_llvm_exegesis_unittest_includes(
   ${LLVM_MAIN_SRC_DIR}/lib/Target/PowerPC
-  ${LLVM_BINARY_DIR}/lib/Target/PowerPC
+  ${LLVM_LIBRARY_DIR}/Target/PowerPC
   ${LLVM_MAIN_SRC_DIR}/tools/llvm-exegesis/lib
   )
 
Index: llvm/unittests/tools/llvm-exegesis/Mips/CMakeLists.txt
===
--- llvm/unittests/tools/llvm-exegesis/Mips/CMakeLists.txt
+++ llvm/unittests/tools/llvm-exegesis/Mips/CMakeLists.txt
@@ -1,6 +1,6 @@
 add_llvm_exegesis_unittest_includes(
   ${LLVM_MAIN_SRC_DIR}/lib/Target/Mips
-  

[PATCH] D127800: [llvm-driver] Generate symlinks instead of executables for tools

2022-09-13 Thread Alex Brachet via Phabricator via cfe-commits
abrachet marked an inline comment as done.
abrachet added inline comments.



Comment at: llvm/cmake/modules/AddLLVM.cmake:1283
 macro(add_llvm_tool name)
+  cmake_parse_arguments(ARG "DEPENDS;GENERATE_DRIVER" "" "" ${ARGN})
   if( NOT LLVM_BUILD_TOOLS )

Amir wrote:
> Sorry for a late question but I don't see any use of ARG_DEPENDS - is it 
> intentional or there's an omission somewhere?
It looks like this was superfluous and likely copied from a place where DEPENDS 
is used. It's not necessary for depends to be used here, as it wasn't being 
used before for `add_llvm_tool`. I'll remove this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127800

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


[PATCH] D111283: [clang] template / auto deduction deduces common sugar

2022-09-13 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov updated this revision to Diff 459959.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111283

Files:
  clang-tools-extra/clangd/unittests/ASTTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp
  clang/test/SemaCXX/deduced-return-void.cpp
  clang/test/SemaCXX/sugared-auto.cpp
  clang/test/SemaTemplate/deduction.cpp
  libcxx/utils/ci/buildkite-pipeline.yml

Index: libcxx/utils/ci/buildkite-pipeline.yml
===
--- libcxx/utils/ci/buildkite-pipeline.yml
+++ libcxx/utils/ci/buildkite-pipeline.yml
@@ -59,19 +59,6 @@
   limit: 2
 timeout_in_minutes: 120
 
-  - label: "Documentation"
-command: "libcxx/utils/ci/run-buildbot documentation"
-artifact_paths:
-  - "**/test-results.xml"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
   #
   # General testing with the default configuration, under all the supported
   # Standard modes, with Clang and GCC. This catches most issues upfront.
@@ -79,273 +66,6 @@
   #
   - wait
 
-  - label: "C++2b"
-command: "libcxx/utils/ci/run-buildbot generic-cxx2b"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-CC: "clang-${LLVM_HEAD_VERSION}"
-CXX: "clang++-${LLVM_HEAD_VERSION}"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  - label: "C++11"
-command: "libcxx/utils/ci/run-buildbot generic-cxx11"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-CC: "clang-${LLVM_HEAD_VERSION}"
-CXX: "clang++-${LLVM_HEAD_VERSION}"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  - label: "C++03"
-command: "libcxx/utils/ci/run-buildbot generic-cxx03"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-CC: "clang-${LLVM_HEAD_VERSION}"
-CXX: "clang++-${LLVM_HEAD_VERSION}"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  - label: "Modular build"
-command: "libcxx/utils/ci/run-buildbot generic-modules"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-CC: "clang-${LLVM_HEAD_VERSION}"
-CXX: "clang++-${LLVM_HEAD_VERSION}"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  - label: "GCC ${GCC_STABLE_VERSION} / C++latest"
-command: "libcxx/utils/ci/run-buildbot generic-gcc"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-CC: "gcc-${GCC_STABLE_VERSION}"
-CXX: "g++-${GCC_STABLE_VERSION}"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  #
-  # All other supported configurations of libc++.
-  #
-  - wait
-
-  - label: "C++20"
-command: "libcxx/utils/ci/run-buildbot generic-cxx20"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-CC: "clang-${LLVM_HEAD_VERSION}"
-CXX: "clang++-${LLVM_HEAD_VERSION}"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  - label: "C++17"
-command: "libcxx/utils/ci/run-buildbot generic-cxx17"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-# TODO (mordante) use head
-#CC: "clang-${LLVM_HEAD_VERSION}"
-#CXX: "clang++-${LLVM_HEAD_VERSION}"
-CC: "clang-15"
-CXX: "clang++-15"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  - label: 

[PATCH] D127800: [llvm-driver] Generate symlinks instead of executables for tools

2022-09-13 Thread Amir Ayupov via Phabricator via cfe-commits
Amir added inline comments.



Comment at: llvm/cmake/modules/AddLLVM.cmake:1283
 macro(add_llvm_tool name)
+  cmake_parse_arguments(ARG "DEPENDS;GENERATE_DRIVER" "" "" ${ARGN})
   if( NOT LLVM_BUILD_TOOLS )

Sorry for a late question but I don't see any use of ARG_DEPENDS - is it 
intentional or there's an omission somewhere?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127800

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


[PATCH] D133341: [C++] [Coroutines] Prefer aligned (de)allocation for coroutines - implement the option2 of P2014R0

2022-09-13 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added inline comments.



Comment at: clang/test/SemaCXX/coroutine-alloc-4.cpp:49
+void return_value(int) {}
+void *operator new(std::size_t, std::align_val_t) noexcept;
+void *operator new(std::size_t) noexcept;

Like this test case, please add additional test cases to check the expected 
look-up order, one test for each consecutive pair should be good.

```
void* T::operator new  ( std::size_t count, std::align_val_t al, 
user-defined-args... );
void* T::operator new  ( std::size_t count, std::align_val_t al);
void* T::operator new  ( std::size_t count, user-defined-args... );
void* T::operator new  ( std::size_t count);
void* operator new  ( std::size_t count, std::align_val_t al );
```




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

https://reviews.llvm.org/D133341

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


[PATCH] D133341: [C++] [Coroutines] Prefer aligned (de)allocation for coroutines - implement the option2 of P2014R0

2022-09-13 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment.

It surprises me that Option 2 does not change 
https://eel.is/c++draft/dcl.fct.def.coroutine#10. For consistency, I think it 
should. And according to your test case, it deals with alignment as expected. 
Probably we should change the P2014R0 wording accordingly. Before that, let's 
just mention this difference in the Clang release notes.




Comment at: clang/docs/ClangCommandLineReference.rst:1580
+
+Enable support for P2014R0 Option2, which will always pursue the aligned 
allocation function.
+This option is disabled by default.





Comment at: clang/docs/ReleaseNotes.rst:152-154
+- Implemented `-fcoro-aligned-allocation` flag. This option2 implement
+  option2 for P2014R0 aligned allocation of coroutine frames
+  (`P2014R0 
`_).





Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11283
+def err_conflicting_aligned_options : Error <
+  "conflicting option '-fcoro-aligned-allocation' and 
'-fno-aligned-allocation'"
 >;

Since we're digressing from the usual operator new/delete look-up rules per 
Option 2, I think it might be easier to just *not* respect 
`-fno-aligned-allocation` and `-fno-sized-deallocation`. Using 
'-fcoro-aligned-allocation' explicitly should permit us to assume these 
functions could be found.



Comment at: clang/include/clang/Basic/LangOptions.def:157
 LANGOPT(Coroutines, 1, 0, "C++20 coroutines")
+LANGOPT(CoroAlignedAllocation, 1, 0, "prefer Aligned Allocation according to 
P2014's option2")
 LANGOPT(DllExportInlines  , 1, 1, "dllexported classes dllexport inline 
methods")





Comment at: clang/lib/Sema/SemaCoroutine.cpp:1302-1318
   // According to [dcl.fct.def.coroutine]p9, Lookup allocation functions using 
a
   // parameter list composed of the requested size of the coroutine state being
   // allocated, followed by the coroutine function's arguments. If a matching
   // allocation function exists, use it. Otherwise, use an allocation function
   // that just takes the requested size.
   //
   // [dcl.fct.def.coroutine]p9

Update comment here.



Comment at: clang/lib/Sema/SemaCoroutine.cpp:1400
 
+  // If we found a non-aligned allocation function in the promise_type,
+  // it indicates the user forgot to update the allocation function. Let's emit

Option 2's order of look-up is 
```
void* T::operator new  ( std::size_t count, std::align_val_t al, 
user-defined-args... );
void* T::operator new  ( std::size_t count, std::align_val_t al);
void* T::operator new  ( std::size_t count, user-defined-args... );
void* T::operator new  ( std::size_t count);
void* operator new  ( std::size_t count, std::align_val_t al );
```
Why not allow `void* T::operator new  ( std::size_t count);` here?



Comment at: clang/test/CodeGenCoroutines/coro-aligned-alloc.cpp:94
+void return_value(int) {}
+void operator delete(void *ptr, std::align_val_t);
+  };

Please add test cases for 
```
void operator delete  ( void* ptr, std::size_t, std::align_val_t);
void operator delete  ( void* ptr, std::size_t);
void operator delete  ( void* ptr);
void ::operator delete  ( void* ptr, std::size_t, std::align_val_t);
void ::operator delete  ( void* ptr, std::size_t);
void ::operator delete  ( void* ptr);
```
in that lookup order, and makes sure the look-up order is expected.


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

https://reviews.llvm.org/D133341

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


[PATCH] D133457: Add Clang driver flags equivalent to cl's /MD, /MT, /MDd, /MTd.

2022-09-13 Thread Hans Wennborg via Phabricator via cfe-commits
hans added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4442
+bool IsClangCL) {
+  unsigned RTOptionID = 0; // MT=0, MTd=1, MD=2, MDd=3
+  bool HasLDdFlag = IsClangCL && Args.hasArg(options::OPT__SLASH_LDd);

Could this be an enum, or reuse existing values like the /Mx option ids?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4443
+  unsigned RTOptionID = 0; // MT=0, MTd=1, MD=2, MDd=3
+  bool HasLDdFlag = IsClangCL && Args.hasArg(options::OPT__SLASH_LDd);
+

The `IsClangCL &&` part seems redundant?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4445
+
+  if (Args.hasArg(options::OPT__SLASH_LDd))
+// The /LDd option implies /MTd. The dependent lib part can be overridden,

Should this use HasLDdFlag?



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4461
+  } else {
+if (Arg *A = Args.getLastArg(options::OPT_fms_runtime_lib_EQ)) {
+  StringRef Val = A->getValue();

Is there a getLastArg() variant that allows getting the last argument of either 
the /Mx options or -fms-runtime-lib, so we don't need to check IsClangCL?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133457

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


[PATCH] D111283: [clang] template / auto deduction deduces common sugar

2022-09-13 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added a comment.

@vhscampos that issue should be fixed now as well, thanks for the report!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111283

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


[PATCH] D111283: [clang] template / auto deduction deduces common sugar

2022-09-13 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov updated this revision to Diff 459946.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111283

Files:
  clang-tools-extra/clangd/unittests/ASTTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp
  clang/test/SemaCXX/deduced-return-void.cpp
  clang/test/SemaCXX/sugared-auto.cpp
  clang/test/SemaTemplate/deduction.cpp
  libcxx/utils/ci/buildkite-pipeline.yml

Index: libcxx/utils/ci/buildkite-pipeline.yml
===
--- libcxx/utils/ci/buildkite-pipeline.yml
+++ libcxx/utils/ci/buildkite-pipeline.yml
@@ -59,19 +59,6 @@
   limit: 2
 timeout_in_minutes: 120
 
-  - label: "Documentation"
-command: "libcxx/utils/ci/run-buildbot documentation"
-artifact_paths:
-  - "**/test-results.xml"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
   #
   # General testing with the default configuration, under all the supported
   # Standard modes, with Clang and GCC. This catches most issues upfront.
@@ -79,273 +66,6 @@
   #
   - wait
 
-  - label: "C++2b"
-command: "libcxx/utils/ci/run-buildbot generic-cxx2b"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-CC: "clang-${LLVM_HEAD_VERSION}"
-CXX: "clang++-${LLVM_HEAD_VERSION}"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  - label: "C++11"
-command: "libcxx/utils/ci/run-buildbot generic-cxx11"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-CC: "clang-${LLVM_HEAD_VERSION}"
-CXX: "clang++-${LLVM_HEAD_VERSION}"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  - label: "C++03"
-command: "libcxx/utils/ci/run-buildbot generic-cxx03"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-CC: "clang-${LLVM_HEAD_VERSION}"
-CXX: "clang++-${LLVM_HEAD_VERSION}"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  - label: "Modular build"
-command: "libcxx/utils/ci/run-buildbot generic-modules"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-CC: "clang-${LLVM_HEAD_VERSION}"
-CXX: "clang++-${LLVM_HEAD_VERSION}"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  - label: "GCC ${GCC_STABLE_VERSION} / C++latest"
-command: "libcxx/utils/ci/run-buildbot generic-gcc"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-CC: "gcc-${GCC_STABLE_VERSION}"
-CXX: "g++-${GCC_STABLE_VERSION}"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  #
-  # All other supported configurations of libc++.
-  #
-  - wait
-
-  - label: "C++20"
-command: "libcxx/utils/ci/run-buildbot generic-cxx20"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-CC: "clang-${LLVM_HEAD_VERSION}"
-CXX: "clang++-${LLVM_HEAD_VERSION}"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  - label: "C++17"
-command: "libcxx/utils/ci/run-buildbot generic-cxx17"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-# TODO (mordante) use head
-#CC: "clang-${LLVM_HEAD_VERSION}"
-#CXX: "clang++-${LLVM_HEAD_VERSION}"
-CC: "clang-15"
-CXX: "clang++-15"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  - label: 

[PATCH] D133622: [clang][test] Disallow using the default module cache path in lit tests

2022-09-13 Thread Bruno Cardoso Lopes via Phabricator via cfe-commits
bruno added a comment.

> I'm not sure how to deal with missing `env -u`.
>
> - We could do `env CLANG_MODULE_CACHE_PATH=` and change the compiler's 
> interpretation of empty string for this variable. I'm not sure if the current 
> behaviour (there will be no module cache in the cc1 at all) is intentional or 
> useful.  Hesitant to change this behaviour.

How about using `self.with_environment('CLANG_MODULE_CACHE_PATH', '')` so we 
don't need to worry about using `env -u` to unset? My understand is that (1) 
`getDefaultModuleCachePath` is the only place using `CLANG_MODULE_CACHE_PATH`, 
and (2) `std::getenv` return nullptr if it's empty, which will fallback to 
system provided path instead.

> - We could try to enumerate all the environments that don't support `env -u` 
> and disable these two tests on  those platforms.  So far it was just one AIX 
> bot, but I wouldn't be surprised if other less commonly used unixes have the 
> same issue.
>
> - We could make the command inscrutable, like `not env -u X true || env -u 
> ... real command ...` so that if `env -u X true` fails (presumably due to not 
> supporting `-u` option) we won't run the rest of the RUN line.

Not sure it helps much but according to `option-X.test`, `system-aix` support 
`unset`.

> If someone has a suggestion for a simple fix, I can try again.  But otherwise 
> I doubt it's worth putting much time into this.

Thanks for trying to improve this :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133622

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


[PATCH] D111283: [clang] template / auto deduction deduces common sugar

2022-09-13 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov updated this revision to Diff 459944.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111283

Files:
  clang-tools-extra/clangd/unittests/ASTTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/SemaCXX/cxx0x-initializer-stdinitializerlist.cpp
  clang/test/SemaCXX/deduced-return-void.cpp
  clang/test/SemaCXX/sugared-auto.cpp
  clang/test/SemaTemplate/deduction.cpp
  libcxx/utils/ci/buildkite-pipeline.yml

Index: libcxx/utils/ci/buildkite-pipeline.yml
===
--- libcxx/utils/ci/buildkite-pipeline.yml
+++ libcxx/utils/ci/buildkite-pipeline.yml
@@ -59,19 +59,6 @@
   limit: 2
 timeout_in_minutes: 120
 
-  - label: "Documentation"
-command: "libcxx/utils/ci/run-buildbot documentation"
-artifact_paths:
-  - "**/test-results.xml"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
   #
   # General testing with the default configuration, under all the supported
   # Standard modes, with Clang and GCC. This catches most issues upfront.
@@ -79,273 +66,6 @@
   #
   - wait
 
-  - label: "C++2b"
-command: "libcxx/utils/ci/run-buildbot generic-cxx2b"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-CC: "clang-${LLVM_HEAD_VERSION}"
-CXX: "clang++-${LLVM_HEAD_VERSION}"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  - label: "C++11"
-command: "libcxx/utils/ci/run-buildbot generic-cxx11"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-CC: "clang-${LLVM_HEAD_VERSION}"
-CXX: "clang++-${LLVM_HEAD_VERSION}"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  - label: "C++03"
-command: "libcxx/utils/ci/run-buildbot generic-cxx03"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-CC: "clang-${LLVM_HEAD_VERSION}"
-CXX: "clang++-${LLVM_HEAD_VERSION}"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  - label: "Modular build"
-command: "libcxx/utils/ci/run-buildbot generic-modules"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-CC: "clang-${LLVM_HEAD_VERSION}"
-CXX: "clang++-${LLVM_HEAD_VERSION}"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  - label: "GCC ${GCC_STABLE_VERSION} / C++latest"
-command: "libcxx/utils/ci/run-buildbot generic-gcc"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-CC: "gcc-${GCC_STABLE_VERSION}"
-CXX: "g++-${GCC_STABLE_VERSION}"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  #
-  # All other supported configurations of libc++.
-  #
-  - wait
-
-  - label: "C++20"
-command: "libcxx/utils/ci/run-buildbot generic-cxx20"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-CC: "clang-${LLVM_HEAD_VERSION}"
-CXX: "clang++-${LLVM_HEAD_VERSION}"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  - label: "C++17"
-command: "libcxx/utils/ci/run-buildbot generic-cxx17"
-artifact_paths:
-  - "**/test-results.xml"
-  - "**/*.abilist"
-env:
-# TODO (mordante) use head
-#CC: "clang-${LLVM_HEAD_VERSION}"
-#CXX: "clang++-${LLVM_HEAD_VERSION}"
-CC: "clang-15"
-CXX: "clang++-15"
-agents:
-  queue: "libcxx-builders"
-  os: "linux"
-retry:
-  automatic:
-- exit_status: -1  # Agent was lost
-  limit: 2
-timeout_in_minutes: 120
-
-  - label: 

[PATCH] D133807: Update Unicode to 15.0

2022-09-13 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment.

Thanks for the review




Comment at: llvm/lib/Support/UnicodeCaseFold.cpp:713
+  // 8 characters
+  if (C <= 0xa7c2)
 return C | 1;

shafik wrote:
> Maybe I am misunderstanding the comments but should this be `0xa7be`?
Quirk of the script,  the comment for C|1 never make sense, the values seems 
correct though (this script is the only think i have not written myself)
https://github.com/llvm/llvm-project/blob/main/llvm/utils/unicode-case-fold.py#L89
So you have 8 even codepoints mapping to  C|1 + 7 odd codepoint mapping to C|1 
which is C. If my math is correct.
I'm a bit reluctant to modify that script


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133807

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


[PATCH] D133807: Update Unicode to 15.0

2022-09-13 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 459941.
cor3ntin added a comment.

Use the name "Unicode 15.0" consistently


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133807

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Lex/UnicodeCharSets.h
  llvm/lib/Support/Unicode.cpp
  llvm/lib/Support/UnicodeCaseFold.cpp
  llvm/lib/Support/UnicodeNameToCodepoint.cpp
  llvm/lib/Support/UnicodeNameToCodepointGenerated.cpp
  llvm/utils/UnicodeData/UnicodeNameMappingGenerator.cpp

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


[PATCH] D111283: [clang] template / auto deduction deduces common sugar

2022-09-13 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:12367
+  case Type::Class:
\
+llvm_unreachable("Unexpected " Kind ": " #Class);
+

aaronpuchert wrote:
> mizvekov wrote:
> > davrec wrote:
> > > mizvekov wrote:
> > > > davrec wrote:
> > > > > mizvekov wrote:
> > > > > > davrec wrote:
> > > > > > > Could we just return `X` here? Would that just default to the old 
> > > > > > > behavior instead of crashing whenever unforeseen cases arise?  
> > > > > > No, I think we should enforce the invariants and make sure we are 
> > > > > > handling everything that can be handled.
> > > > > > 
> > > > > > Classing `TemplateTypeParm`  as sugar-free was what was wrong and 
> > > > > > we missed this on the review.
> > > > > There might always going to be a few rare corner cases vulnerable to 
> > > > > this though, particularly as more types are added and the people 
> > > > > adding them don't pay strict attention to how to incorporate them 
> > > > > here, and don't write the requisite tests (which seem very difficult 
> > > > > to foresee and produce).  When those cases arise we will be crashing 
> > > > > even though we could produce a perfectly good program with the 
> > > > > intended semantics; the only thing that would suffer for most users 
> > > > > is slightly less clear diagnostic messages for those rare cases.  I 
> > > > > think it would be better to let those cases gradually percolate to 
> > > > > our attention via bug reports concerning those diagnostics, rather 
> > > > > than inconveniencing the user and demanding immediate attention via 
> > > > > crashes.  Or am I missing something?
> > > > I could on the same argument remove all asserts here and just let the 
> > > > program not crash on unforeseen circumstances.
> > > > 
> > > > On the other hand, having these asserts here helps us catch bugs not 
> > > > only here, but in other parts of the code. For example uniquing / 
> > > > canonicalization bugs.
> > > > 
> > > > If someone changes the properties of a type so that the assumptions 
> > > > here are not valid anymore, it's helpful to have that pointed out soon.
> > > > 
> > > > Going for as an example this specific bug, if there werent those 
> > > > asserts / unreachables in place and we had weakened the validation 
> > > > here, it would take a very long time for us to figure out we were 
> > > > making the wrong assumption with regards to TemplateTypeParmType.
> > > > 
> > > > I'd rather figure that out sooner on CI / internal testing rather than 
> > > > have a bug created by a user two years from now.
> > > Yes its nicer to developers to get stack traces and reproductions 
> > > whenever something goes wrong, and crashing is a good way to get those, 
> > > but users probably won't be so thrilled about it.  Especially given that 
> > > the main selling point of this patch is that it makes diagnostics nicer 
> > > for users: isn't it a bit absurd to crash whenever we can't guarantee our 
> > > diagnostics will be perfect?
> > > 
> > > And again the real problem is future types not being properly 
> > > incorporated here and properly tested: i.e. the worry is that this will 
> > > be a continuing source of crashes, even if we handle all the present 
> > > types properly.
> > > 
> > > How about we leave it as an unreachable for now, to help ensure all the 
> > > present types are handled, but if months or years from now there continue 
> > > to be crashes due to this, then just return X (while maybe write 
> > > something to llvm::errs() to encourage users to report it), so we don't 
> > > make the perfect the enemy of the good.
> > It's not about crashing when it won't be perfect. We already do these kinds 
> > of things, see the FIXMEs around the TemplateArgument and 
> > NestedNameSpecifier, where we just return something worse than we wish to, 
> > just because we don't have time to implement it now.
> > 
> > These unreachables and asserts here are about testing / documenting our 
> > knowledge of the system and making it easier to find problems. These should 
> > be impossible to happen in correct code, and if they do happen because of 
> > mistakes, fixing them sooner is just going to save us resources.
> > 
> > `llvm::errs` suggestion I perceive as out of line with current practice in 
> > LLVM, we don't and have a system for producing diagnostics for results 
> > possibly affected by FIXME and TODO situations and the like, as far as I 
> > know, and I am not aware of any discussions in that direction. I expect a 
> > lot of complexity and noise if we went this way.
> > And I think this would have even less chance of working out if we started 
> > to incorporate the reporting of violation of invariants into that.
> > 
> > I think even just using raw `llvm::errs` on clang would be incorrect per 
> > design, and could likely break users that parse our output.
> > 
> > 

[PATCH] D125655: [HLSL] add -P option for dxc mode.

2022-09-13 Thread Chris Bieneman via Phabricator via cfe-commits
beanz added inline comments.



Comment at: clang/lib/Driver/ToolChains/HLSL.cpp:181
+if (DAL->hasArg(options::OPT__SLASH_P))
+  getDriver().Diag(diag::warn_drv_dxc_ignore_output_for_preprocess) << 
"Fo";
+

python3kgae wrote:
> beanz wrote:
> > This warning should fire in CL mode too right? Should it not be 
> > dxc-specific?
> cl mode report this warning
> 
> 
> > warning: b.txt: 'linker' input unused [-Wunused-command-line-argument]
> 
> 
> when input is
>   --driver-mode=cl -Fo b.txt  -P -Fi"a.txt"
> 
> For HLSL, both -Fo and -Fi should be output.
Then it shouldn’t be named `dxc` :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125655

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


[PATCH] D133683: [c++] implements tentative DR1432 for partial ordering of function template

2022-09-13 Thread Yuanfang Chen via Phabricator via cfe-commits
ychen added a comment.

@mizvekov Thanks for taking a look.




Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5195-5196
+for (int i = 0, e = std::min(NumParams1, NumParams2); i < e; ++i) {
+  QualType T1 = FD1->getParamDecl(i)->getType().getDesugaredType(Context);
+  QualType T2 = FD2->getParamDecl(i)->getType().getDesugaredType(Context);
+  auto *TST1 = dyn_cast(T1);

mizvekov wrote:
> This should work, as all the properties you are testing below are present on 
> the canonical type.
Gotta admit that I was confused by `DesugaredType` and `CanonicalType`.  But 
since you pointed it out, I could see the differences now. The `CanonicalType` 
could be sugared, and desugaring is not necessary here. Thanks.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5455-5456
 if (!ClangABICompat15) {
-  // Consider this a fix for CWG1432. Similar to the fix for CWG1395.
   auto *TST1 = T1->castAs();
   auto *TST2 = T2->castAs();
+  const TemplateArgument  = TST1->template_arguments().back();

mizvekov wrote:
> This is a bug, T1 and T2 are not in general canonical types for this 
> function, and this castAs can pick up an alias template, and these should 
> never participate in deduction.
> 
> (Also, can you please add a test for that?)
> 
> You should not need to keep sugar here, everything being tested below is 
> present on the canonical type, so this is simple to solve, as suggested.
I see. I'll add a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133683

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


[PATCH] D133157: Add -fsanitizer-coverage=control-flow

2022-09-13 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added inline comments.



Comment at: 
compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_control_flow.cpp:1
+// Tests -fsanitize-coverage=control-flow.
+

I suggest to make this test smaller:
* foo() can by empty (but avoid inlining)
* main() can have a single conditional call to foo

* main should also have a single conditional indirect call to test the -1 indir 
call marker. 

* __sanitizer_cov_cfs_init should capture its arguments and return

* main should insepct the arguments captured in __sanitizer_cov_cfs_init and 
verify that it sees main() and foo(), and that main contains calls to foo() and 
indir function, outside of the entry BB.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133157

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


[PATCH] D133683: [c++] implements tentative DR1432 for partial ordering of function template

2022-09-13 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5195-5196
+for (int i = 0, e = std::min(NumParams1, NumParams2); i < e; ++i) {
+  QualType T1 = FD1->getParamDecl(i)->getType().getDesugaredType(Context);
+  QualType T2 = FD2->getParamDecl(i)->getType().getDesugaredType(Context);
+  auto *TST1 = dyn_cast(T1);

This should work, as all the properties you are testing below are present on 
the canonical type.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5455-5456
 if (!ClangABICompat15) {
-  // Consider this a fix for CWG1432. Similar to the fix for CWG1395.
   auto *TST1 = T1->castAs();
   auto *TST2 = T2->castAs();
+  const TemplateArgument  = TST1->template_arguments().back();

This is a bug, T1 and T2 are not in general canonical types for this function, 
and this castAs can pick up an alias template, and these should never 
participate in deduction.

(Also, can you please add a test for that?)

You should not need to keep sugar here, everything being tested below is 
present on the canonical type, so this is simple to solve, as suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133683

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


[PATCH] D111283: [clang] template / auto deduction deduces common sugar

2022-09-13 Thread Aaron Puchert via Phabricator via cfe-commits
aaronpuchert added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:12367
+  case Type::Class:
\
+llvm_unreachable("Unexpected " Kind ": " #Class);
+

mizvekov wrote:
> davrec wrote:
> > mizvekov wrote:
> > > davrec wrote:
> > > > mizvekov wrote:
> > > > > davrec wrote:
> > > > > > Could we just return `X` here? Would that just default to the old 
> > > > > > behavior instead of crashing whenever unforeseen cases arise?  
> > > > > No, I think we should enforce the invariants and make sure we are 
> > > > > handling everything that can be handled.
> > > > > 
> > > > > Classing `TemplateTypeParm`  as sugar-free was what was wrong and we 
> > > > > missed this on the review.
> > > > There might always going to be a few rare corner cases vulnerable to 
> > > > this though, particularly as more types are added and the people adding 
> > > > them don't pay strict attention to how to incorporate them here, and 
> > > > don't write the requisite tests (which seem very difficult to foresee 
> > > > and produce).  When those cases arise we will be crashing even though 
> > > > we could produce a perfectly good program with the intended semantics; 
> > > > the only thing that would suffer for most users is slightly less clear 
> > > > diagnostic messages for those rare cases.  I think it would be better 
> > > > to let those cases gradually percolate to our attention via bug reports 
> > > > concerning those diagnostics, rather than inconveniencing the user and 
> > > > demanding immediate attention via crashes.  Or am I missing something?
> > > I could on the same argument remove all asserts here and just let the 
> > > program not crash on unforeseen circumstances.
> > > 
> > > On the other hand, having these asserts here helps us catch bugs not only 
> > > here, but in other parts of the code. For example uniquing / 
> > > canonicalization bugs.
> > > 
> > > If someone changes the properties of a type so that the assumptions here 
> > > are not valid anymore, it's helpful to have that pointed out soon.
> > > 
> > > Going for as an example this specific bug, if there werent those asserts 
> > > / unreachables in place and we had weakened the validation here, it would 
> > > take a very long time for us to figure out we were making the wrong 
> > > assumption with regards to TemplateTypeParmType.
> > > 
> > > I'd rather figure that out sooner on CI / internal testing rather than 
> > > have a bug created by a user two years from now.
> > Yes its nicer to developers to get stack traces and reproductions whenever 
> > something goes wrong, and crashing is a good way to get those, but users 
> > probably won't be so thrilled about it.  Especially given that the main 
> > selling point of this patch is that it makes diagnostics nicer for users: 
> > isn't it a bit absurd to crash whenever we can't guarantee our diagnostics 
> > will be perfect?
> > 
> > And again the real problem is future types not being properly incorporated 
> > here and properly tested: i.e. the worry is that this will be a continuing 
> > source of crashes, even if we handle all the present types properly.
> > 
> > How about we leave it as an unreachable for now, to help ensure all the 
> > present types are handled, but if months or years from now there continue 
> > to be crashes due to this, then just return X (while maybe write something 
> > to llvm::errs() to encourage users to report it), so we don't make the 
> > perfect the enemy of the good.
> It's not about crashing when it won't be perfect. We already do these kinds 
> of things, see the FIXMEs around the TemplateArgument and 
> NestedNameSpecifier, where we just return something worse than we wish to, 
> just because we don't have time to implement it now.
> 
> These unreachables and asserts here are about testing / documenting our 
> knowledge of the system and making it easier to find problems. These should 
> be impossible to happen in correct code, and if they do happen because of 
> mistakes, fixing them sooner is just going to save us resources.
> 
> `llvm::errs` suggestion I perceive as out of line with current practice in 
> LLVM, we don't and have a system for producing diagnostics for results 
> possibly affected by FIXME and TODO situations and the like, as far as I 
> know, and I am not aware of any discussions in that direction. I expect a lot 
> of complexity and noise if we went this way.
> And I think this would have even less chance of working out if we started to 
> incorporate the reporting of violation of invariants into that.
> 
> I think even just using raw `llvm::errs` on clang would be incorrect per 
> design, and could likely break users that parse our output.
> 
> I think if months and years from now, if someone stumbled upon an assert 
> firing here and, instead of understanding what is happening and then fixing 
> the code, just removed / weakened 

[PATCH] D133807: Update Unicode to 15.0

2022-09-13 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment.

Thank you for doing this work.




Comment at: llvm/lib/Support/UnicodeCaseFold.cpp:713
+  // 8 characters
+  if (C <= 0xa7c2)
 return C | 1;

Maybe I am misunderstanding the comments but should this be `0xa7be`?



Comment at: llvm/lib/Support/UnicodeNameToCodepoint.cpp:254
 
-// Unicode 14.0
+// Unicode 15.0
 // 3.12 Conjoining Jamo Behavior Common constants

You use `15.0` here and `15.` above, any reason for the difference?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133807

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


[PATCH] D133817: MSVC ABI: Looks like even non-aarch64 uses the MSVC/14 definition for pod/aggregate passing

2022-09-13 Thread David Blaikie via Phabricator via cfe-commits
dblaikie created this revision.
dblaikie added reviewers: ayzhao, hansw.
Herald added subscribers: mstorsjo, kristof.beyls.
Herald added a project: All.
dblaikie requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Details posted here: https://reviews.llvm.org/D119051#3747201

3 cases that were inconsistent with the MSABI without this patch applied:

  https://godbolt.org/z/GY48qxh3G - field with protected member
  https://godbolt.org/z/Mb1PYhjrP - non-static data member initializer
  https://godbolt.org/z/sGvxcEPjo - defaulted copy constructor

I'm not sure what's suitable/sufficient testing for this - I did verify
the three cases above. Though if it helps to add them as explicit tests,
I can do that too.

Also, I was wondering if the other use of isTrivialForAArch64MSVC in
isPermittedToBeHomogenousAggregate could be another source of bugs - I
tried changing the function to unconditionally call
isTrivialFor(AArch64)MSVC without testing AArch64 first, but no tests
fail, so it looks like this is undertested in any case. But I had
trouble figuring out how to exercise this functionality properly to add
test coverage and then compare that to MSVC itself... - I got very
confused/turned around trying to test this, so I've given up enough to
send what I have out for review, but happy to look further into this
with help.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133817

Files:
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp


Index: clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
===
--- clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
+++ clang/test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
@@ -74,6 +74,10 @@
  int i;
 };
 
+struct SmallWithSmallWithPrivate {
+  SmallWithPrivate p;
+};
+
 // WIN32: declare dso_local void @"{{.*take_bools_and_chars.*}}"
 // WIN32:   (<{ i8, [3 x i8], i8, [3 x i8], %struct.SmallWithDtor,
 // WIN32:   i8, [3 x i8], i8, [3 x i8], i32, i8, [3 x i8] }>* 
inalloca(<{ i8, [3 x i8], i8, [3 x i8], %struct.SmallWithDtor, i8, [3 x i8], 
i8, [3 x i8], i32, i8, [3 x i8] }>)
@@ -197,6 +201,10 @@
 // WOA64: define dso_local void 
@"?small_arg_with_private_member@@YA?AUSmallWithPrivate@@U1@@Z"(%struct.SmallWithPrivate*
 inreg noalias sret(%struct.SmallWithPrivate) align 4 %agg.result, i64 
%s.coerce) {{.*}} {
 SmallWithPrivate small_arg_with_private_member(SmallWithPrivate s) { return s; 
}
 
+// WOA64: define dso_local i32 
@"?small_arg_with_small_struct_with_private_member@@YA?AUSmallWithSmallWithPrivate@@U1@@Z"(i64
 %s.coerce) {{.*}} {
+// WIN64: define dso_local i32 
@"?small_arg_with_small_struct_with_private_member@@YA?AUSmallWithSmallWithPrivate@@U1@@Z"(i32
 %s.coerce) {{.*}} {
+SmallWithSmallWithPrivate 
small_arg_with_small_struct_with_private_member(SmallWithSmallWithPrivate s) { 
return s; }
+
 void call_small_arg_with_dtor() {
   small_arg_with_dtor(SmallWithDtor());
 }
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -1086,8 +1086,8 @@
   return isDeletingDtor(GD);
 }
 
-static bool isTrivialForAArch64MSVC(const CXXRecordDecl *RD) {
-  // For AArch64, we use the C++14 definition of an aggregate, so we also
+static bool isTrivialForMSVC(const CXXRecordDecl *RD) {
+  // We use the C++14 definition of an aggregate, so we also
   // check for:
   //   No private or protected non static data members.
   //   No base classes
@@ -1115,15 +1115,7 @@
   if (!RD)
 return false;
 
-  // Normally, the C++ concept of "is trivially copyable" is used to determine
-  // if a struct can be returned directly. However, as MSVC and the language
-  // have evolved, the definition of "trivially copyable" has changed, while 
the
-  // ABI must remain stable. AArch64 uses the C++14 concept of an "aggregate",
-  // while other ISAs use the older concept of "plain old data".
-  bool isTrivialForABI = RD->isPOD();
-  bool isAArch64 = CGM.getTarget().getTriple().isAArch64();
-  if (isAArch64)
-isTrivialForABI = RD->canPassInRegisters() && isTrivialForAArch64MSVC(RD);
+  bool isTrivialForABI = RD->canPassInRegisters() && isTrivialForMSVC(RD);
 
   // MSVC always returns structs indirectly from C++ instance methods.
   bool isIndirectReturn = !isTrivialForABI || FI.isInstanceMethod();
@@ -1137,7 +1129,7 @@
 
 // On AArch64, use the `inreg` attribute if the object is considered to not
 // be trivially copyable, or if this is an instance method struct return.
-FI.getReturnInfo().setInReg(isAArch64);
+FI.getReturnInfo().setInReg(CGM.getTarget().getTriple().isAArch64());
 
 return true;
   }
@@ -4475,5 +4467,5 @@
   // aggregate according to the C++14 spec. This is not consistent with the
   // AAPCS64, but is defacto spec on 

[PATCH] D125655: [HLSL] add -P option for dxc mode.

2022-09-13 Thread Xiang Li via Phabricator via cfe-commits
python3kgae marked 3 inline comments as done.
python3kgae added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5459
+  if (AtTopLevel && !isa(JA) && !isa(JA) &&
+  !(IsDXCMode() && C.getArgs().hasArg(options::OPT__SLASH_P))) {
 if (Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o))

beanz wrote:
> Why is this change needed? Didn't we settle on having `/P` behave the way 
> CL.exe does?
Removed.



Comment at: clang/lib/Driver/ToolChains/HLSL.cpp:181
+if (DAL->hasArg(options::OPT__SLASH_P))
+  getDriver().Diag(diag::warn_drv_dxc_ignore_output_for_preprocess) << 
"Fo";
+

beanz wrote:
> This warning should fire in CL mode too right? Should it not be dxc-specific?
cl mode report this warning


> warning: b.txt: 'linker' input unused [-Wunused-command-line-argument]


when input is
  --driver-mode=cl -Fo b.txt  -P -Fi"a.txt"

For HLSL, both -Fo and -Fi should be output.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125655

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


[PATCH] D133157: Add -fsanitizer-coverage=control-flow

2022-09-13 Thread Kostya Serebryany via Phabricator via cfe-commits
kcc added inline comments.



Comment at: clang/docs/SanitizerCoverage.rst:382
+  void __sanitizer_cov_cfs_init(const uintptr_t *cfs_beg,
+const uintptr_t *cfs_end) {
+// [cfs_beg,cfs_end) is the array of ptr-sized integers representing

vitalybuka wrote:
> vitalybuka wrote:
> > we can also generate normal structs, per function, and pass them into 
> > __sanitizer_cov_cfs_init(const MyStruct* begin, const MyStruct* end);
> > this was can avoid  this magic encoding completely?
> Works for me either way. @kcc WDYT? 
The struct will still have some variable length arrays, right?
Will it be any better?

MyStruct will have to be defined there, and there is no header file for sancov 
for users to include, and I don't think I want one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133157

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


[PATCH] D133741: [IR] Add alignment for llvm.threadlocal.address

2022-09-13 Thread Alexander Shaposhnikov via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb6965f7246bb: [IR] Add alignment for 
llvm.threadlocal.address (authored by alexander-shaposhnikov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133741

Files:
  clang/test/CodeGenCXX/cxx11-thread-local-instantiated.cpp
  clang/test/CodeGenCXX/cxx11-thread-local-reference.cpp
  clang/test/CodeGenCXX/cxx11-thread-local.cpp
  clang/test/CodeGenCXX/cxx1y-variable-template.cpp
  clang/test/CodeGenCXX/cxx2a-thread-local-constinit.cpp
  clang/test/CodeGenCXX/microsoft-abi-thread-safe-statics.cpp
  clang/test/CodeGenCXX/pr18635.cpp
  clang/test/CodeGenCXX/threadlocal_address.cpp
  clang/test/Modules/initializers.cpp
  clang/test/OpenMP/parallel_copyin_codegen.cpp
  clang/test/OpenMP/parallel_copyin_combined_codegen.c
  clang/test/OpenMP/parallel_master_codegen.cpp
  clang/test/OpenMP/teams_distribute_parallel_for_copyin_codegen.cpp
  clang/test/OpenMP/threadprivate_codegen.cpp
  llvm/lib/IR/IRBuilder.cpp

Index: llvm/lib/IR/IRBuilder.cpp
===
--- llvm/lib/IR/IRBuilder.cpp
+++ llvm/lib/IR/IRBuilder.cpp
@@ -526,6 +526,14 @@
   return CreateCall(TheFn, Ops);
 }
 
+static MaybeAlign getAlign(Value *Ptr) {
+  if (auto *O = dyn_cast(Ptr))
+return O->getAlign();
+  if (auto *A = dyn_cast(Ptr))
+return A->getAliaseeObject()->getAlign();
+  return {};
+}
+
 CallInst *IRBuilderBase::CreateThreadLocalAddress(Value *Ptr) {
 #ifndef NDEBUG
   // Handle specially for constexpr cast. This is possible when
@@ -540,8 +548,13 @@
   assert(isa(V) && cast(V)->isThreadLocal() &&
  "threadlocal_address only applies to thread local variables.");
 #endif
-  return CreateIntrinsic(llvm::Intrinsic::threadlocal_address, {Ptr->getType()},
- {Ptr});
+  CallInst *CI = CreateIntrinsic(llvm::Intrinsic::threadlocal_address,
+ {Ptr->getType()}, {Ptr});
+  if (MaybeAlign A = getAlign(Ptr)) {
+CI->addParamAttr(0, Attribute::getWithAlignment(CI->getContext(), *A));
+CI->addRetAttr(Attribute::getWithAlignment(CI->getContext(), *A));
+  }
+  return CI;
 }
 
 CallInst *
Index: clang/test/OpenMP/threadprivate_codegen.cpp
===
--- clang/test/OpenMP/threadprivate_codegen.cpp
+++ clang/test/OpenMP/threadprivate_codegen.cpp
@@ -3662,7 +3662,7 @@
 // CHECK-TLS1-NEXT:[[A1:%.*]] = getelementptr inbounds [[STRUCT_S3:%.*]], %struct.S3* [[TMP4]], i32 0, i32 0
 // CHECK-TLS1-NEXT:[[TMP5:%.*]] = load i32, i32* [[A1]], align 4
 // CHECK-TLS1-NEXT:store i32 [[TMP5]], i32* [[RES]], align 4
-// CHECK-TLS1-NEXT:[[TMP6:%.*]] = call %struct.Smain* @llvm.threadlocal.address.p0s_struct.Smains(%struct.Smain* @_ZZ4mainE2sm)
+// CHECK-TLS1-NEXT:[[TMP6:%.*]] = call align 8 %struct.Smain* @llvm.threadlocal.address.p0s_struct.Smains(%struct.Smain* align 8 @_ZZ4mainE2sm)
 // CHECK-TLS1-NEXT:[[A2:%.*]] = getelementptr inbounds [[STRUCT_SMAIN:%.*]], %struct.Smain* [[TMP6]], i32 0, i32 0
 // CHECK-TLS1-NEXT:[[TMP7:%.*]] = load i32, i32* [[A2]], align 8
 // CHECK-TLS1-NEXT:[[TMP8:%.*]] = load i32, i32* [[RES]], align 4
@@ -3692,12 +3692,12 @@
 // CHECK-TLS1-NEXT:[[TMP19:%.*]] = load i32, i32* [[RES]], align 4
 // CHECK-TLS1-NEXT:[[ADD10:%.*]] = add nsw i32 [[TMP19]], [[TMP18]]
 // CHECK-TLS1-NEXT:store i32 [[ADD10]], i32* [[RES]], align 4
-// CHECK-TLS1-NEXT:[[TMP20:%.*]] = call i32* @llvm.threadlocal.address.p0i32(i32* @_ZN2STIiE2stE)
+// CHECK-TLS1-NEXT:[[TMP20:%.*]] = call align 4 i32* @llvm.threadlocal.address.p0i32(i32* align 4 @_ZN2STIiE2stE)
 // CHECK-TLS1-NEXT:[[TMP21:%.*]] = load i32, i32* [[TMP20]], align 4
 // CHECK-TLS1-NEXT:[[TMP22:%.*]] = load i32, i32* [[RES]], align 4
 // CHECK-TLS1-NEXT:[[ADD11:%.*]] = add nsw i32 [[TMP22]], [[TMP21]]
 // CHECK-TLS1-NEXT:store i32 [[ADD11]], i32* [[RES]], align 4
-// CHECK-TLS1-NEXT:[[TMP23:%.*]] = call float* @llvm.threadlocal.address.p0f32(float* @_ZN2STIfE2stE)
+// CHECK-TLS1-NEXT:[[TMP23:%.*]] = call align 4 float* @llvm.threadlocal.address.p0f32(float* align 4 @_ZN2STIfE2stE)
 // CHECK-TLS1-NEXT:[[TMP24:%.*]] = load float, float* [[TMP23]], align 4
 // CHECK-TLS1-NEXT:[[CONV:%.*]] = fptosi float [[TMP24]] to i32
 // CHECK-TLS1-NEXT:[[TMP25:%.*]] = load i32, i32* [[RES]], align 4
@@ -3716,7 +3716,7 @@
 // CHECK-TLS1-LABEL: define {{[^@]+}}@_ZTWL3gs1
 // CHECK-TLS1-SAME: () #[[ATTR5:[0-9]+]] {
 // CHECK-TLS1-NEXT:call void @_ZTHL3gs1()
-// CHECK-TLS1-NEXT:[[TMP1:%.*]] = call %struct.S1* @llvm.threadlocal.address.p0s_struct.S1s(%struct.S1* @_ZL3gs1)
+// CHECK-TLS1-NEXT:[[TMP1:%.*]] = call align 4 %struct.S1* @llvm.threadlocal.address.p0s_struct.S1s(%struct.S1* align 4 @_ZL3gs1)
 // CHECK-TLS1-NEXT:ret %struct.S1* [[TMP1]]
 //
 //
@@ -3750,7 +3750,7 

[clang] b6965f7 - [IR] Add alignment for llvm.threadlocal.address

2022-09-13 Thread Alexander Shaposhnikov via cfe-commits

Author: Alexander Shaposhnikov
Date: 2022-09-13T23:10:55Z
New Revision: b6965f7246bba1b399755f56d8ae34893e815198

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

LOG: [IR] Add alignment for llvm.threadlocal.address

This diff sets the alignment attribute for the return value
and the argument of llvm.threadlocal.address.

(https://github.com/llvm/llvm-project/issues/57438)

Test plan: ninja check-all

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

Added: 


Modified: 
clang/test/CodeGenCXX/cxx11-thread-local-instantiated.cpp
clang/test/CodeGenCXX/cxx11-thread-local-reference.cpp
clang/test/CodeGenCXX/cxx11-thread-local.cpp
clang/test/CodeGenCXX/cxx1y-variable-template.cpp
clang/test/CodeGenCXX/cxx2a-thread-local-constinit.cpp
clang/test/CodeGenCXX/microsoft-abi-thread-safe-statics.cpp
clang/test/CodeGenCXX/pr18635.cpp
clang/test/CodeGenCXX/threadlocal_address.cpp
clang/test/Modules/initializers.cpp
clang/test/OpenMP/parallel_copyin_codegen.cpp
clang/test/OpenMP/parallel_copyin_combined_codegen.c
clang/test/OpenMP/parallel_master_codegen.cpp
clang/test/OpenMP/teams_distribute_parallel_for_copyin_codegen.cpp
clang/test/OpenMP/threadprivate_codegen.cpp
llvm/lib/IR/IRBuilder.cpp

Removed: 




diff  --git a/clang/test/CodeGenCXX/cxx11-thread-local-instantiated.cpp 
b/clang/test/CodeGenCXX/cxx11-thread-local-instantiated.cpp
index 46c16bdd94e39..fc3514a8b17da 100644
--- a/clang/test/CodeGenCXX/cxx11-thread-local-instantiated.cpp
+++ b/clang/test/CodeGenCXX/cxx11-thread-local-instantiated.cpp
@@ -17,7 +17,7 @@ S *current() { return TLS::mData; };
 
 // CHECK-LABEL: define weak_odr hidden {{.*}} @_ZTWN3TLSI1SE5mDataE() {{.*}} 
comdat {
 // CHECK: call void @_ZTHN3TLSI1SE5mDataE()
-// CHECK: [[TLSmData_ADDR:%[^ ]+]] = call ptr @llvm.threadlocal.address.p0(ptr 
@_ZN3TLSI1SE5mDataE)
+// CHECK: [[TLSmData_ADDR:%[^ ]+]] = call align 8 ptr 
@llvm.threadlocal.address.p0(ptr align 8 @_ZN3TLSI1SE5mDataE)
 // CHECK: ret {{.*}} [[TLSmData_ADDR]]
 
 // Unlike for a global, the global initialization function must not be in a

diff  --git a/clang/test/CodeGenCXX/cxx11-thread-local-reference.cpp 
b/clang/test/CodeGenCXX/cxx11-thread-local-reference.cpp
index 3fc960f1eefc4..1d5143f1d3510 100644
--- a/clang/test/CodeGenCXX/cxx11-thread-local-reference.cpp
+++ b/clang/test/CodeGenCXX/cxx11-thread-local-reference.cpp
@@ -15,7 +15,7 @@ int () { return r; }
 
 // CHECK: define {{.*}} @[[R_INIT:.*]]()
 // CHECK: call noundef nonnull align {{[0-9]+}} dereferenceable({{[0-9]+}}) 
i32* @_Z1fv()
-// CHECK: %[[R_ADDR:.+]] = call i32** @llvm.threadlocal.address.p0p0i32(i32** 
@r)
+// CHECK: %[[R_ADDR:.+]] = call align 8 i32** 
@llvm.threadlocal.address.p0p0i32(i32** align 8 @r)
 // CHECK: store i32* %{{.*}}, i32** %[[R_ADDR]], align 8
 
 // CHECK-LABEL: define{{.*}} nonnull align {{[0-9]+}} 
dereferenceable({{[0-9]+}}) i32* @_Z1gv()
@@ -27,7 +27,7 @@ int () { return r; }
 // DARWIN: define cxx_fast_tlscc noundef i32* @_ZTW1r() [[ATTR1:#[0-9]+]] {
 // LINUX_AIX: call void @_ZTH1r()
 // DARWIN: call cxx_fast_tlscc void @_ZTH1r()
-// CHECK: %[[R_ADDR2:.+]] = call i32** @llvm.threadlocal.address.p0p0i32(i32** 
@r)
+// CHECK: %[[R_ADDR2:.+]] = call align 8 i32** 
@llvm.threadlocal.address.p0p0i32(i32** align 8 @r)
 // CHECK: load i32*, i32** %[[R_ADDR2]], align 8
 // CHECK: ret i32* %{{.*}}
 

diff  --git a/clang/test/CodeGenCXX/cxx11-thread-local.cpp 
b/clang/test/CodeGenCXX/cxx11-thread-local.cpp
index 4023daa8192ab..7b53211e9ceb0 100644
--- a/clang/test/CodeGenCXX/cxx11-thread-local.cpp
+++ b/clang/test/CodeGenCXX/cxx11-thread-local.cpp
@@ -108,7 +108,7 @@ void *e2 = V::m + W::m + ::m;
 
 // CHECK: define {{.*}} @[[A_INIT:.*]]()
 // CHECK: call{{.*}} i32 @_Z1fv()
-// CHECK: [[A_ADDR:%.+]] = call i32* @llvm.threadlocal.address.p0i32(i32* @a)
+// CHECK: [[A_ADDR:%.+]] = call align 4 i32* 
@llvm.threadlocal.address.p0i32(i32* align 4 @a)
 // CHECK-NEXT: store i32 {{.*}}, i32* [[A_ADDR]], align 4
 
 // CHECK-LABEL: define{{.*}} i32 @_Z1fv()
@@ -118,13 +118,13 @@ int f() {
   // CHECK: br i1 %[[NEED_INIT]]{{.*}}
 
   // CHECK: %[[CALL:.*]] = call{{.*}} i32 @_Z1gv()
-  // CHECK: [[N_ADDR:%.+]] = call i32* @llvm.threadlocal.address.p0i32(i32* 
@_ZZ1fvE1n)
+  // CHECK: [[N_ADDR:%.+]] = call align 4 i32* 
@llvm.threadlocal.address.p0i32(i32* align 4 @_ZZ1fvE1n)
   // CHECK: store i32 %[[CALL]], i32* [[N_ADDR]], align 4
   // CHECK: store i8 1, i8* @_ZGVZ1fvE1n
   // CHECK: br label
   static thread_local int n = g();
-  
-  // CHECK: [[N_ADDR2:%.+]] = call i32* @llvm.threadlocal.address.p0i32(i32* 
@_ZZ1fvE1n)
+
+  // CHECK: [[N_ADDR2:%.+]] = call align 4 i32* 
@llvm.threadlocal.address.p0i32(i32* align 4 @_ZZ1fvE1n)
   // CHECK: load i32, i32* [[N_ADDR2]], 

[PATCH] D133711: [Sema] Reject array element types whose alignments are larger than their sizes

2022-09-13 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

Messing with the layout is risky.  If we give an error, the user will notice 
something is wrong, and can adapt their code (e.g. explicitly enforcing 
alignment using alignas).  If we silently use a different layout, and the class 
is used across compilers, the user ends up with a miscompile that's hard to 
diagnose.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133711

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


[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

2022-09-13 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 459911.
jhuber6 added a comment.

Changing to `getDeviceLibs`. I suppose in the future we could make this work 
for CUDA, but for now it won't be defined for that toolchain so it's fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133726

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/lib/Driver/ToolChains/HIPAMD.h
  clang/lib/Driver/ToolChains/HIPSPV.cpp
  clang/lib/Driver/ToolChains/HIPSPV.h
  clang/test/Driver/amdgpu-openmp-toolchain.c

Index: clang/test/Driver/amdgpu-openmp-toolchain.c
===
--- clang/test/Driver/amdgpu-openmp-toolchain.c
+++ clang/test/Driver/amdgpu-openmp-toolchain.c
@@ -49,5 +49,12 @@
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -emit-llvm -S -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx803 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-EMIT-LLVM-IR
 // CHECK-EMIT-LLVM-IR: "-cc1" "-triple" "amdgcn-amd-amdhsa"{{.*}}"-emit-llvm"
 
-// RUN: %clang -### -target x86_64-pc-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx803 -lm --rocm-device-lib-path=%S/Inputs/rocm/amdgcn/bitcode %s 2>&1 | FileCheck %s --check-prefix=CHECK-LIB-DEVICE-NEW
-// CHECK-LIB-DEVICE-NEW: {{.*}}clang-linker-wrapper{{.*}}--bitcode-library=openmp-amdgcn-amd-amdhsa-gfx803={{.*}}ocml.bc"{{.*}}ockl.bc"{{.*}}oclc_daz_opt_on.bc"{{.*}}oclc_unsafe_math_off.bc"{{.*}}oclc_finite_only_off.bc"{{.*}}oclc_correctly_rounded_sqrt_on.bc"{{.*}}oclc_wavefrontsize64_on.bc"{{.*}}oclc_isa_version_803.bc"
+// RUN: %clang -### -target x86_64-pc-linux-gnu -fopenmp --offload-arch=gfx803 \
+// RUN:   --rocm-device-lib-path=%S/Inputs/rocm/amdgcn/bitcode -fopenmp-new-driver %s  2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-LIB-DEVICE
+// CHECK-LIB-DEVICE: "-cc1" {{.*}}ocml.bc"{{.*}}ockl.bc"{{.*}}oclc_daz_opt_on.bc"{{.*}}oclc_unsafe_math_off.bc"{{.*}}oclc_finite_only_off.bc"{{.*}}oclc_correctly_rounded_sqrt_on.bc"{{.*}}oclc_wavefrontsize64_on.bc"{{.*}}oclc_isa_version_803.bc"
+
+// RUN: %clang -### -target x86_64-pc-linux-gnu -fopenmp --offload-arch=gfx803 -nogpulib \
+// RUN:   --rocm-device-lib-path=%S/Inputs/rocm/amdgcn/bitcode -fopenmp-new-driver %s  2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-LIB-DEVICE-NOGPULIB
+// CHECK-LIB-DEVICE-NOGPULIB-NOT: "-cc1" {{.*}}ocml.bc"{{.*}}ockl.bc"{{.*}}oclc_daz_opt_on.bc"{{.*}}oclc_unsafe_math_off.bc"{{.*}}oclc_finite_only_off.bc"{{.*}}oclc_correctly_rounded_sqrt_on.bc"{{.*}}oclc_wavefrontsize64_on.bc"{{.*}}oclc_isa_version_803.bc"
Index: clang/lib/Driver/ToolChains/HIPSPV.h
===
--- clang/lib/Driver/ToolChains/HIPSPV.h
+++ clang/lib/Driver/ToolChains/HIPSPV.h
@@ -69,7 +69,7 @@
   void AddHIPIncludeArgs(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ) const override;
   llvm::SmallVector
-  getHIPDeviceLibs(const llvm::opt::ArgList ) const override;
+  getDeviceLibs(const llvm::opt::ArgList ) const override;
 
   SanitizerMask getSupportedSanitizers() const override;
 
Index: clang/lib/Driver/ToolChains/HIPSPV.cpp
===
--- clang/lib/Driver/ToolChains/HIPSPV.cpp
+++ clang/lib/Driver/ToolChains/HIPSPV.cpp
@@ -154,7 +154,7 @@
 CC1Args.append(
 {"-fvisibility=hidden", "-fapply-global-visibility-to-externs"});
 
-  llvm::for_each(getHIPDeviceLibs(DriverArgs),
+  llvm::for_each(getDeviceLibs(DriverArgs),
  [&](const BitCodeLibraryInfo ) {
CC1Args.append({"-mlink-builtin-bitcode",
DriverArgs.MakeArgString(BCFile.Path)});
@@ -206,7 +206,7 @@
 }
 
 llvm::SmallVector
-HIPSPVToolChain::getHIPDeviceLibs(const llvm::opt::ArgList ) const {
+HIPSPVToolChain::getDeviceLibs(const llvm::opt::ArgList ) const {
   llvm::SmallVector BCLibs;
   if (DriverArgs.hasArg(options::OPT_nogpulib))
 return {};
Index: clang/lib/Driver/ToolChains/HIPAMD.h
===
--- clang/lib/Driver/ToolChains/HIPAMD.h
+++ clang/lib/Driver/ToolChains/HIPAMD.h
@@ -76,7 +76,7 @@
   void AddHIPIncludeArgs(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ) const override;
   llvm::SmallVector
-  getHIPDeviceLibs(const llvm::opt::ArgList ) const override;
+  getDeviceLibs(const llvm::opt::ArgList ) const override;
 
   SanitizerMask getSupportedSanitizers() const override;
 
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- 

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

2022-09-13 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments.



Comment at: clang/include/clang/Driver/ToolChain.h:719
   virtual llvm::SmallVector
-  getHIPDeviceLibs(const llvm::opt::ArgList ) const;
+  getAMDGPUDeviceLibs(const llvm::opt::ArgList ) const;
 

jhuber6 wrote:
> yaxunl wrote:
> > well, HIPSPV toolchain is not for AMDGPU. My thinking is that this API is 
> > to get device libraries for the toolchain, not for a specific target or 
> > platform.
> I guess the problem is that this option can't be totally generic since it's 
> in the top-level `Toolchain` namespace. Otherwise it would get confused with 
> CUDA's device library. What's a good common name to use for it?
This API return device libs for the toolchain.

For AMDGPU, HIPAMD, AMDGPUOpenMP toolchains, it returns device libs for AMDGPU.

For HIPSPV toolchain, it returns device libs for HIP for Intel GPUs.

If a CUDA toolchain decides to support this API, it will return device libs for 
CUDA.

If a toolchain needs to return device libs for multiple targets, we can add 
parameters to control that, although so far there is no such need.

Therefore, I don't see issue to call it getDeviceLibs.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133726

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


[clang] b340c5a - [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash`

2022-09-13 Thread Argyrios Kyrtzidis via cfe-commits

Author: Argyrios Kyrtzidis
Date: 2022-09-13T15:48:50-07:00
New Revision: b340c5ae4221a9752712621cd1df06cbc6dfd50b

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

LOG: [Lex/DependencyDirectivesScanner] Handle the case where the source line 
starts with a `tok::hashhash`

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

Added: 


Modified: 
clang/lib/Lex/DependencyDirectivesScanner.cpp
clang/unittests/Lex/DependencyDirectivesScannerTest.cpp

Removed: 




diff  --git a/clang/lib/Lex/DependencyDirectivesScanner.cpp 
b/clang/lib/Lex/DependencyDirectivesScanner.cpp
index c0b1687b2847f..5d280079b882e 100644
--- a/clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ b/clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -742,6 +742,14 @@ bool Scanner::lexPPLine(const char *, const char 
*const End) {
 
   // Lex '#'.
   const dependency_directives_scan::Token  = lexToken(First, End);
+  if (HashTok.is(tok::hashhash)) {
+// A \p tok::hashhash at this location is passed by the preprocessor to the
+// parser to interpret, like any other token. So for dependency scanning
+// skip it like a normal token not affecting the preprocessor.
+skipLine(First, End);
+assert(First <= End);
+return false;
+  }
   assert(HashTok.is(tok::hash));
   (void)HashTok;
 

diff  --git a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp 
b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
index 7ff09dc5d9c5b..5222df9fb9eb2 100644
--- a/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ b/clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -124,6 +124,21 @@ TEST(MinimizeSourceToDependencyDirectivesTest, EmptyHash) {
   EXPECT_STREQ("#define MACRO a\n", Out.data());
 }
 
+TEST(MinimizeSourceToDependencyDirectivesTest, HashHash) {
+  SmallVector Out;
+
+  StringRef Source = R"(
+#define S
+#if 0
+  ##pragma cool
+  ##include "t.h"
+#endif
+#define E
+)";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ("#define S\n#if 0\n#endif\n#define E\n", Out.data());
+}
+
 TEST(MinimizeSourceToDependencyDirectivesTest, Define) {
   SmallVector Out;
   SmallVector Tokens;



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


[PATCH] D133674: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash`

2022-09-13 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb340c5ae4221: [Lex/DependencyDirectivesScanner] Handle the 
case where the source line starts… (authored by akyrtzi).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133674

Files:
  clang/lib/Lex/DependencyDirectivesScanner.cpp
  clang/unittests/Lex/DependencyDirectivesScannerTest.cpp


Index: clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -124,6 +124,21 @@
   EXPECT_STREQ("#define MACRO a\n", Out.data());
 }
 
+TEST(MinimizeSourceToDependencyDirectivesTest, HashHash) {
+  SmallVector Out;
+
+  StringRef Source = R"(
+#define S
+#if 0
+  ##pragma cool
+  ##include "t.h"
+#endif
+#define E
+)";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ("#define S\n#if 0\n#endif\n#define E\n", Out.data());
+}
+
 TEST(MinimizeSourceToDependencyDirectivesTest, Define) {
   SmallVector Out;
   SmallVector Tokens;
Index: clang/lib/Lex/DependencyDirectivesScanner.cpp
===
--- clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -742,6 +742,14 @@
 
   // Lex '#'.
   const dependency_directives_scan::Token  = lexToken(First, End);
+  if (HashTok.is(tok::hashhash)) {
+// A \p tok::hashhash at this location is passed by the preprocessor to the
+// parser to interpret, like any other token. So for dependency scanning
+// skip it like a normal token not affecting the preprocessor.
+skipLine(First, End);
+assert(First <= End);
+return false;
+  }
   assert(HashTok.is(tok::hash));
   (void)HashTok;
 


Index: clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -124,6 +124,21 @@
   EXPECT_STREQ("#define MACRO a\n", Out.data());
 }
 
+TEST(MinimizeSourceToDependencyDirectivesTest, HashHash) {
+  SmallVector Out;
+
+  StringRef Source = R"(
+#define S
+#if 0
+  ##pragma cool
+  ##include "t.h"
+#endif
+#define E
+)";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ("#define S\n#if 0\n#endif\n#define E\n", Out.data());
+}
+
 TEST(MinimizeSourceToDependencyDirectivesTest, Define) {
   SmallVector Out;
   SmallVector Tokens;
Index: clang/lib/Lex/DependencyDirectivesScanner.cpp
===
--- clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -742,6 +742,14 @@
 
   // Lex '#'.
   const dependency_directives_scan::Token  = lexToken(First, End);
+  if (HashTok.is(tok::hashhash)) {
+// A \p tok::hashhash at this location is passed by the preprocessor to the
+// parser to interpret, like any other token. So for dependency scanning
+// skip it like a normal token not affecting the preprocessor.
+skipLine(First, End);
+assert(First <= End);
+return false;
+  }
   assert(HashTok.is(tok::hash));
   (void)HashTok;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

2022-09-13 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added inline comments.



Comment at: clang/include/clang/Driver/ToolChain.h:719
   virtual llvm::SmallVector
-  getHIPDeviceLibs(const llvm::opt::ArgList ) const;
+  getAMDGPUDeviceLibs(const llvm::opt::ArgList ) const;
 

yaxunl wrote:
> well, HIPSPV toolchain is not for AMDGPU. My thinking is that this API is to 
> get device libraries for the toolchain, not for a specific target or platform.
I guess the problem is that this option can't be totally generic since it's in 
the top-level `Toolchain` namespace. Otherwise it would get confused with 
CUDA's device library. What's a good common name to use for it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133726

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


[PATCH] D132997: [clang][Interp] Handle DeclRefExpr of reference types

2022-09-13 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added a comment.

It breaks the bot https://lab.llvm.org/buildbot/#/builders/5/builds/27410


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132997

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


[PATCH] D133711: [Sema] Reject array element types whose alignments are larger than their sizes

2022-09-13 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

No, I was thinking perhaps clang has to round up the size to the alignment when 
the target is 32-bit MSVC too. The code in https://godbolt.org/z/c1EzdYqPc is 
perfectly valid so I don't think we want to reject it, but we don't want to 
keep emitting buggy code either.

I was just wondering whether we'd run into problems if clang started behaving 
differently than MSVC (i.e., increase `T2`'s size to 8B from 5B).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133711

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


[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

2022-09-13 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments.



Comment at: clang/include/clang/Driver/ToolChain.h:719
   virtual llvm::SmallVector
-  getHIPDeviceLibs(const llvm::opt::ArgList ) const;
+  getAMDGPUDeviceLibs(const llvm::opt::ArgList ) const;
 

well, HIPSPV toolchain is not for AMDGPU. My thinking is that this API is to 
get device libraries for the toolchain, not for a specific target or platform.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133726

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


[PATCH] D133092: [clang] fix generation of .debug_aranges with LTO

2022-09-13 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment.

In D133092#3787379 , @azat wrote:

>> the usual flow would be once you have approval to go ahead and submit the 
>> patch yourself - I take it you don't have commit access?
>
> Yeah, I don't
> I though that only trusted/experienced/... llvm devs can commit.

~sort of. The process is described here: 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access

But generally not every developer is immediately aware of who has/doesn't have 
commit access - so if your patch gets approved and you can't commit it 
yourself, please follow up on the review/ask that someone commits it for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133092

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


[PATCH] D133092: [clang] fix generation of .debug_aranges with LTO

2022-09-13 Thread David Blaikie via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6bf6730ac55e: [clang] fix generation of .debug_aranges with 
LTO (authored by azat, committed by dblaikie).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133092

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/debug-options.c


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -246,7 +246,11 @@
 // RUN: %clang -### -c -glldb %s 2>&1 | FileCheck -check-prefix=NOPUB %s
 // RUN: %clang -### -c -glldb -gno-pubnames %s 2>&1 | FileCheck 
-check-prefix=NOPUB %s
 //
-// RUN: %clang -### -c -gdwarf-aranges %s 2>&1 | FileCheck 
-check-prefix=GARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -c -gdwarf-aranges %s 2>&1 | 
FileCheck -check-prefix=GARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -flto -gdwarf-aranges %s 2>&1 
| FileCheck -check-prefix=LDGARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -flto=thin -gdwarf-aranges %s 
2>&1 | FileCheck -check-prefix=LDGARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -fuse-ld=lld -flto 
-gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=LLDGARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -fuse-ld=lld -flto=thin 
-gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=LLDGARANGE %s
 //
 // RUN: %clang -### -fdebug-types-section -target x86_64-unknown-linux %s 2>&1 
\
 // RUN:| FileCheck -check-prefix=FDTS %s
@@ -371,6 +375,8 @@
 // NORNGBSE-NOT: -fdebug-ranges-base-address
 //
 // GARANGE-DAG: -generate-arange-section
+// LDGARANGE-NOT: {{".*lld.*"}} {{.*}} "-generate-arange-section"
+// LLDGARANGE: {{".*lld.*"}} {{.*}} "-generate-arange-section"
 //
 // FDTS: "-mllvm" "-generate-type-units"
 // FDTSE: error: unsupported option '-fdebug-types-section' for target 
'x86_64-apple-darwin'
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -506,6 +506,19 @@
 Suffix,
 Plugin);
 CmdArgs.push_back(Args.MakeArgString(Plugin));
+  } else {
+// NOTE:
+// - it is not possible to use lld for PS4
+// - addLTOOptions() is not used for PS5
+// Hence no need to handle SCE (like in Clang.cpp::renderDebugOptions()).
+//
+// But note, this solution is far from perfect, better to encode it into IR
+// metadata, but this may not be worth it, since it looks like aranges is
+// on the way out.
+if (Args.hasArg(options::OPT_gdwarf_aranges)) {
+  CmdArgs.push_back(Args.MakeArgString("-mllvm"));
+  CmdArgs.push_back(Args.MakeArgString("-generate-arange-section"));
+}
   }
 
   // Try to pass driver level flags relevant to LTO code generation down to


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -246,7 +246,11 @@
 // RUN: %clang -### -c -glldb %s 2>&1 | FileCheck -check-prefix=NOPUB %s
 // RUN: %clang -### -c -glldb -gno-pubnames %s 2>&1 | FileCheck -check-prefix=NOPUB %s
 //
-// RUN: %clang -### -c -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=GARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -c -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=GARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -flto -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=LDGARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -flto=thin -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=LDGARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -fuse-ld=lld -flto -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=LLDGARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -fuse-ld=lld -flto=thin -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=LLDGARANGE %s
 //
 // RUN: %clang -### -fdebug-types-section -target x86_64-unknown-linux %s 2>&1 \
 // RUN:| FileCheck -check-prefix=FDTS %s
@@ -371,6 +375,8 @@
 // NORNGBSE-NOT: -fdebug-ranges-base-address
 //
 // GARANGE-DAG: -generate-arange-section
+// LDGARANGE-NOT: {{".*lld.*"}} {{.*}} "-generate-arange-section"
+// LLDGARANGE: {{".*lld.*"}} {{.*}} "-generate-arange-section"
 //
 // FDTS: "-mllvm" "-generate-type-units"
 // FDTSE: error: unsupported option '-fdebug-types-section' for target 'x86_64-apple-darwin'
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -506,6 +506,19 @@
 Suffix,
 Plugin);
 CmdArgs.push_back(Args.MakeArgString(Plugin));
+  } else {
+// NOTE:
+// 

[clang] 6bf6730 - [clang] fix generation of .debug_aranges with LTO

2022-09-13 Thread David Blaikie via cfe-commits

Author: Azat Khuzhin
Date: 2022-09-13T22:33:56Z
New Revision: 6bf6730ac55e064edf46915ebba02e9c716f48e8

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

LOG: [clang] fix generation of .debug_aranges with LTO

Right now in case of LTO the section is not emited:

$ cat test.c
void __attribute__((optnone)) bar()
{
}
void __attribute__((optnone)) foo()
{
bar();
}
int main()
{
foo();
}

$ clang -flto=thin -gdwarf-aranges -g -O3 test.c
$ eu-readelf -waranges a.out  | fgrep -c -e foo -e bar
0

$ clang -gdwarf-aranges -g -O3 test.c
$ eu-readelf -waranges a.out  | fgrep -c -e foo -e bar
2

Fix this by passing explicitly -mllvm -generate-arange-section.

P.S. although this looks like a hack, since none of -mllvm was passed to
the lld before.

Signed-off-by: Azat Khuzhin 
Suggested-by: OCHyams 

Reviewed By: dblaikie

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/test/Driver/debug-options.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp 
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 22025d95e7c8f..c61fcfc946b89 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -506,6 +506,19 @@ void tools::addLTOOptions(const ToolChain , 
const ArgList ,
 Suffix,
 Plugin);
 CmdArgs.push_back(Args.MakeArgString(Plugin));
+  } else {
+// NOTE:
+// - it is not possible to use lld for PS4
+// - addLTOOptions() is not used for PS5
+// Hence no need to handle SCE (like in Clang.cpp::renderDebugOptions()).
+//
+// But note, this solution is far from perfect, better to encode it into IR
+// metadata, but this may not be worth it, since it looks like aranges is
+// on the way out.
+if (Args.hasArg(options::OPT_gdwarf_aranges)) {
+  CmdArgs.push_back(Args.MakeArgString("-mllvm"));
+  CmdArgs.push_back(Args.MakeArgString("-generate-arange-section"));
+}
   }
 
   // Try to pass driver level flags relevant to LTO code generation down to

diff  --git a/clang/test/Driver/debug-options.c 
b/clang/test/Driver/debug-options.c
index 04004716aa501..2da192d098e24 100644
--- a/clang/test/Driver/debug-options.c
+++ b/clang/test/Driver/debug-options.c
@@ -246,7 +246,11 @@
 // RUN: %clang -### -c -glldb %s 2>&1 | FileCheck -check-prefix=NOPUB %s
 // RUN: %clang -### -c -glldb -gno-pubnames %s 2>&1 | FileCheck 
-check-prefix=NOPUB %s
 //
-// RUN: %clang -### -c -gdwarf-aranges %s 2>&1 | FileCheck 
-check-prefix=GARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -c -gdwarf-aranges %s 2>&1 | 
FileCheck -check-prefix=GARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -flto -gdwarf-aranges %s 2>&1 
| FileCheck -check-prefix=LDGARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -flto=thin -gdwarf-aranges %s 
2>&1 | FileCheck -check-prefix=LDGARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -fuse-ld=lld -flto 
-gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=LLDGARANGE %s
+// RUN: %clang -### -target x86_64-unknown-linux -fuse-ld=lld -flto=thin 
-gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=LLDGARANGE %s
 //
 // RUN: %clang -### -fdebug-types-section -target x86_64-unknown-linux %s 2>&1 
\
 // RUN:| FileCheck -check-prefix=FDTS %s
@@ -371,6 +375,8 @@
 // NORNGBSE-NOT: -fdebug-ranges-base-address
 //
 // GARANGE-DAG: -generate-arange-section
+// LDGARANGE-NOT: {{".*lld.*"}} {{.*}} "-generate-arange-section"
+// LLDGARANGE: {{".*lld.*"}} {{.*}} "-generate-arange-section"
 //
 // FDTS: "-mllvm" "-generate-type-units"
 // FDTSE: error: unsupported option '-fdebug-types-section' for target 
'x86_64-apple-darwin'



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


[PATCH] D133674: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash`

2022-09-13 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 accepted this revision.
jansvoboda11 added a comment.
This revision is now accepted and ready to land.

Thank you! LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133674

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


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

2022-09-13 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam added inline comments.



Comment at: clang/docs/UsersManual.rst:1453-1455
+   Note: ``DenormalFPMath`` and ``DenormalFP32Math`` are set by default to IEEE
+   (no flush) for ``-fno-fast-math``, ``-fno-unsafe-math-optimizations``, and
+   any setting of ``fp-model``. Clang does enable flush-to-zero when

jcranmer-intel wrote:
> You can replace this text with saying that `-fno-fast-math` implies 
> `-fdenormal-fp-math=ieee`. No need to directly mention `DenormalFPMath`; 
> instead relate it to the other command line flags that are documented.
Not sure that's the change you are proposing?


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

https://reviews.llvm.org/D123630

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


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

2022-09-13 Thread Zahira Ammarguellat via Phabricator via cfe-commits
zahiraam updated this revision to Diff 459893.
zahiraam marked 2 inline comments as done.

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

https://reviews.llvm.org/D123630

Files:
  clang/docs/UsersManual.rst
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/ffp-contract-option.c
  clang/test/Driver/fp-contract.c

Index: clang/test/Driver/fp-contract.c
===
--- /dev/null
+++ clang/test/Driver/fp-contract.c
@@ -0,0 +1,114 @@
+// Test that -ffp-contract is set to the right value when combined with
+// the options -ffast-math and -fno-fast-math.
+
+// RUN: %clang -### -ffast-math -c %s 2>&1  \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+// CHECK-FPC-FAST: "-ffp-contract=fast"
+
+// RUN: %clang -### -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+// CHECK-FPC-ON:   "-ffp-contract=on"
+
+// RUN: %clang -### -ffast-math -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+// CHECK-FPC-OFF:  "-ffp-contract=off"
+
+// RUN: %clang -### -ffast-math -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=fast -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+// RUN: %clang -### -ffp-contract=on -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+// RUN: %clang -### -ffp-contract=off -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=fast -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+// RUN: %clang -### -ffp-contract=on -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+// RUN: %clang -### -ffp-contract=off -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+
+// RUN: %clang -### -ffast-math -ffp-contract=fast -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=on -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=on -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=off -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=off -ffp-contract=fast \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=on -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=off -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=fast -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffast-math -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -fno-fast-math -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -fno-fast-math -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -fno-fast-math -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clang -### -fno-fast-math -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=fast -fno-fast-math -ffp-contract=on \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffp-contract=fast -fno-fast-math -ffp-contract=off \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clang -### -ffp-contract=off -fno-fast-math -ffp-contract=fast \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=off -fno-fast-math -ffp-contract=on \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffp-contract=on -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=off -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=fast -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=on -ffast-math -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffp-contract=off -ffast-math -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: 

[PATCH] D133092: [clang] fix generation of .debug_aranges with LTO

2022-09-13 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment.

> we don't use addLTOOptions() for either PS4 or PS5, so this patch doesn't 
> affect (or help) them. I'm okay with saying we'll need to deal with that case 
> separately.

FTR, I've raised an internal ticket about this, and someone from Sony will 
handle it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133092

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


[PATCH] D133674: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash`

2022-09-13 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi added a comment.

In D133674#3784710 , @akyrtzi wrote:

> In D133674#3784602 , @jansvoboda11 
> wrote:
>
>> Could you explain why this is necessary and even correct? I'd expect Clang 
>> to give an error when seeing `##` in this position, and I'd expect the 
>> scanner to do the same.
>
> `##` is lexed as `tok::hashhash`; it is ignored by the preprocessor (it is 
> not treated as the start of a preprocessor directive) and passed on to the 
> parser to interpret, like any other token.

@jansvoboda11 I've added a comment in code to make it clear why it skips.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133674

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


[PATCH] D133674: [Lex/DependencyDirectivesScanner] Handle the case where the source line starts with a `tok::hashhash`

2022-09-13 Thread Argyrios Kyrtzidis via Phabricator via cfe-commits
akyrtzi updated this revision to Diff 459888.
akyrtzi added a comment.

Add comment to clarify why we skip if `tok::hashhash` is encountered.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133674

Files:
  clang/lib/Lex/DependencyDirectivesScanner.cpp
  clang/unittests/Lex/DependencyDirectivesScannerTest.cpp


Index: clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -124,6 +124,21 @@
   EXPECT_STREQ("#define MACRO a\n", Out.data());
 }
 
+TEST(MinimizeSourceToDependencyDirectivesTest, HashHash) {
+  SmallVector Out;
+
+  StringRef Source = R"(
+#define S
+#if 0
+  ##pragma cool
+  ##include "t.h"
+#endif
+#define E
+)";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ("#define S\n#if 0\n#endif\n#define E\n", Out.data());
+}
+
 TEST(MinimizeSourceToDependencyDirectivesTest, Define) {
   SmallVector Out;
   SmallVector Tokens;
Index: clang/lib/Lex/DependencyDirectivesScanner.cpp
===
--- clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -742,6 +742,14 @@
 
   // Lex '#'.
   const dependency_directives_scan::Token  = lexToken(First, End);
+  if (HashTok.is(tok::hashhash)) {
+// A \p tok::hashhash at this location is passed by the preprocessor to the
+// parser to interpret, like any other token. So for dependency scanning
+// skip it like a normal token not affecting the preprocessor.
+skipLine(First, End);
+assert(First <= End);
+return false;
+  }
   assert(HashTok.is(tok::hash));
   (void)HashTok;
 


Index: clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
===
--- clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
+++ clang/unittests/Lex/DependencyDirectivesScannerTest.cpp
@@ -124,6 +124,21 @@
   EXPECT_STREQ("#define MACRO a\n", Out.data());
 }
 
+TEST(MinimizeSourceToDependencyDirectivesTest, HashHash) {
+  SmallVector Out;
+
+  StringRef Source = R"(
+#define S
+#if 0
+  ##pragma cool
+  ##include "t.h"
+#endif
+#define E
+)";
+  ASSERT_FALSE(minimizeSourceToDependencyDirectives(Source, Out));
+  EXPECT_STREQ("#define S\n#if 0\n#endif\n#define E\n", Out.data());
+}
+
 TEST(MinimizeSourceToDependencyDirectivesTest, Define) {
   SmallVector Out;
   SmallVector Tokens;
Index: clang/lib/Lex/DependencyDirectivesScanner.cpp
===
--- clang/lib/Lex/DependencyDirectivesScanner.cpp
+++ clang/lib/Lex/DependencyDirectivesScanner.cpp
@@ -742,6 +742,14 @@
 
   // Lex '#'.
   const dependency_directives_scan::Token  = lexToken(First, End);
+  if (HashTok.is(tok::hashhash)) {
+// A \p tok::hashhash at this location is passed by the preprocessor to the
+// parser to interpret, like any other token. So for dependency scanning
+// skip it like a normal token not affecting the preprocessor.
+skipLine(First, End);
+assert(First <= End);
+return false;
+  }
   assert(HashTok.is(tok::hash));
   (void)HashTok;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D133711: [Sema] Reject array element types whose alignments are larger than their sizes

2022-09-13 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

Oh, so you mean we need to consider whether we need to avoid emitting an error 
message when we're dealing with C++ code targeting i686-pc-win32, given we 
implement the alignment quirk.  I think I'd like to try printing an error 
message, given it's reasonably likely you'll actually end up with a miscompile 
if you use an array like that.  This might cause breakage, but I don't see a 
good alternative.  (We don't try to mitigate the possibility of misaligned 
operations in clang, so LLVM will see misaligned operations, so LLVM 
optimizations will likely mess up the code.  MSVC gets away with this because 
it doesn't really do alignment-based optimizations.)

For the gcc 10 vs. 11 thing, not sure how many people will notice.  The most 
likely possibility here is that someone tried to align a struct, but put the 
attribute in the wrong place.  Maybe we want a diagnostic note to point out 
this usage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133711

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


[PATCH] D133586: [clang] initialize type qualifiers for FunctionNoProtoType

2022-09-13 Thread Richard Howell via Phabricator via cfe-commits
rmaz added a comment.

In D133586#3781536 , @rtrieu wrote:

> In D133586#3781468 , @rmaz wrote:
>
>> In D133586#3781381 , @rtrieu wrote:
>>
>>> Do you have a test case for this?
>>
>> Was struggling to think of a good one. How about a test that repeatedly 
>> generates a pcm for a func decl with no params and checks if the 
>> DECL_FUNCTION record is the same?
>
> Have you looked at clang/test/Modules/odr_hash.cpp?  It's where most of the 
> ODR hash testing takes place by testing that Decls can be merged properly 
> instead of checking the contents of pcm files..  Using `#if define`, it 
> creates multiple modules from the same file.  I would suggest creating two 
> functions in each of the modules, then in the main file, using the function 
> to force it to be loaded from the modules and merged together.  The test 
> should fail with the current Clang, but pass with your patch.  You may need 
> to create your test file if you need different compiler options.

I took a look at writing a test to cover this, but hit the following problem: 
function qualifiers are only valid on c++ members, and there we cannot create 
FunctionNoProtoTypes. I couldn't think of a way of testing this by comparing 
module output that would fail deterministically, as it relies on reusing 
uninitialized memory that is non-zero.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133586

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


[PATCH] D133586: [clang] do not hash undefined qualifiers for FunctionNoProtoType

2022-09-13 Thread Richard Howell via Phabricator via cfe-commits
rmaz updated this revision to Diff 459884.
rmaz added a comment.

zero out qual types in constructor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133586

Files:
  clang/include/clang/AST/Type.h


Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -1647,6 +1647,7 @@
   /// Only common bits are stored here. Additional uncommon bits are stored
   /// in a trailing object after FunctionProtoType.
   class FunctionTypeBitfields {
+friend class FunctionNoProtoType;
 friend class FunctionProtoType;
 friend class FunctionType;
 
@@ -3926,7 +3927,9 @@
  Result->getDependence() &
  ~(TypeDependence::DependentInstantiation |
TypeDependence::UnexpandedPack),
- Info) {}
+ Info) {
+FunctionTypeBits.FastTypeQuals = 0;
+  }
 
 public:
   // No additional state past what FunctionType provides.


Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -1647,6 +1647,7 @@
   /// Only common bits are stored here. Additional uncommon bits are stored
   /// in a trailing object after FunctionProtoType.
   class FunctionTypeBitfields {
+friend class FunctionNoProtoType;
 friend class FunctionProtoType;
 friend class FunctionType;
 
@@ -3926,7 +3927,9 @@
  Result->getDependence() &
  ~(TypeDependence::DependentInstantiation |
TypeDependence::UnexpandedPack),
- Info) {}
+ Info) {
+FunctionTypeBits.FastTypeQuals = 0;
+  }
 
 public:
   // No additional state past what FunctionType provides.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D128845: [HLSL]Add -O and -Od option for dxc mode.

2022-09-13 Thread Xiang Li via Phabricator via cfe-commits
python3kgae updated this revision to Diff 459883.
python3kgae marked an inline comment as done.
python3kgae added a comment.

Cleanup per comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128845

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/HLSL.cpp
  clang/test/CodeGenHLSL/disable_opt.hlsl
  clang/test/Driver/dxc_O.hlsl
  clang/test/Driver/dxc_fcgl.hlsl

Index: clang/test/Driver/dxc_fcgl.hlsl
===
--- clang/test/Driver/dxc_fcgl.hlsl
+++ clang/test/Driver/dxc_fcgl.hlsl
@@ -1,5 +1,6 @@
 // RUN: %clang_dxc -fcgl -T lib_6_7 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"
+// CHECK:"-S"
+// CHECK-SAME:"-emit-llvm" "-disable-llvm-passes"
 
Index: clang/test/Driver/dxc_O.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_O.hlsl
@@ -0,0 +1,18 @@
+// RUN: %clang_dxc -T lib_6_7  foo.hlsl -### %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -T lib_6_7 -Od foo.hlsl -### %s 2>&1 | FileCheck %s --check-prefix=Od
+// RUN: %clang_dxc -T lib_6_7 -O0 foo.hlsl -### %s 2>&1 | FileCheck %s --check-prefix=O0
+// RUN: %clang_dxc -T lib_6_7 -O1 foo.hlsl -### %s 2>&1 | FileCheck %s --check-prefix=O1
+// RUN: %clang_dxc -T lib_6_7 -O2 foo.hlsl -### %s 2>&1 | FileCheck %s --check-prefix=O2
+// RUN: %clang_dxc -T lib_6_7 -O3 foo.hlsl -### %s 2>&1 | FileCheck %s --check-prefix=O3
+
+// Make sure default is O3.
+// CHECK: "-O3"
+
+// Make sure Od/O0 option flag which translated into "-O0".
+// Od: "-O0"
+// O0: "-O0"
+
+// Make sure O1/O2/O3 is send to cc1.
+// O1: "-O1"
+// O2: "-O2"
+// O3: "-O3"
Index: clang/test/CodeGenHLSL/disable_opt.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/disable_opt.hlsl
@@ -0,0 +1,12 @@
+// RUN: %clang -cc1 -S -triple dxil-pc-shadermodel6.3-library -O0 -emit-llvm -xhlsl  -o - %s | FileCheck %s
+// RUN: %clang -cc1 -S -triple dxil-pc-shadermodel6.3-library -O3 -emit-llvm -xhlsl  -o - %s | FileCheck %s --check-prefix=OPT
+
+// CHECK:!"dx.disable_optimizations", i32 1}
+
+// OPT-NOT:"dx.disable_optimizations"
+
+float bar(float a, float b);
+
+float foo(float a, float b) {
+  return bar(a, b);
+}
Index: clang/lib/Driver/ToolChains/HLSL.cpp
===
--- clang/lib/Driver/ToolChains/HLSL.cpp
+++ clang/lib/Driver/ToolChains/HLSL.cpp
@@ -164,6 +164,18 @@
   A->claim();
   continue;
 }
+if (A->getOption().getID() == options::OPT__SLASH_O) {
+  StringRef OStr = A->getValue();
+  if (OStr == "d") {
+DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_O0));
+A->claim();
+continue;
+  } else {
+DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_O), OStr);
+A->claim();
+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));
@@ -192,6 +204,9 @@
 Opts.getOption(options::OPT_dxil_validator_version),
 DefaultValidatorVer);
   }
+  if (!DAL->hasArg(options::OPT_O_Group)) {
+DAL->AddJoinedArg(nullptr, Opts.getOption(options::OPT_O), "3");
+  }
   // 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
@@ -3520,6 +3520,7 @@
  options::OPT_D,
  options::OPT_I,
  options::OPT_S,
+ options::OPT_O,
  options::OPT_emit_llvm,
  options::OPT_emit_obj,
  options::OPT_disable_llvm_passes,
Index: clang/lib/CodeGen/CGHLSLRuntime.cpp
===
--- clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -46,6 +46,11 @@
   auto *DXILValMD = M.getOrInsertNamedMetadata(DXILValKey);
   DXILValMD->addOperand(Val);
 }
+void addDisableOptimizations(llvm::Module ) {
+  StringRef Key = "dx.disable_optimizations";
+  M.addModuleFlag(llvm::Module::ModFlagBehavior::Override, Key, 1);
+}
+
 } // namespace
 
 void CGHLSLRuntime::finishCodeGen() {
@@ -56,6 +61,8 

[PATCH] D133804: Cuda Check for ignored return errors from api calls to cuda

2022-09-13 Thread Artem Belevich via Phabricator via cfe-commits
tra added inline comments.



Comment at: clang-tools-extra/test/clang-tidy/check_clang_tidy.py:121
 
+# Tests should not rely on a certain cuda device being available on the 
machine,
+# or a certain version of it

The comment is incorrect. These flags have nothing to do with GPU availability, 
but rather with CUDA SDK which is normally expected to provide the 'standard' 
set of CUDA headers and libdevice bitcode.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu:1
+// (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary.
+

This does not look right. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133804

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


[PATCH] D133375: [CMake] Remove CLANG_DEFAULT_STD_C/CLANG_DEFAULT_STD_CXX

2022-09-13 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

In D133375#3774293 , @aaron.ballman 
wrote:

> So the basic idea here is that the default can be specified by a 
> configuration file and thus we don't need any configure-time variable for it? 
> But then why do we need other configuration macros like default stdlib or 
> default linker? Mostly trying to understand what the rule of thumb is for 
> when something should be a configure macro and when something should be left 
> to a configuration file instead.

Also, until https://reviews.llvm.org/D109621 or similar is resolved, I don't 
feel like the actual need is addressed. Forcing distributions to use a prefixed 
Clang binary feels like a hack.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133375

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


[PATCH] D133771: Add a "Potentially Breaking Changes" section to the Clang release notes

2022-09-13 Thread Sam James via Phabricator via cfe-commits
thesamesam added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:52
+  into an error-only diagnostic in the next Clang release. Fixes
+  `Issue 50055: `_.
+- ``-Wincompatible-function-pointer-types`` now defaults to an error in all C

Reflecting on this a bit, do wonder about explicitly calling out configure 
scripts, as I suspect it's something a lot of people may not even think of.

"The LLVM team recommends that projects using configure scripts verify the 
results do not change before/after setting 
`-Werror=implicit-function-declarations` (repeat for others) to avoid 
incompatibility with Clang 16."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133771

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


[PATCH] D133800: [Clang 15.0.1] Downgrade implicit int and implicit function declaration to warning only

2022-09-13 Thread James Y Knight via Phabricator via cfe-commits
jyknight accepted this revision.
jyknight added a comment.
This revision is now accepted and ready to land.

Looks correct to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133800

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


[PATCH] D133807: Update Unicode to 15.0

2022-09-13 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin updated this revision to Diff 459877.
cor3ntin added a comment.

Changelog


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133807

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Lex/UnicodeCharSets.h
  llvm/lib/Support/Unicode.cpp
  llvm/lib/Support/UnicodeCaseFold.cpp
  llvm/lib/Support/UnicodeNameToCodepoint.cpp
  llvm/lib/Support/UnicodeNameToCodepointGenerated.cpp
  llvm/utils/UnicodeData/UnicodeNameMappingGenerator.cpp

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


[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

2022-09-13 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 459874.
jhuber6 added a comment.

Changing interface to `getAMDGPUDeviceLibs`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133726

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/lib/Driver/ToolChains/HIPAMD.h
  clang/lib/Driver/ToolChains/HIPSPV.cpp
  clang/lib/Driver/ToolChains/HIPSPV.h
  clang/test/Driver/amdgpu-openmp-toolchain.c

Index: clang/test/Driver/amdgpu-openmp-toolchain.c
===
--- clang/test/Driver/amdgpu-openmp-toolchain.c
+++ clang/test/Driver/amdgpu-openmp-toolchain.c
@@ -49,5 +49,12 @@
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -emit-llvm -S -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx803 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-EMIT-LLVM-IR
 // CHECK-EMIT-LLVM-IR: "-cc1" "-triple" "amdgcn-amd-amdhsa"{{.*}}"-emit-llvm"
 
-// RUN: %clang -### -target x86_64-pc-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx803 -lm --rocm-device-lib-path=%S/Inputs/rocm/amdgcn/bitcode %s 2>&1 | FileCheck %s --check-prefix=CHECK-LIB-DEVICE-NEW
-// CHECK-LIB-DEVICE-NEW: {{.*}}clang-linker-wrapper{{.*}}--bitcode-library=openmp-amdgcn-amd-amdhsa-gfx803={{.*}}ocml.bc"{{.*}}ockl.bc"{{.*}}oclc_daz_opt_on.bc"{{.*}}oclc_unsafe_math_off.bc"{{.*}}oclc_finite_only_off.bc"{{.*}}oclc_correctly_rounded_sqrt_on.bc"{{.*}}oclc_wavefrontsize64_on.bc"{{.*}}oclc_isa_version_803.bc"
+// RUN: %clang -### -target x86_64-pc-linux-gnu -fopenmp --offload-arch=gfx803 \
+// RUN:   --rocm-device-lib-path=%S/Inputs/rocm/amdgcn/bitcode -fopenmp-new-driver %s  2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-LIB-DEVICE
+// CHECK-LIB-DEVICE: "-cc1" {{.*}}ocml.bc"{{.*}}ockl.bc"{{.*}}oclc_daz_opt_on.bc"{{.*}}oclc_unsafe_math_off.bc"{{.*}}oclc_finite_only_off.bc"{{.*}}oclc_correctly_rounded_sqrt_on.bc"{{.*}}oclc_wavefrontsize64_on.bc"{{.*}}oclc_isa_version_803.bc"
+
+// RUN: %clang -### -target x86_64-pc-linux-gnu -fopenmp --offload-arch=gfx803 -nogpulib \
+// RUN:   --rocm-device-lib-path=%S/Inputs/rocm/amdgcn/bitcode -fopenmp-new-driver %s  2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-LIB-DEVICE-NOGPULIB
+// CHECK-LIB-DEVICE-NOGPULIB-NOT: "-cc1" {{.*}}ocml.bc"{{.*}}ockl.bc"{{.*}}oclc_daz_opt_on.bc"{{.*}}oclc_unsafe_math_off.bc"{{.*}}oclc_finite_only_off.bc"{{.*}}oclc_correctly_rounded_sqrt_on.bc"{{.*}}oclc_wavefrontsize64_on.bc"{{.*}}oclc_isa_version_803.bc"
Index: clang/lib/Driver/ToolChains/HIPSPV.h
===
--- clang/lib/Driver/ToolChains/HIPSPV.h
+++ clang/lib/Driver/ToolChains/HIPSPV.h
@@ -69,7 +69,7 @@
   void AddHIPIncludeArgs(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ) const override;
   llvm::SmallVector
-  getHIPDeviceLibs(const llvm::opt::ArgList ) const override;
+  getAMDGPUDeviceLibs(const llvm::opt::ArgList ) const override;
 
   SanitizerMask getSupportedSanitizers() const override;
 
Index: clang/lib/Driver/ToolChains/HIPSPV.cpp
===
--- clang/lib/Driver/ToolChains/HIPSPV.cpp
+++ clang/lib/Driver/ToolChains/HIPSPV.cpp
@@ -154,7 +154,7 @@
 CC1Args.append(
 {"-fvisibility=hidden", "-fapply-global-visibility-to-externs"});
 
-  llvm::for_each(getHIPDeviceLibs(DriverArgs),
+  llvm::for_each(getAMDGPUDeviceLibs(DriverArgs),
  [&](const BitCodeLibraryInfo ) {
CC1Args.append({"-mlink-builtin-bitcode",
DriverArgs.MakeArgString(BCFile.Path)});
@@ -206,7 +206,8 @@
 }
 
 llvm::SmallVector
-HIPSPVToolChain::getHIPDeviceLibs(const llvm::opt::ArgList ) const {
+HIPSPVToolChain::getAMDGPUDeviceLibs(
+const llvm::opt::ArgList ) const {
   llvm::SmallVector BCLibs;
   if (DriverArgs.hasArg(options::OPT_nogpulib))
 return {};
Index: clang/lib/Driver/ToolChains/HIPAMD.h
===
--- clang/lib/Driver/ToolChains/HIPAMD.h
+++ clang/lib/Driver/ToolChains/HIPAMD.h
@@ -76,7 +76,7 @@
   void AddHIPIncludeArgs(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ) const override;
   llvm::SmallVector
-  getHIPDeviceLibs(const llvm::opt::ArgList ) const override;
+  getAMDGPUDeviceLibs(const llvm::opt::ArgList ) const override;
 
   SanitizerMask getSupportedSanitizers() const override;
 
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -246,7 

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

2022-09-13 Thread Joshua Cranmer via Phabricator via cfe-commits
jcranmer-intel added inline comments.



Comment at: clang/docs/UsersManual.rst:1430
+
+   * ``-ffp-contract=on``
+

You can add `-fdenormal-fp-math=ieee` here.



Comment at: clang/docs/UsersManual.rst:1453-1455
+   Note: ``DenormalFPMath`` and ``DenormalFP32Math`` are set by default to IEEE
+   (no flush) for ``-fno-fast-math``, ``-fno-unsafe-math-optimizations``, and
+   any setting of ``fp-model``. Clang does enable flush-to-zero when

You can replace this text with saying that `-fno-fast-math` implies 
`-fdenormal-fp-math=ieee`. No need to directly mention `DenormalFPMath`; 
instead relate it to the other command line flags that are documented.



Comment at: clang/docs/UsersManual.rst:1455-1458
+   any setting of ``fp-model``. Clang does enable flush-to-zero when
+   ``-fast=math`` or ``-funsafe-math-optimzations`` are used, when it is able 
to
+   find the ``cftfastmath.o``. This will affect not only the current 
compilation
+   but all static and shared libraries included in the program.

`crtfastmath.o` should probably be mentioned in a separate section, like the "A 
note about ..." sections, with the text in `-fno-fast-math` only mentioning 
that it causes code not to be linked with `crtfastmath.o`.


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

https://reviews.llvm.org/D123630

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


[PATCH] D133436: Ground work for cuda-related checks in clang-tidy

2022-09-13 Thread Bartłomiej Cieślar via Phabricator via cfe-commits
barcisz added a comment.

@tschuett does it look alright now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133436

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


[PATCH] D133711: [Sema] Reject array element types whose alignments are larger than their sizes

2022-09-13 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

In D133711#3787604 , @ahatanak wrote:

> Unlike gcc or clang. MSVC doesn't round up the size to its alignment. I don't 
> mean it should reject the code.

Sorry, my comment was misleading. clang has the same behavior as MSVC when the 
target is 32-bit (e.g., `i686-pc-win32`). The size and alignment of `T2` and 
`array` are the same. If the target isn't 32-bit MSVC, the size is correctly 
round up to the alignment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133711

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


[PATCH] D133804: Cuda Check for ignored return errors from api calls to cuda

2022-09-13 Thread Bartłomiej Cieślar via Phabricator via cfe-commits
barcisz updated this revision to Diff 459870.
barcisz added a comment.

Added some explanation comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133804

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h
  clang-tools-extra/clang-tidy/utils/Matchers.h
  clang-tools-extra/test/clang-tidy/check_clang_tidy.py
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda-initializers.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
  clang-tools-extra/test/lit.cfg.py

Index: clang-tools-extra/test/lit.cfg.py
===
--- clang-tools-extra/test/lit.cfg.py
+++ clang-tools-extra/test/lit.cfg.py
@@ -16,7 +16,7 @@
 config.test_format = lit.formats.ShTest(not llvm_config.use_lit_shell)
 
 # suffixes: A list of file extensions to treat as test files.
-config.suffixes = ['.c', '.cpp', '.hpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s',
+config.suffixes = ['.c', '.cpp', '.cu', '.hpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s',
   '.modularize', '.module-map-checker', '.test']
 
 # Test-time dependencies located in directories called 'Inputs' are excluded
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
@@ -0,0 +1,104 @@
+//===--- SlicingCheck.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
+//
+//===--===//
+
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'CUDA_HANDLER'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers -nocudalib -nocudainc -std=c++14
+#include 
+
+class DummyContainer {
+ public:
+  int* begin();
+  int* end();
+};
+
+#define DUMMY_CUDA_HANDLER(stmt) stmt
+#define CUDA_HANDLER(stmt) do {auto err = stmt;} while(0)
+#define API_CALL() do {cudaDeviceReset();} while(0)
+
+void errorCheck();
+void errorCheck(cudaError_t error);
+
+void bad() {
+  API_CALL();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  errorCheck();
+
+  if (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  while (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  do
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+  while(false);
+
+  switch (0) {
+case 0:
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  }
+
+  for(
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+;
+cudaDeviceReset()
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset()){{$}}
+  ) cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}  ) CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  for(int i : DummyContainer())
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}

[PATCH] D133635: [clang-format] Don't insert braces for loops with a null statement

2022-09-13 Thread sstwcw via Phabricator via cfe-commits
sstwcw added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:25370
+   "#if 0\n"
+   " if (a) {\n"
+   "#else\n"

Can you confirm that there is supposed to be only one space here while there 
are two in the next test?  `messUp` turns two spaces into one space.  The 
formatter doesn't change things inside the `#if 0` block, both before and after 
D133647.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133635

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


[clang] 0c9b242 - [HLSL] Adding a test change I forgot to add

2022-09-13 Thread Chris Bieneman via cfe-commits

Author: Chris Bieneman
Date: 2022-09-13T20:53:39-05:00
New Revision: 0c9b242cf0af15fa154d006cb25f20c7bf05d60a

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

LOG: [HLSL] Adding a test change I forgot to add

This test just verifies that even at -O0 the buffer subscript operators
are inlined. The original change was
fb5baffc28c8beaf790a2fb1c8a863d29020bfbe.

Added: 
clang/test/CodeGenHLSL/builtins/RWBuffer-subscript.hlsl

Modified: 


Removed: 




diff  --git a/clang/test/CodeGenHLSL/builtins/RWBuffer-subscript.hlsl 
b/clang/test/CodeGenHLSL/builtins/RWBuffer-subscript.hlsl
new file mode 100644
index 0..da8a1e538ec5e
--- /dev/null
+++ b/clang/test/CodeGenHLSL/builtins/RWBuffer-subscript.hlsl
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -emit-llvm -o - -O0 
%s | FileCheck %s
+
+RWBuffer In;
+RWBuffer Out;
+
+[numthreads(1,1,1)]
+void main(unsigned GI : SV_GroupIndex) {
+  Out[GI] = In[GI];
+}
+
+// Even at -O0 the subscript operators get inlined. The -O0 IR is a bit messy
+// and confusing to follow so the match here is pretty weak.
+
+// CHECK: define internal void @"?main@@YAXI@Z"
+// CHECK-NOT: call
+// CHECK: ret void



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


[PATCH] D133802: [OpenMP] Remove simplified device runtime handling

2022-09-13 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 marked an inline comment as done.
jhuber6 added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:1048
   CGBuilderTy  = CGF.Builder;
-  OMPBuilder.createTargetDeinit(Bld, IsSPMD, requiresFullRuntime());
+  OMPBuilder.createTargetDeinit(Bld, IsSPMD, true);
 }

jdoerfert wrote:
> jhuber6 wrote:
> > jdoerfert wrote:
> > > So, follow up for this?
> > Yes, it changes a lot of tests so I wanted to make it a separate patch.
> Talking about tests, if you pick false here less changes happen, I think.
It's mostly meaningless as I'll remove it later anyway. I figured changing it 
to `true` was closer to what this change means as we always use the full 
runtime.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133802

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


[PATCH] D133711: [Sema] Reject array element types whose alignments are larger than their sizes

2022-09-13 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

I was looking at another example. The following code is accepted by 10.3, but 
is rejected by 11.1 because the size isn't a multiple of the alignment.

  struct __attribute__((packed)) S {
char c;
int i;
  };
  
  typedef S AlignedS __attribute__((aligned(4)));
  AlignedS array[4];

https://godbolt.org/z/hcoznss1d


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133711

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


[PATCH] D133711: [Sema] Reject array element types whose alignments are larger than their sizes

2022-09-13 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

> It looks like gcc started rejecting the code relatively recently (11.1).

On what example?  The code in the commit message gives an error on gcc 5.

> Also, I found out that MSVC doesn't reject the following code:

This looks like a backwards-compatibility quirk in the 32-bit x86 MSVC ABI; 
other MSVC targets round up the size, and even adding `__declspec(align(1))` 
forces the size to something sane.  Don't know if we want to try to emulate 
this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133711

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


[PATCH] D133802: [OpenMP] Remove simplified device runtime handling

2022-09-13 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LG, two comments.




Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:1048
   CGBuilderTy  = CGF.Builder;
-  OMPBuilder.createTargetDeinit(Bld, IsSPMD, requiresFullRuntime());
+  OMPBuilder.createTargetDeinit(Bld, IsSPMD, true);
 }

jhuber6 wrote:
> jdoerfert wrote:
> > So, follow up for this?
> Yes, it changes a lot of tests so I wanted to make it a separate patch.
Talking about tests, if you pick false here less changes happen, I think.



Comment at: clang/test/OpenMP/nvptx_force_full_runtime_SPMD_codegen.cpp:9
 #ifndef HEADER
 #define HEADER
 

Delete this test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133802

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


[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

2022-09-13 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments.



Comment at: clang/include/clang/Driver/ToolChain.h:719
   virtual llvm::SmallVector
-  getHIPDeviceLibs(const llvm::opt::ArgList ) const;
+  getROCmDeviceLibs(const llvm::opt::ArgList ) const;
 

HIPSPV toolchain is not implemented on ROCm platform.

Probably we need to make it even more generic. How about getDeviceLibs ?

Also, you need to update the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133726

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


[PATCH] D133711: [Sema] Reject array element types whose alignments are larger than their sizes

2022-09-13 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

Unlike gcc or clang. MSVC doesn't round up the size to its alignment. I don't 
mean it should reject the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133711

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


[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

2022-09-13 Thread Tom Praschan via Phabricator via cfe-commits
tom-anders updated this revision to Diff 459864.
tom-anders added a comment.

Move logic to pickDeclToUse


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133664

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1630,6 +1630,26 @@
 HI.Type = "int";
 HI.Definition = "int foo";
   }},
+  {
+  R"cpp(// Function definition via using declaration
+namespace ns { 
+  void foo(); 
+}
+int main() {
+  using ns::foo;
+  ^[[foo]]();
+}
+  )cpp",
+  [](HoverInfo ) {
+HI.Name = "foo";
+HI.Kind = index::SymbolKind::Function;
+HI.NamespaceScope = "ns::";
+HI.Type = "void ()";
+HI.Definition = "void foo()";
+HI.Documentation = "";
+HI.ReturnType = "void";
+HI.Parameters = std::vector{};
+  }},
   {
   R"cpp(// Macro
 #define MACRO 0
@@ -1734,6 +1754,25 @@
 HI.Definition = "ONE";
 HI.Value = "0";
   }},
+  {
+  R"cpp(// C++20's using enum
+enum class Hello {
+  ONE, TWO, THREE,
+};
+void foo() {
+  using enum Hello;
+  Hello hello = [[O^NE]];
+}
+  )cpp",
+  [](HoverInfo ) {
+HI.Name = "ONE";
+HI.Kind = index::SymbolKind::EnumConstant;
+HI.NamespaceScope = "";
+HI.LocalScope = "Hello::";
+HI.Type = "enum Hello";
+HI.Definition = "ONE";
+HI.Value = "0";
+  }},
   {
   R"cpp(// Enumerator in anonymous enum
 enum {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -1006,6 +1006,42 @@
   HI.CallPassType.emplace(PassType);
 }
 
+template 
+const NamedDecl *
+pickDeclToUse(const llvm::SmallVector ) {
+  if (Candidates.empty())
+return nullptr;
+  if (Candidates.size() == 1)
+return Candidates.front();
+
+  // This is e.g the case for
+  // namespace ns { void foo(); }
+  // void bar() { using ns::foo; f^oo(); }
+  // One declaration in Candidates will refer to the using declaration,
+  // which isn't really useful for Hover. So use the other one,
+  // which in this example would be the actual declaration of foo.
+  if (Candidates.size() == 2) {
+if (llvm::isa(Candidates.front()))
+  return Candidates.back();
+if (llvm::isa(Candidates.back()))
+  return Candidates.front();
+  }
+
+  // For something like
+  // namespace ns { void foo(int); void foo(char) }
+  // using ns::fo
+  // template  void bar() { fo^o(T{}); }
+  // we actually want to show the using declaration,
+  // it's not clear which declaration to pick otherwise.
+  auto BaseDecls = llvm::make_filter_range(Candidates, [](const NamedDecl *D) {
+return llvm::isa(D);
+  });
+  if (std::distance(BaseDecls.begin(), BaseDecls.end()) == 1)
+return *BaseDecls.begin();
+
+  return Candidates.front();
+}
+
 } // namespace
 
 llvm::Optional getHover(ParsedAST , Position Pos,
@@ -1081,11 +1117,11 @@
   // FIXME: Fill in HighlightRange with range coming from N->ASTNode.
   auto Decls = explicitReferenceTargets(N->ASTNode, DeclRelation::Alias,
 AST.getHeuristicResolver());
-  if (!Decls.empty()) {
-HI = getHoverContents(Decls.front(), PP, Index, TB);
+  if (const auto *DeclToUse = pickDeclToUse(Decls)) {
+HI = getHoverContents(DeclToUse, PP, Index, TB);
 // Layout info only shown when hovering on the field/class itself.
-if (Decls.front() == N->ASTNode.get())
-  addLayoutInfo(*Decls.front(), *HI);
+if (DeclToUse == N->ASTNode.get())
+  addLayoutInfo(*DeclToUse, *HI);
 // Look for a close enclosing expression to show the value of.
 if (!HI->Value)
   HI->Value = printExprValue(N, AST.getASTContext());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D133802: [OpenMP] Remove simplified device runtime handling

2022-09-13 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:1048
   CGBuilderTy  = CGF.Builder;
-  OMPBuilder.createTargetDeinit(Bld, IsSPMD, requiresFullRuntime());
+  OMPBuilder.createTargetDeinit(Bld, IsSPMD, true);
 }

jdoerfert wrote:
> So, follow up for this?
Yes, it changes a lot of tests so I wanted to make it a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133802

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


[PATCH] D133802: [OpenMP] Remove simplified device runtime handling

2022-09-13 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:1048
   CGBuilderTy  = CGF.Builder;
-  OMPBuilder.createTargetDeinit(Bld, IsSPMD, requiresFullRuntime());
+  OMPBuilder.createTargetDeinit(Bld, IsSPMD, true);
 }

So, follow up for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133802

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


[clang] fb5baff - [HLSL] Mark buffer subscript operators as AlwaysInline

2022-09-13 Thread Chris Bieneman via cfe-commits

Author: Chris Bieneman
Date: 2022-09-13T20:31:29-05:00
New Revision: fb5baffc28c8beaf790a2fb1c8a863d29020bfbe

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

LOG: [HLSL] Mark buffer subscript operators as AlwaysInline

HLSL requires aggressive inlineing for resource accesses. This just
enforces that we get resource handle accesses inlined early.

Added: 


Modified: 
clang/lib/Sema/HLSLExternalSemaSource.cpp
clang/test/AST/HLSL/RWBuffer-AST.hlsl

Removed: 




diff  --git a/clang/lib/Sema/HLSLExternalSemaSource.cpp 
b/clang/lib/Sema/HLSLExternalSemaSource.cpp
index ee3aa4d42a049..969d65997cdcb 100644
--- a/clang/lib/Sema/HLSLExternalSemaSource.cpp
+++ b/clang/lib/Sema/HLSLExternalSemaSource.cpp
@@ -270,6 +270,9 @@ struct BuiltinTypeDeclBuilder {
  SourceLocation()));
 MethodDecl->setLexicalDeclContext(Record);
 MethodDecl->setAccess(AccessSpecifier::AS_public);
+MethodDecl->addAttr(AlwaysInlineAttr::CreateImplicit(
+AST, SourceRange(), AttributeCommonInfo::AS_Keyword,
+AlwaysInlineAttr::CXX11_clang_always_inline));
 Record->addDecl(MethodDecl);
 
 return *this;

diff  --git a/clang/test/AST/HLSL/RWBuffer-AST.hlsl 
b/clang/test/AST/HLSL/RWBuffer-AST.hlsl
index 193ef67e152b7..80f77f97d2ef0 100644
--- a/clang/test/AST/HLSL/RWBuffer-AST.hlsl
+++ b/clang/test/AST/HLSL/RWBuffer-AST.hlsl
@@ -49,6 +49,7 @@ RWBuffer Buffer;
 // CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' 
lvalue ->h 0x{{[0-9A-Fa-f]+}}
 // CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 'const 
RWBuffer *' implicit this
 // CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' 
ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'
+// CHECK-NEXT: AlwaysInlineAttr 0x{{[0-9A-Fa-f]+}} <> Implicit 
always_inline
 
 // CHECK-NEXT: CXXMethodDecl 0x{{[0-9A-Fa-f]+}} <>  operator[] 'element_type &(unsigned int)'
 // CHECK-NEXT: ParmVarDecl 0x{{[0-9A-Fa-f]+}} <>  
Idx 'unsigned int'
@@ -58,6 +59,7 @@ RWBuffer Buffer;
 // CHECK-NEXT: MemberExpr 0x{{[0-9A-Fa-f]+}} <> 'element_type *' 
lvalue ->h 0x{{[0-9A-Fa-f]+}}
 // CHECK-NEXT: CXXThisExpr 0x{{[0-9A-Fa-f]+}} <> 
'RWBuffer *' implicit this
 // CHECK-NEXT: DeclRefExpr 0x{{[0-9A-Fa-f]+}} <> 'unsigned int' 
ParmVar 0x{{[0-9A-Fa-f]+}} 'Idx' 'unsigned int'
+// CHECK-NEXT: AlwaysInlineAttr 0x{{[0-9A-Fa-f]+}} <> Implicit 
always_inline
 
 // CHECK: ClassTemplateSpecializationDecl 0x{{[0-9A-Fa-f]+}} <> 
 class RWBuffer definition
 



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


[PATCH] D133807: Update Unicode to 15.0

2022-09-13 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin created this revision.
Herald added subscribers: hiraditya, dschuff.
Herald added a project: All.
cor3ntin requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

Unicode 15.0 adds 4,489 characters, for a total of 149,186 characters.
These additions include 2 new scripts along with 20 new emoji characters,
and 4,193 CJK ideographs.

This changes modify most existing tables including

- XID_Start/XID_Continue in Clang
- The character name database (used by \N{} in Clang)
- The list of formattable/printable codepoints
- The case folding algorithm (which we had not updated since Unicode 9)
- The list of nonspacing/enclosing marks used by the column width computation 
algorithm. The rest of the column width algorithm is not updated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133807

Files:
  clang/lib/Lex/UnicodeCharSets.h
  llvm/lib/Support/Unicode.cpp
  llvm/lib/Support/UnicodeCaseFold.cpp
  llvm/lib/Support/UnicodeNameToCodepoint.cpp
  llvm/lib/Support/UnicodeNameToCodepointGenerated.cpp
  llvm/utils/UnicodeData/UnicodeNameMappingGenerator.cpp

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


[PATCH] D133711: [Sema] Reject array element types whose alignments are larger than their sizes

2022-09-13 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

It looks like gcc started rejecting the code relatively recently (11.1).

Also, I found out that MSVC doesn't reject the following code:

  struct T0 { char c; };
  struct T2 : virtual T0 { };
  T2 array[2];

The size of `T2` is 5 and its alignment is 4. The size of `array` is 10 bytes.

https://godbolt.org/z/c1EzdYqPc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133711

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


[PATCH] D133641: [Clang] [Sema] Ignore invalid multiversion function redeclarations

2022-09-13 Thread Evgeny Shulgin via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG67f08bf1bfa8: [Clang] [Sema] Ignore invalid multiversion 
function redeclarations (authored by Izaron).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133641

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/attr-target-mv.c


Index: clang/test/Sema/attr-target-mv.c
===
--- clang/test/Sema/attr-target-mv.c
+++ clang/test/Sema/attr-target-mv.c
@@ -52,6 +52,15 @@
 // expected-note@-2 {{previous definition is here}}
 int __attribute__((target("default"))) redef2(void) { return 1;}
 
+int redef3(void) { return 1; }
+// expected-warning@+4 {{attribute declaration must precede definition}}
+// expected-note@-2 {{previous definition is here}}
+// expected-error@+2 {{redefinition of 'redef3'}}
+// expected-note@-4 {{previous definition is here}}
+int __attribute__((target("default"))) redef3(void) { return 1; }
+// allow this, since we don't complain about more than one redefinition
+int __attribute__((target("sse4.2"))) redef3(void) { return 1; }
+
 int __attribute__((target("sse4.2"))) mv_after_use(void) { return 1; }
 int use3(void) {
   return mv_after_use();
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -11082,11 +11082,11 @@
   bool MayNeedOverloadableChecks =
   AllowOverloadingOfFunction(Previous, S.Context, NewFD);
 
-  // Next, check ALL non-overloads to see if this is a redeclaration of a
-  // previous member of the MultiVersion set.
+  // Next, check ALL non-invalid non-overloads to see if this is a 
redeclaration
+  // of a previous member of the MultiVersion set.
   for (NamedDecl *ND : Previous) {
 FunctionDecl *CurFD = ND->getAsFunction();
-if (!CurFD)
+if (!CurFD || CurFD->isInvalidDecl())
   continue;
 if (MayNeedOverloadableChecks &&
 S.IsOverload(NewFD, CurFD, UseMemberUsingDeclRules))


Index: clang/test/Sema/attr-target-mv.c
===
--- clang/test/Sema/attr-target-mv.c
+++ clang/test/Sema/attr-target-mv.c
@@ -52,6 +52,15 @@
 // expected-note@-2 {{previous definition is here}}
 int __attribute__((target("default"))) redef2(void) { return 1;}
 
+int redef3(void) { return 1; }
+// expected-warning@+4 {{attribute declaration must precede definition}}
+// expected-note@-2 {{previous definition is here}}
+// expected-error@+2 {{redefinition of 'redef3'}}
+// expected-note@-4 {{previous definition is here}}
+int __attribute__((target("default"))) redef3(void) { return 1; }
+// allow this, since we don't complain about more than one redefinition
+int __attribute__((target("sse4.2"))) redef3(void) { return 1; }
+
 int __attribute__((target("sse4.2"))) mv_after_use(void) { return 1; }
 int use3(void) {
   return mv_after_use();
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -11082,11 +11082,11 @@
   bool MayNeedOverloadableChecks =
   AllowOverloadingOfFunction(Previous, S.Context, NewFD);
 
-  // Next, check ALL non-overloads to see if this is a redeclaration of a
-  // previous member of the MultiVersion set.
+  // Next, check ALL non-invalid non-overloads to see if this is a redeclaration
+  // of a previous member of the MultiVersion set.
   for (NamedDecl *ND : Previous) {
 FunctionDecl *CurFD = ND->getAsFunction();
-if (!CurFD)
+if (!CurFD || CurFD->isInvalidDecl())
   continue;
 if (MayNeedOverloadableChecks &&
 S.IsOverload(NewFD, CurFD, UseMemberUsingDeclRules))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 67f08bf - [Clang] [Sema] Ignore invalid multiversion function redeclarations

2022-09-13 Thread Evgeny Shulgin via cfe-commits

Author: Evgeny Shulgin
Date: 2022-09-13T20:12:09Z
New Revision: 67f08bf1bfa85fcdc1d91a3e6cce703e94611244

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

LOG: [Clang] [Sema] Ignore invalid multiversion function redeclarations

If a redeclaration of a multiversion function is invalid,
it may be in a broken condition (for example, missing an important
attribute). We shouldn't analyze invalid redeclarations.

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

Reviewed By: tahonermann

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

Added: 


Modified: 
clang/lib/Sema/SemaDecl.cpp
clang/test/Sema/attr-target-mv.c

Removed: 




diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 58774c0fa44a3..e4bd827b38d39 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -11082,11 +11082,11 @@ static bool CheckMultiVersionAdditionalDecl(
   bool MayNeedOverloadableChecks =
   AllowOverloadingOfFunction(Previous, S.Context, NewFD);
 
-  // Next, check ALL non-overloads to see if this is a redeclaration of a
-  // previous member of the MultiVersion set.
+  // Next, check ALL non-invalid non-overloads to see if this is a 
redeclaration
+  // of a previous member of the MultiVersion set.
   for (NamedDecl *ND : Previous) {
 FunctionDecl *CurFD = ND->getAsFunction();
-if (!CurFD)
+if (!CurFD || CurFD->isInvalidDecl())
   continue;
 if (MayNeedOverloadableChecks &&
 S.IsOverload(NewFD, CurFD, UseMemberUsingDeclRules))

diff  --git a/clang/test/Sema/attr-target-mv.c 
b/clang/test/Sema/attr-target-mv.c
index b02b13079163b..8218771275e1b 100644
--- a/clang/test/Sema/attr-target-mv.c
+++ b/clang/test/Sema/attr-target-mv.c
@@ -52,6 +52,15 @@ int __attribute__((target("default"))) redef2(void) { return 
1;}
 // expected-note@-2 {{previous definition is here}}
 int __attribute__((target("default"))) redef2(void) { return 1;}
 
+int redef3(void) { return 1; }
+// expected-warning@+4 {{attribute declaration must precede definition}}
+// expected-note@-2 {{previous definition is here}}
+// expected-error@+2 {{redefinition of 'redef3'}}
+// expected-note@-4 {{previous definition is here}}
+int __attribute__((target("default"))) redef3(void) { return 1; }
+// allow this, since we don't complain about more than one redefinition
+int __attribute__((target("sse4.2"))) redef3(void) { return 1; }
+
 int __attribute__((target("sse4.2"))) mv_after_use(void) { return 1; }
 int use3(void) {
   return mv_after_use();



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


[PATCH] D133756: [clangd] Introduce CompileCommandsAdjuster

2022-09-13 Thread Nathan Ridge via Phabricator via cfe-commits
nridge added inline comments.



Comment at: clang-tools-extra/clangd/indexer/IndexerMain.cpp:150-151
   clang::tooling::ArgumentsAdjuster(
-  clang::clangd::CommandMangler::detect()));
+  [Mangler = std::shared_ptr(
+   clang::clangd::CommandMangler::detect().release())](
+  const std::vector , llvm::StringRef File) {

tom-anders wrote:
> Just wondering, why do you need to convert to a shared_ptr here? Couldn't the 
> lambda just take ownership of the unique_ptr?
ArgumentsAdjuster is a typedef for std::function which needs to be copyable, 
not move-only


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133756

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


[PATCH] D133662: [Clang] WIP: Change -ftime-trace storing path and support multiple compilation jobs

2022-09-13 Thread Mészáros Gergely via Phabricator via cfe-commits
Maetveis added a comment.

In D133662#3787250 , @jamieschmeiser 
wrote:

> I'm a little confused as to what is being proposed here.  Is this building on 
> D131469  or is it an alternative?  It seems 
> that there are portions of the code from D131469 
>  included in these changes, which implies 
> that you are building on it.  I think this patch is premature in that the 
> other patch has not yet landed (@MaskRay has asked for revisions that 
> @dongjunduo has made but is waiting for review).  When that patch has landed, 
> this could be reposted based on those changes.

Yes, there is some code taken from D131469 , 
mainly removing the -ftime-trace file logic from cc1.
The way the paths on where to store the traces are different both in 
implementation and in behaviour.
D131469  is rewriting the arguements for 
already created jobs and infers the path based on a link job.
This patch sets up the time trace path for each job that supports it, so it can 
be read during assembling of the commands. The location is based on the final 
output (-o ) and the file names are based on the inputs instead of the 
output names (similar to -save-temps).

I felt that this is a different approach, that might not be taken once D131469 
 is accepted. If it's better to leave these 
points as suggestions on D131469 , than I can 
summarize them over there.
Waiting for it to land is also fine for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133662

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


[PATCH] D129570: [clang-tidy] Add new clang-tidy check to find implicit conversions from enum to integer.

2022-09-13 Thread Paul Fultz II via Phabricator via cfe-commits
pfultz2 added a comment.

@njames93 I fixed the review comments, can this be merged?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129570

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


[clang] a8a4992 - [HLSL] Call global destructors from entries

2022-09-13 Thread Chris Bieneman via cfe-commits

Author: Chris Bieneman
Date: 2022-09-13T15:05:47-05:00
New Revision: a8a49923ddf75be9b6166be2f6a1f50a99817841

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

LOG: [HLSL] Call global destructors from entries

HLSL doesn't have a C++ runtime that supports `atexit` registration. To
enable global destructors we instead rely on the `llvm.global_dtor`
mechanism.

This change disables `atexit` generation for HLSL and updates the HLSL
code generation to call global destructors on the exit from entry
functions.

Depends on D132977.

Reviewed By: aaron.ballman

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

Added: 
clang/test/CodeGenHLSL/GlobalDestructors.hlsl

Modified: 
clang/docs/HLSL/EntryFunctions.rst
clang/lib/CodeGen/CGHLSLRuntime.cpp
clang/lib/CodeGen/CGHLSLRuntime.h
clang/lib/CodeGen/MicrosoftCXXABI.cpp
clang/test/CodeGenHLSL/GlobalConstructorFunction.hlsl

Removed: 




diff  --git a/clang/docs/HLSL/EntryFunctions.rst 
b/clang/docs/HLSL/EntryFunctions.rst
index 8591814777388..518698b9b1f7a 100644
--- a/clang/docs/HLSL/EntryFunctions.rst
+++ b/clang/docs/HLSL/EntryFunctions.rst
@@ -46,7 +46,9 @@ constructors, then instantiations of the user-defined entry 
parameters with
 their semantic values populated, and a call to the user-defined function.
 After the call instruction the return value (if any) is saved using a
 target-appropriate intrinsic for storing outputs (for DirectX, the
-``llvm.dx.store.output``). Global destructors are not supported in HLSL.
+``llvm.dx.store.output``). Lastly, any present global destructors will be 
called
+immediately before the return. HLSL does not support C++ ``atexit``
+registrations, instead calls to global destructors are compile-time generated.
 
 .. note::
 

diff  --git a/clang/lib/CodeGen/CGHLSLRuntime.cpp 
b/clang/lib/CodeGen/CGHLSLRuntime.cpp
index 2f0c00169af69..591d0c43f1825 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ b/clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -55,7 +55,7 @@ void CGHLSLRuntime::finishCodeGen() {
   if (T.getArch() == Triple::ArchType::dxil)
 addDxilValVersion(TargetOpts.DxilValidatorVersion, M);
 
-  generateGlobalCtorCalls();
+  generateGlobalCtorDtorCalls();
 }
 
 void CGHLSLRuntime::annotateHLSLResource(const VarDecl *D, GlobalVariable *GV) 
{
@@ -146,12 +146,13 @@ void CGHLSLRuntime::emitEntryFunction(const FunctionDecl 
*FD,
   B.CreateRetVoid();
 }
 
-void CGHLSLRuntime::generateGlobalCtorCalls() {
-  llvm::Module  = CGM.getModule();
-  const auto *GlobalCtors = M.getNamedGlobal("llvm.global_ctors");
-  if (!GlobalCtors)
+static void gatherFunctions(SmallVectorImpl , llvm::Module ,
+bool CtorOrDtor) {
+  const auto *GV =
+  M.getNamedGlobal(CtorOrDtor ? "llvm.global_ctors" : "llvm.global_dtors");
+  if (!GV)
 return;
-  const auto *CA = dyn_cast(GlobalCtors->getInitializer());
+  const auto *CA = dyn_cast(GV->getInitializer());
   if (!CA)
 return;
   // The global_ctor array elements are a struct [Priority, Fn *, COMDat].
@@ -168,8 +169,16 @@ void CGHLSLRuntime::generateGlobalCtorCalls() {
"HLSL doesn't support setting priority for global ctors.");
 assert(isa(CS->getOperand(2)) &&
"HLSL doesn't support COMDat for global ctors.");
-CtorFns.push_back(cast(CS->getOperand(1)));
+Fns.push_back(cast(CS->getOperand(1)));
   }
+}
+
+void CGHLSLRuntime::generateGlobalCtorDtorCalls() {
+  llvm::Module  = CGM.getModule();
+  SmallVector CtorFns;
+  SmallVector DtorFns;
+  gatherFunctions(CtorFns, M, true);
+  gatherFunctions(DtorFns, M, false);
 
   // Insert a call to the global constructor at the beginning of the entry 
block
   // to externally exported functions. This is a bit of a hack, but HLSL allows
@@ -180,5 +189,10 @@ void CGHLSLRuntime::generateGlobalCtorCalls() {
 IRBuilder<> B((), F.getEntryBlock().begin());
 for (auto *Fn : CtorFns)
   B.CreateCall(FunctionCallee(Fn));
+
+// Insert global dtors before the terminator of the last instruction
+B.SetInsertPoint(F.back().getTerminator());
+for (auto *Fn : DtorFns)
+  B.CreateCall(FunctionCallee(Fn));
   }
 }

diff  --git a/clang/lib/CodeGen/CGHLSLRuntime.h 
b/clang/lib/CodeGen/CGHLSLRuntime.h
index ee265922c0f51..120f53ffec14e 100644
--- a/clang/lib/CodeGen/CGHLSLRuntime.h
+++ b/clang/lib/CodeGen/CGHLSLRuntime.h
@@ -46,7 +46,7 @@ class CGHLSLRuntime {
   virtual ~CGHLSLRuntime() {}
 
   void annotateHLSLResource(const VarDecl *D, llvm::GlobalVariable *GV);
-  void generateGlobalCtorCalls();
+  void generateGlobalCtorDtorCalls();
 
   void finishCodeGen();
 

diff  --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp 
b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
index f0c45654f8d9b..cc6ba4eba6d77 100644
--- 

[PATCH] D133518: [HLSL] Call global destructors from entries

2022-09-13 Thread Chris Bieneman via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa8a49923ddf7: [HLSL] Call global destructors from entries 
(authored by beanz).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133518

Files:
  clang/docs/HLSL/EntryFunctions.rst
  clang/lib/CodeGen/CGHLSLRuntime.cpp
  clang/lib/CodeGen/CGHLSLRuntime.h
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenHLSL/GlobalConstructorFunction.hlsl
  clang/test/CodeGenHLSL/GlobalDestructors.hlsl

Index: clang/test/CodeGenHLSL/GlobalDestructors.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/GlobalDestructors.hlsl
@@ -0,0 +1,57 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -std=hlsl202x -S -emit-llvm -disable-llvm-passes %s -o - | FileCheck %s
+
+struct Tail {
+  Tail() {
+add(1);
+  }
+
+  ~Tail() {
+add(-1);
+  }
+
+  void add(int V) {
+static int Count = 0;
+Count += V;
+  }
+};
+
+struct Pupper {
+  static int Count;
+
+  Pupper() {
+Count += 1; // :)
+  }
+
+  ~Pupper() {
+Count -= 1; // :(
+  }
+} GlobalPup;
+
+void Wag() {
+  static Tail T;
+  T.add(0);
+}
+
+int Pupper::Count = 0;
+
+[numthreads(1,1,1)]
+void main(unsigned GI : SV_GroupIndex) {
+  Wag();
+}
+
+//CHECK:  define void @main()
+//CHECK-NEXT: entry:
+//CHECK-NEXT:   call void @_GLOBAL__sub_I_GlobalDestructors.hlsl()
+//CHECK-NEXT:   %0 = call i32 @llvm.dx.flattened.thread.id.in.group()
+//CHECK-NEXT:   call void @"?main@@YAXI@Z"(i32 %0)
+//CHECK-NEXT:   call void @_GLOBAL__D_a()
+//CHECK-NEXT:   ret void
+
+// This is really just a sanity check I needed for myself to verify that
+// function scope static variables also get destroyed properly.
+
+//CHECK: define internal void @_GLOBAL__D_a()
+//CHECK-NEXT: entry:
+//CHECK-NEXT:   call void @"??1Tail@@QAA@XZ"(ptr @"?T@?1??Wag@@YAXXZ@4UTail@@A")
+//CHECK-NEXT:   call void @"??1Pupper@@QAA@XZ"(ptr @"?GlobalPup@@3UPupper@@A")
+//CHECK-NEXT:   ret void
Index: clang/test/CodeGenHLSL/GlobalConstructorFunction.hlsl
===
--- clang/test/CodeGenHLSL/GlobalConstructorFunction.hlsl
+++ clang/test/CodeGenHLSL/GlobalConstructorFunction.hlsl
@@ -10,6 +10,10 @@
   i = 12;
 }
 
+__attribute__((destructor)) void call_me_last(void) {
+  i = 0;
+}
+
 [numthreads(1,1,1)]
 void main(unsigned GI : SV_GroupIndex) {}
 
@@ -19,4 +23,5 @@
 //CHECK-NEXT:   call void @"?then_call_me@@YAXXZ"()
 //CHECK-NEXT:   %0 = call i32 @llvm.dx.flattened.thread.id.in.group()
 //CHECK-NEXT:   call void @"?main@@YAXI@Z"(i32 %0)
+//CHECK-NEXT:   call void @"?call_me_last@@YAXXZ"(
 //CHECK-NEXT:   ret void
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -2348,6 +2348,10 @@
   if (D.getTLSKind())
 return emitGlobalDtorWithTLRegDtor(CGF, D, Dtor, Addr);
 
+  // HLSL doesn't support atexit.
+  if (CGM.getLangOpts().HLSL)
+return CGM.AddCXXDtorEntry(Dtor, Addr);
+
   // The default behavior is to use atexit.
   CGF.registerGlobalDtorWithAtExit(D, Dtor, Addr);
 }
Index: clang/lib/CodeGen/CGHLSLRuntime.h
===
--- clang/lib/CodeGen/CGHLSLRuntime.h
+++ clang/lib/CodeGen/CGHLSLRuntime.h
@@ -46,7 +46,7 @@
   virtual ~CGHLSLRuntime() {}
 
   void annotateHLSLResource(const VarDecl *D, llvm::GlobalVariable *GV);
-  void generateGlobalCtorCalls();
+  void generateGlobalCtorDtorCalls();
 
   void finishCodeGen();
 
Index: clang/lib/CodeGen/CGHLSLRuntime.cpp
===
--- clang/lib/CodeGen/CGHLSLRuntime.cpp
+++ clang/lib/CodeGen/CGHLSLRuntime.cpp
@@ -55,7 +55,7 @@
   if (T.getArch() == Triple::ArchType::dxil)
 addDxilValVersion(TargetOpts.DxilValidatorVersion, M);
 
-  generateGlobalCtorCalls();
+  generateGlobalCtorDtorCalls();
 }
 
 void CGHLSLRuntime::annotateHLSLResource(const VarDecl *D, GlobalVariable *GV) {
@@ -146,12 +146,13 @@
   B.CreateRetVoid();
 }
 
-void CGHLSLRuntime::generateGlobalCtorCalls() {
-  llvm::Module  = CGM.getModule();
-  const auto *GlobalCtors = M.getNamedGlobal("llvm.global_ctors");
-  if (!GlobalCtors)
+static void gatherFunctions(SmallVectorImpl , llvm::Module ,
+bool CtorOrDtor) {
+  const auto *GV =
+  M.getNamedGlobal(CtorOrDtor ? "llvm.global_ctors" : "llvm.global_dtors");
+  if (!GV)
 return;
-  const auto *CA = dyn_cast(GlobalCtors->getInitializer());
+  const auto *CA = dyn_cast(GV->getInitializer());
   if (!CA)
 return;
   // The global_ctor array elements are a struct [Priority, Fn *, COMDat].
@@ -168,8 +169,16 @@
"HLSL doesn't support setting priority for global ctors.");
 assert(isa(CS->getOperand(2)) &&
"HLSL 

[PATCH] D133802: [OpenMP] Remove simplified device runtime handling

2022-09-13 Thread Jose Manuel Monsalve Diaz via Phabricator via cfe-commits
josemonsalve2 added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2565-2566
   Flags<[NoArgumentUnused, HelpHidden]>;
-def fopenmp_cuda_force_full_runtime : Flag<["-"], 
"fopenmp-cuda-force-full-runtime">, Group,
-  Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
-def fno_openmp_cuda_force_full_runtime : Flag<["-"], 
"fno-openmp-cuda-force-full-runtime">, Group,
-  Flags<[NoArgumentUnused, HelpHidden]>;
+def fopenmp_cuda_force_full_runtime : Flag<["-"], 
"fopenmp-cuda-force-full-runtime">, Flags<[HelpHidden]>;
+def fno_openmp_cuda_force_full_runtime : Flag<["-"], 
"fno-openmp-cuda-force-full-runtime">, Flags<[HelpHidden]>;
 def fopenmp_cuda_number_of_sm_EQ : Joined<["-"], 
"fopenmp-cuda-number-of-sm=">, Group,

jhuber6 wrote:
> josemonsalve2 wrote:
> > Why not remove these? Are they used somewhere else? 
> We usually don't remove driver arguments between releases as this could cause 
> existing applications to stop compiling. Leaving them here will cause Clang 
> to continue compiling but emit an unused flag warning.
Will that generate a warning saying this flag has no use? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133802

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


[PATCH] D133804: Cuda Check for ignored return errors from api calls to cuda

2022-09-13 Thread Bartłomiej Cieślar via Phabricator via cfe-commits
barcisz created this revision.
Herald added subscribers: mattd, carlosgalvezp, yaxunl, mgorny.
Herald added a project: All.
barcisz requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133804

Files:
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  clang-tools-extra/test/clang-tidy/checkers/cuda/.keep
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu

Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
@@ -0,0 +1,104 @@
+//===--- SlicingCheck.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
+//
+//===--===//
+
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'CUDA_HANDLER'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers -nocudalib -nocudainc -std=c++14
+#include 
+
+class DummyContainer {
+ public:
+  int* begin();
+  int* end();
+};
+
+#define DUMMY_CUDA_HANDLER(stmt) stmt
+#define CUDA_HANDLER(stmt) do {auto err = stmt;} while(0)
+#define API_CALL() do {cudaDeviceReset();} while(0)
+
+void errorCheck();
+void errorCheck(cudaError_t error);
+
+void bad() {
+  API_CALL();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  errorCheck();
+
+  if (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  while (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  do
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+  while(false);
+
+  switch (0) {
+case 0:
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  }
+
+  for(
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+;
+cudaDeviceReset()
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset()){{$}}
+  ) cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}  ) CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  for(int i : DummyContainer())
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  auto x = ({
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+true;
+  });
+}
+
+int good() {
+  DUMMY_CUDA_HANDLER(cudaDeviceReset());
+
+  if (cudaDeviceReset()) {
+return 0;
+  }
+
+  switch (cudaDeviceReset()) {
+case cudaErrorInvalidValue: return 1;
+case cudaErrorMemoryAllocation: return 2;
+default: return 3;
+  }
+
+  auto err = ({cudaDeviceReset();});
+  // NOTE: We don't check that `errorCheck()` actually handles the error; we just assume it does.
+  errorCheck(cudaDeviceReset());
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
===
--- /dev/null
+++ 

[PATCH] D133802: [OpenMP] Remove simplified device runtime handling

2022-09-13 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.

Remove the option and let's also remove the device flag too while we are at it.




Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:76
   CGOpenMPRuntimeGPU::ExecutionMode 
   bool SavedRuntimeMode = false;
 

Remove



Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:86
   /// Constructor for SPMD mode.
-  ExecutionRuntimeModesRAII(CGOpenMPRuntimeGPU::ExecutionMode ,
-bool , bool FullRuntimeMode)
-  : ExecMode(ExecMode), RuntimeMode() {
+  ExecutionRuntimeModesRAII(CGOpenMPRuntimeGPU::ExecutionMode , bool)
+  : ExecMode(ExecMode) {




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133802

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


[PATCH] D133802: [OpenMP] Remove simplified device runtime handling

2022-09-13 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2565-2566
   Flags<[NoArgumentUnused, HelpHidden]>;
-def fopenmp_cuda_force_full_runtime : Flag<["-"], 
"fopenmp-cuda-force-full-runtime">, Group,
-  Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
-def fno_openmp_cuda_force_full_runtime : Flag<["-"], 
"fno-openmp-cuda-force-full-runtime">, Group,
-  Flags<[NoArgumentUnused, HelpHidden]>;
+def fopenmp_cuda_force_full_runtime : Flag<["-"], 
"fopenmp-cuda-force-full-runtime">, Flags<[HelpHidden]>;
+def fno_openmp_cuda_force_full_runtime : Flag<["-"], 
"fno-openmp-cuda-force-full-runtime">, Flags<[HelpHidden]>;
 def fopenmp_cuda_number_of_sm_EQ : Joined<["-"], 
"fopenmp-cuda-number-of-sm=">, Group,

josemonsalve2 wrote:
> Why not remove these? Are they used somewhere else? 
We usually don't remove driver arguments between releases as this could cause 
existing applications to stop compiling. Leaving them here will cause Clang to 
continue compiling but emit an unused flag warning.



Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:80-90
   ExecutionRuntimeModesRAII(CGOpenMPRuntimeGPU::ExecutionMode )
   : ExecMode(ExecMode) {
 SavedExecMode = ExecMode;
 ExecMode = CGOpenMPRuntimeGPU::EM_NonSPMD;
   }
   /// Constructor for SPMD mode.
+  ExecutionRuntimeModesRAII(CGOpenMPRuntimeGPU::ExecutionMode , bool)

josemonsalve2 wrote:
> What if we combine these two and just leave one that receives two modes. I am 
> really confused by this code. Is there something I am missing here? 
Just me being lazy, I'll combine it into a single one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133802

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


[PATCH] D133802: [OpenMP] Remove simplified device runtime handling

2022-09-13 Thread Jose Manuel Monsalve Diaz via Phabricator via cfe-commits
josemonsalve2 added a comment.

This is a good idea. Thanks Joseph.

Other than the two comments I made, I think this should be accepted.

Jose




Comment at: clang/include/clang/Driver/Options.td:2565-2566
   Flags<[NoArgumentUnused, HelpHidden]>;
-def fopenmp_cuda_force_full_runtime : Flag<["-"], 
"fopenmp-cuda-force-full-runtime">, Group,
-  Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
-def fno_openmp_cuda_force_full_runtime : Flag<["-"], 
"fno-openmp-cuda-force-full-runtime">, Group,
-  Flags<[NoArgumentUnused, HelpHidden]>;
+def fopenmp_cuda_force_full_runtime : Flag<["-"], 
"fopenmp-cuda-force-full-runtime">, Flags<[HelpHidden]>;
+def fno_openmp_cuda_force_full_runtime : Flag<["-"], 
"fno-openmp-cuda-force-full-runtime">, Flags<[HelpHidden]>;
 def fopenmp_cuda_number_of_sm_EQ : Joined<["-"], 
"fopenmp-cuda-number-of-sm=">, Group,

Why not remove these? Are they used somewhere else? 



Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:80-90
   ExecutionRuntimeModesRAII(CGOpenMPRuntimeGPU::ExecutionMode )
   : ExecMode(ExecMode) {
 SavedExecMode = ExecMode;
 ExecMode = CGOpenMPRuntimeGPU::EM_NonSPMD;
   }
   /// Constructor for SPMD mode.
+  ExecutionRuntimeModesRAII(CGOpenMPRuntimeGPU::ExecutionMode , bool)

What if we combine these two and just leave one that receives two modes. I am 
really confused by this code. Is there something I am missing here? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133802

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


[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

2022-09-13 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 updated this revision to Diff 459848.
jhuber6 added a comment.

Removing old function update for 'mcpu'


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133726

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/lib/Driver/ToolChains/HIPAMD.h
  clang/lib/Driver/ToolChains/HIPSPV.cpp
  clang/lib/Driver/ToolChains/HIPSPV.h
  clang/test/Driver/amdgpu-openmp-toolchain.c

Index: clang/test/Driver/amdgpu-openmp-toolchain.c
===
--- clang/test/Driver/amdgpu-openmp-toolchain.c
+++ clang/test/Driver/amdgpu-openmp-toolchain.c
@@ -49,5 +49,12 @@
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -emit-llvm -S -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx803 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-EMIT-LLVM-IR
 // CHECK-EMIT-LLVM-IR: "-cc1" "-triple" "amdgcn-amd-amdhsa"{{.*}}"-emit-llvm"
 
-// RUN: %clang -### -target x86_64-pc-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx803 -lm --rocm-device-lib-path=%S/Inputs/rocm/amdgcn/bitcode %s 2>&1 | FileCheck %s --check-prefix=CHECK-LIB-DEVICE-NEW
-// CHECK-LIB-DEVICE-NEW: {{.*}}clang-linker-wrapper{{.*}}--bitcode-library=openmp-amdgcn-amd-amdhsa-gfx803={{.*}}ocml.bc"{{.*}}ockl.bc"{{.*}}oclc_daz_opt_on.bc"{{.*}}oclc_unsafe_math_off.bc"{{.*}}oclc_finite_only_off.bc"{{.*}}oclc_correctly_rounded_sqrt_on.bc"{{.*}}oclc_wavefrontsize64_on.bc"{{.*}}oclc_isa_version_803.bc"
+// RUN: %clang -### -target x86_64-pc-linux-gnu -fopenmp --offload-arch=gfx803 \
+// RUN:   --rocm-device-lib-path=%S/Inputs/rocm/amdgcn/bitcode -fopenmp-new-driver %s  2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-LIB-DEVICE
+// CHECK-LIB-DEVICE: "-cc1" {{.*}}ocml.bc"{{.*}}ockl.bc"{{.*}}oclc_daz_opt_on.bc"{{.*}}oclc_unsafe_math_off.bc"{{.*}}oclc_finite_only_off.bc"{{.*}}oclc_correctly_rounded_sqrt_on.bc"{{.*}}oclc_wavefrontsize64_on.bc"{{.*}}oclc_isa_version_803.bc"
+
+// RUN: %clang -### -target x86_64-pc-linux-gnu -fopenmp --offload-arch=gfx803 -nogpulib \
+// RUN:   --rocm-device-lib-path=%S/Inputs/rocm/amdgcn/bitcode -fopenmp-new-driver %s  2>&1 | \
+// RUN: FileCheck %s --check-prefix=CHECK-LIB-DEVICE-NOGPULIB
+// CHECK-LIB-DEVICE-NOGPULIB-NOT: "-cc1" {{.*}}ocml.bc"{{.*}}ockl.bc"{{.*}}oclc_daz_opt_on.bc"{{.*}}oclc_unsafe_math_off.bc"{{.*}}oclc_finite_only_off.bc"{{.*}}oclc_correctly_rounded_sqrt_on.bc"{{.*}}oclc_wavefrontsize64_on.bc"{{.*}}oclc_isa_version_803.bc"
Index: clang/lib/Driver/ToolChains/HIPSPV.h
===
--- clang/lib/Driver/ToolChains/HIPSPV.h
+++ clang/lib/Driver/ToolChains/HIPSPV.h
@@ -69,7 +69,7 @@
   void AddHIPIncludeArgs(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ) const override;
   llvm::SmallVector
-  getHIPDeviceLibs(const llvm::opt::ArgList ) const override;
+  getROCmDeviceLibs(const llvm::opt::ArgList ) const override;
 
   SanitizerMask getSupportedSanitizers() const override;
 
Index: clang/lib/Driver/ToolChains/HIPSPV.cpp
===
--- clang/lib/Driver/ToolChains/HIPSPV.cpp
+++ clang/lib/Driver/ToolChains/HIPSPV.cpp
@@ -154,7 +154,7 @@
 CC1Args.append(
 {"-fvisibility=hidden", "-fapply-global-visibility-to-externs"});
 
-  llvm::for_each(getHIPDeviceLibs(DriverArgs),
+  llvm::for_each(getROCmDeviceLibs(DriverArgs),
  [&](const BitCodeLibraryInfo ) {
CC1Args.append({"-mlink-builtin-bitcode",
DriverArgs.MakeArgString(BCFile.Path)});
@@ -206,7 +206,7 @@
 }
 
 llvm::SmallVector
-HIPSPVToolChain::getHIPDeviceLibs(const llvm::opt::ArgList ) const {
+HIPSPVToolChain::getROCmDeviceLibs(const llvm::opt::ArgList ) const {
   llvm::SmallVector BCLibs;
   if (DriverArgs.hasArg(options::OPT_nogpulib))
 return {};
Index: clang/lib/Driver/ToolChains/HIPAMD.h
===
--- clang/lib/Driver/ToolChains/HIPAMD.h
+++ clang/lib/Driver/ToolChains/HIPAMD.h
@@ -76,7 +76,7 @@
   void AddHIPIncludeArgs(const llvm::opt::ArgList ,
  llvm::opt::ArgStringList ) const override;
   llvm::SmallVector
-  getHIPDeviceLibs(const llvm::opt::ArgList ) const override;
+  getROCmDeviceLibs(const llvm::opt::ArgList ) const override;
 
   SanitizerMask getSupportedSanitizers() const override;
 
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -246,7 +246,7 @@
 

[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

2022-09-13 Thread Joseph Huber via Phabricator via cfe-commits
jhuber6 added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:720-722
+  if (DriverArgs.hasArg(options::OPT_march_EQ))
+return getProcessorFromTargetID(
+getTriple(), DriverArgs.getLastArgValue(options::OPT_march_EQ));

yaxunl wrote:
> It seems the code still there.
Sorry thought I reverted this file. I'll fix it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133726

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


[PATCH] D133726: [OpenMP][AMDGPU] Link bitcode ROCm device libraries per-TU

2022-09-13 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:720-722
+  if (DriverArgs.hasArg(options::OPT_march_EQ))
+return getProcessorFromTargetID(
+getTriple(), DriverArgs.getLastArgValue(options::OPT_march_EQ));

It seems the code still there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133726

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


[PATCH] D133801: Extraction of a matcher for an unused value from an expression from the bugprone-unused-return-value check

2022-09-13 Thread Bartłomiej Cieślar via Phabricator via cfe-commits
barcisz created this revision.
Herald added a subscriber: carlosgalvezp.
Herald added a project: All.
barcisz requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

This diff extracts the matcher for an unused expression result from 
bugprone-unused-return-value into a utility. This diff is a prerequisite to the 
diff that introduces 
the cuda-unsafe-api-call-check


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133801

Files:
  clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
  clang-tools-extra/clang-tidy/utils/Matchers.h


Index: clang-tools-extra/clang-tidy/utils/Matchers.h
===
--- clang-tools-extra/clang-tidy/utils/Matchers.h
+++ clang-tools-extra/clang-tidy/utils/Matchers.h
@@ -49,6 +49,51 @@
   return pointerType(pointee(qualType(isConstQualified(;
 }
 
+// Matches the statements in a GNU statement-expression that are not returned
+// from it.
+AST_MATCHER_P(StmtExpr, hasUnreturning,
+  clang::ast_matchers::internal::Matcher, matcher) {
+  const auto compoundStmt = Node.getSubStmt();
+  assert(compoundStmt);
+
+  clang::ast_matchers::internal::BoundNodesTreeBuilder result;
+  bool matched = false;
+  for (auto stmt = compoundStmt->body_begin();
+   stmt + 1 < compoundStmt->body_end(); ++stmt) {
+clang::ast_matchers::internal::BoundNodesTreeBuilder 
builderInner(*Builder);
+assert(stmt && *stmt);
+if (matcher.matches(**stmt, Finder, )) {
+  result.addMatch(builderInner);
+  matched = true;
+}
+  }
+  *Builder = result;
+  return matched;
+}
+
+// Matches all of the nodes (simmilar to forEach) that match the matcher
+// and have return values not used in any statement.
+AST_MATCHER_FUNCTION_P(ast_matchers::StatementMatcher, isValueUnused,
+   ast_matchers::StatementMatcher, Matcher) {
+  using namespace ast_matchers;
+  const auto UnusedInCompoundStmt =
+  compoundStmt(forEach(Matcher), unless(hasParent(stmtExpr(;
+  const auto UnusedInGnuExprStmt = stmtExpr(hasUnreturning(Matcher));
+  const auto UnusedInIfStmt =
+  ifStmt(eachOf(hasThen(Matcher), hasElse(Matcher)));
+  const auto UnusedInWhileStmt = whileStmt(hasBody(Matcher));
+  const auto UnusedInDoStmt = doStmt(hasBody(Matcher));
+  const auto UnusedInForStmt = forStmt(
+  eachOf(hasLoopInit(Matcher), hasIncrement(Matcher), hasBody(Matcher)));
+  const auto UnusedInRangeForStmt = cxxForRangeStmt(hasBody(Matcher));
+  const auto UnusedInCaseStmt = switchCase(forEach(Matcher));
+  const auto Unused =
+  stmt(anyOf(UnusedInCompoundStmt, UnusedInGnuExprStmt, UnusedInIfStmt,
+ UnusedInWhileStmt, UnusedInDoStmt, UnusedInForStmt,
+ UnusedInRangeForStmt, UnusedInCaseStmt));
+  return stmt(eachOf(Unused, forEachDescendant(Unused)));
+}
+
 // A matcher implementation that matches a list of type name regular 
expressions
 // against a NamedDecl. If a regular expression contains the substring "::"
 // matching will occur against the qualified name, otherwise only the typename.
Index: clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnusedReturnValueCheck.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "UnusedReturnValueCheck.h"
+#include "../utils/Matchers.h"
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
@@ -159,10 +160,7 @@
   auto UnusedInCaseStmt = switchCase(forEach(MatchedCallExpr));
 
   Finder->addMatcher(
-  stmt(anyOf(UnusedInCompoundStmt, UnusedInIfStmt, UnusedInWhileStmt,
- UnusedInDoStmt, UnusedInForStmt, UnusedInRangeForStmt,
- UnusedInCaseStmt)),
-  this);
+  functionDecl(hasBody(matchers::isValueUnused(MatchedCallExpr))), this);
 }
 
 void UnusedReturnValueCheck::check(const MatchFinder::MatchResult ) {


Index: clang-tools-extra/clang-tidy/utils/Matchers.h
===
--- clang-tools-extra/clang-tidy/utils/Matchers.h
+++ clang-tools-extra/clang-tidy/utils/Matchers.h
@@ -49,6 +49,51 @@
   return pointerType(pointee(qualType(isConstQualified(;
 }
 
+// Matches the statements in a GNU statement-expression that are not returned
+// from it.
+AST_MATCHER_P(StmtExpr, hasUnreturning,
+  clang::ast_matchers::internal::Matcher, matcher) {
+  const auto compoundStmt = Node.getSubStmt();
+  assert(compoundStmt);
+
+  clang::ast_matchers::internal::BoundNodesTreeBuilder result;
+  bool matched = false;
+  for (auto stmt = compoundStmt->body_begin();
+   stmt + 1 < compoundStmt->body_end(); ++stmt) {
+

[PATCH] D133800: [Clang 15.0.1] Downgrade implicit int and implicit function declaration to warning only

2022-09-13 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: rjmccall, jyknight, MaskRay.
aaron.ballman added a comment.
Herald added a subscriber: StephenFan.

In D133800#3787378 , @thieta wrote:

> I think the easiest way to handle this is that when you have it approved here 
> - push it to fork on GitHub based on the release branch and create a GitHub 
> issue and write /branch aballman/llvm-project/my_branch in a comment and it 
> will queue up the cherry pick.

Thanks for the suggestion, I'll do that once we have agreement here.

Adding in more reviewers for greater visibility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133800

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


[PATCH] D133092: [clang] fix generation of .debug_aranges with LTO

2022-09-13 Thread Azat Khuzhin via Phabricator via cfe-commits
azat added a comment.

> the usual flow would be once you have approval to go ahead and submit the 
> patch yourself - I take it you don't have commit access?

Yeah, I don't
I though that only trusted/experienced/... llvm devs can commit.

> In which case I can commit this on your behalf.

Yes, please!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133092

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


  1   2   3   >