[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

@skan Does this look good to you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72463



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

[PATCH] D72462: [clangd] Fix markdown rendering in VSCode

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72462



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

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added inline comments.



Comment at: include/clang/Tooling/Syntax/Tokens.h:206
+  /// DECL(a);
+  /// spelledTokens() returns {"#", "define", "DECL", "(", "name", ")", "eof"}.
+  /// FIXME: we do not yet store tokens of directives, like #include, #define,

ilya-biryukov wrote:
> nridge wrote:
> > Is this comment correct?
> > 
> > If so:
> >   * Why are the tokens "int", "name", "=", "10" not included?
> >   * Why are the tokens `"DECL", "(", "a", ")" not included?
> > 
> It isn't. Thanks for catching that.
> This patch went through so many revisions, it was close to impossible to 
> track this down.
Also:
- we do not store 'eof' in the spelled tokens anymore
- the FIXME is stale, we do store tokens of macro directives now

759c90456d418ffe69e1a2b4bcea2792491a6b5a updates the comment.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59887



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

[clang] 759c904 - [Syntax] Update comment, remove stale FIXME. NFC

[Ilya Biryukov via cfe-commits]

Author: Ilya Biryukov
Date: 2020-01-10T08:15:18+01:00
New Revision: 759c90456d418ffe69e1a2b4bcea2792491a6b5a

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

LOG: [Syntax] Update comment, remove stale FIXME. NFC

Added: 


Modified: 
clang/include/clang/Tooling/Syntax/Tokens.h

Removed: 




diff  --git a/clang/include/clang/Tooling/Syntax/Tokens.h 
b/clang/include/clang/Tooling/Syntax/Tokens.h
index 1f9eeae203f1..a210815d49f9 100644
--- a/clang/include/clang/Tooling/Syntax/Tokens.h
+++ b/clang/include/clang/Tooling/Syntax/Tokens.h
@@ -240,9 +240,9 @@ class TokenBuffer {
   /// Lexed tokens of a file before preprocessing. E.g. for the following input
   /// #define DECL(name) int name = 10
   /// DECL(a);
-  /// spelledTokens() returns {"#", "define", "DECL", "(", "name", ")", "eof"}.
-  /// FIXME: we do not yet store tokens of directives, like #include, #define,
-  ///#pragma, etc.
+  /// spelledTokens() returns
+  ///{"#", "define", "DECL", "(", "name", ")", "int", "name", "=", "10",
+  /// "DECL", "(", "a", ")", ";"}
   llvm::ArrayRef spelledTokens(FileID FID) const;
 
   /// Get all tokens that expand a macro in \p FID. For the following input



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

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added inline comments.



Comment at: include/clang/Tooling/Syntax/Tokens.h:206
+  /// DECL(a);
+  /// spelledTokens() returns {"#", "define", "DECL", "(", "name", ")", "eof"}.
+  /// FIXME: we do not yet store tokens of directives, like #include, #define,

nridge wrote:
> Is this comment correct?
> 
> If so:
>   * Why are the tokens "int", "name", "=", "10" not included?
>   * Why are the tokens `"DECL", "(", "a", ")" not included?
> 
It isn't. Thanks for catching that.
This patch went through so many revisions, it was close to impossible to track 
this down.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59887



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

[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!




Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:898
class $10^Foo {
- $11^Foo(int);
+ $11^Foo(int) {}
  $12^Foo(): $13^Foo(111) {}

NIT: is this change redundant now? This was probably a warning, not an error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72355



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

[PATCH] D72227: Add options for clang to align branches within 32B boundary

[Kan Shengchen via Phabricator via cfe-commits]

skan marked an inline comment as done and an inline comment as not done.
skan added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2045
+
+  if (Args.hasFlag(options::OPT_mbranches_within_32B_boundaries,
+   options::OPT_mno_branches_within_32B_boundaries, false)) {

MaskRay wrote:
> LuoYuanke wrote:
> > MaskRay wrote:
> > > skan wrote:
> > > > MaskRay wrote:
> > > > > `OPT_mbranches_within_32B_boundaries` should provide default values 
> > > > > which can be overridden by more specific options.
> > > > Currently, `-mbranches-within-32B-boundaries` is equivalent to 
> > > > `-malign-branch-boundary=32 -malign-branch=fused+jcc+jmp 
> > > > -malign-branch-prefix-size=4
> > > > 
> > > > What is expected behaviour would be very confusing if specific options 
> > > > could override `-mbranches-within-32B-boundaries`. For example, if 
> > > > passed options are
> > > > 
> > > > ```
> > > > -mbranches-within-32B-boundaries -malign-branch-boundary=32 
> > > > -mno-branches-within-32B-boundaries
> > > > ```
> > > > What should the value of `-malign-branch-boundary` be?  Is it 32 or 0?
> > > > 
> > > > If we think `-mno-branches-within-32B-boundaries` is the negative form 
> > > > of `-mbranches-within-32B-boundaries` , then `-malign-branch-boundary` 
> > > > should be 32.
> > > > 
> > > > Or if we think `-mno-branches-within-32B-boundaries` wins since it 
> > > > appears at the end, and `-mno-branches-within-32B-boundaries` means no 
> > > > need to align branches, `-malign-branch-boundary` should be 0.
> > > > 
> > > > As long as we don't support specific options could override 
> > > > `-mbranches-within-32B-boundaries`, the trouble disappears  :-)
> > > > 
> > > > 
> > > > -mbranches-within-32B-boundaries -malign-branch-boundary=32 
> > > > -mno-branches-within-32B-boundaries
> > > 
> > > My preference is that the net effect will be: `-malign-branch-boundary=32`
> > > 
> > > ```
> > > If (Args.hasFlag(options::OPT_mbranches_within_32B_boundaries, 
> > > options::OPT_mno_branches_within_32B_boundaries, false))
> > >   boundary = 32;
> > > if (const Arg *A = Args.getLastArg(options::OPT_malign_branch_EQ))
> > >   boundary = ...
> > > if (boundary)
> > >   add -mllvm boundary
> > > ```
> > > 
> > > but I'd like to hear what others say. @jyknight @reames 
> > I have no preference. What's the general rule for such case in LLVM? Is 
> > there any similar option design before? 
> I think options should follow these principles:
> 
> 1. Different options are position independent. `-mA -mB` should be the same 
> as `-mB -mA`.
> 2. `-mA` and `-mno-A` are position dependent and the last one wins. 
> Sometimes, the set may include more than 2 options, e.g. the last of 
> `-fno-pic` `-fpie` and `-fpic` wins.
> 3. More specific options can override semantics of less specific options. In 
> our case, `-malign-branch*` are more specific than 
> `-malign-branch-within-32B-boundaries`.
> 
> I have implemented these ideas in https://reviews.llvm.org/D72463. I don't 
> include documentation. Maybe documentation can be added in a different change 
> (for example, this one, if D72463 looks good to you).
> More specific options can override semantics of less specific options.

I hold a slightly different opinion about this. I think only when specific 
option appears at the right of the general option, override can happen.




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

https://reviews.llvm.org/D72227



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

[clang-tools-extra] b38d0d5 - clang-tidy doc - remove the widths

[Sylvestre Ledru via cfe-commits]

Author: Sylvestre Ledru
Date: 2020-01-10T08:11:05+01:00
New Revision: b38d0d5bdb353f8496a0fc38b9bbee419b41a321

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

LOG: clang-tidy doc - remove the widths

Added: 


Modified: 
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 




diff  --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst 
b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 1841588b7af3..e5cac0312081 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -5,7 +5,6 @@ Clang-Tidy Checks
 
 .. csv-table::
:header: "Name", "Offers fixes"
-   :widths: 50, 20
 
`abseil-duration-addition `_, "Yes"
`abseil-duration-comparison `_, "Yes"
@@ -287,7 +286,6 @@ Clang-Tidy Checks
 
 .. csv-table:: Aliases..
:header: "Name", "Redirect", "Offers fixes"
-   :widths: 50, 50, 10
 
`cert-dcl03-c `_, `misc-static-assert 
`_, "Yes"
`cert-dcl16-c `_, `readability-uppercase-literal-suffix 
`_, "Yes"



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

[PATCH] D72227: Add options for clang to align branches within 32B boundary

[Kan Shengchen via Phabricator via cfe-commits]

skan updated this revision to Diff 237245.

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

https://reviews.llvm.org/D72227

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/x86-malign-branch.c

Index: clang/test/Driver/x86-malign-branch.c
===
--- /dev/null
+++ clang/test/Driver/x86-malign-branch.c
@@ -0,0 +1,43 @@
+// RUN: %clang -target x86_64-unknown-unknown -malign-branch-boundary=32 -### -c %s 2>&1 | FileCheck %s --check-prefix=CHECK-BOUNDARY
+// CHECK-BOUNDARY: "-mllvm" "-x86-align-branch-boundary=32"
+//
+// RUN: %clang -target x86_64-unknown-unknown -malign-branch=jcc -### -c %s 2>&1 | FileCheck %s --check-prefix=CHECK-JCC
+// CHECK-JCC: "-mllvm" "-x86-align-branch=jcc"
+//
+// RUN: %clang -target x86_64-unknown-unknown -malign-branch=fused -### -c %s 2>&1 | FileCheck %s --check-prefix=CHECK-FUSED
+// CHECK-FUSED: "-mllvm" "-x86-align-branch=fused"
+//
+// RUN: %clang -target x86_64-unknown-unknown -malign-branch=jmp -### -c %s 2>&1 | FileCheck %s --check-prefix=CHECK-JMP
+// CHECK-JMP: "-mllvm" "-x86-align-branch=jmp"
+//
+// RUN: %clang -target x86_64-unknown-unknown -malign-branch=call -### -c %s 2>&1 | FileCheck %s --check-prefix=CHECK-CALL
+// CHECK-CALL: "-mllvm" "-x86-align-branch=call"
+//
+// RUN: %clang -target x86_64-unknown-unknown -malign-branch=ret -### -c %s 2>&1 | FileCheck %s --check-prefix=CHECK-RET
+// CHECK-RET: "-mllvm" "-x86-align-branch=ret"
+//
+// RUN: %clang -target x86_64-unknown-unknown -malign-branch=indirect -### -c %s 2>&1 | FileCheck %s --check-prefix=CHECK-INDIRECT
+// CHECK-INDIRECT: "-mllvm" "-x86-align-branch=indirect"
+//
+// RUN: %clang -target x86_64-unknown-unknown -malign-branch=fused+jcc+jmp+ret+call+indirect -### -c %s 2>&1 | FileCheck %s --check-prefix=CHECK-BRANCH
+// CHECK-BRANCH: "-mllvm" "-x86-align-branch=fused+jcc+jmp+ret+call+indirect"
+//
+// RUN: %clang -target x86_64-unknown-unknown -malign-branch-prefix-size=4 -### -c %s 2>&1 | FileCheck %s --check-prefix=CHECK-PREFIX
+// CHECK-PREFIX: "-mllvm" "-x86-align-branch-prefix-size=4"
+//
+// RUN: %clang -target x86_64-unknown-unknown -mno-branches-within-32B-boundaries -mbranches-within-32B-boundaries -### -c %s 2>&1 | FileCheck %s --check-prefix=CHECK-TOTAL1
+// CHECK-TOTAL1: "-mllvm" "-x86-align-branch-boundary=32" "-mllvm" "-x86-align-branch=fused+jcc+jmp" "-mllvm" "-x86-align-branch-prefix-size=5"
+//
+// RUN: %clang -target x86_64-unknown-unknown -mbranches-within-32B-boundaries -mno-branches-within-32B-boundaries -### -c %s 2>&1 | FileCheck %s --check-prefix=CHECK-TOTAL2
+// CHECK-TOTAL2-NOT: "-mllvm" "-x86-align-branch-boundary=32" "-mllvm" "-x86-align-branch=fused+jcc+jmp" "-mllvm" "-x86-align-branch-prefix-size=5"
+//
+// RUN: %clang -target x86_64-unknown-unknown -malign-branch-boundary=7 -### -c %s 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR
+// RUN: %clang -target x86_64-unknown-unknown -malign-branch=jump -### -c %s 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR
+// RUN: %clang -target x86_64-unknown-unknown -malign-branch-prefix-size=15 -### -c %s 2>&1 | FileCheck %s --check-prefix=CHECK-ERROR
+// CHECK-ERROR: error: unsupported argument
+//
+// RUN: %clang -target aarch64-unknown-unknown -malign-branch-boundary=7 -### -c %s 2>&1 | FileCheck %s --check-prefix=CHECK-WARNING
+// RUN: %clang -target aarch64-unknown-unknown -malign-branch=jump -### -c %s 2>&1 | FileCheck %s --check-prefix=CHECK-WARNING
+// RUN: %clang -target aarch64-unknown-unknown -malign-branch-prefix-size=15 -### -c %s 2>&1 | FileCheck %s --check-prefix=CHECK-WARNING
+// RUN: %clang -target aarch64-unknown-unknown -mbranches-within-32B-boundaries -### -c %s 2>&1 | FileCheck %s --check-prefix=CHECK-WARNING
+// CHECK-WARNING: warning: argument unused
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -2012,8 +2012,64 @@
 CmdArgs.push_back("-mpacked-stack");
 }
 
+static void alignBranchesOptions(const Driver , const ArgList ,
+ ArgStringList ) {
+  if (const Arg *A = Args.getLastArg(options::OPT_malign_branch_boundary_EQ)) {
+StringRef Value = A->getValue();
+uint64_t Num;
+if (!Value.getAsInteger(10, Num) && Num >= 32 && llvm::isPowerOf2_64(Num)) {
+  CmdArgs.push_back("-mllvm");
+  CmdArgs.push_back(
+  Args.MakeArgString("-x86-align-branch-boundary=" + Value));
+} else {
+  D.Diag(diag::err_drv_unsupported_option_argument)
+  << A->getOption().getName() << Value;
+}
+  }
+
+  if (const Arg *A = Args.getLastArg(options::OPT_malign_branch_EQ)) {
+StringRef Value = A->getValue();
+SmallVector BranchTypes;
+Value.split(BranchTypes, '+', -1, false);
+for (auto BranchType : BranchTypes) {
+  if (BranchType != "fused" && BranchType != 

[PATCH] D72227: Add options for clang to align branches within 32B boundary

[Kan Shengchen via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Driver/Options.td:2200
+  "Control how the assembler should align branches with NOP or segment 
"
+  "override prefix. If the boundary's size is not 0, it should be a "
+  "power of 2 and no less than 32. Branches will be aligned to prevent 
"

MaskRay wrote:
> "It should be 0 or a power of 2 no less than 16."
> 
> Do you want to preclude 16?
> 
> Long descriptions should be placed in `DocBrief` values.
No, I don't want to preclude 16.


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

https://reviews.llvm.org/D72227



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

[PATCH] D72378: [clang-tidy] Add `bugprone-reserved-identifier`

[Logan Smith via Phabricator via cfe-commits]

logan-5 updated this revision to Diff 237244.
logan-5 marked 8 inline comments as done.
logan-5 added a comment.

Addressed nits. Added CERT aliases. Adjusted the check to work for both C and 
C++, including where they differ.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72378

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-reserved-identifier.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-dcl37-c.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-dcl51-cpp.rst
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-reserved-identifier/system/system-header.h
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/bugprone-reserved-identifier/user-header.h
  clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier-c.c
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier-invert.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-reserved-identifier.cpp
@@ -0,0 +1,206 @@
+// RUN: %check_clang_tidy %s bugprone-reserved-identifier %t -- -- \
+// RUN:   -I%S/Inputs/bugprone-reserved-identifier \
+// RUN:   -isystem %S/Inputs/bugprone-reserved-identifier/system
+
+// no warnings expected without -header-filter=
+#include "user-header.h"
+#include 
+
+#define _MACRO(m) int m = 0
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses reserved identifier '_MACRO', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}#define MACRO(m) int m = 0{{$}}
+
+namespace _Ns {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses reserved identifier '_Ns', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}namespace Ns {{{$}}
+
+class _Object {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Object', which causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}class Object {{{$}}
+  int _Member;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Member', which causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}  int Member;{{$}}
+};
+
+float _Global;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Global', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}float Global;{{$}}
+
+void _Function() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses reserved identifier '_Function', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}void Function() {}{{$}}
+
+using _Alias = int;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '_Alias', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}using Alias = int;{{$}}
+
+template 
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: declaration uses reserved identifier '_TemplateParam', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}template {{$}}
+struct S {};
+
+} // namespace _Ns
+
+//
+
+#define __macro(m) int m = 0
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: declaration uses reserved identifier '__macro', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}#define macro(m) int m = 0{{$}}
+
+namespace __ns {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: declaration uses reserved identifier '__ns', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}namespace ns {{{$}}
+class __object {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '__object', which causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}class _object {{{$}}
+  int __member;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '__member', which causes undefined behavior [bugprone-reserved-identifier]
+  // CHECK-FIXES: {{^}}  int _member;{{$}}
+};
+
+float __global;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: declaration uses reserved identifier '__global', which causes undefined behavior [bugprone-reserved-identifier]
+// CHECK-FIXES: {{^}}float _global;{{$}}
+
+void __function() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: declaration uses reserved identifier '__function', which causes undefined behavior 

[PATCH] D71124: [RISCV] support clang driver to select cpu

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

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



Comment at: clang/lib/Basic/Targets/RISCV.cpp:164
+
+static constexpr llvm::StringLiteral ValidRV32CPUNames[] = {{"generic-rv32"},
+{"rocket-rv32"}};

lenary wrote:
> khchen wrote:
> > lenary wrote:
> > > Is there not a tablegen'd implementation of these based on 
> > > https://github.com/llvm/llvm-project/blob/master/llvm/lib/Target/RISCV/RISCV.td#L96-L99
> > >  (which will include `rocket-rv32` and `rocket-rv64` when those two 
> > > schedules are landed)?
> > you are right, if generic-cpu uses rocket chip scheduler, it's okay to 
> > abandon this patch.
> > 
> >  
> No, that's not quite what I mean. When we add the rocket schedules, there 
> will be additional `ProcessorModel` entries for the rocket chips, in addition 
> to the current generic models. 
> 
> The point in these `ProcessorModel` entries is if a user passes 
> `-mcpu=generic-rv64`, then `[Feature64Bit, FeatureRVCHints]` will get turned 
> on, because they chose a specific cpu. This is different to validating that 
> the correct features are enabled in order to choose a cpu, which seems the 
> correct way around.
> 
> Then instead of checking against hard-coded lists, you use use 
> `MCSubtargetInfo::isCPUStringValid(StringRef)`, which uses the info from the 
> `ProcessorModel` tablegen entries. 
@lenary 
I see no backend uses the info (ex. RISCVSubTypeKV?) from tablegen entries, 
I saw different targets use different way to impl.  
`TargetInfo::isValidCPUName`.
for example:
x86 uses [[ 
https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Basic/X86Target.def#L9
 | clang/Basic/X86Target.def]] to record (hard-codeed) valid cpu enum and 
string, 
arm/aarch64 uses [[ 
https://github.com/llvm-mirror/llvm/blob/master/include/llvm/Support/ARMTargetParser.def#L184
 | llvm/Support/ARMTargetParser.def]],
and mips just hard code the valid cpu string in [[ 
https://github.com/llvm/llvm-project/blob/master/clang/lib/Basic/Targets/Mips.cpp#L47
 | clang/lib/Basic/Targets/Mips.cpp]].
They don't use backend to check valid cpu string, so maybe this patch is 
workable.
But I wonder if you are asking this patch should implement the checking for 
invalid combination like `-mcpu=generic-rv32` with `-mattr=+64bit` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71124



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

[PATCH] D72222: [Driver][CodeGen] Add -fpatchable-function-entry=N[,0]

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.

Some targets don't have a PC relative relocation of pointer size, e.g. 
`R_MIPS_PC64` (MIPS only has `R_MIPS_PC32`, which is a GNU extension, sigh 
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;f=include/elf/mips.h;h=092dcd755dcdcf664b25a7011fd15957f124c29f).
 If we really need a PC-relative variant, we can use a new syntax 
`-fpatchable-function-entry=2,0,pcrel` with the existing option name. Let me 
commit the patch series.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D7



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

[PATCH] D72386: [ThinLTO] pass UnrollLoops/VectorizeLoop/VectorizeSLP in CGOpts down to pass builder in ltobackend

[Wei Mi via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG21a4710c67a9: [ThinLTO] Pass CodeGenOpts like 
UnrollLoops/VectorizeLoop/VectorizeSLP down to… (authored by wmi).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D72386?vs=237212=237240#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72386

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/test/CodeGen/thinlto-slp-vectorize-pm.c
  lld/COFF/CMakeLists.txt
  lld/ELF/CMakeLists.txt
  lld/ELF/LTO.cpp
  lld/test/ELF/lto/slp-vectorize-pm.ll
  lld/wasm/CMakeLists.txt
  llvm/include/llvm/LTO/Config.h
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/test/Other/new-pm-defaults.ll
  llvm/test/Other/new-pm-thinlto-defaults.ll
  llvm/test/tools/gold/X86/slp-vectorize-pm.ll
  llvm/test/tools/llvm-lto2/X86/slp-vectorize-pm.ll
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/llvm-lto2/CMakeLists.txt
  llvm/tools/llvm-lto2/llvm-lto2.cpp

Index: llvm/tools/llvm-lto2/llvm-lto2.cpp
===
--- llvm/tools/llvm-lto2/llvm-lto2.cpp
+++ llvm/tools/llvm-lto2/llvm-lto2.cpp
@@ -270,6 +270,8 @@
   Conf.OverrideTriple = OverrideTriple;
   Conf.DefaultTriple = DefaultTriple;
   Conf.StatsFile = StatsFile;
+  Conf.PTO.LoopVectorization = Conf.OptLevel > 1;
+  Conf.PTO.SLPVectorization = Conf.OptLevel > 1;
 
   ThinBackend Backend;
   if (ThinLTODistributedIndexes)
Index: llvm/tools/llvm-lto2/CMakeLists.txt
===
--- llvm/tools/llvm-lto2/CMakeLists.txt
+++ llvm/tools/llvm-lto2/CMakeLists.txt
@@ -9,6 +9,7 @@
   LTO
   MC
   Object
+  Passes
   Support
   Target
   )
Index: llvm/tools/gold/gold-plugin.cpp
===
--- llvm/tools/gold/gold-plugin.cpp
+++ llvm/tools/gold/gold-plugin.cpp
@@ -860,6 +860,9 @@
   Conf.CGOptLevel = getCGOptLevel();
   Conf.DisableVerify = options::DisableVerify;
   Conf.OptLevel = options::OptLevel;
+  Conf.PTO.LoopVectorization = options::OptLevel > 1;
+  Conf.PTO.SLPVectorization = options::OptLevel > 1;
+
   if (options::Parallelism)
 Backend = createInProcessThinBackend(options::Parallelism);
   if (options::thinlto_index_only) {
Index: llvm/test/tools/llvm-lto2/X86/slp-vectorize-pm.ll
===
--- /dev/null
+++ llvm/test/tools/llvm-lto2/X86/slp-vectorize-pm.ll
@@ -0,0 +1,51 @@
+; RUN: opt -module-summary %s -o %t1.bc
+
+; Test SLP and Loop Vectorization are enabled by default at O2 and O3.
+; RUN: llvm-lto2 run %t1.bc -o %t2.o -O0 -r %t1.bc,foo,plx -debug-pass-manager \
+; RUN:  -use-new-pm -save-temps 2>&1 | FileCheck %s --check-prefix=CHECK-O0-SLP
+; RUN: llvm-dis %t2.o.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-O0-LPV
+
+; RUN: llvm-lto2 run %t1.bc -o %t3.o -O1 -r %t1.bc,foo,plx -debug-pass-manager \
+; RUN:  -use-new-pm -save-temps 2>&1 | FileCheck %s --check-prefix=CHECK-O1-SLP
+; RUN: llvm-dis %t3.o.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-O1-LPV
+
+; RUN: llvm-lto2 run %t1.bc -o %t4.o -O2 -r %t1.bc,foo,plx -debug-pass-manager \
+; RUN:  -use-new-pm -save-temps 2>&1 | FileCheck %s --check-prefix=CHECK-O2-SLP
+; RUN: llvm-dis %t4.o.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-O2-LPV
+
+; RUN: llvm-lto2 run %t1.bc -o %t5.o -O3 -r %t1.bc,foo,plx -debug-pass-manager \
+; RUN:  -use-new-pm -save-temps 2>&1 | FileCheck %s --check-prefix=CHECK-O3-SLP
+; RUN: llvm-dis %t5.o.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-O3-LPV
+
+; CHECK-O0-SLP-NOT: Running pass: SLPVectorizerPass
+; CHECK-O1-SLP-NOT: Running pass: SLPVectorizerPass
+; CHECK-O2-SLP: Running pass: SLPVectorizerPass
+; CHECK-O3-SLP: Running pass: SLPVectorizerPass
+; CHECK-O0-LPV-NOT: = !{!"llvm.loop.isvectorized", i32 1}
+; CHECK-O1-LPV-NOT: = !{!"llvm.loop.isvectorized", i32 1}
+; CHECK-O2-LPV: = !{!"llvm.loop.isvectorized", i32 1}
+; CHECK-O3-LPV: = !{!"llvm.loop.isvectorized", i32 1}
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @foo(i32* %a) {
+entry:
+  br label %for.body
+
+for.body:
+  %indvars.iv = phi i64 [ 0, %entry ], [ %indvars.iv.next, %for.body ]
+  %red.05 = phi i32 [ 0, %entry ], [ %add, %for.body ]
+  %arrayidx = getelementptr inbounds i32, i32* %a, i64 %indvars.iv
+  %0 = load i32, i32* %arrayidx, align 4
+  %add = add nsw i32 %0, %red.05
+  %indvars.iv.next = add nuw nsw i64 %indvars.iv, 1
+  %exitcond = icmp eq i64 %indvars.iv.next, 255
+  br i1 %exitcond, label %for.end, label %for.body, !llvm.loop !0
+
+for.end:
+  ret i32 %add
+}
+
+!0 = distinct !{!0, !1}
+!1 = !{!"llvm.loop.unroll.disable", i1 true}
Index: llvm/test/tools/gold/X86/slp-vectorize-pm.ll

[PATCH] D71365: expand printf when compiling HIP to AMDGPU

[Sameer Sahasrabuddhe via Phabricator via cfe-commits]

sameerds added inline comments.



Comment at: clang/test/CodeGenHIP/printf.cpp:18
+}
+
+// CHECK: [[BEGIN:%.*]]   = call i64 @__ockl_printf_begin(i64 0)

sameerds wrote:
> arsenm wrote:
> > This could use a lot more testcases. Can you add some half, float, and 
> > double as well as pointers (including different address spaces) and vectors?
> I am not sure what exactly should be tested. The validity of this expansion 
> depends on the signature of the builtin printf function. Since printf is 
> variadic, the "default argument promotions" in the C/C++ spec guarantee that 
> the arguments are 32/64 bit integers or doubles if they are not pointers. The 
> printf signature as well as the device library functions are defined using 
> only generic pointers, so the address space does not matter. Non-scalar 
> arguments are not supported, which is checked by another test using a struct. 
> I could add a vector there, but that doesn't seem to be adding any value.
Bump!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71365



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

[PATCH] D72405: Allow /D flags absent during PCH creation under msvc-compat

[Zachary Henkel via Phabricator via cfe-commits]

zahen added a comment.

My change keeps the diagnostic so consumers can opt into the same enforcement 
that exists today.  Furthermore, the existing fuzzy-pch.c tests show that new 
-D flags are allowed under a "clangier" PCH structure.  None of the existing 
tests error on:

  BAR bar = 17;

when `-DBar=int` is only part of the the test file's command line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72405



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

[PATCH] D72227: Add options for clang to align branches within 32B boundary

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2045
+
+  if (Args.hasFlag(options::OPT_mbranches_within_32B_boundaries,
+   options::OPT_mno_branches_within_32B_boundaries, false)) {

LuoYuanke wrote:
> MaskRay wrote:
> > skan wrote:
> > > MaskRay wrote:
> > > > `OPT_mbranches_within_32B_boundaries` should provide default values 
> > > > which can be overridden by more specific options.
> > > Currently, `-mbranches-within-32B-boundaries` is equivalent to 
> > > `-malign-branch-boundary=32 -malign-branch=fused+jcc+jmp 
> > > -malign-branch-prefix-size=4
> > > 
> > > What is expected behaviour would be very confusing if specific options 
> > > could override `-mbranches-within-32B-boundaries`. For example, if passed 
> > > options are
> > > 
> > > ```
> > > -mbranches-within-32B-boundaries -malign-branch-boundary=32 
> > > -mno-branches-within-32B-boundaries
> > > ```
> > > What should the value of `-malign-branch-boundary` be?  Is it 32 or 0?
> > > 
> > > If we think `-mno-branches-within-32B-boundaries` is the negative form of 
> > > `-mbranches-within-32B-boundaries` , then `-malign-branch-boundary` 
> > > should be 32.
> > > 
> > > Or if we think `-mno-branches-within-32B-boundaries` wins since it 
> > > appears at the end, and `-mno-branches-within-32B-boundaries` means no 
> > > need to align branches, `-malign-branch-boundary` should be 0.
> > > 
> > > As long as we don't support specific options could override 
> > > `-mbranches-within-32B-boundaries`, the trouble disappears  :-)
> > > 
> > > 
> > > -mbranches-within-32B-boundaries -malign-branch-boundary=32 
> > > -mno-branches-within-32B-boundaries
> > 
> > My preference is that the net effect will be: `-malign-branch-boundary=32`
> > 
> > ```
> > If (Args.hasFlag(options::OPT_mbranches_within_32B_boundaries, 
> > options::OPT_mno_branches_within_32B_boundaries, false))
> >   boundary = 32;
> > if (const Arg *A = Args.getLastArg(options::OPT_malign_branch_EQ))
> >   boundary = ...
> > if (boundary)
> >   add -mllvm boundary
> > ```
> > 
> > but I'd like to hear what others say. @jyknight @reames 
> I have no preference. What's the general rule for such case in LLVM? Is there 
> any similar option design before? 
I think options should follow these principles:

1. Different options are position independent. `-mA -mB` should be the same as 
`-mB -mA`.
2. `-mA` and `-mno-A` are position dependent and the last one wins. Sometimes, 
the set may include more than 2 options, e.g. the last of `-fno-pic` `-fpie` 
and `-fpic` wins.
3. More specific options can override semantics of less specific options. In 
our case, `-malign-branch*` are more specific than 
`-malign-branch-within-32B-boundaries`.

I have implemented these ideas in https://reviews.llvm.org/D72463. I don't 
include documentation. Maybe documentation can be added in a different change 
(for example, this one, if D72463 looks good to you).


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

https://reviews.llvm.org/D72227



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

[PATCH] D72459: [CMake] Support running libc++abi tests in CrossWinToARMLinux cache file

[Vlad Vereschaka via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72459



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

[PATCH] D44352: [Concepts] Type Constraints

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

rsmith added inline comments.



Comment at: clang/include/clang/AST/DeclTemplate.h:1297-1300
+  if (TC->hasExplicitTemplateArgs())
+for (const auto  : TC->getTemplateArgsAsWritten()->arguments())
+  if (ArgLoc.getArgument().containsUnexpandedParameterPack())
+return true;

It'd be more efficient to ask if the immediately-declared constraint is a 
`CXXFoldExpr`, but it's a bit ugly to depend on that here. What do you think?



Comment at: clang/lib/Parse/ParseTemplate.cpp:553
+ /*OnlyNamespace=*/false,
+ /*SuppressDiagnostic=*/true);
+  if (ScopeError)

I'm a bit confused by what's going on with this `SuppressDiagnostic` flag. This 
isn't a tentative parse; if we don't produce diagnostics for an invalid scope 
specifier here, I don't see anything that guarantees that any later code will 
produce any diagnostics either, which would be a problem. I would expect to be 
able to parse and produce diagnostics normally here, without this 
`SuppressDiagnostic` flag.



Comment at: clang/lib/Parse/ParseTemplate.cpp:635
+
+  bool ScopeError;
+  if (isStartOfTemplateTypeParameter(ScopeError)) {

If `isStartOfTemplateTypeParameter` fails due to an invalid scope specifier, 
can we just return `nullptr` immediately, rather than making a likely-ill-fated 
attempt to parse a template or non-type parameter? It seems unlikely that we 
can reasonably recover in that case.



Comment at: clang/lib/Parse/ParseTemplate.cpp:676-677
+///
+/// \returns true if a type-constraint was annotated, false if there is no
+/// type-constraint at the current location.
+bool Parser::TryAnnotateTypeConstraint(CXXScopeSpec ) {

This is a bit surprising: the other `bool TryAnnotate*` functions follow the 
normal convention of returning `true` if they've encountered and diagnosed a 
parse error (and the caller is expected to check `Tok` to see whether it was 
annotated). Please consider making this consistent with the other similar 
functions.



Comment at: clang/lib/Parse/ParseTemplate.cpp:715-716
+/*TypeConstraint=*/true);
+  assert(!Failed &&
+ "AnnotateTemplateIdToken should not fail with TypeConstraint=true");
+  return true;

I would expect it to fail if the template argument list is malformed, and in 
that case we should fail too.



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:2484
+
+  if (const TypeConstraint *TC = D->getTypeConstraint())
+if (D->isPackExpansion() && !D->isExpandedParameterPack()) {

Nit: please add braces to this outer `if`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44352



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

[PATCH] D72227: Add options for clang to align branches within 32B boundary

[LuoYuanke via Phabricator via cfe-commits]

LuoYuanke added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2045
+
+  if (Args.hasFlag(options::OPT_mbranches_within_32B_boundaries,
+   options::OPT_mno_branches_within_32B_boundaries, false)) {

MaskRay wrote:
> skan wrote:
> > MaskRay wrote:
> > > `OPT_mbranches_within_32B_boundaries` should provide default values which 
> > > can be overridden by more specific options.
> > Currently, `-mbranches-within-32B-boundaries` is equivalent to 
> > `-malign-branch-boundary=32 -malign-branch=fused+jcc+jmp 
> > -malign-branch-prefix-size=4
> > 
> > What is expected behaviour would be very confusing if specific options 
> > could override `-mbranches-within-32B-boundaries`. For example, if passed 
> > options are
> > 
> > ```
> > -mbranches-within-32B-boundaries -malign-branch-boundary=32 
> > -mno-branches-within-32B-boundaries
> > ```
> > What should the value of `-malign-branch-boundary` be?  Is it 32 or 0?
> > 
> > If we think `-mno-branches-within-32B-boundaries` is the negative form of 
> > `-mbranches-within-32B-boundaries` , then `-malign-branch-boundary` should 
> > be 32.
> > 
> > Or if we think `-mno-branches-within-32B-boundaries` wins since it appears 
> > at the end, and `-mno-branches-within-32B-boundaries` means no need to 
> > align branches, `-malign-branch-boundary` should be 0.
> > 
> > As long as we don't support specific options could override 
> > `-mbranches-within-32B-boundaries`, the trouble disappears  :-)
> > 
> > 
> > -mbranches-within-32B-boundaries -malign-branch-boundary=32 
> > -mno-branches-within-32B-boundaries
> 
> My preference is that the net effect will be: `-malign-branch-boundary=32`
> 
> ```
> If (Args.hasFlag(options::OPT_mbranches_within_32B_boundaries, 
> options::OPT_mno_branches_within_32B_boundaries, false))
>   boundary = 32;
> if (const Arg *A = Args.getLastArg(options::OPT_malign_branch_EQ))
>   boundary = ...
> if (boundary)
>   add -mllvm boundary
> ```
> 
> but I'd like to hear what others say. @jyknight @reames 
I have no preference. What's the general rule for such case in LLVM? Is there 
any similar option design before? 


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

https://reviews.llvm.org/D72227



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

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

[Richard Trieu via Phabricator via cfe-commits]

rtrieu added inline comments.



Comment at: clang/lib/Serialization/ASTReader.cpp:11007
 
+  // Issue any pending ODR-failure diagnostics.
+  for (auto  : RecordOdrMergeFailures) {

Is this just a copy of the other loop?  That's a lot of code duplication.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71734



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

[PATCH] D72488: [clang-tidy] Add check for CERT-OOP57-CPP

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

It may be reasonable to provide options to expand list of problematic 
functions. See bugprone-suspicious-string-compare as example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72488



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

[PATCH] D69878: Consoldiate internal denormal flushing controls

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added inline comments.



Comment at: llvm/docs/LangRef.rst:1837
+   are present, this overrides ``"denormal-fp-math"``. Not all targets
+   support separately setting the denormal mode per type.
+

arsenm wrote:
> andrew.w.kaylor wrote:
> > arsenm wrote:
> > > andrew.w.kaylor wrote:
> > > > Can you document which targets do support the option? What happens if I 
> > > > try to use the option on a target where it is not supported?
> > > I'm not sure where to document this, or if/how/where to diagnose it. I 
> > > don't think the high level LangRef description is the right place to 
> > > discuss specific target handling.
> > > 
> > > Currently it won't error or anything. Code checking the denorm mode will 
> > > see the f32 specific mode, even if the target in the end isn't really 
> > > going to respect this.
> > > 
> > > One problem is this potentially does require coordination with other 
> > > toolchain components. For AMDGPU, the compiler can directly tell the 
> > > driver what FP mode to set on each entry point, but for x86 it requires 
> > > linking in crtfastmath to set the default mode bits. If another target 
> > > had a similar runtime environment requirement, I don't think we can be 
> > > sure the attribute is correct or not.
> > There is precedent for describing target-specific behavior in LangRef. It 
> > just doesn't seem useful to say that not all targets support the attribute 
> > without saying which ones do. We should also say what is expected if a 
> > target doesn't support the attribute. It seems reasonable for the function 
> > attribute to be silently ignored.
> > 
> > > One problem is this potentially does require coordination with other 
> > > toolchain components. For AMDGPU, the compiler can directly tell the 
> > > driver what FP mode to set on each entry point, but for x86 it requires 
> > > linking in crtfastmath to set the default mode bits.
> > 
> > This is a point I'm interested in. I don't like the current crtfastmath.o 
> > handling. It feels almost accidental when FTZ works as expected. My 
> > understanding is we link crtfastmath.o if we find it but if not everything 
> > just goes about its business. The Intel compiler injects code into main() 
> > to explicitly set the FTZ/DAZ control modes. That obviously has problems 
> > too, but it's at least consistent and predictable. As I understand it, 
> > crtfastmath.o sets these modes from a static initializer, but I'm not sure 
> > anything is done to determine the order of that initializer relative to 
> > others.
> > 
> > How does the compiler identify entry points for AMDGPU? And does it emit 
> > code to set FTZ based on the function attribute here?
> The entry points are a specific calling convention. There's no real concept 
> of main. Each kernel has an associated blob of metadata the driver uses to 
> set up various config registers on dispatch.
> 
> I don't think specially recognizing main in the compiler is fundamentally 
> different than having it done in a static constructor. It's still a construct 
> not associated with any particular function or anything.
The problem with having it done in a static constructor is that you have no 
certainty of when it will be done relative to other static constructors. If 
it's in main you can at least say that it's after all the static constructors 
(assuming main is your entry point).


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

https://reviews.llvm.org/D69878



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

[PATCH] D72484: [clang-tidy] Fix check for Abseil internal namespace access

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Please mark fixed comments as done.


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

https://reviews.llvm.org/D72484



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

[PATCH] D71467: [FPEnv] Generate constrained FP comparisons from clang

[Craig Topper via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D71467



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

[PATCH] D72488: [clang-tidy] Add check for CERT-OOP57-CPP

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cert/NotTrivialTypesLibcMemoryCallsCheck.cpp:51
+MatchFinder *Finder) {
+  using namespace ast_matchers::internal;
+  auto IsStructPointer = [](Matcher Constraint = anything(),

Please add check for C++.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:70
 
+- The 'bugprone-suspicous-constructor-and-assignment' check was renamed to 
:doc:`cert-not-trivial-types-libc-memory-calls
+  `

Is this relevant?



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:114
+  ` check.
+  Checks for calls to memset, memcpy, memmove, strcpy, memcmp and strcmp on non
+  trivial types.

Please separate with empty line and synchronize with first statement in 
documentation.

Please highlight memset, memcpy, memmove, strcpy, memcmp and strcmp with double 
back-ticks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72488



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

[clang] f041e9a - CWG2352: Allow qualification conversions during reference binding.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-01-09T18:24:06-08:00
New Revision: f041e9ad706aee7987c5299427c33424fcabbd0d

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

LOG: CWG2352: Allow qualification conversions during reference binding.

The language wording change forgot to update overload resolution to rank
implicit conversion sequences based on qualification conversions in
reference bindings. The anticipated resolution for that oversight is
implemented here -- we order candidates based on qualification
conversion, not only on top-level cv-qualifiers, including ranking
reference bindings against non-reference bindings if they differ in
non-top-level qualification conversions.

For OpenCL/C++, this allows reference binding between pointers with
differing (nested) address spaces. This makes the behavior of reference
binding consistent with that of implicit pointer conversions, as is the
purpose of this change, but that pre-existing behavior for pointer
conversions is itself probably not correct. In any case, it's now
consistently the same behavior and implemented in only one place.

This reinstates commit de21704ba96fa80d3e9402f12c6505917a3885f4,
reverted in commit d8018233d1ea4234de68d5b4593abd773db79484, with
workarounds for some overload resolution ordering problems introduced by
CWG2352.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaInit.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/CXX/drs/dr23xx.cpp
clang/test/CXX/drs/dr4xx.cpp
clang/test/SemaCXX/ref-init-ambiguous.cpp
clang/test/SemaObjCXX/arc-overloading.mm
clang/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
clang/www/cxx_dr_status.html
clang/www/make_cxx_dr_status

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 53d4571fa8ad..b56bfa24ff06 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1933,7 +1933,8 @@ def err_lvalue_reference_bind_to_unrelated : Error<
   "cannot bind to a value of unrelated type}1,2">;
 def err_reference_bind_drops_quals : Error<
   "binding reference %
diff {of type $ to value of type $|to value}0,1 "
-  "%select{drops %3 qualifier%plural{1:|2:|4:|:s}4|changes address space}2">;
+  "%select{drops %3 qualifier%plural{1:|2:|4:|:s}4|changes address space|"
+  "not permitted due to incompatible qualifiers}2">;
 def err_reference_bind_failed : Error<
   "reference %
diff {to %select{type|incomplete type}1 $ could not bind to an "
   "%select{rvalue|lvalue}2 of type $|could not bind to %select{rvalue|lvalue}2 
of "

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 9a1fd70b8897..851b25922f5a 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -10745,10 +10745,11 @@ class Sema final {
 /// whether T1 is reference-compatible with T2.
 enum ReferenceConversions {
   Qualification = 0x1,
-  Function = 0x2,
-  DerivedToBase = 0x4,
-  ObjC = 0x8,
-  ObjCLifetime = 0x10,
+  NestedQualification = 0x2,
+  Function = 0x4,
+  DerivedToBase = 0x8,
+  ObjC = 0x10,
+  ObjCLifetime = 0x20,
 
   LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/ObjCLifetime)
 };

diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index cfb3a05e9c14..17f2028ec5c5 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -5864,6 +5864,8 @@ QualType Sema::CXXCheckConditionalOperands(ExprResult 
, ExprResult ,
   //   one of the operands is reference-compatible with the other, in order
   //   to support conditionals between functions 
diff ering in noexcept. This
   //   will similarly cover 
diff erence in array bounds after P0388R4.
+  // FIXME: If LTy and RTy have a composite pointer type, should we convert to
+  //   that instead?
   ExprValueKind LVK = LHS.get()->getValueKind();
   ExprValueKind RVK = RHS.get()->getValueKind();
   if (!Context.hasSameType(LTy, RTy) &&
@@ -5871,7 +5873,9 @@ QualType Sema::CXXCheckConditionalOperands(ExprResult 
, ExprResult ,
 // DerivedToBase was already handled by the class-specific case above.
 // FIXME: Should we allow ObjC conversions here?
 const ReferenceConversions AllowedConversions =
-ReferenceConversions::Qualification | ReferenceConversions::Function;
+ReferenceConversions::Qualification |
+ReferenceConversions::NestedQualification |
+ReferenceConversions::Function;
 
 ReferenceConversions RefConv;
 if (CompareReferenceRelationship(QuestionLoc, LTy, RTy, ) ==

[PATCH] D72488: [clang-tidy] Add check for CERT-OOP57-CPP

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added projects: clang, clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun, mgorny.

This is a very basic warning implementation of Prefer special member functions 
and overloaded operators to C Standard Library functions 


It absolutely needs some fine tuning though.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72488

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/clang-tidy/cert/CMakeLists.txt
  clang-tools-extra/clang-tidy/cert/NotTrivialTypesLibcMemoryCallsCheck.cpp
  clang-tools-extra/clang-tidy/cert/NotTrivialTypesLibcMemoryCallsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-oop57-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s cert-oop57-cpp %t
+
+namespace std {
+void memset(void *, unsigned char, decltype(sizeof(int)));
+void memcpy(void *, const void *, decltype(sizeof(int)));
+void memmove(void *, const void *, decltype(sizeof(int)));
+void strcpy(void *, const void *, decltype(sizeof(int)));
+int memcmp(const void *, const void *, decltype(sizeof(int)));
+int strcmp(const void *, const void *, decltype(sizeof(int)));
+} // namespace std
+
+struct Trivial {
+  int I;
+  int J;
+};
+
+struct NonTrivial {
+  int I;
+  int J;
+
+  NonTrivial() : I(0), J(0) {}
+  NonTrivial =(const NonTrivial ) {
+I = Other.I;
+J = Other.J;
+return *this;
+  }
+
+  bool operator==(const Trivial ) const {
+return I == Other.I && J == Other.J;
+  }
+  bool operator!=(const Trivial ) const {
+return !(*this == Other);
+  }
+};
+
+void foo(const Trivial ) {
+  Trivial Data;
+  std::memset(, 0, sizeof(Data));
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: Calling 'std::memset' on a non trivially default constructible class is undefined
+  std::memset(, 0, sizeof(Trivial));
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: Calling 'std::memset' on a non trivially default constructible class is undefined
+  std::memcpy(, , sizeof(Data));
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: Calling 'std::memcpy' on a non trivially copyable class is undefined
+  std::memmove(, , sizeof(Data));
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: Calling 'std::memmove' on a non trivially copyable class is undefined
+  std::strcpy(, , sizeof(Data));
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: Calling 'std::strcpy' on a non trivially copyable class is undefined
+  std::memcmp(, , sizeof(Data));
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: consider using comparison operators instead of calling 'std::memcmp'
+  std::strcmp(, , sizeof(Data));
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]:3: warning: consider using comparison operators instead of calling 'std::strcmp'
+}
+
+void bar(const NonTrivial ) {
+  NonTrivial Data;
+  std::memset(, 0, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Calling 'std::memset' on a non trivially default constructible class is undefined
+  // Check it detects sizeof(Type) as well as sizeof(Instantiation)
+  std::memset(, 0, sizeof(NonTrivial));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Calling 'std::memset' on a non trivially default constructible class is undefined
+  std::memcpy(, , sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Calling 'std::memcpy' on a non trivially copyable class is undefined
+  std::memmove(, , sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Calling 'std::memmove' on a non trivially copyable class is undefined
+  std::strcpy(, , sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Calling 'std::strcpy' on a non trivially copyable class is undefined
+  std::memcmp(, , sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: consider using comparison operators instead of calling 'std::memcmp'
+  std::strcmp(, , sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: consider using comparison operators instead of calling 'std::strcmp'
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -100,6 +100,7 @@
`cert-mem57-cpp `_,
`cert-msc50-cpp `_,
`cert-msc51-cpp `_,
+   `cert-oop57-cpp `_,
`cert-oop58-cpp `_,
`clang-analyzer-core.DynamicTypePropagation `_,
`clang-analyzer-core.uninitialized.CapturedBlockVariable `_,
Index: 

[PATCH] D44352: [Concepts] Type Constraints

[Saar Raz via Phabricator via cfe-commits]

saar.raz added a comment.

Addressed all but one comment in recent commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44352



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

[PATCH] D72484: [clang-tidy] Fix check for Abseil internal namespace access

[Gennadiy Rozental via Phabricator via cfe-commits]

rogeeff updated this revision to Diff 237225.

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

https://reviews.llvm.org/D72484

Files:
  clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h
  clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp
@@ -44,5 +44,18 @@
 void MacroUse() {
   USE_INTERNAL(Function); // no-warning
   USE_EXTERNAL(Function);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' 
namespaces; those implementation details are reserved to Abseil
+  // CHECK-MESSAGES: :[[@LINE-5]]:25: warning: do not reference any 'internal' 
namespaces; those implementation details are reserved to Abseil
 }
+
+class A : absl::container_internal::InternalStruct {};
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not reference any 'internal' 
namespaces; those implementation details are reserved to Abseil
+
+template 
+class B : absl::container_internal::InternalTemplate {};
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not reference any 'internal' 
namespaces; those implementation details are reserved to Abseil
+
+template  class C : absl::container_internal::InternalTemplate {
+public:
+  template  static C Make(U *p) { return C{}; }
+};
+// CHECK-MESSAGES: :[[@LINE-4]]:33: warning: do not reference any 'internal' 
namespaces; those implementation details are reserved to Abseil
Index: 
clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h
===
--- 
clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h
+++ 
clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h
@@ -15,6 +15,8 @@
 
 namespace container_internal {
 struct InternalStruct {};
+
+template  struct InternalTemplate {};
 } // namespace container_internal
 } // namespace absl
 
Index: clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp
===
--- clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp
+++ clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp
@@ -37,7 +37,13 @@
   const auto *InternalDependency =
   Result.Nodes.getNodeAs("InternalDep");
 
-  diag(InternalDependency->getBeginLoc(),
+  SourceLocation LocAtFault =
+  Result.SourceManager->getSpellingLoc(InternalDependency->getBeginLoc());
+
+  if (!LocAtFault.isValid())
+return;
+
+  diag(LocAtFault,
"do not reference any 'internal' namespaces; those implementation "
"details are reserved to Abseil");
 }


Index: clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp
@@ -44,5 +44,18 @@
 void MacroUse() {
   USE_INTERNAL(Function); // no-warning
   USE_EXTERNAL(Function);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+  // CHECK-MESSAGES: :[[@LINE-5]]:25: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
 }
+
+class A : absl::container_internal::InternalStruct {};
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+template 
+class B : absl::container_internal::InternalTemplate {};
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+template  class C : absl::container_internal::InternalTemplate {
+public:
+  template  static C Make(U *p) { return C{}; }
+};
+// CHECK-MESSAGES: :[[@LINE-4]]:33: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h
===
--- clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h
@@ -15,6 +15,8 @@
 
 namespace container_internal {
 struct InternalStruct {};
+
+template  struct InternalTemplate {};
 } // namespace container_internal
 } // namespace absl
 
Index: 

[PATCH] D69868: Allow "callbr" to return non-void values

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 237224.
void added a comment.

The plus constraints are now at the end of the input/output/label list.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69868

Files:
  llvm/docs/LangRef.rst
  llvm/include/llvm/CodeGen/MachineBasicBlock.h
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineVerifier.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/CodeGen/X86/callbr-asm-outputs.ll
  llvm/test/CodeGen/X86/callbr-asm.ll

Index: llvm/test/CodeGen/X86/callbr-asm.ll
===
--- llvm/test/CodeGen/X86/callbr-asm.ll
+++ llvm/test/CodeGen/X86/callbr-asm.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc < %s -mtriple=i686-- -O3 | FileCheck %s
+; RUN: llc < %s -mtriple=i686-- -O3 -verify-machineinstrs | FileCheck %s
 
 ; Tests for using callbr as an asm-goto wrapper
 
@@ -159,3 +159,54 @@
 cleanup:  ; preds = %asm.fallthrough, %quux
   ret void
 }
+
+define i32 @test5(i32 %out1, i32 %out2) {
+; Test 5 - asm-goto with output constraints.
+; CHECK-LABEL: test5:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:  movl $-1, %eax
+; CHECK-NEXT:  movl 4(%esp), %ecx
+; CHECK-NEXT:  #APP
+; CHECK-NEXT:  testl %ecx, %ecx
+; CHECK-NEXT:  testl %edx, %ecx
+; CHECK-NEXT:  jne .Ltmp6
+; CHECK-NEXT:  #NO_APP
+; CHECK-NEXT:  .LBB4_1:
+; CHECK-NEXT:  #APP
+; CHECK-NEXT:  testl %ecx, %edx
+; CHECK-NEXT:  testl %ecx, %edx
+; CHECK-NEXT:  jne .Ltmp7
+; CHECK-NEXT:  #NO_APP
+; CHECK-NEXT:  .LBB4_2:
+; CHECK-NEXT:  addl %edx, %ecx
+; CHECK-NEXT:  movl %ecx, %eax
+; CHECK-NEXT:  retl
+; CHECK-NEXT:  .Ltmp6:
+; CHECK-NEXT:  .LBB4_3:
+; CHECK-NEXT:  movl $-2, %eax
+; CHECK-NEXT:  .Ltmp7:
+; CHECK-NEXT:  .LBB4_4:
+; CHECK-NEXT:  retl
+entry:
+  %0 = callbr { i32, i32 } asm sideeffect "testl $0, $0; testl $1, $2; jne ${3:l}", "=r,=r,r,X,X,~{dirflag},~{fpsr},~{flags}"(i32 %out1, i8* blockaddress(@test5, %label_true), i8* blockaddress(@test5, %return))
+  to label %asm.fallthrough [label %label_true, label %return]
+
+asm.fallthrough:  ; preds = %entry
+  %asmresult = extractvalue { i32, i32 } %0, 0
+  %asmresult1 = extractvalue { i32, i32 } %0, 1
+  %1 = callbr { i32, i32 } asm sideeffect "testl $0, $1; testl $2, $3; jne ${5:l}", "=r,=r,r,r,X,X,~{dirflag},~{fpsr},~{flags}"(i32 %asmresult, i32 %asmresult1, i8* blockaddress(@test5, %label_true), i8* blockaddress(@test5, %return))
+  to label %asm.fallthrough2 [label %label_true, label %return]
+
+asm.fallthrough2: ; preds = %asm.fallthrough
+  %asmresult3 = extractvalue { i32, i32 } %1, 0
+  %asmresult4 = extractvalue { i32, i32 } %1, 1
+  %add = add nsw i32 %asmresult3, %asmresult4
+  br label %return
+
+label_true:   ; preds = %asm.fallthrough, %entry
+  br label %return
+
+return:   ; preds = %entry, %asm.fallthrough, %label_true, %asm.fallthrough2
+  %retval.0 = phi i32 [ %add, %asm.fallthrough2 ], [ -2, %label_true ], [ -1, %asm.fallthrough ], [ -1, %entry ]
+  ret i32 %retval.0
+}
Index: llvm/test/CodeGen/X86/callbr-asm-outputs.ll
===
--- llvm/test/CodeGen/X86/callbr-asm-outputs.ll
+++ llvm/test/CodeGen/X86/callbr-asm-outputs.ll
@@ -1,18 +1,118 @@
-; RUN: not llc -mtriple=i686-- < %s 2> %t
-; RUN: FileCheck %s < %t
+; RUN: llc -mtriple=i686-- -verify-machineinstrs < %s | FileCheck %s
 
-; CHECK: error: asm-goto outputs not supported
+; A test for asm-goto output
 
-; A test for asm-goto output prohibition
-
-define i32 @test(i32 %a) {
+; CHECK-LABEL: test1:
+; CHECK:   movl 4(%esp), %eax
+; CHECK-NEXT:  addl $4, %eax
+; CHECK-NEXT:  #APP
+; CHECK-NEXT:  xorl %eax, %eax
+; CHECK-NEXT:  jmp .Ltmp0
+; CHECK-NEXT:  #NO_APP
+; CHECK-NEXT:  .LBB0_1:
+; CHECK-NEXT:  retl
+; CHECK-NEXT:  .Ltmp0:
+define i32 @test1(i32 %x) {
 entry:
-  %0 = add i32 %a, 4
-  %1 = callbr i32 asm "xorl $1, $1; jmp ${1:l}", "=,r,X,~{dirflag},~{fpsr},~{flags}"(i32 %0, i8* blockaddress(@test, %fail)) to label %normal [label %fail]
+  %add = add nsw i32 %x, 4
+  %ret = callbr i32 asm "xorl $1, $0; jmp ${2:l}", "=r,r,X,~{dirflag},~{fpsr},~{flags}"(i32 %add, i8* blockaddress(@test1, %abnormal))
+  to label %normal [label %abnormal]
 
 normal:
-  ret i32 %1
+  ret i32 %ret
 
-fail:
+abnormal:
   ret i32 1
 }
+
+; CHECK-LABEL: test2:
+; CHECK:   # %bb.1: # %if.then
+; CHECK-NEXT:  #APP
+; CHECK-NEXT:  testl %esi, %esi
+; CHECK-NEXT:  testl %edi, %esi
+; CHECK-NEXT:  jne .Ltmp1
+; CHECK-NEXT:  #NO_APP
+; CHECK-NEXT:  .LBB1_2:
+; CHECK-NEXT:  

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D72427#1812954 , @akhuang wrote:

> > What's the plan for this? Is it still in an experimental stage, with the 
> > intent to investigate the types that are no longer emitted unedr the flag & 
> > explain why they're missing (& either have a justification for why that's 
> > acceptable, or work on additional heuristics to address the gaps?)?
> > 
> > If so, I'd probably rather this not be a full driver flag - if it's a 
> > reliable way to reduce debug info size (like the existing heuristics under 
> > -fstandalone-debug*) it should be rolled into -fno-standalone-debug 
> > behavior, and if it's not fully fleshed out yet, I think an -Xclang flag 
> > would be more suitable for the experimental phase.
>
> Pretty much, I think the plan is to investigate further and maybe have more 
> people try it. The -Xclang flag seems reasonable. Do you have thoughts on 
> whether the added DebugInfoKind level makes sense?


Yeah, that seems reasonable - though once this mode has been 
vetted/stabilized/ironed out, hopefully that can be removed and the 
functionality will be rolled into LimitedDebugInfo.




Comment at: clang/include/clang/Basic/CodeGenOptions.h:360-364
+
+  /// Check if full debug info should be emitted.
+  bool isFullDebug() const {
+return getDebugInfo() >= codegenoptions::DebugInfoConstructor;
+  }

Could you commit (the pre-patch equivalent) of this change now/before your 
patch - it'll simplify the diff/make it easier to review.

Though I think the name needs some massaging, since it doesn't return 
"getDebugInfo() == codegenoptions::FullDebugInfo", so the name's a bit 
misleading.

/maybe/ (but honestly I don't have any great names) 
"hasVariableAndTypeDebugInfo" though that's a bit of a mouthful.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2225
+  !CXXDecl->isLambda() && !isClassOrMethodDLLImport(CXXDecl)) {
+for (auto ctor : CXXDecl->ctors()) {
+  if (ctor->isUserProvided())

Is the type here a pointer - if so, then "auto *ctor" would be preferred. (if 
it's not, then probably want to avoid copying it and use "auto ")



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3692
+  Args.getLastArg(options::OPT_flimit_debug_info_constructor), Args, 
D, TC) &&
+DebugInfoKind == codegenoptions::LimitedDebugInfo)
+  DebugInfoKind = codegenoptions::DebugInfoConstructor;

I'm surprised that this flag would only apply if you were already using 
-flimit-debug-info but that looks like what this code does? (I'd probably 
expect these to override each other -flimit-debug-info 
-flimit-debug-info-constructor gets you constructor limited, the other way 
around gets the classic/pre-patch behavior? but once this becomes not a driver 
option anymore, that gets a bit murkier I guess)



Comment at: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp:23
+
+extern int bar(B *b);
+int TestB(B *b) {

"extern" here is redundant - probably best to remove it.



Comment at: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp:35-36
+C *TestC() {
+  C *c = new C();
+  return c;
+}

Skip the local variable if it isn't needed & "return new C();" perhaps?

Though perhaps all these uses of "new" could use the direct type instead (as 
that should produce simpler IR, maybe make the test cases more obvious)?

"void TestC() { C c; }"

(or "C Test() { return C(); }"  but that seems more complicated.

Or maybe even use a straight global variable "C c;" ? (but I guess then you 
might need "extern C c; C c;" in some cases to make sure they're emitted at all 
- I'm not sure)

Or does that not capture what they're intended to test?



Comment at: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp:54-58
+class E {
+public:
+*d = new D();
+  return d;
+}

Did something get mangled in the patch upload, perhaps? - this doesn't look 
like valid code.



Comment at: clang/test/CodeGenCXX/debug-info-limited-ctor.cpp:63-64
+// CHECK-SAME: ){{$}}
+class E {
+public:
+  E();

I'd probably write these all as structs, if the class/struct distinction isn't 
relevant - saves an extra line of "public:".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72427



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

[PATCH] D69876: Allow output constraints on "asm goto"

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 237223.
void added a comment.

Move plus constraints to after the label constraints.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Stmt.h
  clang/lib/AST/Stmt.cpp
  clang/lib/Analysis/UninitializedValues.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/Analysis/uninit-asm-goto.cpp
  clang/test/CodeGen/asm-goto.c
  clang/test/Parser/asm-goto.c
  clang/test/Parser/asm-goto.cpp
  clang/test/Sema/asm-goto.cpp

Index: clang/test/Sema/asm-goto.cpp
===
--- clang/test/Sema/asm-goto.cpp
+++ clang/test/Sema/asm-goto.cpp
@@ -1,38 +1,38 @@
 // RUN: %clang_cc1 %s -triple i386-pc-linux-gnu -verify -fsyntax-only
 // RUN: %clang_cc1 %s -triple x86_64-pc-linux-gnu -verify -fsyntax-only
 
-struct NonTrivial {
-  ~NonTrivial();
+struct S {
+  ~S();
   int f(int);
 private:
   int k;
 };
-void JumpDiagnostics(int n) {
+void test1(int n) {
 // expected-error@+1 {{cannot jump from this goto statement to its label}}
   goto DirectJump;
 // expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
-  NonTrivial tnp1;
+  S s1;
 
 DirectJump:
 // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
   asm goto("jmp %l0;" Later);
 // expected-note@+1 {{jump bypasses variable with a non-trivial destructor}}
-  NonTrivial tnp2;
+  S s2;
 // expected-note@+1 {{possible target of asm goto statement}}
 Later:
   return;
 }
 
-struct S { ~S(); };
-void foo(int a) {
+struct T { ~T(); };
+void test2(int a) {
   if (a) {
 FOO:
 // expected-note@+2 {{jump exits scope of variable with non-trivial destructor}}
 // expected-note@+1 {{jump exits scope of variable with non-trivial destructor}}
-S s;
+T t;
 void *p = &
 // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
-  asm goto("jmp %l0;" BAR);
+asm goto("jmp %l0;" BAR);
 // expected-error@+1 {{cannot jump from this indirect goto statement to one of its possible targets}}
 goto *p;
 p = &
@@ -45,9 +45,7 @@
   return;
 }
 
-
-//Asm goto:
-int test16(int n)
+int test3(int n)
 {
   // expected-error@+2 {{cannot jump from this asm goto statement to one of its possible targets}}
   // expected-error@+1 {{cannot jump from this asm goto statement to one of its possible targets}}
Index: clang/test/Parser/asm-goto.cpp
===
--- clang/test/Parser/asm-goto.cpp
+++ clang/test/Parser/asm-goto.cpp
@@ -1,14 +1,54 @@
 // RUN: %clang_cc1 -triple i386-pc-linux-gnu -fsyntax-only -verify -std=c++11 %s
 // RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -verify -std=c++11 %s
 
-int zoo ()
-{
+int a, b, c, d, e, f, g, h, i, j, k, l;
+
+void test1(void) {
+  __asm__ volatile goto (""
+:: [a] "r" (a), [b] "r" (b), [c] "r" (c), [d] "r" (d),
+   [e] "r" (e), [f] "r" (f), [g] "r" (g), [h] "r" (h),
+   [i] "r" (i), [j] "r" (j), [k] "r" (k), [l] "r" (l)
+::lab1,lab2);
+lab1: return;
+lab2: return;
+}
+
+void test2(void) {
+  __asm__ volatile goto (""
+:: [a] "r,m" (a), [b] "r,m" (b), [c] "r,m" (c), [d] "r,m" (d),
+   [e] "r,m" (e), [f] "r,m" (f), [g] "r,m" (g), [h] "r,m" (h),
+   [i] "r,m" (i), [j] "r,m" (j), [k] "r,m" (k), [l] "r,m" (l)
+:: lab);
+  lab: return;
+}
+
+int test3(int x) {
+  __asm__ volatile goto ("decl %0; jnz %l[a]"
+ : "=r" (x) : "m" (x) : "memory" : a);
+a:
+  return -x;
+}
+
+int test4(int x) {
+  int y;
+  if (x > 42)
+__asm__ volatile goto ("decl %0; jnz %l[a]"
+   : "=r" (x), "=r" (y) : "m" (x) : "memory" : a);
+  else
+__asm__ volatile goto ("decl %0; jnz %l[b]"
+   : "=r" (x), "=r" (y) : "m" (x) : "memory" : b);
+  x = y + 42;
+a:
+  return -x;
+b:
+  return +x;
+}
+
+int test5(void) {
   int x,cond,*e;
   // expected-error@+1 {{expected ')'}}
   asm ("mov %[e], %[e]" : : [e] "rm" (*e)::a)
-  // expected-error@+1  {{'asm goto' cannot have output constraints}}
-  asm goto ("decl %0; jnz %l[a]" :"=r"(x): "m"(x) : "memory" : a);
-  // expected-error@+1 {{expected identifie}}
+  // expected-error@+1 {{expected identifier}}
   asm goto ("decl %0;" :: "m"(x) : "memory" : );
   // expected-error@+1  {{expected ':'}}
   asm goto ("decl %0;" :: "m"(x) : "memory" );
@@ -26,28 +66,24 @@
   return 0;
 }
 
-
-int a, b, c, d, e, f, g, h, i, j, k, l;
-
-void
-fgoto1 (void)
-{
-  __asm__ volatile goto (""
-:: [a] "r" (a), [b] "r" (b), [c] "r" (c), [d] "r" (d),
-   [e] "r" (e), [f] "r" (f), [g] "r" (g), [h] "r" (h),
-   [i] "r" (i), [j] "r" (j), [k] "r" (k), [l] "r" (l)
- 

[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: include/clang/Tooling/Syntax/Tokens.h:206
+  /// DECL(a);
+  /// spelledTokens() returns {"#", "define", "DECL", "(", "name", ")", "eof"}.
+  /// FIXME: we do not yet store tokens of directives, like #include, #define,

Is this comment correct?

If so:
  * Why are the tokens "int", "name", "=", "10" not included?
  * Why are the tokens `"DECL", "(", "a", ")" not included?



Repository:
  rC Clang

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

https://reviews.llvm.org/D59887



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

[clang] 2519554 - When diagnosing the lack of a viable conversion function, also list

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-01-09T15:15:02-08:00
New Revision: 25195541349b1d6dfc03bf7511483110bda69b29

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

LOG: When diagnosing the lack of a viable conversion function, also list
explicit functions that are not candidates.

It's not always obvious that the reason a conversion was not possible is
because the function you wanted to call is 'explicit', so explicitly say
if that's the case.

It would be nice to rank the explicit candidates higher in the
diagnostic if an implicit conversion sequence exists for their
arguments, but unfortunately we can't determine that without potentially
triggering non-immediate-context errors that we're not permitted to
produce.

Added: 


Modified: 
clang/include/clang/AST/DeclCXX.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Overload.h
clang/lib/Sema/SemaInit.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/CXX/basic/basic.lookup/basic.lookup.qual/namespace.qual/p2.cpp
clang/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5.cpp
clang/test/CXX/dcl.decl/dcl.init/p14-0x.cpp
clang/test/CXX/drs/dr15xx.cpp
clang/test/CXX/drs/dr1xx.cpp
clang/test/CXX/expr/expr.prim/expr.prim.lambda/p11-1y.cpp
clang/test/CXX/over/over.match/over.match.funcs/over.match.copy/p1.cpp
clang/test/CXX/special/class.inhctor/p3.cpp
clang/test/PCH/cxx-explicit-specifier.cpp
clang/test/SemaCXX/conversion-function.cpp
clang/test/SemaCXX/convert-to-bool.cpp
clang/test/SemaCXX/converting-constructor.cpp
clang/test/SemaCXX/copy-initialization.cpp
clang/test/SemaCXX/cxx2a-explicit-bool.cpp
clang/test/SemaCXX/default1.cpp
clang/test/SemaCXX/explicit.cpp

Removed: 




diff  --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index aba33e383976..73b4fb94a494 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -1783,7 +1783,7 @@ class CXXRecordDecl : public RecordDecl {
 };
 
 /// Store information needed for an explicit specifier.
-/// used by CXXDeductionGuideDecl, CXXConstructorDecl and CXXConversionDecl.
+/// Used by CXXDeductionGuideDecl, CXXConstructorDecl and CXXConversionDecl.
 class ExplicitSpecifier {
   llvm::PointerIntPair ExplicitSpec{
   nullptr, ExplicitSpecKind::ResolvedFalse};
@@ -1796,20 +1796,22 @@ class ExplicitSpecifier {
   const Expr *getExpr() const { return ExplicitSpec.getPointer(); }
   Expr *getExpr() { return ExplicitSpec.getPointer(); }
 
-  /// Return true if the ExplicitSpecifier isn't defaulted.
+  /// Determine if the declaration had an explicit specifier of any kind.
   bool isSpecified() const {
 return ExplicitSpec.getInt() != ExplicitSpecKind::ResolvedFalse ||
ExplicitSpec.getPointer();
   }
 
-  /// Check for Equivalence of explicit specifiers.
-  /// Return True if the explicit specifier are equivalent false otherwise.
+  /// Check for equivalence of explicit specifiers.
+  /// \return true if the explicit specifier are equivalent, false otherwise.
   bool isEquivalent(const ExplicitSpecifier Other) const;
-  /// Return true if the explicit specifier is already resolved to be explicit.
+  /// Determine whether this specifier is known to correspond to an explicit
+  /// declaration. Returns false if the specifier is absent or has an
+  /// expression that is value-dependent or evaluates to false.
   bool isExplicit() const {
 return ExplicitSpec.getInt() == ExplicitSpecKind::ResolvedTrue;
   }
-  /// Return true if the ExplicitSpecifier isn't valid.
+  /// Determine if the explicit specifier is invalid.
   /// This state occurs after a substitution failures.
   bool isInvalid() const {
 return ExplicitSpec.getInt() == ExplicitSpecKind::Unresolved &&
@@ -1817,9 +1819,7 @@ class ExplicitSpecifier {
   }
   void setKind(ExplicitSpecKind Kind) { ExplicitSpec.setInt(Kind); }
   void setExpr(Expr *E) { ExplicitSpec.setPointer(E); }
-  // getFromDecl - retrieve the explicit specifier in the given declaration.
-  // if the given declaration has no explicit. the returned explicit specifier
-  // is defaulted. .isSpecified() will be false.
+  // Retrieve the explicit specifier in the given declaration, if any.
   static ExplicitSpecifier getFromDecl(FunctionDecl *Function);
   static const ExplicitSpecifier getFromDecl(const FunctionDecl *Function) {
 return getFromDecl(const_cast(Function));

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index e71bddc06b57..53d4571fa8ad 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3871,10 +3871,9 @@ def note_ovl_candidate : Note<
 "| 

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71082



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

[PATCH] D72484: [clang-tidy] Fix check for Abseil internal namespace access

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Should be change mentioned in Release Notes?




Comment at: 
clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp:40
 
-  diag(InternalDependency->getBeginLoc(),
+  auto loc_at_fault =
+  Result.SourceManager->getSpellingLoc(InternalDependency->getBeginLoc());

Please don't use auto when type is not spelled in statement or iterator. Does 
variable name comply to LLVM Coding Guidelines?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D72484



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

[PATCH] D72484: Fix clang-tidy check for Abseil internal namespace access

[Gennadiy Rozental via Phabricator via cfe-commits]

rogeeff created this revision.
Herald added a subscriber: mgehre.
Herald added a project: clang.
rogeeff added a reviewer: EricWF.

This change makes following modifications:

- If reference originated from macro expansion, we report location inside of 
the macro instead of location where macro is referenced
- If for any reason deduced location is not correct we silently ignore it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D72484

Files:
  clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h
  clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp
@@ -44,5 +44,18 @@
 void MacroUse() {
   USE_INTERNAL(Function); // no-warning
   USE_EXTERNAL(Function);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' 
namespaces; those implementation details are reserved to Abseil
+  // CHECK-MESSAGES: :[[@LINE-5]]:25: warning: do not reference any 'internal' 
namespaces; those implementation details are reserved to Abseil
 }
+
+class A : absl::container_internal::InternalStruct {};
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not reference any 'internal' 
namespaces; those implementation details are reserved to Abseil
+
+template 
+class B : absl::container_internal::InternalTemplate {};
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not reference any 'internal' 
namespaces; those implementation details are reserved to Abseil
+
+template  class C : absl::container_internal::InternalTemplate {
+public:
+  template  static C Make(U *p) { return C{}; }
+};
+// CHECK-MESSAGES: :[[@LINE-4]]:33: warning: do not reference any 'internal' 
namespaces; those implementation details are reserved to Abseil
Index: 
clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h
===
--- 
clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h
+++ 
clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h
@@ -15,6 +15,8 @@
 
 namespace container_internal {
 struct InternalStruct {};
+
+template  struct InternalTemplate {};
 } // namespace container_internal
 } // namespace absl
 
Index: clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp
===
--- clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp
+++ clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp
@@ -37,7 +37,13 @@
   const auto *InternalDependency =
   Result.Nodes.getNodeAs("InternalDep");
 
-  diag(InternalDependency->getBeginLoc(),
+  auto loc_at_fault =
+  Result.SourceManager->getSpellingLoc(InternalDependency->getBeginLoc());
+
+  if (!loc_at_fault.isValid())
+return;
+
+  diag(loc_at_fault,
"do not reference any 'internal' namespaces; those implementation "
"details are reserved to Abseil");
 }


Index: clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/abseil-no-internal-dependencies.cpp
@@ -44,5 +44,18 @@
 void MacroUse() {
   USE_INTERNAL(Function); // no-warning
   USE_EXTERNAL(Function);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+  // CHECK-MESSAGES: :[[@LINE-5]]:25: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
 }
+
+class A : absl::container_internal::InternalStruct {};
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+template 
+class B : absl::container_internal::InternalTemplate {};
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+template  class C : absl::container_internal::InternalTemplate {
+public:
+  template  static C Make(U *p) { return C{}; }
+};
+// CHECK-MESSAGES: :[[@LINE-4]]:33: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h
===
--- clang-tools-extra/test/clang-tidy/checkers/Inputs/absl/strings/internal-file.h
+++ 

[PATCH] D72414: Add new AST matchers `hasAnyCapture` and `capturesThis`

[Reid via Phabricator via cfe-commits]

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

Thanks for the review!  I regenerated the docs and switched the `this` capture 
case to be an override of `hasAnyCapture`.




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4056
+///   matches [this](){};
+AST_MATCHER(LambdaExpr, capturesThis) {
+  for (const LambdaCapture  : Node.captures()) {

aaron.ballman wrote:
> I'm a bit less certain about the utility of this matcher. I would have 
> assumed you could do `hasAnyCapture(cxxThisExpr())` to get the same behavior, 
> but I am guessing that won't work -- though you may be able to add an 
> overload to `hasAnyCapture()` to make that work. I think that might be a 
> better approach, if it works.
Thanks for that idea, it does seem a bit cleaner.

It would be nice if we could write a Matcher, so that the macro 
composition could be more generically nested. e.g. hasAnyCapture that just 
loops over the LambdaCaptures and passes them to the child matcher.  However, 
when I tried this, it became apparent that it is not a Node type that can be 
matched on, and would require nontrivial changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72414



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

[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61726 tests passed, 0 failed 
and 779 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72355



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

[PATCH] D71775: [ThreadPool] On Windows, extend usage to all CPU sockets and all NUMA groups

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added inline comments.



Comment at: llvm/lib/Support/Unix/Threading.inc:273
+
+int computeHostNumPhysicalThreads() {
+#if defined(HAVE_SCHED_GETAFFINITY) && defined(HAVE_CPU_COUNT)

rnk wrote:
> I'm not sure it makes sense to say "physical threads". I think "physical 
> cores" was meant to distinguish between hardware threads and cores. 
> Describing hardware execution resources as physical or non-physical isn't 
> very precise or meaningful in the first place, but I don't think it should 
> apply to hyper threads.
`computeHostNumHardwareThreads()` ?


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

https://reviews.llvm.org/D71775



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

[PATCH] D71775: [ThreadPool] On Windows, extend usage to all CPU sockets and all NUMA groups

[Alexandre Ganea via Phabricator via cfe-commits]

aganea updated this revision to Diff 237197.
aganea marked 13 inline comments as done.
aganea added a comment.
Herald added subscribers: lucyrfox, mgester, arpith-jacob, nicolasvasilache, 
antiagainst, shauheen, burmako, jpienaar, rriddle.

Updated as suggested by @rnk.
I've also removed `ThreadPoolStrategy::PinThreads` because it wasn't 
conclusive. In the best case, pinning threads to a core / hyperthread, was 
similar in performance to that of using a full CPU socket affinity; in the 
worst case, it was degrading performance. The NT scheduler seems to be doing a 
pretty good job here, so we'll stick with it :-)


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

https://reviews.llvm.org/D71775

Files:
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/index/Background.cpp
  clang-tools-extra/clangd/index/Background.h
  clang-tools-extra/clangd/index/BackgroundRebuild.h
  clang/lib/Tooling/AllTUsExecution.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningFilesystem.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp
  lld/ELF/SyntheticSections.cpp
  llvm/include/llvm/ADT/BitVector.h
  llvm/include/llvm/ADT/SmallBitVector.h
  llvm/include/llvm/LTO/LTO.h
  llvm/include/llvm/Support/ThreadPool.h
  llvm/include/llvm/Support/Threading.h
  llvm/lib/CodeGen/ParallelCG.cpp
  llvm/lib/ExecutionEngine/Orc/LLJIT.cpp
  llvm/lib/LTO/LTO.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/LTO/ThinLTOCodeGenerator.cpp
  llvm/lib/Support/Host.cpp
  llvm/lib/Support/Parallel.cpp
  llvm/lib/Support/ThreadPool.cpp
  llvm/lib/Support/Threading.cpp
  llvm/lib/Support/Unix/Threading.inc
  llvm/lib/Support/Windows/Threading.inc
  llvm/tools/dsymutil/DwarfLinkerForBinary.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/gold/gold-plugin.cpp
  llvm/tools/llvm-cov/CodeCoverage.cpp
  llvm/tools/llvm-cov/CoverageExporterJson.cpp
  llvm/tools/llvm-cov/CoverageReport.cpp
  llvm/tools/llvm-lto2/llvm-lto2.cpp
  llvm/tools/llvm-profdata/llvm-profdata.cpp
  llvm/unittests/Support/Host.cpp
  llvm/unittests/Support/TaskQueueTest.cpp
  llvm/unittests/Support/ThreadPool.cpp
  llvm/unittests/Support/Threading.cpp
  mlir/lib/Pass/Pass.cpp

Index: mlir/lib/Pass/Pass.cpp
===
--- mlir/lib/Pass/Pass.cpp
+++ mlir/lib/Pass/Pass.cpp
@@ -411,7 +411,8 @@
   // Create the async executors if they haven't been created, or if the main
   // pipeline has changed.
   if (asyncExecutors.empty() || hasSizeMismatch(asyncExecutors.front(), mgrs))
-asyncExecutors.assign(llvm::hardware_concurrency(), mgrs);
+asyncExecutors.assign(llvm::hardware_concurrency().compute_thread_count(),
+  mgrs);
 
   // Run a prepass over the module to collect the operations to execute over.
   // This ensures that an analysis manager exists for each operation, as well as
Index: llvm/unittests/Support/Threading.cpp
===
--- llvm/unittests/Support/Threading.cpp
+++ llvm/unittests/Support/Threading.cpp
@@ -21,7 +21,8 @@
   auto Num = heavyweight_hardware_concurrency();
   // Since Num is unsigned this will also catch us trying to
   // return -1.
-  ASSERT_LE(Num, thread::hardware_concurrency());
+  ASSERT_LE(Num.compute_thread_count(),
+hardware_concurrency().compute_thread_count());
 }
 
 #if LLVM_ENABLE_THREADS
Index: llvm/unittests/Support/ThreadPool.cpp
===
--- llvm/unittests/Support/ThreadPool.cpp
+++ llvm/unittests/Support/ThreadPool.cpp
@@ -8,11 +8,13 @@
 
 #include "llvm/Support/ThreadPool.h"
 
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/TargetSelect.h"
+#include "llvm/Support/Threading.h"
 
 #include "gtest/gtest.h"
 
@@ -69,6 +71,8 @@
 
   void SetUp() override { MainThreadReady = false; }
 
+  void TestAllThreads(ThreadPoolStrategy S);
+
   std::condition_variable WaitMainThread;
   std::mutex WaitMainThreadMutex;
   bool MainThreadReady = false;
@@ -131,7 +135,7 @@
 
 TEST_F(ThreadPoolTest, GetFuture) {
   CHECK_UNSUPPORTED();
-  ThreadPool Pool{2};
+  ThreadPool Pool(hardware_concurrency(2));
   std::atomic_int i{0};
   Pool.async([this, ] {
 waitForMainThread();
@@ -162,3 +166,45 @@
   }
   ASSERT_EQ(5, checked_in);
 }
+
+#if LLVM_ENABLE_THREADS == 1
+
+void ThreadPoolTest::TestAllThreads(ThreadPoolStrategy S) {
+  // FIXME: Skip these tests on non-Windows because multi-socket system were not
+  // tested on Unix yet, and llvm::get_thread_affinity_mask() isn't implemented
+  // for Unix.
+  Triple Host(Triple::normalize(sys::getProcessTriple()));
+  if (!Host.isOSWindows())
+return;
+
+  llvm::DenseSet ThreadsUsed;
+  std::mutex Lock;
+  unsigned Threads = 0;
+  {
+ThreadPool Pool(S);
+Threads 

[PATCH] D67537: [clangd] Client-side support for inactive regions

[Nathan Ridge via Phabricator via cfe-commits]

nridge added reviewers: kadircet, ilya-biryukov, sammccall.
nridge added a comment.

Review ping again :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67537



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

[PATCH] D72414: Add new AST matchers `hasAnyCapture` and `capturesThis`

[Reid via Phabricator via cfe-commits]

rhiro updated this revision to Diff 237195.
rhiro added a comment.

- Change the hasCaptureThis functionality to be an override of hasAnyCapture 
with cxxThisExpr argument. Also generate the documentation and update tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72414

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -454,6 +454,26 @@
   objcMessageExpr(hasReceiver(declRefExpr(to(varDecl(hasName("x";
 }
 
+TEST(Matcher, HasAnyCapture) {
+  auto HasCaptureX = lambdaExpr(hasAnyCapture(varDecl(hasName("x";
+  EXPECT_TRUE(matches("void f() { int x = 3; [x](){}; }", HasCaptureX));
+  EXPECT_TRUE(matches("void f() { int x = 3; [](){}; }", HasCaptureX));
+  EXPECT_TRUE(notMatches("void f() { [](){}; }", HasCaptureX));
+  EXPECT_TRUE(notMatches("void f() { int z = 3; [](){}; }", HasCaptureX));
+  EXPECT_TRUE(
+  notMatches("struct a { void f() { [this](){}; }; };", HasCaptureX));
+}
+
+TEST(Matcher, CapturesThis) {
+  auto HasCaptureThis = lambdaExpr(hasAnyCapture(cxxThisExpr()));
+  EXPECT_TRUE(
+  matches("struct a { void f() { [this](){}; }; };", HasCaptureThis));
+  EXPECT_TRUE(notMatches("void f() { [](){}; }", HasCaptureThis));
+  EXPECT_TRUE(notMatches("void f() { int x = 3; [x](){}; }", HasCaptureThis));
+  EXPECT_TRUE(notMatches("void f() { int x = 3; [](){}; }", HasCaptureThis));
+  EXPECT_TRUE(notMatches("void f() { int z = 3; [](){}; }", HasCaptureThis));
+}
+
 TEST(Matcher, isClassMessage) {
   EXPECT_TRUE(matchesObjC(
   "@interface NSString +(NSString *) stringWithFormat; @end "
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -105,6 +105,7 @@
   // equalsNode
 
   REGISTER_OVERLOADED_2(callee);
+  REGISTER_OVERLOADED_2(hasAnyCapture);
   REGISTER_OVERLOADED_2(hasPrefix);
   REGISTER_OVERLOADED_2(hasType);
   REGISTER_OVERLOADED_2(ignoringParens);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -55,6 +55,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
+#include "clang/AST/LambdaCapture.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/OpenMPClause.h"
 #include "clang/AST/OperationKinds.h"
@@ -4014,6 +4015,54 @@
   return false;
 }
 
+/// Matches any capture of a lambda expression.
+///
+/// Given
+/// \code
+///   void foo() {
+/// int x;
+/// auto f = [x](){};
+///   }
+/// \endcode
+/// lambdaExpr(hasAnyCapture(anything()))
+///   matches [x](){};
+AST_MATCHER_P_OVERLOAD(LambdaExpr, hasAnyCapture, internal::Matcher,
+   InnerMatcher, 0) {
+  for (const LambdaCapture  : Node.captures()) {
+if (Capture.capturesThis()) {
+  continue;
+}
+BoundNodesTreeBuilder Result(*Builder);
+if (InnerMatcher.matches(*Capture.getCapturedVar(), Finder, )) {
+  *Builder = std::move(Result);
+  return true;
+}
+  }
+  return false;
+}
+
+/// Matches any capture of 'this' in a lambda expression.
+///
+/// Given
+/// \code
+///   struct foo {
+/// void bar() {
+///   auto f = [this](){};
+/// }
+///   }
+/// \endcode
+/// lambdaExpr(hasAnyCapture(cxxThisExpr()))
+///   matches [this](){};
+AST_MATCHER_P_OVERLOAD(LambdaExpr, hasAnyCapture,
+   internal::Matcher, InnerMatcher, 1) {
+  for (const LambdaCapture  : Node.captures()) {
+if (Capture.capturesThis()) {
+  return true;
+}
+  }
+  return false;
+}
+
 /// Matches a constructor call expression which uses list initialization.
 AST_MATCHER(CXXConstructExpr, isListInitialization) {
   return Node.isListInitialization();
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -5098,15 +5098,15 @@
 Matches selection statements with initializer.
 
 Given:
- void foo() { 
+ void foo() {
if (int i = foobar(); i  0) {}
switch (int i = foobar(); i) {}
-   for (auto a = get_range(); auto x : a) {} 
+   for (auto a = get_range(); auto x : a) {}
  }
- void bar() { 
+ void bar() {
if (foobar()  0) {}
switch (foobar()) {}
-   for (auto x : get_range()) {} 
+   for (auto x : get_range()) {}
  

[PATCH] D72355: [clangd] Assert that the testcases in FindExplicitReferencesTest.All have no diagnostics

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 237193.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72355

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


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -566,6 +566,10 @@
 TU.ExtraArgs.push_back("-std=c++17");
 
 auto AST = TU.build();
+for (auto  : AST.getDiagnostics()) {
+  if (D.Severity > DiagnosticsEngine::Warning)
+ADD_FAILURE() << D << Code;
+}
 
 auto *TestDecl = (AST, "foo");
 if (auto *T = llvm::dyn_cast(TestDecl))
@@ -718,7 +722,7 @@
 "3: targets = {vb}, decl\n"},
// MemberExpr should know their using declaration.
{R"cpp(
-struct X { void func(int); }
+struct X { void func(int); };
 struct Y : X {
   using X::func;
 };
@@ -824,7 +828,7 @@
 void foo() {
   $0^TT $1^x;
   $2^foo<$3^TT>();
-  $4^foo<$5^vector>()
+  $4^foo<$5^vector>();
   $6^foo<$7^TP...>();
 }
 )cpp",
@@ -891,7 +895,7 @@
};
// delegating initializer
class $10^Foo {
- $11^Foo(int);
+ $11^Foo(int) {}
  $12^Foo(): $13^Foo(111) {}
};
  }
@@ -924,7 +928,7 @@
// Namespace aliases should be handled properly.
{
R"cpp(
-namespace ns { struct Type {} }
+namespace ns { struct Type {}; }
 namespace alias = ns;
 namespace rec_alias = alias;
 


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -566,6 +566,10 @@
 TU.ExtraArgs.push_back("-std=c++17");
 
 auto AST = TU.build();
+for (auto  : AST.getDiagnostics()) {
+  if (D.Severity > DiagnosticsEngine::Warning)
+ADD_FAILURE() << D << Code;
+}
 
 auto *TestDecl = (AST, "foo");
 if (auto *T = llvm::dyn_cast(TestDecl))
@@ -718,7 +722,7 @@
 "3: targets = {vb}, decl\n"},
// MemberExpr should know their using declaration.
{R"cpp(
-struct X { void func(int); }
+struct X { void func(int); };
 struct Y : X {
   using X::func;
 };
@@ -824,7 +828,7 @@
 void foo() {
   $0^TT $1^x;
   $2^foo<$3^TT>();
-  $4^foo<$5^vector>()
+  $4^foo<$5^vector>();
   $6^foo<$7^TP...>();
 }
 )cpp",
@@ -891,7 +895,7 @@
};
// delegating initializer
class $10^Foo {
- $11^Foo(int);
+ $11^Foo(int) {}
  $12^Foo(): $13^Foo(111) {}
};
  }
@@ -924,7 +928,7 @@
// Namespace aliases should be handled properly.
{
R"cpp(
-namespace ns { struct Type {} }
+namespace ns { struct Type {}; }
 namespace alias = ns;
 namespace rec_alias = alias;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71533: [clangd] Show template arguments in type hierarchy when possible

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61729 tests passed, 0 failed 
and 779 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71533



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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Alexander Richardson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8c387cbea76b: Add builtins for aligning and checking 
alignment of pointers and integers (authored by arichardson).

Changed prior to commit:
  https://reviews.llvm.org/D71499?vs=236243=237190#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-align-array.c
  clang/test/CodeGen/builtin-align-assumption.c
  clang/test/CodeGen/builtin-align.c
  clang/test/Sema/builtin-align.c
  clang/test/SemaCXX/builtin-align-cxx.cpp

Index: clang/test/SemaCXX/builtin-align-cxx.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/builtin-align-cxx.cpp
@@ -0,0 +1,236 @@
+// C++-specific checks for the alignment builtins
+// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -std=c++11 -o - %s -fsyntax-only -verify
+
+// Check that we don't crash when using dependent types in __builtin_align:
+template 
+void *c(void *d) { // expected-note{{candidate template ignored}}
+  return __builtin_align_down(d, b);
+}
+
+struct x {};
+x foo;
+void test(void *value) {
+  c(value);
+  c(value); // expected-error{{no matching function for call to 'c'}}
+}
+
+template 
+void test_templated_arguments() {
+  T array[ArraySize];   // expected-error{{variable has incomplete type 'fwddecl'}}
+  static_assert(__is_same(decltype(__builtin_align_up(array, Alignment)), T *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_align_down(array, Alignment)), T *),
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_is_aligned(array, Alignment)), bool),
+"return type should be bool");
+  T *x1 = __builtin_align_up(array, Alignment);
+  T *x2 = __builtin_align_down(array, Alignment);
+  bool x3 = __builtin_align_up(array, Alignment);
+}
+
+void test() {
+  test_templated_arguments(); // fine
+  test_templated_arguments();
+  // expected-note@-1{{in instantiation of function template specialization 'test_templated_arguments'}}
+  // expected-note@-2{{forward declaration of 'fwddecl'}}
+  test_templated_arguments(); // invalid alignment value
+  // expected-note@-1{{in instantiation of function template specialization 'test_templated_arguments'}}
+}
+
+template 
+void test_incorrect_alignment_without_instatiation(T value) {
+  int array[32];
+  static_assert(__is_same(decltype(__builtin_align_up(array, 31)), int *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_align_down(array, 7)), int *), // expected-error{{requested alignment is not a power of 2}}
+"return type should be the decayed array type");
+  static_assert(__is_same(decltype(__builtin_is_aligned(array, -1)), bool), // expected-error{{requested alignment must be 1 or greater}}
+"return type should be bool");
+  __builtin_align_up(array);   // expected-error{{too few arguments to function call, expected 2, have 1}}
+  __builtin_align_up(array, 31);   // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_down(array, 31); // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_up(array, 31);   // expected-error{{requested alignment is not a power of 2}}
+  __builtin_align_up(value, 31);   // This shouldn't want since the type is dependent
+  __builtin_align_up(value);   // Same here
+}
+
+// The original fix for the issue above broke some legitimate code.
+// Here is a regression test:
+typedef __SIZE_TYPE__ size_t;
+void *allocate_impl(size_t size);
+template 
+T *allocate() {
+  constexpr size_t allocation_size =
+  __builtin_align_up(sizeof(T), sizeof(void *));
+  return static_cast(
+  __builtin_assume_aligned(allocate_impl(allocation_size), sizeof(void *)));
+}
+struct Foo {
+  int value;
+};
+void *test2() {
+  return allocate();
+}
+
+// Check that pointers-to-members cannot be used:
+class MemPtr {
+public:
+  int data;
+  void func();
+  virtual void vfunc();
+};
+void test_member_ptr() {
+  __builtin_align_up(::data, 64);// expected-error{{operand of type 'int MemPtr::*' where arithmetic or pointer type is required}}
+  __builtin_align_down(::func, 64);  // expected-error{{operand of type 'void (MemPtr::*)()' where arithmetic or 

[PATCH] D69878: Consoldiate internal denormal flushing controls

[Matt Arsenault via Phabricator via cfe-commits]

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



Comment at: llvm/docs/LangRef.rst:1837
+   are present, this overrides ``"denormal-fp-math"``. Not all targets
+   support separately setting the denormal mode per type.
+

andrew.w.kaylor wrote:
> arsenm wrote:
> > andrew.w.kaylor wrote:
> > > Can you document which targets do support the option? What happens if I 
> > > try to use the option on a target where it is not supported?
> > I'm not sure where to document this, or if/how/where to diagnose it. I 
> > don't think the high level LangRef description is the right place to 
> > discuss specific target handling.
> > 
> > Currently it won't error or anything. Code checking the denorm mode will 
> > see the f32 specific mode, even if the target in the end isn't really going 
> > to respect this.
> > 
> > One problem is this potentially does require coordination with other 
> > toolchain components. For AMDGPU, the compiler can directly tell the driver 
> > what FP mode to set on each entry point, but for x86 it requires linking in 
> > crtfastmath to set the default mode bits. If another target had a similar 
> > runtime environment requirement, I don't think we can be sure the attribute 
> > is correct or not.
> There is precedent for describing target-specific behavior in LangRef. It 
> just doesn't seem useful to say that not all targets support the attribute 
> without saying which ones do. We should also say what is expected if a target 
> doesn't support the attribute. It seems reasonable for the function attribute 
> to be silently ignored.
> 
> > One problem is this potentially does require coordination with other 
> > toolchain components. For AMDGPU, the compiler can directly tell the driver 
> > what FP mode to set on each entry point, but for x86 it requires linking in 
> > crtfastmath to set the default mode bits.
> 
> This is a point I'm interested in. I don't like the current crtfastmath.o 
> handling. It feels almost accidental when FTZ works as expected. My 
> understanding is we link crtfastmath.o if we find it but if not everything 
> just goes about its business. The Intel compiler injects code into main() to 
> explicitly set the FTZ/DAZ control modes. That obviously has problems too, 
> but it's at least consistent and predictable. As I understand it, 
> crtfastmath.o sets these modes from a static initializer, but I'm not sure 
> anything is done to determine the order of that initializer relative to 
> others.
> 
> How does the compiler identify entry points for AMDGPU? And does it emit code 
> to set FTZ based on the function attribute here?
The entry points are a specific calling convention. There's no real concept of 
main. Each kernel has an associated blob of metadata the driver uses to set up 
various config registers on dispatch.

I don't think specially recognizing main in the compiler is fundamentally 
different than having it done in a static constructor. It's still a construct 
not associated with any particular function or anything.


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

https://reviews.llvm.org/D69878



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

[clang] 8c387cb - Add builtins for aligning and checking alignment of pointers and integers

[Alex Richardson via cfe-commits]

Author: Alex Richardson
Date: 2020-01-09T21:48:29Z
New Revision: 8c387cbea76b169f1f8ecc7693797e96567ed896

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

LOG: Add builtins for aligning and checking alignment of pointers and integers

This change introduces three new builtins (which work on both pointers
and integers) that can be used instead of common bitwise arithmetic:
__builtin_align_up(x, alignment), __builtin_align_down(x, alignment) and
__builtin_is_aligned(x, alignment).

I originally added these builtins to the CHERI fork of LLVM a few years ago
to handle the slightly different C semantics that we use for CHERI [1].
Until recently these builtins (or sequences of other builtins) were
required to generate correct code. I have since made changes to the default
C semantics so that they are no longer strictly necessary (but using them
does generate slightly more efficient code). However, based on our experience
using them in various projects over the past few years, I believe that adding
these builtins to clang would be useful.

These builtins have the following benefit over bit-manipulation and casts
via uintptr_t:

- The named builtins clearly convey the semantics of the operation. While
  checking alignment using __builtin_is_aligned(x, 16) versus
  ((x & 15) == 0) is probably not a huge win in readably, I personally find
  __builtin_align_up(x, N) a lot easier to read than (x+(N-1))&~(N-1).
- They preserve the type of the argument (including const qualifiers). When
  using casts via uintptr_t, it is easy to cast to the wrong type or strip
  qualifiers such as const.
- If the alignment argument is a constant value, clang can check that it is
  a power-of-two and within the range of the type. Since the semantics of
  these builtins is well defined compared to arbitrary bit-manipulation,
  it is possible to add a UBSAN checker that the run-time value is a valid
  power-of-two. I intend to add this as a follow-up to this change.
- The builtins avoids int-to-pointer casts both in C and LLVM IR.
  In the future (i.e. once most optimizations handle it), we could use the new
  llvm.ptrmask intrinsic to avoid the ptrtoint instruction that would normally
  be generated.
- They can be used to round up/down to the next aligned value for both
  integers and pointers without requiring two separate macros.
- In many projects the alignment operations are already wrapped in macros (e.g.
  roundup2 and rounddown2 in FreeBSD), so by replacing the macro implementation
  with a builtin call, we get improved diagnostics for many call-sites while
  only having to change a few lines.
- Finally, the builtins also emit assume_aligned metadata when used on pointers.
  This can improve code generation compared to the uintptr_t casts.

[1] In our CHERI compiler we have compilation mode where all pointers are
implemented as capabilities (essentially unforgeable 128-bit fat pointers).
In our original model, casts from uintptr_t (which is a 128-bit capability)
to an integer value returned the "offset" of the capability (i.e. the
difference between the virtual address and the base of the allocation).
This causes problems for cases such as checking the alignment: for example, the
expression `if ((uintptr_t)ptr & 63) == 0` is generally used to check if the
pointer is aligned to a multiple of 64 bytes. The problem with offsets is that
any pointer to the beginning of an allocation will have an offset of zero, so
this check always succeeds in that case (even if the address is not correctly
aligned). The same issues also exist when aligning up or down. Using the
alignment builtins ensures that the address is used instead of the offset. While
I have since changed the default C semantics to return the address instead of
the offset when casting, this offset compilation mode can still be used by
passing a command-line flag.

Reviewers: rsmith, aaron.ballman, theraven, fhahn, lebedev.ri, nlopes, aqjune
Reviewed By: aaron.ballman, lebedev.ri
Differential Revision: https://reviews.llvm.org/D71499

Added: 
clang/test/CodeGen/builtin-align-array.c
clang/test/CodeGen/builtin-align-assumption.c
clang/test/CodeGen/builtin-align.c
clang/test/Sema/builtin-align.c
clang/test/SemaCXX/builtin-align-cxx.cpp

Modified: 
clang/docs/LanguageExtensions.rst
clang/include/clang/Basic/Builtins.def
clang/include/clang/Basic/DiagnosticASTKinds.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/ExprConstant.cpp
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CodeGenFunction.h
clang/lib/Sema/SemaChecking.cpp

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index b0f57202e079..0bd87903f349 100644
--- 

[PATCH] D71533: [clangd] Show template arguments in type hierarchy when possible

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 237186.
nridge added a comment.

Address latest review comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71533

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp

Index: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
@@ -409,17 +409,21 @@
   ASSERT_TRUE(!AST.getDiagnostics().empty());
 
   // Make sure getTypeHierarchy() doesn't get into an infinite recursion.
-  // FIXME(nridge): It would be preferable if the type hierarchy gave us type
-  // names (e.g. "S<0>" for the child and "S<1>" for the parent) rather than
-  // template names (e.g. "S").
+  // The parent is reported as "S" because "S<0>" is an invalid instantiation.
+  // We then iterate once more and find "S" again before detecting the
+  // recursion.
   llvm::Optional Result = getTypeHierarchy(
   AST, Source.points()[0], 0, TypeHierarchyDirection::Parents);
   ASSERT_TRUE(bool(Result));
   EXPECT_THAT(
   *Result,
-  AllOf(WithName("S"), WithKind(SymbolKind::Struct),
-Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
-  SelectionRangeIs(Source.range("SDef")), Parents();
+  AllOf(WithName("S<0>"), WithKind(SymbolKind::Struct),
+Parents(
+AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+  SelectionRangeIs(Source.range("SDef")),
+  Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
+SelectionRangeIs(Source.range("SDef")),
+Parents()));
 }
 
 TEST(TypeHierarchy, RecursiveHierarchyBounded) {
@@ -449,9 +453,12 @@
   ASSERT_TRUE(bool(Result));
   EXPECT_THAT(
   *Result,
-  AllOf(WithName("S"), WithKind(SymbolKind::Struct),
-Parents(AllOf(WithName("S"), WithKind(SymbolKind::Struct),
-  SelectionRangeIs(Source.range("SDef")), Parents();
+  AllOf(WithName("S<2>"), WithKind(SymbolKind::Struct),
+Parents(AllOf(
+WithName("S<1>"), WithKind(SymbolKind::Struct),
+SelectionRangeIs(Source.range("SDef")),
+Parents(AllOf(WithName("S<0>"), WithKind(SymbolKind::Struct),
+  Parents()));
   Result = getTypeHierarchy(AST, Source.point("SRefDependent"), 0,
 TypeHierarchyDirection::Parents);
   ASSERT_TRUE(bool(Result));
@@ -462,6 +469,83 @@
   SelectionRangeIs(Source.range("SDef")), Parents();
 }
 
+TEST(TypeHierarchy, DeriveFromImplicitSpec) {
+  Annotations Source(R"cpp(
+  template 
+  struct Parent {};
+
+  struct Child : Parent {};
+
+  Parent Fo^o;
+  )cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+  auto Index = TU.index();
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  llvm::Optional Result = getTypeHierarchy(
+  AST, Source.points()[0], 2, TypeHierarchyDirection::Children, Index.get(),
+  testPath(TU.Filename));
+  ASSERT_TRUE(bool(Result));
+  EXPECT_THAT(*Result,
+  AllOf(WithName("Parent"), WithKind(SymbolKind::Struct),
+Children(AllOf(WithName("Child"),
+   WithKind(SymbolKind::Struct), Children();
+}
+
+TEST(TypeHierarchy, DeriveFromPartialSpec) {
+  Annotations Source(R"cpp(
+  template  struct Parent {};
+  template  struct Parent {};
+
+  struct Child : Parent {};
+
+  Parent Fo^o;
+  )cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+  auto Index = TU.index();
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  llvm::Optional Result = getTypeHierarchy(
+  AST, Source.points()[0], 2, TypeHierarchyDirection::Children, Index.get(),
+  testPath(TU.Filename));
+  ASSERT_TRUE(bool(Result));
+  EXPECT_THAT(*Result, AllOf(WithName("Parent"),
+ WithKind(SymbolKind::Struct), Children()));
+}
+
+TEST(TypeHierarchy, DeriveFromTemplate) {
+  Annotations Source(R"cpp(
+  template 
+  struct Parent {};
+
+  template 
+  struct Child : Parent {};
+
+  Parent Fo^o;
+  )cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+  auto Index = TU.index();
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  // FIXME: We'd like this to return the implicit specialization Child,
+  //but currently libIndex does not expose relationships between
+  //implicit specializations.
+  llvm::Optional Result = getTypeHierarchy(
+  AST, Source.points()[0], 2, TypeHierarchyDirection::Children, Index.get(),
+  testPath(TU.Filename));
+  ASSERT_TRUE(bool(Result));
+ 

[PATCH] D72449: [PATCH] [llvm-ranlib] Take in consideration UTC offset for D-flag.test

[Adhemerval Zanella via Phabricator via cfe-commits]

zatrazz added a comment.

In D72449#1812818 , @arichardson wrote:

> I'm fine with this workaround although I'm very surprised that the test is 
> not working. Especially since `deterministic-archive.test` and 
> `replace-update.test` also set TZ to get reproducible output
>
> Do you have access to any of the failing bots? If so what happens there if 
> you change the TZ variable?
>  I get the following on all systems I have access to when I run something 
> like `date && date -u && env TZ=CET date && env TZ=CET date -u && env TZ=BST 
> date && env TZ=BST date -u` I get the following output:
>
>   Thu  9 Jan 2020 20:18:10 GMT
>   Thu  9 Jan 2020 20:18:10 UTC
>   Thu  9 Jan 2020 21:18:10 CET
>   Thu  9 Jan 2020 20:18:10 UTC
>   Thu  9 Jan 2020 20:18:10 UTC
>   Thu  9 Jan 2020 20:18:10 UTC
>


I am not sure now if it was something wrong with the bot tzdata, I reinstalled 
the tzdata packages
and reconfigured the timezone and now 'TZ=UTC date -d '@0' +%H:%M' does show 
the expected
00:00.

Indeed, although the workaround shouldn't change the expected result, the issue 
I am observing
on some machine does not make sense (TZ=UTC date -d '@0' +%H:%M' not showing 
00:00).

I will hold this and investigate further.


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

https://reviews.llvm.org/D72449



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

[PATCH] D72119: [clangd] Handle DeducedTemplateSpecializationType in TargetFinder

[Nathan Ridge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6a69d3c6b3da: [clangd] Handle 
DeducedTemplateSpecializationType in TargetFinder (authored by nridge).

Changed prior to commit:
  https://reviews.llvm.org/D72119?vs=236653=237183#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72119

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -315,6 +315,20 @@
   EXPECT_DECLS("TemplateSpecializationTypeLoc",
{"template<> class Foo", Rel::TemplateInstantiation},
{"template  class Foo", Rel::TemplatePattern});
+
+  Code = R"cpp(
+// Class template argument deduction
+template 
+struct Test {
+  Test(T);
+};
+void foo() {
+  [[Test]] a(5);
+}
+  )cpp";
+  Flags.push_back("-std=c++17");
+  EXPECT_DECLS("DeducedTemplateSpecializationTypeLoc",
+   {"struct Test", Rel::TemplatePattern});
 }
 
 TEST_F(TargetDeclTest, FunctionTemplate) {
@@ -549,6 +563,7 @@
 // FIXME: Auto-completion in a template requires disabling delayed template
 // parsing.
 TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+TU.ExtraArgs.push_back("-std=c++17");
 
 auto AST = TU.build();
 
@@ -937,7 +952,20 @@
 };
 )cpp",
"0: targets = {size}, decl\n"
-   "1: targets = {E}\n"}};
+   "1: targets = {E}\n"},
+   // Class template argument deduction
+   {
+   R"cpp(
+template 
+struct Test {
+Test(T);
+  };
+  void foo() {
+$0^Test $1^a(5);
+  }
+)cpp",
+   "0: targets = {Test}\n"
+   "1: targets = {a}, decl\n"}};
 
   for (const auto  : Cases) {
 llvm::StringRef ExpectedCode = C.first;
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -358,6 +358,17 @@
 // TypeLoc never has a deduced type. https://llvm.org/PR42914
 Outer.add(DT->getDeducedType(), Flags | Rel::Underlying);
   }
+  void VisitDeducedTemplateSpecializationType(
+  const DeducedTemplateSpecializationType *DTST) {
+// FIXME: This is a workaround for https://llvm.org/PR42914,
+// which is causing DTST->getDeducedType() to be empty. We
+// fall back to the template pattern and miss the instantiation
+// even when it's known in principle. Once that bug is fixed,
+// this method can be removed (the existing handling in
+// VisitDeducedType() is sufficient).
+if (auto *TD = DTST->getTemplateName().getAsTemplateDecl())
+  Outer.add(TD->getTemplatedDecl(), Flags | Rel::TemplatePattern);
+  }
   void VisitTypedefType(const TypedefType *TT) {
 Outer.add(TT->getDecl(), Flags);
   }


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -315,6 +315,20 @@
   EXPECT_DECLS("TemplateSpecializationTypeLoc",
{"template<> class Foo", Rel::TemplateInstantiation},
{"template  class Foo", Rel::TemplatePattern});
+
+  Code = R"cpp(
+// Class template argument deduction
+template 
+struct Test {
+  Test(T);
+};
+void foo() {
+  [[Test]] a(5);
+}
+  )cpp";
+  Flags.push_back("-std=c++17");
+  EXPECT_DECLS("DeducedTemplateSpecializationTypeLoc",
+   {"struct Test", Rel::TemplatePattern});
 }
 
 TEST_F(TargetDeclTest, FunctionTemplate) {
@@ -549,6 +563,7 @@
 // FIXME: Auto-completion in a template requires disabling delayed template
 // parsing.
 TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+TU.ExtraArgs.push_back("-std=c++17");
 
 auto AST = TU.build();
 
@@ -937,7 +952,20 @@
 };
 )cpp",
"0: targets = {size}, decl\n"
-   "1: targets = {E}\n"}};
+   "1: targets = {E}\n"},
+   // Class template argument deduction
+   {
+   R"cpp(
+template 
+struct Test {
+Test(T);
+  };
+  void foo() {
+$0^Test $1^a(5);
+  }
+)cpp",
+   "0: targets = {Test}\n"
+   "1: targets = {a}, decl\n"}};
 
   for (const auto  : Cases) {
 

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[Amy Huang via Phabricator via cfe-commits]

akhuang added a comment.

> What's the plan for this? Is it still in an experimental stage, with the 
> intent to investigate the types that are no longer emitted unedr the flag & 
> explain why they're missing (& either have a justification for why that's 
> acceptable, or work on additional heuristics to address the gaps?)?
> 
> If so, I'd probably rather this not be a full driver flag - if it's a 
> reliable way to reduce debug info size (like the existing heuristics under 
> -fstandalone-debug*) it should be rolled into -fno-standalone-debug behavior, 
> and if it's not fully fleshed out yet, I think an -Xclang flag would be more 
> suitable for the experimental phase.

Pretty much, I think the plan is to investigate further and maybe have more 
people try it. The -Xclang flag seems reasonable. Do you have thoughts on 
whether the added DebugInfoKind level makes sense?

> Do you have any sample of data on the sort of situations that lead to missing 
> types under this heuristic?

I built clang with the heuristic and linked to a list of missing types in the 
description but haven't really looked into them more specifically.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72427



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

[clang-tools-extra] 6a69d3c - [clangd] Handle DeducedTemplateSpecializationType in TargetFinder

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2020-01-09T16:14:11-05:00
New Revision: 6a69d3c6b3da3d4e7709d11fd52b8e5540265280

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

LOG: [clangd] Handle DeducedTemplateSpecializationType in TargetFinder

Summary:
This is a workaround for https://bugs.llvm.org/show_bug.cgi?id=42914.
Once that is fixed, the handling in VisitDeducedTyped() should be sufficient.

Fixes https://github.com/clangd/clangd/issues/242

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, 
cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/FindTarget.cpp 
b/clang-tools-extra/clangd/FindTarget.cpp
index fb3001fca94c..119b932956be 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -358,6 +358,17 @@ struct TargetFinder {
 // TypeLoc never has a deduced type. https://llvm.org/PR42914
 Outer.add(DT->getDeducedType(), Flags | Rel::Underlying);
   }
+  void VisitDeducedTemplateSpecializationType(
+  const DeducedTemplateSpecializationType *DTST) {
+// FIXME: This is a workaround for https://llvm.org/PR42914,
+// which is causing DTST->getDeducedType() to be empty. We
+// fall back to the template pattern and miss the instantiation
+// even when it's known in principle. Once that bug is fixed,
+// this method can be removed (the existing handling in
+// VisitDeducedType() is sufficient).
+if (auto *TD = DTST->getTemplateName().getAsTemplateDecl())
+  Outer.add(TD->getTemplatedDecl(), Flags | Rel::TemplatePattern);
+  }
   void VisitTypedefType(const TypedefType *TT) {
 Outer.add(TT->getDecl(), Flags);
   }

diff  --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp 
b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index 8c9727fb64eb..f34205a81f30 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -315,6 +315,20 @@ TEST_F(TargetDeclTest, ClassTemplate) {
   EXPECT_DECLS("TemplateSpecializationTypeLoc",
{"template<> class Foo", Rel::TemplateInstantiation},
{"template  class Foo", Rel::TemplatePattern});
+
+  Code = R"cpp(
+// Class template argument deduction
+template 
+struct Test {
+  Test(T);
+};
+void foo() {
+  [[Test]] a(5);
+}
+  )cpp";
+  Flags.push_back("-std=c++17");
+  EXPECT_DECLS("DeducedTemplateSpecializationTypeLoc",
+   {"struct Test", Rel::TemplatePattern});
 }
 
 TEST_F(TargetDeclTest, FunctionTemplate) {
@@ -549,6 +563,7 @@ class FindExplicitReferencesTest : public ::testing::Test {
 // FIXME: Auto-completion in a template requires disabling delayed template
 // parsing.
 TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+TU.ExtraArgs.push_back("-std=c++17");
 
 auto AST = TU.build();
 
@@ -937,7 +952,20 @@ TEST_F(FindExplicitReferencesTest, All) {
 };
 )cpp",
"0: targets = {size}, decl\n"
-   "1: targets = {E}\n"}};
+   "1: targets = {E}\n"},
+   // Class template argument deduction
+   {
+   R"cpp(
+template 
+struct Test {
+Test(T);
+  };
+  void foo() {
+$0^Test $1^a(5);
+  }
+)cpp",
+   "0: targets = {Test}\n"
+   "1: targets = {a}, decl\n"}};
 
   for (const auto  : Cases) {
 llvm::StringRef ExpectedCode = C.first;



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

[PATCH] D72421: [clang-tidy] Refresh the add_new_check.py now that we use a table + autofix

[Sylvestre Ledru via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc348a2674b57: [clang-tidy] Refresh the add_new_check.py now 
that we use a table + autofix (authored by sylvestre.ledru).

Changed prior to commit:
  https://reviews.llvm.org/D72421?vs=236924=237178#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72421

Files:
  clang-tools-extra/clang-tidy/add_new_check.py

Index: clang-tools-extra/clang-tidy/add_new_check.py
===
--- clang-tools-extra/clang-tidy/add_new_check.py
+++ clang-tools-extra/clang-tidy/add_new_check.py
@@ -294,37 +294,89 @@
 def update_checks_list(clang_tidy_path):
   docs_dir = os.path.join(clang_tidy_path, '../docs/clang-tidy/checks')
   filename = os.path.normpath(os.path.join(docs_dir, 'list.rst'))
+  # Read the content of the current list.rst file
   with open(filename, 'r') as f:
 lines = f.readlines()
+  # Get all existing docs
   doc_files = list(filter(lambda s: s.endswith('.rst') and s != 'list.rst',
  os.listdir(docs_dir)))
   doc_files.sort()
 
-  def format_link(doc_file):
+  def has_auto_fix(check_name):
+dirname, _, check_name = check_name.partition("-")
+
+checkerCode = os.path.join(dirname, get_camel_name(check_name)) + ".cpp"
+
+if not os.path.isfile(checkerCode):
+  return ""
+
+with open(checkerCode) as f:
+  code = f.read()
+  if 'FixItHint' in code or "ReplacementText" in code or "fixit" in code:
+# Some simple heuristics to figure out if a checker has an autofix or not.
+return ' "Yes"'
+return ""
+
+  def process_doc(doc_file):
 check_name = doc_file.replace('.rst', '')
+
 with open(os.path.join(docs_dir, doc_file), 'r') as doc:
   content = doc.read()
   match = re.search('.*:orphan:.*', content)
+
   if match:
-return ''
+# Orphan page, don't list it.
+return '', ''
 
   match = re.search('.*:http-equiv=refresh: \d+;URL=(.*).html.*',
 content)
-  if match:
-return '   %(check)s (redirects to %(target)s) <%(check)s>\n' % {
-'check': check_name,
-'target': match.group(1)
-}
-  return '   %s\n' % check_name
+  # Is it a redirect?
+  return check_name, match
+
+  def format_link(doc_file):
+check_name, match = process_doc(doc_file)
+if not match and check_name:
+  return '   `%(check)s <%(check)s.html>`_,%(autofix)s\n' % {
+'check': check_name,
+'autofix': has_auto_fix(check_name)
+  }
+else:
+  return ''
+
+  def format_link_alias(doc_file):
+check_name, match = process_doc(doc_file)
+if match and check_name:
+  if match.group(1) == 'https://clang.llvm.org/docs/analyzer/checkers':
+title_redirect = 'Clang Static Analyzer'
+  else:
+title_redirect = match.group(1)
+  # The checker is just a redirect.
+  return '   `%(check)s <%(check)s.html>`_, `%(title)s <%(target)s.html>`_,%(autofix)s\n' % {
+'check': check_name,
+'target': match.group(1),
+'title': title_redirect,
+'autofix': has_auto_fix(match.group(1))
+  }
+return ''
 
   checks = map(format_link, doc_files)
+  checks_alias = map(format_link_alias, doc_files)
 
   print('Updating %s...' % filename)
   with open(filename, 'w') as f:
 for line in lines:
   f.write(line)
-  if line.startswith('.. toctree::'):
+  if line.strip() == ".. csv-table::":
+# We dump the checkers
+f.write('   :header: "Name", "Offers fixes"\n')
+f.write('   :widths: 50, 20\n\n')
 f.writelines(checks)
+# and the aliases
+f.write('\n\n')
+f.write('.. csv-table:: Aliases..\n')
+f.write('   :header: "Name", "Redirect", "Offers fixes"\n')
+f.write('   :widths: 50, 50, 10\n\n')
+f.writelines(checks_alias)
 break
 
 
@@ -345,6 +397,11 @@
'underline': '=' * len(check_name_dashes)})
 
 
+def get_camel_name(check_name):
+  return ''.join(map(lambda elem: elem.capitalize(),
+ check_name.split('-'))) + 'Check'
+
+
 def main():
   language_to_extension = {
   'c': 'c',
@@ -384,13 +441,11 @@
 
   module = args.module
   check_name = args.check
-
+  check_name_camel = get_camel_name(check_name)
   if check_name.startswith(module):
 print('Check name "%s" must not start with the module "%s". Exiting.' % (
 check_name, module))
 return
-  check_name_camel = ''.join(map(lambda elem: elem.capitalize(),
- check_name.split('-'))) + 'Check'
   clang_tidy_path = os.path.dirname(sys.argv[0])
   module_path = os.path.join(clang_tidy_path, module)
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D72449: [PATCH] [llvm-ranlib] Take in consideration UTC offset for D-flag.test

[Adhemerval Zanella via Phabricator via cfe-commits]

zatrazz added a comment.

In D72449#1812130 , @arichardson wrote:

> I was careful in the test to use `TZ=UTC` for all commands printing dates and 
> it works fine for me in UTC+1 and in the UK which is currently UTC. Does the 
> bot in question ignore the TZ variable?


The issue is setting TZ=UTC still might use internal database with UTC offsets. 
For instance, on the buildbot
that accused the regression, setting TZ=UTC make it use the tz database 
/usr/share/zoneinfo/UTC. At least
on Linux with glibc one way to actually use UTC without transit changes are to 
set TZ=":", it won't try to
access either the default '/etc/localtime'. However, I am not sure how portable 
it is and it might be in fact
an glibc issue (https://sourceware.org/bugzilla/show_bug.cgi?id=24004).


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

https://reviews.llvm.org/D72449



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

[PATCH] D68720: Support -fstack-clash-protection for x86

[Tom Stellard via Phabricator via cfe-commits]

tstellar added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2575
+  default:
+return;
+  case llvm::Triple::ArchType::x86:

Do we need to warn (or error) here for arches that don't implement 
-fstack-clash-protection?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68720



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

[PATCH] D72421: [clang-tidy] Refresh the add_new_check.py now that we use a table + autofix

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru marked 5 inline comments as done.
sylvestre.ledru added inline comments.



Comment at: clang-tools-extra/clang-tidy/add_new_check.py:372
+f.write('   :header: "Name", "Offers fixes"\n')
+f.write('   :widths: 50, 20\n\n')
 f.writelines(checks)

alexfh wrote:
> Is there a way to let sphinx find out column widths itself?
TIL, sphinx doesn't need this indeed!



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72421



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

[clang-tools-extra] c348a26 - [clang-tidy] Refresh the add_new_check.py now that we use a table + autofix

[Sylvestre Ledru via cfe-commits]

Author: Sylvestre Ledru
Date: 2020-01-09T22:11:18+01:00
New Revision: c348a2674b5753afde6842d5a6ee75db67e3

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

LOG: [clang-tidy] Refresh the add_new_check.py now that we use a table + autofix

Reviewers: alexfh

Reviewed By: alexfh

Subscribers: njames93, xazax.hun, mgorny, cfe-commits

Tags: #clang, #clang-tools-extra

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/add_new_check.py

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/add_new_check.py 
b/clang-tools-extra/clang-tidy/add_new_check.py
index 0a8a11b8d0dd..68a723900751 100755
--- a/clang-tools-extra/clang-tidy/add_new_check.py
+++ b/clang-tools-extra/clang-tidy/add_new_check.py
@@ -294,37 +294,89 @@ def write_test(module_path, module, check_name, 
test_extension):
 def update_checks_list(clang_tidy_path):
   docs_dir = os.path.join(clang_tidy_path, '../docs/clang-tidy/checks')
   filename = os.path.normpath(os.path.join(docs_dir, 'list.rst'))
+  # Read the content of the current list.rst file
   with open(filename, 'r') as f:
 lines = f.readlines()
+  # Get all existing docs
   doc_files = list(filter(lambda s: s.endswith('.rst') and s != 'list.rst',
  os.listdir(docs_dir)))
   doc_files.sort()
 
-  def format_link(doc_file):
+  def has_auto_fix(check_name):
+dirname, _, check_name = check_name.partition("-")
+
+checkerCode = os.path.join(dirname, get_camel_name(check_name)) + ".cpp"
+
+if not os.path.isfile(checkerCode):
+  return ""
+
+with open(checkerCode) as f:
+  code = f.read()
+  if 'FixItHint' in code or "ReplacementText" in code or "fixit" in code:
+# Some simple heuristics to figure out if a checker has an autofix or 
not.
+return ' "Yes"'
+return ""
+
+  def process_doc(doc_file):
 check_name = doc_file.replace('.rst', '')
+
 with open(os.path.join(docs_dir, doc_file), 'r') as doc:
   content = doc.read()
   match = re.search('.*:orphan:.*', content)
+
   if match:
-return ''
+# Orphan page, don't list it.
+return '', ''
 
   match = re.search('.*:http-equiv=refresh: \d+;URL=(.*).html.*',
 content)
-  if match:
-return '   %(check)s (redirects to %(target)s) <%(check)s>\n' % {
-'check': check_name,
-'target': match.group(1)
-}
-  return '   %s\n' % check_name
+  # Is it a redirect?
+  return check_name, match
+
+  def format_link(doc_file):
+check_name, match = process_doc(doc_file)
+if not match and check_name:
+  return '   `%(check)s <%(check)s.html>`_,%(autofix)s\n' % {
+'check': check_name,
+'autofix': has_auto_fix(check_name)
+  }
+else:
+  return ''
+
+  def format_link_alias(doc_file):
+check_name, match = process_doc(doc_file)
+if match and check_name:
+  if match.group(1) == 'https://clang.llvm.org/docs/analyzer/checkers':
+title_redirect = 'Clang Static Analyzer'
+  else:
+title_redirect = match.group(1)
+  # The checker is just a redirect.
+  return '   `%(check)s <%(check)s.html>`_, `%(title)s 
<%(target)s.html>`_,%(autofix)s\n' % {
+'check': check_name,
+'target': match.group(1),
+'title': title_redirect,
+'autofix': has_auto_fix(match.group(1))
+  }
+return ''
 
   checks = map(format_link, doc_files)
+  checks_alias = map(format_link_alias, doc_files)
 
   print('Updating %s...' % filename)
   with open(filename, 'w') as f:
 for line in lines:
   f.write(line)
-  if line.startswith('.. toctree::'):
+  if line.strip() == ".. csv-table::":
+# We dump the checkers
+f.write('   :header: "Name", "Offers fixes"\n')
+f.write('   :widths: 50, 20\n\n')
 f.writelines(checks)
+# and the aliases
+f.write('\n\n')
+f.write('.. csv-table:: Aliases..\n')
+f.write('   :header: "Name", "Redirect", "Offers fixes"\n')
+f.write('   :widths: 50, 50, 10\n\n')
+f.writelines(checks_alias)
 break
 
 
@@ -345,6 +397,11 @@ def write_docs(module_path, module, check_name):
'underline': '=' * len(check_name_dashes)})
 
 
+def get_camel_name(check_name):
+  return ''.join(map(lambda elem: elem.capitalize(),
+ check_name.split('-'))) + 'Check'
+
+
 def main():
   language_to_extension = {
   'c': 'c',
@@ -384,13 +441,11 @@ def main():
 
   module = args.module
   check_name = args.check
-
+  check_name_camel = get_camel_name(check_name)
   if check_name.startswith(module):
 print('Check name "%s" must not start with the module "%s". Exiting.' % (
 

[PATCH] D72380: [DataFlow] Factor two worklist implementations out

[Matthias Gehre via Phabricator via cfe-commits]

mgehre added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowWorklist.h:20
+namespace clang {
+template  class DataflowWorklistBase {
+  llvm::BitVector EnqueuedBlocks;

xazax.hun wrote:
> xazax.hun wrote:
> > mgehre wrote:
> > > Should this class have a bit of doxygen and a unit test?
> > We have two users and both users have regression tests. More tests are 
> > always good, but I am not sure if we would get much value in this case. 
> > Having some comments sound very useful though :)
> Added a unit test anyway :)
;-)



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowWorklist.h:20
+namespace clang {
+/// A worklist implementation where the enqueued blocks wwill be dequeued based
+/// on the order defined by 'Comp'.

nit: wwill


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

https://reviews.llvm.org/D72380



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

[PATCH] D72449: [PATCH] [llvm-ranlib] Take in consideration UTC offset for D-flag.test

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added a comment.

I'm fine with this workaround although I'm very surprised that the test is not 
working. Especially since `deterministic-archive.test` and 
`replace-update.test` also set TZ to get reproducible output

Do you have access to any of the failing bots? If so what happens there if you 
change the TZ variable?
I get the following on all systems I have access to when I run something like 
`date && date -u && env TZ=CET date && env TZ=CET date -u && env TZ=BST date && 
env TZ=BST date -u` I get the following output:

  Thu  9 Jan 2020 20:18:10 GMT
  Thu  9 Jan 2020 20:18:10 UTC
  Thu  9 Jan 2020 21:18:10 CET
  Thu  9 Jan 2020 20:18:10 UTC
  Thu  9 Jan 2020 20:18:10 UTC
  Thu  9 Jan 2020 20:18:10 UTC




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

https://reviews.llvm.org/D72449



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

[PATCH] D69876: Allow output constraints on "asm goto"

[Bill Wendling via Phabricator via cfe-commits]

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

In D69876#1812724 , @jyknight wrote:

> Reopening, since this didn't actually land yet. BTW, this review still has 
> the wrong contents in the latest uploaded-diff (showing llvm changes, not 
> clang changes).


*Pinches bridge of nose*
*Curses under breath*




Comment at: clang/lib/AST/Stmt.cpp:646-648
+  // Labels are placed after "InOut" operands. Adjust accordingly.
+  if (IsLabel)
+N += getNumPlusOperands();

jyknight wrote:
> void wrote:
> > jyknight wrote:
> > > I'm confused about this part. Why isn't the "N" specified in the assembly 
> > > string already the correct value for the labels? Is the ordering we use 
> > > internally and that users use externally not the same? I'm assuming your 
> > > code here is correct, just I'm not understanding, so probably an improved 
> > > comment would be nice.
> > The LLVM-specific ordering that I saw was something like:
> > 
> >   `,,, > X>,`
> > 
> > The `` is empty when there are no output constraints. 
> > This just makes up for this. It's probably better to reverse the ` > X>` and `` parts, but I didn't know how pervasive the 
> > order's assumption is in the code.
> Oh, right -- in llvm IR, the "plus" constraints don't exist -- we specify 
> those as separate-but-linked output and input constraints.
> 
> But thinking about this more...I think this code is actually incorrect. It 
> should be translating the numbers based on where it finds the matching 
> argument-number, not on the 'l' character being present. (And it should do so 
> for named-arguments too).
> 
> So, yes, I think it'd be better to just swap the order, and move the 
> plus-operand tied-inputs to the end of the arglist, where they don't affect 
> numbering. I don't _think_ reversing the order will cause other issues.
I agree that moving them would make the most sense. Let me do some experiments 
to see if it breaks anything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876



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

[clang] b35f5d4 - [clang] Enforce triple in mempcpy test

[via cfe-commits]

Author: serge-sans-paille
Date: 2020-01-09T21:09:15+01:00
New Revision: b35f5d4914c979282010c0618a331d11a58493ac

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

LOG: [clang] Enforce triple in mempcpy test

Fixes http://lab.llvm.org:8011/builders/llvm-clang-win-x-armv7l/builds/2597

Added: 


Modified: 
clang/test/CodeGen/mempcpy-libcall.c

Removed: 




diff  --git a/clang/test/CodeGen/mempcpy-libcall.c 
b/clang/test/CodeGen/mempcpy-libcall.c
index b88f494f164d..1458c85f890f 100644
--- a/clang/test/CodeGen/mempcpy-libcall.c
+++ b/clang/test/CodeGen/mempcpy-libcall.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -emit-llvm < %s| FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm < %s| FileCheck 
%s
 
 typedef __SIZE_TYPE__ size_t;
 



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

[PATCH] D72417: [ms] [X86] Use "P" modifier on all branch-target operands in inline X86 assembly.

[Eric Astor via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1c545f6dbcbb: [ms] [X86] Use P modifier on all 
branch-target operands in inline X86… (authored by epastor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72417

Files:
  clang/test/CodeGen/ms-inline-asm-64.c
  llvm/include/llvm/MC/MCInstrDesc.h
  llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
  llvm/lib/Target/X86/AsmParser/X86Operand.h
  llvm/lib/Target/X86/X86InstrControl.td
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/utils/TableGen/InstrInfoEmitter.cpp
  llvm/utils/TableGen/X86RecognizableInstr.cpp

Index: llvm/utils/TableGen/X86RecognizableInstr.cpp
===
--- llvm/utils/TableGen/X86RecognizableInstr.cpp
+++ llvm/utils/TableGen/X86RecognizableInstr.cpp
@@ -879,9 +879,9 @@
   TYPE("i128mem", TYPE_M)
   TYPE("i256mem", TYPE_M)
   TYPE("i512mem", TYPE_M)
-  TYPE("i64i32imm_pcrel", TYPE_REL)
-  TYPE("i16imm_pcrel",TYPE_REL)
-  TYPE("i32imm_pcrel",TYPE_REL)
+  TYPE("i64i32imm_brtarget",  TYPE_REL)
+  TYPE("i16imm_brtarget", TYPE_REL)
+  TYPE("i32imm_brtarget", TYPE_REL)
   TYPE("ccode",   TYPE_IMM)
   TYPE("AVX512RC",TYPE_IMM)
   TYPE("brtarget32",  TYPE_REL)
@@ -1169,45 +1169,45 @@
   if(OpSize != X86Local::OpSize16) {
 // For instructions without an OpSize prefix, a declared 16-bit register or
 // immediate encoding is special.
-ENCODING("i16imm",ENCODING_IW)
+ENCODING("i16imm",   ENCODING_IW)
   }
-  ENCODING("i16imm",  ENCODING_Iv)
-  ENCODING("i16i8imm",ENCODING_IB)
-  ENCODING("i32imm",  ENCODING_Iv)
-  ENCODING("i32i8imm",ENCODING_IB)
-  ENCODING("i64i32imm",   ENCODING_ID)
-  ENCODING("i64i8imm",ENCODING_IB)
-  ENCODING("i8imm",   ENCODING_IB)
-  ENCODING("u8imm",   ENCODING_IB)
-  ENCODING("i16u8imm",ENCODING_IB)
-  ENCODING("i32u8imm",ENCODING_IB)
-  ENCODING("i64u8imm",ENCODING_IB)
-  ENCODING("i64i32imm_pcrel", ENCODING_ID)
-  ENCODING("i16imm_pcrel",ENCODING_IW)
-  ENCODING("i32imm_pcrel",ENCODING_ID)
-  ENCODING("brtarget32",  ENCODING_ID)
-  ENCODING("brtarget16",  ENCODING_IW)
-  ENCODING("brtarget8",   ENCODING_IB)
-  ENCODING("i64imm",  ENCODING_IO)
-  ENCODING("offset16_8",  ENCODING_Ia)
-  ENCODING("offset16_16", ENCODING_Ia)
-  ENCODING("offset16_32", ENCODING_Ia)
-  ENCODING("offset32_8",  ENCODING_Ia)
-  ENCODING("offset32_16", ENCODING_Ia)
-  ENCODING("offset32_32", ENCODING_Ia)
-  ENCODING("offset32_64", ENCODING_Ia)
-  ENCODING("offset64_8",  ENCODING_Ia)
-  ENCODING("offset64_16", ENCODING_Ia)
-  ENCODING("offset64_32", ENCODING_Ia)
-  ENCODING("offset64_64", ENCODING_Ia)
-  ENCODING("srcidx8", ENCODING_SI)
-  ENCODING("srcidx16",ENCODING_SI)
-  ENCODING("srcidx32",ENCODING_SI)
-  ENCODING("srcidx64",ENCODING_SI)
-  ENCODING("dstidx8", ENCODING_DI)
-  ENCODING("dstidx16",ENCODING_DI)
-  ENCODING("dstidx32",ENCODING_DI)
-  ENCODING("dstidx64",ENCODING_DI)
+  ENCODING("i16imm", ENCODING_Iv)
+  ENCODING("i16i8imm",   ENCODING_IB)
+  ENCODING("i32imm", ENCODING_Iv)
+  ENCODING("i32i8imm",   ENCODING_IB)
+  ENCODING("i64i32imm",  ENCODING_ID)
+  ENCODING("i64i8imm",   ENCODING_IB)
+  ENCODING("i8imm",  ENCODING_IB)
+  ENCODING("u8imm",  ENCODING_IB)
+  ENCODING("i16u8imm",   ENCODING_IB)
+  ENCODING("i32u8imm",   ENCODING_IB)
+  ENCODING("i64u8imm",   ENCODING_IB)
+  ENCODING("i64i32imm_brtarget", ENCODING_ID)
+  ENCODING("i16imm_brtarget",ENCODING_IW)
+  ENCODING("i32imm_brtarget",ENCODING_ID)
+  ENCODING("brtarget32", ENCODING_ID)
+  ENCODING("brtarget16", ENCODING_IW)
+  ENCODING("brtarget8",  ENCODING_IB)
+  ENCODING("i64imm", ENCODING_IO)
+  ENCODING("offset16_8", ENCODING_Ia)
+  ENCODING("offset16_16",ENCODING_Ia)
+  ENCODING("offset16_32",ENCODING_Ia)
+  ENCODING("offset32_8", ENCODING_Ia)
+  ENCODING("offset32_16",ENCODING_Ia)
+  ENCODING("offset32_32",ENCODING_Ia)
+  ENCODING("offset32_64",ENCODING_Ia)
+  ENCODING("offset64_8", ENCODING_Ia)
+  ENCODING("offset64_16",ENCODING_Ia)
+  ENCODING("offset64_32",ENCODING_Ia)
+  ENCODING("offset64_64",ENCODING_Ia)
+  ENCODING("srcidx8",ENCODING_SI)
+  ENCODING("srcidx16",   ENCODING_SI)
+  ENCODING("srcidx32",   ENCODING_SI)
+  ENCODING("srcidx64",   ENCODING_SI)
+  ENCODING("dstidx8",

[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

[Fady Ghanim via Phabricator via cfe-commits]

fghanim marked 8 inline comments as done.
fghanim added a comment.

So I modified the patch based on your comments. I removed all the code that I 
intended for llvm pass writers for now. 
I will look at it again later, figure something to do with the whole 
switch-if-cascade thing, and will introduce it in a separate patch. I think 
that would be better than to include it here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72304



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

[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

[Fady Ghanim via Phabricator via cfe-commits]

fghanim updated this revision to Diff 237149.
fghanim added a comment.

- Adding array types to OMPKinds.def. Inlining runtime function calls 
generation. reformatting with clang format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72304

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
  llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
  llvm/lib/Frontend/OpenMP/OMPConstants.cpp
  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
@@ -613,4 +613,179 @@
   }
 }
 
+TEST_F(OpenMPIRBuilderTest, MasterDirective) {
+  using InsertPointTy = OpenMPIRBuilder::InsertPointTy;
+  OpenMPIRBuilder OMPBuilder(*M);
+  OMPBuilder.initialize();
+  F->setName("func");
+  IRBuilder<> Builder(BB);
+
+  OpenMPIRBuilder::LocationDescription Loc({Builder.saveIP(), DL});
+
+  AllocaInst *PrivAI = nullptr;
+
+  BasicBlock *EntryBB = nullptr;
+  BasicBlock *FinalBB = nullptr;
+  BasicBlock *ExitBB = nullptr;
+  BasicBlock *ThenBB = nullptr;
+
+  auto BodyGenCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
+   BasicBlock ) {
+if (AllocaIP.isSet())
+  Builder.restoreIP(AllocaIP);
+else
+  Builder.SetInsertPoint(&*(F->getEntryBlock().getFirstInsertionPt()));
+PrivAI = Builder.CreateAlloca(F->arg_begin()->getType());
+Builder.CreateStore(F->arg_begin(), PrivAI);
+
+llvm::BasicBlock *CodeGenIPBB = CodeGenIP.getBlock();
+llvm::Instruction *CodeGenIPInst = &*CodeGenIP.getPoint();
+EXPECT_EQ(CodeGenIPBB->getTerminator(), CodeGenIPInst);
+
+Builder.restoreIP(CodeGenIP);
+
+// collect some info for checks later
+FinalBB = 
+ExitBB = FiniBB.getUniqueSuccessor();
+ThenBB = Builder.GetInsertBlock();
+EntryBB = ThenBB->getUniquePredecessor();
+
+// simple instructions for body
+Value *PrivLoad = Builder.CreateLoad(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.CreateMaster(Builder, BodyGenCB, FiniCB));
+  Value *EntryBBTI = EntryBB->getTerminator();
+  EXPECT_NE(EntryBBTI, nullptr);
+  EXPECT_TRUE(isa(EntryBBTI));
+  BranchInst *EntryBr = cast(EntryBB->getTerminator());
+  EXPECT_TRUE(EntryBr->isConditional());
+  EXPECT_EQ(EntryBr->getSuccessor(0), ThenBB);
+  EXPECT_EQ(ThenBB->getUniqueSuccessor(), FinalBB);
+  EXPECT_EQ(FinalBB->getUniqueSuccessor(), ExitBB);
+  EXPECT_EQ(EntryBr->getSuccessor(1), ExitBB);
+
+  CmpInst *CondInst = cast(EntryBr->getCondition());
+  EXPECT_TRUE(isa(CondInst->getOperand(0)));
+
+  CallInst *MasterEntryCI = cast(CondInst->getOperand(0));
+  EXPECT_EQ(MasterEntryCI->getNumArgOperands(), 2U);
+  EXPECT_EQ(MasterEntryCI->getCalledFunction()->getName(), "__kmpc_omp_master");
+  EXPECT_TRUE(isa(MasterEntryCI->getArgOperand(0)));
+
+  CallInst *MasterEndCI = nullptr;
+  for (auto  : *FinalBB) {
+Instruction *cur = 
+if (isa(cur)) {
+  MasterEndCI = cast(cur);
+  if (MasterEndCI->getCalledFunction()->getName() ==
+  "__kmpc_omp_end_master")
+break;
+  else
+MasterEndCI = nullptr;
+}
+  }
+  EXPECT_NE(MasterEndCI, nullptr);
+  EXPECT_EQ(MasterEndCI->getNumArgOperands(), 2U);
+  EXPECT_TRUE(isa(MasterEndCI->getArgOperand(0)));
+  EXPECT_EQ(MasterEndCI->getArgOperand(1), MasterEntryCI->getArgOperand(1));
+}
+
+TEST_F(OpenMPIRBuilderTest, CriticalDirective) {
+  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());
+
+  BasicBlock *EntryBB = nullptr;
+  BasicBlock *FinalBB = nullptr;
+  BasicBlock *ExitBB = nullptr;
+
+  auto BodyGenCB = [&](InsertPointTy AllocaIP, InsertPointTy CodeGenIP,
+   BasicBlock ) {
+// collect some info for checks later
+FinalBB = 
+ExitBB = FiniBB.getUniqueSuccessor();
+EntryBB = FinalBB->getUniquePredecessor();
+
+// actual start for bodyCB
+llvm::BasicBlock *CodeGenIPBB = CodeGenIP.getBlock();
+llvm::Instruction *CodeGenIPInst = &*CodeGenIP.getPoint();
+EXPECT_EQ(CodeGenIPBB->getTerminator(), CodeGenIPInst);
+EXPECT_EQ(EntryBB, CodeGenIPBB);
+
+// body begin
+Builder.restoreIP(CodeGenIP);
+Builder.CreateStore(F->arg_begin(), PrivAI);
+Value *PrivLoad = Builder.CreateLoad(PrivAI, 

[clang] 1c545f6 - [ms] [X86] Use "P" modifier on all branch-target operands in inline X86 assembly.

[Eric Astor via cfe-commits]

Author: Eric Astor
Date: 2020-01-09T14:55:03-05:00
New Revision: 1c545f6dbcbb3ada2dfef2c6afbc1ca8939135cb

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

LOG: [ms] [X86] Use "P" modifier on all branch-target operands in inline X86 
assembly.

Summary:
Extend D71677 to apply to all branch-target operands, rather than 
special-casing call instructions.

Also add a regression test for llvm.org/PR44272, since this finishes fixing it.

Reviewers: thakis, rnk

Reviewed By: thakis

Subscribers: merge_guards_bot, hiraditya, cfe-commits, llvm-commits

Tags: #clang, #llvm

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

Added: 


Modified: 
clang/test/CodeGen/ms-inline-asm-64.c
llvm/include/llvm/MC/MCInstrDesc.h
llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
llvm/lib/MC/MCParser/AsmParser.cpp
llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
llvm/lib/Target/X86/AsmParser/X86Operand.h
llvm/lib/Target/X86/X86InstrControl.td
llvm/lib/Target/X86/X86InstrInfo.td
llvm/utils/TableGen/InstrInfoEmitter.cpp
llvm/utils/TableGen/X86RecognizableInstr.cpp

Removed: 




diff  --git a/clang/test/CodeGen/ms-inline-asm-64.c 
b/clang/test/CodeGen/ms-inline-asm-64.c
index ce46b8821dee..20e8228a04b6 100644
--- a/clang/test/CodeGen/ms-inline-asm-64.c
+++ b/clang/test/CodeGen/ms-inline-asm-64.c
@@ -58,3 +58,17 @@ int t4() {
 // CHECK-SAME: mov [ebx + $$4], ecx
 // CHECK-SAME: "*m,~{eax},~{ebx},~{dirflag},~{fpsr},~{flags}"(%struct.t3_type* 
%{{.*}})
 }
+
+void bar() {}
+
+void t5() {
+  __asm {
+call bar
+jmp bar
+  }
+  // CHECK: t5
+  // CHECK: call void asm sideeffect inteldialect
+  // CHECK-SAME: call qword ptr ${0:P}
+  // CHECK-SAME: jmp qword ptr ${1:P}
+  // CHECK-SAME: "*m,*m,~{dirflag},~{fpsr},~{flags}"(void (...)* bitcast (void 
()* @bar to void (...)*), void (...)* bitcast (void ()* @bar to void (...)*))
+}

diff  --git a/llvm/include/llvm/MC/MCInstrDesc.h 
b/llvm/include/llvm/MC/MCInstrDesc.h
index 8791ba3ba425..e6af40868c30 100644
--- a/llvm/include/llvm/MC/MCInstrDesc.h
+++ b/llvm/include/llvm/MC/MCInstrDesc.h
@@ -37,7 +37,12 @@ enum OperandConstraint {
 /// These are flags set on operands, but should be considered
 /// private, all access should go through the MCOperandInfo accessors.
 /// See the accessors for a description of what these are.
-enum OperandFlags { LookupPtrRegClass = 0, Predicate, OptionalDef };
+enum OperandFlags {
+  LookupPtrRegClass = 0,
+  Predicate,
+  OptionalDef,
+  BranchTarget
+};
 
 /// Operands are tagged with one of the values of this enum.
 enum OperandType {
@@ -98,6 +103,9 @@ class MCOperandInfo {
   /// Set if this operand is a optional def.
   bool isOptionalDef() const { return Flags & (1 << MCOI::OptionalDef); }
 
+  /// Set if this operand is a branch target.
+  bool isBranchTarget() const { return Flags & (1 << MCOI::BranchTarget); }
+
   bool isGenericType() const {
 return OperandType >= MCOI::OPERAND_FIRST_GENERIC &&
OperandType <= MCOI::OPERAND_LAST_GENERIC;

diff  --git a/llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h 
b/llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
index 3dde0b8474a2..abb95628c2a9 100644
--- a/llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
+++ b/llvm/include/llvm/MC/MCParser/MCParsedAsmOperand.h
@@ -71,10 +71,6 @@ class MCParsedAsmOperand {
   /// variable/label?   Only valid when parsing MS-style inline assembly.
   virtual bool needAddressOf() const { return false; }
 
-  /// isCallOperand - Is this an operand of an inline-assembly call 
instruction?
-  /// Only valid when parsing MS-style inline assembly.
-  virtual bool isCallOperand() const { return false; }
-
   /// isOffsetOfLocal - Do we need to emit code to get the offset of the local
   /// variable, rather than its value?   Only valid when parsing MS-style 
inline
   /// assembly.

diff  --git a/llvm/lib/MC/MCParser/AsmParser.cpp 
b/llvm/lib/MC/MCParser/AsmParser.cpp
index dc8132b627a6..94a44c1f93b1 100644
--- a/llvm/lib/MC/MCParser/AsmParser.cpp
+++ b/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -5845,7 +5845,7 @@ bool AsmParser::parseMSInlineAsm(
 InputDecls.push_back(OpDecl);
 InputDeclsAddressOf.push_back(Operand.needAddressOf());
 InputConstraints.push_back(Constraint.str());
-if (Operand.isCallOperand())
+if (Desc.OpInfo[i - 1].isBranchTarget())
   AsmStrRewrites.emplace_back(AOK_CallInput, Start, SymName.size());
 else
   AsmStrRewrites.emplace_back(AOK_Input, Start, SymName.size());

diff  --git a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp 
b/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
index 27c6a5f91428..69299ae9b00d 100644
--- a/llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp
+++ 

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D70172#1812664 , @yaxunl wrote:

> In D70172#1812631 , @rjmccall wrote:
>
> > Most uses of the destructor do not use the delete operator, though, and 
> > therefore should not trigger the diagnostics in `f` to be emitted.  And 
> > this really doesn't require a fully-realized use graph; you could very 
> > easily track the current use stack when making a later pass over the 
> > entities used.
>
>
> The call graph is not for this specific situation. A call graph is needed 
> because of the transitive nature of the deferred diagnostic message. That is, 
> if any direct or indirect caller is emitted, the diagnostic msg needs to be 
> emitted.


One of the points that Richard and I have been trying to make is that this 
really isn't specifically about *calls*, it's about *uses*.  You only want to 
emit diagnostics associated with an entity if you actually have to emit that 
entity, and whether you emit an entity has nothing to do with what places might 
*call* it, but rather what places *use* it and therefore force it to be 
emitted.  This is fortunate because call graphs are inherently imperfect 
because of indirect calls, but use graphs are totally reliable.  It's also 
fortunate because it means you can piggy-back on all of the existing logic that 
Sema has for tracking ODR uses.

Richard and I are also pointing out that Sema has to treat the v-table as its 
own separate thing when tracking ODR uses, and so you need to as well.  You 
want to emit diagnostics associated with a virtual function if you're emitting 
code that either (1) directly uses the function (e.g. by calling `x->A::foo()`) 
or (2) directly uses a v-table containing the function.  You can't rely on 
Sema's normal ODR-use tracking for *either* of these, because Sema might have 
observed a use in code that you don't actually have to emit, e.g. host code if 
you're compiling for the device.  That is, a v-table is only a "root" for 
virtual functions if you actually have to emit that v-table, and you can't know 
that without tracking v-tables in your use graph.

> The deferred diagnostic msg is recorded when parsing a function body. At that 
> time we do not know which function will directly or indirectly call it. How 
> do we keep a use stack?

The "use stack" idea would apply if you switched from eagerly creating the 
entire use graph to instead just making a late pass that walked function 
bodies.  If you walk function bodies depth-first, starting from a true root and 
gathering all the ODR-used entities to be  recursively walked, then you can 
maintain a stack of what entities you're currently walking, and that stack is a 
use-path that explains why you need to emit the current function.

It should be straightforward to build a function that walks over the entities 
used by a function body and calls a callback by just extracting it out of the 
code in `MarkDeclarationsUsedInExpr`.


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

https://reviews.llvm.org/D70172



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

[PATCH] D69876: Allow output constraints on "asm goto"

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: clang/lib/AST/Stmt.cpp:646-648
+  // Labels are placed after "InOut" operands. Adjust accordingly.
+  if (IsLabel)
+N += getNumPlusOperands();

void wrote:
> jyknight wrote:
> > I'm confused about this part. Why isn't the "N" specified in the assembly 
> > string already the correct value for the labels? Is the ordering we use 
> > internally and that users use externally not the same? I'm assuming your 
> > code here is correct, just I'm not understanding, so probably an improved 
> > comment would be nice.
> The LLVM-specific ordering that I saw was something like:
> 
>   `,,, X>,`
> 
> The `` is empty when there are no output constraints. This 
> just makes up for this. It's probably better to reverse the `` 
> and `` parts, but I didn't know how pervasive the order's 
> assumption is in the code.
Oh, right -- in llvm IR, the "plus" constraints don't exist -- we specify those 
as separate-but-linked output and input constraints.

But thinking about this more...I think this code is actually incorrect. It 
should be translating the numbers based on where it finds the matching 
argument-number, not on the 'l' character being present. (And it should do so 
for named-arguments too).

So, yes, I think it'd be better to just swap the order, and move the 
plus-operand tied-inputs to the end of the arglist, where they don't affect 
numbering. I don't _think_ reversing the order will cause other issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876



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

[PATCH] D69876: Allow output constraints on "asm goto"

[James Y Knight via Phabricator via cfe-commits]

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

Reopening, since this didn't actually land yet. BTW, this review still has the 
wrong contents in the latest uploaded-diff (showing llvm changes, not clang 
changes).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69876



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

[PATCH] D72242: Fix crash on value dependent bitfields in if conditions in templates

[Elizabeth Andrews via Phabricator via cfe-commits]

eandrews updated this revision to Diff 237132.
eandrews edited the summary of this revision.
eandrews added a comment.

Semantic analysis for value dependent conditions is now skipped as per Erich's 
comment.  Patch adds an argument to CheckBooleanCondition to still do the 
required analysis for noexcept expressions.


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

https://reviews.llvm.org/D72242

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaTemplate/value-dependent-bitfield-cond.cpp


Index: clang/test/SemaTemplate/value-dependent-bitfield-cond.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/value-dependent-bitfield-cond.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -ast-dump | FileCheck %s 
--check-prefix=CHECK-AST
+// expected-no-diagnostics
+
+template 
+class A {
+  int c : b;
+
+public:
+  void f() {
+if (c)
+  ;
+  }
+};
+
+void foo() {
+
+  // CHECK-AST: ClassTemplateSpecializationDecl{{.*}}class A definition
+  // CHECK-AST: TemplateArgument integral 3
+  // CHECK-AST: FieldDecl{{.*}}c 'int'
+  // CHECK-AST-NEXT: ConstantExpr{{.*}}'int' Int: 3
+  // CHECK-AST-NEXT: SubstNonTypeTemplateParmExpr{{.*}}'int'
+  // CHECK-AST-NEXT: IntegerLiteral{{.*}}'int' 3
+  A<3> a;
+
+  // CHECK-AST: IfStmt
+  // CHECK-AST-NEXT: ImplicitCastExpr{{.*}}'bool' 
+  // CHECK-AST-NEXT: ImplicitCastExpr{{.*}}'int' 
+  // CHECK-AST-NEXT: MemberExpr{{.*}}'int' lvalue bitfield ->c
+  // CHECK-AST-NEXT: CXXThisExpr{{.*}}'A<3> *' implicit this
+  a.f();
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17362,7 +17362,7 @@
 }
 
 ExprResult Sema::CheckBooleanCondition(SourceLocation Loc, Expr *E,
-   bool IsConstexpr) {
+   bool IsConstexpr, bool IsNoexceptExpr) {
   DiagnoseAssignmentAsCondition(E);
   if (ParenExpr *parenE = dyn_cast(E))
 DiagnoseEqualityWithExtraParens(parenE);
@@ -17372,8 +17372,11 @@
   E = result.get();
 
   if (!E->isTypeDependent()) {
-if (getLangOpts().CPlusPlus)
+if (getLangOpts().CPlusPlus) {
+  if (E->isValueDependent() && !IsNoexceptExpr)
+return E;
   return CheckCXXBooleanCondition(E, IsConstexpr); // C++ 6.4p4
+}
 
 ExprResult ERes = DefaultFunctionArrayLvalueConversion(E);
 if (ERes.isInvalid())
Index: clang/lib/Sema/SemaExceptionSpec.cpp
===
--- clang/lib/Sema/SemaExceptionSpec.cpp
+++ clang/lib/Sema/SemaExceptionSpec.cpp
@@ -80,7 +80,9 @@
Expr *NoexceptExpr,
ExceptionSpecificationType ) {
   // FIXME: This is bogus, a noexcept expression is not a condition.
-  ExprResult Converted = CheckBooleanCondition(NoexceptLoc, NoexceptExpr);
+  ExprResult Converted = CheckBooleanCondition(NoexceptLoc, NoexceptExpr,
+   /*IsConstexpr*/ false,
+   /*IsNoexceptExpr*/ true);
   if (Converted.isInvalid())
 return Converted;
 
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -10885,7 +10885,8 @@
   /// 'if' keyword.
   /// \return true iff there were any errors
   ExprResult CheckBooleanCondition(SourceLocation Loc, Expr *E,
-   bool IsConstexpr = false);
+   bool IsConstexpr = false,
+   bool IsNoexceptExpr = false);
 
   /// ActOnExplicitBoolSpecifier - Build an ExplicitSpecifier from an 
expression
   /// found in an explicit(bool) specifier.


Index: clang/test/SemaTemplate/value-dependent-bitfield-cond.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/value-dependent-bitfield-cond.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -ast-dump | FileCheck %s --check-prefix=CHECK-AST
+// expected-no-diagnostics
+
+template 
+class A {
+  int c : b;
+
+public:
+  void f() {
+if (c)
+  ;
+  }
+};
+
+void foo() {
+
+  // CHECK-AST: ClassTemplateSpecializationDecl{{.*}}class A definition
+  // CHECK-AST: TemplateArgument integral 3
+  // CHECK-AST: FieldDecl{{.*}}c 'int'
+  // CHECK-AST-NEXT: ConstantExpr{{.*}}'int' Int: 3
+  // CHECK-AST-NEXT: SubstNonTypeTemplateParmExpr{{.*}}'int'
+  // CHECK-AST-NEXT: IntegerLiteral{{.*}}'int' 3
+  A<3> a;
+
+  // CHECK-AST: IfStmt
+  // CHECK-AST-NEXT: ImplicitCastExpr{{.*}}'bool' 
+  // CHECK-AST-NEXT: ImplicitCastExpr{{.*}}'int' 
+  // CHECK-AST-NEXT: MemberExpr{{.*}}'int' lvalue bitfield ->c
+  // CHECK-AST-NEXT: 

[PATCH] D71827: [clang] avoid strict aliasing violation in assert

[Ryan Libby via Phabricator via cfe-commits]

rlibby added a comment.

I am no expert here so I will defer, but I believe those suggestions are weaker 
assertions.

I believe it's really trying to assert that the NamedDecl type is the first 
template type in the point union, and is represented by a 0 bit in the addr, 
and that the pointer was appropriately aligned.  So I tried to
choose something that would fail if those were not true.  I think that the 
dyn_cast<> approach will not fail appropriately if the NamedDecl type is not 
first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71827



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

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 237136.
JonasToth added a comment.

- Merge branch 'master' into feature_transform_const.patch
- actually dismiss function references
- work on exclusion of variables that have type, autotype or reference-type to 
template parameter
- by default analyze the pointers and transform as well, for testing
- fix canResolveToExpr to account for derivedToBaseClass casts
- restore unit tests for expr mut analyzer - there was an oversight in my build 
that lead them not being running
- short id for matcher
- fix test for fixed false negative
- rip out unused matchers that are now unnecessary
- try to ignore auto-types that are type-dependent
- consider 'static_cast()' always a mutation, speculativly fix tests for 
that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

Files:
  clang-tools-extra/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-const-correctness.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-cxx17.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/Analysis/ExprMutationAnalyzer.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
  clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Index: clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -62,13 +62,18 @@
   const auto *const S = selectFirst("stmt", Results);
   SmallVector Chain;
   ExprMutationAnalyzer Analyzer(*S, AST->getASTContext());
+
+  std::string buffer;
   for (const auto *E = selectFirst("expr", Results); E != nullptr;) {
 const Stmt *By = Analyzer.findMutation(E);
-std::string buffer;
+if (!By)
+  break;
+
 llvm::raw_string_ostream stream(buffer);
 By->printPretty(stream, nullptr, AST->getASTContext().getPrintingPolicy());
-Chain.push_back(StringRef(stream.str()).trim().str());
+Chain.emplace_back(StringRef(stream.str()).trim().str());
 E = dyn_cast(By);
+buffer.clear();
   }
   return Chain;
 }
@@ -382,22 +387,26 @@
   "void g(const int&&); void f() { int x; g(static_cast(x)); }");
   auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_FALSE(isMutated(Results, AST.get()));
+  EXPECT_THAT(mutatedBy(Results, AST.get()),
+  ElementsAre("static_cast(x)"));
 
   AST = buildASTFromCode("struct A {}; A operator+(const A&&, int);"
  "void f() { A x; static_cast(x) + 1; }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_FALSE(isMutated(Results, AST.get()));
+  EXPECT_THAT(mutatedBy(Results, AST.get()),
+  ElementsAre("static_cast(x)"));
 
   AST = buildASTFromCode("void f() { struct A { A(const int&&); }; "
  "int x; A y(static_cast(x)); }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_FALSE(isMutated(Results, AST.get()));
+  EXPECT_THAT(mutatedBy(Results, AST.get()),
+  ElementsAre("static_cast(x)"));
 
   AST = buildASTFromCode("void f() { struct A { A(); A(const A&&); }; "
  "A x; A y(static_cast(x)); }");
   Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_FALSE(isMutated(Results, AST.get()));
+  EXPECT_THAT(mutatedBy(Results, AST.get()),
+  ElementsAre("static_cast(x)"));
 }
 
 TEST(ExprMutationAnalyzerTest, Move) {
@@ -575,7 +584,8 @@
   "const int&& f() { int x; return static_cast(x); }");
   const auto Results =
   match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
-  EXPECT_FALSE(isMutated(Results, AST.get()));
+  EXPECT_THAT(mutatedBy(Results, AST.get()),
+  ElementsAre("return static_cast(x);"));
 }
 
 TEST(ExprMutationAnalyzerTest, TakeAddress) {
@@ -865,7 +875,8 @@
   buildASTFromCode("void f() { int x; static_cast(x); }");
   const auto Results =
   

[PATCH] D72449: [PATCH] [llvm-ranlib] Take in consideration UTC offset for D-flag.test

[Adhemerval Zanella via Phabricator via cfe-commits]

zatrazz updated this revision to Diff 237134.
zatrazz added a comment.

Updated patch based on previous comments.


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

https://reviews.llvm.org/D72449

Files:
  llvm/test/tools/llvm-ranlib/D-flag.test


Index: llvm/test/tools/llvm-ranlib/D-flag.test
===
--- llvm/test/tools/llvm-ranlib/D-flag.test
+++ llvm/test/tools/llvm-ranlib/D-flag.test
@@ -10,7 +10,7 @@
 
 ## Check that the -D flag clears the timestamps:
 # RUN: cp %t-no-index.a %t.a && llvm-ranlib -D %t.a
-# RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s 
--check-prefix=DETERMINISTIC-VALUES
+# RUN: (env TZ=UTC date -d '@0' +%H:%M; env TZ=UTC llvm-ar tv %t.a) | 
FileCheck %s --check-prefix=DETERMINISTIC-VALUES
 
 ## Check that the -U flag maintains the timestamps:
 # RUN: cp %t-no-index.a %t.a && llvm-ranlib -U %t.a
@@ -20,7 +20,7 @@
 # RUN: cp %t-no-index.a %t.a && llvm-ranlib -UDU %t.a
 # RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s --check-prefix=REAL-VALUES
 # RUN: cp %t-no-index.a %t.a && llvm-ranlib -UUD %t.a
-# RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s 
--check-prefix=DETERMINISTIC-VALUES
+# RUN: (env TZ=UTC date -d '@0' +%H:%M; env TZ=UTC llvm-ar tv %t.a) | 
FileCheck %s --check-prefix=DETERMINISTIC-VALUES
 
 ## Check arguments can be passed before and after the file name
 # RUN: cp %t-no-index.a %t.a && llvm-ranlib -U %t.a -D -U
@@ -41,5 +41,8 @@
 # RUN: not llvm-ranlib -DxD %t.a 2>&1 | FileCheck %s --check-prefix=BAD-OPT-xD
 # BAD-OPT-xD: error: Invalid option: '-xD'
 
-# DETERMINISTIC-VALUES: {{[rwx-]+}} 0/0712 Jan  1 00:00 1970 
D-flag.test.tmp.o
+## llvm-ar uses localtime to print the object timestamp and it is subject to
+## UTC offset (daylight for instance).
+# DETERMINISTIC-VALUES: [[HHMM:[0-9]+:[0-9]+]]
+# DETERMINISTIC-VALUES: {{[rwx-]+}} 0/0712 Jan  1 [[HHMM]] 1970 
D-flag.test.tmp.o
 # REAL-VALUES: {{[rwx-]+}} {{[0-9]+}}/{{[0-9]+}} 712 Jan  2 03:04 2000 
D-flag.test.tmp.o


Index: llvm/test/tools/llvm-ranlib/D-flag.test
===
--- llvm/test/tools/llvm-ranlib/D-flag.test
+++ llvm/test/tools/llvm-ranlib/D-flag.test
@@ -10,7 +10,7 @@
 
 ## Check that the -D flag clears the timestamps:
 # RUN: cp %t-no-index.a %t.a && llvm-ranlib -D %t.a
-# RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s --check-prefix=DETERMINISTIC-VALUES
+# RUN: (env TZ=UTC date -d '@0' +%H:%M; env TZ=UTC llvm-ar tv %t.a) | FileCheck %s --check-prefix=DETERMINISTIC-VALUES
 
 ## Check that the -U flag maintains the timestamps:
 # RUN: cp %t-no-index.a %t.a && llvm-ranlib -U %t.a
@@ -20,7 +20,7 @@
 # RUN: cp %t-no-index.a %t.a && llvm-ranlib -UDU %t.a
 # RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s --check-prefix=REAL-VALUES
 # RUN: cp %t-no-index.a %t.a && llvm-ranlib -UUD %t.a
-# RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s --check-prefix=DETERMINISTIC-VALUES
+# RUN: (env TZ=UTC date -d '@0' +%H:%M; env TZ=UTC llvm-ar tv %t.a) | FileCheck %s --check-prefix=DETERMINISTIC-VALUES
 
 ## Check arguments can be passed before and after the file name
 # RUN: cp %t-no-index.a %t.a && llvm-ranlib -U %t.a -D -U
@@ -41,5 +41,8 @@
 # RUN: not llvm-ranlib -DxD %t.a 2>&1 | FileCheck %s --check-prefix=BAD-OPT-xD
 # BAD-OPT-xD: error: Invalid option: '-xD'
 
-# DETERMINISTIC-VALUES: {{[rwx-]+}} 0/0712 Jan  1 00:00 1970 D-flag.test.tmp.o
+## llvm-ar uses localtime to print the object timestamp and it is subject to
+## UTC offset (daylight for instance).
+# DETERMINISTIC-VALUES: [[HHMM:[0-9]+:[0-9]+]]
+# DETERMINISTIC-VALUES: {{[rwx-]+}} 0/0712 Jan  1 [[HHMM]] 1970 D-flag.test.tmp.o
 # REAL-VALUES: {{[rwx-]+}} {{[0-9]+}}/{{[0-9]+}} 712 Jan  2 03:04 2000 D-flag.test.tmp.o
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D70172#1812631 , @rjmccall wrote:

> Most uses of the destructor do not use the delete operator, though, and 
> therefore should not trigger the diagnostics in `f` to be emitted.  And this 
> really doesn't require a fully-realized use graph; you could very easily 
> track the current use stack when making a later pass over the entities used.


The call graph is not for this specific situation. A call graph is needed 
because of the transitive nature of the deferred diagnostic message. That is, 
if any direct or indirect caller is emitted, the diagnostic msg needs to be 
emitted.

The deferred diagnostic msg is recorded when parsing a function body. At that 
time we do not know which function will directly or indirectly call it. How do 
we keep a use stack?

When we parsing other function bodies, we only know the direct callee. Since we 
do not know if this function indirectly calls the function with deferred 
diagnostics, we have to keep a record of all the caller/callee edges.


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

https://reviews.llvm.org/D70172



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

[PATCH] D72449: [PATCH] [llvm-ranlib] Take in consideration UTC offset for D-flag.test

[Adhemerval Zanella via Phabricator via cfe-commits]

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



Comment at: llvm/test/tools/llvm-ranlib/D-flag.test:13
 # RUN: cp %t-no-index.a %t.a && llvm-ranlib -D %t.a
-# RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s 
--check-prefix=DETERMINISTIC-VALUES
+# RUN: (env TZ=UTC date -d '@0' +%H:%M; env TZ=UTC llvm-ar tv %t.a) | 
FileCheck %s --check-prefix=DETERMINISTIC-VALUES
 

kristof.beyls wrote:
> I wonder if 'date' is available on all platforms that LLVM builds on. For 
> example, is it available on Windows (with GnuWin32 installed, as per the 
> requirements listed at https://llvm.org/docs/GettingStartedVS.html)? 
> http://gnuwin32.sourceforge.net/packages.html doesn't seem to list "date" 
> explicitly?
It is provided by coreutils 
(http://gnuwin32.sourceforge.net/packages/coreutils.htm).



Comment at: llvm/test/tools/llvm-ranlib/D-flag.test:23
 # RUN: cp %t-no-index.a %t.a && llvm-ranlib -UUD %t.a
 # RUN: env TZ=UTC llvm-ar tv %t.a | FileCheck %s 
--check-prefix=DETERMINISTIC-VALUES
 

arichardson wrote:
> Wouldn't this line also have to change?
It does and I updated the patch.


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

https://reviews.llvm.org/D72449



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

Re: [clang] 24ab9b5 - Generalize the pass registration mechanism used by Polly to any third-party tool

[Michael Kruse via cfe-commits]

Am Mi., 8. Jan. 2020 um 17:20 Uhr schrieb Eric Christopher :
>
>
>
> On Wed, Jan 8, 2020 at 3:07 PM Michael Kruse  
> wrote:
>>
>> Am Di., 7. Jan. 2020 um 18:57 Uhr schrieb Eric Christopher via
>> cfe-commits :
>> >> Is there anything I should have done? Probably reaching llvm-dev before
>> >> commiting. Reaching the right reviewers has always been a challenge to 
>> >> me, I had
>> >> hoped that the mail to llvm-dev would trigger some subscription :-)
>> >
>> >
>> > Hrm. Probably finding some different reviewers, but I can't fault your 
>> > attempts here. Usually you can look at the last few people to make 
>> > substantial work in an area and loop them in via git log. :)
>>
>> In my experience adding more reviewers usually rarely to more people
>> looking into the patch. Especially the people that make substantial
>> work in an area and/or are code owners are added as reviewers to very
>> many commits and cannot look into all of them.
>
>
> As one of those added to a lot of patches I have a lot of sympathy here. The 
> answer isn't to avoid adding people with expertise in an area and no one on 
> the approval list had expertise or had recently hacked on the code.

For some parts you won't find people that recently worked on the code
other than refactoring, such as the legacy pass manager. Together with
Tobias Grosser, I worked out the mechanism that statically linked
Polly into tools which was generalized in this patch. I am not sure
what else would be required to approve this patch.


>
>>
>> Note that in the patch in question, there are comments from people not
>> in the reviewer list. For example, Mehdi with whom I also talked about
>> this patch at the last LLVM DevMtg.
>>
>
> Yes, but none of them ack'd it.

Mehdi did in the comments (and personally at the DevMtg). He just did
not approve the patch because I still had concerns. By coding policy,
only one approval is required.



>> >> >  ; CHECK-EP-VECTORIZER-START-NEXT: Running pass: NoOpFunctionPass
>> >> > +; CHECK-EXT: Running pass: {{.*}}::Bye on foo
>> >> >
>> >> > Why is this running on every test of the pass manager? It should be an 
>> >> > example
>> >> > run in the examples directory and not on by default? Same for every 
>> >> > other PM
>> >> > test. This seems like a bug?
>> >>
>> >> It's not. When the examples are active and if the appropriate cmake flag 
>> >> is set
>> >> (which is not the case by default), the pass is linked in statically, and 
>> >> is run
>> >> in the default pipeline. The CHECK-EXT prefix is disabled otherwise. 
>> >> That's one
>> >> of the configuration I did test :-)
>> >>
>> >
>> > I really don't think this is ideal. The examples directory shouldn't 
>> > affect tests being run or not or in what way. Can we back this part out 
>> > and talk about it a bit more? I don't think we should need to do this to 
>> > test the functionality.
>>
>> In a previous discussion [1] we discussed adding tutorial code to
>> repository which naturally should also be tested to avoid bit-rot. How
>> do you suggest to testing of examples should work? Note that LLVMHello
>> is also tested, but not in the example directory.
>
>
> llvm/tests/examples?

The point is, we don't have an established pattern of how to test
examples yet, and I would also like that we develop one. I also think
this was not in the scope of this patch.


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

[PATCH] D71827: [clang] avoid strict aliasing violation in assert

[Dimitry Andric via Phabricator via cfe-commits]

dim added inline comments.



Comment at: clang/include/clang/AST/DeclContextInternals.h:102
 // at getLookupResult.
-assert(*(NamedDecl **) == ND &&
+assert(Data.getAddrOfPtr1() && *Data.getAddrOfPtr1() == ND &&
"PointerUnion mangles the NamedDecl pointer!");

Hm, I don't have much experience with the `PointerUnion` class, but from a 
cursory look at `llvm/include/llvm/ADT/PointerUnion.h`, I would rather say that 
using `dyn_cast` here would be more appropriate?  E.g. that definition 
literally says:

```
  /// Returns the current pointer if it is of the specified pointer type,
  /// otherwises returns null.
```

So something like:

```
  assert(Data.dyn_cast == ND &&
"PointerUnion mangles the NamedDecl pointer!");
```

or even using the existing `getAsDecl` member function, which does exactly the 
same:

```
  assert(Data.getAsDecl() == ND &&
"PointerUnion mangles the NamedDecl pointer!");
```

Or is this particular check about the type being *only* the base class 
`NamedDecl`, and not any derived class?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71827



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

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D70172#1810665 , @rsmith wrote:

> This doesn't look quite right to me. I don't think we should treat the 
> `delete this;` for a destructor as being emitted-for-device in any 
> translation unit in which the vtable is marked used. (For example, if in your 
> testcase `MSEmitDeletingDtor::CFileStream::CFileStream()` were a `__host__` 
> function, I think you'd still diagnose, but presumably shouldn't do so, 
> because the vtable -- and therefore `CFileStream::operator delete` -- is 
> never referenced / emitted for the device.) Instead, I think we should treat 
> the `delete this;` as being emitted in any translation unit in which the 
> vtable itself is emitted-for-device. Presumably, this means you will need to 
> model / track usage of the vtable itself in your "call graph".


A user declared ctor/dtor by default is `__host__`.

Let's consider this testcase:

  static __device__ __host__ void f(__m256i *p) {
__asm__ volatile("vmovaps  %0, %%ymm0" ::"m"(*(__m256i *)p)
   : "r0"); // MS-error{{unknown register name 'r0' in asm}}
  }
  struct CFileStream {
void operator delete(void *p) {
  f(0);  // MS-note{{called by 'operator delete'}}
}
CFileStream();
virtual ~CFileStream();  // MS-note{{called by '~CFileStream'}}
  };
  
  struct CMultiFileStream {
CFileStream m_fileStream;
~CMultiFileStream();
  };
  
  // This causes vtable emitted so that deleting dtor is emitted for MS.
  CFileStream::CFileStream() {}

In host compilation, vtbl is emitted, since it causes dtor emitted, whereas 
dtor calls f(), therefore the diagnostic msg is emitted.

In device compilation, vtbl is not emitted, therefore dtor is not emitted, and 
the diagnostic msg in f() is not emitted.

We only need an entity in call graph if that entity can be called by other 
entities. Here vtbl is always at the top level of the 'call graph'. Therefore 
it is not needed to be in the call graph.


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

https://reviews.llvm.org/D70172



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

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D70172#1812533 , @yaxunl wrote:

> In D70172#1809571 , @rjmccall wrote:
>
> > I thought you were saying that the destructor decl hadn't been created yet, 
> > but I see now that you're saying something more subtle.
> >
> > `CurContext` is set to the destructor because the standard says in 
> > [class.dtor]p13:
> >
> >   At the point of definition of a virtual destructor (including an implicit 
> > definition), the non-array deallocation function is determined as if for 
> > the expression `delete this` appearing in a non-virtual destructor of the 
> > destructor’s class.
> >   
> >
> > Which is to say that, semantically, the context is as if it were within the 
> > destructor, to the extent that this affects access control and so on.
> >
> > I can see why this causes problems for your call graph (really a use 
> > graph), since it's a use in the apparent context of the destructor at a 
> > point where the destructor is not being defined.  A similar thing happens 
> > with default arguments, but because we don't consider uses from default 
> > arguments to be true ODR-uses until the default argument is used, that 
> > probably doesn't cause problems for you.
> >
> > I don't think the destructor -> deallocation function edge is actually 
> > interesting for your use graph.  It'd be more appropriate to treat the 
> > deallocation function as used by the v-table than by the destructor; I 
> > don't know whether you make any attempt to model v-tables as nodes in your 
> > use graph.  You might consider finding a simple way to suppress adding this 
> > edge, like just not adding edges from a destructor that's not currently 
> > being defined (`D->willHaveBody()`).
> >
> > With all that said, maintaining a use graph for all the functions you might 
> > emit in the entire translation unit seems very expensive and brittle.  Have 
> > you considered doing this walk in a final pass?   You could just build up a 
> > set of all the functions you know you're going to emit and then walk their 
> > bodies looking for uses of lazy-emitted entities.  If we don't already have 
> > a function that calls a callback for every declaration ODR-used by a 
> > function body, we should.
>
>
> The deferred diagnostic mechanism is shared between CUDA/HIP and OpenMP. The 
> diagnostic messages not only depend on the callee, but also depend on the 
> caller, the caller information needs to be kept. Also if a caller is to be 
> emitted, all the deferred diagnostics associated with the direct or indirect 
> callees need to be emitted. Therefore a call graph is needed for this 
> mechanism.
>
> If we ignore the dtor->deallocation edge in the call graph, we may miss 
> diagnostics, e.g.
>
>   static __device__ __host__ void f(__m256i *p) {
> __asm__ volatile("vmovaps  %0, %%ymm0" ::"m"(*(__m256i *)p)
>: "r0"); // MS-error{{unknown register name 'r0' in asm}}
>   }
>   struct CFileStream {
> void operator delete(void *p) {
>   f(0);  // MS-note{{called by 'operator delete'}}
> }
> CFileStream();
> virtual ~CFileStream();  // MS-note{{called by '~CFileStream'}}
>   };
>   
>   struct CMultiFileStream {
> CFileStream m_fileStream;
> ~CMultiFileStream();
>   };
>   
>   // This causes vtable emitted so that deleting dtor is emitted for MS.
>   CFileStream::CFileStream() {}
>   
>
> Assuming the host compilation is on windows.
>
> Here f() is a host device function which is unknown to be emitted, therefore 
> the inline assembly error results in a delayed diagnostic. When f() is 
> checked in the delete operator body, a 'delete operator -> f' edge is added 
> to the call graph since f() is unknown to be emitted.
>
> Since CFileStream::CFileStream is defined, clang sets vtbl to be emitted and 
> does an explicit dtor check even though dtor is not defined. clang knows that 
> this dtor check is for deleting dtor and will check delete operator as 
> referenced, which causes `dtor -> delete operator' to be added to the call 
> graph. Then clang checks dtor as referenced. Since deleting dtor will be 
> emitted together with vtbl, clang should assume dtor is to be emitted. Then 
> clang will found the callees 'delete operator' and f(), and emits the delayed 
> diagnostics associated with them.
>
> If we do not add 'dtor -> delete operator' edge to the call graph, the 
> diagnostic msg in f() will not be emitted.


Most uses of the destructor do not use the delete operator, though, and 
therefore should not trigger the diagnostics in `f` to be emitted.  And this 
really doesn't require a fully-realized use graph; you could very easily track 
the current use stack when making a later pass over the entities used.

Also I agree with Richard that you really need the v-table to be a node in your 
use graph/stack.


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

[PATCH] D72378: [clang-tidy] Add `bugprone-reserved-identifier`

[Logan Smith via Phabricator via cfe-commits]

logan-5 marked 2 inline comments as done.
logan-5 added a comment.

In D72378#1810763 , @aaron.ballman 
wrote:

> This check is missing a whole lot of reserved identifiers. For instance, in 
> C++ it is missing everything from http://eel.is/c++draft/zombie.names and for 
> C it is missing everything from future library directions. Are you intending 
> to cover those cases as well?


I admit that those are outside the scope of what I had originally planned for 
this check -- I was primarily concerned about 'uglified' names, and writing a 
check that was 'invertible' in that regard. Now that you mention these, though, 
I do feel like this check doesn't live up to its name without including them.

I'd be interested in incorporating them. It doesn't sound difficult, but it 
does sound like it'd be a sizable addition to this diff. Still familiarizing 
with the workflow around here... would it be alright to leave a TODO comment in 
this patch and add them in an upcoming patch, to keep this one more 
self-contained?




Comment at: 
clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp:48-51
+  if (hasDoubleUnderscore(Name)) {
+Name.consume_front("__");
+return collapseConsecutive(Name, '_');
+  }

aaron.ballman wrote:
> Can't this still result in a reserved identifier? e.g.,
> ```
> int ___Foobar; // three underscores
> ```
`___Foobar` with 3 underscores will be fixed to `_Foobar` by this fixup, which 
is then passed through the underscore capital fixup, and that will be caught 
there. So it still works. 

Thinking about it more, though, I do think the `consume_front` is unnecessary. 
Without it, `__foo` would get changed (by this fixup) to `_foo`, which will be 
corrected by a later fixup if that's still invalid. If not, that's a smaller 
and less opinionated change to the name than the current `__foo` -> `foo`.

I think I'll take out the `consume_front("__")` and update the tests to match.



Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h:21
+
+/// Checks for usages of identifiers reserved for use by the C++ 
implementation.
+/// The C++ standard reserves the following names for such use:

aaron.ballman wrote:
> Why just C++? C has reserved identifiers as well, and there's a lot of 
> overlap between the two languages in terms of what's reserved.
That's a good point. I'll do some tweaking to make sure this works well for C, 
including any places where C and C++ differ.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72378



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

[PATCH] D72282: [clang-tidy] Add `bugprone-unintended-adl`

[Logan Smith via Phabricator via cfe-commits]

logan-5 marked an inline comment as done.
logan-5 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-unintended-adl.cpp:61
+void templateFunction(T t) {
+  swap(t, t);
+

Quuxplusone wrote:
> logan-5 wrote:
> > Quuxplusone wrote:
> > > This is not the idiomatic way of calling `swap`: there is no ADL swap for 
> > > `int`, for example (so `templateFunction` will hard-error during 
> > > instantiation). It would probably be scope-creep to try to handle the 
> > > "std::swap two-step", but can you leave a TODO comment somewhere to 
> > > revisit this issue?
> > > 
> > I believe this addressed by my juggling the tests around a bit.
> Juggling the tests around doesn't address the fact that any code that does 
> `swap(a,b)` without doing `using std::swap;` first (or `begin(a)` without 
> `using std::begin;`) is almost certainly broken for primitive types.
> 
> My naive thought is that you would //not// do `using std::make_error_code;` 
> because `make_error_code` is definitely never going to be used with primitive 
> types. So "functions okay to call via ADL" and "functions that require the 
> std::swap two-step" actually are slightly different whitelists.
> 
> I was saying that although this issue is probably out-of-scope for what 
> you're doing in this patch, still, it would be nice to leave a TODO 
> somewhere. ...Or maybe you say "nah, that's so far out of scope I don't want 
> to think about it, and it may never get done, so even leaving a TODO is 
> inappropriate."
Thanks for clarifying. I do think that is out of scope for my goals for this 
patch, but I think a TODO comment is reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72282



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

[PATCH] D72448: [clang-tidy] readability-redundant-string-init now flags redundant initialisation in Field Decls and Constructor Initialisers

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 237127.
njames93 added a comment.

updated release notes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72448

Files:
  clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
@@ -227,3 +227,43 @@
   other::wstring e = L"";
   other::wstring f(L"");
 }
+
+class Foo {
+  std::string A = "";
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+  // CHECK-FIXES:  std::string A;
+  std::string B;
+  std::string C;
+  std::string D;
+  std::string E = "NotEmpty";
+
+public:
+  // Check redundant constructor where Field has a redundant initializer.
+  Foo() : A("") {}
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: redundant string initialization
+  // CHECK-FIXES:  Foo()  {}
+
+  // Check redundant constructor where Field has no initializer.
+  Foo(char) : B("") {}
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+  // CHECK-FIXES:  Foo(char)  {}
+
+  // Check redundant constructor where Field has a valid initializer.
+  Foo(long) : E("") {}
+  // CHECK-MESSAGES: [[@LINE-1]]:15: warning: redundant string initialization
+  // CHECK-FIXES:  Foo(long) : E() {}
+
+  // Check how it handles removing 1 initializer, and defaulting the other.
+  Foo(int) : B(""), E("") {}
+  // CHECK-MESSAGES: [[@LINE-1]]:14: warning: redundant string initialization
+  // CHECK-MESSAGES: [[@LINE-2]]:21: warning: redundant string initialization
+  // CHECK-FIXES:  Foo(int) :  E() {}
+
+  // Check how it handles removing some redundant initializers while leaving
+  // valid initializers intact.
+  Foo(std::string Arg) : A(Arg), B(""), C("NonEmpty"), D(R"()"), E("") {}
+  // CHECK-MESSAGES: [[@LINE-1]]:34: warning: redundant string initialization
+  // CHECK-MESSAGES: [[@LINE-2]]:56: warning: redundant string initialization
+  // CHECK-MESSAGES: [[@LINE-3]]:66: warning: redundant string initialization
+  // CHECK-FIXES:  Foo(std::string Arg) : A(Arg),  C("NonEmpty"),  E() {}
+};
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -218,9 +218,11 @@
   `"base class 'Foo' should be explicitly initialized in the copy constructor"`
   warnings or errors with ``gcc -Wextra`` or ``gcc -Werror=extra``.
 
-- The :doc:`readability-redundant-string-init
+- Improved :doc:`readability-redundant-string-init
   ` check now supports a
-  `StringNames` option enabling its application to custom string classes.
+  `StringNames` option enabling its application to custom string classes. The 
+  check now detects in class initializers and constructor initializers which 
+  are deemed to be redundant.
 
 - Improved :doc:`modernize-avoid-bind
   ` check.
Index: clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
@@ -20,6 +20,30 @@
 
 const char DefaultStringNames[] = "::std::basic_string";
 
+static const CXXConstructExpr *
+getConstructExpr(const CXXCtorInitializer ) {
+  const Expr *InitExpr = CtorInit.getInit();
+  if (const auto *CleanUpExpr = dyn_cast(InitExpr))
+InitExpr = CleanUpExpr->getSubExpr();
+  return dyn_cast(InitExpr);
+}
+
+static llvm::Optional
+getConstructExprArgRange(const CXXConstructExpr ) {
+  SourceLocation B, E;
+  for (const Expr *Arg : Construct.arguments()) {
+if (B.isInvalid()) {
+  B = Arg->getBeginLoc();
+}
+if (Arg->getEndLoc().isValid()) {
+  E = Arg->getEndLoc();
+}
+  }
+  if (B.isInvalid() || E.isInvalid())
+return llvm::None;
+  return SourceRange(B, E);
+}
+
 RedundantStringInitCheck::RedundantStringInitCheck(StringRef Name,
ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
@@ -65,29 +89,77 @@
   cxxConstructExpr(StringConstructorExpr,
hasArgument(0, ignoringImplicit(EmptyStringCtorExpr)));
 
+  const auto StringType = hasType(hasUnqualifiedDesugaredType(
+  recordType(hasDeclaration(cxxRecordDecl(hasStringTypeName);
+  const auto EmptyStringInit = expr(ignoringImplicit(
+  anyOf(EmptyStringCtorExpr, EmptyStringCtorExprWithTemporaries)));
+
   // Match a variable 

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D70172#1809571 , @rjmccall wrote:

> I thought you were saying that the destructor decl hadn't been created yet, 
> but I see now that you're saying something more subtle.
>
> `CurContext` is set to the destructor because the standard says in 
> [class.dtor]p13:
>
>   At the point of definition of a virtual destructor (including an implicit 
> definition), the non-array deallocation function is determined as if for the 
> expression `delete this` appearing in a non-virtual destructor of the 
> destructor’s class.
>   
>
> Which is to say that, semantically, the context is as if it were within the 
> destructor, to the extent that this affects access control and so on.
>
> I can see why this causes problems for your call graph (really a use graph), 
> since it's a use in the apparent context of the destructor at a point where 
> the destructor is not being defined.  A similar thing happens with default 
> arguments, but because we don't consider uses from default arguments to be 
> true ODR-uses until the default argument is used, that probably doesn't cause 
> problems for you.
>
> I don't think the destructor -> deallocation function edge is actually 
> interesting for your use graph.  It'd be more appropriate to treat the 
> deallocation function as used by the v-table than by the destructor; I don't 
> know whether you make any attempt to model v-tables as nodes in your use 
> graph.  You might consider finding a simple way to suppress adding this edge, 
> like just not adding edges from a destructor that's not currently being 
> defined (`D->willHaveBody()`).
>
> With all that said, maintaining a use graph for all the functions you might 
> emit in the entire translation unit seems very expensive and brittle.  Have 
> you considered doing this walk in a final pass?   You could just build up a 
> set of all the functions you know you're going to emit and then walk their 
> bodies looking for uses of lazy-emitted entities.  If we don't already have a 
> function that calls a callback for every declaration ODR-used by a function 
> body, we should.


The deferred diagnostic mechanism is shared between CUDA/HIP and OpenMP. The 
diagnostic messages not only depend on the callee, but also depend on the 
caller, the caller information needs to be kept. Also if a caller is to be 
emitted, all the deferred diagnostics associated with the direct or indirect 
callees need to be emitted. Therefore a call graph is needed for this mechanism.

If we ignore the dtor->deallocation edge in the call graph, we may miss 
diagnostics, e.g.

  static __device__ __host__ void f(__m256i *p) {
__asm__ volatile("vmovaps  %0, %%ymm0" ::"m"(*(__m256i *)p)
   : "r0"); // MS-error{{unknown register name 'r0' in asm}}
  }
  struct CFileStream {
void operator delete(void *p) {
  f(0);  // MS-note{{called by 'operator delete'}}
}
CFileStream();
virtual ~CFileStream();  // MS-note{{called by '~CFileStream'}}
  };
  
  struct CMultiFileStream {
CFileStream m_fileStream;
~CMultiFileStream();
  };
  
  // This causes vtable emitted so that deleting dtor is emitted for MS.
  CFileStream::CFileStream() {}

Assuming the host compilation is on windows.

Here f() is a host device function which is unknown to be emitted, therefore 
the inline assembly error results in a delayed diagnostic. When f() is checked 
in the delete operator body, a 'delete operator -> f' edge is added to the call 
graph since f() is unknown to be emitted.

Since CFileStream::CFileStream is defined, clang sets vtbl to be emitted and 
does an explicit dtor check even though dtor is not defined. clang knows that 
this dtor check is for deleting dtor and will check delete operator as 
referenced, which causes `dtor -> delete operator' to be added to the call 
graph. Then clang checks dtor as referenced. Since deleting dtor will be 
emitted together with vtbl, clang should assume dtor is to be emitted. Then 
clang will found the callees 'delete operator' and f(), and emits the delayed 
diagnostics associated with them.

If we do not add 'dtor -> delete operator' edge to the call graph, the 
diagnostic msg in f() will not be emitted.


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

https://reviews.llvm.org/D70172



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

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61683 tests passed, 0 failed 
and 779 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72463



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

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 237122.
yaxunl added a comment.

Add tests for device compilation.

Add a test when both vtbl and deleting dtor are emitted with diagnostic due to 
delete operator.


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

https://reviews.llvm.org/D70172

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/SemaCUDA/deleting-dtor.cu

Index: clang/test/SemaCUDA/deleting-dtor.cu
===
--- /dev/null
+++ clang/test/SemaCUDA/deleting-dtor.cu
@@ -0,0 +1,119 @@
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fsyntax-only -verify=MS -verify=GEN %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux -fsyntax-only -verify=GEN %s
+// RUN: %clang_cc1 -triple nvptx -fcuda-is-device -fsyntax-only -verify=DEV %s
+
+// DEV-no-diagnostics
+#include "Inputs/cuda.h"
+typedef long long __m256i __attribute__((__vector_size__(32)));
+
+namespace NoDiag {
+  struct CFileStream {
+CFileStream();
+virtual ~CFileStream();
+  };
+
+  struct CMultiFileStream {
+CFileStream m_fileStream;
+~CMultiFileStream();
+  };
+
+  CFileStream::CFileStream() {}
+  CFileStream::~CFileStream() {}
+  CMultiFileStream::~CMultiFileStream() {}
+}
+
+// No diagnostic since deleting dtor is not emitted.
+namespace NoVtbl {
+// Intentionally generates delayed diagnostic about r0.
+// This diagnostic is not supposed to be emitted unless f is emitted.
+  static __device__ __host__ void f(__m256i *p) {
+__asm__ volatile("vmovaps  %0, %%ymm0" ::"m"(*(__m256i *)p)
+ : "r0");
+  }
+  struct CFileStream {
+void operator delete(void *p) {
+  f(0);
+}
+CFileStream();
+virtual ~CFileStream();
+  };
+
+  struct CMultiFileStream {
+CFileStream m_fileStream;
+~CMultiFileStream();
+  };
+}
+
+// Only MS has diagnostic since MS requires deleting dtor to be emitted when
+// vtable is emitted, even though dtor is not defined.
+namespace MSEmitDeletingDtor {
+  static __device__ __host__ void f(__m256i *p) {
+__asm__ volatile("vmovaps  %0, %%ymm0" ::"m"(*(__m256i *)p)
+   : "r0"); // MS-error{{unknown register name 'r0' in asm}}
+  }
+  struct CFileStream {
+void operator delete(void *p) {
+  f(0);  // MS-note{{called by 'operator delete'}}
+}
+CFileStream();
+virtual ~CFileStream();  // MS-note{{called by '~CFileStream'}}
+  };
+
+  struct CMultiFileStream {
+CFileStream m_fileStream;
+~CMultiFileStream();
+  };
+
+  // This causes vtable emitted so that deleting dtor is emitted for MS.
+  CFileStream::CFileStream() {}
+}
+
+// Both MS and Linux host compilation has diagnostic since deleting dtor is
+// emitted.
+namespace EmitDeletingDtor {
+  static __device__ __host__ void f(__m256i *p) {
+__asm__ volatile("vmovaps  %0, %%ymm0" ::"m"(*(__m256i *)p)
+   : "r0"); // GEN-error{{unknown register name 'r0' in asm}}
+  }
+  struct CFileStream {
+void operator delete(void *p) {
+  f(0);  // GEN-note{{called by 'operator delete'}}
+}
+CFileStream();
+virtual ~CFileStream();
+  };
+
+  struct CMultiFileStream {
+CFileStream m_fileStream;
+~CMultiFileStream();
+  };
+
+  CFileStream::~CFileStream() {} // GEN-note{{called by '~CFileStream'}}
+  CMultiFileStream::~CMultiFileStream() {}
+  // This causes vtable emitted so that deleting dtor is emitted for MS.
+  CFileStream::CFileStream() {}
+}
+
+namespace EmitDtor {
+  static __device__ __host__ void f(__m256i *p) {
+__asm__ volatile("vmovaps  %0, %%ymm0" ::"m"(*(__m256i *)p)
+ : "r0"); // GEN-error{{unknown register name 'r0' in asm}}
+  }
+  struct CFileStream {
+void operator delete(void *p) {
+  f(0);
+}
+CFileStream();
+virtual ~CFileStream();
+  };
+
+  struct CMultiFileStream {
+CFileStream m_fileStream;
+~CMultiFileStream();
+  };
+
+  CFileStream::~CFileStream() {
+f(0); // GEN-note{{called by '~CFileStream'}}
+  }
+}
+
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -17772,6 +17772,17 @@
   if (FD->isDependentContext())
 return FunctionEmissionStatus::TemplateDiscarded;
 
+  // The Microsoft ABI requires that we perform the destructor body
+  // checks (i.e. operator delete() lookup) when the vtable is marked used, as
+  // the deleting destructor is emitted with the vtable. Such check may happen
+  // even though the destructor is not defined yet.
+  if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+if (auto *DD = dyn_cast(FD)) {
+  if (DD == CurContext && !DD->getDefinition())
+return FunctionEmissionStatus::Emitted;
+}
+  }
+
   FunctionEmissionStatus OMPES = FunctionEmissionStatus::Unknown;
   if (LangOpts.OpenMPIsDevice) {
 Optional DevTy =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D72448: [clang-tidy] readability-redundant-string-init now flags redundant initialisation in Field Decls and Constructor Initialisers

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

It'll be reasonable to mention changes in Release Notes (in checks changes 
section, in alphabetical order).


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

https://reviews.llvm.org/D72448



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

[PATCH] D72427: [DebugInfo] Add option to clang to limit debug info that is emitted for classes.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

What's the plan for this? Is it still in an experimental stage, with the intent 
to investigate the types that are no longer emitted unedr the flag & explain 
why they're missing (& either have a justification for why that's acceptable, 
or work on additional heuristics to address the gaps?)?

If so, I'd probably rather this not be a full driver flag - if it's a reliable 
way to reduce debug info size (like the existing heuristics under 
-fstandalone-debug*) it should be rolled into -fno-standalone-debug behavior, 
and if it's not fully fleshed out yet, I think an -Xclang flag would be more 
suitable for the experimental phase.

Do you have any sample of data on the sort of situations that lead to missing 
types under this heuristic?

- that's also another point, the -flimit-debug-info is sort of the "old" name, 
there was a bit of a lengthy discussion about what to support and how to 
support it and eventually settled on the "-fstandalone-debug" flag name to 
represent the design goal here: debug info emitted under the assumption that no 
other files have debug info, or that all other files have debug info. So by 
that approach, this new behavior seems to fit under the "-fno-standalone-debug" 
behavior, without a new flag, if it's a reliable heuristic - but I understand 
it may be helpful to iterate in-tree on the specific edge cases, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72427



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

[PATCH] D72227: Add options for clang to align branches within 32B boundary

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

Created D72463  
(`-mbranches-within-32B-boundaries` can be overridden by `-malign-branch*`; 
added test coverage)


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

https://reviews.llvm.org/D72227



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

[PATCH] D72463: [Driver][X86] Add -malign-branch* and -malign-branch-within-32B-boundaries

[Fangrui Song via Phabricator via cfe-commits]

MaskRay created this revision.
MaskRay added reviewers: skan, craig.topper, LuoYuanke, annita.zhang, jyknight, 
reames, chandlerc, tstellar.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Based on D72227 .

Add -mbranches-within-32B-boundaries to align branches within a 32-Byte
boundary to reduce the potential performance loss of the microcode
update. It is equivalent to the combination of three options:

-malign-branch=fused+jcc+jmp
-malign-branch-boundary=32
-malign-branch-prefix-size=5

Fine-grained -malign-branch* can override individual values.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72463

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/x86-malign-branch.c
  clang/test/Driver/x86-malign-branch.s

Index: clang/test/Driver/x86-malign-branch.s
===
--- /dev/null
+++ clang/test/Driver/x86-malign-branch.s
@@ -0,0 +1,13 @@
+/// Test that -malign-branch* are handled for assembly files.
+
+// RUN: %clang -target x86_64 -malign-branch=fused+jcc+jmp %s -c -### %s 2>&1 | FileCheck %s --check-prefix=TYPE
+// TYPE: "-mllvm" "-x86-align-branch=fused+jcc+jmp"
+
+// RUN: %clang -target x86_64 -malign-branch=jcc -malign-branch-boundary=16 %s -c -### 2>&1 | FileCheck %s --check-prefix=BOUNDARY
+// BOUNDARY: "-mllvm" "-x86-align-branch-boundary=16"
+
+// RUN: %clang -target x86_64 -malign-branch=jcc -malign-branch-prefix-size=5 %s -c -### 2>&1 | FileCheck %s --check-prefix=PREFIX
+// PREFIX: "-mllvm" "-x86-align-branch-prefix-size=5"
+
+// RUN: %clang -target x86_64 -mbranches-within-32B-boundaries %s -c -### 2>&1 | FileCheck %s --check-prefix=32B
+// 32B: "-mllvm" "-x86-align-branch=fused+jcc+jmp" "-mllvm" "-x86-align-branch-boundary=32" "-mllvm" "-x86-align-branch-prefix-size=5"
Index: clang/test/Driver/x86-malign-branch.c
===
--- /dev/null
+++ clang/test/Driver/x86-malign-branch.c
@@ -0,0 +1,44 @@
+/// Test that -malign-branch* options are parsed and converted to -mllvm options.
+
+/// Test -malign-branch=
+// RUN: %clang -target x86_64 -malign-branch=fused+jcc+jmp %s -c -### %s 2>&1 | FileCheck %s --check-prefix=TYPE0
+// TYPE0: "-mllvm" "-x86-align-branch=fused+jcc+jmp"
+// RUN: %clang -target x86_64 -malign-branch=fused+jcc+jmp+ret+call+indirect %s -c -### %s 2>&1 | FileCheck %s --check-prefix=TYPE1
+// TYPE1: "-mllvm" "-x86-align-branch=fused+jcc+jmp+ret+call+indirect"
+
+// RUN: %clang -target x86_64 -malign-branch=fused+foo+bar %s -c -### %s 2>&1 | FileCheck %s --check-prefix=TYPE-ERR
+// TYPE-ERR: invalid argument 'foo' to -malign-branch=; must be one of: fused, jcc, jmp, call, ret, indirect
+// TYPE-ERR: invalid argument 'bar' to -malign-branch=; must be one of: fused, jcc, jmp, call, ret, indirect
+
+/// Test -malign-branch-boundary=
+// RUN: %clang -target x86_64 -malign-branch=jcc -malign-branch-boundary=16 %s -c -### 2>&1 | FileCheck %s --check-prefix=BOUNDARY
+// BOUNDARY: "-mllvm" "-x86-align-branch-boundary=16"
+
+// RUN: %clang -target x86_64 -malign-branch-boundary=8 %s -c -### 2>&1 | FileCheck %s --check-prefix=BOUNDARY-ERR
+// RUN: %clang -target x86_64 -malign-branch-boundary=15 %s -c -### 2>&1 | FileCheck %s --check-prefix=BOUNDARY-ERR
+// BOUNDARY-ERR: invalid argument {{.*}} to -malign-branch-boundary=
+
+/// Test -malign-branch-prefix-size=
+// RUN: %clang -target x86_64 -malign-branch=jcc -malign-branch-prefix-size=0 %s -c -### 2>&1 | FileCheck %s --check-prefix=PREFIX-0
+// PREFIX-0: "-mllvm" "-x86-align-branch-prefix-size=0"
+// RUN: %clang -target x86_64 -malign-branch=jcc -malign-branch-prefix-size=5 %s -c -### 2>&1 | FileCheck %s --check-prefix=PREFIX-5
+// PREFIX-5: "-mllvm" "-x86-align-branch-prefix-size=5"
+
+// RUN: %clang -target x86_64 -malign-branch-prefix-size=6 %s -c -### 2>&1 | FileCheck %s --check-prefix=PREFIX-6
+// PREFIX-6: invalid argument
+
+/// Test -mbranches-within-32B-boundaries
+// RUN: %clang -target x86_64 -mno-branches-within-32B-boundaries -mbranches-within-32B-boundaries %s -c -### 2>&1 | FileCheck %s --check-prefix=32B
+// 32B:  "-mllvm" "-x86-align-branch=fused+jcc+jmp" "-mllvm" "-x86-align-branch-boundary=32" "-mllvm" "-x86-align-branch-prefix-size=5"
+// RUN: %clang -target x86_64 -mbranches-within-32B-boundaries -mno-branches-within-32B-boundaries %s -c -### 2>&1 | FileCheck %s --check-prefix=NO32B
+// NO32B-NOT: "-x86-align-branch
+/// Test that -malign-branch* can override individual values of -mbranches-within-32B-boundaries
+// RUN: %clang -target x86_64 -malign-branch=jcc -mbranches-within-32B-boundaries %s -c -### 2>&1 | FileCheck %s --check-prefix=32B-TYPE
+// 32B-TYPE:  "-mllvm" "-x86-align-branch=jcc" "-mllvm" "-x86-align-branch-boundary=32" "-mllvm" "-x86-align-branch-prefix-size=5"
+
+/// Unsupported on other targets.
+// RUN: 

[PATCH] D71612: [analyzer] Add PlacementNewChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D71612#1812036 , @martong wrote:

> In D71612#1790045 , @NoQ wrote:
>
> > I wonder if this checker will find any misuses of placement into 
> > `llvm::TrailingObjects`.
>
>
> I've evaluated this checker on LLVM/Clang source and it did not find any 
> results, though there are many placement new calls there. I think this is 
> good, because there are no false positives.


In such cases it's usually a good idea to verify that the checker works 
correctly by artificially injecting a bug into the codebase. If the bug is not 
found, the checker is probably not working. If the bug is found, change it to 
be more and more realistic, so that to see what limitations does the checker 
have in terms of false negatives. Monitor analyzer stats closely (max-nodes 
limits, block count limits, inlining limits) in order to see what exactly goes 
wrong (or debug on the Exploded Graph as usual, depending on how it goes 
wrong). This exercise often uncovers interesting omissions :)

Can we enable the check by default then? What pieces are missing? Like, the 
symbolic extent/offset case is not a must. I think we have everything except 
maybe some deeper testing. I'd rather spend some time on the above exercise, 
but then enable by default if it goes well.

> The two failures are caused by non existing `.inc` files, e.g.:
> 
>   
> /mnt/ssd/egbomrt/WORK/llvm1/git/llvm-project/llvm/unittests/Option/OptionParsingTest.cpp:23:10:
>  fatal error: 'Opts.inc' file not found
>   #include "Opts.inc"
>^~
>   1 error generated.

I've seen these errors with scan-build-py as well. It looks like LLVM's build 
system cannot be correctly reduced to a compilation database (though it's just 
one place - maybe it can be easily fixed).




Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:91
+  SVal SizeOfPlace = getExtentSizeOfPlace(C, Place, State);
+  const auto SizeOfTargetCI = SizeOfTarget.getAs();
+  if (!SizeOfTargetCI)

xazax.hun wrote:
> NoQ wrote:
> > xazax.hun wrote:
> > > martong wrote:
> > > > xazax.hun wrote:
> > > > > Here, instead of getting `SizeOfTarget` and `SizeOfPlace` as 
> > > > > `ConcreteInt`s, I think you should rather use `evalBinOp` to compare 
> > > > > them. That method is more future proof as if we cannot constraint 
> > > > > these values down to a single integer but we still have some 
> > > > > information about them a sufficiently smart solver could prove the 
> > > > > relationship between the symbolic values.
> > > > I am not sure if `evalBinOp` is that useful here, because we need the 
> > > > concrete values anyway when we issue the diagnostics. We'd like to 
> > > > present the concrete sizes in bytes.
> > > The reason why evalbinop might be useful because we might have symbolic 
> > > sizes:
> > > ```
> > > void f(int a) {
> > >  char *buffer = new char[a];
> > > }
> > > ```
> > > 
> > > So in the code snippet above you cannot get a concrete integer for the 
> > > size of the buffer. But in case we already have some constraints about 
> > > the value of `a`, the constraint solver might be able to tell if we are 
> > > sure that the type will not fit into the buffer. I can imagine that this 
> > > scenario is relatively rare, but I think we need relatively little code 
> > > to support this. 
> > > 
> > > So you could potentially warn when:
> > > ```
> > > void f(int a) {
> > >   char *buffer = new char[a];
> > >   if (a > 3)
> > > return;
> > >   int *p = new (buffer) int;
> > > }
> > > ```
> > > 
> > > I know, this is silly code, but we might not know if there are reasonable 
> > > code that has similar patterns.
> > For this sort of stuff i'd strongly recommend
> > 1. explaining the range constraints for the buffer size in the warning 
> > message and
> > 2. making a bug visitor that'll explain how these constraints change across 
> > the path.
> > 
> > I.e., "Assuming that 'a' is less than or equal to 3" => "Buffer size is 
> > constrained to [0, 3]" => "Storage provided to placement new is //at most// 
> > 3 bytes, whereas the allocated type requires 4 bytes".
> > 
> > The same applies to our alpha array bound checkers. We really need this 
> > stuff explained in them. Without such facilities i'd rather stick to 
> > concrete values.
> This makes sense. How about committing this only supporting concrete values 
> and introduce the visitor/symbolic support in a follow-up? (If Gabor Marton 
> is motivated to implement it :) I am also ok if this will not get implemented 
> for now.)
Technically, there's also a chance of the object size being symbolic (i.e., 
"placement `new[]`"), however placement `new[]` is already very weird due to 
the requirement to store the "allocated" size within the storage together with 
the actual buffer (IIRC the size of such header is implementation-defined and 
the operator almost always returns an 

[PATCH] D71612: [analyzer] Add PlacementNewChecker

[Gábor Horváth via Phabricator via cfe-commits]

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



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:91
+  SVal SizeOfPlace = getExtentSizeOfPlace(C, Place, State);
+  const auto SizeOfTargetCI = SizeOfTarget.getAs();
+  if (!SizeOfTargetCI)

NoQ wrote:
> xazax.hun wrote:
> > martong wrote:
> > > xazax.hun wrote:
> > > > Here, instead of getting `SizeOfTarget` and `SizeOfPlace` as 
> > > > `ConcreteInt`s, I think you should rather use `evalBinOp` to compare 
> > > > them. That method is more future proof as if we cannot constraint these 
> > > > values down to a single integer but we still have some information 
> > > > about them a sufficiently smart solver could prove the relationship 
> > > > between the symbolic values.
> > > I am not sure if `evalBinOp` is that useful here, because we need the 
> > > concrete values anyway when we issue the diagnostics. We'd like to 
> > > present the concrete sizes in bytes.
> > The reason why evalbinop might be useful because we might have symbolic 
> > sizes:
> > ```
> > void f(int a) {
> >  char *buffer = new char[a];
> > }
> > ```
> > 
> > So in the code snippet above you cannot get a concrete integer for the size 
> > of the buffer. But in case we already have some constraints about the value 
> > of `a`, the constraint solver might be able to tell if we are sure that the 
> > type will not fit into the buffer. I can imagine that this scenario is 
> > relatively rare, but I think we need relatively little code to support 
> > this. 
> > 
> > So you could potentially warn when:
> > ```
> > void f(int a) {
> >   char *buffer = new char[a];
> >   if (a > 3)
> > return;
> >   int *p = new (buffer) int;
> > }
> > ```
> > 
> > I know, this is silly code, but we might not know if there are reasonable 
> > code that has similar patterns.
> For this sort of stuff i'd strongly recommend
> 1. explaining the range constraints for the buffer size in the warning 
> message and
> 2. making a bug visitor that'll explain how these constraints change across 
> the path.
> 
> I.e., "Assuming that 'a' is less than or equal to 3" => "Buffer size is 
> constrained to [0, 3]" => "Storage provided to placement new is //at most// 3 
> bytes, whereas the allocated type requires 4 bytes".
> 
> The same applies to our alpha array bound checkers. We really need this stuff 
> explained in them. Without such facilities i'd rather stick to concrete 
> values.
This makes sense. How about committing this only supporting concrete values and 
introduce the visitor/symbolic support in a follow-up? (If Gabor Marton is 
motivated to implement it :) I am also ok if this will not get implemented for 
now.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71612



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

[PATCH] D72245: [PoC][RISCV][LTO] Pass target-abi via module flag metadata

[Kuan Hsu Chen (Zakk) via Phabricator via cfe-commits]

khchen updated this revision to Diff 237114.
khchen added a comment.

remote LTO related code.
this PoC include D70837  patch for generate 
correct code.


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

https://reviews.llvm.org/D72245

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/riscv-metadata.c
  llvm/include/llvm/Target/TargetMachine.h
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/Utils/RISCVBaseInfo.cpp
  llvm/lib/Target/TargetMachine.cpp
  llvm/test/CodeGen/RISCV/module-target-abi.ll
  llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
  llvm/test/CodeGen/RISCV/subtarget-features-target-abi.ll
  llvm/tools/llc/llc.cpp

Index: llvm/tools/llc/llc.cpp
===
--- llvm/tools/llc/llc.cpp
+++ llvm/tools/llc/llc.cpp
@@ -455,6 +455,7 @@
   Options.MCOptions.PreserveAsmComments = PreserveComments;
   Options.MCOptions.IASSearchPaths = IncludeDirs;
   Options.MCOptions.SplitDwarfFile = SplitDwarfFile;
+  TargetMachine::initTargetOptions(*M, Options);
 
   std::unique_ptr Target(TheTarget->createTargetMachine(
   TheTriple.getTriple(), CPUStr, FeaturesStr, Options, getRelocModel(),
Index: llvm/test/CodeGen/RISCV/subtarget-features-target-abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/subtarget-features-target-abi.ll
@@ -0,0 +1,26 @@
+; RUN: llc -mtriple=riscv32 < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=DEFAULT %s
+; RUN: llc -mtriple=riscv32 -target-abi ilp32 < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32IF-ILP32 %s
+; RUN: llc -mtriple=riscv32 -target-abi ilp32f < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32IF-ILP32F %s
+; RUN: llc -mtriple=riscv32 -filetype=obj < %s | llvm-readelf -h - | FileCheck -check-prefixes=FLAGS %s
+
+; RV32IF-ILP32: -target-abi option != target-abi module flag(ignoring target-abi option)
+
+; FLAGS: Flags: 0x2, single-float ABI
+
+define float @foo(i32 %a) nounwind #0 {
+; DEFAULT: # %bb.0:
+; DEFAULT-NEXT: fcvt.s.w  fa0, a0
+; DEFAULT-NEXT: ret
+; RV32IF-ILP32F: # %bb.0:
+; RV32IF-ILP32F-NEXT: fcvt.s.w  fa0, a0
+; RV32IF-ILP32F-NEXT: ret
+  %conv = sitofp i32 %a to float
+  ret float %conv
+}
+
+attributes #0 = { "target-features"="+f"}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"target-abi", !"ilp32f"}
Index: llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
===
--- llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
+++ llvm/test/CodeGen/RISCV/subtarget-features-std-ext.ll
@@ -2,12 +2,17 @@
 ; RUN:   | FileCheck -check-prefix=RV32IF-ILP32 %s
 ; RUN: llc -mtriple=riscv32 -target-abi ilp32f < %s 2>&1 \
 ; RUN:   | FileCheck -check-prefix=RV32IF-ILP32F %s
+; RUN: llc -mtriple=riscv32 -mattr=-f -target-abi ilp32f <%s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32I-ILP32F-FAILED %s
+
+; RV32I-ILP32F-FAILED: Hard-float 'f' ABI can't be used for a target that doesn't support the F instruction set extension
 
-; RV32IF-ILP32F: Hard-float 'f' ABI can't be used for a target that doesn't support the F instruction set extension (ignoring target-abi)
 
 define float @foo(i32 %a) nounwind #0 {
-; RV32IF-ILP32: # %bb.0:
-; RV32IF-ILP32-NEXT: fcvt.s.w  ft0, a0
+; RV32IF-ILP32: fcvt.s.w  ft0, a0
+; RV32IF-ILP32-NEXT: fmv.x.w a0, ft0
+; RV32IF-ILP32F: fcvt.s.w fa0, a0
+; RV32IF-ILP32F-NEXT: ret
   %conv = sitofp i32 %a to float
   ret float %conv
 }
Index: llvm/test/CodeGen/RISCV/module-target-abi.ll
===
--- /dev/null
+++ llvm/test/CodeGen/RISCV/module-target-abi.ll
@@ -0,0 +1,24 @@
+; RUN: llc -mtriple=riscv32 < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=DEFAULT %s
+; RUN: llc -mtriple=riscv32 -target-abi ilp32 < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32IF-ILP32 %s
+; RUN: llc -mtriple=riscv32 -target-abi ilp32f < %s 2>&1 \
+; RUN:   | FileCheck -check-prefix=RV32IF-ILP32F %s
+; RUN: llc -mtriple=riscv32 -filetype=obj < %s | llvm-readelf -h - | FileCheck -check-prefixes=FLAGS %s
+
+; RV32IF-ILP32F: -target-abi option != target-abi module flag(ignoring target-abi option)
+
+; FLAGS: Flags: 0x0
+
+define float @foo(i32 %a) nounwind #0 {
+; DEFAULT: # %bb.0:
+; DEFAULT: fmv.x.w a0, ft0
+; RV32IF-ILP32: # %bb.0:
+; RV32IF-ILP32: fmv.x.w a0, ft0
+  %conv = sitofp i32 %a to float
+  ret float %conv
+}
+
+attributes #0 = { "target-features"="+f"}
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"target-abi", !"ilp32"}
Index: llvm/lib/Target/TargetMachine.cpp
===
--- llvm/lib/Target/TargetMachine.cpp
+++ llvm/lib/Target/TargetMachine.cpp
@@ -63,6 +63,19 @@
   RESET_OPTION(NoInfsFPMath, "no-infs-fp-math");
   RESET_OPTION(NoNaNsFPMath, "no-nans-fp-math");
   

[PATCH] D72271: [Clang] Handle target-specific builtins returning aggregates.

[Simon Tatham via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG06d07ec4a372: [Clang] Handle target-specific builtins 
returning aggregates. (authored by simon_tatham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72271

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/arm-mve-intrinsics/vld24.c


Index: clang/test/CodeGen/arm-mve-intrinsics/vld24.c
===
--- clang/test/CodeGen/arm-mve-intrinsics/vld24.c
+++ clang/test/CodeGen/arm-mve-intrinsics/vld24.c
@@ -98,3 +98,45 @@
 vst2q_f16(addr, value);
 #endif /* POLYMORPHIC */
 }
+
+// CHECK-LABEL: @load_into_variable(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = call { <8 x i16>, <8 x i16> } 
@llvm.arm.mve.vld2q.v8i16.p0i16(i16* [[ADDR:%.*]])
+// CHECK-NEXT:[[TMP1:%.*]] = extractvalue { <8 x i16>, <8 x i16> } 
[[TMP0]], 0
+// CHECK-NEXT:[[TMP2:%.*]] = insertvalue [[STRUCT_UINT16X8X2_T:%.*]] 
undef, <8 x i16> [[TMP1]], 0, 0
+// CHECK-NEXT:[[TMP3:%.*]] = extractvalue { <8 x i16>, <8 x i16> } 
[[TMP0]], 1
+// CHECK-NEXT:[[TMP4:%.*]] = insertvalue [[STRUCT_UINT16X8X2_T]] [[TMP2]], 
<8 x i16> [[TMP3]], 0, 1
+// CHECK-NEXT:store <8 x i16> [[TMP1]], <8 x i16>* [[VALUES:%.*]], align 8
+// CHECK-NEXT:[[ARRAYIDX4:%.*]] = getelementptr inbounds <8 x i16>, <8 x 
i16>* [[VALUES]], i32 1
+// CHECK-NEXT:store <8 x i16> [[TMP3]], <8 x i16>* [[ARRAYIDX4]], align 8
+// CHECK-NEXT:ret void
+//
+void load_into_variable(const uint16_t *addr, uint16x8_t *values)
+{
+uint16x8x2_t v;
+#ifdef POLYMORPHIC
+v = vld2q(addr);
+#else /* POLYMORPHIC */
+v = vld2q_u16(addr);
+#endif /* POLYMORPHIC */
+values[0] = v.val[0];
+values[1] = v.val[1];
+}
+
+// CHECK-LABEL: @extract_one_vector(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = call { <4 x i32>, <4 x i32> } 
@llvm.arm.mve.vld2q.v4i32.p0i32(i32* [[ADDR:%.*]])
+// CHECK-NEXT:[[TMP1:%.*]] = extractvalue { <4 x i32>, <4 x i32> } 
[[TMP0]], 0
+// CHECK-NEXT:[[TMP2:%.*]] = insertvalue [[STRUCT_INT32X4X2_T:%.*]] undef, 
<4 x i32> [[TMP1]], 0, 0
+// CHECK-NEXT:[[TMP3:%.*]] = extractvalue { <4 x i32>, <4 x i32> } 
[[TMP0]], 1
+// CHECK-NEXT:[[TMP4:%.*]] = insertvalue [[STRUCT_INT32X4X2_T]] [[TMP2]], 
<4 x i32> [[TMP3]], 0, 1
+// CHECK-NEXT:ret <4 x i32> [[TMP1]]
+//
+int32x4_t extract_one_vector(const int32_t *addr)
+{
+#ifdef POLYMORPHIC
+return vld2q(addr).val[0];
+#else /* POLYMORPHIC */
+return vld2q_s32(addr).val[0];
+#endif /* POLYMORPHIC */
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -4332,9 +4332,29 @@
 return RValue::get(V);
   }
 
-  // See if we have a target specific builtin that needs to be lowered.
-  if (Value *V = EmitTargetBuiltinExpr(BuiltinID, E, ReturnValue))
-return RValue::get(V);
+  // Some target-specific builtins can have aggregate return values, e.g.
+  // __builtin_arm_mve_vld2q_u32. So if the result is an aggregate, force
+  // ReturnValue to be non-null, so that the target-specific emission code can
+  // always just emit into it.
+  TypeEvaluationKind EvalKind = getEvaluationKind(E->getType());
+  if (EvalKind == TEK_Aggregate && ReturnValue.isNull()) {
+Address DestPtr = CreateMemTemp(E->getType(), "agg.tmp");
+ReturnValue = ReturnValueSlot(DestPtr, false);
+  }
+
+  // Now see if we can emit a target-specific builtin.
+  if (Value *V = EmitTargetBuiltinExpr(BuiltinID, E, ReturnValue)) {
+switch (EvalKind) {
+case TEK_Scalar:
+  return RValue::get(V);
+case TEK_Aggregate:
+  return RValue::getAggregate(ReturnValue.getValue(),
+  ReturnValue.isVolatile());
+case TEK_Complex:
+  llvm_unreachable("No current target builtin returns complex");
+}
+llvm_unreachable("Bad evaluation kind in EmitBuiltinExpr");
+  }
 
   ErrorUnsupported(E, "builtin function");
 


Index: clang/test/CodeGen/arm-mve-intrinsics/vld24.c
===
--- clang/test/CodeGen/arm-mve-intrinsics/vld24.c
+++ clang/test/CodeGen/arm-mve-intrinsics/vld24.c
@@ -98,3 +98,45 @@
 vst2q_f16(addr, value);
 #endif /* POLYMORPHIC */
 }
+
+// CHECK-LABEL: @load_into_variable(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = call { <8 x i16>, <8 x i16> } @llvm.arm.mve.vld2q.v8i16.p0i16(i16* [[ADDR:%.*]])
+// CHECK-NEXT:[[TMP1:%.*]] = extractvalue { <8 x i16>, <8 x i16> } [[TMP0]], 0
+// CHECK-NEXT:[[TMP2:%.*]] = insertvalue [[STRUCT_UINT16X8X2_T:%.*]] undef, <8 x i16> [[TMP1]], 0, 0
+// CHECK-NEXT:[[TMP3:%.*]] = extractvalue { <8 x i16>, <8 x i16> } [[TMP0]], 1
+// CHECK-NEXT:[[TMP4:%.*]] = insertvalue [[STRUCT_UINT16X8X2_T]] [[TMP2]], <8 x i16> [[TMP3]], 0, 1
+// 

[PATCH] D71612: [analyzer] Add PlacementNewChecker

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp:91
+  SVal SizeOfPlace = getExtentSizeOfPlace(C, Place, State);
+  const auto SizeOfTargetCI = SizeOfTarget.getAs();
+  if (!SizeOfTargetCI)

xazax.hun wrote:
> martong wrote:
> > xazax.hun wrote:
> > > Here, instead of getting `SizeOfTarget` and `SizeOfPlace` as 
> > > `ConcreteInt`s, I think you should rather use `evalBinOp` to compare 
> > > them. That method is more future proof as if we cannot constraint these 
> > > values down to a single integer but we still have some information about 
> > > them a sufficiently smart solver could prove the relationship between the 
> > > symbolic values.
> > I am not sure if `evalBinOp` is that useful here, because we need the 
> > concrete values anyway when we issue the diagnostics. We'd like to present 
> > the concrete sizes in bytes.
> The reason why evalbinop might be useful because we might have symbolic sizes:
> ```
> void f(int a) {
>  char *buffer = new char[a];
> }
> ```
> 
> So in the code snippet above you cannot get a concrete integer for the size 
> of the buffer. But in case we already have some constraints about the value 
> of `a`, the constraint solver might be able to tell if we are sure that the 
> type will not fit into the buffer. I can imagine that this scenario is 
> relatively rare, but I think we need relatively little code to support this. 
> 
> So you could potentially warn when:
> ```
> void f(int a) {
>   char *buffer = new char[a];
>   if (a > 3)
> return;
>   int *p = new (buffer) int;
> }
> ```
> 
> I know, this is silly code, but we might not know if there are reasonable 
> code that has similar patterns.
For this sort of stuff i'd strongly recommend
1. explaining the range constraints for the buffer size in the warning message 
and
2. making a bug visitor that'll explain how these constraints change across the 
path.

I.e., "Assuming that 'a' is less than or equal to 3" => "Buffer size is 
constrained to [0, 3]" => "Storage provided to placement new is //at most// 3 
bytes, whereas the allocated type requires 4 bytes".

The same applies to our alpha array bound checkers. We really need this stuff 
explained in them. Without such facilities i'd rather stick to concrete values.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71612



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

[clang] 06d07ec - [Clang] Handle target-specific builtins returning aggregates.

[Simon Tatham via cfe-commits]

Author: Simon Tatham
Date: 2020-01-09T17:28:37Z
New Revision: 06d07ec4a372b55e6fb77bf0b97964bde16a3184

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

LOG: [Clang] Handle target-specific builtins returning aggregates.

Summary:
A few of the ARM MVE builtins directly return a structure type. This
causes an assertion failure at code-gen time if you try to assign the
result of the builtin to a variable, because the `RValue` created in
`EmitBuiltinExpr` from the `llvm::Value` produced by codegen is always
made by `RValue::get()`, which creates a non-aggregate `RValue` that
will fail an assertion when `AggExprEmitter::withReturnValueSlot` calls
`Src.getAggregatePointer()`. A similar failure occurs if you try to use
the struct return value directly to extract one field, e.g.
`vld2q(address).val[0]`.

The existing code-gen tests for those MVE builtins pass the returned
structure type directly to the C `return` statement, which apparently
managed to avoid that particular code path, so we didn't notice the
crash.

Now `EmitBuiltinExpr` checks the evaluation kind of the builtin's return
value, and does the necessary handling for aggregate returns. I've added
two extra test cases, both of which crashed before this change.

Reviewers: dmgreen, rjmccall

Reviewed By: rjmccall

Subscribers: kristof.beyls, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/CodeGen/CGBuiltin.cpp
clang/test/CodeGen/arm-mve-intrinsics/vld24.c

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 3fadf09c460d..2842fe826636 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -4332,9 +4332,29 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 return RValue::get(V);
   }
 
-  // See if we have a target specific builtin that needs to be lowered.
-  if (Value *V = EmitTargetBuiltinExpr(BuiltinID, E, ReturnValue))
-return RValue::get(V);
+  // Some target-specific builtins can have aggregate return values, e.g.
+  // __builtin_arm_mve_vld2q_u32. So if the result is an aggregate, force
+  // ReturnValue to be non-null, so that the target-specific emission code can
+  // always just emit into it.
+  TypeEvaluationKind EvalKind = getEvaluationKind(E->getType());
+  if (EvalKind == TEK_Aggregate && ReturnValue.isNull()) {
+Address DestPtr = CreateMemTemp(E->getType(), "agg.tmp");
+ReturnValue = ReturnValueSlot(DestPtr, false);
+  }
+
+  // Now see if we can emit a target-specific builtin.
+  if (Value *V = EmitTargetBuiltinExpr(BuiltinID, E, ReturnValue)) {
+switch (EvalKind) {
+case TEK_Scalar:
+  return RValue::get(V);
+case TEK_Aggregate:
+  return RValue::getAggregate(ReturnValue.getValue(),
+  ReturnValue.isVolatile());
+case TEK_Complex:
+  llvm_unreachable("No current target builtin returns complex");
+}
+llvm_unreachable("Bad evaluation kind in EmitBuiltinExpr");
+  }
 
   ErrorUnsupported(E, "builtin function");
 

diff  --git a/clang/test/CodeGen/arm-mve-intrinsics/vld24.c 
b/clang/test/CodeGen/arm-mve-intrinsics/vld24.c
index 984d5989217e..a0f37fe65d3d 100644
--- a/clang/test/CodeGen/arm-mve-intrinsics/vld24.c
+++ b/clang/test/CodeGen/arm-mve-intrinsics/vld24.c
@@ -98,3 +98,45 @@ void test_vst2q_f16(float16_t *addr, float16x8x2_t value)
 vst2q_f16(addr, value);
 #endif /* POLYMORPHIC */
 }
+
+// CHECK-LABEL: @load_into_variable(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = call { <8 x i16>, <8 x i16> } 
@llvm.arm.mve.vld2q.v8i16.p0i16(i16* [[ADDR:%.*]])
+// CHECK-NEXT:[[TMP1:%.*]] = extractvalue { <8 x i16>, <8 x i16> } 
[[TMP0]], 0
+// CHECK-NEXT:[[TMP2:%.*]] = insertvalue [[STRUCT_UINT16X8X2_T:%.*]] 
undef, <8 x i16> [[TMP1]], 0, 0
+// CHECK-NEXT:[[TMP3:%.*]] = extractvalue { <8 x i16>, <8 x i16> } 
[[TMP0]], 1
+// CHECK-NEXT:[[TMP4:%.*]] = insertvalue [[STRUCT_UINT16X8X2_T]] [[TMP2]], 
<8 x i16> [[TMP3]], 0, 1
+// CHECK-NEXT:store <8 x i16> [[TMP1]], <8 x i16>* [[VALUES:%.*]], align 8
+// CHECK-NEXT:[[ARRAYIDX4:%.*]] = getelementptr inbounds <8 x i16>, <8 x 
i16>* [[VALUES]], i32 1
+// CHECK-NEXT:store <8 x i16> [[TMP3]], <8 x i16>* [[ARRAYIDX4]], align 8
+// CHECK-NEXT:ret void
+//
+void load_into_variable(const uint16_t *addr, uint16x8_t *values)
+{
+uint16x8x2_t v;
+#ifdef POLYMORPHIC
+v = vld2q(addr);
+#else /* POLYMORPHIC */
+v = vld2q_u16(addr);
+#endif /* POLYMORPHIC */
+values[0] = v.val[0];
+values[1] = v.val[1];
+}
+
+// CHECK-LABEL: @extract_one_vector(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[TMP0:%.*]] = call { <4 x i32>, <4 x i32> } 

[PATCH] D72458: [clangd] Adjust diagnostic range to be inside main file

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61663 tests passed, 0 failed 
and 779 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72458



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

[PATCH] D72458: [clangd] Adjust diagnostic range to be inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 237105.
kadircet marked an inline comment as done.
kadircet added a comment.

- Use find_if instead of raw loop


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72458

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1014,6 +1014,29 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(ToLSPDiag, RangeIsInMain) {
+  ClangdDiagnosticOptions Opts;
+  clangd::Diag D;
+  D.Range = {pos(1, 2), pos(3, 4)};
+  D.Notes.emplace_back();
+  Note  = D.Notes.back();
+  N.Range = {pos(2, 3), pos(3, 4)};
+
+  D.InsideMainFile = true;
+  N.InsideMainFile = false;
+  toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef) {
+   EXPECT_EQ(LSPDiag.range, D.Range);
+ });
+
+  D.InsideMainFile = false;
+  N.InsideMainFile = true;
+  toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef) {
+   EXPECT_EQ(LSPDiag.range, N.Range);
+ });
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -22,6 +22,8 @@
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
@@ -328,14 +330,22 @@
 void toLSPDiags(
 const Diag , const URIForFile , const ClangdDiagnosticOptions ,
 llvm::function_ref)> OutFn) {
-  auto FillBasicFields = [](const DiagBase ) -> clangd::Diagnostic {
-clangd::Diagnostic Res;
-Res.range = D.Range;
-Res.severity = getSeverity(D.Severity);
-return Res;
-  };
+  clangd::Diagnostic Main;
+  Main.severity = getSeverity(D.Severity);
+
+  // Main diagnostic should always refer to a range inside main file. If a
+  // diagnostic made it so for, it means either itself or one of its notes is
+  // inside main file.
+  if (D.InsideMainFile) {
+Main.range = D.Range;
+  } else {
+auto It =
+llvm::find_if(D.Notes, [](const Note ) { return N.InsideMainFile; });
+assert(It != D.Notes.end() &&
+   "neither the main diagnostic nor notes are inside main file");
+Main.range = It->Range;
+  }
 
-  clangd::Diagnostic Main = FillBasicFields(D);
   Main.code = D.Name;
   switch (D.Source) {
   case Diag::Clang:
@@ -379,7 +389,9 @@
 for (auto  : D.Notes) {
   if (!Note.InsideMainFile)
 continue;
-  clangd::Diagnostic Res = FillBasicFields(Note);
+  clangd::Diagnostic Res;
+  Res.severity = getSeverity(Note.Severity);
+  Res.range = Note.Range;
   Res.message = noteMessage(D, Note, Opts);
   OutFn(std::move(Res), llvm::ArrayRef());
 }


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1014,6 +1014,29 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(ToLSPDiag, RangeIsInMain) {
+  ClangdDiagnosticOptions Opts;
+  clangd::Diag D;
+  D.Range = {pos(1, 2), pos(3, 4)};
+  D.Notes.emplace_back();
+  Note  = D.Notes.back();
+  N.Range = {pos(2, 3), pos(3, 4)};
+
+  D.InsideMainFile = true;
+  N.InsideMainFile = false;
+  toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef) {
+   EXPECT_EQ(LSPDiag.range, D.Range);
+ });
+
+  D.InsideMainFile = false;
+  N.InsideMainFile = true;
+  toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef) {
+   EXPECT_EQ(LSPDiag.range, N.Range);
+ });
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -22,6 +22,8 @@
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
@@ -328,14 +330,22 @@
 void toLSPDiags(
 const Diag , const URIForFile , const ClangdDiagnosticOptions ,
 llvm::function_ref)> OutFn) {

[PATCH] D72462: [clangd] Fix markdown rendering in VSCode

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61662 tests passed, 0 failed 
and 779 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72462



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

[PATCH] D72458: [clangd] Adjust diagnostic range to be inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG189aa5b7a458: [clangd] Adjust diagnostic range to be inside 
main file (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72458

Files:
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1014,6 +1014,29 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(ToLSPDiag, RangeIsInMain) {
+  ClangdDiagnosticOptions Opts;
+  clangd::Diag D;
+  D.Range = {pos(1, 2), pos(3, 4)};
+  D.Notes.emplace_back();
+  Note  = D.Notes.back();
+  N.Range = {pos(2, 3), pos(3, 4)};
+
+  D.InsideMainFile = true;
+  N.InsideMainFile = false;
+  toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef) {
+   EXPECT_EQ(LSPDiag.range, D.Range);
+ });
+
+  D.InsideMainFile = false;
+  N.InsideMainFile = true;
+  toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef) {
+   EXPECT_EQ(LSPDiag.range, N.Range);
+ });
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -22,6 +22,8 @@
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
@@ -328,14 +330,22 @@
 void toLSPDiags(
 const Diag , const URIForFile , const ClangdDiagnosticOptions ,
 llvm::function_ref)> OutFn) {
-  auto FillBasicFields = [](const DiagBase ) -> clangd::Diagnostic {
-clangd::Diagnostic Res;
-Res.range = D.Range;
-Res.severity = getSeverity(D.Severity);
-return Res;
-  };
+  clangd::Diagnostic Main;
+  Main.severity = getSeverity(D.Severity);
+
+  // Main diagnostic should always refer to a range inside main file. If a
+  // diagnostic made it so for, it means either itself or one of its notes is
+  // inside main file.
+  if (D.InsideMainFile) {
+Main.range = D.Range;
+  } else {
+auto It =
+llvm::find_if(D.Notes, [](const Note ) { return N.InsideMainFile; });
+assert(It != D.Notes.end() &&
+   "neither the main diagnostic nor notes are inside main file");
+Main.range = It->Range;
+  }
 
-  clangd::Diagnostic Main = FillBasicFields(D);
   Main.code = D.Name;
   switch (D.Source) {
   case Diag::Clang:
@@ -379,7 +389,9 @@
 for (auto  : D.Notes) {
   if (!Note.InsideMainFile)
 continue;
-  clangd::Diagnostic Res = FillBasicFields(Note);
+  clangd::Diagnostic Res;
+  Res.severity = getSeverity(Note.Severity);
+  Res.range = Note.Range;
   Res.message = noteMessage(D, Note, Opts);
   OutFn(std::move(Res), llvm::ArrayRef());
 }


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1014,6 +1014,29 @@
   EXPECT_THAT(TU.build().getDiagnostics(), UnorderedElementsAre());
 }
 
+TEST(ToLSPDiag, RangeIsInMain) {
+  ClangdDiagnosticOptions Opts;
+  clangd::Diag D;
+  D.Range = {pos(1, 2), pos(3, 4)};
+  D.Notes.emplace_back();
+  Note  = D.Notes.back();
+  N.Range = {pos(2, 3), pos(3, 4)};
+
+  D.InsideMainFile = true;
+  N.InsideMainFile = false;
+  toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef) {
+   EXPECT_EQ(LSPDiag.range, D.Range);
+ });
+
+  D.InsideMainFile = false;
+  N.InsideMainFile = true;
+  toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef) {
+   EXPECT_EQ(LSPDiag.range, N.Range);
+ });
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/Diagnostics.cpp
===
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -22,6 +22,8 @@
 #include "clang/Lex/Token.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
@@ -328,14 +330,22 @@
 void toLSPDiags(
 const Diag , const URIForFile , const ClangdDiagnosticOptions