[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

[Conrad Poelman via Phabricator via cfe-commits]

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

In D68682#1753357 , @kadircet wrote:

> also could you rename the revision so that it reflects the fact that, this is 
> a change to clang-format and has nothing to do with clang-tidy ?


Done but feel free to suggest a better title.

(This journey started out with me trying to fix what I thought was a small 
quirk in clang-tidy's readability-redundant-member-init. Great feedback from 
more experienced developers along the way steered me to 
`cleanupAroundReplacements`, which feels perfect, but now I realize that gives 
it a broader impact.)

Thank you so much for the review and feedback!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68682



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

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc marked 5 inline comments as done.
poelmanc added inline comments.



Comment at: clang/lib/AST/CommentParser.cpp:19
 
-static inline bool isWhitespace(llvm::StringRef S) {
+// Consider moving this useful function to a more general utility location.
+bool isWhitespace(llvm::StringRef S) {

kadircet wrote:
> poelmanc wrote:
> > gribozavr2 wrote:
> > > clang/include/clang/Basic/CharInfo.h ?
> > Done. I renamed it to `isWhitespaceStringRef` to avoid making it an 
> > overload of the existing `isWhitespace(unsigned char)`, which causes 
> > ambiguous overload errors at QueryParser.cpp:42 & CommentSema.cpp:237.
> > 
> > We could alternatively keep this as an `isWhitespace` overload and instead 
> > change those two lines to use a `static_cast > char)>(::isWhitespace)` or precede them with a line like:
> > ```
> > bool (*isWhitespaceOverload)(unsigned char) = ::isWhitespace;
> > ```
> ah that's unfortunate, I believe it makes sense to have this as an overload 
> though.
> 
> Could you instead make the predicate explicit by putting
> ```[](char c) { return clang::isWhitespace(c); }```
> instead of just `clang::isWhitespace` in those call sites?
Excellent! Better than the two ideas I thought of. Thanks!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68682



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

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc updated this revision to Diff 230375.
poelmanc marked 3 inline comments as done.
poelmanc retitled this revision from "Clang-tidy fix removals removing all 
non-blank text from a line should remove the line" to 
"format::cleanupAroundReplacements removes whole line when Removals leave 
previously non-blank line blank".
poelmanc edited the summary of this revision.
poelmanc added a comment.

I addressed the latest code review comments, added tests to 
`clang/unittests/Format/CleanupTest.cpp`, and updated numerous tests to reflect 
improved removal of blank lines.

I now realize how widely used `cleanupAroundReplacements` is. That's great as 
this fix improves `clang-change-namespace` as a side-effect. However, I have 
little experience running tests beyond clang-tidy or on Linux (I'm testing with 
MSVC) and would appreciate help identifying any test failures or regressions. 
Also please check my change to `FixOnlyAffectedCodeAfterReplacements` in 
clang/unittests/Format/CleanupTest.cpp.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68682

Files:
  clang-tools-extra/clang-query/QueryParser.cpp
  clang-tools-extra/clang-tidy/readability/RedundantControlFlowCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-redundant-void-arg.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-control-flow.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-declaration.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-member-init.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr.cpp
  clang-tools-extra/unittests/clang-change-namespace/ChangeNamespaceTests.cpp
  clang-tools-extra/unittests/clang-move/ClangMoveTests.cpp
  clang/include/clang/Basic/CharInfo.h
  clang/include/clang/Format/Format.h
  clang/lib/AST/CommentLexer.cpp
  clang/lib/AST/CommentParser.cpp
  clang/lib/AST/CommentSema.cpp
  clang/lib/Format/Format.cpp
  clang/unittests/Format/CleanupTest.cpp

Index: clang/unittests/Format/CleanupTest.cpp
===
--- clang/unittests/Format/CleanupTest.cpp
+++ clang/unittests/Format/CleanupTest.cpp
@@ -349,11 +349,13 @@
  "namespace C {\n"
  "namespace D { int i; }\n"
  "inline namespace E { namespace { int y; } }\n"
- "int x= 0;"
+ "\n"
+ "int x= 0;\n"
  "}";
-  std::string Expected = "\n\nnamespace C {\n"
- "namespace D { int i; }\n\n"
- "int x= 0;"
+  std::string Expected = "\nnamespace C {\n"
+ "namespace D { int i; }\n"
+ "\n"
+ "int x= 0;\n"
  "}";
   tooling::Replacements Replaces =
   toReplacements({createReplacement(getOffset(Code, 3, 3), 6, ""),
@@ -362,6 +364,87 @@
   EXPECT_EQ(Expected, formatAndApply(Code, Replaces));
 }
 
+TEST_F(CleanUpReplacementsTest, RemoveLineWhenAllNonWhitespaceRemoved) {
+  std::string Code = "namespace A { // Useless comment\n"
+ "  int x\t = 0;\t\n"
+ "  int y\t = 0;\t\n"
+ "} // namespace A\n";
+  std::string Expected = "namespace A {\n"
+ "  int y\t = 0;\n"
+ "} // namespace A\n";
+  tooling::Replacements Replaces =
+  toReplacements({createReplacement(getOffset(Code, 1, 14), 19, ""),
+  createReplacement(getOffset(Code, 2, 3), 3, ""),
+  createReplacement(getOffset(Code, 2, 7), 1, ""),
+  createReplacement(getOffset(Code, 2, 10), 1, ""),
+  createReplacement(getOffset(Code, 2, 12), 2, ""),
+  createReplacement(getOffset(Code, 3, 14), 1, "")});
+
+  EXPECT_EQ(Expected, apply(Code, Replaces));
+}
+
+TEST_F(CleanUpReplacementsTest, RemoveLinesWhenAllNonWhitespaceRemoved) {
+  std::string Code = "struct A {\n"
+ "  A()\n"
+ "  : f(),\n"
+ "g()\n"
+ "  {}\n"
+ "  int f = 0;\n"
+ "  int g = 0;\n"
+ "};";
+  std::string Expected = "struct A {\n"
+ "  A()\n"
+ "  {}\n"
+ "  int f = 0;\n"
+ "  int g = 0;\n"
+ "};";
+  tooling::Replacements Replaces =
+  toReplacements({createReplacement(getOffset(Code, 3, 5), 3, ""),
+  createReplacement(getOffset(Code, 3, 8), 1, ""),
+  createReplacement(getOffset(Code, 3, 3), 1, ""),
+  createReplacement(getOffset(Code, 4, 5), 3, "")});
+
+  EXPECT_EQ(Expected, apply(Code, 

[PATCH] D70525: [clang][IFS][test] GreenDragon and Fuchsia Darwin bot fix: BindArchClass Nest.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi created this revision.
plotfi added a reviewer: compnerd.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a post commit review. If I can re-implement this in a nicer way I will 
do an NFC commit.

  On Darwin the clang driver does not invoke Driver::BuildActions directly
  due to the need to handle Universal apps. Because of this there is a
  difference in code path where Driver::BuildUniversalActions is called
  instead of Driver::BuildActions. BuildUniversalActions ends up calling
  Driver::BuildAction but what it does differently is it takes the driver
  actions returned and wraps them each into a BindArchAction.
  
  In Driver::BuildJobs there is a check for '-o' to determine that
  multiple files are not specified when passing -o, except for Clang
  Interface Stub this need to be an exception as we actually want to write
  out multiple files: for every libfoo.so we have a libfoo.ifso sidecar
  ifso file, etc. To allow this to happen there is a check for
  IfsMergeAction, which is permitted to write out a secondary file. Except
  on Darwin, the IfsMergeAction gets wrapped in the BindArchAction by
  Driver::BuildUniversalActions so the check fails.
  
  This patch is to look inside a BindArchAction in Driver::BuildJobs to
  determine if there is in fact an IfsMergeAction, and if-so (pun intended)
  allow the secondary sidecard ifs/ifso file to be written out.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70525

Files:
  clang/lib/Driver/Driver.cpp


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3698,7 +3698,9 @@
 unsigned NumOutputs = 0;
 for (const Action *A : C.getActions())
   if (A->getType() != types::TY_Nothing &&
-  A->getKind() != Action::IfsMergeJobClass)
+  !(A->getKind() == Action::IfsMergeJobClass ||
+(A->getKind() == Action::BindArchClass && A->getInputs().size() &&
+ A->getInputs().front()->getKind() == Action::IfsMergeJobClass)))
 ++NumOutputs;
 
 if (NumOutputs > 1) {


Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3698,7 +3698,9 @@
 unsigned NumOutputs = 0;
 for (const Action *A : C.getActions())
   if (A->getType() != types::TY_Nothing &&
-  A->getKind() != Action::IfsMergeJobClass)
+  !(A->getKind() == Action::IfsMergeJobClass ||
+(A->getKind() == Action::BindArchClass && A->getInputs().size() &&
+ A->getInputs().front()->getKind() == Action::IfsMergeJobClass)))
 ++NumOutputs;
 
 if (NumOutputs > 1) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70524: Support DebugInfo generation for auto return type for C++ functions.

[Awanish Pandey via Phabricator via cfe-commits]

awpandey created this revision.
awpandey added reviewers: jini.susan.george, SouraVX, alok, aprantl, dblaikie.
awpandey added projects: clang, LLVM, debug-info.
Herald added subscribers: llvm-commits, cfe-commits, ormris.

This patch will provide support for auto return type for the C++ member 
functions. Before this return type of the member function is deduced and store 
in the DIE.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70524

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/test/CodeGenCXX/debug-info-auto-return.cpp
  llvm/test/DebugInfo/X86/debug-info-auto-return.ll

Index: llvm/test/DebugInfo/X86/debug-info-auto-return.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/debug-info-auto-return.ll
@@ -0,0 +1,171 @@
+;RUN: llc %s -filetype=obj -o - | llvm-dwarfdump -v - | FileCheck %s
+
+; C++ source to regenerate:
+
+;class myClass {
+;int low, high;
+;
+;  public:
+;  myClass(int a, int b) {
+;low = a;
+;high = b;
+;  }
+;auto findMax();
+;};
+;
+;auto myClass::findMax() {
+;if (low > high)
+;return 1;
+;  else
+;return 1;
+;}
+;int main() {
+;myClass f(3, 5);
+;  return 0;
+;}
+
+; CHECK: .debug_info contents:
+
+; CHECK:  DW_TAG_subprogram [7] *
+; CHECK-NEXT: DW_AT_linkage_name [DW_FORM_strx1](indexed (0007) string = "_ZN7myClass7findMaxEv")
+; CHECK: DW_AT_type [DW_FORM_ref4] {{.*}} "auto"
+; CHECK-NEXT: DW_AT_declaration {{.*}} (true)
+
+; CHECK: DW_TAG_unspecified_type
+; CHECK-NEXT:  DW_AT_name [DW_FORM_strx1] {{.*}} "auto"
+
+; ModuleID = 'debug-info-auto-return.cpp'
+source_filename = "debug-info-auto-return.cpp"
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+%class.myClass = type { i32, i32 }
+
+$_ZN7myClassC2Eii = comdat any
+
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local i32 @_ZN7myClass7findMaxEv(%class.myClass* %this) #0 align 2 !dbg !7 {
+entry:
+  %retval = alloca i32, align 4
+  %this.addr = alloca %class.myClass*, align 8
+  store %class.myClass* %this, %class.myClass** %this.addr, align 8
+  call void @llvm.dbg.declare(metadata %class.myClass** %this.addr, metadata !22, metadata !DIExpression()), !dbg !24
+  %this1 = load %class.myClass*, %class.myClass** %this.addr, align 8
+  %low = getelementptr inbounds %class.myClass, %class.myClass* %this1, i32 0, i32 0, !dbg !25
+  %0 = load i32, i32* %low, align 4, !dbg !25
+  %high = getelementptr inbounds %class.myClass, %class.myClass* %this1, i32 0, i32 1, !dbg !27
+  %1 = load i32, i32* %high, align 4, !dbg !27
+  %cmp = icmp sgt i32 %0, %1, !dbg !28
+  br i1 %cmp, label %if.then, label %if.else, !dbg !29
+
+if.then:  ; preds = %entry
+  store i32 1, i32* %retval, align 4, !dbg !30
+  br label %return, !dbg !30
+
+if.else:  ; preds = %entry
+  store i32 1, i32* %retval, align 4, !dbg !31
+  br label %return, !dbg !31
+
+return:   ; preds = %if.else, %if.then
+  %2 = load i32, i32* %retval, align 4, !dbg !32
+  ret i32 %2, !dbg !32
+}
+
+; Function Attrs: nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata) #1
+
+; Function Attrs: noinline norecurse optnone uwtable
+define dso_local i32 @main() #2 !dbg !33 {
+entry:
+  %retval = alloca i32, align 4
+  %f = alloca %class.myClass, align 4
+  store i32 0, i32* %retval, align 4
+  call void @llvm.dbg.declare(metadata %class.myClass* %f, metadata !36, metadata !DIExpression()), !dbg !37
+  call void @_ZN7myClassC2Eii(%class.myClass* %f, i32 3, i32 5), !dbg !37
+  ret i32 0, !dbg !38
+}
+
+; Function Attrs: noinline nounwind optnone uwtable
+define linkonce_odr dso_local void @_ZN7myClassC2Eii(%class.myClass* %this, i32 %a, i32 %b) unnamed_addr #0 comdat align 2 !dbg !39 {
+entry:
+  %this.addr = alloca %class.myClass*, align 8
+  %a.addr = alloca i32, align 4
+  %b.addr = alloca i32, align 4
+  store %class.myClass* %this, %class.myClass** %this.addr, align 8
+  call void @llvm.dbg.declare(metadata %class.myClass** %this.addr, metadata !40, metadata !DIExpression()), !dbg !41
+  store i32 %a, i32* %a.addr, align 4
+  call void @llvm.dbg.declare(metadata i32* %a.addr, metadata !42, metadata !DIExpression()), !dbg !43
+  store i32 %b, i32* %b.addr, align 4
+  call void @llvm.dbg.declare(metadata i32* %b.addr, metadata !44, metadata !DIExpression()), !dbg !45
+  %this1 = load %class.myClass*, %class.myClass** %this.addr, align 8
+  %0 = load i32, i32* %a.addr, align 4, !dbg !46
+  %low = getelementptr inbounds %class.myClass, %class.myClass* %this1, i32 0, i32 0, !dbg !48
+  store i32 %0, i32* %low, align 4, !dbg !49
+  %1 = load i32, i32* %b.addr, align 4, !dbg !50
+  %high = 

[PATCH] D70520: [WebAssembly] Add new `export_name` clang attribute for controlling wasm export names

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

> Do you think that setting this attribute should also prevent GC?

That sounds right to me.

> I could split this up into clang, llvm, and lld parts if it makes reviewing 
> simpler.

For me, it's easier to have it all together in one.

In this patch, `export_name` has a required string operand, which makes sense 
to me. I wonder if it also makes sense to change wasm-ld's `--export` 
command-line option to take a pair of strings, a symbol and a name to export it 
as.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70520



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

[PATCH] D70520: [WebAssembly] Add new `export_name` clang attribute for controlling wasm export names

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

It appears this doesn't handle exporting an imported function yet, which is 
fine for now, but it would be good to issue a warning, because wasm itself is 
capable of representing this:

  void aaa(void) __attribute__((import_module("imp"), import_name("foo"), 
export_name("bar")));




Comment at: lld/wasm/Writer.cpp:523
+  StringRef exportName = f->function->getExportName();
+  if (!exportName.empty()) {
+name = exportName;

For wasm exports, it's valid to have empty strings. In fact, I may even have a 
usecase which wants an empty-string export. It'd be good to use `Optional<>` 
for export names, rather than special-casing the empty string.

(wasm-ld does often special-case the empty string in symbol names, but wasm 
export strings aren't ordinary symbol names, so it'd be good to follow wasm's 
rules for them.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70520



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

[clang] fec3ca7 - [clang][IFS][test] GreenDragon and Fuchsia Darwin bot fix: BindArchClass Nest.

[Puyan Lotfi via cfe-commits]

Author: Puyan Lotfi
Date: 2019-11-20T22:12:28-05:00
New Revision: fec3ca77bbce6917c103963b7b85b60dfb865c7b

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

LOG: [clang][IFS][test] GreenDragon and Fuchsia Darwin bot fix: BindArchClass 
Nest.

On Darwin the clang driver does not invoke Driver::BuildActions directly
due to the need to handle Universal apps. Because of this there is a
difference in code path where Driver::BuildUniversalActions is called
instead of Driver::BuildActions. BuildUniversalActions ends up calling
Driver::BuildAction but what it does differently is it takes the driver
actions returned and wraps them each into a BindArchAction.

In Driver::BuildJobs there is a check for '-o' to determine that
multiple files are not specified when passing -o, except for Clang
Interface Stub this need to be an exception as we actually want to write
out multiple files: for every libfoo.so we have a libfoo.ifso sidecar
ifso file, etc. To allow this to happen there is a check for
IfsMergeAction, which is permitted to write out a secondary file. Except
on Darwin, the IfsMergeAction gets wrapped in the BindArchAction by
Driver::BuildUniversalActions so the check fails.

This patch is to look inside a BindArchAction in Driver::BuildJobs to
determine if there is in fact an IfsMergeAction, and if-so (pun intended)
allow the secondary sidecard ifs/ifso file to be written out.

Added: 


Modified: 
clang/lib/Driver/Driver.cpp

Removed: 




diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 83b5db3b2530..90f3cea5b2af 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -3698,7 +3698,9 @@ void Driver::BuildJobs(Compilation ) const {
 unsigned NumOutputs = 0;
 for (const Action *A : C.getActions())
   if (A->getType() != types::TY_Nothing &&
-  A->getKind() != Action::IfsMergeJobClass)
+  !(A->getKind() == Action::IfsMergeJobClass ||
+(A->getKind() == Action::BindArchClass && A->getInputs().size() &&
+ A->getInputs().front()->getKind() == Action::IfsMergeJobClass)))
 ++NumOutputs;
 
 if (NumOutputs > 1) {



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

[PATCH] D70274: [clang][IFS] Driver pipeline change for clang-ifs: generate interface stubs after standard pipeline.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

I have a fix. About to push.

In D70274#1754306 , @leonardchan wrote:

> Hi! I think this patch might be causing some test failures on our mac bots:
>
>   FAIL: Clang :: InterfaceStubs/merge-conflict-test.c (6268 of 16220)
>    TEST 'Clang :: InterfaceStubs/merge-conflict-test.c' 
> FAILED 
>   Script:
>   --
>   : 'RUN: at line 1';   not 
> /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/clang 
> -fvisibility=default -o libfoo.so -emit-interface-stubs 
> /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/merge-conflict-test.c 
> /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/driver-test.c 2>&1 | 
> /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/FileCheck 
> /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/merge-conflict-test.c
>   --
>   Exit Code: 1
>  
>   Command Output (stderr):
>   --
>   
> /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/merge-conflict-test.c:2:11:
>  error: CHECK: expected string not found in input
>   // CHECK: error: Interface Stub: Type Mismatch
> ^
>   :1:1: note: scanning from here
>   clang-10: error: cannot specify -o when generating multiple output files
>   ^
>   :1:11: note: possible intended match here
>   clang-10: error: cannot specify -o when generating multiple output files
> ^
>  
>   --
>  
>   
>   Testing:  0.. 10.. 20.. 30..
>   FAIL: Clang :: InterfaceStubs/object-float.c (6276 of 16220)
>    TEST 'Clang :: InterfaceStubs/object-float.c' FAILED 
> 
>   Script:
>   --
>   : 'RUN: at line 1';   not 
> /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/clang 
> -fvisibility=default -o - -emit-interface-stubs 
> /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object-float.c 
> /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object.c 2>&1 | 
> /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/FileCheck 
> /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object-float.c
>   --
>   Exit Code: 1
>  
>   Command Output (stderr):
>   --
>   /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object-float.c:2:11: 
> error: CHECK: expected string not found in input
>   // CHECK: error: Interface Stub: Size Mismatch
> ^
>   :1:1: note: scanning from here
>   clang-10: error: cannot specify -o when generating multiple output files
>   ^
>   :1:11: note: possible intended match here
>   clang-10: error: cannot specify -o when generating multiple output files
> ^
>  
>   --
>  
>   
>   Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
>  
>   Testing Time: 131.42s
>   
>   Failing Tests (2):
>   Clang :: InterfaceStubs/merge-conflict-test.c
>   Clang :: InterfaceStubs/object-float.c
>
>
> Could you look into it? Thanks.
>
> Bot link: 
> https://ci.chromium.org/p/fuchsia/builders/ci/clang-mac-x64/b8896234755488932160





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70274



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:105
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();

dschuff wrote:
> This chain is slightly confusing. I assume this `getValue()` captures the 
> number in `-O3`, `-O2`, etc? But why then do we need special-casing for 0 and 
> 4 above?
> 
> For that matter, we should probably not run wasm-opt at all if the opt-level 
> is 0, right?
This isn't a wasm thing; O0 and O4 [are 
special](https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Driver/Options.td#L393).
 See also 
[here](https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/ToolChains/Cuda.cpp#L368)
 and 
[here](https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/ToolChains/HIP.cpp#L97)
 for other code which does similar things. The wasm-opt version here is 
actually simpler because we don't need to special-case Os or Oz.

The `if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {` guards against the 
case where no -O option is given, but you're right, we shouldn't run wasm-opt 
if -O0 is given. I'll update the patch.



Comment at: clang/test/Driver/wasm-toolchain-lto.c:6
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto/

sbc100 wrote:
> Include the final path segment in the expectation?
It's a git revision, so it'd be constantly changing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish updated this revision to Diff 230364.
sunfish added a comment.

Don't run wasm-opt with -O0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/test/Driver/wasm-toolchain-lto.c


Index: clang/test/Driver/wasm-toolchain-lto.c
===
--- /dev/null
+++ clang/test/Driver/wasm-toolchain-lto.c
@@ -0,0 +1,6 @@
+// A basic C link command-line with optimization with known OS and LTO enabled.
+
+// RUN: %clang -### -O2 -flto -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto/
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -8,6 +8,7 @@
 
 #include "WebAssembly.h"
 #include "CommonArgs.h"
+#include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
@@ -90,6 +91,31 @@
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(std::make_unique(JA, *this, Linker, CmdArgs, Inputs));
+
+  // When optimizing, if wasm-opt is in the PATH, run wasm-opt.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (llvm::ErrorOr WasmOptPath =
+   llvm::sys::findProgramByName("wasm-opt")) {
+  StringRef OOpt = "s";
+  if (A->getOption().matches(options::OPT_O4) ||
+  A->getOption().matches(options::OPT_Ofast))
+OOpt = "4";
+  else if (A->getOption().matches(options::OPT_O0))
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();
+
+  if (OOpt != "0") {
+const char *WasmOpt = Args.MakeArgString(*WasmOptPath);
+ArgStringList CmdArgs;
+CmdArgs.push_back(Output.getFilename());
+CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
+CmdArgs.push_back("-o");
+CmdArgs.push_back(Output.getFilename());
+C.addCommand(std::make_unique(JA, *this, WasmOpt, CmdArgs, 
Inputs));
+  }
+}
+  }
 }
 
 WebAssembly::WebAssembly(const Driver , const llvm::Triple ,
@@ -109,6 +135,16 @@
   } else {
 const std::string MultiarchTriple =
 getMultiarchTriple(getDriver(), Triple, getDriver().SysRoot);
+if (D.isUsingLTO()) {
+  auto LLVMRevision = getLLVMRevision();
+  if (!LLVMRevision.empty()) {
+// For LTO, enable use of lto-enabled sysroot libraries too, if 
available.
+// Note that the directory is keyed to the LLVM revision, as LLVM's
+// bitcode format is not stable.
+getFilePaths().push_back(getDriver().SysRoot + "/lib/" + 
MultiarchTriple +
+ "/llvm-lto/" + LLVMRevision);
+  }
+}
 getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple);
   }
 }


Index: clang/test/Driver/wasm-toolchain-lto.c
===
--- /dev/null
+++ clang/test/Driver/wasm-toolchain-lto.c
@@ -0,0 +1,6 @@
+// A basic C link command-line with optimization with known OS and LTO enabled.
+
+// RUN: %clang -### -O2 -flto -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto/
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -8,6 +8,7 @@
 
 #include "WebAssembly.h"
 #include "CommonArgs.h"
+#include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
@@ -90,6 +91,31 @@
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(std::make_unique(JA, *this, Linker, CmdArgs, Inputs));
+
+  // When optimizing, if wasm-opt is in the PATH, run wasm-opt.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (llvm::ErrorOr WasmOptPath =
+   llvm::sys::findProgramByName("wasm-opt")) {
+  StringRef OOpt = "s";
+  if (A->getOption().matches(options::OPT_O4) ||
+  A->getOption().matches(options::OPT_Ofast))
+OOpt = "4";
+  else if (A->getOption().matches(options::OPT_O0))
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();
+
+  if (OOpt != "0") {
+const char *WasmOpt = Args.MakeArgString(*WasmOptPath);
+ArgStringList 

[PATCH] D69620: Add AIX assembler support

[Steven Wan via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:28
+  const char *LinkingOutput) const {
+  claimNoWarnArgs(Args);
+  ArgStringList CmdArgs;

stevewan wrote:
> Xiangling_L wrote:
> > The definition of `claimNoWarnArgs` is to suppress warnings for some 
> > options if they are unused, can you explain a little bit about how did you 
> > figure out that we don't want warnings for those?
> > 
> > Some context of `claimNoWarnArgs`:
> > 
> > ```
> > // Claim options we don't want to warn if they are unused. We do this for
> > // options that build systems might add but are unused when assembling or 
> > only
> > // running the preprocessor for example.
> > void tools::claimNoWarnArgs(const ArgList ) {
> >   // Don't warn about unused -f(no-)?lto.  This can happen when we're
> >   // preprocessing, precompiling or assembling.
> >   Args.ClaimAllArgs(options::OPT_flto_EQ);
> >   Args.ClaimAllArgs(options::OPT_flto);
> >   Args.ClaimAllArgs(options::OPT_fno_lto);
> > }
> > ```
> > 
> The original reason was that, since we are doing assembling here, and as 
> stated in `claimNoWarnArgs`, there might be an unused `-f(no-)?lto` when 
> we're assembling, and we'd like to suppress the warning(s) for that. 
> 
> Looking into it, the three options [[ 
> https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-flto
>  | `flto`, `fno_lto` ]] , and [[ 
> https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang1-flto
>  | `flto_EQ` ]] are used to enable/disable link time optimization, but LTO is 
> not supported by the AIX system linker. Driver would just throw error if the 
> user passes it `-flto` or `-flto=` on AIX, so claiming them or not 
> currently makes no actual difference as far as I'm concerned. 
> 
> That being said, I don't have a strong opinion either way. Let's see how 
> other people think.
It's probably also worth mentioning that, adding `claimNoWarnArgs` does not 
affect those LTO options being parsed and consumed by the driver in 
`Driver::setLTOMode`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69620



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

[PATCH] D69620: Add AIX assembler support

[Steven Wan via Phabricator via cfe-commits]

stevewan updated this revision to Diff 230360.
stevewan marked an inline comment as done.
stevewan added a comment.

Add "-u" to accept undefined symbol as extern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69620

Files:
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/lib/Driver/ToolChains/AIX.h
  clang/test/Driver/aix-as.c

Index: clang/test/Driver/aix-as.c
===
--- /dev/null
+++ clang/test/Driver/aix-as.c
@@ -0,0 +1,52 @@
+// General tests that as invocations on AIX targets are sane. Note that we
+// only test assembler functionalities in this suite.
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit.
+// RUN: %clang -no-canonical-prefixes %s -### -c -o %t.o 2>&1 \
+// RUN: -target powerpc-ibm-aix7.1.0.0 \
+// RUN:   | FileCheck --check-prefix=CHECK-AS32 %s
+// CHECK-AS32-NOT: warning:
+// CHECK-AS32: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-AS32: "{{.*}}as{{(.exe)?}}" 
+// CHECK-AS32: "-a32" 
+// CHECK-AS32: "-u" 
+// CHECK-AS32: "-many" 
+
+// Check powerpc64-ibm-aix7.1.0.0, 64-bit.
+// RUN: %clang -no-canonical-prefixes %s -### -c -o %t.o 2>&1 \
+// RUN: -target powerpc64-ibm-aix7.1.0.0 \
+// RUN:   | FileCheck --check-prefix=CHECK-AS64 %s
+// CHECK-AS64-NOT: warning:
+// CHECK-AS64: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0"
+// CHECK-AS64: "{{.*}}as{{(.exe)?}}" 
+// CHECK-AS64: "-a64" 
+// CHECK-AS64: "-u" 
+// CHECK-AS64: "-many"
+
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -Xassembler  option. 
+// RUN: %clang -no-canonical-prefixes %s -### -c -o %t.o 2>&1 \
+// RUN: -Xassembler -w \
+// RUN: -target powerpc-ibm-aix7.1.0.0 \
+// RUN:   | FileCheck --check-prefix=CHECK-AS32-PTHREAD %s
+// CHECK-AS32-PTHREAD-NOT: warning:
+// CHECK-AS32-PTHREAD: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-AS32-PTHREAD: "{{.*}}as{{(.exe)?}}" 
+// CHECK-AS32-PTHREAD: "-a32" 
+// CHECK-AS32-PTHREAD: "-u" 
+// CHECK-AS32-PTHREAD: "-many"
+// CHECK-AS32-PTHREAD: "-w"
+
+// Check powerpc-ibm-aix7.1.0.0, 64-bit. -Wa,, option.
+// RUN: %clang -no-canonical-prefixes %s -### -c -o %t.o 2>&1 \
+// RUN: -Wa,-v,-w \
+// RUN: -target powerpc64-ibm-aix7.1.0.0 \
+// RUN:   | FileCheck --check-prefix=CHECK-AS64-PTHREAD %s
+// CHECK-AS64-PTHREAD-NOT: warning:
+// CHECK-AS64-PTHREAD: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0"
+// CHECK-AS64-PTHREAD: "{{.*}}as{{(.exe)?}}" 
+// CHECK-AS64-PTHREAD: "-a64" 
+// CHECK-AS64-PTHREAD: "-u" 
+// CHECK-AS64-PTHREAD: "-many"
+// CHECK-AS64-PTHREAD: "-v"
+// CHECK-AS64-PTHREAD: "-w"
Index: clang/lib/Driver/ToolChains/AIX.h
===
--- clang/lib/Driver/ToolChains/AIX.h
+++ clang/lib/Driver/ToolChains/AIX.h
@@ -16,10 +16,21 @@
 namespace driver {
 namespace tools {
 
-/// aix -- Directly call system default linker.
-// TODO: Enable direct call to system default assembler.
+/// aix -- Directly call system default assembler and linker.
 namespace aix {
 
+class LLVM_LIBRARY_VISIBILITY Assembler : public Tool {
+public:
+  Assembler(const ToolChain ) : Tool("aix::Assembler", "assembler", TC) {}
+
+  bool hasIntegratedCPP() const override { return false; }
+
+  void ConstructJob(Compilation , const JobAction ,
+const InputInfo , const InputInfoList ,
+const llvm::opt::ArgList ,
+const char *LinkingOutput) const override;
+};
+
 class LLVM_LIBRARY_VISIBILITY Linker : public Tool {
 public:
   Linker(const ToolChain ) : Tool("aix::Linker", "linker", TC) {}
@@ -53,6 +64,7 @@
   bool isPICDefaultForced() const override { return true; }
 
 protected:
+  Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
 };
 
Index: clang/lib/Driver/ToolChains/AIX.cpp
===
--- clang/lib/Driver/ToolChains/AIX.cpp
+++ clang/lib/Driver/ToolChains/AIX.cpp
@@ -20,6 +20,52 @@
 
 using namespace llvm::opt;
 
+void aix::Assembler::ConstructJob(Compilation , const JobAction ,
+  const InputInfo ,
+  const InputInfoList ,
+  const ArgList ,
+  const char *LinkingOutput) const {
+  claimNoWarnArgs(Args);
+  ArgStringList CmdArgs;
+
+  const bool IsArch32Bit = getToolChain().getTriple().isArch32Bit();
+  const bool IsArch64Bit = getToolChain().getTriple().isArch64Bit();
+  // Only support 32 and 64 bit.
+  if (!IsArch32Bit && !IsArch64Bit)
+llvm_unreachable("Unsupported bit width value.");
+
+  // Specify the mode in which the as command operates.
+  if (IsArch32Bit) {
+CmdArgs.push_back("-a32");
+  } else {
+// Must be 64-bit, otherwise asserted already.
+CmdArgs.push_back("-a64");
+ 

Re: [clang] 7342912 - [clang][IFS] Driver Pipeline: generate stubs after standard pipeline (3)

[Puyan Lotfi via cfe-commits]

I have a solution, stay tuned.

So it turns out since the clang driver has special behavior on Darwin for 
handling Universal apps, there is a different code path that happens during 
BuildActions. clang::driver::Driver::BuildUniversalActions is called instead of 
BuildActions on Darwin, and what BuildUniversalActions ends up doing is it 
calls BuildActions and takes the driver actions returned and wraps them all 
into a BindArchAction each. So when I check for a IfsMergeAction in the driver 
instead I am getting a BindArchAction that has an IfsMergeAction inside of it.

I will post a fix to get Green Dragon and the Fuchsia Darwin bots green again, 
and also post a post-commit review on Phab for good measure. 

Thanks Again for the heads up Alex.

Puyan

Sent with ProtonMail Secure Email.

‐‐‐ Original Message ‐‐‐
On Wednesday, November 20, 2019 4:52 PM, Puyan Lotfi  wrote:

> Looking into this. Thanks for the heads up. 
> 

> PL
> 

> Sent with ProtonMail Secure Email.
> 

> ‐‐‐ Original Message ‐‐‐
> On Wednesday, November 20, 2019 4:12 PM, Alex L  wrote:
> 

> > Hi Puyan,
> > 

> > This commit caused two Clang failures on Darwin:
> > 

> >     Clang :: InterfaceStubs/merge-conflict-test.c
> >     Clang :: InterfaceStubs/object-float.c
> > 

> > Here's the build log from out bot:
> > http://lab.llvm.org:8080/green/job/clang-stage1-RA/3929/console
> > 

> > Can you please resolve the issue with the tests?
> > Thanks,
> > Alex
> > 

> > On Wed, 20 Nov 2019 at 14:16, Puyan Lotfi via cfe-commits 
> >  wrote:
> > 

> > > Author: Puyan Lotfi
> > > Date: 2019-11-20T16:22:50-05:00
> > > New Revision: 73429126c91c2065c6f6ef29b3eec1b7798502bb
> > > 

> > > URL: 
> > > https://github.com/llvm/llvm-project/commit/73429126c91c2065c6f6ef29b3eec1b7798502bb
> > > DIFF: 
> > > https://github.com/llvm/llvm-project/commit/73429126c91c2065c6f6ef29b3eec1b7798502bb.diff
> > > 

> > > LOG: [clang][IFS] Driver Pipeline: generate stubs after standard pipeline 
> > > (3)
> > > 

> > > Third Landing Attempt (dropping any linker invocation from clang driver):
> > > 

> > > Up until now, clang interface stubs has replaced the standard
> > > PP -> C -> BE -> ASM -> LNK pipeline. With this change, it will happen in
> > > conjunction with it. So what when you build your code you will get an
> > > a.out or lib.so as well as an interface stub file.
> > > 

> > > Example:
> > > 

> > > clang -shared -o libfoo.so -emit-interface-stubs ...
> > > 

> > > will generate both a libfoo.so and a libfoo.ifso. The .so file will
> > > contain the code from the standard compilation pipeline and the .ifso
> > > file will contain the ELF stub library.
> > > 

> > > Note: For driver-test.c I've added -S in order to prevent any bot 
> > > failures on
> > > bots that don't have the proper linker for their native triple. You could 
> > > always
> > > specify a triple like x86_64-unknown-linux-gnu and on bots like 
> > > x86_64-scei-ps4
> > > the clang driver would invoke regular ld instead of getting the error
> > > 'Executable "orbis-ld" doesn't exist!' but on bots like ppc64be and s390x 
> > > you'd
> > > get an error "/usr/bin/ld: unrecognised emulation mode: elf_x86_64"
> > > 

> > > Differential Revision: https://reviews.llvm.org/D70274
> > > 

> > > Added:
> > >     clang/test/InterfaceStubs/driver-test2.c
> > >     clang/test/InterfaceStubs/ppc.cpp
> > > 

> > > Modified:
> > >     clang/lib/Driver/Driver.cpp
> > >     clang/lib/Driver/ToolChains/InterfaceStubs.cpp
> > >     clang/lib/Driver/Types.cpp
> > >     clang/test/InterfaceStubs/driver-test.c
> > >     clang/test/InterfaceStubs/windows.cpp
> > > 

> > > Removed:
> > > 

> > > 
> > > diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
> > > index cdf4a579f431..83b5db3b2530 100644
> > > --- a/clang/lib/Driver/Driver.cpp
> > > +++ b/clang/lib/Driver/Driver.cpp
> > > @@ -292,10 +292,6 @@ phases::ID Driver::getFinalPhase(const 
> > > DerivedArgList ,
> > >               (PhaseArg = DAL.getLastArg(options::OPT_emit_ast))) {
> > >      FinalPhase = phases::Compile;
> > > 

> > > -  // clang interface stubs
> > > -  } else if ((PhaseArg = 
> > > DAL.getLastArg(options::OPT_emit_interface_stubs))) {
> > > -    FinalPhase = phases::IfsMerge;
> > > -
> > >    // -S only runs up to the backend.
> > >    } else if ((PhaseArg = DAL.getLastArg(options::OPT_S))) {
> > >      FinalPhase = phases::Backend;
> > > @@ -3502,6 +3498,68 @@ void Driver::BuildActions(Compilation , 
> > > DerivedArgList ,
> > >      Actions.push_back(
> > >          C.MakeAction(MergerInputs, types::TY_Image));
> > > 

> > > +  if (Arg *A = Args.getLastArg(options::OPT_emit_interface_stubs)) {
> > > +    llvm::SmallVector PhaseList;
> > > +    if (Args.hasArg(options::OPT_c)) {
> > > +      llvm::SmallVector 
> > > CompilePhaseList;
> > > +      types::getCompilationPhases(types::TY_IFS_CPP, CompilePhaseList);
> > > +  

[PATCH] D69620: Add AIX assembler support

[Steven Wan via Phabricator via cfe-commits]

stevewan marked 4 inline comments as done.
stevewan added inline comments.



Comment at: clang/lib/Driver/ToolChains/AIX.cpp:28
+  const char *LinkingOutput) const {
+  claimNoWarnArgs(Args);
+  ArgStringList CmdArgs;

Xiangling_L wrote:
> The definition of `claimNoWarnArgs` is to suppress warnings for some options 
> if they are unused, can you explain a little bit about how did you figure out 
> that we don't want warnings for those?
> 
> Some context of `claimNoWarnArgs`:
> 
> ```
> // Claim options we don't want to warn if they are unused. We do this for
> // options that build systems might add but are unused when assembling or only
> // running the preprocessor for example.
> void tools::claimNoWarnArgs(const ArgList ) {
>   // Don't warn about unused -f(no-)?lto.  This can happen when we're
>   // preprocessing, precompiling or assembling.
>   Args.ClaimAllArgs(options::OPT_flto_EQ);
>   Args.ClaimAllArgs(options::OPT_flto);
>   Args.ClaimAllArgs(options::OPT_fno_lto);
> }
> ```
> 
The original reason was that, since we are doing assembling here, and as stated 
in `claimNoWarnArgs`, there might be an unused `-f(no-)?lto` when we're 
assembling, and we'd like to suppress the warning(s) for that. 

Looking into it, the three options [[ 
https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-flto 
| `flto`, `fno_lto` ]] , and [[ 
https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang1-flto
 | `flto_EQ` ]] are used to enable/disable link time optimization, but LTO is 
not supported by the AIX system linker. Driver would just throw error if the 
user passes it `-flto` or `-flto=` on AIX, so claiming them or not 
currently makes no actual difference as far as I'm concerned. 

That being said, I don't have a strong opinion either way. Let's see how other 
people think.



Comment at: clang/lib/Driver/ToolChains/AIX.h:26
+
+  bool hasIntegratedCPP() const override { return false; }
+

daltenty wrote:
> stevewan wrote:
> > Xiangling_L wrote:
> > > I saw a lot of other target also set `hasIntegratedCPP()` as false, but I 
> > > didn't find any explanation in documentation, so I am curious that what 
> > > this is about?
> > I also failed to find useful resources that explains the purpose of this 
> > function. The main rationales of adding it were essentially that,
> > 1. It's a pure virtual function in the base `Tool` class,
> > 2. Most if not all of other targets had overridden this function such that 
> > it returns false.
> > 
> > I'll leave this comment open, and see if someone could enlighten us. 
> Only "Compiler" tools set hasIntegratedCPP() to true. Looking into it, it 
> seems combineWithPreprocessor() uses this to decide whether the tool supports 
> preprocessor actions so it may fold them in to one step.  I think we are safe 
> leaving this as is.
I checked this function on other targets. As David pointed out, only the 
compiler tools would set hasIntegratedCPP() to true, the assembler and linker 
tools set it to false because they do not support combining with preprocessor 
action.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69620



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added inline comments.



Comment at: clang/test/Driver/wasm-toolchain-lto.c:6
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto/

Include the final path segment in the expectation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

LGTM on the approach, just one more question on the wasm-opt flags.




Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:105
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();

This chain is slightly confusing. I assume this `getValue()` captures the 
number in `-O3`, `-O2`, etc? But why then do we need special-casing for 0 and 4 
above?

For that matter, we should probably not run wasm-opt at all if the opt-level is 
0, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70520: [WebAssembly] Add new `export_name` clang attribute for controlling wasm export names

[Sam Clegg via Phabricator via cfe-commits]

sbc100 created this revision.
Herald added subscribers: llvm-commits, cfe-commits, sunfish, aheejin, 
hiraditya, jgravelle-google, dschuff.
Herald added projects: clang, LLVM.
sbc100 added a comment.
sbc100 added reviewers: dschuff, sunfish.

First stab at getting this attribute plumbed all the way through.

Do you think that setting this attribute should also prevent GC?

I could split this up into clang, llvm, and lld parts if it makes reviewing 
simpler.


This is equivalent to the existing `import_name` and `import_module`
attributes which control the import names in the final wasm binary
produced by lld.

This maps the existing

This attribute currently requires a string rather than using the
symbol name for a couple of reasons:

1. Avoid confusion with static and dynamic linking which is based on symbol 
name.  Exporting a function from a wasm module using this directive is 
orthogonal to both static and dynamic linking.
2. Avoids name mangling.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70520

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/wasm-export-name.c
  lld/test/wasm/export-name.ll
  lld/test/wasm/import-name.ll
  lld/test/wasm/import-names.ll
  lld/wasm/InputChunks.h
  lld/wasm/Writer.cpp
  llvm/include/llvm/BinaryFormat/Wasm.h
  llvm/include/llvm/MC/MCSymbolWasm.h
  llvm/include/llvm/Object/Wasm.h
  llvm/lib/MC/WasmObjectWriter.cpp
  llvm/lib/Object/WasmObjectFile.cpp
  llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.cpp
  llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h
  llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
  llvm/test/CodeGen/WebAssembly/export-name.ll
  llvm/test/MC/WebAssembly/export-name.ll

Index: llvm/test/MC/WebAssembly/export-name.ll
===
--- /dev/null
+++ llvm/test/MC/WebAssembly/export-name.ll
@@ -0,0 +1,24 @@
+; RUN: llc -filetype=obj %s -o - | obj2yaml | FileCheck %s
+
+target triple = "wasm32-unknown-unknown"
+
+define void @test() #0 {
+  ret void
+}
+
+attributes #0 = { "wasm-export-name"="foo" }
+
+; CHECK:- Type:EXPORT
+; CHECK-NEXT: Exports:
+; CHECK-NEXT:   - Name:foo
+; CHECK-NEXT: Kind:FUNCTION
+; CHECK-NEXT: Index:   0
+
+; CHECK:  Name:linking
+; CHECK-NEXT: Version: 2
+; CHECK-NEXT: SymbolTable:
+; CHECK-NEXT:   - Index:   0
+; CHECK-NEXT: Kind:FUNCTION
+; CHECK-NEXT: Name:test
+; CHECK-NEXT: Flags:   [ EXPORTED ]
+; CHECK-NEXT: Function:0
Index: llvm/test/CodeGen/WebAssembly/export-name.ll
===
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/export-name.ll
@@ -0,0 +1,12 @@
+; RUN: llc < %s -asm-verbose=false -wasm-keep-registers | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+define void @test() #0 {
+  ret void
+}
+
+attributes #0 = { "wasm-export-name"="foo" }
+
+; CHECK: .export_name test, foo
Index: llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp
@@ -290,6 +290,12 @@
   // FIXME: clean up how params and results are emitted (use signatures)
   getTargetStreamer()->emitFunctionType(WasmSym);
 
+  if (F.hasFnAttribute("wasm-export-name")) {
+StringRef Name = F.getFnAttribute("wasm-export-name").getValueAsString();
+WasmSym->setExportName(Name);
+getTargetStreamer()->emitExportName(WasmSym, Name);
+  }
+
   // Emit the function index.
   if (MDNode *Idx = F.getMetadata("wasm.index")) {
 assert(Idx->getNumOperands() == 1);
Index: llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h
===
--- llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h
+++ llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h
@@ -48,6 +48,9 @@
   /// .import_name
   virtual void emitImportName(const MCSymbolWasm *Sym,
   StringRef ImportName) = 0;
+  /// .export_name
+  virtual void emitExportName(const MCSymbolWasm *Sym,
+  StringRef ExportName) = 0;
 
 protected:
   void emitValueType(wasm::ValType Type);
@@ -68,6 +71,7 @@
   void emitEventType(const MCSymbolWasm *Sym) override;
   void emitImportModule(const MCSymbolWasm *Sym, StringRef ImportModule) override;
   void emitImportName(const MCSymbolWasm *Sym, StringRef ImportName) override;
+  void emitExportName(const 

[PATCH] D70520: [WebAssembly] Add new `export_name` clang attribute for controlling wasm export names

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

First stab at getting this attribute plumbed all the way through.

Do you think that setting this attribute should also prevent GC?

I could split this up into clang, llvm, and lld parts if it makes reviewing 
simpler.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70520



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

[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp:19-20
+// handle variable is passed to different function calls or syscalls, its state
+// changes. The state changes can be generally represented by following ASCII
+// Art:
+//

Btw, this state machine is fairly common. Both MallocChecker and 
SimpleStreamChecker already follow this same model. Do you have any thoughts on 
re-using it in an abstract manner?

Something like this maybe?:
```lang=c++
template 
class SimpleStreamStateMachine {
public:
  SimpleStreamStateMachine(void(*pointerEscapePolicy)(CheckerContext C, Other 
Customizations));

  ProgramStateRef makeOpened(ProgramStateRef State, SymbolRef Key) {
return State->set(Key, TraitID::ValueTy::makeOpened());
  }
  // makeReleased and so on pre-defined for all users,
  // allowing customization when necessary.
};

class SimpleStreamStateMachineBookkeeping : Checker {
public:
  checkDeadSymbols() {
// Perform the state cleanup for all concrete machines
// ever instantiated, in the only possible way, probably
// invoke callbacks for leaks.
  }
  checkPointerEscape() {
// Invoke the passed-down policy for each concrete
// state machine.
  }
  // Other callbacks are implemented in the dependent checker.
};
```
And then:
```lang=c++
REGISTER_MAP_WITH_PROGRAMSTATE(MyTrait, SymbolRef, MyState);

class MyChecker : Checker {
  SimpleStreamStateMachine StM {
  getStateMachine>(
  ::checkPointerEscapeImpl, /*Other Customizations*/)};
  void checkPointerEscapeImpl(...);

public:
  checkPostCall(...) {
C.addTransition(StM.makeOpened(C.getState(), Call.getRetVal()));
  }
};
```

'Cause i have this pipe dream that we make a lot of such abstract state 
machines and then we'll never need to write more of them and that'll make it 
cheaper to introduce non-trivial operations over the program state such as 
replacing values or advanced widening because we'll only have to implement them 
in the few state machines rather than in many checkers.


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

https://reviews.llvm.org/D70470



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

[PATCH] D70411: [analyzer][WIP] CERT: StrChecker: 31.c

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:170
+Report->addVisitor(
+allocation_state::getMallocBRVisitor(DestV.getAsSymbol()));
+  } else {

We can do the opposite to see whether the destination array's type is an array, 
so that we do not need `getDynamicSizeExpr`, yay.


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

https://reviews.llvm.org/D70411



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

[PATCH] D70411: [analyzer][WIP] CERT: StrChecker: 31.c

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D70411#1754356 , @NoQ wrote:

> I think it would really help if you draw a state machine for the checker, 
> like the ASCII-art thing in D70470 ; you 
> don't need to spend a lot of time turning it into ASCII-art, a photo of a 
> quick hand-drawn picture would be totally fine, because it's, first and 
> foremost, for discussion :)


Hm, the idea is cool, but my checker is not that complex, given that now I have 
added comments. Thanks, I will adjust the comments with some kind of drawing.

My main problem was that to create a `NoteTag` when the not null-terminated 
string is read, but it is after when we emit an error, so I could not emit a 
note. That is why it emits two different reports, and somehow I need to convert 
the function call evaluation warning to a note in case when a not 
null-terminated string is read. Do we have any plans with `NoteTags` to support 
the craziest checkers? What do you think about storing the reports in the 
program state?




Comment at: clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp:279
+if (PredNode->getState() == ErrorNode->getState()) {
+  IsFalsePositiveFound = true;
+  PR->markInvalid(nullptr, nullptr);

NoQ wrote:
> Why is this a false positive?
> 
> You're bringing in a completely brand-new machinery here, could you explain 
> how it works and why do you need it?
Hm, yes, I wanted to add comments earlier, sorry. It is still wonky a 
little-bit, somehow I need to merge the two different errors.


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

https://reviews.llvm.org/D70411



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

[PATCH] D70411: [analyzer][WIP] CERT: StrChecker: 31.c

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 230354.
Charusso marked 2 inline comments as done.
Charusso retitled this revision from "[analyzer][WIP] StrChecker: 31.c" to 
"[analyzer][WIP] CERT: StrChecker: 31.c".
Charusso added a comment.

- Added a report when the not null-terminated string is read.
- Added comments.
- Fixed some uninteresting nits.
- No ASCII-art yet.


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

https://reviews.llvm.org/D70411

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSizeInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
  clang/lib/StaticAnalyzer/Checkers/AllocationState.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
  clang/test/Analysis/Inputs/system-header-simulator.h
  clang/test/Analysis/cert/str31-c-fp-suppression.cpp
  clang/test/Analysis/cert/str31-c-notes.cpp
  clang/test/Analysis/cert/str31-c.cpp

Index: clang/test/Analysis/cert/str31-c.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/str31-c.cpp
@@ -0,0 +1,181 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix,security.cert.str.31.c \
+// RUN:  -verify %s
+
+// See the examples on the page of STR31:
+// https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator
+
+#include "../Inputs/system-header-simulator.h"
+
+#define EOF -1
+typedef __SIZE_TYPE__ size_t;
+
+void free(void *memblock);
+void *malloc(size_t size);
+
+namespace test_gets_bad {
+#define BUFFER_SIZE 1024
+
+void func(void) {
+  char buf[BUFFER_SIZE];
+  if (gets(buf)) {}
+  // expected-warning@-1 {{'gets' could write outside of 'buf'}}
+}
+} // namespace test_gets_bad
+
+namespace test_gets_good {
+enum { BUFFERSIZE = 32 };
+
+void func(void) {
+  char buff[BUFFERSIZE];
+
+  if (fgets(buff, sizeof(buff), stdin)) {}
+}
+} // namespace test_gets_good
+
+namespace test_sprintf_bad {
+void func(const char *name) {
+  char buf[128];
+  sprintf(buf, "%s.txt", name);
+  // expected-warning@-1 {{'sprintf' could write outside of 'buf'}}
+}
+} // namespace test_sprintf_bad
+
+namespace test_sprintf_good {
+void func(const char *name) {
+  char buff[128];
+  snprintf(buff, sizeof(buff), "%s.txt", name);
+}
+} // namespace test_sprintf_good
+
+namespace test_fscanf_bad {
+enum { BUF_LENGTH = 1024 };
+
+void get_data(void) {
+  char buf[BUF_LENGTH];
+  if (fscanf(stdin, "%s", buf)) {}
+  // expected-warning@-1 {{'fscanf' could write outside of 'buf'}}
+}
+} // namespace test_fscanf_bad
+
+namespace test_fscanf_good {
+enum { BUF_LENGTH = 1024 };
+
+void get_data(void) {
+  char buff[BUF_LENGTH];
+  if (fscanf(stdin, "%1023s", buff)) {}
+}
+} // namespace test_fscanf_good
+
+namespace test_strcpy_bad {
+int main(int argc, char *argv[]) {
+  const char *const name = (argc && argv[0]) ? argv[0] : "";
+  char prog_name[128];
+  strcpy(prog_name, name);
+  // expected-warning@-1 {{'strcpy' could write outside of 'prog_name'}}
+  return 0;
+}
+
+void func(void) {
+  char buff[256];
+  char *editor = getenv("EDITOR");
+  if (editor != NULL) {
+strcpy(buff, editor);
+// expected-warning@-1 {{'strcpy' could write outside of 'buff'}}
+  }
+}
+} // namespace test_strcpy_bad
+
+namespace test_strcpy_good {
+int main(int argc, char *argv[]) {
+  const char *const name = (argc && argv[0]) ? argv[0] : "";
+  char *prog_name2 = (char *)malloc(strlen(name) + 1);
+  if (prog_name2 != NULL) {
+strcpy(prog_name2, name);
+  }
+  free(prog_name2);
+  return 0;
+}
+
+void func(void) {
+  char *buff2;
+  char *editor = getenv("EDITOR");
+  if (editor != NULL) {
+size_t len = strlen(editor) + 1;
+buff2 = (char *)malloc(len);
+if (buff2 != NULL) {
+  strcpy(buff2, editor);
+}
+free(buff2);
+  }
+}
+} // namespace test_strcpy_good
+
+//===--===//
+// The following is from the rule's page which we do not handle yet.
+//===--===//
+
+namespace test_loop_index_bad {
+void copy(size_t n, char src[n], char dest[n]) {
+  size_t i;
+
+  for (i = 0; src[i] && (i < n); ++i) {
+dest[i] = src[i];
+  }
+  dest[i] = '\0';
+}
+} // namespace test_loop_index_bad
+
+namespace test_loop_index_good {
+void copy(size_t n, char src[n], char dest[n]) {
+  size_t i;
+
+  for (i = 0; src[i] && (i < n - 1); ++i) {
+dest[i] = src[i];
+  }
+  dest[i] = '\0';
+}
+} // namespace test_loop_index_good
+
+namespace test_getchar_bad {
+enum { BUFFERSIZE = 32 };

[PATCH] D70411: [analyzer][WIP] StrChecker: 31.c

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I think it would really help if you draw a state machine for the checker, like 
the ASCII-art thing in D70470 ; you don't need 
to spend a lot of time turning it into ASCII-art, a photo of a quick hand-drawn 
picture would be totally fine, because it's, first and foremost, for discussion 
:)


Repository:
  rC Clang

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

https://reviews.llvm.org/D70411



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

[PATCH] D70274: [clang][IFS] Driver pipeline change for clang-ifs: generate interface stubs after standard pipeline.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

Looking into it. This change also affected Green Dragon. Thanks for the heads 
up.

In D70274#1754306 , @leonardchan wrote:

> Hi! I think this patch might be causing some test failures on our mac bots:
>
>   FAIL: Clang :: InterfaceStubs/merge-conflict-test.c (6268 of 16220)
>    TEST 'Clang :: InterfaceStubs/merge-conflict-test.c' 
> FAILED 
>   Script:
>   --
>   : 'RUN: at line 1';   not 
> /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/clang 
> -fvisibility=default -o libfoo.so -emit-interface-stubs 
> /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/merge-conflict-test.c 
> /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/driver-test.c 2>&1 | 
> /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/FileCheck 
> /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/merge-conflict-test.c
>   --
>   Exit Code: 1
>  
>   Command Output (stderr):
>   --
>   
> /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/merge-conflict-test.c:2:11:
>  error: CHECK: expected string not found in input
>   // CHECK: error: Interface Stub: Type Mismatch
> ^
>   :1:1: note: scanning from here
>   clang-10: error: cannot specify -o when generating multiple output files
>   ^
>   :1:11: note: possible intended match here
>   clang-10: error: cannot specify -o when generating multiple output files
> ^
>  
>   --
>  
>   
>   Testing:  0.. 10.. 20.. 30..
>   FAIL: Clang :: InterfaceStubs/object-float.c (6276 of 16220)
>    TEST 'Clang :: InterfaceStubs/object-float.c' FAILED 
> 
>   Script:
>   --
>   : 'RUN: at line 1';   not 
> /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/clang 
> -fvisibility=default -o - -emit-interface-stubs 
> /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object-float.c 
> /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object.c 2>&1 | 
> /b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/FileCheck 
> /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object-float.c
>   --
>   Exit Code: 1
>  
>   Command Output (stderr):
>   --
>   /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object-float.c:2:11: 
> error: CHECK: expected string not found in input
>   // CHECK: error: Interface Stub: Size Mismatch
> ^
>   :1:1: note: scanning from here
>   clang-10: error: cannot specify -o when generating multiple output files
>   ^
>   :1:11: note: possible intended match here
>   clang-10: error: cannot specify -o when generating multiple output files
> ^
>  
>   --
>  
>   
>   Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
>  
>   Testing Time: 131.42s
>   
>   Failing Tests (2):
>   Clang :: InterfaceStubs/merge-conflict-test.c
>   Clang :: InterfaceStubs/object-float.c
>
>
> Could you look into it? Thanks.
>
> Bot link: 
> https://ci.chromium.org/p/fuchsia/builders/ci/clang-mac-x64/b8896234755488932160





Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70274



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

Re: [clang] 7342912 - [clang][IFS] Driver Pipeline: generate stubs after standard pipeline (3)

[Puyan Lotfi via cfe-commits]

Looking into this. Thanks for the heads up. 

PL

Sent with ProtonMail Secure Email.

‐‐‐ Original Message ‐‐‐
On Wednesday, November 20, 2019 4:12 PM, Alex L  wrote:

> Hi Puyan,
> 

> This commit caused two Clang failures on Darwin:
> 

>     Clang :: InterfaceStubs/merge-conflict-test.c
>     Clang :: InterfaceStubs/object-float.c
> 

> Here's the build log from out bot:
> http://lab.llvm.org:8080/green/job/clang-stage1-RA/3929/console
> 

> Can you please resolve the issue with the tests?
> Thanks,
> Alex
> 

> On Wed, 20 Nov 2019 at 14:16, Puyan Lotfi via cfe-commits 
>  wrote:
> 

> > Author: Puyan Lotfi
> > Date: 2019-11-20T16:22:50-05:00
> > New Revision: 73429126c91c2065c6f6ef29b3eec1b7798502bb
> > 

> > URL: 
> > https://github.com/llvm/llvm-project/commit/73429126c91c2065c6f6ef29b3eec1b7798502bb
> > DIFF: 
> > https://github.com/llvm/llvm-project/commit/73429126c91c2065c6f6ef29b3eec1b7798502bb.diff
> > 

> > LOG: [clang][IFS] Driver Pipeline: generate stubs after standard pipeline 
> > (3)
> > 

> > Third Landing Attempt (dropping any linker invocation from clang driver):
> > 

> > Up until now, clang interface stubs has replaced the standard
> > PP -> C -> BE -> ASM -> LNK pipeline. With this change, it will happen in
> > conjunction with it. So what when you build your code you will get an
> > a.out or lib.so as well as an interface stub file.
> > 

> > Example:
> > 

> > clang -shared -o libfoo.so -emit-interface-stubs ...
> > 

> > will generate both a libfoo.so and a libfoo.ifso. The .so file will
> > contain the code from the standard compilation pipeline and the .ifso
> > file will contain the ELF stub library.
> > 

> > Note: For driver-test.c I've added -S in order to prevent any bot failures 
> > on
> > bots that don't have the proper linker for their native triple. You could 
> > always
> > specify a triple like x86_64-unknown-linux-gnu and on bots like 
> > x86_64-scei-ps4
> > the clang driver would invoke regular ld instead of getting the error
> > 'Executable "orbis-ld" doesn't exist!' but on bots like ppc64be and s390x 
> > you'd
> > get an error "/usr/bin/ld: unrecognised emulation mode: elf_x86_64"
> > 

> > Differential Revision: https://reviews.llvm.org/D70274
> > 

> > Added:
> >     clang/test/InterfaceStubs/driver-test2.c
> >     clang/test/InterfaceStubs/ppc.cpp
> > 

> > Modified:
> >     clang/lib/Driver/Driver.cpp
> >     clang/lib/Driver/ToolChains/InterfaceStubs.cpp
> >     clang/lib/Driver/Types.cpp
> >     clang/test/InterfaceStubs/driver-test.c
> >     clang/test/InterfaceStubs/windows.cpp
> > 

> > Removed:
> > 

> > 
> > diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
> > index cdf4a579f431..83b5db3b2530 100644
> > --- a/clang/lib/Driver/Driver.cpp
> > +++ b/clang/lib/Driver/Driver.cpp
> > @@ -292,10 +292,6 @@ phases::ID Driver::getFinalPhase(const DerivedArgList 
> > ,
> >               (PhaseArg = DAL.getLastArg(options::OPT_emit_ast))) {
> >      FinalPhase = phases::Compile;
> > 

> > -  // clang interface stubs
> > -  } else if ((PhaseArg = 
> > DAL.getLastArg(options::OPT_emit_interface_stubs))) {
> > -    FinalPhase = phases::IfsMerge;
> > -
> >    // -S only runs up to the backend.
> >    } else if ((PhaseArg = DAL.getLastArg(options::OPT_S))) {
> >      FinalPhase = phases::Backend;
> > @@ -3502,6 +3498,68 @@ void Driver::BuildActions(Compilation , 
> > DerivedArgList ,
> >      Actions.push_back(
> >          C.MakeAction(MergerInputs, types::TY_Image));
> > 

> > +  if (Arg *A = Args.getLastArg(options::OPT_emit_interface_stubs)) {
> > +    llvm::SmallVector PhaseList;
> > +    if (Args.hasArg(options::OPT_c)) {
> > +      llvm::SmallVector 
> > CompilePhaseList;
> > +      types::getCompilationPhases(types::TY_IFS_CPP, CompilePhaseList);
> > +      llvm::copy_if(CompilePhaseList, std::back_inserter(PhaseList),
> > +                    [&](phases::ID Phase) { return Phase <= 
> > phases::Compile; });
> > +    } else {
> > +      types::getCompilationPhases(types::TY_IFS_CPP, PhaseList);
> > +    }
> > +
> > +    ActionList MergerInputs;
> > +
> > +    for (auto  : Inputs) {
> > +      types::ID InputType = I.first;
> > +      const Arg *InputArg = I.second;
> > +
> > +      // Currently clang and the llvm assembler do not support generating 
> > symbol
> > +      // stubs from assembly, so we skip the input on asm files. For ifs 
> > files
> > +      // we rely on the normal pipeline setup in the pipeline setup code 
> > above.
> > +      if (InputType == types::TY_IFS || InputType == types::TY_PP_Asm ||
> > +          InputType == types::TY_Asm)
> > +        continue;
> > +
> > +      Action *Current = C.MakeAction(*InputArg, InputType);
> > +
> > +      for (auto Phase : PhaseList) {
> > +        switch (Phase) {
> > +        default:
> > +          llvm_unreachable(
> > +              "IFS Pipeline can only consist of Compile followed by 

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

dschuff wrote:
> sunfish wrote:
> > dschuff wrote:
> > > sunfish wrote:
> > > > dschuff wrote:
> > > > > sunfish wrote:
> > > > > > dschuff wrote:
> > > > > > > What would you think about adding a way to pass arguments through 
> > > > > > > to wasm-opt on the command line, like we have for the linker, 
> > > > > > > assembler, etc? Something like `-Wo,-O2` (or `-Ww` or whatever; 
> > > > > > > analogous to `-Wl` and `-Wa`). That seems nicer than an env var, 
> > > > > > > although it doesn't solve the problem of controlling whether to 
> > > > > > > run the optimizer in the first place.
> > > > > > My guess here is that we don't need clang to have an option for 
> > > > > > passing additional flags -- if people want to do extra special 
> > > > > > things with wasm-opt on clang's output they can just run wasm-opt 
> > > > > > directly themselves. Does that sound reasonable?
> > > > > Maybe. But I still don't like the use of an env var for this kind of 
> > > > > behavior-effecting (i.e. non-debugging) use case.  It's hard enough 
> > > > > to get reproducible and hermetic build behavior as it is, I 
> > > > > definitely wouldn't want to worry about the environment affecting the 
> > > > > output in addition to all the random flags, included files, etc.
> > > > If we did -Wo,-O2 or so, we'd still need to be able to find the 
> > > > wasm-opt program to be able to run it. We could just search for it in 
> > > > PATH, but that's also a little dicey from a hermetic build perspective.
> > > > 
> > > > I borrowed "WASM_OPT" from 
> > > > [cargo-wasi](https://github.com/bytecodealliance/cargo-wasi). I'm also 
> > > > not a fan of environment variables in general, but this way does have 
> > > > the advantage that people can set it once, and not have to modify their 
> > > > Makefiles to add new flags. Users can think of it as just being part of 
> > > > -O2 and friends.
> > > > 
> > > What's the usual way to locate things like external assemblers? 
> > > Presumably we could use the same mechanism for wasm-opt?
> > It checks the COMPILER_PATH environment variable and -B command-line flags, 
> > which I'm not sure we should use here, but it looks like it does fall back 
> > to checking PATH at the end.
> > 
> > So, what would you think of just checking PATH for wasm-opt?
> I suspect we'll end up with -B flags if/when people start building 
> interesting or nontrivial toolchains with clang (or we try to make emscripten 
> more standardish), but I'm fine with leaving that out for now. Checking PATH 
> for wasm-opt seems fine to me to locate the binary. Did you have that in mind 
> as also the way to determine whether or not to run wasm-opt (i.e. run if it's 
> there, don't if it's not)? That seems slightly error-prone in the sense that 
> there would be a silent behavior change if something went wrong (e.g. 
> wasm-opt goes missing) but in a world where most clang users are using 
> wasm-opt then using wasm-opt by default seems reasonable; so that seems fine 
> as a way to start out.
> 
> I do think we will eventually want some way to modify the behavior of 
> wasm-opt though. For that matter wasm-opt might at some point become able to 
> optimize object files (allowing faster links at the cost of less 
> LTO-optimized binaries; we'd run a reduced set of passes post-link or none at 
> all). In that case there'd also have to be further changes if we want builtin 
> support for that.
Yes, we'd just run wasm-opt if it's in the PATH. See the now updated patch. And 
yeah, this means if your wasm-opt disappears, you silently won't get as much 
optimization, which is a little unfortunate, but it is the most convenient for 
the common use case.

And yeah, we can always add more options in the future :-).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish updated this revision to Diff 230350.
sunfish added a comment.

Use PATH instead of WASM_OPT to find wasm-opt.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/test/Driver/wasm-toolchain-lto.c


Index: clang/test/Driver/wasm-toolchain-lto.c
===
--- /dev/null
+++ clang/test/Driver/wasm-toolchain-lto.c
@@ -0,0 +1,6 @@
+// A basic C link command-line with optimization with known OS and LTO enabled.
+
+// RUN: %clang -### -O2 -flto -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto/
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -8,6 +8,7 @@
 
 #include "WebAssembly.h"
 #include "CommonArgs.h"
+#include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
@@ -90,6 +91,29 @@
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(std::make_unique(JA, *this, Linker, CmdArgs, Inputs));
+
+  // When optimizing, if wasm-opt is in the PATH, run wasm-opt.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (llvm::ErrorOr WasmOptPath =
+   llvm::sys::findProgramByName("wasm-opt")) {
+  StringRef OOpt = "s";
+  if (A->getOption().matches(options::OPT_O4) ||
+  A->getOption().matches(options::OPT_Ofast))
+OOpt = "4";
+  else if (A->getOption().matches(options::OPT_O0))
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();
+
+  const char *WasmOpt = Args.MakeArgString(*WasmOptPath);
+  ArgStringList CmdArgs;
+  CmdArgs.push_back(Output.getFilename());
+  CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
+  CmdArgs.push_back("-o");
+  CmdArgs.push_back(Output.getFilename());
+  C.addCommand(std::make_unique(JA, *this, WasmOpt, CmdArgs, 
Inputs));
+}
+  }
 }
 
 WebAssembly::WebAssembly(const Driver , const llvm::Triple ,
@@ -109,6 +133,16 @@
   } else {
 const std::string MultiarchTriple =
 getMultiarchTriple(getDriver(), Triple, getDriver().SysRoot);
+if (D.isUsingLTO()) {
+  auto LLVMRevision = getLLVMRevision();
+  if (!LLVMRevision.empty()) {
+// For LTO, enable use of lto-enabled sysroot libraries too, if 
available.
+// Note that the directory is keyed to the LLVM revision, as LLVM's
+// bitcode format is not stable.
+getFilePaths().push_back(getDriver().SysRoot + "/lib/" + 
MultiarchTriple +
+ "/llvm-lto/" + LLVMRevision);
+  }
+}
 getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple);
   }
 }


Index: clang/test/Driver/wasm-toolchain-lto.c
===
--- /dev/null
+++ clang/test/Driver/wasm-toolchain-lto.c
@@ -0,0 +1,6 @@
+// A basic C link command-line with optimization with known OS and LTO enabled.
+
+// RUN: %clang -### -O2 -flto -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto/
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -8,6 +8,7 @@
 
 #include "WebAssembly.h"
 #include "CommonArgs.h"
+#include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
@@ -90,6 +91,29 @@
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(std::make_unique(JA, *this, Linker, CmdArgs, Inputs));
+
+  // When optimizing, if wasm-opt is in the PATH, run wasm-opt.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (llvm::ErrorOr WasmOptPath =
+   llvm::sys::findProgramByName("wasm-opt")) {
+  StringRef OOpt = "s";
+  if (A->getOption().matches(options::OPT_O4) ||
+  A->getOption().matches(options::OPT_Ofast))
+OOpt = "4";
+  else if (A->getOption().matches(options::OPT_O0))
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();
+
+  const char *WasmOpt = Args.MakeArgString(*WasmOptPath);
+  ArgStringList CmdArgs;
+  CmdArgs.push_back(Output.getFilename());
+  

[PATCH] D70518: [clang-include-fixer] Suppress cmd prompt from Vim on Windows

[Reid Kleckner via Phabricator via cfe-commits]

[PATCH] D70351: [clang][WIP][clang-scan-deps] Add an experimental C API.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added a comment.

In D70351#1754276 , @arphaman wrote:

> Adding experimental APIs is something that we haven't done before, but it be 
> useful for this case.


Yep, I'm currently aware of two other people who care about this interface, so 
I thought it would make sense to do the development of the API upstream.

> I have a couple of questions about the API:
> 
> - Should types be prefixed / suffixed with experimental / v0 in case we need 
> to extend the information passed to the client?

I don't think the type names matter unless someone plans to write a user of 
this API that can handle multiple versions at the same time (which means they 
aren't using this header). The function names matter as that's what shows up in 
the symbol table.

> - The types look to be more geared towards Clang modules (like the addition 
> of a module map path). Do you think the same types could be used for C++ 20 
> modules as well, or should C++ 20 modules have a completely separate API?

C++20 modules will need to use the same API as we intend to support mixing 
Clang modules and C++20 modules. The types will need to be modified slightly to 
work for C++20 modules, I just haven't gotten to that yet. C++20 module 
interfaces don't need a module-map or equivalent, and C++20 header units will 
just need to know the header file they are for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70351



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

[clang] d9957c7 - [Sema] Add a 'Semantic' parameter to Expr::isKnownToHaveBooleanValue

[Erik Pilkington via cfe-commits]

Author: Erik Pilkington
Date: 2019-11-20T16:29:31-08:00
New Revision: d9957c7405bc726422dbc2736ad62f704916fbe8

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

LOG: [Sema] Add a 'Semantic' parameter to Expr::isKnownToHaveBooleanValue

Some clients of this function want to know about any expression that is known
to produce a 0/1 value, and others care about expressions that are semantically
boolean.

This fixes a -Wswitch-bool regression I introduced in 8bfb353bb33c, pointed out
by Chris Hamilton!

Added: 


Modified: 
clang/include/clang/AST/Expr.h
clang/lib/AST/Expr.cpp
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/switch.c

Removed: 




diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index ffa7d4db96a4..867615ded162 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -494,7 +494,13 @@ class Expr : public ValueStmt {
   /// that is known to return 0 or 1.  This happens for _Bool/bool expressions
   /// but also int expressions which are produced by things like comparisons in
   /// C.
-  bool isKnownToHaveBooleanValue() const;
+  ///
+  /// \param Semantic If true, only return true for expressions that are known
+  /// to be semantically boolean, which might not be true even for expressions
+  /// that are known to evaluate to 0/1. For instance, reading an unsigned
+  /// bit-field with width '1' will evaluate to 0/1, but doesn't necessarily
+  /// semantically correspond to a bool.
+  bool isKnownToHaveBooleanValue(bool Semantic = true) const;
 
   /// isIntegerConstantExpr - Return true if this expression is a valid integer
   /// constant expression, and, if so, return its value in Result.  If not a

diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index d5c35e53059b..e5bb8a778c18 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -127,11 +127,7 @@ const Expr *Expr::skipRValueSubobjectAdjustments(
   return E;
 }
 
-/// isKnownToHaveBooleanValue - Return true if this is an integer expression
-/// that is known to return 0 or 1.  This happens for _Bool/bool expressions
-/// but also int expressions which are produced by things like comparisons in
-/// C.
-bool Expr::isKnownToHaveBooleanValue() const {
+bool Expr::isKnownToHaveBooleanValue(bool Semantic) const {
   const Expr *E = IgnoreParens();
 
   // If this value has _Bool type, it is obvious 0/1.
@@ -142,7 +138,7 @@ bool Expr::isKnownToHaveBooleanValue() const {
   if (const UnaryOperator *UO = dyn_cast(E)) {
 switch (UO->getOpcode()) {
 case UO_Plus:
-  return UO->getSubExpr()->isKnownToHaveBooleanValue();
+  return UO->getSubExpr()->isKnownToHaveBooleanValue(Semantic);
 case UO_LNot:
   return true;
 default:
@@ -152,8 +148,9 @@ bool Expr::isKnownToHaveBooleanValue() const {
 
   // Only look through implicit casts.  If the user writes
   // '(int) (a && b)' treat it as an arbitrary int.
+  // FIXME: Should we look through any cast expression in !Semantic mode?
   if (const ImplicitCastExpr *CE = dyn_cast(E))
-return CE->getSubExpr()->isKnownToHaveBooleanValue();
+return CE->getSubExpr()->isKnownToHaveBooleanValue(Semantic);
 
   if (const BinaryOperator *BO = dyn_cast(E)) {
 switch (BO->getOpcode()) {
@@ -172,27 +169,27 @@ bool Expr::isKnownToHaveBooleanValue() const {
 case BO_Xor:  // Bitwise XOR operator.
 case BO_Or:   // Bitwise OR operator.
   // Handle things like (x==2)|(y==12).
-  return BO->getLHS()->isKnownToHaveBooleanValue() &&
- BO->getRHS()->isKnownToHaveBooleanValue();
+  return BO->getLHS()->isKnownToHaveBooleanValue(Semantic) &&
+ BO->getRHS()->isKnownToHaveBooleanValue(Semantic);
 
 case BO_Comma:
 case BO_Assign:
-  return BO->getRHS()->isKnownToHaveBooleanValue();
+  return BO->getRHS()->isKnownToHaveBooleanValue(Semantic);
 }
   }
 
   if (const ConditionalOperator *CO = dyn_cast(E))
-return CO->getTrueExpr()->isKnownToHaveBooleanValue() &&
-   CO->getFalseExpr()->isKnownToHaveBooleanValue();
+return CO->getTrueExpr()->isKnownToHaveBooleanValue(Semantic) &&
+   CO->getFalseExpr()->isKnownToHaveBooleanValue(Semantic);
 
   if (isa(E))
 return true;
 
   if (const auto *OVE = dyn_cast(E))
-return OVE->getSourceExpr()->isKnownToHaveBooleanValue();
+return OVE->getSourceExpr()->isKnownToHaveBooleanValue(Semantic);
 
   if (const FieldDecl *FD = E->getSourceBitField())
-if (FD->getType()->isUnsignedIntegerType() &&
+if (!Semantic && FD->getType()->isUnsignedIntegerType() &&
 !FD->getBitWidth()->isValueDependent() &&
 FD->getBitWidthValue(FD->getASTContext()) == 1)
   return true;

diff  --git a/clang/lib/Sema/SemaChecking.cpp 

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Michael Spencer via Phabricator via cfe-commits]

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



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:31
+  /// Modules that the input directly imports.
+  std::vector DirectModuleDependencies;
+  /// The Clang modules this input transitively depends on that have not 
already

arphaman wrote:
> Are the strings supposed to represent the module names here? For C++20 
> modules, will a single string be enough to represent both the module's name 
> and its partition name?
If it's a partition the module name will just contain a `:`.



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:63
+  llvm::Expected
+  getFullDependencies(const tooling::CompilationDatabase ,
+  StringRef CWD, const llvm::StringSet<> );

arphaman wrote:
> Can you add a comment on how `AlreadySeen` is supposed to be used. Are 
> clients expected to update it once they get more dependencies?
Will do. I'm still working out exactly how `AlreadySeen` should work. Ideally 
it would be shared across all workers of the same `DependencyScanningService`, 
but that needs thread synchronization and could have rather high overhead. The 
current model is that it's per worker, and you should expect to need to 
deduplicate modules across workers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70268



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

Re: [clang] 7342912 - [clang][IFS] Driver Pipeline: generate stubs after standard pipeline (3)

[Alex L via cfe-commits]

Hi Puyan,

This commit caused two Clang failures on Darwin:

Clang :: InterfaceStubs/merge-conflict-test.c
Clang :: InterfaceStubs/object-float.c

Here's the build log from out bot:
http://lab.llvm.org:8080/green/job/clang-stage1-RA/3929/console

Can you please resolve the issue with the tests?
Thanks,
Alex

On Wed, 20 Nov 2019 at 14:16, Puyan Lotfi via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Puyan Lotfi
> Date: 2019-11-20T16:22:50-05:00
> New Revision: 73429126c91c2065c6f6ef29b3eec1b7798502bb
>
> URL:
> https://github.com/llvm/llvm-project/commit/73429126c91c2065c6f6ef29b3eec1b7798502bb
> DIFF:
> https://github.com/llvm/llvm-project/commit/73429126c91c2065c6f6ef29b3eec1b7798502bb.diff
>
> LOG: [clang][IFS] Driver Pipeline: generate stubs after standard pipeline
> (3)
>
> Third Landing Attempt (dropping any linker invocation from clang driver):
>
> Up until now, clang interface stubs has replaced the standard
> PP -> C -> BE -> ASM -> LNK pipeline. With this change, it will happen in
> conjunction with it. So what when you build your code you will get an
> a.out or lib.so as well as an interface stub file.
>
> Example:
>
> clang -shared -o libfoo.so -emit-interface-stubs ...
>
> will generate both a libfoo.so and a libfoo.ifso. The .so file will
> contain the code from the standard compilation pipeline and the .ifso
> file will contain the ELF stub library.
>
> Note: For driver-test.c I've added -S in order to prevent any bot failures
> on
> bots that don't have the proper linker for their native triple. You could
> always
> specify a triple like x86_64-unknown-linux-gnu and on bots like
> x86_64-scei-ps4
> the clang driver would invoke regular ld instead of getting the error
> 'Executable "orbis-ld" doesn't exist!' but on bots like ppc64be and s390x
> you'd
> get an error "/usr/bin/ld: unrecognised emulation mode: elf_x86_64"
>
> Differential Revision: https://reviews.llvm.org/D70274
>
> Added:
> clang/test/InterfaceStubs/driver-test2.c
> clang/test/InterfaceStubs/ppc.cpp
>
> Modified:
> clang/lib/Driver/Driver.cpp
> clang/lib/Driver/ToolChains/InterfaceStubs.cpp
> clang/lib/Driver/Types.cpp
> clang/test/InterfaceStubs/driver-test.c
> clang/test/InterfaceStubs/windows.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
> index cdf4a579f431..83b5db3b2530 100644
> --- a/clang/lib/Driver/Driver.cpp
> +++ b/clang/lib/Driver/Driver.cpp
> @@ -292,10 +292,6 @@ phases::ID Driver::getFinalPhase(const DerivedArgList
> ,
>   (PhaseArg = DAL.getLastArg(options::OPT_emit_ast))) {
>  FinalPhase = phases::Compile;
>
> -  // clang interface stubs
> -  } else if ((PhaseArg =
> DAL.getLastArg(options::OPT_emit_interface_stubs))) {
> -FinalPhase = phases::IfsMerge;
> -
>// -S only runs up to the backend.
>} else if ((PhaseArg = DAL.getLastArg(options::OPT_S))) {
>  FinalPhase = phases::Backend;
> @@ -3502,6 +3498,68 @@ void Driver::BuildActions(Compilation ,
> DerivedArgList ,
>  Actions.push_back(
>  C.MakeAction(MergerInputs, types::TY_Image));
>
> +  if (Arg *A = Args.getLastArg(options::OPT_emit_interface_stubs)) {
> +llvm::SmallVector PhaseList;
> +if (Args.hasArg(options::OPT_c)) {
> +  llvm::SmallVector
> CompilePhaseList;
> +  types::getCompilationPhases(types::TY_IFS_CPP, CompilePhaseList);
> +  llvm::copy_if(CompilePhaseList, std::back_inserter(PhaseList),
> +[&](phases::ID Phase) { return Phase <=
> phases::Compile; });
> +} else {
> +  types::getCompilationPhases(types::TY_IFS_CPP, PhaseList);
> +}
> +
> +ActionList MergerInputs;
> +
> +for (auto  : Inputs) {
> +  types::ID InputType = I.first;
> +  const Arg *InputArg = I.second;
> +
> +  // Currently clang and the llvm assembler do not support generating
> symbol
> +  // stubs from assembly, so we skip the input on asm files. For ifs
> files
> +  // we rely on the normal pipeline setup in the pipeline setup code
> above.
> +  if (InputType == types::TY_IFS || InputType == types::TY_PP_Asm ||
> +  InputType == types::TY_Asm)
> +continue;
> +
> +  Action *Current = C.MakeAction(*InputArg, InputType);
> +
> +  for (auto Phase : PhaseList) {
> +switch (Phase) {
> +default:
> +  llvm_unreachable(
> +  "IFS Pipeline can only consist of Compile followed by
> IfsMerge.");
> +case phases::Compile: {
> +  // Only IfsMerge (llvm-ifs) can handle .o files by looking for
> ifs
> +  // files where the .o file is located. The compile action can
> not
> +  // handle this.
> +  if (InputType == types::TY_Object)
> +break;
> +
> +  Current = C.MakeAction(Current,
> types::TY_IFS_CPP);
> +  break;
> +}
> +case phases::IfsMerge: 

[PATCH] D70390: [clang-tidy] new performance-no-automatic-move check.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp:28
+void NoAutomaticMoveCheck::registerMatchers(MatchFinder *finder) {
+  const auto const_local_variable =
+  varDecl(hasLocalStorage(), unless(hasType(lValueReferenceType())),

the matchers need only to be registered for c++ and and c++11 and later. See 
`getLangOpts()` in other checks.



Comment at: clang-tools-extra/clang-tidy/performance/NoAutomaticMoveCheck.cpp:59
+  const QualType var_type = vardecl->getType();
+  if (var_type.isConstQualified()) {
+diag(ctor_call->getExprLoc(), "constness of '%0' prevents automatic move")

i think `isConstQualified` could move into the matcher with `Matcher 
isConstQualified`.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-no-automatic-move.rst:11
+local values are automatically moved out when returning from a function. A
+common mistake is to declared local ``lvalue`` variables ``const``, which
+prevents the move.

s/declared/declare


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70390



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

[PATCH] D70274: [clang][IFS] Driver pipeline change for clang-ifs: generate interface stubs after standard pipeline.

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

Hi! I think this patch might be causing some test failures on our mac bots:

  FAIL: Clang :: InterfaceStubs/merge-conflict-test.c (6268 of 16220)
   TEST 'Clang :: InterfaceStubs/merge-conflict-test.c' 
FAILED 
  Script:
  --
  : 'RUN: at line 1';   not 
/b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/clang 
-fvisibility=default -o libfoo.so -emit-interface-stubs 
/b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/merge-conflict-test.c 
/b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/driver-test.c 2>&1 | 
/b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/FileCheck 
/b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/merge-conflict-test.c
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  
/b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/merge-conflict-test.c:2:11: 
error: CHECK: expected string not found in input
  // CHECK: error: Interface Stub: Type Mismatch
^
  :1:1: note: scanning from here
  clang-10: error: cannot specify -o when generating multiple output files
  ^
  :1:11: note: possible intended match here
  clang-10: error: cannot specify -o when generating multiple output files
^
  
  --
  
  
  Testing:  0.. 10.. 20.. 30..
  FAIL: Clang :: InterfaceStubs/object-float.c (6276 of 16220)
   TEST 'Clang :: InterfaceStubs/object-float.c' FAILED 

  Script:
  --
  : 'RUN: at line 1';   not 
/b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/clang 
-fvisibility=default -o - -emit-interface-stubs 
/b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object-float.c 
/b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object.c 2>&1 | 
/b/s/w/ir/k/recipe_cleanup/clangKJGdzA/llvm_build_dir/bin/FileCheck 
/b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object-float.c
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  /b/s/w/ir/k/llvm-project/clang/test/InterfaceStubs/object-float.c:2:11: 
error: CHECK: expected string not found in input
  // CHECK: error: Interface Stub: Size Mismatch
^
  :1:1: note: scanning from here
  clang-10: error: cannot specify -o when generating multiple output files
  ^
  :1:11: note: possible intended match here
  clang-10: error: cannot specify -o when generating multiple output files
^
  
  --
  
  
  Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
  
  Testing Time: 131.42s
  
  Failing Tests (2):
  Clang :: InterfaceStubs/merge-conflict-test.c
  Clang :: InterfaceStubs/object-float.c

Could you look into it? Thanks.

Bot link: 
https://ci.chromium.org/p/fuchsia/builders/ci/clang-mac-x64/b8896234755488932160


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70274



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

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

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

xazax.hun updated this revision to Diff 230347.
xazax.hun marked 8 inline comments as done.
xazax.hun added a comment.

- Address review comments.


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

https://reviews.llvm.org/D70469

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

Index: clang/test/Sema/attr-handles.c
===
--- /dev/null
+++ clang/test/Sema/attr-handles.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1  -fsyntax-only -verify %s
+
+void f(int *a __attribute__((acquire_handle)));
+void g(int a __attribute__((acquire_handle))); // expected-error {{attribute only applies to output parameters}}
+void h(int *a __attribute__((acquire_handle(1; // expected-error {{'acquire_handle' attribute takes no arguments}}
+__attribute__((release_handle)) int i(); // expected-warning {{'release_handle' attribute only applies to parameters}}
+__attribute__((use_handle)) int j(); // expected-warning {{'use_handle' attribute only applies to parameters}}
+int a __attribute__((acquire_handle)); // expected-warning {{'acquire_handle' attribute only applies to functions, function pointers, and parameters}}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -9,6 +9,7 @@
 // CHECK-NEXT: AMDGPUWavesPerEU (SubjectMatchRule_function)
 // CHECK-NEXT: AVRSignal (SubjectMatchRule_function)
 // CHECK-NEXT: AbiTag (SubjectMatchRule_record_not_is_union, SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_namespace)
+// CHECK-NEXT: AcquireHandle (SubjectMatchRule_hasType_functionType, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: Alias (SubjectMatchRule_function, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: AlignValue (SubjectMatchRule_variable, SubjectMatchRule_type_alias)
 // CHECK-NEXT: AllocSize (SubjectMatchRule_function)
@@ -126,6 +127,7 @@
 // CHECK-NEXT: ParamTypestate (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: PassObjectSize (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: Pointer (SubjectMatchRule_record_not_is_union)
+// CHECK-NEXT: ReleaseHandle (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: RenderScriptKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ReqdWorkGroupSize (SubjectMatchRule_function)
 // CHECK-NEXT: Restrict (SubjectMatchRule_function)
@@ -143,6 +145,7 @@
 // CHECK-NEXT: Target (SubjectMatchRule_function)
 // CHECK-NEXT: TestTypestate (SubjectMatchRule_function_is_member)
 // CHECK-NEXT: TrivialABI (SubjectMatchRule_record)
+// CHECK-NEXT: UseHandle (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: VecReturn (SubjectMatchRule_record)
 // CHECK-NEXT: VecTypeHint (SubjectMatchRule_function)
 // CHECK-NEXT: WarnUnused (SubjectMatchRule_record)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6518,6 +6518,18 @@
   handleSimpleAttribute(S, D, AL);
 }
 
+static void handeAcquireHandleAttr(Sema , Decl *D, const ParsedAttr ) {
+  // Warn if the parameter is definitely not an output parameter.
+  if (auto *PVD = dyn_cast(D)) {
+if (PVD->getType()->isIntegerType()) {
+  S.Diag(AL.getLoc(), diag::err_attribute_output_parameter)
+  << AL.getRange();
+  return;
+}
+  }
+  handleSimpleAttribute(S, D, AL);
+}
+
 //===--===//
 // Top Level Sema Entry Points
 //===--===//
@@ -7282,6 +7294,16 @@
   case ParsedAttr::AT_ArmMveAlias:
 handleArmMveAliasAttr(S, D, AL);
 break;
+
+  case ParsedAttr::AT_AcquireHandle:
+handeAcquireHandleAttr(S, D, AL);
+break;
+  case ParsedAttr::AT_ReleaseHandle:
+handleSimpleAttribute(S, D, AL);
+break;
+  case ParsedAttr::AT_UseHandle:
+handleSimpleAttribute(S, D, AL);
+break;
   }
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3072,6 +3072,8 @@
 def err_cconv_incomplete_param_type : Error<
   "parameter %0 must have a complete type to use function %1 with the %2 "
   "calling convention">;
+def err_attribute_output_parameter : Error<
+  "attribute only applies to output parameters">;
 
 def ext_cannot_use_trivial_abi : ExtWarn<
   "'trivial_abi' cannot be applied to %0">, InGroup;
Index: 

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

sunfish wrote:
> dschuff wrote:
> > sunfish wrote:
> > > dschuff wrote:
> > > > sunfish wrote:
> > > > > dschuff wrote:
> > > > > > What would you think about adding a way to pass arguments through 
> > > > > > to wasm-opt on the command line, like we have for the linker, 
> > > > > > assembler, etc? Something like `-Wo,-O2` (or `-Ww` or whatever; 
> > > > > > analogous to `-Wl` and `-Wa`). That seems nicer than an env var, 
> > > > > > although it doesn't solve the problem of controlling whether to run 
> > > > > > the optimizer in the first place.
> > > > > My guess here is that we don't need clang to have an option for 
> > > > > passing additional flags -- if people want to do extra special things 
> > > > > with wasm-opt on clang's output they can just run wasm-opt directly 
> > > > > themselves. Does that sound reasonable?
> > > > Maybe. But I still don't like the use of an env var for this kind of 
> > > > behavior-effecting (i.e. non-debugging) use case.  It's hard enough to 
> > > > get reproducible and hermetic build behavior as it is, I definitely 
> > > > wouldn't want to worry about the environment affecting the output in 
> > > > addition to all the random flags, included files, etc.
> > > If we did -Wo,-O2 or so, we'd still need to be able to find the wasm-opt 
> > > program to be able to run it. We could just search for it in PATH, but 
> > > that's also a little dicey from a hermetic build perspective.
> > > 
> > > I borrowed "WASM_OPT" from 
> > > [cargo-wasi](https://github.com/bytecodealliance/cargo-wasi). I'm also 
> > > not a fan of environment variables in general, but this way does have the 
> > > advantage that people can set it once, and not have to modify their 
> > > Makefiles to add new flags. Users can think of it as just being part of 
> > > -O2 and friends.
> > > 
> > What's the usual way to locate things like external assemblers? Presumably 
> > we could use the same mechanism for wasm-opt?
> It checks the COMPILER_PATH environment variable and -B command-line flags, 
> which I'm not sure we should use here, but it looks like it does fall back to 
> checking PATH at the end.
> 
> So, what would you think of just checking PATH for wasm-opt?
I suspect we'll end up with -B flags if/when people start building interesting 
or nontrivial toolchains with clang (or we try to make emscripten more 
standardish), but I'm fine with leaving that out for now. Checking PATH for 
wasm-opt seems fine to me to locate the binary. Did you have that in mind as 
also the way to determine whether or not to run wasm-opt (i.e. run if it's 
there, don't if it's not)? That seems slightly error-prone in the sense that 
there would be a silent behavior change if something went wrong (e.g. wasm-opt 
goes missing) but in a world where most clang users are using wasm-opt then 
using wasm-opt by default seems reasonable; so that seems fine as a way to 
start out.

I do think we will eventually want some way to modify the behavior of wasm-opt 
though. For that matter wasm-opt might at some point become able to optimize 
object files (allowing faster links at the cost of less LTO-optimized binaries; 
we'd run a reduced set of passes post-link or none at all). In that case 
there'd also have to be further changes if we want builtin support for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70351: [clang][WIP][clang-scan-deps] Add an experimental C API.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Adding experimental APIs is something that we haven't done before, but it be 
useful for this case.
I have a couple of questions about the API:

- Should types be prefixed / suffixed with experimental / v0 in case we need to 
extend the information passed to the client?
- The types look to be more geared towards Clang modules (like the addition of 
a module map path). Do you think the same types could be used for C++ 20 
modules as well, or should C++ 20 modules have a completely separate API?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70351



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

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:31
+  /// Modules that the input directly imports.
+  std::vector DirectModuleDependencies;
+  /// The Clang modules this input transitively depends on that have not 
already

Are the strings supposed to represent the module names here? For C++20 modules, 
will a single string be enough to represent both the module's name and its 
partition name?



Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:63
+  llvm::Expected
+  getFullDependencies(const tooling::CompilationDatabase ,
+  StringRef CWD, const llvm::StringSet<> );

Can you add a comment on how `AlreadySeen` is supposed to be used. Are clients 
expected to update it once they get more dependencies?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70268



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

[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG24aafcadff38: [clang-tidy] modernize-use-equals-default 
avoid adding redundant semicolons (authored by mitchell-stellar).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70144

Files:
  clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
  clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
  clang-tools-extra/clang-tidy/utils/LexerUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
@@ -7,7 +7,7 @@
   ~OL();
 };
 
-OL::OL() {}
+OL::OL() {};
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use '= default' to define a trivial default constructor [modernize-use-equals-default]
 // CHECK-FIXES: OL::OL() = default;
 OL::~OL() {}
@@ -17,9 +17,9 @@
 // Inline definitions.
 class IL {
 public:
-  IL() {}
+  IL() {} 	 ; // Note embedded tab on this line
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
-  // CHECK-FIXES: IL() = default;
+  // CHECK-FIXES: IL() = default 	 ; // Note embedded tab on this line
   ~IL() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: ~IL() = default;
@@ -46,18 +46,20 @@
 // Default member initializer
 class DMI {
 public:
-  DMI() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
-  // CHECK-FIXES: DMI() = default;
+  DMI() {} // Comment before semi-colon on next line
+  ;
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use '= default'
+  // CHECK-FIXES: DMI() = default // Comment before semi-colon on next line
+  // CHECK-FIXES-NEXT:   ;
   int Field = 5;
 };
 
 // Class member
 class CM {
 public:
-  CM() {}
+  CM() {} /* Comments */ /* before */ /* semicolon */;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
-  // CHECK-FIXES: CM() = default;
+  // CHECK-FIXES: CM() = default /* Comments */ /* before */ /* semicolon */;
   OL o;
 };
 
@@ -66,7 +68,7 @@
   Priv() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: Priv() = default;
-  ~Priv() {}
+  ~Priv() {};
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: ~Priv() = default;
 };
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
@@ -119,7 +119,7 @@
 struct BF {
   BF() = default;
   BF(const BF ) : Field1(Other.Field1), Field2(Other.Field2), Field3(Other.Field3),
-Field4(Other.Field4) {}
+Field4(Other.Field4) {};
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use '= default'
   // CHECK-FIXES: BF(const BF ) {{$}}
   // CHECK-FIXES: = default;
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -158,6 +158,10 @@
 - The 'objc-avoid-spinlock' check was renamed to :doc:`darwin-avoid-spinlock
   `
 
+- The :doc:`modernize-use-equals-default
+  ` fix no longer adds
+  semicolons where they would be redundant.
+
 - New :doc:`readability-redundant-access-specifiers
   ` check.
 
Index: clang-tools-extra/clang-tidy/utils/LexerUtils.h
===
--- clang-tools-extra/clang-tidy/utils/LexerUtils.h
+++ clang-tools-extra/clang-tidy/utils/LexerUtils.h
@@ -80,6 +80,11 @@
   }
 }
 
+// Finds next token that's not a comment.
+Optional findNextTokenSkippingComments(SourceLocation Start,
+  const SourceManager ,
+  const LangOptions );
+
 /// Re-lex the provide \p Range and return \c false if either a macro spans
 /// multiple tokens, a pre-processor directive or failure to retrieve the
 /// next token is found, otherwise \c true.
Index: clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
@@ -68,6 +68,16 @@
   return findNextAnyTokenKind(Start, SM, LangOpts, tok::comma, tok::semi);
 }
 
+Optional findNextTokenSkippingComments(SourceLocation Start,
+ 

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

dschuff wrote:
> sunfish wrote:
> > dschuff wrote:
> > > sunfish wrote:
> > > > dschuff wrote:
> > > > > What would you think about adding a way to pass arguments through to 
> > > > > wasm-opt on the command line, like we have for the linker, assembler, 
> > > > > etc? Something like `-Wo,-O2` (or `-Ww` or whatever; analogous to 
> > > > > `-Wl` and `-Wa`). That seems nicer than an env var, although it 
> > > > > doesn't solve the problem of controlling whether to run the optimizer 
> > > > > in the first place.
> > > > My guess here is that we don't need clang to have an option for passing 
> > > > additional flags -- if people want to do extra special things with 
> > > > wasm-opt on clang's output they can just run wasm-opt directly 
> > > > themselves. Does that sound reasonable?
> > > Maybe. But I still don't like the use of an env var for this kind of 
> > > behavior-effecting (i.e. non-debugging) use case.  It's hard enough to 
> > > get reproducible and hermetic build behavior as it is, I definitely 
> > > wouldn't want to worry about the environment affecting the output in 
> > > addition to all the random flags, included files, etc.
> > If we did -Wo,-O2 or so, we'd still need to be able to find the wasm-opt 
> > program to be able to run it. We could just search for it in PATH, but 
> > that's also a little dicey from a hermetic build perspective.
> > 
> > I borrowed "WASM_OPT" from 
> > [cargo-wasi](https://github.com/bytecodealliance/cargo-wasi). I'm also not 
> > a fan of environment variables in general, but this way does have the 
> > advantage that people can set it once, and not have to modify their 
> > Makefiles to add new flags. Users can think of it as just being part of -O2 
> > and friends.
> > 
> What's the usual way to locate things like external assemblers? Presumably we 
> could use the same mechanism for wasm-opt?
It checks the COMPILER_PATH environment variable and -B command-line flags, 
which I'm not sure we should use here, but it looks like it does fall back to 
checking PATH at the end.

So, what would you think of just checking PATH for wasm-opt?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[clang-tools-extra] 24aafca - [clang-tidy] modernize-use-equals-default avoid adding redundant semicolons

[Mitchell Balan via cfe-commits]

Author: Mitchell Balan
Date: 2019-11-20T18:08:37-05:00
New Revision: 24aafcadff3851ec3a0c42303fec63e815b19566

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

LOG: [clang-tidy] modernize-use-equals-default avoid adding redundant semicolons

Summary:
`modernize-use-equals-default` replaces default constructors/destructors with 
`= default;`. When the optional semicolon after a member function is present, 
this results in two consecutive semicolons.

This patch checks to see if the next non-comment token after the code to be 
replaced is a semicolon, and if so offers a replacement of `= default` rather 
than `= default;`.

This patch adds trailing comments and semicolons to about 5 existing tests.

Reviewers: malcolm.parsons, angelgarcia, aaron.ballman, alexfh

Patch by: poelmanc

Subscribers: MyDeveloperDay, JonasToth, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
clang-tools-extra/clang-tidy/utils/LexerUtils.h
clang-tools-extra/docs/ReleaseNotes.rst

clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
index 991eada514d3..0309fa8d0a37 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
@@ -10,6 +10,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Lex/Lexer.h"
+#include "../utils/LexerUtils.h"
 
 using namespace clang::ast_matchers;
 
@@ -302,9 +303,16 @@ void UseEqualsDefaultCheck::check(const 
MatchFinder::MatchResult ) {
   auto Diag = diag(Location, "use '= default' to define a trivial " +
  SpecialFunctionName);
 
-  if (ApplyFix)
-Diag << FixItHint::CreateReplacement(Body->getSourceRange(), "= default;")
+  if (ApplyFix) {
+// Skipping comments, check for a semicolon after Body->getSourceRange()
+Optional Token = utils::lexer::findNextTokenSkippingComments(
+Body->getSourceRange().getEnd().getLocWithOffset(1),
+Result.Context->getSourceManager(), Result.Context->getLangOpts());
+StringRef Replacement =
+Token && Token->is(tok::semi) ? "= default" : "= default;";
+Diag << FixItHint::CreateReplacement(Body->getSourceRange(), Replacement)
  << RemoveInitializers;
+  }
 }
 
 } // namespace modernize

diff  --git a/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp 
b/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
index a6e361d66aa7..e80fda6e318e 100644
--- a/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
@@ -68,6 +68,16 @@ SourceLocation findNextTerminator(SourceLocation Start, 
const SourceManager ,
   return findNextAnyTokenKind(Start, SM, LangOpts, tok::comma, tok::semi);
 }
 
+Optional findNextTokenSkippingComments(SourceLocation Start,
+  const SourceManager ,
+  const LangOptions ) {
+  Optional CurrentToken;
+  do {
+CurrentToken = Lexer::findNextToken(Start, SM, LangOpts);
+  } while (CurrentToken && CurrentToken->is(tok::comment));
+  return CurrentToken;
+}
+
 bool rangeContainsExpansionsOrDirectives(SourceRange Range,
  const SourceManager ,
  const LangOptions ) {

diff  --git a/clang-tools-extra/clang-tidy/utils/LexerUtils.h 
b/clang-tools-extra/clang-tidy/utils/LexerUtils.h
index 8330266e6341..2c4a2518259d 100644
--- a/clang-tools-extra/clang-tidy/utils/LexerUtils.h
+++ b/clang-tools-extra/clang-tidy/utils/LexerUtils.h
@@ -80,6 +80,11 @@ SourceLocation findNextAnyTokenKind(SourceLocation Start,
   }
 }
 
+// Finds next token that's not a comment.
+Optional findNextTokenSkippingComments(SourceLocation Start,
+  const SourceManager ,
+  const LangOptions );
+
 /// Re-lex the provide \p Range and return \c false if either a macro spans
 /// multiple tokens, a pre-processor directive or failure to retrieve the
 /// next token is found, otherwise \c true.

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index fc61bda92541..09dc28ae1715 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ 

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

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

merge_guards_bot added a comment.

Build result: pass - 60204 tests passed, 0 failed and 732 were skipped.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70512



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

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This fixes:

- https://github.com/clangd/clangd/issues/126
- https://github.com/clangd/clangd/issues/202

(I've amended the commit message)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70512



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

sunfish wrote:
> dschuff wrote:
> > sunfish wrote:
> > > dschuff wrote:
> > > > What would you think about adding a way to pass arguments through to 
> > > > wasm-opt on the command line, like we have for the linker, assembler, 
> > > > etc? Something like `-Wo,-O2` (or `-Ww` or whatever; analogous to `-Wl` 
> > > > and `-Wa`). That seems nicer than an env var, although it doesn't solve 
> > > > the problem of controlling whether to run the optimizer in the first 
> > > > place.
> > > My guess here is that we don't need clang to have an option for passing 
> > > additional flags -- if people want to do extra special things with 
> > > wasm-opt on clang's output they can just run wasm-opt directly 
> > > themselves. Does that sound reasonable?
> > Maybe. But I still don't like the use of an env var for this kind of 
> > behavior-effecting (i.e. non-debugging) use case.  It's hard enough to get 
> > reproducible and hermetic build behavior as it is, I definitely wouldn't 
> > want to worry about the environment affecting the output in addition to all 
> > the random flags, included files, etc.
> If we did -Wo,-O2 or so, we'd still need to be able to find the wasm-opt 
> program to be able to run it. We could just search for it in PATH, but that's 
> also a little dicey from a hermetic build perspective.
> 
> I borrowed "WASM_OPT" from 
> [cargo-wasi](https://github.com/bytecodealliance/cargo-wasi). I'm also not a 
> fan of environment variables in general, but this way does have the advantage 
> that people can set it once, and not have to modify their Makefiles to add 
> new flags. Users can think of it as just being part of -O2 and friends.
> 
What's the usual way to locate things like external assemblers? Presumably we 
could use the same mechanism for wasm-opt?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70512: [clangd] Rethink how SelectionTree deals with macros and #includes.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay.
Herald added a project: clang.

The exclusive-claim model is successful at resolving conflicts over tokens
between parent/child or siblings. However it's not the right model for selecting
AST nodes produced by a macro expansion, which can produce several independent
nodes that are equally associated with the macro invocation.
Additionally, any model that only uses the endpoints in a range can fail when
a macro invocation occurs inside the node.

To address this, we use the existing TokenBuffer in more depth. SourceRanges
are translated into an array of expanded tokens we can iterate over, and in
principle process token by token (in practice, batching related tokens helps).

For regular tokens (and macro-arg expansions) we claim the tokens as before,
but for tokens from macro bodies we merely check whether the macro name was
selected. Thus tokens in macro bodies are selected by selecting the macro name.

The aggregation of Selection is now more principled as we need to be able to
call claim()/peek() an arbitrary number of times.

One side-effect of iterating over the expanded tokens is that (usually) nothing
claims preprocessor tokens like macro names and directives.
Rather than fixing this I just left them unclaimed, and removed support for
determining the selectedness of TUDecl.
(That was originally implemented in 90a5bf92ff97b1, but doesn't seem to be very
important or worth the complexity any longer).

The expandedTokens(SourceLocation) helper could be added locally, but seems to
make sense on TokenBuffer.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70512

Files:
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/Selection.h
  clang-tools-extra/clangd/unittests/SelectionTests.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp
  clang/include/clang/Tooling/Syntax/Tokens.h
  clang/lib/Tooling/Syntax/Tokens.cpp
  clang/unittests/Tooling/Syntax/TokensTest.cpp

Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -40,6 +40,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Testing/Support/Annotations.h"
 #include "llvm/Testing/Support/SupportHelpers.h"
+#include "gmock/gmock.h"
 #include 
 #include 
 #include 
@@ -663,6 +664,20 @@
   ValueIs(SameRange(findSpelled("not_mapped";
 }
 
+TEST_F(TokenBufferTest, ExpandedTokensForRange) {
+  recordTokens(R"cpp(
+#define SIGN(X) X##_washere
+A SIGN(B) C SIGN(D) E SIGN(F) G
+  )cpp");
+
+  SourceRange R(findExpanded("C").front().location(),
+findExpanded("F_washere").front().location());
+  // Sanity check: expanded and spelled tokens are stored separately.
+  EXPECT_THAT(Buffer.expandedTokens(R),
+  SameRange(findExpanded("C D_washere E F_washere")));
+  EXPECT_THAT(Buffer.expandedTokens(SourceRange()), testing::IsEmpty());
+}
+
 TEST_F(TokenBufferTest, ExpansionStartingAt) {
   // Object-like macro expansions.
   recordTokens(R"cpp(
Index: clang/lib/Tooling/Syntax/Tokens.cpp
===
--- clang/lib/Tooling/Syntax/Tokens.cpp
+++ clang/lib/Tooling/Syntax/Tokens.cpp
@@ -119,6 +119,22 @@
   return Text.substr(Begin, length());
 }
 
+llvm::ArrayRef TokenBuffer::expandedTokens(SourceRange R) const {
+  if (R.isInvalid())
+return {};
+  const Token *Begin =
+  llvm::partition_point(expandedTokens(), [&](const syntax::Token ) {
+return SourceMgr->isBeforeInTranslationUnit(T.location(), R.getBegin());
+  });
+  const Token *End =
+  llvm::partition_point(expandedTokens(), [&](const syntax::Token ) {
+return !SourceMgr->isBeforeInTranslationUnit(R.getEnd(), T.location());
+  });
+  if (Begin > End)
+return {};
+  return {Begin, End};
+}
+
 std::pair
 TokenBuffer::spelledForExpandedToken(const syntax::Token *Expanded) const {
   assert(Expanded);
Index: clang/include/clang/Tooling/Syntax/Tokens.h
===
--- clang/include/clang/Tooling/Syntax/Tokens.h
+++ clang/include/clang/Tooling/Syntax/Tokens.h
@@ -182,6 +182,10 @@
 return ExpandedTokens;
   }
 
+  /// Returns the subrange of expandedTokens() corresponding to the closed
+  /// token range R.
+  llvm::ArrayRef expandedTokens(SourceRange R) const;
+
   /// Find the subrange of spelled tokens that produced the corresponding \p
   /// Expanded tokens.
   ///
Index: clang-tools-extra/clangd/unittests/TweakTests.cpp
===
--- clang-tools-extra/clangd/unittests/TweakTests.cpp
+++ clang-tools-extra/clangd/unittests/TweakTests.cpp
@@ -269,7 +269,7 @@
   

[PATCH] D59321: WIP: AMDGPU: Teach toolchain to link rocm device libs

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/lib/Driver/ToolChains/AMDGPU.cpp:94
+llvm::sys::path::append(IncludePath, InstallPath, "include");
+#if 0
+llvm::sys::path::append(LibDevicePath, InstallPath, "lib",

debugging code needs to be removed


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

https://reviews.llvm.org/D59321



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

[PATCH] D59321: WIP: AMDGPU: Teach toolchain to link rocm device libs

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Add Scott since this may affect comgr. Probably need to add -nodefaultlibs in 
comgr after this change.


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

https://reviews.llvm.org/D59321



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

dschuff wrote:
> sunfish wrote:
> > dschuff wrote:
> > > What would you think about adding a way to pass arguments through to 
> > > wasm-opt on the command line, like we have for the linker, assembler, 
> > > etc? Something like `-Wo,-O2` (or `-Ww` or whatever; analogous to `-Wl` 
> > > and `-Wa`). That seems nicer than an env var, although it doesn't solve 
> > > the problem of controlling whether to run the optimizer in the first 
> > > place.
> > My guess here is that we don't need clang to have an option for passing 
> > additional flags -- if people want to do extra special things with wasm-opt 
> > on clang's output they can just run wasm-opt directly themselves. Does that 
> > sound reasonable?
> Maybe. But I still don't like the use of an env var for this kind of 
> behavior-effecting (i.e. non-debugging) use case.  It's hard enough to get 
> reproducible and hermetic build behavior as it is, I definitely wouldn't want 
> to worry about the environment affecting the output in addition to all the 
> random flags, included files, etc.
If we did -Wo,-O2 or so, we'd still need to be able to find the wasm-opt 
program to be able to run it. We could just search for it in PATH, but that's 
also a little dicey from a hermetic build perspective.

I borrowed "WASM_OPT" from 
[cargo-wasi](https://github.com/bytecodealliance/cargo-wasi). I'm also not a 
fan of environment variables in general, but this way does have the advantage 
that people can set it once, and not have to modify their Makefiles to add new 
flags. Users can think of it as just being part of -O2 and friends.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



___
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

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@rnk : up ?


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

Re: [PATCH] D70284: Constant strings emitted when `-fno-constant-cfstrings` is passed should allow dead stripping

[Ben D. Jones via cfe-commits]

Yup will do. 

Sent from my iPhone

- Ben D. Jones

> On Nov 19, 2019, at 6:32 PM, Akira Hatanaka  wrote:
> 
> Can you check the `#0` at the end of the globals and other strings that 
> precede that? If you do so, we can also check that `no_dead_strip` isn’t 
> added.
> 
>> On Nov 19, 2019, at 6:05 PM, Duncan P. N. Exon Smith via Phabricator via 
>> cfe-commits  wrote:
>> 
>> dexonsmith added a comment.
>> 
>> For some reason this revision is locked down and I'm not allowed to "edit" 
>> it, which includes adding inline review comments.  Can you add me as a 
>> reviewer?
>> 
>> The two comments:
>> 
>> - Please add a period at the end of the sentence in the comment.
>> - Can you give more context about what `objc_arc_inert` is doing, and why 
>> it's necessary now that `no_dead_strip` is gone?
>> 
>> 
>> Repository:
>> rG LLVM Github Monorepo
>> 
>> CHANGES SINCE LAST ACTION
>> https://reviews.llvm.org/D70284/new/
>> 
>> https://reviews.llvm.org/D70284
>> 
>> 
>> 
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 7342912 - [clang][IFS] Driver Pipeline: generate stubs after standard pipeline (3)

[Puyan Lotfi via cfe-commits]

Author: Puyan Lotfi
Date: 2019-11-20T16:22:50-05:00
New Revision: 73429126c91c2065c6f6ef29b3eec1b7798502bb

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

LOG: [clang][IFS] Driver Pipeline: generate stubs after standard pipeline (3)

Third Landing Attempt (dropping any linker invocation from clang driver):

Up until now, clang interface stubs has replaced the standard
PP -> C -> BE -> ASM -> LNK pipeline. With this change, it will happen in
conjunction with it. So what when you build your code you will get an
a.out or lib.so as well as an interface stub file.

Example:

clang -shared -o libfoo.so -emit-interface-stubs ...

will generate both a libfoo.so and a libfoo.ifso. The .so file will
contain the code from the standard compilation pipeline and the .ifso
file will contain the ELF stub library.

Note: For driver-test.c I've added -S in order to prevent any bot failures on
bots that don't have the proper linker for their native triple. You could always
specify a triple like x86_64-unknown-linux-gnu and on bots like x86_64-scei-ps4
the clang driver would invoke regular ld instead of getting the error
'Executable "orbis-ld" doesn't exist!' but on bots like ppc64be and s390x you'd
get an error "/usr/bin/ld: unrecognised emulation mode: elf_x86_64"

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

Added: 
clang/test/InterfaceStubs/driver-test2.c
clang/test/InterfaceStubs/ppc.cpp

Modified: 
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChains/InterfaceStubs.cpp
clang/lib/Driver/Types.cpp
clang/test/InterfaceStubs/driver-test.c
clang/test/InterfaceStubs/windows.cpp

Removed: 




diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index cdf4a579f431..83b5db3b2530 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -292,10 +292,6 @@ phases::ID Driver::getFinalPhase(const DerivedArgList ,
  (PhaseArg = DAL.getLastArg(options::OPT_emit_ast))) {
 FinalPhase = phases::Compile;
 
-  // clang interface stubs
-  } else if ((PhaseArg = DAL.getLastArg(options::OPT_emit_interface_stubs))) {
-FinalPhase = phases::IfsMerge;
-
   // -S only runs up to the backend.
   } else if ((PhaseArg = DAL.getLastArg(options::OPT_S))) {
 FinalPhase = phases::Backend;
@@ -3502,6 +3498,68 @@ void Driver::BuildActions(Compilation , DerivedArgList 
,
 Actions.push_back(
 C.MakeAction(MergerInputs, types::TY_Image));
 
+  if (Arg *A = Args.getLastArg(options::OPT_emit_interface_stubs)) {
+llvm::SmallVector PhaseList;
+if (Args.hasArg(options::OPT_c)) {
+  llvm::SmallVector 
CompilePhaseList;
+  types::getCompilationPhases(types::TY_IFS_CPP, CompilePhaseList);
+  llvm::copy_if(CompilePhaseList, std::back_inserter(PhaseList),
+[&](phases::ID Phase) { return Phase <= phases::Compile; 
});
+} else {
+  types::getCompilationPhases(types::TY_IFS_CPP, PhaseList);
+}
+
+ActionList MergerInputs;
+
+for (auto  : Inputs) {
+  types::ID InputType = I.first;
+  const Arg *InputArg = I.second;
+
+  // Currently clang and the llvm assembler do not support generating 
symbol
+  // stubs from assembly, so we skip the input on asm files. For ifs files
+  // we rely on the normal pipeline setup in the pipeline setup code above.
+  if (InputType == types::TY_IFS || InputType == types::TY_PP_Asm ||
+  InputType == types::TY_Asm)
+continue;
+
+  Action *Current = C.MakeAction(*InputArg, InputType);
+
+  for (auto Phase : PhaseList) {
+switch (Phase) {
+default:
+  llvm_unreachable(
+  "IFS Pipeline can only consist of Compile followed by 
IfsMerge.");
+case phases::Compile: {
+  // Only IfsMerge (llvm-ifs) can handle .o files by looking for ifs
+  // files where the .o file is located. The compile action can not
+  // handle this.
+  if (InputType == types::TY_Object)
+break;
+
+  Current = C.MakeAction(Current, types::TY_IFS_CPP);
+  break;
+}
+case phases::IfsMerge: {
+  assert(Phase == PhaseList.back() &&
+ "merging must be final compilation step.");
+  MergerInputs.push_back(Current);
+  Current = nullptr;
+  break;
+}
+}
+  }
+
+  // If we ended with something, add to the output list.
+  if (Current)
+Actions.push_back(Current);
+}
+
+// Add an interface stubs merge action if necessary.
+if (!MergerInputs.empty())
+  Actions.push_back(
+  C.MakeAction(MergerInputs, types::TY_Image));
+  }
+
   // If --print-supported-cpus, -mcpu=? or -mtune=? is specified, build a 
custom
   // Compile 

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish updated this revision to Diff 230322.
sunfish added a comment.

In D70500#1754012 , @sbc100 wrote:

> On I just remember why this is probably a bad idea.   llvm bitcode is not 
> designed to be stable,  unlike object files, so its probably not a good idea 
> to encourage the distributing of bitcode files in SDKs and such.


That's a good point. I think we can address this by including the LLVM revision 
in the path --I've now updated the patch to do this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/test/Driver/wasm-toolchain-lto.c


Index: clang/test/Driver/wasm-toolchain-lto.c
===
--- /dev/null
+++ clang/test/Driver/wasm-toolchain-lto.c
@@ -0,0 +1,6 @@
+// A basic C link command-line with optimization with known OS and LTO enabled.
+
+// RUN: %clang -### -O2 -flto -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto/
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -8,6 +8,7 @@
 
 #include "WebAssembly.h"
 #include "CommonArgs.h"
+#include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
@@ -90,6 +91,28 @@
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(llvm::make_unique(JA, *this, Linker, CmdArgs, Inputs));
+
+  // When optimizing, if WASM_OPT is set, run wasm-opt.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";
+  if (A->getOption().matches(options::OPT_O4) ||
+  A->getOption().matches(options::OPT_Ofast))
+OOpt = "4";
+  else if (A->getOption().matches(options::OPT_O0))
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();
+
+  const char *WasmOpt = Args.MakeArgString(WasmOptPath);
+  ArgStringList CmdArgs;
+  CmdArgs.push_back(Output.getFilename());
+  CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
+  CmdArgs.push_back("-o");
+  CmdArgs.push_back(Output.getFilename());
+  C.addCommand(llvm::make_unique(JA, *this, WasmOpt, CmdArgs, 
Inputs));
+}
+  }
 }
 
 WebAssembly::WebAssembly(const Driver , const llvm::Triple ,
@@ -109,6 +132,16 @@
   } else {
 const std::string MultiarchTriple =
 getMultiarchTriple(getDriver(), Triple, getDriver().SysRoot);
+if (D.isUsingLTO()) {
+  auto LLVMRevision = getLLVMRevision();
+  if (!LLVMRevision.empty()) {
+// For LTO, enable use of lto-enabled sysroot libraries too, if 
available.
+// Note that the directory is keyed to the LLVM revision, as LLVM's
+// bitcode format is not stable.
+getFilePaths().push_back(getDriver().SysRoot + "/lib/" + 
MultiarchTriple +
+ "/llvm-lto/" + LLVMRevision);
+  }
+}
 getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple);
   }
 }


Index: clang/test/Driver/wasm-toolchain-lto.c
===
--- /dev/null
+++ clang/test/Driver/wasm-toolchain-lto.c
@@ -0,0 +1,6 @@
+// A basic C link command-line with optimization with known OS and LTO enabled.
+
+// RUN: %clang -### -O2 -flto -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto/
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -8,6 +8,7 @@
 
 #include "WebAssembly.h"
 #include "CommonArgs.h"
+#include "clang/Basic/Version.h"
 #include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
@@ -90,6 +91,28 @@
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(llvm::make_unique(JA, *this, Linker, CmdArgs, Inputs));
+
+  // When optimizing, if WASM_OPT is set, run wasm-opt.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";
+  if (A->getOption().matches(options::OPT_O4) ||
+  A->getOption().matches(options::OPT_Ofast))
+OOpt = "4";
+  else 

[PATCH] D69855: [clang-tidy] Fix llvm-namespace-comment for macro expansions

[Marcin Twardak via Phabricator via cfe-commits]

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

@aaron.ballman thanks for the review :) Can you please push the change on my 
behalf? I don't have commit rights.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69855



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

In D70500#1754032 , @dschuff wrote:

> If the SDK is distributed with the compiler and version-locked, it seems like 
> it should be ok.


Right, but if a given SDK chooses to do that, it can always at its own `-L` 
flags.   I'm not sure we want to bake this into the driver.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added a comment.

If the SDK is distributed with the compiler and version-locked, it seems like 
it should be ok.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

sunfish wrote:
> dschuff wrote:
> > What would you think about adding a way to pass arguments through to 
> > wasm-opt on the command line, like we have for the linker, assembler, etc? 
> > Something like `-Wo,-O2` (or `-Ww` or whatever; analogous to `-Wl` and 
> > `-Wa`). That seems nicer than an env var, although it doesn't solve the 
> > problem of controlling whether to run the optimizer in the first place.
> My guess here is that we don't need clang to have an option for passing 
> additional flags -- if people want to do extra special things with wasm-opt 
> on clang's output they can just run wasm-opt directly themselves. Does that 
> sound reasonable?
Maybe. But I still don't like the use of an env var for this kind of 
behavior-effecting (i.e. non-debugging) use case.  It's hard enough to get 
reproducible and hermetic build behavior as it is, I definitely wouldn't want 
to worry about the environment affecting the output in addition to all the 
random flags, included files, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70499: [clang] Fix the path to CrossWinToARMLinux.cmake CMake cache

[Vlad Vereschaka via Phabricator via cfe-commits]

vvereschaka added a comment.

Done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70499



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

[PATCH] D70499: [clang] Fix the path to CrossWinToARMLinux.cmake CMake cache

[Vlad Vereschaka via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0f5aabb91a03: [CMake] Fix the path to 
CrossWinToARMLinux.cmake CMake cache. (authored by vvereschaka).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70499

Files:
  clang/cmake/caches/CrossWinToARMLinux.cmake


Index: clang/cmake/caches/CrossWinToARMLinux.cmake
===
--- clang/cmake/caches/CrossWinToARMLinux.cmake
+++ clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -14,7 +14,7 @@
 #   -DDEFAULT_SYSROOT= ^
 #   -DLLVM_AR=/bin/llvm-ar[.exe] ^
 #   -DCMAKE_CXX_FLAGS="-D__OPTIMIZE__" ^
-#   -C/llvm-project/clang/caches/CrossWinToARMLinux.cmake ^
+#   
-C/llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake ^
 #   /llvm-project/llvm
 # Build:
 #  cmake --build . --target install


Index: clang/cmake/caches/CrossWinToARMLinux.cmake
===
--- clang/cmake/caches/CrossWinToARMLinux.cmake
+++ clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -14,7 +14,7 @@
 #   -DDEFAULT_SYSROOT= ^
 #   -DLLVM_AR=/bin/llvm-ar[.exe] ^
 #   -DCMAKE_CXX_FLAGS="-D__OPTIMIZE__" ^
-#   -C/llvm-project/clang/caches/CrossWinToARMLinux.cmake ^
+#   -C/llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake ^
 #   /llvm-project/llvm
 # Build:
 #  cmake --build . --target install
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 0f5aabb - [CMake] Fix the path to CrossWinToARMLinux.cmake CMake cache.

[Vladimir Vereschaka via cfe-commits]

Author: Vladimir Vereschaka
Date: 2019-11-20T12:51:52-08:00
New Revision: 0f5aabb91a03b40635819f71187333dd9535b9de

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

LOG: [CMake] Fix the path to CrossWinToARMLinux.cmake CMake cache.

The comment was slightly misleading.

Behalf: broadwaylamb (Sergej Jaskiewicz)

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

Added: 


Modified: 
clang/cmake/caches/CrossWinToARMLinux.cmake

Removed: 




diff  --git a/clang/cmake/caches/CrossWinToARMLinux.cmake 
b/clang/cmake/caches/CrossWinToARMLinux.cmake
index 246f6241f0e2..944465411363 100644
--- a/clang/cmake/caches/CrossWinToARMLinux.cmake
+++ b/clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -14,7 +14,7 @@
 #   -DDEFAULT_SYSROOT= ^
 #   -DLLVM_AR=/bin/llvm-ar[.exe] ^
 #   -DCMAKE_CXX_FLAGS="-D__OPTIMIZE__" ^
-#   -C/llvm-project/clang/caches/CrossWinToARMLinux.cmake ^
+#   
-C/llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake ^
 #   /llvm-project/llvm
 # Build:
 #  cmake --build . --target install



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

On I just remember why this is probably a bad idea.   llvm bitcode is not 
designed to be stable,  unlike object files, so its probably not a good idea to 
encourage the distributing of bitcode files in SDKs and such.Unless you 
stronly disagree maybe a good idea to split this patch in two since the 
WASM_OPT part is non-controversial and logically separate?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Alexey Bader via Phabricator via cfe-commits]

bader marked 3 inline comments as done.
bader added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10079-10080
+def warn_sycl_kernel_invalid_template_param_type : Warning<
+  "template parameter of template functions with 'sycl_kernel' attribute must"
+  " be typename">, InGroup;
+def warn_sycl_kernel_num_of_function_params : Warning<

aaron.ballman wrote:
> bader wrote:
> > aaron.ballman wrote:
> > > This diagnostic reads a bit like you cannot do this: `template ` 
> > > when I think the actual restriction is that you cannot do this: `template 
> > > `. Is that correct? If so, I think this could be worded as 
> > > `template parameter of a function template with the 'sycl_kernel' 
> > > attribute must be a template type parameter`.
> > > 
> > > Just double-checking, but you also intend to prohibit template template 
> > > parameters? e.g., you can't do `template  typename C>`
> > > This diagnostic reads a bit like you cannot do this: template  
> > > when I think the actual restriction is that you cannot do this: template 
> > > . Is that correct?
> > 
> > Yes. That is correct.
> > 
> > >  If so, I think this could be worded as template parameter of a function 
> > > template with the 'sycl_kernel' attribute must be a template type 
> > > parameter.
> > 
> > Thanks! Applied your wording.
> > 
> > > Just double-checking, but you also intend to prohibit template template 
> > > parameters? e.g., you can't do template  typename C>
> > 
> > Currently we allow following use case: 
> > https://github.com/intel/llvm/blob/sycl/clang/test/SemaSYCL/mangle-kernel.cpp.
> >  I assume it qualifies as "template type" and not as "template template" 
> > parameter. Right?
> > 
> > Quoting SYCL specification $6.2 Naming of kernels 
> > (https://www.khronos.org/registry/SYCL/specs/sycl-1.2.1.pdf#page=250).
> > 
> > > SYCL kernels are extracted from C++ source files and stored in an 
> > > implementation- defined format. In the case of
> > > the shared-source compilation model, the kernels have to be uniquely 
> > > identified by both host and device compiler.
> > > This is required in order for the host runtime to be able to load the 
> > > kernel by using the OpenCL host runtime
> > > interface.
> > > From this requirement the following rules apply for naming the kernels:
> > > • The kernel name is a C++ typename.
> > > • The kernel needs to have a globally-visible name. In the case of a 
> > > named function object type, the name can
> > > be the typename of the function object, as long as it is 
> > > globally-visible. In the case where it isn’t, a globally visible name has 
> > > to be provided, as template parameter to the kernel invoking interface, 
> > > as described in 4.8.5.
> > > In C++11, lambdas do not have a globally-visible name, so a 
> > > globally-visible typename has to be provided
> > > in the kernel invoking interface, as described in 4.8.5.
> > > • The kernel name has to be a unique identifier in the program.
> > > 
> > 
> > We also have an extension, which lifts these restrictions/requirements when 
> > clang is used as host and device compiler. @erichkeane implemented built-in 
> > function (https://github.com/intel/llvm/pull/250) providing "unique 
> > identifier", which we use as a kernel name for lambda objects. But this is 
> > going to be a separate patch.
> > Currently we allow following use case: 
> > https://github.com/intel/llvm/blob/sycl/clang/test/SemaSYCL/mangle-kernel.cpp.
> >  I assume it qualifies as "template type" and not as "template template" 
> > parameter. Right?
> 
> Yeah, those are template types. A template template parameter would be: 
> https://godbolt.org/z/9kwbW9
> In that example, `C` is a template template parameter and `Ty` is a template 
> type parameter. The part I'm a bit unclear on is why a template template 
> parameter should be disallowed (I believe it names a type, as opposed to a 
> non-type template parameter which names a value)?
I think Mariya implemented this restriction to avoid usages not required for 
SYCL kernel support implementation in run-time library. As it was mentioned 
before, this attribute is intended to be used by SYCL run-time library only and 
current implantation do not require `template template parameter` support.
I think that this might be useful for alternative implementations, so I updated 
the patch to restrict non-type template parameters only.



Comment at: clang/test/Misc/pragma-attribute-supported-attributes-list.test:134
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)

aaron.ballman wrote:
> bader wrote:
> > It looks like this change is not needed anymore. This check fails on my 
> > machine with the latest version of the patch.
> > 
> > @aaron.ballman, I'm not sure if this is a problem of the implementation or 
> 

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:137
+  getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple 
+
+   "/llvm-lto");
+}

sbc100 wrote:
> Is there any kind of president here?   i.e. do any other platforms have 
> support for this kind thing?  Seems like good idea I'd just like to be sure 
> we follow existing practices.
I looked around, but didn't see anything similar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70416: [Driver] Make -static-libgcc imply static libunwind

[Josh Kunz via Phabricator via cfe-commits]

jkz added a comment.

In D70416#1750978 , @joerg wrote:

> This is normally done by using `-Bstatic`/`-Bdynamic` around the library. See 
> `tools::addOpenMPRuntime`.


`-Bstatic`/`-Bdynamic` wrapping does seem to be more common, but that will 
change the linkage for libraries that are added after the run-time libraries 

 (e.g., `-lpthread`). We would need something like `--push-state -Bstatic 
-lunwind --pop-state` to preserve the "mode" of the link. (IMO, the existing 
usages of `-Bstatic`, `-Bdynamic` wrapping are wrong). The current approach is 
simpler.

Additionally, I think saugustine's reasoning makes sense here, and we should 
match the libgcc behavior of requiring a `.a` or `.so` depending on the "mode". 
I'd prefer to keep this patch the way it is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70416



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

Kinds seems like two different changes here.   But lgtm.




Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:137
+  getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple 
+
+   "/llvm-lto");
+}

Is there any kind of president here?   i.e. do any other platforms have support 
for this kind thing?  Seems like good idea I'd just like to be sure we follow 
existing practices.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70499: [clang] Fix the path to CrossWinToARMLinux.cmake CMake cache

[Sergej Jaskiewicz via Phabricator via cfe-commits]

broadwaylamb added a comment.

Can you commit this for me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70499



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

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 230310.
bader added a comment.

Applied code review comments from Aaron.

Allow template template parameters for function templates marked with 
`sycl_kernel` attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenSYCL/device-functions.cpp
  clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
  clang/test/SemaSYCL/device-attributes.cpp

Index: clang/test/SemaSYCL/device-attributes.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+
+[[clang::sycl_kernel]] int gv2 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+__attribute__((sycl_kernel)) int gv3 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+__attribute__((sycl_kernel(1))) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+// Only function templates
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel]] void foo1(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+// At least two template parameters
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{'sycl_kernel' attribute only applies to a function template with at least two template parameters}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{'sycl_kernel' attribute only applies to a function template with at least two template parameters}}
+
+// First two template parameters can't be non-type template parameters
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{template parameter of a function template with the 'sycl_kernel' attribute can't be a non-type template parameter}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{template parameter of a function template with the 'sycl_kernel' attribute can't be a non-type template parameter}}
+
+// Must return void
+template 
+__attribute__((sycl_kernel)) int foo(T P); // expected-warning {{function template with 'sycl_kernel' attribute must have a 'void' return type}}
+template 
+[[clang::sycl_kernel]] int foo1(T P); // expected-warning {{function template with 'sycl_kernel' attribute must have a 'void' return type}}
+
+// Must take at least one argument
+template 
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{function template with 'sycl_kernel' attribute must have a single parameter}}
+template 
+[[clang::sycl_kernel]] void foo1(T t, A a); // expected-warning {{function template with 'sycl_kernel' attribute must have a single parameter}}
+
+// No diagnostics
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -x c++ %s
+
+#ifndef __SYCL_DEVICE_ONLY__
+// expected-warning@+7 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+8 {{'sycl_kernel' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
Index: clang/test/CodeGenSYCL/device-functions.cpp
===
--- /dev/null
+++ clang/test/CodeGenSYCL/device-functions.cpp
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -triple spir64 -fsycl-is-device -S -emit-llvm %s -o - | FileCheck %s
+
+template 
+T bar(T arg);
+
+void foo() {
+  int a = 1 + 1 + bar(1);
+}
+
+template 
+T bar(T arg) {
+  return arg;
+}
+
+template 
+__attribute__((sycl_kernel)) void kernel_single_task(Func kernelFunc) {
+  kernelFunc();
+}
+
+// Make sure that definitions for the types not used in SYCL kernels are not
+// emitted
+// CHECK-NOT: %struct.A
+// CHECK-NOT: @a = {{.*}} %struct.A
+struct A {
+  int x = 10;
+} a;
+
+int main() {
+  a.x = 8;
+  kernel_single_task([]() { foo(); });
+  return 0;
+}
+
+// baz is not called from the SYCL kernel, so it must not be emitted
+// CHECK-NOT: define {{.*}} @{{.*}}baz
+void baz() {}
+

[clang] fd8d915 - Fix parser bug that permitted 'private' as a (no-op) decl-specifier even outside OpenCL.

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-11-20T11:59:58-08:00
New Revision: fd8d9155a997ab0f3ef3d7dff1c56efc9b692bfe

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

LOG: Fix parser bug that permitted 'private' as a (no-op) decl-specifier even 
outside OpenCL.

Added: 


Modified: 
clang/lib/Parse/ParseDecl.cpp
clang/test/Parser/cxx-decl.cpp

Removed: 




diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index e5c17a3131ab..a90147ca4692 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -3949,9 +3949,14 @@ void Parser::ParseDeclarationSpecifiers(DeclSpec ,
 PrevSpec = Tok.getIdentifierInfo()->getNameStart();
 isInvalid = true;
 break;
-  };
+  }
   LLVM_FALLTHROUGH;
 case tok::kw_private:
+  // It's fine (but redundant) to check this for __generic on the
+  // fallthrough path; we only form the __generic token in OpenCL mode.
+  if (!getLangOpts().OpenCL)
+goto DoneWithDeclSpec;
+  LLVM_FALLTHROUGH;
 case tok::kw___private:
 case tok::kw___global:
 case tok::kw___local:

diff  --git a/clang/test/Parser/cxx-decl.cpp b/clang/test/Parser/cxx-decl.cpp
index c60e42f28eef..a868904bb36d 100644
--- a/clang/test/Parser/cxx-decl.cpp
+++ b/clang/test/Parser/cxx-decl.cpp
@@ -6,6 +6,8 @@ const char const *x10; // expected-error {{duplicate 'const' 
declaration specifi
 
 int x(*g); // expected-error {{use of undeclared identifier 'g'}}
 
+private int cplusplus_is_not_opencl; // expected-error {{expected 
unqualified-id}}
+
 struct Type {
   int Type;
 };



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

[PATCH] D70499: [clang] Fix the path to CrossWinToARMLinux.cmake CMake cache

[Vlad Vereschaka via Phabricator via cfe-commits]

vvereschaka accepted this revision.
vvereschaka added a comment.

Thank you Sergej.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70499



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

[clang-tools-extra] 6de4577 - Revert "[clangd] Fix a crash in expected types"

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2019-11-20T14:38:35-05:00
New Revision: 6de45772e0910bf7fa626e5493a2798b071eb26c

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

LOG: Revert "[clangd] Fix a crash in expected types"

This reverts commit b5135a86e04761577494c70e7c0057136cc90b5b.
Test fails on Windows.

Added: 


Modified: 
clang-tools-extra/clangd/ExpectedTypes.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ExpectedTypes.cpp 
b/clang-tools-extra/clangd/ExpectedTypes.cpp
index a82a64cf14e2..3b0779ea66bc 100644
--- a/clang-tools-extra/clangd/ExpectedTypes.cpp
+++ b/clang-tools-extra/clangd/ExpectedTypes.cpp
@@ -44,10 +44,12 @@ static const Type *toEquivClass(ASTContext , QualType 
T) {
 static llvm::Optional
 typeOfCompletion(const CodeCompletionResult ) {
   const NamedDecl *D = R.Declaration;
+  if (!D)
+return llvm::None;
   // Templates do not have a type on their own, look at the templated decl.
-  if (auto *Template = dyn_cast_or_null(D))
+  if (auto *Template = dyn_cast(D))
 D = Template->getTemplatedDecl();
-  auto *VD = dyn_cast_or_null(D);
+  auto *VD = dyn_cast(D);
   if (!VD)
 return llvm::None; // We handle only variables and functions below.
   auto T = VD->getType();

diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp 
b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index e69b2a6205f6..5b50b9fe9f8b 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1030,16 +1030,6 @@ TEST(CompletionTest, DefaultArgs) {
 SnippetSuffix("(${1:int A})";
 }
 
-TEST(CompletionTest, NoCrashWithTemplateParamsAndPreferredTypes) {
-  auto Completions = completions(R"cpp(
-template  class TT> int foo() {
-  int a = ^
-}
-)cpp")
- .Completions;
-  EXPECT_THAT(Completions, Contains(Named("TT")));
-}
-
 SignatureHelp signatures(llvm::StringRef Text, Position Point,
  std::vector IndexSymbols = {}) {
   std::unique_ptr Index;



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

[PATCH] D70041: register cuda language activation event and activate for .cuh files

[Paul Taylor via Phabricator via cfe-commits]

ptaylor updated this revision to Diff 230301.
ptaylor added a comment.

- add package.json entry to contribute cuda language id
- remove extra cuda file patterns, update comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70041

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/package.json
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -83,21 +83,18 @@
   }
   const serverOptions: vscodelc.ServerOptions = clangd;
 
-  // Note that CUDA ('.cu') files are special. When opening files of all other
-  // extensions, VSCode would load clangd automatically. This is achieved by
-  // having a corresponding 'onLanguage:...' activation event in package.json.
-  // However, VSCode does not have CUDA as a supported language yet, so we
-  // cannot add a corresponding activationEvent for CUDA files and clangd will
-  // *not* load itself automatically on '.cu' files.
-  const cudaFilePattern: string = '**/*.{' + [ 'cu' ].join() + '}';
   const clientOptions: vscodelc.LanguageClientOptions = {
 // Register the server for c-family and cuda files.
 documentSelector: [
 { scheme: 'file', language: 'c' },
 { scheme: 'file', language: 'cpp' },
+// Syntax highlighting for the 'cuda' language is
+// provided by the kriegalex.vscode-cudacpp plugin.
+// @see https://github.com/kriegalex/vscode-cuda
+// @see 
https://marketplace.visualstudio.com/items?itemName=kriegalex.vscode-cudacpp
+{ scheme: 'file', language: 'cuda' },
 { scheme: 'file', language: 'objective-c'},
-{ scheme: 'file', language: 'objective-cpp'},
-{ scheme: 'file', pattern: cudaFilePattern },
+{ scheme: 'file', language: 'objective-cpp'}
 ],
 synchronize: !syncFileEvents ? undefined : {
 // FIXME: send sync file events when clangd provides implemenatations.
@@ -111,10 +108,10 @@
 serverOptions, clientOptions);
   if (getConfig('semanticHighlighting')) {
 const semanticHighlightingFeature =
-  new semanticHighlighting.SemanticHighlightingFeature(clangdClient,
-context);
+new semanticHighlighting.SemanticHighlightingFeature(clangdClient,
+ context);
 context.subscriptions.push(
-  vscode.Disposable.from(semanticHighlightingFeature));
+vscode.Disposable.from(semanticHighlightingFeature));
 clangdClient.registerFeature(semanticHighlightingFeature);
   }
   console.log('Clang Language Server is now active!');
Index: clang-tools-extra/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/clangd/clients/clangd-vscode/package.json
@@ -23,6 +23,7 @@
 "activationEvents": [
 "onLanguage:c",
 "onLanguage:cpp",
+"onLanguage:cuda",
 "onLanguage:objective-c",
 "onLanguage:objective-cpp",
 "onCommand:clangd-vscode.activate"
@@ -64,7 +65,14 @@
 "**/MSVC/*/include/**"
 ],
 "firstLine": "^/[/*].*-\\*-\\s*C\\+\\+\\s*-\\*-.*"
-}
+},
+   {
+   "id": "cuda",
+   "extensions": [
+   ".cu",
+   ".cuh"
+   ]
+   }
 ],
 "configuration": {
 "type": "object",


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -83,21 +83,18 @@
   }
   const serverOptions: vscodelc.ServerOptions = clangd;
 
-  // Note that CUDA ('.cu') files are special. When opening files of all other
-  // extensions, VSCode would load clangd automatically. This is achieved by
-  // having a corresponding 'onLanguage:...' activation event in package.json.
-  // However, VSCode does not have CUDA as a supported language yet, so we
-  // cannot add a corresponding activationEvent for CUDA files and clangd will
-  // *not* load itself automatically on '.cu' files.
-  const cudaFilePattern: string = '**/*.{' + [ 'cu' ].join() + '}';
   const clientOptions: vscodelc.LanguageClientOptions = {
 // Register 

[PATCH] D70481: [clangd] Fix a crash in expected types

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Reverted for now in 6de45772e0 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70481



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

[PATCH] D70041: register cuda language activation event and activate for .cuh files

[Paul Taylor via Phabricator via cfe-commits]

ptaylor updated this revision to Diff 230302.
ptaylor added a comment.

- fix package.json whitespace


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70041

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/package.json
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -83,21 +83,18 @@
   }
   const serverOptions: vscodelc.ServerOptions = clangd;
 
-  // Note that CUDA ('.cu') files are special. When opening files of all other
-  // extensions, VSCode would load clangd automatically. This is achieved by
-  // having a corresponding 'onLanguage:...' activation event in package.json.
-  // However, VSCode does not have CUDA as a supported language yet, so we
-  // cannot add a corresponding activationEvent for CUDA files and clangd will
-  // *not* load itself automatically on '.cu' files.
-  const cudaFilePattern: string = '**/*.{' + [ 'cu' ].join() + '}';
   const clientOptions: vscodelc.LanguageClientOptions = {
 // Register the server for c-family and cuda files.
 documentSelector: [
 { scheme: 'file', language: 'c' },
 { scheme: 'file', language: 'cpp' },
+// Syntax highlighting for the 'cuda' language is
+// provided by the kriegalex.vscode-cudacpp plugin.
+// @see https://github.com/kriegalex/vscode-cuda
+// @see 
https://marketplace.visualstudio.com/items?itemName=kriegalex.vscode-cudacpp
+{ scheme: 'file', language: 'cuda' },
 { scheme: 'file', language: 'objective-c'},
-{ scheme: 'file', language: 'objective-cpp'},
-{ scheme: 'file', pattern: cudaFilePattern },
+{ scheme: 'file', language: 'objective-cpp'}
 ],
 synchronize: !syncFileEvents ? undefined : {
 // FIXME: send sync file events when clangd provides implemenatations.
@@ -111,10 +108,10 @@
 serverOptions, clientOptions);
   if (getConfig('semanticHighlighting')) {
 const semanticHighlightingFeature =
-  new semanticHighlighting.SemanticHighlightingFeature(clangdClient,
-context);
+new semanticHighlighting.SemanticHighlightingFeature(clangdClient,
+ context);
 context.subscriptions.push(
-  vscode.Disposable.from(semanticHighlightingFeature));
+vscode.Disposable.from(semanticHighlightingFeature));
 clangdClient.registerFeature(semanticHighlightingFeature);
   }
   console.log('Clang Language Server is now active!');
Index: clang-tools-extra/clangd/clients/clangd-vscode/package.json
===
--- clang-tools-extra/clangd/clients/clangd-vscode/package.json
+++ clang-tools-extra/clangd/clients/clangd-vscode/package.json
@@ -23,6 +23,7 @@
 "activationEvents": [
 "onLanguage:c",
 "onLanguage:cpp",
+"onLanguage:cuda",
 "onLanguage:objective-c",
 "onLanguage:objective-cpp",
 "onCommand:clangd-vscode.activate"
@@ -64,6 +65,13 @@
 "**/MSVC/*/include/**"
 ],
 "firstLine": "^/[/*].*-\\*-\\s*C\\+\\+\\s*-\\*-.*"
+},
+{
+"id": "cuda",
+"extensions": [
+".cu",
+".cuh"
+]
 }
 ],
 "configuration": {


Index: clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
===
--- clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
+++ clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
@@ -83,21 +83,18 @@
   }
   const serverOptions: vscodelc.ServerOptions = clangd;
 
-  // Note that CUDA ('.cu') files are special. When opening files of all other
-  // extensions, VSCode would load clangd automatically. This is achieved by
-  // having a corresponding 'onLanguage:...' activation event in package.json.
-  // However, VSCode does not have CUDA as a supported language yet, so we
-  // cannot add a corresponding activationEvent for CUDA files and clangd will
-  // *not* load itself automatically on '.cu' files.
-  const cudaFilePattern: string = '**/*.{' + [ 'cu' ].join() + '}';
   const clientOptions: vscodelc.LanguageClientOptions = {
 // Register the server for c-family and cuda files.
 documentSelector: [
 { scheme: 'file', language: 'c' },
 { scheme: 'file', language: 'cpp' },
+// Syntax highlighting for the 'cuda' language is

[PATCH] D70488: [InstCombine] Infer fast math flags on fadd/fsub/fmul/fcmp

[Michael Berg via Phabricator via cfe-commits]

mcberg2017 added a comment.

For us this would be an impediment as we have math models that want ieee 
behavior while relaxing precision.  Adding nnan or ninf would obstruct those 
choices.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70488



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

[PATCH] D70481: [clangd] Fix a crash in expected types

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This broke clangd tests on Windows: 
http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/builds/12324

We now have clangd tests running on main llvm bots (namely, on 
http://lab.llvm.org:8011/builders/clang-x64-windows-msvc/). Do they not send 
mail?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70481



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

dschuff wrote:
> What would you think about adding a way to pass arguments through to wasm-opt 
> on the command line, like we have for the linker, assembler, etc? Something 
> like `-Wo,-O2` (or `-Ww` or whatever; analogous to `-Wl` and `-Wa`). That 
> seems nicer than an env var, although it doesn't solve the problem of 
> controlling whether to run the optimizer in the first place.
My guess here is that we don't need clang to have an option for passing 
additional flags -- if people want to do extra special things with wasm-opt on 
clang's output they can just run wasm-opt directly themselves. Does that sound 
reasonable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70477: [Clang] Enable RISC-V support for Fuchsia

[Petr Hosek via Phabricator via cfe-commits]

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



Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:130
 
+  foreach(target x86_64;aarch64)
 # Set the per-target runtimes options.

lenary wrote:
> I don't know what's preventing you from enabling compiling the runtimes. I 
> made a cmake config that built the runtimes, in part based off the fuschia 
> config in this file.
> 
> I suppose this requires you to have a RISC-V sysroot, which you may not have.
Correct, as mentioned in the summary of this change, we don't have a sysroot 
yet. Once the sysroot is ready, we'll enable runtimes as well in a follow up 
change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70477



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

[PATCH] D70111: [DWARF5]Addition of alignment field in the typedef for dwarf5

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Please update the go bindings and include a release note that this API is 
changing & why.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70111



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

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:3450
+def UseHandle : InheritableParamAttr {
+  let Spellings = [Clang<"use_handle">];
+  let Documentation = [UseHandleDocs];

xazax.hun wrote:
> aaron.ballman wrote:
> > Should this have a subject limiting it to parameters?
> My understanding was that the subject list is "inherited" from 
> InheritableParamAttr, which already has it limited to parameters. Is this not 
> the case?
It doesn't seem to be the case looking at Attr.td:557 or thereabouts. Adding 
tests for the attributes should clarify whether the change is needed or not, 
too.



Comment at: clang/include/clang/Basic/AttrDocs.td:4474
+  let Content = [{
+If this annotation is on a function it is assumed to return a new handle.
+In case this annotation is on an output parameter, the function is assumed

xazax.hun wrote:
> aaron.ballman wrote:
> > What is a "handle"? I think some introduction docs are needed.
> Good point. Do you prefer to copy and paste the introduction to all 
> attributes or is it enough to only have it for one of them?
You can set up a documentation category to put all the related attributes 
under, and that category can have the introductory documentation. You can see 
an example of this with the calling convention attributes. There is a 
`DocCatCallingConvs` that is then the `Category` for all the other attributes.


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

https://reviews.llvm.org/D70469



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

[PATCH] D70477: [Clang] Enable RISC-V support for Fuchsia

[Sam Elliott via Phabricator via cfe-commits]

lenary added inline comments.



Comment at: clang/cmake/caches/Fuchsia-stage2.cmake:130
 
+  foreach(target x86_64;aarch64)
 # Set the per-target runtimes options.

I don't know what's preventing you from enabling compiling the runtimes. I made 
a cmake config that built the runtimes, in part based off the fuschia config in 
this file.

I suppose this requires you to have a RISC-V sysroot, which you may not have.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70477



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

[PATCH] D69855: [clang-tidy] Fix llvm-namespace-comment for macro expansions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!




Comment at: 
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:83
+const SourceManager , Preprocessor *PP, Preprocessor *ModuleExpanderPP) 
{
+  PP->addPPCallbacks(std::make_unique(PP, this));
+}

twardakm wrote:
> aaron.ballman wrote:
> > twardakm wrote:
> > > aaron.ballman wrote:
> > > > Rather than making an entire new object for PP callbacks, why not make 
> > > > `NamespaceCommentCheck` inherit from `PPCallbacks`? It seems like it 
> > > > would simplify the interface somewhat.
> > > I think the check hast to inherit from ClangTidyCheck?
> > > 
> > > I duplicated the solution from other checks (e.g. 
> > > IdentifierNamingCheck.cpp). Could you please point to some existing check 
> > > which implements your idea?
> > I don't know if we have other checks doing this -- I was thinking of using 
> > multiple inheritance in this case, because the callbacks are effectively a 
> > mixin.
> > 
> > That said, I don't insist on this change.
> The problem which I see is that addPPCallbacks takes ownership of the object 
> passed to it. I couldn't find any other way of invoking PP callbacks, so I'll 
> leave it as is.
Ah, that makes sense. Thank you for investigating it. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69855



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

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: arphaman.
aaron.ballman added a subscriber: arphaman.
aaron.ballman added inline comments.
Herald added a subscriber: dexonsmith.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10079-10080
+def warn_sycl_kernel_invalid_template_param_type : Warning<
+  "template parameter of template functions with 'sycl_kernel' attribute must"
+  " be typename">, InGroup;
+def warn_sycl_kernel_num_of_function_params : Warning<

bader wrote:
> aaron.ballman wrote:
> > This diagnostic reads a bit like you cannot do this: `template ` 
> > when I think the actual restriction is that you cannot do this: `template 
> > `. Is that correct? If so, I think this could be worded as `template 
> > parameter of a function template with the 'sycl_kernel' attribute must be a 
> > template type parameter`.
> > 
> > Just double-checking, but you also intend to prohibit template template 
> > parameters? e.g., you can't do `template  typename C>`
> > This diagnostic reads a bit like you cannot do this: template  
> > when I think the actual restriction is that you cannot do this: template 
> > . Is that correct?
> 
> Yes. That is correct.
> 
> >  If so, I think this could be worded as template parameter of a function 
> > template with the 'sycl_kernel' attribute must be a template type parameter.
> 
> Thanks! Applied your wording.
> 
> > Just double-checking, but you also intend to prohibit template template 
> > parameters? e.g., you can't do template  typename C>
> 
> Currently we allow following use case: 
> https://github.com/intel/llvm/blob/sycl/clang/test/SemaSYCL/mangle-kernel.cpp.
>  I assume it qualifies as "template type" and not as "template template" 
> parameter. Right?
> 
> Quoting SYCL specification $6.2 Naming of kernels 
> (https://www.khronos.org/registry/SYCL/specs/sycl-1.2.1.pdf#page=250).
> 
> > SYCL kernels are extracted from C++ source files and stored in an 
> > implementation- defined format. In the case of
> > the shared-source compilation model, the kernels have to be uniquely 
> > identified by both host and device compiler.
> > This is required in order for the host runtime to be able to load the 
> > kernel by using the OpenCL host runtime
> > interface.
> > From this requirement the following rules apply for naming the kernels:
> > • The kernel name is a C++ typename.
> > • The kernel needs to have a globally-visible name. In the case of a named 
> > function object type, the name can
> > be the typename of the function object, as long as it is globally-visible. 
> > In the case where it isn’t, a globally visible name has to be provided, as 
> > template parameter to the kernel invoking interface, as described in 4.8.5.
> > In C++11, lambdas do not have a globally-visible name, so a 
> > globally-visible typename has to be provided
> > in the kernel invoking interface, as described in 4.8.5.
> > • The kernel name has to be a unique identifier in the program.
> > 
> 
> We also have an extension, which lifts these restrictions/requirements when 
> clang is used as host and device compiler. @erichkeane implemented built-in 
> function (https://github.com/intel/llvm/pull/250) providing "unique 
> identifier", which we use as a kernel name for lambda objects. But this is 
> going to be a separate patch.
> Currently we allow following use case: 
> https://github.com/intel/llvm/blob/sycl/clang/test/SemaSYCL/mangle-kernel.cpp.
>  I assume it qualifies as "template type" and not as "template template" 
> parameter. Right?

Yeah, those are template types. A template template parameter would be: 
https://godbolt.org/z/9kwbW9
In that example, `C` is a template template parameter and `Ty` is a template 
type parameter. The part I'm a bit unclear on is why a template template 
parameter should be disallowed (I believe it names a type, as opposed to a 
non-type template parameter which names a value)?



Comment at: clang/test/Misc/pragma-attribute-supported-attributes-list.test:134
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)

bader wrote:
> It looks like this change is not needed anymore. This check fails on my 
> machine with the latest version of the patch.
> 
> @aaron.ballman, I'm not sure if this is a problem of the implementation or 
> test issue.
> Do I understand correctly that this test validates the list of the attributes 
> which can be applied using `#pragma clang`?
> If so, removing this check seems to be okay. We need only 
> `[[clang::sycl_kernel]]` or `__attribute__((sycl_kernel))` to work.
Your understanding is correct, and I think it's a bug if you don't need to add 
an entry here for `SYCLKernel`. @arphaman, WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

[PATCH] D70488: [InstCombine] Infer fast math flags on fadd/fsub/fmul/fcmp

[Sanjay Patel via Phabricator via cfe-commits]

spatel added reviewers: cameron.mcinally, mcberg2017, arsenm.
spatel added a comment.
Herald added a subscriber: wdng.

I like the idea, but I'd be more comfortable reviewing the diffs in stages, so 
we know that the test coverage for the value tracking calls is good. So I'd 
prefer if we split this somehow - either by the opcode callers (fadd, fsub, 
fmul...) or the the FMF analysis (nnan, nsz, ninf). That raises a few questions:

1. Why aren't fdiv and frem included?
2. Can we infer FMF for FP intrinsics/libcalls/select/phi? (follow-on patches)
3. We're moving away from FMF on fcmp (recent step: rGebf9bf2cbc8f 
), so is 
it worth including that change, or can we wait for that part to settle? (Side 
question may be if/when we're going to allow FMF on fptrunc/fpextend).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70488



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

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

This is also missing tests for all the Sema handling (that the attributes only 
appertain to the specified subjects, accept no arguments, etc).




Comment at: clang/include/clang/Basic/Attr.td:3445
+  let Spellings = [Clang<"acquire_handle">];
+  let Subjects = SubjectList<[Function, ParmVar], ErrorDiag>;
+  let Documentation = [AcquireHandleDocs];

What about function-like interfaces such as lambdas, blocks, or other callable 
objects that are not a `FunctionDecl`?



Comment at: clang/include/clang/Basic/Attr.td:3450
+def UseHandle : InheritableParamAttr {
+  let Spellings = [Clang<"use_handle">];
+  let Documentation = [UseHandleDocs];

Should this have a subject limiting it to parameters?



Comment at: clang/include/clang/Basic/Attr.td:3455
+def ReleaseHandle : InheritableParamAttr {
+  let Spellings = [Clang<"release_handle">];
+  let Documentation = [ReleaseHandleDocs];

Should this have a subject limiting it to parameters?



Comment at: clang/include/clang/Basic/AttrDocs.td:4474
+  let Content = [{
+If this annotation is on a function it is assumed to return a new handle.
+In case this annotation is on an output parameter, the function is assumed

What is a "handle"? I think some introduction docs are needed.


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

https://reviews.llvm.org/D70469



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

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

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

xazax.hun updated this revision to Diff 230294.
xazax.hun added a comment.

- Check trivial cases when the acquire_handle attribute is not on an output 
parameter.


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

https://reviews.llvm.org/D70469

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

Index: clang/test/Sema/attr-acquire_handle.c
===
--- /dev/null
+++ clang/test/Sema/attr-acquire_handle.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1  -fsyntax-only -verify %s
+
+void f(int *a __attribute__((acquire_handle)));
+void g(int a __attribute__((acquire_handle))); // expected-error {{attribute only applies to output parameters}}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -9,6 +9,7 @@
 // CHECK-NEXT: AMDGPUWavesPerEU (SubjectMatchRule_function)
 // CHECK-NEXT: AVRSignal (SubjectMatchRule_function)
 // CHECK-NEXT: AbiTag (SubjectMatchRule_record_not_is_union, SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_namespace)
+// CHECK-NEXT: AcquireHandle (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: Alias (SubjectMatchRule_function, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: AlignValue (SubjectMatchRule_variable, SubjectMatchRule_type_alias)
 // CHECK-NEXT: AllocSize (SubjectMatchRule_function)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6518,6 +6518,18 @@
   handleSimpleAttribute(S, D, AL);
 }
 
+static void handeAcquireHandleAttr(Sema , Decl *D, const ParsedAttr ) {
+  // Warn if the parameter is definitely not an output parameter.
+  if (auto *PVD = dyn_cast(D)) {
+if (PVD->getType()->isIntegerType()) {
+  S.Diag(AL.getLoc(), diag::err_attribute_output_parameter)
+  << AL.getRange();
+  return;
+}
+  }
+  handleSimpleAttribute(S, D, AL);
+}
+
 //===--===//
 // Top Level Sema Entry Points
 //===--===//
@@ -7282,6 +7294,16 @@
   case ParsedAttr::AT_ArmMveAlias:
 handleArmMveAliasAttr(S, D, AL);
 break;
+
+  case ParsedAttr::AT_AcquireHandle:
+handeAcquireHandleAttr(S, D, AL);
+break;
+  case ParsedAttr::AT_ReleaseHandle:
+handleSimpleAttribute(S, D, AL);
+break;
+  case ParsedAttr::AT_UseHandle:
+handleSimpleAttribute(S, D, AL);
+break;
   }
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3072,6 +3072,8 @@
 def err_cconv_incomplete_param_type : Error<
   "parameter %0 must have a complete type to use function %1 with the %2 "
   "calling convention">;
+def err_attribute_output_parameter : Error<
+  "attribute only applies to output parameters">;
 
 def ext_cannot_use_trivial_abi : ExtWarn<
   "'trivial_abi' cannot be applied to %0">, InGroup;
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -4467,3 +4467,50 @@
   }
   }];
 }
+
+def AcquireHandleDocs : Documentation {
+  let Category = DocCatDecl;
+  let Content = [{
+If this annotation is on a function it is assumed to return a new handle.
+In case this annotation is on an output parameter, the function is assumed
+to fill the corresponding argument with a new handle.
+
+.. code-block:: c++
+
+  // Output arguments from Zircon.
+  zx_status_t zx_socket_create(uint32_t options,
+   zx_handle_t* out0 [[clang::acquire_handle]],
+   zx_handle_t* out1 [[clang::acquire_handle]]);
+
+
+  // Returned handle.
+  [[clang::acquire_handle]] int open(const char *path, int oflag, ... );
+  }];
+}
+
+def UseHandleDocs : Documentation {
+  let Category = DocCatDecl;
+  let Content = [{
+A function taking a handle by value might close the handle. If a function is
+annotated with `use_handle` it is assumed to not to change the state of the
+handle. It is also assumed to require an open handle to work with.
+
+.. code-block:: c++
+
+  zx_status_t zx_port_wait(zx_handle_t handle [[clang::use_handle]],
+   zx_time_t deadline,
+   

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Derek Schuff via Phabricator via cfe-commits]

dschuff added inline comments.
Herald added a subscriber: ormris.



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";

What would you think about adding a way to pass arguments through to wasm-opt 
on the command line, like we have for the linker, assembler, etc? Something 
like `-Wo,-O2` (or `-Ww` or whatever; analogous to `-Wl` and `-Wa`). That seems 
nicer than an env var, although it doesn't solve the problem of controlling 
whether to run the optimizer in the first place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70500



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

[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

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

xazax.hun marked 3 inline comments as done.
xazax.hun added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:3445
+  let Spellings = [Clang<"acquire_handle">];
+  let Subjects = SubjectList<[Function, ParmVar], ErrorDiag>;
+  let Documentation = [AcquireHandleDocs];

aaron.ballman wrote:
> What about function-like interfaces such as lambdas, blocks, or other 
> callable objects that are not a `FunctionDecl`?
Good point!



Comment at: clang/include/clang/Basic/Attr.td:3450
+def UseHandle : InheritableParamAttr {
+  let Spellings = [Clang<"use_handle">];
+  let Documentation = [UseHandleDocs];

aaron.ballman wrote:
> Should this have a subject limiting it to parameters?
My understanding was that the subject list is "inherited" from 
InheritableParamAttr, which already has it limited to parameters. Is this not 
the case?



Comment at: clang/include/clang/Basic/AttrDocs.td:4474
+  let Content = [{
+If this annotation is on a function it is assumed to return a new handle.
+In case this annotation is on an output parameter, the function is assumed

aaron.ballman wrote:
> What is a "handle"? I think some introduction docs are needed.
Good point. Do you prefer to copy and paste the introduction to all attributes 
or is it enough to only have it for one of them?


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

https://reviews.llvm.org/D70469



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

[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Dan Gohman via Phabricator via cfe-commits]

sunfish created this revision.
sunfish added reviewers: sbc100, dschuff, aheejin.
Herald added subscribers: dexonsmith, steven_wu, hiraditya, jgravelle-google, 
inglorion, aprantl, mehdi_amini.
Herald added a project: clang.

When the WASM_OPT environment variable is set, run the wasm-opt tool to 
optimize LLVM's output. Note that using wasm-opt currently means invalidating 
all debug info, however we can just tell users this when we tell them about 
WASM_OPT. This fixes PR43796.

Also, add an "llvm-lto" directory to the sysroot library search paths, so that 
sysroots can optionally provide LTO-enabled system libraries.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70500

Files:
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/test/Driver/wasm-toolchain-lto.c


Index: clang/test/Driver/wasm-toolchain-lto.c
===
--- /dev/null
+++ clang/test/Driver/wasm-toolchain-lto.c
@@ -0,0 +1,6 @@
+// A basic C link command-line with optimization with known OS and LTO enabled.
+
+// RUN: %clang -### -O2 -flto -no-canonical-prefixes -target wasm32-wasi 
--sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto" 
"-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lc" 
"{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -90,6 +90,28 @@
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(llvm::make_unique(JA, *this, Linker, CmdArgs, Inputs));
+
+  // When optimizing, if WASM_OPT is set, run wasm-opt.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";
+  if (A->getOption().matches(options::OPT_O4) ||
+  A->getOption().matches(options::OPT_Ofast))
+OOpt = "4";
+  else if (A->getOption().matches(options::OPT_O0))
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();
+
+  const char *WasmOpt = Args.MakeArgString(WasmOptPath);
+  ArgStringList CmdArgs;
+  CmdArgs.push_back(Output.getFilename());
+  CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
+  CmdArgs.push_back("-o");
+  CmdArgs.push_back(Output.getFilename());
+  C.addCommand(llvm::make_unique(JA, *this, WasmOpt, CmdArgs, 
Inputs));
+}
+  }
 }
 
 WebAssembly::WebAssembly(const Driver , const llvm::Triple ,
@@ -109,6 +131,11 @@
   } else {
 const std::string MultiarchTriple =
 getMultiarchTriple(getDriver(), Triple, getDriver().SysRoot);
+if (D.isUsingLTO()) {
+  // For LTO, enable use of lto-enabled sysroot libraries too, if 
available.
+  getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple 
+
+   "/llvm-lto");
+}
 getFilePaths().push_back(getDriver().SysRoot + "/lib/" + MultiarchTriple);
   }
 }


Index: clang/test/Driver/wasm-toolchain-lto.c
===
--- /dev/null
+++ clang/test/Driver/wasm-toolchain-lto.c
@@ -0,0 +1,6 @@
+// A basic C link command-line with optimization with known OS and LTO enabled.
+
+// RUN: %clang -### -O2 -flto -no-canonical-prefixes -target wasm32-wasi --sysroot=/foo %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=LINK_OPT_KNOWN %s
+// LINK_OPT_KNOWN: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
+// LINK_OPT_KNOWN: wasm-ld{{.*}}" "-L/foo/lib/wasm32-wasi/llvm-lto" "-L/foo/lib/wasm32-wasi" "crt1.o" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
Index: clang/lib/Driver/ToolChains/WebAssembly.cpp
===
--- clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -90,6 +90,28 @@
   CmdArgs.push_back(Output.getFilename());
 
   C.addCommand(llvm::make_unique(JA, *this, Linker, CmdArgs, Inputs));
+
+  // When optimizing, if WASM_OPT is set, run wasm-opt.
+  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+if (const char *WasmOptPath = getenv("WASM_OPT")) {
+  StringRef OOpt = "s";
+  if (A->getOption().matches(options::OPT_O4) ||
+  A->getOption().matches(options::OPT_Ofast))
+OOpt = "4";
+  else if (A->getOption().matches(options::OPT_O0))
+OOpt = "0";
+  else if (A->getOption().matches(options::OPT_O))
+OOpt = A->getValue();
+
+  const char *WasmOpt = Args.MakeArgString(WasmOptPath);
+  ArgStringList CmdArgs;
+  CmdArgs.push_back(Output.getFilename());
+  CmdArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
+  

[PATCH] D69089: [Parser] #pragma clang transform

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69089



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

[PATCH] D63976: Allow clang -Os and -Oz to work with -flto and lld

[Troy Johnson via Phabricator via cfe-commits]

troyj added a comment.

I ran into this same problem and would like to see this patch or a similar one 
land.  Note that there is also a -Og option to consider, which currently has 
the same problem.


Repository:
  rC Clang

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

https://reviews.llvm.org/D63976



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

[PATCH] D70470: [analyzer] Add FuchsiaHandleCheck to catch handle leaks, use after frees and double frees

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

xazax.hun updated this revision to Diff 230287.
xazax.hun marked 10 inline comments as done.
xazax.hun added a comment.

- Explicitly model "maybe" states.
- Fix some escaping issues.
- Address most review comments.


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

https://reviews.llvm.org/D70470

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
  clang/test/Analysis/fuchsia_handle.c

Index: clang/test/Analysis/fuchsia_handle.c
===
--- /dev/null
+++ clang/test/Analysis/fuchsia_handle.c
@@ -0,0 +1,112 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,fuchsia.FuchsiaHandleChecker -verify %s
+
+typedef __typeof__(sizeof(int)) size_t;
+typedef int zx_status_t;
+typedef __typeof__(sizeof(int)) zx_handle_t;
+typedef unsigned int uint32_t;
+#define NULL ((void *)0)
+
+#if defined(__clang__)
+#define XZ_HANDLE_ACQUIRE  __attribute__((acquire_handle))
+#define XZ_HANDLE_RELEASE  __attribute__((release_handle))
+#define XZ_HANDLE_USE  __attribute__((use_handle))
+#else
+#define XZ_HANDLE_ACQUIRE
+#define XZ_HANDLE_RELEASE
+#define XZ_HANDLE_USE
+#endif
+
+zx_status_t zx_channel_create(
+uint32_t options,
+XZ_HANDLE_ACQUIRE zx_handle_t* out0,
+zx_handle_t* out1 XZ_HANDLE_ACQUIRE);
+
+zx_status_t zx_handle_close(
+zx_handle_t handle XZ_HANDLE_RELEASE);
+
+void escape1(zx_handle_t *in);
+void escape2(zx_handle_t in);
+
+void use1(const zx_handle_t *in XZ_HANDLE_USE);
+void use2(zx_handle_t in XZ_HANDLE_USE);
+
+void checkNoLeak01() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  zx_handle_close(sa);
+  zx_handle_close(sb);
+}
+
+void checkNoLeak02() {
+  zx_handle_t ay[2];
+  zx_channel_create(0, [0], [1]);
+  zx_handle_close(ay[0]);
+  zx_handle_close(ay[1]);
+}
+
+void checkNoLeak03() {
+  zx_handle_t ay[2];
+  zx_channel_create(0, [0], [1]);
+  for (int i = 0; i < 2; i++)
+zx_handle_close(ay[i]);
+}
+
+zx_handle_t checkNoLeak04() {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  zx_handle_close(sa);
+  return sb; // no warning
+}
+
+zx_handle_t checkNoLeak05(zx_handle_t *out1) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  *out1 = sa;
+  return sb; // no warning
+}
+
+void checkNoLeak06() {
+  zx_handle_t sa, sb;
+  if (zx_channel_create(0, , ))
+return;
+  zx_handle_close(sa);
+  zx_handle_close(sb);
+} 
+
+void checkNoLeak07(int tag) {
+  zx_handle_t sa, sb;
+  if (zx_channel_create(0, , ))
+return;
+  escape1();
+  escape2(sb);
+}
+
+void checkNoLeak08(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  use1();
+  if (tag)
+zx_handle_close(sa);
+  use2(sb); // expected-warning {{Potential leak of handle}}
+  zx_handle_close(sb);
+}
+
+void checkDoubleRelease01(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  if (tag)
+zx_handle_close(sa);
+  zx_handle_close(sa); // expected-warning {{Releasing a previously released handle}}
+  zx_handle_close(sb);
+}
+
+void checkUseAfterFree01(int tag) {
+  zx_handle_t sa, sb;
+  zx_channel_create(0, , );
+  if (tag) {
+zx_handle_close(sa);
+use1(); // expected-warning {{Using a previously released handle}}
+  }
+  zx_handle_close(sb);
+  use2(sb); // expected-warning {{Using a previously released handle}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
===
--- /dev/null
+++ clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp
@@ -0,0 +1,469 @@
+//=== FuchsiaHandleChecker.cpp - Find handle leaks/double closes -*- C++ -*--=//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This checker checks if the handle of Fuchsia is properly used according to
+// following rules.
+//   - If a handle is acquired, it should be released before execution
+//ends.
+//   - If a handle is released, it should not be released again.
+//   - If a handle is released, it should not be used for other purposes
+//such as I/O.
+//
+// In this checker, each tracked handle is associated with a state. When the
+// handle variable is passed to different function calls or syscalls, its state
+// changes. The state changes can be generally represented by following ASCII
+// Art:
+//
+//+---+
+//|   |
+//|release_func failed|
+//|   +-+ |
+//| 

[PATCH] D70499: [clang] Fix the path to CrossWinToARMLinux.cmake CMake cache

[Andrei Lebedev via Phabricator via cfe-commits]

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

Thanks for catching and fixing this, Sergej!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70499



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

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Alexey Bader via Phabricator via cfe-commits]

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



Comment at: clang/test/Misc/pragma-attribute-supported-attributes-list.test:134
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)

It looks like this change is not needed anymore. This check fails on my machine 
with the latest version of the patch.

@aaron.ballman, I'm not sure if this is a problem of the implementation or test 
issue.
Do I understand correctly that this test validates the list of the attributes 
which can be applied using `#pragma clang`?
If so, removing this check seems to be okay. We need only 
`[[clang::sycl_kernel]]` or `__attribute__((sycl_kernel))` to work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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

[PATCH] D69810: [OpenCL] Fix address space for base method call (PR43145)

[John McCall via Phabricator via cfe-commits]

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

LGTM.


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

https://reviews.llvm.org/D69810



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

[PATCH] D69855: [clang-tidy] Fix llvm-namespace-comment for macro expansions

[Marcin Twardak via Phabricator via cfe-commits]

twardakm marked 9 inline comments as done.
twardakm added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp:83
+const SourceManager , Preprocessor *PP, Preprocessor *ModuleExpanderPP) 
{
+  PP->addPPCallbacks(std::make_unique(PP, this));
+}

aaron.ballman wrote:
> twardakm wrote:
> > aaron.ballman wrote:
> > > Rather than making an entire new object for PP callbacks, why not make 
> > > `NamespaceCommentCheck` inherit from `PPCallbacks`? It seems like it 
> > > would simplify the interface somewhat.
> > I think the check hast to inherit from ClangTidyCheck?
> > 
> > I duplicated the solution from other checks (e.g. 
> > IdentifierNamingCheck.cpp). Could you please point to some existing check 
> > which implements your idea?
> I don't know if we have other checks doing this -- I was thinking of using 
> multiple inheritance in this case, because the callbacks are effectively a 
> mixin.
> 
> That said, I don't insist on this change.
The problem which I see is that addPPCallbacks takes ownership of the object 
passed to it. I couldn't find any other way of invoking PP callbacks, so I'll 
leave it as is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69855



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

[PATCH] D69855: [clang-tidy] Fix llvm-namespace-comment for macro expansions

[Marcin Twardak via Phabricator via cfe-commits]

twardakm updated this revision to Diff 230285.
twardakm added a comment.

Fix style issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69855

Files:
  clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp
  clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/google-readability-namespace-comments.cpp
  clang-tools-extra/test/clang-tidy/checkers/llvm-namespace-comment.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/llvm-namespace-comment.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/llvm-namespace-comment.cpp
@@ -0,0 +1,41 @@
+// RUN: %check_clang_tidy %s llvm-namespace-comment %t
+
+namespace n1 {
+namespace n2 {
+  void f();
+
+
+  // CHECK-MESSAGES: :[[@LINE+2]]:1: warning: namespace 'n2' not terminated with a closing comment [llvm-namespace-comment]
+  // CHECK-MESSAGES: :[[@LINE+1]]:2: warning: namespace 'n1' not terminated with a closing comment [llvm-namespace-comment]
+}}
+// CHECK-FIXES: } // namespace n2
+// CHECK-FIXES: } // namespace n1
+
+#define MACRO macro_expansion
+namespace MACRO {
+  void f();
+  // CHECK-MESSAGES: :[[@LINE+1]]:1: warning: namespace 'MACRO' not terminated with a closing comment [llvm-namespace-comment]
+}
+// CHECK-FIXES: } // namespace MACRO
+
+namespace MACRO {
+  void g();
+} // namespace MACRO
+
+namespace MACRO {
+  void h();
+  // CHECK-MESSAGES: :[[@LINE+1]]:2: warning: namespace 'MACRO' ends with a comment that refers to an expansion of macro [llvm-namespace-comment]
+} // namespace macro_expansion
+// CHECK-FIXES: } // namespace MACRO
+
+namespace n1 {
+namespace MACRO {
+namespace n2 {
+  void f();
+  // CHECK-MESSAGES: :[[@LINE+3]]:1: warning: namespace 'n2' not terminated with a closing comment [llvm-namespace-comment]
+  // CHECK-MESSAGES: :[[@LINE+2]]:2: warning: namespace 'MACRO' not terminated with a closing comment [llvm-namespace-comment]
+  // CHECK-MESSAGES: :[[@LINE+1]]:3: warning: namespace 'n1' not terminated with a closing comment [llvm-namespace-comment]
+}}}
+// CHECK-FIXES: } // namespace n2
+// CHECK-FIXES: } // namespace MACRO
+// CHECK-FIXES: } // namespace n1
Index: clang-tools-extra/test/clang-tidy/checkers/google-readability-namespace-comments.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/google-readability-namespace-comments.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/google-readability-namespace-comments.cpp
@@ -25,10 +25,10 @@
 // 5
 // 6
 // 7
-// CHECK-MESSAGES: :[[@LINE+2]]:1: warning: namespace 'macro_expansion' not terminated with
-// CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace 'macro_expansion' starts here
+// CHECK-MESSAGES: :[[@LINE+2]]:1: warning: namespace 'MACRO' not terminated with
+// CHECK-MESSAGES: :[[@LINE-10]]:11: note: namespace 'MACRO' starts here
 }
-// CHECK-FIXES: }  // namespace macro_expansion
+// CHECK-FIXES: }  // namespace MACRO
 
 namespace short1 {
 namespace short2 {
Index: clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.h
===
--- clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.h
+++ clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.h
@@ -26,14 +26,29 @@
   NamespaceCommentCheck(StringRef Name, ClangTidyContext *Context);
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  void registerPPCallbacks(const SourceManager , Preprocessor *PP,
+   Preprocessor *ModuleExpanderPP) override;
+
+  void addMacro(const std::string , const std::string ) noexcept;
 
 private:
   void storeOptions(ClangTidyOptions::OptionMap ) override;
+  std::string getNamespaceComment(const NamespaceDecl *ND,
+  bool InsertLineBreak);
+  std::string getNamespaceComment(const std::string ,
+  bool InsertLineBreak);
+  bool isNamespaceMacroDefinition(const StringRef NameSpaceName);
+  std::tuple
+  isNamespaceMacroExpansion(const StringRef NameSpaceName);
 
   llvm::Regex NamespaceCommentPattern;
   const unsigned ShortNamespaceLines;
   const unsigned SpacesBeforeComments;
   llvm::SmallVector Ends;
+
+  // Store macros to verify that warning is not thrown when namespace name is a
+  // preprocessed define.
+  std::vector> Macros;
 };
 
 } // namespace readability
Index: clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/NamespaceCommentCheck.cpp
@@ -19,6 +19,44 @@
 namespace tidy {
 namespace readability {
 
+namespace {

[PATCH] D70499: [clang] Fix the path to CrossWinToARMLinux.cmake CMake cache

[Sergej Jaskiewicz via Phabricator via cfe-commits]

broadwaylamb created this revision.
broadwaylamb added reviewers: vvereschaka, andreil99.
broadwaylamb added a project: clang.
Herald added subscribers: cfe-commits, kristof.beyls, mgorny.

The comment was slightly misleading.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70499

Files:
  clang/cmake/caches/CrossWinToARMLinux.cmake


Index: clang/cmake/caches/CrossWinToARMLinux.cmake
===
--- clang/cmake/caches/CrossWinToARMLinux.cmake
+++ clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -14,7 +14,7 @@
 #   -DDEFAULT_SYSROOT= ^
 #   -DLLVM_AR=/bin/llvm-ar[.exe] ^
 #   -DCMAKE_CXX_FLAGS="-D__OPTIMIZE__" ^
-#   -C/llvm-project/clang/caches/CrossWinToARMLinux.cmake ^
+#   
-C/llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake ^
 #   /llvm-project/llvm
 # Build:
 #  cmake --build . --target install


Index: clang/cmake/caches/CrossWinToARMLinux.cmake
===
--- clang/cmake/caches/CrossWinToARMLinux.cmake
+++ clang/cmake/caches/CrossWinToARMLinux.cmake
@@ -14,7 +14,7 @@
 #   -DDEFAULT_SYSROOT= ^
 #   -DLLVM_AR=/bin/llvm-ar[.exe] ^
 #   -DCMAKE_CXX_FLAGS="-D__OPTIMIZE__" ^
-#   -C/llvm-project/clang/caches/CrossWinToARMLinux.cmake ^
+#   -C/llvm-project/clang/cmake/caches/CrossWinToARMLinux.cmake ^
 #   /llvm-project/llvm
 # Build:
 #  cmake --build . --target install
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D69897: Add #pragma clang loop vectorize_assume_alignment(n)

[Michael Kruse via cfe-commits]

Am Mi., 20. Nov. 2019 um 10:21 Uhr schrieb HAPPY Mahto
:
>> #pragma clang loop vectorize_assume_alignment(32)
>> for(int i = 0;i < n; i++){
>> a[i] = b[i] + i*i;
>> }
>
>  for this all-access inside the loop will be aligned to 32bit,
> ex  IR
>>
>> for.cond: ; preds = %for.inc, %entry
>>   %5 = load i32, i32* %i, align 32, !llvm.access.group !2
>>   %6 = load i32, i32* %n, align 32, !llvm.access.group !2
>>   %cmp = icmp slt i32 %5, %6
>>   br i1 %cmp, label %for.body, label %for.end
>>
>> for.body: ; preds = %for.cond
>>   %7 = load i32, i32* %i, align 32, !llvm.access.group !2
>>   %8 = load i32, i32* %i, align 32, !llvm.access.group !2
>>   %idxprom = sext i32 %8 to i64
>>   %arrayidx = getelementptr inbounds i32, i32* %vla1, i64 %idxprom
>>   store i32 %7, i32* %arrayidx, align 32, !llvm.access.group !2
>>   br label %for.inc
>>
>> for.inc:  ; preds = %for.body
>>   %9 = load i32, i32* %i, align 32, !llvm.access.group !2
>>   %inc = add nsw i32 %9, 1
>>   store i32 %inc, i32* %i, align 32, !llvm.access.group !2
>>   br label %for.cond, !llvm.loop !3
>
> You will not need to create pointers for every array(or operand you want to 
> perform the operation on).

IMHO it is better if the programmer has to. It is not always obvious
which arrays are used in the loop. Also, the information can be used
by other optimzations that the vectorizer.


>>
>> void mult(float* x, int size, float factor){
>>   float* ax = (float*)__builtin_assume_aligned(x, 64);
>>   for (int i = 0; i < size; ++i)
>>  ax[i] *= factor;
>> }

https://godbolt.org/z/Fd6HMe

> the alignment is assumed whereas in #pragma it is set to the number specified.

Semantically, it is the same.

I wonder how you expect the assembly output to change? The
__builtin_assume_aligned, will be picked up by the backend and result
in movaps to be used instead of movups.


> it'll be easier, and having a pragma for doing this will help as it's 
> provided in OMP and intel compilers.

This is a compiler-specific extension. It does not have an influence
on what other compilers do. Even with clang, if you try to do

#pragma clang loop vectorize_assume_alignment(32)
#pragma omp simd
for (int i = 0; i < size; ++i)

clang will silently swallow the vectorize_assume_alignment.


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

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Alexey Bader via Phabricator via cfe-commits]

bader updated this revision to Diff 230281.
bader marked 5 inline comments as done.
bader added a comment.

Applied code review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenSYCL/device-functions.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
  clang/test/SemaSYCL/device-attributes.cpp

Index: clang/test/SemaSYCL/device-attributes.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes.cpp
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+
+[[clang::sycl_kernel]] int gv2 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+__attribute__((sycl_kernel)) int gv3 = 0; // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+__attribute__((sycl_kernel(1))) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel(1)]] void foo2(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+// Only function templates
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+[[clang::sycl_kernel]] void foo1(); // expected-warning {{'sycl_kernel' attribute only applies to function templates}}
+
+// At least two template parameters
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{'sycl_kernel' attribute only applies to a function template with at least two template parameters}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{'sycl_kernel' attribute only applies to a function template with at least two template parameters}}
+
+// Both first two template parameters must be a template type parameter
+template 
+__attribute__((sycl_kernel)) void foo(T P); // expected-warning {{template parameter of a function template with the 'sycl_kernel' attribute must be a template type parameter}}
+template 
+[[clang::sycl_kernel]] void foo1(T P); // expected-warning {{template parameter of a function template with the 'sycl_kernel' attribute must be a template type parameter}}
+
+// Must return void
+template 
+__attribute__((sycl_kernel)) int foo(T P); // expected-warning {{function template with 'sycl_kernel' attribute must have a 'void' return type}}
+template 
+[[clang::sycl_kernel]] int foo1(T P); // expected-warning {{function template with 'sycl_kernel' attribute must have a 'void' return type}}
+
+// Must take at least one argument
+template 
+__attribute__((sycl_kernel)) void foo(); // expected-warning {{function template with 'sycl_kernel' attribute must have a single parameter}}
+template 
+[[clang::sycl_kernel]] void foo1(T t, A a); // expected-warning {{function template with 'sycl_kernel' attribute must have a single parameter}}
+
+// No diagnostics
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
Index: clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/device-attributes-on-non-sycl.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fsycl-is-device -verify %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify -x c++ %s
+
+#ifndef __SYCL_DEVICE_ONLY__
+// expected-warning@+7 {{'sycl_kernel' attribute ignored}}
+// expected-warning@+8 {{'sycl_kernel' attribute ignored}}
+#else
+// expected-no-diagnostics
+#endif
+
+template 
+__attribute__((sycl_kernel)) void foo(T P);
+template 
+[[clang::sycl_kernel]] void foo1(T P);
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -131,6 +131,7 @@
 // CHECK-NEXT: ReturnTypestate (SubjectMatchRule_function, SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: ReturnsNonNull (SubjectMatchRule_objc_method, SubjectMatchRule_function)
 // CHECK-NEXT: ReturnsTwice (SubjectMatchRule_function)
+// CHECK-NEXT: SYCLKernel (SubjectMatchRule_function)
 // CHECK-NEXT: ScopedLockable (SubjectMatchRule_record)
 // CHECK-NEXT: Section (SubjectMatchRule_function, SubjectMatchRule_variable_is_global, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
 // CHECK-NEXT: SetTypestate (SubjectMatchRule_function_is_member)
Index: clang/test/CodeGenSYCL/device-functions.cpp

[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Alexey Bader via Phabricator via cfe-commits]

bader added a subscriber: erichkeane.
bader added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10079-10080
+def warn_sycl_kernel_invalid_template_param_type : Warning<
+  "template parameter of template functions with 'sycl_kernel' attribute must"
+  " be typename">, InGroup;
+def warn_sycl_kernel_num_of_function_params : Warning<

aaron.ballman wrote:
> This diagnostic reads a bit like you cannot do this: `template ` 
> when I think the actual restriction is that you cannot do this: `template 
> `. Is that correct? If so, I think this could be worded as `template 
> parameter of a function template with the 'sycl_kernel' attribute must be a 
> template type parameter`.
> 
> Just double-checking, but you also intend to prohibit template template 
> parameters? e.g., you can't do `template  typename C>`
> This diagnostic reads a bit like you cannot do this: template  when 
> I think the actual restriction is that you cannot do this: template . 
> Is that correct?

Yes. That is correct.

>  If so, I think this could be worded as template parameter of a function 
> template with the 'sycl_kernel' attribute must be a template type parameter.

Thanks! Applied your wording.

> Just double-checking, but you also intend to prohibit template template 
> parameters? e.g., you can't do template  typename C>

Currently we allow following use case: 
https://github.com/intel/llvm/blob/sycl/clang/test/SemaSYCL/mangle-kernel.cpp. 
I assume it qualifies as "template type" and not as "template template" 
parameter. Right?

Quoting SYCL specification $6.2 Naming of kernels 
(https://www.khronos.org/registry/SYCL/specs/sycl-1.2.1.pdf#page=250).

> SYCL kernels are extracted from C++ source files and stored in an 
> implementation- defined format. In the case of
> the shared-source compilation model, the kernels have to be uniquely 
> identified by both host and device compiler.
> This is required in order for the host runtime to be able to load the kernel 
> by using the OpenCL host runtime
> interface.
> From this requirement the following rules apply for naming the kernels:
> • The kernel name is a C++ typename.
> • The kernel needs to have a globally-visible name. In the case of a named 
> function object type, the name can
> be the typename of the function object, as long as it is globally-visible. In 
> the case where it isn’t, a globally visible name has to be provided, as 
> template parameter to the kernel invoking interface, as described in 4.8.5.
> In C++11, lambdas do not have a globally-visible name, so a globally-visible 
> typename has to be provided
> in the kernel invoking interface, as described in 4.8.5.
> • The kernel name has to be a unique identifier in the program.
> 

We also have an extension, which lifts these restrictions/requirements when 
clang is used as host and device compiler. @erichkeane implemented built-in 
function (https://github.com/intel/llvm/pull/250) providing "unique 
identifier", which we use as a kernel name for lambda objects. But this is 
going to be a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60455



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