[PATCH] D152206: [Basic] Support 64-bit x86 target for UEFI

2023-06-07 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

This needs a lit test.




Comment at: clang/lib/Driver/ToolChains/UEFI.cpp:92-97
+  StringRef Linker =
+  Args.getLastArgValue(options::OPT_fuse_ld_EQ, CLANG_DEFAULT_LINKER);
+  if (Linker.empty() || Linker.equals_insensitive("lld"))
+Linker = "lld-link";
+
+  linkPath = TC.GetProgramPath(Linker.str().c_str());

This would ideally be handled by `ToolChain::GetLinkerPath`, can you leave a 
comment here along those lines?



Comment at: clang/lib/Driver/ToolChains/UEFI.cpp:97
+
+  linkPath = TC.GetProgramPath(Linker.str().c_str());
+

This could be simplified as suggested, also the name of the variable should be 
`LinkerPath`.



Comment at: clang/lib/Driver/ToolChains/UEFI.cpp:102
+  Args.MakeArgString(linkPath), CmdArgs, Inputs, Output);
+  if (!Environment.empty())
+LinkCmd->setEnvironment(Environment);

The `Environment` variable is never modified so this condition will never hold.



Comment at: clang/lib/Driver/ToolChains/UEFI.h:10-11
+#ifndef LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_UEFI_H
+#define LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_UEFI_H
+#include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"

Nit: Can you add an empty line here?



Comment at: clang/lib/Driver/ToolChains/UEFI.h:54-55
+  bool isPICDefaultForced() const override { return true; }
+};
+} // namespace toolchains
+} // namespace driver

Nit: Can you leave an empty line here for consistency?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152206

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


[PATCH] D152345: [include-cleaner] Report all specializations if the primary template is introduced by a using-decl.

2023-06-07 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: kadircet.
Herald added a project: All.
hokein requested review of this revision.
Herald added a project: clang-tools-extra.

This will fix unused-include false positive.

  // primary.h
  namespace ns {
  template class Z {}; // primary template
  }
  
  // partial.h
  namespace ns {
  template class Z {}; // partial specialization
  }
  
  // main.cpp
  
  using ns::Z; // refs to the primary
  void k() {
Z z; // use the partial specialization
  }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152345

Files:
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -252,6 +252,60 @@
"auto x = [] { ^foo(); };"),
   ElementsAre(Decl::FunctionTemplate));
 }
+TEST(WalkAST, TemplateSpecializationsFromUsingDecl) {
+  // Class templates
+  testWalk(R"cpp(
+namespace ns {
+template class $ambiguous^Z {}; // primary template
+template class $ambiguous^Z {}; // partial specialization
+template class $ambiguous^Z {}; // partial specialization
+}
+  )cpp",
+   "using ns::^Z;");
+  testWalk(R"cpp(
+namespace ns {
+template class $ambiguous^Z {}; // primary template
+template<> class $ambiguous^Z {};// full specialization
+template<> class $ambiguous^Z {};   // full specialization
+}
+  )cpp",
+   "using ns::^Z;");
+
+  // Var templates
+  testWalk(R"cpp(
+namespace ns {
+template
+T $ambiguous^foo; // primary
+
+template<>
+int* $ambiguous^foo; // full specialization
+}
+  )cpp",
+   "using ns::^foo;");
+  testWalk(R"cpp(
+namespace ns {
+template
+T $ambiguous^bar;   // primary
+
+template
+T* $ambiguous^bar; // partial specialization
+}
+  )cpp",
+   "using ns::^bar;");
+
+  // Function templates, no partial template specializations.
+  testWalk(R"cpp(
+namespace ns {
+template
+void $ambiguous^function(T); // primary
+
+template<>
+void $ambiguous^function(int); // full specialization
+}
+  )cpp",
+   "using ns::^function;");
+}
+
 
 TEST(WalkAST, Alias) {
   testWalk(R"cpp(
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -20,6 +20,7 @@
 #include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/IdentifierTable.h"
+#include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/STLExtras.h"
@@ -169,12 +170,38 @@
 return true;
   }
 
+  // Report all (partial) specializations of a class/var template decl.
+  template 
+  void reportSpecializations(SourceLocation Loc, NamedDecl *ND) {
+if (const auto *TD = dyn_cast(ND)) {
+  for (auto *Spec : TD->specializations())
+report(Loc, Spec, RefType::Ambiguous);
+  SmallVector PartialSpecializations;
+  TD->getPartialSpecializations(PartialSpecializations);
+  for (auto *PartialSpec : PartialSpecializations) {
+report(Loc, PartialSpec, RefType::Ambiguous);
+  }
+}
+  }
   bool VisitUsingDecl(UsingDecl *UD) {
 for (const auto *Shadow : UD->shadows()) {
   auto *TD = Shadow->getTargetDecl();
   auto IsUsed = TD->isUsed() || TD->isReferenced();
   report(UD->getLocation(), TD,
  IsUsed ? RefType::Explicit : RefType::Ambiguous);
+
+  // All (partial) template specializations are visible via a using-decl,
+  // However a using-decl only refers to the primary template (per C++ name
+  // lookup). Thus, we need to manually report all specializations.
+  reportSpecializations(
+  UD->getLocation(), TD);
+  reportSpecializations(
+  UD->getLocation(), TD);
+  if (const auto *FTD = dyn_cast(TD))
+for (auto *Spec : FTD->specializations())
+  report(UD->getLocation(), Spec, RefType::Ambiguous);
 }
 return true;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D152226: [FunctionAttrs] Propagate some func/arg/ret attributes from caller to callsite (WIP)

2023-06-07 Thread Noah Goldstein via Phabricator via cfe-commits
goldstein.w.n updated this revision to Diff 529201.
goldstein.w.n added a comment.

Add header comments/license.
Use enum for kMaybe, kYes, kNo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152226

Files:
  clang/test/CodeGen/LoongArch/inline-asm-constraints.c
  clang/test/CodeGen/LoongArch/inline-asm-operand-modifiers.c
  clang/test/CodeGen/LoongArch/intrinsic-la32.c
  clang/test/CodeGen/LoongArch/intrinsic-la64.c
  clang/test/CodeGen/PowerPC/builtins-ppc-build-pair-mma.c
  clang/test/CodeGen/PowerPC/builtins-ppc-pair-mma.c
  
clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReseve-StoreCond-64bit-only.c
  clang/test/CodeGen/PowerPC/builtins-ppc-xlcompat-LoadReseve-StoreCond.c
  clang/test/CodeGen/PowerPC/ppc64-inline-asm.c
  clang/test/CodeGen/RISCV/riscv-inline-asm.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/vwrite-csr.c
  clang/test/CodeGen/X86/fma-builtins-constrained.c
  clang/test/CodeGen/X86/ms-x86-intrinsics.c
  clang/test/CodeGen/aarch64-bf16-ldst-intrinsics.c
  clang/test/CodeGen/aarch64-ls64-inline-asm.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1sb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1sh.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1sw.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1ub.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1uh.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_ldnt1uw.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_stnt1.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_stnt1b.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_stnt1h.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_stnt1w.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_whilege.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_whilegt.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_whilerw-bfloat.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_whilerw.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_whilewr-bfloat.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_whilewr.c
  
clang/test/CodeGen/aarch64_neon_sve_bridge_intrinsics/acle_neon_sve_bridge_dup_neonq.c
  
clang/test/CodeGen/aarch64_neon_sve_bridge_intrinsics/acle_neon_sve_bridge_get_neonq.c
  
clang/test/CodeGen/aarch64_neon_sve_bridge_intrinsics/acle_neon_sve_bridge_set_neonq.c
  clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
  clang/test/CodeGen/attr-riscv-rvv-vector-bits-call.c
  clang/test/CodeGen/attr-riscv-rvv-vector-bits-cast.c
  clang/test/CodeGen/msp430-builtins.c
  clang/test/CodeGen/nofpclass.c
  clang/test/Headers/wasm.c
  llvm/include/llvm/Transforms/Utils/InferCallsiteAttrs.h
  llvm/lib/Transforms/IPO/FunctionAttrs.cpp
  llvm/lib/Transforms/Utils/CMakeLists.txt
  llvm/lib/Transforms/Utils/InferCallsiteAttrs.cpp
  llvm/test/Other/cgscc-devirt-iteration.ll
  llvm/test/Transforms/FunctionAttrs/nonnull.ll
  llvm/test/Transforms/FunctionAttrs/readattrs.ll
  llvm/test/Transforms/FunctionAttrs/willreturn-callsites.ll
  llvm/test/Transforms/MergeFunc/mergefunc-preserve-debug-info.ll
  llvm/test/Transforms/PhaseOrdering/X86/loop-idiom-vs-indvars.ll
  llvm/test/Transforms/PhaseOrdering/memset-tail.ll

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


[PATCH] D143260: [clangd] Add semantic token for labels

2023-06-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

alright then, this patch LGTM. going forward let's try to introduce new kinds 
(or handling for different semantic constructs) in a more holistic manner.

concerns with new language structs are OK, as the language evolves we'll surely 
need to add more highlighting, hopefully we can fit them under existing 
highlight kinds.

(sorry for the late reply)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143260

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


[PATCH] D147357: [clang-tidy] Add bugprone-optional-value-conversion check

2023-06-07 Thread Piotr Zegar via Phabricator via cfe-commits
PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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


[PATCH] D148461: [clang-tidy] Support C++17/20 in bugprone-exception-escape

2023-06-07 Thread Piotr Zegar via Phabricator via cfe-commits
PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148461

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


[PATCH] D151594: [clang-tidy] Optimize misc-confusable-identifiers

2023-06-07 Thread Piotr Zegar via Phabricator via cfe-commits
PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151594

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


[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

2023-06-07 Thread Vikram Hegde via Phabricator via cfe-commits
vikramRH updated this revision to Diff 529207.
vikramRH added a comment.

Further review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150427

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/TargetOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGGPUBuiltin.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGenHIP/default-attributes.hip
  clang/test/CodeGenHIP/printf-kind-module-flag.hip
  clang/test/CodeGenHIP/printf_nonhostcall.cpp
  clang/test/CodeGenHIP/sanitize-undefined-null.hip
  clang/test/Driver/hip-options.hip
  llvm/include/llvm/Transforms/Utils/AMDGPUEmitPrintf.h
  llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp

Index: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp
===
--- llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp
+++ llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp
@@ -17,6 +17,9 @@
 #include "llvm/Transforms/Utils/AMDGPUEmitPrintf.h"
 #include "llvm/ADT/SparseBitVector.h"
 #include "llvm/Analysis/ValueTracking.h"
+#include "llvm/Support/DataExtractor.h"
+#include "llvm/Support/MD5.h"
+#include "llvm/Support/MathExtras.h"
 
 using namespace llvm;
 
@@ -179,11 +182,7 @@
 
 // Scan the format string to locate all specifiers, and mark the ones that
 // specify a string, i.e, the "%s" specifier with optional '*' characters.
-static void locateCStrings(SparseBitVector<8> &BV, Value *Fmt) {
-  StringRef Str;
-  if (!getConstantStringInfo(Fmt, Str) || Str.empty())
-return;
-
+static void locateCStrings(SparseBitVector<8> &BV, StringRef Str) {
   static const char ConvSpecifiers[] = "diouxXfFeEgGaAcspn";
   size_t SpecPos = 0;
   // Skip the first argument, the format string.
@@ -207,14 +206,304 @@
   }
 }
 
-Value *llvm::emitAMDGPUPrintfCall(IRBuilder<> &Builder,
-  ArrayRef Args) {
+// helper struct to package the string related data
+struct StringData {
+  StringRef Str;
+  Value *RealSize = nullptr;
+  Value *AlignedSize = nullptr;
+  bool IsConst = true;
+
+  StringData(StringRef ST, Value *RS, Value *AS, bool IC)
+  : Str(ST), RealSize(RS), AlignedSize(AS), IsConst(IC) {}
+};
+
+// Calculates frame size required for current printf expansion and allocates
+// space on printf buffer. Printf frame includes following contents
+// [ ControlDWord , format string/Hash , Arguments (each aligned to 8 byte) ]
+static Value *callBufferedPrintfStart(
+IRBuilder<> &Builder, ArrayRef Args, Value *Fmt,
+bool isConstFmtStr, SparseBitVector<8> &SpecIsCString,
+SmallVectorImpl &StringContents, Value *&ArgSize) {
+  Module *M = Builder.GetInsertBlock()->getModule();
+  Value *NonConstStrLen = nullptr;
+  Value *LenWithNull = nullptr;
+  Value *LenWithNullAligned = nullptr;
+  Value *TempAdd = nullptr;
+
+  // First 4 bytes to be reserved for control dword
+  size_t BufSize = 4;
+  if (isConstFmtStr)
+// First 8 bytes of MD5 hash
+BufSize += 8;
+  else {
+LenWithNull = getStrlenWithNull(Builder, Fmt);
+
+// Align the computed length to next 8 byte boundary
+TempAdd = Builder.CreateAdd(LenWithNull,
+ConstantInt::get(LenWithNull->getType(), 7U));
+NonConstStrLen = Builder.CreateAnd(
+TempAdd, ConstantInt::get(LenWithNull->getType(), ~7U));
+
+StringContents.push_back(
+StringData(StringRef(), LenWithNull, NonConstStrLen, false));
+  }
+
+  for (size_t i = 1; i < Args.size(); i++) {
+if (SpecIsCString.test(i)) {
+  StringRef ArgStr;
+  if (getConstantStringInfo(Args[i], ArgStr)) {
+auto alignedLen = alignTo(ArgStr.size() + 1, 8);
+StringContents.push_back(StringData(
+ArgStr,
+/*RealSize*/ nullptr, /*AlignedSize*/ nullptr, /*IsConst*/ true));
+BufSize += alignedLen;
+  } else {
+LenWithNull = getStrlenWithNull(Builder, Args[i]);
+
+// Align the computed length to next 8 byte boundary
+TempAdd = Builder.CreateAdd(
+LenWithNull, ConstantInt::get(LenWithNull->getType(), 7U));
+LenWithNullAligned = Builder.CreateAnd(
+TempAdd, ConstantInt::get(LenWithNull->getType(), ~7U));
+
+if (NonConstStrLen) {
+  auto Val = Builder.CreateAdd(LenWithNullAligned, NonConstStrLen,
+   "cumulativeAdd");
+  NonConstStrLen = Val;
+} else
+  NonConstStrLen = LenWithNullAligned;
+
+StringContents.push_back(
+StringData("", LenWithNull, LenWithNullAligned, false));
+  }
+} else
+  // We end up expanding non string arguments to 8 bytes
+  BufSize += 8;
+  }
+
+  // calculate final size value to be passed to printf_alloc
+  Value *SizeToReserve = ConstantInt::get(Builder.getInt64Ty(), BufSize, false);
+  SmallVector Alloc_args;
+  if (NonConstStr

[PATCH] D144135: [clang-tidy] Add performance-enum-size check

2023-06-07 Thread Piotr Zegar via Phabricator via cfe-commits
PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144135

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


[PATCH] D144748: [clang-tidy] Add bugprone-empty-catch check

2023-06-07 Thread Piotr Zegar via Phabricator via cfe-commits
PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144748

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


[PATCH] D144429: [clang-tidy] Add bugprone-chained-comparison check

2023-06-07 Thread Piotr Zegar via Phabricator via cfe-commits
PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144429

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


[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check

2023-06-07 Thread Piotr Zegar via Phabricator via cfe-commits
PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146368

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


[PATCH] D149015: [clang-tidy] Added bugprone-inc-dec-in-conditions check

2023-06-07 Thread Piotr Zegar via Phabricator via cfe-commits
PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149015

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


[PATCH] D152194: [StaticAnalyzer] Fix nullptr dereference issue found by static analyzer tool

2023-06-07 Thread Balázs Benics via Phabricator via cfe-commits
steakhal accepted this revision.
steakhal added a comment.
This revision is now accepted and ready to land.

Please update the title and the summary of this patch to reflect what it 
actually achieves.
We also usually use the `[analyzer]` tag only for changes touching 
`StaticAnalyzer` stuff.

After these, you can merge the patch.
Thanks for refactoring the to express the intent more clearly.


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

https://reviews.llvm.org/D152194

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


[PATCH] D152109: Update clang-repl documentation

2023-06-07 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev added a comment.

I think this is heading in a good direction. Please find some comments inline.




Comment at: clang/docs/ClangRepl.rst:60
+
+Clang-Repl-Usage
+





Comment at: clang/docs/ClangRepl.rst:69
+
+Build Instructions:
+===

I find it surprising to see the build instructions as part of the "usage" 
section. Can you move it out?



Comment at: clang/docs/ClangRepl.rst:88
+
+**Clang-repl** is built under llvm-project/build/bin.roceeding into the 
directory **llvm-project/build/bin**
+

A typo?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152109

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


[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

2023-06-07 Thread Vikram Hegde via Phabricator via cfe-commits
vikramRH marked 4 inline comments as done.
vikramRH added inline comments.



Comment at: clang/test/CodeGenHIP/printf_nonhostcall.cpp:137
+
+__device__ float f1 = 3.14f;
+__device__ double f2 = 2.71828;

arsenm wrote:
> Also half 
C++ default arg promotions does not seem to list float16 case and consequently 
clang does not handle it. I have handled this case by promoting to double 
currenlty. Clang however still emits a warning when used with a %f conv 
specifier



Comment at: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp:386-387
+} else {
+  auto IntTy = dyn_cast(Args[i]->getType());
+  if (IntTy && IntTy->getBitWidth() == 32)
+WhatToStore.push_back(

arsenm wrote:
> isIntegerTy(32).
> 
> I also do not understand why only 32-bit integers would be promoted to 64-bit 
> (or why this would be zext). This doesn't match varargs ABI handling, where 
> everything smaller would be promoted to i32.
Right, But default promotions would have happened already, this additional 
promotion is due to runtime processing requirement


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150427

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


[PATCH] D152345: [include-cleaner] Report all specializations if the primary template is introduced by a using-decl.

2023-06-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks!




Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:176
+  void reportSpecializations(SourceLocation Loc, NamedDecl *ND) {
+if (const auto *TD = dyn_cast(ND)) {
+  for (auto *Spec : TD->specializations())

nit: prefer early exit



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:176
+  void reportSpecializations(SourceLocation Loc, NamedDecl *ND) {
+if (const auto *TD = dyn_cast(ND)) {
+  for (auto *Spec : TD->specializations())

kadircet wrote:
> nit: prefer early exit
s/dyn_cast/llvm::dyn_cast



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:179
+report(Loc, Spec, RefType::Ambiguous);
+  SmallVector PartialSpecializations;
+  TD->getPartialSpecializations(PartialSpecializations);

s/SmallVector/llvm::SmallVector



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:202
+  UD->getLocation(), TD);
+  if (const auto *FTD = dyn_cast(TD))
+for (auto *Spec : FTD->specializations())

s/dyn_cast/llvm::dyn_cast





Comment at: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp:257-272
+  testWalk(R"cpp(
+namespace ns {
+template class $ambiguous^Z {}; // primary template
+template class $ambiguous^Z {}; // partial specialization
+template class $ambiguous^Z {}; // partial specialization
+}
+  )cpp",

i think we can merge these into a single test with something like:
```
namespace ns {
  template  struct Z {};
  template  struct Z {};
  template <> struct Z {};
}

using ns::Z; // uses all 3.
```

same applies to var template tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152345

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


[PATCH] D151785: [clangd] Desugar dependent type aliases for auto type hints

2023-06-07 Thread Younan Zhang via Phabricator via cfe-commits
zyounan added inline comments.



Comment at: clang-tools-extra/clangd/InlayHints.cpp:267
 StructuredBindingPolicy = TypeHintPolicy;
 StructuredBindingPolicy.PrintCanonicalTypes = true;
   }

nridge wrote:
> zyounan wrote:
> > `PrintCanonicalTypes` turns on printing default template arguments, which 
> > would prevent the patch from printing, e.g., `std::basic_string`, 
> > within the default length limitation.
> > 
> > ```
> > std::map Map;
> > 
> > for (auto &[Key, Value] : Map) // Key: basic_string > char_traits, allocator>, whose length exceeds the default 
> > threshold.
> > 
> > ```
> > 
> > Is it safe to drop it now? I believe this patch can handle the case the 
> > comment mentioned.
> Unfortunately, I don't think it works in general. I tried commenting out this 
> line and 
> `TypeHints.StructuredBindings_TupleLike` failed.
> 
> (Note, this was with the `BuiltinType` heuristic removed. If we keep the 
> `BuilinType` heuristic, a modified version of the testcase (e.g. struct 
> members are changed from `int` to a class type) still fails.)
> 
Thank you for providing me with the case. I think the flag 
`PrintCanonicalTypes` actually controls two aspects of styles, if I understand 
TypePrinter correctly:

1. For type aliases (a.k.a. typedefs and using alias), the 'canonical' type 
(i.e., the "most" desugared type) is [[ 
https://searchfox.org/llvm/rev/3a458256ee22a0e7c31529de42fa6caa263d88fe/clang/lib/AST/TypePrinter.cpp#179
 | obtained ]] before printing.

2. For template arguments, print [[ 
https://searchfox.org/llvm/rev/3a458256ee22a0e7c31529de42fa6caa263d88fe/clang/lib/AST/TypePrinter.cpp#2158
 | default parameters ]] even they weren't specified.

For 1, we could achieve the same goal at the caller site: For 
`DecompositionDecl`, instead of creating another different printing policy, 
call `QualType::getCanonicalType()` to get the QualType to be passed into 
`addTypeHint`. I did a modification and found that test cases from `TypeHints` 
are all passed even we disable `StructuredBindingPolicy.PrintCanonicalTypes`.

For 2, I think for most of the cases, printing default template parameters is 
an unexpected side effect. (I didn't see any test cases requiring default 
template parameters on structure bindings, but please correct me if I'm missing 
something.)

What do you think? I'd like to submit another review addressing this if it 
looks fine to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151785

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


[clang] 22e95e0 - [clang] Fix assertion while parsing an invalid for loop

2023-06-07 Thread Corentin Jabot via cfe-commits

Author: Corentin Jabot
Date: 2023-06-07T10:31:11+02:00
New Revision: 22e95e0bf375660dd3d083b16a5d92c33559a4a3

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

LOG: [clang] Fix assertion while parsing an invalid for loop

with multiple declarations followed by a colon.

Fixes #63010

Reviewed By: shafik

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Parse/ParseExprCXX.cpp
clang/lib/Parse/ParseStmt.cpp
clang/test/Parser/cxx0x-for-range.cpp
clang/test/Parser/objc-foreach-syntax.m

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d6b498ced6ca2..7ec9b3911ad5d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -267,7 +267,7 @@ Attribute Changes in Clang
   the compilation of the foreign language sources (e.g. Swift).
 - The ``__has_attribute``, ``__has_c_attribute`` and ``__has_cpp_attribute``
   preprocessor operators now return 1 also for attributes defined by plugins.
-- Improve the AST fidelity of ``alignas`` and ``_Alignas`` attribute. Before, 
we 
+- Improve the AST fidelity of ``alignas`` and ``_Alignas`` attribute. Before, 
we
   model ``alignas(type-id)`` as though the user wrote 
``alignas(alignof(type-id))``,
   now we directly use ``alignas(type-id)``.
 
@@ -321,7 +321,7 @@ Improvements to Clang's diagnostics
   (`#62850: `_).
 - Clang now warns when any predefined macro is undefined or redefined, instead
   of only some of them.
-- Clang now correctly diagnoses when the argument to ``alignas`` or 
``_Alignas`` 
+- Clang now correctly diagnoses when the argument to ``alignas`` or 
``_Alignas``
   is an incomplete type.
   (`#55175: `_, and fixes an
   incorrect mention of ``alignof`` in a diagnostic about ``alignas``).
@@ -478,6 +478,9 @@ Bug Fixes in This Version
   (`#63008 `_).
 - Reject increment of bool value in unevaluated contexts after C++17.
   (`#47517 `_).
+- Fix assertion and quality of diagnostic messages in a for loop
+  containing multiple declarations and a range specifier
+  (`#63010 `_).
 
 Bug Fixes to Compiler Builtins
 ^^

diff  --git a/clang/lib/Parse/ParseExprCXX.cpp 
b/clang/lib/Parse/ParseExprCXX.cpp
index 037dc923c47eb..981345a7d55ca 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -2140,8 +2140,6 @@ Parser::ParseCXXCondition(StmtResult *InitStmt, 
SourceLocation Loc,
 DeclGroupPtrTy DG = ParseSimpleDeclaration(
 DeclaratorContext::ForInit, DeclEnd, attrs, DeclSpecAttrs, false, FRI);
 FRI->LoopVar = Actions.ActOnDeclStmt(DG, DeclStart, Tok.getLocation());
-assert((FRI->ColonLoc.isValid() || !DG) &&
-   "cannot find for range declaration");
 return Sema::ConditionResult();
   }
 

diff  --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index aea810e8cf45c..224df67b47f8f 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -2202,9 +2202,7 @@ StmtResult Parser::ParseForStatement(SourceLocation 
*TrailingElseLoc) {
 if (Tok.isNot(tok::semi)) {
   if (!SecondPart.isInvalid())
 Diag(Tok, diag::err_expected_semi_for);
-  else
-// Skip until semicolon or rparen, don't consume it.
-SkipUntil(tok::r_paren, StopAtSemi | StopBeforeMatch);
+  SkipUntil(tok::r_paren, StopAtSemi | StopBeforeMatch);
 }
 
 if (Tok.is(tok::semi)) {

diff  --git a/clang/test/Parser/cxx0x-for-range.cpp 
b/clang/test/Parser/cxx0x-for-range.cpp
index c3276ebeaabbc..a265c4764af56 100644
--- a/clang/test/Parser/cxx0x-for-range.cpp
+++ b/clang/test/Parser/cxx0x-for-range.cpp
@@ -60,3 +60,22 @@ void f() {
   }
 }
 }
+
+namespace GH63010 {
+void foo(int n) {
+int a[] = {1, 2, 3, 4, 5};
+{
+for (auto x = n ? 1 : 2 : a); // expected-error {{expected ';' in 
'for' statement specifier}} \
+// expected-error {{expected expression}}
+for (int i = 1; auto x = n ? 1 : 2 : a); // expected-error {{expected 
';' in 'for' statement specifier}}
+}
+{
+for (auto x = n ? 1 : 2 : a)  // expected-error {{expected ';' in 
'for' statement specifier}} \
+  // expected-error {{expected expression}}
+
+}  // expected-error {{expected statement}}
+{
+for (int i = 1; auto x = n ? 1 : 2 : a) // expected-error {{expected 
';' in 'for' statement specifier}}
+} // expected-error {{expected s

[PATCH] D152009: [clang] Fix assertion while parsing an invalid for loop

2023-06-07 Thread Corentin Jabot via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG22e95e0bf375: [clang] Fix assertion while parsing an invalid 
for loop (authored by cor3ntin).

Changed prior to commit:
  https://reviews.llvm.org/D152009?vs=529186&id=529212#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152009

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/test/Parser/cxx0x-for-range.cpp
  clang/test/Parser/objc-foreach-syntax.m

Index: clang/test/Parser/objc-foreach-syntax.m
===
--- clang/test/Parser/objc-foreach-syntax.m
+++ clang/test/Parser/objc-foreach-syntax.m
@@ -21,8 +21,6 @@
 
 
 static int test7(id keys) {
-  // FIXME: would be nice to suppress the secondary diagnostics.
   for (id key; in keys) ;  // expected-error {{use of undeclared identifier 'in'}} \
-   // expected-error {{expected ';' in 'for' statement specifier}} \
-   // expected-warning {{expression result unused}}
+   // expected-error {{expected ';' in 'for' statement specifier}}
 }
Index: clang/test/Parser/cxx0x-for-range.cpp
===
--- clang/test/Parser/cxx0x-for-range.cpp
+++ clang/test/Parser/cxx0x-for-range.cpp
@@ -60,3 +60,22 @@
   }
 }
 }
+
+namespace GH63010 {
+void foo(int n) {
+int a[] = {1, 2, 3, 4, 5};
+{
+for (auto x = n ? 1 : 2 : a); // expected-error {{expected ';' in 'for' statement specifier}} \
+// expected-error {{expected expression}}
+for (int i = 1; auto x = n ? 1 : 2 : a); // expected-error {{expected ';' in 'for' statement specifier}}
+}
+{
+for (auto x = n ? 1 : 2 : a)  // expected-error {{expected ';' in 'for' statement specifier}} \
+  // expected-error {{expected expression}}
+
+}  // expected-error {{expected statement}}
+{
+for (int i = 1; auto x = n ? 1 : 2 : a) // expected-error {{expected ';' in 'for' statement specifier}}
+} // expected-error {{expected statement}}
+}
+}
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -2202,9 +2202,7 @@
 if (Tok.isNot(tok::semi)) {
   if (!SecondPart.isInvalid())
 Diag(Tok, diag::err_expected_semi_for);
-  else
-// Skip until semicolon or rparen, don't consume it.
-SkipUntil(tok::r_paren, StopAtSemi | StopBeforeMatch);
+  SkipUntil(tok::r_paren, StopAtSemi | StopBeforeMatch);
 }
 
 if (Tok.is(tok::semi)) {
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -2140,8 +2140,6 @@
 DeclGroupPtrTy DG = ParseSimpleDeclaration(
 DeclaratorContext::ForInit, DeclEnd, attrs, DeclSpecAttrs, false, FRI);
 FRI->LoopVar = Actions.ActOnDeclStmt(DG, DeclStart, Tok.getLocation());
-assert((FRI->ColonLoc.isValid() || !DG) &&
-   "cannot find for range declaration");
 return Sema::ConditionResult();
   }
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -267,7 +267,7 @@
   the compilation of the foreign language sources (e.g. Swift).
 - The ``__has_attribute``, ``__has_c_attribute`` and ``__has_cpp_attribute``
   preprocessor operators now return 1 also for attributes defined by plugins.
-- Improve the AST fidelity of ``alignas`` and ``_Alignas`` attribute. Before, we 
+- Improve the AST fidelity of ``alignas`` and ``_Alignas`` attribute. Before, we
   model ``alignas(type-id)`` as though the user wrote ``alignas(alignof(type-id))``,
   now we directly use ``alignas(type-id)``.
 
@@ -321,7 +321,7 @@
   (`#62850: `_).
 - Clang now warns when any predefined macro is undefined or redefined, instead
   of only some of them.
-- Clang now correctly diagnoses when the argument to ``alignas`` or ``_Alignas`` 
+- Clang now correctly diagnoses when the argument to ``alignas`` or ``_Alignas``
   is an incomplete type.
   (`#55175: `_, and fixes an
   incorrect mention of ``alignof`` in a diagnostic about ``alignas``).
@@ -478,6 +478,9 @@
   (`#63008 `_).
 - Reject increment of bool value in unevaluated contexts after C++17.
   (`#47517 `_).
+- Fix assertion and quality of diagnostic messages in a for loop
+  containing multiple declarations and a range spec

[PATCH] D152318: [Fuchsia] Add llvm-strings to Fuchsia clang build

2023-06-07 Thread Haowei Wu via Phabricator via cfe-commits
haowei added a comment.

In D152318#4402234 , @mstorsjo wrote:

> FWIW, I'm curious about where you need `llvm-strings` in a MinGW setting. 
> While it does match a GNU binutils tool, I'm kinda curious where it is needed.

llvm-strings exists in the `output_dir/bin` from a local llvm-mingw build using 
the scripts in your repo. So I think we might need it. I didn't dig into it why 
it exists in the first place. The current effort is to replicate the results. 
If it is not required I think there is no harm to added to Fuchsia's Clang 
configuration.

We are trying to build and setup a automated builder for llvm-mingw project 
using Fuchsia's Clang toolchain (instead of a pinned version of LLVM from 
llvm-mingw's build script) and using it to build some Windows host tools.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152318

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


[PATCH] D152345: [include-cleaner] Report all specializations if the primary template is introduced by a using-decl.

2023-06-07 Thread Haojian Wu via Phabricator via cfe-commits
hokein updated this revision to Diff 529215.
hokein marked 5 inline comments as done.
hokein added a comment.

address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152345

Files:
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -252,6 +252,36 @@
"auto x = [] { ^foo(); };"),
   ElementsAre(Decl::FunctionTemplate));
 }
