[PATCH] D97223: Add Alignment argument to IRBuilder CreateAtomicRMW and CreateAtomicCmpXchg.

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet accepted this revision.
gchatelet added a comment.

LGTM as well


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97223

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

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

In D97297#2583391 , @Eugene.Zelenko 
wrote:

> Please mention new option in Release Notes.

The base check is not yet accepted or in. I plan to land the entire thing in 
one go once everything is accepted because a partial landing would result in a 
useless check (in one way or another).




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst:89
+warning about the two parameters are silenced.
+Defaults to ``1``.
+Might be any positive integer.

Eugene.Zelenko wrote:
> Please use single back-ticks for option values. Same below.
**All** values (including above the ``_Bool`` and such, or only the numeric 
constants? The ``LHS`` and ``RHS`` below are examples of source code, not a 
valid value for the option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97297

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

[PATCH] D95458: [PowerPC] Exploit xxsplti32dx (constant materialization) for scalars

[Albion Fung via Phabricator via cfe-commits]

Conanap updated this revision to Diff 326000.
Conanap marked 6 inline comments as done.
Conanap added a comment.

Addressed some nits and a problem where sometimes the compiler would spill half 
way through materialization.


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

https://reviews.llvm.org/D95458

Files:
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/lib/Target/PowerPC/PPCISelLowering.h
  llvm/lib/Target/PowerPC/PPCInstrInfo.cpp
  llvm/lib/Target/PowerPC/PPCInstrInfo.td
  llvm/lib/Target/PowerPC/PPCInstrPrefix.td
  llvm/test/CodeGen/PowerPC/constant-pool.ll
  llvm/test/CodeGen/PowerPC/p10-splatImm-CPload-pcrel.ll
  llvm/test/CodeGen/PowerPC/pcrel-call-linkage-leaf.ll
  llvm/test/CodeGen/PowerPC/pcrel-linkeropt.ll
  llvm/test/CodeGen/PowerPC/pcrel.ll

Index: llvm/test/CodeGen/PowerPC/pcrel.ll
===
--- llvm/test/CodeGen/PowerPC/pcrel.ll
+++ llvm/test/CodeGen/PowerPC/pcrel.ll
@@ -8,13 +8,14 @@
 
 ; Constant Pool Index.
 ; CHECK-S-LABEL: ConstPool
-; CHECK-S:   plfd f1, .LCPI0_0@PCREL(0), 1
+; CHECK-S:   xxsplti32dx vs1, 0, 1081002676
+; CHECK-S-NEXT:   xxsplti32dx vs1, 1, 962072674
 ; CHECK-S:   blr
 
 ; CHECK-O-LABEL: ConstPool
-; CHECK-O:   plfd 1, 0(0), 1
-; CHECK-O-NEXT:  R_PPC64_PCREL34  .rodata.cst8
-; CHECK-O:   blr
+; CHECK-O:   xxsplti32dx 1, 0, 1081002676
+; CHECK-O-NEXT:  xxsplti32dx 1, 1, 962072674
+; CHECK-O-NEXT:  blr
 define dso_local double @ConstPool() local_unnamed_addr {
   entry:
 ret double 0x406ECAB439581062
Index: llvm/test/CodeGen/PowerPC/pcrel-linkeropt.ll
===
--- llvm/test/CodeGen/PowerPC/pcrel-linkeropt.ll
+++ llvm/test/CodeGen/PowerPC/pcrel-linkeropt.ll
@@ -35,6 +35,9 @@
 @FuncPtrOut = external local_unnamed_addr global void (...)*, align 8
 
 define dso_local void @ReadWrite8() local_unnamed_addr #0 {
+; In this test the stb r3, 0(r4) cannot be optimized because it
+; uses the register r3 and that register is defined by lbz r3, 0(r3)
+; which is defined between the pld and the stb.
 ; CHECK-LABEL: ReadWrite8:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:pld r3, input8@got@pcrel(0), 1
@@ -42,9 +45,6 @@
 ; CHECK-NEXT:pld r4, output8@got@pcrel(0), 1
 ; CHECK-NEXT:.reloc .Lpcrel0-8,R_PPC64_PCREL_OPT,.-(.Lpcrel0-8)
 ; CHECK-NEXT:lbz r3, 0(r3)
-; In this test the stb r3, 0(r4) cannot be optimized because it
-; uses the register r3 and that register is defined by lbz r3, 0(r3)
-; which is defined between the pld and the stb.
 ; CHECK-NEXT:stb r3, 0(r4)
 ; CHECK-NEXT:blr
 entry:
@@ -54,6 +54,9 @@
 }
 
 define dso_local void @ReadWrite16() local_unnamed_addr #0 {
+; In this test the sth r3, 0(r4) cannot be optimized because it
+; uses the register r3 and that register is defined by lhz r3, 0(r3)
+; which is defined between the pld and the sth.
 ; CHECK-LABEL: ReadWrite16:
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:pld r3, input16@got@pcrel(0), 1
@@ -61,9 +64,6 @@
 ; CHECK-NEXT:pld r4, output16@got@pcrel(0), 1
 ; CHECK-NEXT:.reloc .Lpcrel1-8,R_PPC64_PCREL_OPT,.-(.Lpcrel1-8)
 ; CHECK-NEXT:lhz r3, 0(r3)
-; In this test the sth r3, 0(r4) cannot be optimized because it
-; uses the register r3 and that register is defined by lhz r3, 0(r3)
-; which is defined between the pld and the sth.
 ; CHECK-NEXT:sth r3, 0(r4)
 ; CHECK-NEXT:blr
 entry:
@@ -144,7 +144,8 @@
 ; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:pld r3, inputf64@got@pcrel(0), 1
 ; CHECK-NEXT:  .Lpcrel5:
-; CHECK-NEXT:plfd f1, .LCPI6_0@PCREL(0), 1
+; CHECK-NEXT:xxsplti32dx vs1, 0, 1075524403
+; CHECK-NEXT:xxsplti32dx vs1, 1, 858993459
 ; CHECK-NEXT:.reloc .Lpcrel5-8,R_PPC64_PCREL_OPT,.-(.Lpcrel5-8)
 ; CHECK-NEXT:lfd f0, 0(r3)
 ; CHECK-NEXT:pld r3, outputf64@got@pcrel(0), 1
@@ -286,8 +287,7 @@
 
 define dso_local void @FuncPtrCall() local_unnamed_addr #0 {
 ; CHECK-LABEL: FuncPtrCall:
-; CHECK: .localentry FuncPtrCall, 1
-; CHECK-NEXT:  # %bb.0: # %entry
+; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:pld r3, FuncPtrIn@got@pcrel(0), 1
 ; CHECK-NEXT:  .Lpcrel10:
 ; CHECK-NEXT:.reloc .Lpcrel10-8,R_PPC64_PCREL_OPT,.-(.Lpcrel10-8)
@@ -317,8 +317,7 @@
 
 define dso_local signext i32 @VecMultiUse() local_unnamed_addr #0 {
 ; CHECK-LABEL: VecMultiUse:
-; CHECK: .localentry VecMultiUse, 1
-; CHECK-NEXT:  # %bb.0: # %entry
+; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:mflr r0
 ; CHECK-NEXT:std r29, -24(r1) # 8-byte Folded Spill
 ; CHECK-NEXT:std r30, -16(r1) # 8-byte Folded Spill
@@ -355,8 +354,7 @@
 
 define dso_local signext i32 @UseAddr(i32 signext %a) local_unnamed_addr #0 {
 ; CHECK-LABEL: UseAddr:
-; CHECK: .localentry UseAddr, 1
-; CHECK-NEXT:  # %bb.0: # %entry
+; CHECK:   # %bb.0: # %entry
 ; CHECK-NEXT:mflr r0
 ; CHECK-NEXT:std r30, -16(r1) # 8-byte Folded Spill
 ; CHECK-NEXT:std r0, 16(r1)
Index: llvm/te

[PATCH] D95458: [PowerPC] Exploit xxsplti32dx (constant materialization) for scalars

[Albion Fung via Phabricator via cfe-commits]

Conanap added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.h:1321
   bool convertToNonDenormSingle(APFloat &ArgAPFloat);
+  bool checkNonDenormCannotConvertToSingle(APInt &ArgAPInt);
+  bool checkNonDenormCannotConvertToSingle(APFloat &ArgAPFloat);

stefanp wrote:
> Is the APInt version of this function used anywere?
> 
Hm I don't think so, although I implemented it for consistency with `XXSPLTIDP` 
(`convertToNonDenormSingle`). I'll remove this if that is preferred.



Comment at: llvm/test/CodeGen/PowerPC/constant-pool.ll:363
+; CHECK-NEXT:stxv vs3, 32(r1) # 16-byte Folded Spill
+; CHECK-NEXT:xxsplti32dx vs3, 1, -343597384
+; CHECK-NEXT:# kill: def $f3 killed $f3 killed $vsl3

stefanp wrote:
> What is going on here?
> It almost looks like we are spilling `vs3` half way through materializing a 
> constant.
This is fixed now, thank you for spotting this.



Comment at: llvm/test/CodeGen/PowerPC/pcrel-linkeropt.ll:147
 ; CHECK-NEXT:  .Lpcrel5:
-; CHECK-NEXT:plfd f1, .LCPI6_0@PCREL(0), 1
+; CHECK-NEXT:xxsplti32dx vs1, 0, 1075524403
+; CHECK-NEXT:xxsplti32dx vs1, 1, 858993459

Just a heads up - the tests in this file are autogenerated, hence some 
unrelated changes.


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

https://reviews.llvm.org/D95458

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

[PATCH] D97358: [X86] Support amx-bf16 intrinsic.

[LiuChen via Phabricator via cfe-commits]

LiuChen3 updated this revision to Diff 326002.
LiuChen3 added a comment.

Address Pengfei and Yuanke's comments. We don't need more tile type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97358

Files:
  clang/include/clang/Basic/BuiltinsX86_64.def
  clang/lib/Headers/amxintrin.h
  clang/test/CodeGen/X86/amx_api.c
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/lib/Target/X86/X86ExpandPseudo.cpp
  llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
  llvm/lib/Target/X86/X86InstrAMX.td
  llvm/lib/Target/X86/X86LowerAMXType.cpp
  llvm/lib/Target/X86/X86PreTileConfig.cpp
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/test/CodeGen/X86/AMX/amx-tile-basic.ll

Index: llvm/test/CodeGen/X86/AMX/amx-tile-basic.ll
===
--- llvm/test/CodeGen/X86/AMX/amx-tile-basic.ll
+++ llvm/test/CodeGen/X86/AMX/amx-tile-basic.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+amx-tile -mattr=+avx512f -verify-machineinstrs | FileCheck %s
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+amx-tile,+amx-int8,+amx-bf16,+avx512f -verify-machineinstrs | FileCheck %s
 
 define void @test_amx(i8* %pointer, i8* %base, i64 %stride) {
 ; CHECK-LABEL: test_amx:
@@ -22,6 +22,7 @@
 ; CHECK-NEXT:tdpbsud %tmm2, %tmm1, %tmm0
 ; CHECK-NEXT:tdpbusd %tmm2, %tmm1, %tmm0
 ; CHECK-NEXT:tdpbuud %tmm2, %tmm1, %tmm0
+; CHECK-NEXT:tdpbf16ps %tmm2, %tmm1, %tmm0
 ; CHECK-NEXT:tilestored %tmm0, (%rdi,%rdx)
 ; CHECK-NEXT:tilerelease
 ; CHECK-NEXT:vzeroupper
@@ -33,7 +34,8 @@
   %d1 = call x86_amx @llvm.x86.tdpbsud.internal(i16 8, i16 8, i16 8, x86_amx %d0, x86_amx %a, x86_amx %b)
   %d2 = call x86_amx @llvm.x86.tdpbusd.internal(i16 8, i16 8, i16 8, x86_amx %d1, x86_amx %a, x86_amx %b)
   %d3 = call x86_amx @llvm.x86.tdpbuud.internal(i16 8, i16 8, i16 8, x86_amx %d2, x86_amx %a, x86_amx %b)
-  call void @llvm.x86.tilestored64.internal(i16 8, i16 8, i8* %pointer, i64 %stride, x86_amx %d3)
+  %d4 = call x86_amx @llvm.x86.tdpbf16ps.internal(i16 8, i16 8, i16 8, x86_amx %d3, x86_amx %a, x86_amx %b)
+  call void @llvm.x86.tilestored64.internal(i16 8, i16 8, i8* %pointer, i64 %stride, x86_amx %d4)
 
   ret void
 }
@@ -44,4 +46,5 @@
 declare x86_amx @llvm.x86.tdpbsud.internal(i16, i16, i16, x86_amx, x86_amx, x86_amx)
 declare x86_amx @llvm.x86.tdpbusd.internal(i16, i16, i16, x86_amx, x86_amx, x86_amx)
 declare x86_amx @llvm.x86.tdpbuud.internal(i16, i16, i16, x86_amx, x86_amx, x86_amx)
+declare x86_amx @llvm.x86.tdpbf16ps.internal(i16, i16, i16, x86_amx, x86_amx, x86_amx)
 declare void @llvm.x86.tilestored64.internal(i16, i16, i8*, i64, x86_amx)
Index: llvm/lib/Target/X86/X86RegisterInfo.cpp
===
--- llvm/lib/Target/X86/X86RegisterInfo.cpp
+++ llvm/lib/Target/X86/X86RegisterInfo.cpp
@@ -888,6 +888,7 @@
   case X86::PTDPBUSDV:
   case X86::PTDPBUUDV:
   case X86::PTILEZEROV:
+  case X86::PTDPBF16PSV:
 MachineOperand &MO1 = MI->getOperand(1);
 MachineOperand &MO2 = MI->getOperand(2);
 ShapeT Shape(&MO1, &MO2, MRI);
Index: llvm/lib/Target/X86/X86PreTileConfig.cpp
===
--- llvm/lib/Target/X86/X86PreTileConfig.cpp
+++ llvm/lib/Target/X86/X86PreTileConfig.cpp
@@ -159,6 +159,7 @@
   case X86::PTDPBUSDV:
   case X86::PTDPBUUDV:
   case X86::PTILEZEROV:
+  case X86::PTDPBF16PSV:
 MachineOperand &MO1 = const_cast(MI.getOperand(1));
 MachineOperand &MO2 = const_cast(MI.getOperand(2));
 ShapeT Shape(&MO1, &MO2, MRI);
@@ -256,6 +257,7 @@
   case X86::PTDPBUSDV:
   case X86::PTDPBUUDV:
   case X86::PTILEZEROV:
+  case X86::PTDPBF16PSV:
 return true;
   }
 }
Index: llvm/lib/Target/X86/X86LowerAMXType.cpp
===
--- llvm/lib/Target/X86/X86LowerAMXType.cpp
+++ llvm/lib/Target/X86/X86LowerAMXType.cpp
@@ -70,7 +70,8 @@
   case Intrinsic::x86_tdpbssd_internal:
   case Intrinsic::x86_tdpbsud_internal:
   case Intrinsic::x86_tdpbusd_internal:
