[PATCH] D148981: [clang][Interp] PointerToBoolean casts

[Timm Bäder via Phabricator via cfe-commits]

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

Just emit `p != nullptr` for this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148981

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/test/AST/Interp/literals.cpp


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -106,6 +106,19 @@
 }
 static_assert(gimme(5) == 5, "");
 
+namespace PointerToBool {
+
+  constexpr void *N = nullptr;
+  constexpr bool B = N;
+  static_assert(!B, "");
+  static_assert(!N, "");
+
+  constexpr float F = 1.0;
+  constexpr const float *FP = 
+  static_assert(FP, "");
+  static_assert(!!FP, "");
+}
+
 namespace SizeOf {
   constexpr int soint = sizeof(int);
   constexpr int souint = sizeof(unsigned int);
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -204,6 +204,17 @@
 return this->emitCast(*FromT, *ToT, CE);
   }
 
+  case CK_PointerToBoolean: {
+// Just emit p != nullptr for this.
+if (!this->visit(SubExpr))
+  return false;
+
+if (!this->emitNullPtr(CE))
+  return false;
+
+return this->emitNEPtr(CE);
+  }
+
   case CK_ToVoid:
 return discard(SubExpr);
 


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -106,6 +106,19 @@
 }
 static_assert(gimme(5) == 5, "");
 
+namespace PointerToBool {
+
+  constexpr void *N = nullptr;
+  constexpr bool B = N;
+  static_assert(!B, "");
+  static_assert(!N, "");
+
+  constexpr float F = 1.0;
+  constexpr const float *FP = 
+  static_assert(FP, "");
+  static_assert(!!FP, "");
+}
+
 namespace SizeOf {
   constexpr int soint = sizeof(int);
   constexpr int souint = sizeof(unsigned int);
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -204,6 +204,17 @@
 return this->emitCast(*FromT, *ToT, CE);
   }
 
+  case CK_PointerToBoolean: {
+// Just emit p != nullptr for this.
+if (!this->visit(SubExpr))
+  return false;
+
+if (!this->emitNullPtr(CE))
+  return false;
+
+return this->emitNEPtr(CE);
+  }
+
   case CK_ToVoid:
 return discard(SubExpr);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added inline comments.
Herald added a subscriber: bviyer.



Comment at: 
mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp:1533
+ompLoc, builder.saveIP(), mapTypeFlags, mapNames, mapperAllocas,
+mapperFunc, deviceID, ifCond, processMapOpCB, bodyCB));
+  } else {

mapperFunc is used uninitialized here which is UB, can you look into this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142914

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

[PATCH] D147844: [clang][Sema]Print diagnostic warning when implicit cast from int to bool happens in an conditional operator expression

[NagaChaitanya Vellanki via Phabricator via cfe-commits]

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

Revise commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

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

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

[PATCH] D147844: Emit warning when implicit cast from int to bool happens in an conditional operator expression

[NagaChaitanya Vellanki via Phabricator via cfe-commits]

chaitanyav updated this revision to Diff 515974.
chaitanyav marked an inline comment as done.
chaitanyav added a comment.

Modify the commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

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

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

[PATCH] D148975: -fdebug-prefix-map=: make the last win when multiple prefixes match

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

For me long matching prefix makes more sense, but if the same prefix is used 
multiple times, the last option should win.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148975

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

[PATCH] D148757: [clang] Follow user-provided order for prefix map

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D148757#4289076 , @MaskRay wrote:

> I'm creating a patch to change DebugPrefixMap to respect the command line 
> option order...

Created D148975  to follow GCC's behavior 
(the last applicable option wins). You may drop the `DebugPrefixMap` 
(clang/include/clang/Basic/CodeGenOptions.h) change (a no-op without other 
changes to `DebugPrefixMap` ).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148757

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

[PATCH] D148975: -fdebug-prefix-map=: make the last win when multiple prefixes match

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: debug-info, dankm, gulfem, phosek.
Herald added subscribers: hiraditya, emaste.
Herald added a project: All.
MaskRay requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: llvm-commits, cfe-commits, jplehr, sstefan1.
Herald added projects: clang, LLVM.

For
`clang -c -g -fdebug-prefix-map=a/b=y -fdebug-prefix-map=a=x a/b/c.c`,
we apply the longest prefix substitution, but
GCC has always been picking the last applicable option (`a=x`, see
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109591).

I feel that GCC's behavior is reasonable given the convention that the last
value wins for the same option.

Before D49466 , Clang appeared to apply the 
shortest prefix substitution,
which likely made the least sense.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148975

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/debug-prefix-map.c
  clang/tools/driver/cc1as_main.cpp
  llvm/include/llvm/MC/MCContext.h
  llvm/lib/MC/MCContext.cpp
  llvm/test/MC/ELF/debug-prefix-map.s

Index: llvm/test/MC/ELF/debug-prefix-map.s
===
--- llvm/test/MC/ELF/debug-prefix-map.s
+++ llvm/test/MC/ELF/debug-prefix-map.s
@@ -14,9 +14,13 @@
 # RUN: cd %t.foo/bar
 # RUN: llvm-mc -triple=x86_64 -g -dwarf-version=4 %t.foo%{fs-sep}bar%{fs-sep}src.s -filetype=obj -o mapabs.4.o -fdebug-prefix-map=%t.foo=/broken_root -fdebug-prefix-map=%t.foo%{fs-sep}bar=/src_root
 # RUN: llvm-dwarfdump -v -debug-info -debug-line mapabs.4.o | FileCheck --check-prefix=MAPABS_V4 %s
-# RUN: llvm-mc -triple=x86_64 -g -dwarf-version=5 %t.foo%{fs-sep}bar%{fs-sep}src.s -filetype=obj -o mapabs.5.o -fdebug-prefix-map=%t.foo%{fs-sep}bar=/src_root -fdebug-prefix-map=%t.foo=/broken_root
+# RUN: llvm-mc -triple=x86_64 -g -dwarf-version=5 %t.foo%{fs-sep}bar%{fs-sep}src.s -filetype=obj -o mapabs.5.o -fdebug-prefix-map=%t.foo=/broken_root -fdebug-prefix-map=%t.foo%{fs-sep}bar=/src_root
 # RUN: llvm-dwarfdump -v -debug-info -debug-line mapabs.5.o | FileCheck --check-prefix=MAPABS_V5 %s
 
+## The last -fdebug-prefix-map= wins even if the prefix is shorter.
+# RUN: llvm-mc -triple=x86_64 -g -dwarf-version=5 %t.foo%{fs-sep}bar%{fs-sep}src.s -filetype=obj -o mapabs.5.o -fdebug-prefix-map=%t.foo%{fs-sep}bar=/broken_root -fdebug-prefix-map=%t.foo=/src_root
+# RUN: llvm-dwarfdump -v -debug-info -debug-line mapabs.5.o | FileCheck --check-prefix=MAPABS_V5_A %s
+
 f:
   nop
 
@@ -46,3 +50,6 @@
 # MAPABS_V5:  DW_AT_comp_dir [DW_FORM_string] ("{{(/|\\)+}}src_root")
 # MAPABS_V5:  DW_AT_decl_file [DW_FORM_data4] ("/src_root{{(/|\\)+}}src.s")
 # MAPABS_V5:  include_directories[ 0] = .debug_line_str[0x] = "/src_root"
+
+# MAPABS_V5_A:DW_AT_comp_dir [DW_FORM_string] ("{{(/|\\)+}}src_root/bar")
+# MAPABS_V5_A:DW_AT_decl_file [DW_FORM_data4] ("/src_root{{(/|\\)+}}bar{{(/|\\)+}}src.s")
Index: llvm/lib/MC/MCContext.cpp
===
--- llvm/lib/MC/MCContext.cpp
+++ llvm/lib/MC/MCContext.cpp
@@ -881,11 +881,11 @@
 
 void MCContext::addDebugPrefixMapEntry(const std::string ,
const std::string ) {
-  DebugPrefixMap.insert(std::make_pair(From, To));
+  DebugPrefixMap.emplace_back(From, To);
 }
 
 void MCContext::remapDebugPath(SmallVectorImpl ) {
-  for (const auto &[From, To] : DebugPrefixMap)
+  for (const auto &[From, To] : llvm::reverse(DebugPrefixMap))
 if (llvm::sys::path::replace_path_prefix(Path, From, To))
   break;
 }
Index: llvm/include/llvm/MC/MCContext.h
===
--- llvm/include/llvm/MC/MCContext.h
+++ llvm/include/llvm/MC/MCContext.h
@@ -190,7 +190,7 @@
   SmallString<128> CompilationDir;
 
   /// Prefix replacement map for source file information.
-  std::map> DebugPrefixMap;
+  SmallVector, 0> DebugPrefixMap;
 
   /// The main file name if passed in explicitly.
   std::string MainFileName;
Index: clang/tools/driver/cc1as_main.cpp
===
--- clang/tools/driver/cc1as_main.cpp
+++ clang/tools/driver/cc1as_main.cpp
@@ -97,7 +97,8 @@
   std::string DwarfDebugFlags;
   std::string DwarfDebugProducer;
   std::string DebugCompilationDir;
-  std::map DebugPrefixMap;
+  llvm::SmallVector, 0>
+  DebugPrefixMap;
   llvm::DebugCompressionType CompressDebugSections =
   llvm::DebugCompressionType::None;
   std::string MainFileName;
@@ -275,8 +276,7 @@
 
   for (const auto  : Args.getAllArgValues(OPT_fdebug_prefix_map_EQ)) {
 auto Split = StringRef(Arg).split('=');
-Opts.DebugPrefixMap.insert(
-{std::string(Split.first), std::string(Split.second)});
+

[PATCH] D148757: [clang] Follow user-provided order for prefix map

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

I'm creating a patch to change DebugPrefixMap to respect the command line 
option order...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148757

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

[PATCH] D148757: [clang] Follow user-provided order for prefix map

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

I wasn't aware that `debug-prefix-map` prefers longest prefix. I agree that 
ideally `coverage-prefix-map`, `debug-prefix-map` and `macro-prefix-map` should 
be consistent across all LLVM tools and ideally also across compilers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148757

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

[PATCH] D148757: [clang] Follow user-provided order for prefix map

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Created https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109591 to discuss GCC's 
behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148757

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

[PATCH] D148444: [clang-tidy] Prevent `llvmlibc-inline-function-decl` triggering on lambdas

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

> Additionally, lambdas are always going to have internal linkage so they will 
> not differ accross TUs.

This isn't true - if a lambda appears in a header in any way, it probably has 
linkage the same as an inline function, not internal.

(eg: a lambda inside an inline function has linkage so it gets deduplicated 
with other copies from different inclusions of a header)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148444

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

[PATCH] D148757: [clang] Follow user-provided order for prefix map

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D148757#4285735 , @gulfem wrote:

> I compared Clang and GCC behavior.
> **1) Following user-specified prefix mapping order**
> GCC uses a linked list to store the prefix mappings, so it applies the prefix 
> mappings based on the user-provided order.
> https://github.com/gcc-mirror/gcc/blob/master/gcc/file-prefix-map.cc#L27
> Whereas, Clang currently uses an `std::map`, which does **NOT** follow 
> user-provided order.  So, Clang and GCC behavior do not match on that.
>
> **2) Applying multiple prefix mappings**
> Both Clang and GCC apply the first matching prefix mapping, and does not 
> apply the following prefix mappings. So, they are consistent in that.
>
> https://github.com/gcc-mirror/gcc/blob/master/gcc/file-prefix-map.cc#L88
> https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CoverageMappingGen.cpp#L1653

Sorry, request changes as I think I see conflicting needs. Cc #debug-info 


We have DebugPrefixMap and CoveragePrefixMap, and I think their order behavior 
should be the same.

D49466  added `DebugPrefixMap` to Clang which 
prefers a long prefix.
D132390  is a MC version of `DebugPrefixMap` 
that prefers a long prefix.

The current order of `CoveragePrefixMap` is definitely weird as it prefers a 
shorter prefix.
This patch is going to change the order to the command line option order, but 
it will be different from `DebugPrefixMap`.

Can `CoveragePrefixMap` be made to prefer a long prefix as `DebugPrefixMap` 
does?

> `-ffile-prefix-map==out/ to prepend out/ to any unmatched files`

This operation feels a bit strange to me...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148757

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

[PATCH] D148958: Size and element numbers are often swapped when calling calloc

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D148958#4288782 , @vzakhari wrote:

> In D148958#4288728 , @MaskRay wrote:
>
>> This is an unnecessary change. The arguments are interchangeable.
>> In musl, there are at least 2 places where the parameters could be swapped, 
>> but I not unsure this warrants a commit.
>
> Isn't the memory alignment dependent on the element size in general?  In this 
> case it might be a potential correctness issue fix.
> I understand that many `calloc` implementations may align them to 
> works-for-all boundary, but it may still worth be "fixing".

No. The two arguments are interchangeable. Neither argument affects the 
alignment. Use `aligned_alloc` to control alignment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148958

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

[PATCH] D148967: WIP: Disable atexit()-based lowering when LTO'ing kernel/kext code

[Julian Lettner via Phabricator via cfe-commits]

yln created this revision.
Herald added subscribers: llvm-commits, cfe-commits, ormris, steven_wu, 
hiraditya, inglorion.
Herald added projects: clang, LLVM, All.
yln requested review of this revision.
Herald added a subscriber: MaskRay.

rdar://93536111


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148967

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-ld-lto.c
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/CodeGen/TargetPassConfig.cpp
  llvm/test/CodeGen/ARM/ctors_dtors.ll


Index: llvm/test/CodeGen/ARM/ctors_dtors.ll
===
--- llvm/test/CodeGen/ARM/ctors_dtors.ll
+++ llvm/test/CodeGen/ARM/ctors_dtors.ll
@@ -1,4 +1,5 @@
 ; RUN: llc < %s -mtriple=arm-apple-darwin  | FileCheck %s -check-prefix=DARWIN
+; RUN: llc < %s -mtriple=arm-apple-darwin 
-disable-atexit-based-global-dtor-lowering  | FileCheck %s 
-check-prefix=DARWIN-LEGACY
 ; RUN: llc < %s -mtriple=arm-linux-gnu -target-abi=apcs  | FileCheck %s 
-check-prefix=ELF
 ; RUN: llc < %s -mtriple=arm-linux-gnueabi | FileCheck %s -check-prefix=GNUEABI
 
@@ -7,6 +8,10 @@
 ; DARWIN: .section __DATA,__mod_init_func,mod_init_funcs
 ; DARWIN-NOT: __mod_term_func
 
+; DARWIN-LEGACY-NOT: atexit
+; DARWIN-LEGACY: .section  __DATA,__mod_init_func,mod_init_funcs
+; DARWIN-LEGACY: .section  __DATA,__mod_term_func,mod_term_funcs
+
 ; ELF: .section .ctors,"aw",%progbits
 ; ELF: .section .dtors,"aw",%progbits
 
Index: llvm/lib/CodeGen/TargetPassConfig.cpp
===
--- llvm/lib/CodeGen/TargetPassConfig.cpp
+++ llvm/lib/CodeGen/TargetPassConfig.cpp
@@ -100,6 +100,9 @@
 cl::desc("Disable Copy Propagation pass"));
 static cl::opt 
DisablePartialLibcallInlining("disable-partial-libcall-inlining",
 cl::Hidden, cl::desc("Disable Partial Libcall Inlining"));
+static cl::opt DisableAtExitBasedGlobalDtorLowering(
+"disable-atexit-based-global-dtor-lowering", cl::Hidden,
+cl::desc("For MachO, disable atexit()-based global destructor lowering"));
 static cl::opt EnableImplicitNullChecks(
 "enable-implicit-null-checks",
 cl::desc("Fold null checks into faulting memory operations"),
@@ -878,7 +881,8 @@
 
   // For MachO, lower @llvm.global_dtors into @llvm.global_ctors with
   // __cxa_atexit() calls to avoid emitting the deprecated __mod_term_func.
-  if (TM->getTargetTriple().isOSBinFormatMachO())
+  if (TM->getTargetTriple().isOSBinFormatMachO() &&
+  !DisableAtExitBasedGlobalDtorLowering)
 addPass(createLowerGlobalDtorsLegacyPass());
 
   // Make sure that no unreachable blocks are instruction selected.
Index: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
===
--- llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -1202,7 +1202,10 @@
 
 MCSection *TargetLoweringObjectFileMachO::getStaticDtorSection(
 unsigned Priority, const MCSymbol *KeySym) const {
-  report_fatal_error("@llvm.global_dtors should have been lowered already");
+  return StaticDtorSection;
+  // kernel/kext do not provide atexit() for lowering.  See the
+  // -disable-atexit-based-global-dtor-lowering CodeGen flag.
+  // report_fatal_error("@llvm.global_dtors should have been lowered already");
 }
 
 void TargetLoweringObjectFileMachO::emitModuleMetadata(MCStreamer ,
Index: clang/test/Driver/darwin-ld-lto.c
===
--- clang/test/Driver/darwin-ld-lto.c
+++ clang/test/Driver/darwin-ld-lto.c
@@ -40,3 +40,11 @@
 // GISEL: {{ld(.exe)?"}}
 // GISEL: "-mllvm" "-global-isel"
 // GISEL: "-mllvm" "-global-isel-abort=0"
+
+
+// Check that we disable atexit()-based global destructor lowering when
+// compiling/linking for kernel/kext/freestanding.
+// RUN: %clang -target arm64-apple-darwin %s -flto -fapple-kext -### 2>&1 | \
+// RUN:   FileCheck --check-prefix=KEXT %s
+// KEXT: {{ld(.exe)?"}}
+// KEXT: "-mllvm" "-disable-atexit-based-global-dtor-lowering"
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -393,6 +393,13 @@
 }
   }
 
+  if (Args.hasArg(options::OPT_mkernel) ||
+  Args.hasArg(options::OPT_fapple_kext) ||
+  Args.hasArg(options::OPT_ffreestanding)) {
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-disable-atexit-based-global-dtor-lowering");
+  }
+
   Args.AddLastArg(CmdArgs, options::OPT_prebind);
   Args.AddLastArg(CmdArgs, options::OPT_noprebind);
   Args.AddLastArg(CmdArgs, options::OPT_nofixprebinding);


Index: llvm/test/CodeGen/ARM/ctors_dtors.ll
===
--- llvm/test/CodeGen/ARM/ctors_dtors.ll
+++ llvm/test/CodeGen/ARM/ctors_dtors.ll
@@ -1,4 +1,5 @@
 ; RUN: llc < 

[PATCH] D148961: [clang] Remove unnecessary virtual inheritance in `TargetInfo`

[Petr Hosek via Phabricator via cfe-commits]

phosek accepted this revision.
phosek 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/D148961/new/

https://reviews.llvm.org/D148961

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

[PATCH] D131618: [clang][llvm][lld] FatLTO Prototype

[Paul Kirth via Phabricator via cfe-commits]

paulkirth abandoned this revision.
paulkirth added a comment.

This can be abandoned in favor of https://reviews.llvm.org/D146776, 
https://reviews.llvm.org/D146777, and https://reviews.llvm.org/D146778


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131618

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

[PATCH] D141824: [clang-repl] Add a command to load dynamic libraries

[Han Zhu via Phabricator via cfe-commits]

zhuhan0 added a comment.

Hi, I have just got the time to come back to https://reviews.llvm.org/D147823, 
but I wanna check here first if you guys have any fix in flight? In addition to 
the linking issue, we also encountered the "cstdio" not found issue as 
mentioned above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141824

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

[PATCH] D148958: Size and element numbers are often swapped when calling calloc

[Slava Zakharin via Phabricator via cfe-commits]

vzakhari added a comment.

In D148958#4288728 , @MaskRay wrote:

> This is an unnecessary change. The arguments are interchangeable.
> In musl, there are at least 2 places where the parameters could be swapped, 
> but I not unsure this warrants a commit.

Isn't the memory alignment dependent on the element size in general?  In this 
case it might be a potential correntess issue fix.
I understand that many `calloc` implementations may align them to works-for-all 
boundary, but it may still worth be "fixing".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148958

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

[PATCH] D134475: [Clang] Add support for [[msvc::constexpr]] C++11 attribute

[Richard Dzenis via Phabricator via cfe-commits]

RIscRIpt planned changes to this revision.
RIscRIpt marked 2 inline comments as done.
RIscRIpt added a comment.

> Does it prohibit the inverse?  I think this documentation overall needs a 
> much better description of what the semantics are here, particularly anything 
> that you found in experimentation on the MS implementation.

Do you mean, can `[[msvc::constexpr]]` function call `constexpr` and remain 
const-evaluated? Yes.

From my experiments all Cody's points are valid, except for "An extended 
constexpr function can call other extended constexpr functions without a 
statement-level annotation".

I tried to make doc notes as compact as possible. Recapping his points, which 
were confirmed by tests:

1. `[[msvc::constexpr]]` can be applied only to function definition or a return 
statement.
2. A function can be either `constexpr` or `consteval` or 
`[[msvc::constexpr]]`, but no combination of these.
3. `[[msvc::constexpr]]` function is treated the same way as `constexpr` 
function if it is evaluated in terms of `[[msvc::constexpr]] return` statement, 
otherwise it's a regular function.

By function I mean anything function-like: functions, member-functions, 
constructors, destructors, operators.