+TEST(WalkAST, TemplateSpecializationsFromUsingDecl) {
+  // Class templates
+  testWalk(R"cpp(
+namespace ns {
+template class $ambiguous^Z {};  // primary template
+template class $ambiguous^Z {};  // partial specialization
+template<> class $ambiguous^Z {};// full specialization
+}
+  )cpp",
+   "using ns::^Z;");
+
+  // Var templates
+  testWalk(R"cpp(
+namespace ns {
+template T $ambiguous^foo;  // primary template
+template T $ambiguous^foo;  // partial specialization
+template<> int* $ambiguous^foo; // full specialization
+}
+  )cpp",
+   "using ns::^foo;");
+  // Function templates, no partial template specializations.
+  testWalk(R"cpp(
+namespace ns {
+template void $ambiguous^function(T);  // primary template
+template<> void $ambiguous^function(int);   // full specialization
+}
+  )cpp",
+   "using ns::^function;");
+}
+
 
 TEST(WalkAST, Alias) {
   testWalk(R"cpp(
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -24,6 +24,7 @@
 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Casting.h"
 
 namespace clang::include_cleaner {
@@ -169,12 +170,39 @@
 return true;
   }
 
+  // Report all (partial) specializations of a class/var template decl.
+  template 
+  void reportSpecializations(SourceLocation Loc, NamedDecl *ND) {
+const auto *TD = llvm::dyn_cast(ND);
+if (!TD)
+  return;
+
+for (auto *Spec : TD->specializations())
+  report(Loc, Spec, RefType::Ambiguous);
+llvm::SmallVector PartialSpecializations;
+TD->getPartialSpecializations(PartialSpecializations);
+for (auto *PartialSpec : PartialSpecializations)
+  report(Loc, PartialSpec, RefType::Ambiguous);
+  }
   bool VisitUsingDecl(UsingDecl *UD) {
 for (const auto *Shadow : UD->shadows()) {
   auto *TD = Shadow->getTargetDecl();
   auto IsUsed = TD->isUsed() || TD->isReferenced();
   report(UD->getLocation(), TD,
  IsUsed ? RefType::Explicit : RefType::Ambiguous);
+
+  // All (partial) template specializations are visible via a using-decl,
+  // However a using-decl only refers to the primary template (per C++ name
+  // lookup). Thus, we need to manually report all specializations.
+  reportSpecializations(
+  UD->getLocation(), TD);
+  reportSpecializations(
+  UD->getLocation(), TD);
+  if (const auto *FTD = llvm::dyn_cast(TD))
+for (auto *Spec : FTD->specializations())
+  report(UD->getLocation(), Spec, RefType::Ambiguous);
 }
 return true;
   }


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -252,6 +252,36 @@
"auto x = [] { ^foo(); };"),
   ElementsAre(Decl::FunctionTemplate));
 }
+TEST(WalkAST, TemplateSpecializationsFromUsingDecl) {
+  // Class templates
+  testWalk(R"cpp(
+namespace ns {
+template class $ambiguous^Z {};  // primary template
+template class $ambiguous^Z {};  // partial specialization
+template<> class $ambiguous^Z {};// full specialization
+}
+  )cpp",
+   "using ns::^Z;");
+
+  // Var templates
+  testWalk(R"cpp(
+namespace ns {
+template T $ambiguous^foo;  // primary template
+template T $ambiguous^foo;  // partial specialization
+template<> int* $ambiguous^foo; // full specialization
+}
+  )cpp",
+   "using ns::^foo;");
+  // Function templates, no partial template specializations.
+  testWalk(R"cpp(
+namespace ns {
+template void $ambiguous^function(T);  // primary template
+template<> void $ambiguous^function(int);   // full specialization
+}
+  )cpp",
+   "using ns::^function;");
+}
+
 
 TEST(WalkAST, Alias) {
   testWalk(R"cpp(
Index: clang-tools-extra/inclu

cfe-commits@lists.llvm.org

2023-06-07 Thread Jens Massberg via Phabricator via cfe-commits
massberg updated this revision to Diff 529216.
massberg added a comment.

Fix status of paper in status page to unreleased.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148924

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
  clang/test/CodeGenCXX/virtual-compare.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -532,7 +532,7 @@
   

 https://wg21.link/p2002r1";>P2002R1
-Partial
+Clang 17
   
   
 https://wg21.link/p2085r0";>P2085R0
Index: clang/test/CodeGenCXX/virtual-compare.cpp
===
--- clang/test/CodeGenCXX/virtual-compare.cpp
+++ clang/test/CodeGenCXX/virtual-compare.cpp
@@ -21,8 +21,6 @@
   virtual std::strong_ordering operator<=>(const A &) const & = default;
   // CHECK-SAME: @_ZN1A1gEv
   virtual void g();
-  // CHECK-SAME: @_ZNKO1AssERKS_
-  virtual std::strong_ordering operator<=>(const A &) const && = default;
   // CHECK-SAME: @_ZN1A1hEv
   virtual void h();
 
@@ -35,9 +33,6 @@
 
   // CHECK-SAME: @_ZNKR1AeqERKS_
   // implicit virtual A &operator==(const A&) const & = default;
-
-  // CHECK-SAME: @_ZNKO1AeqERKS_
-  // implicit virtual A &operator==(const A&) const && = default;
 };
 
 // For Y:
Index: clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
===
--- clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
+++ clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
@@ -15,7 +15,8 @@
 
   bool operator<(const A&) const;
   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 a defaulted comparison operator}}
+  bool operator>=(const A&) const volatile = default; // expected-error {{defaulted comparison function must not be volatile}}
   bool operator<=>(const A&) = default; // expected-error {{defaulted member three-way comparison operator must be const-qualified}}
   bool operator>=(const B&) const = default; // expected-error-re {{invalid parameter type for defaulted relational comparison operator; found 'const B &', expected 'const A &'{{$
   static bool operator>(const B&) = default; // expected-error {{overloaded 'operator>' cannot be a static member function}}
@@ -195,6 +196,11 @@
 friend bool operator==(const S6&, const S6&) = default; // expected-error {{because it was already declared outside}}
 };
 
+struct S7 {
+  bool operator==(S7 const &) const &&;
+};
+bool S7::operator==(S7 const &) const && = default; // expected-error {{ref-qualifier '&&' is not allowed on a defaulted comparison operator}}
+
 enum e {};
 bool operator==(e, int) = default; // expected-error{{expected class or reference to a constant class}}
 
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -8582,8 +8582,8 @@
   // C++2a [class.compare.default]p1:
   //   A defaulted comparison operator function for some class C shall be a
   //   non-template function declared in the member-specification of C that is
-  //-- a non-static const member of C having one parameter of type
-  //   const C&, or
+  //-- a non-static const non-volatile member of C having one parameter of type
+  //   const C& and either no ref-qualifier or the ref-qualifier &, or
   //-- a friend of C having two parameters of type const C& or two
   //   parameters of type C.
 
@@ -8593,6 +8593,17 @@
 auto *MD = cast(FD);
 assert(!MD->isStatic() && "comparison function cannot be a static member");
 
+if (MD->getRefQualifier() == RQ_RValue) {
+  Diag(MD->getLocation(), diag::err_ref_qualifier_comparison_operator);
+
+  // Remove the ref qualifier to recover.
+  const auto *FPT = MD->getType()->castAs();
+  FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+  EPI.RefQualifier = RQ_None;
+  MD->setType(Context.getFunctionType(FPT->getReturnType(),
+  FPT->getParamTypes(), EPI));
+}
+
 // If we're out-of-class, this is the class we're comparing.
 if (!RD)
   RD = MD->getParent();
@@ -8615,6 +8626,17 @@
   MD->setType(Context.getFunctionType(FPT->getReturnType(),
   FPT->getParamTypes(), EPI));
 }
+
+if (MD->isVolatile()) {
+  Diag(MD->getLocation(), diag::err_volatile_comparison_operator)

[PATCH] D148088: [RFC][clangd] Move preamble index out of document open critical path

2023-06-07 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

thanks, mostly LG. some nits around comments and request for a knob :)




Comment at: clang-tools-extra/clangd/ClangdServer.cpp:80
+
+auto Task = [FIndex(FIndex), Path(Path.str()), Version(Version.str()),
+ ASTCtx(std::move(ASTCtx)),

let's leave a comment here saying that `FIndex` outlives the 
`UpdateIndexCallbacks`.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:83
+ CanonIncludes(CanonIncludes)]() mutable {
+  FIndex->updatePreamble(Path, Version, ASTCtx.getASTContext(),
+ ASTCtx.getPreprocessor(), *CanonIncludes);

can you add a `trace::Span Tracer("PreambleIndexing");` here



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:87
 
-if (FIndex)
-  FIndex->updatePreamble(Path, Version, Ctx, PP, CanonIncludes);
+if (Tasks) {
+  Tasks->runAsync("Preamble indexing for:" + Path + Version,

can you also introduce a `bool AsyncPreambleIndexing = false;` into 
`ClangdServer::Options` struct and pass it into `UpdateIndexCallbacks` and make 
this conditional on that? after testing it for couple weeks, we can make it the 
default.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:211
 IndexTasks.emplace();
+
   // Pass a callback into `WorkScheduler` to extract symbols from a newly

nit: let's revert this change



Comment at: clang-tools-extra/clangd/Preamble.cpp:105
   void AfterExecute(CompilerInstance &CI) override {
-if (ParsedCallback) {
-  trace::Span Tracer("Running PreambleCallback");
-  ParsedCallback(CI.getASTContext(), CI.getPreprocessor(), CanonIncludes);
+// Set PrecompilePreambleConsumer/PCHGenerator to null.
+if (CI.getASTReader()) {

the comments are still just talking about what the code is already doing, and 
not "why".

```
// ASTContext and CompilerInstance can keep references to ASTConsumer 
(PCHGenerator in case of preamble builds).
// These references will become dangling after preamble build finishes, even if 
they didn't we don't want operations on the preamble to mutate PCH afterwards.
// So clear those references explicitly here.
```



Comment at: clang-tools-extra/clangd/Preamble.cpp:696
+  auto Ctx = CapturedInfo.takeLife();
+  // Set the FileManager VFS to consuming FS.
+  auto StatCacheFS = Result->StatCache->getConsumingFS(VFS);

again comment is talking about "what" the code does rather than "why", what 
about:
```
// Stat cache is thread safe only when there are no producers. Hence change the 
VFS underneath to a consuming fs.
```



Comment at: clang-tools-extra/clangd/Preamble.cpp:698
+  auto StatCacheFS = Result->StatCache->getConsumingFS(VFS);
+  Ctx->getFileManager().setVirtualFileSystem(StatCacheFS);
+  // While extending the life of FileMgr and VFS, StatCache should also be

nit: std::move(StatCacheFS) or just inline the initializer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148088

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


[PATCH] D152345: [include-cleaner] Report all specializations if the primary template is introduced by a using-decl.

2023-06-07 Thread Haojian Wu 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 rG100ffbf991e7: [include-cleaner] Report all specializations 
if the primary template is… (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152345

Files:
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -252,6 +252,36 @@
"auto x = [] { ^foo(); };"),
   ElementsAre(Decl::FunctionTemplate));
 }
+TEST(WalkAST, TemplateSpecializationsFromUsingDecl) {
+  // Class templates
+  testWalk(R"cpp(
+namespace ns {
+template class $ambiguous^Z {};  // primary template
+template class $ambiguous^Z {};  // partial specialization
+template<> class $ambiguous^Z {};// full specialization
+}
+  )cpp",
+   "using ns::^Z;");
+
+  // Var templates
+  testWalk(R"cpp(
+namespace ns {
+template T $ambiguous^foo;  // primary template
+template T $ambiguous^foo;  // partial specialization
+template<> int* $ambiguous^foo; // full specialization
+}
+  )cpp",
+   "using ns::^foo;");
+  // Function templates, no partial template specializations.
+  testWalk(R"cpp(
+namespace ns {
+template void $ambiguous^function(T);  // primary template
+template<> void $ambiguous^function(int);   // full specialization
+}
+  )cpp",
+   "using ns::^function;");
+}
+
 
 TEST(WalkAST, Alias) {
   testWalk(R"cpp(
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -24,6 +24,7 @@
 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Casting.h"
 
 namespace clang::include_cleaner {
@@ -169,12 +170,39 @@
 return true;
   }
 
+  // Report all (partial) specializations of a class/var template decl.
+  template 
+  void reportSpecializations(SourceLocation Loc, NamedDecl *ND) {
+const auto *TD = llvm::dyn_cast(ND);
+if (!TD)
+  return;
+
+for (auto *Spec : TD->specializations())
+  report(Loc, Spec, RefType::Ambiguous);
+llvm::SmallVector PartialSpecializations;
+TD->getPartialSpecializations(PartialSpecializations);
+for (auto *PartialSpec : PartialSpecializations)
+  report(Loc, PartialSpec, RefType::Ambiguous);
+  }
   bool VisitUsingDecl(UsingDecl *UD) {
 for (const auto *Shadow : UD->shadows()) {
   auto *TD = Shadow->getTargetDecl();
   auto IsUsed = TD->isUsed() || TD->isReferenced();
   report(UD->getLocation(), TD,
  IsUsed ? RefType::Explicit : RefType::Ambiguous);
+
+  // All (partial) template specializations are visible via a using-decl,
+  // However a using-decl only refers to the primary template (per C++ name
+  // lookup). Thus, we need to manually report all specializations.
+  reportSpecializations(
+  UD->getLocation(), TD);
+  reportSpecializations(
+  UD->getLocation(), TD);
+  if (const auto *FTD = llvm::dyn_cast(TD))
+for (auto *Spec : FTD->specializations())
+  report(UD->getLocation(), Spec, RefType::Ambiguous);
 }
 return true;
   }


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -252,6 +252,36 @@
"auto x = [] { ^foo(); };"),
   ElementsAre(Decl::FunctionTemplate));
 }
+TEST(WalkAST, TemplateSpecializationsFromUsingDecl) {
+  // Class templates
+  testWalk(R"cpp(
+namespace ns {
+template class $ambiguous^Z {};  // primary template
+template class $ambiguous^Z {};  // partial specialization
+template<> class $ambiguous^Z {};// full specialization
+}
+  )cpp",
+   "using ns::^Z;");
+
+  // Var templates
+  testWalk(R"cpp(
+namespace ns {
+template T $ambiguous^foo;  // primary template
+template T $ambiguous^foo;  // partial specialization
+template<> int* $ambiguous^foo; // full specialization
+}
+  )cpp",
+   "using ns::^foo;");
+  // Function templates, no partial template specializations.
+  testWalk(R"cpp(
+namespace ns {
+template void $ambiguous^function(T);  // primary template
+template<> void $ambiguous^function(int);   // full specialization
+}
+  )c

[clang-tools-extra] 100ffbf - [include-cleaner] Report all specializations if the primary template is introduced by a using-decl.

2023-06-07 Thread Haojian Wu via cfe-commits

Author: Haojian Wu
Date: 2023-06-07T10:52:36+02:00
New Revision: 100ffbf991e78bb3e94a9d5c8d656a9b8648082d

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

LOG: [include-cleaner] Report all specializations if the primary template is 
introduced by a using-decl.

This will fix unused-include false positive.

```
// primary.h
namespace ns {
template class Z {}; // primary template
}

// partial.h
namespace ns {
template class Z {}; // partial specialization
}

// main.cpp

using ns::Z; // refs to the primary
void k() {
  Z z; // use the partial specialization
}
```

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

Added: 


Modified: 
clang-tools-extra/include-cleaner/lib/WalkAST.cpp
clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp

Removed: 




diff  --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp 
b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
index c593192c50191..fc392fec36865 100644
--- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -24,6 +24,7 @@
 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/STLFunctionalExtras.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Casting.h"
 
 namespace clang::include_cleaner {
@@ -169,12 +170,39 @@ class ASTWalker : public RecursiveASTVisitor {
 return true;
   }
 
+  // Report all (partial) specializations of a class/var template decl.
+  template 
+  void reportSpecializations(SourceLocation Loc, NamedDecl *ND) {
+const auto *TD = llvm::dyn_cast(ND);
+if (!TD)
+  return;
+
+for (auto *Spec : TD->specializations())
+  report(Loc, Spec, RefType::Ambiguous);
+llvm::SmallVector PartialSpecializations;
+TD->getPartialSpecializations(PartialSpecializations);
+for (auto *PartialSpec : PartialSpecializations)
+  report(Loc, PartialSpec, RefType::Ambiguous);
+  }
   bool VisitUsingDecl(UsingDecl *UD) {
 for (const auto *Shadow : UD->shadows()) {
   auto *TD = Shadow->getTargetDecl();
   auto IsUsed = TD->isUsed() || TD->isReferenced();
   report(UD->getLocation(), TD,
  IsUsed ? RefType::Explicit : RefType::Ambiguous);
+
+  // All (partial) template specializations are visible via a using-decl,
+  // However a using-decl only refers to the primary template (per C++ name
+  // lookup). Thus, we need to manually report all specializations.
+  reportSpecializations(
+  UD->getLocation(), TD);
+  reportSpecializations(
+  UD->getLocation(), TD);
+  if (const auto *FTD = llvm::dyn_cast(TD))
+for (auto *Spec : FTD->specializations())
+  report(UD->getLocation(), Spec, RefType::Ambiguous);
 }
 return true;
   }

diff  --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp 
b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
index 0d504c3303f9e..525f645ec91ef 100644
--- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -252,6 +252,36 @@ TEST(WalkAST, FunctionTemplates) {
"auto x = [] { ^foo(); };"),
   ElementsAre(Decl::FunctionTemplate));
 }
