[PATCH] D74572: [BPF] preserve debuginfo types for builtin __builtin__btf_type_id()

[Yonghong Song via Phabricator via cfe-commits]

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

fix a typo (BPFPreserveType.cpp => BPFPreserveDIType.cpp) in the comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74572

Files:
  llvm/lib/Target/BPF/BPF.h
  llvm/lib/Target/BPF/BPFCORE.h
  llvm/lib/Target/BPF/BPFMISimplifyPatchable.cpp
  llvm/lib/Target/BPF/BPFPreserveDIType.cpp
  llvm/lib/Target/BPF/BPFTargetMachine.cpp
  llvm/lib/Target/BPF/BTFDebug.cpp
  llvm/lib/Target/BPF/BTFDebug.h
  llvm/lib/Target/BPF/CMakeLists.txt
  llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll

Index: llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll
===
--- /dev/null
+++ llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll
@@ -0,0 +1,147 @@
+; RUN: llc -march=bpfel -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
+; RUN: llc -march=bpfeb -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
+; RUN: llc -march=bpfel -mattr=+alu32 -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
+; RUN: llc -march=bpfeb -mattr=+alu32 -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
+;
+; Source code:
+;   static int (*bpf_log)(unsigned tid, void *data, int data_size) = (void *)999;
+;   struct {
+; char f1[100];
+; typeof(3) f2;
+;   } tmp__abc = {1, 3};
+;   void prog1() {
+; bpf_log(__builtin_btf_type_id(tmp__abc), &tmp__abc, sizeof(tmp__abc));
+;   }
+;   void prog2() {
+; bpf_log(__builtin_btf_type_id(&tmp__abc), &tmp__abc, sizeof(tmp__abc));
+;   }
+;   void prog3() {
+; bpf_log(__builtin_btf_type_id(tmp__abc.f1[3]), &tmp__abc, sizeof(tmp__abc));
+;   }
+; Compilation flag:
+;   clang -target bpf -O2 -g -S -emit-llvm test.c
+
+%struct.anon = type { [100 x i8], i32 }
+
+@tmp__abc = dso_local global { <{ i8, i8, [98 x i8] }>, i32 } { <{ i8, i8, [98 x i8] }> <{ i8 1, i8 3, [98 x i8] zeroinitializer }>, i32 0 }, align 4, !dbg !0
+
+; Function Attrs: nounwind
+define dso_local void @prog1() local_unnamed_addr #0 !dbg !28 {
+entry:
+  %0 = tail call i32 @llvm.bpf.btf.type.id.p0s_struct.anons.i32(%struct.anon* bitcast ({ <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc to %struct.anon*), i32 1), !dbg !31, !llvm.preserve.access.index !7
+  %call = tail call i32 inttoptr (i64 999 to i32 (i32, i8*, i32)*)(i32 %0, i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 0), i32 104) #2, !dbg !32
+  ret void, !dbg !33
+}
+
+; Function Attrs: nounwind readnone
+declare i32 @llvm.bpf.btf.type.id.p0s_struct.anons.i32(%struct.anon*, i32) #1
+
+; Function Attrs: nounwind
+define dso_local void @prog2() local_unnamed_addr #0 !dbg !34 {
+entry:
+  %0 = tail call i32 @llvm.bpf.btf.type.id.p0s_struct.anons.i32(%struct.anon* bitcast ({ <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc to %struct.anon*), i32 0), !dbg !35, !llvm.preserve.access.index !6
+  %call = tail call i32 inttoptr (i64 999 to i32 (i32, i8*, i32)*)(i32 %0, i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 0), i32 104) #2, !dbg !36
+  ret void, !dbg !37
+}
+
+; Function Attrs: nounwind
+define dso_local void @prog3() local_unnamed_addr #0 !dbg !38 {
+entry:
+  %0 = tail call i32 @llvm.bpf.btf.type.id.p0i8.i32(i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 2, i64 1), i32 1), !dbg !39, !llvm.preserve.access.index !11
+  %call = tail call i32 inttoptr (i64 999 to i32 (i32, i8*, i32)*)(i32 %0, i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 0), i32 104) #2, !dbg !40
+  ret void, !dbg !41
+}
+
+; CHECK-LABEL:   prog1
+; CHECK: r1 = 3
+; CHECK-LABEL:   prog2
+; CHECK: r1 = 10
+; CHECK-LABEL:   prog3
+; CHECK: r1 = 4
+;
+; CHECK: .long   0   # BTF_KIND_STRUCT(id = 3)
+; CHECK-NEXT:.long   67108866# 0x402
+; CHECK-NEXT:.long   104
+; CHECK-NEXT:.long   13
+; CHECK-NEXT:.long   5
+; CHECK-NEXT:.long   0   # 0x0
+; CHECK-NEXT:.long   16
+; CHECK-NEXT:.long   7
+; CHECK-NEXT:.long   800 # 0x320
+; CHECK-NEXT:.long   19  # BTF_KIND_INT(id = 4)
+; CHECK-NEXT:.long   16777216# 0x100
+; CHECK-NEXT:.long   1
+; CHECK-NEXT:.long   16777224# 0x108
+; CHECK: .long   0   # BTF_KIND_PTR(id = 10)
+; CHECK-NEXT:.long   33554432# 0x200
+; CHECK-NEXT:.long   3
+;
+; CHECK: .long   16  # FieldReloc
+; CHECK-NEXT:.long   {{[0-9]+}}  # Field reloc section string offset={{[0-9]+}}
+; CHECK-NEXT:.long   3
+; CHECK-NEXT:.long   .Ltmp{{

[PATCH] D69868: Allow "callbr" to return non-void values

[Chris Lattner via Phabricator via cfe-commits]

lattner accepted this revision.
lattner added a comment.

I'm super excited to see this progress towards supporting 'asm goto' with 
results!  Great work!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69868



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

[PATCH] D74757: Fix compiler extension in standalone mode

[serge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3a0f6e699bb6: Fix compiler extension in standalone mode 
(authored by serge-sans-paille).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74757

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

Index: llvm/cmake/modules/CMakeLists.txt
===
--- llvm/cmake/modules/CMakeLists.txt
+++ llvm/cmake/modules/CMakeLists.txt
@@ -136,6 +136,7 @@
 FILES_MATCHING PATTERN *.cmake
 PATTERN .svn EXCLUDE
 PATTERN LLVMConfig.cmake EXCLUDE
+PATTERN LLVMConfigExtensions.cmake EXCLUDE
 PATTERN LLVMConfigVersion.cmake EXCLUDE
 PATTERN LLVM-Config.cmake EXCLUDE
 PATTERN GetHostTriple.cmake EXCLUDE)
Index: llvm/cmake/modules/AddLLVM.cmake
===
--- llvm/cmake/modules/AddLLVM.cmake
+++ llvm/cmake/modules/AddLLVM.cmake
@@ -878,63 +878,62 @@
 if (TARGET intrinsics_gen)
   add_dependencies(obj.${name} intrinsics_gen)
 endif()
-message(STATUS "Registering ${name} as a pass plugin (static build: ${LLVM_${name_upper}_LINK_INTO_TOOLS})")
-set_property(GLOBAL APPEND PROPERTY LLVM_COMPILE_EXTENSIONS ${name})
+set_property(GLOBAL APPEND PROPERTY LLVM_STATIC_EXTENSIONS ${name})
   elseif(NOT ARG_NO_MODULE)
 add_llvm_library(${name} MODULE ${ARG_UNPARSED_ARGUMENTS})
   else()
 add_llvm_library(${name} OBJECT ${ARG_UNPARSED_ARGUMENTS})
   endif()
+  message(STATUS "Registering ${name} as a pass plugin (static build: ${LLVM_${name_upper}_LINK_INTO_TOOLS})")
 
 endfunction(add_llvm_pass_plugin)
 
-# process_llvm_pass_plugins([NO_GEN])
+# process_llvm_pass_plugins([GEN_CONFIG])
 #
 # Correctly set lib dependencies between plugins and tools, based on tools
 # registered with the ENABLE_PLUGINS option.
 #
-# Unless NO_GEN option is set, also generate X Macro file for extension
+# if GEN_CONFIG option is set, also generate X Macro file for extension
 # handling. It provides a HANDLE_EXTENSION(extension_namespace, ExtensionProject)
 # call for each extension allowing client code to define
 # HANDLE_EXTENSION to have a specific code be run for each extension.
 #
 function(process_llvm_pass_plugins)
   cmake_parse_arguments(ARG
-  "NO_GEN" "" ""
+  "GEN_CONFIG" "" ""
 ${ARGN})
 
+  if(ARG_GEN_CONFIG)
+  get_property(LLVM_STATIC_EXTENSIONS GLOBAL PROPERTY LLVM_STATIC_EXTENSIONS)
+  else()
+  include(LLVMConfigExtensions)
+  endif()
+
   # Add static plugins to each plugin target.
-  get_property(LLVM_EXTENSIONS GLOBAL PROPERTY LLVM_COMPILE_EXTENSIONS)
-  foreach(llvm_extension ${LLVM_EXTENSIONS})
-string(TOUPPER ${llvm_extension} llvm_extension_upper)
-string(TOLOWER ${llvm_extension} llvm_extension_lower)
-
-if(LLVM_${llvm_extension_upper}_LINK_INTO_TOOLS)
-  get_property(llvm_plugin_targets GLOBAL PROPERTY LLVM_PLUGIN_TARGETS)
-  foreach(llvm_plugin_target ${llvm_plugin_targets})
-set_property(TARGET ${llvm_plugin_target} APPEND PROPERTY LINK_LIBRARIES ${llvm_extension})
-set_property(TARGET ${llvm_plugin_target} APPEND PROPERTY INTERFACE_LINK_LIBRARIES ${llvm_extension})
-  endforeach()
-else()
-  add_llvm_library(${llvm_extension_lower} MODULE obj.${llvm_extension_lower})
-endif()
+  foreach(llvm_extension ${LLVM_STATIC_EXTENSIONS})
+get_property(llvm_plugin_targets GLOBAL PROPERTY LLVM_PLUGIN_TARGETS)
+foreach(llvm_plugin_target ${llvm_plugin_targets})
+  set_property(TARGET ${llvm_plugin_target} APPEND PROPERTY LINK_LIBRARIES ${llvm_extension})
+  set_property(TARGET ${llvm_plugin_target} APPEND PROPERTY INTERFACE_LINK_LIBRARIES ${llvm_extension})
+endforeach()
   endforeach()
 
-  # Eventually generate the extension header.
-  if(NOT ARG_NO_GEN)
-  file(WRITE "${LLVM_BINARY_DIR}/include/llvm/Support/Extension.def.tmp" "//extension handlers\n")
-  foreach(llvm_extension ${LLVM_EXTENSIONS})
-string(TOLOWER ${llvm_extension} llvm_extension_lower)
-
-string(TOUPPER ${llvm_extension} llvm_extension_upper)
-string(SUBSTRING ${llvm_extension_upper} 0 1 llvm_extension_upper_first)
-string(SUBSTRING ${llvm_extension_lower} 1 -1 llvm_extension_lower_tail)
-string(CONCAT llvm_extension_project ${llvm_extension_upper_first} ${llvm_extension_lower_tail})
-
-if(LLVM_${llvm_extension_upper}_LINK_INTO_TOOLS)
-  file(APPEND "${LLVM_BINARY_DIR}/include/llvm/Support/Extension.def.tmp" "HANDLE_EXTENSION(${llvm_extension_project})\n")
-endif()
+  # Eventually generate the extension header, and store config to a cmake file
+  # for usage in third-party configuration.
+  if(ARG_GEN_CONFIG)
+  set(LLVM_INSTALL_PACKAGE_DIR lib${LLVM_LIBDIR_SUFFIX}/cmake/llvm)
+  set(llvm_cma

[clang] 3a0f6e6 - Fix compiler extension in standalone mode

[via cfe-commits]

Author: serge-sans-paille
Date: 2020-02-20T07:19:04+01:00
New Revision: 3a0f6e699bb6d96dc62dce6faef20ac26cf103fd

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

LOG: Fix compiler extension in standalone mode

Use a dedicated cmake file to store the extension configured within LLVM. That
way, a standalone build of clang can load this cmake file and get all the
configured standalone extensions.

This patch is related to https://reviews.llvm.org/D74602

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

Added: 


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

Removed: 




diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index 093c2225e525..dc1413f4b597 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -864,7 +864,7 @@ add_subdirectory(utils/hmaptool)
 
 if(CLANG_BUILT_STANDALONE)
   llvm_distribution_add_targets()
-  process_llvm_pass_plugins(NO_GEN)
+  process_llvm_pass_plugins()
 endif()
 
 configure_file(

diff  --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index fa6e418ac06f..5133139d6ce3 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -1075,7 +1075,7 @@ endif()
 # after all targets are created.
 include(LLVMDistributionSupport)
 llvm_distribution_add_targets()
-process_llvm_pass_plugins()
+process_llvm_pass_plugins(GEN_CONFIG)
 
 # This allows us to deploy the Universal CRT DLLs by passing 
-DCMAKE_INSTALL_UCRT_LIBRARIES=ON to CMake
 if (MSVC AND CMAKE_HOST_SYSTEM_NAME STREQUAL "Windows" AND 
CMAKE_INSTALL_UCRT_LIBRARIES)

diff  --git a/llvm/cmake/modules/AddLLVM.cmake 
b/llvm/cmake/modules/AddLLVM.cmake
index 408423ba1887..f66a8ccdb469 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -878,63 +878,62 @@ function(add_llvm_pass_plugin name)
 if (TARGET intrinsics_gen)
   add_dependencies(obj.${name} intrinsics_gen)
 endif()
-message(STATUS "Registering ${name} as a pass plugin (static build: 
${LLVM_${name_upper}_LINK_INTO_TOOLS})")
-set_property(GLOBAL APPEND PROPERTY LLVM_COMPILE_EXTENSIONS ${name})
+set_property(GLOBAL APPEND PROPERTY LLVM_STATIC_EXTENSIONS ${name})
   elseif(NOT ARG_NO_MODULE)
 add_llvm_library(${name} MODULE ${ARG_UNPARSED_ARGUMENTS})
   else()
 add_llvm_library(${name} OBJECT ${ARG_UNPARSED_ARGUMENTS})
   endif()
+  message(STATUS "Registering ${name} as a pass plugin (static build: 
${LLVM_${name_upper}_LINK_INTO_TOOLS})")
 
 endfunction(add_llvm_pass_plugin)
 
-# process_llvm_pass_plugins([NO_GEN])
+# process_llvm_pass_plugins([GEN_CONFIG])
 #
 # Correctly set lib dependencies between plugins and tools, based on tools
 # registered with the ENABLE_PLUGINS option.
 #
-# Unless NO_GEN option is set, also generate X Macro file for extension
+# if GEN_CONFIG option is set, also generate X Macro file for extension
 # handling. It provides a HANDLE_EXTENSION(extension_namespace, 
ExtensionProject)
 # call for each extension allowing client code to define
 # HANDLE_EXTENSION to have a specific code be run for each extension.
 #
 function(process_llvm_pass_plugins)
   cmake_parse_arguments(ARG
-  "NO_GEN" "" ""
+  "GEN_CONFIG" "" ""
 ${ARGN})
 
+  if(ARG_GEN_CONFIG)
+  get_property(LLVM_STATIC_EXTENSIONS GLOBAL PROPERTY 
LLVM_STATIC_EXTENSIONS)
+  else()
+  include(LLVMConfigExtensions)
+  endif()
+
   # Add static plugins to each plugin target.
-  get_property(LLVM_EXTENSIONS GLOBAL PROPERTY LLVM_COMPILE_EXTENSIONS)
-  foreach(llvm_extension ${LLVM_EXTENSIONS})
-string(TOUPPER ${llvm_extension} llvm_extension_upper)
-string(TOLOWER ${llvm_extension} llvm_extension_lower)
-
-if(LLVM_${llvm_extension_upper}_LINK_INTO_TOOLS)
-  get_property(llvm_plugin_targets GLOBAL PROPERTY LLVM_PLUGIN_TARGETS)
-  foreach(llvm_plugin_target ${llvm_plugin_targets})
-set_property(TARGET ${llvm_plugin_target} APPEND PROPERTY 
LINK_LIBRARIES ${llvm_extension})
-set_property(TARGET ${llvm_plugin_target} APPEND PROPERTY 
INTERFACE_LINK_LIBRARIES ${llvm_extension})
-  endforeach()
-else()
-  add_llvm_library(${llvm_extension_lower} MODULE 
obj.${llvm_extension_lower})
-endif()
+  foreach(llvm_extension ${LLVM_STATIC_EXTENSIONS})
+get_property(llvm_plugin_targets GLOBAL PROPERTY LLVM_PLUGIN_TARGETS)
+foreach(llvm_plugin_target ${llvm_plugin_targets})
+  set_property(TARGET ${llvm_plugin_target} APPEND PROPERTY LINK_LIBRARIES 
${llvm_extension})
+  set_property(TARGET ${llvm_plugin_target} APPEND PROPERTY 
INTERFACE_LINK_LIBRARIES ${llvm_extension})
+endforeach()
   endforeach()
 
-  # Eventually generate the extension header.
-  if(NOT ARG_NO_GEN)
-  file(WRITE "${LLVM_BINARY_DIR}/include/l

[PATCH] D74871: Fix interaction between -fdiscard-value-names and LLVM Bitcode

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

In D74871#1883884 , @EricWF wrote:

> Why is this not a bug in w/e is handling the IR?


It may totally be. Consider this review as a way to start the discussion. The 
problem, as it appears, is triggered when some value is parsed in non-discard 
mode (which is the only valid mode when parsing IR (see 
https://github.com/llvm/llvm-project/blob/master/llvm/lib/AsmParser/LLParser.cpp#L71))
 and then the Value is destroyed. Another working patch I've experimented with 
modifies `Value::setName` and still runs the function body in discard mode if 
the value actually has a name. Would you prefer that approach?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74871



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

[PATCH] D74871: Fix interaction between -fdiscard-value-names and LLVM Bitcode

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 245578.
serge-sans-paille added a comment.

Added test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74871

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3603,6 +3603,11 @@
 LangOpts.PIE = Args.hasArg(OPT_pic_is_pie);
 parseSanitizerKinds("-fsanitize=", Args.getAllArgValues(OPT_fsanitize_EQ),
 Diags, LangOpts.Sanitize);
+// Cannot discard value names when processing llvm-ir, because IR loading
+// is conservative wrt. names.
+if (Res.getCodeGenOpts().DiscardValueNames) {
+  Res.getCodeGenOpts().DiscardValueNames = false;
+}
   } else {
 // Other LangOpts are only initialized when the input is not AST or LLVM 
IR.
 // FIXME: Should we really be calling this for an Language::Asm input?
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4332,8 +4332,15 @@
 
   // Discard value names in assert builds unless otherwise specified.
   if (Args.hasFlag(options::OPT_fdiscard_value_names,
-   options::OPT_fno_discard_value_names, !IsAssertBuild))
+   options::OPT_fno_discard_value_names, !IsAssertBuild)) {
+if (std::any_of(Inputs.begin(), Inputs.end(),
+[](const clang::driver::InputInfo &II) {
+  return types::isLLVMIR(II.getType());
+})) {
+  D.Diag(diag::warn_ignoring_fdiscard_for_bitcode);
+}
 CmdArgs.push_back("-discard-value-names");
+  }
 
   // Set the main file name, so that debug info works even with
   // -save-temps.
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -271,6 +271,9 @@
   InGroup;
 def warn_c_kext : Warning<
   "ignoring -fapple-kext which is valid for C++ and Objective-C++ only">;
+def warn_ignoring_fdiscard_for_bitcode : Warning<
+  "ignoring -fdiscard-value-names for LLVM Bitcode">,
+  InGroup;
 def warn_drv_input_file_unused : Warning<
   "%0: '%1' input unused%select{ when '%3' is present|}2">,
   InGroup;


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3603,6 +3603,11 @@
 LangOpts.PIE = Args.hasArg(OPT_pic_is_pie);
 parseSanitizerKinds("-fsanitize=", Args.getAllArgValues(OPT_fsanitize_EQ),
 Diags, LangOpts.Sanitize);
+// Cannot discard value names when processing llvm-ir, because IR loading
+// is conservative wrt. names.
+if (Res.getCodeGenOpts().DiscardValueNames) {
+  Res.getCodeGenOpts().DiscardValueNames = false;
+}
   } else {
 // Other LangOpts are only initialized when the input is not AST or LLVM IR.
 // FIXME: Should we really be calling this for an Language::Asm input?
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4332,8 +4332,15 @@
 
   // Discard value names in assert builds unless otherwise specified.
   if (Args.hasFlag(options::OPT_fdiscard_value_names,
-   options::OPT_fno_discard_value_names, !IsAssertBuild))
+   options::OPT_fno_discard_value_names, !IsAssertBuild)) {
+if (std::any_of(Inputs.begin(), Inputs.end(),
+[](const clang::driver::InputInfo &II) {
+  return types::isLLVMIR(II.getType());
+})) {
+  D.Diag(diag::warn_ignoring_fdiscard_for_bitcode);
+}
 CmdArgs.push_back("-discard-value-names");
+  }
 
   // Set the main file name, so that debug info works even with
   // -save-temps.
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -271,6 +271,9 @@
   InGroup;
 def warn_c_kext : Warning<
   "ignoring -fapple-kext which is valid for C++ and Objective-C++ only">;
+def warn_ignoring_fdiscard_for_bitcode : Warning<
+  "ignoring -fdiscard-value-names for LLVM Bitcode">,
+  InGroup;
 def warn_drv_input_file_unused : Warning<
   "%0: '%1' input unused%select{ 

[PATCH] D74572: [BPF] preserve debuginfo types for builtin __builtin__btf_type_id()

[Alexei Starovoitov via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/BPF/BPFPreserveDIType.cpp:1
+//===- BPFPreserveType.cpp - Preserve DebugInfo Types 
-===//
+//

ast wrote:
> BPFPreserveDIType.cpp
this typo wasn't fixed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74572



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

[PATCH] D74811: [Driver] Escape the program path for -frecord-command-line

[Ravi Ramaseshan via Phabricator via cfe-commits]

ravi-ramaseshan added a comment.

Can you please land this change for me since I do not have commit rights. 
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74811



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

[PATCH] D74572: [BPF] preserve debuginfo types for builtin __builtin__btf_type_id()

[Yonghong Song via Phabricator via cfe-commits]

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

change llvm.bpf_pdit to llvm.btf_type_id variable name to make it easy to 
understand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74572

Files:
  llvm/lib/Target/BPF/BPF.h
  llvm/lib/Target/BPF/BPFCORE.h
  llvm/lib/Target/BPF/BPFMISimplifyPatchable.cpp
  llvm/lib/Target/BPF/BPFPreserveDIType.cpp
  llvm/lib/Target/BPF/BPFTargetMachine.cpp
  llvm/lib/Target/BPF/BTFDebug.cpp
  llvm/lib/Target/BPF/BTFDebug.h
  llvm/lib/Target/BPF/CMakeLists.txt
  llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll

Index: llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll
===
--- /dev/null
+++ llvm/test/CodeGen/BPF/BTF/builtin-btf-type-id.ll
@@ -0,0 +1,147 @@
+; RUN: llc -march=bpfel -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
+; RUN: llc -march=bpfeb -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
+; RUN: llc -march=bpfel -mattr=+alu32 -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
+; RUN: llc -march=bpfeb -mattr=+alu32 -filetype=asm -o - %s | FileCheck -check-prefixes=CHECK %s
+;
+; Source code:
+;   static int (*bpf_log)(unsigned tid, void *data, int data_size) = (void *)999;
+;   struct {
+; char f1[100];
+; typeof(3) f2;
+;   } tmp__abc = {1, 3};
+;   void prog1() {
+; bpf_log(__builtin_btf_type_id(tmp__abc), &tmp__abc, sizeof(tmp__abc));
+;   }
+;   void prog2() {
+; bpf_log(__builtin_btf_type_id(&tmp__abc), &tmp__abc, sizeof(tmp__abc));
+;   }
+;   void prog3() {
+; bpf_log(__builtin_btf_type_id(tmp__abc.f1[3]), &tmp__abc, sizeof(tmp__abc));
+;   }
+; Compilation flag:
+;   clang -target bpf -O2 -g -S -emit-llvm test.c
+
+%struct.anon = type { [100 x i8], i32 }
+
+@tmp__abc = dso_local global { <{ i8, i8, [98 x i8] }>, i32 } { <{ i8, i8, [98 x i8] }> <{ i8 1, i8 3, [98 x i8] zeroinitializer }>, i32 0 }, align 4, !dbg !0
+
+; Function Attrs: nounwind
+define dso_local void @prog1() local_unnamed_addr #0 !dbg !28 {
+entry:
+  %0 = tail call i32 @llvm.bpf.btf.type.id.p0s_struct.anons.i32(%struct.anon* bitcast ({ <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc to %struct.anon*), i32 1), !dbg !31, !llvm.preserve.access.index !7
+  %call = tail call i32 inttoptr (i64 999 to i32 (i32, i8*, i32)*)(i32 %0, i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 0), i32 104) #2, !dbg !32
+  ret void, !dbg !33
+}
+
+; Function Attrs: nounwind readnone
+declare i32 @llvm.bpf.btf.type.id.p0s_struct.anons.i32(%struct.anon*, i32) #1
+
+; Function Attrs: nounwind
+define dso_local void @prog2() local_unnamed_addr #0 !dbg !34 {
+entry:
+  %0 = tail call i32 @llvm.bpf.btf.type.id.p0s_struct.anons.i32(%struct.anon* bitcast ({ <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc to %struct.anon*), i32 0), !dbg !35, !llvm.preserve.access.index !6
+  %call = tail call i32 inttoptr (i64 999 to i32 (i32, i8*, i32)*)(i32 %0, i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 0), i32 104) #2, !dbg !36
+  ret void, !dbg !37
+}
+
+; Function Attrs: nounwind
+define dso_local void @prog3() local_unnamed_addr #0 !dbg !38 {
+entry:
+  %0 = tail call i32 @llvm.bpf.btf.type.id.p0i8.i32(i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 2, i64 1), i32 1), !dbg !39, !llvm.preserve.access.index !11
+  %call = tail call i32 inttoptr (i64 999 to i32 (i32, i8*, i32)*)(i32 %0, i8* getelementptr inbounds ({ <{ i8, i8, [98 x i8] }>, i32 }, { <{ i8, i8, [98 x i8] }>, i32 }* @tmp__abc, i64 0, i32 0, i32 0), i32 104) #2, !dbg !40
+  ret void, !dbg !41
+}
+
+; CHECK-LABEL:   prog1
+; CHECK: r1 = 3
+; CHECK-LABEL:   prog2
+; CHECK: r1 = 10
+; CHECK-LABEL:   prog3
+; CHECK: r1 = 4
+;
+; CHECK: .long   0   # BTF_KIND_STRUCT(id = 3)
+; CHECK-NEXT:.long   67108866# 0x402
+; CHECK-NEXT:.long   104
+; CHECK-NEXT:.long   13
+; CHECK-NEXT:.long   5
+; CHECK-NEXT:.long   0   # 0x0
+; CHECK-NEXT:.long   16
+; CHECK-NEXT:.long   7
+; CHECK-NEXT:.long   800 # 0x320
+; CHECK-NEXT:.long   19  # BTF_KIND_INT(id = 4)
+; CHECK-NEXT:.long   16777216# 0x100
+; CHECK-NEXT:.long   1
+; CHECK-NEXT:.long   16777224# 0x108
+; CHECK: .long   0   # BTF_KIND_PTR(id = 10)
+; CHECK-NEXT:.long   33554432# 0x200
+; CHECK-NEXT:.long   3
+;
+; CHECK: .long   16  # FieldReloc
+; CHECK-NEXT:.long   {{[0-9]+}}  # Field reloc section string offset={{[0-9]+}}
+; CHECK-NEXT:.long   3
+; CHECK-NEXT:.lo