-  case Intrinsic::x86_tdpbuud_internal: {
+  case Intrinsic::x86_tdpbuud_internal:
+  case Intrinsic::x86_tdpbf16ps_internal: {
 switch (OpNo) {
 case 3:
   Row = II->getArgOperand(0);
Index: llvm/lib/Target/X86/X86InstrAMX.td
===
--- llvm/lib/Target/X86/X86InstrAMX.td
+++ llvm/lib/Target/X86/X86InstrAMX.td
@@ -138,6 +138,16 @@
   "tdpbf16ps\t{$src3, $src2, $dst|$dst, $src2, $src3}",
   []>, VEX_4V, T8XS;
 
+// Pseduo instruction for RA.
+let Constraints = "$src4 = $dst" in
+  def PTDPBF16PSV : PseudoI<(outs TILE: $dst), (ins GR16:$src1,
+ GR16:$src2, GR16:$src3, TILE:$src4,
+ TILE

[PATCH] D82547: [Debugify] Expose original debug info preservation check as CC1 option

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro updated this revision to Diff 326005.
djtodoro retitled this revision from "[VerifyDIPreserve] Expose original 
debuginfo preservation check as CC1 option" to "[Debugify] Expose original 
debug info preservation check as CC1 option".
djtodoro added a comment.
Herald added subscribers: jansvoboda11, dexonsmith.

- Rebase on top of trunk


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

https://reviews.llvm.org/D82547

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/verify-debug-info-preservation.c
  llvm/docs/HowToUpdateDebugInfo.rst

Index: llvm/docs/HowToUpdateDebugInfo.rst
===
--- llvm/docs/HowToUpdateDebugInfo.rst
+++ llvm/docs/HowToUpdateDebugInfo.rst
@@ -376,6 +376,17 @@
 
   $ llvm-original-di-preservation.py sample.json sample.html
 
+Testing of original debug info preservation can be invoked from front-end level
+as follows:
+
+.. code-block:: bash
+
+  # Test each pass.
+  $ clang -Xclang -fverify-debuginfo-preserve -g -O2 sample.c
+
+  # Test each pass and export the issues report into the JSON file.
+  $ clang -Xclang -fverify-debuginfo-preserve -Xclang -fverify-debuginfo-preserve-export=sample.json -g -O2 sample.c
+
 Mutation testing for MIR-level transformations
 --
 
Index: clang/test/Driver/verify-debug-info-preservation.c
===
--- /dev/null
+++ clang/test/Driver/verify-debug-info-preservation.c
@@ -0,0 +1,14 @@
+// We support the CC1 options for testing whether each LLVM pass preserves
+// original debug info.
+
+// RUN: %clang -g -Xclang -fverify-debuginfo-preserve -### %s 2>&1 \
+// RUN: | FileCheck --check-prefix=VERIFYDIPRESERVE %s
+
+// VERIFYDIPRESERVE: "-fverify-debuginfo-preserve"
+
+// RUN: %clang -g -Xclang -fverify-debuginfo-preserve \
+// RUN: -Xclang -fverify-debuginfo-preserve-export=%t.json -### %s 2>&1 \
+// RUN: | FileCheck --check-prefix=VERIFYDIPRESERVE-JSON-EXPORT %s
+
+// VERIFYDIPRESERVE-JSON-EXPORT: "-fverify-debuginfo-preserve"
+// VERIFYDIPRESERVE-JSON-EXPORT: "-fverify-debuginfo-preserve-export={{.*}}"
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1634,6 +1634,16 @@
   llvm::is_contained(DebugEntryValueArchs, T.getArch()))
 Opts.EmitCallSiteInfo = true;
 
+  Opts.EnableDIPreservationVerify =
+  Args.hasArg(OPT_fverify_debuginfo_preserve);
+  // Ignore the option if the -fverify-debuginfo-preserve wasn't enabled.
+  if (Opts.EnableDIPreservationVerify &&
+  Args.hasArg(OPT_fverify_debuginfo_preserve_export)) {
+ Opts.DIBugsReportFilePath =
+  std::string(
+  Args.getLastArgValue(OPT_fverify_debuginfo_preserve_export));
+  }
+
   Opts.NewStructPathTBAA = !Args.hasArg(OPT_no_struct_path_tbaa) &&
Args.hasArg(OPT_new_struct_path_tbaa);
   Opts.OptimizeSize = getOptimizationLevelSize(Args);
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -85,6 +85,7 @@
 #include "llvm/Transforms/Utils/NameAnonGlobals.h"
 #include "llvm/Transforms/Utils/SymbolRewriter.h"
 #include "llvm/Transforms/Utils/UniqueInternalLinkageNames.h"
+#include "llvm/Transforms/Utils/Debugify.h"
 #include 
 using namespace clang;
 using namespace llvm;
@@ -926,7 +927,17 @@
   if (TM)
 TheModule->setDataLayout(TM->createDataLayout());
 
-  legacy::PassManager PerModulePasses;
+  DebugifyCustomPassManager PerModulePasses;
+  DebugInfoPerPassMap DIPreservationMap;
+  if (CodeGenOpts.EnableDIPreservationVerify) {
+PerModulePasses.setDebugifyMode(
+DebugifyMode::OriginalDebugInfo);
+PerModulePasses.setDIPreservationMap(DIPreservationMap);
+
+if (!CodeGenOpts.DIBugsReportFilePath.empty())
+  PerModulePasses.setOrigDIVerifyBugsReportFilePath(
+  CodeGenOpts.DIBugsReportFilePath);
+  }
   PerModulePasses.add(
   createTargetTransformInfoWrapperPass(getTargetIRAnalysis()));
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4788,6 +4788,16 @@
 "fexperimental-debug-variable-locations">,
 HelpText<"Use experimental new value-tracking variable locations">,
 MarshallingInfoFlag>;
+def fverify_debuginfo_preserve
+: Flag<["-"], "fverify-debuginfo-preserve">,
+  HelpText<"Enable Debug Info Metadata preservation testing in "
+   "optimizations

[PATCH] D97364: [docs] Add a release note for the removing of -Wreturn-std-move-in-c++11

[Yang Fan via Phabricator via cfe-commits]

nullptr.cpp created this revision.
nullptr.cpp requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

`-Wreturn-std-move-in-c++11` has been removed in 
fbee4a0c79cc4ee87c34e51342742a5bc6fcf872 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97364

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -79,6 +79,10 @@
 - The clang-cl ``/fallback`` flag, which made clang-cl invoke Microsoft Visual
   C++ on files it couldn't compile itself, has been removed.
 
+- ``-Wreturn-std-move-in-c++11``, which checked whether an entity is affected 
by
+  `CWG1579 `_ to become implicitly movable, has been
+  removed.
+
 New Pragmas in Clang
 
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -79,6 +79,10 @@
 - The clang-cl ``/fallback`` flag, which made clang-cl invoke Microsoft Visual
   C++ on files it couldn't compile itself, has been removed.
 
+- ``-Wreturn-std-move-in-c++11``, which checked whether an entity is affected by
+  `CWG1579 `_ to become implicitly movable, has been
+  removed.
+
 New Pragmas in Clang
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88220: [C++20] P1825R0: More implicit moves

[Yang Fan via Phabricator via cfe-commits]

nullptr.cpp added a comment.

In D88220#2583486 , @aaronpuchert 
wrote:

> In D88220#2581538 , @aaron.ballman 
> wrote:
>
>> We usually rely on the release notes to say something, but we didn't do that 
>> here.
>
> Perhaps @nullptr.cpp could add something there? The file is 
> `clang/docs/ReleaseNotes.rst`.

Add by D97364 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

[PATCH] D97358: [X86] Support amx-bf16 intrinsic.

[Pengfei Wang via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97358

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

[clang] 85eb12e - [OpenCL] Add declarations with enum/typedef args

[Sven van Haastregt via cfe-commits]

Author: Sven van Haastregt
Date: 2021-02-24T09:27:35Z
New Revision: 85eb12eefdf6a95afc49bc66df41738d19702977

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

LOG: [OpenCL] Add declarations with enum/typedef args

Add the remaining missing builtin function declarations that have enum
or typedef argument or return types.

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

Added: 


Modified: 
clang/lib/Sema/OpenCLBuiltins.td
clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl

Removed: 




diff  --git a/clang/lib/Sema/OpenCLBuiltins.td 
b/clang/lib/Sema/OpenCLBuiltins.td
index 7f0e0c1e8297..77d0203629eb 100644
--- a/clang/lib/Sema/OpenCLBuiltins.td
+++ b/clang/lib/Sema/OpenCLBuiltins.td
@@ -310,6 +310,7 @@ def Event : Type<"event_t", 
QualType<"Context.OCLEventTy">>;
 def Queue : Type<"queue_t", QualType<"Context.OCLQueueTy">>;
 def ReserveId : Type<"reserve_id_t", 
QualType<"Context.OCLReserveIDTy">>;
 def MemFenceFlags : TypedefType<"cl_mem_fence_flags">;
+def ClkProfilingInfo  : TypedefType<"clk_profiling_info">;
 
 // OpenCL v2.0 s6.13.11: Atomic integer and floating-point types.
 def AtomicInt : Type<"atomic_int", 
QualType<"Context.getAtomicType(Context.IntTy)">>;
@@ -323,6 +324,7 @@ def AtomicUIntPtr : Type<"atomic_uintptr_t", 
QualType<"Context.getAtomic
 def AtomicSize: Type<"atomic_size_t", 
QualType<"Context.getAtomicType(Context.getSizeType())">>;
 def AtomicPtrDiff : Type<"atomic_ptr
diff _t", QualType<"Context.getAtomicType(Context.getPointerDiffType())">>;
 
+def AtomicFlag: TypedefType<"atomic_flag">;
 def MemoryOrder   : EnumType<"memory_order">;
 def MemoryScope   : EnumType<"memory_scope">;
 
@@ -913,6 +915,26 @@ foreach AS = [ConstantAS] in {
 
 // OpenCL v3.0 s6.15.8 - Synchronization Functions.
 def : Builtin<"barrier", [Void, MemFenceFlags], Attr.Convergent>;
+let MinVersion = CL20 in {
+  def : Builtin<"work_group_barrier", [Void, MemFenceFlags], Attr.Convergent>;
+  def : Builtin<"work_group_barrier", [Void, MemFenceFlags, MemoryScope], 
Attr.Convergent>;
+}
+
+// OpenCL v3.0 s6.15.9 - Legacy Explicit Memory Fence Functions.
+def : Builtin<"mem_fence", [Void, MemFenceFlags]>;
+def : Builtin<"read_mem_fence", [Void, MemFenceFlags]>;
+def : Builtin<"write_mem_fence", [Void, MemFenceFlags]>;
+
+// OpenCL v3.0 s6.15.10 - Address Space Qualifier Functions.
+// to_global, to_local, to_private are declared in Builtins.def.
+
+let MinVersion = CL20 in {
+  // The OpenCL 3.0 specification defines these with a "gentype" argument 
indicating any builtin
+  // type or user-defined type, which cannot be represented currently.  Hence 
we slightly diverge
+  // by providing only the following overloads with a void pointer.
+  def : Builtin<"get_fence", [MemFenceFlags, PointerType]>;
+  def : Builtin<"get_fence", [MemFenceFlags, PointerType, 
GenericAS>]>;
+}
 
 //
 // OpenCL v1.1 s6.11.10, v1.2 s6.12.10, v2.0 s6.13.10: Async Copies from 
Global to Local Memory, Local to Global Memory, and Prefetch
@@ -1030,6 +1052,8 @@ foreach AS = [GlobalAS, LocalAS] in {
 }
 // OpenCL v2.0 s6.13.11 - Atomic Functions.
 let MinVersion = CL20 in {
+  def : Builtin<"atomic_work_item_fence", [Void, MemFenceFlags, MemoryOrder, 
MemoryScope]>;
+
   foreach TypePair = [[AtomicInt, Int], [AtomicUInt, UInt],
   [AtomicLong, Long], [AtomicULong, ULong],
   [AtomicFloat, Float], [AtomicDouble, Double]] in {
@@ -1037,10 +1061,22 @@ let MinVersion = CL20 in {
 [Void, PointerType, GenericAS>, 
TypePair[1]]>;
 def : Builtin<"atomic_store",
 [Void, PointerType, GenericAS>, 
TypePair[1]]>;
+def : Builtin<"atomic_store_explicit",
+[Void, PointerType, GenericAS>, TypePair[1], 
MemoryOrder]>;
+def : Builtin<"atomic_store_explicit",
+[Void, PointerType, GenericAS>, TypePair[1], 
MemoryOrder, MemoryScope]>;
 def : Builtin<"atomic_load",
 [TypePair[1], PointerType, GenericAS>]>;
+def : Builtin<"atomic_load_explicit",
+[TypePair[1], PointerType, GenericAS>, 
MemoryOrder]>;
+def : Builtin<"atomic_load_explicit",
+[TypePair[1], PointerType, GenericAS>, 
MemoryOrder, MemoryScope]>;
 def : Builtin<"atomic_exchange",
 [TypePair[1], PointerType, GenericAS>, 
TypePair[1]]>;
+def : Builtin<"atomic_exchange_explicit",
+[TypePair[1], PointerType, GenericAS>, 
TypePair[1], MemoryOrder]>;
+def : Builtin<"atomic_exchange_explicit",
+[TypePair[1], PointerType, GenericAS>, 
TypePair[1], MemoryOrder, MemoryScope]>;
 foreach Variant = ["weak", "strong"] in {
   def :

[clang] 0344aea - [OpenCL] Add ndrange builtin functions to TableGen

[Sven van Haastregt via cfe-commits]

Author: Sven van Haastregt
Date: 2021-02-24T09:27:36Z
New Revision: 0344aea6ea379d945d1de1f5c258414dc61ccacd

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

LOG: [OpenCL] Add ndrange builtin functions to TableGen

Also ensure all kernel enqueue functions have CL 2.0 as minimum
version.

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

Added: 


Modified: 
clang/lib/Sema/OpenCLBuiltins.td
clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl

Removed: 




diff  --git a/clang/lib/Sema/OpenCLBuiltins.td 
b/clang/lib/Sema/OpenCLBuiltins.td
index 77d0203629eb..f2a9b98196e8 100644
--- a/clang/lib/Sema/OpenCLBuiltins.td
+++ b/clang/lib/Sema/OpenCLBuiltins.td
@@ -311,6 +311,7 @@ def Queue : Type<"queue_t", 
QualType<"Context.OCLQueueTy">>;
 def ReserveId : Type<"reserve_id_t", 
QualType<"Context.OCLReserveIDTy">>;
 def MemFenceFlags : TypedefType<"cl_mem_fence_flags">;
 def ClkProfilingInfo  : TypedefType<"clk_profiling_info">;
+def NDRange   : TypedefType<"ndrange_t">;
 
 // OpenCL v2.0 s6.13.11: Atomic integer and floating-point types.
 def AtomicInt : Type<"atomic_int", 
QualType<"Context.getAtomicType(Context.IntTy)">>;
@@ -1353,21 +1354,38 @@ def : Builtin<"is_valid_reserve_id", [Bool, ReserveId]>;
 // Defined in Builtins.def
 
 // --- Table 33 ---
-def : Builtin<"enqueue_marker",
-[Int, Queue, UInt, PointerType, GenericAS>, 
PointerType]>;
-
-// --- Table 34 ---
-def : Builtin<"retain_event", [Void, ClkEvent]>;
-def : Builtin<"release_event", [Void, ClkEvent]>;
-def : Builtin<"create_user_event", [ClkEvent]>;
-def : Builtin<"is_valid_event", [Bool, ClkEvent]>;
-def : Builtin<"set_user_event_status", [Void, ClkEvent, Int]>;
-def : Builtin<"capture_event_profiling_info",
-[Void, ClkEvent, ClkProfilingInfo, PointerType]>;
-
-// --- Table 35 ---
-def : Builtin<"get_default_queue", [Queue]>;
-// TODO: ndrange functions
+let MinVersion = CL20 in {
+  def : Builtin<"enqueue_marker",
+  [Int, Queue, UInt, PointerType, GenericAS>, 
PointerType]>;
+
+  // --- Table 34 ---
+  def : Builtin<"retain_event", [Void, ClkEvent]>;
+  def : Builtin<"release_event", [Void, ClkEvent]>;
+  def : Builtin<"create_user_event", [ClkEvent]>;
+  def : Builtin<"is_valid_event", [Bool, ClkEvent]>;
+  def : Builtin<"set_user_event_status", [Void, ClkEvent, Int]>;
+  def : Builtin<"capture_event_profiling_info",
+  [Void, ClkEvent, ClkProfilingInfo, PointerType]>;
+
+  // --- Table 35 ---
+  def : Builtin<"get_default_queue", [Queue]>;
+
+  def : Builtin<"ndrange_1D", [NDRange, Size]>;
+  def : Builtin<"ndrange_1D", [NDRange, Size, Size]>;
+  def : Builtin<"ndrange_1D", [NDRange, Size, Size, Size]>;
+  def : Builtin<"ndrange_2D", [NDRange, PointerType, 
PrivateAS>]>;
+  def : Builtin<"ndrange_2D", [NDRange, PointerType, 
PrivateAS>,
+PointerType, 
PrivateAS>]>;
+  def : Builtin<"ndrange_2D", [NDRange, PointerType, 
PrivateAS>,
+PointerType, 
PrivateAS>,
+PointerType, 
PrivateAS>]>;
+  def : Builtin<"ndrange_3D", [NDRange, PointerType, 
PrivateAS>]>;
+  def : Builtin<"ndrange_3D", [NDRange, PointerType, 
PrivateAS>,
+PointerType, 
PrivateAS>]>;
+  def : Builtin<"ndrange_3D", [NDRange, PointerType, 
PrivateAS>,
+PointerType, 
PrivateAS>,
+PointerType, 
PrivateAS>]>;
+}
 
 
 //

diff  --git a/clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl 
b/clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
index b7a1d6fbbcf1..9a5ed77b1635 100644
--- a/clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ b/clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -51,6 +51,8 @@ typedef int clk_profiling_info;
 typedef uint cl_mem_fence_flags;
 #define CLK_GLOBAL_MEM_FENCE 0x02
 
+typedef struct {int a;} ndrange_t;
+
 // Enable extensions that are enabled in opencl-c-base.h.
 #if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
 #define cl_khr_subgroup_extended_types 1
@@ -88,6 +90,9 @@ void test_typedef_args(clk_event_t evt, volatile atomic_flag 
*flg, global unsign
 
   atomic_flag_clear(flg);
   bool result = atomic_flag_test_and_set(flg);
+
+  size_t ws[2] = {2, 8};
+  ndrange_t r = ndrange_2D(ws);
 }
 #endif
 



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

[PATCH] D96860: [OpenCL] Add declarations with enum/typedef args

[Sven van Haastregt via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG85eb12eefdf6: [OpenCL] Add declarations with enum/typedef 
args (authored by svenvh).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96860

Files:
  clang/lib/Sema/OpenCLBuiltins.td
  clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl

Index: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -45,6 +45,9 @@
 typedef uint uint4 __attribute__((ext_vector_type(4)));
 typedef long long2 __attribute__((ext_vector_type(2)));
 
+typedef int clk_profiling_info;
+#define CLK_PROFILING_COMMAND_EXEC_TIME 0x1
+
 typedef uint cl_mem_fence_flags;
 #define CLK_GLOBAL_MEM_FENCE 0x02
 
@@ -79,6 +82,15 @@
 }
 #endif
 
+#if defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200
+void test_typedef_args(clk_event_t evt, volatile atomic_flag *flg, global unsigned long long *values) {
+  capture_event_profiling_info(evt, CLK_PROFILING_COMMAND_EXEC_TIME, values);
+
+  atomic_flag_clear(flg);
+  bool result = atomic_flag_test_and_set(flg);
+}
+#endif
+
 kernel void basic_conversion() {
   double d;
   float f;
@@ -167,6 +179,11 @@
   // expected-error@-2{{implicit declaration of function 'get_sub_group_size' is invalid in OpenCL}}
   // expected-error@-3{{implicit conversion changes signedness}}
 #endif
+
+// Only test when the base header is included, because we need the enum declarations.
+#if !defined(NO_HEADER) && (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
+  sub_group_barrier(CLK_GLOBAL_MEM_FENCE, memory_scope_device);
+#endif
 }
 
 kernel void extended_subgroup(global uint4 *out, global int *scalar, global char2 *c2) {
Index: clang/lib/Sema/OpenCLBuiltins.td
===
--- clang/lib/Sema/OpenCLBuiltins.td
+++ clang/lib/Sema/OpenCLBuiltins.td
@@ -310,6 +310,7 @@
 def Queue : Type<"queue_t", QualType<"Context.OCLQueueTy">>;
 def ReserveId : Type<"reserve_id_t", QualType<"Context.OCLReserveIDTy">>;
 def MemFenceFlags : TypedefType<"cl_mem_fence_flags">;
+def ClkProfilingInfo  : TypedefType<"clk_profiling_info">;
 
 // OpenCL v2.0 s6.13.11: Atomic integer and floating-point types.
 def AtomicInt : Type<"atomic_int", QualType<"Context.getAtomicType(Context.IntTy)">>;
@@ -323,6 +324,7 @@
 def AtomicSize: Type<"atomic_size_t", QualType<"Context.getAtomicType(Context.getSizeType())">>;
 def AtomicPtrDiff : Type<"atomic_ptrdiff_t", QualType<"Context.getAtomicType(Context.getPointerDiffType())">>;
 
+def AtomicFlag: TypedefType<"atomic_flag">;
 def MemoryOrder   : EnumType<"memory_order">;
 def MemoryScope   : EnumType<"memory_scope">;
 
@@ -913,6 +915,26 @@
 
 // OpenCL v3.0 s6.15.8 - Synchronization Functions.
 def : Builtin<"barrier", [Void, MemFenceFlags], Attr.Convergent>;
+let MinVersion = CL20 in {
+  def : Builtin<"work_group_barrier", [Void, MemFenceFlags], Attr.Convergent>;
+  def : Builtin<"work_group_barrier", [Void, MemFenceFlags, MemoryScope], Attr.Convergent>;
+}
+
+// OpenCL v3.0 s6.15.9 - Legacy Explicit Memory Fence Functions.
+def : Builtin<"mem_fence", [Void, MemFenceFlags]>;
+def : Builtin<"read_mem_fence", [Void, MemFenceFlags]>;
+def : Builtin<"write_mem_fence", [Void, MemFenceFlags]>;
+
+// OpenCL v3.0 s6.15.10 - Address Space Qualifier Functions.
+// to_global, to_local, to_private are declared in Builtins.def.
+
+let MinVersion = CL20 in {
+  // The OpenCL 3.0 specification defines these with a "gentype" argument indicating any builtin
+  // type or user-defined type, which cannot be represented currently.  Hence we slightly diverge
+  // by providing only the following overloads with a void pointer.
+  def : Builtin<"get_fence", [MemFenceFlags, PointerType]>;
+  def : Builtin<"get_fence", [MemFenceFlags, PointerType, GenericAS>]>;
+}
 
 //
 // OpenCL v1.1 s6.11.10, v1.2 s6.12.10, v2.0 s6.13.10: Async Copies from Global to Local Memory, Local to Global Memory, and Prefetch
@@ -1030,6 +1052,8 @@
 }
 // OpenCL v2.0 s6.13.11 - Atomic Functions.
 let MinVersion = CL20 in {
+  def : Builtin<"atomic_work_item_fence", [Void, MemFenceFlags, MemoryOrder, MemoryScope]>;
+
   foreach TypePair = [[AtomicInt, Int], [AtomicUInt, UInt],
   [AtomicLong, Long], [AtomicULong, ULong],
   [AtomicFloat, Float], [AtomicDouble, Double]] in {
@@ -1037,10 +1061,22 @@
 [Void, PointerType, GenericAS>, TypePair[1]]>;
 def : Builtin<"atomic_store",
 [Void, PointerType, GenericAS>, TypePair[1]]>;
+def : Builtin<"atomic_store_explicit",
+[Void, PointerType, GenericAS>, T

[PATCH] D97060: [OpenCL] Add ndrange builtin functions to TableGen

[Sven van Haastregt 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 rG0344aea6ea37: [OpenCL] Add ndrange builtin functions to 
TableGen (authored by svenvh).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97060

Files:
  clang/lib/Sema/OpenCLBuiltins.td
  clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl


Index: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -51,6 +51,8 @@
 typedef uint cl_mem_fence_flags;
 #define CLK_GLOBAL_MEM_FENCE 0x02
 
+typedef struct {int a;} ndrange_t;
+
 // Enable extensions that are enabled in opencl-c-base.h.
 #if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
 #define cl_khr_subgroup_extended_types 1
@@ -88,6 +90,9 @@
 
   atomic_flag_clear(flg);
   bool result = atomic_flag_test_and_set(flg);
+
+  size_t ws[2] = {2, 8};
+  ndrange_t r = ndrange_2D(ws);
 }
 #endif
 
Index: clang/lib/Sema/OpenCLBuiltins.td
===
--- clang/lib/Sema/OpenCLBuiltins.td
+++ clang/lib/Sema/OpenCLBuiltins.td
@@ -311,6 +311,7 @@
 def ReserveId : Type<"reserve_id_t", 
QualType<"Context.OCLReserveIDTy">>;
 def MemFenceFlags : TypedefType<"cl_mem_fence_flags">;
 def ClkProfilingInfo  : TypedefType<"clk_profiling_info">;
+def NDRange   : TypedefType<"ndrange_t">;
 
 // OpenCL v2.0 s6.13.11: Atomic integer and floating-point types.
 def AtomicInt : Type<"atomic_int", 
QualType<"Context.getAtomicType(Context.IntTy)">>;
@@ -1353,21 +1354,38 @@
 // Defined in Builtins.def
 
 // --- Table 33 ---
-def : Builtin<"enqueue_marker",
-[Int, Queue, UInt, PointerType, GenericAS>, 
PointerType]>;
-
-// --- Table 34 ---
-def : Builtin<"retain_event", [Void, ClkEvent]>;
-def : Builtin<"release_event", [Void, ClkEvent]>;
-def : Builtin<"create_user_event", [ClkEvent]>;
-def : Builtin<"is_valid_event", [Bool, ClkEvent]>;
-def : Builtin<"set_user_event_status", [Void, ClkEvent, Int]>;
-def : Builtin<"capture_event_profiling_info",
-[Void, ClkEvent, ClkProfilingInfo, PointerType]>;
-
-// --- Table 35 ---
-def : Builtin<"get_default_queue", [Queue]>;
-// TODO: ndrange functions
+let MinVersion = CL20 in {
+  def : Builtin<"enqueue_marker",
+  [Int, Queue, UInt, PointerType, GenericAS>, 
PointerType]>;
+
+  // --- Table 34 ---
+  def : Builtin<"retain_event", [Void, ClkEvent]>;
+  def : Builtin<"release_event", [Void, ClkEvent]>;
+  def : Builtin<"create_user_event", [ClkEvent]>;
+  def : Builtin<"is_valid_event", [Bool, ClkEvent]>;
+  def : Builtin<"set_user_event_status", [Void, ClkEvent, Int]>;
+  def : Builtin<"capture_event_profiling_info",
+  [Void, ClkEvent, ClkProfilingInfo, PointerType]>;
+
+  // --- Table 35 ---
+  def : Builtin<"get_default_queue", [Queue]>;
+
+  def : Builtin<"ndrange_1D", [NDRange, Size]>;
+  def : Builtin<"ndrange_1D", [NDRange, Size, Size]>;
+  def : Builtin<"ndrange_1D", [NDRange, Size, Size, Size]>;
+  def : Builtin<"ndrange_2D", [NDRange, PointerType, 
PrivateAS>]>;
+  def : Builtin<"ndrange_2D", [NDRange, PointerType, 
PrivateAS>,
+PointerType, 
PrivateAS>]>;
+  def : Builtin<"ndrange_2D", [NDRange, PointerType, 
PrivateAS>,
+PointerType, 
PrivateAS>,
+PointerType, 
PrivateAS>]>;
+  def : Builtin<"ndrange_3D", [NDRange, PointerType, 
PrivateAS>]>;
+  def : Builtin<"ndrange_3D", [NDRange, PointerType, 
PrivateAS>,
+PointerType, 
PrivateAS>]>;
+  def : Builtin<"ndrange_3D", [NDRange, PointerType, 
PrivateAS>,
+PointerType, 
PrivateAS>,
+PointerType, 
PrivateAS>]>;
+}
 
 
 //


Index: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -51,6 +51,8 @@
 typedef uint cl_mem_fence_flags;
 #define CLK_GLOBAL_MEM_FENCE 0x02
 
+typedef struct {int a;} ndrange_t;
+
 // Enable extensions that are enabled in opencl-c-base.h.
 #if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
 #define cl_khr_subgroup_extended_types 1
@@ -88,6 +90,9 @@
 
   atomic_flag_clear(flg);
   bool result = atomic_flag_test_and_set(flg);
+
+  size_t ws[2] = {2, 8};
+  ndrange_t r = ndrange_2D(ws);
 }
 #endif
 
Index: clang/lib/Sema/OpenCLBuiltins.td
===
--- clang/lib/Sema/OpenCLBuiltins.td
+++ c

[PATCH] D61112: AMDGPU: Enable _Float16

[Jay Foad via Phabricator via cfe-commits]

foad added a comment.
Herald added a subscriber: kerbowa.

Should have updated 
https://clang.llvm.org/docs/LanguageExtensions.html#half-precision-floating-point
 "_Float16 is currently only supported on the following targets, with further 
targets pending ABI standardization: ..."


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

https://reviews.llvm.org/D61112

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

[PATCH] D97366: [clangd] Fix a race

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97366

Files:
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp


Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -314,8 +314,14 @@
 ~AsyncCounter() {
   // Verify shutdown sequence was performed.
   // Real modules would not do this, to be robust to no ClangdServer.
-  EXPECT_TRUE(ShouldStop) << "ClangdServer should request shutdown";
-  EXPECT_EQ(Queue.size(), 0u) << "ClangdServer should block until idle";
+  {
+// We still need the lock here, as Queue might be empty when
+// ClangdServer calls blockUntilIdle, but run() might not have returned
+// yet.
+std::lock_guard Lock(Mu);
+EXPECT_TRUE(ShouldStop) << "ClangdServer should request shutdown";
+EXPECT_EQ(Queue.size(), 0u) << "ClangdServer should block until idle";
+  }
   Thread.join();
 }
 


Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -314,8 +314,14 @@
 ~AsyncCounter() {
   // Verify shutdown sequence was performed.
   // Real modules would not do this, to be robust to no ClangdServer.
-  EXPECT_TRUE(ShouldStop) << "ClangdServer should request shutdown";
-  EXPECT_EQ(Queue.size(), 0u) << "ClangdServer should block until idle";
+  {
+// We still need the lock here, as Queue might be empty when
+// ClangdServer calls blockUntilIdle, but run() might not have returned
+// yet.
+std::lock_guard Lock(Mu);
+EXPECT_TRUE(ShouldStop) << "ClangdServer should request shutdown";
+EXPECT_EQ(Queue.size(), 0u) << "ClangdServer should block until idle";
+  }
   Thread.join();
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97366: [clangd] Fix a race

[Sam McCall via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97366

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

[PATCH] D96638: [Driver][Windows] Support per-target runtimes dir layout for profile instr generate

[Markus Böck via Phabricator via cfe-commits]

zero9178 updated this revision to Diff 326022.
zero9178 added a comment.

Undid refactoring of getCompilerRTBasename. Previous diffs introduced a new 
internal function that built the basename. In that version 
getCompilerRTBasename simply called getCompilerRT and extracted the filename 
component.

This lead to issues as Toolchains such as Baremetal override 
getCompilerRTBasename to provide a different basename then default.

This diff now instead changes the default implementation of 
getCompilerRTBasename to also auto detect whether an architecture suffix should 
be used or not. Therefore callers of getCompilerRTBasename can rely on it 
picking the right filenames as well. getCompilerRT still calls into 
getCompilerRTBasename to get the filename, allowing Toolchains to override the 
virtual function.

There is now sadly a bit of repetition as both getCompilerRT and 
getCompilerRTBasename have to look through the library path but this should not 
cause any issues I believe.

Sorry for the inconvenience and the test failure.


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

https://reviews.llvm.org/D96638

Files:
  clang/lib/Driver/ToolChain.cpp


Index: clang/lib/Driver/ToolChain.cpp
===
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -22,6 +22,7 @@
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
 #include "clang/Driver/XRayArgs.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
@@ -413,12 +414,21 @@
   return std::string(Path.str());
 }
 
-static std::string buildCompilerRTBasename(const ToolChain &toolchain,
-   const ArgList &Args,
-   StringRef Component,
-   ToolChain::FileType Type,
-   bool AddArch) {
-  const llvm::Triple &TT = toolchain.getTriple();
+static Optional findInLibraryPath(const ToolChain &TC,
+   StringRef filename) {
+  for (const auto &LibPath : TC.getLibraryPaths()) {
+SmallString<128> P(LibPath);
+llvm::sys::path::append(P, filename);
+if (TC.getVFS().exists(P))
+  return std::string(P.str());
+  }
+  return llvm::None;
+}
+
+std::string ToolChain::getCompilerRTBasename(const ArgList &Args,
+ StringRef Component,
+ FileType Type) const {
+  const llvm::Triple &TT = getTriple();
   bool IsITANMSVCWindows =
   TT.isWindowsMSVCEnvironment() || TT.isWindowsItaniumEnvironment();
 
@@ -433,44 +443,35 @@
 Suffix = IsITANMSVCWindows ? ".lib" : ".a";
 break;
   case ToolChain::FT_Shared:
-Suffix = TT.isOSWindows()
- ? (TT.isWindowsGNUEnvironment() ? ".dll.a" : ".lib")
+Suffix = Triple.isOSWindows()
+ ? (Triple.isWindowsGNUEnvironment() ? ".dll.a" : ".lib")
  : ".so";
 break;
   }
 
-  std::string ArchAndEnv;
-  if (AddArch) {
-StringRef Arch = getArchNameForCompilerRTLib(toolchain, Args);
-const char *Env = TT.isAndroid() ? "-android" : "";
-ArchAndEnv = ("-" + Arch + Env).str();
+  std::string CRTWithoutArch =
+  (Prefix + Twine("clang_rt.") + Component + Suffix).str();
+  if (findInLibraryPath(*this, CRTWithoutArch).hasValue()) {
+return CRTWithoutArch;
   }
-  return (Prefix + Twine("clang_rt.") + Component + ArchAndEnv + Suffix).str();
-}
 
-std::string ToolChain::getCompilerRTBasename(const ArgList &Args,
- StringRef Component,
- FileType Type) const {
-  std::string absolutePath = getCompilerRT(Args, Component, Type);
-  return llvm::sys::path::filename(absolutePath).str();
+  StringRef Arch = getArchNameForCompilerRTLib(*this, Args);
+  const char *Env = TT.isAndroid() ? "-android" : "";
+  std::string ArchAndEnv = ("-" + Arch + Env).str();
+
+  return (Prefix + Twine("clang_rt.") + Component + ArchAndEnv + Suffix).str();
 }
 
 std::string ToolChain::getCompilerRT(const ArgList &Args, StringRef Component,
  FileType Type) const {
   // Check for runtime files in the new layout without the architecture first.
-  std::string CRTBasename =
-  buildCompilerRTBasename(*this, Args, Component, Type, /*AddArch=*/false);
-  for (const auto &LibPath : getLibraryPaths()) {
-SmallString<128> P(LibPath);
-llvm::sys::path::append(P, CRTBasename);
-if (getVFS().exists(P))
-  return std::string(P.str());
+  std::string CRTBasename = getCompilerRTBasename(Args, Component, Type);
+  if (auto value = findInLibraryPath(*this, CRTBasename)) {
+return value.getValue();
   }
 
   // Fall back to the old expected compiler-rt name if the new one does n

[PATCH] D97072: [OpenCL][Docs] Add guidelines for adding new extensions and features

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added a comment.

Some minor typos and requests for clarifications, looks like reasonable 
guidelines other than that.




Comment at: clang/docs/OpenCLSupport.rst:175
+
+If extensions modifies the standard parsing they need to be added to
+the clang frontend source code. This also means that the associate macro

Probably easier to keep this section singular (i.e. talk about a adding a 
single extension), instead of switching between extension and extensions all 
the time.



Comment at: clang/docs/OpenCLSupport.rst:176
+If extensions modifies the standard parsing they need to be added to
+the clang frontend source code. This also means that the associate macro
+indicating the presence of the extensions should be added to clang.

associated



Comment at: clang/docs/OpenCLSupport.rst:177
+the clang frontend source code. This also means that the associate macro
+indicating the presence of the extensions should be added to clang.
+

Again, keep it singular.



Comment at: clang/docs/OpenCLSupport.rst:179
+
+The default flow for adding the new extensions into the frontend is to
+modify `OpenCLExtensions.def





Comment at: clang/docs/OpenCLSupport.rst:195
+
+If an extension adds the functionality that does not modify standard language
+parsing it should not require clang source code modifications. Most commonly





Comment at: clang/docs/OpenCLSupport.rst:196
+If an extension adds the functionality that does not modify standard language
+parsing it should not require clang source code modifications. Most commonly
+such extensions add functionality via libraries (by adding new non-native

This might need some rephrasing to take TableGen BIFs into account.  I would 
consider `clang/lib/Sema/OpenCLBuiltins.td` an integral part of the clang 
source code.

Maybe rephrase "clang source code modifications" into "clang parser source code 
modifications"?



Comment at: clang/docs/OpenCLSupport.rst:201
+
+Clang has standard headers where new types and functions are being added,
+for more details refer to

Should this mention `clang/lib/Sema/OpenCLBuiltins.td` too?



Comment at: clang/docs/OpenCLSupport.rst:212
+
+Clang provides mechanism to add the standard extension pragma
+``OPENCL EXTENSION`` by setting a dedicated flag in the extension list entry of





Comment at: clang/docs/OpenCLSupport.rst:216
+standard extension pragmas as it is not specified (for the standards up to and
+including version 3.0) with the sufficient level of details and, therefore,
+there is no default functionality provided by clang.





Comment at: clang/docs/OpenCLSupport.rst:219
+
+Pragmas without detailed information of its behavior (explanation of changes it
+triggers in the parsing) should not be added to clang. Moreover, the acceptable





Comment at: clang/docs/OpenCLSupport.rst:221
+triggers in the parsing) should not be added to clang. Moreover, the acceptable
+behavior of pragmas should provide useful functionality to the user.
+

Remove "acceptable behavior of".

How do you define "useful functionality"?



Comment at: clang/docs/OpenCLSupport.rst:226
+the use of types or functions. This functionality is not guaranteed to remain 
in
+the future releases, however, any future changes should not affect backward
+compatibility.




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

https://reviews.llvm.org/D97072

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

[PATCH] D97137: Bug fix of clang-format, the AlignConsecutiveDeclarations option doesn't handle pointer properly

[Darwin Xu via Phabricator via cfe-commits]

darwin added a comment.

Hi guys, thanks for accepting the change. But I don't have have commit access 
of LLVM. Can someone commit it for me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97137

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

[PATCH] D97370: [clang][cli] Remove marshalling from Opt{In, Out}FFlag

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith.
Herald added a subscriber: dang.
jansvoboda11 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We can now express all marshalling semantics in `Opt{In,Out}FFlag` via 
`BoolFOption`.

This patch moves remaining `Opt{In,Out}FFlag` instances using marshalling to 
`BoolFOption` and removes marshalling capabilities from `Opt{In,Out}FFlag` 
entirely.

This simplifies the decisions developers have to make when creating new boolean 
options:

- For simple cc1 flag pairs, use `Bool{,F,G}Option`.
- For cc1 flag pairs that require complex marshalling logic, use 
`Opt{In,Out}FFlag` and implement marshalling manually.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97370

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp

Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1383,9 +1383,6 @@
   GenerateArg(Args, OPT_ftime_report, SA);
   }
 
-  if (Opts.FunctionSections)
-GenerateArg(Args, OPT_ffunction_sections, SA);
-
   if (Opts.PrepareForLTO && !Opts.PrepareForThinLTO)
 GenerateArg(Args, OPT_flto, SA);
 
@@ -1676,9 +1673,6 @@
 }
   }
 
-  // Basic Block Sections implies Function Sections.
-  Opts.FunctionSections = Args.hasArg(OPT_ffunction_sections);
-
   Opts.PrepareForLTO = Args.hasArg(OPT_flto, OPT_flto_EQ);
   Opts.PrepareForThinLTO = false;
   if (Arg *A = Args.getLastArg(OPT_flto_EQ)) {
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -267,32 +267,26 @@
   : KeyPathAndMacro<"MigratorOpts.", base, "MIGRATOR_"> {}
 
 // A boolean option which is opt-in in CC1. The positive option exists in CC1 and
-// Args.hasArg(OPT_ffoo) is used to check that the flag is enabled.
+// Args.hasArg(OPT_ffoo) can be used to check that the flag is enabled.
 // This is useful if the option is usually disabled.
+// Use this only when the option cannot be declared via BoolFOption.
 multiclass OptInFFlag flags=[],
-  KeyPathAndMacro kpm = EmptyKPM,
-  list enablers = []> {
+  string help="", list flags=[]> {
   def f#NAME : Flag<["-"], "f"#name>, Flags,
-   Group, HelpText,
-   MarshallingInfoFlag,
-   ImpliedByAnyOf;
+   Group, HelpText;
   def fno_#NAME : Flag<["-"], "fno-"#name>, Flags,
-   Group, HelpText;
+  Group, HelpText;
 }
 
 // A boolean option which is opt-out in CC1. The negative option exists in CC1 and
-// Args.hasArg(OPT_fno_foo) is used to check that the flag is disabled.
+// Args.hasArg(OPT_fno_foo) can be used to check that the flag is disabled.
+// Use this only when the option cannot be declared via BoolFOption.
 multiclass OptOutFFlag flags=[],
-   KeyPathAndMacro kpm = EmptyKPM,
-   list disablers = []> {
+   string help="", list flags=[]> {
   def f#NAME : Flag<["-"], "f"#name>, Flags,
Group, HelpText;
   def fno_#NAME : Flag<["-"], "fno-"#name>, Flags,
-   Group, HelpText,
-   MarshallingInfoFlag,
-   ImpliedByAnyOf;
+  Group, HelpText;
 }
 
 //===--===//
@@ -892,9 +886,10 @@
   NegFlag>;
 def : Flag<["-"], "fcuda-rdc">, Alias;
 def : Flag<["-"], "fno-cuda-rdc">, Alias;
-defm cuda_short_ptr : OptInFFlag<"cuda-short-ptr",
-  "Use 32-bit pointers for accessing const/local/shared address spaces", "", "",
-  [], TargetOpts<"NVPTXUseShortPointers">>;
+defm cuda_short_ptr : BoolFOption<"cuda-short-ptr",
+  TargetOpts<"NVPTXUseShortPointers">, DefaultFalse,
+  PosFlag,
+  NegFlag>;
 def rocm_path_EQ : Joined<["--"], "rocm-path=">, Group,
   HelpText<"ROCm installation path, used for finding and automatically linking required bitcode libraries.">;
 def rocm_device_lib_path_EQ : Joined<["--"], "rocm-device-lib-path=">, Group,
@@ -1381,8 +1376,11 @@
   Values<"ignore,maytrap,strict">, NormalizedValuesScope<"LangOptions">,
   NormalizedValues<["FPE_Ignore", "FPE_MayTrap", "FPE_Strict"]>,
   MarshallingInfoString, "FPE_Ignore">, AutoNormalizeEnum;
-defm fast_math : OptInFFlag<"fast-math", "Allow aggressive, lossy floating-point optimizations", "", "", [],
-  LangOpts<"FastMath">, [cl_fast_relaxed_math.KeyPath]>;
+defm fast_math : BoolFOption<"fast-math",
+  LangOpts<"FastMath">, DefaultFalse,
+  PosFlag,
+  NegFlag>;
 def menable_unsafe_fp_math : Flag<["-"], "menable-unsafe-fp-math">, Flags<[CC1Option]>,
   HelpText<"Allow unsafe floating-point math optimizations which ma

[PATCH] D97371: [clang][parser] Remove questionable ProhibitAttributes() call in objc parsing

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

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

Clang uses GNU-style attributes in objc code in (for example, I guess?) 
`tests/Parser/stmt-attributes.m`:

  __attribute__((nomerge)) @try {
[getTest() foo];
  } @finally {
  }

Once the `ProhibitAttributes()` call in question actually starts handling 
GNU-style attributes, these tests fail.

The call was introduced in c202b2809ac814bcae8553cd772ec4901fdb8441 by Richard 
Smith (I'll add him as a reviewer), but with a `TODO` comment stating that it 
might be incorrect.

I'm having a hard time finding any concrete information on whether GNU-style 
attributes on `@try`/`@catch` statements are allowed in objc, so maybe someone 
with more experience can comment?

Thanks,
Timm


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97371

Files:
  clang/lib/Parse/ParseStmt.cpp


Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -172,7 +172,6 @@
   switch (Kind) {
   case tok::at: // May be a @try or @throw statement
 {
-  ProhibitAttributes(Attrs); // TODO: is it correct?
   AtLoc = ConsumeToken();  // consume @
   return ParseObjCAtStatement(AtLoc, StmtCtx);
 }


Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -172,7 +172,6 @@
   switch (Kind) {
   case tok::at: // May be a @try or @throw statement
 {
-  ProhibitAttributes(Attrs); // TODO: is it correct?
   AtLoc = ConsumeToken();  // consume @
   return ParseObjCAtStatement(AtLoc, StmtCtx);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D96638: [Driver][Windows] Support per-target runtimes dir layout for profile instr generate

[Markus Böck via Phabricator via cfe-commits]

zero9178 requested review of this revision.
zero9178 added a comment.

I think this change, while functionality fixing the same as my previous diff, 
but also fixing the test failure, does deviate a bit from the original review, 
so I'd like it to be reviewed again if that isn't a problem.


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

https://reviews.llvm.org/D96638

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

[PATCH] D97366: [clangd] Fix a race

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc94ecf3f81ca: [clangd] Fix a race (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97366

Files:
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp


Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -314,8 +314,14 @@
 ~AsyncCounter() {
   // Verify shutdown sequence was performed.
   // Real modules would not do this, to be robust to no ClangdServer.
-  EXPECT_TRUE(ShouldStop) << "ClangdServer should request shutdown";
-  EXPECT_EQ(Queue.size(), 0u) << "ClangdServer should block until idle";
+  {
+// We still need the lock here, as Queue might be empty when
+// ClangdServer calls blockUntilIdle, but run() might not have returned
+// yet.
+std::lock_guard Lock(Mu);
+EXPECT_TRUE(ShouldStop) << "ClangdServer should request shutdown";
+EXPECT_EQ(Queue.size(), 0u) << "ClangdServer should block until idle";
+  }
   Thread.join();
 }
 


Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -314,8 +314,14 @@
 ~AsyncCounter() {
   // Verify shutdown sequence was performed.
   // Real modules would not do this, to be robust to no ClangdServer.
-  EXPECT_TRUE(ShouldStop) << "ClangdServer should request shutdown";
-  EXPECT_EQ(Queue.size(), 0u) << "ClangdServer should block until idle";
+  {
+// We still need the lock here, as Queue might be empty when
+// ClangdServer calls blockUntilIdle, but run() might not have returned
+// yet.
+std::lock_guard Lock(Mu);
+EXPECT_TRUE(ShouldStop) << "ClangdServer should request shutdown";
+EXPECT_EQ(Queue.size(), 0u) << "ClangdServer should block until idle";
+  }
   Thread.join();
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] c94ecf3 - [clangd] Fix a race

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2021-02-24T12:15:16+01:00
New Revision: c94ecf3f81ca42a98b3b279a7b210b67715f1c41

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

LOG: [clangd] Fix a race

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

Added: 


Modified: 
clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp 
b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
index 4542f7423a48..695ed89ae7f4 100644
--- a/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -314,8 +314,14 @@ TEST_F(LSPTest, ModulesThreadingTest) {
 ~AsyncCounter() {
   // Verify shutdown sequence was performed.
   // Real modules would not do this, to be robust to no ClangdServer.
-  EXPECT_TRUE(ShouldStop) << "ClangdServer should request shutdown";
-  EXPECT_EQ(Queue.size(), 0u) << "ClangdServer should block until idle";
+  {
+// We still need the lock here, as Queue might be empty when
+// ClangdServer calls blockUntilIdle, but run() might not have returned
+// yet.
+std::lock_guard Lock(Mu);
+EXPECT_TRUE(ShouldStop) << "ClangdServer should request shutdown";
+EXPECT_EQ(Queue.size(), 0u) << "ClangdServer should block until idle";
+  }
   Thread.join();
 }
 



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

[PATCH] D97358: [X86] Support amx-bf16 intrinsic.

[LiuChen via Phabricator via cfe-commits]

LiuChen3 added a comment.

I don't know why pre-merge-checks failed. I can check-all successfully locally 
in redhat8. I don't have  debian mainchine to reproduce this problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97358

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

[PATCH] D97375: [clang][cli] Add MarshallingInfoEnum multiclass

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith.
Herald added a subscriber: dang.
jansvoboda11 requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This patch introduces a tablegen multiclass called `MarshallingInfoEnum`. It 
has the same semantics as `MarshallingInfoString` had in combination with 
`AutoNormalizeEnum`, but it's easier to use and follows the convention used for 
other `MarshallingInfoXxx` multiclasses.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97375

Files:
  clang/docs/InternalsManual.rst
  clang/include/clang/Driver/Options.td
  llvm/include/llvm/Option/OptParser.td

Index: llvm/include/llvm/Option/OptParser.td
===
--- llvm/include/llvm/Option/OptParser.td
+++ llvm/include/llvm/Option/OptParser.td
@@ -211,6 +211,14 @@
   code Denormalizer = "makeBooleanOptionDenormalizer("#value#")";
 }
 
+// Marshalling info for enums. Typically used with `Values`, `NormalizedValues`
+// and `NormalizedValuesScope` mixins.
+class MarshallingInfoEnum
+  : MarshallingInfo {
+  code Normalizer = "normalizeSimpleEnum";
+  code Denormalizer = "denormalizeSimpleEnum";
+}
+
 // Mixins for additional marshalling attributes.
 
 class ShouldParseIf { code ShouldParse = condition; }
@@ -219,10 +227,6 @@
 class Denormalizer { code Denormalizer = denormalizer; }
 class NormalizedValuesScope { code NormalizedValuesScope = scope; }
 class NormalizedValues definitions> { list NormalizedValues = definitions; } 
-class AutoNormalizeEnum {
-  code Normalizer = "normalizeSimpleEnum";
-  code Denormalizer = "denormalizeSimpleEnum";
-}
 class ValueMerger { code ValueMerger = merger; }
 class ValueExtractor { code ValueExtractor = extractor; }
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -427,11 +427,10 @@
 
 // FIXME: Diagnose if target does not support protected visibility.
 class MarshallingInfoVisibility
-  : MarshallingInfoString,
+  : MarshallingInfoEnum,
 Values<"default,hidden,internal,protected">,
 NormalizedValues<["DefaultVisibility", "HiddenVisibility",
-  "HiddenVisibility", "ProtectedVisibility"]>,
-AutoNormalizeEnum {}
+  "HiddenVisibility", "ProtectedVisibility"]> {}
 
 // Key paths that are constant during parsing of options with the same key path prefix.
 defvar cplusplus = LangOpts<"CPlusPlus">;
@@ -1062,7 +1061,7 @@
 HelpText<"Embed LLVM bitcode (option: off, all, bitcode, marker)">,
 Values<"off,all,bitcode,marker">, NormalizedValuesScope<"CodeGenOptions">,
 NormalizedValues<["Embed_Off", "Embed_All", "Embed_Bitcode", "Embed_Marker"]>,
-MarshallingInfoString, "Embed_Off">, AutoNormalizeEnum;
+MarshallingInfoEnum, "Embed_Off">;
 def fembed_bitcode : Flag<["-"], "fembed-bitcode">, Group,
   Alias, AliasArgs<["all"]>,
   HelpText<"Embed LLVM IR bitcode as data">;
@@ -1248,7 +1247,7 @@
 Flags<[CC1Option]>, Values<"unspecified,standalone,objc,swift,swift-5.0,swift-4.2,swift-4.1">,
 NormalizedValuesScope<"LangOptions::CoreFoundationABI">,
 NormalizedValues<["ObjectiveC", "ObjectiveC", "ObjectiveC", "Swift5_0", "Swift5_0", "Swift4_2", "Swift4_1"]>,
-MarshallingInfoString, "ObjectiveC">, AutoNormalizeEnum;
+MarshallingInfoEnum, "ObjectiveC">;
 defm constant_cfstrings : BoolFOption<"constant-cfstrings",
   LangOpts<"NoConstantCFStrings">, DefaultFalse,
   NegFlag,
@@ -1356,8 +1355,7 @@
   Values<"dwarf,sjlj,seh,wasm">,
   NormalizedValuesScope<"llvm::ExceptionHandling">,
   NormalizedValues<["DwarfCFI", "SjLj", "WinEH", "Wasm"]>,
-  MarshallingInfoString, "None">,
-  AutoNormalizeEnum;
+  MarshallingInfoEnum, "None">;
 def exception_model_EQ : Joined<["-"], "exception-model=">,
   Flags<[CC1Option, NoDriverOption]>, Alias;
 def fignore_exceptions : Flag<["-"], "fignore-exceptions">, Group, Flags<[CC1Option]>,
@@ -1380,7 +1378,7 @@
   HelpText<"Specifies the exception behavior of floating-point operations.">,
   Values<"ignore,maytrap,strict">, NormalizedValuesScope<"LangOptions">,
   NormalizedValues<["FPE_Ignore", "FPE_MayTrap", "FPE_Strict"]>,
-  MarshallingInfoString, "FPE_Ignore">, AutoNormalizeEnum;
+  MarshallingInfoEnum, "FPE_Ignore">;
 defm fast_math : OptInFFlag<"fast-math", "Allow aggressive, lossy floating-point optimizations", "", "", [],
   LangOpts<"FastMath">, [cl_fast_relaxed_math.KeyPath]>;
 def menable_unsafe_fp_math : Flag<["-"], "menable-unsafe-fp-math">, Flags<[CC1Option]>,
@@ -1820,10 +1818,9 @@
   NormalizedValues<["LangOptions::LaxVectorConversionKind::None",
 "LangOptions::LaxVectorConversionKind::Integer",
 "LangOptions::LaxVectorConversionKind::All"]>,
-  MarshallingInfoString,
-!str

[PATCH] D97351: [clangd] Use flags from open files when opening headers they include

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 326042.
sammccall added a comment.
Herald added a subscriber: jfb.

Record memory usage


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97351

Files:
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang/include/clang/Tooling/CompilationDatabase.h
  clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp

Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -843,6 +843,18 @@
   EXPECT_EQ(getCommand("bar.h"), "clang -D bar.cpp --driver-mode=cl /TP");
 }
 
+TEST(TransferCompileCommandTest, Smoke) {
+  CompileCommand Cmd;
+  Cmd.Filename = "foo.cc";
+  Cmd.CommandLine = {"clang", "-Wall", "foo.cc"};
+  Cmd.Directory = "dir";
+  CompileCommand Transferred = transferCompileCommand(std::move(Cmd), "foo.h");
+  EXPECT_EQ(Transferred.Filename, "foo.h");
+  EXPECT_THAT(Transferred.CommandLine,
+  ElementsAre("clang", "-Wall", "-x", "c++-header", "foo.h"));
+  EXPECT_EQ(Transferred.Directory, "dir");
+}
+
 TEST(CompileCommandTest, EqualityOperator) {
   CompileCommand CCRef("/foo/bar", "hello.c", {"a", "b"}, "hello.o");
   CompileCommand CCTest = CCRef;
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -204,8 +204,10 @@
   }
 
   // Produce a CompileCommand for \p filename, based on this one.
-  CompileCommand transferTo(StringRef Filename) const {
-CompileCommand Result = Cmd;
+  // (This consumes the TransferableCommand just to avoid copying Cmd).
+  CompileCommand transferTo(StringRef Filename) && {
+CompileCommand Result = std::move(Cmd);
+Result.Heuristic = "inferred from " + Result.Filename;
 Result.Filename = std::string(Filename);
 bool TypeCertain;
 auto TargetType = guessType(Filename, &TypeCertain);
@@ -234,7 +236,6 @@
   LangStandard::getLangStandardForKind(Std).getName()).str());
 }
 Result.CommandLine.push_back(std::string(Filename));
-Result.Heuristic = "inferred from " + Cmd.Filename;
 return Result;
   }
 
@@ -521,7 +522,7 @@
 Inner->getCompileCommands(Index.chooseProxy(Filename, foldType(Lang)));
 if (ProxyCommands.empty())
   return {};
-return {TransferableCommand(ProxyCommands[0]).transferTo(Filename)};
+return {transferCompileCommand(std::move(ProxyCommands.front()), Filename)};
   }
 
   std::vector getAllFiles() const override {
@@ -544,5 +545,10 @@
   return std::make_unique(std::move(Inner));
 }
 
+tooling::CompileCommand transferCompileCommand(CompileCommand Cmd,
+   StringRef Filename) {
+  return TransferableCommand(std::move(Cmd)).transferTo(Filename);
+}
+
 } // namespace tooling
 } // namespace clang
Index: clang/include/clang/Tooling/CompilationDatabase.h
===
--- clang/include/clang/Tooling/CompilationDatabase.h
+++ clang/include/clang/Tooling/CompilationDatabase.h
@@ -213,6 +213,12 @@
   std::vector CompileCommands;
 };
 
+/// Transforms a compile command so that it applies the same configuration to
+/// a different file. Most args are left intact, but tweaks may be needed
+/// to certain flags (-x, -std etc).
+tooling::CompileCommand transferCompileCommand(tooling::CompileCommand,
+   StringRef Filename);
+
 /// Returns a wrapped CompilationDatabase that defers to the provided one,
 /// but getCompileCommands() will infer commands for unknown files.
 /// The return value of getAllFiles() or getAllCompileCommands() is unchanged.
Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -9,6 +9,7 @@
 #include "Annotations.h"
 #include "ClangdServer.h"
 #include "Diagnostics.h"
+#include "GlobalCompilationDatabase.h"
 #include "Matchers.h"
 #include "ParsedAST.h"
 #include "Preamble.h"
@@ -43,12 +44,15 @@
 namespace clangd {
 namespace {
 
+using ::testing::AllOf;
 using ::testing::AnyOf;
+using ::testing::Contains;
 using ::testing::Each;
 using ::testing::ElementsAre;
 using ::testing::Eq;
 using ::testing::Field;
 using ::testing::IsEmpty;
+using ::testing::Not;
 using ::testing::Pair;
 using ::testing::Pointee;
 using ::testing::SizeIs;
@@ -1161,6 +1165,103 @@
   Ready.

[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Basic/riscv_vector.td:148
+  // If HasMask, this flag states that this builtin has a maskedoff operand. It
+  // is always the first operand.
+  bit HasMaskedOffOperand = true;

Isn't mask the first operand maskedoff the second operand?



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:157
+  ArrayRef getIntrinsicTypes() const { return IntrinsicTypes; }
+  std::string getIRName() const { return IRName; }
+  uint8_t getRISCV_Extensions() const { return RISCV_Extensions; }

Return by const reference or use StringRef.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:175
+
+using TypeString = std::string;
+class RVVEmitter {

Why not just use std::string in the one place this is used?



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:862
+
+  // The same intrinsic name has the same switch body.
+  llvm::StringMap, 128>> DefsSet;

Might be better to just sort the vector by the IR name and then walk the vector 
looking for the boundaries where the name changes from the previous iteration.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:899
+// (operand) in ProtoSeq. ProtoSeq[0] is output operand.
+SmallVector ProtoSeq;
+const StringRef Primaries("evwqom0ztc");

I think this is something like

```
while (!Prototypes.empty()) {
 auto Idx = Prototypes.find_first_of(Primaries);
 assert(Idx != StringRef::npos);
 ProtoSeq.push_back(Prototypes.slice(0, Idx+1).str());
 Prototypes = Prototypes.drop_front(Idx+1);
}
```

Which might be easier to understand.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:970
+  if (It != LegalTypes.end())
+return Optional(&(It->second));
+  if (IllegalTypes.count(Idx))

Why does this need to explicitly create an Optional? Shouldn't we just be able 
to return &(It->second)?



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:990
+  // Collect the same extension intrinsic in the one set for arch guard marco.
+  DenseMap, 256>> DefsSet;
+  for (auto &def : Defs) {

Could we maybe just sort the original vector by extension and just loop over 
it. Keep track of the extensions from previous iteration and emit the guard 
when this iteration doesn't match the previous iteration.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1012
+
+SmallVector RVVEmitter::getExtStrings(uint8_t Extents) {
+  if (Extents == 0)

This is only called in one place which immediate loops over and prints the 
contents. Could we just pass the raw_ostream in here and do the printing 
directly?



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1015
+return {};
+  SmallVector ExtVector;
+  // D implies F

Even if we don't print directly. This can be a vector of StringRef. These are 
all string literals.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1016
+  SmallVector ExtVector;
+  // D implies F
+  if (Extents & RISCV_Extension::F) {

I don't understand this. It says D implies F but we're checking for F. So that 
seems like F implies D.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95016

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

[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:567
+  // Compute type transformers
+  for (char I : Transformer.take_front(Transformer.size() - 1)) {
+switch (I) {

Can we do Transformer = Transformer.drop_back() right before this loop. That 
take_front code is harder to think about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95016

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

[PATCH] D95691: Implement P2173 for attributes on lambdas

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D95691#2572114 , @aaron.ballman 
wrote:

> In D95691#2545808 , @aaron.ballman 
> wrote:
>
>> Updated based on review feedback.
>
> Ping

Ping


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

https://reviews.llvm.org/D95691

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

[clang] abbdb56 - [OpenCL] Allow taking address of functions as an extension.

[Anastasia Stulova via cfe-commits]

Author: Anastasia Stulova
Date: 2021-02-24T12:32:02Z
New Revision: abbdb5639c70d167bd66cd62296927330782c3b4

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

LOG: [OpenCL] Allow taking address of functions as an extension.

When '__cl_clang_function_pointers' extension is enabled
the parser should allow obtaining the function address.

This fixes PR49264!

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

Added: 


Modified: 
clang/lib/Parse/ParseExpr.cpp
clang/test/SemaOpenCL/func.cl

Removed: 




diff  --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 6acf76d713fd..3b0dd3f4036f 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -1807,7 +1807,8 @@ ExprResult Parser::ParseCastExpression(CastParseKind 
ParseKind,
   // These can be followed by postfix-expr pieces.
   PreferredType = SavedType;
   Res = ParsePostfixExpressionSuffix(Res);
-  if (getLangOpts().OpenCL)
+  if (getLangOpts().OpenCL && !getActions().getOpenCLOptions().isEnabled(
+  "__cl_clang_function_pointers"))
 if (Expr *PostfixExpr = Res.get()) {
   QualType Ty = PostfixExpr->getType();
   if (!Ty.isNull() && Ty->isFunctionType()) {

diff  --git a/clang/test/SemaOpenCL/func.cl b/clang/test/SemaOpenCL/func.cl
index 8a9c20289f59..bcac30b67e37 100644
--- a/clang/test/SemaOpenCL/func.cl
+++ b/clang/test/SemaOpenCL/func.cl
@@ -38,6 +38,9 @@ typedef struct s
 
 //Function pointer
 void foo(void*);
+#ifdef FUNCPTREXT
+//expected-note@-2{{passing argument to parameter here}}
+#endif
 
 // Expect no diagnostics for an empty parameter list.
 void bar();
@@ -51,11 +54,30 @@ void bar()
 #endif
 
   // taking the address of a function is an error
-  foo((void*)foo); // expected-error{{taking address of function is not 
allowed}}
-  foo(&foo); // expected-error{{taking address of function is not allowed}}
+  foo((void*)foo);
+#ifndef FUNCPTREXT
+  // expected-error@-2{{taking address of function is not allowed}}
+#else
+  // FIXME: Functions should probably be in the address space defined by the
+  // implementation. It might make sense to put them into the Default address
+  // space that is bind to a physical segment by the target rather than fixing
+  // it to any of the concrete OpenCL address spaces during parsing.
+  // expected-error@-8{{casting 'void (*)(__private void *__private)' to type 
'__private void *' changes address space}}
+#endif
 
+  foo(&foo);
+#ifndef FUNCPTREXT
+  // expected-error@-2{{taking address of function is not allowed}}
+#else
+  // expected-error@-4{{passing 'void (*)(__private void *__private)' to 
parameter of type '__private void *' changes address space of pointer}}
+#endif
+
+  // FIXME: If we stop rejecting the line below a bug (PR49315) gets
+  // hit due to incorrectly handled pointer conversion.
+#ifndef FUNCPTREXT
   // initializing an array with the address of functions is an error
   void* vptrarr[2] = {foo, &foo}; // expected-error{{taking address of 
function is not allowed}} expected-error{{taking address of function is not 
allowed}}
+#endif
 
   // just calling a function is correct
   foo(0);



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

[PATCH] D97203: [OpenCL][PR49264] Allow taking address of functions by enabling the extension

[Anastasia Stulova via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGabbdb5639c70: [OpenCL] Allow taking address of functions as 
an extension. (authored by Anastasia).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97203

Files:
  clang/lib/Parse/ParseExpr.cpp
  clang/test/SemaOpenCL/func.cl


Index: clang/test/SemaOpenCL/func.cl
===
--- clang/test/SemaOpenCL/func.cl
+++ clang/test/SemaOpenCL/func.cl
@@ -38,6 +38,9 @@
 
 //Function pointer
 void foo(void*);
+#ifdef FUNCPTREXT
+//expected-note@-2{{passing argument to parameter here}}
+#endif
 
 // Expect no diagnostics for an empty parameter list.
 void bar();
@@ -51,11 +54,30 @@
 #endif
 
   // taking the address of a function is an error
-  foo((void*)foo); // expected-error{{taking address of function is not 
allowed}}
-  foo(&foo); // expected-error{{taking address of function is not allowed}}
+  foo((void*)foo);
+#ifndef FUNCPTREXT
+  // expected-error@-2{{taking address of function is not allowed}}
+#else
+  // FIXME: Functions should probably be in the address space defined by the
+  // implementation. It might make sense to put them into the Default address
+  // space that is bind to a physical segment by the target rather than fixing
+  // it to any of the concrete OpenCL address spaces during parsing.
+  // expected-error@-8{{casting 'void (*)(__private void *__private)' to type 
'__private void *' changes address space}}
+#endif
 
+  foo(&foo);
+#ifndef FUNCPTREXT
+  // expected-error@-2{{taking address of function is not allowed}}
+#else
+  // expected-error@-4{{passing 'void (*)(__private void *__private)' to 
parameter of type '__private void *' changes address space of pointer}}
+#endif
+
+  // FIXME: If we stop rejecting the line below a bug (PR49315) gets
+  // hit due to incorrectly handled pointer conversion.
+#ifndef FUNCPTREXT
   // initializing an array with the address of functions is an error
   void* vptrarr[2] = {foo, &foo}; // expected-error{{taking address of 
function is not allowed}} expected-error{{taking address of function is not 
allowed}}
+#endif
 
   // just calling a function is correct
   foo(0);
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -1807,7 +1807,8 @@
   // These can be followed by postfix-expr pieces.
   PreferredType = SavedType;
   Res = ParsePostfixExpressionSuffix(Res);
-  if (getLangOpts().OpenCL)
+  if (getLangOpts().OpenCL && !getActions().getOpenCLOptions().isEnabled(
+  "__cl_clang_function_pointers"))
 if (Expr *PostfixExpr = Res.get()) {
   QualType Ty = PostfixExpr->getType();
   if (!Ty.isNull() && Ty->isFunctionType()) {


Index: clang/test/SemaOpenCL/func.cl
===
--- clang/test/SemaOpenCL/func.cl
+++ clang/test/SemaOpenCL/func.cl
@@ -38,6 +38,9 @@
 
 //Function pointer
 void foo(void*);
+#ifdef FUNCPTREXT
+//expected-note@-2{{passing argument to parameter here}}
+#endif
 
 // Expect no diagnostics for an empty parameter list.
 void bar();
@@ -51,11 +54,30 @@
 #endif
 
   // taking the address of a function is an error
-  foo((void*)foo); // expected-error{{taking address of function is not allowed}}
-  foo(&foo); // expected-error{{taking address of function is not allowed}}
+  foo((void*)foo);
+#ifndef FUNCPTREXT
+  // expected-error@-2{{taking address of function is not allowed}}
+#else
+  // FIXME: Functions should probably be in the address space defined by the
+  // implementation. It might make sense to put them into the Default address
+  // space that is bind to a physical segment by the target rather than fixing
+  // it to any of the concrete OpenCL address spaces during parsing.
+  // expected-error@-8{{casting 'void (*)(__private void *__private)' to type '__private void *' changes address space}}
+#endif
 
+  foo(&foo);
+#ifndef FUNCPTREXT
+  // expected-error@-2{{taking address of function is not allowed}}
+#else
+  // expected-error@-4{{passing 'void (*)(__private void *__private)' to parameter of type '__private void *' changes address space of pointer}}
+#endif
+
+  // FIXME: If we stop rejecting the line below a bug (PR49315) gets
+  // hit due to incorrectly handled pointer conversion.
+#ifndef FUNCPTREXT
   // initializing an array with the address of functions is an error
   void* vptrarr[2] = {foo, &foo}; // expected-error{{taking address of function is not allowed}} expected-error{{taking address of function is not allowed}}
+#endif
 
   // just calling a function is correct
   foo(0);
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Pa

[PATCH] D94640: adds more checks to -Wfree-nonheap-object

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:10252
 namespace {
-void CheckFreeArgumentsOnLvalue(Sema &S, const std::string &CalleeName,
-const UnaryOperator *UnaryExpr,
-const VarDecl *Var) {
-  StorageClass Class = Var->getStorageClass();
-  if (Class == StorageClass::SC_Extern ||
-  Class == StorageClass::SC_PrivateExtern ||
-  Var->getType()->isReferenceType())
-return;
-
-  S.Diag(UnaryExpr->getBeginLoc(), diag::warn_free_nonheap_object)
-  << CalleeName << Var;
-}
+constexpr int Placeholder = 0;
 

cjdb wrote:
> aaron.ballman wrote:
> > After seeing my suggested workaround in practice, I'm hopeful we can figure 
> > out how to get the diagnostics engine to not require us to jump through 
> > these hoops. I wouldn't block the patch over this approach, but it's not 
> > very obvious what's happening from reading the code either.
> Sorry, I'm not entirely sure what you're getting at here? :/
I used to find the `Placeholder` stuff to be confusing in practice and would 
have preferred to see a diagnostic solution that doesn't need the extra 
argument. Now I think I'm changing my mind and have a different suggestion -- 
remove the `Placeholder` and `NoPlaceholder` variables and pass in a literal 
with a comment:
```
S.Diag(UnaryExpr->getBeginLoc(), diag::warn_free_nonheap_object)
<< CalleeName << 0 /*object*/<< cast(D);

  S.Diag(Lambda->getBeginLoc(), diag::warn_free_nonheap_object)
  << CalleeName << 2 /*lambda*/;
```
That removes the akwardness of trying to name the variables, which is what was 
was I found non-obvious before.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94640

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

[PATCH] D96832: [Clang][Attributes] Allow not_tail_called attribute to be applied to virtual function.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:4109
-  [[clang::not_tail_called]] int foo2() override;
-};
   }];

rnk wrote:
> aaron.ballman wrote:
> > Quuxplusone wrote:
> > > aaron.ballman wrote:
> > > > ahatanak wrote:
> > > > > Quuxplusone wrote:
> > > > > > (Moving into a thread)
> > > > > > 
> > > > > > > This patch doesn't prevent the call to method in the code below 
> > > > > > > from being tail called,
> > > > > > > but I suppose users would expect the attribute to prevent the 
> > > > > > > tail call?
> > > > > > ```
> > > > > > struct B {
> > > > > >   virtual void method();  
> > > > > > };
> > > > > > struct D : B {
> > > > > >   [[clang::not_tail_called]] void method() override; 
> > > > > > };
> > > > > > ```
> > > > > > 
> > > > > > The way virtual calls are handled in C++ is, all attributes and 
> > > > > > properties of the call are determined based on the //static// type 
> > > > > > at the call site; and then the //runtime destination// of the call 
> > > > > > is determined from the pointer in the vtable. Attributes and 
> > > > > > properties have no runtime existence, and so they physically cannot 
> > > > > > affect anything at runtime. Consider https://godbolt.org/z/P3799e :
> > > > > > 
> > > > > > ```
> > > > > > struct Ba {
> > > > > >   virtual Ba *method(int x = 1);  
> > > > > > };
> > > > > > struct Da : Ba {
> > > > > >   [[clang::not_tail_called]] [[nodiscard]] Da *method(int x = 2) 
> > > > > > noexcept override; 
> > > > > > };
> > > > > > auto callera(Da& da) {
> > > > > > Ba& ba = da;
> > > > > > ba.method();
> > > > > > }
> > > > > > ```
> > > > > > Here the call that is made is a //virtual// call (because 
> > > > > > `Ba::method` is virtual); with a default argument value of `1` 
> > > > > > (because `Ba::method`'s `x` parameter has a default value of `1`); 
> > > > > > and it returns something of type `Ba*` (because that's what 
> > > > > > `Ba::method` returns); and it is not considered to be noexcept 
> > > > > > (because `Ba::method` isn't marked noexcept); and it's okay to 
> > > > > > discard the result (because `Ba::method` is not nodiscard) and it 
> > > > > > is tail-called (because `Ba::method` doesn't disallow tail calls). 
> > > > > > All of these attributes and properties are based on the //static// 
> > > > > > type of variable `ba`, despite the fact that //at runtime// we'll 
> > > > > > end up jumping to the code for `Da::method`. According to the 
> > > > > > source code, statically, `Da::method` has a default argument of 
> > > > > > `2`, returns `Da*`, is noexcept, and is nodiscard, and disallows 
> > > > > > tail-calls. But we're not calling `da.method()`, we're calling 
> > > > > > `ba.method()`; so none of that matters to our call site at 
> > > > > > `callera`.
> > > > > > 
> > > > > > I think this patch is a good thing.
> > > > > OK, I see. I think this patch is fine then.
> > > > > 
> > > > > Should we add an explanation of how virtual functions are handled? 
> > > > > The doc currently just says the attribute prevents tail-call 
> > > > > optimization on statically bound calls.
> > > > > I think this patch is a good thing.
> > > > 
> > > > I agree with your explanation but I'm not certain I agree with your 
> > > > conclusion. :-) I think the examples you point out are more often a 
> > > > source of confusion for users than not because of the nature of static 
> > > > vs dynamic dispatch, and so saying "this behavior can be consistent 
> > > > with all of these other things people often get confused by" may be 
> > > > justifiable but also seems a bit user-hostile.
> > > > 
> > > > Taking a slightly different example:
> > > > ```
> > > > struct Ba {
> > > >   [[clang::not_tail_called]] virtual Ba *method();  
> > > > };
> > > > struct Da : Ba {
> > > >   Ba *method() override; 
> > > > };
> > > > 
> > > > void callera(Da& da) {
> > > > Ba& ba = da;
> > > > ba.method();
> > > > }
> > > > ```
> > > > There's no guarantee that `Ba::method()` and `Da::method()` have the 
> > > > same not-tail-called properties. The codegen for this case will still 
> > > > attach `notail` to the call site even though the dynamic target may not 
> > > > meet that requirement.
> > > > 
> > > > tl;dr: I think `notail` is a property of the call expression and the 
> > > > only way to know that's valid is when you know what's being called, so 
> > > > the current behavior is more user-friendly for avoiding optimization 
> > > > issues. I'd prefer not to relax that unless there was a significantly 
> > > > motivating example beyond what's presented here so far.
> > > > saying "this behavior can be consistent with all of these other things 
> > > > people often get confused by" may be justifiable but also seems a bit 
> > > > user-hostile.
> > > 
> > > I disagree. In areas where (we agree that) users are already a bit 
> > > confused, I want to treat consistency-of-in

[PATCH] D97251: [llvm] Add assertions for the smart pointers with the possibility to be null in ClangAttrEmitter

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D97251#2583843 , @OikawaKirie wrote:

> @aaron.ballman
>
> My only concern is the recursive call to this function on line 1359. If you 
> think it is also OK for the recursive call on line 1359, I will update the 
> patch as you mentioned above.
>
> BTW, how can I test and debug this tblgen backend?

There really is no way to test these changes, but you can debug the generator 
by running `clang-tblgen -gen-whatever-you-want -I path\to\clang\include 
path\to\clang\include\clang\Basic\Attr.td` (the available generators are listed 
in clang\utils\TableGen\TableGen.cpp).




Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:1359
 for (const auto &Base : llvm::reverse(Bases)) {
   if ((Ptr = createArgument(Arg, Attr, Base.first)))
 break;

OikawaKirie wrote:
> The recursive call is here.
> 
> The for loop here seems to allow a failed attempt on its bases. Although the 
> top-level callers expect the return value of this function is always 
> non-null, what about the call here?
I believe this call is fine as-is.



Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:1364
 
+  assert(Ptr && "Cannot create argument.");
+

OikawaKirie wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > OikawaKirie wrote:
> > > > Just as what has been suggested by @dexonsmith in D91844, I add the 
> > > > assertion here. However, as a recursive function it is (line 1359), I 
> > > > still concern whether the assertion here will lead to a crash.
> > > > 
> > > > What do you think?
> > > The intent with this function is that it never returns `nullptr` unless 
> > > the programmer messed up their call of the function (passed in the wrong 
> > > kind of `Record` kind of problem). All of the callers assume this returns 
> > > non-null, so I think it's reasonable to assert that the value is non-null 
> > > by this point.
> > 
> However, there is a recursive call to this function on line 1359, and it 
> seems that `nullptr` is allowed to be returned for the call. I am just 
> worrying about whether the assertion here will lead to a crash in the 
> recursive calls.
> 
> IMO, a better way is to wrap this function and assert the return value of 
> this function in the wrapper function.
I don't think we need to go to those lengths -- if the recursive call fails, 
there was a Clang programmer mistake (they wrote the wrong thing in Attr.td or 
they forgot to write the right things in ClangAttrEmitter.cpp).

Put another way: we could use `PrintFatalError(Arg.getLoc(), "Failed to create 
the argument")` instead of `assert` if we wanted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97251

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

[PATCH] D93594: [X86] Pass to transform amx intrinsics to scalar operation.

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: llvm/include/llvm/CodeGen/Passes.h:496
+
+  /// The pass transform amx intrinsics to scalar operation if the function has
+  /// optnone attribute or it is O0.

transforms



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:1
+//===- llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp 
--===//
+//

We usually comment as //===--- filename - description ---===//
See `head -n1 llvm/lib/Target/X86/*.cpp`



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:51
+  BasicBlock *Header = BasicBlock::Create(
+  Preheader->getContext(), Name + ".header", Preheader->getParent(), Exit);
+  BasicBlock *Body = BasicBlock::Create(Header->getContext(), Name + ".body",

Ctx



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:88
+
+template `? I think it also can reduce the branch.



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:99
+  Loop *RowLoop = LI.AllocateLoop();
+  Loop *ColLoop = LI.AllocateLoop();
+  RowLoop->addChildLoop(ColLoop);

Not sure how about the arithmetic intrinsics. But at least for load and store 
intrinsics we can use LLVM intrinsic `llvm.masked.load/store` to reduce the 
inner loop.



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:166
+Value *Vec = nullptr;
+if (auto *BitCast = dyn_cast(Tile))
+  Vec = BitCast->getOperand(0);

Maybe we can just use cast to help to raise the assertion.



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:223
+  Value *VecC, *VecA, *VecB;
+  if (auto *BitCast = dyn_cast(Acc))
+VecC = BitCast->getOperand(0);

You can use cast to help to check the failure so that VecA/B/C won't be 
uninitialized.



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:229
+  // to vector. However with -O0, it doesn't happen.
+  if (auto *BitCast = dyn_cast(LHS))
+VecA = BitCast->getOperand(0);

ditto



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:231
+VecA = BitCast->getOperand(0);
+  assert(VecA->getType()->isVectorTy() && "bitcast from non-v256i32 to 
x86amx");
+  if (auto *BitCast = dyn_cast(RHS))

Should check it is V256I32?



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:232
+  assert(VecA->getType()->isVectorTy() && "bitcast from non-v256i32 to 
x86amx");
+  if (auto *BitCast = dyn_cast(RHS))
+VecB = BitCast->getOperand(0);

ditto



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:288
+  // %acc = call i32 @llvm.vector.reduce.add.v4i32(<4 x i32> %131)
+  // %neweltc = add i32 %elt, %acc
+  // %NewVecC = insertelement <256 x i32> %vec.c.inner.phi, i32 %neweltc,

eltc?



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:311
+  Value *ResElt = B.CreateAdd(EltC, SubVecR);
+  Value *NewVecC = B.CreateInsertElement(VecCPhi, ResElt, IdxC);
+  Value *NewVecD = B.CreateInsertElement(VecDPhi, ResElt, IdxC);

Is it necessary to insert the ResElt to VecC?



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:340
+IntrID == Intrinsic::x86_tilestored64_internal>::type>
+  bool lowerTileLoadStore(Instruction *TileLoad);
+  bool lowerTileLoad(Instruction *TileLoad);

TileLoadStore



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:341
+  bool lowerTileLoadStore(Instruction *TileLoad);
+  bool lowerTileLoad(Instruction *TileLoad);
+  bool lowerTileDPBSSD(Instruction *TileDPBSSD);

Forgot to remove?



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:343
+  bool lowerTileDPBSSD(Instruction *TileDPBSSD);
+  bool lowerTileStore(Instruction *TileStore);
+  bool lowerTileZero(Instruction *TileZero);

ditto



Comment at: llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp:387
+
+template ::type>
+bool X86LowerAMXIntrinsics::lowerTileLoadStore(Instruction *TileLoad) {
+  Value *M, *N, *Ptr, *Stride, *Tile;

ditto



Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:333
+TargetMachine *TM = 
&getAnalysis().getTM();
+if (F.hasFnAttribute(Attribute::OptimizeNone) ||
+TM->getOptLevel() == CodeGenOpt::None)

ditto



Comment at: llvm/test/CodeGen/X86/AMX/amx-low-intrinsics-no-bitcast.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -mtriple=x86_64 -lower-amx-intrinsics %s -S | FileCheck %s

Better name it amx-low-intrinsics-no-amx-bitcast.ll



Comment at: llvm/test/CodeGen/X86/AMX/amx-low-intrinsics-no-bitcast.ll:13
+; CHECK-NEXT:br label [[TILELO

[PATCH] D97072: [OpenCL][Docs] Add guidelines for adding new extensions and features

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 326055.
Anastasia added a comment.

Added suggestions from Sven.


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

https://reviews.llvm.org/D97072

Files:
  clang/docs/OpenCLSupport.rst


Index: clang/docs/OpenCLSupport.rst
===
--- clang/docs/OpenCLSupport.rst
+++ clang/docs/OpenCLSupport.rst
@@ -111,6 +111,8 @@
 Note that this is a frontend-only flag and therefore it requires the use of
 flags that forward options to the frontend e.g. ``-cc1`` or ``-Xclang``.
 
+.. _opencl_builtins:
+
 OpenCL builtins
 ---
 
@@ -153,6 +155,81 @@
   inserted using ``InsertOCLBuiltinDeclarationsFromTable`` and overload
   resolution takes place.
 
+OpenCL Extensions and Features
+--
+
+Clang implements various extensions to OpenCL kernel languages.
+
+New functionality is accepted as soon as the documentation is detailed to the
+level sufficient to be implemented. There should be an evidence that the
+extension is designed with implementation feasibility in consideration and
+assessment of complexity for C/C++ based compilers. Alternatively, the
+documentation can be accepted in a format of a draft that can be further
+refined during the implementation.
+
+Implementation guidelines
+^
+
+This section explains how to extend clang with the new functionality.
+
+**Parsing functionality**
+
+If an extension modifies the standard parsing it needs to be added to
+the clang frontend source code. This also means that the associated macro
+indicating the presence of the extension should be added to clang.
+
+The default flow for adding a new extension into the frontend is to
+modify `OpenCLExtensions.def
+`_
+
+This will add the macro automatically and also add a field in the target
+options ``clang::TargetOptions::OpenCLFeaturesMap`` to control the exposure
+of the new extension during the compilation.
+
+Note that by default targets like `SPIR` or `X86` expose all the OpenCL
+extensions. For all other targets the configuration has to be made explicitly.
+
+Note that the target extension support performed by clang can be overridden
+with :ref:`-cl-ext ` command-line flags.
+
+**Library functionality**
+
+If an extension adds functionality that does not modify standard language
+parsing it should not require clang source code modifications. Most commonly
+such extensions add functionality via libraries (by adding new non-native
+types or functions) parsed regularly. Similar to other languages this is the
+most common way to add new functionality. Note that new functionality in
+``OpenCLBuiltins.td`` detailed in :ref:`` belong to this
+category even if it is part of the clang source code.
+
+Clang has standard headers where new types and functions are being added,
+for more details refer to
+:ref:`the section on the OpenCL Header `. The macros indicating
+the presence of such extensions can be added in the standard header files
+conditioned on target specific predefined macros or/and language version
+predefined macros.
+
+**Pragmas**
+
+Some extensions alter standard parsing dynamically via pragmas.
+
+Clang provides a mechanism to add the standard extension pragma
+``OPENCL EXTENSION`` by setting a dedicated flag in the extension list entry of
+``OpenCLExtensions.def``. Note that there is no default behavior for the
+standard extension pragmas as it is not specified (for the standards up to and
+including version 3.0) in a sufficient level of detail and, therefore,
+there is no default functionality provided by clang.
+
+Pragmas without detailed information of its behavior (e.g. an explanation of
+changes it triggers in the parsing) should not be added to clang. Moreover, the
+pragmas should provide useful functionality to the user.
+
+Note that some legacy extensions (published prior to OpenCL 3.0), however still
+provide some non-conformant functionality for pragmas e.g. add diagnostics on
+the use of types or functions. This functionality is not guaranteed to remain 
in
+future releases. However, any future changes should not affect backward
+compatibility.
+
 .. _opencl_addrsp:
 
 Address spaces attribute


Index: clang/docs/OpenCLSupport.rst
===
--- clang/docs/OpenCLSupport.rst
+++ clang/docs/OpenCLSupport.rst
@@ -111,6 +111,8 @@
 Note that this is a frontend-only flag and therefore it requires the use of
 flags that forward options to the frontend e.g. ``-cc1`` or ``-Xclang``.
 
+.. _opencl_builtins:
+
 OpenCL builtins
 ---
 
@@ -153,6 +155,81 @@
   inserted using ``InsertOCLBuiltinDeclarationsFromTable`` and overload
   resolution takes place.
 
+OpenCL Extensions and Features
+--
+
+Clang implements various extensions to OpenCL kernel languages.

[PATCH] D97072: [OpenCL][Docs] Add guidelines for adding new extensions and features

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: clang/docs/OpenCLSupport.rst:196
+If an extension adds the functionality that does not modify standard language
+parsing it should not require clang source code modifications. Most commonly
+such extensions add functionality via libraries (by adding new non-native

svenvh wrote:
> This might need some rephrasing to take TableGen BIFs into account.  I would 
> consider `clang/lib/Sema/OpenCLBuiltins.td` an integral part of the clang 
> source code.
> 
> Maybe rephrase "clang source code modifications" into "clang parser source 
> code modifications"?
I am trying to avoid using parsing in relation to clang since someone might 
understand it as clang `Parser` library functionality. I have added a note 
about this case explicitly

> Note that new functionality in ``OpenCLBuiltins.td`` detailed in 
> :ref:`` belong to this category even if it is part of the 
> clang source code.

Although we do refer to header functionality in the next paragraph anyway where 
this is referenced but indirectly. I hope it provides enough clarifications for 
now.



Comment at: clang/docs/OpenCLSupport.rst:201
+
+Clang has standard headers where new types and functions are being added,
+for more details refer to

svenvh wrote:
> Should this mention `clang/lib/Sema/OpenCLBuiltins.td` too?
I think here I don't want to go to too many details of how headers are 
implemented. I refer to this: 
https://clang.llvm.org/docs/UsersManual.html#opencl-header that also refers to 
`OpenCLSupport` page where we explain details on Tablegen header. However, the 
header topic does require another round of clarifications and I do plan to 
improve it further iin the near future. 



Comment at: clang/docs/OpenCLSupport.rst:221
+triggers in the parsing) should not be added to clang. Moreover, the acceptable
+behavior of pragmas should provide useful functionality to the user.
+

svenvh wrote:
> Remove "acceptable behavior of".
> 
> How do you define "useful functionality"?
I think this is one of those situations that is hard to define unambiguously, 
so I am open to suggestions. However, I do want to prevent changes that are 
reinventing the wheel (i.e. C/C++ already had another way to do the same thing) 
or functionality that doesn't add any value (i.e. requiring pragma enable to 
use already added types or functions). This has happened in the past multiple 
times so I think it is good to be specific that when new functionality is added 
it should have a reason for it.

I think the other statement above:

> New functionality is accepted as soon as the documentation is detailed to the 
> level sufficient to be implemented.

is similar. It is not very easy to tell what is the detailed level of 
documentation. FYI it generally aligns with clang overall policy:

https://clang.llvm.org/get_involved.html

> A specification: The specification must be sufficient to understand the 
> design of the feature as well as interpret the meaning of specific examples. 
> The specification should be detailed enough that another compiler vendor 
> could implement the feature.

I am just emphasizing this item here.

Regarding 'usefulness' I would even suggest adding it as a general guideline 
for clang but I think this is not very common. So for now I want to make sure 
we have it in our guidelines at least since we have encountered this.


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

https://reviews.llvm.org/D97072

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

[PATCH] D95119: Prefer /usr/bin/env xxx over /usr/bin/xxx where xxx = perl, python, awk

[Harmen Stoppels via Phabricator via cfe-commits]

haampie added a comment.

Should this be merged?


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

https://reviews.llvm.org/D95119

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

[PATCH] D97322: [clang-tidy][test] Allow specifying potentially unused suffixes

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97322

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

[PATCH] D96209: [clang-tidy] Fix readability-avoid-const-params-in-decls removing const in template paramaters

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, what a nice simplification!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96209

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

[PATCH] D61112: AMDGPU: Enable _Float16

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: test/CodeGenCXX/amdgpu-float16.cpp:7
+  _Float16 x, y, z;
+  // CHECK: v_add_f16_e64
+  // NOF16: v_add_f32_e64

I thought clang tests were supposed to stop at the IR


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

https://reviews.llvm.org/D61112

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

[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4050
<< ", mem=" << ProcStat->PeakMemory << " Kb\n";
-  }
-  if (!StatReportFile.empty()) {
+  } else {
 // CSV format.

vvereschaka wrote:
> aganea wrote:
> > The previous behavior was printing both the output and the CSV file when 
> > specifying `-fproc-stat-report -fproc-stat-report=file.csv`, now it would 
> > only emit the CSV, is that intended? @sepavloff what do you think?
> >now it would only emit the CSV, is that intended?
> 
> yes, I thought in that way. I dоn't think that usage of both options at the 
> same time was specially designed. I don't see a reasonable usage case for 
> that for now actually, but I could be wrong. @sepavloff ?
> > now it would only emit the CSV, is that intended?

> yes, I thought in that way. I dоn't think that usage of both options at the 
> same time was specially designed. I don't see a reasonable usage case for 
> that for now actually, but I could be wrong. @sepavloff ?

The previous behavior (printing both the output and the CSV file) was not 
intentional, it was the implementation peculiarity. I don't see real use case 
for both outputs. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97094

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

[PATCH] D97052: [OpenCL] Prevent adding extension pragma by default

[Marco Antognini via Phabricator via cfe-commits]

mantognini added a comment.

Some relatively minor comments, overall direction seems good to me.




Comment at: clang/include/clang/Basic/OpenCLOptions.h:67
 public:
   struct OpenCLOptionInfo {
 // Option starts to be available in this OpenCL version

In OpenCLExtensions.def, the order is: pragma, avail, core, opt.
In this class it is: avail, core, opt, pragma (for both the attributes and the 
constructor parameters).

There are very few usages of this, so the impact of this inconsistency is 
minor. However, because the conversion between unsigned and bool is implicit an 
error might go unnoticed for a long time.



Comment at: clang/lib/Basic/OpenCLOptions.cpp:89-91
+  OptMap.insert(   
\
+  std::make_pair(llvm::StringRef{#Ext},
\
+ OpenCLOptionInfo{AvailVer, CoreVer, OptVer, Pragma}));

This could be done using `insert_or_assign(key, value)`.



Comment at: clang/lib/Basic/Targets.cpp:738
   };
-#define OPENCL_GENERIC_EXTENSION(Ext, Avail, Core, Opt)
\
+#define OPENCL_GENERIC_EXTENSION(Ext, WithPragma, Avail, Core, Opt)
\
   defineOpenCLExtMacro(#Ext, Avail, Core, Opt);

For consistency with OpenCLExtensions.def.


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

https://reviews.llvm.org/D97052

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

[PATCH] D61112: AMDGPU: Enable _Float16

[Yaxun Liu via Phabricator via cfe-commits]

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

In D61112#2584279 , @foad wrote:

> Should have updated 
> https://clang.llvm.org/docs/LanguageExtensions.html#half-precision-floating-point
>  "_Float16 is currently only supported on the following targets, with further 
> targets pending ABI standardization: ..."

will update the doc




Comment at: test/CodeGenCXX/amdgpu-float16.cpp:7
+  _Float16 x, y, z;
+  // CHECK: v_add_f16_e64
+  // NOF16: v_add_f32_e64

arsenm wrote:
> I thought clang tests were supposed to stop at the IR
If you 'grep -- '-S ' clang/test/CodeGen/* | grep -v emit-llvm | grep clang', 
there are plenty of clang codegen tests checks assembly code. In this test we 
want to make sure f16 instructions are emitted for specific GPUs.


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

https://reviews.llvm.org/D61112

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

[PATCH] D97233: Support `#pragma clang section` directives on MachO targets

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added inline comments.



Comment at: clang/lib/Sema/SemaAttr.cpp:304
 
+  if (llvm::Error E = Context.getTargetInfo().isValidSectionSpecifier(SecName))
+Diag(PragmaLoc, diag::err_pragma_section_invalid_for_target)

Shouldn't this block return so the section doesn't get marked as valid later?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97233

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

[PATCH] D95246: [test] Use host platform specific error message substitution in lit tests

[Abhina Sree via Phabricator via cfe-commits]

abhina.sreeskantharajan added a comment.

@ASDenysPetrov are you running python inside MSYS bash that is installed by

> pacman -S --needed base-devel mingw-w64-x86_64-toolchain

? I learned that the python should give different output

  >>> sys.platform
  msys
  >>> platform.system()
  MSYS_NT-10.0-19041
  >>> sysconfig.get_platform()
  msys-3.1.7-x86_64

After taking a look at the CMakeCache.txt, I think a potential workaround would 
be to check if config.host_cxx contains MSYS2 which is set to 
CMAKE_CXX_COMPILER:FILEPATH=D:/WORK/Utilities/MSYS2/mingw64/bin/c++.exe if we 
can't determine MSYS through python.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95246

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

[PATCH] D97297: [clang-tidy] Suppress reports to patternedly named parameters in 'bugprone-easily-swappable-parameters'

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-easily-swappable-parameters.rst:89
+warning about the two parameters are silenced.
+Defaults to ``1``.
+Might be any positive integer.

whisperity wrote:
> Eugene.Zelenko wrote:
> > Please use single back-ticks for option values. Same below.
> **All** values (including above the ``_Bool`` and such, or only the numeric 
> constants? The ``LHS`` and ``RHS`` below are examples of source code, not a 
> valid value for the option.
All option values.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97297

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

[clang-tools-extra] b90fdb7 - [clang-tidy][test] Allow specifying potentially unused suffixes

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2021-02-24T15:22:54Z
New Revision: b90fdb7c117fab83a8a2d1c95ed468c47e480f89

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

LOG: [clang-tidy][test] Allow specifying potentially unused suffixes

If a check-suffix is only required for a CHECK-FIXES or CHECK-MESSAGES. 
check_clang_tidy will pass the prefixes CHECK-FIXES<...> and 
CHECK-MESSAGES<...> to FileCheck.
This will result in a FileCheck failing because of an unused prefix.

This addresses the problem by not passing unused prefixes. Its also possible to 
fix this be passing `--allow-unused-prefixes` flag to FileCheck, but seeing as 
we have already done the legwork in the script to see its unused, this fix 
seems the better way to go.

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang-tools-extra/test/clang-tidy/check_clang_tidy.py

clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp

Removed: 




diff  --git a/clang-tools-extra/test/clang-tidy/check_clang_tidy.py 
b/clang-tools-extra/test/clang-tidy/check_clang_tidy.py
index 0031d9b04ad1..5ebe3a1c1491 100755
--- a/clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ b/clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -119,9 +119,12 @@ def run_test_once(args, extra_args):
 has_check_messages = has_check_messages or has_check_message
 has_check_notes = has_check_notes or has_check_note
 
-check_fixes_prefixes.append(check_fixes_prefix)
-check_messages_prefixes.append(check_messages_prefix)
-check_notes_prefixes.append(check_notes_prefix)
+if has_check_fix:
+  check_fixes_prefixes.append(check_fixes_prefix)
+if has_check_message:
+  check_messages_prefixes.append(check_messages_prefix)
+if has_check_note:
+  check_notes_prefixes.append(check_notes_prefix)
 
   assert has_check_fixes or has_check_messages or has_check_notes
   # Remove the contents of the CHECK lines to avoid CHECKs matching on

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp
index c82c29173604..63da130e74c6 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp
@@ -30,13 +30,6 @@
 // RUN:   | FileCheck %s -check-prefix=CHECK-HEADER-NO-FUNC \
 // RUN:   -implicit-check-not="{{warning|error}}:"
 
-/// Suppress FileCheck --allow-unused-prefixes=false diagnostics.
-// CHECK-MESSAGES-RANGES: {{^}}
-// CHECK-MESSAGES-CUSTOM: {{^}}
-// CHECK-MESSAGES-CUSTOM-SYS: {{^}}
-// CHECK-MESSAGES-CUSTOM-NO-SYS: {{^}}
-// CHECK-MESSAGES-CUSTOM-NO-HEADER: {{^}}
-
 // CHECK-HEADER-NO-FUNC: warning: modernize-loop-convert: 
'MakeReverseRangeHeader' is set but 'MakeReverseRangeFunction' is not, 
disabling reverse loop transformation
 
 // Make sure appropiate headers are included



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

[PATCH] D97322: [clang-tidy][test] Allow specifying potentially unused suffixes

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb90fdb7c117f: [clang-tidy][test] Allow specifying 
potentially unused suffixes (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97322

Files:
  clang-tools-extra/test/clang-tidy/check_clang_tidy.py
  clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp
@@ -30,13 +30,6 @@
 // RUN:   | FileCheck %s -check-prefix=CHECK-HEADER-NO-FUNC \
 // RUN:   -implicit-check-not="{{warning|error}}:"
 
-/// Suppress FileCheck --allow-unused-prefixes=false diagnostics.
-// CHECK-MESSAGES-RANGES: {{^}}
-// CHECK-MESSAGES-CUSTOM: {{^}}
-// CHECK-MESSAGES-CUSTOM-SYS: {{^}}
-// CHECK-MESSAGES-CUSTOM-NO-SYS: {{^}}
-// CHECK-MESSAGES-CUSTOM-NO-HEADER: {{^}}
-
 // CHECK-HEADER-NO-FUNC: warning: modernize-loop-convert: 
'MakeReverseRangeHeader' is set but 'MakeReverseRangeFunction' is not, 
disabling reverse loop transformation
 
 // Make sure appropiate headers are included
Index: clang-tools-extra/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -119,9 +119,12 @@
 has_check_messages = has_check_messages or has_check_message
 has_check_notes = has_check_notes or has_check_note
 
-check_fixes_prefixes.append(check_fixes_prefix)
-check_messages_prefixes.append(check_messages_prefix)
-check_notes_prefixes.append(check_notes_prefix)
+if has_check_fix:
+  check_fixes_prefixes.append(check_fixes_prefix)
+if has_check_message:
+  check_messages_prefixes.append(check_messages_prefix)
+if has_check_note:
+  check_notes_prefixes.append(check_notes_prefix)
 
   assert has_check_fixes or has_check_messages or has_check_notes
   # Remove the contents of the CHECK lines to avoid CHECKs matching on


Index: clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-loop-convert-reverse.cpp
@@ -30,13 +30,6 @@
 // RUN:   | FileCheck %s -check-prefix=CHECK-HEADER-NO-FUNC \
 // RUN:   -implicit-check-not="{{warning|error}}:"
 
-/// Suppress FileCheck --allow-unused-prefixes=false diagnostics.
-// CHECK-MESSAGES-RANGES: {{^}}
-// CHECK-MESSAGES-CUSTOM: {{^}}
-// CHECK-MESSAGES-CUSTOM-SYS: {{^}}
-// CHECK-MESSAGES-CUSTOM-NO-SYS: {{^}}
-// CHECK-MESSAGES-CUSTOM-NO-HEADER: {{^}}
-
 // CHECK-HEADER-NO-FUNC: warning: modernize-loop-convert: 'MakeReverseRangeHeader' is set but 'MakeReverseRangeFunction' is not, disabling reverse loop transformation
 
 // Make sure appropiate headers are included
Index: clang-tools-extra/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -119,9 +119,12 @@
 has_check_messages = has_check_messages or has_check_message
 has_check_notes = has_check_notes or has_check_note
 
-check_fixes_prefixes.append(check_fixes_prefix)
-check_messages_prefixes.append(check_messages_prefix)
-check_notes_prefixes.append(check_notes_prefix)
+if has_check_fix:
+  check_fixes_prefixes.append(check_fixes_prefix)
+if has_check_message:
+  check_messages_prefixes.append(check_messages_prefix)
+if has_check_note:
+  check_notes_prefixes.append(check_notes_prefix)
 
   assert has_check_fixes or has_check_messages or has_check_notes
   # Remove the contents of the CHECK lines to avoid CHECKs matching on
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D95246: [test] Use host platform specific error message substitution in lit tests

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@abhina.sreeskantharajan wrote:

> @ASDenysPetrov are you running python inside MSYS bash? that is installed by

No, I installed Python as usual via Windows msi got from official site.

Here is what I've got from Python:

>>> sys.platform

'win32'

>>> platform.system()

'Windows'

>>> sysconfig.get_platform()

'win32'

>>> 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95246

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

[PATCH] D97361: [clang-tidy] Add misc-redundant-using check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

I think this check belongs to `readability` module.




Comment at: clang-tools-extra/clang-tidy/misc/RedundantUsingCheck.cpp:1
+//===--- RedundantUsingCheck.cpp - clang-tidy
+//===//

Please make single line from two. See other checks as example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97361

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

[PATCH] D97386: update AMDGPU _Float16 support in clang doc

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: foad, arsenm, rampitec.
Herald added subscribers: t-tye, tpr, dstuttard, kzhuravl.
yaxunl requested review of this revision.
Herald added a subscriber: wdng.

https://reviews.llvm.org/D97386

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -524,6 +524,7 @@
 
 * 32-bit ARM
 * 64-bit ARM (AArch64)
+* AMDGPU
 * SPIR
 
 ``_Float16`` will be supported on more targets as they define ABIs for it.


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -524,6 +524,7 @@
 
 * 32-bit ARM
 * 64-bit ARM (AArch64)
+* AMDGPU
 * SPIR
 
 ``_Float16`` will be supported on more targets as they define ABIs for it.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97388: [analyzer] Replace StoreManager::evalIntegralCast with SValBuilder::evalCast

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov created this revision.
ASDenysPetrov added reviewers: steakhal, xazax.hun, NoQ, vsavchenko.
ASDenysPetrov added a project: clang.
Herald added subscribers: martong, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware.
ASDenysPetrov requested review of this revision.
Herald added a subscriber: cfe-commits.

Simplify and move functionality from `evalIntegralCast` to `evalCast`. Replace 
`evalIntegralCast` with `evalCast`.

This patch shall not change any behavior.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97388

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -493,44 +493,6 @@
   return true;
 }
 
-// Handles casts of type CK_IntegralCast.
-// At the moment, this function will redirect to evalCast, except when the range
-// of the original value is known to be greater than the max of the target type.
-SVal SValBuilder::evalIntegralCast(ProgramStateRef state, SVal val,
-   QualType castTy, QualType originalTy) {
-  // No truncations if target type is big enough.
-  if (getContext().getTypeSize(castTy) >= getContext().getTypeSize(originalTy))
-return evalCast(state, val, castTy, originalTy);
-
-  SymbolRef se = val.getAsSymbol();
-  if (!se) // Let evalCast handle non symbolic expressions.
-return evalCast(state, val, castTy, originalTy);
-
-  // Find the maximum value of the target type.
-  APSIntType ToType(getContext().getTypeSize(castTy),
-castTy->isUnsignedIntegerType());
-  llvm::APSInt ToTypeMax = ToType.getMaxValue();
-  NonLoc ToTypeMaxVal =
-  makeIntVal(ToTypeMax.isUnsigned() ? ToTypeMax.getZExtValue()
-: ToTypeMax.getSExtValue(),
- castTy)
-  .castAs();
-  // Check the range of the symbol being casted against the maximum value of the
-  // target type.
-  NonLoc FromVal = val.castAs();
-  QualType CmpTy = getConditionType();
-  NonLoc CompVal =
-  evalBinOpNN(state, BO_LE, FromVal, ToTypeMaxVal, CmpTy).castAs();
-  ProgramStateRef IsNotTruncated, IsTruncated;
-  std::tie(IsNotTruncated, IsTruncated) = state->assume(CompVal);
-  if (!IsNotTruncated && IsTruncated) {
-// Symbol is truncated so we evaluate it as a cast.
-NonLoc CastVal = makeNonLoc(se, originalTy, castTy);
-return CastVal;
-  }
-  return evalCast(state, val, castTy, originalTy);
-}
-
 //===--===//
 // Cast methods.
 // `evalCast` is the main method
@@ -915,6 +877,31 @@
   return makeNonLoc(SE, BO_NE, BVF.getValue(0, SE->getType()), CastTy);
 }
   } else {
+
+if (!OriginalTy.isNull() && CastTy->isIntegralOrEnumerationType() &&
+OriginalTy->isIntegralOrEnumerationType() &&
+getContext().getTypeSize(CastTy) <
+getContext().getTypeSize(OriginalTy)) {
+  // Find the maximum value of the target type.
+  llvm::APSInt ToTypeMax = llvm::APSInt::getMaxValue(
+  getContext().getTypeSize(CastTy), CastTy->isUnsignedIntegerType());
+  NonLoc ToTypeMaxVal =
+  makeIntVal(ToTypeMax.getExtValue(), CastTy).castAs();
+  // Check the range of the symbol being casted against the maximum value of
+  // the target type.
+  NonLoc FromVal = V.castAs();
+  QualType CmpTy = getConditionType();
+  NonLoc CompVal = evalBinOpNN(State, BO_LE, FromVal, ToTypeMaxVal, CmpTy)
+   .castAs();
+  ProgramStateRef IsNotTruncated, IsTruncated;
+  std::tie(IsNotTruncated, IsTruncated) = State->assume(CompVal);
+  if (!IsNotTruncated && IsTruncated) {
+// Symbol is truncated so we evaluate it as a cast.
+NonLoc CastVal = makeNonLoc(SE, OriginalTy, CastTy);
+return CastVal;
+  }
+}
+
 // Symbol to integer, float.
 QualType T = Context.getCanonicalType(SE->getType());
 // If types are the same or both are integers, ignore the cast.
Index: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -431,7 +431,7 @@
   case CK_IntegralCast: {
 // Delegate to SValBuilder to process.
 SVal V = state->getSVal(Ex, LCtx);
-V = svalBuilder.evalIntegralCast(state, V, T, ExTy);
+V = svalBuilder.evalCast(state, V, T, ExTy);
 state = state->BindExpr(CastE, LCtx, V);
 Bldr.generateNode(CastE, Pred, state);
 continue;
Index: clang/include/clang/StaticAnalyzer/Core/PathSe

[PATCH] D97375: [clang][cli] Add MarshallingInfoEnum multiclass

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

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97375

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

[PATCH] D97370: [clang][cli] Remove marshalling from Opt{In, Out}FFlag

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

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97370

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

[clang-tools-extra] a34532c - [clang-tidy] Fix readability-avoid-const-params-in-decls removing const in template paramaters

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2021-02-24T15:54:10Z
New Revision: a34532c330f61c35612bb0c4b753979307020608

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

LOG: [clang-tidy] Fix readability-avoid-const-params-in-decls removing const in 
template paramaters

Fixes https://bugs.llvm.org/show_bug.cgi?id=38035

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp

clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp 
b/clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
index d35cc079d78a..e655f013a254 100644
--- a/clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
+++ b/clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "AvoidConstParamsInDecls.h"
+#include "../utils/LexerUtils.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Lex/Lexer.h"
@@ -20,10 +21,8 @@ namespace readability {
 namespace {
 
 SourceRange getTypeRange(const ParmVarDecl &Param) {
-  if (Param.getIdentifier() != nullptr)
-return SourceRange(Param.getBeginLoc(),
-   Param.getEndLoc().getLocWithOffset(-1));
-  return Param.getSourceRange();
+  return SourceRange(Param.getBeginLoc(),
+ Param.getLocation().getLocWithOffset(-1));
 }
 
 } // namespace
@@ -38,34 +37,6 @@ void AvoidConstParamsInDecls::registerMatchers(MatchFinder 
*Finder) {
   this);
 }
 
-// Re-lex the tokens to get precise location of last 'const'
-static llvm::Optional constTok(CharSourceRange Range,
-  const MatchFinder::MatchResult &Result) {
-  const SourceManager &Sources = *Result.SourceManager;
-  std::pair LocInfo =
-  Sources.getDecomposedLoc(Range.getBegin());
-  StringRef File = Sources.getBufferData(LocInfo.first);
-  const char *TokenBegin = File.data() + LocInfo.second;
-  Lexer RawLexer(Sources.getLocForStartOfFile(LocInfo.first),
- Result.Context->getLangOpts(), File.begin(), TokenBegin,
- File.end());
-  Token Tok;
-  llvm::Optional ConstTok;
-  while (!RawLexer.LexFromRawLexer(Tok)) {
-if (Sources.isBeforeInTranslationUnit(Range.getEnd(), Tok.getLocation()))
-  break;
-if (Tok.is(tok::raw_identifier)) {
-  IdentifierInfo &Info = Result.Context->Idents.get(StringRef(
-  Sources.getCharacterData(Tok.getLocation()), Tok.getLength()));
-  Tok.setIdentifierInfo(&Info);
-  Tok.setKind(Info.getTokenID());
-}
-if (Tok.is(tok::kw_const))
-  ConstTok = Tok;
-  }
-  return ConstTok;
-}
-
 void AvoidConstParamsInDecls::check(const MatchFinder::MatchResult &Result) {
   const auto *Func = Result.Nodes.getNodeAs("func");
   const auto *Param = Result.Nodes.getNodeAs("param");
@@ -101,7 +72,8 @@ void AvoidConstParamsInDecls::check(const 
MatchFinder::MatchResult &Result) {
   if (!FileRange.isValid())
 return;
 
-  auto Tok = constTok(FileRange, Result);
+  auto Tok = tidy::utils::lexer::getQualifyingToken(
+  tok::kw_const, FileRange, *Result.Context, *Result.SourceManager);
   if (!Tok)
 return;
   Diag << FixItHint::CreateRemoval(

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp
index f3d27e563193..4ba1f2482d2d 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp
@@ -59,6 +59,25 @@ int F13(const bool b = true);
 // CHECK-FIXES: int F13(bool b = true);
 int f13 = F13();
 
+template 
+struct A {};
+
+void F14(const A);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 1 is const-qualified
+// CHECK-FIXES: void F14(A);
+
+void F15(const A Named);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 'Named' is 
const-qualified
+// CHECK-FIXES: void F15(A Named);
+
+void F16(const A *const);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 1 is const-qualified
+// CHECK-FIXES: void F16(const A *);
+
+void F17(const A *const Named);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 'Named' is 
const-qualified
+// CHECK-FIXES: void F17(const A *Named);
+
 struct Foo {
   Foo(const int i);
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: parameter 'i'



__

[PATCH] D96209: [clang-tidy] Fix readability-avoid-const-params-in-decls removing const in template paramaters

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa34532c330f6: [clang-tidy] Fix 
readability-avoid-const-params-in-decls removing const in… (authored by 
njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96209

Files:
  clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp
@@ -59,6 +59,25 @@
 // CHECK-FIXES: int F13(bool b = true);
 int f13 = F13();
 
+template 
+struct A {};
+
+void F14(const A);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 1 is const-qualified
+// CHECK-FIXES: void F14(A);
+
+void F15(const A Named);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 'Named' is 
const-qualified
+// CHECK-FIXES: void F15(A Named);
+
+void F16(const A *const);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 1 is const-qualified
+// CHECK-FIXES: void F16(const A *);
+
+void F17(const A *const Named);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 'Named' is 
const-qualified
+// CHECK-FIXES: void F17(const A *Named);
+
 struct Foo {
   Foo(const int i);
   // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: parameter 'i'
Index: clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
===
--- clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
+++ clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "AvoidConstParamsInDecls.h"
+#include "../utils/LexerUtils.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Lex/Lexer.h"
@@ -20,10 +21,8 @@
 namespace {
 
 SourceRange getTypeRange(const ParmVarDecl &Param) {
-  if (Param.getIdentifier() != nullptr)
-return SourceRange(Param.getBeginLoc(),
-   Param.getEndLoc().getLocWithOffset(-1));
-  return Param.getSourceRange();
+  return SourceRange(Param.getBeginLoc(),
+ Param.getLocation().getLocWithOffset(-1));
 }
 
 } // namespace
@@ -38,34 +37,6 @@
   this);
 }
 
-// Re-lex the tokens to get precise location of last 'const'
-static llvm::Optional constTok(CharSourceRange Range,
-  const MatchFinder::MatchResult &Result) {
-  const SourceManager &Sources = *Result.SourceManager;
-  std::pair LocInfo =
-  Sources.getDecomposedLoc(Range.getBegin());
-  StringRef File = Sources.getBufferData(LocInfo.first);
-  const char *TokenBegin = File.data() + LocInfo.second;
-  Lexer RawLexer(Sources.getLocForStartOfFile(LocInfo.first),
- Result.Context->getLangOpts(), File.begin(), TokenBegin,
- File.end());
-  Token Tok;
-  llvm::Optional ConstTok;
-  while (!RawLexer.LexFromRawLexer(Tok)) {
-if (Sources.isBeforeInTranslationUnit(Range.getEnd(), Tok.getLocation()))
-  break;
-if (Tok.is(tok::raw_identifier)) {
-  IdentifierInfo &Info = Result.Context->Idents.get(StringRef(
-  Sources.getCharacterData(Tok.getLocation()), Tok.getLength()));
-  Tok.setIdentifierInfo(&Info);
-  Tok.setKind(Info.getTokenID());
-}
-if (Tok.is(tok::kw_const))
-  ConstTok = Tok;
-  }
-  return ConstTok;
-}
-
 void AvoidConstParamsInDecls::check(const MatchFinder::MatchResult &Result) {
   const auto *Func = Result.Nodes.getNodeAs("func");
   const auto *Param = Result.Nodes.getNodeAs("param");
@@ -101,7 +72,8 @@
   if (!FileRange.isValid())
 return;
 
-  auto Tok = constTok(FileRange, Result);
+  auto Tok = tidy::utils::lexer::getQualifyingToken(
+  tok::kw_const, FileRange, *Result.Context, *Result.SourceManager);
   if (!Tok)
 return;
   Diag << FixItHint::CreateRemoval(


Index: clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-avoid-const-params-in-decls.cpp
@@ -59,6 +59,25 @@
 // CHECK-FIXES: int F13(bool b = true);
 int f13 = F13();
 
+template 
+struct A {};
+
+void F14(const A);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 1 is const-qualified
+// CHECK-FIXES: void F14(A);
+
+void F15(const A Named);
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: parameter 'Named' is const-qualified
+//

[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

[Zakk Chen via Phabricator via cfe-commits]

khchen updated this revision to Diff 326104.
khchen marked 10 inline comments as done.
khchen added a comment.

1. Rebase
2. Address Craig's comments.
3. Change the operand orders of builtin to the same order of IR intrinsics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95016

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/riscv_vector.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/test/CodeGen/RISCV/rvv-intrinsics-generic/vadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-generic/vfadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfadd.c
  clang/test/CodeGen/RISCV/vadd.c
  clang/test/Headers/riscv-vector-header.c
  clang/utils/TableGen/CMakeLists.txt
  clang/utils/TableGen/RISCVVEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h
  llvm/docs/CommandGuide/tblgen.rst

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

[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

[Zakk Chen via Phabricator via cfe-commits]

khchen added inline comments.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:899
+// (operand) in ProtoSeq. ProtoSeq[0] is output operand.
+SmallVector ProtoSeq;
+const StringRef Primaries("evwqom0ztc");

craig.topper wrote:
> I think this is something like
> 
> ```
> while (!Prototypes.empty()) {
>  auto Idx = Prototypes.find_first_of(Primaries);
>  assert(Idx != StringRef::npos);
>  ProtoSeq.push_back(Prototypes.slice(0, Idx+1).str());
>  Prototypes = Prototypes.drop_front(Idx+1);
> }
> ```
> 
> Which might be easier to understand.
Thanks, it's clear.



Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1016
+  SmallVector ExtVector;
+  // D implies F
+  if (Extents & RISCV_Extension::F) {

craig.topper wrote:
> I don't understand this. It says D implies F but we're checking for F. So 
> that seems like F implies D.
Remove extension implying rule. 
I thought we don't need to consider implying rule because it's clang's 
responsibility. 
The original thinking was when predecessor "only" defines `__riscv_d`, and 
floating instruction also need to supported. But in fact,  when enabling 
__riscv_d predecessor will also define `__riscv_f`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95016

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

[PATCH] D97364: [docs] Add a release note for the removing of -Wreturn-std-move-in-c++11

[Adrian McCarthy via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97364

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

[PATCH] D97361: [clang-tidy] Add misc-redundant-using check

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/RedundantUsingCheck.cpp:23
+  Finder->addMatcher(
+  usingDecl(isExpansionInMainFile()).bind("using-declaration"), this);
+  Finder->addMatcher(

Why restrict these to main file only?



Comment at: clang-tools-extra/clang-tidy/misc/RedundantUsingCheck.cpp:30-33
+  if (const auto *Declaration =
+  Result.Nodes.getNodeAs("using-declaration")) {
+checkUsingDecl(Declaration, Result);
+  }

nit: Elide braces of single line if statements.



Comment at: clang-tools-extra/clang-tidy/misc/RedundantUsingCheck.cpp:35-38
+  if (const auto *Directive =
+  Result.Nodes.getNodeAs("using-directive")) {
+checkUsingDirective(Directive, Result);
+  }





Comment at: clang-tools-extra/clang-tidy/misc/RedundantUsingCheck.cpp:45
+
+  for (const auto *UsingShadow : Declaration->shadows()) {
+const Decl *TargetDecl = UsingShadow->getTargetDecl()->getCanonicalDecl();

nit: don't use auto as type isn't explicitly spelled in the initialization.



Comment at: clang-tools-extra/clang-tidy/misc/RedundantUsingCheck.cpp:50
+  const auto *NSD = cast(TargetDecl->getDeclContext());
+  diagUsing("declaration", Declaration, Declaration, NSD, Result);
+}

Shouldn't there be a break after we have diagnosed this?



Comment at: clang-tools-extra/clang-tidy/misc/RedundantUsingCheck.cpp:78
+  diag(Using->getLocation(),
+   "using %0 %1 is redundant, already in namespace %2")
+  << Type << ND << NSD;

nit: The canonical way to do diagnostics like this is with select. Then pass 0 
for declaration and 1 for directive.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97361

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

[PATCH] D88220: [C++20] P1825R0: More implicit moves

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth added a comment.

The option is used in the wild, but not as widely as I first believed.  We've 
already fixed a couple projects that were using it.  I think the Release Note 
is the right solution.  Thanks for doing that.  I withdraw my suggestion to 
allow and ignore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

[PATCH] D97233: Support `#pragma clang section` directives on MachO targets

[Jon Roelofs via Phabricator via cfe-commits]

jroelofs added inline comments.



Comment at: clang/lib/Sema/SemaAttr.cpp:304
 
+  if (llvm::Error E = Context.getTargetInfo().isValidSectionSpecifier(SecName))
+Diag(PragmaLoc, diag::err_pragma_section_invalid_for_target)

t.p.northover wrote:
> Shouldn't this block return so the section doesn't get marked as valid later?
yes!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97233

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

[PATCH] D95016: [Clang][RISCV] Add custom TableGen backend for riscv-vector intrinsics.

[Zakk Chen via Phabricator via cfe-commits]

khchen updated this revision to Diff 326107.
khchen marked 2 inline comments as done.
khchen added a comment.

address https://reviews.llvm.org/D95016?id=324197#inline-912573


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95016

Files:
  clang/include/clang/Basic/BuiltinsRISCV.def
  clang/include/clang/Basic/CMakeLists.txt
  clang/include/clang/Basic/riscv_vector.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/test/CodeGen/RISCV/rvv-intrinsics-generic/vadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-generic/vfadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vadd.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vfadd.c
  clang/test/CodeGen/RISCV/vadd.c
  clang/test/Headers/riscv-vector-header.c
  clang/utils/TableGen/CMakeLists.txt
  clang/utils/TableGen/RISCVVEmitter.cpp
  clang/utils/TableGen/TableGen.cpp
  clang/utils/TableGen/TableGenBackends.h
  llvm/docs/CommandGuide/tblgen.rst

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

[PATCH] D97072: [OpenCL][Docs] Add guidelines for adding new extensions and features

[Marco Antognini via Phabricator via cfe-commits]

mantognini added a comment.

Some minor comments.




Comment at: clang/docs/OpenCLSupport.rst:197
+parsing it should not require clang source code modifications. Most commonly
+such extensions add functionality via libraries (by adding new non-native
+types or functions) parsed regularly. Similar to other languages this is the





Comment at: clang/docs/OpenCLSupport.rst:219
+
+Pragmas without detailed information of its behavior (explanation of changes it
+triggers in the parsing) should not be added to clang. Moreover, the acceptable

svenvh wrote:
> 
"of //their// behavior", I think.



Comment at: clang/docs/OpenCLSupport.rst:223
+
+Note that some legacy extensions (published prior to OpenCL 3.0), however still
+provide some non-conformant functionality for pragmas e.g. add diagnostics on




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

https://reviews.llvm.org/D97072

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

[PATCH] D96838: Add GNU attribute 'retain'

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

Aha; attribute `used` *by itself* is not sufficient to preserve sections in the 
output.  But the `__start_/__stop_` symbols implicitly create a reference to 
each of the named sections, and that implicit reference can preserve them in 
the output (assuming gc roots etc).
So, the idea is that attribute `retain` can be used *instead* of the 
`__start_/__stop_` symbols, to preserve sections in the output (with the 
advantage that it will work even for sections that do not have a C-identifier 
name).

Thanks for helping me understand this from a user perspective.  That will be 
important when you go to write the release note for this new attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96838

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

[PATCH] D96769: [OpenMP][AMDGPU] Skip backend and assemble phases for amdgcn

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Replacing `CC1Args.push_back("-emit-llvm-bc");` with 
`CC1Args.push_back("-emit-llvm-bc");` as suggested on the call does not work. 
This hook is downstream of the clang driver, so all it does under save temps is 
lead to `clang -E -emit-llvm`, which generated llvm as requested, that cannot 
be fed into `clang -x cpp-output`.

The handling of `clang -save-temps` that strips out `emit-llvm` from the 
preprocessor pass runs before this.

I do not want to change the semantics of emit-llvm, emit-llvm-bc, or save-temps 
to help out the amdgcn openmp target. I can see that breaking lots of other 
users of clang.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96769

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

[PATCH] D96769: [OpenMP][AMDGPU] Skip backend and assemble phases for amdgcn

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp:192
   CC1Args.push_back("-fcuda-is-device");
-  CC1Args.push_back("-emit-llvm-bc");
 

vaguely interesting that `-emit-llvm` here appears to generate the same code as 
`-emit-llvm-bc`, though neither compose correctly for `-save-temps`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96769

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

[PATCH] D96838: Add GNU attribute 'retain'

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

We have two implementation choices.

1. Lift the source language restriction (it is currently discouraged) on 
llvm.compiler.used, let llvm.used use `SHF_GNU_RETAIN` on ELF, and change 
`__attribute__((used))` to use llvm.compiler.used on ELF.
2. Don't touch the backend semantics of llvm.used or llvm.compiler.used. Add a 
metadata `!retain` and let `__attribute__((retain))` use that. This is what has 
been implemented in D96837  and this patch.

I posted https://lists.llvm.org/pipermail/llvm-dev/2021-February/148760.html 
(llvm-dev & cfe-dev) (my first message mentions that I lean to 1. I actually 
lean to 2.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96838

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

[PATCH] D97273: OpenMP: Fix object clobbering issue when using save-temps

[Johannes Doerfert via Phabricator via cfe-commits]

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

LGTM, assuming it doesn't break support the reasoning makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97273

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

[PATCH] D97273: OpenMP: Fix object clobbering issue when using save-temps

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Works everywhere we have tried it. Fundamentally it renames a temporary file, 
so shouldn't break much. Will be great to have -save-temps working for nvptx.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97273

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

[PATCH] D97072: [OpenCL][Docs] Add guidelines for adding new extensions and features

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added inline comments.



Comment at: clang/docs/OpenCLSupport.rst:196
+If an extension adds the functionality that does not modify standard language
+parsing it should not require clang source code modifications. Most commonly
+such extensions add functionality via libraries (by adding new non-native

Anastasia wrote:
> svenvh wrote:
> > This might need some rephrasing to take TableGen BIFs into account.  I 
> > would consider `clang/lib/Sema/OpenCLBuiltins.td` an integral part of the 
> > clang source code.
> > 
> > Maybe rephrase "clang source code modifications" into "clang parser source 
> > code modifications"?
> I am trying to avoid using parsing in relation to clang since someone might 
> understand it as clang `Parser` library functionality. I have added a note 
> about this case explicitly
> 
> > Note that new functionality in ``OpenCLBuiltins.td`` detailed in 
> > :ref:`` belong to this category even if it is part of the 
> > clang source code.
> 
> Although we do refer to header functionality in the next paragraph anyway 
> where this is referenced but indirectly. I hope it provides enough 
> clarifications for now.
This makes it very confusing for the reader, because you're referring to "this 
category" without defining what a category is.

How about removing that note, and appending to the line above instead:

If an extension adds functionality that does not modify standard language 
parsing it should not require //modifying anything other than header files and 
``OpenCLBuiltins.td`` as detailed in :ref:``//



Comment at: clang/docs/OpenCLSupport.rst:201
+
+Clang has standard headers where new types and functions are being added,
+for more details refer to

Anastasia wrote:
> svenvh wrote:
> > Should this mention `clang/lib/Sema/OpenCLBuiltins.td` too?
> I think here I don't want to go to too many details of how headers are 
> implemented. I refer to this: 
> https://clang.llvm.org/docs/UsersManual.html#opencl-header that also refers 
> to `OpenCLSupport` page where we explain details on Tablegen header. However, 
> the header topic does require another round of clarifications and I do plan 
> to improve it further iin the near future. 
Fair enough; with the addition I'm suggesting above my concern would be 
addressed anyway.



Comment at: clang/docs/OpenCLSupport.rst:219
+
+Pragmas without detailed information of its behavior (explanation of changes it
+triggers in the parsing) should not be added to clang. Moreover, the acceptable

mantognini wrote:
> svenvh wrote:
> > 
> "of //their// behavior", I think.
Yes!



Comment at: clang/docs/OpenCLSupport.rst:221
+triggers in the parsing) should not be added to clang. Moreover, the acceptable
+behavior of pragmas should provide useful functionality to the user.
+

Anastasia wrote:
> svenvh wrote:
> > Remove "acceptable behavior of".
> > 
> > How do you define "useful functionality"?
> I think this is one of those situations that is hard to define unambiguously, 
> so I am open to suggestions. However, I do want to prevent changes that are 
> reinventing the wheel (i.e. C/C++ already had another way to do the same 
> thing) or functionality that doesn't add any value (i.e. requiring pragma 
> enable to use already added types or functions). This has happened in the 
> past multiple times so I think it is good to be specific that when new 
> functionality is added it should have a reason for it.
> 
> I think the other statement above:
> 
> > New functionality is accepted as soon as the documentation is detailed to 
> > the level sufficient to be implemented.
> 
> is similar. It is not very easy to tell what is the detailed level of 
> documentation. FYI it generally aligns with clang overall policy:
> 
> https://clang.llvm.org/get_involved.html
> 
> > A specification: The specification must be sufficient to understand the 
> > design of the feature as well as interpret the meaning of specific 
> > examples. The specification should be detailed enough that another compiler 
> > vendor could implement the feature.
> 
> I am just emphasizing this item here.
> 
> Regarding 'usefulness' I would even suggest adding it as a general guideline 
> for clang but I think this is not very common. So for now I want to make sure 
> we have it in our guidelines at least since we have encountered this.
I see, though I'm a bit hesitant to accept the word "useful" without any 
further explanation. Would adding the following make sense?

... should provide useful functionality //that cannot be achieved by other 
means// to the user.


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

https://reviews.llvm.org/D97072

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

[PATCH] D97233: Support `#pragma clang section` directives on MachO targets

[Jon Roelofs via Phabricator via cfe-commits]

jroelofs updated this revision to Diff 326126.
jroelofs added a comment.

Mark section invalid if the target doesn't like how it's spelled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97233

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenCXX/clang-sections.cpp
  clang/test/Sema/pragma-clang-section-macho.c
  llvm/include/llvm/MC/MCSectionMachO.h
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/MC/MCParser/DarwinAsmParser.cpp
  llvm/lib/MC/MCSectionMachO.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  llvm/test/CodeGen/AArch64/clang-section-macho.ll

Index: llvm/test/CodeGen/AArch64/clang-section-macho.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/clang-section-macho.ll
@@ -0,0 +1,11 @@
+;RUN: llc -mtriple=arm64-apple-ios %s -o - | FileCheck %s
+
+define dso_local void @foo() #0 {
+entry:
+  ret void
+}
+
+attributes #0 = { "implicit-section-name"="__TEXT,__mytext" }
+
+; CHECK:  .section	__TEXT,__mytext
+; CHECK-NEXT: .globl	_foo
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -1899,9 +1899,8 @@
   StringRef ParsedSegment, ParsedSection;
   unsigned TAA = 0, StubSize = 0;
   bool TAAParsed;
-  std::string ErrorCode = MCSectionMachO::ParseSectionSpecifier(
-  Section, ParsedSegment, ParsedSection, TAA, TAAParsed, StubSize);
-  assert(ErrorCode.empty() && "Invalid section specifier.");
+  cantFail(MCSectionMachO::ParseSectionSpecifier(
+  Section, ParsedSegment, ParsedSection, TAA, TAAParsed, StubSize));
 
   // Ignore the globals from the __OBJC section. The ObjC runtime assumes
   // those conform to /usr/lib/objc/runtime.h, so we can't add redzones to
Index: llvm/lib/MC/MCSectionMachO.cpp
===
--- llvm/lib/MC/MCSectionMachO.cpp
+++ llvm/lib/MC/MCSectionMachO.cpp
@@ -174,12 +174,12 @@
 /// flavored .s file.  If successful, this fills in the specified Out
 /// parameters and returns an empty string.  When an invalid section
 /// specifier is present, this returns a string indicating the problem.
-std::string MCSectionMachO::ParseSectionSpecifier(StringRef Spec,// In.
-  StringRef &Segment,// Out.
-  StringRef &Section,// Out.
-  unsigned  &TAA,// Out.
-  bool  &TAAParsed,  // Out.
-  unsigned  &StubSize) { // Out.
+Error MCSectionMachO::ParseSectionSpecifier(StringRef Spec,   // In.
+StringRef &Segment,   // Out.
+StringRef &Section,   // Out.
+unsigned &TAA,// Out.
+bool &TAAParsed,  // Out.
+unsigned &StubSize) { // Out.
   TAAParsed = false;
 
   SmallVector SplitSpec;
@@ -194,25 +194,23 @@
   StringRef Attrs = GetEmptyOrTrim(3);
   StringRef StubSizeStr = GetEmptyOrTrim(4);
 
-  // Verify that the segment is present and not too long.
-  if (Segment.empty() || Segment.size() > 16)
-return "mach-o section specifier requires a segment whose length is "
-   "between 1 and 16 characters";
-
-  // Verify that the section is present and not too long.
+  // Verify that the section is present.
   if (Section.empty())
-return "mach-o section specifier requires a segment and section "
-   "separated by a comma";
+return createStringError(inconvertibleErrorCode(),
+ "mach-o section specifier requires a segment "
+ "and section separated by a comma");
 
+  // Verify that the section is not too long.
   if (Section.size() > 16)
-return "mach-o section specifier requires a section whose length is "
-   "between 1 and 16 characters";
+return createStringError(inconvertibleErrorCode(),
+ "mach-o section specifier requires a section "
+ "whose length is between 1 and 16 characters");
 
   // If there is no comma after the section, we're done.
   TAA = 0;
   StubSize = 0;
   if (SectionType.empty())
-return "";
+return Error::success();
 
   // Figure out which section type it is.
   auto Type

[PATCH] D97226: [clangd] Show hex value of numeric constants

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.

still lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97226

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

[PATCH] D96572: [Clang][ASan] Introduce `-fsanitize-address-destructor-kind=` driver & frontend option.

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

> @jansvoboda11 Thanks. If it's okay with you I'll make the change to use the 
> new marshalling infrastructure in a separate patch 
> (https://reviews.llvm.org/D97327) because I may need to back port this patch 
> to an older LLVM branch.

That's fine by me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96572

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

[PATCH] D97327: [NFC] Switch to auto marshalling infrastructure for `-fsanitize-address-destructor-kind=` flag.

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 accepted this revision.
jansvoboda11 added a comment.

LGTM.

If you don't get around committing this today, please rebase this on top of 
D97375  that I'm going to commit tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97327

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

[PATCH] D96847: [clang][cli] Store additional optimization remarks info

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96847

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

[PATCH] D97400: [clang][NFC] Remove unnecessary string copies in CustomDiagInfo

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: rsmith, aaron.ballman.
njames93 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Lookup the map using a string ref and store indexed diag info using a StringRef 
to the map.
It may be worth using DenseMap to store this, but I'll leave that option on the 
table for now.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97400

Files:
  clang/lib/Basic/DiagnosticIDs.cpp


Index: clang/lib/Basic/DiagnosticIDs.cpp
===
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -356,42 +356,56 @@
 
 namespace clang {
   namespace diag {
-class CustomDiagInfo {
-  typedef std::pair DiagDesc;
-  std::vector DiagInfo;
-  std::map DiagIDs;
-public:
-
-  /// getDescription - Return the description of the specified custom
-  /// diagnostic.
-  StringRef getDescription(unsigned DiagID) const {
-assert(DiagID - DIAG_UPPER_LIMIT < DiagInfo.size() &&
-   "Invalid diagnostic ID");
-return DiagInfo[DiagID-DIAG_UPPER_LIMIT].second;
-  }
-
-  /// getLevel - Return the level of the specified custom diagnostic.
-  DiagnosticIDs::Level getLevel(unsigned DiagID) const {
-assert(DiagID - DIAG_UPPER_LIMIT < DiagInfo.size() &&
-   "Invalid diagnostic ID");
-return DiagInfo[DiagID-DIAG_UPPER_LIMIT].first;
-  }
-
-  unsigned getOrCreateDiagID(DiagnosticIDs::Level L, StringRef Message,
- DiagnosticIDs &Diags) {
-DiagDesc D(L, std::string(Message));
-// Check to see if it already exists.
-std::map::iterator I = DiagIDs.lower_bound(D);
-if (I != DiagIDs.end() && I->first == D)
-  return I->second;
-
-// If not, assign a new ID.
-unsigned ID = DiagInfo.size()+DIAG_UPPER_LIMIT;
-DiagIDs.insert(std::make_pair(D, ID));
-DiagInfo.push_back(D);
-return ID;
-  }
-};
+  using DiagDesc = std::pair;
+  struct DiagDescRef {
+DiagDescRef(DiagnosticIDs::Level Level, StringRef FormatString)
+: Level(Level), FormatString(FormatString) {}
+DiagDescRef(const DiagDesc &D) : DiagDescRef(D.first, D.second) {}
+DiagnosticIDs::Level Level;
+StringRef FormatString;
+  };
+  static bool operator<(const DiagDesc &LHS, const DiagDescRef &RHS) {
+if (LHS.first == RHS.Level)
+  return LHS.second < RHS.FormatString;
+return LHS.first < RHS.Level;
+  }
+  class CustomDiagInfo {
+std::vector DiagInfo;
+std::map> DiagIDs;
+
+  public:
+/// getDescription - Return the description of the specified custom
+/// diagnostic.
+StringRef getDescription(unsigned DiagID) const {
+  assert(DiagID - DIAG_UPPER_LIMIT < DiagInfo.size() &&
+ "Invalid diagnostic ID");
+  return DiagInfo[DiagID - DIAG_UPPER_LIMIT].FormatString;
+}
+
+/// getLevel - Return the level of the specified custom diagnostic.
+DiagnosticIDs::Level getLevel(unsigned DiagID) const {
+  assert(DiagID - DIAG_UPPER_LIMIT < DiagInfo.size() &&
+ "Invalid diagnostic ID");
+  return DiagInfo[DiagID - DIAG_UPPER_LIMIT].Level;
+}
+
+unsigned getOrCreateDiagID(DiagnosticIDs::Level L, StringRef Message,
+   DiagnosticIDs &Diags) {
+  // Check to see if it already exists.
+
+  auto I = DiagIDs.lower_bound(DiagDescRef(L, Message));
+  if (I != DiagIDs.end() && I->first.first == L &&
+  I->first.second == Message)
+return I->second;
+
+  // If not, assign a new ID.
+  unsigned ID = DiagInfo.size() + DIAG_UPPER_LIMIT;
+  auto InsertRet =
+  DiagIDs.insert(std::make_pair(DiagDesc{L, Message.str()}, ID));
+  DiagInfo.emplace_back(InsertRet.first->first);
+  return ID;
+}
+  };
 
   } // end diag namespace
 } // end clang namespace


Index: clang/lib/Basic/DiagnosticIDs.cpp
===
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -356,42 +356,56 @@
 
 namespace clang {
   namespace diag {
-class CustomDiagInfo {
-  typedef std::pair DiagDesc;
-  std::vector DiagInfo;
-  std::map DiagIDs;
-public:
-
-  /// getDescription - Return the description of the specified custom
-  /// diagnostic.
-  StringRef getDescription(unsigned DiagID) const {
-assert(DiagID - DIAG_UPPER_LIMIT < DiagInfo.size() &&
-   "Invalid diagnostic ID");
-return DiagInfo[DiagID-DIAG_UPPER_LIMIT].second;
-  }
-
-  /// getLevel - Return the level of the specified custom diagnostic.
-  DiagnosticIDs::Level getLevel(unsigned DiagID) const {
-assert(DiagID - DIAG_UPPER_LIMIT < DiagInfo.size() &&
-   "Invalid diagnostic ID");
-return DiagInfo[Diag

[PATCH] D97340: [HIP] Support Spack packages

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:25-31
+// Look for sub-directory starts with Prefix under Path. If there is one and
+// only one matching sub-directory found, append the sub-directory to Path. If
+// there is no matching sub-directory or there are more than one matching
+// sub-directories, diagnose them. Returns true if there is only one matching
+// sub-directory.
+static void handleSPACKPackage(const Driver &D, SmallString<0> &Path,
+   StringRef Prefix) {

`void` function can not return `true`, so the comment needs updating.

Also, Using Path as both an input and an output is odd. We should probably 
return the full path or an empty string and call the function 
`findSPACKPackage`.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:27-28
+// only one matching sub-directory found, append the sub-directory to Path. If
+// there is no matching sub-directory or there are more than one matching
+// sub-directories, diagnose them. Returns true if there is only one matching
+// sub-directory.

How are users expected to handle cases when they do have multiple rocm versions 
installed with spack (I assume that having multiple packages  *is* possible. 
E.g. package-4.0, package-3.7 should be able to co-exist). They may have 
legitimate reasons to have more than one rocm version installed *and* they may 
need to be able to build with a particular one.

Can users use `--hip-path=specific/hip/version/in-spack` ?



Comment at: clang/lib/Driver/ToolChains/ROCm.h:56
+// convention --.
+llvm::SmallString<0> SPACKReleaseStr;
+  };

Nit: using `SmallString` here and everywhere else in this patch does not buy us 
anything. `std::string` would be fine.
My own rule of thumb is to use standard types by default and optimized types 
when they are needed. 


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

https://reviews.llvm.org/D97340

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

[PATCH] D92195: [OPENMP50]Mapping of the subcomponents with the 'default' mappers.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev updated this revision to Diff 326144.
ABataev added a comment.

Fixes and updates


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92195

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/declare_mapper_codegen.cpp
  clang/test/OpenMP/target_map_codegen_34.cpp
  openmp/libomptarget/src/omptarget.cpp
  openmp/libomptarget/src/private.h
  openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers.cpp

Index: openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/declare_mapper_nested_default_mappers.cpp
@@ -0,0 +1,63 @@
+// RUN: %libomptarget-compilexx-run-and-check-aarch64-unknown-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64le-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-x86_64-pc-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-nvptx64-nvidia-cuda
+
+#include 
+#include 
+
+typedef struct {
+  int a;
+  double *b;
+} C1;
+#pragma omp declare mapper(C1 s) map(to : s.a) map(from : s.b [0:2])
+
+typedef struct {
+  int a;
+  double *b;
+  C1 c;
+} C;
+#pragma omp declare mapper(C s) map(to : s.a, s.c) map(from : s.b [0:2])
+
+typedef struct {
+  int e;
+  C f;
+  int h;
+} D;
+
+int main() {
+  constexpr int N = 10;
+  D s;
+  s.e = 111;
+  s.f.a = 222;
+  s.f.c.a = 777;
+  double x[2];
+  double x1[2];
+  x[1] = 20;
+  s.f.b = &x[0];
+  s.f.c.b = &x1[0];
+  s.h = N;
+
+  D *sp = &s;
+  D **spp = &sp;
+
+  printf("%d %d %d %4.5f %d\n", spp[0][0].e, spp[0][0].f.a, spp[0][0].f.c.a,
+ spp[0][0].f.b[1], spp[0][0].f.b == &x[0] ? 1 : 0);
+  // CHECK: 111 222 777 20.0 1
+
+  __intptr_t p = reinterpret_cast<__intptr_t>(&x[0]);
+#pragma omp target map(tofrom : spp[0][0]) firstprivate(p)
+  {
+printf("%d %d %d\n", spp[0][0].f.a, spp[0][0].f.c.a,
+   spp[0][0].f.b == reinterpret_cast(p) ? 1 : 0);
+// CHECK: 222 777 0
+spp[0][0].e = 333;
+spp[0][0].f.a = 444;
+spp[0][0].f.c.a = 555;
+spp[0][0].f.b[1] = 40;
+  }
+  printf("%d %d %d %4.5f %d\n", spp[0][0].e, spp[0][0].f.a, spp[0][0].f.c.a,
+ spp[0][0].f.b[1], spp[0][0].f.b == &x[0] ? 1 : 0);
+  // CHECK: 333 222 777 40.0 1
+}
Index: openmp/libomptarget/src/private.h
===
--- openmp/libomptarget/src/private.h
+++ openmp/libomptarget/src/private.h
@@ -23,17 +23,20 @@
 extern int targetDataBegin(ident_t *loc, DeviceTy &Device, int32_t arg_num,
void **args_base, void **args, int64_t *arg_sizes,
int64_t *arg_types, map_var_info_t *arg_names,
-   void **arg_mappers, AsyncInfoTy &AsyncInfo);
+   void **arg_mappers, AsyncInfoTy &AsyncInfo,
+   bool FromMapper = false);
 
 extern int targetDataEnd(ident_t *loc, DeviceTy &Device, int32_t ArgNum,
  void **ArgBases, void **Args, int64_t *ArgSizes,
  int64_t *ArgTypes, map_var_info_t *arg_names,
- void **ArgMappers, AsyncInfoTy &AsyncInfo);
+ void **ArgMappers, AsyncInfoTy &AsyncInfo,
+ bool FromMapper = false);
 
 extern int targetDataUpdate(ident_t *loc, DeviceTy &Device, int32_t arg_num,
 void **args_base, void **args, int64_t *arg_sizes,
 int64_t *arg_types, map_var_info_t *arg_names,
-void **arg_mappers, AsyncInfoTy &AsyncInfo);
+void **arg_mappers, AsyncInfoTy &AsyncInfo,
+bool FromMapper = false);
 
 extern int target(ident_t *loc, DeviceTy &Device, void *HostPtr, int32_t ArgNum,
   void **ArgBases, void **Args, int64_t *ArgSizes,
@@ -76,7 +79,8 @@
 // targetDataEnd and targetDataUpdate).
 typedef int (*TargetDataFuncPtrTy)(ident_t *, DeviceTy &, int32_t, void **,
void **, int64_t *, int64_t *,
-   map_var_info_t *, void **, AsyncInfoTy &);
+   map_var_info_t *, void **, AsyncInfoTy &,
+   bool);
 
 // Implemented in libomp, they are called from within __tgt_* functions.
 #ifdef __cplusplus
Index: openmp/libomptarget/src/omptarget.cpp
===
--- openmp/libomptarget/src/omptarget.cpp
+++ openmp/libomptarget/src/omptarget.cpp
@@ -269,10 +269,11 @@
 MapperArgNames[I] = C.Name;
   }
 
-  int rc = target_data_function(
-  loc, Device, MapperComponents.Components.size(), MapperArgsBa

[PATCH] D96976: [analyzer] Fix reinterpret_cast handling for pointer-to-member

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD added a comment.

@steakhal, could you please review this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96976

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

[PATCH] D97123: [clangd] Support FixIts that use InsertFromRange instead of inserting raw text

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/SourceCode.cpp:555
+auto Insert = Lexer::getSourceText(FixIt.InsertFromRange, M, L, &Invalid);
+if (!Invalid)
+  Result.newText = Insert.str();

njames93 wrote:
> kadircet wrote:
> > njames93 wrote:
> > > kadircet wrote:
> > > > it feels like correct semantics would be to make this function fail 
> > > > now. as the resulting value will likely be used for editing the source 
> > > > code and we don't want to propagate some garbage.
> > > While I agree with this in principal. We currently don't handle the case 
> > > where RemoveRange doesn't correspond to a FileCharRange which would 
> > > likely need propagating. Though likely in a separate patch. 
> > i discussed the situation with sam offline (as you've also noticed most of 
> > the sourcelocation -> lsp conversations within this file doesn't really 
> > check for errors), and it looks like this was intentional. we are trying to 
> > cover as much ground as possible in diagnostics.cpp, especially in 
> > https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/Diagnostics.cpp#L677.
> >  we should do the same for `InsertFromRange`. all the macroarg logic isn't 
> > necessary (until we notice it is), so just bailing out when the 
> > `InsertFromRange` is a macroid or outside mainfile in `AddFix` should be 
> > enough.
> > 
> > as for testing it, looks like you can invoke a fixit with `InsertFromRange` 
> > via:
> > ```
> > struct Foo {};
> > struct Bar : public [[noreturn]] Foo {};
> > ```
> > 
> > it would be nice to also check with an attribute coming from a macro 
> > expansions.
> Can you elaborate on why we should disregard `InsertFromRange` if it points 
> outside the main file. That seems like an unnecessary restriction. It's 
> probably also safe if its a macro ID, though there's more likely to be some 
> edge case there.
> 
> As for the diagnostic. I don't think there's any value testing that. The test 
> in `SourceCodeTests` covers inserting from range pretty well. It would be 
> nice to test the synthetic messages, but I don't think any diagnostic in 
> clang/clang-tidy contains just 1 fix-it that's an InsertFromRange.
> Can you elaborate on why we should disregard InsertFromRange if it points 
> outside the main file. That seems like an unnecessary restriction.

because we build preamble and main file separately, the sourcemanager is likely 
missing files apart from the main file. hence when one tries to access a 
sourcelocation in them, the file will be re-read from disk and if contents on 
disk change for some reason, clangd might crash when the offsets are off.

>  It's probably also safe if its a macro ID, though there's more likely to be 
> some edge case there.

safeness is a little wrinkly here due to the edge cases (that are hard to 
predict, until we see them in practice) you point out as well. but moreover, do 
we really want to introduce a piece of text coming from a macro expansion into 
source code? is there any use cases for that? (i'd say it should be the macro 
name that should be inserted, not its expansion.)

> As for the diagnostic. I don't think there's any value testing that. 

i wasn't suggesting to introduce a diagnostic test to check the behaviour of 
`toTextEdit`. it was for testing the new logic we'll introduce into 
diagnostics.cpp, sorry if i wasn't clear:/. the tests you currently have are 
totally fine for testing `toTextEdit`.



Comment at: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp:805
 
+Expected sourceRangeInMainFile(SourceManager &SM, Range R) {
+  if (auto Beg = sourceLocationInMainFile(SM, R.start)) {

sorry if i was vague in preivous comment, i was suggesting making this function 
return a SourceRange all the time by wrapping `sourceLocationInMainFile`s with 
`cantFail`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97123

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

[PATCH] D95119: Prefer /usr/bin/env xxx over /usr/bin/xxx where xxx = perl, python, awk

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added a comment.

In D95119#2584709 , @haampie wrote:

> Should this be merged?

Do you have commit access? If not I can land this for you.


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

https://reviews.llvm.org/D95119

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

[PATCH] D95119: Prefer /usr/bin/env xxx over /usr/bin/xxx where xxx = perl, python, awk

[Harmen Stoppels via Phabricator via cfe-commits]

haampie added a comment.

I don't have commit access, would be great if you could do that for me!


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

https://reviews.llvm.org/D95119

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

[PATCH] D96832: [Clang][Attributes] Allow not_tail_called attribute to be applied to virtual function.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:4109
-  [[clang::not_tail_called]] int foo2() override;
-};
   }];

aaron.ballman wrote:
> rnk wrote:
> > aaron.ballman wrote:
> > > Quuxplusone wrote:
> > > > aaron.ballman wrote:
> > > > > ahatanak wrote:
> > > > > > Quuxplusone wrote:
> > > > > > > (Moving into a thread)
> > > > > > > 
> > > > > > > > This patch doesn't prevent the call to method in the code below 
> > > > > > > > from being tail called,
> > > > > > > > but I suppose users would expect the attribute to prevent the 
> > > > > > > > tail call?
> > > > > > > ```
> > > > > > > struct B {
> > > > > > >   virtual void method();  
> > > > > > > };
> > > > > > > struct D : B {
> > > > > > >   [[clang::not_tail_called]] void method() override; 
> > > > > > > };
> > > > > > > ```
> > > > > > > 
> > > > > > > The way virtual calls are handled in C++ is, all attributes and 
> > > > > > > properties of the call are determined based on the //static// 
> > > > > > > type at the call site; and then the //runtime destination// of 
> > > > > > > the call is determined from the pointer in the vtable. Attributes 
> > > > > > > and properties have no runtime existence, and so they physically 
> > > > > > > cannot affect anything at runtime. Consider 
> > > > > > > https://godbolt.org/z/P3799e :
> > > > > > > 
> > > > > > > ```
> > > > > > > struct Ba {
> > > > > > >   virtual Ba *method(int x = 1);  
> > > > > > > };
> > > > > > > struct Da : Ba {
> > > > > > >   [[clang::not_tail_called]] [[nodiscard]] Da *method(int x = 2) 
> > > > > > > noexcept override; 
> > > > > > > };
> > > > > > > auto callera(Da& da) {
> > > > > > > Ba& ba = da;
> > > > > > > ba.method();
> > > > > > > }
> > > > > > > ```
> > > > > > > Here the call that is made is a //virtual// call (because 
> > > > > > > `Ba::method` is virtual); with a default argument value of `1` 
> > > > > > > (because `Ba::method`'s `x` parameter has a default value of 
> > > > > > > `1`); and it returns something of type `Ba*` (because that's what 
> > > > > > > `Ba::method` returns); and it is not considered to be noexcept 
> > > > > > > (because `Ba::method` isn't marked noexcept); and it's okay to 
> > > > > > > discard the result (because `Ba::method` is not nodiscard) and it 
> > > > > > > is tail-called (because `Ba::method` doesn't disallow tail 
> > > > > > > calls). All of these attributes and properties are based on the 
> > > > > > > //static// type of variable `ba`, despite the fact that //at 
> > > > > > > runtime// we'll end up jumping to the code for `Da::method`. 
> > > > > > > According to the source code, statically, `Da::method` has a 
> > > > > > > default argument of `2`, returns `Da*`, is noexcept, and is 
> > > > > > > nodiscard, and disallows tail-calls. But we're not calling 
> > > > > > > `da.method()`, we're calling `ba.method()`; so none of that 
> > > > > > > matters to our call site at `callera`.
> > > > > > > 
> > > > > > > I think this patch is a good thing.
> > > > > > OK, I see. I think this patch is fine then.
> > > > > > 
> > > > > > Should we add an explanation of how virtual functions are handled? 
> > > > > > The doc currently just says the attribute prevents tail-call 
> > > > > > optimization on statically bound calls.
> > > > > > I think this patch is a good thing.
> > > > > 
> > > > > I agree with your explanation but I'm not certain I agree with your 
> > > > > conclusion. :-) I think the examples you point out are more often a 
> > > > > source of confusion for users than not because of the nature of 
> > > > > static vs dynamic dispatch, and so saying "this behavior can be 
> > > > > consistent with all of these other things people often get confused 
> > > > > by" may be justifiable but also seems a bit user-hostile.
> > > > > 
> > > > > Taking a slightly different example:
> > > > > ```
> > > > > struct Ba {
> > > > >   [[clang::not_tail_called]] virtual Ba *method();  
> > > > > };
> > > > > struct Da : Ba {
> > > > >   Ba *method() override; 
> > > > > };
> > > > > 
> > > > > void callera(Da& da) {
> > > > > Ba& ba = da;
> > > > > ba.method();
> > > > > }
> > > > > ```
> > > > > There's no guarantee that `Ba::method()` and `Da::method()` have the 
> > > > > same not-tail-called properties. The codegen for this case will still 
> > > > > attach `notail` to the call site even though the dynamic target may 
> > > > > not meet that requirement.
> > > > > 
> > > > > tl;dr: I think `notail` is a property of the call expression and the 
> > > > > only way to know that's valid is when you know what's being called, 
> > > > > so the current behavior is more user-friendly for avoiding 
> > > > > optimization issues. I'd prefer not to relax that unless there was a 
> > > > > significantly motivating example beyond what's presented here so far.
> > > > > saying "this behavior can be consistent with all of these other 
> > > > > things

[PATCH] D96847: [clang][cli] Store additional optimization remarks info

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

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96847

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

[PATCH] D96638: [Driver][Windows] Support per-target runtimes dir layout for profile instr generate

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/lib/Driver/ToolChain.cpp:446
   case ToolChain::FT_Shared:
-Suffix = TT.isOSWindows()
- ? (TT.isWindowsGNUEnvironment() ? ".dll.a" : ".lib")
+Suffix = Triple.isOSWindows()
+ ? (Triple.isWindowsGNUEnvironment() ? ".dll.a" : ".lib")

This is the same as `TT`, right? Please use TT here or remove TT and use Triple 
across this function. I guess I lean towards keeping TT, but it's up to you.



Comment at: clang/lib/Driver/ToolChain.cpp:468-469
   // Check for runtime files in the new layout without the architecture first.
-  std::string CRTBasename =
-  buildCompilerRTBasename(*this, Args, Component, Type, /*AddArch=*/false);
-  for (const auto &LibPath : getLibraryPaths()) {
-SmallString<128> P(LibPath);
-llvm::sys::path::append(P, CRTBasename);
-if (getVFS().exists(P))
-  return std::string(P.str());
+  std::string CRTBasename = getCompilerRTBasename(Args, Component, Type);
+  if (auto value = findInLibraryPath(*this, CRTBasename)) {
+return value.getValue();

This does twice as many stat syscalls as we need. That seems worth optimizing. 
First, in getCompilerRTBasename, we search the library path, we return a 
result, and then we search it again here. I think the nicest solution is 
probably to have one shared implementation that takes a boolean and returns the 
full path or the basename.


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

https://reviews.llvm.org/D96638

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

[PATCH] D96638: [Driver][Windows] Support per-target runtimes dir layout for profile instr generate

[Markus Böck via Phabricator via cfe-commits]

zero9178 updated this revision to Diff 326178.
zero9178 added a comment.

Addressed one of the reviewer requests and also uploaded additional files part 
of the patch that were accidently left out.


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

https://reviews.llvm.org/D96638

Files:
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/BareMetal.h
  clang/test/Driver/cl-options.c
  clang/test/Driver/fsanitize.c
  clang/test/Driver/instrprof-ld.c
  clang/test/Driver/sanitizer-ld.c

Index: clang/test/Driver/sanitizer-ld.c
===
--- clang/test/Driver/sanitizer-ld.c
+++ clang/test/Driver/sanitizer-ld.c
@@ -666,16 +666,16 @@
 // RUN: -target x86_64-pc-windows \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CFI-STATS-WIN64 %s
-// CHECK-CFI-STATS-WIN64: "--dependent-lib=clang_rt.stats_client-x86_64.lib"
-// CHECK-CFI-STATS-WIN64: "--dependent-lib=clang_rt.stats-x86_64.lib"
+// CHECK-CFI-STATS-WIN64: "--dependent-lib=clang_rt.stats_client{{(-x86_64)?}}.lib"
+// CHECK-CFI-STATS-WIN64: "--dependent-lib=clang_rt.stats{{(-x86_64)?}}.lib"
 // CHECK-CFI-STATS-WIN64: "--linker-option=/include:__sanitizer_stats_register"
 
 // RUN: %clang -fsanitize=cfi -fsanitize-stats %s -### -o %t.o 2>&1 \
 // RUN: -target i686-pc-windows \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-CFI-STATS-WIN32 %s
-// CHECK-CFI-STATS-WIN32: "--dependent-lib=clang_rt.stats_client-i386.lib"
-// CHECK-CFI-STATS-WIN32: "--dependent-lib=clang_rt.stats-i386.lib"
+// CHECK-CFI-STATS-WIN32: "--dependent-lib=clang_rt.stats_client{{(-i386)?}}.lib"
+// CHECK-CFI-STATS-WIN32: "--dependent-lib=clang_rt.stats{{(-i386)?}}.lib"
 // CHECK-CFI-STATS-WIN32: "--linker-option=/include:___sanitizer_stats_register"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
Index: clang/test/Driver/instrprof-ld.c
===
--- clang/test/Driver/instrprof-ld.c
+++ clang/test/Driver/instrprof-ld.c
@@ -112,7 +112,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-WINDOWS-I386 %s
 //
 // CHECK-WINDOWS-I386: "{{.*}}link{{(.exe)?}}"
-// CHECK-WINDOWS-I386: "{{.*}}clang_rt.profile-i386.lib"
+// CHECK-WINDOWS-I386: "{{.*}}clang_rt.profile{{(-i386)?}}.lib"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-pc-win32 -fprofile-instr-generate \
@@ -120,7 +120,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-WINDOWS-X86-64 %s
 //
 // CHECK-WINDOWS-X86-64: "{{.*}}link{{(.exe)?}}"
-// CHECK-WINDOWS-X86-64: "{{.*}}clang_rt.profile-x86_64.lib"
+// CHECK-WINDOWS-X86-64: "{{.*}}clang_rt.profile{{(-x86_64)?}}.lib"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: -target x86_64-mingw32 -fprofile-instr-generate -fuse-ld=ld \
@@ -136,7 +136,7 @@
 // RUN: -fprofile-instr-generate 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-WINDOWS-X86-64-DEPENDENT-LIB %s
 //
-// CHECK-WINDOWS-X86-64-DEPENDENT-LIB: "--dependent-lib={{[^"]*}}clang_rt.profile-{{[^"]*}}.lib"
+// CHECK-WINDOWS-X86-64-DEPENDENT-LIB: "--dependent-lib={{[^"]*}}clang_rt.profile{{[^"]*}}.lib"
 //
 // RUN: %clang %s -### -o %t.o -target x86_64-mingw32 \
 // RUN: -fprofile-instr-generate 2>&1 \
Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -20,17 +20,17 @@
 // RUN: %clang -target x86_64-pc-win32 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-UNDEFINED-WIN64,CHECK-UNDEFINED-MSVC
 // RUN: %clang -target x86_64-w64-mingw32 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-WIN64-MINGW
 // RUN: %clang -target x86_64-pc-win32 -fsanitize=undefined -x c++ %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-UNDEFINED-WIN64,CHECK-UNDEFINED-WIN-CXX,CHECK-UNDEFINED-MSVC
-// CHECK-UNDEFINED-WIN32: "--dependent-lib={{[^"]*}}ubsan_standalone-i386.lib"
-// CHECK-UNDEFINED-WIN64: "--dependent-lib={{[^"]*}}ubsan_standalone-x86_64.lib"
-// CHECK-UNDEFINED-WIN64-MINGW: "--dependent-lib={{[^"]*}}libclang_rt.ubsan_standalone-x86_64.a"
+// CHECK-UNDEFINED-WIN32: "--dependent-lib={{[^"]*}}ubsan_standalone{{(-i386)?}}.lib"
+// CHECK-UNDEFINED-WIN64: "--dependent-lib={{[^"]*}}ubsan_standalone{{(-x86_64)?}}.lib"
+// CHECK-UNDEFINED-WIN64-MINGW: "--dependent-lib={{[^"]*}}libclang_rt.ubsan_standalone{{(-x86_64)?}}.a"
 // CHECK-UNDEFINED-WIN-CXX: "--dependent-lib={{[^"]*}}ubsan_standalone_cxx{{[^"]*}}.lib"
 // CHECK-UNDEFINED-MSVC-SAME: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|builtin|returns-nonnull-attribute|

[PATCH] D96638: [Driver][Windows] Support per-target runtimes dir layout for profile instr generate

[Markus Böck via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChain.cpp:468-469
   // Check for runtime files in the new layout without the architecture first.
-  std::string CRTBasename =
-  buildCompilerRTBasename(*this, Args, Component, Type, /*AddArch=*/false);
-  for (const auto &LibPath : getLibraryPaths()) {
-SmallString<128> P(LibPath);
-llvm::sys::path::append(P, CRTBasename);
-if (getVFS().exists(P))
-  return std::string(P.str());
+  std::string CRTBasename = getCompilerRTBasename(Args, Component, Type);
+  if (auto value = findInLibraryPath(*this, CRTBasename)) {
+return value.getValue();

rnk wrote:
> This does twice as many stat syscalls as we need. That seems worth 
> optimizing. First, in getCompilerRTBasename, we search the library path, we 
> return a result, and then we search it again here. I think the nicest 
> solution is probably to have one shared implementation that takes a boolean 
> and returns the full path or the basename.
Refactoring the current implementation out of getCompilerRTBasename is 
currently not possible as that would break the BareMetal toolchain as it 
overrides getCompilerRTBasename to build up a different scheme for it's runtime 
libraries. Not calling getCompilerRTBasename is what caused the test failure 
previously.

Unless you mean that I should add an optional boolean operand to 
getCompilerRTBasename that would get either the absolute path if possible or 
always return a relative path? That could work and would change getCompilerRT 
to not go through the library path but to instead check if that path is 
absolute and return if it is already. 






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

https://reviews.llvm.org/D96638

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

[PATCH] D96832: [Clang][Attributes] Allow not_tail_called attribute to be applied to virtual function.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:4109
-  [[clang::not_tail_called]] int foo2() override;
-};
   }];

rnk wrote:
> aaron.ballman wrote:
> > rnk wrote:
> > > aaron.ballman wrote:
> > > > Quuxplusone wrote:
> > > > > aaron.ballman wrote:
> > > > > > ahatanak wrote:
> > > > > > > Quuxplusone wrote:
> > > > > > > > (Moving into a thread)
> > > > > > > > 
> > > > > > > > > This patch doesn't prevent the call to method in the code 
> > > > > > > > > below from being tail called,
> > > > > > > > > but I suppose users would expect the attribute to prevent the 
> > > > > > > > > tail call?
> > > > > > > > ```
> > > > > > > > struct B {
> > > > > > > >   virtual void method();  
> > > > > > > > };
> > > > > > > > struct D : B {
> > > > > > > >   [[clang::not_tail_called]] void method() override; 
> > > > > > > > };
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > The way virtual calls are handled in C++ is, all attributes and 
> > > > > > > > properties of the call are determined based on the //static// 
> > > > > > > > type at the call site; and then the //runtime destination// of 
> > > > > > > > the call is determined from the pointer in the vtable. 
> > > > > > > > Attributes and properties have no runtime existence, and so 
> > > > > > > > they physically cannot affect anything at runtime. Consider 
> > > > > > > > https://godbolt.org/z/P3799e :
> > > > > > > > 
> > > > > > > > ```
> > > > > > > > struct Ba {
> > > > > > > >   virtual Ba *method(int x = 1);  
> > > > > > > > };
> > > > > > > > struct Da : Ba {
> > > > > > > >   [[clang::not_tail_called]] [[nodiscard]] Da *method(int x = 
> > > > > > > > 2) noexcept override; 
> > > > > > > > };
> > > > > > > > auto callera(Da& da) {
> > > > > > > > Ba& ba = da;
> > > > > > > > ba.method();
> > > > > > > > }
> > > > > > > > ```
> > > > > > > > Here the call that is made is a //virtual// call (because 
> > > > > > > > `Ba::method` is virtual); with a default argument value of `1` 
> > > > > > > > (because `Ba::method`'s `x` parameter has a default value of 
> > > > > > > > `1`); and it returns something of type `Ba*` (because that's 
> > > > > > > > what `Ba::method` returns); and it is not considered to be 
> > > > > > > > noexcept (because `Ba::method` isn't marked noexcept); and it's 
> > > > > > > > okay to discard the result (because `Ba::method` is not 
> > > > > > > > nodiscard) and it is tail-called (because `Ba::method` doesn't 
> > > > > > > > disallow tail calls). All of these attributes and properties 
> > > > > > > > are based on the //static// type of variable `ba`, despite the 
> > > > > > > > fact that //at runtime// we'll end up jumping to the code for 
> > > > > > > > `Da::method`. According to the source code, statically, 
> > > > > > > > `Da::method` has a default argument of `2`, returns `Da*`, is 
> > > > > > > > noexcept, and is nodiscard, and disallows tail-calls. But we're 
> > > > > > > > not calling `da.method()`, we're calling `ba.method()`; so none 
> > > > > > > > of that matters to our call site at `callera`.
> > > > > > > > 
> > > > > > > > I think this patch is a good thing.
> > > > > > > OK, I see. I think this patch is fine then.
> > > > > > > 
> > > > > > > Should we add an explanation of how virtual functions are 
> > > > > > > handled? The doc currently just says the attribute prevents 
> > > > > > > tail-call optimization on statically bound calls.
> > > > > > > I think this patch is a good thing.
> > > > > > 
> > > > > > I agree with your explanation but I'm not certain I agree with your 
> > > > > > conclusion. :-) I think the examples you point out are more often a 
> > > > > > source of confusion for users than not because of the nature of 
> > > > > > static vs dynamic dispatch, and so saying "this behavior can be 
> > > > > > consistent with all of these other things people often get confused 
> > > > > > by" may be justifiable but also seems a bit user-hostile.
> > > > > > 
> > > > > > Taking a slightly different example:
> > > > > > ```
> > > > > > struct Ba {
> > > > > >   [[clang::not_tail_called]] virtual Ba *method();  
> > > > > > };
> > > > > > struct Da : Ba {
> > > > > >   Ba *method() override; 
> > > > > > };
> > > > > > 
> > > > > > void callera(Da& da) {
> > > > > > Ba& ba = da;
> > > > > > ba.method();
> > > > > > }
> > > > > > ```
> > > > > > There's no guarantee that `Ba::method()` and `Da::method()` have 
> > > > > > the same not-tail-called properties. The codegen for this case will 
> > > > > > still attach `notail` to the call site even though the dynamic 
> > > > > > target may not meet that requirement.
> > > > > > 
> > > > > > tl;dr: I think `notail` is a property of the call expression and 
> > > > > > the only way to know that's valid is when you know what's being 
> > > > > > called, so the current behavior is more user-friendly for avoiding 
> > > > > > optimization issues. I'd

[PATCH] D96769: [OpenMP][AMDGPU] Skip backend and assemble phases for amdgcn

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Something we should probably check is the interaction between -save-temps and 
whether we are trying to compile a single file or an executable, e.g. the 
difference between clang and clang -c.

If trying to compile foo.c directly to an executable, -save-temps should 
probably print (along with lots of others) an assembly file containing the 
contents of the code object (the shared elf containing amdgcn machine code). If 
trying to compile only the host part, we probably want to emit asm. If trying 
to compile only the target part, we probably don't - the llc part is likely to 
error if the rest of the application is missing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96769

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

[PATCH] D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete.

[Amy Huang via Phabricator via cfe-commits]

akhuang created this revision.
akhuang added reviewers: rnk, dblaikie, rsmith.
Herald added a reviewer: aaron.ballman.
akhuang requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This was motivated by the fact that constructor type homing (debug info
optimization that we want to turn on by default) drops some libc++ types,
so an attribute would allow us to override constructor homing and emit
them anyway. I'm currently looking into the particular libc++ issue, but
even if we do fix that, this issue might come up elsewhere and it might be
nice to have this.

As I've implemented it now, the attribute isn't specific to the
constructor homing optimization and overrides all of the debug info
optimizations.

Open to discussion about naming, specifics on what the attribute should do, etc.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97411

Files:
  clang/include/clang/Basic/Attr.td
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCXX/force-debug-attribute.cpp


Index: clang/test/CodeGenCXX/force-debug-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/force-debug-attribute.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -DSETATTR=0 -emit-llvm -std=c++14 
-debug-info-kind=constructor %s -o - | FileCheck %s --check-prefix=DEBUG
+// RUN: %clang_cc1 -DSETATTR=1 -emit-llvm -std=c++14 
-debug-info-kind=constructor %s -o - | FileCheck %s --check-prefix=WITHATTR
+
+#if SETATTR
+#define DEBUGASNEEDED __attribute__((force_debug_if_required_type))
+#else
+#define DEBUGASNEEDED
+#endif
+
+// Struct that isn't constructed, so its full type info should be omitted with
+// -debug-info-kind=constructor.
+struct DEBUGASNEEDED some_struct {
+  some_struct() {}
+};
+
+void func1(some_struct s) {}
+// void func2() { func1(); }
+// DEBUG:  !DICompositeType({{.*}}name: "some_struct"
+// DEBUG-SAME:  flags: {{.*}}DIFlagFwdDecl
+// WITHATTR: !DICompositeType({{.*}}name: "some_struct"
+// WITHATTR-NOT: DIFlagFwdDecl
+
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2399,6 +2399,10 @@
   if (!CXXDecl)
 return false;
 
+  // Don't omit definition if marked with attribute.
+  if (RD->hasAttr())
+return false;
+
   // Only emit complete debug info for a dynamic class when its vtable is
   // emitted.  However, Microsoft debuggers don't resolve type information
   // across DLL boundaries, so skip this optimization if the class or any of 
its
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -1660,6 +1660,13 @@
   let Documentation = [NoDebugDocs];
 }
 
+def ForceDebugIfRequiredType : InheritableAttr {
+  let Spellings = [Clang<"force_debug_if_required_type">];
+  let Subjects = SubjectList<[CXXRecord]>;
+  let Documentation = [Undocumented];
+  let SimpleHandler = 1;
+}
+
 def NoDuplicate : InheritableAttr {
   let Spellings = [Clang<"noduplicate">];
   let Subjects = SubjectList<[Function]>;


Index: clang/test/CodeGenCXX/force-debug-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/force-debug-attribute.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -DSETATTR=0 -emit-llvm -std=c++14 -debug-info-kind=constructor %s -o - | FileCheck %s --check-prefix=DEBUG
+// RUN: %clang_cc1 -DSETATTR=1 -emit-llvm -std=c++14 -debug-info-kind=constructor %s -o - | FileCheck %s --check-prefix=WITHATTR
+
+#if SETATTR
+#define DEBUGASNEEDED __attribute__((force_debug_if_required_type))
+#else
+#define DEBUGASNEEDED
+#endif
+
+// Struct that isn't constructed, so its full type info should be omitted with
+// -debug-info-kind=constructor.
+struct DEBUGASNEEDED some_struct {
+  some_struct() {}
+};
+
+void func1(some_struct s) {}
+// void func2() { func1(); }
+// DEBUG:  !DICompositeType({{.*}}name: "some_struct"
+// DEBUG-SAME:  flags: {{.*}}DIFlagFwdDecl
+// WITHATTR: !DICompositeType({{.*}}name: "some_struct"
+// WITHATTR-NOT: DIFlagFwdDecl
+
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2399,6 +2399,10 @@
   if (!CXXDecl)
 return false;
 
+  // Don't omit definition if marked with attribute.
+  if (RD->hasAttr())
+return false;
+
   // Only emit complete debug info for a dynamic class when its vtable is
   // emitted.  However, Microsoft debuggers don't resolve type information
   // across DLL boundaries, so skip this optimization if the class or any of its
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/At

[PATCH] D65696: Implements CWG 2082 Referring to parameters in unevaluated operands of default arguments

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

rsmith added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:143-148
+  /// CheckDefaultArgumentVisitorODR - C++ [dcl.fct.default] Traverses
+  /// the default argument of a parameter to determine whether it
+  /// contains ODR violations. These violations cannot be checked in
+  /// \ref CheckDefaultArgumentVisitor since the DeclRefExp's may be changed to
+  /// an implicit cast from an LValue to RValue by \ref 
SetParamDefaultArgument.
+  /// When that happens the ODR usage may be allowed.

Instead of visiting the default argument twice, can we call 
`SetParamDefaultArgument` first before doing the checks?


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

https://reviews.llvm.org/D65696

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

[PATCH] D96638: [Driver][Windows] Support per-target runtimes dir layout for profile instr generate

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: clang/lib/Driver/ToolChain.cpp:468-469
   // Check for runtime files in the new layout without the architecture first.
-  std::string CRTBasename =
-  buildCompilerRTBasename(*this, Args, Component, Type, /*AddArch=*/false);
-  for (const auto &LibPath : getLibraryPaths()) {
-SmallString<128> P(LibPath);
-llvm::sys::path::append(P, CRTBasename);
-if (getVFS().exists(P))
-  return std::string(P.str());
+  std::string CRTBasename = getCompilerRTBasename(Args, Component, Type);
+  if (auto value = findInLibraryPath(*this, CRTBasename)) {
+return value.getValue();

zero9178 wrote:
> rnk wrote:
> > This does twice as many stat syscalls as we need. That seems worth 
> > optimizing. First, in getCompilerRTBasename, we search the library path, we 
> > return a result, and then we search it again here. I think the nicest 
> > solution is probably to have one shared implementation that takes a boolean 
> > and returns the full path or the basename.
> Refactoring the current implementation out of getCompilerRTBasename is 
> currently not possible as that would break the BareMetal toolchain as it 
> overrides getCompilerRTBasename to build up a different scheme for it's 
> runtime libraries. Not calling getCompilerRTBasename is what caused the test 
> failure previously.
> 
> Unless you mean that I should add an optional boolean operand to 
> getCompilerRTBasename that would get either the absolute path if possible or 
> always return a relative path? That could work and would change getCompilerRT 
> to not go through the library path but to instead check if that path is 
> absolute and return if it is already. 
> 
> 
> 
> 
I see, good point. I'd prefer to avoid extra boolean parameters when possible. 
My next best idea is to separate getCompilerRTBasename into two methods, one 
public non-virtual, and one protected virtual (getCompilerRTBasenameImpl? ech) 
to allow baremetal to override. The public one would do the obvious thing of 
returning sys::fs::filename of getCompilerRT. The protected one would retain 
the current implementation, and we'd update the bare metal toolchains to 
override the protected one.

We have prior history of doing excessive amounts of Linux Distro detection 
(D87187), which is why I'm being a bit fussy about the stat calls.


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

https://reviews.llvm.org/D96638

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

[PATCH] D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete.

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

To help bikeshed the name, I can imagine a few other use cases:

- an attribute to suppress type info emission, never emit full type info for 
this type (similar to nodebug / artificial attributes for functions) -- this 
could help optimize debug info size
- an attribute to always emit type information, whether it is required to be 
complete or not (similar to -fno-eliminate-unused-types behavior)
- non-use-case: It's not clear if it's useful to have an attribute to 
explicitly indicate that a type should use the vptr, constructor, or extern 
template heuristics.

I thought maybe we could have a single attribute that takes a mode, something 
like 
`emit_debug_type_info("never/always/when_required_complete/with_ctor/with_vtable")`.
 Or maybe shorter is better: `emit_typeinfo("when_required_complete")`. But 
that sounds like we're talking about RTTI, not debug info.




Comment at: clang/include/clang/Basic/Attr.td:1666
+  let Subjects = SubjectList<[CXXRecord]>;
+  let Documentation = [Undocumented];
+  let SimpleHandler = 1;

I understand that this patch is mainly to open discussion, but we should 
document the attribute before landing the patch.



Comment at: clang/test/CodeGenCXX/force-debug-attribute.cpp:10-11
+
+// Struct that isn't constructed, so its full type info should be omitted with
+// -debug-info-kind=constructor.
+struct DEBUGASNEEDED some_struct {

This attribute should also work with vtable and extern template instantiation 
type homing, right? So for example, I would see this template type in the debug 
info in the standard compilation modes as well:
  template  struct Foo { Foo() {} T x; };
  extern template struct DEBUGASNEEDED Foo; // I expect to see a complete 
Foo DICompositeType


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97411

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

[PATCH] D95561: [Clang] Introduce Swift async calling convention.

[Varun Gandhi via Phabricator via cfe-commits]

varungandhi-apple updated this revision to Diff 326194.
varungandhi-apple added a comment.

Address review feedback; remove centralized target check for CC_SwiftAsync.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95561

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/CodeGen/SwiftCallingConv.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/SystemZ.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/arm-swiftcall.c
  clang/test/CodeGen/debug-info-cc.c
  clang/test/CodeGen/swift-call-conv.c
  clang/test/Sema/attr-c2x.c
  clang/test/Sema/attr-swiftcall.c
  clang/test/Sema/no_callconv.cpp
  clang/test/SemaCXX/attr-swiftcall.cpp
  clang/tools/libclang/CXType.cpp
  llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
  llvm/lib/Demangle/MicrosoftDemangle.cpp
  llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
  llvm/test/Demangle/ms-mangle.test

Index: llvm/test/Demangle/ms-mangle.test
===
--- llvm/test/Demangle/ms-mangle.test
+++ llvm/test/Demangle/ms-mangle.test
@@ -341,6 +341,9 @@
 ?swift_func@@YSXXZ
 ; CHECK: void __attribute__((__swiftcall__)) swift_func(void)
 
+?swift_async_func@@YTXXZ
+; CHECK: void __attribute__((__swiftasynccall__)) swift_async_func(void)
+
 ??$fn_tmpl@$1?extern_c_func@@YAXXZ@@YAXXZ
 ; CHECK: void __cdecl fn_tmpl<&void __cdecl extern_c_func(void)>(void)
 
Index: llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
===
--- llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
+++ llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
@@ -110,6 +110,9 @@
   case CallingConv::Swift:
 OS << "__attribute__((__swiftcall__)) ";
 break;
+  case CallingConv::SwiftAsync:
+OS << "__attribute__((__swiftasynccall__)) ";
+break;
   default:
 break;
   }
Index: llvm/lib/Demangle/MicrosoftDemangle.cpp
===
--- llvm/lib/Demangle/MicrosoftDemangle.cpp
+++ llvm/lib/Demangle/MicrosoftDemangle.cpp
@@ -1713,6 +1713,8 @@
 return CallingConv::Vectorcall;
   case 'S':
 return CallingConv::Swift;
+  case 'T':
+return CallingConv::SwiftAsync;
   }
 
   return CallingConv::None;
Index: llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
===
--- llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
+++ llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
@@ -67,7 +67,8 @@
   Eabi,
   Vectorcall,
   Regcall,
-  Swift, // Clang-only
+  Swift,  // Clang-only
+  SwiftAsync, // Clang-only
 };
 
 enum class ReferenceKind : uint8_t { None, LValueRef, RValueRef };
Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -664,6 +664,7 @@
   TCALLINGCONV(AAPCS_VFP);
   TCALLINGCONV(IntelOclBicc);
   TCALLINGCONV(Swift);
+  TCALLINGCONV(SwiftAsync);
   TCALLINGCONV(PreserveMost);
   TCALLINGCONV(PreserveAll);
 case CC_SpirFunction: return CXCallingConv_Unexposed;
Index: clang/test/SemaCXX/attr-swiftcall.cpp
===
--- clang/test/SemaCXX/attr-swiftcall.cpp
+++ clang/test/SemaCXX/attr-swiftcall.cpp
@@ -1,14 +1,20 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -verify %s
 
 #define SWIFTCALL __attribute__((swiftcall))
+#define SWIFTASYNCCALL __attribute__((swiftasynccall))
 #define INDIRECT_RESULT __attribute__((swift_indirect_result))
 #define ERROR_RESULT __attribute__((swift_error_result))
 #define CONTEXT __attribute__((swift_context))
+#define ASYNC_CONTEXT __attribute__((swift_async_context))
 
 int notAFunction SWIFTCALL; // expected-warning {{'swiftcall' only applies to function types; type here is 'int'}}
+int notAnAsyncFunction SWIFTASYNCCALL; // expected-warning {{'swiftasynccall' only applies to function types; type here is 'int'}}
 void variadic(int x, ...) SWIFTCALL; // expected-error {{variadic function cannot use swiftcall calling convention}}
+void variadic_async(int x, ...) SWIFTASYNCCALL; // expected-error {{variadic function cannot use swiftasynccall calling convention}}
 void multiple_ccs(int x) SWIFTCALL __attribute__((vectorcall)); // expected-error {{vectorcall and swiftcall attributes are not 

[PATCH] D97094: [Driver] Print process statistics report on CC_PRINT_PROC_STAT env variable.

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

Thanks for the update @vvereschaka ! Just a few minor things and it should be 
good to go.
Also please use `git diff -U9` next time to add context to the patch.




Comment at: clang/docs/UsersManual.rst:783
   It is possible to specify this option without any value. In this case 
statistics
   is printed on standard output in human readable format:
   

I think this should read //"In this case statistics **are** printed.."// Would 
you mind changing this as well?



Comment at: clang/docs/UsersManual.rst:799
+  setting the ``CC_PRINT_PROC_STAT`` and ``CC_PRINT_PROC_STAT_FILE`` 
environment
+  variables. Use ``CC_PRINT_PROC_STAT_FILE`` to provide a file name to store
+  the statistics.

Do you think it would be possible to rephrase a bit to avoid the repetition of 
`CC_PRINT_PROC_STAT_FILE`? Perhaps also explicitate that `CC_PRINT_PROC_STAT` 
is for "enabling the feature and logging to stdout"; while 
`CC_PRINT_PROC_STAT_FILE` only "redirects the log to a file".



Comment at: clang/lib/Driver/Driver.cpp:1108
+CCPrintProcessStats = true;
+  Args.ClaimAllArgs(options::OPT_fproc_stat_report);
+

Remove this too, `hasArg` claims all arguments.



Comment at: clang/lib/Driver/Driver.cpp:4039
+
+  if (CCPrintStatReportFilename == nullptr) {
 using namespace llvm;

Suggestion: I find `if (!CCPrintStatReportFilename)` more intutive, more 
compact and easier to read, but that's my personal preference. There are 
arguments both ways probably, up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97094

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

[PATCH] D97411: [DebugInfo] Add an attribute to force type info to be emitted for types that are required to be complete.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Generally OK with this, as it seemed the libc++ issue might be a bit thorny to 
solve. Though I'd like to hear from libc++ maintainers about how they feel to 
ensure they're comfortable adding this attribute in the interim (or if this 
might be a motivation to fix libc++ instead of adding the attribute)

As for the name - I take it the "if required" is meant to connote the fact that 
this attribute only has an effect if the type is used/reachable from the DWARF, 
and doesn't force the type to be emitted even when unused/unreferenced?

Maybe "full_debug_if_used"? or "debug_full_if_used" (or maybe even reuse the 
equivalent compiler flag: standalone_debug?)

In D97411#2585910 , @rnk wrote:

> To help bikeshed the name, I can imagine a few other use cases:
>
> - an attribute to suppress type info emission, never emit full type info for 
> this type (similar to nodebug / artificial attributes for functions) -- this 
> could help optimize debug info size

nodebug is already supported on types for this purpose - we used it in libc++ 
to remove some type trait debug info to trim it down.

> - an attribute to always emit type information, whether it is required to be 
> complete or not (similar to -fno-eliminate-unused-types behavior)
> - non-use-case: It's not clear if it's useful to have an attribute to 
> explicitly indicate that a type should use the vptr, constructor, or extern 
> template heuristics.
>
> I thought maybe we could have a single attribute that takes a mode, something 
> like 
> `emit_debug_type_info("never/always/when_required_complete/with_ctor/with_vtable")`.
>  Or maybe shorter is better: `emit_typeinfo("when_required_complete")`. But 
> that sounds like we're talking about RTTI, not debug info.

Yeah, might be useful to have something that supports the different type homing 
strategies separately, though it is a more complicated user facing feature. The 
"always" mode could be handy, though I hesitate to build more features/surface 
area without users in mind, especially since these sort of things may paper 
over debug info issues we might be better off fixing generally.

I'd like to keep "debug" or "debuginfo" in the name, as you say, to avoid some 
confusion with RTTI for instance.




Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2402-2405
+  // Don't omit definition if marked with attribute.
+  if (RD->hasAttr())
+return false;
+

Perahps this should be a bit further up in this function? For instance the 
template specialization homing seems like it'd override this behavior. Perhaps 
someone has a template specialization that shouldn't be homed for some reason? 
& similarly with modular debug info (both the "isDefinediInClangModule" and 
"hasExternalDefinitions" cases are different kinds of modular debug info homing)

Hmm, I guess the modular debug info is less likely to have these sort of 
problems - it's more explicit about what is homed, both explicitly making a 
home, and explicitly communicating the presence of a home to other compilations 
that rely on that data. So it might be unfortunate to pessimize that scenario 
unnecessarily.

@rnk - any thoughts on the tradeoff of uniformity of the attribute (ie: applies 
to all type homing strategies) V applying the attribute to address shortcomings 
in the source that only affect some homing strategies and not others?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97411

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

[PATCH] D97138: [clang][flang] Improve the consistency of the code-base

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added inline comments.



Comment at: clang/tools/driver/driver.cpp:349
 "source, and associated run script.\n");
-  SmallVector argv(argv_, argv_ + argc_);
+  SmallVector ArgValues(Argv, Argv + Argc);
 

What about just `Args`? "Values" sounds a bit too broad.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97138

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