[PATCH] D105426: [clangd] IWYU as a library: Find all references to symbols in the file

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 365094.
kbobyrev marked 15 inline comments as done.
kbobyrev added a comment.

Thank you for the great suggestions! Indeed, we already have the confusion
regarding IWYU having the CLI flag and users are thinking we provide some sort
of integration/implementation. Having even more confusion (implementing
similar) functionality for our needs is not what we want (also, apologies for
confusion,"as a library" was more about the patch in this context - meaning not
a "usable" feature yet but a smaller scope where we just start the
implementation as the library that can only be used in tests for now). Hence,
renamed the feature to "Include Sanity" (close to Include Hygiene but sounds
better IMO, please let me know if you think otherwise).

I have addressed the review comments and will base my next patch on this (this
patch is probably mostly the blocker for the upcoming ones).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105426

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/IncludeSanity.cpp
  clang-tools-extra/clangd/IncludeSanity.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/IncludeSanityTests.cpp

Index: clang-tools-extra/clangd/unittests/IncludeSanityTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/IncludeSanityTests.cpp
@@ -0,0 +1,136 @@
+//===--- IWYUTests.cpp --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Annotations.h"
+#include "IncludeSanity.h"
+#include "TestTU.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(IWYU, ReferencedLocations) {
+  struct TestCase {
+std::string HeaderCode;
+std::string MainCode;
+  };
+  TestCase Cases[] = {
+  // DeclRefExpr
+  {
+  "int ^x();",
+  "int y = x();",
+  },
+  // RecordDecl
+  {
+  "class ^X;",
+  "X *y;",
+  },
+  // TypedefType and UsingDecls
+  {
+  "using ^Integer = int;",
+  "Integer x;",
+  },
+  {
+  "namespace ns { struct ^X; struct ^X {}; }",
+  "using ns::X;",
+  },
+  {
+  "namespace ns { struct X; struct X {}; }",
+  "using namespace ns;",
+  },
+  {
+  "struct ^A {}; using B = A; using ^C = B;",
+  "C a;",
+  },
+  {
+  "typedef bool ^Y; template  struct ^X {};",
+  "X x;",
+  },
+  {
+  "struct Foo; struct ^Foo{}; typedef Foo ^Bar;",
+  "Bar b;",
+  },
+  // MemberExpr
+  {
+  "struct ^X{int ^a;}; X ^foo();",
+  "int y = foo().a;",
+  },
+  // Expr (type is traversed)
+  {
+  "class ^X{}; X ^foo();",
+  "auto bar() { return foo(); }",
+  },
+  // Redecls
+  {
+  "class ^X; class ^X{}; class ^X;",
+  "X *y;",
+  },
+  // Constructor
+  {
+  "struct ^X { ^X(int) {} int ^foo(); };",
+  "auto x = X(42); auto y = x.foo();",
+  },
+  // Static function
+  {
+  "struct ^X { static bool ^foo(); }; bool X::^foo() {}",
+  "auto b = X::foo();",
+  },
+  // TemplateRecordDecl
+  {
+  "template  class ^X;",
+  "X *y;",
+  },
+  // Type name not spelled out in code
+  {
+  "class ^X{}; X ^getX();",
+  "auto x = getX();",
+  },
+  // Enums
+  {
+  "enum ^Color { ^Red = 42, Green = 9000};",
+  "int MyColor = Red;",
+  },
+  {
+  "struct ^X { enum ^Language { ^CXX = 42, Python = 9000}; };",
+  "int Lang = X::CXX;",
+  },
+  {
+  // When a type is resolved via a using declaration, the
+  // UsingShadowDecl is not referenced in the AST.
+  // Compare to TypedefType, or DeclRefExpr::getFoundDecl().
+  // ^
+  "namespace ns { class ^X; }; using ns::X;",
+  "X *y;",
+  }};
+  for (const TestCase &T : Cases) {
+TestTU TU;
+TU.Code = T.MainCode;
+Annotations Header(T.HeaderCode);
+TU.HeaderCode = Header.code().str();
+auto AST = TU.build();
+
+std::vector Points;
+for (const auto &Loc : findReferencedLocations(AST)) {
+  if (AST.getSourceManager().getBufferName(Loc).endswith(
+  TU.HeaderFilename)) {
+Points.push_back(offsetToPosition(
+TU.HeaderCode, AST.getSourceManager().getFileOffset(Loc)));
+  }
+}
+  

[PATCH] D107137: clangd: Provide hover info for include directives

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added a comment.

Thanks for the review. Can you please merge it? I don't have the permissions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107137

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

[PATCH] D101641: [Sema] Preserve invalid CXXCtorInitializers using RecoveryExpr in initializer

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:4475
   //   name that denotes that base class type.
-  bool Dependent = BaseType->isDependentType() || Init->isTypeDependent();
+  bool Dependent = CurContext->isDependentContext() &&
+   (BaseType->isDependentType() || Init->isTypeDependent());

sammccall wrote:
> hokein wrote:
> > it is unclear to me why we need a `isDependentContext()` here? looks like 
> > this would special-case RecoveryExpr, the following dependent code-path 
> > would not be executed for RecoveryExpr case? 
> Right, if the init expr is type-dependent because it contains errors, but 
> we're not actually in a template, we don't want to take the dependent path.
> 
> The reason is that the dependent path produces an "as-written" form of the 
> CXXCtorInitializer, which has weaker invariants, because we know we're not 
> going to examine it much until template instantiation time.
> e.g. an initializer that is actually delegating is stored as a base 
> initializer instead. (Thus avoiding the impossible requirement to check 
> whether two dependent types are the same).
> Later on in Sema::SetCtorInitializers, we check 
> `Constructor->isDependentContext()` to decide whether to propagate the 
> "as-written" initializers without checking, but of course this is false. So 
> then we go and shuffle the initializers into the correct order, build default 
> initializers for the uninitialized members etc, and the delegating 
> initializer is just... lost in the shuffle. We think it's a base init, so 
> it's put in a map of type->init, and then it never gets consumed from there 
> because the type is not actually a base class.
> 
> I'll try to add a comment that hints at this, without getting into the 
> concrete details...
I see, fair enough, thanks for the classifications. 



Comment at: clang/test/AST/ast-dump-recovery.cpp:305
+S s;
+MemberInit() : x(invalid), y(invalid, invalid), s(1,2) {}
+// CHECK:  CXXConstructorDecl {{.*}} MemberInit 'void ()'

For completeness, I would add case 'x(undefine())', even though it already 
works without this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101641

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

[PATCH] D105263: [X86] AVX512FP16 instructions enabling 1/6

[LuoYuanke via Phabricator via cfe-commits]

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

LGTM, but may wait 1 or 2 days for the comments from others.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105263

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

[PATCH] D105331: [CFE][X86] Enable complex _Float16.

[LuoYuanke via Phabricator via cfe-commits]

LuoYuanke added a comment.

Would you check the failure of the test cases?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105331

___
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

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D104854#2932444 , @nemanjai wrote:

> Rather than reverting this commit again, I pushed 62fe3dcf98d1 
>  to use 
> the same expansion as before (using unordered comparison) for `ppc_fp128`.

Thank you very much for fixing that!

> I am not sure if there are other types that suffer from the same problem 
> (perhaps the Intel 80-bit long double).

Intel `fp80` is also non-IEEE type but it got custom lowering in this patch. 
There is little chance for such type to work properly without custom lowering.

I am working on patch that would add custom lowering of `llvm.isnan` to PowerPC.




Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:6964
+return DAG.getSetCC(DL, ResultVT, Op, DAG.getConstantFP(0.0, DL, 
OperandVT),
+ISD::SETUO);
+

nemanjai wrote:
> sepavloff wrote:
> > efriedma wrote:
> > > Maybe we want to consider falling back to the integer path if SETCC isn't 
> > > legal for the given operand type?  We could do that as a followup, though.
> > It makes sense, it could be beneficial for targets that have limited set of 
> > floating point comparisons. However straightforward check like:
> > 
> > if (Flags.hasNoFPExcept() && isOperationLegalOrCustom(ISD::SETCC, 
> > OperandVT))
> > 
> > results in worse code, mainly for vector types. It should be more complex 
> > check.
> >  
> Out of curiosity, why was this added when you recognized that it results in 
> worse code? This is certainly part of the reason for the regression for 
> `ppc_fp128`.
> 
> It would appear that before this patch, we would emit a comparison for all 
> types that are not IEEE FP types (such as `ppc_fp128`). Those semantics do 
> not seem to have carried over.
> Out of curiosity, why was this added when you recognized that it results in 
> worse code? This is certainly part of the reason for the regression for 
> ppc_fp128.

It is my mistake. After experiments I forgot to remove this change. I am sorry.

For x86 and AArch64 I used modified `test-suite`, with changes from D106804. 
Without proper tests it is hard to reveal why one intrinsic starts to fail.

> It would appear that before this patch, we would emit a comparison for all 
> types that are not IEEE FP types (such as ppc_fp128). Those semantics do not 
> seem to have carried over.

The previous behavior is not correct in non-default FP environment. Unordered 
comparison raises Invalid exception if an operand is signaling NaN. On the 
other hand, `isnan` must never raise exceptions.


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

[clang] 5f99670 - [RISCV] Half-precision for vget/vset.

[Hsiangkai Wang via cfe-commits]

Author: Hsiangkai Wang
Date: 2021-08-09T17:38:15+08:00
New Revision: 5f996705e0cadc43771da12dec0670680230ca56

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

LOG: [RISCV] Half-precision for vget/vset.

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

Added: 


Modified: 
clang/include/clang/Basic/riscv_vector.td
clang/test/CodeGen/RISCV/rvv-intrinsics/vget.c
clang/test/CodeGen/RISCV/rvv-intrinsics/vset.c

Removed: 




diff  --git a/clang/include/clang/Basic/riscv_vector.td 
b/clang/include/clang/Basic/riscv_vector.td
index 48c032dd1422b..ebe32860882a4 100644
--- a/clang/include/clang/Basic/riscv_vector.td
+++ b/clang/include/clang/Basic/riscv_vector.td
@@ -2086,7 +2086,7 @@ let HasMask = false, HasVL = false, IRName = "" in {
   }
   }] in {
 foreach dst_lmul = ["(SFixedLog2LMUL:0)", "(SFixedLog2LMUL:1)", 
"(SFixedLog2LMUL:2)"] in {
-  def : RVVBuiltin<"v" # dst_lmul # "v", dst_lmul # "vvKz", "csilfd", 
dst_lmul # "v">;
+  def : RVVBuiltin<"v" # dst_lmul # "v", dst_lmul # "vvKz", "csilxfd", 
dst_lmul # "v">;
   def : RVVBuiltin<"Uv" # dst_lmul # "Uv", dst_lmul # "UvUvKz", "csil", 
dst_lmul # "Uv">;
 }
   }
@@ -2105,7 +2105,7 @@ let HasMask = false, HasVL = false, IRName = "" in {
   }
   }] in {
 foreach dst_lmul = ["(LFixedLog2LMUL:1)", "(LFixedLog2LMUL:2)", 
"(LFixedLog2LMUL:3)"] in {
-  def : RVVBuiltin<"v" # dst_lmul # "v", dst_lmul # "v" # dst_lmul # 
"vKzv", "csilfd">;
+  def : RVVBuiltin<"v" # dst_lmul # "v", dst_lmul # "v" # dst_lmul # 
"vKzv", "csilxfd">;
   def : RVVBuiltin<"Uv" # dst_lmul # "Uv", dst_lmul # "Uv" # dst_lmul 
#"UvKzUv", "csil">;
 }
   }

diff  --git a/clang/test/CodeGen/RISCV/rvv-intrinsics/vget.c 
b/clang/test/CodeGen/RISCV/rvv-intrinsics/vget.c
index ac287ff294019..4526d1891bcfb 100644
--- a/clang/test/CodeGen/RISCV/rvv-intrinsics/vget.c
+++ b/clang/test/CodeGen/RISCV/rvv-intrinsics/vget.c
@@ -1,6 +1,7 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // REQUIRES: riscv-registered-target
-// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d 
-target-feature +experimental-v \
+// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d \
+// RUN:   -target-feature +experimental-v -target-feature +experimental-zfh \
 // RUN:   -disable-O0-optnone -emit-llvm %s -o - | opt -S -mem2reg | FileCheck 
--check-prefix=CHECK-RV64 %s
 
 #include 