[PATCH] D74572: [BPF] preserve debuginfo types for builtin __builtin__btf_type_id()

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song marked an inline comment as done.
yonghong-song added inline comments.



Comment at: llvm/lib/Target/BPF/BPFPreserveDIType.cpp:95
+
+  std::string BaseName = "llvm.bpf_pdit.";
+  int Count = 0;

ast wrote:
> may be "llvm.btf_type_id." instead?
> 
Oh, yes. Make sense. Forget to change this one. Will update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74572



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

[PATCH] D74692: [clang-tidy] Make bugprone-use-after-move ignore std::move for const values

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

(I sent this to the mailing list, but I guess it doesn't show up here unless I 
do it through Phab. Quoting myself—)

I see your point about how users who care should always be passing this check 
alongside "performance-move-const-arg"; but IMHO it still makes sense for 
clang-tidy to warn unconditionally about code of the form

  x = std::move(y);
  use(y);

regardless of the incidental type of `y`. Sure, it's //technically// not wrong 
if `y` is const... or if `y` is a primitive type, such as `Widget*`... or if 
`y` is a well-defined library type, such as `std::shared_ptr`... or if 
`y` is a library type that works in practice 
,
 such as `std::list`... but regardless of its technical merits, the code 
is still //logically semantically// wrong, and I think that's what the 
clang-tidy check should be trying to diagnose.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74692



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

[PATCH] D69471: [Coverage] Revise format to reduce binary size

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

dexonsmith added a comment.

In D69471#1883912 , @rnk wrote:

> Everything is off-by-one because the empty bases are not zero sized. The MSVC 
> record layout algorithm is just different in this area. =/


Do all the MSVCs we support building with support `__declspec(empty_bases)`?
https://devblogs.microsoft.com/cppblog/optimizing-the-layout-of-empty-base-classes-in-vs2015-update-2-3/

> So, I think this patch would be fine if you refactor it to avoid the accessor 
> classes. I took a stab at it, but it's not straightforward.

Another option is to chain the accessor classes:

  template  struct MyAccessor1 : Base { /* ... */ };
  template  struct MyAccessor1 { /* ... */ };
  template  struct MyAccessor2 : Base { /* ... */ };
  template  struct MyAccessor2 { /* ... */ };

Then you can:

  struct MyFormat : MyAccessor1> { /* ... */ };


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

https://reviews.llvm.org/D69471



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

[PATCH] D73580: [clang-tidy] rename_check.py: maintain alphabetical order in Renamed checks section

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

In D73580#1883861 , @njames93 wrote:

> LGTM, For what this is trying to achieve I have no issue, follow up patches 
> can be submitted for the other enhancements needed.


Please commit patch if you'll have time. See summary for details.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73580



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

[clang] 490a9a4 - [examples] Fix the clang-interpreter example for changes in 85fb997659b.

[Lang Hames via cfe-commits]

Author: Lang Hames
Date: 2020-02-19T19:01:32-08:00
New Revision: 490a9a4b77ea23f388cae67d732af6bd8aa576f9

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

LOG: [examples] Fix the clang-interpreter example for changes in 85fb997659b.

Added: 


Modified: 
clang/examples/clang-interpreter/main.cpp

Removed: 




diff  --git a/clang/examples/clang-interpreter/main.cpp 
b/clang/examples/clang-interpreter/main.cpp
index c0aae4722306..6b4cdca15fb0 100644
--- a/clang/examples/clang-interpreter/main.cpp
+++ b/clang/examples/clang-interpreter/main.cpp
@@ -54,7 +54,7 @@ class SimpleJIT {
   std::unique_ptr TM;
   const DataLayout DL;
   MangleAndInterner Mangle{ES, DL};
-  JITDylib &MainJD{ES.createJITDylib("")};
+  JITDylib &MainJD{ES.createBareJITDylib("")};
   RTDyldObjectLinkingLayer ObjectLayer{ES, createMemMgr};
   IRCompileLayer CompileLayer{ES, ObjectLayer,
   std::make_unique(*TM)};



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

[clang-tools-extra] 6730f39 - Fixup test after changes made in 709fd989.

[Douglas Yung via cfe-commits]

Author: Douglas Yung
Date: 2020-02-19T18:39:54-08:00
New Revision: 6730f390a1fb069e567ebd635390de381d21b994

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

LOG: Fixup test after changes made in 709fd989.

The change added a test that required exceptions, so enable that explicitly
so that it works on platforms that default to having exceptions disabled
(like the PS4).

Added: 


Modified: 

clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp

Removed: 




diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
index 84dfeb215f4f..3570fcff4a5c 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy -std=c++11,c++14,c++17 %s 
modernize-use-default-member-init %t
+// RUN: %check_clang_tidy -std=c++11,c++14,c++17 %s 
modernize-use-default-member-init %t -- -- -fexceptions
 // FIXME: Fix the checker to work in C++2a mode.
 
 struct S {



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

[PATCH] D69471: [Coverage] Revise format to reduce binary size

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Compiling with -fdump-record-layouts revealed the problem:

  *** Dumping AST Record Layout
   0 | struct llvm::coverage::CovMapFunctionRecordV3
   0 |   struct llvm::coverage::accessors::FuncHashAndDataSize (base) (empty)
   1 |   struct llvm::coverage::accessors::HashedNameRef (base) (empty)
   1 |   const int64_t NameRef
   9 |   const uint32_t DataSize
  13 |   const uint64_t FuncHash
  21 |   const uint64_t FilenamesRef
  29 |   const char CoverageMapping
 | [sizeof=30, align=1,
 |  nvsize=30, nvalign=1]

Everything is off-by-one because the empty bases are not zero sized. The MSVC 
record layout algorithm is just different in this area. =/

So, I think this patch would be fine if you refactor it to avoid the accessor 
classes. I took a stab at it, but it's not straightforward.


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

https://reviews.llvm.org/D69471



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

[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:1514
+  void visitUsedDecl(SourceLocation Loc, Decl *D) {
+if (auto *TD = dyn_cast(D)) {
+  for (auto *DD : TD->decls()) {

yaxunl wrote:
> rjmccall wrote:
> > erichkeane wrote:
> > > rjmccall wrote:
> > > > erichkeane wrote:
> > > > > Note that when recommitting this (if you choose to), this needs to 
> > > > > also handle NamespaceDecl.  We're a downstream and discovered that 
> > > > > this doesn't properly handle functions or records handled in a 
> > > > > namespace.
> > > > > 
> > > > > It can be implemented identically to TranslationUnitDecl.
> > > > Wait, what?  We shouldn't be doing this for TranslationUnitDecl either. 
> > > >   I don't even know how we're "using" a TranslationUnitDecl, but 
> > > > neither this case not the case for `NamespaceDecl` should be 
> > > > recursively using every declaration declared inside it.  If there's a 
> > > > declaration in a namespace that's being used, it should be getting 
> > > > visited as part of the actual use of it.
> > > > 
> > > > The logic for `RecordDecl` has the same problem.  
> > > Despite the name, this seems to be more of a home-written ast walking 
> > > class.  The entry point is the 'translation unit' which seems to walk 
> > > through everything in an attempt to find all the functions (including 
> > > those that are 'marked' as used by an attribute).
> > > 
> > > You'll see the FunctionDecl section makes this assumption as well (not 
> > > necessarily that we got to a function via a call). IMO, this approach is 
> > > strange, and we should register entry points in some manner (functions 
> > > marked as emitted to the device in some fashion), then just follow its 
> > > call-graph (via the clang::CallGraph?) to emit all of these functions.
> > > 
> > > It seemed really odd to see this approach here, but it seemed well 
> > > reviewed by the time I noticed it (via a downstream bug) so I figured I'd 
> > > lost my chance to disagree with the approach.
> > > 
> > > 
> > Sure, but `visitUsedDecl` isn't the right place to be entering the walk.  
> > `visitUsedDecl` is supposed to be the *callback* from the walk.  If they 
> > need to walk all the global declarations to find kernels instead of 
> > tracking the kernels as they're encountered (which would be a *much* better 
> > approach), it should be done as a separate function.
> > 
> > I just missed this in the review.
> The deferred diagnostics could be initiated by non-kernel functions or even 
> host functions.
> 
> Let's consider a device code library where no kernels are defined. A device 
> function is emitted, which calls a host device function which has a deferred 
> diagnostic. All device functions that are emitted need to be checked.
> 
> Same with host functions that are emitted, which may call a host device 
> function which has deferred diagnostic.
> 
> Also not just function calls need to be checked. A function address may be 
> taken then called through function pointer. Therefore any reference to a 
> function needs to be followed.
> 
> In the case of OpenMP, the initialization of a global function pointer which 
> refers a function may trigger a deferred diangostic. There are tests for that.
Right, I get that emitting deferred diagnostics for a declaration D needs to 
trigger any deferred diagnostics in declarations used by D, recursively.  You 
essentially have a graph of lazily-emitted declarations (which may or may not 
have deferred diagnostics) and a number of eagerly-emitted "root" declarations 
with use-edges leading into that graph.  Any declaration that's reachable from 
a root will need to be emitted and so needs to have any deferred diagnostics 
emitted as well.  My question is why you're finding these roots with a 
retroactive walk of the entire translation unit instead of either building a 
list of roots as you go or (better yet) building a list of lazily-emitted 
declarations that are used by those roots.  You can unambiguously identify at 
the point of declaration whether an entity will be eagerly or lazily emitted, 
right?  If you just store those initial edges into the lazily-emitted 
declarations graph and then initiate the recursive walk from them at the end of 
the translation unit, you'll only end up walking declarations that are actually 
relevant to your compilation, so you'll have much better locality and (if this 
matters to you) you'll naturally work a lot better with PCH and modules.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70172



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

[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

I'd suggest possibly adding 2 Options at Global Level SourceFileExtensions and 
HeaderFileExtensions, both would take semicolon seperated lists. 
Reason they are Global is there are probably other checks that could use them.
The SourceFileExtensions could be defaulted to `.c;.cc;.cpp;.cxx` and the 
HeaderFileExtensions `.h;.hh;.hpp;`.
You need the `` because how Options get parsed, then it can manually be 
transformed to `""` when you read it.


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

https://reviews.llvm.org/D74669



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

[PATCH] D74572: [BPF] preserve debuginfo types for builtin __builtin__btf_type_id()

[Alexei Starovoitov via Phabricator via cfe-commits]

ast added inline comments.



Comment at: llvm/lib/Target/BPF/BPFPreserveDIType.cpp:1
+//===- BPFPreserveType.cpp - Preserve DebugInfo Types 
-===//
+//

BPFPreserveDIType.cpp



Comment at: llvm/lib/Target/BPF/BPFPreserveDIType.cpp:95
+
+  std::string BaseName = "llvm.bpf_pdit.";
+  int Count = 0;

may be "llvm.btf_type_id." instead?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74572



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

[PATCH] D74668: [Clang][BPF] implement __builtin_btf_type_id() builtin function

[Alexei Starovoitov via Phabricator via cfe-commits]

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

lgtm. Thanks for explaining lvalue/value trick.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74668



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

[PATCH] D74452: [MS] Mark vectorcall FP and vector args inreg

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0edb2129258c: [MS] Mark vectorcall FP and vector args inreg 
(authored by rnk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74452

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/vectorcall.c
  clang/test/CodeGenCXX/inalloca-vector.cpp

Index: clang/test/CodeGenCXX/inalloca-vector.cpp
===
--- clang/test/CodeGenCXX/inalloca-vector.cpp
+++ clang/test/CodeGenCXX/inalloca-vector.cpp
@@ -66,14 +66,13 @@
  __m128 w, int edx, __m128 q, NonTrivial nt) {
   gv128 = x + y + z + w + q;
 }
-// FIXME: Enable these checks, clang generates wrong IR.
 // CHECK-LABEL: define dso_local x86_vectorcallcc void @"?vectorcall_receive_vec@@Y{{[^"]*}}"
-// CHECKX-SAME: (double inreg %xmm0,
-// CHECKX-SAME: double inreg %xmm1,
-// CHECKX-SAME: double inreg %xmm2,
-// CHECKX-SAME: <4 x float> inreg %x,
-// CHECKX-SAME: <4 x float> inreg %y,
-// CHECKX-SAME: <4 x float> inreg %z,
-// CHECKX-SAME: <4 x float>* inreg %0,
-// CHECKX-SAME: i32 inreg %edx,
-// CHECKX-SAME: <{ <4 x float>*, %struct.NonTrivial }>* inalloca %1)
+// CHECK-SAME: (double inreg %xmm0,
+// CHECK-SAME: double inreg %xmm1,
+// CHECK-SAME: double inreg %xmm2,
+// CHECK-SAME: <4 x float> inreg %x,
+// CHECK-SAME: <4 x float> inreg %y,
+// CHECK-SAME: <4 x float> inreg %z,
+// CHECK-SAME: <4 x float>* inreg %0,
+// CHECK-SAME: i32 inreg %edx,
+// CHECK-SAME: <{ <4 x float>*, %struct.NonTrivial }>* inalloca %1)
Index: clang/test/CodeGen/vectorcall.c
===
--- clang/test/CodeGen/vectorcall.c
+++ clang/test/CodeGen/vectorcall.c
@@ -31,13 +31,13 @@
 // indirectly. Additional vector arguments can consume the rest of the SSE
 // registers.
 void __vectorcall hfa2(struct HFA4 a, struct HFA4 b, double c) {}
-// X32: define dso_local x86_vectorcallcc void @"\01hfa2@@72"(%struct.HFA4 inreg %a.coerce, %struct.HFA4* inreg %b, double %c)
+// X32: define dso_local x86_vectorcallcc void @"\01hfa2@@72"(%struct.HFA4 inreg %a.coerce, %struct.HFA4* inreg %b, double inreg %c)
 // X64: define dso_local x86_vectorcallcc void @"\01hfa2@@72"(%struct.HFA4 inreg %a.coerce, %struct.HFA4* %b, double %c)
 
 // Ensure that we pass builtin types directly while counting them against the
 // SSE register usage.
 void __vectorcall hfa3(double a, double b, double c, double d, double e, struct HFA2 f) {}
-// X32: define dso_local x86_vectorcallcc void @"\01hfa3@@56"(double %a, double %b, double %c, double %d, double %e, %struct.HFA2* inreg %f)
+// X32: define dso_local x86_vectorcallcc void @"\01hfa3@@56"(double inreg %a, double inreg %b, double inreg %c, double inreg %d, double inreg %e, %struct.HFA2* inreg %f)
 // X64: define dso_local x86_vectorcallcc void @"\01hfa3@@56"(double %a, double %b, double %c, double %d, double %e, %struct.HFA2* %f)
 
 // Aggregates with more than four elements are not HFAs and are passed byval.
@@ -64,21 +64,21 @@
 // X64: define dso_local x86_vectorcallcc <4 x float> @"\01hva1@@80"(i32 %a, %struct.HVA4 inreg %b.coerce, i32 %c)
 
 v4f32 __vectorcall hva2(struct HVA4 a, struct HVA4 b, v4f32 c) {return c;}
-// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva2@@144"(%struct.HVA4 inreg %a.coerce, %struct.HVA4* inreg %b, <4 x float> %c)
+// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva2@@144"(%struct.HVA4 inreg %a.coerce, %struct.HVA4* inreg %b, <4 x float> inreg %c)
 // X64: define dso_local x86_vectorcallcc <4 x float> @"\01hva2@@144"(%struct.HVA4 inreg %a.coerce, %struct.HVA4* %b, <4 x float> %c)
 
 v4f32 __vectorcall hva3(v4f32 a, v4f32 b, v4f32 c, v4f32 d, v4f32 e, struct HVA2 f) {return f.x;}
-// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva3@@112"(<4 x float> %a, <4 x float> %b, <4 x float> %c, <4 x float> %d, <4 x float> %e, %struct.HVA2* inreg %f)
+// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva3@@112"(<4 x float> inreg %a, <4 x float> inreg %b, <4 x float> inreg %c, <4 x float> inreg %d, <4 x float> inreg %e, %struct.HVA2* inreg %f)
 // X64: define dso_local x86_vectorcallcc <4 x float> @"\01hva3@@112"(<4 x float> %a, <4 x float> %b, <4 x float> %c, <4 x float> %d, <4 x float> %e, %struct.HVA2* %f)
 
 // vector types have higher priority then HVA structures, So vector types are allocated first
 // and HVAs are allocated if enough registers are available
 v4f32 __vectorcall hva4(struct HVA4 a, struct HVA2 b, v4f32 c) {return b.y;}
-// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva4@@112"(%struct.HVA4 inreg %a.coerce, %struct.HVA2* inreg %b, <4 x float> %c)
+// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva4@@112"(%struct.HVA4 inreg %a.coerce, %struct.HVA2* inreg %b, <4 x float> inreg %c)
 // X64: define dso_local x86_vectorcallcc <4 x 

[PATCH] D74878: [remark][diagnostics] [codegen] Fix PR44896

[Rong Xu via Phabricator via cfe-commits]

xur created this revision.
xur added reviewers: tejohnson, jeroen.dobbelaere.

This patch fixes PR44896.

For IR input files, option fdiscard-value-names should be ignored as we need 
named values in loadModule().
Commit 60d3947922 
 sets this 
option after loadModule() where valued names already created. This creates an 
inconsistent state in setNameImpl() that leads to a seg fault.
This patch forces fdiscard-value-names to be false for IR input files.


https://reviews.llvm.org/D74878

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/test/CodeGen/PR44896.ll


Index: clang/test/CodeGen/PR44896.ll
===
--- /dev/null
+++ clang/test/CodeGen/PR44896.ll
@@ -0,0 +1,7 @@
+; RUN: %clang_cc1 -triple x86_64-unknown-unknown -O3 -S -emit-llvm %s 
-discard-value-names -o -
+; PR 44896
+
+define linkonce i8* @b(i8* %a) {
+  ret i8* %a
+}
+
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -1146,6 +1146,9 @@
CI.getTargetOpts(), CI.getLangOpts(),
CI.getFrontendOpts().ShowTimers,
std::move(LinkModules), *VMContext, nullptr);
+// PR44896: Force DiscardValueNames as false. DiscardValueNames cannot be
+// true here because the valued names are needed for reading textual IR.
+Ctx.setDiscardValueNames(false);
 Ctx.setDiagnosticHandler(
 std::make_unique(CodeGenOpts, &Result));
 


Index: clang/test/CodeGen/PR44896.ll
===
--- /dev/null
+++ clang/test/CodeGen/PR44896.ll
@@ -0,0 +1,7 @@
+; RUN: %clang_cc1 -triple x86_64-unknown-unknown -O3 -S -emit-llvm %s -discard-value-names -o -
+; PR 44896
+
+define linkonce i8* @b(i8* %a) {
+  ret i8* %a
+}
+
Index: clang/lib/CodeGen/CodeGenAction.cpp
===
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -1146,6 +1146,9 @@
CI.getTargetOpts(), CI.getLangOpts(),
CI.getFrontendOpts().ShowTimers,
std::move(LinkModules), *VMContext, nullptr);
+// PR44896: Force DiscardValueNames as false. DiscardValueNames cannot be
+// true here because the valued names are needed for reading textual IR.
+Ctx.setDiscardValueNames(false);
 Ctx.setDiagnosticHandler(
 std::make_unique(CodeGenOpts, &Result));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73580: [clang-tidy] rename_check.py: maintain alphabetical order in Renamed checks section

[Nathan James via Phabricator via cfe-commits]

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

LGTM, For what this is trying to achieve I have no issue, follow up patches can 
be submitted for the other enhancements needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73580



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

[PATCH] D74871: Fix interaction between -fdiscard-value-names and LLVM Bitcode

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

This seems wrong to me. Discarding value names is the default and has been 
forever in non-assert builds.
Why is this not a bug in w/e is handling the IR?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74871



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

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.



Comment at: libcxx/include/new:229-237
+#if !defined(_LIBCPP_CXX03_LANG)
+using __libcpp_max_align_t = max_align_t;
+#else
+union __libcpp_max_align_t {
+  void * __f1;
+  long long int __f2;
+  long double __f3;

rsmith wrote:
> Is there any ODR risk from this, or similar? Does libc++ support building in 
> mixed C++98 / C++11 mode? If different TUs disagree on this alignment, we can 
> end up allocating with the aligned allocator and deallocating with the 
> unaligned allocator, which is not guaranteed to work.
> 
> We could always use the union approach if we don't know the default new 
> alignment. But from the code below it looks like we might only ever use this 
> if we have aligned allocation support, in which case we can just assume that 
> the default new alignment is defined. So perhaps we can just hide the entire 
> definition of `__is_overaligned_for_new` behind a `#ifdef 
> __STDCPP_DEFAULT_NEW_ALIGNMENT__` and never even consider `max_align_t`?
`max_align_t` should be ABI stable. I would rather we copy/paste the clang 
definition into the libc++ sources so we can use it when it's not provided by 
the compiler.

Though this raises another can of worms because GCC and Clang don't agree on a 
size for `max_align_t`.



Comment at: libcxx/include/new:232
+#else
+union __libcpp_max_align_t {
+  void * __f1;

If we're going to go this route, we should expose the `__libcpp_max_align_t` 
definition in all dialects, and compare it's size and alignment to the real 
`max_align_t` when we have it.


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

https://reviews.llvm.org/D73245



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

[PATCH] D74871: Fix interaction between -fdiscard-value-names and LLVM Bitcode

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

You mention a test case in the description but I don't see it?




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3607
+// Cannot discard value names when processing llvm-ir, because IR loading
+// is conservative wrt. names.
+if (Res.getCodeGenOpts().DiscardValueNames) {

What do you mean by this?



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3610
+  Res.getCodeGenOpts().DiscardValueNames = false;
+}
   } else {

nit: you don't need to test, you can always assign to false


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74871



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

[PATCH] D74564: libclang: Add static build support for Windows

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/tools/libclang/CMakeLists.txt:117
 if(ENABLE_SHARED)
+  target_compile_definitions(libclang PUBLIC CINDEX_EXPORTS)
   if(WIN32)

Is this enough? Every target that depends on libclang now needs to define 
CINDEX_EXPORTS – is that already the case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74564



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

[PATCH] D69868: Allow "callbr" to return non-void values

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

Would you be okay with me submitting this and working on making INLINEASM_BR a 
non-terminator? I'd like to give this feature some bake time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69868



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

[PATCH] D74813: [RFC] Add hash of block contents to function block names

[Alex Borcan via Phabricator via cfe-commits]

alexbdv added a comment.

@vsk - about breaking existing workflows - I was referring only to if / when 
this gets shipped out as the default - all the names for the function blocks 
will change and this might cause issue with tooling that relies on symbol names 
being consistent across builds.

@dexonsmith

- Just to make sure - this isn't dumping LLVM IR but a textual representation 
of the AST. Does this have the same issues with metadata / metadata numbering 
that LLVM IR has, or you were referring to the AST text dump, not llvm IR ? 
Also I don't think that clang would be a good test case here - it doesn't have 
many block functions.
- Stability wise - I'm still not sure if this has the metadata numbering issue 
(since it's AST text representation), perhaps you can tell me how to check.
- Correct, the hash might change if the block contents changes - this is kind 
of the intended use for this. As the flag mentions, it is hash-based.

Moving to a per-function index-based approach seems like the correct default 
approach. I would still like to have the hash version under a flag though. 
Moving to per-function index-based should be as simple as not adding the  
discriminator at all. Since function blocks contain the parent's function's 
name and given that llvm auto-renames duplicate symbols by adding an 
incremental number - the end result is that function blocks will be 
incrementally named based on the order that they have in the parent function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74813



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

[clang] 0edb212 - [MS] Mark vectorcall FP and vector args inreg

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2020-02-19T16:37:50-08:00
New Revision: 0edb2129258c727e9efe6fa234b28880ff64c1b9

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

LOG: [MS] Mark vectorcall FP and vector args inreg

This has no effect on how LLVM passes the arguments, but it prevents
rewriteWithInAlloca from thinking that these parameters should be part
of the inalloca pack.

Follow-up to D72114

Reviewed By: erichkeane

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

Added: 


Modified: 
clang/lib/CodeGen/TargetInfo.cpp
clang/test/CodeGen/vectorcall.c
clang/test/CodeGenCXX/inalloca-vector.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/TargetInfo.cpp 
b/clang/lib/CodeGen/TargetInfo.cpp
index e40f24d0ca4d..21d5cd08c9fa 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -1687,7 +1687,7 @@ void X86_32ABIInfo::runVectorCallFirstPass(CGFunctionInfo 
&FI, CCState &State) c
 isHomogeneousAggregate(Ty, Base, NumElts)) {
   if (State.FreeSSERegs >= NumElts) {
 State.FreeSSERegs -= NumElts;
-Args[I].info = ABIArgInfo::getDirect();
+Args[I].info = ABIArgInfo::getDirectInReg();
 State.IsPreassigned.set(I);
   }
 }

diff  --git a/clang/test/CodeGen/vectorcall.c b/clang/test/CodeGen/vectorcall.c
index aa529d087011..77db600a7f89 100644
--- a/clang/test/CodeGen/vectorcall.c
+++ b/clang/test/CodeGen/vectorcall.c
@@ -31,13 +31,13 @@ void __vectorcall hfa1(int a, struct HFA4 b, int c) {}
 // indirectly. Additional vector arguments can consume the rest of the SSE
 // registers.
 void __vectorcall hfa2(struct HFA4 a, struct HFA4 b, double c) {}
-// X32: define dso_local x86_vectorcallcc void @"\01hfa2@@72"(%struct.HFA4 
inreg %a.coerce, %struct.HFA4* inreg %b, double %c)
+// X32: define dso_local x86_vectorcallcc void @"\01hfa2@@72"(%struct.HFA4 
inreg %a.coerce, %struct.HFA4* inreg %b, double inreg %c)
 // X64: define dso_local x86_vectorcallcc void @"\01hfa2@@72"(%struct.HFA4 
inreg %a.coerce, %struct.HFA4* %b, double %c)
 
 // Ensure that we pass builtin types directly while counting them against the
 // SSE register usage.
 void __vectorcall hfa3(double a, double b, double c, double d, double e, 
struct HFA2 f) {}
-// X32: define dso_local x86_vectorcallcc void @"\01hfa3@@56"(double %a, 
double %b, double %c, double %d, double %e, %struct.HFA2* inreg %f)
+// X32: define dso_local x86_vectorcallcc void @"\01hfa3@@56"(double inreg %a, 
double inreg %b, double inreg %c, double inreg %d, double inreg %e, 
%struct.HFA2* inreg %f)
 // X64: define dso_local x86_vectorcallcc void @"\01hfa3@@56"(double %a, 