In D134475#4287513 , @erichkeane 
wrote:

> Still suspicious about this/whether it models the MSVC implementation 
> correctly, but no real implementation concerns.  I REALLY want us to document 
> the attribute extensively however.

Thanks to your concern I decided to test each point of `[dcl.constexpr]` : 
`9.2.6 The constexpr and consteval specifiers` in Compiler Explorer: 
https://godbolt.org/z/qj3fnK5aW I am going to update tests accordingly.

Regarding on absolute matching MSVC implementation, I am not sure we will be 
able to achieve it with reasonable changes. I am skeptical, because I 
discovered the following test case:

  struct S2 {
  [[msvc::constexpr]] S2() {}
  [[msvc::constexpr]] bool value() { return true; } 
  static constexpr bool check() { [[msvc::constexpr]] return S2{}.value(); }
  };
  static_assert(S2::check());

It's a valid code for MSVC; nothing special https://godbolt.org/z/znnaonEhM
However supporting this code seems to be difficult in Clang:

1. S2 fails checks of "literal type" in this callstack:
  1. [[ 
https://github.com/llvm/llvm-project/blob/bc73ef0/clang/include/clang/AST/DeclCXX.h#L1419-L1445
 | `clang::CXXRecordDecl::isLiteral` ]]
  2. [[ 
https://github.com/llvm/llvm-project/blob/e98776a/clang/lib/AST/Type.cpp#L2746 
| `clang::Type::isLiteralType` ]]
  3. [[ 
https://github.com/llvm/llvm-project/blob/a4edc2c/clang/lib/AST/ExprConstant.cpp#L2317-L2320
 | `CheckLiteralType` ]]
2. We have information about CanEvalMSConstexpr only in `CheckLiteralType`. So, 
currently, I don't see how we can reasonably check whether `S2` is a valid 
literal type in context of `[[msvc::constexpr]]`. Two obvious **ugly** 
solutions are:
  1. Copy all checks from `clang::CXXRecordDecl::isLiteral` to 
`CheckLiteralType` - violates DRY; two places shall be maintained.
  2. Propagate `CanEvalMSConstexpr` down to `clang::CXXRecordDecl::isLiteral`. 
I don't think it's reasonable for supporting rare vendor-specific attribute.




Comment at: clang/include/clang/Basic/AttrDocs.td:3543
+definition or a return statement. It has no effect on function declarations.
+This attribute permits constant evaluation of ``[[msvc::constexpr]]`` functions
+in ``[[msvc::constexpr]] return`` statements inside ``constexpr`` or

erichkeane wrote:
> Does it prohibit the inverse?  I think this documentation overall needs a 
> much better description of what the semantics are here, particularly anything 
> that you found in experimentation on the MS implementation.
> Does it prohibit the inverse?
Do you mean, can `[[msvc::constexpr]]` function call `constexpr` and remain 
const-evaluated? Yes.

I elaborated on semantics in non-inline comment.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:7057-7058
+static void handleMSConstexprAttr(Sema , Decl *D, const ParsedAttr ) {
+  if (auto *FD = dyn_cast(D))
+FD->setConstexprKind(ConstexprSpecKind::Constexpr);
+  D->addAttr(::new (S.Context) MSConstexprAttr(S.Context, AL));

RIscRIpt wrote:
> aaron.ballman wrote:
> > We can use `cast` here because we know the declaration must have already 
> > been determined to be a function (from the subjects list in Attr.td).
> > 
> > That said, I think there's more semantic checking we want to do here. For 
> > example, can you use this attribute on a virtual function? What about a 
> > function already marked `constexpr`? Even more scary, what about a function 
> > marked `consteval`?
> > 
> > Also, I presume a `constexpr` function should be able to call an 
> > `[[msvc::constexpr]]` function and vice versa?
> > We can use cast here because we know the declaration must have already been 
> > determined to be a function 

[PATCH] D148958: Size and element numbers are often swapped when calling calloc

[Fangrui Song via Phabricator via cfe-commits]

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

This is an unnecessary change. In musl, there are at least 2 places where the 
parameters could be swapped, but I not unsure this warrants a commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148958

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

[PATCH] D148960: [Clang][AIX] Add back error for -fprofile-sample-generate/use on AIX

[Qiongsi Wu via Phabricator via cfe-commits]

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

Thanks for the fix Zarko! LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148960

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

[PATCH] D148962: [RISCV] Make Zicntr and Zihpm imply Zicsr.

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: asb, reames, kito-cheng, jrtc27.
Herald added subscribers: jobnoorman, luke, VincentWu, vkmr, frasercrmck, 
jdoerfert, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, 
psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, 
edward-jones, zzheng, shiva0217, niosHD, sabuasal, simoncook, johnrusso, rbar, 
hiraditya, arichardson.
Herald added a project: All.
craig.topper requested review of this revision.
Herald added subscribers: pcwang-thead, eopXD, MaskRay.
Herald added a project: LLVM.

Zicntr and Zihpm are names for groups of CSRs so they should imply
that CSRs exist.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148962

Files:
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/RISCVFeatures.td
  llvm/test/CodeGen/RISCV/attributes.ll


Index: llvm/test/CodeGen/RISCV/attributes.ll
===
--- llvm/test/CodeGen/RISCV/attributes.ll
+++ llvm/test/CodeGen/RISCV/attributes.ll
@@ -205,8 +205,8 @@
 ; RV32ZCF: .attribute 5, "rv32i2p1_zcf1p0"
 ; RV32ZICSR: .attribute 5, "rv32i2p1_zicsr2p0"
 ; RV32ZIFENCEI: .attribute 5, "rv32i2p1_zifencei2p0"
-; RV32ZICNTR: .attribute 5, "rv32i2p1_zicntr1p0"
-; RV32ZIHPM: .attribute 5, "rv32i2p1_zihpm1p0"
+; RV32ZICNTR: .attribute 5, "rv32i2p1_zicntr1p0_zicsr2p0"
+; RV32ZIHPM: .attribute 5, "rv32i2p1_zicsr2p0_zihpm1p0"
 ; RV32ZFA: .attribute 5, "rv32i2p1_f2p2_zicsr2p0_zfa0p2"
 ; RV32ZVBB: .attribute 5, "rv32i2p1_zicsr2p0_zvbb0p5_zve32x1p0_zvl32b1p0"
 ; RV32ZVBC: .attribute 5, 
"rv32i2p1_zicsr2p0_zvbc0p5_zve32x1p0_zve64x1p0_zvl32b1p0_zvl64b1p0"
@@ -282,8 +282,8 @@
 ; RV64ZCD: .attribute 5, "rv64i2p1_zcd1p0"
 ; RV64ZICSR: .attribute 5, "rv64i2p1_zicsr2p0"
 ; RV64ZIFENCEI: .attribute 5, "rv64i2p1_zifencei2p0"
-; RV64ZICNTR: .attribute 5, "rv64i2p1_zicntr1p0"
-; RV64ZIHPM: .attribute 5, "rv64i2p1_zihpm1p0"
+; RV64ZICNTR: .attribute 5, "rv64i2p1_zicntr1p0_zicsr2p0"
+; RV64ZIHPM: .attribute 5, "rv64i2p1_zicsr2p0_zihpm1p0"
 ; RV64ZFA: .attribute 5, "rv64i2p1_f2p2_zicsr2p0_zfa0p2"
 ; RV64ZVBB: .attribute 5, "rv64i2p1_zicsr2p0_zvbb0p5_zve32x1p0_zvl32b1p0"
 ; RV64ZVBC: .attribute 5, 
"rv64i2p1_zicsr2p0_zvbc0p5_zve32x1p0_zve64x1p0_zvl32b1p0_zvl64b1p0"
Index: llvm/lib/Target/RISCV/RISCVFeatures.td
===
--- llvm/lib/Target/RISCV/RISCVFeatures.td
+++ llvm/lib/Target/RISCV/RISCVFeatures.td
@@ -88,11 +88,13 @@
 
 def FeatureStdExtZicntr
 : SubtargetFeature<"zicntr", "HasStdExtZicntr", "true",
-   "'zicntr' (Base Counters and Timers)">;
+   "'zicntr' (Base Counters and Timers)",
+   [FeatureStdExtZicsr]>;
 
 def FeatureStdExtZihpm
 : SubtargetFeature<"zihpm", "HasStdExtZihpm", "true",
-   "'zihpm' (Hardware Performance Counters)">;
+   "'zihpm' (Hardware Performance Counters)",
+   [FeatureStdExtZicsr]>;
 
 def FeatureStdExtZfhmin
 : SubtargetFeature<"zfhmin", "HasStdExtZfhmin", "true",
Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -928,6 +928,8 @@
 static const char *ImpliedExtsZfinx[] = {"zicsr"};
 static const char *ImpliedExtsZhinx[] = {"zfinx"};
 static const char *ImpliedExtsZhinxmin[] = {"zfinx"};
+static const char *ImpliedExtsZicntr[] = {"zicsr"};
+static const char *ImpliedExtsZihpm[] = {"zicsr"};
 static const char *ImpliedExtsZk[] = {"zkn", "zkt", "zkr"};
 static const char *ImpliedExtsZkn[] = {"zbkb", "zbkc", "zbkx",
"zkne", "zknd", "zknh"};
@@ -983,6 +985,8 @@
 {{"zfinx"}, {ImpliedExtsZfinx}},
 {{"zhinx"}, {ImpliedExtsZhinx}},
 {{"zhinxmin"}, {ImpliedExtsZhinxmin}},
+{{"zicntr"}, {ImpliedExtsZicntr}},
+{{"zihpm"}, {ImpliedExtsZihpm}},
 {{"zk"}, {ImpliedExtsZk}},
 {{"zkn"}, {ImpliedExtsZkn}},
 {{"zks"}, {ImpliedExtsZks}},


Index: llvm/test/CodeGen/RISCV/attributes.ll
===
--- llvm/test/CodeGen/RISCV/attributes.ll
+++ llvm/test/CodeGen/RISCV/attributes.ll
@@ -205,8 +205,8 @@
 ; RV32ZCF: .attribute 5, "rv32i2p1_zcf1p0"
 ; RV32ZICSR: .attribute 5, "rv32i2p1_zicsr2p0"
 ; RV32ZIFENCEI: .attribute 5, "rv32i2p1_zifencei2p0"
-; RV32ZICNTR: .attribute 5, "rv32i2p1_zicntr1p0"
-; RV32ZIHPM: .attribute 5, "rv32i2p1_zihpm1p0"
+; RV32ZICNTR: .attribute 5, "rv32i2p1_zicntr1p0_zicsr2p0"
+; RV32ZIHPM: .attribute 5, "rv32i2p1_zicsr2p0_zihpm1p0"
 ; RV32ZFA: .attribute 5, "rv32i2p1_f2p2_zicsr2p0_zfa0p2"
 ; RV32ZVBB: .attribute 5, "rv32i2p1_zicsr2p0_zvbb0p5_zve32x1p0_zvl32b1p0"
 ; RV32ZVBC: .attribute 5, "rv32i2p1_zicsr2p0_zvbc0p5_zve32x1p0_zve64x1p0_zvl32b1p0_zvl64b1p0"
@@ -282,8 +282,8 @@
 ; RV64ZCD: .attribute 5, "rv64i2p1_zcd1p0"
 ; RV64ZICSR: 

[PATCH] D148954: [clang] Make access to submodules via `iterator_range`

[Sviatoslav Osipov via Phabricator via cfe-commits]

Stoorx added a comment.

Can you commit it? I have no commit access.

Stoorx
me[at]stoorx.one


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

https://reviews.llvm.org/D148954

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

[PATCH] D148961: [clang] Remove unnecessary virtual inheritance in `TargetInfo`

[Sviatoslav Osipov via Phabricator via cfe-commits]

Stoorx created this revision.
Herald added a project: All.
Stoorx requested review of this revision.
Herald added a project: clang.

Since the `TargetInfo` has no diamond-like inheritance diagram,
the `virtual` keyword is not necessary.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148961

Files:
  clang/include/clang/Basic/TargetInfo.h


Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -202,7 +202,7 @@
 
 /// Exposes information about the current target.
 ///
-class TargetInfo : public virtual TransferrableTargetInfo,
+class TargetInfo : public TransferrableTargetInfo,
public RefCountedBase {
   std::shared_ptr TargetOpts;
   llvm::Triple Triple;


Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -202,7 +202,7 @@
 
 /// Exposes information about the current target.
 ///
-class TargetInfo : public virtual TransferrableTargetInfo,
+class TargetInfo : public TransferrableTargetInfo,
public RefCountedBase {
   std::shared_ptr TargetOpts;
   llvm::Triple Triple;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148960: [Clang][AIX] Add back error for -fprofile-sample-generate/use on AIX

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA created this revision.
ZarkoCA added reviewers: qiongsiwu1, w2yehia.
Herald added a project: All.
ZarkoCA requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

D148177  also removed the error for sampling 
based profiling which is not currently
supported on AIX. Adding that error back.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148960

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/unsupported-option.c


Index: clang/test/Driver/unsupported-option.c
===
--- clang/test/Driver/unsupported-option.c
+++ clang/test/Driver/unsupported-option.c
@@ -13,3 +13,7 @@
 // RUN: not %clang --target=powerpc64-ibm-aix %s -mlong-double-128 2>&1 | \
 // RUN: FileCheck %s --check-prefix=AIX64-LONGDOUBLE128-ERR
 // AIX64-LONGDOUBLE128-ERR: error: unsupported option '-mlong-double-128' for 
target 'powerpc64-ibm-aix'
+
+// RUN: not %clang -fprofile-sample-use=code.prof --target=powerpc-ibm-aix %s 
2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-SAMPLE
+// AIX-PROFILE-SAMPLE: error: unsupported option '-fprofile-sample-use=' for 
target
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -740,6 +740,12 @@
 PGOGenerateArg = nullptr;
   }
 
+  if (TC.getTriple().isOSAIX()) {
+if (Arg *ProfileSampleUseArg = getLastProfileSampleUseArg(Args))
+  D.Diag(diag::err_drv_unsupported_opt_for_target)
+  << ProfileSampleUseArg->getSpelling() << TC.getTriple().str();
+  }
+
   if (ProfileGenerateArg) {
 if (ProfileGenerateArg->getOption().matches(
 options::OPT_fprofile_instr_generate_EQ))


Index: clang/test/Driver/unsupported-option.c
===
--- clang/test/Driver/unsupported-option.c
+++ clang/test/Driver/unsupported-option.c
@@ -13,3 +13,7 @@
 // RUN: not %clang --target=powerpc64-ibm-aix %s -mlong-double-128 2>&1 | \
 // RUN: FileCheck %s --check-prefix=AIX64-LONGDOUBLE128-ERR
 // AIX64-LONGDOUBLE128-ERR: error: unsupported option '-mlong-double-128' for target 'powerpc64-ibm-aix'
+
+// RUN: not %clang -fprofile-sample-use=code.prof --target=powerpc-ibm-aix %s 2>&1 | \
+// RUN: FileCheck %s --check-prefix=AIX-PROFILE-SAMPLE
+// AIX-PROFILE-SAMPLE: error: unsupported option '-fprofile-sample-use=' for target
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -740,6 +740,12 @@
 PGOGenerateArg = nullptr;
   }
 
+  if (TC.getTriple().isOSAIX()) {
+if (Arg *ProfileSampleUseArg = getLastProfileSampleUseArg(Args))
+  D.Diag(diag::err_drv_unsupported_opt_for_target)
+  << ProfileSampleUseArg->getSpelling() << TC.getTriple().str();
+  }
+
   if (ProfileGenerateArg) {
 if (ProfileGenerateArg->getOption().matches(
 options::OPT_fprofile_instr_generate_EQ))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146777: [clang] Preliminary fat-lto-object support

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 515910.
paulkirth added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146777

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/embed-lto-fatlto.c
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/fatlto-objects.c

Index: clang/test/Driver/fatlto-objects.c
===
--- /dev/null
+++ clang/test/Driver/fatlto-objects.c
@@ -0,0 +1,10 @@
+// REQUIRES: x86_64-linux
+// RUN: %clang -target x86_64-unknown-linux-gnu -flto -ffat-lto-objects -fintegrated-as -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC
+// CHECK-CC: -cc1
+// CHECK-CC: -emit-obj
+// CHECK-CC: -ffat-lto-objects
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -ffat-lto-objects -fintegrated-as -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC-NOLTO
+// CHECK-CC-NOLTO: -cc1
+// CHECK-CC-NOLTO: -emit-obj
+// CHECK-CC-NOLTO-NOT: -ffat-lto-objects
Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -421,7 +421,6 @@
 // CHECK-WARNING-DAG: optimization flag '-fwhole-program' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fcaller-saves' is not supported
 // CHECK-WARNING-DAG: optimization flag '-freorder-blocks' is not supported
-// CHECK-WARNING-DAG: optimization flag '-ffat-lto-objects' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fmerge-constants' is not supported
 // CHECK-WARNING-DAG: optimization flag '-finline-small-functions' is not supported
 // CHECK-WARNING-DAG: optimization flag '-ftree-dce' is not supported
Index: clang/test/CodeGen/embed-lto-fatlto.c
===
--- /dev/null
+++ clang/test/CodeGen/embed-lto-fatlto.c
@@ -0,0 +1,9 @@
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -S -flto=full -ffat-lto-objects -emit-llvm < %s  | FileCheck %s
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -S -flto=full -ffat-lto-objects -emit-llvm < %s  | FileCheck %s
+//
+// CHECK: !{{[0-9]+}} = !{i32 1, !"ThinLTO", i32 0}
+// CHECK: !{{[0-9]+}} = !{i32 1, !"EnableSplitLTOUnit", i32 1}
+
+int test(void) {
+  return 0xabcd;
+}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7262,6 +7262,14 @@
 }
   }
 
+  if (IsUsingLTO && Args.getLastArg(options::OPT_ffat_lto_objects)) {
+assert(LTOMode == LTOK_Full || LTOMode == LTOK_Thin);
+CmdArgs.push_back(Args.MakeArgString(
+Twine("-flto=") + (LTOMode == LTOK_Thin ? "thin" : "full")));
+CmdArgs.push_back("-flto-unit");
+CmdArgs.push_back("-ffat-lto-objects");
+  }
+
   if (Args.hasArg(options::OPT_forder_file_instrumentation)) {
  CmdArgs.push_back("-forder-file-instrumentation");
  // Enable order file instrumentation when ThinLTO is not on. When ThinLTO is
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4641,8 +4641,13 @@
   }
   case phases::Backend: {
 if (isUsingLTO() && TargetDeviceOffloadKind == Action::OFK_None) {
-  types::ID Output =
-  Args.hasArg(options::OPT_S) ? types::TY_LTO_IR : types::TY_LTO_BC;
+  types::ID Output;
+  if (Args.hasArg(options::OPT_S))
+Output = types::TY_LTO_IR;
+  else if (Args.hasArg(options::OPT_ffat_lto_objects))
+Output = types::TY_PP_Asm;
+  else
+Output = types::TY_LTO_BC;
   return C.MakeAction(Input, Output);
 }
 if (isUsingLTO(/* IsOffload */ true) &&
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -24,6 +24,7 @@
 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
 #include "llvm/Bitcode/BitcodeWriterPass.h"
+#include "llvm/Bitcode/EmbedBitcodePass.h"
 #include "llvm/CodeGen/RegAllocRegistry.h"
 #include "llvm/CodeGen/SchedulerRegistry.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
@@ -991,7 +992,10 @@
 MPM.addPass(InstrProfiling(*Options, false));
   });
 
-if (IsThinLTO) {
+if (CodeGenOpts.FatLTO) {
+  MPM = PB.buildFatLTODefaultPipeline(
+  Level, IsThinLTO, IsThinLTO || shouldEmitRegularLTOSummary());
+} else if (IsThinLTO) {
   MPM = PB.buildThinLTOPreLinkDefaultPipeline(Level);
 } else if (IsLTO) {
   MPM = 

[PATCH] D148958: Size and element numbers are often swapped when calling calloc

[Alf via Phabricator via cfe-commits]

gAlfonso-bit created this revision.
gAlfonso-bit added a reviewer: MaskRay.
Herald added a subscriber: Enna1.
Herald added projects: Flang, All.
gAlfonso-bit requested review of this revision.
Herald added subscribers: Sanitizers, cfe-commits, jdoerfert.
Herald added projects: clang, Sanitizers.

The element number comes first when calling calloc, but sometimes the element 
number is passed as the size and vice-versa.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148958

Files:
  clang/test/Analysis/malloc.mm
  clang/test/Analysis/uninit-vals.m
  clang/test/CodeGen/alloc-size.c
  compiler-rt/lib/profile/InstrProfilingFile.c
  compiler-rt/test/tsan/java_finalizer2.cpp
  flang/runtime/ragged.cpp

Index: flang/runtime/ragged.cpp
===
--- flang/runtime/ragged.cpp
+++ flang/runtime/ragged.cpp
@@ -32,10 +32,9 @@
 header->flags = (rank << 1) | isHeader;
 header->extentPointer = extentVector;
 if (isHeader) {
-  header->bufferPointer = std::calloc(sizeof(RaggedArrayHeader), size);
+  header->bufferPointer = std::calloc(size, sizeof(RaggedArrayHeader));
 } else {
-  header->bufferPointer =
-  static_cast(std::calloc(elementSize, size));
+  header->bufferPointer = std::calloc(size, elementSize);
 }
 return header;
   } else {
Index: compiler-rt/test/tsan/java_finalizer2.cpp
===
--- compiler-rt/test/tsan/java_finalizer2.cpp
+++ compiler-rt/test/tsan/java_finalizer2.cpp
@@ -51,7 +51,7 @@
 }
 
 int main() {
-  Heap* heap = (Heap*)calloc(sizeof(Heap), 2) + 1;
+  Heap* heap = (Heap*)calloc(2, sizeof(Heap)) + 1;
   __tsan_java_init((jptr)heap, sizeof(*heap));
   __tsan_java_alloc((jptr)heap, sizeof(*heap));
   // Ballast threads merely make the bug a bit easier to trigger.
Index: compiler-rt/lib/profile/InstrProfilingFile.c
===
--- compiler-rt/lib/profile/InstrProfilingFile.c
+++ compiler-rt/lib/profile/InstrProfilingFile.c
@@ -293,10 +293,10 @@
 COMPILER_RT_VISIBILITY ProfBufferIO *
 lprofCreateBufferIOInternal(void *File, uint32_t BufferSz) {
   FreeHook = 
-  DynamicBufferIOBuffer = (uint8_t *)calloc(BufferSz, 1);
+  DynamicBufferIOBuffer = (uint8_t *)calloc(1, BufferSz);
   VPBufferSize = BufferSz;
   ProfDataWriter *fileWriter =
-  (ProfDataWriter *)calloc(sizeof(ProfDataWriter), 1);
+  (ProfDataWriter *)calloc(1, sizeof(ProfDataWriter));
   initFileWriter(fileWriter, File);
   ProfBufferIO *IO = lprofCreateBufferIO(fileWriter);
   IO->OwnFileWriter = 1;
Index: clang/test/CodeGen/alloc-size.c
===
--- clang/test/CodeGen/alloc-size.c
+++ clang/test/CodeGen/alloc-size.c
@@ -137,7 +137,7 @@
   // CHECK: store i32 36
   gi = OBJECT_SIZE_BUILTIN(>t[1], 3);
 
-  struct Data *const arr = my_calloc(sizeof(*data), 2);
+  struct Data *const arr = my_calloc(2, sizeof(*data));
   // CHECK: store i32 96
   gi = OBJECT_SIZE_BUILTIN(arr, 0);
   // CHECK: store i32 96
@@ -171,7 +171,7 @@
   // CHECK: store i32 11
   gi = OBJECT_SIZE_BUILTIN(data->end, 3);
 
-  struct Data *const arr = my_calloc(sizeof(*arr) + 5, 3);
+  struct Data *const arr = my_calloc(3, sizeof(*arr) + 5);
   // AFAICT, GCC treats malloc and calloc identically. So, we should do the
   // same.
   //
Index: clang/test/Analysis/uninit-vals.m
===
--- clang/test/Analysis/uninit-vals.m
+++ clang/test/Analysis/uninit-vals.m
@@ -158,7 +158,7 @@
 }
 
 void PR14765_test(void) {
-  Circle *testObj = calloc(sizeof(Circle), 1);
+  Circle *testObj = calloc(1, sizeof(Circle));
 
   clang_analyzer_eval(testObj->size == 0); // expected-warning{{TRUE}}
// expected-note@-1{{TRUE}}
@@ -207,7 +207,7 @@
 }
 
 void PR14765_test_int(void) {
-  IntCircle *testObj = calloc(sizeof(IntCircle), 1);
+  IntCircle *testObj = calloc(1, sizeof(IntCircle));
 
   clang_analyzer_eval(testObj->size == 0); // expected-warning{{TRUE}}
// expected-note@-1{{TRUE}}
@@ -311,7 +311,7 @@
 }
 
 void testSmallStructInLargerStruct(void) {
-  IntCircle2D *testObj = calloc(sizeof(IntCircle2D), 1);
+  IntCircle2D *testObj = calloc(1, sizeof(IntCircle2D));
 
   clang_analyzer_eval(testObj->size == 0); // expected-warning{{TRUE}}
// expected-note@-1{{TRUE}}
Index: clang/test/Analysis/malloc.mm
===
--- clang/test/Analysis/malloc.mm
+++ clang/test/Analysis/malloc.mm
@@ -116,17 +116,17 @@
 }
 
 void testNoCopy() {
-  char *p = (char *)calloc(sizeof(int), 1);
+  char *p = (char *)calloc(1, sizeof(int));
   CustomData *w = [CustomData somethingNoCopy:p]; // no-warning
 }
 
 void testFreeWhenDone() {
-  char *p = (char *)calloc(sizeof(int), 

[PATCH] D148954: [clang] Make access to submodules via `iterator_range`

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D148954

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

[PATCH] D142907: LangRef: Add "dynamic" option to "denormal-fp-math"

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

If you have a library function that's built with 
"denormal-fp-math"="dynamic,dynamic", you can link it into code built in any 
mode, and LTO should be able to propagate that mode from the caller to the 
callee.  That doesn't require clang to do anything special; you can just 
specify -fdenormal-fp-math=dynamic while building the library, and the user 
specifies -fdenormal-fp-math=ieee while building their code.

I guess you're worried specifically about ODR inline functions, defined in 
headers?  The user specifies a specific mode because they know their code 
honors it... but the user might not be aware of the effect on functions defined 
in library headers.  Other libraries in the same binary might use the same 
header, but specify a different mode.  So if the user specifies a denormal 
mode, we should ignore it for ODR inline functions, because they didn't 
actually mean to apply the denormal mode to those definitions?

I'm not sure about applying those semantics automatically; I don't think 
there's any precedent in clang for anything like this.  The closest thing I can 
think of is -fvisibility-inlines-hidden.  I'd prefer to RFC it separately from 
the rest of the patch, and loop in clang frontend owners, since the precedent 
we set here will apply to other sorts of attributes.


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

https://reviews.llvm.org/D142907

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

[PATCH] D148954: [clang] Make access to submodules via `iterator_range`

[Sviatoslav Osipov via Phabricator via cfe-commits]

Stoorx updated this revision to Diff 515892.
Stoorx added a comment.

Rebase


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

https://reviews.llvm.org/D148954

Files:
  clang-tools-extra/modularize/CoverageChecker.cpp
  clang-tools-extra/modularize/ModularizeUtilities.cpp
  clang/include/clang/Basic/Module.h
  clang/lib/Basic/Module.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -500,8 +500,7 @@
   // Submodule order depends on order of header includes for inferred submodules
   // we don't care about the exact order, so sort so that it's consistent across
   // TUs to improve sharing.
-  SmallVector Submodules(M->submodule_begin(),
- M->submodule_end());
+  SmallVector Submodules(M->submodules());
   llvm::stable_sort(Submodules, [](const Module *A, const Module *B) {
 return A->Name < B->Name;
   });
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1903,18 +1903,16 @@
 SavedStrings.push_back(FilenameDup.data());
 
 HeaderFileInfoTrait::key_type Key = {
-  FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0
-};
+FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0};
 HeaderFileInfoTrait::data_type Data = {
-  Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}
-};
+Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}};
 // FIXME: Deal with cases where there are multiple unresolved header
 // directives in different submodules for the same header.
 Generator.insert(Key, Data, GeneratorTrait);
 ++NumHeaderSearchEntries;
   }