+TEST(WalkAST, TemplateSpecializationsFromUsingDecl) {
+  // Class templates
+  testWalk(R"cpp(
+namespace ns {
+template class $ambiguous^Z {};  // primary template
+template class $ambiguous^Z {};  // partial specialization
+template<> class $ambiguous^Z {};// full specialization
+}
+  )cpp",
+   "using ns::^Z;");
+
+  // Var templates
+  testWalk(R"cpp(
+namespace ns {
+template T $ambiguous^foo;  // primary template
+template T $ambiguous^foo;  // partial specialization
+template<> int* $ambiguous^foo; // full specialization
+}
+  )cpp",
+   "using ns::^foo;");
+  // Function templates, no partial template specializations.
+  testWalk(R"cpp(
+namespace ns {
+template void $ambiguous^function(T);  // primary template
+template<> void $ambiguous^function(int);   // full specialization
+}
+  )cpp",
+   "using ns::^function;");
+}
+
 
 TEST(WalkAST, Alias) {
   testWalk(R"cpp(



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


[PATCH] D148461: [clang-tidy] Support C++17/20 in bugprone-exception-escape

2023-06-07 Thread Domján Dániel via Phabricator via cfe-commits
isuckatcs added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:306
+
+  if (From->isMemberPointerType() || To->isMemberPointerType())
 return false;

isuckatcs wrote:
> Please cover this line with both positive and negative test cases.
> 
> Also upon looking up both [[ 
> https://www.open-std.org/jtc1/sc22/wg21/docs/standards | N4849 ]] (C++ 20 
> draft) and [[ https://github.com/cplusplus/draft/releases | N4917]] (C++ 23 
> draft), they both say for qualification conversion that 
> 
> 
> > each P_i is ... pointer to member of class C_i of type, ...
> 
> Why are we not allowing them if the standard is at least C++ 20?
Please resolve this thread.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148461

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


[PATCH] D152318: [Fuchsia] Add llvm-strings to Fuchsia clang build

2023-06-07 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment.

In D152318#4402493 , @haowei wrote:

> In D152318#4402234 , @mstorsjo 
> wrote:
>
>> FWIW, I'm curious about where you need `llvm-strings` in a MinGW setting. 
>> While it does match a GNU binutils tool, I'm kinda curious where it is 
>> needed.
>
> llvm-strings exists in the `output_dir/bin` from a local llvm-mingw build 
> using the scripts in your repo. So I think we might need it. I didn't dig 
> into it why it exists in the first place. The current effort is to replicate 
> the results. If it is not required I think there is no harm to added to 
> Fuchsia's Clang configuration.
>
> We are trying to build and setup a automated builder for llvm-mingw project 
> using Fuchsia's Clang toolchain (instead of a pinned version of LLVM from 
> llvm-mingw's build script) and using it to build some Windows host tools.

Ok, I see. Yeah there's not much effort in providing it. I tried digging in to 
see when I added it, and it seems I added it in 
https://github.com/mstorsjo/llvm-mingw/commit/55e8aef024faa83c17e18ece15c191bbc1ab7936,
 without any specific explanation why this particular tool was added, so I 
guess I added it mostly for completeness. In any case, it doesn't hurt to 
include it for consistency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152318

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


[PATCH] D152351: [clang] Add __builtin_isfpclass

2023-06-07 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff created this revision.
sepavloff added reviewers: rjmccall, aaron.ballman, arsenm, kpn, qiucf, 
efriedma.
Herald added a subscriber: hiraditya.
Herald added a project: All.
sepavloff requested review of this revision.
Herald added subscribers: llvm-commits, wdng.
Herald added projects: clang, LLVM.

A new builting function __builtin_isfpclass is added. It is called as:

  __builtin_isfpclass(, )

and returns an integer value, which is non-zero if the floating point
argument belongs to one of the classes specified by the first argument,
and zero otherwise. The set of classes is an integer value, where each
value class is represented by a bit. There are ten data classes, as
defined by the IEEE-754 standard, they are represented by bits:

  0x0001 - Signaling NaN
  0x0002 - Quiet NaN
  0x0004 - Negative infinity
  0x0008 - Negative normal
  0x0010 - Negative subnormal
  0x0020 - Negative zero
  0x0040 - Positive zero
  0x0080 - Positive subnormal
  0x0100 - Positive normal
  0x0200 - Positive infinity

The data class encoding is identical to that used in llvm.is.fpclass
function.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152351

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins.c
  clang/test/CodeGen/isfpclass.c
  llvm/include/llvm/IR/IRBuilder.h
  llvm/lib/IR/IRBuilder.cpp

Index: llvm/lib/IR/IRBuilder.cpp
===
--- llvm/lib/IR/IRBuilder.cpp
+++ llvm/lib/IR/IRBuilder.cpp
@@ -1375,6 +1375,14 @@
   return Fn;
 }
 
+Value *IRBuilderBase::createIsFPClass(Value *FPNum, unsigned Test) {
+  auto TestV = llvm::ConstantInt::get(Type::getInt32Ty(Context), Test);
+  Module *M = BB->getParent()->getParent();
+  Function *FnIsFPClass =
+  Intrinsic::getDeclaration(M, Intrinsic::is_fpclass, {FPNum->getType()});
+  return CreateCall(FnIsFPClass, {FPNum, TestV});
+}
+
 CallInst *IRBuilderBase::CreateAlignmentAssumptionHelper(const DataLayout &DL,
  Value *PtrValue,
  Value *AlignValue,
Index: llvm/include/llvm/IR/IRBuilder.h
===
--- llvm/include/llvm/IR/IRBuilder.h
+++ llvm/include/llvm/IR/IRBuilder.h
@@ -2518,6 +2518,8 @@
  unsigned Index, unsigned FieldIndex,
  MDNode *DbgInfo);
 
+  Value *createIsFPClass(Value *FPNum, unsigned Test);
+
 private:
   /// Helper function that creates an assume intrinsic call that
   /// represents an alignment assumption on the provided pointer \p PtrValue
Index: clang/test/CodeGen/isfpclass.c
===
--- /dev/null
+++ clang/test/CodeGen/isfpclass.c
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -O1 -emit-llvm %s -o - | FileCheck %s
+
+_Bool check_isfpclass_finite(float x) {
+  return __builtin_isfpclass(504 /*Finite*/, x);
+}
+// CHECK-LABEL: define {{.*}} i1 @check_isfpclass_finite(
+// CHECK: call i1 @llvm.is.fpclass.f32(float {{.*}}, i32 504)
+
+_Bool check_isfpclass_finite_strict(float x) {
+#pragma STDC FENV_ACCESS ON
+  return __builtin_isfpclass(504 /*Finite*/, x);
+}
+// CHECK-LABEL: define {{.*}} i1 @check_isfpclass_finite_strict(
+// CHECK: call i1 @llvm.is.fpclass.f32(float {{.*}}, i32 504)
+
+_Bool check_isfpclass_nan_f32(float x) {
+#pragma STDC FENV_ACCESS ON
+  return __builtin_isfpclass(3 /*NaN*/, x);
+}
+// CHECK-LABEL: define {{.*}} i1 @check_isfpclass_nan_f32(
+// CHECK: call i1 @llvm.is.fpclass.f32(float {{.*}}, i32 3)
+
+_Bool check_isfpclass_nan_f64(double x) {
+#pragma STDC FENV_ACCESS ON
+  return __builtin_isfpclass(3 /*NaN*/, x);
+}
+// CHECK-LABEL: define {{.*}} i1 @check_isfpclass_nan_f64(
+// CHECK: call i1 @llvm.is.fpclass.f64(double {{.*}}, i32 3)
Index: clang/test/CodeGen/builtins.c
===
--- clang/test/CodeGen/builtins.c
+++ clang/test/CodeGen/builtins.c
@@ -63,6 +63,7 @@
   P(isinf, (1.));
   P(isinf_sign, (1.));
   P(isnan, (1.));
+  P(isfpclass, (1, 1.));
 
   // Bitwise & Numeric Functions
 
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -3143,6 +3143,17 @@
 return RValue::get(V);
   }
 
+  case Builtin::BI__builtin_isfpclass: {
+Expr::EvalResult Result;
+if (!E->getArg(0)->EvaluateAsInt(Result, CGM.getContext()))
+  break;
+uint64_t Test = Result.Val.getInt().getLimitedValue();
+CodeGenFunction::CGFPOptionsRAII FPOptsRAII(*this, E);
+Value *V = EmitScalarExpr(E->getArg(1));
+return RValue::get(Builder.CreateZExt(Builder.createIsFPClass(V, Test),
+  ConvertType(E->getTy

[PATCH] D150427: [AMDGPU] Non hostcall printf support for HIP

2023-06-07 Thread Vikram Hegde via Phabricator via cfe-commits
vikramRH updated this revision to Diff 529219.
vikramRH marked an inline comment as done.
vikramRH added a comment.

updated formating


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150427

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/TargetOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGGPUBuiltin.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGenHIP/default-attributes.hip
  clang/test/CodeGenHIP/printf-kind-module-flag.hip
  clang/test/CodeGenHIP/printf_nonhostcall.cpp
  clang/test/CodeGenHIP/sanitize-undefined-null.hip
  clang/test/Driver/hip-options.hip
  llvm/include/llvm/Transforms/Utils/AMDGPUEmitPrintf.h
  llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp

Index: llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp
===
--- llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp
+++ llvm/lib/Transforms/Utils/AMDGPUEmitPrintf.cpp
@@ -17,6 +17,9 @@
 #include "llvm/Transforms/Utils/AMDGPUEmitPrintf.h"
 #include "llvm/ADT/SparseBitVector.h"
 #include "llvm/Analysis/ValueTracking.h"
+#include "llvm/Support/DataExtractor.h"
+#include "llvm/Support/MD5.h"
+#include "llvm/Support/MathExtras.h"
 
 using namespace llvm;
 
@@ -179,11 +182,7 @@
 
 // Scan the format string to locate all specifiers, and mark the ones that
 // specify a string, i.e, the "%s" specifier with optional '*' characters.
-static void locateCStrings(SparseBitVector<8> &BV, Value *Fmt) {
-  StringRef Str;
-  if (!getConstantStringInfo(Fmt, Str) || Str.empty())
-return;
-
+static void locateCStrings(SparseBitVector<8> &BV, StringRef Str) {
   static const char ConvSpecifiers[] = "diouxXfFeEgGaAcspn";
   size_t SpecPos = 0;
   // Skip the first argument, the format string.
@@ -207,14 +206,305 @@
   }
 }
 
-Value *llvm::emitAMDGPUPrintfCall(IRBuilder<> &Builder,
-  ArrayRef Args) {
+// helper struct to package the string related data
+struct StringData {
+  StringRef Str;
+  Value *RealSize = nullptr;
+  Value *AlignedSize = nullptr;
+  bool IsConst = true;
+
+  StringData(StringRef ST, Value *RS, Value *AS, bool IC)
+  : Str(ST), RealSize(RS), AlignedSize(AS), IsConst(IC) {}
+};
+
+// Calculates frame size required for current printf expansion and allocates
+// space on printf buffer. Printf frame includes following contents
+// [ ControlDWord , format string/Hash , Arguments (each aligned to 8 byte) ]
+static Value *callBufferedPrintfStart(
+IRBuilder<> &Builder, ArrayRef Args, Value *Fmt,
+bool isConstFmtStr, SparseBitVector<8> &SpecIsCString,
+SmallVectorImpl &StringContents, Value *&ArgSize) {
+  Module *M = Builder.GetInsertBlock()->getModule();
+  Value *NonConstStrLen = nullptr;
+  Value *LenWithNull = nullptr;
+  Value *LenWithNullAligned = nullptr;
+  Value *TempAdd = nullptr;
+
+  // First 4 bytes to be reserved for control dword
+  size_t BufSize = 4;
+  if (isConstFmtStr)
+// First 8 bytes of MD5 hash
+BufSize += 8;
+  else {
+LenWithNull = getStrlenWithNull(Builder, Fmt);
+
+// Align the computed length to next 8 byte boundary
+TempAdd = Builder.CreateAdd(LenWithNull,
+ConstantInt::get(LenWithNull->getType(), 7U));
+NonConstStrLen = Builder.CreateAnd(
+TempAdd, ConstantInt::get(LenWithNull->getType(), ~7U));
+
+StringContents.push_back(
+StringData(StringRef(), LenWithNull, NonConstStrLen, false));
+  }
+
+  for (size_t i = 1; i < Args.size(); i++) {
+if (SpecIsCString.test(i)) {
+  StringRef ArgStr;
+  if (getConstantStringInfo(Args[i], ArgStr)) {
+auto alignedLen = alignTo(ArgStr.size() + 1, 8);
+StringContents.push_back(StringData(
+ArgStr,
+/*RealSize*/ nullptr, /*AlignedSize*/ nullptr, /*IsConst*/ true));
+BufSize += alignedLen;
+  } else {
+LenWithNull = getStrlenWithNull(Builder, Args[i]);
+
+// Align the computed length to next 8 byte boundary
+TempAdd = Builder.CreateAdd(
+LenWithNull, ConstantInt::get(LenWithNull->getType(), 7U));
+LenWithNullAligned = Builder.CreateAnd(
+TempAdd, ConstantInt::get(LenWithNull->getType(), ~7U));
+
+if (NonConstStrLen) {
+  auto Val = Builder.CreateAdd(LenWithNullAligned, NonConstStrLen,
+   "cumulativeAdd");
+  NonConstStrLen = Val;
+} else
+  NonConstStrLen = LenWithNullAligned;
+
+StringContents.push_back(
+StringData("", LenWithNull, LenWithNullAligned, false));
+  }
+} else
+  // We end up expanding non string arguments to 8 bytes
+  BufSize += 8;
+  }
+
+  // calculate final size value to be passed to printf_alloc
+  Value *SizeToReserve = ConstantInt::get(Builder.getInt64Ty(), BufSize, false);
+  Small

[PATCH] D151553: [clang] Fix consteval operators in template contexts

2023-06-07 Thread Mariya Podchishchaeva via Phabricator via cfe-commits
Fznamznon updated this revision to Diff 529220.
Fznamznon added a comment.

Use 1-element UnresolvedSet, rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151553

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/consteval-operators.cpp
  clang/test/SemaCXX/overloaded-operator.cpp

Index: clang/test/SemaCXX/overloaded-operator.cpp
===
--- clang/test/SemaCXX/overloaded-operator.cpp
+++ clang/test/SemaCXX/overloaded-operator.cpp
@@ -585,3 +585,16 @@
   float &operator->*(B, B);
   template void f();
 }
+
+namespace test {
+namespace A {
+template T f(T t) {
+  T operator+(T, T);
+  return t + t;
+}
+}
+namespace B {
+  struct X {};
+}
+void g(B::X x) { A::f(x); }
+}
Index: clang/test/SemaCXX/consteval-operators.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/consteval-operators.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -std=c++2a -emit-llvm-only -Wno-unused-value %s -verify
+
+// expected-no-diagnostics
+
+struct A {
+  consteval A operator+() { return {}; }
+};
+consteval A operator~(A) { return {}; }
+consteval A operator+(A, A) { return {}; }
+
+template  void f() {
+  A a;
+  A b = ~a;
+  A c = a + a;
+  A d = +a;
+}
+template void f();
+
+template  void foo() {
+  T a;
+  T b = ~a;
+  T c = a + a;
+  T d = +a;
+}
+
+template void foo();
+
+template  struct B { DataT D; };
+
+template 
+consteval B operator+(B lhs, B rhs) {
+  return B{lhs.D + rhs.D};
+}
+
+template  consteval T template_add(T a, T b) { return a + b; }
+
+consteval B non_template_add(B a, B b) { return a + b; }
+
+void bar() {
+  constexpr B a{};
+  constexpr B b{};
+  auto constexpr c = a + b;
+}
+
+static_assert((template_add(B{7}, B{3})).D == 10);
+static_assert((non_template_add(B{7}, B{3})).D == 10);
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3053,10 +3053,11 @@
   /// argument-dependent lookup, etc. Subclasses may override this routine to
   /// provide different behavior.
   ExprResult RebuildCXXOperatorCallExpr(OverloadedOperatorKind Op,
-  SourceLocation OpLoc,
-  Expr *Callee,
-  Expr *First,
-  Expr *Second);
+SourceLocation OpLoc,
+SourceLocation CalleeLoc,
+bool RequiresADL,
+const UnresolvedSetImpl &Functions,
+Expr *First, Expr *Second);
 
   /// Build a new C++ "named" cast expression, such as static_cast or
   /// reinterpret_cast.
@@ -11962,10 +11963,6 @@
 llvm_unreachable("not an overloaded operator?");
   }
 
-  ExprResult Callee = getDerived().TransformExpr(E->getCallee());
-  if (Callee.isInvalid())
-return ExprError();
-
   ExprResult First;
   if (E->getOperator() == OO_Amp)
 First = getDerived().TransformAddressOfOperand(E->getArg(0));
@@ -11982,23 +11979,41 @@
   return ExprError();
   }
 
-  if (!getDerived().AlwaysRebuild() &&
-  Callee.get() == E->getCallee() &&
-  First.get() == E->getArg(0) &&
-  (E->getNumArgs() != 2 || Second.get() == E->getArg(1)))
-return SemaRef.MaybeBindToTemporary(E);
-
   Sema::FPFeaturesStateRAII FPFeaturesState(getSema());
   FPOptionsOverride NewOverrides(E->getFPFeatures());
   getSema().CurFPFeatures =
   NewOverrides.applyOverrides(getSema().getLangOpts());
   getSema().FpPragmaStack.CurrentValue = NewOverrides;
 
-  return getDerived().RebuildCXXOperatorCallExpr(E->getOperator(),
- E->getOperatorLoc(),
- Callee.get(),
- First.get(),
- Second.get());
+  bool RequiresADL = false;
+  Expr *Callee = E->getCallee();
+  if (UnresolvedLookupExpr *ULE = dyn_cast(Callee)) {
+RequiresADL = ULE->requiresADL();
+LookupResult R(SemaRef, ULE->getName(), ULE->getNameLoc(),
+   Sema::LookupOrdinaryName);
+if (getDerived().TransformOverloadExprDecls(ULE, ULE->requiresADL(), R))
+  return ExprError();
+
+return getDerived().RebuildCXXOperatorCallExpr(
+E->getOperator(), E->getOperatorLoc(), Callee->getBeginLoc(),
+RequiresADL, R.asUnresolvedSet(), First.get(), Second.get());
+  }
+
+  UnresolvedSet<1> Functions;
+  if (ImplicitCastExpr *ICE = dyn_cast(Callee))
+Callee = ICE->getSubExprAsWritten();
+  NamedDecl *DR = cast(Callee)->getDecl();
+  ValueDecl *VD = c

[PATCH] D152305: [clang-format] Add the KeepEmptyLinesAtEOF option

2023-06-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision.
MyDeveloperDay added a comment.
This revision is now accepted and ready to land.

LGTM


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

https://reviews.llvm.org/D152305

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


[PATCH] D151553: [clang] Fix consteval operators in template contexts

2023-06-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments.



Comment at: clang/lib/Sema/TreeTransform.h:11988
 
-  return getDerived().RebuildCXXOperatorCallExpr(E->getOperator(),
- E->getOperatorLoc(),
- Callee.get(),
- First.get(),
- Second.get());
+  bool RequiresADL = false;
+  Expr *Callee = E->getCallee();

I don't think this is useful, it is only used L11999 but not in L11994, and in 
either cases you can use `ULE->requiresADL()`



Comment at: clang/lib/Sema/TreeTransform.h:12015
+  return getDerived().RebuildCXXOperatorCallExpr(
+  E->getOperator(), E->getOperatorLoc(), Callee->getBeginLoc(), 
RequiresADL,
+  Functions, First.get(), Second.get());

here i think RequiresADL is always false



Comment at: clang/lib/Sema/TreeTransform.h:15216-15217
 
-  if (Op == OO_Subscript) {
-SourceLocation LBrace;
-SourceLocation RBrace;
-
-if (DeclRefExpr *DRE = dyn_cast(Callee)) {
-  DeclarationNameLoc NameLoc = DRE->getNameInfo().getInfo();
-  LBrace = NameLoc.getCXXOperatorNameBeginLoc();
-  RBrace = NameLoc.getCXXOperatorNameEndLoc();
-} else {
-  LBrace = Callee->getBeginLoc();
-  RBrace = OpLoc;
-}
-
-return SemaRef.CreateOverloadedArraySubscriptExpr(LBrace, RBrace,
-  First, Second);
-  }
+  // FIXME: The following code for subscript expression is either never 
executed
+  // or not tested by check-clang.
+  if (Op == OO_Subscript)

Maybe it would be worth investigating that further before merging the PR? 
Although the pr is a clear improvement so I'll let you decide what to do!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151553

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


[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

2023-06-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment.

In D151761#4400056 , @galenelias 
wrote:

> In D151761#4394758 , 
> @MyDeveloperDay wrote:
>
>> did you consider a case where the case falls through? (i.e. no return)
>>
>>   "case log::info :return \"info\";\n"
>>   "case log::warning :\n"
>>   "default :   return \"default\";\n"
>
> That's a great point.  I didn't really consider this, and currently this 
> particular case won't align the case statements if they have an empty case 
> block, however if you had some tokens (e.g. `// fallthrough`) it would.  It's 
> not immediately clear to me what the expectation would be.  I guess to align 
> as if there was an invisible trailing token, but it's a bit awkward if the 
> cases missing a body are the 'long' cases that push out the alignment.  Also, 
> I don't think it's possible to use `AlignTokens` and get this behavior, as 
> there is no token on those lines to align, so it's not straightforward to 
> handle.  I guess I'll be curious to see if there is feedback or cases where 
> this behavior is desired, and if so, I can look into adding that 
> functionality later.  Since right now it would involve a completely custom 
> AlignTokens clone, my preference would be to just leave this as not supported.

Can you add a unit test with a // fallthough comment, and a /* FALLTHROUGH */ 
and [[fallthrough]] so we know whats going to happen in those cases at a 
minimum.


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

https://reviews.llvm.org/D151761

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


[PATCH] D152353: [NFC][Driver] Change MultilibBuilder flags argument order

2023-06-07 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings created this revision.
michaelplatings added reviewers: phosek, simon_tatham.
Herald added a subscriber: abrachet.
Herald added a project: All.
michaelplatings requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Follow up to D151437 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152353

Files:
  clang/include/clang/Driver/MultilibBuilder.h
  clang/lib/Driver/MultilibBuilder.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/unittests/Driver/MultilibBuilderTest.cpp

Index: clang/unittests/Driver/MultilibBuilderTest.cpp
===
--- clang/unittests/Driver/MultilibBuilderTest.cpp
+++ clang/unittests/Driver/MultilibBuilderTest.cpp
@@ -27,22 +27,22 @@
 
   ASSERT_TRUE(MultilibBuilder().isValid()) << "Empty multilib is not valid";
 
-  ASSERT_TRUE(MultilibBuilder().flag(true, "-foo").isValid())
+  ASSERT_TRUE(MultilibBuilder().flag("-foo").isValid())
   << "Single indicative flag is not valid";
 
-  ASSERT_TRUE(MultilibBuilder().flag(false, "-foo").isValid())
+  ASSERT_TRUE(MultilibBuilder().flag("-foo", /*Negate=*/true).isValid())
   << "Single contraindicative flag is not valid";
 
   ASSERT_FALSE(
-  MultilibBuilder().flag(true, "-foo").flag(false, "-foo").isValid())
+  MultilibBuilder().flag("-foo").flag("-foo", /*Negate=*/true).isValid())
   << "Conflicting flags should invalidate the Multilib";
 
-  ASSERT_TRUE(MultilibBuilder().flag(true, "-foo").flag(true, "-foo").isValid())
+  ASSERT_TRUE(MultilibBuilder().flag("-foo").flag("-foo").isValid())
   << "Multilib should be valid even if it has the same flag "
  "twice";
 
   ASSERT_TRUE(
-  MultilibBuilder().flag(true, "-foo").flag(false, "-foobar").isValid())
+  MultilibBuilder().flag("-foo").flag("-foobar", /*Negate=*/true).isValid())
   << "Seemingly conflicting prefixes shouldn't actually conflict";
 }
 
@@ -55,7 +55,7 @@
 
 TEST(MultilibBuilderTest, Construction3) {
   MultilibBuilder M =
-  MultilibBuilder().flag(true, "-f1").flag(true, "-f2").flag(false, "-f3");
+  MultilibBuilder().flag("-f1").flag("-f2").flag("-f3", /*Negate=*/true);
   for (const std::string &A : M.flags()) {
 ASSERT_TRUE(llvm::StringSwitch(A)
 .Cases("-f1", "-f2", "!f3", true)
@@ -66,7 +66,7 @@
 TEST(MultilibBuilderTest, SetConstruction1) {
   // Single maybe
   MultilibSet MS = MultilibSetBuilder()
-   .Maybe(MultilibBuilder("64").flag(true, "-m64"))
+   .Maybe(MultilibBuilder("64").flag("-m64"))
.makeMultilibSet();
   ASSERT_TRUE(MS.size() == 2);
   for (MultilibSet::const_iterator I = MS.begin(), E = MS.end(); I != E; ++I) {
@@ -82,8 +82,8 @@
 TEST(MultilibBuilderTest, SetConstruction2) {
   // Double maybe
   MultilibSet MS = MultilibSetBuilder()
-   .Maybe(MultilibBuilder("sof").flag(true, "-sof"))
-   .Maybe(MultilibBuilder("el").flag(true, "-EL"))
+   .Maybe(MultilibBuilder("sof").flag("-sof"))
+   .Maybe(MultilibBuilder("el").flag("-EL"))
.makeMultilibSet();
   ASSERT_TRUE(MS.size() == 4);
   for (MultilibSet::const_iterator I = MS.begin(), E = MS.end(); I != E; ++I) {
@@ -157,7 +157,7 @@
 
 TEST(MultilibBuilderTest, SetSelection1) {
   MultilibSet MS1 = MultilibSetBuilder()
-.Maybe(MultilibBuilder("64").flag(true, "-m64"))
+.Maybe(MultilibBuilder("64").flag("-m64"))
 .makeMultilibSet();
 
   Multilib::flags_list FlagM64 = {"-m64"};
@@ -177,8 +177,8 @@
 
 TEST(MultilibBuilderTest, SetSelection2) {
   MultilibSet MS2 = MultilibSetBuilder()
-.Maybe(MultilibBuilder("el").flag(true, "-EL"))
-.Maybe(MultilibBuilder("sf").flag(true, "-SF"))
+.Maybe(MultilibBuilder("el").flag("-EL"))
+.Maybe(MultilibBuilder("sf").flag("-SF"))
 .makeMultilibSet();
 
   for (unsigned I = 0; I < 4; ++I) {
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -1063,38 +1063,38 @@
   // Check for Code Sourcery toolchain multilibs
   MultilibSet CSMipsMultilibs;
   {
-auto MArchMips16 =
-MultilibBuilder("/mips16").flag(true, "-m32").flag(true, "-mips16");
+auto MArchMips16 = MultilibBuilder("/mips16").flag("-m32").flag("-mips16");
 
-auto MArchMicroMips = MultilibBuilder("/micromips")
-  .flag(true, "-m32")
-  .flag(true, "-mmicromips");
+auto MArchMicroMips =
+MultilibBuilder("

[PATCH] D148700: [clang] Add support for “regular” keyword attributes

2023-06-07 Thread Richard Sandiford via Phabricator via cfe-commits
rsandifo-arm added a comment.

In D148700#4401451 , @rsmith wrote:

> In D148700#4401353 , @jyknight 
> wrote:
>
>> Yes, standard attributes aren't supposed to be used for things which affect 
>> the type system (although, we certainly have many, already, which do, since 
>> we expose most GCC-syntax attributes also as C++-standard attribute syntax!)
>
> The rule that standard attributes don't affect semantics only applies to 
> attributes specified by the language standard. There is no expectation that 
> vendor attributes avoid such effects.

Ah.  Does that mean that, when the standard says (sorry for the direct latex 
quote):

> For an \grammarterm{attribute-token} (including an 
> \grammarterm{attribute-scoped-token}) not specified in this document, the 
> behavior is \impldef{behavior of non-standard attributes}; any such 
> \grammarterm{attribute-token} that is not recognized by the implementation is 
> ignored.

the “is ignored” rule only applies to unscoped attributes and to attributes in 
one of the std:: namespaces?  I.e. to things that could reasonably be 
standardised in future, rather than to vendor attributes?

> In particular, I'm concerned by this in the description of this change:
>
> In D148700 , @rsandifo-arm wrote:
>
>> Really, the “only” thing wrong with using standard attributes is that 
>> standard attributes cannot affect semantics.
>
> If the only reason for this patch series is an idea that vendor attributes 
> using `[[...]]` syntax can't affect program semantics, then I think this 
> change is not justified, because vendor attributes using `[[...]]` syntax can 
> and usually do affect program semantics. But the documentation change here 
> makes the point that using a keyword attribute may be as good idea in cases 
> where you would always want compilation to fail on a compiler that doesn't 
> understand the annotation, rather than the annotation being ignored (likely 
> with a warning), so maybe that's reasonable justification for this direction.

FWIW, the original plan had been to use standard attribute syntax with the 
`arm` vendor namespace.  We started out with things like `[[arm::streaming]]`, 
but then a colleague pointed out that standard attributes aren't supposed to 
affect semantics.  So then we switched to GNU attributes, which have long 
included things that can't be ignored (such as `vector_size`).  But there was 
pushback against that because even unrecognised GNU attributes only generate a 
warning.  (I think GCC made a mistake by establishing that unrecognised GNU 
attributes are only a warning.)

If using standard attributes with a vendor namespace is acceptable for things 
that affect semantics and ABI, then that would actually be more convenient, for 
a few reasons:

- It would provide proper scoping (via namespaces), rather than the clunky 
`__arm_` prefixes.  E.g. some of the ACLE intrinsics have `__arm_streaming 
__arm_shared_za __arm_preserves_za`, which is quite a mouthful :)

- It would allow other attribute-related features to be used, such as `#pragma 
clang attribute`

- It would allow attributes to take arguments, without any compatibility 
concerns.  Keyword attributes could support arguments, but the decision about 
whether an attribute takes arguments would probably have to be made when the 
attribute is first added.  Trying to retrofit arguments to a keyword attribute 
that didn't previously take arguments could lead to parsing ambiguities.  In 
contrast, the standard attribute syntax would allow optional arguments to be 
added later without any backward compatibility concerns.

The main drawback would still be that unrecognised attributes only generate a 
warning.  But if vendor attributes are allowed to affect semantics, we could 
“solve” the warning problem for current and future compilers by reporting an 
error for unrecognised `arm::` attributes.  That wouldn't help with older 
compilers, but TBH, I think older compilers would reject any practical use of 
SME attributes anyway.  E.g. the attributes would almost always (perhaps 
always) be used in conjunction with the `arm_sme.h` header, which older 
compilers don't provide.  We also provide a feature preprocessor macro that 
careful code can check.

So if using an `arm` namespace is acceptable, if the current line about what 
can affect semantics is likely to become more fuzzy, and if there's a risk that 
keyword attributes are a dead end that no-one else adopts, then TBH I'd still 
be in favour of using `arm` namespaces for SME.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148700

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


[PATCH] D151437: [NFC][Driver] Change MultilibBuilder interface

2023-06-07 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings marked an inline comment as done.
michaelplatings added inline comments.



Comment at: clang/include/clang/Driver/MultilibBuilder.h:80
+  /// \p Flag must be a flag accepted by the driver.
+  MultilibBuilder &flag(bool Required, StringRef Flag);
 

phosek wrote:
> I think it would be cleaner to swap the order of arguments and give the 
> boolean argument a default value. When setting the optional argument, ideally 
> we would always use put the argument in the comment which is a standard 
> convention in LLVM.
> 
> I also think that `Required` is not the best name because it might suggest 
> that `Required=false` means that the argument is optional. A better name 
> might be something like `Negate` or `Disallow` which would also match the `!` 
> notation.
> 
> Here's a concrete example of what I have in mind:
> 
> ```
>   Multilibs.push_back(MultilibBuilder("asan+noexcept", {}, {})
>   .flag("-fsanitize=address")
>   .flag("-fexceptions", /*Negate=*/false)
>   .flag("-fno-exceptions")
>   .makeMultilib());
> ```
OK, I've created D152353

`/*Negate=*/false` would mean the flag is not negated so I've changed that to 
`/*Negate=*/true`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151437

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


[PATCH] D152269: [StaticAnalyzer] Fix false negative on NilArgChecker when creating literal object

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

I had some improvement opportunities in mind scattered, so I decided to do 
them, and here is how the diff looks for me now: F27853795: 
recommendation.patch 

Basically, I reworked the two branches a bit to express the intent more strait 
forward.
I also enable all `osx.cocoa` checkers, as this file is all about objc stuff 
anyway - this also meant to diagnose two non-fatal leak errors, which are not 
tied to this patch otherwise.

I'm also testing that the "assumption" after the objc call thingie (message 
passing?) the pointer is assumed to be "nonnull". For this, I'm using the 
`eagerly-assume=false` to ensure we don't split null and non-null states 
eagerly when we encounter the `p == nil` inside `clang_analyzer_eval()` call 
argument.

I think all my changes are aligned with your intent, right?

One more thing I was thinking of was to make this ObjC null checking checker 
depend on the "nullability" checker package, but it turns out we can only 
depend on individual checkers as of now. I tried some ideas there, e.g. 
depending on the base checker of that package but it didn't work, so I dropped 
the idea. (`clang/include/clang/StaticAnalyzer/Checkers/Checkers.td`)

Do you plan to contribute more ObjC-related fixes in the future?
Please note that I have absolutely no experience with ObjC stuff.

And I also wanted to thank you for the high-quality patches you submitted so 
far!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152269

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


[PATCH] D152285: Add support for the NO_COLOR environment variable

2023-06-07 Thread Jan Niklas Hasse via Phabricator via cfe-commits
jhasse added a comment.

There's a valid usecase `CLICOLOR_FORCE`: Force color diagnostics. The "disable 
colors" part of https://bixense.com/clicolors/ is not that important to me, I 
could change it to point to `NO_COLOR` instead?

btw: I've tried to join the "standards" a few years ago: 
https://github.com/jcs/no_color/issues/28

Right now I think it would be best to drop `CLICOLOR` and have the following 
simple rules:

- if `NO_COLOR` set: disable colors
- if `CLICOLOR_FORCE` set: always set colors
- else: isatty

What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152285

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


[PATCH] D151553: [clang] Fix consteval operators in template contexts

2023-06-07 Thread Mariya Podchishchaeva via Phabricator via cfe-commits
Fznamznon added inline comments.



Comment at: clang/lib/Sema/TreeTransform.h:15216-15217
 
-  if (Op == OO_Subscript) {
-SourceLocation LBrace;
-SourceLocation RBrace;
-
-if (DeclRefExpr *DRE = dyn_cast(Callee)) {
-  DeclarationNameLoc NameLoc = DRE->getNameInfo().getInfo();
-  LBrace = NameLoc.getCXXOperatorNameBeginLoc();
-  RBrace = NameLoc.getCXXOperatorNameEndLoc();
-} else {
-  LBrace = Callee->getBeginLoc();
-  RBrace = OpLoc;
-}
-
-return SemaRef.CreateOverloadedArraySubscriptExpr(LBrace, RBrace,
-  First, Second);
-  }
+  // FIXME: The following code for subscript expression is either never 
executed
+  // or not tested by check-clang.
+  if (Op == OO_Subscript)

cor3ntin wrote:
> Maybe it would be worth investigating that further before merging the PR? 
> Although the pr is a clear improvement so I'll let you decide what to do!
I've noticed that it is likely a dead code, so I didn't want to pass more 
parameters to `RebuildCXXOperatorCallExpr` in order to support dead code, but 
didn't feel confident enough to remove it. So I left this FIXME.
`RebuildCXXOperatorCallExpr` is only called by `TransformCXXOperatorCallExpr` 
and `TransformCXXFoldExpr`. When subscript expression comes to 
`TransformCXXOperatorCallExpr`, it never falls down to 
`RebuildCXXOperatorCallExpr` after c1512250960bd247519a9f959ad4af202402dcc4 , 
and I don't think that it is possible to have subscript expression as a part of 
a fold expression. So, for me, it seems the whole `if` and FIXME actually can 
be removed. WDYT? 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151553

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


[PATCH] D152351: [clang] Add __builtin_isfpclass

2023-06-07 Thread Qiu Chaofan via Phabricator via cfe-commits
qiucf added a comment.

It's necessary to check range of first argument in `SemaChecking.cpp` using 
`SemaBuiltinConstantArgRange`.

  _Bool check_isfpclass_1(float x) { return __builtin_isfpclass(123456, x); } 
// ICE
  
  int g;
  // error: cannot compile this builtin function yet
  // there's better diagnostics when 1st argument is not constant
  _Bool check_isfpclass_2(float x) { return __builtin_isfpclass(g, x); }




Comment at: clang/include/clang/Basic/Builtins.def:491
 BUILTIN(__builtin_isnormal,   "i.", "FnctE")
+BUILTIN(__builtin_isfpclass,  "iCi.", "nctE")
 

Why these intrinsics' type spec end with dot? I thought they are for vaargs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152351

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


[PATCH] D151553: [clang] Fix consteval operators in template contexts

2023-06-07 Thread Mariya Podchishchaeva via Phabricator via cfe-commits
Fznamznon updated this revision to Diff 529229.
Fznamznon added a comment.

Remove RequiresADL


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151553

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/consteval-operators.cpp
  clang/test/SemaCXX/overloaded-operator.cpp

Index: clang/test/SemaCXX/overloaded-operator.cpp
===
--- clang/test/SemaCXX/overloaded-operator.cpp
+++ clang/test/SemaCXX/overloaded-operator.cpp
@@ -585,3 +585,16 @@
   float &operator->*(B, B);
   template void f();
 }
+
+namespace test {
+namespace A {
+template T f(T t) {
+  T operator+(T, T);
+  return t + t;
+}
+}
+namespace B {
+  struct X {};
+}
+void g(B::X x) { A::f(x); }
+}
Index: clang/test/SemaCXX/consteval-operators.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/consteval-operators.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -std=c++2a -emit-llvm-only -Wno-unused-value %s -verify
+
+// expected-no-diagnostics
+
+struct A {
+  consteval A operator+() { return {}; }
+};
+consteval A operator~(A) { return {}; }
+consteval A operator+(A, A) { return {}; }
+
+template  void f() {
+  A a;
+  A b = ~a;
+  A c = a + a;
+  A d = +a;
+}
+template void f();
+
+template  void foo() {
+  T a;
+  T b = ~a;
+  T c = a + a;
+  T d = +a;
+}
+
+template void foo();
+
+template  struct B { DataT D; };
+
+template 
+consteval B operator+(B lhs, B rhs) {
+  return B{lhs.D + rhs.D};
+}
+
+template  consteval T template_add(T a, T b) { return a + b; }
+
+consteval B non_template_add(B a, B b) { return a + b; }
+
+void bar() {
+  constexpr B a{};
+  constexpr B b{};
+  auto constexpr c = a + b;
+}
+
+static_assert((template_add(B{7}, B{3})).D == 10);
+static_assert((non_template_add(B{7}, B{3})).D == 10);
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3053,10 +3053,11 @@
   /// argument-dependent lookup, etc. Subclasses may override this routine to
   /// provide different behavior.
   ExprResult RebuildCXXOperatorCallExpr(OverloadedOperatorKind Op,
-  SourceLocation OpLoc,
-  Expr *Callee,
-  Expr *First,
-  Expr *Second);
+SourceLocation OpLoc,
+SourceLocation CalleeLoc,
+bool RequiresADL,
+const UnresolvedSetImpl &Functions,
+Expr *First, Expr *Second);
 
   /// Build a new C++ "named" cast expression, such as static_cast or
   /// reinterpret_cast.
@@ -11962,10 +11963,6 @@
 llvm_unreachable("not an overloaded operator?");
   }
 
-  ExprResult Callee = getDerived().TransformExpr(E->getCallee());
-  if (Callee.isInvalid())
-return ExprError();
-
   ExprResult First;
   if (E->getOperator() == OO_Amp)
 First = getDerived().TransformAddressOfOperand(E->getArg(0));
@@ -11982,23 +11979,39 @@
   return ExprError();
   }
 
-  if (!getDerived().AlwaysRebuild() &&
-  Callee.get() == E->getCallee() &&
-  First.get() == E->getArg(0) &&
-  (E->getNumArgs() != 2 || Second.get() == E->getArg(1)))
-return SemaRef.MaybeBindToTemporary(E);
-
   Sema::FPFeaturesStateRAII FPFeaturesState(getSema());
   FPOptionsOverride NewOverrides(E->getFPFeatures());
   getSema().CurFPFeatures =
   NewOverrides.applyOverrides(getSema().getLangOpts());
   getSema().FpPragmaStack.CurrentValue = NewOverrides;
 
-  return getDerived().RebuildCXXOperatorCallExpr(E->getOperator(),
- E->getOperatorLoc(),
- Callee.get(),
- First.get(),
- Second.get());
+  Expr *Callee = E->getCallee();
+  if (UnresolvedLookupExpr *ULE = dyn_cast(Callee)) {
+LookupResult R(SemaRef, ULE->getName(), ULE->getNameLoc(),
+   Sema::LookupOrdinaryName);
+if (getDerived().TransformOverloadExprDecls(ULE, ULE->requiresADL(), R))
+  return ExprError();
+
+return getDerived().RebuildCXXOperatorCallExpr(
+E->getOperator(), E->getOperatorLoc(), Callee->getBeginLoc(),
+ULE->requiresADL(), R.asUnresolvedSet(), First.get(), Second.get());
+  }
+
+  UnresolvedSet<1> Functions;
+  if (ImplicitCastExpr *ICE = dyn_cast(Callee))
+Callee = ICE->getSubExprAsWritten();
+  NamedDecl *DR = cast(Callee)->getDecl();
+  ValueDecl *VD = cast_or_null(
+  getDerived().TransformDecl(DR->getLocation(), DR));
+  if 

cfe-commits@lists.llvm.org

2023-06-07 Thread Jens Massberg via Phabricator via cfe-commits
massberg updated this revision to Diff 529231.
massberg added a comment.

Format code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148924

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
  clang/test/CodeGenCXX/virtual-compare.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -532,7 +532,7 @@
   

 https://wg21.link/p2002r1";>P2002R1
-Partial
+Clang 17
   
   
 https://wg21.link/p2085r0";>P2085R0
Index: clang/test/CodeGenCXX/virtual-compare.cpp
===
--- clang/test/CodeGenCXX/virtual-compare.cpp
+++ clang/test/CodeGenCXX/virtual-compare.cpp
@@ -21,8 +21,6 @@
   virtual std::strong_ordering operator<=>(const A &) const & = default;
   // CHECK-SAME: @_ZN1A1gEv
   virtual void g();
-  // CHECK-SAME: @_ZNKO1AssERKS_
-  virtual std::strong_ordering operator<=>(const A &) const && = default;
   // CHECK-SAME: @_ZN1A1hEv
   virtual void h();
 
@@ -35,9 +33,6 @@
 
   // CHECK-SAME: @_ZNKR1AeqERKS_
   // implicit virtual A &operator==(const A&) const & = default;
-
-  // CHECK-SAME: @_ZNKO1AeqERKS_
-  // implicit virtual A &operator==(const A&) const && = default;
 };
 
 // For Y:
Index: clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
===
--- clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
+++ clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
@@ -15,7 +15,8 @@
 
   bool operator<(const A&) const;
   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 a defaulted comparison operator}}
+  bool operator>=(const A&) const volatile = default; // expected-error {{defaulted comparison function must not be volatile}}
   bool operator<=>(const A&) = default; // expected-error {{defaulted member three-way comparison operator must be const-qualified}}
   bool operator>=(const B&) const = default; // expected-error-re {{invalid parameter type for defaulted relational comparison operator; found 'const B &', expected 'const A &'{{$
   static bool operator>(const B&) = default; // expected-error {{overloaded 'operator>' cannot be a static member function}}
@@ -195,6 +196,11 @@
 friend bool operator==(const S6&, const S6&) = default; // expected-error {{because it was already declared outside}}
 };
 
+struct S7 {
+  bool operator==(S7 const &) const &&;
+};
+bool S7::operator==(S7 const &) const && = default; // expected-error {{ref-qualifier '&&' is not allowed on a defaulted comparison operator}}
+
 enum e {};
 bool operator==(e, int) = default; // expected-error{{expected class or reference to a constant class}}
 
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -8582,8 +8582,8 @@
   // C++2a [class.compare.default]p1:
   //   A defaulted comparison operator function for some class C shall be a
   //   non-template function declared in the member-specification of C that is
-  //-- a non-static const member of C having one parameter of type
-  //   const C&, or
+  //-- a non-static const non-volatile member of C having one parameter of
+  //   type const C& and either no ref-qualifier or the ref-qualifier &, or
   //-- a friend of C having two parameters of type const C& or two
   //   parameters of type C.
 
@@ -8593,6 +8593,17 @@
 auto *MD = cast(FD);
 assert(!MD->isStatic() && "comparison function cannot be a static member");
 
+if (MD->getRefQualifier() == RQ_RValue) {
+  Diag(MD->getLocation(), diag::err_ref_qualifier_comparison_operator);
+
+  // Remove the ref qualifier to recover.
+  const auto *FPT = MD->getType()->castAs();
+  FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+  EPI.RefQualifier = RQ_None;
+  MD->setType(Context.getFunctionType(FPT->getReturnType(),
+  FPT->getParamTypes(), EPI));
+}
+
 // If we're out-of-class, this is the class we're comparing.
 if (!RD)
   RD = MD->getParent();
@@ -8615,6 +8626,17 @@
   MD->setType(Context.getFunctionType(FPT->getReturnType(),
   FPT->getParamTypes(), EPI));
 }
+
+if (MD->isVolatile()) {
+  Diag(MD->getLocation(), diag::err_volatile_comparison_operator);
+
+  // Remove the 'volatile' f

[clang-tools-extra] e72baa7 - [clangd] Add semantic token for labels

2023-06-07 Thread Christian Kandeler via cfe-commits

Author: Christian Kandeler
Date: 2023-06-07T12:28:06+02:00
New Revision: e72baa76b91fbcb2b16747cb7d2088723478a754

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

LOG: [clangd] Add semantic token for labels

Reviewed By: kadircet

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

Added: 


Modified: 
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/SemanticHighlighting.h
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp 
b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index ec37476cf94ea..6a835f31f064b 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -166,6 +166,8 @@ std::optional kindForDecl(const NamedDecl 
*D,
 return HighlightingKind::TemplateParameter;
   if (isa(D))
 return HighlightingKind::Concept;
+  if (isa(D))
+return HighlightingKind::Label;
   if (const auto *UUVD = dyn_cast(D)) {
 auto Targets = Resolver->resolveUsingValueDecl(UUVD);
 if (!Targets.empty() && Targets[0] != UUVD) {
@@ -1271,6 +1273,8 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, 
HighlightingKind K) {
 return OS << "Operator";
   case HighlightingKind::Bracket:
 return OS << "Bracket";
+  case HighlightingKind::Label:
+return OS << "Label";
   case HighlightingKind::InactiveCode:
 return OS << "InactiveCode";
   }
@@ -1470,6 +1474,8 @@ llvm::StringRef toSemanticTokenType(HighlightingKind 
Kind) {
 return "operator";
   case HighlightingKind::Bracket:
 return "bracket";
+  case HighlightingKind::Label:
+return "label";
   case HighlightingKind::InactiveCode:
 return "comment";
   }

diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.h 
b/clang-tools-extra/clangd/SemanticHighlighting.h
index c9db598ff08c9..59d742b83ee52 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.h
+++ b/clang-tools-extra/clangd/SemanticHighlighting.h
@@ -52,6 +52,7 @@ enum class HighlightingKind {
   Modifier,
   Operator,
   Bracket,
+  Label,
 
   // This one is 
diff erent from the other kinds as it's a line style
   // rather than a token style.

diff  --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp 
b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
index 9c6e5246f5c37..ff052e6be9549 100644
--- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -1028,6 +1028,16 @@ sizeof...($TemplateParameter[[Elements]]);
 template $Bracket[[<]]$Concept[[C2]]$Bracket[[<]]int$Bracket[[>]] 
$TemplateParameter_def[[A]]$Bracket[[>]]
 class $Class_def[[B]] {};
   )cpp",
+  // Labels
+  R"cpp(
+bool $Function_def[[funcWithGoto]](bool $Parameter_def[[b]]) {
+  if ($Parameter[[b]])
+goto $Label[[return_true]];
+  return false;
+  $Label_decl[[return_true]]:
+return true;
+}
+  )cpp",
   // no crash
   R"cpp(
 struct $Class_def[[Foo]] {



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


[PATCH] D143260: [clangd] Add semantic token for labels

2023-06-07 Thread Christian Kandeler via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe72baa76b91f: [clangd] Add semantic token for labels 
(authored by ckandeler).

Changed prior to commit:
  https://reviews.llvm.org/D143260?vs=494583&id=529234#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143260

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -1028,6 +1028,16 @@
 template $Bracket[[<]]$Concept[[C2]]$Bracket[[<]]int$Bracket[[>]] 
$TemplateParameter_def[[A]]$Bracket[[>]]
 class $Class_def[[B]] {};
   )cpp",
+  // Labels
+  R"cpp(
+bool $Function_def[[funcWithGoto]](bool $Parameter_def[[b]]) {
+  if ($Parameter[[b]])
+goto $Label[[return_true]];
+  return false;
+  $Label_decl[[return_true]]:
+return true;
+}
+  )cpp",
   // no crash
   R"cpp(
 struct $Class_def[[Foo]] {
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -52,6 +52,7 @@
   Modifier,
   Operator,
   Bracket,
+  Label,
 
   // This one is different from the other kinds as it's a line style
   // rather than a token style.
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -166,6 +166,8 @@
 return HighlightingKind::TemplateParameter;
   if (isa(D))
 return HighlightingKind::Concept;
+  if (isa(D))
+return HighlightingKind::Label;
   if (const auto *UUVD = dyn_cast(D)) {
 auto Targets = Resolver->resolveUsingValueDecl(UUVD);
 if (!Targets.empty() && Targets[0] != UUVD) {
@@ -1271,6 +1273,8 @@
 return OS << "Operator";
   case HighlightingKind::Bracket:
 return OS << "Bracket";
+  case HighlightingKind::Label:
+return OS << "Label";
   case HighlightingKind::InactiveCode:
 return OS << "InactiveCode";
   }
@@ -1470,6 +1474,8 @@
 return "operator";
   case HighlightingKind::Bracket:
 return "bracket";
+  case HighlightingKind::Label:
+return "label";
   case HighlightingKind::InactiveCode:
 return "comment";
   }


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -1028,6 +1028,16 @@
 template $Bracket[[<]]$Concept[[C2]]$Bracket[[<]]int$Bracket[[>]] $TemplateParameter_def[[A]]$Bracket[[>]]
 class $Class_def[[B]] {};
   )cpp",
+  // Labels
+  R"cpp(
+bool $Function_def[[funcWithGoto]](bool $Parameter_def[[b]]) {
+  if ($Parameter[[b]])
+goto $Label[[return_true]];
+  return false;
+  $Label_decl[[return_true]]:
+return true;
+}
+  )cpp",
   // no crash
   R"cpp(
 struct $Class_def[[Foo]] {
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -52,6 +52,7 @@
   Modifier,
   Operator,
   Bracket,
+  Label,
 
   // This one is different from the other kinds as it's a line style
   // rather than a token style.
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -166,6 +166,8 @@
 return HighlightingKind::TemplateParameter;
   if (isa(D))
 return HighlightingKind::Concept;
+  if (isa(D))
+return HighlightingKind::Label;
   if (const auto *UUVD = dyn_cast(D)) {
 auto Targets = Resolver->resolveUsingValueDecl(UUVD);
 if (!Targets.empty() && Targets[0] != UUVD) {
@@ -1271,6 +1273,8 @@
 return OS << "Operator";
   case HighlightingKind::Bracket:
 return OS << "Bracket";
+  case HighlightingKind::Label:
+return OS << "Label";
   case HighlightingKind::InactiveCode:
 return OS << "InactiveCode";
   }
@@ -1470,6 +1474,8 @@
 return "operator";
   case HighlightingKind::Bracket:
 return "bracket";
+  case Highli

cfe-commits@lists.llvm.org

2023-06-07 Thread Jens Massberg via cfe-commits

Author: Jens Massberg
Date: 2023-06-07T12:56:35+02:00
New Revision: 593a2740f7a499e35f19e64d180d0b8246b52ba3

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

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

This patch implemed the change proposed in [P2002R1] to 11.11.1 
[class.compare.default] paragraph 1.

A defaulted compariosn operator function must be non-volatile and must either 
have no ref-qualifier or the ref-qualifier &.

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
clang/test/CodeGenCXX/virtual-compare.cpp
clang/www/cxx_status.html

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 7ec9b3911ad5d..733a003f5321b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -110,6 +110,7 @@ C++20 Feature Support
   unevaluated contexts will be surfaced as errors. They were previously 
handled as
   SFINAE.
 - Clang now supports `requires cplusplus20` for module maps.
+- Implemented missing parts of `P2002R1: Consistent comparison operators 
`_
 
 C++23 Feature Support
 ^

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 5895888c175fa..e03e27d196731 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9462,6 +9462,10 @@ def note_defaulted_comparison_not_constexpr_here : Note<
 def note_in_declaration_of_implicit_equality_comparison : Note<
   "while declaring the corresponding implicit 'operator==' "
   "for this defaulted 'operator<=>'">;
+def err_volatile_comparison_operator : Error<
+  "defaulted comparison function must not be volatile">;
+def err_ref_qualifier_comparison_operator : Error<
+  "ref-qualifier '&&' is not allowed on a defaulted comparison operator">;
 
 def ext_implicit_exception_spec_mismatch : ExtWarn<
   "function previously declared with an %select{explicit|implicit}0 exception "

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 3169b381071bb..9b3bcc296ddb7 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -8582,8 +8582,8 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, 
FunctionDecl *FD,
   // C++2a [class.compare.default]p1:
   //   A defaulted comparison operator function for some class C shall be a
   //   non-template function declared in the member-specification of C that is
-  //-- a non-static const member of C having one parameter of type
-  //   const C&, or
+  //-- a non-static const non-volatile member of C having one parameter of
+  //   type const C& and either no ref-qualifier or the ref-qualifier &, or
   //-- a friend of C having two parameters of type const C& or two
   //   parameters of type C.
 
@@ -8593,6 +8593,17 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, 
FunctionDecl *FD,
 auto *MD = cast(FD);
 assert(!MD->isStatic() && "comparison function cannot be a static member");
 
+if (MD->getRefQualifier() == RQ_RValue) {
+  Diag(MD->getLocation(), diag::err_ref_qualifier_comparison_operator);
+
+  // Remove the ref qualifier to recover.
+  const auto *FPT = MD->getType()->castAs();
+  FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+  EPI.RefQualifier = RQ_None;
+  MD->setType(Context.getFunctionType(FPT->getReturnType(),
+  FPT->getParamTypes(), EPI));
+}
+
 // If we're out-of-class, this is the class we're comparing.
 if (!RD)
   RD = MD->getParent();
@@ -8615,6 +8626,17 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, 
FunctionDecl *FD,
   MD->setType(Context.getFunctionType(FPT->getReturnType(),
   FPT->getParamTypes(), EPI));
 }
+
+if (MD->isVolatile()) {
+  Diag(MD->getLocation(), diag::err_volatile_comparison_operator);
+
+  // Remove the 'volatile' from the type to recover.
+  const auto *FPT = MD->getType()->castAs();
+  FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+  EPI.TypeQuals.removeVolatile();
+  MD->setType(Context.getFunctionType(FPT->getReturnType(),
+  FPT->getParamTypes(), EPI));
+}
   }
 
   if (FD->getNumParams() != (IsMethod ? 1 : 2)) {

diff  --git a/clang/test/CXX/class/class.compare/class.compare.default/p1.cpp 
b/clang/test/

cfe-commits@lists.llvm.org

2023-06-07 Thread Jens Massberg via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG593a2740f7a4: [clang] Show error if defaulted comparions 
operator function is volatile or has… (authored by massberg).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148924

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
  clang/test/CodeGenCXX/virtual-compare.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -532,7 +532,7 @@
   

 https://wg21.link/p2002r1";>P2002R1
-Partial
+Clang 17
   
   
 https://wg21.link/p2085r0";>P2085R0
Index: clang/test/CodeGenCXX/virtual-compare.cpp
===
--- clang/test/CodeGenCXX/virtual-compare.cpp
+++ clang/test/CodeGenCXX/virtual-compare.cpp
@@ -21,8 +21,6 @@
   virtual std::strong_ordering operator<=>(const A &) const & = default;
   // CHECK-SAME: @_ZN1A1gEv
   virtual void g();
-  // CHECK-SAME: @_ZNKO1AssERKS_
-  virtual std::strong_ordering operator<=>(const A &) const && = default;
   // CHECK-SAME: @_ZN1A1hEv
   virtual void h();
 
@@ -35,9 +33,6 @@
 
   // CHECK-SAME: @_ZNKR1AeqERKS_
   // implicit virtual A &operator==(const A&) const & = default;
-
-  // CHECK-SAME: @_ZNKO1AeqERKS_
-  // implicit virtual A &operator==(const A&) const && = default;
 };
 
 // For Y:
Index: clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
===
--- clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
+++ clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
@@ -15,7 +15,8 @@
 
   bool operator<(const A&) const;
   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 a defaulted comparison operator}}
+  bool operator>=(const A&) const volatile = default; // expected-error {{defaulted comparison function must not be volatile}}
   bool operator<=>(const A&) = default; // expected-error {{defaulted member three-way comparison operator must be const-qualified}}
   bool operator>=(const B&) const = default; // expected-error-re {{invalid parameter type for defaulted relational comparison operator; found 'const B &', expected 'const A &'{{$
   static bool operator>(const B&) = default; // expected-error {{overloaded 'operator>' cannot be a static member function}}
@@ -195,6 +196,11 @@
 friend bool operator==(const S6&, const S6&) = default; // expected-error {{because it was already declared outside}}
 };
 