double %b, double %c, double %d, double %e, %struct.HFA2* %f)
 
 // Aggregates with more than four elements are not HFAs and are passed byval.
@@ -64,21 +64,21 @@ v4f32 __vectorcall hva1(int a, struct HVA4 b, int c) 
{return b.w;}
 // X64: define dso_local x86_vectorcallcc <4 x float> @"\01hva1@@80"(i32 %a, 
%struct.HVA4 inreg %b.coerce, i32 %c)
 
 v4f32 __vectorcall hva2(struct HVA4 a, struct HVA4 b, v4f32 c) {return c;}
-// X32: define dso_local x86_vectorcallcc <4 x float> 
@"\01hva2@@144"(%struct.HVA4 inreg %a.coerce, %struct.HVA4* inreg %b, <4 x 
float> %c)
+// X32: define dso_local x86_vectorcallcc <4 x float> 
@"\01hva2@@144"(%struct.HVA4 inreg %a.coerce, %struct.HVA4* inreg %b, <4 x 
float> inreg %c)
 // X64: define dso_local x86_vectorcallcc <4 x float> 
@"\01hva2@@144"(%struct.HVA4 inreg %a.coerce, %struct.HVA4* %b, <4 x float> %c)
 
 v4f32 __vectorcall hva3(v4f32 a, v4f32 b, v4f32 c, v4f32 d, v4f32 e, struct 
HVA2 f) {return f.x;}
-// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva3@@112"(<4 x 
float> %a, <4 x float> %b, <4 x float> %c, <4 x float> %d, <4 x float> %e, 
%struct.HVA2* inreg %f)
+// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva3@@112"(<4 x 
float> inreg %a, <4 x float> inreg %b, <4 x float> inreg %c, <4 x float> inreg 
%d, <4 x float> inreg %e, %struct.HVA2* inreg %f)
 // X64: define dso_local x86_vectorcallcc <4 x float> @"\01hva3@@112"(<4 x 
float> %a, <4 x float> %b, <4 x float> %c, <4 x float> %d, <4 x float> %e, 
%struct.HVA2* %f)
 
 // vector types have higher priority then HVA structures, So vector types are 
allocated first
 // and HVAs are allocated if enough registers are available
 v4f32 __vectorcall hva4(struct HVA4 a, struct HVA2 b, v4f32 c) {return b.y;}
-// X32: define dso_local x86_vectorcallcc <4 x float> 
@"\01hva4@@112"(%struct.HVA4 inreg %a.coerce, %struct.HVA2* inreg %b, <4 x 
float> %c)
+// X32: define dso_local x86_vectorcallcc <4 x float> 
@"\01hva4@@112"(%struct.HVA4 inreg %a.coerce, %struct.HVA2* inreg %b, <4 x 
float> inreg %c)
 // X64: define dso_local x86_vectorcallcc <4 x float> 
@"\01hva4@@112"(%struct.HVA4 inreg %a.c

[PATCH] D74795: Make diagnostic reporting more robust in presence of corrupt PCH data.

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.

Thanks for working on this.

> I'm not sure how (or if it's possible) to add a test that would demonstrate 
> the before/after here. Happy to take advice!

Yea, those are some times hard. Did you try to write a unittest that hits the 
assertion and trigger this case? Perhaps 
`clang/unittests/Frontend/PCHPreambleTest.cpp` would be a good example to start 
off.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74795



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

[clang-tools-extra] d1d5180 - [NFC] Fix issues with clang-tidy checks list.rst

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-02-19T23:19:09Z
New Revision: d1d5180e6904b3fa86f750a05a6721163e115f4d

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

LOG: [NFC] Fix issues with clang-tidy checks list.rst

Added FixItHint comments to ReservedIdentifierCheck and IdentifierNamingCheck 
to trick the python scripts into detecting a fix it is provided as it can't see 
the FixItHints in RenamerClangTidyCheck.cpp

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
index 55d8884484b5..d562d8c3d213 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
@@ -14,6 +14,8 @@
 #include 
 #include 
 