-
-  Worklist.append(M->submodule_begin(), M->submodule_end());
+  auto SubmodulesRange = M->submodules();
+  Worklist.append(SubmodulesRange.begin(), SubmodulesRange.end());
 }
   }
 
@@ -2701,9 +2699,8 @@
 /// given module).
 static unsigned getNumberOfModules(Module *Mod) {
   unsigned ChildModules = 0;
-  for (auto Sub = Mod->submodule_begin(), SubEnd = Mod->submodule_end();
-   Sub != SubEnd; ++Sub)
-ChildModules += getNumberOfModules(*Sub);
+  for (auto *Submodule : Mod->submodules())
+ChildModules += getNumberOfModules(Submodule);
 
   return ChildModules + 1;
 }
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -4309,16 +4309,13 @@
 /*IsInclusionDirective=*/false);
 // Enumerate submodules.
 if (Mod) {
-  for (Module::submodule_iterator Sub = Mod->submodule_begin(),
-  SubEnd = Mod->submodule_end();
-   Sub != SubEnd; ++Sub) {
-
+  for (auto *Submodule : Mod->submodules()) {
 Builder.AddTypedTextChunk(
-Builder.getAllocator().CopyString((*Sub)->Name));
+Builder.getAllocator().CopyString(Submodule->Name));
 Results.AddResult(Result(
 Builder.TakeString(), CCP_Declaration, CXCursor_ModuleImportDecl,
-(*Sub)->isAvailable() ? CXAvailability_Available
-  : CXAvailability_NotAvailable));
+Submodule->isAvailable() ? CXAvailability_Available
+ : CXAvailability_NotAvailable));
   }
 }
   }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1240,7 +1240,8 @@
 ModMap.resolveConflicts(Mod, /*Complain=*/false);
 
 // Queue the submodules, so their exports will also be resolved.
-Stack.append(Mod->submodule_begin(), Mod->submodule_end());
+auto SubmodulesRange = Mod->submodules();
+Stack.append(SubmodulesRange.begin(), SubmodulesRange.end());
   }
 }
 
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -429,11 +429,9 @@
   }
 
   // Recurse into submodules.
-  for (clang::Module::submodule_iterator Sub = Module->submodule_begin(),
-  SubEnd = Module->submodule_end();
-   Sub != SubEnd; ++Sub)
+  for 

[PATCH] D148954: [clang] Make access to submodules via `iterator_range`

[Sviatoslav Osipov via Phabricator via cfe-commits]

Stoorx created this revision.
Herald added a project: All.
Stoorx requested review of this revision.
Herald added projects: clang, clang-tools-extra.

In file `clang/lib/Basic/Module.cpp` the `Module` class had `submodule_begin()` 
and `submodule_end()` functions to retrieve corresponding iterators for private 
vector of Modules. This commit removes mentioned functions, and replaces all of 
theirs usages with `submodules()` function and range-based for-loops.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148954

Files:
  clang-tools-extra/modularize/CoverageChecker.cpp
  clang-tools-extra/modularize/ModularizeUtilities.cpp
  clang/include/clang/Basic/Module.h
  clang/lib/Basic/Module.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -500,8 +500,7 @@
   // Submodule order depends on order of header includes for inferred submodules
   // we don't care about the exact order, so sort so that it's consistent across
   // TUs to improve sharing.
-  SmallVector Submodules(M->submodule_begin(),
- M->submodule_end());
+  SmallVector Submodules(M->submodules());
   llvm::stable_sort(Submodules, [](const Module *A, const Module *B) {
 return A->Name < B->Name;
   });
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1903,18 +1903,16 @@
 SavedStrings.push_back(FilenameDup.data());
 
 HeaderFileInfoTrait::key_type Key = {
-  FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0
-};
+FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0};
 HeaderFileInfoTrait::data_type Data = {
-  Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}
-};
+Empty, {}, {M, ModuleMap::headerKindToRole(U.Kind)}};
 // FIXME: Deal with cases where there are multiple unresolved header
 // directives in different submodules for the same header.
 Generator.insert(Key, Data, GeneratorTrait);
 ++NumHeaderSearchEntries;
   }
-
-  Worklist.append(M->submodule_begin(), M->submodule_end());
+  auto SubmodulesRange = M->submodules();
+  Worklist.append(SubmodulesRange.begin(), SubmodulesRange.end());
 }
   }
 
@@ -2701,9 +2699,8 @@
 /// given module).
 static unsigned getNumberOfModules(Module *Mod) {
   unsigned ChildModules = 0;
-  for (auto Sub = Mod->submodule_begin(), SubEnd = Mod->submodule_end();
-   Sub != SubEnd; ++Sub)
-ChildModules += getNumberOfModules(*Sub);
+  for (auto *Submodule : Mod->submodules())
+ChildModules += getNumberOfModules(Submodule);
 
   return ChildModules + 1;
 }
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -4309,16 +4309,13 @@
 /*IsInclusionDirective=*/false);
 // Enumerate submodules.
 if (Mod) {
-  for (Module::submodule_iterator Sub = Mod->submodule_begin(),
-  SubEnd = Mod->submodule_end();
-   Sub != SubEnd; ++Sub) {
-
+  for (auto *Submodule : Mod->submodules()) {
 Builder.AddTypedTextChunk(
-Builder.getAllocator().CopyString((*Sub)->Name));
+Builder.getAllocator().CopyString(Submodule->Name));
 Results.AddResult(Result(
 Builder.TakeString(), CCP_Declaration, CXCursor_ModuleImportDecl,
-(*Sub)->isAvailable() ? CXAvailability_Available
-  : CXAvailability_NotAvailable));
+Submodule->isAvailable() ? CXAvailability_Available
+ : CXAvailability_NotAvailable));
   }
 }
   }
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1240,7 +1240,8 @@
 ModMap.resolveConflicts(Mod, /*Complain=*/false);
 
 // Queue the submodules, so their exports will also be resolved.
-Stack.append(Mod->submodule_begin(), Mod->submodule_end());
+auto SubmodulesRange = Mod->submodules();
+Stack.append(SubmodulesRange.begin(), SubmodulesRange.end());
   }
 }
 
Index: clang/lib/Frontend/FrontendAction.cpp

[PATCH] D148835: [clang] removes trailing whitespace

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D148835#4288151 , @erichkeane 
wrote:

> In D148835#4288148 , @cjdb wrote:
>
>> In D148835#4286924 , 
>> @aaron.ballman wrote:
>>
>>> In D148835#4286905 , @erichkeane 
>>> wrote:
>>>
 In D148835#4284871 , @cjdb wrote:

> I've had some very good input about why this probably shouldn't go ahead: 
> git history erasure :')

 While that is a common criticism, if we're going to do this at all, we 
 should do it in a single patch, as we can use --ignore-rev: 
 https://akrabat.com/ignoring-revisions-with-git-blame/

 This is a pretty common thing to do, and I don't think it should prevent 
 us from doing this, which I think is the 'greater good' here.  The 
 alternative is to continue having a bunch of unrelated patches piece-meal 
 'erase' git history as people accidentally fix these.
>>>
>>> We already ignore blame revisions like this today: 
>>> https://github.com/llvm/llvm-project/blob/main/.git-blame-ignore-revs
>>
>> Oh cool, I had no idea this was possible :)
>>
>>> However, I'd ask that we hold off on this change unless we have some 
>>> tooling in place that rejects new additions of trailing whitespace, 
>>> otherwise we're going to end up in this exact same situation again in 
>>> another week or two. When CI starts failing on patches that insert *new* 
>>> trailing whitespace, then I'm all for this change because we can fix it 
>>> once and hopefully keep it fixed.
>>
>> Since Clang doesn't have pre-commit CI, how can we meaningfully progress 
>> here?
>
> Phab at least has Pre-commit CI, i would expect that to be sufficient? It 
> would at least cut down accidential whitespace by orders of magnitude.

Right, but Phab's precommit CI doesn't seem to be a blocker for merging, and 
often has false negatives, so I expect stuff to slip through :/

In D148835#4288248 , @sbc100 wrote:

> In D148835#4288151 , @erichkeane 
> wrote:
>
>> In D148835#4288148 , @cjdb wrote:
>>
>>> In D148835#4286924 , 
>>> @aaron.ballman wrote:
>>>
 In D148835#4286905 , @erichkeane 
 wrote:

> In D148835#4284871 , @cjdb 
> wrote:
>
>> I've had some very good input about why this probably shouldn't go 
>> ahead: git history erasure :')
>
> While that is a common criticism, if we're going to do this at all, we 
> should do it in a single patch, as we can use --ignore-rev: 
> https://akrabat.com/ignoring-revisions-with-git-blame/
>
> This is a pretty common thing to do, and I don't think it should prevent 
> us from doing this, which I think is the 'greater good' here.  The 
> alternative is to continue having a bunch of unrelated patches piece-meal 
> 'erase' git history as people accidentally fix these.

 We already ignore blame revisions like this today: 
 https://github.com/llvm/llvm-project/blob/main/.git-blame-ignore-revs
>>>
>>> Oh cool, I had no idea this was possible :)
>>>
 However, I'd ask that we hold off on this change unless we have some 
 tooling in place that rejects new additions of trailing whitespace, 
 otherwise we're going to end up in this exact same situation again in 
 another week or two. When CI starts failing on patches that insert *new* 
 trailing whitespace, then I'm all for this change because we can fix it 
 once and hopefully keep it fixed.
>>>
>>> Since Clang doesn't have pre-commit CI, how can we meaningfully progress 
>>> here?
>>
>> Phab at least has Pre-commit CI, i would expect that to be sufficient? It 
>> would at least cut down accidential whitespace by orders of magnitude.
>
> Yes, isn't this already covered by the existing use of `clang-format` on 
> upload to phabricator (`arc diff`) ?At least for me it always gives me 
> warnings if I try to upload anything that doesn't match clang-format.

There are folks who have historically uploaded using the web UI specifically to 
ignore clang-format's suggestions, and `arc diff --nolint` will skip the 
formatting stage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148835

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

[PATCH] D146777: [clang] Preliminary fat-lto-object support

[Paul Kirth via Phabricator via cfe-commits]

paulkirth updated this revision to Diff 515880.
paulkirth added a comment.

Add relese note.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146777

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/embed-lto-fatlto.c
  clang/test/Driver/clang_f_opts.c
  clang/test/Driver/fatlto-objects.c

Index: clang/test/Driver/fatlto-objects.c
===
--- /dev/null
+++ clang/test/Driver/fatlto-objects.c
@@ -0,0 +1,10 @@
+// REQUIRES: x86_64-linux
+// RUN: %clang -target x86_64-unknown-linux-gnu -flto -ffat-lto-objects -fintegrated-as -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC
+// CHECK-CC: -cc1
+// CHECK-CC: -emit-obj
+// CHECK-CC: -ffat-lto-objects
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -ffat-lto-objects -fintegrated-as -### %s -c 2>&1 | FileCheck %s -check-prefix=CHECK-CC-NOLTO
+// CHECK-CC-NOLTO: -cc1
+// CHECK-CC-NOLTO: -emit-obj
+// CHECK-CC-NOLTO-NOT: -ffat-lto-objects
Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -421,7 +421,6 @@
 // CHECK-WARNING-DAG: optimization flag '-fwhole-program' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fcaller-saves' is not supported
 // CHECK-WARNING-DAG: optimization flag '-freorder-blocks' is not supported
-// CHECK-WARNING-DAG: optimization flag '-ffat-lto-objects' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fmerge-constants' is not supported
 // CHECK-WARNING-DAG: optimization flag '-finline-small-functions' is not supported
 // CHECK-WARNING-DAG: optimization flag '-ftree-dce' is not supported
Index: clang/test/CodeGen/embed-lto-fatlto.c
===
--- /dev/null
+++ clang/test/CodeGen/embed-lto-fatlto.c
@@ -0,0 +1,9 @@
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -S -flto=full -ffat-lto-objects -emit-llvm < %s  | FileCheck %s
+// RUN: %clang -cc1 -triple x86_64-unknown-linux-gnu -S -flto=full -ffat-lto-objects -emit-llvm < %s  | FileCheck %s
+//
+// CHECK: !{{[0-9]+}} = !{i32 1, !"ThinLTO", i32 0}
+// CHECK: !{{[0-9]+}} = !{i32 1, !"EnableSplitLTOUnit", i32 1}
+
+int test(void) {
+  return 0xabcd;
+}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7262,6 +7262,14 @@
 }
   }
 
+  if (IsUsingLTO && Args.getLastArg(options::OPT_ffat_lto_objects)) {
+assert(LTOMode == LTOK_Full || LTOMode == LTOK_Thin);
+CmdArgs.push_back(Args.MakeArgString(
+Twine("-flto=") + (LTOMode == LTOK_Thin ? "thin" : "full")));
+CmdArgs.push_back("-flto-unit");
+CmdArgs.push_back("-ffat-lto-objects");
+  }
+
   if (Args.hasArg(options::OPT_forder_file_instrumentation)) {
  CmdArgs.push_back("-forder-file-instrumentation");
  // Enable order file instrumentation when ThinLTO is not on. When ThinLTO is
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4641,8 +4641,13 @@
   }
   case phases::Backend: {
 if (isUsingLTO() && TargetDeviceOffloadKind == Action::OFK_None) {
-  types::ID Output =
-  Args.hasArg(options::OPT_S) ? types::TY_LTO_IR : types::TY_LTO_BC;
+  types::ID Output;
+  if (Args.hasArg(options::OPT_S))
+Output = types::TY_LTO_IR;
+  else if (Args.hasArg(options::OPT_ffat_lto_objects))
+Output = types::TY_PP_Asm;
+  else
+Output = types::TY_LTO_BC;
   return C.MakeAction(Input, Output);
 }
 if (isUsingLTO(/* IsOffload */ true) &&
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -24,6 +24,7 @@
 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
 #include "llvm/Bitcode/BitcodeWriterPass.h"
+#include "llvm/Bitcode/EmbedBitcodePass.h"
 #include "llvm/CodeGen/RegAllocRegistry.h"
 #include "llvm/CodeGen/SchedulerRegistry.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
@@ -991,7 +992,10 @@
 MPM.addPass(InstrProfiling(*Options, false));
   });
 
