[PATCH] D107876: [CSSPGO] Allow the use of debug-info-for-profiling and pseudo-probe-for-profiling together

[Wei Mi via Phabricator via cfe-commits]

wmi accepted this revision.
wmi added a comment.

In D107876#2939691 , @hoy wrote:

> In D107876#2939624 , @wmi wrote:
>
>> Could you remind me the discriminator difference between debug info based 
>> AFDO and pseudo probe based AFDO? I forgot it. I am sure it is described in 
>> some patch but I havn't found it. Give me a pointer is good enough.
>
> In short, unlike block pseudo probe, a callsite probe relies on the calliste 
> discriminator to be tracked. This has a couple usage: 1. to track direct call 
> and indirect callsite 2. to model inline contexts with a list of callsite 
> probes.
>
> A probe discriminator is treated as empty or null for AutoFDO since it has 
> lowest three bits as "111".

Thanks for the explanation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107876

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

[PATCH] D104854: Introduce intrinsic llvm.isnan

[Siva Chandra via Phabricator via cfe-commits]

sivachandra added inline comments.



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:22193
+DAG.getConstant(0x45, DL, MVT::i8));
+
+  return DAG.getSetCC(DL, ResultVT, Extract, DAG.getConstant(1, DL, MVT::i8),

While I do not understand the code mechanics of this patch, I am mostly in 
agreement with the general direction of this patch. However, it has lead to a 
change in behavior wrt 80-bit x86 floating point numbers. Unlike the 32-bit and 
64-bit floating point numbers, 80-bit numbers have an additional class of 
"Unsupported Numbers". Those numbers were previously treated as NaNs. Since 
this change uses the `fxam` instruction to classify the input number, that is 
not the case any more as the `fxam` instruction distinguishes between 
unsupported numbers and NaNs. So, to restore the previous behavior, can we 
extend this patch to treat unsupported numbers as NaNs? At a high level, what I 
am effectively saying is that we should implement `isnan` this way:

```
bool isnan(long double x) {
  uint16_t status;
  __asm__ __volatile__("fldt %0" : : "m"(x));
  __asm__ __volatile__("fxam");
  __asm__ __volatile__("fnstsw %0": "=m"(status):);
  uint16_t c0c2c3 = (status >> 8) & 0x45;
  return c0c2c3 <= 1; // This patch seems to be only doing c0c2c3 == 1 check.
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104854

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

[PATCH] D107876: [CSSPGO] Allow the use of debug-info-for-profiling and pseudo-probe-for-profiling together

[Wenlei He via Phabricator via cfe-commits]

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

lgtm, thanks.




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3896
-  // These two forms of profiling info can't be used together.
-  if (const Arg *A1 = 
Args.getLastArg(options::OPT_fpseudo_probe_for_profiling))
-if (const Arg *A2 = 
Args.getLastArg(options::OPT_fdebug_info_for_profiling))

hoy wrote:
> wenlei wrote:
> > Would a warning be useful to catch accidental misuse? 
> I was thinking about that. It may be useful, but downstream may have to 
> suppress that warning to be treated as error for a clean build. What do you 
> think?
Fair point. We could add warning after transition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107876

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

[PATCH] D107944: [hmaptool] Port to python3

[Nathan Lanza via Phabricator via cfe-commits]

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

This is just a few trivial changes -- change the interpreter and fix a
few byte-vs-string issues.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107944

Files:
  clang/utils/hmaptool/hmaptool


Index: clang/utils/hmaptool/hmaptool
===
--- clang/utils/hmaptool/hmaptool
+++ clang/utils/hmaptool/hmaptool
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 from __future__ import absolute_import, division, print_function
 
 import json
@@ -9,8 +9,8 @@
 
 ###
 
-k_header_magic_LE = 'pamh'
-k_header_magic_BE = 'hmap'
+k_header_magic_LE = b'pamh'
+k_header_magic_BE = b'hmap'
 
 def hmap_hash(str):
 """hash(str) -> int
@@ -83,7 +83,7 @@
 if len(strtable) != strtable_size:
 raise SystemExit("error: %s: unable to read complete string 
table"%(
 path,))
-if strtable[-1] != '\0':
+if strtable[-1] != 0:
 raise SystemExit("error: %s: invalid string table in 
headermap" % (
 path,))
 
@@ -98,7 +98,7 @@
 if idx >= len(self.strtable):
 raise SystemExit("error: %s: invalid string index" % (
 path,))
-end_idx = self.strtable.index('\0', idx)
+end_idx = self.strtable.index(0, idx)
 return self.strtable[idx:end_idx]
 
 @property


Index: clang/utils/hmaptool/hmaptool
===
--- clang/utils/hmaptool/hmaptool
+++ clang/utils/hmaptool/hmaptool
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 from __future__ import absolute_import, division, print_function
 
 import json
@@ -9,8 +9,8 @@
 
 ###
 
-k_header_magic_LE = 'pamh'
-k_header_magic_BE = 'hmap'
+k_header_magic_LE = b'pamh'
+k_header_magic_BE = b'hmap'
 
 def hmap_hash(str):
 """hash(str) -> int
@@ -83,7 +83,7 @@
 if len(strtable) != strtable_size:
 raise SystemExit("error: %s: unable to read complete string table"%(
 path,))
-if strtable[-1] != '\0':
+if strtable[-1] != 0:
 raise SystemExit("error: %s: invalid string table in headermap" % (
 path,))
 
@@ -98,7 +98,7 @@
 if idx >= len(self.strtable):
 raise SystemExit("error: %s: invalid string index" % (
 path,))
-end_idx = self.strtable.index('\0', idx)
+end_idx = self.strtable.index(0, idx)
 return self.strtable[idx:end_idx]
 
 @property
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105264: [X86] AVX512FP16 instructions enabling 2/6

[LuoYuanke via Phabricator via cfe-commits]

LuoYuanke added inline comments.



Comment at: llvm/lib/Target/X86/X86InstrFoldTables.cpp:4838
   { X86::VMULSDZrr_Intk,X86::VMULSDZrm_Intk,
TB_NO_REVERSE },
+  { X86::VMULSHZrr_Intk,X86::VMULSHZrm_Intk,
TB_NO_REVERSE },
   { X86::VMULSSZrr_Intk,X86::VMULSSZrm_Intk,
TB_NO_REVERSE },

Is this because intrinsics always assume the arguments are passed in register?



Comment at: llvm/test/CodeGen/X86/avx512fp16-fmaxnum.ll:26
+; CHECK:   # %bb.0:
+; CHECK-NEXT:vmaxph %xmm0, %xmm1, %xmm2 # encoding: 
[0x62,0xf5,0x74,0x08,0x5f,0xd0]
+; CHECK-NEXT:vcmpunordph %xmm0, %xmm0, %k1 # encoding: 
[0x62,0xf3,0x7c,0x08,0xc2,0xc8,0x03]

Is it legal without avx512vl?



Comment at: llvm/test/CodeGen/X86/avx512fp16-fold-load-binops.ll:7
+;  _mm_add_ss(a, _mm_load_ss(b));
+
+define <8 x half> @addsh(<8 x half> %va, half* %pb) {

Any case for max/min?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105264

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

[PATCH] D105264: [X86] AVX512FP16 instructions enabling 2/6

[Pengfei Wang via Phabricator via cfe-commits]

pengfei updated this revision to Diff 365899.
pengfei added a comment.

Update missing changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105264

Files:
  clang/include/clang/Basic/BuiltinsX86.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/avx512fp16intrin.h
  clang/lib/Headers/avx512vlfp16intrin.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/X86/avx512fp16-builtins.c
  clang/test/CodeGen/X86/avx512vlfp16-builtins.c
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/MCTargetDesc/X86ATTInstPrinter.cpp
  llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86InstrAVX512.td
  llvm/lib/Target/X86/X86InstrFoldTables.cpp
  llvm/lib/Target/X86/X86InstrInfo.cpp
  llvm/lib/Target/X86/X86IntrinsicsInfo.h
  llvm/test/CodeGen/X86/avx512fp16-arith-intrinsics.ll
  llvm/test/CodeGen/X86/avx512fp16-arith-vl-intrinsics.ll
  llvm/test/CodeGen/X86/avx512fp16-arith.ll
  llvm/test/CodeGen/X86/avx512fp16-fmaxnum.ll
  llvm/test/CodeGen/X86/avx512fp16-fminnum.ll
  llvm/test/CodeGen/X86/avx512fp16-fold-load-binops.ll
  llvm/test/CodeGen/X86/avx512fp16-fold-xmm-zero.ll
  llvm/test/CodeGen/X86/avx512fp16-fp-logic.ll
  llvm/test/CodeGen/X86/avx512fp16-intrinsics.ll
  llvm/test/CodeGen/X86/avx512fp16-machine-combiner.ll
  llvm/test/CodeGen/X86/avx512fp16-mov.ll
  llvm/test/CodeGen/X86/avx512fp16-unsafe-fp-math.ll
  llvm/test/CodeGen/X86/fp-strict-scalar-cmp-fp16.ll
  llvm/test/CodeGen/X86/fp-strict-scalar-fp16.ll
  llvm/test/CodeGen/X86/pseudo_cmov_lower-fp16.ll
  llvm/test/CodeGen/X86/stack-folding-fp-avx512fp16.ll
  llvm/test/CodeGen/X86/stack-folding-fp-avx512fp16vl.ll
  llvm/test/CodeGen/X86/vec-strict-128-fp16.ll
  llvm/test/CodeGen/X86/vec-strict-256-fp16.ll
  llvm/test/CodeGen/X86/vec-strict-512-fp16.ll
  llvm/test/CodeGen/X86/vec-strict-cmp-128-fp16.ll
  llvm/test/CodeGen/X86/vec-strict-cmp-256-fp16.ll
  llvm/test/CodeGen/X86/vec-strict-cmp-512-fp16.ll
  llvm/test/CodeGen/X86/vector-reduce-fmax-nnan.ll
  llvm/test/CodeGen/X86/vector-reduce-fmin-nnan.ll
  llvm/test/MC/Disassembler/X86/avx512fp16.txt
  llvm/test/MC/Disassembler/X86/avx512fp16vl.txt
  llvm/test/MC/X86/avx512fp16.s
  llvm/test/MC/X86/avx512fp16vl.s
  llvm/test/MC/X86/intel-syntax-avx512fp16.s
  llvm/test/MC/X86/intel-syntax-avx512fp16vl.s

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

[PATCH] D105264: [X86] AVX512FP16 instructions enabling 2/6

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:3197
+  else if (PatchedName.endswith("sh"))
+PatchedName = IsVCMP ? "vcmpsh" : "cmpsh";
+  else if (PatchedName.endswith("ph"))

LuoYuanke wrote:
> There is no cmpsh?
Good catch!



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:1873
   setOperationAction(ISD::SETCC,  VT, Custom);
+  setOperationAction(ISD::STRICT_FSETCC,  VT, Custom);
+  setOperationAction(ISD::STRICT_FSETCCS, VT, Custom);

LuoYuanke wrote:
> Is this related to FP16?
Yes, but it's better to move them together with other FP16 settings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105264

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

[PATCH] D105264: [X86] AVX512FP16 instructions enabling 2/6

[Pengfei Wang via Phabricator via cfe-commits]

pengfei updated this revision to Diff 365895.
pengfei marked 3 inline comments as done.
pengfei added a comment.

Address Yuanke's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105264

Files:
  clang/include/clang/Basic/BuiltinsX86.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/avx512fp16intrin.h
  clang/lib/Headers/avx512vlfp16intrin.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/X86/avx512fp16-builtins.c
  clang/test/CodeGen/X86/avx512vlfp16-builtins.c
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/MCTargetDesc/X86ATTInstPrinter.cpp
  llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86InstrAVX512.td
  llvm/lib/Target/X86/X86InstrFoldTables.cpp
  llvm/lib/Target/X86/X86InstrInfo.cpp
  llvm/lib/Target/X86/X86IntrinsicsInfo.h
  llvm/test/CodeGen/X86/avx512fp16-arith-intrinsics.ll
  llvm/test/CodeGen/X86/avx512fp16-arith-vl-intrinsics.ll
  llvm/test/CodeGen/X86/avx512fp16-arith.ll
  llvm/test/CodeGen/X86/avx512fp16-fmaxnum.ll
  llvm/test/CodeGen/X86/avx512fp16-fminnum.ll
  llvm/test/CodeGen/X86/avx512fp16-fold-load-binops.ll
  llvm/test/CodeGen/X86/avx512fp16-fold-xmm-zero.ll
  llvm/test/CodeGen/X86/avx512fp16-fp-logic.ll
  llvm/test/CodeGen/X86/avx512fp16-intrinsics.ll
  llvm/test/CodeGen/X86/avx512fp16-machine-combiner.ll
  llvm/test/CodeGen/X86/avx512fp16-mov.ll
  llvm/test/CodeGen/X86/avx512fp16-unsafe-fp-math.ll
  llvm/test/CodeGen/X86/fp-strict-scalar-cmp-fp16.ll
  llvm/test/CodeGen/X86/fp-strict-scalar-fp16.ll
  llvm/test/CodeGen/X86/pseudo_cmov_lower-fp16.ll
  llvm/test/CodeGen/X86/stack-folding-fp-avx512fp16.ll
  llvm/test/CodeGen/X86/stack-folding-fp-avx512fp16vl.ll
  llvm/test/CodeGen/X86/vec-strict-128-fp16.ll
  llvm/test/CodeGen/X86/vec-strict-256-fp16.ll
  llvm/test/CodeGen/X86/vec-strict-512-fp16.ll
  llvm/test/CodeGen/X86/vec-strict-cmp-128-fp16.ll
  llvm/test/CodeGen/X86/vec-strict-cmp-256-fp16.ll
  llvm/test/CodeGen/X86/vec-strict-cmp-512-fp16.ll
  llvm/test/CodeGen/X86/vector-reduce-fmax-nnan.ll
  llvm/test/CodeGen/X86/vector-reduce-fmin-nnan.ll
  llvm/test/MC/Disassembler/X86/avx512fp16.txt
  llvm/test/MC/Disassembler/X86/avx512fp16vl.txt
  llvm/test/MC/X86/avx512fp16.s
  llvm/test/MC/X86/avx512fp16vl.s
  llvm/test/MC/X86/intel-syntax-avx512fp16.s
  llvm/test/MC/X86/intel-syntax-avx512fp16vl.s

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

[PATCH] D107882: BPF: Enable frontend constant folding for VLA size

[Yonghong Song via Phabricator via cfe-commits]

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

- fix clant-format warnings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107882

Files:
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/bpf-vla.c
  llvm/include/llvm/ADT/Triple.h

Index: llvm/include/llvm/ADT/Triple.h
===
--- llvm/include/llvm/ADT/Triple.h
+++ llvm/include/llvm/ADT/Triple.h
@@ -738,6 +738,11 @@
: PointerWidth == 64;
   }
 
+  /// Tests whether the target is BPF (little and big endian).
+  bool isBPF() const {
+return getArch() == Triple::bpfel || getArch() == Triple::bpfeb;
+  }
+
   /// Tests whether the target is MIPS 32-bit (little and big endian).
   bool isMIPS32() const {
 return getArch() == Triple::mips || getArch() == Triple::mipsel;
Index: clang/test/CodeGen/bpf-vla.c
===
--- /dev/null
+++ clang/test/CodeGen/bpf-vla.c
@@ -0,0 +1,29 @@
+// REQUIRES: bpf-registered-target
+// RUN: %clang -target bpf -emit-llvm -S -g %s -o - | FileCheck %s
+
+#define AA 40
+struct t {
+  char a[20];
+};
+void foo(void *);
+int test() {
+   const int a = 8;
+   char tmp[AA + sizeof(struct t) + a];
+
+// CHECK: define dso_local i32 @test(
+// CHECK: %tmp = alloca [68 x i8], align 1
+// CHECK-NOT: llvm.stacksave
+
+   foo(tmp);
+   return 0;
+}
+
+int test2(int b) {
+   const int a = 8;
+   char tmp[a + b];
+
+// CHECK: define dso_local i32 @test2(
+// CHECK: llvm.stacksave
+   foo(tmp);
+   return 0;
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -2279,7 +2279,7 @@
 /// ExprError().
 static ExprResult checkArraySize(Sema , Expr *,
  llvm::APSInt , unsigned VLADiag,
- bool VLAIsError) {
+ bool VLAIsError, Sema::AllowFoldKind CanFold) {
   if (S.getLangOpts().CPlusPlus14 &&
   (VLAIsError ||
!ArraySize->getType()->isIntegralOrUnscopedEnumerationType())) {
@@ -2323,8 +2323,8 @@
 }
   } Diagnoser(VLADiag, VLAIsError);
 
-  ExprResult R =
-  S.VerifyIntegerConstantExpression(ArraySize, , Diagnoser);
+  ExprResult R = S.VerifyIntegerConstantExpression(ArraySize, ,
+   Diagnoser, CanFold);
   if (Diagnoser.IsVLA)
 return ExprResult();
   return R;
@@ -2442,10 +2442,20 @@
   // VLAs always produce at least a -Wvla diagnostic, sometimes an error.
   unsigned VLADiag;
   bool VLAIsError;
+  AllowFoldKind CanFold = Sema::NoFold;
   if (getLangOpts().OpenCL) {
 // OpenCL v1.2 s6.9.d: variable length arrays are not supported.
 VLADiag = diag::err_opencl_vla;
 VLAIsError = true;
+  } else if (Context.getTargetInfo().getTriple().isBPF()) {
+// BPF target does not support variable length array, but
+// we don't force an error here. We will try to do constant
+// folding for array size as much as possible. LLVM backend
+// will do some checking and warn users if variable length
+// array still remains.
+VLADiag = diag::warn_vla_used;
+VLAIsError = false;
+CanFold = Sema::AllowFold;
   } else if (getLangOpts().C99) {
 VLADiag = diag::warn_vla_used;
 VLAIsError = false;
@@ -2471,8 +2481,8 @@
   } else if (ArraySize->isTypeDependent() || ArraySize->isValueDependent()) {
 T = Context.getDependentSizedArrayType(T, ArraySize, ASM, Quals, Brackets);
   } else {
-ExprResult R =
-checkArraySize(*this, ArraySize, ConstVal, VLADiag, VLAIsError);
+ExprResult R = checkArraySize(*this, ArraySize, ConstVal, VLADiag,
+  VLAIsError, CanFold);
 if (R.isInvalid())
   return QualType();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

[Florin Iucha via Phabricator via cfe-commits]

0x8000- updated this revision to Diff 365891.
0x8000- added a comment.

One more format nit.


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

https://reviews.llvm.org/D97753

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-length.rst
  clang-tools-extra/test/clang-tidy/checkers/readability-identifier-length.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-length.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-length.cpp
@@ -0,0 +1,63 @@
+// RUN: %check_clang_tidy %s readability-identifier-length %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: readability-identifier-length.IgnoredVariableNames, value: "^[xy]$"}]}' \
+// RUN: --
+
+struct myexcept {
+  int val;
+};
+
+struct simpleexcept {
+  int other;
+};
+
+void doIt();
+
+void tooShortVariableNames(int z)
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: parameter name 'z' is too short, expected at least 3 characters [readability-identifier-length]
+{
+  int i = 5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable name 'i' is too short, expected at least 3 characters [readability-identifier-length]
+
+  int jj = z;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable name 'jj' is too short, expected at least 3 characters [readability-identifier-length]
+
+  for (int m = 0; m < 5; ++m)
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: loop variable name 'm' is too short, expected at least 2 characters [readability-identifier-length]
+  {
+doIt();
+  }
+
+  try {
+doIt();
+  } catch (const myexcept )
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: exception variable name 'x' is too short, expected at least 2 characters [readability-identifier-length]
+  {
+doIt();
+  }
+}
+
+void longEnoughVariableNames(int n) // argument 'n' ignored by default configuration
+{
+  int var = 5;
+
+  for (int i = 0; i < 42; ++i) // 'i' is default allowed, for historical reasons
+  {
+doIt();
+  }
+
+  for (int kk = 0; kk < 42; ++kk) {
+doIt();
+  }
+
+  try {
+doIt();
+  } catch (const simpleexcept ) // ignored by default configuration
+  {
+doIt();
+  } catch (const myexcept ) {
+doIt();
+  }
+
+  int x = 5; // ignored by configuration
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-length.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability-identifier-length.rst
@@ -0,0 +1,122 @@
+.. title:: clang-tidy - readability-identifier-length
+
+readability-identifier-length
+=
+
+This check finds variables and function parameters whose length are too short.
+The desired name length is configurable.
+
+Special cases are supported for loop counters and for exception variable names.
+
+Options
+---
+
+The following options are described below:
+
+ - :option:`MinimumVariableNameLength`, :option:`IgnoredVariableNames`
+ - :option:`MinimumParameterNameLength`, :option:`IgnoredParameterNames`
+ - :option:`MinimumLoopCounterNameLength`, :option:`IgnoredLoopCounterNames`
+ - :option:`MinimumExceptionNameLength`, :option:`IgnoredExceptionVariableNames`
+
+.. option:: MinimumVariableNameLength
+
+All variables (other than loop counter, exception names and function
+parameters) are expected to have at least a length of
+`MinimumVariableNameLength` (default is `3`). Setting it to `0` or `1`
+disables the check entirely.
+
+
+.. code-block:: c++
+
+ int doubler(int x)   // warns that x is too short
+ {
+return 2 * x;
+ }
+
+This check does not have any fix suggestions in the general case since
+variable names have semantic value.
+
+.. option:: IgnoredVariableNames
+
+Specifies a regular expression for variable names that are
+to be ignored. The default value is empty, thus no names are ignored.
+
+.. option:: MinimumParameterNameLength
+
+All function parameter names are expected to have a length of at least
+`MinimumParameterNameLength` (default is `3`). Setting it to `0` or `1`
+disables the check entirely.
+
+
+.. code-block:: c++
+
+  int i = 42;// warns that 'i' is too short
+
+This check does not have any fix suggestions in the general case since
+variable names have semantic value.
+
+.. option:: IgnoredParameterNames
+
+Specifies a regular expression for parameters that are to be ignored.
+The default value is `^[n]$` for historical reasons.
+
+.. 

[PATCH] D107939: [clang][Arm

[Sarah Purohit via Phabricator via cfe-commits]

sarahpurohit created this revision.
sarahpurohit added a project: clang.
Herald added a subscriber: kristof.beyls.
sarahpurohit requested review of this revision.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107939

Files:
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  clang/test/Driver/windows-macho.c


Index: clang/test/Driver/windows-macho.c
===
--- /dev/null
+++ clang/test/Driver/windows-macho.c
@@ -0,0 +1,11 @@
+// RUN: %clang -target armv7-pc-win32-macho -msoft-float -### -c %s 2>&1 \
+// RUN: | FileCheck %s --check-prefix CHECK-SOFTFLOAT
+// CHECK-SOFTFLOAT-NOT: error: unsupported option '-msoft-float' for target 
'thumbv7-pc-windows-macho'
+
+// RUN: %clang -target armv7-pc-win32-macho -mhard-float -### -c %s 2>&1 \
+// RUN: | FileCheck %s --check-prefix CHECK-HARDFLOAT
+// CHECK-HARDFLOAT: error: unsupported option '-mhard-float' for target 
'thumbv7-pc-windows-macho'
+
+// RUN: %clang -target armv7-pc-win32-macho -### -c %s 2>&1 \
+// RUN: | FileCheck %s --check-prefix CHECK-DEFAULT-SOFTFLOAT-ABI
+// CHECK-DEFAULT-SOFTFLOAT-ABI: "-mfloat-abi" "soft"
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -314,6 +314,11 @@
 
   // FIXME: this is invalid for WindowsCE
   case llvm::Triple::Win32:
+// It is incorrect to select hard float ABI on MachO platforms if the ABI 
is
+// "apcs-gnu".
+if (Triple.isOSBinFormatMachO() && !useAAPCSForMachO(Triple)) {
+  return FloatABI::Soft;
+}
 return FloatABI::Hard;
 
   case llvm::Triple::NetBSD:


Index: clang/test/Driver/windows-macho.c
===
--- /dev/null
+++ clang/test/Driver/windows-macho.c
@@ -0,0 +1,11 @@
+// RUN: %clang -target armv7-pc-win32-macho -msoft-float -### -c %s 2>&1 \
+// RUN: | FileCheck %s --check-prefix CHECK-SOFTFLOAT
+// CHECK-SOFTFLOAT-NOT: error: unsupported option '-msoft-float' for target 'thumbv7-pc-windows-macho'
+
+// RUN: %clang -target armv7-pc-win32-macho -mhard-float -### -c %s 2>&1 \
+// RUN: | FileCheck %s --check-prefix CHECK-HARDFLOAT
+// CHECK-HARDFLOAT: error: unsupported option '-mhard-float' for target 'thumbv7-pc-windows-macho'
+
+// RUN: %clang -target armv7-pc-win32-macho -### -c %s 2>&1 \
+// RUN: | FileCheck %s --check-prefix CHECK-DEFAULT-SOFTFLOAT-ABI
+// CHECK-DEFAULT-SOFTFLOAT-ABI: "-mfloat-abi" "soft"
Index: clang/lib/Driver/ToolChains/Arch/ARM.cpp
===
--- clang/lib/Driver/ToolChains/Arch/ARM.cpp
+++ clang/lib/Driver/ToolChains/Arch/ARM.cpp
@@ -314,6 +314,11 @@
 
   // FIXME: this is invalid for WindowsCE
   case llvm::Triple::Win32:
+// It is incorrect to select hard float ABI on MachO platforms if the ABI is
+// "apcs-gnu".
+if (Triple.isOSBinFormatMachO() && !useAAPCSForMachO(Triple)) {
+  return FloatABI::Soft;
+}
 return FloatABI::Hard;
 
   case llvm::Triple::NetBSD:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107095: Implement #pragma clang header_unsafe

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a reviewer: lebedev.ri.
beanz added a subscriber: lebedev.ri.
beanz added a comment.

+@lebedev.ri

@aaron.ballman thank you for all the feedback and support!

I'm not really sure where to go on the naming. I'm not attached to 
`header_unsafe`, and totally understand the confusion. I don't really love the 
`reserved*` wording either as it doesn't really convey meaning.

For my other patch I like `final` because it means something similar to what 
the `final` keyword means in C++ and other languages (i.e. don't change this 
again).

For this I also thought about variations on `restrict` to signify restrictions 
on the macro's expansion contexts.

The goal is to warn on expansions that are outside the main source file, so 
maybe something like `restrict_header_expansion`?

Very open to feedback here on making clear naming. After all naming is one of 
the hardest problems in computer science :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107095

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

[PATCH] D106614: [Clang] add btf_tag attribute

[Yonghong Song via Phabricator via cfe-commits]

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

- fix a few nits in AttrDocs.td


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106614

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/attr-btf_tag.c

Index: clang/test/Sema/attr-btf_tag.c
===
--- /dev/null
+++ clang/test/Sema/attr-btf_tag.c
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -x c -triple x86_64-pc-linux-gnu -dwarf-version=4 -fsyntax-only -verify %s
+
+#define __tag1 __attribute__((btf_tag("tag1")))
+#define __tag2 __attribute__((btf_tag("tag2")))
+#define __tag3 __attribute__((btf_tag("tag3")))
+
+#define __tag_no_arg __attribute__((btf_tag()))
+#define __tag_2_arg __attribute__((btf_tag("tag1", "tag2")))
+#define __invalid __attribute__((btf_tag(1)))
+
+struct __tag1 __tag2 t1;
+struct t1 {
+  int a __tag1;
+} __tag3;
+
+struct __tag1 t2;
+struct __tag2 __tag3 t2 {
+  int a __tag1;
+};
+
+int g1 __tag1;
+int g2 __tag_no_arg; // expected-error {{'btf_tag' attribute takes one argument}}
+int g3 __tag_2_arg; // expected-error {{'btf_tag' attribute takes one argument}}
+int i1 __invalid; // expected-error {{'btf_tag' attribute requires a string}}
+
+enum e1 {
+  E1
+} __tag1; // expected-error {{'btf_tag' attribute only applies to variables, functions, structs, unions, classes, and non-static data members}}
+
+enum e2 {
+  E2
+} __tag_no_arg; // expected-error {{'btf_tag' attribute only applies to variables, functions, structs, unions, classes, and non-static data members}}
+
+enum e3 {
+  E3
+} __tag_2_arg; // expected-error {{'btf_tag' attribute only applies to variables, functions, structs, unions, classes, and non-static data members}}
+
+int __tag1 __tag2 foo(struct t1 *arg, struct t2 *arg2);
+int __tag2 __tag3 foo(struct t1 *arg, struct t2 *arg2);
+int __tag1 foo(struct t1 *arg __tag1, struct t2 *arg2) {
+  return arg->a + arg2->a;
+}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -22,6 +22,7 @@
 // CHECK-NEXT: Assumption (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: Availability ((SubjectMatchRule_record, SubjectMatchRule_enum, SubjectMatchRule_enum_constant, SubjectMatchRule_field, SubjectMatchRule_function, SubjectMatchRule_namespace, SubjectMatchRule_objc_category, SubjectMatchRule_objc_implementation, SubjectMatchRule_objc_interface, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_objc_protocol, SubjectMatchRule_record, SubjectMatchRule_type_alias, SubjectMatchRule_variable))
 // CHECK-NEXT: BPFPreserveAccessIndex (SubjectMatchRule_record)
+// CHECK-NEXT: BTFTag (SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_record, SubjectMatchRule_field)
 // CHECK-NEXT: BuiltinAlias (SubjectMatchRule_function)
 // CHECK-NEXT: CFAuditedTransfer (SubjectMatchRule_function)
 // CHECK-NEXT: CFConsumed (SubjectMatchRule_variable_is_parameter)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6842,6 +6842,30 @@
   Rec->addAttr(::new (S.Context) BPFPreserveAccessIndexAttr(S.Context, AL));
 }
 
+static bool hasBTFTagAttr(Decl *D, StringRef Tag) {
+  for (const auto *I : D->specific_attrs()) {
+if (I->getBTFTag() == Tag)
+  return true;
+  }
+  return false;
+}
+
+static void handleBTFTagAttr(Sema , Decl *D, const ParsedAttr ) {
+  StringRef Str;
+  if (!S.checkStringLiteralArgumentAttr(AL, 0, Str))
+return;
+  if (hasBTFTagAttr(D, Str))
+return;
+
+  D->addAttr(::new (S.Context) BTFTagAttr(S.Context, AL, Str));
+}
+
+BTFTagAttr *Sema::mergeBTFTagAttr(Decl *D, const BTFTagAttr ) {
+  if (hasBTFTagAttr(D, AL.getBTFTag()))
+return nullptr;
+  return ::new (Context) BTFTagAttr(Context, AL, AL.getBTFTag());
+}
+
 static void handleWebAssemblyExportNameAttr(Sema , Decl *D, const ParsedAttr ) {
   if (!isFunctionOrMethod(D)) {
 S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
@@ -7879,6 +7903,9 @@
   case ParsedAttr::AT_BPFPreserveAccessIndex:
 handleBPFPreserveAccessIndexAttr(S, D, AL);
 break;
+  case ParsedAttr::AT_BTFTag:
+handleBTFTagAttr(S, D, AL);
+break;
   case ParsedAttr::AT_WebAssemblyExportName:
 handleWebAssemblyExportNameAttr(S, D, AL);
 break;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- 

[PATCH] D106614: [Clang] add btf_tag attribute

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

> Assuming I have that correct now, this approach looks correct to me (without 
> diagnosing the ignored duplicates).

I agree that we don't need to diagnose the ignored duplicates. The following is 
an example,

$ cat t.c
int g __attribute__((section("a"))) __attribute__((section("a")));
$ clang -g -c t.c -Wall -Werror
$

duplicates are silently ignored and there is no warning.

A warning/error will be issued if two section names are different.
$ cat t.c
int g __attribute__((section("a"))) __attribute__((section("b")));
$ clang -g -c t.c -Wall -Werror
t.c:1:22: error: section does not match previous declaration [-Werror,-Wsection]
int g __attribute__((section("a"))) __attribute__((section("b")));

  ^

t.c:1:52: note: previous attribute is here
int g __attribute__((section("a"))) __attribute__((section("b")));

  ^

1 error generated.
$

So for btf_tag attributes, since we just ignore duplicated attributes, we 
should be fine without emitting any warn/error messages.

> Assuming we're back on the same page again, I think the only thing left in 
> this review is a commenting nit.

There are a couple of nits in AttrDocs.td. Will fix them and resubmit the patch.
Thanks for your careful review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106614

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

[PATCH] D107843: [X86] Add parentheses around casts in some of the X86 intrinsic headers.

[Craig Topper via Phabricator via cfe-commits]

craig.topper updated this revision to Diff 365880.
craig.topper added a comment.

Fix two functions I missed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107843

Files:
  clang/lib/Headers/__wmmintrin_aes.h
  clang/lib/Headers/avx2intrin.h
  clang/lib/Headers/avxintrin.h
  clang/lib/Headers/emmintrin.h
  clang/lib/Headers/smmintrin.h
  clang/lib/Headers/tmmintrin.h
  clang/lib/Headers/xmmintrin.h

Index: clang/lib/Headers/xmmintrin.h
===
--- clang/lib/Headers/xmmintrin.h
+++ clang/lib/Headers/xmmintrin.h
@@ -2181,7 +2181,7 @@
 ///3: Bits [63:48] are copied to the destination.
 /// \returns A 16-bit integer containing the extracted 16 bits of packed data.
 #define _mm_extract_pi16(a, n) \
-  (int)__builtin_ia32_vec_ext_v4hi((__v4hi)a, (int)n)
+  ((int)__builtin_ia32_vec_ext_v4hi((__v4hi)a, (int)n))
 
 /// Copies data from the 64-bit vector of [4 x i16] to the destination,
 ///and inserts the lower 16-bits of an integer operand at the 16-bit offset
@@ -2212,7 +2212,7 @@
 /// \returns A 64-bit integer vector containing the copied packed data from the
 ///operands.
 #define _mm_insert_pi16(a, d, n) \
-  (__m64)__builtin_ia32_vec_set_v4hi((__v4hi)a, (int)d, (int)n)
+  ((__m64)__builtin_ia32_vec_set_v4hi((__v4hi)a, (int)d, (int)n))
 
 /// Compares each of the corresponding packed 16-bit integer values of
 ///the 64-bit integer vectors, and writes the greater value to the
@@ -2359,7 +2359,7 @@
 ///11: assigned from bits [63:48] of \a a.
 /// \returns A 64-bit integer vector containing the shuffled values.
 #define _mm_shuffle_pi16(a, n) \
-  (__m64)__builtin_ia32_pshufw((__v4hi)(__m64)(a), (n))
+  ((__m64)__builtin_ia32_pshufw((__v4hi)(__m64)(a), (n)))
 
 /// Conditionally copies the values from each 8-bit element in the first
 ///64-bit integer vector operand to the specified memory location, as
@@ -2601,8 +2601,8 @@
 ///11: Bits [127:96] copied from the specified operand.
 /// \returns A 128-bit vector of [4 x float] containing the shuffled values.
 #define _mm_shuffle_ps(a, b, mask) \
-  (__m128)__builtin_ia32_shufps((__v4sf)(__m128)(a), (__v4sf)(__m128)(b), \
-(int)(mask))
+  ((__m128)__builtin_ia32_shufps((__v4sf)(__m128)(a), (__v4sf)(__m128)(b), \
+ (int)(mask)))
 
 /// Unpacks the high-order (index 2,3) values from two 128-bit vectors of
 ///[4 x float] and interleaves them into a 128-bit vector of [4 x float].
Index: clang/lib/Headers/tmmintrin.h
===
--- clang/lib/Headers/tmmintrin.h
+++ clang/lib/Headers/tmmintrin.h
@@ -145,8 +145,8 @@
 /// \returns A 128-bit integer vector containing the concatenated right-shifted
 ///value.
 #define _mm_alignr_epi8(a, b, n) \
-  (__m128i)__builtin_ia32_palignr128((__v16qi)(__m128i)(a), \
- (__v16qi)(__m128i)(b), (n))
+  ((__m128i)__builtin_ia32_palignr128((__v16qi)(__m128i)(a), \
+  (__v16qi)(__m128i)(b), (n)))
 
 /// Concatenates the two 64-bit integer vector operands, and right-shifts
 ///the result by the number of bytes specified in the immediate operand.
@@ -168,7 +168,7 @@
 /// \returns A 64-bit integer vector containing the concatenated right-shifted
 ///value.
 #define _mm_alignr_pi8(a, b, n) \
-  (__m64)__builtin_ia32_palignr((__v8qi)(__m64)(a), (__v8qi)(__m64)(b), (n))
+  ((__m64)__builtin_ia32_palignr((__v8qi)(__m64)(a), (__v8qi)(__m64)(b), (n)))
 
 /// Horizontally adds the adjacent pairs of values contained in 2 packed
 ///128-bit vectors of [8 x i16].
Index: clang/lib/Headers/smmintrin.h
===
--- clang/lib/Headers/smmintrin.h
+++ clang/lib/Headers/smmintrin.h
@@ -231,7 +231,7 @@
 ///  11: Truncated
 /// \returns A 128-bit vector of [4 x float] containing the rounded values.
 #define _mm_round_ps(X, M) \
-  (__m128)__builtin_ia32_roundps((__v4sf)(__m128)(X), (M))
+  ((__m128)__builtin_ia32_roundps((__v4sf)(__m128)(X), (M)))
 
 /// Copies three upper elements of the first 128-bit vector operand to
 ///the corresponding three upper elements of the 128-bit result vector of
@@ -272,8 +272,8 @@
 /// \returns A 128-bit vector of [4 x float] containing the copied and rounded
 ///values.
 #define _mm_round_ss(X, Y, M) \
-  (__m128)__builtin_ia32_roundss((__v4sf)(__m128)(X), \
- (__v4sf)(__m128)(Y), (M))
+  ((__m128)__builtin_ia32_roundss((__v4sf)(__m128)(X), \
+  (__v4sf)(__m128)(Y), (M)))
 
 /// Rounds each element of the 128-bit vector of [2 x double] to an
 ///integer value according to the rounding control specified by the second
@@ -306,7 +306,7 @@
 ///  11: Truncated
 /// \returns A 128-bit vector of [2 x double] 

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

[Florin Iucha via Phabricator via cfe-commits]

0x8000- updated this revision to Diff 365879.
0x8000- added a comment.

Reformatted the test file.


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

https://reviews.llvm.org/D97753

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-length.rst
  clang-tools-extra/test/clang-tidy/checkers/readability-identifier-length.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-length.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-length.cpp
@@ -0,0 +1,63 @@
+// RUN: %check_clang_tidy %s readability-identifier-length %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: readability-identifier-length.IgnoredVariableNames, value: "^[xy]$"}]}' \
+// RUN: --
+
+struct myexcept {
+  int val;
+};
+
+struct simpleexcept {
+  int other;
+};
+
+void doIt();
+
+void tooShortVariableNames(int z)
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: parameter name 'z' is too short, expected at least 3 characters [readability-identifier-length]
+{
+  int i = 5;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable name 'i' is too short, expected at least 3 characters [readability-identifier-length]
+
+  int jj = z;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable name 'jj' is too short, expected at least 3 characters [readability-identifier-length]
+
+  for (int m = 0; m < 5; ++m)
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: loop variable name 'm' is too short, expected at least 2 characters [readability-identifier-length]
+  {
+doIt();
+  }
+
+  try {
+doIt();
+  } catch (const myexcept )
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: exception variable name 'x' is too short, expected at least 2 characters [readability-identifier-length]
+  {
+doIt();
+  }
+}
+
+void longEnoughVariableNames(int n) // argument 'n' ignored by default configuration
+{
+  int var = 5;
+
+  for (int i = 0; i < 42; ++i) // 'i' is default allowed, for historical reasons
+  {
+doIt();
+  }
+
+  for (int kk = 0; kk < 42; ++kk) {
+doIt();
+  }
+
+  try {
+doIt();
+  } catch (const simpleexcept ) // ignored by default configuration
+  {
+doIt();
+  } catch (const myexcept ) {
+doIt();
+  }
+
+  int x = 5; // ignored by configuration
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-length.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability-identifier-length.rst
@@ -0,0 +1,122 @@
+.. title:: clang-tidy - readability-identifier-length
+
+readability-identifier-length
+=
+
+This check finds variables and function parameters whose length are too short.
+The desired name length is configurable.
+
+Special cases are supported for loop counters and for exception variable names.
+
+Options
+---
+
+The following options are described below:
+
+ - :option:`MinimumVariableNameLength`, :option:`IgnoredVariableNames`
+ - :option:`MinimumParameterNameLength`, :option:`IgnoredParameterNames`
+ - :option:`MinimumLoopCounterNameLength`, :option:`IgnoredLoopCounterNames`
+ - :option:`MinimumExceptionNameLength`, :option:`IgnoredExceptionVariableNames`
+
+.. option:: MinimumVariableNameLength
+
+All variables (other than loop counter, exception names and function
+parameters) are expected to have at least a length of
+`MinimumVariableNameLength` (default is `3`). Setting it to `0` or `1`
+disables the check entirely.
+
+
+.. code-block:: c++
+
+ int doubler(int x)   // warns that x is too short
+ {
+return 2 * x;
+ }
+
+This check does not have any fix suggestions in the general case since
+variable names have semantic value.
+
+.. option:: IgnoredVariableNames
+
+Specifies a regular expression for variable names that are
+to be ignored. The default value is empty, thus no names are ignored.
+
+.. option:: MinimumParameterNameLength
+
+All function parameter names are expected to have a length of at least
+`MinimumParameterNameLength` (default is `3`). Setting it to `0` or `1`
+disables the check entirely.
+
+
+.. code-block:: c++
+
+  int i = 42;// warns that 'i' is too short
+
+This check does not have any fix suggestions in the general case since
+variable names have semantic value.
+
+.. option:: IgnoredParameterNames
+
+Specifies a regular expression for parameters that are to be ignored.
+The default value is `^[n]$` for historical reasons.
+

[PATCH] D107933: [clang] Expose unreachable fallthrough annotation warning

[Nathan Chancellor via Phabricator via cfe-commits]

nathanchance created this revision.
nathanchance added reviewers: dblaikie, rsmith, nickdesaulniers.
nathanchance requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The Linux kernel has a macro called IS_ENABLED(), which evaluates to a
constant 1 or 0 based on Kconfig selections, allowing C code to be
unconditionally enabled or disabled at build time. For example:

int foo(struct *a, int b) {

  switch (b) {
  case 1:
  if (a->flag || !IS_ENABLED(CONFIG_64BIT))
  return 1;
  __attribute__((fallthrough));
  case 2:
  return 2;
  default:
  return 3;
  }

}

There is an unreachable warning about the fallthrough annotation in the
first case because !IS_ENABLED(CONFIG_64BIT) can be evaluated to 1,
which looks like

  return 1;
  __attribute__((fallthrough));

to clang.

This type of warning is pointless for the Linux kernel because it does
this trick all over the place due to the sheer number of configuration
options that it has.

Add -Wimplicit-fallthrough-unreachable, which is default enabled with
-Wimplicit-fallthrough to keep the status quo but allows projects to opt
out of the warning when they know these annotations may be unreachable
in certain conditions.

Fixes PR51094.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107933

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/SemaCXX/switch-implicit-fallthrough.cpp


Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -193,6 +193,28 @@
 ;
   }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wimplicit-fallthrough-unreachable"
+  switch (n) {
+  n += 300;
+  [[clang::fallthrough]];  // no warning here
+case 221:
+  return 1;
+  [[clang::fallthrough]];  // no warning here
+case 222:
+  return 2;
+  __attribute__((fallthrough)); // no warning here
+case 223:
+  if (1)
+return 3;
+  __attribute__((fallthrough)); // no warning here
+case 224:
+  n += 400;
+case 225: // expected-warning{{unannotated fall-through between switch 
labels}} expected-note{{insert '[[clang::fallthrough]];' to silence this 
warning}} expected-note{{insert 'break;' to avoid fall-through}}
+;
+  }
+#pragma clang diagnostic pop
+
   long p = static_cast(n) * n;
   switch (sizeof(p)) {
 case 9:
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9580,7 +9580,7 @@
   "fallthrough annotation does not directly precede switch label">;
 def warn_fallthrough_attr_unreachable : Warning<
   "fallthrough annotation in unreachable code">,
-  InGroup, DefaultIgnore;
+  InGroup, DefaultIgnore;
 
 def warn_unreachable_default : Warning<
   "default label in switch which covers all enumeration values">,
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -671,8 +671,11 @@
 def Switch : DiagGroup<"switch">;
 def ImplicitFallthroughPerFunction :
   DiagGroup<"implicit-fallthrough-per-function">;
+def ImplicitFallthroughUnreachable :
+  DiagGroup<"implicit-fallthrough-unreachable">;
 def ImplicitFallthrough  : DiagGroup<"implicit-fallthrough",
- [ImplicitFallthroughPerFunction]>;
+ [ImplicitFallthroughPerFunction,
+  ImplicitFallthroughUnreachable]>;
 def InvalidPPToken : DiagGroup<"invalid-pp-token">;
 def Trigraphs  : DiagGroup<"trigraphs">;
 


Index: clang/test/SemaCXX/switch-implicit-fallthrough.cpp
===
--- clang/test/SemaCXX/switch-implicit-fallthrough.cpp
+++ clang/test/SemaCXX/switch-implicit-fallthrough.cpp
@@ -193,6 +193,28 @@
 ;
   }
 
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wimplicit-fallthrough-unreachable"
+  switch (n) {
+  n += 300;
+  [[clang::fallthrough]];  // no warning here
+case 221:
+  return 1;
+  [[clang::fallthrough]];  // no warning here
+case 222:
+  return 2;
+  __attribute__((fallthrough)); // no warning here
+case 223:
+  if (1)
+return 3;
+  __attribute__((fallthrough)); // no warning here
+case 224:
+  n += 400;
+case 225: // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} expected-note{{insert 'break;' 

[PATCH] D71734: [Modules] Handle tag types and complain about bad merges in C/Objective-C mode

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Found another case that doesn't emit an error

  #if defined(FIRST)
  struct Indirect {
int x;
  };
  struct Direct {
struct Indirect i;
  };
  #elif defined(SECOND)
  struct Indirect {
double a;
  };
  struct Direct {
struct Indirect i;
  };
  #else
  struct Direct d;
  #endif

According to my debugging there is no error because `Direct` fields aren't 
deserialized in -fsyntax-only mode and therefore `Indirect` definitions aren't 
compared. But during IRGen there is diagnostic and that's because calculating 
record layout triggers full deserialization. Also there is diagnostic in C++ 
because we are dealing with default initialization and 
`DeclareImplicitDefaultConstructor` iterates through all the fields 
deserializing them.

I believe the best user experience is consistent diagnostic, so we should emit 
the error even with -fsyntax-only. If anybody has any objections, please let me 
know, it would save time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71734

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

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

[Florin Iucha via Phabricator via cfe-commits]

0x8000- added a comment.

@aaron.ballman - thank you for the review; please submit on my behalf.


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

https://reviews.llvm.org/D97753

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

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

[Florin Iucha via Phabricator via cfe-commits]

0x8000- updated this revision to Diff 365852.
0x8000- added a comment.

Removed extra 'const' and parentheses that are not congruent with the accepted 
code style.


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

https://reviews.llvm.org/D97753

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.h
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-length.rst
  clang-tools-extra/test/clang-tidy/checkers/readability-identifier-length.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-length.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-length.cpp
@@ -0,0 +1,72 @@
+// RUN: %check_clang_tidy %s readability-identifier-length %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: readability-identifier-length.IgnoredVariableNames, value: "^[xy]$"}]}' \
+// RUN: --
+
+struct myexcept
+{
+   int val;
+};
+
+struct simpleexcept
+{
+   int other;
+};
+
+void doIt();
+
+void tooShortVariableNames(int z)
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: parameter name 'z' is too short, expected at least 3 characters [readability-identifier-length]
+{
+   int i = 5;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: variable name 'i' is too short, expected at least 3 characters [readability-identifier-length]
+
+   int jj = z;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: variable name 'jj' is too short, expected at least 3 characters [readability-identifier-length]
+
+   for (int m = 0; m < 5; ++ m)
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: loop variable name 'm' is too short, expected at least 2 characters [readability-identifier-length]
+   {
+  doIt();
+   }
+
+   try
+   {
+  doIt();
+   }
+   catch (const myexcept& x)
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: exception variable name 'x' is too short, expected at least 2 characters [readability-identifier-length]
+   {
+  doIt();
+   }
+}
+
+void longEnoughVariableNames(int n) // argument 'n' ignored by default configuration
+{
+   int var = 5;
+
+   for (int i = 0; i < 42; ++ i) // 'i' is default allowed, for historical reasons
+   {
+  doIt();
+   }
+
+   for (int kk = 0; kk < 42; ++ kk)
+   {
+  doIt();
+   }
+
+   try
+   {
+  doIt();
+   }
+   catch (const simpleexcept& e) // ignored by default configuration
+   {
+  doIt();
+   }
+   catch (const myexcept& ex)
+   {
+  doIt();
+   }
+
+   int x = 5;  // ignored by configuration
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-length.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability-identifier-length.rst
@@ -0,0 +1,122 @@
+.. title:: clang-tidy - readability-identifier-length
+
+readability-identifier-length
+=
+
+This check finds variables and function parameters whose length are too short.
+The desired name length is configurable.
+
+Special cases are supported for loop counters and for exception variable names.
+
+Options
+---
+
+The following options are described below:
+
+ - :option:`MinimumVariableNameLength`, :option:`IgnoredVariableNames`
+ - :option:`MinimumParameterNameLength`, :option:`IgnoredParameterNames`
+ - :option:`MinimumLoopCounterNameLength`, :option:`IgnoredLoopCounterNames`
+ - :option:`MinimumExceptionNameLength`, :option:`IgnoredExceptionVariableNames`
+
+.. option:: MinimumVariableNameLength
+
+All variables (other than loop counter, exception names and function
+parameters) are expected to have at least a length of
+`MinimumVariableNameLength` (default is `3`). Setting it to `0` or `1`
+disables the check entirely.
+
+
+.. code-block:: c++
+
+ int doubler(int x)   // warns that x is too short
+ {
+return 2 * x;
+ }
+
+This check does not have any fix suggestions in the general case since
+variable names have semantic value.
+
+.. option:: IgnoredVariableNames
+
+Specifies a regular expression for variable names that are
+to be ignored. The default value is empty, thus no names are ignored.
+
+.. option:: MinimumParameterNameLength
+
+All function parameter names are expected to have a length of at least
+`MinimumParameterNameLength` (default is `3`). Setting it to `0` or `1`
+disables the check entirely.
+
+
+.. code-block:: c++
+
+  int i = 42;// warns that 'i' is too short
+
+This check does not have any fix suggestions in the general case since
+variable names have semantic value.
+
+.. option:: IgnoredParameterNames
+

[PATCH] D107540: [OMPIRBuilder] Clarify CanonicalLoopInfo. NFC.

[Alex Zinenko via Phabricator via cfe-commits]

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

Thanks!




Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:364
+  /// over all chunks that are executed on the same thread. Returning
+  /// CanonicalLoopInfo objects representing then may eventually be useful for
+  /// the apply clause planned in OpenMP 6.0, but currently whether these are





Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:393
+  /// \param DL   Debug location for instructions added for the
+  ///workshare-loop construct itself.
   /// \param CLI  A descriptor of the canonical loop to workshare.





Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:420
+  /// \param DL   Debug location for instructions added for the
+  ///workshare-loop construct itself.
   /// \param CLI  A descriptor of the canonical loop to workshare.





Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1287
+/// The loop is thought to start at PreheaderIP (at the Preheader's terminator,
+/// including) and at AfterIP (at the After's first instruction, excluding).
+/// That is, instructions in the Preheader and After blocks (except the





Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1315
+/// basic blocks. After invalidation, the CanonicalLoopInfo must not be used
+/// anymore as its underlaying control flow does not exist anymore.
+/// Loop-transformation methods such as tileLoops, collapseLoops and unrollLoop

I suppose it may still exist in some cases.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1323
+/// modified loop. What is done is an implementation detail of
+/// transformation-implementing method and callers should always assume the the
+/// CanonicalLoopInfo passed to it is invalidated and a new object is returned.





Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1330
+/// Generally, methods consuming CanonicalLoopInfo do not need an
+/// OpenMPIRBuilder::InsertPointTy as argument, but uses the locations of the
+/// CanonicalLoopInfo to insert new or modify existing instructions. Unless




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107540

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

[PATCH] D107900: Add a new clang-tidy check for trivially copyable types passed by const reference

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst:21
+
+   If set to `true`, this check will limit itself to the `builtinType()` 
types. Default is `false`.

jmarrec wrote:
> Quuxplusone wrote:
> > D107873 is related.
> > 
> > I'd like to see some tests/discussion around large types, e.g.
> > ```
> > struct Widget { int a[1000]; };
> > void f(const Widget&);
> > ```
> > (I think D107873 at least makes a conscious attempt to get the "would it be 
> > passed in registers?" check right.)
> > 
> > I'd like to see some tests/discussion around generic code, e.g.
> > ```
> > template
> > struct Max {
> > static T max(const T& a, const T& b);
> > };
> > int x = Max::max(1, 2);  // passes `int` by const reference, but this 
> > is fine
> > ```
> Should I close this one in favor of https://reviews.llvm.org/D107873?
Up to you. There's always a chance that D107873 doesn't get approved. What I 
find most interesting is that we were independently working on this at the same 
time :-)


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

https://reviews.llvm.org/D107900

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

[PATCH] D106030: [Clang] add support for error+warning fn attrs

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:964-965
+bool match =
+(EA->isError() && NewAttrSpellingIndex < ErrorAttr::GNU_warning) ||
+(EA->isWarning() && NewAttrSpellingIndex >= ErrorAttr::GNU_warning);
+if (!match) {

nickdesaulniers wrote:
> aaron.ballman wrote:
> > It's a bit tricky for me to tell from the patch -- are the enumerators in 
> > the correct order that this logic still works for when the [[]] spelling is 
> > used?
> For reference, the generated `enum Spelling` looks like:
> ```
>  3611 public: 
>   
> 
>  3612   enum Spelling {   
>   
> 
>  3613 GNU_error = 0,  
>   
> 
>  3614 CXX11_gnu_error = 1,
>   
> 
>  3615 C2x_gnu_error = 2,  
>   
> 
>  3616 GNU_warning = 3,
>   
> 
>  3617 CXX11_gnu_warning = 4,  
>   
> 
>  3618 C2x_gnu_warning = 5,
>   
> 
>  3619   SpellingNotCalculated = 15
>   
> 
>  3620 
>   
> 
>  3621   };
> ```
> while using `getAttributeSpellingListIndex` rather than 
> `getNormalizedFullName` allows us to avoid a pair of string comparisons in 
> favor of a pair of `unsigned` comparisons, we now have an issue because there 
> are technically six spellings. (I guess it's not clear to me what's 
> `CXX11_gnu_*` vs `C2x_gnu_*`?)  We could be explicit against checking all six 
> rather than two comparisons.
> 
> Or I can use local variables with more helpful identifiers like:
> 
> ```
> bool NewAttrIsError = NewAttrSpellingIndex < ErrorAttr::GNU_warning;
> bool NewAttrIsWarning = NewAttrSpellingIndex >= ErrorAttr::GNU_warning;
> bool Match = (EA->isError() && NewAttrIsError) || (EA->isWarning() && 
> NewAttrIsWarning);
> ```
> 
> WDYT?
> (I guess it's not clear to me what's CXX11_gnu_* vs C2x_gnu_*?)

A bit of a historical thing. C2x `[[]]` and C++11 `[[]]` are modeled as 
different spellings. This allows us to specify attributes with a `[[]]` 
spelling in only one of the languages without having to do an awkward dance 
involving language options. e.g., it makes it easier to support an attribute 
spelled `__attribute__((foo))` and `[[vendor::foo]]` in C2x and spelled 
`[[foo]]` in C++. it picks up the `gnu` bit in the spelling by virtue of being 
an attribute with a `GCC` spelling -- IIRC, we needed to distinguish between 
GCC and Clang there for some reason I no longer recall.

> WDYT?

I think the current approach is reasonable, but I think the previous approach 
(doing the string compare) was more readable. If you have a preference for 
using the string compare approach as it originally was, I'd be fine with that 
now that I see how my suggestion is playing out in practice. If you'd prefer to 
keep the current approach, I'd also be fine with that. Thank you for trying 
this out, sorry for the hassle!



Comment at: clang/test/Frontend/backend-attribute-error-warning-optimize.c:1
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 -O2 -verify -emit-codegen-only %s

Out of curiosity, why is this required? I would have assumed this would be 
backend-independent functionality?

Also, I have no idea where these tests should live. They're not really frontend 
tests, it's more about the integration between the frontend and the backend. We 
do have `clang/test/Integration` but there's not a lot of content there to be 
able to say "oh yeah, this is testing the same sort of stuff". No idea if other 
reviewers have opinions on this.



Comment at: 

[PATCH] D107461: [PowerPC] Do not define __PRIVILEGED__

[Stefan Pintilie 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 rGa614a28772cb: [PowerPC] Do not define __PRIVILEGED__ 
(authored by stefanp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107461

Files:
  clang/lib/Basic/Targets/PPC.cpp
  clang/test/Driver/ppc-mprivileged-support-check.c
  clang/test/Preprocessor/init-ppc64.c

Index: clang/test/Preprocessor/init-ppc64.c
===
--- clang/test/Preprocessor/init-ppc64.c
+++ clang/test/Preprocessor/init-ppc64.c
@@ -565,7 +565,6 @@
 // PPCPWR8:#define _ARCH_PWR7 1
 // PPCPWR8:#define _ARCH_PWR8 1
 // PPCPWR8-NOT:#define __ROP_PROTECT__ 1
-// PPCPWR8-NOT:#define __PRIVILEGED__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-cpu power8 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPCPOWER8 %s
 //
@@ -584,7 +583,6 @@
 // PPCPOWER8:#define _ARCH_PWR7 1
 // PPCPOWER8:#define _ARCH_PWR8 1
 // PPCPOWER8-NOT:#define __ROP_PROTECT__ 1
-// PPCPOWER8-NOT:#define __PRIVILEGED__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-cpu pwr9 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPCPWR9 %s
 //
@@ -600,7 +598,6 @@
 // PPCPWR9:#define _ARCH_PWR7 1
 // PPCPWR9:#define _ARCH_PWR9 1
 // PPCPWR9-NOT:#define __ROP_PROTECT__ 1
-// PPCPWR9-NOT:#define __PRIVILEGED__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-cpu power9 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPCPOWER9 %s
 //
@@ -616,7 +613,6 @@
 // PPCPOWER9:#define _ARCH_PWR7 1
 // PPCPOWER9:#define _ARCH_PWR9 1
 // PPCPOWER9-NOT:#define __ROP_PROTECT__ 1
-// PPCPOWER9-NOT:#define __PRIVILEGED__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-cpu pwr10 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPCPOWER10 %s
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-cpu power10 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPCPOWER10 %s
@@ -637,7 +633,6 @@
 // PPCPOWER10:#define __MMA__ 1
 // PPCPOWER10:#define __PCREL__ 1
 // PPCPOWER10-NOT:#define __ROP_PROTECT__ 1
-// PPCPOWER10-NOT:#define __PRIVILEGED__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-cpu future -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPCFUTURE %s
 //
@@ -658,7 +653,6 @@
 // PPCFUTURE:#define __MMA__ 1
 // PPCFUTURE:#define __PCREL__ 1
 // PPCFUTURE-NOT:#define __ROP_PROTECT__ 1
-// PPCFUTURE-NOT:#define __PRIVILEGED__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-feature +mma -target-cpu power10 -fno-signed-char < /dev/null | FileCheck -check-prefix PPC-MMA %s
 // PPC-MMA:#define __MMA__ 1
@@ -668,11 +662,6 @@
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-feature +rop-protect -target-cpu power8 -fno-signed-char < /dev/null | FileCheck -check-prefix PPC-ROP %s
 // PPC-ROP:#define __ROP_PROTECT__ 1
 //
-// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-feature +privileged -target-cpu power10 -fno-signed-char < /dev/null | FileCheck -check-prefix PPC-PRIV %s
-// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-feature +privileged -target-cpu power9 -fno-signed-char < /dev/null | FileCheck -check-prefix PPC-PRIV %s
-// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-feature +privileged -target-cpu power8 -fno-signed-char < /dev/null | FileCheck -check-prefix PPC-PRIV %s
-// PPC-PRIV:#define __PRIVILEGED__ 1
-//
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-feature +float128 -target-cpu power9 -fno-signed-char < /dev/null | FileCheck -check-prefix PPC-FLOAT128 %s
 // PPC-FLOAT128:#define __FLOAT128__ 1
 //
Index: clang/test/Driver/ppc-mprivileged-support-check.c
===
--- clang/test/Driver/ppc-mprivileged-support-check.c
+++ clang/test/Driver/ppc-mprivileged-support-check.c
@@ -1,26 +1,29 @@
-// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=pwr10 -mprivileged %s 2>&1 | FileCheck %s --check-prefix=HASPRIV
-// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=power10 -mprivileged %s 2>&1 | FileCheck %s --check-prefix=HASPRIV
-// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=pwr9 -mprivileged %s 2>&1 | FileCheck %s --check-prefix=HASPRIV
-// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=power9 -mprivileged %s 2>&1 | FileCheck %s --check-prefix=HASPRIV
-// RUN: not 

[clang] a614a28 - [PowerPC] Do not define __PRIVILEGED__

[Stefan Pintilie via cfe-commits]

Author: Stefan Pintilie
Date: 2021-08-11T14:10:22-05:00
New Revision: a614a28772cbd8e0fc3c5fcf836493c2c8bc80da

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

LOG: [PowerPC] Do not define __PRIVILEGED__

We do not want to define __PRIVILEGED__. There is no use case for the
definition and gcc does not define it. This patch removes that definition.

Reviewed By: lei, NeHuang

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

Added: 


Modified: 
clang/lib/Basic/Targets/PPC.cpp
clang/test/Driver/ppc-mprivileged-support-check.c
clang/test/Preprocessor/init-ppc64.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/PPC.cpp b/clang/lib/Basic/Targets/PPC.cpp
index 33f266f02b697..839fb93ff3d0c 100644
--- a/clang/lib/Basic/Targets/PPC.cpp
+++ b/clang/lib/Basic/Targets/PPC.cpp
@@ -373,8 +373,6 @@ void PPCTargetInfo::getTargetDefines(const LangOptions 
,
 Builder.defineMacro("__MMA__");
   if (HasROPProtect)
 Builder.defineMacro("__ROP_PROTECT__");
-  if (HasPrivileged)
-Builder.defineMacro("__PRIVILEGED__");
   if (HasP10Vector)
 Builder.defineMacro("__POWER10_VECTOR__");
   if (HasPCRelativeMemops)

diff  --git a/clang/test/Driver/ppc-mprivileged-support-check.c 
b/clang/test/Driver/ppc-mprivileged-support-check.c
index 164b4d9483d32..88b8103b79b1d 100644
--- a/clang/test/Driver/ppc-mprivileged-support-check.c
+++ b/clang/test/Driver/ppc-mprivileged-support-check.c
@@ -1,26 +1,29 @@
-// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=pwr10 -mprivileged %s 2>&1 | FileCheck %s 
--check-prefix=HASPRIV
-// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=power10 -mprivileged %s 2>&1 | FileCheck %s 
--check-prefix=HASPRIV
-// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=pwr9 -mprivileged %s 2>&1 | FileCheck %s --check-prefix=HASPRIV
-// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=power9 -mprivileged %s 2>&1 | FileCheck %s 
--check-prefix=HASPRIV
-// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=pwr8 -mprivileged %s 2>&1 | FileCheck %s --check-prefix=HASPRIV
-// RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
-// RUN:   -mcpu=power8 -mprivileged %s 2>&1 | FileCheck %s 
--check-prefix=HASPRIV
+// RUN: %clang -target powerpc64le-unknown-linux-gnu -S -emit-llvm \
+// RUN:   -mcpu=pwr10 -mprivileged %s -o - | FileCheck %s 
--check-prefix=HASPRIV
+// RUN: %clang -target powerpc64le-unknown-linux-gnu -S -emit-llvm \
+// RUN:   -mcpu=power10 -mprivileged %s -o - | FileCheck %s 
--check-prefix=HASPRIV
+// RUN: %clang -target powerpc64le-unknown-linux-gnu -S -emit-llvm \
+// RUN:   -mcpu=pwr9 -mprivileged %s -o - | FileCheck %s --check-prefix=HASPRIV
+// RUN: %clang -target powerpc64le-unknown-linux-gnu -S -emit-llvm \
+// RUN:   -mcpu=power9 -mprivileged %s -o - | FileCheck %s 
--check-prefix=HASPRIV
+// RUN: %clang -target powerpc64le-unknown-linux-gnu -S -emit-llvm \
+// RUN:   -mcpu=pwr8 -mprivileged %s -o - | FileCheck %s --check-prefix=HASPRIV
+// RUN: %clang -target powerpc64le-unknown-linux-gnu -S -emit-llvm \
+// RUN:   -mcpu=power8 -mprivileged %s -o - | FileCheck %s 
--check-prefix=HASPRIV
 
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
 // RUN:   -mcpu=pwr7 -mprivileged %s 2>&1 | FileCheck %s --check-prefix=NOPRIV
 // RUN: not %clang -target powerpc64le-unknown-linux-gnu -fsyntax-only \
 // RUN:   -mcpu=power7 -mprivileged %s 2>&1 | FileCheck %s 
--check-prefix=NOPRIV
 
-#ifdef __PRIVILEGED__
-static_assert(false, "Privileged instructions enabled");
-#endif
+// __SP__ TODO: Fix this test case to check for the attribute.
+int test() {
+  return 0;
+}
+
+// HASPRIV: test() #0 {
+// HASPRIV: attributes #0 = {
+// HASPRIV-SAME: +privileged
 
-// HASPRIV: Privileged instructions enabled
-// HASPRIV-NOT: option '-mprivileged' cannot be specified with
 // NOPRIV: option '-mprivileged' cannot be specified with
 

diff  --git a/clang/test/Preprocessor/init-ppc64.c 
b/clang/test/Preprocessor/init-ppc64.c
index 45a43b6328d99..fbb3fd7400086 100644
--- a/clang/test/Preprocessor/init-ppc64.c
+++ b/clang/test/Preprocessor/init-ppc64.c
@@ -565,7 +565,6 @@
 // PPCPWR8:#define _ARCH_PWR7 1
 // PPCPWR8:#define _ARCH_PWR8 1
 // PPCPWR8-NOT:#define __ROP_PROTECT__ 1
-// PPCPWR8-NOT:#define __PRIVILEGED__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none 
-target-cpu power8 -fno-signed-char < /dev/null | FileCheck -match-full-lines 
-check-prefix PPCPOWER8 %s
 //
@@ -584,7 +583,6 @@
 // PPCPOWER8:#define _ARCH_PWR7 1
 // PPCPOWER8:#define _ARCH_PWR8 1
 // PPCPOWER8-NOT:#define __ROP_PROTECT__ 1
-// 

[PATCH] D107907: [clang-format] handle trailing comments in function definition detection

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2484-2490
+  if (Next->Next && Next->Next->is(tok::identifier)) {
+const FormatToken *Last = Line.Last;
+if (Last && Last->is(tok::comment))
+  Last = Last->getPreviousNonComment();
+if (Last && Last->isNot(tok::semi))
+  return true;
+  }

  if (Next->Next && Next->Next->is(tok::identifier) &&
  !Line.endsWith(tok::semi))
return true;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107907

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

[PATCH] D107907: [clang-format] handle trailing comments in function definition detection

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

I thought of it last time but didn't try hard to come up with a test case that 
would make a difference. :( Thanks for finding the hole!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107907

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

[PATCH] D107002: [PowerPC] Implement XL compatibility builtin __addex

[Lei Huang via Phabricator via cfe-commits]

lei updated this revision to Diff 365816.
lei added a comment.

Add -W flag to new warning message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107002

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-ppc-xlcompat-pwr9-64bit.c
  clang/test/CodeGen/builtins-ppc-xlcompat-pwr9-error.c
  clang/test/CodeGen/builtins-ppc-xlcompat-pwr9-warning.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/P9InstrResources.td
  llvm/lib/Target/PowerPC/PPCInstr64Bit.td
  llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-pwr9-64bit.ll

Index: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-pwr9-64bit.ll
===
--- llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-pwr9-64bit.ll
+++ llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-pwr9-64bit.ll
@@ -29,3 +29,14 @@
   ret double %0
 }
 declare double @llvm.ppc.insert.exp(double, i64)
+
+declare i64 @llvm.ppc.addex(i64, i64, i32 immarg)
+define dso_local i64 @call_addex_0(i64 %a, i64 %b) {
+; CHECK-LABEL: call_addex_0:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:addex 3, 3, 4, 0
+; CHECK-NEXT:blr
+entry:
+  %0 = tail call i64 @llvm.ppc.addex(i64 %a, i64 %b, i32 0)
+  ret i64 %0
+}
Index: llvm/lib/Target/PowerPC/PPCInstr64Bit.td
===
--- llvm/lib/Target/PowerPC/PPCInstr64Bit.td
+++ llvm/lib/Target/PowerPC/PPCInstr64Bit.td
@@ -1670,6 +1670,13 @@
  "hashchkp $RB, $D_RA_XD", IIC_IntGeneral, []>;
 }
 
+let Interpretation64Bit = 1, isCodeGenOnly = 1, hasSideEffects = 1 in
+def ADDEX8 : Z23Form_RTAB5_CY2<31, 170, (outs g8rc:$rT),
+  (ins g8rc:$rA, g8rc:$rB, u2imm:$CY),
+  "addex $rT, $rA, $rB, $CY", IIC_IntGeneral,
+  [(set i64:$rT, (int_ppc_addex i64:$rA, i64:$rB,
+timm:$CY))]>;
+
 //===--===//
 // Instruction Patterns
 //
Index: llvm/lib/Target/PowerPC/P9InstrResources.td
===
--- llvm/lib/Target/PowerPC/P9InstrResources.td
+++ llvm/lib/Target/PowerPC/P9InstrResources.td
@@ -1430,5 +1430,6 @@
   DCBI,
   DCCCI,
   ICCCI,
-  ADDEX
+  ADDEX,
+  ADDEX8
 )> { let Unsupported = 1; }
Index: llvm/include/llvm/IR/IntrinsicsPowerPC.td
===
--- llvm/include/llvm/IR/IntrinsicsPowerPC.td
+++ llvm/include/llvm/IR/IntrinsicsPowerPC.td
@@ -1706,7 +1706,10 @@
   def int_ppc_fres
   : GCCBuiltin<"__builtin_ppc_fres">,
 Intrinsic <[llvm_float_ty], [llvm_float_ty], [IntrNoMem]>;
-  
+  def int_ppc_addex
+  : GCCBuiltin<"__builtin_ppc_addex">,
+Intrinsic<[llvm_i64_ty], [llvm_i64_ty, llvm_i64_ty, llvm_i32_ty],
+  [IntrNoMem, IntrHasSideEffects, ImmArg>]>;
   def int_ppc_fsel : GCCBuiltin<"__builtin_ppc_fsel">,
  Intrinsic<[llvm_double_ty], [llvm_double_ty, llvm_double_ty, 
   llvm_double_ty], [IntrNoMem]>;
Index: clang/test/CodeGen/builtins-ppc-xlcompat-pwr9-warning.c
===
--- /dev/null
+++ clang/test/CodeGen/builtins-ppc-xlcompat-pwr9-warning.c
@@ -0,0 +1,11 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -target-cpu pwr9 \
+// RUN:   -verify %s
+
+extern unsigned long long ull;
+extern long long ll;
+
+void test_builtin_ppc_addex() {
+  long long res = __builtin_ppc_addex(ll, ll, 1); // expected-warning {{argument value 1 will result in undefined behaviour}}
+  unsigned long long res2 = __builtin_ppc_addex(ull, ull, 3); // expected-warning {{argument value 3 will result in undefined behaviour}}
+}
Index: clang/test/CodeGen/builtins-ppc-xlcompat-pwr9-error.c
===
--- clang/test/CodeGen/builtins-ppc-xlcompat-pwr9-error.c
+++ clang/test/CodeGen/builtins-ppc-xlcompat-pwr9-error.c
@@ -9,7 +9,18 @@
 // RUN:   -fsyntax-only -Wall -Werror -verify %s
 
 extern unsigned int ui;
+extern unsigned long long ull;
+extern long long ll;
 
 void test_builtin_ppc_cmprb() {
   int res = __builtin_ppc_cmprb(3, ui, ui); // expected-error {{argument value 3 is outside the valid range [0, 1]}}
 }
+
+#ifdef __PPC64__
+
+void test_builtin_ppc_addex() {
+  long long res = __builtin_ppc_addex(ll, ll, 4); // expected-error {{argument value 4 is outside the valid range [0, 3]}}
+  unsigned long long res2 = __builtin_ppc_addex(ull, ull, -1); // expected-error {{argument value -1 is outside the valid range [0, 

[PATCH] D107843: [X86] Add parentheses around casts in some of the X86 intrinsic headers.

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

rsmith added a comment.

I found a few more macro hygiene issues in these headers.




Comment at: clang/lib/Headers/avx2intrin.h:23
 #define _mm256_mpsadbw_epu8(X, Y, M) \
   (__m256i)__builtin_ia32_mpsadbw256((__v32qi)(__m256i)(X), \
  (__v32qi)(__m256i)(Y), (int)(M))

Parens missing here still



Comment at: clang/lib/Headers/avx2intrin.h:1094
 #define _mm256_i64gather_ps(m, i, s) \
   (__m128)__builtin_ia32_gatherq_ps256((__v4sf)_mm_undefined_ps(), \
(float const *)(m), \

Parens missing here still.



Comment at: clang/lib/Headers/smmintrin.h:868-871
 #define _mm_extract_ps(X, N) (__extension__  \
   ({ union { int __i; float __f; } __t;  \
  __t.__f = __builtin_ia32_vec_ext_v4sf((__v4sf)(__m128)(X), (int)(N)); \
  __t.__i;}))

This is gross. I wonder if we can use `__builtin_bit_cast` here instead of a 
cast through a union.




Comment at: clang/lib/Headers/smmintrin.h:876
 #define _MM_EXTRACT_FLOAT(D, X, N) \
   { (D) = __builtin_ia32_vec_ext_v4sf((__v4sf)(__m128)(X), (int)(N)); }
 

The existing code is the wrong way to define a statement-like macro, but I 
can't find any documentation (anywhere!) for `_MM_EXTRACT_FLOAT` to confirm 
what the expected valid uses are. The existing formulation would not work in 
contexts such as:

```
if (1)
  _MM_EXTRACT_FLOAT(d, x, n);
else
  ...
```

... because the semicolon would terminate the `if`, and it would incorrectly 
work in contexts requiring braces such as:

```
void f(float *pd, __m128 x, int n) _MM_EXTRACT_FLOAT(*pd, x, n)
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107843

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

[clang] 718c632 - Simplify dllexport class member code, NFC

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2021-08-11T11:42:20-07:00
New Revision: 718c63258202e8e674c47c9ece16f396065e7c2a

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

LOG: Simplify dllexport class member code, NFC

We can hoist the check for the dllexport attribute to before the check
if this is a static data member or method.

Added: 


Modified: 
clang/lib/Sema/SemaDeclCXX.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 112722be2fa1..38f21c8ec47c 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -5983,11 +5983,14 @@ static void ReferenceDllExportedMembers(Sema , 
CXXRecordDecl *Class) {
 S.MarkVTableUsed(Class->getLocation(), Class, true);
 
   for (Decl *Member : Class->decls()) {
+// Skip members that were not marked exported.
+if (!Member->hasAttr())
+  continue;
+
 // Defined static variables that are members of an exported base
 // class must be marked export too.
 auto *VD = dyn_cast(Member);
-if (VD && Member->getAttr() &&
-VD->getStorageClass() == SC_Static &&
+if (VD && VD->getStorageClass() == SC_Static &&
 TSK == TSK_ImplicitInstantiation)
   S.MarkVariableReferenced(VD->getLocation(), VD);
 
@@ -5995,40 +5998,38 @@ static void ReferenceDllExportedMembers(Sema , 
CXXRecordDecl *Class) {
 if (!MD)
   continue;
 
-if (Member->getAttr()) {
-  if (MD->isUserProvided()) {
-// Instantiate non-default class member functions ...
+if (MD->isUserProvided()) {
+  // Instantiate non-default class member functions ...
 
-// .. except for certain kinds of template specializations.
-if (TSK == TSK_ImplicitInstantiation && !ClassAttr->isInherited())
-  continue;
+  // .. except for certain kinds of template specializations.
+  if (TSK == TSK_ImplicitInstantiation && !ClassAttr->isInherited())
+continue;
 
-S.MarkFunctionReferenced(Class->getLocation(), MD);
+  S.MarkFunctionReferenced(Class->getLocation(), MD);
 
-// The function will be passed to the consumer when its definition is
-// encountered.
-  } else if (MD->isExplicitlyDefaulted()) {
-// Synthesize and instantiate explicitly defaulted methods.
-S.MarkFunctionReferenced(Class->getLocation(), MD);
+  // The function will be passed to the consumer when its definition is
+  // encountered.
+} else if (MD->isExplicitlyDefaulted()) {
+  // Synthesize and instantiate explicitly defaulted methods.
+  S.MarkFunctionReferenced(Class->getLocation(), MD);
 
-if (TSK != TSK_ExplicitInstantiationDefinition) {
-  // Except for explicit instantiation defs, we will not see the
-  // definition again later, so pass it to the consumer now.
-  S.Consumer.HandleTopLevelDecl(DeclGroupRef(MD));
-}
-  } else if (!MD->isTrivial() ||
- MD->isCopyAssignmentOperator() ||
- MD->isMoveAssignmentOperator()) {
-// Synthesize and instantiate non-trivial implicit methods, and the 
copy
-// and move assignment operators. The latter are exported even if they
-// are trivial, because the address of an operator can be taken and
-// should compare equal across libraries.
-S.MarkFunctionReferenced(Class->getLocation(), MD);
-
-// There is no later point when we will see the definition of this
-// function, so pass it to the consumer now.
+  if (TSK != TSK_ExplicitInstantiationDefinition) {
+// Except for explicit instantiation defs, we will not see the
+// definition again later, so pass it to the consumer now.
 S.Consumer.HandleTopLevelDecl(DeclGroupRef(MD));
   }
+} else if (!MD->isTrivial() ||
+   MD->isCopyAssignmentOperator() ||
+   MD->isMoveAssignmentOperator()) {
+  // Synthesize and instantiate non-trivial implicit methods, and the copy
+  // and move assignment operators. The latter are exported even if they
+  // are trivial, because the address of an operator can be taken and
+  // should compare equal across libraries.
+  S.MarkFunctionReferenced(Class->getLocation(), MD);
+
+  // There is no later point when we will see the definition of this
+  // function, so pass it to the consumer now.
+  S.Consumer.HandleTopLevelDecl(DeclGroupRef(MD));
 }
   }
 }



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

[PATCH] D107921: [Modules] Fix bug where header resolution in modules doesn't work when compiling with relative paths.

[Amy Huang via Phabricator via cfe-commits]

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

Currently when searching for header files (with relative paths) we concatenate
the path to the module map with the header filename. However when
searching for system headers it seems like we should start from the
current directory and not the path to the module map.

No test added because I'm not sure there's a way to use relative paths
to system headers in the lit tests.

Bug: https://bugs.llvm.org/show_bug.cgi?id=47209


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107921

Files:
  clang/lib/Lex/HeaderSearch.cpp


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -819,9 +819,19 @@
   bool IncluderIsSystemHeader =
   Includer ? getFileInfo(Includer).DirInfo != SrcMgr::C_User :
   BuildSystemModule;
-  if (Optional FE = getFileAndSuggestModule(
-  TmpDir, IncludeLoc, IncluderAndDir.second, 
IncluderIsSystemHeader,
-  RequestingModule, SuggestedModule)) {
+
+  Optional FE = getFileAndSuggestModule(
+  TmpDir, IncludeLoc, IncluderAndDir.second, IncluderIsSystemHeader,
+  RequestingModule, SuggestedModule);
+
+  // If this is a system header, we should also search from the current
+  // working directory and not the directory of the module map.
+  if (!FE && IncluderIsSystemHeader)
+FE = getFileAndSuggestModule(
+Filename, IncludeLoc, IncluderAndDir.second, 
IncluderIsSystemHeader,
+RequestingModule, SuggestedModule);
+
+  if (FE) {
 if (!Includer) {
   assert(First && "only first includer can have no file");
   return FE;


Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -819,9 +819,19 @@
   bool IncluderIsSystemHeader =
   Includer ? getFileInfo(Includer).DirInfo != SrcMgr::C_User :
   BuildSystemModule;
-  if (Optional FE = getFileAndSuggestModule(
-  TmpDir, IncludeLoc, IncluderAndDir.second, IncluderIsSystemHeader,
-  RequestingModule, SuggestedModule)) {
+
+  Optional FE = getFileAndSuggestModule(
+  TmpDir, IncludeLoc, IncluderAndDir.second, IncluderIsSystemHeader,
+  RequestingModule, SuggestedModule);
+
+  // If this is a system header, we should also search from the current
+  // working directory and not the directory of the module map.
+  if (!FE && IncluderIsSystemHeader)
+FE = getFileAndSuggestModule(
+Filename, IncludeLoc, IncluderAndDir.second, IncluderIsSystemHeader,
+RequestingModule, SuggestedModule);
+
+  if (FE) {
 if (!Includer) {
   assert(First && "only first includer can have no file");
   return FE;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106614: [Clang] add btf_tag attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Assuming we're back on the same page again, I think the only thing left in this 
review is a commenting nit.




Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6846-6847
+return;
+  if (hasBTFTagAttr(D, Str))
+return;
+

yonghong-song wrote:
> aaron.ballman wrote:
> > This should diagnose that the attribute is being ignored due to the 
> > mismatch in tags (and probably have a note to the previous declaration as 
> > well so users can see both sides of the conflict).
> Just to make sure we are on the same page. The attribute is ignored because 
> it has been defined in the declaration. This is specifically to handle cases 
> like:
> #define __tag1 __attribute__((btf_tag("info")))
> #define __tag2 __attribute__((btf_tag("info2")))
> int var __tag1 __tag1, __tag2;
> The first __tag1 will be added to declaration successfully, but the second 
> __tag1 will be ignored since there exists an identical one. __tag2 will be 
> added to the declaration successfully.
> 
> I think handleBTFTagAttr() does not handle merging declarations? It is 
> mergeBTFTagAttr below to handle merging? If handleBTFTagAttr() is to handle 
> merging declarations, it will handle attributes the same as below 
> mergeBTFTagAttr, i.e., merging all attributes at the same time doing dedup.
> 
> Do you want issue an warning for ignored attribute?
ohhh, I think we weren't on the same page, sorry about that! I had the 
semantics wrong in my head -- I was thinking we wanted to reject different 
tags, but it's the opposite, we want to allow multiple tags so long as they 
have different arguments. Assuming I have that correct now, this approach looks 
correct to me (without diagnosing the ignored duplicates).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106614

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

[PATCH] D107717: [LLVM][CMake][NFC] Resolve FIXME: Rename LLVM_CMAKE_PATH to LLVM_CMAKE_DIR throughout the project

[Alf via Phabricator via cfe-commits]

gAlfonso-bit added a comment.

@ldionne is this ok? any other changes?


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

https://reviews.llvm.org/D107717

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

[PATCH] D107095: Implement #pragma clang header_unsafe

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

The technical bits all LGTM, thank you for the functionality! There was a 
comment on IRC that `header_unsafe` might be confusing to users as it may 
suggest that the *header* is unsafe rather than the use of the macro. I don't 
know if we resolved the naming question on IRC or whether that's something 
you'd like to bikeshed on more before we land this? I'll mark as approved once 
we're comfortable with the name.

Btw, I think you should add something to the release notes to alert folks to 
all of the awesome new preprocessor functionality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107095

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

[PATCH] D107696: [CodeComplete] Basic code completion for attribute names.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D107696#2939827 , @aaron.ballman 
wrote:

> Btw, I'm not certain why patch application is failing for you in the 
> precommit CI: 
> https://buildkite.com/llvm-project/diff-checks/builds/58571#c186a7d3-f5c9-4ad2-ae27-07408b1c5dad
>  It doesn't seem like your patch is the cause of it, but this review is the 
> only one where I've noticed the issue.

Literally the next review I looked at had this same issue 
(https://reviews.llvm.org/D107095), so I'm more comfortable saying the CI 
failure is unrelated to anything in your patch. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107696

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

[PATCH] D107684: [NFC][AVR][clang] Add test for D107672

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D107684#2936034 , @mhjacobson 
wrote:

> In D107684#2936033 , @MaskRay wrote:
>
>> `--sysroot %S/Inputs/basic_avr_tree/usr/lib` seems incorrect.
>>
>> sysroot is usually a directory which contains `usr/lib` and `lib/`. 
>> `somewhere/usr/lib` is not usually used as a sysroot.
>
> Yeah, I agree it's weird.  This is basically a shortcut to avoid creating a 
> duplicate sysroot in `Inputs`.  Would you prefer I create the duplicate tree?

A duplicate tree is better. Otherwise you can use 
`unittests/Driver/ToolChainTest.cpp`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107684

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

[PATCH] D107696: [CodeComplete] Basic code completion for attribute names.

[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.

Btw, I'm not certain why patch application is failing for you in the precommit 
CI: 
https://buildkite.com/llvm-project/diff-checks/builds/58571#c186a7d3-f5c9-4ad2-ae27-07408b1c5dad
 It doesn't seem like your patch is the cause of it, but this review is the 
only one where I've noticed the issue. Perhaps if you upload a new patch CI 
will be happier? Otherwise, beware of bot stampedes when you land. :-)

This LGTM (if you want to, feel free to switch to `Twine` in the one place I 
called out, but we don't need another round of review for that), thank you for 
the patch!




Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4390
+  auto AddCompletions = [&](const ParsedAttrInfo ) {
+if (A.IsTargetSpecific && !A.existsInTarget(Context.getTargetInfo()))
+  return;

sammccall wrote:
> aaron.ballman wrote:
> > Should we also early return if the attribute is ignored? (See `IgnoredAttr` 
> > in `Attr.td`) I'm assuming that attributes which do nothing are unlikely to 
> > be high-value attributes to autocomplete (so maybe they'd go at the end of 
> > the list if we wanted to keep them).
> Hmm, I'm not sure about that.
> They do nothing *in clang*, but using clang-based code completion doesn't 
> particularly mean we'll use clang to build the code.
> Something must care about the attribute, even if it's just a doc generator or 
> something.
> 
> In practice, there are 6 attrs with Ignored = 1:
>  - `bounded` is an openbsd-gcc extension
>  - 3 are cuda-specific and probably only used by cuda stdlib headers
>  - `__w64` is a keyword so not relevant here
>  - `property` is actually supported by clang, it seems `Ignored` is lying!
> 
> So none of these are *terribly* important, but they also don't seem so 
> worthless that it's important to exclude them. (There are other attributes 
> that aren't important too!)
> Hmm, I'm not sure about that.

I'm convinced by your reasoning, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107696

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

[PATCH] D107850: [WIP][asan] Implemented custom calling convention similar used by HWASan for X86. The feature should be code complete. The tests are coming in a later revision.

[Kirill Stoimenov via Phabricator via cfe-commits]

kstoimenov updated this revision to Diff 365787.
kstoimenov added a comment.

Address lint comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107850

Files:
  clang/test/CodeGen/asan-use-callbacks.cpp
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/Transforms/Instrumentation/AddressSanitizer.h
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/lib/Target/X86/X86AsmPrinter.h
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86MCInstLower.cpp
  llvm/lib/Target/X86/X86RegisterInfo.td
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -348,6 +348,10 @@
 static cl::opt ClOpt("asan-opt", cl::desc("Optimize instrumentation"),
cl::Hidden, cl::init(true));
 
+static cl::opt ClOptimizeCallbacks("asan-optimize-callbacks",
+ cl::desc("Optimize callbacks"),
+ cl::Hidden, cl::init(false));
+
 static cl::opt ClOptSameTemp(
 "asan-opt-same-temp", cl::desc("Instrument the same temp just once"),
 cl::Hidden, cl::init(true));
@@ -623,6 +627,7 @@
 C = &(M.getContext());
 LongSize = M.getDataLayout().getPointerSizeInBits();
 IntptrTy = Type::getIntNTy(*C, LongSize);
+Int8PtrTy = Type::getInt8PtrTy(*C);
 TargetTriple = Triple(M.getTargetTriple());
 
 Mapping = getShadowMapping(TargetTriple, LongSize, this->CompileKernel);
@@ -673,6 +678,7 @@
  Value *SizeArgument, uint32_t Exp);
   void instrumentMemIntrinsic(MemIntrinsic *MI);
   Value *memToShadow(Value *Shadow, IRBuilder<> );
+  void encodeMemToShadowInfo(int64_t *AccessInfo);
   bool suppressInstrumentationSiteForDebug(int );
   bool instrumentFunction(Function , const TargetLibraryInfo *TLI);
   bool maybeInsertAsanInitAtFunctionEntry(Function );
@@ -713,6 +719,7 @@
   bool UseAfterScope;
   AsanDetectStackUseAfterReturnMode UseAfterReturn;
   Type *IntptrTy;
+  Type *Int8PtrTy;
   ShadowMapping Mapping;
   FunctionCallee AsanHandleNoReturnFunc;
   FunctionCallee AsanPtrCmpFunction, AsanPtrSubFunction;
@@ -1361,6 +1368,15 @@
 return IRB.CreateAdd(Shadow, ShadowBase);
 }
 
+void AddressSanitizer::encodeMemToShadowInfo(int64_t *AccessInfo) {
+  *AccessInfo +=
+  (Mapping.Scale << AsanAccessInfo::MappingScaleShift) +
+  (Mapping.OrShadowOffset << AsanAccessInfo::OrShadowOffsetShift);
+  if (LocalDynamicShadow || Mapping.InGlobal) {
+// TODO(kstoimenov): for now fail.
+  }
+}
+
 // Instrument memset/memmove/memcpy
 void AddressSanitizer::instrumentMemIntrinsic(MemIntrinsic *MI) {
   IRBuilder<> IRB(MI);
@@ -1742,12 +1758,25 @@
   size_t AccessSizeIndex = TypeSizeToSizeIndex(TypeSize);
 
   if (UseCalls) {
-if (Exp == 0)
-  IRB.CreateCall(AsanMemoryAccessCallback[IsWrite][0][AccessSizeIndex],
- AddrLong);
-else
-  IRB.CreateCall(AsanMemoryAccessCallback[IsWrite][1][AccessSizeIndex],
- {AddrLong, ConstantInt::get(IRB.getInt32Ty(), Exp)});
+if (ClOptimizeCallbacks) {
+  Value *Ptr8 = IRB.CreatePointerCast(Addr, Int8PtrTy);
+  Module *M = IRB.GetInsertBlock()->getParent()->getParent();
+  int64_t AccessInfo =
+  (IsWrite << AsanAccessInfo::IsWriteShift) +
+  (AccessSizeIndex << AsanAccessInfo::AccessSizeIndexShift);
+  encodeMemToShadowInfo();
+  IRB.CreateCall(
+  Intrinsic::getDeclaration(M, Intrinsic::asan_check_memaccess),
+  {Ptr8, ConstantInt::get(IRB.getInt64Ty(), Mapping.Offset),
+   ConstantInt::get(IRB.getInt32Ty(), AccessInfo)});
+} else {
+  if (Exp == 0)
+IRB.CreateCall(AsanMemoryAccessCallback[IsWrite][0][AccessSizeIndex],
+   AddrLong);
+  else
+IRB.CreateCall(AsanMemoryAccessCallback[IsWrite][1][AccessSizeIndex],
+   {AddrLong, ConstantInt::get(IRB.getInt32Ty(), Exp)});
+}
 return;
   }
 
Index: llvm/lib/Target/X86/X86RegisterInfo.td
===
--- llvm/lib/Target/X86/X86RegisterInfo.td
+++ llvm/lib/Target/X86/X86RegisterInfo.td
@@ -436,6 +436,12 @@
  (add RAX, RCX, RDX, RSI, RDI, R8, R9, R10, R11,
   RBX, R14, R15, R12, R13, RBP, RSP, RIP)>;
 
+// GR64 - 64-bit GPRs without RAX and RIP. Could be used when emitting code for
+// intrinsics, which use implict input registers.
+def GR64NoRAX : RegisterClass<"X86", [i64], 64,
+  (add RCX, RDX, RSI, RDI, R8, R9, R10, R11,
+   RBX, R14, R15, R12, R13, RBP, RSP)>;
+
 // Segment registers for use by MOV 

[PATCH] D107907: [clang-format] handle trailing comments in function definition detection

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay accepted this revision.
MyDeveloperDay 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/D107907/new/

https://reviews.llvm.org/D107907

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

[PATCH] D107893: [clang] [MinGW] Consider the per-target libc++ include directory too

[Petr Hosek via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107893

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

[PATCH] D107900: Add a new clang-tidy check for trivially copyable types passed by const reference

[Julien Marrec via Phabricator via cfe-commits]

jmarrec updated this revision to Diff 365779.
jmarrec added a comment.

Forgot to run clang format on the changes


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

https://reviews.llvm.org/D107900

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/PodConstRefToValueCheck.cpp
  clang-tools-extra/clang-tidy/misc/PodConstRefToValueCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-pod-const-ref-to-value.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-pod-const-ref-to-value.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-pod-const-ref-to-value.cpp
@@ -0,0 +1,75 @@
+// RUN: %check_clang_tidy %s misc-pod-const-ref-to-value %t
+
+int f1(const int );
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'i' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f1(int i);
+int f2(const int , double d);
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'i' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f2(int i, double d);
+
+int f3(double d, const int );
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: argument 'i' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f3(double d, int i);
+
+int f4(const double , const double );
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'd' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-MESSAGES: :[[@LINE-2]]:25: warning: argument 'd2' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f4(double d, double d2);
+
+// clang-format off
+int f5(const int& i);
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'i' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f5(int i);
+// clang-format on
+
+namespace n1 {
+class A {
+  static int g(const double );
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: argument 'd' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+  // CHECK-FIXES: static int g(double d);
+};
+
+int A::g(const double ) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: argument 'd' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+  // CHECK-FIXES: int A::g(double d) {
+  return static_cast(d);
+}
+} // namespace n1
+
+// Not triggering the check
+
+int f5(int i);
+int f6(int i, double d);
+int f7(int ); // Might be used for return...
+
+namespace n2 {
+class A {
+  static int g(double d);
+};
+
+int A::g(double d) {
+  return static_cast(d);
+}
+
+class B {
+  static int g(double );
+};
+
+int B::g(double ) {
+  return static_cast(d);
+}
+} // namespace n2
+
+template 
+void f(Args &&...args);
+
+struct Widget {
+  int a[1000];
+};
+void f(const Widget &);
+
+template 
+struct Max {
+  static T max(const T , const T );
+};
+int x = Max::max(1, 2); // passes `int` by const reference, but this is fine
Index: clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst
@@ -0,0 +1,25 @@
+.. title:: clang-tidy - misc-pod-const-ref-to-value
+
+misc-pod-const-ref-to-value
+===
+
+This check detects when ``trivially_copyable`` types are passed by const-reference
+to a function and changes that to by value
+
+For example in the following code, it is replaced by ``void f(int i, double d)``
+
+.. code-block:: c++
+
+  void f(const int& i, const double& d);
+
+
+Options
+---
+
+.. option:: RestrictToBuiltInTypes
+
+   If set to `true`, this check will limit itself to the `builtinType()` types. Default is `false`.
+
+.. option:: MaxSize
+
+   MaxSize: int, default 16. Above this size, passing by const-reference makes sense
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -212,6 +212,7 @@
`misc-no-recursion `_,
`misc-non-copyable-objects `_,
`misc-non-private-member-variables-in-classes `_,
+   `misc-pod-const-ref-to-value `_, "Yes"

[PATCH] D107900: Add a new clang-tidy check for trivially copyable types passed by const reference

[Julien Marrec via Phabricator via cfe-commits]

jmarrec updated this revision to Diff 365778.
jmarrec added a comment.

Added a MaxSize option. defaults to 16. Also skip templates. like in D107873 


Dealt with the `f(int )` case instead of `f(int& i)`


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

https://reviews.llvm.org/D107900

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/PodConstRefToValueCheck.cpp
  clang-tools-extra/clang-tidy/misc/PodConstRefToValueCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-pod-const-ref-to-value.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-pod-const-ref-to-value.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-pod-const-ref-to-value.cpp
@@ -0,0 +1,73 @@
+// RUN: %check_clang_tidy %s misc-pod-const-ref-to-value %t
+
+int f1(const int );
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'i' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f1(int i);
+int f2(const int , double d);
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'i' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f2(int i, double d);
+
+int f3(double d, const int );
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: argument 'i' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f3(double d, int i);
+
+int f4(const double , const double );
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'd' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-MESSAGES: :[[@LINE-2]]:25: warning: argument 'd2' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f4(double d, double d2);
+
+// clang-format off
+int f5(const int& i);
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'i' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f5(int i);
+// clang-format on
+
+namespace n1 {
+class A {
+  static int g(const double );
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: argument 'd' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+  // CHECK-FIXES: static int g(double d);
+};
+
+int A::g(const double ) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: argument 'd' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+  // CHECK-FIXES: int A::g(double d) {
+  return static_cast(d);
+}
+} // namespace n1
+
+// Not triggering the check
+
+int f5(int i);
+int f6(int i, double d);
+int f7(int ); // Might be used for return...
+
+namespace n2 {
+class A {
+  static int g(double d);
+};
+
+int A::g(double d) {
+  return static_cast(d);
+}
+
+class B {
+  static int g(double );
+};
+
+int B::g(double ) {
+  return static_cast(d);
+}
+} // namespace n2
+
+template 
+void f(Args&&... args);
+
+struct Widget { int a[1000]; };
+void f(const Widget&);
+
+template
+struct Max {
+static T max(const T& a, const T& b);
+};
+int x = Max::max(1, 2);  // passes `int` by const reference, but this is fine
Index: clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst
@@ -0,0 +1,25 @@
+.. title:: clang-tidy - misc-pod-const-ref-to-value
+
+misc-pod-const-ref-to-value
+===
+
+This check detects when ``trivially_copyable`` types are passed by const-reference
+to a function and changes that to by value
+
+For example in the following code, it is replaced by ``void f(int i, double d)``
+
+.. code-block:: c++
+
+  void f(const int& i, const double& d);
+
+
+Options
+---
+
+.. option:: RestrictToBuiltInTypes
+
+   If set to `true`, this check will limit itself to the `builtinType()` types. Default is `false`.
+
+.. option:: MaxSize
+
+   MaxSize: int, default 16. Above this size, passing by const-reference makes sense
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -212,6 +212,7 @@
`misc-no-recursion `_,
`misc-non-copyable-objects `_,

[PATCH] D107703: [AST][clangd] Expose documentation of Attrs on hover.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107703

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

[PATCH] D107876: [CSSPGO] Allow the use of debug-info-for-profiling and pseudo-probe-for-profiling together

[Hongtao Yu via Phabricator via cfe-commits]

hoy added a comment.

In D107876#2939624 , @wmi wrote:

> Could you remind me the discriminator difference between debug info based 
> AFDO and pseudo probe based AFDO? I forgot it. I am sure it is described in 
> some patch but I havn't found it. Give me a pointer is good enough.

In short, unlike block pseudo probe, a callsite probe relies on the calliste 
discriminator to be tracked. This has a couple usage: 1. to track direct call 
and indirect callsite 2. to model inline contexts with a list of callsite 
probes.

A probe discriminator is treated as empty or null for AutoFDO since it has 
lowest three bits as "111".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107876

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

[PATCH] D107876: [CSSPGO] Allow the use of debug-info-for-profiling and pseudo-probe-for-profiling together

[Hongtao Yu via Phabricator via cfe-commits]

hoy added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3896
-  // These two forms of profiling info can't be used together.
-  if (const Arg *A1 = 
Args.getLastArg(options::OPT_fpseudo_probe_for_profiling))
-if (const Arg *A2 = 
Args.getLastArg(options::OPT_fdebug_info_for_profiling))

wenlei wrote:
> Would a warning be useful to catch accidental misuse? 
I was thinking about that. It may be useful, but downstream may have to 
suppress that warning to be treated as error for a clean build. What do you 
think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107876

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

[PATCH] D102693: Do not create LLVM IR `constant`s for objects with dynamic initialisation

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

I ran into a regression caused by this change, see 
https://bugs.llvm.org/show_bug.cgi?id=51442. This breaks building Qt tests with 
LLVM 13.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102693

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

[PATCH] D107876: [CSSPGO] Allow the use of debug-info-for-profiling and pseudo-probe-for-profiling together

[Wei Mi via Phabricator via cfe-commits]

wmi added a comment.

Could you remind me the discriminator difference between debug info based AFDO 
and pseudo probe based AFDO? I forgot it. I am sure it is described in some 
patch but I havn't found it. Give me a pointer is good enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107876

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

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 365765.
MyDeveloperDay added a comment.

- Remove the "CV" from the options
- Add support for ordering static/inline/const/volatile around the type, 
allowing for mixing both left and right alignment at the same time

  QualifierAlignment: Custom
  QualifierOrder: [ "inline", "static", "volatile", "type", const ]



- Add "Custom" format to allow use of QualifierOrder  (may want to change the 
`Qualifier` name as its now both Qualifiers and Specifiers)
- Add Parse errors and test to check for unsupported specifiers/qualifiers, 
duplicates,missing 'type' field and empty Order lists
- Add Defaults for Left and Right Alignment (may need to discuss suitable 
defaults).
- Removal of excess braces (if only clang-format could do that for us!!)

NOTE: Known issue:

Despite the clang-format file being already format correctly, I seem to be 
getting replacements: (e.g.)

For the following example

  inline volatile int const c;

with the .clang-format file of:

  ---
  Language: Cpp
  BasedOnStyle: LLVM
  QualifierAlignment: Custom
  QualifierOrder: [ inline, static, volatile, type, const ]

I get the following:

  $ clang-format -n test1.cpp
  test1.cpp:1:1: warning: code should be clang-formatted 
[-Wclang-format-violations]
  inline volatile int const c;
  ^

I believe caused by the existence of replacements that effectively do nothing.

  $ clang-format test1.cpp --output-replacements-xml
  
  
  inline volatile
  

Its likely I am switching the order from:

  inline volatile int const c;

to

  volatile inline int const c;

and then back to

  inline volatile int const c;

Which I think likely becomes  a single `inline volatile` replacement. This 
could in theory be a corner case for tooling::Replacements but I need to dig in 
more, its more likely an artefact of the reversing the `LeftOrder` rather 
keeping the Left order and not pushing specifiers though other specifiers that 
are more `LeftMost` combined with each specifier/qualifier being handled in its 
own pass.

I'll look into that next, but wanted to park the current implementation as 
functionally this is a significant step closer.


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

https://reviews.llvm.org/D69764

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/CMakeLists.txt
  clang/lib/Format/Format.cpp
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/lib/Format/QualifierAlignmentFixer.h
  clang/tools/clang-format/ClangFormat.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18180,6 +18180,10 @@
   EXPECT_EQ(0, parseConfiguration(TEXT, ).value());  \
   EXPECT_EQ(VALUE, Style.FIELD) << "Unexpected value after parsing!"
 
+#define FAIL_PARSE(TEXT, FIELD, VALUE) \
+  EXPECT_NE(0, parseConfiguration(TEXT, ).value());  \
+  EXPECT_EQ(VALUE, Style.FIELD) << "Unexpected value after parsing!"
+
 TEST_F(FormatTest, ParsesConfigurationBools) {
   FormatStyle Style = {};
   Style.Language = FormatStyle::LK_Cpp;
@@ -18277,6 +18281,26 @@
   CHECK_PARSE("ContinuationIndentWidth: 11", ContinuationIndentWidth, 11u);
   CHECK_PARSE("CommentPragmas: '// abc$'", CommentPragmas, "// abc$");
 
+  Style.QualifierAlignment = FormatStyle::QAS_Right;
+  CHECK_PARSE("QualifierAlignment: Leave", QualifierAlignment,
+  FormatStyle::QAS_Leave);
+  CHECK_PARSE("QualifierAlignment: Right", QualifierAlignment,
+  FormatStyle::QAS_Right);
+  CHECK_PARSE("QualifierAlignment: Left", QualifierAlignment,
+  FormatStyle::QAS_Left);
+  CHECK_PARSE("QualifierAlignment: Custom", QualifierAlignment,
+  FormatStyle::QAS_Custom);
+
+  Style.QualifierOrder.clear();
+  CHECK_PARSE("QualifierOrder: [ const, volatile, type ]", QualifierOrder,
+  std::vector({"const", "volatile", "type"}));
+  Style.QualifierOrder.clear();
+  CHECK_PARSE("QualifierOrder: [const, type]", QualifierOrder,
+  std::vector({"const", "type"}));
+  Style.QualifierOrder.clear();
+  CHECK_PARSE("QualifierOrder: [volatile, type]", QualifierOrder,
+  std::vector({"volatile", "type"}));
+
   Style.AlignConsecutiveAssignments = FormatStyle::ACS_Consecutive;
   CHECK_PARSE("AlignConsecutiveAssignments: None", AlignConsecutiveAssignments,
   FormatStyle::ACS_None);
@@ -18918,8 +18942,6 @@
   EXPECT_EQ(FormatStyle::LK_Cpp, Style.Language);
 }
 
-#undef CHECK_PARSE
-
 TEST_F(FormatTest, UsesLanguageForBasedOnStyle) {
   FormatStyle Style = {};
   Style.Language = FormatStyle::LK_JavaScript;
@@ -0,6 +22242,494 @@
   "}";
   EXPECT_EQ(Code, format(Code, Style));
 }
+
+TEST_F(FormatTest, LeftRightQualifier) {

[PATCH] D107641: [clang-tidy] fix duplicate '{}' in cppcoreguidelines-pro-type-member-init

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp:439
+   [&](const FieldDecl *F) { 
+if (!HasRecordClasMemberSet.count(F))
+{

MTC wrote:
> I believe `DenseSet::contains` is more appropriate here.
+1 to the suggestion to use `contains()`.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h:79
+  // initialization.
+  llvm::DenseSet HasRecordClasMemberSet;
 };




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

https://reviews.llvm.org/D107641

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

[PATCH] D107641: [clang-tidy] fix duplicate '{}' in cppcoreguidelines-pro-type-member-init

[gehry via Phabricator via cfe-commits]

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

Any thoughts? : )


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

https://reviews.llvm.org/D107641

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

[PATCH] D107294: [clang-tidy] adds warning to suggest users replace symbols with words

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp:51-54
+  if (!getLangOpts().C99) {
+return llvm::None;
+  }
+





Comment at: 
clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp:69
+  // Only insert spaces if there aren't already spaces between operators
+  StringRef SpaceBefore = std::isspace(lookahead(SM, Loc, -1)) ? "" : " ";
+  StringRef SpaceAfter =

Hrmm, I feel like `std::isspace()` is not going to properly handle all the 
Unicode whitespace, but I don't think we handle them all anyway as I notice the 
lexer is using `isHorizontalWhitespace()`, `isVerticalWhitespace()`, and 
`isWhitespace()` from `CharInfo.h`. Maybe we should use the same utility here 
just to match the lexer? (I don't feel strongly, just tickled my spidey senses.)



Comment at: 
clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp:78-79
+   const UnaryOperator ) {
+  // TODO: make check configurable so users can decide which operators are to 
be
+  // symbols and which are to be words.
+

Are you planning to do this as part of this patch (if not, should this switch 
to FIXME)?



Comment at: 
clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp:85
+  assert(First != nullptr);
+  if (std::isalpha(*First) || Loc.isMacroID())
+return;

`isLetter()`? (or potentially `isIdentifierHead()` if we need to care about 
dollar-sign prefixed identifiers)



Comment at: 
clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp:104
+   const BinaryOperator ) {
+  // TODO: make check configurable so users can decide which operators are to 
be
+  // symbols and which are to be words.

Same here as above.



Comment at: 
clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp:111
+  assert(First != nullptr);
+  if (std::isalpha(*First) || Loc.isMacroID())
+return;

Same here as above.



Comment at: 
clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp:117-120
+  case Opcode::BO_LAnd:
+diag(Loc, "use 'and' for logical conjunctions")
+<< createReplacement(SM, Loc, "and", 2) << includeIso646(SM, Loc);
+break;

I think these can be replaced with a helper function along these lines:
```
void emitDiag(const SourceManager , SourceLocation Loc, StringRef Op, 
StringRef OpDesc, int Lookahead) {
  diag(Loc, ("use '" + Op + "' for " + OpDesc).str()) <<
createReplacement(SM, Loc, Op, Lookahead) << includeIso646(SM, Loc);
}
```
WDYT?



Comment at: 
clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp:144-148
+  // TODO: make check configurable so users can decide which operators are to 
be
+  // symbols and which are to be words.
+
+  // TODO: add check to make it possible to "intelligently" determine if symbol
+  // is always preferred (e.g. operator| being used for composition).

FIXMEs?



Comment at: 
clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp:158-164
+  constexpr Opcode And = Opcode::OO_AmpAmp;
+  constexpr Opcode Bitand = Opcode::OO_Amp;
+  constexpr Opcode Bitor = Opcode::OO_Pipe;
+  constexpr Opcode Compl = Opcode::OO_Tilde;
+  constexpr Opcode Not = Opcode::OO_Exclaim;
+  constexpr Opcode Or = Opcode::OO_PipePipe;
+  constexpr Opcode Xor = Opcode::OO_Caret;

Nit: given that these are only used once, I'd rather just spell out the `case` 
labels longhand.



Comment at: clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.h:35
+   Preprocessor *ModuleExpanderPP) override {
+// TODO: add support for checking preprocessor directives
+Includer.registerPreprocessor(PP);





Comment at: clang-tools-extra/clang-tidy/readability/StrictConstCorrectness.h:9
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRICTCONSTCORRECTNESS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRICTCONSTCORRECTNESS_H

Did you mean to include this file in this review?



Comment at: 
clang-tools-extra/clang-tidy/readability/StrictConstCorrectness.kpp:1
+//===-- StringCompareCheck.cpp - 
clang-tidy===//
+//

Same for this one?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-alternative-tokens.rst:4
+readability-alternative-tokens
+==
+





Comment at: 
clang/test/Analysis/diagnostics/readability-strict-const-correctness.cpp:1
+// RUN: %check_clang_tidm1 %s readabilitm1-strict-const-correctness %t
+int f1()

[PATCH] D107646: [PowerPC] Fix the frame addresss computing return address for `__builtin_return_address`

[Nemanja Ivanovic via Phabricator via cfe-commits]

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

LGTM. Thanks for fixing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107646

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

[PATCH] D106262: [clang][analyzer] Use generic note tag in alpha.unix.Stream .

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

Really I still not understand why the previous `BugType` dependent `NoteTag` 
functions were bad design (except that it could make the code difficult to 
understand). If we would have the BugType available in the NoteTag, we could 
make the decision about what to display and no "or"s are needed in the message. 
We do not need a "Stream either reaches end-of-file, or fails and has its file 
position indicator left indeterminate and the error flag set." message if the 
information is available about what the exact problem was (from the BugType) 
and we can build a "Stream reaches end-of-file." if the bug was end-of-file 
related, and so on. (Really instead of the bug type other information could be 
used that is passed from the bug report to the note tag, but there is no way 
for this to do?) Otherwise just the user has to find this out the same thing by 
looking at later functions and notes. So I want to wait for the opinion of 
another reviewer(s) before proceeding.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106262

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

[PATCH] D107292: [clang] adds warning to alert user when they use alternative tokens in declarations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D107292#2923261 , @cjdb wrote:

> Patch 2: expressions
>
> xor {}
> bitand x // warning will suggest std::addressof in C++ land
> and label

An additional expression to cover, not that I think anyone would be this awful 
by accident, is: `foo->compl Foo(); // Pseudo-destructor call`

One question I have about both declarations and expressions are whether we have 
an appetite to diagnose overloaded operators or not. Personally, I think it'd 
be reasonable to diagnose something like `foo->operator bitand();` or `operator 
not_eq(A, B);` as expressions, but not reasonable to diagnose the declaration 
of the overloaded operators using alternative tokens.




Comment at: clang/lib/Parse/ParseDecl.cpp:5830-5832
+constexpr int Caret = 3;
+constexpr int Empty = 2;
+constexpr int BlockPointers = 2;

We typically either use comments for one-off uses, or we hoist this sort of 
thing to an enumeration associated with the diagnostic if we think the 
diagnostic will be used in enough places to warrant it. I think we should 
probably stick to comments for this one (e.g., `Diag(Loc, diag::whatever) << 
/*frobble*/1;`, but if you think a hoisted enum is a better approach I won't 
argue it.

(Same comment applies elsewhere in the patch.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107292

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

[PATCH] D107873: [WIP][clang-tidy] adds a const-qualified parameter check

[Julien Marrec via Phabricator via cfe-commits]

jmarrec added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-const-parameter-value-or-ref.rst:90
+Passing a ``shared_ptr`` by reference-to-``const`` is also acceptable in 
certain
+situaitons. As such, ``std::shared_ptr`` and ``boost::shared_ptr`` will
+not warn in either case by default.





Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-const-parameter-value-or-ref.rst:101
+
+The default value for ``SmallMaxSize`` was determined by through benchmarking
+when passing by value was no longer competetive with passing by reference for





Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-const-parameter-value-or-ref.rst:104
+various `boards `_. The benchmark can be found on
+`Compiler Explorer `_, with the used to inform
+the threshold.

typo here, not sure what the intent was.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107873

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

[PATCH] D107900: Add a new clang-tidy check for trivially copyable types passed by const reference

[Julien Marrec via Phabricator via cfe-commits]

jmarrec added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst:21
+
+   If set to `true`, this check will limit itself to the `builtinType()` 
types. Default is `false`.

Quuxplusone wrote:
> D107873 is related.
> 
> I'd like to see some tests/discussion around large types, e.g.
> ```
> struct Widget { int a[1000]; };
> void f(const Widget&);
> ```
> (I think D107873 at least makes a conscious attempt to get the "would it be 
> passed in registers?" check right.)
> 
> I'd like to see some tests/discussion around generic code, e.g.
> ```
> template
> struct Max {
> static T max(const T& a, const T& b);
> };
> int x = Max::max(1, 2);  // passes `int` by const reference, but this is 
> fine
> ```
Should I close this one in favor of https://reviews.llvm.org/D107873?


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

https://reviews.llvm.org/D107900

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

[PATCH] D107907: [clang-format] handle trailing comments in function definition detection

[Krasimir Georgiev via Phabricator via cfe-commits]

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

A follow-up to
https://github.com/llvm/llvm-project/commit/f6bc614546e169bb1b17a29c422ebace038e6c62
where we handle the case where the semicolon is followed by a trailing
comment.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107907

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8252,6 +8252,9 @@
   verifyFormat("Tttt ppp\n"
"ABSL_GUARDED_BY(mutex) = {};",
getGoogleStyleWithColumns(40));
+  verifyFormat("Tttt ppp\n"
+   "ABSL_GUARDED_BY(mutex);  // comment",
+   getGoogleStyleWithColumns(40));
 
   Style = getGNUStyle();
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2481,9 +2481,13 @@
   //   {
   // return i + 1;
   //   }
-  if (Next->Next && Next->Next->is(tok::identifier) &&
-  Line.Last->isNot(tok::semi))
-return true;
+  if (Next->Next && Next->Next->is(tok::identifier)) {
+const FormatToken *Last = Line.Last;
+if (Last && Last->is(tok::comment))
+  Last = Last->getPreviousNonComment();
+if (Last && Last->isNot(tok::semi))
+  return true;
+  }
   for (const FormatToken *Tok = Next->Next; Tok && Tok != Next->MatchingParen;
Tok = Tok->Next) {
 if (Tok->is(TT_TypeDeclarationParen))


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8252,6 +8252,9 @@
   verifyFormat("Tttt ppp\n"
"ABSL_GUARDED_BY(mutex) = {};",
getGoogleStyleWithColumns(40));
+  verifyFormat("Tttt ppp\n"
+   "ABSL_GUARDED_BY(mutex);  // comment",
+   getGoogleStyleWithColumns(40));
 
   Style = getGNUStyle();
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2481,9 +2481,13 @@
   //   {
   // return i + 1;
   //   }
-  if (Next->Next && Next->Next->is(tok::identifier) &&
-  Line.Last->isNot(tok::semi))
-return true;
+  if (Next->Next && Next->Next->is(tok::identifier)) {
+const FormatToken *Last = Line.Last;
+if (Last && Last->is(tok::comment))
+  Last = Last->getPreviousNonComment();
+if (Last && Last->isNot(tok::semi))
+  return true;
+  }
   for (const FormatToken *Tok = Next->Next; Tok && Tok != Next->MatchingParen;
Tok = Tok->Next) {
 if (Tok->is(TT_TypeDeclarationParen))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107646: [PowerPC] Fix the frame addresss computing return address for `__builtin_return_address`

[Victor Huang via Phabricator via cfe-commits]

NeHuang updated this revision to Diff 365761.
NeHuang added a comment.

Address review comment from Nemanja.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107646

Files:
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/test/CodeGen/PowerPC/2010-05-03-retaddr1.ll
  llvm/test/CodeGen/PowerPC/retaddr_multi_levels.ll

Index: llvm/test/CodeGen/PowerPC/retaddr_multi_levels.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/retaddr_multi_levels.ll
@@ -0,0 +1,140 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs < %s -mtriple=powerpc64le-unknown-linux \
+; RUN:   -mcpu=pwr8 | FileCheck %s -check-prefix=CHECK-64B-LE
+; RUN: llc -verify-machineinstrs < %s -mtriple=powerpc64-unknown-linux \
+; RUN:   -mcpu=pwr7 | FileCheck %s -check-prefix=CHECK-64B-BE
+; RUN: llc -verify-machineinstrs < %s -mtriple=powerpc64-unknown-aix \
+; RUN:   -mcpu=pwr7 | FileCheck %s -check-prefix=CHECK-64B-BE
+; RUN: llc -verify-machineinstrs < %s -mtriple=powerpc-unknown-aix \
+; RUN:   -mcpu=pwr7 | FileCheck %s -check-prefix=CHECK-32B-BE
+
+declare i8* @llvm.returnaddress(i32) nounwind readnone
+
+define i8* @test0() nounwind readnone {
+; CHECK-64B-LE-LABEL: test0:
+; CHECK-64B-LE:   # %bb.0: # %entry
+; CHECK-64B-LE-NEXT:mflr 0
+; CHECK-64B-LE-NEXT:std 0, 16(1)
+; CHECK-64B-LE-NEXT:stdu 1, -32(1)
+; CHECK-64B-LE-NEXT:ld 3, 48(1)
+; CHECK-64B-LE-NEXT:addi 1, 1, 32
+; CHECK-64B-LE-NEXT:ld 0, 16(1)
+; CHECK-64B-LE-NEXT:mtlr 0
+; CHECK-64B-LE-NEXT:blr
+;
+; CHECK-64B-BE-LABEL: test0:
+; CHECK-64B-BE:   # %bb.0: # %entry
+; CHECK-64B-BE-NEXT:mflr 0
+; CHECK-64B-BE-NEXT:std 0, 16(1)
+; CHECK-64B-BE-NEXT:stdu 1, -48(1)
+; CHECK-64B-BE-NEXT:ld 3, 64(1)
+; CHECK-64B-BE-NEXT:addi 1, 1, 48
+; CHECK-64B-BE-NEXT:ld 0, 16(1)
+; CHECK-64B-BE-NEXT:mtlr 0
+; CHECK-64B-BE-NEXT:blr
+;
+; CHECK-32B-BE-LABEL: test0:
+; CHECK-32B-BE:   # %bb.0: # %entry
+; CHECK-32B-BE-NEXT:mflr 0
+; CHECK-32B-BE-NEXT:stw 0, 8(1)
+; CHECK-32B-BE-NEXT:stwu 1, -32(1)
+; CHECK-32B-BE-NEXT:lwz 3, 40(1)
+; CHECK-32B-BE-NEXT:addi 1, 1, 32
+; CHECK-32B-BE-NEXT:lwz 0, 8(1)
+; CHECK-32B-BE-NEXT:mtlr 0
+; CHECK-32B-BE-NEXT:blr
+entry:
+  %0 = tail call i8* @llvm.returnaddress(i32 0);
+  ret i8* %0
+}
+
+define i8* @test1() nounwind readnone {
+; CHECK-64B-LE-LABEL: test1:
+; CHECK-64B-LE:   # %bb.0: # %entry
+; CHECK-64B-LE-NEXT:mflr 0
+; CHECK-64B-LE-NEXT:std 0, 16(1)
+; CHECK-64B-LE-NEXT:stdu 1, -32(1)
+; CHECK-64B-LE-NEXT:ld 3, 0(1)
+; CHECK-64B-LE-NEXT:ld 3, 0(3)
+; CHECK-64B-LE-NEXT:ld 3, 16(3)
+; CHECK-64B-LE-NEXT:addi 1, 1, 32
+; CHECK-64B-LE-NEXT:ld 0, 16(1)
+; CHECK-64B-LE-NEXT:mtlr 0
+; CHECK-64B-LE-NEXT:blr
+;
+; CHECK-64B-BE-LABEL: test1:
+; CHECK-64B-BE:   # %bb.0: # %entry
+; CHECK-64B-BE-NEXT:mflr 0
+; CHECK-64B-BE-NEXT:std 0, 16(1)
+; CHECK-64B-BE-NEXT:stdu 1, -48(1)
+; CHECK-64B-BE-NEXT:ld 3, 0(1)
+; CHECK-64B-BE-NEXT:ld 3, 0(3)
+; CHECK-64B-BE-NEXT:ld 3, 16(3)
+; CHECK-64B-BE-NEXT:addi 1, 1, 48
+; CHECK-64B-BE-NEXT:ld 0, 16(1)
+; CHECK-64B-BE-NEXT:mtlr 0
+; CHECK-64B-BE-NEXT:blr
+;
+; CHECK-32B-BE-LABEL: test1:
+; CHECK-32B-BE:   # %bb.0: # %entry
+; CHECK-32B-BE-NEXT:mflr 0
+; CHECK-32B-BE-NEXT:stw 0, 8(1)
+; CHECK-32B-BE-NEXT:stwu 1, -32(1)
+; CHECK-32B-BE-NEXT:lwz 3, 0(1)
+; CHECK-32B-BE-NEXT:lwz 3, 0(3)
+; CHECK-32B-BE-NEXT:lwz 3, 8(3)
+; CHECK-32B-BE-NEXT:addi 1, 1, 32
+; CHECK-32B-BE-NEXT:lwz 0, 8(1)
+; CHECK-32B-BE-NEXT:mtlr 0
+; CHECK-32B-BE-NEXT:blr
+entry:
+  %0 = tail call i8* @llvm.returnaddress(i32 1);
+  ret i8* %0
+}
+
+define i8* @test2() nounwind readnone {
+; CHECK-64B-LE-LABEL: test2:
+; CHECK-64B-LE:   # %bb.0: # %entry
+; CHECK-64B-LE-NEXT:mflr 0
+; CHECK-64B-LE-NEXT:std 0, 16(1)
+; CHECK-64B-LE-NEXT:stdu 1, -32(1)
+; CHECK-64B-LE-NEXT:ld 3, 0(1)
+; CHECK-64B-LE-NEXT:ld 3, 0(3)
+; CHECK-64B-LE-NEXT:ld 3, 0(3)
+; CHECK-64B-LE-NEXT:ld 3, 16(3)
+; CHECK-64B-LE-NEXT:addi 1, 1, 32
+; CHECK-64B-LE-NEXT:ld 0, 16(1)
+; CHECK-64B-LE-NEXT:mtlr 0
+; CHECK-64B-LE-NEXT:blr
+;
+; CHECK-64B-BE-LABEL: test2:
+; CHECK-64B-BE:   # %bb.0: # %entry
+; CHECK-64B-BE-NEXT:mflr 0
+; CHECK-64B-BE-NEXT:std 0, 16(1)
+; CHECK-64B-BE-NEXT:stdu 1, -48(1)
+; CHECK-64B-BE-NEXT:ld 3, 0(1)
+; CHECK-64B-BE-NEXT:ld 3, 0(3)
+; CHECK-64B-BE-NEXT:ld 3, 0(3)
+; CHECK-64B-BE-NEXT:ld 3, 16(3)
+; CHECK-64B-BE-NEXT:addi 1, 1, 48
+; CHECK-64B-BE-NEXT:ld 0, 16(1)
+; CHECK-64B-BE-NEXT:mtlr 0
+; CHECK-64B-BE-NEXT:blr
+;
+; CHECK-32B-BE-LABEL: test2:
+; CHECK-32B-BE:   # %bb.0: # %entry
+; CHECK-32B-BE-NEXT:mflr 0
+; CHECK-32B-BE-NEXT:

[PATCH] D107900: Add a new clang-tidy check for trivially copyable types passed by const reference

[Julien Marrec via Phabricator via cfe-commits]

jmarrec added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/misc-pod-const-ref-to-value.cpp:3-5
+int f1(const int );
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'i' is a trivially 
copyable type and should not be passed by const-reference but by value 
[misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f1(int i);

This is now failing after I applied clang-format.  `int f1(const int& i)` would 
correctly produce `int f1(int i);`, now it does `int f1(inti);`... I need to 
find a way to fix that.


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

https://reviews.llvm.org/D107900

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

[PATCH] D105264: [X86] AVX512FP16 instructions enabling 2/6

[LuoYuanke via Phabricator via cfe-commits]

LuoYuanke added inline comments.



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:3197
+  else if (PatchedName.endswith("sh"))
+PatchedName = IsVCMP ? "vcmpsh" : "cmpsh";
+  else if (PatchedName.endswith("ph"))

There is no cmpsh?



Comment at: llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp:3199
+  else if (PatchedName.endswith("ph"))
+PatchedName = IsVCMP ? "vcmpph" : "cmpph";
   else

We only have vcmpph?



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:1873
   setOperationAction(ISD::SETCC,  VT, Custom);
+  setOperationAction(ISD::STRICT_FSETCC,  VT, Custom);
+  setOperationAction(ISD::STRICT_FSETCCS, VT, Custom);

Is this related to FP16?



Comment at: llvm/lib/Target/X86/X86InstrAVX512.td:2674
+let Predicates = [HasFP16] in {
+  def : Pat<(v1i1 (X86cmpms(loadf16 addr:$src2), FR16X:$src1,
+  CommutableCMPCC : $cc)),

X86cmpms (



Comment at: llvm/lib/Target/X86/X86InstrAVX512.td:2675
+  def : Pat<(v1i1 (X86cmpms(loadf16 addr:$src2), FR16X:$src1,
+  CommutableCMPCC : $cc)),
+   (VCMPSHZrm FR16X:$src1, addr:$src2, imm:$cc)>;

CommutableCMPCC:$cc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105264

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

[PATCH] D107841: CodeGen: No need to check for isExternC if HasStrictReturn is already false

[Arnold Schwaighofer 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 rG9eb99d2e73b5: CodeGen: No need to check for isExternC if 
HasStrictReturn is already false (authored by aschwaighofer).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107841

Files:
  clang/lib/CodeGen/CGCall.cpp


Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2241,7 +2241,7 @@
   // C++ explicitly makes returning undefined values UB. C's rule only applies
   // to used values, so we never mark them noundef for now.
   bool HasStrictReturn = getLangOpts().CPlusPlus;
-  if (TargetDecl) {
+  if (TargetDecl && HasStrictReturn) {
 if (const FunctionDecl *FDecl = dyn_cast(TargetDecl))
   HasStrictReturn &= !FDecl->isExternC();
 else if (const VarDecl *VDecl = dyn_cast(TargetDecl))


Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -2241,7 +2241,7 @@
   // C++ explicitly makes returning undefined values UB. C's rule only applies
   // to used values, so we never mark them noundef for now.
   bool HasStrictReturn = getLangOpts().CPlusPlus;
-  if (TargetDecl) {
+  if (TargetDecl && HasStrictReturn) {
 if (const FunctionDecl *FDecl = dyn_cast(TargetDecl))
   HasStrictReturn &= !FDecl->isExternC();
 else if (const VarDecl *VDecl = dyn_cast(TargetDecl))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 9eb99d2 - CodeGen: No need to check for isExternC if HasStrictReturn is already false

[Arnold Schwaighofer via cfe-commits]

Author: Arnold Schwaighofer
Date: 2021-08-11T07:42:48-07:00
New Revision: 9eb99d2e73b5598076fbdd8abceb3549afa8f0ae

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

LOG: CodeGen: No need to check for isExternC if HasStrictReturn is already false

NFC intended.

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

Added: 


Modified: 
clang/lib/CodeGen/CGCall.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 1296dfa18b9a5..43be6755a0745 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -2241,7 +2241,7 @@ void CodeGenModule::ConstructAttributeList(StringRef Name,
   // C++ explicitly makes returning undefined values UB. C's rule only applies
   // to used values, so we never mark them noundef for now.
   bool HasStrictReturn = getLangOpts().CPlusPlus;
-  if (TargetDecl) {
+  if (TargetDecl && HasStrictReturn) {
 if (const FunctionDecl *FDecl = dyn_cast(TargetDecl))
   HasStrictReturn &= !FDecl->isExternC();
 else if (const VarDecl *VDecl = dyn_cast(TargetDecl))



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

[PATCH] D97753: [clang-tidy] Add a check for enforcing minimum length for variable names

[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 aside from some style nits (removing some top-level `const` we don't 
typically use, unnecessary paren expressions, and a typo fix), thank you for 
the check!




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp:99
+
+const StringRef VarName = StandaloneVar->getName();
+





Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp:101
+
+if ((VarName.size() >= MinimumVariableNameLength) ||
+IgnoredVariableNames.match(VarName))





Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp:115-116
+
+const StringRef VarName = ExceptionVarName->getName();
+if ((VarName.size() >= MinimumExceptionNameLength) ||
+IgnoredExceptionVariableNames.match(VarName))





Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp:129
+
+const StringRef VarName = LoopVar->getName();
+





Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierLengthCheck.cpp:144
+
+const StringRef VarName = ParamVar->getName();
+





Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability-identifier-length.rst:81
+The default value is `^[ijk_]$`; the first three symbols for historical
+reasons and the last one since it is frequently used as a "dont't care"
+value, specifically in tools such as Google Benchmark.




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

https://reviews.llvm.org/D97753

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

[PATCH] D107668: [OpenMP]Fix PR50336: Remove temporary files in the offload bundler tool

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D107668#2939365 , @yaxunl wrote:

> This may break -save-temps since the input to clang-offload-bundler may not 
> be temporary files when -save-temps is enabled.
>
> I think clang-offload-bundler is not the right place to decide whether a file 
> is a temporary file. Whether a file is a temporary file should be determined 
> at its point of creation and if it is a temporary file it should be 
> addTempFile there, instead of guessing that later.

The `.cubin` files are still present when I tested it with `save-temps`. We 
already do something similar in the OpenMPLinker job for the Cuda driver, which 
is why these files were removed for straight compilation but kept when using 
the offload bundler when compiling with `-c`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107668

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

[PATCH] D107899: [PowerPC] Implement builtin for vbpermd

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added inline comments.



Comment at: clang/lib/Headers/altivec.h:17337
 static __inline__ vector long long __ATTRS_o_ai
 vec_vbpermq(vector unsigned char __a, vector unsigned char __b) {
   return __builtin_altivec_vbpermq(__a, __b);

This should be guarded under P8. It would also be good to add a 
`vec_vbpermd(vector unsigned long long ...)` counter part under 
`__POWER9_VECTOR__` for consistency.



Comment at: clang/lib/Headers/altivec.h:17349
+static __inline__ vector unsigned char __ATTRS_o_ai
+vec_bperm(vector unsigned char __a, vector unsigned char __b) {
+  return __builtin_altivec_vbpermq(__a, __b);

`vbpermq` variants should be guarded under `__POWER8_VECTOR__`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107899

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

[PATCH] D107668: [OpenMP]Fix PR50336: Remove temporary files in the offload bundler tool

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

This may break -save-temps since the input to clang-offload-bundler may not be 
temporary files when -save-temps is enabled.

I think clang-offload-bundler is not the right place to decide whether a file 
is a temporary file. Whether a file is a temporary file should be determined at 
its point of creation and if it is a temporary file it should be addTempFile 
there, instead of guessing that later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107668

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

[PATCH] D105264: [X86] AVX512FP16 instructions enabling 2/6

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added a comment.

Thanks for the review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105264

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

[PATCH] D105264: [X86] AVX512FP16 instructions enabling 2/6

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: clang/include/clang/Basic/BuiltinsX86.def:1860
+TARGET_BUILTIN(__builtin_ia32_minph512,  "V32xV32xV32xIi", "ncV:512:", 
"avx512fp16")
+
+TARGET_BUILTIN(__builtin_ia32_minph256,  "V16xV16xV16x", "ncV:256:", 
"avx512fp16,avx512vl")

LuoYuanke wrote:
> Why there is no 256 and 128 version for addph, subph, mulph, divph?
Because they don't support rounding control, thus we can directly use 
instructions like fadd, fsub etc.



Comment at: clang/lib/Headers/avx512fp16intrin.h:312
+   __m128h B) {
+  return __builtin_ia32_vcomish((__v8hf)A, (__v8hf)B, _CMP_NEQ_US,
+_MM_FROUND_CUR_DIRECTION);

LuoYuanke wrote:
> _CMP_NEQ_OS?
`_CMP_NEQ_US` is correct, see the operation in intrinsic guide (we should 
update for sh):
https://software.intel.com/sites/landingpage/IntrinsicsGuide/#text=_mm_comineq_ss=1365,1366



Comment at: clang/lib/Headers/avx512fp16intrin.h:318
+   __m128h B) {
+  return __builtin_ia32_vcomish((__v8hf)A, (__v8hf)B, _CMP_EQ_OQ,
+_MM_FROUND_CUR_DIRECTION);

LuoYuanke wrote:
> Why it is OQ not UQ? Ditto for all other ucomi intrinsics.
OQ requires both inputs are non-nan. This is the same as the intrinsic's 
behavior:
https://software.intel.com/sites/landingpage/IntrinsicsGuide/#text=_mm_ucomieq_ss=1354,7232

The "u" in intrinsic name corresponds to not "U" but "Q" in the macro name.



Comment at: clang/lib/Headers/avx512fp16intrin.h:516
+  return (__m512h)__builtin_ia32_maxph512((__v32hf)__A, (__v32hf)__B,
+  _MM_FROUND_CUR_DIRECTION);
+}

LuoYuanke wrote:
> Why there is rounding control for max/min operation?
They still need to control the exception.



Comment at: clang/lib/Headers/avx512fp16intrin.h:669
+  __A = _mm_div_sh(__A, __B);
+  return __builtin_ia32_selectsh_128(__U, __A, __W);
+}

LuoYuanke wrote:
> Will it be combined to one instruction? If __B[0] is 0, and mask[0] is 0, 
> there is no exception? 
We should consider this case, but only under strict FP mode. We have ss/sd 
defined in this way too. We should fix them in future.



Comment at: clang/lib/Headers/avx512fp16intrin.h:698
+  (__v8hf)__A, (__v8hf)__B, (__v8hf)_mm_setzero_ph(), (__mmask8)-1,
+  _MM_FROUND_CUR_DIRECTION);
+}

LuoYuanke wrote:
> Do we have rounding control for min?
No. But we need control exception.



Comment at: clang/lib/Headers/avx512fp16intrin.h:757
+
+#define _mm_max_round_sh(A, B, R)  
\
+  (__m128h) __builtin_ia32_maxsh_round_mask(   
\

LuoYuanke wrote:
> This name may be misleading, it means suppress exception. Right?
Yes. But we are widely using it for SAE in existing code :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105264

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

[PATCH] D105264: [X86] AVX512FP16 instructions enabling 2/6

[Pengfei Wang via Phabricator via cfe-commits]

pengfei updated this revision to Diff 365738.
pengfei marked 6 inline comments as done.
pengfei added a comment.

1. Rebase to the first merged FP16 patch.
2. Address Yuanke's comments.
3. Add parentheses around casts.
4. Remove OptForSize predicate for vmovsh.
5. Add more immediate value for encoding tests of vcmpph/sh.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105264

Files:
  clang/include/clang/Basic/BuiltinsX86.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Headers/avx512fp16intrin.h
  clang/lib/Headers/avx512vlfp16intrin.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/X86/avx512fp16-builtins.c
  clang/test/CodeGen/X86/avx512vlfp16-builtins.c
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/MCTargetDesc/X86ATTInstPrinter.cpp
  llvm/lib/Target/X86/MCTargetDesc/X86InstPrinterCommon.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86InstrAVX512.td
  llvm/lib/Target/X86/X86InstrFoldTables.cpp
  llvm/lib/Target/X86/X86InstrInfo.cpp
  llvm/lib/Target/X86/X86IntrinsicsInfo.h
  llvm/test/CodeGen/X86/avx512fp16-arith-intrinsics.ll
  llvm/test/CodeGen/X86/avx512fp16-arith-vl-intrinsics.ll
  llvm/test/CodeGen/X86/avx512fp16-arith.ll
  llvm/test/CodeGen/X86/avx512fp16-fmaxnum.ll
  llvm/test/CodeGen/X86/avx512fp16-fminnum.ll
  llvm/test/CodeGen/X86/avx512fp16-fold-load-binops.ll
  llvm/test/CodeGen/X86/avx512fp16-fold-xmm-zero.ll
  llvm/test/CodeGen/X86/avx512fp16-fp-logic.ll
  llvm/test/CodeGen/X86/avx512fp16-intrinsics.ll
  llvm/test/CodeGen/X86/avx512fp16-machine-combiner.ll
  llvm/test/CodeGen/X86/avx512fp16-mov.ll
  llvm/test/CodeGen/X86/avx512fp16-unsafe-fp-math.ll
  llvm/test/CodeGen/X86/fp-strict-scalar-cmp-fp16.ll
  llvm/test/CodeGen/X86/fp-strict-scalar-fp16.ll
  llvm/test/CodeGen/X86/pseudo_cmov_lower-fp16.ll
  llvm/test/CodeGen/X86/stack-folding-fp-avx512fp16.ll
  llvm/test/CodeGen/X86/stack-folding-fp-avx512fp16vl.ll
  llvm/test/CodeGen/X86/vec-strict-128-fp16.ll
  llvm/test/CodeGen/X86/vec-strict-256-fp16.ll
  llvm/test/CodeGen/X86/vec-strict-512-fp16.ll
  llvm/test/CodeGen/X86/vec-strict-cmp-128-fp16.ll
  llvm/test/CodeGen/X86/vec-strict-cmp-256-fp16.ll
  llvm/test/CodeGen/X86/vec-strict-cmp-512-fp16.ll
  llvm/test/CodeGen/X86/vector-reduce-fmax-nnan.ll
  llvm/test/CodeGen/X86/vector-reduce-fmin-nnan.ll
  llvm/test/MC/Disassembler/X86/avx512fp16.txt
  llvm/test/MC/Disassembler/X86/avx512fp16vl.txt
  llvm/test/MC/X86/avx512fp16.s
  llvm/test/MC/X86/avx512fp16vl.s
  llvm/test/MC/X86/intel-syntax-avx512fp16.s
  llvm/test/MC/X86/intel-syntax-avx512fp16vl.s

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

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D69764#2938973 , @MyDeveloperDay 
wrote:

> With the introduction of `CVQualifierOrder` the need for 
> `CVQualifierAlignment:` doesn't make so much sense
>
>   CVQualifierAlignment: Left
>   CVQualifierOrder: [ "inline", "static", "volatile", "const" ]
>
> I'm leaning toward introducing  into the `CVQualifierOrder` allowing 
> for some qualifiers to be Left of the type and some to be Right (this is not 
> dissimilar to what was discussed some time ago, and similar to the concept of 
> the resharper image above)
>
> So we would be able to have:
>
>   CVQualifierOrder: [ "inline", "static", "volatile", "", "const", 
> "restrict" ]
>
> By doing so `inline,static,and volatile` are Left and `const,restrict` are 
> Right and so a single setting of Left and Right isn't applicable.
>
> The whole feature could then be determined ONLY using the `CVQualifierOrder` 
> however it doesn't feel like there is a good way to say "Don't change order" 
> (unless of course I use an empty `CVQualifierOrder` list), but this doesn't 
> feel very off by default enough, I like the explicitness of `Leave`.
>
> My gut feeling is to make `Left` and `Right` options define a predetermined 
> CVQualifierOrder, and introduce `Custom` as a new CVQualifierAlignment 
> setting which would require you to define the Order, this has the advantage 
> of driving a sort of semi standard as to what we mean by `Left` and `Right`
>
> Left would by default define:
>
>   CVQualifierOrder: [ "inline", "static", "volatile", "const", "restrict", 
> ""]
>
> Right would define:
>
>   CVQualifierOrder: [ "inline", "static", "", "volatile", "const", 
> "restrict" ]
>
> (we might want to discuss what these defaults should be, I'm not a language 
> lawyer!)
>
> I realise "inline" and "static" are to the left in the Right alignment, but 
> I'm not sure them being to the right is even legal (its certainly not common) 
> or what is most likely wanted.
>
> I also think if a `CVQualifierOrder` is defined and the 
> `CVQualifierAlignment` is not `Custom` that should be a configuration error 
> to ensure there isn't any ambiguity
>
> Does anyone have any thoughts on this approach?

I think the general idea (letting the user specify the order of elements 
relative to one another rather than trying to come up with individual controls 
for each qualifier), I think that'll be easier for people to configure. I do 
worry about calling it "qualifier order" though as some of the things are 
definitely not qualifiers (like `static` or `inline` which are specifiers). A 
few questions about using an ordered list like this are: 0) should we diagnose 
typos (e.g., the user writes `inlnie` instead of `inline`)?, 1) should we 
diagnose duplications (e.g., the user lists `friend` twice)?

Btw, 
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Sema/DeclSpec.h#L229
 has quite a bit of information about the kinds of qualifiers and specifiers 
Clang deals with at the parsing stage, in case you were looking for a list of 
things to care about.

> Oh man you are killing me with good suggestions ;-)
> So this will be hard with the current implementation because [ [ noreturn ] ] 
> are all separate tokens. However maybe if we COULD merge the tokens into 1 
> token (like we do in FormatTokenLexer) even it it was just temporarily for 
> this pass we maybe could do that.

I'd like to second the request for attribute support, even if it's follow-up 
work. Things like address space attributes, GC attributes, `__unaligned`, etc 
are all type qualifiers (I think we have a list of the attributes which are 
qualifiers in Type.h), so it's a natural extension. However, attributes in 
general are somewhat tricky because the attribute style (`__attribute__` vs 
`[[]]`) can be syntactically location sensitive. e.g., `[[noreturn]] void 
func()` means something different from `void [[noreturn]]` func()` and we have 
some attributes that can appertain to a declaration or a type (so the 
positioning can have silent semantic changes).

> I'm in no rush, I want to let the RFC have a good amount time for people to 
> read and to gather all the feedback, but I'm encouraged enough to think my 
> efforts won't be wasted and I want to address some of @aaron.ballman concerns 
> to extend a olive branch in the hope that I could at least partially gain 
> approval.

Not being in a rush sounds like a great plan! FWIW, I think we're moving 
steadily towards approval. I mentioned it on the RFC thread, but I'll mention 
it again here: thank you for starting the RFC process to have the broader 
discussion!


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

https://reviews.llvm.org/D69764

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

[PATCH] D107900: Add a new clang-tidy check for trivially copyable types passed by const reference

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

Quuxplusone added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst:21
+
+   If set to `true`, this check will limit itself to the `builtinType()` 
types. Default is `false`.

D107873 is related.

I'd like to see some tests/discussion around large types, e.g.
```
struct Widget { int a[1000]; };
void f(const Widget&);
```
(I think D107873 at least makes a conscious attempt to get the "would it be 
passed in registers?" check right.)

I'd like to see some tests/discussion around generic code, e.g.
```
template
struct Max {
static T max(const T& a, const T& b);
};
int x = Max::max(1, 2);  // passes `int` by const reference, but this is 
fine
```


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

https://reviews.llvm.org/D107900

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

[PATCH] D107366: [analyzer] Adjust JS code of analyzer's HTML report for IE support.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

Is it ready to load now?


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

https://reviews.llvm.org/D107366

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

[PATCH] D106960: [OffloadArch] Library to query properties of current offload archicture

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

T




Comment at: llvm/lib/OffloadArch/amdgpu/vendor_specific_capabilities.cpp:25
+//
+#include "hsa-subset.h"
+#include 

yaxunl wrote:
> It would be much simpler to use HIP API to get device name and capabilities 
> e.g. gfx906:xnack+:sramecc-
> 
> https://github.com/ROCm-Developer-Tools/HIP/blob/rocm-4.2.x/samples/1_Utils/hipInfo/hipInfo.cpp
> 
> It will work on both Linux and Windows. On Linux the availability of HIP 
> runtime is the same as HSA runtime. On Windows HIP runtime is shipped with 
> display driver, whereas HSA runtime is not available.
> On Linux the availability of HIP runtime is the same as HSA runtime

This is probably not true. If ROCm is installed somewhere, both HIP and HSA 
runtimes are available. If building from source, HSA is much quicker and easier 
to build than the HIP runtimes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106960

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

[PATCH] D107900: Add a new clang-tidy check for trivially copyable types passed by const reference

[Julien Marrec via Phabricator via cfe-commits]

jmarrec updated this revision to Diff 365735.
jmarrec added a comment.

Added content to the 
`clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst` file


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

https://reviews.llvm.org/D107900

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/PodConstRefToValueCheck.cpp
  clang-tools-extra/clang-tidy/misc/PodConstRefToValueCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-pod-const-ref-to-value.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-pod-const-ref-to-value.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-pod-const-ref-to-value.cpp
@@ -0,0 +1,55 @@
+// RUN: %check_clang_tidy %s misc-pod-const-ref-to-value %t
+
+int f1(const int );
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'i' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f1(int i);
+int f2(const int , double d);
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'i' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f2(int i, double d);
+
+int f3(double d, const int );
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: argument 'i' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f3(double d, int i);
+
+int f4(const double , const double );
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'd' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-MESSAGES: :[[@LINE-2]]:25: warning: argument 'd2' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f4(double d, double d2);
+
+namespace n1 {
+class A {
+  static int g(const double );
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: argument 'd' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+  // CHECK-FIXES: static int g(double d);
+};
+
+int A::g(const double ) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: argument 'd' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+  // CHECK-FIXES: int A::g(double d) {
+  return static_cast(d);
+}
+} // namespace n1
+
+// Not triggering the check
+
+int f5(int i);
+int f6(int i, double d);
+int f7(int ); // Might be used for return...
+
+namespace n2 {
+class A {
+  static int g(double d);
+};
+
+int A::g(double d) {
+  return static_cast(d);
+}
+
+class B {
+  static int g(double );
+};
+
+int B::g(double ) {
+  return static_cast(d);
+}
+} // namespace n2
Index: clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - misc-pod-const-ref-to-value
+
+misc-pod-const-ref-to-value
+===
+
+This check detects when ``trivially_copyable`` types are passed by const-reference
+to a function and changes that to by value
+
+For example in the following code, it is replaced by ``void f(int i, double d)``
+
+.. code-block:: c++
+
+  void f(const int& i, const double& d);
+
+
+Options
+---
+
+.. option:: RestrictToBuiltInTypes
+
+   If set to `true`, this check will limit itself to the `builtinType()` types. Default is `false`.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -212,6 +212,7 @@
`misc-no-recursion `_,
`misc-non-copyable-objects `_,
`misc-non-private-member-variables-in-classes `_,
+   `misc-pod-const-ref-to-value `_, "Yes"
`misc-redundant-expression `_, "Yes"
`misc-static-assert `_, "Yes"
`misc-throw-by-value-catch-by-reference `_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -72,6 +72,12 @@
 New checks
 ^^
 
+- New :doc:`misc-pod-const-ref-to-value
+  ` check.
+
+  Finds trivially_copyable types are passed by const-reference to a function
+  and changes that to by value
+
 New check aliases
 ^
 
Index: 

[PATCH] D107900: Add a new clang-tidy check for trivially copyable types passed by const reference

[Julien Marrec via Phabricator via cfe-commits]

jmarrec created this revision.
jmarrec added a reviewer: steveire.
Herald added a subscriber: mgorny.
jmarrec requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107900

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/PodConstRefToValueCheck.cpp
  clang-tools-extra/clang-tidy/misc/PodConstRefToValueCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-pod-const-ref-to-value.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc-pod-const-ref-to-value.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-pod-const-ref-to-value.cpp
@@ -0,0 +1,55 @@
+// RUN: %check_clang_tidy %s misc-pod-const-ref-to-value %t
+
+int f1(const int );
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'i' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f1(int i);
+int f2(const int , double d);
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'i' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f2(int i, double d);
+
+int f3(double d, const int );
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: argument 'i' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f3(double d, int i);
+
+int f4(const double , const double );
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: argument 'd' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-MESSAGES: :[[@LINE-2]]:25: warning: argument 'd2' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+// CHECK-FIXES: int f4(double d, double d2);
+
+namespace n1 {
+class A {
+  static int g(const double );
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: argument 'd' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+  // CHECK-FIXES: static int g(double d);
+};
+
+int A::g(const double ) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: argument 'd' is a trivially copyable type and should not be passed by const-reference but by value [misc-pod-const-ref-to-value]
+  // CHECK-FIXES: int A::g(double d) {
+  return static_cast(d);
+}
+} // namespace n1
+
+// Not triggering the check
+
+int f5(int i);
+int f6(int i, double d);
+int f7(int ); // Might be used for return...
+
+namespace n2 {
+class A {
+  static int g(double d);
+};
+
+int A::g(double d) {
+  return static_cast(d);
+}
+
+class B {
+  static int g(double );
+};
+
+int B::g(double ) {
+  return static_cast(d);
+}
+} // namespace n2
Index: clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-pod-const-ref-to-value.rst
@@ -0,0 +1,6 @@
+.. title:: clang-tidy - misc-pod-const-ref-to-value
+
+misc-pod-const-ref-to-value
+===
+
+FIXME: Describe what patterns does the check detect and why. Give examples.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -212,6 +212,7 @@
`misc-no-recursion `_,
`misc-non-copyable-objects `_,
`misc-non-private-member-variables-in-classes `_,
+   `misc-pod-const-ref-to-value `_, "Yes"
`misc-redundant-expression `_, "Yes"
`misc-static-assert `_, "Yes"
`misc-throw-by-value-catch-by-reference `_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -72,6 +72,12 @@
 New checks
 ^^
 
+- New :doc:`misc-pod-const-ref-to-value
+  ` check.
+
+  Finds trivially_copyable types are passed by const-reference to a function
+  and changes that to by value
+
 New check aliases
 ^
 
Index: clang-tools-extra/clang-tidy/misc/PodConstRefToValueCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/PodConstRefToValueCheck.h
@@ -0,0 +1,41 @@
+//===--- PodConstRefToValueCheck.h - clang-tidy -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache 

[PATCH] D107899: [PowerPC] Implement builtin for vbpermd

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai created this revision.
nemanjai added a reviewer: PowerPC.
Herald added subscribers: shchenz, kbarton, hiraditya.
nemanjai requested review of this revision.
Herald added projects: clang, LLVM.

The instruction has similar semantics to vbpermq but for doublewords. It was 
added in P9  and the ABI documents the builtin.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107899

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Headers/altivec.h
  clang/test/CodeGen/builtins-ppc-p8vector.c
  clang/test/CodeGen/builtins-ppc-p9vector.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCInstrAltivec.td
  llvm/test/CodeGen/PowerPC/p9-vbpermd.ll

Index: llvm/test/CodeGen/PowerPC/p9-vbpermd.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/p9-vbpermd.ll
@@ -0,0 +1,32 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu \
+; RUN:  -mcpu=pwr9 < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:  -mcpu=pwr9 < %s | FileCheck %s
+
+@vull = common global <2 x i64> zeroinitializer, align 16
+@vuc = common global <16 x i8> zeroinitializer, align 16
+@res_vull = common global <2 x i64> zeroinitializer, align 16
+
+define void @test1() {
+; CHECK-LABEL: test1:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:addis 3, 2, .LC0@toc@ha
+; CHECK-NEXT:ld 3, .LC0@toc@l(3)
+; CHECK-NEXT:lxv 34, 0(3)
+; CHECK-NEXT:addis 3, 2, .LC1@toc@ha
+; CHECK-NEXT:ld 3, .LC1@toc@l(3)
+; CHECK-NEXT:lxv 35, 0(3)
+; CHECK-NEXT:addis 3, 2, .LC2@toc@ha
+; CHECK-NEXT:ld 3, .LC2@toc@l(3)
+; CHECK-NEXT:vbpermd 2, 2, 3
+; CHECK-NEXT:stxv 34, 0(3)
+; CHECK-NEXT:blr
+entry:
+  %0 = load <2 x i64>, <2 x i64>* @vull, align 16
+  %1 = load <16 x i8>, <16 x i8>* @vuc, align 16
+  %2 = call <2 x i64> @llvm.ppc.altivec.vbpermd(<2 x i64> %0, <16 x i8> %1)
+  store <2 x i64> %2, <2 x i64>* @res_vull, align 16
+  ret void
+}
+declare <2 x i64> @llvm.ppc.altivec.vbpermd(<2 x i64>, <16 x i8>)
Index: llvm/lib/Target/PowerPC/PPCInstrAltivec.td
===
--- llvm/lib/Target/PowerPC/PPCInstrAltivec.td
+++ llvm/lib/Target/PowerPC/PPCInstrAltivec.td
@@ -1518,8 +1518,8 @@
 (int_ppc_altivec_vprtybq v1i128:$vB))]>;
 
 // Vector (Bit) Permute (Right-indexed)
-def VBPERMD : VXForm_1<1484, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB),
-   "vbpermd $vD, $vA, $vB", IIC_VecFP, []>;
+def VBPERMD : VX1_Int_Ty3<1484, "vbpermd", int_ppc_altivec_vbpermd,
+  v2i64, v2i64, v16i8>;
 def VPERMR : VAForm_1a<59, (outs vrrc:$vD), (ins vrrc:$vA, vrrc:$vB, vrrc:$vC),
"vpermr $vD, $vA, $vB, $vC", IIC_VecFP, []>;
 
Index: llvm/include/llvm/IR/IntrinsicsPowerPC.td
===
--- llvm/include/llvm/IR/IntrinsicsPowerPC.td
+++ llvm/include/llvm/IR/IntrinsicsPowerPC.td
@@ -1042,6 +1042,9 @@
   def int_ppc_altivec_vbpermq : GCCBuiltin<"__builtin_altivec_vbpermq">,
   Intrinsic<[llvm_v2i64_ty], [llvm_v16i8_ty, llvm_v16i8_ty],
 [IntrNoMem]>;
+  def int_ppc_altivec_vbpermd : GCCBuiltin<"__builtin_altivec_vbpermd">,
+  Intrinsic<[llvm_v2i64_ty], [llvm_v2i64_ty, llvm_v16i8_ty],
+[IntrNoMem]>;
 }
 
 def int_ppc_altivec_vexptefp  : PowerPC_Vec_FF_Intrinsic<"vexptefp">;
Index: clang/test/CodeGen/builtins-ppc-p9vector.c
===
--- clang/test/CodeGen/builtins-ppc-p9vector.c
+++ clang/test/CodeGen/builtins-ppc-p9vector.c
@@ -1260,3 +1260,9 @@
 // CHECK-NEXT: ret <2 x i64>
 return vec_signextll(vsia);
 }
+
+vector unsigned long long test_vbpermd(void) {
+  // CHECK: @llvm.ppc.altivec.vbpermd(<2 x i64>
+  // CHECK-BE: @llvm.ppc.altivec.vbpermd(<2 x i64>
+  return vec_bperm(vula, vuca);
+}
Index: clang/test/CodeGen/builtins-ppc-p8vector.c
===
--- clang/test/CodeGen/builtins-ppc-p8vector.c
+++ clang/test/CodeGen/builtins-ppc-p8vector.c
@@ -1177,10 +1177,13 @@
 // CHECK: llvm.ppc.altivec.vgbbd
 // CHECK-LE: llvm.ppc.altivec.vgbbd
 
-  res_vull = vec_bperm(vux, vux);
-// CHECK: llvm.ppc.altivec.vbpermq
-// CHECK-LE: llvm.ppc.altivec.vbpermq
-// CHECK-PPC: warning: implicit declaration of function 'vec_bperm'
+  res_vull = vec_bperm(vux, vuc);
+  // CHECK: llvm.ppc.altivec.vbpermq
+  // CHECK-LE: llvm.ppc.altivec.vbpermq
+
+  res_vull = vec_bperm(vuc, vuc);
+  // CHECK: llvm.ppc.altivec.vbpermq
+  // CHECK-LE: llvm.ppc.altivec.vbpermq
 
   res_vsll = vec_neg(vsll);
 // CHECK: sub <2 x i64> zeroinitializer, {{%[0-9]+}}
Index: clang/lib/Headers/altivec.h

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a subscriber: asb.
MyDeveloperDay added a comment.

> In D69764#2939037 , 
> @HazardyKnusperkeks wrote:



> I would like to remove the CV, only QualifierOrder.

Oh gosh, I agree here so much with both you and @curdeius , I keep miss 
spelling the CV and it makes the code noisy... let me rework everything now to 
remove the CV including all the options,

I get @curdeius point about `Specifier` too... I'm OK about going with a 
concencus (I'm not married to the idea of just `Qualifier`)

> And please take attributes into account, C++ attributes like [[noreturn]], 
> but also compiler attributes like __attribute__((noreturn)) or 
> __declspec(noreturn), I think it should be possible to add  to 
> your list.

Oh man you are killing me with good suggestions ;-)

So this will be hard with the current implementation because `[` `[` `noreturn` 
`]` `]` are all separate tokens. However maybe if we COULD merge the tokens 
into 1 token (like we do in FormatTokenLexer) even it it was just temporarily 
for this pass we maybe could do that.

I think I want to follow @klimek suggestion to give the QualifierAlignmentFixer 
its own set of proper unit tests to ensure I test its individual function at a 
lower unit level, rather than just looking at the output.

I'm in no rush, I want to let the RFC have a good amount time for people to 
read and to gather all the feedback, but I'm encouraged enough to think my 
efforts won't be wasted and I want to address some of @aaron.ballman  concerns 
to extend a olive branch in the hope that I could at least partially gain 
approval.

It needs to wait for the next LLVM weekly in my view, where I'm thinking @asb 
might give it a mention, and then we should wait too for more feedback, as that 
could be a different audience.

So if you don't mind all the little and often updates (that's how I tend to 
work) I think its better I keep beating on this until everyone is happy.

If you are subscriber (as that list seems long) and don't want to listen to all 
the chatter I won't be offended if you drop off.




Comment at: clang/docs/ClangFormatStyleOptions.rst:2107
+  * ``CVQAS_Leave`` (in configuration: ``Leave``)
+Don't change cvqualifier to either Left or Right alignment
+

curdeius wrote:
> Nit: `cvqualifier` seems ugly, here and below.
I agree, I'm going to begin removal completely...



Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:140
+  // We only need to think about streams that begin with const.
+  if (!Tok->is(CVQualifierType)) {
+return Tok;

HazardyKnusperkeks wrote:
> More Braces (follow)
Uh! this is my own style bleeding through... its why I need clang-format to 
remove them for me! ;-)



Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:201
+  }
+  Next = Next->Next;
+}

curdeius wrote:
> That will access a nullptr if `Next` in the previous `if` was null. Is that 
> possible BTW? Can we add a test for this? Maybe some possibly invalid code 
> that starts has identifier and coloncolon but doesn't have a template opener? 
> (maybe just `const std::Foo`)?
I can't actually get this to produce a nullptr trying various

```
const std::Foo
const std::Foo<
const std::Foo<>
const std::Foo
```

I feel this is because its not going to be a `TT_TemplateOpener` if there isn't 
both `<` and `>` otherwise its a less than. I'll add some asserts to try and 
see if it ever happens




Comment at: clang/lib/Format/QualifierAlignmentFixer.h:24
+class QualifierAlignmentFixer : public TokenAnalyzer {
+  std::string CVQualifier;
+

HazardyKnusperkeks wrote:
> I would go for only Qualifier.
Do you think everywhere now too? including the options?


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

https://reviews.llvm.org/D69764

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

[PATCH] D107138: [PowerPC] Implement cmplxl builtins

[Victor Huang via Phabricator via cfe-commits]

NeHuang accepted this revision.
NeHuang added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107138

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

[PATCH] D107461: [PowerPC] Do not define __PRIVILEGED__

[Victor Huang via Phabricator via cfe-commits]

NeHuang accepted this revision as: NeHuang.
NeHuang added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107461

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

[PATCH] D107668: [OpenMP]Fix PR50336: Remove temporary files in the offload bundler tool

[Joseph Huber via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG01d59c0de822: [OpenMP]Fix PR50336: Remove temporary files in 
the offload bundler tool (authored by jhuber6).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107668

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


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7737,8 +7737,11 @@
 assert(CurTC == nullptr && "Expected one dependence!");
 CurTC = TC;
   });
+  UB += C.addTempFile(
+  C.getArgs().MakeArgString(CurTC->getInputFilename(Inputs[I])));
+} else {
+  UB += CurTC->getInputFilename(Inputs[I]);
 }
-UB += CurTC->getInputFilename(Inputs[I]);
   }
   CmdArgs.push_back(TCArgs.MakeArgString(UB));
 


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7737,8 +7737,11 @@
 assert(CurTC == nullptr && "Expected one dependence!");
 CurTC = TC;
   });
+  UB += C.addTempFile(
+  C.getArgs().MakeArgString(CurTC->getInputFilename(Inputs[I])));
+} else {
+  UB += CurTC->getInputFilename(Inputs[I]);
 }
-UB += CurTC->getInputFilename(Inputs[I]);
   }
   CmdArgs.push_back(TCArgs.MakeArgString(UB));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 01d59c0 - [OpenMP]Fix PR50336: Remove temporary files in the offload bundler tool

[via cfe-commits]

Author: Joseph Huber
Date: 2021-08-11T08:50:47-04:00
New Revision: 01d59c0de822099c62f12f275c41338f6df9f5ac

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

LOG: [OpenMP]Fix PR50336: Remove temporary files in the offload bundler tool

Temporary files created by the offloading device toolchain are not removed
after compilation when using a two-step compilation. The offload-bundler uses a
different filename for the device binary than the `.o` file present in the
Job's input list. This is not listed as a temporary file so it is never
removed. This patch explicitly adds the device binary as a temporary file to
consume it. This fixes PR50336.

Reviewed By: jdoerfert

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 956b31e18ef8..ceeae94e5667 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -7737,8 +7737,11 @@ void OffloadBundler::ConstructJob(Compilation , const 
JobAction ,
 assert(CurTC == nullptr && "Expected one dependence!");
 CurTC = TC;
   });
+  UB += C.addTempFile(
+  C.getArgs().MakeArgString(CurTC->getInputFilename(Inputs[I])));
+} else {
+  UB += CurTC->getInputFilename(Inputs[I]);
 }
-UB += CurTC->getInputFilename(Inputs[I]);
   }
   CmdArgs.push_back(TCArgs.MakeArgString(UB));
 



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

[PATCH] D107002: [PowerPC] Implement XL compatibility builtin __addex

[Victor Huang via Phabricator via cfe-commits]

NeHuang added a comment.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107002

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

[PATCH] D106262: [clang][analyzer] Use generic note tag in alpha.unix.Stream .

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:696
   // Add transition for the failed state.
   Optional RetVal = makeRetVal(C, CE).castAs();
   assert(RetVal && "Value should be NonLoc.");

balazske wrote:
> Szelethus wrote:
> > Lets leave a TODO here, before we forget it:
> > [[ http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf | C'99, pdf 
> > page 313, §7.19.8.1.2]], Description of `fread`:
> > > If a partial element is read, its value is indeterminate.
> This means that the (content of the) buffer passed to `fread` should become 
> in a "uninitialized" (undefined) state?
On the return value:
> The fread function returns the number of elements successfully read, which 
> may be less than nmemb if a read error or end-of-file is encountered.
So I guess only the (return value + 1)th element of the array is indeterminate.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:726-729
+"Stream reaches end-of-file or operation fails here"));
   else
-C.addTransition(StateFailed);
+C.addTransition(StateFailed, constructFailureNoteTag(
+ C, StreamSym, "Operation fails here"));

balazske wrote:
> Szelethus wrote:
> > We can be more specific here. While the standard doesn't explicitly specify 
> > that a read failure could result in ferror being set, it does state that 
> > the file position indicator will be indeterminate:
> > 
> > [[ http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf | C'99, pdf 
> > page 313, §7.19.8.1.2]], Description of `fread`:
> > > If an error occurs, the resulting value of the file position indicator 
> > > for the stream is indeterminate.
> > 
> > [[ http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf | C'99, pdf 
> > page 313, §7.19.8.2.2]], Description of `fwrite`:
> > > If an error occurs, the resulting value of the file position indicator 
> > > for the stream is indeterminate.
> > 
> > Since this is the event to highlight, I'd like to see it mentioned. How 
> > about:
> > > Stream either reaches end-of-file, or fails and has its file position 
> > > indicator left indeterminate, or the error flag set.
> > > After this operation fails, the stream either has its file position 
> > > indicator left indeterminate, or the error flag set.
> > 
> > Same for any other case where indeterminate file positions could occur.
> For the `fread` and `fwrite` cases, I think that the error flag **and** the 
> indeterminate position is always set if error occurs. It looks more natural 
> to tell the user that "the operation fails" than "file position becomes 
> indeterminate". And the user could see that the operation fails and file 
> position is "indeterminate" from the error reports, the failure causes the 
> indeterminate (or "undefined"?) position.
> 
> Only the `fseek` is where indeterminate position can appear without setting 
> the ferror flag (but the failure is discoverable by checking the return value 
> of `fseek`). Still the cases "operation fails" (set ferror flag and/or leave 
> file position indeterminate, return nonzero) and "stream reaches end-of-file" 
> are the ones that are possible. The checker documentation can contain more 
> exactly why the checker works this way.
Well, to me, seeing both the error flag and the file position indicator being 
mentioned here sounds nice, since we are already in the possession of that 
information. How about

>Stream either reaches end-of-file, or fails and has its file position 
>indicator left indeterminate and the error flag set.
>After this operation fails, the stream either has its file position indicator 
>left indeterminate and the error flag set.
?

> The checker documentation can contain more exactly why the checker works this 
> way.
I think adding this bit about the file position indicator shouldn't be in the 
docs only, though explaining the schrödinger-like behaviour in there would be 
nice.

>I think that the error flag and the indeterminate position is always set if 
>error occurs.

This means that we need to make our `ferror` in the future smarter. Can you 
leave a TODO about that `ferror` needs to check what was the last stream 
operation that may have failed? In the case where it was an `fread`/`fwrite`, 
on its false branch, we need to clear both ferror and the file position 
indocator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106262

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

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

In D69764#2939037 , 
@HazardyKnusperkeks wrote:

> In D69764#2938973 , @MyDeveloperDay 
> wrote:
>
>> I'm leaning toward introducing  into the `CVQualifierOrder` allowing 
>> for some qualifiers to be Left of the type and some to be Right (this is not 
>> dissimilar to what was discussed some time ago, and similar to the concept 
>> of the resharper image above)
>
> Seems reasonable. I would like to remove the `CV`, only `QualifierOrder`. And 
> please take attributes into account, C++ attributes like `[[noreturn]]`, but 
> also compiler attributes like `__attribute__((noreturn))` or 
> `__declspec(noreturn)`, I think it should be possible to add `` 
> to your list.

I like both proposals.




Comment at: clang/docs/ClangFormatStyleOptions.rst:2101
 
+**CVQualifierAlignment** (``CVQualifierAlignmentStyle``)
+  Different ways to arrange const/volatile qualifiers.

Why not just `Qualifier` to allow future additions for other qualifiers?
Technically, `static` or `inline` are not qualifiers, but that maybe is clear 
enough?

Other possibility, `Specifier` (as in "inline specifier")?
OTOH, `Keyword` may be too generic.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2107
+  * ``CVQAS_Leave`` (in configuration: ``Leave``)
+Don't change cvqualifier to either Left or Right alignment
+

Nit: `cvqualifier` seems ugly, here and below.



Comment at: clang/lib/Format/Format.cpp:2908-2909
+// Depending on the placement style of left or right you need
+// To iterate forward or backward though the order list as qualifier
+// can push though each other.
+// Copy the very small list and reverse the order if left aligned.

Nit: typos.



Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:201
+  }
+  Next = Next->Next;
+}

That will access a nullptr if `Next` in the previous `if` was null. Is that 
possible BTW? Can we add a test for this? Maybe some possibly invalid code that 
starts has identifier and coloncolon but doesn't have a template opener? (maybe 
just `const std::Foo`)?


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

https://reviews.llvm.org/D69764

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

[PATCH] D107893: [clang] [MinGW] Consider the per-target libc++ include directory too

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: phosek, MaskRay.
mstorsjo requested review of this revision.
Herald added a project: clang.

The existing logic for per-target libc++ include directories only
seem to exist for the Gnu and Fuchsia drivers, added in
ea12d779bc238c387511fe7462020f4ecf4a8246 
 / D89013 
.

This is less generic than the corresponding case in the Gnu driver,
but matches the existing level of genericity in the MinGW driver
(and others too).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107893

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


Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -588,12 +588,16 @@
   StringRef Slash = llvm::sys::path::get_separator();
 
   switch (GetCXXStdlibType(DriverArgs)) {
-  case ToolChain::CST_Libcxx:
+  case ToolChain::CST_Libcxx: {
+std::string TargetDir = (Base + "include" + Slash + getTripleString() + 
Slash + "c++" + Slash + "v1").str();
+if (getDriver().getVFS().exists(TargetDir))
+  addSystemInclude(DriverArgs, CC1Args, TargetDir);
 addSystemInclude(DriverArgs, CC1Args, Base + Arch + Slash + "include" +
   Slash + "c++" + Slash + "v1");
 addSystemInclude(DriverArgs, CC1Args,
  Base + "include" + Slash + "c++" + Slash + "v1");
 break;
+  }
 
   case ToolChain::CST_Libstdcxx:
 llvm::SmallVector, 4> CppIncludeBases;


Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -588,12 +588,16 @@
   StringRef Slash = llvm::sys::path::get_separator();
 
   switch (GetCXXStdlibType(DriverArgs)) {
-  case ToolChain::CST_Libcxx:
+  case ToolChain::CST_Libcxx: {
+std::string TargetDir = (Base + "include" + Slash + getTripleString() + Slash + "c++" + Slash + "v1").str();
+if (getDriver().getVFS().exists(TargetDir))
+  addSystemInclude(DriverArgs, CC1Args, TargetDir);
 addSystemInclude(DriverArgs, CC1Args, Base + Arch + Slash + "include" +
   Slash + "c++" + Slash + "v1");
 addSystemInclude(DriverArgs, CC1Args,
  Base + "include" + Slash + "c++" + Slash + "v1");
 break;
+  }
 
   case ToolChain::CST_Libstdcxx:
 llvm::SmallVector, 4> CppIncludeBases;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107402: Correct a lot of diagnostic wordings for the driver

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

This was committed in 530ea28fefc4652823a187a149e8001cbf4af18f 
.


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

https://reviews.llvm.org/D107402

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

[PATCH] D107051: [clang][analyzer] Improve bug report in alpha.security.ReturnPtrRange

[Balázs Kéri 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 rG9f517fd11ee9: [clang][analyzer] Improve bug report in 
alpha.security.ReturnPtrRange (authored by balazske).

Changed prior to commit:
  https://reviews.llvm.org/D107051?vs=365693=365708#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107051

Files:
  clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/return-ptr-range.cpp

Index: clang/test/Analysis/return-ptr-range.cpp
===
--- clang/test/Analysis/return-ptr-range.cpp
+++ clang/test/Analysis/return-ptr-range.cpp
@@ -1,29 +1,39 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.security.ReturnPtrRange -verify %s
-
-int arr[10];
-int *ptr;
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.security.ReturnPtrRange -analyzer-output text -verify %s
 
 int conjure_index();
 
-int *test_element_index_lifetime() {
-  do {
+namespace test_element_index_lifetime {
+
+int arr[10]; // expected-note{{Original object declared here}} expected-note{{Original object declared here}}
+int *ptr;
+
+int *test_global_ptr() {
+  do { // expected-note{{Loop condition is false.  Exiting loop}}
 int x = conjure_index();
-ptr = arr + x;
-if (x != 20)
+ptr = arr + x; // expected-note{{Value assigned to 'ptr'}}
+if (x != 20) // expected-note{{Assuming 'x' is equal to 20}}
+ // expected-note@-1{{Taking false branch}}
   return arr; // no-warning
-  } while (0);
-  return ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}}
+  } while(0);
+  return ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow) [alpha.security.ReturnPtrRange]}}
+  // expected-note@-1{{Returned pointer value points outside the original object (potential buffer overflow)}}
+  // expected-note@-2{{Original object 'arr' is an array of 10 'int' objects}}
 }
 
-int *test_element_index_lifetime_with_local_ptr() {
+int *test_local_ptr() {
   int *local_ptr;
-  do {
+  do { // expected-note{{Loop condition is false.  Exiting loop}}
 int x = conjure_index();
-local_ptr = arr + x;
-if (x != 20)
+local_ptr = arr + x; // expected-note{{Value assigned to 'local_ptr'}}
+if (x != 20) // expected-note{{Assuming 'x' is equal to 20}}
+ // expected-note@-1{{Taking false branch}}
   return arr; // no-warning
-  } while (0);
-  return local_ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}}
+  } while(0);
+  return local_ptr; // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow) [alpha.security.ReturnPtrRange]}}
+// expected-note@-1{{Returned pointer value points outside the original object (potential buffer overflow)}}
+// expected-note@-2{{Original object 'arr' is an array of 10 'int' objects}}
+}
+
 }
 
 template 
@@ -55,17 +65,53 @@
 
 template 
 class BadIterable {
-  int buffer[N];
+  int buffer[N]; // expected-note{{Original object declared here}}
   int *start, *finish;
 
 public:
-  BadIterable() : start(buffer), finish(buffer + N) {}
+  BadIterable() : start(buffer), finish(buffer + N) {} // expected-note{{Value assigned to 'iter.finish'}}
 
   int* begin() { return start; }
-  int* end() { return finish + 1; } // expected-warning{{Returned pointer value points outside the original object (potential buffer overflow)}}
+  int* end() { return finish + 1; } // expected-warning{{Returned pointer value points outside the original object}}
+// expected-note@-1{{Returned pointer value points outside the original object}}
+// expected-note@-2{{Original object 'buffer' is an array of 20 'int' objects, returned pointer points at index 21}}
 };
 
 void use_bad_iterable_object() {
-  BadIterable<20> iter;
-  iter.end();
+  BadIterable<20> iter; // expected-note{{Calling default constructor for 'BadIterable<20>'}}
+// expected-note@-1{{Returning from default constructor for 'BadIterable<20>'}}
+  iter.end(); // expected-note{{Calling 'BadIterable::end'}}
 }
+
+int *test_idx_sym(int I) {
+  static int arr[10]; // expected-note{{Original object declared here}} expected-note{{'arr' initialized here}}
+
+  if (I != 11) // expected-note{{Assuming 'I' is equal to 11}}
+   // expected-note@-1{{Taking false branch}}
+return arr;
+  return arr + I; // expected-warning{{Returned pointer value points outside the original object}}
+  // expected-note@-1{{Returned 

[clang] 9f517fd - [clang][analyzer] Improve bug report in alpha.security.ReturnPtrRange

[Balázs Kéri via cfe-commits]

Author: Balázs Kéri
Date: 2021-08-11T13:04:55+02:00
New Revision: 9f517fd11ee9b9e49eabff7400cfe13b424e9b06

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

LOG: [clang][analyzer] Improve bug report in alpha.security.ReturnPtrRange

Add some notes and track of bad return value.

Reviewed By: steakhal

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
clang/test/Analysis/misc-ps-region-store.m
clang/test/Analysis/return-ptr-range.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
index 885750218b9e5..c4dc06d4a0777 100644
--- a/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/ReturnPointerRangeChecker.cpp
@@ -12,6 +12,7 @@
 
//===--===//
 
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
@@ -79,16 +80,44 @@ void ReturnPointerRangeChecker::checkPreStmt(const 
ReturnStmt *RS,
   "Returned pointer value points outside the original object "
   "(potential buffer overflow)"));
 
-// FIXME: It would be nice to eventually make this diagnostic more clear,
-// e.g., by referencing the original declaration or by saying *why* this
-// reference is outside the range.
-
 // Generate a report for this bug.
-auto report =
+auto Report =
 std::make_unique(*BT, BT->getDescription(), N);
-
-report->addRange(RetE->getSourceRange());
-C.emitReport(std::move(report));
+Report->addRange(RetE->getSourceRange());
+
+const auto ConcreteElementCount = 
ElementCount.getAs();
+const auto ConcreteIdx = Idx.getAs();
+
+const auto *DeclR = ER->getSuperRegion()->getAs();
+
+if (DeclR)
+  Report->addNote("Original object declared here",
+  {DeclR->getDecl(), C.getSourceManager()});
+
+if (ConcreteElementCount) {
+  SmallString<128> SBuf;
+  llvm::raw_svector_ostream OS(SBuf);
+  OS << "Original object ";
+  if (DeclR) {
+OS << "'";
+DeclR->getDecl()->printName(OS);
+OS << "' ";
+  }
+  OS << "is an array of " << ConcreteElementCount->getValue() << " '";
+  ER->getValueType().print(OS,
+   
PrintingPolicy(C.getASTContext().getLangOpts()));
+  OS << "' objects";
+  if (ConcreteIdx) {
+OS << ", returned pointer points at index " << ConcreteIdx->getValue();
+  }
+
+  Report->addNote(SBuf,
+  {RetE, C.getSourceManager(), C.getLocationContext()});
+}
+
+bugreporter::trackExpressionValue(N, RetE, *Report);
+
+C.emitReport(std::move(Report));
   }
 }
 

diff  --git a/clang/test/Analysis/misc-ps-region-store.m 
b/clang/test/Analysis/misc-ps-region-store.m
index 9c31324e7e59d..6332ff5a50103 100644
--- a/clang/test/Analysis/misc-ps-region-store.m
+++ b/clang/test/Analysis/misc-ps-region-store.m
@@ -461,10 +461,11 @@ void element_region_with_symbolic_superregion(int* p) {
 // Test returning an out-of-bounds pointer (CWE-466)
 
//===--===//
 
-static int test_cwe466_return_outofbounds_pointer_a[10];
+static int test_cwe466_return_outofbounds_pointer_a[10]; // 
expected-note{{Original object declared here}}
 int *test_cwe466_return_outofbounds_pointer() {
   int *p = test_cwe466_return_outofbounds_pointer_a+11;
   return p; // expected-warning{{Returned pointer value points outside the 
original object}}
+// expected-note@-1{{Original object 
'test_cwe466_return_outofbounds_pointer_a' is an array of 10 'int' objects, 
returned pointer points at index 11}}
 }
 
 
//===--===//

diff  --git a/clang/test/Analysis/return-ptr-range.cpp 
b/clang/test/Analysis/return-ptr-range.cpp
index 9763b51264e6f..34c953ee213b7 100644
--- a/clang/test/Analysis/return-ptr-range.cpp
+++ b/clang/test/Analysis/return-ptr-range.cpp
@@ -1,29 +1,39 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=alpha.security.ReturnPtrRange 
-verify %s
-
-int arr[10];
-int *ptr;
+// RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.security.ReturnPtrRange -analyzer-output text 
-verify %s
 
 int conjure_index();
 
-int *test_element_index_lifetime() {
-  do {
+namespace test_element_index_lifetime {
+

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for FP atomics

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 365625.
gandhi21299 added a comment.

- reverted the patch to the previous one


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

Files:
  clang/test/CodeGenCUDA/fp-atomics-optremarks.cu
  clang/test/CodeGenOpenCL/fp-atomics-optremarks-gfx90a.cl
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/test/CodeGen/AMDGPU/fp-atomics-remarks-gfx90a.ll
  llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll

Index: llvm/test/CodeGen/X86/opt-pipeline.ll
===
--- llvm/test/CodeGen/X86/opt-pipeline.ll
+++ llvm/test/CodeGen/X86/opt-pipeline.ll
@@ -16,15 +16,20 @@
 ; CHECK-NEXT: Target Pass Configuration
 ; CHECK-NEXT: Machine Module Information
 ; CHECK-NEXT: Target Transform Information
+; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Type-Based Alias Analysis
 ; CHECK-NEXT: Scoped NoAlias Alias Analysis
 ; CHECK-NEXT: Assumption Cache Tracker
-; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
+; CHECK-NEXT:   Dominator Tree Construction
+; CHECK-NEXT:   Natural Loop Information
+; CHECK-NEXT:   Lazy Branch Probability Analysis
+; CHECK-NEXT:   Lazy Block Frequency Analysis
+; CHECK-NEXT:   Optimization Remark Emitter
 ; CHECK-NEXT:   Expand Atomic instructions
 ; CHECK-NEXT:   Lower AMX intrinsics
 ; CHECK-NEXT:   Lower AMX type for load/store
Index: llvm/test/CodeGen/X86/O0-pipeline.ll
===
--- llvm/test/CodeGen/X86/O0-pipeline.ll
+++ llvm/test/CodeGen/X86/O0-pipeline.ll
@@ -10,13 +10,18 @@
 ; CHECK-NEXT: Target Pass Configuration
 ; CHECK-NEXT: Machine Module Information
 ; CHECK-NEXT: Target Transform Information
+; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
 ; CHECK-NEXT: Assumption Cache Tracker
-; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
+; CHECK-NEXT:   Dominator Tree Construction
+; CHECK-NEXT:   Natural Loop Information
+; CHECK-NEXT:   Lazy Branch Probability Analysis
+; CHECK-NEXT:   Lazy Block Frequency Analysis
+; CHECK-NEXT:   Optimization Remark Emitter
 ; CHECK-NEXT:   Expand Atomic instructions
 ; CHECK-NEXT:   Lower AMX intrinsics
 ; CHECK-NEXT:   Lower AMX type for load/store
Index: llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
===
--- llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
+++ llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
@@ -44,6 +44,11 @@
 ; GCN-O0-NEXT:Lower OpenCL enqueued blocks
 ; GCN-O0-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O0-NEXT:FunctionPass Manager
+; GCN-O0-NEXT:  Dominator Tree Construction
+; GCN-O0-NEXT:  Natural Loop Information
+; GCN-O0-NEXT:  Lazy Branch Probability Analysis
+; GCN-O0-NEXT:  Lazy Block Frequency Analysis
+; GCN-O0-NEXT:  Optimization Remark Emitter
 ; GCN-O0-NEXT:  Expand Atomic instructions
 ; GCN-O0-NEXT:  Lower constant intrinsics
 ; GCN-O0-NEXT:  Remove unreachable blocks from the CFG
@@ -180,6 +185,11 @@
 ; GCN-O1-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O1-NEXT:FunctionPass Manager
 ; GCN-O1-NEXT:  Infer address spaces
+; GCN-O1-NEXT:  Dominator Tree Construction
+; GCN-O1-NEXT:  Natural Loop Information
+; GCN-O1-NEXT:  Lazy Branch Probability Analysis
+; GCN-O1-NEXT:  Lazy Block Frequency Analysis
+; GCN-O1-NEXT:  Optimization Remark Emitter
 ; GCN-O1-NEXT:  Expand Atomic instructions
 ; GCN-O1-NEXT:  AMDGPU Promote Alloca
 ; GCN-O1-NEXT:  Dominator Tree Construction
@@ -431,6 +441,11 @@
 ; GCN-O1-OPTS-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O1-OPTS-NEXT:FunctionPass Manager
 ; GCN-O1-OPTS-NEXT:  Infer address spaces
+; GCN-O1-OPTS-NEXT:  Dominator Tree Construction
+; GCN-O1-OPTS-NEXT:  Natural Loop Information
+; GCN-O1-OPTS-NEXT:  Lazy Branch Probability Analysis
+; GCN-O1-OPTS-NEXT:  Lazy Block Frequency Analysis
+; GCN-O1-OPTS-NEXT:  Optimization Remark Emitter
 ; GCN-O1-OPTS-NEXT:  Expand 

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for FP atomics

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added a comment.

In D106891#2938128 , @gandhi21299 
wrote:

> @rampitec besides the remarks, am I missing anything else in the patch?

You should not use AMD specific code in the common code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for FP atomics

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 added a comment.

@rampitec besides the remarks, am I missing anything else in the patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

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

[PATCH] D106891: [AMDGPU] [Remarks] Emit optimization remarks for FP atomics

[Anshil Gandhi via Phabricator via cfe-commits]

gandhi21299 updated this revision to Diff 365564.
gandhi21299 marked an inline comment as done.
gandhi21299 added a comment.

requested change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

Files:
  clang/test/CodeGenCUDA/fp-atomics-optremarks.cu
  clang/test/CodeGenOpenCL/fp-atomics-optremarks-gfx90a.cl
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/lib/CodeGen/AtomicExpandPass.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp
  llvm/lib/Target/AMDGPU/AMDGPUISelLowering.h
  llvm/lib/Target/AMDGPU/SIISelLowering.cpp
  llvm/lib/Target/AMDGPU/SIISelLowering.h
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/test/CodeGen/AMDGPU/fp-atomics-remarks-gfx90a.ll
  llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll

Index: llvm/test/CodeGen/X86/opt-pipeline.ll
===
--- llvm/test/CodeGen/X86/opt-pipeline.ll
+++ llvm/test/CodeGen/X86/opt-pipeline.ll
@@ -16,15 +16,20 @@
 ; CHECK-NEXT: Target Pass Configuration
 ; CHECK-NEXT: Machine Module Information
 ; CHECK-NEXT: Target Transform Information
+; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Type-Based Alias Analysis
 ; CHECK-NEXT: Scoped NoAlias Alias Analysis
 ; CHECK-NEXT: Assumption Cache Tracker
-; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
+; CHECK-NEXT:   Dominator Tree Construction
+; CHECK-NEXT:   Natural Loop Information
+; CHECK-NEXT:   Lazy Branch Probability Analysis
+; CHECK-NEXT:   Lazy Block Frequency Analysis
+; CHECK-NEXT:   Optimization Remark Emitter
 ; CHECK-NEXT:   Expand Atomic instructions
 ; CHECK-NEXT:   Lower AMX intrinsics
 ; CHECK-NEXT:   Lower AMX type for load/store
Index: llvm/test/CodeGen/X86/O0-pipeline.ll
===
--- llvm/test/CodeGen/X86/O0-pipeline.ll
+++ llvm/test/CodeGen/X86/O0-pipeline.ll
@@ -10,13 +10,18 @@
 ; CHECK-NEXT: Target Pass Configuration
 ; CHECK-NEXT: Machine Module Information
 ; CHECK-NEXT: Target Transform Information
+; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Create Garbage Collector Module Metadata
 ; CHECK-NEXT: Assumption Cache Tracker
-; CHECK-NEXT: Profile summary info
 ; CHECK-NEXT: Machine Branch Probability Analysis
 ; CHECK-NEXT:   ModulePass Manager
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
+; CHECK-NEXT:   Dominator Tree Construction
+; CHECK-NEXT:   Natural Loop Information
+; CHECK-NEXT:   Lazy Branch Probability Analysis
+; CHECK-NEXT:   Lazy Block Frequency Analysis
+; CHECK-NEXT:   Optimization Remark Emitter
 ; CHECK-NEXT:   Expand Atomic instructions
 ; CHECK-NEXT:   Lower AMX intrinsics
 ; CHECK-NEXT:   Lower AMX type for load/store
Index: llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
===
--- llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
+++ llvm/test/CodeGen/AMDGPU/llc-pipeline.ll
@@ -44,6 +44,11 @@
 ; GCN-O0-NEXT:Lower OpenCL enqueued blocks
 ; GCN-O0-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O0-NEXT:FunctionPass Manager
+; GCN-O0-NEXT:  Dominator Tree Construction
+; GCN-O0-NEXT:  Natural Loop Information
+; GCN-O0-NEXT:  Lazy Branch Probability Analysis
+; GCN-O0-NEXT:  Lazy Block Frequency Analysis
+; GCN-O0-NEXT:  Optimization Remark Emitter
 ; GCN-O0-NEXT:  Expand Atomic instructions
 ; GCN-O0-NEXT:  Lower constant intrinsics
 ; GCN-O0-NEXT:  Remove unreachable blocks from the CFG
@@ -180,6 +185,11 @@
 ; GCN-O1-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O1-NEXT:FunctionPass Manager
 ; GCN-O1-NEXT:  Infer address spaces
+; GCN-O1-NEXT:  Dominator Tree Construction
+; GCN-O1-NEXT:  Natural Loop Information
+; GCN-O1-NEXT:  Lazy Branch Probability Analysis
+; GCN-O1-NEXT:  Lazy Block Frequency Analysis
+; GCN-O1-NEXT:  Optimization Remark Emitter
 ; GCN-O1-NEXT:  Expand Atomic instructions
 ; GCN-O1-NEXT:  AMDGPU Promote Alloca
 ; GCN-O1-NEXT:  Dominator Tree Construction
@@ -431,6 +441,11 @@
 ; GCN-O1-OPTS-NEXT:Lower uses of LDS variables from non-kernel functions
 ; GCN-O1-OPTS-NEXT:FunctionPass Manager
 ; GCN-O1-OPTS-NEXT:  Infer address spaces
+; GCN-O1-OPTS-NEXT:  Dominator Tree Construction
+; GCN-O1-OPTS-NEXT:  Natural Loop Information
+; GCN-O1-OPTS-NEXT:  Lazy Branch Probability Analysis
+; GCN-O1-OPTS-NEXT:  Lazy Block Frequency Analysis
+; GCN-O1-OPTS-NEXT:  Optimization Remark Emitter
 ; 

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added a comment.

In D69764#2938973 , @MyDeveloperDay 
wrote:

> With the introduction of `CVQualifierOrder` the need for 
> `CVQualifierAlignment:` doesn't make so much sense
>
>   CVQualifierAlignment: Left
>   CVQualifierOrder: [ "inline", "static", "volatile", "const" ]
>
> I'm leaning toward introducing  into the `CVQualifierOrder` allowing 
> for some qualifiers to be Left of the type and some to be Right (this is not 
> dissimilar to what was discussed some time ago, and similar to the concept of 
> the resharper image above)
>
> So we would be able to have:
>
>   CVQualifierOrder: [ "inline", "static", "volatile", "", "const", 
> "restrict" ]
>
> By doing so `inline,static,and volatile` are Left and `const,restrict` are 
> Right and so a single setting of Left and Right isn't applicable.
>
> The whole feature could then be determined ONLY using the `CVQualifierOrder` 
> however it doesn't feel like there is a good way to say "Don't change order" 
> (unless of course I use an empty `CVQualifierOrder` list), but this doesn't 
> feel very off by default enough, I like the explicitness of `Leave`.
>
> My gut feeling is to make `Left` and `Right` options define a predetermined 
> CVQualifierOrder, and introduce `Custom` as a new CVQualifierAlignment 
> setting which would require you to define the Order, this has the advantage 
> of driving a sort of semi standard as to what we mean by `Left` and `Right`
>
> Left would by default define:
>
>   CVQualifierOrder: [ "inline", "static", "volatile", "const", "restrict", 
> ""]
>
> Right would define:
>
>   CVQualifierOrder: [ "inline", "static", "", "volatile", "const", 
> "restrict" ]
>
> (we might want to discuss what these defaults should be, I'm not a language 
> lawyer!)
>
> I realise "inline" and "static" are to the left in the Right alignment, but 
> I'm not sure them being to the right is even legal (its certainly not common) 
> or what is most likely wanted.
>
> I also think if a `CVQualifierOrder` is defined and the 
> `CVQualifierAlignment` is not `Custom` that should be a configuration error 
> to ensure there isn't any ambiguity
>
> Does anyone have any thoughts on this approach?

Seems reasonable. I would like to remove the `CV`, only `QualifierOrder`. And 
please take attributes into account, C++ attributes like `[[noreturn]]`, but 
also compiler attributes like `__attribute__((noreturn))` or 
`__declspec(noreturn)`, I think it should be possible to add `` to 
your list.




Comment at: clang/lib/Format/Format.cpp:1494
+  // If its empty then it means don't do anything.
+  if (Style->CVQualifierOrder.empty()) {
+return true;

Braces :)



Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:32
+  if (Err) {
+llvm::errs() << "Error while rearranging const : "
+ << llvm::toString(std::move(Err)) << "\n";

Qualifier?



Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:140
+  // We only need to think about streams that begin with const.
+  if (!Tok->is(CVQualifierType)) {
+return Tok;

More Braces (follow)



Comment at: clang/lib/Format/QualifierAlignmentFixer.h:24
+class QualifierAlignmentFixer : public TokenAnalyzer {
+  std::string CVQualifier;
+

I would go for only Qualifier.


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

https://reviews.llvm.org/D69764

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

[PATCH] D107887: scan-build-py: Force the opening in utf-8

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru created this revision.
sylvestre.ledru added reviewers: phosek, isthismyaccount, aabbaabb.
Herald added a subscriber: whisperity.
sylvestre.ledru requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

It fails on ubuntu bionic otherwise with:

  scan-build-py-14: Run 'scan-view 
/tmp/scan-build-2021-08-09-09-14-36-765350-nx9s888s' to examine bug reports.
  scan-build-py-14: Internal error.
  Traceback (most recent call last):
File "/usr/lib/llvm-14/lib/libscanbuild/__init__.py", line 125, in wrapper
  return function(*args, **kwargs)
File "/usr/lib/llvm-14/lib/libscanbuild/analyze.py", line 72, in scan_build
  number_of_bugs = document(args)
File "/usr/lib/llvm-14/lib/libscanbuild/report.py", line 35, in document
  for bug in read_bugs(args.output, html_reports_available):
File "/usr/lib/llvm-14/lib/libscanbuild/report.py", line 282, in read_bugs
  for bug in parser(bug_file):
File "/usr/lib/llvm-14/lib/libscanbuild/report.py", line 421, in 
parse_bug_html
  for line in handler.readlines():
File "/usr/lib/python3.6/encodings/ascii.py", line 26, in decode
  return codecs.ascii_decode(input, self.errors)[0]
  UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 3360: 
ordinal not in range(128)
  scan-build-py-14: Please run this command again and turn on verbose mode (add 
'-' as argument).

I guess it is caused by a problem in Python 3.6


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107887

Files:
  clang/tools/scan-build-py/lib/libscanbuild/report.py


Index: clang/tools/scan-build-py/lib/libscanbuild/report.py
===
--- clang/tools/scan-build-py/lib/libscanbuild/report.py
+++ clang/tools/scan-build-py/lib/libscanbuild/report.py
@@ -417,7 +417,7 @@
 'bug_path_length': 1
 }
 
-with open(filename) as handler:
+with open(filename, encoding="utf-8") as handler:
 for line in handler.readlines():
 # do not read the file further
 if endsign.match(line):


Index: clang/tools/scan-build-py/lib/libscanbuild/report.py
===
--- clang/tools/scan-build-py/lib/libscanbuild/report.py
+++ clang/tools/scan-build-py/lib/libscanbuild/report.py
@@ -417,7 +417,7 @@
 'bug_path_length': 1
 }
 
-with open(filename) as handler:
+with open(filename, encoding="utf-8") as handler:
 for line in handler.readlines():
 # do not read the file further
 if endsign.match(line):
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107632: [clangd] Avoid "expected one compiler job" by picking the first eligible job.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, lgtm!




Comment at: clang-tools-extra/clangd/Compiler.cpp:42
  const clang::Diagnostic ) {
+  DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info);
   IgnoreDiagnostics::log(DiagLevel, Info);

not sure i follow this change. is this for making sure fatal/error diagnostic 
counts can be checked (so that actions can signal failure) or something else?



Comment at: clang/lib/Frontend/CreateInvocationFromCommandLine.cpp:92
-  const driver::Command  = cast(*Jobs.begin());
-  if (StringRef(Cmd.getCreator().getName()) != "clang") {
 Diags->Report(diag::err_fe_expected_clang_command);

sammccall wrote:
> kadircet wrote:
> > hmm, this looks like a subtle behaviour change. I definitely like the 
> > behaviour you proposed better, but maybe do it in a separate patch for 
> > making the revert easier if need be?
> Which behavior change are do you mean?
> 
> When RecoverFromErrors is false and OffloadCompilation is false, behavior is 
> same as before: if there isn't exactly one job we diagnose that and exit, if 
> it's not suitable we diagnose that and exit.
> 
> When RecoverFromErrors is true we search for the first suitable job, instead 
> of complaining that the number is wrong or the one at the front isn't 
> suitable, this is the intended change (and basically only affects clangd).
> 
> When OffloadCompilation is true, there is a behavior change: previously we'd 
> use the first job and fail if it's not clang, now we'll use the first clang 
> job. The old behavior seems brittle and the change only affects (previously) 
> error cases, and I'd have to add more complexity to preserve it - is this 
> worthwhile?
> When OffloadCompilation is true, there is a behavior change: previously we'd 
> use the first job and fail if it's not clang, now we'll use the first clang 
> job.

I was talking about this particular change.

> The old behavior seems brittle and the change only affects (previously) error 
> cases

As mentioned I also like the new behaviour more, I just don't know about what 
the offloading logic does (i suppose it is about cuda compilation) and I was 
anxious about it breaking when one of the Jobs satisfy the constraint but it is 
not the first one.

> and I'd have to add more complexity to preserve it - is this worthwhile?

One option could be to just keep this part the same (i.e. only use the first 
Job as long as `PickFirstOfMany` is set, even if `OffloadCompilation` is 
false). Then send a follow-up change, that'll search for a suitable job. That 
way we can only revert the last one if it breaks and know the constraints. (And 
decide to do the search iff `ShouldRecoverOnErorrs && !Offload`). But it is not 
that important, feel free to just land as-is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107632

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

[PATCH] D107051: [clang][analyzer] Improve bug report in alpha.security.ReturnPtrRange

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

steakhal accepted this revision.
steakhal added inline comments.



Comment at: clang/test/Analysis/return-ptr-range.cpp:17
   return arr; // no-warning
-  } while (0);
-  return ptr; // expected-warning{{Returned pointer value points outside the 
original object (potential buffer overflow)}}
+  }
+  return ptr; // expected-warning{{Returned pointer value points outside the 
original object (potential buffer overflow) [alpha.security.ReturnPtrRange]}}

balazske wrote:
> steakhal wrote:
> > I don't think we need this extra scope. Same for the others.
> The original test was used to make the `x` "dead" at return after the loop 
> (at least without the fix), see D12726. In the case of loop the `x` is 
> garbage-collected at end of the loop, if a block is used only at the end of 
> the function (?) but not at end of the block. So I want to put back the loop 
> to preserve the original code.
Thank you for letting me know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107051

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

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

With the introduction of `CVQualifierOrder` the need for 
`CVQualifierAlignment:` doesn't make so much sense

  CVQualifierAlignment: Left
  CVQualifierOrder: [ "inline", "static", "volatile", "const" ]

I'm leaning toward introducing  into the `CVQualifierOrder` allowing for 
some qualifiers to be Left of the type and some to be Right (this is not 
dissimilar to what was discussed some time ago, an similar to the concept of 
the resharper image above)

So we would be able to have:

  CVQualifierOrder: [ "inline", "static", "volatile", "", "const", 
"restrict" ]

By doing so `inline,static,and volatile` are Left and `const,restrict` are 
Right and so a single setting of Left and Right isn't applicable.

The whole feature could then be determined ONLY using the `CVQualifierOrder` 
however it doesn't feel like there is a good way to say "Don't change order" 
(unless of course I use an empty `CVQualifierOrder` list), but this doesn't 
feel very off by default enough, I like the explicitness of `Leave`.

My gut feeling is to make `Left` and `Right` options define a predetermined 
CVQualifierOrder, and introduce `Custom` as a new CVQualifierAlignment setting 
which would require you to define the Order, this has the advance of driving a 
sort of semi standard as to what we mean by `Left` and `Right`

Left would by default define:

  CVQualifierOrder: [ "inline", "static", "volatile", "const", "restrict", 
""]

Right would define:

  CVQualifierOrder: [ "inline", "static", "", "volatile", "const", 
"restrict" ]

(we might want to discuss what these defaults should be, I'm not a language 
lawyer!)

I realise "inline" and "static" are to the left in the Right alignment, but I'm 
not sure them being to the right is even legal (its certainly not common) or 
what is most likely wanted.

I also think if a `CVQualifierOrder` is defined and the `CVQualifierAlignment` 
is not `Custom` that should be a configuration error to ensure there isn't any 
ambiguity

Does anyone have any thoughts on this approach?


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

https://reviews.llvm.org/D69764

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

[libunwind] 6b6d344 - [libunwind] Compile with -Wunused-but-set-variable

[Daniel Kiss via cfe-commits]

Author: Daniel Kiss
Date: 2021-08-11T10:15:54+02:00
New Revision: 6b6d3447317673015f62206b2669c2d0a74132dc

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

LOG: [libunwind] Compile with -Wunused-but-set-variable

-Wunused-but-set-variable triggers a warning even the block of code is 
effectively dead.

Reviewed By: MaskRay

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

Added: 


Modified: 
libunwind/CMakeLists.txt
libunwind/src/Unwind-EHABI.cpp
libunwind/src/Unwind-seh.cpp
libunwind/src/UnwindLevel1.c

Removed: 




diff  --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt
index b1ef11bf7327..a73f5b0c7bdf 100644
--- a/libunwind/CMakeLists.txt
+++ b/libunwind/CMakeLists.txt
@@ -191,6 +191,7 @@ add_compile_flags_if_supported(-Wsign-compare)
 add_compile_flags_if_supported(-Wsign-conversion)
 add_compile_flags_if_supported(-Wstrict-aliasing=2)
 add_compile_flags_if_supported(-Wstrict-overflow=4)
+add_compile_flags_if_supported(-Wunused-but-set-variable)
 add_compile_flags_if_supported(-Wunused-parameter)
 add_compile_flags_if_supported(-Wunused-variable)
 add_compile_flags_if_supported(-Wwrite-strings)

diff  --git a/libunwind/src/Unwind-EHABI.cpp b/libunwind/src/Unwind-EHABI.cpp
index ba6064d3ef00..a564fd5240dd 100644
--- a/libunwind/src/Unwind-EHABI.cpp
+++ b/libunwind/src/Unwind-EHABI.cpp
@@ -463,6 +463,7 @@ unwind_phase1(unw_context_t *uc, unw_cursor_t *cursor, 
_Unwind_Exception *except
   return _URC_FATAL_PHASE1_ERROR;
 }
 
+#ifndef NDEBUG
 // When tracing, print state information.
 if (_LIBUNWIND_TRACING_UNWINDING) {
   char functionBuf[512];
@@ -481,6 +482,7 @@ unwind_phase1(unw_context_t *uc, unw_cursor_t *cursor, 
_Unwind_Exception *except
   frameInfo.start_ip, functionName,
   frameInfo.lsda, frameInfo.handler);
 }
+#endif
 
 // If there is a personality routine, ask it if it will want to stop at
 // this frame.
@@ -582,6 +584,7 @@ static _Unwind_Reason_Code unwind_phase2(unw_context_t *uc, 
unw_cursor_t *cursor
   return _URC_FATAL_PHASE2_ERROR;
 }
 
+#ifndef NDEBUG
 // When tracing, print state information.
 if (_LIBUNWIND_TRACING_UNWINDING) {
   char functionBuf[512];
@@ -598,6 +601,7 @@ static _Unwind_Reason_Code unwind_phase2(unw_context_t *uc, 
unw_cursor_t *cursor
   functionName, sp, frameInfo.lsda,
   frameInfo.handler);
 }
+#endif
 
 // If there is a personality routine, tell it we are unwinding.
 if (frameInfo.handler != 0) {
@@ -689,6 +693,7 @@ unwind_phase2_forced(unw_context_t *uc, unw_cursor_t 
*cursor,
   return _URC_FATAL_PHASE2_ERROR;
 }
 
+#ifndef NDEBUG
 // When tracing, print state information.
 if (_LIBUNWIND_TRACING_UNWINDING) {
   char functionBuf[512];
@@ -704,6 +709,7 @@ unwind_phase2_forced(unw_context_t *uc, unw_cursor_t 
*cursor,
   (void *)exception_object, frameInfo.start_ip, functionName,
   frameInfo.lsda, frameInfo.handler);
 }
+#endif
 
 // Call stop function at each frame.
 _Unwind_Action action =

diff  --git a/libunwind/src/Unwind-seh.cpp b/libunwind/src/Unwind-seh.cpp
index 6e2b4e73e41e..5a6a719730c8 100644
--- a/libunwind/src/Unwind-seh.cpp
+++ b/libunwind/src/Unwind-seh.cpp
@@ -244,6 +244,7 @@ unwind_phase2_forced(unw_context_t *uc,
   return _URC_FATAL_PHASE2_ERROR;
 }
 
+#ifndef NDEBUG
 // When tracing, print state information.
 if (_LIBUNWIND_TRACING_UNWINDING) {
   char functionBuf[512];
@@ -259,6 +260,7 @@ unwind_phase2_forced(unw_context_t *uc,
   (void *)exception_object, frameInfo.start_ip, functionName,
   frameInfo.lsda, frameInfo.handler);
 }
+#endif
 
 // Call stop function at each frame.
 _Unwind_Action action =

diff  --git a/libunwind/src/UnwindLevel1.c b/libunwind/src/UnwindLevel1.c
index 68e5e48b8c05..8b8797fb88ad 100644
--- a/libunwind/src/UnwindLevel1.c
+++ b/libunwind/src/UnwindLevel1.c
@@ -68,6 +68,7 @@ unwind_phase1(unw_context_t *uc, unw_cursor_t *cursor, 
_Unwind_Exception *except
   return _URC_FATAL_PHASE1_ERROR;
 }
 
+#ifndef NDEBUG
 // When tracing, print state information.
 if (_LIBUNWIND_TRACING_UNWINDING) {
   char functionBuf[512];
@@ -85,6 +86,7 @@ unwind_phase1(unw_context_t *uc, unw_cursor_t *cursor, 
_Unwind_Exception *except
   (void *)exception_object, pc, frameInfo.start_ip, functionName,
   frameInfo.lsda, frameInfo.handler);
 }
+#endif
 
 // If there is a personality routine, ask it if it will want to stop at
 // this frame.
@@ -167,6 +169,7 @@ unwind_phase2(unw_context_t *uc, unw_cursor_t *cursor, 
_Unwind_Exception *except
   return _URC_FATAL_PHASE2_ERROR;
 }
 
+#ifndef NDEBUG
 // When tracing, print 

[libunwind] db126ae - [Arm][Unwind][libc++abi] Add _Unwind_ForcedUnwind to EHABI.

[Daniel Kiss via cfe-commits]

Author: Daniel Kiss
Date: 2021-08-11T10:15:53+02:00
New Revision: db126ae243cd70e4f68fd50a7c619740e90e1dc6

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

LOG: [Arm][Unwind][libc++abi] Add _Unwind_ForcedUnwind to EHABI.

_Unwind_ForcedUnwind is not mandated by the EHABI but for compatibilty
reasons adding so the interface to higher layers would be the same.
Dropping EHABI specific _Unwind_Stop_Fn definition since it is not defined by 
EHABI.

Reviewed By: MaskRay

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

Added: 
libunwind/test/forceunwind.pass.cpp

Modified: 
libcxxabi/src/cxa_personality.cpp
libcxxabi/test/forced_unwind1.pass.cpp
libcxxabi/test/forced_unwind2.pass.cpp
libunwind/include/unwind.h
libunwind/include/unwind_arm_ehabi.h
libunwind/include/unwind_itanium.h
libunwind/src/Unwind-EHABI.cpp
libunwind/src/UnwindLevel1-gcc-ext.c

Removed: 




diff  --git a/libcxxabi/src/cxa_personality.cpp 
b/libcxxabi/src/cxa_personality.cpp
index a4f81d74735f1..d63741b19b3dc 100644
--- a/libcxxabi/src/cxa_personality.cpp
+++ b/libcxxabi/src/cxa_personality.cpp
@@ -1109,7 +1109,14 @@ __gxx_personality_v0(_Unwind_State state,
 // Either we didn't do a phase 1 search (due to forced unwinding), or
 //  phase 1 reported no catching-handlers.
 // Search for a (non-catching) cleanup
-scan_eh_tab(results, _UA_CLEANUP_PHASE, native_exception, 
unwind_exception, context);
+if (is_force_unwinding)
+  scan_eh_tab(
+  results,
+  static_cast<_Unwind_Action>(_UA_CLEANUP_PHASE | 
_UA_FORCE_UNWIND),
+  native_exception, unwind_exception, context);
+else
+  scan_eh_tab(results, _UA_CLEANUP_PHASE, native_exception,
+  unwind_exception, context);
 if (results.reason == _URC_HANDLER_FOUND)
 {
 // Found a non-catching handler

diff  --git a/libcxxabi/test/forced_unwind1.pass.cpp 
b/libcxxabi/test/forced_unwind1.pass.cpp
index 69f93ffaacc0c..b6963a0242996 100644
--- a/libcxxabi/test/forced_unwind1.pass.cpp
+++ b/libcxxabi/test/forced_unwind1.pass.cpp
@@ -20,11 +20,6 @@
 #include 
 #include <__cxxabi_config.h>
 
-#if defined(_LIBCXXABI_ARM_EHABI)
-int main(int, char**) {
-  return 0;
-}
-#else
 static int bits = 0;
 
 struct C {
@@ -84,4 +79,3 @@ int main(int, char**) {
   test();
   return bits != 15;
 }
-#endif

diff  --git a/libcxxabi/test/forced_unwind2.pass.cpp 
b/libcxxabi/test/forced_unwind2.pass.cpp
index cb527581687a1..037f0499282f6 100644
--- a/libcxxabi/test/forced_unwind2.pass.cpp
+++ b/libcxxabi/test/forced_unwind2.pass.cpp
@@ -21,11 +21,6 @@
 #include 
 #include <__cxxabi_config.h>
 
-#if defined(_LIBCXXABI_ARM_EHABI)
-int main(int, char**) {
-  return 0;
-}
-#else
 template 
 struct Stop;
 
@@ -64,4 +59,3 @@ int main(int, char**) {
   }
   abort();
 }
-#endif

diff  --git a/libunwind/include/unwind.h b/libunwind/include/unwind.h
index e8d114854325c..87c3cf6c804ec 100644
--- a/libunwind/include/unwind.h
+++ b/libunwind/include/unwind.h
@@ -61,6 +61,14 @@ typedef struct _Unwind_Context _Unwind_Context;   // opaque
 #include "unwind_itanium.h"
 #endif
 
+typedef _Unwind_Reason_Code (*_Unwind_Stop_Fn)
+(int version,
+ _Unwind_Action actions,
+ uint64_t exceptionClass,
+ _Unwind_Exception* exceptionObject,
+ struct _Unwind_Context* context,
+ void* stop_parameter);
+
 #ifdef __cplusplus
 extern "C" {
 #endif

diff  --git a/libunwind/include/unwind_arm_ehabi.h 
b/libunwind/include/unwind_arm_ehabi.h
index 58444d14eb8dc..5ad0887225605 100644
--- a/libunwind/include/unwind_arm_ehabi.h
+++ b/libunwind/include/unwind_arm_ehabi.h
@@ -26,7 +26,7 @@ typedef uint32_t _Unwind_EHT_Header;
 
 struct _Unwind_Control_Block;
 typedef struct _Unwind_Control_Block _Unwind_Control_Block;
-typedef struct _Unwind_Control_Block _Unwind_Exception; /* Alias */
+#define _Unwind_Exception _Unwind_Control_Block /* Alias */
 
 struct _Unwind_Control_Block {
   uint64_t exception_class;
@@ -63,11 +63,6 @@ struct _Unwind_Control_Block {
   long long int :0; /* Enforce the 8-byte alignment */
 } __attribute__((__aligned__(8)));
 
-typedef _Unwind_Reason_Code (*_Unwind_Stop_Fn)
-  (_Unwind_State state,
-   _Unwind_Exception* exceptionObject,
-   struct _Unwind_Context* context);
-
 typedef _Unwind_Reason_Code (*_Unwind_Personality_Fn)(
 _Unwind_State state, _Unwind_Exception *exceptionObject,
 struct _Unwind_Context *context);

diff  --git a/libunwind/include/unwind_itanium.h 
b/libunwind/include/unwind_itanium.h
index 1e1389c7f0da0..eeb45f6228323 100644
--- a/libunwind/include/unwind_itanium.h
+++ b/libunwind/include/unwind_itanium.h
@@ -39,14 +39,6 @@ struct _Unwind_Exception {
   // alignment 

[libunwind] 9ed1c7e - [Unwind] Split unwind.h

[Daniel Kiss via cfe-commits]

Author: Daniel Kiss
Date: 2021-08-11T10:15:51+02:00
New Revision: 9ed1c7e4964382b95a5886279c0dfc7147a57b17

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

LOG: [Unwind] Split unwind.h

Moving Itanium and ArmEHABI specific implementations to dedicated files.
This is a NFC patch.

Reviewed By: MaskRay

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

Added: 
libunwind/include/unwind_arm_ehabi.h
libunwind/include/unwind_itanium.h

Modified: 
libunwind/include/unwind.h
libunwind/src/CMakeLists.txt

Removed: 




diff  --git a/libunwind/include/unwind.h b/libunwind/include/unwind.h
index 1d3444cd83b46..e8d114854325c 100644
--- a/libunwind/include/unwind.h
+++ b/libunwind/include/unwind.h
@@ -56,211 +56,15 @@ typedef enum {
 typedef struct _Unwind_Context _Unwind_Context;   // opaque
 
 #if defined(_LIBUNWIND_ARM_EHABI)
-typedef uint32_t _Unwind_State;
-
-static const _Unwind_State _US_VIRTUAL_UNWIND_FRAME   = 0;
-static const _Unwind_State _US_UNWIND_FRAME_STARTING  = 1;
-static const _Unwind_State _US_UNWIND_FRAME_RESUME= 2;
-static const _Unwind_State _US_ACTION_MASK= 3;
-/* Undocumented flag for force unwinding. */
-static const _Unwind_State _US_FORCE_UNWIND   = 8;
-
-typedef uint32_t _Unwind_EHT_Header;
-
-struct _Unwind_Control_Block;
-typedef struct _Unwind_Control_Block _Unwind_Control_Block;
-typedef struct _Unwind_Control_Block _Unwind_Exception; /* Alias */
-
-struct _Unwind_Control_Block {
-  uint64_t exception_class;
-  void (*exception_cleanup)(_Unwind_Reason_Code, _Unwind_Control_Block*);
-
-  /* Unwinder cache, private fields for the unwinder's use */
-  struct {
-uint32_t reserved1; /* init reserved1 to 0, then don't touch */
-uint32_t reserved2;
-uint32_t reserved3;
-uint32_t reserved4;
-uint32_t reserved5;
-  } unwinder_cache;
-
-  /* Propagation barrier cache (valid after phase 1): */
-  struct {
-uint32_t sp;
-uint32_t bitpattern[5];
-  } barrier_cache;
-
-  /* Cleanup cache (preserved over cleanup): */
-  struct {
-uint32_t bitpattern[4];
-  } cleanup_cache;
-
-  /* Pr cache (for pr's benefit): */
-  struct {
-uint32_t fnstart; /* function start address */
-_Unwind_EHT_Header* ehtp; /* pointer to EHT entry header word */
-uint32_t additional;
-uint32_t reserved1;
-  } pr_cache;
-
-  long long int :0; /* Enforce the 8-byte alignment */
-} __attribute__((__aligned__(8)));
-
-typedef _Unwind_Reason_Code (*_Unwind_Stop_Fn)
-  (_Unwind_State state,
-   _Unwind_Exception* exceptionObject,
-   struct _Unwind_Context* context);
-
-typedef _Unwind_Reason_Code (*_Unwind_Personality_Fn)(
-_Unwind_State state, _Unwind_Exception *exceptionObject,
-struct _Unwind_Context *context);
-#else
-struct _Unwind_Context;   // opaque
-struct _Unwind_Exception; // forward declaration
-typedef struct _Unwind_Exception _Unwind_Exception;
-
-struct _Unwind_Exception {
-  uint64_t exception_class;
-  void (*exception_cleanup)(_Unwind_Reason_Code reason,
-_Unwind_Exception *exc);
-#if defined(__SEH__) && !defined(__USING_SJLJ_EXCEPTIONS__)
-  uintptr_t private_[6];
+#include "unwind_arm_ehabi.h"
 #else
-  uintptr_t private_1; // non-zero means forced unwind
-  uintptr_t private_2; // holds sp that phase1 found for phase2 to use
-#endif
-#if __SIZEOF_POINTER__ == 4
-  // The implementation of _Unwind_Exception uses an attribute mode on the
-  // above fields which has the side effect of causing this whole struct to
-  // round up to 32 bytes in size (48 with SEH). To be more explicit, we add
-  // pad fields added for binary compatibility.
-  uint32_t reserved[3];
-#endif
-  // The Itanium ABI requires that _Unwind_Exception objects are "double-word
-  // aligned".  GCC has interpreted this to mean "use the maximum useful
-  // alignment for the target"; so do we.
-} __attribute__((__aligned__));
-
-typedef _Unwind_Reason_Code (*_Unwind_Stop_Fn)
-(int version,
- _Unwind_Action actions,
- uint64_t exceptionClass,
- _Unwind_Exception* exceptionObject,
- struct _Unwind_Context* context,
- void* stop_parameter );
-
-typedef _Unwind_Reason_Code (*_Unwind_Personality_Fn)(
-int version, _Unwind_Action actions, uint64_t exceptionClass,
-_Unwind_Exception *exceptionObject, struct _Unwind_Context *context);
+#include "unwind_itanium.h"
 #endif
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
-//
-// The following are the base functions documented by the C++ ABI
-//
-#ifdef __USING_SJLJ_EXCEPTIONS__
-extern _Unwind_Reason_Code
-_Unwind_SjLj_RaiseException(_Unwind_Exception *exception_object);
-extern void _Unwind_SjLj_Resume(_Unwind_Exception *exception_object);
-#else
-extern _Unwind_Reason_Code
-