+struct S7 {
+  bool operator==(S7 const &) const &&;
+};
+bool S7::operator==(S7 const &) const && = default; // expected-error {{ref-qualifier '&&' is not allowed on a defaulted comparison operator}}
+
 enum e {};
 bool operator==(e, int) = default; // expected-error{{expected class or reference to a constant class}}
 
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -8582,8 +8582,8 @@
   // C++2a [class.compare.default]p1:
   //   A defaulted comparison operator function for some class C shall be a
   //   non-template function declared in the member-specification of C that is
-  //-- a non-static const member of C having one parameter of type
-  //   const C&, or
+  //-- a non-static const non-volatile member of C having one parameter of
+  //   type const C& and either no ref-qualifier or the ref-qualifier &, or
   //-- a friend of C having two parameters of type const C& or two
   //   parameters of type C.
 
@@ -8593,6 +8593,17 @@
 auto *MD = cast(FD);
 assert(!MD->isStatic() && "comparison function cannot be a static member");
 
+if (MD->getRefQualifier() == RQ_RValue) {
+  Diag(MD->getLocation(), diag::err_ref_qualifier_comparison_operator);
+
+  // Remove the ref qualifier to recover.
+  const auto *FPT = MD->getType()->castAs();
+  FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+  EPI.RefQualifier = RQ_None;
+  MD->setType(Context.getFunctionType(FPT->getReturnType(),
+  FPT->getParamTypes(), EPI));
+}
+
 // If we're out-of-class, this is the class we're comparing.
 if (!RD)
   RD = MD->getParent();
@@ -8615,6 +8626,17 @@
   MD->setType(Context.getFunctionType(FPT->getReturnType(),
   FPT->getParamTypes(), EPI));
 }
+
+if (MD

[PATCH] D151553: [clang] Fix consteval operators in template contexts

2023-06-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments.



Comment at: clang/lib/Sema/TreeTransform.h:15216-15217
 
-  if (Op == OO_Subscript) {
-SourceLocation LBrace;
-SourceLocation RBrace;
-
-if (DeclRefExpr *DRE = dyn_cast(Callee)) {
-  DeclarationNameLoc NameLoc = DRE->getNameInfo().getInfo();
-  LBrace = NameLoc.getCXXOperatorNameBeginLoc();
-  RBrace = NameLoc.getCXXOperatorNameEndLoc();
-} else {
-  LBrace = Callee->getBeginLoc();
-  RBrace = OpLoc;
-}
-
-return SemaRef.CreateOverloadedArraySubscriptExpr(LBrace, RBrace,
-  First, Second);
-  }
+  // FIXME: The following code for subscript expression is either never 
executed
+  // or not tested by check-clang.
+  if (Op == OO_Subscript)

Fznamznon wrote:
> cor3ntin wrote:
> > Maybe it would be worth investigating that further before merging the PR? 
> > Although the pr is a clear improvement so I'll let you decide what to do!
> I've noticed that it is likely a dead code, so I didn't want to pass more 
> parameters to `RebuildCXXOperatorCallExpr` in order to support dead code, but 
> didn't feel confident enough to remove it. So I left this FIXME.
> `RebuildCXXOperatorCallExpr` is only called by `TransformCXXOperatorCallExpr` 
> and `TransformCXXFoldExpr`. When subscript expression comes to 
> `TransformCXXOperatorCallExpr`, it never falls down to 
> `RebuildCXXOperatorCallExpr` after c1512250960bd247519a9f959ad4af202402dcc4 , 
> and I don't think that it is possible to have subscript expression as a part 
> of a fold expression. So, for me, it seems the whole `if` and FIXME actually 
> can be removed. WDYT? 
> 
I think you are right, It's seems like something I should have done as part of 
rGc1512250960bd247519a9f959ad4af202402dcc4.

It think you should remove it. (if we break something we can put it back with 
tests!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151553

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


[PATCH] D152351: [clang] Add __builtin_isfpclass

2023-06-07 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff updated this revision to Diff 529243.
sepavloff added a comment.

Add check for test bit mask value


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152351

Files:
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins.c
  clang/test/CodeGen/isfpclass.c
  clang/test/Sema/builtins.c
  llvm/include/llvm/IR/IRBuilder.h
  llvm/lib/IR/IRBuilder.cpp

Index: llvm/lib/IR/IRBuilder.cpp
===
--- llvm/lib/IR/IRBuilder.cpp
+++ llvm/lib/IR/IRBuilder.cpp
@@ -1375,6 +1375,14 @@
   return Fn;
 }
 
+Value *IRBuilderBase::createIsFPClass(Value *FPNum, unsigned Test) {
+  auto TestV = llvm::ConstantInt::get(Type::getInt32Ty(Context), Test);
+  Module *M = BB->getParent()->getParent();
+  Function *FnIsFPClass =
+  Intrinsic::getDeclaration(M, Intrinsic::is_fpclass, {FPNum->getType()});
+  return CreateCall(FnIsFPClass, {FPNum, TestV});
+}
+
 CallInst *IRBuilderBase::CreateAlignmentAssumptionHelper(const DataLayout &DL,
  Value *PtrValue,
  Value *AlignValue,
Index: llvm/include/llvm/IR/IRBuilder.h
===
--- llvm/include/llvm/IR/IRBuilder.h
+++ llvm/include/llvm/IR/IRBuilder.h
@@ -2518,6 +2518,8 @@
  unsigned Index, unsigned FieldIndex,
  MDNode *DbgInfo);
 
+  Value *createIsFPClass(Value *FPNum, unsigned Test);
+
 private:
   /// Helper function that creates an assume intrinsic call that
   /// represents an alignment assumption on the provided pointer \p PtrValue
Index: clang/test/Sema/builtins.c
===
--- clang/test/Sema/builtins.c
+++ clang/test/Sema/builtins.c
@@ -378,3 +378,7 @@
 }
 
 _Complex double builtin_complex_static_init = __builtin_complex(1.0, 2.0);