+// FixItHint
+
 using namespace clang::ast_matchers;
 
 namespace clang {

diff  --git 
a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
index 0171964b5264..45cf87bccfc3 100644
--- a/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -18,6 +18,8 @@
 
 #define DEBUG_TYPE "clang-tidy"
 
+// FixItHint
+
 using namespace clang::ast_matchers;
 
 namespace clang {

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst 
b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index ce4b20f8693f..3b9d12ec56ad 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -72,6 +72,7 @@ Clang-Tidy Checks
`bugprone-not-null-terminated-result 
`_, "Yes"
`bugprone-parent-virtual-call `_, "Yes"
`bugprone-posix-return `_, "Yes"
+   `bugprone-reserved-identifier `_, "Yes"
`bugprone-signed-char-misuse `_,
`bugprone-sizeof-container `_,
`bugprone-sizeof-expression `_,
@@ -189,7 +190,7 @@ Clang-Tidy Checks
`misc-definitions-in-headers `_, "Yes"
`misc-misplaced-const `_,
`misc-new-delete-overloads `_,
-   `misc-no-recursion `_,
+   `misc-no-recursion `_,
`misc-non-copyable-objects `_,
`misc-non-private-member-variables-in-classes 
`_,
`misc-redundant-expression `_, "Yes"
@@ -299,6 +300,8 @@ Clang-Tidy Checks
 
`cert-dcl03-c `_, `misc-static-assert 
`_, "Yes"
`cert-dcl16-c `_, `readability-uppercase-literal-suffix 
`_, "Yes"
+   `cert-dcl37-c `_, `bugprone-reserved-identifier 
`_, "Yes"
+   `cert-dcl51-cpp `_, `bugprone-reserved-identifier 
`_, "Yes"
`cert-dcl54-cpp `_, `misc-new-delete-overloads 
`_,
`cert-dcl59-cpp `_, `google-build-namespaces 
`_,
`cert-err09-cpp `_, 
`misc-throw-by-value-catch-by-reference 
`_,



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

[PATCH] D74800: [clang-tidy] fix readability-redundant-member-init auto-fix of Function-try-block

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG709fd989b644: [clang-tidy] fix 
readability-redundant-member-init auto-fix of Function-try… (authored by 
AlexanderLanin, committed by njames93).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74800

Files:
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/CleanupTest.cpp

Index: clang/unittests/Format/CleanupTest.cpp
===
--- clang/unittests/Format/CleanupTest.cpp
+++ clang/unittests/Format/CleanupTest.cpp
@@ -151,6 +151,31 @@
   EXPECT_EQ(Expected, cleanupAroundOffsets({15}, Code));
 }
 
+// regression test for bug 39310
+TEST_F(CleanupTest, CtorInitializationSimpleRedundantCommaInFunctionTryBlock) {
+  std::string Code = "class A {\nA() try : , {} };";
+  std::string Expected = "class A {\nA() try  {} };";
+  EXPECT_EQ(Expected, cleanupAroundOffsets({21, 23}, Code));
+
+  Code = "class A {\nA() try : x(1), {} };";
+  Expected = "class A {\nA() try : x(1) {} };";
+  EXPECT_EQ(Expected, cleanupAroundOffsets({27}, Code));
+
+  Code = "class A {\nA() try :{} };";
+  Expected = "class A {\nA() try {} };";
+  EXPECT_EQ(Expected, cleanupAroundOffsets({19}, Code));
+
+  Code = "class A {\nA() try : x(1),,, {} };";
+  Expected = "class A {\nA() try : x(1) {} };";
+  EXPECT_EQ(Expected, cleanupAroundOffsets({27}, Code));
+
+  // Do not remove every comma following a colon as it simply doesn't make
+  // sense in some situations.
+  Code = "try : , {}";
+  Expected = "try : , {}";
+  EXPECT_EQ(Expected, cleanupAroundOffsets({8}, Code));
+}
+
 TEST_F(CleanupTest, CtorInitializationSimpleRedundantColon) {
   std::string Code = "class A {\nA() : =default; };";
   std::string Expected = "class A {\nA()  =default; };";
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1849,11 +1849,20 @@
   if (FormatTok->is(tok::colon)) {
 // We are in a function try block, what comes is an initializer list.
 nextToken();
+
+// In case identifiers were removed by clang-tidy, what might follow is
+// multiple commas in sequence - before the first identifier.
+while (FormatTok->is(tok::comma))
+  nextToken();
+
 while (FormatTok->is(tok::identifier)) {
   nextToken();
   if (FormatTok->is(tok::l_paren))
 parseParens();
-  if (FormatTok->is(tok::comma))
+
+  // In case identifiers were removed by clang-tidy, what might follow is
+  // multiple commas in sequence - after the first identifier.
+  while (FormatTok->is(tok::comma))
 nextToken();
 }
   }
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -819,10 +819,15 @@
 Tok->Type = TT_BitFieldColon;
   } else if (Contexts.size() == 1 &&
  !Line.First->isOneOf(tok::kw_enum, tok::kw_case)) {
-if (Tok->getPreviousNonComment()->isOneOf(tok::r_paren,
-  tok::kw_noexcept))
+FormatToken *Prev = Tok->getPreviousNonComment();
+if (Prev->isOneOf(tok::r_paren, tok::kw_noexcept))
   Tok->Type = TT_CtorInitializerColon;
-else
+else if (Prev->is(tok::kw_try)) {
+  // Member initializer list within function try block.
+  FormatToken *PrevPrev = Prev->getPreviousNonComment();
+  if (PrevPrev && PrevPrev->isOneOf(tok::r_paren, tok::kw_noexcept))
+Tok->Type = TT_CtorInitializerColon;
+} else
   Tok->Type = TT_InheritanceColon;
   } else if (canBeObjCSelectorComponent(*Tok->Previous) && Tok->Next &&
  (Tok->Next->isOneOf(tok::r_paren, tok::comma) ||
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
@@ -418,3 +418,17 @@
   };
 
 MACRO();
+
+
+class FunctionTryBlock {
+public:
+  FunctionTryBlock() try : i(5), k(8) {}
+  // CHECK-FIXES: FunctionTryBlock() try  {}
+  catch (...) {}
+
+private:
+  int i, k;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer for 'i' [modernize-use-default-member-init]
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: use default member initializer for 'k' [modernize-use-defaul

[clang] 709fd98 - [clang-tidy] fix readability-redundant-member-init auto-fix of Function-try-block

[Nathan James via cfe-commits]

Author: Alexander Lanin
Date: 2020-02-19T23:04:05Z
New Revision: 709fd989b644a80527e0f4a22503d54255bc095c

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

LOG: [clang-tidy] fix readability-redundant-member-init auto-fix of 
Function-try-block

Summary: This fixes https://bugs.llvm.org/show_bug.cgi?id=39310

Reviewers: malcolm.parsons, ioeric

Reviewed By: malcolm.parsons

Subscribers: xazax.hun

Tags: #clang-format, #clang-tools-extra

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

Added: 


Modified: 

clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/CleanupTest.cpp

Removed: 




diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
index 196eec91b8e8..84dfeb215f4f 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/modernize-use-default-member-init.cpp
@@ -418,3 +418,17 @@ NegativeTemplateExisting nted(0);
   };
 
 MACRO();
+
+
+class FunctionTryBlock {
+public:
+  FunctionTryBlock() try : i(5), k(8) {}
+  // CHECK-FIXES: FunctionTryBlock() try  {}
+  catch (...) {}
+
+private:
+  int i, k;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use default member initializer 
for 'i' [modernize-use-default-member-init]
+  // CHECK-MESSAGES: :[[@LINE-2]]:10: warning: use default member initializer 
for 'k' [modernize-use-default-member-init]
+  // CHECK-FIXES: int i{5}, k{8};
+};

diff  --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
index 42963ca105a9..d71c4f470378 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -819,10 +819,15 @@ class AnnotatingParser {
 Tok->Type = TT_BitFieldColon;
   } else if (Contexts.size() == 1 &&
  !Line.First->isOneOf(tok::kw_enum, tok::kw_case)) {
-if (Tok->getPreviousNonComment()->isOneOf(tok::r_paren,
-  tok::kw_noexcept))
+FormatToken *Prev = Tok->getPreviousNonComment();
+if (Prev->isOneOf(tok::r_paren, tok::kw_noexcept))
   Tok->Type = TT_CtorInitializerColon;
-else
+else if (Prev->is(tok::kw_try)) {
+  // Member initializer list within function try block.
+  FormatToken *PrevPrev = Prev->getPreviousNonComment();
+  if (PrevPrev && PrevPrev->isOneOf(tok::r_paren, tok::kw_noexcept))
+Tok->Type = TT_CtorInitializerColon;
+} else
   Tok->Type = TT_InheritanceColon;
   } else if (canBeObjCSelectorComponent(*Tok->Previous) && Tok->Next &&
  (Tok->Next->isOneOf(tok::r_paren, tok::comma) ||

diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index b455a7f2ebed..802bb514a38f 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1849,11 +1849,20 @@ void UnwrappedLineParser::parseTryCatch() {
   if (FormatTok->is(tok::colon)) {
 // We are in a function try block, what comes is an initializer list.
 nextToken();
+
+// In case identifiers were removed by clang-tidy, what might follow is
+// multiple commas in sequence - before the first identifier.
+while (FormatTok->is(tok::comma))
+  nextToken();
+
 while (FormatTok->is(tok::identifier)) {
   nextToken();
   if (FormatTok->is(tok::l_paren))
 parseParens();
-  if (FormatTok->is(tok::comma))
+
+  // In case identifiers were removed by clang-tidy, what might follow is
+  // multiple commas in sequence - after the first identifier.
+  while (FormatTok->is(tok::comma))
 nextToken();
 }
   }

diff  --git a/clang/unittests/Format/CleanupTest.cpp 
b/clang/unittests/Format/CleanupTest.cpp
index b0c81b509d2a..70741d239c82 100644
--- a/clang/unittests/Format/CleanupTest.cpp
+++ b/clang/unittests/Format/CleanupTest.cpp
@@ -151,6 +151,31 @@ TEST_F(CleanupTest, 
CtorInitializationSimpleRedundantComma) {
   EXPECT_EQ(Expected, cleanupAroundOffsets({15}, Code));
 }
 
+// regression test for bug 39310
+TEST_F(CleanupTest, CtorInitializationSimpleRedundantCommaInFunctionTryBlock) {
+  std::string Code = "class A {\nA() try : , {} };";
+  std::string Expected = "class A {\nA() try  {} };";
+  EXPECT_EQ(Expected, cleanupAroundOffsets({21, 23}, Code));
+
+  Code = "class A {\nA() try : x(1), {} };";
+  Expected = "class A {\nA() try : x(1) {} };";
+  EXPECT_EQ(Expected, cleanupAroundOffsets({27}, Code));
+
+  Code = "class 

[PATCH] D69868: Allow "callbr" to return non-void values

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In D69868#1883687 , @jyknight wrote:

> Ugh, it's actually been that long, hasn't it...I'm really sorry about that. :(


No worries. Thanks for getting back to us!

> I've been actively spending time to look at this over the last couple weeks. 
> I haven't been able to convince myself that the weird-successors and having 
> allocatable registers across BBs here is not going to cause codegen issues 
> after optimization passes run on it. Unfortunately, despite spending time 
> looking into it, I also wasn't able to convince myself that this *IS* broken. 
> Maybe someone else can chime in here and either assuage or confirm my worries?

I've been concerned about the register live-ins too (I'm less concerned about 
the successors issue). Is there documentation on the original decision to 
disallow physical register live-ins for MBBs before register allocation? We 
could then check to see if we're violating the original reasoning.

> I also see some other minor issues, which I need to write up, but I'd been 
> blocking writing up a comment for that behind the larger question.
> 
> Anyways, while looking at this I started thinking it might actually be better 
> to actually have INLINEASM_BR (both with or without outputs) _not_ be a 
> terminator MachineInstr. (remaining a terminator in IR form, however). This 
> would then be similar to how "invoke" works -- at the MachineInstr level, the 
> call is not a terminator, even though it can jump out to the EH successors. 
> And, the return-value handling remains in the same basic block as the call. I 
> tried making that change, but doing it correctly has other impacts -- much of 
> the code which currently special-cases isEHPad() needs to be updated (Which 
> does mean I now feel like I have a good handle on the problems the _previous_ 
> version of this code, which didn't split the block, had.). 
> MachineBasicBlock::updateTerminator is a good example of the problematic 
> cases -- it trawls the successor list to look for the "correct" successor, 
> filtering out isEHPad successors. But unlike EH Pads, indirect targets of a 
> INLINEASM_BR are not used only for that purpose, so can't be so quickly 
> distinguished in the successors list.

I had a very similar idea. There are some people *cough*Chris*cough* who insist 
that `INLINEASM_BR` makes sense as a terminator. I don't deny that it's very 
tempting to make it one, but because of this instruction's behavior it doesn't 
*act* like a normal terminator (explicitly branching to some destination).

As for `isEHPad`, do you think it would make sense to have a generic 
`isIndirectTarget` predicate, which we could then use everywhere and it would 
automagically filter out blocks that aren't interesting?

> So, I started updating some of that code to not have that assumption. While 
> doing so, I noticed that fixing this would also allow analyzeBranch to ignore 
> the inlineasm branch instructions -- and remain correct -- which actually 
> would allow llvm to do better block placement of the inlineasm_br blocks, 
> because it enables it to move the normal successor jump/fallthrough. So, I 
> think that would actually be a good idea to make that change.

Now that I know I'm not crazy for wanting to make INLINEASM_BR a non-terminator 
(I'm assuming you're not crazy at least :-), I'll give it a go. Feel free to 
send me patches if you have them.

> But whether it'd be _necessary_ to make such a change (or other changes) for 
> this patch, or just a nice thing to fix, I'm still just unsure about.

Given your concerns over the patch in its current form, let's at least try the 
non-terminator path. If we can make it work, then your concerns will be 
assuaged (as would mine).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69868



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

[PATCH] D74807: Add cl_khr_mipmap_image_writes as supported to AMDGPU

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGed07c89fc50f: Add cl_khr_mipmap_image_writes as supported to 
AMDGPU (authored by yaxunl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74807

Files:
  clang/lib/Basic/Targets/AMDGPU.h


Index: clang/lib/Basic/Targets/AMDGPU.h
===
--- clang/lib/Basic/Targets/AMDGPU.h
+++ clang/lib/Basic/Targets/AMDGPU.h
@@ -263,6 +263,7 @@
   Opts.support("cl_khr_int64_base_atomics");
   Opts.support("cl_khr_int64_extended_atomics");
   Opts.support("cl_khr_mipmap_image");
+  Opts.support("cl_khr_mipmap_image_writes");
   Opts.support("cl_khr_subgroups");
   Opts.support("cl_khr_3d_image_writes");
   Opts.support("cl_amd_media_ops");


Index: clang/lib/Basic/Targets/AMDGPU.h
===
--- clang/lib/Basic/Targets/AMDGPU.h
+++ clang/lib/Basic/Targets/AMDGPU.h
@@ -263,6 +263,7 @@
   Opts.support("cl_khr_int64_base_atomics");
   Opts.support("cl_khr_int64_extended_atomics");
   Opts.support("cl_khr_mipmap_image");
+  Opts.support("cl_khr_mipmap_image_writes");
   Opts.support("cl_khr_subgroups");
   Opts.support("cl_khr_3d_image_writes");
   Opts.support("cl_amd_media_ops");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

It is used both in `` and `` and the use in the latter is 
currently unconditional AFAICT. I don't have a problem splitting the 
conditional to avoid the typedef. That would address the ODR concern?


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

https://reviews.llvm.org/D73245



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

[clang] ed07c89 - Add cl_khr_mipmap_image_writes as supported to AMDGPU

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2020-02-19T17:40:40-05:00
New Revision: ed07c89fc50f38c4f1403b19897468edd7e5cbf3

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

LOG: Add cl_khr_mipmap_image_writes as supported to AMDGPU

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

Added: 


Modified: 
clang/lib/Basic/Targets/AMDGPU.h

Removed: 




diff  --git a/clang/lib/Basic/Targets/AMDGPU.h 
b/clang/lib/Basic/Targets/AMDGPU.h
index 456cb2ebb8b5..ed6929214f6e 100644
--- a/clang/lib/Basic/Targets/AMDGPU.h
+++ b/clang/lib/Basic/Targets/AMDGPU.h
@@ -263,6 +263,7 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : 
public TargetInfo {
   Opts.support("cl_khr_int64_base_atomics");
   Opts.support("cl_khr_int64_extended_atomics");
   Opts.support("cl_khr_mipmap_image");
+  Opts.support("cl_khr_mipmap_image_writes");
   Opts.support("cl_khr_subgroups");
   Opts.support("cl_khr_3d_image_writes");
   Opts.support("cl_amd_media_ops");



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

[PATCH] D69868: Allow "callbr" to return non-void values

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Ugh, it's actually been that long, hasn't it...I'm really sorry about that. :(

I've been actively spending time to look at this over the last couple weeks. I 
haven't been able to convince myself that the weird-successors and having 
allocatable registers across BBs here is not going to cause codegen issues 
after optimization passes run on it. Unfortunately, despite spending time 
looking into it, I also wasn't able to convince myself that this *IS* broken. 
Maybe someone else can chime in here and either assuage or confirm my worries?

I also see some other minor issues, which I need to write up, but I'd been 
blocking writing up a comment for that behind the larger question.

Anyways, while looking at this I started thinking it might actually be better 
to actually have INLINEASM_BR (both with or without outputs) _not_ be a 
terminator MachineInstr. (remaining a terminator in IR form, however). This 
would then be similar to how "invoke" works -- at the MachineInstr level, the 
call is not a terminator, even though it can jump out to the EH successors. 
And, the return-value handling remains in the same basic block as the call. I 
tried making that change, but doing it correctly has other impacts -- much of 
the code which currently special-cases isEHPad() needs to be updated (Which 
does mean I now feel like I have a good handle on the problems the _previous_ 
version of this code, which didn't split the block, had.). 
MachineBasicBlock::updateTerminator is a good example of the problematic cases 
-- it trawls the successor list to look for the "correct" successor, filtering 
out isEHPad successors. But unlike EH Pads, indirect targets of a INLINEASM_BR 
are not used only for that purpose, so can't be so quickly distinguished in the 
successors list.

So, I started updating some of that code to not have that assumption. While 
doing so, I noticed that fixing this would also allow analyzeBranch to ignore 
the inlineasm branch instructions -- and remain correct -- which actually would 
allow llvm to do better block placement of the inlineasm_br blocks, because it 
enables it to move the normal successor jump/fallthrough. So, I think that 
would actually be a good idea to make that change.

But whether it'd be _necessary_ to make such a change (or other changes) for 
this patch, or just a nice thing to fix, I'm still just unsure about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69868



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

[PATCH] D74811: [Driver] Escape the program path for -frecord-command-line

[Scott Linder via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74811



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

[clang] 4960eb4 - Another fix for 7d91633a2b9b1f563dc14c632cc0c461c3651f76

[David Goldman via cfe-commits]

Author: David Goldman
Date: 2020-02-19T17:15:11-05:00
New Revision: 4960eb4a1bdd6f469d5ad6d5049609cf97a982f3

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

LOG: Another fix for 7d91633a2b9b1f563dc14c632cc0c461c3651f76

Forgot to update lines for RUNs

Added: 


Modified: 
clang/test/CodeCompletion/included-symlinks.cpp

Removed: 




diff  --git a/clang/test/CodeCompletion/included-symlinks.cpp 
b/clang/test/CodeCompletion/included-symlinks.cpp
index daa0c02c91a6..bccaaa616214 100644
--- a/clang/test/CodeCompletion/included-symlinks.cpp
+++ b/clang/test/CodeCompletion/included-symlinks.cpp
@@ -6,11 +6,11 @@
 
 // Suggest symlinked header files.
 #include "foo.h"
-// RUN: %clang -fsyntax-only -I%t/links -Xclang -code-completion-at=%s:7:13 %s 
| FileCheck -check-prefix=CHECK-1 %s
+// RUN: %clang -fsyntax-only -I%t/links -Xclang -code-completion-at=%s:8:13 %s 
| FileCheck -check-prefix=CHECK-1 %s
 // CHECK-1: foo.h"
 // CHECK-1: foobar.h"
 
 // Suggest symlinked folder.
 #include "mypr"
-// RUN: %clang -fsyntax-only -I%t/links -Xclang -code-completion-at=%s:13:13 
%s | FileCheck -check-prefix=CHECK-2 %s
+// RUN: %clang -fsyntax-only -I%t/links -Xclang -code-completion-at=%s:14:13 
%s | FileCheck -check-prefix=CHECK-2 %s
 // CHECK-2: myproj/



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

[PATCH] D74564: libclang: Add static build support for Windows

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7ff1f55a1219: libclang: Add static build support for Windows 
(authored by cristian.adam, committed by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74564

Files:
  clang/include/clang-c/Platform.h
  clang/tools/libclang/CMakeLists.txt


Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -77,11 +77,11 @@
   set(LLVM_EXPORTED_SYMBOL_FILE)
 endif()
 
-if(LLVM_ENABLE_PIC OR WIN32)
+if(LLVM_ENABLE_PIC)
   set(ENABLE_SHARED SHARED)
 endif()
 
-if((NOT LLVM_ENABLE_PIC OR LIBCLANG_BUILD_STATIC) AND NOT WIN32)
+if(NOT LLVM_ENABLE_PIC OR LIBCLANG_BUILD_STATIC)
   set(ENABLE_STATIC STATIC)
 endif()
 
@@ -114,6 +114,7 @@
   )
 
 if(ENABLE_SHARED)
+  target_compile_definitions(libclang PUBLIC CINDEX_EXPORTS)
   if(WIN32)
 set_target_properties(libclang
   PROPERTIES
Index: clang/include/clang-c/Platform.h
===
--- clang/include/clang-c/Platform.h
+++ clang/include/clang-c/Platform.h
@@ -18,14 +18,20 @@
 
 LLVM_CLANG_C_EXTERN_C_BEGIN
 
-/* MSVC DLL import/export. */
-#ifdef _MSC_VER
-  #ifdef _CINDEX_LIB_
-#define CINDEX_LINKAGE __declspec(dllexport)
-  #else
-#define CINDEX_LINKAGE __declspec(dllimport)
+/* Windows DLL import/export. */
+#ifdef _WIN32
+  #ifdef CINDEX_EXPORTS
+#ifdef _CINDEX_LIB_
+  #define CINDEX_LINKAGE __declspec(dllexport)
+#else
+  #define CINDEX_LINKAGE __declspec(dllimport)
+#endif
   #endif
-#else
+#elif defined(CINDEX_EXPORTS)
+  #define CINDEX_LINKAGE __attribute__((visibility("default")))
+#endif
+
+#ifndef CINDEX_LINKAGE
   #define CINDEX_LINKAGE
 #endif
 


Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -77,11 +77,11 @@
   set(LLVM_EXPORTED_SYMBOL_FILE)
 endif()
 
-if(LLVM_ENABLE_PIC OR WIN32)
+if(LLVM_ENABLE_PIC)
   set(ENABLE_SHARED SHARED)
 endif()
 
-if((NOT LLVM_ENABLE_PIC OR LIBCLANG_BUILD_STATIC) AND NOT WIN32)
+if(NOT LLVM_ENABLE_PIC OR LIBCLANG_BUILD_STATIC)
   set(ENABLE_STATIC STATIC)
 endif()
 
@@ -114,6 +114,7 @@
   )
 
 if(ENABLE_SHARED)
+  target_compile_definitions(libclang PUBLIC CINDEX_EXPORTS)
   if(WIN32)
 set_target_properties(libclang
   PROPERTIES
Index: clang/include/clang-c/Platform.h
===
--- clang/include/clang-c/Platform.h
+++ clang/include/clang-c/Platform.h
@@ -18,14 +18,20 @@
 
 LLVM_CLANG_C_EXTERN_C_BEGIN
 
-/* MSVC DLL import/export. */
-#ifdef _MSC_VER
-  #ifdef _CINDEX_LIB_
-#define CINDEX_LINKAGE __declspec(dllexport)
-  #else
-#define CINDEX_LINKAGE __declspec(dllimport)
+/* Windows DLL import/export. */
+#ifdef _WIN32
+  #ifdef CINDEX_EXPORTS
+#ifdef _CINDEX_LIB_
+  #define CINDEX_LINKAGE __declspec(dllexport)
+#else
+  #define CINDEX_LINKAGE __declspec(dllimport)
+#endif
   #endif
-#else
+#elif defined(CINDEX_EXPORTS)
+  #define CINDEX_LINKAGE __attribute__((visibility("default")))
+#endif
+
+#ifndef CINDEX_LINKAGE
   #define CINDEX_LINKAGE
 #endif
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked an inline comment as done.
yaxunl added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:1514
+  void visitUsedDecl(SourceLocation Loc, Decl *D) {
+if (auto *TD = dyn_cast(D)) {
+  for (auto *DD : TD->decls()) {

rjmccall wrote:
> erichkeane wrote:
> > rjmccall wrote:
> > > erichkeane wrote:
> > > > Note that when recommitting this (if you choose to), this needs to also 
> > > > handle NamespaceDecl.  We're a downstream and discovered that this 
> > > > doesn't properly handle functions or records handled in a namespace.
> > > > 
> > > > It can be implemented identically to TranslationUnitDecl.
> > > Wait, what?  We shouldn't be doing this for TranslationUnitDecl either.   
> > > I don't even know how we're "using" a TranslationUnitDecl, but neither 
> > > this case not the case for `NamespaceDecl` should be recursively using 
> > > every declaration declared inside it.  If there's a declaration in a 
> > > namespace that's being used, it should be getting visited as part of the 
> > > actual use of it.
> > > 
> > > The logic for `RecordDecl` has the same problem.  
> > Despite the name, this seems to be more of a home-written ast walking 
> > class.  The entry point is the 'translation unit' which seems to walk 
> > through everything in an attempt to find all the functions (including those 
> > that are 'marked' as used by an attribute).
> > 
> > You'll see the FunctionDecl section makes this assumption as well (not 
> > necessarily that we got to a function via a call). IMO, this approach is 
> > strange, and we should register entry points in some manner (functions 
> > marked as emitted to the device in some fashion), then just follow its 
> > call-graph (via the clang::CallGraph?) to emit all of these functions.
> > 
> > It seemed really odd to see this approach here, but it seemed well reviewed 
> > by the time I noticed it (via a downstream bug) so I figured I'd lost my 
> > chance to disagree with the approach.
> > 
> > 
> Sure, but `visitUsedDecl` isn't the right place to be entering the walk.  
> `visitUsedDecl` is supposed to be the *callback* from the walk.  If they need 
> to walk all the global declarations to find kernels instead of tracking the 
> kernels as they're encountered (which would be a *much* better approach), it 
> should be done as a separate function.
> 
> I just missed this in the review.
The deferred diagnostics could be initiated by non-kernel functions or even 
host functions.

Let's consider a device code library where no kernels are defined. A device 
function is emitted, which calls a host device function which has a deferred 
diagnostic. All device functions that are emitted need to be checked.

Same with host functions that are emitted, which may call a host device 
function which has deferred diagnostic.

Also not just function calls need to be checked. A function address may be 
taken then called through function pointer. Therefore any reference to a 
function needs to be followed.

In the case of OpenMP, the initialization of a global function pointer which 
refers a function may trigger a deferred diangostic. There are tests for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70172



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

[clang] 7ff1f55 - libclang: Add static build support for Windows

[Martin Storsjö via cfe-commits]

Author: Cristian Adam
Date: 2020-02-20T00:05:46+02:00
New Revision: 7ff1f55a1219719f57a6f7905c26ce41d1767e4c

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

LOG: libclang: Add static build support for Windows

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

Added: 


Modified: 
clang/include/clang-c/Platform.h
clang/tools/libclang/CMakeLists.txt

Removed: 




diff  --git a/clang/include/clang-c/Platform.h 
b/clang/include/clang-c/Platform.h
index 3bb66bb0df48..9c15516f362f 100644
--- a/clang/include/clang-c/Platform.h
+++ b/clang/include/clang-c/Platform.h
@@ -18,14 +18,20 @@
 
 LLVM_CLANG_C_EXTERN_C_BEGIN
 
-/* MSVC DLL import/export. */
-#ifdef _MSC_VER
-  #ifdef _CINDEX_LIB_
-#define CINDEX_LINKAGE __declspec(dllexport)
-  #else
-#define CINDEX_LINKAGE __declspec(dllimport)
+/* Windows DLL import/export. */
+#ifdef _WIN32
+  #ifdef CINDEX_EXPORTS
+#ifdef _CINDEX_LIB_
+  #define CINDEX_LINKAGE __declspec(dllexport)
+#else
+  #define CINDEX_LINKAGE __declspec(dllimport)
+#endif
   #endif
-#else
+#elif defined(CINDEX_EXPORTS)
+  #define CINDEX_LINKAGE __attribute__((visibility("default")))
+#endif
+
+#ifndef CINDEX_LINKAGE
   #define CINDEX_LINKAGE
 #endif
 

diff  --git a/clang/tools/libclang/CMakeLists.txt 
b/clang/tools/libclang/CMakeLists.txt
index bd0c945a5e12..99de3b399855 100644
--- a/clang/tools/libclang/CMakeLists.txt
+++ b/clang/tools/libclang/CMakeLists.txt
@@ -77,11 +77,11 @@ if(MSVC)
   set(LLVM_EXPORTED_SYMBOL_FILE)
 endif()
 
-if(LLVM_ENABLE_PIC OR WIN32)
+if(LLVM_ENABLE_PIC)
   set(ENABLE_SHARED SHARED)
 endif()
 
-if((NOT LLVM_ENABLE_PIC OR LIBCLANG_BUILD_STATIC) AND NOT WIN32)
+if(NOT LLVM_ENABLE_PIC OR LIBCLANG_BUILD_STATIC)
   set(ENABLE_STATIC STATIC)
 endif()
 
@@ -114,6 +114,7 @@ add_clang_library(libclang ${ENABLE_SHARED} 
${ENABLE_STATIC} INSTALL_WITH_TOOLCH
   )
 
 if(ENABLE_SHARED)
+  target_compile_definitions(libclang PUBLIC CINDEX_EXPORTS)
   if(WIN32)
 set_target_properties(libclang
   PROPERTIES



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

[PATCH] D74790: [Sema][CodeComplete] Handle symlinks for include code completion

[David Goldman via Phabricator via cfe-commits]

dgoldman added a comment.

Submitted 
https://github.com/llvm/llvm-project/commit/7d91633a2b9b1f563dc14c632cc0c461c3651f76


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74790



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

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

On the assumption that we will never get a `::max_align_t` in C++98 mode anyway 
(which will be the case if the `` is conforming), this looks like the 
best we can do to me.




Comment at: libcxx/include/new:229-237
+#if !defined(_LIBCPP_CXX03_LANG)
+using __libcpp_max_align_t = max_align_t;
+#else
+union __libcpp_max_align_t {
+  void * __f1;
+  long long int __f2;
+  long double __f3;

Is there any ODR risk from this, or similar? Does libc++ support building in 
mixed C++98 / C++11 mode? If different TUs disagree on this alignment, we can 
end up allocating with the aligned allocator and deallocating with the 
unaligned allocator, which is not guaranteed to work.

We could always use the union approach if we don't know the default new 
alignment. But from the code below it looks like we might only ever use this if 
we have aligned allocation support, in which case we can just assume that the 
default new alignment is defined. So perhaps we can just hide the entire 
definition of `__is_overaligned_for_new` behind a `#ifdef 
__STDCPP_DEFAULT_NEW_ALIGNMENT__` and never even consider `max_align_t`?


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

https://reviews.llvm.org/D73245



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

[PATCH] D74813: [RFC] Add hash of block contents to function block names

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

dexonsmith added a comment.

Thanks for working on this, I agree it's a really important problem.

I'm as optimistic as Vedant that this is the right approach though.

- On compile time, I do think there's reason to be concerned, since dumping IR 
was fairly expensive last I checked due to numbering metadata.  Have you tried 
measuring a the time for a bootstrap of Clang?
- On stability, I'm skeptical this will be an improvement in the general case.  
Metadata numbering is unstable, and having that as an input isn't great.  It 
could even change between compiler versions.
- I'm also worried that the hash will change any time someone touches the body 
of the block.  That could even cause a change if other code changes a `typedef` 
which is used inside the block.

Is the current numbering per-module?  Can we change that somehow to 
per-function?  Then blocks would be numbered by order within a function, which 
seems fairly stable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74813



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

[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D70172#1883567 , @ABataev wrote:

> Seems to me, it causes some other issues. See 
> https://bugs.llvm.org/show_bug.cgi?id=44948 for example


I will fix that bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70172



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

[clang] 7d91633 - Fix broken test on Windows caused by D74790

[David Goldman via cfe-commits]

Author: David Goldman
Date: 2020-02-19T16:58:22-05:00
New Revision: 7d91633a2b9b1f563dc14c632cc0c461c3651f76

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

LOG: Fix broken test on Windows caused by D74790

Added: 


Modified: 
clang/test/CodeCompletion/included-symlinks.cpp

Removed: 




diff  --git a/clang/test/CodeCompletion/included-symlinks.cpp 
b/clang/test/CodeCompletion/included-symlinks.cpp
index a57a96b78474..daa0c02c91a6 100644
--- a/clang/test/CodeCompletion/included-symlinks.cpp
+++ b/clang/test/CodeCompletion/included-symlinks.cpp
@@ -1,3 +1,4 @@
+// REQUIRES: shell
 // RUN: rm -rf %t && mkdir -p %t/real/myproj && mkdir -p %t/links
 // RUN: touch %t/real/foo.h && ln -s %t/real/foo.h %t/links/foo.h
 // RUN: touch %t/real/foobar.h && ln -s %t/real/foobar.h %t/links/foobar.h



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

[PATCH] D74871: Fix interaction between -fdiscard-value-names and LLVM Bitcode

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
serge-sans-paille added reviewers: EricWF, mehdi_amini.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Without that patch, the test case associated to this patch segfaults in Release 
mode. This also makes the llvm-test-suite build fails, including in 10.0.0 rc2.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74871

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3603,6 +3603,11 @@
 LangOpts.PIE = Args.hasArg(OPT_pic_is_pie);
 parseSanitizerKinds("-fsanitize=", Args.getAllArgValues(OPT_fsanitize_EQ),
 Diags, LangOpts.Sanitize);
+// Cannot discard value names when processing llvm-ir, because IR loading
+// is conservative wrt. names.
+if (Res.getCodeGenOpts().DiscardValueNames) {
+  Res.getCodeGenOpts().DiscardValueNames = false;
+}
   } else {
 // Other LangOpts are only initialized when the input is not AST or LLVM 
IR.
 // FIXME: Should we really be calling this for an Language::Asm input?
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4332,8 +4332,15 @@
 
   // Discard value names in assert builds unless otherwise specified.
   if (Args.hasFlag(options::OPT_fdiscard_value_names,
-   options::OPT_fno_discard_value_names, !IsAssertBuild))
+   options::OPT_fno_discard_value_names, !IsAssertBuild)) {
+if (std::any_of(Inputs.begin(), Inputs.end(),
+[](const clang::driver::InputInfo &II) {
+  return types::isLLVMIR(II.getType());
+})) {
+  D.Diag(diag::warn_ignoring_fdiscard_for_bitcode);
+}
 CmdArgs.push_back("-discard-value-names");
+  }
 
   // Set the main file name, so that debug info works even with
   // -save-temps.
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -271,6 +271,9 @@
   InGroup;
 def warn_c_kext : Warning<
   "ignoring -fapple-kext which is valid for C++ and Objective-C++ only">;
+def warn_ignoring_fdiscard_for_bitcode : Warning<
+  "ignoring -fdiscard-value-names for LLVM Bitcode">,
+  InGroup;
 def warn_drv_input_file_unused : Warning<
   "%0: '%1' input unused%select{ when '%3' is present|}2">,
   InGroup;


Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3603,6 +3603,11 @@
 LangOpts.PIE = Args.hasArg(OPT_pic_is_pie);
 parseSanitizerKinds("-fsanitize=", Args.getAllArgValues(OPT_fsanitize_EQ),
 Diags, LangOpts.Sanitize);
+// Cannot discard value names when processing llvm-ir, because IR loading
+// is conservative wrt. names.
+if (Res.getCodeGenOpts().DiscardValueNames) {
+  Res.getCodeGenOpts().DiscardValueNames = false;
+}
   } else {
 // Other LangOpts are only initialized when the input is not AST or LLVM IR.
 // FIXME: Should we really be calling this for an Language::Asm input?
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4332,8 +4332,15 @@
 
   // Discard value names in assert builds unless otherwise specified.
   if (Args.hasFlag(options::OPT_fdiscard_value_names,
-   options::OPT_fno_discard_value_names, !IsAssertBuild))
+   options::OPT_fno_discard_value_names, !IsAssertBuild)) {
+if (std::any_of(Inputs.begin(), Inputs.end(),
+[](const clang::driver::InputInfo &II) {
+  return types::isLLVMIR(II.getType());
+})) {
+  D.Diag(diag::warn_ignoring_fdiscard_for_bitcode);
+}
 CmdArgs.push_back("-discard-value-names");
+  }
 
   // Set the main file name, so that debug info works even with
   // -save-temps.
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -271,6 +271,9 @@
   InGroup;
 def warn_c_kext : Warning<
   "ignoring -fapple-kext which is valid for C++ and Objective-C++ only">;
+def warn_ignoring_fdiscard_for_bitcode 

[PATCH] D74813: [RFC] Add hash of block contents to function block names

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

In D74813#1883241 , @alexbdv wrote:

> As for making it default - would rather have this under a flag as hashing the 
> block contents does have some overhead and I imagine this feature wouldn't be 
> beneficial in most scenarios. Also, unexpectedly (by default) changing the 
> name of the function blocks might have a negative impact on some existing 
> workflows.


I agree there is a non-zero compile-time overhead, but I expect it to be very 
small. Istm the relative cost of adding a flag would be much higher, in terms 
of added complexity and adoption time.

What kind of workflow would this change break? Part of the motivation seems to 
be that workflows that rely on blocks having stable names are brittle. This 
seems like it makes things strictly less brittle.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74813



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

[PATCH] D74790: [Sema][CodeComplete] Handle symlinks for include code completion

[David Goldman via Phabricator via cfe-commits]

dgoldman added a comment.

In D74790#1883466 , @dyung wrote:

> `The test you added in this change seems to be failing on Windows, can you 
> take a look?
>
> http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/14409
>
>   FAIL: Clang :: CodeCompletion/included-symlinks.cpp (1779 of 16867)
>    TEST 'Clang :: CodeCompletion/included-symlinks.cpp' 
> FAILED 
>   Script:
>   --
>   : 'RUN: at line 1';   rm -rf 
> C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp
>  && mkdir -p 
> C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj
>  && mkdir -p 
> C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links
>   : 'RUN: at line 2';   touch 
> C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foo.h
>  && ln -s 
> C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foo.h
>  
> C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/foo.h
>   : 'RUN: at line 3';   touch 
> C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foobar.h
>  && ln -s 
> C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foobar.h
>  
> C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/foobar.h
>   : 'RUN: at line 4';   touch 
> C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj/test.h
>  && ln -s 
> C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj
>  
> C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/myproj
>   : 'RUN: at line 8';   
> c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\clang.exe -fsyntax-only 
> -IC:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links
>  -Xclang 
> -code-completion-at=C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp:7:13
>  
> C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp
>  | c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\filecheck.exe 
> -check-prefix=CHECK-1 
> C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp
>   : 'RUN: at line 14';   
> c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\clang.exe -fsyntax-only 
> -IC:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links
>  -Xclang 
> -code-completion-at=C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp:13:13
>  
> C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp
>  | c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\filecheck.exe 
> -check-prefix=CHECK-2 
> C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp
>   --
>   Exit Code: 1
>  
>   Command Output (stdout):
>   --
>   $ ":" "RUN: at line 1"
>   $ "rm" "-rf" 
> "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp"
>   $ "mkdir" "-p" 
> "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj"
>   $ "mkdir" "-p" 
> "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links"
>   $ ":" "RUN: at line 2"
>   $ "touch" 
> "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foo.h"
>   $ "ln" "-s" 
> "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foo.h"
>  
> "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/foo.h"
>   $ ":" "RUN: at line 3"
>   $ "touch" 
> "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foobar.h"
>   $ "ln" "-s" 
> "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foobar.h"
>  
> "C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/foobar.h"
>   $ ":" "RUN: at line 4"
>   $ "touch" 
> "C:\b\slave\clang-x64-windows-msvc

[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Seems to me, it causes some other issues. See 
https://bugs.llvm.org/show_bug.cgi?id=44948 for example


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70172



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

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

[Adrian Prantl via Phabricator via cfe-commits]

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

LGTM with outstanding inline comments addressed.


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

https://reviews.llvm.org/D73462



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

[PATCH] D73462: [dwarf-5] Support DebugInfo for Defaulted parameters for C++ templates

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:2187
+  DEFINE_MDNODE_GET(DITemplateTypeParameter,
+(MDString * Name, Metadata *Type, bool IsDefault),
+(Name, Type, IsDefault))

clang-format, please.
 `(MDString *Name`



Comment at: llvm/lib/Bitcode/Reader/MetadataLoader.cpp:1685
+  (Context, getMDString(Record[1]),
+   getDITypeRefOrNull(Record[2]), false)),
+  NextMetadataNo);

```
MetadataList.assignValue(
  GET_OR_DISTINCT(DITemplateTypeParameter,
  (Context, getMDString(Record[1]),
   getDITypeRefOrNull(Record[2]), false)),
 Record.size() == 4 ? getMDOrNull(Record[3]) : false),
   NextMetadataNo);
```



Comment at: llvm/lib/Bitcode/Reader/MetadataLoader.cpp:1708
+   getDITypeRefOrNull(Record[3]), false,
+   getMDOrNull(Record[4]))),
+  NextMetadataNo);

Same here.



Comment at: llvm/test/Bitcode/DITemplateParameter-5.0.ll:50
+; CHECK: !DITemplateTypeParameter({{.*}}
+!14 = !DITemplateTypeParameter(name: "T", type: !10)
+

I think you can reduce the size of the binary bitcode file significantly by 
just having a raw TemplateParameter in the file, like so:

```
!named = !{!0, !1}
!0 = !DITemplateTypeParameter(name: "T", type: !10)
!1 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
```



Comment at: llvm/unittests/IR/MetadataTest.cpp:2078
   StringRef Name = "template";
   DIType *Type = getBasicType("basic");
 

For symmetry:
`bool defaulted = false;`



Comment at: llvm/unittests/IR/MetadataTest.cpp:2080
 
-  auto *N = DITemplateTypeParameter::get(Context, Name, Type);
+  auto *N = DITemplateTypeParameter::get(Context, Name, Type, false);
 

`get(Context, Name, Type, defaulted);`



Comment at: llvm/unittests/IR/MetadataTest.cpp:2085
   EXPECT_EQ(Type, N->getType());
-  EXPECT_EQ(N, DITemplateTypeParameter::get(Context, Name, Type));
+  EXPECT_EQ(N, DITemplateTypeParameter::get(Context, Name, Type, false));
 

same here ...



Comment at: llvm/unittests/IR/MetadataTest.cpp:2090
+getBasicType("other"), false));
+  EXPECT_NE(N, DITemplateTypeParameter::get(Context, Name, Type, true));
 

This can stay. 


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

https://reviews.llvm.org/D73462



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

[PATCH] D74860: [Sema] Fix pointer-to-int-cast diagnostic for _Bool

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

Also, make sure to run `git-clang-format HEAD~` on the patch, as the linter 
suggests. Thanks for the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74860



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

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Ping2?


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

https://reviews.llvm.org/D73245



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

[PATCH] D74790: [Sema][CodeComplete] Handle symlinks for include code completion

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

The test you added in this change seems to be failing on Windows, can you take 
a look?

http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/14409

FAIL: Clang :: CodeCompletion/included-symlinks.cpp (1779 of 16867)

- TEST 'Clang :: CodeCompletion/included-symlinks.cpp' FAILED 


Script:
---

: 'RUN: at line 1';   rm -rf 
C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp
 && mkdir -p 
C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj
 && mkdir -p 
C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links
: 'RUN: at line 2';   touch 
C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foo.h
 && ln -s 
C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foo.h
 
C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/foo.h
: 'RUN: at line 3';   touch 
C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foobar.h
 && ln -s 
C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foobar.h
 
C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/foobar.h
: 'RUN: at line 4';   touch 
C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj/test.h
 && ln -s 
C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj
 
C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/myproj
: 'RUN: at line 8';   
c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\clang.exe -fsyntax-only 
-IC:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links
 -Xclang 
-code-completion-at=C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp:7:13
 
C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp
 | c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\filecheck.exe 
-check-prefix=CHECK-1 
C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp

: 'RUN: at line 14';   
c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\clang.exe -fsyntax-only 
-IC:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links
 -Xclang 
-code-completion-at=C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp:13:13
 
C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp
 | c:\b\slave\clang-x64-windows-msvc\build\stage1\bin\filecheck.exe 
-check-prefix=CHECK-2 
C:\b\slave\clang-x64-windows-msvc\llvm-project\clang\test\CodeCompletion\included-symlinks.cpp
-

Exit Code: 1

Command Output (stdout):


$ ":" "RUN: at line 1"
$ "rm" "-rf" 
"C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp"
$ "mkdir" "-p" 
"C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/myproj"
$ "mkdir" "-p" 
"C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links"
$ ":" "RUN: at line 2"
$ "touch" 
"C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foo.h"
$ "ln" "-s" 
"C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foo.h"
 
"C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/links/foo.h"
$ ":" "RUN: at line 3"
$ "touch" 
"C:\b\slave\clang-x64-windows-msvc\build\stage1\tools\clang\test\CodeCompletion\Output\included-symlinks.cpp.tmp/real/foobar.h"

[PATCH] D74860: [Sema] Fix pointer-to-int-cast diagnostic for _Bool

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Can we test the same thing in C++?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74860



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

[PATCH] D74860: [Sema] Fix pointer-to-int-cast diagnostic for _Bool

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: dblaikie, rjmccall, rsmith, nathanchance, 
nickdesaulniers.
Mordante added a project: clang.

The diagnostic added in D72231  also shows a 
diagnostic when casting to a _Bool. This is unwanted. This patch removes the 
diagnostic for _Bool.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74860

Files:
  clang/lib/Sema/SemaCast.cpp
  clang/test/Sema/MicrosoftExtensions.c
  clang/test/Sema/cast.c


Index: clang/test/Sema/cast.c
===
--- clang/test/Sema/cast.c
+++ clang/test/Sema/cast.c
@@ -151,7 +151,7 @@
 }
 
 void testVoidPtr(VoidPtr v) {
-  (void) (Bool) v; // expected-warning{{cast to smaller integer type 'Bool' 
(aka '_Bool') from 'VoidPtr' (aka 'void *')}}
+  (void) (Bool) v;
   (void) (Int) v; // expected-warning{{cast to smaller integer type 'Int' (aka 
'int') from 'VoidPtr' (aka 'void *')}}
   (void) (Long) v;
   (void) (VoidPtr) v;
@@ -160,12 +160,12 @@
   // from other -Wpointer-to-int-cast warnings.
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wvoid-pointer-to-int-cast"
-  (void) (Bool) v; // no-warning
+  (void) (Int) v; // no-warning
 #pragma clang diagnostic pop
 }
 
 void testCharPtr(CharPtr v) {
-  (void) (Bool) v; // expected-warning{{cast to smaller integer type 'Bool' 
(aka '_Bool') from 'CharPtr' (aka 'char *')}}
+  (void) (Bool) v;
   (void) (Int) v; // expected-warning{{cast to smaller integer type 'Int' (aka 
'int') from 'CharPtr' (aka 'char *')}}
   (void) (Long) v;
   (void) (VoidPtr) v;
@@ -174,7 +174,7 @@
   // from other -Wpointer-to-int-cast warnings.
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wvoid-pointer-to-int-cast"
-  (void) (Bool) v; // expected-warning{{cast to smaller integer type 'Bool' 
(aka '_Bool') from 'CharPtr' (aka 'char *')}}
+  (void) (Int) v; // expected-warning{{cast to smaller integer type 'Int' (aka 
'int') from 'CharPtr' (aka 'char *')}}
 #pragma clang diagnostic pop
 }
 
Index: clang/test/Sema/MicrosoftExtensions.c
===
--- clang/test/Sema/MicrosoftExtensions.c
+++ clang/test/Sema/MicrosoftExtensions.c
@@ -99,7 +99,7 @@
sh = (short)ptr; // expected-warning{{cast to smaller integer type 'short' 
from 'char *' is a Microsoft extension}}
 
// This is valid ISO C.
-   _Bool b = (_Bool)ptr; // expected-warning{{cast to smaller integer type 
'_Bool' from 'char *' is a Microsoft extension}}
+   _Bool b = (_Bool)ptr;
 }
 
 typedef struct {
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2764,7 +2764,8 @@
 }
 
 if ((Self.Context.getTypeSize(SrcType) >
- Self.Context.getTypeSize(DestType))) {
+ Self.Context.getTypeSize(DestType)) &&
+!DestType->isBooleanType()) {
   // C 6.3.2.3p6: Any pointer type may be converted to an integer type.
   // Except as previously specified, the result is implementation-defined.
   // If the result cannot be represented in the integer type, the behavior


Index: clang/test/Sema/cast.c
===
--- clang/test/Sema/cast.c
+++ clang/test/Sema/cast.c
@@ -151,7 +151,7 @@
 }
 
 void testVoidPtr(VoidPtr v) {
-  (void) (Bool) v; // expected-warning{{cast to smaller integer type 'Bool' (aka '_Bool') from 'VoidPtr' (aka 'void *')}}
+  (void) (Bool) v;
   (void) (Int) v; // expected-warning{{cast to smaller integer type 'Int' (aka 'int') from 'VoidPtr' (aka 'void *')}}
   (void) (Long) v;
   (void) (VoidPtr) v;
@@ -160,12 +160,12 @@
   // from other -Wpointer-to-int-cast warnings.
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wvoid-pointer-to-int-cast"
-  (void) (Bool) v; // no-warning
+  (void) (Int) v; // no-warning
 #pragma clang diagnostic pop
 }
 
 void testCharPtr(CharPtr v) {
-  (void) (Bool) v; // expected-warning{{cast to smaller integer type 'Bool' (aka '_Bool') from 'CharPtr' (aka 'char *')}}
+  (void) (Bool) v;
   (void) (Int) v; // expected-warning{{cast to smaller integer type 'Int' (aka 'int') from 'CharPtr' (aka 'char *')}}
   (void) (Long) v;
   (void) (VoidPtr) v;
@@ -174,7 +174,7 @@
   // from other -Wpointer-to-int-cast warnings.
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wvoid-pointer-to-int-cast"
-  (void) (Bool) v; // expected-warning{{cast to smaller integer type 'Bool' (aka '_Bool') from 'CharPtr' (aka 'char *')}}
+  (void) (Int) v; // expected-warning{{cast to smaller integer type 'Int' (aka 'int') from 'CharPtr' (aka 'char *')}}
 #pragma clang diagnostic pop
 }
 
Index: clang/test/Sema/MicrosoftExtensions.c
===
--- clang/test/Sema/MicrosoftExtensions.c
+++ clang/test/Sema/MicrosoftExtensions.c
@@ -99,7 +99,7

[PATCH] D74455: [MS] Pass aligned, non-trivially copyable things indirectly on x86

[Reid Kleckner via Phabricator via cfe-commits]

rnk updated this revision to Diff 245491.
rnk added a comment.

- rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74455

Files:
  clang/lib/CodeGen/MicrosoftCXXABI.cpp
  clang/test/CodeGenCXX/inalloca-overaligned.cpp


Index: clang/test/CodeGenCXX/inalloca-overaligned.cpp
===
--- clang/test/CodeGenCXX/inalloca-overaligned.cpp
+++ clang/test/CodeGenCXX/inalloca-overaligned.cpp
@@ -4,10 +4,6 @@
 // MSVC passes overaligned types indirectly since MSVC 2015. Make sure that
 // works with inalloca.
 
-// FIXME: Pass non-trivial *and* overaligned types indirectly. Right now the 
C++
-// ABI rules say to use inalloca, and they take precedence, so it's not easy to
-// implement this.
-
 
 struct NonTrivial {
   NonTrivial();
@@ -50,3 +46,27 @@
 // CHECK: getelementptr inbounds <{ %struct.NonTrivial, %struct.OverAligned* 
}>, <{ %struct.NonTrivial, %struct.OverAligned* }>* %{{.*}}, i32 0, i32 1
 // CHECK: store %struct.OverAligned* [[TMP]], %struct.OverAligned** %{{.*}}, 
align 4
 // CHECK: call i32 @"?receive_inalloca_overaligned@@Y{{.*}}"(<{ 
%struct.NonTrivial, %struct.OverAligned* }>* inalloca %argmem)
+
+struct __declspec(align(64)) Both {
+  Both();
+  Both(const Both &o);
+  int x;
+};
+
+int receive_both1(Both b) {
+  return b.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_both1@@Y{{.*}}"
+// CHECK-SAME: (%struct.Both* %b)
+
+int receive_both2(Both x, Both y) {
+  return x.x + y.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_both2@@Y{{.*}}"
+// CHECK-SAME: (%struct.Both* %x, %struct.Both* %y)
+
+int receive_nt_and_both(NonTrivial x, Both y) {
+  return x.x + y.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_nt_and_both@@Y{{.*}}"
+// CHECK-SAME: (<{ %struct.NonTrivial, %struct.Both* }>* inalloca %0)
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp
===
--- clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -821,18 +821,19 @@
 return !RD->canPassInRegisters() ? RAA_Indirect : RAA_Default;
 
   case llvm::Triple::x86:
-// All record arguments are passed in memory on x86.  Decide whether to
-// construct the object directly in argument memory, or to construct the
-// argument elsewhere and copy the bytes during the call.
-
-// If C++ prohibits us from making a copy, construct the arguments directly
-// into argument memory.
-if (!RD->canPassInRegisters())
-  return RAA_DirectInMemory;
-
-// Otherwise, construct the argument into a temporary and copy the bytes
-// into the outgoing argument memory.
-return RAA_Default;
+// If C++ allows us to copy the struct bytes, use the normal C convention
+// rules.
+if (RD->canPassInRegisters())
+  return RAA_Default;
+
+// If the type has a required alignment (alignas, __declspec(align)), MSVC
+// passes it indirectly.
+if (getContext().isAlignmentRequired(getContext().getTypeDeclType(RD)))
+  return RAA_Indirect;
+
+// In the worst case, the argument must be constructed directly into the
+// outgoing argument memory slots.
+return RAA_DirectInMemory;
 
   case llvm::Triple::x86_64:
   case llvm::Triple::aarch64:


Index: clang/test/CodeGenCXX/inalloca-overaligned.cpp
===
--- clang/test/CodeGenCXX/inalloca-overaligned.cpp
+++ clang/test/CodeGenCXX/inalloca-overaligned.cpp
@@ -4,10 +4,6 @@
 // MSVC passes overaligned types indirectly since MSVC 2015. Make sure that
 // works with inalloca.
 
-// FIXME: Pass non-trivial *and* overaligned types indirectly. Right now the C++
-// ABI rules say to use inalloca, and they take precedence, so it's not easy to
-// implement this.
-
 
 struct NonTrivial {
   NonTrivial();
@@ -50,3 +46,27 @@
 // CHECK: getelementptr inbounds <{ %struct.NonTrivial, %struct.OverAligned* }>, <{ %struct.NonTrivial, %struct.OverAligned* }>* %{{.*}}, i32 0, i32 1
 // CHECK: store %struct.OverAligned* [[TMP]], %struct.OverAligned** %{{.*}}, align 4
 // CHECK: call i32 @"?receive_inalloca_overaligned@@Y{{.*}}"(<{ %struct.NonTrivial, %struct.OverAligned* }>* inalloca %argmem)
+
+struct __declspec(align(64)) Both {
+  Both();
+  Both(const Both &o);
+  int x;
+};
+
+int receive_both1(Both b) {
+  return b.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_both1@@Y{{.*}}"
+// CHECK-SAME: (%struct.Both* %b)
+
+int receive_both2(Both x, Both y) {
+  return x.x + y.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_both2@@Y{{.*}}"
+// CHECK-SAME: (%struct.Both* %x, %struct.Both* %y)
+
+int receive_nt_and_both(NonTrivial x, Both y) {
+  return x.x + y.x;
+}
+// CHECK-LABEL: define dso_local i32 @"?receive_nt_and_both@@Y{{.*}}"
+// CHECK-SAME: (<{ %struct.NonTrivial, %struct.Both* }>* inalloca %0)
Index: clang/lib/CodeGen/MicrosoftCXXABI.cpp

[PATCH] D74452: [MS] Mark vectorcall FP and vector args inreg

[Reid Kleckner via Phabricator via cfe-commits]

rnk updated this revision to Diff 245490.
rnk added a comment.

- rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74452

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/vectorcall.c
  clang/test/CodeGenCXX/inalloca-vector.cpp

Index: clang/test/CodeGenCXX/inalloca-vector.cpp
===
--- clang/test/CodeGenCXX/inalloca-vector.cpp
+++ clang/test/CodeGenCXX/inalloca-vector.cpp
@@ -66,14 +66,13 @@
  __m128 w, int edx, __m128 q, NonTrivial nt) {
   gv128 = x + y + z + w + q;
 }
-// FIXME: Enable these checks, clang generates wrong IR.
 // CHECK-LABEL: define dso_local x86_vectorcallcc void @"?vectorcall_receive_vec@@Y{{[^"]*}}"
-// CHECKX-SAME: (double inreg %xmm0,
-// CHECKX-SAME: double inreg %xmm1,
-// CHECKX-SAME: double inreg %xmm2,
-// CHECKX-SAME: <4 x float> inreg %x,
-// CHECKX-SAME: <4 x float> inreg %y,
-// CHECKX-SAME: <4 x float> inreg %z,
-// CHECKX-SAME: <4 x float>* inreg %0,
-// CHECKX-SAME: i32 inreg %edx,
-// CHECKX-SAME: <{ <4 x float>*, %struct.NonTrivial }>* inalloca %1)
+// CHECK-SAME: (double inreg %xmm0,
+// CHECK-SAME: double inreg %xmm1,
+// CHECK-SAME: double inreg %xmm2,
+// CHECK-SAME: <4 x float> inreg %x,
+// CHECK-SAME: <4 x float> inreg %y,
+// CHECK-SAME: <4 x float> inreg %z,
+// CHECK-SAME: <4 x float>* inreg %0,
+// CHECK-SAME: i32 inreg %edx,
+// CHECK-SAME: <{ <4 x float>*, %struct.NonTrivial }>* inalloca %1)
Index: clang/test/CodeGen/vectorcall.c
===
--- clang/test/CodeGen/vectorcall.c
+++ clang/test/CodeGen/vectorcall.c
@@ -31,13 +31,13 @@
 // indirectly. Additional vector arguments can consume the rest of the SSE
 // registers.
 void __vectorcall hfa2(struct HFA4 a, struct HFA4 b, double c) {}
-// X32: define dso_local x86_vectorcallcc void @"\01hfa2@@72"(%struct.HFA4 inreg %a.coerce, %struct.HFA4* inreg %b, double %c)
+// X32: define dso_local x86_vectorcallcc void @"\01hfa2@@72"(%struct.HFA4 inreg %a.coerce, %struct.HFA4* inreg %b, double inreg %c)
 // X64: define dso_local x86_vectorcallcc void @"\01hfa2@@72"(%struct.HFA4 inreg %a.coerce, %struct.HFA4* %b, double %c)
 
 // Ensure that we pass builtin types directly while counting them against the
 // SSE register usage.
 void __vectorcall hfa3(double a, double b, double c, double d, double e, struct HFA2 f) {}
-// X32: define dso_local x86_vectorcallcc void @"\01hfa3@@56"(double %a, double %b, double %c, double %d, double %e, %struct.HFA2* inreg %f)
+// X32: define dso_local x86_vectorcallcc void @"\01hfa3@@56"(double inreg %a, double inreg %b, double inreg %c, double inreg %d, double inreg %e, %struct.HFA2* inreg %f)
 // X64: define dso_local x86_vectorcallcc void @"\01hfa3@@56"(double %a, double %b, double %c, double %d, double %e, %struct.HFA2* %f)
 
 // Aggregates with more than four elements are not HFAs and are passed byval.
@@ -64,21 +64,21 @@
 // X64: define dso_local x86_vectorcallcc <4 x float> @"\01hva1@@80"(i32 %a, %struct.HVA4 inreg %b.coerce, i32 %c)
 
 v4f32 __vectorcall hva2(struct HVA4 a, struct HVA4 b, v4f32 c) {return c;}
-// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva2@@144"(%struct.HVA4 inreg %a.coerce, %struct.HVA4* inreg %b, <4 x float> %c)
+// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva2@@144"(%struct.HVA4 inreg %a.coerce, %struct.HVA4* inreg %b, <4 x float> inreg %c)
 // X64: define dso_local x86_vectorcallcc <4 x float> @"\01hva2@@144"(%struct.HVA4 inreg %a.coerce, %struct.HVA4* %b, <4 x float> %c)
 
 v4f32 __vectorcall hva3(v4f32 a, v4f32 b, v4f32 c, v4f32 d, v4f32 e, struct HVA2 f) {return f.x;}
-// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva3@@112"(<4 x float> %a, <4 x float> %b, <4 x float> %c, <4 x float> %d, <4 x float> %e, %struct.HVA2* inreg %f)
+// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva3@@112"(<4 x float> inreg %a, <4 x float> inreg %b, <4 x float> inreg %c, <4 x float> inreg %d, <4 x float> inreg %e, %struct.HVA2* inreg %f)
 // X64: define dso_local x86_vectorcallcc <4 x float> @"\01hva3@@112"(<4 x float> %a, <4 x float> %b, <4 x float> %c, <4 x float> %d, <4 x float> %e, %struct.HVA2* %f)
 
 // vector types have higher priority then HVA structures, So vector types are allocated first
 // and HVAs are allocated if enough registers are available
 v4f32 __vectorcall hva4(struct HVA4 a, struct HVA2 b, v4f32 c) {return b.y;}
-// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva4@@112"(%struct.HVA4 inreg %a.coerce, %struct.HVA2* inreg %b, <4 x float> %c)
+// X32: define dso_local x86_vectorcallcc <4 x float> @"\01hva4@@112"(%struct.HVA4 inreg %a.coerce, %struct.HVA2* inreg %b, <4 x float> inreg %c)
 // X64: define dso_local x86_vectorcallcc <4 x float> @"\01hva4@@112"(%struct.HVA4 inreg %a.coerce, %struct.HVA2* %b, <4 x float> %c)
 
 v4f32 __v

[PATCH] D74116: [Sema][C++] Propagate conversion type in order to specialize the diagnostics

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Yeah, looks like returning true causes the variable to be marked as invalid 
(not unreasonable, since maybe its type is wrong), which causes downstream 
diagnostics to be suppressed.

Both test cases should probably be fixed to assign into a different variable so 
that we continue to test the diagnostic.


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

https://reviews.llvm.org/D74116



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

[PATCH] D74452: [MS] Mark vectorcall FP and vector args inreg

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74452



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

[PATCH] D73996: [Sema] Demote call-site-based 'alignment is a power of two' check for AllocAlignAttr into a warning

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D73996#1883360 , @erichkeane wrote:

> 1 nit, otherwise LGTM.


Thank you for the review!




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2996
+def warn_alignment_not_power_of_two : Warning<
+  "requested alignment is not a power of 2">,
+  InGroup>;

erichkeane wrote:
> Can you use the above one (err_alignment_not_power_of_two .Text) so that they 
> are forced to have the same message?
Oh, i always forget about that feature, thanks for pointing it out!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73996



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

[PATCH] D73996: [Sema] Demote call-site-based 'alignment is a power of two' check for AllocAlignAttr into a warning

[Erich Keane via Phabricator via cfe-commits]

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

1 nit, otherwise LGTM.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2996
+def warn_alignment_not_power_of_two : Warning<
+  "requested alignment is not a power of 2">,
+  InGroup>;

Can you use the above one (err_alignment_not_power_of_two .Text) so that they 
are forced to have the same message?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73996



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

[PATCH] D74562: [OpenMP][OMPIRBuilder] Introducing the `OMPBuilderCBHelpers` helper class

[Johannes Doerfert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGba3f863dfb9c: [OpenMP][OMPIRBuilder] Introducing the 
`OMPBuilderCBHelpers` helper class (authored by fghanim, committed by 
jdoerfert).

Changed prior to commit:
  https://reviews.llvm.org/D74562?vs=245227&id=245486#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74562

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/OpenMP/cancel_codegen.cpp

Index: clang/test/OpenMP/cancel_codegen.cpp
===
--- clang/test/OpenMP/cancel_codegen.cpp
+++ clang/test/OpenMP/cancel_codegen.cpp
@@ -193,11 +193,9 @@
 // IRBUILDER: br i1 [[CMP]], label %[[CONTINUE:[^,].+]], label %[[EXIT:.+]]
 // IRBUILDER: [[EXIT]]
 // IRBUILDER: br label %[[EXIT2:.+]]
-// IRBUILDER: [[EXIT2]]
-// IRBUILDER: br label %[[EXIT3:.+]]
 // IRBUILDER: [[CONTINUE]]
 // IRBUILDER: br label %[[ELSE:.+]]
-// IRBUILDER: [[EXIT3]]
+// IRBUILDER: [[EXIT2]]
 // IRBUILDER: br label %[[RETURN]]
 
 #endif
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -36,6 +36,7 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
 #include "llvm/IR/ValueHandle.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Transforms/Utils/SanitizerStats.h"
@@ -255,6 +256,114 @@
 unsigned Index;
   };
 
+  // Helper class for the OpenMP IR Builder. Allows reusability of code used for
+  // region body, and finalization codegen callbacks. This will class will also
+  // contain privatization functions used by the privatization call backs
+  struct OMPBuilderCBHelpers {
+
+using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
+
+/// Emit the Finalization for an OMP region
+/// \param CGF	The Codegen function this belongs to
+/// \param IP	Insertion point for generating the finalization code.
+static void FinalizeOMPRegion(CodeGenFunction &CGF, InsertPointTy IP) {
+  CGBuilderTy::InsertPointGuard IPG(CGF.Builder);
+  assert(IP.getBlock()->end() != IP.getPoint() &&
+ "OpenMP IR Builder should cause terminated block!");
+
+  llvm::BasicBlock *IPBB = IP.getBlock();
+  llvm::BasicBlock *DestBB = IPBB->getUniqueSuccessor();
+  assert(DestBB && "Finalization block should have one successor!");
+
+  // erase and replace with cleanup branch.
+  IPBB->getTerminator()->eraseFromParent();
+  CGF.Builder.SetInsertPoint(IPBB);
+  CodeGenFunction::JumpDest Dest = CGF.getJumpDestInCurrentScope(DestBB);
+  CGF.EmitBranchThroughCleanup(Dest);
+}
+
+/// Emit the body of an OMP region
+/// \param CGF	The Codegen function this belongs to
+/// \param RegionBodyStmt	The body statement for the OpenMP region being
+/// 			 generated
+/// \param CodeGenIP	Insertion point for generating the body code.
+/// \param FiniBB	The finalization basic block
+static void EmitOMPRegionBody(CodeGenFunction &CGF,
+  const Stmt *RegionBodyStmt,
+  InsertPointTy CodeGenIP,
+  llvm::BasicBlock &FiniBB) {
+  llvm::BasicBlock *CodeGenIPBB = CodeGenIP.getBlock();
+  if (llvm::Instruction *CodeGenIPBBTI = CodeGenIPBB->getTerminator())
+CodeGenIPBBTI->eraseFromParent();
+
+  CGF.Builder.SetInsertPoint(CodeGenIPBB);
+
+  CGF.EmitStmt(RegionBodyStmt);
+
+  if (CGF.Builder.saveIP().isSet())
+CGF.Builder.CreateBr(&FiniBB);
+}
+
+/// RAII for preserving necessary info during Outlined region body codegen.
+class OutlinedRegionBodyRAII {
+
+  llvm::AssertingVH OldAllocaIP;
+  CodeGenFunction::JumpDest OldReturnBlock;
+  CodeGenFunction &CGF;
+
+public:
+  OutlinedRegionBodyRAII(CodeGenFunction &cgf, InsertPointTy &AllocaIP,
+ llvm::BasicBlock &RetBB)
+  : CGF(cgf) {
+assert(AllocaIP.isSet() &&
+   "Must specify Insertion point for allocas of outlined function");
+OldAllocaIP = CGF.AllocaInsertPt;
+CGF.AllocaInsertPt = &*AllocaIP.getPoint();
+
+OldReturnBlock = CGF.ReturnBlock;
+CGF.ReturnBlock = CGF.getJumpDestInCurrentScope(&RetBB);
+  }
+
+  ~OutlinedRegionBodyRAII() {
+CGF.AllocaInsertPt = OldAllocaIP;
+CGF.ReturnBlock = OldReturnBlock;
+  }
+};
+
+/// RAII for preserving necessary info during inlined region body codegen.
+class InlinedRegionBodyRAII {
+
+  llvm::AssertingVH OldAllocaIP;
+  CodeGenFunction &CGF;
+
+public:
+  InlinedRegionBodyRAII(CodeGenFunction &cgf, InsertPointTy &AllocaIP,
+   

[PATCH] D71227: [cuda][hip] Fix function overload resolution in the global initiailizer.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:11442
+  ContextDecl = getCUDACurrentNonLocalVariable();
+return ContextDecl;
+  }

This is tricky because we could be in a nested context, not just the 
initializer, and that context just might not be a function.   For example, 
there could be a local class in a lambda or something like that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71227



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

[PATCH] D74116: [Sema][C++] Propagate conversion type in order to specialize the diagnostics

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia marked an inline comment as done.
Anastasia added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:14829
+if (getLangOpts().CPlusPlus)
+  isInvalid = true;
 break;

rjmccall wrote:
> Could you hoist this up to the place where we pick the diagnostic and then 
> make it unconditional in the path that always selects an error?
> 
> That's a general comment: we should be setting `isInvalid` for all the places 
> below where we select an error diagnostic.
> That's a general comment: we should be setting isInvalid for all the places 
> below where we select an error diagnostic.

If I change to set `isInvalid` for all error cases I get less diagnostics. I 
guess it's because we now stop earlier and don't continue to diagnosing in some 
cases.


```
FAIL: Clang :: Sema/block-call.c (9327 of 16808)
 TEST 'Clang :: Sema/block-call.c' FAILED 

Script:
--
: 'RUN: at line 1';   /data/llvm/llvm-forcommits/build/bin/clang -cc1 
-internal-isystem /data/llvm/llvm-forcommits/build/lib/clang/11.0.0/include 
-nostdsysteminc -fsyntax-only -verify 
/data/llvm/llvm-forcommits/clang/test/Sema/block-call.c -fblocks
--
Exit Code: 1

Command Output (stderr):
--
error: 'error' diagnostics expected but not seen: 
  File /data/llvm/llvm-forcommits/clang/test/Sema/block-call.c Line 36: invalid 
block pointer conversion assigning to 'int *(^)()' from 'int'
1 error generated.

--


FAIL: Clang :: SemaObjC/arc.m (10804 of 16808)
 TEST 'Clang :: SemaObjC/arc.m' FAILED 
Script:
--
: 'RUN: at line 1';   /data/llvm/llvm-forcommits/build/bin/clang -cc1 
-internal-isystem /data/llvm/llvm-forcommits/build/lib/clang/11.0.0/include 
-nostdsysteminc -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak 
-fsyntax-only -fobjc-arc -fblocks -verify -Wno-objc-root-class 
/data/llvm/llvm-forcommits/clang/test/SemaObjC/arc.m
: 'RUN: at line 2';   not /data/llvm/llvm-forcommits/build/bin/clang -cc1 
-internal-isystem /data/llvm/llvm-forcommits/build/lib/clang/11.0.0/include 
-nostdsysteminc -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak 
-fsyntax-only -fobjc-arc -fblocks -Wno-objc-root-class 
-fdiagnostics-parseable-fixits 
/data/llvm/llvm-forcommits/clang/test/SemaObjC/arc.m 2>&1
--
Exit Code: 1

Command Output (stderr):
--
error: 'error' diagnostics expected but not seen: 
  File /data/llvm/llvm-forcommits/clang/test/SemaObjC/arc.m Line 117: assigning 
'__strong id *' to '__autoreleasing id *' changes retain/release properties of 
pointer
1 error generated.

```


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

https://reviews.llvm.org/D74116



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

[clang] ba3f863 - [OpenMP][OMPIRBuilder] Introducing the `OMPBuilderCBHelpers` helper class

[Johannes Doerfert via cfe-commits]

Author: Fady Ghanim
Date: 2020-02-19T14:11:17-06:00
New Revision: ba3f863dfb9c5f9bf5e6fdca2198b609df3b7761

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

LOG: [OpenMP][OMPIRBuilder] Introducing the `OMPBuilderCBHelpers` helper class

This patch introduces a new helper class `OMPBuilderCBHelpers`,
which will contain all reusable C/C++ language specific function-
alities required by the `OMPIRBuilder`.

Initially, this helper class contains the body and finalization
codegen functionalities implemented using callbacks which were
moved here for reusability among the different directives
implemented in the `OMPIRBuilder`, along with RAIIs for preserving
state prior to emitting outlined and/or inlined OpenMP regions.

In the future this helper class will also contain all the different
call backs required by OpenMP clauses/variable privatization.

Reviewed By: jdoerfert

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

Added: 


Modified: 
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/test/OpenMP/cancel_codegen.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp 
b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index d7115e00836c..bcd2d0635caf 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -1450,15 +1450,7 @@ void CodeGenFunction::EmitOMPParallelDirective(const 
OMPParallelDirective &S) {
 // The cleanup callback that finalizes all variabels at the given location,
 // thus calls destructors etc.
 auto FiniCB = [this](InsertPointTy IP) {
-  CGBuilderTy::InsertPointGuard IPG(Builder);
-  assert(IP.getBlock()->end() != IP.getPoint() &&
- "OpenMP IR Builder should cause terminated block!");
-  llvm::BasicBlock *IPBB = IP.getBlock();
-  llvm::BasicBlock *DestBB = IPBB->splitBasicBlock(IP.getPoint());
-  IPBB->getTerminator()->eraseFromParent();
-  Builder.SetInsertPoint(IPBB);
-  CodeGenFunction::JumpDest Dest = getJumpDestInCurrentScope(DestBB);
-  EmitBranchThroughCleanup(Dest);
+  OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
 };
 
 // Privatization callback that performs appropriate action for
@@ -1480,25 +1472,10 @@ void CodeGenFunction::EmitOMPParallelDirective(const 
OMPParallelDirective &S) {
 auto BodyGenCB = [ParallelRegionBodyStmt,
   this](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
 llvm::BasicBlock &ContinuationBB) {
-  auto OldAllocaIP = AllocaInsertPt;
-  AllocaInsertPt = &*AllocaIP.getPoint();
-
-  auto OldReturnBlock = ReturnBlock;
-  ReturnBlock = getJumpDestInCurrentScope(&ContinuationBB);
-
-  llvm::BasicBlock *CodeGenIPBB = CodeGenIP.getBlock();
-  CodeGenIPBB->splitBasicBlock(CodeGenIP.getPoint());
-  llvm::Instruction *CodeGenIPBBTI = CodeGenIPBB->getTerminator();
-  CodeGenIPBBTI->removeFromParent();
-
-  Builder.SetInsertPoint(CodeGenIPBB);
-
-  EmitStmt(ParallelRegionBodyStmt);
-
-  Builder.Insert(CodeGenIPBBTI);
-
-  AllocaInsertPt = OldAllocaIP;
-  ReturnBlock = OldReturnBlock;
+  OMPBuilderCBHelpers::OutlinedRegionBodyRAII ORB(*this, AllocaIP,
+  ContinuationBB);
+  OMPBuilderCBHelpers::EmitOMPRegionBody(*this, ParallelRegionBodyStmt,
+ CodeGenIP, ContinuationBB);
 };
 
 CGCapturedStmtInfo CGSI(*CS, CR_OpenMP);
@@ -3152,55 +3129,18 @@ void CodeGenFunction::EmitOMPMasterDirective(const 
OMPMasterDirective &S) {
 const CapturedStmt *CS = S.getInnermostCapturedStmt();
 const Stmt *MasterRegionBodyStmt = CS->getCapturedStmt();
 
-// TODO: Replace with a generic helper function for finalization
 auto FiniCB = [this](InsertPointTy IP) {
-  CGBuilderTy::InsertPointGuard IPG(Builder);
-  assert(IP.getBlock()->end() != IP.getPoint() &&
- "OpenMP IR Builder should cause terminated block!");
-
-  llvm::BasicBlock *IPBB = IP.getBlock();
-  llvm::BasicBlock *DestBB = IPBB->getUniqueSuccessor();
-  assert(DestBB && "Finalization block should have one successor!");
-
-  // erase and replace with cleanup branch.
-  IPBB->getTerminator()->eraseFromParent();
-  Builder.SetInsertPoint(IPBB);
-  CodeGenFunction::JumpDest Dest = getJumpDestInCurrentScope(DestBB);
-  EmitBranchThroughCleanup(Dest);
+  OMPBuilderCBHelpers::FinalizeOMPRegion(*this, IP);
 };
 
-// TODO: Replace with a generic helper function for emitting body
 auto BodyGenCB = [MasterRegionBodyStmt, this](InsertPointTy AllocaIP,
   InsertPointTy CodeGenIP,

[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Basic/FixedPoint.cpp:242
+  } else
+Overflowed = Result < Min || Result > Max;
+

leonardchan wrote:
> ebevhan wrote:
> > rjmccall wrote:
> > > ebevhan wrote:
> > > > rjmccall wrote:
> > > > > leonardchan wrote:
> > > > > > ebevhan wrote:
> > > > > > > rjmccall wrote:
> > > > > > > > ebevhan wrote:
> > > > > > > > > ebevhan wrote:
> > > > > > > > > > rjmccall wrote:
> > > > > > > > > > > If the maximum expressible value is *k*, and the 
> > > > > > > > > > > fully-precise multiplication yields *k+e* for some 
> > > > > > > > > > > epsilon *e* that isn't representable in the result 
> > > > > > > > > > > semantics, is that considered an overflow?  If so, I 
> > > > > > > > > > > think you need to do the shift after these bound checks, 
> > > > > > > > > > > since the shift destroys the difference between *k* and 
> > > > > > > > > > > *k+e*.  That is, unless there's a compelling mathematical 
> > > > > > > > > > > argument that it's not possible to overflow only in the 
> > > > > > > > > > > fully-precision multiplication — but while I think that's 
> > > > > > > > > > > possibly true of `_Fract` (since *k^2 < k*), it seems 
> > > > > > > > > > > unlikely to be true of `_Accum`, although I haven't 
> > > > > > > > > > > looked for a counter-example.  And if there is a 
> > > > > > > > > > > compelling argument, it should probably be at least 
> > > > > > > > > > > alluded to in a comment.
> > > > > > > > > > > 
> > > > > > > > > > > Would this algorithm be simpler if you took advantage of 
> > > > > > > > > > > the fact that `APFixedPointSemantics` doesn't have to 
> > > > > > > > > > > correspond to a real type?  You could probably just 
> > > > > > > > > > > convert to a double-width common semantics, right?
> > > > > > > > > > > If the maximum expressible value is *k*, and the 
> > > > > > > > > > > fully-precise multiplication yields *k+e* for some 
> > > > > > > > > > > epsilon *e* that isn't representable in the result 
> > > > > > > > > > > semantics, is that considered an overflow? If so, I think 
> > > > > > > > > > > you need to do the shift after these bound checks, since 
> > > > > > > > > > > the shift destroys the difference between *k* and *k+e*.
> > > > > > > > > > 
> > > > > > > > > > I don't think I would consider that to be overflow; that's 
> > > > > > > > > > precision loss. E-C considers these to be different:
> > > > > > > > > > 
> > > > > > > > > > > If the source value cannot be represented exactly by the 
> > > > > > > > > > > fixed-point type, the source value is rounded to either 
> > > > > > > > > > > the closest fixed-point value greater than the source 
> > > > > > > > > > > value (rounded up) or to the closest fixed-point value 
> > > > > > > > > > > less than the source value (rounded down).
> > > > > > > > > > >
> > > > > > > > > > > When the source value does not fit within the range of 
> > > > > > > > > > > the fixed-point type, the conversion overflows. [...]
> > > > > > > > > > >
> > > > > > > > > > > [...]
> > > > > > > > > > >
> > > > > > > > > > > If the result type of an arithmetic operation is a 
> > > > > > > > > > > fixed-point type, [...] the calculated result is the 
> > > > > > > > > > > mathematically exact result with overflow handling and 
> > > > > > > > > > > rounding performed to the full precision of the result 
> > > > > > > > > > > type as explained in 4.1.3. 
> > > > > > > > > > 
> > > > > > > > > > There is also no value of `e` that would affect saturation. 
> > > > > > > > > > Any full precision calculation that gives `k+e` must be `k` 
> > > > > > > > > > after downscaling, since the bits that represent `e` must 
> > > > > > > > > > come from the extra precision range. Even though `k+e` is 
> > > > > > > > > > technically larger than `k`, saturation would still just 
> > > > > > > > > > give us `k` after truncating out `e`, so the end result is 
> > > > > > > > > > the same.
> > > > > > > > > > 
> > > > > > > > > > > Would this algorithm be simpler if you took advantage of 
> > > > > > > > > > > the fact that APFixedPointSemantics doesn't have to 
> > > > > > > > > > > correspond to a real type? You could probably just 
> > > > > > > > > > > convert to a double-width common semantics, right?
> > > > > > > > > > 
> > > > > > > > > > It's likely possible to use APFixedPoint in the 
> > > > > > > > > > calculations here, but I used APInt to make the behavior 
> > > > > > > > > > explicit and not accidentally be dependent on the behavior 
> > > > > > > > > > of APFixedPoint's conversions or operations.
> > > > > > > > > Although.,. I guess I see your point in that an intermediate 
> > > > > > > > > result of k+e technically "does not fit within the range of 
> > > > > > > > > the fixed-point type"... but I wonder if treating such cases 
> > > > > > > > > as overflow is particularly meaningful. I don't find there to 
> > > > > > > > > be much of a distinction 

[clang] f6875c4 - Reapply [IRBuilder] Always respect inserter/folder

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2020-02-19T20:51:38+01:00
New Revision: f6875c434ec20eb4f24495317592f64334347784

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

LOG: Reapply [IRBuilder] Always respect inserter/folder

Some IRBuilder methods that were originally defined on
IRBuilderBase do not respect custom IRBuilder inserters/folders,
because those were not accessible prior to D73835. Fix this by
making use of existing (and now accessible) IRBuilder methods,
which will handle inserters/folders correctly.

There are some changes in OpenMP and Instrumentation tests, where
bitcasts now get constant folded. I've also highlighted one
InstCombine test which now finishes in two rather than three
iterations, thanks to new instructions being inserted into the
worklist.

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

Added: 


Modified: 
clang/test/OpenMP/parallel_codegen.cpp
clang/test/OpenMP/target_firstprivate_codegen.cpp
llvm/lib/IR/IRBuilder.cpp
llvm/test/Instrumentation/MemorySanitizer/Mips/vararg-mips64.ll
llvm/test/Instrumentation/MemorySanitizer/Mips/vararg-mips64el.ll
llvm/test/Instrumentation/MemorySanitizer/PowerPC/vararg-ppc64.ll
llvm/test/Instrumentation/MemorySanitizer/PowerPC/vararg-ppc64le.ll
llvm/test/Instrumentation/MemorySanitizer/X86/vararg_call.ll
llvm/test/Instrumentation/MemorySanitizer/byval-alignment.ll
llvm/test/Instrumentation/MemorySanitizer/msan_basic.ll
llvm/test/Transforms/InstCombine/saturating-add-sub.ll

Removed: 




diff  --git a/clang/test/OpenMP/parallel_codegen.cpp 
b/clang/test/OpenMP/parallel_codegen.cpp
index f96ad406c25f..586187f09521 100644
--- a/clang/test/OpenMP/parallel_codegen.cpp
+++ b/clang/test/OpenMP/parallel_codegen.cpp
@@ -60,8 +60,7 @@ int main (int argc, char **argv) {
 // ALL-DEBUG-LABEL: define i32 @main(i32 %argc, i8** %argv)
 // CHECK-DEBUG:   [[LOC_2_ADDR:%.+]] = alloca %struct.ident_t
 // CHECK-DEBUG:   [[KMPC_LOC_VOIDPTR:%.+]] = bitcast %struct.ident_t* 
[[LOC_2_ADDR]] to i8*
-// CHECK-DEBUG-NEXT:  [[KMPC_DEFAULT_LOC_VOIDPTR:%.+]] = bitcast 
%struct.ident_t* [[DEF_LOC_2]] to i8*
-// CHECK-DEBUG-NEXT:   call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 
[[KMPC_LOC_VOIDPTR]], i8* align 8 [[KMPC_DEFAULT_LOC_VOIDPTR]], i64 24, i1 
false)
+// CHECK-DEBUG-NEXT:   call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 
[[KMPC_LOC_VOIDPTR]], i8* align 8 bitcast (%struct.ident_t* [[DEF_LOC_2]] to 
i8*), i64 24, i1 false)
 // ALL-DEBUG:   store i32 %argc, i32* [[ARGC_ADDR:%.+]],
 // ALL-DEBUG:   [[VLA:%.+]] = alloca i32, i64 [[VLA_SIZE:%[^,]+]],
 // CHECK-DEBUG:   [[KMPC_LOC_PSOURCE_REF:%.+]] = getelementptr inbounds 
%struct.ident_t, %struct.ident_t* [[LOC_2_ADDR]], i32 0, i32 4
@@ -118,8 +117,7 @@ int main (int argc, char **argv) {
 // ALL-DEBUG:   define linkonce_odr i32 [[TMAIN]](i8** %argc)
 // CHECK-DEBUG-DAG:   [[LOC_2_ADDR:%.+]] = alloca %struct.ident_t
 // CHECK-DEBUG:   [[KMPC_LOC_VOIDPTR:%.+]] = bitcast %struct.ident_t* 
[[LOC_2_ADDR]] to i8*
-// CHECK-DEBUG-NEXT:  [[KMPC_DEFAULT_LOC_VOIDPTR:%.+]] = bitcast 
%struct.ident_t* [[DEF_LOC_2]] to i8*
-// CHECK-DEBUG-NEXT:   call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 
[[KMPC_LOC_VOIDPTR]], i8* align 8 [[KMPC_DEFAULT_LOC_VOIDPTR]], i64 24, i1 
false)
+// CHECK-DEBUG-NEXT:   call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 
[[KMPC_LOC_VOIDPTR]], i8* align 8 bitcast (%struct.ident_t* [[DEF_LOC_2]] to 
i8*), i64 24, i1 false)
 // CHECK-DEBUG-NEXT:  store i8** %argc, i8*** [[ARGC_ADDR:%.+]],
 // CHECK-DEBUG:  [[KMPC_LOC_PSOURCE_REF:%.+]] = getelementptr inbounds 
%struct.ident_t, %struct.ident_t* [[LOC_2_ADDR]], i32 0, i32 4
 // CHECK-DEBUG-NEXT:  store i8* getelementptr inbounds ([{{.+}} x i8], [{{.+}} 
x i8]* [[LOC2]], i32 0, i32 0), i8** [[KMPC_LOC_PSOURCE_REF]]

diff  --git a/clang/test/OpenMP/target_firstprivate_codegen.cpp 
b/clang/test/OpenMP/target_firstprivate_codegen.cpp
index 0b5f5f9b2b17..b90ab42f5ae2 100644
--- a/clang/test/OpenMP/target_firstprivate_codegen.cpp
+++ b/clang/test/OpenMP/target_firstprivate_codegen.cpp
@@ -336,9 +336,8 @@ int foo(int n, double *ptr) {
   }
   // CHECK:  [[PTR_ADDR_REF:%.+]] = load double*, double** [[PTR_ADDR]],
 
-  // CHECK:  [[FP_E_BC:%.+]] = bitcast [[TTII]]* [[FP_E]] to i8*
   // CHECK:  [[E_BC:%.+]] = bitcast [[TTII]]* [[E:%.+]] to i8*
-  // CHECK:  call void @llvm.memcpy.p0i8.p0i8.i{{64|32}}(i8* {{.*}} 
[[FP_E_BC]], i8* {{.*}} [[E_BC]], i{{64|32}} 8, i1 false)
+  // CHECK:  call void @llvm.memcpy.p0i8.p0i8.i{{64|32}}(i8* {{.*}} bitcast 
([[TTII]]* [[FP_E]] to i8*), i8* {{.*}} [[E_BC]], i{{64|32}} 8, i1 false)
   // CHECK:  [[BASE_PTR_GEP3_0:%.+]] = getelementptr inbounds [2 x i8*], [2 x 
i8*]* [[BASE_PTR_ARR3]], i{{[0-9]+}} 0, i{{[0-9]+}} 0
   // CHECK:  [[BCAS

[PATCH] D72231: [Sema] Adds the pointer-to-int-cast diagnostic

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

In D72231#1882658 , @rjmccall wrote:

> Okay.  Can we raise this with the kernel folks instead of just assuming 
> they'll be opposed?  An obvious patch to fix a few dozen places where they're 
> hit by a warning they intentionally enabled really doesn't seem like a 
> burden.  If they push back, fine, we can enable the warning without covering 
> enums.


In the original diff the warning was opt in, in the final version it's enabled 
by default. So they didn't enable the warning intentionally. I agree; if the 
kernel folks rather not change their code I can create an option to disable the 
warning for enums, just like it can already be disabled for `void*`s.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72231



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

[PATCH] D73186: [AST] Add fixed-point multiplication constant evaluation.

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: clang/lib/Basic/FixedPoint.cpp:242
+  } else
+Overflowed = Result < Min || Result > Max;
+

ebevhan wrote:
> rjmccall wrote:
> > ebevhan wrote:
> > > rjmccall wrote:
> > > > leonardchan wrote:
> > > > > ebevhan wrote:
> > > > > > rjmccall wrote:
> > > > > > > ebevhan wrote:
> > > > > > > > ebevhan wrote:
> > > > > > > > > rjmccall wrote:
> > > > > > > > > > If the maximum expressible value is *k*, and the 
> > > > > > > > > > fully-precise multiplication yields *k+e* for some epsilon 
> > > > > > > > > > *e* that isn't representable in the result semantics, is 
> > > > > > > > > > that considered an overflow?  If so, I think you need to do 
> > > > > > > > > > the shift after these bound checks, since the shift 
> > > > > > > > > > destroys the difference between *k* and *k+e*.  That is, 
> > > > > > > > > > unless there's a compelling mathematical argument that it's 
> > > > > > > > > > not possible to overflow only in the fully-precision 
> > > > > > > > > > multiplication — but while I think that's possibly true of 
> > > > > > > > > > `_Fract` (since *k^2 < k*), it seems unlikely to be true of 
> > > > > > > > > > `_Accum`, although I haven't looked for a counter-example.  
> > > > > > > > > > And if there is a compelling argument, it should probably 
> > > > > > > > > > be at least alluded to in a comment.
> > > > > > > > > > 
> > > > > > > > > > Would this algorithm be simpler if you took advantage of 
> > > > > > > > > > the fact that `APFixedPointSemantics` doesn't have to 
> > > > > > > > > > correspond to a real type?  You could probably just convert 
> > > > > > > > > > to a double-width common semantics, right?
> > > > > > > > > > If the maximum expressible value is *k*, and the 
> > > > > > > > > > fully-precise multiplication yields *k+e* for some epsilon 
> > > > > > > > > > *e* that isn't representable in the result semantics, is 
> > > > > > > > > > that considered an overflow? If so, I think you need to do 
> > > > > > > > > > the shift after these bound checks, since the shift 
> > > > > > > > > > destroys the difference between *k* and *k+e*.
> > > > > > > > > 
> > > > > > > > > I don't think I would consider that to be overflow; that's 
> > > > > > > > > precision loss. E-C considers these to be different:
> > > > > > > > > 
> > > > > > > > > > If the source value cannot be represented exactly by the 
> > > > > > > > > > fixed-point type, the source value is rounded to either the 
> > > > > > > > > > closest fixed-point value greater than the source value 
> > > > > > > > > > (rounded up) or to the closest fixed-point value less than 
> > > > > > > > > > the source value (rounded down).
> > > > > > > > > >
> > > > > > > > > > When the source value does not fit within the range of the 
> > > > > > > > > > fixed-point type, the conversion overflows. [...]
> > > > > > > > > >
> > > > > > > > > > [...]
> > > > > > > > > >
> > > > > > > > > > If the result type of an arithmetic operation is a 
> > > > > > > > > > fixed-point type, [...] the calculated result is the 
> > > > > > > > > > mathematically exact result with overflow handling and 
> > > > > > > > > > rounding performed to the full precision of the result type 
> > > > > > > > > > as explained in 4.1.3. 
> > > > > > > > > 
> > > > > > > > > There is also no value of `e` that would affect saturation. 
> > > > > > > > > Any full precision calculation that gives `k+e` must be `k` 
> > > > > > > > > after downscaling, since the bits that represent `e` must 
> > > > > > > > > come from the extra precision range. Even though `k+e` is 
> > > > > > > > > technically larger than `k`, saturation would still just give 
> > > > > > > > > us `k` after truncating out `e`, so the end result is the 
> > > > > > > > > same.
> > > > > > > > > 
> > > > > > > > > > Would this algorithm be simpler if you took advantage of 
> > > > > > > > > > the fact that APFixedPointSemantics doesn't have to 
> > > > > > > > > > correspond to a real type? You could probably just convert 
> > > > > > > > > > to a double-width common semantics, right?
> > > > > > > > > 
> > > > > > > > > It's likely possible to use APFixedPoint in the calculations 
> > > > > > > > > here, but I used APInt to make the behavior explicit and not 
> > > > > > > > > accidentally be dependent on the behavior of APFixedPoint's 
> > > > > > > > > conversions or operations.
> > > > > > > > Although.,. I guess I see your point in that an intermediate 
> > > > > > > > result of k+e technically "does not fit within the range of the 
> > > > > > > > fixed-point type"... but I wonder if treating such cases as 
> > > > > > > > overflow is particularly meaningful. I don't find there to be 
> > > > > > > > much of a distinction between such a case and the case where 
> > > > > > > > the exact result lands inbetween two representable values. We 
> > > > > > > > just end up with a less precise result.
> > > > > > > Righ

[PATCH] D74814: IR printing for single function with the new pass manager.

[Hongtao Yu via Phabricator via cfe-commits]

hoyFB updated this revision to Diff 245484.
hoyFB added a comment.

Updating D74814 : IR printing for single 
function with the new pass manager.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74814

Files:
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/Other/module-pass-printer.ll


Index: llvm/test/Other/module-pass-printer.ll
===
--- llvm/test/Other/module-pass-printer.ll
+++ llvm/test/Other/module-pass-printer.ll
@@ -1,13 +1,43 @@
 ; Check pass name is only printed once.
-; RUN: opt < %s 2>&1 -forceattrs -disable-output -print-after-all | FileCheck 
%s
-; RUN: opt < %s 2>&1 -forceattrs -disable-output -print-after-all 
-filter-print-funcs=foo,bar | FileCheck %s
+; Check only one function is printed
+; RUN: opt < %s 2>&1 -forceattrs -disable-output -print-after-all 
-filter-print-funcs=foo | FileCheck %s  -check-prefix=FOO
+; RUN: opt < %s 2>&1 -passes=forceattrs -disable-output -print-after-all 
-filter-print-funcs=foo | FileCheck %s  -check-prefix=FOO
+
+; Check pass name is only printed once.
+; Check both functions are printed
+; RUN: opt < %s 2>&1 -forceattrs -disable-output -print-after-all 
-filter-print-funcs=foo,bar | FileCheck %s -check-prefix=BOTH
+; RUN: opt < %s 2>&1 -passes=forceattrs -disable-output -print-after-all 
-filter-print-funcs=foo,bar | FileCheck %s -check-prefix=BOTH
 
 ; Check pass name is not printed if a module doesn't include any function 
specified in -filter-print-funcs.
 ; RUN: opt < %s 2>&1 -forceattrs -disable-output -print-after-all 
-filter-print-funcs=baz | FileCheck %s -allow-empty -check-prefix=EMPTY
+; RUN: opt < %s 2>&1 -passes=forceattrs -disable-output -print-after-all 
-filter-print-funcs=baz | FileCheck %s -allow-empty -check-prefix=EMPTY
+
+; Check whole module is printed with user-specified wildcast switch 
-filter-print-funcs=* or -print-module-scope
+; RUN: opt < %s 2>&1 -forceattrs -disable-output -print-after-all | FileCheck 
%s -check-prefix=ALL
+; RUN: opt < %s 2>&1 -forceattrs -disable-output  -print-after-all 
-filter-print-funcs=* | FileCheck %s -check-prefix=ALL
+; RUN: opt < %s 2>&1 -forceattrs -disable-output -print-after-all 
-filter-print-funcs=foo -print-module-scope | FileCheck %s -check-prefix=ALL
+; RUN: opt < %s 2>&1 -passes=forceattrs -disable-output -print-after-all | 
FileCheck %s -check-prefix=ALL
+; RUN: opt < %s 2>&1 -passes=forceattrs -disable-output -print-after-all 
-filter-print-funcs=* | FileCheck %s -check-prefix=ALL
+; RUN: opt < %s 2>&1 -passes=forceattrs -disable-output -print-after-all 
-filter-print-funcs=foo -print-module-scope | FileCheck %s -check-prefix=ALL
+
+; FOO:  IR Dump After {{Force set function 
attributes|ForceFunctionAttrsPass}}
+; FOO:  define void @foo
+; FOO-NOT:  define void @bar
+; FOO-NOT:  IR Dump After {{Force set function 
attributes|ForceFunctionAttrsPass}}
+
+; BOTH: IR Dump After {{Force set function 
attributes|ForceFunctionAttrsPass}}
+; BOTH: define void @foo
+; BOTH: define void @bar
+; BOTH-NOT: IR Dump After {{Force set function 
attributes|ForceFunctionAttrsPass}}
+; BOTH-NOT: ModuleID =
+
+; EMPTY-NOT: IR Dump After {{Force set function 
attributes|ForceFunctionAttrsPass}}
 
-; CHECK: *** IR Dump After Force set function attributes ***
-; CHECK-NOT: *** IR Dump After Force set function attributes ***
-; EMPTY-NOT: *** IR Dump After Force set function attributes ***
+; ALL:  IR Dump After {{Force set function attributes|ForceFunctionAttrsPass}}
+; ALL:  ModuleID =
+; ALL:  define void @foo
+; ALL:  define void @bar
+; ALL-NOT: IR Dump After {{Force set function 
attributes|ForceFunctionAttrsPass}}
 
 define void @foo() {
   ret void
Index: llvm/lib/Passes/StandardInstrumentations.cpp
===
--- llvm/lib/Passes/StandardInstrumentations.cpp
+++ llvm/lib/Passes/StandardInstrumentations.cpp
@@ -70,16 +70,24 @@
   llvm_unreachable("Unknown IR unit");
 }
 
-void printIR(const Module *M, StringRef Banner, StringRef Extra = StringRef()) 
{
-  dbgs() << Banner << Extra << "\n";
-  M->print(dbgs(), nullptr, false);
-}
 void printIR(const Function *F, StringRef Banner,
  StringRef Extra = StringRef()) {
   if (!llvm::isFunctionInPrintList(F->getName()))
 return;
   dbgs() << Banner << Extra << "\n" << static_cast(*F);
 }
+
+void printIR(const Module *M, StringRef Banner, StringRef Extra = StringRef()) 
{
+  if (llvm::isFunctionInPrintList("*") || llvm::forcePrintModuleIR()) {
+dbgs() << Banner << Extra << "\n";
+M->print(dbgs(), nullptr, false);
+  } else {
+for (const auto &F : M->functions()) {
+  printIR(&F, Banner, Extra);
+}
+  }
+}
+
 void printIR(const LazyCallGraph::SCC *C, StringRef Banner,
  StringRef Extra = StringRef()) {
   bool BannerPrinted = false;


Index: llvm/tes

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro added a comment.

In D73534#1883022 , @nickdesaulniers 
wrote:

> In D73534#1882136 , @djtodoro wrote:
>
> > - Address the issue with ARM `describeLoadedValue()` (thanks to @vsk, I've 
> > reduced the test 
> > `llvm/test/DebugInfo/MIR/ARM/dbgcallsite-noreg-is-imm-check.mir`)
>
>
> I'd like to help test this, but `arc patch D73534` is now failing with merge 
> conflicts. Can you please rebase this on master?


I’ve already pushed this. Please rebase on the latest commits.


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

https://reviews.llvm.org/D73534



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

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D73534#1883048 , @djtodoro wrote:

> I’ve already pushed this. Please rebase on the latest commits.


Ah faff707db82d7db12fcd9f7826b8741261230e63 
. Cool, I 
can no longer reproduce the previous issue. Thanks for the quick fix.


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

https://reviews.llvm.org/D73534



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

[PATCH] D74813: [RFC] Add hash of block contents to function block names

[Alex Borcan via Phabricator via cfe-commits]

alexbdv added a subscriber: vsk.
alexbdv added a comment.

@vsk  - sure will add tests when removing from RFC.
As for making it default - would rather have this under a flag as hashing the 
block contents does have some overhead and I imagine this feature wouldn't be 
beneficial in most scenarios. Also, unexpectedly (by default) changing the name 
of the function blocks might have a negative impact on some existing workflows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74813



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

[PATCH] D74814: IR printing for single function with the new pass manager.

[Wenlei He via Phabricator via cfe-commits]

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

Thanks for the making the changes, it would be nice to have some consistency 
(the same structure) between test cases for legacy PM and new PM, e.g. `EMPTY` 
is only tested for legacy PM, but not new; multi-function filter also only 
tested for legacy PM, would be nice to do them for both, and unify the 
check-prefix, otherwise LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74814



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

[PATCH] D74814: IR printing for single function with the new pass manager.

[Hongtao Yu via Phabricator via cfe-commits]

hoyFB updated this revision to Diff 245474.
hoyFB added a comment.

Updating D74814 : IR printing for single 
function with the new pass manager.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74814

Files:
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/test/Other/module-pass-printer.ll


Index: llvm/test/Other/module-pass-printer.ll
===
--- llvm/test/Other/module-pass-printer.ll
+++ llvm/test/Other/module-pass-printer.ll
@@ -1,14 +1,28 @@
 ; Check pass name is only printed once.
 ; RUN: opt < %s 2>&1 -forceattrs -disable-output -print-after-all | FileCheck 
%s
 ; RUN: opt < %s 2>&1 -forceattrs -disable-output -print-after-all 
-filter-print-funcs=foo,bar | FileCheck %s
+; RUN: opt < %s 2>&1 -passes=forceattrs -disable-output -print-after-all 
-filter-print-funcs=foo | FileCheck %s  -check-prefix=FOO
 
 ; Check pass name is not printed if a module doesn't include any function 
specified in -filter-print-funcs.
 ; RUN: opt < %s 2>&1 -forceattrs -disable-output -print-after-all 
-filter-print-funcs=baz | FileCheck %s -allow-empty -check-prefix=EMPTY
 
+; Check whole module is printed with user-specified wildcast switch 
-filter-print-funcs=* or -print-module-scope
+; RUN: opt < %s 2>&1 -passes=forceattrs -disable-output -print-after-all | 
FileCheck %s -check-prefix=ALL
+; RUN: opt < %s 2>&1 -passes=forceattrs -disable-output -print-after-all 
-filter-print-funcs=* | FileCheck %s -check-prefix=ALL
+; RUN: opt < %s 2>&1 -passes=forceattrs -disable-output -print-after-all 
-filter-print-funcs=foo -print-module-scope | FileCheck %s -check-prefix=ALL
+
 ; CHECK: *** IR Dump After Force set function attributes ***
 ; CHECK-NOT: *** IR Dump After Force set function attributes ***
 ; EMPTY-NOT: *** IR Dump After Force set function attributes ***
 
+; FOO:  IR Dump After ForceFunctionAttrsPass
+; FOO:  define void @foo
+; FOO-NOT:  define void @bar
+
+; ALL:  IR Dump After ForceFunctionAttrsPass
+; ALL:  define void @foo
+; ALL:  define void @bar
+
 define void @foo() {
   ret void
 }
Index: llvm/lib/Passes/StandardInstrumentations.cpp
===
--- llvm/lib/Passes/StandardInstrumentations.cpp
+++ llvm/lib/Passes/StandardInstrumentations.cpp
@@ -70,16 +70,24 @@
   llvm_unreachable("Unknown IR unit");
 }
 
-void printIR(const Module *M, StringRef Banner, StringRef Extra = StringRef()) 
{
-  dbgs() << Banner << Extra << "\n";
-  M->print(dbgs(), nullptr, false);
-}
 void printIR(const Function *F, StringRef Banner,
  StringRef Extra = StringRef()) {
   if (!llvm::isFunctionInPrintList(F->getName()))
 return;
   dbgs() << Banner << Extra << "\n" << static_cast(*F);
 }
+
+void printIR(const Module *M, StringRef Banner, StringRef Extra = StringRef()) 
{
+  if (llvm::isFunctionInPrintList("*") || llvm::forcePrintModuleIR()) {
+dbgs() << Banner << Extra << "\n";
+M->print(dbgs(), nullptr, false);
+  } else {
+for (const auto &F : M->functions()) {
+  printIR(&F, Banner, Extra);
+}
+  }
+}
+
 void printIR(const LazyCallGraph::SCC *C, StringRef Banner,
  StringRef Extra = StringRef()) {
   bool BannerPrinted = false;


Index: llvm/test/Other/module-pass-printer.ll
===
--- llvm/test/Other/module-pass-printer.ll
+++ llvm/test/Other/module-pass-printer.ll
@@ -1,14 +1,28 @@
 ; Check pass name is only printed once.
 ; RUN: opt < %s 2>&1 -forceattrs -disable-output -print-after-all | FileCheck %s
 ; RUN: opt < %s 2>&1 -forceattrs -disable-output -print-after-all -filter-print-funcs=foo,bar | FileCheck %s
+; RUN: opt < %s 2>&1 -passes=forceattrs -disable-output -print-after-all -filter-print-funcs=foo | FileCheck %s  -check-prefix=FOO
 
 ; Check pass name is not printed if a module doesn't include any function specified in -filter-print-funcs.
 ; RUN: opt < %s 2>&1 -forceattrs -disable-output -print-after-all -filter-print-funcs=baz | FileCheck %s -allow-empty -check-prefix=EMPTY
 
+; Check whole module is printed with user-specified wildcast switch -filter-print-funcs=* or -print-module-scope
+; RUN: opt < %s 2>&1 -passes=forceattrs -disable-output -print-after-all | FileCheck %s -check-prefix=ALL
+; RUN: opt < %s 2>&1 -passes=forceattrs -disable-output -print-after-all -filter-print-funcs=* | FileCheck %s -check-prefix=ALL
+; RUN: opt < %s 2>&1 -passes=forceattrs -disable-output -print-after-all -filter-print-funcs=foo -print-module-scope | FileCheck %s -check-prefix=ALL
+
 ; CHECK: *** IR Dump After Force set function attributes ***
 ; CHECK-NOT: *** IR Dump After Force set function attributes ***
 ; EMPTY-NOT: *** IR Dump After Force set function attributes ***
 
+; FOO:  IR Dump After ForceFunctionAttrsPass
+; FOO:  define void @foo
+; FOO-NOT:  define void 

[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:1514
+  void visitUsedDecl(SourceLocation Loc, Decl *D) {
+if (auto *TD = dyn_cast(D)) {
+  for (auto *DD : TD->decls()) {

erichkeane wrote:
> rjmccall wrote:
> > erichkeane wrote:
> > > Note that when recommitting this (if you choose to), this needs to also 
> > > handle NamespaceDecl.  We're a downstream and discovered that this 
> > > doesn't properly handle functions or records handled in a namespace.
> > > 
> > > It can be implemented identically to TranslationUnitDecl.
> > Wait, what?  We shouldn't be doing this for TranslationUnitDecl either.   I 
> > don't even know how we're "using" a TranslationUnitDecl, but neither this 
> > case not the case for `NamespaceDecl` should be recursively using every 
> > declaration declared inside it.  If there's a declaration in a namespace 
> > that's being used, it should be getting visited as part of the actual use 
> > of it.
> > 
> > The logic for `RecordDecl` has the same problem.  
> Despite the name, this seems to be more of a home-written ast walking class.  
> The entry point is the 'translation unit' which seems to walk through 
> everything in an attempt to find all the functions (including those that are 
> 'marked' as used by an attribute).
> 
> You'll see the FunctionDecl section makes this assumption as well (not 
> necessarily that we got to a function via a call). IMO, this approach is 
> strange, and we should register entry points in some manner (functions marked 
> as emitted to the device in some fashion), then just follow its call-graph 
> (via the clang::CallGraph?) to emit all of these functions.
> 
> It seemed really odd to see this approach here, but it seemed well reviewed 
> by the time I noticed it (via a downstream bug) so I figured I'd lost my 
> chance to disagree with the approach.
> 
> 
Sure, but `visitUsedDecl` isn't the right place to be entering the walk.  
`visitUsedDecl` is supposed to be the *callback* from the walk.  If they need 
to walk all the global declarations to find kernels instead of tracking the 
kernels as they're encountered (which would be a *much* better approach), it 
should be done as a separate function.

I just missed this in the review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70172



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

[PATCH] D69868: Allow "callbr" to return non-void values

[Nick Desaulniers via Phabricator via cfe-commits]

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

This code has now been tested on a running Linux kernel making use of the 
feature.

I still would like @jyknight to clarify his comments, consider explicitly 
requesting changes to this CL, or consider resigning as reviewer.




Comment at: llvm/include/llvm/CodeGen/MachineBasicBlock.h:137
+  /// List of indirect targets of the callbr of a basic block.
+  SmallPtrSet InlineAsmBrIndirectTargets;
+

It's likely the count here is 0, or maybe 1.  We don't see too often a large 
list of labels here.



Comment at: llvm/lib/CodeGen/MachineVerifier.cpp:701
+  } else if (MBB->succ_size() == LandingPadSuccs.size() ||
+ MBB->succ_size() == IndirectPadSuccs.size()) {
 // It's possible that the block legitimately ends with a noreturn

void wrote:
> void wrote:
> > jyknight wrote:
> > > void wrote:
> > > > jyknight wrote:
> > > > > This isn't correct.
> > > > > 
> > > > > This line here, is looking at a block which doesn't end in a jump to 
> > > > > a successor. So, it's trying to verify that the successor list makes 
> > > > > sense in that context.
> > > > > 
> > > > > The unstated assumption in the code is that the only successors will 
> > > > > be landing pads. Instead of actually checking each one, instead it 
> > > > > just checks that the count is the number of landing pads, with the 
> > > > > assumption that all the successors should be landing pads, and that 
> > > > > all the landing pads should be successors.
> > > > > 
> > > > > The next clause is then checking for the case where there's a 
> > > > > fallthrough to the next block. In that case, the successors should've 
> > > > > been all the landing pads, and the single fallthrough block.
> > > > > 
> > > > > Adding similar code to check for the number of callbr targets doesn't 
> > > > > really make sense. It's certainly not the case that all callbr 
> > > > > targets are targets of all 
> > > > > callbr instructions. And even if it was, this still wouldn't be 
> > > > > counting things correctly.
> > > > > 
> > > > > 
> > > > > However -- I think i'd expect analyzeBranch to error out (returning 
> > > > > true) when confronted by a callbr instruction, because it cannot 
> > > > > actually tell what's going on there. If that were the case, nothing 
> > > > > in this block should even be invoked. But I guess that's probably not 
> > > > > happening, due to the terminator being followed by non-terminators.
> > > > > 
> > > > > That seems likely to be a problem that needs to be fixed. (And if 
> > > > > that is fixed, I think the changes here aren't needed anymore)
> > > > > 
> > > > > 
> > > > Your comment is very confusing. Could you please give an example of 
> > > > where this fails?
> > > Sorry about that, I should've delimited the parts of that message 
> > > better...
> > > Basically:
> > > - Paragraphs 2-4 are describing why the code before this patch appears to 
> > > be correct for landing pad, even though it's taking some shortcuts and 
> > > making some non-obvious assumptions.
> > > - Paragraph 5 ("Adding similar code"...) is why it's not correct for 
> > > callbr.
> > > - Paragraph 6-7 are how I'd suggest to resolve it.
> > > 
> > > 
> > > I believe the code as of your patch will fail validation if you have a 
> > > callbr instruction which has a normal-successor block which is an 
> > > indirect target of a *different* callbr in the function.
> > > 
> > > I believe it'll also fail if you have any landing-pad successors, since 
> > > those aren't being added to the count of expected successors, but rather 
> > > checked separately.
> > > 
> > > But more seriously than these potential verifier failures, I expect that 
> > > analyzeBranch returning wrong answers (in that it may report that a block 
> > > unconditionally-jumps to a successor, while it really has both a callbr 
> > > and jump, separated by the non-terminator copies) will cause 
> > > miscompilation. I'm not sure exactly how that will exhibit, but I'm 
> > > pretty sure it's not going to be good.
> > > 
> > > And, if analyzeBranch properly said "no idea" when confronted by callbr 
> > > control flow, then this code in the verifier wouldn't be reached.
> > I didn't need a delineation of the parts of the comment. I needed a clearer 
> > description of what your concern is, and to give an example of code that 
> > fails here.
> > 
> > This bit of code is simply saying that if the block containing the 
> > `INLINEASM_BR` doesn't end with a `BR` instruction, then the number of its 
> > successors should be equal to the number of indirect successors. This is 
> > correct, as it's not valid to have a duplicate label used in a `callbr` 
> > instruction:
> > 
> > ```
> > $ llc -o /dev/null x.ll
> > Duplicate callbr destination!
> >   %3 = callbr i32 asm sideeffect "testl $0, $

[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:1514
+  void visitUsedDecl(SourceLocation Loc, Decl *D) {
+if (auto *TD = dyn_cast(D)) {
+  for (auto *DD : TD->decls()) {

rjmccall wrote:
> erichkeane wrote:
> > Note that when recommitting this (if you choose to), this needs to also 
> > handle NamespaceDecl.  We're a downstream and discovered that this doesn't 
> > properly handle functions or records handled in a namespace.
> > 
> > It can be implemented identically to TranslationUnitDecl.
> Wait, what?  We shouldn't be doing this for TranslationUnitDecl either.   I 
> don't even know how we're "using" a TranslationUnitDecl, but neither this 
> case not the case for `NamespaceDecl` should be recursively using every 
> declaration declared inside it.  If there's a declaration in a namespace 
> that's being used, it should be getting visited as part of the actual use of 
> it.
> 
> The logic for `RecordDecl` has the same problem.  
Despite the name, this seems to be more of a home-written ast walking class.  
The entry point is the 'translation unit' which seems to walk through 
everything in an attempt to find all the functions (including those that are 
'marked' as used by an attribute).

You'll see the FunctionDecl section makes this assumption as well (not 
necessarily that we got to a function via a call). IMO, this approach is 
strange, and we should register entry points in some manner (functions marked 
as emitted to the device in some fashion), then just follow its call-graph (via 
the clang::CallGraph?) to emit all of these functions.

It seemed really odd to see this approach here, but it seemed well reviewed by 
the time I noticed it (via a downstream bug) so I figured I'd lost my chance to 
disagree with the approach.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70172



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

[PATCH] D71227: [cuda][hip] Fix function overload resolution in the global initiailizer.

[Michael Liao via Phabricator via cfe-commits]

hliao updated this revision to Diff 245461.
hliao added a comment.

Rebase to the latest trunk code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71227

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCUDA/function-overload.cu
  clang/test/SemaCUDA/global-initializers-host.cu
  clang/test/SemaCUDA/hip-pinned-shadow.cu

Index: clang/test/SemaCUDA/hip-pinned-shadow.cu
===
--- clang/test/SemaCUDA/hip-pinned-shadow.cu
+++ clang/test/SemaCUDA/hip-pinned-shadow.cu
@@ -13,13 +13,19 @@
 
 template 
 struct texture : public textureReference {
+// expected-note@-1{{candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 0 were provided}}
+// expected-note@-2{{candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 0 were provided}}
+// expected-note@-3{{candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 0 were provided}}
+// expected-note@-4{{candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 0 were provided}}
 texture() { a = 1; }
+// expected-note@-1{{candidate constructor not viable: call to __host__ function from __device__ function}}
+// expected-note@-2{{candidate constructor not viable: call to __host__ function from __device__ function}}
 };
 
 __hip_pinned_shadow__ texture tex;
 __device__ __hip_pinned_shadow__ texture tex2; // expected-error{{'hip_pinned_shadow' and 'device' attributes are not compatible}}
-// expected-error@-1{{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables}}
-// expected-note@-2{{conflicting attribute is here}}
+// expected-note@-1{{conflicting attribute is here}}
+// expected-error@-2{{no matching constructor for initialization of 'texture'}}
 __constant__ __hip_pinned_shadow__ texture tex3; // expected-error{{'hip_pinned_shadow' and 'constant' attributes are not compatible}}
-  // expected-error@-1{{dynamic initialization is not supported for __device__, __constant__, and __shared__ variables}}
-  // expected-note@-2{{conflicting attribute is here}}
+  // expected-note@-1{{conflicting attribute is here}}
+  // expected-error@-2{{no matching constructor for initialization of 'texture'}}
Index: clang/test/SemaCUDA/global-initializers-host.cu
===
--- clang/test/SemaCUDA/global-initializers-host.cu
+++ clang/test/SemaCUDA/global-initializers-host.cu
@@ -6,12 +6,14 @@
 // module initializer.
 
 struct S {
+  // expected-note@-1 {{candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 0 were provided}}
+  // expected-note@-2 {{candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 0 were provided}}
   __device__ S() {}
-  // expected-note@-1 {{'S' declared here}}
+  // expected-note@-1 {{candidate constructor not viable: call to __device__ function from __host__ function}}
 };
 
 S s;
-// expected-error@-1 {{reference to __device__ function 'S' in global initializer}}
+// expected-error@-1 {{no matching constructor for initialization of 'S'}}
 
 struct T {
   __host__ __device__ T() {}
@@ -19,14 +21,17 @@
 T t;  // No error, this is OK.
 
 struct U {
+  // expected-note@-1 {{candidate constructor (the implicit copy constructor) not viable: no known conversion from 'int' to 'const U' for 1st argument}}
+  // expected-note@-2 {{candidate constructor (the implicit move constructor) not viable: no known conversion from 'int' to 'U' for 1st argument}}
   __host__ U() {}
+  // expected-note@-1 {{candidate constructor not viable: requires 0 arguments, but 1 was provided}}
   __device__ U(int) {}
-  // expected-note@-1 {{'U' declared here}}
+  // expected-note@-1 {{candidate constructor not viable: call to __device__ function from __host__ function}}
 };
 U u(42);
-// expected-error@-1 {{reference to __device__ function 'U' in global initializer}}
+// expected-error@-1 {{no matching constructor for initialization of 'U'}}
 
 __device__ int device_fn() { return 42; }
-// expected-note@-1 {{'device_fn' declared here}}
+// expected-note@-1 {{candidate function not viable: call to __devi

[PATCH] D74811: [Driver] Escape the program path for -frecord-command-line

[Ravi Ramaseshan via Phabricator via cfe-commits]

ravi-ramaseshan added a comment.

In D74811#1882783 , @scott.linder 
wrote:

> This LGTM, but could you add a simple test with a command-line that needs the 
> escaping?


Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74811



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

[PATCH] D74811: [Driver] Escape the program path for -frecord-command-line

[Ravi Ramaseshan via Phabricator via cfe-commits]

ravi-ramaseshan updated this revision to Diff 245456.
ravi-ramaseshan added a comment.

Add a simple test with a command-line that needs the escaping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74811

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/clang_f_opts.c


Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -581,6 +581,11 @@
 // CHECK-RECORD-GCC-SWITCHES: "-record-command-line"
 // CHECK-NO-RECORD-GCC-SWITCHES-NOT: "-record-command-line"
 // CHECK-RECORD-GCC-SWITCHES-ERROR: error: unsupported option 
'-frecord-command-line' for target
+// Test when clang is in a path containing a space.
+// RUN: mkdir -p "%t.r/with spaces"
+// RUN: cp %clang "%t.r/with spaces/clang"
+// RUN: "%t.r/with spaces/clang" -### -S -target x86_64-unknown-linux 
-frecord-gcc-switches %s 2>&1 | FileCheck 
-check-prefix=CHECK-RECORD-GCC-SWITCHES-ESCAPED %s
+// CHECK-RECORD-GCC-SWITCHES-ESCAPED: "-record-command-line" "{{.+}}/with\\ 
spaces/clang {{.+}}"
 
 // RUN: %clang -### -S -ftrivial-auto-var-init=uninitialized %s 2>&1 | 
FileCheck -check-prefix=CHECK-TRIVIAL-UNINIT %s
 // RUN: %clang -### -S -ftrivial-auto-var-init=pattern %s 2>&1 | FileCheck 
-check-prefix=CHECK-TRIVIAL-PATTERN %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5880,7 +5880,7 @@
   Arg->render(Args, OriginalArgs);
 
 SmallString<256> Flags;
-Flags += Exec;
+EscapeSpacesAndBackslashes(Exec, Flags);
 for (const char *OriginalArg : OriginalArgs) {
   SmallString<128> EscapedArg;
   EscapeSpacesAndBackslashes(OriginalArg, EscapedArg);
@@ -6788,7 +6788,7 @@
 
 SmallString<256> Flags;
 const char *Exec = getToolChain().getDriver().getClangProgramPath();
-Flags += Exec;
+EscapeSpacesAndBackslashes(Exec, Flags);
 for (const char *OriginalArg : OriginalArgs) {
   SmallString<128> EscapedArg;
   EscapeSpacesAndBackslashes(OriginalArg, EscapedArg);


Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -581,6 +581,11 @@
 // CHECK-RECORD-GCC-SWITCHES: "-record-command-line"
 // CHECK-NO-RECORD-GCC-SWITCHES-NOT: "-record-command-line"
 // CHECK-RECORD-GCC-SWITCHES-ERROR: error: unsupported option '-frecord-command-line' for target
+// Test when clang is in a path containing a space.
+// RUN: mkdir -p "%t.r/with spaces"
+// RUN: cp %clang "%t.r/with spaces/clang"
+// RUN: "%t.r/with spaces/clang" -### -S -target x86_64-unknown-linux -frecord-gcc-switches %s 2>&1 | FileCheck -check-prefix=CHECK-RECORD-GCC-SWITCHES-ESCAPED %s
+// CHECK-RECORD-GCC-SWITCHES-ESCAPED: "-record-command-line" "{{.+}}/with\\ spaces/clang {{.+}}"
 
 // RUN: %clang -### -S -ftrivial-auto-var-init=uninitialized %s 2>&1 | FileCheck -check-prefix=CHECK-TRIVIAL-UNINIT %s
 // RUN: %clang -### -S -ftrivial-auto-var-init=pattern %s 2>&1 | FileCheck -check-prefix=CHECK-TRIVIAL-PATTERN %s
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5880,7 +5880,7 @@
   Arg->render(Args, OriginalArgs);
 
 SmallString<256> Flags;
-Flags += Exec;
+EscapeSpacesAndBackslashes(Exec, Flags);
 for (const char *OriginalArg : OriginalArgs) {
   SmallString<128> EscapedArg;
   EscapeSpacesAndBackslashes(OriginalArg, EscapedArg);
@@ -6788,7 +6788,7 @@
 
 SmallString<256> Flags;
 const char *Exec = getToolChain().getDriver().getClangProgramPath();
-Flags += Exec;
+EscapeSpacesAndBackslashes(Exec, Flags);
 for (const char *OriginalArg : OriginalArgs) {
   SmallString<128> EscapedArg;
   EscapeSpacesAndBackslashes(OriginalArg, EscapedArg);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/Sema.cpp:1514
+  void visitUsedDecl(SourceLocation Loc, Decl *D) {
+if (auto *TD = dyn_cast(D)) {
+  for (auto *DD : TD->decls()) {

erichkeane wrote:
> Note that when recommitting this (if you choose to), this needs to also 
> handle NamespaceDecl.  We're a downstream and discovered that this doesn't 
> properly handle functions or records handled in a namespace.
> 
> It can be implemented identically to TranslationUnitDecl.
Wait, what?  We shouldn't be doing this for TranslationUnitDecl either.   I 
don't even know how we're "using" a TranslationUnitDecl, but neither this case 
not the case for `NamespaceDecl` should be recursively using every declaration 
declared inside it.  If there's a declaration in a namespace that's being used, 
it should be getting visited as part of the actual use of it.

The logic for `RecordDecl` has the same problem.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70172



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

[PATCH] D74814: IR printing for single function with the new pass manager.

[Wenlei He via Phabricator via cfe-commits]

wenlei added a comment.



> Sounds good. I was trying to figure out how to invoke the new pass manager 
> with OPT. Is there a command line switch to do that?

If you just do `opt -passes=`, it will invoke new pass manager for 
`opt`. See https://reviews.llvm.org/D66560 for example - that was for legacy 
PM, the equivalent for new PM would be something like `opt < %s 2>&1 
-passes=forceattrs ...`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74814



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

[PATCH] D74850: [clangd] Migrate Lexer usages in TypeHierarchy to TokenBuffers

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay, 
ilya-biryukov.
Herald added a project: clang.

Also fixes a bug, resulting from directly using ND.getEndLoc() for end
location of the range. As ND.getEndLoc() points to the begining of the last
token, whereas it should point one past the end, since LSP ranges are half open
(exclusive on the end).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74850

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/test/type-hierarchy.test

Index: clang-tools-extra/clangd/test/type-hierarchy.test
===
--- clang-tools-extra/clangd/test/type-hierarchy.test
+++ clang-tools-extra/clangd/test/type-hierarchy.test
@@ -48,7 +48,7 @@
 # CHECK-NEXT:"parents": [],
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
-# CHECK-NEXT:"character": 15,
+# CHECK-NEXT:"character": 16,
 # CHECK-NEXT:"line": 0
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "start": {
@@ -71,7 +71,7 @@
 # CHECK-NEXT:],
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
-# CHECK-NEXT:"character": 24,
+# CHECK-NEXT:"character": 25,
 # CHECK-NEXT:"line": 1
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "start": {
@@ -94,7 +94,7 @@
 # CHECK-NEXT:],
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
-# CHECK-NEXT:"character": 24,
+# CHECK-NEXT:"character": 25,
 # CHECK-NEXT:"line": 2
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "start": {
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -36,6 +36,7 @@
 #include "clang/Index/IndexingAction.h"
 #include "clang/Index/IndexingOptions.h"
 #include "clang/Index/USRGeneration.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/STLExtras.h"
@@ -592,22 +593,25 @@
 
 // FIXME(nridge): Reduce duplication between this function and declToSym().
 static llvm::Optional
-declToTypeHierarchyItem(ASTContext &Ctx, const NamedDecl &ND) {
+declToTypeHierarchyItem(ASTContext &Ctx, const NamedDecl &ND,
+const syntax::TokenBuffer &TB) {
   auto &SM = Ctx.getSourceManager();
-
   SourceLocation NameLoc = nameLocation(ND, Ctx.getSourceManager());
-  // getFileLoc is a good choice for us, but we also need to make sure
-  // sourceLocToPosition won't switch files, so we call getSpellingLoc on top of
-  // that to make sure it does not switch files.
-  // FIXME: sourceLocToPosition should not switch files!
-  SourceLocation BeginLoc = SM.getSpellingLoc(SM.getFileLoc(ND.getBeginLoc()));
-  SourceLocation EndLoc = SM.getSpellingLoc(SM.getFileLoc(ND.getEndLoc()));
-  if (NameLoc.isInvalid() || BeginLoc.isInvalid() || EndLoc.isInvalid())
-return llvm::None;
+  auto FilePath =
+  getCanonicalPath(SM.getFileEntryForID(SM.getFileID(NameLoc)), SM);
+  auto TUPath = getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
+  if (!FilePath || !TUPath)
+return llvm::None; // Not useful without a uri.
 
-  Position NameBegin = sourceLocToPosition(SM, NameLoc);
-  Position NameEnd = sourceLocToPosition(
-  SM, Lexer::getLocForEndOfToken(NameLoc, 0, SM, Ctx.getLangOpts()));
+  auto Tokens = TB.spelledForExpanded(TB.expandedTokens(ND.getSourceRange()));
+  if (!Tokens || Tokens->empty())
+return llvm::None;
+  const auto *NameTok =
+  llvm::partition_point(*Tokens, [&SM, &NameLoc](const syntax::Token &T) {
+return SM.isBeforeInTranslationUnit(T.location(), NameLoc);
+  });
+  if (!NameTok)
+return llvm::None;
 
   index::SymbolInfo SymInfo = index::getSymbolInfo(&ND);
   // FIXME: this is not classifying constructors, destructors and operators
@@ -618,20 +622,16 @@
   THI.name = printName(Ctx, ND);
   THI.kind = SK;
   THI.deprecated = ND.isDeprecated();
-  THI.range =
-  Range{sourceLocToPosition(SM, BeginLoc), sourceLocToPosition(SM, EndLoc)};
-  THI.selectionRange = Range{NameBegin, NameEnd};
+  THI.range = halfOpenToRange(
+  SM, syntax::Token::range(SM, Tokens->front(), Tokens->back())
+  .toCharRange(SM));
+  THI.selectionRange = halfOpenToRange(SM, NameTok->range(SM).toCharRange(SM));
   if (!THI.range.contains(THI.selectionRange)) {
 // 'selectionRange' must be contained in 'range', so in cases where clang
 // reports unrelated ranges we need to reconcile somehow.
 THI.range = THI.selectionRange;
   }
 
-  auto FilePath =
-  getCanonicalPath(SM.getFileEntryForID(SM.getFileID(BeginLoc)), SM);
-  auto TUPath = getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
-  if (!FilePath || !TUPath)
- 

[PATCH] D73534: [DebugInfo] Enable the debug entry values feature by default

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D73534#1882136 , @djtodoro wrote:

> - Address the issue with ARM `describeLoadedValue()` (thanks to @vsk, I've 
> reduced the test 
> `llvm/test/DebugInfo/MIR/ARM/dbgcallsite-noreg-is-imm-check.mir`)


I'd like to help test this, but `arc patch D73534` is now failing with merge 
conflicts. Can you please rebase this on master?


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

https://reviews.llvm.org/D73534



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

[PATCH] D74847: [CodeGen][RISCV] Fix clang/test/CodeGen/atomic_ops.c for RISC-V

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

On a side-note, we could enhance the test runner to support "XFAIL: 
riscv32-default-target" if we thought it would be useful.  But again, I'm not a 
fan of tests that depend on the default target in the first place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74847



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

[PATCH] D74847: [CodeGen][RISCV] Fix clang/test/CodeGen/atomic_ops.c for RISC-V

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I'm not really a big fan of running tests with the host target triple, anyway; 
it seems to create work with almost no benefit.  I'd be happy to just run the 
test with one target that has native atomics, and one target that doesn't.  
(The relevant code is all target-independent, aside from the atomic widths, so 
we aren't really gaining anything by testing more targets.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74847



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

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

In D74784#1882974 , @steven_wu wrote:

> I forgot if there is reason to use the option by default at all time (I did 
> ask that in the previous review but Alex might have given more context 
> offline).


I would really like to hear from @arphaman, but they haven't responded to 
anything since I filed PR44813 nearly two weeks ago. Do you have any means to 
contact them?

> You should definitely add test for this change. The fact that you change all 
> `-mlinker-version=400` to `-mlinker-version=0` but not change any CHECK lines 
> means the change is definitely not tested :)

Can you elaborate on what you would like to see tested? I thought I _was_ 
updating the tests to match the code. The `-mlinker-version=0` lines check that 
we now use the old behavior by default, and the existing `-mlinker-version=520` 
lines in e.g. `darwin-ld-platform-version-macos.c` test that we still use the 
new behavior when requested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74784



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

[PATCH] D74784: [driver][darwin] Don't use -platform_version flag by default

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

I forgot if there is reason to use the option by default at all time (I did ask 
that in the previous review but Alex might have given more context offline).

You should definitely add test for this change. The fact that you change all 
`-mlinker-version=400` to `-mlinker-version=0` but not change any CHECK lines 
means the change is definitely not tested :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74784



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

[PATCH] D74847: [CodeGen][RISCV] Fix clang/test/CodeGen/atomic_ops.c for RISC-V

[Luís Marques via Phabricator via cfe-commits]

luismarques created this revision.
luismarques added reviewers: jyknight, eli.friedman, lenary.
Herald added subscribers: cfe-commits, evandro, sameer.abuasal, s.egerton, Jim, 
benna, psnobl, PkmX, jfb, rkruppe, rogfer01, shiva0217, kito-cheng, simoncook.
Herald added a project: clang.

By default the RISC-V target doesn't have the atomics standard extension 
enabled. The first RUN line in `clang/test/CodeGen/atomic_ops.c` doesn't 
specify a target triple, which means that on RISC-V Linux hosts it will target 
RISC-V, but because we use clang cc1 we don't get the toolchain driver 
functionality to automatically turn on the extensions implied by the target 
triple (riscv64-linux includes atomics). This causes the test to fail on RISC-V 
hosts.

I waffled a bit regarding the best way to fix this. In the end I decided to 1) 
use XFAIL to blacklist RISC-V hosts and 2) add explicitly run lines for 
riscv32/64, with the atomics extension enabled. With that choice we 
//effectively// don't do the test on RISC-V hosts, but still get to test those 
two RISC-V targets on other hosts.

An alternative approach would be to eliminate the first RUN line and explicitly 
list all of the target triples we want to test. But looking at the other clang 
tests, there doesn't seem to be any test that comprehensively lists all of the 
targets, and it wasn't quite clear which should be listed on this test. Running 
clang with the non-cc1 interface would also be an option, but in principle that 
could still fail in some hypothetical scenarios, and the tests seem to prefer 
to use cc1.

(It's a shame that XFAIL necessarily means blacklisting host triples, and not 
target triples. That doesn't interact very well with cross-compilers.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74847

Files:
  clang/test/CodeGen/atomic_ops.c


Index: clang/test/CodeGen/atomic_ops.c
===
--- clang/test/CodeGen/atomic_ops.c
+++ clang/test/CodeGen/atomic_ops.c
@@ -1,7 +1,13 @@
 // XFAIL: hexagon,sparc
 //(due to not having native load atomic support)
+// XFAIL: riscv
+//(due to requiring an explicit -target-feature +a)
 // RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -triple mips-linux-gnu -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple riscv64 -target-feature +a -emit-llvm %s \
+// RUN:   -o - | FileCheck %s
+// RUN: %clang_cc1 -triple riscv32 -target-feature +a -emit-llvm %s \
+// RUN:   -o - | FileCheck %s
 
 void foo(int x)
 {


Index: clang/test/CodeGen/atomic_ops.c
===
--- clang/test/CodeGen/atomic_ops.c
+++ clang/test/CodeGen/atomic_ops.c
@@ -1,7 +1,13 @@
 // XFAIL: hexagon,sparc
 //(due to not having native load atomic support)
+// XFAIL: riscv
+//(due to requiring an explicit -target-feature +a)
 // RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -triple mips-linux-gnu -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple riscv64 -target-feature +a -emit-llvm %s \
+// RUN:   -o - | FileCheck %s
+// RUN: %clang_cc1 -triple riscv32 -target-feature +a -emit-llvm %s \
+// RUN:   -o - | FileCheck %s
 
 void foo(int x)
 {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D74845: [ARM,MVE] Add vqdmull[b,t]q intrinsic families

[Mark Murray via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74845



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

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

[Sean Fertile via Phabricator via cfe-commits]

sfertile added a comment.

Patch LGTM as far a formatting/naming/testing etc. C++ specifics is outside my 
wheelhouse though, so I can't confirm things like the tail padding rules are 
correct for AIX. Because of that I'm not comfortable being the one to accept 
the patch.


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

https://reviews.llvm.org/D74015



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

[PATCH] D74844: [clangd] Get rid of Lexer usage in CodeComplete

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:593
+  llvm::StringRef SpelledSpecifier =
+  syntax::FileRange(CCSema.SourceMgr, SemaSpecifier->getBeginLoc(),
+SemaSpecifier->getEndLoc())

what if beginLoc and endLoc are in different files?
(this is most of what Lexer deals with, I think)

I think you want TokenBuffer::expandedTokens(SourceRange), then 
spelledForExpanded, and then FileRange(...).text().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74844



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

[PATCH] D74846: fix -fcodegen-modules code when used with PCH (PR44958)

[Luboš Luňák via Phabricator via cfe-commits]

llunak created this revision.
llunak added reviewers: hans, dblaikie.
llunak added a project: clang.
Herald added a subscriber: cfe-commits.

In D69778  I incorrectly handled two cases 
(checking for -building-pch-with-obj without also checking -fmodules-codegen).


Repository:
  rC Clang

https://reviews.llvm.org/D74846

Files:
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/PCH/Inputs/declspec-selectany-pch.cpp
  clang/test/PCH/Inputs/declspec-selectany-pch.h
  clang/test/PCH/declspec-selectany.cpp


Index: clang/test/PCH/declspec-selectany.cpp
===
--- /dev/null
+++ clang/test/PCH/declspec-selectany.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cl /Yc%S/Inputs/declspec-selectany-pch.h 
%S/Inputs/declspec-selectany-pch.cpp /c /Fo%t.obj /Fp%t.pch
+// RUN: %clang_cl /Yu%S/Inputs/declspec-selectany-pch.h %s /I%S/Inputs /c 
/Fo%t.obj /Fp%t.pch
+
+#include "declspec-selectany-pch.h"
+
+int main() {
+  CD3DX12_CPU_DESCRIPTOR_HANDLE desc = 
CD3DX12_CPU_DESCRIPTOR_HANDLE(D3D12_DEFAULT);
+  return desc.ptr;
+}
Index: clang/test/PCH/Inputs/declspec-selectany-pch.h
===
--- /dev/null
+++ clang/test/PCH/Inputs/declspec-selectany-pch.h
@@ -0,0 +1,7 @@
+struct CD3DX12_DEFAULT {};
+extern const __declspec(selectany) CD3DX12_DEFAULT D3D12_DEFAULT;
+
+struct CD3DX12_CPU_DESCRIPTOR_HANDLE {
+  CD3DX12_CPU_DESCRIPTOR_HANDLE(CD3DX12_DEFAULT) { ptr = 0; }
+  size_t ptr;
+};
Index: clang/test/PCH/Inputs/declspec-selectany-pch.cpp
===
--- /dev/null
+++ clang/test/PCH/Inputs/declspec-selectany-pch.cpp
@@ -0,0 +1 @@
+#include "declspec-selectany-pch.h"
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1017,10 +1017,12 @@
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline variables are
   // still emitted in module users.)
+  // Similarly if a PCH is built with codegen and its own object file.
   ModulesCodegen =
   (((Writer.WritingModule &&
  Writer.WritingModule->Kind == Module::ModuleInterfaceUnit) ||
-Writer.Context->getLangOpts().BuildingPCHWithObjectFile) &&
+(Writer.Context->getLangOpts().BuildingPCHWithObjectFile &&
+ Writer.Context->getLangOpts().ModulesCodegen)) &&
Writer.Context->GetGVALinkageForVariable(D) == GVA_StrongExternal);
 }
 Record.push_back(ModulesCodegen);
@@ -2451,9 +2453,8 @@
   bool ModulesCodegen = false;
   if (!FD->isDependentContext()) {
 Optional Linkage;
-if ((Writer->WritingModule &&
- Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) ||
-Writer->Context->getLangOpts().BuildingPCHWithObjectFile) {
+if (Writer->WritingModule &&
+Writer->WritingModule->Kind == Module::ModuleInterfaceUnit) {
   // When building a C++ Modules TS module interface unit, a strong
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline functions are


Index: clang/test/PCH/declspec-selectany.cpp
===
--- /dev/null
+++ clang/test/PCH/declspec-selectany.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cl /Yc%S/Inputs/declspec-selectany-pch.h %S/Inputs/declspec-selectany-pch.cpp /c /Fo%t.obj /Fp%t.pch
+// RUN: %clang_cl /Yu%S/Inputs/declspec-selectany-pch.h %s /I%S/Inputs /c /Fo%t.obj /Fp%t.pch
+
+#include "declspec-selectany-pch.h"
+
+int main() {
+  CD3DX12_CPU_DESCRIPTOR_HANDLE desc = CD3DX12_CPU_DESCRIPTOR_HANDLE(D3D12_DEFAULT);
+  return desc.ptr;
+}
Index: clang/test/PCH/Inputs/declspec-selectany-pch.h
===
--- /dev/null
+++ clang/test/PCH/Inputs/declspec-selectany-pch.h
@@ -0,0 +1,7 @@
+struct CD3DX12_DEFAULT {};
+extern const __declspec(selectany) CD3DX12_DEFAULT D3D12_DEFAULT;
+
+struct CD3DX12_CPU_DESCRIPTOR_HANDLE {
+  CD3DX12_CPU_DESCRIPTOR_HANDLE(CD3DX12_DEFAULT) { ptr = 0; }
+  size_t ptr;
+};
Index: clang/test/PCH/Inputs/declspec-selectany-pch.cpp
===
--- /dev/null
+++ clang/test/PCH/Inputs/declspec-selectany-pch.cpp
@@ -0,0 +1 @@
+#include "declspec-selectany-pch.h"
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1017,10 +1017,12 @@
   // definition in the module interface is provided by the compilation of
   // that module interface unit, not by its users. (Inline variables a

[PATCH] D68578: [HIP] Fix device stub name

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/include/clang/AST/GlobalDecl.h:40
+  Stub = 1,
+};
+

rjmccall wrote:
> tra wrote:
> > rjmccall wrote:
> > > tra wrote:
> > > > rjmccall wrote:
> > > > > The attribute here is `CUDAGlobalAttr`; should this be named in terms 
> > > > > of CUDA, or is the CUDA model sufficiently different  from HIP that 
> > > > > the same implementation concept doesn't apply?
> > > > I believe the attribute serves the same purpose in both CUDA and HIP 
> > > > and could be renamed appropriately in a separate patch.
> > > > 
> > > > While the changes in this patch are not required for CUDA, CUDA would 
> > > > benefit from them. We could use a generic GPU prefix and migrate CUDA 
> > > > to the same model later. A TODO comment about that would be nice. 
> > > I'd just like consistency.  If they're serving the same purpose, then as 
> > > someone with no dog in this fight, I would give precedence to CUDA over 
> > > HIP in names since it's both the older language and was implemented first 
> > > in Clang (even if only partially, IIUC).  I don't think a generic name 
> > > works unless we can meaningfully generalize it to all languages with a 
> > > similar feature, e.g. OpenCL and so on.
> > Naming, the hardest problem in computer science. :-)
> > I personally would prefer generalization-with-exclusions over specific name 
> > which is inconsistently commingles things that's really specific to that 
> > name and things that are more widely used. Alas, right now CUDA is the 
> > example of the latter case -- some parts are CUDA-specific and a lot are 
> > shared with HIP.
> > 
> > For the new features we've been sort of sticking with using CUDA/HIP for 
> > specific parts and GPU for shared functionality, but as things are a lot of 
> > shared bits are still 'CUDA' and it's hard to tell them apart. As you point 
> > it out, renaming the incumbent names would be a pain, so here we are.
> > 
> > I think using `GPUKernelKind` with a comment that it reflects HIP & CUDA 
> > kernels would be somewhat better choice than `CUDAKernelKind` which would 
> > be somewhat confusing at this point given that CUDA actually does not use 
> > it yet. I'm also fine with keeping it `HIPKernelKind` and postpone the 
> > naming decision until CUDA-related parts are actually implemented.
> Maybe `KernelReferenceKind`?  It's probably a common concept across all 
> heterogenous-computing models.
SGTM.


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

https://reviews.llvm.org/D68578



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