-if (IsThinLTO) {
+if (CodeGenOpts.FatLTO) {
+  MPM = PB.buildFatLTODefaultPipeline(
+  Level, IsThinLTO, IsThinLTO || shouldEmitRegularLTOSummary());
+} else if (IsThinLTO) {
   MPM = PB.buildThinLTOPreLinkDefaultPipeline(Level);
 } else if (IsLTO) {
   MPM = 

[PATCH] D129835: [clang] adds a discardable attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

Yeah, in retrospect, I agree (and may need to clean up a few iterator types 
now, grr).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129835

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

[PATCH] D142907: LangRef: Add "dynamic" option to "denormal-fp-math"

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D142907#4288247 , @efriedma wrote:

> Given that, I don't follow the whole "merging" thing... we should just be 
> setting whatever mode is active.  The attribute setting should not depend on 
> whether the function is interposable.  If you have a ODR function, all 
> definitions must have a mode compatible with whatever mode will be used at 
> runtime.  If you have a non-ODR weak function, optimizations shouldn't 
> propagate that mode from the callee to the caller.

The point is to have code that works with either mode. The intent is to avoid 
duplicating code for an extremely marginal difference. Having to duplicate 
every library function for denormal flushing and ieee handling defeats the 
purpose. The specialization will be realized after linking / inlining


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

https://reviews.llvm.org/D142907

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

[PATCH] D142569: [OpenMP] Introduce kernel environment

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 515876.
tianshilei1992 added a comment.

update more tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142569

Files:
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/test/Headers/amdgcn_openmp_device_math_c.c
  clang/test/OpenMP/amdgcn_target_codegen.cpp
  clang/test/OpenMP/amdgpu_target_with_aligned_attribute.c
  clang/test/OpenMP/declare_target_codegen_globalization.cpp
  clang/test/OpenMP/nvptx_SPMD_codegen.cpp
  clang/test/OpenMP/nvptx_data_sharing.cpp
  clang/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_multi_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_nested_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_for_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_num_threads_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_proc_bind_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/nvptx_target_printf_codegen.c
  clang/test/OpenMP/nvptx_target_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
  
clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_simd_codegen.cpp
  clang/test/OpenMP/nvptx_teams_codegen.cpp
  clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
  clang/test/OpenMP/reduction_implicit_map.cpp
  clang/test/OpenMP/remarks_parallel_in_multiple_target_state_machines.c
  clang/test/OpenMP/remarks_parallel_in_target_state_machine.c
  clang/test/OpenMP/target_parallel_debug_codegen.cpp
  clang/test/OpenMP/target_parallel_for_debug_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/Transforms/IPO/OpenMPOpt.cpp
  llvm/test/Transforms/Attributor/reduced/aa_execution_domain_wrong_fn.ll
  llvm/test/Transforms/Attributor/value-simplify-local-remote.ll
  llvm/test/Transforms/OpenMP/add_attributes.ll
  llvm/test/Transforms/OpenMP/always_inline_device.ll
  llvm/test/Transforms/OpenMP/custom_state_machines.ll
  llvm/test/Transforms/OpenMP/custom_state_machines_pre_lto.ll
  llvm/test/Transforms/OpenMP/custom_state_machines_remarks.ll
  llvm/test/Transforms/OpenMP/deduplication_target.ll
  llvm/test/Transforms/OpenMP/get_hardware_num_threads_in_block_fold.ll
  llvm/test/Transforms/OpenMP/get_hardware_num_threads_in_block_fold_optnone.ll
  llvm/test/Transforms/OpenMP/global_constructor.ll
  llvm/test/Transforms/OpenMP/globalization_remarks.ll
  llvm/test/Transforms/OpenMP/gpu_state_machine_function_ptr_replacement.ll
  llvm/test/Transforms/OpenMP/is_spmd_exec_mode_fold.ll
  llvm/test/Transforms/OpenMP/nested_parallelism.ll
  llvm/test/Transforms/OpenMP/parallel_level_fold.ll
  llvm/test/Transforms/OpenMP/remove_globalization.ll
  llvm/test/Transforms/OpenMP/replace_globalization.ll
  llvm/test/Transforms/OpenMP/single_threaded_execution.ll
  llvm/test/Transforms/OpenMP/spmdization.ll
  llvm/test/Transforms/OpenMP/spmdization_assumes.ll
  llvm/test/Transforms/OpenMP/spmdization_guarding.ll
  llvm/test/Transforms/OpenMP/spmdization_guarding_two_reaching_kernels.ll
  llvm/test/Transforms/OpenMP/spmdization_no_guarding_two_reaching_kernels.ll
  llvm/test/Transforms/OpenMP/spmdization_remarks.ll
  llvm/test/Transforms/OpenMP/value-simplify-openmp-opt.ll
  llvm/test/Transforms/PhaseOrdering/openmp-opt-module.ll
  openmp/libomptarget/DeviceRTL/CMakeLists.txt
  openmp/libomptarget/DeviceRTL/include/Debug.h
  openmp/libomptarget/DeviceRTL/include/Interface.h
  openmp/libomptarget/DeviceRTL/include/State.h
  openmp/libomptarget/DeviceRTL/src/Configuration.cpp
  openmp/libomptarget/DeviceRTL/src/Debug.cpp
  openmp/libomptarget/DeviceRTL/src/Kernel.cpp
  openmp/libomptarget/DeviceRTL/src/State.cpp
  openmp/libomptarget/include/DeviceEnvironment.h
  openmp/libomptarget/include/Environment.h
  openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp
  openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
  openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h
  openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp
  openmp/libomptarget/plugins-nextgen/generic-elf-64bit/src/rtl.cpp
  openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
  openmp/libomptarget/plugins/cuda/src/rtl.cpp

___
cfe-commits mailing list
cfe-commits@lists.llvm.org

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Precommit CI found a relevant failure that should be addressed.




Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8593
 
+const FunctionProtoType *FnType = 
FD->getType()->castAs();
+if (FnType->isVolatile()) {

ilya-biryukov wrote:
> NIT: this version is simpler and more aligned with the code below.
> Also, could you move the `volatile` handling after the check for `const`?
> 
> 
> Additionally, we seem to recover in case of `const` by adding it to the type 
> and suggesting a fix-it to add it in the code.
> Could we do the same for `volatile` and `ref-qualifier`, i.e. suggest a 
> fix-it to remove the `ref-qualifier` and `volatile` and change the 
> corresponding type to make AST pretend they were never there?




Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8593-8601
+const FunctionProtoType *FnType = 
FD->getType()->castAs();
+if (FnType->isVolatile()) {
+  Diag(FD->getLocation(), diag::err_volatile_comparison_operator);
+  return true;
+}
+if (FnType->getRefQualifier() == RQ_RValue) {
+  Diag(FD->getLocation(), diag::err_ref_qualifier_comparison_operator);

aaron.ballman wrote:
> ilya-biryukov wrote:
> > NIT: this version is simpler and more aligned with the code below.
> > Also, could you move the `volatile` handling after the check for `const`?
> > 
> > 
> > Additionally, we seem to recover in case of `const` by adding it to the 
> > type and suggesting a fix-it to add it in the code.
> > Could we do the same for `volatile` and `ref-qualifier`, i.e. suggest a 
> > fix-it to remove the `ref-qualifier` and `volatile` and change the 
> > corresponding type to make AST pretend they were never there?
> 
Note, you can shorten it further with:
```
return Diag(...);
```
because that will return true for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148924

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

[PATCH] D148951: [dataflow] HTMLLogger: fix off-by-one in the BB listing

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Before: F27217511: image.png 
After: F27217508: image.png 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148951

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

[PATCH] D148951: [dataflow] HTMLLogger: fix off-by-one in the BB listing

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: mboehme.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
sammccall requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The indexes of analysis state within a BB element is a bit odd:

  BB.0 is the initial state
  BB.1 is the state after the first element

etc

This means we have N+1 states and we need N+1 elements in the BB list.
We add a dummy element at the beginning rather than the end, because we want
selecting a CFG element to show the state *afterwards*.
For example, if we click on an expr, we want to be able to see its value model!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148951

Files:
  clang/lib/Analysis/FlowSensitive/HTMLLogger.html


Index: clang/lib/Analysis/FlowSensitive/HTMLLogger.html
===
--- clang/lib/Analysis/FlowSensitive/HTMLLogger.html
+++ clang/lib/Analysis/FlowSensitive/HTMLLogger.html
@@ -35,9 +35,15 @@
   
 
 
+
+  
+{{selection.bb}}.0
+(initial state)
+  
+
 
-  
-{{selection.bb}}.{{elt_index}}
+  
+{{selection.bb}}.{{elt_index+1}}
 {{elt}}
   
 


Index: clang/lib/Analysis/FlowSensitive/HTMLLogger.html
===
--- clang/lib/Analysis/FlowSensitive/HTMLLogger.html
+++ clang/lib/Analysis/FlowSensitive/HTMLLogger.html
@@ -35,9 +35,15 @@
   
 
 
+
+  
+{{selection.bb}}.0
+(initial state)
+  
+
 
-  
-{{selection.bb}}.{{elt_index}}
+  
+{{selection.bb}}.{{elt_index+1}}
 {{elt}}
   
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148276: [clang] trigger -Wcast-qual on functional casts

[Thurston Dang via Phabricator via cfe-commits]

thurston added a comment.

Thanks Aaron and Jorge for the quick revert and replies! :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148276

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

[clang] fa2a8c2 - [dataflow] Don't crash in Environment::dump when decls have weird names

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2023-04-21T21:08:26+02:00
New Revision: fa2a8c2e1cdf188ce985d69fca6f78866390b715

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

LOG: [dataflow] Don't crash in Environment::dump when decls have weird names

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 446178a2a5eae..35988af6d5bc6 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -790,7 +790,7 @@ void Environment::dump(raw_ostream ) const {
   // fields are printed.
   OS << "DeclToLoc:\n";
   for (auto [D, L] : DeclToLoc)
-OS << "  [" << D->getName() << ", " << L << "]\n";
+OS << "  [" << D->getNameAsString() << ", " << L << "]\n";
 
   OS << "ExprToLoc:\n";
   for (auto [E, L] : ExprToLoc)



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

[PATCH] D148949: [dataflow] HTMLLogger - show the value of the current expr

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
sammccall requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148949

Files:
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
  clang/lib/Analysis/FlowSensitive/HTMLLogger.css
  clang/lib/Analysis/FlowSensitive/HTMLLogger.html
  clang/lib/Analysis/FlowSensitive/HTMLLogger.js
  clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/LoggerTest.cpp
@@ -176,6 +176,7 @@
   << "has analysis point state";
   EXPECT_THAT(Logs[0], HasSubstr("transferBranch(0)")) << "has analysis logs";
   EXPECT_THAT(Logs[0], HasSubstr("LocToVal")) << "has built-in lattice dump";
+  EXPECT_THAT(Logs[0], HasSubstr("\"type\": \"int\"")) << "has value dump";
 }
 
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/HTMLLogger.js
===
--- clang/lib/Analysis/FlowSensitive/HTMLLogger.js
+++ clang/lib/Analysis/FlowSensitive/HTMLLogger.js
@@ -73,6 +73,9 @@
 }
 return parent.insertBefore(clone, next);
   }
+  // data-use="xyz": use  instead. (Allows recursion.)
+  if ('use' in tmpl.dataset)
+return inflate(document.getElementById(tmpl.dataset.use), data, parent, next);
   //  tag handling. Base case: recursively inflate.
   function handle(data) {
 for (c of tmpl.content.childNodes)
Index: clang/lib/Analysis/FlowSensitive/HTMLLogger.html
===
--- clang/lib/Analysis/FlowSensitive/HTMLLogger.html
+++ clang/lib/Analysis/FlowSensitive/HTMLLogger.html
@@ -10,6 +10,30 @@
 
 
 
+
+
+  
+
+  {{v.kind}}
+#{{v.value_id}}
+  
+  
+{{v.type}} @{{v.location}}
+  
+
+
+  {{kv[0]}}
+{{kv[1]}}
+
+  
+
+  
+
+  
+
+
 
 
 
@@ -50,6 +74,10 @@
   {{state.block}} (iteration {{state.iter}}) initial state
   Element {{selection.elt}} (iteration {{state.iter}})
 
+
+  Value
+  
+
 
   Logs
   {{state.logs}}
Index: clang/lib/Analysis/FlowSensitive/HTMLLogger.css
===
--- clang/lib/Analysis/FlowSensitive/HTMLLogger.css
+++ clang/lib/Analysis/FlowSensitive/HTMLLogger.css
@@ -116,3 +116,27 @@
   position: relative;
   margin-left: 0.5em;
 }
+
+.value {
+  border: 1px solid #888;
+  font-size: x-small;
+  flex-grow: 1;
+}
+.value summary {
+  background-color: #ace;
+  display: flex;
+  justify-content: space-between;
+}
+.value .address {
+  font-size: xx-small;
+  font-family: monospace;
+  color: #888;
+}
+.value .property {
+  display: flex;
+  margin-top: 0.5em;
+}
+.value .property .key {
+  font-weight: bold;
+  min-width: 5em;
+}
Index: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
===
--- clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
+++ clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp
@@ -67,6 +67,7 @@
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/Program.h"
+#include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/raw_ostream.h"
 // Defines assets: HTMLLogger_{html_js,css}
 #include "HTMLLogger.inc"
@@ -79,6 +80,94 @@
 
 using StreamFactory = std::function()>;
 
+// Recursively dumps Values/StorageLocations as JSON
+class ModelDumper {
+public:
+  ModelDumper(llvm::json::OStream , const Environment )
+  : JOS(JOS), Env(Env) {}
+
+  void dump(Value ) {
+JOS.attribute("value_id", llvm::to_string());
+if (!Visited.insert().second)
+  return;
+
+JOS.attribute("kind", debugString(V.getKind()));
+for (const auto& Prop : V.properties())
+  JOS.attributeObject(Prop.first(), [&] { dump(*Prop.second); });
+
+// Running the SAT solver is expensive, but knowing which booleans are
+// guaranteed true/false here is valuable and hard to determine by hand.
+if (auto *B = llvm::dyn_cast()) {
+  JOS.attribute("truth", Env.flowConditionImplies(*B) ? "true"
+ : Env.flowConditionImplies(Env.makeNot(*B))
+ ? "false"
+ : "unknown");
+}
+
+switch (V.getKind()) {
+case Value::Kind::Integer:
+case Value::Kind::TopBool:
+case Value::Kind::AtomicBool:
+  break;
+case Value::Kind::Reference:
+  JOS.attributeObject(
+  "referent", [&] { dump(cast(V).getReferentLoc()); });
+  break;
+case Value::Kind::Pointer:
+  JOS.attributeObject(
+  

[PATCH] D148276: [clang] trigger -Wcast-qual on functional casts

[Jorge Pinto Sousa via Phabricator via cfe-commits]

sousajo added a comment.

In D148276#4288236 , @aaron.ballman 
wrote:

> In D148276#4288145 , @sousajo wrote:
>
>> hey :( I will try to investigate it a bit sometime next week ^^ thanks for 
>> pointing it out
>
> Thank you for looking into it! I didn't think about `remove_const`, 
> `remove_cv`, etc. when considering test cases, sorry about that!

No worries :) Ill add those and try to craft a fix. If I am really lost I also 
let you know ^^


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148276

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

[PATCH] D129835: [clang] adds a discardable attribute

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D129835#4288240 , @aaron.ballman 
wrote:

> In D129835#4287866 , @cjdb wrote:
>
 With [[discardable]] one just needs to push/pop at the extremes of a file 
 (and if a future version of module maps supports global pragmas for a 
 module, there too, but that's a discussion that requires a design doc).
>>>
>>> I understood that, I just don't think that's a good thing. This is 
>>> basically an attribute that says "I know we said we wanted everything here 
>>> to be nodiscard, but JUST KIDDING not this one!" which is not a very clean 
>>> approach to writing headers.
>>
>> @aaron.ballman Ugh, I've finally come up with a good use-case for 
>> `[[discardable]]`.
>>
>>   class [[nodiscard]] iterator {
>>   public:
>> // usual iterator stuff
>>   
>> [[clang::discardable]] iterator operator++(int);
>> [[clang::discardable]] iterator operator--(int);
>>   };
>>
>> I hate it, but the alternative is to decorate everything else with 
>> `[[nodiscard]]` just to facilitate these two operators. It's a compelling 
>> use-case, but I'm not sure if it's compelling enough on its own. WDYT?
>> (I personally think that those two should be nodiscard, but `i++` is 
>> pervasive enough that it might never be practical to correct.)
>
> I'm still not super motivated because I think this encourages a confused 
> design, because marking a *type* as `nodiscard` is making a promise about the 
> type as a whole. That asserts there's *never* a valid time to ignore a value 
> of the type. If you have times when that type is reasonable to ignore, then 
> the type isn't actually non-discardable, and you should mark the methods 
> returning the type. (IOW, "nodiscard" is part of the contract for the type in 
> some limited ways even if it's not reflected by the type system.)
>
> What's more, I think `iterator` is a case where it'd be a mistake to mark the 
> type `nodiscard`, because there are plenty times when it's valid to ignore 
> the type (consider many of the functions in ``, like 
> `std::copy()`). I think it's more appropriate to mark the functions returning 
> an iterator rather than the type itself.

I'm a little more sympathetic to the use than Aaron here, but I'm still at 
'unmotivated' here.  As he said, `iterator` I think is a poor candidate for 
`nodiscard`, as the iterator type itself is not the 'costly to ignore' part 
here, just the `begin`/`end` methods (and perhaps some of algorithms?).  I can 
see some logic to having this in very limited situations, but I just don't see 
a motivating use case yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129835

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

[PATCH] D148835: [clang] removes trailing whitespace

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

In D148835#4288151 , @erichkeane 
wrote:

> In D148835#4288148 , @cjdb wrote:
>
>> In D148835#4286924 , 
>> @aaron.ballman wrote:
>>
>>> In D148835#4286905 , @erichkeane 
>>> wrote:
>>>
 In D148835#4284871 , @cjdb wrote:

> I've had some very good input about why this probably shouldn't go ahead: 
> git history erasure :')

 While that is a common criticism, if we're going to do this at all, we 
 should do it in a single patch, as we can use --ignore-rev: 
 https://akrabat.com/ignoring-revisions-with-git-blame/

 This is a pretty common thing to do, and I don't think it should prevent 
 us from doing this, which I think is the 'greater good' here.  The 
 alternative is to continue having a bunch of unrelated patches piece-meal 
 'erase' git history as people accidentally fix these.
>>>
>>> We already ignore blame revisions like this today: 
>>> https://github.com/llvm/llvm-project/blob/main/.git-blame-ignore-revs
>>
>> Oh cool, I had no idea this was possible :)
>>
>>> However, I'd ask that we hold off on this change unless we have some 
>>> tooling in place that rejects new additions of trailing whitespace, 
>>> otherwise we're going to end up in this exact same situation again in 
>>> another week or two. When CI starts failing on patches that insert *new* 
>>> trailing whitespace, then I'm all for this change because we can fix it 
>>> once and hopefully keep it fixed.
>>
>> Since Clang doesn't have pre-commit CI, how can we meaningfully progress 
>> here?
>
> Phab at least has Pre-commit CI, i would expect that to be sufficient? It 
> would at least cut down accidential whitespace by orders of magnitude.

Yes, isn't this already covered by the existing use of `clang-format` on upload 
to phabricator (`arc diff`) ?At least for me it always gives me warnings if 
I try to upload anything that doesn't match clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148835

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

[PATCH] D142907: LangRef: Add "dynamic" option to "denormal-fp-math"

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I'm having trouble understanding the changes on the clang side.

If I'm following correctly; the "denormal-fp-math" setting is a promise from 
the user to the compiler: if the setting is not "dynamic", the user promises 
that the definition will only execute in the specified denormal mode.  This is 
similar to, for example, the rounding mode pragmas: the user promises a 
specific rounding mode unless they explicitly request dynamic rounding.

Given that, I don't follow the whole "merging" thing... we should just be 
setting whatever mode is active.  The attribute setting should not depend on 
whether the function is interposable.  If you have a ODR function, all 
definitions must have a mode compatible with whatever mode will be used at 
runtime.  If you have a non-ODR weak function, optimizations shouldn't 
propagate that mode from the callee to the caller.


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

https://reviews.llvm.org/D142907

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

[PATCH] D148944: [clang][Driver] Fix crash with unsupported architectures in MinGW and CrossWindows.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

Thank you for this fix. Can you please add a summary in the details explaining 
the motivation for this fix and a link to the github bug report as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148944

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

[PATCH] D129835: [clang] adds a discardable attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: erichkeane.
aaron.ballman added a comment.

In D129835#4287866 , @cjdb wrote:

>>> With [[discardable]] one just needs to push/pop at the extremes of a file 
>>> (and if a future version of module maps supports global pragmas for a 
>>> module, there too, but that's a discussion that requires a design doc).
>>
>> I understood that, I just don't think that's a good thing. This is basically 
>> an attribute that says "I know we said we wanted everything here to be 
>> nodiscard, but JUST KIDDING not this one!" which is not a very clean 
>> approach to writing headers.
>
> @aaron.ballman Ugh, I've finally come up with a good use-case for 
> `[[discardable]]`.
>
>   class [[nodiscard]] iterator {
>   public:
> // usual iterator stuff
>   
> [[clang::discardable]] iterator operator++(int);
> [[clang::discardable]] iterator operator--(int);
>   };
>
> I hate it, but the alternative is to decorate everything else with 
> `[[nodiscard]]` just to facilitate these two operators. It's a compelling 
> use-case, but I'm not sure if it's compelling enough on its own. WDYT?
> (I personally think that those two should be nodiscard, but `i++` is 
> pervasive enough that it might never be practical to correct.)

I'm still not super motivated because I think this encourages a confused 
design, because marking a *type* as `nodiscard` is making a promise about the 
type as a whole. That asserts there's *never* a valid time to ignore a value of 
the type. If you have times when that type is reasonable to ignore, then the 
type isn't actually non-discardable, and you should mark the methods returning 
the type. (IOW, "nodiscard" is part of the contract for the type in some 
limited ways even if it's not reflected by the type system.)

What's more, I think `iterator` is a case where it'd be a mistake to mark the 
type `nodiscard`, because there are plenty times when it's valid to ignore the 
type (consider many of the functions in ``, like `std::copy()`). I 
think it's more appropriate to mark the functions returning an iterator rather 
than the type itself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129835

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

[PATCH] D148276: [clang] trigger -Wcast-qual on functional casts

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D148276#4288145 , @sousajo wrote:

> hey :( I will try to investigate it a bit sometime next week ^^ thanks for 
> pointing it out

Thank you for looking into it! I didn't think about `remove_const`, 
`remove_cv`, etc. when considering test cases, sorry about that!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148276

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

[PATCH] D142569: [OpenMP] Introduce kernel environment

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 updated this revision to Diff 515854.
tianshilei1992 added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142569

Files:
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/test/Headers/amdgcn_openmp_device_math_c.c
  clang/test/OpenMP/amdgcn_target_codegen.cpp
  clang/test/OpenMP/declare_target_codegen_globalization.cpp
  clang/test/OpenMP/nvptx_SPMD_codegen.cpp
  clang/test/OpenMP/nvptx_data_sharing.cpp
  clang/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_multi_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_nested_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_for_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_num_threads_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_proc_bind_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/nvptx_target_printf_codegen.c
  clang/test/OpenMP/nvptx_target_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
  
clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_simd_codegen.cpp
  clang/test/OpenMP/nvptx_teams_codegen.cpp
  clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
  clang/test/OpenMP/reduction_implicit_map.cpp
  clang/test/OpenMP/remarks_parallel_in_multiple_target_state_machines.c
  clang/test/OpenMP/remarks_parallel_in_target_state_machine.c
  clang/test/OpenMP/target_parallel_debug_codegen.cpp
  clang/test/OpenMP/target_parallel_for_debug_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/lib/Transforms/IPO/OpenMPOpt.cpp
  llvm/test/Transforms/Attributor/reduced/aa_execution_domain_wrong_fn.ll
  llvm/test/Transforms/Attributor/value-simplify-local-remote.ll
  llvm/test/Transforms/OpenMP/add_attributes.ll
  llvm/test/Transforms/OpenMP/always_inline_device.ll
  llvm/test/Transforms/OpenMP/custom_state_machines.ll
  llvm/test/Transforms/OpenMP/custom_state_machines_pre_lto.ll
  llvm/test/Transforms/OpenMP/custom_state_machines_remarks.ll
  llvm/test/Transforms/OpenMP/deduplication_target.ll
  llvm/test/Transforms/OpenMP/get_hardware_num_threads_in_block_fold.ll
  llvm/test/Transforms/OpenMP/get_hardware_num_threads_in_block_fold_optnone.ll
  llvm/test/Transforms/OpenMP/global_constructor.ll
  llvm/test/Transforms/OpenMP/globalization_remarks.ll
  llvm/test/Transforms/OpenMP/gpu_state_machine_function_ptr_replacement.ll
  llvm/test/Transforms/OpenMP/is_spmd_exec_mode_fold.ll
  llvm/test/Transforms/OpenMP/nested_parallelism.ll
  llvm/test/Transforms/OpenMP/parallel_level_fold.ll
  llvm/test/Transforms/OpenMP/remove_globalization.ll
  llvm/test/Transforms/OpenMP/replace_globalization.ll
  llvm/test/Transforms/OpenMP/single_threaded_execution.ll
  llvm/test/Transforms/OpenMP/spmdization.ll
  llvm/test/Transforms/OpenMP/spmdization_assumes.ll
  llvm/test/Transforms/OpenMP/spmdization_guarding.ll
  llvm/test/Transforms/OpenMP/spmdization_guarding_two_reaching_kernels.ll
  llvm/test/Transforms/OpenMP/spmdization_no_guarding_two_reaching_kernels.ll
  llvm/test/Transforms/OpenMP/spmdization_remarks.ll
  llvm/test/Transforms/OpenMP/value-simplify-openmp-opt.ll
  llvm/test/Transforms/PhaseOrdering/openmp-opt-module.ll
  openmp/libomptarget/DeviceRTL/CMakeLists.txt
  openmp/libomptarget/DeviceRTL/include/Debug.h
  openmp/libomptarget/DeviceRTL/include/Interface.h
  openmp/libomptarget/DeviceRTL/include/State.h
  openmp/libomptarget/DeviceRTL/src/Configuration.cpp
  openmp/libomptarget/DeviceRTL/src/Debug.cpp
  openmp/libomptarget/DeviceRTL/src/Kernel.cpp
  openmp/libomptarget/DeviceRTL/src/State.cpp
  openmp/libomptarget/include/DeviceEnvironment.h
  openmp/libomptarget/include/Environment.h
  openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp
  openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
  openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h
  openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp
  openmp/libomptarget/plugins-nextgen/generic-elf-64bit/src/rtl.cpp
  openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
  openmp/libomptarget/plugins/cuda/src/rtl.cpp

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

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

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/test/CXX/class/class.compare/class.compare.default/p1.cpp:18
   bool operator<=(const A&) const = default;
-  bool operator==(const A&) const volatile && = default; // surprisingly, OK
+  bool operator==(const A&) const && = default; // expected-error 
{{ref-qualifier '&&' is not allowed on defaulted comparison operators}}
+  bool operator>=(const A&) const volatile = default; // expected-error 
{{defaulted comparison operator function must not be volatile}}

Can we also have examples with out of line defaulting. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148924

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

[PATCH] D148835: [clang] removes trailing whitespace

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D148835#4288148 , @cjdb wrote:

> In D148835#4286924 , @aaron.ballman 
> wrote:
>
>> In D148835#4286905 , @erichkeane 
>> wrote:
>>
>>> In D148835#4284871 , @cjdb wrote:
>>>
 I've had some very good input about why this probably shouldn't go ahead: 
 git history erasure :')
>>>
>>> While that is a common criticism, if we're going to do this at all, we 
>>> should do it in a single patch, as we can use --ignore-rev: 
>>> https://akrabat.com/ignoring-revisions-with-git-blame/
>>>
>>> This is a pretty common thing to do, and I don't think it should prevent us 
>>> from doing this, which I think is the 'greater good' here.  The alternative 
>>> is to continue having a bunch of unrelated patches piece-meal 'erase' git 
>>> history as people accidentally fix these.
>>
>> We already ignore blame revisions like this today: 
>> https://github.com/llvm/llvm-project/blob/main/.git-blame-ignore-revs
>
> Oh cool, I had no idea this was possible :)
>
>> However, I'd ask that we hold off on this change unless we have some tooling 
>> in place that rejects new additions of trailing whitespace, otherwise we're 
>> going to end up in this exact same situation again in another week or two. 
>> When CI starts failing on patches that insert *new* trailing whitespace, 
>> then I'm all for this change because we can fix it once and hopefully keep 
>> it fixed.
>
> Since Clang doesn't have pre-commit CI, how can we meaningfully progress here?

Phab at least has Pre-commit CI, i would expect that to be sufficient? It would 
at least cut down accidential whitespace by orders of magnitude.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148835

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

[PATCH] D147888: Update declaration message of extern linkage

[Krishna Narayanan via Phabricator via cfe-commits]

Krishna-13-cyber updated this revision to Diff 515843.
Krishna-13-cyber added a comment.

I have tried a little modification from my side thinking on a beneficial note. 
I will make the changes to all the other test files as well if this diagnostic 
representation goes well after mentor review.

For refactoring and restructuring the whole of the re-declaration would need 
some time I have been through it and would initiate another patch for that,In 
the concern of giving or having just one diagnostic for getting all cases of 
re-declaration would also need multiple conditional or switch statements inside 
our function block.At present we have the same with conditional statements 
taking care of each linkage but it has multiple permutations of diagnostic 
messages which is nice but can be improved.GCC differs only for this case of 
extern linkage which can be better/precise in clang where as others seem to be 
more precise in clang than latter as I worked out with good number of test 
cases regarding .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147888

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCXX/extern_static.cpp


Index: clang/test/SemaCXX/extern_static.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/extern_static.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 -Wabstract-vbase-init
+void f(void)
+{
+static int x; // expected-note {{previous definition is here}}
+extern int x; // expected-error {{extern declaration of 'x' follows static 
declaration;Please pick exactly one (static or extern)}}
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -4637,6 +4637,15 @@
   //   the prior declaration. If no prior declaration is visible, or
   //   if the prior declaration specifies no linkage, then the
   //   identifier has external linkage.
+
+  if (New->hasExternalStorage() &&
+  Old->getCanonicalDecl()->getStorageClass() == SC_Static &&
+  Old->isLocalVarDeclOrParm()) {
+Diag(New->getLocation(), diag::err_extern_static) << New->getDeclName();
+Diag(OldLocation, PrevDiag);
+return New->setInvalidDecl();
+  }
+
   if (New->hasExternalStorage() && Old->hasLinkage())
 /* Okay */;
   else if (New->getCanonicalDecl()->getStorageClass() != SC_Static &&
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5798,11 +5798,13 @@
   "redeclaring non-static %0 as static is a Microsoft extension">,
   InGroup;
 def err_non_static_static : Error<
-  "non-static declaration of %0 follows static declaration">;
+  "non-static declaration of %0 follows static declaration;Please pick exactly 
one (static or non-static)">;
+def err_extern_static : Error<
+  "extern declaration of %0 follows static declaration;Please pick exactly one 
(static or extern)">;
 def err_extern_non_extern : Error<
-  "extern declaration of %0 follows non-extern declaration">;
+  "extern declaration of %0 follows non-extern declaration;Please pick exactly 
one (non-extern or extern)">;
 def err_non_extern_extern : Error<
-  "non-extern declaration of %0 follows extern declaration">;
+  "non-extern declaration of %0 follows extern declaration;Please pick exactly 
one (extern or non-extern)">;
 def err_non_thread_thread : Error<
   "non-thread-local declaration of %0 follows thread-local declaration">;
 def err_thread_non_thread : Error<


Index: clang/test/SemaCXX/extern_static.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/extern_static.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 -Wabstract-vbase-init
+void f(void)
+{
+static int x; // expected-note {{previous definition is here}}
+extern int x; // expected-error {{extern declaration of 'x' follows static declaration;Please pick exactly one (static or extern)}}
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -4637,6 +4637,15 @@
   //   the prior declaration. If no prior declaration is visible, or
   //   if the prior declaration specifies no linkage, then the
   //   identifier has external linkage.
+
+  if (New->hasExternalStorage() &&
+  Old->getCanonicalDecl()->getStorageClass() == SC_Static &&
+  Old->isLocalVarDeclOrParm()) {
+Diag(New->getLocation(), diag::err_extern_static) << New->getDeclName();
+Diag(OldLocation, PrevDiag);
+

[PATCH] D148835: [clang] removes trailing whitespace

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

In D148835#4286924 , @aaron.ballman 
wrote:

> In D148835#4286905 , @erichkeane 
> wrote:
>
>> In D148835#4284871 , @cjdb wrote:
>>
>>> I've had some very good input about why this probably shouldn't go ahead: 
>>> git history erasure :')
>>
>> While that is a common criticism, if we're going to do this at all, we 
>> should do it in a single patch, as we can use --ignore-rev: 
>> https://akrabat.com/ignoring-revisions-with-git-blame/
>>
>> This is a pretty common thing to do, and I don't think it should prevent us 
>> from doing this, which I think is the 'greater good' here.  The alternative 
>> is to continue having a bunch of unrelated patches piece-meal 'erase' git 
>> history as people accidentally fix these.
>
> We already ignore blame revisions like this today: 
> https://github.com/llvm/llvm-project/blob/main/.git-blame-ignore-revs

Oh cool, I had no idea this was possible :)

> However, I'd ask that we hold off on this change unless we have some tooling 
> in place that rejects new additions of trailing whitespace, otherwise we're 
> going to end up in this exact same situation again in another week or two. 
> When CI starts failing on patches that insert *new* trailing whitespace, then 
> I'm all for this change because we can fix it once and hopefully keep it 
> fixed.

Since Clang doesn't have pre-commit CI, how can we meaningfully progress here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148835

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

[PATCH] D148276: [clang] trigger -Wcast-qual on functional casts

[Jorge Pinto Sousa via Phabricator via cfe-commits]

sousajo added a comment.

hey :( I will try to investigate it a bit sometime next week ^^ thanks for 
pointing it out


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148276

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

[PATCH] D148757: [clang] Follow user-provided order for prefix map

[Petr Hosek via Phabricator via cfe-commits]

phosek accepted this revision.
phosek 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/D148757/new/

https://reviews.llvm.org/D148757

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

[PATCH] D148945: [clang] Remove workaround for old LLVM_ENABLE_PROJECTS=libcxx build

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Related: D122444 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148945

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

[PATCH] D148945: [clang] Remove workaround for old LLVM_ENABLE_PROJECTS=libcxx build

[Petr Hosek via Phabricator via cfe-commits]

phosek accepted this revision.
phosek 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/D148945/new/

https://reviews.llvm.org/D148945

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

[PATCH] D145197: [clang][deps] NFC: Refactor and comment ModuleDeps sorting

[Jan Svoboda via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6b80f00bac6a: [clang][deps] NFC: Refactor and comment 
ModuleDeps sorting (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145197

Files:
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -323,11 +323,10 @@
   return llvm::json::Array(Strings);
 }
 
+// Technically, we don't need to sort the dependency list to get determinism.
+// Leaving these be will simply preserve the import order.
 static llvm::json::Array toJSONSorted(std::vector V) {
-  llvm::sort(V, [](const ModuleID , const ModuleID ) {
-return std::tie(A.ModuleName, A.ContextHash) <
-   std::tie(B.ModuleName, B.ContextHash);
-  });
+  llvm::sort(V);
 
   llvm::json::Array Ret;
   for (const ModuleID  : V)
@@ -406,11 +405,7 @@
 std::vector ModuleIDs;
 for (auto & : Modules)
   ModuleIDs.push_back(M.first);
-llvm::sort(ModuleIDs,
-   [](const IndexedModuleID , const IndexedModuleID ) {
- return std::tie(A.ID.ModuleName, A.InputIndex) <
-std::tie(B.ID.ModuleName, B.InputIndex);
-   });
+llvm::sort(ModuleIDs);
 
 using namespace llvm::json;
 
@@ -470,20 +465,37 @@
 private:
   struct IndexedModuleID {
 ModuleID ID;
+
+// FIXME: This is mutable so that it can still be updated after insertion
+//  into an unordered associative container. This is "fine", since this
+//  field doesn't contribute to the hash, but it's a brittle hack.
 mutable size_t InputIndex;
 
 bool operator==(const IndexedModuleID ) const {
-  return ID.ModuleName == Other.ID.ModuleName &&
- ID.ContextHash == Other.ID.ContextHash;
+  return std::tie(ID.ModuleName, ID.ContextHash) ==
+ std::tie(Other.ID.ModuleName, Other.ID.ContextHash);
 }
-  };
-
-  struct IndexedModuleIDHasher {
-std::size_t operator()(const IndexedModuleID ) const {
-  using llvm::hash_combine;
 
-  return hash_combine(IMID.ID.ModuleName, IMID.ID.ContextHash);
+bool operator<(const IndexedModuleID ) const {
+  /// We need the output of clang-scan-deps to be deterministic. However,
+  /// the dependency graph may contain two modules with the same name. How
+  /// do we decide which one to print first? If we made that decision based
+  /// on the context hash, the ordering would be deterministic, but
+  /// different across machines. This can happen for example when the inputs
+  /// or the SDKs (which both contribute to the "context" hash) live in
+  /// different absolute locations. We solve that by tracking the index of
+  /// the first input TU that (transitively) imports the dependency, which
+  /// is always the same for the same input, resulting in deterministic
+  /// sorting that's also reproducible across machines.
+  return std::tie(ID.ModuleName, InputIndex) <
+ std::tie(Other.ID.ModuleName, Other.InputIndex);
 }
+
+struct Hasher {
+  std::size_t operator()(const IndexedModuleID ) const {
+return llvm::hash_combine(IMID.ID.ModuleName, IMID.ID.ContextHash);
+  }
+};
   };
 
   struct InputDeps {
@@ -496,7 +508,7 @@
   };
 
   std::mutex Lock;
-  std::unordered_map
+  std::unordered_map
   Modules;
   std::vector Inputs;
 };
Index: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
===
--- clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -59,7 +59,13 @@
   std::string ContextHash;
 
   bool operator==(const ModuleID ) const {
-return ModuleName == Other.ModuleName && ContextHash == Other.ContextHash;
+return std::tie(ModuleName, ContextHash) ==
+   std::tie(Other.ModuleName, Other.ContextHash);
+  }
+
+  bool operator<(const ModuleID& Other) const {
+return std::tie(ModuleName, ContextHash) <
+   std::tie(Other.ModuleName, Other.ContextHash);
   }
 };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 6b80f00 - [clang][deps] NFC: Refactor and comment ModuleDeps sorting

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2023-04-21T10:56:42-07:00
New Revision: 6b80f00bac6a4832af4f8ebde96564690a35f7e1

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

LOG: [clang][deps] NFC: Refactor and comment ModuleDeps sorting

I once again stumbled across `IndexedModuleID` and was confused by the mutable 
`InputIndex` field.

This patch adds comments around that piece of code and unifies/simplifies 
related functions.

Reviewed By: Bigcheese

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

Added: 


Modified: 
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/tools/clang-scan-deps/ClangScanDeps.cpp

Removed: 




diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h 
b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index 24d7337d26e4c..0a5cbc4f046aa 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -59,7 +59,13 @@ struct ModuleID {
   std::string ContextHash;
 
   bool operator==(const ModuleID ) const {
-return ModuleName == Other.ModuleName && ContextHash == Other.ContextHash;
+return std::tie(ModuleName, ContextHash) ==
+   std::tie(Other.ModuleName, Other.ContextHash);
+  }
+
+  bool operator<(const ModuleID& Other) const {
+return std::tie(ModuleName, ContextHash) <
+   std::tie(Other.ModuleName, Other.ContextHash);
   }
 };
 

diff  --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp 
b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index 93c60eb2cdf59..9018acc8e682f 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -323,11 +323,10 @@ static llvm::json::Array toJSONSorted(const 
llvm::StringSet<> ) {
   return llvm::json::Array(Strings);
 }
 
+// Technically, we don't need to sort the dependency list to get determinism.
+// Leaving these be will simply preserve the import order.
 static llvm::json::Array toJSONSorted(std::vector V) {
-  llvm::sort(V, [](const ModuleID , const ModuleID ) {
-return std::tie(A.ModuleName, A.ContextHash) <
-   std::tie(B.ModuleName, B.ContextHash);
-  });
+  llvm::sort(V);
 
   llvm::json::Array Ret;
   for (const ModuleID  : V)
@@ -406,11 +405,7 @@ class FullDeps {
 std::vector ModuleIDs;
 for (auto & : Modules)
   ModuleIDs.push_back(M.first);
-llvm::sort(ModuleIDs,
-   [](const IndexedModuleID , const IndexedModuleID ) {
- return std::tie(A.ID.ModuleName, A.InputIndex) <
-std::tie(B.ID.ModuleName, B.InputIndex);
-   });
+llvm::sort(ModuleIDs);
 
 using namespace llvm::json;
 
@@ -470,20 +465,37 @@ class FullDeps {
 private:
   struct IndexedModuleID {
 ModuleID ID;
+
+// FIXME: This is mutable so that it can still be updated after insertion
+//  into an unordered associative container. This is "fine", since this
+//  field doesn't contribute to the hash, but it's a brittle hack.
 mutable size_t InputIndex;
 
 bool operator==(const IndexedModuleID ) const {
-  return ID.ModuleName == Other.ID.ModuleName &&
- ID.ContextHash == Other.ID.ContextHash;
+  return std::tie(ID.ModuleName, ID.ContextHash) ==
+ std::tie(Other.ID.ModuleName, Other.ID.ContextHash);
 }
-  };
-
-  struct IndexedModuleIDHasher {
-std::size_t operator()(const IndexedModuleID ) const {
-  using llvm::hash_combine;
 
-  return hash_combine(IMID.ID.ModuleName, IMID.ID.ContextHash);
+bool operator<(const IndexedModuleID ) const {
+  /// We need the output of clang-scan-deps to be deterministic. However,
+  /// the dependency graph may contain two modules with the same name. How
+  /// do we decide which one to print first? If we made that decision based
+  /// on the context hash, the ordering would be deterministic, but
+  /// 
diff erent across machines. This can happen for example when the inputs
+  /// or the SDKs (which both contribute to the "context" hash) live in
+  /// 
diff erent absolute locations. We solve that by tracking the index of
+  /// the first input TU that (transitively) imports the dependency, which
+  /// is always the same for the same input, resulting in deterministic
+  /// sorting that's also reproducible across machines.
+  return std::tie(ID.ModuleName, InputIndex) <
+ std::tie(Other.ID.ModuleName, Other.InputIndex);
 }
+
+struct Hasher {
+  std::size_t operator()(const IndexedModuleID ) const {
+return llvm::hash_combine(IMID.ID.ModuleName, IMID.ID.ContextHash);
+  }
+};
   };
 
   struct InputDeps {
@@ -496,7 

[PATCH] D148817: [RISCV] Add Tag_RISCV_arch attribute by default when using clang as an assembler.

[Craig Topper via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGea0dae096189: [RISCV] Add Tag_RISCV_arch attribute by 
default when using clang as an… (authored by craig.topper).

Changed prior to commit:
  https://reviews.llvm.org/D148817?vs=515511=515832#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148817

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/riscv-default-build-attributes.s


Index: clang/test/Driver/riscv-default-build-attributes.s
===
--- /dev/null
+++ clang/test/Driver/riscv-default-build-attributes.s
@@ -0,0 +1,29 @@
+ Enabled by default for assembly
+// RUN: %clang --target=riscv32 -### %s 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ENABLED
+// RUN: %clang --target=riscv64 -### %s 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ENABLED
+
+/// Can be forced on or off for assembly.
+// RUN: %clang --target=riscv32 -### %s 2>&1 -mno-default-build-attributes \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DISABLED
+// RUN: %clang --target=riscv64 -### %s 2>&1 -mno-default-build-attributes \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DISABLED
+// RUN: %clang --target=riscv32 -### %s 2>&1 -mdefault-build-attributes \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ENABLED
+// RUN: %clang --target=riscv64 -### %s 2>&1 -mdefault-build-attributes \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ENABLED
+
+/// Option ignored for C/C++ (since we always emit hardware and ABI build
+/// attributes during codegen).
+// RUN: %clang --target=riscv32 -### -x c %s -mdefault-build-attributes 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DISABLED
+// RUN: %clang --target=riscv64 -### -x c %s -mdefault-build-attributes 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DISABLED
+// RUN: %clang --target=riscv32 -### -x c++ %s -mdefault-build-attributes 2>&1 
\
+// RUN:   | FileCheck %s --check-prefix=CHECK-DISABLED
+// RUN: %clang --target=riscv64 -### -x c++ %s -mdefault-build-attributes 2>&1 
\
+// RUN:   | FileCheck %s --check-prefix=CHECK-DISABLED
+
+// CHECK-DISABLED-NOT: "-riscv-add-build-attributes"
+// CHECK-ENABLED: "-riscv-add-build-attributes"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7903,6 +7903,12 @@
 
   CmdArgs.push_back("-target-abi");
   CmdArgs.push_back(ABIName.data());
+
+  if (Args.hasFlag(options::OPT_mdefault_build_attributes,
+   options::OPT_mno_default_build_attributes, true)) {
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back("-riscv-add-build-attributes");
+  }
 }
 
 void ClangAs::ConstructJob(Compilation , const JobAction ,


Index: clang/test/Driver/riscv-default-build-attributes.s
===
--- /dev/null
+++ clang/test/Driver/riscv-default-build-attributes.s
@@ -0,0 +1,29 @@
+ Enabled by default for assembly
+// RUN: %clang --target=riscv32 -### %s 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ENABLED
+// RUN: %clang --target=riscv64 -### %s 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ENABLED
+
+/// Can be forced on or off for assembly.
+// RUN: %clang --target=riscv32 -### %s 2>&1 -mno-default-build-attributes \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DISABLED
+// RUN: %clang --target=riscv64 -### %s 2>&1 -mno-default-build-attributes \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DISABLED
+// RUN: %clang --target=riscv32 -### %s 2>&1 -mdefault-build-attributes \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ENABLED
+// RUN: %clang --target=riscv64 -### %s 2>&1 -mdefault-build-attributes \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ENABLED
+
+/// Option ignored for C/C++ (since we always emit hardware and ABI build
+/// attributes during codegen).
+// RUN: %clang --target=riscv32 -### -x c %s -mdefault-build-attributes 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DISABLED
+// RUN: %clang --target=riscv64 -### -x c %s -mdefault-build-attributes 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DISABLED
+// RUN: %clang --target=riscv32 -### -x c++ %s -mdefault-build-attributes 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DISABLED
+// RUN: %clang --target=riscv64 -### -x c++ %s -mdefault-build-attributes 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DISABLED
+
+// CHECK-DISABLED-NOT: "-riscv-add-build-attributes"
+// CHECK-ENABLED: "-riscv-add-build-attributes"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7903,6 +7903,12 @@
 
   

[clang] ea0dae0 - [RISCV] Add Tag_RISCV_arch attribute by default when using clang as an assembler.

[Craig Topper via cfe-commits]

Author: Craig Topper
Date: 2023-04-21T10:54:24-07:00
New Revision: ea0dae0961897da69b960caf68063db86312674d

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

LOG: [RISCV] Add Tag_RISCV_arch attribute by default when using clang as an 
assembler.

Can be disabled with -mno-default-build-attributes just like ARM.

Reviewed By: asb

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

Added: 
clang/test/Driver/riscv-default-build-attributes.s

Modified: 
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 7676cbe5e6c0e..56250fccfa855 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7903,6 +7903,12 @@ void ClangAs::AddRISCVTargetArgs(const ArgList ,
 
   CmdArgs.push_back("-target-abi");
   CmdArgs.push_back(ABIName.data());
+
+  if (Args.hasFlag(options::OPT_mdefault_build_attributes,
+   options::OPT_mno_default_build_attributes, true)) {
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back("-riscv-add-build-attributes");
+  }
 }
 
 void ClangAs::ConstructJob(Compilation , const JobAction ,

diff  --git a/clang/test/Driver/riscv-default-build-attributes.s 
b/clang/test/Driver/riscv-default-build-attributes.s
new file mode 100644
index 0..5d2e08371cb5d
--- /dev/null
+++ b/clang/test/Driver/riscv-default-build-attributes.s
@@ -0,0 +1,29 @@
+ Enabled by default for assembly
+// RUN: %clang --target=riscv32 -### %s 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ENABLED
+// RUN: %clang --target=riscv64 -### %s 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ENABLED
+
+/// Can be forced on or off for assembly.
+// RUN: %clang --target=riscv32 -### %s 2>&1 -mno-default-build-attributes \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DISABLED
+// RUN: %clang --target=riscv64 -### %s 2>&1 -mno-default-build-attributes \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DISABLED
+// RUN: %clang --target=riscv32 -### %s 2>&1 -mdefault-build-attributes \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ENABLED
+// RUN: %clang --target=riscv64 -### %s 2>&1 -mdefault-build-attributes \
+// RUN:   | FileCheck %s --check-prefix=CHECK-ENABLED
+
+/// Option ignored for C/C++ (since we always emit hardware and ABI build
+/// attributes during codegen).
+// RUN: %clang --target=riscv32 -### -x c %s -mdefault-build-attributes 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DISABLED
+// RUN: %clang --target=riscv64 -### -x c %s -mdefault-build-attributes 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-DISABLED
+// RUN: %clang --target=riscv32 -### -x c++ %s -mdefault-build-attributes 2>&1 
\
+// RUN:   | FileCheck %s --check-prefix=CHECK-DISABLED
+// RUN: %clang --target=riscv64 -### -x c++ %s -mdefault-build-attributes 2>&1 
\
+// RUN:   | FileCheck %s --check-prefix=CHECK-DISABLED
+
+// CHECK-DISABLED-NOT: "-riscv-add-build-attributes"
+// CHECK-ENABLED: "-riscv-add-build-attributes"



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

[PATCH] D146987: [Assignment Tracking] Enable by default

[Steven Johnson via Phabricator via cfe-commits]

srj added a comment.

In D146987#4287847 , @srj wrote:

> In D146987#4287819 , @jmorse wrote:
>
>> Should have been reverted in rG0ba922f60046 
>>  
>> earlier today, are you still experiencing the same behaviour with that patch?
>
> Ah, sorry, I didn't see that -- I'll pull and rebuild this morning to verify!

Looks good now, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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

[PATCH] D148945: [clang] Remove workaround for old LLVM_ENABLE_PROJECTS=libcxx build

[Louis Dionne via Phabricator via cfe-commits]

ldionne added subscribers: phosek, Ericson2314, aaron.ballman.
ldionne added a comment.

CCing some folks that were watching D132324 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148945

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

[PATCH] D148945: [clang] Remove workaround for old LLVM_ENABLE_PROJECTS=libcxx build

[Louis Dionne via Phabricator via cfe-commits]

ldionne created this revision.
Herald added a project: All.
ldionne requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

We don't support the LLVM_ENABLE_PROJECTS=libcxx build anymore (and have
not for over a year), so it should be fine to remove this workaround now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148945

Files:
  clang/lib/Driver/ToolChains/Linux.cpp


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -299,13 +299,6 @@
 
   Generic_GCC::AddMultiarchPaths(D, SysRoot, OSLibDir, Paths);
 
-  // The deprecated -DLLVM_ENABLE_PROJECTS=libcxx configuration installs
-  // libc++.so in D.Dir+"/../lib/". Detect this path.
-  // TODO Remove once LLVM_ENABLE_PROJECTS=libcxx is unsupported.
-  if (StringRef(D.Dir).startswith(SysRoot) &&
-  D.getVFS().exists(D.Dir + "/../lib/libc++.so"))
-addPathIfExists(D, D.Dir + "/../lib", Paths);
-
   addPathIfExists(D, concat(SysRoot, "/lib"), Paths);
   addPathIfExists(D, concat(SysRoot, "/usr/lib"), Paths);
 }


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -299,13 +299,6 @@
 
   Generic_GCC::AddMultiarchPaths(D, SysRoot, OSLibDir, Paths);
 
-  // The deprecated -DLLVM_ENABLE_PROJECTS=libcxx configuration installs
-  // libc++.so in D.Dir+"/../lib/". Detect this path.
-  // TODO Remove once LLVM_ENABLE_PROJECTS=libcxx is unsupported.
-  if (StringRef(D.Dir).startswith(SysRoot) &&
-  D.getVFS().exists(D.Dir + "/../lib/libc++.so"))
-addPathIfExists(D, D.Dir + "/../lib", Paths);
-
   addPathIfExists(D, concat(SysRoot, "/lib"), Paths);
   addPathIfExists(D, concat(SysRoot, "/usr/lib"), Paths);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137258: [clang] Optimize storage and lookup of analyzer options

[Jan Svoboda via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG53a4a2b45bb2: [clang] NFCI: Optimize storage and lookup of 
analyzer options (authored by jansvoboda11).

Changed prior to commit:
  https://reviews.llvm.org/D137258?vs=515455=515828#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137258

Files:
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h


Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===
--- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -260,9 +260,10 @@
 #undef ANALYZER_OPTION
 #undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE
 
-  // Create an array of all -analyzer-config command line options. Sort it in
-  // the constructor.
-  std::vector AnalyzerConfigCmdFlags = {
+  bool isUnknownAnalyzerConfig(llvm::StringRef Name) {
+static std::vector AnalyzerConfigCmdFlags = []() {
+  // Create an array of all -analyzer-config command line options.
+  std::vector AnalyzerConfigCmdFlags = {
 #define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC,
\
  SHALLOW_VAL, DEEP_VAL)
\
   ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, SHALLOW_VAL)
@@ -273,10 +274,11 @@
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.def"
 #undef ANALYZER_OPTION
 #undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE
-  };
-
-  bool isUnknownAnalyzerConfig(StringRef Name) const {
-assert(llvm::is_sorted(AnalyzerConfigCmdFlags));
+  };
+  // FIXME: Sort this at compile-time when we get constexpr sort (C++20).
+  llvm::sort(AnalyzerConfigCmdFlags);
+  return AnalyzerConfigCmdFlags;
+}();
 
 return !std::binary_search(AnalyzerConfigCmdFlags.begin(),
AnalyzerConfigCmdFlags.end(), Name);
@@ -292,9 +294,7 @@
 AnalyzerDisplayProgress(false), eagerlyAssumeBinOpBifurcation(false),
 TrimGraph(false), visualizeExplodedGraphWithGraphViz(false),
 UnoptimizedCFG(false), PrintStats(false), NoRetryExhausted(false),
-AnalyzerWerror(false) {
-llvm::sort(AnalyzerConfigCmdFlags);
-  }
+AnalyzerWerror(false) {}
 
   /// Interprets an option's string value as a boolean. The "true" string is
   /// interpreted as true and the "false" string is interpreted as false.


Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===
--- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -260,9 +260,10 @@
 #undef ANALYZER_OPTION
 #undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE
 
-  // Create an array of all -analyzer-config command line options. Sort it in
-  // the constructor.
-  std::vector AnalyzerConfigCmdFlags = {
+  bool isUnknownAnalyzerConfig(llvm::StringRef Name) {
+static std::vector AnalyzerConfigCmdFlags = []() {
+  // Create an array of all -analyzer-config command line options.
+  std::vector AnalyzerConfigCmdFlags = {
 #define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC,\
  SHALLOW_VAL, DEEP_VAL)\
   ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, SHALLOW_VAL)
@@ -273,10 +274,11 @@
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.def"
 #undef ANALYZER_OPTION
 #undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE
-  };
-
-  bool isUnknownAnalyzerConfig(StringRef Name) const {
-assert(llvm::is_sorted(AnalyzerConfigCmdFlags));
+  };
+  // FIXME: Sort this at compile-time when we get constexpr sort (C++20).
+  llvm::sort(AnalyzerConfigCmdFlags);
+  return AnalyzerConfigCmdFlags;
+}();
 
 return !std::binary_search(AnalyzerConfigCmdFlags.begin(),
AnalyzerConfigCmdFlags.end(), Name);
@@ -292,9 +294,7 @@
 AnalyzerDisplayProgress(false), eagerlyAssumeBinOpBifurcation(false),
 TrimGraph(false), visualizeExplodedGraphWithGraphViz(false),
 UnoptimizedCFG(false), PrintStats(false), NoRetryExhausted(false),
-AnalyzerWerror(false) {
-llvm::sort(AnalyzerConfigCmdFlags);
-  }
+AnalyzerWerror(false) {}
 
   /// Interprets an option's string value as a boolean. The "true" string is
   /// interpreted as true and the "false" string is interpreted as false.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 53a4a2b - [clang] NFCI: Optimize storage and lookup of analyzer options

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2023-04-21T10:39:55-07:00
New Revision: 53a4a2b45bb2407f3249dea54f1a8b3e230b188a

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

LOG: [clang] NFCI: Optimize storage and lookup of analyzer options

This patch moves `llvm::sort()` from `AnalyzerOptions` constructor to 
initialization of local static variable in `isUnknownAnalyzerConfig()`.

This avoids unnecessary work, which can speed up Clang tools that initialize 
lots of `CompilerInvocation`s (and therefore `AnalyzerOptions`).

Reviewed By: steakhal

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

Removed: 




diff  --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h 
b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index e81d7bbb8823e..a947bd0867025 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -260,9 +260,10 @@ class AnalyzerOptions : public 
RefCountedBase {
 #undef ANALYZER_OPTION
 #undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE
 
-  // Create an array of all -analyzer-config command line options. Sort it in
-  // the constructor.
-  std::vector AnalyzerConfigCmdFlags = {
+  bool isUnknownAnalyzerConfig(llvm::StringRef Name) {
+static std::vector AnalyzerConfigCmdFlags = []() {
+  // Create an array of all -analyzer-config command line options.
+  std::vector AnalyzerConfigCmdFlags = {
 #define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC,
\
  SHALLOW_VAL, DEEP_VAL)
\
   ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, SHALLOW_VAL)
@@ -273,10 +274,11 @@ class AnalyzerOptions : public 
RefCountedBase {
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.def"
 #undef ANALYZER_OPTION
 #undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE
-  };
-
-  bool isUnknownAnalyzerConfig(StringRef Name) const {
-assert(llvm::is_sorted(AnalyzerConfigCmdFlags));
+  };
+  // FIXME: Sort this at compile-time when we get constexpr sort (C++20).
+  llvm::sort(AnalyzerConfigCmdFlags);
+  return AnalyzerConfigCmdFlags;
+}();
 
 return !std::binary_search(AnalyzerConfigCmdFlags.begin(),
AnalyzerConfigCmdFlags.end(), Name);
@@ -292,9 +294,7 @@ class AnalyzerOptions : public 
RefCountedBase {
 AnalyzerDisplayProgress(false), eagerlyAssumeBinOpBifurcation(false),
 TrimGraph(false), visualizeExplodedGraphWithGraphViz(false),
 UnoptimizedCFG(false), PrintStats(false), NoRetryExhausted(false),
-AnalyzerWerror(false) {
-llvm::sort(AnalyzerConfigCmdFlags);
-  }
+AnalyzerWerror(false) {}
 
   /// Interprets an option's string value as a boolean. The "true" string is
   /// interpreted as true and the "false" string is interpreted as false.



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

[PATCH] D137258: [clang] Optimize storage and lookup of analyzer options

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

Sorting at compile-time would be nicer, but I don't have the bandwidth to 
implement something like that. I'll leave a FIXME in case someone wants to pick 
it up. Thanks for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137258

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

[PATCH] D148276: [clang] trigger -Wcast-qual on functional casts

[Aaron Ballman via Phabricator via cfe-commits]

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

Reverted in 7f00ecd029213aee9baf297554b0cb25a5ffbd0f 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148276

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

[clang] 7f00ecd - Revert "[clang] trigger -Wcast-qual on functional casts"

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2023-04-21T13:33:39-04:00
New Revision: 7f00ecd029213aee9baf297554b0cb25a5ffbd0f

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

LOG: Revert "[clang] trigger -Wcast-qual on functional casts"

This reverts commit 7c0021923503a9a5fe1ba1f0b778b5b83c42aa43.

It broke post-commit buildbots:
https://lab.llvm.org/buildbot/#/builders/37/builds/21631

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaCast.cpp
clang/test/SemaCXX/warn-cast-qual.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 9cc3936745ac6..7beae03247796 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -231,8 +231,6 @@ Improvements to Clang's diagnostics
 - ``-Wformat`` now recognizes ``%lb`` for the ``printf``/``scanf`` family of
   functions.
   (`#62247: `_).
-- ``-Wcast-qual`` now triggers on function-style casts.
-  (`#62083 `_)
 
 Bug Fixes in This Version
 -

diff  --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index be30d36a4f9a4..cd71288476345 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -3332,9 +3332,6 @@ ExprResult 
Sema::BuildCXXFunctionalCastExpr(TypeSourceInfo *CastTypeInfo,
   if (auto *ConstructExpr = dyn_cast(SubExpr))
 ConstructExpr->setParenOrBraceRange(SourceRange(LPLoc, RPLoc));
 
-  // -Wcast-qual
-  DiagnoseCastQual(Op.Self, Op.SrcExpr, Op.DestType);
-
   return Op.complete(CXXFunctionalCastExpr::Create(
   Context, Op.ResultType, Op.ValueKind, CastTypeInfo, Op.Kind,
   Op.SrcExpr.get(), , CurFPFeatureOverrides(), LPLoc, RPLoc));

diff  --git a/clang/test/SemaCXX/warn-cast-qual.cpp 
b/clang/test/SemaCXX/warn-cast-qual.cpp
index 4fe8de9ae59c8..d1f0cc73a63d4 100644
--- a/clang/test/SemaCXX/warn-cast-qual.cpp
+++ b/clang/test/SemaCXX/warn-cast-qual.cpp
@@ -34,17 +34,6 @@ void foo_0() {
   const int  = (int &)((int &)a);   // expected-warning {{cast from 
'const int' to 'int &' drops const qualifier}}
   const int  = (int &)((const int &)a); // expected-warning {{cast from 
'const int' to 'int &' drops const qualifier}}
   const int  = (const int &)((int &)a); // expected-warning {{cast from 
'const int' to 'int &' drops const qualifier}}
-
-  using T = int&;
-  using T2 = const int&;
-  const int  =T2(a);  // no warning
-  int a22 = T(a); // expected-warning {{cast from 'const int' to 
'int &' drops const qualifier}}
-  const int  = T(a);  // expected-warning {{cast from 'const int' to 
'int &' drops const qualifier}}
-  int  = T(T2(a));// expected-warning {{cast from 'const int' to 
'int &' drops const qualifier}}
-  int  = T(T(a)); // expected-warning {{cast from 'const int' to 
'int &' drops const qualifier}}
-  const int  = T(T(a));   // expected-warning {{cast from 'const int' to 
'int &' drops const qualifier}}
-  int  = T(T2(a));// expected-warning {{cast from 'const int' to 
'int &' drops const qualifier}}
-  const int  = T2(T(a));  // expected-warning {{cast from 'const int' to 
'int &' drops const qualifier}}
 }
 
 void foo_1() {
@@ -60,17 +49,6 @@ void foo_1() {
   volatile int  = (int &)((int &)a);  // expected-warning {{cast 
from 'volatile int' to 'int &' drops volatile qualifier}}
   volatile int  = (int &)((volatile int &)a); // expected-warning {{cast 
from 'volatile int' to 'int &' drops volatile qualifier}}
   volatile int  = (volatile int &)((int &)a); // expected-warning {{cast 