+
+int test_is_fpclass(float x) {
+  return __builtin_isfpclass(1024, x); // expected-error {{argument value 1024 is outside the valid range [0, 1023]}}
+}
Index: clang/test/CodeGen/isfpclass.c
===
--- /dev/null
+++ clang/test/CodeGen/isfpclass.c
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -O1 -emit-llvm %s -o - | FileCheck %s
+
+_Bool check_isfpclass_finite(float x) {
+  return __builtin_isfpclass(504 /*Finite*/, x);
+}
+// CHECK-LABEL: define {{.*}} i1 @check_isfpclass_finite(
+// CHECK: call i1 @llvm.is.fpclass.f32(float {{.*}}, i32 504)
+
+_Bool check_isfpclass_finite_strict(float x) {
+#pragma STDC FENV_ACCESS ON
+  return __builtin_isfpclass(504 /*Finite*/, x);
+}
+// CHECK-LABEL: define {{.*}} i1 @check_isfpclass_finite_strict(
+// CHECK: call i1 @llvm.is.fpclass.f32(float {{.*}}, i32 504)
+
+_Bool check_isfpclass_nan_f32(float x) {
+#pragma STDC FENV_ACCESS ON
+  return __builtin_isfpclass(3 /*NaN*/, x);
+}
+// CHECK-LABEL: define {{.*}} i1 @check_isfpclass_nan_f32(
+// CHECK: call i1 @llvm.is.fpclass.f32(float {{.*}}, i32 3)
+
+_Bool check_isfpclass_nan_f64(double x) {
+#pragma STDC FENV_ACCESS ON
+  return __builtin_isfpclass(3 /*NaN*/, x);
+}
+// CHECK-LABEL: define {{.*}} i1 @check_isfpclass_nan_f64(
+// CHECK: call i1 @llvm.is.fpclass.f64(double {{.*}}, i32 3)
Index: clang/test/CodeGen/builtins.c
===
--- clang/test/CodeGen/builtins.c
+++ clang/test/CodeGen/builtins.c
@@ -63,6 +63,7 @@
   P(isinf, (1.));
   P(isinf_sign, (1.));
   P(isnan, (1.));
+  P(isfpclass, (1, 1.));
 
   // Bitwise & Numeric Functions
 
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -2770,7 +2770,13 @@
diag::err_hip_invalid_args_builtin_mangled_name);
   return ExprError();
 }
+break;
   }
+
+  case Builtin::BI__builtin_isfpclass:
+if (SemaBuiltinConstantArgRange(TheCall, 0, 0, llvm::fcAllFlags))
+  return ExprError();
+break;
   }
 
   // Since the target specific builtins for each arch overlap, only check those
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -3143,6 +3143,17 @@
 return RValue::get(V);
   }
 
+  case Builtin::BI__builtin_isfpclass: {
+Expr::EvalResult Result;
+if (!E->getArg(0)->EvaluateAsInt(Result, CGM.getContext()))
+  break;
+uint64_t Test = Result.Val.getInt().getLimitedValue();
+CodeGenFunction::CGFPOptionsRAII FPOptsRAII(*this, E);
+Value *V = EmitScalarExpr(E->getArg(1));
+return RValue::get(Buil

[PATCH] D151553: [clang] Fix consteval operators in template contexts

2023-06-07 Thread Mariya Podchishchaeva via Phabricator via cfe-commits
Fznamznon updated this revision to Diff 529244.
Fznamznon added a comment.

Remove dead code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151553

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaCXX/consteval-operators.cpp
  clang/test/SemaCXX/overloaded-operator.cpp

Index: clang/test/SemaCXX/overloaded-operator.cpp
===
--- clang/test/SemaCXX/overloaded-operator.cpp
+++ clang/test/SemaCXX/overloaded-operator.cpp
@@ -585,3 +585,16 @@
   float &operator->*(B, B);
   template void f();
 }
+
+namespace test {
+namespace A {
+template T f(T t) {
+  T operator+(T, T);
+  return t + t;
+}
+}
+namespace B {
+  struct X {};
+}
+void g(B::X x) { A::f(x); }
+}
Index: clang/test/SemaCXX/consteval-operators.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/consteval-operators.cpp
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -std=c++2a -emit-llvm-only -Wno-unused-value %s -verify
+
+// expected-no-diagnostics
+
+struct A {
+  consteval A operator+() { return {}; }
+};
+consteval A operator~(A) { return {}; }
+consteval A operator+(A, A) { return {}; }
+
+template  void f() {
+  A a;
+  A b = ~a;
+  A c = a + a;
+  A d = +a;
+}
+template void f();
+
+template  void foo() {
+  T a;
+  T b = ~a;
+  T c = a + a;
+  T d = +a;
+}
+
+template void foo();
+
+template  struct B { DataT D; };
+
+template 
+consteval B operator+(B lhs, B rhs) {
+  return B{lhs.D + rhs.D};
+}
+
+template  consteval T template_add(T a, T b) { return a + b; }
+
+consteval B non_template_add(B a, B b) { return a + b; }
+
+void bar() {
+  constexpr B a{};
+  constexpr B b{};
+  auto constexpr c = a + b;
+}
+
+static_assert((template_add(B{7}, B{3})).D == 10);
+static_assert((non_template_add(B{7}, B{3})).D == 10);
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -3053,10 +3053,11 @@
   /// argument-dependent lookup, etc. Subclasses may override this routine to
   /// provide different behavior.
   ExprResult RebuildCXXOperatorCallExpr(OverloadedOperatorKind Op,
-  SourceLocation OpLoc,
-  Expr *Callee,
-  Expr *First,
-  Expr *Second);
+SourceLocation OpLoc,
+SourceLocation CalleeLoc,
+bool RequiresADL,
+const UnresolvedSetImpl &Functions,
+Expr *First, Expr *Second);
 
   /// Build a new C++ "named" cast expression, such as static_cast or
   /// reinterpret_cast.
@@ -11962,10 +11963,6 @@
 llvm_unreachable("not an overloaded operator?");
   }
 
-  ExprResult Callee = getDerived().TransformExpr(E->getCallee());
-  if (Callee.isInvalid())
-return ExprError();
-
   ExprResult First;
   if (E->getOperator() == OO_Amp)
 First = getDerived().TransformAddressOfOperand(E->getArg(0));
@@ -11982,23 +11979,39 @@
   return ExprError();
   }
 
-  if (!getDerived().AlwaysRebuild() &&
-  Callee.get() == E->getCallee() &&
-  First.get() == E->getArg(0) &&
-  (E->getNumArgs() != 2 || Second.get() == E->getArg(1)))
-return SemaRef.MaybeBindToTemporary(E);
-
   Sema::FPFeaturesStateRAII FPFeaturesState(getSema());
   FPOptionsOverride NewOverrides(E->getFPFeatures());
   getSema().CurFPFeatures =
   NewOverrides.applyOverrides(getSema().getLangOpts());
   getSema().FpPragmaStack.CurrentValue = NewOverrides;
 
-  return getDerived().RebuildCXXOperatorCallExpr(E->getOperator(),
- E->getOperatorLoc(),
- Callee.get(),
- First.get(),
- Second.get());
+  Expr *Callee = E->getCallee();
+  if (UnresolvedLookupExpr *ULE = dyn_cast(Callee)) {
+LookupResult R(SemaRef, ULE->getName(), ULE->getNameLoc(),
+   Sema::LookupOrdinaryName);
+if (getDerived().TransformOverloadExprDecls(ULE, ULE->requiresADL(), R))
+  return ExprError();
+
+return getDerived().RebuildCXXOperatorCallExpr(
+E->getOperator(), E->getOperatorLoc(), Callee->getBeginLoc(),
+ULE->requiresADL(), R.asUnresolvedSet(), First.get(), Second.get());
+  }
+
+  UnresolvedSet<1> Functions;
+  if (ImplicitCastExpr *ICE = dyn_cast(Callee))
+Callee = ICE->getSubExprAsWritten();
+  NamedDecl *DR = cast(Callee)->getDecl();
+  ValueDecl *VD = cast_or_null(
+  getDerived().TransformDecl(DR->getLocation(), DR));
+  if (!

[PATCH] D151553: [clang] Fix consteval operators in template contexts

2023-06-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision.
cor3ntin added a comment.
This revision is now accepted and ready to land.

I think this looks good!
Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151553

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


[PATCH] D152351: [clang] Add __builtin_isfpclass

2023-06-07 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff added a comment.

In D152351#4402615 , @qiucf wrote:

> It's necessary to check range of first argument in `SemaChecking.cpp` using 
> `SemaBuiltinConstantArgRange`.
>
>   _Bool check_isfpclass_1(float x) { return __builtin_isfpclass(123456, x); } 
> // ICE
>   
>   int g;
>   // error: cannot compile this builtin function yet
>   // there's better diagnostics when 1st argument is not constant
>   _Bool check_isfpclass_2(float x) { return __builtin_isfpclass(g, x); }

Added the check and a test for it. Thanks!




Comment at: clang/include/clang/Basic/Builtins.def:491
 BUILTIN(__builtin_isnormal,   "i.", "FnctE")
+BUILTIN(__builtin_isfpclass,  "iCi.", "nctE")
 

qiucf wrote:
> Why these intrinsics' type spec end with dot? I thought they are for vaargs.
It is a hack to have polymorphic builtin function. The argument `.` represents 
one argument but it can be of any type. So no need to have separate functions 
for every type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152351

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


[PATCH] D152351: [clang] Add __builtin_isfpclass

2023-06-07 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added inline comments.



Comment at: clang/test/CodeGen/isfpclass.c:1
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -O1 -emit-llvm %s -o - | 
FileCheck %s
+

Use generated checks?



Comment at: clang/test/CodeGen/isfpclass.c:2
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -S -O1 -emit-llvm %s -o - | 
FileCheck %s
+
+_Bool check_isfpclass_finite(float x) {

 Can you also add a half test, also vectors



Comment at: clang/test/CodeGen/isfpclass.c:11
+#pragma STDC FENV_ACCESS ON
+  return __builtin_isfpclass(504 /*Finite*/, x);
+}

These operands are backwards, the test should be second 



Comment at: clang/test/Sema/builtins.c:383
+int test_is_fpclass(float x) {
+  return __builtin_isfpclass(1024, x); // expected-error {{argument value 1024 
is outside the valid range [0, 1023]}}
+}

Need a test for non-immediate class mask. Also a negative value 



Comment at: clang/test/Sema/builtins.c:384
+  return __builtin_isfpclass(1024, x); // expected-error {{argument value 1024 
is outside the valid range [0, 1023]}}
+}

What happens if you pass complex?



Comment at: llvm/lib/IR/IRBuilder.cpp:1379
+Value *IRBuilderBase::createIsFPClass(Value *FPNum, unsigned Test) {
+  auto TestV = llvm::ConstantInt::get(Type::getInt32Ty(Context), Test);
+  Module *M = BB->getParent()->getParent();

Use IRBuilder::getInt32


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152351

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


[PATCH] D152351: [clang] Add __builtin_isfpclass

2023-06-07 Thread Matt Arsenault via Phabricator via cfe-commits
arsenm added a comment.

Also should get mentioned in the builtin docs and release notes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152351

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


[PATCH] D152002: [clang][wip] Better handling of dependent lambda. This is an attempt to fix GH62916. However, it breaks GH57960 I seem unable to find something that works for both issues.

2023-06-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin abandoned this revision.
cor3ntin added a subscriber: Eric.
cor3ntin added a comment.

@eric suggested we should not try to transform the body of the lambda at all, 
which may be a better approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152002

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


[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

2023-06-07 Thread Andrew Gozillon via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcda46cc4f921: [Clang][OpenMP][IRBuilder] Move 
registerTargetGlobalVariable &… (authored by agozillon).

Changed prior to commit:
  https://reviews.llvm.org/D149162?vs=524279&id=529258#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149162

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -5855,4 +5855,84 @@
   GlobalValue::WeakAnyLinkage);
   EXPECT_TRUE(InfoManager.hasDeviceGlobalVarEntryInfo("gvar"));
 }
+
+// Tests both registerTargetGlobalVariable and getAddrOfDeclareTargetVar as they
+// call each other (recursively in some cases). The test case test these
+// functions by utilising them for host code generation for declare target
+// global variables
+TEST_F(OpenMPIRBuilderTest, registerTargetGlobalVariable) {
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+  OpenMPIRBuilderConfig Config(false, false, false, false);
+  OMPBuilder.setConfig(Config);
+
+  std::vector TargetTriple;
+  TargetTriple.emplace_back("amdgcn-amd-amdhsa");
+
+  TargetRegionEntryInfo EntryInfo("", 42, 4711, 17);
+  std::vector RefsGathered;
+
+  std::vector Globals;
+  auto *IntTy = Type::getInt32Ty(Ctx);
+  for (int I = 0; I < 2; ++I) {
+Globals.push_back(M->getOrInsertGlobal(
+"test_data_int_" + std::to_string(I), IntTy, [&]() -> GlobalVariable * {
+  return new GlobalVariable(
+  *M, IntTy, false, GlobalValue::LinkageTypes::WeakAnyLinkage,
+  ConstantInt::get(IntTy, I), "test_data_int_" + std::to_string(I));
+}));
+  }
+
+  OMPBuilder.registerTargetGlobalVariable(
+  OffloadEntriesInfoManager::OMPTargetGlobalVarEntryTo,
+  OffloadEntriesInfoManager::OMPTargetDeviceClauseAny, false, true,
+  EntryInfo, Globals[0]->getName(), RefsGathered, false, TargetTriple,
+  nullptr, nullptr, Globals[0]->getType(), Globals[0]);
+
+  OMPBuilder.registerTargetGlobalVariable(
+  OffloadEntriesInfoManager::OMPTargetGlobalVarEntryLink,
+  OffloadEntriesInfoManager::OMPTargetDeviceClauseAny, false, true,
+  EntryInfo, Globals[1]->getName(), RefsGathered, false, TargetTriple,
+  nullptr, nullptr, Globals[1]->getType(), Globals[1]);
+
+  llvm::OpenMPIRBuilder::EmitMetadataErrorReportFunctionTy &&ErrorReportfn =
+  [](llvm::OpenMPIRBuilder::EmitMetadataErrorKind Kind,
+ const llvm::TargetRegionEntryInfo &EntryInfo) -> void {
+// If this is invoked, then we want to emit an error, even if it is not
+// neccesarily the most readable, as something has went wrong. The
+// test-suite unfortunately eats up all error output
+ASSERT_EQ(Kind, Kind);
+  };
+
+  OMPBuilder.createOffloadEntriesAndInfoMetadata(ErrorReportfn);
+
+  // Clauses for data_int_0 with To + Any clauses for the host
+  std::vector OffloadEntries;
+  OffloadEntries.push_back(M->getNamedGlobal(".omp_offloading.entry_name"));
+  OffloadEntries.push_back(
+  M->getNamedGlobal(".omp_offloading.entry.test_data_int_0"));
+
+  // Clauses for data_int_1 with Link + Any clauses for the host
+  OffloadEntries.push_back(
+  M->getNamedGlobal("test_data_int_1_decl_tgt_ref_ptr"));
+  OffloadEntries.push_back(M->getNamedGlobal(".omp_offloading.entry_name.1"));
+  OffloadEntries.push_back(M->getNamedGlobal(
+  ".omp_offloading.entry.test_data_int_1_decl_tgt_ref_ptr"));
+
+  for (unsigned I = 0; I < OffloadEntries.size(); ++I)
+EXPECT_NE(OffloadEntries[I], nullptr);
+
+  // Metadata generated for the host offload module
+  NamedMDNode *OffloadMetadata = M->getNamedMetadata("omp_offload.info");
+  EXPECT_NE(OffloadMetadata, nullptr);
+  if (OffloadMetadata) {
+EXPECT_EQ(OffloadMetadata->getOperand(0)->getOperand(1).equalsStr(
+  "test_data_int_0"),
+  true);
+EXPECT_EQ(OffloadMetadata->getOperand(1)->getOperand(1).equalsStr(
+  "test_data_int_1_decl_tgt_ref_ptr"),
+  true);
+  }
+}
+
 } // namespace
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -34,6 +34,7 @@
 #include "llvm/IR/Value.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Target/TargetOptions.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