@@ -544,3 +545,57 @@ vfloat64m2_t test_vget_v_f64m8_f64m2(vfloat64m8_t src) {
 vfloat64m4_t test_vget_v_f64m8_f64m4(vfloat64m8_t src) {
   return vget_v_f64m8_f64m4(src, 1);
 }
+
+// CHECK-RV64-LABEL: @test_vget_v_f16m2_f16m1(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  
@llvm.experimental.vector.extract.nxv4f16.nxv8f16( 
[[SRC:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m1_t test_vget_v_f16m2_f16m1 (vfloat16m2_t src) {
+  return vget_v_f16m2_f16m1(src, 0);
+}
+
+// CHECK-RV64-LABEL: @test_vget_v_f16m4_f16m1(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  
@llvm.experimental.vector.extract.nxv4f16.nxv16f16( 
[[SRC:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m1_t test_vget_v_f16m4_f16m1 (vfloat16m4_t src) {
+  return vget_v_f16m4_f16m1(src, 0);
+}
+
+// CHECK-RV64-LABEL: @test_vget_v_f16m8_f16m1(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  
@llvm.experimental.vector.extract.nxv4f16.nxv32f16( 
[[SRC:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m1_t test_vget_v_f16m8_f16m1 (vfloat16m8_t src) {
+  return vget_v_f16m8_f16m1(src, 0);
+}
+
+// CHECK-RV64-LABEL: @test_vget_v_f16m4_f16m2(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  
@llvm.experimental.vector.extract.nxv8f16.nxv16f16( 
[[SRC:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m2_t test_vget_v_f16m4_f16m2 (vfloat16m4_t src) {
+  return vget_v_f16m4_f16m2(src, 0);
+}
+
+// CHECK-RV64-LABEL: @test_vget_v_f16m8_f16m2(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  
@llvm.experimental.vector.extract.nxv8f16.nxv32f16( 
[[SRC:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m2_t test_vget_v_f16m8_f16m2 (vfloat16m8_t src) {
+  return vget_v_f16m8_f16m2(src, 0);
+}
+
+// CHECK-RV64-LABEL: @test_vget_v_f16m8_f16m4(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  
@llvm.experimental.vector.extract.nxv16f16.nxv32f16( 
[[SRC:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m4_t test_vget_v_f16m8_f16m4 (vfloat16m8_t src) {
+  return vget_v_f16m8_f16m4(src, 0);
+}

diff  --git a/clang/test/CodeGen/RISCV/rvv-intrinsics/vset.c 
b/clang/test/CodeGen/RISCV/rvv-intrinsics/

[PATCH] D107433: [RISCV] Half-precision for vget/vset.

[Hsiangkai Wang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5f996705e0ca: [RISCV] Half-precision for vget/vset. 
(authored by HsiangKai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107433

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics/vget.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vset.c

Index: clang/test/CodeGen/RISCV/rvv-intrinsics/vset.c
===
--- clang/test/CodeGen/RISCV/rvv-intrinsics/vset.c
+++ clang/test/CodeGen/RISCV/rvv-intrinsics/vset.c
@@ -1,6 +1,7 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // REQUIRES: riscv-registered-target
-// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d -target-feature +experimental-v \
+// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d \
+// RUN:   -target-feature +experimental-v -target-feature +experimental-zfh \
 // RUN:   -disable-O0-optnone -emit-llvm %s -o - | opt -S -mem2reg | FileCheck --check-prefix=CHECK-RV64 %s
 
 #include 
@@ -544,3 +545,57 @@
 vfloat64m8_t test_vset_v_f64m4_f64m8(vfloat64m8_t dest, vfloat64m4_t val) {
   return vset_v_f64m4_f64m8(dest, 1, val);
 }
+
+// CHECK-RV64-LABEL: @test_vset_v_f16m1_f16m2(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.experimental.vector.insert.nxv8f16.nxv4f16( [[DEST:%.*]],  [[VAL:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m2_t test_vset_v_f16m1_f16m2 (vfloat16m2_t dest, vfloat16m1_t val) {
+  return vset_v_f16m1_f16m2(dest, 0, val);
+}
+
+// CHECK-RV64-LABEL: @test_vset_v_f16m1_f16m4(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.experimental.vector.insert.nxv16f16.nxv4f16( [[DEST:%.*]],  [[VAL:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m4_t test_vset_v_f16m1_f16m4 (vfloat16m4_t dest, vfloat16m1_t val) {
+  return vset_v_f16m1_f16m4(dest, 0, val);
+}
+
+// CHECK-RV64-LABEL: @test_vset_v_f16m2_f16m4(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.experimental.vector.insert.nxv16f16.nxv8f16( [[DEST:%.*]],  [[VAL:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m4_t test_vset_v_f16m2_f16m4 (vfloat16m4_t dest, vfloat16m2_t val) {
+  return vset_v_f16m2_f16m4(dest, 0, val);
+}
+
+// CHECK-RV64-LABEL: @test_vset_v_f16m1_f16m8(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.experimental.vector.insert.nxv32f16.nxv4f16( [[DEST:%.*]],  [[VAL:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m8_t test_vset_v_f16m1_f16m8 (vfloat16m8_t dest, vfloat16m1_t val) {
+  return vset_v_f16m1_f16m8(dest, 0, val);
+}
+
+// CHECK-RV64-LABEL: @test_vset_v_f16m2_f16m8(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.experimental.vector.insert.nxv32f16.nxv8f16( [[DEST:%.*]],  [[VAL:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m8_t test_vset_v_f16m2_f16m8 (vfloat16m8_t dest, vfloat16m2_t val) {
+  return vset_v_f16m2_f16m8(dest, 0, val);
+}
+
+// CHECK-RV64-LABEL: @test_vset_v_f16m4_f16m8(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.experimental.vector.insert.nxv32f16.nxv16f16( [[DEST:%.*]],  [[VAL:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m8_t test_vset_v_f16m4_f16m8 (vfloat16m8_t dest, vfloat16m4_t val) {
+  return vset_v_f16m4_f16m8(dest, 0, val);
+}
Index: clang/test/CodeGen/RISCV/rvv-intrinsics/vget.c
===
--- clang/test/CodeGen/RISCV/rvv-intrinsics/vget.c
+++ clang/test/CodeGen/RISCV/rvv-intrinsics/vget.c
@@ -1,6 +1,7 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // REQUIRES: riscv-registered-target
-// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d -target-feature +experimental-v \
+// RUN: %clang_cc1 -triple riscv64 -target-feature +f -target-feature +d \
+// RUN:   -target-feature +experimental-v -target-feature +experimental-zfh \
 // RUN:   -disable-O0-optnone -emit-llvm %s -o - | opt -S -mem2reg | FileCheck --check-prefix=CHECK-RV64 %s
 
 #include 
@@ -544,3 +545,57 @@
 vfloat64m4_t test_vget_v_f64m8_f64m4(vfloat64m8_t src) {
   return vget_v_f64m8_f64m4(src, 1);
 }
+
+// CHECK-RV64-LABEL: @test_vget_v_f16m2_f16m1(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.experimental.vector.extract.nxv4f16.nxv8f16( [[SRC:%.*]], i64 0)
+// CHECK-RV64-NEXT:ret  [[TMP0]]
+//
+vfloat16m1_t test_vget_v_f16m2_f16m1 (vfloat16m2_t src) {
+  return vget_v_f16m2_f16m1(src, 0);
+}
+
+// CHECK-RV64-LABEL: @test_vget_v_f16m4_f16m1(
+// CHECK-RV64-NEXT:  entry:
+// CHECK-RV64-NEXT:[[TMP0:%.*]] = call  @llvm.experimental.vector.extract.nxv4f16.nxv16f16( [[SRC:%.*]], 

[PATCH] D107049: [clang-repl] Re-implement clang-interpreter as a test case.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev updated this revision to Diff 365134.
v.g.vassilev added a comment.

rebase


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

https://reviews.llvm.org/D107049

Files:
  clang/docs/ClangFormattedStatus.rst
  clang/examples/CMakeLists.txt
  clang/examples/clang-interpreter/CMakeLists.txt
  clang/examples/clang-interpreter/README.txt
  clang/examples/clang-interpreter/Test.cxx
  clang/include/clang/Interpreter/Interpreter.h
  clang/lib/Interpreter/IncrementalExecutor.cpp
  clang/lib/Interpreter/IncrementalExecutor.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/test/CMakeLists.txt
  clang/test/Misc/interpreter.c
  clang/test/lit.cfg.py
  clang/unittests/Interpreter/CMakeLists.txt
  clang/unittests/Interpreter/InterpreterTest.cpp

Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -14,10 +14,14 @@
 
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclGroup.h"
+#include "clang/Basic/Version.h"
+#include "clang/Config/config.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ExecutionEngine/Orc/LLJIT.h"
+#include "llvm/Support/TargetSelect.h"
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -53,6 +57,92 @@
   EXPECT_EQ(1U, DeclsSize(R2.TUPart));
 }
 
+// This function isn't referenced outside its translation unit, but it
+// can't use the "static" keyword because its address is used for
+// GetMainExecutable (since some platforms don't support taking the
+// address of main, and some platforms can't implement GetMainExecutable
+// without being given the address of a function in the main executable).
+std::string GetExecutablePath(const char *Argv0, void *MainAddr) {
+  return llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
+}
+
+static std::string MakeResourcesPath() {
+  // Dir is bin/ or lib/, depending on where BinaryPath is.
+  void *MainAddr = (void *)(intptr_t)GetExecutablePath;
+  std::string BinaryPath = GetExecutablePath(/*Argv0=*/nullptr, MainAddr);
+
+  // build/tools/clang/unittests/Interpreter/Executable -> build/
+  llvm::StringRef Dir = llvm::sys::path::parent_path(BinaryPath);
+
+  Dir = llvm::sys::path::parent_path(Dir);
+  Dir = llvm::sys::path::parent_path(Dir);
+  Dir = llvm::sys::path::parent_path(Dir);
+  Dir = llvm::sys::path::parent_path(Dir);
+  SmallString<128> P(Dir);
+  llvm::sys::path::append(P, Twine("lib") + CLANG_LIBDIR_SUFFIX, "clang",
+  CLANG_VERSION_STRING);
+
+  return std::string(P.str());
+}
+
+#ifdef _MSC_VER
+// Tell the windows linker to export the type_info symbol required by exceptions
+#pragma comment(linker, "/export:??_7type_info@@6B@")
+#endif // _MSC_VER
+TEST(InterpreterTest, CatchException) {
+  llvm::InitializeNativeTarget();
+  llvm::InitializeNativeTargetAsmPrinter();
+
+  {
+auto J = llvm::orc::LLJITBuilder().create();
+if (!J) {
+  // The platform does not support JITs.
+  llvm::consumeError(J.takeError());
+  return;
+}
+  }
+  const char ExceptionCode[] =
+  R"(
+#include 
+#include 
+
+static void ThrowerAnError(const char* Name) {
+  throw std::runtime_error(Name);
+}
+
+int main(int argc, const char** argv) {
+  // FIXME: Somehow when we build this test in release mode argc is not 0.
+  // printf("%d\n", argc);
+  // for (int I = 0; I < argc; ++I)
+  //   printf("arg[%d]='%s'\n", I, argv[I]);
+
+  try {
+ThrowerAnError("In JIT");
+  } catch (const std::exception& E) {
+printf("Caught: '%s'\n", E.what());
+  } catch (...) {
+printf("Unknown exception\n");
+  }
+  ThrowerAnError("From JIT");
+  return 0;
+}
+)";
+  std::string ResourceDir = MakeResourcesPath();
+  std::unique_ptr Interp =
+  createInterpreter({"-resource-dir", ResourceDir.c_str()});
+  // Adjust the resource-dir
+  llvm::cantFail(Interp->ParseAndExecute(ExceptionCode));
+#if !defined(NDEBUG) && GTEST_HAS_DEATH_TEST && !defined(_WIN32)
+  testing::internal::CaptureStdout();
+  auto Main = (int (*)(...))llvm::cantFail(Interp->getSymbolAddress("main"));
+  // Compiling this file with exceptions on is problematic on some platforms.
+  // Instead we expect std::terminate to be called upon thrown exception.
+  EXPECT_DEATH(Main(), "terminate called after throwing an instance of '.*'");
+#endif
+  std::string CapturedStdOut = testing::internal::GetCapturedStdout();
+  EXPECT_EQ(CapturedStdOut, "Caught: 'In JIT'\n");
+}
+
 static std::string DeclToString(Decl *D) {
   return llvm::cast(D)->getQualifiedNameAsString();
 }
Index: clang/unittests/Interpreter/CMakeLists.txt
===
--- clang/unittests/Interpreter/CMakeLists.txt
+++ clang/unittests/Interpreter/CMakeLists.txt
@@ -12,3 +12,4 @@
   clangInterpreter
   clangFrontend
   )
+add_dependencies(

[PATCH] D107078: [analyzer] Catch leaking stack addresses via stack variables

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

steakhal updated this revision to Diff 365133.
steakhal marked 2 inline comments as done.
steakhal added a comment.

Removed the extra note about the temporary expression.

---

In D107078#2933682 , @NoQ wrote:

> In D107078#2927899 , @steakhal 
> wrote:
>
>> I did not fix the extra blue 'bubble' note issue, and I think that is out of 
>> the scope of this patch.
>
> This is an on-by-default checker. We should not knowingly regress it, even if 
> temporarily.

I'm not exactly sure what would I regress, but to be on the safe side I won't 
emit any extra notes this time.

WDYT about the current form?


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

https://reviews.llvm.org/D107078

Files:
  clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  clang/test/Analysis/copy-elision.cpp
  clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
  clang/test/Analysis/loop-block-counts.c
  clang/test/Analysis/stack-addr-ps.cpp

Index: clang/test/Analysis/stack-addr-ps.cpp
===
--- clang/test/Analysis/stack-addr-ps.cpp
+++ clang/test/Analysis/stack-addr-ps.cpp
@@ -137,3 +137,28 @@
   }
 }
 
+void write_stack_address_to(char **q) {
+  char local;
+  *q = &local;
+  // expected-warning@-1 {{Address of stack memory associated with local \
+variable 'local' is still referred to by the stack variable 'p' upon \
+returning to the caller}}
+}
+
+void test_stack() {
+  char *p;
+  write_stack_address_to(&p);
+}
+
+struct C {
+  ~C() {} // non-trivial class
+};
+
+C make1() {
+  C c;
+  return c; // no-warning
+}
+
+void test_copy_elision() {
+  C c1 = make1();
+}
Index: clang/test/Analysis/loop-block-counts.c
===
--- clang/test/Analysis/loop-block-counts.c
+++ clang/test/Analysis/loop-block-counts.c
@@ -5,6 +5,9 @@
 void callee(void **p) {
   int x;
   *p = &x;
+  // expected-warning@-1 {{Address of stack memory associated with local \
+variable 'x' is still referred to by the stack variable 'arr' upon \
+returning to the caller}}
 }
 
 void loop() {
Index: clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
===
--- clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
+++ clang/test/Analysis/cxx-uninitialized-object-ptr-ref.cpp
@@ -71,6 +71,9 @@
   void *allocaPtr;
   int dontGetFilteredByNonPedanticMode = 0;
 
+  // expected-warning-re@+3 {{Address of stack memory allocated by call to \
+alloca() on line {{[0-9]+}} is still referred to by a temporary object on the \
+stack upon returning to the caller.  This will be a dangling reference}}
   UntypedAllocaTest() : allocaPtr(__builtin_alloca(sizeof(int))) {
 // All good!
   }
@@ -86,6 +89,9 @@
 
   TypedAllocaTest1() // expected-warning{{1 uninitialized field}}
   : allocaPtr(static_cast(__builtin_alloca(sizeof(int {}
+  // expected-warning-re@-2 {{Address of stack memory allocated by call to \
+alloca() on line {{[0-9]+}} is still referred to by a temporary object on the \
+stack upon returning to the caller.  This will be a dangling reference}}
 };
 
 void fTypedAllocaTest1() {
@@ -96,6 +102,9 @@
   int *allocaPtr;
   int dontGetFilteredByNonPedanticMode = 0;
 
+  // expected-warning-re@+5 {{Address of stack memory allocated by call to \
+alloca() on line {{[0-9]+}} is still referred to by a temporary object on the \
+stack upon returning to the caller.  This will be a dangling reference}}
   TypedAllocaTest2()
   : allocaPtr(static_cast(__builtin_alloca(sizeof(int {
 *allocaPtr = 5;
@@ -341,6 +350,9 @@
   void *&&vptrrref; // expected-note {{here}}
 
 public:
+  // expected-warning@+3 {{Address of stack memory associated with local \
+variable 'vptr' is still referred to by a temporary object on the stack \
+upon returning to the caller.  This will be a dangling reference}}
   VoidPointerRRefTest1(void *vptr, char) : vptrrref(static_cast(vptr)) { // expected-warning {{binding reference member 'vptrrref' to stack allocated parameter 'vptr'}}
 // All good!
   }
@@ -355,6 +367,9 @@
   void **&&vpptrrref; // expected-note {{here}}
 
 public:
+  // expected-warning@+3 {{Address of stack memory associated with local \
+variable 'vptr' is still referred to by a temporary object on the stack \
+upon returning to the caller.  This will be a dangling reference}}
   VoidPointerRRefTest2(void **vptr, char) : vpptrrref(static_cast(vptr)) { // expected-warning {{binding reference member 'vpptrrref' to stack allocated parameter 'vptr'}}
 // All good!
   }
@@ -369,6 +384,9 @@
   void *&vptrrref; // expected-note {{here}}
 
 public:
+  // expected-warning@+3 {{Address of stack memory associated with local \
+variable 'vptr' is still referred to by a temporary object on the stack \
+upon returning to the caller.  This will be a dangling reference}}

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added subscribers: djasper, klimek.
MyDeveloperDay added a comment.

In D69764#2932929 , @steveire wrote:

> 

@steveire, thanks for your comments, I also agree that a second tool shouldn't 
be needed, especially as this functionality is off by default (and needs to 
stay that way). This was my suggestion for a compromise.

Its a tough call, but ultimately no on ever gave this a LGTM so I couldn't 
proceed anyway. Whilst I think we can cover the other qualifiers with a similar 
approach I don't want to waste more of my time until the fundamental question 
has been answered if its ok for clang-format to insert/modify the order of 
tokens and that is more a collective decision.

It was suggested to me that I write up and RFC on the matter, I'm not a massive 
fan of those as I don't really see a formal process for handling them (the 
conversation seems to turn into a series of personal preferences then dwindles 
and dies without conclusion). But if people think it would solve something I 
guess I could try.

From my perspective I'm more interested in the views of the major clang-format 
contributors (@curdeius, @krasimir, @owenpan , @sammccall, @HazardyKnusperkeks  
and people like yourselves who have put time and effort into blogging about 
these tools), ultimately it will be us who have to handle the impact of this 
(and maybe the #clangd people) and I don't want to inconvenience them or make 
work for them.

So here is what is needed to land this in my view:

1. I'd want at least 2 to 3 of the current reviewers to LGTM.
2. I'd want 1 person (like yourself) a community contributor to also give their 
approval (I guess I have that conceptually from you!)

or

1. @klimek or @djasper to give us an its ok for "clang-format to insert/modify 
the order of tokens" in a controlled manner decision
2. 1 LGTM

Whilst I respect the views of those few who have objected, they don't to my 
knowledge make significant contributions to clang-format (not in the last 5-6 
years I've been following it anyway)

I feel this team owns the decision as we are the ones that look after it, warts 
and all. We'll always have a situation where some don't agree, thats ok, but I 
feel there is more agreement that this funcationality is ok rather than not.

Until one of those 2 things happens, we are in limbo.


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

[clang] f9ffe61 - [OpenCL] Factor out OpenCLBuiltinFileEmitterBase; NFC

[Sven van Haastregt via cfe-commits]

Author: Sven van Haastregt
Date: 2021-08-09T11:41:25+01:00
New Revision: f9ffe61fb53f595b7be136c340508c094482fcff

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

LOG: [OpenCL] Factor out OpenCLBuiltinFileEmitterBase; NFC

Factor out functionality that can be shared with a header file emitter
that is to be added in the future.

Added: 


Modified: 
clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp

Removed: 




diff  --git a/clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp 
b/clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
index a4cb5b7cacd98..457313f1839fd 100644
--- a/clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
+++ b/clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
@@ -233,19 +233,17 @@ class BuiltinNameEmitter {
   MapVector SignatureListMap;
 };
 
-// OpenCL builtin test generator.  This class processes the same TableGen input
-// as BuiltinNameEmitter, but generates a .cl file that contains a call to each
-// builtin function described in the .td input.
-class OpenCLBuiltinTestEmitter {
+/// Base class for emitting a file (e.g. header or test) from OpenCLBuiltins.td
+class OpenCLBuiltinFileEmitterBase {
 public:
-  OpenCLBuiltinTestEmitter(RecordKeeper &Records, raw_ostream &OS)
+  OpenCLBuiltinFileEmitterBase(RecordKeeper &Records, raw_ostream &OS)
   : Records(Records), OS(OS) {}
 
   // Entrypoint to generate the functions for testing all OpenCL builtin
   // functions.
-  void emit();
+  virtual void emit() = 0;
 
-private:
+protected:
   struct TypeFlags {
 TypeFlags() : IsConst(false), IsVolatile(false), IsPointer(false) {}
 bool IsConst : 1;
@@ -282,6 +280,18 @@ class OpenCLBuiltinTestEmitter {
   expandTypesInSignature(const std::vector &Signature,
  SmallVectorImpl> &Types);
 
+  // Emit extension enabling pragmas.
+  void emitExtensionSetup();
+
+  // Emit an #if guard for a Builtin's extension.  Return the corresponding
+  // closing #endif, or an empty string if no extension #if guard was emitted.
+  std::string emitExtensionGuard(const Record *Builtin);
+
+  // Emit an #if guard for a Builtin's language version.  Return the
+  // corresponding closing #endif, or an empty string if no version #if guard
+  // was emitted.
+  std::string emitVersionGuard(const Record *Builtin);
+
   // Contains OpenCL builtin functions and related information, stored as
   // Record instances. They are coming from the associated TableGen file.
   RecordKeeper &Records;
@@ -290,6 +300,19 @@ class OpenCLBuiltinTestEmitter {
   raw_ostream &OS;
 };
 
+// OpenCL builtin test generator.  This class processes the same TableGen input
+// as BuiltinNameEmitter, but generates a .cl file that contains a call to each
+// builtin function described in the .td input.
+class OpenCLBuiltinTestEmitter : public OpenCLBuiltinFileEmitterBase {
+public:
+  OpenCLBuiltinTestEmitter(RecordKeeper &Records, raw_ostream &OS)
+  : OpenCLBuiltinFileEmitterBase(Records, OS) {}
+
+  // Entrypoint to generate the functions for testing all OpenCL builtin
+  // functions.
+  void emit() override;
+};
+
 } // namespace
 
 void BuiltinNameEmitter::Emit() {
@@ -923,9 +946,9 @@ static void OCL2Qual(Sema &S, const OpenCLTypeStruct &Ty,
   OS << "\n} // OCL2Qual\n";
 }
 
-std::string OpenCLBuiltinTestEmitter::getTypeString(const Record *Type,
-TypeFlags Flags,
-int VectorSize) const {
+std::string OpenCLBuiltinFileEmitterBase::getTypeString(const Record *Type,
+TypeFlags Flags,
+int VectorSize) const {
   std::string S;
   if (Type->getValueAsBit("IsConst") || Flags.IsConst) {
 S += "const ";
@@ -970,7 +993,7 @@ std::string OpenCLBuiltinTestEmitter::getTypeString(const 
Record *Type,
   return S;
 }
 
-void OpenCLBuiltinTestEmitter::getTypeLists(
+void OpenCLBuiltinFileEmitterBase::getTypeLists(
 Record *Type, TypeFlags &Flags, std::vector &TypeList,
 std::vector &VectorList) const {
   bool isGenType = Type->isSubClassOf("GenericType");
@@ -1003,7 +1026,7 @@ void OpenCLBuiltinTestEmitter::getTypeLists(
   VectorList.push_back(Type->getValueAsInt("VecWidth"));
 }
 
-void OpenCLBuiltinTestEmitter::expandTypesInSignature(
+void OpenCLBuiltinFileEmitterBase::expandTypesInSignature(
 const std::vector &Signature,
 SmallVectorImpl> &Types) {
   // Find out if there are any GenTypes in this signature, and if so, calculate
@@ -1044,10 +1067,7 @@ void OpenCLBuiltinTestEmitter::expandTypesInSignature(
   }
 }
 
-void OpenCLBuiltinTestEmitter::emit() {
-  emitSourceFileHeader("OpenCL Builtin exhaustive testing", OS);
-
-  // Enable some ex

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

[Anshil Gandhi via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12120
+   OptimizationRemarkEmitter *ORE,
+   OptimizationRemark OptRemark) {
+  ORE->emit([&]() { return OptRemark; });

gandhi21299 wrote:
> rampitec wrote:
> > gandhi21299 wrote:
> > > rampitec wrote:
> > > > gandhi21299 wrote:
> > > > > rampitec wrote:
> > > > > > Why OptRemark and not just StringRef? I really want to see as 
> > > > > > little churn as possible at the call site.
> > > > > With only StringRef, we would also have to pass in RMW since 
> > > > > OptimizationRemark constructor depends on that.
> > > > It seems better be a lamda where you can just capture most of the stuff.
> > > What would the type be for the following lambda?
> > > 
> > > ```
> > > [&](){
> > > return OptimizationRemark(...);
> > > }
> > > ```
> > You need to return TargetLowering::AtomicExpansionKind, not 
> > OptimizationRemark.
> This is what goes into ORE->emit() as an argument though. I suspect the 
> remark won't be emitted if I return AtomicExpansionKind within the lambda.
@rampitec I don't think there is a way to pass the ORE->emit argument lambda 
expression into the reportAtomicExpand() function because of its capture. It 
needs access to RMW from the enclosing scope. (Well, I could define a copy of 
RMW and somehow pass it into the lambda but that seems too much for this task)


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

[Stanislav Mekhanoshin via Phabricator via cfe-commits]

rampitec added inline comments.



Comment at: llvm/lib/Target/AMDGPU/SIISelLowering.cpp:12120
+   OptimizationRemarkEmitter *ORE,
+   OptimizationRemark OptRemark) {
+  ORE->emit([&]() { return OptRemark; });

gandhi21299 wrote:
> gandhi21299 wrote:
> > rampitec wrote:
> > > gandhi21299 wrote:
> > > > rampitec wrote:
> > > > > gandhi21299 wrote:
> > > > > > rampitec wrote:
> > > > > > > Why OptRemark and not just StringRef? I really want to see as 
> > > > > > > little churn as possible at the call site.
> > > > > > With only StringRef, we would also have to pass in RMW since 
> > > > > > OptimizationRemark constructor depends on that.
> > > > > It seems better be a lamda where you can just capture most of the 
> > > > > stuff.
> > > > What would the type be for the following lambda?
> > > > 
> > > > ```
> > > > [&](){
> > > > return OptimizationRemark(...);
> > > > }
> > > > ```
> > > You need to return TargetLowering::AtomicExpansionKind, not 
> > > OptimizationRemark.
> > This is what goes into ORE->emit() as an argument though. I suspect the 
> > remark won't be emitted if I return AtomicExpansionKind within the lambda.
> @rampitec I don't think there is a way to pass the ORE->emit argument lambda 
> expression into the reportAtomicExpand() function because of its capture. It 
> needs access to RMW from the enclosing scope. (Well, I could define a copy of 
> RMW and somehow pass it into the lambda but that seems too much for this task)
Do not pass it there. Turn reportAtomicExpand into lambda.


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 364891.
gandhi21299 marked an inline comment as done.
gandhi21299 added a comment.

- turned reportAtomicExpand(...) into a lambda as requested


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:  Op

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

[Alf via Phabricator via cfe-commits]

gAlfonso-bit created this revision.
gAlfonso-bit added a project: LLVM.
Herald added subscribers: libcxx-commits, tstellar, mgorny.
Herald added a reviewer: MaskRay.
Herald added a project: libunwind.
Herald added a reviewer: libunwind.
gAlfonso-bit requested review of this revision.
Herald added projects: clang, Sanitizers, LLDB, libc++.
Herald added subscribers: llvm-commits, lldb-commits, Sanitizers, cfe-commits.
Herald added a reviewer: libc++.

This way, we do not need to set LLVM_CMAKE_PATH to LLVM_CMAKE_DIR when (NOT 
LLVM_CONFIG_FOUND)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107717

Files:
  clang/CMakeLists.txt
  clang/lib/Basic/CMakeLists.txt
  compiler-rt/cmake/Modules/CompilerRTMockLLVMCMakeConfig.cmake
  compiler-rt/cmake/Modules/CompilerRTUtils.cmake
  flang/CMakeLists.txt
  libcxx/cmake/Modules/HandleOutOfTreeLLVM.cmake
  libunwind/CMakeLists.txt
  lld/CMakeLists.txt
  lld/Common/CMakeLists.txt
  lldb/cmake/modules/LLDBStandalone.cmake
  lldb/source/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/include/llvm/Support/CMakeLists.txt
  runtimes/CMakeLists.txt

Index: runtimes/CMakeLists.txt
===
--- runtimes/CMakeLists.txt
+++ runtimes/CMakeLists.txt
@@ -65,7 +65,7 @@
 
 # This variable makes sure that e.g. llvm-lit is found.
 set(LLVM_MAIN_SRC_DIR ${LLVM_BUILD_MAIN_SRC_DIR})
-set(LLVM_CMAKE_PATH ${LLVM_MAIN_SRC_DIR}/cmake/modules)
+set(LLVM_CMAKE_DIR ${LLVM_MAIN_SRC_DIR}/cmake/modules)
 
 # This variable is used by individual runtimes to locate LLVM files.
 set(LLVM_PATH ${LLVM_BUILD_MAIN_SRC_DIR})
Index: llvm/include/llvm/Support/CMakeLists.txt
===
--- llvm/include/llvm/Support/CMakeLists.txt
+++ llvm/include/llvm/Support/CMakeLists.txt
@@ -3,7 +3,7 @@
 # The VC revision include that we want to generate.
 set(version_inc "${CMAKE_CURRENT_BINARY_DIR}/VCSRevision.h")
 
-set(generate_vcs_version_script "${LLVM_CMAKE_PATH}/GenerateVersionFromVCS.cmake")
+set(generate_vcs_version_script "${LLVM_CMAKE_DIR}/GenerateVersionFromVCS.cmake")
 
 if(LLVM_APPEND_VC_REV)
   set(llvm_source_dir ${LLVM_MAIN_SRC_DIR})
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -295,8 +295,8 @@
 set(LLVM_MAIN_INCLUDE_DIR ${LLVM_MAIN_SRC_DIR}/include ) # --includedir
 set(LLVM_BINARY_DIR   ${CMAKE_CURRENT_BINARY_DIR}  ) # --prefix
 
-# Note: LLVM_CMAKE_PATH does not include generated files
-set(LLVM_CMAKE_PATH ${LLVM_MAIN_SRC_DIR}/cmake/modules)
+# Note: LLVM_CMAKE_DIR does not include generated files
+set(LLVM_CMAKE_DIR ${LLVM_MAIN_SRC_DIR}/cmake/modules)
 set(LLVM_EXAMPLES_BINARY_DIR ${LLVM_BINARY_DIR}/examples)
 set(LLVM_INCLUDE_DIR ${CMAKE_CURRENT_BINARY_DIR}/include)
 
Index: lldb/source/CMakeLists.txt
===
--- lldb/source/CMakeLists.txt
+++ lldb/source/CMakeLists.txt
@@ -8,7 +8,7 @@
 find_first_existing_vc_file("${LLDB_SOURCE_DIR}" lldb_vc)
 
 set(version_inc "${CMAKE_CURRENT_BINARY_DIR}/VCSVersion.inc")
-set(generate_vcs_version_script "${LLVM_CMAKE_PATH}/GenerateVersionFromVCS.cmake")
+set(generate_vcs_version_script "${LLVM_CMAKE_DIR}/GenerateVersionFromVCS.cmake")
 
 if(lldb_vc AND LLVM_APPEND_VC_REV)
   set(lldb_source_dir ${LLDB_SOURCE_DIR})
Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -3,8 +3,8 @@
 find_package(LLVM REQUIRED CONFIG HINTS ${LLVM_DIR} NO_CMAKE_FIND_ROOT_PATH)
 find_package(Clang REQUIRED CONFIG HINTS ${Clang_DIR} ${LLVM_DIR}/../clang NO_CMAKE_FIND_ROOT_PATH)
 
-# We set LLVM_CMAKE_PATH so that GetSVN.cmake is found correctly when building SVNVersion.inc
-set(LLVM_CMAKE_PATH ${LLVM_CMAKE_DIR} CACHE PATH "Path to LLVM CMake modules")
+# We set LLVM_CMAKE_DIR so that GetSVN.cmake is found correctly when building SVNVersion.inc
+set(LLVM_CMAKE_DIR ${LLVM_CMAKE_DIR} CACHE PATH "Path to LLVM CMake modules")
 
 set(LLVM_MAIN_SRC_DIR ${LLVM_BUILD_MAIN_SRC_DIR} CACHE PATH "Path to LLVM source tree")
 set(LLVM_MAIN_INCLUDE_DIR ${LLVM_MAIN_INCLUDE_DIR} CACHE PATH "Path to llvm/include")
Index: lld/Common/CMakeLists.txt
===
--- lld/Common/CMakeLists.txt
+++ lld/Common/CMakeLists.txt
@@ -8,7 +8,7 @@
 find_first_existing_vc_file("${LLD_SOURCE_DIR}" lld_vc)
 
 set(version_inc "${CMAKE_CURRENT_BINARY_DIR}/VCSVersion.inc")
-set(generate_vcs_version_script "${LLVM_CMAKE_PATH}/GenerateVersionFromVCS.cmake")
+set(generate_vcs_version_script "${LLVM_CMAKE_DIR}/GenerateVersionFromVCS.cmake")
 
 if(lld_vc AND LLVM_APPEND_VC_REV)
   set(lld_source_dir ${LLD_SOURCE_DIR})
Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ 

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

[Alf via Phabricator via cfe-commits]

gAlfonso-bit updated this revision to Diff 365039.
Herald added a subscriber: JDevlieghere.

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

https://reviews.llvm.org/D107717

Files:
  clang/CMakeLists.txt
  clang/lib/Basic/CMakeLists.txt
  compiler-rt/cmake/Modules/CompilerRTMockLLVMCMakeConfig.cmake
  compiler-rt/cmake/Modules/CompilerRTUtils.cmake
  flang/CMakeLists.txt
  libcxx/cmake/Modules/HandleOutOfTreeLLVM.cmake
  libunwind/CMakeLists.txt
  lld/CMakeLists.txt
  lld/Common/CMakeLists.txt
  lldb/cmake/modules/LLDBStandalone.cmake
  lldb/source/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/include/llvm/Support/CMakeLists.txt
  runtimes/CMakeLists.txt

Index: runtimes/CMakeLists.txt
===
--- runtimes/CMakeLists.txt
+++ runtimes/CMakeLists.txt
@@ -65,7 +65,7 @@
 
 # This variable makes sure that e.g. llvm-lit is found.
 set(LLVM_MAIN_SRC_DIR ${LLVM_BUILD_MAIN_SRC_DIR})
-set(LLVM_CMAKE_PATH ${LLVM_MAIN_SRC_DIR}/cmake/modules)
+set(LLVM_CMAKE_DIR ${LLVM_MAIN_SRC_DIR}/cmake/modules)
 
 # This variable is used by individual runtimes to locate LLVM files.
 set(LLVM_PATH ${LLVM_BUILD_MAIN_SRC_DIR})
Index: llvm/include/llvm/Support/CMakeLists.txt
===
--- llvm/include/llvm/Support/CMakeLists.txt
+++ llvm/include/llvm/Support/CMakeLists.txt
@@ -3,7 +3,7 @@
 # The VC revision include that we want to generate.
 set(version_inc "${CMAKE_CURRENT_BINARY_DIR}/VCSRevision.h")
 
-set(generate_vcs_version_script "${LLVM_CMAKE_PATH}/GenerateVersionFromVCS.cmake")
+set(generate_vcs_version_script "${LLVM_CMAKE_DIR}/GenerateVersionFromVCS.cmake")
 
 if(LLVM_APPEND_VC_REV)
   set(llvm_source_dir ${LLVM_MAIN_SRC_DIR})
Index: llvm/CMakeLists.txt
===
--- llvm/CMakeLists.txt
+++ llvm/CMakeLists.txt
@@ -295,8 +295,8 @@
 set(LLVM_MAIN_INCLUDE_DIR ${LLVM_MAIN_SRC_DIR}/include ) # --includedir
 set(LLVM_BINARY_DIR   ${CMAKE_CURRENT_BINARY_DIR}  ) # --prefix
 
-# Note: LLVM_CMAKE_PATH does not include generated files
-set(LLVM_CMAKE_PATH ${LLVM_MAIN_SRC_DIR}/cmake/modules)
+# Note: LLVM_CMAKE_DIR does not include generated files
+set(LLVM_CMAKE_DIR ${LLVM_MAIN_SRC_DIR}/cmake/modules)
 set(LLVM_EXAMPLES_BINARY_DIR ${LLVM_BINARY_DIR}/examples)
 set(LLVM_INCLUDE_DIR ${CMAKE_CURRENT_BINARY_DIR}/include)
 
Index: lldb/source/CMakeLists.txt
===
--- lldb/source/CMakeLists.txt
+++ lldb/source/CMakeLists.txt
@@ -8,7 +8,7 @@
 find_first_existing_vc_file("${LLDB_SOURCE_DIR}" lldb_vc)
 
 set(version_inc "${CMAKE_CURRENT_BINARY_DIR}/VCSVersion.inc")
-set(generate_vcs_version_script "${LLVM_CMAKE_PATH}/GenerateVersionFromVCS.cmake")
+set(generate_vcs_version_script "${LLVM_CMAKE_DIR}/GenerateVersionFromVCS.cmake")
 
 if(lldb_vc AND LLVM_APPEND_VC_REV)
   set(lldb_source_dir ${LLDB_SOURCE_DIR})
Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -3,8 +3,8 @@
 find_package(LLVM REQUIRED CONFIG HINTS ${LLVM_DIR} NO_CMAKE_FIND_ROOT_PATH)
 find_package(Clang REQUIRED CONFIG HINTS ${Clang_DIR} ${LLVM_DIR}/../clang NO_CMAKE_FIND_ROOT_PATH)
 
-# We set LLVM_CMAKE_PATH so that GetSVN.cmake is found correctly when building SVNVersion.inc
-set(LLVM_CMAKE_PATH ${LLVM_CMAKE_DIR} CACHE PATH "Path to LLVM CMake modules")
+# We set LLVM_CMAKE_DIR so that GetSVN.cmake is found correctly when building SVNVersion.inc
+set(LLVM_CMAKE_DIR ${LLVM_CMAKE_DIR} CACHE PATH "Path to LLVM CMake modules")
 
 set(LLVM_MAIN_SRC_DIR ${LLVM_BUILD_MAIN_SRC_DIR} CACHE PATH "Path to LLVM source tree")
 set(LLVM_MAIN_INCLUDE_DIR ${LLVM_MAIN_INCLUDE_DIR} CACHE PATH "Path to llvm/include")
Index: lld/Common/CMakeLists.txt
===
--- lld/Common/CMakeLists.txt
+++ lld/Common/CMakeLists.txt
@@ -8,7 +8,7 @@
 find_first_existing_vc_file("${LLD_SOURCE_DIR}" lld_vc)
 
 set(version_inc "${CMAKE_CURRENT_BINARY_DIR}/VCSVersion.inc")
-set(generate_vcs_version_script "${LLVM_CMAKE_PATH}/GenerateVersionFromVCS.cmake")
+set(generate_vcs_version_script "${LLVM_CMAKE_DIR}/GenerateVersionFromVCS.cmake")
 
 if(lld_vc AND LLVM_APPEND_VC_REV)
   set(lld_source_dir ${LLD_SOURCE_DIR})
Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -27,7 +27,7 @@
 
   list(GET LLVM_CONFIG_OUTPUT 0 OBJ_ROOT)
   list(GET LLVM_CONFIG_OUTPUT 1 MAIN_INCLUDE_DIR)
-  list(GET LLVM_CONFIG_OUTPUT 2 LLVM_CMAKE_PATH)
+  list(GET LLVM_CONFIG_OUTPUT 2 LLVM_CMAKE_DIR)
   list(GET LLVM_CONFIG_OUTPUT 3 MAIN_SRC_DIR)
 
   set(LLVM_OBJ_ROOT ${OBJ_ROOT} CACHE PATH "path to LLVM build tree")
@@ -35,14 +35,14 @@
   set(LLVM_MAIN_SRC_DIR ${MAIN_SRC_DIR} CACHE PATH "Path t

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

[Anshil Gandhi via Phabricator via cfe-commits]

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

refreshing patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106891

Files:
  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/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
@@ -45,6 +45,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
@@ -182,6 +187,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
@@ -434,6 +444,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 Atomic instructions
 ; GCN-O1-OPTS-NEXT:  AMDGPU Promote Alloca
 ; GCN-O1-OPTS-NEXT:  Dominator Tree Construction
@@ -719,6 +734,11 @@
 ; GCN-O2-NEXT:Lower uses of LDS variables from 

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D69764#2934032 , @MyDeveloperDay 
wrote:

> In D69764#2932929 , @steveire wrote:
>
>> 
>
> @steveire, thanks for your comments, I also agree that a second tool 
> shouldn't be needed, especially as this functionality is off by default (and 
> needs to stay that way). This was my suggestion for a compromise.
>
> Its a tough call, but ultimately no on ever gave this a LGTM so I couldn't 
> proceed anyway. Whilst I think we can cover the other qualifiers with a 
> similar approach I don't want to waste more of my time until the fundamental 
> question has been answered if its ok for clang-format to insert/modify the 
> order of tokens and that is more a collective decision.
>
> It was suggested to me that I write up and RFC on the matter, I'm not a 
> massive fan of those as I don't really see a formal process for handling them 
> (the conversation seems to turn into a series of personal preferences then 
> dwindles and dies without conclusion). But if people think it would solve 
> something I guess I could try.
>
> From my perspective I'm more interested in the views of the major 
> clang-format contributors (@curdeius, @krasimir, @owenpan , @sammccall, 
> @HazardyKnusperkeks  and people like yourselves who have put time and effort 
> into blogging about these tools), ultimately it will be us who have to handle 
> the impact of this (and maybe the #clangd people) and I don't want to 
> inconvenience them or make work for them.
>
> So here is what is needed to land this in my view:
>
> 1. I'd want at least 2 to 3 of the current reviewers to LGTM.
> 2. I'd want 1 person (like yourself) a community contributor to also give 
> their approval (I guess I have that conceptually from you!)
>
> or
>
> 1. @klimek or @djasper to give us an its ok for "clang-format to 
> insert/modify the order of tokens" in a controlled manner decision
> 2. 1 LGTM
>
> Whilst I respect the views of those few who have objected, they don't to my 
> knowledge make significant contributions to clang-format (not in the last 5-6 
> years I've been following it anyway)
>
> I feel this team owns the decision as we are the ones that look after it, 
> warts and all. We'll always have a situation where some don't agree, thats 
> ok, but I feel there is more agreement that this funcationality is ok rather 
> than not.
>
> Until one of those 2 things happens, we are in limbo.

I think we had a really good, inclusive discussion on this change, so I don't 
think an RFC would add anything to it.

I think we have consensus on, if we want this, we:

1. want it behind an option that is not enabled in default-configs
2. we want users to understand that this has the potential to introduce bugs

I also think this is a feature that seems useful enough for a large enough 
group, to provide it in clang-format.
I'd put a lot of weight on @MyDeveloperDay's evaluation, as in the end, he's 
currently taking the most active role in keeping clang-format alive & well.

One idea for clang-format going forward is: can we make it easier for people to 
understand what flags can introduce non-whitespace-changes? E.g. have flag name 
prefixes like semantic-*.

So, I'm generally OK with this.

One minor nit I have is that I'd personally add unit tests to the 
EastWestConstFixer, as it's quite a large class, but given that I'll duck out 
after this comment, I'll not insist :)


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] D69764: [clang-format] Add East/West Const fixer capability

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D69764#2934124 , @klimek wrote:

> In D69764#2934032 , @MyDeveloperDay 
> wrote:
>
>> It was suggested to me that I write up and RFC on the matter, I'm not a 
>> massive fan of those as I don't really see a formal process for handling 
>> them (the conversation seems to turn into a series of personal preferences 
>> then dwindles and dies without conclusion). But if people think it would 
>> solve something I guess I could try.
>>
>> From my perspective I'm more interested in the views of the major 
>> clang-format contributors (@curdeius, @krasimir, @owenpan , @sammccall, 
>> @HazardyKnusperkeks  and people like yourselves who have put time and effort 
>> into blogging about these tools), ultimately it will be us who have to 
>> handle the impact of this (and maybe the #clangd people) and I don't want to 
>> inconvenience them or make work for them.
>>
>> So here is what is needed to land this in my view:
>>
>> 1. I'd want at least 2 to 3 of the current reviewers to LGTM.
>> 2. I'd want 1 person (like yourself) a community contributor to also give 
>> their approval (I guess I have that conceptually from you!)
>>
>> or
>>
>> 1. @klimek or @djasper to give us an its ok for "clang-format to 
>> insert/modify the order of tokens" in a controlled manner decision
>> 2. 1 LGTM
>>
>> Whilst I respect the views of those few who have objected, they don't to my 
>> knowledge make significant contributions to clang-format (not in the last 
>> 5-6 years I've been following it anyway)
>>
>> I feel this team owns the decision as we are the ones that look after it, 
>> warts and all. We'll always have a situation where some don't agree, thats 
>> ok, but I feel there is more agreement that this funcationality is ok rather 
>> than not.
>>
>> Until one of those 2 things happens, we are in limbo.
>
> I think we had a really good, inclusive discussion on this change, so I don't 
> think an RFC would add anything to it.

FWIW, the RFC I requested off-list wasn't about east/west const fixing, it was 
about whether the community was okay with a new direction of clang-format that 
breaks code or not. This is a shift in direction from the original design of 
clang-format. The waters were muddied somewhat by include sorting, but in my 
totally unscientific polls on the matter, I found basically no one concerned 
about include sorting while I found a not-insignificant number of people who 
were concerned about qualifier formatting and breakage in general. The 
prevailing sentiments were that breakage from include sorting demonstrates 
fragility with your code, but qualifier shuffling may or may not. That said, I 
did find that about 1/3 of the people were fine with breakage in general so 
long as it's mandated to be an opt-in feature as a matter of policy (and about 
65% were uncomfortable with breakage from a code formatting tool in general). 
Take that "data" with a heaping grain of salt, it was a Twitter poll with ~150 
respondents.

Personally, I still think an RFC (as frustrating of a process as it can be) is 
valuable because this strikes me as a policy change for a very popular tool. I 
highly doubt much of the community is following this review with that in mind, 
let alone the greater user base for clang-format.

> I think we have consensus on, if we want this, we:
>
> 1. want it behind an option that is not enabled in default-configs
> 2. we want users to understand that this has the potential to introduce bugs

+1

> I also think this is a feature that seems useful enough for a large enough 
> group, to provide it in clang-format.
> I'd put a lot of weight on @MyDeveloperDay's evaluation, as in the end, he's 
> currently taking the most active role in keeping clang-format alive & well.
>
> One idea for clang-format going forward is: can we make it easier for people 
> to understand what flags can introduce non-whitespace-changes? E.g. have flag 
> name prefixes like semantic-*.

There are orthogonal ideas here. 1) I think having the flag name clearly show 
that this may break the user's code is a good idea. 2) I do not think this 
review should set a new policy that breaking changes in clang-format are 
acceptable unless there is a larger community discussion specifically about 
that point.

> So, I'm generally OK with this.

I continue to oppose the decision on the grounds that code formatting tools 
should not be opinionated on the formatting of their input source code. This 
reduces the utility of a tool that's used by a *much* wider audience than 
people represented on this review. That said, I'll abide by the consensus 
position.


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] D93769: [clang] Add support for option -ffp-eval-method and extend #pragma float_control similarly

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

This patch was reverted because SPEC2006 and SPEC2017 are failing.
More information here:
https://bugs.llvm.org/show_bug.cgi?id=51346


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93769

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

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

This patch had to be reverted because SPEC2006 and SPEC2017 are failing.
More information here:
https://bugs.llvm.org/show_bug.cgi?id=51346


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74436

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

[PATCH] D107648: [OpenCL] Clang diagnostics allow reporting C++ for OpenCL version.

[Justas Janickas via Phabricator via cfe-commits]

Topotuna updated this revision to Diff 365157.
Topotuna added a comment.
Herald added a subscriber: dexonsmith.

Diagnostics reworded. Helper function `getOpenCLVersionString()` added. Tests 
updated


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

https://reviews.llvm.org/D107648

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/lib/Basic/LangOptions.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Frontend/opencl.cl
  clang/test/SemaOpenCL/ext_vectors.cl
  clang/test/SemaOpenCL/nosvm.cl

Index: clang/test/SemaOpenCL/nosvm.cl
===
--- clang/test/SemaOpenCL/nosvm.cl
+++ clang/test/SemaOpenCL/nosvm.cl
@@ -1,13 +1,16 @@
 // RUN: %clang_cc1 -verify %s
 // RUN: %clang_cc1 -verify -cl-std=CL2.0 -D CL20 %s
+// RUN: %clang_cc1 -verify -cl-std=clc++1.0 -D CLCPP10 %s
 // RUN: %clang_cc1 -verify -x c -D NOCL %s
 
 #ifndef NOCL
 kernel void f(__attribute__((nosvm)) global int* a);
-#ifndef CL20
-// expected-error@-2 {{'nosvm' attribute requires OpenCL version 2.0}}
+#if defined(CL20)
+// expected-warning@-2 {{'nosvm' attribute is deprecated and ignored in OpenCL C version 2.0}}
+#elif defined(CLCPP10)
+// expected-warning@-4 {{'nosvm' attribute is deprecated and ignored in C++ for OpenCL version 1.0}}
 #else
-// expected-warning@-4 {{'nosvm' attribute is deprecated and ignored in OpenCL version 2.0}}
+// expected-error@-6 {{attribute 'nosvm' is supported in the OpenCL version 2.0}}
 #endif
 
 __attribute__((nosvm)) void g(); // expected-warning {{'nosvm' attribute only applies to variables}}
Index: clang/test/SemaOpenCL/ext_vectors.cl
===
--- clang/test/SemaOpenCL/ext_vectors.cl
+++ clang/test/SemaOpenCL/ext_vectors.cl
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL1.1
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=clc++1.0
 
 typedef float float4 __attribute__((ext_vector_type(4)));
 
@@ -9,12 +10,12 @@
 
   V = V.abgr;
 #if (__OPENCL_C_VERSION__ < 300)
-  // expected-warning@-2 {{vector component name 'a' is an OpenCL C version 3.0 feature}}
+  // expected-warning@-2 {{vector component name 'a' is a feature from OpenCL version 3.0 onwards}}
 #endif
 
   V = V.xyzr;
   // expected-error@-1 {{illegal vector component name 'r'}}
 #if (__OPENCL_C_VERSION__ < 300)
-  // expected-warning@-3 {{vector component name 'r' is an OpenCL C version 3.0 feature}}
+  // expected-warning@-3 {{vector component name 'r' is a feature from OpenCL version 3.0 onwards}}
 #endif
 }
Index: clang/test/Frontend/opencl.cl
===
--- clang/test/Frontend/opencl.cl
+++ clang/test/Frontend/opencl.cl
@@ -2,7 +2,7 @@
 // RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=CL1.1 -DSYNTAX
 // RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=CL1.2 -DSYNTAX
 // RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=CL2.0 -DSYNTAX
-// RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=clc++ -DSYNTAX
+// RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=clc++1.0 -DSYNTAX
 // RUN: %clang_cc1 %s -verify -fsyntax-only -fblocks -DBLOCKS -DSYNTAX
 // RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=CL1.1 -fblocks -DBLOCKS -DSYNTAX
 // RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=CL1.2 -fblocks -DBLOCKS -DSYNTAX
@@ -10,6 +10,7 @@
 // RUN: %clang_cc1 -cl-std=CL1.1 -cl-strict-aliasing -fblocks %s 2>&1 | FileCheck --check-prefix=CHECK-INVALID-OPENCL-VERSION11 %s
 // RUN: %clang_cc1 -cl-std=CL1.2 -cl-strict-aliasing -fblocks %s 2>&1 | FileCheck --check-prefix=CHECK-INVALID-OPENCL-VERSION12 %s
 // RUN: %clang_cc1 -cl-std=CL2.0 -cl-strict-aliasing %s 2>&1 | FileCheck --check-prefix=CHECK-INVALID-OPENCL-VERSION20 %s
+// RUN: %clang_cc1 -cl-std=clc++1.0 -cl-strict-aliasing -fblocks %s 2>&1 | FileCheck --check-prefix=CHECK-INVALID-OPENCLCPP-VERSION10 %s
 
 #ifdef SYNTAX
 class test{
@@ -30,6 +31,7 @@
   // expected-error@-6{{blocks support disabled - compile with -fblocks or pick a deployment target that supports them}}
 #endif
 
-// CHECK-INVALID-OPENCL-VERSION11: warning: OpenCL version 1.1 does not support the option '-cl-strict-aliasing'
-// CHECK-INVALID-OPENCL-VERSION12: warning: OpenCL version 1.2 does not support the option '-cl-strict-aliasing'
-// CHECK-INVALID-OPENCL-VERSION20: warning: OpenCL version 2.0 does not support the option '-cl-strict-aliasing'
+// CHECK-INVALID-OPENCL-VERSION11: warning: OpenCL C version 1.1 does not support the option '-cl-strict-aliasing'
+// CHECK-INVALID-OPENCL-VERSION12: warning: OpenCL C version 1.2 does not support the option '-cl-strict-aliasing'
+// CHECK-INVALID-OPENCL-VERSION20: war

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

[gehry via Phabricator via cfe-commits]

Sockke updated this revision to Diff 365159.
Sockke added a comment.

Format and add some description.


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

https://reviews.llvm.org/D107641

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
@@ -208,9 +208,8 @@
   PositiveMultipleConstructors(const PositiveMultipleConstructors &) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize 
these fields: A, B
 
-  // FIXME: The fix-its here collide providing an erroneous fix
   int A, B;
-  // CHECK-FIXES: int A{}{}{}, B{}{}{};
+  // CHECK-FIXES: int A{}, B{};
 };
 
 typedef struct {
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
@@ -10,6 +10,7 @@
 #define 
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_MEMBER_INIT_H
 
 #include "../ClangTidyCheck.h"
+#include "llvm/ADT/DenseSet.h"
 
 namespace clang {
 namespace tidy {
@@ -72,6 +73,10 @@
   // instead of brace initialization. Only effective in C++11 mode. Default is
   // false.
   bool UseAssignment;
+
+  // Record the member variables that have been initialized to prevent repeated
+  // initialization.
+  llvm::DenseSet HasRecordClasMemberSet;
 };
 
 } // namespace cppcoreguidelines
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -433,17 +433,25 @@
[&](const FieldDecl *F) { OrderedFields.push_back(F); });
 
   // Collect all the fields we need to initialize, including indirect fields.
+  // It only includes fields that have not been fixed
   SmallPtrSet AllFieldsToInit;
-  forEachField(ClassDecl, FieldsToInit,
-   [&](const FieldDecl *F) { AllFieldsToInit.insert(F); });
-  if (AllFieldsToInit.empty())
+  forEachField(ClassDecl, FieldsToInit, [&](const FieldDecl *F) {
+if (!HasRecordClasMemberSet.count(F)) {
+  AllFieldsToInit.insert(F);
+  HasRecordClasMemberSet.insert(F);
+}
+  });
+  if (FieldsToInit.empty())
 return;
 
   DiagnosticBuilder Diag =
   diag(Ctor ? Ctor->getBeginLoc() : ClassDecl.getLocation(),
"%select{|union }0constructor %select{does not|should}0 initialize "
"%select{|one of }0these fields: %1")
-  << IsUnion << toCommaSeparatedString(OrderedFields, AllFieldsToInit);
+  << IsUnion << toCommaSeparatedString(OrderedFields, FieldsToInit);
+
+  if (AllFieldsToInit.empty())
+return;
 
   // Do not propose fixes for constructors in macros since we cannot place them
   // correctly.


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
@@ -208,9 +208,8 @@
   PositiveMultipleConstructors(const PositiveMultipleConstructors &) {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: constructor does not initialize these fields: A, B
 
-  // FIXME: The fix-its here collide providing an erroneous fix
   int A, B;
-  // CHECK-FIXES: int A{}{}{}, B{}{}{};
+  // CHECK-FIXES: int A{}, B{};
 };
 
 typedef struct {
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_MEMBER_INIT_H
 
 #include "../ClangTidyCheck.h"
+#include "llvm/ADT/DenseSet.h"
 
 namespace clang {
 namespace tidy {
@@ -72,6 +73,10 @@
   // instead of brace initialization. Only effective in C++11 mode. Default is
   // false.
   bool UseAssignment;
+
+  // Record the member variables that have been initialized to prevent repeated
+  // initialization.
+  llvm::DenseSet HasRecordClasMemberSet;
 };
 
 } // namespa

[PATCH] D107723: [Clang] Moved warning of warnReturnTypestateForUnconsumableType back to SemaDeclAttr

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

It looks like there are still test failures happening from these changes. We're 
now getting more `return state set for an unconsumable type ` diagnostics.


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

https://reviews.llvm.org/D107723

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

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 365161.
mgartmann added a comment.

- Updated this patch with the current state of the LLVM project GitHub 
repository


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-class-destructor.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
@@ -0,0 +1,204 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-virtual-class-destructor %t -- --fix-notes
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PrivateVirtualBaseStruct' is private and prevents using the type [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+2]]:8: note: make it protected
+// As we have 2 conflicting fixes in notes, no fix is applied.
+struct PrivateVirtualBaseStruct {
+  virtual void f();
+
+private:
+  virtual ~PrivateVirtualBaseStruct() {}
+};
+
+struct PublicVirtualBaseStruct { // OK
+  virtual void f();
+  virtual ~PublicVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'ProtectedVirtualBaseStruct' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it protected and non-virtual
+struct ProtectedVirtualBaseStruct {
+  virtual void f();
+
+protected:
+  virtual ~ProtectedVirtualBaseStruct() {}
+  // CHECK-FIXES: ~ProtectedVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'ProtectedVirtualDefaultBaseStruct' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it protected and non-virtual
+struct ProtectedVirtualDefaultBaseStruct {
+  virtual void f();
+
+protected:
+  virtual ~ProtectedVirtualDefaultBaseStruct() = default;
+  // CHECK-FIXES: ~ProtectedVirtualDefaultBaseStruct() = default;
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PrivateNonVirtualBaseStruct' is private and prevents using the type [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+2]]:8: note: make it protected
+// As we have 2 conflicting fixes in notes, no fix is applied.
+struct PrivateNonVirtualBaseStruct {
+  virtual void f();
+
+private:
+  ~PrivateNonVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'PublicNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it public and virtual
+struct PublicNonVirtualBaseStruct {
+  virtual void f();
+  ~PublicNonVirtualBaseStruct() {}
+  // CHECK-FIXES: virtual ~PublicNonVirtualBaseStruct() {}
+};
+
+struct PublicNonVirtualNonBaseStruct { // OK according to C.35, since this struct does not have any virtual methods.
+  void f();
+  ~PublicNonVirtualNonBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PublicImplicitNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-FIXES: struct PublicImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicImplicitNonVirtualBaseStruct() = default;
+struct PublicImplicitNonVirtualBaseStruct {
+  virtual void f();
+};
+
+// CHECK-MESSAGES: :[[@LINE+5]]:8: warning: destructor of 'PublicASImplicitNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+4]]:8: note: make it public and virtual
+// CHECK-FIXES: struct PublicASImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicASImplicitNonVirtualBaseStruct() = default;
+// CHECK-FIXES-NEXT: private:
+struct PublicASImplicitNonVirtualBaseStruct {
+private:
+  virtual void f();
+};
+
+struct ProtectedNonVirtualBaseStruct { // OK
+  virtual void f();
+
+protected:
+  ~ProtectedNonVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:7: warning: destructor of 'PrivateVirtualBaseClass' is private and prevents using the type [cppcoreguidelines-virtual-class-dest

[PATCH] D107760: [lldb] Fix warning -Wnon-virtual-dtor.

[Adrian Kuegel via Phabricator via cfe-commits]

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

When making the emit function virtual, the destructor needs to become
virtual as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107760

Files:
  clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp


Index: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
===
--- clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
+++ clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
@@ -238,6 +238,7 @@
 public:
   OpenCLBuiltinFileEmitterBase(RecordKeeper &Records, raw_ostream &OS)
   : Records(Records), OS(OS) {}
+  virtual ~OpenCLBuiltinFileEmitterBase() = default;
 
   // Entrypoint to generate the functions for testing all OpenCL builtin
   // functions.
@@ -307,6 +308,7 @@
 public:
   OpenCLBuiltinTestEmitter(RecordKeeper &Records, raw_ostream &OS)
   : OpenCLBuiltinFileEmitterBase(Records, OS) {}
+  virtual ~OpenCLBuiltinTestEmitter() = default;
 
   // Entrypoint to generate the functions for testing all OpenCL builtin
   // functions.


Index: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
===
--- clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
+++ clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
@@ -238,6 +238,7 @@
 public:
   OpenCLBuiltinFileEmitterBase(RecordKeeper &Records, raw_ostream &OS)
   : Records(Records), OS(OS) {}
+  virtual ~OpenCLBuiltinFileEmitterBase() = default;
 
   // Entrypoint to generate the functions for testing all OpenCL builtin
   // functions.
@@ -307,6 +308,7 @@
 public:
   OpenCLBuiltinTestEmitter(RecordKeeper &Records, raw_ostream &OS)
   : OpenCLBuiltinFileEmitterBase(Records, OS) {}
+  virtual ~OpenCLBuiltinTestEmitter() = default;
 
   // Entrypoint to generate the functions for testing all OpenCL builtin
   // functions.
___
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 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp:136
+CheckFactories.registerCheck(
+"readability-variable-length");
   }

0x8000- wrote:
> aaron.ballman wrote:
> > Is there a reason this should be restricted to variables? For example, 
> > wouldn't the same functionality be useful for type names, or dare I say it, 
> > even macro names? I'm wondering if this should be 
> > `readability-identifier-length` to be more generic.
> I consider those to be in separate namespaces. I suppose we could have a 
> single checker with multiple rules, but on the other hand I don't want to 
> combine too many things into one, just because they share the "compare 
> length" dimension.
I see where you're coming from, but I come down on the other side. Running a 
check is expensive (especially on Windows where process creation can be really 
slow), so having multiple checks that traverse the AST gives worse performance 
than having a single check that only traverses the AST once. I'd rather see 
related functionality grouped together and use options to control behavior when 
it's a natural fit to do so.

I should note that I don't mean *you* have to implement this other 
functionality (as part of this patch or otherwise). Just that I think we should 
leave the check name ambiguous enough that we could grow it to do that work in 
the future.

WDYT?



Comment at: clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp:20
+
+const unsigned DefaultMinimumVariableNameLength = 3;
+const unsigned DefaultMinimumLoopCounterNameLength = 2;

0x8000- wrote:
> aaron.ballman wrote:
> > Should it be possible to enforce parameters differently than local 
> > variables? (It seems a bit odd that we'd allow you to specify exception 
> > variable length separate from loops but not function parameters separate 
> > from locals.)
> Exceptions and Loops historically are "throw-away" / "don't care" / "one-off" 
> variables. Parameter names are names, and they are the interface between the 
> outside of the routine and the processing inside; other than historical, I 
> don't see good arguments (sic) to allow single-letter parameter names.
> 
> Note that this check will be quite noisy on a legacy code base, and people 
> will find little reason to invest the work to remove the warnings. But if 
> somebody starts something new and want to enforce some quality attributes, it 
> is the right tool at the right time. There will be new projects starting in 
> 2021 and 2022 that could benefit from this, I hope.
> Exceptions and Loops historically are "throw-away" / "don't care" / "one-off" 
> variables. Parameter names are names, and they are the interface between the 
> outside of the routine and the processing inside; other than historical, I 
> don't see good arguments (sic) to allow single-letter parameter names.

I largely agree with you, but there are definitely cases where single-letter 
parameter names are extremely common. For example, coordinates are often `x`, 
`y`, and `z`, colors are often `r`, `g`, and `b` (among many others), and 
physics applications often do things like `f = m * a` with their variables. 
Also, there are cases where the parameter names are often not super meaningful, 
like with functions `min` and `max` so the parameter names may be quite short.



Comment at: clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp:26
+
+const char ErrorMessage[] = "%select{|exception |loop }0 variable name %1 is "
+"too short, expected at least %2 characters";

0x8000- wrote:
> aaron.ballman wrote:
> > It looks like there will be whitespace issues with this. If the variable is 
> > a loop or exception, it'll have an extra space (`loop  variable name`) and 
> > if it's not, it will start with a leading space (` variable name`).
> This was recommended by a previous reviewer. Any alternative suggestion here?
I *think* the diagnostic will work if you write it like this:
`"%select{variable |exception variable |loop variable |parameter }0name %1 is 
too short, expected at least %2 characters"`




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





Comment at: clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp:114
+
+const StringRef VarName = ExceptionVarName->getName();
+if ((VarName.size() >= MinimumExceptionNameLength) ||





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





Comment at: clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp:143
+
+const StringRef VarName = ParamVar-

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

For my part, I'm convinced and now +1 (or at least +0.5) on this being OK to 
include.
In users' minds this is a formatting/style operation, and the UX of 
clang-format and its integrations is much better than clang-tidy.

Implementation quality problems are a risk, but can be mitigated:

- today: clang-format itself is mature and enforced on some projects. And this 
feature isn't as mature yet, and can be dangerous. Guarding the feature as 
experimental by a flag and/or prefixed config names seems like a good way to 
tell people that.
- forever: if this can never be made sufficiently robust, it should not be 
promoted to a "standard" feature. This would be sad/awkward, but the history of 
clang-format suggests it's not that likely.

@MyDeveloperDay: sorry that it's been (and continues to be) hard to get 
consensus here. It's not surprising: there are strong arguments in both 
directions, and they appeal to different values.
@steveire: I don't think it's true or helpful to suggest the downsides aren't 
real, or to isolate people as holdouts.

> I think we had a really good, inclusive discussion on this change, so I don't 
> think an RFC would add anything to it.

I agree with this. I'd be surprised if the balance of arguments about 
const-fixing was substantially different.
A more abstract RFC about direction seems unikely to be productive. I think 
*all* uses of clang-format can break certain code, it's a (large) difference in 
degree. So the concrete details matter.


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] D107760: [clang] Fix warning -Wnon-virtual-dtor.

[Benjamin Kramer via Phabricator via cfe-commits]

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

thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107760

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

[clang] 19bd806 - [OpenCL] Add missing virtual destructor

[Sven van Haastregt via cfe-commits]

Author: Sven van Haastregt
Date: 2021-08-09T13:49:52+01:00
New Revision: 19bd806a1a443e4ce45ccc670861848fb1579022

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

LOG: [OpenCL] Add missing virtual destructor

Followup after f9ffe61fb53f ("[OpenCL] Factor out
OpenCLBuiltinFileEmitterBase; NFC", 2021-08-09) introduced a
-Wnon-virtual-dtor warning.

Added: 


Modified: 
clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp

Removed: 




diff  --git a/clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp 
b/clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
index 457313f1839f..6cc4bf5fd856 100644
--- a/clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
+++ b/clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
@@ -238,6 +238,7 @@ class OpenCLBuiltinFileEmitterBase {
 public:
   OpenCLBuiltinFileEmitterBase(RecordKeeper &Records, raw_ostream &OS)
   : Records(Records), OS(OS) {}
+  virtual ~OpenCLBuiltinFileEmitterBase() = default;
 
   // Entrypoint to generate the functions for testing all OpenCL builtin
   // functions.



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

[PATCH] D107760: [clang] Fix warning -Wnon-virtual-dtor.

[Adrian Kuegel via Phabricator via cfe-commits]

akuegel added a comment.

Already landed as 
https://github.com/llvm/llvm-project/commit/19bd806a1a443e4ce45ccc670861848fb1579022,
 I will close this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107760

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

[PATCH] D107506: [PowerPC][AIX] Warn when using pragma align(packed) on AIX.

[Chris Bowler via Phabricator via cfe-commits]

cebowleratibm accepted this revision.
cebowleratibm added a comment.

The new version is better than the previous.  LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107506

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

[PATCH] D107430: [OMPIRBuilder] Add ordered without depend and simd clause to OMPBuilder

[Peixin Qiao via Phabricator via cfe-commits]

peixin updated this revision to Diff 365170.
peixin added a comment.

Revise the implementation by not generating "kmp_ordered" runtime functions 
when -fopenmp-enable-irbuilder is not enabled, which is consistent with that 
not using OpenMPIRBuilder.


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

https://reviews.llvm.org/D107430

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/ordered_codegen.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
  llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp

Index: llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
===
--- llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -2115,6 +2115,78 @@
   EXPECT_EQ(CriticalEndCI->getArgOperand(2)->getType(), CriticalNamePtrTy);
 }
 
+TEST_F(OpenMPIRBuilderTest, OrderedDirective) {
+  using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+  F->setName("func");
+  IRBuilder<> Builder(BB);
+
+  OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL});
+
+  AllocaInst *PrivAI =
+  Builder.CreateAlloca(F->arg_begin()->getType(), nullptr, "priv.inst");
+
+  BasicBlock *EntryBB = nullptr;
+
+  auto BodyGenCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
+   BasicBlock &FiniBB) {
+EntryBB = FiniBB.getUniquePredecessor();
+
+llvm::BasicBlock *CodeGenIPBB = CodeGenIP.getBlock();
+llvm::Instruction *CodeGenIPInst = &*CodeGenIP.getPoint();
+EXPECT_EQ(CodeGenIPBB->getTerminator(), CodeGenIPInst);
+EXPECT_EQ(EntryBB, CodeGenIPBB);
+
+Builder.restoreIP(CodeGenIP);
+Builder.CreateStore(F->arg_begin(), PrivAI);
+Value *PrivLoad =
+Builder.CreateLoad(PrivAI->getAllocatedType(), PrivAI, "local.use");
+Builder.CreateICmpNE(F->arg_begin(), PrivLoad);
+  };
+
+  auto FiniCB = [&](InsertPointTy IP) {
+BasicBlock *IPBB = IP.getBlock();
+EXPECT_NE(IPBB->end(), IP.getPoint());
+  };
+
+  Builder.restoreIP(
+  OMPBuilder.createOrdered(Builder, BodyGenCB, FiniCB, false));
+
+  Value *EntryBBTI = EntryBB->getTerminator();
+  EXPECT_EQ(EntryBBTI, nullptr);
+
+  CallInst *OrderedEntryCI = nullptr;
+  for (auto &EI : *EntryBB) {
+Instruction *Cur = &EI;
+if (isa(Cur)) {
+  OrderedEntryCI = cast(Cur);
+  if (OrderedEntryCI->getCalledFunction()->getName() == "__kmpc_ordered")
+break;
+  OrderedEntryCI = nullptr;
+}
+  }
+  EXPECT_NE(OrderedEntryCI, nullptr);
+  EXPECT_EQ(OrderedEntryCI->getNumArgOperands(), 2U);
+  EXPECT_EQ(OrderedEntryCI->getCalledFunction()->getName(), "__kmpc_ordered");
+  EXPECT_TRUE(isa(OrderedEntryCI->getArgOperand(0)));
+
+  CallInst *OrderedEndCI = nullptr;
+  for (auto &FI : *EntryBB) {
+Instruction *Cur = &FI;
+if (isa(Cur)) {
+  OrderedEndCI = cast(Cur);
+  if (OrderedEndCI->getCalledFunction()->getName() == "__kmpc_end_ordered")
+break;
+  OrderedEndCI = nullptr;
+}
+  }
+  EXPECT_NE(OrderedEndCI, nullptr);
+  EXPECT_EQ(OrderedEndCI->getNumArgOperands(), 2U);
+  EXPECT_TRUE(isa(OrderedEndCI->getArgOperand(0)));
+  EXPECT_EQ(OrderedEndCI->getArgOperand(1), OrderedEntryCI->getArgOperand(1));
+}
+
 TEST_F(OpenMPIRBuilderTest, CopyinBlocks) {
   OpenMPIRBuilder OMPBuilder(*M);
   OMPBuilder.initialize();
Index: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
===
--- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -2133,6 +2133,35 @@
   /*Conditional*/ false, /*hasFinalize*/ true);
 }
 
+OpenMPIRBuilder::InsertPointTy
+OpenMPIRBuilder::createOrdered(const LocationDescription &Loc,
+   BodyGenCallbackTy BodyGenCB,
+   FinalizeCallbackTy FiniCB, bool IsThreads) {
+  if (!updateToLocation(Loc))
+return Loc.IP;
+
+  Directive OMPD = Directive::OMPD_ordered;
+  Instruction *EntryCall = nullptr;
+  Instruction *ExitCall = nullptr;
+
+  if (IsThreads) {
+Constant *SrcLocStr = getOrCreateSrcLocStr(Loc);
+Value *Ident = getOrCreateIdent(SrcLocStr);
+Value *ThreadId = getOrCreateThreadID(Ident);
+Value *Args[] = {Ident, ThreadId};
+
+Function *EntryRTLFn = getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_ordered);
+EntryCall = Builder.CreateCall(EntryRTLFn, Args);
+
+Function *ExitRTLFn =
+getOrCreateRuntimeFunctionPtr(OMPRTL___kmpc_end_ordered);
+ExitCall = Builder.CreateCall(ExitRTLFn, Args);
+  }
+
+  return EmitOMPInlinedRegion(OMPD, EntryCall, ExitCall, BodyGenCB, FiniCB,
+  /*Conditional*/ false, /*hasFinalize*/ true);
+}
+
 OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::EmitOMPInlinedRegion(
 Directive OMPD, Instruction *EntryCall, Instruction 

[PATCH] D107648: [OpenCL] Clang diagnostics allow reporting C++ for OpenCL version.

[Anastasia Stulova via Phabricator via cfe-commits]

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

LGTM! Thanks!

Btw I think we could also use this helper in another diagnostic 
(`err_opencl_unknown_type_specifier`) in 
`include/clang/Basic/DiagnosticCommonKinds.td`. But it can be updated 
separately though.


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

https://reviews.llvm.org/D107648

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

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[Erich Keane via Phabricator via cfe-commits]

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

I've just been watching this from the sideline, but the cases where this breaks 
code are unacceptable for this tool, it is a complete direction change for the 
tool, and making that direction change silently on a review of a 15 month 
patch, where TWO code owners have said 'no' for that reason is absurd.

I use this tool daily as a part of my 'upload' script, having it silently bust 
code between when I validate it and when I upload it is terrible, and makes the 
tool unusable for my purposes.  If we change this direction without a full RFC, 
my next step is going to be an RFC to remove clang-format from the check-in 
requirements of the entire LLVM project.


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] D105904: [clangd] Support `#pragma mark` in the outline

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

thanks, mostly looks good couple of nits!




Comment at: clang-tools-extra/clangd/CollectMacros.cpp:42
+if (isInsideMainFile(Loc, SM)) {
+  Position Start = sourceLocToPosition(SM, Loc);
+  Position End = {Start.line + 1, 0};

are we fine with these annotations spanning `#pragma [[mark XX]]` rather than 
the whole line or just `XX` ?



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:581
+  // Pragma owns all the children between P and NextPragma
+  auto It = std::partition(Cur->children.begin(), Cur->children.end(),
+   [&P, &NextPragma](const auto &S) -> bool {

nit: `s/std::partition(Cur->children.begin(), 
Cur->children.end()/llvm::partition(Cur->children/`



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:641
+
+  std::vector Pragmas;
+  for (const auto &P : PragmaMarks)

nit: Pragmas.reserve



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:645
+  Range EntireFile = {
+  {-1, -1},
+  {std::numeric_limits::max(), std::numeric_limits::max()}};

nit: `0,0` should suffice + makes sure the range is valid (in theory these are 
zero-based)



Comment at: clang-tools-extra/clangd/FindSymbols.cpp:651
+  Root.range = EntireFile;
+  mergePragmas(Root, PragmasRef);
+  return Root.children;

nit: `llvm::makeArrayRef(Pragmas)`



Comment at: clang-tools-extra/clangd/FindTarget.cpp:655
 llvm::iterator_range Locations) {
-  for (const auto &P : llvm::zip(Protocols, Locations)) {
 Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),

this change does not look relevant, can you please revert?



Comment at: clang-tools-extra/clangd/ParsedAST.cpp:441
+  if (Preamble)
+Marks = Preamble->Marks;
+  Clang->getPreprocessor().addPPCallbacks(

We are not patching marks for stale preambles, which I believe is fine but 
worth a FIXME. If you are interested please take a look at 
https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/Preamble.cpp#L427,
 it should be fairly easy to handle movements of `#pragma mark`s in preamble 
section.



Comment at: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp:1055
+
+  #pragma $End[[mark - End
+]]

kadircet wrote:
> can you also add something like:
> 
> ```
> #pragma mark - Foo
> struct Foo {
>   #pragma mark - Bar
>   void bar() {
>  #pragma mark - NotTopDecl // I don't think we want this to be part of 
> documentsymbol.
>   }
> };
> void bar() {}
> ```
can you add these cases as well to make sure details of the merge logic are 
working as expected?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105904

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

Thank you for working on this, I think it's fantastic functionality!




Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4357
+continue;
+  llvm::StringRef Name = S.NormalizedFullName;
+  if (Completion == AttributeCompletion::Scope) {

Should we also add some special handling for attributes optionally starting 
with double underscores? e.g., `__attribute__((const))` and 
`__attribute__((__const__))`` are both equally useful to complete.

Also, we should add some explicit testing for `omp::sequence` and 
`omp::directive` as those are handled very specially and won't appear in the 
parsed attribute map. I think the OpenMP code completion would be super useful, 
but can be handled in a follow-up if you want.


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] D106644: [clang][analyzer] Add standard streams to alpha.unix.Stream checker.

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

balazske updated this revision to Diff 365175.
balazske added a comment.

Simplified a lambda usage.
Moved some functions, changed documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106644

Files:
  clang/lib/StaticAnalyzer/Checkers/ASTLookup.cpp
  clang/lib/StaticAnalyzer/Checkers/ASTLookup.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/test/Analysis/stream.c

Index: clang/test/Analysis/stream.c
===
--- clang/test/Analysis/stream.c
+++ clang/test/Analysis/stream.c
@@ -1,7 +1,9 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.unix.Stream,debug.ExprInspection -verify %s
 
 #include "Inputs/system-header-simulator.h"
 
+void clang_analyzer_eval(int);
+
 void check_fread() {
   FILE *fp = tmpfile();
   fread(0, 0, 0, fp); // expected-warning {{Stream pointer might be NULL}}
@@ -267,3 +269,23 @@
 } // expected-warning {{Opened stream never closed. Potential resource leak}}
 // FIXME: This warning should be placed at the `return` above.
 // See https://reviews.llvm.org/D83120 about details.
+
+void check_fopen_is_not_std() {
+  FILE *F = fopen("file", "r");
+  if (!F)
+return;
+  clang_analyzer_eval(F != stdin);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(F != stdout); // expected-warning{{TRUE}}
+  clang_analyzer_eval(F != stderr); // expected-warning{{TRUE}}
+  fclose(F);
+}
+
+void check_tmpfile_is_not_std() {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  clang_analyzer_eval(F != stdin);  // expected-warning{{TRUE}}
+  clang_analyzer_eval(F != stdout); // expected-warning{{TRUE}}
+  clang_analyzer_eval(F != stderr); // expected-warning{{TRUE}}
+  fclose(F);
+}
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -10,6 +10,8 @@
 //
 //===--===//
 
+#include "ASTLookup.h"
+#include "clang/AST/ParentMapContext.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
@@ -157,6 +159,11 @@
 // StreamChecker class and utility functions.
 //===--===//
 
+// This map holds the state of a stream.
+// The stream is identified with a SymbolRef that is created when a stream
+// opening function is modeled by the checker.
+REGISTER_MAP_WITH_PROGRAMSTATE(StreamMap, SymbolRef, StreamState)
+
 namespace {
 
 class StreamChecker;
@@ -172,6 +179,23 @@
   ArgNoTy StreamArgNo;
 };
 
+/// Find symbolic value of a global "standard stream"
+/// (stdin, stdout, stderr) variable.
+/// The StdDecl value is allowed to be nullptr.
+SymbolRef getStdStreamSym(const VarDecl *StdDecl, CheckerContext &C) {
+  if (!StdDecl)
+return nullptr;
+
+  const LocationContext *LCtx = C.getLocationContext();
+
+  const VarRegion *RegionOfStdStreamVar =
+  C.getState()->getRegion(StdDecl, LCtx);
+  if (!RegionOfStdStreamVar)
+return nullptr;
+  SVal StdVal = C.getState()->getSVal(RegionOfStdStreamVar, StdDecl->getType());
+  return StdVal.getAsSymbol();
+}
+
 /// Get the value of the stream argument out of the passed call event.
 /// The call should contain a function that is described by Desc.
 SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
@@ -206,8 +230,9 @@
   return State;
 }
 
-class StreamChecker : public Checker {
+class StreamChecker
+: public Checker {
   BugType BT_FileNull{this, "NULL stream pointer", "Stream handling error"};
   BugType BT_UseAfterClose{this, "Closed stream", "Stream handling error"};
   BugType BT_UseAfterOpenFailed{this, "Invalid stream",
@@ -221,6 +246,7 @@
   /*SuppressOnSink =*/true};
 
 public:
+  void checkBeginFunction(CheckerContext &C) const;
   void checkPreCall(const CallEvent &Call, CheckerContext &C) const;
   bool evalCall(const CallEvent &Call, CheckerContext &C) const;
   void checkDeadSymbols(SymbolReaper &SymReaper, CheckerContext &C) const;
@@ -285,6 +311,17 @@
 0}},
   };
 
+  mutable bool StdStreamsInitialized = false;
+  mutable Optional FILEType;
+  // These can be nullptr if not found.
+  mutable const VarDecl *StdinDecl;
+  mutable const VarDecl *StdoutDecl;
+  mutable const VarDecl *StderrDecl;
+  // These can be nullptr if not found.
+  mutable SymbolRef StdinSym;
+  mutable SymbolRef StdoutSym;
+  mutable SymbolRef StderrSym;
+
   void evalFopen(const FnDescription *Desc, const CallEvent

[PATCH] D107760: [clang] Fix warning -Wnon-virtual-dtor.

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

Quuxplusone added inline comments.



Comment at: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp:311
   : OpenCLBuiltinFileEmitterBase(Records, OS) {}
+  virtual ~OpenCLBuiltinTestEmitter() = default;
 

Tip: When you're putting a defaulted virtual destructor in a derived class 
"just to make sure the base class's dtor was virtual," I recommend doing it 
with the `override` keyword:

~OpenCLBuiltinTestEmitter() override = default;

This way, if the base class dtor was somehow //not// virtual, then you'll get a 
nice compiler error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107760

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

[PATCH] D107693: [Parser] Fix attr infloop on "int x [[c"

[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, thank you for the fix!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107693

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

[PATCH] D107242: [AIX] Define __HOS_AIX__ macro

[Chris Bowler via Phabricator via cfe-commits]

cebowleratibm added a comment.

In D107242#2932289 , @joerg wrote:

> I'm puzzled by this change. I don't think we have any case so far where the 
> compiler behavior changes with the host OS and I don't think it should. 
> What's the point / use case of this macro?

__HOS_AIX__ is a macro defined by the XL C/C++ compilers on AIX.  Although 
there was never an AIX cross compiler offering from IBM, the intention of the 
macro was to indicate that the host is AIX.  __HOS_AIX__ is being defined to 
minimize porting pain for existing code compiled with xlc/xlC.  Given that LLVM 
has the infrastructure to be a cross-compiler, I consider it important to 
implement the macro to the original intention.  Because the macro is a "legacy" 
AIX xlc macro I don't think it's relevant to define for non-AIX targets.  If 
there's no objection to the change I can work with Jake to elaborate on the 
comments with an improved commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107242

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

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D69764#2934378 , @erichkeane wrote:

> I've just been watching this from the sideline, but the cases where this 
> breaks code are unacceptable for this tool, it is a complete direction change 
> for the tool, and making that direction change silently on a review of a 15 
> month patch, where TWO code owners have said 'no' for that reason is absurd.
>
> I use this tool daily as a part of my 'upload' script, having it silently 
> bust code between when I validate it and when I upload it is terrible, and 
> makes the tool unusable for my purposes.  If we change this direction without 
> a full RFC, my next step is going to be an RFC to remove clang-format from 
> the check-in requirements of the entire LLVM project.

This and other potentially other mutating options would and MUST in my view 
ALWAYS be 100% "off by default" for all default style options (as -fix is for 
clang-tidy), it would be a purely "opt in" basis. (via .clang-format or command 
line)

I personally use this in a non modifying way "using the -dry-run mode" to catch 
new"east/const violations" and report failure back rather than "change the code 
itself"

I would not expect clang-format usage to change unless someone specially opted 
in to using it.


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] D107718: [cuda] Mark builtin texture/surface reference variable as 'externally_initialized'.

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4441
   if (Linkage != llvm::GlobalValue::InternalLinkage &&
-  (D->hasAttr() || D->hasAttr()))
+  (D->hasAttr() || D->hasAttr() ||
+   D->getType()->isCUDADeviceBuiltinSurfaceType() ||

we need also do this with variables with HIPManagedAttr and add a test for that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107718

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

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D69764#2934473 , @MyDeveloperDay 
wrote:

> In D69764#2934378 , @erichkeane 
> wrote:
>
>> I've just been watching this from the sideline, but the cases where this 
>> breaks code are unacceptable for this tool, it is a complete direction 
>> change for the tool, and making that direction change silently on a review 
>> of a 15 month patch, where TWO code owners have said 'no' for that reason is 
>> absurd.
>>
>> I use this tool daily as a part of my 'upload' script, having it silently 
>> bust code between when I validate it and when I upload it is terrible, and 
>> makes the tool unusable for my purposes.  If we change this direction 
>> without a full RFC, my next step is going to be an RFC to remove 
>> clang-format from the check-in requirements of the entire LLVM project.
>
> This and other potentially other mutating options would and MUST in my view 
> ALWAYS be 100% "off by default" for all default style options (as -fix is for 
> clang-tidy), it would be a purely "opt in" basis. (via .clang-format or 
> command line)
>
> I personally use this in a non modifying way "using the -dry-run mode" to 
> catch new"east/const violations" and report failure back rather than "change 
> the code itself"
>
> I would not expect clang-format usage to change unless someone specially 
> opted in to using it.

That seems just as bad, if not worse.  Clang-format isn't an analysis tool, its 
a format tool.  If you have an option that can only reasonably be run in 
'dry-run' mode, it seems that putting it in a 'format' tool is the wrong place.


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] D107703: [AST][clangd] Expose documentation of Attrs on hover.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:4220
+  )cpp";
+  std::vector Attrs = Records.getAllDerivedDefinitions("Attr");
+  for (const auto *A : Attrs) {

FWIW, this will miss `omp::sequence` and `omp::directive`, but that's not the 
end of the world. May be worth a fixme comment in case we want to solve it 
someday.



Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:4242
+  llvm::StringRef clang::Attr::getDocumentation(clang::attr::Kind K) {
+assert(K < llvm::array_lengthof(AttrDoc));
+return AttrDoc[K];

H, I am not 100% certain this assertion is correct -- the user may have 
plugin attributes, which I believe are given a "kind" that's larger than the 
last-known builtin attribute kind.


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] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D69764#2934378 , @erichkeane wrote:

> I've just been watching this from the sideline, but the cases where this 
> breaks code are unacceptable for this tool, it is a complete direction change 
> for the tool, and making that direction change silently on a review of a 15 
> month patch, where TWO code owners have said 'no' for that reason is absurd.
>
> I use this tool daily as a part of my 'upload' script, having it silently 
> bust code between when I validate it and when I upload it is terrible, and 
> makes the tool unusable for my purposes.  If we change this direction without 
> a full RFC, my next step is going to be an RFC to remove clang-format from 
> the check-in requirements of the entire LLVM project.

Can I just say, marking this review as requiring changes, I presume because you 
don't agree with it conceptually isn't very helpful to the consensus building 
process, unless you have an inline comment of something you think is wrong.  
What changes are you requesting?

F18452578: image.png 

I've tried to find compromises to mitigate peoples strong views, I know this is 
contentious, I've not tried to rush it in. I am a major contributor to 
clang-format, and I'd like to continue to move it forward..

as @klimek mentioned I'm one of the people really trying to help look after 
clang-format, I wouldn't do anything that I think damages it or its reputation, 
but I think there is value in this and other proposed changes. I know some 
people disagree but we also have to recognise that some agree too!

A "no" is a "no for everyone", a "yes" is a "yes and no" based on the 
configuration, I know what I think is the fairer 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

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D69764#2934483 , @erichkeane wrote:

> In D69764#2934473 , @MyDeveloperDay 
> wrote:
>
>> In D69764#2934378 , @erichkeane 
>> wrote:
>>
>>> I've just been watching this from the sideline, but the cases where this 
>>> breaks code are unacceptable for this tool, it is a complete direction 
>>> change for the tool, and making that direction change silently on a review 
>>> of a 15 month patch, where TWO code owners have said 'no' for that reason 
>>> is absurd.
>>>
>>> I use this tool daily as a part of my 'upload' script, having it silently 
>>> bust code between when I validate it and when I upload it is terrible, and 
>>> makes the tool unusable for my purposes.  If we change this direction 
>>> without a full RFC, my next step is going to be an RFC to remove 
>>> clang-format from the check-in requirements of the entire LLVM project.
>>
>> This and other potentially other mutating options would and MUST in my view 
>> ALWAYS be 100% "off by default" for all default style options (as -fix is 
>> for clang-tidy), it would be a purely "opt in" basis. (via .clang-format or 
>> command line)
>>
>> I personally use this in a non modifying way "using the -dry-run mode" to 
>> catch new"east/const violations" and report failure back rather than "change 
>> the code itself"
>>
>> I would not expect clang-format usage to change unless someone specially 
>> opted in to using it.
>
> That seems just as bad, if not worse.  Clang-format isn't an analysis tool, 
> its a format tool.  If you have an option that can only reasonably be run in 
> 'dry-run' mode, it seems that putting it in a 'format' tool is the wrong 
> place.

This is exactly what the "llvm-premerge checks" do!, why can't it also be an 
analysis tool? your own usage scenario may not be the same as everyone elses, 
that doesn't make it wrong!

> If you have an option that can only reasonably be run in 'dry-run' mode

This isn't true, I successfully "east consted" all of clang causing no build 
errors and from what I can tell no unit test failure either, I'm just saying 
using it in dry-run mode is an equally useful usage scenario.


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] D69764: [clang-format] Add East/West Const fixer capability

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D69764#2934489 , @MyDeveloperDay 
wrote:

> In D69764#2934378 , @erichkeane 
> wrote:
>
>> I've just been watching this from the sideline, but the cases where this 
>> breaks code are unacceptable for this tool, it is a complete direction 
>> change for the tool, and making that direction change silently on a review 
>> of a 15 month patch, where TWO code owners have said 'no' for that reason is 
>> absurd.
>>
>> I use this tool daily as a part of my 'upload' script, having it silently 
>> bust code between when I validate it and when I upload it is terrible, and 
>> makes the tool unusable for my purposes.  If we change this direction 
>> without a full RFC, my next step is going to be an RFC to remove 
>> clang-format from the check-in requirements of the entire LLVM project.
>
> Can I just say, marking this review as requiring changes, I presume because 
> you don't agree with it conceptually isn't very helpful to the consensus 
> building process, unless you have an inline comment of something you think is 
> wrong.  What changes are you requesting?
>
> F18452578: image.png 
>
> I've tried to find compromises to mitigate peoples strong views, I know this 
> is contentious, I've not tried to rush it in. I am a major contributor to 
> clang-format, and I'd like to continue to move it forward..
>
> as @klimek mentioned I'm one of the people really trying to help look after 
> clang-format, I wouldn't do anything that I think damages it or its 
> reputation, but I think there is value in this and other proposed changes. I 
> know some people disagree but we also have to recognise that some agree too!
>
> A "no" is a "no for everyone", a "yes" is a "yes and no" based on the 
> configuration, I know what I think is the fairer approach.

My 'requires changes' is that this needs an LLVM-project-level RFC to change 
the charter of one of its projects, doing so in a 15 month long patch, against 
the wishes of TWO maintainers is a violation of the LLVM community practice.  
I'm completely willing to disagree-and-commit here once that happens, but 
allowing this patch in without that decision being made intentionally by the 
project seems like a violation of trust.


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] D105462: [X86] Add CRC32 feature.

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: clang/lib/Headers/crc32intrin.h:13
+
+#define __DEFAULT_FN_ATTRS \
+  __attribute__((__always_inline__, __nodebug__, __target__("crc32")))

Better to follow Lint's suggestions.



Comment at: clang/lib/Headers/crc32intrin.h:31
+static __inline__ unsigned int __DEFAULT_FN_ATTRS
+_mm_crc32_u8(unsigned int __C, unsigned char __D)
+{

ditto.



Comment at: clang/lib/Headers/immintrin.h:518
+defined(__CRC32__)
+#include 
+#endif

Should it be better to move together with "include "?



Comment at: clang/lib/Headers/smmintrin.h:2347
 
+#include 
+

Should it be added to gprintrin.h too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105462

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

[PATCH] D105331: [CFE][X86] Enable complex _Float16.

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added a comment.

In D105331#2933893 , @LuoYuanke wrote:

> Would you check the failure of the test cases?

They failed due to they are built based on the ToT instead of the 1st patch of 
FP16.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105331

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

[PATCH] D106815: Update: clang/test/Profile/gcc-flag-compatibility.c to have -flto on AIX

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added a comment.

In D106815#2926363 , @madanial wrote:

> I need some help to commit this change as I do not have commit access as of 
> now.

I will commit it for you today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106815

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

[PATCH] D107648: [OpenCL] Clang diagnostics allow reporting C++ for OpenCL version.

[Justas Janickas via Phabricator via cfe-commits]

Topotuna updated this revision to Diff 365179.
Topotuna added a comment.

Diagnostic `err_opencl_unknown_type_specifier` addressed. Deprecation 
conditions changed in `nosvm` attribute test.


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

https://reviews.llvm.org/D107648

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/LangOptions.h
  clang/lib/Basic/LangOptions.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Frontend/opencl.cl
  clang/test/SemaOpenCL/ext_vectors.cl
  clang/test/SemaOpenCL/nosvm.cl

Index: clang/test/SemaOpenCL/nosvm.cl
===
--- clang/test/SemaOpenCL/nosvm.cl
+++ clang/test/SemaOpenCL/nosvm.cl
@@ -1,13 +1,16 @@
 // RUN: %clang_cc1 -verify %s
 // RUN: %clang_cc1 -verify -cl-std=CL2.0 -D CL20 %s
+// RUN: %clang_cc1 -verify -cl-std=clc++1.0 -D CLCPP10 %s
 // RUN: %clang_cc1 -verify -x c -D NOCL %s
 
 #ifndef NOCL
 kernel void f(__attribute__((nosvm)) global int* a);
-#ifndef CL20
-// expected-error@-2 {{'nosvm' attribute requires OpenCL version 2.0}}
+#if defined(CL20)
+// expected-warning@-2 {{'nosvm' attribute is deprecated and ignored in OpenCL C version 2.0}}
+#elif defined(CLCPP10)
+// expected-warning@-4 {{'nosvm' attribute is deprecated and ignored in C++ for OpenCL version 1.0}}
 #else
-// expected-warning@-4 {{'nosvm' attribute is deprecated and ignored in OpenCL version 2.0}}
+// expected-error@-6 {{attribute 'nosvm' is supported in the OpenCL version 2.0 onwards}}
 #endif
 
 __attribute__((nosvm)) void g(); // expected-warning {{'nosvm' attribute only applies to variables}}
Index: clang/test/SemaOpenCL/ext_vectors.cl
===
--- clang/test/SemaOpenCL/ext_vectors.cl
+++ clang/test/SemaOpenCL/ext_vectors.cl
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL1.1
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL2.0
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL3.0
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=clc++1.0
 
 typedef float float4 __attribute__((ext_vector_type(4)));
 
@@ -9,12 +10,12 @@
 
   V = V.abgr;
 #if (__OPENCL_C_VERSION__ < 300)
-  // expected-warning@-2 {{vector component name 'a' is an OpenCL C version 3.0 feature}}
+  // expected-warning@-2 {{vector component name 'a' is a feature from OpenCL version 3.0 onwards}}
 #endif
 
   V = V.xyzr;
   // expected-error@-1 {{illegal vector component name 'r'}}
 #if (__OPENCL_C_VERSION__ < 300)
-  // expected-warning@-3 {{vector component name 'r' is an OpenCL C version 3.0 feature}}
+  // expected-warning@-3 {{vector component name 'r' is a feature from OpenCL version 3.0 onwards}}
 #endif
 }
Index: clang/test/Frontend/opencl.cl
===
--- clang/test/Frontend/opencl.cl
+++ clang/test/Frontend/opencl.cl
@@ -2,7 +2,7 @@
 // RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=CL1.1 -DSYNTAX
 // RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=CL1.2 -DSYNTAX
 // RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=CL2.0 -DSYNTAX
-// RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=clc++ -DSYNTAX
+// RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=clc++1.0 -DSYNTAX
 // RUN: %clang_cc1 %s -verify -fsyntax-only -fblocks -DBLOCKS -DSYNTAX
 // RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=CL1.1 -fblocks -DBLOCKS -DSYNTAX
 // RUN: %clang_cc1 %s -verify -fsyntax-only -cl-std=CL1.2 -fblocks -DBLOCKS -DSYNTAX
@@ -10,6 +10,7 @@
 // RUN: %clang_cc1 -cl-std=CL1.1 -cl-strict-aliasing -fblocks %s 2>&1 | FileCheck --check-prefix=CHECK-INVALID-OPENCL-VERSION11 %s
 // RUN: %clang_cc1 -cl-std=CL1.2 -cl-strict-aliasing -fblocks %s 2>&1 | FileCheck --check-prefix=CHECK-INVALID-OPENCL-VERSION12 %s
 // RUN: %clang_cc1 -cl-std=CL2.0 -cl-strict-aliasing %s 2>&1 | FileCheck --check-prefix=CHECK-INVALID-OPENCL-VERSION20 %s
+// RUN: %clang_cc1 -cl-std=clc++1.0 -cl-strict-aliasing -fblocks %s 2>&1 | FileCheck --check-prefix=CHECK-INVALID-OPENCLCPP-VERSION10 %s
 
 #ifdef SYNTAX
 class test{
@@ -30,6 +31,7 @@
   // expected-error@-6{{blocks support disabled - compile with -fblocks or pick a deployment target that supports them}}
 #endif
 
-// CHECK-INVALID-OPENCL-VERSION11: warning: OpenCL version 1.1 does not support the option '-cl-strict-aliasing'
-// CHECK-INVALID-OPENCL-VERSION12: warning: OpenCL version 1.2 does not support the option '-cl-strict-aliasing'
-// CHECK-INVALID-OPENCL-VERSION20: warning: OpenCL version 2.0 does not support the option '-cl-strict-aliasing'
+// CHECK-INVALID-OPENCL-VERSION11: warning: OpenCL C version 1.1 does not support the option '-cl-strict-aliasing'
+// CHECK-INVALID-OPENCL-VERSION12: warni

[PATCH] D105177: [clangd] Implemented indexing of standard library

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel updated this revision to Diff 365181.
kuhnel marked 29 inline comments as done.
kuhnel edited the summary of this revision.
kuhnel added a comment.
Herald added a subscriber: mgrang.

addressed code review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105177

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/FS.h
  clang-tools-extra/clangd/index/StdLib.cpp
  clang-tools-extra/clangd/index/StdLib.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/StdLibIndexTests.cpp

Index: clang-tools-extra/clangd/unittests/StdLibIndexTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/StdLibIndexTests.cpp
@@ -0,0 +1,58 @@
+//===-- StdLibIndexTests.cpp *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "TestFS.h"
+#include "TestIndex.h"
+#include "index/StdLib.h"
+#include "support/Logger.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace testing;
+
+namespace clang {
+namespace clangd {
+
+/// check that the generated header sources contains some usual standard library
+/// headers
+TEST(StdLibIndexTests, generateUmbrellaHeader) {
+  MockFS FS;
+  StandardLibraryIndex SLI(FS, StandardLibraryVersion::cxx14);
+  auto UmbrellaHeader = SLI.generateIncludeHeader();
+
+  EXPECT_THAT(UmbrellaHeader, HasSubstr("#include "));
+  EXPECT_THAT(UmbrellaHeader, HasSubstr("#include "));
+  EXPECT_THAT(UmbrellaHeader, HasSubstr("#include "));
+}
+
+/// build the index and check if it contains the right symbols
+TEST(StdLibIndexTests, buildIndex) {
+  MockFS FS;
+  StandardLibraryIndex SLI(FS, StandardLibraryVersion::cxx14);
+  // TODO: maybe find a way to use a local libcxx for testing if that is
+  //   available on the machine
+  std::string HeaderMock = R"CPP(
+int myfunc(int a);
+bool otherfunc(int a, int b);
+  )CPP";
+  auto Index = SLI.indexHeaders(HeaderMock);
+  EXPECT_THAT_EXPECTED(Index, llvm::Succeeded());
+
+  FuzzyFindRequest Req;
+  Req.AnyScope = true;
+
+  EXPECT_THAT(match(**Index, Req), UnorderedElementsAre("myfunc", "otherfunc"));
+}
+
+// TODO: add tests for indexStandardLibrary()
+// TODO: test with different library versions
+
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -79,6 +79,7 @@
   SemanticSelectionTests.cpp
   SerializationTests.cpp
   SourceCodeTests.cpp
+  StdLibIndexTests.cpp
   SymbolCollectorTests.cpp
   SymbolInfoTests.cpp
   SyncAPI.cpp
Index: clang-tools-extra/clangd/index/StdLib.h
===
--- /dev/null
+++ clang-tools-extra/clangd/index/StdLib.h
@@ -0,0 +1,70 @@
+//===--- StdLib.h *- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+// Clangd indexer for the C++ standard library.
+//
+// The index only contains symbols that are part of the translation unit. So
+// if your translation unit does not yet #include , you do not get
+// auto completion for std::string. However we expect that many users would
+// like to use the the standard library anyway, so we could index that by
+// default an offer e.g. code completion without requiring #includes.
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_STDLIB_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_STDLIB_H
+
+#include "index/Index.h"
+#include "index/MemIndex.h"
+#include "support/ThreadsafeFS.h"
+#include "clang/AST/Expr.h"
+#include "llvm/ADT/StringRef.h"
+#include 
+
+namespace clang {
+namespace clangd {
+
+// Enumeration of supported Standard Library versions.
+// FIXME: support muiltiple languages (e.g. C and C++) and versions (e.g. 11,
+// 14, 17) of the standard library. Right now hardcoded to one verison.
+// FIXME: add feature to detect this version somehow (magically).
+enum StandardLibraryVersion { cxx14 = 0 };
+
+// external interface for getting a standard library index.
+Expected>
+indexStandardLibrary(const Threa

[PATCH] D105177: [clangd] Implemented indexing of standard library

[Christian Kühnel via Phabricator via cfe-commits]

kuhnel added a comment.

> Main points in the implementation are:
>
> - simplify the exposed interface

Good point, I added a new function `indexStandardLibrary()` as external 
interface.

> - i don't think we need to mess with VFS at all actually

Yes, removed that.

> - we should think a little about which index data we actually want to keep/use

I removed Refs, Relations and Graph as per your comment. However I have to 
admit, I don't know what they are and how they are used.
What's a good place to look at so that I learn what they do?

> Next design questions seem to be about lifetime/triggering:
>
> - how many configurations of stdlib index to have
> - when do we build the indexes, and who owns them
> - what logic governs whether/which stdlib index is triggered, and where do we 
> put it

While you're out, I'll try to set up something so that I can do some manual 
tests with a real standard library.




Comment at: clang-tools-extra/clangd/index/StdLib.cpp:98
+  auto Buffer = FS->getBufferForFile(StdLibHeaderFileName);
+  if (!Buffer)
+return error("Could not read file from InMemoryFileSystem");

sammccall wrote:
> CreateMemBuffer instead to avoid expressing this impossible error condition?
Not sure what you mean with CreateMemBuffer.
I replaced the `if` with an `assert` as this should not fail.



Comment at: clang-tools-extra/clangd/index/StdLib.cpp:101
+  auto Clang = prepareCompilerInstance(std::move(CI), /*Preamble=*/nullptr,
+   std::move(*Buffer), FS, IgnoreDiags);
+  if (!Clang)

sammccall wrote:
> hang on, if we're providing the overridden buffer to prepareCompilerInstance 
> (like we do for edited files in clangd that may or may not be saved on disk 
> yet) then why do we need the VFS at all?
We don't need the VFS at all, you're right. Handing over a buffer is good 
enough.



Comment at: clang-tools-extra/clangd/unittests/StdLibIndexTests.cpp:47
+  //   available on the machine
+  std::string HeaderMock = R"CPP(
+int myfunc(int a);

sammccall wrote:
> I'm not sure we're testing what we want to be testing here.
> 
> In real life, the symbols are not going to be in the entrypoint, but a file 
> included from it.
> And clangd's indexer *does* treat the main file differently from others.
> 
> It's pretty awkward to actually fix but maybe worth a comment.
So you would prefer that we change `HeaderMock` to only contain `#include` and 
then create the mock headers as virtual files in the MockFS?



Comment at: clang-tools-extra/clangd/unittests/StdLibIndexTests.cpp:56
+  Req.Query = "myfunc";
+  Req.AnyScope = true;
+  Req.Limit = 100;

sammccall wrote:
> AnyScope and Limit are not needed
`AnyScope` is actually needed, otherwise the result is empty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105177

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

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

> My 'requires changes' is that this needs an LLVM-project-level RFC to change 
> the charter of one of its projects, doing so in a 15 month long patch, 
> against the wishes of TWO maintainers is a violation of the LLVM community 
> practice.  I'm completely willing to disagree-and-commit here once that 
> happens, but allowing this patch in without that decision being made 
> intentionally by the project seems like a violation of trust.

Ok, thats fair and thank you for verbalising what the changes are. I'm not 
closed to the idea of the RFC just didn't want to go down that road if it just 
ended up with 2 opposing views and not getting to a conclusion.

I will challenge a couple of things:

1. I'm not sure there is currently a "charter" that says we won't modify the 
contents of a TU, and actually in my view that has already changed when we 
added. (include sorting, namespace comments, javascript requoter, trailing 
comment inserter).

2. I agree if we want to use this as formalising that "change" in charter then 
I'm ok to try via the RFC but I think we'll get 2 very opposing views, and 
likely no concencus. So I don't want to just cause a rift in the community any 
more than this is already.

3. As for the "TWO maintainers", I don't deny their extremely excellent 
contributions, far greater than mine could ever be. But in fairness they are 
not frequent maintainers here! On the other hand I am and have been for a 
number of years. When @djasper and @klimek stepped back a bit, I've really 
tried to help by filling even a little, the void of their enormous shoes.

I can't even think of emulating all those peoples amazing efforts, but I do 
this in my free time as I assume other do, and I like to think there is value 
in me continuing to improve and debug clang-format. So I'd like to think my 
view isn't disregarded just because others with more muscle disagree, I mean I 
assumed this was at least a democracy where we could find a fair concencus.


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] D69764: [clang-format] Add East/West Const fixer capability

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D69764#2934535 , @MyDeveloperDay 
wrote:

>> My 'requires changes' is that this needs an LLVM-project-level RFC to change 
>> the charter of one of its projects, doing so in a 15 month long patch, 
>> against the wishes of TWO maintainers is a violation of the LLVM community 
>> practice.  I'm completely willing to disagree-and-commit here once that 
>> happens, but allowing this patch in without that decision being made 
>> intentionally by the project seems like a violation of trust.
>
> Ok, thats fair and thank you for verbalising what the changes are. I'm not 
> closed to the idea of the RFC just didn't want to go down that road if it 
> just ended up with 2 opposing views and not getting to a conclusion.
>
> I will challenge a couple of things:
>
> 1. I'm not sure there is currently a "charter" that says we won't modify the 
> contents of a TU, and actually in my view that has already changed when we 
> added. (include sorting, namespace comments, javascript requoter, trailing 
> comment inserter).
>
> 2. I agree if we want to use this as formalising that "change" in charter 
> then I'm ok to try via the RFC but I think we'll get 2 very opposing views, 
> and likely no concencus. So I don't want to just cause a rift in the 
> community any more than this is already.

For better or worse, RFCs are our way of changing these things. RFCs definitely 
succeed after discussion sometimes. _I_ would be completely against this patch 
unless some level of community consensus was formed via RFC, and I believe a 
few others above have made the same point.

> 3. As for the "TWO maintainers", I don't deny their extremely excellent 
> contributions, far greater than mine could ever be. But in fairness they are 
> not frequent maintainers here! On the other hand I am and have been for a 
> number of years. When @djasper and @klimek stepped back a bit, I've really 
> tried to help by filling even a little, the void of their enormous shoes.
>
> I can't even think of emulating all those peoples amazing efforts, but I do 
> this in my free time as I assume other do, and I like to think there is value 
> in me continuing to improve and debug clang-format.

I was referring to @rsmith and @aaron.ballman (to clarify), both are 
maintainers in 'clang', the former of which is the 'superset' maintainer of 
this format project based on its directory. While Aaron is a peer-maintainer to 
this project, his contributions are immense, and his points are 
too-well-reasoned and motivated to be dismissed.

> So I'd like to think my view isn't disregarded just because others with more 
> muscle disagree, I mean I assumed this was at least a democracy where we 
> could find a fair concencus.

It _IS_ a democracy where we can find a fair consensus!  And the mechanism with 
which to obtain said `fair consensus` is an RFC.


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] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

> It _IS_ a democracy where we can find a fair consensus!  And the mechanism 
> with which to obtain said `fair consensus` is an RFC.

Then I think in the interest of finding one we should start with the RFC.


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

[clang] 39ca3e5 - Update: clang/test/Profile/gcc-flag-compatibility.c to have -flto on AIX

[Whitney Tsang via cfe-commits]

Author: Mark Danial
Date: 2021-08-09T14:57:38Z
New Revision: 39ca3e5541d0f9634ee22e3bcd932c47cb37306a

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

LOG: Update: clang/test/Profile/gcc-flag-compatibility.c to have -flto on AIX

Reviewed By: Whitney

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

Added: 
clang/test/Profile/gcc-flag-compatibility-aix.c

Modified: 
clang/test/Profile/gcc-flag-compatibility.c

Removed: 




diff  --git a/clang/test/Profile/gcc-flag-compatibility-aix.c 
b/clang/test/Profile/gcc-flag-compatibility-aix.c
new file mode 100644
index 0..215adfc0e4a3b
--- /dev/null
+++ b/clang/test/Profile/gcc-flag-compatibility-aix.c
@@ -0,0 +1,68 @@
+// Tests for -fprofile-generate and -fprofile-use flag compatibility. These two
+// flags behave similarly to their GCC counterparts:
+//
+// -fprofile-generate Generates the profile file ./default.profraw
+// -fprofile-generate=   Generates the profile file /default.profraw
+// -fprofile-use  Uses the profile file ./default.profdata
+// -fprofile-use=Uses the profile file /default.profdata
+// -fprofile-use=/file   Uses the profile file /file
+
+// On AIX, -flto is required with -fprofile-generate
+
+// RUN: %clang %s -c -S -o - -emit-llvm -target powerpc64-unknown-aix -flto 
-fprofile-generate -fno-experimental-new-pass-manager | FileCheck 
-check-prefix=PROFILE-GEN %s
+// RUN: %clang %s -c -S -o - -emit-llvm -target powerpc64-unknown-aix -flto 
-fprofile-generate -fexperimental-new-pass-manager | FileCheck 
-check-prefix=PROFILE-GEN %s
+// PROFILE-GEN: @__profc_main = {{(private|internal)}} global [2 x i64] 
zeroinitializer, section
+// PROFILE-GEN: @__profd_main =
+
+// Check that -fprofile-generate=/path/to generates /path/to/default.profraw
+// RUN: %clang %s -c -S -o - -emit-llvm -target powerpc64-unknown-aix -flto 
-fprofile-generate=/path/to -fno-experimental-new-pass-manager | FileCheck 
-check-prefixes=PROFILE-GEN,PROFILE-GEN-EQ %s
+// RxUN: %clang %s -c -S -o - -emit-llvm -target powerpc64-unknown-aix -flto 
-fprofile-generate=/path/to -fexperimental-new-pass-manager | FileCheck 
-check-prefixes=PROFILE-GEN,PROFILE-GEN-EQ %s
+// PROFILE-GEN-EQ: constant [{{.*}} x i8] c"/path/to{{/|}}{{.*}}\00"
+
+// Check that -fprofile-use=some/path reads some/path/default.profdata
+// This uses Clang FE format profile.
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir/some/path
+// RUN: llvm-profdata merge %S/Inputs/gcc-flag-compatibility.proftext -o 
%t.dir/some/path/default.profdata
+// RUN: %clang %s -o - -Xclang -disable-llvm-passes -emit-llvm -S 
-fprofile-use=%t.dir/some/path -fno-experimental-new-pass-manager | FileCheck 
-check-prefix=PROFILE-USE %s
+// RUN: %clang %s -o - -Xclang -disable-llvm-passes -emit-llvm -S 
-fprofile-use=%t.dir/some/path -fexperimental-new-pass-manager | FileCheck 
-check-prefix=PROFILE-USE %s
+
+// Check that -fprofile-use=some/path/file.prof reads some/path/file.prof
+// This uses Clang FE format profile.
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir/some/path
+// RUN: llvm-profdata merge %S/Inputs/gcc-flag-compatibility.proftext -o 
%t.dir/some/path/file.prof
+// RUN: %clang %s -o - -Xclang -disable-llvm-passes -emit-llvm -S 
-fprofile-use=%t.dir/some/path/file.prof -fno-experimental-new-pass-manager | 
FileCheck -check-prefix=PROFILE-USE %s
+// RUN: %clang %s -o - -Xclang -disable-llvm-passes -emit-llvm -S 
-fprofile-use=%t.dir/some/path/file.prof -fexperimental-new-pass-manager | 
FileCheck -check-prefix=PROFILE-USE %s
+// PROFILE-USE: = !{!"branch_weights", i32 101, i32 2}
+
+// Check that -fprofile-use=some/path reads some/path/default.profdata
+// This uses LLVM IR format profile.
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir/some/path
+// RUN: llvm-profdata merge %S/Inputs/gcc-flag-compatibility_IR.proftext -o 
%t.dir/some/path/default.profdata
+// RUN: %clang %s -o - -emit-llvm -S -fprofile-use=%t.dir/some/path 
-fno-experimental-new-pass-manager | FileCheck -check-prefix=PROFILE-USE-IR %s
+// RUN: %clang %s -o - -emit-llvm -S -fprofile-use=%t.dir/some/path 
-fexperimental-new-pass-manager | FileCheck -check-prefix=PROFILE-USE-IR %s
+
+// Check that -fprofile-use=some/path/file.prof reads some/path/file.prof
+// This uses LLVM IR format profile.
+// RUN: rm -rf %t.dir
+// RUN: mkdir -p %t.dir/some/path
+// RUN: llvm-profdata merge %S/Inputs/gcc-flag-compatibility_IR.proftext -o 
%t.dir/some/path/file.prof
+// RUN: %clang %s -o - -emit-llvm -S -fprofile-use=%t.dir/some/path/file.prof 
-fno-experimental-new-pass-manager | FileCheck -check-prefix=PROFILE-USE-IR %s
+// RUN: %clang %s -o - -emit-llvm -S -fprofile-use=%t.dir/some/path/file.prof 
-fexperimental-new-pass-manager | FileCheck -check-prefix=PROFILE-USE-IR %s
+//
+// RUN

[PATCH] D106815: Update: clang/test/Profile/gcc-flag-compatibility.c to have -flto on AIX

[Whitney Tsang 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 rG39ca3e5541d0: Update: 
clang/test/Profile/gcc-flag-compatibility.c to have -flto on AIX (authored by 
madanial, committed by Whitney).

Changed prior to commit:
  https://reviews.llvm.org/D106815?vs=362829&id=365185#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106815

Files:
  clang/test/Profile/gcc-flag-compatibility-aix.c
  clang/test/Profile/gcc-flag-compatibility.c


Index: clang/test/Profile/gcc-flag-compatibility.c
===
--- clang/test/Profile/gcc-flag-compatibility.c
+++ clang/test/Profile/gcc-flag-compatibility.c
@@ -7,6 +7,8 @@
 // -fprofile-use=Uses the profile file /default.profdata
 // -fprofile-use=/file   Uses the profile file /file
 
+// On AIX, -flto is required with -fprofile-generate. 
gcc-flag-compatibility-aix.c is used to do the testing on AIX with -flto
+// XFAIL: aix
 // RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate 
-fno-experimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN %s
 // RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate 
-fexperimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN %s
 // PROFILE-GEN: @__profc_main = {{(private|internal)}} global [2 x i64] 
zeroinitializer, section
Index: clang/test/Profile/gcc-flag-compatibility-aix.c
===
--- clang/test/Profile/gcc-flag-compatibility-aix.c
+++ clang/test/Profile/gcc-flag-compatibility-aix.c
@@ -7,14 +7,16 @@
 // -fprofile-use=Uses the profile file /default.profdata
 // -fprofile-use=/file   Uses the profile file /file
 
-// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate 
-fno-experimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN %s
-// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate 
-fexperimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN %s
+// On AIX, -flto is required with -fprofile-generate
+
+// RUN: %clang %s -c -S -o - -emit-llvm -target powerpc64-unknown-aix -flto 
-fprofile-generate -fno-experimental-new-pass-manager | FileCheck 
-check-prefix=PROFILE-GEN %s
+// RUN: %clang %s -c -S -o - -emit-llvm -target powerpc64-unknown-aix -flto 
-fprofile-generate -fexperimental-new-pass-manager | FileCheck 
-check-prefix=PROFILE-GEN %s
 // PROFILE-GEN: @__profc_main = {{(private|internal)}} global [2 x i64] 
zeroinitializer, section
 // PROFILE-GEN: @__profd_main =
 
 // Check that -fprofile-generate=/path/to generates /path/to/default.profraw
-// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate=/path/to 
-fno-experimental-new-pass-manager | FileCheck 
-check-prefixes=PROFILE-GEN,PROFILE-GEN-EQ %s
-// RxUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate=/path/to 
-fexperimental-new-pass-manager | FileCheck 
-check-prefixes=PROFILE-GEN,PROFILE-GEN-EQ %s
+// RUN: %clang %s -c -S -o - -emit-llvm -target powerpc64-unknown-aix -flto 
-fprofile-generate=/path/to -fno-experimental-new-pass-manager | FileCheck 
-check-prefixes=PROFILE-GEN,PROFILE-GEN-EQ %s
+// RxUN: %clang %s -c -S -o - -emit-llvm -target powerpc64-unknown-aix -flto 
-fprofile-generate=/path/to -fexperimental-new-pass-manager | FileCheck 
-check-prefixes=PROFILE-GEN,PROFILE-GEN-EQ %s
 // PROFILE-GEN-EQ: constant [{{.*}} x i8] c"/path/to{{/|}}{{.*}}\00"
 
 // Check that -fprofile-use=some/path reads some/path/default.profdata


Index: clang/test/Profile/gcc-flag-compatibility.c
===
--- clang/test/Profile/gcc-flag-compatibility.c
+++ clang/test/Profile/gcc-flag-compatibility.c
@@ -7,6 +7,8 @@
 // -fprofile-use=Uses the profile file /default.profdata
 // -fprofile-use=/file   Uses the profile file /file
 
+// On AIX, -flto is required with -fprofile-generate. gcc-flag-compatibility-aix.c is used to do the testing on AIX with -flto
+// XFAIL: aix
 // RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate -fno-experimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN %s
 // RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate -fexperimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN %s
 // PROFILE-GEN: @__profc_main = {{(private|internal)}} global [2 x i64] zeroinitializer, section
Index: clang/test/Profile/gcc-flag-compatibility-aix.c
===
--- clang/test/Profile/gcc-flag-compatibility-aix.c
+++ clang/test/Profile/gcc-flag-compatibility-aix.c
@@ -7,14 +7,16 @@
 // -fprofile-use=Uses the profile file /default.profdata
 // -fprofile-use=/file   Uses the profile file /file
 
-// RUN: %clang %s -c -S -o - -emit-llvm -fprofile-generate -fno-experimental-new-pass-manager | FileCheck -check-prefix=PROFILE-GEN %s
-// RUN: %clang %s -c -S -

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D69764#2934560 , @MyDeveloperDay 
wrote:

>> It _IS_ a democracy where we can find a fair consensus!  And the mechanism 
>> with which to obtain said `fair consensus` is an RFC.
>
> Then I think in the interest of finding one we should start with the RFC.

FWIW, if we gain consensus, I go from a -1000 on this to a +.75.  From a 
technical perspective, I find it weird that we aren't handling ALL of the CV 
qualifiers.

From a "we should be inclusive" perspective, my understanding is that 
east==right, west==left is considered by some to be euro-centric due to its 
north-up bias. I don't feel strongly about it, but it at least gives me a 
somewhat significant preference for left/right vs east/west.


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] D107696: [CodeComplete] Basic code completion for attribute names.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4357
+continue;
+  llvm::StringRef Name = S.NormalizedFullName;
+  if (Completion == AttributeCompletion::Scope) {

aaron.ballman wrote:
> Should we also add some special handling for attributes optionally starting 
> with double underscores? e.g., `__attribute__((const))` and 
> `__attribute__((__const__))`` are both equally useful to complete.
> 
> Also, we should add some explicit testing for `omp::sequence` and 
> `omp::directive` as those are handled very specially and won't appear in the 
> parsed attribute map. I think the OpenMP code completion would be super 
> useful, but can be handled in a follow-up if you want.
> Should we also add some special handling for attributes optionally starting 
> with double underscores?

I think so. Two questions:
 - Do I understand right that this is "just" a matter of adding 
leading/trailing `__` as a second option, for AS_GNU?
 - are there similar rules for other syntaxes I've missed?

Offering both seems potentially confusing for users who don't care (especially 
in the case of const!). But I guess enough users will care about macros. At 
least in clangd the underscore versions will get ranked lower for having 
"internal" names though.

FWIW The no-underscores version appears to be ~3x more common (87k vs 27k hits 
in third-party code in google's repo). Among headers, no-underscores is "only" 
2x more common (40k vs 21k). 

---

> Also, we should add some explicit testing for omp::sequence and 
> omp::directive as those are handled very specially and won't appear in the 
> parsed attribute map. 

Yeah, I punted on these because it seems they will need special case logic, 
I'll add some tests that they don't do anything.


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] D107242: [AIX] Define __HOS_AIX__ macro

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

clang is fundamentally a cross-compiler only. I don't see any point for having 
host-specific branches in this case at all. Either the macro should be 
specified for the target all the time or not at all, but it should most 
definitely not depend on the host. That's actually breaking a number of 
existing use case for clang in subtle ways, e.g. partially preprocessed files 
(`-frewrite-includes`) should behave the same on any platform given the same 
command line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107242

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

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.



> I was referring to @rsmith and @aaron.ballman (to clarify), both are 
> maintainers in 'clang', the former of which is the 'superset' maintainer of 
> this format project based on its directory. While Aaron is a peer-maintainer 
> to this project, his contributions are immense, and his points are 
> too-well-reasoned and motivated to be dismissed.

Just so we are clear I'm not dismissing anyone opinions, @arron.ballman and I 
have been going at this quite a bit both on and off list. I have huge respect 
for these people, (even if the defence of my review it might not seem so). I 
can't even think to emulate their contributions.

Ideally I'd like their blessing, but alas I fear that might be beyond my 
capabilities, but if there are things like the RFC that could even go some way 
to potentially them seeing a way forward that is mutually beneficial so that 
the door is even open a jar for this then I'm happy to try.

If ultimately the answer is "no" then I need to understand what can be done if 
anything, if that means a new tool then I'm ok with that as the compromise. Out 
right dismissal, I would be very sorry to see.


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] D106262: [clang][analyzer] Use generic note tag in alpha.unix.Stream .

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

Szelethus added a comment.

Alright, I think we're almost ready to go! I left a few comments, please make 
sure to mark those done that you feel are properly addressed.




Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:393-394
+  /// in this sense if a resource leak is detected later.
+  /// At a bug report the "failed operation" is always the last in the path
+  /// (where this note is displayed) and previous such notes are not displayed.
+  const NoteTag *constructFailureNoteTag(CheckerContext &C, SymbolRef 
StreamSym,

The main thing to highlight here is that its not only the last failing 
operation, but more importantly that this operation caused the bug to occur.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:412-413
+  /// the path (if no other failing operation follows).
+  /// This note is inserted into places where something important about
+  /// the last failing operation is discovered.
+  const NoteTag *constructNonFailureNoteTag(CheckerContext &C,

Same here, mention that its the failed stream operation that **caused** the bug 
is what we're specifying further.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:518
+  C.addTransition(StateNull,
+  constructFailureNoteTag(C, RetSym, "Stream open fails 
here"));
 }





Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:576
+  C.addTransition(StateRetNull, constructFailureNoteTag(
+C, StreamSym, "Stream reopen fails here"));
 }





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.");

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.



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"));

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.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:791
+  constructFailureNoteTag(
+  C, StreamSym, "Operation fails or stream reaches end-of-file here"));
 }

Like here.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:933
   BT_UseAfterClose,
-  "Stream might be already closed. Causes undefined behaviour.", N));
+  "Stream might be already closed. Causes undefined behaviour.", N);
+  R->markInteresting(Sym);

Please leave a TODO here, don't fix now.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:969
   "File position of the stream might be 'indeterminate' "
   "after a failed operation. "
   "Can cause undefined behavior.";

Stating that it happened as a result of a failed operation seems kind of 
redundant, especially if the `NoteTag` states that as well. Lets leave a TODO 
here to address this warning message, but leave as-is for now.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:1158
 }
\ No newline at end of file


Lets put one there!



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:387-391
+  /// Create a NoteTag to display a note if a later bug report is generated.
+  /// The `BT` should contain all bug typ

[PATCH] D107720: [analyzer] Cleanup a FIXME in SValBuilder.cpp

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers added a comment.

I'll add a test and repost. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107720

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

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.



> I find it weird that we aren't handling ALL of the CV qualifiers.

I will probably try and address this, I do have some ideas, but this will I 
believe complicate the implementation. For now I really want to understand if 
conceptually such a feature can be landed, not so much land it in entirely this 
form. if I can get some agreement and that really means that people need to 
generally be in agreement.

> From a "we should be inclusive" perspective, my understanding is that 
> east==right, west==left is considered by some to be euro-centric due to its 
> north-up bias. I don't feel strongly about it, but it at least gives me a 
> somewhat significant preference for left/right vs east/west.

I think this is partially historical because I was following the whole east 
const vs west const 
(https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rl-const,https://mariusbancila.ro/blog/2018/11/23/join-the-east-const-revolution/,
 https://www.youtube.com/watch?v=fv--IKZFVO8). I don't have a problem 
supporting one/both or either, I tend to think that is more arguing semantics.

After seeing those videos I wanted clang-format to end the east vs west war, 
like clang-format ended the whitespace war!, I didn't realise I'd cause a 
clang war in the process! oops!! ;-)


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] D69764: [clang-format] Add East/West Const fixer capability

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D69764#2934646 , @MyDeveloperDay 
wrote:

>> I find it weird that we aren't handling ALL of the CV qualifiers.
>
> I will probably try and address this, I do have some ideas, but this will I 
> believe complicate the implementation. For now I really want to understand if 
> conceptually such a feature can be landed, not so much land it in entirely 
> this form. if I can get some agreement and that really means that people need 
> to generally be in agreement.

Understood.

>> From a "we should be inclusive" perspective, my understanding is that 
>> east==right, west==left is considered by some to be euro-centric due to its 
>> north-up bias. I don't feel strongly about it, but it at least gives me a 
>> somewhat significant preference for left/right vs east/west.
>
> I think this is partially historical because I was following the whole east 
> const vs west const 
> (https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rl-const,https://mariusbancila.ro/blog/2018/11/23/join-the-east-const-revolution/,
>  https://www.youtube.com/watch?v=fv--IKZFVO8). I don't have a problem 
> supporting one/both or either, I tend to think that is more arguing semantics.
>
> After seeing those videos I wanted clang-format to end the east vs west war, 
> like clang-format ended the whitespace war!, I didn't realise I'd cause a 
> clang war in the process! oops!! ;-)

I understand the origin of east/west in this case, I was around when those 
bracelets came out :)  Someone internally pointed out the anti-inclusivity of 
the terminology, so I figured I'd bring it up.


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] D69764: [clang-format] Add East/West Const fixer capability

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D69764#2934612 , @MyDeveloperDay 
wrote:

>> I was referring to @rsmith and @aaron.ballman (to clarify), both are 
>> maintainers in 'clang', the former of which is the 'superset' maintainer of 
>> this format project based on its directory. While Aaron is a peer-maintainer 
>> to this project, his contributions are immense, and his points are 
>> too-well-reasoned and motivated to be dismissed.
>
> Just so we are clear I'm not dismissing anyone opinions, @arron.ballman and I 
> have been going at this quite a bit both on and off list. I have huge respect 
> for these people, (even if the defence of my review it might not seem so). I 
> can't even think to emulate their contributions.
>
> Ideally I'd like their blessing, but alas I fear that might be beyond my 
> capabilities, but if there are things like the RFC that could even go some 
> way to potentially them seeing a way forward that is mutually beneficial so 
> that the door is even open a jar for this then I'm happy to try.
>
> If ultimately the answer is "no" then I need to understand what can be done 
> if anything, if that means a new tool then I'm ok with that as the 
> compromise. Out right dismissal, I would be very sorry to see.

Here are my thoughts put in one place.

0) I like the idea of being able to format qualifier location, and I think 
clang-format is the tool I would reach for to perform that work
.33) I wish this was generalized to handle all qualifiers rather than just 
`const`. (This is not a blocking wish.)
.66) I wish this used "left" and "right" rather than "east" and "west" for 
user-facing options and documentation. (This is Very Strong wish but I won't 
block the patch over it.)

1. In general, I do not like the idea of my code formatting tool having 
opinions about the input source's existing formatting (I think valid code 
should remain valid). This is the area of concern causing me to ask for an RFC 
because I'm operating under the impression that not breaking code is 
(generally) the status quo for clang-format.
2. If the community consensus is that formatting can break code (blanket 
permission, case by case basis, whatever), I'll definitely abide by that 
decision. I just want it to be an explicit decision from a wider audience than 
people who might be following this very long-running patch.
3. The proposed opt-in nature of the check is a blessing and a curse. It 
alleviates some of the danger (it's not dangerous by default, you have to 
manually say you want it). But it doesn't eliminate the danger (code can still 
be broken) and it does raise the question of how often people opt into this 
sort of thing and whether it's worth the maintenance costs. I don't think any 
of us can answer those "do people opt in" questions though, so that's not a 
concern I think we should hold anything up over unless someone has actual data 
on usage of opt-in features for clang-format. I think opt-in alleviates enough 
of the danger that it's a worthwhile approach for this patch (and likely may be 
a good idea for a policy on future changes of the same kind).

tl;dr: if there's an RFC and the community says "breaking changes aren't that 
big of a deal to us", it tips me from "against" to "in favor" for this 
functionality.


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] D107717: [LLVM][CMake][NFC] Resolve FIXME: Rename LLVM_CMAKE_PATH to LLVM_CMAKE_DIR throughout the project

[Louis Dionne via Phabricator via cfe-commits]

ldionne accepted this revision.
ldionne added a comment.

This LGTM.


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] D106644: [clang][analyzer] Add standard streams to alpha.unix.Stream checker.

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

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

It's fine on my part.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106644

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

[PATCH] D105904: [clangd] Support `#pragma mark` in the outline

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 365196.
dgoldman marked 9 inline comments as done.
dgoldman added a comment.

Address nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105904

Files:
  clang-tools-extra/clangd/CollectMacros.cpp
  clang-tools-extra/clangd/CollectMacros.h
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Preamble.h
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
  clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
  clang/include/clang/Lex/PPCallbacks.h

Index: clang/include/clang/Lex/PPCallbacks.h
===
--- clang/include/clang/Lex/PPCallbacks.h
+++ clang/include/clang/Lex/PPCallbacks.h
@@ -492,6 +492,11 @@
 Second->PragmaComment(Loc, Kind, Str);
   }
 
+  void PragmaMark(SourceLocation Loc, StringRef Trivia) override {
+First->PragmaMark(Loc, Trivia);
+Second->PragmaMark(Loc, Trivia);
+  }
+
   void PragmaDetectMismatch(SourceLocation Loc, StringRef Name,
 StringRef Value) override {
 First->PragmaDetectMismatch(Loc, Name, Value);
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -99,6 +99,8 @@
   return arg.beginOffset() == R.Begin && arg.endOffset() == R.End;
 }
 
+MATCHER_P(PragmaTrivia, P, "") { return arg.Trivia == P; }
+
 MATCHER(EqInc, "") {
   Inclusion Actual = testing::get<0>(arg);
   Inclusion Expected = testing::get<1>(arg);
@@ -886,6 +888,27 @@
   EXPECT_FALSE(mainIsGuarded(AST));
 }
 
+TEST(ParsedASTTest, DiscoversPragmaMarks) {
+  TestTU TU;
+  TU.AdditionalFiles["Header.h"] = R"(
+#pragma mark - Something API
+int something();
+#pragma mark Something else
+  )";
+  TU.Code = R"cpp(
+#include "Header.h"
+#pragma mark In Preamble
+#pragma mark - Something Impl
+int something() { return 1; }
+#pragma mark End
+  )cpp";
+  auto AST = TU.build();
+
+  EXPECT_THAT(AST.getMarks(), ElementsAre(PragmaTrivia(" In Preamble"),
+  PragmaTrivia(" - Something Impl"),
+  PragmaTrivia(" End")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -42,7 +42,7 @@
 MATCHER_P(SymNameRange, Range, "") { return arg.selectionRange == Range; }
 template 
 ::testing::Matcher Children(ChildMatchers... ChildrenM) {
-  return Field(&DocumentSymbol::children, ElementsAre(ChildrenM...));
+  return Field(&DocumentSymbol::children, UnorderedElementsAre(ChildrenM...));
 }
 
 std::vector getSymbols(TestTU &TU, llvm::StringRef Query,
@@ -1027,6 +1027,101 @@
 AllOf(WithName("-pur"), WithKind(SymbolKind::Method));
 }
 
+TEST(DocumentSymbolsTest, PragmaMarkGroups) {
+  TestTU TU;
+  TU.ExtraArgs = {"-xobjective-c++", "-Wno-objc-root-class"};
+  Annotations Main(R"cpp(
+  $DogDef[[@interface Dog
+  @end]]
+
+  $DogImpl[[@implementation Dog
+
+  + (id)sharedDoggo { return 0; }
+
+  #pragma $Overrides[[mark - Overrides
+
+  - (id)init {
+return self;
+  }
+  - (void)bark {}]]
+
+  #pragma $Specifics[[mark - Dog Specifics
+
+  - (int)isAGoodBoy {
+return 1;
+  }]]
+  @]]end  // FIXME: Why doesn't this include the 'end'?
+
+  #pragma $End[[mark - End
+]]
+)cpp");
+  TU.Code = Main.code().str();
+  EXPECT_THAT(
+  getSymbols(TU.build()),
+  UnorderedElementsAre(
+  AllOf(WithName("Dog"), SymRange(Main.range("DogDef"))),
+  AllOf(WithName("Dog"), SymRange(Main.range("DogImpl")),
+Children(AllOf(WithName("+sharedDoggo"),
+   WithKind(SymbolKind::Method)),
+ AllOf(WithName("Overrides"),
+   SymRange(Main.range("Overrides")),
+   Children(AllOf(WithName("-init"),
+  WithKind(SymbolKind::Method)),
+AllOf(WithName("-bark"),
+  WithKind(SymbolKind::Method,
+ AllOf(WithName("Dog Specifics"),
+   SymRange(Main.range("Specifics")),
+   Children(AllOf(WithName("-isAGoodBoy"),
+  

[PATCH] D105904: [clangd] Support `#pragma mark` in the outline

[David Goldman via Phabricator via cfe-commits]

dgoldman added inline comments.



Comment at: clang-tools-extra/clangd/CollectMacros.cpp:42
+if (isInsideMainFile(Loc, SM)) {
+  Position Start = sourceLocToPosition(SM, Loc);
+  Position End = {Start.line + 1, 0};

kadircet wrote:
> are we fine with these annotations spanning `#pragma [[mark XX]]` rather than 
> the whole line or just `XX` ?
I think it's okay for now, I added a FIXME which I'll address in a follow up



Comment at: clang-tools-extra/clangd/FindTarget.cpp:655
 llvm::iterator_range Locations) {
-  for (const auto &P : llvm::zip(Protocols, Locations)) {
 Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),

kadircet wrote:
> this change does not look relevant, can you please revert?
Reverted, will send out another change to fix the warning



Comment at: clang-tools-extra/clangd/ParsedAST.cpp:441
+  if (Preamble)
+Marks = Preamble->Marks;
+  Clang->getPreprocessor().addPPCallbacks(

kadircet wrote:
> We are not patching marks for stale preambles, which I believe is fine but 
> worth a FIXME. If you are interested please take a look at 
> https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/Preamble.cpp#L427,
>  it should be fairly easy to handle movements of `#pragma mark`s in preamble 
> section.
Added a FIXME. Does something similar need to be done for MainFileMacros?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105904

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

[PATCH] D107769: [OpenCL] Make generic addrspace optional for -fdeclare-opencl-builtins

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh created this revision.
svenvh added reviewers: Anastasia, airlied, azabaznov.
svenvh added a project: clang.
Herald added subscribers: ldrumm, yaxunl.
svenvh requested review of this revision.
Herald added a subscriber: cfe-commits.

Currently, -fdeclare-opencl-builtins always adds the generic address
space overloads of e.g. the vload builtin functions in OpenCL 3.0
mode, even when the generic address space feature is disabled.

Guard the generic address space overloads by the
`__opencl_c_generic_address_space` feature instead of by OpenCL
version.

Add a new field `RequireDisabledExtension` to the `Builtin` class so
that we can make certain builtins available only when an extension is
disabled.  Thus, we can provide generic address space overloads OR
private/global/local address space overloads depending on the generic
address space feature availability.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107769

Files:
  clang/lib/Sema/OpenCLBuiltins.td
  clang/lib/Sema/SemaLookup.cpp
  clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl
  clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
  clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp

Index: clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
===
--- clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
+++ clang/utils/TableGen/ClangOpenCLBuiltinEmitter.cpp
@@ -425,6 +425,8 @@
   const bool IsConv : 1;
   // OpenCL extension(s) required for this overload.
   const unsigned short Extension;
+  // OpenCL extension(s) required to be disabled for this overload.
+  const unsigned short DisabledExtension;
   // OpenCL versions in which this overload is available.
   const unsigned short Versions;
 };
@@ -611,6 +613,8 @@
 
 for (const auto &Overload : SLM.second.Signatures) {
   StringRef ExtName = Overload.first->getValueAsDef("Extension")->getName();
+  StringRef DisabledExtName =
+  Overload.first->getValueAsDef("RequireDisabledExtension")->getName();
   unsigned int MinVersion =
   Overload.first->getValueAsDef("MinVersion")->getValueAsInt("ID");
   unsigned int MaxVersion =
@@ -622,6 +626,7 @@
  << (Overload.first->getValueAsBit("IsConst")) << ", "
  << (Overload.first->getValueAsBit("IsConv")) << ", "
  << FunctionExtensionIndex[ExtName] << ", "
+ << FunctionExtensionIndex[DisabledExtName] << ", "
  << EncodeVersions(MinVersion, MaxVersion) << " },\n";
   Index++;
 }
@@ -648,7 +653,9 @@
 Rec->getValueAsDef("MaxVersion")->getValueAsInt("ID") ==
 Rec2->getValueAsDef("MaxVersion")->getValueAsInt("ID") &&
 Rec->getValueAsDef("Extension")->getName() ==
-Rec2->getValueAsDef("Extension")->getName()) {
+Rec2->getValueAsDef("Extension")->getName() &&
+Rec->getValueAsDef("RequireDisabledExtension")->getName() ==
+Rec2->getValueAsDef("RequireDisabledExtension")->getName()) {
   return true;
 }
   }
@@ -1085,11 +1092,27 @@
 OpenCLBuiltinFileEmitterBase::emitExtensionGuard(const Record *Builtin) {
   StringRef Extensions =
   Builtin->getValueAsDef("Extension")->getValueAsString("ExtName");
-  if (Extensions.empty())
-return "";
+  StringRef DisabledExtensions =
+  Builtin->getValueAsDef("RequireDisabledExtension")
+  ->getValueAsString("ExtName");
+
+  assert((Extensions.empty() || DisabledExtensions.empty()) &&
+ "enabling and disabling extensions simultaneously not supported yet!");
+
+  bool RequireDisabled = false;
+  if (Extensions.empty()) {
+if (DisabledExtensions.empty())
+  return "";
+
+Extensions = DisabledExtensions;
+RequireDisabled = true;
+  }
 
   OS << "#if";
 
+  // At this point, Extensions contains a space-separated list of either
+  // the required extensions or the required-to-be-disabled extensions.
+  // RequireDisabled is true if those extensions need to be disabled.
   SmallVector ExtVec;
   Extensions.split(ExtVec, " ");
   bool isFirst = true;
@@ -1097,7 +1120,11 @@
 if (!isFirst) {
   OS << " &&";
 }
-OS << " defined(" << Ext << ")";
+OS << " ";
+if (RequireDisabled) {
+  OS << "!";
+}
+OS << "defined(" << Ext << ")";
 isFirst = false;
   }
   OS << "\n";
Index: clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
===
--- clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
+++ clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
@@ -63,6 +63,7 @@
 
 // Enable extensions that are enabled in opencl-c-base.h.
 #if (defined(__OPENCL_CPP_VERSION__) || __OPENCL_C_VERSION__ >= 200)
+#define __opencl_c_generic_address_space 1
 #define cl_khr_subgroup_extended_types 1
 #define cl_khr_subgroup_ballot 1
 #define cl_khr_subgroup_non_uniform_arithmetic 1
Index: clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4357
+continue;
+  llvm::StringRef Name = S.NormalizedFullName;
+  if (Completion == AttributeCompletion::Scope) {

sammccall wrote:
> aaron.ballman wrote:
> > Should we also add some special handling for attributes optionally starting 
> > with double underscores? e.g., `__attribute__((const))` and 
> > `__attribute__((__const__))`` are both equally useful to complete.
> > 
> > Also, we should add some explicit testing for `omp::sequence` and 
> > `omp::directive` as those are handled very specially and won't appear in 
> > the parsed attribute map. I think the OpenMP code completion would be super 
> > useful, but can be handled in a follow-up if you want.
> > Should we also add some special handling for attributes optionally starting 
> > with double underscores?
> 
> I think so. Two questions:
>  - Do I understand right that this is "just" a matter of adding 
> leading/trailing `__` as a second option, for AS_GNU?
>  - are there similar rules for other syntaxes I've missed?
> 
> Offering both seems potentially confusing for users who don't care 
> (especially in the case of const!). But I guess enough users will care about 
> macros. At least in clangd the underscore versions will get ranked lower for 
> having "internal" names though.
> 
> FWIW The no-underscores version appears to be ~3x more common (87k vs 27k 
> hits in third-party code in google's repo). Among headers, no-underscores is 
> "only" 2x more common (40k vs 21k). 
> 
> ---
> 
> > Also, we should add some explicit testing for omp::sequence and 
> > omp::directive as those are handled very specially and won't appear in the 
> > parsed attribute map. 
> 
> Yeah, I punted on these because it seems they will need special case logic, 
> I'll add some tests that they don't do anything.
> Do I understand right that this is "just" a matter of adding leading/trailing 
> __ as a second option, for AS_GNU?
> are there similar rules for other syntaxes I've missed?

Clang supports GNU attributes in either `__attribute__((foo))` or 
`__attribute__((__foo__))` forms. So I'd say that autocompleting after the 
second `(` should either suggest attributes (preferred) or `__` (for the poor 
folks writing libraries). If the user wants to autocomplete after 
`__attribute__((__`, I think it should suggest `foo__` as the rest of the 
attribute name. (Basically, if the user looks like they want underscores, give 
them all the underscores.)

Clang also supports `[[]]` attributes but with somewhat different rules. We 
support `[[gnu::attr]]`, `[[__gnu__::attr]]`, `[[gnu::__attr__]]`, and 
`[[__gnu__::__attr__]]` for GCC attributes. We support `[[clang::attr]]`, 
`[[_Clang::attr]]`, `[[clang::__attr__]]`, and `[[_Clang::__attr__]]` for Clang 
attributes. For vendors other than Clang and GCC, we don't support any 
additional underscores for either the scope or the attribute name. I would say 
that if the user asked for underscores for the vendor scope, they likely want 
the underscores for the attribute as well.

I suppose there's a third case. That horrible `using` syntax that I've never 
really seen used in the wild. e.g., ``[[using clang: attr]``. We do support the 
underscore behavior there as well.

> Offering both seems potentially confusing for users who don't care 
> (especially in the case of const!). But I guess enough users will care about 
> macros.

Yeah, users who are writing portable libraries are far more likely to care than 
users writing typical application code.


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] D107718: [cuda] Mark builtin texture/surface reference variable as 'externally_initialized'.

[Michael Liao via Phabricator via cfe-commits]

hliao added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4441
   if (Linkage != llvm::GlobalValue::InternalLinkage &&
-  (D->hasAttr() || D->hasAttr()))
+  (D->hasAttr() || D->hasAttr() ||
+   D->getType()->isCUDADeviceBuiltinSurfaceType() ||

yaxunl wrote:
> we need also do this with variables with HIPManagedAttr and add a test for 
> that
those managed var seems already being marked 'externally_initalized' in 
lib/CodeGen/CGCUDANV.cpp. Do you have missing places to mark them?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107718

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

[PATCH] D107718: [cuda] Mark builtin texture/surface reference variable as 'externally_initialized'.

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

LGTM on HIP side. I am not sure whether CUDA needs this.




Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4441
   if (Linkage != llvm::GlobalValue::InternalLinkage &&
-  (D->hasAttr() || D->hasAttr()))
+  (D->hasAttr() || D->hasAttr() ||
+   D->getType()->isCUDADeviceBuiltinSurfaceType() ||

hliao wrote:
> yaxunl wrote:
> > we need also do this with variables with HIPManagedAttr and add a test for 
> > that
> those managed var seems already being marked 'externally_initalized' in 
> lib/CodeGen/CGCUDANV.cpp. Do you have missing places to mark them?
sorry, I fogot. Then it should be OK. We don't have other places to handle 
managed variables.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107718

___
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

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added inline comments.



Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:6964
+return DAG.getSetCC(DL, ResultVT, Op, DAG.getConstantFP(0.0, DL, 
OperandVT),
+ISD::SETUO);
+

sepavloff wrote:
> nemanjai wrote:
> > sepavloff wrote:
> > > efriedma wrote:
> > > > Maybe we want to consider falling back to the integer path if SETCC 
> > > > isn't legal for the given operand type?  We could do that as a 
> > > > followup, though.
> > > It makes sense, it could be beneficial for targets that have limited set 
> > > of floating point comparisons. However straightforward check like:
> > > 
> > > if (Flags.hasNoFPExcept() && isOperationLegalOrCustom(ISD::SETCC, 
> > > OperandVT))
> > > 
> > > results in worse code, mainly for vector types. It should be more complex 
> > > check.
> > >  
> > Out of curiosity, why was this added when you recognized that it results in 
> > worse code? This is certainly part of the reason for the regression for 
> > `ppc_fp128`.
> > 
> > It would appear that before this patch, we would emit a comparison for all 
> > types that are not IEEE FP types (such as `ppc_fp128`). Those semantics do 
> > not seem to have carried over.
> > Out of curiosity, why was this added when you recognized that it results in 
> > worse code? This is certainly part of the reason for the regression for 
> > ppc_fp128.
> 
> It is my mistake. After experiments I forgot to remove this change. I am 
> sorry.
> 
> For x86 and AArch64 I used modified `test-suite`, with changes from D106804. 
> Without proper tests it is hard to reveal why one intrinsic starts to fail.
> 
> > It would appear that before this patch, we would emit a comparison for all 
> > types that are not IEEE FP types (such as ppc_fp128). Those semantics do 
> > not seem to have carried over.
> 
> The previous behavior is not correct in non-default FP environment. Unordered 
> comparison raises Invalid exception if an operand is signaling NaN. On the 
> other hand, `isnan` must never raise exceptions.
Well, if the **must never raise exceptions** is an IEEE-754 requirement (i.e. 
as noted in 5.7.2), I think it is reasonable that operations on types that do 
not conform to IEEE-754 are not bound by it.


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] D105263: [X86] AVX512FP16 instructions enabling 1/6

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:599
 * SPIR
+* X86
 

Might be worth mentioning that it requires AVX512FP16 here



Comment at: clang/lib/CodeGen/TargetInfo.cpp:2817
   Current = SSE;
+} else if (k == BuiltinType::Float16) {
+  Current = SSE;

Merge with the previous if?



Comment at: clang/lib/CodeGen/TargetInfo.cpp:2948
 Lo = Hi = Integer;
+} else if (ET->isFloat16Type()) {
+  Current = SSE;

Merge with the FloatTy if?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105263

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

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

Please also see https://bugs.llvm.org/show_bug.cgi?id=51416


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

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

[Sam McCall via Phabricator via cfe-commits]

sammccall planned changes to this revision.
sammccall added a comment.

Thanks, I know what to do next!

While I have you here, any thoughts on future patches:

> Only the bare name is completed, with no args.
> For args to be useful we need arg names. These *are* in the tablegen but
> not currently emitted in usable form, so left this as future work.

How do you feel about this plan:

- add an `ArrayRef` to `ParsedAttrInfo` for this purpose? (Or a 
null-terminated `char**` if we're worried about sizeof).
- It'd be populated by the names of the tablegen `Args`.
- If empty, completions render as now. If nonempty they render as `foo(bar, 
baz)` where `bar` and `baz` are placeholders - just like function args but 
without types.

> There's also no filtering by langopts: this is because currently the
> only way of checking is to try to produce diagnostics, which requires a
> valid ParsedAttr which is hard to get.

And for this, WDYT about making `diagLangOpts` non-virtual and moving the 
attr-specific test into a `virtual bool supportsLangOpts()` function with no 
side-effects?




Comment at: clang/lib/Sema/SemaCodeComplete.cpp:4357
+continue;
+  llvm::StringRef Name = S.NormalizedFullName;
+  if (Completion == AttributeCompletion::Scope) {

aaron.ballman wrote:
> sammccall wrote:
> > aaron.ballman wrote:
> > > Should we also add some special handling for attributes optionally 
> > > starting with double underscores? e.g., `__attribute__((const))` and 
> > > `__attribute__((__const__))`` are both equally useful to complete.
> > > 
> > > Also, we should add some explicit testing for `omp::sequence` and 
> > > `omp::directive` as those are handled very specially and won't appear in 
> > > the parsed attribute map. I think the OpenMP code completion would be 
> > > super useful, but can be handled in a follow-up if you want.
> > > Should we also add some special handling for attributes optionally 
> > > starting with double underscores?
> > 
> > I think so. Two questions:
> >  - Do I understand right that this is "just" a matter of adding 
> > leading/trailing `__` as a second option, for AS_GNU?
> >  - are there similar rules for other syntaxes I've missed?
> > 
> > Offering both seems potentially confusing for users who don't care 
> > (especially in the case of const!). But I guess enough users will care 
> > about macros. At least in clangd the underscore versions will get ranked 
> > lower for having "internal" names though.
> > 
> > FWIW The no-underscores version appears to be ~3x more common (87k vs 27k 
> > hits in third-party code in google's repo). Among headers, no-underscores 
> > is "only" 2x more common (40k vs 21k). 
> > 
> > ---
> > 
> > > Also, we should add some explicit testing for omp::sequence and 
> > > omp::directive as those are handled very specially and won't appear in 
> > > the parsed attribute map. 
> > 
> > Yeah, I punted on these because it seems they will need special case logic, 
> > I'll add some tests that they don't do anything.
> > Do I understand right that this is "just" a matter of adding 
> > leading/trailing __ as a second option, for AS_GNU?
> > are there similar rules for other syntaxes I've missed?
> 
> Clang supports GNU attributes in either `__attribute__((foo))` or 
> `__attribute__((__foo__))` forms. So I'd say that autocompleting after the 
> second `(` should either suggest attributes (preferred) or `__` (for the poor 
> folks writing libraries). If the user wants to autocomplete after 
> `__attribute__((__`, I think it should suggest `foo__` as the rest of the 
> attribute name. (Basically, if the user looks like they want underscores, 
> give them all the underscores.)
> 
> Clang also supports `[[]]` attributes but with somewhat different rules. We 
> support `[[gnu::attr]]`, `[[__gnu__::attr]]`, `[[gnu::__attr__]]`, and 
> `[[__gnu__::__attr__]]` for GCC attributes. We support `[[clang::attr]]`, 
> `[[_Clang::attr]]`, `[[clang::__attr__]]`, and `[[_Clang::__attr__]]` for 
> Clang attributes. For vendors other than Clang and GCC, we don't support any 
> additional underscores for either the scope or the attribute name. I would 
> say that if the user asked for underscores for the vendor scope, they likely 
> want the underscores for the attribute as well.
> 
> I suppose there's a third case. That horrible `using` syntax that I've never 
> really seen used in the wild. e.g., ``[[using clang: attr]``. We do support 
> the underscore behavior there as well.
> 
> > Offering both seems potentially confusing for users who don't care 
> > (especially in the case of const!). But I guess enough users will care 
> > about macros.
> 
> Yeah, users who are writing portable libraries are far more likely to care 
> than users writing typical application code.
> So I'd say that autocompleting after the second ( should either suggest 
> attributes (preferred) or __ (for the poor folks writing libraries).

This is clever, unfort

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

[Anshil Gandhi via Phabricator via cfe-commits]

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

rebase


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 Atomic instructions
 ; GCN-O1-OPTS-NEXT:

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

[Anshil Gandhi via Phabricator via cfe-commits]

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

- emit CAS loop remark only if the instruction is of floating point type


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-

[PATCH] D107667: [clang/test] Run thinlto-clang-diagnostic-handler-in-be.c on x86

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

Please don't use `x86_64` as target or triple since it might default to some 
weird platform. Can you spell out a valid triple/platform?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107667

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

[PATCH] D107718: [cuda] Mark builtin texture/surface reference variable as 'externally_initialized'.

[Artem Belevich via Phabricator via cfe-commits]

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

CUDA lowers texture/surface types into a special handle. I do not think 
`externally_initialized` matters for it.
AFAICT this change is a no-op for CUDA.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107718

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

[clang] 6ec36d1 - [cuda] Mark builtin texture/surface reference variable as 'externally_initialized'.

[Michael Liao via cfe-commits]

Author: Michael Liao
Date: 2021-08-09T13:27:40-04:00
New Revision: 6ec36d18ec7b29b471bbe50502beb5b35de3975c

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

LOG: [cuda] Mark builtin texture/surface reference variable as 
'externally_initialized'.

- They need to be preserved even if there's no reference within the
  device code as the host code may need to initialize them based on the
  application logic.

Reviewed By: tra

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

Added: 


Modified: 
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGenCUDA/surface.cu
clang/test/CodeGenCUDA/texture.cu

Removed: 




diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 49a1396b58e3a..13520861fe9b6 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -4438,7 +4438,9 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl 
*D,
   if (GV && LangOpts.CUDA) {
 if (LangOpts.CUDAIsDevice) {
   if (Linkage != llvm::GlobalValue::InternalLinkage &&
-  (D->hasAttr() || D->hasAttr()))
+  (D->hasAttr() || D->hasAttr() ||
+   D->getType()->isCUDADeviceBuiltinSurfaceType() ||
+   D->getType()->isCUDADeviceBuiltinTextureType()))
 GV->setExternallyInitialized(true);
 } else {
   getCUDARuntime().internalizeDeviceSideVar(D, Linkage);

diff  --git a/clang/test/CodeGenCUDA/surface.cu 
b/clang/test/CodeGenCUDA/surface.cu
index 0bf17091081b1..eedae5473fcfc 100644
--- a/clang/test/CodeGenCUDA/surface.cu
+++ b/clang/test/CodeGenCUDA/surface.cu
@@ -19,7 +19,7 @@ struct __attribute__((device_builtin_surface_type)) 
surface : public
 };
 
 // On the device side, surface references are represented as `i64` handles.
-// DEVICE: @surf ={{.*}} addrspace(1) global i64 undef, align 4
+// DEVICE: @surf ={{.*}} addrspace(1) externally_initialized global i64 undef, 
align 4
 // On the host side, they remain in the original type.
 // HOST: @surf = internal global %struct.surface
 // HOST: @0 = private unnamed_addr constant [5 x i8] c"surf\00"

diff  --git a/clang/test/CodeGenCUDA/texture.cu 
b/clang/test/CodeGenCUDA/texture.cu
index 8a966194340aa..0bb8cd48dcaa7 100644
--- a/clang/test/CodeGenCUDA/texture.cu
+++ b/clang/test/CodeGenCUDA/texture.cu
@@ -19,8 +19,8 @@ struct __attribute__((device_builtin_texture_type)) texture : 
public textureRefe
 };
 
 // On the device side, texture references are represented as `i64` handles.
-// DEVICE: @tex ={{.*}} addrspace(1) global i64 undef, align 4
-// DEVICE: @norm ={{.*}} addrspace(1) global i64 undef, align 4
+// DEVICE: @tex ={{.*}} addrspace(1) externally_initialized global i64 undef, 
align 4
+// DEVICE: @norm ={{.*}} addrspace(1) externally_initialized global i64 undef, 
align 4
 // On the host side, they remain in the original type.
 // HOST: @tex = internal global %struct.texture
 // HOST: @norm = internal global %struct.texture



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

[PATCH] D107718: [cuda] Mark builtin texture/surface reference variable as 'externally_initialized'.

[Michael Liao via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6ec36d18ec7b: [cuda] Mark builtin texture/surface reference 
variable as… (authored by hliao).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107718

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGenCUDA/surface.cu
  clang/test/CodeGenCUDA/texture.cu


Index: clang/test/CodeGenCUDA/texture.cu
===
--- clang/test/CodeGenCUDA/texture.cu
+++ clang/test/CodeGenCUDA/texture.cu
@@ -19,8 +19,8 @@
 };
 
 // On the device side, texture references are represented as `i64` handles.
-// DEVICE: @tex ={{.*}} addrspace(1) global i64 undef, align 4
-// DEVICE: @norm ={{.*}} addrspace(1) global i64 undef, align 4
+// DEVICE: @tex ={{.*}} addrspace(1) externally_initialized global i64 undef, 
align 4
+// DEVICE: @norm ={{.*}} addrspace(1) externally_initialized global i64 undef, 
align 4
 // On the host side, they remain in the original type.
 // HOST: @tex = internal global %struct.texture
 // HOST: @norm = internal global %struct.texture
Index: clang/test/CodeGenCUDA/surface.cu
===
--- clang/test/CodeGenCUDA/surface.cu
+++ clang/test/CodeGenCUDA/surface.cu
@@ -19,7 +19,7 @@
 };
 
 // On the device side, surface references are represented as `i64` handles.
-// DEVICE: @surf ={{.*}} addrspace(1) global i64 undef, align 4
+// DEVICE: @surf ={{.*}} addrspace(1) externally_initialized global i64 undef, 
align 4
 // On the host side, they remain in the original type.
 // HOST: @surf = internal global %struct.surface
 // HOST: @0 = private unnamed_addr constant [5 x i8] c"surf\00"
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4438,7 +4438,9 @@
   if (GV && LangOpts.CUDA) {
 if (LangOpts.CUDAIsDevice) {
   if (Linkage != llvm::GlobalValue::InternalLinkage &&
-  (D->hasAttr() || D->hasAttr()))
+  (D->hasAttr() || D->hasAttr() ||
+   D->getType()->isCUDADeviceBuiltinSurfaceType() ||
+   D->getType()->isCUDADeviceBuiltinTextureType()))
 GV->setExternallyInitialized(true);
 } else {
   getCUDARuntime().internalizeDeviceSideVar(D, Linkage);


Index: clang/test/CodeGenCUDA/texture.cu
===
--- clang/test/CodeGenCUDA/texture.cu
+++ clang/test/CodeGenCUDA/texture.cu
@@ -19,8 +19,8 @@
 };
 
 // On the device side, texture references are represented as `i64` handles.
-// DEVICE: @tex ={{.*}} addrspace(1) global i64 undef, align 4
-// DEVICE: @norm ={{.*}} addrspace(1) global i64 undef, align 4
+// DEVICE: @tex ={{.*}} addrspace(1) externally_initialized global i64 undef, align 4
+// DEVICE: @norm ={{.*}} addrspace(1) externally_initialized global i64 undef, align 4
 // On the host side, they remain in the original type.
 // HOST: @tex = internal global %struct.texture
 // HOST: @norm = internal global %struct.texture
Index: clang/test/CodeGenCUDA/surface.cu
===
--- clang/test/CodeGenCUDA/surface.cu
+++ clang/test/CodeGenCUDA/surface.cu
@@ -19,7 +19,7 @@
 };
 
 // On the device side, surface references are represented as `i64` handles.
-// DEVICE: @surf ={{.*}} addrspace(1) global i64 undef, align 4
+// DEVICE: @surf ={{.*}} addrspace(1) externally_initialized global i64 undef, align 4
 // On the host side, they remain in the original type.
 // HOST: @surf = internal global %struct.surface
 // HOST: @0 = private unnamed_addr constant [5 x i8] c"surf\00"
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -4438,7 +4438,9 @@
   if (GV && LangOpts.CUDA) {
 if (LangOpts.CUDAIsDevice) {
   if (Linkage != llvm::GlobalValue::InternalLinkage &&
-  (D->hasAttr() || D->hasAttr()))
+  (D->hasAttr() || D->hasAttr() ||
+   D->getType()->isCUDADeviceBuiltinSurfaceType() ||
+   D->getType()->isCUDADeviceBuiltinTextureType()))
 GV->setExternallyInitialized(true);
 } else {
   getCUDARuntime().internalizeDeviceSideVar(D, Linkage);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107723: [Clang] Moved warning of warnReturnTypestateForUnconsumableType back to SemaDeclAttr

[Alf via Phabricator via cfe-commits]

gAlfonso-bit updated this revision to Diff 365224.
gAlfonso-bit added a comment.

Moved warnReturnTypestateForUnconsumableType back to SemaDeclCXX


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

https://reviews.llvm.org/D107723

Files:
  clang/include/clang/Analysis/Analyses/Consumed.h
  clang/lib/Analysis/Consumed.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/SemaCXX/warn-consumed-analysis.cpp
  clang/test/SemaCXX/warn-consumed-parsing.cpp

Index: clang/test/SemaCXX/warn-consumed-parsing.cpp
===
--- clang/test/SemaCXX/warn-consumed-parsing.cpp
+++ clang/test/SemaCXX/warn-consumed-parsing.cpp
@@ -6,16 +6,6 @@
 #define RETURN_TYPESTATE(state) __attribute__ ((return_typestate(state)))
 #define TEST_TYPESTATE(state)   __attribute__ ((test_typestate(state)))
 
-// FIXME: This test is here because the warning is issued by the Consumed
-//analysis, not SemaDeclAttr.  The analysis won't run after an error
-//has been issued.  Once the attribute propagation for template
-//instantiation has been fixed, this can be moved somewhere else and the
-//definition can be removed.
-int returnTypestateForUnconsumable() RETURN_TYPESTATE(consumed); // expected-warning {{return state set for an unconsumable type 'int'}}
-int returnTypestateForUnconsumable() {
-  return 0;
-}
-
 class AttrTester0 {
   void consumes()   __attribute__ ((set_typestate())); // expected-error {{'set_typestate' attribute takes one argument}}
   bool testUnconsumed() __attribute__ ((test_typestate())); // expected-error {{'test_typestate' attribute takes one argument}}
Index: clang/test/SemaCXX/warn-consumed-analysis.cpp
===
--- clang/test/SemaCXX/warn-consumed-analysis.cpp
+++ clang/test/SemaCXX/warn-consumed-analysis.cpp
@@ -74,6 +74,11 @@
 void baf5(ConsumableClass  *var);
 void baf6(ConsumableClass &&var);
 
+int returnTypestateForUnconsumable() RETURN_TYPESTATE(consumed); // expected-warning {{return state set for an unconsumable type 'int'}}
+int returnTypestateForUnconsumable() {
+  return 0;
+}
+
 ConsumableClass returnsUnconsumed() {
   return ConsumableClass(); // expected-warning {{return value not in expected state; expected 'unconsumed', observed 'consumed'}}
 }
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -1190,17 +1190,14 @@
 return;
   }
 
-  // FIXME: This check is currently being done in the analysis.  It can be
-  //enabled here only after the parser propagates attributes at
-  //template specialization definition, not declaration.
-  //QualType ReturnType = cast(D)->getType();
-  //const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl();
-  //
-  //if (!RD || !RD->hasAttr()) {
-  //S.Diag(AL.getLoc(), diag::warn_return_state_for_unconsumable_type) <<
-  //  ReturnType.getAsString();
-  //return;
-  //}
+  QualType ReturnType = cast(D)->getType();
+  const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl();
+
+  if (!RD || !RD->hasAttr()) {
+S.Diag(AL.getLoc(), diag::warn_return_typestate_for_unconsumable_type)
+<< ReturnType.getAsString();
+return;
+  }
 
   D->addAttr(::new (S.Context) ParamTypestateAttr(S.Context, AL, ParamState));
 }
@@ -1222,30 +1219,27 @@
 return;
   }
 
-  // FIXME: This check is currently being done in the analysis.  It can be
-  //enabled here only after the parser propagates attributes at
-  //template specialization definition, not declaration.
-  //QualType ReturnType;
-  //
-  //if (const ParmVarDecl *Param = dyn_cast(D)) {
-  //  ReturnType = Param->getType();
-  //
-  //} else if (const CXXConstructorDecl *Constructor =
-  // dyn_cast(D)) {
-  //  ReturnType = Constructor->getThisType()->getPointeeType();
-  //
-  //} else {
-  //
-  //  ReturnType = cast(D)->getCallResultType();
-  //}
-  //
-  //const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl();
-  //
-  //if (!RD || !RD->hasAttr()) {
-  //S.Diag(Attr.getLoc(), diag::warn_return_state_for_unconsumable_type) <<
-  //  ReturnType.getAsString();
-  //return;
-  //}
+  QualType ReturnType;
+
+  if (const ParmVarDecl *Param = dyn_cast(D)) {
+ReturnType = Param->getType();
+
+  } else if (const CXXConstructorDecl *Constructor =
+ dyn_cast(D)) {
+ReturnType = Constructor->getThisType()->getPointeeType();
+
+  } else {
+
+ReturnType = cast(D)->getCallResultType();
+  }
+
+  const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl();
+
+  if (!RD || !RD->hasAttr()) {
+S.Diag(AL.getLoc(), diag::warn_return_typestate_for_unconsumable_type)
+<< ReturnType.getAsString();
+return;
+  }
 
   D->addAttr(::new (S.Context) ReturnTypestateAttr(S.Context, AL,

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D107696#2934881 , @sammccall wrote:

> Thanks, I know what to do next!
>
> While I have you here, any thoughts on future patches:
>
>> Only the bare name is completed, with no args.
>> For args to be useful we need arg names. These *are* in the tablegen but
>> not currently emitted in usable form, so left this as future work.
>
> How do you feel about this plan:
>
> - add an `ArrayRef` to `ParsedAttrInfo` for this purpose? (Or a 
> null-terminated `char**` if we're worried about sizeof).
> - It'd be populated by the names of the tablegen `Args`.
> - If empty, completions render as now. If nonempty they render as `foo(bar, 
> baz)` where `bar` and `baz` are placeholders - just like function args but 
> without types.

I think args may be... interesting. Many attributes have arguments that can't 
really be completed because they're an integer, a string literal, whatever. But 
there are still quite a few that accept an enumeration, and those enumeration 
values would be super useful to autocomplete. And still others have arbitrary 
expressions where autocompletion would be fantastic (like `diagnose_if`).

I'm not certain that an `ArrayRef` would be sufficient to the task, but I do 
think we could probably generate something out of ClangAttrEmitter.cpp to help 
on an attribute-by-attribute basis.

>> There's also no filtering by langopts: this is because currently the
>> only way of checking is to try to produce diagnostics, which requires a
>> valid ParsedAttr which is hard to get.
>
> And for this, WDYT about making `diagLangOpts` non-virtual and moving the 
> attr-specific test into a `virtual bool supportsLangOpts()` function with no 
> side-effects?

I think that'd work fine -- it'll break plugin authors, but that should 
hopefully be a loud break when they go to recompile their plugin.


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] D106891: [AMDGPU] [Remarks] Emit optimization remarks for FP atomics

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: llvm/lib/CodeGen/AtomicExpandPass.cpp:626-632
+  if (AI->isFloatingPointOperation())
+emitAtomicExpansionRemarks(
+AI, Kind,
+Remark << "A hardware CAS loop generated: if the memory is "
+  "known to be coarse-grain allocated then a hardware "
+  "floating-point"
+  " atomic could be requested");

This is an AMDGPU specific message/restriction. The floating point operation 
isn't relevant, and you don't even know the specific reason at this point.


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] D69764: [clang-format] Add East/West Const fixer capability

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D69764#2934686 , @aaron.ballman 
wrote:

> In D69764#2934612 , @MyDeveloperDay 
> wrote:
>
>>> I was referring to @rsmith and @aaron.ballman (to clarify), both are 
>>> maintainers in 'clang', the former of which is the 'superset' maintainer of 
>>> this format project based on its directory. While Aaron is a 
>>> peer-maintainer to this project, his contributions are immense, and his 
>>> points are too-well-reasoned and motivated to be dismissed.
>>
>> Just so we are clear I'm not dismissing anyone opinions, @arron.ballman and 
>> I have been going at this quite a bit both on and off list. I have huge 
>> respect for these people, (even if the defence of my review it might not 
>> seem so). I can't even think to emulate their contributions.
>>
>> Ideally I'd like their blessing, but alas I fear that might be beyond my 
>> capabilities, but if there are things like the RFC that could even go some 
>> way to potentially them seeing a way forward that is mutually beneficial so 
>> that the door is even open a jar for this then I'm happy to try.
>>
>> If ultimately the answer is "no" then I need to understand what can be done 
>> if anything, if that means a new tool then I'm ok with that as the 
>> compromise. Out right dismissal, I would be very sorry to see.
>
> Here are my thoughts put in one place.
>
> 0) I like the idea of being able to format qualifier location, and I think 
> clang-format is the tool I would reach for to perform that work
> .33) I wish this was generalized to handle all qualifiers rather than just 
> `const`. (This is not a blocking wish.)
> .66) I wish this used "left" and "right" rather than "east" and "west" for 
> user-facing options and documentation. (This is Very Strong wish but I won't 
> block the patch over it.)
>
> 1. In general, I do not like the idea of my code formatting tool having 
> opinions about the input source's existing formatting (I think valid code 
> should remain valid). This is the area of concern causing me to ask for an 
> RFC because I'm operating under the impression that not breaking code is 
> (generally) the status quo for clang-format.
> 2. If the community consensus is that formatting can break code (blanket 
> permission, case by case basis, whatever), I'll definitely abide by that 
> decision. I just want it to be an explicit decision from a wider audience 
> than people who might be following this very long-running patch.
> 3. The proposed opt-in nature of the check is a blessing and a curse. It 
> alleviates some of the danger (it's not dangerous by default, you have to 
> manually say you want it). But it doesn't eliminate the danger (code can 
> still be broken) and it does raise the question of how often people opt into 
> this sort of thing and whether it's worth the maintenance costs. I don't 
> think any of us can answer those "do people opt in" questions though, so 
> that's not a concern I think we should hold anything up over unless someone 
> has actual data on usage of opt-in features for clang-format. I think opt-in 
> alleviates enough of the danger that it's a worthwhile approach for this 
> patch (and likely may be a good idea for a policy on future changes of the 
> same kind).
>
> tl;dr: if there's an RFC and the community says "breaking changes aren't that 
> big of a deal to us", it tips me from "against" to "in favor" for this 
> functionality.

Happy to go the RFC route if @MyDeveloperDay wants to do that.

FWIW, I don't understand the idea of the community being a democracy. It is 
not, and never was. Chris has designed an escalation process, when this was 
raised to him - I don't know whether this was ever made official or tested.
I also don't see this as a big change from clang-format's principles - C++ is 
(unfortunately) way too complex to not be able to introduce subtle bugs - the 
history of clang-format is full of them.

Note that I don't think the change will get consensus on an RFC, but not making 
the change will not get consensus, either - in that case, other than escalating 
to a clear higher decision power, or letting main contributors make the call, I 
don't know what to do.


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] D107095: Implement #pragma clang header_unsafe

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:3913
+Clang supports the pragma ``#pragma clang header_unsafe``, which can be used to
+mark macros as unsafe to use in headers. This can be valuable when providing
+headers with ABI stability requirements. For example:

I think it should be made more clear as to whether this "unsafe to use in 
headers" means "unsafe to use in the header declaring the attribute as unsafe" 
vs "unsafe to use in any header included by the one marked as unsafe.", etc.

There's also the rather interesting example of:
```
// foo.h
#define FROBBLE 1
#pragma clang header_unsafe(FROBBLE, "why would you ever frobble things?")

// bar.h
#if 1
#elifdef FROBBLE
#endif

// source.c
#include "foo.h"
#include "bar.h"
```
Do we diagnose use of FROBBLE in bar.h despite it not directly including foo.h? 
Do we diagnose even though it's in a branch that's not possible to take? 
Does/should the answer change if we swap the order of includes? What if we 
change `#if 1` into something that's actually variable like `FOO > 8`?



Comment at: clang/include/clang/Basic/DiagnosticGroups.td:1309
+
+// Warnings and extensions to make preprocessor macro usage pedantic
+def PedanticMacros : DiagGroup<"pedantic-macros",

Speaking of making things pedantic... :-D



Comment at: clang/include/clang/Basic/IdentifierTable.h:127
 
-  // 24 bits left in a 64-bit word.
+  // True if this macro is unsafe in headers
+  unsigned IsHeaderUnsafe : 1;





Comment at: clang/include/clang/Basic/IdentifierTable.h:192-193
 } else {
+  // Since calling the setters of these calls recompute, just set them
+  // manually to avoid calling recompute a bunch of times.
+  IsDeprecatedMacro = false;





Comment at: clang/include/clang/Lex/Preprocessor.h:798
 
+  /// Usage warning for macros marked by #pragma clang header_unsafe
+  llvm::DenseMap HeaderUnsafeMacroMsgs;





Comment at: clang/include/clang/Lex/Preprocessor.h:2414
+  void addHeaderUnsafeMsg(const IdentifierInfo *II, std::string Msg) {
+HeaderUnsafeMacroMsgs.insert(std::make_pair(II, Msg));
+  }

Is this going to cause a copy for `Msg` without a call to `move()`?



Comment at: clang/lib/Lex/Preprocessor.cpp:1416-1434
+void Preprocessor::emitMacroDeprecationWarning(const Token &Identifier) {
+  auto DepMsg = getMacroDeprecationMsg(Identifier.getIdentifierInfo());
+  if (!DepMsg)
+Diag(Identifier, diag::warn_pragma_deprecated_macro_use)
+<< Identifier.getIdentifierInfo() << 0;
+  else
+Diag(Identifier, diag::warn_pragma_deprecated_macro_use)

I think it'd make sense to have a "macro marked  here" note in both 
of these pragmas (the deprecated one can be handled separately as a later 
thing), WDYT?



Comment at: clang/test/Lexer/Inputs/unsafe-macro-2.h:23-26
+// not-expected-warning@+1{{macro 'UNSAFE_MACRO_2' has been marked as unsafe 
for use in headers}}
+#undef UNSAFE_MACRO_2
+// not-expected-warning@+1{{macro 'UNSAFE_MACRO_2' has been marked as unsafe 
for use in headers}}
+#define UNSAFE_MACRO_2 2

Why do we not expect warnings for these cases? I would have expected that 
undefining a macro is just as unsafe for ABI reasons as defining a macro is.


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] D107723: [Clang] Moved warning of warnReturnTypestateForUnconsumableType back to SemaDeclAttr

[Alf via Phabricator via cfe-commits]

gAlfonso-bit updated this revision to Diff 365233.

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

https://reviews.llvm.org/D107723

Files:
  clang/include/clang/Analysis/Analyses/Consumed.h
  clang/lib/Analysis/Consumed.cpp
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/SemaCXX/warn-consumed-analysis.cpp
  clang/test/SemaCXX/warn-consumed-parsing.cpp

Index: clang/test/SemaCXX/warn-consumed-parsing.cpp
===
--- clang/test/SemaCXX/warn-consumed-parsing.cpp
+++ clang/test/SemaCXX/warn-consumed-parsing.cpp
@@ -6,16 +6,6 @@
 #define RETURN_TYPESTATE(state) __attribute__ ((return_typestate(state)))
 #define TEST_TYPESTATE(state)   __attribute__ ((test_typestate(state)))
 
-// FIXME: This test is here because the warning is issued by the Consumed
-//analysis, not SemaDeclAttr.  The analysis won't run after an error
-//has been issued.  Once the attribute propagation for template
-//instantiation has been fixed, this can be moved somewhere else and the
-//definition can be removed.
-int returnTypestateForUnconsumable() RETURN_TYPESTATE(consumed); // expected-warning {{return state set for an unconsumable type 'int'}}
-int returnTypestateForUnconsumable() {
-  return 0;
-}
-
 class AttrTester0 {
   void consumes()   __attribute__ ((set_typestate())); // expected-error {{'set_typestate' attribute takes one argument}}
   bool testUnconsumed() __attribute__ ((test_typestate())); // expected-error {{'test_typestate' attribute takes one argument}}
Index: clang/test/SemaCXX/warn-consumed-analysis.cpp
===
--- clang/test/SemaCXX/warn-consumed-analysis.cpp
+++ clang/test/SemaCXX/warn-consumed-analysis.cpp
@@ -74,6 +74,11 @@
 void baf5(ConsumableClass  *var);
 void baf6(ConsumableClass &&var);
 
+int returnTypestateForUnconsumable() RETURN_TYPESTATE(consumed); // expected-warning {{return state set for an unconsumable type 'int'}}
+int returnTypestateForUnconsumable() {
+  return 0;
+}
+
 ConsumableClass returnsUnconsumed() {
   return ConsumableClass(); // expected-warning {{return value not in expected state; expected 'unconsumed', observed 'consumed'}}
 }
@@ -275,9 +280,7 @@
 *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
 *var2; // expected-warning {{invalid invocation of method 'operator*' on object 'var2' while it is in the 'consumed' state}}
   }
-  
-#if 0
-  // FIXME: Get this test to pass.
+
   if (var0 || var1 || var2) {
 *var0;
 *var1;
@@ -288,7 +291,6 @@
 *var1; // expected-warning {{invalid invocation of method 'operator*' on object 'var1' while it is in the 'consumed' state}}
 *var2; // expected-warning {{invalid invocation of method 'operator*' on object 'var2' while it is in the 'consumed' state}}
   }
-#endif
 }
 
 void testComplexConditionals1() {
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -1190,17 +1190,14 @@
 return;
   }
 
-  // FIXME: This check is currently being done in the analysis.  It can be
-  //enabled here only after the parser propagates attributes at
-  //template specialization definition, not declaration.
-  //QualType ReturnType = cast(D)->getType();
-  //const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl();
-  //
-  //if (!RD || !RD->hasAttr()) {
-  //S.Diag(AL.getLoc(), diag::warn_return_state_for_unconsumable_type) <<
-  //  ReturnType.getAsString();
-  //return;
-  //}
+  QualType ReturnType = cast(D)->getType();
+  const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl();
+
+  if (!RD || !RD->hasAttr()) {
+S.Diag(AL.getLoc(), diag::warn_return_typestate_for_unconsumable_type)
+<< ReturnType.getAsString();
+return;
+  }
 
   D->addAttr(::new (S.Context) ParamTypestateAttr(S.Context, AL, ParamState));
 }
@@ -1222,30 +1219,27 @@
 return;
   }
 
-  // FIXME: This check is currently being done in the analysis.  It can be
-  //enabled here only after the parser propagates attributes at
-  //template specialization definition, not declaration.
-  //QualType ReturnType;
-  //
-  //if (const ParmVarDecl *Param = dyn_cast(D)) {
-  //  ReturnType = Param->getType();
-  //
-  //} else if (const CXXConstructorDecl *Constructor =
-  // dyn_cast(D)) {
-  //  ReturnType = Constructor->getThisType()->getPointeeType();
-  //
-  //} else {
-  //
-  //  ReturnType = cast(D)->getCallResultType();
-  //}
-  //
-  //const CXXRecordDecl *RD = ReturnType->getAsCXXRecordDecl();
-  //
-  //if (!RD || !RD->hasAttr()) {
-  //S.Diag(Attr.getLoc(), diag::warn_return_state_for_unconsumable_type) <<
-  //  ReturnType.getAsString();
-  //return;
-  //}
+  Qual

[PATCH] D107633: Set supported target for asan-use-callbacks test

[Vitaly Buka via Phabricator via cfe-commits]

vitalybuka added a comment.

Why other tests, like CodeGen/asan-strings.c do not need REQUIRES: ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107633

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

[PATCH] D107633: Set supported target for asan-use-callbacks test

[Vitaly Buka via Phabricator via cfe-commits]

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

All but line 1 is LGTM
Line 1 is probably not needed, but I don't know why.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107633

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

[PATCH] D107775: [Clang][AST] Resolve FIXME: Remove ObjCObjectPointer from isSpecifierType

[Alf via Phabricator via cfe-commits]

gAlfonso-bit created this revision.
gAlfonso-bit added reviewers: clang, LLVM.
gAlfonso-bit requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

ObjCObjectPointer is NOT a type specifier, so remove it from the list


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107775

Files:
  clang/lib/AST/Type.cpp


Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2785,7 +2785,6 @@
   case DependentTemplateSpecialization:
   case ObjCInterface:
   case ObjCObject:
-  case ObjCObjectPointer: // FIXME: object pointers aren't really specifiers
 return true;
   default:
 return false;


Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -2785,7 +2785,6 @@
   case DependentTemplateSpecialization:
   case ObjCInterface:
   case ObjCObject:
-  case ObjCObjectPointer: // FIXME: object pointers aren't really specifiers
 return true;
   default:
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D69764#2935036 , @klimek wrote:

> Happy to go the RFC route if @MyDeveloperDay wants to do that.

I'm happy to do that (in fact I've written a draft), do people want to code 
review the RFC draft (as I could easily be presenting a biased viewpoint and 
don't want to) or are you happy to just comment on the RFC when it appears

I took the viewpoint of trying to discuss the concept of clang-format making 
code altering changes rather than focusing on east/west conversion.


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] D107719: [Clang][AST][NFC] Resolve FIXME: Remove unused QualType ElementType member from the ASTContext class.

[Alf via Phabricator via cfe-commits]

gAlfonso-bit updated this revision to Diff 365249.
gAlfonso-bit added a comment.

Remove other changes


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

https://reviews.llvm.org/D107719

Files:
  clang/include/clang/AST/Type.h


Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -3450,10 +3450,6 @@
 protected:
   friend class ASTContext;
 
-  /// The element type of the matrix.
-  // FIXME: Appears to be unused? There is also MatrixType::ElementType...
-  QualType ElementType;
-
   /// Number of rows and columns.
   unsigned NumRows;
   unsigned NumColumns;


Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -3450,10 +3450,6 @@
 protected:
   friend class ASTContext;
 
-  /// The element type of the matrix.
-  // FIXME: Appears to be unused? There is also MatrixType::ElementType...
-  QualType ElementType;
-
   /// Number of rows and columns.
   unsigned NumRows;
   unsigned NumColumns;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D69764#2934666 , @erichkeane wrote:

>> Someone internally pointed out the anti-inclusivity of the terminology, so I 
>> figured I'd bring it up.

I apologise if I'm proliferating that, but could you educate me why its 
"anti-inclusive" I certainly didn't mean any offence in its use.

I'm absolutely OK about dropping east/west especially if we think its going to 
cause offence and we just want to avoid that.


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] D105904: [clangd] Support `#pragma mark` in the outline

[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/ParsedAST.cpp:441
+  if (Preamble)
+Marks = Preamble->Marks;
+  Clang->getPreprocessor().addPPCallbacks(

dgoldman wrote:
> kadircet wrote:
> > We are not patching marks for stale preambles, which I believe is fine but 
> > worth a FIXME. If you are interested please take a look at 
> > https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/Preamble.cpp#L427,
> >  it should be fairly easy to handle movements of `#pragma mark`s in 
> > preamble section.
> Added a FIXME. Does something similar need to be done for MainFileMacros?
we already handle macros today in 
https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/Preamble.cpp#L208.
 we just need to handle pragma mark in the callbacks and generate the relevant 
patch.



Comment at: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp:1100
+Children(AllOf(WithName("bar"),
+   Children(AllOf(WithName(
+   
"NotTopDecl",

nit: drop `AllOf`



Comment at: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp:1102
+   
"NotTopDecl",
+   AllOf(WithName("bar"));
+}

nit: drop `AllOf`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105904

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