from 'volatile int' to 'int &' drops volatile qualifier}}
-
-  using T = int&;
-  using T2 = volatile int&;
-  volatile int  =T2(a); // no warning
-  int a22 = T(a);   // expected-warning {{cast from 'volatile int' 
to 'int &' drops volatile qualifier}}
-  volatile int  = T(a); // expected-warning {{cast from 'volatile int' 
to 'int &' drops volatile qualifier}}
-  int  = T(T2(a));  // expected-warning {{cast from 'volatile int' 
to 'int &' drops volatile qualifier}}
-  int  = T(T(a));   // expected-warning {{cast from 'volatile int' 
to 'int &' drops volatile qualifier}}
-  volatile int  = T(T(a));  // expected-warning {{cast from 'volatile int' 
to 'int &' drops volatile qualifier}}
-  int  = T(T2(a));  // expected-warning {{cast from 'volatile int' 
to 'int &' drops volatile qualifier}}
-  volatile int  = T2(T(a)); // expected-warning {{cast from 'volatile int' 
to 'int &' drops volatile qualifier}}
 }
 
 void foo_2() {
@@ -86,17 +64,6 @@ void foo_2() {
   const volatile int  = (int &)((int &)a);// 
expected-warning {{cast from 'const volatile int' to 'int &' drops const 

[PATCH] D148276: [clang] trigger -Wcast-qual on functional casts

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D148276#4287931 , @aaron.ballman 
wrote:

> In D148276#4287900 , @thurston 
> wrote:
>
>> I think this change is triggering an error on a buildbot 
>> (https://lab.llvm.org/buildbot/#/builders/37/builds/21631/steps/20/logs/stdio):
>>
>>   
>> /b/sanitizer-x86_64-linux/build/llvm-project/clang/lib/Analysis/UnsafeBufferUsage.cpp:1348:45:
>>  error: cast from 'const clang::ASTContext' to 'clang::ASTContext &' drops 
>> const qualifier [-Werror,-Wcast-qual]
>> std::remove_const_t(Ctx),
>>
>> even though the code is deliberately trying to remove the const qualifier 
>> the right (?) way.
>
> Ouch, that's pretty sad. I'll work around it and land a fix shortly, thank 
> you for pointing this out!

Actually, that's going to be more involved than a quick fix. I think I'll 
revert the patch and reopen this review so that @sousajo can investigate the 
proper fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148276

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

[PATCH] D148276: [clang] trigger -Wcast-qual on functional casts

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D148276#4287900 , @thurston wrote:

> I think this change is triggering an error on a buildbot 
> (https://lab.llvm.org/buildbot/#/builders/37/builds/21631/steps/20/logs/stdio):
>
>   
> /b/sanitizer-x86_64-linux/build/llvm-project/clang/lib/Analysis/UnsafeBufferUsage.cpp:1348:45:
>  error: cast from 'const clang::ASTContext' to 'clang::ASTContext &' drops 
> const qualifier [-Werror,-Wcast-qual]
> std::remove_const_t(Ctx),
>
> even though the code is deliberately trying to remove the const qualifier the 
> right (?) way.

Ouch, that's pretty sad. I'll work around it and land a fix shortly, thank you 
for pointing this out!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148276

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

[PATCH] D148817: [RISCV] Add Tag_RISCV_arch attribute by default when using clang as an assembler.

[Alex Bradbury via Phabricator via cfe-commits]

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

I'd appreciate some riscv32 RUN lines for completeness, but otherwise LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148817

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

[PATCH] D147844: Emit warning when implicit cast from int to bool happens in an conditional operator expression

[Mark de Wever via Phabricator via cfe-commits]

Mordante accepted this revision.
Mordante added a comment.

I too am not really convinced by all changes in libc++. On the other hand I 
don't think the change make the libc++ code looking bad. So I don't feel a 
reason to reject the libc++ changes.

I don't like the commit message, there's only a title and a bug report. Please 
add a bit more information in the message, with an example of code that now 
gives a diagnostic.

Note I only reviewed the libc++ changes, I leave the clang part to 
@aaron.ballman.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

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

[PATCH] D148276: [clang] trigger -Wcast-qual on functional casts

[Thurston Dang via Phabricator via cfe-commits]

thurston added a comment.

I think this change is triggering an error on a buildbot 
(https://lab.llvm.org/buildbot/#/builders/37/builds/21631/steps/20/logs/stdio):

  
/b/sanitizer-x86_64-linux/build/llvm-project/clang/lib/Analysis/UnsafeBufferUsage.cpp:1348:45:
 error: cast from 'const clang::ASTContext' to 'clang::ASTContext &' drops 
const qualifier [-Werror,-Wcast-qual]
std::remove_const_t(Ctx),

even though the code is deliberately trying to remove the const qualifier the 
right (?) way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148276

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

[PATCH] D129835: [clang] adds a discardable attribute

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added a comment.

>> With [[discardable]] one just needs to push/pop at the extremes of a file 
>> (and if a future version of module maps supports global pragmas for a 
>> module, there too, but that's a discussion that requires a design doc).
>
> I understood that, I just don't think that's a good thing. This is basically 
> an attribute that says "I know we said we wanted everything here to be 
> nodiscard, but JUST KIDDING not this one!" which is not a very clean approach 
> to writing headers.

@aaron.ballman Ugh, I've finally come up with a good use-case for 
`[[discardable]]`.

  class [[nodiscard]] iterator {
  public:
// usual iterator stuff
  
[[clang::discardable]] iterator operator++(int);
[[clang::discardable]] iterator operator--(int);
  };

I hate it, but the alternative is to decorate everything else with 
`[[nodiscard]]` just to facilitate these two operators. It's a compelling 
use-case, but I'm not sure if it's compelling enough on its own. WDYT?
(I personally think that those two should be nodiscard, but `i++` is pervasive 
enough that it might never be practical to correct.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129835

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

[PATCH] D148944: [clang][Driver] Fix crash with unsupported architectures in MinGW and CrossWindows.

[KOMATA Manabu via Phabricator via cfe-commits]

k-mana created this revision.
k-mana added a reviewer: fhahn.
Herald added subscribers: StephenFan, mstorsjo.
Herald added a project: All.
k-mana requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148944

Files:
  clang/lib/Driver/ToolChains/CrossWindows.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/unsupported-target-arch.c


Index: clang/test/Driver/unsupported-target-arch.c
===
--- clang/test/Driver/unsupported-target-arch.c
+++ clang/test/Driver/unsupported-target-arch.c
@@ -23,3 +23,11 @@
 // RUN: not %clang --target=noarch-unknown-nacl -o %t.o %s 2> %t.err
 // RUN: FileCheck --input-file=%t.err --check-prefix=CHECK-NOARCH-NACL %s
 // CHECK-NOARCH-NACL:  error: the target architecture 'noarch' is not 
supported by the target 'Native Client'
+//
+// RUN: not %clang --target=noarch-unknown-windows-gnu -o %t.o %s 2> %t.err
+// RUN: FileCheck --input-file=%t.err --check-prefix=CHECK-NOARCH-MINGW %s
+// CHECK-NOARCH-MINGW: error: unknown target triple 
'noarch-unknown-windows-gnu', please use -triple or -arch
+//
+// RUN: not %clang --target=noarch-unknown-windows-itanium -o %t.o %s 2> %t.err
+// RUN: FileCheck --input-file=%t.err --check-prefix=CHECK-NOARCH-CROSSWINDOWS 
%s
+// CHECK-NOARCH-CROSSWINDOWS: error: unknown target triple 
'noarch-unknown-windows-itanium', please use -triple or -arch
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -135,7 +135,7 @@
 CmdArgs.push_back("arm64pe");
 break;
   default:
-llvm_unreachable("Unsupported target architecture.");
+D.Diag(diag::err_target_unknown_triple) << TC.getEffectiveTriple().str();
   }
 
   Arg *SubsysArg =
Index: clang/lib/Driver/ToolChains/CrossWindows.cpp
===
--- clang/lib/Driver/ToolChains/CrossWindows.cpp
+++ clang/lib/Driver/ToolChains/CrossWindows.cpp
@@ -94,7 +94,8 @@
   CmdArgs.push_back("-m");
   switch (TC.getArch()) {
   default:
-llvm_unreachable("unsupported architecture");
+D.Diag(diag::err_target_unknown_triple) << TC.getEffectiveTriple().str();
+break;
   case llvm::Triple::arm:
   case llvm::Triple::thumb:
 // FIXME: this is incorrect for WinCE


Index: clang/test/Driver/unsupported-target-arch.c
===
--- clang/test/Driver/unsupported-target-arch.c
+++ clang/test/Driver/unsupported-target-arch.c
@@ -23,3 +23,11 @@
 // RUN: not %clang --target=noarch-unknown-nacl -o %t.o %s 2> %t.err
 // RUN: FileCheck --input-file=%t.err --check-prefix=CHECK-NOARCH-NACL %s
 // CHECK-NOARCH-NACL:  error: the target architecture 'noarch' is not supported by the target 'Native Client'
+//
+// RUN: not %clang --target=noarch-unknown-windows-gnu -o %t.o %s 2> %t.err
+// RUN: FileCheck --input-file=%t.err --check-prefix=CHECK-NOARCH-MINGW %s
+// CHECK-NOARCH-MINGW: error: unknown target triple 'noarch-unknown-windows-gnu', please use -triple or -arch
+//
+// RUN: not %clang --target=noarch-unknown-windows-itanium -o %t.o %s 2> %t.err
+// RUN: FileCheck --input-file=%t.err --check-prefix=CHECK-NOARCH-CROSSWINDOWS %s
+// CHECK-NOARCH-CROSSWINDOWS: error: unknown target triple 'noarch-unknown-windows-itanium', please use -triple or -arch
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -135,7 +135,7 @@
 CmdArgs.push_back("arm64pe");
 break;
   default:
-llvm_unreachable("Unsupported target architecture.");
+D.Diag(diag::err_target_unknown_triple) << TC.getEffectiveTriple().str();
   }
 
   Arg *SubsysArg =
Index: clang/lib/Driver/ToolChains/CrossWindows.cpp
===
--- clang/lib/Driver/ToolChains/CrossWindows.cpp
+++ clang/lib/Driver/ToolChains/CrossWindows.cpp
@@ -94,7 +94,8 @@
   CmdArgs.push_back("-m");
   switch (TC.getArch()) {
   default:
-llvm_unreachable("unsupported architecture");
+D.Diag(diag::err_target_unknown_triple) << TC.getEffectiveTriple().str();
+break;
   case llvm::Triple::arm:
   case llvm::Triple::thumb:
 // FIXME: this is incorrect for WinCE
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146987: [Assignment Tracking] Enable by default

[Steven Johnson via Phabricator via cfe-commits]

srj added a comment.

In D146987#4287819 , @jmorse wrote:

> Should have been reverted in rG0ba922f60046 
>  earlier 
> today, are you still experiencing the same behaviour with that patch?

Ah, sorry, I didn't see that -- I'll pull and rebuild this morning to verify!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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

[PATCH] D146987: [Assignment Tracking] Enable by default

[Jeremy Morse via Phabricator via cfe-commits]

jmorse added a comment.

In D146987#4287794 , @srj wrote:

> (But more seriously, could we please revert all of this unless/until a fix is 
> imminent? Our testing is dead in the water at the moment.)

Should have been reverted in rG0ba922f60046 
 earlier 
today, are you still experiencing the same behaviour with that patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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

[PATCH] D146987: [Assignment Tracking] Enable by default

[Steven Johnson via Phabricator via cfe-commits]

srj added a comment.

In D146987#4287137 , @jmorse wrote:

> Ah, you're right, it's actually the same assertion message that @srj reported 
> which I totally skipped over. Currently I suspect that's due to different 
> std::sort implementations.

...`std::stable_sort()`?

(But more seriously, could we please revert all of this unless/until a fix is 
imminent? Our testing is dead in the water at the moment.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146987

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

[PATCH] D146557: [MLIR][OpenMP] Refactoring how map clause is processed

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis marked 2 inline comments as done.
TIFitis added inline comments.



Comment at: mlir/test/Target/LLVMIR/omptarget-llvm.mlir:4-5
-llvm.func @_QPopenmp_target_data() {
-  %0 = llvm.mlir.constant(1 : i64) : i64
-  %1 = llvm.alloca %0 x i32 {bindc_name = "i", in_type = i32, 
operand_segment_sizes = array, uniq_name = 
"_QFopenmp_target_dataEi"} : (i64) -> !llvm.ptr
   omp.target_data   map((tofrom -> %1 : !llvm.ptr)) {

kiranchandramohan wrote:
> TIFitis wrote:
> > kiranchandramohan wrote:
> > > Why is the test changed?
> > Moved the map_operand to function parameter to be consistent with other 
> > tests. I think it can remain unchanged if you prefer.
> Consistency can be achieved in a follow-up NFC patch that you can submit 
> without review. If the test can only show the differences of this patch it is 
> easier to review.
Thanks for letting me know. I've updated the tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146557

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

[PATCH] D146557: [MLIR][OpenMP] Refactoring how map clause is processed

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis updated this revision to Diff 515805.
TIFitis marked an inline comment as done.
TIFitis edited the summary of this revision.
TIFitis added a comment.

Updated tests. Addressed reviewer comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146557

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
  mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp
  mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
  mlir/test/Target/LLVMIR/omptarget-llvm.mlir

Index: mlir/test/Target/LLVMIR/omptarget-llvm.mlir
===
--- mlir/test/Target/LLVMIR/omptarget-llvm.mlir
+++ mlir/test/Target/LLVMIR/omptarget-llvm.mlir
@@ -12,32 +12,24 @@
 }
 
 // CHECK: @.offload_maptypes = private unnamed_addr constant [1 x i64] [i64 3]
+// CHECK: @.offload_sizes = private unnamed_addr constant [1 x i64] [i64 4]
 // CHECK-LABEL: define void @_QPopenmp_target_data() {
 // CHECK: %[[VAL_0:.*]] = alloca [1 x ptr], align 8
 // CHECK: %[[VAL_1:.*]] = alloca [1 x ptr], align 8
-// CHECK: %[[VAL_2:.*]] = alloca [1 x i64], align 8
-// CHECK: %[[VAL_3:.*]] = alloca i32, i64 1, align 4
-// CHECK: br label %[[VAL_4:.*]]
-// CHECK:   entry:; preds = %[[VAL_5:.*]]
-// CHECK: %[[VAL_6:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
-// CHECK: store ptr %[[VAL_3]], ptr %[[VAL_6]], align 8
-// CHECK: %[[VAL_7:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
-// CHECK: store ptr %[[VAL_3]], ptr %[[VAL_7]], align 8
-// CHECK: %[[VAL_8:.*]] = getelementptr inbounds [1 x i64], ptr %[[VAL_2]], i32 0, i32 0
-// CHECK: store i64 ptrtoint (ptr getelementptr (ptr, ptr null, i32 1) to i64), ptr %[[VAL_8]], align 4
+// CHECK: %[[VAL_2:.*]] = alloca i32, i64 1, align 4
+// CHECK: br label %[[VAL_3:.*]]
+// CHECK:   entry:; preds = %[[VAL_4:.*]]
+// CHECK: %[[VAL_5:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
+// CHECK: store ptr %[[VAL_2]], ptr %[[VAL_5]], align 8
+// CHECK: %[[VAL_6:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
+// CHECK: store ptr %[[VAL_2]], ptr %[[VAL_6]], align 8
+// CHECK: %[[VAL_7:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
+// CHECK: %[[VAL_8:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
+// CHECK: call void @__tgt_target_data_begin_mapper(ptr @2, i64 -1, i32 1, ptr %[[VAL_7]], ptr %[[VAL_8]], ptr @.offload_sizes, ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
+// CHECK: store i32 99, ptr %[[VAL_2]], align 4
 // CHECK: %[[VAL_9:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
 // CHECK: %[[VAL_10:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
-// CHECK: %[[VAL_11:.*]] = getelementptr inbounds [1 x i64], ptr %[[VAL_2]], i32 0, i32 0
-// CHECK: call void @__tgt_target_data_begin_mapper(ptr @2, i64 -1, i32 1, ptr %[[VAL_9]], ptr %[[VAL_10]], ptr %[[VAL_11]], ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
-// CHECK: br label %[[VAL_12:.*]]
-// CHECK:   omp.data.region:  ; preds = %[[VAL_4]]
-// CHECK: store i32 99, ptr %[[VAL_3]], align 4
-// CHECK: br label %[[VAL_13:.*]]
-// CHECK:   omp.region.cont:  ; preds = %[[VAL_12]]
-// CHECK: %[[VAL_14:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_0]], i32 0, i32 0
-// CHECK: %[[VAL_15:.*]] = getelementptr inbounds [1 x ptr], ptr %[[VAL_1]], i32 0, i32 0
-// CHECK: %[[VAL_16:.*]] = getelementptr inbounds [1 x i64], ptr %[[VAL_2]], i32 0, i32 0
-// CHECK: call void @__tgt_target_data_end_mapper(ptr @2, i64 -1, i32 1, ptr %[[VAL_14]], ptr %[[VAL_15]], ptr %[[VAL_16]], ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
+// CHECK: call void @__tgt_target_data_end_mapper(ptr @2, i64 -1, i32 1, ptr %[[VAL_9]], ptr %[[VAL_10]], ptr @.offload_sizes, ptr @.offload_maptypes, ptr @.offload_mapnames, ptr null)
 // CHECK: ret void
 
 // -
@@ -56,38 +48,30 @@
 }
 
 // CHECK: @.offload_maptypes = private unnamed_addr constant [1 x i64] [i64 2]
+// CHECK: @.offload_sizes = private unnamed_addr constant [1 x i64] [i64 4096]
 // CHECK-LABEL: define void @_QPopenmp_target_data_region
 // CHECK: (ptr %[[ARG_0:.*]]) {
 // CHECK: %[[VAL_0:.*]] = alloca [1 x ptr], align 8
 // CHECK: 

[PATCH] D148925: [clang][Interp] Fix discarding void-typed comma expressions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

In D148925#4287502 , @erichkeane 
wrote:

> Cool!  Mind adding the tests I proposed?  it would be nice to ensure/show 
> side-effects are guaranteed.

Sure, thanks for the test case!


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

https://reviews.llvm.org/D148925

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

[PATCH] D148925: [clang][Interp] Fix discarding void-typed comma expressions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 515800.

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

https://reviews.llvm.org/D148925

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/test/AST/Interp/literals.cpp


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -808,9 +808,23 @@
   (a); // expected-warning {{unused}} \
// ref-warning {{unused}}
 
+  (void)5, (void)6;
+
   return 0;
 }
 
+/// Ignored comma expressions still have their
+/// expressions evaluated.
+constexpr int Comma(int start) {
+int i = start;
+
+(void)i++;
+(void)i++,(void)i++;
+return i;
+}
+constexpr int Value = Comma(5);
+static_assert(Value == 8);
+
 #endif
 
 namespace PredefinedExprs {
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -264,8 +264,12 @@
 
   // Deal with operations which have composite or void types.
   if (BO->isCommaOp()) {
-if (!discard(LHS))
+if (!this->discard(LHS))
   return false;
+if (RHS->getType()->isVoidType())
+  return this->discard(RHS);
+
+// Otherwise, visit RHS and optionally discard its value.
 return Discard(this->visit(RHS));
   }
 
@@ -1650,10 +1654,12 @@
   if (!visit(Exp))
 return false;
 
+  if (Exp->getType()->isVoidType())
+return this->emitRetVoid(Exp);
+
   if (std::optional T = classify(Exp))
 return this->emitRet(*T, Exp);
-  else
-return this->emitRetValue(Exp);
+  return this->emitRetValue(Exp);
 }
 
 /// Toplevel visitDecl().


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -808,9 +808,23 @@
   (a); // expected-warning {{unused}} \
// ref-warning {{unused}}
 
+  (void)5, (void)6;
+
   return 0;
 }
 
+/// Ignored comma expressions still have their
+/// expressions evaluated.
+constexpr int Comma(int start) {
+int i = start;
+
+(void)i++;
+(void)i++,(void)i++;
+return i;
+}
+constexpr int Value = Comma(5);
+static_assert(Value == 8);
+
 #endif
 
 namespace PredefinedExprs {
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -264,8 +264,12 @@
 
   // Deal with operations which have composite or void types.
   if (BO->isCommaOp()) {
-if (!discard(LHS))
+if (!this->discard(LHS))
   return false;
+if (RHS->getType()->isVoidType())
+  return this->discard(RHS);
+
+// Otherwise, visit RHS and optionally discard its value.
 return Discard(this->visit(RHS));
   }
 
@@ -1650,10 +1654,12 @@
   if (!visit(Exp))
 return false;
 
+  if (Exp->getType()->isVoidType())
+return this->emitRetVoid(Exp);
+
   if (std::optional T = classify(Exp))
 return this->emitRet(*T, Exp);
-  else
-return this->emitRetValue(Exp);
+  return this->emitRetValue(Exp);
 }
 
 /// Toplevel visitDecl().
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Alexander Shaposhnikov via Phabricator via cfe-commits]

alexander-shaposhnikov added a comment.

reduced test case (that is currently blocking this diff) :

  namespace outer::internal {
  
  template 
  concept myconcept = true;
  
  }
  
  namespace outer {
  
  template  class Foo;
  
  template  struct Bar {
template  friend class Foo;
  };
  
  Bar S;
  
  namespace internal {
  int f(Foo);
  }
  
  template  struct Foo {
friend int internal::f(Foo);
  };
  
  } // namespace outer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146178

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

[PATCH] D134475: [Clang] Add support for [[msvc::constexpr]] C++11 attribute

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Still suspicious about this/whether it models the MSVC implementation 
correctly, but no real implementation concerns.  I REALLY want us to document 
the attribute extensively however.




Comment at: clang/include/clang/Basic/AttrDocs.td:3543
+definition or a return statement. It has no effect on function declarations.
+This attribute permits constant evaluation of ``[[msvc::constexpr]]`` functions
+in ``[[msvc::constexpr]] return`` statements inside ``constexpr`` or

Does it prohibit the inverse?  I think this documentation overall needs a much 
better description of what the semantics are here, particularly anything that 
you found in experimentation on the MS implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134475

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

[PATCH] D148925: Fix discarding void-typed comma expressions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D148925#4287497 , @tbaeder wrote:

> In D148925#4287218 , @erichkeane 
> wrote:
>
>> So things cast to 'void' can have side effects, right?  Does the call to 
>> discard still evaluate them?
>>
>> https://godbolt.org/z/a3ej9bxKe
>
> Yes. `discard(E)` is just `visit(E)`, except that it sets `DiscardResult` to 
> `true` for that one level of visiting. Following `visit()` calls will have 
> `DiscardResult = false` again.

Cool!  Mind adding the tests I proposed?  it would be nice to ensure/show 
side-effects are guaranteed.


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

https://reviews.llvm.org/D148925

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

[PATCH] D148925: Fix discarding void-typed comma expressions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

In D148925#4287218 , @erichkeane 
wrote:

> So things cast to 'void' can have side effects, right?  Does the call to 
> discard still evaluate them?
>
> https://godbolt.org/z/a3ej9bxKe

Yes. `discard(E)` is just `visit(E)`, except that it sets `DiscardResult` to 
`true` for that one level of visiting. Following `visit()` calls will have 
`DiscardResult = false` again.


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

https://reviews.llvm.org/D148925

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

[PATCH] D148925: Fix discarding void-typed comma expressions

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 515762.

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

https://reviews.llvm.org/D148925

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/test/AST/Interp/literals.cpp


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -808,6 +808,8 @@
   (a); // expected-warning {{unused}} \
// ref-warning {{unused}}
 
+  (void)5, (void)6;
+
   return 0;
 }
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -264,8 +264,12 @@
 
   // Deal with operations which have composite or void types.
   if (BO->isCommaOp()) {
-if (!discard(LHS))
+if (!this->discard(LHS))
   return false;
+if (RHS->getType()->isVoidType())
+  return this->discard(RHS);
+
+// Otherwise, visit RHS and optionally discard its value.
 return Discard(this->visit(RHS));
   }
 
@@ -1650,10 +1654,12 @@
   if (!visit(Exp))
 return false;
 
+  if (Exp->getType()->isVoidType())
+return this->emitRetVoid(Exp);
+
   if (std::optional T = classify(Exp))
 return this->emitRet(*T, Exp);
-  else
-return this->emitRetValue(Exp);
+  return this->emitRetValue(Exp);
 }
 
 /// Toplevel visitDecl().


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -808,6 +808,8 @@
   (a); // expected-warning {{unused}} \
// ref-warning {{unused}}
 
+  (void)5, (void)6;
+
   return 0;
 }
 
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -264,8 +264,12 @@
 
   // Deal with operations which have composite or void types.
   if (BO->isCommaOp()) {
-if (!discard(LHS))
+if (!this->discard(LHS))
   return false;
+if (RHS->getType()->isVoidType())
+  return this->discard(RHS);
+
+// Otherwise, visit RHS and optionally discard its value.
 return Discard(this->visit(RHS));
   }
 
@@ -1650,10 +1654,12 @@
   if (!visit(Exp))
 return false;
 
+  if (Exp->getType()->isVoidType())
+return this->emitRetVoid(Exp);
+
   if (std::optional T = classify(Exp))
 return this->emitRet(*T, Exp);
-  else
-return this->emitRetValue(Exp);
+  return this->emitRetValue(Exp);
 }
 
 /// Toplevel visitDecl().
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a reviewer: clang-language-wg.
ilya-biryukov added a comment.

Thanks! I believe we should also recover in the similar manner we do for 
missing `const`, see corresponding comment.

Extra NITs:

- there is a typo in description: *compariosn* should be comparison,
- we probably want to add this change to release notes.

Also adding @clang-language-wg in case someone else wants to chime in.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9434
+def err_volatile_comparison_operator : Error<
+  "defaulted comparison operator function must not be volatile">;
+def err_ref_qualifier_comparison_operator : Error<

NIT: for consistency with wording of notes above.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:9436
+def err_ref_qualifier_comparison_operator : Error<
+  "ref-qualifier '&&' is not allowed on defaulted comparison operators">;
 

NIT: for consistency with the wording of `err_ref_qualifier_constructor`



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8593-8601
+const FunctionProtoType *FnType = 
FD->getType()->castAs();
+if (FnType->isVolatile()) {
+  Diag(FD->getLocation(), diag::err_volatile_comparison_operator);
+  return true;
+}
+if (FnType->getRefQualifier() == RQ_RValue) {
+  Diag(FD->getLocation(), diag::err_ref_qualifier_comparison_operator);

NIT: this version is simpler and more aligned with the code below.
Also, could you move the `volatile` handling after the check for `const`?


Additionally, we seem to recover in case of `const` by adding it to the type 
and suggesting a fix-it to add it in the code.
Could we do the same for `volatile` and `ref-qualifier`, i.e. suggest a fix-it 
to remove the `ref-qualifier` and `volatile` and change the corresponding type 
to make AST pretend they were never there?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148924

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

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

[jonathan molinatto via Phabricator via cfe-commits]

jrmolin added inline comments.



Comment at: clang/include/clang/Format/Format.h:851
+  /// The function parameter breaking style to use.
+  /// \version 16.0
+  FunctionParameterBreakingStyle BreakBeforeFunctionParameters;

so should this be 17.0 now? This has gone on quite long.



Comment at: clang/lib/Format/Format.cpp:345
+
+// For backward compatibility.
+IO.enumCase(Value, "false", FormatStyle::FPBS_Leave);

I should probably delete these, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125171

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

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

[jonathan molinatto via Phabricator via cfe-commits]

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

a unit test was failing, so I fixed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125171

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

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

[PATCH] D148799: [clang] Return `std::string_view` from `TargetInfo::getClobbers()`

[David Spickett via Phabricator via cfe-commits]

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

If you have commit access already, wait until the pre-commit builds finish and 
check there isn't anything failing there.

If you don't, I can commit this for you as I did before. I'll do that on Monday.


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

https://reviews.llvm.org/D148799

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

[PATCH] D148799: [clang] Return `std::string_view` from `TargetInfo::getClobbers()`

[Sviatoslav Osipov via Phabricator via cfe-commits]

Stoorx updated this revision to Diff 515752.
Stoorx edited the summary of this revision.
Stoorx added a comment.

Fix, Rebase & retitle


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

https://reviews.llvm.org/D148799

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/ARC.h
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/ARM.h
  clang/lib/Basic/Targets/AVR.h
  clang/lib/Basic/Targets/BPF.h
  clang/lib/Basic/Targets/CSKY.h
  clang/lib/Basic/Targets/DirectX.h
  clang/lib/Basic/Targets/Hexagon.h
  clang/lib/Basic/Targets/Lanai.h
  clang/lib/Basic/Targets/Le64.h
  clang/lib/Basic/Targets/LoongArch.h
  clang/lib/Basic/Targets/M68k.cpp
  clang/lib/Basic/Targets/M68k.h
  clang/lib/Basic/Targets/MSP430.h
  clang/lib/Basic/Targets/Mips.h
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/PNaCl.h
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Basic/Targets/SPIR.h
  clang/lib/Basic/Targets/Sparc.h
  clang/lib/Basic/Targets/SystemZ.h
  clang/lib/Basic/Targets/TCE.h
  clang/lib/Basic/Targets/VE.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/Basic/Targets/XCore.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGStmt.cpp

Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2779,7 +2779,7 @@
  "unwind clobber can't be used with asm goto");
 
   // Add machine specific clobbers
-  std::string MachineClobbers = getTarget().getClobbers();
+  std::string_view MachineClobbers = getTarget().getClobbers();
   if (!MachineClobbers.empty()) {
 if (!Constraints.empty())
   Constraints += ',';
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -939,7 +939,7 @@
 
   // Build the constraints. FIXME: We should support immediates when possible.
   std::string Constraints = "={@ccc},r,r,~{cc},~{memory}";
-  std::string MachineClobbers = CGF.getTarget().getClobbers();
+  std::string_view MachineClobbers = CGF.getTarget().getClobbers();
   if (!MachineClobbers.empty()) {
 Constraints += ',';
 Constraints += MachineClobbers;
@@ -1082,7 +1082,7 @@
   AsmOS << "$0, ${1:y}";
 
   std::string Constraints = "=r,*Z,~{memory}";
-  std::string MachineClobbers = CGF.getTarget().getClobbers();
+  std::string_view MachineClobbers = CGF.getTarget().getClobbers();
   if (!MachineClobbers.empty()) {
 Constraints += ',';
 Constraints += MachineClobbers;
Index: clang/lib/Basic/Targets/XCore.h
===
--- clang/lib/Basic/Targets/XCore.h
+++ clang/lib/Basic/Targets/XCore.h
@@ -49,7 +49,7 @@
 return TargetInfo::VoidPtrBuiltinVaList;
   }
 
-  const char *getClobbers() const override { return ""; }
+  std::string_view getClobbers() const override { return ""; }
 
   ArrayRef getGCCRegNames() const override {
 static const char *const GCCRegNames[] = {
Index: clang/lib/Basic/Targets/X86.h
===
--- clang/lib/Basic/Targets/X86.h
+++ clang/lib/Basic/Targets/X86.h
@@ -262,7 +262,7 @@
StringRef Constraint, unsigned Size) const;
 
   std::string convertConstraint(const char *) const override;
-  const char *getClobbers() const override {
+  std::string_view getClobbers() const override {
 return "~{dirflag},~{fpsr},~{flags}";
   }
 
Index: clang/lib/Basic/Targets/WebAssembly.h
===
--- clang/lib/Basic/Targets/WebAssembly.h
+++ clang/lib/Basic/Targets/WebAssembly.h
@@ -130,7 +130,7 @@
 return false;
   }
 
-  const char *getClobbers() const final { return ""; }
+  std::string_view getClobbers() const final { return ""; }
 
   bool isCLZForZeroUndef() const final { return false; }
 
Index: clang/lib/Basic/Targets/VE.h
===
--- clang/lib/Basic/Targets/VE.h
+++ clang/lib/Basic/Targets/VE.h
@@ -69,7 +69,7 @@
 }
   }
 
-  const char *getClobbers() const override { return ""; }
+  std::string_view getClobbers() const override { return ""; }
 
   ArrayRef getGCCRegNames() const override {
 static const char *const GCCRegNames[] = {
Index: clang/lib/Basic/Targets/TCE.h
===
--- clang/lib/Basic/Targets/TCE.h
+++ clang/lib/Basic/Targets/TCE.h
@@ -99,7 +99,7 @@
 return std::nullopt;
   }
 
-  const char *getClobbers() const override { return ""; }
+  std::string_view getClobbers() const override { return ""; }
 
   BuiltinVaListKind getBuiltinVaListKind() const override {
 return 

[PATCH] D134475: Add C++11 attribute msvc::constexpr

[Richard Dzenis via Phabricator via cfe-commits]

RIscRIpt updated this revision to Diff 515740.
RIscRIpt added a comment.

A proper implementation

In D134475#4224082 , @erichkeane 
wrote:

> I don't have a good idea, I don't think any sort of RAII object is the right 
> way (since it will be wrong on child calls/etc), and the ParentMap is 
> definitely not the right way.  Perhaps just a flag for 
> CheckConstexprFunction?  I've not spent too much time in this section of the 
> compiler, so hopefully someone else can come along and help. I suspect it is 
> a property of `CallStackFrame`?  But that already contains a link to the 
> FucntionDecl, right?  As far as the 'on return statement', I think the bit on 
> CallStackFrame/EvalInfo is probably what is necessary.



> But that already contains a link to the FucntionDecl, right?

Yes, but we need to know current context (whether we're in `[[msvc::constexpr]] 
return` statement).

> As far as the 'on return statement', I think the bit on 
> CallStackFrame/EvalInfo is probably what is necessary.

That's what I ended-up doing. This was the most reasonable approach I could 
come-up with.

I added a property to `CallStackFrame`, because it's a property of current 
function: whether we are in valid context which permits evaluation of 
`[[msvc::constexpr]]`. Additionally I added `MSConstexprContextRAII` which is 
used in `case Stmt::AttributedStmtClass:` to alter the property.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134475

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/AST/msvc-attrs-invalid.cpp
  clang/test/AST/msvc-attrs.cpp
  clang/test/AST/msvc-constexpr-new.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -83,6 +83,7 @@
 // CHECK-NEXT: LoaderUninitialized (SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: Lockable (SubjectMatchRule_record)
 // CHECK-NEXT: MIGServerRoutine (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_block)
+// CHECK-NEXT: MSConstexpr (SubjectMatchRule_function)
 // CHECK-NEXT: MSStruct (SubjectMatchRule_record)
 // CHECK-NEXT: MaybeUndef (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: MicroMips (SubjectMatchRule_function)
Index: clang/test/AST/msvc-constexpr-new.cpp
===
--- /dev/null
+++ clang/test/AST/msvc-constexpr-new.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fms-extensions -std=c++20 -ast-dump %s | FileCheck %s
+
+// CHECK: used operator new
+// CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} 
+[[nodiscard]] [[msvc::constexpr]] inline void* __cdecl operator new(decltype(sizeof(void*)), void* p) noexcept { return p; }
+
+// CHECK: used constexpr construct_at
+// CHECK: AttributedStmt 0x{{[0-9a-f]+}} 
+// CHECK-NEXT: MSConstexprAttr 0x{{[0-9a-f]+}} 
+// CHECK-NEXT: ReturnStmt 0x{{[0-9a-f]+}} 
+constexpr int* construct_at(int* p, int v) { [[msvc::constexpr]] return ::new (p) int(v); }
+
+constexpr bool check_construct_at() { int x; return *construct_at(, 42) == 42; }
+
+static_assert(check_construct_at());
Index: clang/test/AST/msvc-attrs.cpp
===
--- /dev/null
+++ clang/test/AST/msvc-attrs.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -fms-extensions -std=c++20 -ast-dump %s | FileCheck %s
+
+// [[msvc::constexpr]] tests
+
+// MSConstexprDocs (1)
+// CHECK: used f1 'bool ()'
+// CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} 
+[[msvc::constexpr]] bool f1() { return true; }
+
+// MSConstexprDocs (2)
+// CHECK: used constexpr f2 'bool ()'
+// CHECK: AttributedStmt 0x{{[0-9a-f]+}} 
+// CHECK-NEXT: MSConstexprAttr 0x{{[0-9a-f]+}} 
+// CHECK-NEXT: ReturnStmt 0x{{[0-9a-f]+}} 
+constexpr bool f2() { [[msvc::constexpr]] return f1(); }
+
+static_assert(f2());
+
+constexpr bool f3() { [[msvc::constexpr]] return f1() || f1() && f1(); }
+static_assert(f3());
+
+[[msvc::constexpr]] int f4(int x) { [[msvc::constexpr]] return x > 1 ? 1 + f4(x / 2) : 0; }
+constexpr bool f5() { [[msvc::constexpr]] return f4(32) == 5; }
+static_assert(f5());
+
+[[msvc::constexpr]] int f6(int x)
+{
+switch (x)
+{
+case 42: return 1337;
+default:
+ if (x < 0) [[msvc::constexpr]] return f4(-x);
+ else return x;
+}
+}
+
+constexpr bool f7() { [[msvc::constexpr]] return f6(f6(42) - 1337 

[PATCH] D148914: [Sema][NFC] add check before using `BuildExpressionFromIntegralTemplateArgument`

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

See my comment on https://github.com/llvm/llvm-project/issues/62265 for results 
of my debugging.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148914

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

[PATCH] D148355: [analyzer] Fix comparison logic in ArrayBoundCheckerV2

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

steakhal added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:167-168
 
-  NonLoc rawOffsetVal = rawOffset.getByteOffset();
-
-  // CHECK LOWER BOUND: Is byteOffset < extent begin?
-  //  If so, we are doing a load/store
-  //  before the first valid offset in the memory region.
+  // At this point we know that rawOffset is not default-constructed so we may
+  // call this:
+  NonLoc ByteOffset = rawOffset.getByteOffset();

donat.nagy wrote:
> steakhal wrote:
> > I don't think we need an explanation here.
> > BTW This "optional-like" `RegionRawOffsetV2` makes me angry.  It should be 
> > an `std::optional` in the first place.
> > You don't need to take actions, I'm just ranting...
> Yes, I was also very annoyed by this "intrustive optional" behavior and I 
> seriously considered refactoring the code to use a real std::optional, but I 
> didn't want to snowball this change into a full-scale rewrite so I ended up 
> with the assert in the constructor and this comment. Perhaps I'll refactor it 
> in a separate NFC...
Please :D



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:173
+  const MemSpaceRegion *SR = rawOffset.getRegion()->getMemorySpace();
+  if (SR->getKind() != MemRegion::UnknownSpaceRegionKind) {
+// a pointer to UnknownSpaceRegionKind may point to the middle of

donat.nagy wrote:
> steakhal wrote:
> > 
> You're completely right, I just blindly copied this test from the needlessly 
> overcomplicated `computeExtentBegin()`.
Hold on. This would only skip the lower bounds check if it's an 
`UnknownSpaceRegion`.
Shouldn't we early return instead?



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:212-215
 }
 
-assert(state_withinUpperBound);
-state = state_withinUpperBound;
+if (state_withinUpperBound)
+  state = state_withinUpperBound;

donat.nagy wrote:
> steakhal wrote:
> > You just left the guarded block in the previous line.
> > By moving this statement there you could spare the `if`.
> Nice catch :)
On second though no. The outer if guards `state_exceedsUpperBound`.
So this check seems necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148355

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

[PATCH] D148926: [Clang] Add support for [[msvc::constexpr]] C++11 attribute

[Richard Dzenis via Phabricator via cfe-commits]

RIscRIpt abandoned this revision.
RIscRIpt added a comment.

This had to be submitted to https://reviews.llvm.org/D134475


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148926

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

[PATCH] D148355: [analyzer] Fix comparison logic in ArrayBoundCheckerV2

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy added a comment.

@steakhal Thanks for the review, it was really instructive :). I'll probably 
upload the updated commit on Monday.

What kind of measurements are you suggesting? I analyzed a few open source 
projects with the patched Clang SA (see results in my previous commit) and a 
few additional analysis runs are still ongoing. (I didn't measure 
performance/runtime, because I don't think that this change significantly 
affects those.)




Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:167-168
 
-  NonLoc rawOffsetVal = rawOffset.getByteOffset();
-
-  // CHECK LOWER BOUND: Is byteOffset < extent begin?
-  //  If so, we are doing a load/store
-  //  before the first valid offset in the memory region.
+  // At this point we know that rawOffset is not default-constructed so we may
+  // call this:
+  NonLoc ByteOffset = rawOffset.getByteOffset();

steakhal wrote:
> I don't think we need an explanation here.
> BTW This "optional-like" `RegionRawOffsetV2` makes me angry.  It should be an 
> `std::optional` in the first place.
> You don't need to take actions, I'm just ranting...
Yes, I was also very annoyed by this "intrustive optional" behavior and I 
seriously considered refactoring the code to use a real std::optional, but I 
didn't want to snowball this change into a full-scale rewrite so I ended up 
with the assert in the constructor and this comment. Perhaps I'll refactor it 
in a separate NFC...



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:173
+  const MemSpaceRegion *SR = rawOffset.getRegion()->getMemorySpace();
+  if (SR->getKind() != MemRegion::UnknownSpaceRegionKind) {
+// a pointer to UnknownSpaceRegionKind may point to the middle of

steakhal wrote:
> 
You're completely right, I just blindly copied this test from the needlessly 
overcomplicated `computeExtentBegin()`.



Comment at: clang/lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp:212-215
 }
 
-assert(state_withinUpperBound);
-state = state_withinUpperBound;
+if (state_withinUpperBound)
+  state = state_withinUpperBound;

steakhal wrote:
> You just left the guarded block in the previous line.
> By moving this statement there you could spare the `if`.
Nice catch :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148355

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

[PATCH] D148799: [clang] Return `std::string_view` from `TargetInfo::getClobbers()`

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

Build issues will also show up in the "Build Status" here.


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

https://reviews.llvm.org/D148799

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