@@ -5132,7 +5133,8 @@
   static_cast(
   

[clang] cda46cc - [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

2023-06-07 Thread Andrew Gozillon via cfe-commits

Author: Andrew Gozillon
Date: 2023-06-07T07:02:25-05:00
New Revision: cda46cc4f921f6b288c57a68b901ec2f57134606

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

LOG: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & 
getAddrOfDeclareTargetVar into the OMPIRBuilder

This change tries to move registerTargetglobalVariable and
getAddrOfDeclareTargetVar out of Clang's CGOpenMPRuntime
and into the OMPIRBuilder for shared use with MLIR's OpenMPDialect
and Flang (or other languages that may want to utilise it).

This primarily does this by trying to hoist the Clang specific
types into arguments or callback functions in the form of
lambdas, replacing it with LLVM equivelants and
utilising shared OMPIRBuilder enumerators for
the clauses, rather than Clang's own variation.

Reviewers: jsjodin, jdoerfert

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

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 2feab9e9a3322..6958ed28bffb2 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -1601,71 +1601,79 @@ CGOpenMPRuntime::createDispatchNextFunction(unsigned 
IVSize, bool IVSigned) {
   return CGM.CreateRuntimeFunction(FnTy, Name);
 }
 
-/// Obtain information that uniquely identifies a target entry. This
-/// consists of the file and device IDs as well as line number associated with
-/// the relevant entry source location.
-static llvm::TargetRegionEntryInfo
-getTargetEntryUniqueInfo(ASTContext &C, SourceLocation Loc,
- StringRef ParentName = "") {
-  SourceManager &SM = C.getSourceManager();
-
-  // The loc should be always valid and have a file ID (the user cannot use
-  // #pragma directives in macros)
-
-  assert(Loc.isValid() && "Source location is expected to be always valid.");
-
-  PresumedLoc PLoc = SM.getPresumedLoc(Loc);
-  assert(PLoc.isValid() && "Source location is expected to be always valid.");
+llvm::OffloadEntriesInfoManager::OMPTargetDeviceClauseKind
+convertDeviceClause(const VarDecl *VD) {
+  std::optional DevTy =
+  OMPDeclareTargetDeclAttr::getDeviceType(VD);
+  if (!DevTy)
+return llvm::OffloadEntriesInfoManager::OMPTargetDeviceClauseNone;
 
-  llvm::sys::fs::UniqueID ID;
-  if (auto EC = llvm::sys::fs::getUniqueID(PLoc.getFilename(), ID)) {
-PLoc = SM.getPresumedLoc(Loc, /*UseLineDirectives=*/false);
-assert(PLoc.isValid() && "Source location is expected to be always 
valid.");
-if (auto EC = llvm::sys::fs::getUniqueID(PLoc.getFilename(), ID))
-  SM.getDiagnostics().Report(diag::err_cannot_open_file)
-  << PLoc.getFilename() << EC.message();
+  switch (*DevTy) {
+  case OMPDeclareTargetDeclAttr::DT_Host:
+return llvm::OffloadEntriesInfoManager::OMPTargetDeviceClauseHost;
+break;
+  case OMPDeclareTargetDeclAttr::DT_NoHost:
+return llvm::OffloadEntriesInfoManager::OMPTargetDeviceClauseNoHost;
+break;
+  case OMPDeclareTargetDeclAttr::DT_Any:
+return llvm::OffloadEntriesInfoManager::OMPTargetDeviceClauseAny;
+break;
+  default:
+return llvm::OffloadEntriesInfoManager::OMPTargetDeviceClauseNone;
+break;
   }
+}
 
-  return llvm::TargetRegionEntryInfo(ParentName, ID.getDevice(), ID.getFile(),
- PLoc.getLine());
+llvm::OffloadEntriesInfoManager::OMPTargetGlobalVarEntryKind
+convertCaptureClause(const VarDecl *VD) {
+  std::optional MapType =
+  OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD);
+  if (!MapType)
+return llvm::OffloadEntriesInfoManager::OMPTargetGlobalVarEntryNone;
+  switch (*MapType) {
+  case OMPDeclareTargetDeclAttr::MapTypeTy::MT_To:
+return llvm::OffloadEntriesInfoManager::OMPTargetGlobalVarEntryTo;
+break;
+  case OMPDeclareTargetDeclAttr::MapTypeTy::MT_Enter:
+return llvm::OffloadEntriesInfoManager::OMPTargetGlobalVarEntryEnter;
+break;
+  case OMPDeclareTargetDeclAttr::MapTypeTy::MT_Link:
+return llvm::OffloadEntriesInfoManager::OMPTargetGlobalVarEntryLink;
+break;
+  default:
+return llvm::OffloadEntriesInfoManager::OMPTargetGlobalVarEntryNone;
+break;
+  }
 }
 
 Address CGOpenMPRuntime::getAddrOfDeclareTargetVar(const VarDecl *VD) {
-  if (CGM.getLangOpts().OpenMPSimd)
-return Address::invalid();
-  std::optional Res =
-  OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD);
-  if (Res && (*Res == OMPDeclareTargetDeclAttr::MT_Link ||
-  ((*Res == OMPDeclareTargetDeclAttr::MT_To ||
-*Res == OMPDeclareTarget

[clang] f10b1c5 - [Clang][OpenMP] Fix -Wcovered-switch-default in CGOpenMPRuntime.cpp (NFC)

2023-06-07 Thread Jie Fu via cfe-commits

Author: Jie Fu
Date: 2023-06-07T20:23:48+08:00
New Revision: f10b1c5f3cbef76b189566332eebabd017296655

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

LOG: [Clang][OpenMP] Fix -Wcovered-switch-default in CGOpenMPRuntime.cpp (NFC)

/data/llvm-project/clang/lib/CodeGen/CGOpenMPRuntime.cpp:1621:3: error: default 
label in switch which covers all enumeration values 
[-Werror,-Wcovered-switch-default]
  default:
  ^
/data/llvm-project/clang/lib/CodeGen/CGOpenMPRuntime.cpp:1643:3: error: default 
label in switch which covers all enumeration values 
[-Werror,-Wcovered-switch-default]
  default:
  ^
2 errors generated.

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 6958ed28bffb2..c7d62fc075fbb 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -1608,7 +1608,7 @@ convertDeviceClause(const VarDecl *VD) {
   if (!DevTy)
 return llvm::OffloadEntriesInfoManager::OMPTargetDeviceClauseNone;
 
-  switch (*DevTy) {
+  switch ((int)*DevTy) { // Avoid -Wcovered-switch-default
   case OMPDeclareTargetDeclAttr::DT_Host:
 return llvm::OffloadEntriesInfoManager::OMPTargetDeviceClauseHost;
 break;
@@ -1630,7 +1630,7 @@ convertCaptureClause(const VarDecl *VD) {
   OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD);
   if (!MapType)
 return llvm::OffloadEntriesInfoManager::OMPTargetGlobalVarEntryNone;
-  switch (*MapType) {
+  switch ((int)*MapType) { // Avoid -Wcovered-switch-default
   case OMPDeclareTargetDeclAttr::MapTypeTy::MT_To:
 return llvm::OffloadEntriesInfoManager::OMPTargetGlobalVarEntryTo;
 break;



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


[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

2023-06-07 Thread Jens Massberg via Phabricator via cfe-commits
massberg added a comment.

Any advice how I should proceed here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139400

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


[PATCH] D148700: [clang] Add support for “regular” keyword attributes

2023-06-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In D148700#4401451 , @rsmith wrote:

> In D148700#4401353 , @jyknight 
> wrote:
>
>> Yes, standard attributes aren't supposed to be used for things which affect 
>> the type system (although, we certainly have many, already, which do, since 
>> we expose most GCC-syntax attributes also as C++-standard attribute syntax!)
>
> The rule that standard attributes don't affect semantics only applies to 
> attributes specified by the language standard. There is no expectation that 
> vendor attributes avoid such effects. In particular, I'm concerned by this in 
> the description of this change:
>
> In D148700 , @rsandifo-arm wrote:
>
>> Really, the “only” thing wrong with using standard attributes is that 
>> standard attributes cannot affect semantics.
>
> If the only reason for this patch series is an idea that vendor attributes 
> using `[[...]]` syntax can't affect program semantics, then I think this 
> change is not justified, because vendor attributes using `[[...]]` syntax can 
> and usually do affect program semantics. But the documentation change here 
> makes the point that using a keyword attribute may be as good idea in cases 
> where you would always want compilation to fail on a compiler that doesn't 
> understand the annotation, rather than the annotation being ignored (likely 
> with a warning), so maybe that's reasonable justification for this direction.

That is the reason for this direction -- we have enough instances where people 
want to add a vendor attribute but it is not safe for other implementations to 
ignore it (due to ABI, correctness, etc). This feature allows adding the 
attribute as a keyword which can appear anywhere the attribute can appear. Due 
to it being expressed as a keyword in source, the other implementations will 
fail to accept the code, whereas use of a vendor-specified attribute would be 
(potentially silently) accepted with ill-effects.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148700

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


[PATCH] D152207: [HIP] Instruct lld to go through all archives

2023-06-07 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152207

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


[PATCH] D142932: Multilib YAML parsing

2023-06-07 Thread Michael Platings via Phabricator via cfe-commits
michaelplatings updated this revision to Diff 529269.
michaelplatings added a comment.

@phosek thanks for your suggestion, that's now implemented. In practise for 
LLVM Embedded Toolchain for Arm we haven't yet needed `NoMatchFlags` so I've 
removed that feature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142932

Files:
  clang/include/clang/Driver/Multilib.h
  clang/lib/Driver/Multilib.cpp
  clang/unittests/Driver/MultilibTest.cpp

Index: clang/unittests/Driver/MultilibTest.cpp
===
--- clang/unittests/Driver/MultilibTest.cpp
+++ clang/unittests/Driver/MultilibTest.cpp
@@ -13,6 +13,7 @@
 #include "clang/Driver/Multilib.h"
 #include "../../lib/Driver/ToolChains/CommonArgs.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/Version.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
@@ -187,3 +188,350 @@
   EXPECT_EQ("/a", Selection[0].gccSuffix());
   EXPECT_EQ("/b", Selection[1].gccSuffix());
 }
+
+static void diagnosticCallback(const llvm::SMDiagnostic &D, void *Out) {
+  *reinterpret_cast(Out) = D.getMessage();
+}
+
+static bool parseYaml(MultilibSet &MS, std::string &Diagnostic,
+  const char *Data) {
+  auto ErrorOrMS = MultilibSet::parseYaml(llvm::MemoryBufferRef(Data, "TEST"),
+  diagnosticCallback, &Diagnostic);
+  if (ErrorOrMS.getError())
+return false;
+  MS = std::move(ErrorOrMS.get());
+  return true;
+}
+
+static bool parseYaml(MultilibSet &MS, const char *Data) {
+  auto ErrorOrMS = MultilibSet::parseYaml(llvm::MemoryBufferRef(Data, "TEST"));
+  if (ErrorOrMS.getError())
+return false;
+  MS = std::move(ErrorOrMS.get());
+  return true;
+}
+
+// When updating this version also update MultilibVersionCurrent in Multilib.cpp
+#define YAML_PREAMBLE "MultilibVersion: 1.0\n"
+
+TEST(MultilibTest, ParseInvalid) {
+  std::string Diagnostic;
+
+  MultilibSet MS;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, R"(
+Variants: []
+)"));
+  EXPECT_TRUE(
+  StringRef(Diagnostic).contains("missing required key 'MultilibVersion'"))
+  << Diagnostic;
+
+  // Reject files with a different major version
+  EXPECT_FALSE(parseYaml(MS, Diagnostic,
+ R"(
+MultilibVersion: 2.0
+Variants: []
+)"));
+  EXPECT_TRUE(
+  StringRef(Diagnostic).contains("multilib version 2.0 is unsupported"))
+  << Diagnostic;
+  EXPECT_FALSE(parseYaml(MS, Diagnostic,
+ R"(
+MultilibVersion: 0.1
+Variants: []
+)"));
+  EXPECT_TRUE(
+  StringRef(Diagnostic).contains("multilib version 0.1 is unsupported"))
+  << Diagnostic;
+
+  // Reject files with a later minor version
+  EXPECT_FALSE(parseYaml(MS, Diagnostic,
+ R"(
+MultilibVersion: 1.9
+Variants: []
+)"));
+  EXPECT_TRUE(
+  StringRef(Diagnostic).contains("multilib version 1.9 is unsupported"))
+  << Diagnostic;
+
+  // Accept files with the same major version and the same or earlier minor
+  // version
+  EXPECT_TRUE(parseYaml(MS, Diagnostic, R"(
+MultilibVersion: 1.0
+Variants: []
+)")) << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("missing required key 'Variants'"))
+  << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE R"(
+Variants:
+- Dir: /abc
+  Flags: []
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("paths must be relative"))
+  << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE R"(
+Variants:
+- Flags: []
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("missing required key 'Dir'"))
+  << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE R"(
+Variants:
+- Dir: .
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("missing required key 'Flags'"))
+  << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE R"(
+Variants: []
+FlagMap:
+- Match: abc
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("value required for 'Flags'"))
+  << Diagnostic;
+
+  EXPECT_FALSE(parseYaml(MS, Diagnostic, YAML_PREAMBLE R"(
+Variants: []
+FlagMap:
+- Dir: .
+  Match: '('
+  Flags: []
+)"));
+  EXPECT_TRUE(StringRef(Diagnostic).contains("parentheses not balanced"))
+  << Diagnostic;
+}
+
+TEST(MultilibTest, Parse) {
+  MultilibSet MS;
+  EXPECT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
+Variants:
+- Dir: .
+  Flags: []
+)"));
+  EXPECT_EQ(1U, MS.size());
+  EXPECT_EQ("", MS.begin()->gccSuffix());
+
+  EXPECT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
+Variants:
+- Dir: abc
+  Flags: []
+)"));
+  EXPECT_EQ(1U, MS.size());
+  EXPECT_EQ("/abc", MS.begin()->gccSuffix());
+
+  EXPECT_TRUE(parseYaml(MS, YAML_PREAMBLE R"(
+Variants:
+- Dir: pqr
+  Flags: [-mfloat-abi=soft]
+)"));
+  EXPECT_EQ(1U, MS.size());
+  EXPECT_EQ("/pqr", MS.begin()->gccSuffix()

[PATCH] D152312: HIP: Use frexp builtins in math headers

2023-06-07 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks.


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

https://reviews.llvm.org/D152312

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


[PATCH] D138396: HIP: Directly call signbit builtins

2023-06-07 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks.


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

https://reviews.llvm.org/D138396

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


[PATCH] D138399: HIP: Directly call isinf builtins

2023-06-07 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks


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

https://reviews.llvm.org/D138399

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


[PATCH] D138395: HIP: Directly call fmin/fmax builtins

2023-06-07 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl accepted this revision.
yaxunl added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks.


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

https://reviews.llvm.org/D138395

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


[PATCH] D152285: Add support for the NO_COLOR environment variable

2023-06-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman marked 2 inline comments as done.
aaron.ballman added a comment.

In D152285#4401348 , @MaskRay wrote:

> If we don't intend to support both standards, we can close 
> https://github.com/llvm/llvm-project/issues/23983 (CLICOLOR) as a wontfix :)

That was my plan.

In D152285#4402603 , @jhasse wrote:

> There's a valid usecase `CLICOLOR_FORCE`: Force color diagnostics. The 
> "disable colors" part of https://bixense.com/clicolors/ is not that important 
> to me, I could change it to point to `NO_COLOR` instead?
>
> btw: I've tried to join the "standards" a few years ago: 
> https://github.com/jcs/no_color/issues/28
>
> Right now I think it would be best to drop `CLICOLOR` and have the following 
> simple rules:
>
> - if `NO_COLOR` set: disable colors
> - if `CLICOLOR_FORCE` set: always set colors
> - else: isatty
>
> What do you think?

I don't think we should implement all of one standard and part of another. My 
thinking is: users will have to do `NO_COLOR=1` and `CLICOLOR_FORCE=0` to 
disable colors due to there being competing standards, so we need to support 
only one of these. `CLICOLOR_FORCE` is the more powerful option, but I don't 
know what compelling use case there is for forcing colors *on*, but forcing 
them *off* makes a lot of sense to me from an accessibility standpoint. So I 
think supporting `NO_COLOR` alone is sufficient until we know why users need to 
force-enable colors. If we are compelled by that reasoning, we could implement 
the other standard at that point (and I'd suggest we implement it fully). But 
I'd like to avoid that right now because then we have to think about what 
happens in circumstances like `NO_COLOR=1 CLICOLOR_FORCE=1`




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2346
+  bool ShowColors = true;
+  if (std::optional NoColor =
+  llvm::sys::Process::GetEnv("NO_COLOR")) {

MaskRay wrote:
> We should inspect that `NO_COLOR` is not empty.
> 
> > Command-line software which adds ANSI color to its output by default should 
> > check for a NO_COLOR environment variable that, when **present and not an 
> > empty string** (regardless of its value), prevents the addition of ANSI 
> > color.
Good catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152285

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


[PATCH] D152251: [clang][CodeGen] Fix GPU-specific attributes being dropped by bitcode linking

2023-06-07 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl accepted this revision.
yaxunl added a comment.

LGTM. Thanks




Comment at: 
clang/test/CodeGenCUDA/link-builtin-bitcode-gpu-attrs-preserved.cu:35
+// INTERNALIZE: attributes #[[ATTR]] = {{.*}} "target-cpu"="gfx{{.*}}" 
"target-features"="+gfx11-insts"
+// NOINTERNALIZE: attributes #[[ATTR]] = {{.*}} 
"target-features"="+gfx11-insts"
+

Do we know why internalize keeps the target-cpu attribute but non-internalize 
does not?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152251

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


[PATCH] D152321: [clang] Replace use of Type::getPointerTo() (NFC)

2023-06-07 Thread Nikita Popov via Phabricator via cfe-commits
nikic added inline comments.



Comment at: clang/lib/CodeGen/Address.h:135
 llvm::Constant *BitCast = llvm::ConstantExpr::getBitCast(
-getPointer(), ElemTy->getPointerTo(getAddressSpace()));
+getPointer(), llvm::PointerType::get(ElemTy, getAddressSpace()));
 return ConstantAddress(BitCast, ElemTy, getAlignment());

In these cases, what we want to do is remove the `BitCast`, which is no longer 
necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152321

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


[PATCH] D152285: Add support for the NO_COLOR environment variable

2023-06-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment.

As I told Aaron, someone who disables color for accessibility reason is likely 
to have both variables(NO_COLOR/CLICOLOR) set. So supporting one is enough. 
Picking the one that is the simplest and most popular makes perfect sense.
This is great.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152285

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


[PATCH] D139541: Fix parameter name in Sema::addInitCapture to ByRef.

2023-06-07 Thread Jens Massberg via Phabricator via cfe-commits
massberg updated this revision to Diff 529277.
massberg added a comment.

Use correct format for burprone-argument-comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139541

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaLambda.cpp


Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -798,12 +798,11 @@
   return NewVD;
 }
 
-void Sema::addInitCapture(LambdaScopeInfo *LSI, VarDecl *Var,
-  bool isReferenceType) {
+void Sema::addInitCapture(LambdaScopeInfo *LSI, VarDecl *Var, bool ByRef) {
   assert(Var->isInitCapture() && "init capture flag should be set");
-  LSI->addCapture(Var, /*isBlock*/ false, isReferenceType,
-  /*isNested*/ false, Var->getLocation(), SourceLocation(),
-  Var->getType(), /*Invalid*/ false);
+  LSI->addCapture(Var, /*isBlock=*/ false, ByRef,
+  /*isNested=*/ false, Var->getLocation(), SourceLocation(),
+  Var->getType(), /*Invalid=*/ false);
 }
 
 // Unlike getCurLambda, getCurrentLambdaScopeUnsafe doesn't
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -7171,8 +7171,7 @@
   IdentifierInfo *Id, unsigned InitStyle, Expr *Init, DeclContext 
*DeclCtx);
 
   /// Add an init-capture to a lambda scope.
-  void addInitCapture(sema::LambdaScopeInfo *LSI, VarDecl *Var,
-  bool isReferenceType);
+  void addInitCapture(sema::LambdaScopeInfo *LSI, VarDecl *Var, bool ByRef);
 
   /// Note that we have finished the explicit captures for the
   /// given lambda.


Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -798,12 +798,11 @@
   return NewVD;
 }
 
-void Sema::addInitCapture(LambdaScopeInfo *LSI, VarDecl *Var,
-  bool isReferenceType) {
+void Sema::addInitCapture(LambdaScopeInfo *LSI, VarDecl *Var, bool ByRef) {
   assert(Var->isInitCapture() && "init capture flag should be set");
-  LSI->addCapture(Var, /*isBlock*/ false, isReferenceType,
-  /*isNested*/ false, Var->getLocation(), SourceLocation(),
-  Var->getType(), /*Invalid*/ false);
+  LSI->addCapture(Var, /*isBlock=*/ false, ByRef,
+  /*isNested=*/ false, Var->getLocation(), SourceLocation(),
+  Var->getType(), /*Invalid=*/ false);
 }
 
 // Unlike getCurLambda, getCurrentLambdaScopeUnsafe doesn't
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -7171,8 +7171,7 @@
   IdentifierInfo *Id, unsigned InitStyle, Expr *Init, DeclContext *DeclCtx);
 
   /// Add an init-capture to a lambda scope.
-  void addInitCapture(sema::LambdaScopeInfo *LSI, VarDecl *Var,
-  bool isReferenceType);
+  void addInitCapture(sema::LambdaScopeInfo *LSI, VarDecl *Var, bool ByRef);
 
   /// Note that we have finished the explicit captures for the
   /// given lambda.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D148216: Add support for annotations in UpdateTestChecks (NFC)

2023-06-07 Thread Henrik G Olsson via Phabricator via cfe-commits
hnrklssn marked an inline comment as done.
hnrklssn added a comment.

@nikic Did you have a look at the new patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148216

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


[PATCH] D152369: [clang][dataflow][NFC] Expand comments on losing values in optional checker.

2023-06-07 Thread Martin Böhme via Phabricator via cfe-commits
mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

While working on the ongoing migration to strict handling of value
categories (see https://discourse.llvm.org/t/70086), I ran into issues related
to losing the value associated with an optional.

This issue is hinted at in the existing comments, but the issue didn't become
sufficiently clear to me from those, so I thought it would be worth capturing
more details, along with ideas for how this issue might be fixed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152369

Files:
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp


Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -315,11 +315,16 @@
 auto &ValueLoc = ValuePtr->getPointeeLoc();
 if (Env.getValue(ValueLoc) == nullptr) {
   // The property was previously set, but the value has been lost. This can
-  // happen, for example, because of an environment merge (where the two
-  // environments mapped the property to different values, which resulted 
in
-  // them both being discarded), or when two blocks in the CFG, with 
neither
-  // a dominator of the other, visit the same optional value, or even when 
a
-  // block is revisited during testing to collect per-statement state.
+  // happen in various situations, for example:
+  // - Because of an environment merge (where the two environments mapped
+  //   the property to different values, which resulted in them both being
+  //   discarded).
+  // - When two blocks in the CFG, with neither a dominator of the other,
+  //   visit the same optional value. (FIXME: This is something we can and
+  //   should fix -- see also the lengthy FIXME below.)
+  // - Or even when a block is revisited during testing to collect
+  //   per-statement state.
+  //
   // FIXME: This situation means that the optional contents are not shared
   // between branches and the like. Practically, this lack of sharing
   // reduces the precision of the model when the contents are relevant to
@@ -340,6 +345,51 @@
   auto &ValueLoc = Env.createStorageLocation(Ty);
   Env.setValue(ValueLoc, *ValueVal);
   auto &ValuePtr = Env.create(ValueLoc);
+  // FIXME:
+  // The change we make to the `value` property below may become visible to
+  // other blocks that aren't successors of the current block and therefore
+  // don't see the change we made above mapping `ValueLoc` to `ValueVal`. For
+  // example:
+  //
+  //   void target(optional oo, bool b) {
+  // // `oo` is associated with a `StructValue` here, which we will call
+  // // `OptionalVal`.
+  //
+  // // The `has_value` property is set on `OptionalVal` (but not the
+  // // `value` property yet).
+  // if (!oo.has_value()) return;
+  //
+  // if (b) {
+  //   // Let's assume we transfer the `if` branch first.
+  //   //
+  //   // This causes us to call `maybeInitializeOptionalValueMember()`,
+  //   // which causes us to set the `value` property on `OptionalVal`
+  //   // (which had not been set until this point). This `value` property
+  //   // refers to a `PointerValue`, which in turn refers to a
+  //   // StorageLocation` that is associated to an `IntegerValue`.
+  //   oo.value();
+  // } else {
+  //   // Let's assume we transfer the `else` branch after the `if` branch.
+  //   //
+  //   // We see the `value` property that the `if` branch set on
+  //   // `OptionalVal`, but in the environment for this block, the
+  //   // `StorageLocation` in the `PointerValue` is not associated with 
any
+  //   // `Value`.
+  //   oo.value();
+  // }
+  //   }
+  //
+  // This situation is currently "saved" by the code above that checks whether
+  // the `value` property is already set, and if, the `ValueLoc` is not
+  // associated with a `ValueVal`, creates a new `ValueVal`.
+  //
+  // However, what we should really do is to make sure that the change to the
+  // `value` property does not "leak" to other blocks that are not successors
+  // of this block. To do this, instead of simply setting the `value` property
+  // on the existing `OptionalVal`, we should create a new `Value` for the
+  // optional, set the property on that, and associate the storage location 
that
+  // is currently associated with the existing `OptionalVal` with the newly
+  // created `Value` instead.
   OptionalVal.setProperty("value", ValuePtr);
   return &ValueLoc;
 }


Index: clang/lib/An

[PATCH] D152017: [DebugInfo] Add flag to only emit referenced member functions

2023-06-07 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment.

Could always go with `-gsuppress-undefined-methods` if you're not happy about 
default-on options. I don't have a strong opinion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152017

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


[PATCH] D152335: [Clang] Add check to Sema::AddAlignedAttr to verify active bits is not out of range

2023-06-07 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4480
 MaximumAlignment = std::min(MaximumAlignment, uint64_t(8192));
-  if (AlignVal > MaximumAlignment) {
+  bool TooManyActiveBits = Alignment.getActiveBits() > llvm::APInt(64, 
MaximumAlignment).getActiveBits();
+

What is the purpose of using 'ActiveBits' here?  Why is this not a sub-set of 
the operator `>` from below?  That is, 'ActiveBits' allows for 0xFF when the 
'MaxValue' is 0x80 (top bit set), but the operator > would have caught that 
anyway, right?  

Also, how does 'ActiveBits' work with negative numbers? 


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

https://reviews.llvm.org/D152335

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


[PATCH] D150446: [analyzer] Check ArraySubscriptExprs in ArrayBoundCheckerV2

2023-06-07 Thread Balázs Benics via Phabricator via cfe-commits
steakhal requested changes to this revision.
steakhal added a comment.
This revision now requires changes to proceed.

In D150446#4337657 , @donat.nagy 
wrote:

> After some thinking and discussion with @gamesh411 I decided that it'd be 
> better to replace the callback used by this checker. This is a clean but 
> rough draft of this concept; for a final version I'd consider:
>
> - adding  a secondary callback that handles `*ptr` equivalently to `ptr[0]`;
> - including a special case that it's valid to form an after-the-end pointer 
> as `&arr[N]` where `N` is the length of the array;
> - including a check::Location callback that creates bug reports when these 
> after-the-end pointers are dereferenced.
>
> @steakhal What do you think about this design direction?

Could you elaborate on this part?
To clarify my position, I think we can go with either approach. We can stay 
with the check::Location, or with the PreStmt route.
To me, the only important aspect is to not regress, or prove that the places 
where we regress are for a good reason, and in the end we provide more valuable 
reports to the user.
For example, we can sacrifice some TPs for dropping a large number of FPs. That 
seems to be a good deal, even if we "regress" on some aspect.

---

Anyway, I just checked the impact of this patch and it was so big that I rerun 
the measurement to be sure (thus the delay :D), but it didn't get any better.
There are a lot more reports if I apply this patch, which suggests to me that 
there is something wrong.

For instance, in this example, we currently don't report any warnings, but with 
the patch, we would have some. https://godbolt.org/z/sjcrfP8df
We also lose some reports like this: https://godbolt.org/z/8113nrYhe

Please investigate it and also do comparative checks on real codebases to make 
sure the change is aligned with the expectations.
Given that some of our users depend on the current behavior, we should ensure 
that no reports disappear or appear unless they are justified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150446

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


[PATCH] D152335: [Clang] Add check to Sema::AddAlignedAttr to verify active bits is not out of range

2023-06-07 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4484
+  if (!TooManyActiveBits) {
+AlignVal = Alignment.getZExtValue();
+// C++11 [dcl.align]p2:

So looking more closely, THIS is the problem right here.  I think we probably 
should just be storing `MaximumAlignment` and `AlignVal` as an llvm::APInt and 
just do the comparison on `APInt`, rather than try to be tricky with the bits 
here.

Basically, don't extract the 'Alignment' from the `APInt` until after the test 
for >.


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

https://reviews.llvm.org/D152335

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


[PATCH] D144999: [Clang][MC][MachO]Only emits compact-unwind format for "canonical" personality symbols. For the rest, use DWARFs.

2023-06-07 Thread Vy Nguyen via Phabricator via cfe-commits
oontvoo updated this revision to Diff 529283.
oontvoo added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144999

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/femit-dwarf-unwind.c
  clang/test/Driver/femit-dwarf-unwind.s
  clang/tools/driver/cc1as_main.cpp
  lld/MachO/UnwindInfoSection.cpp
  lld/test/MachO/Inputs/eh-frame-x86_64-r.o
  lld/test/MachO/compact-unwind-both-local-and-dylib-personality.s
  lld/test/MachO/compact-unwind-generated.test
  lld/test/MachO/compact-unwind-lsda-folding.s
  lld/test/MachO/compact-unwind-stack-ind.s
  lld/test/MachO/compact-unwind.s
  lld/test/MachO/eh-frame-personality-dedup.s
  lld/test/MachO/eh-frame.s
  lld/test/MachO/icf-only-lsda-folded.s
  lld/test/MachO/icf.s
  lld/test/MachO/invalid/compact-unwind-bad-reloc.s
  lld/test/MachO/invalid/compact-unwind-personalities.s
  llvm/include/llvm/MC/MCAsmBackend.h
  llvm/include/llvm/MC/MCContext.h
  llvm/include/llvm/MC/MCTargetOptions.h
  llvm/include/llvm/MC/MCTargetOptionsCommandFlags.h
  llvm/lib/MC/MCAsmBackend.cpp
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCStreamer.cpp
  llvm/lib/MC/MCTargetOptionsCommandFlags.cpp
  llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
  llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackendDarwin.h
  llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
  llvm/test/CodeGen/X86/2014-08-29-CompactUnwind.ll
  llvm/test/CodeGen/X86/compact-unwind.ll
  llvm/test/MC/AArch64/arm64-leaf-compact-unwind.s
  llvm/test/MC/MachO/AArch64/emit-dwarf-unwind.s
  llvm/test/MC/MachO/ARM/compact-unwind-armv7k.s
  llvm/test/MC/X86/compact-unwind.s

Index: llvm/test/MC/X86/compact-unwind.s
===
--- llvm/test/MC/X86/compact-unwind.s
+++ llvm/test/MC/X86/compact-unwind.s
@@ -1,4 +1,4 @@
-# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin10.0 %s | llvm-objdump --unwind-info - | FileCheck %s
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin10.0 %s -emit-compact-unwind-non-canonical=true | llvm-objdump --unwind-info - | FileCheck %s
 
 	.section	__TEXT,__text,regular,pure_instructions
 	.macosx_version_min 10, 10
Index: llvm/test/MC/MachO/ARM/compact-unwind-armv7k.s
===
--- llvm/test/MC/MachO/ARM/compact-unwind-armv7k.s
+++ llvm/test/MC/MachO/ARM/compact-unwind-armv7k.s
@@ -1,4 +1,4 @@
-@ RUN: llvm-mc -triple=thumbv7k-apple-watchos2.0.0 -filetype=obj -o %t < %s && llvm-objdump --unwind-info %t | FileCheck %s
+@ RUN: llvm-mc -triple=thumbv7k-apple-watchos2.0.0 -emit-compact-unwind-non-canonical=true -filetype=obj -o %t < %s && llvm-objdump --unwind-info %t | FileCheck %s
 
 @ CHECK: Contents of __compact_unwind section:
 
Index: llvm/test/MC/MachO/AArch64/emit-dwarf-unwind.s
===
--- llvm/test/MC/MachO/AArch64/emit-dwarf-unwind.s
+++ llvm/test/MC/MachO/AArch64/emit-dwarf-unwind.s
@@ -1,11 +1,11 @@
 // RUN: rm -rf %t; mkdir %t
-// RUN: llvm-mc -triple x86_64-apple-macos11.0 %s -filetype=obj -o %t/x86_64.o
+// RUN: llvm-mc -triple x86_64-apple-macos11.0 %s -filetype=obj -o %t/x86_64.o -emit-compact-unwind-non-canonical=true
 // RUN: llvm-objdump --macho --dwarf=frames %t/x86_64.o | FileCheck %s --check-prefix TWO-FDES
-// RUN: llvm-mc -triple arm64-apple-macos11.0 %s -filetype=obj -o %t/arm64.o
+// RUN: llvm-mc -triple arm64-apple-macos11.0 %s -filetype=obj -o %t/arm64.o -emit-compact-unwind-non-canonical=true
 // RUN: llvm-objdump --macho --dwarf=frames %t/arm64.o | FileCheck %s --check-prefix ONE-FDE
-// RUN: llvm-mc -triple x86_64-apple-macos11.0 %s -filetype=obj --emit-dwarf-unwind no-compact-unwind -o %t/x86_64-no-dwarf.o
+// RUN: llvm-mc -triple x86_64-apple-macos11.0 %s -filetype=obj --emit-dwarf-unwind no-compact-unwind -o %t/x86_64-no-dwarf.o -emit-compact-unwind-non-canonical=true
 // RUN: llvm-objdump --macho --dwarf=frames %t/x86_64-no-dwarf.o | FileCheck %s --check-prefix ONE-FDE
-// RUN: llvm-mc -triple arm64-apple-macos11.0 %s -filetype=obj --emit-dwarf-unwind always -o %t/arm64-dwarf.o
+// RUN: llvm-mc -triple arm64-apple-macos11.0 %s -filetype=obj --emit-dwarf-unwind always -o %t/arm64-dwarf.o -emit-compact-unwind-non-canonical=true
 // RUN: llvm-objdump --macho --dwarf=frames %t/arm64-dwarf.o | FileCheck %s --check-prefix TWO-FDES
 
 // TWO-FDES: FDE
Index: llvm/test/MC/AArch64/arm64-leaf-compact-unwind.s
===
--- llvm/test/MC/AArch64/arm64-leaf-compact-unwind.s
+++ llvm/test/MC/AArch64/arm64-leaf-compact-unwind.s
@@ -1,4 +1,4 @@
-// RUN: llvm-mc -triple=arm64-apple-ios -filetype=obj < %s | \
+// RUN: llvm-mc -triple=arm64-apple-ios -filety

[PATCH] D152197: Fix static analyzer bugs with null pointer dereferences in CheckSizelessVectorOperands()

2023-06-07 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:11148
   if (LHSType->isVLSTBuiltinType() && RHSType->isVLSTBuiltinType() &&
+  LHSBuiltinTy && RHSBuiltinTy &&
   Context.getBuiltinVectorTypeInfo(LHSBuiltinTy).EC !=

I think this is unnecessary.  `isVLSTBuiltinType` only returns true if 
`LHSType` is a `BuiltinType` already (or at least, a subset-of). See : 
https://clang.llvm.org/doxygen/Type_8cpp_source.html#l02409




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152197

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


[PATCH] D152251: [clang][CodeGen] Fix GPU-specific attributes being dropped by bitcode linking

2023-06-07 Thread Pierre van Houtryve via Phabricator via cfe-commits
Pierre-vh marked 2 inline comments as done.
Pierre-vh added inline comments.



Comment at: 
clang/test/CodeGenCUDA/link-builtin-bitcode-gpu-attrs-preserved.cu:34
+// CHECK: define {{.*}} i32 @do_intrin_stuff() #[[ATTR:[0-9]+]]
+// CHECK: attributes #[[ATTR]] = {{.*}} "target-features"="+gfx11-insts"
+

arsenm wrote:
> arsenm wrote:
> > Also should make sure target-cpu was set
> Did this previously receive the target-features spam implied by the target?
> Did this previously receive the target-features spam implied by the target?

I think it did, the attributes were filled with things like "+gfx9-insts", etc.

> Do we know why internalize keeps the target-cpu attribute but non-internalize 
> does not?

PropagateAttrs is only set for -mlink-builtin-bitcode, see 
CompilerInvocation.cpp@1888 (where OPT_mlink_builtin_bitcode is processed)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152251

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


[PATCH] D152251: [clang][CodeGen] Fix GPU-specific attributes being dropped by bitcode linking

2023-06-07 Thread Pierre van Houtryve via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Pierre-vh marked an inline comment as done.
Closed by commit rG23431b524603: [clang][CodeGen] Fix GPU-specific attributes 
being dropped by bitcode linking (authored by Pierre-vh).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152251

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCUDA/Inputs/ocml-sample-target-attrs.cl
  clang/test/CodeGenCUDA/link-builtin-bitcode-denormal-fp-mode.cu
  clang/test/CodeGenCUDA/link-builtin-bitcode-gpu-attrs-preserved.cu

Index: clang/test/CodeGenCUDA/link-builtin-bitcode-gpu-attrs-preserved.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/link-builtin-bitcode-gpu-attrs-preserved.cu
@@ -0,0 +1,48 @@
+// Verify the behavior of the +gfxN-insts in the way that
+// rocm-device-libs should be built with. e.g. If the device libraries has a function
+// with "+gfx11-insts", that attribute should still be present after linking and not
+// overwritten with the current target's settings.
+
+// This is important because at this time, many device-libs functions that are only
+// available on some GPUs put an attribute such as "+gfx11-insts" so that
+// AMDGPURemoveIncompatibleFunctions can detect & remove them if needed.
+
+// Build the fake device library in the way rocm-device-libs should be built.
+//
+// RUN: %clang_cc1 -x cl -triple amdgcn-amd-amdhsa\
+// RUN:   -mcode-object-version=none -emit-llvm-bc \
+// RUN:   %S/Inputs/ocml-sample-target-attrs.cl -o %t.bc
+
+// Check the default behavior
+// RUN: %clang_cc1 -x hip -triple amdgcn-amd-amdhsa -target-cpu gfx803 -fcuda-is-device \
+// RUN:   -mlink-builtin-bitcode %t.bc \
+// RUN:   -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,INTERNALIZE
+
+// RUN: %clang_cc1 -x hip -triple amdgcn-amd-amdhsa -target-cpu gfx1101 -fcuda-is-device \
+// RUN:   -mlink-builtin-bitcode %t.bc -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,INTERNALIZE
+
+// Check the case where no internalization is performed
+// RUN: %clang_cc1 -x hip -triple amdgcn-amd-amdhsa -target-cpu gfx803 \
+// RUN:   -fcuda-is-device -mlink-bitcode-file %t.bc -emit-llvm %s -o -  | FileCheck %s --check-prefixes=CHECK,NOINTERNALIZE
+
+// Check the case where no internalization is performed
+// RUN: %clang_cc1 -x hip -triple amdgcn-amd-amdhsa -target-cpu gfx1101 \
+// RUN:   -fcuda-is-device -mlink-bitcode-file %t.bc -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,NOINTERNALIZE
+
+
+// CHECK: define {{.*}} i64 @do_intrin_stuff() #[[ATTR:[0-9]+]]
+// INTERNALIZE: attributes #[[ATTR]] = {{.*}} "target-cpu"="gfx{{.*}}" "target-features"="+gfx11-insts"
+// NOINTERNALIZE: attributes #[[ATTR]] = {{.*}} "target-features"="+gfx11-insts"
+
+#define __device__ __attribute__((device))
+#define __global__ __attribute__((global))
+
+typedef unsigned long ulong;
+
+extern "C" {
+__device__ ulong do_intrin_stuff(void);
+
+__global__ void kernel_f16(ulong* out) {
+*out = do_intrin_stuff();
+  }
+}
Index: clang/test/CodeGenCUDA/link-builtin-bitcode-denormal-fp-mode.cu
===
--- clang/test/CodeGenCUDA/link-builtin-bitcode-denormal-fp-mode.cu
+++ clang/test/CodeGenCUDA/link-builtin-bitcode-denormal-fp-mode.cu
@@ -132,24 +132,32 @@
 
 // Default mode relies on the implicit check-not for the denormal-fp-math.
 
-// PSZ: #[[$KERNELATTR]] = { {{.*}} "denormal-fp-math"="preserve-sign,preserve-sign" {{.*}} "target-cpu"="gfx803" {{.*}} }
-// PSZ: #[[$FUNCATTR]] = { {{.*}} "denormal-fp-math-f32"="preserve-sign,preserve-sign" {{.*}} "target-cpu"="gfx803" {{.*}} }
-// PSZ: #[[$WEAK_FUNCATTR]] = { {{.*}} "denormal-fp-math-f32"="preserve-sign,preserve-sign" {{.*}} "target-cpu"="gfx803" {{.*}} }
+// PSZ: #[[$KERNELATTR]] = { {{.*}} "denormal-fp-math"="preserve-sign,preserve-sign"
+// PSZ-SAME: "target-cpu"="gfx803"
+// PSZ: #[[$FUNCATTR]] = { {{.*}} "denormal-fp-math-f32"="preserve-sign,preserve-sign"
+// PSZ-SAME: "target-cpu"="gfx803"
+// PSZ: #[[$WEAK_FUNCATTR]] = { {{.*}} "denormal-fp-math-f32"="preserve-sign,preserve-sign"
+// PSZ-SAME: "target-cpu"="gfx803"
 
 // FIXME: Should check-not "denormal-fp-math" within the line
-// IEEEF64-PSZF32: #[[$KERNELATTR]] = { {{.*}} "denormal-fp-math-f32"="preserve-sign,preserve-sign" {{.*}} "target-cpu"="gfx803" {{.*}} }
-// IEEEF64-PSZF32: #[[$FUNCATTR]] = { {{.*}} "denormal-fp-math-f32"="preserve-sign,preserve-sign" {{.*}} "target-cpu"="gfx803" {{.*}} }
-// IEEEF64-PSZF32: #[[$WEAK_FUNCATTR]] = { {{.*}} "denormal-fp-math-f32"="preserve-sign,preserve-sign" {{.*}} "target-cpu"="gfx803" {{.*}} }
+// IEEEF64-PSZF32: #[[$KERNELATTR]] = { {{.*}} "denormal-fp-math-f32"="preserve-sign,preserve-sign"
+// IEEEF64-PSZF32-SAME

[clang] 23431b5 - [clang][CodeGen] Fix GPU-specific attributes being dropped by bitcode linking

2023-06-07 Thread via cfe-commits

Author: pvanhout
Date: 2023-06-07T15:51:52+02:00
New Revision: 23431b52460328c554ad244fd7b50ecb751cec31

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

LOG: [clang][CodeGen] Fix GPU-specific attributes being dropped by bitcode 
linking

Device libs make use of patterns like this:
```
__attribute__((target("gfx11-insts")))
static unsigned do_intrin_stuff(void)
{
  return __builtin_amdgcn_s_sendmsg_rtnl(0x0);
}
```
For functions that are assumed to be eliminated if the currennt GPU target 
doesn't support them.
At O0 such functions aren't eliminated by common optimizations but often by 
AMDGPURemoveIncompatibleFunctions instead, which sees the "+gfx11-insts" 
attribute on, say, GFX9 and knows it's not valid, so it removes the function.

D142907 accidentally made it so such attributes were dropped during bitcode 
linking, making it impossible for RemoveIncompatibleFunctions to catch the 
functions and causing ISel to catch fire eventually.

This fixes the issue and adds a new test to ensure we don't accidentally fall 
into this trap again.

Fixes SWDEV-403642

Reviewed By: arsenm, yaxunl

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

Added: 
clang/test/CodeGenCUDA/Inputs/ocml-sample-target-attrs.cl
clang/test/CodeGenCUDA/link-builtin-bitcode-gpu-attrs-preserved.cu

Modified: 
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/test/CodeGenCUDA/link-builtin-bitcode-denormal-fp-mode.cu

Removed: 




diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index b6b04fca66820..eb45e82fe8256 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -2025,7 +2025,8 @@ void 
CodeGenModule::mergeDefaultFunctionDefinitionAttributes(
   llvm::AttrBuilder FuncAttrs(F.getContext());
   getTrivialDefaultFunctionAttributes(F.getName(), F.hasOptNone(),
   /*AttrOnCallSite=*/false, FuncAttrs);
-  GetCPUAndFeaturesAttributes(GlobalDecl(), FuncAttrs);
+  GetCPUAndFeaturesAttributes(GlobalDecl(), FuncAttrs,
+  /*AddTargetFeatures=*/false);
 
   if (!WillInternalize && F.isInterposable()) {
 // Do not promote "dynamic" denormal-fp-math to this translation unit's

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 49b670487749a..41ceebfba404a 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -2226,7 +2226,8 @@ void CodeGenModule::SetCommonAttributes(GlobalDecl GD, 
llvm::GlobalValue *GV) {
 }
 
 bool CodeGenModule::GetCPUAndFeaturesAttributes(GlobalDecl GD,
-llvm::AttrBuilder &Attrs) {
+llvm::AttrBuilder &Attrs,
+bool SetTargetFeatures) {
   // Add target-cpu and target-features attributes to functions. If
   // we have a decl for the function and it has a target attribute then
   // parse that and add it to the feature set.
@@ -2286,7 +2287,7 @@ bool 
CodeGenModule::GetCPUAndFeaturesAttributes(GlobalDecl GD,
 Attrs.addAttribute("tune-cpu", TuneCPU);
 AddedAttr = true;
   }
-  if (!Features.empty()) {
+  if (!Features.empty() && SetTargetFeatures) {
 llvm::sort(Features);
 Attrs.addAttribute("target-features", llvm::join(Features, ","));
 AddedAttr = true;

diff  --git a/clang/lib/CodeGen/CodeGenModule.h 
b/clang/lib/CodeGen/CodeGenModule.h
index e4ef209195275..43d61b40f76b4 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -1583,7 +1583,8 @@ class CodeGenModule : public CodeGenTypeCache {
 ForDefinition_t IsForDefinition = NotForDefinition);
 
   bool GetCPUAndFeaturesAttributes(GlobalDecl GD,
-   llvm::AttrBuilder &AttrBuilder);
+   llvm::AttrBuilder &AttrBuilder,
+   bool SetTargetFeatures = true);
   void setNonAliasAttributes(GlobalDecl GD, llvm::GlobalObject *GO);
 
   /// Set function attributes for a function declaration.

diff  --git a/clang/test/CodeGenCUDA/Inputs/ocml-sample-target-attrs.cl 
b/clang/test/CodeGenCUDA/Inputs/ocml-sample-target-attrs.cl
new file mode 100644
index 0..b26595da4e9af
--- /dev/null
+++ b/clang/test/CodeGenCUDA/Inputs/ocml-sample-target-attrs.cl
@@ -0,0 +1,7 @@
+typedef unsigned long ulong;
+
+__attribute__((target("gfx11-insts")))
+ulong do_intrin_stuff(void)
+{
+  return __builtin_amdgcn_s_sendmsg_rtnl(0x0);
+}

diff  --git a/clang/test/CodeGenCUDA/link-builtin-bitcode-denormal-fp-mode.cu 
b/clang/test/CodeGenCUDA/link-builtin-bitcode-denormal-fp-mode.cu
index 

[PATCH] D152109: Update clang-repl documentation

2023-06-07 Thread Krishna Narayanan via Phabricator via cfe-commits
Krishna-13-cyber updated this revision to Diff 529287.
Krishna-13-cyber added a comment.

- Update with addressing the comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152109

Files:
  clang/docs/ClangRepl.rst

Index: clang/docs/ClangRepl.rst
===
--- clang/docs/ClangRepl.rst
+++ clang/docs/ClangRepl.rst
@@ -16,22 +16,6 @@
 of Cling upstream, making them useful and available to a broader audience.
 
 
-
-Clang-Repl Usage
-
-
-
-.. code-block:: text
-
-  clang-repl> #include 
-  clang-repl> int f() { std::cout << "Hello Interpreted World!\n"; return 0; }
-  clang-repl> auto r = f();
-  // Prints Hello Interpreted World!
-
-Note that the implementation is not complete and highly experimental. We do
-not yet support statements on the global scope, for example.
-
-
 Clang-Repl Basic Data Flow
 ==
 
@@ -63,14 +47,135 @@
 
 8. The machine code is then executed.
 
+===
+Build Instructions:
+===
+
+
+.. code-block:: console
+
+
+   $ cd llvm-project
+   $ mkdir build
+   $ cd build
+   $ cmake -DCMAKE_BUILD_TYPE=RelWithDebInfo -DLLVM_ENABLE_PROJECTS=clang -G "Unix Makefiles" ../llvm
+
+**Note here**, above RelWithDebInfo - Debug / Release
+
+.. code-block:: console
+
+   cmake --build . --target clang clang-repl -j n
+  OR
+   cmake --build . --target clang clang-repl
+
+**Clang-repl** is built under llvm-project/build/bin. Proceed into the directory **llvm-project/build/bin**
+
+.. code-block:: console
+
+   ./clang-repl
+   clang-repl>
+
+
+
+Clang-Repl-Usage
+
+
+**Clang-Repl** is an interactive C++ interpreter that allows for incremental
+compilation. It supports interactive programming for C++ in a
+read-evaluate-print-loop (REPL) style. It uses Clang as a library to compile the
+high level programming language into LLVM IR. Then the LLVM IR is executed by
+the LLVM just-in-time (JIT) infrastructure.
+
+
+Basic:
+==
+
+
+.. code-block:: text
+
+  clang-repl> #include 
+  clang-repl> int f() { std::cout << "Hello Interpreted World!\n"; return 0; }
+  clang-repl> auto r = f();
+   // Prints Hello Interpreted World!
+
+.. code-block:: text
+
+   clang-repl> #include
+   clang-repl> using namespace std;
+   clang-repl> std::cout << "Welcome to CLANG-REPL" << std::endl;
+   Welcome to CLANG-REPL
+   // Prints Welcome to CLANG-REPL
+
+
+Function Definitions and Calls:
+===
+
+
+.. code-block:: text
+
+   clang-repl> #include
+   clang-repl> int sum(int a, int b){ return a+b; };
+   clang-repl> int c = sum(9,10);
+   clang-repl> std::cout << c;
+   19clang-repl>
+
+Iterative Structures:
+=
+
+
+.. code-block:: text
+
+   clang-repl> #include
+   clang-repl> for (int i = 0;i < 3;i++){ std::cout << i << std::endl;}
+   0
+   1
+   2
+   clang-repl> while(i < 7){ i++; std::cout << i << std::endl;}
+   4
+   5
+   6
+   7
+
+Classes and Structures:
+===
+
+
+.. code-block:: text
+
+   clang-repl> #include
+   clang-repl> class Rectangle {int width, height; public: void set_values (int,int);int area() {return width*height;}};
+   clang-repl>  void Rectangle::set_values (int x, int y) { width = x;height = y;}
+   clang-repl> int main () { Rectangle rect;rect.set_values (3,4); std::cout << "area: " << rect.area();return 0;}
+   clang-repl> main();
+   area: 12clang-repl>
+
+Lamdas:
+===
+
+
+.. code-block:: text
+
+   clang-repl> #include
+   clang-repl> using namespace std;
+   clang-repl> auto welcome = []()  { std::cout << "Welcome to REPL" << std::endl;};
+   clang-repl> welcome();
+   Welcome to REPL
+
+Closure or Termination:
+===
+
+
+.. code-block:: text
+
+   clang-repl>%quit
+
 
-Just like Clang, Clang-Repl can be integrated in existing applications as a
-library (via using the clangInterpreter library). This turning your C++ compiler
-into a service which incrementally can consume and execute code. The
-**Compiler as A Service** (**CaaS**) concept helps supporting move advanced use
-cases such as template instantiations on demand and automatic language
-interoperability. It also helps static languages such as C/C++ become apt for
-data science.
+Just like Clang, Clang-Repl can be integrated in existing applications as a library 
+(using the clangInterpreter library). This turns your C++ compiler into a service that 
+can incrementally consume and execute code. The **Compiler as A Service** (**CaaS**) 
+concept helps support advanced use cases such as template instantiations on demand and 
+automatic language interoperability. It also helps static languages such as C/C++ become 
+apt for data science.
 
 
 Related Reading
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman

[PATCH] D152021: [clang][AIX] Fix Overly Strick LTO Option Checking against `data-sections` when `mxcoff-roptr` is in Effect

2023-06-07 Thread Qiongsi Wu via Phabricator via cfe-commits
qiongsiwu1 added a comment.

Ping for review. Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152021

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


[PATCH] D152051: libclang-cpp: Add external visibility attribute to all classes

2023-06-07 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd added a comment.

In D152051#4392612 , @tstellar wrote:

> I was not sure what to do with inline functions and also functions that were 
> implemented in the headers, so I did not add the LLVM_EXTERNAL_VISIBILITY 
> macro to most of those functions.

I think that inline functions should have the attribute applied as well.  That 
should be the equivalent of `-fvisibility-inlines-hidden` and should allow this 
to be useable for windows as well.

Please do not use LLVM_EXTERNAL_VISIBILITY but rather introduce a new macro 
(this will prevent the use on Windows).

I did write a tool to help with this: https://GitHub.com/compnerd/ids


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152051

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


[clang] e60b30d - Reland "D144999 [MC][MachO]Only emits compact-unwind format for "canonical" personality symbols. For the rest, use DWARFs."

2023-06-07 Thread Vy Nguyen via cfe-commits

Author: Vy Nguyen
Date: 2023-06-07T10:03:50-04:00
New Revision: e60b30d5e3878e7d91f8872ec4c4dca00d4a2dfc

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

LOG: Reland "D144999 [MC][MachO]Only emits compact-unwind format for 
"canonical" personality symbols. For the rest, use DWARFs."

Reasons for rolling forward:
- the crash reported from Chromium was fixed in D151824 (not related to 
this patch at all)
- since D152824 was committed, it should now be safe to roll this forward.

New change:
- add an additional _ in name check

This reverts commit 4980eead4d0b4666d53dad07afb091375b3a13a0.

Added: 


Modified: 
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/femit-dwarf-unwind.c
clang/test/Driver/femit-dwarf-unwind.s
clang/tools/driver/cc1as_main.cpp
lld/MachO/UnwindInfoSection.cpp
lld/test/MachO/Inputs/eh-frame-x86_64-r.o
lld/test/MachO/compact-unwind-both-local-and-dylib-personality.s
lld/test/MachO/compact-unwind-generated.test
lld/test/MachO/compact-unwind-lsda-folding.s
lld/test/MachO/compact-unwind-stack-ind.s
lld/test/MachO/compact-unwind.s
lld/test/MachO/eh-frame-personality-dedup.s
lld/test/MachO/eh-frame.s
lld/test/MachO/icf-only-lsda-folded.s
lld/test/MachO/icf.s
lld/test/MachO/invalid/compact-unwind-bad-reloc.s
lld/test/MachO/invalid/compact-unwind-personalities.s
llvm/include/llvm/MC/MCAsmBackend.h
llvm/include/llvm/MC/MCContext.h
llvm/include/llvm/MC/MCTargetOptions.h
llvm/include/llvm/MC/MCTargetOptionsCommandFlags.h
llvm/lib/MC/MCAsmBackend.cpp
llvm/lib/MC/MCContext.cpp
llvm/lib/MC/MCStreamer.cpp
llvm/lib/MC/MCTargetOptionsCommandFlags.cpp
llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackendDarwin.h
llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp
llvm/test/CodeGen/X86/2014-08-29-CompactUnwind.ll
llvm/test/CodeGen/X86/compact-unwind.ll
llvm/test/MC/AArch64/arm64-leaf-compact-unwind.s
llvm/test/MC/MachO/AArch64/emit-dwarf-unwind.s
llvm/test/MC/MachO/ARM/compact-unwind-armv7k.s
llvm/test/MC/X86/compact-unwind.s

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.def 
b/clang/include/clang/Basic/CodeGenOptions.def
index 5752c6e285b98..53d92c4c76673 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -114,6 +114,9 @@ CODEGENOPT(StackSizeSection  , 1, 0) ///< Set when 
-fstack-size-section is enabl
 CODEGENOPT(ForceDwarfFrameSection , 1, 0) ///< Set when -fforce-dwarf-frame is
   ///< enabled.
 
+///< Set when -femit-compact-unwind-non-canonical is enabled.
+CODEGENOPT(EmitCompactUnwindNonCanonical, 1, 0)
+
 ///< Set when -femit-dwarf-unwind is passed.
 ENUM_CODEGENOPT(EmitDwarfUnwind, llvm::EmitDwarfUnwindType, 2,
 llvm::EmitDwarfUnwindType::Default)

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 3326db0791aaf..7fad5b27fdb6e 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -3285,6 +3285,9 @@ def femit_dwarf_unwind_EQ : Joined<["-"], 
"femit-dwarf-unwind=">,
   NormalizedValues<["Always", "NoCompactUnwind", "Default"]>,
   NormalizedValuesScope<"llvm::EmitDwarfUnwindType">,
   MarshallingInfoEnum, "Default">;
+defm emit_compact_unwind_non_canonical : 
BoolFOption<"emit-compact-unwind-non-canonical",
+  CodeGenOpts<"EmitCompactUnwindNonCanonical">, DefaultFalse,
+  PosFlag, 
NegFlag>;
 def g_Flag : Flag<["-"], "g">, Group,
 Flags<[CoreOption,FlangOption]>, HelpText<"Generate source-level debug 
information">;
 def gline_tables_only : Flag<["-"], "gline-tables-only">, Group,

diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index d4498ebaf8dea..c736f38e01f65 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -454,6 +454,8 @@ static bool initTargetOptions(DiagnosticsEngine &Diags,
 
   Options.MCOptions.SplitDwarfFile = CodeGenOpts.SplitDwarfFile;
   Options.MCOptions.EmitDwarfUnwind = CodeGenOpts.getEmitDwarfUnwind();
+  Options.MCOptions.EmitCompactUnwindNonCanonical =
+  CodeGenOpts.EmitCompactUnwindNonCanonical;
   Options.MCOptions.MCRelaxAll = CodeGenOpts.RelaxAll;
   Options.MCOptions.MCSaveTempLabels = CodeGenOpts.SaveTempLabels;
   Options.MCOptions.MCUseDwarfDirectory =

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
inde

[PATCH] D152051: libclang-cpp: Add external visibility attribute to all classes

2023-06-07 Thread Tom Stellard via Phabricator via cfe-commits
tstellar added a comment.

> ! In D152051#4403167 , @compnerd 
> wrote:
>  Please do not use LLVM_EXTERNAL_VISIBILITY but rather introduce a new macro 
> (this will prevent the use on Windows).

OK, so should I create a clang specific macro for this?  And should it behave 
the same as LLVM_EXTERNAL_VISIBILITLY or do I need to have it expand to 
different values depending on the OS?

> I did write a tool to help with this: https://GitHub.com/compnerd/ids

OK, thanks, I will check this out.




Comment at: clang/include/clang/Format/Format.h:4532
 /// Returns ``true`` if the Style has been set.
-bool getPredefinedStyle(StringRef Name, FormatStyle::LanguageKind Language,
+LLVM_EXTERNAL_VISIBILITY bool getPredefinedStyle(StringRef Name, 
FormatStyle::LanguageKind Language,
 FormatStyle *Style);

HazardyKnusperkeks wrote:
> This doesn't look formatted.
I ran git-clang-format on the patch and it introduced a lot of unrelated 
changes, so I ended up dropping that part of the patch.  I can go through and 
manually discard some of the more intrusive format changes.  Any suggestions on 
which kind of formatting changes to keep and which ones to ignore?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152051

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


[PATCH] D139541: Fix parameter name in Sema::addInitCapture to ByRef.

2023-06-07 Thread Jens Massberg via Phabricator via cfe-commits
massberg updated this revision to Diff 529293.
massberg added a comment.

Fix formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139541

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaLambda.cpp


Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -798,12 +798,11 @@
   return NewVD;
 }
 
-void Sema::addInitCapture(LambdaScopeInfo *LSI, VarDecl *Var,
-  bool isReferenceType) {
+void Sema::addInitCapture(LambdaScopeInfo *LSI, VarDecl *Var, bool ByRef) {
   assert(Var->isInitCapture() && "init capture flag should be set");
-  LSI->addCapture(Var, /*isBlock*/ false, isReferenceType,
-  /*isNested*/ false, Var->getLocation(), SourceLocation(),
-  Var->getType(), /*Invalid*/ false);
+   LSI->addCapture(Var, /*isBlock=*/ false, ByRef,
+   /*isNested=*/ false, Var->getLocation(), SourceLocation(),
+   Var->getType(), /*Invalid=*/ false);
 }
 
 // Unlike getCurLambda, getCurrentLambdaScopeUnsafe doesn't
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -7171,8 +7171,7 @@
   IdentifierInfo *Id, unsigned InitStyle, Expr *Init, DeclContext 
*DeclCtx);
 
   /// Add an init-capture to a lambda scope.
-  void addInitCapture(sema::LambdaScopeInfo *LSI, VarDecl *Var,
-  bool isReferenceType);
+  void addInitCapture(sema::LambdaScopeInfo *LSI, VarDecl *Var, bool ByRef);
 
   /// Note that we have finished the explicit captures for the
   /// given lambda.


Index: clang/lib/Sema/SemaLambda.cpp
===
--- clang/lib/Sema/SemaLambda.cpp
+++ clang/lib/Sema/SemaLambda.cpp
@@ -798,12 +798,11 @@
   return NewVD;
 }
 
-void Sema::addInitCapture(LambdaScopeInfo *LSI, VarDecl *Var,
-  bool isReferenceType) {
+void Sema::addInitCapture(LambdaScopeInfo *LSI, VarDecl *Var, bool ByRef) {
   assert(Var->isInitCapture() && "init capture flag should be set");
-  LSI->addCapture(Var, /*isBlock*/ false, isReferenceType,
-  /*isNested*/ false, Var->getLocation(), SourceLocation(),
-  Var->getType(), /*Invalid*/ false);
+   LSI->addCapture(Var, /*isBlock=*/ false, ByRef,
+   /*isNested=*/ false, Var->getLocation(), SourceLocation(),
+   Var->getType(), /*Invalid=*/ false);
 }
 
 // Unlike getCurLambda, getCurrentLambdaScopeUnsafe doesn't
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -7171,8 +7171,7 @@
   IdentifierInfo *Id, unsigned InitStyle, Expr *Init, DeclContext *DeclCtx);
 
   /// Add an init-capture to a lambda scope.
-  void addInitCapture(sema::LambdaScopeInfo *LSI, VarDecl *Var,
-  bool isReferenceType);
+  void addInitCapture(sema::LambdaScopeInfo *LSI, VarDecl *Var, bool ByRef);
 
   /// Note that we have finished the explicit captures for the
   /// given lambda.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D151594: [clang-tidy] Optimize misc-confusable-identifiers

2023-06-07 Thread serge via Phabricator via cfe-commits
serge-sans-paille accepted this revision.
serge-sans-paille added a comment.
This revision is now accepted and ready to land.

LGTM with a minor nit. Thanks for the optimization!




Comment at: clang-tools-extra/clang-tidy/misc/ConfusableIdentifierCheck.cpp:124
+if (ND0->getAccess() != AS_private && isMemberOf(DC0, DC1))
+  return true;
+  }

I <3 this simplification. It's more difficult to understand though, could you 
add a small content along those lines: `if any of the declaration is a 
non-private member of the other declaration, it's shadowed by the former`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151594

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


[clang] 4418434 - [DebugMetadata] Simplify handling subprogram's retainedNodes field. NFCI (1/7)

2023-06-07 Thread Vladislav Dzhidzhoev via cfe-commits

Author: Kristina Bessonova
Date: 2023-06-07T16:43:12+02:00
New Revision: 4418434c6de7a861e241ba2448ea4a12080cf08f

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

LOG: [DebugMetadata] Simplify handling subprogram's retainedNodes field. NFCI 
(1/7)

RFC 
https://discourse.llvm.org/t/rfc-dwarfdebug-fix-and-improve-handling-imported-entities-types-and-static-local-in-subprogram-and-lexical-block-scopes/68544

Currently, `retainedNodes` tracks function-local variables and labels.
To support function-local import, types and static variables (which are globals
in LLVM IR), subsequent patches use the same field. So this patch makes
preliminary refactoring of the code tracking local entities to apply future
functional changes lucidly and cleanly.

No functional changes intended.

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

Added: 


Modified: 
clang/test/CodeGen/attr-btf_tag-disubprogram-callsite.c
clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
clang/test/CodeGenCXX/debug-info-cxx1y.cpp
clang/test/CodeGenCXX/debug-info-template.cpp
clang/test/CodeGenObjC/debug-info-category.m
llvm/include/llvm/IR/DIBuilder.h
llvm/include/llvm/IR/DebugInfoMetadata.h
llvm/lib/IR/DIBuilder.cpp

Removed: 




diff  --git a/clang/test/CodeGen/attr-btf_tag-disubprogram-callsite.c 
b/clang/test/CodeGen/attr-btf_tag-disubprogram-callsite.c
index e3bfd71a56aad..5bbb05e6b8098 100644
--- a/clang/test/CodeGen/attr-btf_tag-disubprogram-callsite.c
+++ b/clang/test/CodeGen/attr-btf_tag-disubprogram-callsite.c
@@ -13,7 +13,7 @@ int foo2(struct t1 *arg) {
   return foo(arg);
 }
 
-// CHECK: ![[#]] = !DISubprogram(name: "foo", scope: ![[#]], file: ![[#]], 
line: [[#]], type: ![[#]], flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, 
retainedNodes: ![[#]], annotations: ![[ANNOT:[0-9]+]])
+// CHECK: ![[#]] = !DISubprogram(name: "foo", scope: ![[#]], file: ![[#]], 
line: [[#]], type: ![[#]], flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, 
annotations: ![[ANNOT:[0-9]+]])
 // CHECK: ![[ANNOT]] = !{![[TAG1:[0-9]+]], ![[TAG2:[0-9]+]]}
 // CHECK: ![[TAG1]] = !{!"btf_decl_tag", !"tag1"}
 // CHECK: ![[TAG2]] = !{!"btf_decl_tag", !"tag2"}

diff  --git a/clang/test/CodeGenCXX/aix-static-init-debug-info.cpp 
b/clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
index a35ca27e10d2a..6453e4379ae9a 100644
--- a/clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
+++ b/clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
@@ -52,15 +52,15 @@ X v;
 // CHECK:   ret void
 // CHECK: }
 
-// CHECK: ![[DBGVAR16]] = distinct !DISubprogram(name: 
"__cxx_global_var_init", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, type: 
!{{[0-9]+}}, flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | 
DISPFlagDefinition, unit: !{{[0-9]+}}, retainedNodes: !{{[0-9]+}})
+// CHECK: ![[DBGVAR16]] = distinct !DISubprogram(name: 
"__cxx_global_var_init", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, type: 
!{{[0-9]+}}, flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | 
DISPFlagDefinition, unit: !{{[0-9]+}})
 // CHECK: ![[DBGVAR19]] = !DILocation(line: 14, column: 3, scope: 
![[DBGVAR16]])
 // CHECK: ![[DBGVAR19b]] = !DILocation(line: 0, scope: ![[DBGVAR16]])
-// CHECK: ![[DBGVAR20]] = distinct !DISubprogram(name: "__dtor_v", scope: 
!{{[0-9]+}}, file: !{{[0-9]+}}, line: 14, type: !{{[0-9]+}}, scopeLine: 14, 
flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, 
unit: !{{[0-9]+}}, retainedNodes: !{{[0-9]+}})
+// CHECK: ![[DBGVAR20]] = distinct !DISubprogram(name: "__dtor_v", scope: 
!{{[0-9]+}}, file: !{{[0-9]+}}, line: 14, type: !{{[0-9]+}}, scopeLine: 14, 
flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, 
unit: !{{[0-9]+}})
 // CHECK: ![[DBGVAR21b]] = !DILocation(line: 0, scope: ![[DBGVAR20]])
 // CHECK: ![[DBGVAR21]] = !DILocation(line: 14, column: 3, scope: 
![[DBGVAR20]])
-// CHECK: ![[DBGVAR22]] = distinct !DISubprogram(linkageName: "__finalize_v", 
scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 14, type: !{{[0-9]+}}, scopeLine: 
14, flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, 
unit: !{{[0-9]+}}, retainedNodes: !{{[0-9]+}})
+// CHECK: ![[DBGVAR22]] = distinct !DISubprogram(linkageName: "__finalize_v", 
scope: !{{[0-9]+}}, file: !{{[0-9]+}}, line: 14, type: !{{[0-9]+}}, scopeLine: 
14, flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, 
unit: !{{[0-9]+}})
 // CHECK: ![[DBGVAR24]] = !DILocation(line: 14, column: 3, scope: 
![[DBGVAR22]])
-// CHECK: ![[DBGVAR25]] = distinct !DISubprogram(linkageName: 
"_GLOBAL__sub_I__", scope: !{{[0-9]+}}, file: !{{[0-9]+}}, type: !{{[0-9]+}}, 
flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition, 
unit: !{{[0-9]+}}, retainedNodes: !{{[0-9]+}})
+// CH

[PATCH] D143984: [DebugMetadata] Simplify handling subprogram's retainedNodes field. NFCI (1/7)

2023-06-07 Thread Vladislav Dzhidzhoev 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 rG4418434c6de7: [DebugMetadata] Simplify handling 
subprogram's retainedNodes field. NFCI (1/7) (authored by krisb, committed 
by dzhidzhoev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143984

Files:
  clang/test/CodeGen/attr-btf_tag-disubprogram-callsite.c
  clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
  clang/test/CodeGenCXX/debug-info-cxx1y.cpp
  clang/test/CodeGenCXX/debug-info-template.cpp
  clang/test/CodeGenObjC/debug-info-category.m
  llvm/include/llvm/IR/DIBuilder.h
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/lib/IR/DIBuilder.cpp

Index: llvm/lib/IR/DIBuilder.cpp
===
--- llvm/lib/IR/DIBuilder.cpp
+++ llvm/lib/IR/DIBuilder.cpp
@@ -52,23 +52,11 @@
 }
 
 void DIBuilder::finalizeSubprogram(DISubprogram *SP) {
-  MDTuple *Temp = SP->getRetainedNodes().get();
-  if (!Temp || !Temp->isTemporary())
-return;
-
-  SmallVector RetainedNodes;
-
-  auto PV = PreservedVariables.find(SP);
-  if (PV != PreservedVariables.end())
-RetainedNodes.append(PV->second.begin(), PV->second.end());
-
-  auto PL = PreservedLabels.find(SP);
-  if (PL != PreservedLabels.end())
-RetainedNodes.append(PL->second.begin(), PL->second.end());
-
-  DINodeArray Node = getOrCreateArray(RetainedNodes);
-
-  TempMDTuple(Temp)->replaceAllUsesWith(Node.get());
+  auto PN = SubprogramTrackedNodes.find(SP);
+  if (PN != SubprogramTrackedNodes.end())
+SP->replaceRetainedNodes(
+MDTuple::get(VMContext, SmallVector(PN->second.begin(),
+PN->second.end(;
 }
 
 void DIBuilder::finalize() {
@@ -766,26 +754,20 @@
 
 static DILocalVariable *createLocalVariable(
 LLVMContext &VMContext,
-DenseMap> &PreservedVariables,
-DIScope *Scope, StringRef Name, unsigned ArgNo, DIFile *File,
+SmallVectorImpl &PreservedNodes,
+DIScope *Context, StringRef Name, unsigned ArgNo, DIFile *File,
 unsigned LineNo, DIType *Ty, bool AlwaysPreserve, DINode::DIFlags Flags,
 uint32_t AlignInBits, DINodeArray Annotations = nullptr) {
-  // FIXME: Why getNonCompileUnitScope()?
-  // FIXME: Why is "!Context" okay here?
   // FIXME: Why doesn't this check for a subprogram or lexical block (AFAICT
   // the only valid scopes)?
-  DIScope *Context = getNonCompileUnitScope(Scope);
-
-  auto *Node = DILocalVariable::get(
-  VMContext, cast_or_null(Context), Name, File, LineNo, Ty,
-  ArgNo, Flags, AlignInBits, Annotations);
+  auto *Scope = cast(Context);
+  auto *Node = DILocalVariable::get(VMContext, Scope, Name, File, LineNo, Ty,
+ArgNo, Flags, AlignInBits, Annotations);
   if (AlwaysPreserve) {
 // The optimizer may remove local variables. If there is an interest
 // to preserve variable info in such situation then stash it in a
 // named mdnode.
-DISubprogram *Fn = getDISubprogram(Scope);
-assert(Fn && "Missing subprogram for local variable");
-PreservedVariables[Fn].emplace_back(Node);
+PreservedNodes.emplace_back(Node);
   }
   return Node;
 }
@@ -795,9 +777,11 @@
DIType *Ty, bool AlwaysPreserve,
DINode::DIFlags Flags,
uint32_t AlignInBits) {
-  return createLocalVariable(VMContext, PreservedVariables, Scope, Name,
- /* ArgNo */ 0, File, LineNo, Ty, AlwaysPreserve,
- Flags, AlignInBits);
+  assert(Scope && isa(Scope) &&
+ "Unexpected scope for a local variable.");
+  return createLocalVariable(
+  VMContext, getSubprogramNodesTrackingVector(Scope), Scope, Name,
+  /* ArgNo */ 0, File, LineNo, Ty, AlwaysPreserve, Flags, AlignInBits);
 }
 
 DILocalVariable *DIBuilder::createParameterVariable(
@@ -805,25 +789,23 @@
 unsigned LineNo, DIType *Ty, bool AlwaysPreserve, DINode::DIFlags Flags,
 DINodeArray Annotations) {
   assert(ArgNo && "Expected non-zero argument number for parameter");
-  return createLocalVariable(VMContext, PreservedVariables, Scope, Name, ArgNo,
- File, LineNo, Ty, AlwaysPreserve, Flags,
- /*AlignInBits=*/0, Annotations);
+  assert(Scope && isa(Scope) &&
+ "Unexpected scope for a local variable.");
+  return createLocalVariable(
+  VMContext, getSubprogramNodesTrackingVector(Scope), Scope, Name, ArgNo,
+  File, LineNo, Ty, AlwaysPreserve, Flags, /*AlignInBits=*/0, Annotations);
 }
 
-DILabel *DIBuilder::createLabel(DIScope *Scope, StringRef Name, DIFile *File,
-unsigned LineNo, bool AlwaysPreserve) {
-  DIScope *Context = getNonCompileUnitScope(

[PATCH] D99201: [HIP] Diagnose unaligned atomic for amdgpu

2023-06-07 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl marked 2 inline comments as done.
yaxunl added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7215
+// warnings as errors.
+CmdArgs.push_back("-Werror=atomic-alignment");
   }

tra wrote:
> Should it be done from `HIPAMDToolChain::addClangWarningOptions` ?
> 
> That's where Darwin does similar propotion from a waring to an error.
will do. Since this option is shared between all AMDGPU toolchains, I will add 
it to AMDGPUToolChain::addClangWarningOptions and let the derived toolchains 
call it.



Comment at: clang/test/Driver/hip-options.hip:144
+
+// RUN: %clang -### -target x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --cuda-gpu-arch=gfx906 %s 2>&1 | FileCheck -check-prefix=WARN-ATOMIC 
%s

MaskRay wrote:
> Prefer `--target=` to deprecated (since 3.x) `-target `
will fix


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

https://reviews.llvm.org/D99201

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


[PATCH] D152269: [StaticAnalyzer] Fix false negative on NilArgChecker when creating literal object

2023-06-07 Thread tripleCC via Phabricator via cfe-commits
tripleCC added a comment.

In D152269#4402593 , @steakhal wrote:

> I had some improvement opportunities in mind scattered, so I decided to do 
> them, and here is how the diff looks for me now: F27853795: 
> recommendation.patch 
>
> Basically, I reworked the two branches a bit to express the intent more 
> strait forward.
> I also enable all `osx.cocoa` checkers, as this file is all about objc stuff 
> anyway - this also meant to diagnose two non-fatal leak errors, which are not 
> tied to this patch otherwise.
>
> I'm also testing that the "assumption" after the objc call thingie (message 
> passing?) the pointer is assumed to be "nonnull". For this, I'm using the 
> `eagerly-assume=false` to ensure we don't split null and non-null states 
> eagerly when we encounter the `p == nil` inside `clang_analyzer_eval()` call 
> argument.
>
> I think all my changes are aligned with your intent, right?
>
> One more thing I was thinking of was to make this ObjC null checking checker 
> depend on the "nullability" checker package, but it turns out we can only 
> depend on individual checkers as of now. I tried some ideas there, e.g. 
> depending on the base checker of that package but it didn't work, so I 
> dropped the idea. (`clang/include/clang/StaticAnalyzer/Checkers/Checkers.td`)
>
> Do you plan to contribute more ObjC-related fixes in the future?
> Please note that I have absolutely no experience with ObjC stuff.
>
> And I also wanted to thank you for the high-quality patches you submitted so 
> far!

Yes, your changes are aligned with my intent. It seems like you have made 
excellent optimizations to this patch. 
To eliminate the following two warnings, I add the `-fobjc-arc` compilation 
option for NSContainers test .

  +  MyView *view = b ? [[MyView alloc] init] : 0; // expected-warning 
{{Potential leak of an object stored into 'view'}}
  +  NSMutableArray *subviews = [[view subviews] mutableCopy]; // 
expected-warning {{Potential leak of an object stored into 'subviews'}}

I will continue to contribute more ObjC-related issue fixes in the future, and 
currently, my work is also related to this. 
Thank you very much for your review. Do you mind if I merge your 
recommendations?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152269

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


[PATCH] D99201: [AMDGPU] Diagnose unaligned atomic for amdgpu

2023-06-07 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl updated this revision to Diff 529301.
yaxunl marked 2 inline comments as done.
yaxunl retitled this revision from "[HIP] Diagnose unaligned atomic for amdgpu" 
to "[AMDGPU] Diagnose unaligned atomic for amdgpu".
yaxunl added a comment.
Herald added a reviewer: jdoerfert.
Herald added subscribers: jplehr, sstefan1.

revised by Artem's and Fangrui's comments


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

https://reviews.llvm.org/D99201

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/AMDGPU.h
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/HIPAMD.cpp
  clang/test/Driver/amdgpu-openmp-toolchain.c
  clang/test/Driver/amdgpu-toolchain-opencl.cl
  clang/test/Driver/hip-options.hip


Index: clang/test/Driver/hip-options.hip
===
--- clang/test/Driver/hip-options.hip
+++ clang/test/Driver/hip-options.hip
@@ -138,3 +138,10 @@
 // RUN: %clang -### -nogpuinc -nogpulib -mamdgpu-ieee -mno-amdgpu-ieee 
-ffast-math \
 // RUN:   --cuda-gpu-arch=gfx906  %s 2>&1 | FileCheck 
-check-prefixes=IEEE-OFF-NEG %s
 // IEEE-OFF-NEG-NOT: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-mamdgpu-ieee"
+
+// Check -Werror=atomic-alignment is passed for amdpu by default.
+
+// RUN: %clang -### --target=x86_64-unknown-linux-gnu -nogpuinc -nogpulib \
+// RUN:   --cuda-gpu-arch=gfx906 %s 2>&1 | FileCheck -check-prefix=WARN-ATOMIC 
%s
+// WARN-ATOMIC: clang{{.*}} "-triple" "amdgcn-amd-amdhsa" {{.*}} 
"-Werror=atomic-alignment"
+// WARN-ATOMIC-NOT: clang{{.*}} "-triple" "x86_64-unknown-linux-gnu" {{.*}} 
"-Werror=atomic-alignment"
Index: clang/test/Driver/amdgpu-toolchain-opencl.cl
===
--- clang/test/Driver/amdgpu-toolchain-opencl.cl
+++ clang/test/Driver/amdgpu-toolchain-opencl.cl
@@ -28,3 +28,6 @@
 
 // RUN: %clang -### --target=amdgcn-amd-amdhsa-opencl -x cl -emit-llvm 
-mcpu=fiji %s 2>&1 | FileCheck -check-prefix=CHK-LINK %s
 // CHK-LINK: ld.lld{{.*}} "--no-undefined" "-shared"
+
+// RUN: %clang -### --target=amdgcn-amd-amdhsa-opencl -x cl -c -emit-llvm 
-mcpu=fiji %s 2>&1 | FileCheck -check-prefix=CHECK-WARN-ATOMIC %s
+// CHECK-WARN-ATOMIC: "-cc1"{{.*}} "-Werror=atomic-alignment"
Index: clang/test/Driver/amdgpu-openmp-toolchain.c
===
--- clang/test/Driver/amdgpu-openmp-toolchain.c
+++ clang/test/Driver/amdgpu-openmp-toolchain.c
@@ -62,3 +62,7 @@
 // RUN:   --rocm-device-lib-path=%S/Inputs/rocm/amdgcn/bitcode 
-fopenmp-new-driver %s  2>&1 | \
 // RUN: FileCheck %s --check-prefix=CHECK-LIB-DEVICE-NOGPULIB
 // CHECK-LIB-DEVICE-NOGPULIB-NOT: "-cc1" 
{{.*}}ocml.bc"{{.*}}ockl.bc"{{.*}}oclc_daz_opt_on.bc"{{.*}}oclc_unsafe_math_off.bc"{{.*}}oclc_finite_only_off.bc"{{.*}}oclc_correctly_rounded_sqrt_on.bc"{{.*}}oclc_wavefrontsize64_on.bc"{{.*}}oclc_isa_version_803.bc"
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -emit-llvm -S -fopenmp 
-fopenmp-targets=amdgcn-amd-amdhsa -Xopenmp-target=amdgcn-amd-amdhsa 
-march=gfx803 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-WARN-ATOMIC
+// CHECK-WARN-ATOMIC-NOT: "-cc1" "-triple" 
"x86_64-unknown-linux-gnu"{{.*}}"-Werror=atomic-alignment"
+// CHECK-WARN-ATOMIC: "-cc1" "-triple" 
"amdgcn-amd-amdhsa"{{.*}}"-Werror=atomic-alignment"
Index: clang/lib/Driver/ToolChains/HIPAMD.cpp
===
--- clang/lib/Driver/ToolChains/HIPAMD.cpp
+++ clang/lib/Driver/ToolChains/HIPAMD.cpp
@@ -291,6 +291,7 @@
 }
 
 void HIPAMDToolChain::addClangWarningOptions(ArgStringList &CC1Args) const {
+  AMDGPUToolChain::addClangWarningOptions(CC1Args);
   HostTC.addClangWarningOptions(CC1Args);
 }
 
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -118,6 +118,7 @@
 
 void AMDGPUOpenMPToolChain::addClangWarningOptions(
 ArgStringList &CC1Args) const {
+  AMDGPUToolChain::addClangWarningOptions(CC1Args);
   HostTC.addClangWarningOptions(CC1Args);
 }
 
Index: clang/lib/Driver/ToolChains/AMDGPU.h
===
--- clang/lib/Driver/ToolChains/AMDGPU.h
+++ clang/lib/Driver/ToolChains/AMDGPU.h
@@ -121,6 +121,9 @@
   /// Get GPU arch from -mcpu without checking.
   StringRef getGPUArch(const llvm::opt::ArgList &DriverArgs) const;
 
+  /// Common warning options shared by AMDGPU HIP, OpenCL and OpenMP 
toolchains.
+  /// Language specific warning options should go to derived classes.
+  void addClangWarningOptions(llvm::opt::ArgStringList &CC1Args) const 
override;
 };
 
 class LLVM_LIBRARY_VISIBILITY ROCMToolChain : public AMDGPUToolChain {
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/D

[PATCH] D152285: Add support for the NO_COLOR environment variable

2023-06-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 529299.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.

Update based on review feedback so that we check for an empty environment 
variable value.

This was a fun rabbit hole to fall into: lit is not a PTY, so our fallback of 
"automatically enable colors" means `env NO_COLOR=` still disables colors as 
far as lit tests are concerned. See 
https://github.com/llvm/llvm-project/blob/4418434c6de7a861e241ba2448ea4a12080cf08f/clang/test/Driver/color-diagnostics.c#L25
 for another test running into this. What's more, Windows does not have a way 
to set an empty environment variable on the command line from what I can find, 
so testing this manually also doesn't work (at least for me). However, because 
this piggy-backs on the functionality for `-fdiagnostics-color=auto`, I think 
the test coverage I added is sufficient.


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

https://reviews.llvm.org/D152285

Files:
  clang/docs/ReleaseNotes.rst
  clang/docs/UsersManual.rst
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/no-color.c


Index: clang/test/Driver/no-color.c
===
--- /dev/null
+++ clang/test/Driver/no-color.c
@@ -0,0 +1,17 @@
+// RUN: env NO_COLOR=1 %clang -### %s 2>&1 | FileCheck --check-prefix=NO-COLOR 
%s
+// RUN: env NO_COLOR=1 %clang -fcolor-diagnostics -### %s 2>&1 | FileCheck 
--check-prefix=COLOR %s
+// RUN: env NO_COLOR=1 %clang -fdiagnostics-color=auto -### %s 2>&1 | 
FileCheck --check-prefix=NO-COLOR %s
+// RUN: env NO_COLOR=1 %clang -fdiagnostics-color=always -### %s 2>&1 | 
FileCheck --check-prefix=COLOR %s
+// RUN: env NO_COLOR=1 %clang -fdiagnostics-color=never -### %s 2>&1 | 
FileCheck --check-prefix=NO-COLOR %s
+
+// Note, the value of the environment variable does not matter, only that it 
is defined and not empty.
+// RUN: env NO_COLOR=0 %clang -### %s 2>&1 | FileCheck --check-prefix=NO-COLOR 
%s
+// Note, an empty value means we automatically decide whether to enable colors 
or not, and lit tests
+// are not run in a PTY, so colors are disabled by default. There is no easy 
way for us to test this
+// configuration where auto enables colors.
+// RUN: env NO_COLOR= %clang -### %s 2>&1 | FileCheck --check-prefix=NO-COLOR 
%s
+
+int main(void) {}
+
+// COLOR: -fcolor-diagnostics
+// NO-COLOR-NOT: -fcolor-diagnostics
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2341,10 +2341,20 @@
   unsigned MissingArgIndex, MissingArgCount;
   InputArgList Args = getDriverOptTable().ParseArgs(
   Argv.slice(1), MissingArgIndex, MissingArgCount);
+
+  bool ShowColors = true;
+  if (std::optional NoColor =
+  llvm::sys::Process::GetEnv("NO_COLOR");
+  NoColor && !NoColor->empty() && NoColor->at(0) != '\0') {
+// If the user set the NO_COLOR environment variable, we'll honor that
+// unless the command line overrides it.
+ShowColors = false;
+  }
+
   // We ignore MissingArgCount and the return value of ParseDiagnosticArgs.
   // Any errors that would be diagnosed here will also be diagnosed later,
   // when the DiagnosticsEngine actually exists.
-  (void)ParseDiagnosticArgs(*DiagOpts, Args);
+  (void)ParseDiagnosticArgs(*DiagOpts, Args, /*Diags=*/nullptr, ShowColors);
   return DiagOpts;
 }
 
Index: clang/docs/UsersManual.rst
===
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -251,6 +251,11 @@
 ^
 //
 
+   If the ``NO_COLOR`` environment variable is defined and not empty
+   (regardless of value), color diagnostics are disabled. If ``NO_COLOR`` is
+   defined and ``-fcolor-diagnostics`` is passed on the command line, Clang
+   will honor the command line argument.
+
 .. option:: -fansi-escape-codes
 
Controls whether ANSI escape codes are used instead of the Windows Console
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -222,6 +222,8 @@
 - Clang now support ``__builtin_FUNCSIG()`` which returns the same information
   as the ``__FUNCSIG__`` macro (available only with ``-fms-extensions`` flag).
   This fixes (`#58951 `_).
+- Clang now supports the `NO_COLOR `_ environment
+  variable as a way to disable color diagnostics.
 
 New Compiler Flags
 --


Index: clang/test/Driver/no-color.c
===
--- /dev/null
+++ clang/test/Driver/no-color.c
@@ -0,0 +1,17 @@
+// RUN: env NO_COLOR=1 %clang -### %s 2>&1 | FileCheck --check-prefix=NO-COLOR %s
+// RUN: env NO_COLOR=1 %c

[PATCH] D150860: [OpenMP] Change clang emitTargetDataCalls to use OMPIRBuilder

2023-06-07 Thread Akash Banerjee via Phabricator via cfe-commits
TIFitis marked an inline comment as done.
TIFitis added inline comments.



Comment at: clang/test/OpenMP/target_data_codegen.cpp:355-356
 // Region 00
+// CK2-DAG: [[DEV:%[^,]+]] = sext i32 [[DEVi32:%[^,]+]] to i64
+// CK2-DAG: [[DEVi32]] = load i32, ptr %{{[^,]+}},
 // CK2: br i1 %{{[^,]+}}, label %[[IFTHEN:[^,]+]], label %[[IFELSE:[^,]+]]

jsjodin wrote:
> TIFitis wrote:
> > When both if clause and device clause are present, the device clause 
> > argument is inadvertently brought outside the `IfThen` region as we emit 
> > the `llvm:Value` from the `Clang::Expr` for it before making call to 
> > createTargetData.
> > 
> > I don't think this change would affect any use cases.
> > When both if clause and device clause are present, the device clause 
> > argument is inadvertently brought outside the `IfThen` region as we emit 
> > the `llvm:Value` from the `Clang::Expr` for it before making call to 
> > createTargetData.
> > 
> > I don't think this change would affect any use cases.
> 
> Is it at all possible that the load could cause an exception if moved outside 
> the if?
> 
Int pointer is not allowed inside device clause, so I don't think think the 
load can cause exceptions.

Also this would fix a scenario similar to the if clause, where in the following 
code `target_begin` would get device as //10// and `target_end` mapper call 
would get device as //100//.

```
int arg = 10;
#pragma omp target data map(to: arg) if(arg < 20) device(arg)
  {arg = 100;}
```



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150860

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


[PATCH] D146924: [clang] Add support for dollar sign in ud_suffix of numeric constants

2023-06-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment.

In D146924#4224237 , @tahonermann 
wrote:

>> It's not in the range of any of the universal-character-names.
>
> I assume you mean that it isn't included in the range of UCNs that are 
> allowed in identifiers. If so, that is correct; it is neither in XID_Start 
> 
>  or XID_Continue 
> 
>  (note that the current wording for the C and C++ standards was recently 
> changed for C23 and C++23 to defer to the Unicode standard for what 
> characters are allowed in identifiers; see N2836 
>  and P1949 
> ).
>
> Interesting, Clang currently crashes when `$` is specified using a UCN in an 
> identifier with `-fno-dollars-in-identifiers`. It would be nice to correct 
> that issue with this patch. See https://godbolt.org/z/9c7Pc8jbT.
>
> There is also a proposal to WG14 to allow `$` in identifiers. See N3046 
> . I don't think 
> that paper has been reviewed by WG14 yet. If approved by WG14, I would expect 
> a similar proposal to be submitted to WG21. Nothing to do here now; just 
> something to be aware of.
>
> There is a proposal to WG21 to add `$` (and other characters) to the basic 
> character set. See P2558 . If accepted as currently 
> proposed, it would make use of a UCN to name `$` outside of character and 
> string literals ill-formed. Again, nothing to do here now; just something to 
> be aware of.

I missed that comment @tahonermann 
The crash is there https://github.com/llvm/llvm-project/issues/62133
But fundamentally C and C++ having different behavior is nasty, so I wrote a 
paper https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3124.pdf


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146924

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


[clang] 2878282 - [clang][DeclPrinter] Fix AST print of out-of-line record definitions

2023-06-07 Thread Aaron Ballman via cfe-commits

Author: Timo Stripf
Date: 2023-06-07T10:57:31-04:00
New Revision: 2878282dc550e6a8174917480a2dc22bbe283ed1

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

LOG: [clang][DeclPrinter] Fix AST print of out-of-line record definitions

DeclPrinter::VisitCXXRecordDecl did not output qualifiers for records.
As result, the output of out-of-line record definitions was incorrect.

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

Added: 


Modified: 
clang/lib/AST/DeclPrinter.cpp
clang/test/AST/ast-print-record-decl.c

Removed: 




diff  --git a/clang/lib/AST/DeclPrinter.cpp b/clang/lib/AST/DeclPrinter.cpp
index f3bad9f45b74d..932e9b23ebe7f 100644
--- a/clang/lib/AST/DeclPrinter.cpp
+++ b/clang/lib/AST/DeclPrinter.cpp
@@ -997,7 +997,10 @@ void DeclPrinter::VisitCXXRecordDecl(CXXRecordDecl *D) {
   prettyPrintAttributes(D);
 
   if (D->getIdentifier()) {
-Out << ' ' << *D;
+Out << ' ';
+if (auto *NNS = D->getQualifier())
+  NNS->print(Out, Policy);
+Out << *D;
 
 if (auto S = dyn_cast(D)) {
   ArrayRef Args = S->getTemplateArgs().asArray();

diff  --git a/clang/test/AST/ast-print-record-decl.c 
b/clang/test/AST/ast-print-record-decl.c
index 8553e98b8251f..d3717a4feab83 100644
--- a/clang/test/AST/ast-print-record-decl.c
+++ b/clang/test/AST/ast-print-record-decl.c
@@ -289,3 +289,19 @@ KW DeclGroupInMemberList {
 
 // A tag decl group in the tag decl's own member list is exercised in
 // defSelfRef above.
+
+
+// Check out-of-line record definition
+#ifdef __cplusplus
+// PRINT-CXX-NEXT: [[KW]] OutOfLineRecord {
+KW OutOfLineRecord {
+  // PRINT-CXX-NEXT: [[KW]] Inner
+  KW Inner;
+  // PRINT-CXX-NEXT: };
+};
+
+// PRINT-CXX-NEXT: [[KW]] OutOfLineRecord::Inner {
+KW OutOfLineRecord::Inner {
+  // PRINT-CXX-NEXT: };
+};
+#endif



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


[PATCH] D151528: [clang][DeclPrinter] Fix AST print of out-of-line record definitions

2023-06-07 Thread Aaron Ballman 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 rG2878282dc550: [clang][DeclPrinter] Fix AST print of 
out-of-line record definitions (authored by strimo378, committed by 
aaron.ballman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151528

Files:
  clang/lib/AST/DeclPrinter.cpp
  clang/test/AST/ast-print-record-decl.c


Index: clang/test/AST/ast-print-record-decl.c
===
--- clang/test/AST/ast-print-record-decl.c
+++ clang/test/AST/ast-print-record-decl.c
@@ -289,3 +289,19 @@
 
 // A tag decl group in the tag decl's own member list is exercised in
 // defSelfRef above.
+
+
+// Check out-of-line record definition
+#ifdef __cplusplus
+// PRINT-CXX-NEXT: [[KW]] OutOfLineRecord {
+KW OutOfLineRecord {
+  // PRINT-CXX-NEXT: [[KW]] Inner
+  KW Inner;
+  // PRINT-CXX-NEXT: };
+};
+
+// PRINT-CXX-NEXT: [[KW]] OutOfLineRecord::Inner {
+KW OutOfLineRecord::Inner {
+  // PRINT-CXX-NEXT: };
+};
+#endif
Index: clang/lib/AST/DeclPrinter.cpp
===
--- clang/lib/AST/DeclPrinter.cpp
+++ clang/lib/AST/DeclPrinter.cpp
@@ -997,7 +997,10 @@
   prettyPrintAttributes(D);
 
   if (D->getIdentifier()) {
-Out << ' ' << *D;
+Out << ' ';
+if (auto *NNS = D->getQualifier())
+  NNS->print(Out, Policy);
+Out << *D;
 
 if (auto S = dyn_cast(D)) {
   ArrayRef Args = S->getTemplateArgs().asArray();


Index: clang/test/AST/ast-print-record-decl.c
===
--- clang/test/AST/ast-print-record-decl.c
+++ clang/test/AST/ast-print-record-decl.c
@@ -289,3 +289,19 @@
 
 // A tag decl group in the tag decl's own member list is exercised in
 // defSelfRef above.
+
+
+// Check out-of-line record definition
+#ifdef __cplusplus
+// PRINT-CXX-NEXT: [[KW]] OutOfLineRecord {
+KW OutOfLineRecord {
+  // PRINT-CXX-NEXT: [[KW]] Inner
+  KW Inner;
+  // PRINT-CXX-NEXT: };
+};
+
+// PRINT-CXX-NEXT: [[KW]] OutOfLineRecord::Inner {
+KW OutOfLineRecord::Inner {
+  // PRINT-CXX-NEXT: };
+};
+#endif
Index: clang/lib/AST/DeclPrinter.cpp
===
--- clang/lib/AST/DeclPrinter.cpp
+++ clang/lib/AST/DeclPrinter.cpp
@@ -997,7 +997,10 @@
   prettyPrintAttributes(D);
 
   if (D->getIdentifier()) {
-Out << ' ' << *D;
+Out << ' ';
+if (auto *NNS = D->getQualifier())
+  NNS->print(Out, Policy);
+Out << *D;
 
 if (auto S = dyn_cast(D)) {
   ArrayRef Args = S->getTemplateArgs().asArray();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D152285: Add support for the NO_COLOR environment variable

2023-06-07 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

LGTM. Glad that my comment on https://github.com/llvm/llvm-project/issues/23983 
might bring attention to users.

I'd like to make my stance softer: if we find `CLICOLOR_FORCE` useful, we can 
implement it later.
I don't mind how we handle `NO_COLOR=1 CLICOLOR_FORCE=1`. This seems invalid 
input from the user and making either option win should be fine (I'd prefer 
that `NO_COLOR` wins.)


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

https://reviews.llvm.org/D152285

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


  1   2   3   >